diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4b2bbbe..cd79951 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,24 @@ jobs: build: name: Typecheck + build runs-on: ubuntu-latest + # Refresh-token rotation is a race between two UPDATEs contending for one row, and the + # decision it turns on is a column in that row. Nothing about it can be tested against a + # stub, so the suite gets a real Postgres; without this it skips its own database cases + # and the only thing left running is the log-serializer half. + services: + postgres: + image: postgres:16-alpine + env: + POSTGRES_USER: nimbus + POSTGRES_PASSWORD: nimbus + POSTGRES_DB: nimbus_test + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U nimbus" + --health-interval 5s + --health-timeout 5s + --health-retries 10 steps: - uses: actions/checkout@v4 - uses: pnpm/action-setup@v4 @@ -20,6 +38,38 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm typecheck - run: pnpm build + # Suites are named one by one rather than run with `pnpm -r test`, so that + # giving a workspace a suite is a deliberate act here and never a script + # that silently does not exist. All three workspaces have one today. + # The database cases skip themselves when NIMBUS_TEST_DATABASE_URL is + # unset, so it is set here and the name must contain "test" — the suite + # refuses any other database, having migrations and a DELETE in it. + - run: pnpm --filter @cloudsforge/nimbus test + env: + NIMBUS_TEST_DATABASE_URL: postgres://nimbus:nimbus@localhost:5432/nimbus_test + # The signing key is encrypted at rest and env.ts refuses to boot + # without a secret to encrypt it under (CF-16), so the suite needs one. + # A literal is right here and only here: this database exists for the + # length of one job and holds nothing but the keys these cases mint. + # It is long enough and is not on env.ts's placeholder list on purpose + # — a CI value that trips the fail-closed check would prove nothing + # except that the check runs. + NIMBUS_KEY_SECRET: ci-only-signing-key-secret-not-a-real-one + # The console's suite needs nothing: withdrawalQueue.ts imports no runtime + # module, which is the whole reason the withdrawals panel's rules are + # testable at all. It asserts WHICH REQUEST the panel sends — the defect + # CF-33 fixed was never in the arithmetic, so a test of the arithmetic + # would have passed against it. + - run: pnpm --filter @cloudsforge/admin test + # The marketing site's suite is the only thing in the estate that holds its + # PUBLIC CLAIMS to the code they describe: it reads EMBER's confirmation + # depth out of `@cloudsforge/shared`'s deposit registry — the same module + # forge-pay's watcher reads — and fails if the copy quotes a different + # number, or if a retired claim ("credits at the chain tip", "cosmetics are + # stored in your own browser") comes back. It needs no database and no + # browser. If it goes red after a shared-libs bump, the site is out of date, + # not the test. + - run: pnpm --filter @cloudsforge/site test images: name: Docker images diff --git a/MAP.md b/MAP.md index 0cb398c..7a37729 100644 --- a/MAP.md +++ b/MAP.md @@ -58,14 +58,47 @@ A single audience means any product's token is valid at every other product. There is no per-product scoping and no per-product client id. The role claim is the only authorisation carried in the token. -**The signing key lives in Postgres, not in a file or an env var.** First boot -generates an extractable RS256 pair, mints an 8-byte hex `kid` and persists both -JWKs to `signing_keys` (`services/nimbus/src/keys.ts:36-51`); every later boot -loads and caches it (`:23-33`). `getSigningKey()` is called during startup -(`services/nimbus/src/index.ts:79`) so the key exists before the first request. -`getJwks()` publishes exactly one key (`services/nimbus/src/keys.ts:58-61`) — -**there is no rotation path**: no second key, no `onConflictDoNothing` recovery -that would produce two, and no expiry. +**The signing key lives in Postgres, encrypted, not in a file or an env var.** +First boot generates an extractable RS256 pair, mints an 8-byte hex `kid`, and +persists the public JWK as `jsonb` and the private one as an AES-256-GCM +envelope in `private_jwk_enc` (`services/nimbus/src/keys.ts` `mint`, +`services/nimbus/src/keyEnvelope.ts`). The envelope is scrypt-derived from +`NIMBUS_KEY_SECRET`, which nimbus refuses to boot without and rejects when it is +a known placeholder or under 24 characters (`src/env.ts` `requireKeySecret`) — +the same shape and the same fail-closed rule as ForgeKeyvault's +`KEYVAULT_MASTER_SECRET`, deliberately, because two envelope formats in one +estate is one format nobody can read in an incident. + +Until CF-16 the private key was plaintext `jsonb` in a database eight services +share one Postgres role on, and it is the estate's universal forging credential: +every service checks `iss` plus `aud: 'cloudsforge'` and nothing else, so a +self-minted `{sub, roles:['admin']}` is admin in all of them. What the envelope +closes is the read-only vector — a stolen backup, a `SELECT`-only injection, a +copied dump — where the secret is not in the artifact. It does not help against +code execution in a container, which has `.env`. A boot that finds a plaintext +row re-encrypts it and NULLs the column in one statement +(`keys.ts` `encryptStoredSigningKeys`, wired at `src/index.ts` as the +`encrypt-signing-keys` startup step) and warns that older backups still carry +the key. + +**Rotation.** `signing_keys.status` is `active` (the one key that signs), +`published` (in the JWKS, signing nothing) or `retired` (out of the JWKS). +`getJwks()` publishes every non-retired key in a fixed order — active first, +then oldest, then `kid` — so the document is byte-identical across instances, +and `getSigningKey()` returns the single `active` row, memoised for 30 seconds +rather than for the process lifetime. A rotation is three admin calls: + +| Method | Path | Effect | +| --- | --- | --- | +| GET | `/admin/signing-keys` | kid, status and timestamps. Never key material | +| POST | `/admin/signing-keys` | mints a key as `published` — in the JWKS, signing nothing | +| POST | `/admin/signing-keys/:kid/activate` | it starts signing; the old signer becomes `published`. **409 `publish_window`** until the key has been published for 20 minutes (one access-token TTL plus margin) — activating sooner mints tokens under a `kid` no consumer has fetched, and every service 401s until its cache turns over | +| POST | `/admin/signing-keys/:kid/retire` | drops it from the JWKS. 409 while it is the active key | + +Losing the key is survivable and was overstated as an estate-wide logout: access +tokens are 15 minutes and refresh tokens are opaque SHA-256 rows rather than +signed (`tokens.ts:19-29,40-48`), so the worst case is a 15-minute window of +401s that clients recover from through `/auth/refresh`. **Passwords** are scrypt with a 16-byte random salt, stored `salt:hash` hex (`services/nimbus/src/passwords.ts:9-13`), verified in constant time @@ -93,20 +126,48 @@ SHA-256 hex (`services/nimbus/src/tokens.ts:32-34,42-48`). Every rotation inherits the previous row's `familyId`; a fresh login omits it and the column default starts a new family (`:41`, `services/nimbus/src/db/schema.ts:24-26`). -`rotateRefreshToken` (`services/nimbus/src/tokens.ts:66-99`): - -1. Conditional `UPDATE … RETURNING` sets `revoked = true` where the hash - matches **and** `revoked = false` **and** `expiresAt > now` (`:68-78`). Two - concurrent rotations cannot both win — one gets zero rows. -2. A winning row issues a successor in the same family (`:82`). -3. Zero rows: look the hash up unconditionally (`:86-91`). Absent or expired → - `invalid` (`:92`). Present, unexpired and already revoked → **replay**: every - unrevoked token in that `familyId` is revoked (`:94-97`) and the result is - `reuse` (`:98`). Both the thief and the victim are forced back to a real - login. The route logs it and answers 401 (`services/nimbus/src/routes/auth.ts:195-200`). +`rotateRefreshToken` (`services/nimbus/src/tokens.ts:119-168`): + +1. Conditional `UPDATE … RETURNING` sets `revoked = true` **and** + `rotatedAt = now` where the hash matches **and** `revoked = false` **and** + `expiresAt > now` (`:125-136`). Two concurrent rotations cannot both win — + one gets zero rows. +2. A winning row issues a successor in the same family (`:139`) and returns + `concurrent: false`. +3. Zero rows: look the hash up unconditionally (`:143-147`). Absent or expired → + `invalid` (`:149`). +4. Present, unexpired, already revoked, and rotated within `ROTATION_GRACE_MS` + (10s, `:92`) → **two tabs refreshing at once**, not theft: a second token is + issued into the same family and nothing is revoked (`:151-160`, + `concurrent: true`). The route logs that at `info` + (`services/nimbus/src/routes/auth.ts:203-214`). The successor cannot be + handed back instead, because only its SHA-256 was kept. +5. Present, unexpired, already revoked, and either rotated longer ago than that + or never rotated at all — logout, a password change, an earlier burn all + leave `rotatedAt` null (`db/schema.ts:29-35`) → **replay**: every unrevoked + token in that `familyId` is revoked (`:162-165`) and the result is `reuse` + (`:166`). Both the thief and the victim are forced back to a real login. The + route logs it at `warn` and answers 401 (`routes/auth.ts:196-202`). + +Step 4 is CF-36. A refresh token lives in `localStorage`, which every tab of an +origin shares, and each client's single-flight guard is per-document — so two +tabs restored together present the same token milliseconds apart. Without the +window that is indistinguishable from a replay, and the burn took the winner's +brand-new successor with it: both tabs signed out mid-work and a false theft +alert in the operator log. `services/nimbus/src/tokens.test.ts` races two +rotations against a real Postgres and asserts both survive. + +Step 4's other consequence: a family can hold more than one live token at a time, +so anything that ends a session has to end the family. `revokeRefreshToken` +(logout) therefore looks the presented hash up, takes its `familyId` and revokes +every unrevoked row in it (`tokens.ts:229-241`) — matching the row whether or not +it is itself already revoked, so signing out with a token another tab has since +rotated past still works. Revoking only the presented row, which is what it did +before CF-36, left the sibling from a two-tab race live for the rest of its +thirty days, held by no client and reachable by no user action. `revokeAllRefreshTokens` burns every family a user holds and returns the count -(`services/nimbus/src/tokens.ts:108-115`); it is called on password change +(`services/nimbus/src/tokens.ts:190-197`); it is called on password change (`services/nimbus/src/routes/auth.ts:337`) and password reset (`:415`). ### 2.4 The SSO handshake, step by step @@ -228,14 +289,14 @@ must wire up. | --- | --- | --- | --- | --- | --- | | POST | `/auth/register` | none | Creates a `player`; 409 on duplicate email or handle, including the lost unique-index race | Portal `/register`; nothing else | `:111` | | POST | `/auth/login` | none | Lock-out check → credential check → tokens | Portal `/login`; admin console `api.login` (`apps/admin/src/lib/api.ts:379`, dead in practice — the console redirects to the portal, `apps/admin/src/pages/Login.tsx:22`) | `:150` | -| POST | `/auth/refresh` | refresh token in body | Rotates; burns the family on replay | Every product's HTTP client; portal (`portal.ts:519`); admin (`api.ts:282`); site (`auth.tsx:54`) | `:188` | +| POST | `/auth/refresh` | refresh token in body | Rotates; burns the family on a replay, except one presented within 10s of the rotation that spent it — that is two tabs, and gets its own token (§2.3) | Every product's HTTP client; portal (`portal.ts:519`); admin (`api.ts:282`); site (`auth.tsx:54`) | `:188` | | POST | `/auth/exchange` | code in body **+ `Origin` header** | Redeems the SSO hand-off code for tokens | `consumeAuthCallback()` in every SPA (`shared-libs/packages/ui/src/index.tsx:211`) | `:220` | -| POST | `/auth/logout` | refresh token in body | Idempotent revoke; 204 | Portal `/logout` (`portal.ts:1059`); admin (`api.ts:398`) | `:252` | +| POST | `/auth/logout` | refresh token in body | Idempotent; revokes the whole family the token belongs to, not just that row (§2.3) | Portal `/logout` (`portal.ts:1059`); admin (`api.ts:398`) | `:252` | | GET | `/auth/me` | Bearer | Current `PublicUser`; 404 if the row is gone | Every product; admin (`api.ts:390`), site (`auth.tsx:87`), portal `/account` (`portal.ts:976`) | `:262` | | POST | `/auth/password` | Bearer **+ current password** | Change password; revokes everything, returns a fresh pair | Portal `/account` only (`portal.ts:1001`) | `:277` | -| POST | `/auth/password/forgot` | none | Always 202. Mints a token for a known email, logs a warning for an unknown one | Portal `/forgot` only (`portal.ts:810`) | `:352` | +| POST | `/auth/password/forgot` | none | Always 202, and answered BEFORE the token is minted or the mail sent, so the response time does not say whether the address exists either | Portal `/forgot` only (`portal.ts:810`) | `:352` | | POST | `/auth/password/reset` | reset token in body | Sets the password, revokes every session, clears the lock-out | Portal `/reset` only (`portal.ts:887`) | `:386` | -| GET | `/.well-known/jwks.json` | none | The one public signing key | forge-pay, forge-mint, crucible, forge-keyvault, game, Lantern (see §6) | `:427` | +| GET | `/.well-known/jwks.json` | none | Every non-retired public signing key, active first (§2.1) — one of them except during a rotation | forge-pay, forge-mint, crucible, forge-keyvault, game, Lantern (see §6) | `:427` | ### 3.2 Portal (`src/routes/portal.ts`) — server-rendered HTML, no build step @@ -250,7 +311,7 @@ Inline CSS + vanilla JS driving Nimbus's own `/auth` API on the same origin | GET | `/login?return=` | Resume-or-form. Banner when the return URL was rejected | `:637` | | GET | `/register?return=` | Registration form, then the same hand-off | `:717` | | GET | `/forgot` | Requests a reset. Copy states plainly that nothing sends email | `:778` | -| GET | `/reset?token=` | Sets a new password; strips the token from the address bar with `replaceState` and clears the local portal session before submitting | `:829` | +| GET | `/reset#token=` | Sets a new password. The token is in the **fragment**, so it never reaches the server, any proxy log or a `Referer`; the page reads it from `location.hash` (a `?token=` link minted before that change is still honoured), strips it from the address bar with `replaceState`, and clears the local portal session before submitting | `:829` | | GET | `/account` | Identity, role badges, the launcher (admin-only surfaces hidden for non-admins, `:951-961`), and the **password change form** | `:905` | | GET | `/logout?return=` | Clears local storage, best-effort `POST /auth/logout`, redirects | `:1041` | @@ -319,20 +380,26 @@ alone, spending the "you must be on the box or the compose network" factor (`:99-111`). **Pay proxy → Forge Pay.** A *different* reason, stated so nobody deletes one of -them (`services/nimbus/src/routes/pay.ts:5-44`). Pay **is** routed +them (`services/nimbus/src/routes/pay.ts:5-56`). Pay **is** routed (`deploy/cloudflared/config.example.yml:55`) and `admin.` is on its CORS -allowlist, so the reads would work direct. The **write** would not: pay registers -`@fastify/cors` with no `methods`, so its preflight answers -`access-control-allow-methods: GET,HEAD,POST`, and `PUT /admin/prices/:coin` is -the only way to set the administered EMBER price. Widening that is a change to a -repo this one does not own. +allowlist, so **every** route below would work direct from the console — the +`PUT` included. It did not always: pay's preflight named GET, HEAD and POST only +until forge-pay f3eb408 (2026-07-28) stated its method list and included PUT, and +this paragraph asserted the old behaviour for a day afterwards (CF-46). What is +left is the argument that never depended on a CORS header: one origin, one auth +path and one failure mode for a panel whose every other call is to Nimbus, and no +dependence on the console's own apex being kept on a list inside another service +— a preflight refused there 204s with nothing in either log. A future `PUT` or +`DELETE` on pay's operator surface is one more row in this table, not a second +proxy layer. | Method | Path | Relays to | Line | | --- | --- | --- | --- | -| GET | `/admin/pay/prices` | `GET /admin/prices` | `:109` | -| PUT | `/admin/pay/prices/:coin` | `PUT /admin/prices/:coin`, body rebuilt as `{usd}` only | `:123` | -| GET | `/admin/pay/treasury` | `GET /admin/treasury` | `:148` | -| GET | `/admin/pay/withdrawals?status=` | `GET /admin/withdrawals` | `:151` | +| GET | `/admin/pay/prices` | `GET /admin/prices` | `:125` | +| PUT | `/admin/pay/prices/:coin` | `PUT /admin/prices/:coin`, body rebuilt as `{usd}` only | `:139` | +| GET | `/admin/pay/treasury` | `GET /admin/treasury` | `:164` | +| GET | `/admin/pay/withdrawals?status=` | `GET /admin/withdrawals` | `:167` | +| POST | `/admin/pay/withdrawals/:id/abandon` | `POST /admin/withdrawals/:id/abandon`. **No body** — pay takes the decision from the row and the chain, not from anything the console asserts | `:200` | Both relay the response **as text**, never re-serialised. For pay that is load-bearing: prices are exact decimal strings from fixed-point integer @@ -344,8 +411,15 @@ rather than an HTML page handed to a `JSON.parse` (`vault.ts:75-80`, `pay.ts:91-96`); an unreachable upstream becomes a 502 naming the env var (`vault.ts:33-42`, `pay.ts:47-53`). -Deliberately narrow: nothing here abandons a withdrawal or records a sweep -(`pay.ts:41-43`). +Deliberately narrow: the price write, three reads and **one** remediation. +Nothing here records a sweep (`pay.ts:41-46`). Abandon is the first write on a +withdrawal the console has ever had (CF-34) and it was held back on purpose: +until forge-pay's CF-06 it refunded on a missing receipt, which on an EVM chain +means only "this node has no receipt" — the nonce is unconsumed and the signed +bytes can still be mined. Pay now refuses unless the chain proves those bytes +can never apply, so the relay passes back a 409 far more often than a refund, +and every refusal is relayed **word for word**: pay is the only place the reason +is stated, and each reason implies a different next action (`pay.ts:178-206`). ### 3.5 Everything else @@ -390,6 +464,21 @@ shared product registry (`:123-130`), so product names, order, accents and the "mine it, trade it, mint it, spend it, play in it" line (`:139`) cannot drift from the switcher. +**The public claims are held to the code by a test, not by good intentions.** +`apps/site/src/lib/site.test.ts` is the site's only suite and it asserts nothing +about rendering: it reads EMBER's confirmation depth from +`@cloudsforge/shared`'s deposit registry — the same module forge-pay's deposit +watcher reads — and fails if the hero caveat (`lib/site.ts:253`) or the roadmap +entry (`:406-413`) quotes a different number, and it sweeps every string the module +exports for claims that have been retired. Two of those are there because they +were live on the public site long after the code stopped doing them (CF-37): +EMBER credits at `tip − 60`, not at the tip, and a cosmetic is a database row +fanned out to every roster, not a localStorage entry only you can see. The depth +is written in `lib/site.ts` rather than imported because the shared entrypoint +pulls zod into the bundle and does not tree-shake out (+15 kB gzipped for one +integer, measured) — the test is what makes writing it down safe, so do not +delete it as redundant. + ### 4.2 apps/admin — the operator console Router at `apps/admin/src/App.tsx:16-67`. Every route except `/login` is wrapped @@ -405,7 +494,7 @@ bar, nav and sign-out (`apps/admin/src/components/Shell.tsx:6-11,54-95`). | `/worlds/new` | `CreateWorld` | dashboard button | `POST /admin/worlds` (`api.ts:446`), validated client-side against `createWorldSchema` first (`pages/CreateWorld.tsx:42`) | | `/worlds/:id` | `WorldDetail` | world card | `GET /admin/worlds/:id/stats`, `POST …/start`, `POST …/tick`, `PUT …/bots` (`api.ts:452-473`) | | `/vault` | `Vault` | nav "Vault" | `GET /admin/vault/keys`, `GET /admin/vault/capabilities`, `POST /admin/vault/keys/:address/reveal` — all **through Nimbus** (`api.ts:479-493`). Reveal is a type-`REVEAL`-to-confirm modal (`pages/Vault.tsx:53-70`) | -| `/prices` | `Prices` | nav "Prices" | `GET /admin/pay/prices`, `PUT /admin/pay/prices/:coin`, `GET /admin/pay/treasury`, `GET /admin/pay/withdrawals` (`api.ts:505-533`). Treasury and withdrawals load out-of-band and are not allowed to take the price panel down (`pages/Prices.tsx:410-421`) | +| `/prices` | `Prices` | nav "Prices" | `GET /admin/pay/prices`, `PUT /admin/pay/prices/:coin`, `GET /admin/pay/treasury`, `GET /admin/pay/withdrawals?status=`, `POST /admin/pay/withdrawals/:id/abandon` (`api.ts:555-611`). Treasury and withdrawals load out-of-band and are not allowed to take the price panel down (`pages/Prices.tsx:626-670`); when one of them fails it says so in place of its rows rather than rendering as empty — see §4.4 (`lib/panelRead.ts`). A status chip is a **query, not a filter**, and the "is anything stuck?" line is its own `?status=stuck` read independent of the chip — see §4.3 (`lib/withdrawalQueue.ts`). **Abandon and refund** is a type-`ABANDON`-to-confirm modal on `pending`/`signed`/`broadcast`/`stuck` rows (`pages/Prices.tsx:401-604`) that renders pay's refusal verbatim, in amber rather than red — a refusal there is the chain check working, not a fault | | `/users` | `Users` | nav "Users" | `GET /admin/users`, `GET /admin/password-resets`, `POST /admin/users/:id/password-reset`, `PUT /admin/users/:id/roles` (`api.ts:410-438`) | | `*` | — | — | redirect to `/` (`App.tsx:66`) | @@ -414,7 +503,8 @@ Two service origins only: `HOSTS.nimbus` and `HOSTS.api` (the game service) origin** (`:20-25`, `:495-504`). The request core (`apps/admin/src/lib/api.ts:326-373`) does single-flight -refresh-on-401 (`:274-316`), distinguishes "nothing answered" from "the server +refresh-on-401 (`:274-316`) — per document, so two tabs of the console still +refresh independently and rely on the server's grace window (§2.3) — distinguishes "nothing answered" from "the server said no" (`:230-238`), turns an HTML error page into a status rather than `Unexpected token '<'` (`:250-254`), surfaces `x-request-id` so a user can quote it (`:242,264`), and reports only 0/5xx/non-JSON to Lantern (`:359-370`). A @@ -425,6 +515,75 @@ The roles modal computes `selfLockout` and `lastAdmin` client-side and disables save (`apps/admin/src/pages/Users.tsx:60-64`) — a courtesy over the server-side guards, not a substitute for them. +### 4.3 The withdrawals queue: a count comes from a query, never from a page + +Forge Pay answers `GET /admin/withdrawals` with the newest **200 rows across all +statuses** (its `store.ts` `limit = 200`). The console used to make that one +request and answer everything else in the browser — the status chips filtered the +array and the header counted it — so a stuck withdrawal with two hundred newer +rows behind it fell out of the window and the panel said "Nothing is stuck." and +"No stuck withdrawals." while the same service would have answered +`?status=stuck` with the row (CF-33). + +The rule now, kept in `apps/admin/src/lib/withdrawalQueue.ts` rather than in the +page so it is testable without a DOM: **a count and an absence come from a +request that asked for that status.** + +- Selecting a chip re-requests with `?status=` (`readWithdrawalPage`), latest-wins + by sequence number because a chip is now a round trip rather than a local filter + (`pages/Prices.tsx:648-660`). +- The header's stuck line is its **own** `?status=stuck` read (`readStuckQueue`), + independent of which chip is selected and re-read after an abandon rather than + decremented locally (`:638-640`, `:715-726`). +- That read has three outcomes and not a number — `loading`, `unreadable`, + `known` — because a failed read collapsing into `0` would print the same + reassuring absolute through a different door. Only `known` with a count of zero + is allowed to say "Nothing is stuck."; a full page says "At least 200 are". +- The table's own truncation was never the defect and is not treated as one: the + sentence beside it says which window it is ("The 200 most recent *stuck* + withdrawals, newest first."). There is no page two, and the console does not + pretend there is. + +`apps/admin/src/lib/withdrawalQueue.test.ts` asserts **which request is sent**, +not how an answer is filtered — the arithmetic was never wrong, so a test of the +arithmetic would have passed against the defect. It runs in CI +(`pnpm --filter @cloudsforge/admin test`) and needs no database. + +### 4.4 A failed panel read is an unknown, never an absence + +The same rule one level up, and the one the Prices page broke in two more places +(CF-35). Its two secondary panels load out-of-band so that neither can take the +price panel down — right, and unchanged — but they did it with +`.catch(() => setTreasuries([]))`. The copy hanging off those arrays asserts +facts, so a 502 `pay_unreachable` from the relay, an edge timeout on the +chain-balance-heavy `/admin/treasury` answering with an HTML page, or a 500 from +`/admin/withdrawals` all rendered as **"No treasury addresses configured."** and +**"No withdrawals yet."** — a confident description of a system nothing had been +read from, shown to an operator who had opened the page *because* of the +incident. The error object was dropped too, so there was no trace in the browser +console either. + +Both panels now hold a `PanelRead` (`apps/admin/src/lib/panelRead.ts`): +`loading` | `known` | `unreadable`, the same three states as `StuckQueue` above. + +- `known` is the only state carrying rows, so the empty-state copy is + unreachable from a failure by construction rather than by a guard someone has + to remember (`pages/Prices.tsx:840-857`, `:942-952`, `:1030`). +- `readPanel` never rejects, so "non-fatal" is preserved exactly as it was; what + changed is what it hands back when it fails. +- An `unreadable` panel renders `unreadableNotice`, which states the unknown + before it states the reason — "Treasury addresses could not be read — this is + not a statement that there are none. …" — and ends with the server's own + sentence, unedited, `x-request-id` and all. +- It also writes one `console.error` carrying the `ApiError` itself. That is part + of the fix, not debug residue: the old catches left a support conversation with + no status, no code and no reference to work from. + +`apps/admin/src/lib/panelRead.test.ts` drives the **real** api client through a +stubbed `fetch`, so the sentence under test is the one `lib/api.ts` really builds +from a 502 body. Every case is about what survives a failure — a test of the +happy path would have passed against the defect. + --- ## 5. Data model @@ -437,11 +596,11 @@ table and no down path. | Table | Key | Invariants that matter | Source | | --- | --- | --- | --- | | `users` | `id` uuid | `email` and `handle` both UNIQUE — the index, not the pre-check, decides a race (`routes/auth.ts:134-141`). `roles` is a Postgres `text[]`, default `{}` | `db/schema.ts:5-13` | -| `refresh_tokens` | `id` uuid | `token_hash` UNIQUE — the raw token is never stored. `family_id` NOT NULL, defaults to a fresh uuid, inherited on rotation. Revocation is a conditional UPDATE, never a delete, so a replayed spent token is still findable | `db/schema.ts:15-30`, `tokens.ts:66-99` | +| `refresh_tokens` | `id` uuid | `token_hash` UNIQUE — the raw token is never stored. `family_id` NOT NULL, defaults to a fresh uuid, inherited on rotation. Revocation is a conditional UPDATE, never a delete, so a replayed spent token is still findable. `rotated_at` is written by rotation and by nothing else, which is what separates two tabs racing from a replay | `db/schema.ts:15-37`, `tokens.ts:119-168` | | `auth_exchange_codes` | `code_hash` (PK) | Single-use `redeemed` flag, `redirect_origin` NOT NULL, 60s `expires_at`. Expired rows swept opportunistically on each mint (`exchange.ts:16`) | `db/schema.ts:41-51` | | `password_reset_tokens` | `token_hash` (PK) | Hash only — not even DB access recovers an issued link. `issued_by` FK to `users` `ON DELETE SET NULL`, null = self-service. `used_at` is the single-use marker. Burned on completed reset and on any password change | `db/schema.ts:53-67`, `passwordReset.ts:100-105` | | `login_attempts` | `email` (PK, lower-cased) | **Rows exist for unknown emails**, which is what makes lock-out non-enumerating | `db/schema.ts:32-39` | -| `signing_keys` | `kid` (PK) | Private and public JWK as `jsonb`. Read once and cached in-process (`keys.ts:14,27`) — a row changed underneath a running instance is not picked up | `db/schema.ts:69-74` | +| `signing_keys` | `kid` (PK) | Public JWK as `jsonb`; private JWK as an AES-256-GCM envelope in `private_jwk_enc`. `private_jwk` is the old plaintext column, kept only to be emptied — `SELECT count(*) FROM signing_keys WHERE private_jwk IS NOT NULL` should answer 0. `status` (`active`/`published`/`retired`, CHECK-constrained) with `status_changed_at`; the active key is memoised for 30s, so a rotation lands on every instance within that | `db/schema.ts:69-107`, `keys.ts` | **Role guards** (`services/nimbus/src/routes/admin.ts`): @@ -497,8 +656,8 @@ and an unreachable upstream is a 502 on an admin route, not an outage. | `NIMBUS_DATABASE_URL` | **none — refuses to boot** | Must match `postgres(ql)?://`; an absent value used to make `postgres()` silently dial libpq defaults | `env.ts:80-100` | | `NIMBUS_PORT` | 4001 | | `env.ts:159` | | `NIMBUS_ISSUER` | `http://localhost:4001` | Baked into every JWT. A localhost issuer alongside public https origins logs a structured **error** at boot (`env.ts:65-73`) | `env.ts:51` | -| `CORS_ORIGINS` | five localhost ports | Browser allowlist. `https://account.cloudsforge.online` is appended unconditionally (`env.ts:31-33`) | `env.ts:26-29` | -| `PORTAL_ALLOWED_ORIGINS` | falls back to `CORS_ORIGINS` | Return-URL allowlist. Never a wildcard; `*.apex` entries match subdomains | `env.ts:39-41` | +| `CORS_ORIGINS` | six localhost ports | Browser allowlist, verbatim — nothing is appended. It used to gain a literal `https://account.cloudsforge.online`, which is a foreign domain on any other apex and leaked into the return-URL allowlist below; compose supplies `account.${CLOUDSFORGE_APEX}` instead | `env.ts:47-50` | +| `PORTAL_ALLOWED_ORIGINS` | falls back to `CORS_ORIGINS` | Return-URL allowlist. Never a wildcard; `*.apex` entries match subdomains. Note the fallback: an origin added for CORS is also a legal destination for a live sign-in code | `env.ts:56-58` | | `TRUST_PROXY` | `loopback,linklocal,uniquelocal` | Who may set `X-Forwarded-*`. A blanket `true` would let anyone reaching the published port spoof their IP into a private rate-limit bucket | `env.ts:47-49` | | **`KEYVAULT_URL`** | `http://localhost:4005` | Vault proxy target. **Must be set on the nimbus service in compose** | `env.ts:107`, set at `docker-compose.yml:139` | | **`PAY_API_URL`** | `http://pay:4003` | Pay proxy target. Default is the compose-network name **on purpose** | `env.ts:150`, set at `docker-compose.yml:146` | @@ -662,6 +821,12 @@ derives the same way and adds exactly one variable of its own, `*.apiKey` does not match `nowApiKey`, which is why the real names are spelled out. `*.password`, `*.newPassword`, `*.accessToken`, `*.refreshToken` and `*.privateKey` are all covered. +- **Query-string VALUES are never logged** (`obs.ts` `safeRequestUrl`). Fastify + writes an "incoming request" line before any handler runs, and `redact` cannot + see inside `req.url`, so the serializer keeps the path and the parameter names + and replaces every value with `[redacted]` — fail-closed, so a credential + someone puts in a URL tomorrow is covered today. The other services' vendored + copies still need this re-copied into them (`src/obs.test.ts` is the check). - **`GET /health` is the compose healthcheck**, and the game, forge-mint, crucible and forge-pay services all `depends_on: nimbus: service_healthy` (e.g. `docker-compose.yml:182-183`). A slow migration blocks the stack. diff --git a/README.md b/README.md index 71a93f4..7ebc878 100644 --- a/README.md +++ b/README.md @@ -30,14 +30,36 @@ alongside the service. See `services/nimbus/.env.example`. - **The apps take no build-time configuration.** Nothing reads `import.meta.env.VITE_*`; hosts are resolved in the browser at runtime by `cloudsforgeHosts()`, so one image serves every environment. -- **Password reset is complete except for delivery.** Tokens are single-use, - 30-minute, hashed at rest, and burn every refresh-token family when spent. - Nothing in this estate can send email, so `deliverPasswordReset()` - (`services/nimbus/src/passwordReset.ts`) is a marked seam that currently - delivers nothing: a user's own request is recorded and surfaced in the admin - console under Users, where an operator issues the link and passes it on out of - band. To finish it, replace that one function — the routes, the table and the - TTL are done, and the copy already says the truth in the meantime. +- **The signing key is encrypted at rest, and replaceable.** `NIMBUS_KEY_SECRET` + is required — Nimbus refuses to boot without it, rejects a placeholder or + anything under 24 characters, and never falls back to storing the key in the + clear. It was plaintext JSONB in a database every service shares one Postgres + role on, which turned any read-only leak (a stolen backup, a `SELECT`-only + injection) into silent, offline, unrevocable admin in every product; the + envelope is the same scrypt + AES-256-GCM shape ForgeKeyvault uses for custody + keys. Replacing a key is three admin calls — mint, wait one access-token TTL, + activate — and `/admin/signing-keys/:kid/activate` **refuses** until the key + has been in the JWKS long enough for every service to have fetched it. See + MAP.md §2.1. +- **The password-reset link is built from `NIMBUS_PUBLIC_URL`, never the request.** + Everything else Nimbus renders derives its URLs from the host it was asked on, + which is right when the link goes back to the same browser. A reset link is + emailed, so a request-derived host let a stranger have this deployment's own + relay send a victim a genuine, correctly branded email pointing wherever they + liked. Unset, it falls back to `NIMBUS_ISSUER`; compose sets it to + `https://account.`. +- **Password reset emails over any SMTP relay, or none.** Tokens are single-use, + 30-minute, hashed at rest, and burn every refresh-token family when spent. Set + `SMTP_HOST`, `SMTP_USER`, `SMTP_PASS` and `SMTP_FROM` and + `deliverPasswordReset()` (`services/nimbus/src/passwordReset.ts`) mails the + link through `src/mailer.ts` — a generic nodemailer transport with no provider + in it, so Brevo, Resend, SendGrid, Mailtrap, a Gmail app password or your own + postfix are the same four settings and switching is never a code change. See + the stack's `.env.example`, "Outbound mail", for free credentials in about two + minutes. Leaving `SMTP_HOST` unset is a supported mode, not a broken one: the + request is recorded and surfaced in the admin console under Users, where an + operator issues the link and passes it on out of band. The user-facing copy + says whichever of the two is true. - **The admin console reads ForgeKeyvault through Nimbus.** Keyvault holds custody keys, binds to loopback and is deliberately never routed, so the browser cannot reach it; `services/nimbus/src/routes/vault.ts` relays two @@ -46,6 +68,46 @@ alongside the service. See `services/nimbus/.env.example`. `KEYVAULT_URL: http://forge-keyvault:4005` on the `nimbus` service in the stack's `docker-compose.yml`** — it is set on `pay` and `forge-mint` but not yet on `nimbus`. Reveal is not relayed unless `NIMBUS_VAULT_REVEAL_PROXY=true`. +- **"Nothing is stuck." is now a fact the console asked for.** The withdrawals + panel used to make one unqualified request, which Forge Pay caps at the newest + 200 rows across every status, and then count and filter that array in the + browser — so a stuck payment with two hundred newer ones behind it produced an + empty human-intervention queue that was not empty. A status chip is a query + now, and the stuck line is its own `?status=stuck` read that does not depend on + which chip is selected; a read that fails says so instead of rendering as zero + (`apps/admin/src/lib/withdrawalQueue.ts`, MAP.md §4.3). +- **An outage no longer reads as reassurance.** The Prices page's treasury and + withdrawals panels load out-of-band so neither can take the price panel down, + and they used to write that as `.catch(() => setTreasuries([]))` — so a 502 + from the pay relay, or a gateway timeout on the chain-balance read, rendered as + "No treasury addresses configured." and "No withdrawals yet." to an operator + who had opened the page *because* something was wrong. A failed read is now an + unknown and never an absence: only an answered request may state one, and the + panel shows the server's own sentence in its place (`lib/panelRead.ts`, + MAP.md §4.4). +- **The console can now return the money on a stuck withdrawal.** Forge Pay had + the whole remediation path and nothing called it, so a payment that never + landed left a user's custodial coin balance debited until somebody hand-rolled + a bearer POST from a shell. Prices → Withdrawals has an **Abandon and refund** + action (type `ABANDON` to confirm) that relays through + `services/nimbus/src/routes/pay.ts`. Pay asks the chain first and refuses + unless it can prove the signed bytes can never be applied; that refusal is + shown word for word, because it is the only statement of why and each one + implies a different next action. **This is only safe against a Forge Pay that + checks the nonce** (its CF-06) — an older pay refunded on a missing receipt, + which on an EVM chain proves nothing. +- **The public site is tested against the code it describes.** The front page and + /roadmap said EMBER credited at the chain tip and that game cosmetics lived in + your own browser where nobody else saw them. Neither had been true for a while: + Forge Pay reads an EMBER balance 60 blocks back — the depth Hearth publishes to + exchanges — and a cosmetic is a database row, entitlement-checked on equip and + fanned out to every roster you appear on. The roadmap promises "nothing claimed + here that you cannot go and use" and was breaking it by *understating* what + runs. `apps/site/src/lib/site.test.ts` now reads the confirmation depth out of + the shared deposit registry and fails the build if the copy quotes another + number, and sweeps the module's strings for retired claims. Marketing copy is + the only part of this estate with no compiler behind it, which is exactly why + it drifts. - **Services ship no build artifact.** `tsx` runs the TypeScript sources directly and `build` is `tsc --noEmit`, so CI builds the Docker images for real — a green typecheck cannot tell you whether the image resolves its diff --git a/apps/admin/package.json b/apps/admin/package.json index 7ff0e42..ef0f5ed 100644 --- a/apps/admin/package.json +++ b/apps/admin/package.json @@ -7,7 +7,8 @@ "dev": "vite --port 3002", "build": "tsc --noEmit && vite build", "preview": "vite preview --port 3002", - "typecheck": "tsc --noEmit" + "typecheck": "tsc --noEmit", + "test": "node --import tsx --test \"src/**/*.test.ts\"" }, "dependencies": { "@cloudsforge/shared": "^0.4.0", @@ -24,6 +25,7 @@ "@types/react-dom": "^19.0.2", "@vitejs/plugin-react": "^4.3.4", "tailwindcss": "^4.0.0", + "tsx": "^4.19.2", "typescript": "^5.7.3", "vite": "^6.0.7" } diff --git a/apps/admin/src/lib/api.ts b/apps/admin/src/lib/api.ts index 488073a..f939701 100644 --- a/apps/admin/src/lib/api.ts +++ b/apps/admin/src/lib/api.ts @@ -544,14 +544,18 @@ export const api = { // Forge Pay — operator surface, proxied by Nimbus. // - // Not called direct, and for a narrower reason than the vault above: pay IS routed - // and IS on this origin's CORS allowlist, so these three GETs would work against - // HOSTS.pay. The PUT would not — pay's preflight answers - // `access-control-allow-methods: GET,HEAD,POST`, so no browser will send it, and - // widening that is a change to a repo this one does not own. Setting the price is - // the point of the panel, so all four go the one way that can carry it. Pay - // re-verifies the same bearer on the far side and attributes the change to the real - // operator. See services/nimbus/src/routes/pay.ts. + // Not called direct, and for a narrower reason than the vault above: keyvault is + // unroutable, whereas pay IS routed and IS on this origin's CORS allowlist, so every + // call below would work against HOSTS.pay — the PUT included, since forge-pay f3eb408 + // (2026-07-28) widened pay's preflight to name PUT. They come through Nimbus anyway so + // that the panel has one origin, one auth path and one failure mode instead of two, + // and so that no part of it depends on the console's apex being kept on a CORS list + // in another service. Pay re-verifies the same bearer on the far side and attributes + // the change to the real operator. + // + // A future PUT or DELETE on pay's operator surface therefore needs no second proxy + // layer and no cross-repo negotiation — it is one more route in + // services/nimbus/src/routes/pay.ts, which is also where the full argument lives. listAdministeredPrices(): Promise<{ prices: AdministeredPrice[] }> { return request<{ prices: AdministeredPrice[] }>(NIMBUS_URL, '/admin/pay/prices') }, @@ -581,4 +585,33 @@ export const api = { `/admin/pay/withdrawals${qs}`, ) }, + + /** + * Give up on a withdrawal and return the money to the user's custodial coin balance. + * + * Resolves with the withdrawal as it now stands — `failed`, carrying the refund + * reason — so the caller can replace its row rather than guess at the new state. + * + * REJECTS FAR MORE OFTEN THAN IT RESOLVES, and that is the design. Forge Pay asks the + * chain first and refuses unless it can PROVE the signed bytes can never be applied, + * because a refund of a payment that did land is the one thing it cannot take back. + * The `ApiError.message` on those refusals is pay's own sentence, and it is the only + * statement of WHY anywhere in the system — never replace it with wording of ours. + * The distinct outcomes worth handling by `code`: + * + * 409 withdrawal_on_chain | withdrawal_still_applicable — the network can still + * apply this payment. Not now, and possibly not ever; nothing was refunded. + * 409 withdrawal_unprovable — pay cannot ask the question (no treasury row for the + * coin, or an XRP blob whose sequence it never recorded). Nothing was refunded. + * 409 withdrawal_not_abandonable — the row moved on (confirmed, or already failed). + * 503 chain_unreachable — the node did not answer. The next action is to retry, not + * to escalate, which is why this is not a 5xx of ours. + */ + abandonWithdrawal(id: string): Promise { + return request( + NIMBUS_URL, + `/admin/pay/withdrawals/${encodeURIComponent(id)}/abandon`, + { method: 'POST' }, + ) + }, } diff --git a/apps/admin/src/lib/panelRead.test.ts b/apps/admin/src/lib/panelRead.test.ts new file mode 100644 index 0000000..63065db --- /dev/null +++ b/apps/admin/src/lib/panelRead.test.ts @@ -0,0 +1,233 @@ +import assert from 'node:assert/strict' +import { test, type TestContext } from 'node:test' + +/** + * What the Prices page's two secondary panels say when their read fails (CF-35). + * + * WHAT THE DEFECT WAS. Both panels loaded out-of-band so neither could take the price panel + * down, and both wrote that as `.catch(() => setX([]))`. The copy hanging off those arrays + * asserts facts: "No treasury addresses configured." and "No withdrawals yet." So a 502 + * `pay_unreachable` from the Nimbus relay, an edge timeout on `/admin/treasury` answering + * with an HTML page, or a 500 from `/admin/withdrawals` all rendered as a confident + * description of a system nothing had been read from — and the error object was dropped, so + * there was nothing in the browser console either. + * + * EVERY CASE HERE IS ABOUT WHAT SURVIVES A FAILURE, because a test that fed these functions + * a successful response and checked the rows would have passed against the defect: the happy + * path was never wrong. The api client is the REAL one, driven by a stubbed `fetch`, so the + * `reason` under test is the sentence lib/api.ts actually builds from a real response — + * including the `(reference …)` suffix it appends on a 5xx — and not a message this file + * made up. + * + * No DOM: `localStorage` is the one browser global the client touches on this path and it is + * shimmed below. Run with `pnpm --filter @cloudsforge/admin test`. + */ + +/* ------------------------------ browser shims ------------------------------ */ + +// The real `request()` reads `tokens.access` off localStorage to set the bearer. Shimming it +// is what lets the genuine api client run here; nothing else in this path needs a window. +const store = new Map() +;(globalThis as unknown as { localStorage: Storage }).localStorage = { + getItem: (k: string) => store.get(k) ?? null, + setItem: (k: string, v: string) => void store.set(k, v), + removeItem: (k: string) => void store.delete(k), + clear: () => store.clear(), + key: () => null, + length: 0, +} as Storage + +type Answer = { status: number; body: string; contentType?: string } + +/** Answer every request from a table keyed by pathname, the way Nimbus's relay would. */ +function stubFetch(answers: Record): { asked: string[] } { + const asked: string[] = [] + globalThis.fetch = (async (input: string | URL | Request) => { + const url = new URL(typeof input === 'string' ? input : input.toString()) + asked.push(url.pathname) + const answer = answers[url.pathname] + if (!answer) throw new TypeError('fetch failed') + return new Response(answer.body, { + status: answer.status, + headers: { 'content-type': answer.contentType ?? 'application/json' }, + }) + }) as typeof fetch + return { asked } +} + +/** Nimbus's own body when it cannot reach Forge Pay — services/nimbus/src/routes/pay.ts. */ +const PAY_UNREACHABLE: Answer = { + status: 502, + body: JSON.stringify({ + error: 'Forge Pay is not reachable from Nimbus.', + code: 'pay_unreachable', + requestId: 'req_cf35', + }), +} + +/** What an edge or a tunnel answers with when the chain-balance read times out. */ +const GATEWAY_HTML: Answer = { + status: 504, + body: '504 Gateway Time-out…', + contentType: 'text/html', +} + +const PRICES_OK: Answer = { + status: 200, + body: JSON.stringify({ + prices: [ + { + coin: 'EMBER', + usd: '0.30', + source: 'administered', + setBy: null, + setByHandle: null, + updatedAt: '2026-07-01T00:00:00.000Z', + }, + ], + }), +} + +/* --------------------------------- imports --------------------------------- */ +// Below the shims: lib/api.ts resolves its hosts at import and its `request` runs against +// whatever `fetch` is bound when it is called, so the order matters here. + +const { api } = await import('./api.ts') +const { readPanel, unreadableNotice, failureReason } = await import('./panelRead.ts') +const { readStuckQueue, readWithdrawalPage, stuckNotice, pageSubject } = await import( + './withdrawalQueue.ts' +) + +/** Keep the console line under test out of the test output, and let it be asserted. */ +function quietConsole(t: TestContext) { + return t.mock.method(console, 'error', () => {}) +} + +/* ------------------------------ the scenario ------------------------------ */ + +test('the register\'s scenario: prices answer, both secondary reads do not', async (t) => { + const logged = quietConsole(t) + stubFetch({ + '/admin/pay/prices': PRICES_OK, + // Reviewer note: with pay FULLY down the prices read fails first and `load()` returns + // early, so the page shows a red banner and the panels stay skeletons — the lie is never + // rendered. The reachable case is the one below: prices succeed and a secondary read + // fails on its own. `/admin/treasury` reads a balance per address off a chain, so it is + // the request an edge times out on first, and it comes back as HTML. + '/admin/pay/treasury': GATEWAY_HTML, + '/admin/pay/withdrawals': PAY_UNREACHABLE, + }) + + // The price panel is unaffected — that is the property the old `.catch(() => [])` was + // protecting, and it still holds. + const { prices } = await api.listAdministeredPrices() + assert.equal(prices.length, 1) + + // What the page used to do with the other two, spelled out so the defect is on the record. + const oldTreasury = await api + .listTreasury() + .then((r) => r.treasuries) + .catch(() => []) + assert.deepEqual(oldTreasury, [], 'the scenario is only a scenario if the old catch emptied it') + + // What it does now. + const treasury = await readPanel('Treasury addresses', async () => + (await api.listTreasury()).treasuries, + ) + assert.equal(treasury.kind, 'unreadable') + assert.equal( + treasury.kind === 'unreadable' && treasury.reason, + 'The server hit an error handling this request.', + ) + assert.equal( + unreadableNotice('Treasury addresses', treasury.kind === 'unreadable' ? treasury.reason : ''), + 'Treasury addresses could not be read — this is not a statement that there are none. ' + + 'The server hit an error handling this request.', + ) + + const page = await readPanel(pageSubject('stuck'), () => + readWithdrawalPage(api.listWithdrawals, 'stuck'), + ) + assert.equal(page.kind, 'unreadable') + assert.match( + unreadableNotice('Stuck withdrawals', page.kind === 'unreadable' ? page.reason : ''), + /^Stuck withdrawals could not be read — this is not a statement that there are none\. Forge Pay is not reachable from Nimbus\./, + ) + + // The header line beside them, for completeness: it already refused to say zero (CF-33). + const stuck = await readStuckQueue(api.listWithdrawals) + assert.equal(stuck.kind, 'unreadable') + assert.notEqual(stuckNotice(stuck).text, 'Nothing is stuck.') + + // And the whole point, stated once as a property rather than three times as strings: not + // one of the three failed reads produced rows for the page to describe. + for (const state of [treasury, page]) { + assert.notDeepEqual(state, { kind: 'known', data: [] }) + } + + // The error objects are no longer swallowed — one console line per failed panel, carrying + // the ApiError itself so its status, code and x-request-id are all still there. + assert.equal(logged.mock.callCount(), 2) + const [subject, err] = logged.mock.calls[1].arguments as [string, unknown] + assert.equal(subject, 'Stuck withdrawals could not be read') + assert.equal((err as { status?: number }).status, 502) + assert.equal((err as { code?: string }).code, 'pay_unreachable') + assert.equal((err as { requestId?: string }).requestId, 'req_cf35') +}) + +/* --------------------------- what may be said --------------------------- */ + +test('an absence is only spoken when the panel was answered', async (t) => { + quietConsole(t) + + // Answered with none: the empty state is the truth, and it survives. + stubFetch({ '/admin/pay/treasury': { status: 200, body: JSON.stringify({ treasuries: [] }) } }) + const answered = await readPanel('Treasury addresses', async () => + (await api.listTreasury()).treasuries, + ) + assert.deepEqual(answered, { kind: 'known', data: [] }) + + // Not answered at all — nothing on the wire, the case `.catch(() => [])` handled + // identically to the one above. + stubFetch({}) + const unreachable = await readPanel('Treasury addresses', async () => + (await api.listTreasury()).treasuries, + ) + assert.equal(unreachable.kind, 'unreadable') + assert.match( + unreachable.kind === 'unreadable' ? unreachable.reason : '', + /^Cannot reach localhost:4001\./, + ) +}) + +test('the notice states the unknown before it states the reason', () => { + const notice = unreadableNotice('Withdrawals', 'Forge Pay is not reachable from Nimbus.') + assert.match(notice, /this is not a statement that there are none/) + assert.match(notice, /Forge Pay is not reachable from Nimbus\.$/) + // The sentences the panels are no longer allowed to reach on a failure. + for (const lie of ['No treasury addresses configured.', 'No withdrawals yet.', 'Nothing is stuck.']) { + assert.ok(!notice.includes(lie)) + } +}) + +test('a rejection that is not an Error still gets a sentence', () => { + assert.equal(failureReason(new Error('boom')), 'boom') + // `String({})` is "[object Object]" and an empty `message` renders as nothing at all — + // both of which put an operator in front of a blank error block. + assert.equal(failureReason({ nope: true }), 'The request failed and gave no reason.') + assert.equal(failureReason(new Error('')), 'The request failed and gave no reason.') +}) + +test('the notice names the status the table asked for, not withdrawals in general', () => { + assert.equal(pageSubject('all'), 'Withdrawals') + assert.equal(pageSubject('stuck'), 'Stuck withdrawals') + assert.equal(pageSubject('broadcast'), 'Broadcast withdrawals') +}) + +test('a panel read never rejects — the price panel cannot be taken down by it', async (t) => { + quietConsole(t) + const state = await readPanel('Treasury addresses', () => { + throw new Error('thrown synchronously, before any promise exists') + }) + assert.equal(state.kind, 'unreadable') +}) diff --git a/apps/admin/src/lib/panelRead.ts b/apps/admin/src/lib/panelRead.ts new file mode 100644 index 0000000..034e6fd --- /dev/null +++ b/apps/admin/src/lib/panelRead.ts @@ -0,0 +1,86 @@ +/* ------------------------------------------------------------------ * + * What a secondary panel is allowed to say when its own read failed. + * + * WHAT THE DEFECT WAS (CF-35). The Prices page loaded treasury and withdrawals + * out-of-band so that neither could take the price panel down with it — which is right — + * and implemented that as `.catch(() => setTreasuries([]))`. An empty array is not a + * neutral value here: the copy hung off it asserts facts. A 502 `pay_unreachable` from the + * Nimbus relay, an edge timeout on the chain-balance-heavy `/admin/treasury` returning an + * HTML page (`non_json`), or a 500 from `/admin/withdrawals` all rendered as "No treasury + * addresses configured." and "No withdrawals yet." An operator opening /prices during + * exactly that incident read a confident description of a system nothing had been read + * from. The catch also swallowed the error object entirely, so there was no trace in the + * browser console either — the failure was undiagnosable rather than merely mis-rendered. + * + * So the rule this module exists to keep: A FAILED READ IS AN UNKNOWN, NEVER AN ABSENCE. + * Three states and not an array, the same shape and for the same reason as `StuckQueue` in + * withdrawalQueue.ts (CF-33): "not asked yet", "asked and could not be told" and "asked and + * told none" are three different sentences, and only the last of them may be spoken as a + * fact about the system. It is a separate module from the page so that rule is testable + * without a DOM — see panelRead.test.ts. + * + * Non-fatal is preserved: `readPanel` never rejects, so a caller still cannot be taken down + * by the panel beside it. What changed is what it hands back when it fails. + * ------------------------------------------------------------------ */ + +/** + * What a panel knows about its own data. + * + * `known` is the ONLY state carrying rows, so an empty table and a broken read cannot be + * confused by a caller that forgets to check: there is no array to read on `unreadable`. + */ +export type PanelRead = + | { kind: 'loading' } + | { kind: 'known'; data: T } + | { kind: 'unreadable'; reason: string } + +/** + * The sentence to show instead of the rows. + * + * Every service in this estate answers `{error, code, requestId}` and lib/api.ts turns that + * into an `ApiError` whose `message` is the server's own words — including the reference id + * on a 5xx. Rendering it unedited is what the rest of this console does (`pinsError` in + * Vault.tsx, every `catch` in Users.tsx), so a panel error is quotable in a ticket. + */ +export function failureReason(err: unknown): string { + if (err instanceof Error && err.message) return err.message + // A non-Error rejection is a fault in the client, not an answer from a server, and + // `String(err)` on an object gives "[object Object]". Say the honest thing instead. + return 'The request failed and gave no reason.' +} + +/** + * Read one secondary panel. + * + * `subject` is the panel in the operator's words ("Treasury addresses"), used for the + * console line and by `unreadableNotice`. + * + * THE console.error IS PART OF THE FIX, not debug residue. The old catches discarded the + * error object, so a support conversation had nothing to work from: no status, no code, no + * `x-request-id`, no stack. The rendered sentence is for the operator; this line is for + * whoever they hand the tab to. It logs the object rather than a string so the ApiError's + * `status`/`code`/`requestId` are all expandable in the console. + */ +export async function readPanel(subject: string, read: () => Promise): Promise> { + try { + return { kind: 'known', data: await read() } + } catch (err) { + console.error(`${subject} could not be read`, err) + return { kind: 'unreadable', reason: failureReason(err) } + } +} + +/** + * What a panel says in place of its empty state when the read failed. + * + * The middle clause is the whole point and is not decoration: without it the operator reads + * a reason for a fact they have already accepted, rather than being told the fact was never + * established. Worded to match `stuckNotice`'s "this is not a statement that nothing is", + * because they appear within a few hundred pixels of each other on the same page. + * + * The reason goes last so no punctuation has to be guessed at the seam — it is the server's + * sentence, ending however the server ended it, and it is not rewritten here. + */ +export function unreadableNotice(subject: string, reason: string): string { + return `${subject} could not be read — this is not a statement that there are none. ${reason}` +} diff --git a/apps/admin/src/lib/withdrawalQueue.test.ts b/apps/admin/src/lib/withdrawalQueue.test.ts new file mode 100644 index 0000000..6ffb9c0 --- /dev/null +++ b/apps/admin/src/lib/withdrawalQueue.test.ts @@ -0,0 +1,174 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { + WITHDRAWAL_PAGE, + readStuckQueue, + readWithdrawalPage, + stuckNotice, + windowSentence, + type ListWithdrawals, +} from './withdrawalQueue.ts' +import type { AdminWithdrawal, WithdrawalStatus } from './api.ts' + +/** + * The withdrawals panel's questions (CF-33). + * + * WHAT THE DEFECT WAS. The Prices page made one request — `GET /admin/pay/withdrawals` + * with no status — and answered everything from it in the browser. Pay caps that at the + * newest 200 rows across all statuses, so a stuck withdrawal with 200 newer ones behind it + * fell out of the array and the page said "Nothing is stuck." and "No stuck withdrawals." + * while the same service would have answered `?status=stuck` with the row. + * + * SO EVERY CASE HERE IS ABOUT WHICH REQUEST IS SENT, not about how an answer is filtered. + * A test that fed these functions the right rows and checked the arithmetic would have + * passed against the defect — the arithmetic was never wrong. The `list` below records its + * argument, and the first two cases assert what was asked for; the scenario case then + * plays out the failure exactly as the register describes it, against a fake pay that + * truncates the way the real one does. + * + * No DOM and no api client: `withdrawalQueue.ts` imports nothing at runtime, which is what + * makes the panel's rules testable at all. Run with `pnpm --filter @cloudsforge/admin test`. + */ + +/* -------------------------------- fakes -------------------------------- */ + +function row(id: string, status: WithdrawalStatus, createdAt: string): AdminWithdrawal { + return { + id, + userId: 'u1', + coin: 'ETH', + network: 'mainnet', + destination: '0xdead', + amount: '0.5', + fee: '0.001', + net: '0.499', + status, + txid: null, + confirmations: 0, + requiredConfirmations: 12, + failureReason: null, + createdAt, + updatedAt: createdAt, + broadcastAt: null, + } +} + +/** + * A Forge Pay holding `rows`, answering with its own semantics: filter by status FIRST, + * then newest-first, then cap at 200 (its store.ts `listAllWithdrawals`). The order of + * those three steps is the entire defect — a cap applied before the filter is what the + * browser was doing. + */ +function fakePay(rows: AdminWithdrawal[]): ListWithdrawals & { asked: (string | undefined)[] } { + const asked: (string | undefined)[] = [] + const list = async (status?: WithdrawalStatus) => { + asked.push(status) + const matching = rows + .filter((r) => !status || r.status === status) + .sort((a, b) => (a.createdAt < b.createdAt ? 1 : -1)) + .slice(0, WITHDRAWAL_PAGE) + return { withdrawals: matching } + } + return Object.assign(list, { asked }) +} + +/** One stuck withdrawal, then 200 newer ones that confirmed — the register's scenario. */ +function scenario(): AdminWithdrawal[] { + return [ + row('wd_stuck', 'stuck', '2026-07-01T00:00:00.000Z'), + ...Array.from({ length: WITHDRAWAL_PAGE }, (_, i) => + row(`wd_ok_${i}`, 'confirmed', new Date(Date.UTC(2026, 6, 2, 0, i)).toISOString()), + ), + ] +} + +/* ------------------------- what gets asked for ------------------------- */ + +test('the stuck count is its own ?status=stuck request', async () => { + const pay = fakePay([]) + await readStuckQueue(pay) + assert.deepEqual(pay.asked, ['stuck']) +}) + +test('a selected chip becomes a ?status= on the request, not a filter afterwards', async () => { + const pay = fakePay(scenario()) + await readWithdrawalPage(pay, 'stuck') + await readWithdrawalPage(pay, 'all') + // `undefined` for `all` and nothing else: the console has one page size and asking pay + // for a status it did not select would be a second, contradictory window. + assert.deepEqual(pay.asked, ['stuck', undefined]) +}) + +/* --------------------------- the scenario --------------------------- */ + +test('a stuck withdrawal 200 rows deep is still counted, and the chip still shows it', async () => { + const pay = fakePay(scenario()) + + // What the page used to do: one unqualified fetch, counted in the browser. + const truncated = await readWithdrawalPage(pay, 'all') + assert.equal(truncated.length, WITHDRAWAL_PAGE) + assert.equal( + truncated.filter((w) => w.status === 'stuck').length, + 0, + 'the scenario is only a scenario if the stuck row really has fallen out of the window', + ) + + // What it does now. + const queue = await readStuckQueue(pay) + assert.deepEqual(queue, { kind: 'known', count: 1, capped: false }) + assert.equal( + stuckNotice(queue).text, + '1 is stuck and waiting on a human — abandon one to return the money to its owner.', + ) + assert.equal(stuckNotice(queue).urgent, true) + + const chip = await readWithdrawalPage(pay, 'stuck') + assert.deepEqual( + chip.map((w) => w.id), + ['wd_stuck'], + ) +}) + +/* ------------------------ what may be said ------------------------ */ + +test('"Nothing is stuck." is only said when pay was asked and answered none', () => { + assert.equal(stuckNotice({ kind: 'known', count: 0, capped: false }).text, 'Nothing is stuck.') + + // The two states that must never collapse into that sentence. Before the fix a failed + // read set the array to [] and the page said "Nothing is stuck." on a 502. + for (const queue of [ + { kind: 'loading' } as const, + { kind: 'unreadable', reason: 'Forge Pay is not reachable from Nimbus.' } as const, + ]) { + assert.notEqual(stuckNotice(queue).text, 'Nothing is stuck.') + } + + const unreadable = stuckNotice({ kind: 'unreadable', reason: 'pay_unreachable' }) + assert.match(unreadable.text, /could not be read/) + assert.match(unreadable.text, /pay_unreachable/) + assert.equal(unreadable.urgent, true) +}) + +test('a failed read is unreadable, never zero', async () => { + const queue = await readStuckQueue(async () => { + throw new Error('Forge Pay is not reachable from Nimbus.') + }) + assert.deepEqual(queue, { + kind: 'unreadable', + reason: 'Forge Pay is not reachable from Nimbus.', + }) +}) + +test('a full page of stuck rows is reported as a bound, not as an exact count', async () => { + const many = Array.from({ length: 260 }, (_, i) => + row(`wd_${i}`, 'stuck', new Date(Date.UTC(2026, 6, 2, 0, i)).toISOString()), + ) + const queue = await readStuckQueue(fakePay(many)) + assert.deepEqual(queue, { kind: 'known', count: WITHDRAWAL_PAGE, capped: true }) + assert.match(stuckNotice(queue).text, /^At least 200 are stuck/) +}) + +test('the window sentence names the status the table was actually asked for', () => { + assert.equal(windowSentence('all'), 'The 200 most recent, newest first.') + assert.equal(windowSentence('stuck'), 'The 200 most recent stuck withdrawals, newest first.') +}) diff --git a/apps/admin/src/lib/withdrawalQueue.ts b/apps/admin/src/lib/withdrawalQueue.ts new file mode 100644 index 0000000..d6d8f84 --- /dev/null +++ b/apps/admin/src/lib/withdrawalQueue.ts @@ -0,0 +1,147 @@ +import type { AdminWithdrawal, WithdrawalStatus } from './api' +import { failureReason } from './panelRead' + +/* ------------------------------------------------------------------ * + * How the withdrawals panel asks Forge Pay a question, and what it is allowed to say + * about the answer. + * + * WHAT THE DEFECT WAS (CF-33). The panel made ONE request — `GET /admin/pay/withdrawals` + * with no status — and did everything else in the browser: the status chips filtered that + * array, and the "is anything stuck?" line counted it. Pay answers that request with the + * newest 200 rows ACROSS ALL STATUSES (its store.ts `limit = 200`), so a stuck withdrawal + * with two hundred newer ones behind it is simply not in the array. The chip then showed + * "No stuck withdrawals." and the header said "Nothing is stuck." — the one panel whose + * job is the human-intervention queue reporting an empty queue that was not empty, while + * the same service would have answered `?status=stuck` with the row. + * + * The table's own truncation was never the problem and is not treated as one: it says + * "The 200 most recent" next to itself. The problem was the two sentences beside that + * caveat which claimed something absolute — that nothing is stuck — on the strength of a + * window that cannot support it. + * + * So the rule this module exists to keep: A COUNT AND AN ABSENCE COME FROM A QUERY THAT + * ASKED FOR THAT STATUS. Nothing here derives either by filtering a page it was handed for + * some other purpose. It is a separate module from the page so that rule is testable + * without a DOM — see withdrawalQueue.test.ts, which is a test of WHICH REQUEST IS SENT, + * because that is what was wrong. + * ------------------------------------------------------------------ */ + +/** + * Forge Pay's page size for `GET /admin/withdrawals` (its store.ts `limit = 200`). + * + * Restated here to QUALIFY, not to paginate: the console has no way to ask for page two, + * and does not pretend to. It is used to say "the 200 most recent" honestly, and to notice + * when a count comes back exactly full — a number that might be a truncation is not a + * count, and reporting it as one is the same mistake in a smaller form. + */ +export const WITHDRAWAL_PAGE = 200 + +/** The `list` half of the api client, taken as a parameter so this stays DOM-free. */ +export type ListWithdrawals = ( + status?: WithdrawalStatus, +) => Promise<{ withdrawals: AdminWithdrawal[] }> + +/** + * What the page knows about the stuck queue. + * + * Three states and not a number, because "not asked yet", "asked and could not be told" + * and "asked and told zero" are three different sentences and only the last of them is + * "Nothing is stuck." Collapsing any of the others into 0 recreates CF-33 through a + * different door: a 502 from the relay would render as a reassuring absolute. + */ +export type StuckQueue = + | { kind: 'loading' } + | { kind: 'known'; count: number; capped: boolean } + | { kind: 'unreadable'; reason: string } + +/** + * Ask pay how many withdrawals are stuck. + * + * Its own request, deliberately, and NOT a filter over whatever the table is showing. The + * header line is the reason an operator opens this panel at all, so it may not depend on + * which chip happens to be selected, nor on a page of rows fetched for the table. + */ +export async function readStuckQueue(list: ListWithdrawals): Promise { + try { + const { withdrawals } = await list('stuck') + return { + kind: 'known', + count: withdrawals.length, + // A full page means pay had at least this many and stopped counting. Nothing here + // can turn that into an exact number, so it is reported as the bound it is. + capped: withdrawals.length >= WITHDRAWAL_PAGE, + } + } catch (err) { + // One place turns a rejection into a sentence, shared with the other two panels on this + // page (lib/panelRead.ts) so the three do not drift into three ways of saying it. + return { kind: 'unreadable', reason: failureReason(err) } + } +} + +/** + * Fetch the rows for one chip. + * + * A thin wrapper on purpose: the ONE decision it holds — that a selected status becomes a + * `?status=` on the request rather than a filter afterwards — is precisely the decision + * that was missing, so it lives somewhere a test can watch it being made. Errors are the + * caller's to render; this does not convert a failure into an empty table. + */ +export async function readWithdrawalPage( + list: ListWithdrawals, + filter: WithdrawalStatus | 'all', +): Promise { + const { withdrawals } = await list(filter === 'all' ? undefined : filter) + return withdrawals +} + +export interface Notice { + text: string + /** Amber rather than fog: something is waiting on a person, or cannot be checked. */ + urgent: boolean +} + +/** The sentence under the "Withdrawals" heading about the human-intervention queue. */ +export function stuckNotice(queue: StuckQueue): Notice { + switch (queue.kind) { + case 'loading': + return { text: 'Checking the stuck queue…', urgent: false } + case 'unreadable': + // Named as an unknown, never as a zero. This sentence replaces the one place the + // old page was most confidently wrong, so it says outright what it is not. + // The reason goes last so no punctuation has to be guessed at the seam — it is + // pay's sentence, or the relay's, and neither is rewritten here. + return { + text: `Whether anything is stuck could not be read — this is not a statement that nothing is. ${queue.reason}`, + urgent: true, + } + case 'known': { + if (queue.count === 0) return { text: 'Nothing is stuck.', urgent: false } + const subject = queue.capped + ? `At least ${WITHDRAWAL_PAGE} are` + : `${queue.count} ${queue.count === 1 ? 'is' : 'are'}` + return { + text: `${subject} stuck and waiting on a human — abandon one to return the money to its owner.`, + urgent: true, + } + } + } +} + +/** + * What the table is a list of, in the operator's words. + * + * Named after the status that was ASKED FOR, like everything else here: when the read fails + * the notice has to say which question went unanswered, and "Withdrawals could not be read" + * on a page whose `stuck` chip is selected would be one status too broad. + */ +export function pageSubject(filter: WithdrawalStatus | 'all'): string { + if (filter === 'all') return 'Withdrawals' + return `${filter[0].toUpperCase()}${filter.slice(1)} withdrawals` +} + +/** What the table below is, and is not, showing. */ +export function windowSentence(filter: WithdrawalStatus | 'all'): string { + return filter === 'all' + ? `The ${WITHDRAWAL_PAGE} most recent, newest first.` + : `The ${WITHDRAWAL_PAGE} most recent ${filter} withdrawals, newest first.` +} diff --git a/apps/admin/src/pages/Prices.tsx b/apps/admin/src/pages/Prices.tsx index d8316ef..a7751af 100644 --- a/apps/admin/src/pages/Prices.tsx +++ b/apps/admin/src/pages/Prices.tsx @@ -1,12 +1,22 @@ -import { useCallback, useEffect, useMemo, useState } from 'react' +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { SUPPORTED_DEPOSIT_COINS } from '@cloudsforge/shared' import { api, + ApiError, type AdministeredPrice, type AdminWithdrawal, type TreasuryAccount, type WithdrawalStatus, } from '../lib/api' +import { + pageSubject, + readStuckQueue, + readWithdrawalPage, + stuckNotice, + windowSentence, + type StuckQueue, +} from '../lib/withdrawalQueue' +import { readPanel, unreadableNotice, type PanelRead } from '../lib/panelRead' import Shell from '../components/Shell' import CopyButton from '../components/CopyButton' @@ -390,14 +400,228 @@ function truncate(s: string): string { return s.length <= 16 ? s : `${s.slice(0, 8)}…${s.slice(-6)}` } +// ---------- Abandon (the only write on a withdrawal) ---------- + +/** + * The statuses Forge Pay will refund from, mirroring the `from` list it passes to + * `refundWithdrawal` (its routes/admin.ts). `confirmed` and `failed` are absent because + * the money has already gone or already come back. + * + * `signed` is on this list even though it looks like the most dangerous one to offer — + * bytes exist and may be in a mempool. Leaving it off would hide the button on precisely + * the rows that strand money longest, and it would not make anything safer: pay decides, + * not this page, and it refuses a signed row unless the chain proves those bytes can + * never apply. A console that hides an action the server would allow is the defect this + * panel already had, one status further along. + */ +const ABANDONABLE: readonly WithdrawalStatus[] = ['pending', 'signed', 'broadcast', 'stuck'] + +const ABANDON_PHRASE = 'ABANDON' + +/** + * Codes on which Forge Pay declined to refund, as opposed to failing. + * + * Worth separating in the UI: a refusal is the safety check working, and the operator's + * next move is to read it and wait, not to retry or escalate. Rendering it in the same + * red as "the request broke" teaches people to dismiss both. + */ +const REFUSAL_CODES = new Set([ + 'withdrawal_on_chain', + 'withdrawal_still_applicable', + 'withdrawal_unprovable', + 'withdrawal_not_abandonable', + 'chain_unreachable', +]) + +function AbandonModal({ + withdrawal, + onClose, + onAbandoned, +}: { + withdrawal: AdminWithdrawal + onClose: () => void + onAbandoned: (updated: AdminWithdrawal) => void +}) { + const [phrase, setPhrase] = useState('') + const [understood, setUnderstood] = useState(false) + const [busy, setBusy] = useState(false) + const [error, setError] = useState<{ message: string; code?: string } | null>(null) + + const canAbandon = understood && phrase.trim() === ABANDON_PHRASE && !busy + // `pending` is the only state where nothing was ever signed, so it is the only one + // where no chain question is asked. Saying which of the two this is up front stops + // an operator reading a refusal on a signed row as a fault. + const signed = withdrawal.status !== 'pending' + + async function abandon() { + if (!canAbandon) return + setBusy(true) + setError(null) + try { + onAbandoned(await api.abandonWithdrawal(withdrawal.id)) + } catch (err) { + // Forge Pay's own sentence, unedited. It is the only place the reason is stated. + setError( + err instanceof ApiError + ? { message: err.message, code: err.code } + : { message: err instanceof Error ? err.message : 'Failed to abandon this withdrawal' }, + ) + } finally { + setBusy(false) + } + } + + const refusal = error?.code !== undefined && REFUSAL_CODES.has(error.code) + + return ( +
+
e.stopPropagation()} + > +
+ + ⚠ + +
+

