Detect React Router apps outside the app directory in app security check

.changeset/app-security-react-router-roots.md

@@ -0,0 +1,5 @@
+---
+'@shopify/app': patch
+---
+
+Detect React Router app code outside the app directory, such as in `--include-dir` or web directories, in `shopify app security check`

packages/app/src/cli/services/app-security-engine/capabilities/detect.ts

@@ -1,6 +1,9 @@
+import {dirname} from '@shopify/cli-kit/node/path'
 import type {AppTomlContent, ExtensionInfo, ManifestFile, SourceFile} from '../scanners/types.js'
 import type {Capabilities, DetectedLanguage, ProjectDetection, SourceCandidate} from '../types.js'
 
+const REACT_ROUTER_PACKAGE = '@shopify/shopify-app-react-router'
+
 /** Capabilities describe observed behavior. They do not imply framework support. */
 export function detectCapabilities(
   appToml: AppTomlContent | null,
@@ -38,29 +41,133 @@ export function detectCapabilities(
   }
 }
 
+/**
+ * Find the React Router app roots, relative to the app directory, with `.` for the app directory itself.
+ * A root needs a package.json that declares `@shopify/shopify-app-react-router` plus the conventional
+ * app/routes + app/shopify.server structure, so code gathered from outside the app directory, such as an
+ * --include-dir or web directory, is recognised. The app directory accepts the package from any gathered
+ * manifest, such as a workspace root's.
+ *
+ * Roots in another app's directory belong to that app. An app directory above this one also holds this app and
+ * the packages it shares, so only that app's own root and its `app` and `web` directories are left out.
+ */
+export function detectReactRouterRoots(
+  manifests: ManifestFile[],
+  candidates: SourceCandidate[],
+  otherAppDirectories: string[] = [],
+): string[] {
+  const declaringManifests = manifests.filter(
+    (manifest) =>
+      manifest.dependencies[REACT_ROUTER_PACKAGE] !== undefined ||
+      manifest.devDependencies?.[REACT_ROUTER_PACKAGE] !== undefined,
+  )
+  if (declaringManifests.length === 0) return []
+  const candidatePaths = candidates.map((candidate) => candidate.path)
+  const roots = new Set(['.', ...declaringManifests.map((manifest) => dirname(manifest.path))])
+  return [...roots]
+    .filter((root) => hasReactRouterStructure(root, candidatePaths))
+    .filter((root) => !belongsToOtherApp(root, otherAppDirectories))
+    .sort()
+}
+
+/** Whether `path` is the app/shopify.server module of the React Router app at `root`. */
+export function isReactRouterServerPath(root: string, path: string): boolean {
+  const pathInRoot = pathWithinRoot(root, path)
+  return pathInRoot !== undefined && /^app\/shopify\.server\.[cm]?[jt]sx?$/.test(pathInRoot)
+}
+
+/**
+ * Whether `path` is input for React Router source analysis: inside the app directory or one of `reactRouterRoots`,
+ * and outside other apps' code. The `.` root covers every gathered path.
+ *
+ * Code in the app directory belongs to this app even outside its detected roots, such as a `lib` module that a
+ * route in `web` imports, so it stays checked.
+ *
+ * Only other apps' code outside the detected roots is left out. An app configuration file anywhere inside a root,
+ * such as in the app directory's `app` or `lib`, marks part of this app's own source, so it can't hide that source
+ * from the checks.
+ */
+export function isReactRouterSourcePath(
+  path: string,
+  reactRouterRoots: string[],
+  otherAppDirectories: string[],
+): boolean {
+  const otherAppCode = otherAppCodeDirectories(otherAppDirectories).filter(
+    (directory) => !isInsideReactRouterRoot(directory, reactRouterRoots),
+  )
+  const isThisAppsCode =
+    !climbsOutOfDirectory(path) || reactRouterRoots.some((root) => pathWithinRoot(root, path) !== undefined)
+  return isThisAppsCode && !otherAppCode.some((directory) => pathWithinRoot(directory, path) !== undefined)
+}
+
+/** Whether `directory` is or is inside one of `reactRouterRoots`. For the `.` root, that's the app directory. */
+function isInsideReactRouterRoot(directory: string, reactRouterRoots: string[]): boolean {
+  return reactRouterRoots.some((root) =>
+    root === '.'
+      ? !climbsOutOfDirectory(directory)
+      : directory === root || pathWithinRoot(root, directory) !== undefined,
+  )
+}
+
+/** Whether `path`, relative to a directory, climbs out of that directory. */
+function climbsOutOfDirectory(path: string): boolean {
+  return path === '..' || path.startsWith('../')
+}
+
+/**
+ * `path` relative to `root`, or undefined when it is outside. Both use forward slashes, as gathering does.
+ * A root above the app directory, such as `../..`, prefixes paths that climb past it, such as
+ * `../../../other-app/...`, so the part after the root must not climb out of it.
+ */
+function pathWithinRoot(root: string, path: string): string | undefined {
+  if (root === '.') return path
+  if (!path.startsWith(`${root}/`)) return undefined
+  const pathInRoot = path.slice(root.length + 1)
+  return climbsOutOfDirectory(pathInRoot) ? undefined : pathInRoot
+}
+
+function hasReactRouterStructure(root: string, paths: string[]): boolean {
+  return (
+    paths.some((path) => pathWithinRoot(root, path)?.startsWith('app/routes/')) &&
+    paths.some((path) => isReactRouterServerPath(root, path))
+  )
+}
+
+function belongsToOtherApp(root: string, otherAppDirectories: string[]): boolean {
+  if (root === '.') return false
+  return (
+    otherAppDirectories.includes(root) ||
+    otherAppCodeDirectories(otherAppDirectories).some(
+      (directory) => root === directory || pathWithinRoot(directory, root) !== undefined,
+    )
+  )
+}
+
+/**
+ * Directories that hold another app's code. An app directory above this one also holds this app and the
+ * packages it shares, so only that app's conventional `app` and `web` directories count as its code.
+ */
+function otherAppCodeDirectories(otherAppDirectories: string[]): string[] {
+  return otherAppDirectories.flatMap((directory) =>
+    isAncestorOfAppDirectory(directory) ? [`${directory}/app`, `${directory}/web`] : [directory],
+  )
+}
+
+function isAncestorOfAppDirectory(directory: string): boolean {
+  return directory.split('/').every((segment) => segment === '..')
+}
+
 /**
  * Detect the framework and product surface independently from capabilities.
- * React Router support requires both its manifest package and the conventional
- * app/routes + app/shopify.server structure; a coincidental route export is
- * not enough to claim deterministic coverage.
+ * React Router support requires at least one root from `detectReactRouterRoots`;
+ * a coincidental route export is not enough to claim deterministic coverage.
  */
 export function detectProject(
-  manifests: ManifestFile[],
   extensions: ExtensionInfo[],
   candidates: SourceCandidate[],
+  reactRouterRoots: string[],
 ): ProjectDetection {
-  const dependencyNames = new Set(
-    manifests.flatMap((manifest) => [
-      ...Object.keys(manifest.dependencies),
-      ...Object.keys(manifest.devDependencies ?? {}),
-    ]),
-  )
-  const candidatePaths = new Set(candidates.map((candidate) => candidate.path))
-  const hasReactRouterPackage = dependencyNames.has('@shopify/shopify-app-react-router')
-  const hasReactRouterStructure =
-    [...candidatePaths].some((path) => path.startsWith('app/routes/')) &&
-    [...candidatePaths].some((path) => /^app\/shopify\.server\.[cm]?[jt]sx?$/.test(path))
-  const reactRouter = hasReactRouterPackage && hasReactRouterStructure
+  const reactRouter = reactRouterRoots.length > 0
   const themeExtensions = extensions.filter((extension) => extension.type === 'theme')
   const themeExtension = themeExtensions.length > 0
   const themePaths = new Set(themeExtensions.flatMap((extension) => extension.files.map((file) => file.path)))

packages/app/src/cli/services/app-security-engine/checks/APP_PROXY_LIQUID_INJECTION.md

@@ -1,6 +1,6 @@
 ---
 id: APP_PROXY_LIQUID_INJECTION
-version: 2
+version: 3
 severity: high
 precedence: prefer-agent
 ---

packages/app/src/cli/services/app-security-engine/checks/CREDENTIAL_BROWSER_LEAKAGE.md

@@ -1,6 +1,6 @@
 ---
 id: CREDENTIAL_BROWSER_LEAKAGE
-version: 1
+version: 2
 severity: high
 precedence: prefer-agent
 ---

packages/app/src/cli/services/app-security-engine/checks/CREDENTIAL_LOG_LEAKAGE.md

@@ -1,6 +1,6 @@
 ---
 id: CREDENTIAL_LOG_LEAKAGE
-version: 1
+version: 2
 severity: high
 precedence: prefer-agent
 ---

packages/app/src/cli/services/app-security-engine/checks/DEPRECATED_SCRIPT_TAG_SCOPE.md

@@ -1,6 +1,6 @@
 ---
 id: DEPRECATED_SCRIPT_TAG_SCOPE
-version: 1
+version: 2
 severity: medium
 precedence: prefer-agent
 ---

packages/app/src/cli/services/app-security-engine/checks/EOL_API_VERSION.md

@@ -1,6 +1,6 @@
 ---
 id: EOL_API_VERSION
-version: 1
+version: 2
 severity: low
 precedence: prefer-agent
 ---

packages/app/src/cli/services/app-security-engine/checks/EXPIRING_OFFLINE_TOKEN.md

@@ -1,6 +1,6 @@
 ---
 id: EXPIRING_OFFLINE_TOKEN
-version: 1
+version: 2
 severity: medium
 precedence: prefer-agent
 ---

packages/app/src/cli/services/app-security-engine/checks/REQUEST_CONTROLLED_ADMIN_CONTEXT.md

@@ -1,6 +1,6 @@
 ---
 id: REQUEST_CONTROLLED_ADMIN_CONTEXT
-version: 3
+version: 4
 severity: high
 precedence: prefer-agent
 ---

packages/app/src/cli/services/app-security-engine/checks/UNAUTHENTICATED_ENDPOINT.md

@@ -1,6 +1,6 @@
 ---
 id: UNAUTHENTICATED_ENDPOINT
-version: 2
+version: 3
 severity: high
 precedence: union
 ---

packages/app/src/cli/services/app-security-engine/checks/UNSAFE_INNERHTML.md

@@ -1,6 +1,6 @@
 ---
 id: UNSAFE_INNERHTML
-version: 2
+version: 3
 severity: high
 precedence: union
 ---

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

@@ -1,3 +1,4 @@
+import {isReactRouterServerPath} from '../capabilities/detect.js'
 import {relativePath} from '@shopify/cli-kit/node/path'
 import type {Issue} from '../types.js'
 import type {ScanContext, Rule, SourceFile} from './types.js'
@@ -65,12 +66,14 @@ export function scanEolApiVersions(context: ScanContext, referenceDate = new Dat
   })
 
   if (context.detection.framework !== 'react_router') return configIssues
