From 5950a659ff800f2bfa90e466ed18f60e8c8a7658 Mon Sep 17 00:00:00 2001 From: goetchstone Date: Fri, 19 Jun 2026 16:14:37 -0400 Subject: [PATCH] ci: allowlist triaged no-fix npm audit advisories next-auth pins nodemailer 7 (no fix), so --audit-level=high left the gate permanently red. Run npm audit through scripts/audit-check.mjs, which allowlists the triaged-unreachable GHSAs but still fails on any new high/critical. --- .github/workflows/ci.yml | 11 +++++-- CLAUDE.md | 8 ++--- docs/DECISIONS.md | 10 ++++++ scripts/audit-check.mjs | 71 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 8 deletions(-) create mode 100644 scripts/audit-check.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3a79109..55ce8d8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,9 +45,14 @@ jobs: cache: npm # Production deps only — dev tooling CVEs add noise without changing - # production attack surface. Re-evaluate if this rule starts hiding real issues. - - name: npm audit (prod, high+) - run: npm audit --audit-level=high --omit=dev + # production attack surface. The allowlist in scripts/audit-check.mjs covers + # triaged, no-fix advisories (nodemailer is pinned to 7 by next-auth) so the + # gate stays meaningful instead of permanently red — it still fails on any + # NEW high/critical advisory. + - name: npm audit (prod, high/critical, triaged allowlist) + run: | + npm audit --json --omit=dev > npm-audit.json || true + node scripts/audit-check.mjs npm-audit.json test: name: Lint, type-check, test diff --git a/CLAUDE.md b/CLAUDE.md index cd73076..ea9d19e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -170,11 +170,9 @@ This is a combined RMS (Resource Management System) backend + marketing site for - **Rate limiting is in-memory:** `server/rate-limit.ts` uses a Map — resets on server restart, doesn't work across multiple instances. Acceptable for single-instance VPS deployment. - **No email queue retry backoff:** `app/api/cron/process-emails/route.ts` retries failed emails up to 3 times with no delay between attempts — should add exponential backoff if email failures become common - **Portal token is permanent:** Client portal tokens (cuid) never expire and can't be rotated without DB manual intervention — add rotation mechanism before handling sensitive client data -- **Accepted-risk dependency vulns:** Three medium-severity Dependabot alerts remain open and are deliberately not patched: - - **nodemailer GHSA-c7w3-x93f-qmm8** (SMTP command injection via `envelope.size`) — we never set `envelope.size`; vulnerable code path not reached. - - **nodemailer GHSA-vvjj-xcjg-gr5g** (CRLF in transport `name` option for EHLO/HELO) — we never set transport `name`; nodemailer uses system hostname automatically. Vulnerable code path not reached. - - Both nodemailer alerts: a fix requires bumping past 7.x, but next-auth's `@auth/core` peer-dep pins us to nodemailer 7. Revisit when next-auth 5 stable supports nodemailer 8+. - - **postcss GHSA-qx2v-qp2m-jg93** (XSS in CSS stringify) — transitive via Next.js's bundled postcss. Direct fix requires downgrading Next to 9.x (years-old). Will resolve when Next.js bumps its bundled postcss. Not exploitable through our application surface (we don't pass untrusted input to postcss stringify). +- **Accepted-risk dependency vulns (CI-allowlisted):** Several transitive advisories are open with **no fix available** and are deliberately allowlisted in `scripts/audit-check.mjs` — the `npm audit` CI gate still fails on any *new* high/critical advisory, just not these. Re-triage if `EmailPayload` or the SMTP transport options in `server/email/index.ts` ever widen. + - **nodemailer — 6 advisories (incl. high-severity):** GHSA-c7w3-x93f-qmm8 (`envelope.size`), GHSA-vvjj-xcjg-gr5g (transport `name`), GHSA-268h-hp4c-crq3 (`List-*` header CRLF), GHSA-wqvq-jvpq-h66f (`jsonTransport` bypass), GHSA-r7g4-qg5f-qqm2 (OAuth2 token TLS), GHSA-p6gq-j5cr-w38f (`raw` option file-read/SSRF). All unreachable: `sendEmail()` accepts only the `EmailPayload` allowlist (`from/to/subject/html/text/replyTo`) and the transport sets only `host/port/secure/auth{user,pass}` — so we never set `envelope`, transport `name`, `List-*` headers, `raw`, `jsonTransport`, or OAuth2 auth. Fix needs nodemailer 8+, but next-auth's `@auth/core` peer-dep pins us to 7. Revisit when next-auth 5 stable supports nodemailer 8+. + - **postcss GHSA-qx2v-qp2m-jg93** (moderate, XSS in CSS stringify) — transitive via Next.js's bundled postcss; not exploitable through our surface (no untrusted input to postcss stringify). Resolves when Next bumps its bundled postcss. ### Testing & Deployment diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index a4d35ef..7553616 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -170,3 +170,13 @@ Append-only log. Each entry: date, decision, why, and what alternatives were con The AI framework is the cleaner reference implementation of the pattern. **Workflow change:** direct pushes to `main` will be blocked; future work uses PRs. Hotfixes can use admin-bypass when truly time-critical. + +--- + +### 2026-06-19: npm-audit CI gate uses a per-advisory allowlist, not a lowered threshold + +**Decision:** The `Dependency audit (npm)` job runs `npm audit --json --omit=dev` through `scripts/audit-check.mjs`, which fails on any high/critical advisory EXCEPT a small allowlist of triaged, no-fix-available GHSAs (currently 6 nodemailer + 1 postcss). + +**Why:** A cluster of nodemailer advisories (one high) has no fix — next-auth's `@auth/core` peer-dep pins nodemailer to 7 — so `--audit-level=high` left the gate permanently red, blocking every PR. Each was triaged unreachable against the actual code (`server/email/index.ts` exposes only the `EmailPayload` allowlist; the transport sets no `envelope`/`name`/`raw`/`jsonTransport`/OAuth2). Allowlisting the specific GHSAs keeps the gate meaningful — a new, unrelated high/critical still fails the build. + +**Alternatives considered:** lower to `--audit-level=critical` (would silently ignore future *high* vulns — too loose); `|| true` on the step (disables the gate); add `audit-ci`/`better-npm-audit` (a new dependency for what ~40 lines of zero-dep JS does). The allowlist must shrink as fixes land — revisit when next-auth 5 supports nodemailer 8+. diff --git a/scripts/audit-check.mjs b/scripts/audit-check.mjs new file mode 100644 index 0000000..31fa447 --- /dev/null +++ b/scripts/audit-check.mjs @@ -0,0 +1,71 @@ +// scripts/audit-check.mjs +// CI gate over `npm audit` for production deps. Fails on any high/critical +// advisory EXCEPT a small allowlist of triaged, no-fix-available advisories +// (documented in CLAUDE.md "Known Issues"). This keeps the gate meaningful — a +// new, unrelated high/critical vuln still fails the build — without it being +// permanently red over transitive advisories we cannot patch (nodemailer is +// pinned to 7 by next-auth's @auth/core peer dependency). +// +// Usage (see .github/workflows/ci.yml): +// npm audit --json --omit=dev > npm-audit.json || true +// node scripts/audit-check.mjs npm-audit.json + +import { readFileSync } from "node:fs"; + +// Each entry is accepted because the vulnerable code path is unreachable given +// how server/email/index.ts uses nodemailer: sendEmail() accepts only the +// EmailPayload allowlist (from/to/subject/html/text/replyTo) and the transport +// sets only host/port/secure/auth{user,pass}. Re-triage if EmailPayload or the +// transport options ever widen. +const ALLOW = new Map([ + ["GHSA-c7w3-x93f-qmm8", "nodemailer envelope.size injection — we never set envelope"], + ["GHSA-vvjj-xcjg-gr5g", "nodemailer transport name CRLF — we never set name"], + ["GHSA-268h-hp4c-crq3", "nodemailer List-* header CRLF — we never set headers/list"], + ["GHSA-wqvq-jvpq-h66f", "nodemailer jsonTransport bypass — SMTP transport only"], + ["GHSA-r7g4-qg5f-qqm2", "nodemailer OAuth2 TLS — we use auth:{user,pass}, not OAuth2"], + ["GHSA-p6gq-j5cr-w38f", "nodemailer raw option file-read/SSRF — not in EmailPayload"], + ["GHSA-qx2v-qp2m-jg93", "postcss stringify XSS — transitive via Next; no untrusted input"], +]); + +const BLOCK = new Set(["high", "critical"]); + +const file = process.argv[2]; +if (!file) { + console.error("usage: node scripts/audit-check.mjs "); + process.exit(2); +} + +let report; +try { + report = JSON.parse(readFileSync(file, "utf8")); +} catch (e) { + console.error(`audit-check: could not read/parse ${file}: ${e.message}`); + process.exit(2); +} + +const ghsaFromUrl = (url) => (typeof url === "string" ? url.split("/").pop() : ""); + +const offenders = new Set(); +const accepted = new Set(); +for (const v of Object.values(report.vulnerabilities ?? {})) { + for (const via of v.via ?? []) { + if (typeof via !== "object" || via === null) continue; // string = dep path, not an advisory + if (!BLOCK.has(via.severity)) continue; + const ghsa = ghsaFromUrl(via.url); + if (ALLOW.has(ghsa)) accepted.add(`${ghsa} (${via.severity})`); + else offenders.add(`${(via.severity || "?").toUpperCase()} ${via.title ?? ghsa ?? "unknown"} ${via.url ?? ""}`); + } +} + +if (accepted.size) { + console.log(`audit-check: ignoring ${accepted.size} allowlisted high/critical advisory(ies): ${[...accepted].join(", ")}`); +} + +if (offenders.size) { + console.error(`audit-check: ${offenders.size} high/critical advisory(ies) NOT on the allowlist:`); + for (const o of offenders) console.error(" " + o); + console.error("Triage, then either fix or add to the allowlist in scripts/audit-check.mjs with a documented reason."); + process.exit(1); +} + +console.log("audit-check: no unallowlisted high/critical advisories. OK.");