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 50d401f770d..b1ab37dcd76 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,9 +1,11 @@ import SecurityCheck from './check.js' import {appSecurityArtifactPaths} from '../../../services/app-security-artifacts.js' import {deterministicFindingsDocumentSchema} from '../../../services/app-security-engine/results/schema.js' +import {appFromIdentifiers} from '../../../services/context.js' import {validAppConfiguration} from '../../../services/app-security-selection.test-data.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' @@ -26,6 +28,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') @@ -305,4 +312,48 @@ 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}) + }) + }) }) diff --git a/packages/app/src/cli/commands/app/security/check.ts b/packages/app/src/cli/commands/app/security/check.ts index 8a8140e2a7e..efb313aaef5 100644 --- a/packages/app/src/cli/commands/app/security/check.ts +++ b/packages/app/src/cli/commands/app/security/check.ts @@ -13,13 +13,13 @@ 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 (\`{"files": [...]}\` with \`--json\`), and then stops. It writes no results and never prompts. \`--client-id\` is still checked, 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.` 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..abb94632476 100644 --- a/packages/app/src/cli/commands/app/security/instructions.test.ts +++ b/packages/app/src/cli/commands/app/security/instructions.test.ts @@ -5,9 +5,11 @@ import {appSecurityArtifactPaths} from '../../../services/app-security-artifacts import {resolveAppSecurityCommands} from '../../../services/app-security-commands.js' import deliverAppSecurityInstructions from '../../../services/app-security-instructions.js' import {resolveAppSecuritySelection, type AppSecuritySelection} from '../../../services/app-security-selection.js' +import {validAppConfiguration} from '../../../services/app-security-selection.test-data.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' @@ -39,6 +41,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)} } @@ -173,4 +194,20 @@ 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(deliverAppSecurityInstructions).toHaveBeenCalledWith( + expect.objectContaining({resultsKey: 'mistyped-client-id'}), + ) + }) + }) }) 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-engine/tests/layout-catalogue.test.ts b/packages/app/src/cli/services/app-security-engine/tests/layout-catalogue.test.ts index f706048b0a0..696fd284077 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 @@ -11,6 +11,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 { 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 5e02373c5b1..e7f4946fbe5 100644 --- a/packages/app/src/cli/services/app-security-selection.test.ts +++ b/packages/app/src/cli/services/app-security-selection.test.ts @@ -13,8 +13,10 @@ 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 {defaultDeveloperPlatformClient} from '../utilities/developer-platform-client.js' +import {testDeveloperPlatformClient} from '../models/app/app.test-data.js' import {AbortError} from '@shopify/cli-kit/node/error' import {fileRealPath, inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs' import {basename, joinPath} from '@shopify/cli-kit/node/path' @@ -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, } } @@ -626,6 +634,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 c3ba4fa1959..66c903f0098 100644 --- a/packages/app/src/cli/services/app-security-selection.ts +++ b/packages/app/src/cli/services/app-security-selection.ts @@ -1,6 +1,6 @@ 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 {extensionFilesForConfig, webFilesForConfig} from '../models/project/config-selection.js' import {NoAppConfigurationFoundError, Project} from '../models/project/project.js' @@ -57,18 +57,22 @@ export interface AppSecurityScanDirectory { origin: 'app_directory' | 'include_dir' | 'app_config_directory' } -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 = { @@ -81,6 +85,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. */ @@ -135,9 +143,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', } @@ -153,6 +163,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', @@ -203,6 +214,8 @@ async function resolveWithoutAppConfigurationFile( if (!options.allowPrompts) abortNoAppConfigurationFound(options.path) const appDirectory = await realDirectory(options.path) + // Before the prompt, so a mistyped client ID fails without asking anything first. + await lookUpClientIdFlag(options, dependencies) if (!(await dependencies.confirmScanWithoutAppConfig(options.path))) abortNoAppConfigurationFound(options.path) if (options.clientId) return {kind: 'no-config', appDirectory, clientId: options.clientId, clientIdSource: 'flag'} @@ -214,6 +227,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 { clientId: undefined, withoutAppConfig: false, allowPrompts: false, + validateClientIdFlag: true, }) expect(dependencies.execute).toHaveBeenCalledWith({ appDirectory, @@ -1069,3 +1072,85 @@ describe('securityCheck --list-files', () => { }) }) }) + +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 + } + + 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(), resolveSelection: resolveSelectionWith(lookUpApp)} + + await securityCheck({...testOptions(), directory: appRoot, clientId: 'flag-client-id', 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.each([false, true])( + 'aborts on an unknown --client-id before gathering files or writing results (--list-files: %s)', + async (listFiles) => { + await inTemporaryDirectory(async (directory) => { + const appRoot = await createApp(directory) + const dependencies = { + ...testDependencies(), + writeArtifacts: writeCheckArtifacts, + resolveSelection: resolveSelectionWith(async () => { + throw unknownClientId + }), + } + + await expect( + securityCheck({...testOptions(), directory: appRoot, clientId: 'unknown-client-id', listFiles}, dependencies), + ).rejects.toBe(unknownClientId) + + expect(dependencies.listFiles).not.toHaveBeenCalled() + expect(dependencies.execute).not.toHaveBeenCalled() + expect(dependencies.recordMetadata).not.toHaveBeenCalled() + expect(dependencies.output).not.toHaveBeenCalled() + 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 securityCheck( + {...testOptions(), directory: appRoot}, + {...testDependencies(), resolveSelection: resolveSelectionWith(lookUpApp)}, + ) + + 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 02d1bda20ea..ef833c58dcd 100644 --- a/packages/app/src/cli/services/security-check.ts +++ b/packages/app/src/cli/services/security-check.ts @@ -16,6 +16,7 @@ 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' @@ -65,13 +66,7 @@ interface SecurityCheckResolution { export type AppSecurityInstructionsDestination = 'copy' | 'print' | 'nothing' interface SecurityDependencies { - resolveSelection(options: { - path: string - config?: string - clientId?: string - withoutAppConfig: boolean - allowPrompts: boolean - }): Promise + resolveSelection(options: AppSecuritySelectionOptions): Promise execute(options: ScanInput & Required): Promise listFiles( options: ScanInput & Required, @@ -218,6 +213,7 @@ export default async function securityCheck( clientId: options.clientId, withoutAppConfig: options.withoutAppConfig, allowPrompts: canPrompt, + validateClientIdFlag: true, }) const {appDirectory} = selection const scope: AppSecurityScope = { 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 5bcbc66c28b..495218cf0ca 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -3982,8 +3982,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 (`{\"files\": [...]}` with `--json`), and then stops. It writes no results and never prompts. `--client-id` is still checked, 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. 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. `--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 (`{\"files\": [...]}` with `--json`), and then stops. It writes no results and never prompts. `--client-id` is still checked, 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. JSON output never prompts or prints those instructions. You can also run `shopify app security instructions` to print, copy, or write them later.", "enableJsonFlag": false, "flags": { "blocking": { @@ -4394,8 +4394,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": { @@ -4499,8 +4499,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 \"app_config_directory\"\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 \"app_config_directory\"\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": {