[Security 8/9] Supply-chain hygiene: CVE patches, audit ratchet, Dependabot

[amal66]

[Security 8/9] Supply-chain hygiene — CVE patches, an audit ratchet, and Dependabot

Part of the split of #227 into single-topic PRs. Index: tracking comment on #227.

TL;DR

Patch known-vulnerable transitive dependencies via scoped overrides where an in-range fix exists (tmp ≥0.2.6, form-data 4.0.6), clear what npm audit fix can clear semver-compatibly (no --force), add a blocking CI audit gate for every directory that owns a lockfile (root, backend, frontend) with an explicit, documented GHSA allowlist — currently down to one entry — for advisories that have no in-range fix anywhere, and add Dependabot so the fix for a newly-published CVE arrives automatically as a bot PR. Lockfiles were regenerated on current main, never hand-merged.

Risk to user data

Severity: medium (systemic). Most of the code we ship is dependencies, and they go bad two ways: honest packages with disclosed CVEs, and outright compromised packages. Both run with our server's privileges — a vulnerable or malicious dependency in the request path can read documents, leak secrets, or execute code. Advisories fixed in-range here: tmp symlink traversal, form-data predictable multipart boundaries, ip-address SSRF-bypass (via express-rate-limit), nanoid infinite-loop DoS, undici moderates, plus highs in @xmldom/xmldom, protobufjs, ws, and others.

Flows affected

• Build / dependency resolution (root, backend, and frontend — every directory with its own package-lock.json).
• CI (.github/workflows/security.yml) — new blocking gate for all three trees, actions SHA-pinned per the [Security 19] security posture: SECURITY.md, CodeQL, secret scan, Scorecard #246 posture.
• Frontend app code: adopting the stricter linter the upgraded floor pulls in (details below) — behavior-preserving restructurings, no feature change.

Attack precedent

• event-stream (2018) — a popular package taken over and injected with wallet-stealing code. (writeup)
• XZ Utils backdoor (2024) — a multi-year social-engineering supply-chain attack. (overview)
• Vulnerable/compromised components is OWASP A06:2021.

Possible fixes, and what we chose

Option	Verdict
npm audit fix --force	Rejected — swaps in different major versions untested (here it would downgrade next to v9). Never in CI.
One-time npm audit fix and move on	Clears today's advisories but the state rots the moment a new CVE lands. Necessary but not sufficient.
Global overrides forcing versions outside a consumer's declared range	Rejected after review (thanks @willchen96) — an earlier revision forced sharp 0.35.x and postcss 8.5.x under a Next that declared sharp ^0.34.5 and pinned postcss 8.4.31 exactly. That ships combinations upstream never tested to make a scanner green. Overrides are only used where the forced version is inside the consumer's declared range (tmp, form-data).
Report-only (continue-on-error) frontend gate	Rejected — it tolerates every future advisory silently, not just the known one.
Scoped in-range overrides + semver-safe audit fix + a blocking gate with a documented GHSA allowlist + Dependabot	Chosen. Advisories with no in-range fix anywhere get an allowlist entry carrying the reason, the actual exposure, and an explicit REMOVE WHEN exit condition — printed by the gate on every run so they stay visible. Everything else blocks.

flowchart LR
    CVE["new advisory published<br/>(GitHub Advisory DB)"] --> Gate["CI audit gate goes red<br/>scripts/audit-gate.mjs"]
    CVE --> Bot["Dependabot opens a<br/>version-bump PR"]
    Bot --> CI["full CI runs on the bot PR:<br/>audit + tests + lint"]
    CI --> Merge["human reviews & merges"]
    Merge --> Green["gate green again<br/>(and allowlist entries retire)"]

Loading

Why the gate is a ratchet: scripts/audit-gate.mjs re-audits straight from each lockfile on every push (seconds, no install), so a newly published advisory fails the build even with zero code change — that fired three times during this PR's own lifetime: the July brace-expansion wave (absorbed in-range, its allowlist entry retired), the early-August ip-address/undici wave (absorbed in-range), and the mid-August nanoid wave (GHSA-28wg-ghj8-5hjv, GHSA-2v37-7h3g-55p8 — absorbed in-range: docx declares nanoid ^5.1.3, postcss declares ^3.3.16, so the patched 5.1.16/3.3.18 land without leaving any consumer's tested range). The gate fails closed: if npm audit itself errors (registry outage), the job fails rather than reporting a false green.

The allowlist is now down to 1 entry (scripts/audit-allowlist.json):

• undici GHSA-4cwx-7wf7-3272 — dev-only, via miniflare's exact undici 7.28.0 pin. Remove when wrangler ships miniflare with undici ≥7.29.0.