+ Abandon and refund +

+

+ This gives up on the payment and returns{' '} + + {withdrawal.amount} {withdrawal.coin} + {' '} + to the user's custodial {withdrawal.coin} balance — the amount debited when + they asked to withdraw, fee included. No Shards move. +

+
+
+ +
+
To
+
{withdrawal.destination}
+
Network
+
+ {withdrawal.coin} · {withdrawal.network} +
+
Status
+
{withdrawal.status}
+
Withdrawal
+
{withdrawal.id}
+ {withdrawal.txid && ( + <> +
Txid
+
{withdrawal.txid}
+ + )} +
+ +
+ {signed ? ( + <> + Forge Pay asks the chain first and will refuse{' '} + unless it can prove these signed bytes can never be applied — an unreachable + node, or a transaction that could still land, means nothing is refunded and you + will be told so here. That check is the only thing standing between this button + and paying the user twice, so a refusal is the system working. + + ) : ( + <> + Nothing has been signed for this withdrawal yet, so there are no bytes that + could still land and the refund is unconditional. + + )} +
+ + + +
+ + setPhrase(e.target.value)} + placeholder={ABANDON_PHRASE} + /> +
+ + {error && ( +
+ {refusal && ( +
+ Forge Pay refused — nothing was refunded +
+ )} + {/* Verbatim. Pay words its own refusals and each one implies a different + next action; a summary of them here would erase the only part worth + reading. */} + {error.message} + {error.code && ( +
{error.code}
+ )} +
+ )} + +
+ + +
+
+
+ ) +} + // ---------- Page ---------- export default function Prices() { const [prices, setPrices] = useState(null) - const [treasuries, setTreasuries] = useState(null) - const [withdrawals, setWithdrawals] = useState(null) + // Both secondary panels hold a three-state read, never a bare array: an empty array here + // is a claim about the estate ("No treasury addresses configured.") and a failed request + // is not entitled to make it. See lib/panelRead.ts — CF-35. + const [treasuries, setTreasuries] = useState>({ kind: 'loading' }) + const [withdrawals, setWithdrawals] = useState>({ kind: 'loading' }) const [error, setError] = useState(null) const [statusFilter, setStatusFilter] = useState('all') + // How many withdrawals are stuck, from its own `?status=stuck` request. Never counted + // out of the table's rows — see lib/withdrawalQueue.ts for why that was wrong. + const [stuck, setStuck] = useState({ kind: 'loading' }) + // The row an operator is being asked to confirm abandoning, and the last one that was. + const [abandoning, setAbandoning] = useState(null) + const [abandoned, setAbandoned] = useState(null) const load = useCallback(async () => { setError(null) @@ -408,22 +632,61 @@ export default function Prices() { setError(err instanceof Error ? err.message : 'Failed to load prices') return } - // The two read-only panels below are context, not the page. Treasury in - // particular reads balances off-chain per request and can be slow or partly - // broken; neither is allowed to take the price panel down with it. - api - .listTreasury() - .then((r) => setTreasuries(r.treasuries)) - .catch(() => setTreasuries([])) - api - .listWithdrawals() - .then((r) => setWithdrawals(r.withdrawals)) - .catch(() => setWithdrawals([])) + // Treasury is context, not the page. It reads balances off-chain per request and can + // be slow or partly broken; it is not allowed to take the price panel down with it — + // `readPanel` never rejects, so that property is unchanged. What it no longer does is + // answer a failure with an empty list, which this panel renders as a statement that the + // treasury was never set up. + // Back to the skeleton first, so a re-read never shows last minute's balances beside a + // panel whose own copy says they are read from the chain and never cached. + setTreasuries({ kind: 'loading' }) + setTreasuries( + await readPanel('Treasury addresses', async () => (await api.listTreasury()).treasuries), + ) }, []) + const loadStuck = useCallback(async () => { + setStuck(await readStuckQueue(api.listWithdrawals)) + }, []) + + /** + * Latest-wins. Chips are clickable faster than pay answers, and two requests in flight + * can land out of order — without the sequence number, selecting `stuck` and then `all` + * can leave the all-statuses chip displaying the stuck page. It matters more now than + * it would have before: every chip is a round trip, where it used to be a local filter. + */ + const pageRequest = useRef(0) + + const loadWithdrawals = useCallback(async (filter: WithdrawalStatus | 'all') => { + const seq = ++pageRequest.current + setWithdrawals({ kind: 'loading' }) + // The failure is carried, not converted: `catch { rows = [] }` here rendered a 500 from + // pay as "No withdrawals yet." / "No stuck withdrawals." — the same reassuring absolute + // CF-33 removed from the header, still being said by the table underneath it. + const page = await readPanel(pageSubject(filter), () => + readWithdrawalPage(api.listWithdrawals, filter), + ) + if (seq === pageRequest.current) setWithdrawals(page) + }, []) + + // Prices and treasury on arrival only; the withdrawals table on every chip, because the + // chip is now a query rather than a filter. The two are separate effects so that + // changing the chip does not re-read a balance off a chain. + useEffect(() => { + load() + loadStuck() + }, [load, loadStuck]) + useEffect(() => { + loadWithdrawals(statusFilter) + }, [loadWithdrawals, statusFilter]) + + /** The Refresh button means the whole page, including both withdrawal reads. */ + const refresh = useCallback(() => { load() - }, [load]) + loadStuck() + loadWithdrawals(statusFilter) + }, [load, loadStuck, loadWithdrawals, statusFilter]) const administeredCoins = useMemo( () => new Set((prices ?? []).map((p) => p.coin)), @@ -441,15 +704,47 @@ export default function Prices() { [administeredCoins], ) - const visibleWithdrawals = useMemo( - () => - (withdrawals ?? []).filter( - (w) => statusFilter === 'all' || w.status === statusFilter, - ), - [withdrawals, statusFilter], - ) + // The sentence itself, not a number: "not asked yet", "could not be read" and "asked + // and told zero" are three different things to say and only the last of them is + // "Nothing is stuck." The wording lives in lib/withdrawalQueue.ts with the rule it + // keeps, so this page cannot restate the queue in terms the queue was not asked in. + const stuckLine = useMemo(() => stuckNotice(stuck), [stuck]) - const stuck = (withdrawals ?? []).filter((w) => w.status === 'stuck').length + /** + * Put the refunded row back where it was, in place. + * + * Not a reload of everything: `load()` refetches prices and treasury as well, and the + * treasury read goes to a chain per address — an operator who has just moved money + * should see the result of it immediately, not after the slowest panel on the page. Pay + * answers the abandon with the withdrawal as it now stands, so the row follows from + * this one substitution. + * + * TWO THINGS IT DOES NOT DO LOCALLY. The row is dropped rather than restyled when its + * new status no longer matches the selected chip: the chip is a query now, and leaving a + * `failed` row under `stuck` would be this panel claiming something about a status the + * server was not asked about — the same fault CF-33 fixed, one row wide. And the stuck + * count is re-read from pay instead of decremented here, because the number on that line + * is a fact about the queue and not an arithmetic consequence of what this tab just did. + */ + const replaceWithdrawal = useCallback( + (updated: AdminWithdrawal) => { + setWithdrawals((page) => { + // Only a page that was actually read has a row to substitute. Materialising one + // over a `loading` or `unreadable` panel would turn a single successful write into + // a one-row table that reads as the whole answer. + if (page.kind !== 'known') return page + return { + kind: 'known', + data: page.data.flatMap((w) => { + if (w.id !== updated.id) return [w] + return statusFilter === 'all' || updated.status === statusFilter ? [updated] : [] + }), + } + }) + loadStuck() + }, + [statusFilter, loadStuck], + ) return ( @@ -462,7 +757,7 @@ export default function Prices() { act, not a preference.

