diff --git a/packages/app/src/cli/commands/app/security/check.integration.test.ts b/packages/app/src/cli/commands/app/security/check.integration.test.ts index be1d78c2135..329a7a6ef56 100644 --- a/packages/app/src/cli/commands/app/security/check.integration.test.ts +++ b/packages/app/src/cli/commands/app/security/check.integration.test.ts @@ -1,8 +1,13 @@ import SecurityCheck from './check.js' +import SecurityInstructions from './instructions.js' import {appSecurityArtifactPaths} from '../../../services/app-security-artifacts.js' +import {appFromIdentifiers} from '../../../services/context.js' import {validAppConfiguration} from '../../../services/app-security-selection.test-data.js' +import {securityCheckJsonOutputSchema} from '../../../services/security-check-json.js' +import {securityInstructionsJsonOutputSchema} from '../../../services/security-instructions-json.js' import {Config} from '@oclif/core' -import {fileRealPath, inTemporaryDirectory} from '@shopify/cli-kit/node/fs' +import {AbortError} from '@shopify/cli-kit/node/error' +import {fileExists, fileRealPath, inTemporaryDirectory} from '@shopify/cli-kit/node/fs' import {unstyled} from '@shopify/cli-kit/node/output' import {joinPath, normalizePath} from '@shopify/cli-kit/node/path' import {describe, expect, test, vi} from 'vitest' @@ -25,6 +30,11 @@ vi.mock('@shopify/cli-kit/node/session', async (importOriginal) => ({ ...(await importOriginal()), setCurrentSessionAlias: vi.fn(), })) +// The --client-id lookup needs a login and the network. The mock finds every client ID unless a test rejects it. +vi.mock('../../../services/context.js', async (importOriginal) => ({ + ...(await importOriginal()), + appFromIdentifiers: vi.fn(), +})) async function createApp(directory: string): Promise<{nestedDirectory: string}> { const routesDirectory = joinPath(directory, 'app', 'routes') @@ -52,7 +62,7 @@ function expectMentionsPath(message: string, path: string): void { expect(message.replaceAll(' ', '')).toContain(path) } -async function runCommand(argv: string[]) { +async function runCommand(argv: string[], command: typeof SecurityCheck | typeof SecurityInstructions = SecurityCheck) { let stdout = '' let stderr = '' const previousExitCode = process.exitCode @@ -76,7 +86,7 @@ async function runCommand(argv: string[]) { const config = await Config.load(import.meta.url) // This test invokes the app command directly, not as a separately installed CLI plugin. config.plugins.clear() - await SecurityCheck.run(argv, config) + await command.run(argv, config) return {stdout, stderr, exitCode: process.exitCode} } finally { warn.mockRestore() @@ -88,6 +98,23 @@ async function runCommand(argv: string[]) { } describe('app security check command boundary', () => { + test('puts the post-scan instructions in the JSON result with --yes, and prints nothing else to stdout', async () => { + await inTemporaryDirectory(async (directory) => { + await createApp(directory) + + const result = await runCommand(['--path', directory, '--json', '--yes']) + + expect(result.exitCode).toBe(0) + const output = JSON.parse(result.stdout) + expect(securityCheckJsonOutputSchema.validate(output)).toEqual(output) + expect(output.instructions).toEqual({ + content: expect.stringContaining('Use the existing scan results'), + copiedToClipboard: false, + path: null, + }) + }) + }) + test('scans an app from a nested directory and writes deterministic-findings.json and agent-checks.json', async () => { await inTemporaryDirectory(async (directory) => { const {nestedDirectory} = await createApp(directory) @@ -98,17 +125,25 @@ describe('app security check command boundary', () => { expect(result.exitCode).toBe(0) const output = JSON.parse(result.stdout) - expect(Object.keys(output).sort()).toEqual(['agent_checks_path', 'deterministic_findings', 'engine', 'selection']) - expect(output.agent_checks_path).toBe(paths.agentChecksPath) + expect(securityCheckJsonOutputSchema.validate(output)).toEqual(output) + expect(Object.keys(output).sort()).toEqual([ + 'agentChecksPath', + 'deterministicFindings', + 'instructions', + 'selection', + 'status', + ]) + expect(output.status).toBe('success') + expect(output.agentChecksPath).toBe(paths.agentChecksPath) + expect(output.instructions).toBeNull() expect(output.selection).toEqual({ - app_directory: appDirectory, - app_config_file: joinPath(appDirectory, 'shopify.app.toml'), - client_id: 'test-client-id', - client_id_source: 'config', - scan_directories: [{directory: appDirectory, origin: 'app_directory'}], + directory: appDirectory, + configPath: joinPath(appDirectory, 'shopify.app.toml'), + clientId: 'test-client-id', + clientIdSource: 'config', + scanDirectories: [{directory: appDirectory, origin: 'app-directory'}], }) - expect(output.engine).toMatchObject({name: 'shopify-app-security'}) - await expect(readJson(paths.deterministicFindingsPath)).resolves.toEqual(output.deterministic_findings) + await expect(readJson(paths.deterministicFindingsPath)).resolves.toEqual(output.deterministicFindings) await expect(readJson(paths.deterministicFindingsPath)).resolves.toMatchObject({ schema_version: 1, source: 'deterministic', @@ -141,7 +176,7 @@ describe('app security check command boundary', () => { ]) expect(result.exitCode).toBe(0) - expect(JSON.parse(result.stdout).agent_checks_path).toBe(paths.agentChecksPath) + expect(JSON.parse(result.stdout).agentChecksPath).toBe(paths.agentChecksPath) await expect(readJson(paths.deterministicFindingsPath)).resolves.toMatchObject({source: 'deterministic'}) await expect( readFile(appSecurityArtifactPaths(appDirectory, 'shopify.app').agentChecksPath), @@ -170,10 +205,10 @@ describe('app security check command boundary', () => { expect(result.exitCode).toBe(0) expect(JSON.parse(result.stdout).selection).toMatchObject({ - app_directory: appDirectory, - app_config_file: null, - client_id: 'configless-client-id', - client_id_source: 'flag', + directory: appDirectory, + configPath: null, + clientId: 'configless-client-id', + clientIdSource: 'flag', }) const deterministicFindings = await readJson(paths.deterministicFindingsPath) expect(deterministicFindings).toMatchObject({ @@ -262,4 +297,75 @@ describe('app security check command boundary', () => { await expect(readFile(paths.deterministicFindingsPath)).rejects.toMatchObject({code: 'ENOENT'}) }) }) + + test.each([[['--skip-instructions']], [['--list-files']]])( + 'aborts on an unknown --client-id before writing or listing anything (%j)', + async (modeFlags) => { + await inTemporaryDirectory(async (directory) => { + await createApp(directory) + const paths = appSecurityArtifactPaths(await fileRealPath(directory), 'unknown-client-id') + vi.mocked(appFromIdentifiers).mockRejectedValue(new AbortError('No app with client ID unknown-client-id found')) + + const result = await runCommand(['--path', directory, '--client-id', 'unknown-client-id', ...modeFlags]) + + expect(result.exitCode).toBe(1) + expect(errorText(result.stderr)).toContain('No app with client ID unknown-client-id found') + expect(result.stdout).toBe('') + expect(appFromIdentifiers).toHaveBeenCalledWith({apiKey: 'unknown-client-id', offerReset: false}) + await expect(fileExists(paths.resultsDirectory)).resolves.toBe(false) + }) + }, + ) + + test('looks up an empty --client-id= before listing anything', async () => { + await inTemporaryDirectory(async (directory) => { + await createApp(directory) + vi.mocked(appFromIdentifiers).mockRejectedValue(new AbortError('No app with client ID found')) + + const result = await runCommand(['--path', directory, '--client-id=', '--list-files']) + + expect(result.exitCode).toBe(1) + expect(appFromIdentifiers).toHaveBeenCalledWith({apiKey: '', offerReset: false}) + expect(result.stdout).toBe('') + }) + }) + + test('looks up --client-id, not the TOML client ID', async () => { + await inTemporaryDirectory(async (directory) => { + await createApp(directory) + + await runCommand(['--path', directory, '--client-id', 'other-client-id', '--json', '--skip-instructions']) + await runCommand(['--path', directory, '--json', '--skip-instructions']) + + expect(appFromIdentifiers).toHaveBeenCalledOnce() + expect(appFromIdentifiers).toHaveBeenCalledWith({apiKey: 'other-client-id', offerReset: false}) + }) + }) +}) + +describe('app security instructions command boundary', () => { + test('prints only the JSON result with --json --write, and writes the same instructions to the file', async () => { + await inTemporaryDirectory(async (directory) => { + await createApp(directory) + // instructions needs the results directory that check creates. + await runCommand(['--path', directory, '--json', '--skip-instructions']) + const instructionsPath = joinPath(directory, 'handoff.md') + + const result = await runCommand( + ['--path', directory, '--json', '--write', instructionsPath], + SecurityInstructions, + ) + + expect(result.exitCode).toBe(0) + const output = JSON.parse(result.stdout) + expect(securityInstructionsJsonOutputSchema.validate(output)).toEqual(output) + expect(output.instructions).toEqual({ + content: expect.stringContaining('Run the scan'), + copiedToClipboard: false, + path: instructionsPath, + }) + await expect(readFile(instructionsPath, 'utf8')).resolves.toBe(`${output.instructions.content}\n`) + expect(result.stderr).not.toContain('Wrote app security check instructions') + }) + }) }) diff --git a/packages/app/src/cli/commands/app/security/check.test.ts b/packages/app/src/cli/commands/app/security/check.test.ts index 5c10c47d6b5..72668fde23f 100644 --- a/packages/app/src/cli/commands/app/security/check.test.ts +++ b/packages/app/src/cli/commands/app/security/check.test.ts @@ -1,14 +1,38 @@ import SecurityCheck from './check.js' import {appFlags} from '../../../flags.js' -import securityCheck from '../../../services/security-check.js' +import securityCheck, {resolveSecurityCheckSelection} from '../../../services/security-check.js' +import {securityCheckJsonOutputSchema} from '../../../services/security-check-json.js' +import {renderSecurityCheckPromptsNotice, renderSecurityCheckResult} from '../../../services/security-output.js' import AppLinkedCommand from '../../../utilities/app-linked-command.js' import BaseCommand from '@shopify/cli-kit/node/base-command' import {globalFlags} from '@shopify/cli-kit/node/cli' import {resolvePath} from '@shopify/cli-kit/node/path' +import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system' import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output' import {describe, expect, test, vi} from 'vitest' -vi.mock('../../../services/security-check.js') +vi.mock('../../../services/security-check.js', () => ({ + resolveSecurityCheckSelection: vi.fn(async () => ({kind: 'resolved', prompted: false, commands: {}})), + default: vi.fn(async (resolution: unknown) => ({ + kind: 'file-list', + resolution, + paths: [], + ignoredScanDirectories: [], + })), +})) +vi.mock('../../../services/security-output.js') +vi.mock('@shopify/cli-kit/node/system', async (importOriginal) => ({ + ...(await importOriginal()), + terminalSupportsPrompting: vi.fn(() => false), +})) + +function selectionOptions() { + return vi.mocked(resolveSecurityCheckSelection).mock.calls[0]![0] +} + +function renderOptions() { + return vi.mocked(renderSecurityCheckResult).mock.calls[0]![1] +} describe('app security check command', () => { test('is hidden and does not require linked app context', () => { @@ -45,20 +69,26 @@ describe('app security check command', () => { import.meta.url, ) - expect(securityCheck).toHaveBeenCalledWith({ + expect(selectionOptions()).toEqual({ directory: resolvePath('./fixtures/unlinked-app'), configName: undefined, clientId: undefined, withoutAppConfig: false, - json: true, + includeDirs: [], + excludePatterns: [], + noGitIgnore: false, + allowPrompts: false, + }) + const resolution = await vi.mocked(resolveSecurityCheckSelection).mock.results[0]!.value + expect(securityCheck).toHaveBeenCalledWith(resolution, {listFiles: false}) + const result = await vi.mocked(securityCheck).mock.results[0]!.value + expect(renderSecurityCheckResult).toHaveBeenCalledWith(result, { + format: 'json', verbose: true, blocking: 'high', yes: false, skipInstructions: true, - includeDirs: [], - excludePatterns: [], - noGitIgnore: false, - listFiles: false, + canPrompt: false, }) }) @@ -68,9 +98,8 @@ describe('app security check command', () => { import.meta.url, ) - expect(securityCheck).toHaveBeenCalledWith( - expect.objectContaining({excludePatterns: ['generated', '../shared/**', 'a b/'], skipInstructions: true}), - ) + expect(selectionOptions()).toMatchObject({excludePatterns: ['generated', '../shared/**', 'a b/']}) + expect(renderOptions()).toMatchObject({skipInstructions: true}) }) test('forwards repeated --include-dir values exactly as typed, without resolving them', async () => { @@ -79,15 +108,13 @@ describe('app security check command', () => { import.meta.url, ) - expect(securityCheck).toHaveBeenCalledWith( - expect.objectContaining({includeDirs: ['../backend', './lib/', 'a b'], skipInstructions: true}), - ) + expect(selectionOptions()).toMatchObject({includeDirs: ['../backend', './lib/', 'a b']}) }) test('forwards --no-git-ignore', async () => { await SecurityCheck.run(['--no-git-ignore', '--skip-instructions'], import.meta.url) - expect(securityCheck).toHaveBeenCalledWith(expect.objectContaining({noGitIgnore: true, skipInstructions: true})) + expect(selectionOptions()).toMatchObject({noGitIgnore: true}) }) test('reads --include-dir and --exclude only from the command line', () => { @@ -100,7 +127,8 @@ describe('app security check command', () => { test('forwards --list-files, which is also set by its environment variable', async () => { await SecurityCheck.run(['--list-files', '--json'], import.meta.url) - expect(securityCheck).toHaveBeenCalledWith(expect.objectContaining({listFiles: true, json: true})) + expect(securityCheck).toHaveBeenCalledWith(expect.anything(), {listFiles: true}) + expect(renderOptions()).toMatchObject({format: 'json'}) expect(SecurityCheck.flags['list-files'].env).toBe('SHOPIFY_FLAG_LIST_FILES') }) @@ -120,29 +148,30 @@ describe('app security check command', () => { test('forwards --yes without requiring an app configuration', async () => { await SecurityCheck.run(['--path', '/tmp/directory-without-shopify-toml', '--yes'], import.meta.url) - expect(securityCheck).toHaveBeenCalledWith({ + expect(selectionOptions()).toEqual({ directory: '/tmp/directory-without-shopify-toml', configName: undefined, clientId: undefined, withoutAppConfig: false, - json: false, + includeDirs: [], + excludePatterns: [], + noGitIgnore: false, + allowPrompts: false, + }) + expect(renderOptions()).toEqual({ + format: 'text', verbose: false, blocking: 'none', yes: true, skipInstructions: false, - includeDirs: [], - excludePatterns: [], - noGitIgnore: false, - listFiles: false, + canPrompt: false, }) }) test('forwards --client-id and --without-app-config', async () => { await SecurityCheck.run(['--without-app-config', '--client-id', 'abc123', '--skip-instructions'], import.meta.url) - expect(securityCheck).toHaveBeenCalledWith( - expect.objectContaining({clientId: 'abc123', withoutAppConfig: true, skipInstructions: true}), - ) + expect(selectionOptions()).toMatchObject({clientId: 'abc123', withoutAppConfig: true}) }) test.each([ @@ -158,7 +187,7 @@ describe('app security check command', () => { 'process.exit unexpectedly called with "1"', ) expect(outputMock.error()).toContain(expectedFlag) - expect(securityCheck).not.toHaveBeenCalled() + expect(resolveSecurityCheckSelection).not.toHaveBeenCalled() } finally { consoleErrorSpy.mockRestore() outputMock.clear() @@ -171,7 +200,7 @@ describe('app security check command', () => { import.meta.url, ) - expect(securityCheck).toHaveBeenCalledWith(expect.objectContaining({configName: 'staging', skipInstructions: true})) + expect(selectionOptions()).toMatchObject({configName: 'staging'}) }) test.each(['--findings', '--clean'])('rejects the removed %s flag', async (removedFlag) => { @@ -217,9 +246,84 @@ describe('app security check command', () => { expect(SecurityCheck.descriptionWithMarkdown).not.toContain('--ignore') }) - test('allows --yes in JSON mode while preserving non-interactive output behavior', async () => { + test('allows --yes in JSON mode', async () => { await SecurityCheck.run(['--json', '--yes'], import.meta.url) - expect(securityCheck).toHaveBeenCalledWith(expect.objectContaining({json: true, yes: true})) + expect(renderOptions()).toMatchObject({format: 'json', yes: true}) + }) + + test.each([[[]], [['--json']]])( + 'lets the selection and the instructions prompt in an interactive terminal (%j)', + async (flags) => { + vi.mocked(terminalSupportsPrompting).mockReturnValue(true) + + await SecurityCheck.run(flags, import.meta.url) + + expect(selectionOptions()).toMatchObject({allowPrompts: true}) + expect(renderOptions()).toMatchObject({canPrompt: true}) + }, + ) + + test('never prompts with --list-files, even in an interactive terminal', async () => { + vi.mocked(terminalSupportsPrompting).mockReturnValue(true) + + await SecurityCheck.run(['--list-files'], import.meta.url) + + expect(selectionOptions()).toMatchObject({allowPrompts: false}) + expect(renderOptions()).toMatchObject({canPrompt: false}) + }) + + test.each([ + ['text', []], + ['json', ['--json']], + ])('shows how to skip the prompts after a selection that prompted, before scanning (%s)', async (format, flags) => { + const commands = {scan: {args: ['app', 'security', 'check']}} + vi.mocked(resolveSecurityCheckSelection).mockResolvedValueOnce({ + kind: 'resolved', + prompted: true, + commands, + } as unknown as Awaited>) + + await SecurityCheck.run(flags, import.meta.url) + + expect(renderSecurityCheckPromptsNotice).toHaveBeenCalledWith(commands, format) + expect(vi.mocked(renderSecurityCheckPromptsNotice).mock.invocationCallOrder[0]).toBeLessThan( + vi.mocked(securityCheck).mock.invocationCallOrder[0]!, + ) + }) + + test.each([ + ['text', []], + ['json', ['--json']], + ])('presents a cancelled run without scanning or showing how to skip the prompts (%s)', async (format, flags) => { + vi.mocked(resolveSecurityCheckSelection).mockResolvedValueOnce({kind: 'cancelled'}) + + await SecurityCheck.run(flags, import.meta.url) + + expect(securityCheck).not.toHaveBeenCalled() + expect(renderSecurityCheckPromptsNotice).not.toHaveBeenCalled() + expect(renderSecurityCheckResult).toHaveBeenCalledWith({kind: 'cancelled'}, expect.objectContaining({format})) + }) + + test('does not show how to skip prompts that were not shown', async () => { + await SecurityCheck.run([], import.meta.url) + + expect(renderSecurityCheckPromptsNotice).not.toHaveBeenCalled() + }) + + test('exposes its JSON result schema', () => { + expect(SecurityCheck.jsonOutputSchema).toBe(securityCheckJsonOutputSchema) + expect(SecurityCheck.description).toContain('AppSecurityCheckResult') + }) + + test('documents its prompts, that --no-input turns them off, and that --client-id can need a login', () => { + expect(SecurityCheck.descriptionWithMarkdown).toContain('can also ask which app configuration to scan') + expect(SecurityCheck.descriptionWithMarkdown).toContain('pick or create the app') + expect(SecurityCheck.descriptionWithMarkdown).toContain('pass `--no-input` to turn every prompt off') + expect(SecurityCheck.descriptionWithMarkdown).toContain( + 'A choice the command would have asked for then becomes an error', + ) + expect(SecurityCheck.descriptionWithMarkdown).toContain('which can require you to log in') + expect(SecurityCheck.descriptionWithMarkdown).not.toMatch(/never prompts/) }) }) diff --git a/packages/app/src/cli/commands/app/security/check.ts b/packages/app/src/cli/commands/app/security/check.ts index 8a8140e2a7e..11de091e64c 100644 --- a/packages/app/src/cli/commands/app/security/check.ts +++ b/packages/app/src/cli/commands/app/security/check.ts @@ -1,9 +1,16 @@ import {appSecurityBlockingFlag} from './blocking-flag.js' import {appSecuritySelectionFlags} from './selection-flags.js' -import securityCheck from '../../../services/security-check.js' +import securityCheck, {resolveSecurityCheckSelection} from '../../../services/security-check.js' +import {securityCheckJsonOutputSchema} from '../../../services/security-check-json.js' +import { + renderSecurityCheckPromptsNotice, + renderSecurityCheckResult, + type SecurityCheckRenderOptions, +} from '../../../services/security-output.js' import {Flags} from '@oclif/core' import BaseCommand from '@shopify/cli-kit/node/base-command' import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli' +import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system' export default class SecurityCheck extends BaseCommand { static hidden = true @@ -13,17 +20,23 @@ export default class SecurityCheck extends BaseCommand { static descriptionWithMarkdown = `Runs an app security check locally and writes \`deterministic-findings.json\` and \`agent-checks.json\` to the results directory, \`.shopify/app-security//\`. The results key is \`--client-id\` when you pass it, and otherwise the name of the app configuration file without \`.toml\`; the other \`app security\` commands take the same selection flags and find the same directory. Every run replaces both files, so it's always safe to run the check again. -\`deterministic-findings.json\` holds the deterministic scan results. \`agent-checks.json\` holds the checks for your coding agent to investigate; the agent's results are recorded with \`shopify app security record\`. Use \`--config\` to select a specific app configuration when the project has multiple \`shopify.app*.toml\` files; the check inspects only that configuration. Use \`--client-id\` to replace the configuration's client ID for this run. When no app configuration exists, use \`--without-app-config --client-id \` to scan \`--path\` anyway with config checks skipped; in an interactive terminal the command offers to do this. +\`deterministic-findings.json\` holds the deterministic scan results. \`agent-checks.json\` holds the checks for your coding agent to investigate; the agent's results are recorded with \`shopify app security record\`. Use \`--config\` to select a specific app configuration when the project has multiple \`shopify.app*.toml\` files; the check inspects only that configuration. Use \`--client-id\` to replace the configuration's client ID for this run. \`--client-id\` is checked against your Shopify account before anything is scanned, so it needs you to be logged in. When no app configuration exists, use \`--without-app-config --client-id \` to scan \`--path\` anyway with config checks skipped; in an interactive terminal the command offers to do this. The check scans the app directory and each \`--include-dir\`. Git ignore rules apply by default: a file or directory that Git ignores is skipped, using the rules of the repository that contains it, while files that Git tracks are always scanned. Use \`--no-git-ignore\` to turn Git ignore rules off for every scanned directory. Use \`--exclude\` to skip more paths. Each value is a glob that is matched against the path relative to the working directory, so a path above it starts with \`../\`, and a name at any depth needs \`**/\`, for example \`--exclude '**/generated'\`. Repeat the flag to add globs. An exclusion can't remove the selected app configuration file. Quote each value so your shell doesn't expand \`*\`. The coding-agent instructions this check offers repeat the globs. Other \`app security\` commands don't take \`--exclude\` or \`--no-git-ignore\`, so pass the same flags each time you run the check. -Use \`--list-files\` to check the scope before scanning: it prints the files the check would gather, one path per line and relative to the app directory (\`{"files": [...]}\` with \`--json\`), and then stops. It writes no results and never prompts. \`--client-id\` is accepted but has no effect on the list. +Use \`--list-files\` to check the scope before scanning: it prints the files the check would gather, one path per line and relative to the app directory, and then stops. It writes no results and shows no prompts. \`--client-id\` is still checked, which can require you to log in, but doesn't change the list. -In interactive terminals, the command offers to copy the coding-agent instructions, print them, or choose nothing; copying is the default. In CI and other non-interactive environments, instructions aren't offered unless you pass \`--yes\`, which prints them. JSON output never prompts or prints those instructions. You can also run \`shopify app security instructions\` to print, copy, or write them later.` +In interactive terminals, the command offers to copy the coding-agent instructions, print them, or choose nothing; copying is the default. In CI and other non-interactive environments, instructions aren't offered unless you pass \`--yes\`, which prints them. You can also run \`shopify app security instructions\` to print, copy, or write them later. - static description = this.descriptionWithoutMarkdown() +The command can also ask which app configuration to scan, or offer to scan without one and then ask you to pick or create the app. In automation, pass \`--no-input\` to turn every prompt off. A choice the command would have asked for then becomes an error, so pass \`--config\`, or \`--without-app-config --client-id \`, instead; a \`--client-id\` that needs a login fails instead of opening the browser.` + + static get jsonOutputSchema() { + return securityCheckJsonOutputSchema + } + + static description = this.descriptionForHelp() static flags = { ...globalFlags, @@ -71,21 +84,35 @@ In interactive terminals, the command offers to copy the coding-agent instructio public async run(): Promise { const {flags} = await this.parse(SecurityCheck) + const format = flags.json ? 'json' : 'text' + const listFiles = Boolean(flags['list-files']) + const canPrompt = !listFiles && terminalSupportsPrompting() + const renderOptions: SecurityCheckRenderOptions = { + format, + verbose: Boolean(flags.verbose), + blocking: flags.blocking, + yes: flags.yes, + skipInstructions: flags['skip-instructions'], + canPrompt, + } - await securityCheck({ + const resolution = await resolveSecurityCheckSelection({ directory: flags.path, configName: flags.config, clientId: flags['client-id'], withoutAppConfig: Boolean(flags['without-app-config']), - json: flags.json, - verbose: Boolean(flags.verbose), - blocking: flags.blocking, - yes: flags.yes, - skipInstructions: flags['skip-instructions'], includeDirs: flags['include-dir'] ?? [], excludePatterns: flags.exclude ?? [], noGitIgnore: Boolean(flags['no-git-ignore']), - listFiles: Boolean(flags['list-files']), + allowPrompts: canPrompt, }) + if (resolution.kind === 'cancelled') { + await renderSecurityCheckResult(resolution, renderOptions) + return + } + if (resolution.prompted) renderSecurityCheckPromptsNotice(resolution.commands, format) + + const result = await securityCheck(resolution, {listFiles}) + await renderSecurityCheckResult(result, renderOptions) } } diff --git a/packages/app/src/cli/commands/app/security/clean.test.ts b/packages/app/src/cli/commands/app/security/clean.test.ts index 04de13ae9ba..6f2895c421d 100644 --- a/packages/app/src/cli/commands/app/security/clean.test.ts +++ b/packages/app/src/cli/commands/app/security/clean.test.ts @@ -3,10 +3,12 @@ import SecurityCheck from './check.js' import {appFlags} from '../../../flags.js' import {appSecurityArtifactPaths} from '../../../services/app-security-artifacts.js' import {resolveAppDirectory, resolveAppSecuritySelection} from '../../../services/app-security-selection.js' +import {validAppConfiguration} from '../../../services/app-security-selection.test-data.js' import securityClean, {renderSecurityCleanResult} from '../../../services/security-clean.js' import {securityCleanJsonOutputSchema} from '../../../services/security-clean-json.js' import AppLinkedCommand from '../../../utilities/app-linked-command.js' import BaseCommand from '@shopify/cli-kit/node/base-command' +import {AbortError} from '@shopify/cli-kit/node/error' import {fileRealPath, inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs' import {cwd, joinPath} from '@shopify/cli-kit/node/path' import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output' @@ -37,6 +39,25 @@ async function createApp(directory: string, {withResults = true} = {}): Promise< return appDirectory } +/** Makes the mocked resolver run the real one, with a client ID lookup that fails for every client ID. */ +async function resolveWithFailingLookUp() { + const actual = await vi.importActual( + '../../../services/app-security-selection.js', + ) + const lookUpApp = vi.fn(async (clientId: string) => { + throw new AbortError(`No app with client ID ${clientId} found`) + }) + vi.mocked(resolveAppSecuritySelection).mockImplementation((options) => + actual.resolveAppSecuritySelection(options, { + confirmScanWithoutAppConfig: async () => true, + pickClientId: async () => 'picked-client-id', + pickConfigFile: async () => 'shopify.app.toml', + lookUpApp, + }), + ) + return lookUpApp +} + /** Runs the command expecting it to fail, and returns what it printed to stderr. */ async function runRejected(argv: string[]): Promise { const output = mockAndCaptureOutput() @@ -222,4 +243,27 @@ describe('app security clean command', () => { expect(securityClean).not.toHaveBeenCalled() }) }) + + test('does not look up --client-id, so results saved under a mistyped client ID can be cleaned', async () => { + await inTemporaryDirectory(async (directory) => { + const appDirectory = await fileRealPath(directory) + await writeFile(joinPath(appDirectory, 'shopify.app.toml'), validAppConfiguration('toml-client-id')) + await mkdir(appSecurityArtifactPaths(appDirectory, 'mistyped-client-id').resultsDirectory) + const lookUpApp = await resolveWithFailingLookUp() + vi.mocked(securityClean).mockResolvedValue(cleanedResult(appDirectory)) + const output = mockAndCaptureOutput() + + try { + await SecurityClean.run(['--path', directory, '--client-id', 'mistyped-client-id', '--json'], import.meta.url) + + expect(lookUpApp).not.toHaveBeenCalled() + expect(securityClean).toHaveBeenCalledWith({ + all: false, + selection: expect.objectContaining({clientIdOverride: 'mistyped-client-id'}), + }) + } finally { + output.clear() + } + }) + }) }) diff --git a/packages/app/src/cli/commands/app/security/instructions.test.ts b/packages/app/src/cli/commands/app/security/instructions.test.ts index dad8e3d9545..fa7052b3786 100644 --- a/packages/app/src/cli/commands/app/security/instructions.test.ts +++ b/packages/app/src/cli/commands/app/security/instructions.test.ts @@ -3,16 +3,26 @@ import SecurityCheck from './check.js' import {appFlags} from '../../../flags.js' import {appSecurityArtifactPaths} from '../../../services/app-security-artifacts.js' import {resolveAppSecurityCommands} from '../../../services/app-security-commands.js' -import deliverAppSecurityInstructions from '../../../services/app-security-instructions.js' +import {appSecurityInstructions} from '../../../services/app-security-instructions.js' +import { + deliverAppSecurityInstructions, + renderAppSecurityInstructions, +} from '../../../services/app-security-instructions-output.js' import {resolveAppSecuritySelection, type AppSecuritySelection} from '../../../services/app-security-selection.js' +import {validAppConfiguration} from '../../../services/app-security-selection.test-data.js' +import {securityInstructionsJsonOutputSchema} from '../../../services/security-instructions-json.js' import AppLinkedCommand from '../../../utilities/app-linked-command.js' import BaseCommand from '@shopify/cli-kit/node/base-command' -import {fileRealPath, inTemporaryDirectory, mkdir} from '@shopify/cli-kit/node/fs' +import {AbortError} from '@shopify/cli-kit/node/error' +import {fileRealPath, inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs' import {cwd, joinPath, resolvePath} from '@shopify/cli-kit/node/path' import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output' import {describe, expect, test, vi} from 'vitest' -vi.mock('../../../services/app-security-instructions.js') +vi.mock('../../../services/app-security-instructions.js', () => ({ + appSecurityInstructions: vi.fn(() => '# Instructions'), +})) +vi.mock('../../../services/app-security-instructions-output.js') vi.mock('../../../services/app-security-selection.js', async (importOriginal) => ({ ...(await importOriginal()), resolveAppSecuritySelection: vi.fn(), @@ -39,6 +49,25 @@ async function createApp( return appDirectory } +/** Makes the mocked resolver run the real one, with a client ID lookup that fails for every client ID. */ +async function resolveWithFailingLookUp() { + const actual = await vi.importActual( + '../../../services/app-security-selection.js', + ) + const lookUpApp = vi.fn(async (clientId: string) => { + throw new AbortError(`No app with client ID ${clientId} found`) + }) + vi.mocked(resolveAppSecuritySelection).mockImplementation((options) => + actual.resolveAppSecuritySelection(options, { + confirmScanWithoutAppConfig: async () => true, + pickClientId: async () => 'picked-client-id', + pickConfigFile: async () => 'shopify.app.toml', + lookUpApp, + }), + ) + return lookUpApp +} + function configSelection(appDirectory: string, configFileName: string): AppSecuritySelection { return {kind: 'config', appDirectory, appConfigFilePath: joinPath(appDirectory, configFileName)} } @@ -49,6 +78,8 @@ describe('app security instructions command', () => { expect(SecurityInstructions.prototype).toBeInstanceOf(BaseCommand) expect(SecurityInstructions.prototype).not.toBeInstanceOf(AppLinkedCommand) expect(SecurityInstructions.args).not.toHaveProperty('directory') + expect(SecurityInstructions.flags).toHaveProperty('json') + expect(SecurityInstructions.jsonOutputSchema).toBe(securityInstructionsJsonOutputSchema) }) test('defines the selection flags as check does', () => { @@ -71,13 +102,84 @@ describe('app security instructions command', () => { withoutAppConfig: undefined, allowPrompts: false, }) - expect(deliverAppSecurityInstructions).toHaveBeenCalledWith({ + expect(appSecurityInstructions).toHaveBeenCalledWith({ appDirectory, resultsKey: 'shopify.app', commands: resolveAppSecurityCommands(configSelection(appDirectory, 'shopify.app.toml'), cwd()), - copy: false, - writePath: undefined, }) + expect(deliverAppSecurityInstructions).toHaveBeenCalledWith('# Instructions', {copy: false, writePath: undefined}) + expect(renderAppSecurityInstructions).toHaveBeenCalledWith({ + content: '# Instructions', + copiedToClipboard: false, + path: null, + }) + }) + }) + + test('prints the delivered instructions as the JSON result with --json, after delivering them', async () => { + await inTemporaryDirectory(async (directory) => { + await createApp(directory) + const instructionsPath = resolvePath('./instructions.md') + const output = mockAndCaptureOutput() + output.clear() + + try { + await SecurityInstructions.run(['--write', './instructions.md', '--json'], import.meta.url) + + expect(deliverAppSecurityInstructions).toHaveBeenCalledWith('# Instructions', { + copy: false, + writePath: instructionsPath, + }) + expect(renderAppSecurityInstructions).not.toHaveBeenCalled() + expect(JSON.parse(output.info())).toEqual({ + instructions: {content: '# Instructions', copiedToClipboard: false, path: instructionsPath}, + }) + } finally { + output.clear() + } + }) + }) + + test('prints no JSON result when the delivery fails', async () => { + await inTemporaryDirectory(async (directory) => { + await createApp(directory) + vi.mocked(deliverAppSecurityInstructions).mockRejectedValue(new AbortError('Clipboard unavailable')) + const output = mockAndCaptureOutput() + output.clear() + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + try { + await expect(SecurityInstructions.run(['--copy', '--json'], import.meta.url)).rejects.toThrow( + 'process.exit unexpectedly called with "1"', + ) + + expect(output.error()).toContain('Clipboard unavailable') + expect(output.info()).not.toContain('"instructions"') + } finally { + consoleErrorSpy.mockRestore() + output.clear() + } + }) + }) + + test('renders the instructions instead of a JSON result without --json', async () => { + await inTemporaryDirectory(async (directory) => { + await createApp(directory) + const output = mockAndCaptureOutput() + output.clear() + + try { + await SecurityInstructions.run(['--copy'], import.meta.url) + + expect(renderAppSecurityInstructions).toHaveBeenCalledWith({ + content: '# Instructions', + copiedToClipboard: true, + path: null, + }) + expect(output.info()).toBe('') + } finally { + output.clear() + } }) }) @@ -90,7 +192,10 @@ describe('app security instructions command', () => { expect(resolveAppSecuritySelection).toHaveBeenCalledWith( expect.objectContaining({path: resolvePath('./fixtures/unlinked-app')}), ) - expect(deliverAppSecurityInstructions).toHaveBeenCalledWith(expect.objectContaining({copy: true})) + expect(deliverAppSecurityInstructions).toHaveBeenCalledWith( + '# Instructions', + expect.objectContaining({copy: true}), + ) }) }) @@ -100,9 +205,10 @@ describe('app security instructions command', () => { await SecurityInstructions.run(['--write', './instructions.md'], import.meta.url) - expect(deliverAppSecurityInstructions).toHaveBeenCalledWith( - expect.objectContaining({copy: false, writePath: resolvePath('./instructions.md')}), - ) + expect(deliverAppSecurityInstructions).toHaveBeenCalledWith('# Instructions', { + copy: false, + writePath: resolvePath('./instructions.md'), + }) }) }) @@ -116,7 +222,7 @@ describe('app security instructions command', () => { await SecurityInstructions.run(['--path', './fixtures/unlinked-app', '--config', 'staging'], import.meta.url) expect(resolveAppSecuritySelection).toHaveBeenCalledWith(expect.objectContaining({config: 'staging'})) - expect(deliverAppSecurityInstructions).toHaveBeenCalledWith( + expect(appSecurityInstructions).toHaveBeenCalledWith( expect.objectContaining({ resultsKey: 'shopify.app.staging', commands: resolveAppSecurityCommands( @@ -144,7 +250,7 @@ describe('app security instructions command', () => { expect(resolveAppSecuritySelection).toHaveBeenCalledWith( expect.objectContaining({clientId: 'abc123', withoutAppConfig: true, allowPrompts: false}), ) - expect(deliverAppSecurityInstructions).toHaveBeenCalledWith(expect.objectContaining({resultsKey: 'abc123'})) + expect(appSecurityInstructions).toHaveBeenCalledWith(expect.objectContaining({resultsKey: 'abc123'})) }) }) @@ -173,4 +279,18 @@ describe('app security instructions command', () => { expect(SecurityInstructions.flags.copy.exclusive).toEqual(['write']) expect(SecurityInstructions.flags.write.exclusive).toEqual(['copy']) }) + + test('does not look up --client-id', async () => { + await inTemporaryDirectory(async (directory) => { + const appDirectory = await fileRealPath(directory) + await writeFile(joinPath(appDirectory, 'shopify.app.toml'), validAppConfiguration('toml-client-id')) + await mkdir(appSecurityArtifactPaths(appDirectory, 'mistyped-client-id').resultsDirectory) + const lookUpApp = await resolveWithFailingLookUp() + + await SecurityInstructions.run(['--path', directory, '--client-id', 'mistyped-client-id'], import.meta.url) + + expect(lookUpApp).not.toHaveBeenCalled() + expect(appSecurityInstructions).toHaveBeenCalledWith(expect.objectContaining({resultsKey: 'mistyped-client-id'})) + }) + }) }) diff --git a/packages/app/src/cli/commands/app/security/instructions.ts b/packages/app/src/cli/commands/app/security/instructions.ts index f37e0d2ed93..4481db7eeb6 100644 --- a/packages/app/src/cli/commands/app/security/instructions.ts +++ b/packages/app/src/cli/commands/app/security/instructions.ts @@ -1,11 +1,20 @@ import {appSecuritySelectionFlags} from './selection-flags.js' import {resolveAppSecurityCommands} from '../../../services/app-security-commands.js' -import deliverAppSecurityInstructions from '../../../services/app-security-instructions.js' +import {appSecurityInstructions} from '../../../services/app-security-instructions.js' +import { + deliverAppSecurityInstructions, + renderAppSecurityInstructions, +} from '../../../services/app-security-instructions-output.js' import {requireResultsDirectory} from '../../../services/app-security-results.js' import {resolveAppSecuritySelection, resultsKey} from '../../../services/app-security-selection.js' +import { + securityInstructionsJsonOutputSchema, + toAppSecurityInstructionsJson, +} from '../../../services/security-instructions-json.js' import {Flags} from '@oclif/core' import BaseCommand from '@shopify/cli-kit/node/base-command' -import {globalFlags} from '@shopify/cli-kit/node/cli' +import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli' +import {outputResult} from '@shopify/cli-kit/node/output' import {resolvePath} from '@shopify/cli-kit/node/path' export default class SecurityInstructions extends BaseCommand { @@ -17,7 +26,11 @@ export default class SecurityInstructions extends BaseCommand { By default, the instructions are printed to stdout. Use \`--copy\` to copy them to the clipboard or \`--write\` to write them to a file. Standalone instructions always start by running \`shopify app security check\`; only that invocation's generated review pack is trusted as workflow input.` - static description = this.descriptionWithoutMarkdown() + static get jsonOutputSchema() { + return securityInstructionsJsonOutputSchema + } + + static description = this.descriptionForHelp() static flags = { ...globalFlags, @@ -34,6 +47,7 @@ By default, the instructions are printed to stdout. Use \`--copy\` to copy them parse: async (input) => resolvePath(input), env: 'SHOPIFY_FLAG_APP_SECURITY_INSTRUCTIONS_WRITE', }), + ...jsonFlag, } public async run(): Promise { @@ -48,12 +62,19 @@ By default, the instructions are printed to stdout. Use \`--copy\` to copy them }) await requireResultsDirectory(selection, flags.path) - await deliverAppSecurityInstructions({ + const content = appSecurityInstructions({ appDirectory: selection.appDirectory, resultsKey: resultsKey(selection), commands: resolveAppSecurityCommands(selection, flags.path), - copy: flags.copy, - writePath: flags.write, }) + const delivery = {copy: flags.copy, writePath: flags.write} + await deliverAppSecurityInstructions(content, delivery) + const instructions = toAppSecurityInstructionsJson(content, delivery) + + if (flags.json) { + outputResult(securityInstructionsJsonOutputSchema.encode({instructions})) + } else { + renderAppSecurityInstructions(instructions) + } } } diff --git a/packages/app/src/cli/commands/app/security/record.test.ts b/packages/app/src/cli/commands/app/security/record.test.ts index c6ca31488cb..46e8521dc52 100644 --- a/packages/app/src/cli/commands/app/security/record.test.ts +++ b/packages/app/src/cli/commands/app/security/record.test.ts @@ -3,11 +3,13 @@ import SecurityCheck from './check.js' import {appFlags} from '../../../flags.js' import {appSecurityArtifactPaths} from '../../../services/app-security-artifacts.js' import {resolveAppSecuritySelection} from '../../../services/app-security-selection.js' +import {validAppConfiguration} from '../../../services/app-security-selection.test-data.js' import securityRecord, {renderSecurityRecordResult} from '../../../services/security-record.js' import {securityRecordJsonOutputSchema} from '../../../services/security-record-json.js' import AppLinkedCommand from '../../../utilities/app-linked-command.js' import BaseCommand from '@shopify/cli-kit/node/base-command' -import {fileRealPath, inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs' +import {AbortError} from '@shopify/cli-kit/node/error' +import {fileExists, fileRealPath, inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs' import {cwd, joinPath} from '@shopify/cli-kit/node/path' import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output' import {describe, expect, test, vi} from 'vitest' @@ -34,6 +36,32 @@ async function createApp(directory: string, {withResults = true} = {}): Promise< return appDirectory } +/** + * Creates an app that the real resolver loads, with a results directory for `resultsKey`, and makes the mocked + * resolver run the real one with the client ID lookup replaced. + */ +async function createLinkedApp( + directory: string, + resultsKey: string, + lookUpApp: (clientId: string) => Promise, +): Promise { + const appDirectory = await fileRealPath(directory) + await writeFile(joinPath(appDirectory, 'shopify.app.toml'), validAppConfiguration('toml-client-id')) + await mkdir(appSecurityArtifactPaths(appDirectory, resultsKey).resultsDirectory) + const actual = await vi.importActual( + '../../../services/app-security-selection.js', + ) + vi.mocked(resolveAppSecuritySelection).mockImplementation((options) => + actual.resolveAppSecuritySelection(options, { + confirmScanWithoutAppConfig: async () => true, + pickClientId: async () => 'picked-client-id', + pickConfigFile: async () => 'shopify.app.toml', + lookUpApp, + }), + ) + return appDirectory +} + function recordedResult(appRoot: string) { return {path: appSecurityArtifactPaths(appRoot, 'shopify.app').agentFindingsPath, checks: 2, findings: 3} } @@ -72,6 +100,7 @@ describe('app security record command', () => { clientId: undefined, withoutAppConfig: undefined, allowPrompts: false, + validateClientIdFlag: true, }) const selection = await vi.mocked(resolveAppSecuritySelection).mock.results[0]!.value expect(securityRecord).toHaveBeenCalledWith({selection, path: cwd()}) @@ -135,6 +164,7 @@ describe('app security record command', () => { clientId: 'abc123', withoutAppConfig: true, allowPrompts: false, + validateClientIdFlag: true, }) } finally { output.clear() @@ -162,4 +192,68 @@ describe('app security record command', () => { } }) }) + + test('looks up --client-id and records when it is found', async () => { + await inTemporaryDirectory(async (directory) => { + const lookUpApp = vi.fn(async (_clientId: string) => {}) + const appRoot = await createLinkedApp(directory, 'flag-client-id', lookUpApp) + vi.mocked(securityRecord).mockResolvedValue(recordedResult(appRoot)) + const output = mockAndCaptureOutput() + + try { + await SecurityRecord.run(['--path', directory, '--client-id', 'flag-client-id', '--json'], import.meta.url) + + expect(lookUpApp).toHaveBeenCalledWith('flag-client-id') + expect(securityRecord).toHaveBeenCalledWith({ + selection: expect.objectContaining({clientIdOverride: 'flag-client-id'}), + path: directory, + }) + } finally { + output.clear() + } + }) + }) + + test('aborts on an unknown --client-id before reading stdin or writing anything', async () => { + await inTemporaryDirectory(async (directory) => { + const appRoot = await createLinkedApp(directory, 'unknown-client-id', async () => { + throw new AbortError('No app with client ID unknown-client-id found') + }) + const output = mockAndCaptureOutput() + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + try { + await expect( + SecurityRecord.run(['--path', directory, '--client-id', 'unknown-client-id'], import.meta.url), + ).rejects.toThrow('process.exit unexpectedly called with "1"') + + expect(output.error()).toContain('No app with client ID unknown-client-id found') + expect(securityRecord).not.toHaveBeenCalled() + await expect( + fileExists(appSecurityArtifactPaths(appRoot, 'unknown-client-id').agentFindingsPath), + ).resolves.toBe(false) + } finally { + consoleErrorSpy.mockRestore() + output.clear() + } + }) + }) + + test('does not look up the TOML client ID', async () => { + await inTemporaryDirectory(async (directory) => { + const lookUpApp = vi.fn(async (_clientId: string) => {}) + const appRoot = await createLinkedApp(directory, 'shopify.app', lookUpApp) + vi.mocked(securityRecord).mockResolvedValue(recordedResult(appRoot)) + const output = mockAndCaptureOutput() + + try { + await SecurityRecord.run(['--path', directory, '--json'], import.meta.url) + + expect(lookUpApp).not.toHaveBeenCalled() + expect(securityRecord).toHaveBeenCalled() + } finally { + output.clear() + } + }) + }) }) diff --git a/packages/app/src/cli/commands/app/security/record.ts b/packages/app/src/cli/commands/app/security/record.ts index 27d50033f0d..0311cc4b051 100644 --- a/packages/app/src/cli/commands/app/security/record.ts +++ b/packages/app/src/cli/commands/app/security/record.ts @@ -12,7 +12,7 @@ export default class SecurityRecord extends BaseCommand { static summary = 'Record agent findings from an app security check.' - static descriptionWithMarkdown = `Reads a coding agent's complete findings document from stdin, validates it, and replaces \`agent-findings.json\` in the results directory, \`.shopify/app-security//\`. The results key is \`--client-id\` when you pass it, and otherwise the name of the app configuration file without \`.toml\`. + static descriptionWithMarkdown = `Reads a coding agent's complete findings document from stdin, validates it, and replaces \`agent-findings.json\` in the results directory, \`.shopify/app-security//\`. The results key is \`--client-id\` when you pass it, and otherwise the name of the app configuration file without \`.toml\`. \`--client-id\` is checked against your Shopify account before anything is read, so it needs you to be logged in. The document must include a \`scope\` with the \`include_dirs\`, \`excludes\` and \`no_git_ignore\` values of the \`check\` run it describes, exactly as typed. It's recorded as reported and never compared with the scan's files. @@ -39,6 +39,7 @@ The document is recorded all or nothing: if anything is invalid, the command fai clientId: flags['client-id'], withoutAppConfig: flags['without-app-config'], allowPrompts: false, + validateClientIdFlag: true, }) await requireResultsDirectory(selection, flags.path) const result = await securityRecord({selection, path: flags.path}) diff --git a/packages/app/src/cli/commands/app/security/review.ts b/packages/app/src/cli/commands/app/security/review.ts index 624bc71a8be..e23ed52864f 100644 --- a/packages/app/src/cli/commands/app/security/review.ts +++ b/packages/app/src/cli/commands/app/security/review.ts @@ -11,7 +11,7 @@ export default class SecurityReview extends BaseCommand { static summary = 'Show the combined app security check results.' - static descriptionWithMarkdown = `Combines the deterministic results (\`deterministic-findings.json\`, written by \`shopify app security check\`) with the recorded agent results (\`agent-findings.json\`, written by \`shopify app security record\`) and shows one view of every check: its findings, status and source. Both files are in the results directory, \`.shopify/app-security//\`. + static descriptionWithMarkdown = `Combines the deterministic results (\`deterministic-findings.json\`, written by \`shopify app security check\`) with the recorded agent results (\`agent-findings.json\`, written by \`shopify app security record\`) and shows one view of every check: its findings, status and source. Both files are in the results directory, \`.shopify/app-security//\`. \`--client-id\` is checked against your Shopify account before any results are read, so it needs you to be logged in. The summary shows the scan directories and the scope of the latest scan, and the scope the agent reported. It notes when the agent findings were recorded for a different scope than the latest scan; that doesn't change the exit code. diff --git a/packages/app/src/cli/services/app-security-api.ts b/packages/app/src/cli/services/app-security-api.ts index 7cec981c336..0b1b3b9b463 100644 --- a/packages/app/src/cli/services/app-security-api.ts +++ b/packages/app/src/cli/services/app-security-api.ts @@ -2,15 +2,12 @@ import { listGatheredPaths, scanApp, SEVERITY_RANK, - type AppSecurityEngineMetadata, type AppSecurityScan, type ScanInput, type ScanOptions, type Severity, } from './app-security-engine/index.js' -export type {AppSecurityEngineMetadata} - export type AppSecurityBlockingLevel = Severity | 'none' export type AppSecurityExecution = AppSecurityScan & {elapsedMilliseconds: number} diff --git a/packages/app/src/cli/services/app-security-engine/tests/layout-catalogue.test.ts b/packages/app/src/cli/services/app-security-engine/tests/layout-catalogue.test.ts index 912ab83bf48..eec6f5acaba 100644 --- a/packages/app/src/cli/services/app-security-engine/tests/layout-catalogue.test.ts +++ b/packages/app/src/cli/services/app-security-engine/tests/layout-catalogue.test.ts @@ -1,6 +1,7 @@ /* eslint-disable no-restricted-imports -- layouts are real temporary repositories and directories */ import {git, isolateGitConfig} from './git-test-helpers.js' -import securityCheck from '../../security-check.js' +import securityCheck, {resolveSecurityCheckSelection} from '../../security-check.js' +import {renderSecurityCheckResult} from '../../security-output.js' import {mergeScanDirectories, resolveIncludeDirectories} from '../../app-security-selection.js' import {AbortError} from '@shopify/cli-kit/node/error' import {unstyled} from '@shopify/cli-kit/node/output' @@ -11,6 +12,12 @@ import {mkdir, mkdtemp, realpath, rm, writeFile} from 'node:fs/promises' import {tmpdir} from 'node:os' import {dirname, join, resolve} from 'node:path' +// The --client-id lookup needs a login and the network. The mock finds every client ID. +vi.mock('../../context.js', async (importOriginal) => ({ + ...(await importOriginal()), + appFromIdentifiers: vi.fn(), +})) + type FileSpec = string | readonly [path: string, content: string] interface Layout { @@ -87,14 +94,14 @@ async function inLayout(layout: Layout, run: (temporaryDirectory: string) => Pro } } -/** Runs `check --list-files` from `workingDirectory` (relative to `T`), the way the command calls the service. */ +/** Runs `check --list-files` from `workingDirectory` (relative to `T`), the way the command calls the services. */ async function checkListFiles(temporaryDirectory: string, workingDirectory: string, flags: CheckFlags = {}) { const absoluteWorkingDirectory = resolve(temporaryDirectory, workingDirectory) vi.stubEnv('INIT_CWD', absoluteWorkingDirectory) const directory = resolve(absoluteWorkingDirectory, flags.path ?? '.') return withCapturedStandardStreams(async ({stdout, stderr}) => { - const resolution = await securityCheck({ + const resolution = await resolveSecurityCheckSelection({ directory, configName: flags.config, clientId: flags.clientId, @@ -102,12 +109,17 @@ async function checkListFiles(temporaryDirectory: string, workingDirectory: stri includeDirs: flags.includeDirs ?? [], excludePatterns: flags.excludes ?? [], noGitIgnore: flags.noGitIgnore ?? false, - listFiles: true, - json: flags.json ?? false, + allowPrompts: false, + }) + if (resolution.kind === 'cancelled') throw new Error('Without prompts, nothing can be declined.') + const result = await securityCheck(resolution, {listFiles: true}) + await renderSecurityCheckResult(result, { + format: flags.json ? 'json' : 'text', verbose: false, blocking: 'none', yes: false, skipInstructions: false, + canPrompt: false, }) return {resolution, stdout: stdout(), stderr: stderr()} }) @@ -933,12 +945,14 @@ describe('layout catalogue: check --list-files', () => { await inLayout(twoRepositories, async (root) => { const {stdout} = await checkListFiles(root, 'app', {includeDirs: ['../backend'], json: true}) + // JSON lists absolute paths, while the text output lists them relative to the app directory. expect(JSON.parse(stdout)).toEqual({ + status: 'success', files: [ - '../backend/src/admin/index.ts', - '../backend/src/server.ts', - 'extensions/checkout-ui/src/Checkout.tsx', - 'shopify.app.toml', + joinPath(root, 'backend/src/admin/index.ts'), + joinPath(root, 'backend/src/server.ts'), + joinPath(root, 'app/extensions/checkout-ui/src/Checkout.tsx'), + joinPath(root, 'app/shopify.app.toml'), ], }) }) diff --git a/packages/app/src/cli/services/app-security-instructions-output.test.ts b/packages/app/src/cli/services/app-security-instructions-output.test.ts new file mode 100644 index 00000000000..491c4431458 --- /dev/null +++ b/packages/app/src/cli/services/app-security-instructions-output.test.ts @@ -0,0 +1,94 @@ +import {deliverAppSecurityInstructions, renderAppSecurityInstructions} from './app-security-instructions-output.js' +import {inTemporaryDirectory, readFile, writeFile} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' +import {describe, expect, test, vi} from 'vitest' + +function deliveryDependencies() { + return { + copyToClipboard: vi.fn(async (_content: string) => {}), + writeToFile: vi.fn(writeFile), + } +} + +function renderDependencies() { + return { + output: vi.fn(), + outputConfirmation: vi.fn(), + } +} + +describe('deliverAppSecurityInstructions', () => { + test('copies the instructions to the clipboard', async () => { + const dependencies = deliveryDependencies() + + await deliverAppSecurityInstructions('# Instructions', {copy: true}, dependencies) + + expect(dependencies.copyToClipboard).toHaveBeenCalledWith('# Instructions') + expect(dependencies.writeToFile).not.toHaveBeenCalled() + }) + + test('writes the instructions to a real file, ending with a newline', async () => { + await inTemporaryDirectory(async (directory) => { + const dependencies = deliveryDependencies() + const instructionsPath = joinPath(directory, 'handoff.md') + + await deliverAppSecurityInstructions('# Instructions', {copy: false, writePath: instructionsPath}, dependencies) + + await expect(readFile(instructionsPath)).resolves.toBe('# Instructions\n') + expect(dependencies.copyToClipboard).not.toHaveBeenCalled() + }) + }) + + test('does nothing with instructions that are neither copied nor written', async () => { + const dependencies = deliveryDependencies() + + await deliverAppSecurityInstructions('# Instructions', {copy: false}, dependencies) + + expect(dependencies.copyToClipboard).not.toHaveBeenCalled() + expect(dependencies.writeToFile).not.toHaveBeenCalled() + }) + + test('fails when the file cannot be written', async () => { + await inTemporaryDirectory(async (directory) => { + await expect( + deliverAppSecurityInstructions('# Instructions', {copy: false, writePath: directory}, deliveryDependencies()), + ).rejects.toThrow() + }) + }) +}) + +describe('renderAppSecurityInstructions', () => { + test('prints instructions that were neither copied nor written', () => { + const dependencies = renderDependencies() + + renderAppSecurityInstructions({content: '# Instructions', copiedToClipboard: false, path: null}, dependencies) + + expect(dependencies.output).toHaveBeenCalledWith('# Instructions') + expect(dependencies.outputConfirmation).not.toHaveBeenCalled() + }) + + test('confirms a copy without printing the instructions', () => { + const dependencies = renderDependencies() + + renderAppSecurityInstructions({content: '# Instructions', copiedToClipboard: true, path: null}, dependencies) + + expect(dependencies.outputConfirmation).toHaveBeenCalledWith( + 'Copied app security check instructions to the clipboard', + ) + expect(dependencies.output).not.toHaveBeenCalled() + }) + + test('confirms a written file by its path without printing the instructions', () => { + const dependencies = renderDependencies() + + renderAppSecurityInstructions( + {content: '# Instructions', copiedToClipboard: false, path: '/tmp/handoff.md'}, + dependencies, + ) + + expect(dependencies.outputConfirmation).toHaveBeenCalledWith( + 'Wrote app security check instructions to /tmp/handoff.md', + ) + expect(dependencies.output).not.toHaveBeenCalled() + }) +}) diff --git a/packages/app/src/cli/services/app-security-instructions-output.ts b/packages/app/src/cli/services/app-security-instructions-output.ts new file mode 100644 index 00000000000..a810166fd01 --- /dev/null +++ b/packages/app/src/cli/services/app-security-instructions-output.ts @@ -0,0 +1,57 @@ +import {writeFile} from '@shopify/cli-kit/node/fs' +import {outputResult} from '@shopify/cli-kit/node/output' +import {renderSuccess} from '@shopify/cli-kit/node/ui' +import clipboard from 'clipboardy' +import type {AppSecurityInstructionsJson} from './security-instructions-json.js' + +interface AppSecurityInstructionsDeliveryDependencies { + copyToClipboard(content: string): Promise + writeToFile(path: string, content: string): Promise +} + +interface AppSecurityInstructionsRenderDependencies { + output(content: string): void + outputConfirmation(content: string): void +} + +const defaultDeliveryDependencies: AppSecurityInstructionsDeliveryDependencies = { + copyToClipboard: (content) => clipboard.write(content), + writeToFile: writeFile, +} + +const defaultRenderDependencies: AppSecurityInstructionsRenderDependencies = { + output: outputResult, + outputConfirmation: (content) => { + renderSuccess({headline: content}) + }, +} + +/** + * Copies the instructions to the clipboard or writes them to `writePath`, and otherwise does nothing. It shows + * nothing in the terminal, so it runs the same way in JSON mode. + */ +export async function deliverAppSecurityInstructions( + content: string, + delivery: {copy: boolean; writePath?: string}, + dependencies: AppSecurityInstructionsDeliveryDependencies = defaultDeliveryDependencies, +): Promise { + if (delivery.copy) { + await dependencies.copyToClipboard(content) + } else if (delivery.writePath) { + await dependencies.writeToFile(delivery.writePath, `${content}\n`) + } +} + +/** Confirms delivered instructions in the terminal, or prints them when they were neither copied nor written. */ +export function renderAppSecurityInstructions( + instructions: AppSecurityInstructionsJson, + dependencies: AppSecurityInstructionsRenderDependencies = defaultRenderDependencies, +): void { + if (instructions.copiedToClipboard) { + dependencies.outputConfirmation('Copied app security check instructions to the clipboard') + } else if (instructions.path) { + dependencies.outputConfirmation(`Wrote app security check instructions to ${instructions.path}`) + } else { + dependencies.output(instructions.content) + } +} diff --git a/packages/app/src/cli/services/app-security-instructions.test.ts b/packages/app/src/cli/services/app-security-instructions.test.ts index 0ab157e83a0..37fd190947b 100644 --- a/packages/app/src/cli/services/app-security-instructions.test.ts +++ b/packages/app/src/cli/services/app-security-instructions.test.ts @@ -1,25 +1,13 @@ -import deliverAppSecurityInstructions, { - appSecurityInstructions as instructionsFor, - shellQuote, -} from './app-security-instructions.js' +import {appSecurityInstructions as instructionsFor, shellQuote} from './app-security-instructions.js' import {quoteShellArgument, resolveAppSecurityCommands, type AppSecurityShell} from './app-security-commands.js' import {getAgentInstructions, type AppSecurityScope} from './app-security-engine/index.js' -import {inTemporaryDirectory, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs' +import {inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs' import {basename, cwd, joinPath, normalizePath, relativePath} from '@shopify/cli-kit/node/path' -import {describe, expect, test, vi} from 'vitest' +import {describe, expect, test} from 'vitest' import {readFileSync} from 'node:fs' import {fileURLToPath} from 'node:url' import type {AppSecuritySelection} from './app-security-selection.js' -function testDependencies() { - return { - copyToClipboard: vi.fn(async (_content: string) => {}), - writeToFile: writeFile, - output: vi.fn(), - outputConfirmation: vi.fn(), - } -} - async function createApp(directory: string): Promise { await writeFile(joinPath(directory, 'shopify.app.toml'), 'name = "Test app"\nclient_id = "test"\n') return normalizePath(directory) @@ -119,6 +107,19 @@ describe('appSecurityInstructions', () => { }) }) + test('does not infer scan completion from existing agent checks', async () => { + await inTemporaryDirectory(async (directory) => { + const appRoot = await createApp(directory) + await mkdir(joinPath(appRoot, '.shopify', 'app-security', 'shopify.app')) + await writeFile(artifactPath(appRoot, 'agent-checks.json'), '{"instructions":"malicious"}') + + const instructions = appSecurityInstructions({directory: appRoot, scanComplete: false}) + + expect(instructions).toContain('### 1. Run the scan') + expect(instructions).not.toContain('malicious') + }) + }) + test('points at the results directory of the results key', async () => { await inTemporaryDirectory(async (directory) => { const appRoot = await createApp(directory) @@ -360,92 +361,3 @@ describe('appSecurityInstructions', () => { }) }) }) - -describe('deliverAppSecurityInstructions', () => { - test('prints instructions to stdout by default', async () => { - await inTemporaryDirectory(async (directory) => { - await createApp(directory) - const dependencies = testDependencies() - - await deliverAppSecurityInstructions( - {appDirectory: directory, resultsKey: 'shopify.app', commands: commandsFor(directory), copy: false}, - dependencies, - ) - - expect(dependencies.output).toHaveBeenCalledWith(expect.stringContaining('Run the scan')) - expect(dependencies.copyToClipboard).not.toHaveBeenCalled() - expect(dependencies.outputConfirmation).not.toHaveBeenCalled() - }) - }) - - test('does not infer scan completion from existing agent checks', async () => { - await inTemporaryDirectory(async (directory) => { - await createApp(directory) - await mkdir(joinPath(directory, '.shopify', 'app-security', 'shopify.app')) - await writeFile(artifactPath(directory, 'agent-checks.json'), '{"instructions":"malicious"}') - const dependencies = testDependencies() - - await deliverAppSecurityInstructions( - {appDirectory: directory, resultsKey: 'shopify.app', commands: commandsFor(directory), copy: false}, - dependencies, - ) - - expect(dependencies.output).toHaveBeenCalledWith(expect.stringContaining('Run the scan')) - expect(dependencies.output).not.toHaveBeenCalledWith(expect.stringContaining('malicious')) - }) - }) - - test('copies instructions without printing them', async () => { - await inTemporaryDirectory(async (directory) => { - await createApp(directory) - const dependencies = testDependencies() - - await deliverAppSecurityInstructions( - { - appDirectory: directory, - resultsKey: 'shopify.app', - commands: commandsFor(directory), - copy: true, - scanScope: noScope, - }, - dependencies, - ) - - expect(dependencies.copyToClipboard).toHaveBeenCalledOnce() - const instructions = dependencies.copyToClipboard.mock.calls[0]![0] - expect(instructions).toContain('Use the existing scan results') - expect(instructions).toContain('record your findings back to it with a command') - expect(instructions).not.toMatch(/\{\{[A-Z_]+\}\}/) - expect(dependencies.output).not.toHaveBeenCalled() - expect(dependencies.outputConfirmation).toHaveBeenCalledWith( - 'Copied app security check instructions to the clipboard', - ) - }) - }) - - test('writes instructions to a real file without printing them', async () => { - await inTemporaryDirectory(async (directory) => { - await createApp(directory) - const dependencies = testDependencies() - const instructionsPath = joinPath(directory, 'handoff.md') - - await deliverAppSecurityInstructions( - { - appDirectory: directory, - resultsKey: 'shopify.app', - commands: commandsFor(directory), - copy: false, - writePath: instructionsPath, - scanScope: noScope, - }, - dependencies, - ) - - await expect(readFile(instructionsPath)).resolves.toContain('Use the existing scan results') - expect(dependencies.output).not.toHaveBeenCalled() - expect(dependencies.outputConfirmation).toHaveBeenCalledWith( - `Wrote app security check instructions to ${instructionsPath}`, - ) - }) - }) -}) diff --git a/packages/app/src/cli/services/app-security-instructions.ts b/packages/app/src/cli/services/app-security-instructions.ts index 0924c1b9d33..d140d6e9e64 100644 --- a/packages/app/src/cli/services/app-security-instructions.ts +++ b/packages/app/src/cli/services/app-security-instructions.ts @@ -9,11 +9,7 @@ import { type AppSecurityShell, } from './app-security-commands.js' import {getAgentInstructions, type AppSecurityScope} from './app-security-engine/index.js' -import {writeFile} from '@shopify/cli-kit/node/fs' -import {outputResult} from '@shopify/cli-kit/node/output' import {cwd} from '@shopify/cli-kit/node/path' -import {renderSuccess} from '@shopify/cli-kit/node/ui' -import clipboard from 'clipboardy' const SCAN_CONTEXT_PLACEHOLDER = '{{SCAN_CONTEXT}}' const RECORD_DOCUMENT_PLACEHOLDER = '' @@ -147,32 +143,6 @@ function fillTemplate(template: string, values: {[placeholder: string]: string}) ) } -interface AppSecurityInstructionsOptions { - appDirectory: string - resultsKey: string - copy: boolean - writePath?: string - commands: AppSecurityCommands - /** Present when `check` has just run in this process. */ - scanScope?: AppSecurityScope -} - -interface AppSecurityInstructionsDependencies { - copyToClipboard(content: string): Promise - writeToFile(path: string, content: string): Promise - output(content: string): void - outputConfirmation(content: string): void -} - -const defaultDependencies: AppSecurityInstructionsDependencies = { - copyToClipboard: (content) => clipboard.write(content), - writeToFile: writeFile, - output: outputResult, - outputConfirmation: (content) => { - renderSuccess({headline: content}) - }, -} - export function appSecurityInstructions(options: { appDirectory: string resultsKey: string @@ -200,25 +170,3 @@ export function appSecurityInstructions(options: { '{{AGENT_FINDINGS_PATH}}': markdownPath(paths.agentFindingsPath), }).trimEnd() } - -export default async function deliverAppSecurityInstructions( - options: AppSecurityInstructionsOptions, - dependencies: AppSecurityInstructionsDependencies = defaultDependencies, -): Promise { - const instructions = appSecurityInstructions({ - appDirectory: options.appDirectory, - resultsKey: options.resultsKey, - commands: options.commands, - scanScope: options.scanScope, - }) - - if (options.copy) { - await dependencies.copyToClipboard(instructions) - dependencies.outputConfirmation('Copied app security check instructions to the clipboard') - } else if (options.writePath) { - await dependencies.writeToFile(options.writePath, `${instructions}\n`) - dependencies.outputConfirmation(`Wrote app security check instructions to ${options.writePath}`) - } else { - dependencies.output(instructions) - } -} diff --git a/packages/app/src/cli/services/app-security-json-fixtures/check.json b/packages/app/src/cli/services/app-security-json-fixtures/check.json index 886e0b99127..f540063083e 100644 --- a/packages/app/src/cli/services/app-security-json-fixtures/check.json +++ b/packages/app/src/cli/services/app-security-json-fixtures/check.json @@ -1,17 +1,13 @@ { - "engine": { - "name": "shopify-app-security", - "version": "1.2.3", - "ruleset": "2026.08.28" - }, + "status": "success", "selection": { - "app_directory": "/tmp/app", - "app_config_file": "/tmp/app/shopify.app.toml", - "client_id": "test-client-id", - "client_id_source": "config", - "scan_directories": [{"directory": "/tmp/app", "origin": "app_directory"}] + "directory": "/tmp/app", + "configPath": "/tmp/app/shopify.app.toml", + "clientId": "test-client-id", + "clientIdSource": "config", + "scanDirectories": [{"directory": "/tmp/app", "origin": "app-directory"}] }, - "deterministic_findings": { + "deterministicFindings": { "schema_version": 1, "source": "deterministic", "engine": { @@ -43,5 +39,6 @@ }, "checks": [] }, - "agent_checks_path": "/tmp/app/.shopify/app-security/agent-checks.json" + "agentChecksPath": "/tmp/app/.shopify/app-security/agent-checks.json", + "instructions": null } diff --git a/packages/app/src/cli/services/app-security-json.test.ts b/packages/app/src/cli/services/app-security-json.test.ts deleted file mode 100644 index b9cf5521257..00000000000 --- a/packages/app/src/cli/services/app-security-json.test.ts +++ /dev/null @@ -1,87 +0,0 @@ -import {toSecurityJson, encodeSecurityJson} from './security-json.js' -import {readFile} from '@shopify/cli-kit/node/fs' -import {joinPath} from '@shopify/cli-kit/node/path' -import {describe, expect, test} from 'vitest' -import {fileURLToPath} from 'node:url' -import type {AppSecuritySelection} from './app-security-selection.js' -import type {DeterministicFindingsDocument} from './app-security-engine/index.js' - -const fixtureDirectory = fileURLToPath(new URL('./app-security-json-fixtures', import.meta.url)) - -const engine = { - name: 'shopify-app-security' as const, - version: '1.2.3', - ruleset: '2026.08.28', -} - -const deterministicFindings: DeterministicFindingsDocument = { - schema_version: 1, - source: 'deterministic', - engine, - generated_at: '2026-08-24T00:00:00.000Z', - detection: {framework: 'none', surface: 'config_only', languages: []}, - coverage: { - files_scanned: 1, - files_skipped: [], - gaps: [], - scope: {include_dirs: [], excludes: [], no_git_ignore: false}, - scan_directories: [{directory: '.', origin: 'app_directory'}], - }, - checks: [], -} - -const appDirectory = '/tmp/app' -const agentChecksPath = '/tmp/app/.shopify/app-security/agent-checks.json' -const scanDirectories = [{directory: appDirectory, origin: 'app_directory' as const}] - -function selectionJson(selection: AppSecuritySelection) { - return toSecurityJson({engine, deterministicFindings}, agentChecksPath, selection, scanDirectories).selection -} - -describe('app security JSON contract', () => { - test('encodes the engine, the selection, the deterministic findings document, and the agent checks path', async () => { - const encoded = encodeSecurityJson( - toSecurityJson( - {engine, deterministicFindings}, - agentChecksPath, - { - kind: 'config', - appDirectory, - appConfigFilePath: '/tmp/app/shopify.app.toml', - configClientId: 'test-client-id', - }, - scanDirectories, - ), - ) - const fixture = await readFile(joinPath(fixtureDirectory, 'check.json')) - expect(JSON.parse(encoded)).toEqual(JSON.parse(fixture)) - }) - - test('reports the client ID override as coming from the flag', () => { - expect( - selectionJson({ - kind: 'config', - appDirectory, - appConfigFilePath: '/tmp/app/shopify.app.staging.toml', - configClientId: 'toml-client-id', - clientIdOverride: 'flag-client-id', - }), - ).toMatchObject({client_id: 'flag-client-id', client_id_source: 'flag'}) - }) - - test('reports an unlinked configuration with no client ID', () => { - expect(selectionJson({kind: 'config', appDirectory, appConfigFilePath: '/tmp/app/shopify.app.toml'})).toMatchObject( - {client_id: null, client_id_source: null}, - ) - }) - - test.each(['flag', 'picker'] as const)('reports no configuration file and a %s client ID', (clientIdSource) => { - expect(selectionJson({kind: 'no-config', appDirectory, clientId: 'chosen-id', clientIdSource})).toEqual({ - app_directory: appDirectory, - app_config_file: null, - client_id: 'chosen-id', - client_id_source: clientIdSource, - scan_directories: scanDirectories, - }) - }) -}) diff --git a/packages/app/src/cli/services/app-security-selection.test.ts b/packages/app/src/cli/services/app-security-selection.test.ts index 8f16887ac58..28a8c383c1e 100644 --- a/packages/app/src/cli/services/app-security-selection.test.ts +++ b/packages/app/src/cli/services/app-security-selection.test.ts @@ -13,9 +13,11 @@ import { import {validAppConfiguration} from './app-security-selection.test-data.js' import {getCachedAppInfo, setCachedAppInfo} from './local-storage.js' import {appCreationDefaults} from './app/config/link.js' -import {fetchOrCreateOrganizationApp} from './context.js' +import {appFromIdentifiers, fetchOrCreateOrganizationApp} from './context.js' import use from './app/config/use.js' -import {AbortError} from '@shopify/cli-kit/node/error' +import {defaultDeveloperPlatformClient} from '../utilities/developer-platform-client.js' +import {testDeveloperPlatformClient} from '../models/app/app.test-data.js' +import {AbortError, CancelExecution} from '@shopify/cli-kit/node/error' import {fileRealPath, inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' import {renderConfirmationPrompt} from '@shopify/cli-kit/node/ui' @@ -29,8 +31,13 @@ vi.mock('./local-storage.js', async (importOriginal) => ({ setCachedAppInfo: vi.fn(), })) vi.mock('./app/config/use.js', () => ({default: vi.fn()})) +vi.mock('../utilities/developer-platform-client.js', async (importOriginal) => ({ + ...(await importOriginal()), + defaultDeveloperPlatformClient: vi.fn(), +})) vi.mock('./context.js', async (importOriginal) => ({ ...(await importOriginal()), + appFromIdentifiers: vi.fn(), fetchOrCreateOrganizationApp: vi.fn(), })) vi.mock('@shopify/cli-kit/node/ui', async (importOriginal) => ({ @@ -55,6 +62,7 @@ function promptDependencies(overrides: Partial confirmScanWithoutAppConfig: vi.fn(async (_directory: string) => true), pickClientId: vi.fn(async (_appDirectory: string) => 'picked-client-id'), pickConfigFile: vi.fn(async (_appDirectory: string) => 'shopify.app.staging.toml'), + lookUpApp: vi.fn(async (_clientId: string) => {}), ...overrides, } } @@ -439,15 +447,14 @@ describe('resolveAppSecuritySelection when no TOML is found', () => { }) }) - test('aborts with the same error when the prompt is answered no', async () => { + test('cancels the run when the prompt is answered no', async () => { await inTemporaryDirectory(async (directory) => { const dependencies = promptDependencies({confirmScanWithoutAppConfig: vi.fn(async () => false)}) - const error = await selectionError( - resolveAppSecuritySelection({path: directory, clientId: 'abc', allowPrompts: true}, dependencies), - ) + await expect( + resolveAppSecuritySelection({path: directory, allowPrompts: true}, dependencies), + ).rejects.toBeInstanceOf(CancelExecution) - expect(error.message).toBe(`No app configuration found at or above ${directory}.`) expect(dependencies.confirmScanWithoutAppConfig).toHaveBeenCalledWith(directory) expect(dependencies.pickClientId).not.toHaveBeenCalled() }) @@ -507,6 +514,205 @@ describe('resolveAppSecuritySelection when no TOML is found', () => { }) }) +describe('resolveAppSecuritySelection with validateClientIdFlag', () => { + const unknownClientId = new AbortError('No app with client ID unknown-client-id found') + + test('looks up --client-id when it overrides a TOML, not the TOML client ID', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, 'toml-client-id') + const dependencies = promptDependencies() + + const selection = await resolveAppSecuritySelection( + {path: directory, clientId: 'flag-client-id', allowPrompts: false, validateClientIdFlag: true}, + dependencies, + ) + + expect(dependencies.lookUpApp).toHaveBeenCalledOnce() + expect(dependencies.lookUpApp).toHaveBeenCalledWith('flag-client-id') + expect(selection).toMatchObject({kind: 'config', clientIdOverride: 'flag-client-id'}) + }) + }) + + test('looks up --client-id with --without-app-config', async () => { + await inTemporaryDirectory(async (directory) => { + const dependencies = promptDependencies({ + lookUpApp: vi.fn(async () => { + throw unknownClientId + }), + }) + + const error = await selectionError( + resolveAppSecuritySelection( + { + path: directory, + clientId: 'unknown-client-id', + withoutAppConfig: true, + allowPrompts: false, + validateClientIdFlag: true, + }, + dependencies, + ), + ) + + expect(error).toBe(unknownClientId) + expect(dependencies.lookUpApp).toHaveBeenCalledWith('unknown-client-id') + }) + }) + + test('looks up --client-id before the no-TOML prompt, which an unknown client ID never reaches', async () => { + await inTemporaryDirectory(async (directory) => { + const dependencies = promptDependencies({ + lookUpApp: vi.fn(async () => { + throw unknownClientId + }), + }) + + const error = await selectionError( + resolveAppSecuritySelection( + {path: directory, clientId: 'unknown-client-id', allowPrompts: true, validateClientIdFlag: true}, + dependencies, + ), + ) + + expect(error).toBe(unknownClientId) + expect(dependencies.confirmScanWithoutAppConfig).not.toHaveBeenCalled() + expect(dependencies.pickClientId).not.toHaveBeenCalled() + }) + }) + + test('looks up an empty --client-id, because it was passed', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, 'toml-client-id') + const dependencies = promptDependencies({ + lookUpApp: vi.fn(async () => { + throw unknownClientId + }), + }) + + const error = await selectionError( + resolveAppSecuritySelection( + {path: directory, clientId: '', allowPrompts: false, validateClientIdFlag: true}, + dependencies, + ), + ) + + expect(error).toBe(unknownClientId) + expect(dependencies.lookUpApp).toHaveBeenCalledWith('') + }) + }) + + test.each([ + { + failure: 'a missing --path', + prepare: async (directory: string) => joinPath(directory, 'missing'), + }, + { + failure: 'a TOML that does not parse', + prepare: async (directory: string) => { + await writeFile(joinPath(directory, 'shopify.app.toml'), 'name = [') + return directory + }, + }, + { + failure: 'several TOMLs with none selected', + prepare: async (directory: string) => { + await writeConfiguration(directory, 'staging-client-id', 'shopify.app.staging.toml') + await writeConfiguration(directory, 'production-client-id', 'shopify.app.production.toml') + return directory + }, + }, + ])('aborts on $failure before looking up --client-id', async ({prepare}) => { + await inTemporaryDirectory(async (directory) => { + const path = await prepare(directory) + const dependencies = promptDependencies() + + await selectionError( + resolveAppSecuritySelection( + {path, clientId: 'flag-client-id', allowPrompts: true, validateClientIdFlag: true}, + dependencies, + ), + ) + + expect(dependencies.lookUpApp).not.toHaveBeenCalled() + }) + }) + + test('does not look up the TOML client ID', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, 'toml-client-id') + const dependencies = promptDependencies() + + await resolveAppSecuritySelection( + {path: directory, allowPrompts: false, validateClientIdFlag: true}, + dependencies, + ) + + expect(dependencies.lookUpApp).not.toHaveBeenCalled() + }) + }) + + test('does not look up the client ID from the picker', async () => { + await inTemporaryDirectory(async (directory) => { + const dependencies = promptDependencies() + + const selection = await resolveAppSecuritySelection( + {path: directory, allowPrompts: true, validateClientIdFlag: true}, + dependencies, + ) + + expect(selection).toMatchObject({clientId: 'picked-client-id', clientIdSource: 'picker'}) + expect(dependencies.lookUpApp).not.toHaveBeenCalled() + }) + }) + + test('does not look up --client-id unless asked to', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, 'toml-client-id') + const dependencies = promptDependencies() + + await resolveAppSecuritySelection( + {path: directory, clientId: 'flag-client-id', allowPrompts: false}, + dependencies, + ) + await resolveAppSecuritySelection( + {path: directory, clientId: 'flag-client-id', withoutAppConfig: true, allowPrompts: false}, + dependencies, + ) + + expect(dependencies.lookUpApp).not.toHaveBeenCalled() + }) + }) + + test('looks up the app by its client ID in the API by default, without suggesting --reset', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, 'toml-client-id') + const context = await vi.importActual('./context.js') + vi.mocked(appFromIdentifiers).mockImplementation(context.appFromIdentifiers) + vi.mocked(defaultDeveloperPlatformClient).mockReturnValue( + testDeveloperPlatformClient({ + appFromIdentifiers: () => Promise.resolve(undefined), + accountInfo: () => Promise.resolve({type: 'UserAccount', email: 'user@example.com'}), + }), + ) + + const error = await selectionError( + resolveAppSecuritySelection({ + path: directory, + clientId: 'unknown-client-id', + allowPrompts: false, + validateClientIdFlag: true, + }), + ) + + expect(appFromIdentifiers).toHaveBeenCalledWith({apiKey: 'unknown-client-id', offerReset: false}) + expect(error.message).toBe('No app with client ID unknown-client-id found') + const nextSteps = JSON.stringify(error.tryMessage) + expect(nextSteps).toContain('shopify auth login') + expect(nextSteps).not.toContain('--reset') + }) + }) +}) + describe('selection helpers', () => { test('shows the client ID of a no-config selection', () => { const selection: AppSecuritySelection = { diff --git a/packages/app/src/cli/services/app-security-selection.ts b/packages/app/src/cli/services/app-security-selection.ts index 0c074e15336..f2ef5b9f708 100644 --- a/packages/app/src/cli/services/app-security-selection.ts +++ b/packages/app/src/cli/services/app-security-selection.ts @@ -1,12 +1,12 @@ import {localAppContext} from './app-context.js' import {appCreationDefaults} from './app/config/link.js' -import {fetchOrCreateOrganizationApp} from './context.js' +import {appFromIdentifiers, fetchOrCreateOrganizationApp} from './context.js' import {getCachedAppInfo} from './local-storage.js' import {NoAppConfigurationFoundError, Project} from '../models/project/project.js' import {getAppConfigurationShorthand} from '../models/app/config-file-naming.js' import {findConfigFiles, selectConfigFile} from '../prompts/config.js' import {configurationFileNames} from '../constants.js' -import {AbortError} from '@shopify/cli-kit/node/error' +import {AbortError, CancelExecution} from '@shopify/cli-kit/node/error' import {fileExistsSync, fileRealPath, isDirectory} from '@shopify/cli-kit/node/fs' import {basename, cwd, isSubpath, joinPath, normalizePath, relativePath, resolvePath} from '@shopify/cli-kit/node/path' import {renderConfirmationPrompt} from '@shopify/cli-kit/node/ui' @@ -40,18 +40,22 @@ export interface AppSecurityScanDirectory { origin: 'app_directory' | 'include_dir' } -interface AppSecuritySelectionOptions { +export interface AppSecuritySelectionOptions { path: string config?: string clientId?: string withoutAppConfig?: boolean allowPrompts: boolean + /** Look up the `--client-id` value in the user's account, which needs a login. Off by default. */ + validateClientIdFlag?: boolean } export interface AppSecuritySelectionDependencies { confirmScanWithoutAppConfig(directory: string): Promise pickClientId(appDirectory: string): Promise pickConfigFile(appDirectory: string): Promise + /** Aborts when the user's account has no app with this client ID. */ + lookUpApp(clientId: string): Promise } const defaultDependencies: AppSecuritySelectionDependencies = { @@ -64,6 +68,10 @@ const defaultDependencies: AppSecuritySelectionDependencies = { }), pickClientId: async (appDirectory) => (await fetchOrCreateOrganizationApp(appCreationDefaults(appDirectory))).apiKey, pickConfigFile: async (appDirectory) => (await selectConfigFile(appDirectory)).valueOrAbort(), + lookUpApp: async (clientId) => { + // The app security commands don't take `--reset`. + await appFromIdentifiers({apiKey: clientId, offerReset: false}) + }, } /** The name of the selected TOML, or undefined when there is none. */ @@ -118,9 +126,11 @@ export async function resolveAppSecuritySelection( ): Promise { if (options.withoutAppConfig) { if (!options.clientId) throw new AbortError('--without-app-config requires --client-id.') + const appDirectory = await realDirectory(options.path) + await lookUpClientIdFlag(options, dependencies) return { kind: 'no-config', - appDirectory: await realDirectory(options.path), + appDirectory, clientId: options.clientId, clientIdSource: 'flag', } @@ -136,6 +146,7 @@ export async function resolveAppSecuritySelection( userProvidedConfigName: options.config ?? unselectedConfigFile?.fileName, skipPrompts: !options.allowPrompts, }) + await lookUpClientIdFlag(options, dependencies) const appDirectory = await fileRealPath(app.directory) return { kind: 'config', @@ -185,7 +196,10 @@ async function resolveWithoutAppConfigurationFile( if (!options.allowPrompts) abortNoAppConfigurationFound(options.path) const appDirectory = await realDirectory(options.path) - if (!(await dependencies.confirmScanWithoutAppConfig(options.path))) abortNoAppConfigurationFound(options.path) + // Before the prompt, so a mistyped client ID fails without asking anything first. + await lookUpClientIdFlag(options, dependencies) + // Declining is the user's choice, not a failure: a caller that allows prompts reports the run as cancelled. + if (!(await dependencies.confirmScanWithoutAppConfig(options.path))) throw new CancelExecution() if (options.clientId) return {kind: 'no-config', appDirectory, clientId: options.clientId, clientIdSource: 'flag'} return { @@ -196,6 +210,17 @@ async function resolveWithoutAppConfigurationFile( } } +/** + * Only the `--client-id` value is looked up: the TOML's own `client_id` is never validated, and the picker's + * client ID already comes from the API. An empty value is looked up too, because it was passed. + */ +async function lookUpClientIdFlag( + options: AppSecuritySelectionOptions, + dependencies: AppSecuritySelectionDependencies, +): Promise { + if (options.validateClientIdFlag && options.clientId !== undefined) await dependencies.lookUpApp(options.clientId) +} + /** * Resolves each `--include-dir` value, as typed, against the working directory. Returns real paths in the order * given. Another app's directory is allowed. diff --git a/packages/app/src/cli/services/context.test.ts b/packages/app/src/cli/services/context.test.ts index 4d0dc3c66f1..3431ea595c9 100644 --- a/packages/app/src/cli/services/context.test.ts +++ b/packages/app/src/cli/services/context.test.ts @@ -396,13 +396,29 @@ describe('appFromIdentifiers', () => { }), ) }) + + test('omits the --reset step when offerReset is false', async () => { + vi.mocked(isUserAccount).mockReturnValue(true) + const developerPlatformClient = testDeveloperPlatformClient({ + appFromIdentifiers: () => Promise.resolve(undefined), + accountInfo: () => Promise.resolve({type: 'UserAccount', email: 'user@example.com'}), + }) + vi.mocked(defaultDeveloperPlatformClient).mockReturnValue(developerPlatformClient) + + await expect(appFromIdentifiers({apiKey: 'apiKey-12345', offerReset: false})).rejects.toThrowError( + expect.objectContaining({ + message: 'No app with client ID apiKey-12345 found', + tryMessage: renderTryMessage(false, 'user@example.com', false), + }), + ) + }) }) function emptyDeployIdentifiers() { return {appModuleUuids: {}, appModuleRegistrationIds: {}} } -const renderTryMessage = (isOrg: boolean, identifier: string) => [ +const renderTryMessage = (isOrg: boolean, identifier: string, offerReset = true) => [ { list: { title: 'Next steps:', @@ -416,7 +432,7 @@ const renderTryMessage = (isOrg: boolean, identifier: string) => [ 'than', {bold: identifier}, ], - ['Pass', {command: '--reset'}, 'to your command to create a new app'], + ...(offerReset ? [['Pass', {command: '--reset'}, 'to your command to create a new app']] : []), ], }, }, diff --git a/packages/app/src/cli/services/context.ts b/packages/app/src/cli/services/context.ts index 325a5dc7832..37d39243ef2 100644 --- a/packages/app/src/cli/services/context.ts +++ b/packages/app/src/cli/services/context.ts @@ -33,7 +33,7 @@ export const resetHelpMessage = [ 'to your command to reset your app configuration.', ] -const appNotFoundHelpMessage = (accountIdentifier: string, isOrg = false) => [ +const appNotFoundHelpMessage = (accountIdentifier: string, isOrg: boolean, offerReset: boolean) => [ { list: { title: 'Next steps:', @@ -47,7 +47,7 @@ const appNotFoundHelpMessage = (accountIdentifier: string, isOrg = false) => [ 'than', {bold: accountIdentifier}, ], - ['Pass', {command: '--reset'}, 'to your command to create a new app'], + ...(offerReset ? [['Pass', {command: '--reset'}, 'to your command to create a new app']] : []), ], }, }, @@ -55,6 +55,8 @@ const appNotFoundHelpMessage = (accountIdentifier: string, isOrg = false) => [ interface AppFromIdOptions { apiKey: string + /** Suggest `--reset` when no app is found. Pass false from commands that don't take `--reset`. Defaults to true. */ + offerReset?: boolean } export const appFromIdentifiers = async (options: AppFromIdOptions): Promise => { @@ -75,7 +77,7 @@ export const appFromIdentifiers = async (options: AppFromIdOptions): Promise { + test('encodes the selection, the deterministic findings document, and the agent checks path', async () => { + const encoded = securityCheckJsonOutputSchema.encode( + toSecurityCheckJson({deterministicFindings}, agentChecksPath, configSelection, scanDirectories, null), + ) + const fixture = await readFile(joinPath(fixtureDirectory, 'check.json')) + expect(JSON.parse(encoded)).toEqual(JSON.parse(fixture)) + }) + + test('encodes the chosen instructions in the scan result as instructions --json does', () => { + const instructions = toAppSecurityInstructionsJson('# Instructions', {copy: true}) + + const checkResult = JSON.parse( + securityCheckJsonOutputSchema.encode( + toSecurityCheckJson({deterministicFindings}, agentChecksPath, configSelection, scanDirectories, instructions), + ), + ) + const instructionsResult = JSON.parse(securityInstructionsJsonOutputSchema.encode({instructions})) + + expect(checkResult.instructions).toEqual({content: '# Instructions', copiedToClipboard: true, path: null}) + expect(instructionsResult).toEqual({instructions: checkResult.instructions}) + }) + + test('encodes the written file of instructions --write as its path', () => { + expect(toAppSecurityInstructionsJson('# Instructions', {copy: false, writePath: '/tmp/handoff.md'})).toEqual({ + content: '# Instructions', + copiedToClipboard: false, + path: '/tmp/handoff.md', + }) + }) + + test('encodes the --list-files result as a success with the absolute path of each file', () => { + const fileList = toSecurityCheckFileListJson(appDirectory, [ + 'shopify.app.toml', + 'app/index.ts', + '../backend/index.ts', + ]) + + expect(JSON.parse(securityCheckJsonOutputSchema.encode(fileList))).toEqual({ + status: 'success', + files: ['/tmp/app/shopify.app.toml', '/tmp/app/app/index.ts', '/tmp/backend/index.ts'], + }) + }) + + test('encodes a cancelled run as its status only', () => { + expect(JSON.parse(securityCheckJsonOutputSchema.encode(toSecurityCheckCancelledJson()))).toEqual({ + status: 'cancelled', + }) + }) + + test('describes the --list-files paths as absolute', () => { + const fileList = ( + securityCheckJsonOutputSchema.jsonSchema as { + definitions: {AppSecurityCheckFileListResult: {properties: {files: {description?: string}}}} + } + ).definitions.AppSecurityCheckFileListResult.properties.files + + expect(fileList.description).toBe('The absolute path of each file the check would gather.') + }) + + test('rejects a check result that is not a scan, a file list or a cancelled run', () => { + expect(() => securityCheckJsonOutputSchema.validate({status: 'success', files: 'shopify.app.toml'})).toThrow() + expect(() => securityCheckJsonOutputSchema.validate({status: 'success', agentChecksPath})).toThrow() + expect(() => securityCheckJsonOutputSchema.validate({status: 'skipped'})).toThrow() + }) + + describe('rejects a key the contract does not define', () => { + const instructions = toAppSecurityInstructionsJson('# Instructions', {copy: false}) + const scanResult = toSecurityCheckJson( + {deterministicFindings}, + agentChecksPath, + configSelection, + scanDirectories, + instructions, + ) + + test('accepts the scan result as built', () => { + expect(securityCheckJsonOutputSchema.validate(scanResult)).toEqual(scanResult) + }) + + test.each([ + ['the scan result', {...scanResult, engine: deterministicFindings.engine}], + ['the selection', {...scanResult, selection: {...scanResult.selection, kind: 'config'}}], + [ + 'a scan directory', + { + ...scanResult, + selection: { + ...scanResult.selection, + scanDirectories: [{directory: appDirectory, origin: 'app-directory', requested: true}], + }, + }, + ], + ['the instructions', {...scanResult, instructions: {...instructions, writePath: '/tmp/handoff.md'}}], + ['the --list-files result', {status: 'success', files: ['shopify.app.toml'], directory: appDirectory}], + ['the cancelled result', {status: 'cancelled', files: []}], + ])('in %s of check', (_, result) => { + expect(() => securityCheckJsonOutputSchema.validate(result)).toThrow(/Unrecognized key/) + }) + + test.each([ + ['the instructions result', {instructions, path: '/tmp/handoff.md'}], + ['the instructions', {instructions: {...instructions, writePath: '/tmp/handoff.md'}}], + ])('in %s of instructions', (_, result) => { + expect(() => securityInstructionsJsonOutputSchema.validate(result)).toThrow(/Unrecognized key/) + }) + }) + + test('publishes the scan, file list and cancelled results, and one instructions definition for both commands', () => { + const checkSchema = securityCheckJsonOutputSchema.jsonSchema as { + anyOf: unknown[] + definitions: Record + } + const instructionsSchema = securityInstructionsJsonOutputSchema.jsonSchema as { + definitions: Record + } + + expect(checkSchema.anyOf).toEqual([ + {$ref: '#/definitions/AppSecurityCheckScanResult'}, + {$ref: '#/definitions/AppSecurityCheckFileListResult'}, + {$ref: '#/definitions/AppSecurityCheckCancelledResult'}, + ]) + expect(checkSchema.definitions.AppSecurityInstructions).toEqual( + instructionsSchema.definitions.AppSecurityInstructions, + ) + }) + + test('reports the client ID override as coming from the flag', () => { + expect( + selectionJson({ + kind: 'config', + appDirectory, + appConfigFilePath: '/tmp/app/shopify.app.staging.toml', + configClientId: 'toml-client-id', + clientIdOverride: 'flag-client-id', + }), + ).toMatchObject({clientId: 'flag-client-id', clientIdSource: 'flag'}) + }) + + test('reports an unlinked configuration with no client ID', () => { + expect(selectionJson({kind: 'config', appDirectory, appConfigFilePath: '/tmp/app/shopify.app.toml'})).toMatchObject( + {clientId: null, clientIdSource: null}, + ) + }) + + test.each(['flag', 'picker'] as const)('reports no configuration file and a %s client ID', (clientIdSource) => { + expect(selectionJson({kind: 'no-config', appDirectory, clientId: 'chosen-id', clientIdSource})).toEqual({ + directory: appDirectory, + configPath: null, + clientId: 'chosen-id', + clientIdSource, + scanDirectories: [{directory: appDirectory, origin: 'app-directory'}], + }) + }) + + test('spells each scan directory origin in kebab case', () => { + const selection = toSecurityCheckJson( + {deterministicFindings}, + agentChecksPath, + configSelection, + [ + {directory: appDirectory, origin: 'app_directory'}, + {directory: '/tmp/backend', origin: 'include_dir'}, + ], + null, + ).selection + + expect(selection.scanDirectories).toEqual([ + {directory: appDirectory, origin: 'app-directory'}, + {directory: '/tmp/backend', origin: 'include-dir'}, + ]) + }) + + test('describes the deterministic findings document as keeping its own conventions and version', () => { + const scanResult = ( + securityCheckJsonOutputSchema.jsonSchema as { + definitions: {AppSecurityCheckScanResult: {properties: {deterministicFindings: {description?: string}}}} + } + ).definitions.AppSecurityCheckScanResult.properties.deterministicFindings + + expect(scanResult.description).toContain('keeps its own field conventions') + expect(scanResult.description).toContain('schema_version') + }) +}) diff --git a/packages/app/src/cli/services/security-check-json.ts b/packages/app/src/cli/services/security-check-json.ts new file mode 100644 index 00000000000..3dd66ff400b --- /dev/null +++ b/packages/app/src/cli/services/security-check-json.ts @@ -0,0 +1,106 @@ +import {clientIdSource, effectiveClientId} from './app-security-selection.js' +import {appSecurityInstructionsSchema, type AppSecurityInstructionsJson} from './security-instructions-json.js' +import {deterministicFindingsDocumentSchema} from './app-security-engine/index.js' +import {defineJsonOutputSchema} from '@shopify/cli-kit/node/json-output-schema' +import {resolvePath} from '@shopify/cli-kit/node/path' +import {zod} from '@shopify/cli-kit/node/schema' +import type {AppSecurityExecution} from './app-security-api.js' +import type {AppSecurityScanDirectory, AppSecuritySelection} from './app-security-selection.js' + +const scanDirectorySchema = zod + .object({ + directory: zod.string(), + origin: zod.enum(['app-directory', 'include-dir']), + }) + .strict() + +// The internal origins keep the spelling of the deterministic findings document's `coverage.scan_directories`. +const SCAN_DIRECTORY_ORIGINS: { + [origin in AppSecurityScanDirectory['origin']]: zod.infer['origin'] +} = { + app_directory: 'app-directory', + include_dir: 'include-dir', +} + +const scanResultSchema = zod + .object({ + status: zod.literal('success'), + selection: zod + .object({ + directory: zod.string(), + configPath: zod.string().nullable(), + clientId: zod.string().nullable(), + clientIdSource: zod.enum(['config', 'flag', 'picker']).nullable(), + scanDirectories: zod.array(scanDirectorySchema), + }) + .strict(), + deterministicFindings: deterministicFindingsDocumentSchema.describe( + 'The deterministic findings document, as written to deterministic-findings.json. It keeps its own field conventions and is versioned by its schema_version, independently of this result.', + ), + agentChecksPath: zod.string(), + /** The coding-agent instructions chosen at the prompt or with `--yes`; null when none were. */ + instructions: appSecurityInstructionsSchema.nullable(), + }) + .strict() + +const fileListResultSchema = zod + .object({ + status: zod.literal('success'), + files: zod.array(zod.string()).describe('The absolute path of each file the check would gather.'), + }) + .strict() + +const cancelledResultSchema = zod.object({status: zod.literal('cancelled')}).strict() + +// `--list-files` stops before scanning, so its result shares only `status` with a scan's; consumers know which they +// asked for. The file list has `status` although it can't be cancelled, so a prompt added there later keeps its shape. +export const securityCheckJsonOutputSchema = defineJsonOutputSchema({ + name: 'AppSecurityCheckResult', + schema: zod.union([scanResultSchema, fileListResultSchema, cancelledResultSchema]), + definitions: { + AppSecurityCheckScanResult: scanResultSchema, + AppSecurityCheckFileListResult: fileListResultSchema, + AppSecurityCheckCancelledResult: cancelledResultSchema, + AppSecurityInstructions: appSecurityInstructionsSchema, + }, +}) + +type SecurityCheckScanJsonResult = zod.infer + +/** `paths` are relative to the app directory, as the text output prints them. */ +export function toSecurityCheckFileListJson( + appDirectory: string, + paths: string[], +): zod.infer { + return {status: 'success', files: paths.map((path) => resolvePath(appDirectory, path))} +} + +/** The user declined to scan without app configuration. */ +export function toSecurityCheckCancelledJson(): zod.infer { + return {status: 'cancelled'} +} + +export function toSecurityCheckJson( + execution: Pick, + agentChecksPath: string, + selection: AppSecuritySelection, + scanDirectories: AppSecurityScanDirectory[], + instructions: AppSecurityInstructionsJson | null, +): SecurityCheckScanJsonResult { + return { + status: 'success', + selection: { + directory: selection.appDirectory, + configPath: selection.kind === 'config' ? selection.appConfigFilePath : null, + clientId: effectiveClientId(selection) ?? null, + clientIdSource: clientIdSource(selection) ?? null, + scanDirectories: scanDirectories.map(({directory, origin}) => ({ + directory, + origin: SCAN_DIRECTORY_ORIGINS[origin], + })), + }, + deterministicFindings: execution.deterministicFindings, + agentChecksPath, + instructions, + } +} diff --git a/packages/app/src/cli/services/security-check.test.ts b/packages/app/src/cli/services/security-check.test.ts index b5ec26f3b8a..4cc02f02e3b 100644 --- a/packages/app/src/cli/services/security-check.test.ts +++ b/packages/app/src/cli/services/security-check.test.ts @@ -1,14 +1,15 @@ -import securityCheck, {appSecurityInstructionsPrompt} from './security-check.js' -import {formatAppSecurityCommand, resolveAppSecurityCommands} from './app-security-commands.js' +import securityCheck, {resolveSecurityCheckSelection, type SecurityCheckResolution} from './security-check.js' +import {resolveAppSecurityCommands} from './app-security-commands.js' import {appSecurityArtifactPaths, writeCheckArtifacts} from './app-security-artifacts.js' +import {resolveAppSecuritySelection} from './app-security-selection.js' import {validAppConfiguration} from './app-security-selection.test-data.js' +import {listAppSecurityFiles} from './app-security-api.js' +import {AbortError} from '@shopify/cli-kit/node/error' import {fileExists, fileRealPath, inTemporaryDirectory, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs' import {cwd, joinPath, relativePath} from '@shopify/cli-kit/node/path' -import {withCapturedStandardStreams} from '@shopify/cli-kit/node/testing/output' import {afterEach, describe, expect, test, vi} from 'vitest' import type {AppSecurityExecution} from './app-security-api.js' -import type {AppSecuritySelection} from './app-security-selection.js' -import type {AppSecurityInstructionsDestination} from './security-check.js' +import type {AppSecuritySelection, AppSecuritySelectionOptions} from './app-security-selection.js' import type { AgentChecks, AppSecurityScope, @@ -46,12 +47,6 @@ const scan: ScanResult = { issues: [], } -const engine = { - name: 'shopify-app-security', - version: '1.2.3', - ruleset: '2026.08.28', -} - const deterministicFindings: DeterministicFindingsDocument = { schema_version: 1, source: 'deterministic', @@ -72,13 +67,7 @@ const agentChecks: AgentChecks = { schema_version: 1, engine: {name: 'shopify-app-security', version: '1.2.3'}, generated_at: '2026-08-24T00:00:00.000Z', - checks: Array.from({length: 31}, (_, index) => ({ - id: `CHECK_${index}`, - version: 1, - prompt: 'prompt', - severity: 'medium' as const, - docs_url: `https://shopify.dev/docs/apps/build/security/app-security-checks/check-${index}`, - })), + checks: [], instructions: 'review', } @@ -87,7 +76,7 @@ const scanExecution: AppSecurityExecution = { ignoredScanDirectories: [], deterministicFindings, agentChecks, - engine, + engine: {name: 'shopify-app-security', version: '1.2.3', ruleset: '2026.08.28'}, elapsedMilliseconds: 12, } @@ -105,8 +94,6 @@ const configSelection: AppSecuritySelection = { configClientId: 'toml-client-id', } -const scanDirectories = [{directory: appDirectory, origin: 'app_directory' as const}] - const noScope: AppSecurityScope = {include_dirs: [], excludes: [], no_git_ignore: false} /** The commands `check` generates for `--path` `appDirectory`, run from some other directory. */ @@ -114,61 +101,68 @@ function commandsFor(selection: AppSecuritySelection = configSelection, scope: A return resolveAppSecurityCommands(selection, appDirectory, scope) } -function stagingSelection(): AppSecuritySelection { +function selectionOptions() { return { - kind: 'config', - appDirectory, - appConfigFilePath: `${appDirectory}/shopify.app.staging.toml`, - configClientId: 'toml-client-id', + directory: appDirectory, + withoutAppConfig: false, + includeDirs: [], + excludePatterns: [], + noGitIgnore: false, + allowPrompts: false, } } -function testDependencies( - execution: AppSecurityExecution = scanExecution, +function selectionDependencies(selection: AppSecuritySelection = configSelection) { + return {resolveSelection: vi.fn(async (_options: AppSecuritySelectionOptions) => selection)} +} + +/** A resolution as `resolveSecurityCheckSelection` returns it for the configuration and scope given. */ +function resolutionFor( selection: AppSecuritySelection = configSelection, -) { + overrides: Partial = {}, +): SecurityCheckResolution { return { - resolveSelection: vi.fn(async () => selection), - execute: vi.fn(async () => execution), - listFiles: vi.fn(async () => ({paths: ['shopify.app.toml'], ignoredScanDirectories: [] as string[]})), - writeArtifacts: vi.fn(async () => artifacts), - canPrompt: vi.fn(() => false), - selectInstructionsDestination: vi.fn(async (): Promise => 'nothing'), - deliverInstructions: vi.fn(async () => {}), - output: vi.fn(), - renderInfo: vi.fn(), - renderWarning: vi.fn(), - renderReport: vi.fn(), - setExitCode: vi.fn(), - recordMetadata: vi.fn(async () => {}), + kind: 'resolved', + selection, + resultsKey: 'shopify.app', + commands: commandsFor(selection), + scope: noScope, + includeDirectories: [], + prompted: false, + ...overrides, } } -function testOptions() { +/** `resolveSecurityCheckSelection`, for a run that isn't cancelled. */ +async function resolveUncancelledSelection( + ...args: Parameters +): Promise { + const resolution = await resolveSecurityCheckSelection(...args) + if (resolution.kind === 'cancelled') throw new Error('Expected the selection to resolve, but the run was cancelled.') + return resolution +} + +function testDependencies(execution: AppSecurityExecution = scanExecution) { return { - directory: appDirectory, - withoutAppConfig: false, - json: false, - verbose: false, - blocking: 'none' as const, - yes: false, - skipInstructions: false, - includeDirs: [], - excludePatterns: [], - noGitIgnore: false, - listFiles: false, + execute: vi.fn(async () => execution), + listFiles: vi.fn(async () => ({paths: ['shopify.app.toml'], ignoredScanDirectories: [] as string[]})), + writeArtifacts: vi.fn(async () => artifacts), + recordMetadata: vi.fn(async () => {}), } } -describe('securityCheck', () => { - afterEach(() => { - vi.unstubAllEnvs() - }) +afterEach(() => { + vi.unstubAllEnvs() +}) - test('executes, writes artifacts, then renders a report', async () => { - const dependencies = testDependencies() +describe('resolveSecurityCheckSelection', () => { + test('resolves the selection, validating --client-id, and returns its results key, commands and scope', async () => { + const dependencies = selectionDependencies() - await securityCheck({...testOptions(), verbose: true, blocking: 'high'}, dependencies) + const resolution = await resolveUncancelledSelection( + {...selectionOptions(), excludePatterns: ['generated'], noGitIgnore: true}, + dependencies, + ) expect(dependencies.resolveSelection).toHaveBeenCalledWith({ path: appDirectory, @@ -176,506 +170,302 @@ describe('securityCheck', () => { clientId: undefined, withoutAppConfig: false, allowPrompts: false, + validateClientIdFlag: true, }) - expect(dependencies.execute).toHaveBeenCalledWith({ - appDirectory, - scanDirectories: [appDirectory], - requestedScanDirectories: [appDirectory], - appConfigFilePath: `${appDirectory}/shopify.app.toml`, - clientId: 'toml-client-id', - includeDirs: [], - excludePatterns: [], - noGitIgnore: false, - }) - expect(dependencies.writeArtifacts).toHaveBeenCalledWith(appDirectory, 'shopify.app', { - deterministicFindings, - agentChecks, - }) - expect(dependencies.renderReport).toHaveBeenCalledWith({ - scan, + expect(resolution).toEqual({ + kind: 'resolved', selection: configSelection, - scanDirectories, - engine, - verbose: true, - elapsedMilliseconds: 12, - commands: commandsFor(), - deterministicFindingsPath: artifacts.deterministicFindingsPath, - agentChecksPath: artifacts.agentChecksPath, - agentCheckCount: 31, + resultsKey: 'shopify.app', + commands: commandsFor(configSelection, {include_dirs: [], excludes: ['generated'], no_git_ignore: true}), + scope: {include_dirs: [], excludes: ['generated'], no_git_ignore: true}, + includeDirectories: [], + prompted: false, }) - expect(dependencies.output).not.toHaveBeenCalled() + expect(resolution.commands.scan.args).toEqual([ + 'app', + 'security', + 'check', + {flag: '--path', value: relativePath(cwd(), appDirectory)}, + {flag: '--exclude', value: 'generated'}, + '--no-git-ignore', + ]) }) - test('forwards the config name and includes --config in generated commands', async () => { - const dependencies = testDependencies(scanExecution, stagingSelection()) + test.each([true, false])('lets the selection prompt only when allowed (allowPrompts: %s)', async (allowPrompts) => { + const dependencies = selectionDependencies() - await securityCheck({...testOptions(), configName: 'staging'}, dependencies) + await resolveSecurityCheckSelection({...selectionOptions(), allowPrompts}, dependencies) - expect(dependencies.resolveSelection).toHaveBeenCalledWith(expect.objectContaining({config: 'staging'})) - expect(dependencies.execute).toHaveBeenCalledWith( - expect.objectContaining({appConfigFilePath: `${appDirectory}/shopify.app.staging.toml`}), - ) - expect(dependencies.renderReport).toHaveBeenCalledWith( - expect.objectContaining({commands: commandsFor(stagingSelection())}), - ) + expect(dependencies.resolveSelection).toHaveBeenCalledWith(expect.objectContaining({allowPrompts})) }) - test('scans with the --client-id override as the effective client ID', async () => { - const dependencies = testDependencies(scanExecution, {...configSelection, clientIdOverride: 'flag-client-id'}) - - await securityCheck({...testOptions(), clientId: 'flag-client-id'}, dependencies) + test('forwards the config name and includes --config in generated commands', async () => { + const staging: AppSecuritySelection = { + ...configSelection, + appConfigFilePath: `${appDirectory}/shopify.app.staging.toml`, + } + const dependencies = selectionDependencies(staging) - expect(dependencies.resolveSelection).toHaveBeenCalledWith(expect.objectContaining({clientId: 'flag-client-id'})) - expect(dependencies.execute).toHaveBeenCalledWith(expect.objectContaining({clientId: 'flag-client-id'})) - }) + const resolution = await resolveUncancelledSelection({...selectionOptions(), configName: 'staging'}, dependencies) - test('forwards the scope flags to the scan and repeats them in generated commands and instructions', async () => { - const dependencies = testDependencies() - dependencies.canPrompt.mockReturnValue(true) - dependencies.selectInstructionsDestination.mockResolvedValue('print') - const excludePatterns = ['generated', '../shared/**'] - - await securityCheck({...testOptions(), excludePatterns, noGitIgnore: true}, dependencies) - - const commands = commandsFor(configSelection, {include_dirs: [], excludes: excludePatterns, no_git_ignore: true}) - expect(commands.scan.args).toContainEqual({flag: '--exclude', value: 'generated'}) - expect(commands.scan.args).toContain('--no-git-ignore') - expect(dependencies.execute).toHaveBeenCalledWith(expect.objectContaining({excludePatterns, noGitIgnore: true})) - expect(dependencies.renderReport).toHaveBeenCalledWith(expect.objectContaining({commands})) - expect(dependencies.deliverInstructions).toHaveBeenCalledWith(expect.objectContaining({commands})) + expect(dependencies.resolveSelection).toHaveBeenCalledWith(expect.objectContaining({config: 'staging'})) + expect(resolution.resultsKey).toBe('shopify.app.staging') + expect(resolution.commands.scan.args).toContainEqual({flag: '--config', value: 'staging'}) }) - test('scans each --include-dir after the app directory, reports it, and repeats it before --exclude', async () => { - await inTemporaryDirectory(async (directory) => { - await mkdir(joinPath(directory, 'backend')) - vi.stubEnv('INIT_CWD', directory) - const backend = await fileRealPath(joinPath(directory, 'backend')) - const dependencies = testDependencies() - dependencies.canPrompt.mockReturnValue(true) - dependencies.selectInstructionsDestination.mockResolvedValue('print') - const options = {...testOptions(), includeDirs: ['backend', './backend'], excludePatterns: ['generated']} + test('forwards --client-id and --without-app-config, and uses the client ID as the results key', async () => { + const selection: AppSecuritySelection = { + kind: 'no-config', + appDirectory, + clientId: 'flag-client-id', + clientIdSource: 'flag', + } + const dependencies = selectionDependencies(selection) - await securityCheck(options, dependencies) + const resolution = await resolveUncancelledSelection( + {...selectionOptions(), withoutAppConfig: true, clientId: 'flag-client-id'}, + dependencies, + ) - expect(dependencies.execute).toHaveBeenCalledWith( - expect.objectContaining({ - scanDirectories: [appDirectory, backend], - requestedScanDirectories: [appDirectory, backend], - }), - ) - const commands = commandsFor(configSelection, { - include_dirs: ['backend', './backend'], - excludes: ['generated'], - no_git_ignore: false, - }) - expect(commands.scan.args.slice(-3)).toEqual([ - {flag: '--include-dir', value: 'backend'}, - {flag: '--include-dir', value: './backend'}, - {flag: '--exclude', value: 'generated'}, - ]) - expect(dependencies.renderReport).toHaveBeenCalledWith( - expect.objectContaining({ - commands, - scanDirectories: [ - {directory: appDirectory, origin: 'app_directory'}, - {directory: backend, origin: 'include_dir'}, - ], - }), - ) - expect(dependencies.deliverInstructions).toHaveBeenCalledWith(expect.objectContaining({commands})) - }) + expect(dependencies.resolveSelection).toHaveBeenCalledWith( + expect.objectContaining({withoutAppConfig: true, clientId: 'flag-client-id'}), + ) + expect(resolution.resultsKey).toBe('flag-client-id') + expect(resolution.prompted).toBe(false) }) - test('lists absolute scan directories with their origins in the JSON selection', async () => { + test('keeps the scope as typed and repeats each --include-dir before --exclude in the generated commands', async () => { await inTemporaryDirectory(async (directory) => { await mkdir(joinPath(directory, 'backend')) vi.stubEnv('INIT_CWD', directory) const backend = await fileRealPath(joinPath(directory, 'backend')) - const dependencies = testDependencies() + const scope = {include_dirs: ['backend', './backend/'], excludes: ['**/generated', '!keep'], no_git_ignore: true} - await securityCheck({...testOptions(), json: true, includeDirs: ['backend']}, dependencies) + const resolution = await resolveUncancelledSelection( + { + ...selectionOptions(), + includeDirs: scope.include_dirs, + excludePatterns: scope.excludes, + noGitIgnore: true, + }, + selectionDependencies(), + ) - expect(JSON.parse(dependencies.output.mock.calls[0]![0]).selection.scan_directories).toEqual([ - {directory: appDirectory, origin: 'app_directory'}, - {directory: backend, origin: 'include_dir'}, + expect(resolution.scope).toEqual(scope) + expect(resolution.includeDirectories).toEqual([backend, backend]) + expect(resolution.commands).toEqual(commandsFor(configSelection, scope)) + expect(resolution.commands.scan.args.slice(-5)).toEqual([ + {flag: '--include-dir', value: 'backend'}, + {flag: '--include-dir', value: './backend/'}, + {flag: '--exclude', value: '**/generated'}, + {flag: '--exclude', value: '!keep'}, + '--no-git-ignore', ]) }) }) - test('leaves out an --include-dir inside the app directory, but still passes it for the Git ignore warning', async () => { + test('aborts on a wrong --include-dir before resolving the selection', async () => { await inTemporaryDirectory(async (directory) => { - const realDirectory = await fileRealPath(directory) - await mkdir(joinPath(directory, 'vendor')) vi.stubEnv('INIT_CWD', directory) - const dependencies = testDependencies(scanExecution, {...configSelection, appDirectory: realDirectory}) + const dependencies = selectionDependencies() - await securityCheck({...testOptions(), includeDirs: ['vendor']}, dependencies) - - expect(dependencies.execute).toHaveBeenCalledWith( - expect.objectContaining({ - scanDirectories: [realDirectory], - requestedScanDirectories: [realDirectory, await fileRealPath(joinPath(directory, 'vendor'))], - }), - ) - }) - }) - - test('aborts on a wrong --include-dir before resolving the selection or scanning', async () => { - await inTemporaryDirectory(async (directory) => { - vi.stubEnv('INIT_CWD', directory) - const dependencies = testDependencies() - - await expect(securityCheck({...testOptions(), includeDirs: ['missing']}, dependencies)).rejects.toThrow( - "--include-dir missing: directory doesn't exist.", - ) + await expect( + resolveSecurityCheckSelection({...selectionOptions(), includeDirs: ['missing']}, dependencies), + ).rejects.toThrow("--include-dir missing: directory doesn't exist.") expect(dependencies.resolveSelection).not.toHaveBeenCalled() - expect(dependencies.execute).not.toHaveBeenCalled() }) }) - test('warns once for each scan directory that Git ignores, relative to the working directory', async () => { - vi.stubEnv('INIT_CWD', '/tmp') - const dependencies = testDependencies({...scanExecution, ignoredScanDirectories: [appDirectory, '/tmp']}) + test('reports prompts after the no-TOML prompt flow, and a command that skips them', async () => { + const selection: AppSecuritySelection = { + kind: 'no-config', + appDirectory, + clientId: 'picked-client-id', + clientIdSource: 'picker', + } - await securityCheck(testOptions(), dependencies) + const resolution = await resolveUncancelledSelection( + {...selectionOptions(), allowPrompts: true}, + selectionDependencies(selection), + ) - expect(dependencies.renderWarning).toHaveBeenCalledTimes(2) - expect(dependencies.renderWarning).toHaveBeenNthCalledWith(1, { - headline: 'unlinked-app is ignored by Git, so only the files Git tracks in it are scanned.', - body: ['Use', {command: '--no-git-ignore'}, 'to scan everything in it.'], - }) - expect(dependencies.renderWarning).toHaveBeenNthCalledWith(2, { - headline: '. is ignored by Git, so only the files Git tracks in it are scanned.', - body: ['Use', {command: '--no-git-ignore'}, 'to scan everything in it.'], - }) + expect(resolution.prompted).toBe(true) + expect(resolution.commands.scan.args.slice(-2)).toEqual([ + {flag: '--client-id', value: 'picked-client-id'}, + '--without-app-config', + ]) }) - test('does not warn when no scan directory is ignored', async () => { - const dependencies = testDependencies() + test('reports prompts after asking which TOML to scan, and a command that skips them with --config', async () => { + const selection: AppSecuritySelection = { + kind: 'config', + appDirectory, + appConfigFilePath: `${appDirectory}/shopify.app.staging.toml`, + configClientId: 'toml-client-id', + appConfigFilePicked: true, + } - await securityCheck(testOptions(), dependencies) + const resolution = await resolveUncancelledSelection( + {...selectionOptions(), allowPrompts: true}, + selectionDependencies(selection), + ) - expect(dependencies.renderWarning).not.toHaveBeenCalled() + expect(resolution.prompted).toBe(true) + expect(resolution.commands.scan.args).toContainEqual({flag: '--config', value: 'staging'}) }) - test('re-scanning overwrites the check artifacts without prompting and leaves agent findings untouched', async () => { - await inTemporaryDirectory(async (appRoot) => { - const paths = appSecurityArtifactPaths(appRoot, 'shopify.app') - await mkdir(paths.resultsDirectory) - await writeFile(paths.deterministicFindingsPath, '{"previous": "scan"}\n') - await writeFile(paths.agentChecksPath, '{"previous": "agent checks"}\n') - // Not valid findings on purpose: check must not read, validate, or rewrite this file. - const agentFindings = '{"recorded": "by the agent", "kept": "byte for byte"}' - await writeFile(paths.agentFindingsPath, agentFindings) - - const rescanFindings: DeterministicFindingsDocument = { - ...deterministicFindings, - generated_at: '2026-09-01T00:00:00.000Z', - } - const dependencies = { - ...testDependencies( - {...scanExecution, deterministicFindings: rescanFindings}, - {...configSelection, appDirectory: appRoot, appConfigFilePath: `${appRoot}/shopify.app.toml`}, - ), - writeArtifacts: writeCheckArtifacts, - canPrompt: vi.fn(() => true), - } - - await securityCheck({...testOptions(), directory: appRoot, skipInstructions: true}, dependencies) + test('is cancelled when the user declines to scan without app configuration', async () => { + await inTemporaryDirectory(async (directory) => { + vi.stubEnv('INIT_CWD', directory) - expect(JSON.parse(await readFile(paths.deterministicFindingsPath))).toEqual(rescanFindings) - expect(JSON.parse(await readFile(paths.agentChecksPath))).toEqual(agentChecks) - await expect(readFile(paths.agentFindingsPath)).resolves.toBe(agentFindings) - expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() - expect(dependencies.renderReport).toHaveBeenCalledWith( - expect.objectContaining({ - deterministicFindingsPath: paths.deterministicFindingsPath, - agentChecksPath: paths.agentChecksPath, - }), + const resolution = await resolveSecurityCheckSelection( + {...selectionOptions(), directory, allowPrompts: true}, + { + resolveSelection: (options) => + resolveAppSecuritySelection(options, { + confirmScanWithoutAppConfig: async () => false, + pickClientId: async () => 'picked-client-id', + pickConfigFile: async () => 'shopify.app.toml', + lookUpApp: async () => {}, + }), + }, ) - expect(dependencies.setExitCode).not.toHaveBeenCalled() + + expect(resolution).toEqual({kind: 'cancelled'}) }) }) - test('allows prompts only in an interactive terminal without --json', async () => { - const interactive = testDependencies() - interactive.canPrompt.mockReturnValue(true) - await securityCheck({...testOptions(), skipInstructions: true}, interactive) - expect(interactive.resolveSelection).toHaveBeenCalledWith(expect.objectContaining({allowPrompts: true})) + test('reports no prompts when a TOML was found', async () => { + const resolution = await resolveUncancelledSelection( + {...selectionOptions(), allowPrompts: true}, + selectionDependencies(), + ) - const nonInteractive = testDependencies() - await securityCheck({...testOptions(), skipInstructions: true}, nonInteractive) - expect(nonInteractive.resolveSelection).toHaveBeenCalledWith(expect.objectContaining({allowPrompts: false})) + expect(resolution.prompted).toBe(false) }) +}) - test('scans without app configuration, with no selected TOML and the client ID from the flag', async () => { - const selection: AppSecuritySelection = { - kind: 'no-config', - appDirectory, - clientId: 'flag-client-id', - clientIdSource: 'flag', - } - const dependencies = testDependencies(scanExecution, selection) - dependencies.canPrompt.mockReturnValue(true) +describe('securityCheck', () => { + test('scans the selection, records the findings count, writes the artifacts and returns them', async () => { + const dependencies = testDependencies() + const resolution = resolutionFor() - await securityCheck( - {...testOptions(), withoutAppConfig: true, clientId: 'flag-client-id', skipInstructions: true}, - dependencies, - ) + const result = await securityCheck(resolution, {listFiles: false}, dependencies) - expect(dependencies.resolveSelection).toHaveBeenCalledWith( - expect.objectContaining({withoutAppConfig: true, clientId: 'flag-client-id'}), - ) expect(dependencies.execute).toHaveBeenCalledWith({ appDirectory, scanDirectories: [appDirectory], requestedScanDirectories: [appDirectory], - appConfigFilePath: undefined, - clientId: 'flag-client-id', + appConfigFilePath: `${appDirectory}/shopify.app.toml`, + clientId: 'toml-client-id', includeDirs: [], excludePatterns: [], noGitIgnore: false, }) - expect(dependencies.writeArtifacts).toHaveBeenCalledWith(appDirectory, 'flag-client-id', expect.anything()) - expect(dependencies.renderInfo).not.toHaveBeenCalled() - expect(dependencies.renderReport).toHaveBeenCalledWith(expect.objectContaining({commands: commandsFor(selection)})) - }) - - test('shows the generated check command after the no-TOML prompt flow', async () => { - const selection: AppSecuritySelection = { - kind: 'no-config', - appDirectory, - clientId: 'picked-client-id', - clientIdSource: 'picker', - } - const dependencies = testDependencies(scanExecution, selection) - dependencies.canPrompt.mockReturnValue(true) - - await securityCheck({...testOptions(), skipInstructions: true}, dependencies) - - const {scan} = commandsFor(selection) - expect(scan.args.slice(-2)).toEqual([{flag: '--client-id', value: 'picked-client-id'}, '--without-app-config']) - expect(dependencies.renderInfo).toHaveBeenCalledWith({ - headline: 'To skip these prompts next time, run:', - body: [{command: formatAppSecurityCommand(scan)}], + expect(dependencies.recordMetadata).toHaveBeenCalledWith({num_security_findings: 0}) + expect(dependencies.writeArtifacts).toHaveBeenCalledWith(appDirectory, 'shopify.app', { + deterministicFindings, + agentChecks, }) - expect(dependencies.renderInfo.mock.invocationCallOrder[0]).toBeLessThan( - dependencies.execute.mock.invocationCallOrder[0]!, - ) - }) - - test('shows the generated check command, with --config, after asking which TOML to scan', async () => { - const selection: AppSecuritySelection = { - kind: 'config', - appDirectory, - appConfigFilePath: `${appDirectory}/shopify.app.staging.toml`, - configClientId: 'toml-client-id', - appConfigFilePicked: true, - } - const dependencies = testDependencies(scanExecution, selection) - dependencies.canPrompt.mockReturnValue(true) - - await securityCheck({...testOptions(), skipInstructions: true}, dependencies) - - const {scan} = commandsFor(selection) - expect(scan.args).toContainEqual({flag: '--config', value: 'staging'}) - expect(dependencies.renderInfo).toHaveBeenCalledWith({ - headline: 'To skip these prompts next time, run:', - body: [{command: formatAppSecurityCommand(scan)}], + expect(dependencies.listFiles).not.toHaveBeenCalled() + expect(result).toEqual({ + kind: 'scan', + resolution, + scanDirectories: [{directory: appDirectory, origin: 'app_directory'}], + execution: scanExecution, + artifacts, }) }) - test('does not show the prompt-flow command when a TOML was found', async () => { + test('scans with the --client-id override as the effective client ID', async () => { const dependencies = testDependencies() - await securityCheck(testOptions(), dependencies) + await securityCheck( + resolutionFor({...configSelection, clientIdOverride: 'flag-client-id'}), + {listFiles: false}, + dependencies, + ) - expect(dependencies.renderInfo).not.toHaveBeenCalled() + expect(dependencies.execute).toHaveBeenCalledWith(expect.objectContaining({clientId: 'flag-client-id'})) }) - test('does not write artifacts when the scan fails', async () => { + test('scans without app configuration, with no selected TOML, under the client ID results key', async () => { + const selection: AppSecuritySelection = { + kind: 'no-config', + appDirectory, + clientId: 'flag-client-id', + clientIdSource: 'flag', + } const dependencies = testDependencies() - dependencies.execute.mockRejectedValue(new Error('scan failed')) - - await expect(securityCheck(testOptions(), dependencies)).rejects.toThrow('scan failed') - expect(dependencies.writeArtifacts).not.toHaveBeenCalled() - }) + await securityCheck(resolutionFor(selection, {resultsKey: 'flag-client-id'}), {listFiles: false}, dependencies) - test('prints the engine, selection, deterministic findings, and agent checks path as JSON', async () => { - const dependencies = testDependencies() - dependencies.canPrompt.mockReturnValue(true) - - await securityCheck({...testOptions(), json: true, yes: true}, dependencies) - - expect(JSON.parse(dependencies.output.mock.calls[0]![0])).toEqual({ - engine, - selection: { - app_directory: appDirectory, - app_config_file: `${appDirectory}/shopify.app.toml`, - client_id: 'toml-client-id', - client_id_source: 'config', - scan_directories: scanDirectories, - }, - deterministic_findings: deterministicFindings, - agent_checks_path: artifacts.agentChecksPath, - }) - expect(dependencies.renderReport).not.toHaveBeenCalled() - expect(dependencies.resolveSelection).toHaveBeenCalledWith(expect.objectContaining({allowPrompts: false})) - expect(dependencies.canPrompt).not.toHaveBeenCalled() - expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() - expect(dependencies.deliverInstructions).not.toHaveBeenCalled() + expect(dependencies.execute).toHaveBeenCalledWith( + expect.objectContaining({appConfigFilePath: undefined, clientId: 'flag-client-id'}), + ) + expect(dependencies.writeArtifacts).toHaveBeenCalledWith(appDirectory, 'flag-client-id', expect.anything()) }) - test('does not offer coding-agent instructions in CI or another non-interactive environment', async () => { + test('scans with the scope of the run, as typed', async () => { const dependencies = testDependencies() + const scope = {include_dirs: ['backend', './backend/'], excludes: ['**/generated', '!keep'], no_git_ignore: true} - await securityCheck(testOptions(), dependencies) - - expect(dependencies.canPrompt).toHaveBeenCalledOnce() - expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() - expect(dependencies.deliverInstructions).not.toHaveBeenCalled() - }) + await securityCheck(resolutionFor(configSelection, {scope}), {listFiles: false}, dependencies) - test('prioritizes copying instructions that start from the scan results', async () => { - const dependencies = testDependencies() - dependencies.canPrompt.mockReturnValue(true) - dependencies.selectInstructionsDestination.mockResolvedValue('copy') - - await securityCheck(testOptions(), dependencies) - - expect(appSecurityInstructionsPrompt(31)).toEqual({ - message: - '31 recommended agent checks available to complete your scan. How do you want to pass that prompt to your agent?', - choices: [ - {label: 'Copy instructions to the clipboard', value: 'copy'}, - {label: 'Print instructions to the terminal', value: 'print'}, - {label: 'Nothing', value: 'nothing'}, - ], - defaultValue: 'copy', - }) - expect(dependencies.selectInstructionsDestination).toHaveBeenCalledOnce() - expect(dependencies.selectInstructionsDestination).toHaveBeenCalledWith(31) - expect(dependencies.deliverInstructions).toHaveBeenCalledWith({ - appDirectory, - resultsKey: 'shopify.app', - copy: true, - scanScope: noScope, - commands: commandsFor(), - }) - }) - - test('names a single recommended agent check in the singular', () => { - expect(appSecurityInstructionsPrompt(1).message).toBe( - '1 recommended agent check available to complete your scan. How do you want to pass that prompt to your agent?', + expect(dependencies.execute).toHaveBeenCalledWith( + expect.objectContaining({ + includeDirs: ['backend', './backend/'], + excludePatterns: ['**/generated', '!keep'], + noGitIgnore: true, + }), ) }) - test('gives the instructions the exact scope of the run, as typed', async () => { + test('scans each --include-dir after the app directory and returns it with its origin', async () => { await inTemporaryDirectory(async (directory) => { await mkdir(joinPath(directory, 'backend')) - vi.stubEnv('INIT_CWD', directory) + const backend = await fileRealPath(joinPath(directory, 'backend')) const dependencies = testDependencies() - await securityCheck( - { - ...testOptions(), - yes: true, - includeDirs: ['backend', './backend/'], - excludePatterns: ['**/generated', '!keep'], - noGitIgnore: true, - }, + const result = await securityCheck( + resolutionFor(configSelection, {includeDirectories: [backend, backend]}), + {listFiles: false}, dependencies, ) - const scope = { - include_dirs: ['backend', './backend/'], - excludes: ['**/generated', '!keep'], - no_git_ignore: true, - } - expect(dependencies.deliverInstructions).toHaveBeenCalledWith( - expect.objectContaining({scanScope: scope, commands: commandsFor(configSelection, scope)}), - ) expect(dependencies.execute).toHaveBeenCalledWith( - expect.objectContaining({includeDirs: ['backend', './backend/']}), + expect.objectContaining({ + scanDirectories: [appDirectory, backend], + requestedScanDirectories: [appDirectory, backend], + }), ) + expect(result).toMatchObject({ + scanDirectories: [ + {directory: appDirectory, origin: 'app_directory'}, + {directory: backend, origin: 'include_dir'}, + ], + }) }) }) - test('prints post-scan instructions when selected', async () => { - const dependencies = testDependencies() - dependencies.canPrompt.mockReturnValue(true) - dependencies.selectInstructionsDestination.mockResolvedValue('print') - - await securityCheck(testOptions(), dependencies) - - expect(dependencies.deliverInstructions).toHaveBeenCalledWith({ - appDirectory, - resultsKey: 'shopify.app', - copy: false, - scanScope: noScope, - commands: commandsFor(), - }) - }) - - test('does nothing when selected', async () => { - const dependencies = testDependencies() - dependencies.canPrompt.mockReturnValue(true) - - await securityCheck(testOptions(), dependencies) - - expect(dependencies.selectInstructionsDestination).toHaveBeenCalledOnce() - expect(dependencies.deliverInstructions).not.toHaveBeenCalled() - }) - - test('--yes prints post-scan instructions without prompting, including in CI', async () => { - const dependencies = testDependencies() - - await securityCheck({...testOptions(), yes: true}, dependencies) - - expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() - expect(dependencies.deliverInstructions).toHaveBeenCalledWith({ - appDirectory, - resultsKey: 'shopify.app', - copy: false, - scanScope: noScope, - commands: commandsFor(), - }) - }) - - test('--skip-instructions never offers instructions', async () => { - const dependencies = testDependencies() - dependencies.canPrompt.mockReturnValue(true) - - await securityCheck({...testOptions(), skipInstructions: true}, dependencies) + test('leaves out an --include-dir inside the app directory, but still passes it for the Git ignore warning', async () => { + await inTemporaryDirectory(async (directory) => { + const realDirectory = await fileRealPath(directory) + await mkdir(joinPath(directory, 'vendor')) + const vendor = await fileRealPath(joinPath(directory, 'vendor')) + const dependencies = testDependencies() - expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() - expect(dependencies.deliverInstructions).not.toHaveBeenCalled() - }) + await securityCheck( + resolutionFor({...configSelection, appDirectory: realDirectory}, {includeDirectories: [vendor]}), + {listFiles: false}, + dependencies, + ) - test('sets a blocking exit code from the execution result', async () => { - const dependencies = testDependencies({ - ...scanExecution, - scan: { - ...scan, - issues: [ - { - id: 'COMMITTED_SECRET', - severity: 'high', - points: -25, - title: 'Secret', - message: 'secret', - location: {file: 'app/routes/index.ts'}, - fix: {automated: false, description: 'remove it'}, - }, - ], - }, + expect(dependencies.execute).toHaveBeenCalledWith( + expect.objectContaining({scanDirectories: [realDirectory], requestedScanDirectories: [realDirectory, vendor]}), + ) }) - - await securityCheck({...testOptions(), blocking: 'high'}, dependencies) - - expect(dependencies.setExitCode).toHaveBeenCalledWith(1) }) test('records the number of deterministic issues in the command metadata', async () => { @@ -693,105 +483,87 @@ describe('securityCheck', () => { scan: {...scan, issues: [issue, {...issue, location: {file: 'app/routes/other.ts'}}]}, }) - await securityCheck({...testOptions(), json: true}, dependencies) + await securityCheck(resolutionFor(), {listFiles: false}, dependencies) expect(dependencies.recordMetadata).toHaveBeenCalledWith({num_security_findings: 2}) }) - test('records zero findings when the scan finds no issues', async () => { + test('does not write artifacts when the scan fails', async () => { const dependencies = testDependencies() + dependencies.execute.mockRejectedValue(new Error('scan failed')) - await securityCheck(testOptions(), dependencies) + await expect(securityCheck(resolutionFor(), {listFiles: false}, dependencies)).rejects.toThrow('scan failed') - expect(dependencies.recordMetadata).toHaveBeenCalledWith({num_security_findings: 0}) + expect(dependencies.writeArtifacts).not.toHaveBeenCalled() }) - test('keeps the default exit code when no finding reaches the blocking level', async () => { - const dependencies = testDependencies() + test('re-scanning overwrites the check artifacts and leaves agent findings untouched', async () => { + await inTemporaryDirectory(async (appRoot) => { + const paths = appSecurityArtifactPaths(appRoot, 'shopify.app') + await mkdir(paths.resultsDirectory) + await writeFile(paths.deterministicFindingsPath, '{"previous": "scan"}\n') + await writeFile(paths.agentChecksPath, '{"previous": "agent checks"}\n') + // Not valid findings on purpose: check must not read, validate, or rewrite this file. + const agentFindings = '{"recorded": "by the agent", "kept": "byte for byte"}' + await writeFile(paths.agentFindingsPath, agentFindings) + const rescanFindings: DeterministicFindingsDocument = { + ...deterministicFindings, + generated_at: '2026-09-01T00:00:00.000Z', + } - await securityCheck({...testOptions(), blocking: 'low'}, dependencies) + const result = await securityCheck( + resolutionFor({...configSelection, appDirectory: appRoot, appConfigFilePath: `${appRoot}/shopify.app.toml`}), + {listFiles: false}, + { + ...testDependencies({...scanExecution, deterministicFindings: rescanFindings}), + writeArtifacts: writeCheckArtifacts, + }, + ) - expect(dependencies.setExitCode).not.toHaveBeenCalled() + expect(JSON.parse(await readFile(paths.deterministicFindingsPath))).toEqual(rescanFindings) + expect(JSON.parse(await readFile(paths.agentChecksPath))).toEqual(agentChecks) + await expect(readFile(paths.agentFindingsPath)).resolves.toBe(agentFindings) + expect(result).toMatchObject({ + artifacts: {deterministicFindingsPath: paths.deterministicFindingsPath, agentChecksPath: paths.agentChecksPath}, + }) + }) }) }) describe('securityCheck --list-files', () => { - const listFilesOptions = {...testOptions(), listFiles: true} - - afterEach(() => { - vi.unstubAllEnvs() - }) - - test('prints each gathered path on its own line and does nothing else', async () => { + test('gathers the files and returns them without scanning or writing anything', async () => { const dependencies = testDependencies() dependencies.listFiles.mockResolvedValue({ paths: ['../backend/server.ts', 'app/routes/index.ts', 'shopify.app.toml'], - ignoredScanDirectories: [], + ignoredScanDirectories: [appDirectory], }) + const resolution = resolutionFor() - await securityCheck(listFilesOptions, dependencies) + const result = await securityCheck(resolution, {listFiles: true}, dependencies) - expect(dependencies.output).toHaveBeenCalledOnce() - expect(dependencies.output).toHaveBeenCalledWith('../backend/server.ts\napp/routes/index.ts\nshopify.app.toml') + expect(result).toEqual({ + kind: 'file-list', + resolution, + paths: ['../backend/server.ts', 'app/routes/index.ts', 'shopify.app.toml'], + ignoredScanDirectories: [appDirectory], + }) expect(dependencies.execute).not.toHaveBeenCalled() + expect(dependencies.recordMetadata).not.toHaveBeenCalled() expect(dependencies.writeArtifacts).not.toHaveBeenCalled() - expect(dependencies.renderReport).not.toHaveBeenCalled() - expect(dependencies.renderInfo).not.toHaveBeenCalled() - expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() - expect(dependencies.deliverInstructions).not.toHaveBeenCalled() - expect(dependencies.setExitCode).not.toHaveBeenCalled() - }) - - test('prints {"files": [...]} with --json', async () => { - const dependencies = testDependencies() - dependencies.listFiles.mockResolvedValue({ - paths: ['app/routes/index.ts', 'shopify.app.toml'], - ignoredScanDirectories: [], - }) - - await securityCheck({...listFilesOptions, json: true}, dependencies) - - expect(dependencies.output).toHaveBeenCalledOnce() - expect(JSON.parse(dependencies.output.mock.calls[0]![0])).toEqual({ - files: ['app/routes/index.ts', 'shopify.app.toml'], - }) }) - test('prints nothing when no path is gathered, and an empty list with --json', async () => { - const dependencies = testDependencies() - dependencies.listFiles.mockResolvedValue({paths: [], ignoredScanDirectories: []}) - - await securityCheck(listFilesOptions, dependencies) - expect(dependencies.output).not.toHaveBeenCalled() - - await securityCheck({...listFilesOptions, json: true}, dependencies) - expect(JSON.parse(dependencies.output.mock.calls[0]![0])).toEqual({files: []}) - }) - - test('resolves without prompts, even in an interactive terminal', async () => { - const dependencies = testDependencies() - dependencies.canPrompt.mockReturnValue(true) - - await securityCheck(listFilesOptions, dependencies) - - expect(dependencies.resolveSelection).toHaveBeenCalledWith(expect.objectContaining({allowPrompts: false})) - }) - - test('gathers with the scan directories and the scope of the run, and ignores --client-id', async () => { + test('gathers with the scan directories and the scope of the run', async () => { await inTemporaryDirectory(async (directory) => { await mkdir(joinPath(directory, 'backend')) - vi.stubEnv('INIT_CWD', directory) const backend = await fileRealPath(joinPath(directory, 'backend')) const dependencies = testDependencies() await securityCheck( - { - ...listFilesOptions, - clientId: 'ignored-client-id', - includeDirs: ['backend'], - excludePatterns: ['generated'], - noGitIgnore: true, - }, + resolutionFor(configSelection, { + scope: {include_dirs: ['backend'], excludes: ['generated'], no_git_ignore: true}, + includeDirectories: [backend], + }), + {listFiles: true}, dependencies, ) @@ -808,41 +580,7 @@ describe('securityCheck --list-files', () => { }) }) - test('warns about an ignored scan directory through renderWarning', async () => { - const dependencies = testDependencies() - dependencies.listFiles.mockResolvedValue({paths: ['shopify.app.toml'], ignoredScanDirectories: [appDirectory]}) - vi.stubEnv('INIT_CWD', '/tmp') - - await securityCheck(listFilesOptions, dependencies) - - expect(dependencies.renderWarning).toHaveBeenCalledWith({ - headline: 'unlinked-app is ignored by Git, so only the files Git tracks in it are scanned.', - body: ['Use', {command: '--no-git-ignore'}, 'to scan everything in it.'], - }) - expect(dependencies.output).toHaveBeenCalledWith('shopify.app.toml') - }) - - test('returns the selection, the results key and the generated check command', async () => { - const dependencies = testDependencies() - - const resolution = await securityCheck( - {...listFilesOptions, includeDirs: [], excludePatterns: ['generated'], noGitIgnore: true}, - dependencies, - ) - - expect(resolution.selection).toBe(configSelection) - expect(resolution.resultsKey).toBe('shopify.app') - expect(resolution.commands.scan.args).toEqual([ - 'app', - 'security', - 'check', - {flag: '--path', value: relativePath(cwd(), appDirectory)}, - {flag: '--exclude', value: 'generated'}, - '--no-git-ignore', - ]) - }) - - test('lists the real files, one path per line, and writes no results', async () => { + test('gathers the real files, relative to the app directory, and writes no results', async () => { await inTemporaryDirectory(async (directory) => { const appRoot = await fileRealPath(directory) await mkdir(joinPath(appRoot, 'app')) @@ -852,37 +590,102 @@ describe('securityCheck --list-files', () => { await writeFile(joinPath(appRoot, 'generated', 'out.ts'), 'export {}\n') vi.stubEnv('INIT_CWD', appRoot) - const {stdout, resolution} = await withCapturedStandardStreams(async ({stdout: captured}) => { - const result = await securityCheck({ - ...listFilesOptions, - directory: appRoot, - excludePatterns: ['**/generated'], - }) - return {stdout: captured(), resolution: result} + const resolution = await resolveUncancelledSelection({ + ...selectionOptions(), + directory: appRoot, + excludePatterns: ['**/generated'], }) + const result = await securityCheck( + resolution, + {listFiles: true}, + {...testDependencies(), listFiles: listAppSecurityFiles}, + ) // Outside a repository, the `.shopify` files that resolving the selection writes are gathered too. - expect(stdout).toBe( - ['.shopify/.gitignore', '.shopify/project.json', 'app/index.ts', 'shopify.app.toml'].join('\n').concat('\n'), - ) + expect(result).toMatchObject({ + paths: ['.shopify/.gitignore', '.shopify/project.json', 'app/index.ts', 'shopify.app.toml'], + }) expect(resolution.resultsKey).toBe('shopify.app') await expect(fileExists(joinPath(appRoot, '.shopify', 'app-security'))).resolves.toBe(false) }) }) +}) - test('lists the real files as JSON', async () => { - await inTemporaryDirectory(async (directory) => { - const appRoot = await fileRealPath(directory) - await writeFile(joinPath(appRoot, 'shopify.app.toml'), validAppConfiguration('')) - await writeFile(joinPath(appRoot, 'index.ts'), 'export {}\n') - vi.stubEnv('INIT_CWD', appRoot) +describe('securityCheck --client-id lookup', () => { + const unknownClientId = new AbortError('No app with client ID unknown-client-id found') + + /** The real selection resolver, with the client ID lookup replaced. */ + function resolveSelectionWith(lookUpApp: (clientId: string) => Promise) { + return (options: AppSecuritySelectionOptions) => + resolveAppSecuritySelection(options, { + confirmScanWithoutAppConfig: async () => true, + pickClientId: async () => 'picked-client-id', + pickConfigFile: async () => 'shopify.app.toml', + lookUpApp, + }) + } + + async function createApp(directory: string): Promise { + const appRoot = await fileRealPath(directory) + await writeFile(joinPath(appRoot, 'shopify.app.toml'), validAppConfiguration('toml-client-id')) + vi.stubEnv('INIT_CWD', appRoot) + return appRoot + } - const stdout = await withCapturedStandardStreams(async ({stdout: captured}) => { - await securityCheck({...listFilesOptions, directory: appRoot, json: true}) - return captured() + test.each([false, true])( + 'looks up --client-id and proceeds when it is found (--list-files: %s)', + async (listFiles) => { + await inTemporaryDirectory(async (directory) => { + const appRoot = await createApp(directory) + const lookUpApp = vi.fn(async (_clientId: string) => {}) + const dependencies = testDependencies() + + const resolution = await resolveUncancelledSelection( + {...selectionOptions(), directory: appRoot, clientId: 'flag-client-id'}, + {resolveSelection: resolveSelectionWith(lookUpApp)}, + ) + await securityCheck(resolution, {listFiles}, dependencies) + + expect(lookUpApp).toHaveBeenCalledWith('flag-client-id') + if (listFiles) { + expect(dependencies.listFiles).toHaveBeenCalledWith(expect.objectContaining({clientId: 'flag-client-id'})) + } else { + expect(dependencies.writeArtifacts).toHaveBeenCalledWith(appRoot, 'flag-client-id', expect.anything()) + } }) + }, + ) + + test('aborts on an unknown --client-id before anything is gathered or written', async () => { + await inTemporaryDirectory(async (directory) => { + const appRoot = await createApp(directory) + + await expect( + resolveSecurityCheckSelection( + {...selectionOptions(), directory: appRoot, clientId: 'unknown-client-id'}, + { + resolveSelection: resolveSelectionWith(async () => { + throw unknownClientId + }), + }, + ), + ).rejects.toBe(unknownClientId) + + await expect(fileExists(joinPath(appRoot, '.shopify', 'app-security'))).resolves.toBe(false) + }) + }) + + test('does not look up the TOML client ID', async () => { + await inTemporaryDirectory(async (directory) => { + const appRoot = await createApp(directory) + const lookUpApp = vi.fn(async (_clientId: string) => {}) + + await resolveSecurityCheckSelection( + {...selectionOptions(), directory: appRoot}, + {resolveSelection: resolveSelectionWith(lookUpApp)}, + ) - expect(JSON.parse(stdout)).toEqual({files: ['index.ts', 'shopify.app.toml']}) + expect(lookUpApp).not.toHaveBeenCalled() }) }) }) diff --git a/packages/app/src/cli/services/security-check.ts b/packages/app/src/cli/services/security-check.ts index 05ae9cf75ef..5912ece939f 100644 --- a/packages/app/src/cli/services/security-check.ts +++ b/packages/app/src/cli/services/security-check.ts @@ -1,11 +1,6 @@ -import {securityExitCode, executeAppSecurity, listAppSecurityFiles} from './app-security-api.js' +import {executeAppSecurity, listAppSecurityFiles} from './app-security-api.js' import {writeCheckArtifacts} from './app-security-artifacts.js' -import deliverAppSecurityInstructions from './app-security-instructions.js' -import { - formatAppSecurityCommand, - resolveAppSecurityCommands, - type AppSecurityCommands, -} from './app-security-commands.js' +import {resolveAppSecurityCommands, type AppSecurityCommands} from './app-security-commands.js' import { effectiveClientId, mergeScanDirectories, @@ -14,14 +9,10 @@ import { resultsKey, type AppSecurityScanDirectory, type AppSecuritySelection, + type AppSecuritySelectionOptions, } from './app-security-selection.js' -import {encodeSecurityJson, toSecurityJson} from './security-json.js' -import {renderSecurityReport} from './security-output.js' import {recordAppSecurityMetadata, type AppSecurityMetadata} from './app-security-metadata.js' -import {outputResult} from '@shopify/cli-kit/node/output' -import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system' -import {cwd, relativePath} from '@shopify/cli-kit/node/path' -import {renderInfo, renderSelectPrompt, renderWarning} from '@shopify/cli-kit/node/ui' +import {CancelExecution} from '@shopify/cli-kit/node/error' import type {CheckArtifactPaths} from './app-security-artifacts.js' import type { AgentChecks, @@ -30,45 +21,60 @@ import type { ScanInput, ScanOptions, } from './app-security-engine/index.js' -import type {AppSecurityBlockingLevel, AppSecurityExecution} from './app-security-api.js' -import type {SecurityReportInput} from './security-output.js' -import type {RenderAlertOptions, RenderSelectPromptOptions} from '@shopify/cli-kit/node/ui' +import type {AppSecurityExecution} from './app-security-api.js' -interface SecurityOptions { +interface SecurityCheckSelectionOptions { directory: string configName?: string clientId?: string withoutAppConfig: boolean - json: boolean - verbose: boolean - blocking: AppSecurityBlockingLevel - yes: boolean - skipInstructions: boolean includeDirs: ReadonlyArray excludePatterns: ReadonlyArray noGitIgnore: boolean - /** Only resolve and gather: print the gathered paths and stop. */ - listFiles: boolean + allowPrompts: boolean } -/** What a run resolved, whether it scanned or only listed files. */ -interface SecurityCheckResolution { +/** What a run resolved before it scans or lists files. */ +export interface SecurityCheckResolution { + kind: 'resolved' selection: AppSecuritySelection resultsKey: string /** The commands that repeat this run, including its scope. */ commands: AppSecurityCommands + scope: AppSecurityScope + /** The real paths of the `--include-dir` directories. */ + includeDirectories: string[] + /** Whether choosing the selection showed prompts, which `commands.scan` skips next time. */ + prompted: boolean +} + +/** The run stopped before scanning because the user declined to scan without app configuration. */ +export interface SecurityCheckCancelled { + kind: 'cancelled' } -export type AppSecurityInstructionsDestination = 'copy' | 'print' | 'nothing' +export type SecurityCheckResult = + | { + kind: 'file-list' + resolution: SecurityCheckResolution + /** Relative to the app directory. */ + paths: string[] + ignoredScanDirectories: string[] + } + | { + kind: 'scan' + resolution: SecurityCheckResolution + scanDirectories: AppSecurityScanDirectory[] + execution: AppSecurityExecution + artifacts: CheckArtifactPaths + } + | SecurityCheckCancelled + +interface SecurityCheckSelectionDependencies { + resolveSelection(options: AppSecuritySelectionOptions): Promise +} -interface SecurityDependencies { - resolveSelection(options: { - path: string - config?: string - clientId?: string - withoutAppConfig: boolean - allowPrompts: boolean - }): Promise +interface SecurityCheckDependencies { execute(options: ScanInput & Required): Promise listFiles(options: ScanInput & Required): Promise<{paths: string[]; ignoredScanDirectories: string[]}> writeArtifacts( @@ -76,196 +82,98 @@ interface SecurityDependencies { resultsKey: string, artifacts: {deterministicFindings: DeterministicFindingsDocument; agentChecks: AgentChecks}, ): Promise - canPrompt(): boolean - selectInstructionsDestination(agentCheckCount: number): Promise - deliverInstructions(options: { - appDirectory: string - resultsKey: string - copy: boolean - scanScope: AppSecurityScope - commands: AppSecurityCommands - }): Promise - output(content: string): void - renderInfo(options: RenderAlertOptions): void - renderWarning(options: RenderAlertOptions): void - renderReport(input: SecurityReportInput): void - setExitCode(exitCode: number): void recordMetadata(fields: AppSecurityMetadata): Promise } -export function appSecurityInstructionsPrompt( - agentCheckCount: number, -): RenderSelectPromptOptions { - return { - message: `${agentCheckCount} recommended agent ${agentCheckCount === 1 ? 'check' : 'checks'} available to complete your scan. How do you want to pass that prompt to your agent?`, - choices: [ - {label: 'Copy instructions to the clipboard', value: 'copy'}, - {label: 'Print instructions to the terminal', value: 'print'}, - {label: 'Nothing', value: 'nothing'}, - ], - defaultValue: 'copy', - } +const defaultSelectionDependencies: SecurityCheckSelectionDependencies = { + resolveSelection: resolveAppSecuritySelection, } -const defaultDependencies: SecurityDependencies = { - resolveSelection: resolveAppSecuritySelection, +const defaultDependencies: SecurityCheckDependencies = { execute: executeAppSecurity, listFiles: listAppSecurityFiles, writeArtifacts: writeCheckArtifacts, - canPrompt: terminalSupportsPrompting, - selectInstructionsDestination: (agentCheckCount) => - renderSelectPrompt(appSecurityInstructionsPrompt(agentCheckCount)), - deliverInstructions: deliverAppSecurityInstructions, - output: outputResult, - renderInfo, - renderWarning, - renderReport: renderSecurityReport, - setExitCode: (exitCode) => { - process.exitCode = exitCode - }, recordMetadata: recordAppSecurityMetadata, } -async function instructionsDestination( - options: SecurityOptions, - dependencies: SecurityDependencies, - canPrompt: boolean, - agentCheckCount: number, -): Promise { - if (options.json || options.skipInstructions) return 'nothing' - if (options.yes) return 'print' - if (!canPrompt) return 'nothing' - return dependencies.selectInstructionsDestination(agentCheckCount) -} - -function securityReportInput( - execution: AppSecurityExecution, - artifacts: CheckArtifactPaths, - verbose: boolean, - commands: AppSecurityCommands, - selection: AppSecuritySelection, - scanDirectories: AppSecurityScanDirectory[], -): SecurityReportInput { - return { - scan: execution.scan, - selection, - scanDirectories, - engine: execution.engine, - verbose, - elapsedMilliseconds: execution.elapsedMilliseconds, - commands, - deterministicFindingsPath: artifacts.deterministicFindingsPath, - agentChecksPath: artifacts.agentChecksPath, - agentCheckCount: execution.agentChecks.checks.length, - } -} - -function renderIgnoredScanDirectoryWarnings(ignoredScanDirectories: string[], dependencies: SecurityDependencies) { - for (const directory of ignoredScanDirectories) { - dependencies.renderWarning({ - headline: `${relativePath(cwd(), directory) || '.'} is ignored by Git, so only the files Git tracks in it are scanned.`, - body: ['Use', {command: '--no-git-ignore'}, 'to scan everything in it.'], - }) - } -} - /** - * Scans the app and replaces deterministic-findings.json and agent-checks.json. Scanning never reads or - * changes the agent's recorded findings, so it's always safe to run again. - * - * With `listFiles`, it only resolves and gathers: it prints the gathered paths and writes nothing. - * `directory` is the `--path` value, an absolute path. + * Resolves what `check` scans: the `--include-dir` directories, then the selection, which prompts when + * `allowPrompts` is set and the selection needs a choice. `directory` is the `--path` value, an absolute path. + * Cancelled when the user declines to scan without app configuration. */ -export default async function securityCheck( - options: SecurityOptions, - dependencies: SecurityDependencies = defaultDependencies, -): Promise { +export async function resolveSecurityCheckSelection( + options: SecurityCheckSelectionOptions, + dependencies: SecurityCheckSelectionDependencies = defaultSelectionDependencies, +): Promise { // Resolved first so a mistyped directory fails before any prompt. const includeDirectories = await resolveIncludeDirectories(options.includeDirs) - const canPrompt = !options.json && !options.listFiles && dependencies.canPrompt() - const selection = await dependencies.resolveSelection({ - path: options.directory, - config: options.configName, - clientId: options.clientId, - withoutAppConfig: options.withoutAppConfig, - allowPrompts: canPrompt, - }) - const {appDirectory} = selection + let selection: AppSecuritySelection + try { + selection = await dependencies.resolveSelection({ + path: options.directory, + config: options.configName, + clientId: options.clientId, + withoutAppConfig: options.withoutAppConfig, + allowPrompts: options.allowPrompts, + validateClientIdFlag: true, + }) + } catch (error) { + if (!(error instanceof CancelExecution)) throw error + return {kind: 'cancelled'} + } const scope: AppSecurityScope = { include_dirs: [...options.includeDirs], excludes: [...options.excludePatterns], no_git_ignore: options.noGitIgnore, } - const commands = resolveAppSecurityCommands(selection, options.directory, scope) - const resolution = {selection, resultsKey: resultsKey(selection), commands} - // Prompts are shown when no TOML was found and `--without-app-config` wasn't passed, or when `check` asked which TOML - // to scan. - const prompted = selection.kind === 'no-config' ? !options.withoutAppConfig : selection.appConfigFilePicked === true - if (prompted) { - dependencies.renderInfo({ - headline: 'To skip these prompts next time, run:', - body: [{command: formatAppSecurityCommand(commands.scan)}], - }) + return { + kind: 'resolved', + selection, + resultsKey: resultsKey(selection), + commands: resolveAppSecurityCommands(selection, options.directory, scope), + scope, + includeDirectories, + // Prompts are shown when no TOML was found and `--without-app-config` wasn't passed, or when `check` asked + // which TOML to scan. + prompted: selection.kind === 'no-config' ? !options.withoutAppConfig : selection.appConfigFilePicked === true, } - const {scanDirectories, requestedScanDirectories} = mergeScanDirectories(appDirectory, includeDirectories) +} +/** + * Scans the app and replaces deterministic-findings.json and agent-checks.json. Scanning never reads or + * changes the agent's recorded findings, so it's always safe to run again. It shows nothing in the terminal. + * + * With `listFiles`, it only gathers the files it would scan and writes nothing. + */ +export default async function securityCheck( + resolution: SecurityCheckResolution, + options: {listFiles: boolean}, + dependencies: SecurityCheckDependencies = defaultDependencies, +): Promise { + const {selection, scope} = resolution + const {appDirectory} = selection + const {scanDirectories, requestedScanDirectories} = mergeScanDirectories(appDirectory, resolution.includeDirectories) const scanOptions = { appDirectory, scanDirectories: scanDirectories.map(({directory}) => directory), requestedScanDirectories, appConfigFilePath: selection.kind === 'config' ? selection.appConfigFilePath : undefined, clientId: effectiveClientId(selection), - includeDirs: options.includeDirs, - excludePatterns: options.excludePatterns, - noGitIgnore: options.noGitIgnore, + includeDirs: scope.include_dirs, + excludePatterns: scope.excludes, + noGitIgnore: scope.no_git_ignore, } if (options.listFiles) { const {paths, ignoredScanDirectories} = await dependencies.listFiles(scanOptions) - renderIgnoredScanDirectoryWarnings(ignoredScanDirectories, dependencies) - if (options.json) { - dependencies.output(JSON.stringify({files: paths}, null, 2)) - } else if (paths.length > 0) { - dependencies.output(paths.join('\n')) - } - return resolution + return {kind: 'file-list', resolution, paths, ignoredScanDirectories} } const execution = await dependencies.execute(scanOptions) - renderIgnoredScanDirectoryWarnings(execution.ignoredScanDirectories, dependencies) await dependencies.recordMetadata({num_security_findings: execution.scan.issues.length}) - const artifacts = await dependencies.writeArtifacts(appDirectory, resultsKey(selection), { + const artifacts = await dependencies.writeArtifacts(appDirectory, resolution.resultsKey, { deterministicFindings: execution.deterministicFindings, agentChecks: execution.agentChecks, }) - - if (options.json) { - dependencies.output( - encodeSecurityJson(toSecurityJson(execution, artifacts.agentChecksPath, selection, scanDirectories)), - ) - } else { - dependencies.renderReport( - securityReportInput(execution, artifacts, options.verbose, commands, selection, scanDirectories), - ) - } - - const destination = await instructionsDestination( - options, - dependencies, - canPrompt, - execution.agentChecks.checks.length, - ) - if (destination !== 'nothing') { - await dependencies.deliverInstructions({ - appDirectory, - resultsKey: resultsKey(selection), - copy: destination === 'copy', - scanScope: scope, - commands, - }) - } - - const exitCode = securityExitCode(execution, options.blocking) - if (exitCode !== 0) dependencies.setExitCode(exitCode) - return resolution + return {kind: 'scan', resolution, scanDirectories, execution, artifacts} } diff --git a/packages/app/src/cli/services/security-instructions-json.ts b/packages/app/src/cli/services/security-instructions-json.ts new file mode 100644 index 00000000000..df536edf05d --- /dev/null +++ b/packages/app/src/cli/services/security-instructions-json.ts @@ -0,0 +1,31 @@ +import {defineJsonOutputSchema} from '@shopify/cli-kit/node/json-output-schema' +import {zod} from '@shopify/cli-kit/node/schema' + +/** Shared by `instructions --json` and `check --json`, so an agent reads the instructions the same way from both. */ +export const appSecurityInstructionsSchema = zod + .object({ + content: zod.string(), + copiedToClipboard: zod.boolean(), + /** The file written by `instructions --write`. */ + path: zod.string().nullable(), + }) + .strict() + +export type AppSecurityInstructionsJson = zod.infer + +export const securityInstructionsJsonOutputSchema = defineJsonOutputSchema({ + name: 'AppSecurityInstructionsResult', + schema: zod.object({instructions: appSecurityInstructionsSchema}).strict(), + definitions: {AppSecurityInstructions: appSecurityInstructionsSchema}, +}) + +/** + * The instructions as delivered: copied, written to `writePath`, or neither. Build it after the delivery succeeds, + * so the result never reports a copy or a file that failed. + */ +export function toAppSecurityInstructionsJson( + content: string, + delivery: {copy: boolean; writePath?: string}, +): AppSecurityInstructionsJson { + return {content, copiedToClipboard: delivery.copy, path: delivery.writePath ?? null} +} diff --git a/packages/app/src/cli/services/security-json.ts b/packages/app/src/cli/services/security-json.ts deleted file mode 100644 index 165d9279f66..00000000000 --- a/packages/app/src/cli/services/security-json.ts +++ /dev/null @@ -1,41 +0,0 @@ -import {clientIdSource, effectiveClientId} from './app-security-selection.js' -import type {AppSecurityEngineMetadata, AppSecurityExecution} from './app-security-api.js' -import type {AppSecurityScanDirectory, AppSecuritySelection} from './app-security-selection.js' -import type {DeterministicFindingsDocument} from './app-security-engine/index.js' - -interface AppSecurityJsonResult { - engine: AppSecurityEngineMetadata - selection: { - app_directory: string - app_config_file: string | null - client_id: string | null - client_id_source: 'config' | 'flag' | 'picker' | null - scan_directories: AppSecurityScanDirectory[] - } - deterministic_findings: DeterministicFindingsDocument - agent_checks_path: string -} - -export function toSecurityJson( - execution: Pick, - agentChecksPath: string, - selection: AppSecuritySelection, - scanDirectories: AppSecurityScanDirectory[], -): AppSecurityJsonResult { - return { - engine: execution.engine, - selection: { - app_directory: selection.appDirectory, - app_config_file: selection.kind === 'config' ? selection.appConfigFilePath : null, - client_id: effectiveClientId(selection) ?? null, - client_id_source: clientIdSource(selection) ?? null, - scan_directories: scanDirectories, - }, - deterministic_findings: execution.deterministicFindings, - agent_checks_path: agentChecksPath, - } -} - -export function encodeSecurityJson(result: AppSecurityJsonResult): string { - return `${JSON.stringify(result, null, 2)}\n` -} diff --git a/packages/app/src/cli/services/security-output.test.ts b/packages/app/src/cli/services/security-output.test.ts index aa840d26a19..05d64e85ec7 100644 --- a/packages/app/src/cli/services/security-output.test.ts +++ b/packages/app/src/cli/services/security-output.test.ts @@ -1,10 +1,26 @@ -import {buildSecurityAlert} from './security-output.js' +import { + appSecurityInstructionsPrompt, + buildSecurityAlert, + renderSecurityCheckPromptsNotice, + renderSecurityCheckResult, +} from './security-output.js' import {formatAppSecurityCommand, resolveAppSecurityCommands} from './app-security-commands.js' +import {appSecurityInstructions} from './app-security-instructions.js' +import {runWithCommandEventsForCommand} from '@shopify/cli-kit/node/command-events' +import {unstyled} from '@shopify/cli-kit/node/output' import {cwd, joinPath} from '@shopify/cli-kit/node/path' -import {describe, expect, test} from 'vitest' -import type {SecurityReportInput} from './security-output.js' +import {withCapturedStandardStreams} from '@shopify/cli-kit/node/testing/output' +import {afterEach, describe, expect, test, vi} from 'vitest' +import type {AppSecurityInstructionsDestination, SecurityReportInput} from './security-output.js' +import type {AppSecurityExecution} from './app-security-api.js' +import type {SecurityCheckResolution, SecurityCheckResult} from './security-check.js' import type {AppSecuritySelection} from './app-security-selection.js' -import type {ScanResult} from './app-security-engine/index.js' +import type { + AgentChecks, + AppSecurityScope, + DeterministicFindingsDocument, + ScanResult, +} from './app-security-engine/index.js' const appSelection: AppSecuritySelection = { kind: 'config', @@ -439,3 +455,458 @@ describe('buildSecurityAlert', () => { expect(serialized).not.toContain('shpat_') }) }) + +const deterministicFindings: DeterministicFindingsDocument = { + schema_version: 1, + source: 'deterministic', + engine: {name: 'shopify-app-security', version: '1.2.3', ruleset: '2026.08.28'}, + generated_at: '2026-08-24T00:00:00.000Z', + detection: scanWithIssues.detection, + coverage: { + files_scanned: 12, + files_skipped: [], + gaps: [], + scope: {include_dirs: [], excludes: [], no_git_ignore: false}, + scan_directories: [{directory: '.', origin: 'app_directory'}], + }, + checks: [], +} + +const agentChecks: AgentChecks = { + schema_version: 1, + engine: {name: 'shopify-app-security', version: '1.2.3'}, + generated_at: '2026-08-24T00:00:00.000Z', + checks: Array.from({length: 31}, (_, index) => ({ + id: `CHECK_${index}`, + version: 1, + prompt: 'prompt', + severity: 'medium' as const, + docs_url: `https://shopify.dev/docs/apps/build/security/app-security-checks/check-${index}`, + })), + instructions: 'review', +} + +const cleanExecution: AppSecurityExecution = { + scan: {...scanWithIssues, issues: []}, + ignoredScanDirectories: [], + deterministicFindings, + agentChecks, + engine, + elapsedMilliseconds: 125, +} + +const checkArtifacts = { + deterministicFindingsPath: '/tmp/app/.shopify/app-security/shopify.app/deterministic-findings.json', + agentChecksPath: '/tmp/app/.shopify/app-security/shopify.app/agent-checks.json', +} + +const noScope: AppSecurityScope = {include_dirs: [], excludes: [], no_git_ignore: false} + +function checkResolution(scope: AppSecurityScope = noScope): SecurityCheckResolution { + return { + kind: 'resolved', + selection: appSelection, + resultsKey: 'shopify.app', + commands: resolveAppSecurityCommands(appSelection, cwd(), scope), + scope, + includeDirectories: [], + prompted: false, + } +} + +function scanResult( + overrides: {execution?: AppSecurityExecution; resolution?: SecurityCheckResolution} = {}, +): SecurityCheckResult { + return { + kind: 'scan', + resolution: overrides.resolution ?? checkResolution(), + scanDirectories: [{directory: '/tmp/app', origin: 'app_directory'}], + execution: overrides.execution ?? cleanExecution, + artifacts: checkArtifacts, + } +} + +function fileListResult(paths: string[], ignoredScanDirectories: string[] = []): SecurityCheckResult { + return {kind: 'file-list', resolution: checkResolution(), paths, ignoredScanDirectories} +} + +/** The instructions `check` offers after a scan of `checkResolution(scope)`. */ +function postScanInstructions(scope: AppSecurityScope = noScope): string { + const {selection, resultsKey, commands} = checkResolution(scope) + return appSecurityInstructions({appDirectory: selection.appDirectory, resultsKey, commands, scanScope: scope}) +} + +function renderOptions(overrides: Partial[1]> = {}) { + return { + format: 'text' as const, + verbose: false, + blocking: 'none' as const, + yes: false, + skipInstructions: false, + canPrompt: false, + ...overrides, + } +} + +function renderDependencies(destination: AppSecurityInstructionsDestination = 'nothing') { + return { + selectInstructionsDestination: vi.fn(async (_agentCheckCount: number) => destination), + deliverInstructions: vi.fn(async (_content: string, _delivery: {copy: boolean}) => {}), + setExitCode: vi.fn(), + } +} + +interface CapturedStreams { + stdout(): string + stderr(): string +} + +/** Runs `render` as a command in `format` would, with the real output writers, and captures both streams. */ +async function captureOutput(format: 'json' | 'text', render: (streams: CapturedStreams) => Promise | void) { + return withCapturedStandardStreams(async (streams) => { + await runWithCommandEventsForCommand(format === 'json' ? ['--json'] : [], () => render(streams)) + return {stdout: streams.stdout(), stderr: streams.stderr()} + }) +} + +function renderCheck( + result: SecurityCheckResult, + options: ReturnType, + dependencies: ReturnType, +) { + return captureOutput(options.format, () => renderSecurityCheckResult(result, options, dependencies)) +} + +/** Every line on stderr parsed as a side event: in JSON mode nothing else may be written there. */ +function sideEvents(stderr: string): unknown[] { + return stderr + .split('\n') + .filter((line) => line !== '') + .map((line) => JSON.parse(line)) +} + +/** Banner text with its styling and frame removed, so a wrapped line reads as one sentence. */ +function bannerText(stderr: string): string { + return unstyled(stderr) + .replaceAll(/[│╭╮╰╯─]/g, ' ') + .replaceAll(/\s+/g, ' ') +} + +describe('renderSecurityCheckResult', () => { + afterEach(() => { + vi.unstubAllEnvs() + }) + + test('prints one JSON document on stdout with the instructions --yes chose, and nothing on stderr', async () => { + const dependencies = renderDependencies() + + const {stdout, stderr} = await renderCheck(scanResult(), renderOptions({format: 'json', yes: true}), dependencies) + + expect(JSON.parse(stdout)).toEqual({ + status: 'success', + selection: { + directory: '/tmp/app', + configPath: '/tmp/app/shopify.app.toml', + clientId: 'toml-client-id', + clientIdSource: 'config', + scanDirectories: [{directory: '/tmp/app', origin: 'app-directory'}], + }, + deterministicFindings, + agentChecksPath: checkArtifacts.agentChecksPath, + instructions: {content: postScanInstructions(), copiedToClipboard: false, path: null}, + }) + expect(stderr).toBe('') + expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() + expect(dependencies.deliverInstructions).toHaveBeenCalledWith(postScanInstructions(), {copy: false}) + }) + + test('asks for the instructions before printing the JSON result, and puts the copied instructions in it', async () => { + const dependencies = renderDependencies() + let stdoutWhenAsked: string | undefined + + const {stdout} = await captureOutput('json', async (streams) => { + dependencies.selectInstructionsDestination.mockImplementation(async () => { + stdoutWhenAsked = streams.stdout() + return 'copy' + }) + await renderSecurityCheckResult(scanResult(), renderOptions({format: 'json', canPrompt: true}), dependencies) + }) + + expect(stdoutWhenAsked).toBe('') + expect(dependencies.selectInstructionsDestination).toHaveBeenCalledWith(31) + expect(dependencies.deliverInstructions).toHaveBeenCalledWith(postScanInstructions(), {copy: true}) + expect(JSON.parse(stdout).instructions).toEqual({ + content: postScanInstructions(), + copiedToClipboard: true, + path: null, + }) + }) + + test.each([ + ['no instructions are chosen', {canPrompt: true, skipInstructions: false}], + ['--skip-instructions is passed', {canPrompt: true, skipInstructions: true}], + ['the terminal is not interactive', {canPrompt: false, skipInstructions: false}], + ])('puts null instructions in the JSON result when %s', async (_, {canPrompt, skipInstructions}) => { + const dependencies = renderDependencies('nothing') + + const {stdout} = await renderCheck( + scanResult(), + renderOptions({format: 'json', canPrompt, skipInstructions}), + dependencies, + ) + + expect(dependencies.deliverInstructions).not.toHaveBeenCalled() + expect(JSON.parse(stdout).instructions).toBeNull() + }) + + test('renders the report on stderr, then offers the instructions and prints the chosen ones on stdout', async () => { + const dependencies = renderDependencies() + let stderrWhenAsked: string | undefined + + const {stdout, stderr} = await captureOutput('text', async (streams) => { + dependencies.selectInstructionsDestination.mockImplementation(async () => { + stderrWhenAsked = streams.stderr() + return 'print' + }) + await renderSecurityCheckResult(scanResult(), renderOptions({canPrompt: true}), dependencies) + }) + + expect(bannerText(stderrWhenAsked ?? '')).toContain('No security issues found.') + expect(bannerText(stderr)).toContain( + 'Agent security check instructions: .shopify/app-security/shopify.app/agent-checks.json', + ) + expect(dependencies.deliverInstructions).toHaveBeenCalledWith(postScanInstructions(), {copy: false}) + expect(stdout).toBe(`${postScanInstructions()}\n`) + }) + + test('confirms copied instructions without printing them', async () => { + const dependencies = renderDependencies('copy') + + const {stdout, stderr} = await renderCheck(scanResult(), renderOptions({canPrompt: true}), dependencies) + + expect(dependencies.deliverInstructions).toHaveBeenCalledWith(postScanInstructions(), {copy: true}) + expect(bannerText(stderr)).toContain('Copied app security check instructions to the clipboard') + expect(stdout).toBe('') + }) + + test('--yes prints the instructions without prompting, including in CI', async () => { + const dependencies = renderDependencies() + + const {stdout} = await renderCheck(scanResult(), renderOptions({yes: true}), dependencies) + + expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() + expect(stdout).toBe(`${postScanInstructions()}\n`) + }) + + test.each([ + ['in CI or another non-interactive environment', {canPrompt: false, skipInstructions: false}], + ['with --skip-instructions', {canPrompt: true, skipInstructions: true}], + ])('does not offer the instructions %s', async (_, {canPrompt, skipInstructions}) => { + const dependencies = renderDependencies('print') + + const {stdout} = await renderCheck(scanResult(), renderOptions({canPrompt, skipInstructions}), dependencies) + + expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() + expect(dependencies.deliverInstructions).not.toHaveBeenCalled() + expect(stdout).toBe('') + }) + + test('builds the instructions from the exact scope and the commands of the run', async () => { + const scope = {include_dirs: ['backend', './backend/'], excludes: ['**/generated', '!keep'], no_git_ignore: true} + + const {stdout} = await renderCheck( + scanResult({resolution: checkResolution(scope)}), + renderOptions({format: 'json', yes: true}), + renderDependencies(), + ) + + const {content} = JSON.parse(stdout).instructions + expect(content).toBe(postScanInstructions(scope)) + expect(content).toContain(JSON.stringify(scope)) + }) + + test('warns once for each scan directory that Git ignores, relative to the working directory', async () => { + vi.stubEnv('INIT_CWD', '/tmp') + + const {stderr} = await renderCheck( + scanResult({execution: {...cleanExecution, ignoredScanDirectories: ['/tmp/app', '/tmp']}}), + renderOptions(), + renderDependencies(), + ) + + const text = bannerText(stderr) + expect(text.match(/is ignored by Git/g)).toHaveLength(2) + expect(text).toContain('app is ignored by Git, so only the files Git tracks in it are scanned.') + expect(text).toContain('. is ignored by Git, so only the files Git tracks in it are scanned.') + }) + + test('warns about an ignored scan directory as a diagnostic event with --json, keeping stdout one document', async () => { + vi.stubEnv('INIT_CWD', '/tmp') + + const {stdout, stderr} = await renderCheck( + scanResult({execution: {...cleanExecution, ignoredScanDirectories: ['/tmp/app']}}), + renderOptions({format: 'json'}), + renderDependencies(), + ) + + expect(sideEvents(stderr)).toEqual([ + expect.objectContaining({ + type: 'diagnostic', + level: 'warning', + message: + 'app is ignored by Git, so only the files Git tracks in it are scanned. Use --no-git-ignore to scan everything in it.', + }), + ]) + expect(JSON.parse(stdout)).toHaveProperty('agentChecksPath') + }) + + test.each(['text', 'json'] as const)('sets a blocking exit code from the findings (%s)', async (format) => { + const dependencies = renderDependencies() + + await renderCheck( + scanResult({execution: {...cleanExecution, scan: scanWithIssues}}), + renderOptions({format, blocking: 'high'}), + dependencies, + ) + + expect(dependencies.setExitCode).toHaveBeenCalledWith(1) + }) + + test('keeps the default exit code when no finding reaches the blocking level', async () => { + const dependencies = renderDependencies() + + await renderCheck(scanResult(), renderOptions({blocking: 'low'}), dependencies) + + expect(dependencies.setExitCode).not.toHaveBeenCalled() + }) +}) + +describe('renderSecurityCheckResult --list-files', () => { + afterEach(() => { + vi.unstubAllEnvs() + }) + + test('prints each gathered path on its own line, relative to the app directory, and nothing else', async () => { + const dependencies = renderDependencies('print') + + const {stdout, stderr} = await renderCheck( + fileListResult(['../backend/server.ts', 'app/routes/index.ts', 'shopify.app.toml']), + renderOptions({canPrompt: true}), + dependencies, + ) + + expect(stdout).toBe('../backend/server.ts\napp/routes/index.ts\nshopify.app.toml\n') + expect(stderr).toBe('') + expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() + expect(dependencies.setExitCode).not.toHaveBeenCalled() + }) + + test('prints the absolute paths as one JSON document with --json', async () => { + const {stdout, stderr} = await renderCheck( + fileListResult(['../backend/server.ts', 'shopify.app.toml']), + renderOptions({format: 'json'}), + renderDependencies(), + ) + + expect(JSON.parse(stdout)).toEqual({ + status: 'success', + files: ['/tmp/backend/server.ts', joinPath('/tmp/app', 'shopify.app.toml')], + }) + expect(stderr).toBe('') + }) + + test.each([ + ['text', ''], + ['json', '{\n "status": "success",\n "files": []\n}\n'], + ] as const)('prints nothing for no gathered file, and an empty list with --json (%s)', async (format, expected) => { + const {stdout} = await renderCheck(fileListResult([]), renderOptions({format}), renderDependencies()) + + expect(stdout).toBe(expected) + }) + + test.each(['text', 'json'] as const)('warns about an ignored scan directory (%s)', async (format) => { + vi.stubEnv('INIT_CWD', '/tmp') + + const {stderr} = await renderCheck( + fileListResult(['shopify.app.toml'], ['/tmp/app']), + renderOptions({format}), + renderDependencies(), + ) + + if (format === 'json') { + expect(sideEvents(stderr)).toEqual([expect.objectContaining({type: 'diagnostic', level: 'warning'})]) + } else { + expect(bannerText(stderr)).toContain('app is ignored by Git, so only the files Git tracks in it are scanned.') + } + }) +}) + +describe('renderSecurityCheckResult cancelled', () => { + test.each([ + ['json', '{\n "status": "cancelled"\n}\n'], + ['text', ''], + ] as const)( + 'prints only the cancelled status with --json, and keeps the default exit code (%s)', + async (format, expected) => { + const dependencies = renderDependencies('print') + + const {stdout, stderr} = await renderCheck( + {kind: 'cancelled'}, + renderOptions({format, canPrompt: true}), + dependencies, + ) + + expect(stdout).toBe(expected) + expect(stderr).toBe('') + expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() + expect(dependencies.setExitCode).not.toHaveBeenCalled() + }, + ) +}) + +describe('renderSecurityCheckPromptsNotice', () => { + const commands = checkResolution().commands + + test('shows the command that skips the prompts in a banner', async () => { + const {stdout, stderr} = await captureOutput('text', () => renderSecurityCheckPromptsNotice(commands, 'text')) + + expect(bannerText(stderr)).toContain( + `To skip these prompts next time, run: \`${formatAppSecurityCommand(commands.scan)}\``, + ) + expect(stdout).toBe('') + }) + + test('shows the command that skips the prompts as a diagnostic event with --json', async () => { + const {stdout, stderr} = await captureOutput('json', () => renderSecurityCheckPromptsNotice(commands, 'json')) + + expect(sideEvents(stderr)).toEqual([ + expect.objectContaining({ + type: 'diagnostic', + level: 'info', + message: `To skip these prompts next time, run: ${formatAppSecurityCommand(commands.scan)}`, + }), + ]) + expect(stdout).toBe('') + }) +}) + +describe('appSecurityInstructionsPrompt', () => { + test('prioritizes copying instructions for the recommended agent checks', () => { + expect(appSecurityInstructionsPrompt(31)).toEqual({ + message: + '31 recommended agent checks available to complete your scan. How do you want to pass that prompt to your agent?', + choices: [ + {label: 'Copy instructions to the clipboard', value: 'copy'}, + {label: 'Print instructions to the terminal', value: 'print'}, + {label: 'Nothing', value: 'nothing'}, + ], + defaultValue: 'copy', + }) + }) + + test('names a single recommended agent check in the singular', () => { + expect(appSecurityInstructionsPrompt(1).message).toBe( + '1 recommended agent check available to complete your scan. How do you want to pass that prompt to your agent?', + ) + }) +}) diff --git a/packages/app/src/cli/services/security-output.ts b/packages/app/src/cli/services/security-output.ts index 9d3326ba9ed..936eb953592 100644 --- a/packages/app/src/cli/services/security-output.ts +++ b/packages/app/src/cli/services/security-output.ts @@ -1,4 +1,14 @@ +import {securityExitCode, type AppSecurityBlockingLevel} from './app-security-api.js' import {formatAppSecurityCommand, type AppSecurityCommands} from './app-security-commands.js' +import {appSecurityInstructions} from './app-security-instructions.js' +import {deliverAppSecurityInstructions, renderAppSecurityInstructions} from './app-security-instructions-output.js' +import { + securityCheckJsonOutputSchema, + toSecurityCheckCancelledJson, + toSecurityCheckFileListJson, + toSecurityCheckJson, +} from './security-check-json.js' +import {toAppSecurityInstructionsJson, type AppSecurityInstructionsJson} from './security-instructions-json.js' import { effectiveClientId, selectedConfigFileName, @@ -14,9 +24,18 @@ import { type ScanResult, type Severity, } from './app-security-engine/index.js' -import {relativePath} from '@shopify/cli-kit/node/path' -import {renderError, renderSuccess, renderWarning} from '@shopify/cli-kit/node/ui' -import type {AlertCustomSection, InlineToken, RenderAlertOptions, Token, TokenItem} from '@shopify/cli-kit/node/ui' +import {outputInfo, outputResult, outputWarn} from '@shopify/cli-kit/node/output' +import {cwd, relativePath} from '@shopify/cli-kit/node/path' +import {renderError, renderInfo, renderSelectPrompt, renderSuccess, renderWarning} from '@shopify/cli-kit/node/ui' +import type {SecurityCheckResult} from './security-check.js' +import type { + AlertCustomSection, + InlineToken, + RenderAlertOptions, + RenderSelectPromptOptions, + Token, + TokenItem, +} from '@shopify/cli-kit/node/ui' interface SecurityEngineMetadata { name: string @@ -74,7 +93,7 @@ export function buildSecurityAlert(input: SecurityReportInput): SecurityAlert { } } -export function renderSecurityReport(input: SecurityReportInput): void { +function renderSecurityReport(input: SecurityReportInput): void { const {type, options} = buildSecurityAlert(input) if (type === 'success') { renderSuccess(options) @@ -87,6 +106,166 @@ export function renderSecurityReport(input: SecurityReportInput): void { renderError(options) } +type SecurityCheckOutputFormat = 'json' | 'text' + +type SecurityCheckScan = Extract + +export type AppSecurityInstructionsDestination = 'copy' | 'print' | 'nothing' + +export interface SecurityCheckRenderOptions { + format: SecurityCheckOutputFormat + verbose: boolean + blocking: AppSecurityBlockingLevel + yes: boolean + skipInstructions: boolean + canPrompt: boolean +} + +interface SecurityCheckRenderDependencies { + selectInstructionsDestination(agentCheckCount: number): Promise + deliverInstructions(content: string, delivery: {copy: boolean}): Promise + setExitCode(exitCode: number): void +} + +export function appSecurityInstructionsPrompt( + agentCheckCount: number, +): RenderSelectPromptOptions { + return { + message: `${agentCheckCount} recommended agent ${agentCheckCount === 1 ? 'check' : 'checks'} available to complete your scan. How do you want to pass that prompt to your agent?`, + choices: [ + {label: 'Copy instructions to the clipboard', value: 'copy'}, + {label: 'Print instructions to the terminal', value: 'print'}, + {label: 'Nothing', value: 'nothing'}, + ], + defaultValue: 'copy', + } +} + +const defaultCheckRenderDependencies: SecurityCheckRenderDependencies = { + selectInstructionsDestination: (agentCheckCount) => + renderSelectPrompt(appSecurityInstructionsPrompt(agentCheckCount)), + deliverInstructions: deliverAppSecurityInstructions, + setExitCode: (exitCode) => { + process.exitCode = exitCode + }, +} + +/** Shows the command that repeats a run without the prompts it just showed. */ +export function renderSecurityCheckPromptsNotice( + commands: AppSecurityCommands, + format: SecurityCheckOutputFormat, +): void { + const scanCommand = formatAppSecurityCommand(commands.scan) + if (format === 'json') { + outputInfo(`To skip these prompts next time, run: ${scanCommand}`) + } else { + renderInfo({headline: 'To skip these prompts next time, run:', body: [{command: scanCommand}]}) + } +} + +/** + * Presents a check result: the gathered files, or the scan report or JSON result with the coding-agent instructions + * chosen for it. In JSON mode the instructions are chosen and delivered before the result is printed, since they're + * part of it. A finding at the `blocking` level sets the exit code. A cancelled run prints nothing in text mode, since + * the user just declined the prompt. + */ +export async function renderSecurityCheckResult( + result: SecurityCheckResult, + options: SecurityCheckRenderOptions, + dependencies: SecurityCheckRenderDependencies = defaultCheckRenderDependencies, +): Promise { + if (result.kind === 'cancelled') { + if (options.format === 'json') outputResult(securityCheckJsonOutputSchema.encode(toSecurityCheckCancelledJson())) + return + } + + if (result.kind === 'file-list') { + warnAboutIgnoredScanDirectories(result.ignoredScanDirectories, options.format) + if (options.format === 'json') { + const {appDirectory} = result.resolution.selection + outputResult(securityCheckJsonOutputSchema.encode(toSecurityCheckFileListJson(appDirectory, result.paths))) + } else if (result.paths.length > 0) { + outputResult(result.paths.join('\n')) + } + return + } + + const {execution, artifacts, resolution, scanDirectories} = result + warnAboutIgnoredScanDirectories(execution.ignoredScanDirectories, options.format) + if (options.format === 'json') { + const instructions = await deliverChosenInstructions(result, options, dependencies) + outputResult( + securityCheckJsonOutputSchema.encode( + toSecurityCheckJson(execution, artifacts.agentChecksPath, resolution.selection, scanDirectories, instructions), + ), + ) + } else { + renderSecurityReport(securityReportInput(result, options.verbose)) + const instructions = await deliverChosenInstructions(result, options, dependencies) + if (instructions) renderAppSecurityInstructions(instructions) + } + + const exitCode = securityExitCode(execution, options.blocking) + if (exitCode !== 0) dependencies.setExitCode(exitCode) +} + +async function deliverChosenInstructions( + result: SecurityCheckScan, + options: SecurityCheckRenderOptions, + dependencies: SecurityCheckRenderDependencies, +): Promise { + const destination = await instructionsDestination(options, dependencies, result.execution.agentChecks.checks.length) + if (destination === 'nothing') return null + const {selection, resultsKey, commands, scope} = result.resolution + const content = appSecurityInstructions({ + appDirectory: selection.appDirectory, + resultsKey, + commands, + scanScope: scope, + }) + const delivery = {copy: destination === 'copy'} + await dependencies.deliverInstructions(content, delivery) + return toAppSecurityInstructionsJson(content, delivery) +} + +async function instructionsDestination( + options: SecurityCheckRenderOptions, + dependencies: SecurityCheckRenderDependencies, + agentCheckCount: number, +): Promise { + if (options.skipInstructions) return 'nothing' + if (options.yes) return 'print' + if (!options.canPrompt) return 'nothing' + return dependencies.selectInstructionsDestination(agentCheckCount) +} + +function securityReportInput(result: SecurityCheckScan, verbose: boolean): SecurityReportInput { + const {execution, artifacts, resolution, scanDirectories} = result + return { + scan: execution.scan, + selection: resolution.selection, + scanDirectories, + engine: execution.engine, + verbose, + elapsedMilliseconds: execution.elapsedMilliseconds, + commands: resolution.commands, + deterministicFindingsPath: artifacts.deterministicFindingsPath, + agentChecksPath: artifacts.agentChecksPath, + agentCheckCount: execution.agentChecks.checks.length, + } +} + +function warnAboutIgnoredScanDirectories(ignoredScanDirectories: string[], format: SecurityCheckOutputFormat) { + for (const directory of ignoredScanDirectories) { + const headline = `${relativePath(cwd(), directory) || '.'} is ignored by Git, so only the files Git tracks in it are scanned.` + if (format === 'json') { + outputWarn(`${headline} Use --no-git-ignore to scan everything in it.`) + } else { + renderWarning({headline, body: ['Use', {command: '--no-git-ignore'}, 'to scan everything in it.']}) + } + } +} + function coverageIncomplete(input: SecurityReportInput): boolean { return input.scan.scan.coverage_gaps.length > 0 } diff --git a/packages/app/src/cli/services/security-review.test.ts b/packages/app/src/cli/services/security-review.test.ts index 2f69f760c07..d14872fb347 100644 --- a/packages/app/src/cli/services/security-review.test.ts +++ b/packages/app/src/cli/services/security-review.test.ts @@ -1,5 +1,7 @@ import securityReview, {reviewAppSecurityResults, type SecurityReviewDependencies} from './security-review.js' import {appSecurityArtifactPaths} from './app-security-artifacts.js' +import {resolveAppSecuritySelection, type AppSecuritySelectionOptions} from './app-security-selection.js' +import {validAppConfiguration} from './app-security-selection.test-data.js' import {loadAppSecurityResults} from './app-security-results.js' import {appSecurityResultsFor} from './app-security-results.test-data.js' import { @@ -34,11 +36,11 @@ async function createApp(directory: string, files: Files): Promise { function testDependencies() { const dependencies = { - resolveSelection: async ({directory}) => + resolveSelection: async ({path}) => ({ kind: 'config', - appDirectory: directory, - appConfigFilePath: joinPath(directory, 'shopify.app.toml'), + appDirectory: path, + appConfigFilePath: joinPath(path, 'shopify.app.toml'), }) as const, loadResults: loadAppSecurityResults, output: vi.fn(), @@ -425,3 +427,77 @@ describe('securityReview', () => { }) }) }) + +describe('securityReview --client-id lookup', () => { + const unknownClientId = new AbortError('No app with client ID unknown-client-id found') + const reviewOptions = {json: true, verbose: false, checkIds: [], blocking: 'none' as const} + + /** The real selection resolver, with the client ID lookup replaced, and a spy on the results loader. */ + function lookUpDependencies(lookUpApp: (clientId: string) => Promise) { + return { + ...testDependencies(), + resolveSelection: (options: AppSecuritySelectionOptions) => + resolveAppSecuritySelection(options, { + confirmScanWithoutAppConfig: async () => true, + pickClientId: async () => 'picked-client-id', + pickConfigFile: async () => 'shopify.app.toml', + lookUpApp, + }), + loadResults: vi.fn(loadAppSecurityResults), + } + } + + /** An app that the real resolver loads, with an empty results directory for `resultsKey`. */ + async function createLinkedApp(directory: string, resultsKey: string): Promise { + const appRoot = await fileRealPath(directory) + await writeFile(joinPath(appRoot, 'shopify.app.toml'), validAppConfiguration('toml-client-id')) + await mkdir(appSecurityArtifactPaths(appRoot, resultsKey).resultsDirectory) + return appRoot + } + + test('looks up --client-id and reads its results when it is found', async () => { + await inTemporaryDirectory(async (directory) => { + const appRoot = await createLinkedApp(directory, 'flag-client-id') + const lookUpApp = vi.fn(async (_clientId: string) => {}) + const dependencies = lookUpDependencies(lookUpApp) + + await securityReview({...reviewOptions, directory: appRoot, clientId: 'flag-client-id'}, dependencies) + + expect(lookUpApp).toHaveBeenCalledWith('flag-client-id') + expect(dependencies.loadResults).toHaveBeenCalledWith( + expect.objectContaining({clientIdOverride: 'flag-client-id'}), + appRoot, + ) + expect(dependencies.output).toHaveBeenCalled() + }) + }) + + test('aborts on an unknown --client-id before reading any results', async () => { + await inTemporaryDirectory(async (directory) => { + const appRoot = await createLinkedApp(directory, 'unknown-client-id') + const dependencies = lookUpDependencies(async () => { + throw unknownClientId + }) + + await expect( + securityReview({...reviewOptions, directory: appRoot, clientId: 'unknown-client-id'}, dependencies), + ).rejects.toBe(unknownClientId) + + expect(dependencies.loadResults).not.toHaveBeenCalled() + expect(dependencies.output).not.toHaveBeenCalled() + expect(dependencies.render).not.toHaveBeenCalled() + expect(dependencies.recordMetadata).not.toHaveBeenCalled() + }) + }) + + test('does not look up the TOML client ID', async () => { + await inTemporaryDirectory(async (directory) => { + const appRoot = await createLinkedApp(directory, RESULTS_KEY) + const lookUpApp = vi.fn(async (_clientId: string) => {}) + + await securityReview({...reviewOptions, directory: appRoot}, lookUpDependencies(lookUpApp)) + + expect(lookUpApp).not.toHaveBeenCalled() + }) + }) +}) diff --git a/packages/app/src/cli/services/security-review.ts b/packages/app/src/cli/services/security-review.ts index c519b1fac5a..2fe0f50b347 100644 --- a/packages/app/src/cli/services/security-review.ts +++ b/packages/app/src/cli/services/security-review.ts @@ -1,6 +1,11 @@ import {appSecurityArtifactPaths} from './app-security-artifacts.js' import {resolveAppSecurityCommands} from './app-security-commands.js' -import {resolveAppSecuritySelection, resultsKey, type AppSecuritySelection} from './app-security-selection.js' +import { + resolveAppSecuritySelection, + resultsKey, + type AppSecuritySelection, + type AppSecuritySelectionOptions, +} from './app-security-selection.js' import {loadAppSecurityResults, type AppSecurityResults} from './app-security-results.js' import {securityReviewJsonOutputSchema, toSecurityReviewJson} from './security-review-json.js' import {renderSecurityReview, type SecurityReviewPresenterInput} from './security-review-output.js' @@ -41,7 +46,7 @@ export interface SecurityReviewResult { } export interface SecurityReviewDependencies { - resolveSelection(options: SecurityReviewOptions): Promise + resolveSelection(options: AppSecuritySelectionOptions): Promise loadResults(selection: AppSecuritySelection, path: string): Promise output(content: string): void render(input: SecurityReviewPresenterInput): void @@ -51,14 +56,7 @@ export interface SecurityReviewDependencies { } const defaultDependencies: SecurityReviewDependencies = { - resolveSelection: (options) => - resolveAppSecuritySelection({ - path: options.directory, - config: options.configName, - clientId: options.clientId, - withoutAppConfig: options.withoutAppConfig, - allowPrompts: false, - }), + resolveSelection: resolveAppSecuritySelection, loadResults: loadAppSecurityResults, output: outputResult, render: renderSecurityReview, @@ -139,7 +137,14 @@ export default async function securityReview( options: SecurityReviewOptions, dependencies: SecurityReviewDependencies = defaultDependencies, ): Promise { - const selection = await dependencies.resolveSelection(options) + const selection = await dependencies.resolveSelection({ + path: options.directory, + config: options.configName, + clientId: options.clientId, + withoutAppConfig: options.withoutAppConfig, + allowPrompts: false, + validateClientIdFlag: true, + }) const results = await dependencies.loadResults(selection, options.directory) const result = reviewAppSecurityResults(results, { resultsDirectory: appSecurityArtifactPaths(selection.appDirectory, resultsKey(selection)).resultsDirectory, diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index 20822604f7b..558f078d084 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -3994,8 +3994,8 @@ "args": { }, "customPluginName": "@shopify/app", - "description": "Runs an app security check locally and writes `deterministic-findings.json` and `agent-checks.json` to the results directory, `.shopify/app-security//`. The results key is `--client-id` when you pass it, and otherwise the name of the app configuration file without `.toml`; the other `app security` commands take the same selection flags and find the same directory. Every run replaces both files, so it's always safe to run the check again.\n\n`deterministic-findings.json` holds the deterministic scan results. `agent-checks.json` holds the checks for your coding agent to investigate; the agent's results are recorded with `shopify app security record`. Use `--config` to select a specific app configuration when the project has multiple `shopify.app*.toml` files; the check inspects only that configuration. Use `--client-id` to replace the configuration's client ID for this run. When no app configuration exists, use `--without-app-config --client-id ` to scan `--path` anyway with config checks skipped; in an interactive terminal the command offers to do this.\n\nThe check scans the app directory and each `--include-dir`. Git ignore rules apply by default: a file or directory that Git ignores is skipped, using the rules of the repository that contains it, while files that Git tracks are always scanned. Use `--no-git-ignore` to turn Git ignore rules off for every scanned directory.\n\nUse `--exclude` to skip more paths. Each value is a glob that is matched against the path relative to the working directory, so a path above it starts with `../`, and a name at any depth needs `**/`, for example `--exclude '**/generated'`. Repeat the flag to add globs. An exclusion can't remove the selected app configuration file. Quote each value so your shell doesn't expand `*`. The coding-agent instructions this check offers repeat the globs. Other `app security` commands don't take `--exclude` or `--no-git-ignore`, so pass the same flags each time you run the check.\n\nUse `--list-files` to check the scope before scanning: it prints the files the check would gather, one path per line and relative to the app directory (`{\"files\": [...]}` with `--json`), and then stops. It writes no results and never prompts. `--client-id` is accepted but has no effect on the list.\n\nIn interactive terminals, the command offers to copy the coding-agent instructions, print them, or choose nothing; copying is the default. In CI and other non-interactive environments, instructions aren't offered unless you pass `--yes`, which prints them. JSON output never prompts or prints those instructions. You can also run `shopify app security instructions` to print, copy, or write them later.", - "descriptionWithMarkdown": "Runs an app security check locally and writes `deterministic-findings.json` and `agent-checks.json` to the results directory, `.shopify/app-security//`. The results key is `--client-id` when you pass it, and otherwise the name of the app configuration file without `.toml`; the other `app security` commands take the same selection flags and find the same directory. Every run replaces both files, so it's always safe to run the check again.\n\n`deterministic-findings.json` holds the deterministic scan results. `agent-checks.json` holds the checks for your coding agent to investigate; the agent's results are recorded with `shopify app security record`. Use `--config` to select a specific app configuration when the project has multiple `shopify.app*.toml` files; the check inspects only that configuration. Use `--client-id` to replace the configuration's client ID for this run. When no app configuration exists, use `--without-app-config --client-id ` to scan `--path` anyway with config checks skipped; in an interactive terminal the command offers to do this.\n\nThe check scans the app directory and each `--include-dir`. Git ignore rules apply by default: a file or directory that Git ignores is skipped, using the rules of the repository that contains it, while files that Git tracks are always scanned. Use `--no-git-ignore` to turn Git ignore rules off for every scanned directory.\n\nUse `--exclude` to skip more paths. Each value is a glob that is matched against the path relative to the working directory, so a path above it starts with `../`, and a name at any depth needs `**/`, for example `--exclude '**/generated'`. Repeat the flag to add globs. An exclusion can't remove the selected app configuration file. Quote each value so your shell doesn't expand `*`. The coding-agent instructions this check offers repeat the globs. Other `app security` commands don't take `--exclude` or `--no-git-ignore`, so pass the same flags each time you run the check.\n\nUse `--list-files` to check the scope before scanning: it prints the files the check would gather, one path per line and relative to the app directory (`{\"files\": [...]}` with `--json`), and then stops. It writes no results and never prompts. `--client-id` is accepted but has no effect on the list.\n\nIn interactive terminals, the command offers to copy the coding-agent instructions, print them, or choose nothing; copying is the default. In CI and other non-interactive environments, instructions aren't offered unless you pass `--yes`, which prints them. JSON output never prompts or prints those instructions. You can also run `shopify app security instructions` to print, copy, or write them later.", + "description": "Runs an app security check locally and writes `deterministic-findings.json` and `agent-checks.json` to the results directory, `.shopify/app-security//`. The results key is `--client-id` when you pass it, and otherwise the name of the app configuration file without `.toml`; the other `app security` commands take the same selection flags and find the same directory. Every run replaces both files, so it's always safe to run the check again.\n\n`deterministic-findings.json` holds the deterministic scan results. `agent-checks.json` holds the checks for your coding agent to investigate; the agent's results are recorded with `shopify app security record`. Use `--config` to select a specific app configuration when the project has multiple `shopify.app*.toml` files; the check inspects only that configuration. Use `--client-id` to replace the configuration's client ID for this run. `--client-id` is checked against your Shopify account before anything is scanned, so it needs you to be logged in. When no app configuration exists, use `--without-app-config --client-id ` to scan `--path` anyway with config checks skipped; in an interactive terminal the command offers to do this.\n\nThe check scans the app directory and each `--include-dir`. Git ignore rules apply by default: a file or directory that Git ignores is skipped, using the rules of the repository that contains it, while files that Git tracks are always scanned. Use `--no-git-ignore` to turn Git ignore rules off for every scanned directory.\n\nUse `--exclude` to skip more paths. Each value is a glob that is matched against the path relative to the working directory, so a path above it starts with `../`, and a name at any depth needs `**/`, for example `--exclude '**/generated'`. Repeat the flag to add globs. An exclusion can't remove the selected app configuration file. Quote each value so your shell doesn't expand `*`. The coding-agent instructions this check offers repeat the globs. Other `app security` commands don't take `--exclude` or `--no-git-ignore`, so pass the same flags each time you run the check.\n\nUse `--list-files` to check the scope before scanning: it prints the files the check would gather, one path per line and relative to the app directory, and then stops. It writes no results and shows no prompts. `--client-id` is still checked, which can require you to log in, but doesn't change the list.\n\nIn interactive terminals, the command offers to copy the coding-agent instructions, print them, or choose nothing; copying is the default. In CI and other non-interactive environments, instructions aren't offered unless you pass `--yes`, which prints them. You can also run `shopify app security instructions` to print, copy, or write them later.\n\nThe command can also ask which app configuration to scan, or offer to scan without one and then ask you to pick or create the app. In automation, pass `--no-input` to turn every prompt off. A choice the command would have asked for then becomes an error, so pass `--config`, or `--without-app-config --client-id `, instead; a `--client-id` that needs a login fails instead of opening the browser.\n\nUse `--json-schema` to print the result, error, and event schemas.\n\nOutput from `--json` conforms to the `AppSecurityCheckResult` schema.\n\n```json\n{\n \"anyOf\": [\n {\n \"$ref\": \"#/definitions/AppSecurityCheckScanResult\"\n },\n {\n \"$ref\": \"#/definitions/AppSecurityCheckFileListResult\"\n },\n {\n \"$ref\": \"#/definitions/AppSecurityCheckCancelledResult\"\n }\n ],\n \"title\": \"AppSecurityCheckResult\",\n \"definitions\": {\n \"AppSecurityCheckScanResult\": {\n \"type\": \"object\",\n \"properties\": {\n \"status\": {\n \"type\": \"string\",\n \"const\": \"success\"\n },\n \"selection\": {\n \"type\": \"object\",\n \"properties\": {\n \"directory\": {\n \"type\": \"string\"\n },\n \"configPath\": {\n \"type\": [\n \"string\",\n \"null\"\n ]\n },\n \"clientId\": {\n \"type\": [\n \"string\",\n \"null\"\n ]\n },\n \"clientIdSource\": {\n \"anyOf\": [\n {\n \"type\": \"string\",\n \"enum\": [\n \"config\",\n \"flag\",\n \"picker\"\n ]\n },\n {\n \"type\": \"null\"\n }\n ]\n },\n \"scanDirectories\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"directory\": {\n \"type\": \"string\"\n },\n \"origin\": {\n \"type\": \"string\",\n \"enum\": [\n \"app-directory\",\n \"include-dir\"\n ]\n }\n },\n \"required\": [\n \"directory\",\n \"origin\"\n ],\n \"additionalProperties\": false\n }\n }\n },\n \"required\": [\n \"directory\",\n \"configPath\",\n \"clientId\",\n \"clientIdSource\",\n \"scanDirectories\"\n ],\n \"additionalProperties\": false\n },\n \"deterministicFindings\": {\n \"type\": \"object\",\n \"properties\": {\n \"schema_version\": {\n \"type\": \"number\",\n \"const\": 1\n },\n \"generated_at\": {\n \"type\": \"string\"\n },\n \"checks\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"id\": {\n \"type\": \"string\"\n },\n \"version\": {\n \"type\": \"number\"\n },\n \"status\": {\n \"type\": \"string\",\n \"enum\": [\n \"executed\",\n \"not_applicable\",\n \"unresolved\"\n ]\n },\n \"reason\": {\n \"type\": \"object\",\n \"properties\": {\n \"code\": {\n \"type\": \"string\"\n },\n \"message\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"code\",\n \"message\"\n ],\n \"additionalProperties\": false\n },\n \"analysis_mode\": {\n \"type\": \"string\",\n \"enum\": [\n \"regex\",\n \"structured_config\",\n \"ast\"\n ]\n },\n \"snapshot\": {\n \"type\": \"object\",\n \"properties\": {\n \"title\": {\n \"type\": \"string\"\n },\n \"severity\": {\n \"type\": \"string\",\n \"enum\": [\n \"high\",\n \"medium\",\n \"low\"\n ]\n },\n \"description\": {\n \"type\": \"string\"\n },\n \"guide\": {\n \"type\": \"string\"\n },\n \"docs_url\": {\n \"type\": \"string\"\n },\n \"current_version\": {\n \"type\": \"number\"\n },\n \"precedence\": {\n \"type\": \"string\",\n \"enum\": [\n \"union\",\n \"prefer-agent\"\n ]\n }\n },\n \"required\": [\n \"title\",\n \"severity\",\n \"description\",\n \"current_version\"\n ],\n \"additionalProperties\": false\n },\n \"findings\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"location\": {\n \"type\": \"object\",\n \"properties\": {\n \"file\": {\n \"type\": \"string\"\n },\n \"line\": {\n \"type\": \"number\"\n },\n \"column\": {\n \"type\": \"number\"\n }\n },\n \"required\": [\n \"file\"\n ],\n \"additionalProperties\": false\n },\n \"message\": {\n \"type\": \"string\"\n },\n \"evidence\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"location\": {\n \"$ref\": \"#/definitions/AppSecurityCheckScanResult/properties/deterministicFindings/properties/checks/items/properties/findings/items/properties/location\"\n },\n \"quote\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"location\"\n ],\n \"additionalProperties\": false\n }\n },\n \"snippet\": {\n \"type\": \"string\"\n },\n \"fix\": {\n \"type\": \"object\",\n \"properties\": {\n \"automated\": {\n \"type\": \"boolean\"\n },\n \"guide\": {\n \"type\": \"string\"\n },\n \"description\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"automated\",\n \"description\"\n ],\n \"additionalProperties\": false\n },\n \"confidence\": {\n \"type\": \"string\",\n \"enum\": [\n \"high\",\n \"medium\",\n \"low\"\n ]\n },\n \"reasoning\": {\n \"type\": \"string\"\n },\n \"suppression\": {\n \"type\": \"object\",\n \"properties\": {\n \"justification\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"justification\"\n ],\n \"additionalProperties\": false\n }\n },\n \"required\": [\n \"location\",\n \"message\",\n \"evidence\"\n ],\n \"additionalProperties\": false\n }\n }\n },\n \"required\": [\n \"id\",\n \"version\",\n \"status\",\n \"snapshot\",\n \"findings\"\n ],\n \"additionalProperties\": false\n }\n },\n \"source\": {\n \"type\": \"string\",\n \"const\": \"deterministic\"\n },\n \"engine\": {\n \"type\": \"object\",\n \"properties\": {\n \"name\": {\n \"type\": \"string\",\n \"const\": \"shopify-app-security\"\n },\n \"version\": {\n \"type\": \"string\"\n },\n \"ruleset\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"name\",\n \"version\",\n \"ruleset\"\n ],\n \"additionalProperties\": false\n },\n \"detection\": {\n \"type\": \"object\",\n \"properties\": {\n \"framework\": {\n \"type\": \"string\",\n \"enum\": [\n \"react_router\",\n \"none\",\n \"unknown\",\n \"mixed\"\n ]\n },\n \"surface\": {\n \"type\": \"string\",\n \"enum\": [\n \"react_router\",\n \"theme_app_extension\",\n \"config_only\",\n \"unknown\",\n \"mixed\"\n ]\n },\n \"languages\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"name\": {\n \"type\": \"string\"\n },\n \"support\": {\n \"type\": \"string\",\n \"enum\": [\n \"supported\",\n \"unsupported\"\n ]\n },\n \"files\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n }\n },\n \"required\": [\n \"name\",\n \"support\",\n \"files\"\n ],\n \"additionalProperties\": false\n }\n }\n },\n \"required\": [\n \"framework\",\n \"surface\",\n \"languages\"\n ],\n \"additionalProperties\": false\n },\n \"coverage\": {\n \"type\": \"object\",\n \"properties\": {\n \"files_scanned\": {\n \"type\": \"number\"\n },\n \"files_skipped\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"path\": {\n \"type\": \"string\"\n },\n \"reason\": {\n \"type\": \"string\",\n \"enum\": [\n \"too_large\",\n \"unreadable\"\n ]\n },\n \"size_bytes\": {\n \"type\": \"number\"\n },\n \"detail\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"path\",\n \"reason\"\n ],\n \"additionalProperties\": false\n }\n },\n \"gaps\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"code\": {\n \"type\": \"string\",\n \"enum\": [\n \"skipped_file\",\n \"unsupported_framework\",\n \"unsupported_language\",\n \"unresolved_check\"\n ]\n },\n \"message\": {\n \"type\": \"string\"\n },\n \"check_id\": {\n \"type\": \"string\"\n },\n \"file\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"code\",\n \"message\"\n ],\n \"additionalProperties\": false\n }\n },\n \"scope\": {\n \"type\": \"object\",\n \"properties\": {\n \"include_dirs\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n },\n \"excludes\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n },\n \"no_git_ignore\": {\n \"type\": \"boolean\"\n }\n },\n \"required\": [\n \"include_dirs\",\n \"excludes\",\n \"no_git_ignore\"\n ],\n \"additionalProperties\": false\n },\n \"scan_directories\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"directory\": {\n \"type\": \"string\"\n },\n \"origin\": {\n \"type\": \"string\",\n \"enum\": [\n \"app_directory\",\n \"include_dir\"\n ]\n }\n },\n \"required\": [\n \"directory\",\n \"origin\"\n ],\n \"additionalProperties\": false\n }\n }\n },\n \"required\": [\n \"files_scanned\",\n \"files_skipped\",\n \"gaps\",\n \"scope\",\n \"scan_directories\"\n ],\n \"additionalProperties\": false\n }\n },\n \"required\": [\n \"schema_version\",\n \"generated_at\",\n \"checks\",\n \"source\",\n \"engine\",\n \"detection\",\n \"coverage\"\n ],\n \"additionalProperties\": false,\n \"description\": \"The deterministic findings document, as written to deterministic-findings.json. It keeps its own field conventions and is versioned by its schema_version, independently of this result.\"\n },\n \"agentChecksPath\": {\n \"type\": \"string\"\n },\n \"instructions\": {\n \"anyOf\": [\n {\n \"$ref\": \"#/definitions/AppSecurityInstructions\"\n },\n {\n \"type\": \"null\"\n }\n ]\n }\n },\n \"required\": [\n \"status\",\n \"selection\",\n \"deterministicFindings\",\n \"agentChecksPath\",\n \"instructions\"\n ],\n \"additionalProperties\": false\n },\n \"AppSecurityCheckFileListResult\": {\n \"type\": \"object\",\n \"properties\": {\n \"status\": {\n \"type\": \"string\",\n \"const\": \"success\"\n },\n \"files\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n },\n \"description\": \"The absolute path of each file the check would gather.\"\n }\n },\n \"required\": [\n \"status\",\n \"files\"\n ],\n \"additionalProperties\": false\n },\n \"AppSecurityCheckCancelledResult\": {\n \"type\": \"object\",\n \"properties\": {\n \"status\": {\n \"type\": \"string\",\n \"const\": \"cancelled\"\n }\n },\n \"required\": [\n \"status\"\n ],\n \"additionalProperties\": false\n },\n \"AppSecurityInstructions\": {\n \"type\": \"object\",\n \"properties\": {\n \"content\": {\n \"type\": \"string\"\n },\n \"copiedToClipboard\": {\n \"type\": \"boolean\"\n },\n \"path\": {\n \"type\": [\n \"string\",\n \"null\"\n ]\n }\n },\n \"required\": [\n \"content\",\n \"copiedToClipboard\",\n \"path\"\n ],\n \"additionalProperties\": false\n }\n },\n \"$schema\": \"http://json-schema.org/draft-07/schema#\"\n}\n```", + "descriptionWithMarkdown": "Runs an app security check locally and writes `deterministic-findings.json` and `agent-checks.json` to the results directory, `.shopify/app-security//`. The results key is `--client-id` when you pass it, and otherwise the name of the app configuration file without `.toml`; the other `app security` commands take the same selection flags and find the same directory. Every run replaces both files, so it's always safe to run the check again.\n\n`deterministic-findings.json` holds the deterministic scan results. `agent-checks.json` holds the checks for your coding agent to investigate; the agent's results are recorded with `shopify app security record`. Use `--config` to select a specific app configuration when the project has multiple `shopify.app*.toml` files; the check inspects only that configuration. Use `--client-id` to replace the configuration's client ID for this run. `--client-id` is checked against your Shopify account before anything is scanned, so it needs you to be logged in. When no app configuration exists, use `--without-app-config --client-id ` to scan `--path` anyway with config checks skipped; in an interactive terminal the command offers to do this.\n\nThe check scans the app directory and each `--include-dir`. Git ignore rules apply by default: a file or directory that Git ignores is skipped, using the rules of the repository that contains it, while files that Git tracks are always scanned. Use `--no-git-ignore` to turn Git ignore rules off for every scanned directory.\n\nUse `--exclude` to skip more paths. Each value is a glob that is matched against the path relative to the working directory, so a path above it starts with `../`, and a name at any depth needs `**/`, for example `--exclude '**/generated'`. Repeat the flag to add globs. An exclusion can't remove the selected app configuration file. Quote each value so your shell doesn't expand `*`. The coding-agent instructions this check offers repeat the globs. Other `app security` commands don't take `--exclude` or `--no-git-ignore`, so pass the same flags each time you run the check.\n\nUse `--list-files` to check the scope before scanning: it prints the files the check would gather, one path per line and relative to the app directory, and then stops. It writes no results and shows no prompts. `--client-id` is still checked, which can require you to log in, but doesn't change the list.\n\nIn interactive terminals, the command offers to copy the coding-agent instructions, print them, or choose nothing; copying is the default. In CI and other non-interactive environments, instructions aren't offered unless you pass `--yes`, which prints them. You can also run `shopify app security instructions` to print, copy, or write them later.\n\nThe command can also ask which app configuration to scan, or offer to scan without one and then ask you to pick or create the app. In automation, pass `--no-input` to turn every prompt off. A choice the command would have asked for then becomes an error, so pass `--config`, or `--without-app-config --client-id `, instead; a `--client-id` that needs a login fails instead of opening the browser.", "enableJsonFlag": false, "flags": { "blocking": { @@ -4289,7 +4289,7 @@ "args": { }, "customPluginName": "@shopify/app", - "description": "Prints the complete workflow that a coding agent should follow to review app security check results.\n\nBy default, the instructions are printed to stdout. Use `--copy` to copy them to the clipboard or `--write` to write them to a file. Standalone instructions always start by running `shopify app security check`; only that invocation's generated review pack is trusted as workflow input.", + "description": "Prints the complete workflow that a coding agent should follow to review app security check results.\n\nBy default, the instructions are printed to stdout. Use `--copy` to copy them to the clipboard or `--write` to write them to a file. Standalone instructions always start by running `shopify app security check`; only that invocation's generated review pack is trusted as workflow input.\n\nUse `--json-schema` to print the result, error, and event schemas.\n\nOutput from `--json` conforms to the `AppSecurityInstructionsResult` schema.\n\n```json\n{\n \"type\": \"object\",\n \"properties\": {\n \"instructions\": {\n \"$ref\": \"#/definitions/AppSecurityInstructions\"\n }\n },\n \"required\": [\n \"instructions\"\n ],\n \"additionalProperties\": false,\n \"title\": \"AppSecurityInstructionsResult\",\n \"definitions\": {\n \"AppSecurityInstructions\": {\n \"type\": \"object\",\n \"properties\": {\n \"content\": {\n \"type\": \"string\"\n },\n \"copiedToClipboard\": {\n \"type\": \"boolean\"\n },\n \"path\": {\n \"type\": [\n \"string\",\n \"null\"\n ]\n }\n },\n \"required\": [\n \"content\",\n \"copiedToClipboard\",\n \"path\"\n ],\n \"additionalProperties\": false\n }\n },\n \"$schema\": \"http://json-schema.org/draft-07/schema#\"\n}\n```", "descriptionWithMarkdown": "Prints the complete workflow that a coding agent should follow to review app security check results.\n\nBy default, the instructions are printed to stdout. Use `--copy` to copy them to the clipboard or `--write` to write them to a file. Standalone instructions always start by running `shopify app security check`; only that invocation's generated review pack is trusted as workflow input.", "enableJsonFlag": false, "flags": { @@ -4325,6 +4325,15 @@ "name": "copy", "type": "boolean" }, + "json": { + "allowNo": false, + "char": "j", + "description": "Output the result as JSON. Automatically disables color output.", + "env": "SHOPIFY_FLAG_JSON", + "hidden": false, + "name": "json", + "type": "boolean" + }, "json-schema": { "allowNo": false, "description": "Print the command's JSON schemas.", @@ -4406,8 +4415,8 @@ "args": { }, "customPluginName": "@shopify/app", - "description": "Reads a coding agent's complete findings document from stdin, validates it, and replaces `agent-findings.json` in the results directory, `.shopify/app-security//`. The results key is `--client-id` when you pass it, and otherwise the name of the app configuration file without `.toml`.\n\nThe document must include a `scope` with the `include_dirs`, `excludes` and `no_git_ignore` values of the `check` run it describes, exactly as typed. It's recorded as reported and never compared with the scan's files.\n\nThe document is recorded all or nothing: if anything is invalid, the command fails with every error, writes nothing, and exits with a non-zero code. With `--json`, the errors are listed in the error document's `details.errors`. It needs the results directory that `shopify app security check` creates.\n\nUse `--json-schema` to print the result, error, and event schemas.\n\nOutput from `--json` conforms to the `AppSecurityRecordResult` schema.\n\n```json\n{\n \"type\": \"object\",\n \"properties\": {\n \"path\": {\n \"type\": \"string\"\n },\n \"checks\": {\n \"type\": \"integer\"\n },\n \"findings\": {\n \"type\": \"integer\"\n }\n },\n \"required\": [\n \"path\",\n \"checks\",\n \"findings\"\n ],\n \"additionalProperties\": false,\n \"title\": \"AppSecurityRecordResult\",\n \"$schema\": \"http://json-schema.org/draft-07/schema#\"\n}\n```", - "descriptionWithMarkdown": "Reads a coding agent's complete findings document from stdin, validates it, and replaces `agent-findings.json` in the results directory, `.shopify/app-security//`. The results key is `--client-id` when you pass it, and otherwise the name of the app configuration file without `.toml`.\n\nThe document must include a `scope` with the `include_dirs`, `excludes` and `no_git_ignore` values of the `check` run it describes, exactly as typed. It's recorded as reported and never compared with the scan's files.\n\nThe document is recorded all or nothing: if anything is invalid, the command fails with every error, writes nothing, and exits with a non-zero code. With `--json`, the errors are listed in the error document's `details.errors`. It needs the results directory that `shopify app security check` creates.", + "description": "Reads a coding agent's complete findings document from stdin, validates it, and replaces `agent-findings.json` in the results directory, `.shopify/app-security//`. The results key is `--client-id` when you pass it, and otherwise the name of the app configuration file without `.toml`. `--client-id` is checked against your Shopify account before anything is read, so it needs you to be logged in.\n\nThe document must include a `scope` with the `include_dirs`, `excludes` and `no_git_ignore` values of the `check` run it describes, exactly as typed. It's recorded as reported and never compared with the scan's files.\n\nThe document is recorded all or nothing: if anything is invalid, the command fails with every error, writes nothing, and exits with a non-zero code. With `--json`, the errors are listed in the error document's `details.errors`. It needs the results directory that `shopify app security check` creates.\n\nUse `--json-schema` to print the result, error, and event schemas.\n\nOutput from `--json` conforms to the `AppSecurityRecordResult` schema.\n\n```json\n{\n \"type\": \"object\",\n \"properties\": {\n \"path\": {\n \"type\": \"string\"\n },\n \"checks\": {\n \"type\": \"integer\"\n },\n \"findings\": {\n \"type\": \"integer\"\n }\n },\n \"required\": [\n \"path\",\n \"checks\",\n \"findings\"\n ],\n \"additionalProperties\": false,\n \"title\": \"AppSecurityRecordResult\",\n \"$schema\": \"http://json-schema.org/draft-07/schema#\"\n}\n```", + "descriptionWithMarkdown": "Reads a coding agent's complete findings document from stdin, validates it, and replaces `agent-findings.json` in the results directory, `.shopify/app-security//`. The results key is `--client-id` when you pass it, and otherwise the name of the app configuration file without `.toml`. `--client-id` is checked against your Shopify account before anything is read, so it needs you to be logged in.\n\nThe document must include a `scope` with the `include_dirs`, `excludes` and `no_git_ignore` values of the `check` run it describes, exactly as typed. It's recorded as reported and never compared with the scan's files.\n\nThe document is recorded all or nothing: if anything is invalid, the command fails with every error, writes nothing, and exits with a non-zero code. With `--json`, the errors are listed in the error document's `details.errors`. It needs the results directory that `shopify app security check` creates.", "enableJsonFlag": false, "flags": { "client-id": { @@ -4511,8 +4520,8 @@ "args": { }, "customPluginName": "@shopify/app", - "description": "Combines the deterministic results (`deterministic-findings.json`, written by `shopify app security check`) with the recorded agent results (`agent-findings.json`, written by `shopify app security record`) and shows one view of every check: its findings, status and source. Both files are in the results directory, `.shopify/app-security//`.\n\nThe summary shows the scan directories and the scope of the latest scan, and the scope the agent reported. It notes when the agent findings were recorded for a different scope than the latest scan; that doesn't change the exit code.\n\nThe agent results are optional. Use `--check-id` to narrow the review to specific checks, `--verbose` for full reasoning, evidence and suppressed findings, and `--blocking` to exit with code 1 when a check with findings is at or above a severity.\n\nUse `--json-schema` to print the result, error, and event schemas.\n\nOutput from `--json` conforms to the `AppSecurityReviewResult` schema.\n\n```json\n{\n \"type\": \"object\",\n \"properties\": {\n \"filter\": {\n \"anyOf\": [\n {\n \"type\": \"object\",\n \"properties\": {\n \"check_ids\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n }\n },\n \"required\": [\n \"check_ids\"\n ],\n \"additionalProperties\": false\n },\n {\n \"type\": \"null\"\n }\n ]\n },\n \"sources\": {\n \"type\": \"object\",\n \"properties\": {\n \"deterministic\": {\n \"anyOf\": [\n {\n \"$ref\": \"#/definitions/AppSecurityDeterministicSource\"\n },\n {\n \"type\": \"null\"\n }\n ]\n },\n \"agent\": {\n \"anyOf\": [\n {\n \"$ref\": \"#/definitions/AppSecurityAgentSource\"\n },\n {\n \"type\": \"null\"\n }\n ]\n }\n },\n \"required\": [\n \"deterministic\",\n \"agent\"\n ],\n \"additionalProperties\": false\n },\n \"scope_differs\": {\n \"type\": \"boolean\"\n },\n \"checks\": {\n \"type\": \"array\",\n \"items\": {\n \"$ref\": \"#/definitions/AppSecurityCombinedCheck\"\n }\n }\n },\n \"required\": [\n \"filter\",\n \"sources\",\n \"scope_differs\",\n \"checks\"\n ],\n \"additionalProperties\": false,\n \"title\": \"AppSecurityReviewResult\",\n \"definitions\": {\n \"AppSecurityDeterministicSource\": {\n \"type\": \"object\",\n \"properties\": {\n \"path\": {\n \"type\": \"string\"\n },\n \"schema_version\": {\n \"type\": \"number\",\n \"const\": 1\n },\n \"engine\": {\n \"type\": \"object\",\n \"properties\": {\n \"name\": {\n \"type\": \"string\",\n \"const\": \"shopify-app-security\"\n },\n \"version\": {\n \"type\": \"string\"\n },\n \"ruleset\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"name\",\n \"version\",\n \"ruleset\"\n ],\n \"additionalProperties\": false\n },\n \"generated_at\": {\n \"type\": \"string\"\n },\n \"detection\": {\n \"type\": \"object\",\n \"properties\": {\n \"framework\": {\n \"type\": \"string\",\n \"enum\": [\n \"react_router\",\n \"none\",\n \"unknown\",\n \"mixed\"\n ]\n },\n \"surface\": {\n \"type\": \"string\",\n \"enum\": [\n \"react_router\",\n \"theme_app_extension\",\n \"config_only\",\n \"unknown\",\n \"mixed\"\n ]\n },\n \"languages\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"name\": {\n \"type\": \"string\"\n },\n \"support\": {\n \"type\": \"string\",\n \"enum\": [\n \"supported\",\n \"unsupported\"\n ]\n },\n \"files\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n }\n },\n \"required\": [\n \"name\",\n \"support\",\n \"files\"\n ],\n \"additionalProperties\": false\n }\n }\n },\n \"required\": [\n \"framework\",\n \"surface\",\n \"languages\"\n ],\n \"additionalProperties\": false\n },\n \"coverage\": {\n \"type\": \"object\",\n \"properties\": {\n \"files_scanned\": {\n \"type\": \"number\"\n },\n \"files_skipped\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"path\": {\n \"type\": \"string\"\n },\n \"reason\": {\n \"type\": \"string\",\n \"enum\": [\n \"too_large\",\n \"unreadable\"\n ]\n },\n \"size_bytes\": {\n \"type\": \"number\"\n },\n \"detail\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"path\",\n \"reason\"\n ],\n \"additionalProperties\": false\n }\n },\n \"gaps\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"code\": {\n \"type\": \"string\",\n \"enum\": [\n \"skipped_file\",\n \"unsupported_framework\",\n \"unsupported_language\",\n \"unresolved_check\"\n ]\n },\n \"message\": {\n \"type\": \"string\"\n },\n \"check_id\": {\n \"type\": \"string\"\n },\n \"file\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"code\",\n \"message\"\n ],\n \"additionalProperties\": false\n }\n },\n \"scope\": {\n \"type\": \"object\",\n \"properties\": {\n \"include_dirs\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n },\n \"excludes\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n },\n \"no_git_ignore\": {\n \"type\": \"boolean\"\n }\n },\n \"required\": [\n \"include_dirs\",\n \"excludes\",\n \"no_git_ignore\"\n ],\n \"additionalProperties\": false\n },\n \"scan_directories\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"directory\": {\n \"type\": \"string\"\n },\n \"origin\": {\n \"type\": \"string\",\n \"enum\": [\n \"app_directory\",\n \"include_dir\"\n ]\n }\n },\n \"required\": [\n \"directory\",\n \"origin\"\n ],\n \"additionalProperties\": false\n }\n }\n },\n \"required\": [\n \"files_scanned\",\n \"files_skipped\",\n \"gaps\",\n \"scope\",\n \"scan_directories\"\n ],\n \"additionalProperties\": false\n }\n },\n \"required\": [\n \"path\",\n \"schema_version\",\n \"engine\",\n \"generated_at\",\n \"detection\",\n \"coverage\"\n ],\n \"additionalProperties\": false\n },\n \"AppSecurityAgentSource\": {\n \"type\": \"object\",\n \"properties\": {\n \"path\": {\n \"type\": \"string\"\n },\n \"schema_version\": {\n \"type\": \"number\",\n \"const\": 1\n },\n \"engine\": {\n \"type\": \"object\",\n \"properties\": {\n \"name\": {\n \"type\": \"string\",\n \"const\": \"shopify-app-security\"\n },\n \"version\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"name\",\n \"version\"\n ],\n \"additionalProperties\": false\n },\n \"generated_at\": {\n \"type\": \"string\"\n },\n \"scope\": {\n \"type\": \"object\",\n \"properties\": {\n \"include_dirs\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n },\n \"excludes\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n },\n \"no_git_ignore\": {\n \"type\": \"boolean\"\n }\n },\n \"required\": [\n \"include_dirs\",\n \"excludes\",\n \"no_git_ignore\"\n ],\n \"additionalProperties\": false\n }\n },\n \"required\": [\n \"path\",\n \"schema_version\",\n \"engine\",\n \"generated_at\",\n \"scope\"\n ],\n \"additionalProperties\": false\n },\n \"AppSecurityCombinedCheck\": {\n \"type\": \"object\",\n \"properties\": {\n \"id\": {\n \"type\": \"string\"\n },\n \"title\": {\n \"type\": \"string\"\n },\n \"severity\": {\n \"type\": \"string\",\n \"enum\": [\n \"high\",\n \"medium\",\n \"low\"\n ]\n },\n \"description\": {\n \"type\": \"string\"\n },\n \"guide\": {\n \"type\": \"string\"\n },\n \"docs_url\": {\n \"type\": \"string\"\n },\n \"precedence\": {\n \"type\": \"string\",\n \"enum\": [\n \"union\",\n \"prefer-agent\"\n ]\n },\n \"applied_precedence\": {\n \"$ref\": \"#/definitions/AppSecurityCombinedCheck/properties/precedence\"\n },\n \"status\": {\n \"type\": \"string\",\n \"enum\": [\n \"executed\",\n \"not_applicable\",\n \"unresolved\"\n ]\n },\n \"by_source\": {\n \"type\": \"object\",\n \"properties\": {\n \"deterministic\": {\n \"anyOf\": [\n {\n \"type\": \"object\",\n \"properties\": {\n \"version\": {\n \"type\": \"number\"\n },\n \"status\": {\n \"$ref\": \"#/definitions/AppSecurityCombinedCheck/properties/status\"\n },\n \"reason\": {\n \"type\": \"object\",\n \"properties\": {\n \"code\": {\n \"type\": \"string\"\n },\n \"message\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"code\",\n \"message\"\n ],\n \"additionalProperties\": false\n },\n \"analysis_mode\": {\n \"type\": \"string\",\n \"enum\": [\n \"regex\",\n \"structured_config\",\n \"ast\"\n ]\n },\n \"generated_at\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"version\",\n \"status\",\n \"generated_at\"\n ],\n \"additionalProperties\": false\n },\n {\n \"type\": \"null\"\n }\n ]\n },\n \"agent\": {\n \"anyOf\": [\n {\n \"$ref\": \"#/definitions/AppSecurityCombinedCheck/properties/by_source/properties/deterministic/anyOf/0\"\n },\n {\n \"type\": \"null\"\n }\n ]\n }\n },\n \"required\": [\n \"deterministic\",\n \"agent\"\n ],\n \"additionalProperties\": false\n },\n \"findings\": {\n \"type\": \"array\",\n \"items\": {\n \"$ref\": \"#/definitions/AppSecurityCombinedFinding\"\n }\n }\n },\n \"required\": [\n \"id\",\n \"title\",\n \"severity\",\n \"description\",\n \"precedence\",\n \"applied_precedence\",\n \"status\",\n \"by_source\",\n \"findings\"\n ],\n \"additionalProperties\": false\n },\n \"AppSecurityCombinedFinding\": {\n \"type\": \"object\",\n \"properties\": {\n \"source\": {\n \"type\": \"string\",\n \"enum\": [\n \"deterministic\",\n \"agent\"\n ]\n },\n \"disposition\": {\n \"type\": \"string\",\n \"enum\": [\n \"active\",\n \"suppressed\",\n \"superseded\"\n ]\n },\n \"location\": {\n \"type\": \"object\",\n \"properties\": {\n \"file\": {\n \"type\": \"string\"\n },\n \"line\": {\n \"type\": \"number\"\n },\n \"column\": {\n \"type\": \"number\"\n }\n },\n \"required\": [\n \"file\"\n ],\n \"additionalProperties\": false\n },\n \"message\": {\n \"type\": \"string\"\n },\n \"evidence\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"location\": {\n \"$ref\": \"#/definitions/AppSecurityCombinedFinding/properties/location\"\n },\n \"quote\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"location\"\n ],\n \"additionalProperties\": false\n }\n },\n \"snippet\": {\n \"type\": \"string\"\n },\n \"fix\": {\n \"type\": \"object\",\n \"properties\": {\n \"automated\": {\n \"type\": \"boolean\"\n },\n \"guide\": {\n \"type\": \"string\"\n },\n \"description\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"automated\",\n \"description\"\n ],\n \"additionalProperties\": false\n },\n \"confidence\": {\n \"type\": \"string\",\n \"enum\": [\n \"high\",\n \"medium\",\n \"low\"\n ]\n },\n \"reasoning\": {\n \"type\": \"string\"\n },\n \"suppression\": {\n \"type\": \"object\",\n \"properties\": {\n \"justification\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"justification\"\n ],\n \"additionalProperties\": false\n }\n },\n \"required\": [\n \"source\",\n \"disposition\",\n \"location\",\n \"message\",\n \"evidence\"\n ],\n \"additionalProperties\": false\n }\n },\n \"$schema\": \"http://json-schema.org/draft-07/schema#\"\n}\n```", - "descriptionWithMarkdown": "Combines the deterministic results (`deterministic-findings.json`, written by `shopify app security check`) with the recorded agent results (`agent-findings.json`, written by `shopify app security record`) and shows one view of every check: its findings, status and source. Both files are in the results directory, `.shopify/app-security//`.\n\nThe summary shows the scan directories and the scope of the latest scan, and the scope the agent reported. It notes when the agent findings were recorded for a different scope than the latest scan; that doesn't change the exit code.\n\nThe agent results are optional. Use `--check-id` to narrow the review to specific checks, `--verbose` for full reasoning, evidence and suppressed findings, and `--blocking` to exit with code 1 when a check with findings is at or above a severity.", + "description": "Combines the deterministic results (`deterministic-findings.json`, written by `shopify app security check`) with the recorded agent results (`agent-findings.json`, written by `shopify app security record`) and shows one view of every check: its findings, status and source. Both files are in the results directory, `.shopify/app-security//`. `--client-id` is checked against your Shopify account before any results are read, so it needs you to be logged in.\n\nThe summary shows the scan directories and the scope of the latest scan, and the scope the agent reported. It notes when the agent findings were recorded for a different scope than the latest scan; that doesn't change the exit code.\n\nThe agent results are optional. Use `--check-id` to narrow the review to specific checks, `--verbose` for full reasoning, evidence and suppressed findings, and `--blocking` to exit with code 1 when a check with findings is at or above a severity.\n\nUse `--json-schema` to print the result, error, and event schemas.\n\nOutput from `--json` conforms to the `AppSecurityReviewResult` schema.\n\n```json\n{\n \"type\": \"object\",\n \"properties\": {\n \"filter\": {\n \"anyOf\": [\n {\n \"type\": \"object\",\n \"properties\": {\n \"check_ids\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n }\n },\n \"required\": [\n \"check_ids\"\n ],\n \"additionalProperties\": false\n },\n {\n \"type\": \"null\"\n }\n ]\n },\n \"sources\": {\n \"type\": \"object\",\n \"properties\": {\n \"deterministic\": {\n \"anyOf\": [\n {\n \"$ref\": \"#/definitions/AppSecurityDeterministicSource\"\n },\n {\n \"type\": \"null\"\n }\n ]\n },\n \"agent\": {\n \"anyOf\": [\n {\n \"$ref\": \"#/definitions/AppSecurityAgentSource\"\n },\n {\n \"type\": \"null\"\n }\n ]\n }\n },\n \"required\": [\n \"deterministic\",\n \"agent\"\n ],\n \"additionalProperties\": false\n },\n \"scope_differs\": {\n \"type\": \"boolean\"\n },\n \"checks\": {\n \"type\": \"array\",\n \"items\": {\n \"$ref\": \"#/definitions/AppSecurityCombinedCheck\"\n }\n }\n },\n \"required\": [\n \"filter\",\n \"sources\",\n \"scope_differs\",\n \"checks\"\n ],\n \"additionalProperties\": false,\n \"title\": \"AppSecurityReviewResult\",\n \"definitions\": {\n \"AppSecurityDeterministicSource\": {\n \"type\": \"object\",\n \"properties\": {\n \"path\": {\n \"type\": \"string\"\n },\n \"schema_version\": {\n \"type\": \"number\",\n \"const\": 1\n },\n \"engine\": {\n \"type\": \"object\",\n \"properties\": {\n \"name\": {\n \"type\": \"string\",\n \"const\": \"shopify-app-security\"\n },\n \"version\": {\n \"type\": \"string\"\n },\n \"ruleset\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"name\",\n \"version\",\n \"ruleset\"\n ],\n \"additionalProperties\": false\n },\n \"generated_at\": {\n \"type\": \"string\"\n },\n \"detection\": {\n \"type\": \"object\",\n \"properties\": {\n \"framework\": {\n \"type\": \"string\",\n \"enum\": [\n \"react_router\",\n \"none\",\n \"unknown\",\n \"mixed\"\n ]\n },\n \"surface\": {\n \"type\": \"string\",\n \"enum\": [\n \"react_router\",\n \"theme_app_extension\",\n \"config_only\",\n \"unknown\",\n \"mixed\"\n ]\n },\n \"languages\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"name\": {\n \"type\": \"string\"\n },\n \"support\": {\n \"type\": \"string\",\n \"enum\": [\n \"supported\",\n \"unsupported\"\n ]\n },\n \"files\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n }\n },\n \"required\": [\n \"name\",\n \"support\",\n \"files\"\n ],\n \"additionalProperties\": false\n }\n }\n },\n \"required\": [\n \"framework\",\n \"surface\",\n \"languages\"\n ],\n \"additionalProperties\": false\n },\n \"coverage\": {\n \"type\": \"object\",\n \"properties\": {\n \"files_scanned\": {\n \"type\": \"number\"\n },\n \"files_skipped\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"path\": {\n \"type\": \"string\"\n },\n \"reason\": {\n \"type\": \"string\",\n \"enum\": [\n \"too_large\",\n \"unreadable\"\n ]\n },\n \"size_bytes\": {\n \"type\": \"number\"\n },\n \"detail\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"path\",\n \"reason\"\n ],\n \"additionalProperties\": false\n }\n },\n \"gaps\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"code\": {\n \"type\": \"string\",\n \"enum\": [\n \"skipped_file\",\n \"unsupported_framework\",\n \"unsupported_language\",\n \"unresolved_check\"\n ]\n },\n \"message\": {\n \"type\": \"string\"\n },\n \"check_id\": {\n \"type\": \"string\"\n },\n \"file\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"code\",\n \"message\"\n ],\n \"additionalProperties\": false\n }\n },\n \"scope\": {\n \"type\": \"object\",\n \"properties\": {\n \"include_dirs\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n },\n \"excludes\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n },\n \"no_git_ignore\": {\n \"type\": \"boolean\"\n }\n },\n \"required\": [\n \"include_dirs\",\n \"excludes\",\n \"no_git_ignore\"\n ],\n \"additionalProperties\": false\n },\n \"scan_directories\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"directory\": {\n \"type\": \"string\"\n },\n \"origin\": {\n \"type\": \"string\",\n \"enum\": [\n \"app_directory\",\n \"include_dir\"\n ]\n }\n },\n \"required\": [\n \"directory\",\n \"origin\"\n ],\n \"additionalProperties\": false\n }\n }\n },\n \"required\": [\n \"files_scanned\",\n \"files_skipped\",\n \"gaps\",\n \"scope\",\n \"scan_directories\"\n ],\n \"additionalProperties\": false\n }\n },\n \"required\": [\n \"path\",\n \"schema_version\",\n \"engine\",\n \"generated_at\",\n \"detection\",\n \"coverage\"\n ],\n \"additionalProperties\": false\n },\n \"AppSecurityAgentSource\": {\n \"type\": \"object\",\n \"properties\": {\n \"path\": {\n \"type\": \"string\"\n },\n \"schema_version\": {\n \"type\": \"number\",\n \"const\": 1\n },\n \"engine\": {\n \"type\": \"object\",\n \"properties\": {\n \"name\": {\n \"type\": \"string\",\n \"const\": \"shopify-app-security\"\n },\n \"version\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"name\",\n \"version\"\n ],\n \"additionalProperties\": false\n },\n \"generated_at\": {\n \"type\": \"string\"\n },\n \"scope\": {\n \"type\": \"object\",\n \"properties\": {\n \"include_dirs\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n },\n \"excludes\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"string\"\n }\n },\n \"no_git_ignore\": {\n \"type\": \"boolean\"\n }\n },\n \"required\": [\n \"include_dirs\",\n \"excludes\",\n \"no_git_ignore\"\n ],\n \"additionalProperties\": false\n }\n },\n \"required\": [\n \"path\",\n \"schema_version\",\n \"engine\",\n \"generated_at\",\n \"scope\"\n ],\n \"additionalProperties\": false\n },\n \"AppSecurityCombinedCheck\": {\n \"type\": \"object\",\n \"properties\": {\n \"id\": {\n \"type\": \"string\"\n },\n \"title\": {\n \"type\": \"string\"\n },\n \"severity\": {\n \"type\": \"string\",\n \"enum\": [\n \"high\",\n \"medium\",\n \"low\"\n ]\n },\n \"description\": {\n \"type\": \"string\"\n },\n \"guide\": {\n \"type\": \"string\"\n },\n \"docs_url\": {\n \"type\": \"string\"\n },\n \"precedence\": {\n \"type\": \"string\",\n \"enum\": [\n \"union\",\n \"prefer-agent\"\n ]\n },\n \"applied_precedence\": {\n \"$ref\": \"#/definitions/AppSecurityCombinedCheck/properties/precedence\"\n },\n \"status\": {\n \"type\": \"string\",\n \"enum\": [\n \"executed\",\n \"not_applicable\",\n \"unresolved\"\n ]\n },\n \"by_source\": {\n \"type\": \"object\",\n \"properties\": {\n \"deterministic\": {\n \"anyOf\": [\n {\n \"type\": \"object\",\n \"properties\": {\n \"version\": {\n \"type\": \"number\"\n },\n \"status\": {\n \"$ref\": \"#/definitions/AppSecurityCombinedCheck/properties/status\"\n },\n \"reason\": {\n \"type\": \"object\",\n \"properties\": {\n \"code\": {\n \"type\": \"string\"\n },\n \"message\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"code\",\n \"message\"\n ],\n \"additionalProperties\": false\n },\n \"analysis_mode\": {\n \"type\": \"string\",\n \"enum\": [\n \"regex\",\n \"structured_config\",\n \"ast\"\n ]\n },\n \"generated_at\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"version\",\n \"status\",\n \"generated_at\"\n ],\n \"additionalProperties\": false\n },\n {\n \"type\": \"null\"\n }\n ]\n },\n \"agent\": {\n \"anyOf\": [\n {\n \"$ref\": \"#/definitions/AppSecurityCombinedCheck/properties/by_source/properties/deterministic/anyOf/0\"\n },\n {\n \"type\": \"null\"\n }\n ]\n }\n },\n \"required\": [\n \"deterministic\",\n \"agent\"\n ],\n \"additionalProperties\": false\n },\n \"findings\": {\n \"type\": \"array\",\n \"items\": {\n \"$ref\": \"#/definitions/AppSecurityCombinedFinding\"\n }\n }\n },\n \"required\": [\n \"id\",\n \"title\",\n \"severity\",\n \"description\",\n \"precedence\",\n \"applied_precedence\",\n \"status\",\n \"by_source\",\n \"findings\"\n ],\n \"additionalProperties\": false\n },\n \"AppSecurityCombinedFinding\": {\n \"type\": \"object\",\n \"properties\": {\n \"source\": {\n \"type\": \"string\",\n \"enum\": [\n \"deterministic\",\n \"agent\"\n ]\n },\n \"disposition\": {\n \"type\": \"string\",\n \"enum\": [\n \"active\",\n \"suppressed\",\n \"superseded\"\n ]\n },\n \"location\": {\n \"type\": \"object\",\n \"properties\": {\n \"file\": {\n \"type\": \"string\"\n },\n \"line\": {\n \"type\": \"number\"\n },\n \"column\": {\n \"type\": \"number\"\n }\n },\n \"required\": [\n \"file\"\n ],\n \"additionalProperties\": false\n },\n \"message\": {\n \"type\": \"string\"\n },\n \"evidence\": {\n \"type\": \"array\",\n \"items\": {\n \"type\": \"object\",\n \"properties\": {\n \"location\": {\n \"$ref\": \"#/definitions/AppSecurityCombinedFinding/properties/location\"\n },\n \"quote\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"location\"\n ],\n \"additionalProperties\": false\n }\n },\n \"snippet\": {\n \"type\": \"string\"\n },\n \"fix\": {\n \"type\": \"object\",\n \"properties\": {\n \"automated\": {\n \"type\": \"boolean\"\n },\n \"guide\": {\n \"type\": \"string\"\n },\n \"description\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"automated\",\n \"description\"\n ],\n \"additionalProperties\": false\n },\n \"confidence\": {\n \"type\": \"string\",\n \"enum\": [\n \"high\",\n \"medium\",\n \"low\"\n ]\n },\n \"reasoning\": {\n \"type\": \"string\"\n },\n \"suppression\": {\n \"type\": \"object\",\n \"properties\": {\n \"justification\": {\n \"type\": \"string\"\n }\n },\n \"required\": [\n \"justification\"\n ],\n \"additionalProperties\": false\n }\n },\n \"required\": [\n \"source\",\n \"disposition\",\n \"location\",\n \"message\",\n \"evidence\"\n ],\n \"additionalProperties\": false\n }\n },\n \"$schema\": \"http://json-schema.org/draft-07/schema#\"\n}\n```", + "descriptionWithMarkdown": "Combines the deterministic results (`deterministic-findings.json`, written by `shopify app security check`) with the recorded agent results (`agent-findings.json`, written by `shopify app security record`) and shows one view of every check: its findings, status and source. Both files are in the results directory, `.shopify/app-security//`. `--client-id` is checked against your Shopify account before any results are read, so it needs you to be logged in.\n\nThe summary shows the scan directories and the scope of the latest scan, and the scope the agent reported. It notes when the agent findings were recorded for a different scope than the latest scan; that doesn't change the exit code.\n\nThe agent results are optional. Use `--check-id` to narrow the review to specific checks, `--verbose` for full reasoning, evidence and suppressed findings, and `--blocking` to exit with code 1 when a check with findings is at or above a severity.", "enableJsonFlag": false, "flags": { "blocking": {