diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a669c12ac..ae7c2d12f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -188,6 +188,15 @@ jobs: # "stale" and the run hard-failed on diffs that touched nothing related. # This must stay equal to CI_REBASE_BASE_SHA; a test asserts that parity. GSD_EMITTED_BASE: ${{ github.event.pull_request.base.sha }} + # #4196: pin the npm-audit baseline the SAME way GSD_EMITTED_BASE pins + # its own baseline (see the comment above) -- origin/next is live under + # fetch-depth: 0 and can advance mid-run; base.sha is fixed for the life + # of the run. For a push event, github.event.before is git's own record + # of the ref's state immediately before this push landed -- correct + # even when a rebase-merged PR lands as multiple discrete commits in + # one push (HEAD~1 would be wrong there: it could already contain an + # earlier commit's newly-introduced vulnerable package, masking it). + AUDIT_BASELINE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || (github.event_name == 'push' && github.event.before) || '' }} strategy: fail-fast: false matrix: @@ -419,6 +428,15 @@ jobs: # list that can include the emitted gate, and the invariant is easier to keep # with no exceptions: if a job merges a pinned base, the gate uses that base. GSD_EMITTED_BASE: ${{ github.event.pull_request.base.sha }} + # #4196: pin the npm-audit baseline the SAME way GSD_EMITTED_BASE pins + # its own baseline (see the comment above) -- origin/next is live under + # fetch-depth: 0 and can advance mid-run; base.sha is fixed for the life + # of the run. For a push event, github.event.before is git's own record + # of the ref's state immediately before this push landed -- correct + # even when a rebase-merged PR lands as multiple discrete commits in + # one push (HEAD~1 would be wrong there: it could already contain an + # earlier commit's newly-introduced vulnerable package, masking it). + AUDIT_BASELINE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || (github.event_name == 'push' && github.event.before) || '' }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -513,6 +531,15 @@ jobs: # "stale" and the run hard-failed on diffs that touched nothing related. # This must stay equal to CI_REBASE_BASE_SHA; a test asserts that parity. GSD_EMITTED_BASE: ${{ github.event.pull_request.base.sha }} + # #4196: pin the npm-audit baseline the SAME way GSD_EMITTED_BASE pins + # its own baseline (see the comment above) -- origin/next is live under + # fetch-depth: 0 and can advance mid-run; base.sha is fixed for the life + # of the run. For a push event, github.event.before is git's own record + # of the ref's state immediately before this push landed -- correct + # even when a rebase-merged PR lands as multiple discrete commits in + # one push (HEAD~1 would be wrong there: it could already contain an + # earlier commit's newly-introduced vulnerable package, masking it). + AUDIT_BASELINE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || (github.event_name == 'push' && github.event.before) || '' }} strategy: fail-fast: false matrix: diff --git a/scripts/npm-audit-baseline.cjs b/scripts/npm-audit-baseline.cjs new file mode 100644 index 000000000..324bd0167 --- /dev/null +++ b/scripts/npm-audit-baseline.cjs @@ -0,0 +1,238 @@ +#!/usr/bin/env node +'use strict'; + +/** + * #4196: npm-audit baseline diff. + * + * The #3588 gate (tests/npm-integrity-gate.test.cjs) used to fail on ANY + * advisory present in the production tree, regardless of whether the PR + * being checked introduced it. Because npm's advisory database updates + * continuously and independently of repo state, that made the gate fail + * for reasons no PR caused -- see #4196 for the incident where PR #4188 + * passed this gate at merge time and failed it ~15 minutes later on the + * identical commit, purely because a new advisory was disclosed in the + * interim. + * + * This module computes which vulnerable packages are NEW relative to a + * baseline tree (typically the PR's target branch), so the gate blocks a + * PR only for advisories it actually introduces -- a pre-existing advisory + * in an untouched transitive dependency no longer blocks unrelated work. + */ + +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { execFileSync } = require('node:child_process'); + +const AUDIT_TIMEOUT_MS = 180_000; + +const AUDIT_DIFF_REASON = Object.freeze({ + OK_NO_NEW_VULNERABILITIES: 'ok_no_new_vulnerabilities', + FAIL_NEW_VULNERABLE_PACKAGE: 'fail_new_vulnerable_package', +}); + +/** + * Pure diff: which package names are vulnerable in `headVulnerabilities` + * but were NOT already vulnerable in `baselineVulnerabilities`. + * + * Both args are the `.vulnerabilities` object from `npm audit --json` + * (keyed by package name). Matched by package NAME only -- not by the + * specific advisory ID or severity -- so a package that stays vulnerable + * across a *different* newly-disclosed advisory for the same package is + * still "pre-existing", not new. A package whose vulnerability *worsens* + * while remaining the same package name is deliberately NOT flagged here; + * that tradeoff keeps the predicate simple and matched to the #4196 + * incident shape (a transitive dependency neither side of the diff + * touched). Severity escalation on an already-known-vulnerable package is + * exactly the kind of thing the scheduled Dependabot channel should catch + * instead (see #4196, PR #4200's auto-merge workflow). + */ +function diffNewVulnerablePackages(baselineVulnerabilities, headVulnerabilities) { + const baselineNames = new Set(Object.keys(baselineVulnerabilities || {})); + return Object.keys(headVulnerabilities || {}).filter((name) => !baselineNames.has(name)); +} + +/** + * Typed verdict wrapping diffNewVulnerablePackages. `ok: false` means the + * diff (head vs baseline) is non-empty -- this PR/push introduced at + * least one newly-vulnerable package. + */ +function evaluateAuditDiff({ baselineVulnerabilities, headVulnerabilities }) { + const newlyIntroduced = diffNewVulnerablePackages(baselineVulnerabilities, headVulnerabilities); + if (newlyIntroduced.length > 0) { + return { ok: false, reason: AUDIT_DIFF_REASON.FAIL_NEW_VULNERABLE_PACKAGE, newlyIntroduced }; + } + return { + ok: true, + reason: AUDIT_DIFF_REASON.OK_NO_NEW_VULNERABILITIES, + preExisting: Object.keys(headVulnerabilities || {}), + }; +} + +/** + * Runs `npm audit --package-lock-only --omit=dev --json` in `cwd` and + * returns the parsed JSON, or `null` if `cwd` has no package.json or no + * package-lock.json (not an auditable tree -- callers treat this as + * "skip"/"no baseline available", not an error). + * + * `--package-lock-only` deliberately avoids requiring `node_modules/` to + * be installed: it lets a baseline tree be audited from nothing but an + * extracted package.json + package-lock.json (see extractBaselineTree), + * without a second full `npm ci`. + */ +function runPackageLockAudit(cwd) { + if (!fs.existsSync(path.join(cwd, 'package.json'))) return null; + if (!fs.existsSync(path.join(cwd, 'package-lock.json'))) return null; + const isWindows = process.platform === 'win32'; + const npmCandidates = isWindows ? ['npm.cmd', 'npm'] : ['npm']; + const args = ['audit', '--package-lock-only', '--omit=dev', '--json']; + let out; + let lastErr = null; + for (const npmCmd of npmCandidates) { + try { + out = execFileSync( + npmCmd, + args, + { + cwd, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + timeout: AUDIT_TIMEOUT_MS, + shell: isWindows, + }, + ); + lastErr = null; + break; + } catch (e) { + // `npm audit` exits non-zero when advisories are present; the JSON is + // still on stdout in that case. Recover and let the caller classify. + if (e && typeof e.stdout !== 'undefined' && e.stdout !== undefined && e.stdout !== null) { + out = Buffer.isBuffer(e.stdout) ? e.stdout.toString('utf-8') : String(e.stdout); + lastErr = null; + break; + } + lastErr = e; + } + } + if (lastErr) throw lastErr; + const parsed = JSON.parse(out); + if (parsed && parsed.metadata && parsed.metadata.vulnerabilities) { + return parsed; + } + throw new Error(`Unexpected npm audit JSON shape in ${cwd}: missing metadata.vulnerabilities`); +} + +/** + * Extracts package.json + package-lock.json (optionally under `subdir`, + * e.g. 'sdk') from `ref` at git object level into a fresh temp directory -- + * no working-tree checkout, no `node_modules` install. Returns the temp + * dir path, or `null` if `ref` cannot be resolved locally (e.g. a shallow + * clone that never fetched it) or either file is absent at that ref -- + * callers treat `null` as "no baseline available", not an error. + */ +function extractBaselineTree(ref, repoRoot, subdir = '') { + const rel = (name) => (subdir ? path.posix.join(subdir, name) : name); + let pkgJson; + let lockJson; + try { + pkgJson = execFileSync('git', ['show', `${ref}:${rel('package.json')}`], { + cwd: repoRoot, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'ignore'], + }); + lockJson = execFileSync('git', ['show', `${ref}:${rel('package-lock.json')}`], { + cwd: repoRoot, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'ignore'], + }); + } catch { + return null; + } + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-audit-baseline-')); + fs.writeFileSync(path.join(dir, 'package.json'), pkgJson); + fs.writeFileSync(path.join(dir, 'package-lock.json'), lockJson); + return dir; +} + +// All-zeros is git's documented sentinel for "this ref did not exist +// before this push" (a brand-new branch's first push) -- never a real +// commit to diff against. +const NULL_SHA = '0000000000000000000000000000000000000000'; + +/** + * Resolves the git ref to diff against, in priority order: + * 1. AUDIT_BASELINE_REF env var -- the primary mechanism. CI sets this + * explicitly (see .github/workflows/test.yml) to github.event.pull_ + * request.base.sha on a pull_request event, or github.event.before on + * a push event -- both pinned, race-free values Git/GitHub track for + * exactly this purpose. Prefer this over anything below whenever the + * caller can provide it. + * 2. GITHUB_BASE_REF (GitHub Actions sets this on pull_request events) + * resolved against the LIVE origin/ tip. Only reached if + * AUDIT_BASELINE_REF wasn't set -- e.g. a workflow that forgot to + * wire it. origin/ can advance mid-run (see the GSD_EMITTED_ + * BASE precedent in test.yml), so this is a degraded fallback, not + * the intended path for pull_request events in this repo's own CI. + * 3. On a `push` event, `HEAD~1` -- correct ONLY when the push added + * exactly one commit (true for a squash-merge or a single ordinary + * commit). This repo also allows rebase-merge (allow_rebase_merge: + * true), which can land a PR as several discrete commits in one + * push -- HEAD~1 then lands on an EARLIER commit in the same push, + * which may already contain a vulnerable package that commit itself + * introduced, silently marking it "pre-existing". CI never reaches + * this branch (AUDIT_BASELINE_REF is always set by test.yml for + * push events); it exists only for out-of-band invocations (e.g. + * gsd-test) that don't set any of the above. + * 4. `origin/next`, else a plain local branch named `next` (gsd-test's + * sandbox fetches the base as a local branch, not a remote-tracking + * ref -- see gsd-test-merges-into-LOCAL-base-branch in this repo's + * own operational notes), if either exists locally (this repo's + * integration branch -- matches DEFAULT_BASE in + * scripts/changeset/lint.cjs). + * Returns '' if none resolve -- callers fall back to strict zero-tolerance + * rather than silently skipping the gate. + */ +function resolveBaselineRef(repoRoot) { + if (process.env.AUDIT_BASELINE_REF) return process.env.AUDIT_BASELINE_REF; + if (process.env.GITHUB_BASE_REF) return `origin/${process.env.GITHUB_BASE_REF}`; + if (process.env.GITHUB_EVENT_NAME === 'push') { + try { + const parent = execFileSync('git', ['rev-parse', '--verify', 'HEAD~1'], { + cwd: repoRoot, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'ignore'], + }).trim(); + if (parent && parent !== NULL_SHA) return parent; + } catch { + // not enough history; fall through + } + } + try { + execFileSync('git', ['rev-parse', '--verify', 'origin/next'], { + cwd: repoRoot, + stdio: 'ignore', + }); + return 'origin/next'; + } catch { + // fall through to a plain local branch + } + try { + execFileSync('git', ['rev-parse', '--verify', 'next'], { + cwd: repoRoot, + stdio: 'ignore', + }); + return 'next'; + } catch { + return ''; + } +} + +module.exports = { + AUDIT_DIFF_REASON, + diffNewVulnerablePackages, + evaluateAuditDiff, + runPackageLockAudit, + extractBaselineTree, + resolveBaselineRef, + NULL_SHA, +}; diff --git a/tests/npm-audit-baseline.test.cjs b/tests/npm-audit-baseline.test.cjs new file mode 100644 index 000000000..51328b3d4 --- /dev/null +++ b/tests/npm-audit-baseline.test.cjs @@ -0,0 +1,296 @@ +'use strict'; + +/** + * Unit tests for scripts/npm-audit-baseline.cjs (#4196). + * + * Covers the pure diff/verdict functions directly, plus the git-object-level + * extraction and env-driven ref resolution using real throwaway git fixture + * repos (no network, no real npm registry round-trip -- runPackageLockAudit + * is only exercised here for its filesystem-only skip conditions). + */ + +const { test, describe, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { execFileSync } = require('node:child_process'); + +const { + AUDIT_DIFF_REASON, + diffNewVulnerablePackages, + evaluateAuditDiff, + runPackageLockAudit, + extractBaselineTree, + resolveBaselineRef, + NULL_SHA, +} = require('../scripts/npm-audit-baseline.cjs'); + +const { createTempDir, cleanup } = require('./helpers.cjs'); + +const GIT_TIMEOUT_MS = 30_000; + +function git(args, cwd) { + return execFileSync('git', args, { + cwd, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + timeout: GIT_TIMEOUT_MS, + }).trim(); +} + +/** + * Builds a throwaway git repo with one commit containing package.json + + * package-lock.json (and optionally under a subdir), returning + * { dir, commitSha }. + */ +function makeCommittedFixtureRepo(t, { subdir = '', pkgContent = '{"name":"fixture"}', lockContent = '{"lockfileVersion":3}' } = {}) { + const dir = createTempDir('gsd-audit-baseline-fixture-'); + t.after(() => cleanup(dir)); + git(['init', '-q'], dir); + git(['config', 'user.email', 'test@example.com'], dir); + git(['config', 'user.name', 'Test'], dir); + const targetDir = subdir ? path.join(dir, subdir) : dir; + fs.mkdirSync(targetDir, { recursive: true }); + fs.writeFileSync(path.join(targetDir, 'package.json'), pkgContent); + fs.writeFileSync(path.join(targetDir, 'package-lock.json'), lockContent); + git(['add', '-A'], dir); + git(['commit', '-q', '-m', 'fixture commit'], dir); + const commitSha = git(['rev-parse', 'HEAD'], dir); + return { dir, commitSha }; +} + +// ─── diffNewVulnerablePackages ──────────────────────────────────────────── + +describe('diffNewVulnerablePackages', () => { + test('empty baseline + empty head -> []', () => { + assert.deepStrictEqual(diffNewVulnerablePackages({}, {}), []); + }); + + test('baseline and head share the same packages -> [] (nothing new)', () => { + const baseline = { a: {}, b: {} }; + const head = { a: {}, b: {} }; + assert.deepStrictEqual(diffNewVulnerablePackages(baseline, head), []); + }); + + test('head adds a package not in baseline -> only the addition', () => { + const baseline = { a: {} }; + const head = { a: {}, b: {} }; + assert.deepStrictEqual(diffNewVulnerablePackages(baseline, head), ['b']); + }); + + test('empty baseline, head has one package -> that package', () => { + assert.deepStrictEqual(diffNewVulnerablePackages({}, { a: {} }), ['a']); + }); + + test('packages removed from head (present in baseline only) are not "new"', () => { + const baseline = { a: {}, b: {}, c: {} }; + const head = { a: {} }; + assert.deepStrictEqual(diffNewVulnerablePackages(baseline, head), []); + }); + + test('baseline and head both undefined -> [] (must not throw)', () => { + assert.deepStrictEqual(diffNewVulnerablePackages(undefined, undefined), []); + }); +}); + +// ─── evaluateAuditDiff ───────────────────────────────────────────────────── + +describe('evaluateAuditDiff', () => { + test('no new packages -> ok:true with OK_NO_NEW_VULNERABILITIES and preExisting list', () => { + const baselineVulnerabilities = { a: {} }; + const headVulnerabilities = { a: {} }; + const result = evaluateAuditDiff({ baselineVulnerabilities, headVulnerabilities }); + assert.deepStrictEqual(result, { + ok: true, + reason: AUDIT_DIFF_REASON.OK_NO_NEW_VULNERABILITIES, + preExisting: ['a'], + }); + }); + + test('one new package -> ok:false with FAIL_NEW_VULNERABLE_PACKAGE and exact newlyIntroduced', () => { + const baselineVulnerabilities = { a: {} }; + const headVulnerabilities = { a: {}, b: {} }; + const result = evaluateAuditDiff({ baselineVulnerabilities, headVulnerabilities }); + assert.strictEqual(result.ok, false); + assert.strictEqual(result.reason, AUDIT_DIFF_REASON.FAIL_NEW_VULNERABLE_PACKAGE); + assert.deepStrictEqual(result.newlyIntroduced, ['b']); + }); + + test('multiple new packages -> all listed', () => { + const baselineVulnerabilities = {}; + const headVulnerabilities = { a: {}, b: {}, c: {} }; + const result = evaluateAuditDiff({ baselineVulnerabilities, headVulnerabilities }); + assert.strictEqual(result.ok, false); + assert.deepStrictEqual(result.newlyIntroduced.sort(), ['a', 'b', 'c']); + }); +}); + +// ─── resolveBaselineRef ───────────────────────────────────────────────────── + +describe('resolveBaselineRef', () => { + const envKeys = ['AUDIT_BASELINE_REF', 'GITHUB_BASE_REF', 'GITHUB_EVENT_NAME']; + let originalEnv; + + beforeEach(() => { + originalEnv = Object.fromEntries(envKeys.map((key) => [key, process.env[key]])); + for (const key of envKeys) delete process.env[key]; + }); + + afterEach(() => { + for (const key of envKeys) { + if (originalEnv[key] === undefined) delete process.env[key]; + else process.env[key] = originalEnv[key]; + } + }); + + test('AUDIT_BASELINE_REF set -> returned verbatim, highest priority even with others set', (t) => { + const { dir } = makeCommittedFixtureRepo(t); + process.env.AUDIT_BASELINE_REF = 'some/explicit-ref'; + process.env.GITHUB_BASE_REF = 'next'; + process.env.GITHUB_EVENT_NAME = 'push'; + assert.strictEqual(resolveBaselineRef(dir), 'some/explicit-ref'); + }); + + test('AUDIT_BASELINE_REF unset, GITHUB_BASE_REF=next -> origin/next', (t) => { + const { dir } = makeCommittedFixtureRepo(t); + process.env.GITHUB_BASE_REF = 'next'; + assert.strictEqual(resolveBaselineRef(dir), 'origin/next'); + }); + + test('neither set, push event, HEAD~1 resolves -> returns that parent sha', (t) => { + const dir = createTempDir('gsd-audit-baseline-fixture-'); + t.after(() => cleanup(dir)); + git(['init', '-q'], dir); + git(['config', 'user.email', 'test@example.com'], dir); + git(['config', 'user.name', 'Test'], dir); + fs.writeFileSync(path.join(dir, 'a.txt'), 'first'); + git(['add', '-A'], dir); + git(['commit', '-q', '-m', 'first commit'], dir); + fs.writeFileSync(path.join(dir, 'a.txt'), 'second'); + git(['add', '-A'], dir); + git(['commit', '-q', '-m', 'second commit'], dir); + + // Compute expected parent sha independently, not via resolveBaselineRef. + const expectedParentSha = git(['rev-parse', 'HEAD~1'], dir); + + process.env.GITHUB_EVENT_NAME = 'push'; + assert.strictEqual(resolveBaselineRef(dir), expectedParentSha); + }); + + test('NULL_SHA is the documented all-zeros 40-char sentinel', () => { + assert.strictEqual(NULL_SHA, '0'.repeat(40)); + assert.strictEqual(NULL_SHA.length, 40); + }); + + test('push event but only one commit (HEAD~1 does not exist) falls through without choking', (t) => { + const dir = createTempDir('gsd-audit-baseline-fixture-'); + t.after(() => cleanup(dir)); + git(['init', '-q'], dir); + git(['config', 'user.email', 'test@example.com'], dir); + git(['config', 'user.name', 'Test'], dir); + fs.writeFileSync(path.join(dir, 'a.txt'), 'only'); + git(['add', '-A'], dir); + git(['commit', '-q', '-m', 'only commit'], dir); + + process.env.GITHUB_EVENT_NAME = 'push'; + // No origin/next remote-tracking ref exists in this throwaway repo, so + // this must fall all the way through to ''. + assert.strictEqual(resolveBaselineRef(dir), ''); + }); + + test('no origin/next, but a plain local branch named next exists -> returns "next"', (t) => { + const dir = createTempDir('gsd-audit-baseline-fixture-'); + t.after(() => cleanup(dir)); + git(['init', '-q'], dir); + git(['config', 'user.email', 'test@example.com'], dir); + git(['config', 'user.name', 'Test'], dir); + fs.writeFileSync(path.join(dir, 'a.txt'), 'first'); + git(['add', '-A'], dir); + git(['commit', '-q', '-m', 'first commit'], dir); + // rename the default branch to "next" so it's a plain LOCAL branch, not + // a remote-tracking origin/next ref -- mirrors gsd-test's sandbox shape. + git(['branch', '-M', 'next'], dir); + + process.env.GITHUB_EVENT_NAME = 'push'; + assert.strictEqual(resolveBaselineRef(dir), 'next'); + }); + + test('nothing resolves at all (no env vars, not a git repo) -> returns ""', (t) => { + const dir = createTempDir('gsd-audit-baseline-nongit-'); + t.after(() => cleanup(dir)); + assert.strictEqual(resolveBaselineRef(dir), ''); + }); +}); + +// ─── extractBaselineTree ───────────────────────────────────────────────────── + +describe('extractBaselineTree', () => { + test('extracts package.json + package-lock.json at root from a real commit', (t) => { + const pkgContent = JSON.stringify({ name: 'root-fixture' }); + const lockContent = JSON.stringify({ lockfileVersion: 3, name: 'root-fixture' }); + const { dir, commitSha } = makeCommittedFixtureRepo(t, { pkgContent, lockContent }); + + const extracted = extractBaselineTree(commitSha, dir); + assert.notStrictEqual(extracted, null); + t.after(() => cleanup(extracted)); + + assert.strictEqual(fs.readFileSync(path.join(extracted, 'package.json'), 'utf-8'), pkgContent); + assert.strictEqual(fs.readFileSync(path.join(extracted, 'package-lock.json'), 'utf-8'), lockContent); + }); + + test('extracts package.json + package-lock.json from a subdir', (t) => { + const pkgContent = JSON.stringify({ name: 'sdk-fixture' }); + const lockContent = JSON.stringify({ lockfileVersion: 3, name: 'sdk-fixture' }); + const { dir, commitSha } = makeCommittedFixtureRepo(t, { subdir: 'sdk', pkgContent, lockContent }); + + const extracted = extractBaselineTree(commitSha, dir, 'sdk'); + assert.notStrictEqual(extracted, null); + t.after(() => cleanup(extracted)); + + assert.strictEqual(fs.readFileSync(path.join(extracted, 'package.json'), 'utf-8'), pkgContent); + assert.strictEqual(fs.readFileSync(path.join(extracted, 'package-lock.json'), 'utf-8'), lockContent); + }); + + test('a ref that does not exist -> null', (t) => { + const { dir } = makeCommittedFixtureRepo(t); + const bogusSha = 'f'.repeat(40); + assert.strictEqual(extractBaselineTree(bogusSha, dir), null); + }); + + test('ref exists but package-lock.json was never committed at that ref -> null', (t) => { + const dir = createTempDir('gsd-audit-baseline-nolock-'); + t.after(() => cleanup(dir)); + git(['init', '-q'], dir); + git(['config', 'user.email', 'test@example.com'], dir); + git(['config', 'user.name', 'Test'], dir); + fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"nolock"}'); + git(['add', '-A'], dir); + git(['commit', '-q', '-m', 'no lockfile'], dir); + const sha = git(['rev-parse', 'HEAD'], dir); + + assert.strictEqual(extractBaselineTree(sha, dir), null); + }); +}); + +// ─── runPackageLockAudit (filesystem-only skip conditions, no registry) ──── + +describe('runPackageLockAudit', () => { + test('missing package.json -> null', () => { + const dir = createTempDir('gsd-audit-baseline-empty-'); + try { + assert.strictEqual(runPackageLockAudit(dir), null); + } finally { + cleanup(dir); + } + }); + + test('package.json present but no package-lock.json -> null', () => { + const dir = createTempDir('gsd-audit-baseline-nolock2-'); + try { + fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"nolock2"}'); + assert.strictEqual(runPackageLockAudit(dir), null); + } finally { + cleanup(dir); + } + }); +}); diff --git a/tests/npm-integrity-gate.test.cjs b/tests/npm-integrity-gate.test.cjs index 16f79eb41..9ab0e9df6 100644 --- a/tests/npm-integrity-gate.test.cjs +++ b/tests/npm-integrity-gate.test.cjs @@ -179,14 +179,18 @@ describe('#114: npm integrity gate — --help output', () => { * high or moderate npm-audit advisories. * * Strategy: run `npm audit --omit=dev --json` against both the root - * workspace and the embedded SDK package and assert that the metadata - * vulnerability counts are zero across info/low/moderate/high/critical. + * workspace and the embedded SDK package, then diff the resulting + * vulnerable-package set against a baseline tree (see #4196 and + * scripts/npm-audit-baseline.cjs) so the gate only fails on advisories + * this PR/push actually introduces — not on pre-existing advisories in + * an untouched transitive dependency. When no baseline can be resolved, + * falls back to the original zero-tolerance check across + * info/low/moderate/high/critical. * - * The test is intentionally strict — any advisory of any severity (other - * than 'low' if the maintainer accepts it; that branch is left explicit - * here) blocks CI. If a future advisory lands without an upstream patch, - * either bump the patched transitive (preferred), or annotate the - * acceptance below with a justification AND a link to the upstream tracker. + * If a future advisory lands without an upstream patch on a package this + * PR touches, either bump the patched transitive (preferred), or annotate + * the acceptance below with a justification AND a link to the upstream + * tracker. * * Skips automatically when `node_modules/` is absent (a fresh checkout * before `npm install`) so the test does not falsely report on developer @@ -198,6 +202,13 @@ const assert = require('node:assert/strict'); const path = require('node:path'); const fs = require('node:fs'); const { execFileSync } = require('node:child_process'); +const { + evaluateAuditDiff, + runPackageLockAudit, + extractBaselineTree, + resolveBaselineRef, +} = require('../scripts/npm-audit-baseline.cjs'); +const { cleanup } = require('./helpers.cjs'); const ROOT = path.resolve(__dirname, '..'); const SDK = path.join(ROOT, 'sdk'); @@ -249,37 +260,55 @@ function auditProductionVulns(cwd) { // (npm changed its output format, audit aborted before metadata, etc.) — // throw so the test fails loudly instead of skipping silently. if (parsed && parsed.metadata && parsed.metadata.vulnerabilities) { - return parsed.metadata.vulnerabilities; + return parsed; } throw new Error(`Unexpected npm audit JSON shape in ${cwd}: missing metadata.vulnerabilities`); } -describe('#3588: npm audit --omit=dev reports zero advisories', () => { - test('root workspace production tree has no advisories', { timeout: TEST_TIMEOUT_MS }, (t) => { - const vulns = auditProductionVulns(ROOT); - if (vulns === null) { - t.skip('auditable npm package not present or node_modules/ missing'); +describe('#3588: npm audit --omit=dev introduces no NEW advisories vs baseline (#4196)', () => { + // #4196: a pre-existing advisory in an untouched transitive dependency + // must not block this PR/push -- only an advisory THIS change actually + // introduces should fail the gate. When no baseline can be resolved + // (e.g. a bare local run with no git history), fall back to the + // original #3588 zero-tolerance behavior rather than silently skipping. + function checkTreeAgainstBaseline(t, cwd, subdir, skipMessage) { + const audit = auditProductionVulns(cwd); + if (audit === null) { + t.skip(skipMessage); return; } - assert.strictEqual(vulns.critical, 0, `expected 0 critical; got ${vulns.critical}`); - assert.strictEqual(vulns.high, 0, `expected 0 high; got ${vulns.high}`); - assert.strictEqual(vulns.moderate, 0, `expected 0 moderate; got ${vulns.moderate}`); - // Low advisories are not explicitly forbidden by the #3588 acceptance - // criterion but the issue listed only high/moderate as actual findings — - // tighten if any future low advisory is introduced. - assert.strictEqual(vulns.low, 0, `expected 0 low; got ${vulns.low}`); + const baselineRef = resolveBaselineRef(ROOT); + const baselineDir = baselineRef ? extractBaselineTree(baselineRef, ROOT, subdir) : null; + if (baselineDir === null) { + const vulns = audit.metadata.vulnerabilities; + assert.strictEqual(vulns.critical, 0, `no baseline available; falling back to zero-tolerance -- expected 0 critical; got ${vulns.critical}`); + assert.strictEqual(vulns.high, 0, `no baseline available; falling back to zero-tolerance -- expected 0 high; got ${vulns.high}`); + assert.strictEqual(vulns.moderate, 0, `no baseline available; falling back to zero-tolerance -- expected 0 moderate; got ${vulns.moderate}`); + assert.strictEqual(vulns.low, 0, `no baseline available; falling back to zero-tolerance -- expected 0 low; got ${vulns.low}`); + return; + } + t.after(() => cleanup(baselineDir)); + const baselineAudit = runPackageLockAudit(baselineDir); + const baselineVulns = (baselineAudit && baselineAudit.vulnerabilities) || {}; + const result = evaluateAuditDiff({ + baselineVulnerabilities: baselineVulns, + headVulnerabilities: audit.vulnerabilities || {}, + }); + assert.strictEqual( + result.ok, + true, + result.ok + ? '' + : `new advisory introduced vs baseline (${baselineRef}): ${result.newlyIntroduced.join(', ')}. Pre-existing advisories are tracked separately (see #4196) and do not block this change.`, + ); + } + + test('root workspace production tree introduces no new advisories', { timeout: TEST_TIMEOUT_MS }, (t) => { + checkTreeAgainstBaseline(t, ROOT, '', 'auditable npm package not present or node_modules/ missing'); }); - test('sdk/ production tree has no advisories', { timeout: TEST_TIMEOUT_MS }, (t) => { - const vulns = auditProductionVulns(SDK); - if (vulns === null) { - t.skip('sdk/ is not an auditable npm package or sdk/node_modules/ is missing'); - return; - } - assert.strictEqual(vulns.critical, 0, `expected 0 critical; got ${vulns.critical}`); - assert.strictEqual(vulns.high, 0, `expected 0 high; got ${vulns.high}`); - assert.strictEqual(vulns.moderate, 0, `expected 0 moderate; got ${vulns.moderate}`); - assert.strictEqual(vulns.low, 0, `expected 0 low; got ${vulns.low}`); + test('sdk/ production tree introduces no new advisories', { timeout: TEST_TIMEOUT_MS }, (t) => { + checkTreeAgainstBaseline(t, SDK, 'sdk', 'sdk/ is not an auditable npm package or sdk/node_modules/ is missing'); }); }); });