- @@ -544,17 +839,26 @@ export default function Prices() { when it matters.

- {treasuries === null &&
} + {treasuries.kind === 'loading' &&
} - {treasuries !== null && treasuries.length === 0 && ( + {/* The read failed. Rose and not fog, because this is a fault to act on and not a + quiet fact about the estate — and it is the block that used to say "No treasury + addresses configured." over a request nothing answered. */} + {treasuries.kind === 'unreadable' && ( +
+ {unreadableNotice('Treasury addresses', treasuries.reason)} +
+ )} + + {treasuries.kind === 'known' && treasuries.data.length === 0 && (
No treasury addresses configured.
)} - {treasuries !== null && treasuries.length > 0 && ( + {treasuries.kind === 'known' && treasuries.data.length > 0 && (
- {treasuries.map((t) => ( + {treasuries.data.map((t) => (
{t.coin} @@ -580,19 +884,20 @@ export default function Prices() {
)} - {/* ---------- Withdrawals (read-only) ---------- */} + {/* ---------- Withdrawals ---------- */}

Withdrawals

+ {/* Two sentences, and only the first of them is about the table. The second is + answered by its own `?status=stuck` request, so it stays true whichever chip + is selected and however far the table's window has been pushed. */}

- The 200 most recent, newest first.{' '} - {stuck > 0 ? ( - - {stuck} {stuck === 1 ? 'is' : 'are'} stuck and waiting on a human. - + {windowSentence(statusFilter)}{' '} + {stuckLine.urgent ? ( + {stuckLine.text} ) : ( - 'Nothing is stuck.' + stuckLine.text )}

@@ -614,9 +919,39 @@ export default function Prices() {
- {withdrawals === null &&
} + {/* The one outcome worth stating outside the modal: the modal closes on success, and + the row it changed is one line among two hundred. Naming the amount and the + destination is what makes it checkable against the ticket the operator is working. */} + {abandoned && ( +
+

+ Abandoned — {abandoned.amount} {abandoned.coin} is + back in the owner's custodial {abandoned.coin} balance. Nothing further is owed + on{' '} + {abandoned.destination}; the + row now reads {abandoned.status}. +

+ +
+ )} + + {withdrawals.kind === 'loading' &&
} + + {/* In place of the table, never inside it: an empty table body with an error row + reads as "here are the withdrawals, and there are none". */} + {withdrawals.kind === 'unreadable' && ( +
+ {unreadableNotice(pageSubject(statusFilter), withdrawals.reason)} +
+ )} - {withdrawals !== null && ( + {withdrawals.kind === 'known' && (
@@ -628,10 +963,11 @@ export default function Prices() { + - {visibleWithdrawals.map((w) => ( + {withdrawals.data.map((w) => ( {when(w.createdAt)} + ))} - {visibleWithdrawals.length === 0 && ( + {withdrawals.data.length === 0 && ( -
Status Confirmations CreatedAction
+ {ABANDONABLE.includes(w.status) ? ( + + ) : ( + // An em dash, not a disabled button: `confirmed` and `failed` are + // finished, and offering something greyed out on them invites the + // question of what would enable it. + — + )} +
+ + {/* An absence pay was asked for directly, AND answered: this row is + inside `kind === 'known'`, so it can no longer be reached by a + failed read. It used to be an absence from the newest 200 rows of + every status, which for `stuck` is the one absence in this console + nobody may be told wrongly. */} {statusFilter === 'all' ? 'No withdrawals yet.' : `No ${statusFilter} withdrawals.`} @@ -688,6 +1048,18 @@ export default function Prices() { )} + + {abandoning && ( + setAbandoning(null)} + onAbandoned={(updated) => { + replaceWithdrawal(updated) + setAbandoning(null) + setAbandoned(updated) + }} + /> + )} ) } diff --git a/apps/site/package.json b/apps/site/package.json index 029cf27..8395348 100644 --- a/apps/site/package.json +++ b/apps/site/package.json @@ -7,7 +7,8 @@ "dev": "vite --port 3000", "build": "tsc --noEmit && vite build", "preview": "vite preview --port 3000", - "typecheck": "tsc --noEmit" + "typecheck": "tsc --noEmit", + "test": "node --import tsx --test \"src/**/*.test.ts\"" }, "dependencies": { "@cloudsforge/shared": "^0.4.0", @@ -23,6 +24,7 @@ "@types/react-dom": "^19.0.0", "@vitejs/plugin-react": "^4.3.4", "tailwindcss": "^4.0.0", + "tsx": "^4.19.2", "typescript": "^5.7.3", "vite": "^6.0.0" } diff --git a/apps/site/src/lib/site.test.ts b/apps/site/src/lib/site.test.ts new file mode 100644 index 0000000..fdf58b6 --- /dev/null +++ b/apps/site/src/lib/site.test.ts @@ -0,0 +1,156 @@ +/* + * The public copy must not disagree with the code it describes. + * + * This site is marketing, so nothing here has a runtime behaviour to test. What + * it has instead is claims, and the /roadmap page makes one about itself: + * "nothing claimed here that you cannot go and use". The defect these cases + * exist for (CF-37) was that inversion: EMBER moved from crediting at the chain + * tip to waiting a real confirmation depth, and cosmetics moved from + * localStorage to a database row that every roster renders, and both sentences + * on the site went on saying the old thing. The site was understating what runs. + * + * So the assertions are of two kinds: + * + * 1. AGAINST THE REGISTRY. `@cloudsforge/shared`'s deposit registry is the + * same module forge-pay's watcher reads, so EMBER's depth here is EMBER's + * depth there. A case that hardcoded 60 would pass on the day it was + * written and rot exactly like the copy did; these read the registry. + * + * 2. AGAINST THE OLD CLAIMS. A fail-closed sweep over every string this module + * exports, so a retired claim cannot come back in a sentence nobody thought + * to re-check. It walks the exports rather than naming the constants: new + * copy is covered the moment it is added, which is the only way a rule like + * this survives contact with a marketing page. + * + * Run: pnpm --filter @cloudsforge/site test + */ +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { depositChainInfo, SUPPORTED_DEPOSIT_COINS } from '@cloudsforge/shared' +import { + EMBER_CONFIRMATIONS, + EMBER_CONFIRMATION_MINUTES, + LOOP_CAVEAT, + ROADMAP, +} from './site.ts' +import * as site from './site.ts' + +/** Every string reachable from the module's exports, however deeply nested. */ +function everyString(value: unknown, seen = new Set()): string[] { + if (typeof value === 'string') return [value] + if (value === null || typeof value !== 'object') return [] + if (seen.has(value)) return [] + seen.add(value) + return Object.values(value as Record).flatMap((v) => everyString(v, seen)) +} + +const COPY = everyString(site) + +/** The roadmap entry the register names, found by title so a reorder is fine. */ +const emberEntry = ROADMAP.find((e) => e.title === 'EMBER credited honestly') +const forSaleEntry = ROADMAP.find((e) => e.title === 'Deliver everything already for sale') + +test('the depth the site quotes is the depth Forge Pay credits at', () => { + // THE LOAD-BEARING CASE. `site.ts` writes the number rather than importing it, + // because the shared entrypoint drags zod into the browser bundle and does not + // tree-shake out (+15 kB gzipped for one integer; the reasoning is above + // `EMBER_CONFIRMATIONS`). This is what makes that safe: the registry is read + // here, in node, where it is free — so the day someone changes EMBER's depth in + // shared-libs, this repo's build goes red instead of the site going quietly + // stale, which is precisely how it went stale last time. + assert.equal(EMBER_CONFIRMATIONS, depositChainInfo('EMBER').confirmations) + assert.ok(EMBER_CONFIRMATIONS > 0, 'the site must not claim a zero-confirmation credit') + assert.equal(EMBER_CONFIRMATION_MINUTES, Math.round((EMBER_CONFIRMATIONS * 15) / 60)) +}) + +test('the pages that quote a depth quote the registry’s depth', () => { + const depth = `${depositChainInfo('EMBER').confirmations} blocks` + assert.ok(LOOP_CAVEAT.includes(depth), `the home-page caveat should say "${depth}"`) + assert.ok(emberEntry, 'the roadmap entry for EMBER should still exist') + assert.ok(emberEntry!.body.includes(depth), `the roadmap entry should say "${depth}"`) +}) + +test('no public copy claims EMBER credits at the chain tip', () => { + // Verbatim fragments of the sentences CF-37 found, plus the shapes they could + // come back as. forge-pay reads `eth_getBalance` at `latest - confirmations` + // and the tip reading only ever reaches `pending` — there is no code path that + // credits at the tip, so no sentence here may say there is. + const banned = [ + /credits? (?:it )?at the (?:chain )?tip/i, + /instead of waiting for confirmations/i, + /exposes only the balance at the tip/i, + /counts the moment it is seen/i, + /without waiting for confirmations/i, + ] + for (const pattern of banned) { + const offender = COPY.find((s) => pattern.test(s)) + assert.equal(offender, undefined, `retired claim ${pattern} is back in: ${offender}`) + } +}) + +test('EMBER is on the shipped side of the roadmap, with every other coin', () => { + assert.ok(emberEntry, 'the roadmap entry for EMBER should still exist') + // Every supported coin now has a non-zero depth and is credited at it, so an + // entry saying this is still being worked on is the understatement the ticket + // is about. It stays as a Shipped entry rather than being deleted — the work + // happened, and a roadmap that only lists what is missing is not a roadmap. + assert.equal(emberEntry!.phase, 'Shipped') + assert.equal(emberEntry!.status, 'shipping') + for (const chain of SUPPORTED_DEPOSIT_COINS) { + assert.ok(chain.confirmations > 0, `${chain.coin} must credit at a real depth`) + } +}) + +test('the caveat survives, and for the reason that is still true', () => { + // Not "delete the caveat": EMBER waits its depth now, but the deposit registry + // says the controls that matter on a young CPU-mined chain are economic and + // "live in forge-pay", and they are not implemented there. The caveat is only + // honest while it names that, so this asserts the reason and not just that + // some sentence is present. + assert.ok(/\bcaps?\b/i.test(LOOP_CAVEAT), 'the caveat should name the missing per-user caps') + assert.ok(/reorg/i.test(LOOP_CAVEAT), 'the caveat should name the missing reorg halt') +}) + +test('no public copy claims cosmetics are private to your own browser', () => { + // A cosmetic is a `player_cosmetics` row, checked against Pay's entitlements on + // equip and fanned out to `players.cosmeticStyle` for every world you are in + // (ninety-days-after `services/game/src/routes/cosmetics.ts`). Other people see + // it. The old sentence was an argument against buying one, and it was wrong. + const banned = [ + /stored in your own browser/i, + /nobody else ever sees/i, + /only you (?:can )?see/i, + ] + for (const pattern of banned) { + const offender = COPY.find((s) => pattern.test(s)) + assert.equal(offender, undefined, `retired claim ${pattern} is back in: ${offender}`) + } +}) + +test('the undelivered private world is still admitted to', () => { + // The other half of that roadmap entry is NOT stale and must not be swept away + // with the cosmetics sentence: Forge Pay still charges for `private_world` and + // nothing in the estate provisions one. + assert.ok(forSaleEntry, 'the "already for sale" entry should still exist') + assert.match(forSaleEntry!.body, /private world/i) + assert.match(forSaleEntry!.body, /provisions nothing/i) +}) + +test('the entry names every kind the game classifies as undelivered', () => { + // Striking the cosmetics sentence left this entry naming the private world and + // nothing else, while `ninety-days-after/apps/game/src/lib/shop.ts` enumerates + // three: `private_world` ("no world was raised"), `convenience` ("the feature + // does not exist") and `cosmetic` ("nothing draws it"). An exhaustive-sounding + // list of one is the same understatement this file exists to catch, and it is + // the shape the next SKU that ships undelivered would drift into. + // + // The kinds are matched by the words the copy has to use rather than by + // importing the game's module: that repo is a sibling checkout and may not be + // present, and this suite has to be able to run on a runner that has only + // this one. A rename over there breaks nothing here; a SENTENCE going missing + // over here is what this catches. + assert.ok(forSaleEntry, 'the "already for sale" entry should still exist') + const body = forSaleEntry!.body + assert.match(body, /convenience/i, 'the convenience items are sold and undelivered too') + assert.match(body, /cosmetic kinds nothing draws/i, 'so are the cosmetic kinds with no renderer') +}) diff --git a/apps/site/src/lib/site.ts b/apps/site/src/lib/site.ts index 7f52b3a..7181ec5 100644 --- a/apps/site/src/lib/site.ts +++ b/apps/site/src/lib/site.ts @@ -196,12 +196,62 @@ export const LOOP = [ ].map((s) => ({ ...s, accent: surface(s.accentKey).accent })) /** - * The part of the loop we will not oversell. EMBER is the only coin Forge Pay - * credits at the chain tip rather than after confirmations, because Hearth's API - * exposes nothing else yet. + * EMBER's confirmation depth — the number Forge Pay actually credits at, said + * once so the two places that quote it cannot disagree with each other. + * + * IT IS WRITTEN HERE AND NOT IMPORTED, which needs defending, because the file's + * whole habit is to derive rather than restate. `depositChainInfo('EMBER')` in + * `@cloudsforge/shared` is the registry forge-pay's watcher itself reads (it + * probes `eth_getBalance` at `latest - confirmations`, `services/pay/src/ + * chains.ts`), so importing it would make the drift impossible by construction — + * but the shared entrypoint carries zod, and it does not tree-shake out: the + * import costs this bundle +62 kB raw / +15 kB gzipped, measured, on the front + * door of the site. `src/lib/obs.tsx` refused the same trade for the same reason + * and reaches for the zero-dependency `/products` subpath instead; there is no + * equivalent subpath for the deposit registry. + * + * SO THE BINDING IS A TEST INSTEAD, and it is not optional: `site.test.ts` reads + * `depositChainInfo('EMBER').confirmations` — in node, where zod is free — and + * fails if the copy below does not quote it. Move the depth in shared-libs and + * this repo's build goes red until someone rewrites the sentences, which is what + * has to happen anyway: "about fifteen minutes" is not a number a script can + * update. + * + * That is the defect this constant exists for. EMBER stopped crediting at the + * chain tip when Hearth became an account-model EVM chain, and the site went on + * saying it did, because nothing anywhere would ever have noticed. + */ +export const EMBER_CONFIRMATIONS = 60 + +/** + * Hearth's block interval, in seconds. It matches the "15-second blocks" already + * claimed in the Hearth product copy above, and the ~15 minutes Hearth's own + * exchange-integration guide quotes for a 60-block deposit — change all three + * together or none. + */ +const HEARTH_BLOCK_SECONDS = 15 + +/** The depth spelled as a wait, because "60 blocks" tells a reader nothing. */ +export const EMBER_CONFIRMATION_MINUTES = Math.round( + (EMBER_CONFIRMATIONS * HEARTH_BLOCK_SECONDS) / 60, +) + +/** + * The part of the loop we will not oversell — and the reason has changed. + * + * It used to be that EMBER credited at the chain tip. It does not any more: it + * waits the depth above, the same depth Hearth publishes to exchanges. What is + * still true is that depth was never the real control on a young chain mined + * with ordinary CPUs — the deposit registry itself says the controls that matter + * are economic (per-user caps, a halt on any reorg past a few blocks) and that + * they "live in forge-pay". They are not implemented there yet; checked before + * this sentence was written, and there is nothing in + * `forge-pay/services/pay/src` that caps a user or halts on a reorg. So the + * caveat stays and only its reason changes. Delete it when those land, not + * before. */ export const LOOP_CAVEAT = - 'Hearth is on testnet, and EMBER is the one coin Forge Pay still credits at the chain tip instead of waiting for confirmations. Until that is fixed it is a loop we will demonstrate, not one we would ask you to trust with size.' + `Hearth is on testnet, and an EMBER deposit now waits ${EMBER_CONFIRMATIONS} blocks — about ${EMBER_CONFIRMATION_MINUTES} minutes — before it credits, the same depth Hearth publishes to exchanges. Depth is not the whole story on a young chain mined with ordinary CPUs: the per-user caps and the automatic halt on a deep reorg are not built yet. It is a loop we will demonstrate, not one we would ask you to trust with size.` /** Company accent, for the surfaces that carry no product colour of their own. */ export const COMPANY_ACCENT = CLOUDSFORGE_EMBER @@ -355,11 +405,11 @@ export const ROADMAP = [ body: 'Hearth mines on testnet with real proof-of-work, difficulty retargeting and reorgs, and a browser wallet that signs its own transactions. ForgeMint deploys compiled contracts across five EVM chains and Solana. Crucible backtests ten strategies against real exchange history with fees and slippage charged. Ninety Days After resolves a full 90-day season and seals it into a readable archive.', }, { - phase: 'Now', + phase: 'Shipped', title: 'EMBER credited honestly', - status: 'building', + status: 'shipping', spans: 'Hearth · Forge Pay', - body: 'BTC, ETH, SOL and XRP deposits already wait for real confirmation depth before they credit. EMBER does not — Hearth’s API exposes only the balance at the tip, so a deposit counts the moment it is seen. Fixing that needs a change in the node, and it is the single thing standing between the mine-to-spend loop and a headline.', + body: `Every coin waits for real confirmation depth before it credits, EMBER included. Hearth became an account-model chain with an Ethereum JSON-RPC, so Forge Pay reads your balance as of ${EMBER_CONFIRMATIONS} blocks back — about ${EMBER_CONFIRMATION_MINUTES} minutes — rather than at the tip: a deposit is visible as pending the moment we see it, and spendable only once it is that deep. That is the same depth Hearth publishes to exchanges. What is not built yet is the economics around it — per-user deposit caps, and an automatic halt on a deep reorg — which is why we still say not to trust the loop with size.`, }, { phase: 'Now', @@ -373,7 +423,22 @@ export const ROADMAP = [ title: 'Deliver everything already for sale', status: 'planned', spans: 'Games · Forge Pay', - body: 'A rented private world charges Shards and provisions nothing. Cosmetics are stored in your own browser, so nobody else ever sees what you bought — which removes the entire reason to buy one. Both are sold today. Both get built or pulled.', + // The cosmetics sentence that used to be here came off, not the entry: what + // an account wears is a row in the game's database now, entitlement-checked + // against Forge Pay on equip and fanned out to every roster you appear on, + // so the "nobody else sees it" half was simply false. + // + // What replaced it is the game's own list rather than the one thing this + // entry used to name. `ninety-days-after/apps/game/src/lib/shop.ts` + // classifies THREE kinds as sold-and-not-delivered — `private_world` ("no + // world was raised"), `convenience` ("the feature does not exist") and + // `cosmetic` for the kinds with no renderer ("nothing draws it") — and the + // shop withholds all three listings. Withholding a listing stops the next + // charge and does nothing for the ones already taken, and Forge Pay still + // serves every one of those routes to any caller that is not that page. + // Naming one of the three read as an exhaustive list of one, which is the + // same understatement CF-37 is about. + body: 'A rented private world charges Shards and provisions nothing: Forge Pay takes the money, writes the entitlement, and no world is ever raised from it. Two more sit beside it — four convenience items whose features do not exist, and cosmetic kinds nothing draws. The game has pulled all three listings from its shop, which stops the next charge and does nothing for the money already taken; Forge Pay still answers those routes for anything that is not that page. They get built or they get refunded.', }, { phase: 'Next', diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 30ee869..9fdf48c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -54,6 +54,9 @@ importers: tailwindcss: specifier: ^4.0.0 version: 4.3.3 + tsx: + specifier: ^4.19.2 + version: 4.23.1 typescript: specifier: ^5.7.3 version: 5.9.3 @@ -97,6 +100,9 @@ importers: tailwindcss: specifier: ^4.0.0 version: 4.3.3 + tsx: + specifier: ^4.19.2 + version: 4.23.1 typescript: specifier: ^5.7.3 version: 5.9.3 @@ -130,6 +136,9 @@ importers: jose: specifier: ^5.9.6 version: 5.10.0 + nodemailer: + specifier: ^8.0.11 + version: 8.0.11 postgres: specifier: ^3.4.5 version: 3.4.9 @@ -140,6 +149,9 @@ importers: '@types/node': specifier: ^22.10.5 version: 22.20.1 + '@types/nodemailer': + specifier: ^8.0.1 + version: 8.0.1 tsx: specifier: ^4.19.2 version: 4.23.1 @@ -862,6 +874,9 @@ packages: '@types/node@22.20.1': resolution: {integrity: sha512-EANqOCF9QFyra+4pfxUcX9STKJpCLjMbObVzljIJomAWSnuSIEAvyzEU53GaajbXJEgdh0iEcPL+DGvpUd4k1Q==} + '@types/nodemailer@8.0.1': + resolution: {integrity: sha512-PxpaInm8V1JQDd4j0ds5HfvWQk8JupS1C0Picb96QJsrrRDjBH+DlK7L4ZdNSqNULhiZRQHc40nLVShaGxXAMw==} + '@types/react-dom@19.2.3': resolution: {integrity: sha512-jp2L/eY6fn+KgVVQAOqYItbF0VY/YApe5Mz2F0aykSO8gx31bYCZyvSeYxCHKvzHG5eZjc+zyaS5BrBWya2+kQ==} peerDependencies: @@ -1281,6 +1296,10 @@ packages: resolution: {integrity: sha512-wRNIrw4DmVLKQlbgOMdkMx27Wrpzes2hh5Jtbi2bjPd+4wJstWIqP5A+lscnqbm0xxmT5Bpg8Lec5ItEBwx6BQ==} engines: {node: '>=18'} + nodemailer@8.0.11: + resolution: {integrity: sha512-nrO/pDAUKl+wXX+lx16tDLbnm0fW6sK/x8mgohaCpg+CdCEl482bD4tCuAZk2DyliruiNTIZxRCoWkDqJEnAiA==} + engines: {node: '>=6.0.0'} + on-exit-leak-free@2.1.2: resolution: {integrity: sha512-0eJJY6hXLGf1udHwfNftBqH+g73EU4B504nZeKpz1sYRKafAghwxEJunB2O7rDZkL4PGfsMVnTXZ2EjibbqcsA==} engines: {node: '>=14.0.0'} @@ -2041,6 +2060,10 @@ snapshots: dependencies: undici-types: 6.21.0 + '@types/nodemailer@8.0.1': + dependencies: + '@types/node': 22.20.1 + '@types/react-dom@19.2.3(@types/react@19.2.17)': dependencies: '@types/react': 19.2.17 @@ -2369,6 +2392,8 @@ snapshots: node-releases@2.0.51: {} + nodemailer@8.0.11: {} + on-exit-leak-free@2.1.2: {} path-scurry@2.0.2: diff --git a/services/nimbus/.env.example b/services/nimbus/.env.example index 740a894..8eecfcf 100644 --- a/services/nimbus/.env.example +++ b/services/nimbus/.env.example @@ -13,6 +13,27 @@ # "use the libpq defaults" and dials a database nobody configured. NIMBUS_DATABASE_URL=postgres://cloudsforge:cloudsforge@localhost:5432/nimbus +# Encrypts the RS256 signing key at rest. REQUIRED, min 24 chars, placeholders +# refused — the service will not fall back to storing the key in the clear. +# Generate: openssl rand -hex 32 +# +# WHY IT IS NOT OPTIONAL. The private JWK is the estate's universal forging +# credential: every service verifies `iss` plus `aud: cloudsforge` and nothing +# else, so a self-minted {sub, roles:['admin']} is admin in all of them. It used +# to sit as plaintext JSONB in a database every service shares one role on, which +# turned any read-only leak — a stolen backup, a SELECT-only injection, a copied +# dump — into silent, offline, unrevocable impersonation everywhere. +# +# KEEP IT OUT OF THE DATABASE AND OUT OF BACKUPS OF IT. That separation is the +# entire mechanism; a copy of this string stored next to the ciphertext protects +# nothing. +# +# Changing it makes the stored key undecryptable, and nimbus says so rather than +# signing with something else. Recover by minting a replacement key +# (POST /admin/signing-keys, then activate it), or on a deployment where every +# session may be dropped, by deleting the signing_keys rows and restarting. +NIMBUS_KEY_SECRET= + # --- Identity ------------------------------------------------------------- NIMBUS_PORT=4001 @@ -21,13 +42,80 @@ NIMBUS_PORT=4001 # public https origins is logged as an error at boot. NIMBUS_ISSUER=http://localhost:4001 +# Where the account portal lives for real users, and the ONLY thing a link that +# leaves this process is built from — today that means the password-reset link, +# both the emailed one and the one the operator console is handed. +# +# It is configuration and not the request's Host header on purpose. A reset link +# is minted for one person and read by another, out of an email the deployment's +# own relay sent; deriving it from the request let an unauthenticated stranger +# choose where it pointed, with `curl -H 'Host: evil.test' -X POST +# /auth/password/forgot`. Nothing about the fragment helps — the attacker's page +# reads location.hash. +# +# Unset, it falls back to NIMBUS_ISSUER, which is already the URL browsers reach +# this service on. Set it when the portal has a friendlier hostname than the +# issuer does: in the composed stack that is https://account. while the +# issuer is https://nimbus., and compose sets it for you. +# NIMBUS_PUBLIC_URL=http://localhost:4001 + +# --- Outbound mail -------------------------------------------------------- +# The only mail this service sends: the password-reset link. Set SMTP_HOST and a +# user who clicks "forgot password" is emailed a single-use link that expires in +# 30 minutes; leave it unset and nothing is sent — the request is still recorded +# and an operator issues the link from the admin console (Users → Reset +# password), which is how it worked before this existed and is a supported way +# to run, not a degraded one. Nimbus says which of the two it is, at info, on +# every boot, and both the /forgot page and the 202 body change their wording to +# match, so a user is never promised an email nobody can send. +# +# There is no provider code and no SDK — this is plain SMTP, so Brevo, Resend, +# SendGrid, Mailtrap, a Gmail app password and your own postfix are the same four +# settings and changing provider is an edit here and a restart. Free credentials +# in two minutes: Brevo (300/day, no card, no custom domain) — SMTP & API → SMTP +# → generate a key, then SMTP_HOST=smtp-relay.brevo.com, SMTP_PORT=587, +# SMTP_USER=, SMTP_PASS=, SMTP_FROM=. +# +# The link that is emailed is built from NIMBUS_PUBLIC_URL above and never from +# the request — read the note there before changing either. +# +# Unset = no mail. Everything below it is only read when it is set. +SMTP_HOST= +# 587 is STARTTLS and is what every provider documents first. 465 is implicit TLS +# and additionally needs SMTP_SECURE=true; the two are NOT interchangeable, and +# 465 with secure unset hangs until the connect timeout. +SMTP_PORT=587 +SMTP_SECURE=false +SMTP_USER= +# Treat as a secret: it can send mail as you, and nothing rate-limits that but +# the provider. Never your account password — providers issue a separate key. +SMTP_PASS= +# The From header, as a full RFC 5322 address. Must be an address the provider +# has authorised for you, or it accepts the connection and rejects the message. +# SMTP_HOST set without this (or without USER/PASS) is a boot-time error, not a +# silent fallback to no mail. +SMTP_FROM=CloudsForge +# Optional. Where a reply goes, if you would rather it was not the From address. +SMTP_REPLY_TO= + # --- Origins -------------------------------------------------------------- # Browser origins allowed to call this API (CORS). Comma-separated, never a -# wildcard. account. is always allowed on top of whatever is set here. +# wildcard, and EXACTLY what you write here — nothing is appended. +# +# This used to say account. was always allowed on top. It was not: the +# service appended the literal https://account.cloudsforge.online, which on any +# other apex is a foreign domain, and never your account.. The portal is +# served by Nimbus itself and calls this API same-origin, so it needs no entry; +# list account. yourself only if something ELSE is served from there. In +# the composed stack docker-compose.yml supplies the whole list. CORS_ORIGINS=http://localhost:3000,http://localhost:3001,http://localhost:3002,http://localhost:3003,http://localhost:4004,http://localhost:4006,http://localhost:4010 # Origins the account portal will hand a single-use sign-in code back to. # Defaults to CORS_ORIGINS. Product surfaces only — never an API service. +# Because it defaults to the list above, an origin added there for CORS also +# becomes a legal destination for a live sign-in code. Set this explicitly when +# those two sets differ. # PORTAL_ALLOWED_ORIGINS= # Which peers may set X-Forwarded-For/-Proto/-Host. Defaults to private ranges, diff --git a/services/nimbus/package.json b/services/nimbus/package.json index 798c29b..9668229 100644 --- a/services/nimbus/package.json +++ b/services/nimbus/package.json @@ -7,7 +7,8 @@ "dev": "tsx watch src/index.ts", "start": "tsx src/index.ts", "typecheck": "tsc --noEmit", - "build": "tsc --noEmit" + "build": "tsc --noEmit", + "test": "node --import tsx --test --test-concurrency=1 src/*.test.ts" }, "dependencies": { "@cloudsforge/shared": "^0.4.0", @@ -18,11 +19,13 @@ "drizzle-orm": "^0.45.2", "fastify": "^5.2.0", "jose": "^5.9.6", + "nodemailer": "^8.0.11", "postgres": "^3.4.5", "zod": "^3.24.1" }, "devDependencies": { "@types/node": "^22.10.5", + "@types/nodemailer": "^8.0.1", "tsx": "^4.19.2", "typescript": "^5.7.3" } diff --git a/services/nimbus/src/bootLog.ts b/services/nimbus/src/bootLog.ts index 64df58d..fcafffc 100644 --- a/services/nimbus/src/bootLog.ts +++ b/services/nimbus/src/bootLog.ts @@ -7,7 +7,10 @@ * Fields mirror loggerOptions() in ./obs.ts so the two are indistinguishable * downstream. */ -const LEVELS = { warn: 40, error: 50, fatal: 60 } as const +// `info` is here for the settings that are legitimately optional: a deployment +// that has not configured outbound mail is a supported one, and announcing it at +// warn trains an operator to ignore the level that the broken settings use. +const LEVELS = { info: 30, warn: 40, error: 50, fatal: 60 } as const export function bootLog( level: keyof typeof LEVELS, diff --git a/services/nimbus/src/db/migrate.ts b/services/nimbus/src/db/migrate.ts index 5b9ee69..51424b7 100644 --- a/services/nimbus/src/db/migrate.ts +++ b/services/nimbus/src/db/migrate.ts @@ -104,16 +104,104 @@ const STEPS: { name: string; ddl: string }[] = [ ); `, }, + { + name: 'refresh_token_rotation_grace', + // Nullable and with no backfill on purpose. `revoked` alone cannot say WHY a + // row is spent, and the family burn is only correct for a replay of a token + // that was already rotated some time ago; a token rotated 200ms ago is two + // tabs refreshing at once, not a thief. rotated_at is the only thing that + // distinguishes them. Every row that exists when this runs is left null, so + // it keeps exactly today's behaviour — a re-presentation of it burns the + // family — and only rotations performed after this deploy earn the grace. + ddl: ` + ALTER TABLE refresh_tokens + ADD COLUMN IF NOT EXISTS rotated_at TIMESTAMPTZ; + `, + }, + { + name: 'signing_key_envelope_and_rotation', + // Three changes, one step, because a half-applied set of them is a service + // that cannot decide which key signs. + // + // 1. private_jwk_enc holds the AES-256-GCM envelope (src/keyEnvelope.ts). + // 2. private_jwk stops being NOT NULL so keys.ts can empty it in place. The + // column is NOT dropped here: dropping it in the same deploy that starts + // writing the new one leaves no way back if the envelope is wrong, and a + // NULL column is auditable — `SELECT count(*) FROM signing_keys WHERE + // private_jwk IS NOT NULL` is the question "is the key still readable to + // anyone who can read this database", and it should answer 0. + // 3. status/status_changed_at make rotation expressible. Existing rows + // default to 'active' and now(), which is what they are: the one key that + // signs. The CHECK is deliberate — a typo'd status that silently drops a + // key out of the JWKS would present as every service rejecting every + // token, a long way from the UPDATE that caused it. + ddl: ` + ALTER TABLE signing_keys + ADD COLUMN IF NOT EXISTS private_jwk_enc TEXT, + ADD COLUMN IF NOT EXISTS status TEXT NOT NULL DEFAULT 'active', + ADD COLUMN IF NOT EXISTS status_changed_at TIMESTAMPTZ NOT NULL DEFAULT now(); + + ALTER TABLE signing_keys + ALTER COLUMN private_jwk DROP NOT NULL; + + DO $$ + BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint WHERE conname = 'signing_keys_status_check' + ) THEN + ALTER TABLE signing_keys + ADD CONSTRAINT signing_keys_status_check + CHECK (status IN ('active', 'published', 'retired')); + END IF; + END $$; + + CREATE INDEX IF NOT EXISTS signing_keys_status_idx ON signing_keys(status); + `, + }, ] +/** + * The advisory-lock key this migration serialises on. + * + * Any 64-bit constant would do; this one is `nimbus` in hex so that a + * `SELECT * FROM pg_locks WHERE locktype = 'advisory'` during a stuck boot names + * the thing holding it. + */ +const MIGRATION_LOCK_KEY = 0x6e696d627573 // 'nimbus', 121399085790579 + export async function migrate(log: FastifyBaseLogger): Promise { - for (const { name, ddl } of STEPS) { - try { - await sql.unsafe(ddl) - } catch (err) { - log.error({ err, migration: name }, `migration failed: ${name}`) - throw err + /* ONE MIGRATOR AT A TIME, and the lock is not belt-and-braces. + * + * `CREATE TABLE IF NOT EXISTS` is idempotent but it is NOT concurrency-safe: + * two sessions that both find the table missing both proceed to create it, and + * the loser gets `duplicate key value violates unique constraint + * "pg_type_typname_nsp_index"` out of the system catalogue rather than a + * clean no-op. The same is true of `ADD COLUMN IF NOT EXISTS` and of the + * DO-block that adds the status CHECK. That is not hypothetical: two test + * suites that node --test was running in parallel against one scratch database + * raced here, and the failure — `duplicate key … pg_type_typname_nsp_index` — + * named neither of them. The suites take turns now (package.json's + * --test-concurrency=1), which is why that particular collision cannot recur; + * the lock is for the case nothing can serialise from the outside, which is + * two containers of this service starting together. + * + * pg_advisory_xact_lock rather than the session form: it is released by the + * commit or the rollback, so a migration that throws cannot leave the lock + * held by a pooled connection that is handed to the next caller. Taking it + * inside sql.begin() is also what pins every statement below to ONE + * connection, which the pool does not otherwise promise. */ + await sql.begin(async (tx) => { + await tx`SELECT pg_advisory_xact_lock(${MIGRATION_LOCK_KEY})` + for (const { name, ddl } of STEPS) { + try { + await tx.unsafe(ddl) + } catch (err) { + // Named here and rethrown, so the boot dies on the step that broke + // rather than serving requests against a half-migrated schema. + log.error({ err, migration: name }, `migration failed: ${name}`) + throw err + } } - } + }) log.info({ migrations: STEPS.length }, 'migrations applied') } diff --git a/services/nimbus/src/db/schema.ts b/services/nimbus/src/db/schema.ts index 0e083e2..f049120 100644 --- a/services/nimbus/src/db/schema.ts +++ b/services/nimbus/src/db/schema.ts @@ -26,6 +26,13 @@ export const refreshTokens = pgTable('refresh_tokens', { .default(sql`gen_random_uuid()`), expiresAt: timestamp('expires_at', { withTimezone: true }).notNull(), revoked: boolean('revoked').notNull().default(false), + // When this row was spent BY A ROTATION, and only by a rotation. `revoked` is + // set by four different things (rotation, logout, a password change, a family + // burn) and cannot tell them apart; this column can, which is what lets a + // re-presentation seconds after a rotation be answered as the concurrent + // refresh it almost always is instead of as theft. Null on a live row and on + // every row revoked some other way — those keep burning the family. + rotatedAt: timestamp('rotated_at', { withTimezone: true }), createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), }) @@ -66,11 +73,44 @@ export const passwordResetTokens = pgTable('password_reset_tokens', { createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), }) +/** + * Where a signing key is in its life. Exactly one row is `active` at a time. + * + * `published` is the state that makes rotation safe in both directions. A key + * must be in the JWKS before it signs anything, or the tokens it mints are + * rejected by every service whose JWKS cache has not refreshed; and a key must + * STAY in the JWKS after it stops signing, until the last access token it minted + * has expired. Both of those are the same state — in the document, not signing — + * so there is one name for it. + * + * `retired` is the only state that leaves the JWKS. Nothing deletes a row: the + * kid of a key that ever signed is worth keeping so an old token in a log can be + * attributed to the key that made it. + */ +export const SIGNING_KEY_STATUSES = ['active', 'published', 'retired'] as const +export type SigningKeyStatus = (typeof SIGNING_KEY_STATUSES)[number] + export const signingKeys = pgTable('signing_keys', { kid: text('kid').primaryKey(), - privateJwk: jsonb('private_jwk').$type>().notNull(), + // The private half, AES-256-GCM under NIMBUS_KEY_SECRET (../keyEnvelope.ts). + // Nullable only because the plaintext column below outlived it for one deploy; + // a row written by this build always has it. + privateJwkEnc: text('private_jwk_enc'), + // THE PLAINTEXT COLUMN, KEPT ONLY TO BE EMPTIED. It held the RS256 private key + // as readable JSONB — the estate's universal forging credential, in a database + // eight services share a role on (CF-16). keys.ts re-encrypts any row that + // still has one at boot and NULLs it in the same statement. The column stays + // so that upgrade is a no-op on a fresh database and so nothing silently + // writes here again; the day every deployment has run this build once, it can + // be dropped. Note what re-encrypting does NOT undo: a backup taken before the + // upgrade still contains the key, and the only fix for that is rotation. + privateJwk: jsonb('private_jwk').$type>(), publicJwk: jsonb('public_jwk').$type>().notNull(), + status: text('status').$type().notNull().default('active'), createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(), + // When it entered its current state. `activate` refuses a key that has not + // been published for a whole access-token TTL, and that is the clock it reads. + statusChangedAt: timestamp('status_changed_at', { withTimezone: true }).notNull().defaultNow(), }) export type UserRow = typeof users.$inferSelect diff --git a/services/nimbus/src/env.test.ts b/services/nimbus/src/env.test.ts new file mode 100644 index 0000000..ba3f790 --- /dev/null +++ b/services/nimbus/src/env.test.ts @@ -0,0 +1,208 @@ +import assert from 'node:assert/strict' +import { execFile } from 'node:child_process' +import { dirname, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' +import { promisify } from 'node:util' +import { test } from 'node:test' + +/** + * What the origin allowlists are allowed to contain. + * + * WHY THIS TEST SPAWNS A PROCESS. env.ts is evaluated once, at import, off `process.env`, + * and it calls `process.exit(1)` on a bad database URL. There is no seam to inject a + * different environment into, and `import()`-ing it twice in one process gets the module + * cache, not a re-read. So each case is a real boot of the real module with a real + * environment, and the assertions are made against what that boot produced. Anything less + * would be testing a copy of the logic rather than the logic. + * + * THE DEFECT (CF-47). env.ts appended the literal `https://account.cloudsforge.online` to + * corsOrigins on every boot, on the reasoning that nimbus serves the account portal. The + * apex is configurable — compose builds every origin from ${CLOUDSFORGE_APEX} — so on any + * other apex that is a host on somebody else's domain. And corsOrigins is the DEFAULT for + * portalAllowedOrigins, the return-URL allowlist: a foreign-apex deployment that had not set + * PORTAL_ALLOWED_ORIGINS would accept `/login?return=https://account.cloudsforge.online/...` + * and hand that domain a live single-use sign-in code (routes/portal.ts:74-77). + * + * `shipped()` below is the derivation as it stood before the fix, run over the same inputs, + * so each case shows itself catching the version that had the bug. + * + * NOTE ON HERMETICITY. env.ts loads services/nimbus/.env and then the repo root .env, and + * dotenv keeps the first value it sees — but it never overwrites a variable already present + * in the environment. Every variable a case depends on is therefore set explicitly below, + * except PORTAL_ALLOWED_ORIGINS, whose absence is the whole point of the second case. A + * stray local .env that sets it will fail this file loudly rather than pass it quietly. + */ + +const execFileAsync = promisify(execFile) +const pkgRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..') + +/** The corsOrigins derivation as it was before this change. */ +function shipped(configured: string[]): string[] { + const out = [...configured] + if (!out.includes('https://account.cloudsforge.online')) { + out.push('https://account.cloudsforge.online') + } + return out +} + +interface Booted { + corsOrigins: string[] + portalAllowedOrigins: string[] + /** Every structured line env.ts wrote to stdout while booting. */ + bootLines: { level: number; msg: string }[] +} + +/** + * Boot env.ts in a child process under `vars` and return what it produced. The environment + * is replaced, not extended: an inherited CORS_ORIGINS would otherwise decide the answer. + */ +async function boot(vars: Record): Promise { + const MARK = '__ENV_JSON__' + const source = ` + import { env } from './src/env.ts' + console.log('${MARK}' + JSON.stringify({ + corsOrigins: env.corsOrigins, + portalAllowedOrigins: env.portalAllowedOrigins, + })) + ` + const { stdout } = await execFileAsync(process.execPath, ['--import', 'tsx', '-e', source], { + cwd: pkgRoot, + env: { + // Enough to find node and give tsx somewhere to write its cache, and no more. + PATH: process.env.PATH ?? '', + HOME: process.env.HOME ?? '', + TMPDIR: process.env.TMPDIR ?? '', + NIMBUS_DATABASE_URL: 'postgres://u:p@localhost:5432/nimbus', + // env.ts refuses to boot without it (CF-16), so every child boot needs one. + NIMBUS_KEY_SECRET: 'a-test-secret-that-is-long-enough', + ...vars, + }, + }) + const line = stdout.split('\n').find((l) => l.startsWith(MARK)) + assert.ok(line, `env.ts printed no result. stdout:\n${stdout}`) + const parsed = JSON.parse(line.slice(MARK.length)) as Omit + const bootLines = stdout + .split('\n') + .filter((l) => l.startsWith('{')) + .map((l) => JSON.parse(l) as { level: number; msg: string }) + return { ...parsed, bootLines } +} + +test('a foreign apex is never added to either allowlist', async () => { + // The scenario from the register: an operator deploys on forge.example and sets + // CORS_ORIGINS by hand. PORTAL_ALLOWED_ORIGINS is left unset, so it inherits. + const configured = ['https://forge.example', 'https://admin.forge.example'] + const booted = await boot({ + NIMBUS_ISSUER: 'https://nimbus.forge.example', + CORS_ORIGINS: configured.join(','), + }) + + assert.deepEqual(booted.corsOrigins, configured) + assert.deepEqual(booted.portalAllowedOrigins, configured) + + // The property, stated the way it matters: no origin outside the operator's own apex may + // receive a sign-in code. Asserting the exact lists above would pass just as well if the + // append moved somewhere else, so say it once more in the form the harm takes. + const foreign = booted.portalAllowedOrigins.filter((o) => !o.endsWith('forge.example')) + assert.deepEqual(foreign, [], 'a domain the operator does not control can receive a sign-in code') + + // ...and the same inputs through the shipped derivation, which fails it. + assert.ok(shipped(configured).some((o) => !o.endsWith('forge.example'))) +}) + +test('nothing is appended when CORS_ORIGINS is not set either', async () => { + const booted = await boot({ NIMBUS_ISSUER: 'https://nimbus.forge.example' }) + assert.deepEqual( + booted.corsOrigins.filter((o) => !o.startsWith('http://localhost:')), + [], + 'the built-in default must be local ports only', + ) +}) + +test('a pure-localhost boot does not report public origins', async () => { + // The second, quieter half of the same defect. With the append in place, the default + // portalAllowedOrigins inherited an https:// origin on a laptop with nothing public + // configured at all — so every `pnpm dev` logged "NIMBUS_ISSUER is a localhost URL but + // public origins are allowed" at error. A boot error that is wrong on the developer path + // is one nobody reads on the deploy path. + const booted = await boot({ + NIMBUS_ISSUER: 'http://localhost:4001', + CORS_ORIGINS: 'http://localhost:3000,http://localhost:4004', + }) + const issuerErrors = booted.bootLines.filter((l) => l.msg.startsWith('NIMBUS_ISSUER')) + assert.deepEqual(issuerErrors, [], 'the localhost issuer check fired with no public origin set') + + // The check must still fire when an https origin really is configured, or the fix above + // has simply disabled it. + const real = await boot({ + NIMBUS_ISSUER: 'http://localhost:4001', + CORS_ORIGINS: 'https://account.forge.example', + }) + assert.equal(real.bootLines.filter((l) => l.msg.startsWith('NIMBUS_ISSUER')).length, 1) +}) + +/* ------------------------- the key secret is fail-closed ------------------------- */ + +/** + * Boot env.ts and report whether it survived. `boot()` above cannot be used: it parses a + * result line the process never prints when it exits at import, and a `boot()` that tolerated + * that would stop being able to tell a bad config from a working one. + */ +async function bootExpectingExit( + vars: Record, +): Promise<{ code: number; lines: { level: number; msg: string }[] }> { + const source = "import './src/env.ts'" + try { + const { stdout } = await execFileAsync(process.execPath, ['--import', 'tsx', '-e', source], { + cwd: pkgRoot, + env: { PATH: process.env.PATH ?? '', HOME: process.env.HOME ?? '', TMPDIR: process.env.TMPDIR ?? '', ...vars }, + }) + return { code: 0, lines: parseLines(stdout) } + } catch (err) { + const e = err as { code?: number; stdout?: string } + return { code: e.code ?? -1, lines: parseLines(e.stdout ?? '') } + } +} + +function parseLines(stdout: string): { level: number; msg: string }[] { + return stdout + .split('\n') + .filter((l) => l.startsWith('{')) + .map((l) => JSON.parse(l) as { level: number; msg: string }) +} + +const DB = 'postgres://u:p@localhost:5432/nimbus' + +test('nimbus refuses to boot without NIMBUS_KEY_SECRET rather than storing the key in the clear', async () => { + // CF-16. The RS256 private key was plaintext JSONB in a database every service shares one + // Postgres role on, and it is the estate's universal forging credential. A fallback here + // would be worse than none at all: the column would look protected while every deployment + // that never set the variable kept the key readable. + const booted = await bootExpectingExit({ NIMBUS_DATABASE_URL: DB }) + assert.equal(booted.code, 1, 'a missing key secret must be fatal, not a warning') + const fatal = booted.lines.filter((l) => l.level === 60 && l.msg.startsWith('NIMBUS_KEY_SECRET')) + assert.equal(fatal.length, 1, 'and it must say which variable, at fatal, in the log stream') +}) + +test('a placeholder or a short NIMBUS_KEY_SECRET is refused too', async () => { + // The same rules ForgeKeyvault applies to KEYVAULT_MASTER_SECRET, deliberately: a secret + // copied out of an example file is a secret an attacker also has. + // The third is a real-looking secret that is simply too short. Not the literal "short": + // the refusal's own message contains that word, and the assertion below would read it as + // the value having been echoed. + for (const value of ['changeme', 'dev-nimbus-key-secret', 'x7QwPz2m']) { + const booted = await bootExpectingExit({ NIMBUS_DATABASE_URL: DB, NIMBUS_KEY_SECRET: value }) + assert.equal(booted.code, 1, `"${value}" was accepted as a key secret`) + // The refusal must never quote the value back — this one is a placeholder, the next one + // an operator pastes will not be. + assert.ok(!JSON.stringify(booted.lines).includes(value)) + } +}) + +test('a real secret boots', async () => { + const booted = await bootExpectingExit({ + NIMBUS_DATABASE_URL: DB, + NIMBUS_KEY_SECRET: 'a-test-secret-that-is-long-enough', + }) + assert.equal(booted.code, 0) +}) diff --git a/services/nimbus/src/env.ts b/services/nimbus/src/env.ts index a385b8a..e2358ba 100644 --- a/services/nimbus/src/env.ts +++ b/services/nimbus/src/env.ts @@ -19,18 +19,41 @@ const splitList = (raw: string) => .map((o) => o.trim()) .filter(Boolean) +/** Trimmed, with the empty string treated as absent. */ +const optional = (raw: string | undefined): string | undefined => { + const v = raw?.trim() + return v ? v : undefined +} + // Allowed browser origins for CORS. Prod sets CORS_ORIGINS (comma-separated, -// e.g. the *.cloudsforge.online subdomains behind the Cloudflare Tunnel); -// dev falls back to the local app ports. Nimbus now also serves the shared -// CloudsForge account portal, so account.cloudsforge.online is always allowed. -const configuredCors = splitList( +// e.g. the subdomains of the deployment's apex behind the Cloudflare Tunnel); +// dev falls back to the local app ports. +// +// THIS LIST IS EXACTLY WHAT THE OPERATOR CONFIGURED — nothing is appended. It +// used to push `https://account.cloudsforge.online` unconditionally, on the +// reasoning that nimbus serves the shared account portal and should therefore +// always allow its own subdomain. Both halves of that were wrong: +// +// * The portal is served BY nimbus and fetches its own API with relative +// URLs, so it is same-origin and needs no CORS entry at all. +// * The apex is configurable. docker-compose.yml builds every origin from +// ${CLOUDSFORGE_APEX}, so on a deployment whose apex is not +// cloudsforge.online this appended a host on somebody else's domain — and +// corsOrigins is the DEFAULT for portalAllowedOrigins below, which is the +// return-URL allowlist. A foreign-apex run that did not set +// PORTAL_ALLOWED_ORIGINS would therefore accept +// /login?return=https://account.cloudsforge.online/... and hand a live +// single-use sign-in code to a domain the operator does not control. +// +// The remedy is not to derive `account.` here — nimbus reads no apex, and +// a service carrying its own copy of the origin list is precisely what +// docker-compose.yml:57-61 records as having drifted before. Compose already +// interpolates https://account.${CLOUDSFORGE_APEX} into both CORS_ORIGINS and +// PORTAL_ALLOWED_ORIGINS, so the one list stays one list. +const corsOrigins = splitList( process.env.CORS_ORIGINS ?? 'http://localhost:3000,http://localhost:3001,http://localhost:3002,http://localhost:3003,http://localhost:3004,http://localhost:4004', ) -const corsOrigins = [...configuredCors] -if (!corsOrigins.includes('https://account.cloudsforge.online')) { - corsOrigins.push('https://account.cloudsforge.online') -} // Origins the account portal may hand a sign-in code back to (return-URL // allowlist). Defaults to the CORS origins — an explicit list of real product @@ -56,9 +79,13 @@ const issuer = process.env.NIMBUS_ISSUER ?? 'http://localhost:4001' // It fails as "logged in, then bounced", nowhere near the setting that caused // it — the exact class of silence this service has already been bitten by, so // say it once, loudly, at boot rather than leaving it to be inferred. -// Read the CONFIGURED lists, not corsOrigins — account.cloudsforge.online is -// appended unconditionally above, so it is public on a pure-localhost setup too. -const publicOrigins = [...configuredCors, ...portalAllowedOrigins].filter((o) => +// Both lists, because either can be set on its own. Neither is synthesised any +// more: while corsOrigins carried an appended https://account.cloudsforge.online +// this check fired on every plain `pnpm dev` — the default portalAllowedOrigins +// inherited that origin, so a pure-localhost setup always had one "public" +// origin and always logged this error. A boot error that is wrong on the +// developer path is a boot error nobody reads on the deploy path. +const publicOrigins = [...corsOrigins, ...portalAllowedOrigins].filter((o) => o.startsWith('https://'), ) const issuerIsLocal = /^https?:\/\/(localhost|127\.0\.0\.1)(:|$)/.test(issuer) @@ -72,6 +99,89 @@ if (issuerIsLocal && publicOrigins.length > 0) { }) } +/* ------------------------------------------------------------------ * + * The origin a link that LEAVES this process is built from. + * + * Everything nimbus renders for the browser derives its URLs from the request + * (routes/portal.ts baseUrl), and that is correct there: those links are handed + * straight back to the same browser on the same host, so reflecting the host + * only ever reflects it at itself. + * + * A password-reset link is not that. It is minted for one person and read by + * another — by a mail client, hours later, out of a message the deployment's + * own relay signed and sent. Deriving THAT from `req.host` means an + * unauthenticated stranger picks where it points: `curl -H 'Host: evil.test' + * -X POST /auth/password/forgot -d '{"email":"victim@…"}'` produces a genuine, + * correctly branded CloudsForge email whose button is + * `http://evil.test/reset#token=`. The victim clicks a link that + * arrived exactly as it should have, and hands the attacker's page a single-use + * token that changes their password. `X-Forwarded-Host` is the same hole one + * hop further out: cloudflared is a private-range peer and trustProxy above + * believes it, so the header comes off the internet. + * + * So this is configuration, not a header. There used to be no such setting, on + * the reasoning that nimbus answers on two hostnames and a link minted on + * whichever one the caller used is reachable by construction — true, and beside + * the point once anything mails the link somewhere. + * + * NIMBUS_PUBLIC_URL is where the account portal lives for real users + * (https://account.). Unset, it falls back to NIMBUS_ISSUER, which is + * already the URL browsers reach nimbus on in every deployment and defaults to + * this process's own localhost port in development — so the fallback is a + * working link, never a header. + * ------------------------------------------------------------------ */ + +/** Scheme + host + port, no trailing slash, or null if `raw` is not that. */ +function originOf(raw: string): string | null { + let url: URL + try { + url = new URL(raw) + } catch { + return null + } + if (url.protocol !== 'http:' && url.protocol !== 'https:') return null + return url.origin +} + +function readPublicUrl(): string { + const issuerOrigin = originOf(issuer) + // NIMBUS_ISSUER is validated nowhere else and is a plain string in a JWT + // claim, so it can be something that is not a URL at all. Fall back to the + // port this process actually listens on rather than to nothing. + const fallback = issuerOrigin ?? `http://localhost:${process.env.NIMBUS_PORT ?? 4001}` + + const raw = optional(process.env.NIMBUS_PUBLIC_URL) + if (!raw) return fallback + + const origin = originOf(raw) + if (!origin) { + // Fail back to the fallback, never to the request. A reset link built from + // a header is the defect this setting exists to close, so a malformed value + // must not reopen it — it costs a wrong-looking (but working) link and a + // boot error that says exactly which variable is wrong. + bootLog('error', 'NIMBUS_PUBLIC_URL is not an http(s) URL', { + value: raw, + using: fallback, + fix: 'Set it to the origin users reach the account portal on, scheme included and nothing after the host — e.g. https://account.. Unset it to use NIMBUS_ISSUER.', + }) + return fallback + } + return origin +} + +const publicUrl = readPublicUrl() + +/** + * The origins nimbus expects to be ADDRESSED on. Not an allowlist — nothing is + * authorised by it and no link is built from it. It exists so that a request + * arriving on some other host can be told apart from the two legitimate ones + * and logged, which is the only warning anybody gets that a Host-header probe + * is happening or that NIMBUS_PUBLIC_URL is set to the wrong thing. + */ +const publicUrlAliases = [publicUrl, originOf(issuer)].filter( + (o, i, all): o is string => typeof o === 'string' && all.indexOf(o) === i, +) + // Nimbus cannot do one useful thing without a database, and postgres() treats an // undefined connection string as "use the libpq defaults" — so an absent value // did not fail, it quietly dialled localhost:5432 as whatever user the process @@ -99,6 +209,66 @@ function requireDatabaseUrl(): string { return raw } +/** + * The secret the RS256 signing key is encrypted at rest under (CF-16). + * + * REQUIRED, AND FAIL-CLOSED, for the same reason KEYVAULT_MASTER_SECRET is: a + * default here would be worse than no encryption at all, because everyone would + * believe the column was protected while the key that unlocks it is in the + * published source. The private JWK is the estate's universal forging + * credential — every service checks `iss` plus `aud: 'cloudsforge'` and nothing + * more — and it is the thing forge-keyvault's admin surface trusts. + * + * The rules are keyvault's rules, deliberately: present, not a known + * placeholder, at least 24 characters. This is the one variable nimbus will not + * start without besides the database, and the refusal is loud because a silent + * downgrade to plaintext is exactly the failure this closes. + * + * WHAT CHANGING IT COSTS. The stored key becomes undecryptable, which is + * survivable and not silent: nimbus refuses to serve rather than signing with + * something else. The recovery is to insert a new signing key — see + * `POST /admin/signing-keys` — or, on a deployment where every session may be + * dropped, to delete the rows and let first-boot generation run again. Access + * tokens are 15 minutes and refresh tokens are opaque DB rows rather than + * signed, so the cost of losing the key is a 15-minute window of 401s that + * clients recover from through /auth/refresh — not an estate-wide logout. + */ +const REJECTED_SECRETS = new Set([ + 'changeme', + 'change-me', + 'dev-master-secret-change-me', + 'dev-nimbus-key-secret', + 'nimbus-key-secret', + 'secret', +]) + +function requireKeySecret(): string { + const raw = optional(process.env.NIMBUS_KEY_SECRET) + const generate = 'Generate one with `openssl rand -hex 32` and set NIMBUS_KEY_SECRET in .env. Keep it OUT of the database and out of any backup of it — that separation is the whole point.' + if (!raw) { + bootLog('fatal', 'NIMBUS_KEY_SECRET is not set', { + fix: `The RS256 signing key is stored encrypted under it, and nimbus will not fall back to plaintext. ${generate}`, + }) + process.exit(1) + } + if (REJECTED_SECRETS.has(raw)) { + bootLog('fatal', 'NIMBUS_KEY_SECRET is set to a known placeholder', { + fix: generate, + }) + process.exit(1) + } + if (raw.length < 24) { + bootLog('fatal', 'NIMBUS_KEY_SECRET is too short', { + // The length, never the value. + length: raw.length, + minimum: 24, + fix: generate, + }) + process.exit(1) + } + return raw +} + // Where ForgeKeyvault answers. Nimbus proxies the admin console's vault reads // through itself because keyvault is deliberately never routed publicly (it // holds custody keys and binds to loopback), so the browser cannot reach it. @@ -126,15 +296,19 @@ if (keyvaultIsLocal && !issuerIsLocal) { /** * Where Forge Pay answers, from THIS process. * - * Nimbus proxies the admin console's Forge Pay reads and the one administered-price - * write, for a different reason than the keyvault proxy above. Pay IS publicly routed - * (deploy/cloudflared/config.example.yml routes pay., with /internal refused at the - * edge), and admin. is on pay's CORS allowlist — so the console can and does read - * from it directly in principle. What it cannot do is WRITE: pay registers @fastify/cors - * without a `methods` option, and the preflight it actually returns is - * `access-control-allow-methods: GET,HEAD,POST`. `PUT /admin/prices/:coin` is the only way - * to set the EMBER price, and no browser will send it. The proxy is how that write happens - * at all; the reads come with it so the panel has one origin and one failure mode. + * Nimbus proxies the admin console's Forge Pay reads, the one administered-price write + * and the withdrawal abandon, for a different reason than the keyvault proxy above. + * Keyvault is unroutable and the console physically cannot reach it. Pay IS publicly + * routed (deploy/cloudflared/config.example.yml routes pay., with /internal refused + * at the edge) and admin. is on pay's CORS allowlist, so the console could call + * every one of those routes direct. It goes through here for the panel's sake rather than + * the network's: one origin, one auth path and one failure mode for a screen whose every + * other call is to Nimbus on the same bearer. + * + * This note used to say the console could not send the price PUT, because pay's preflight + * named GET, HEAD and POST only. That was true until forge-pay f3eb408 (2026-07-28) stated + * pay's method list explicitly and included PUT; the argument that survived it is written + * out at the top of routes/pay.ts (CF-46). * * NOTE THE DEFAULT, which differs from keyvaultUrl's above ON PURPOSE. docker-compose.yml * gives the nimbus service KEYVAULT_URL explicitly, so that one can default to localhost @@ -149,6 +323,101 @@ if (keyvaultIsLocal && !issuerIsLocal) { */ const payUrl = process.env.PAY_API_URL ?? 'http://pay:4003' +/* ------------------------------------------------------------------ * + * Outbound mail (SMTP). + * + * One env-driven transport and no provider code. Brevo's free relay, a Gmail + * app password, Resend, SendGrid, Mailtrap and a self-hosted postfix all speak + * the same four settings, so switching provider is an .env edit and a restart — + * never a code change, never a second secret, never an SDK that has to be kept + * current. The only thing nimbus sends is a password-reset link. + * + * UNSET IS A SUPPORTED MODE, not a degraded one. With no SMTP_HOST the reset + * flow behaves exactly as it did before mail existed: the token is minted and + * recorded, and an operator issues the link from the console. That is announced + * at info, because an operator who has deliberately not configured mail must not + * be handed a warning every boot — the level has to keep meaning something for + * the checks above it. + * ------------------------------------------------------------------ */ + +export interface SmtpConfig { + host: string + port: number + /** true = implicit TLS (465). false = plain connect then STARTTLS (587). */ + secure: boolean + user: string + pass: string + /** RFC 5322 From, e.g. `CloudsForge `. */ + from: string + replyTo?: string +} + +function readSmtp(): SmtpConfig | null { + const host = optional(process.env.SMTP_HOST) + if (!host) { + bootLog('info', 'SMTP is not configured — password reset links are issued by an operator', { + fix: 'Optional. To send reset emails set SMTP_HOST, SMTP_USER, SMTP_PASS and SMTP_FROM (see .env.example, "Outbound mail"). Until then a self-service reset is recorded and an admin hands the link over from the console: Users → Reset password.', + }) + return null + } + + const user = optional(process.env.SMTP_USER) + const pass = optional(process.env.SMTP_PASS) + const from = optional(process.env.SMTP_FROM) + + // Half-configured is the dangerous state, and it is the likely one: an + // operator pastes the host out of a provider's quickstart and comes back for + // the credentials later. nodemailer would then connect and be refused with a + // 5xx per send — inside deliverPasswordReset, which swallows everything so the + // route cannot become an enumeration oracle. So the only place this can be + // said out loud is here, at boot, before it is silent forever. + const missing = [ + !user && 'SMTP_USER', + !pass && 'SMTP_PASS', + !from && 'SMTP_FROM', + ].filter(Boolean) as string[] + if (missing.length > 0) { + bootLog('error', 'SMTP_HOST is set but the rest of the SMTP settings are not', { + host, + missing, + fix: `Set ${missing.join(', ')} as well, or unset SMTP_HOST to go back to operator-issued reset links. Credentials come from your provider (Brevo: SMTP & API → SMTP keys). SMTP_FROM must be a full address, e.g. "CloudsForge ".`, + }) + return null + } + + // 587 (STARTTLS) is the default because it is what every relay in the list + // above documents first. 465 is implicit TLS and needs SMTP_SECURE=true: the + // two are not interchangeable, and a 465 connection with secure=false hangs + // until the connection timeout rather than failing with anything readable. + const rawPort = optional(process.env.SMTP_PORT) + const port = rawPort ? Number(rawPort) : 587 + if (!Number.isInteger(port) || port < 1 || port > 65535) { + bootLog('error', 'SMTP_PORT is not a port number', { + value: rawPort, + fix: 'Use 587 for STARTTLS (the usual choice) or 465 with SMTP_SECURE=true for implicit TLS. Leave it unset for 587.', + }) + return null + } + const secure = optional(process.env.SMTP_SECURE) === 'true' + if (port === 465 && !secure) { + bootLog('warn', 'SMTP_PORT is 465 but SMTP_SECURE is not true', { + fix: 'Port 465 is implicit TLS: set SMTP_SECURE=true, or use port 587, which negotiates STARTTLS after connecting.', + }) + } + + bootLog('info', 'SMTP is configured — password reset links are emailed', { + host, + port, + secure, + // The address, never the credential. `user` is frequently an account id or + // an email and is not a secret; `pass` never appears in any log line here. + from, + }) + return { host, port, secure, user: user!, pass: pass!, from: from!, replyTo: optional(process.env.SMTP_REPLY_TO) } +} + +const smtp = readSmtp() + // Reveal returns a plaintext custody private key. Proxying it would make it // reachable from the public internet on an admin token alone, where today it // additionally requires network access to a loopback-bound service — a second @@ -161,10 +430,18 @@ export const env = { trustProxy, portalAllowedOrigins, issuer, + /** Origin every link that leaves this process is built from. Never a header. */ + publicUrl, + /** The origins nimbus expects to be addressed on. For logging only. */ + publicUrlAliases, databaseUrl: requireDatabaseUrl(), + /** Encrypts `signing_keys.private_jwk_enc`. Never logged, never returned. */ + keySecret: requireKeySecret(), keyvaultUrl, payUrl, vaultRevealProxy, + /** null when outbound mail is not configured — a supported deployment. */ + smtp, adminEmail: process.env.ADMIN_EMAIL, adminPassword: process.env.ADMIN_PASSWORD, adminHandle: process.env.ADMIN_HANDLE, diff --git a/services/nimbus/src/forgotTiming.test.ts b/services/nimbus/src/forgotTiming.test.ts new file mode 100644 index 0000000..424f0d1 --- /dev/null +++ b/services/nimbus/src/forgotTiming.test.ts @@ -0,0 +1,156 @@ +import assert from 'node:assert/strict' +import { randomUUID } from 'node:crypto' +import { createServer, type Server, type Socket, type AddressInfo } from 'node:net' +import { after, test } from 'node:test' +import Fastify from 'fastify' +import type { FastifyBaseLogger } from 'fastify' + +/** + * POST /auth/password/forgot answers in the same time for an address that exists and one + * that does not. + * + * THE DEFECT. Wiring SMTP delivery into this route made it `await` the send before replying. + * The status code and the body were written with great care to be identical for both cases — + * the route's own comment calls a differing "status, body or TIMING" an account-enumeration + * oracle — and then the clock said everything they refused to. Measured against a relay that + * accepts the connection and never speaks: an unknown address answered in 10ms and a known + * one in 6015ms, the whole attempt budget. Even a healthy local sink split 4ms from 33ms, and + * a real relay's TLS handshake is wider than that. Any relay outage silently converted this + * endpoint into a reliable "does this person have an account" API, answerable one request at + * a time by anyone. + * + * WHAT IS ASSERTED. That the reply does not wait for the relay: the send is still in flight — + * this file's fake relay has the connection open and is deliberately saying nothing — when + * both answers have already been written, and the two answers are within a few milliseconds + * of each other rather than a few seconds. + * + * WHY A HUNG RELAY AND NOT A REFUSED PORT. A refused connection fails in under a millisecond, + * so a route that awaited it would still look fast here and the case would pass against the + * bug. The failure mode that matters is the slow one, so the relay accepts and stalls. Put + * the route back the way it shipped — mint, send, THEN reply — and this case reports the + * known address at 6078ms against the unknown one's 5ms, and fails on both assertions. + * + * HOW TO RUN IT. The same scratch database as tokens.test.ts, for the same reason: the branch + * under test is chosen by a real SELECT against `users`. + */ + +function testDatabase(): { url: string } | { skip: string } { + const raw = process.env.NIMBUS_TEST_DATABASE_URL?.trim() + if (!raw) { + return { skip: 'NIMBUS_TEST_DATABASE_URL is not set — see the comment at the top of this file' } + } + let name: string + try { + name = new URL(raw).pathname.replace(/^\//, '') + } catch { + return { skip: 'NIMBUS_TEST_DATABASE_URL is not a URL' } + } + if (!/test/i.test(name)) { + return { skip: `refusing to run against database "${name}": its name must contain "test"` } + } + return { url: raw } +} + +const target = testDatabase() + +/* ------------------------- the relay that never answers ------------------------- */ + +/** Connections it has accepted. Nothing is ever written back on any of them. */ +let connections = 0 +const sockets: Socket[] = [] +const relay: Server = createServer((socket) => { + connections += 1 + sockets.push(socket) + socket.on('error', () => {}) +}) +await new Promise((resolve) => relay.listen(0, '127.0.0.1', resolve)) +const relayPort = (relay.address() as AddressInfo).port + +/* env.ts is evaluated once, at import, off process.env — so the SMTP settings have to be in + * place before anything below is loaded, exactly as pay.test.ts does with PAY_API_URL. */ +let client!: typeof import('./db/client.js') +let authRoutes!: (typeof import('./routes/auth.js'))['authRoutes'] + +if ('url' in target) { + process.env.NIMBUS_DATABASE_URL = target.url + process.env.NIMBUS_KEY_SECRET ??= 'test-secret-that-is-long-enough-32b' + process.env.NIMBUS_PUBLIC_URL = 'https://account.forge.example' + process.env.NIMBUS_ISSUER = 'https://nimbus.forge.example' + process.env.SMTP_HOST = '127.0.0.1' + process.env.SMTP_PORT = String(relayPort) + process.env.SMTP_USER = 'u' + process.env.SMTP_PASS = 'p' + process.env.SMTP_FROM = 'CloudsForge ' + client = await import('./db/client.js') + authRoutes = (await import('./routes/auth.js')).authRoutes + const { migrate } = await import('./db/migrate.js') + await migrate({ info() {}, error() {} } as unknown as FastifyBaseLogger) +} + +const skip = 'skip' in target ? target.skip : false + +after(async () => { + // Cut the stalled send short rather than leaving the runner waiting out the mailer's + // six-second attempt budget after the last assertion has passed. The listener goes first: + // a destroyed socket is a connection reset, which the mailer retries once, and the retry + // has to find nothing listening or it stalls for another six seconds. + relay.close() + for (const socket of sockets) socket.destroy() + if ('url' in target) { + await client.sql`DELETE FROM users WHERE email LIKE 'timing-%@example.invalid'` + await client.sql.end() + } +}) + +test('the reply does not wait for the relay, so the clock says nothing about the account', { skip }, async () => { + const id = randomUUID() + const known = `timing-${id}@example.invalid` + await client.sql` + INSERT INTO users (id, email, handle, password_hash) + VALUES (${id}, ${known}, ${`timing-${id.slice(0, 8)}`}, 'not-a-hash') + ` + + const app = Fastify({ logger: false }) + await app.register(authRoutes) + await app.ready() + + const ask = async (email: string): Promise<{ status: number; ms: number; body: string }> => { + const started = performance.now() + const res = await app.inject({ + method: 'POST', + url: '/auth/password/forgot', + payload: { email }, + }) + return { status: res.statusCode, ms: performance.now() - started, body: res.body } + } + + const missing = await ask(`timing-${randomUUID()}@example.invalid`) + const present = await ask(known) + await app.close() + + // The two answers a caller can see, unchanged and still identical. + assert.equal(missing.status, 202) + assert.equal(present.status, 202) + assert.equal(missing.body, present.body) + + // The one it could read off a stopwatch. The mailer's budget is 6s and the pre-fix known + // branch spent all of it; half a second is far below anything a relay can cost and far + // above the jitter of two injected requests. + assert.ok( + present.ms < 500, + `the known-address branch took ${present.ms.toFixed(0)}ms — it is waiting for the relay`, + ) + assert.ok( + Math.abs(present.ms - missing.ms) < 500, + `known ${present.ms.toFixed(0)}ms vs unknown ${missing.ms.toFixed(0)}ms — that gap is the oracle`, + ) + + // …and the assertion above only means something if a send was really attempted and really + // hung. The relay has the connection open and has said nothing on it. + const deadline = Date.now() + 3_000 + while (connections === 0 && Date.now() < deadline) await new Promise((r) => setTimeout(r, 25)) + assert.ok( + connections > 0, + 'no SMTP connection was ever made, so this case did not measure a detached send', + ) +}) diff --git a/services/nimbus/src/index.ts b/services/nimbus/src/index.ts index 8578e11..f4f4246 100644 --- a/services/nimbus/src/index.ts +++ b/services/nimbus/src/index.ts @@ -14,7 +14,7 @@ import { db } from './db/client.js' import { users } from './db/schema.js' import { migrate } from './db/migrate.js' import { hashPassword } from './passwords.js' -import { getSigningKey } from './keys.js' +import { encryptStoredSigningKeys, getSigningKey } from './keys.js' import { authRoutes } from './routes/auth.js' import { portalRoutes } from './routes/portal.js' import { adminRoutes } from './routes/admin.js' @@ -75,6 +75,10 @@ async function main(): Promise { installProcessHandlers(app) await step(app.log, 'migrate', () => migrate(app.log)) + // Empty the plaintext private_jwk column before anything can read it (CF-16). + // Before the signing-key step below, so the very first key this process loads + // is loaded through the envelope. + await step(app.log, 'encrypt-signing-keys', () => encryptStoredSigningKeys(app.log)) // Ensure the signing key exists / is cached before serving. await step(app.log, 'signing-key', () => getSigningKey()) await step(app.log, 'seed-admin', () => seedAdmin(app.log)) @@ -136,9 +140,11 @@ async function main(): Promise { // Admin-gated read proxy in front of ForgeKeyvault, which is never routed // publicly. Separately encapsulated for the same reason adminRoutes is. await app.register(vaultProxyRoutes) - // Admin-gated proxy in front of Forge Pay's operator surface. Pay IS routed, but - // its CORS preflight allows GET/HEAD/POST only, so the browser cannot PUT the - // administered EMBER price. Encapsulated like the two above. + // Admin-gated proxy in front of Forge Pay's operator surface. Unlike keyvault, pay + // IS routed and the console could call it direct; these routes come through here so + // the price panel has one origin and one auth path (see routes/pay.ts, which states + // the whole case now that pay's preflight allows the PUT). Encapsulated like the two + // above. await app.register(payProxyRoutes) await step(app.log, 'listen', () => app.listen({ host: '0.0.0.0', port: env.port })) diff --git a/services/nimbus/src/keyEnvelope.ts b/services/nimbus/src/keyEnvelope.ts new file mode 100644 index 0000000..30207fc --- /dev/null +++ b/services/nimbus/src/keyEnvelope.ts @@ -0,0 +1,83 @@ +import { createCipheriv, createDecipheriv, randomBytes, scryptSync } from 'node:crypto' +import { env } from './env.js' + +/* ------------------------------------------------------------------ * + * The envelope the RS256 signing key is stored in. + * + * WHY THERE IS ONE (CF-16). `signing_keys.private_jwk` held the private key as + * plaintext JSONB. Every service in the estate connects to the same Postgres as + * the same role, and that key is the estate's universal forging credential: + * every service verifies `iss` plus `aud: 'cloudsforge'` and nothing else, so a + * self-minted `{sub, roles:['admin']}` is accepted by pay, game, crucible, + * forge-mint and forge-keyvault alike. Contrast forge-keyvault, which encrypts + * the custody keys nimbus's token unlocks and refuses to boot on a placeholder + * secret — the key above it had no equivalent. + * + * WHAT IT ACTUALLY BUYS, stated honestly, because the answer is narrower than + * "the JWK is now safe". An attacker who can run code in a container has .env, + * and .env has this secret; an attacker with write-capable SQL can insert their + * own admin `users` row and needs no key at all. What encryption at rest closes + * is the READ-ONLY vector: a stolen pg backup, a SELECT-only injection, a copied + * dump. Those turn a database read into silent, unrevocable, offline-usable + * impersonation across every product, and they are the vectors an env-held + * secret genuinely defeats, because the secret is not in the artifact. + * + * WHY IT IS THE SAME SHAPE AS KEYVAULT'S (`forge-keyvault/src/crypto.ts`): + * scrypt from an env secret, AES-256-GCM, `v:` + base64(iv||tag||ct). Two + * envelope formats in one estate is one format nobody can read in an incident. + * The version prefix is what makes NIMBUS_KEY_SECRET rotatable later: a v2 + * derives differently while every v1 blob keeps decrypting under its own rule. + * ------------------------------------------------------------------ */ + +const CURRENT_VERSION = 1 + +/** `v:`. ':' is not in the base64 alphabet, so this cannot be ambiguous. */ +const VERSIONED = /^v([0-9]{1,3}):([A-Za-z0-9+/]+={0,2})$/ + +/** + * Per-key data key: scrypt(NIMBUS_KEY_SECRET, salt) -> 32 bytes. + * + * The kid is in the salt so one derived key never unlocks another signing key — + * which matters the moment there is more than one, i.e. the moment rotation + * exists. The version is in the salt too, so a derived key does not survive a + * rotation of the secret itself. + */ +function deriveKey(version: number, kid: string): Buffer { + return scryptSync(env.keySecret, `nimbus:v${version}:${kid}`, 32) +} + +/** AES-256-GCM the private JWK of `kid`. Returns `v:base64(iv||tag||ct)`. */ +export function encryptPrivateJwk(kid: string, jwk: Record): string { + const key = deriveKey(CURRENT_VERSION, kid) + const iv = randomBytes(12) + const cipher = createCipheriv('aes-256-gcm', key, iv) + const ct = Buffer.concat([cipher.update(JSON.stringify(jwk), 'utf8'), cipher.final()]) + const tag = cipher.getAuthTag() + return `v${CURRENT_VERSION}:${Buffer.concat([iv, tag, ct]).toString('base64')}` +} + +export function decryptPrivateJwk(kid: string, blob: string): Record { + const match = VERSIONED.exec(blob.trim()) + if (!match) { + // Not a shape this ever wrote. Say that rather than letting it fail as a GCM + // authentication error, which reads like the wrong secret and sends an + // operator rotating something that was never the problem. + throw new Error(`signing key ${kid} is not in a recognised envelope format`) + } + const version = Number(match[1]) + if (version > CURRENT_VERSION) { + throw new Error( + `signing key ${kid} is envelope version ${version}; this build reads up to v${CURRENT_VERSION}`, + ) + } + const buf = Buffer.from(match[2]!, 'base64') + const iv = buf.subarray(0, 12) + const tag = buf.subarray(12, 28) + const ct = buf.subarray(28) + const decipher = createDecipheriv('aes-256-gcm', deriveKey(version, kid), iv) + decipher.setAuthTag(tag) + return JSON.parse(Buffer.concat([decipher.update(ct), decipher.final()]).toString('utf8')) as Record< + string, + unknown + > +} diff --git a/services/nimbus/src/keys.test.ts b/services/nimbus/src/keys.test.ts new file mode 100644 index 0000000..c5a14d3 --- /dev/null +++ b/services/nimbus/src/keys.test.ts @@ -0,0 +1,271 @@ +import assert from 'node:assert/strict' +import { after, test } from 'node:test' +import type { FastifyBaseLogger } from 'fastify' + +/** + * The signing key: encrypted at rest, and replaceable without a flag day (CF-16). + * + * THE DEFECT. `signing_keys.private_jwk` was the RS256 private key as plaintext JSONB. Every + * service in the estate connects to the same Postgres as the same role, and that key is the + * estate's universal forging credential — every service verifies `iss` plus + * `aud: 'cloudsforge'` and nothing else, so a self-minted `{sub, roles:['admin']}` is admin in + * forge-pay, the game, Crucible, ForgeMint and ForgeKeyvault alike. A stolen backup or a + * SELECT-only injection was therefore silent, offline, unrevocable impersonation everywhere. + * There was also no way to replace the key: `getJwks()` published exactly one, and + * `getSigningKey()` cached for the process lifetime, so a swap meant hand-editing a row, a + * restart, and a window in which every live token was rejected. + * + * WHY IT NEEDS A REAL DATABASE. Both halves are properties of what is IN the table — that the + * plaintext column ends up NULL, that exactly one row is active, that the JWKS is what a + * verifier would fetch — and the migration under test is DDL. A stub would assert that this + * file's own idea of a row matches itself. + * + * HOW TO RUN IT. Point NIMBUS_TEST_DATABASE_URL at a scratch database, as tokens.test.ts + * documents at length. Every case skips without one; CI provides one. + * + * WHY THE SUITE RUNS ONE FILE AT A TIME (`--test-concurrency=1`, package.json). The cases + * below empty `signing_keys` and then plant rows that are deliberately not usable keys — the + * plaintext-migration case writes `{kty:'RSA', d:…, n:…}`, which has no `e` and cannot be + * imported. Every other suite in this service signs an access token, and node --test runs + * FILES in parallel by default: pay.test.ts would intermittently pick up this file's fixture + * as the active key and die inside jose with "key.e must be of type string". One database, one + * `signing_keys` table, and no way for a suite to own a row it did not create — so the files + * take turns instead. Cases within a file already run in order. + */ + +/** The scratch database, or the reason there is not one. */ +function testDatabase(): { url: string } | { skip: string } { + const raw = process.env.NIMBUS_TEST_DATABASE_URL?.trim() + if (!raw) { + return { skip: 'NIMBUS_TEST_DATABASE_URL is not set — see the comment at the top of this file' } + } + let name: string + try { + name = new URL(raw).pathname.replace(/^\//, '') + } catch { + return { skip: 'NIMBUS_TEST_DATABASE_URL is not a URL' } + } + if (!/test/i.test(name)) { + return { skip: `refusing to run against database "${name}": its name must contain "test"` } + } + return { url: raw } +} + +const target = testDatabase() + +let keys!: typeof import('./keys.js') +let envelope!: typeof import('./keyEnvelope.js') +let client!: typeof import('./db/client.js') +let tokens!: typeof import('./tokens.js') +let adminRoutes!: typeof import('./routes/admin.js')['adminRoutes'] +let issuer!: string + +const quiet = { info() {}, warn() {}, error() {} } as unknown as FastifyBaseLogger + +if ('url' in target) { + process.env.NIMBUS_DATABASE_URL = target.url + // env.ts refuses to boot without one, which is itself asserted below in a child process. + process.env.NIMBUS_KEY_SECRET ??= 'test-secret-that-is-long-enough-32b' + client = await import('./db/client.js') + envelope = await import('./keyEnvelope.js') + keys = await import('./keys.js') + tokens = await import('./tokens.js') + adminRoutes = (await import('./routes/admin.js')).adminRoutes + issuer = (await import('./env.js')).env.issuer + const { migrate } = await import('./db/migrate.js') + await migrate(quiet) +} + +const skip = 'skip' in target ? target.skip : false + +/** Every case starts from an empty table; the module cache does not, so clear both. */ +async function reset(): Promise { + await client.sql`DELETE FROM signing_keys` + // getSigningKey() memoises the active key for thirty seconds and these cases run in + // milliseconds, so without this every case after the first would sign with a row it had + // just deleted. It is the same call activateSigningKey makes for the same reason. + keys.forgetActiveSigningKey() +} + +after(async () => { + if ('url' in target) { + await client.sql`DELETE FROM signing_keys` + await client.sql.end() + } +}) + +/* ---------------------------- encryption at rest ---------------------------- */ + +test('a stored signing key is not readable from the database', { skip }, async () => { + await reset() + const key = await keys.getSigningKey() + + const rows = await client.sql<{ private_jwk: unknown; private_jwk_enc: string | null }[]>` + SELECT private_jwk, private_jwk_enc FROM signing_keys WHERE kid = ${key.kid} + ` + const row = rows[0]! + // THE ASSERTION THE TICKET IS ABOUT. Everything a reader of this database gets. + assert.equal(row.private_jwk, null, 'the plaintext column must never be written again') + assert.ok(row.private_jwk_enc, 'the private half must be stored, encrypted') + // An RSA private JWK is recognisable by its private exponent. If any of `d`, `p` or `q` + // is legible in the stored blob, the envelope is not doing anything. + const blob = row.private_jwk_enc! + assert.match(blob, /^v1:[A-Za-z0-9+/]+={0,2}$/, 'the envelope must be versioned base64') + for (const field of ['"d"', '"p"', '"q"', '"dp"', '"qi"']) { + assert.ok(!blob.includes(field), `the blob leaks ${field}`) + } +}) + +test('a key stored in plaintext by an older build is encrypted at boot', { skip }, async () => { + await reset() + const key = await keys.getSigningKey() + + // Exactly the row the pre-fix build left: the private JWK legible in the JSONB column. + const legible = { kty: 'RSA', d: 'this-is-the-private-exponent', n: 'modulus' } + // Serialised with an explicit cast rather than a bound object: postgres-js has no type for + // an untyped parameter here and hands the object straight to Buffer.byteLength. + await client.sql` + UPDATE signing_keys + SET private_jwk = ${JSON.stringify(legible)}::jsonb, private_jwk_enc = NULL + WHERE kid = ${key.kid} + ` + const before = await client.sql<{ n: number }[]>` + SELECT count(*)::int AS n FROM signing_keys WHERE private_jwk IS NOT NULL + ` + assert.equal(before[0]!.n, 1, 'the pre-fix state is one readable key') + + assert.equal(await keys.encryptStoredSigningKeys(quiet), 1) + + const after = await client.sql<{ n: number }[]>` + SELECT count(*)::int AS n FROM signing_keys WHERE private_jwk IS NOT NULL + ` + assert.equal(after[0]!.n, 0, 'the plaintext column must be empty after a boot') + // …and the same call again finds nothing to do, because a boot runs it every time. + assert.equal(await keys.encryptStoredSigningKeys(quiet), 0) + + const rows = await client.sql<{ private_jwk_enc: string }[]>` + SELECT private_jwk_enc FROM signing_keys WHERE kid = ${key.kid} + ` + assert.deepEqual(envelope.decryptPrivateJwk(key.kid, rows[0]!.private_jwk_enc), legible) +}) + +test('the envelope is bound to the kid, so one blob does not open another key', { skip }, async () => { + const jwk = { kty: 'RSA', d: 'secret' } + const blob = envelope.encryptPrivateJwk('aaaaaaaaaaaaaaaa', jwk) + assert.deepEqual(envelope.decryptPrivateJwk('aaaaaaaaaaaaaaaa', blob), jwk) + // The kid is in the scrypt salt. Reading it under a different kid is a GCM failure, not a + // different plaintext — which is what makes it safe to have more than one key at a time. + assert.throws(() => envelope.decryptPrivateJwk('bbbbbbbbbbbbbbbb', blob)) +}) + +test('a blob that is not an envelope is refused by name, not by GCM', { skip }, async () => { + // The failure mode this avoids: a value that was never encrypted (a hand-edited row, a + // restored dump from before this build) fails as "unable to authenticate data", which + // reads like the wrong secret and sends an operator rotating something that was fine. + assert.throws( + () => envelope.decryptPrivateJwk('aaaaaaaaaaaaaaaa', '{"kty":"RSA"}'), + /not in a recognised envelope format/, + ) +}) + +/* -------------------------------- rotation -------------------------------- */ + +/** Backdate a key's status change, i.e. wait out the publish window without waiting. */ +async function agePublication(kid: string, byMs: number): Promise { + const at = new Date(Date.now() - byMs).toISOString() + await client.sql` + UPDATE signing_keys SET status_changed_at = ${at}::timestamptz WHERE kid = ${kid} + ` +} + +test('a new key is published before it signs, and refuses to be rushed', { skip }, async () => { + await reset() + const original = await keys.getSigningKey() + + const minted = await keys.mintSigningKey() + assert.equal(minted.status, 'published') + + // STEP ONE'S WHOLE POINT: it is in the document immediately, so every verifier can fetch + // it, and it has signed nothing. + const jwks = await keys.getJwks() + assert.deepEqual( + jwks.keys.map((k) => k.kid), + [original.kid, minted.kid], + 'the active key first, then the newcomer — a fixed order, so the document is comparable', + ) + assert.equal((await keys.getSigningKey()).kid, original.kid, 'it must not sign yet') + + // STEP TWO IS ENFORCED, not documented. Activating now would mint tokens under a kid no + // consumer has fetched, and every service in the estate 401s until its cache turns over. + const early = await keys.activateSigningKey(minted.kid) + assert.equal(early.status, 'too_soon') + + await agePublication(minted.kid, keys.PUBLISH_BEFORE_ACTIVE_MS + 1_000) + const activated = await keys.activateSigningKey(minted.kid) + assert.equal(activated.status, 'ok') + + // STEP THREE: the new key signs, and — the part that makes this not a flag day — the old + // one is still in the JWKS, so every token it minted in the last fifteen minutes still + // verifies. + assert.equal((await keys.getSigningKey()).kid, minted.kid) + const after = await keys.getJwks() + assert.deepEqual( + after.keys.map((k) => k.kid), + [minted.kid, original.kid], + ) + + // And the pre-fix behaviour, for contrast: publishing only the signing key meant that at + // this exact moment every outstanding token was unverifiable. + assert.equal(after.keys.length, 2, 'the shipped getJwks() returned one key here') +}) + +test('a retired key leaves the JWKS, and the active one cannot be retired', { skip }, async () => { + await reset() + const original = await keys.getSigningKey() + const minted = await keys.mintSigningKey() + await agePublication(minted.kid, keys.PUBLISH_BEFORE_ACTIVE_MS + 1_000) + assert.equal((await keys.activateSigningKey(minted.kid)).status, 'ok') + + // Nothing could sign afterwards, so this must be refused rather than leaving the estate + // to discover it one login later. + assert.equal((await keys.retireSigningKey(minted.kid)).status, 'is_active') + + const retired = await keys.retireSigningKey(original.kid) + assert.equal(retired.status, 'ok') + const jwks = await keys.getJwks() + assert.deepEqual( + jwks.keys.map((k) => k.kid), + [minted.kid], + ) + // The row survives retirement: an old token in a log is still attributable to the key + // that made it. + const listed = await keys.listSigningKeys() + assert.equal(listed.find((k) => k.kid === original.kid)?.status, 'retired') +}) + +test('activate refuses a key it has never published, and one that does not exist', { skip }, async () => { + await reset() + const original = await keys.getSigningKey() + const minted = await keys.mintSigningKey() + await keys.retireSigningKey(minted.kid) + + assert.equal((await keys.activateSigningKey(minted.kid)).status, 'not_published') + assert.equal((await keys.activateSigningKey(original.kid)).status, 'is_active') + assert.equal((await keys.activateSigningKey('ffffffffffffffff')).status, 'not_found') +}) + +test('nothing the management surface returns contains key material', { skip }, async () => { + await reset() + await keys.getSigningKey() + await keys.mintSigningKey() + + const listed = JSON.stringify(await keys.listSigningKeys()) + for (const field of ['"d"', '"p"', '"q"', 'private_jwk', 'privateJwk']) { + assert.ok(!listed.includes(field), `listSigningKeys() leaks ${field}`) + } + // The JWKS is public by design and must carry the PUBLIC halves and nothing more. + const jwks = JSON.stringify(await keys.getJwks()) + for (const field of ['"d"', '"p"', '"q"', '"dp"', '"qi"']) { + assert.ok(!jwks.includes(field), `the JWKS leaks ${field}`) + } +}) diff --git a/services/nimbus/src/keys.ts b/services/nimbus/src/keys.ts index 2c1ca27..ae834da 100644 --- a/services/nimbus/src/keys.ts +++ b/services/nimbus/src/keys.ts @@ -1,7 +1,45 @@ import { randomBytes } from 'node:crypto' +import { and, asc, eq, isNotNull, ne } from 'drizzle-orm' +import type { FastifyBaseLogger } from 'fastify' import { exportJWK, generateKeyPair, importJWK, type JWK, type KeyLike } from 'jose' import { db } from './db/client.js' -import { signingKeys } from './db/schema.js' +import { signingKeys, type SigningKeyStatus } from './db/schema.js' +import { decryptPrivateJwk, encryptPrivateJwk } from './keyEnvelope.js' + +/* ------------------------------------------------------------------ * + * The RS256 signing key: where it is kept, which one signs, and how one is + * replaced without signing anybody out (CF-16). + * + * AT REST it is AES-256-GCM under NIMBUS_KEY_SECRET — see ./keyEnvelope.ts for + * what that does and does not buy. Nothing in this file writes the private half + * anywhere else and nothing returns it: `SigningKey.privateKey` is a jose + * KeyLike, not bytes, and the management routes hand back a kid and a status. + * + * WHICH ONE SIGNS is the single row with status 'active'. Which ones VERIFY is + * every row that is not 'retired', which is what getJwks() publishes. Those are + * different questions and used to be the same one — getJwks() returned the + * signing key and nothing else — which is what made rotation impossible without + * a flag day: a swap invalidated every token minted under the old key at the + * instant it happened, and the new key started signing before a single consumer + * had seen it in the document. + * + * SO A ROTATION IS THREE STEPS, none of which drops a request: + * + * 1. POST /admin/signing-keys mints a key as 'published'. It is in the + * JWKS immediately and signs nothing. + * 2. wait one access-token TTL every consumer's JWKS cache now has it. + * activateSigningKey ENFORCES this rather + * than trusting the operator to have counted. + * 3. POST …/:kid/activate it starts signing; the old key becomes + * 'published', so the tokens it already + * minted keep verifying until they expire. + * …/:kid/retire drops it from the document + * once they have. + * + * THE CACHE IS TIME-BOUNDED, and that is what makes step 3 take effect at all. + * It used to be for the lifetime of the process, so activating a key required a + * restart of every instance — the same flag day under another name. + * ------------------------------------------------------------------ */ export interface SigningKey { kid: string @@ -11,28 +49,88 @@ export interface SigningKey { publicJwk: JWK } -let cached: SigningKey | null = null +/** + * How long the active key is reused before the table is consulted again. + * + * Every login and every refresh signs a token, so this is a hot path and the + * read has to be amortised. Thirty seconds is the whole cost of a rotation + * propagating across instances, and it is safe to be that lazy for one reason: + * the key being activated has already been in the JWKS for an access-token TTL, + * so a straggler still signing with the previous key for another half a minute + * mints tokens every consumer can still verify. + */ +const CACHE_TTL_MS = 30_000 /** - * Load the RS256 signing key from the DB, or generate + persist one on first boot. - * The private JWK and a stable kid are stored in `signing_keys` and reused across restarts. + * How long a key must have been published before it may be activated. + * + * One access-token TTL (15m — see issueAccessToken in tokens.ts) plus a margin + * for consumers that cache the JWKS a little longer than they should. + * Activating sooner mints tokens under a `kid` verifiers have not fetched yet, + * and the symptom is every service 401ing every request until its cache turns + * over: a self-inflicted outage that looks exactly like a key compromise. */ -export async function getSigningKey(): Promise { - if (cached) return cached - - const rows = await db.select().from(signingKeys).limit(1) - const existing = rows[0] - if (existing) { - const privateKey = await importJWK(existing.privateJwk as unknown as JWK, 'RS256') - cached = { - kid: existing.kid, - privateKey, - publicJwk: existing.publicJwk as unknown as JWK, - } - return cached +export const PUBLISH_BEFORE_ACTIVE_MS = 20 * 60_000 + +/** + * How stale a verifier cache may be before an unknown `kid` forces a re-read. + * + * The `kid` in a token header is attacker-chosen, so a miss must not be a free + * database query: without this floor, a stream of tokens carrying random kids is + * a stream of SELECTs. One second is far shorter than a rotation and far longer + * than a burst, so a key minted by another instance is verifiable within a + * second of it existing, and a flood costs one query per second. + */ +const VERIFIER_MISS_RELOAD_MS = 1_000 + +let cached: { key: SigningKey; at: number } | null = null +let verifiers: { keys: Map; at: number } | null = null + +/** + * Forget the memoised active key, so the next signature re-reads the table. + * + * Exported because activateSigningKey is not the only caller that needs it: a + * suite that empties `signing_keys` between cases would otherwise keep signing + * with a row that no longer exists. It is safe to call at any time — the cost is + * one SELECT — and it cannot be used to change WHICH key signs, only to notice + * sooner that it changed. The instances that did not call it notice within + * CACHE_TTL_MS. + * + * It drops the VERIFIER cache too. Those are two caches over one table and a + * caller who wants the table re-read wants both; leaving the verifiers behind + * would make a suite that empties `signing_keys` keep accepting tokens signed by + * a key that no longer exists. + */ +export function forgetActiveSigningKey(): void { + cached = null + verifiers = null +} + +/** Row shape every reader here needs. Kept narrow so the private half is opt-in. */ +interface StoredKey { + kid: string + privateJwkEnc: string | null + publicJwk: Record | null +} + +/** Decode a stored row into something that can sign. */ +async function toSigningKey(row: StoredKey): Promise { + if (!row.privateJwkEnc) { + // Only reachable if something wrote a row without the envelope, which this + // build cannot do. Fail loudly rather than falling back to the plaintext + // column — a fallback is how a plaintext column survives forever. + throw new Error(`signing key ${row.kid} has no encrypted private half`) + } + const jwk = decryptPrivateJwk(row.kid, row.privateJwkEnc) as unknown as JWK + return { + kid: row.kid, + privateKey: await importJWK(jwk, 'RS256'), + publicJwk: row.publicJwk as unknown as JWK, } +} - // First boot: generate an extractable RS256 keypair and persist it. +/** Generate a keypair and store it in `status`. Returns the new kid. */ +async function mint(status: SigningKeyStatus): Promise { const { publicKey, privateKey } = await generateKeyPair('RS256', { extractable: true }) const kid = randomBytes(8).toString('hex') const privateJwk = await exportJWK(privateKey) @@ -41,21 +139,303 @@ export async function getSigningKey(): Promise { publicJwk.alg = 'RS256' publicJwk.use = 'sig' - await db - .insert(signingKeys) - .values({ - kid, - privateJwk: privateJwk as unknown as Record, - publicJwk: publicJwk as unknown as Record, - }) - .onConflictDoNothing() + await db.insert(signingKeys).values({ + kid, + // Encrypted before it is handed to the driver, so the plaintext never + // reaches a query log, a connection trace or a statement-level slow log. + privateJwkEnc: encryptPrivateJwk(kid, privateJwk as unknown as Record), + publicJwk: publicJwk as unknown as Record, + status, + }) + return kid +} + +/** + * Re-encrypt any signing key still sitting in the plaintext column, and empty it. + * + * Runs once per boot, before anything is served. Idempotent — a database this + * has already run against has no rows to find — and deliberately NOT part of + * migrate.ts: the DDL there must not need a secret, and this needs the one + * env.ts refuses to boot without. + * + * The UPDATE writes the envelope and NULLs the plaintext in one statement, so + * there is no window where the row has both and no way to end with neither. + * What it cannot do is reach backwards: a dump taken before this ran still holds + * the key in the clear, which is why it says so at warn and names rotation as + * the only actual remedy. + */ +export async function encryptStoredSigningKeys(log: FastifyBaseLogger): Promise { + const rows = await db + .select({ kid: signingKeys.kid, privateJwk: signingKeys.privateJwk }) + .from(signingKeys) + .where(isNotNull(signingKeys.privateJwk)) + + for (const row of rows) { + await db + .update(signingKeys) + .set({ privateJwkEnc: encryptPrivateJwk(row.kid, row.privateJwk!), privateJwk: null }) + .where(eq(signingKeys.kid, row.kid)) + } + + if (rows.length > 0) { + log.warn( + { + audit: 'signing_key_encrypted_at_rest', + kids: rows.map((r) => r.kid), + fix: 'Every backup, dump and replica taken before this boot still contains the key in plaintext and this cannot reach them. Treat it as disclosed: mint a replacement (POST /admin/signing-keys), wait one access-token TTL, activate it, then retire this one.', + }, + 'signing key was stored in plaintext and has been encrypted at rest', + ) + } + return rows.length +} - cached = { kid, privateKey, publicJwk } - return cached +/** + * The key that signs. Generates and activates one against an empty table. + * + * Cached for CACHE_TTL_MS. On the first boot of a fresh database two instances + * can both find no active key; both insert, and the deterministic ordering below + * decides which of the two wins — the same one in every process, which is the + * property that matters. Both are published, so tokens minted under either + * verify everywhere regardless. + */ +export async function getSigningKey(): Promise { + if (cached && Date.now() - cached.at < CACHE_TTL_MS) return cached.key + + const active = await db + .select() + .from(signingKeys) + .where(eq(signingKeys.status, 'active')) + // Oldest first, kid as the tie-break: a total order, so every instance picks + // the same row without coordinating, and the key that has been signing + // longest keeps signing rather than a boot race changing the answer. + .orderBy(asc(signingKeys.createdAt), asc(signingKeys.kid)) + .limit(1) + + const row = active[0] + if (row) { + const key = await toSigningKey(row) + cached = { key, at: Date.now() } + return key + } + + // Empty table (first boot), or an operator retired everything. Either way this + // service cannot mint a token without a key, so make one and use it. + const kid = await mint('active') + const created = await db.select().from(signingKeys).where(eq(signingKeys.kid, kid)).limit(1) + const key = await toSigningKey(created[0]!) + cached = { key, at: Date.now() } + return key } -/** JWKS document exposing the public signing key(s). */ +/** + * JWKS document: every key that is not retired, in a deterministic order. + * + * NOT just the signing key, which is what it used to be. A verifier keyed by + * `kid` needs the public half of whatever minted the token in front of it, and + * during a rotation that is not the key currently signing — in one direction + * because the new key must be fetchable before it signs, and in the other + * because the old key's tokens outlive its last signature by up to their TTL. + * + * The order is fixed (active first, then oldest, then kid) so the document is + * byte-identical across instances and across calls. Consumers cache this, some + * of them by comparing what they fetched with what they held; a set that + * shuffles makes that comparison meaningless and the cache useless. + */ export async function getJwks(): Promise<{ keys: JWK[] }> { - const key = await getSigningKey() - return { keys: [key.publicJwk] } + const rows = await db + .select({ + kid: signingKeys.kid, + publicJwk: signingKeys.publicJwk, + status: signingKeys.status, + createdAt: signingKeys.createdAt, + }) + .from(signingKeys) + .where(ne(signingKeys.status, 'retired')) + .orderBy(asc(signingKeys.createdAt), asc(signingKeys.kid)) + + if (rows.length === 0) { + // No key has ever been generated, or every one was retired. Generating on + // demand keeps /.well-known/jwks.json consistent with what /auth/login would + // mint a millisecond later, rather than publishing an empty document that + // teaches every consumer to cache "this issuer has no keys". + const key = await getSigningKey() + return { keys: [key.publicJwk] } + } + + const active = rows.filter((r) => r.status === 'active') + const rest = rows.filter((r) => r.status !== 'active') + return { keys: [...active, ...rest].map((r) => r.publicJwk as unknown as JWK) } +} + +/** + * The public key a token signed BY THIS SERVICE should be verified against. + * + * Nimbus verifies its own tokens locally instead of fetching its own JWKS, and + * that shortcut used to mean "verify against whatever is signing right now". + * That is the one thing a rotation deliberately makes untrue: the moment a + * replacement key is activated, every token minted in the previous fifteen + * minutes was signed by a key that is still published, still in the document + * every other service verifies against — and would have been rejected here. The + * operator who clicked activate is holding one of those tokens, so the symptom + * of a correct rotation was the console 401ing at the admin who performed it, + * while pay, game and Crucible went on accepting the same token happily. Verify + * against the same SET this issuer publishes, and the three-step rotation above + * is true for nimbus as well. + * + * NOT a weakening: the set is exactly the non-retired keys, i.e. the ones this + * service minted and still stands behind. Retiring a key still ends it here, in + * the same breath as it leaves the JWKS. + */ +export async function getVerificationKey(kid: string | undefined): Promise { + if (!verifiers || Date.now() - verifiers.at >= CACHE_TTL_MS) { + verifiers = await loadVerifiers() + } + if (kid) { + const hit = verifiers.keys.get(kid) + if (hit) return hit + // A kid this process has not seen. Either another instance minted a key + // seconds ago, or the header is forged — and only the first deserves a + // query, so the re-read is floored (see VERIFIER_MISS_RELOAD_MS). + if (Date.now() - verifiers.at >= VERIFIER_MISS_RELOAD_MS) { + verifiers = await loadVerifiers() + const retry = verifiers.keys.get(kid) + if (retry) return retry + } + } + // No kid, or one nothing published. Hand back the active key so the answer is + // a signature failure — which is the caller's doing and a 401 — rather than an + // error, which would read as this service being unavailable. + return importJWK((await getSigningKey()).publicJwk as JWK, 'RS256') +} + +async function loadVerifiers(): Promise<{ keys: Map; at: number }> { + const { keys } = await getJwks() + const map = new Map() + for (const jwk of keys) { + if (!jwk.kid) continue + map.set(jwk.kid, await importJWK(jwk, 'RS256')) + } + return { keys: map, at: Date.now() } +} + +/* ------------------------------- rotation ------------------------------- */ + +export interface SigningKeyRecord { + kid: string + status: SigningKeyStatus + createdAt: string + statusChangedAt: string + /** True while this key is in /.well-known/jwks.json. */ + published: boolean + /** When it may be activated. Null unless it is waiting out the publish window. */ + activatableAt: string | null +} + +/** Every signing key, public metadata only. The private half is not selected. */ +export async function listSigningKeys(): Promise { + const rows = await db + .select({ + kid: signingKeys.kid, + status: signingKeys.status, + createdAt: signingKeys.createdAt, + statusChangedAt: signingKeys.statusChangedAt, + }) + .from(signingKeys) + .orderBy(asc(signingKeys.createdAt), asc(signingKeys.kid)) + + return rows.map((r) => ({ + kid: r.kid, + status: r.status, + createdAt: r.createdAt.toISOString(), + statusChangedAt: r.statusChangedAt.toISOString(), + published: r.status !== 'retired', + activatableAt: + r.status === 'published' + ? new Date(r.statusChangedAt.getTime() + PUBLISH_BEFORE_ACTIVE_MS).toISOString() + : null, + })) +} + +/** Mint a replacement key. Published immediately; signs nothing yet. */ +export async function mintSigningKey(): Promise { + const kid = await mint('published') + const all = await listSigningKeys() + return all.find((r) => r.kid === kid)! +} + +/** Outcomes shared by both transitions. Each one is a distinct 4xx at the route. */ +export type RetireKeyResult = + | { status: 'ok'; keys: SigningKeyRecord[] } + | { status: 'not_found' } + | { status: 'is_active' } + +export type ActivateKeyResult = + | RetireKeyResult + | { status: 'too_soon'; activatableAt: string } + | { status: 'not_published' } + +/** + * Make a published key the signing key. The current signer becomes 'published'. + * + * REFUSES a key that has not been published for PUBLISH_BEFORE_ACTIVE_MS, and + * that refusal is the point of the design rather than an inconvenience to route + * around: activating early mints tokens under a `kid` no consumer has fetched, + * and every service in the estate then rejects every request until its JWKS + * cache turns over. The operator rotating a key BECAUSE it leaked is exactly the + * operator most likely to skip the wait, so the wait is enforced here instead of + * written in a runbook. The way to end a leaked key sooner is to activate the + * replacement and then retire the old one, which costs only the tokens it has + * already minted — at most fifteen minutes of them. + */ +export async function activateSigningKey(kid: string): Promise { + const rows = await db.select().from(signingKeys).where(eq(signingKeys.kid, kid)).limit(1) + const row = rows[0] + if (!row) return { status: 'not_found' } + if (row.status === 'active') return { status: 'is_active' } + if (row.status !== 'published') return { status: 'not_published' } + + const readyAt = row.statusChangedAt.getTime() + PUBLISH_BEFORE_ACTIVE_MS + if (Date.now() < readyAt) { + return { status: 'too_soon', activatableAt: new Date(readyAt).toISOString() } + } + + const now = new Date() + // Demote first. If the second statement fails, the estate has no active key + // for an instant and getSigningKey() mints one — recoverable, and every token + // already out there still verifies. The other order leaves two active keys, + // and then which one signs depends on which instance you ask. + await db + .update(signingKeys) + .set({ status: 'published', statusChangedAt: now }) + .where(and(eq(signingKeys.status, 'active'), ne(signingKeys.kid, kid))) + await db + .update(signingKeys) + .set({ status: 'active', statusChangedAt: now }) + .where(eq(signingKeys.kid, kid)) + + // This process is the one that changed it; the others notice within + // CACHE_TTL_MS. + forgetActiveSigningKey() + return { status: 'ok', keys: await listSigningKeys() } +} + +/** Drop a key from the JWKS. Refuses the active one — nothing else could sign. */ +export async function retireSigningKey(kid: string): Promise { + const rows = await db.select().from(signingKeys).where(eq(signingKeys.kid, kid)).limit(1) + const row = rows[0] + if (!row) return { status: 'not_found' } + if (row.status === 'active') return { status: 'is_active' } + + await db + .update(signingKeys) + .set({ status: 'retired', statusChangedAt: new Date() }) + .where(eq(signingKeys.kid, kid)) + + // Retiring is the operator saying "stop honouring this key", usually because + // it leaked, so this process must stop honouring it now rather than at the end + // of a cache window it cannot see. The other instances follow within + // CACHE_TTL_MS, the same bound every other change to this table has. + forgetActiveSigningKey() + return { status: 'ok', keys: await listSigningKeys() } } diff --git a/services/nimbus/src/mailer.ts b/services/nimbus/src/mailer.ts new file mode 100644 index 0000000..20b5075 --- /dev/null +++ b/services/nimbus/src/mailer.ts @@ -0,0 +1,150 @@ +import { createTransport, type Transporter } from 'nodemailer' +import { env } from './env.js' + +/* ------------------------------------------------------------------ * + * Outbound mail — one generic SMTP transport, configured entirely from + * the environment (see the SMTP block in ./env.ts). + * + * There is deliberately no provider in this file. Brevo, Resend, SendGrid, + * Mailtrap, a Gmail app password and a self-hosted relay are the same four + * settings and the same protocol; anything that named one of them here would + * make changing provider a code change, which is exactly the thing the owner + * asked not to happen. If a provider ever needs something SMTP cannot express, + * that is the moment to reconsider — not before. + * + * Nimbus sends exactly one kind of message today (a password-reset link), from + * a request path that must answer in bounded time, so the shape here is: reuse + * one transport, bound every attempt with a wall clock, retry at most once, and + * let the caller decide what a failure means. Nothing in here logs; the caller + * has the request-scoped logger and, more importantly, knows which of its own + * values are secret. + * ------------------------------------------------------------------ */ + +/** + * How long one send may take, end to end. + * + * Every SMTP phase gets the same budget and the whole attempt is raced against + * it as well, because nodemailer's per-phase timeouts do not cover DNS: an + * unresolvable SMTP_HOST hangs in getaddrinfo, before the connection timeout is + * armed, and would otherwise hold the HTTP request open for the resolver's own + * timeout. Bare `sendMail` has no such bound at all. + * + * This is the worst case for POST /auth/password/forgot too, because the one + * retry below never applies to a timeout — see MAX_ATTEMPTS. + */ +const ATTEMPT_TIMEOUT_MS = 6_000 + +/** + * One retry, and only one: a reset link is not worth a queue. + * + * It does not apply to a timeout. A relay that accepted the connection and then + * said nothing for six seconds is not going to answer a second six-second wait, + * and the user is sitting in front of the request while it happens — a hung + * relay must cost 6s, not 12s. The retry is for the failures that a second + * attempt genuinely fixes: a refused or reset connection, a transient 4xx. + */ +const MAX_ATTEMPTS = 2 + +/** Distinguishable so the retry can decline to wait a second time. */ +class SmtpTimeoutError extends Error { + constructor(ms: number) { + super(`smtp send timed out after ${ms}ms`) + this.name = 'SmtpTimeoutError' + } +} + +/** + * Built on first use and then reused. + * + * Not pooled. A pool holds a socket open to the relay between sends, and this + * service sends minutes or days apart — the connection is always dead by the + * next one, and a stale pooled socket fails in a way that looks like the relay + * is down. Non-pooled costs a TCP+TLS handshake per message, which for a + * password reset is not a cost anyone can measure. + */ +let transport: Transporter | null = null + +function getTransport(): Transporter | null { + const cfg = env.smtp + if (!cfg) return null + if (!transport) { + transport = createTransport({ + host: cfg.host, + port: cfg.port, + // false means "connect in the clear, then STARTTLS", which nodemailer does + // by default and requires when it is offered. 465 is implicit TLS. + secure: cfg.secure, + auth: { user: cfg.user, pass: cfg.pass }, + connectionTimeout: ATTEMPT_TIMEOUT_MS, + greetingTimeout: ATTEMPT_TIMEOUT_MS, + socketTimeout: ATTEMPT_TIMEOUT_MS, + }) + } + return transport +} + +/** True when mail can be sent at all, so callers can choose another route. */ +export function mailerConfigured(): boolean { + return env.smtp !== null +} + +export interface OutboundMail { + to: string + subject: string + /** Always send both parts. A text-only mail lands in spam; an HTML-only one is unreadable in a client that refuses HTML. */ + text: string + html: string +} + +function withTimeout(work: Promise, ms: number): Promise { + let timer: NodeJS.Timeout + return Promise.race([ + work, + new Promise((_, reject) => { + timer = setTimeout(() => reject(new SmtpTimeoutError(ms)), ms) + }), + ]).finally(() => clearTimeout(timer)) as Promise +} + +/** + * Send one message. Throws on failure — including when SMTP is not configured, + * which callers are expected to test for with `mailerConfigured()` first rather + * than discover here. + * + * The thrown error is whatever SMTP said, unmodified. Callers that hold a secret + * in the message body MUST NOT log it as-is: an SMTP server is free to quote the + * message it rejected back at you. + */ +export async function sendMail(mail: OutboundMail): Promise { + const cfg = env.smtp + const tx = getTransport() + if (!cfg || !tx) throw new Error('SMTP is not configured') + + let lastErr: unknown + for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { + try { + await withTimeout( + tx.sendMail({ + from: cfg.from, + to: mail.to, + ...(cfg.replyTo ? { replyTo: cfg.replyTo } : {}), + subject: mail.subject, + text: mail.text, + html: mail.html, + }), + ATTEMPT_TIMEOUT_MS, + ) + return + } catch (err) { + lastErr = err + // Do not spend the caller's time twice on a relay that is not answering. + if (err instanceof SmtpTimeoutError) break + // A permanent rejection (5xx: bad credentials, sender not authorised, + // recipient refused) will be rejected identically a second time, and the + // second attempt costs the caller another round trip for nothing. + const code = (err as { responseCode?: number }).responseCode + if (typeof code === 'number' && code >= 500 && code < 600) break + } + } + throw lastErr +} diff --git a/services/nimbus/src/obs.test.ts b/services/nimbus/src/obs.test.ts new file mode 100644 index 0000000..d3d4247 --- /dev/null +++ b/services/nimbus/src/obs.test.ts @@ -0,0 +1,124 @@ +import assert from 'node:assert/strict' +import { Writable } from 'node:stream' +import { test } from 'node:test' +import Fastify, { type FastifyRequest } from 'fastify' +import { loggerOptions, safeRequestUrl } from './obs.js' + +/** + * What the request logger is allowed to write down. + * + * WHY THIS IS AN END-TO-END TEST AND NOT ONLY A UNIT ONE. The defect was never in a function + * — it was that Fastify emits an "incoming request" line for every request, before any + * handler runs, through a serializer that copied `req.url` verbatim. No route could have + * prevented it and no reviewer of `routes/portal.ts` would have seen it, because the leak is + * a property of the logger and the URL shape together. So half of this file boots a real + * Fastify with the real `loggerOptions`, points pino at a buffer, and reads what actually + * came out. + * + * WHAT `shipped` IS. The serializer as it stood before this change, run through the same + * harness. A test that only asserts the fixed behaviour proves the harness cannot see the + * bug; this one shows it catching the version that had it, and then not catching the version + * that does not. + * + * THE CREDENTIAL IN QUESTION. A password-reset token: 32 random bytes as hex, the whole + * credential for taking over an account, valid for thirty minutes and stored on our side only + * as its SHA-256 precisely so that nobody with database access can recover an issued link. + * Writing it to stdout on the request that spends it hands it to Lantern, which persists it + * in an indexed column and in every backup. + */ + +const TOKEN = 'fb957465df336cc7e6e27fa9fdfd94897e00d1c64841233a3c6b0f9fa2040b24' + +/** The `req` serializer as it was before this change. */ +const shipped = { + req(req: FastifyRequest) { + return { method: req.method, url: req.url, route: req.routeOptions?.url } + }, +} + +/** + * Serve one request through a real Fastify with a real pino, and return every line it wrote. + * `url` is the raw request target, exactly as it arrives on the socket. + */ +async function logLinesFor( + url: string, + serializers?: { req(req: FastifyRequest): Record }, +): Promise { + let out = '' + const stream = new Writable({ + write(chunk, _enc, done) { + out += String(chunk) + done() + }, + }) + const options = loggerOptions('nimbus') + const app = Fastify({ + logger: { ...options, serializers: { ...options.serializers, ...serializers }, stream }, + }) + // The real page is rendered by routes/portal.ts; all this stands in for is a route at the + // same path, because what is on trial is the line Fastify writes before reaching it. + app.get('/reset', async () => 'ok') + await app.inject({ method: 'GET', url }) + await app.close() + return out +} + +/* ------------------------------ the leak ------------------------------ */ + +test('the serializer that shipped wrote the reset token to the log stream', async () => { + const lines = await logLinesFor(`/reset?token=${TOKEN}`, shipped) + // This is CF-17 verbatim: `{"req":{"method":"GET","url":"/reset?token=9f2c…"}}` at info, on + // the request that spends the token, from a process whose stdout Lantern stores. + assert.ok(lines.includes(`"url":"/reset?token=${TOKEN}"`)) +}) + +test('the serializer in use does not, for the same request', async () => { + const lines = await logLinesFor(`/reset?token=${TOKEN}`) + assert.ok(!lines.includes(TOKEN)) + // The name survives, so the line still says a token was presented — which is the part with + // diagnostic value and no secrecy. + assert.ok(lines.includes('"url":"/reset?token=[redacted]"')) + assert.ok(lines.includes('"route":"/reset"')) +}) + +test('a link that carries its token in the fragment never puts it on the wire at all', async () => { + // What a browser sends for https://account.example/reset#token=… — the fragment is not in + // the request line, so this is the whole of it. + const lines = await logLinesFor('/reset') + assert.ok(!lines.includes(TOKEN)) + assert.ok(lines.includes('"url":"/reset"')) +}) + +test('a hand-written request target that smuggles a fragment is still stripped', async () => { + // No browser sends this. A curl -g against a raw path can, and `req.url` is whatever came + // down the socket, so the serializer must not assume the '?' branch is the only one. + const lines = await logLinesFor(`/reset#token=${TOKEN}`) + assert.ok(!lines.includes(TOKEN)) +}) + +/* --------------------------- the serializer --------------------------- */ + +test('safeRequestUrl keeps every parameter name and no parameter value', () => { + assert.equal(safeRequestUrl('/account'), '/account') + assert.equal(safeRequestUrl('/reset?token=abc'), '/reset?token=[redacted]') + assert.equal( + safeRequestUrl('/login?return=https%3A%2F%2Fpay.example%2F&hint=x'), + '/login?return=[redacted]&hint=[redacted]', + ) + // A bare '?' and a bare name are both legal request targets. + assert.equal(safeRequestUrl('/health?'), '/health') + assert.equal(safeRequestUrl('/health?verbose'), '/health?verbose=[redacted]') +}) + +test('safeRequestUrl names a repeated parameter once and caps how many it names', () => { + assert.equal(safeRequestUrl('/x?a=1&a=2&b=3'), '/x?a=[redacted]&b=[redacted]') + const many = safeRequestUrl(`/x?${Array.from({ length: 30 }, (_, i) => `p${i}=v`).join('&')}`) + assert.equal(many.split('&').length, 13) + assert.ok(many.endsWith('&…')) +}) + +test('safeRequestUrl cannot be made to forge parameters in the log line', () => { + // The name is percent-decoded by URLSearchParams and must be re-encoded, or a caller + // chooses what the log line appears to say. + assert.equal(safeRequestUrl('/x?a%26b%3Dc=1'), '/x?a%26b%3Dc=[redacted]') +}) diff --git a/services/nimbus/src/obs.ts b/services/nimbus/src/obs.ts index 47f16a1..a0743cb 100644 --- a/services/nimbus/src/obs.ts +++ b/services/nimbus/src/obs.ts @@ -96,6 +96,56 @@ function scrub(text: string): string { return out } +/** How many parameter names one logged URL is allowed to name. */ +const MAX_LOGGED_PARAMS = 12 + +/** + * A request target safe to write down: the path, and the NAMES of its query + * parameters, never their values. + * + * `redact` cannot help here — it matches object paths, and the whole query + * string arrives as one opaque string in `req.url`. So the only place this can + * be decided is right here, and it is decided fail-closed: every value goes, + * including ones nobody has thought of yet. The alternative shape, an allowlist + * of "sensitive" names, is a list somebody has to remember to extend on the day + * they add a credential to a URL, which is the day they are thinking about + * something else. + * + * This is not hypothetical. Nimbus's password-reset link was `GET + * /reset?token=…`, so Fastify's own "incoming request" line wrote a live + * account-takeover credential to stdout, from where Lantern lifted it into an + * indexed column and every backup — on the one request that spends it, and in + * the one service whose schema goes to the trouble of storing only the token's + * SHA-256 so that not even an operator with database access can recover it. + * + * The names are kept because they are the part with diagnostic value and no + * secrecy: `?page=[redacted]&sort=[redacted]` still tells you which variant of + * a route was hit, and `?token=[redacted]` still tells you the caller brought + * one. `route` (the Fastify route pattern) is logged alongside and is + * unaffected. + */ +export function safeRequestUrl(url: string): string { + // A '#' cannot appear in a well-formed request target — a browser never sends + // the fragment — but `req.url` is whatever came down the socket, and the + // reset link now carries its token after exactly that character. Cut on + // whichever delimiter comes first so a hand-written request cannot smuggle a + // secret past the query-string branch below. + const cut = url.search(/[?#]/) + if (cut === -1) return url + const path = url.slice(0, cut) + if (url[cut] === '#') return `${path}#[redacted]` + + // URLSearchParams both splits and percent-decodes; re-encode so a name + // carrying '&' or '=' cannot forge extra parameters in the log line. + const names = [...new Set(new URLSearchParams(url.slice(cut + 1)).keys())] + if (names.length === 0) return path + const shown = names + .slice(0, MAX_LOGGED_PARAMS) + .map((name) => `${encodeURIComponent(name).slice(0, 64)}=[redacted]`) + if (names.length > MAX_LOGGED_PARAMS) shown.push('…') + return `${path}?${shown.join('&')}` +} + /** * Pino config for a service. `LOG_LEVEL` is the point of most of it: the moment * you need detail is never the moment you want to be editing source and @@ -116,7 +166,10 @@ export function loggerOptions(service: string) { req(req: FastifyRequest) { return { method: req.method, - url: req.url, + // Never `req.url` raw: see safeRequestUrl. Fastify logs this line for + // every request before the route runs, so anything in the query + // string is written down whether the handler wanted it or not. + url: safeRequestUrl(req.url), route: req.routeOptions?.url, remoteAddress: req.ip, userAgent: req.headers['user-agent'], diff --git a/services/nimbus/src/passwordReset.test.ts b/services/nimbus/src/passwordReset.test.ts new file mode 100644 index 0000000..fe1f946 --- /dev/null +++ b/services/nimbus/src/passwordReset.test.ts @@ -0,0 +1,246 @@ +import assert from 'node:assert/strict' +import { execFile } from 'node:child_process' +import { dirname, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' +import { promisify } from 'node:util' +import { test } from 'node:test' +import type { FastifyBaseLogger, FastifyRequest } from 'fastify' + +/** + * Where a password-reset link points, and what a failed send is allowed to write down. + * + * THE DEFECT. `resetUrlFor` built the link as `${req.protocol}://${req.host}` — the same + * derivation routes/portal.ts uses for its own redirects, where it is correct, because those + * go straight back to the browser that sent the host. A reset link is not that: it is minted + * for one person and read by another, out of an email this deployment's own relay signed and + * sent. While nothing delivered it, that was latent. Wiring SMTP made it an unauthenticated + * account takeover: + * + * curl -H 'Host: evil.attacker.test' -X POST .../auth/password/forgot \ + * -d '{"email":"victim@example.com"}' + * + * returned 202 and the relay delivered a genuine, correctly branded CloudsForge email whose + * button read http://evil.attacker.test/reset#token=. `X-Forwarded-Host` did the + * same from the far side of the tunnel — cloudflared is a private-range peer and TRUST_PROXY + * believes it. The '#' does not help: the attacker's own page reads location.hash. + * + * WHAT IS ASSERTED. That the host in the link comes from configuration and cannot be moved by + * anything on the request, and that a delivery failure still produces the one scrubbed audit + * line an operator greps for — never a stack trace, and never the token. + * + * NO DATABASE. passwordReset.ts imports db/client.ts, but postgres() does not dial until a + * query runs and neither function under test issues one, so NIMBUS_DATABASE_URL below only has + * to be a syntactically valid URL. The env-derivation cases boot env.ts in a child process for + * the reason env.test.ts gives: it is evaluated once, at import, off process.env. + */ + +const execFileAsync = promisify(execFile) +const pkgRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..') + +/** 32 random bytes as hex, the shape createPasswordResetToken issues. */ +const TOKEN = '1b0a9a8ab40a4f0e9c1d2e3f4a5b6c7d8e9f0a1b2c3d4e5f60718293a4b5c6d7' + +const PUBLIC_URL = 'https://account.forge.example' + +process.env.NIMBUS_DATABASE_URL ??= 'postgres://u:p@localhost:5432/nimbus' +// env.ts refuses to boot without one (CF-16), and importing the module below boots it. +process.env.NIMBUS_KEY_SECRET ??= 'a-test-secret-that-is-long-enough' +process.env.NIMBUS_PUBLIC_URL = PUBLIC_URL +process.env.NIMBUS_ISSUER = 'https://nimbus.forge.example' +// Loopback with nothing listening: getTransport() builds a transport, every send is refused +// at connect, and the failure path runs in milliseconds without touching the network. +process.env.SMTP_HOST = '127.0.0.1' +process.env.SMTP_PORT = '1' +process.env.SMTP_USER = 'u' +process.env.SMTP_PASS = 'p' +process.env.SMTP_FROM = 'CloudsForge ' + +const { deliverPasswordReset, resetUrlFor } = await import('./passwordReset.js') + +/* ------------------------------- helpers ------------------------------- */ + +interface Line { + obj: Record + msg: string +} + +/** Just enough logger to record what was written, in the shape pino takes it. */ +function recorder(): { log: FastifyBaseLogger; lines: Line[] } { + const lines: Line[] = [] + const write = (obj: unknown, msg?: string) => + lines.push({ obj: (obj ?? {}) as Record, msg: msg ?? '' }) + const log = { info: write, warn: write, error: write, debug: write, fatal: write, trace: write } + return { log: log as unknown as FastifyBaseLogger, lines } +} + +/** + * A request as Fastify presents one. `host` is already the resolved value: Fastify has + * applied X-Forwarded-Host by then when the peer is trusted, which is exactly why a forged + * header and a forged Host line are the same test here. + */ +function request(host: string, protocol: 'http' | 'https' = 'http'): { + req: FastifyRequest + lines: Line[] +} { + const { log, lines } = recorder() + return { req: { host, protocol, log } as unknown as FastifyRequest, lines } +} + +/* --------------------------- where the link points --------------------------- */ + +test('a forged Host cannot move the reset link', () => { + const { req, lines } = request('evil.attacker.test') + const url = resetUrlFor(req, TOKEN) + + assert.equal(url, `${PUBLIC_URL}/reset#token=${TOKEN}`) + // Said again as the harm rather than as the value, so that a future change which merely + // relocates the interpolation cannot pass this. + assert.equal(new URL(url).origin, PUBLIC_URL) + assert.ok(!url.includes('attacker'), 'the attacker chose where a live token is delivered') + + // The one thing the request DOES get: a line naming the host it asked for. It is the only + // warning anybody receives that this is being probed. + const warned = lines.filter((l) => l.obj.audit === 'password_reset_host_ignored') + assert.equal(warned.length, 1) + assert.equal(warned[0]!.obj.addressedAs, 'http://evil.attacker.test') + assert.equal(warned[0]!.obj.minted, PUBLIC_URL) + // …and it must not contain the credential it is warning about. + assert.ok(!JSON.stringify(warned[0]).includes(TOKEN)) +}) + +test('an X-Forwarded-Proto upgrade cannot move it either', () => { + // Fastify resolves X-Forwarded-Proto into req.protocol for a trusted peer, so an https + // scheme with an attacker host is the second half of the same probe. + const { req } = request('xfh.attacker.test', 'https') + assert.equal(resetUrlFor(req, TOKEN), `${PUBLIC_URL}/reset#token=${TOKEN}`) +}) + +test('the shipped derivation is what this catches', () => { + // `${req.protocol}://${req.host}` — the line as it stood. Kept here so the case above is + // demonstrably able to see the bug rather than only to agree with the fix. + const shipped = (host: string, protocol: string) => `${protocol}://${host}/reset#token=${TOKEN}` + assert.equal(shipped('evil.attacker.test', 'http'), `http://evil.attacker.test/reset#token=${TOKEN}`) + assert.notEqual(shipped('evil.attacker.test', 'http'), resetUrlFor(request('evil.attacker.test').req, TOKEN)) +}) + +test('a request on a configured host is minted without a warning', () => { + for (const host of ['account.forge.example', 'nimbus.forge.example']) { + // Both are legitimate ways to reach nimbus (the portal hostname and the issuer), so + // neither may cry wolf — a warning that fires on the normal path is a warning nobody + // reads on the abnormal one. + const { req, lines } = request(host, 'https') + assert.equal(resetUrlFor(req, TOKEN), `${PUBLIC_URL}/reset#token=${TOKEN}`) + assert.deepEqual(lines, [], `${host} is a real nimbus host and must not warn`) + } +}) + +test('the token is in the fragment, so it is not in any request line', () => { + const url = new URL(resetUrlFor(request('account.forge.example', 'https').req, TOKEN)) + assert.equal(url.pathname, '/reset') + assert.equal(url.search, '') + assert.equal(new URLSearchParams(url.hash.slice(1)).get('token'), TOKEN) +}) + +/* ------------------------ what a failed send writes down ------------------------ */ + +test('a delivery failure is reported, not thrown, and carries no token', async () => { + const { log, lines } = recorder() + const url = `${PUBLIC_URL}/reset#token=${TOKEN}` + + const result = await deliverPasswordReset(log, { userId: 'u1', email: 'someone@example.com' }, url) + + assert.deepEqual(result, { delivered: false, channel: 'none' }) + const failed = lines.filter((l) => l.obj.audit === 'password_reset_delivery_failed') + assert.equal(failed.length, 1, 'the one line an operator greps for when mail stops working') + assert.ok(!JSON.stringify(lines).includes(TOKEN)) +}) + +test('a resetUrl the WHATWG URL parser rejects does not throw out of the delivery path', async () => { + // `new URL()` used to run over the resetUrl on this path, and the resetUrl used to come + // from the Host header — which Node's HTTP parser accepts in shapes the URL parser refuses + // (`Host: a^b`). The TypeError escaped into the route's catch, so the scrubbed audit line + // above was replaced by a stack trace: mail stopped working and the line saying so was the + // line that went missing. The link cannot take that shape any more; the contract that + // nothing in here throws is asserted anyway, because it is the contract. + const { log, lines } = recorder() + const url = `http://a^b/reset#token=${TOKEN}` + + const result = await deliverPasswordReset(log, { userId: 'u1', email: 'someone@example.com' }, url) + + assert.deepEqual(result, { delivered: false, channel: 'none' }) + assert.equal(lines.filter((l) => l.obj.audit === 'password_reset_delivery_failed').length, 1) + assert.ok(!JSON.stringify(lines).includes(TOKEN)) +}) + +/* ---------------------------- where the origin comes from ---------------------------- */ + +/** Boot env.ts under `vars` and report what it made of NIMBUS_PUBLIC_URL. */ +async function bootPublicUrl( + vars: Record, +): Promise<{ publicUrl: string; aliases: string[]; bootLines: { level: number; msg: string }[] }> { + const MARK = '__PUBLIC_URL__' + const source = ` + import { env } from './src/env.ts' + console.log('${MARK}' + JSON.stringify({ publicUrl: env.publicUrl, aliases: env.publicUrlAliases })) + ` + const { stdout } = await execFileAsync(process.execPath, ['--import', 'tsx', '-e', source], { + cwd: pkgRoot, + env: { + PATH: process.env.PATH ?? '', + HOME: process.env.HOME ?? '', + TMPDIR: process.env.TMPDIR ?? '', + NIMBUS_DATABASE_URL: 'postgres://u:p@localhost:5432/nimbus', + // env.ts refuses to boot without it (CF-16), so every child boot needs one. + NIMBUS_KEY_SECRET: 'a-test-secret-that-is-long-enough', + ...vars, + }, + }) + const line = stdout.split('\n').find((l) => l.startsWith(MARK)) + assert.ok(line, `env.ts printed no result. stdout:\n${stdout}`) + return { + ...(JSON.parse(line.slice(MARK.length)) as { publicUrl: string; aliases: string[] }), + bootLines: stdout + .split('\n') + .filter((l) => l.startsWith('{')) + .map((l) => JSON.parse(l) as { level: number; msg: string }), + } +} + +test('unset, the origin falls back to the issuer — never to a header', async () => { + const booted = await bootPublicUrl({ NIMBUS_ISSUER: 'https://nimbus.forge.example' }) + assert.equal(booted.publicUrl, 'https://nimbus.forge.example') + assert.deepEqual(booted.aliases, ['https://nimbus.forge.example']) +}) + +test('a malformed NIMBUS_PUBLIC_URL falls back and says so, rather than reopening the hole', async () => { + const booted = await bootPublicUrl({ + NIMBUS_ISSUER: 'https://nimbus.forge.example', + NIMBUS_PUBLIC_URL: 'account.forge.example', + }) + // No scheme is the likely mistake, and it is not a URL. Falling back to the issuer costs a + // link on the less friendly hostname; falling back to the request would cost an account. + assert.equal(booted.publicUrl, 'https://nimbus.forge.example') + const errs = booted.bootLines.filter((l) => l.msg.startsWith('NIMBUS_PUBLIC_URL')) + assert.equal(errs.length, 1) + assert.equal(errs[0]!.level, 50) +}) + +test('a path or query on NIMBUS_PUBLIC_URL is reduced to the origin', async () => { + const booted = await bootPublicUrl({ + NIMBUS_ISSUER: 'https://nimbus.forge.example', + NIMBUS_PUBLIC_URL: 'https://account.forge.example/portal?x=1', + }) + assert.equal(booted.publicUrl, 'https://account.forge.example') +}) + +test('a non-http scheme is refused', async () => { + // `javascript:` and `data:` parse perfectly well as URLs. A link built from one would be + // rendered as an anchor href in an email and pasted into a browser bar by a user who was + // told to expect exactly that. + const booted = await bootPublicUrl({ + NIMBUS_ISSUER: 'https://nimbus.forge.example', + NIMBUS_PUBLIC_URL: 'javascript:alert(1)', + }) + assert.equal(booted.publicUrl, 'https://nimbus.forge.example') + assert.equal(booted.bootLines.filter((l) => l.msg.startsWith('NIMBUS_PUBLIC_URL')).length, 1) +}) diff --git a/services/nimbus/src/passwordReset.ts b/services/nimbus/src/passwordReset.ts index 58f797e..fa9d467 100644 --- a/services/nimbus/src/passwordReset.ts +++ b/services/nimbus/src/passwordReset.ts @@ -2,6 +2,7 @@ import { createHash, randomBytes } from 'node:crypto' import { and, desc, eq, isNull, lt, gt } from 'drizzle-orm' import type { FastifyBaseLogger, FastifyRequest } from 'fastify' import { env } from './env.js' +import { mailerConfigured, sendMail } from './mailer.js' import { db } from './db/client.js' import { passwordResetTokens, users } from './db/schema.js' @@ -59,21 +60,66 @@ export async function createPasswordResetToken( } /** - * The absolute /reset link carrying a token, built from the host this request - * actually arrived on — the same derivation the portal uses for its own return - * URLs (routes/portal.ts baseUrl). There is deliberately no NIMBUS_PUBLIC_URL - * setting to get wrong: nimbus answers on two hostnames (nimbus.* and - * account.*), both serve this portal, and a link minted on the host the - * operator is already talking to is reachable by construction. + * The absolute /reset link carrying a token. + * + * THE HOST COMES FROM CONFIGURATION, NOT FROM THE REQUEST, and that is the + * whole of this function. It used to be `${req.protocol}://${req.host}`, the + * same derivation routes/portal.ts uses for its own return URLs — which is + * right THERE, because those links go straight back to the browser that sent + * the host, and wrong here, because this one is put in an email and read by + * somebody else. While nothing delivered it that was a latent bug; wiring SMTP + * turned it into an unauthenticated account takeover. `curl -H 'Host: + * evil.test' -X POST /auth/password/forgot -d '{"email":"victim@…"}'` had the + * deployment's own relay send the victim a genuine, correctly branded reset + * email whose button was `http://evil.test/reset#token=`, and + * `X-Forwarded-Host` did the same from the far side of the tunnel, because + * cloudflared is a private-range peer that env.trustProxy believes. See + * env.publicUrl / NIMBUS_PUBLIC_URL. + * + * The fragment does not save it: the attacker's own page reads + * `location.hash`. Nothing about where the token sits in a URL helps when the + * attacker chose the origin. + * + * `req` is still taken, for one reason: a request arriving on a host that is + * neither NIMBUS_PUBLIC_URL nor NIMBUS_ISSUER is either a probe for exactly + * this or a misconfiguration, and this is the only place either can be seen. + * The link is minted the same way regardless. * * Both callers live here rather than restating the format: the self-service * /auth/password/forgot route and the operator's POST * /admin/users/:id/password-reset must produce the identical link, or one of * the two paths quietly stops working. + * + * THE TOKEN GOES AFTER THE '#', NOT AFTER THE '?'. A fragment is the one part + * of a URL a browser keeps to itself: it is not in the request line, so it + * reaches no server log, no reverse-proxy access log and no Referer header on + * the next navigation. It used to be `?token=…`, and the consequence was that + * Fastify's own "incoming request" line wrote the live credential to stdout on + * the very request that spends it — from where Lantern lifted it into an + * indexed column and every backup. obs.ts now strips query values as well, but + * these two defences protect different things and both are wanted: the + * serializer covers OUR logs, and the fragment covers everything between the + * user's browser and us, which we do not run and cannot redact. + * + * routes/portal.ts reads it back out of `location.hash`. */ export function resetUrlFor(req: FastifyRequest, token: string): string { - const base = `${req.protocol}://${req.host || `localhost:${env.port}`}` - return `${base}/reset?token=${encodeURIComponent(token)}` + const addressedAs = req.host ? `${req.protocol}://${req.host}` : null + if (addressedAs && !env.publicUrlAliases.includes(addressedAs)) { + // Not an error and not a refusal — a request on an unexpected host is + // answered exactly like any other, because refusing would tell the sender + // which hosts are real. The link simply does not follow it. + req.log.warn( + { + audit: 'password_reset_host_ignored', + addressedAs, + minted: env.publicUrl, + fix: 'If this host is a legitimate way to reach the portal, set NIMBUS_PUBLIC_URL to it (or add the route in front of nimbus). If it is not, somebody is probing the reset link with a forged Host or X-Forwarded-Host header; the link they got points here, not there.', + }, + 'password reset requested on an unrecognised host — link minted from NIMBUS_PUBLIC_URL', + ) + } + return `${env.publicUrl}/reset#token=${encodeURIComponent(token)}` } /** Redeem a reset token exactly once. Returns the user id, or null. */ @@ -146,48 +192,225 @@ export async function listPendingResets(): Promise { } /* ------------------------------------------------------------------ * - * DELIVERY SEAM — READ THIS BEFORE WIRING ANYTHING UP - * - * There is no mail infrastructure anywhere in this estate, and inventing one - * here would have meant a dependency, a secret and an outbound network path - * that nothing else in the platform has. So the token half above is complete - * and correct, and delivery is this one function. - * - * Today it delivers NOTHING. A self-service reset request mints a valid token - * that is then dropped on the floor, and the working path is the operator one: - * an admin issues a link from the console (POST /admin/users/:id/password-reset) - * and hands it over out of band. `listPendingResets()` is how they see who is - * waiting. That is deliberate and the user-facing copy says so plainly. - * - * To finish it, replace the body below with a real send and return - * `{ delivered: true, channel: '' }`. Requirements: - * - `resetUrl` is the only copy of the token in existence. Do not log it, do - * not persist it, do not put it in an error message. + * DELIVERY — the seam, now wired. + * + * The token half above is complete on its own, and delivery is this one + * function. It has two supported modes and both are real deployments: + * + * SMTP_HOST set — the link is emailed, over a generic SMTP transport that + * any relay can serve (see ./mailer.ts and env.ts). + * SMTP_HOST unset — nothing is sent, exactly as before mail existed: the + * request is recorded, and an operator issues the link + * from the console (POST /admin/users/:id/password-reset). + * `listPendingResets()` is how they see who is waiting. + * + * The three rules the seam was written under still hold, and every one of them + * is load-bearing: + * - `resetUrl` is the only copy of the token in existence. It is not logged, + * not persisted, and not allowed into an error message — an SMTP server may + * quote the message it rejected back at us, so the failure path scrubs the + * token out of the error text rather than trusting it not to be there. * - Throwing here must not fail the request: the caller answers 202 * regardless, because a different answer for a known and an unknown email - * is an account-enumeration oracle. - * - Nothing else needs to change. The routes, the table and the TTL are done. + * is an account-enumeration oracle. Nothing below can throw; failure is + * reported through the return value. + * - The routes, the table and the TTL are unchanged. * ------------------------------------------------------------------ */ export interface ResetDelivery { delivered: boolean - /** What carried it, for the log line. 'none' while the seam is unwired. */ + /** What carried it, for the log line. 'none' when no mail is configured. */ channel: string } +/** HTML-escape. The link's host comes from the request, so it is not ours to trust. */ +const esc = (s: string) => + s.replace(/&/g, '&').replace(//g, '>').replace(/"/g, '"') + +/** + * Remove the token from anything about to be written down. + * + * Belt and braces over "do not log the URL": the values that reach a log line + * on the failure path are error messages from a remote server, and the one + * thing that must never appear in one is the string we just handed it. + */ +function withoutToken(text: string, resetUrl: string): string { + let out = text.split(resetUrl).join('[reset link redacted]') + + // Read the token with a regexp rather than `new URL()`. The parser is the + // better tool right up until it throws: this runs on the failure path, inside + // the one function whose contract says nothing in it can throw, and the + // exception would replace the scrubbed audit line — the single line an + // operator greps for when mail stops working — with a stack trace. That was + // not theoretical while the URL came off the Host header, which Node's HTTP + // parser accepts in shapes the WHATWG URL parser rejects (`Host: a^b`). + // + // The token rides in the fragment (see resetUrlFor) and is read from the query + // string as well, because a link minted before that change is still redeemable + // for its remaining thirty minutes and its token must be scrubbed too. + const token = /[#?&]token=([^&\s]+)/.exec(resetUrl)?.[1] + if (!token) return out + + out = out.split(token).join('[redacted]') + // The link is percent-encoded; a server quoting it back may not be. The token + // is hex today, so this is a no-op — and it is here so it stays one if the + // alphabet ever changes. + let decoded: string | null = null + try { + decoded = decodeURIComponent(token) + } catch { + decoded = null + } + if (decoded && decoded !== token) out = out.split(decoded).join('[redacted]') + return out +} + +/* The portal's palette, flattened for mail clients — no CSS variables, no + * color-mix(), no gradients, no