From 83b22d9b3b2b64f02832c25ebbbfb74efb852497 Mon Sep 17 00:00:00 2001 From: Meredith McGee Date: Sat, 29 Aug 2026 13:03:10 -0400 Subject: [PATCH] docs: canonical dependency-security consensus plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two-agent review (Codex brief, Claude independent review, two reconciliation rounds) reached consensus on how to resolve the current npm advisories. docs/DEPENDENCY-SECURITY-PLAN.md is canonical and internally consistent: - Stage 1: compatible refresh within Astro 5 (Vitest 4.1.x clears the critical, Resend 6.25.0 drops the svix/uuid chain), dependency reclassification, and a full-graph critical-severity CI audit gate. - Stage 2: Astro 7.2.9, floor 7.1.0. GHSA-4g3v affects 7.0.9 inclusive, so no 7.0.x is a safe stopping point. Astro 6 is not a valid intermediate: three advisories affect 6.4.6 with no backport. - Standing rule: stay on the current Astro major. Across all six Astro-family advisories examined, every fix landed on the then-current major and none was backported. The Astro 5.18.2 bump is hygiene, not remediation — no 5.x fix exists for either Astro advisory, and the branch summary must say so. All advisory ranges verified against the GitHub Advisory Database and npm registry, not taken from either agent's summary. Every affected advisory is currently unreachable in this codebase; the reachability conditions being relied upon are recorded in §7 so that a future change which violates one is recognized as a security event rather than a feature. The brief and the review are retained for provenance with SUPERSEDED headers. Do not work from either. Docs only, no build impact. Implementation not started. Co-Authored-By: Claude Opus 5 --- ...UDE-DEPENDENCY-SECURITY-CONSENSUS-BRIEF.md | 188 ++++++++ docs/CLAUDE-DEPENDENCY-SECURITY-REVIEW.md | 426 ++++++++++++++++++ docs/DEPENDENCY-SECURITY-PLAN.md | 343 ++++++++++++++ 3 files changed, 957 insertions(+) create mode 100644 docs/CLAUDE-DEPENDENCY-SECURITY-CONSENSUS-BRIEF.md create mode 100644 docs/CLAUDE-DEPENDENCY-SECURITY-REVIEW.md create mode 100644 docs/DEPENDENCY-SECURITY-PLAN.md diff --git a/docs/CLAUDE-DEPENDENCY-SECURITY-CONSENSUS-BRIEF.md b/docs/CLAUDE-DEPENDENCY-SECURITY-CONSENSUS-BRIEF.md new file mode 100644 index 0000000..1ae6609 --- /dev/null +++ b/docs/CLAUDE-DEPENDENCY-SECURITY-CONSENSUS-BRIEF.md @@ -0,0 +1,188 @@ +> **SUPERSEDED 2026-08-29.** This document contains recommendations that were +> revised during reconciliation and it is NOT internally consistent. Do not work +> from it. The canonical agreed plan is +> [`DEPENDENCY-SECURITY-PLAN.md`](DEPENDENCY-SECURITY-PLAN.md). +> Retained for provenance only. + +--- + +# RateMyPlace dependency-security decision brief + +**Audience:** Claude Project review, followed by a Codex/Claude consensus pass +**Prepared:** 2026-08-28 +**Decision requested:** Choose the safest proportionate path for resolving the current npm dependency advisories without disguising framework-migration risk as routine maintenance. + +## How to use this brief + +Please review this as an independent technical adviser. Do not implement anything yet. Challenge the risk assessment, identify assumptions that are not adequately supported, and recommend a concrete sequence of work. The final section gives a response format so your advice can be returned to Codex for a point-by-point consensus pass. + +If the Project supports file attachments, also attach the current `AGENTS.md`, `package.json`, and `package-lock.json`. The repository rules in `AGENTS.md` are binding; in particular, `main` auto-deploys, Astro major upgrades trigger a CSRF re-audit, and completion requires check, test, build, and proportionate flow verification. + +## Project and release context + +RateMyPlace is an Astro 5 server-rendered tenant housing review platform deployed to Cloudflare Pages. It uses React islands, Cloudflare D1 and R2, Lucia authentication, Resend email, Turnstile, Vitest, and Playwright. It is a small production service maintained by one person, so the plan must reduce security risk without creating an oversized maintenance burden. + +Release-safety work was just completed and merged through PR #13. CI now checks types, tests, and the production build, and the release process can smoke-test the immutable Cloudflare Pages deployment for the exact commit before separately verifying the custom domain. The dependency work should use those gates rather than bypassing them. + +## Confirmed audit snapshot + +The following was verified on 2026-08-28 against the checked-in dependency graph and source. No package or source files were changed during the audit. + +| Check | Result | +|---|---| +| `npm audit --json` | 29 advisories: 1 critical, 15 high, 12 moderate, 1 low | +| `npm audit --omit=dev --json` | 27 advisories: 0 critical, 14 high, 12 moderate, 1 low | +| Non-major dependency-resolution simulation | 10 advisories: 0 critical, 6 high, 4 moderate, 0 low | +| Coordinated Astro 7 dependency-resolution simulation | 1 low advisory, in the esbuild/tooling family | + +The `--omit=dev` result must not be confused with the deployed attack surface. Several build and Cloudflare-development packages are declared as production dependencies even though they are not necessarily reachable in the deployed Worker. Conversely, being present in a bundled file is not by itself proof that the application calls the vulnerable operation. + +### Current direct versions + +- Astro 5.16.11; current compatible Astro 5 release resolved in simulation: 5.18.2 +- `@astrojs/cloudflare` 12.6.13 +- `@astrojs/react` 3.6.3 +- Vitest 4.0.18; patched current release resolved in simulation: 4.1.11 +- happy-dom 20.5.1; compatible update resolved in simulation: 20.11.13 +- Resend 6.9.2; compatible update resolved in simulation: 6.25.0 +- `@astrojs/check` 0.9.6; compatible update resolved in simulation: 0.9.10 +- React and React DOM 18.3.1 + +### Evidence limits + +Both upgrade simulations ran in temporary directories and proved dependency resolution and the resulting audit counts only. They did not prove that the upgraded application passes type-checking, tests, build, E2E, or runtime smoke checks. Those are implementation gates, not facts already established by this brief. + +## Exposure assessment + +### 1. Critical Vitest advisory + +The critical advisory is [GHSA-5xrq-8626-4rwp](https://github.com/advisories/GHSA-5xrq-8626-4rwp), affecting Vitest versions from 4.0.0 through versions before 4.1.0. Its exploit conditions involve an exposed Vitest UI/API or Browser Mode on Windows or a network-accessible test service. + +This repository runs `vitest run` with `happy-dom`. `vitest.config.ts` does not enable the UI, Browser Mode, or an API host. Vitest is not deployed as an application service. The current evidence therefore does **not** indicate a critical production exploit path, but the package should still be patched promptly because the fix is compatible and low-cost. + +### 2. Astro advisory aggregate + +Astro 5.16.11 is associated with multiple advisories, which causes npm to aggregate several findings under the direct Astro dependency. A source sweep found no use of `define:vars`, Astro spread attributes, dynamic slot names, or Astro transition directives in the application. + +The Host-header SSRF advisory [GHSA-2pvr-wf23-7pc7](https://github.com/withastro/astro/security/advisories/GHSA-2pvr-wf23-7pc7) explicitly says the Cloudflare adapter is not affected. The dynamic-slot XSS advisory [GHSA-8hv8-536x-4wqp](https://github.com/withastro/astro/security/advisories/GHSA-8hv8-536x-4wqp) requires attacker-controlled dynamic slot names, which the source sweep did not find. This lowers current exploitability; it does not make the outdated framework line a permanent acceptable state. + +### 3. Cloudflare adapter image SSRF + +`@astrojs/cloudflare` 12.6.13 is below the fixed line for [GHSA-88gm-j2wx-58h6](https://github.com/advisories/GHSA-88gm-j2wx-58h6), an SSRF involving remote-image allowlist bypass through redirects. The built Worker contains the Astro image route, but its generated configuration has no allowed remote domains or patterns. The application source does not import `astro:assets` or render Astro `Image` or `Picture` components. + +The current evidence therefore suggests that the vulnerable operation is not reachable through an application-configured remote-image source. Fixing the adapter still matters, but the compatible fixed adapter requires a coordinated Astro major upgrade rather than a simple patch. + +### 4. Resend transitive chain + +Resend 6.9.2 brings in `svix`, which brings in a vulnerable `uuid` version. The application uses `new Resend(...).emails.send(...)` in `src/lib/email.ts`; it does not use Resend webhook/Svix processing or the affected UUID buffer APIs. A Resend update to 6.25.0 removed the `svix`/`uuid` advisory chain in the simulation without requiring an application API change. + +### 5. Development and build tools + +The remaining Vite/esbuild findings concern development-server or file-serving behavior. Repository scripts do not expose the development server with `--host`. Wrangler, Miniflare, undici, and ws are used in local/build/deployment tooling; the audit did not find those modules as application runtime dependencies in the deployed Worker. Sharp is related to the image path described above. + +### Overall assessment + +**Codex's provisional rating is medium maintenance risk, not an active critical production incident.** The critical label is real at the package level, but the known exploit preconditions are absent from the deployed application. The correct response is prompt, controlled remediation with release verification, not an emergency unreviewed framework migration. + +## Options + +### Option A: Patch only the obviously reachable or critical packages + +Update Vitest and Resend, leave the rest alone, and defer framework work. + +**Advantage:** smallest diff and fastest execution. +**Problem:** leaves avoidable compatible updates undone, retains most audit noise, and makes future critical findings harder to distinguish from known debt. This is not the recommended option. + +### Option B: Two-stage remediation + +First perform a non-major dependency refresh within the current architecture. Then handle Astro and its adapters as a separately reviewed framework migration. + +**Advantage:** removes the critical finding and most total findings quickly while keeping the behavioral diff narrow. It also gives the Astro major upgrade the CSRF, adapter, build, and release scrutiny required by the repository rules. +**Problem:** the repository temporarily retains 10 known advisories after Stage 1, including the Cloudflare adapter advisory, so the second stage must be scheduled rather than allowed to disappear into a backlog. + +**This is Codex's recommendation.** + +### Option C: Upgrade directly to Astro 7 in one change + +Perform the compatible updates and framework/adapter migration together. + +**Advantage:** the simulation reduced the graph to one low advisory in one project. +**Problem:** it mixes low-risk package maintenance with a framework major upgrade, expands the regression surface, and triggers a required CSRF re-audit. A successful install and build would not by themselves prove that authentication, origin checking, SSR behavior, middleware, and Cloudflare bindings retained their semantics. + +## Proposed staged path + +### Stage 1: compatible security refresh + +Work on a feature branch such as `codex/dependency-security-refresh`. Do not use `npm audit fix --force`. + +The expected intended changes are: + +- raise the minimum compatible versions for Astro 5, Vitest, happy-dom, Resend, and `@astrojs/check` to the tested versions listed above; +- refresh compatible transitive dependencies in `package-lock.json`; +- review every direct and transitive lockfile change rather than accepting an opaque forced rewrite; +- add a CI critical-advisory gate after the critical finding is removed, so it cannot silently return; +- add weekly Dependabot updates with a low-noise grouping policy, if the maintenance burden is acceptable. + +The simulation also updated compatible transitive packages including Vite, PostCSS, nanoid, js-yaml, fast-uri, defu, and devalue. The actual branch should record the final resolved versions and explain any difference from the simulation. + +Expected audit result, based on the resolution simulation: 29 advisories become 10, with no critical or low findings. The real branch must reproduce that result. + +No product behavior is intended to change. This stage should not alter application features, design, copy, scoring, database data or migrations, Cloudflare settings, environment variables, authentication policy, moderation behavior, or privacy rules. Because dependency updates can still cause regressions, “no intended behavior change” is not the same as “no risk.” + +Required evidence before merge: + +1. clean install from the committed lockfile; +2. `npm run check`; +3. full Vitest suite; +4. production build; +5. full local Playwright suite because build/runtime libraries changed; +6. before/after audit output and a reviewed lockfile diff; +7. immutable preview smoke tied to the exact branch SHA; +8. separate user approval before merge; +9. post-merge exact-SHA production smoke and custom-domain verification. + +### Stage 2: coordinated Astro major migration + +Use a separate feature branch and review. The dependency-resolution simulation used: + +- Astro 7.2.9; +- `@astrojs/cloudflare` 14.2.5; +- `@astrojs/react` 6.0.4; +- `@cloudflare/workers-types` 5.20260828.1; +- React and React DOM remaining on 18.3.1. + +Compatibility observations from package metadata: + +- Astro 7.2.9 requires Node 22.12 or newer; the release-safety baseline pins Node 22.16; +- Cloudflare adapter 14.2.5 peers with Astro 7.2 and a current Wrangler 4 line; +- React adapter 6.0.4 supports React 18; +- stopping at Cloudflare adapter 13.1.10 would still require Astro 6, so there is no adapter-only patch on the current Astro 5 architecture. + +This stage must include the repository-required CSRF re-audit. It should re-check `security.checkOrigin`, authenticated and unauthenticated POST behavior, middleware/session cookies, JSON-route protections, Cloudflare runtime bindings, SSR output, image endpoint exposure, and error behavior. Preview limitations for Turnstile and Maps remain expected; those integrations require production verification. + +Expected audit result from the dependency simulation: one low esbuild/tooling advisory. Any source or configuration changes needed for Astro 7 must be reviewed as functional changes, not hidden inside the package update. + +## Questions Claude should resolve + +1. Do you agree that there is no evidence of a currently reachable critical production exploit? If not, name the exact application path and advisory preconditions that make it reachable. +2. Does the Cloudflare adapter image SSRF justify skipping directly to Astro 7 despite the absent remote-image configuration and component usage? +3. Is the proposed Stage 1 package set complete and appropriately constrained? Should any package be excluded or added? +4. Should Stage 1 change both `package.json` minimums and the lockfile, or only refresh the lockfile within existing ranges? Explain the maintenance and reproducibility implications. +5. Is a full-graph `npm audit --audit-level=critical` CI gate the right first ratchet, or should the gate use production-only dependencies or a documented allowlist? How should it avoid normalizing permanent exceptions? +6. Should Dependabot be enabled in Stage 1, and how should updates be grouped to keep a single-maintainer repository manageable? +7. What additional tests or manual checks are necessary for the Astro 7 migration beyond the listed CSRF, auth, SSR, Cloudflare-binding, E2E, preview, and production checks? +8. Are there safer supported mitigations for the 10 interim advisories that do not require unsupported overrides or `--force`? + +## Requested response format + +Please return: + +1. **Verdict:** concur, concur with changes, or reject the two-stage recommendation. +2. **Risk assessment:** active incident vs maintenance risk, with the strongest evidence. +3. **Agreement table:** each major Codex claim, agree/disagree/uncertain, and why. +4. **Recommended Stage 1:** exact dependency and CI changes, verification, and rollback criteria. +5. **Recommended Stage 2:** exact migration boundary, required audits, and verification. +6. **Missing evidence:** anything that must be checked before implementation. +7. **Consensus candidate:** a short proposed plan that both Claude and Codex could adopt. + +Do not recommend an implementation solely because it produces a lower `npm audit` count. Optimize for actual production risk reduction, supported upgrade paths, reproducibility, and confidence that RateMyPlace's privacy and security behavior remains intact. diff --git a/docs/CLAUDE-DEPENDENCY-SECURITY-REVIEW.md b/docs/CLAUDE-DEPENDENCY-SECURITY-REVIEW.md new file mode 100644 index 0000000..c098679 --- /dev/null +++ b/docs/CLAUDE-DEPENDENCY-SECURITY-REVIEW.md @@ -0,0 +1,426 @@ +> **SUPERSEDED 2026-08-29.** This document contains recommendations that were +> revised during reconciliation and it is NOT internally consistent. Do not work +> from it. The canonical agreed plan is +> [`DEPENDENCY-SECURITY-PLAN.md`](DEPENDENCY-SECURITY-PLAN.md). +> Retained for provenance only. + +--- + +# Independent review: RateMyPlace dependency-security plan + +**Reviewer:** Claude (second technical reviewer) +**Date:** 2026-08-29 +**Reviewing:** `CLAUDE-DEPENDENCY-SECURITY-CONSENSUS-BRIEF.md` (Codex, 2026-08-28) +**Evidence:** `AGENTS.md`, `package.json`, `package-lock.json`, repository source at `origin/main` (a18254a), and primary advisory sources via web access. + +**Web access: available.** Advisory version ranges, severities, and peer-dependency +requirements below were verified against GitHub Advisory Database and the npm registry, +not taken from the brief. + +--- + +## 1. Verdict + +**Concur with changes.** + +The two-stage structure is correct and I endorse it. Option C (direct Astro 7) is properly +rejected. Option A is properly rejected. + +Three changes are required, one of which is material: + +1. **Stage 1 must stop being described as fixing the Astro advisories. It does not.** + Both Astro advisories are patched only in the 6.x line. Astro 5.18.2 — the last 5.x + release — remains affected by both. The brief's framing implies the residual 10 + advisories are a temporary state that Stage 2 will clear on some schedule. They are + not temporary. They are permanent for as long as the application runs Astro 5, and + they will not shrink with time or further 5.x releases. +2. **The Cloudflare adapter SSRF should be removed as a driver of Stage 2 urgency.** + It is CVSS 2.2 (Low) and requires configured remote-image allowlists that this + repository does not have. The brief treats it as a notable residual risk; it is close + to a non-risk here. +3. **Stage 2's target should be reconsidered: Astro 6, not Astro 7.** Astro 6.4.6 clears + 100% of the security-relevant advisories. Astro 7 is a currency decision, not a + security one, and conflating them repeats the mistake the brief correctly warns against. + +--- + +## 2. Risk assessment + +**Maintenance risk. Not an active incident.** I agree with Codex's rating, and the +evidence is stronger than the brief argues. + +**Strongest evidence, in order:** + +1. **The critical advisory is dev-classified in the lockfile itself**, not merely + unreachable by argument. `package-lock.json` marks `node_modules/vitest` and + `node_modules/happy-dom` as `"dev": true`. It never enters the deployed Worker graph. + This is stronger than the brief's reachability reasoning and independently corroborates + the `--omit=dev` result of 0 critical. + +2. **Vitest exploit preconditions are absent.** GHSA-5xrq-8626-4rwp (CVE-2026-47429, + CVSS 9.8) requires either `--api.host` / `api.host` config, or Vitest UI / Browser Mode. + `vitest.config.ts` sets only `environment: 'happy-dom'`, `include`, and `exclude`. No + UI, no browser mode, no API host. `npm test` is `vitest run`. + +3. **Both Astro High advisories have verified exclusions for this application.** + - GHSA-2pvr-wf23-7pc7 (CVSS 7.5): the advisory's own "Not affected" list names + `@astrojs/cloudflare` because it uses the ASSETS binding rather than HTTP fetch. + - GHSA-8hv8-536x-4wqp (CVSS 7.1): requires an attacker-controlled slot name on a + `client:*` component. A source sweep found no dynamic slot names, no `define:vars`, + and no transition directives. + +4. **The adapter SSRF precondition is absent.** GHSA-88gm-j2wx-58h6 (CVE-2026-41321, + CVSS 2.2) requires `image.domains` or `image.remotePatterns` to be configured. + `astro.config.mjs` has no `image` block at all, and no source file imports + `astro:assets` or renders `` / ``. The attack additionally requires + finding an open redirect on an already-allowlisted domain — of which there are none. + +**One risk the brief does not name.** The Vitest advisory's second precondition is UI or +Browser Mode **on Windows**. This repository's maintainer develops on Windows. Nothing +today meets the precondition, but a single future `vitest --ui` invocation would. That is +a developer-workstation risk, not a production risk, and it is a better argument for +prompt patching than "the fix is cheap." + +--- + +## 3. Agreement table + +| # | Codex claim | Verdict | Basis | +|---|---|---|---| +| 1 | Direct versions as listed (Astro 5.16.11, adapter 12.6.13, react adapter 3.6.3, Vitest 4.0.18, happy-dom 20.5.1, Resend 6.9.2, check 0.9.6, React 18.3.1) | **Agree** | Verified against `package-lock.json`. All exact. | +| 2 | No currently reachable critical production exploit | **Agree, with stronger evidence** | Vitest is `dev: true` in the lockfile; config enables no UI/browser/API host. | +| 3 | Host-header SSRF does not affect the Cloudflare adapter | **Agree** | Advisory's "Not affected" section names `@astrojs/cloudflare` (ASSETS binding). | +| 4 | Dynamic-slot XSS requires attacker-controlled slot names, not present | **Agree** | Source sweep confirms; advisory confirms the precondition. | +| 5 | Adapter image SSRF not reachable (no remote-image config, no `astro:assets`) | **Agree** | No `image` config in `astro.config.mjs`; no `astro:assets` / `` / `` in source. | +| 6 | No adapter-only patch exists on Astro 5 | **Agree — verified from registry** | `12.6.13` is the **last** 12.x release. `13.1.10` peers `astro: ^6.0.0`. Confirmed via `npm view`. | +| 7 | Resend 6.25.0 removes the `svix`/`uuid` chain | **Agree — verified** | `resend@6.9.2` deps: `svix 1.84.1` (which deps `uuid ^10.0.0`). `resend@6.25.0` deps: `postal-mime`, `standardwebhooks` — `svix` gone entirely. | +| 8 | App uses `emails.send`, not Resend webhooks/Svix | **Agree** | `src/lib/email.ts` uses `new Resend(...)` + `.emails.send(...)` only. No webhook/Svix import. | +| 9 | Dev server not exposed with `--host` | **Agree** | `"dev": "astro dev"` — no `--host`. | +| 10 | Overall rating: medium maintenance risk | **Agree** | See §2. | +| 11 | "Stage 1 raises Astro 5 to 5.18.2" as part of security remediation | **Disagree — material** | Neither Astro advisory has a 5.x fix. GHSA-8hv8 patches at **6.3.3**; GHSA-2pvr patches at **6.4.6**. 5.18.2 is the last 5.x and is affected by both. The bump is hygiene, not remediation. | +| 12 | The 10 residual advisories are an interim state Stage 2 will clear | **Disagree — reframe** | They are permanent on Astro 5, not interim. No further 5.x release will reduce them. Astro 5 is effectively a security-EOL line. | +| 13 | Adapter image SSRF is a notable residual risk justifying Stage 2 scheduling pressure | **Disagree** | CVSS 2.2 Low with an absent precondition. It should not carry scheduling weight. | +| 14 | Stage 2 target is Astro 7.2.9 | **Uncertain — challenge** | Astro 6.4.6+ clears every security-relevant advisory. Astro 7 is currency, not security. See §5. | +| 15 | Node 22.12+ required for Astro 7; baseline pins 22.16 | **Agree** | `.node-version` on `origin/main` is `22.16.0`. | +| 16 | CI checks types, tests, build (PR #13) | **Agree** | `.github/workflows/ci.yml` on `origin/main` runs `npm ci` → `npm run check` → `npm test` → `npm run build`, Node from `.node-version`. | +| 17 | Some build packages are declared as production dependencies | **Agree, and under-stated** | Six of them. See §4.1 — this has direct consequences for the CI gate design in Q5. | +| 18 | Audit counts 29 → 10 after Stage 1 | **Uncertain — not verified** | I did not run `npm audit`. I verified advisory metadata and reachability, not the counts. Codex's simulation is the only evidence; the real branch must reproduce it. | + +**Note on the provided artifacts.** The `package.json` supplied for review is stale by +exactly one line relative to `origin/main` — it lacks `"check": "astro check"`. The +`package-lock.json` is **identical** to `origin/main` (680 packages, zero version +differences). The dependency evidence base is therefore sound, and Stage 1's reliance on +`npm run check` is valid because that script does exist on `main`. + +--- + +## 4. Recommended Stage 1 + +Keep Codex's Stage 1 largely as written. Four changes. + +### 4.1 Add: correct the production/development dependency split + +Six build-only packages are declared in `dependencies`: + +| Package | Actual role | +|---|---| +| `@astrojs/check` | Type checking (`astro check`) — build/CI only | +| `typescript` | Build only | +| `@types/react` | Types only | +| `@types/react-dom` | Types only | +| `@tailwindcss/vite` | Build-time Vite plugin | +| `tailwindcss` | Build-time | + +None ships in the Worker. While they sit in `dependencies`, `npm audit --omit=dev` reports +on packages that are not in production — which is precisely why the `--omit=dev` figure +(27 advisories) is nearly as bad as the full-graph figure (29) and why the brief has to +warn readers not to trust it. + +This is a zero-runtime-risk change that makes the production graph mean something. It +should land in Stage 1 because Q5's gate design depends on it. + +**Blocking precondition:** verify that the Cloudflare Pages build installs +devDependencies. If the Pages build runs `npm ci --omit=dev` or sets +`NODE_ENV=production` in a way that skips them, this change breaks the production build. +CI uses plain `npm ci` (installs dev), but the Pages build configuration is not visible in +the repository. **Confirm before moving anything.** If Pages does skip dev deps, keep the +current classification and document why. + +### 4.2 Change: reframe the Astro 5.18.2 bump + +Keep the bump — it carries genuine bug fixes and reduces drift before a major migration. +**Stop describing it as addressing the Astro advisories.** The branch summary and any CI +gate rationale should state plainly that Astro advisories are not remediated in Stage 1 +and cannot be on the 5.x line. + +### 4.3 Keep, unchanged + +- Vitest → 4.1.x (patched line is ≥ 4.1.0; verified) +- Resend → 6.25.0 (removes `svix`/`uuid`; verified). Note this satisfies the existing + `^6.9.2` range, so a lockfile refresh alone reaches it. +- happy-dom → 20.11.x +- `@astrojs/check` → 0.9.10 +- No `npm audit fix --force` +- Reviewed lockfile diff, not an opaque rewrite + +### 4.4 Package set completeness + +The set is appropriate. I would **not** add anything else. In particular, do not attempt +`@astrojs/cloudflare` or `@astrojs/react` in Stage 1 — the adapter fix requires Astro 6 +(§3 row 6) and the React adapter major belongs with the framework migration. + +### Verification (in addition to the brief's list) + +The brief's nine evidence items are appropriate and I would not weaken any of them. Add: + +10. **A production-graph audit recorded separately** from the full-graph audit, after the + dependency reclassification, so the two numbers are meaningful going forward. +11. **Confirm the Astro image endpoint's behavior is unchanged** — it is the one route + touched by an advisory in this stage's blast radius. + +### Rollback criteria + +Revert the branch, do not patch forward, if any of: + +- `npm run check` fails and the cause is not a trivial type-import change; +- any Vitest suite fails that is not a test-framework API change with an obvious, + reviewed fix; +- the production build emits any new error; +- Playwright shows any auth, session, or moderation regression; +- the immutable preview smoke fails at the exact branch SHA; +- the lockfile diff contains a package change that cannot be explained. + +Because `main` auto-deploys, rollback after merge means `git revert` plus a re-verified +deploy, not a force-push. + +--- + +## 5. Recommended Stage 2 + +> ### ⚠️ SUPERSEDED 2026-08-29 — this section's recommendation was wrong +> +> I recommended stopping at Astro 6.4.6. Codex reproduced the audit and identified three +> advisories that still affect 6.4.6, which I verified against primary sources: +> +> | Advisory | Affected | Patched | Severity | +> |---|---|---|---| +> | GHSA-f48w-9m4c-m7f5 (CVE-2026-59729) | `< 7.0.6` | 7.0.6 | Moderate 5.1 | +> | GHSA-7pw4-f3q4-r2p2 (CVE-2026-59727) | `>= 3.10.0, < 7.0.4` | 7.0.4 | Low | +> | GHSA-4g3v-8h47-v7g6 (CVE-2026-73422) | `>= 2.9.0, <= 7.0.9` | 7.1.0 | Moderate 5.3 | +> +> None has a 6.x backport. Stopping at 6 would mean paying for a full major migration and +> a CSRF re-audit and still sitting on a line that cannot receive these fixes. +> +> **Revised recommendation: Astro 7.2.9, as Codex originally proposed.** Note the floor is +> **7.1.0**, not 7.0.x — GHSA-4g3v affects up to and including 7.0.9, so a "compromise" at +> 7.0.6 would still be exposed. 7.2.9 clears it with margin. +> +> The reasoning in the rest of this section — one major at a time, independently revertable +> — remains sound in the abstract. It was defeated by the specific fact that Astro does not +> backport security fixes to prior majors. **The required audits and verification steps +> below still stand in full.** +> +> **Generalizable lesson, worth writing into the repo:** across all five Astro advisories +> examined, every fix landed on the then-current major and none was backported. This +> project cannot sit a major behind and expect patches. Staying on the current Astro major +> is an operational requirement, not hygiene. +> +> All three advisories are currently **unreachable** in this codebase — verified: no +> `transition:*` directives, no `ViewTransitions`/`ClientRouter` import, no spread +> attributes in `.astro` files. So this is not urgent. It is an argument for not doing the +> migration twice. + +### The boundary I originally recommended (superseded — retained for the record) + +The brief targets **Astro 7.2.9**. I recommend targeting **Astro 6 (≥ 6.4.6)** with +`@astrojs/cloudflare` 13.1.10+ instead, and treating Astro 7 as a separate, later, +non-security decision. + +**Why:** + +- Astro 6.4.6 clears **every** security-relevant advisory in this graph. GHSA-8hv8 patches + at 6.3.3; GHSA-2pvr at 6.4.6; the adapter fix (13.1.10) peers `astro: ^6.0.0`. There is + no advisory in this set that requires Astro 7. +- The brief's own core argument is that framework migration must not be smuggled inside + routine maintenance. Targeting 7 when 6 resolves the security case smuggles a second + major upgrade inside the security work — the same category error, one level up. +- One major at a time is independently verifiable and independently revertable. A solo + maintainer with an auto-deploying `main` benefits from that more than from saving a + second migration. + +**The honest counter-argument**, which Codex should weigh: `AGENTS.md` requires a CSRF +re-audit per Astro major. Two majors means two re-audits. Going straight to 7 does that +work once. If the re-audit is the dominant cost rather than the regression risk, 7 in one +step is defensible. + +**My position:** security-motivated work should stop at the version that fixes the security +issue. Astro 7 should be scheduled on its own merits — support window, ecosystem currency — +with its own justification and its own review. If Codex prefers 7 in one step, that is +acceptable provided the branch summary states explicitly that the increment from 6 to 7 is +currency, not remediation. + +### Required audits, either way + +Everything the brief lists, which is appropriate: CSRF re-audit against +`.planning/audits/csrf-2026-04.md`, `security.checkOrigin` behavior, authenticated and +unauthenticated POST paths, the JSON content-type gap on `/api/disputes`, middleware and +session cookie attributes (`SameSite=Lax`), Cloudflare runtime bindings (`DB`, +`VERIFICATION_BUCKET`), SSR output, image endpoint exposure, and error behavior. + +**Add three:** + +1. **`getEnv` / `fireAndForget` contract.** `src/lib/runtime.ts` depends on + `context.locals.runtime` and `ctx.waitUntil`. An adapter major is the most likely place + for that shape to change. A silent `waitUntil` regression would break outbound email + with no test failure and no user-visible error. +2. **Rate-limiter fail-closed behavior.** Confirm a limiter DB error still yields 503 and + not an open pass. This is a security control that no type check will catch. +3. **Audit-log writes still succeed post-migration**, given migration 0028 only just + repaired the CHECK constraints. + +### Verification + +Preview cannot exercise Turnstile or the map (`AGENTS.md`). Both must be verified on +production after deploy, along with a full pass of the pre-deploy QA checklist — +this is exactly the change class it exists for. + +--- + +## 6. Missing evidence + +Must be established before implementation: + +1. **Cloudflare Pages install behavior** — does the Pages build install devDependencies? + Blocks §4.1. Highest priority. +2. **A real `npm audit` run.** I verified advisory metadata and reachability from primary + sources; I did **not** reproduce the 29 / 27 / 10 / 1 counts. Those remain + single-sourced from Codex's simulation. +3. **Current E2E status.** The brief requires a full Playwright pass as Stage 1 evidence, + but nothing establishes that the suite is green *today*. A pre-existing failure would + be misread as a dependency regression. +4. **Astro 6 (or 7) breaking-change review** against the official migration guide, + specifically for SSR, middleware, and adapter API changes. Not attempted here. +5. **Whether the Astro image endpoint is actually emitted** in the deployed Worker and + what it returns for a remote URL. The brief asserts the route is present; if it can be + removed or made inert by configuration, that would neutralize the adapter advisory + independently of any upgrade — worth ten minutes before committing to a schedule. +6. **`@astrojs/react` 3.6.3 is pinned exactly** (no caret) in `package.json`. Confirm + whether that pin is deliberate; if it was a workaround, the reason should be recorded + before Stage 2 changes it. + +--- + +## 7. Consensus candidate + +A plan both reviewers should be able to adopt: + +**Stage 1 — compatible security refresh** (branch: `codex/dependency-security-refresh`) + +1. Verify Cloudflare Pages installs devDependencies. If yes, move `@astrojs/check`, + `typescript`, `@types/react`, `@types/react-dom`, `@tailwindcss/vite`, and `tailwindcss` + to `devDependencies`. If no, document why they stay. +2. Update Vitest → 4.1.x, Resend → 6.25.0, happy-dom → 20.11.x, `@astrojs/check` → 0.9.10, + Astro → 5.18.2. No `--force`. Review the lockfile diff line by line. +3. State explicitly in the branch summary that **Astro advisories are not remediated by + this stage and cannot be on the 5.x line.** +4. Add a CI gate: `npm audit --audit-level=critical` on the full graph. This is the right + first ratchet — it is the one severity currently at zero after Stage 1, so it cannot + normalize an exception. Do **not** add a high-severity gate yet; it would fail + immediately on the permanent Astro findings and train everyone to ignore it. +5. Record the production-graph audit separately once classification is fixed. Revisit a + `--omit=dev --audit-level=high` gate only after Stage 2. +6. Dependabot: enable, but **grouped and monthly** for non-security updates and weekly + only for security advisories. Weekly ungrouped updates on a single-maintainer repo + produce noise that gets ignored, which is worse than no gate. +7. Evidence: the brief's nine items, plus a separately recorded production-graph audit. + +**Between stages — do not let this sit silently** + +Because the residual 10 are permanent rather than interim, record them where they will be +seen: a dated entry in `.planning/` naming the Astro line as security-EOL, the two High +advisories, and the specific reason each is currently unreachable. If any of those reasons +change — a `client:*` component gains a dynamic slot name, or remote image patterns get +configured — the risk changes immediately and the schedule must move. + +**Stage 2 — framework migration** (separate branch, separate review) — *revised 2026-08-29* + +- Target **Astro 7.2.9** with `@astrojs/cloudflare` 14.2.5, `@astrojs/react` 6.0.4, + `@cloudflare/workers-types` 5.x, React staying at 18.3.1. Node 22.16 pin already + satisfies Astro 7's 22.12 floor. +- **The security floor is 7.1.0**, not 7.0.x — GHSA-4g3v-8h47-v7g6 affects up to and + including 7.0.9. Do not treat any 7.0.x as a safe stopping point. +- Astro 6 is **not** a valid intermediate: three advisories affect 6.4.6 with no backport. + See the superseded note in §5. +- Review **both** major migration guides (5→6 and 6→7); a single step across two majors + does not mean a single set of breaking changes. +- One comprehensive CSRF and runtime audit covering both majors' changes, plus the three + additions in §5: the `getEnv` / `fireAndForget` (`ctx.waitUntil`) contract, the rate + limiter's fail-closed 503, and audit-log writes post-migration 0028. +- Adopt a standing rule: **stay on the current Astro major.** Every advisory examined here + was fixed only on the then-current major, never backported. + +**Standing principle for both stages:** do not adopt a change because it lowers an +`npm audit` count. Every item above is justified by reachability, supported upgrade paths, +or reproducibility — not by the number. + +--- + +## Appendix: answers to the brief's eight questions + +**Q1 — Reachable critical production exploit?** +No. Vitest is `dev: true` in the lockfile and never enters the Worker graph; the config +enables no UI, Browser Mode, or API host. Patch it anyway — promptly — because the +maintainer develops on Windows and one future `vitest --ui` would meet the precondition. + +**Q2 — Does the adapter image SSRF justify skipping to Astro 7?** +No. CVSS 2.2 (Low), and it requires `image.domains` / `image.remotePatterns`, neither of +which is configured, plus an open redirect on an allowlisted domain, of which there are +none. It should carry no scheduling weight. The genuine argument for leaving Astro 5 is +the two High advisories with no 5.x fix — and those point to Astro 6, not 7. + +**Q3 — Is the Stage 1 package set complete and constrained?** +Complete and correctly constrained. Add only the dependency reclassification (§4.1). +Exclude `@astrojs/cloudflare` and `@astrojs/react` — both belong to Stage 2. + +**Q4 — `package.json` minimums plus lockfile, or lockfile only?** +Both. Lockfile-only refresh would reach Resend 6.25.0 (it satisfies `^6.9.2`) but leaves +the declared floor at a known-vulnerable version, so a future `npm install` or a fresh +resolution could legitimately go backwards. Raising the floor makes the security decision +explicit and reviewable in `package.json` rather than implicit in a 680-package lockfile. +Reproducibility comes from committing the lockfile; *intent* comes from the range. Both +are needed. + +**Q5 — Is a full-graph `--audit-level=critical` gate the right first ratchet?** +Yes, as the first ratchet — with a caveat. It works precisely because critical is at zero +after Stage 1, so it can never require an exception and therefore cannot normalize one. +Do not start with `--omit=dev`: while six build packages sit in `dependencies`, the +production graph is not the production graph. Do not start with `high`: it would fail on +day one against permanent Astro findings, and a gate that fails permanently is a gate that +gets bypassed permanently. Sequence: critical gate now → fix classification → production +high gate after Stage 2. If an allowlist ever becomes necessary, require an expiry date on +each entry and fail the build when it passes, so exceptions cannot become permanent by +inattention. + +**Q6 — Dependabot in Stage 1, and grouping?** +Yes, in Stage 1. Group aggressively: one grouped PR monthly for non-security updates, and +security advisories separately and promptly. Weekly ungrouped updates on a solo repo +generate more PRs than one person will review, and unreviewed dependency PRs are worse +than none — they train the maintainer to merge without reading, which is the exact failure +mode this whole exercise exists to prevent. + +**Q7 — Additional Astro-major checks beyond the listed set?** +Three, all invisible to type checks and builds: the `getEnv` / `fireAndForget` +(`ctx.waitUntil`) contract in `src/lib/runtime.ts`, whose silent failure would break +outbound email with no error; the rate limiter's fail-closed 503 behavior; and audit-log +writes, given migration 0028 only just repaired the CHECK constraints. See §5. + +**Q8 — Safer supported mitigations for the interim advisories?** +Largely no, and that is the point — which is why they should be documented as permanent +rather than interim. Two exceptions worth investigating: (a) if the Astro image endpoint +can be removed or made inert by configuration, the adapter advisory is neutralized without +any upgrade; (b) the two Astro Highs are already mitigated by architecture rather than by +patch — the Cloudflare adapter's ASSETS binding and the absence of dynamic slot names. +Those mitigations should be written down as *conditions being relied upon*, so that a +future change which violates one is recognized as a security event. Do not use `overrides` +to force transitive versions the packages do not support; that trades a known advisory for +an unknown incompatibility. diff --git a/docs/DEPENDENCY-SECURITY-PLAN.md b/docs/DEPENDENCY-SECURITY-PLAN.md new file mode 100644 index 0000000..d150a07 --- /dev/null +++ b/docs/DEPENDENCY-SECURITY-PLAN.md @@ -0,0 +1,343 @@ +# Dependency security plan + +**Status:** Agreed. Canonical. Supersedes the Codex consensus brief (2026-08-28) and the +Claude independent review (2026-08-29) in full. +**Agreed:** 2026-08-29 by Codex and Claude after two reconciliation rounds. +**Implementation:** not started. + +This is the single source of truth. Where an earlier document disagrees with this one, +this one wins. Do not work from the brief or the review. + +--- + +## 1. Decision + +| | | +|---|---| +| **Stage 1** | Compatible dependency refresh within Astro 5, plus a critical-severity CI audit gate | +| **Stage 2** | Astro 7.2.9 framework migration, with **7.1.0 as the absolute security floor** | +| **Migration guides** | Review **both** 5→6 and 6→7. One step across two majors is not one set of breaking changes. | +| **Re-audit scope** | CSRF, Cloudflare runtime and `waitUntil`, fail-closed rate limiting, audit logging | +| **Standing rule** | Remaining on the current Astro major is an operational security requirement | + +**Rejected:** patching only Vitest and Resend (leaves avoidable work undone, retains audit +noise). **Rejected:** a single combined dependency-and-framework change (mixes low-risk +maintenance with a double-major migration). + +**Considered and rejected during review:** stopping at Astro 6.4.6. Three advisories affect +6.4.6 with no backport (§4). Stopping there would cost a full major migration and a CSRF +re-audit and still leave the project on a line that cannot receive the fixes. + +--- + +## 2. Risk assessment + +**Maintenance risk. Not an active incident.** + +The critical label is real at the package level. The exploit preconditions are absent from +the deployed application, and the strongest evidence is structural rather than +argumentative: **Vitest is `"dev": true` in `package-lock.json`** and never enters the +deployed Worker graph. + +Every advisory examined is either dev-scoped, architecturally excluded, or gated on a +feature this codebase does not use. **Verified absent from source:** `astro:assets`, +``, ``, `define:vars`, dynamic slot names, `transition:*` directives, +`ViewTransitions` / `ClientRouter` imports, spread attributes in `.astro` files, and any +`image.domains` / `image.remotePatterns` configuration. + +This justifies controlled remediation with release verification. It does not justify an +emergency unreviewed migration, and it does not justify indefinite deferral. + +--- + +## 3. Verified state + +Confirmed against `package-lock.json` at `origin/main`: + +| Package | Version | Scope | +|---|---|---| +| astro | 5.16.11 | production | +| @astrojs/cloudflare | 12.6.13 | production | +| @astrojs/react | 3.6.3 (exact pin, no caret) | production | +| @astrojs/check | 0.9.6 | declared production, actually build-only | +| resend | 6.9.2 → `svix` 1.84.1 → `uuid` ^10.0.0 | production | +| vitest | 4.0.18 | **dev** | +| happy-dom | 20.5.1 | **dev** | +| react / react-dom | 18.3.1 | production | + +Node pinned at 22.16.0 in `.node-version`, which satisfies Astro 7's 22.12 floor. +CI (`.github/workflows/ci.yml`) runs `npm ci` → `npm run check` → `npm test` → +`npm run build`. **No audit gate exists yet.** + +--- + +## 4. Advisory evidence + +All version ranges verified against the GitHub Advisory Database and the npm registry. + +### Resolved by Stage 1 + +| Advisory | Package | Affected | Patched | Severity | +|---|---|---|---|---| +| GHSA-5xrq-8626-4rwp (CVE-2026-47429) | vitest | `>= 4.0.0, < 4.1.0` | 4.1.0 | Critical 9.8 | +| `uuid` chain via `svix` | resend | 6.9.2 | resend 6.25.0 drops `svix` for `standardwebhooks` | — | + +The Vitest advisory requires `--api.host` / `api.host`, or Vitest UI / Browser Mode. +`vitest.config.ts` enables none of these. Note the Windows precondition applies to this +maintainer's environment, so a future `vitest --ui` would meet it. That is a +developer-workstation risk, and a better reason to patch than low cost. + +### Requires Astro 6 — not fixed on the 5.x line + +| Advisory | Affected | Patched | Severity | Why unreachable here | +|---|---|---|---|---| +| GHSA-2pvr-wf23-7pc7 (CVE-2026-54299) | `< 6.4.6` | 6.4.6 | High 7.5 | Advisory's own "Not affected" list names `@astrojs/cloudflare` (uses the ASSETS binding) | +| GHSA-8hv8-536x-4wqp (CVE-2026-50146) | `< 6.3.3` | 6.3.3 | High 7.1 | Requires attacker-controlled slot name on a `client:*` component; none exist | + +### Requires Astro 7 — not fixed on the 6.x line + +| Advisory | Affected | Patched | Severity | Why unreachable here | +|---|---|---|---|---| +| GHSA-f48w-9m4c-m7f5 (CVE-2026-59729) | `< 7.0.6` | 7.0.6 | Moderate 5.1 | Requires spread props on HTMLElement-subclass components; no spread attributes in `.astro` | +| GHSA-7pw4-f3q4-r2p2 (CVE-2026-59727) | `>= 3.10.0, < 7.0.4` | 7.0.4 | Low | Requires untrusted input in `transition:persist` / `scope` / `persist-props`; no transition directives | +| GHSA-4g3v-8h47-v7g6 (CVE-2026-73422) | `>= 2.9.0, <= 7.0.9` | **7.1.0** | Moderate 5.3 | Requires attacker-controlled View Transition animation values; no View Transitions in use | + +**GHSA-4g3v affects 7.0.9 inclusive. Any 7.0.x is exposed. 7.1.0 is the floor; 7.2.9 is the target.** + +### Not a driver + +| Advisory | Affected | Patched | Severity | Status | +|---|---|---|---|---| +| GHSA-88gm-j2wx-58h6 (CVE-2026-41321) | @astrojs/cloudflare `< 13.1.10` | 13.1.10 | **Low 2.2** | Requires configured `image.domains` / `image.remotePatterns`, plus an open redirect on an allowlisted domain. No image config exists. Carries no scheduling weight. | + +`@astrojs/cloudflare` 12.6.13 is the **last** 12.x release, and 13.1.10 peers `astro: ^6.0.0`. +There is no adapter-only patch available on Astro 5. Verified via `npm view`. + +### The pattern + +Across all six Astro-family advisories, **every fix landed on the then-current major and +none was backported.** This is why Astro 6 is not a valid intermediate, and why §7 makes +staying current an operational requirement rather than hygiene. + +--- + +## 5. Stage 1 — compatible security refresh + +Branch: `codex/dependency-security-refresh`. Do not use `npm audit fix --force`. + +### 5.1 Dependency reclassification + +**Precondition, blocking:** confirm the Cloudflare Pages build installs devDependencies. +CI uses plain `npm ci` (which does), but the Pages build configuration is not visible in +the repository. If Pages skips dev dependencies, **do not make this change** — record why +and move on. + +Once confirmed, move these six from `dependencies` to `devDependencies`. None ships in the +Worker: + +`@astrojs/check` · `typescript` · `@types/react` · `@types/react-dom` · +`@tailwindcss/vite` · `tailwindcss` + +**Why it matters:** while build tooling is declared as production, `npm audit --omit=dev` +reports on packages that are not in production — which is why that figure (27) is nearly +identical to the full-graph figure (29) and currently means nothing. §5.4's gate design +depends on fixing this. + +### 5.2 Version updates + +| Package | To | Note | +|---|---|---| +| vitest | 4.1.x | Clears the critical advisory | +| resend | 6.25.0 | Drops `svix`/`uuid`. Satisfies existing `^6.9.2`, so a lockfile refresh alone reaches it | +| happy-dom | 20.11.x | | +| @astrojs/check | 0.9.10 | | +| astro | 5.18.2 | **Hygiene, not remediation.** See below | + +**Do not add** `@astrojs/cloudflare` or `@astrojs/react`. Both belong to Stage 2. + +**The Astro 5.18.2 bump must not be described as addressing the Astro advisories.** It does +not, and cannot — no 5.x fix exists. Keep it for bug fixes and to reduce drift before the +major migration, and say so plainly in the branch summary. + +### 5.3 package.json and lockfile + +Change **both**. A lockfile-only refresh reaches Resend 6.25.0 but leaves the declared +floor at a known-vulnerable version, so a future resolution could legitimately go +backwards. Reproducibility comes from committing the lockfile; *intent* comes from the +declared range. + +Review the lockfile diff line by line. Do not accept an opaque forced rewrite. + +### 5.4 CI audit gate + +Add to `.github/workflows/ci.yml`: + +``` +npm audit --audit-level=critical +``` + +**Full graph, critical only.** This is the correct first ratchet because critical is at +zero after Stage 1, so it can never require an exception and therefore cannot normalize +one. + +- **Not `--omit=dev`** until §5.1 lands. Before that, the production graph is not the + production graph. +- **Not `high`.** It would fail on day one against the permanent Astro findings, and a gate + that always fails is a gate that gets bypassed permanently. +- **Sequence:** critical gate now → reclassification → reconsider a + `--omit=dev --audit-level=high` gate after Stage 2. +- If an allowlist ever becomes necessary, require an expiry date per entry and fail the + build when it passes. Exceptions must not become permanent through inattention. + +### 5.5 Dependabot + +Two independent mechanisms, configured in two different places. GitHub's documentation is +explicit: *"There is no interaction between the settings specified in the `dependabot.yml` +file and Dependabot security alerts."* + +| | Configured | Trigger | Setting | +|---|---|---|---| +| **Security updates** | Repository Settings → Code security | **Advisory publication.** Not scheduled. | Enable. No cadence to choose. | +| **Version updates** | `.github/dependabot.yml` | `schedule.interval` | **Monthly, grouped.** | + +Enable security updates. Configure version updates monthly and grouped. + +Weekly ungrouped version updates on a single-maintainer repository produce more pull +requests than one person will review, and unreviewed dependency PRs are worse than none: +they train the maintainer to merge without reading, which is the exact failure this work +exists to prevent. + +### 5.6 Evidence required before merge + +1. Clean install from the committed lockfile +2. `npm run check` +3. Full Vitest suite +4. Production build +5. Full local Playwright suite (build and runtime libraries changed) +6. Before/after audit output and a reviewed lockfile diff +7. Production-graph audit recorded **separately**, after reclassification +8. Immutable preview smoke tied to the exact branch SHA +9. Separate user approval before merge +10. Post-merge exact-SHA production smoke and custom-domain verification + +### 5.7 Rollback criteria + +Revert the branch rather than patching forward if any of: + +- `npm run check` fails for anything beyond a trivial reviewed type-import change +- A Vitest failure that is not an obvious, reviewed test-framework API change +- Any new build error +- Any Playwright regression in auth, session, or moderation +- Preview smoke fails at the exact branch SHA +- Any lockfile change that cannot be explained + +`main` auto-deploys, so post-merge rollback is `git revert` plus a re-verified deploy, +never a force-push. + +--- + +## 6. Stage 2 — Astro 7 migration + +Separate branch, separate review. Do not begin until Stage 1 is merged and verified. + +### 6.1 Target + +| Package | Target | +|---|---| +| astro | **7.2.9** (floor: 7.1.0 — see §4) | +| @astrojs/cloudflare | 14.2.5 | +| @astrojs/react | 6.0.4 (supports React 18) | +| @cloudflare/workers-types | 5.x | +| react / react-dom | stay at 18.3.1 | + +Node 22.16.0 already satisfies the 22.12 floor. + +Review **both** the 5→6 and 6→7 migration guides. A single step across two majors carries +two majors' worth of breaking changes. + +### 6.2 Required audit + +One comprehensive pass covering both majors' changes. + +**CSRF** (required by `AGENTS.md` on any Astro major; baseline is +`.planning/audits/csrf-2026-04.md`): `security.checkOrigin` behavior, authenticated and +unauthenticated POST paths, the `application/json` gap on `/api/disputes`, middleware and +session cookie attributes (`SameSite=Lax`). + +**Runtime and bindings:** Cloudflare bindings (`DB`, `VERIFICATION_BUCKET`), SSR output, +image endpoint exposure, error behavior. + +**Three checks that no type check or build will catch:** + +1. **`getEnv` / `fireAndForget`.** `src/lib/runtime.ts` depends on + `context.locals.runtime` and `ctx.waitUntil`. An adapter major is the likeliest place + for that shape to change, and a silent `waitUntil` regression breaks outbound email with + no test failure and no user-visible error. +2. **Rate limiter fail-closed.** Confirm a limiter DB error still yields 503 and not an + open pass. +3. **Audit-log writes.** Confirm they still succeed, given migration 0028 only recently + repaired the CHECK constraints. + +### 6.3 Verification + +Full pre-deploy QA checklist (`AGENTS.md`) — this is exactly the change class it exists +for. Turnstile and the map cannot be exercised on preview and must be verified on +production after deploy. + +Any source or configuration change required by Astro 7 is a functional change and gets +reviewed as one. It does not ride along inside the package update. + +--- + +## 7. Standing rules + +Adopted as outcomes of this review. + +1. **Stay on the current Astro major.** Every advisory examined was fixed only on the + then-current major; none was backported. Falling a major behind means accumulating + unfixable findings. +2. **Never adopt a change because it lowers an `npm audit` count.** Justify by + reachability, supported upgrade path, or reproducibility. +3. **Record architectural mitigations as conditions being relied upon.** Several advisories + are neutralized by what this codebase does not do — no View Transitions, no + `astro:assets`, no dynamic slot names, no remote image allowlist, the Cloudflare + adapter's ASSETS binding. If any of those changes, the risk changes that day. A future + `` import is a security event, not a feature. +4. **Distinguish remediation from hygiene in every branch summary.** A version bump that + fixes nothing must not be described as fixing something. + +--- + +## 8. Open items + +Establish before or during implementation: + +| # | Item | Blocks | +|---|---|---| +| 1 | Does the Cloudflare Pages build install devDependencies? | §5.1 | +| 2 | Is the Playwright suite green **today**? A pre-existing failure would be misread as a dependency regression. | §5.6 item 5 | +| 3 | Is `@astrojs/react`'s exact pin (3.6.3, no caret) deliberate? If it was a workaround, record why before Stage 2 changes it. | §6.1 | +| 4 | Can the Astro image endpoint be removed or made inert by configuration? If so, GHSA-88gm is neutralized independently of any upgrade. | Nothing — opportunistic | +| 5 | Astro 5→6 and 6→7 breaking-change review against the official guides | §6 | + +The 29 / 27 / 10 / 1 audit counts come from Codex's resolution simulation and its +reproduction run. Claude verified advisory metadata and reachability but did not +independently reproduce the counts. The real branch must reproduce them. + +--- + +## 9. Provenance + +- **Codex consensus brief**, 2026-08-28 — audit snapshot, exposure assessment, two-stage + proposal. *Superseded by this document.* +- **Claude independent review**, 2026-08-29 — verified claims against lockfile, source, and + primary advisory sources; corrected the Astro 5 remediation framing; identified the + dependency misclassification; recommended Astro 6 as the Stage 2 target. + *Superseded by this document.* +- **Codex reconciliation**, 2026-08-29 — accepted the Stage 1 corrections; produced the + three Astro 7 advisories that defeat the Astro 6 target; corrected the Dependabot + security-update cadence. +- **Claude concurrence**, 2026-08-29 — verified all three advisories and the Dependabot + mechanism against primary sources; withdrew the Astro 6 recommendation. + +Both reviewers concur on the whole of this document. Implementation has not started.