App Security: scan multiple directories with --include-dir

packages/app/src/cli/commands/app/security/check.test.ts

@@ -28,6 +28,7 @@ describe('app security check command', () => {
       'client-id',
       'config',
       'exclude',
+      'include-dir',
       'json',
       'no-git-ignore',
       'path',
@@ -53,6 +54,7 @@ describe('app security check command', () => {
       blocking: 'high',
       yes: false,
       skipInstructions: true,
+      includeDirs: [],
       excludePatterns: [],
       noGitIgnore: false,
     })
@@ -69,13 +71,26 @@ describe('app security check command', () => {
     )
   })
 
+  test('forwards repeated --include-dir values exactly as typed, without resolving them', async () => {
+    await SecurityCheck.run(
+      ['--include-dir', '../backend', '--include-dir', './lib/', '--include-dir', 'a b', '--skip-instructions'],
+      import.meta.url,
+    )
+
+    expect(securityCheck).toHaveBeenCalledWith(
+      expect.objectContaining({includeDirs: ['../backend', './lib/', 'a b'], skipInstructions: true}),
+    )
+  })
+
   test('forwards --no-git-ignore', async () => {
     await SecurityCheck.run(['--no-git-ignore', '--skip-instructions'], import.meta.url)
 
     expect(securityCheck).toHaveBeenCalledWith(expect.objectContaining({noGitIgnore: true, skipInstructions: true}))
   })
 
-  test('does not read --exclude or --no-git-ignore from the environment', () => {
+  test('reads --include-dir and --exclude only from the command line', () => {
+    expect(SecurityCheck.flags['include-dir'].env).toBeUndefined()
+    expect(SecurityCheck.flags['include-dir'].multiple).toBe(true)
     expect(SecurityCheck.flags.exclude.env).toBeUndefined()
     expect(SecurityCheck.flags['no-git-ignore'].env).toBe('SHOPIFY_FLAG_NO_GIT_IGNORE')
   })
@@ -97,6 +112,7 @@ describe('app security check command', () => {
       blocking: 'none',
       yes: true,
       skipInstructions: false,
+      includeDirs: [],
       excludePatterns: [],
       noGitIgnore: false,
     })

packages/app/src/cli/commands/app/security/check.ts

@@ -26,6 +26,13 @@ In interactive terminals, the command offers to copy the coding-agent instructio
   static flags = {
     ...globalFlags,
     ...appSecuritySelectionFlags,
+    // No environment variable: oclif passes a repeatable flag's variable as one string, so it could hold only one directory.
+    // eslint-disable-next-line @shopify/cli/command-flags-with-env
+    'include-dir': Flags.string({
+      description:
+        'Also scan this directory, relative to the working directory. Repeat the flag to add directories. Use it for code that lives outside the app directory, such as a backend or a shared library.',
+      multiple: true,
+    }),
     // 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
     exclude: Flags.string({
@@ -67,6 +74,7 @@ In interactive terminals, the command offers to copy the coding-agent instructio
       blocking: flags.blocking,
       yes: flags.yes,
       skipInstructions: flags['skip-instructions'],
+      includeDirs: flags['include-dir'] ?? [],
       excludePatterns: flags.exclude ?? [],
       noGitIgnore: Boolean(flags['no-git-ignore']),
     })

packages/app/src/cli/services/app-security-api.test.ts

@@ -20,6 +20,7 @@ function scanInputFor(directory: string) {
   return {
     appDirectory: directory,
     scanDirectories: [directory],
+    requestedScanDirectories: [directory],
     appConfigFilePath: joinPath(directory, 'shopify.app.toml'),
   }
 }

packages/app/src/cli/services/app-security-commands.test.ts

@@ -192,6 +192,31 @@ describe('resolveAppSecurityCommands', () => {
     expect(commands.clean.args).toEqual(['app', 'security', 'clean', {flag: '--path', value: '/tmp/app'}])
   })
 
+  test('repeats --include-dir values as typed, in order, before --exclude, on scan only', () => {
+    const commands = resolveAppSecurityCommands('/tmp/app', 'shopify.app.staging.toml', ['generated'], true, [
+      '../backend',
+      './lib/',
+      '../backend',
+    ])
+
+    expect(commands.scan.args).toEqual([
+      'app',
+      'security',
+      'check',
+      {flag: '--path', value: '/tmp/app'},
+      {flag: '--config', value: 'staging'},
+      {flag: '--include-dir', value: '../backend'},
+      {flag: '--include-dir', value: './lib/'},
+      {flag: '--include-dir', value: '../backend'},
+      {flag: '--exclude', value: 'generated'},
+      '--no-git-ignore',
+    ])
+    expect(commands.record.args).toEqual(['app', 'security', 'record', {flag: '--path', value: '/tmp/app'}])
+    expect(formatAppSecurityCommand(commands.scan, 'posix')).toContain(
+      "--include-dir '../backend' --include-dir './lib/' --include-dir '../backend' --exclude 'generated'",
+    )
+  })
+
   test('omits --exclude and --no-git-ignore when they were not passed', () => {
     expect(resolveAppSecurityCommands('/tmp/app', undefined, [], false).scan.args).toEqual([
       'app',

packages/app/src/cli/services/app-security-commands.ts

@@ -19,16 +19,17 @@ export interface AppSecurityCommands {
   clean: AppSecurityCommand
 }
 
-/** `excludePatterns` and `noGitIgnore` are repeated on scan so that rerunning the check gathers the same files. */
+/** `includeDirs`, `excludePatterns` and `noGitIgnore` are repeated on scan so that rerunning the check gathers the same files. */
 export function resolveAppSecurityCommands(
   appRoot: string,
   configFileName?: string,
   excludePatterns: ReadonlyArray<string> = [],
   noGitIgnore = false,
+  includeDirs: ReadonlyArray<string> = [],
 ): AppSecurityCommands {
   const configFlag = configFileName ? getAppConfigurationShorthand(configFileName) : undefined
   const command = 'shopify'
-  // Only check reads the app configuration and discovers files, so it's the only command that takes --config, --exclude or --no-git-ignore.
+  // Only check reads the app configuration and discovers files, so it's the only command that takes --config, --include-dir, --exclude or --no-git-ignore.
   const subcommandArgs = (subcommand: string): AppSecurityArgument[] => [
     'app',
     'security',
@@ -42,6 +43,7 @@ export function resolveAppSecurityCommands(
       args: [
         ...subcommandArgs('check'),
         ...(configFlag ? [{flag: '--config', value: configFlag}] : []),
+        ...includeDirs.map((includeDir) => ({flag: '--include-dir', value: includeDir})),
         ...excludePatterns.map((excludePattern) => ({flag: '--exclude', value: excludePattern})),
         ...(noGitIgnore ? ['--no-git-ignore'] : []),
       ],

packages/app/src/cli/services/app-security-engine/checks/index.ts

@@ -207,20 +207,13 @@ const isRecord = (value: unknown): value is Record<string, unknown> =>
 const isPositiveInteger = (value: unknown): value is number => Number.isInteger(value) && (value as number) >= 1
 
 /**
- * Reject paths that escape the app root or are absolute.
- *
- * The findings document is developer-controlled input that ends up verbatim in
- * agent-findings.json. A path like `../../../etc/passwd` is never a legitimate
- * finding location.
+ * Reject absolute paths. A path may start with `../`: a scan directory can sit outside the app directory, and
+ * finding locations are relative to the app directory.
  */
 const isSafeRelativePath = (path: string): boolean => {
   if (!path) return false
-  // Reject absolute POSIX and Windows paths.
   if (path.startsWith('/') || /^[a-zA-Z]:[\\/]/.test(path)) return false
-  if (path.includes('\0')) return false
-  // Normalise separators and reject any traversal segment.
-  const segments = path.replace(/\\/g, '/').split('/')
-  return !segments.includes('..')
+  return !path.includes('\0')
 }
 
 /**
@@ -240,8 +233,7 @@ export const validateFinding = (value: unknown): string | undefined => {
     return 'finding requires at least one evidence citation'
   if (finding.evidence.length > MAX_EVIDENCE) return `finding exceeds ${MAX_EVIDENCE} evidence citations`
   if (finding.file.length > MAX_PATH_LENGTH) return `file path exceeds ${MAX_PATH_LENGTH} characters`
-  if (!isSafeRelativePath(finding.file))
-    return `unsafe file path (must be relative and inside the app): ${finding.file}`
+  if (!isSafeRelativePath(finding.file)) return `unsafe file path (must be relative): ${finding.file}`
   if (finding.message.length > MAX_MESSAGE_LENGTH) return `message exceeds ${MAX_MESSAGE_LENGTH} characters`
   if (finding.snippet !== undefined && typeof finding.snippet !== 'string') return 'snippet must be a string'
   if (typeof finding.snippet === 'string' && finding.snippet.length > MAX_SNIPPET_LENGTH)

packages/app/src/cli/services/app-security-engine/rules/secret-rules.ts

@@ -1,4 +1,4 @@
-import {dirname, joinPath} from '@shopify/cli-kit/node/path'
+import {basename, dirname} from '@shopify/cli-kit/node/path'
 import {captureOutputWithExitCode} from '@shopify/cli-kit/node/system'
 import type {ScanContext, SourceFile} from './types.js'
 import type {Issue} from '../types.js'
@@ -156,15 +156,14 @@ async function gitTopLevel(cwd: string): Promise<string | undefined> {
 
 /** When git listed ignored paths, a nested repository is confirmed by comparing top levels. */
 async function ignoredFileScanReason(
-  appRoot: string,
-  path: string,
+  file: SourceFile,
   gitIgnoreListing: ScanContext['gitIgnoreListing'],
   appTopLevel: () => Promise<string | undefined>,
 ): Promise<{reason: IgnoredFileScanReason; evidence: string[]}> {
   if (gitIgnoreListing === 'failed') return {reason: 'listing-failed', evidence: []}
   if (gitIgnoreListing === 'not-a-repository') return {reason: 'unknown', evidence: []}
 
-  const fileTopLevel = await gitTopLevel(dirname(joinPath(appRoot, path)))
+  const fileTopLevel = await gitTopLevel(dirname(file.absolutePath))
   const appRootTopLevel = await appTopLevel()
   const bothKnown = fileTopLevel !== undefined && appRootTopLevel !== undefined
   const nested = bothKnown && fileTopLevel !== appRootTopLevel
@@ -173,7 +172,7 @@ async function ignoredFileScanReason(
   else if (bothKnown) verdict = 'same'
   return {
     reason: nested ? 'nested-repository' : 'unknown',
-    evidence: [`git rev-parse --show-toplevel → ${verdict} for ${path} and the app root`],
+    evidence: [`git rev-parse --show-toplevel → ${verdict} for ${file.path} and the app root`],
   }
 }
 
@@ -263,7 +262,7 @@ export async function scanCommittedSecrets(
 
     // Keep git probes sequential to avoid spawning competing processes for one repository.
     // eslint-disable-next-line no-await-in-loop
-    const status = await gitStatusFor(appRoot, file.path)
+    const status = await gitStatusFor(file)
     // Empty named secret files stay fail-closed only when git confirms they are
     // tracked — history may still contain prior secrets. Unknown git plus empty
     // contents is not a provable leak.
@@ -279,7 +278,7 @@ export async function scanCommittedSecrets(
       // With --no-git-ignore the file was scanned only because filtering was off, and Git still protects it.
       if (gitIgnoreListing === 'git-ignore-off') continue
       // eslint-disable-next-line no-await-in-loop
-      const scanReason = await ignoredFileScanReason(appRoot, file.path, gitIgnoreListing, appRootTopLevel)
+      const scanReason = await ignoredFileScanReason(file, gitIgnoreListing, appRootTopLevel)
       ignoredScanReason = scanReason.reason
       evidence = [...evidence, ...scanReason.evidence]
     }
@@ -353,8 +352,10 @@ async function runGit(cwd: string, args: string[]): Promise<{exitCode?: number;
   }
 }
 
-export async function gitStatusFor(appRoot: string, file: string): Promise<GitFileStatus> {
-  const run = async (args: string[]) => runGit(appRoot, args)
+/** Asks the file's own repository, which may differ from the app's: a scan directory can be another repository. */
+export async function gitStatusFor(file: Pick<SourceFile, 'path' | 'absolutePath'>): Promise<GitFileStatus> {
+  const run = async (args: string[]) => runGit(dirname(file.absolutePath), args)
+  const name = basename(file.absolutePath)
 
   // Is this even a git repo? If not, we cannot confirm anything.
   const inRepo = await run(['rev-parse', '--is-inside-work-tree'])
@@ -371,25 +372,25 @@ export async function gitStatusFor(appRoot: string, file: string): Promise<GitFi
 
   // Exit 1 is git's conclusive "no match" result. Any other failure remains
   // unknown instead of being silently interpreted as an untracked file.
-  const ls = await run(['ls-files', '--error-unmatch', '--', file])
+  const ls = await run(['ls-files', '--error-unmatch', '--', name])
   let tracked: boolean | undefined
   if (ls.exitCode === 0 && ls.out.length > 0) tracked = true
   else if (ls.exitCode === 1) tracked = false
   let trackedVerdict = 'unknown'
   if (tracked === true) trackedVerdict = 'TRACKED'
   else if (tracked === false) trackedVerdict = 'untracked'
-  evidence.push(`git ls-files --error-unmatch ${file} → ${trackedVerdict}`)
+  evidence.push(`git ls-files --error-unmatch ${file.path} → ${trackedVerdict}`)
 
   // check-ignore likewise defines 0 as ignored and 1 as conclusively not
   // ignored. Exit codes above 1 are command errors and must fail closed.
-  const checkIgnore = await run(['check-ignore', '-q', '--', file])
+  const checkIgnore = await run(['check-ignore', '-q', '--', name])
   let ignored: boolean | undefined
   if (checkIgnore.exitCode === 0) ignored = true
   else if (checkIgnore.exitCode === 1) ignored = false
   let ignoredVerdict = 'unknown'
   if (ignored === true) ignoredVerdict = 'ignored'
   else if (ignored === false) ignoredVerdict = 'not ignored'
-  evidence.push(`git check-ignore -q ${file} → ${ignoredVerdict}`)
+  evidence.push(`git check-ignore -q ${file.path} → ${ignoredVerdict}`)
 
   return {
     tracked,

packages/app/src/cli/services/app-security-engine/scanners/discover.ts

@@ -203,6 +203,11 @@ interface GatherInput {
   appDirectory: string
   /** Absolute real paths of the directories to walk. */
   scanDirectories: ReadonlyArray<string>
+  /**
+   * Every directory that was asked for, including those inside another scan directory, so each can be checked for the
+   * ignored-scan-directory warning. It contains every entry of `scanDirectories`.
+   */
+  requestedScanDirectories: ReadonlyArray<string>
   /** Added after filtering, so no path rule can remove it. */
   selectedAppConfigFilePath?: string
   rules: PathRules
@@ -211,28 +216,36 @@ interface GatherInput {
 interface GatheredPaths {
   /** Sorted and unique, relative to the app directory, written with `/`. They may start with `../`. */
   paths: string[]
-  /** Scan directories that their repository ignores, so only the files Git tracks in them were gathered. */
+  /** Requested directories that their repository ignores, so only the files Git tracks in them are gathered. */
   ignoredScanDirectories: string[]
   /** How the first scan directory's ignored paths were found. Secret findings use it to explain why an ignored file was scanned. */
   listingStatus: GatheredListingStatus
 }
 
 interface GatheredScanDirectory {
   absolutePaths: string[]
-  ignored: boolean
   listingStatus: GatheredPaths['listingStatus']
 }
 
 export async function gatherPaths({
   appDirectory,
   scanDirectories,
+  requestedScanDirectories,
   selectedAppConfigFilePath,
   rules,
 }: GatherInput): Promise<GatheredPaths> {
+  const ignoredScanDirectories: string[] = []
+  if (rules.gitFiltering) {
+    for (const requestedDirectory of requestedScanDirectories) {
+      // eslint-disable-next-line no-await-in-loop
+      if (await isIgnoredByParentRepository(requestedDirectory)) ignoredScanDirectories.push(requestedDirectory)
+    }
+  }
+
   const gathered: GatheredScanDirectory[] = []
   for (const scanDirectory of scanDirectories) {
     // eslint-disable-next-line no-await-in-loop
-    gathered.push(await gatherScanDirectory(scanDirectory, rules))
+    gathered.push(await gatherScanDirectory(scanDirectory, rules, ignoredScanDirectories.includes(scanDirectory)))
   }
 
   const absolutePaths = [
@@ -241,21 +254,24 @@ export async function gatherPaths({
   ]
   return {
     paths: [...new Set(absolutePaths.map((path) => normalizeCliPath(relativePath(appDirectory, path))))].sort(),
-    ignoredScanDirectories: scanDirectories.filter((_directory, index) => gathered[index]?.ignored),
+    ignoredScanDirectories,
     listingStatus: gathered[0]?.listingStatus ?? (rules.gitFiltering ? 'tracked-only' : 'git-ignore-off'),
   }
 }
 
-async function gatherScanDirectory(scanDirectory: string, rules: PathRules): Promise<GatheredScanDirectory> {
+async function gatherScanDirectory(
+  scanDirectory: string,
+  rules: PathRules,
+  ignoredByRepository: boolean,
+): Promise<GatheredScanDirectory> {
   if (!rules.gitFiltering) {
     return {
       absolutePaths: await walkDirectory(scanDirectory, rules, undefined),
-      ignored: false,
       listingStatus: 'git-ignore-off',
     }
   }
 
-  if (await isIgnoredByParentRepository(scanDirectory)) {
+  if (ignoredByRepository) {
     const trackedPaths = await listTrackedFiles(scanDirectory)
     // A normal walk would scan the untracked files that Git was told to ignore.
     if (trackedPaths === undefined) {
@@ -266,15 +282,13 @@ async function gatherScanDirectory(scanDirectory: string, rules: PathRules): Pro
         .filter((trackedPath) => !isDroppedTrackedPath(rules, scanDirectory, trackedPath))
         .map((trackedPath) => joinPath(scanDirectory, trackedPath))
         .filter(isTrackedFile),
-      ignored: true,
       listingStatus: 'tracked-only',
     }
   }
 
   const listing = await listGitIgnoredPaths(scanDirectory)
   return {
     absolutePaths: await walkDirectory(scanDirectory, rules, repositoryIgnoredPaths(scanDirectory, listing)),
-    ignored: false,
     listingStatus: listing.status,
   }
 }
@@ -483,15 +497,7 @@ type InspectedPath = {status: 'missing'} | {status: 'file'; path: string} | {sta
 
 function repositoryDisplayPath(path: string): string {
   const relative = normalizeCliPath(relativePath(configuredReader().appDirectory, path))
-  if (
-    relative.length === 0 ||
-    relative === '.' ||
-    relative === '..' ||
-    relative.startsWith('../') ||
-    isAbsolutePath(relative)
-  ) {
-    return 'path'
-  }
+  if (relative.length === 0 || relative === '.' || isAbsolutePath(relative)) return 'path'
   return relative
 }