fix(ci): scope the npm audit gate to production dependencies (#1166)

[itsmiso-ai]

Problem

Security Audit failed twice on main (b0863de9, 34f01623): npm run audit reports 5 high on braces (GHSA-vfj7-8cjw-p6xm) through eslint-config-next → @next/eslint-plugin-next → fast-glob → micromatch → braces. Every lockfile entry in that chain is dev:true. The advisory range is <=3.0.3 and the latest published braces is 3.0.3 — there is no patched release, so no override/upgrade can fix it, and npm audit fix --force's only "fix" is a breaking eslint-config-next downgrade that lands in the same fast-glob chain anyway.

Closes #1166 (and the identical earlier filing #1162).

Root cause (empirically verified, npm 11.19 / node 24, this lockfile)

.npmrc sets include=dev (intentional, #428: npm ci must install devDeps even with a global omit=dev). In npm's config the include set wins over omit, so the gate's --omit=dev was silently ignored and the audit covered the dev tree the whole time. The unfixable dev-only advisory is what exposed it.

Evidence (full-tree runs):

• npm audit --omit=dev → reports the braces chain, exit 1
• npm audit --omit=dev --include=prod → found 0 vulnerabilities, exit 0
• scratch project, braces as a real production dependency (devDeps installed, .npmrc include=dev) → --include=prod still reports it, exit 1
• scratch project, braces as an optionalDependency → --include=prod still reports it, exit 1
• npm ls --omit=dev --include=prod braces → empty

Fix

• package.json audit script: explicit CLI --include=prod replaces the .npmrc include value (same-key CLI precedence), so --omit=dev finally takes effect. Adjacent pair --omit=dev --include=prod is the empirically validated combination.
• package.json "//".audit: rationale comment (matches repo convention for "//".overrides).
• .github/workflows/security-audit.yaml: new non-blocking (continue-on-error: true) dev-inclusive audit step so future dev-only advisories stay visible without turning main red.
• package-audit.test.ts (new, root, mirrors package-overrides.test.ts conventions): regression-guards the flag pair, the "//".audit rationale, and the workflow wiring (blocking gate present, dev pass non-blocking). Deliberately does not pin .npmrc — the gate is robust to it at any level, and CI installs pass --include=dev explicitly via the setup-node action.
• SECURITY-ACCEPTED-RISKS.md: accepts the dev-only braces advisory with precise shipping-surface claims, updates the documented gate command, records the resolution in Previous Resolution History.

No dependency or lockfile changes.

Invariants this change must keep — and how each is kept

1. The blocking gate fails on high/critical in production AND optional dependencies. Kept by the --omit=dev --include=prod pair: optional deps are controlled by omit=optional, not the include set, and stay in scope — verified empirically (a vulnerable optionalDependency is still reported, exit 1; a vulnerable production dep is still reported, exit 1).
2. Dev-only advisory chains must not fail CI. Kept by the same flags (repo audit exits 0 with the dev tree installed); regression-guarded by package-audit.test.ts (flag-pair + wiring assertions) and kept visible by the new non-blocking dev-inclusive step.
3. npm ci in this repo keeps installing devDependencies (the whole point of .npmrc include=dev, chore: fix npmrc dev dependency config #428; lint/typecheck/test depend on it). Kept by leaving .npmrc and .github/actions/setup-node/action.yml (npm ci --include=dev) untouched — the fix changes only the audit invocation, not installs.
4. The server runtime image stays production-only. Kept by leaving the Dockerfile untouched: the prod-deps stage copies only package.json/package-lock.json (no .npmrc), so its npm ci --omit=dev was never affected; the runner stage copies only prod-deps node_modules.
5. Existing overrides remediations (postcss/sharp/deepmerge-ts/mysql2) and their rationale comment stay intact. Kept byte-identical; package-overrides.test.ts passes.
6. The workflow keeps a blocking npm run audit and the dev pass stays advisory. Kept and asserted in package-audit.test.ts (npm run audit present; continue-on-error: true precedes npm audit --include=dev).

Other code paths enforcing the same / adjacent rules (audited)

• .github/actions/setup-node/action.yml (npm ci --include=dev, prisma generate) — install path for every other workflow; untouched; audit flags do not affect installs.
• .npmrc (include=dev) — install-time only; untouched; the audit script is now immune to it (CLI --include replaces the project value for the audit process only).
• Dockerfile deps / prod-deps / mcp stages — untouched; prod-deps's --omit=dev genuinely omits devDeps because the stage has no .npmrc. Note (documented, not claimed away): the separately published -mcp image runs off the deps stage tree (entrypoint tsx, a devDependency), so the dev lint chain is present there as inert tooling — flagged as follow-up, out of scope here.
• package-overrides.test.ts — same file-family conventions; passes unchanged.
• pr-smoke.yaml — consumes setup-node only; unaffected.
• Repo-wide grep for npm audit / omit=dev / include=dev finds no other consumers.

Verification

npm run audit exit 0; lint clean; tsc --noEmit clean; full vitest suite 3789 passed / 16 skipped / 2 todo (165 files), including the 6 new assertions.

Related

#1169 (for the duplicate #1162) independently diagnosed the same root cause and chose --include=optional. It overlaps this diff (audit script + accepted-risks doc), so the two PRs are alternatives, not stackable — whichever merges first, the other will need a rebase or should be closed in favor. This PR additionally adds the regression test, the non-blocking dev-visibility step, and corrects the shipping-surface claim for the -mcp image.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: glm-5.3-flash (openai) — routed smart (risk match: linked_priority_p1) · pr-reviewer-action v3.2.0

Partial coverage: required-check coverage is incomplete — this review did not resolve every required check and must not be read as a complete pass.

Recommendation: Approve. This is a well-evidenced, narrowly scoped CI fix for the P1 Security Audit red-on-main (PR 1166). Every quantified claim in the PR body held under verification against the lockfile, the advisory database, the Dockerfile, and the authoritative CI results for the head commit. No blocking or major issues found.

Claims to Falsify — Results

• Claim 1 (advisory range <=3.0.3, no patched release, audit fix --force only offers a breaking downgrade) — held: 7/7 items. The GitHub advisory API for GHSA-vfj7-8cjw-p6xm confirms vulnerable_version_range: "<= 3.0.3" with first_patched_version: null (severity high, CVSS 7.5). The lockfile pins braces@3.0.3 (package-lock.json:4967), i.e. the latest published version is installed and no override/upgrade can clear it. The issue PR 1166 log shows npm audit fix --force would install eslint-config-next@14.2.35 (a breaking downgrade), and eslint-config-next is a devDependency (package.json devDependencies), so the "fix" stays inside the same dev-only chain. All cited SECURITY-ACCEPTED-RISKS.md lines, package.json:67, and the test's packageJsonPath were verified in the diff/head files.
• Claim 2 (.npmrc sets include=dev, intentional per PR 428) — held on all verifiable items. Direct reads of .npmrc were blocked by the review tooling as a sensitive path, but git_grep for include=dev|omit=dev matched two lines inside .npmrc (content redacted by the harness), repository history shows d6d4fb5 chore: fix npmrc dev dependency config (#428), .github/actions/setup-node/action.yml:17 runs npm ci --include=dev explicitly, and the before/after CI behavior (below) corroborates the include-overrides-omit mechanism. Workflow lines 18/26, the SECURITY-ACCEPTED-RISKS.md references, the test's readJson/audit/workflow assertions, and package.json:67 all verified.
• Claim 3 (the unfixable advisory is dev-only, which is what exposed the latent gate bug) — held: 5/5 items. Every lockfile entry in the chain is marked "dev": true: braces@3.0.3 (:4967), micromatch@4.0.8 (:8507), fast-glob@3.3.1 (:6687), @next/eslint-plugin-next@16.3.8 (:2063), eslint-config-next@16.3.8 (:6135). The failing command in the issue log was npm audit --omit=dev reporting the dev chain — direct evidence the omit flag was being cancelled.
• Claim 4 (new non-blocking dev-inclusive audit step; blocking gate retained) — held: 12/12 items. The head .github/workflows/security-audit.yaml retains - run: npm run audit as a blocking step (line 20) and adds the dev-inclusive pass with continue-on-error: true (lines 24–26). The workflow triggers on push and PR to main. A repo-wide grep for npm run audit|run: npm audit found no other callers, so no sibling call site was missed.
• Claim 5 (accepted-risks doc updated with precise shipping-surface claims) — held: 9/9 items. The new Dev-Only Advisories section, the updated gate command in the Mitigation line, and the new Previous Resolution History row are all present. The shipping-surface claims check out against the actual Dockerfile: the prod-deps stage runs npm ci --omit=dev and copies only package.json/package-lock.json (no .npmrc), and the runner stage copies only prod-deps node_modules — so the main server image is prod-only. The -mcp caveat is also accurate: the mcp stage copies the deps stage tree (npm ci, dev included) and image.yaml builds target: mcp as a separate image; a grep of src/ found no micromatch/braces/fast-glob usage, supporting the "inert tooling, never invoked" claim.

Overall: 5/5 claims held; no counterexample found. The strongest end-to-end evidence is the authoritative CI status for head f11da3d: the npm audit check succeeded with the dev tree installed (.npmrc include=dev + npm ci --include=dev in setup-node), which empirically confirms the --include=prod CLI flag cancels the .npmrc include value and lets --omit=dev take effect on this exact lockfile — the same configuration that failed twice on main before the fix.

Change-by-Change Findings

• package.json — the audit script gains --include=prod between --omit=dev and --audit-level=high; the "//".audit rationale follows the existing "//".overrides convention (mirrored assertion style in the new test). No dependency or lockfile changes, as claimed.
• .github/workflows/security-audit.yaml — blocking gate unchanged in behavior except for the scoping fix delivered via the script; the new informational dev-inclusive step is correctly continue-on-error: true and reuses the same fetch-retry flags. (Pre-existing, unchanged: the workflow runs plain npm ci after the setup-node composite action already ran npm ci --include=dev — a redundant second install, but harmless and out of scope for this PR.)
• package-audit.test.ts — new root-level test mirroring package-overrides.test.ts conventions (@vitest-environment node, process.cwd() root resolution, readJson helper). It guards the flag pair, the "//".audit rationale, and the workflow wiring (blocking gate present, dev pass non-blocking). Deliberately not pinning .npmrc is documented and reasonable given the gate is now robust to .npmrc at any level and CI installs pass --include=dev explicitly.
• SECURITY-ACCEPTED-RISKS.md — accurate and precise; notably honest about the -mcp image carrying the dev chain as inert tooling.

Standards Compliance

• Lint/typecheck must pass (AGENTS.md): satisfied — the authoritative CI results for head f11da3d show Lint, Typecheck, Tests, and Build all succeeded.
• "//" rationale convention: the new "//".audit entry matches the established "//".overrides pattern and is regression-guarded.
• No secrets committed; no .env/build output: the diff touches only CI config, docs, package metadata, and a config-reading test. No token handling is introduced.
• Trivy advisory-only continue-on-error precedent (AGENTS.md): the new non-blocking dev-audit step is consistent with the repo's existing pattern of advisory-only security visibility steps.

Linked Issue Fit

Issue PR 1166 (priority/p1, type/bug) reports Security Audit failing repeatedly on main because npm run audit (npm audit --omit=dev ...) reports 5 high on the dev-only braces chain. The PR addresses exactly this: it makes --omit=dev effective (root cause: .npmrc include=dev winning over omit), keeps dev-only advisories visible via a non-blocking pass, documents the acceptance, and adds regression tests. The issue's evidence (failing command, chain, npm audit fix --force suggestion) matches the PR's root-cause analysis line for line. The PR body also references the duplicate filing PR 1162; I could not fetch it (GitHub API transport unavailable in this environment), but it does not affect the fix's correctness. No acceptance criteria in the issue are left unmet.

Tool Harness Findings

29 tool calls ran. Key results: head security-audit.yaml, setup-node/action.yml, package.json, Dockerfile, package-overrides.test.ts, and vitest.config.ts were read successfully and match the PR's descriptions; the GitHub advisory API confirmed the <=3.0.3 range with no patched version; lockfile spot-checks confirmed all five chain entries are dev: true; a src/ grep found no production use of the vulnerable chain; image.yaml confirmed the runner and mcp image targets. Failures: .npmrc reads/greps were blocked or redacted as a sensitive path (grep still matched include|omit patterns on two lines), gh_api endpoints returned "platform transport not configured" (issue PR 1162, npm config docs), and a raw.githubusercontent fetch was redirect-blocked. None of these failures undermines the verdict; see Unknowns.

Unknowns or Needs Verification

• .npmrc exact content could not be read directly (tooling blocks it as sensitive). The include-overrides-omit mechanism is nonetheless corroborated three ways: grep pattern matches inside the file, the PR 428 history, and — decisively — the before/after CI behavior on this lockfile (failing --omit=dev run in the issue log vs. passing npm audit check on head with the new flag pair).
• npm's documented include/omit precedence could not be fetched from npm docs (transport unavailable). The empirical CI result on the head commit is sufficient evidence for this repository's configuration; no action needed.
• The PR body's scratch-project experiments (prod-installed and optionalDependency braces still reported under --include=prod) are outside what this corpus can reproduce; they are consistent with the observed CI outcome and the documented npm semantics, and the invariant they protect (prod + optional stay in scope) is the status quo the diff does not weaken.

Requirement trace

3 of 3 requirement(s) not fully traced to enforcement and a test:

• req-e589bfe1ad7c — unverifiable (no valid enforcement location)
• req-ad902b2a08da — unverifiable (no valid enforcement location)
• req-7c2ada56c159 — unverifiable (no valid enforcement location)

Approval withheld: this review's coverage is incomplete — required-check coverage or the tool-loop investigation did not finish, so it is publishing as an advisory comment rather than an approval.