The expiry mechanism has now demonstrated itself three times: the brace-expansion entry retired when the fix propagated in-range, the sharp entry retired when Next 16.3.0 began declaring sharp ^0.35.3, and both postcss entries retired when Next 16.3.0 moved its exact pin to postcss 8.5.23 — each removal is a commit that quotes the entry's own REMOVE WHEN condition coming true.

Adopting the linter the floor pulled in: regenerating the frontend lockfile moved eslint-plugin-react-hooks in-range from 7.0.1 to 7.1.1, whose new React-compiler-powered rules (set-state-in-effect, refs, preserve-manual-memoization, purity) flagged 70 errors across 37 files — and CI lints as a blocking gate. Suppressing the rules would discard exactly the signal the upgrade bought, so every error is fixed by restructuring (React-docs "adjust state during render" pattern for effect-based resets, ref access moved out of render, memo dependency arrays made verifiable), with a single documented eslint-disable where an effect genuinely coordinates committed DOM. All restructurings are behavior-preserving; the test suites and a production build back that claim.

Lockfile discipline: all lockfiles were regenerated with npm install --package-lock-only on the current base — never hand-edited or hand-merged. (Manually reconciling a lockfile can silently corrupt the dependency tree; regeneration from the manifest is the only safe path.)

Verified locally on the rebased branch: backend 550 tests + build pass; frontend lint 0 errors, 280 tests + coverage ratchet pass, and a full next build production build succeeds on the upgraded tree (Next 16.3.0 with the exact sharp/postcss versions it declares). All three audit gates exit 0.

What's in this PR

