From 4ca931c2e24dd66a6b28af3a2099d0444d2c4c0c Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Tue, 15 Sep 2026 13:23:23 -0500 Subject: [PATCH] Recognize GitLab CI dependency auditing in App Doctor Co-authored-by: AI (Pi/GPT-6 Astra) --- ...d-app-doctor-gitlab-dependency-auditing.md | 5 + .../dependency-auditing-gitlab-schema.ts | 43 +++++++ .../rules/dependency-auditing-rules.ts | 111 +++++++++++++++++ .../app-doctor-engine/scanners/discover.ts | 4 +- .../app-doctor-engine/scanners/index.ts | 4 +- .../dependency-auditing-discovery.test.ts | 2 + .../tests/dependency-auditing-rules.test.ts | 113 ++++++++++++++++++ 7 files changed, 280 insertions(+), 2 deletions(-) create mode 100644 .changeset/add-app-doctor-gitlab-dependency-auditing.md create mode 100644 packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-gitlab-schema.ts diff --git a/.changeset/add-app-doctor-gitlab-dependency-auditing.md b/.changeset/add-app-doctor-gitlab-dependency-auditing.md new file mode 100644 index 00000000000..1728f0bfd1b --- /dev/null +++ b/.changeset/add-app-doctor-gitlab-dependency-auditing.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +Recognize GitLab CI dependency vulnerability auditing configuration in App Doctor. diff --git a/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-gitlab-schema.ts b/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-gitlab-schema.ts new file mode 100644 index 00000000000..8b1a50cde47 --- /dev/null +++ b/packages/app/src/cli/services/app-doctor-engine/rules/dependency-auditing-gitlab-schema.ts @@ -0,0 +1,43 @@ +import {zod} from '@shopify/cli-kit/node/schema' + +export const GitLabConfigurationSchema = zod + .object({ + workflow: zod.object({rules: zod.array(zod.unknown()).optional()}).optional(), + variables: zod.unknown(), + include: zod.unknown(), + default: zod.unknown(), + before_script: zod.unknown(), + after_script: zod.unknown(), + }) + // Non-reserved top-level keys are job definitions, inspected individually. + .passthrough() + +export const GitLabVariablesSchema = zod.object({DS_DISABLED: zod.unknown()}).default({}) +export const GitLabDefaultsSchema = zod.object({before_script: zod.unknown(), after_script: zod.unknown()}).default({}) + +export const GitLabJobSchema = zod.object({ + rules: zod.array(zod.unknown()).optional(), + when: zod.unknown(), + extends: zod.unknown(), + inherit: zod.unknown(), + // Select inherited scripts only after checking whether the job executes. + before_script: zod.unknown(), + script: zod.unknown(), + after_script: zod.unknown(), +}) + +const CommandListSchema = zod + .union([zod.string().transform((command) => [command]), zod.array(zod.string())]) + .default([]) + +export const GitLabScriptsSchema = zod.object({ + before_script: CommandListSchema, + script: CommandListSchema, + after_script: CommandListSchema, +}) + +export const GitLabNeverRulesSchema = zod.array(zod.object({when: zod.literal('never')})).nonempty() +export const GitLabIncludeHeaderSchema = zod.object({rules: zod.unknown()}) +export const GitLabDependencyScanningTemplateSchema = zod.object({ + template: zod.enum(['Security/Dependency-Scanning.gitlab-ci.yml', 'Jobs/Dependency-Scanning.gitlab-ci.yml']), +}) 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 index eb8b35f7e9e..4b6699269d5 100644 --- 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 @@ -13,6 +13,16 @@ import { SnykArgsInputsSchema, SnykCommandInputsSchema, } from './dependency-auditing-github-schema.js' +import { + GitLabConfigurationSchema, + GitLabDefaultsSchema, + GitLabDependencyScanningTemplateSchema, + GitLabIncludeHeaderSchema, + GitLabJobSchema, + GitLabNeverRulesSchema, + GitLabScriptsSchema, + GitLabVariablesSchema, +} from './dependency-auditing-gitlab-schema.js' import {parseDocument} from 'yaml' import type {GitHubActionStep, GitHubDefaults, GitHubRunStep} from './dependency-auditing-github-schema.js' import type {Issue} from '../types.js' @@ -34,6 +44,19 @@ 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 GITLAB_RESERVED_KEYS = new Set([ + 'after_script', + 'before_script', + 'cache', + 'default', + 'image', + 'include', + 'pages', + 'services', + 'stages', + 'variables', + 'workflow', +]) 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([ @@ -123,6 +146,9 @@ function analyzeConfiguration( if (/^\.github\/workflows\/[^/]+\.ya?ml$/i.test(path)) { return combine(warnings, analyzeGitHubWorkflow(parsed.value, path, manifests)) } + if (/(?:^|\/)\.gitlab-ci\.ya?ml$/i.test(path)) { + return combine(warnings, analyzeGitLabConfiguration(parsed.value, path, manifests)) + } return warnings } @@ -289,6 +315,91 @@ function analyzeGitHubAction(step: GitHubActionStep, workflowPath: string, manif return noEvidence() } +function analyzeGitLabConfiguration(value: unknown, path: string, manifests: ManifestFile[]): Analysis { + const configuration = GitLabConfigurationSchema.safeParse(value) + if (!configuration.success) return obstacle(`GitLab CI configuration has an unsupported structure: ${path}`) + const root = configuration.data + if (GitLabNeverRulesSchema.safeParse(root.workflow?.rules).success) return noEvidence() + + const variables = GitLabVariablesSchema.safeParse(root.variables) + if (!variables.success) return obstacle(`GitLab CI variables have an unsupported structure: ${path}`) + const dependencyScanningDisabled = variables.data.DS_DISABLED === true || variables.data.DS_DISABLED === 'true' + let analysis = analyzeGitLabIncludes(root.include, path, dependencyScanningDisabled) + const defaults = GitLabDefaultsSchema.safeParse(root.default) + if (!defaults.success) { + analysis = combine(analysis, obstacle(`GitLab CI default has an unsupported structure: ${path}`)) + } + const defaultConfiguration = defaults.success ? defaults.data : undefined + + for (const [jobName, jobValue] of Object.entries(root)) { + if (jobName.startsWith('.') || GITLAB_RESERVED_KEYS.has(jobName)) continue + const jobResult = GitLabJobSchema.safeParse(jobValue) + if (!jobResult.success) { + analysis = combine(analysis, obstacle(`GitLab CI has an unsupported job in ${path}`)) + continue + } + const job = jobResult.data + if (job.when === 'never' || GitLabNeverRulesSchema.safeParse(job.rules).success) continue + if (job.extends !== undefined || job.inherit !== undefined) { + analysis = combine(analysis, obstacle(`Unsupported GitLab job inheritance referenced by ${path}`)) + continue + } + if (job.script === undefined) continue + + const scripts = GitLabScriptsSchema.safeParse({ + before_script: job.before_script ?? defaultConfiguration?.before_script ?? root.before_script, + script: job.script, + after_script: job.after_script ?? defaultConfiguration?.after_script ?? root.after_script, + }) + if (!scripts.success) { + analysis = combine(analysis, obstacle(`GitLab CI job has an unsupported script structure in ${path}`)) + continue + } + const mainCommands = [...scripts.data.before_script, ...scripts.data.script] + if (mainCommands.length > 0) { + analysis = combine( + analysis, + analyzeCommands(mainCommands.join('\n'), {directory: '', manifests, allowPackageScript: true}, path), + ) + } + if (scripts.data.after_script.length > 0) { + analysis = combine( + analysis, + analyzeCommands( + scripts.data.after_script.join('\n'), + {directory: '', manifests, allowPackageScript: true}, + path, + ), + ) + } + } + return analysis +} + +function analyzeGitLabIncludes(value: unknown, path: string, dependencyScanningDisabled: boolean): Analysis { + if (value === undefined) return noEvidence() + let analysis = noEvidence() + for (const include of Array.isArray(value) ? value : [value]) { + const header = GitLabIncludeHeaderSchema.safeParse(include) + if (!header.success) { + analysis = combine(analysis, obstacle(`Unsupported GitLab include referenced by ${path}`)) + continue + } + if (header.data.rules !== undefined) { + if (GitLabNeverRulesSchema.safeParse(header.data.rules).success) continue + analysis = combine(analysis, obstacle(`Conditional GitLab include can't be resolved in ${path}`)) + continue + } + if (GitLabDependencyScanningTemplateSchema.safeParse(include).success) { + if (dependencyScanningDisabled) continue + analysis = combine(analysis, evidence()) + } else { + analysis = combine(analysis, obstacle(`Unsupported GitLab include referenced by ${path}`)) + } + } + return analysis +} + function analyzeCommands(command: string, context: CommandContext, configurationPath: string): Analysis { const parsedShell = splitShell(command) if (!parsedShell.ok) return obstacle(`Unsupported shell syntax referenced by ${configurationPath}`) 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..2cbfe1b80fb 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 @@ -749,6 +749,8 @@ function nestedRepositoryReason(appRoot: string): string | undefined { } } +const DEPENDENCY_AUDITING_FILES = ['.gitlab-ci.yml'] + /** Discover only repository CI files that can explicitly invoke dependency auditing. */ export function findDependencyAuditingInputs(appRoot: string): DependencyAuditingInputs { let canonicalRoot: string @@ -765,7 +767,7 @@ export function findDependencyAuditingInputs(appRoot: string): DependencyAuditin const repositoryReason = nestedRepositoryReason(canonicalRoot) if (repositoryReason) return {files: [], unresolvedReason: repositoryReason} - const candidatePaths: string[] = [] + const candidatePaths = [...DEPENDENCY_AUDITING_FILES] const workflows = inspectAllowedPath(canonicalRoot, '.github/workflows', 'directory') if (workflows.unresolvedReason) return {files: [], unresolvedReason: workflows.unresolvedReason} if (workflows.exists) { 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..3f8527488b7 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 @@ -547,7 +547,9 @@ function skippedInputsForCheck( !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) + /(^|\/)package\.json$/.test(path) || + /^\.github\/workflows\/[^/]+\.ya?ml$/i.test(path) || + /^\.gitlab-ci\.ya?ml$/i.test(path) const isSecretInput = (file: SkippedFile) => !file.detail?.includes('could not be parsed') && (context.sourceCandidates.some((candidate) => candidate.path === file.path) || 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 index eb3a16586c1..16a3f242271 100644 --- 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 @@ -44,6 +44,7 @@ describe('dependency auditing input discovery', () => { '.github/workflows/dependencies.yml': workflow, '.github/workflows/release.yaml': 'name: release\n', '.github/workflows/ignored.json': '{}', + '.gitlab-ci.yml': 'audit:\n script: npm audit\n', '.hidden/config.yml': 'not: allowlisted\n', }) @@ -53,6 +54,7 @@ describe('dependency auditing input discovery', () => { expect(result.files.map(({path}) => path)).toEqual([ '.github/workflows/dependencies.yml', '.github/workflows/release.yaml', + '.gitlab-ci.yml', ]) const discoveredWorkflow = result.files.find(({path}) => path.endsWith('dependencies.yml')) expect(discoveredWorkflow?.content).toBe(workflow) 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 index 6e67588aa17..0c8a75d03e3 100644 --- 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 @@ -27,6 +27,8 @@ jobs: ${body}`, ) +const gitlab = (content: string): SourceFile => configuration('.gitlab-ci.yml', content) + function scan(files: SourceFile[], manifests: ManifestFile[] = [manifest()]) { return scanDependencyAuditing({manifests, dependencyAuditing: {files}}) } @@ -387,6 +389,102 @@ describe('package scripts and bounded shell handling', () => { }) }) +describe('GitLab dependency scanning', () => { + test.each(['Security/Dependency-Scanning.gitlab-ci.yml', 'Jobs/Dependency-Scanning.gitlab-ci.yml'])( + 'recognizes the %s template', + (template) => expectRecognized([gitlab(`include:\n - template: ${template}`)]), + ) + + test('recognizes explicit commands only in executable job fields', () => { + expectRecognized( + [gitlab('security:\n script:\n - cd packages/web\n - npm audit')], + [manifest('packages/web/package.json')], + ) + expectMissing([gitlab('variables:\n AUDIT_EXAMPLE: "npm audit"\nsecurity:\n script: npm test')]) + }) + + test('ignores an unexecuted hidden job and an always-disabled job', () => { + expectMissing([ + gitlab( + '.audit-template:\n script: npm audit\naudit:\n when: never\n script: npm audit\nbuild:\n script: npm test', + ), + ]) + }) + + test('leaves an unknown include unresolved, unless direct evidence is present', () => { + const unknown = gitlab('include:\n - local: .gitlab/security.yml\nbuild:\n script: npm test') + expect(scan([unknown])).toMatchObject({issues: [], unresolvedReason: expect.any(String)}) + + expectRecognized([gitlab('include:\n - project: platform/ci\n file: audit.yml\naudit:\n script: npm audit')]) + }) + + test('does not recognize disabled dependency-scanning templates or pipelines', () => { + expectMissing([ + gitlab('include:\n - template: Security/Dependency-Scanning.gitlab-ci.yml\n rules:\n - when: never'), + ]) + expectMissing([ + gitlab('variables:\n DS_DISABLED: "true"\ninclude:\n - template: Security/Dependency-Scanning.gitlab-ci.yml'), + ]) + expectMissing([ + gitlab( + 'workflow:\n rules:\n - if: $ON_BRANCH\n when: never\n - when: never\ninclude:\n - template: Security/Dependency-Scanning.gitlab-ci.yml', + ), + ]) + }) + + test('leaves conditional includes unresolved', () => { + expectUnresolved([ + gitlab( + 'include:\n - template: Security/Dependency-Scanning.gitlab-ci.yml\n rules:\n - if: $RUN_SECURITY', + ), + ]) + }) + + test('keeps one shell directory across inherited before_script and script fields', () => { + const file = gitlab('before_script:\n - cd packages/web\naudit:\n script:\n - npm audit') + expectRecognized([file], [manifest('packages/web/package.json')]) + expectMissing([file], [manifest('package.json')]) + + expectRecognized([gitlab('default:\n before_script:\n - npm audit\nbuild:\n script: npm test')]) + }) + + test('does not let a direct command bypass unsupported inheritance in the same job', () => { + expectUnresolved([gitlab('.base:\n script: npm test\naudit:\n extends: .base\n script: npm audit')]) + expectUnresolved([gitlab('before_script: npm audit\nbuild:\n inherit: false\n script: npm test')]) + }) + + test.each([ + '.unused: invalid\nbuild:\n script: npm test', + 'audit:\n when: never\n script: {invalid: true}', + 'audit:\n rules:\n - when: never\n script: {invalid: true}', + 'workflow:\n rules:\n - when: never\nvariables: invalid\naudit: invalid', + 'include:\n template: [invalid]\n rules:\n - when: never', + 'build:\n before_script: {invalid: true}', + ])('does not validate scripts or templates that do not execute: %s', (content) => { + expectMissing([gitlab(content)]) + }) + + test.each([ + 'default:\n before_script: {invalid: true}\naudit:\n before_script: []\n script: npm audit', + 'before_script: {invalid: true}\naudit:\n before_script: []\n script: npm audit', + 'variables:\n EXAMPLE: [npm, audit]\naudit:\n stage: security\n script: npm audit', + 'broken: invalid\naudit:\n script: npm audit', + ])('preserves jobs and evidence when parsing configuration subsets: %s', (content) => { + expectRecognized([gitlab(content)]) + }) + + test('normalizes scalar and array scripts without sharing the after_script directory', () => { + expectRecognized([ + gitlab('before_script: cd packages/web\naudit:\n script: [npm test]\n after_script: npm audit'), + ]) + }) + + test('treats malformed jobs and scripts as unresolved', () => { + expectUnresolved([gitlab('audit: invalid')]) + expectUnresolved([gitlab('audit:\n script:\n command: npm audit')]) + }) +}) + describe('execution boundaries', () => { test.each(['repository: other/project', 'path: other'])('leaves a scoped checkout unresolved (%s)', (input) => { const file = github( @@ -403,6 +501,11 @@ describe('execution boundaries', () => { expectMissing([file]) }) + test('honors disabled jobs inherited through YAML merge keys', () => { + const file = gitlab('.disabled: &disabled\n when: never\naudit:\n <<: *disabled\n script: npm audit') + expectMissing([file]) + }) + test.each([ ['quoted heredoc', "cat <<'EOF'\nnpm audit\nEOF"], ['function definition', 'audit() {\n npm audit\n}'], @@ -429,17 +532,27 @@ describe('execution boundaries', () => { expectMissing([github(` steps:\n - run: ${command}`)]) }, ) + + test('does not crash on unsupported GitLab rule value objects', () => { + const file = gitlab('audit:\n when: {toString: false}\n script: echo done') + expect(() => scan([file])).not.toThrow() + }) }) describe('configuration obstacles', () => { test.each([ ['malformed GitHub workflow', configuration('.github/workflows/ci.yml', 'jobs: [')], + ['malformed GitLab configuration', gitlab('include: [')], [ 'excessive YAML aliases', github( ` x: &values [one, two]\n y: [${Array.from({length: 21}, () => '*values').join(', ')}]\n steps:\n - run: npm test`, ), ], + [ + 'unsupported GitLab YAML reference', + gitlab('.security:\n script: npm audit\naudit:\n script: !reference [.security, script]'), + ], [ 'unknown GitHub reusable workflow', configuration(