Files
docs/website/API_V2_PLAN.md
wtclaude 45fce7cb9f docs(website): record the PR 4 admin router split and the end of admin.routes.js
Covers website PR 4, the last admin split PR: shard (16), uo-link (5), email
(6), discord-bot (2), settings (2) and dashboard/site-mode (2) leave the
residual file, which is deleted. The admin group is fully split.

API_V2_PLAN.md gains a "PR 4 — as landed" section recording the two decisions a
reviewer would otherwise have to reconstruct: dashboard.router.js is mounted at
the group root (the single relaxation of the mount-at-a-prefix rule, safe only
because it declares no router-level middleware), and /shard keeps two gate tiers
in one router because prefix ownership beats swagger-tag grouping. Sequencing
item 7 is marked landed; PR 5 (public/player/auth) is the only split PR left.

BACKEND_DESIGN.md §2 gets the six new routers in the folder tree and drops the
residual entry; §4's /admin preamble now describes the ops/config gates instead
of pointing at a file that no longer exists.

WIKI_UPGRADE.md's two links into admin.routes.js are repointed at wiki.router.js
and admin/imageUpload.js.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-07-27 20:02:49 -05:00

644 lines
42 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Website API — router domain split + CSP hardening
Status: **in progress** — PR 0 (route manifest), CSP report-only, and split PRs 12 of 5 have landed ·
Target repo: `website/` · Docs owner: this file + `BACKEND_DESIGN.md`
> **This file replaces the earlier "API v2" plan** (auth merge → CSP → domain split, with a parallel
> `/api/v2` mount and an `/api/mobile` facade). Three of those four pieces are **not being built**:
> the auth merge and the mobile facade are deferred with their reasoning recorded below, and the
> parallel-version scaffold in [API_V2_SKELETON.md](./API_V2_SKELETON.md) is superseded. The filename
> is kept so existing links resolve. What remains is genuinely useful work:
>
> 1. **CSP hardening** — small, independent, ships on its own cadence.
> 2. **The domain split** — `admin.routes.js` (1552 lines, 110 routes) broken into one router file per
> business capability, **in place, with every URL unchanged**. This is the actual driver.
---
## Why the auth merge is out
The original plan replaced httpOnly session cookies with a bearer JWT + rotating refresh token for
every client, so web and mobile would share one session model. Reasons that no longer hold up:
1. **The current model is the more secure one.** httpOnly + SameSite cookies are unreadable from JS
and carry CSRF protection by default. Every migration target is a sideways or backwards move:
- Refresh token in `localStorage` → any XSS becomes **persistent full account takeover**, not a
bounded access-token window. A short access TTL does not help; the attacker mints new pairs.
- Refresh token in an httpOnly cookie scoped to the refresh endpoint → safe, but that is
*cookies with extra steps*. It concedes the premise.
2. **"One session model everywhere" is already true where it matters.** `auth/session.service.js`
unifies cookie and bearer into a single session, and `auth/token.js` already extracts from either
`Cookie` or `Authorization: Bearer`. That abstraction is written, working, and paid for. The merge
would move complexity *out* of `extractToken` and *into* the SPA.
3. **The cookie codepath survives the merge anyway.** The SSO / email-connect redirect flow must keep
its short-lived httpOnly tx / PKCE-verifier / pending-TOTP cookies — the browser leaves for the
IdP and returns with no JS context. So the merge never actually delivered "cookies are gone."
4. **It carried the plan's most bug-prone work as a dependency.** The admin SSE rewrite
(`EventSource` → hand-rolled `fetch` + `ReadableStream` + SSE frame parser + reconnect/backoff +
refresh-on-401) existed *only* to serve the bearer model. Without the merge, the admin stream stays
on `EventSource` with `withCredentials` and that code is never written.
5. **It is a contract change with no user-visible payoff**, competing for the same review attention as
the domain split, which is the thing that actually hurts today.
### Dropped with it
- v2 bearer auth routes (`/api/v2/auth/{login,refresh,logout,login/totp}`).
- Deletion of the `setAuthCookie` / `clearAuthCookie` path.
- `rg_trust` cookie → `X-Trust-Token` header migration for web (the header stays available for native
clients via `extractTrustToken`, unchanged).
- `api/client.js` bearer + silent-refresh rewrite.
- `lib/useShardFeed.js` admin-stream fetch rewrite. **The admin SSE stream stays as-is.**
### Kept from it
- **CSP hardening** — now its own phase (below). It was justified as a compensating control for a
JS-held token; it is worth doing regardless, just no longer urgent.
- **The public/admin SSE allowlist split** — unchanged security boundary, unrelated to session model.
- **The tx-cookie carve-out reasoning** — recorded here so a future merge attempt doesn't rediscover it.
---
## Deferred: the auth merge
Not cancelled — parked behind trigger conditions. Revisit if **any** of these become true:
| Trigger | Why it changes the answer |
|---|---|
| The API becomes genuinely cross-origin (separate API host) | `SameSite` cookies stop being the easy path; bearer becomes the natural model. |
| Third-party or OAuth clients are introduced | Cookies don't serve clients you don't control. |
| Mobile and web session behavior diverge enough to cause real bugs | The unification argument gets teeth it currently lacks. |
If it is ever revived, two specs the original plan lacked must be written **first**:
- **Refresh-token reuse detection.** Rotation is only useful with it: replay of an already-consumed
refresh must revoke the entire token family, not just fail the one request.
- **Rollback procedure.** Once web clients have discarded their cookies, a bad deploy locks everyone
out. Needs a documented path back.
---
## Why there is no `/api/v2`
The domain split reorganizes router *files*. It does not need to move a single URL — because the URL
surface is **already grouped by capability**. Inventory taken from the live Express stack — 196
`/api/v1` routes, plus three outside it (`GET /api/health`, `GET /api/docs.json`,
`GET /.well-known/assetlinks.json`) and 2 on the internal port. Full machine-readable list:
[`api-route-inventory.json`](./api-route-inventory.json).
| Group | Routes | Second segment → capability |
|---|---|---|
| `/admin` | 110 | `shard` 16 · `moderation` 15 · `users` 15 · `wiki` 14 · `posts` 9 · `pages` 7 · `account` 6 · `email` 6 · `uo-link` 5 · `auth` 4 · `invites` 3 · `bot-activity` 2 · `discord-bot` 2 · `settings` 2 · `activity` 1 · `dashboard` 1 · `site-mode` 1 · `uploads` 1 |
| `/auth` | 42 | `me` 23 · `mobile` 5 · `sso` 4 · `password` 3 · `invite` 2 · `login` 2 · `logout` 1 · `providers` 1 · `register` 1 |
| `/public` | 24 | `shard` 12 · `wiki` 4 · `pages` 2 · `posts` 2 · `contact` 1 · `settings` 1 · `status` 1 · `version` 1 |
| `/player` | 20 | `account` 8 · `shard` 8 · `appeals` 4 |
Every capability already owns a URL prefix, so each new router file mounts at the prefix it already
owns and the emitted paths are **byte-identical**. No URL change means no contract change, and no
contract change means no reason to mount a parallel version.
Consequences of doing it in place:
- No `/api/v2`, no dual mount, no route-by-route migration, no v1-usage telemetry project, and no
v1-retirement sequence.
- `BASE = /api/v1` in `client/src/api/client.js` never changes. The Discord bot's `SITE_PUBLIC_URL`
never changes. The Android app is untouched.
- [API_V2_SKELETON.md](./API_V2_SKELETON.md) (the `router/v2/` scaffold) is **superseded and not
scheduled**. It is kept as the concrete recipe if a real contract break ever forces a versioned API.
**Tripwire:** if any endpoint turns out to *need* a new URL, that is a contract change, not a
refactor. List it explicitly, and reopen the versioning question before writing the code — do not
smuggle a URL change into a "mechanical" PR.
---
## Deferred: the `/api/mobile` facade and the app-version floor
The earlier plan's Phase 0 stood up a version-agnostic `/api/mobile` namespace and migrated the
Android app onto it, plus an app-version header and a server-side min-version floor.
**Why it was proposed:** the app hardcodes **69** distinct `api/v1/…` paths (`data/api/*.kt`,
`core/net/ShardStreamClient.kt`, `core/net/HostSelectionInterceptor.kt`,
`core/auth/sso/SsoAuthManager.kt`), has no version negotiation and no force-update, and installs in
the wild cannot be forced forward. Under a parallel-`/api/v2` plan that made the app the load-bearing
coupling: v1 could not be retired until the fleet aged out.
**Why it is deferred:** with the split done in place, no URL moves and nothing is being deleted — so
there is no fleet to sunset and no coupling to break. A facade would add ~70 permanently maintained
delegate routes plus a contract-test suite to solve a problem that does not currently exist. The
version floor was scoped to sunsetting the pre-facade fleet, so it goes with it.
**If it is ever revived** (the trigger is the mobile contract genuinely needing to diverge from web —
different response shapes, a mobile-only aggregation endpoint, a real breaking change):
- **Start with the alias mount, not a delegate layer:** `apiRouter.use('/mobile', v1Router)` gives the
app a stable, version-agnostic namespace with identical wiring, identical middleware and zero
per-route maintenance. Build hand-written delegates only for the routes that actually diverge.
- **A facade is a security surface, not a convenience alias.** Any hand-written route must carry the
*same* middleware chain as the route it mirrors (`requireAuth`, `staffOnly`/`adminOnly`, validators,
the public/admin SSE allowlist split). A re-exposed admin route missing `adminOnly` is privilege
escalation.
- **It needs contract tests.** The moment the app pins a namespace, its response shapes are a
committed contract; an internal refactor that changes a shape must fail a test before it ships to
installed apps.
- **The mobile SSE stream stays anonymous** under whatever path it gets — the app sends no
`Authorization` header.
---
## Phase 1 — CSP hardening (independent)
Previously bundled with the auth merge as a compensating control for a JS-held token. With no token in
JS, this is **defense in depth on its own merits** — cheap, worth doing, blocking nothing. It has no
dependency on the domain split and can ship at any time.
**Sequencing fix from the original plan:** the old version both "ships with the auth merge" and called
for a one-release report-only soak. Those contradict. Correct order is **report-only first, observe one
release, then enforce** — now trivially satisfiable since nothing waits on it.
The app already ships a tuned policy (`server/src/app.js`). Two directives are load-bearing and are
**already correct** — the job is to keep them that way:
- `script-src 'self'` — no `'unsafe-inline'` / `'unsafe-eval'`. Primary defense.
- `connect-src 'self'` — the exfiltration channel. Don't widen it unless the API genuinely becomes
cross-origin (which would also reopen the auth-merge question — see the trigger table).
`style-src 'unsafe-inline'` stays — it permits inline styling, not script execution, and React's
pervasive `style={{…}}` attributes can't be nonce'd. Not a meaningful hole. The `/api/docs` route keeps
its deliberately looser policy (swagger-ui injects an inline bootstrap script); that carve-out is
scoped to the one route and stays scoped.
Target enforced policy:
```
default-src 'self';
script-src 'self';
connect-src 'self';
img-src 'self' data: https:;
style-src 'self' 'unsafe-inline'; /* + fonts.googleapis.com only until fonts are self-hosted */
font-src 'self'; /* + fonts.gstatic.com only until fonts are self-hosted */
object-src 'none';
base-uri 'self';
form-action 'self';
frame-ancestors 'none';
```
Delta vs. the policy in `server/src/app.js` today. This was written as two directives; on
implementation it turned out to be **one**:
- ~~**Add `form-action 'self'`** (currently absent)~~ — **it was not absent.** The directives object in
`app.js` does not list it, but the middleware is configured `useDefaults: true`, and helmet's default
set already supplies `form-action 'self'` — so the header served in production has carried it all
along. Verified by capturing the live `Content-Security-Policy` header from the running app rather
than reading the config, which is how the plan got this wrong. **No behavioural change here.** It is
now written out explicitly in `config/csp.js` anyway: a security directive should not depend on a
third-party library's defaults surviving its next major version.
- **Tighten `frame-ancestors`** `'self'``'none'` — nothing legitimately frames the site. **This is
the entire behavioural delta of the phase.**
- Unchanged: `default-src`, `script-src`, `connect-src`, `object-src 'none'`, `base-uri 'self'`, and
`img-src … https:` (external `BRAND_*` logo/hero and `<img>` in sanitized wiki/news bodies rely on
`https:`).
The soak is still worth running for that one directive, and arguably it is the directive that most
needs one: a `frame-ancestors` report is generated by the browser of *whoever framed the site*, so it
is the only way to discover that something legitimately embeds us before the enforcing policy breaks
it. Nothing else can tell us that.
**Rollout:** ship via `Content-Security-Policy-Report-Only` with `report-to` for one release, watch for
violations, then flip to enforce.
Before trusting `script-src 'self'`: Vite's build injects an inline modulepreload-polyfill `<script>`
into `dist/index.html`, which that directive blocks (harmless, but throws a violation).
**Already handled**`client/vite.config.js` sets `modulePreload: { polyfill: false }`, so the build
emits no inline bootstrap script. (The `renderIndexHtml` branding injection adds only `<meta>`/`<link>`
tags — no inline script, no nonce needed.)
### Where reports go
`report-to` needs somewhere to point, so the report-only PR stands up a same-origin sink:
**`POST /api/csp-report`** (`server/src/router/cspReport.controller.js`, wired in `app.js`). Same-origin
on purpose — violation reports describe attacks against this site and are not handed to a third-party
collector. It writes to the `csp` log tag and stores nothing.
It is mounted outside `/api/v1`, alongside `/api/health`: the browser learns the path from the policy
header, never from a client build, so it is not part of the versioned client contract. This is the
`+1` in the route manifest that made PR 0 go first (see § Sequencing).
Necessary properties, since it is an unauthenticated public `POST` (browsers send reports with no
session, and gating it would silence exactly the anonymous visitors worth hearing about):
- **Both wire formats.** `report-uri` (Firefox, Safari) sends `application/csp-report` with a single
hyphenated-key object; `report-to` (Chrome) sends `application/reports+json` with an array of
camelCase envelopes. Handling one silently drops half the browsers. Both directives are emitted, and
`report-to` additionally needs a `Reporting-Endpoints` response header or it is inert.
- **Always 204, even for junk.** A 4xx would make the global error handler write an ERROR line quoting
the attacker-supplied body — turning an open endpoint into a log-flood primitive. A browser cannot
act on an error from a report sink anyway.
- **Bounded everywhere:** 16 KB body cap, a per-IP rate limit, a fixed field allowlist, and every
logged field truncated (`script-sample` is attacker-influenced and can carry a whole inline script).
**Retiring it:** the sink exists for the soak. When the tightened policy flips to enforced and the
report-only twin is deleted, this endpoint goes with it — *unless* a `report-to` group is deliberately
kept on the enforced policy, which is a reasonable thing to want. Decide that in the enforce PR rather
than leaving an orphan route behind.
Tracked follow-ups (own PRs):
- **Self-host the Cinzel font** → drop `fonts.googleapis.com` from `style-src` and `fonts.gstatic.com`
from `font-src`, removing two third-party origins from the trust surface.
- **Trusted Types** — `require-trusted-types-for 'script'` + a `trusted-types` policy, report-only
first. Audit `dangerouslySetInnerHTML` + the `sanitizeHtml` render path first.
---
## Phase 2 — The domain split
**This is the reason the plan exists.** Everything else is supporting work.
**Rule:** one router file = one business capability; the URL names the domain; related endpoints live
together regardless of HTTP method; no generic `admin.routes.js` catch-all. Controllers are **already**
domain-split — this re-wires routes, not logic.
**Invariant:** each capability router mounts at the prefix it already owns, so the emitted URL set does
not change. Proved per PR by the route manifest (§ PR 0).
Target tree — derived from the inventory above, **inside `router/v1/`** (no `v2/` directory):
```
router/v1/
admin/
index.js # mounts the capability routers below under /admin, keeps the
# shared `noindex, isLoggedIn, staffOnly` gate in one place
users.router.js account.router.js invites.router.js
authProviders.router.js moderation.router.js botActivity.router.js
posts.router.js pages.router.js wiki.router.js
uploads.router.js shard.router.js uoLink.router.js
email.router.js discordBot.router.js settings.router.js
activity.router.js # the staff audit log — landed in PR 2, not with dashboard
dashboard.router.js # + the /site-mode singleton
auth/
login.router.js register.router.js password.router.js invite.router.js
sso.router.js mobile.router.js me.routes.js (already split, 23 routes)
public/
news.router.js (posts) pages.router.js wiki.router.js shard.router.js
site.router.js # status, settings, version, contact
player/
account.router.js shard.router.js appeals.router.js
internal/ (unchanged — stays on the unpublished port, never mounted publicly)
```
Steps:
1. **Land PR 0 (route manifest) first** — the mechanical proof that later PRs move no URL.
2. Carve `admin.routes.js` into the per-capability files above, each requiring its already-existing
controller. `admin/index.js` keeps the shared gate (`noindex, isLoggedIn, staffOnly`) and mounts
each capability router at its existing prefix; the `adminOnly` / `modAccess` gates move with the
routes that use them.
3. Split `public.routes.js`, `player.routes.js`, and the remaining `auth.routes.js` groups the same
way. `auth/me.routes.js` is already a separate file and stays.
4. Keep `/internal` off the public listener exactly as today (separate `internalApp.js` port).
5. Move each route's `#swagger.*` annotations **with** the route, then regenerate
(`cd website/server && npm run swagger`).
6. Update `BACKEND_DESIGN.md` §2 (folder structure) and §4 (API contract — it names
`admin.routes.js → admin.controller.js` and friends) as routers move, plus `PROJECT_TREE.md`.
Because the auth model is untouched and the URLs are frozen, each PR is a **pure mechanical refactor
with a green test suite and a zero-diff route manifest as its acceptance criteria** — which is what
makes grouped PRs actually reviewable.
> **Step 6 correction:** `PROJECT_TREE.md` is no longer hand-edited. Since website#98 it is
> auto-generated by the `sync-project-tree` CI workflow, which opens its own docs PR after a merge to
> `main`. Leave it alone in split PRs. `BACKEND_DESIGN.md` §2/§4 are still manual.
### PR 1 — as landed
`admin/index.js` owns the shared `noindex, isLoggedIn, staffOnly` gate and the mount table, and
declares no routes itself. The gate sits **ahead of every mount**, so a capability router extracted in
a later PR cannot silently ship without it. Route counts:
| Router | Routes | Prefix | Extra gate |
|---|---|---|---|
| `account.router.js` | 6 | `/admin/account` | none — self-service, an editor manages their own 2FA |
| `users.router.js` | 15 | `/admin/users` | `adminOnly` at router level |
| `invites.router.js` | 3 | `/admin/invites` | `adminOnly` per route |
| `authProviders.router.js` | 4 | `/admin/auth` (routes are `/providers[/:id]`) | `adminOnly` per route |
| `admin.routes.js` (residual) | 82 | group root, mounted last | unchanged |
6 + 15 + 3 + 4 + 82 = the 110 inventoried admin routes. None of the four prefixes appears in the
residual file, so nothing depends on mount ordering.
Three findings worth carrying into PRs 25:
- **The self-service `/shard/*` routes stay with `shard` (PR 4), despite their `Admin · Account`
swagger tag.** The invariant is *prefix ownership*, not tag agreement: one router owns `/shard`, so
splitting six routes off it by capability would mean two routers mounting under the same prefix and
an ordering hazard for no gain. Retag them in PR 4 if the tag still grates.
- **`usersRouter.use(adminOnly)` is exactly equivalent to the old `adminRouter.use('/users', adminOnly)`**
now that the router is mounted at `/users` — but only because it is mounted at a prefix. Under a
*pathless* mount, a bare `use(gate)` would run for every request passing through en route to a later
mount, 403-ing an editor on `/admin/posts`. Do not "simplify" a prefix mount away.
- **`routes.guards.json` came back zero-diff too**, not just the manifest — no route lost or gained a
gate. Worth checking both every time; the guards file is the one that would catch a dropped
`adminOnly` that the method+path freeze cannot see.
The OpenAPI spec was also byte-for-byte unchanged, which required a prerequisite fix — see below.
### PR 2 — as landed
`moderation`, `bot-activity` and `activity` — 18 routes, leaving 64 in the residual file.
| Router | Routes | Prefix | Extra gate |
|---|---|---|---|
| `moderation.router.js` | 15 | `/admin/moderation` | `modAccess` (admin + moderator) at router level |
| `botActivity.router.js` | 2 | `/admin/bot-activity` | `adminOnly` per route |
| `activity.router.js` | 1 | `/admin/activity` | none — staff-wide audit log |
| `admin.routes.js` (residual) | 64 | group root, mounted last | unchanged |
All four gates came back zero-diff: `routes.manifest.json` (200 public + 2 internal),
`routes.guards.json`, `swagger-output.json` (198 operations), and `docs/website/api-route-inventory.json`
was already in sync. 434 server tests green.
Notes:
- **`/activity` gets its own file, deviating from the target tree above**, which parked it as a
singleton inside `dashboard.router.js`. It is folded into PR 2 by the sequencing list, and PR 4 is
where `dashboard` lands — so honouring the tree would have meant leaving one route in the residual
file for two PRs to satisfy a filename. It is also a genuinely separate capability: `/activity` is
the **staff audit log** (`activity.model.js`), while `/dashboard` is a stats overview and
`/bot-activity` is the botScore middleware's in-memory ban state. Three different things that read
alike. **PR 4 mounts `dashboard` and `site-mode` only.**
- **`modAccess` moved to a router-level `use`; `adminOnly` on bot-activity deliberately did not.**
Moderation was already gated by a *prefix* mount (`adminRouter.use('/moderation', modAccess)`), so
`moderationRouter.use(modAccess)` is the exact equivalent (the PR 1 `users` case). Bot-activity's
gate was per-route, and keeping it per-route is what holds the per-route handler count — the one
number in `routes.guards.json` that would catch a dropped `adminOnly`, since `requireRole(...)`
returns an anonymous arrow and never shows up by name. **Rule for PRs 35: move a gate to router
level only where it was already a prefix mount; otherwise leave it on the route.**
- **`modAccess` stays in the residual file** — the `/shard/*` in-game staff operations still use it
and do not move until PR 4. Its comment there was retargeted rather than deleted.
### PR 3 — as landed
`posts`, `uploads`, `wiki` and `pages` — 31 routes, leaving 33 in the residual file. The content tier,
and the first split PR where no gate moved at all: all four capabilities are editor-tier, so the shared
`staffOnly` in `admin/index.js` is their whole gate.
| Router | Routes | Prefix | Extra gate |
|---|---|---|---|
| `posts.router.js` | 9 | `/admin/posts` | none — editor tier |
| `uploads.router.js` | 1 | `/admin/uploads` | none — editor tier |
| `wiki.router.js` | 14 | `/admin/wiki` | none — editor tier |
| `pages.router.js` | 7 | `/admin/pages` | none — editor tier |
| `admin.routes.js` (residual) | 33 | group root, mounted last | unchanged |
All four gates zero-diff: `routes.manifest.json` (200 public + 2 internal), `routes.guards.json`,
`swagger-output.json` (198 operations), and `docs/website/api-route-inventory.json` was already in
sync. 434 server tests green.
Notes:
- **The residual 33 is exactly PR 4's list** — `shard` 16, `email` 6, `uo-link` 5, `settings` 2,
`discord-bot` 2, `dashboard` 1, `site-mode` 1. So `admin.routes.js` is deleted by PR 4, one PR
earlier than the sequencing list implies, and PR 5 touches only `public/*`, `player/*` and `auth/*`.
- **A shared module was unavoidable here, and it is the first one in the split.** The multer config
(upload dir, mimetype→extension allowlist, 8 MB cap) was defined inline in `admin.routes.js` and used
by *two* routes that this PR puts in different files: `POST /posts/upload` (→ `{image_url}`) and
`POST /uploads` (→ `{url}`). It moved to `admin/imageUpload.js` rather than being duplicated —
duplicating a security allowlist is how the two copies drift. It stays in `admin/` deliberately:
`UPLOAD_DIR` is resolved `__dirname`-relative, so relocating the file would silently repoint the
upload directory. Guard freshness is unaffected — multer's middleware is named `multerMiddleware`
wherever it is constructed, so `routes.guards.json` did not move.
- **`POST /uploads` keeps its `Admin · Posts` swagger tag**, which now disagrees with its filename. The
acceptance criterion is a byte-identical spec, so retagging is a real OpenAPI diff and does not
belong in a route-move PR. Same call as PR 1's `/shard/*` tag mismatch: fix tags in a PR that is
*about* tags.
- **The wiki router is the first one with load-bearing intra-file route order.** `/categories` and
`/tags` are literal paths that must stay ahead of `/:slug`, or `GET /admin/wiki/categories` gets
dispatched as a page whose slug is "categories". **The manifest cannot catch this — it sorts its
entries, so a reordering is invisible in all three gates.** It was verified separately by
introspecting the built router stack and asserting the last literal layer precedes the first `/:slug`
layer. Any future PR moving `/:slug`-style routes needs the same explicit check.
- **`/admin/pages` (CMS page builder) and `/admin/shard/pages` (in-game help-page queue) are unrelated
capabilities that read alike** — the latter stays with `shard` in PR 4. Same trap as PR 2's
`activity` / `dashboard` / `bot-activity` trio.
### PR 4 — as landed
`shard`, `uo-link`, `email`, `discord-bot`, `settings` and `dashboard`/`site-mode` — the whole residual
33. **`admin.routes.js` is deleted**, so the admin group is fully split and every one of its 110 routes
is declared in a capability router.
| Router | Routes | Prefix | Extra gate |
|---|---|---|---|
| `shard.router.js` | 16 | `/admin/shard` | none on the 7 self-service routes; `modAccess` per route on the 9 staff ops |
| `uoLink.router.js` | 5 | `/admin/uo-link` | `adminOnly` per route |
| `email.router.js` | 6 | `/admin/email` | `adminOnly` per route |
| `discordBot.router.js` | 2 | `/admin/discord-bot` | `adminOnly` per route |
| `settings.router.js` | 2 | `/admin/settings` | `adminOnly` per route |
| `dashboard.router.js` | 2 | **group root** (`/dashboard`, `/site-mode`) | `adminOnly` per route on `/site-mode` only |
| ~~`admin.routes.js`~~ | — | deleted | — |
16 + 5 + 6 + 2 + 2 + 2 = 33. All four gates zero-diff: `routes.manifest.json` (200 public + 2
internal), `routes.guards.json`, `swagger-output.json` (198 operations), and
`docs/website/api-route-inventory.json` was already in sync. 434 server tests green.
Notes:
- **`dashboard.router.js` is mounted at the group root, not a prefix — the one relaxation of the
"always mount at a prefix" rule, and it is deliberate.** `GET /dashboard` and `PUT /site-mode` own no
common path segment, so a prefix mount would mean two one-route files instead of the single file the
target tree calls for. It is safe **only** because the file declares no router-level middleware: a
bare `use(gate)` in a root-mounted router runs for every request passing through toward another
mount and would 403 an editor on an unrelated route (the PR 1 finding). The file says so in a
comment, because the next person to add a gate there is the one who needs to know.
- **`/shard` is the first prefix where two tiers share one router**, and it is why prefix ownership
beats swagger-tag grouping. The 7 self-service routes (`link`, `accounts`, `roster/:account`,
`vendors/:account`, `char/:serial`, `sales`, `POST account`) are tagged `Admin · Account`, run with
no gate beyond the shared `staffOnly`, and are served by the very same `player/shard.controller`
handlers as `/player/shard` — staff are a superset of players, and the controller keys off
`req.user.id`. The 9 in-game ops are tagged `Admin · Shard` and carry `modAccess`. Splitting them by
tag would put two routers under one prefix for no gain; instead one router owns `/shard` and gates
per route. The tag mismatch stays, on the PR 1 and PR 3 precedent: retagging is a real spec diff and
belongs in a PR that is about tags.
- **No gate moved to router level anywhere in this PR.** Every `adminOnly` in the residual file was
per-route, and `modAccess` on `/shard` must stay per-route because half that router must *not* have
it. This keeps the per-route handler count intact — the one number `routes.guards.json` can actually
check, since `requireRole(...)` returns an anonymous arrow.
- **The `/:param` shadowing check was run again and is clean**, since the manifest sorts and therefore
cannot see declaration order. Introspecting the built stack, all 110 admin routes and all 59 literal
admin paths dispatch to their own layer — nothing is captured first by a `:param` sibling. The
near-misses worth naming: `GET /shard/pages` (help-page queue) sits alongside
`POST /shard/pages/:id/respond|close`, and `POST /shard/towncrier` alongside
`DELETE /uo-link/towncrier/:id` — different depths and methods, so neither collides.
- **Deleting the file left dangling `see admin.routes.js` pointers**, which were repointed in the same
PR: `botActivity.controller.js``botActivity.router.js`, `moderation.controller.js`
`moderation.router.js`, `announceJobs.logic.js`'s town-crier cap mirror → `admin/uoLink.router.js`,
and the "route paths sit on the line *after* `adminRouter.get(`" rationale in
`scripts/routeManifest.js`, `README.md` and `pr-checks.yml` was generalized (it was never about that
one file).
- **`/admin/shard/pages` vs `/admin/pages` stayed separate**, as PR 3 flagged: the former is the
in-game help-page (support) queue and belongs to `shard`; the latter is the CMS page builder.
### The swagger path-normalization prerequisite (landed before PR 1)
swagger-autogen builds a path by string-concatenating the mount prefix with the route argument, so a
capability router mounted at `/users` whose collection route is `router.get('/')` documents as
`/api/v1/admin/users/` — advertising a URL no client calls while dropping the one the SPA, the Android
app and the Discord bot all do. Express is indifferent; the published spec is not. It also emits path
keys in *router-traversal order*, so moving a route between files rewrote most of the ~5k-line
committed artifact even when the API was provably unchanged — burying the one line a reviewer needs.
Both are fixed once in `server/swagger/swagger.js`, which post-processes the generator's output to
strip trailing slashes and sort path keys (throwing on a collision rather than silently dropping an
operation). It shipped as its own PR ahead of PR 1, verified inert by the regenerated spec being
byte-for-byte the sorted form of the previously committed one — 198 operations, none added or removed.
`#swagger.path` was rejected as the fix: it bypasses the mount prefix, so every route would hardcode
its absolute path in a comment that silently lies the moment a mount moves.
**Consequence for PRs 25: the swagger diff is now a signal.** With sorting in place, a pure route
move produces *no* spec diff at all, so any diff there means an annotation actually changed. Treat
`swagger-output.json`, `routes.manifest.json` and `routes.guards.json` as three zero-diff gates.
### 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.
- **Generator:** `server/scripts/routeManifest.js`, wired as `npm run routes:manifest`. It requires
`src/app.js` (which exports the app and neither listens nor connects to the DB — `server.js` owns
those), walks `app._router.stack` recursively through mounted routers, reconstructs each full path
from the layer regexps, and writes a **sorted** array of
`{ "method": "GET", "path": "/api/v1/admin/users/:id" }` to `server/routes.manifest.json`.
`internalApp.js` is walked into a separate `internal` section so the unpublished port is inventoried
without being confused for public surface.
- **Scope it to the API surface, or it won't be deterministic.** Three mounts are *filesystem*
conditional: the SPA catch-all `GET *` (only when `client/dist/index.html` exists), the `/brand`
static mount, and `/api/docs*` (only when `swagger-output.json` is present — it is committed, so it
is stable). The manifest keeps only `/api/**`, `/.well-known/**`, and the internal app's routes, so
it does not change depending on whether CI built the client. Static mounts are not API contract.
- **The baseline already exists:** [`api-route-inventory.json`](./api-route-inventory.json) in this
directory is the frozen surface — **199 API routes** (110 of them `/api/v1/admin`) plus 2
internal at the time PR 0 was written. PR 0's generator must **reproduce this file byte-for-byte**;
that is PR 0's own acceptance test, and it means the freeze is already in effect before the first
router moves. *(It did, on first run. The file has since moved to **200** — the CSP report-only PR
added `POST /api/csp-report`, the first deliberate, reviewed manifest diff.)*
- **Runtime introspection, not source parsing.** It is authoritative about mounts, and the route paths
in `admin.routes.js` sit on the line *after* `adminRouter.get(`, which defeats naive greps.
- **Not `swagger-output.json`.** That is annotation-derived (only annotated routes appear) and churns
for unrelated reasons; it documents intent, the manifest records reality.
- **Frozen key: method + path only.** That is exactly the contract being preserved. Handler names are
useless as a guard check here — `requireRole(...)` returns an anonymous arrow, and router-level gates
like `adminRouter.use(noindex, isLoggedIn, staffOnly)` never appear in a route's own stack.
- **Guard coverage, separately.** The generator also emits a non-gated review aid: per route, the
handler count plus any *named* middleware collected along the mount chain. If a stable behavioral
check proves cheap, prefer it — a test that fires an **unauthenticated** request at every manifest
path and snapshots the status code catches a dropped `adminOnly` (403 → 200) in a way names cannot.
Try it in PR 0; if DB-touching public routes make it slow or noisy against the dead-port pool the
tests use, drop it rather than ship a flaky gate.
- **CI:** `.gitea/workflows/pr-checks.yml` runs `npm run routes:manifest` and
`git diff --exit-code server/routes.manifest.json`. A PR that moves a URL fails unless it
deliberately commits the new manifest — which puts the URL change in front of a reviewer instead of
letting it pass silently.
- **Published copy:** `docs/website/api-route-inventory.json` mirrors `server/routes.manifest.json` and
is refreshed in each split PR's mandatory docs edit. The markdown table above is orientation for a
human reader; **the manifest is the authoritative freeze.**
---
## Sequencing & PR breakdown
CSP and the split are independent; the only hard ordering is PR 0 before the first split PR.
**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` — the first deliberate, reviewed manifest diff). ✅ landed
3. **PR — CSP enforce.** One release later, assuming a clean violation report. **Blocked on real soak
data**, not on code: watch the `csp` log tag for `frame-ancestors` reports across one release before
flipping. Also decide there whether `/api/csp-report` is retired with the report-only twin or kept
as a `report-to` group on the enforced policy.
4. **PR 1 — admin:** `users`, `account`, `invites`, `auth` (providers). ✅ landed
5. **PR 2 — admin:** `moderation`, `bot-activity`, `activity`. ✅ landed
6. **PR 3 — admin (content):** `posts`, `pages`, `wiki`, `uploads`. ✅ landed
7. **PR 4 — admin (ops/config):** `shard`, `uo-link`, `email`, `discord-bot`, `settings`, `site-mode`,
`dashboard`. (`activity` went with PR 2 — see § PR 2 — as landed.) **This is the whole residual
file** — `admin.routes.js` is deleted here, not by PR 5. ✅ landed
8. **PR 5 — `public/*` + `player/*`** (and the residual `auth/*` grouping). **The only split PR left.**
Each PR: **zero-line diff in `routes.manifest.json`**, server tests green
(`cd website/server && npm test`), Swagger regenerated, matching `docs/` edit, Conventional Commit,
AI-disclosure trailer, branch from a freshly-pulled `main`.
---
## Cross-component blast radius
Three independent clients consume the site's HTTP/SSE API, two of them in separate repos on separate
release cadences. **With the auth merge and the version bump both gone, the blast radius is empty**
no client's URLs or authentication change at all.
| Consumer | Repo (cadence) | Pinning | Impact under this plan |
|---|---|---|---|
| Browser SPA | `website/client` (lockstep) | `BASE = /api/v1` in `client/src/api/client.js` | **None.** Same URLs, same cookie session. |
| Android app | `android-app` (app-store cadence, un-updatable installs in the wild) | 69 hardcoded `api/v1/…` paths; SSE path in `ShardStreamClient.kt`; SSO in `SsoAuthManager.kt` | **None.** No repoint, no release required. |
| Discord bot | `website/bot` (separate deploy, env-configured) | `SITE_PUBLIC_URL` env → `/api/v1/public` | **None.** Anonymous public reads on unchanged paths. |
Standing constraints, unchanged:
- **The public SSE stream stays anonymous.** Consumed by logged-out browser visitors *and* the Android
`ShardStreamClient`, neither of which sends an `Authorization` header. Adding `requireAuth` blacks
out the public live boards on web and mobile. The single most likely regression in a careless
refactor is reflexively wrapping *both* shard streams in auth.
- **The admin SSE stream keeps its current cookie-based gating** (`isLoggedIn`, `EventSource` +
`withCredentials`) — the rewrite that would have changed this went out with the auth merge.
- **The public/admin allowlist split is a security boundary**, not an implementation detail. Preserve
it verbatim in `utils/shardBroadcast.js` / `utils/shardIngest.js` as routes move.
- **SSO / email-connect transaction cookies are load-bearing for web *and* native.** The Android SSO
flow opens a Custom Tab to the website's `/auth/…/sso/start` and rides the same server-side redirect
transaction and the same tx cookies. Nothing in this plan touches them; don't let a future "cookies
go away" push delete them.
- **`link/` is out of scope.** The sidecar contract (`uoLinkConfig`, `utils/uoLinkClient.js`,
`utils/shardIngest.js`, `X-UOLink-Version`) is a separate versioning axis. `PROTOCOL_VERSION` does
**not** bump for this work.
---
## Appendix: mapping from the previous plan
| Previous | Now |
|---|---|
| Phase 0 — `/api/mobile` facade + app-version floor | **Deferred.** See § Deferred: the `/api/mobile` facade |
| Phase 1 — auth merge | **Removed.** See § Why the auth merge is out and § Deferred: the auth merge |
| Phase 1b — CSP hardening (shipped with the auth merge) | **Phase 1**, standalone; report-only-first ordering fixed |
| Phase 2 — domain split under `router/v2/` | **Phase 2**, in place under `router/v1/`; now the primary driver |
| PR 1 — `/api/v2` scaffold ([API_V2_SKELETON.md](./API_V2_SKELETON.md)) | **Superseded**, kept as the recipe if a versioned API is ever forced |
| PR final — retire v1 | **Not applicable** — v1 is never replaced |