From b1a474a7ebde7f7dcd9a4181397b1e7571873195 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Mon, 27 Jul 2026 14:55:57 -0500 Subject: [PATCH] docs(website): record the landed route manifest and resequence CSP after PR 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Companion to website PR 0 (chore(server): freeze the URL surface with a generated route manifest). BACKEND_DESIGN.md gains § 4.0, naming the two generated artifacts that are actually authoritative about the API and what each is authoritative *for*: the manifest records which URLs exist (introspection-derived, reality), the Swagger spec records what they mean (annotation-derived, intent). The prose tables in § 4 are orientation and can drift; those two files cannot. Also documents routes.guards.json as a review aid that is explicitly not a contract. API_V2_PLAN.md marks PR 0 shipped and records its two deviations. The optional unauthenticated-status snapshot was tried and dropped exactly as that section allowed — against the dead-port mariadb pool the tests use it sits on the acquire timeout rather than failing fast — replaced by a deterministic assertion that every /admin/** and /player/** route still carries requireAuth. routes.guards.json is committed and staleness-checked even though a diff in it is not a contract change, because an ungenerated review aid rots into a misleading one. The sequencing section is corrected: PR 0 now runs before the CSP pair. The CSP report-only PR must stand up a POST /api/csp-report collector for `report-to` to target, which is a new URL under /api/**; landing it first would have left PR 0 generating 200 routes against a 199-route baseline, destroying its own acceptance test. With PR 0 first, the collector appears as a reviewed, deliberate +1 in the manifest — the mechanism working as intended. api-route-inventory.json is unchanged, which is the point: the generator reproduced it byte-for-byte on first run. Co-Authored-By: Claude --- website/API_V2_PLAN.md | 39 +++++++++++++++++++++++++++++++++++---- website/BACKEND_DESIGN.md | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/website/API_V2_PLAN.md b/website/API_V2_PLAN.md index c5b738d..0a1b4a3 100644 --- a/website/API_V2_PLAN.md +++ b/website/API_V2_PLAN.md @@ -261,7 +261,30 @@ Because the auth model is untouched and the URLs are frozen, each PR is a **pure with a green test suite and a zero-diff route manifest as its acceptance criteria** — which is what makes grouped PRs actually reviewable. -### PR 0 — the route manifest (prerequisite of the first split PR) +### PR 0 — the route manifest (prerequisite of the first split PR) — **landed** + +> **Status: shipped.** `server/scripts/routeManifest.js` + `npm run routes:manifest`, +> `server/routes.manifest.json` (199 public + 2 internal), `server/routes.guards.json`, +> `server/test/routeManifest.test.js`, and a `routes:manifest -- --check` step in +> `.gitea/workflows/pr-checks.yml`. No router file moved. **The generator reproduced +> `api-route-inventory.json` byte-for-byte on first run**, so the freeze is in effect and the +> committed baseline is confirmed accurate rather than merely asserted. +> +> Two deviations from the design below, both deliberate: +> +> - **The unauthenticated-status snapshot was tried and dropped**, exactly as this section allowed. +> Firing unauthenticated GETs at every manifest path against the dead-port mariadb pool the tests +> use does not fail fast — the pool sits on its acquire timeout, and a partial sweep had not +> finished after two minutes. A flaky two-minute gate is worse than none. What replaced it is +> cheap and deterministic: the test suite asserts from the introspected stack that every +> `/api/v1/admin/**` and `/api/v1/player/**` route still carries `requireAuth`. +> - **`routes.guards.json` is committed and staleness-checked**, though a diff in it is explicitly +> *not* a contract change. Left ungenerated it would rot into a misleading review aid within a +> release. The gate is on freshness; the meaning of a guards diff is still "read this", not +> "justify this". The generator drops app-level plumbing (helmet, morgan, the JSON parser, the bot +> guard) since it applies uniformly to all 199 routes and would bury the per-route gates. + + "Every URL is unchanged" must be *proved by a diff*, not asserted in review. PR 0 lands the tool that proves it, with no router file moved. @@ -309,9 +332,17 @@ proves it, with no router file moved. CSP and the split are independent; the only hard ordering is PR 0 before the first split PR. -1. **PR — CSP report-only.** Tightened policy behind `Content-Security-Policy-Report-Only` + `report-to`. -2. **PR — CSP enforce.** One release later, assuming a clean violation report. -3. **PR 0 — route manifest.** Generator + CI check + committed baseline of today's surface. No routers moved. +**Resequenced during implementation: PR 0 ships first, before the CSP pair.** The CSP report-only PR +has to stand up a violation collector (`POST /api/csp-report`) for `report-to` to point at — which is +a new URL under `/api/**`. Landing it first would mean PR 0's generator emitting 200 routes against a +199-route committed baseline, so PR 0 could no longer prove itself by reproducing +`api-route-inventory.json` byte-for-byte. With PR 0 first, the collector shows up as a reviewed, +deliberate `+1` in the manifest — which is exactly the mechanism working as designed. + +1. **PR 0 — route manifest.** Generator + CI check + committed baseline of today's surface. No routers moved. ✅ landed +2. **PR — CSP report-only.** Tightened policy behind `Content-Security-Policy-Report-Only` + `report-to`, + plus the report collector (manifest `+1`). +3. **PR — CSP enforce.** One release later, assuming a clean violation report. 4. **PR 1 — admin:** `users`, `account`, `invites`, `auth` (providers). 5. **PR 2 — admin:** `moderation`, `bot-activity`, `activity`. 6. **PR 3 — admin (content):** `posts`, `pages`, `wiki`, `uploads`. diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index 49a73cb..84ce089 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -33,6 +33,12 @@ Skeleton from the spec, with a small number of justified additions marked **(+)* > 110 routes) are being split into one router file per business capability — **in place, with every > URL unchanged**. This section and §4 get updated as each split PR lands. See > [API_V2_PLAN.md](./API_V2_PLAN.md) § Phase 2. +> +> "Every URL unchanged" is enforced mechanically, not by review: `server/scripts/routeManifest.js` +> (`npm run routes:manifest`) walks the live Express stack and writes the sorted +> `{ method, path }` freeze to `server/routes.manifest.json`, mirrored here as +> [api-route-inventory.json](./api-route-inventory.json). PR checks regenerate it and fail on any +> diff, so a split PR that moves a URL cannot merge silently. See § 4.0. ``` server/ @@ -257,6 +263,34 @@ marker. Cleared wholesale on TOTP disable / password change / password reset. Base path `/api/v1`. JSON in/out. Auth via httpOnly cookie (`isLoggedIn` reads it; also accepts `Authorization: Bearer` for API testing). +### 4.0 The authoritative route list + +The prose tables below are **orientation for a human reader** and can drift. Two generated artifacts +are authoritative, and they answer different questions: + +| Artifact | Source of truth for | Generated by | +|---|---|---| +| `server/routes.manifest.json` — mirrored as [api-route-inventory.json](./api-route-inventory.json) | **What URLs exist.** 199 public routes + 2 on the internal listener, sorted, method + path only. | `npm run routes:manifest`, by walking the live Express stack | +| `server/swagger/swagger-output.json` — served at `/api/docs` | **What each route means.** Parameters, bodies, response codes, security. | `npm run swagger`, from `#swagger.*` annotations | + +The split is deliberate: Swagger is annotation-derived, so an unannotated route is invisible in it and +it churns whenever a description is reworded — it documents *intent*. The manifest is introspection- +derived and records *reality*, which is why it, not Swagger, is the thing PR checks freeze +(`npm run routes:manifest -- --check`). + +Scope: the manifest keeps `/api/**` and `/.well-known/**` from the public app plus everything on the +internal listener. The SPA catch-all, `/uploads` and `/brand` are filesystem-conditional static +mounts — not API contract, and including them would make the output depend on whether CI had built +the client. + +A third generated file, `server/routes.guards.json`, is a **review aid and not a contract**: per route, +the middleware handler count plus the *named* middleware on its mount chain. It exists because a +router-level `router.use(noindex, isLoggedIn, staffOnly)` gate never appears in an individual route's +own stack, so a capability router extracted without re-applying the gate would otherwise publish +authenticated endpoints silently. Names are a hint only — `requireRole(...)` returns an anonymous +arrow and cannot be observed — but a *missing* `requireAuth` is unambiguous, and the server test suite +asserts every `/admin/**` and `/player/**` route still carries it. + ### /auth (auth.routes.js → auth.controller.js) | Method | Path | Auth | Body | Purpose | |---|---|---|---|---| -- 2.49.1