Check dependency automation at the repository root for nested apps

.changeset/app-security-dependency-automation-repository-root.md

@@ -0,0 +1,5 @@
+---
+'@shopify/app': patch
+---
+
+Check for Dependabot or Renovate configuration at the Git repository root when `shopify app security check` scans an app below it

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

@@ -649,6 +649,18 @@ function readRepositoryFile(absolutePath: string): RepositoryReadResult {
   return result
 }
 
+/**
+ * Read a fixed path below a directory that isn't scanned, with that directory as the containment boundary.
+ * `undefined` means the path doesn't exist.
+ */
+function readFileContainedBy(directory: string, path: string): RepositoryReadResult | undefined {
+  const inspected = inspectRepositoryPath(directory, path)
+  if (inspected.status === 'missing') return undefined
+  const result = inspected.status === 'file' ? readBoundedFile(inspected.path) : repositoryPathFailure(inspected.reason)
+  if (!result.ok) recordSkippedFile(joinPath(directory, path), result)
+  return result
+}
+
 function readRepositoryText(absolutePath: string): string | undefined {
   const result = readRepositoryFile(absolutePath)
   return result.ok ? result.content.toString() : undefined
@@ -798,17 +810,20 @@ export function findSensitiveFiles(
   })
 }
 
-function nestedRepositoryReason(appRoot: string): string | undefined {
+/** Dependabot and Renovate read configuration from the root of the nearest repository that holds the app. */
+function dependencyAutomationRoot(appRoot: string): {directory: string} | {unresolvedReason: string} {
   const marker = findRepositoryMarker(appRoot)
-  if (marker.status === 'none') return undefined
-  if (marker.status === 'ambiguous') return marker.reason
-  return marker.directory === appRoot ? undefined : 'App root is nested below a parent Git repository'
+  if (marker.status === 'ambiguous') return {unresolvedReason: marker.reason}
+  return {directory: marker.status === 'found' ? marker.directory : appRoot}
 }
 
 /**
  * Read local bot configuration only; hosted integrations and CI workflows are
- * outside this check's scope. Only gathered paths are read, through the reader,
- * so a gathered symbolic link that leaves its scan directory is reported as unresolved.
+ * outside this check's scope. Configuration inside a scan directory is read only
+ * when it was gathered, through the reader, so path rules apply and a gathered
+ * symbolic link that leaves its scan directory is reported as unresolved. The root
+ * of a repository that holds the app below its top level may be outside every scan
+ * directory, so an allowlisted path outside them is read directly, contained by that root.
  */
 export function findDependencyAutomationInputs(
   appRoot: string,
@@ -825,16 +840,23 @@ export function findDependencyAutomationInputs(
     return {files: [], unresolvedReason: inspectErrorReason('app root', error)}
   }
 
-  const repositoryReason = nestedRepositoryReason(canonicalRoot)
-  if (repositoryReason) return {files: [], unresolvedReason: repositoryReason}
+  const configurationRoot = dependencyAutomationRoot(canonicalRoot)
+  if ('unresolvedReason' in configurationRoot) return {files: [], unresolvedReason: configurationRoot.unresolvedReason}
+  const repositoryRoot = configurationRoot.directory
+  const {scanDirectories} = configuredReader()
+  const isScanned = (absolutePath: string) =>
+    repositoryRoot === canonicalRoot || scanDirectories.some((directory) => isSubpath(directory, absolutePath))
 
   const gathered = new Set(gatheredPaths)
   const files: SourceFile[] = []
   let unresolvedReason: string | undefined
-  for (const relative of DEPENDENCY_AUTOMATION_CONFIG_PATHS) {
-    if (!gathered.has(relative)) continue
-    const absolutePath = joinPath(canonicalRoot, relative)
-    const result = readRepositoryFile(absolutePath)
+  for (const configurationPath of DEPENDENCY_AUTOMATION_CONFIG_PATHS) {
+    const absolutePath = joinPath(repositoryRoot, configurationPath)
+    const relative = normalizeCliPath(relativePath(canonicalRoot, absolutePath))
+    const scanned = isScanned(absolutePath)
+    if (scanned && !gathered.has(relative)) continue
+    const result = scanned ? readRepositoryFile(absolutePath) : readFileContainedBy(repositoryRoot, configurationPath)
+    if (result === undefined) continue
     if (!result.ok) {
       unresolvedReason ??=
         result.reason === 'too_large'

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

@@ -103,7 +103,7 @@ const DETERMINISTIC_CHECK_DEFINITIONS: ReadonlyArray<DeterministicCheckDefinitio
   configRule(missingComplianceWebhooks),
   {
     id: 'MISSING_DEPENDENCY_SECURITY_AUTOMATION',
-    version: 1,
+    version: 2,
     lifecycle: 'active',
     analysisMode: 'structured_config',
     target: 'dependency_automation',

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

@@ -44,10 +44,10 @@ export interface SourceFile {
   content?: string
 }
 
-/** Explicitly allowlisted dependency-management configuration inside the app-root evidence boundary. */
+/** Explicitly allowlisted dependency-management configuration at the root of the repository that holds the app. */
 export interface DependencyAutomationInputs {
   files: SourceFile[]
-  /** A specific discovery obstacle, including an app nested below its repository root. */
+  /** A specific discovery obstacle, such as an unreadable configuration file. */
   unresolvedReason?: string
 }
 

packages/app/src/cli/services/app-security-engine/tests/dependency-automation-discovery.test.ts

@@ -9,6 +9,7 @@ import {
 import {DEPENDENCY_AUTOMATION_CONFIG_PATHS} from '../rules/dependency-automation-rules.js'
 import {createPathRules} from '../scanners/path-rules.js'
 import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs'
+import {joinPath} from '@shopify/cli-kit/node/path'
 import {afterEach, describe, expect, test, vi} from 'vitest'
 import {execFileSync} from 'node:child_process'
 import {mkdir, symlink, writeFile} from 'node:fs/promises'
@@ -142,21 +143,171 @@ describe('dependency automation discovery', () => {
     })
   })
 
-  test.each(['directory', 'worktree file'])('respects repository %s boundaries', async (marker) => {
-    await inTemporaryDirectory(async (repository) => {
+  describe('apps below the repository root', () => {
+    async function writeRepositoryMarker(directory: string, marker: string): Promise<void> {
+      if (marker === 'directory') await mkdir(join(directory, '.git'))
+      else await writeFile(join(directory, '.git'), 'gitdir: /outside/not-read')
+    }
+
+    async function makeMonorepo(repository: string, marker: string): Promise<string> {
       const app = join(repository, 'apps', 'example')
-      await writeFiles(app, {'.github/dependabot.yml': 'version: 2\nupdates: []'})
-      if (marker === 'directory') await mkdir(join(repository, '.git'))
-      else await writeFile(join(repository, '.git'), 'gitdir: /outside/not-read')
-      const nested = await findInputs(app)
-      expect(nested).toMatchObject({
-        files: [],
-        unresolvedReason: 'App root is nested below a parent Git repository',
+      await mkdir(app, {recursive: true})
+      await writeRepositoryMarker(repository, marker)
+      return app
+    }
+
+    test.each(['directory', 'worktree file'])(
+      'reads configuration at the root of a %s repository that is not scanned',
+      async (marker) => {
+        await inTemporaryDirectory(async (repository) => {
+          const app = await makeMonorepo(repository, marker)
+          await expect(findInputs(app)).resolves.toEqual({files: []})
+          const content = 'version: 2\nupdates: []\n'
+          await writeFiles(repository, {'.github/dependabot.yml': content})
+          const result = await findInputs(app)
+          expect(result.unresolvedReason).toBeUndefined()
+          expect(result.files).toEqual([
+            expect.objectContaining({
+              path: '../../.github/dependabot.yml',
+              absolutePath: joinPath(repository, '.github/dependabot.yml'),
+              ext: '.yml',
+              content,
+            }),
+          ])
+        })
+      },
+    )
+
+    test('ignores configuration in the app directory, which bots do not read below the repository root', async () => {
+      await inTemporaryDirectory(async (repository) => {
+        const app = await makeMonorepo(repository, 'directory')
+        await writeFiles(app, {'.github/dependabot.yml': 'version: 2\nupdates: []'})
+        await expect(findInputs(app)).resolves.toEqual({files: []})
+      })
+    })
+
+    test.each(['directory', 'worktree file'])(
+      'keeps an app with its own %s repository inside that repository',
+      async (marker) => {
+        await inTemporaryDirectory(async (repository) => {
+          const app = await makeMonorepo(repository, 'directory')
+          await writeFiles(repository, {'.github/dependabot.yml': 'version: 2\nupdates: []'})
+          await writeRepositoryMarker(app, marker)
+          await expect(findInputs(app)).resolves.toEqual({files: []})
+          await writeFiles(app, {'renovate.json': '{}'})
+          expect((await findInputs(app)).files).toMatchObject([{path: 'renovate.json'}])
+        })
+      },
+    )
+
+    test('applies path rules to the repository root when it is a scan directory', async () => {
+      await inTemporaryDirectory(async (repository) => {
+        vi.stubEnv('INIT_CWD', repository)
+        const app = await makeMonorepo(repository, 'directory')
+        await writeFiles(repository, {'.github/dependabot.yml': 'version: 2\nupdates: []'})
+        const findScanningRoot = async (excludePatterns: string[]) => {
+          configureRepositoryReader({appDirectory: app, scanDirectories: [repository], explicitInputs: new Set()})
+          const {paths} = await gatherPaths({
+            appDirectory: app,
+            scanDirectories: [repository],
+            requestedScanDirectories: [app, repository],
+            rules: createPathRules({excludePatterns, noGitIgnore: true}),
+          })
+          return findDependencyAutomationInputs(app, paths)
+        }
+        expect((await findScanningRoot([])).files).toMatchObject([{path: '../../.github/dependabot.yml'}])
+        await expect(findScanningRoot(['.github'])).resolves.toEqual({files: []})
+      })
+    })
+
+    async function findIncludingGitHubDirectory(
+      repository: string,
+      app: string,
+      rules: {excludePatterns?: string[]; noGitIgnore?: boolean},
+    ) {
+      const scanDirectories = [app, joinPath(repository, '.github')]
+      configureRepositoryReader({appDirectory: app, scanDirectories, explicitInputs: new Set()})
+      const {paths} = await gatherPaths({
+        appDirectory: app,
+        scanDirectories,
+        requestedScanDirectories: scanDirectories,
+        rules: createPathRules({excludePatterns: [], noGitIgnore: true, ...rules}),
+      })
+      return findDependencyAutomationInputs(app, paths)
+    }
+
+    test('reads configuration that an included directory below the repository root gathered', async () => {
+      await inTemporaryDirectory(async (repository) => {
+        vi.stubEnv('INIT_CWD', repository)
+        const app = await makeMonorepo(repository, 'directory')
+        await writeFiles(repository, {'.github/dependabot.yml': 'version: 2\nupdates: []'})
+        expect((await findIncludingGitHubDirectory(repository, app, {})).files).toMatchObject([
+          {path: '../../.github/dependabot.yml', absolutePath: joinPath(repository, '.github/dependabot.yml')},
+        ])
+      })
+    })
+
+    test('applies exclusions to configuration in an included directory below the repository root', async () => {
+      await inTemporaryDirectory(async (repository) => {
+        vi.stubEnv('INIT_CWD', repository)
+        const app = await makeMonorepo(repository, 'directory')
+        await writeFiles(repository, {'.github/dependabot.yml': 'version: 2\nupdates: []'})
+        const excludePatterns = ['.github/dependabot.yml']
+        await expect(findIncludingGitHubDirectory(repository, app, {excludePatterns})).resolves.toEqual({files: []})
+        expect(getSkippedFiles()).toEqual([])
+
+        await writeFiles(repository, {'renovate.json': '{}'})
+        const result = await findIncludingGitHubDirectory(repository, app, {excludePatterns})
+        expect(result.files).toMatchObject([{path: '../../renovate.json', content: '{}'}])
+        expect(result.unresolvedReason).toBeUndefined()
+      })
+    })
+
+    test('does not read Git-ignored configuration in an included directory below the repository root', async () => {
+      await inTemporaryDirectory(async (repository) => {
+        const app = joinPath(repository, 'apps/example')
+        await mkdir(app, {recursive: true})
+        execFileSync('git', ['init', '--quiet'], {cwd: repository})
+        await writeFiles(repository, {
+          '.gitignore': '.github/dependabot.yml\n',
+          '.github/dependabot.yml': 'version: 2\nupdates: []',
+        })
+        await expect(findIncludingGitHubDirectory(repository, app, {noGitIgnore: false})).resolves.toEqual({
+          files: [],
+        })
+      })
+    })
+
+    test('does not follow a root configuration link that leaves the repository', async () => {
+      await inTemporaryDirectory(async (repository) => {
+        await inTemporaryDirectory(async (outside) => {
+          const app = await makeMonorepo(repository, 'directory')
+          await writeFile(join(outside, 'dependabot.yml'), 'version: 2')
+          await mkdir(join(repository, '.github'))
+          await symlink(join(outside, 'dependabot.yml'), join(repository, '.github/dependabot.yml'))
+          const result = await findInputs(app)
+          expect(result).toMatchObject({files: [], unresolvedReason: expect.stringContaining('outside')})
+          expect(result.unresolvedReason).not.toContain(repository)
+          expect(result.unresolvedReason).not.toContain(outside)
+          expect(getSkippedFiles()).toContainEqual(
+            expect.objectContaining({path: '../../.github/dependabot.yml', reason: 'unreadable'}),
+          )
+        })
+      })
+    })
+
+    test('preserves bounded reads of root configuration', async () => {
+      await inTemporaryDirectory(async (repository) => {
+        const app = await makeMonorepo(repository, 'directory')
+        await writeFiles(repository, {'.github/dependabot.yml': 'x'.repeat(500_001)})
+        await expect(findInputs(app)).resolves.toMatchObject({
+          files: [],
+          unresolvedReason: expect.stringContaining('too large'),
+        })
+        expect(getSkippedFiles()).toEqual([
+          expect.objectContaining({path: '../../.github/dependabot.yml', reason: 'too_large', size_bytes: 500_001}),
+        ])
       })
-      expect(nested.unresolvedReason).not.toContain(repository)
-      if (marker === 'directory') await mkdir(join(app, '.git'))
-      else await writeFile(join(app, '.git'), 'gitdir: /outside/not-read')
-      expect((await findInputs(app)).files).toMatchObject([{path: '.github/dependabot.yml'}])
     })
   })