• backend/package.json, frontend/package.json — scoped, in-range overrides only.
• Both workspace package-lock.json files — regenerated on current main.
• scripts/audit-gate.mjs + scripts/audit-allowlist.json — the blocking gate and its documented exceptions (now a single entry).
• .github/workflows/security.yml — the audit gate for root, backend, and frontend, actions pinned to commit SHAs.
• .github/dependabot.yml — grouped CVE-fix PRs + weekly dev-dep bumps for all three npm trees and GitHub Actions.
• frontend/src/** — the eslint-plugin-react-hooks 7.1 adoption described above.

One repo setting must be flipped after merge (config files can't enable it): Settings → Advanced Security → "Dependabot security updates" → Enable (free for public repos). Without it this file still delivers weekly bumps, but the within-hours CVE-fix PRs are the point.

📚 Primer: package handling from first principles

New to lockfiles, semver, overrides, or audits? This section builds up every concept this PR relies on, from zero.

1. The dependency tree: you ship code you didn't choose

When frontend/package.json says "next": "^16.2.6", we chose Next. But Next has its own package.json declaring its dependencies (postcss, sharp, …), and those have theirs. The app ships this entire tree — roughly 1,500 packages of which we picked about 60. The ones we named are direct dependencies; everything they drag in are transitive dependencies. The asymmetry that drives this whole PR: we control our own declarations, but Next's declarations are Next's — written and tested by the Next team, frozen into each Next release.

2. Semver: what a version range promises

A version 8.4.31 is major.minor.patch. The semantic-versioning convention: patch = bug fixes only, minor = backward-compatible additions, major = may break you. Ranges express trust in that convention:

Declaration	Meaning
"postcss": "8.4.31"	exact pin — this version, nothing else
"postcss": "^8.4.31"	caret — anything >=8.4.31 <9.0.0
"sharp": "^0.34.5"	caret on a 0.x version — see below

The 0.x special rule. Below 1.0.0, semver says the API is unstable and anything may break, so by convention the minor position plays the role the major position normally plays. npm's caret encodes this with one rule: allow changes that don't touch the leftmost non-zero digit. So ^1.2.3 allows up to <2.0.0, but ^0.34.5 allows only >=0.34.5 <0.35.0 — sharp 0.34.5 → 0.35.3 looks like a tiny step, but it is a major upgrade in disguise (sharp 0.35 ships documented breaking changes). This single fact is why the earlier revision of this PR was wrong to force sharp 0.35 under a Next that declared ^0.34.5. (Next 16.3.0 has since moved its own declaration to sharp ^0.35.3 — the upgrade arrived the legitimate way, tested upstream, and it is what retired the sharp allowlist entry.)

3. package.json vs package-lock.json: the question and the answer

package.json declares ranges — many versions would satisfy it. package-lock.json records one concrete resolution: exactly which version of every package in the whole tree, with a hash of each, so every developer and CI run installs an identical tree. The lockfile must always be derivable from the manifests — which is why this PR regenerates lockfiles with npm install --package-lock-only and never hand-edits them (hand-merging a lockfile can silently corrupt the tree while looking fine in a diff).

4. Why a package pins exactly

Next 16.2.x declared "postcss": "8.4.31" — no range at all. That is the Next team saying: we compiled, tested, and shipped our CSS pipeline against this precise version; no substitutions. A declaration like this is a tested-compatibility statement. Nothing mechanically stops you from violating it — but everything downstream of violating it is your problem, not upstream's. (16.3.0 moved the pin to 8.5.23, which is how both postcss allowlist entries retired.)

5. Overrides: one tool, two opposite uses

npm's overrides field tells the resolver: "wherever anything in the tree asks for package X, hand it version Y instead — regardless of what was asked for."

• Legitimate use (this PR's tmp and form-data entries): a patched version exists inside the range consumers already declared. Some link in the release chain is just slow to re-lock. The override accelerates delivery of a combination upstream already tests. Runtime risk ≈ zero.
• Misuse (this PR's earlier sharp/postcss overrides, since removed): forcing a version outside a consumer's declared range. That fabricates a combination upstream never tested — for sharp, a native C image library (libvips bindings) running under next/image in production. It makes the scanner green by increasing runtime risk, which inverts the point of the scanner.

Same JSON key, opposite relationship to upstream's testing. The difference is not degree but kind.

6. Advisories and npm audit: set intersection, not code analysis

When a vulnerability is disclosed it gets a GHSA id in the GitHub Advisory Database, recording the package, the vulnerable version range, and a severity. npm audit intersects our lockfile with that database: "does any resolved version fall inside any advisory's vulnerable range?" No code runs, no install needed — which is why the gate costs seconds and why it can go red with zero code change: the database moved, not our tree. (It fired three times during this PR's own lifetime: the July brace-expansion wave, the early-August ip-address/undici wave, and the mid-August nanoid wave.)

This also explains the perverse incentive the earlier revision fell into: audit only compares versions. Force sharp to 0.35.3 and the intersection is empty — green — even though what actually happened is that runtime risk went up.

7. Why overrides or allowlists at all: the three honest decisions

A blocking gate forces every advisory to a decision, and there are only three honest ones: fix the version, accept the risk, or stop shipping. Nobody chooses the third, so the real machinery is:

• Fix the version when the polite path can't do it yet → an in-range override (the fix exists; the release chain hasn't propagated it).
• Accept the risk when no acceptable fix exists anywhere → an allowlist entry (as was true for sharp and postcss until Next 16.3.0, and remains true for undici-under-miniflare today).

Everything else people do — npm audit fix --force, report-only jobs, deleting the audit step, out-of-range overrides — is one of those two decisions made invisibly, where nobody can review, challenge, or undo it.

8. Why blocking + allowlist beats report-only

Scenario	report-only job	blocking + allowlist
Known advisory with no fix (undici-under-miniflare)	tolerated	tolerated — via one documented entry
Brand-new critical published tomorrow	silently tolerated	fails the build

Report-only doesn't tolerate one advisory; it tolerates every future advisory, silently. A gate that never fires is not a gate. The allowlist keeps the gate armed and makes each exception narrow (one GHSA id), visible (printed with its reason on every run, nagged when stale), and expiring (every entry carries a REMOVE WHEN condition naming the upstream event that retires it). Changing accepted risk requires a reviewed commit to scripts/audit-allowlist.json — a version-controlled risk register.

9. Vulnerable ≠ exploitable: reading the allowlist reasons

Each entry states the actual exposure, because a vulnerable version only matters if the vulnerable path is reachable: the undici flaw lives only under miniflare, wrangler's local dev simulator — a devDependency that is never part of the deployed artifact. (The retired postcss entries argued the same way: Next's pinned copy only processed this repo's own CSS at build time.) This reasoning is what a reviewer should challenge in the entry — that's why it's written down.

10. Why the production build is the load-bearing evidence

Declarations, lockfiles, and audit results are all static facts. The only thing that tests the tree as an integrated artifact is building it: next build compiles every route, type-checks, and exercises the CSS pipeline and image tooling against the real resolved versions. In review, an untested claim and a false claim are indistinguishable — so the claim "what Next declares actually works in this app" is backed by a green production build in CI, not asserted.

One-sentence summary: we stopped lying to the resolver about what versions Next can handle, told the truth in a documented, expiring risk register instead, and then proved the truthful configuration actually builds. Every mechanism here — semver ranges, lockfiles, audit, allowlist, build — is a different layer of making claims checkable by someone else.

Reading

npm audit docs · GitHub Advisory Database · OWASP A06:2021

---

_{Revision note: an earlier version of this description said the frontend gate was report-only, and an earlier revision of the branch replaced that with global sharp/postcss overrides without updating this text. Both are gone: the gate blocks everywhere, out-of-range overrides were removed in favor of the documented allowlist. The 2026-08-12 re-review round then rebased onto current main, absorbed the nanoid wave in-range, retired the sharp and both postcss allowlist entries (their exit conditions came true with Next 16.3.0), extended the gate and Dependabot to the repo-root lockfile, and adopted the eslint-plugin-react-hooks 7.1 rules the upgraded floor pulled in. This description matches the implementation as of that round.}

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[willchen96]

I do not think this should be merged as-is. The audit gate and Dependabot configuration are useful, and both audit jobs currently pass, but a few issues should be resolved first:

• The frontend CI check is red. The coverage failure also existed on the PR's old base, so it may not have been introduced here, but it caused lint and the production build to be skipped. Current main is green; please rebase onto current main, regenerate the lockfiles, and rerun all checks.
• The global sharp override resolves to 0.35.3, while Next 16.2.12 declares sharp compatibility as ^0.34.5. Sharp 0.35 includes documented breaking changes, so this forces Next outside its supported range without a successful production-build check demonstrating compatibility. Please remove the global override, upgrade to a Next version that supports Sharp 0.35, or explicitly demonstrate and justify compatibility. A temporary, documented GHSA allowlist is safer than silently overriding Next's declared range.
• The PR still requires review.
• The PR description says the frontend audit is report-only, but the current workflow makes both audit jobs blocking. Please update the description to match the implementation.

Once the branch is rebased, all CI—including the frontend production build—is green, and the Sharp compatibility issue is resolved, I would be comfortable merging the supply-chain automation.

[amal66]

@willchen96 Thanks for the review, addressed all points 1,2,4

1. Rebased and regenerated lockfiles since the PR had drifted. All checks now run green.
2. Removed the sharp override and global post css override so we just resolve the lockfile to what Next has declared and tested
3. Your review is needed?
4. Updated PR description to describe accurately how we block both workspaces with an allowlist.

[willchen96]

Some issues:

Resolve the 60 frontend lint errors introduced by the dependency upgrade.

Remove the two obsolete PostCSS entries from the vulnerability allowlist.

Add the root npm dependencies to the audit workflow and Dependabot.

Update the PR description to reflect the current dependencies and results.

Ensure every CI check passes before merging.

[willchen96]

@amal66 Hi Amal could you resolve the remaining issues in my previous comment and rebase this PR? Then we can get this merged in.

[amal66]

@willchen96 Thanks — all points addressed, plus the rebase:

1. Frontend lint errors — after the rebase onto current main the count was 70 (main grew since your review); all came from the in-range eslint-plugin-react-hooks 7.0.1 → 7.1.1 bump enabling the new React-compiler rules. All 70 are fixed by restructuring rather than suppression (mostly the React-docs "adjust state during render" pattern; ref access moved out of render; memo deps made verifiable). One documented eslint-disable remains where an effect genuinely coordinates committed DOM. Lint now reports 0 errors.
2. Obsolete postcss allowlist entries — both removed. Their REMOVE WHEN condition ("Next bumps its postcss pin past 8.5.17") came true: Next 16.3.0 pins postcss 8.5.23. The sharp entry retired the same way (Next 16.3.0 declares sharp ^0.35.3), so the allowlist is down to a single entry — undici under miniflare, dev-only.
3. Root npm dependencies — the audit-gate matrix and Dependabot now cover every directory that owns a lockfile: root, backend, frontend.
4. PR description — rewritten to match the current tree and results.
5. CI — rebased onto current main with lockfiles regenerated (not hand-merged). Verified locally: backend 550 tests + build; frontend 0 lint errors, 280 tests + coverage ratchet, full production next build; all three audit gates exit 0. One note: a fresh nanoid advisory wave (GHSA-28wg-ghj8-5hjv, GHSA-2v37-7h3g-55p8) landed mid-revision and was absorbed in-range — docx declares ^5.1.3 and postcss ^3.3.16, so patched 5.1.16/3.3.18 resolve without any override or allowlist entry.

Ready for re-review.

---

CI note on the rebased head: every check is green except "Fresh install vs upgraded deployment" (Schema drift), which is failing on current main as well (same whitespace-only formatting mismatch in the chats RPC between schema.sql and the migration path). It predates this PR — no SQL is touched here — so I've left it to be fixed where it was introduced rather than folding an unrelated schema fix into a supply-chain PR.

[amal66]

The red "Fresh install vs upgraded deployment" check mentioned above now has a fix up: #330 re-syncs schema.sql with the get_chats_overview migration text (formatting-only drift from 57d754c on main). Once that merges and this branch picks it up, every check here goes green.