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 f73ce0c321f..4877ea7e391 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,10 @@ import SecurityCheck from './check.js' import {appSecurityArtifactPaths} from '../../../services/app-security-artifacts.js' +import {validAppConfiguration} from '../../../services/app-security-selection.test-data.js' import {Config} from '@oclif/core' -import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs' +import {fileRealPath, inTemporaryDirectory} from '@shopify/cli-kit/node/fs' import {unstyled} from '@shopify/cli-kit/node/output' -import {joinPath} from '@shopify/cli-kit/node/path' +import {joinPath, normalizePath} from '@shopify/cli-kit/node/path' import {describe, expect, test, vi} from 'vitest' import {mkdir, readFile, writeFile} from 'node:fs/promises' // eslint-disable-next-line n/prefer-global/console @@ -28,7 +29,7 @@ vi.mock('@shopify/cli-kit/node/session', async (importOriginal) => ({ async function createApp(directory: string): Promise<{nestedDirectory: string}> { const routesDirectory = joinPath(directory, 'app', 'routes') await mkdir(routesDirectory, {recursive: true}) - await writeFile(joinPath(directory, 'shopify.app.toml'), 'name = "Test app"\nclient_id = "test"\n') + await writeFile(joinPath(directory, 'shopify.app.toml'), validAppConfiguration()) await writeFile( joinPath(directory, 'package.json'), '{"name":"test-app","dependencies":{"@shopify/shopify-app-react-router":"1.0.0"}}\n', @@ -90,14 +91,22 @@ describe('app security check command boundary', () => { 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) - const paths = appSecurityArtifactPaths(directory) + const appDirectory = await fileRealPath(directory) + const paths = appSecurityArtifactPaths(appDirectory) const result = await runCommand(['--path', nestedDirectory, '--json', '--skip-instructions']) expect(result.exitCode).toBe(0) const output = JSON.parse(result.stdout) - expect(Object.keys(output).sort()).toEqual(['agent_checks_path', 'deterministic_findings', 'engine']) + expect(Object.keys(output).sort()).toEqual(['agent_checks_path', 'deterministic_findings', 'engine', 'selection']) expect(output.agent_checks_path).toBe(paths.agentChecksPath) + 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'}], + }) expect(output.engine).toMatchObject({name: 'shopify-app-security'}) await expect(readJson(paths.deterministicFindingsPath)).resolves.toEqual(output.deterministic_findings) await expect(readJson(paths.deterministicFindingsPath)).resolves.toMatchObject({ @@ -163,8 +172,8 @@ describe('app security check command boundary', () => { expect(result.exitCode).toBe(1) const message = errorText(result.stderr) - expect(message).toContain("Couldn't find app configuration at") - expectMentionsPath(message, joinPath(directory, 'shopify.app.shopifyappdev-dashboardjson.toml')) + expect(message).toContain("Couldn't find shopify.app.shopifyappdev-dashboardjson.toml in") + expectMentionsPath(message, normalizePath(await fileRealPath(directory))) await expect(readFile(paths.deterministicFindingsPath)).rejects.toMatchObject({code: 'ENOENT'}) }) }) 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 047b32d896c..f6788ada3ec 100644 --- a/packages/app/src/cli/commands/app/security/check.test.ts +++ b/packages/app/src/cli/commands/app/security/check.test.ts @@ -23,7 +23,17 @@ describe('app security check command', () => { test('accepts only the scan flags, with no findings or clean flags', () => { const commandFlags = Object.keys(SecurityCheck.flags).filter((name) => !(name in globalFlags)) - expect(commandFlags.sort()).toEqual(['blocking', 'config', 'ignore', 'json', 'path', 'skip-instructions', 'yes']) + expect(commandFlags.sort()).toEqual([ + 'blocking', + 'client-id', + 'config', + 'ignore', + 'json', + 'path', + 'skip-instructions', + 'without-app-config', + 'yes', + ]) }) test('forwards --path and flags to the service', async () => { @@ -35,6 +45,8 @@ describe('app security check command', () => { expect(securityCheck).toHaveBeenCalledWith({ directory: resolvePath('./fixtures/unlinked-app'), configName: undefined, + clientId: undefined, + withoutAppConfig: false, json: true, verbose: true, blocking: 'high', @@ -85,6 +97,8 @@ describe('app security check command', () => { expect(securityCheck).toHaveBeenCalledWith({ directory: '/tmp/directory-without-shopify-toml', configName: undefined, + clientId: undefined, + withoutAppConfig: false, json: false, verbose: false, blocking: 'none', @@ -94,6 +108,34 @@ describe('app security check command', () => { }) }) + 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}), + ) + }) + + test.each([ + [['--without-app-config'], 'client-id'], + [['--without-app-config', '--client-id', 'abc123', '--config', 'staging'], 'config'], + [['--client-id', 'abc123', '--config', 'staging'], 'config'], + ])('rejects the flags %j', async (flags, expectedFlag) => { + const outputMock = mockAndCaptureOutput() + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + try { + await expect(SecurityCheck.run([...flags, '--skip-instructions'], import.meta.url)).rejects.toThrow( + 'process.exit unexpectedly called with "1"', + ) + expect(outputMock.error()).toContain(expectedFlag) + expect(securityCheck).not.toHaveBeenCalled() + } finally { + consoleErrorSpy.mockRestore() + outputMock.clear() + } + }) + test('forwards --config without requiring a linked app', async () => { await SecurityCheck.run( ['--path', './fixtures/unlinked-app', '--config', 'staging', '--skip-instructions'], diff --git a/packages/app/src/cli/commands/app/security/check.ts b/packages/app/src/cli/commands/app/security/check.ts index 99d151f8b39..c51f19019b9 100644 --- a/packages/app/src/cli/commands/app/security/check.ts +++ b/packages/app/src/cli/commands/app/security/check.ts @@ -15,7 +15,7 @@ export default class SecurityCheck extends BaseCommand { static descriptionWithMarkdown = `Runs Shopify App Security locally and writes \`deterministic-findings.json\` and \`agent-checks.json\` to \`.shopify/app-security/\`. 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; App Security inspects only that configuration. +\`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; App Security 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. Use \`--ignore\` to change which files are scanned. Each value is one \`.gitignore\` pattern relative to the app directory; prefix it with \`!\` to include a file again when it is ignored by default or by \`.gitignore\`. Repeat the flag to add patterns; later patterns take precedence. A file can't be included again while its parent folder is ignored, so include the folder again instead, for example \`--ignore '!build/'\`. Quote each value so your shell doesn't expand \`!\` or \`*\` (single quotes in POSIX shells and PowerShell). The coding-agent instructions this check offers repeat the patterns. Other \`app security\` commands don't take \`--ignore\`, so pass the same patterns each time you run the check. @@ -27,6 +27,14 @@ In interactive terminals, the command offers to copy the coding-agent instructio ...globalFlags, path: appFlags.path, config: appFlags.config, + 'client-id': appFlags['client-id'], + 'without-app-config': Flags.boolean({ + description: + 'Scan --path as an app with no app configuration file. Config checks are skipped. Requires --client-id.', + env: 'SHOPIFY_FLAG_WITHOUT_APP_CONFIG', + exclusive: ['config'], + dependsOn: ['client-id'], + }), // No environment variable: oclif passes a repeatable flag's variable as one string, so it could hold only one pattern. // eslint-disable-next-line @shopify/cli/command-flags-with-env ignore: Flags.string({ @@ -61,6 +69,8 @@ In interactive terminals, the command offers to copy the coding-agent instructio await securityCheck({ 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, 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 230bdf150a3..2ec8cd1397d 100644 --- a/packages/app/src/cli/commands/app/security/clean.test.ts +++ b/packages/app/src/cli/commands/app/security/clean.test.ts @@ -1,21 +1,29 @@ import SecurityClean from './clean.js' import {appFlags} from '../../../flags.js' -import {resolveAppSecurityRoot} from '../../../services/app-security-api.js' +import {resolveAppSecuritySelection} from '../../../services/app-security-selection.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 {inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs' -import {joinPath} from '@shopify/cli-kit/node/path' +import {fileRealPath, inTemporaryDirectory, 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' import type {SecurityCleanResult} from '../../../services/security-clean-json.js' vi.mock('../../../services/security-clean.js') +vi.mock('../../../services/app-security-selection.js') +/** Creates an app directory and makes the selection resolver find it, as the real resolver would. */ async function createApp(directory: string): Promise { await writeFile(joinPath(directory, 'shopify.app.toml'), 'client_id = "test"\n') - return resolveAppSecurityRoot(directory) + const appDirectory = await fileRealPath(directory) + vi.mocked(resolveAppSecuritySelection).mockResolvedValue({ + kind: 'config', + appDirectory, + appConfigFilePath: joinPath(appDirectory, 'shopify.app.toml'), + }) + return appDirectory } function cleanedResult(appRoot: string): SecurityCleanResult { @@ -44,6 +52,7 @@ describe('app security clean command', () => { try { await SecurityClean.run([], import.meta.url) + expect(resolveAppSecuritySelection).toHaveBeenCalledWith({path: cwd(), allowPrompts: false}) expect(securityClean).toHaveBeenCalledWith({appRoot}) expect(renderSecurityCleanResult).toHaveBeenCalledWith(result, appRoot) expect(output.info()).toBe('') @@ -65,6 +74,7 @@ describe('app security clean command', () => { try { await SecurityClean.run(['--path', directory, '--json'], import.meta.url) + expect(resolveAppSecuritySelection).toHaveBeenCalledWith({path: directory, allowPrompts: false}) expect(securityClean).toHaveBeenCalledWith({appRoot}) expect(output.info()).toBe( ['{', ' "removed": [', ` ${JSON.stringify(result.removed[0])}`, ' ]', '}'].join('\n'), diff --git a/packages/app/src/cli/commands/app/security/clean.ts b/packages/app/src/cli/commands/app/security/clean.ts index a2dc6fe4a76..0fd3d8df282 100644 --- a/packages/app/src/cli/commands/app/security/clean.ts +++ b/packages/app/src/cli/commands/app/security/clean.ts @@ -1,5 +1,5 @@ import {appFlags} from '../../../flags.js' -import {resolveAppSecurityRoot} from '../../../services/app-security-api.js' +import {resolveAppSecuritySelection} from '../../../services/app-security-selection.js' import securityClean, {renderSecurityCleanResult} from '../../../services/security-clean.js' import {securityCleanJsonOutputSchema} from '../../../services/security-clean-json.js' import BaseCommand from '@shopify/cli-kit/node/base-command' @@ -28,7 +28,7 @@ export default class SecurityClean extends BaseCommand { public async run(): Promise { const {flags} = await this.parse(SecurityClean) - const appRoot = resolveAppSecurityRoot(flags.path) + const {appDirectory: appRoot} = await resolveAppSecuritySelection({path: flags.path, allowPrompts: false}) const result = await securityClean({appRoot}) if (flags.json) { 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 26073fd54df..4dc80192284 100644 --- a/packages/app/src/cli/commands/app/security/instructions.test.ts +++ b/packages/app/src/cli/commands/app/security/instructions.test.ts @@ -1,14 +1,28 @@ import SecurityInstructions from './instructions.js' import {appFlags} from '../../../flags.js' +import {resolveAppSecurityCommands} from '../../../services/app-security-commands.js' import deliverAppSecurityInstructions from '../../../services/app-security-instructions.js' +import {resolveAppSecuritySelection} from '../../../services/app-security-selection.js' import AppLinkedCommand from '../../../utilities/app-linked-command.js' import BaseCommand from '@shopify/cli-kit/node/base-command' import {cwd, resolvePath} from '@shopify/cli-kit/node/path' -import {describe, expect, test, vi} from 'vitest' +import {beforeEach, describe, expect, test, vi} from 'vitest' vi.mock('../../../services/app-security-instructions.js') +vi.mock('../../../services/app-security-selection.js', async (importOriginal) => ({ + ...(await importOriginal()), + resolveAppSecuritySelection: vi.fn(), +})) describe('app security instructions command', () => { + beforeEach(() => { + vi.mocked(resolveAppSecuritySelection).mockResolvedValue({ + kind: 'config', + appDirectory: '/tmp/app', + appConfigFilePath: '/tmp/app/shopify.app.toml', + }) + }) + test('is hidden and does not require linked app context', () => { expect(SecurityInstructions.hidden).toBe(true) expect(SecurityInstructions.prototype).toBeInstanceOf(BaseCommand) @@ -21,9 +35,10 @@ describe('app security instructions command', () => { test('prints instructions for the current directory by default', async () => { await SecurityInstructions.run([], import.meta.url) + expect(resolveAppSecuritySelection).toHaveBeenCalledWith({path: cwd(), config: undefined, allowPrompts: false}) expect(deliverAppSecurityInstructions).toHaveBeenCalledWith({ - directory: cwd(), - configName: undefined, + appDirectory: '/tmp/app', + commands: resolveAppSecurityCommands('/tmp/app', 'shopify.app.toml'), copy: false, writePath: undefined, }) @@ -32,29 +47,33 @@ describe('app security instructions command', () => { test('forwards --path and --copy', async () => { await SecurityInstructions.run(['--path', './fixtures/unlinked-app', '--copy'], import.meta.url) - expect(deliverAppSecurityInstructions).toHaveBeenCalledWith({ - directory: resolvePath('./fixtures/unlinked-app'), - configName: undefined, - copy: true, - writePath: undefined, - }) + expect(resolveAppSecuritySelection).toHaveBeenCalledWith( + expect.objectContaining({path: resolvePath('./fixtures/unlinked-app')}), + ) + expect(deliverAppSecurityInstructions).toHaveBeenCalledWith(expect.objectContaining({copy: true})) }) test('resolves and forwards --write', async () => { await SecurityInstructions.run(['--write', './instructions.md'], import.meta.url) - expect(deliverAppSecurityInstructions).toHaveBeenCalledWith({ - directory: cwd(), - configName: undefined, - copy: false, - writePath: resolvePath('./instructions.md'), - }) + expect(deliverAppSecurityInstructions).toHaveBeenCalledWith( + expect.objectContaining({copy: false, writePath: resolvePath('./instructions.md')}), + ) }) - test('forwards --config', async () => { + test('selects the configuration named by --config and puts it in the commands', async () => { + vi.mocked(resolveAppSecuritySelection).mockResolvedValue({ + kind: 'config', + appDirectory: '/tmp/app', + appConfigFilePath: '/tmp/app/shopify.app.staging.toml', + }) + await SecurityInstructions.run(['--path', './fixtures/unlinked-app', '--config', 'staging'], import.meta.url) - expect(deliverAppSecurityInstructions).toHaveBeenCalledWith(expect.objectContaining({configName: 'staging'})) + expect(resolveAppSecuritySelection).toHaveBeenCalledWith(expect.objectContaining({config: 'staging'})) + expect(deliverAppSecurityInstructions).toHaveBeenCalledWith( + expect.objectContaining({commands: resolveAppSecurityCommands('/tmp/app', 'shopify.app.staging.toml')}), + ) }) test('keeps --copy and --write mutually exclusive', () => { diff --git a/packages/app/src/cli/commands/app/security/instructions.ts b/packages/app/src/cli/commands/app/security/instructions.ts index 02f63b7423e..28e801632a2 100644 --- a/packages/app/src/cli/commands/app/security/instructions.ts +++ b/packages/app/src/cli/commands/app/security/instructions.ts @@ -1,5 +1,7 @@ import {appFlags} from '../../../flags.js' +import {resolveAppSecurityCommands} from '../../../services/app-security-commands.js' import deliverAppSecurityInstructions from '../../../services/app-security-instructions.js' +import {resolveAppSecuritySelection, selectedConfigFileName} from '../../../services/app-security-selection.js' import {Flags} from '@oclif/core' import BaseCommand from '@shopify/cli-kit/node/base-command' import {globalFlags} from '@shopify/cli-kit/node/cli' @@ -37,9 +39,15 @@ By default, the instructions are printed to stdout. Use \`--copy\` to copy them public async run(): Promise { const {flags} = await this.parse(SecurityInstructions) + const selection = await resolveAppSecuritySelection({ + path: flags.path, + config: flags.config, + allowPrompts: false, + }) + await deliverAppSecurityInstructions({ - directory: flags.path, - configName: flags.config, + appDirectory: selection.appDirectory, + commands: resolveAppSecurityCommands(selection.appDirectory, selectedConfigFileName(selection)), copy: flags.copy, writePath: flags.write, }) 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 dab84a410ab..3a0e54bbb10 100644 --- a/packages/app/src/cli/commands/app/security/record.test.ts +++ b/packages/app/src/cli/commands/app/security/record.test.ts @@ -1,20 +1,28 @@ import SecurityRecord from './record.js' import {appFlags} from '../../../flags.js' -import {resolveAppSecurityRoot} from '../../../services/app-security-api.js' +import {resolveAppSecuritySelection} from '../../../services/app-security-selection.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 {inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs' -import {joinPath} from '@shopify/cli-kit/node/path' +import {fileRealPath, inTemporaryDirectory, 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' vi.mock('../../../services/security-record.js') +vi.mock('../../../services/app-security-selection.js') +/** Creates an app directory and makes the selection resolver find it, as the real resolver would. */ async function createApp(directory: string): Promise { await writeFile(joinPath(directory, 'shopify.app.toml'), 'client_id = "test"\n') - return resolveAppSecurityRoot(directory) + const appDirectory = await fileRealPath(directory) + vi.mocked(resolveAppSecuritySelection).mockResolvedValue({ + kind: 'config', + appDirectory, + appConfigFilePath: joinPath(appDirectory, 'shopify.app.toml'), + }) + return appDirectory } function recordedResult(appRoot: string) { @@ -44,6 +52,7 @@ describe('app security record command', () => { try { await SecurityRecord.run([], import.meta.url) + expect(resolveAppSecuritySelection).toHaveBeenCalledWith({path: cwd(), allowPrompts: false}) expect(securityRecord).toHaveBeenCalledWith({appRoot}) expect(renderSecurityRecordResult).toHaveBeenCalledWith(result, appRoot) expect(output.info()).toBe('') @@ -64,6 +73,7 @@ describe('app security record command', () => { try { await SecurityRecord.run(['--path', directory, '--json'], import.meta.url) + expect(resolveAppSecuritySelection).toHaveBeenCalledWith({path: directory, allowPrompts: false}) expect(securityRecord).toHaveBeenCalledWith({appRoot}) expect(output.info()).toBe( [ diff --git a/packages/app/src/cli/commands/app/security/record.ts b/packages/app/src/cli/commands/app/security/record.ts index cbd2fbd6984..e17eb1fcd74 100644 --- a/packages/app/src/cli/commands/app/security/record.ts +++ b/packages/app/src/cli/commands/app/security/record.ts @@ -1,5 +1,5 @@ import {appFlags} from '../../../flags.js' -import {resolveAppSecurityRoot} from '../../../services/app-security-api.js' +import {resolveAppSecuritySelection} from '../../../services/app-security-selection.js' import securityRecord, {renderSecurityRecordResult} from '../../../services/security-record.js' import {securityRecordJsonOutputSchema} from '../../../services/security-record-json.js' import BaseCommand from '@shopify/cli-kit/node/base-command' @@ -31,7 +31,7 @@ The document is recorded all or nothing: if anything is invalid, the command fai public async run(): Promise { const {flags} = await this.parse(SecurityRecord) - const appRoot = resolveAppSecurityRoot(flags.path) + const {appDirectory: appRoot} = await resolveAppSecuritySelection({path: flags.path, allowPrompts: false}) const result = await securityRecord({appRoot}) if (flags.json) { diff --git a/packages/app/src/cli/models/project/project.test.ts b/packages/app/src/cli/models/project/project.test.ts index 2bdba160a65..4d9b67adfc5 100644 --- a/packages/app/src/cli/models/project/project.test.ts +++ b/packages/app/src/cli/models/project/project.test.ts @@ -1,5 +1,6 @@ -import {Project} from './project.js' +import {NoAppConfigurationFoundError, Project} from './project.js' import {describe, expect, test} from 'vitest' +import {AbortError} from '@shopify/cli-kit/node/error' import {inTemporaryDirectory, writeFile, mkdir} from '@shopify/cli-kit/node/fs' import {joinPath, normalizePath} from '@shopify/cli-kit/node/path' @@ -122,6 +123,18 @@ describe('Project', () => { }) }) + test('throws a NoAppConfigurationFoundError, an AbortError, when no app config file is at or above the directory', async () => { + await inTemporaryDirectory(async (dir) => { + const error = await Project.load(dir).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(NoAppConfigurationFoundError) + expect(error).toBeInstanceOf(AbortError) + expect((error as AbortError).message).toBe( + `Could not find a Shopify app configuration file. Looked in ${dir} and parent directories.`, + ) + }) + }) + test('uses custom extension_directories from app config', async () => { await inTemporaryDirectory(async (dir) => { await writeAppToml(dir, 'client_id = "abc"\nextension_directories = ["custom/*"]') diff --git a/packages/app/src/cli/models/project/project.ts b/packages/app/src/cli/models/project/project.ts index 1b0c39c31b5..688d97a19fe 100644 --- a/packages/app/src/cli/models/project/project.ts +++ b/packages/app/src/cli/models/project/project.ts @@ -160,6 +160,8 @@ export class Project { // ── Filesystem discovery functions ────────────────────────── +export class NoAppConfigurationFoundError extends AbortError {} + async function findProjectRoot(startDirectory: string): Promise { const found = await findPathUp( async (directory) => { @@ -172,7 +174,7 @@ async function findProjectRoot(startDirectory: string): Promise { }, ) if (!found) { - throw new AbortError( + throw new NoAppConfigurationFoundError( `Could not find a Shopify app configuration file. Looked in ${startDirectory} and parent directories.`, ) } diff --git a/packages/app/src/cli/services/app-security-api.test.ts b/packages/app/src/cli/services/app-security-api.test.ts index 8b8632df144..cd5ab5f8ddc 100644 --- a/packages/app/src/cli/services/app-security-api.test.ts +++ b/packages/app/src/cli/services/app-security-api.test.ts @@ -1,7 +1,6 @@ import { securityExitCode, executeAppSecurity, - resolveAppSecurityRoot, type AppSecurityBlockingLevel, type AppSecurityExecution, } from './app-security-api.js' @@ -17,10 +16,13 @@ function artifactPath(directory: string, name: string): string { return joinPath(directory, '.shopify', 'app-security', name) } +function scanInputFor(directory: string) { + return {appDirectory: directory, appConfigFilePath: joinPath(directory, 'shopify.app.toml')} +} + async function runSecurity(options: {directory: string; blocking: AppSecurityBlockingLevel}) { - const appRoot = resolveAppSecurityRoot(options.directory) - const execution = await executeAppSecurity({appRoot}) - const artifacts = await writeCheckArtifacts(execution.appRoot, execution) + const execution = await executeAppSecurity(scanInputFor(options.directory)) + const artifacts = await writeCheckArtifacts(options.directory, execution) return {execution, artifacts, exitCode: securityExitCode(execution, options.blocking)} } @@ -114,36 +116,15 @@ describe('App Security CLI integration', () => { await createApp(directory) await mkdir(joinPath(directory, 'generated')) await writeFile(joinPath(directory, 'generated', 'client.ts'), 'export const generated = true\n') - const appRoot = resolveAppSecurityRoot(directory) + const scanInput = scanInputFor(directory) - const unfiltered = await executeAppSecurity({appRoot}) - const filtered = await executeAppSecurity({appRoot, ignorePatterns: ['generated/']}) + const unfiltered = await executeAppSecurity(scanInput) + const filtered = await executeAppSecurity({...scanInput, ignorePatterns: ['generated/']}) expect(unfiltered.scan.scan.files_scanned - filtered.scan.scan.files_scanned).toBe(1) }) }) - test('translates a missing --path into an AbortError with a next step', async () => { - await inTemporaryDirectory(async (directory) => { - const missing = joinPath(directory, 'missing-app') - - await expect(runSecurity({directory: missing, blocking: 'none'})).rejects.toMatchObject({ - constructor: AbortError, - message: `App path does not exist: ${missing}`, - tryMessage: 'Run this command from a Shopify app directory or pass --path to one.', - }) - }) - }) - - test('translates a directory without an app configuration into an AbortError', async () => { - await inTemporaryDirectory(async (directory) => { - await expect(runSecurity({directory, blocking: 'none'})).rejects.toBeInstanceOf(AbortError) - await expect(runSecurity({directory, blocking: 'none'})).rejects.toThrow( - `Could not find a shopify.app*.toml from: ${directory}`, - ) - }) - }) - test('rejects artifact symlinks that target outside the app', async () => { await inTemporaryDirectory(async (directory) => { await createApp(directory) diff --git a/packages/app/src/cli/services/app-security-api.ts b/packages/app/src/cli/services/app-security-api.ts index 47d2fbb2a82..7f13edf5e70 100644 --- a/packages/app/src/cli/services/app-security-api.ts +++ b/packages/app/src/cli/services/app-security-api.ts @@ -1,13 +1,11 @@ import { - AppRootDiscoveryError, - findAppRoot, scanApp, SEVERITY_RANK, type AppSecurityEngineMetadata, type AppSecurityScan, + type ScanInput, type Severity, } from './app-security-engine/index.js' -import {AbortError} from '@shopify/cli-kit/node/error' export type {AppSecurityEngineMetadata} @@ -21,24 +19,12 @@ export function securityExitCode(execution: AppSecurityExecution, blocking: AppS return blocks ? 1 : 0 } -export function resolveAppSecurityRoot(directory?: string): string { - try { - return findAppRoot(directory) - } catch (error) { - if (error instanceof AppRootDiscoveryError) { - throw new AbortError(error.message, 'Run this command from a Shopify app directory or pass --path to one.') - } - throw error - } -} - -export async function executeAppSecurity(options: { - appRoot: string - configFileName?: string - ignorePatterns?: ReadonlyArray -}): Promise { +export async function executeAppSecurity({ + ignorePatterns, + ...scanInput +}: ScanInput & {ignorePatterns?: ReadonlyArray}): Promise { const startTime = Date.now() - const result = await scanApp(options.appRoot, options.configFileName, {ignorePatterns: options.ignorePatterns}) + const result = await scanApp(scanInput, {ignorePatterns}) return { ...result, elapsedMilliseconds: Date.now() - startTime, diff --git a/packages/app/src/cli/services/app-security-artifacts.test.ts b/packages/app/src/cli/services/app-security-artifacts.test.ts index 01d11566945..d6d5e65e449 100644 --- a/packages/app/src/cli/services/app-security-artifacts.test.ts +++ b/packages/app/src/cli/services/app-security-artifacts.test.ts @@ -5,7 +5,7 @@ import { writeAgentFindings, writeCheckArtifacts, } from './app-security-artifacts.js' -import {scanApp} from './app-security-engine/index.js' +import {scanAppDirectory} from './app-security-engine/tests/scan-directory.js' import {agentFindingsDocument} from './app-security-engine/tests/fixtures/findings-documents.js' import {AbortError} from '@shopify/cli-kit/node/error' import {fileExists, inTemporaryDirectory, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs' @@ -15,7 +15,7 @@ import {symlink} from 'node:fs/promises' async function scanTestApp(directory: string) { await writeFile(joinPath(directory, 'shopify.app.toml'), 'name = "Test"\nclient_id = "test"\n') - return scanApp(directory) + return scanAppDirectory(directory) } async function writeEveryArtifact(directory: string): Promise { diff --git a/packages/app/src/cli/services/app-security-config.test.ts b/packages/app/src/cli/services/app-security-config.test.ts deleted file mode 100644 index 127db06205b..00000000000 --- a/packages/app/src/cli/services/app-security-config.test.ts +++ /dev/null @@ -1,69 +0,0 @@ -import {requireSecurityConfigFileName, resolveSecurityConfigFileName} from './app-security-config.js' -import {getCachedAppInfo} from './local-storage.js' -import {beforeEach, describe, expect, test, vi} from 'vitest' -import {AbortError} from '@shopify/cli-kit/node/error' -import {inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs' -import {joinPath} from '@shopify/cli-kit/node/path' - -vi.mock('./local-storage.js', () => ({getCachedAppInfo: vi.fn()})) - -beforeEach(() => { - vi.mocked(getCachedAppInfo).mockReset() -}) - -describe('resolveSecurityConfigFileName', () => { - test.each(['production-eu', 'shopify.app.production-eu.toml', 'Production EU'])( - 'canonicalizes the config name %s without reading cache', - async (configName) => { - await inTemporaryDirectory(async (directory) => { - expect(resolveSecurityConfigFileName(directory, configName)).toBe('shopify.app.production-eu.toml') - expect(getCachedAppInfo).not.toHaveBeenCalled() - }) - }, - ) - - test('uses the cached config instead of the default config', async () => { - await inTemporaryDirectory(async (directory) => { - vi.mocked(getCachedAppInfo).mockReturnValue({directory, configFile: 'shopify.app.production.toml'}) - - expect(resolveSecurityConfigFileName(directory)).toBe('shopify.app.production.toml') - expect(getCachedAppInfo).toHaveBeenCalledWith(directory) - }) - }) - - test('falls back to shopify.app.toml when no flag or cache is present', async () => { - await inTemporaryDirectory(async (directory) => { - expect(resolveSecurityConfigFileName(directory)).toBe('shopify.app.toml') - expect(getCachedAppInfo).toHaveBeenCalledWith(directory) - }) - }) -}) - -describe('requireSecurityConfigFileName', () => { - test('returns the selected file when it exists', async () => { - await inTemporaryDirectory(async (directory) => { - await writeFile(joinPath(directory, 'shopify.app.staging.toml'), 'name = "Staging"\n') - - expect(requireSecurityConfigFileName(directory, 'staging')).toBe('shopify.app.staging.toml') - }) - }) - - test('rejects a missing selected config without falling back to a sibling', async () => { - await inTemporaryDirectory(async (directory) => { - await writeFile(joinPath(directory, 'shopify.app.other.toml'), 'name = "Other"\n') - - expect(() => requireSecurityConfigFileName(directory, 'missing')).toThrow(AbortError) - expect(() => requireSecurityConfigFileName(directory, 'missing')).toThrow(/shopify\.app\.missing\.toml/) - }) - }) - - test('rejects a stale cached config without falling back to the default file', async () => { - await inTemporaryDirectory(async (directory) => { - await writeFile(joinPath(directory, 'shopify.app.toml'), 'name = "Default"\n') - vi.mocked(getCachedAppInfo).mockReturnValue({directory, configFile: 'shopify.app.deleted.toml'}) - - expect(() => requireSecurityConfigFileName(directory)).toThrow(AbortError) - expect(() => requireSecurityConfigFileName(directory)).toThrow(/shopify\.app\.deleted\.toml/) - }) - }) -}) diff --git a/packages/app/src/cli/services/app-security-config.ts b/packages/app/src/cli/services/app-security-config.ts deleted file mode 100644 index c77c95480dd..00000000000 --- a/packages/app/src/cli/services/app-security-config.ts +++ /dev/null @@ -1,25 +0,0 @@ -import {getCachedAppInfo} from './local-storage.js' -import {getAppConfigurationFileName} from '../models/app/config-file-naming.js' -import {AbortError} from '@shopify/cli-kit/node/error' -import {fileExistsSync} from '@shopify/cli-kit/node/fs' -import {joinPath} from '@shopify/cli-kit/node/path' - -/** Resolve the app configuration filename without loading or linking the app. */ -export function resolveSecurityConfigFileName(directory: string, configName?: string): string { - return getAppConfigurationFileName(configName ?? getCachedAppInfo(directory)?.configFile) -} - -/** Resolve the selected configuration and abort if that file is missing. */ -export function requireSecurityConfigFileName(directory: string, configName?: string): string { - const configFileName = resolveSecurityConfigFileName(directory, configName) - const configPath = joinPath(directory, configFileName) - - // Do not fall back to another config: that would scan or describe a different app. - if (!fileExistsSync(configPath)) { - throw new AbortError(`Couldn't find app configuration at ${configPath}.`, null, [ - 'Pass `--config ` to select an existing app configuration.', - ]) - } - - return configFileName -} diff --git a/packages/app/src/cli/services/app-security-engine/index.ts b/packages/app/src/cli/services/app-security-engine/index.ts index 9b454862459..5d7a4ad58a6 100644 --- a/packages/app/src/cli/services/app-security-engine/index.ts +++ b/packages/app/src/cli/services/app-security-engine/index.ts @@ -2,21 +2,14 @@ * Public App Security engine API. * * CLI code outside this directory should import only these operations and result - * types: locate an app, read its git state, scan, record agent findings, translate + * types: read the git state, scan, record agent findings, translate * a stored findings document (deterministic-findings.json or agent-findings.json, which share the * converged FindingsDocument schema), combine the two result files into per-check results, group * issues for display, and validate `--ignore` patterns. The stored documents' Zod * schemas are exported too, so the public `review --json` schema is composed from them rather than re-declared. * Keep scanners, registries, validators, redaction, and the rest of the stored-schema details inside the engine. */ -export { - AppRootDiscoveryError, - findAppRoot, - getAgentInstructions, - getEngineVersion, - readProjectState, - scanApp, -} from './run.js' +export {getAgentInstructions, getEngineVersion, readProjectState, scanApp} from './run.js' export type {AppSecurityEngineMetadata, AppSecurityScan} from './run.js' export {translateFindingsDocument} from './results/translate.js' export type {TranslateFindingsDocumentResult} from './results/translate.js' @@ -64,6 +57,7 @@ export type { FindingsSource, Issue, ProjectState, + ScanInput, ScanResult, Severity, StoredCheck, diff --git a/packages/app/src/cli/services/app-security-engine/run.ts b/packages/app/src/cli/services/app-security-engine/run.ts index 538c746f5ff..d2ad5ca52d3 100644 --- a/packages/app/src/cli/services/app-security-engine/run.ts +++ b/packages/app/src/cli/services/app-security-engine/run.ts @@ -1,12 +1,11 @@ import {EMBEDDED_APP_SECURITY_INSTRUCTIONS} from './checks/embedded.js' import {buildAgentChecks, type AgentChecks} from './checks/index.js' -import {AppRootDiscoveryError, findAppRoot} from './scanners/discover.js' import {readProjectState, scan} from './scanners/index.js' import {buildDeterministicFindings} from './scan-artifact/index.js' import {getEngineVersion} from './version.js' -import type {DeterministicFindingsDocument, ScanOptions, ScanResult} from './types.js' +import type {DeterministicFindingsDocument, ScanInput, ScanOptions, ScanResult} from './types.js' -export {AppRootDiscoveryError, findAppRoot, getEngineVersion, readProjectState} +export {getEngineVersion, readProjectState} export interface AppSecurityEngineMetadata { name: string @@ -15,7 +14,6 @@ export interface AppSecurityEngineMetadata { } export interface AppSecurityScan { - appRoot: string scan: ScanResult deterministicFindings: DeterministicFindingsDocument agentChecks: AgentChecks @@ -26,17 +24,11 @@ export function getAgentInstructions(): string { return EMBEDDED_APP_SECURITY_INSTRUCTIONS } -export async function scanApp( - directory?: string, - configFileName?: string, - options?: ScanOptions, -): Promise { - const appRoot = findAppRoot(directory) - const result = await scan(appRoot, configFileName, options) +export async function scanApp(input: ScanInput, options?: ScanOptions): Promise { + const result = await scan(input, options) const engineVersion = getEngineVersion() const deterministicFindings = buildDeterministicFindings(result, {engineVersion}) return { - appRoot, scan: result, deterministicFindings, agentChecks: buildAgentChecks(engineVersion), diff --git a/packages/app/src/cli/services/app-security-engine/scanners/discover.ts b/packages/app/src/cli/services/app-security-engine/scanners/discover.ts index db814f4fd09..22410aa9c5f 100644 --- a/packages/app/src/cli/services/app-security-engine/scanners/discover.ts +++ b/packages/app/src/cli/services/app-security-engine/scanners/discover.ts @@ -2,14 +2,13 @@ import {inspectErrorReason, isMissingFilesystemEntry} from './filesystem-errors. import {createFilePathMatcher, createPathMatcher} from './path-rules.js' import {findRepositoryMarker} from './repository-marker.js' import {DEPENDENCY_AUTOMATION_CONFIG_PATHS} from '../rules/dependency-automation-rules.js' -import {APP_CONFIG_FILE_GLOB, isValidFormatAppConfigurationFileName} from '../../../models/app/config-file-naming.js' +import {isValidFormatAppConfigurationFileName} from '../../../models/app/config-file-naming.js' import {AppAccessScopesSchema, AppAuthSchema} from '../../../models/extensions/specifications/app_config_app_access.js' import {WebhookSubscriptionSchema} from '../../../models/extensions/specifications/app_config_webhook_schemas/webhook_subscription_schema.js' import {removeTrailingSlash} from '../../../models/extensions/specifications/validation/common.js' -import {fileExistsSync, fileSizeSync, globSync, readFileSync} from '@shopify/cli-kit/node/fs' +import {fileSizeSync, readFileSync} from '@shopify/cli-kit/node/fs' import { basename, - cwd, dirname, extname, isAbsolutePath, @@ -34,75 +33,19 @@ import type { } from './types.js' import type {Dirent} from 'node:fs' -/** Expected user error while locating a Shopify app root. */ -export class AppRootDiscoveryError extends Error { - constructor(message: string) { - super(message) - this.name = 'AppRootDiscoveryError' - } -} - -/** - * Find the nearest app root without ever substituting CWD for a bad explicit path. - * - * App identity matches the rest of Shopify CLI: walk up for - * `shopify.app.toml` / `shopify.app..toml` using - * `isValidFormatAppConfigurationFileName`. This is not `Project.load()` — that - * loader also reads package, environment, and hidden configuration, follows - * different symlink policy, and does not keep the bounded raw bytes App Security - * hashes and reports as coverage. - */ -export function findAppRoot(startPath?: string): string { - const requestedPath = resolvePath(startPath ?? cwd()) - if (startPath && !fileExistsSync(requestedPath)) { - throw new AppRootDiscoveryError(`App path does not exist: ${startPath}`) - } - - let directory = requestedPath - if (startPath && lstatSync(requestedPath).isFile()) { - if (!isValidFormatAppConfigurationFileName(basename(requestedPath))) { - throw new AppRootDiscoveryError(`App path is not a directory or Shopify app configuration file: ${startPath}`) - } - return dirname(requestedPath) - } - if (!lstatSync(directory).isDirectory()) { - throw new AppRootDiscoveryError(`App path is not a directory: ${startPath ?? directory}`) - } - - while (true) { - if (listAppConfigFiles(directory).length > 0) return directory - - const parent = dirname(directory) - if (parent === directory) break - directory = parent - } - - throw new AppRootDiscoveryError(`Could not find a shopify.app*.toml from: ${startPath ?? cwd()}`) -} - -function listAppConfigFiles(directory: string): string[] { - return globSync(APP_CONFIG_FILE_GLOB, { - cwd: directory, - deep: 1, - dot: false, - onlyFiles: false, - followSymbolicLinks: false, - }).filter((file) => isValidFormatAppConfigurationFileName(basename(file))) -} - /** - * Load a specific shopify.app.toml file. + * Load the selected shopify.app.toml file. */ -export function loadAppToml(tomlPath: string, appRoot = dirname(tomlPath)): AppTomlContent | null { - const content = readRepositoryText(appRoot, tomlPath) +export function loadAppToml(appConfigFilePath: string, appDirectory: string): AppTomlContent | null { + const content = readRepositoryText(appDirectory, appConfigFilePath) if (content === undefined) return null try { const raw = decodeToml(content) as Record - return parseAppToml(raw, tomlPath, content, appRoot) + return parseAppToml(raw, appConfigFilePath, content, appDirectory) // Invalid repository TOML is a coverage gap, not a scanner crash. // eslint-disable-next-line no-catch-all/no-catch-all } catch { - recordSkippedFile(appRoot, tomlPath, { + recordSkippedFile(appDirectory, appConfigFilePath, { ok: false, reason: 'unreadable', detail: 'TOML could not be parsed', diff --git a/packages/app/src/cli/services/app-security-engine/scanners/index.ts b/packages/app/src/cli/services/app-security-engine/scanners/index.ts index 65b2bce5f9c..2724dcdcfcc 100644 --- a/packages/app/src/cli/services/app-security-engine/scanners/index.ts +++ b/packages/app/src/cli/services/app-security-engine/scanners/index.ts @@ -1,5 +1,4 @@ import { - findAppRoot, loadAppToml, findExtensions, findAppSourceFiles, @@ -34,8 +33,7 @@ import {scanDependencyAutomation} from '../rules/dependency-automation-rules.js' import {RULE_CATALOG} from '../rules/catalog.js' import {redactIssue} from '../scan-artifact/index.js' import {getEngineVersion} from '../version.js' -import {getAppConfigurationFileName} from '../../../models/app/config-file-naming.js' -import {joinPath, relativePath} from '@shopify/cli-kit/node/path' +import {basename, relativePath} from '@shopify/cli-kit/node/path' import {captureOutputWithExitCode} from '@shopify/cli-kit/node/system' import type {Rule, ScanContext} from '../rules/types.js' import type {RunnerImplementationResult, RunnerResult, SourceFile} from './types.js' @@ -47,6 +45,7 @@ import type { CoverageGap, Issue, ProjectState, + ScanInput, ScanOptions, ScanResult, SkippedFile, @@ -569,14 +568,12 @@ function normalizeRunnerResult(value: Issue[] | RunnerResult): RunnerResult { } export async function scan( - startPath?: string, - configFileName?: string, + {appDirectory: appRoot, appConfigFilePath}: ScanInput, options: ScanOptions = {}, ): Promise { - const appRoot = findAppRoot(startPath) resetSkippedFiles() - const selectedFileName = getAppConfigurationFileName(configFileName) - const appToml = loadAppToml(joinPath(appRoot, selectedFileName), appRoot) + const selectedFileName = appConfigFilePath ? basename(appConfigFilePath) : undefined + const appToml = appConfigFilePath ? loadAppToml(appConfigFilePath, appRoot) : null const appTomls = appToml ? [appToml] : [] const overrides = ignorePatternRules(options.ignorePatterns ?? []) const gitIgnoreListing = await listGitIgnoredPaths(appRoot, { @@ -592,7 +589,8 @@ export async function scan( const sourceFiles = findAppSourceFiles(appRoot, repositoryFiles) // The selected app configuration is an explicit input, not a discovered path: it's loaded and // scanned for secrets even when path rules exclude it. - const sensitivePaths = appToml ? [...new Set([...repositoryFiles, selectedFileName])].sort() : repositoryFiles + const sensitivePaths = + appToml && selectedFileName ? [...new Set([...repositoryFiles, selectedFileName])].sort() : repositoryFiles const sensitiveFiles = findSensitiveFiles(appRoot, sensitivePaths, selectedFileName) const manifestPaths = findManifestPaths(repositoryFiles) const manifests = findManifests(appRoot, manifestPaths) diff --git a/packages/app/src/cli/services/app-security-engine/tests/dependency-automation.test.ts b/packages/app/src/cli/services/app-security-engine/tests/dependency-automation.test.ts index fd014f92a7c..1bb66b955d5 100644 --- a/packages/app/src/cli/services/app-security-engine/tests/dependency-automation.test.ts +++ b/packages/app/src/cli/services/app-security-engine/tests/dependency-automation.test.ts @@ -1,11 +1,11 @@ /* eslint-disable no-restricted-imports -- integration coverage uses real temporary repositories */ import {git, isolateGitConfig} from './git-test-helpers.js' +import {scanAppDirectory as scanApp, scanDirectory as scan} from './scan-directory.js' import {securityExitCode} from '../../app-security-api.js' import {formatJson} from '../output/format.js' import {getRegistry} from '../registry/index.js' -import {DETERMINISTIC_CHECKS, scan} from '../scanners/index.js' +import {DETERMINISTIC_CHECKS} from '../scanners/index.js' import {translateFindingsDocument} from '../results/translate.js' -import {scanApp} from '../run.js' import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs' import {fetch} from '@shopify/cli-kit/node/http' import {captureOutputWithExitCode} from '@shopify/cli-kit/node/system' diff --git a/packages/app/src/cli/services/app-security-engine/tests/discovery-safety.test.ts b/packages/app/src/cli/services/app-security-engine/tests/discovery-safety.test.ts index df161956009..39f3c62b4d4 100644 --- a/packages/app/src/cli/services/app-security-engine/tests/discovery-safety.test.ts +++ b/packages/app/src/cli/services/app-security-engine/tests/discovery-safety.test.ts @@ -1,17 +1,15 @@ /* eslint-disable no-restricted-imports -- discovery boundaries use real temporary repositories */ import {git, isolateGitConfig} from './git-test-helpers.js' +import {scanDirectory as scan} from './scan-directory.js' import { - AppRootDiscoveryError, - findAppRoot, findExtensions, findSourceCandidates, getSkippedFiles, listRepositoryFiles, resetSkippedFiles, } from '../scanners/discover.js' -import {scan} from '../scanners/index.js' import {buildPathRules, listGitIgnoredPaths} from '../scanners/path-rules.js' -import {joinPath, normalizePath} from '@shopify/cli-kit/node/path' +import {joinPath} from '@shopify/cli-kit/node/path' import {afterEach, beforeEach, describe, expect, test} from 'vitest' import {chmod, mkdir, mkdtemp, rm, symlink, writeFile} from 'node:fs/promises' import {tmpdir} from 'node:os' @@ -77,53 +75,6 @@ async function scanPathRules(appRoot: string): Promise { return buildPathRules({gitIgnoredPaths: listing.status === 'listed' ? listing.paths : []}) } -describe.sequential('app root discovery', () => { - test('walks up from explicit and current subdirectories and accepts an explicit TOML', async () => { - const root = await makeDirectory() - const routes = join(root, 'app', 'routes') - const toml = join(root, 'shopify.app.staging.toml') - await mkdir(routes, {recursive: true}) - await writeFile(toml, appConfiguration) - - const normalizedRoot = normalizePath(root) - expect(findAppRoot(routes)).toBe(normalizedRoot) - expect(findAppRoot(toml)).toBe(normalizedRoot) - - const previousInitialDirectory = process.env.INIT_CWD - process.env.INIT_CWD = routes - try { - expect(findAppRoot()).toBe(normalizedRoot) - } finally { - if (previousInitialDirectory === undefined) delete process.env.INIT_CWD - else process.env.INIT_CWD = previousInitialDirectory - } - }) - - test('fails clearly for an explicit missing path instead of scanning cwd', async () => { - const root = await makeDirectory() - const missing = join(root, 'missing-app') - expect(() => findAppRoot(missing)).toThrow(AppRootDiscoveryError) - expect(() => findAppRoot(missing)).toThrow(`App path does not exist: ${missing}`) - }) - - test('ignores files that match the glob but are not CLI app configuration names', async () => { - const root = await makeDirectory() - await writeFile(join(root, 'shopify.application.toml'), appConfiguration) - await writeFile(join(root, 'shopify.app.foo.bar.toml'), appConfiguration) - expect(() => findAppRoot(root)).toThrow(AppRootDiscoveryError) - }) - - test('rejects an explicit non-app TOML path with a configuration-file error', async () => { - const root = await makeDirectory() - const webToml = join(root, 'shopify.web.toml') - await writeFile(webToml, 'type = "frontend"\n') - expect(() => findAppRoot(webToml)).toThrow(AppRootDiscoveryError) - expect(() => findAppRoot(webToml)).toThrow( - `App path is not a directory or Shopify app configuration file: ${webToml}`, - ) - }) -}) - describe('repository discovery exclusions', () => { let restoreGitConfig: (() => void) | undefined beforeEach(() => { diff --git a/packages/app/src/cli/services/app-security-engine/tests/scan-artifact.test.ts b/packages/app/src/cli/services/app-security-engine/tests/scan-artifact.test.ts index 96cadc4d43c..769c99bc543 100644 --- a/packages/app/src/cli/services/app-security-engine/tests/scan-artifact.test.ts +++ b/packages/app/src/cli/services/app-security-engine/tests/scan-artifact.test.ts @@ -1,8 +1,8 @@ +import {scanDirectory as scan} from './scan-directory.js' import {formatJson} from '../output/format.js' import {combineFindings} from '../results/combine.js' import {translateFindingsDocument} from '../results/translate.js' import {RULE_CATALOG} from '../rules/catalog.js' -import {scan} from '../scanners/index.js' import {buildDeterministicFindings} from '../scan-artifact/index.js' import {inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' diff --git a/packages/app/src/cli/services/app-security-engine/tests/scan-contract.test.ts b/packages/app/src/cli/services/app-security-engine/tests/scan-contract.test.ts index c0f7888e678..b8de9b58a09 100644 --- a/packages/app/src/cli/services/app-security-engine/tests/scan-contract.test.ts +++ b/packages/app/src/cli/services/app-security-engine/tests/scan-contract.test.ts @@ -1,7 +1,8 @@ /* eslint-disable no-restricted-imports -- detector coverage uses real temporary repositories */ +import {scanDirectory as scan} from './scan-directory.js' import {buildAgentChecks} from '../checks/index.js' import {assertRegistryInvariants, getRegistry} from '../registry/index.js' -import {DETERMINISTIC_CHECKS, scan} from '../scanners/index.js' +import {DETERMINISTIC_CHECKS} from '../scanners/index.js' import {RULE_CATALOG} from '../rules/catalog.js' import {afterEach, describe, expect, test} from 'vitest' import {mkdir, mkdtemp, rm, writeFile} from 'node:fs/promises' diff --git a/packages/app/src/cli/services/app-security-engine/tests/scan-directory.ts b/packages/app/src/cli/services/app-security-engine/tests/scan-directory.ts new file mode 100644 index 00000000000..f2370a28f90 --- /dev/null +++ b/packages/app/src/cli/services/app-security-engine/tests/scan-directory.ts @@ -0,0 +1,18 @@ +import {scanApp} from '../run.js' +import {scan} from '../scanners/index.js' +import {getAppConfigurationFileName} from '../../../models/app/config-file-naming.js' +import {joinPath} from '@shopify/cli-kit/node/path' +import type {ScanInput, ScanOptions} from '../types.js' + +/** Resolves a test app the way the CLI would: the named configuration file in the directory is the selected TOML. */ +function scanInputFor(appDirectory: string, configName?: string): ScanInput { + return {appDirectory, appConfigFilePath: joinPath(appDirectory, getAppConfigurationFileName(configName))} +} + +export function scanDirectory(appDirectory: string, configName?: string, options?: ScanOptions) { + return scan(scanInputFor(appDirectory, configName), options) +} + +export function scanAppDirectory(appDirectory: string, configName?: string, options?: ScanOptions) { + return scanApp(scanInputFor(appDirectory, configName), options) +} diff --git a/packages/app/src/cli/services/app-security-engine/tests/secret-safety.test.ts b/packages/app/src/cli/services/app-security-engine/tests/secret-safety.test.ts index d538311c573..906b637c0f6 100644 --- a/packages/app/src/cli/services/app-security-engine/tests/secret-safety.test.ts +++ b/packages/app/src/cli/services/app-security-engine/tests/secret-safety.test.ts @@ -1,6 +1,6 @@ /* eslint-disable id-length, line-comment-position, no-restricted-imports -- security fixtures exercise raw git and filesystem behavior */ import {git, isolateGitConfig} from './git-test-helpers.js' -import {scan} from '../scanners/index.js' +import {scanDirectory as scan} from './scan-directory.js' import { SHOPIFY_SECRET_PATTERNS, redactMatch, diff --git a/packages/app/src/cli/services/app-security-engine/types.ts b/packages/app/src/cli/services/app-security-engine/types.ts index ed64c6410cf..fc14e2784d9 100644 --- a/packages/app/src/cli/services/app-security-engine/types.ts +++ b/packages/app/src/cli/services/app-security-engine/types.ts @@ -73,6 +73,14 @@ export interface ProjectDetection { languages: DetectedLanguage[] } +/** What to scan, already resolved: the engine doesn't look for an app directory or choose a configuration. */ +export interface ScanInput { + appDirectory: string + /** Absolute path of the selected app configuration file. Absent when scanning without app configuration. */ + appConfigFilePath?: string + clientId?: string +} + export interface ScanOptions { ignorePatterns?: ReadonlyArray } 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 5c4df8575ca..23ad2e84572 100644 --- a/packages/app/src/cli/services/app-security-instructions.test.ts +++ b/packages/app/src/cli/services/app-security-instructions.test.ts @@ -1,7 +1,9 @@ -import deliverAppSecurityInstructions, {appSecurityInstructions, shellQuote} from './app-security-instructions.js' -import {quoteShellArgument, type AppSecurityShell} from './app-security-commands.js' +import deliverAppSecurityInstructions, { + appSecurityInstructions as instructionsFor, + shellQuote, +} from './app-security-instructions.js' +import {quoteShellArgument, resolveAppSecurityCommands, type AppSecurityShell} from './app-security-commands.js' import {getAgentInstructions} from './app-security-engine/index.js' -import {AbortError} from '@shopify/cli-kit/node/error' import {inTemporaryDirectory, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs' import {joinPath, normalizePath} from '@shopify/cli-kit/node/path' import {describe, expect, test, vi} from 'vitest' @@ -22,6 +24,21 @@ async function createApp(directory: string): Promise { return normalizePath(directory) } +function commandsFor(appRoot: string, configFileName = 'shopify.app.toml') { + return resolveAppSecurityCommands(appRoot, configFileName) +} + +/** The instructions for an app directory whose selected TOML is `shopify.app.toml`, unless `configFileName` says otherwise. */ +function appSecurityInstructions(options: { + directory: string + scanComplete: boolean + configFileName?: string + shell?: AppSecurityShell +}): string { + const {directory, configFileName, ...rest} = options + return instructionsFor({...rest, appDirectory: directory, commands: commandsFor(directory, configFileName)}) +} + function artifactPath(appRoot: string, name: string): string { return joinPath(appRoot, '.shopify', 'app-security', name) } @@ -83,7 +100,7 @@ describe('appSecurityInstructions', () => { const instructions = appSecurityInstructions({ directory: appRoot, scanComplete: false, - configName: 'staging', + configFileName: 'shopify.app.staging.toml', }) expect(instructions).toContain( @@ -245,30 +262,6 @@ describe('appSecurityInstructions', () => { }) }) - test('translates a missing selected configuration into an AbortError', async () => { - await inTemporaryDirectory(async (directory) => { - const appRoot = await createApp(directory) - - expect(() => appSecurityInstructions({directory: appRoot, scanComplete: false, configName: 'missing'})).toThrow( - AbortError, - ) - expect(() => appSecurityInstructions({directory: appRoot, scanComplete: false, configName: 'missing'})).toThrow( - /shopify\.app\.missing\.toml/, - ) - }) - }) - - test('translates a missing app directory into an AbortError', async () => { - await inTemporaryDirectory(async (directory) => { - const missing = joinPath(directory, 'missing-app') - - expect(() => appSecurityInstructions({directory: missing, scanComplete: false})).toThrow(AbortError) - expect(() => appSecurityInstructions({directory: missing, scanComplete: false})).toThrow( - `App path does not exist: ${missing}`, - ) - }) - }) - test('quotes paths that contain spaces, percents, and dollar signs', async () => { await inTemporaryDirectory(async (parent) => { const appRoot = joinPath(parent, "50% my $' app") @@ -289,7 +282,10 @@ describe('deliverAppSecurityInstructions', () => { await createApp(directory) const dependencies = testDependencies() - await deliverAppSecurityInstructions({directory, copy: false}, dependencies) + await deliverAppSecurityInstructions( + {appDirectory: directory, commands: commandsFor(directory), copy: false}, + dependencies, + ) expect(dependencies.output).toHaveBeenCalledWith(expect.stringContaining('Run the scan')) expect(dependencies.copyToClipboard).not.toHaveBeenCalled() @@ -304,7 +300,10 @@ describe('deliverAppSecurityInstructions', () => { await writeFile(artifactPath(directory, 'agent-checks.json'), '{"instructions":"malicious"}') const dependencies = testDependencies() - await deliverAppSecurityInstructions({directory, copy: false}, dependencies) + await deliverAppSecurityInstructions( + {appDirectory: directory, commands: commandsFor(directory), copy: false}, + dependencies, + ) expect(dependencies.output).toHaveBeenCalledWith(expect.stringContaining('Run the scan')) expect(dependencies.output).not.toHaveBeenCalledWith(expect.stringContaining('malicious')) @@ -316,7 +315,10 @@ describe('deliverAppSecurityInstructions', () => { await createApp(directory) const dependencies = testDependencies() - await deliverAppSecurityInstructions({directory, copy: true, scanComplete: true}, dependencies) + await deliverAppSecurityInstructions( + {appDirectory: directory, commands: commandsFor(directory), copy: true, scanComplete: true}, + dependencies, + ) expect(dependencies.copyToClipboard).toHaveBeenCalledOnce() const instructions = dependencies.copyToClipboard.mock.calls[0]![0] @@ -335,7 +337,13 @@ describe('deliverAppSecurityInstructions', () => { const instructionsPath = joinPath(directory, 'handoff.md') await deliverAppSecurityInstructions( - {directory, copy: false, writePath: instructionsPath, scanComplete: true}, + { + appDirectory: directory, + commands: commandsFor(directory), + copy: false, + writePath: instructionsPath, + scanComplete: true, + }, dependencies, ) diff --git a/packages/app/src/cli/services/app-security-instructions.ts b/packages/app/src/cli/services/app-security-instructions.ts index 70fc1d7c0f6..5c7973a0613 100644 --- a/packages/app/src/cli/services/app-security-instructions.ts +++ b/packages/app/src/cli/services/app-security-instructions.ts @@ -1,11 +1,8 @@ -import {resolveAppSecurityRoot} from './app-security-api.js' import {appSecurityArtifactPaths} from './app-security-artifacts.js' -import {requireSecurityConfigFileName} from './app-security-config.js' import { formatAppSecurityCommand, formatAppSecurityInlineStdinCommand, quoteShellArgument, - resolveAppSecurityCommands, shellForPlatform, type AppSecurityCommand, type AppSecurityCommands, @@ -14,7 +11,6 @@ import { import {getAgentInstructions} from './app-security-engine/index.js' import {writeFile} from '@shopify/cli-kit/node/fs' import {outputResult} from '@shopify/cli-kit/node/output' -import {resolvePath} from '@shopify/cli-kit/node/path' import {renderSuccess} from '@shopify/cli-kit/node/ui' import clipboard from 'clipboardy' @@ -87,22 +83,18 @@ ${fileCommand}${encodingNote}` } function instructionPaths( - directory: string, + appDirectory: string, shell: AppSecurityShell, - commands?: AppSecurityCommands, - configName?: string, + commands: AppSecurityCommands, ): AppSecurityInstructionPaths { - const appRoot = resolveAppSecurityRoot(resolvePath(directory)) const {artifactDirectory, deterministicFindingsPath, agentChecksPath, agentFindingsPath} = - appSecurityArtifactPaths(appRoot) - const resolvedCommands = - commands ?? resolveAppSecurityCommands(appRoot, requireSecurityConfigFileName(appRoot, configName)) + appSecurityArtifactPaths(appDirectory) return { - commands: resolvedCommands, - scanCommand: formatAppSecurityCommand(resolvedCommands.scan, shell), - recordInstructions: recordInstructions(resolvedCommands.record, shell), - reviewCommand: formatAppSecurityCommand(resolvedCommands.review, shell), - cleanCommand: formatAppSecurityCommand(resolvedCommands.clean, shell), + commands, + scanCommand: formatAppSecurityCommand(commands.scan, shell), + recordInstructions: recordInstructions(commands.record, shell), + reviewCommand: formatAppSecurityCommand(commands.review, shell), + cleanCommand: formatAppSecurityCommand(commands.clean, shell), deterministicFindingsPath, agentChecksPath, agentFindingsPath, @@ -139,12 +131,11 @@ function fillTemplate(template: string, values: {[placeholder: string]: string}) } interface AppSecurityInstructionsOptions { - directory: string + appDirectory: string copy: boolean writePath?: string scanComplete?: boolean - commands?: AppSecurityCommands - configName?: string + commands: AppSecurityCommands } interface AppSecurityInstructionsDependencies { @@ -164,14 +155,13 @@ const defaultDependencies: AppSecurityInstructionsDependencies = { } export function appSecurityInstructions(options: { - directory: string + appDirectory: string scanComplete: boolean - commands?: AppSecurityCommands - configName?: string + commands: AppSecurityCommands shell?: AppSecurityShell }): string { const shell = options.shell ?? shellForPlatform() - const paths = instructionPaths(options.directory, shell, options.commands, options.configName) + const paths = instructionPaths(options.appDirectory, shell, options.commands) const scanContext = options.scanComplete ? completedScanInstructions(paths) : initialScanInstructions(paths) // Fill the scan context first: it may contain the other placeholders. return fillTemplate(getAgentInstructions(), { @@ -191,10 +181,9 @@ export default async function deliverAppSecurityInstructions( dependencies: AppSecurityInstructionsDependencies = defaultDependencies, ): Promise { const instructions = appSecurityInstructions({ - directory: options.directory, + appDirectory: options.appDirectory, scanComplete: options.scanComplete ?? false, commands: options.commands, - configName: options.configName, }) if (options.copy) { 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 61d95fbca69..82ede92e1f2 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 @@ -4,6 +4,13 @@ "version": "1.2.3", "ruleset": "2026.08.28" }, + "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"}] + }, "deterministic_findings": { "schema_version": 1, "source": "deterministic", diff --git a/packages/app/src/cli/services/app-security-json.test.ts b/packages/app/src/cli/services/app-security-json.test.ts index 606762082f8..83884209bc9 100644 --- a/packages/app/src/cli/services/app-security-json.test.ts +++ b/packages/app/src/cli/services/app-security-json.test.ts @@ -3,6 +3,7 @@ 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)) @@ -24,12 +25,58 @@ const deterministicFindings: DeterministicFindingsDocument = { 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 deterministic findings document, and the agent checks path', async () => { + test('encodes the engine, the selection, the deterministic findings document, and the agent checks path', async () => { const encoded = encodeSecurityJson( - toSecurityJson({engine, deterministicFindings}, '/tmp/app/.shopify/app-security/agent-checks.json'), + 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-results.test.ts b/packages/app/src/cli/services/app-security-results.test.ts index a9921946b35..14adf3145fa 100644 --- a/packages/app/src/cli/services/app-security-results.test.ts +++ b/packages/app/src/cli/services/app-security-results.test.ts @@ -1,7 +1,8 @@ import {loadAppSecurityResults} from './app-security-results.js' import {appSecurityArtifactPaths, writeAgentFindings, writeCheckArtifacts} from './app-security-artifacts.js' import {formatAppSecurityCommand, resolveAppSecurityCommands} from './app-security-commands.js' -import {combineFindings, scanApp} from './app-security-engine/index.js' +import {combineFindings} from './app-security-engine/index.js' +import {scanAppDirectory} from './app-security-engine/tests/scan-directory.js' import {agentFindingsDocument} from './app-security-engine/tests/fixtures/findings-documents.js' import {AbortError, handler} from '@shopify/cli-kit/node/error' import {inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs' @@ -19,7 +20,7 @@ async function createApp(directory: string): Promise { /** Writes a real deterministic-findings.json by running `check` on the app. */ async function writeDeterministicFindings(appRoot: string): Promise { - const {deterministicFindings, agentChecks} = await scanApp(appRoot) + const {deterministicFindings, agentChecks} = await scanAppDirectory(appRoot) await writeCheckArtifacts(appRoot, {deterministicFindings, agentChecks}) return deterministicFindings } diff --git a/packages/app/src/cli/services/app-security-selection.test-data.ts b/packages/app/src/cli/services/app-security-selection.test-data.ts new file mode 100644 index 00000000000..3e9e218a093 --- /dev/null +++ b/packages/app/src/cli/services/app-security-selection.test-data.ts @@ -0,0 +1,14 @@ +/** The smallest app configuration that the CLI's loader accepts. An empty `clientId` is an unlinked configuration. */ +export function validAppConfiguration(clientId = 'test-client-id'): string { + return `name = "Test app" +client_id = "${clientId}" +application_url = "https://example.com" +embedded = true + +[webhooks] +api_version = "2025-01" + +[auth] +redirect_urls = ["https://example.com/auth/callback"] +` +} diff --git a/packages/app/src/cli/services/app-security-selection.test.ts b/packages/app/src/cli/services/app-security-selection.test.ts new file mode 100644 index 00000000000..8dd3bef2f5b --- /dev/null +++ b/packages/app/src/cli/services/app-security-selection.test.ts @@ -0,0 +1,413 @@ +import { + clientIdSource, + effectiveClientId, + resolveAppSecuritySelection, + selectedConfigFileName, + type AppSecuritySelection, + type AppSecuritySelectionDependencies, +} from './app-security-selection.js' +import {validAppConfiguration} from './app-security-selection.test-data.js' +import {getCachedAppInfo} from './local-storage.js' +import {appCreationDefaults} from './app/config/link.js' +import {fetchOrCreateOrganizationApp} from './context.js' +import {AbortError} 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' +import {beforeEach, describe, expect, test, vi} from 'vitest' +import {symlink} from 'node:fs/promises' +import type {OrganizationApp} from '../models/organization.js' + +vi.mock('./local-storage.js', async (importOriginal) => ({ + ...(await importOriginal()), + getCachedAppInfo: vi.fn(), +})) +vi.mock('./context.js', async (importOriginal) => ({ + ...(await importOriginal()), + fetchOrCreateOrganizationApp: vi.fn(), +})) +vi.mock('@shopify/cli-kit/node/ui', async (importOriginal) => ({ + ...(await importOriginal()), + renderConfirmationPrompt: vi.fn(), +})) + +beforeEach(() => { + vi.mocked(getCachedAppInfo).mockReturnValue(undefined) +}) + +async function writeConfiguration(directory: string, clientId: string, name = 'shopify.app.toml'): Promise { + await writeFile(joinPath(directory, name), validAppConfiguration(clientId)) +} + +function promptDependencies(overrides: Partial = {}) { + return { + confirmScanWithoutAppConfig: vi.fn(async (_directory: string) => true), + pickClientId: vi.fn(async (_appDirectory: string) => 'picked-client-id'), + ...overrides, + } +} + +async function selectionError(promise: Promise): Promise { + const error: unknown = await promise.catch((caught: unknown) => caught) + expect(error).toBeInstanceOf(AbortError) + return error as AbortError +} + +describe('resolveAppSecuritySelection with an app configuration', () => { + test('selects the default TOML', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, 'default-client-id') + const appDirectory = await fileRealPath(directory) + + const selection = await resolveAppSecuritySelection({path: directory, allowPrompts: false}) + + expect(selection).toEqual({ + kind: 'config', + appDirectory, + appConfigFilePath: joinPath(appDirectory, 'shopify.app.toml'), + configClientId: 'default-client-id', + clientIdOverride: undefined, + }) + }) + }) + + test('walks up from a subdirectory to the directory holding the TOML', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, 'default-client-id') + const subdirectory = joinPath(directory, 'app', 'routes') + await mkdir(subdirectory) + + const selection = await resolveAppSecuritySelection({path: subdirectory, allowPrompts: false}) + + expect(selection).toMatchObject({kind: 'config', appDirectory: await fileRealPath(directory)}) + }) + }) + + test('aborts when --path does not exist instead of walking up to the app above it', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, 'default-client-id') + const missing = joinPath(directory, 'missing-app') + + const error = await selectionError(resolveAppSecuritySelection({path: missing, allowPrompts: false})) + + expect(error.message).toBe(`--path ${missing}: not a directory.`) + }) + }) + + test('roots the TOML path on the real app directory when --path goes through a symbolic link', async () => { + await inTemporaryDirectory(async (directory) => { + const appDirectory = joinPath(directory, 'real-app') + await mkdir(appDirectory) + await writeConfiguration(appDirectory, 'default-client-id') + const link = joinPath(directory, 'linked-app') + await symlink(appDirectory, link, 'dir') + const realAppDirectory = await fileRealPath(appDirectory) + + const selection = await resolveAppSecuritySelection({path: link, allowPrompts: false}) + + expect(selection).toMatchObject({ + appDirectory: realAppDirectory, + appConfigFilePath: joinPath(realAppDirectory, 'shopify.app.toml'), + }) + }) + }) + + test('selects the TOML named by --config', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, 'default-client-id') + await writeConfiguration(directory, 'staging-client-id', 'shopify.app.staging.toml') + + const selection = await resolveAppSecuritySelection({path: directory, config: 'staging', allowPrompts: false}) + + expect(selection).toMatchObject({ + kind: 'config', + appConfigFilePath: joinPath(await fileRealPath(directory), 'shopify.app.staging.toml'), + configClientId: 'staging-client-id', + }) + }) + }) + + test('selects the TOML cached by `app config use`', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, 'default-client-id') + await writeConfiguration(directory, 'production-client-id', 'shopify.app.production.toml') + vi.mocked(getCachedAppInfo).mockReturnValue({directory, configFile: 'shopify.app.production.toml'}) + + const selection = await resolveAppSecuritySelection({path: directory, allowPrompts: false}) + + expect(selection).toMatchObject({ + kind: 'config', + appConfigFilePath: joinPath(await fileRealPath(directory), 'shopify.app.production.toml'), + configClientId: 'production-client-id', + }) + }) + }) + + test('keeps the TOML and records --client-id as an override of the TOML client ID', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, 'toml-client-id') + + const selection = await resolveAppSecuritySelection({ + path: directory, + clientId: 'flag-client-id', + allowPrompts: false, + }) + + expect(selection).toMatchObject({ + kind: 'config', + configClientId: 'toml-client-id', + clientIdOverride: 'flag-client-id', + }) + expect(effectiveClientId(selection)).toBe('flag-client-id') + expect(clientIdSource(selection)).toBe('flag') + }) + }) + + test('reports a TOML with an empty client ID as not linked', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, '') + + const selection = await resolveAppSecuritySelection({path: directory, allowPrompts: false}) + + expect(selection).toMatchObject({kind: 'config', configClientId: undefined}) + expect(effectiveClientId(selection)).toBeUndefined() + expect(clientIdSource(selection)).toBeUndefined() + }) + }) + + test('aborts on an invalid TOML without offering to scan without app configuration', async () => { + await inTemporaryDirectory(async (directory) => { + await writeFile(joinPath(directory, 'shopify.app.toml'), 'name = [') + const dependencies = promptDependencies() + + const error = await selectionError( + resolveAppSecuritySelection({path: directory, allowPrompts: true}, dependencies), + ) + + expect(error.message).not.toContain('No app configuration found') + expect(dependencies.confirmScanWithoutAppConfig).not.toHaveBeenCalled() + }) + }) + + test('aborts when --config names a TOML that does not exist', async () => { + await inTemporaryDirectory(async (directory) => { + await writeConfiguration(directory, 'default-client-id') + const dependencies = promptDependencies() + + const error = await selectionError( + resolveAppSecuritySelection({path: directory, config: 'missing', allowPrompts: false}, dependencies), + ) + + expect(error.message).not.toContain('No app configuration found') + expect(dependencies.confirmScanWithoutAppConfig).not.toHaveBeenCalled() + }) + }) +}) + +describe('resolveAppSecuritySelection with --without-app-config', () => { + test('aborts without --client-id', async () => { + await inTemporaryDirectory(async (directory) => { + const error = await selectionError( + resolveAppSecuritySelection({path: directory, withoutAppConfig: true, allowPrompts: false}), + ) + + expect(error.message).toBe('--without-app-config requires --client-id.') + }) + }) + + test('scans the real path of --path inside an app without walking up or reading its TOML', async () => { + await inTemporaryDirectory(async (directory) => { + await writeFile(joinPath(directory, 'shopify.app.toml'), 'name = [') + const subdirectory = joinPath(directory, 'packages', 'worker') + await mkdir(subdirectory) + + const selection = await resolveAppSecuritySelection({ + path: subdirectory, + clientId: 'flag-client-id', + withoutAppConfig: true, + allowPrompts: false, + }) + + expect(selection).toEqual({ + kind: 'no-config', + appDirectory: await fileRealPath(subdirectory), + clientId: 'flag-client-id', + clientIdSource: 'flag', + }) + expect(selectedConfigFileName(selection)).toBeUndefined() + }) + }) + + test('scans a directory outside any app', async () => { + await inTemporaryDirectory(async (directory) => { + const selection = await resolveAppSecuritySelection({ + path: directory, + clientId: 'flag-client-id', + withoutAppConfig: true, + allowPrompts: true, + }) + + expect(selection).toEqual({ + kind: 'no-config', + appDirectory: await fileRealPath(directory), + clientId: 'flag-client-id', + clientIdSource: 'flag', + }) + }) + }) + + test('uses the real path when --path is a symbolic link to a directory', async () => { + await inTemporaryDirectory(async (directory) => { + const target = joinPath(directory, 'target') + await mkdir(target) + const link = joinPath(directory, 'link') + await symlink(target, link, 'dir') + + const selection = await resolveAppSecuritySelection({ + path: link, + clientId: 'flag-client-id', + withoutAppConfig: true, + allowPrompts: false, + }) + + expect(selection.appDirectory).toBe(await fileRealPath(target)) + }) + }) + + test('aborts when --path is a file', async () => { + await inTemporaryDirectory(async (directory) => { + const file = joinPath(directory, 'file.txt') + await writeFile(file, 'text') + + const error = await selectionError( + resolveAppSecuritySelection({path: file, clientId: 'abc', withoutAppConfig: true, allowPrompts: false}), + ) + + expect(error.message).toBe(`--path ${file}: not a directory.`) + }) + }) + + test('aborts when --path does not exist', async () => { + await inTemporaryDirectory(async (directory) => { + const missing = joinPath(directory, 'missing') + + const error = await selectionError( + resolveAppSecuritySelection({path: missing, clientId: 'abc', withoutAppConfig: true, allowPrompts: false}), + ) + + expect(error.message).toBe(`--path ${missing}: not a directory.`) + }) + }) +}) + +describe('resolveAppSecuritySelection when no TOML is found', () => { + test('aborts with the next step when prompts are not allowed', async () => { + await inTemporaryDirectory(async (directory) => { + const dependencies = promptDependencies() + + const error = await selectionError( + resolveAppSecuritySelection({path: directory, allowPrompts: false}, dependencies), + ) + + expect(error.message).toBe(`No app configuration found at or above ${directory}.`) + expect(error.tryMessage).toBe( + 'Pass `--path` to your app directory, or scan without app configuration with `--without-app-config --client-id `.', + ) + expect(dependencies.confirmScanWithoutAppConfig).not.toHaveBeenCalled() + }) + }) + + test('aborts with the same error 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), + ) + + expect(error.message).toBe(`No app configuration found at or above ${directory}.`) + expect(dependencies.confirmScanWithoutAppConfig).toHaveBeenCalledWith(directory) + expect(dependencies.pickClientId).not.toHaveBeenCalled() + }) + }) + + test('scans without app configuration with the --client-id flag when the prompt is answered yes', async () => { + await inTemporaryDirectory(async (directory) => { + const dependencies = promptDependencies() + + const selection = await resolveAppSecuritySelection( + {path: directory, clientId: 'flag-client-id', allowPrompts: true}, + dependencies, + ) + + expect(selection).toEqual({ + kind: 'no-config', + appDirectory: await fileRealPath(directory), + clientId: 'flag-client-id', + clientIdSource: 'flag', + }) + expect(dependencies.pickClientId).not.toHaveBeenCalled() + }) + }) + + test('takes the client ID from the picker when the prompt is answered yes without --client-id', async () => { + await inTemporaryDirectory(async (directory) => { + const dependencies = promptDependencies() + + const selection = await resolveAppSecuritySelection({path: directory, allowPrompts: true}, dependencies) + + expect(selection).toEqual({ + kind: 'no-config', + appDirectory: await fileRealPath(directory), + clientId: 'picked-client-id', + clientIdSource: 'picker', + }) + expect(dependencies.pickClientId).toHaveBeenCalledWith(await fileRealPath(directory)) + }) + }) + + test('asks the no-TOML question and picks the app with the link command defaults', async () => { + await inTemporaryDirectory(async (directory) => { + vi.mocked(renderConfirmationPrompt).mockResolvedValue(true) + vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue({apiKey: 'created-client-id'} as OrganizationApp) + + const selection = await resolveAppSecuritySelection({path: directory, allowPrompts: true}) + + expect(renderConfirmationPrompt).toHaveBeenCalledWith({ + message: `No app configuration found at or above ${directory}. Scan it without app configuration? Config checks will be skipped.`, + confirmationMessage: 'Yes, scan without app configuration', + cancellationMessage: 'No', + defaultValue: false, + }) + expect(fetchOrCreateOrganizationApp).toHaveBeenCalledWith(appCreationDefaults(await fileRealPath(directory))) + expect(selection).toMatchObject({kind: 'no-config', clientId: 'created-client-id', clientIdSource: 'picker'}) + }) + }) +}) + +describe('selection helpers', () => { + test('shows the client ID of a no-config selection', () => { + const selection: AppSecuritySelection = { + kind: 'no-config', + appDirectory: '/app', + clientId: 'abc', + clientIdSource: 'picker', + } + + expect(effectiveClientId(selection)).toBe('abc') + expect(clientIdSource(selection)).toBe('picker') + }) + + test('falls back to the TOML client ID when there is no override', () => { + const selection: AppSecuritySelection = { + kind: 'config', + appDirectory: '/app', + appConfigFilePath: '/app/shopify.app.staging.toml', + configClientId: 'toml-id', + } + + expect(effectiveClientId(selection)).toBe('toml-id') + expect(clientIdSource(selection)).toBe('config') + expect(selectedConfigFileName(selection)).toBe('shopify.app.staging.toml') + }) +}) diff --git a/packages/app/src/cli/services/app-security-selection.ts b/packages/app/src/cli/services/app-security-selection.ts new file mode 100644 index 00000000000..5982870a9ea --- /dev/null +++ b/packages/app/src/cli/services/app-security-selection.ts @@ -0,0 +1,148 @@ +import {localAppContext} from './app-context.js' +import {appCreationDefaults} from './app/config/link.js' +import {fetchOrCreateOrganizationApp} from './context.js' +import {NoAppConfigurationFoundError} from '../models/project/project.js' +import {AbortError} from '@shopify/cli-kit/node/error' +import {fileRealPath, isDirectory} from '@shopify/cli-kit/node/fs' +import {basename, joinPath} from '@shopify/cli-kit/node/path' +import {renderConfirmationPrompt} from '@shopify/cli-kit/node/ui' + +export type AppSecuritySelection = + | { + kind: 'config' + /** Absolute; the real path. */ + appDirectory: string + /** Absolute path of the selected TOML, as found (not the symbolic link's target). */ + appConfigFilePath: string + /** The TOML's `client_id`, if any. */ + configClientId?: string + /** The `--client-id` value, if passed. */ + clientIdOverride?: string + } + | { + kind: 'no-config' + /** Absolute; the real path of `--path`. */ + appDirectory: string + clientId: string + clientIdSource: 'flag' | 'picker' + } + +/** A directory that is walked. Until `--include-dir` exists, the app directory is the only one. */ +export interface AppSecurityScanDirectory { + directory: string + origin: 'app_directory' | 'include_dir' +} + +interface AppSecuritySelectionOptions { + path: string + config?: string + clientId?: string + withoutAppConfig?: boolean + allowPrompts: boolean +} + +export interface AppSecuritySelectionDependencies { + confirmScanWithoutAppConfig(directory: string): Promise + pickClientId(appDirectory: string): Promise +} + +const defaultDependencies: AppSecuritySelectionDependencies = { + confirmScanWithoutAppConfig: (directory) => + renderConfirmationPrompt({ + message: `No app configuration found at or above ${directory}. Scan it without app configuration? Config checks will be skipped.`, + confirmationMessage: 'Yes, scan without app configuration', + cancellationMessage: 'No', + defaultValue: false, + }), + pickClientId: async (appDirectory) => (await fetchOrCreateOrganizationApp(appCreationDefaults(appDirectory))).apiKey, +} + +/** The name of the selected TOML, or undefined when there is none. */ +export function selectedConfigFileName(selection: AppSecuritySelection): string | undefined { + return selection.kind === 'config' ? basename(selection.appConfigFilePath) : undefined +} + +/** The client ID to show people: `undefined` means the selected TOML isn't linked. */ +export function effectiveClientId(selection: AppSecuritySelection): string | undefined { + return selection.kind === 'no-config' ? selection.clientId : (selection.clientIdOverride ?? selection.configClientId) +} + +export function clientIdSource(selection: AppSecuritySelection): 'config' | 'flag' | 'picker' | undefined { + if (selection.kind === 'no-config') return selection.clientIdSource + if (selection.clientIdOverride) return 'flag' + return selection.configClientId ? 'config' : undefined +} + +export async function resolveAppSecuritySelection( + options: AppSecuritySelectionOptions, + dependencies: AppSecuritySelectionDependencies = defaultDependencies, +): Promise { + if (options.withoutAppConfig) { + if (!options.clientId) throw new AbortError('--without-app-config requires --client-id.') + return { + kind: 'no-config', + appDirectory: await realDirectory(options.path), + clientId: options.clientId, + clientIdSource: 'flag', + } + } + + // Walking up from a missing directory would find, and scan, the app above it. + await realDirectory(options.path) + + try { + const {app} = await localAppContext({ + directory: options.path, + userProvidedConfigName: options.config, + skipPrompts: !options.allowPrompts, + }) + const appDirectory = await fileRealPath(app.directory) + return { + kind: 'config', + appDirectory, + // Re-rooted on the real app directory so the engine's containment check compares like with like. + appConfigFilePath: joinPath(appDirectory, basename(app.configPath)), + configClientId: app.configuration.client_id || undefined, + clientIdOverride: options.clientId, + } + } catch (error) { + if (!(error instanceof NoAppConfigurationFoundError)) throw error + return resolveWithoutAppConfigurationFile(options, dependencies) + } +} + +async function resolveWithoutAppConfigurationFile( + options: AppSecuritySelectionOptions, + dependencies: AppSecuritySelectionDependencies, +): Promise { + if (!options.allowPrompts) abortNoAppConfigurationFound(options.path) + + const appDirectory = await realDirectory(options.path) + if (!(await dependencies.confirmScanWithoutAppConfig(options.path))) abortNoAppConfigurationFound(options.path) + + if (options.clientId) return {kind: 'no-config', appDirectory, clientId: options.clientId, clientIdSource: 'flag'} + return { + kind: 'no-config', + appDirectory, + clientId: await dependencies.pickClientId(appDirectory), + clientIdSource: 'picker', + } +} + +function abortNoAppConfigurationFound(directory: string): never { + throw new AbortError( + `No app configuration found at or above ${directory}.`, + 'Pass `--path` to your app directory, or scan without app configuration with `--without-app-config --client-id `.', + ) +} + +async function realDirectory(path: string): Promise { + const realPath = await realPathIfExists(path) + if (realPath === undefined || !(await isDirectory(realPath))) throw new AbortError(`--path ${path}: not a directory.`) + return realPath +} + +// On Windows, `fileExists` is true for a dangling symbolic link, so a successful realpath is what proves the path exists. +async function realPathIfExists(path: string): Promise { + return fileRealPath(path).catch(() => undefined) +} diff --git a/packages/app/src/cli/services/app/config/link.ts b/packages/app/src/cli/services/app/config/link.ts index 92d80fa90e1..31083764e01 100644 --- a/packages/app/src/cli/services/app/config/link.ts +++ b/packages/app/src/cli/services/app/config/link.ts @@ -168,13 +168,6 @@ async function getAppCreationDefaultsFromLocalApp(options: LinkOptions): Promise creationOptions: CreateAppOptions appDirectory?: string }> { - const appCreationDefaults = { - isLaunchable: false, - scopesArray: [] as string[], - name: '', - directory: options.directory, - isEmbedded: false, - } try { const app = await loadApp({ specifications: await loadLocalExtensionsSpecifications(), @@ -188,7 +181,17 @@ async function getAppCreationDefaultsFromLocalApp(options: LinkOptions): Promise // eslint-disable-next-line no-catch-all/no-catch-all } catch (error) { - return {creationOptions: appCreationDefaults} + return {creationOptions: appCreationDefaults(options.directory)} + } +} + +export function appCreationDefaults(directory: string): CreateAppOptions { + return { + isLaunchable: false, + scopesArray: [], + name: '', + directory, + isEmbedded: false, } } diff --git a/packages/app/src/cli/services/security-check.test.ts b/packages/app/src/cli/services/security-check.test.ts index 1e932f48120..877695d9b8b 100644 --- a/packages/app/src/cli/services/security-check.test.ts +++ b/packages/app/src/cli/services/security-check.test.ts @@ -1,18 +1,13 @@ import securityCheck, {appSecurityInstructionsPrompt} from './security-check.js' -import {resolveAppSecurityCommands} from './app-security-commands.js' +import {formatAppSecurityCommand, resolveAppSecurityCommands} from './app-security-commands.js' import {appSecurityArtifactPaths, writeCheckArtifacts} from './app-security-artifacts.js' import {inTemporaryDirectory, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs' import {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 {AgentChecks, DeterministicFindingsDocument, ScanResult} from './app-security-engine/index.js' -vi.mock('./app-security-config.js', async (importOriginal) => { - const original = await importOriginal() - // Unit tests use a fake app directory; check.integration.test.ts covers the missing-config error. - return {...original, requireSecurityConfigFileName: original.resolveSecurityConfigFileName} -}) - const scan: ScanResult = { version: '0.1.0', timestamp: '2026-08-24T00:00:00.000Z', @@ -75,7 +70,6 @@ const agentChecks: AgentChecks = { } const scanExecution: AppSecurityExecution = { - appRoot: '/tmp/unlinked-app', scan, deterministicFindings, agentChecks, @@ -88,15 +82,30 @@ const artifacts = { agentChecksPath: '/tmp/unlinked-app/.shopify/app-security/agent-checks.json', } -function testDependencies(execution: AppSecurityExecution = scanExecution) { +const appDirectory = '/tmp/unlinked-app' + +const configSelection: AppSecuritySelection = { + kind: 'config', + appDirectory, + appConfigFilePath: `${appDirectory}/shopify.app.toml`, + configClientId: 'toml-client-id', +} + +const scanDirectories = [{directory: appDirectory, origin: 'app_directory' as const}] + +function testDependencies( + execution: AppSecurityExecution = scanExecution, + selection: AppSecuritySelection = configSelection, +) { return { - resolveRoot: vi.fn(() => execution.appRoot), + resolveSelection: vi.fn(async () => selection), execute: vi.fn(async () => execution), 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(), renderReport: vi.fn(), setExitCode: vi.fn(), } @@ -104,7 +113,8 @@ function testDependencies(execution: AppSecurityExecution = scanExecution) { function testOptions() { return { - directory: '/tmp/unlinked-app', + directory: appDirectory, + withoutAppConfig: false, json: false, verbose: false, blocking: 'none' as const, @@ -120,19 +130,28 @@ describe('securityCheck', () => { await securityCheck({...testOptions(), verbose: true, blocking: 'high'}, dependencies) - expect(dependencies.resolveRoot).toHaveBeenCalledWith('/tmp/unlinked-app') + expect(dependencies.resolveSelection).toHaveBeenCalledWith({ + path: appDirectory, + config: undefined, + clientId: undefined, + withoutAppConfig: false, + allowPrompts: false, + }) expect(dependencies.execute).toHaveBeenCalledWith({ - appRoot: '/tmp/unlinked-app', - configFileName: 'shopify.app.toml', + appDirectory, + appConfigFilePath: `${appDirectory}/shopify.app.toml`, + clientId: 'toml-client-id', ignorePatterns: [], }) - expect(dependencies.writeArtifacts).toHaveBeenCalledWith('/tmp/unlinked-app', {deterministicFindings, agentChecks}) + expect(dependencies.writeArtifacts).toHaveBeenCalledWith(appDirectory, {deterministicFindings, agentChecks}) expect(dependencies.renderReport).toHaveBeenCalledWith({ scan, + selection: configSelection, + scanDirectories, engine, verbose: true, elapsedMilliseconds: 12, - commands: resolveAppSecurityCommands(scanExecution.appRoot, 'shopify.app.toml'), + commands: resolveAppSecurityCommands(appDirectory, 'shopify.app.toml'), deterministicFindingsPath: artifacts.deterministicFindingsPath, agentChecksPath: artifacts.agentChecksPath, agentCheckCount: 31, @@ -140,23 +159,32 @@ describe('securityCheck', () => { expect(dependencies.output).not.toHaveBeenCalled() }) - test('forwards configName and includes --config in generated commands', async () => { - const dependencies = testDependencies() + test('forwards the config name and includes --config in generated commands', async () => { + const dependencies = testDependencies(scanExecution, { + ...configSelection, + appConfigFilePath: `${appDirectory}/shopify.app.staging.toml`, + }) await securityCheck({...testOptions(), configName: 'staging'}, dependencies) - expect(dependencies.execute).toHaveBeenCalledWith({ - appRoot: '/tmp/unlinked-app', - configFileName: 'shopify.app.staging.toml', - ignorePatterns: [], - }) + 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: resolveAppSecurityCommands(scanExecution.appRoot, 'shopify.app.staging.toml'), - }), + expect.objectContaining({commands: resolveAppSecurityCommands(appDirectory, 'shopify.app.staging.toml')}), ) }) + 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) + + expect(dependencies.resolveSelection).toHaveBeenCalledWith(expect.objectContaining({clientId: 'flag-client-id'})) + expect(dependencies.execute).toHaveBeenCalledWith(expect.objectContaining({clientId: 'flag-client-id'})) + }) + test('forwards ignorePatterns to the scan and repeats them in generated commands and instructions', async () => { const dependencies = testDependencies() dependencies.canPrompt.mockReturnValue(true) @@ -165,13 +193,9 @@ describe('securityCheck', () => { await securityCheck({...testOptions(), ignorePatterns}, dependencies) - const commands = resolveAppSecurityCommands(scanExecution.appRoot, 'shopify.app.toml', ignorePatterns) + const commands = resolveAppSecurityCommands(appDirectory, 'shopify.app.toml', ignorePatterns) expect(commands.scan.args).toContainEqual({flag: '--ignore', value: 'generated/'}) - expect(dependencies.execute).toHaveBeenCalledWith({ - appRoot: '/tmp/unlinked-app', - configFileName: 'shopify.app.toml', - ignorePatterns, - }) + expect(dependencies.execute).toHaveBeenCalledWith(expect.objectContaining({ignorePatterns})) expect(dependencies.renderReport).toHaveBeenCalledWith(expect.objectContaining({commands})) expect(dependencies.deliverInstructions).toHaveBeenCalledWith(expect.objectContaining({commands})) }) @@ -191,7 +215,10 @@ describe('securityCheck', () => { generated_at: '2026-09-01T00:00:00.000Z', } const dependencies = { - ...testDependencies({...scanExecution, appRoot, deterministicFindings: rescanFindings}), + ...testDependencies( + {...scanExecution, deterministicFindings: rescanFindings}, + {...configSelection, appDirectory: appRoot, appConfigFilePath: `${appRoot}/shopify.app.toml`}, + ), writeArtifacts: writeCheckArtifacts, canPrompt: vi.fn(() => true), } @@ -201,7 +228,6 @@ describe('securityCheck', () => { 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.canPrompt).not.toHaveBeenCalled() expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() expect(dependencies.renderReport).toHaveBeenCalledWith( expect.objectContaining({ @@ -213,6 +239,76 @@ describe('securityCheck', () => { }) }) + 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})) + + const nonInteractive = testDependencies() + await securityCheck({...testOptions(), skipInstructions: true}, nonInteractive) + expect(nonInteractive.resolveSelection).toHaveBeenCalledWith(expect.objectContaining({allowPrompts: 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) + + await securityCheck( + {...testOptions(), withoutAppConfig: true, clientId: 'flag-client-id', skipInstructions: true}, + dependencies, + ) + + expect(dependencies.resolveSelection).toHaveBeenCalledWith( + expect.objectContaining({withoutAppConfig: true, clientId: 'flag-client-id'}), + ) + expect(dependencies.execute).toHaveBeenCalledWith({ + appDirectory, + appConfigFilePath: undefined, + clientId: 'flag-client-id', + ignorePatterns: [], + }) + expect(dependencies.renderInfo).not.toHaveBeenCalled() + expect(dependencies.renderReport).toHaveBeenCalledWith( + expect.objectContaining({commands: resolveAppSecurityCommands(appDirectory)}), + ) + }) + + 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) + + expect(dependencies.renderInfo).toHaveBeenCalledWith({ + headline: 'To skip these prompts next time, run:', + body: [{command: formatAppSecurityCommand(resolveAppSecurityCommands(appDirectory).scan)}], + }) + expect(dependencies.renderInfo.mock.invocationCallOrder[0]).toBeLessThan( + dependencies.execute.mock.invocationCallOrder[0]!, + ) + }) + + test('does not show the prompt-flow command when a TOML was found', async () => { + const dependencies = testDependencies() + + await securityCheck(testOptions(), dependencies) + + expect(dependencies.renderInfo).not.toHaveBeenCalled() + }) + test('does not write artifacts when the scan fails', async () => { const dependencies = testDependencies() dependencies.execute.mockRejectedValue(new Error('scan failed')) @@ -222,17 +318,26 @@ describe('securityCheck', () => { expect(dependencies.writeArtifacts).not.toHaveBeenCalled() }) - test('prints the engine, deterministic findings, and agent checks path as JSON', async () => { + 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() @@ -266,10 +371,10 @@ describe('securityCheck', () => { }) expect(dependencies.selectInstructionsDestination).toHaveBeenCalledOnce() expect(dependencies.deliverInstructions).toHaveBeenCalledWith({ - directory: '/tmp/unlinked-app', + appDirectory, copy: true, scanComplete: true, - commands: resolveAppSecurityCommands(scanExecution.appRoot), + commands: resolveAppSecurityCommands(appDirectory, 'shopify.app.toml'), }) }) @@ -281,10 +386,10 @@ describe('securityCheck', () => { await securityCheck(testOptions(), dependencies) expect(dependencies.deliverInstructions).toHaveBeenCalledWith({ - directory: '/tmp/unlinked-app', + appDirectory, copy: false, scanComplete: true, - commands: resolveAppSecurityCommands(scanExecution.appRoot), + commands: resolveAppSecurityCommands(appDirectory, 'shopify.app.toml'), }) }) @@ -303,13 +408,12 @@ describe('securityCheck', () => { await securityCheck({...testOptions(), yes: true}, dependencies) - expect(dependencies.canPrompt).not.toHaveBeenCalled() expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() expect(dependencies.deliverInstructions).toHaveBeenCalledWith({ - directory: '/tmp/unlinked-app', + appDirectory, copy: false, scanComplete: true, - commands: resolveAppSecurityCommands(scanExecution.appRoot), + commands: resolveAppSecurityCommands(appDirectory, 'shopify.app.toml'), }) }) @@ -319,7 +423,6 @@ describe('securityCheck', () => { await securityCheck({...testOptions(), skipInstructions: true}, dependencies) - expect(dependencies.canPrompt).not.toHaveBeenCalled() expect(dependencies.selectInstructionsDestination).not.toHaveBeenCalled() expect(dependencies.deliverInstructions).not.toHaveBeenCalled() }) diff --git a/packages/app/src/cli/services/security-check.ts b/packages/app/src/cli/services/security-check.ts index bea0acbdcf5..35fbbfa0515 100644 --- a/packages/app/src/cli/services/security-check.ts +++ b/packages/app/src/cli/services/security-check.ts @@ -1,22 +1,34 @@ -import {securityExitCode, executeAppSecurity, resolveAppSecurityRoot} from './app-security-api.js' +import {securityExitCode, executeAppSecurity} from './app-security-api.js' import {writeCheckArtifacts} from './app-security-artifacts.js' -import {requireSecurityConfigFileName} from './app-security-config.js' import deliverAppSecurityInstructions from './app-security-instructions.js' -import {resolveAppSecurityCommands, type AppSecurityCommands} from './app-security-commands.js' +import { + formatAppSecurityCommand, + resolveAppSecurityCommands, + type AppSecurityCommands, +} from './app-security-commands.js' +import { + effectiveClientId, + resolveAppSecuritySelection, + selectedConfigFileName, + type AppSecurityScanDirectory, + type AppSecuritySelection, +} from './app-security-selection.js' import {encodeSecurityJson, toSecurityJson} from './security-json.js' import {renderSecurityReport} from './security-output.js' import {outputResult} from '@shopify/cli-kit/node/output' import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system' -import {renderSelectPrompt} from '@shopify/cli-kit/node/ui' +import {renderInfo, renderSelectPrompt} from '@shopify/cli-kit/node/ui' import type {CheckArtifactPaths} from './app-security-artifacts.js' -import type {AgentChecks, DeterministicFindingsDocument} from './app-security-engine/index.js' +import type {AgentChecks, DeterministicFindingsDocument, ScanInput} from './app-security-engine/index.js' import type {AppSecurityBlockingLevel, AppSecurityExecution} from './app-security-api.js' import type {SecurityReportInput} from './security-output.js' -import type {RenderSelectPromptOptions} from '@shopify/cli-kit/node/ui' +import type {RenderAlertOptions, RenderSelectPromptOptions} from '@shopify/cli-kit/node/ui' interface SecurityOptions { directory: string configName?: string + clientId?: string + withoutAppConfig: boolean json: boolean verbose: boolean blocking: AppSecurityBlockingLevel @@ -28,12 +40,14 @@ interface SecurityOptions { export type AppSecurityInstructionsDestination = 'copy' | 'print' | 'nothing' interface SecurityDependencies { - resolveRoot(directory: string): string - execute(options: { - appRoot: string - configFileName: string - ignorePatterns: ReadonlyArray - }): Promise + resolveSelection(options: { + path: string + config?: string + clientId?: string + withoutAppConfig: boolean + allowPrompts: boolean + }): Promise + execute(options: ScanInput & {ignorePatterns: ReadonlyArray}): Promise writeArtifacts( appRoot: string, artifacts: {deterministicFindings: DeterministicFindingsDocument; agentChecks: AgentChecks}, @@ -41,12 +55,13 @@ interface SecurityDependencies { canPrompt(): boolean selectInstructionsDestination(): Promise deliverInstructions(options: { - directory: string + appDirectory: string copy: boolean scanComplete: boolean commands: AppSecurityCommands }): Promise output(content: string): void + renderInfo(options: RenderAlertOptions): void renderReport(input: SecurityReportInput): void setExitCode(exitCode: number): void } @@ -62,13 +77,14 @@ export const appSecurityInstructionsPrompt: RenderSelectPromptOptions renderSelectPrompt(appSecurityInstructionsPrompt), deliverInstructions: deliverAppSecurityInstructions, output: outputResult, + renderInfo, renderReport: renderSecurityReport, setExitCode: (exitCode) => { process.exitCode = exitCode @@ -78,10 +94,11 @@ const defaultDependencies: SecurityDependencies = { async function instructionsDestination( options: SecurityOptions, dependencies: SecurityDependencies, + canPrompt: boolean, ): Promise { if (options.json || options.skipInstructions) return 'nothing' if (options.yes) return 'print' - if (!dependencies.canPrompt()) return 'nothing' + if (!canPrompt) return 'nothing' return dependencies.selectInstructionsDestination() } @@ -90,9 +107,13 @@ function securityReportInput( artifacts: CheckArtifactPaths, verbose: boolean, commands: AppSecurityCommands, + selection: AppSecuritySelection, + scanDirectories: AppSecurityScanDirectory[], ): SecurityReportInput { return { scan: execution.scan, + selection, + scanDirectories, engine: execution.engine, verbose, elapsedMilliseconds: execution.elapsedMilliseconds, @@ -111,26 +132,50 @@ export default async function securityCheck( options: SecurityOptions, dependencies: SecurityDependencies = defaultDependencies, ): Promise { - const appRoot = dependencies.resolveRoot(options.directory) - const configFileName = requireSecurityConfigFileName(appRoot, options.configName) - const commands = resolveAppSecurityCommands(appRoot, configFileName, options.ignorePatterns) + const canPrompt = !options.json && dependencies.canPrompt() + const selection = await dependencies.resolveSelection({ + path: options.directory, + config: options.configName, + clientId: options.clientId, + withoutAppConfig: options.withoutAppConfig, + allowPrompts: canPrompt, + }) + const {appDirectory} = selection + const commands = resolveAppSecurityCommands(appDirectory, selectedConfigFileName(selection), options.ignorePatterns) + // The prompt is only shown when no TOML was found and `--without-app-config` wasn't passed. + if (selection.kind === 'no-config' && !options.withoutAppConfig) { + dependencies.renderInfo({ + headline: 'To skip these prompts next time, run:', + body: [{command: formatAppSecurityCommand(commands.scan)}], + }) + } + const scanDirectories: AppSecurityScanDirectory[] = [{directory: appDirectory, origin: 'app_directory'}] - const execution = await dependencies.execute({appRoot, configFileName, ignorePatterns: options.ignorePatterns}) - const artifacts = await dependencies.writeArtifacts(appRoot, { + const execution = await dependencies.execute({ + appDirectory, + appConfigFilePath: selection.kind === 'config' ? selection.appConfigFilePath : undefined, + clientId: effectiveClientId(selection), + ignorePatterns: options.ignorePatterns, + }) + const artifacts = await dependencies.writeArtifacts(appDirectory, { deterministicFindings: execution.deterministicFindings, agentChecks: execution.agentChecks, }) if (options.json) { - dependencies.output(encodeSecurityJson(toSecurityJson(execution, artifacts.agentChecksPath))) + dependencies.output( + encodeSecurityJson(toSecurityJson(execution, artifacts.agentChecksPath, selection, scanDirectories)), + ) } else { - dependencies.renderReport(securityReportInput(execution, artifacts, options.verbose, commands)) + dependencies.renderReport( + securityReportInput(execution, artifacts, options.verbose, commands, selection, scanDirectories), + ) } - const destination = await instructionsDestination(options, dependencies) + const destination = await instructionsDestination(options, dependencies, canPrompt) if (destination !== 'nothing') { await dependencies.deliverInstructions({ - directory: options.directory, + appDirectory, copy: destination === 'copy', scanComplete: true, commands, diff --git a/packages/app/src/cli/services/security-clean.test.ts b/packages/app/src/cli/services/security-clean.test.ts index be13782987b..4539c620e51 100644 --- a/packages/app/src/cli/services/security-clean.test.ts +++ b/packages/app/src/cli/services/security-clean.test.ts @@ -1,8 +1,7 @@ import securityClean, {renderSecurityCleanResult} from './security-clean.js' import {securityCleanJsonOutputSchema} from './security-clean-json.js' -import {resolveAppSecurityRoot} from './app-security-api.js' import {appSecurityArtifactPaths, cleanAppSecurityArtifacts} from './app-security-artifacts.js' -import {fileExists, inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs' +import {fileExists, fileRealPath, inTemporaryDirectory, mkdir, writeFile} from '@shopify/cli-kit/node/fs' import {AbortError} from '@shopify/cli-kit/node/error' import {joinPath} from '@shopify/cli-kit/node/path' import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output' @@ -13,7 +12,7 @@ import type {AppSecurityArtifactPaths} from './app-security-artifacts.js' async function createApp(directory: string): Promise { await writeFile(joinPath(directory, 'shopify.app.toml'), 'client_id = "test"\n') - return resolveAppSecurityRoot(directory) + return fileRealPath(directory) } async function writeEveryArtifact(paths: AppSecurityArtifactPaths): Promise { diff --git a/packages/app/src/cli/services/security-json.ts b/packages/app/src/cli/services/security-json.ts index f8d8ebb157d..165d9279f66 100644 --- a/packages/app/src/cli/services/security-json.ts +++ b/packages/app/src/cli/services/security-json.ts @@ -1,8 +1,17 @@ +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 } @@ -10,9 +19,18 @@ interface AppSecurityJsonResult { 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, } diff --git a/packages/app/src/cli/services/security-output.test.ts b/packages/app/src/cli/services/security-output.test.ts index ee59b2b6f68..d86226582e8 100644 --- a/packages/app/src/cli/services/security-output.test.ts +++ b/packages/app/src/cli/services/security-output.test.ts @@ -2,6 +2,7 @@ import {buildSecurityAlert} from './security-output.js' import {formatAppSecurityCommand, resolveAppSecurityCommands} from './app-security-commands.js' import {describe, expect, test} from 'vitest' import type {SecurityReportInput} from './security-output.js' +import type {AppSecuritySelection} from './app-security-selection.js' import type {ScanResult} from './app-security-engine/index.js' const engine = { @@ -70,6 +71,13 @@ const scanWithIssues: ScanResult = { function reportInput(overrides: Partial = {}): SecurityReportInput { return { scan: scanWithIssues, + selection: { + kind: 'config', + appDirectory: '/tmp/app', + appConfigFilePath: '/tmp/app/shopify.app.toml', + configClientId: 'toml-client-id', + }, + scanDirectories: [{directory: '/tmp/app', origin: 'app_directory'}], engine, verbose: false, elapsedMilliseconds: 125, @@ -85,7 +93,66 @@ function section(input: SecurityReportInput, title: string) { return buildSecurityAlert(input).options.customSections?.find((entry) => entry.title === title) } +function selectionRows(selection: AppSecuritySelection) { + return section(reportInput({selection}), 'Selection')?.body +} + describe('buildSecurityAlert', () => { + test('shows the app directory, the config file, the client ID and the scan directories', () => { + expect(selectionRows(reportInput().selection)).toEqual({ + tabularData: [ + ['App directory', '/tmp/app'], + ['Config file', 'shopify.app.toml'], + ['Client ID', 'toml-client-id'], + ['Scan directories', '.'], + ], + firstColumnSubdued: true, + }) + }) + + test('says where the client ID came from when --client-id overrides the TOML', () => { + const rows = selectionRows({ + kind: 'config', + appDirectory: '/tmp/app', + appConfigFilePath: '/tmp/app/shopify.app.staging.toml', + configClientId: 'toml-client-id', + clientIdOverride: 'flag-client-id', + }) + + expect(rows).toMatchObject({ + tabularData: expect.arrayContaining([ + ['Config file', 'shopify.app.staging.toml'], + ['Client ID', 'flag-client-id (from --client-id)'], + ]), + }) + }) + + test('shows an unlinked configuration as not linked', () => { + const rows = selectionRows({ + kind: 'config', + appDirectory: '/tmp/app', + appConfigFilePath: '/tmp/app/shopify.app.toml', + }) + + expect(rows).toMatchObject({tabularData: expect.arrayContaining([['Client ID', 'not linked']])}) + }) + + test('shows no config file when scanning without app configuration', () => { + const rows = selectionRows({ + kind: 'no-config', + appDirectory: '/tmp/app', + clientId: 'chosen-id', + clientIdSource: 'picker', + }) + + expect(rows).toMatchObject({ + tabularData: expect.arrayContaining([ + ['Config file', 'none'], + ['Client ID', 'chosen-id'], + ]), + }) + }) + test('renders a concise grouped error report for high-severity issues', () => { const alert = buildSecurityAlert(reportInput()) const serialized = JSON.stringify(alert) diff --git a/packages/app/src/cli/services/security-output.ts b/packages/app/src/cli/services/security-output.ts index 2d357ad8f44..6f9de3c42af 100644 --- a/packages/app/src/cli/services/security-output.ts +++ b/packages/app/src/cli/services/security-output.ts @@ -1,4 +1,10 @@ import {formatAppSecurityCommand, type AppSecurityCommands} from './app-security-commands.js' +import { + effectiveClientId, + selectedConfigFileName, + type AppSecurityScanDirectory, + type AppSecuritySelection, +} from './app-security-selection.js' import { groupIssues, type Capabilities, @@ -7,6 +13,7 @@ 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' @@ -18,6 +25,8 @@ interface SecurityEngineMetadata { export interface SecurityReportInput { scan: ScanResult + selection: AppSecuritySelection + scanDirectories: AppSecurityScanDirectory[] engine: SecurityEngineMetadata verbose: boolean elapsedMilliseconds: number @@ -126,8 +135,32 @@ function securityNextSteps(input: SecurityReportInput): TokenItem[] ] } +function formatClientId(selection: AppSecuritySelection): string { + const clientId = effectiveClientId(selection) ?? 'not linked' + return selection.kind === 'config' && selection.clientIdOverride ? `${clientId} (from --client-id)` : clientId +} + +function selectionSection(input: SecurityReportInput): AlertCustomSection { + const {selection} = input + return { + title: 'Selection', + body: { + tabularData: [ + ['App directory', selection.appDirectory], + ['Config file', selectedConfigFileName(selection) ?? 'none'], + ['Client ID', formatClientId(selection)], + [ + 'Scan directories', + input.scanDirectories.map(({directory}) => relativePath(selection.appDirectory, directory) || '.').join(', '), + ], + ], + firstColumnSubdued: true, + }, + } +} + function securityCustomSections(input: SecurityReportInput, groups: IssueGroup[]): AlertCustomSection[] { - const sections: AlertCustomSection[] = [] + const sections: AlertCustomSection[] = [selectionSection(input)] for (const severity of ['high', 'medium', 'low'] as const) { const severityGroups = groups.filter((group) => group.severity === severity) diff --git a/packages/app/src/cli/services/security-record.test.ts b/packages/app/src/cli/services/security-record.test.ts index fbc6e8f40f5..58b8713bea3 100644 --- a/packages/app/src/cli/services/security-record.test.ts +++ b/packages/app/src/cli/services/security-record.test.ts @@ -1,9 +1,16 @@ import securityRecord, {renderSecurityRecordResult} from './security-record.js' import {securityRecordJsonOutputSchema} from './security-record-json.js' -import {resolveAppSecurityRoot} from './app-security-api.js' import {appSecurityArtifactPaths, readFindingsDocument, writeAgentFindings} from './app-security-artifacts.js' import {formatAppSecurityCommand, resolveAppSecurityCommands} from './app-security-commands.js' -import {fileExists, fileSize, inTemporaryDirectory, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs' +import { + fileExists, + fileRealPath, + fileSize, + inTemporaryDirectory, + mkdir, + readFile, + writeFile, +} from '@shopify/cli-kit/node/fs' import {AbortError, handler} from '@shopify/cli-kit/node/error' import {joinPath} from '@shopify/cli-kit/node/path' import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output' @@ -46,7 +53,7 @@ function validDocument(): Record { async function createApp(directory: string): Promise { await writeFile(joinPath(directory, 'shopify.app.toml'), 'client_id = "test"\n') - return resolveAppSecurityRoot(directory) + return fileRealPath(directory) } function testDependencies(stdin: string | undefined): SecurityRecordDependencies { diff --git a/packages/app/src/cli/services/security-review.test.ts b/packages/app/src/cli/services/security-review.test.ts index 1512a3953fd..55c7a5013bf 100644 --- a/packages/app/src/cli/services/security-review.test.ts +++ b/packages/app/src/cli/services/security-review.test.ts @@ -1,5 +1,4 @@ import securityReview, {reviewAppSecurityResults, type SecurityReviewDependencies} from './security-review.js' -import {resolveAppSecurityRoot} from './app-security-api.js' import {appSecurityArtifactPaths} from './app-security-artifacts.js' import {loadAppSecurityResults} from './app-security-results.js' import {appSecurityResultsFor} from './app-security-results.test-data.js' @@ -32,7 +31,7 @@ async function createApp(directory: string, files: Files): Promise { function testDependencies() { const dependencies = { - resolveRoot: resolveAppSecurityRoot, + resolveRoot: async (directory: string) => directory, loadResults: loadAppSecurityResults, output: vi.fn(), render: vi.fn(), diff --git a/packages/app/src/cli/services/security-review.ts b/packages/app/src/cli/services/security-review.ts index ab7d277cbc5..5bda49b1cfd 100644 --- a/packages/app/src/cli/services/security-review.ts +++ b/packages/app/src/cli/services/security-review.ts @@ -1,11 +1,12 @@ -import {resolveAppSecurityRoot, type AppSecurityBlockingLevel} from './app-security-api.js' import {resolveAppSecurityCommands} from './app-security-commands.js' +import {resolveAppSecuritySelection} 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' import {activeFindings, SEVERITY_RANK, type CombinedCheck} from './app-security-engine/index.js' import {AbortError} from '@shopify/cli-kit/node/error' import {outputResult} from '@shopify/cli-kit/node/output' +import type {AppSecurityBlockingLevel} from './app-security-api.js' interface SecurityReviewOptions { directory: string @@ -33,7 +34,7 @@ export interface SecurityReviewResult { } export interface SecurityReviewDependencies { - resolveRoot(directory: string): string + resolveRoot(directory: string): Promise loadResults(appRoot: string): Promise output(content: string): void render(input: SecurityReviewPresenterInput): void @@ -42,7 +43,8 @@ export interface SecurityReviewDependencies { } const defaultDependencies: SecurityReviewDependencies = { - resolveRoot: resolveAppSecurityRoot, + resolveRoot: async (directory) => + (await resolveAppSecuritySelection({path: directory, allowPrompts: false})).appDirectory, loadResults: loadAppSecurityResults, output: outputResult, render: renderSecurityReview, @@ -115,7 +117,7 @@ export default async function securityReview( options: SecurityReviewOptions, dependencies: SecurityReviewDependencies = defaultDependencies, ): Promise { - const appRoot = dependencies.resolveRoot(options.directory) + const appRoot = await dependencies.resolveRoot(options.directory) const results = await dependencies.loadResults(appRoot) const result = reviewAppSecurityResults(results, { appRoot, diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index 6f8b062eafa..45737607e79 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -3994,8 +3994,8 @@ "args": { }, "customPluginName": "@shopify/app", - "description": "Runs Shopify App Security locally and writes `deterministic-findings.json` and `agent-checks.json` to `.shopify/app-security/`. 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; App Security inspects only that configuration.\n\nUse `--ignore` to change which files are scanned. Each value is one `.gitignore` pattern relative to the app directory; prefix it with `!` to include a file again when it is ignored by default or by `.gitignore`. Repeat the flag to add patterns; later patterns take precedence. A file can't be included again while its parent folder is ignored, so include the folder again instead, for example `--ignore '!build/'`. Quote each value so your shell doesn't expand `!` or `*` (single quotes in POSIX shells and PowerShell). The coding-agent instructions this check offers repeat the patterns. Other `app security` commands don't take `--ignore`, so pass the same patterns each time you run the check.\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 Shopify App Security locally and writes `deterministic-findings.json` and `agent-checks.json` to `.shopify/app-security/`. 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; App Security inspects only that configuration.\n\nUse `--ignore` to change which files are scanned. Each value is one `.gitignore` pattern relative to the app directory; prefix it with `!` to include a file again when it is ignored by default or by `.gitignore`. Repeat the flag to add patterns; later patterns take precedence. A file can't be included again while its parent folder is ignored, so include the folder again instead, for example `--ignore '!build/'`. Quote each value so your shell doesn't expand `!` or `*` (single quotes in POSIX shells and PowerShell). The coding-agent instructions this check offers repeat the patterns. Other `app security` commands don't take `--ignore`, so pass the same patterns each time you run the check.\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 Shopify App Security locally and writes `deterministic-findings.json` and `agent-checks.json` to `.shopify/app-security/`. 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; App Security 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\nUse `--ignore` to change which files are scanned. Each value is one `.gitignore` pattern relative to the app directory; prefix it with `!` to include a file again when it is ignored by default or by `.gitignore`. Repeat the flag to add patterns; later patterns take precedence. A file can't be included again while its parent folder is ignored, so include the folder again instead, for example `--ignore '!build/'`. Quote each value so your shell doesn't expand `!` or `*` (single quotes in POSIX shells and PowerShell). The coding-agent instructions this check offers repeat the patterns. Other `app security` commands don't take `--ignore`, so pass the same patterns each time you run the check.\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 Shopify App Security locally and writes `deterministic-findings.json` and `agent-checks.json` to `.shopify/app-security/`. 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; App Security 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\nUse `--ignore` to change which files are scanned. Each value is one `.gitignore` pattern relative to the app directory; prefix it with `!` to include a file again when it is ignored by default or by `.gitignore`. Repeat the flag to add patterns; later patterns take precedence. A file can't be included again while its parent folder is ignored, so include the folder again instead, for example `--ignore '!build/'`. Quote each value so your shell doesn't expand `!` or `*` (single quotes in POSIX shells and PowerShell). The coding-agent instructions this check offers repeat the patterns. Other `app security` commands don't take `--ignore`, so pass the same patterns each time you run the check.\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": { @@ -4013,6 +4013,18 @@ ], "type": "option" }, + "client-id": { + "description": "The Client ID of your app.", + "env": "SHOPIFY_FLAG_CLIENT_ID", + "exclusive": [ + "config" + ], + "hasDynamicHelp": false, + "hidden": false, + "multiple": false, + "name": "client-id", + "type": "option" + }, "config": { "char": "c", "description": "The name of the app configuration.", @@ -4088,6 +4100,19 @@ "name": "verbose", "type": "boolean" }, + "without-app-config": { + "allowNo": false, + "dependsOn": [ + "client-id" + ], + "description": "Scan --path as an app with no app configuration file. Config checks are skipped. Requires --client-id.", + "env": "SHOPIFY_FLAG_WITHOUT_APP_CONFIG", + "exclusive": [ + "config" + ], + "name": "without-app-config", + "type": "boolean" + }, "yes": { "allowNo": false, "description": "Print coding-agent instructions without prompting.",