From 9aad542ea1503dc069646e957f1e9fe40f67cdcb Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Tue, 15 Sep 2026 12:26:45 -0500 Subject: [PATCH 1/4] Add GitHub Actions dependency-auditing check to App Doctor Co-authored-by: AI (Pi/GPT-6 Astra) --- .../add-app-doctor-dependency-auditing.md | 5 + packages/app/package.json | 3 +- .../app-doctor-engine/rules/catalog.ts | 8 + .../dependency-auditing-github-schema.ts | 57 ++ .../rules/dependency-auditing-rules.ts | 737 ++++++++++++++++++ .../services/app-doctor-engine/rules/types.ts | 12 +- .../app-doctor-engine/scanners/discover.ts | 224 +++++- .../app-doctor-engine/scanners/index.ts | 57 +- .../app-doctor-engine/scanners/types.ts | 9 + .../dependency-auditing-discovery.test.ts | 261 +++++++ .../tests/dependency-auditing-rules.test.ts | 465 +++++++++++ .../tests/dependency-auditing.test.ts | 344 ++++++++ .../tests/deterministic-rules.test.ts | 3 +- .../tests/rule-analysis.test.ts | 1 + pnpm-lock.yaml | 3 + 15 files changed, 2174 insertions(+), 15 deletions(-) create mode 100644 .changeset/add-app-doctor-dependency-auditing.md create mode 100644 packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-github-schema.ts create mode 100644 packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-rules.ts create mode 100644 packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-discovery.test.ts create mode 100644 packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-rules.test.ts create mode 100644 packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing.test.ts diff --git a/.changeset/add-app-doctor-dependency-auditing.md b/.changeset/add-app-doctor-dependency-auditing.md new file mode 100644 index 00000000000..0aa19137288 --- /dev/null +++ b/.changeset/add-app-doctor-dependency-auditing.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +Add an App Doctor check for dependency vulnerability auditing in GitHub Actions. diff --git a/packages/app/package.json b/packages/app/package.json index 6176cc4a125..b8707057712 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -78,7 +78,8 @@ "proper-lockfile": "4.1.2", "react": "19.2.4", "which": "4.0.0", - "ws": "8.21.0" + "ws": "8.21.0", + "yaml": "2.9.0" }, "devDependencies": { "@types/diff": "^5.0.3", diff --git a/packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts b/packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts index 1ff1fd4e694..c8f2b98bde9 100644 --- a/packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts +++ b/packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts @@ -145,6 +145,14 @@ export const RULE_CATALOG: RuleCatalogEntry[] = [ fix: 'Add the missing webhook subscriptions to shopify.app.toml.', guide: 'https://shopify.dev/docs/apps/webhooks/configuration/mandatory-webhooks', }, + { + id: 'MISSING_DEPENDENCY_AUDITING', + title: 'Dependency auditing configuration not detected', + severity: 'low', + points: -5, + description: 'Detects apps with dependencies but no recognized automated dependency vulnerability auditing.', + fix: 'Add dependency vulnerability auditing to an allowlisted CI configuration for the app.', + }, { id: 'EOL_API_VERSION', title: 'End-of-life API version', diff --git a/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-github-schema.ts b/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-github-schema.ts new file mode 100644 index 00000000000..a7d22f3fe04 --- /dev/null +++ b/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-github-schema.ts @@ -0,0 +1,57 @@ +import {zod} from '@shopify/cli-kit/node/schema' + +// Validate executable children separately: disabled jobs, unused definitions, and +// overridden defaults must not prevent analysis of the configuration we inspect. +export const GitHubWorkflowSchema = zod.object({ + jobs: zod.record(zod.unknown()).optional(), + defaults: zod.unknown(), +}) + +export const GitHubReusableDefinitionSchema = zod.object({ + on: zod.union([ + zod.literal('workflow_call'), + zod.tuple([zod.literal('workflow_call')]), + zod.record(zod.unknown()).refine((events) => Object.keys(events).length === 1 && 'workflow_call' in events), + ]), +}) + +// Headers select which schema to apply without validating disabled execution paths. +export const GitHubJobHeaderSchema = zod.object({if: zod.unknown(), uses: zod.unknown()}) +export const GitHubStepsJobSchema = zod.object({steps: zod.array(zod.unknown()), defaults: zod.unknown()}) +export const GitHubReusableJobSchema = zod.object({uses: zod.string(), with: zod.unknown()}) +export const GitHubStepHeaderSchema = GitHubJobHeaderSchema.extend({run: zod.unknown()}) + +export const GitHubRunStepSchema = zod.object({ + run: zod.string(), + // Selection and shell/path validation happen after applying defaults and overrides. + shell: zod.unknown(), + 'working-directory': zod.unknown(), +}) +export const GitHubActionStepSchema = zod.object({ + if: zod.unknown(), + uses: zod.string(), + with: zod.record(zod.unknown()).optional(), +}) + +export const GitHubDefaultsSchema = zod + .object({ + run: zod.object({shell: zod.unknown(), 'working-directory': zod.unknown()}).optional(), + }) + .default({}) + +// OSV inputs can change scan coverage. Do not strip unknown keys before checking them. +export const OsvWorkflowInputsSchema = zod.object({}).strict().optional() + +export const DependencyReviewInputsSchema = zod + .object({ + 'config-file': zod.unknown(), + 'vulnerability-check': zod.unknown(), + }) + .default({}) + +export const SnykCommandInputsSchema = zod.object({command: zod.string().optional()}).default({}) +export const SnykArgsInputsSchema = zod.object({args: zod.string().default('')}).default({}) + +export type GitHubRunStep = zod.infer +export type GitHubActionStep = zod.infer +export type GitHubDefaults = zod.infer diff --git a/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-rules.ts b/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-rules.ts new file mode 100644 index 00000000000..eb8b35f7e9e --- /dev/null +++ b/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-rules.ts @@ -0,0 +1,737 @@ +import { + DependencyReviewInputsSchema, + GitHubActionStepSchema, + GitHubDefaultsSchema, + GitHubJobHeaderSchema, + GitHubReusableDefinitionSchema, + GitHubReusableJobSchema, + GitHubRunStepSchema, + GitHubStepHeaderSchema, + GitHubStepsJobSchema, + GitHubWorkflowSchema, + OsvWorkflowInputsSchema, + SnykArgsInputsSchema, + SnykCommandInputsSchema, +} from './dependency-auditing-github-schema.js' +import {parseDocument} from 'yaml' +import type {GitHubActionStep, GitHubDefaults, GitHubRunStep} from './dependency-auditing-github-schema.js' +import type {Issue} from '../types.js' +import type {ManifestFile, ScanContext, SourceFile} from './types.js' + +interface Analysis { + recognized: boolean + obstacles: string[] +} + +interface CommandContext { + directory: string + manifests: ManifestFile[] + allowPackageScript: boolean +} + +type ParsedConfiguration = {ok: true; value: unknown; warnings: string[]} | {ok: false; reason: string} +type ResolvedValue = {ok: true; value: string} | {ok: false} +type OptionalValue = {ok: true; present: boolean; value?: unknown} | {ok: false} +type ParsedShell = {ok: true; segments: string[]} | {ok: false} + +const SUPPORTED_PACKAGE_MANAGERS = new Set(['javascript', 'node', 'npm', 'pnpm', 'yarn']) +const LOCAL_SCRIPT_EXECUTORS = new Set(['.', 'bash', 'node', 'sh', 'source', 'tsx']) +const SHELL_CONTROL_WORDS = new Set([ + 'break', + 'case', + 'continue', + 'do', + 'done', + 'elif', + 'else', + 'eval', + 'exit', + 'false', + 'fi', + 'for', + 'function', + 'if', + 'return', + 'set', + 'then', + 'trap', + 'until', + 'while', +]) +const MAX_YAML_ALIASES = 20 + +/** + * Recognizes a deliberately small set of dependency-auditing executions. + * Unknown indirection is left unresolved rather than interpreted as shell or CI code. + */ +export function scanDependencyAuditing(context: Pick): { + issues: Issue[] + unresolvedReason?: string +} { + const applicableManifests = context.manifests.filter(hasDependencies) + if (applicableManifests.length === 0) return {issues: []} + if (context.dependencyAuditing.unresolvedReason) { + return {issues: [], unresolvedReason: context.dependencyAuditing.unresolvedReason} + } + + const parsedFiles = context.dependencyAuditing.files.map((file) => ({file, parsed: parseConfiguration(file)})) + const malformed = parsedFiles.find(({parsed}) => !parsed.ok) + if (malformed && !malformed.parsed.ok) return {issues: [], unresolvedReason: malformed.parsed.reason} + + const analysis = parsedFiles.reduce((combined, {file, parsed}) => { + if (!parsed.ok) return combined + return combine(combined, analyzeConfiguration(file, parsed, context.manifests)) + }, noEvidence()) + if (analysis.recognized) return {issues: []} + if (analysis.obstacles.length > 0) return {issues: [], unresolvedReason: analysis.obstacles[0]} + + const manifest = [...applicableManifests].sort((left, right) => left.path.localeCompare(right.path))[0]! + return { + issues: [ + { + id: 'MISSING_DEPENDENCY_AUDITING', + severity: 'low', + points: -5, + title: 'Dependency auditing configuration not detected', + message: + 'No recognized dependency-auditing configuration was found in the inspected files. Add an automated dependency vulnerability check, or verify that your existing integration covers this app. Hosted integrations and repository settings were not inspected.', + location: {file: normalizePath(manifest.path)}, + fix: { + automated: false, + description: 'Add an automated dependency vulnerability check for this package.', + }, + }, + ], + } +} + +function hasDependencies(manifest: ManifestFile): boolean { + return Object.keys(manifest.dependencies).length > 0 || Object.keys(manifest.devDependencies ?? {}).length > 0 +} + +function analyzeConfiguration( + file: SourceFile, + parsed: Extract, + manifests: ManifestFile[], +): Analysis { + const path = normalizePath(file.path) + const warnings = parsed.warnings.reduce( + (analysis, warning) => combine(analysis, obstacle(`${warning}: ${path}`)), + noEvidence(), + ) + if (parsed.warnings.length > 0 || parsed.value === null || parsed.value === undefined) return warnings + if (/^\.github\/workflows\/[^/]+\.ya?ml$/i.test(path)) { + return combine(warnings, analyzeGitHubWorkflow(parsed.value, path, manifests)) + } + return warnings +} + +function parseConfiguration(file: SourceFile): ParsedConfiguration { + if (file.content === undefined) { + return {ok: false, reason: `Dependency-auditing configuration was unreadable: ${file.path}`} + } + try { + const document = parseDocument(file.content, {schema: 'core', merge: true, uniqueKeys: true}) + if (document.errors.length > 0) { + return {ok: false, reason: `Dependency-auditing YAML is malformed: ${file.path}`} + } + const value = document.toJS({maxAliasCount: MAX_YAML_ALIASES}) as unknown + const warnings = document.warnings.map(() => 'Unsupported YAML feature prevents complete auditing analysis') + return {ok: true, value, warnings} + // YAML conversion may reject excessive aliases or invalid custom structures. + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + return {ok: false, reason: `Dependency-auditing YAML could not be parsed safely: ${file.path}`} + } +} + +function analyzeGitHubWorkflow(value: unknown, path: string, manifests: ManifestFile[]): Analysis { + // A reusable-only workflow is a definition, not evidence of invocation. Local callers remain unresolved. + if (GitHubReusableDefinitionSchema.safeParse(value).success) return noEvidence() + const workflowResult = GitHubWorkflowSchema.safeParse(value) + if (!workflowResult.success) return obstacle(`GitHub Actions workflow has an unsupported structure: ${path}`) + const root = workflowResult.data + if (root.jobs === undefined) return noEvidence() + + let analysis = noEvidence() + const workflowDefaults = GitHubDefaultsSchema.safeParse(root.defaults) + const workflowDirectory: OptionalValue = workflowDefaults.success + ? readGitHubDefaultDirectory(workflowDefaults.data) + : {ok: false} + for (const jobValue of Object.values(root.jobs)) { + const jobResult = GitHubJobHeaderSchema.safeParse(jobValue) + if (!jobResult.success) { + analysis = combine(analysis, obstacle(`GitHub Actions workflow has an unsupported job in ${path}`)) + continue + } + const job = jobResult.data + if (isDisabled(job.if)) continue + if (job.uses !== undefined) { + const reusableJob = GitHubReusableJobSchema.safeParse(jobValue) + if (!reusableJob.success) { + analysis = combine(analysis, obstacle(`GitHub Actions reusable workflow has an unsupported structure: ${path}`)) + } else if (isKnownOsvWorkflow(reusableJob.data.uses)) { + analysis = combine(analysis, analyzeOsvWorkflow(reusableJob.data.with, path)) + } else { + analysis = combine(analysis, obstacle(`Unsupported reusable workflow referenced by ${path}`)) + } + continue + } + + const stepsJob = GitHubStepsJobSchema.safeParse(jobValue) + if (!stepsJob.success) { + analysis = combine(analysis, obstacle(`GitHub Actions workflow has an unsupported steps section: ${path}`)) + continue + } + if (stepsJob.data.steps.some(hasUnsupportedCheckout)) { + analysis = combine(analysis, obstacle(`Unsupported checkout repository or path in ${path}`)) + continue + } + const jobDefaults = GitHubDefaultsSchema.safeParse(stepsJob.data.defaults) + const jobDirectory: OptionalValue = jobDefaults.success ? readGitHubDefaultDirectory(jobDefaults.data) : {ok: false} + for (const stepValue of stepsJob.data.steps) { + const stepResult = GitHubStepHeaderSchema.safeParse(stepValue) + if (!stepResult.success) { + analysis = combine(analysis, obstacle(`GitHub Actions workflow has an unsupported step in ${path}`)) + continue + } + const step = stepResult.data + if (isDisabled(step.if)) continue + if (step.uses !== undefined) { + const action = GitHubActionStepSchema.safeParse(stepValue) + if (action.success) { + // defaults.run and working-directory apply only to run steps. Actions execute from checkout root. + analysis = combine(analysis, analyzeGitHubAction(action.data, path, manifests)) + } else { + analysis = combine(analysis, obstacle(`GitHub Actions workflow has an unsupported action step in ${path}`)) + } + } + if (step.run !== undefined) { + const run = GitHubRunStepSchema.safeParse(stepValue) + if (!run.success) { + analysis = combine(analysis, obstacle(`GitHub Actions workflow has an unsupported run step in ${path}`)) + continue + } + const shell = + run.data.shell ?? + (jobDefaults.success ? jobDefaults.data.run?.shell : undefined) ?? + (workflowDefaults.success ? workflowDefaults.data.run?.shell : undefined) + if (shell !== undefined && (typeof shell !== 'string' || !/^(?:bash|sh|cmd|pwsh|powershell)$/.test(shell))) { + analysis = combine(analysis, obstacle(`Unsupported execution shell in ${path}`)) + continue + } + const directory = resolveGitHubRunDirectory(run.data, jobDirectory, workflowDirectory) + analysis = combine( + analysis, + directory.ok + ? analyzeCommands(run.data.run, {directory: directory.value, manifests, allowPackageScript: true}, path) + : obstacle(`Unsupported working-directory prevents auditing analysis in ${path}`), + ) + } + } + } + return analysis +} + +function hasUnsupportedCheckout(value: unknown): boolean { + const result = GitHubActionStepSchema.safeParse(value) + if (!result.success || isDisabled(result.data.if) || !/^actions\/checkout@/i.test(result.data.uses)) return false + const inputs = result.data.with + return ( + inputs?.repository !== undefined || + (inputs?.path !== undefined && inputs.path !== '.' && inputs.path !== './') || + inputs?.['sparse-checkout'] !== undefined + ) +} + +function analyzeOsvWorkflow(inputs: unknown, path: string): Analysis { + return OsvWorkflowInputsSchema.safeParse(inputs).success + ? evidence() + : obstacle(`Unsupported OSV reusable workflow input referenced by ${path}`) +} + +function analyzeGitHubAction(step: GitHubActionStep, workflowPath: string, manifests: ManifestFile[]): Analysis { + const uses = step.uses.toLowerCase() + if (/^actions\/dependency-review-action@[^\s]+$/i.test(uses)) { + const result = DependencyReviewInputsSchema.safeParse(step.with) + if (!result.success) return obstacle(`Unsupported dependency review input in ${workflowPath}`) + const inputs = result.data + if (inputs['config-file'] !== undefined) { + return obstacle(`Dependency review config-file can't be inspected from ${workflowPath}`) + } + const vulnerabilityCheck = inputs?.['vulnerability-check'] + if (isDisabled(vulnerabilityCheck)) return noEvidence() + if (vulnerabilityCheck !== undefined && vulnerabilityCheck !== true && vulnerabilityCheck !== 'true') { + return obstacle(`Unsupported dependency review input in ${workflowPath}`) + } + return evidence() + } + if (/^snyk\/actions\/node(?:-\d+)?@[^\s]+$/i.test(uses)) { + const command = SnykCommandInputsSchema.safeParse(step.with) + if (!command.success) return obstacle(`Unsupported Snyk command input in ${workflowPath}`) + const commandValue = command.data.command + if (commandValue !== undefined && isDynamic(commandValue)) { + return obstacle(`Dynamic Snyk command input in ${workflowPath}`) + } + if (commandValue !== undefined && commandValue.trim() !== 'test') return noEvidence() + const argsResult = SnykArgsInputsSchema.safeParse(step.with) + if (!argsResult.success) return obstacle(`Unsupported Snyk args input in ${workflowPath}`) + const args = argsResult.data.args + const parsedArgs = splitShell(args) + if (!parsedArgs.ok || parsedArgs.segments.length > 1 || isDynamic(args)) { + return obstacle(`Unsupported Snyk args input in ${workflowPath}`) + } + if (/\b(?:code|container)\s+test\b/i.test(args)) return noEvidence() + return analyzeCommands(`snyk test ${args}`, {directory: '', manifests, allowPackageScript: false}, workflowPath) + } + if (/^snyk\/actions\/(?:setup|code|docker|container)@[^\s]+$/i.test(uses)) return noEvidence() + if (uses.startsWith('./')) return obstacle(`Unsupported local action referenced by ${workflowPath}`) + return noEvidence() +} + +function analyzeCommands(command: string, context: CommandContext, configurationPath: string): Analysis { + const parsedShell = splitShell(command) + if (!parsedShell.ok) return obstacle(`Unsupported shell syntax referenced by ${configurationPath}`) + + const segments = parsedShell.segments.map((segment) => ({segment, tokens: tokenize(segment)})) + if ( + segments.some( + ({segment, tokens}) => + segment.includes('`') || segment.includes('$(') || SHELL_CONTROL_WORDS.has(firstExecutable(tokens) ?? ''), + ) + ) { + return obstacle(`Unsupported shell control flow referenced by ${configurationPath}`) + } + + let analysis = noEvidence() + let directory = normalizeDirectory(context.directory) + for (const {tokens} of segments) { + if (tokens.length === 0) continue + const executableIndex = tokens.findIndex((token) => !/^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) + if (executableIndex < 0) continue + const executable = tokens[executableIndex]! + const commandTokens = tokens.slice(executableIndex) + if (executable === 'echo' || executable === 'printf') continue + if (executable === 'cd') { + const target = commandTokens[1] + const resolved = target === undefined ? undefined : resolveDirectory(directory, target) + if (resolved === undefined) { + analysis = combine(analysis, obstacle(`Unsupported shell directory in ${configurationPath}`)) + } else { + directory = resolved + } + continue + } + if (isPotentialUnsupportedPackageInvocation(commandTokens)) { + analysis = combine(analysis, obstacle(`Unsupported package manager selector in ${configurationPath}`)) + continue + } + if (isRecognizedAuditCommand(commandTokens)) { + if (hasHelpOrVersion(commandTokens)) continue + const packageManagerOption = selectedOption(commandTokens, ['--package-manager']) + if (packageManagerOption.present) { + if (!isUsableOptionValue(packageManagerOption.value) || isDynamic(packageManagerOption.value)) { + analysis = combine(analysis, obstacle(`Unsupported scanner package manager in ${configurationPath}`)) + continue + } + if (!SUPPORTED_PACKAGE_MANAGERS.has(packageManagerOption.value.toLowerCase())) continue + } + const targetOption = selectedOption(commandTokens, scannerTargetOptionNames(commandTokens[0])) + if ( + targetOption.present && + (!isUsableOptionValue(targetOption.value) || !isSafeRelativePath(targetOption.value)) + ) { + analysis = combine(analysis, obstacle(`Unsupported scanner target in ${configurationPath}`)) + } else if (commandCoversManifest(commandTokens, directory, targetOption.value, context.manifests)) { + analysis = combine(analysis, evidence()) + } + continue + } + + const packageScript = parsePackageScript(commandTokens) + if (packageScript.kind === 'unsupported') { + analysis = combine(analysis, obstacle(`Unsupported package script invocation in ${configurationPath}`)) + continue + } + if (packageScript.kind === 'script') { + if (!context.allowPackageScript) { + analysis = combine(analysis, obstacle(`Nested package script referenced by ${configurationPath}`)) + continue + } + const scriptDirectory = packageScript.target ? resolveDirectory(directory, packageScript.target) : directory + if (scriptDirectory === undefined) { + analysis = combine(analysis, obstacle(`Unsupported package script target in ${configurationPath}`)) + continue + } + const selectedManifest = manifestInDirectory(context.manifests, scriptDirectory) + const script = selectedManifest?.scripts?.[packageScript.name] + if (!selectedManifest || script === undefined) { + analysis = combine(analysis, obstacle(`Package script can't be resolved in ${configurationPath}`)) + continue + } + analysis = combine( + analysis, + analyzeCommands( + script, + {directory: manifestDirectory(selectedManifest), manifests: context.manifests, allowPackageScript: false}, + configurationPath, + ), + ) + continue + } + + if (isDynamic(executable) || referencesLocalScript(commandTokens) || invokesShellCommand(commandTokens)) { + analysis = combine(analysis, obstacle(`Unsupported local script referenced by ${configurationPath}`)) + } + } + + // An unsupported construct in this execution block may govern any command in that block. + return analysis.obstacles.length > 0 ? {recognized: false, obstacles: analysis.obstacles} : analysis +} + +function isRecognizedAuditCommand(tokens: string[]): boolean { + const words = tokens.map((token) => token.toLowerCase()) + const executable = words[0] + if (executable === 'npm' || executable === 'pnpm') return packageManagerCommand(words) === 'audit' + if (executable === 'yarn') { + const commandWords = packageManagerCommandWords(words) + return commandWords[0] === 'audit' || (commandWords[0] === 'npm' && commandWords[1] === 'audit') + } + let snykIndex = -1 + if (executable === 'snyk') snykIndex = 0 + else if (executable === 'npx' && words[1] === 'snyk') snykIndex = 1 + if (snykIndex >= 0) return words[snykIndex + 1] === 'test' + return executable === 'semgrep' && words[1] === 'ci' && words.includes('--supply-chain') +} + +function packageManagerCommand(tokens: string[]): string | undefined { + return packageManagerCommandWords(tokens)[0] +} + +function packageManagerCommandWords(tokens: string[]): string[] { + const words = tokens.slice(1) + const manager = tokens[0] + const supportedOptions = packageManagerDirectoryOptionNames(manager) + while (words[0]?.startsWith('-')) { + const option = words.shift()! + if (!option.includes('=') && supportedOptions.includes(option)) words.shift() + } + return words +} + +type PackageScript = {kind: 'none'} | {kind: 'unsupported'} | {kind: 'script'; name: string; target?: string} + +function parsePackageScript(tokens: string[]): PackageScript { + const manager = tokens[0]?.toLowerCase() + if (!manager || !['npm', 'pnpm', 'yarn'].includes(manager)) return {kind: 'none'} + const targetOption = selectedOption(tokens, scannerTargetOptionNames(manager)) + if (targetOption.present && (!isUsableOptionValue(targetOption.value) || !isSafeRelativePath(targetOption.value))) { + return {kind: 'unsupported'} + } + const target = targetOption.value + const words = packageManagerCommandWords(tokens.map((token) => token.toLowerCase())) + const commandIndex = tokens.length - words.length + if (['install', 'add', 'remove', 'update', 'upgrade', 'login', 'logout', 'config'].includes(words[0] ?? '')) { + return {kind: 'none'} + } + if (words[0] === 'run' || words[0] === 'run-script') { + const scriptName = tokens[commandIndex + 1] + if (words.length !== 2 || !scriptName || !/^[A-Za-z0-9_.:-]+$/.test(scriptName)) { + return {kind: 'unsupported'} + } + return {kind: 'script', name: scriptName, target} + } + if ( + (manager === 'pnpm' || manager === 'yarn') && + words.length === 1 && + words[0] && + /^[A-Za-z0-9_.:-]+$/.test(words[0]) + ) { + return {kind: 'script', name: tokens[commandIndex]!, target} + } + return {kind: 'none'} +} + +function isPotentialUnsupportedPackageInvocation(tokens: string[]): boolean { + const manager = tokens[0]?.toLowerCase() + if (!manager || !['npm', 'pnpm', 'yarn'].includes(manager)) return false + const lower = tokens.map((token) => token.toLowerCase()) + const invokesAuditOrScript = lower.includes('audit') || lower.includes('run') || lower.includes('run-script') + if (!invokesAuditOrScript) return false + if ( + lower.some( + (token) => /^(?:--filter|--workspace|--workspaces|--recursive)(?:=|$)/i.test(token) || /^-[frw]/i.test(token), + ) + ) { + return true + } + + const supportedOptions = packageManagerDirectoryOptionNames(manager) + for (let index = 1; index < lower.length && lower[index]?.startsWith('-'); index++) { + const optionName = lower[index]!.split('=')[0]! + if (!supportedOptions.includes(optionName)) return true + if (!lower[index]!.includes('=')) index++ + } + return false +} + +function packageManagerDirectoryOptionNames(manager: string | undefined): string[] { + if (manager === 'npm') return ['--prefix'] + if (manager === 'pnpm') return ['--dir', '-c'] + return ['--cwd'] +} + +function scannerTargetOptionNames(executable: string | undefined): string[] { + const normalizedExecutable = executable?.toLowerCase() + if (normalizedExecutable === 'npm') return ['--file', '--target-file', '--prefix'] + if (normalizedExecutable === 'pnpm') return ['--file', '--target-file', '--dir', '-c'] + if (normalizedExecutable === 'yarn') return ['--file', '--target-file', '--cwd'] + return ['--file', '--target-file'] +} + +function invokesShellCommand(tokens: string[]): boolean { + return ['bash', 'sh'].includes(tokens[0]?.toLowerCase() ?? '') && tokens.slice(1).some((token) => token === '-c') +} + +function referencesLocalScript(tokens: string[]): boolean { + const executable = tokens[0]?.toLowerCase() + if (!executable) return false + if (/^(?:\.\.?\/|\/).+/.test(executable)) return true + if (!LOCAL_SCRIPT_EXECUTORS.has(executable)) return false + return tokens.slice(1).some((token) => /(?:^|\/)[^\s]+\.(?:[cm]?[jt]sx?|sh)$/.test(token)) +} + +function commandCoversManifest( + tokens: string[], + directory: string, + target: string | undefined, + manifests: ManifestFile[], +): boolean { + if (target) return targetCoversManifest(directory, target, manifests) + const executable = tokens[0]?.toLowerCase() + const scansRecursively = + (executable === 'semgrep' && tokens.includes('--supply-chain')) || tokens.includes('--all-projects') + if (!scansRecursively) return targetCoversManifest(directory, undefined, manifests) + const normalizedDirectory = normalizeDirectory(directory) + return manifests.some((manifest) => { + if (!hasDependencies(manifest)) return false + const candidateDirectory = manifestDirectory(manifest) + return ( + !normalizedDirectory || + candidateDirectory === normalizedDirectory || + candidateDirectory.startsWith(`${normalizedDirectory}/`) + ) + }) +} + +function targetCoversManifest( + directory: string | undefined, + target: string | undefined, + manifests: ManifestFile[], +): boolean { + const baseDirectory = normalizeDirectory(directory ?? '') + if (!target) { + return manifests.some((manifest) => hasDependencies(manifest) && manifestDirectory(manifest) === baseDirectory) + } + const normalizedTarget = resolveDirectory(baseDirectory, target) + if (normalizedTarget === undefined) return false + const targetBasename = normalizedTarget.split('/').at(-1) ?? '' + const targetDirectory = /^(?:package\.json|package-lock\.json|pnpm-lock\.yaml|yarn\.lock)$/i.test(targetBasename) + ? normalizeDirectory(normalizedTarget.slice(0, -(targetBasename.length + (normalizedTarget.includes('/') ? 1 : 0)))) + : normalizedTarget + return manifests.some((manifest) => hasDependencies(manifest) && manifestDirectory(manifest) === targetDirectory) +} + +function manifestInDirectory(manifests: ManifestFile[], directory: string): ManifestFile | undefined { + return manifests.find((manifest) => manifestDirectory(manifest) === normalizeDirectory(directory)) +} + +function manifestDirectory(manifest: ManifestFile): string { + const path = normalizePath(manifest.path) + return normalizeDirectory(path.includes('/') ? path.slice(0, path.lastIndexOf('/')) : '') +} + +function resolveGitHubRunDirectory( + step: GitHubRunStep, + jobDirectory: OptionalValue, + workflowDirectory: OptionalValue, +): ResolvedValue { + if (step['working-directory'] !== undefined) return resolveSelectedDirectory(step['working-directory']) + if (!jobDirectory.ok) return {ok: false} + if (jobDirectory.present) return resolveSelectedDirectory(jobDirectory.value) + if (!workflowDirectory.ok) return {ok: false} + return resolveSelectedDirectory(workflowDirectory.value) +} + +function readGitHubDefaultDirectory(defaults: GitHubDefaults): OptionalValue { + const directory = defaults.run?.['working-directory'] + return {ok: true, present: directory !== undefined, value: directory} +} + +function resolveSelectedDirectory(value: unknown): ResolvedValue { + if (value === undefined) return {ok: true, value: ''} + if (typeof value !== 'string' || !isSafeRelativePath(value)) return {ok: false} + return {ok: true, value: normalizeDirectory(value)} +} + +function normalizeDirectory(value: string): string { + return normalizePath(value) + .split('/') + .filter((part) => part && part !== '.') + .join('/') +} + +function resolveDirectory(base: string, target: string): string | undefined { + if (!isSafeRelativePath(target)) return undefined + const normalizedBase = normalizeDirectory(base) + const normalizedTarget = normalizeDirectory(target) + return [normalizedBase, normalizedTarget].filter(Boolean).join('/') +} + +function isSafeRelativePath(value: string): boolean { + const normalized = normalizePath(value.trim()) + if (!normalized || normalized === '.') return true + if ( + isDynamic(normalized) || + normalized.startsWith('/') || + /^[A-Za-z]:\//.test(normalized) || + normalized.startsWith('~/') + ) { + return false + } + return !normalized.split('/').includes('..') +} + +function normalizePath(path: string): string { + return path.replace(/\\/g, '/').replace(/^\.\//, '') +} + +function isKnownOsvWorkflow(uses: string): boolean { + return /^google\/osv-scanner\/\.github\/workflows\/osv-scanner-reusable(?:-pr)?\.yml@[^\s]+$/i.test(uses) +} + +function isDisabled(value: unknown): boolean { + return value === false || (typeof value === 'string' && /^(?:false|\$\{\{\s*false\s*\}\})$/i.test(value.trim())) +} + +function isDynamic(value: string): boolean { + return /\$|`|\{\{/.test(value) +} + +function selectedOption(tokens: string[], names: string[]): {present: boolean; value?: string} { + for (let index = 0; index < tokens.length; index++) { + const token = tokens[index]! + for (const name of names) { + if (token.toLowerCase() === name) return {present: true, value: tokens[index + 1]} + if (token.toLowerCase().startsWith(`${name}=`)) { + return {present: true, value: token.slice(name.length + 1)} + } + } + } + return {present: false} +} + +function isUsableOptionValue(value: string | undefined): value is string { + return value !== undefined && value.length > 0 && !value.startsWith('-') +} + +function hasHelpOrVersion(tokens: string[]): boolean { + return tokens.some((token) => ['--help', '--version', '-h', '-v'].includes(token.toLowerCase())) +} + +function firstExecutable(tokens: string[]): string | undefined { + return tokens.find((token) => !/^[A-Za-z_][A-Za-z0-9_]*=/.test(token))?.toLowerCase() +} + +/** Split only sequential basic shell statements. Conditional operators and ambiguous comments are unsupported. */ +function splitShell(command: string): ParsedShell { + const segments: string[] = [] + let current = '' + let quote: string | undefined + let escaped = false + let comment = false + for (const character of command) { + if (comment) { + if (character === '\\') return {ok: false} + if (character === '\n') comment = false + else continue + } + if (escaped) { + if (character !== '\n') return {ok: false} + escaped = false + continue + } + if (character === '\\' && quote !== "'") { + escaped = true + continue + } + if (quote) { + current += character + if (character === quote) quote = undefined + continue + } + if (character === "'" || character === '"') { + quote = character + current += character + continue + } + if (character === '#' && (current.length === 0 || /\s/.test(current.at(-1)!))) { + comment = true + continue + } + // Redirections, heredocs, subshells, and function bodies require a shell interpreter. + if ('|&<>(){}'.includes(character)) return {ok: false} + if (character === '\n' || character === ';') { + if (current.trim()) segments.push(current.trim()) + current = '' + continue + } + current += character + } + if (quote || escaped) return {ok: false} + if (current.trim()) segments.push(current.trim()) + return {ok: true, segments} +} + +function tokenize(command: string): string[] { + const tokens: string[] = [] + let current = '' + let quote: string | undefined + let escaped = false + for (const character of command) { + if (escaped) { + current += character + escaped = false + } else if (character === '\\' && quote !== "'") { + escaped = true + } else if (quote) { + if (character === quote) quote = undefined + else current += character + } else if (character === "'" || character === '"') { + quote = character + } else if (/\s/.test(character)) { + if (current) tokens.push(current) + current = '' + } else { + current += character + } + } + if (current) tokens.push(current) + return tokens +} + +function combine(left: Analysis, right: Analysis): Analysis { + return {recognized: left.recognized || right.recognized, obstacles: [...left.obstacles, ...right.obstacles]} +} + +function evidence(): Analysis { + return {recognized: true, obstacles: []} +} + +function obstacle(reason: string): Analysis { + return {recognized: false, obstacles: [reason]} +} + +function noEvidence(): Analysis { + return {recognized: false, obstacles: []} +} diff --git a/packages/app/src/cli/services/app-doctor-engine/rules/types.ts b/packages/app/src/cli/services/app-doctor-engine/rules/types.ts index bbd93d3f3af..a3f39c53b79 100644 --- a/packages/app/src/cli/services/app-doctor-engine/rules/types.ts +++ b/packages/app/src/cli/services/app-doctor-engine/rules/types.ts @@ -1,5 +1,11 @@ import type {Issue, Capabilities, ProjectDetection, Severity, SourceCandidate} from '../types.js' -import type {AppTomlContent, ExtensionInfo, ManifestFile, SourceFile} from '../scanners/types.js' +import type { + AppTomlContent, + DependencyAuditingInputs, + ExtensionInfo, + ManifestFile, + SourceFile, +} from '../scanners/types.js' export type {AppTomlContent, ExtensionInfo, ManifestFile, SourceFile} from '../scanners/types.js' @@ -38,8 +44,10 @@ export interface ScanContext { extensions: ExtensionInfo[] /** Source files read for supported non-secret deterministic analysis. */ sourceFiles: SourceFile[] - /** Package manifest files found (package.json, Gemfile, composer.json) */ + /** Supported JavaScript package manifests found. */ manifests: ManifestFile[] + /** Allowlisted dependency-auditing configuration and discovery obstacles. */ + dependencyAuditing: DependencyAuditingInputs /** Safely readable repository text evidence available to secret scanning. */ sensitiveFiles: SourceFile[] /** Detected capabilities */ diff --git a/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts b/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts index e8676379371..559387a8003 100644 --- a/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts +++ b/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts @@ -3,12 +3,28 @@ import {AppAccessScopesSchema, AppAuthSchema} from '../../../models/extensions/s 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 {basename, cwd, dirname, extname, joinPath, relativePath, resolvePath} from '@shopify/cli-kit/node/path' +import { + basename, + cwd, + dirname, + extname, + isSubpath, + joinPath, + relativePath, + resolvePath, +} from '@shopify/cli-kit/node/path' import {zod} from '@shopify/cli-kit/node/schema' import {decodeToml} from '@shopify/cli-kit/node/toml/codec' -import {lstatSync} from 'node:fs' +import {lstatSync, readdirSync, realpathSync} from 'node:fs' import type {SourceCandidate} from '../types.js' -import type {AppTomlContent, ExtensionInfo, SourceFile, ManifestFile, WebhookSubscription} from './types.js' +import type { + AppTomlContent, + DependencyAuditingInputs, + ExtensionInfo, + SourceFile, + ManifestFile, + WebhookSubscription, +} from './types.js' /** Expected user error while locating a Shopify app root. */ export class AppRootDiscoveryError extends Error { @@ -412,13 +428,46 @@ function readBoundedFile(path: string): RepositoryReadResult { } } +function repositoryPathFailure(detail: string): RepositoryReadFailure { + return {ok: false, reason: 'unreadable', detail} +} + +function containedRepositoryPath(appRoot: string, path: string): {path?: string; failure?: RepositoryReadFailure} { + const absoluteRoot = resolvePath(appRoot) + const absolutePath = resolvePath(path) + if (!isSubpath(absoluteRoot, absolutePath)) { + return {failure: repositoryPathFailure('path escapes the app root')} + } + + try { + const canonicalRoot = realpathSync(absoluteRoot) + const canonicalPath = realpathSync(absolutePath) + if (!isSubpath(canonicalRoot, canonicalPath)) { + return {failure: repositoryPathFailure('symbolic link escapes the app root')} + } + if (!lstatSync(canonicalPath).isFile()) { + return {failure: repositoryPathFailure('path is not a regular file')} + } + return {path: canonicalPath} + // Dangling links and paths whose metadata cannot be read are coverage gaps. + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + return { + failure: repositoryPathFailure(error instanceof Error ? error.message : String(error)), + } + } +} + function readRepositoryFile(appRoot: string, path: string): RepositoryReadResult { + const absoluteRoot = resolvePath(appRoot) const absolutePath = resolvePath(path) - const cached = repositoryFileCache.get(absolutePath) + const cacheKey = `${absoluteRoot}\0${absolutePath}` + const cached = repositoryFileCache.get(cacheKey) if (cached) return cached - const result = readBoundedFile(absolutePath) - repositoryFileCache.set(absolutePath, result) + const containedPath = containedRepositoryPath(absoluteRoot, absolutePath) + const result = containedPath.path ? readBoundedFile(containedPath.path) : containedPath.failure! + repositoryFileCache.set(cacheKey, result) if (!result.ok) recordSkippedFile(appRoot, path, result) return result } @@ -599,6 +648,160 @@ export function findSensitiveFiles(appRoot: string): SourceFile[] { }) } +interface AllowedPathResult { + exists: boolean + path?: string + unresolvedReason?: string +} + +function filesystemError(path: string, error: unknown): string { + const detail = error instanceof Error ? error.message : String(error) + return `Could not inspect ${path}: ${detail}` +} + +function isMissingFilesystemEntry(error: unknown): boolean { + return error instanceof Error && 'code' in error && (error.code === 'ENOENT' || error.code === 'ENOTDIR') +} + +/** + * Resolve an allowlisted path while preserving the difference between absence + * and an unsafe or unreadable entry. `realpathSync` follows every intermediate + * directory link, allowing contained links but rejecting escapes and dangling + * links explicitly. + */ +function inspectAllowedPath(appRoot: string, relative: string, expectedType: 'file' | 'directory'): AllowedPathResult { + const path = resolvePath(appRoot, relative) + if (!isSubpath(appRoot, path)) return {exists: false, unresolvedReason: `${relative} escapes the app root`} + + const segments = relative.replace(/\\/g, '/').split('/').filter(Boolean) + let currentPath = appRoot + for (const [index, segment] of segments.entries()) { + currentPath = joinPath(currentPath, segment) + try { + lstatSync(currentPath) + const canonicalPath = realpathSync(currentPath) + if (!isSubpath(appRoot, canonicalPath)) { + return {exists: false, unresolvedReason: `${relative} resolves outside the app root`} + } + + const stats = lstatSync(canonicalPath) + const isLastSegment = index === segments.length - 1 + const requiredType = isLastSegment ? expectedType : 'directory' + const hasRequiredType = requiredType === 'file' ? stats.isFile() : stats.isDirectory() + if (!hasRequiredType) { + return {exists: false, unresolvedReason: `${relative} contains an entry that is not a ${requiredType}`} + } + // Discovery must distinguish a missing optional allowlist entry from an + // entry that exists but cannot be inspected safely. + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + if (isMissingFilesystemEntry(error)) { + // lstat succeeds for a dangling link, so ENOENT from realpath is an + // unsafe existing entry rather than ordinary allowlist absence. + try { + if (lstatSync(currentPath).isSymbolicLink()) { + return {exists: false, unresolvedReason: `${relative} contains a dangling symbolic link`} + } + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + return {exists: false} + } + } + return {exists: false, unresolvedReason: filesystemError(relative, error)} + } + } + + return {exists: true, path} +} + +function nestedRepositoryReason(appRoot: string): string | undefined { + const rootMarker = joinPath(appRoot, '.git') + try { + const stats = lstatSync(rootMarker) + if (stats.isDirectory() || stats.isFile()) return undefined + // A root marker establishes the app's repository boundary only when it is + // a directory or worktree file. Never follow ambiguous marker entries. + return `Could not determine repository ownership from ${rootMarker}` + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + if (!isMissingFilesystemEntry(error)) return filesystemError(rootMarker, error) + } + + let ancestor = dirname(appRoot) + while (true) { + const marker = joinPath(ancestor, '.git') + try { + const stats = lstatSync(marker) + if (stats.isDirectory() || stats.isFile()) { + return `App root is nested below repository root ${ancestor}` + } + // A symlink or special file is not a supported repository marker, but + // treating it as absent would make repository ownership ambiguous. + return `Could not determine repository ownership from ${marker}` + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + if (!isMissingFilesystemEntry(error)) return filesystemError(marker, error) + } + + const parent = dirname(ancestor) + if (parent === ancestor) return undefined + ancestor = parent + } +} + +/** Discover only repository CI files that can explicitly invoke dependency auditing. */ +export function findDependencyAuditingInputs(appRoot: string): DependencyAuditingInputs { + let canonicalRoot: string + try { + canonicalRoot = realpathSync(resolvePath(appRoot)) + if (!lstatSync(canonicalRoot).isDirectory()) { + return {files: [], unresolvedReason: `App root is not a directory: ${appRoot}`} + } + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + return {files: [], unresolvedReason: filesystemError(appRoot, error)} + } + + const repositoryReason = nestedRepositoryReason(canonicalRoot) + if (repositoryReason) return {files: [], unresolvedReason: repositoryReason} + + const candidatePaths: string[] = [] + const workflows = inspectAllowedPath(canonicalRoot, '.github/workflows', 'directory') + if (workflows.unresolvedReason) return {files: [], unresolvedReason: workflows.unresolvedReason} + if (workflows.exists) { + try { + candidatePaths.push( + ...readdirSync(workflows.path!, {withFileTypes: true}) + .filter((entry) => /\.ya?ml$/u.test(entry.name)) + .map((entry) => `.github/workflows/${entry.name}`), + ) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + return {files: [], unresolvedReason: filesystemError('.github/workflows', error)} + } + } + + const files: SourceFile[] = [] + for (const relative of [...new Set(candidatePaths)].sort()) { + const inspected = inspectAllowedPath(canonicalRoot, relative, 'file') + if (inspected.unresolvedReason) return {files, unresolvedReason: inspected.unresolvedReason} + if (!inspected.exists) continue + + const absolutePath = inspected.path! + const result = readRepositoryFile(canonicalRoot, absolutePath) + files.push({ + path: relative, + absolutePath, + ext: extname(relative), + content: result.ok ? result.content.toString() : undefined, + }) + if (!result.ok && result.reason === 'unreadable') { + return {files, unresolvedReason: `Could not read ${relative}: ${result.detail ?? 'unknown filesystem error'}`} + } + } + return {files} +} + /** Find JavaScript package manifests. Dependency analysis intentionally supports JavaScript only. */ export function findManifestPaths(appRoot: string): string[] { const paths = globSync(['**/package.json'], { @@ -612,6 +815,12 @@ export function findManifestPaths(appRoot: string): string[] { return [...new Set(paths)].sort() } +const PackageManifestSchema = zod.object({ + dependencies: zod.record(zod.string()).optional(), + devDependencies: zod.record(zod.string()).optional(), + scripts: zod.record(zod.string()).optional(), +}) + export function findManifests(appRoot: string, discoveredPaths = findManifestPaths(appRoot)): ManifestFile[] { const manifests: ManifestFile[] = [] @@ -622,7 +831,7 @@ export function findManifests(appRoot: string, discoveredPaths = findManifestPat const content = readRepositoryText(appRoot, fullPath) if (content === undefined) continue try { - const pkg = JSON.parse(content) + const pkg = PackageManifestSchema.parse(JSON.parse(content)) manifests.push({ path: pkgPath, absolutePath: fullPath, @@ -630,6 +839,7 @@ export function findManifests(appRoot: string, discoveredPaths = findManifestPat content, dependencies: pkg.dependencies ?? {}, devDependencies: pkg.devDependencies ?? {}, + scripts: pkg.scripts, }) // Invalid repository JSON is a coverage gap, not a scanner crash. // eslint-disable-next-line no-catch-all/no-catch-all diff --git a/packages/app/src/cli/services/app-doctor-engine/scanners/index.ts b/packages/app/src/cli/services/app-doctor-engine/scanners/index.ts index c6a8d0bc98b..a5c5169ded3 100644 --- a/packages/app/src/cli/services/app-doctor-engine/scanners/index.ts +++ b/packages/app/src/cli/services/app-doctor-engine/scanners/index.ts @@ -10,6 +10,7 @@ import { getSkippedFiles, findManifests, findManifestPaths, + findDependencyAuditingInputs, } from './discover.js' import {detectCapabilities, detectProject} from '../capabilities/detect.js' import {calculateScore, computeScanMetadata} from '../scorer/index.js' @@ -28,6 +29,7 @@ import {missingComplianceWebhooks, scanEolApiVersions} from '../rules/compliance import {scanAppProxyLiquidInjection} from '../rules/proxy-rules.js' import {scanExpiringOfflineTokens} from '../rules/token-rules.js' import {scanStaticFrameAncestors} from '../rules/csp-rules.js' +import {scanDependencyAuditing} from '../rules/dependency-auditing-rules.js' import {RULE_CATALOG} from '../rules/catalog.js' import {redactIssue} from '../trace/index.js' import {getEngineVersion} from '../version.js' @@ -47,7 +49,15 @@ import type { SkippedFile, } from '../types.js' -type CheckTarget = 'config' | 'source' | 'app_source' | 'theme' | 'secrets' | 'config_and_source' | 'source_and_theme' +type CheckTarget = + | 'config' + | 'source' + | 'app_source' + | 'theme' + | 'secrets' + | 'config_and_source' + | 'source_and_theme' + | 'dependency_auditing' interface RunnerImplementationResult { id: string analysisMode: AnalysisMode @@ -108,6 +118,16 @@ const jsCheck = ( /** The only active deterministic product checks. Shared agent IDs are deliberate fallback coverage. */ const DETERMINISTIC_CHECK_DEFINITIONS: ReadonlyArray = [ configRule(missingComplianceWebhooks), + { + id: 'MISSING_DEPENDENCY_AUDITING', + version: 1, + lifecycle: 'active', + analysisMode: 'structured_config', + target: 'dependency_auditing', + guidance: + 'Resolve unreadable or malformed inputs and inspect unsupported configuration references manually. Repository-level CI configuration is not inspected for nested apps; verify that dependency vulnerability auditing covers this app.', + runner: (context) => scanDependencyAuditing(context), + }, { id: 'EOL_API_VERSION', version: 1, @@ -362,6 +382,11 @@ async function gitProject(appRoot: string): Promise { function selectedFiles(definition: DeterministicCheckDefinition, context: ScanContext): string[] { const configurations = context.appTomls.map((toml) => relativePath(context.appRoot, toml.path).replace(/\\/g, '/')) if (definition.target === 'config') return configurations + if (definition.target === 'dependency_auditing') + return [ + ...context.manifests.map((manifest) => manifest.path), + ...context.dependencyAuditing.files.filter((file) => file.content !== undefined).map((file) => file.path), + ] if (definition.target === 'secrets') return context.sensitiveFiles.filter((file) => file.content !== undefined).map((file) => file.path) let files = definition.target === 'app_source' ? appSourceFiles(context) : reactRouterFiles(context) @@ -389,6 +414,13 @@ function executionDisposition( const reactRouterSupported = context.detection.framework === 'react_router' const hasTheme = context.capabilities.theme_app_extension + if (definition.target === 'dependency_auditing' && !context.manifests.some(manifestHasDependencies)) + return { + status: 'not_applicable', + required: false, + applicable: false, + reason: {code: 'no_relevant_files', message: 'No supported package manifest declares dependencies.'}, + } if (definition.target === 'source' && !reactRouterSupported) { if (nonThemeCandidates.length > 0) return { @@ -497,6 +529,10 @@ function executionDisposition( return {status: 'executed', required: true, applicable: true} } +function manifestHasDependencies(manifest: ScanContext['manifests'][number]): boolean { + return Object.keys(manifest.dependencies).length > 0 || Object.keys(manifest.devDependencies ?? {}).length > 0 +} + function skippedInputsForCheck( definition: DeterministicCheckDefinition, context: ScanContext, @@ -510,6 +546,8 @@ function skippedInputsForCheck( const isSourcePath = (path: string) => !isThemePath(path) && Boolean(definition.extensions?.some((extension) => path.endsWith(extension))) const isConfig = (path: string) => /^shopify\.app(?:\.[^/]+)?\.toml$/.test(path) + const isDependencyAuditingInput = (path: string) => + /(^|\/)package\.json$/.test(path) || /^\.github\/workflows\/[^/]+\.ya?ml$/i.test(path) const isSecretInput = (file: SkippedFile) => !file.detail?.includes('could not be parsed') && (context.sourceCandidates.some((candidate) => candidate.path === file.path) || @@ -518,6 +556,7 @@ function skippedInputsForCheck( return skippedFiles.filter((file) => { if (definition.target === 'config') return isConfig(file.path) + if (definition.target === 'dependency_auditing') return isDependencyAuditingInput(file.path) if (definition.target === 'config_and_source') return isConfig(file.path) || isSourcePath(file.path) if (definition.target === 'source' || definition.target === 'app_source') return isSourcePath(file.path) if (definition.target === 'theme') return isThemePath(file.path) @@ -562,6 +601,9 @@ export async function scan(startPath?: string): Promise { const sensitiveFiles = findSensitiveFiles(appRoot) const manifestPaths = findManifestPaths(appRoot) const manifests = findManifests(appRoot, manifestPaths) + const dependencyAuditing = manifests.some(manifestHasDependencies) + ? findDependencyAuditingInputs(appRoot) + : {files: []} const requestedAppToml = startPath?.endsWith('.toml') ? (appTomls.find((toml) => basename(toml.path) === basename(startPath)) ?? loadAppToml(joinPath(appRoot, basename(startPath)), appRoot)) @@ -583,6 +625,7 @@ export async function scan(startPath?: string): Promise { extensions, sourceFiles, manifests, + dependencyAuditing, sensitiveFiles, capabilities, detection, @@ -596,10 +639,16 @@ export async function scan(startPath?: string): Promise { let inspectedFiles = disposition.status === 'unsupported_framework' ? [] : selectedFiles(definition, context) let implementations: RunnerImplementationResult[] | undefined const before = issues.length - if ((disposition.status === 'executed' || disposition.status === 'unresolved') && definition.runner) { + const rejectedInputs = skippedInputsForCheck(definition, context, getSkippedFiles()) + const suppressRunnerForRejectedInput = definition.target === 'dependency_auditing' && rejectedInputs.length > 0 + if ( + (disposition.status === 'executed' || disposition.status === 'unresolved') && + definition.runner && + !suppressRunnerForRejectedInput + ) { // eslint-disable-next-line no-await-in-loop const output = normalizeRunnerResult(await definition.runner(runnerContext(definition, context))) - issues.push(...output.issues) + if (definition.target !== 'dependency_auditing' || !output.unresolvedReason) issues.push(...output.issues) implementations = output.implementations if (output.inspectedFiles) inspectedFiles = output.inspectedFiles if (output.unresolvedReason) @@ -613,7 +662,6 @@ export async function scan(startPath?: string): Promise { }, } } - const rejectedInputs = skippedInputsForCheck(definition, context, getSkippedFiles()) if (disposition.status !== 'unsupported_framework' && rejectedInputs.length > 0) { const reason = skippedInputReason(definition, rejectedInputs) disposition = {status: 'unresolved', required: true, applicable: true, reason} @@ -683,6 +731,7 @@ export async function scan(startPath?: string): Promise { ...appTomls.map((toml) => ({absolutePath: toml.path, content: toml.content})), ...extensions.map((extension) => ({absolutePath: joinPath(appRoot, extension.path), content: extension.content})), ...manifests.map((manifest) => ({absolutePath: manifest.absolutePath, content: manifest.content})), + ...dependencyAuditing.files.map((file) => ({absolutePath: joinPath(appRoot, file.path), content: file.content})), ] for (const {absolutePath, content} of configFiles) if (content !== undefined) { diff --git a/packages/app/src/cli/services/app-doctor-engine/scanners/types.ts b/packages/app/src/cli/services/app-doctor-engine/scanners/types.ts index eeb7bfbb999..a56337ae054 100644 --- a/packages/app/src/cli/services/app-doctor-engine/scanners/types.ts +++ b/packages/app/src/cli/services/app-doctor-engine/scanners/types.ts @@ -42,6 +42,13 @@ export interface SourceFile { content?: string } +/** Explicitly allowlisted CI configuration inside the app-root evidence boundary. */ +export interface DependencyAuditingInputs { + files: SourceFile[] + /** A specific discovery obstacle, including an app nested below its repository root. */ + unresolvedReason?: string +} + export interface ManifestFile { path: string absolutePath: string @@ -51,4 +58,6 @@ export interface ManifestFile { /** Parsed dependencies, keyed by name with version specifications as values. */ dependencies: Record devDependencies?: Record + /** Literal package scripts; only one CI-invoked script reference is followed. */ + scripts?: Record } diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-discovery.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-discovery.test.ts new file mode 100644 index 00000000000..eb3a16586c1 --- /dev/null +++ b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-discovery.test.ts @@ -0,0 +1,261 @@ +/* eslint-disable no-restricted-imports -- discovery boundaries use real temporary repositories */ +import {findDependencyAuditingInputs, findManifests, getSkippedFiles, resetSkippedFiles} from '../scanners/discover.js' +import {afterEach, describe, expect, test} from 'vitest' +import {execFileSync} from 'node:child_process' +import {createHash} from 'node:crypto' +import {mkdir, mkdtemp, rm, symlink, writeFile} from 'node:fs/promises' +import {tmpdir} from 'node:os' +import {dirname, join} from 'node:path' + +const temporaryDirectories: string[] = [] + +afterEach(async () => { + resetSkippedFiles() + await Promise.all(temporaryDirectories.splice(0).map((directory) => rm(directory, {recursive: true, force: true}))) +}) + +async function makeDirectory(prefix = 'app-doctor-dependency-discovery-'): Promise { + const directory = await mkdtemp(join(tmpdir(), prefix)) + temporaryDirectories.push(directory) + return directory +} + +async function writeFiles(root: string, files: Record): Promise { + await Promise.all( + Object.entries(files).map(async ([path, content]) => { + const fullPath = join(root, path) + await mkdir(dirname(fullPath), {recursive: true}) + await writeFile(fullPath, content) + }), + ) +} + +describe('dependency auditing input discovery', () => { + test('treats an empty allowlist as complete discovery', async () => { + const root = await makeDirectory() + + expect(findDependencyAuditingInputs(root)).toEqual({files: []}) + }) + + test('reads only explicitly allowlisted hidden CI configuration with exact raw content', async () => { + const root = await makeDirectory() + const workflow = 'name: dependency review\r\npermissions: {}\r\n' + await writeFiles(root, { + '.github/workflows/dependencies.yml': workflow, + '.github/workflows/release.yaml': 'name: release\n', + '.github/workflows/ignored.json': '{}', + '.hidden/config.yml': 'not: allowlisted\n', + }) + + const result = findDependencyAuditingInputs(root) + + expect(result.unresolvedReason).toBeUndefined() + expect(result.files.map(({path}) => path)).toEqual([ + '.github/workflows/dependencies.yml', + '.github/workflows/release.yaml', + ]) + const discoveredWorkflow = result.files.find(({path}) => path.endsWith('dependencies.yml')) + expect(discoveredWorkflow?.content).toBe(workflow) + expect( + createHash('sha256') + .update(discoveredWorkflow?.content ?? '') + .digest('hex'), + ).toBe(createHash('sha256').update(workflow).digest('hex')) + }) + + test('preserves bounded reads and skipped-file coverage', async () => { + const root = await makeDirectory() + await writeFiles(root, {'.github/workflows/ci.yml': 'x'.repeat(500_001)}) + resetSkippedFiles() + + const result = findDependencyAuditingInputs(root) + + expect(result).toMatchObject({files: [{path: '.github/workflows/ci.yml', content: undefined}]}) + expect(result.unresolvedReason).toBeUndefined() + expect(getSkippedFiles()).toEqual([ + expect.objectContaining({path: '.github/workflows/ci.yml', reason: 'too_large', size_bytes: 500_001}), + ]) + }) + + test.each(['directory', 'worktree file'])( + 'does not inspect app config nested below a repository %s marker', + async (marker) => { + const repository = await makeDirectory() + const app = join(repository, 'apps', 'example') + await writeFiles(app, {'.github/workflows/audit.yml': 'name: audit\n'}) + if (marker === 'directory') await mkdir(join(repository, '.git')) + else await writeFile(join(repository, '.git'), 'gitdir: /outside/not-read\n') + + const result = findDependencyAuditingInputs(app) + + expect(result.files).toEqual([]) + expect(result.unresolvedReason).toContain('nested below repository root') + }, + ) + + test.each(['directory', 'worktree file'])( + 'uses an app-root repository %s marker without inspecting a parent repository marker', + async (marker) => { + const repository = await makeDirectory() + const app = join(repository, 'apps', 'example') + await writeFiles(app, {'.github/workflows/ci.yml': 'name: audit\n'}) + await mkdir(join(repository, '.git')) + if (marker === 'directory') await mkdir(join(app, '.git')) + else await writeFile(join(app, '.git'), 'gitdir: /outside/not-read\n') + + const result = findDependencyAuditingInputs(app) + + expect(result.files).toMatchObject([{path: '.github/workflows/ci.yml', content: 'name: audit\n'}]) + expect(result.unresolvedReason).toBeUndefined() + }, + ) + + test('rejects an app-root repository marker symlink as ambiguous', async () => { + const root = await makeDirectory() + await mkdir(join(root, 'repository-metadata')) + await symlink(join(root, 'repository-metadata'), join(root, '.git'), 'dir') + + const result = findDependencyAuditingInputs(root) + + expect(result.files).toEqual([]) + expect(result.unresolvedReason).toContain('Could not determine repository ownership') + }) + + test.skipIf(process.platform === 'win32')('rejects an app-root special repository marker as ambiguous', async () => { + const root = await makeDirectory() + execFileSync('mkfifo', [join(root, '.git')]) + + const result = findDependencyAuditingInputs(root) + + expect(result.files).toEqual([]) + expect(result.unresolvedReason).toContain('Could not determine repository ownership') + }) + + test('rejects an allowlisted file symlink that escapes the app root', async () => { + const root = await makeDirectory() + const outside = await makeDirectory('app-doctor-dependency-outside-') + await writeFile(join(outside, 'pipeline.yml'), 'audit: true\n') + await mkdir(join(root, '.github', 'workflows'), {recursive: true}) + await symlink(join(outside, 'pipeline.yml'), join(root, '.github', 'workflows', 'ci.yml')) + + const result = findDependencyAuditingInputs(root) + + expect(result.files).toEqual([]) + expect(result.unresolvedReason).toContain('outside the app root') + }) + + test.each(['.github', '.github/workflows'])( + 'reports an escaping %s directory symlink instead of treating workflows as absent', + async (linkedDirectory) => { + const root = await makeDirectory() + const outside = await makeDirectory('app-doctor-workflows-outside-') + await writeFiles(outside, {'workflows/audit.yml': 'name: audit\n', 'audit.yml': 'name: audit\n'}) + await mkdir(dirname(join(root, linkedDirectory)), {recursive: true}) + await symlink(outside, join(root, linkedDirectory), 'dir') + + const result = findDependencyAuditingInputs(root) + + expect(result.files).toEqual([]) + expect(result.unresolvedReason).toContain('outside the app root') + }, + ) + + test('reports dangling allowlisted directory symlinks', async () => { + const root = await makeDirectory() + await mkdir(join(root, '.github')) + await symlink(join(root, 'missing-workflows'), join(root, '.github', 'workflows'), 'dir') + + const result = findDependencyAuditingInputs(root) + + expect(result.files).toEqual([]) + expect(result.unresolvedReason).toContain('dangling symbolic link') + }) +}) + +describe('manifest safety and validation', () => { + test('rejects explicit parent paths and manifest symlinks outside the app root', async () => { + const root = await makeDirectory() + const outside = await makeDirectory('app-doctor-manifest-outside-') + await writeFile(join(outside, 'package.json'), JSON.stringify({dependencies: {unsafe: '1.0.0'}})) + await symlink(join(outside, 'package.json'), join(root, 'package.json')) + resetSkippedFiles() + + expect(findManifests(root, ['package.json', '../package.json'])).toEqual([]) + expect(getSkippedFiles()).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + path: 'package.json', + reason: 'unreadable', + detail: 'symbolic link escapes the app root', + }), + expect.objectContaining({path: '../package.json', reason: 'unreadable', detail: 'path escapes the app root'}), + ]), + ) + }) + + test.skipIf(process.platform === 'win32')('rejects a manifest FIFO before attempting to read it', async () => { + const root = await makeDirectory() + execFileSync('mkfifo', [join(root, 'package.json')]) + resetSkippedFiles() + + expect(findManifests(root, ['package.json'])).toEqual([]) + expect(getSkippedFiles()).toEqual([ + expect.objectContaining({path: 'package.json', reason: 'unreadable', detail: 'path is not a regular file'}), + ]) + }) + + test('keeps manifests without dependencies as valid inputs', async () => { + const root = await makeDirectory() + await writeFile( + join(root, 'package.json'), + JSON.stringify({name: 'dependency-free-app', scripts: {start: 'shopify app dev'}}), + ) + + expect(findManifests(root)).toMatchObject([ + { + dependencies: {}, + devDependencies: {}, + scripts: {start: 'shopify app dev'}, + }, + ]) + expect(getSkippedFiles()).toEqual([]) + }) + + test('stores string-valued scripts and dependency records', async () => { + const root = await makeDirectory() + await writeFiles(root, { + 'package.json': JSON.stringify({ + dependencies: {production: '^1.0.0'}, + devDependencies: {development: '^2.0.0'}, + scripts: {audit: 'npm audit'}, + }), + }) + + expect(findManifests(root)).toMatchObject([ + { + dependencies: {production: '^1.0.0'}, + devDependencies: {development: '^2.0.0'}, + scripts: {audit: 'npm audit'}, + }, + ]) + }) + + test.each([ + ['null package', 'null'], + ['array package', '[]'], + ['non-string dependency', JSON.stringify({dependencies: {unsafe: 1}})], + ['non-string development dependency', JSON.stringify({devDependencies: {unsafe: null}})], + ['non-string script', JSON.stringify({scripts: {audit: ['npm audit']}})], + ])('keeps a malformed %s as skipped input coverage', async (_description, content) => { + const root = await makeDirectory() + await writeFile(join(root, 'package.json'), content) + resetSkippedFiles() + + const manifests = findManifests(root) + + expect(manifests).toMatchObject([{dependencies: {}, devDependencies: {}}]) + expect(getSkippedFiles()).toEqual([ + expect.objectContaining({path: 'package.json', reason: 'unreadable', detail: 'manifest could not be parsed'}), + ]) + }) +}) diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-rules.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-rules.test.ts new file mode 100644 index 00000000000..6e67588aa17 --- /dev/null +++ b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-rules.test.ts @@ -0,0 +1,465 @@ +import {scanDependencyAuditing} from '../rules/dependency-auditing-rules.js' +import {describe, expect, test} from 'vitest' +import type {ManifestFile, SourceFile} from '../rules/types.js' + +const manifest = (path = 'package.json', scripts?: Record): ManifestFile => ({ + path, + absolutePath: `/${path}`, + type: 'npm', + dependencies: {react: '19.0.0'}, + scripts, +}) + +const configuration = (path: string, content: string): SourceFile => ({ + path, + absolutePath: `/${path}`, + ext: path.endsWith('.yaml') ? '.yaml' : '.yml', + content, +}) + +const github = (body: string): SourceFile => + configuration( + '.github/workflows/ci.yml', + `on: push +jobs: + check: + runs-on: ubuntu-latest +${body}`, + ) + +function scan(files: SourceFile[], manifests: ManifestFile[] = [manifest()]) { + return scanDependencyAuditing({manifests, dependencyAuditing: {files}}) +} + +function expectRecognized(files: SourceFile[], manifests?: ManifestFile[]) { + expect(scan(files, manifests)).toEqual({issues: []}) +} + +function expectMissing(files: SourceFile[], manifests?: ManifestFile[]) { + const result = scan(files, manifests) + expect(result.unresolvedReason).toBeUndefined() + expect(result.issues).toHaveLength(1) + expect(result.issues[0]).toMatchObject({ + id: 'MISSING_DEPENDENCY_AUDITING', + severity: 'low', + points: -5, + title: 'Dependency auditing configuration not detected', + location: {file: manifests?.[0]?.path ?? 'package.json'}, + fix: {automated: false}, + }) +} + +function expectUnresolved(files: SourceFile[], manifests?: ManifestFile[]) { + const result = scan(files, manifests) + expect(result.issues).toEqual([]) + expect(result.unresolvedReason).toBeTruthy() +} + +describe('dependency-auditing applicability and finding', () => { + test('is not applicable without a manifest that declares dependencies', () => { + const emptyManifest = {...manifest(), dependencies: {}} + expect(scan([], [emptyManifest])).toEqual({issues: []}) + }) + + test('emits exactly one finding anchored to an actual dependency manifest', () => { + const manifests = [manifest('packages/web/package.json'), manifest('package.json')] + const result = scan([], manifests) + + expect(result.issues).toHaveLength(1) + expect(result.issues[0]).toEqual({ + id: 'MISSING_DEPENDENCY_AUDITING', + severity: 'low', + points: -5, + title: 'Dependency auditing configuration not detected', + message: + 'No recognized dependency-auditing configuration was found in the inspected files. Add an automated dependency vulnerability check, or verify that your existing integration covers this app. Hosted integrations and repository settings were not inspected.', + location: {file: 'package.json'}, + fix: {automated: false, description: 'Add an automated dependency vulnerability check for this package.'}, + }) + }) + + test('preserves a scanner discovery obstacle without emitting a finding', () => { + expect( + scanDependencyAuditing({ + manifests: [manifest()], + dependencyAuditing: {files: [], unresolvedReason: 'The app is nested below the repository root.'}, + }), + ).toEqual({issues: [], unresolvedReason: 'The app is nested below the repository root.'}) + }) + + test.each(['', '# comments only\n', 'null\n'])('treats empty YAML as no evidence', (content) => { + expectMissing([configuration('.github/workflows/empty.yml', content)]) + }) +}) + +describe('GitHub Actions recognition', () => { + test.each([ + ['dependency review action', ' - uses: actions/dependency-review-action@v4'], + [ + 'OSV reusable workflow', + undefined, + `on: pull_request +jobs: + osv: + uses: google/osv-scanner/.github/workflows/osv-scanner-reusable-pr.yml@v2.0.1`, + ], + ['Snyk Node action', ' - uses: snyk/actions/node@master'], + ['npm audit', ' - run: npm audit --audit-level=high'], + ['pnpm audit', ' - run: pnpm audit'], + ['Yarn classic audit', ' - run: yarn audit'], + ['Yarn modern audit', ' - run: yarn npm audit'], + ['Snyk test', ' - run: npx snyk test --severity-threshold=high'], + ['Semgrep supply chain', ' - run: semgrep ci --supply-chain'], + ['continue-on-error audit', ' - run: npm audit\n continue-on-error: true'], + ])('recognizes %s', (_name, step, completeWorkflow?: string) => { + const file = completeWorkflow + ? configuration('.github/workflows/security.yml', completeWorkflow) + : github(` steps:\n${step}`) + expectRecognized([file]) + }) + + test.each([ + ['disabled job', github(` if: ${['$', '{{ false }}'].join('')}\n steps:\n - run: npm audit`)], + ['disabled step', github(' steps:\n - if: false\n run: npm audit')], + [ + 'disabled dependency review vulnerability check', + github( + ' steps:\n - uses: actions/dependency-review-action@v4\n with:\n vulnerability-check: false', + ), + ], + ['Snyk setup action', github(' steps:\n - uses: snyk/actions/setup@master')], + ['Snyk code action', github(' steps:\n - uses: snyk/actions/code@master')], + ['Snyk container action', github(' steps:\n - uses: snyk/actions/docker@master')], + ['scanner installation', github(' steps:\n - run: npm install --global snyk')], + ['step name', github(' steps:\n - name: npm audit\n run: npm test')], + ['comment', github(' steps:\n - run: |\n # npm audit\n npm test')], + ['echo output', github(' steps:\n - run: echo "npm audit"')], + ['printf output', github(" steps:\n - run: printf 'snyk test\\n'")], + ['Dependabot update action', github(' steps:\n - uses: dependabot/fetch-metadata@v2')], + ['CodeQL action', github(' steps:\n - uses: github/codeql-action/analyze@v3')], + ['workflow metadata only', configuration('.github/workflows/ci.yml', 'name: npm audit')], + ])('does not recognize %s', (_name, file) => expectMissing([file])) + + test('matches a straightforward working-directory to the dependency manifest', () => { + const file = github( + ' defaults:\n run:\n working-directory: packages/web\n steps:\n - run: npm audit', + ) + expectRecognized([file], [manifest('packages/web/package.json')]) + expectMissing([file], [manifest('packages/admin/package.json')]) + }) + + test('matches explicit scanner files and excludes unrelated paths and ecosystems', () => { + const scopedSnyk = github( + ' steps:\n - uses: snyk/actions/node@master\n with:\n args: --file=packages/web/package.json --package-manager=npm', + ) + expectRecognized([scopedSnyk], [manifest('packages/web/package.json')]) + expectMissing([scopedSnyk], [manifest('packages/admin/package.json')]) + + const unrelated = github( + ' steps:\n - uses: snyk/actions/node@master\n with:\n args: --file=requirements.txt --package-manager=poetry', + ) + expectMissing([unrelated]) + }) + + test.each(['install', 'monitor', 'auth', 'help'])('does not recognize Snyk action command %s', (command) => { + expectMissing([ + github(` steps:\n - uses: snyk/actions/node@master\n with:\n command: ${command}`), + ]) + }) + + test('recognizes only the Snyk Node action default or exact test command', () => { + expectRecognized([github(' steps:\n - uses: snyk/actions/node@master')]) + expectRecognized([ + github(' steps:\n - uses: snyk/actions/node@master\n with:\n command: test'), + ]) + expectMissing([ + github( + ' steps:\n - uses: snyk/actions/node@master\n with:\n command: test --all-projects', + ), + ]) + }) + + test.each([ + 'npm audit --help', + 'pnpm audit --version', + 'yarn npm audit --help', + 'snyk test --help', + 'snyk test --package-manager=pip', + 'snyk test --file=requirements.txt', + ])('does not treat non-scanning command %s as an audit', (command) => { + expectMissing([github(` steps:\n - run: ${command}`)]) + }) + + test('runs actions at checkout root rather than defaults.run working-directory', () => { + const file = github( + ' defaults:\n run:\n working-directory: packages/web\n steps:\n - uses: snyk/actions/node@master', + ) + expectMissing([file], [manifest('packages/web/package.json')]) + expectRecognized([file], [manifest('package.json')]) + }) + + test('does not trust scoped OSV workflow inputs or dependency-review config files', () => { + expectUnresolved([ + configuration( + '.github/workflows/security.yml', + 'on: pull_request\njobs:\n osv:\n uses: google/osv-scanner/.github/workflows/osv-scanner-reusable-pr.yml@v2.0.1\n with:\n scan-args: --lockfile=other/package-lock.json', + ), + ]) + expectUnresolved([ + github( + ' steps:\n - uses: actions/dependency-review-action@v4\n with:\n config-file: .github/dependency-review.yml', + ), + ]) + }) + + const dynamicWorkingDirectory = ['$', '{{ github.workspace }}'].join('') + test.each([ + ['workflow', `defaults:\n run:\n working-directory: ${dynamicWorkingDirectory}\n`], + ['job', ''], + ])('leaves dynamic %s defaults.run working-directory unresolved', (_scope, workflowDefaults) => { + const jobDefaults = workflowDefaults + ? '' + : ` defaults:\n run:\n working-directory: ${dynamicWorkingDirectory}\n` + const file = configuration( + '.github/workflows/ci.yml', + `on: push\n${workflowDefaults}jobs:\n check:\n runs-on: ubuntu-latest\n${jobDefaults} steps:\n - run: npm audit`, + ) + expectUnresolved([file]) + }) + + test.each([ + ' if: false\n steps: invalid', + ' steps:\n - if: false\n run: [npm, audit]\n uses: 123\n with: invalid', + ' defaults: invalid\n steps:\n - uses: actions/setup-node@v4', + ' steps:\n - uses: snyk/actions/node@master\n with:\n command: monitor\n args: [invalid]', + ])('does not validate configuration that is not executed: %s', (body) => { + expectMissing([github(body)]) + }) + + test.each([ + ' defaults: invalid\n steps:\n - run: npm audit\n working-directory: .', + ' defaults:\n run:\n working-directory: [invalid]\n shell: [invalid]\n steps:\n - run: npm audit\n working-directory: .\n shell: bash', + ' defaults: invalid\n steps:\n - uses: actions/dependency-review-action@v4', + ' steps:\n - uses: actions/dependency-review-action@v4\n with:\n fail-on-severity: high', + ' steps:\n - run: npm audit\n - run: [invalid]', + ])('retains independent evidence and overridden defaults: %s', (body) => { + expectRecognized([github(body)]) + }) + + test.each(['workflow_call', '[workflow_call]', '{workflow_call: {inputs: {}}}'])( + 'does not validate jobs in a reusable-only definition: %s', + (trigger) => { + expectMissing([configuration('.github/workflows/reusable.yml', `on: ${trigger}\njobs: invalid`)]) + }, + ) + + test('keeps other workflow triggers when checking reusable-only definitions', () => { + expectRecognized([ + configuration( + '.github/workflows/security.yml', + 'on: {workflow_call: {}, push: {}}\njobs:\n audit:\n steps:\n - run: npm audit', + ), + ]) + }) + + test.each(['null', '[]', '{unexpected-input: true}'])('does not discard unsupported OSV inputs: %s', (inputs) => { + expectUnresolved([ + configuration( + '.github/workflows/security.yml', + `on: push\njobs:\n audit:\n uses: google/osv-scanner/.github/workflows/osv-scanner-reusable.yml@v2.0.1\n with: ${inputs}`, + ), + ]) + }) + + test('treats malformed jobs, steps, and run values as unresolved', () => { + expectUnresolved([configuration('.github/workflows/ci.yml', 'on: push\njobs:\n check: invalid')]) + expectUnresolved([github(' steps: invalid')]) + expectUnresolved([github(' steps:\n - run: [npm, audit]')]) + }) +}) + +describe('package scripts and bounded shell handling', () => { + test('follows one literal CI-invoked package script', () => { + expectRecognized( + [github(' steps:\n - run: npm run security')], + [manifest('package.json', {security: 'snyk test'})], + ) + expectRecognized( + [github(' steps:\n - run: npm --prefix packages/web run security')], + [manifest('packages/web/package.json', {security: 'npm audit'})], + ) + }) + + test('lets explicitly recursive supply-chain scans cover nested manifests', () => { + expectRecognized( + [github(' steps:\n - run: semgrep ci --supply-chain')], + [manifest('packages/web/package.json')], + ) + expectRecognized( + [github(' steps:\n - run: snyk test --all-projects')], + [manifest('packages/web/package.json')], + ) + }) + + test('does not inspect unused package scripts', () => { + expectMissing( + [github(' steps:\n - run: npm test')], + [manifest('package.json', {test: 'vitest', security: 'npm audit'})], + ) + }) + + test('does not mistake a package script named audit for an executed scanner', () => { + expectMissing( + [github(' steps:\n - run: npm run audit')], + [manifest('package.json', {audit: 'echo no scan'})], + ) + }) + + test.each([ + ['nested package script', 'npm run security', {security: 'npm run actual', actual: 'npm audit'}], + ['local shell script', 'bash scripts/security.sh', undefined], + ['local executable', './scripts/security.sh', undefined], + ['shell variable', '$SECURITY_COMMAND', undefined], + ['shell control flow', 'if npm audit; then echo ok; fi', undefined], + ['shell interpreter command', 'sh -c "npm audit"', undefined], + ])('leaves %s unresolved', (_name, command, scripts) => { + const result = scan([github(` steps:\n - run: ${command}`)], [manifest('package.json', scripts)]) + expect(result.issues).toEqual([]) + expect(result.unresolvedReason).toBeTruthy() + }) + + test('allows recognized evidence despite an unrelated unsupported script', () => { + expectRecognized([github(' steps:\n - run: ./scripts/build.sh\n - run: pnpm audit')]) + }) + + test.each([ + 'if false; then npm audit; fi', + 'echo start; if false; then echo skipped; fi; npm audit', + 'cat < { + expectUnresolved([github(` steps:\n - run: |\n ${command.replaceAll('\n', '\n ')}`)]) + }) + + test('still accepts a distinct supported step after an unsupported execution block', () => { + expectRecognized([github(' steps:\n - run: if false; then npm audit; fi\n - run: npm audit')]) + }) + + test('handles escaped-newline command continuation and ignores quoted separators', () => { + expectRecognized([github(' steps:\n - run: |\n npm \\\n audit')]) + expectMissing([github(' steps:\n - run: echo "ignored; npm audit"')]) + expectMissing([github(" steps:\n - run: printf 'ignored\\n| snyk test'")]) + }) + + test.each(['cd ../; npm audit', 'cd /tmp; npm audit', 'cd $DIR; npm audit'])( + 'leaves unsafe directory change %s unresolved', + (command) => expectUnresolved([github(` steps:\n - run: ${command}`)]), + ) + + test.each([ + 'npm --prefix ../ audit', + 'pnpm --dir /tmp audit', + 'yarn --cwd $DIR npm audit', + 'npm --workspace other audit', + 'pnpm --filter unrelated audit', + 'npm --silent audit', + 'npm --prefix= audit', + 'snyk test --file=', + 'npm run $SCRIPT', + 'npm run missing', + ])('leaves unsupported target or package-script invocation unresolved: %s', (command) => { + expectUnresolved([github(` steps:\n - run: ${command}`)]) + }) + + test.each(['npm --prefix packages/web audit', 'pnpm --dir packages/web audit', 'yarn --cwd packages/web npm audit'])( + 'supports a bounded literal package directory: %s', + (command) => { + expectRecognized([github(` steps:\n - run: ${command}`)], [manifest('packages/web/package.json')]) + }, + ) + + test('leaves package-script arguments unresolved rather than ignoring them', () => { + expectUnresolved( + [github(' steps:\n - run: npm run security -- --help')], + [manifest('package.json', {security: 'snyk test'})], + ) + }) +}) + +describe('execution boundaries', () => { + test.each(['repository: other/project', 'path: other'])('leaves a scoped checkout unresolved (%s)', (input) => { + const file = github( + ` steps:\n - uses: actions/checkout@v4\n with:\n ${input}\n - run: npm audit`, + ) + expect(scan([file])).toMatchObject({issues: [], unresolvedReason: expect.any(String)}) + }) + + test('does not recognize an uninvoked reusable-only workflow', () => { + const file = configuration( + '.github/workflows/reusable.yml', + 'on: workflow_call\njobs:\n audit:\n steps:\n - run: npm audit', + ) + expectMissing([file]) + }) + + test.each([ + ['quoted heredoc', "cat <<'EOF'\nnpm audit\nEOF"], + ['function definition', 'audit() {\n npm audit\n}'], + ])('does not count commands inside %s', (_name, command) => { + const file = github( + ` steps:\n - run: |\n${command + .split('\n') + .map((line) => ` ${line}`) + .join('\n')}`, + ) + expect(scan([file])).toMatchObject({issues: [], unresolvedReason: expect.any(String)}) + }) + + test('does not count an audit printed through a custom shell', () => { + expect(scan([github(' steps:\n - run: npm audit\n shell: echo {0}')])).toMatchObject({ + issues: [], + unresolvedReason: expect.any(String), + }) + }) + + test.each(['pnpm install', 'yarn install', 'pnpm add snyk'])( + 'treats %s as setup, not a script reference', + (command) => { + expectMissing([github(` steps:\n - run: ${command}`)]) + }, + ) +}) + +describe('configuration obstacles', () => { + test.each([ + ['malformed GitHub workflow', configuration('.github/workflows/ci.yml', 'jobs: [')], + [ + 'excessive YAML aliases', + github( + ` x: &values [one, two]\n y: [${Array.from({length: 21}, () => '*values').join(', ')}]\n steps:\n - run: npm test`, + ), + ], + [ + 'unknown GitHub reusable workflow', + configuration( + '.github/workflows/ci.yml', + 'on: push\njobs:\n delegated:\n uses: acme/ci/.github/workflows/security.yml@v1', + ), + ], + ['unknown local GitHub action', github(' steps:\n - uses: ./.github/actions/security')], + ])('returns no finding and an unresolved reason for %s', (_name, file) => { + const result = scan([file]) + expect(result.issues).toEqual([]) + expect(result.unresolvedReason).toBeTruthy() + }) + + test('malformed relevant YAML remains unresolved even when another file has evidence', () => { + const result = scan([ + github(' steps:\n - run: npm audit'), + configuration('.github/workflows/malformed.yml', 'jobs: ['), + ]) + expect(result.issues).toEqual([]) + expect(result.unresolvedReason).toContain('malformed') + }) +}) diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing.test.ts new file mode 100644 index 00000000000..eb9ba8b882d --- /dev/null +++ b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing.test.ts @@ -0,0 +1,344 @@ +/* eslint-disable no-restricted-imports -- integration coverage uses real temporary repositories */ +import {doctorExitCode} from '../../app-doctor-api.js' +import {formatJson} from '../output/format.js' +import {getRegistry} from '../registry/index.js' +import {RULE_CATALOG} from '../rules/catalog.js' +import {DETERMINISTIC_CHECKS, scan} from '../scanners/index.js' +import {buildSubmission} from '../submission/index.js' +import {compileTrace, sha256, validateTrace} from '../trace/index.js' +import {scanApp} from '../run.js' +import {fetch} from '@shopify/cli-kit/node/http' +import {captureOutputWithExitCode} from '@shopify/cli-kit/node/system' +import {afterEach, describe, expect, test, vi} from 'vitest' +import {mkdir, mkdtemp, rm, unlink, writeFile} from 'node:fs/promises' +import {tmpdir} from 'node:os' +import {dirname, join} from 'node:path' + +vi.mock('@shopify/cli-kit/node/http', async (importActual) => { + const actual: any = await importActual() + return {...actual, fetch: vi.fn(actual.fetch)} +}) + +vi.mock('@shopify/cli-kit/node/system', async (importActual) => { + const actual: any = await importActual() + return {...actual, captureOutputWithExitCode: vi.fn(actual.captureOutputWithExitCode)} +}) + +const temporaryDirectories: string[] = [] +const appConfiguration = `name = "Dependency auditing integration" +[webhooks] +api_version = "unstable" +[[webhooks.subscriptions]] +compliance_topics = ["shop/redact", "customers/data_request", "customers/redact"] +uri = "https://example.test/webhooks" +` + +afterEach(async () => { + await Promise.all(temporaryDirectories.splice(0).map((directory) => rm(directory, {recursive: true, force: true}))) +}) + +async function makeDirectory(prefix = 'app-doctor-dependency-auditing-'): Promise { + const directory = await mkdtemp(join(tmpdir(), prefix)) + temporaryDirectories.push(directory) + return directory +} + +async function writeFiles(root: string, files: Record): Promise { + await Promise.all( + Object.entries(files).map(async ([path, content]) => { + const absolutePath = join(root, path) + await mkdir(dirname(absolutePath), {recursive: true}) + await writeFile(absolutePath, content) + }), + ) +} + +async function makeApp(files: Record): Promise { + const directory = await makeDirectory() + await writeFiles(directory, {'shopify.app.toml': appConfiguration, ...files}) + return directory +} + +function dependencyExecution(result: Awaited>) { + return result.scan.checks_executed.find((execution) => execution.id === 'MISSING_DEPENDENCY_AUDITING')! +} + +function dependencyFindings(result: Awaited>) { + return result.issues.filter((issue) => issue.id === 'MISSING_DEPENDENCY_AUDITING') +} + +describe('dependency-auditing scanner integration', () => { + test('registers one active, framework-independent structured-config implementation', () => { + const definition = DETERMINISTIC_CHECKS.get('MISSING_DEPENDENCY_AUDITING') + expect(definition).toMatchObject({ + version: 1, + lifecycle: 'active', + analysisMode: 'structured_config', + target: 'dependency_auditing', + }) + expect(definition).not.toHaveProperty('requires') + const catalogEntry = RULE_CATALOG.find((entry) => entry.id === 'MISSING_DEPENDENCY_AUDITING') + expect(catalogEntry).toMatchObject({ + title: 'Dependency auditing configuration not detected', + severity: 'low', + points: -5, + }) + expect(catalogEntry).not.toHaveProperty('status') + expect(catalogEntry).not.toHaveProperty('requires') + const registryEntries = getRegistry().filter((entry) => entry.id === 'MISSING_DEPENDENCY_AUDITING') + expect(registryEntries).toEqual([ + expect.objectContaining({ + kind: 'deterministic', + version: 1, + status: 'active', + severity: 'low', + points: -5, + title: 'Dependency auditing configuration not detected', + }), + ]) + }) + + test('is not applicable when no supported manifest declares dependencies', async () => { + const directory = await makeApp({ + 'package.json': JSON.stringify({name: 'empty'}), + '.github/workflows/irrelevant.yml': 'x'.repeat(500_001), + }) + + const result = await scan(directory) + + expect(dependencyFindings(result)).toEqual([]) + expect(dependencyExecution(result)).toMatchObject({ + status: 'not_applicable', + required: false, + applicable: false, + inspected_files: ['package.json'], + findings: 0, + analysis_mode: 'structured_config', + reason: {code: 'no_relevant_files'}, + }) + }) + + test('emits exactly one ordinary low finding and honors low blocking', async () => { + const directory = await makeApp({'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}})}) + + const execution = await scanApp(directory) + const result = execution.scan + + expect(dependencyFindings(result)).toEqual([ + expect.objectContaining({ + id: 'MISSING_DEPENDENCY_AUDITING', + severity: 'low', + points: -5, + title: 'Dependency auditing configuration not detected', + location: {file: 'package.json'}, + found_by: 'static', + rule_version: 1, + }), + ]) + expect(dependencyExecution(result)).toMatchObject({ + status: 'executed', + required: true, + applicable: true, + inspected_files: ['package.json'], + findings: 1, + analysis_mode: 'structured_config', + }) + expect(result.score).toEqual({total: 95, baseline: 100, grade: 'EXCELLENT'}) + expect(formatJson(result)).toContain('MISSING_DEPENDENCY_AUDITING') + expect(execution.trace.findings).toContainEqual( + expect.objectContaining({ + source: 'deterministic', + rule_id: 'MISSING_DEPENDENCY_AUDITING', + rule_version: 1, + severity: 'low', + }), + ) + const submission = buildSubmission(execution.trace, { + cliVersion: '3.99.0', + submittedAt: '2026-09-15T00:01:00.000Z', + }) + expect(submission.report.findings).toContainEqual( + expect.objectContaining({ + source: 'deterministic', + rule_id: 'MISSING_DEPENDENCY_AUDITING', + rule_version: 1, + severity: 'low', + }), + ) + expect(doctorExitCode({...execution, elapsedMilliseconds: 0}, 'low')).toBe(1) + expect(doctorExitCode({...execution, elapsedMilliseconds: 0}, 'medium')).toBe(0) + expect(doctorExitCode({...execution, elapsedMilliseconds: 0}, 'none')).toBe(0) + }) + + test('treats an empty allowlisted CI file as negative evidence, not an unresolved parse', async () => { + const directory = await makeApp({ + 'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), + '.github/workflows/dependencies.yml': '', + }) + + const result = await scan(directory) + + expect(dependencyFindings(result)).toHaveLength(1) + expect(dependencyExecution(result)).toMatchObject({ + status: 'executed', + inspected_files: ['package.json', '.github/workflows/dependencies.yml'], + findings: 1, + }) + }) + + test('passes when allowlisted CI runs auditing without executing it or making network requests', async () => { + const directory = await makeApp({ + 'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), + '.github/workflows/dependencies.yml': `on: pull_request +jobs: + audit: + runs-on: ubuntu-latest + steps: + - run: npm audit +`, + }) + + const result = await scan(directory) + + expect(dependencyFindings(result)).toEqual([]) + expect(dependencyExecution(result)).toMatchObject({ + status: 'executed', + inspected_files: ['package.json', '.github/workflows/dependencies.yml'], + findings: 0, + }) + expect( + vi.mocked(captureOutputWithExitCode).mock.calls.map(([command, arguments_]) => [command, arguments_]), + ).toEqual([ + ['git', ['rev-parse', 'HEAD']], + ['git', ['status', '--porcelain']], + ]) + expect(fetch).not.toHaveBeenCalled() + }) + + test.each([ + ['malformed', '.github/workflows/dependencies.yml', 'jobs: ['], + ['unreadable', '.github/workflows/oversized.yml', 'x'.repeat(500_001)], + ])('leaves %s configuration unresolved without a missing finding', async (_kind, path, content) => { + const directory = await makeApp({ + 'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), + [path]: content, + }) + + const result = await scan(directory) + + expect(dependencyFindings(result)).toEqual([]) + expect(dependencyExecution(result)).toMatchObject({ + status: 'unresolved', + required: true, + applicable: true, + inspected_files: _kind === 'unreadable' ? ['package.json'] : ['package.json', path], + findings: 0, + reason: {code: expect.stringMatching(/^(parser_unavailable|input_rejected)$/)}, + }) + expect(result.score).toBeNull() + expect(result.scan.coverage_gaps).toContainEqual( + expect.objectContaining({code: 'unresolved_check', check_id: 'MISSING_DEPENDENCY_AUDITING'}), + ) + }) + + test('suppresses the missing finding when any relevant manifest cannot be parsed', async () => { + const directory = await makeApp({ + 'package.json': '{invalid', + 'packages/web/package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), + }) + + const result = await scan(directory) + + expect(dependencyFindings(result)).toEqual([]) + expect(dependencyExecution(result)).toMatchObject({ + status: 'unresolved', + findings: 0, + inspected_files: ['package.json', 'packages/web/package.json'], + reason: {code: 'parser_unavailable'}, + }) + }) + + test('leaves an app below its repository root unresolved without a missing finding', async () => { + const repository = await makeDirectory('app-doctor-dependency-repository-') + const directory = join(repository, 'apps', 'example') + await mkdir(join(repository, '.git')) + await writeFiles(directory, { + 'shopify.app.toml': appConfiguration, + 'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), + }) + + const result = await scan(directory) + + expect(dependencyFindings(result)).toEqual([]) + expect(dependencyExecution(result)).toMatchObject({ + status: 'unresolved', + inspected_files: ['package.json'], + findings: 0, + reason: {code: 'parser_unavailable', message: expect.stringContaining('nested below repository root')}, + }) + }) + + test('hashes exact configuration bytes and tracks configuration changes, additions, and deletion', async () => { + const directory = await makeApp({'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}})}) + const configurationPath = join(directory, '.github', 'workflows', 'dependencies.yml') + const initial = await scan(directory) + const firstContent = 'jobs:\n audit:\n steps:\n - run: npm audit\n' + await writeFiles(directory, {'.github/workflows/dependencies.yml': firstContent}) + const added = await scan(directory) + const secondContent = `${firstContent}# changed without normalization\r\n` + await writeFile(configurationPath, secondContent) + const changed = await scan(directory) + await unlink(configurationPath) + const deleted = await scan(directory) + + expect(initial.scan.file_hashes).not.toHaveProperty('.github/workflows/dependencies.yml') + expect(added.scan.file_hashes?.['.github/workflows/dependencies.yml']).toBe(sha256(firstContent)) + expect(changed.scan.file_hashes?.['.github/workflows/dependencies.yml']).toBe(sha256(secondContent)) + expect(deleted.scan.file_hashes).not.toHaveProperty('.github/workflows/dependencies.yml') + expect(new Set([initial.scan.input_hash, added.scan.input_hash, changed.scan.input_hash])).toHaveProperty('size', 3) + expect(deleted.scan.input_hash).toBe(initial.scan.input_hash) + expect(dependencyExecution(changed).inspected_files).toEqual(['package.json', '.github/workflows/dependencies.yml']) + }) + + test('routes only paths, hashes, and static metadata through output, trace, and submission', async () => { + const privateConfigurationMarker = 'DO_NOT_SUBMIT_DEPENDENCY_AUDIT_CONFIGURATION_CONTENT' + const privateConfigurationContent = `# ${privateConfigurationMarker} +jobs: + audit: + steps: + - run: npm audit +` + const directory = await makeApp({ + 'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), + '.github/workflows/dependencies.yml': privateConfigurationContent, + }) + const result = await scan(directory) + const trace = compileTrace(result, {generatedAt: '2026-09-15T00:00:00.000Z'}) + const submission = buildSubmission(trace, { + cliVersion: '3.99.0', + submittedAt: '2026-09-15T00:01:00.000Z', + }) + const serializedOutputs = [formatJson(result), JSON.stringify(trace), JSON.stringify(submission)] + + expect(validateTrace(trace)).toEqual({valid: true, errors: []}) + expect(trace.project.input_hashes['.github/workflows/dependencies.yml']).toBe(sha256(privateConfigurationContent)) + expect(trace.checks_executed).toContainEqual( + expect.objectContaining({ + id: 'MISSING_DEPENDENCY_AUDITING', + analysis_mode: 'structured_config', + status: 'executed', + inspected_files: ['package.json', '.github/workflows/dependencies.yml'], + }), + ) + expect(submission.report.checks_executed).toContainEqual( + expect.objectContaining({ + id: 'MISSING_DEPENDENCY_AUDITING', + analysis_mode: 'structured_config', + status: 'executed', + inspected_file_count: 2, + finding_count: 0, + }), + ) + for (const output of serializedOutputs) expect(output).not.toContain(privateConfigurationMarker) + }) +}) diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/deterministic-rules.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/deterministic-rules.test.ts index 0b5cadb715d..3224582cc8b 100644 --- a/packages/app/src/cli/services/app-doctor-engine/tests/deterministic-rules.test.ts +++ b/packages/app/src/cli/services/app-doctor-engine/tests/deterministic-rules.test.ts @@ -16,6 +16,7 @@ import type {SourceFile} from '../rules/types.js' const ACTIVE_IDS = [ 'MISSING_COMPLIANCE_WEBHOOKS', + 'MISSING_DEPENDENCY_AUDITING', 'EOL_API_VERSION', 'EXPIRING_OFFLINE_TOKEN', 'UNAUTHENTICATED_ENDPOINT', @@ -39,7 +40,7 @@ const source = (content: string, path = 'app/routes/example.tsx'): SourceFile => }) describe('deterministic rules product contract', () => { - test('has exactly fourteen active executable deterministic identities', () => { + test('has exactly fifteen active executable deterministic identities', () => { expect([...DETERMINISTIC_CHECKS.keys()].sort()).toEqual(ACTIVE_IDS) expect([...DETERMINISTIC_CHECKS.values()].every((check) => check.lifecycle === 'active' && check.runner)).toBe(true) const registry = getRegistry() diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/rule-analysis.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/rule-analysis.test.ts index aca081967be..0faf11d762a 100644 --- a/packages/app/src/cli/services/app-doctor-engine/tests/rule-analysis.test.ts +++ b/packages/app/src/cli/services/app-doctor-engine/tests/rule-analysis.test.ts @@ -35,6 +35,7 @@ function context( extensions: [], sourceFiles: input.files ?? [], manifests: [], + dependencyAuditing: {files: []}, sensitiveFiles: [], capabilities: { theme_app_extension: false, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3cda6dbddfc..91f97063e57 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -236,6 +236,9 @@ importers: ws: specifier: 8.21.0 version: 8.21.0 + yaml: + specifier: 2.9.0 + version: 2.9.0 devDependencies: '@types/diff': specifier: ^5.0.3 From 511403ecc95533e1fe0f3d042c46d8f0a1745801 Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Tue, 15 Sep 2026 15:25:58 -0500 Subject: [PATCH 2/4] Simplify App Doctor dependency checks to configuration file presence Co-authored-by: AI (Pi/GPT-6 Astra) --- .../add-app-doctor-dependency-auditing.md | 5 - .../add-app-doctor-dependency-automation.md | 5 + packages/app/package.json | 3 +- .../app-doctor-engine/rules/catalog.ts | 8 +- .../dependency-auditing-github-schema.ts | 57 -- .../rules/dependency-auditing-rules.ts | 737 ------------------ .../rules/dependency-automation-rules.ts | 54 ++ .../services/app-doctor-engine/rules/types.ts | 6 +- .../app-doctor-engine/scanners/discover.ts | 37 +- .../app-doctor-engine/scanners/index.ts | 38 +- .../app-doctor-engine/scanners/types.ts | 6 +- .../dependency-auditing-discovery.test.ts | 261 ------- .../tests/dependency-auditing-rules.test.ts | 465 ----------- .../tests/dependency-auditing.test.ts | 344 -------- .../dependency-automation-discovery.test.ts | 168 ++++ .../tests/dependency-automation-rules.test.ts | 55 ++ .../tests/dependency-automation.test.ts | 213 +++++ .../tests/deterministic-rules.test.ts | 2 +- .../tests/rule-analysis.test.ts | 2 +- pnpm-lock.yaml | 3 - 20 files changed, 537 insertions(+), 1932 deletions(-) delete mode 100644 .changeset/add-app-doctor-dependency-auditing.md create mode 100644 .changeset/add-app-doctor-dependency-automation.md delete mode 100644 packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-github-schema.ts delete mode 100644 packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-rules.ts create mode 100644 packages/app/src/cli/services/app-doctor-engine/rules/dependency-automation-rules.ts delete mode 100644 packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-discovery.test.ts delete mode 100644 packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-rules.test.ts delete mode 100644 packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing.test.ts create mode 100644 packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-discovery.test.ts create mode 100644 packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-rules.test.ts create mode 100644 packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts diff --git a/.changeset/add-app-doctor-dependency-auditing.md b/.changeset/add-app-doctor-dependency-auditing.md deleted file mode 100644 index 0aa19137288..00000000000 --- a/.changeset/add-app-doctor-dependency-auditing.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@shopify/app': patch ---- - -Add an App Doctor check for dependency vulnerability auditing in GitHub Actions. diff --git a/.changeset/add-app-doctor-dependency-automation.md b/.changeset/add-app-doctor-dependency-automation.md new file mode 100644 index 00000000000..b8be89382f3 --- /dev/null +++ b/.changeset/add-app-doctor-dependency-automation.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +Check for the presence of a local Dependabot or Renovate configuration file in App Doctor. diff --git a/packages/app/package.json b/packages/app/package.json index b8707057712..6176cc4a125 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -78,8 +78,7 @@ "proper-lockfile": "4.1.2", "react": "19.2.4", "which": "4.0.0", - "ws": "8.21.0", - "yaml": "2.9.0" + "ws": "8.21.0" }, "devDependencies": { "@types/diff": "^5.0.3", diff --git a/packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts b/packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts index c8f2b98bde9..3b6fa0b89bd 100644 --- a/packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts +++ b/packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts @@ -146,12 +146,12 @@ export const RULE_CATALOG: RuleCatalogEntry[] = [ guide: 'https://shopify.dev/docs/apps/webhooks/configuration/mandatory-webhooks', }, { - id: 'MISSING_DEPENDENCY_AUDITING', - title: 'Dependency auditing configuration not detected', + id: 'MISSING_DEPENDENCY_SECURITY_AUTOMATION', + title: 'Dependency management configuration file not detected', severity: 'low', points: -5, - description: 'Detects apps with dependencies but no recognized automated dependency vulnerability auditing.', - fix: 'Add dependency vulnerability auditing to an allowlisted CI configuration for the app.', + description: 'Looks for a local Dependabot or Renovate configuration file without validating its contents.', + fix: 'Configure Dependabot or Renovate for this app, or verify existing coverage.', }, { id: 'EOL_API_VERSION', diff --git a/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-github-schema.ts b/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-github-schema.ts deleted file mode 100644 index a7d22f3fe04..00000000000 --- a/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-github-schema.ts +++ /dev/null @@ -1,57 +0,0 @@ -import {zod} from '@shopify/cli-kit/node/schema' - -// Validate executable children separately: disabled jobs, unused definitions, and -// overridden defaults must not prevent analysis of the configuration we inspect. -export const GitHubWorkflowSchema = zod.object({ - jobs: zod.record(zod.unknown()).optional(), - defaults: zod.unknown(), -}) - -export const GitHubReusableDefinitionSchema = zod.object({ - on: zod.union([ - zod.literal('workflow_call'), - zod.tuple([zod.literal('workflow_call')]), - zod.record(zod.unknown()).refine((events) => Object.keys(events).length === 1 && 'workflow_call' in events), - ]), -}) - -// Headers select which schema to apply without validating disabled execution paths. -export const GitHubJobHeaderSchema = zod.object({if: zod.unknown(), uses: zod.unknown()}) -export const GitHubStepsJobSchema = zod.object({steps: zod.array(zod.unknown()), defaults: zod.unknown()}) -export const GitHubReusableJobSchema = zod.object({uses: zod.string(), with: zod.unknown()}) -export const GitHubStepHeaderSchema = GitHubJobHeaderSchema.extend({run: zod.unknown()}) - -export const GitHubRunStepSchema = zod.object({ - run: zod.string(), - // Selection and shell/path validation happen after applying defaults and overrides. - shell: zod.unknown(), - 'working-directory': zod.unknown(), -}) -export const GitHubActionStepSchema = zod.object({ - if: zod.unknown(), - uses: zod.string(), - with: zod.record(zod.unknown()).optional(), -}) - -export const GitHubDefaultsSchema = zod - .object({ - run: zod.object({shell: zod.unknown(), 'working-directory': zod.unknown()}).optional(), - }) - .default({}) - -// OSV inputs can change scan coverage. Do not strip unknown keys before checking them. -export const OsvWorkflowInputsSchema = zod.object({}).strict().optional() - -export const DependencyReviewInputsSchema = zod - .object({ - 'config-file': zod.unknown(), - 'vulnerability-check': zod.unknown(), - }) - .default({}) - -export const SnykCommandInputsSchema = zod.object({command: zod.string().optional()}).default({}) -export const SnykArgsInputsSchema = zod.object({args: zod.string().default('')}).default({}) - -export type GitHubRunStep = zod.infer -export type GitHubActionStep = zod.infer -export type GitHubDefaults = zod.infer diff --git a/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-rules.ts b/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-rules.ts deleted file mode 100644 index eb8b35f7e9e..00000000000 --- a/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-rules.ts +++ /dev/null @@ -1,737 +0,0 @@ -import { - DependencyReviewInputsSchema, - GitHubActionStepSchema, - GitHubDefaultsSchema, - GitHubJobHeaderSchema, - GitHubReusableDefinitionSchema, - GitHubReusableJobSchema, - GitHubRunStepSchema, - GitHubStepHeaderSchema, - GitHubStepsJobSchema, - GitHubWorkflowSchema, - OsvWorkflowInputsSchema, - SnykArgsInputsSchema, - SnykCommandInputsSchema, -} from './dependency-auditing-github-schema.js' -import {parseDocument} from 'yaml' -import type {GitHubActionStep, GitHubDefaults, GitHubRunStep} from './dependency-auditing-github-schema.js' -import type {Issue} from '../types.js' -import type {ManifestFile, ScanContext, SourceFile} from './types.js' - -interface Analysis { - recognized: boolean - obstacles: string[] -} - -interface CommandContext { - directory: string - manifests: ManifestFile[] - allowPackageScript: boolean -} - -type ParsedConfiguration = {ok: true; value: unknown; warnings: string[]} | {ok: false; reason: string} -type ResolvedValue = {ok: true; value: string} | {ok: false} -type OptionalValue = {ok: true; present: boolean; value?: unknown} | {ok: false} -type ParsedShell = {ok: true; segments: string[]} | {ok: false} - -const SUPPORTED_PACKAGE_MANAGERS = new Set(['javascript', 'node', 'npm', 'pnpm', 'yarn']) -const LOCAL_SCRIPT_EXECUTORS = new Set(['.', 'bash', 'node', 'sh', 'source', 'tsx']) -const SHELL_CONTROL_WORDS = new Set([ - 'break', - 'case', - 'continue', - 'do', - 'done', - 'elif', - 'else', - 'eval', - 'exit', - 'false', - 'fi', - 'for', - 'function', - 'if', - 'return', - 'set', - 'then', - 'trap', - 'until', - 'while', -]) -const MAX_YAML_ALIASES = 20 - -/** - * Recognizes a deliberately small set of dependency-auditing executions. - * Unknown indirection is left unresolved rather than interpreted as shell or CI code. - */ -export function scanDependencyAuditing(context: Pick): { - issues: Issue[] - unresolvedReason?: string -} { - const applicableManifests = context.manifests.filter(hasDependencies) - if (applicableManifests.length === 0) return {issues: []} - if (context.dependencyAuditing.unresolvedReason) { - return {issues: [], unresolvedReason: context.dependencyAuditing.unresolvedReason} - } - - const parsedFiles = context.dependencyAuditing.files.map((file) => ({file, parsed: parseConfiguration(file)})) - const malformed = parsedFiles.find(({parsed}) => !parsed.ok) - if (malformed && !malformed.parsed.ok) return {issues: [], unresolvedReason: malformed.parsed.reason} - - const analysis = parsedFiles.reduce((combined, {file, parsed}) => { - if (!parsed.ok) return combined - return combine(combined, analyzeConfiguration(file, parsed, context.manifests)) - }, noEvidence()) - if (analysis.recognized) return {issues: []} - if (analysis.obstacles.length > 0) return {issues: [], unresolvedReason: analysis.obstacles[0]} - - const manifest = [...applicableManifests].sort((left, right) => left.path.localeCompare(right.path))[0]! - return { - issues: [ - { - id: 'MISSING_DEPENDENCY_AUDITING', - severity: 'low', - points: -5, - title: 'Dependency auditing configuration not detected', - message: - 'No recognized dependency-auditing configuration was found in the inspected files. Add an automated dependency vulnerability check, or verify that your existing integration covers this app. Hosted integrations and repository settings were not inspected.', - location: {file: normalizePath(manifest.path)}, - fix: { - automated: false, - description: 'Add an automated dependency vulnerability check for this package.', - }, - }, - ], - } -} - -function hasDependencies(manifest: ManifestFile): boolean { - return Object.keys(manifest.dependencies).length > 0 || Object.keys(manifest.devDependencies ?? {}).length > 0 -} - -function analyzeConfiguration( - file: SourceFile, - parsed: Extract, - manifests: ManifestFile[], -): Analysis { - const path = normalizePath(file.path) - const warnings = parsed.warnings.reduce( - (analysis, warning) => combine(analysis, obstacle(`${warning}: ${path}`)), - noEvidence(), - ) - if (parsed.warnings.length > 0 || parsed.value === null || parsed.value === undefined) return warnings - if (/^\.github\/workflows\/[^/]+\.ya?ml$/i.test(path)) { - return combine(warnings, analyzeGitHubWorkflow(parsed.value, path, manifests)) - } - return warnings -} - -function parseConfiguration(file: SourceFile): ParsedConfiguration { - if (file.content === undefined) { - return {ok: false, reason: `Dependency-auditing configuration was unreadable: ${file.path}`} - } - try { - const document = parseDocument(file.content, {schema: 'core', merge: true, uniqueKeys: true}) - if (document.errors.length > 0) { - return {ok: false, reason: `Dependency-auditing YAML is malformed: ${file.path}`} - } - const value = document.toJS({maxAliasCount: MAX_YAML_ALIASES}) as unknown - const warnings = document.warnings.map(() => 'Unsupported YAML feature prevents complete auditing analysis') - return {ok: true, value, warnings} - // YAML conversion may reject excessive aliases or invalid custom structures. - // eslint-disable-next-line no-catch-all/no-catch-all - } catch { - return {ok: false, reason: `Dependency-auditing YAML could not be parsed safely: ${file.path}`} - } -} - -function analyzeGitHubWorkflow(value: unknown, path: string, manifests: ManifestFile[]): Analysis { - // A reusable-only workflow is a definition, not evidence of invocation. Local callers remain unresolved. - if (GitHubReusableDefinitionSchema.safeParse(value).success) return noEvidence() - const workflowResult = GitHubWorkflowSchema.safeParse(value) - if (!workflowResult.success) return obstacle(`GitHub Actions workflow has an unsupported structure: ${path}`) - const root = workflowResult.data - if (root.jobs === undefined) return noEvidence() - - let analysis = noEvidence() - const workflowDefaults = GitHubDefaultsSchema.safeParse(root.defaults) - const workflowDirectory: OptionalValue = workflowDefaults.success - ? readGitHubDefaultDirectory(workflowDefaults.data) - : {ok: false} - for (const jobValue of Object.values(root.jobs)) { - const jobResult = GitHubJobHeaderSchema.safeParse(jobValue) - if (!jobResult.success) { - analysis = combine(analysis, obstacle(`GitHub Actions workflow has an unsupported job in ${path}`)) - continue - } - const job = jobResult.data - if (isDisabled(job.if)) continue - if (job.uses !== undefined) { - const reusableJob = GitHubReusableJobSchema.safeParse(jobValue) - if (!reusableJob.success) { - analysis = combine(analysis, obstacle(`GitHub Actions reusable workflow has an unsupported structure: ${path}`)) - } else if (isKnownOsvWorkflow(reusableJob.data.uses)) { - analysis = combine(analysis, analyzeOsvWorkflow(reusableJob.data.with, path)) - } else { - analysis = combine(analysis, obstacle(`Unsupported reusable workflow referenced by ${path}`)) - } - continue - } - - const stepsJob = GitHubStepsJobSchema.safeParse(jobValue) - if (!stepsJob.success) { - analysis = combine(analysis, obstacle(`GitHub Actions workflow has an unsupported steps section: ${path}`)) - continue - } - if (stepsJob.data.steps.some(hasUnsupportedCheckout)) { - analysis = combine(analysis, obstacle(`Unsupported checkout repository or path in ${path}`)) - continue - } - const jobDefaults = GitHubDefaultsSchema.safeParse(stepsJob.data.defaults) - const jobDirectory: OptionalValue = jobDefaults.success ? readGitHubDefaultDirectory(jobDefaults.data) : {ok: false} - for (const stepValue of stepsJob.data.steps) { - const stepResult = GitHubStepHeaderSchema.safeParse(stepValue) - if (!stepResult.success) { - analysis = combine(analysis, obstacle(`GitHub Actions workflow has an unsupported step in ${path}`)) - continue - } - const step = stepResult.data - if (isDisabled(step.if)) continue - if (step.uses !== undefined) { - const action = GitHubActionStepSchema.safeParse(stepValue) - if (action.success) { - // defaults.run and working-directory apply only to run steps. Actions execute from checkout root. - analysis = combine(analysis, analyzeGitHubAction(action.data, path, manifests)) - } else { - analysis = combine(analysis, obstacle(`GitHub Actions workflow has an unsupported action step in ${path}`)) - } - } - if (step.run !== undefined) { - const run = GitHubRunStepSchema.safeParse(stepValue) - if (!run.success) { - analysis = combine(analysis, obstacle(`GitHub Actions workflow has an unsupported run step in ${path}`)) - continue - } - const shell = - run.data.shell ?? - (jobDefaults.success ? jobDefaults.data.run?.shell : undefined) ?? - (workflowDefaults.success ? workflowDefaults.data.run?.shell : undefined) - if (shell !== undefined && (typeof shell !== 'string' || !/^(?:bash|sh|cmd|pwsh|powershell)$/.test(shell))) { - analysis = combine(analysis, obstacle(`Unsupported execution shell in ${path}`)) - continue - } - const directory = resolveGitHubRunDirectory(run.data, jobDirectory, workflowDirectory) - analysis = combine( - analysis, - directory.ok - ? analyzeCommands(run.data.run, {directory: directory.value, manifests, allowPackageScript: true}, path) - : obstacle(`Unsupported working-directory prevents auditing analysis in ${path}`), - ) - } - } - } - return analysis -} - -function hasUnsupportedCheckout(value: unknown): boolean { - const result = GitHubActionStepSchema.safeParse(value) - if (!result.success || isDisabled(result.data.if) || !/^actions\/checkout@/i.test(result.data.uses)) return false - const inputs = result.data.with - return ( - inputs?.repository !== undefined || - (inputs?.path !== undefined && inputs.path !== '.' && inputs.path !== './') || - inputs?.['sparse-checkout'] !== undefined - ) -} - -function analyzeOsvWorkflow(inputs: unknown, path: string): Analysis { - return OsvWorkflowInputsSchema.safeParse(inputs).success - ? evidence() - : obstacle(`Unsupported OSV reusable workflow input referenced by ${path}`) -} - -function analyzeGitHubAction(step: GitHubActionStep, workflowPath: string, manifests: ManifestFile[]): Analysis { - const uses = step.uses.toLowerCase() - if (/^actions\/dependency-review-action@[^\s]+$/i.test(uses)) { - const result = DependencyReviewInputsSchema.safeParse(step.with) - if (!result.success) return obstacle(`Unsupported dependency review input in ${workflowPath}`) - const inputs = result.data - if (inputs['config-file'] !== undefined) { - return obstacle(`Dependency review config-file can't be inspected from ${workflowPath}`) - } - const vulnerabilityCheck = inputs?.['vulnerability-check'] - if (isDisabled(vulnerabilityCheck)) return noEvidence() - if (vulnerabilityCheck !== undefined && vulnerabilityCheck !== true && vulnerabilityCheck !== 'true') { - return obstacle(`Unsupported dependency review input in ${workflowPath}`) - } - return evidence() - } - if (/^snyk\/actions\/node(?:-\d+)?@[^\s]+$/i.test(uses)) { - const command = SnykCommandInputsSchema.safeParse(step.with) - if (!command.success) return obstacle(`Unsupported Snyk command input in ${workflowPath}`) - const commandValue = command.data.command - if (commandValue !== undefined && isDynamic(commandValue)) { - return obstacle(`Dynamic Snyk command input in ${workflowPath}`) - } - if (commandValue !== undefined && commandValue.trim() !== 'test') return noEvidence() - const argsResult = SnykArgsInputsSchema.safeParse(step.with) - if (!argsResult.success) return obstacle(`Unsupported Snyk args input in ${workflowPath}`) - const args = argsResult.data.args - const parsedArgs = splitShell(args) - if (!parsedArgs.ok || parsedArgs.segments.length > 1 || isDynamic(args)) { - return obstacle(`Unsupported Snyk args input in ${workflowPath}`) - } - if (/\b(?:code|container)\s+test\b/i.test(args)) return noEvidence() - return analyzeCommands(`snyk test ${args}`, {directory: '', manifests, allowPackageScript: false}, workflowPath) - } - if (/^snyk\/actions\/(?:setup|code|docker|container)@[^\s]+$/i.test(uses)) return noEvidence() - if (uses.startsWith('./')) return obstacle(`Unsupported local action referenced by ${workflowPath}`) - return noEvidence() -} - -function analyzeCommands(command: string, context: CommandContext, configurationPath: string): Analysis { - const parsedShell = splitShell(command) - if (!parsedShell.ok) return obstacle(`Unsupported shell syntax referenced by ${configurationPath}`) - - const segments = parsedShell.segments.map((segment) => ({segment, tokens: tokenize(segment)})) - if ( - segments.some( - ({segment, tokens}) => - segment.includes('`') || segment.includes('$(') || SHELL_CONTROL_WORDS.has(firstExecutable(tokens) ?? ''), - ) - ) { - return obstacle(`Unsupported shell control flow referenced by ${configurationPath}`) - } - - let analysis = noEvidence() - let directory = normalizeDirectory(context.directory) - for (const {tokens} of segments) { - if (tokens.length === 0) continue - const executableIndex = tokens.findIndex((token) => !/^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) - if (executableIndex < 0) continue - const executable = tokens[executableIndex]! - const commandTokens = tokens.slice(executableIndex) - if (executable === 'echo' || executable === 'printf') continue - if (executable === 'cd') { - const target = commandTokens[1] - const resolved = target === undefined ? undefined : resolveDirectory(directory, target) - if (resolved === undefined) { - analysis = combine(analysis, obstacle(`Unsupported shell directory in ${configurationPath}`)) - } else { - directory = resolved - } - continue - } - if (isPotentialUnsupportedPackageInvocation(commandTokens)) { - analysis = combine(analysis, obstacle(`Unsupported package manager selector in ${configurationPath}`)) - continue - } - if (isRecognizedAuditCommand(commandTokens)) { - if (hasHelpOrVersion(commandTokens)) continue - const packageManagerOption = selectedOption(commandTokens, ['--package-manager']) - if (packageManagerOption.present) { - if (!isUsableOptionValue(packageManagerOption.value) || isDynamic(packageManagerOption.value)) { - analysis = combine(analysis, obstacle(`Unsupported scanner package manager in ${configurationPath}`)) - continue - } - if (!SUPPORTED_PACKAGE_MANAGERS.has(packageManagerOption.value.toLowerCase())) continue - } - const targetOption = selectedOption(commandTokens, scannerTargetOptionNames(commandTokens[0])) - if ( - targetOption.present && - (!isUsableOptionValue(targetOption.value) || !isSafeRelativePath(targetOption.value)) - ) { - analysis = combine(analysis, obstacle(`Unsupported scanner target in ${configurationPath}`)) - } else if (commandCoversManifest(commandTokens, directory, targetOption.value, context.manifests)) { - analysis = combine(analysis, evidence()) - } - continue - } - - const packageScript = parsePackageScript(commandTokens) - if (packageScript.kind === 'unsupported') { - analysis = combine(analysis, obstacle(`Unsupported package script invocation in ${configurationPath}`)) - continue - } - if (packageScript.kind === 'script') { - if (!context.allowPackageScript) { - analysis = combine(analysis, obstacle(`Nested package script referenced by ${configurationPath}`)) - continue - } - const scriptDirectory = packageScript.target ? resolveDirectory(directory, packageScript.target) : directory - if (scriptDirectory === undefined) { - analysis = combine(analysis, obstacle(`Unsupported package script target in ${configurationPath}`)) - continue - } - const selectedManifest = manifestInDirectory(context.manifests, scriptDirectory) - const script = selectedManifest?.scripts?.[packageScript.name] - if (!selectedManifest || script === undefined) { - analysis = combine(analysis, obstacle(`Package script can't be resolved in ${configurationPath}`)) - continue - } - analysis = combine( - analysis, - analyzeCommands( - script, - {directory: manifestDirectory(selectedManifest), manifests: context.manifests, allowPackageScript: false}, - configurationPath, - ), - ) - continue - } - - if (isDynamic(executable) || referencesLocalScript(commandTokens) || invokesShellCommand(commandTokens)) { - analysis = combine(analysis, obstacle(`Unsupported local script referenced by ${configurationPath}`)) - } - } - - // An unsupported construct in this execution block may govern any command in that block. - return analysis.obstacles.length > 0 ? {recognized: false, obstacles: analysis.obstacles} : analysis -} - -function isRecognizedAuditCommand(tokens: string[]): boolean { - const words = tokens.map((token) => token.toLowerCase()) - const executable = words[0] - if (executable === 'npm' || executable === 'pnpm') return packageManagerCommand(words) === 'audit' - if (executable === 'yarn') { - const commandWords = packageManagerCommandWords(words) - return commandWords[0] === 'audit' || (commandWords[0] === 'npm' && commandWords[1] === 'audit') - } - let snykIndex = -1 - if (executable === 'snyk') snykIndex = 0 - else if (executable === 'npx' && words[1] === 'snyk') snykIndex = 1 - if (snykIndex >= 0) return words[snykIndex + 1] === 'test' - return executable === 'semgrep' && words[1] === 'ci' && words.includes('--supply-chain') -} - -function packageManagerCommand(tokens: string[]): string | undefined { - return packageManagerCommandWords(tokens)[0] -} - -function packageManagerCommandWords(tokens: string[]): string[] { - const words = tokens.slice(1) - const manager = tokens[0] - const supportedOptions = packageManagerDirectoryOptionNames(manager) - while (words[0]?.startsWith('-')) { - const option = words.shift()! - if (!option.includes('=') && supportedOptions.includes(option)) words.shift() - } - return words -} - -type PackageScript = {kind: 'none'} | {kind: 'unsupported'} | {kind: 'script'; name: string; target?: string} - -function parsePackageScript(tokens: string[]): PackageScript { - const manager = tokens[0]?.toLowerCase() - if (!manager || !['npm', 'pnpm', 'yarn'].includes(manager)) return {kind: 'none'} - const targetOption = selectedOption(tokens, scannerTargetOptionNames(manager)) - if (targetOption.present && (!isUsableOptionValue(targetOption.value) || !isSafeRelativePath(targetOption.value))) { - return {kind: 'unsupported'} - } - const target = targetOption.value - const words = packageManagerCommandWords(tokens.map((token) => token.toLowerCase())) - const commandIndex = tokens.length - words.length - if (['install', 'add', 'remove', 'update', 'upgrade', 'login', 'logout', 'config'].includes(words[0] ?? '')) { - return {kind: 'none'} - } - if (words[0] === 'run' || words[0] === 'run-script') { - const scriptName = tokens[commandIndex + 1] - if (words.length !== 2 || !scriptName || !/^[A-Za-z0-9_.:-]+$/.test(scriptName)) { - return {kind: 'unsupported'} - } - return {kind: 'script', name: scriptName, target} - } - if ( - (manager === 'pnpm' || manager === 'yarn') && - words.length === 1 && - words[0] && - /^[A-Za-z0-9_.:-]+$/.test(words[0]) - ) { - return {kind: 'script', name: tokens[commandIndex]!, target} - } - return {kind: 'none'} -} - -function isPotentialUnsupportedPackageInvocation(tokens: string[]): boolean { - const manager = tokens[0]?.toLowerCase() - if (!manager || !['npm', 'pnpm', 'yarn'].includes(manager)) return false - const lower = tokens.map((token) => token.toLowerCase()) - const invokesAuditOrScript = lower.includes('audit') || lower.includes('run') || lower.includes('run-script') - if (!invokesAuditOrScript) return false - if ( - lower.some( - (token) => /^(?:--filter|--workspace|--workspaces|--recursive)(?:=|$)/i.test(token) || /^-[frw]/i.test(token), - ) - ) { - return true - } - - const supportedOptions = packageManagerDirectoryOptionNames(manager) - for (let index = 1; index < lower.length && lower[index]?.startsWith('-'); index++) { - const optionName = lower[index]!.split('=')[0]! - if (!supportedOptions.includes(optionName)) return true - if (!lower[index]!.includes('=')) index++ - } - return false -} - -function packageManagerDirectoryOptionNames(manager: string | undefined): string[] { - if (manager === 'npm') return ['--prefix'] - if (manager === 'pnpm') return ['--dir', '-c'] - return ['--cwd'] -} - -function scannerTargetOptionNames(executable: string | undefined): string[] { - const normalizedExecutable = executable?.toLowerCase() - if (normalizedExecutable === 'npm') return ['--file', '--target-file', '--prefix'] - if (normalizedExecutable === 'pnpm') return ['--file', '--target-file', '--dir', '-c'] - if (normalizedExecutable === 'yarn') return ['--file', '--target-file', '--cwd'] - return ['--file', '--target-file'] -} - -function invokesShellCommand(tokens: string[]): boolean { - return ['bash', 'sh'].includes(tokens[0]?.toLowerCase() ?? '') && tokens.slice(1).some((token) => token === '-c') -} - -function referencesLocalScript(tokens: string[]): boolean { - const executable = tokens[0]?.toLowerCase() - if (!executable) return false - if (/^(?:\.\.?\/|\/).+/.test(executable)) return true - if (!LOCAL_SCRIPT_EXECUTORS.has(executable)) return false - return tokens.slice(1).some((token) => /(?:^|\/)[^\s]+\.(?:[cm]?[jt]sx?|sh)$/.test(token)) -} - -function commandCoversManifest( - tokens: string[], - directory: string, - target: string | undefined, - manifests: ManifestFile[], -): boolean { - if (target) return targetCoversManifest(directory, target, manifests) - const executable = tokens[0]?.toLowerCase() - const scansRecursively = - (executable === 'semgrep' && tokens.includes('--supply-chain')) || tokens.includes('--all-projects') - if (!scansRecursively) return targetCoversManifest(directory, undefined, manifests) - const normalizedDirectory = normalizeDirectory(directory) - return manifests.some((manifest) => { - if (!hasDependencies(manifest)) return false - const candidateDirectory = manifestDirectory(manifest) - return ( - !normalizedDirectory || - candidateDirectory === normalizedDirectory || - candidateDirectory.startsWith(`${normalizedDirectory}/`) - ) - }) -} - -function targetCoversManifest( - directory: string | undefined, - target: string | undefined, - manifests: ManifestFile[], -): boolean { - const baseDirectory = normalizeDirectory(directory ?? '') - if (!target) { - return manifests.some((manifest) => hasDependencies(manifest) && manifestDirectory(manifest) === baseDirectory) - } - const normalizedTarget = resolveDirectory(baseDirectory, target) - if (normalizedTarget === undefined) return false - const targetBasename = normalizedTarget.split('/').at(-1) ?? '' - const targetDirectory = /^(?:package\.json|package-lock\.json|pnpm-lock\.yaml|yarn\.lock)$/i.test(targetBasename) - ? normalizeDirectory(normalizedTarget.slice(0, -(targetBasename.length + (normalizedTarget.includes('/') ? 1 : 0)))) - : normalizedTarget - return manifests.some((manifest) => hasDependencies(manifest) && manifestDirectory(manifest) === targetDirectory) -} - -function manifestInDirectory(manifests: ManifestFile[], directory: string): ManifestFile | undefined { - return manifests.find((manifest) => manifestDirectory(manifest) === normalizeDirectory(directory)) -} - -function manifestDirectory(manifest: ManifestFile): string { - const path = normalizePath(manifest.path) - return normalizeDirectory(path.includes('/') ? path.slice(0, path.lastIndexOf('/')) : '') -} - -function resolveGitHubRunDirectory( - step: GitHubRunStep, - jobDirectory: OptionalValue, - workflowDirectory: OptionalValue, -): ResolvedValue { - if (step['working-directory'] !== undefined) return resolveSelectedDirectory(step['working-directory']) - if (!jobDirectory.ok) return {ok: false} - if (jobDirectory.present) return resolveSelectedDirectory(jobDirectory.value) - if (!workflowDirectory.ok) return {ok: false} - return resolveSelectedDirectory(workflowDirectory.value) -} - -function readGitHubDefaultDirectory(defaults: GitHubDefaults): OptionalValue { - const directory = defaults.run?.['working-directory'] - return {ok: true, present: directory !== undefined, value: directory} -} - -function resolveSelectedDirectory(value: unknown): ResolvedValue { - if (value === undefined) return {ok: true, value: ''} - if (typeof value !== 'string' || !isSafeRelativePath(value)) return {ok: false} - return {ok: true, value: normalizeDirectory(value)} -} - -function normalizeDirectory(value: string): string { - return normalizePath(value) - .split('/') - .filter((part) => part && part !== '.') - .join('/') -} - -function resolveDirectory(base: string, target: string): string | undefined { - if (!isSafeRelativePath(target)) return undefined - const normalizedBase = normalizeDirectory(base) - const normalizedTarget = normalizeDirectory(target) - return [normalizedBase, normalizedTarget].filter(Boolean).join('/') -} - -function isSafeRelativePath(value: string): boolean { - const normalized = normalizePath(value.trim()) - if (!normalized || normalized === '.') return true - if ( - isDynamic(normalized) || - normalized.startsWith('/') || - /^[A-Za-z]:\//.test(normalized) || - normalized.startsWith('~/') - ) { - return false - } - return !normalized.split('/').includes('..') -} - -function normalizePath(path: string): string { - return path.replace(/\\/g, '/').replace(/^\.\//, '') -} - -function isKnownOsvWorkflow(uses: string): boolean { - return /^google\/osv-scanner\/\.github\/workflows\/osv-scanner-reusable(?:-pr)?\.yml@[^\s]+$/i.test(uses) -} - -function isDisabled(value: unknown): boolean { - return value === false || (typeof value === 'string' && /^(?:false|\$\{\{\s*false\s*\}\})$/i.test(value.trim())) -} - -function isDynamic(value: string): boolean { - return /\$|`|\{\{/.test(value) -} - -function selectedOption(tokens: string[], names: string[]): {present: boolean; value?: string} { - for (let index = 0; index < tokens.length; index++) { - const token = tokens[index]! - for (const name of names) { - if (token.toLowerCase() === name) return {present: true, value: tokens[index + 1]} - if (token.toLowerCase().startsWith(`${name}=`)) { - return {present: true, value: token.slice(name.length + 1)} - } - } - } - return {present: false} -} - -function isUsableOptionValue(value: string | undefined): value is string { - return value !== undefined && value.length > 0 && !value.startsWith('-') -} - -function hasHelpOrVersion(tokens: string[]): boolean { - return tokens.some((token) => ['--help', '--version', '-h', '-v'].includes(token.toLowerCase())) -} - -function firstExecutable(tokens: string[]): string | undefined { - return tokens.find((token) => !/^[A-Za-z_][A-Za-z0-9_]*=/.test(token))?.toLowerCase() -} - -/** Split only sequential basic shell statements. Conditional operators and ambiguous comments are unsupported. */ -function splitShell(command: string): ParsedShell { - const segments: string[] = [] - let current = '' - let quote: string | undefined - let escaped = false - let comment = false - for (const character of command) { - if (comment) { - if (character === '\\') return {ok: false} - if (character === '\n') comment = false - else continue - } - if (escaped) { - if (character !== '\n') return {ok: false} - escaped = false - continue - } - if (character === '\\' && quote !== "'") { - escaped = true - continue - } - if (quote) { - current += character - if (character === quote) quote = undefined - continue - } - if (character === "'" || character === '"') { - quote = character - current += character - continue - } - if (character === '#' && (current.length === 0 || /\s/.test(current.at(-1)!))) { - comment = true - continue - } - // Redirections, heredocs, subshells, and function bodies require a shell interpreter. - if ('|&<>(){}'.includes(character)) return {ok: false} - if (character === '\n' || character === ';') { - if (current.trim()) segments.push(current.trim()) - current = '' - continue - } - current += character - } - if (quote || escaped) return {ok: false} - if (current.trim()) segments.push(current.trim()) - return {ok: true, segments} -} - -function tokenize(command: string): string[] { - const tokens: string[] = [] - let current = '' - let quote: string | undefined - let escaped = false - for (const character of command) { - if (escaped) { - current += character - escaped = false - } else if (character === '\\' && quote !== "'") { - escaped = true - } else if (quote) { - if (character === quote) quote = undefined - else current += character - } else if (character === "'" || character === '"') { - quote = character - } else if (/\s/.test(character)) { - if (current) tokens.push(current) - current = '' - } else { - current += character - } - } - if (current) tokens.push(current) - return tokens -} - -function combine(left: Analysis, right: Analysis): Analysis { - return {recognized: left.recognized || right.recognized, obstacles: [...left.obstacles, ...right.obstacles]} -} - -function evidence(): Analysis { - return {recognized: true, obstacles: []} -} - -function obstacle(reason: string): Analysis { - return {recognized: false, obstacles: [reason]} -} - -function noEvidence(): Analysis { - return {recognized: false, obstacles: []} -} diff --git a/packages/app/src/cli/services/app-doctor-engine/rules/dependency-automation-rules.ts b/packages/app/src/cli/services/app-doctor-engine/rules/dependency-automation-rules.ts new file mode 100644 index 00000000000..90723e6d51d --- /dev/null +++ b/packages/app/src/cli/services/app-doctor-engine/rules/dependency-automation-rules.ts @@ -0,0 +1,54 @@ +import type {Issue} from '../types.js' +import type {ScanContext} from './types.js' + +export const DEPENDENCY_AUTOMATION_CONFIG_PATHS = [ + '.github/dependabot.yml', + '.github/dependabot.yaml', + 'renovate.json', + 'renovate.jsonc', + 'renovate.json5', + '.github/renovate.json', + '.github/renovate.jsonc', + '.github/renovate.json5', + '.gitlab/renovate.json', + '.gitlab/renovate.jsonc', + '.gitlab/renovate.json5', + '.renovaterc', + '.renovaterc.json', + '.renovaterc.jsonc', + '.renovaterc.json5', +] + +/** File presence is an adoption signal, not proof of valid configuration, execution, or dependency coverage. */ +export function scanDependencyAutomation(context: Pick): { + issues: Issue[] + unresolvedReason?: string +} { + const manifests = context.manifests.filter( + (manifest) => + Object.keys(manifest.dependencies).length > 0 || Object.keys(manifest.devDependencies ?? {}).length > 0, + ) + if (manifests.length === 0) return {issues: []} + const {files, unresolvedReason} = context.dependencyAutomation + if (unresolvedReason) return {issues: [], unresolvedReason} + if (files.length > 0) return {issues: []} + + const manifest = [...manifests].sort((left, right) => left.path.localeCompare(right.path))[0]! + return { + issues: [ + { + id: 'MISSING_DEPENDENCY_SECURITY_AUTOMATION', + severity: 'low', + points: -5, + title: 'Dependency management configuration file not detected', + message: + 'No recognized Dependabot or Renovate configuration file was found. Add dependency update automation, or verify that an existing integration covers this app. This check only looks for local configuration files; it does not validate their contents or inspect hosted integrations, CI workflows, or execution results.', + location: {file: manifest.path.replace(/\\/g, '/')}, + fix: { + automated: false, + description: 'Configure Dependabot or Renovate for this app, or verify existing coverage.', + }, + }, + ], + } +} diff --git a/packages/app/src/cli/services/app-doctor-engine/rules/types.ts b/packages/app/src/cli/services/app-doctor-engine/rules/types.ts index a3f39c53b79..783da46584d 100644 --- a/packages/app/src/cli/services/app-doctor-engine/rules/types.ts +++ b/packages/app/src/cli/services/app-doctor-engine/rules/types.ts @@ -1,7 +1,7 @@ import type {Issue, Capabilities, ProjectDetection, Severity, SourceCandidate} from '../types.js' import type { AppTomlContent, - DependencyAuditingInputs, + DependencyAutomationInputs, ExtensionInfo, ManifestFile, SourceFile, @@ -46,8 +46,8 @@ export interface ScanContext { sourceFiles: SourceFile[] /** Supported JavaScript package manifests found. */ manifests: ManifestFile[] - /** Allowlisted dependency-auditing configuration and discovery obstacles. */ - dependencyAuditing: DependencyAuditingInputs + /** Local dependency-management configuration and discovery obstacles. */ + dependencyAutomation: DependencyAutomationInputs /** Safely readable repository text evidence available to secret scanning. */ sensitiveFiles: SourceFile[] /** Detected capabilities */ diff --git a/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts b/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts index 559387a8003..8e451b7cd3b 100644 --- a/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts +++ b/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts @@ -1,3 +1,4 @@ +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 {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' @@ -15,11 +16,11 @@ import { } from '@shopify/cli-kit/node/path' import {zod} from '@shopify/cli-kit/node/schema' import {decodeToml} from '@shopify/cli-kit/node/toml/codec' -import {lstatSync, readdirSync, realpathSync} from 'node:fs' +import {lstatSync, realpathSync} from 'node:fs' import type {SourceCandidate} from '../types.js' import type { AppTomlContent, - DependencyAuditingInputs, + DependencyAutomationInputs, ExtensionInfo, SourceFile, ManifestFile, @@ -669,7 +670,7 @@ function isMissingFilesystemEntry(error: unknown): boolean { * directory link, allowing contained links but rejecting escapes and dangling * links explicitly. */ -function inspectAllowedPath(appRoot: string, relative: string, expectedType: 'file' | 'directory'): AllowedPathResult { +function inspectAllowedPath(appRoot: string, relative: string): AllowedPathResult { const path = resolvePath(appRoot, relative) if (!isSubpath(appRoot, path)) return {exists: false, unresolvedReason: `${relative} escapes the app root`} @@ -686,7 +687,7 @@ function inspectAllowedPath(appRoot: string, relative: string, expectedType: 'fi const stats = lstatSync(canonicalPath) const isLastSegment = index === segments.length - 1 - const requiredType = isLastSegment ? expectedType : 'directory' + const requiredType = isLastSegment ? 'file' : 'directory' const hasRequiredType = requiredType === 'file' ? stats.isFile() : stats.isDirectory() if (!hasRequiredType) { return {exists: false, unresolvedReason: `${relative} contains an entry that is not a ${requiredType}`} @@ -749,8 +750,8 @@ function nestedRepositoryReason(appRoot: string): string | undefined { } } -/** Discover only repository CI files that can explicitly invoke dependency auditing. */ -export function findDependencyAuditingInputs(appRoot: string): DependencyAuditingInputs { +/** Read local bot configuration only; hosted integrations and CI workflows are outside this check's scope. */ +export function findDependencyAutomationInputs(appRoot: string): DependencyAutomationInputs { let canonicalRoot: string try { canonicalRoot = realpathSync(resolvePath(appRoot)) @@ -765,25 +766,9 @@ export function findDependencyAuditingInputs(appRoot: string): DependencyAuditin const repositoryReason = nestedRepositoryReason(canonicalRoot) if (repositoryReason) return {files: [], unresolvedReason: repositoryReason} - const candidatePaths: string[] = [] - const workflows = inspectAllowedPath(canonicalRoot, '.github/workflows', 'directory') - if (workflows.unresolvedReason) return {files: [], unresolvedReason: workflows.unresolvedReason} - if (workflows.exists) { - try { - candidatePaths.push( - ...readdirSync(workflows.path!, {withFileTypes: true}) - .filter((entry) => /\.ya?ml$/u.test(entry.name)) - .map((entry) => `.github/workflows/${entry.name}`), - ) - // eslint-disable-next-line no-catch-all/no-catch-all - } catch (error) { - return {files: [], unresolvedReason: filesystemError('.github/workflows', error)} - } - } - const files: SourceFile[] = [] - for (const relative of [...new Set(candidatePaths)].sort()) { - const inspected = inspectAllowedPath(canonicalRoot, relative, 'file') + for (const relative of DEPENDENCY_AUTOMATION_CONFIG_PATHS) { + const inspected = inspectAllowedPath(canonicalRoot, relative) if (inspected.unresolvedReason) return {files, unresolvedReason: inspected.unresolvedReason} if (!inspected.exists) continue @@ -798,6 +783,8 @@ export function findDependencyAuditingInputs(appRoot: string): DependencyAuditin if (!result.ok && result.reason === 'unreadable') { return {files, unresolvedReason: `Could not read ${relative}: ${result.detail ?? 'unknown filesystem error'}`} } + // One safely inspected configuration file is enough for this presence-only check. + break } return {files} } @@ -818,7 +805,6 @@ export function findManifestPaths(appRoot: string): string[] { const PackageManifestSchema = zod.object({ dependencies: zod.record(zod.string()).optional(), devDependencies: zod.record(zod.string()).optional(), - scripts: zod.record(zod.string()).optional(), }) export function findManifests(appRoot: string, discoveredPaths = findManifestPaths(appRoot)): ManifestFile[] { @@ -839,7 +825,6 @@ export function findManifests(appRoot: string, discoveredPaths = findManifestPat content, dependencies: pkg.dependencies ?? {}, devDependencies: pkg.devDependencies ?? {}, - scripts: pkg.scripts, }) // Invalid repository JSON is a coverage gap, not a scanner crash. // eslint-disable-next-line no-catch-all/no-catch-all diff --git a/packages/app/src/cli/services/app-doctor-engine/scanners/index.ts b/packages/app/src/cli/services/app-doctor-engine/scanners/index.ts index a5c5169ded3..275c4f1cfce 100644 --- a/packages/app/src/cli/services/app-doctor-engine/scanners/index.ts +++ b/packages/app/src/cli/services/app-doctor-engine/scanners/index.ts @@ -10,7 +10,7 @@ import { getSkippedFiles, findManifests, findManifestPaths, - findDependencyAuditingInputs, + findDependencyAutomationInputs, } from './discover.js' import {detectCapabilities, detectProject} from '../capabilities/detect.js' import {calculateScore, computeScanMetadata} from '../scorer/index.js' @@ -29,7 +29,7 @@ import {missingComplianceWebhooks, scanEolApiVersions} from '../rules/compliance import {scanAppProxyLiquidInjection} from '../rules/proxy-rules.js' import {scanExpiringOfflineTokens} from '../rules/token-rules.js' import {scanStaticFrameAncestors} from '../rules/csp-rules.js' -import {scanDependencyAuditing} from '../rules/dependency-auditing-rules.js' +import {scanDependencyAutomation} from '../rules/dependency-automation-rules.js' import {RULE_CATALOG} from '../rules/catalog.js' import {redactIssue} from '../trace/index.js' import {getEngineVersion} from '../version.js' @@ -57,7 +57,7 @@ type CheckTarget = | 'secrets' | 'config_and_source' | 'source_and_theme' - | 'dependency_auditing' + | 'dependency_automation' interface RunnerImplementationResult { id: string analysisMode: AnalysisMode @@ -119,14 +119,14 @@ const jsCheck = ( const DETERMINISTIC_CHECK_DEFINITIONS: ReadonlyArray = [ configRule(missingComplianceWebhooks), { - id: 'MISSING_DEPENDENCY_AUDITING', + id: 'MISSING_DEPENDENCY_SECURITY_AUTOMATION', version: 1, lifecycle: 'active', analysisMode: 'structured_config', - target: 'dependency_auditing', + target: 'dependency_automation', guidance: - 'Resolve unreadable or malformed inputs and inspect unsupported configuration references manually. Repository-level CI configuration is not inspected for nested apps; verify that dependency vulnerability auditing covers this app.', - runner: (context) => scanDependencyAuditing(context), + 'Resolve unreadable configuration files. Repository-level configuration is not inspected for nested apps; verify existing dependency automation manually.', + runner: (context) => scanDependencyAutomation(context), }, { id: 'EOL_API_VERSION', @@ -382,10 +382,10 @@ async function gitProject(appRoot: string): Promise { function selectedFiles(definition: DeterministicCheckDefinition, context: ScanContext): string[] { const configurations = context.appTomls.map((toml) => relativePath(context.appRoot, toml.path).replace(/\\/g, '/')) if (definition.target === 'config') return configurations - if (definition.target === 'dependency_auditing') + if (definition.target === 'dependency_automation') return [ ...context.manifests.map((manifest) => manifest.path), - ...context.dependencyAuditing.files.filter((file) => file.content !== undefined).map((file) => file.path), + ...context.dependencyAutomation.files.filter((file) => file.content !== undefined).map((file) => file.path), ] if (definition.target === 'secrets') return context.sensitiveFiles.filter((file) => file.content !== undefined).map((file) => file.path) @@ -414,7 +414,7 @@ function executionDisposition( const reactRouterSupported = context.detection.framework === 'react_router' const hasTheme = context.capabilities.theme_app_extension - if (definition.target === 'dependency_auditing' && !context.manifests.some(manifestHasDependencies)) + if (definition.target === 'dependency_automation' && !context.manifests.some(manifestHasDependencies)) return { status: 'not_applicable', required: false, @@ -546,8 +546,8 @@ function skippedInputsForCheck( const isSourcePath = (path: string) => !isThemePath(path) && Boolean(definition.extensions?.some((extension) => path.endsWith(extension))) const isConfig = (path: string) => /^shopify\.app(?:\.[^/]+)?\.toml$/.test(path) - const isDependencyAuditingInput = (path: string) => - /(^|\/)package\.json$/.test(path) || /^\.github\/workflows\/[^/]+\.ya?ml$/i.test(path) + const isDependencyAutomationInput = (path: string) => + /(^|\/)package\.json$/.test(path) || context.dependencyAutomation.files.some((file) => file.path === path) const isSecretInput = (file: SkippedFile) => !file.detail?.includes('could not be parsed') && (context.sourceCandidates.some((candidate) => candidate.path === file.path) || @@ -556,7 +556,7 @@ function skippedInputsForCheck( return skippedFiles.filter((file) => { if (definition.target === 'config') return isConfig(file.path) - if (definition.target === 'dependency_auditing') return isDependencyAuditingInput(file.path) + if (definition.target === 'dependency_automation') return isDependencyAutomationInput(file.path) if (definition.target === 'config_and_source') return isConfig(file.path) || isSourcePath(file.path) if (definition.target === 'source' || definition.target === 'app_source') return isSourcePath(file.path) if (definition.target === 'theme') return isThemePath(file.path) @@ -601,8 +601,8 @@ export async function scan(startPath?: string): Promise { const sensitiveFiles = findSensitiveFiles(appRoot) const manifestPaths = findManifestPaths(appRoot) const manifests = findManifests(appRoot, manifestPaths) - const dependencyAuditing = manifests.some(manifestHasDependencies) - ? findDependencyAuditingInputs(appRoot) + const dependencyAutomation = manifests.some(manifestHasDependencies) + ? findDependencyAutomationInputs(appRoot) : {files: []} const requestedAppToml = startPath?.endsWith('.toml') ? (appTomls.find((toml) => basename(toml.path) === basename(startPath)) ?? @@ -625,7 +625,7 @@ export async function scan(startPath?: string): Promise { extensions, sourceFiles, manifests, - dependencyAuditing, + dependencyAutomation, sensitiveFiles, capabilities, detection, @@ -640,7 +640,7 @@ export async function scan(startPath?: string): Promise { let implementations: RunnerImplementationResult[] | undefined const before = issues.length const rejectedInputs = skippedInputsForCheck(definition, context, getSkippedFiles()) - const suppressRunnerForRejectedInput = definition.target === 'dependency_auditing' && rejectedInputs.length > 0 + const suppressRunnerForRejectedInput = definition.target === 'dependency_automation' && rejectedInputs.length > 0 if ( (disposition.status === 'executed' || disposition.status === 'unresolved') && definition.runner && @@ -648,7 +648,7 @@ export async function scan(startPath?: string): Promise { ) { // eslint-disable-next-line no-await-in-loop const output = normalizeRunnerResult(await definition.runner(runnerContext(definition, context))) - if (definition.target !== 'dependency_auditing' || !output.unresolvedReason) issues.push(...output.issues) + if (definition.target !== 'dependency_automation' || !output.unresolvedReason) issues.push(...output.issues) implementations = output.implementations if (output.inspectedFiles) inspectedFiles = output.inspectedFiles if (output.unresolvedReason) @@ -731,7 +731,7 @@ export async function scan(startPath?: string): Promise { ...appTomls.map((toml) => ({absolutePath: toml.path, content: toml.content})), ...extensions.map((extension) => ({absolutePath: joinPath(appRoot, extension.path), content: extension.content})), ...manifests.map((manifest) => ({absolutePath: manifest.absolutePath, content: manifest.content})), - ...dependencyAuditing.files.map((file) => ({absolutePath: joinPath(appRoot, file.path), content: file.content})), + ...dependencyAutomation.files.map((file) => ({absolutePath: joinPath(appRoot, file.path), content: file.content})), ] for (const {absolutePath, content} of configFiles) if (content !== undefined) { diff --git a/packages/app/src/cli/services/app-doctor-engine/scanners/types.ts b/packages/app/src/cli/services/app-doctor-engine/scanners/types.ts index a56337ae054..eb880447844 100644 --- a/packages/app/src/cli/services/app-doctor-engine/scanners/types.ts +++ b/packages/app/src/cli/services/app-doctor-engine/scanners/types.ts @@ -42,8 +42,8 @@ export interface SourceFile { content?: string } -/** Explicitly allowlisted CI configuration inside the app-root evidence boundary. */ -export interface DependencyAuditingInputs { +/** Explicitly allowlisted dependency-management configuration inside the app-root evidence boundary. */ +export interface DependencyAutomationInputs { files: SourceFile[] /** A specific discovery obstacle, including an app nested below its repository root. */ unresolvedReason?: string @@ -58,6 +58,4 @@ export interface ManifestFile { /** Parsed dependencies, keyed by name with version specifications as values. */ dependencies: Record devDependencies?: Record - /** Literal package scripts; only one CI-invoked script reference is followed. */ - scripts?: Record } diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-discovery.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-discovery.test.ts deleted file mode 100644 index eb3a16586c1..00000000000 --- a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-discovery.test.ts +++ /dev/null @@ -1,261 +0,0 @@ -/* eslint-disable no-restricted-imports -- discovery boundaries use real temporary repositories */ -import {findDependencyAuditingInputs, findManifests, getSkippedFiles, resetSkippedFiles} from '../scanners/discover.js' -import {afterEach, describe, expect, test} from 'vitest' -import {execFileSync} from 'node:child_process' -import {createHash} from 'node:crypto' -import {mkdir, mkdtemp, rm, symlink, writeFile} from 'node:fs/promises' -import {tmpdir} from 'node:os' -import {dirname, join} from 'node:path' - -const temporaryDirectories: string[] = [] - -afterEach(async () => { - resetSkippedFiles() - await Promise.all(temporaryDirectories.splice(0).map((directory) => rm(directory, {recursive: true, force: true}))) -}) - -async function makeDirectory(prefix = 'app-doctor-dependency-discovery-'): Promise { - const directory = await mkdtemp(join(tmpdir(), prefix)) - temporaryDirectories.push(directory) - return directory -} - -async function writeFiles(root: string, files: Record): Promise { - await Promise.all( - Object.entries(files).map(async ([path, content]) => { - const fullPath = join(root, path) - await mkdir(dirname(fullPath), {recursive: true}) - await writeFile(fullPath, content) - }), - ) -} - -describe('dependency auditing input discovery', () => { - test('treats an empty allowlist as complete discovery', async () => { - const root = await makeDirectory() - - expect(findDependencyAuditingInputs(root)).toEqual({files: []}) - }) - - test('reads only explicitly allowlisted hidden CI configuration with exact raw content', async () => { - const root = await makeDirectory() - const workflow = 'name: dependency review\r\npermissions: {}\r\n' - await writeFiles(root, { - '.github/workflows/dependencies.yml': workflow, - '.github/workflows/release.yaml': 'name: release\n', - '.github/workflows/ignored.json': '{}', - '.hidden/config.yml': 'not: allowlisted\n', - }) - - const result = findDependencyAuditingInputs(root) - - expect(result.unresolvedReason).toBeUndefined() - expect(result.files.map(({path}) => path)).toEqual([ - '.github/workflows/dependencies.yml', - '.github/workflows/release.yaml', - ]) - const discoveredWorkflow = result.files.find(({path}) => path.endsWith('dependencies.yml')) - expect(discoveredWorkflow?.content).toBe(workflow) - expect( - createHash('sha256') - .update(discoveredWorkflow?.content ?? '') - .digest('hex'), - ).toBe(createHash('sha256').update(workflow).digest('hex')) - }) - - test('preserves bounded reads and skipped-file coverage', async () => { - const root = await makeDirectory() - await writeFiles(root, {'.github/workflows/ci.yml': 'x'.repeat(500_001)}) - resetSkippedFiles() - - const result = findDependencyAuditingInputs(root) - - expect(result).toMatchObject({files: [{path: '.github/workflows/ci.yml', content: undefined}]}) - expect(result.unresolvedReason).toBeUndefined() - expect(getSkippedFiles()).toEqual([ - expect.objectContaining({path: '.github/workflows/ci.yml', reason: 'too_large', size_bytes: 500_001}), - ]) - }) - - test.each(['directory', 'worktree file'])( - 'does not inspect app config nested below a repository %s marker', - async (marker) => { - const repository = await makeDirectory() - const app = join(repository, 'apps', 'example') - await writeFiles(app, {'.github/workflows/audit.yml': 'name: audit\n'}) - if (marker === 'directory') await mkdir(join(repository, '.git')) - else await writeFile(join(repository, '.git'), 'gitdir: /outside/not-read\n') - - const result = findDependencyAuditingInputs(app) - - expect(result.files).toEqual([]) - expect(result.unresolvedReason).toContain('nested below repository root') - }, - ) - - test.each(['directory', 'worktree file'])( - 'uses an app-root repository %s marker without inspecting a parent repository marker', - async (marker) => { - const repository = await makeDirectory() - const app = join(repository, 'apps', 'example') - await writeFiles(app, {'.github/workflows/ci.yml': 'name: audit\n'}) - await mkdir(join(repository, '.git')) - if (marker === 'directory') await mkdir(join(app, '.git')) - else await writeFile(join(app, '.git'), 'gitdir: /outside/not-read\n') - - const result = findDependencyAuditingInputs(app) - - expect(result.files).toMatchObject([{path: '.github/workflows/ci.yml', content: 'name: audit\n'}]) - expect(result.unresolvedReason).toBeUndefined() - }, - ) - - test('rejects an app-root repository marker symlink as ambiguous', async () => { - const root = await makeDirectory() - await mkdir(join(root, 'repository-metadata')) - await symlink(join(root, 'repository-metadata'), join(root, '.git'), 'dir') - - const result = findDependencyAuditingInputs(root) - - expect(result.files).toEqual([]) - expect(result.unresolvedReason).toContain('Could not determine repository ownership') - }) - - test.skipIf(process.platform === 'win32')('rejects an app-root special repository marker as ambiguous', async () => { - const root = await makeDirectory() - execFileSync('mkfifo', [join(root, '.git')]) - - const result = findDependencyAuditingInputs(root) - - expect(result.files).toEqual([]) - expect(result.unresolvedReason).toContain('Could not determine repository ownership') - }) - - test('rejects an allowlisted file symlink that escapes the app root', async () => { - const root = await makeDirectory() - const outside = await makeDirectory('app-doctor-dependency-outside-') - await writeFile(join(outside, 'pipeline.yml'), 'audit: true\n') - await mkdir(join(root, '.github', 'workflows'), {recursive: true}) - await symlink(join(outside, 'pipeline.yml'), join(root, '.github', 'workflows', 'ci.yml')) - - const result = findDependencyAuditingInputs(root) - - expect(result.files).toEqual([]) - expect(result.unresolvedReason).toContain('outside the app root') - }) - - test.each(['.github', '.github/workflows'])( - 'reports an escaping %s directory symlink instead of treating workflows as absent', - async (linkedDirectory) => { - const root = await makeDirectory() - const outside = await makeDirectory('app-doctor-workflows-outside-') - await writeFiles(outside, {'workflows/audit.yml': 'name: audit\n', 'audit.yml': 'name: audit\n'}) - await mkdir(dirname(join(root, linkedDirectory)), {recursive: true}) - await symlink(outside, join(root, linkedDirectory), 'dir') - - const result = findDependencyAuditingInputs(root) - - expect(result.files).toEqual([]) - expect(result.unresolvedReason).toContain('outside the app root') - }, - ) - - test('reports dangling allowlisted directory symlinks', async () => { - const root = await makeDirectory() - await mkdir(join(root, '.github')) - await symlink(join(root, 'missing-workflows'), join(root, '.github', 'workflows'), 'dir') - - const result = findDependencyAuditingInputs(root) - - expect(result.files).toEqual([]) - expect(result.unresolvedReason).toContain('dangling symbolic link') - }) -}) - -describe('manifest safety and validation', () => { - test('rejects explicit parent paths and manifest symlinks outside the app root', async () => { - const root = await makeDirectory() - const outside = await makeDirectory('app-doctor-manifest-outside-') - await writeFile(join(outside, 'package.json'), JSON.stringify({dependencies: {unsafe: '1.0.0'}})) - await symlink(join(outside, 'package.json'), join(root, 'package.json')) - resetSkippedFiles() - - expect(findManifests(root, ['package.json', '../package.json'])).toEqual([]) - expect(getSkippedFiles()).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - path: 'package.json', - reason: 'unreadable', - detail: 'symbolic link escapes the app root', - }), - expect.objectContaining({path: '../package.json', reason: 'unreadable', detail: 'path escapes the app root'}), - ]), - ) - }) - - test.skipIf(process.platform === 'win32')('rejects a manifest FIFO before attempting to read it', async () => { - const root = await makeDirectory() - execFileSync('mkfifo', [join(root, 'package.json')]) - resetSkippedFiles() - - expect(findManifests(root, ['package.json'])).toEqual([]) - expect(getSkippedFiles()).toEqual([ - expect.objectContaining({path: 'package.json', reason: 'unreadable', detail: 'path is not a regular file'}), - ]) - }) - - test('keeps manifests without dependencies as valid inputs', async () => { - const root = await makeDirectory() - await writeFile( - join(root, 'package.json'), - JSON.stringify({name: 'dependency-free-app', scripts: {start: 'shopify app dev'}}), - ) - - expect(findManifests(root)).toMatchObject([ - { - dependencies: {}, - devDependencies: {}, - scripts: {start: 'shopify app dev'}, - }, - ]) - expect(getSkippedFiles()).toEqual([]) - }) - - test('stores string-valued scripts and dependency records', async () => { - const root = await makeDirectory() - await writeFiles(root, { - 'package.json': JSON.stringify({ - dependencies: {production: '^1.0.0'}, - devDependencies: {development: '^2.0.0'}, - scripts: {audit: 'npm audit'}, - }), - }) - - expect(findManifests(root)).toMatchObject([ - { - dependencies: {production: '^1.0.0'}, - devDependencies: {development: '^2.0.0'}, - scripts: {audit: 'npm audit'}, - }, - ]) - }) - - test.each([ - ['null package', 'null'], - ['array package', '[]'], - ['non-string dependency', JSON.stringify({dependencies: {unsafe: 1}})], - ['non-string development dependency', JSON.stringify({devDependencies: {unsafe: null}})], - ['non-string script', JSON.stringify({scripts: {audit: ['npm audit']}})], - ])('keeps a malformed %s as skipped input coverage', async (_description, content) => { - const root = await makeDirectory() - await writeFile(join(root, 'package.json'), content) - resetSkippedFiles() - - const manifests = findManifests(root) - - expect(manifests).toMatchObject([{dependencies: {}, devDependencies: {}}]) - expect(getSkippedFiles()).toEqual([ - expect.objectContaining({path: 'package.json', reason: 'unreadable', detail: 'manifest could not be parsed'}), - ]) - }) -}) diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-rules.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-rules.test.ts deleted file mode 100644 index 6e67588aa17..00000000000 --- a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing-rules.test.ts +++ /dev/null @@ -1,465 +0,0 @@ -import {scanDependencyAuditing} from '../rules/dependency-auditing-rules.js' -import {describe, expect, test} from 'vitest' -import type {ManifestFile, SourceFile} from '../rules/types.js' - -const manifest = (path = 'package.json', scripts?: Record): ManifestFile => ({ - path, - absolutePath: `/${path}`, - type: 'npm', - dependencies: {react: '19.0.0'}, - scripts, -}) - -const configuration = (path: string, content: string): SourceFile => ({ - path, - absolutePath: `/${path}`, - ext: path.endsWith('.yaml') ? '.yaml' : '.yml', - content, -}) - -const github = (body: string): SourceFile => - configuration( - '.github/workflows/ci.yml', - `on: push -jobs: - check: - runs-on: ubuntu-latest -${body}`, - ) - -function scan(files: SourceFile[], manifests: ManifestFile[] = [manifest()]) { - return scanDependencyAuditing({manifests, dependencyAuditing: {files}}) -} - -function expectRecognized(files: SourceFile[], manifests?: ManifestFile[]) { - expect(scan(files, manifests)).toEqual({issues: []}) -} - -function expectMissing(files: SourceFile[], manifests?: ManifestFile[]) { - const result = scan(files, manifests) - expect(result.unresolvedReason).toBeUndefined() - expect(result.issues).toHaveLength(1) - expect(result.issues[0]).toMatchObject({ - id: 'MISSING_DEPENDENCY_AUDITING', - severity: 'low', - points: -5, - title: 'Dependency auditing configuration not detected', - location: {file: manifests?.[0]?.path ?? 'package.json'}, - fix: {automated: false}, - }) -} - -function expectUnresolved(files: SourceFile[], manifests?: ManifestFile[]) { - const result = scan(files, manifests) - expect(result.issues).toEqual([]) - expect(result.unresolvedReason).toBeTruthy() -} - -describe('dependency-auditing applicability and finding', () => { - test('is not applicable without a manifest that declares dependencies', () => { - const emptyManifest = {...manifest(), dependencies: {}} - expect(scan([], [emptyManifest])).toEqual({issues: []}) - }) - - test('emits exactly one finding anchored to an actual dependency manifest', () => { - const manifests = [manifest('packages/web/package.json'), manifest('package.json')] - const result = scan([], manifests) - - expect(result.issues).toHaveLength(1) - expect(result.issues[0]).toEqual({ - id: 'MISSING_DEPENDENCY_AUDITING', - severity: 'low', - points: -5, - title: 'Dependency auditing configuration not detected', - message: - 'No recognized dependency-auditing configuration was found in the inspected files. Add an automated dependency vulnerability check, or verify that your existing integration covers this app. Hosted integrations and repository settings were not inspected.', - location: {file: 'package.json'}, - fix: {automated: false, description: 'Add an automated dependency vulnerability check for this package.'}, - }) - }) - - test('preserves a scanner discovery obstacle without emitting a finding', () => { - expect( - scanDependencyAuditing({ - manifests: [manifest()], - dependencyAuditing: {files: [], unresolvedReason: 'The app is nested below the repository root.'}, - }), - ).toEqual({issues: [], unresolvedReason: 'The app is nested below the repository root.'}) - }) - - test.each(['', '# comments only\n', 'null\n'])('treats empty YAML as no evidence', (content) => { - expectMissing([configuration('.github/workflows/empty.yml', content)]) - }) -}) - -describe('GitHub Actions recognition', () => { - test.each([ - ['dependency review action', ' - uses: actions/dependency-review-action@v4'], - [ - 'OSV reusable workflow', - undefined, - `on: pull_request -jobs: - osv: - uses: google/osv-scanner/.github/workflows/osv-scanner-reusable-pr.yml@v2.0.1`, - ], - ['Snyk Node action', ' - uses: snyk/actions/node@master'], - ['npm audit', ' - run: npm audit --audit-level=high'], - ['pnpm audit', ' - run: pnpm audit'], - ['Yarn classic audit', ' - run: yarn audit'], - ['Yarn modern audit', ' - run: yarn npm audit'], - ['Snyk test', ' - run: npx snyk test --severity-threshold=high'], - ['Semgrep supply chain', ' - run: semgrep ci --supply-chain'], - ['continue-on-error audit', ' - run: npm audit\n continue-on-error: true'], - ])('recognizes %s', (_name, step, completeWorkflow?: string) => { - const file = completeWorkflow - ? configuration('.github/workflows/security.yml', completeWorkflow) - : github(` steps:\n${step}`) - expectRecognized([file]) - }) - - test.each([ - ['disabled job', github(` if: ${['$', '{{ false }}'].join('')}\n steps:\n - run: npm audit`)], - ['disabled step', github(' steps:\n - if: false\n run: npm audit')], - [ - 'disabled dependency review vulnerability check', - github( - ' steps:\n - uses: actions/dependency-review-action@v4\n with:\n vulnerability-check: false', - ), - ], - ['Snyk setup action', github(' steps:\n - uses: snyk/actions/setup@master')], - ['Snyk code action', github(' steps:\n - uses: snyk/actions/code@master')], - ['Snyk container action', github(' steps:\n - uses: snyk/actions/docker@master')], - ['scanner installation', github(' steps:\n - run: npm install --global snyk')], - ['step name', github(' steps:\n - name: npm audit\n run: npm test')], - ['comment', github(' steps:\n - run: |\n # npm audit\n npm test')], - ['echo output', github(' steps:\n - run: echo "npm audit"')], - ['printf output', github(" steps:\n - run: printf 'snyk test\\n'")], - ['Dependabot update action', github(' steps:\n - uses: dependabot/fetch-metadata@v2')], - ['CodeQL action', github(' steps:\n - uses: github/codeql-action/analyze@v3')], - ['workflow metadata only', configuration('.github/workflows/ci.yml', 'name: npm audit')], - ])('does not recognize %s', (_name, file) => expectMissing([file])) - - test('matches a straightforward working-directory to the dependency manifest', () => { - const file = github( - ' defaults:\n run:\n working-directory: packages/web\n steps:\n - run: npm audit', - ) - expectRecognized([file], [manifest('packages/web/package.json')]) - expectMissing([file], [manifest('packages/admin/package.json')]) - }) - - test('matches explicit scanner files and excludes unrelated paths and ecosystems', () => { - const scopedSnyk = github( - ' steps:\n - uses: snyk/actions/node@master\n with:\n args: --file=packages/web/package.json --package-manager=npm', - ) - expectRecognized([scopedSnyk], [manifest('packages/web/package.json')]) - expectMissing([scopedSnyk], [manifest('packages/admin/package.json')]) - - const unrelated = github( - ' steps:\n - uses: snyk/actions/node@master\n with:\n args: --file=requirements.txt --package-manager=poetry', - ) - expectMissing([unrelated]) - }) - - test.each(['install', 'monitor', 'auth', 'help'])('does not recognize Snyk action command %s', (command) => { - expectMissing([ - github(` steps:\n - uses: snyk/actions/node@master\n with:\n command: ${command}`), - ]) - }) - - test('recognizes only the Snyk Node action default or exact test command', () => { - expectRecognized([github(' steps:\n - uses: snyk/actions/node@master')]) - expectRecognized([ - github(' steps:\n - uses: snyk/actions/node@master\n with:\n command: test'), - ]) - expectMissing([ - github( - ' steps:\n - uses: snyk/actions/node@master\n with:\n command: test --all-projects', - ), - ]) - }) - - test.each([ - 'npm audit --help', - 'pnpm audit --version', - 'yarn npm audit --help', - 'snyk test --help', - 'snyk test --package-manager=pip', - 'snyk test --file=requirements.txt', - ])('does not treat non-scanning command %s as an audit', (command) => { - expectMissing([github(` steps:\n - run: ${command}`)]) - }) - - test('runs actions at checkout root rather than defaults.run working-directory', () => { - const file = github( - ' defaults:\n run:\n working-directory: packages/web\n steps:\n - uses: snyk/actions/node@master', - ) - expectMissing([file], [manifest('packages/web/package.json')]) - expectRecognized([file], [manifest('package.json')]) - }) - - test('does not trust scoped OSV workflow inputs or dependency-review config files', () => { - expectUnresolved([ - configuration( - '.github/workflows/security.yml', - 'on: pull_request\njobs:\n osv:\n uses: google/osv-scanner/.github/workflows/osv-scanner-reusable-pr.yml@v2.0.1\n with:\n scan-args: --lockfile=other/package-lock.json', - ), - ]) - expectUnresolved([ - github( - ' steps:\n - uses: actions/dependency-review-action@v4\n with:\n config-file: .github/dependency-review.yml', - ), - ]) - }) - - const dynamicWorkingDirectory = ['$', '{{ github.workspace }}'].join('') - test.each([ - ['workflow', `defaults:\n run:\n working-directory: ${dynamicWorkingDirectory}\n`], - ['job', ''], - ])('leaves dynamic %s defaults.run working-directory unresolved', (_scope, workflowDefaults) => { - const jobDefaults = workflowDefaults - ? '' - : ` defaults:\n run:\n working-directory: ${dynamicWorkingDirectory}\n` - const file = configuration( - '.github/workflows/ci.yml', - `on: push\n${workflowDefaults}jobs:\n check:\n runs-on: ubuntu-latest\n${jobDefaults} steps:\n - run: npm audit`, - ) - expectUnresolved([file]) - }) - - test.each([ - ' if: false\n steps: invalid', - ' steps:\n - if: false\n run: [npm, audit]\n uses: 123\n with: invalid', - ' defaults: invalid\n steps:\n - uses: actions/setup-node@v4', - ' steps:\n - uses: snyk/actions/node@master\n with:\n command: monitor\n args: [invalid]', - ])('does not validate configuration that is not executed: %s', (body) => { - expectMissing([github(body)]) - }) - - test.each([ - ' defaults: invalid\n steps:\n - run: npm audit\n working-directory: .', - ' defaults:\n run:\n working-directory: [invalid]\n shell: [invalid]\n steps:\n - run: npm audit\n working-directory: .\n shell: bash', - ' defaults: invalid\n steps:\n - uses: actions/dependency-review-action@v4', - ' steps:\n - uses: actions/dependency-review-action@v4\n with:\n fail-on-severity: high', - ' steps:\n - run: npm audit\n - run: [invalid]', - ])('retains independent evidence and overridden defaults: %s', (body) => { - expectRecognized([github(body)]) - }) - - test.each(['workflow_call', '[workflow_call]', '{workflow_call: {inputs: {}}}'])( - 'does not validate jobs in a reusable-only definition: %s', - (trigger) => { - expectMissing([configuration('.github/workflows/reusable.yml', `on: ${trigger}\njobs: invalid`)]) - }, - ) - - test('keeps other workflow triggers when checking reusable-only definitions', () => { - expectRecognized([ - configuration( - '.github/workflows/security.yml', - 'on: {workflow_call: {}, push: {}}\njobs:\n audit:\n steps:\n - run: npm audit', - ), - ]) - }) - - test.each(['null', '[]', '{unexpected-input: true}'])('does not discard unsupported OSV inputs: %s', (inputs) => { - expectUnresolved([ - configuration( - '.github/workflows/security.yml', - `on: push\njobs:\n audit:\n uses: google/osv-scanner/.github/workflows/osv-scanner-reusable.yml@v2.0.1\n with: ${inputs}`, - ), - ]) - }) - - test('treats malformed jobs, steps, and run values as unresolved', () => { - expectUnresolved([configuration('.github/workflows/ci.yml', 'on: push\njobs:\n check: invalid')]) - expectUnresolved([github(' steps: invalid')]) - expectUnresolved([github(' steps:\n - run: [npm, audit]')]) - }) -}) - -describe('package scripts and bounded shell handling', () => { - test('follows one literal CI-invoked package script', () => { - expectRecognized( - [github(' steps:\n - run: npm run security')], - [manifest('package.json', {security: 'snyk test'})], - ) - expectRecognized( - [github(' steps:\n - run: npm --prefix packages/web run security')], - [manifest('packages/web/package.json', {security: 'npm audit'})], - ) - }) - - test('lets explicitly recursive supply-chain scans cover nested manifests', () => { - expectRecognized( - [github(' steps:\n - run: semgrep ci --supply-chain')], - [manifest('packages/web/package.json')], - ) - expectRecognized( - [github(' steps:\n - run: snyk test --all-projects')], - [manifest('packages/web/package.json')], - ) - }) - - test('does not inspect unused package scripts', () => { - expectMissing( - [github(' steps:\n - run: npm test')], - [manifest('package.json', {test: 'vitest', security: 'npm audit'})], - ) - }) - - test('does not mistake a package script named audit for an executed scanner', () => { - expectMissing( - [github(' steps:\n - run: npm run audit')], - [manifest('package.json', {audit: 'echo no scan'})], - ) - }) - - test.each([ - ['nested package script', 'npm run security', {security: 'npm run actual', actual: 'npm audit'}], - ['local shell script', 'bash scripts/security.sh', undefined], - ['local executable', './scripts/security.sh', undefined], - ['shell variable', '$SECURITY_COMMAND', undefined], - ['shell control flow', 'if npm audit; then echo ok; fi', undefined], - ['shell interpreter command', 'sh -c "npm audit"', undefined], - ])('leaves %s unresolved', (_name, command, scripts) => { - const result = scan([github(` steps:\n - run: ${command}`)], [manifest('package.json', scripts)]) - expect(result.issues).toEqual([]) - expect(result.unresolvedReason).toBeTruthy() - }) - - test('allows recognized evidence despite an unrelated unsupported script', () => { - expectRecognized([github(' steps:\n - run: ./scripts/build.sh\n - run: pnpm audit')]) - }) - - test.each([ - 'if false; then npm audit; fi', - 'echo start; if false; then echo skipped; fi; npm audit', - 'cat < { - expectUnresolved([github(` steps:\n - run: |\n ${command.replaceAll('\n', '\n ')}`)]) - }) - - test('still accepts a distinct supported step after an unsupported execution block', () => { - expectRecognized([github(' steps:\n - run: if false; then npm audit; fi\n - run: npm audit')]) - }) - - test('handles escaped-newline command continuation and ignores quoted separators', () => { - expectRecognized([github(' steps:\n - run: |\n npm \\\n audit')]) - expectMissing([github(' steps:\n - run: echo "ignored; npm audit"')]) - expectMissing([github(" steps:\n - run: printf 'ignored\\n| snyk test'")]) - }) - - test.each(['cd ../; npm audit', 'cd /tmp; npm audit', 'cd $DIR; npm audit'])( - 'leaves unsafe directory change %s unresolved', - (command) => expectUnresolved([github(` steps:\n - run: ${command}`)]), - ) - - test.each([ - 'npm --prefix ../ audit', - 'pnpm --dir /tmp audit', - 'yarn --cwd $DIR npm audit', - 'npm --workspace other audit', - 'pnpm --filter unrelated audit', - 'npm --silent audit', - 'npm --prefix= audit', - 'snyk test --file=', - 'npm run $SCRIPT', - 'npm run missing', - ])('leaves unsupported target or package-script invocation unresolved: %s', (command) => { - expectUnresolved([github(` steps:\n - run: ${command}`)]) - }) - - test.each(['npm --prefix packages/web audit', 'pnpm --dir packages/web audit', 'yarn --cwd packages/web npm audit'])( - 'supports a bounded literal package directory: %s', - (command) => { - expectRecognized([github(` steps:\n - run: ${command}`)], [manifest('packages/web/package.json')]) - }, - ) - - test('leaves package-script arguments unresolved rather than ignoring them', () => { - expectUnresolved( - [github(' steps:\n - run: npm run security -- --help')], - [manifest('package.json', {security: 'snyk test'})], - ) - }) -}) - -describe('execution boundaries', () => { - test.each(['repository: other/project', 'path: other'])('leaves a scoped checkout unresolved (%s)', (input) => { - const file = github( - ` steps:\n - uses: actions/checkout@v4\n with:\n ${input}\n - run: npm audit`, - ) - expect(scan([file])).toMatchObject({issues: [], unresolvedReason: expect.any(String)}) - }) - - test('does not recognize an uninvoked reusable-only workflow', () => { - const file = configuration( - '.github/workflows/reusable.yml', - 'on: workflow_call\njobs:\n audit:\n steps:\n - run: npm audit', - ) - expectMissing([file]) - }) - - test.each([ - ['quoted heredoc', "cat <<'EOF'\nnpm audit\nEOF"], - ['function definition', 'audit() {\n npm audit\n}'], - ])('does not count commands inside %s', (_name, command) => { - const file = github( - ` steps:\n - run: |\n${command - .split('\n') - .map((line) => ` ${line}`) - .join('\n')}`, - ) - expect(scan([file])).toMatchObject({issues: [], unresolvedReason: expect.any(String)}) - }) - - test('does not count an audit printed through a custom shell', () => { - expect(scan([github(' steps:\n - run: npm audit\n shell: echo {0}')])).toMatchObject({ - issues: [], - unresolvedReason: expect.any(String), - }) - }) - - test.each(['pnpm install', 'yarn install', 'pnpm add snyk'])( - 'treats %s as setup, not a script reference', - (command) => { - expectMissing([github(` steps:\n - run: ${command}`)]) - }, - ) -}) - -describe('configuration obstacles', () => { - test.each([ - ['malformed GitHub workflow', configuration('.github/workflows/ci.yml', 'jobs: [')], - [ - 'excessive YAML aliases', - github( - ` x: &values [one, two]\n y: [${Array.from({length: 21}, () => '*values').join(', ')}]\n steps:\n - run: npm test`, - ), - ], - [ - 'unknown GitHub reusable workflow', - configuration( - '.github/workflows/ci.yml', - 'on: push\njobs:\n delegated:\n uses: acme/ci/.github/workflows/security.yml@v1', - ), - ], - ['unknown local GitHub action', github(' steps:\n - uses: ./.github/actions/security')], - ])('returns no finding and an unresolved reason for %s', (_name, file) => { - const result = scan([file]) - expect(result.issues).toEqual([]) - expect(result.unresolvedReason).toBeTruthy() - }) - - test('malformed relevant YAML remains unresolved even when another file has evidence', () => { - const result = scan([ - github(' steps:\n - run: npm audit'), - configuration('.github/workflows/malformed.yml', 'jobs: ['), - ]) - expect(result.issues).toEqual([]) - expect(result.unresolvedReason).toContain('malformed') - }) -}) diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing.test.ts deleted file mode 100644 index eb9ba8b882d..00000000000 --- a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-auditing.test.ts +++ /dev/null @@ -1,344 +0,0 @@ -/* eslint-disable no-restricted-imports -- integration coverage uses real temporary repositories */ -import {doctorExitCode} from '../../app-doctor-api.js' -import {formatJson} from '../output/format.js' -import {getRegistry} from '../registry/index.js' -import {RULE_CATALOG} from '../rules/catalog.js' -import {DETERMINISTIC_CHECKS, scan} from '../scanners/index.js' -import {buildSubmission} from '../submission/index.js' -import {compileTrace, sha256, validateTrace} from '../trace/index.js' -import {scanApp} from '../run.js' -import {fetch} from '@shopify/cli-kit/node/http' -import {captureOutputWithExitCode} from '@shopify/cli-kit/node/system' -import {afterEach, describe, expect, test, vi} from 'vitest' -import {mkdir, mkdtemp, rm, unlink, writeFile} from 'node:fs/promises' -import {tmpdir} from 'node:os' -import {dirname, join} from 'node:path' - -vi.mock('@shopify/cli-kit/node/http', async (importActual) => { - const actual: any = await importActual() - return {...actual, fetch: vi.fn(actual.fetch)} -}) - -vi.mock('@shopify/cli-kit/node/system', async (importActual) => { - const actual: any = await importActual() - return {...actual, captureOutputWithExitCode: vi.fn(actual.captureOutputWithExitCode)} -}) - -const temporaryDirectories: string[] = [] -const appConfiguration = `name = "Dependency auditing integration" -[webhooks] -api_version = "unstable" -[[webhooks.subscriptions]] -compliance_topics = ["shop/redact", "customers/data_request", "customers/redact"] -uri = "https://example.test/webhooks" -` - -afterEach(async () => { - await Promise.all(temporaryDirectories.splice(0).map((directory) => rm(directory, {recursive: true, force: true}))) -}) - -async function makeDirectory(prefix = 'app-doctor-dependency-auditing-'): Promise { - const directory = await mkdtemp(join(tmpdir(), prefix)) - temporaryDirectories.push(directory) - return directory -} - -async function writeFiles(root: string, files: Record): Promise { - await Promise.all( - Object.entries(files).map(async ([path, content]) => { - const absolutePath = join(root, path) - await mkdir(dirname(absolutePath), {recursive: true}) - await writeFile(absolutePath, content) - }), - ) -} - -async function makeApp(files: Record): Promise { - const directory = await makeDirectory() - await writeFiles(directory, {'shopify.app.toml': appConfiguration, ...files}) - return directory -} - -function dependencyExecution(result: Awaited>) { - return result.scan.checks_executed.find((execution) => execution.id === 'MISSING_DEPENDENCY_AUDITING')! -} - -function dependencyFindings(result: Awaited>) { - return result.issues.filter((issue) => issue.id === 'MISSING_DEPENDENCY_AUDITING') -} - -describe('dependency-auditing scanner integration', () => { - test('registers one active, framework-independent structured-config implementation', () => { - const definition = DETERMINISTIC_CHECKS.get('MISSING_DEPENDENCY_AUDITING') - expect(definition).toMatchObject({ - version: 1, - lifecycle: 'active', - analysisMode: 'structured_config', - target: 'dependency_auditing', - }) - expect(definition).not.toHaveProperty('requires') - const catalogEntry = RULE_CATALOG.find((entry) => entry.id === 'MISSING_DEPENDENCY_AUDITING') - expect(catalogEntry).toMatchObject({ - title: 'Dependency auditing configuration not detected', - severity: 'low', - points: -5, - }) - expect(catalogEntry).not.toHaveProperty('status') - expect(catalogEntry).not.toHaveProperty('requires') - const registryEntries = getRegistry().filter((entry) => entry.id === 'MISSING_DEPENDENCY_AUDITING') - expect(registryEntries).toEqual([ - expect.objectContaining({ - kind: 'deterministic', - version: 1, - status: 'active', - severity: 'low', - points: -5, - title: 'Dependency auditing configuration not detected', - }), - ]) - }) - - test('is not applicable when no supported manifest declares dependencies', async () => { - const directory = await makeApp({ - 'package.json': JSON.stringify({name: 'empty'}), - '.github/workflows/irrelevant.yml': 'x'.repeat(500_001), - }) - - const result = await scan(directory) - - expect(dependencyFindings(result)).toEqual([]) - expect(dependencyExecution(result)).toMatchObject({ - status: 'not_applicable', - required: false, - applicable: false, - inspected_files: ['package.json'], - findings: 0, - analysis_mode: 'structured_config', - reason: {code: 'no_relevant_files'}, - }) - }) - - test('emits exactly one ordinary low finding and honors low blocking', async () => { - const directory = await makeApp({'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}})}) - - const execution = await scanApp(directory) - const result = execution.scan - - expect(dependencyFindings(result)).toEqual([ - expect.objectContaining({ - id: 'MISSING_DEPENDENCY_AUDITING', - severity: 'low', - points: -5, - title: 'Dependency auditing configuration not detected', - location: {file: 'package.json'}, - found_by: 'static', - rule_version: 1, - }), - ]) - expect(dependencyExecution(result)).toMatchObject({ - status: 'executed', - required: true, - applicable: true, - inspected_files: ['package.json'], - findings: 1, - analysis_mode: 'structured_config', - }) - expect(result.score).toEqual({total: 95, baseline: 100, grade: 'EXCELLENT'}) - expect(formatJson(result)).toContain('MISSING_DEPENDENCY_AUDITING') - expect(execution.trace.findings).toContainEqual( - expect.objectContaining({ - source: 'deterministic', - rule_id: 'MISSING_DEPENDENCY_AUDITING', - rule_version: 1, - severity: 'low', - }), - ) - const submission = buildSubmission(execution.trace, { - cliVersion: '3.99.0', - submittedAt: '2026-09-15T00:01:00.000Z', - }) - expect(submission.report.findings).toContainEqual( - expect.objectContaining({ - source: 'deterministic', - rule_id: 'MISSING_DEPENDENCY_AUDITING', - rule_version: 1, - severity: 'low', - }), - ) - expect(doctorExitCode({...execution, elapsedMilliseconds: 0}, 'low')).toBe(1) - expect(doctorExitCode({...execution, elapsedMilliseconds: 0}, 'medium')).toBe(0) - expect(doctorExitCode({...execution, elapsedMilliseconds: 0}, 'none')).toBe(0) - }) - - test('treats an empty allowlisted CI file as negative evidence, not an unresolved parse', async () => { - const directory = await makeApp({ - 'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), - '.github/workflows/dependencies.yml': '', - }) - - const result = await scan(directory) - - expect(dependencyFindings(result)).toHaveLength(1) - expect(dependencyExecution(result)).toMatchObject({ - status: 'executed', - inspected_files: ['package.json', '.github/workflows/dependencies.yml'], - findings: 1, - }) - }) - - test('passes when allowlisted CI runs auditing without executing it or making network requests', async () => { - const directory = await makeApp({ - 'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), - '.github/workflows/dependencies.yml': `on: pull_request -jobs: - audit: - runs-on: ubuntu-latest - steps: - - run: npm audit -`, - }) - - const result = await scan(directory) - - expect(dependencyFindings(result)).toEqual([]) - expect(dependencyExecution(result)).toMatchObject({ - status: 'executed', - inspected_files: ['package.json', '.github/workflows/dependencies.yml'], - findings: 0, - }) - expect( - vi.mocked(captureOutputWithExitCode).mock.calls.map(([command, arguments_]) => [command, arguments_]), - ).toEqual([ - ['git', ['rev-parse', 'HEAD']], - ['git', ['status', '--porcelain']], - ]) - expect(fetch).not.toHaveBeenCalled() - }) - - test.each([ - ['malformed', '.github/workflows/dependencies.yml', 'jobs: ['], - ['unreadable', '.github/workflows/oversized.yml', 'x'.repeat(500_001)], - ])('leaves %s configuration unresolved without a missing finding', async (_kind, path, content) => { - const directory = await makeApp({ - 'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), - [path]: content, - }) - - const result = await scan(directory) - - expect(dependencyFindings(result)).toEqual([]) - expect(dependencyExecution(result)).toMatchObject({ - status: 'unresolved', - required: true, - applicable: true, - inspected_files: _kind === 'unreadable' ? ['package.json'] : ['package.json', path], - findings: 0, - reason: {code: expect.stringMatching(/^(parser_unavailable|input_rejected)$/)}, - }) - expect(result.score).toBeNull() - expect(result.scan.coverage_gaps).toContainEqual( - expect.objectContaining({code: 'unresolved_check', check_id: 'MISSING_DEPENDENCY_AUDITING'}), - ) - }) - - test('suppresses the missing finding when any relevant manifest cannot be parsed', async () => { - const directory = await makeApp({ - 'package.json': '{invalid', - 'packages/web/package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), - }) - - const result = await scan(directory) - - expect(dependencyFindings(result)).toEqual([]) - expect(dependencyExecution(result)).toMatchObject({ - status: 'unresolved', - findings: 0, - inspected_files: ['package.json', 'packages/web/package.json'], - reason: {code: 'parser_unavailable'}, - }) - }) - - test('leaves an app below its repository root unresolved without a missing finding', async () => { - const repository = await makeDirectory('app-doctor-dependency-repository-') - const directory = join(repository, 'apps', 'example') - await mkdir(join(repository, '.git')) - await writeFiles(directory, { - 'shopify.app.toml': appConfiguration, - 'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), - }) - - const result = await scan(directory) - - expect(dependencyFindings(result)).toEqual([]) - expect(dependencyExecution(result)).toMatchObject({ - status: 'unresolved', - inspected_files: ['package.json'], - findings: 0, - reason: {code: 'parser_unavailable', message: expect.stringContaining('nested below repository root')}, - }) - }) - - test('hashes exact configuration bytes and tracks configuration changes, additions, and deletion', async () => { - const directory = await makeApp({'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}})}) - const configurationPath = join(directory, '.github', 'workflows', 'dependencies.yml') - const initial = await scan(directory) - const firstContent = 'jobs:\n audit:\n steps:\n - run: npm audit\n' - await writeFiles(directory, {'.github/workflows/dependencies.yml': firstContent}) - const added = await scan(directory) - const secondContent = `${firstContent}# changed without normalization\r\n` - await writeFile(configurationPath, secondContent) - const changed = await scan(directory) - await unlink(configurationPath) - const deleted = await scan(directory) - - expect(initial.scan.file_hashes).not.toHaveProperty('.github/workflows/dependencies.yml') - expect(added.scan.file_hashes?.['.github/workflows/dependencies.yml']).toBe(sha256(firstContent)) - expect(changed.scan.file_hashes?.['.github/workflows/dependencies.yml']).toBe(sha256(secondContent)) - expect(deleted.scan.file_hashes).not.toHaveProperty('.github/workflows/dependencies.yml') - expect(new Set([initial.scan.input_hash, added.scan.input_hash, changed.scan.input_hash])).toHaveProperty('size', 3) - expect(deleted.scan.input_hash).toBe(initial.scan.input_hash) - expect(dependencyExecution(changed).inspected_files).toEqual(['package.json', '.github/workflows/dependencies.yml']) - }) - - test('routes only paths, hashes, and static metadata through output, trace, and submission', async () => { - const privateConfigurationMarker = 'DO_NOT_SUBMIT_DEPENDENCY_AUDIT_CONFIGURATION_CONTENT' - const privateConfigurationContent = `# ${privateConfigurationMarker} -jobs: - audit: - steps: - - run: npm audit -` - const directory = await makeApp({ - 'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), - '.github/workflows/dependencies.yml': privateConfigurationContent, - }) - const result = await scan(directory) - const trace = compileTrace(result, {generatedAt: '2026-09-15T00:00:00.000Z'}) - const submission = buildSubmission(trace, { - cliVersion: '3.99.0', - submittedAt: '2026-09-15T00:01:00.000Z', - }) - const serializedOutputs = [formatJson(result), JSON.stringify(trace), JSON.stringify(submission)] - - expect(validateTrace(trace)).toEqual({valid: true, errors: []}) - expect(trace.project.input_hashes['.github/workflows/dependencies.yml']).toBe(sha256(privateConfigurationContent)) - expect(trace.checks_executed).toContainEqual( - expect.objectContaining({ - id: 'MISSING_DEPENDENCY_AUDITING', - analysis_mode: 'structured_config', - status: 'executed', - inspected_files: ['package.json', '.github/workflows/dependencies.yml'], - }), - ) - expect(submission.report.checks_executed).toContainEqual( - expect.objectContaining({ - id: 'MISSING_DEPENDENCY_AUDITING', - analysis_mode: 'structured_config', - status: 'executed', - inspected_file_count: 2, - finding_count: 0, - }), - ) - for (const output of serializedOutputs) expect(output).not.toContain(privateConfigurationMarker) - }) -}) diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-discovery.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-discovery.test.ts new file mode 100644 index 00000000000..0f789ac36f0 --- /dev/null +++ b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-discovery.test.ts @@ -0,0 +1,168 @@ +/* eslint-disable no-restricted-imports -- discovery boundaries use real temporary repositories */ +import { + findDependencyAutomationInputs, + findManifests, + getSkippedFiles, + resetSkippedFiles, +} from '../scanners/discover.js' +import {DEPENDENCY_AUTOMATION_CONFIG_PATHS} from '../rules/dependency-automation-rules.js' +import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs' +import {afterEach, describe, expect, test} from 'vitest' +import {execFileSync} from 'node:child_process' +import {mkdir, symlink, writeFile} from 'node:fs/promises' +import {dirname, join} from 'node:path' + +afterEach(() => resetSkippedFiles()) + +async function writeFiles(root: string, files: Record): Promise { + await Promise.all( + Object.entries(files).map(async ([path, content]) => { + await mkdir(dirname(join(root, path)), {recursive: true}) + await writeFile(join(root, path), content) + }), + ) +} + +describe('dependency automation discovery', () => { + test('reads only allowlisted config, preserving exact bytes and ignoring workflows', async () => { + await inTemporaryDirectory(async (root) => { + expect(findDependencyAutomationInputs(root)).toEqual({files: []}) + const content = 'version: 2\r\nupdates: []\r\n' + await writeFiles(root, { + '.github/dependabot.yml': content, + '.gitlab/renovate.json': '{}', + '.github/workflows/audit.yml': 'jobs: [', + '.gitlab-ci.yml': 'include: [', + '.circleci/config.yml': 'jobs: [', + '.snyk': 'version: v1.25.0', + }) + const result = findDependencyAutomationInputs(root) + expect(result.unresolvedReason).toBeUndefined() + expect(result.files.map(({path}) => path)).toEqual(['.github/dependabot.yml']) + expect(result.files[0]?.content).toBe(content) + }) + }) + + test.each(DEPENDENCY_AUTOMATION_CONFIG_PATHS)('discovers the allowlisted file %s', async (path) => { + await inTemporaryDirectory(async (root) => { + await writeFiles(root, {[path]: '{}'}) + const result = findDependencyAutomationInputs(root) + expect(result.files).toMatchObject([{path, content: '{}'}]) + expect(result.unresolvedReason).toBeUndefined() + }) + }) + + test('stops discovery after finding one configuration file', async () => { + await inTemporaryDirectory(async (root) => { + await writeFiles(root, {'renovate.json': '{}', '.renovaterc': 'x'.repeat(500_001)}) + expect(findDependencyAutomationInputs(root).files).toMatchObject([{path: 'renovate.json'}]) + expect(getSkippedFiles()).toEqual([]) + }) + }) + + test('preserves bounded reads and skipped-file coverage', async () => { + await inTemporaryDirectory(async (root) => { + await writeFiles(root, {'.github/dependabot.yml': 'x'.repeat(500_001)}) + expect(findDependencyAutomationInputs(root)).toMatchObject({ + files: [{path: '.github/dependabot.yml', content: undefined}], + }) + expect(getSkippedFiles()).toEqual([ + expect.objectContaining({path: '.github/dependabot.yml', reason: 'too_large', size_bytes: 500_001}), + ]) + }) + }) + + test.each(['directory', 'worktree file'])('respects repository %s boundaries', async (marker) => { + await inTemporaryDirectory(async (repository) => { + 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') + expect(findDependencyAutomationInputs(app)).toMatchObject({ + files: [], + unresolvedReason: expect.stringContaining('nested below repository root'), + }) + if (marker === 'directory') await mkdir(join(app, '.git')) + else await writeFile(join(app, '.git'), 'gitdir: /outside/not-read') + expect(findDependencyAutomationInputs(app).files).toMatchObject([{path: '.github/dependabot.yml'}]) + }) + }) + + test.each(['.github', '.gitlab', '.git'])('rejects escaping or ambiguous %s directory links', async (path) => { + await inTemporaryDirectory(async (root) => { + await inTemporaryDirectory(async (outside) => { + await symlink(outside, join(root, path), 'dir') + expect(findDependencyAutomationInputs(root)).toMatchObject({ + files: [], + unresolvedReason: expect.any(String), + }) + }) + }) + }) + + test('rejects dangling directory links', async () => { + await inTemporaryDirectory(async (root) => { + await symlink(join(root, 'missing'), join(root, '.github'), 'dir') + expect(findDependencyAutomationInputs(root).unresolvedReason).toContain('dangling symbolic link') + }) + }) + + test('rejects escaping config-file links and accepts contained ones', async () => { + await inTemporaryDirectory(async (root) => { + await inTemporaryDirectory(async (outside) => { + await writeFile(join(outside, 'config.json'), '{}') + await symlink(join(outside, 'config.json'), join(root, 'renovate.json')) + expect(findDependencyAutomationInputs(root).unresolvedReason).toContain('outside the app root') + await writeFiles(root, {'.github/config.yml': 'version: 2\nupdates: []'}) + await symlink(join(root, '.github/config.yml'), join(root, '.github/dependabot.yml')) + const result = findDependencyAutomationInputs(root) + expect(result.files).toMatchObject([{path: '.github/dependabot.yml'}]) + expect(result.unresolvedReason).toBeUndefined() + }) + }) + }) + + test.skipIf(process.platform === 'win32')('rejects special files without attempting to read them', async () => { + await inTemporaryDirectory(async (root) => { + execFileSync('mkfifo', [join(root, 'renovate.json')]) + expect(findDependencyAutomationInputs(root).unresolvedReason).toContain('not a file') + execFileSync('mkfifo', [join(root, '.git')]) + expect(findDependencyAutomationInputs(root).unresolvedReason).toContain('repository ownership') + }) + }) +}) + +describe('manifest safety', () => { + test('rejects escaped manifests and parent paths', async () => { + await inTemporaryDirectory(async (root) => { + await inTemporaryDirectory(async (outside) => { + await writeFile(join(outside, 'package.json'), '{"dependencies":{"react":"19.0.0"}}') + await symlink(join(outside, 'package.json'), join(root, 'package.json')) + expect(findManifests(root, ['package.json', '../package.json'])).toEqual([]) + expect(getSkippedFiles()).toHaveLength(2) + }) + }) + }) + + test.each(['null', '[]', '{"dependencies":{"react":1}}', '{"devDependencies":{"vitest":null}}'])( + 'records malformed manifests as coverage gaps: %s', + async (content) => { + await inTemporaryDirectory(async (root) => { + await writeFile(join(root, 'package.json'), content) + expect(findManifests(root)).toMatchObject([{dependencies: {}, devDependencies: {}}]) + expect(getSkippedFiles()).toEqual([ + expect.objectContaining({reason: 'unreadable', detail: 'manifest could not be parsed'}), + ]) + }) + }, + ) + + test('does not retain or validate package scripts for this check', async () => { + await inTemporaryDirectory(async (root) => { + await writeFile(join(root, 'package.json'), '{"scripts":{"audit":["npm audit"]}}') + expect(findManifests(root)).toMatchObject([{dependencies: {}, devDependencies: {}}]) + expect(findManifests(root)[0]).not.toHaveProperty('scripts') + expect(getSkippedFiles()).toEqual([]) + }) + }) +}) diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-rules.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-rules.test.ts new file mode 100644 index 00000000000..c8a8c0100b7 --- /dev/null +++ b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-rules.test.ts @@ -0,0 +1,55 @@ +import {scanDependencyAutomation} from '../rules/dependency-automation-rules.js' +import {describe, expect, test} from 'vitest' +import type {ManifestFile, ScanContext} from '../rules/types.js' + +const manifest = (path = 'package.json'): ManifestFile => ({ + path, + absolutePath: `/${path}`, + type: 'npm', + dependencies: {react: '19.0.0'}, +}) + +function scan(dependencyAutomation: ScanContext['dependencyAutomation'] = {files: []}, manifests = [manifest()]) { + return scanDependencyAutomation({manifests, dependencyAutomation}) +} + +describe('dependency-management configuration file presence', () => { + test('is not applicable without declared dependencies', () => { + expect(scan({files: []}, [])).toEqual({issues: []}) + expect(scan({files: []}, [{...manifest(), dependencies: {}}])).toEqual({issues: []}) + }) + + test('includes development dependencies and anchors one finding to a real manifest', () => { + const result = scan({files: []}, [manifest('packages/web/package.json'), manifest()]) + expect( + scan({files: []}, [{...manifest(), dependencies: {}, devDependencies: {vitest: '4.0.0'}}]).issues, + ).toHaveLength(1) + expect(result.issues).toEqual([ + expect.objectContaining({ + id: 'MISSING_DEPENDENCY_SECURITY_AUTOMATION', + severity: 'low', + points: -5, + title: 'Dependency management configuration file not detected', + location: {file: 'package.json'}, + fix: expect.objectContaining({automated: false}), + }), + ]) + expect(result.issues[0]?.message).toContain('does not validate their contents') + }) + + test('preserves discovery obstacles without a finding', () => { + expect(scan({files: [], unresolvedReason: 'Nested repository'})).toEqual({ + issues: [], + unresolvedReason: 'Nested repository', + }) + }) + + test.each(['', '{malformed', '{"enabled":false}', '{"extends":["local>org/config"]}'])( + 'treats presence as sufficient, regardless of configuration content: %s', + (content) => { + expect(scan({files: [{path: 'renovate.json', absolutePath: '/renovate.json', ext: '.json', content}]})).toEqual({ + issues: [], + }) + }, + ) +}) diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts new file mode 100644 index 00000000000..cc9c4c769f8 --- /dev/null +++ b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts @@ -0,0 +1,213 @@ +/* eslint-disable no-restricted-imports -- integration coverage uses real temporary repositories */ +import {doctorExitCode} from '../../app-doctor-api.js' +import {formatJson} from '../output/format.js' +import {getRegistry} from '../registry/index.js' +import {DETERMINISTIC_CHECKS, scan} from '../scanners/index.js' +import {buildSubmission} from '../submission/index.js' +import {sha256, validateTrace} from '../trace/index.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' +import {describe, expect, test, vi} from 'vitest' +import {mkdir, unlink, writeFile} from 'node:fs/promises' +import {dirname, join} from 'node:path' + +vi.mock('@shopify/cli-kit/node/http', async (importActual) => { + const actual: any = await importActual() + return {...actual, fetch: vi.fn(actual.fetch)} +}) +vi.mock('@shopify/cli-kit/node/system', async (importActual) => { + const actual: any = await importActual() + return {...actual, captureOutputWithExitCode: vi.fn(actual.captureOutputWithExitCode)} +}) + +const checkId = 'MISSING_DEPENDENCY_SECURITY_AUTOMATION' +const dependabot = '# Configuration contents are not validated.\n' + +async function writeFiles(root: string, files: Record): Promise { + await Promise.all( + Object.entries(files).map(async ([path, content]) => { + await mkdir(dirname(join(root, path)), {recursive: true}) + await writeFile(join(root, path), content) + }), + ) +} + +async function makeApp(root: string, files: Record = {}): Promise { + await writeFiles(root, { + 'shopify.app.toml': `name = "Dependency automation integration" +[webhooks] +api_version = "unstable" +[[webhooks.subscriptions]] +compliance_topics = ["shop/redact", "customers/data_request", "customers/redact"] +uri = "https://example.test/webhooks" +`, + 'package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), + ...files, + }) +} + +function dependencyExecution(result: Awaited>) { + return result.scan.checks_executed.find((execution) => execution.id === checkId)! +} + +function dependencyFindings(result: Awaited>) { + return result.issues.filter((issue) => issue.id === checkId) +} + +describe('dependency automation scanner integration', () => { + test('registers one framework-independent low-severity structured-config check', () => { + expect(DETERMINISTIC_CHECKS.get(checkId)).toMatchObject({ + version: 1, + lifecycle: 'active', + analysisMode: 'structured_config', + target: 'dependency_automation', + }) + expect(DETERMINISTIC_CHECKS.get(checkId)).not.toHaveProperty('requires') + expect(getRegistry().filter((entry) => entry.id === checkId)).toEqual([ + expect.objectContaining({kind: 'deterministic', status: 'active', severity: 'low', points: -5}), + ]) + }) + + test('does not inspect bot configuration when no supported dependencies apply', async () => { + await inTemporaryDirectory(async (root) => { + await makeApp(root, {'package.json': '{}', '.github/dependabot.yml': 'x'.repeat(500_001)}) + const result = await scan(root) + expect(dependencyFindings(result)).toEqual([]) + expect(dependencyExecution(result)).toMatchObject({status: 'not_applicable', findings: 0, applicable: false}) + }) + }) + + test('reports one ordinary finding through scoring, blocking, trace, and submission', async () => { + await inTemporaryDirectory(async (root) => { + await makeApp(root) + const execution = await scanApp(root) + expect(dependencyFindings(execution.scan)).toEqual([ + expect.objectContaining({id: checkId, severity: 'low', points: -5, location: {file: 'package.json'}}), + ]) + expect(dependencyExecution(execution.scan)).toMatchObject({ + status: 'executed', + required: true, + findings: 1, + inspected_files: ['package.json'], + }) + expect(execution.scan.score).toEqual({total: 95, baseline: 100, grade: 'EXCELLENT'}) + expect(formatJson(execution.scan)).toContain(checkId) + expect(validateTrace(execution.trace)).toEqual({valid: true, errors: []}) + const submission = buildSubmission(execution.trace, {cliVersion: '3.99.0', submittedAt: '2026-09-15T00:00:00Z'}) + for (const findings of [execution.trace.findings, submission.report.findings]) { + expect(findings).toContainEqual(expect.objectContaining({rule_id: checkId, severity: 'low'})) + } + expect(doctorExitCode({...execution, elapsedMilliseconds: 0}, 'low')).toBe(1) + expect(doctorExitCode({...execution, elapsedMilliseconds: 0}, 'medium')).toBe(0) + expect(doctorExitCode({...execution, elapsedMilliseconds: 0}, 'none')).toBe(0) + }) + }) + + test.each([ + ['.github/dependabot.yml', dependabot], + ['.gitlab/renovate.json', '{"extends":["local>org/renovate-config"]}'], + ['renovate.json', '{"enabled":false}'], + ['.github/dependabot.yaml', 'not valid YAML: ['], + ['.renovaterc', ''], + ])( + 'recognizes %s without network requests, repository commands, or configuration disclosure', + async (path, content) => { + await inTemporaryDirectory(async (root) => { + await makeApp(root, {[path]: content}) + const execution = await scanApp(root) + const expectedFiles = ['package.json', path] + expect(dependencyFindings(execution.scan)).toEqual([]) + expect(dependencyExecution(execution.scan)).toMatchObject({ + status: 'executed', + findings: 0, + inspected_files: expectedFiles, + }) + expect(execution.scan.score).toEqual({total: 100, baseline: 100, grade: 'EXCELLENT'}) + expect(execution.trace.project.input_hashes[path]).toBe(sha256(content)) + expect(doctorExitCode({...execution, elapsedMilliseconds: 0}, 'low')).toBe(0) + const submission = buildSubmission(execution.trace, {cliVersion: '3.99.0', submittedAt: '2026-09-15T00:00:00Z'}) + expect(JSON.stringify(submission)).not.toContain('local>org/renovate-config') + expect(JSON.stringify(execution.trace)).not.toContain('local>org/renovate-config') + expect(vi.mocked(captureOutputWithExitCode).mock.calls.map(([command, args]) => [command, args])).toEqual([ + ['git', ['rev-parse', 'HEAD']], + ['git', ['status', '--porcelain']], + ]) + expect(fetch).not.toHaveBeenCalled() + }) + }, + ) + + test('ignores workflow contents entirely, including malformed CI configuration', async () => { + await inTemporaryDirectory(async (root) => { + await makeApp(root) + const initial = await scan(root) + await writeFiles(root, { + '.github/workflows/audit.yml': 'jobs: [', + '.gitlab-ci.yml': 'include: [', + '.circleci/config.yml': 'jobs: [', + '.snyk': 'version: v1.25.0', + }) + const result = await scan(root) + expect(dependencyFindings(result)).toHaveLength(1) + expect(dependencyExecution(result)).toMatchObject({status: 'executed', inspected_files: ['package.json']}) + expect(result.scan.input_hash).toBe(initial.scan.input_hash) + }) + }) + + test('does not treat package.json as a Renovate configuration filename', async () => { + await inTemporaryDirectory(async (root) => { + await makeApp(root, {'package.json': '{"dependencies":{"react":"19.0.0"},"renovate":{}}'}) + expect(dependencyFindings(await scan(root))).toHaveLength(1) + }) + }) + + test.each([ + ['.github/dependabot.yml', 'x'.repeat(500_001)], + ['packages/web/package.json', '{invalid'], + ])('leaves malformed or rejected input unresolved: %s', async (path, content) => { + await inTemporaryDirectory(async (root) => { + await makeApp(root, {[path]: content}) + const result = await scan(root) + expect(dependencyFindings(result)).toEqual([]) + expect(dependencyExecution(result)).toMatchObject({status: 'unresolved', findings: 0}) + expect(result.score).toBeNull() + }) + }) + + test('does not inspect repository-level configuration outside a nested app root', async () => { + await inTemporaryDirectory(async (repository) => { + const app = join(repository, 'apps/example') + await mkdir(join(repository, '.git')) + await makeApp(app, {'.github/dependabot.yml': dependabot}) + const result = await scan(app) + expect(dependencyFindings(result)).toEqual([]) + expect(dependencyExecution(result)).toMatchObject({ + status: 'unresolved', + reason: {message: expect.stringContaining('nested below repository root')}, + }) + }) + }) + + test.each(['.github/dependabot.yml', 'renovate.json'])( + 'hashes config changes, additions, and deletions: %s', + async (path) => { + await inTemporaryDirectory(async (root) => { + await makeApp(root) + const initial = await scan(root) + const content = path.endsWith('.yml') ? dependabot : '{}' + await writeFiles(root, {[path]: content}) + const added = await scan(root) + await writeFile(join(root, path), `${content}\r\n`) + const changed = await scan(root) + await unlink(join(root, path)) + const deleted = await scan(root) + expect(added.scan.file_hashes?.[path]).toBe(sha256(content)) + expect(changed.scan.file_hashes?.[path]).toBe(sha256(`${content}\r\n`)) + expect(new Set([initial.scan.input_hash, added.scan.input_hash, changed.scan.input_hash]).size).toBe(3) + expect(deleted.scan.input_hash).toBe(initial.scan.input_hash) + }) + }, + ) +}) diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/deterministic-rules.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/deterministic-rules.test.ts index 3224582cc8b..d8c050054e1 100644 --- a/packages/app/src/cli/services/app-doctor-engine/tests/deterministic-rules.test.ts +++ b/packages/app/src/cli/services/app-doctor-engine/tests/deterministic-rules.test.ts @@ -16,7 +16,7 @@ import type {SourceFile} from '../rules/types.js' const ACTIVE_IDS = [ 'MISSING_COMPLIANCE_WEBHOOKS', - 'MISSING_DEPENDENCY_AUDITING', + 'MISSING_DEPENDENCY_SECURITY_AUTOMATION', 'EOL_API_VERSION', 'EXPIRING_OFFLINE_TOKEN', 'UNAUTHENTICATED_ENDPOINT', diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/rule-analysis.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/rule-analysis.test.ts index 0faf11d762a..6fddcb696bd 100644 --- a/packages/app/src/cli/services/app-doctor-engine/tests/rule-analysis.test.ts +++ b/packages/app/src/cli/services/app-doctor-engine/tests/rule-analysis.test.ts @@ -35,7 +35,7 @@ function context( extensions: [], sourceFiles: input.files ?? [], manifests: [], - dependencyAuditing: {files: []}, + dependencyAutomation: {files: []}, sensitiveFiles: [], capabilities: { theme_app_extension: false, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 91f97063e57..3cda6dbddfc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -236,9 +236,6 @@ importers: ws: specifier: 8.21.0 version: 8.21.0 - yaml: - specifier: 2.9.0 - version: 2.9.0 devDependencies: '@types/diff': specifier: ^5.0.3 From 19dafdeb2a4583e56259fb127c8257196c63ccd1 Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Wed, 16 Sep 2026 13:07:55 -0500 Subject: [PATCH 3/4] Address App Doctor dependency-automation review comments Co-authored-by: AI (Pi/GPT-6 Astra) --- .../app-doctor-engine/scanners/discover.ts | 231 ++++++++++-------- .../dependency-automation-discovery.test.ts | 42 +++- .../tests/dependency-automation.test.ts | 2 +- 3 files changed, 166 insertions(+), 109 deletions(-) diff --git a/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts b/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts index 8e451b7cd3b..d8e12ff9870 100644 --- a/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts +++ b/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts @@ -9,8 +9,10 @@ import { cwd, dirname, extname, + isAbsolutePath, isSubpath, joinPath, + normalizePath as normalizeCliPath, relativePath, resolvePath, } from '@shopify/cli-kit/node/path' @@ -433,30 +435,106 @@ function repositoryPathFailure(detail: string): RepositoryReadFailure { return {ok: false, reason: 'unreadable', detail} } -function containedRepositoryPath(appRoot: string, path: string): {path?: string; failure?: RepositoryReadFailure} { +type InspectedPath = {status: 'missing'} | {status: 'file'; path: string} | {status: 'unresolved'; reason: string} + +function isMissingFilesystemEntry(error: unknown): boolean { + return error instanceof Error && 'code' in error && (error.code === 'ENOENT' || error.code === 'ENOTDIR') +} + +function inspectErrorReason(target: string, error: unknown): string { + const code = error instanceof Error && 'code' in error && typeof error.code === 'string' ? error.code : undefined + return code ? `Could not inspect ${target} (${code})` : `Could not inspect ${target}` +} + +function repositoryDisplayPath(appRoot: string, path: string): string { + const relative = normalizeCliPath(relativePath(appRoot, path)) + if ( + relative.length === 0 || + relative === '.' || + relative === '..' || + relative.startsWith('../') || + isAbsolutePath(relative) + ) { + return 'path' + } + return relative +} + +/** + * Resolve a repository path while preserving the difference between absence + * and an unsafe or unreadable entry. Walking each segment lets contained + * directory links through, but treats dangling or escaping intermediates as + * unresolved instead of ordinary allowlist absence. + */ +function inspectRepositoryPath(appRoot: string, path: string): InspectedPath { const absoluteRoot = resolvePath(appRoot) - const absolutePath = resolvePath(path) + const absolutePath = resolvePath(absoluteRoot, path) + const display = repositoryDisplayPath(absoluteRoot, absolutePath) if (!isSubpath(absoluteRoot, absolutePath)) { - return {failure: repositoryPathFailure('path escapes the app root')} + return {status: 'unresolved', reason: `${display} escapes the app root`} } + let canonicalRoot: string try { - const canonicalRoot = realpathSync(absoluteRoot) - const canonicalPath = realpathSync(absolutePath) - if (!isSubpath(canonicalRoot, canonicalPath)) { - return {failure: repositoryPathFailure('symbolic link escapes the app root')} - } - if (!lstatSync(canonicalPath).isFile()) { - return {failure: repositoryPathFailure('path is not a regular file')} - } - return {path: canonicalPath} - // Dangling links and paths whose metadata cannot be read are coverage gaps. + canonicalRoot = realpathSync(absoluteRoot) // eslint-disable-next-line no-catch-all/no-catch-all } catch (error) { - return { - failure: repositoryPathFailure(error instanceof Error ? error.message : String(error)), + return {status: 'unresolved', reason: inspectErrorReason('app root', error)} + } + + const segments = normalizeCliPath(relativePath(absoluteRoot, absolutePath)) + .split('/') + .filter((segment) => segment.length > 0 && segment !== '.') + if (segments.includes('..')) return {status: 'unresolved', reason: `${display} escapes the app root`} + + let currentPath = canonicalRoot + for (const [index, segment] of segments.entries()) { + currentPath = joinPath(currentPath, segment) + try { + lstatSync(currentPath) + const canonicalPath = realpathSync(currentPath) + if (!isSubpath(canonicalRoot, canonicalPath)) { + return {status: 'unresolved', reason: `${display} resolves outside the app root`} + } + + const stats = lstatSync(canonicalPath) + const isLastSegment = index === segments.length - 1 + if (isLastSegment) { + if (!stats.isFile()) return {status: 'unresolved', reason: `${display} is not a file`} + } else if (!stats.isDirectory()) { + return {status: 'unresolved', reason: `${display} contains an entry that is not a directory`} + } + currentPath = canonicalPath + // Discovery must distinguish a missing optional allowlist entry from an + // entry that exists but cannot be inspected safely. + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + if (isMissingFilesystemEntry(error)) { + // lstat succeeds for a dangling link, so ENOENT from realpath is an + // unsafe existing entry rather than ordinary allowlist absence. + try { + if (lstatSync(currentPath).isSymbolicLink()) { + return {status: 'unresolved', reason: `${display} contains a dangling symbolic link`} + } + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + return {status: 'missing'} + } + return {status: 'missing'} + } + return {status: 'unresolved', reason: inspectErrorReason(display, error)} } } + + if (segments.length === 0) return {status: 'unresolved', reason: `${display} is not a file`} + return {status: 'file', path: currentPath} +} + +function containedRepositoryPath(appRoot: string, path: string): {path?: string; failure?: RepositoryReadFailure} { + const inspected = inspectRepositoryPath(appRoot, path) + if (inspected.status === 'file') return {path: inspected.path} + if (inspected.status === 'missing') return {failure: repositoryPathFailure('path does not exist')} + return {failure: repositoryPathFailure(inspected.reason)} } function readRepositoryFile(appRoot: string, path: string): RepositoryReadResult { @@ -649,99 +727,31 @@ export function findSensitiveFiles(appRoot: string): SourceFile[] { }) } -interface AllowedPathResult { - exists: boolean - path?: string - unresolvedReason?: string -} - -function filesystemError(path: string, error: unknown): string { - const detail = error instanceof Error ? error.message : String(error) - return `Could not inspect ${path}: ${detail}` -} - -function isMissingFilesystemEntry(error: unknown): boolean { - return error instanceof Error && 'code' in error && (error.code === 'ENOENT' || error.code === 'ENOTDIR') -} - -/** - * Resolve an allowlisted path while preserving the difference between absence - * and an unsafe or unreadable entry. `realpathSync` follows every intermediate - * directory link, allowing contained links but rejecting escapes and dangling - * links explicitly. - */ -function inspectAllowedPath(appRoot: string, relative: string): AllowedPathResult { - const path = resolvePath(appRoot, relative) - if (!isSubpath(appRoot, path)) return {exists: false, unresolvedReason: `${relative} escapes the app root`} - - const segments = relative.replace(/\\/g, '/').split('/').filter(Boolean) - let currentPath = appRoot - for (const [index, segment] of segments.entries()) { - currentPath = joinPath(currentPath, segment) - try { - lstatSync(currentPath) - const canonicalPath = realpathSync(currentPath) - if (!isSubpath(appRoot, canonicalPath)) { - return {exists: false, unresolvedReason: `${relative} resolves outside the app root`} - } - - const stats = lstatSync(canonicalPath) - const isLastSegment = index === segments.length - 1 - const requiredType = isLastSegment ? 'file' : 'directory' - const hasRequiredType = requiredType === 'file' ? stats.isFile() : stats.isDirectory() - if (!hasRequiredType) { - return {exists: false, unresolvedReason: `${relative} contains an entry that is not a ${requiredType}`} - } - // Discovery must distinguish a missing optional allowlist entry from an - // entry that exists but cannot be inspected safely. - // eslint-disable-next-line no-catch-all/no-catch-all - } catch (error) { - if (isMissingFilesystemEntry(error)) { - // lstat succeeds for a dangling link, so ENOENT from realpath is an - // unsafe existing entry rather than ordinary allowlist absence. - try { - if (lstatSync(currentPath).isSymbolicLink()) { - return {exists: false, unresolvedReason: `${relative} contains a dangling symbolic link`} - } - // eslint-disable-next-line no-catch-all/no-catch-all - } catch { - return {exists: false} - } - } - return {exists: false, unresolvedReason: filesystemError(relative, error)} - } - } - - return {exists: true, path} -} - function nestedRepositoryReason(appRoot: string): string | undefined { - const rootMarker = joinPath(appRoot, '.git') try { - const stats = lstatSync(rootMarker) + const stats = lstatSync(joinPath(appRoot, '.git')) if (stats.isDirectory() || stats.isFile()) return undefined // A root marker establishes the app's repository boundary only when it is // a directory or worktree file. Never follow ambiguous marker entries. - return `Could not determine repository ownership from ${rootMarker}` + return 'Could not determine repository ownership from .git' // eslint-disable-next-line no-catch-all/no-catch-all } catch (error) { - if (!isMissingFilesystemEntry(error)) return filesystemError(rootMarker, error) + if (!isMissingFilesystemEntry(error)) return inspectErrorReason('.git', error) } let ancestor = dirname(appRoot) while (true) { - const marker = joinPath(ancestor, '.git') try { - const stats = lstatSync(marker) + const stats = lstatSync(joinPath(ancestor, '.git')) if (stats.isDirectory() || stats.isFile()) { - return `App root is nested below repository root ${ancestor}` + return 'App root is nested below a parent Git repository' } // A symlink or special file is not a supported repository marker, but // treating it as absent would make repository ownership ambiguous. - return `Could not determine repository ownership from ${marker}` + return 'Could not determine repository ownership from .git' // eslint-disable-next-line no-catch-all/no-catch-all } catch (error) { - if (!isMissingFilesystemEntry(error)) return filesystemError(marker, error) + if (!isMissingFilesystemEntry(error)) return inspectErrorReason('.git', error) } const parent = dirname(ancestor) @@ -750,43 +760,60 @@ function nestedRepositoryReason(appRoot: string): string | undefined { } } +function recordRejectedAllowlistPath(appRoot: string, relative: string, failure: RepositoryReadFailure): void { + recordSkippedFile(appRoot, resolvePath(appRoot, relative), failure) +} + /** Read local bot configuration only; hosted integrations and CI workflows are outside this check's scope. */ export function findDependencyAutomationInputs(appRoot: string): DependencyAutomationInputs { let canonicalRoot: string try { canonicalRoot = realpathSync(resolvePath(appRoot)) if (!lstatSync(canonicalRoot).isDirectory()) { - return {files: [], unresolvedReason: `App root is not a directory: ${appRoot}`} + return {files: [], unresolvedReason: 'App root is not a directory'} } // eslint-disable-next-line no-catch-all/no-catch-all } catch (error) { - return {files: [], unresolvedReason: filesystemError(appRoot, error)} + return {files: [], unresolvedReason: inspectErrorReason('app root', error)} } const repositoryReason = nestedRepositoryReason(canonicalRoot) if (repositoryReason) return {files: [], unresolvedReason: repositoryReason} const files: SourceFile[] = [] + let unresolvedReason: string | undefined for (const relative of DEPENDENCY_AUTOMATION_CONFIG_PATHS) { - const inspected = inspectAllowedPath(canonicalRoot, relative) - if (inspected.unresolvedReason) return {files, unresolvedReason: inspected.unresolvedReason} - if (!inspected.exists) continue + const inspected = inspectRepositoryPath(canonicalRoot, relative) + if (inspected.status === 'missing') continue + if (inspected.status === 'unresolved') { + recordRejectedAllowlistPath(canonicalRoot, relative, { + ok: false, + reason: 'unreadable', + detail: inspected.reason, + }) + unresolvedReason ??= inspected.reason + continue + } + + const absolutePath = resolvePath(canonicalRoot, relative) + const result = readBoundedFile(inspected.path) + if (!result.ok) { + recordRejectedAllowlistPath(canonicalRoot, relative, result) + unresolvedReason ??= + result.reason === 'too_large' ? `${relative} is too large to inspect` : `Could not read ${relative}` + continue + } - const absolutePath = inspected.path! - const result = readRepositoryFile(canonicalRoot, absolutePath) files.push({ path: relative, absolutePath, ext: extname(relative), - content: result.ok ? result.content.toString() : undefined, + content: result.content.toString(), }) - if (!result.ok && result.reason === 'unreadable') { - return {files, unresolvedReason: `Could not read ${relative}: ${result.detail ?? 'unknown filesystem error'}`} - } // One safely inspected configuration file is enough for this presence-only check. break } - return {files} + return files.length > 0 ? {files} : {files, ...(unresolvedReason ? {unresolvedReason} : {})} } /** Find JavaScript package manifests. Dependency analysis intentionally supports JavaScript only. */ diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-discovery.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-discovery.test.ts index 0f789ac36f0..6bf63cf05e3 100644 --- a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-discovery.test.ts +++ b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-discovery.test.ts @@ -64,7 +64,8 @@ describe('dependency automation discovery', () => { await inTemporaryDirectory(async (root) => { await writeFiles(root, {'.github/dependabot.yml': 'x'.repeat(500_001)}) expect(findDependencyAutomationInputs(root)).toMatchObject({ - files: [{path: '.github/dependabot.yml', content: undefined}], + files: [], + unresolvedReason: expect.stringContaining('too large'), }) expect(getSkippedFiles()).toEqual([ expect.objectContaining({path: '.github/dependabot.yml', reason: 'too_large', size_bytes: 500_001}), @@ -78,10 +79,12 @@ describe('dependency automation discovery', () => { 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') - expect(findDependencyAutomationInputs(app)).toMatchObject({ + const nested = findDependencyAutomationInputs(app) + expect(nested).toMatchObject({ files: [], - unresolvedReason: expect.stringContaining('nested below repository root'), + unresolvedReason: 'App root is nested below a parent Git repository', }) + 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(findDependencyAutomationInputs(app).files).toMatchObject([{path: '.github/dependabot.yml'}]) @@ -92,10 +95,13 @@ describe('dependency automation discovery', () => { await inTemporaryDirectory(async (root) => { await inTemporaryDirectory(async (outside) => { await symlink(outside, join(root, path), 'dir') - expect(findDependencyAutomationInputs(root)).toMatchObject({ + const result = findDependencyAutomationInputs(root) + expect(result).toMatchObject({ files: [], unresolvedReason: expect.any(String), }) + expect(result.unresolvedReason).not.toContain(root) + expect(result.unresolvedReason).not.toContain(outside) }) }) }) @@ -103,7 +109,27 @@ describe('dependency automation discovery', () => { test('rejects dangling directory links', async () => { await inTemporaryDirectory(async (root) => { await symlink(join(root, 'missing'), join(root, '.github'), 'dir') - expect(findDependencyAutomationInputs(root).unresolvedReason).toContain('dangling symbolic link') + const result = findDependencyAutomationInputs(root) + expect(result.unresolvedReason).toContain('dangling symbolic link') + expect(result.unresolvedReason).not.toContain(root) + expect(getSkippedFiles()).toContainEqual( + expect.objectContaining({path: '.github/dependabot.yml', reason: 'unreadable'}), + ) + }) + }) + + test('keeps looking after an unsafe allowlisted path', async () => { + await inTemporaryDirectory(async (root) => { + await inTemporaryDirectory(async (outside) => { + await symlink(outside, join(root, '.github'), 'dir') + await writeFiles(root, {'renovate.json': '{}'}) + const result = findDependencyAutomationInputs(root) + expect(result.files).toMatchObject([{path: 'renovate.json', content: '{}'}]) + expect(result.unresolvedReason).toBeUndefined() + expect(getSkippedFiles()).toContainEqual( + expect.objectContaining({path: '.github/dependabot.yml', reason: 'unreadable'}), + ) + }) }) }) @@ -112,7 +138,11 @@ describe('dependency automation discovery', () => { await inTemporaryDirectory(async (outside) => { await writeFile(join(outside, 'config.json'), '{}') await symlink(join(outside, 'config.json'), join(root, 'renovate.json')) - expect(findDependencyAutomationInputs(root).unresolvedReason).toContain('outside the app root') + const rejected = findDependencyAutomationInputs(root) + expect(rejected.unresolvedReason).toContain('outside the app root') + expect(rejected.unresolvedReason).not.toContain(root) + expect(rejected.unresolvedReason).not.toContain(outside) + expect(getSkippedFiles()).toContainEqual(expect.objectContaining({path: 'renovate.json', reason: 'unreadable'})) await writeFiles(root, {'.github/config.yml': 'version: 2\nupdates: []'}) await symlink(join(root, '.github/config.yml'), join(root, '.github/dependabot.yml')) const result = findDependencyAutomationInputs(root) diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts index cc9c4c769f8..28ea9801b9f 100644 --- a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts +++ b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts @@ -185,7 +185,7 @@ describe('dependency automation scanner integration', () => { expect(dependencyFindings(result)).toEqual([]) expect(dependencyExecution(result)).toMatchObject({ status: 'unresolved', - reason: {message: expect.stringContaining('nested below repository root')}, + reason: {message: 'App root is nested below a parent Git repository'}, }) }) }) From c8553626e6569f23fb76cf27e407b734dd9daffa Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Thu, 17 Sep 2026 09:00:12 -0500 Subject: [PATCH 4/4] Address remaining App Doctor dependency-automation review comments Co-authored-by: AI (Pi/GPT-6 Astra) --- .../app-doctor-engine/rules/catalog.ts | 7 ++-- .../rules/dependency-automation-rules.ts | 37 ++++++++++++++----- .../app-doctor-engine/scanners/discover.ts | 27 ++++++++++++-- .../tests/dependency-automation-rules.test.ts | 21 ++++++++++- .../tests/dependency-automation.test.ts | 6 ++- 5 files changed, 78 insertions(+), 20 deletions(-) diff --git a/packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts b/packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts index 3b6fa0b89bd..42f5f181b91 100644 --- a/packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts +++ b/packages/app/src/cli/services/app-doctor-engine/rules/catalog.ts @@ -147,11 +147,12 @@ export const RULE_CATALOG: RuleCatalogEntry[] = [ }, { id: 'MISSING_DEPENDENCY_SECURITY_AUTOMATION', - title: 'Dependency management configuration file not detected', + title: 'Repository-level dependency management configuration not detected', severity: 'low', points: -5, - description: 'Looks for a local Dependabot or Renovate configuration file without validating its contents.', - fix: 'Configure Dependabot or Renovate for this app, or verify existing coverage.', + description: + 'Looks for a local Dependabot or Renovate configuration file at the repository root without validating its contents.', + fix: 'Add a Dependabot or Renovate configuration file at the repository root, or verify existing coverage.', }, { id: 'EOL_API_VERSION', diff --git a/packages/app/src/cli/services/app-doctor-engine/rules/dependency-automation-rules.ts b/packages/app/src/cli/services/app-doctor-engine/rules/dependency-automation-rules.ts index 90723e6d51d..60ef90a0b75 100644 --- a/packages/app/src/cli/services/app-doctor-engine/rules/dependency-automation-rules.ts +++ b/packages/app/src/cli/services/app-doctor-engine/rules/dependency-automation-rules.ts @@ -1,5 +1,5 @@ import type {Issue} from '../types.js' -import type {ScanContext} from './types.js' +import type {ManifestFile, ScanContext} from './types.js' export const DEPENDENCY_AUTOMATION_CONFIG_PATHS = [ '.github/dependabot.yml', @@ -19,34 +19,51 @@ export const DEPENDENCY_AUTOMATION_CONFIG_PATHS = [ '.renovaterc.json5', ] +function hasDeclaredDependencies(manifest: ManifestFile): boolean { + return Object.keys(manifest.dependencies).length > 0 || Object.keys(manifest.devDependencies ?? {}).length > 0 +} + +function repositoryManifestPath(path: string): string { + return path.replace(/\\/g, '/') +} + +/** Dependabot and Renovate configuration is repository-level, so prefer the root manifest. */ +function dependencyAutomationFindingFile(manifests: ManifestFile[]): string { + const paths = manifests.map((manifest) => repositoryManifestPath(manifest.path)) + const root = paths.find((path) => path === 'package.json') + if (root) return root + + return [...paths].sort((left, right) => { + const depthDelta = left.split('/').length - right.split('/').length + return depthDelta === 0 ? left.localeCompare(right) : depthDelta + })[0]! +} + /** File presence is an adoption signal, not proof of valid configuration, execution, or dependency coverage. */ export function scanDependencyAutomation(context: Pick): { issues: Issue[] unresolvedReason?: string } { - const manifests = context.manifests.filter( - (manifest) => - Object.keys(manifest.dependencies).length > 0 || Object.keys(manifest.devDependencies ?? {}).length > 0, - ) + const manifests = context.manifests.filter(hasDeclaredDependencies) if (manifests.length === 0) return {issues: []} const {files, unresolvedReason} = context.dependencyAutomation if (unresolvedReason) return {issues: [], unresolvedReason} if (files.length > 0) return {issues: []} - const manifest = [...manifests].sort((left, right) => left.path.localeCompare(right.path))[0]! return { issues: [ { id: 'MISSING_DEPENDENCY_SECURITY_AUTOMATION', severity: 'low', points: -5, - title: 'Dependency management configuration file not detected', + title: 'Repository-level dependency management configuration not detected', message: - 'No recognized Dependabot or Renovate configuration file was found. Add dependency update automation, or verify that an existing integration covers this app. This check only looks for local configuration files; it does not validate their contents or inspect hosted integrations, CI workflows, or execution results.', - location: {file: manifest.path.replace(/\\/g, '/')}, + 'No recognized Dependabot or Renovate configuration file was found at the repository root. Add dependency update automation there, or verify that an existing integration covers this app. This check only looks for local configuration files; it does not validate their contents or inspect hosted integrations, CI workflows, or execution results.', + location: {file: dependencyAutomationFindingFile(manifests)}, fix: { automated: false, - description: 'Configure Dependabot or Renovate for this app, or verify existing coverage.', + description: + 'Add a Dependabot or Renovate configuration file at the repository root, or verify existing coverage.', }, }, ], diff --git a/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts b/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts index d8e12ff9870..0903037cfbe 100644 --- a/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts +++ b/packages/app/src/cli/services/app-doctor-engine/scanners/discover.ts @@ -462,9 +462,9 @@ function repositoryDisplayPath(appRoot: string, path: string): string { /** * Resolve a repository path while preserving the difference between absence - * and an unsafe or unreadable entry. Walking each segment lets contained - * directory links through, but treats dangling or escaping intermediates as - * unresolved instead of ordinary allowlist absence. + * and an unsafe or unreadable entry. Existing files take a single realpath; + * prefix walking runs only after ENOENT/ENOTDIR so optional allowlist paths + * can distinguish ordinary absence from a dangling or escaping intermediate. */ function inspectRepositoryPath(appRoot: string, path: string): InspectedPath { const absoluteRoot = resolvePath(appRoot) @@ -486,7 +486,27 @@ function inspectRepositoryPath(appRoot: string, path: string): InspectedPath { .split('/') .filter((segment) => segment.length > 0 && segment !== '.') if (segments.includes('..')) return {status: 'unresolved', reason: `${display} escapes the app root`} + if (segments.length === 0) return {status: 'unresolved', reason: `${display} is not a file`} + try { + const canonicalPath = realpathSync(absolutePath) + if (!isSubpath(canonicalRoot, canonicalPath)) { + return {status: 'unresolved', reason: `${display} resolves outside the app root`} + } + if (!lstatSync(canonicalPath).isFile()) { + return {status: 'unresolved', reason: `${display} is not a file`} + } + return {status: 'file', path: canonicalPath} + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + if (isMissingFilesystemEntry(error)) { + return inspectMissingRepositoryPath(canonicalRoot, segments, display) + } + return {status: 'unresolved', reason: inspectErrorReason(display, error)} + } +} + +function inspectMissingRepositoryPath(canonicalRoot: string, segments: string[], display: string): InspectedPath { let currentPath = canonicalRoot for (const [index, segment] of segments.entries()) { currentPath = joinPath(currentPath, segment) @@ -526,7 +546,6 @@ function inspectRepositoryPath(appRoot: string, path: string): InspectedPath { } } - if (segments.length === 0) return {status: 'unresolved', reason: `${display} is not a file`} return {status: 'file', path: currentPath} } diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-rules.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-rules.test.ts index c8a8c0100b7..ed1cfe2e9e7 100644 --- a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-rules.test.ts +++ b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation-rules.test.ts @@ -29,14 +29,33 @@ describe('dependency-management configuration file presence', () => { id: 'MISSING_DEPENDENCY_SECURITY_AUTOMATION', severity: 'low', points: -5, - title: 'Dependency management configuration file not detected', + title: 'Repository-level dependency management configuration not detected', location: {file: 'package.json'}, fix: expect.objectContaining({automated: false}), }), ]) + expect(result.issues[0]?.message).toContain('repository root') expect(result.issues[0]?.message).toContain('does not validate their contents') }) + test('prefers the root manifest over a nested extension even when localeCompare would not', () => { + const result = scan({files: []}, [ + manifest('extensions/app-home/package.json'), + manifest('web/package.json'), + manifest(), + ]) + expect(result.issues[0]?.location).toEqual({file: 'package.json'}) + }) + + test('falls back to the shallowest nested manifest when the root has no dependencies', () => { + const result = scan({files: []}, [ + {...manifest(), dependencies: {}}, + manifest('extensions/app-home/package.json'), + manifest('web/package.json'), + ]) + expect(result.issues[0]?.location).toEqual({file: 'web/package.json'}) + }) + test('preserves discovery obstacles without a finding', () => { expect(scan({files: [], unresolvedReason: 'Nested repository'})).toEqual({ issues: [], diff --git a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts index 28ea9801b9f..9af1a07a0cc 100644 --- a/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts +++ b/packages/app/src/cli/services/app-doctor-engine/tests/dependency-automation.test.ts @@ -81,7 +81,9 @@ describe('dependency automation scanner integration', () => { test('reports one ordinary finding through scoring, blocking, trace, and submission', async () => { await inTemporaryDirectory(async (root) => { - await makeApp(root) + await makeApp(root, { + 'extensions/app-home/package.json': JSON.stringify({dependencies: {react: '^19.0.0'}}), + }) const execution = await scanApp(root) expect(dependencyFindings(execution.scan)).toEqual([ expect.objectContaining({id: checkId, severity: 'low', points: -5, location: {file: 'package.json'}}), @@ -90,7 +92,7 @@ describe('dependency automation scanner integration', () => { status: 'executed', required: true, findings: 1, - inspected_files: ['package.json'], + inspected_files: ['extensions/app-home/package.json', 'package.json'], }) expect(execution.scan.score).toEqual({total: 95, baseline: 100, grade: 'EXCELLENT'}) expect(formatJson(execution.scan)).toContain(checkId)