App Security: resolve the app selection once

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'})
     })
   })

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'],

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 <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,

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<string> {
   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'),

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<void> {
     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) {

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<typeof import('../../../services/app-security-selection.js')>()),
+  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', () => {

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<void> {
     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,
     })