Skip to content

Commit 7de5120

Browse files
authored
Merge pull request #8963 from Shopify/app-security/support-json-schema-clean
Add typed JSON output to `app security clean`
2 parents 3bc4f28 + e71ac0e commit 7de5120

20 files changed

Lines changed: 755 additions & 75 deletions

‎.changeset/app-security-json.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@shopify/app': minor
3+
'@shopify/cli': minor
4+
---
5+
6+
Add typed JSON output to `app security` commands.

‎docs-shopify.dev/generated/generated_docs_data_v2.json‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3539,9 +3539,18 @@
35393539
"description": "The name of the app configuration.",
35403540
"isOptional": true,
35413541
"environmentValue": "SHOPIFY_FLAG_APP_CONFIG"
3542+
},
3543+
{
3544+
"filePath": "docs-shopify.dev/commands/interfaces/app-security-clean.interface.ts",
3545+
"syntaxKind": "PropertySignature",
3546+
"name": "-j, --json",
3547+
"value": "''",
3548+
"description": "Output the result as JSON. Automatically disables color output.",
3549+
"isOptional": true,
3550+
"environmentValue": "SHOPIFY_FLAG_JSON"
35423551
}
35433552
],
3544-
"value": "export interface appsecurityclean {\n /**\n * Delete every results directory under .shopify/app-security/, not only the selected one.\n *\n */\n '--all'?: ''\n\n /**\n * The Client ID of your app.\n * @environment SHOPIFY_FLAG_CLIENT_ID\n */\n '--client-id <value>'?: string\n\n /**\n * The name of the app configuration.\n * @environment SHOPIFY_FLAG_APP_CONFIG\n */\n '-c, --config <value>'?: string\n\n /**\n * Print the command's JSON schemas.\n * @environment SHOPIFY_FLAG_JSON_SCHEMA\n */\n '--json-schema'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Disable interactive prompts and browser authentication.\n * @environment SHOPIFY_FLAG_NO_INPUT\n */\n '--no-input'?: ''\n\n /**\n * The path to your app directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path <value>'?: string\n\n /**\n * Increase the verbosity of the output. May include sensitive data.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n\n /**\n * Scan --path as an app with no app configuration file. Config checks are skipped. Requires --client-id.\n * @environment SHOPIFY_FLAG_WITHOUT_APP_CONFIG\n */\n '--without-app-config'?: ''\n}"
3553+
"value": "export interface appsecurityclean {\n /**\n * Delete every results directory under .shopify/app-security/, not only the selected one.\n *\n */\n '--all'?: ''\n\n /**\n * The Client ID of your app.\n * @environment SHOPIFY_FLAG_CLIENT_ID\n */\n '--client-id <value>'?: string\n\n /**\n * The name of the app configuration.\n * @environment SHOPIFY_FLAG_APP_CONFIG\n */\n '-c, --config <value>'?: string\n\n /**\n * Output the result as JSON. Automatically disables color output.\n * @environment SHOPIFY_FLAG_JSON\n */\n '-j, --json'?: ''\n\n /**\n * Print the command's JSON schemas.\n * @environment SHOPIFY_FLAG_JSON_SCHEMA\n */\n '--json-schema'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Disable interactive prompts and browser authentication.\n * @environment SHOPIFY_FLAG_NO_INPUT\n */\n '--no-input'?: ''\n\n /**\n * The path to your app directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path <value>'?: string\n\n /**\n * Increase the verbosity of the output. May include sensitive data.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n\n /**\n * Scan --path as an app with no app configuration file. Config checks are skipped. Requires --client-id.\n * @environment SHOPIFY_FLAG_WITHOUT_APP_CONFIG\n */\n '--without-app-config'?: ''\n}"
35453554
}
35463555
},
35473556
"appsecurityinstructions": {
Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
import SecurityClean from './clean.js'
2+
import {appSecurityArtifactPaths} from '../../../services/app-security-artifacts.js'
3+
import {validAppConfiguration} from '../../../services/app-security-selection.test-data.js'
4+
import {Config} from '@oclif/core'
5+
import {fileExists, fileRealPath, inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs'
6+
import {unstyled} from '@shopify/cli-kit/node/output'
7+
import {joinPath} from '@shopify/cli-kit/node/path'
8+
import {withCapturedStandardStreams} from '@shopify/cli-kit/node/testing/output'
9+
import {describe, expect, test, vi} from 'vitest'
10+
// eslint-disable-next-line no-restricted-imports -- Verify native filesystem paths in JSON output.
11+
import {resolve} from 'node:path'
12+
13+
// Skip the installed-CLI error reporting setup, as `check`'s command boundary test does.
14+
vi.mock('@shopify/cli-kit/node/context/local', async (importOriginal) => ({
15+
...(await importOriginal<typeof import('@shopify/cli-kit/node/context/local')>()),
16+
isDevelopment: () => true,
17+
}))
18+
// Command lifecycle telemetry is unrelated to cleaning. Keep the real error handler and renderer.
19+
vi.mock('@shopify/cli-kit/node/analytics', async (importOriginal) => ({
20+
...(await importOriginal<typeof import('@shopify/cli-kit/node/analytics')>()),
21+
reportAnalyticsEvent: vi.fn(),
22+
}))
23+
24+
/** Creates an app whose shopify.app.toml is linked to `clientId`, and returns its real path. */
25+
async function createApp(directory: string, clientId = 'test-client-id'): Promise<string> {
26+
await writeFile(joinPath(directory, 'shopify.app.toml'), validAppConfiguration(clientId))
27+
return fileRealPath(directory)
28+
}
29+
30+
async function writeResults(appDirectory: string, resultsKey: string): Promise<string> {
31+
const paths = appSecurityArtifactPaths(appDirectory, resultsKey)
32+
await mkdir(paths.resultsDirectory)
33+
await writeFile(paths.deterministicFindingsPath, '{}\n')
34+
return paths.resultsDirectory
35+
}
36+
37+
// Boxes wrap text across lines and draw borders, so compare their text with the borders and line breaks removed.
38+
function boxText(output: string): string {
39+
return unstyled(output).replaceAll('│', '').replace(/\s+/g, ' ')
40+
}
41+
42+
async function runCommand(argv: string[]) {
43+
const previousExitCode = process.exitCode
44+
process.exitCode = 0
45+
// Observe the real Oclif error handler's requested exit without terminating the test worker.
46+
const exit = vi.spyOn(process, 'exit').mockImplementation((code) => {
47+
process.exitCode = code ?? 0
48+
return undefined as never
49+
})
50+
try {
51+
const output = await withCapturedStandardStreams(async ({stdout, stderr}) => {
52+
const config = await Config.load(import.meta.url)
53+
// This test invokes the app command directly, not as a separately installed CLI plugin.
54+
config.plugins.clear()
55+
await SecurityClean.run(argv, config)
56+
return {stdout: stdout(), stderr: stderr()}
57+
})
58+
return {...output, exitCode: process.exitCode}
59+
} finally {
60+
exit.mockRestore()
61+
process.exitCode = previousExitCode
62+
}
63+
}
64+
65+
describe('app security clean command boundary', () => {
66+
test('--json removes the selected results directory and writes one result with the selected app', async () => {
67+
await inTemporaryDirectory(async (directory) => {
68+
const appDirectory = await createApp(directory)
69+
const resultsDirectory = await writeResults(appDirectory, 'shopify.app')
70+
const otherResultsDirectory = await writeResults(appDirectory, 'shopify.app.production')
71+
72+
const result = await runCommand(['--path', directory, '--json'])
73+
74+
expect(result.exitCode).toBe(0)
75+
expect(JSON.parse(result.stdout)).toStrictEqual({
76+
status: 'success',
77+
changed: true,
78+
app: {
79+
directory: resolve(appDirectory),
80+
configurationPath: resolve(appDirectory, 'shopify.app.toml'),
81+
clientId: 'test-client-id',
82+
},
83+
resultsDirectories: [resolve(resultsDirectory)],
84+
})
85+
expect(result.stderr).toBe('')
86+
await expect(fileExists(resultsDirectory)).resolves.toBe(false)
87+
await expect(fileExists(otherResultsDirectory)).resolves.toBe(true)
88+
})
89+
})
90+
91+
test('--json with --without-app-config has a null configuration path and the --client-id value', async () => {
92+
await inTemporaryDirectory(async (directory) => {
93+
const appDirectory = await fileRealPath(directory)
94+
const resultsDirectory = await writeResults(appDirectory, 'flag-client-id')
95+
96+
const result = await runCommand([
97+
'--path',
98+
directory,
99+
'--without-app-config',
100+
'--client-id',
101+
'flag-client-id',
102+
'--json',
103+
])
104+
105+
expect(result.exitCode).toBe(0)
106+
expect(JSON.parse(result.stdout)).toStrictEqual({
107+
status: 'success',
108+
changed: true,
109+
app: {directory: resolve(appDirectory), configurationPath: null, clientId: 'flag-client-id'},
110+
resultsDirectories: [resolve(resultsDirectory)],
111+
})
112+
expect(result.stderr).toBe('')
113+
})
114+
})
115+
116+
test('--json has a null client ID when the app configuration is not linked', async () => {
117+
await inTemporaryDirectory(async (directory) => {
118+
const appDirectory = await createApp(directory, '')
119+
await writeResults(appDirectory, 'shopify.app')
120+
121+
const result = await runCommand(['--path', directory, '--json'])
122+
123+
expect(result.exitCode).toBe(0)
124+
expect(JSON.parse(result.stdout)).toMatchObject({
125+
app: {configurationPath: resolve(appDirectory, 'shopify.app.toml'), clientId: null},
126+
})
127+
})
128+
})
129+
130+
test('--all --json removes every results directory and writes a result with only the app directory', async () => {
131+
await inTemporaryDirectory(async (directory) => {
132+
const appDirectory = await createApp(directory)
133+
const productionResultsDirectory = await writeResults(appDirectory, 'shopify.app.production')
134+
const resultsDirectory = await writeResults(appDirectory, 'shopify.app')
135+
136+
const result = await runCommand(['--path', directory, '--all', '--json'])
137+
138+
expect(result.exitCode).toBe(0)
139+
expect(JSON.parse(result.stdout)).toStrictEqual({
140+
status: 'success',
141+
changed: true,
142+
app: {directory: resolve(appDirectory)},
143+
resultsDirectories: [resolve(resultsDirectory), resolve(productionResultsDirectory)],
144+
})
145+
expect(result.stderr).toBe('')
146+
await expect(Promise.all([resultsDirectory, productionResultsDirectory].map(fileExists))).resolves.toEqual([
147+
false,
148+
false,
149+
])
150+
})
151+
})
152+
153+
test('--all --json with nothing to remove writes a result without a change', async () => {
154+
await inTemporaryDirectory(async (directory) => {
155+
const appDirectory = await createApp(directory)
156+
157+
const result = await runCommand(['--path', directory, '--all', '--json'])
158+
159+
expect(result.exitCode).toBe(0)
160+
expect(JSON.parse(result.stdout)).toStrictEqual({
161+
status: 'success',
162+
changed: false,
163+
app: {directory: resolve(appDirectory)},
164+
resultsDirectories: [],
165+
})
166+
expect(result.stderr).toBe('')
167+
})
168+
})
169+
170+
test('--json without a results directory writes one error document with its code and exits 1', async () => {
171+
await inTemporaryDirectory(async (directory) => {
172+
const appDirectory = await createApp(directory)
173+
// The error handler reads --json from the process arguments, not from the arguments given to `run`.
174+
vi.stubEnv('SHOPIFY_FLAG_JSON', '1')
175+
176+
try {
177+
const result = await runCommand(['--path', directory, '--json'])
178+
179+
expect(result.exitCode).toBe(1)
180+
expect(JSON.parse(result.stdout)).toStrictEqual({
181+
error: {
182+
type: 'abort',
183+
message: `No app security check results for shopify.app in ${appDirectory}.`,
184+
code: 'results-directory-not-found',
185+
tryMessage: expect.stringMatching(/^Run `shopify app security check --path .+`\.$/),
186+
},
187+
})
188+
expect(result.stderr).toBe('')
189+
} finally {
190+
vi.unstubAllEnvs()
191+
}
192+
})
193+
})
194+
195+
test('without --json, prints the removed path on stderr and nothing on stdout', async () => {
196+
await inTemporaryDirectory(async (directory) => {
197+
const appDirectory = await createApp(directory)
198+
await writeResults(appDirectory, 'shopify.app')
199+
200+
const result = await runCommand(['--path', directory])
201+
202+
expect(result.exitCode).toBe(0)
203+
expect(result.stdout).toBe('')
204+
expect(boxText(result.stderr)).toContain('App security check results removed.')
205+
})
206+
})
207+
})

‎packages/app/src/cli/commands/app/security/clean.test.ts‎

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ import {appFlags} from '../../../flags.js'
44
import {appSecurityArtifactPaths} from '../../../services/app-security-artifacts.js'
55
import {resolveAppDirectory, resolveAppSecuritySelection} from '../../../services/app-security-selection.js'
66
import {validAppConfiguration} from '../../../services/app-security-selection.test-data.js'
7-
import securityClean, {renderSecurityCleanResult} from '../../../services/security-clean.js'
7+
import securityClean from '../../../services/security-clean.js'
8+
import {renderSecurityCleanResult} from '../../../services/security-clean/result.js'
9+
import {appSecurityCleanJsonOutputSchema} from '../../../services/security-clean/types.js'
810
import AppLinkedCommand from '../../../utilities/app-linked-command.js'
911
import BaseCommand from '@shopify/cli-kit/node/base-command'
12+
import {jsonFlag} from '@shopify/cli-kit/node/cli'
1013
import {AbortError} from '@shopify/cli-kit/node/error'
1114
import {fileRealPath, inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs'
1215
import {cwd, joinPath} from '@shopify/cli-kit/node/path'
@@ -15,6 +18,7 @@ import {describe, expect, test, vi} from 'vitest'
1518
import type {SecurityCleanResult} from '../../../services/security-clean.js'
1619

1720
vi.mock('../../../services/security-clean.js')
21+
vi.mock('../../../services/security-clean/result.js')
1822
vi.mock('../../../services/app-security-selection.js', async (importOriginal) => ({
1923
...(await importOriginal<typeof import('../../../services/app-security-selection.js')>()),
2024
resolveAppSecuritySelection: vi.fn(),
@@ -78,8 +82,11 @@ describe('app security clean command', () => {
7882
test('does not require linked app context', () => {
7983
expect(SecurityClean.prototype).toBeInstanceOf(BaseCommand)
8084
expect(SecurityClean.prototype).not.toBeInstanceOf(AppLinkedCommand)
81-
expect(SecurityClean.flags).not.toHaveProperty('json')
82-
expect(SecurityClean.jsonOutputSchema).toBeUndefined()
85+
})
86+
87+
test('defines --json and exposes the result schema', () => {
88+
expect(SecurityClean.flags.json).toBe(jsonFlag.json)
89+
expect(SecurityClean.jsonOutputSchema).toBe(appSecurityCleanJsonOutputSchema)
8390
})
8491

8592
test('defines the selection flags as check does, except --without-app-config', () => {
@@ -118,12 +125,10 @@ describe('app security clean command', () => {
118125
withoutAppConfig: undefined,
119126
allowPrompts: false,
120127
})
121-
expect(securityClean).toHaveBeenCalledWith({
122-
all: false,
123-
selection: await vi.mocked(resolveAppSecuritySelection).mock.results[0]!.value,
124-
})
128+
const options = {all: false, selection: await vi.mocked(resolveAppSecuritySelection).mock.results[0]!.value}
129+
expect(securityClean).toHaveBeenCalledWith(options)
125130
expect(resolveAppDirectory).not.toHaveBeenCalled()
126-
expect(renderSecurityCleanResult).toHaveBeenCalledWith(result, appDirectory)
131+
expect(renderSecurityCleanResult).toHaveBeenCalledWith(result, options, 'text')
127132
expect(output.info()).toBe('')
128133
} finally {
129134
vi.unstubAllEnvs()
@@ -151,13 +156,33 @@ describe('app security clean command', () => {
151156
withoutAppConfig: undefined,
152157
allowPrompts: false,
153158
})
154-
expect(renderSecurityCleanResult).toHaveBeenCalledWith(result, appDirectory)
159+
expect(renderSecurityCleanResult).toHaveBeenCalledWith(
160+
result,
161+
{all: false, selection: expect.objectContaining({appDirectory})},
162+
'text',
163+
)
155164
} finally {
156165
output.clear()
157166
}
158167
})
159168
})
160169

170+
test.each([['--json'], ['-j']])('presents the result as JSON with %s', async (flag) => {
171+
await inTemporaryDirectory(async (directory) => {
172+
const appDirectory = await createApp(directory)
173+
const result = cleanedResult(appDirectory)
174+
vi.mocked(securityClean).mockResolvedValue(result)
175+
176+
await SecurityClean.run(['--path', directory, flag], import.meta.url)
177+
178+
expect(renderSecurityCleanResult).toHaveBeenCalledWith(
179+
result,
180+
{all: false, selection: expect.objectContaining({appDirectory})},
181+
'json',
182+
)
183+
})
184+
})
185+
161186
test('aborts with "No results found" and removes nothing when the results directory does not exist', async () => {
162187
await inTemporaryDirectory(async (directory) => {
163188
await createApp(directory, {withResults: false})
@@ -189,7 +214,7 @@ describe('app security clean command', () => {
189214
})
190215
expect(resolveAppSecuritySelection).not.toHaveBeenCalled()
191216
expect(securityClean).toHaveBeenCalledWith({all: true, appDirectory})
192-
expect(renderSecurityCleanResult).toHaveBeenCalledWith(result, appDirectory)
217+
expect(renderSecurityCleanResult).toHaveBeenCalledWith(result, {all: true, appDirectory}, 'text')
193218
} finally {
194219
output.clear()
195220
}

‎packages/app/src/cli/commands/app/security/clean.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import {appSecurityCleanSelectionFlags} from './selection-flags.js'
22
import {requireResultsDirectory} from '../../../services/app-security-results.js'
33
import {resolveAppDirectory, resolveAppSecuritySelection} from '../../../services/app-security-selection.js'
4-
import securityClean, {renderSecurityCleanResult} from '../../../services/security-clean.js'
4+
import securityClean from '../../../services/security-clean.js'
5+
import {renderSecurityCleanResult} from '../../../services/security-clean/result.js'
6+
import {appSecurityCleanJsonOutputSchema} from '../../../services/security-clean/types.js'
57
import {Flags} from '@oclif/core'
68
import BaseCommand from '@shopify/cli-kit/node/base-command'
7-
import {globalFlags} from '@shopify/cli-kit/node/cli'
9+
import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli'
810

911
export default class SecurityClean extends BaseCommand {
1012
static summary = 'Remove local app security check results.'
@@ -13,11 +15,16 @@ export default class SecurityClean extends BaseCommand {
1315
1416
Use \`--all\` to delete every results directory under \`.shopify/app-security/\` instead. \`--all\` takes neither \`--config\` nor \`--client-id\`, and with \`--without-app-config\` it doesn't need \`--client-id\`.`
1517

18+
static get jsonOutputSchema() {
19+
return appSecurityCleanJsonOutputSchema
20+
}
21+
1622
static description = this.descriptionForHelp()
1723

1824
static flags = {
1925
...globalFlags,
2026
...appSecurityCleanSelectionFlags,
27+
...jsonFlag,
2128
// Destructive: must be typed, never inherited from the environment.
2229
// eslint-disable-next-line @shopify/cli/command-flags-with-env
2330
all: Flags.boolean({
@@ -41,7 +48,6 @@ Use \`--all\` to delete every results directory under \`.shopify/app-security/\`
4148
if (!options.all) await requireResultsDirectory(options.selection, flags.path)
4249

4350
const result = await securityClean(options)
44-
const appDirectory = options.all ? options.appDirectory : options.selection.appDirectory
45-
renderSecurityCleanResult(result, appDirectory)
51+
renderSecurityCleanResult(result, options, flags.json ? 'json' : 'text')
4652
}
4753
}

‎packages/app/src/cli/services/app-security-artifacts.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,22 @@ describe('cleanAllResultsDirectories', () => {
456456
})
457457
})
458458

459+
test('returns the removed paths sorted by results key', async () => {
460+
await inTemporaryDirectory(async (directory) => {
461+
await writeEveryArtifact(directory, 'shopify.app.staging')
462+
await writeEveryArtifact(directory, 'abc')
463+
await writeEveryArtifact(directory, 'shopify.app')
464+
465+
const removed = await cleanAllResultsDirectories(directory)
466+
467+
expect(removed).toEqual([
468+
joinPath(directory, '.shopify', 'app-security', 'abc'),
469+
joinPath(directory, '.shopify', 'app-security', 'shopify.app'),
470+
joinPath(directory, '.shopify', 'app-security', 'shopify.app.staging'),
471+
])
472+
})
473+
})
474+
459475
test('returns an empty list without creating anything when app-security does not exist', async () => {
460476
await inTemporaryDirectory(async (directory) => {
461477
await expect(cleanAllResultsDirectories(directory)).resolves.toEqual([])

0 commit comments

Comments
 (0)