-  const sourceIssues = context.sourceFiles.flatMap((file) => scanReactRouterApiVersion(file, referenceDate))
+  const sourceIssues = context.sourceFiles.flatMap((file) =>
+    scanReactRouterApiVersion(file, context.reactRouterRoots, referenceDate),
+  )
   return [...configIssues, ...sourceIssues]
 }
 
-function scanReactRouterApiVersion(file: SourceFile, referenceDate: Date): Issue[] {
-  if (!file.content || !/^app\/shopify\.server\.[cm]?[jt]sx?$/.test(file.path)) return []
+function scanReactRouterApiVersion(file: SourceFile, reactRouterRoots: string[], referenceDate: Date): Issue[] {
+  if (!file.content || !reactRouterRoots.some((root) => isReactRouterServerPath(root, file.path))) return []
   const content = maskStringsExceptVersions(stripComments(file.content))
   const declarations = [
     ...content.matchAll(/\bapiVersion\s*:\s*["'](\d{4}-(?:01|04|07|10))["']/g),

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

@@ -55,6 +55,10 @@ export interface ScanContext {
   capabilities: Capabilities
   /** Framework, surface, and language inventory. */
   detection: ProjectDetection
+  /** React Router app roots relative to the app root, with `.` for the app root. Empty unless the framework is React Router. */
+  reactRouterRoots: string[]
+  /** Other apps' directories relative to the app root, with forward slashes. */
+  otherAppDirectories: string[]
   /** Path-only inventory, including unsupported source candidates. */
   sourceCandidates: SourceCandidate[]
   /** Outcome of listing git's ignored paths for the first scan directory. */