From 6f6b54a5093b65e5ed3222da74b22a837c881b9d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 21:01:19 -0500 Subject: [PATCH] =?UTF-8?q?docs(website):=20=C2=A77=20rewritten=20for=20SM?= =?UTF-8?q?TP,=20plus=20an=20upgrade-notes=20home?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Engagement Phase 1's documentation (ENGAGEMENT.md §6.0b, row 1). BACKEND_DESIGN.md §7 is REWRITTEN rather than amended, as the plan requires: it documented Gmail OAuth2 as the mechanism. It now covers the transport registry and why credentialFields is a single declaration three consumers read, the credential blob and its fail-safe decrypt, the no-phone-home rule and its CI guardrail, the three SMTP postures, why Send test is the only verification left, the five failure contracts, and the silent upgrade. The §2 tree, the PR-4 route-count note, the contact row and the dashboard row are corrected alongside it — the dashboard now returns warnings[]. UPGRADE_NOTES.md is new, and is the home for every later phase's operator note as well (1b, 6, 10 and 13 each owe one). Newest first, and scoped deliberately: an upgrade that needs nothing does not get an entry. The entries worth writing are the ones that fail QUIETLY, which is exactly what the Gmail removal does — mail stops, nothing errors, and the only signal is a dashboard warning. ENGAGEMENT.md records Phase 1 as built, with the four things that are not what the plan says: only the transport half of §3.1 was built (a channel nothing calls is a shape frozen too early), `enabled` now gates every sender where it used to gate none of the direct ones, the G22 warning reads the deprecated column on purpose, and `npm run swagger` had to be fixed first — the "per-file route limit" phases 8 and 9 recorded does not exist, and the real rule matters to every later phase here, all of which add routes. Co-Authored-By: Claude --- README.md | 1 + website/BACKEND_DESIGN.md | 80 ++++++++++++++++++++++++++++++++------- website/ENGAGEMENT.md | 40 +++++++++++++++++++- website/UPGRADE_NOTES.md | 74 ++++++++++++++++++++++++++++++++++++ 4 files changed, 180 insertions(+), 15 deletions(-) create mode 100644 website/UPGRADE_NOTES.md diff --git a/README.md b/README.md index 699ceab..28e4e8b 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,7 @@ sidecar as a service, and hands you the values the website needs. | [THEMING_AND_NAV.md](website/THEMING_AND_NAV.md) | Admin-configurable theme, brand assets and navigation — build contract | | [MODULE_SYSTEM.md](website/MODULE_SYSTEM.md) | Making the site game-agnostic: game logic becomes an installable module — design of record | | [MODULE_API.md](website/MODULE_API.md) | The module ↔ core contract: `ctx`, the `register*` calls, the client registry and the loader's obligations | +| [UPGRADE_NOTES.md](website/UPGRADE_NOTES.md) | **Operator-facing, newest first** — the upgrades that need an operator to do something, or that change behaviour quietly enough to be discovered by accident | | [WIKI_UPGRADE.md](website/WIKI_UPGRADE.md) | Wiki subsystem upgrade notes | | [SHARD_VISIBILITY.md](website/SHARD_VISIBILITY.md) | Who sees which shard data — the admin-configurable audience framework | | [TRUSTED_DEVICES_MFA.md](website/TRUSTED_DEVICES_MFA.md) | TOTP two-factor, trusted devices and recovery codes | diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index 34c0ee1..6e3d892 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -40,7 +40,8 @@ Skeleton from the spec, with a small number of justified additions marked **(+)* > > `users`, `account`, `invites`, `auth/providers` (PR 1, 28 routes), `moderation`, `bot-activity`, > `activity` (PR 2, 18 routes), `posts`, `uploads`, `wiki`, `pages` (PR 3, 31 routes) and `shard`, -> `uo-link`, `email`, `discord-bot`, `settings`, `dashboard`/`site-mode` (PR 4, 33 routes) each live +> `uo-link`, `email`, `discord-bot`, `settings`, `dashboard`/`site-mode` (PR 4, 33 routes at the time; +> `email` has since lost its two Gmail connect routes, §7) each live > in their own router under `admin/`, behind `admin/index.js`. PR 5 did the same for `public/` (24), > `player/` (20) and the residual `auth/` (10). **`admin.routes.js`, `public.routes.js`, > `player.routes.js` and `auth.routes.js` are all deleted**; each group is now a directory whose @@ -170,8 +171,11 @@ server/ from a manifest URL, enable, disable, uninstall, purge, restart and the source allowlist - email.router.js (6) /admin/email — Gmail OAuth2 - delivery — adminOnly + email.router.js (4) /admin/email — outbound mail: + transport + credentials + send + test — adminOnly. The two + /connect/* routes went with Gmail + OAuth2 (§7) discordBot.router.js (2) /admin/discord-bot — adminOnly settings.router.js (4) /admin/settings — adminOnly. The DELETE /:key is "reset to default" @@ -207,7 +211,8 @@ server/ utils/ auth.js JWT sign/verify, isLoggedIn middleware db.js MariaDB pool + ensureSchema() - mailer.js (+) nodemailer; mailto fallback if SMTP unset + mailer.js (+) nodemailer over a registered transport; + mailto fallback when unconfigured (§7) client/ built in Phase 2/3 (React + Vite) Dockerfile docker-compose.yml @@ -923,7 +928,7 @@ from the per-route **siteMode** middleware (§5), never from an auth gate. | GET | `/posts/:category/:idOrSlug` | single published post | | GET | `/wiki` | list of pages (slug + title) | | GET | `/wiki/:slug` | single page | -| POST | `/contact` | (rate-limited) send mail via SMTP; if unconfigured, respond `{fallback:"mailto", email}` | +| POST | `/contact` | (rate-limited) send mail via the configured transport (§7); if unconfigured, respond `{fallback:"mailto", email}` | | GET | `/teams/by-external/:moduleId/:externalId` | one Team named the way the OWNING MODULE names it. Exists so a module's page can find core's Team without holding core's identifiers, which are core-internal. The module id is matched rather than trusted: an external id is unique only within a module | | GET | `/teams` | active, publicly visible Teams, paged. Every payload carries `{ configured, stale, lastSyncAt }` so a page can say how recently the projection was confirmed rather than presenting a stale roster as current, plus `enabled` — whether this deployment has Teams at all | | GET | `/teams/:slug` | one Team. An **archived** Team still resolves, read-only, and names its successor when it was renamed — an old bookmark or Discord link lands somewhere that explains itself. A **hidden** Team returns 404, indistinguishable from one that does not exist: "absent from every public surface" includes not confirming it is there. Carries `id`/`externalId`/`moduleId` — this route only, since the index has no use for them | @@ -973,7 +978,7 @@ never shown a control that would 403. The URLs below are unaffected by which file a route sits in — that is the property the route manifest freezes. | Method | Path | Purpose | |---|---|---| -| GET | `/dashboard` | current mode, last change time + who, content counts, recent activity | +| GET | `/dashboard` | current mode, last change time + who, content counts, recent activity, and `warnings[]` — operator conditions that are quietly not working and would otherwise be discovered by somebody not receiving an email. Normally empty. Each entry is `{code, message, href}` and each one is computed defensively: a warning that can 500 the admin landing page is a worse bug than the one it reports. Today there is one, `EMAIL_TRANSPORT_MIGRATION` (§7) | | PUT | `/site-mode` | `{mode}` → update settings, stamp who/when, log `site_mode.change` | | GET | `/posts?category=` | all posts incl. unpublished | | POST | `/posts` | create | @@ -1144,13 +1149,62 @@ still at [`SHARD_VISIBILITY.md`](SHARD_VISIBILITY.md). ## 7. Email -`utils/mailer.js` (nodemailer) sends through **Gmail over OAuth2 (SMTP XOAUTH2)**, configured in -Admin → Settings → Email — not env. The mailbox is authorized by an in-app "Connect Gmail" consent -flow (`/admin/email/*`) that captures a refresh token, stored AES-GCM-encrypted in the `email_config` -singleton (never returned over the API). The OAuth client id/secret are reused from the `google` -auth-providers row. Recipient is the `contact_email` site setting. If email is unconfigured/disabled, -`POST /public/contact` returns `{fallback:"mailto", email}` so the client renders a `mailto:` link -instead. Errors never leak credentials. +`utils/mailer.js` (nodemailer) sends through a **registered mail transport**, configured in +Admin → Settings → Email — never env, and never a compiled-in provider. Gmail OAuth2 and its consent +flow were **removed** in engagement Phase 1 ([`ENGAGEMENT.md`](ENGAGEMENT.md) §1.2a); SMTP is the +baseline and the only transport core ships. + +**Transport, not provider.** `server/src/engagement/transports/` holds the registry and its one +registration. A transport declares an id, a label and its own **`credentialFields`**, and that +declaration is the single thing the admin form renders, the request sanitizer filters against, and +the "is this value secret" answer comes from — so adding a relay is a registration, not four edits +across a form, a validator, a column set and a model. `registerDeliveryChannel`, the other half of +§3.1, arrives with the engine that consumes it. + +**Configuration** lives in the `email_config` singleton: `transport` (default `smtp`), +`sender_email`, `sender_name`, `reply_to`, and `credential_enc` — the transport's whole credential +set as ONE AES-256-GCM JSON blob (`utils/secretBox.js`), because the field list belongs to the +transport and a column per union member would make each new transport a schema change. The blob is +**write-only over the API**: secret fields are never returned, only a per-field `secretsSet` flag, +and a blob that will not decrypt reads as *absent* rather than raising — a rotated `SECRET_ENC_KEY` +must land an admin on a screen that says "unconfigured", not a 500 that takes the contact form with +it. `provider` and `refresh_token_enc` remain as **deprecated, unread columns** under the +additive-only discipline. + +**No phone-home.** No transport may ship a default host, port, endpoint or sender +([`ENGAGEMENT.md`](ENGAGEMENT.md) §3.2). A transport with no operator configuration is +`unconfigured` and its channel is off — it never falls back to a destination we chose. +`npm run check:hosts` is the CI guardrail; it reads code, not prose, so documentation naming a host +is fine and a literal in a transport is not. + +**Three supported SMTP postures**, in the order an operator should consider them: + +| Posture | Shape | When | +| --- | --- | --- | +| **A relay** (recommended) | Mailgun / SES / Postmark, host + port 587 + API-key-as-password | Anything with real volume. Reputation, bounce handling and DKIM are the relay's problem, not the operator's | +| **A mailbox provider over SMTP** | e.g. `smtp.gmail.com` port 587 with an **app password** (not the account password) | A small deployment, and the migration path off the removed OAuth2 flow. Subject to the provider's own daily send caps | +| **A self-hosted MTA** | An unauthenticated relay on port 25 on the same host | An operator who already runs mail. `user` and `password` are left blank; the transport treats a username with no password as incomplete, since that authenticates as nobody | + +`secure` is the field operators get wrong: on for implicit TLS on 465, **off** for 587, which +nodemailer upgrades with STARTTLS. 587-with-secure-on hangs rather than erroring cleanly. + +**"Send test" is the verification, and it has to be.** Under the removed consent flow the sending +address came back from Google's userinfo and was guaranteed to be a mailbox the credential owned. +Operator-typed, it can be refused by the relay — a silent SPF/DMARC deliverability failure, not an +error — so `POST /admin/email/test` is the only thing that proves the whole configuration, and its +failures name the sender and the likely cause rather than passing a bare `550` through. + +**Failure contracts.** Recipient for the contact form is the `contact_email` site setting. If email +is unconfigured or disabled, `POST /public/contact` returns `{fallback:"mailto", email}` so the +client renders a `mailto:` link, invites return the accept link for an admin to share by hand, +password resets still answer a generic 200, and Team notifications are logged and swallowed. Only +the admin test send throws — it is the only one with someone waiting to be told. Errors never leak +credentials. + +**The upgrade is silent by design and therefore announced.** An existing deployment backfills to +`transport='smtp'` with no credentials, so every sink above politely does nothing and **mail simply +stops**. The admin dashboard warns whenever the deprecated Gmail token is present and no replacement +credential is; see [`UPGRADE_NOTES.md`](UPGRADE_NOTES.md). --- diff --git a/website/ENGAGEMENT.md b/website/ENGAGEMENT.md index df07a5a..df78384 100644 --- a/website/ENGAGEMENT.md +++ b/website/ENGAGEMENT.md @@ -1,6 +1,7 @@ # The Engagement System — findings and plan -**Status:** design of record for the next workstream. No code written yet. The scope decisions below are +**Status:** design of record. **Phase 1 is built** (website#165 + docs#178, with website#164 as its +prerequisite); everything from Phase 1b on is still design. The scope decisions below are settled; **four of the eight questions in §7.1 were answered by the org lead on 2026-08-28** — Q1, Q3, Q5 and Q7, and Q1's answer added a whole phase (**Phase 1b**, unique email addresses). Q2, Q4, Q6 and Q8 remain open and block Phases 4, 5b, 2 and 8 respectively. Per CLAUDE.md § Conventions, no implementation @@ -1236,7 +1237,7 @@ Phase 4**, **Q4 before Phase 5b**, **Q8 before Phase 8**. --- -### Phase 1 — Remove Gmail OAuth2; `DeliveryChannel` + SMTP +### Phase 1 — Remove Gmail OAuth2; the transport registry + SMTP ✅ **This phase is a subtraction and a replacement in one PR**, because leaving the OAuth2 flow half-wired across a release is worse than either end state. @@ -1261,6 +1262,41 @@ deployment does today — contact form falls back to `mailto`, invites surface t **Guardrails:** swagger regen + `routes:manifest --check` (two routes removed); no-hardcoded-host check (§3.2 rule 4) — which the deleted `smtp.gmail.com` literal is the first real test of. +#### As built (website#165, docs#178) + +Delivered as specified, with four things worth recording because they are not what the plan above says. + +1. **Only half of §3.1 was built, deliberately.** `registerMailTransport` ships; + `registerDeliveryChannel` does not. Phase 1 has no consumer for `addressFor`/`render`/`deliver` — + the engine that calls them is Phase 4 — and a registered channel nothing calls is a shape frozen + before anything tried to use it. It arrives with the phase that consumes it. `credentialFields` is + the piece that mattered here, since it is what makes the admin form generic. +2. **`enabled` now gates every sender, which it did not before.** `buildTransport()` used to test only + "is there a refresh token and a sender", so the contact form kept sending after an admin unticked + *Enable email sending* — `isConfigured()` honoured the toggle but the five direct senders bypassed + it. The connect flow used to set `enabled` as a side effect of a consent redirect; with a credential + form it has to mean what it says, so the gate moved onto the one path every sender shares. **A + deliberate behaviour change, not a refactor**, and the only one in the phase. +3. **The G22 warning reads the deprecated column.** `refresh_token_enc` stays unread *as + configuration*, but its presence is exactly "this deployment had working mail before the upgrade", + which is the warning's whole condition. `hadLegacyConnection && !hasCredential` fires for the one + deployment this happens to and stays silent on a fresh install, which has never had mail and would + only learn to ignore the banner. The warning clears itself once a credential is saved — nothing has + to remember to dismiss it. +4. **`npm run swagger` was already broken and had to be fixed first** (website#164, its own PR). It + died with swagger-autogen's "invalid array length" runaway on a pristine `edge`, under Node 20 and + 24, and at the commit whose own PR last regenerated the spec — so no phase that touches a route + could have met the standing obligation. Bisected to one statement in `teams.router.js`: + `param('teamId').custom((v) => v === 'default' || TEAM_ID.test(v))`. **The rule is that nothing + reaching `.test(` may sit inside a route statement**, and the "per-file route limit" that phases 8 + and 9 recorded does not exist — a three-route file carrying only that one route dies too. Worth + knowing for every later phase in this workstream, all of which add routes. + +Two smaller decisions: the credential is one encrypted JSON blob rather than a column per field, so +adding a transport is never a schema change; and a blob that will not decrypt reads as **absent** +rather than raising, so a rotated `SECRET_ENC_KEY` lands an admin on an "unconfigured" screen instead +of a 500 that takes the contact form with it. + --- ### Phase 1b — Unique, changeable, verifiable email addresses *(decision 6)* diff --git a/website/UPGRADE_NOTES.md b/website/UPGRADE_NOTES.md new file mode 100644 index 0000000..fbab6b5 --- /dev/null +++ b/website/UPGRADE_NOTES.md @@ -0,0 +1,74 @@ +# Upgrade notes — website + +**Operator-facing. Newest first.** One entry per change that requires an operator to *do* something, +or that changes behaviour they would otherwise discover by accident. + +Most upgrades need nothing: the schema is replayed on every boot with `IF NOT EXISTS` migrations, and +a deployment that pulls a new image and restarts is done. This file exists for the exceptions — and +the exceptions worth writing down are almost always the ones that **fail quietly**, because a loud +failure announces itself and a quiet one does not. + +**Read the entries between the version you are on and the version you are moving to.** Each names the +action, whether it is required, and what happens if you do nothing. + +--- + +## Outbound email: the Gmail connect flow is gone; configure SMTP + +**Required, if this deployment currently sends email.** Engagement Phase 1. + +### What changed + +The "Connect Gmail" consent flow has been **removed**. Email now sends through an ordinary mail +transport whose credentials an admin types in — SMTP is the baseline and the only transport shipped. + +Removed with it: `GET /api/v1/admin/email/connect/start`, `GET /api/v1/admin/email/connect/callback`, +and email's reuse of the `google` authentication provider's client id and secret. That reuse is worth +a sentence of its own, because it was a coupling nobody could see: rotating the Google **SSO** secret +silently broke outbound **mail**, with nothing on either screen relating the two. Single sign-on is +otherwise unaffected — the `google` provider row exists for SSO in its own right and is untouched. + +### What you must do + +On the first boot after the upgrade, `email_config.transport` backfills to `smtp` with **no +credentials**. Until you supply some, **no mail is sent** — and nothing errors, because every sender +is written to degrade politely: + +- the contact form falls back to a `mailto:` link, +- an invite returns the accept link for you to send by hand, +- a password reset still answers its usual generic success, +- Team notifications are logged and dropped. + +That is the risk this note exists for: **email stops and nothing complains.** The admin dashboard +shows a warning while it is true, and the warning goes away by itself once a credential is saved. + +Go to **Admin → Settings → Email**, choose the posture that fits, then use **Send test** — which is +now the real verification of the whole configuration, not a formality. + +| Posture | Settings | Notes | +| --- | --- | --- | +| **A relay** — recommended | The relay's host, port `587`, *Implicit TLS* **off**, your API key as the password | Mailgun, SES, Postmark and equivalents. Reputation, DKIM and bounce handling are the relay's problem. The right answer for anything with volume | +| **Gmail over SMTP** — the shortest migration | `smtp.gmail.com`, port `587`, *Implicit TLS* **off**, your Google address as the username, and an **app password** | This is the direct replacement for the removed connect flow. It needs an [app password](https://support.google.com/accounts/answer/185833) — 2-Step Verification must be on, and your ordinary account password will not work. Google's per-day send caps apply | +| **Your own MTA** | Its host, port `25`, *Implicit TLS* **off**, username and password left blank | For an operator already running mail on the same host | + +Two fields cause most failures: + +- **Implicit TLS** is *on* only for port **465**. On port 587 leave it **off** — the connection still + upgrades to TLS, via STARTTLS. Port 587 with it on does not error; it hangs. +- **Send from** is now typed by you rather than read back from Google, so nothing guarantees the + server will accept it. It must be an address the account is allowed to send as, or the mail is + refused for SPF/DMARC reasons that look like nothing at all. Send test names this specifically when + it happens. + +### If you do nothing + +Mail stays off. Nothing breaks and no data is lost — the site, the admin panel, logins, SSO and the +shard integration all behave exactly as before. The dashboard warning stays up until you either +configure a transport or press **Clear credentials**, which is how you say "this deployment +deliberately sends no mail". + +### Data + +Nothing is dropped. `email_config` gains `transport`, `credential_enc` and `reply_to`; the old +`provider` and `refresh_token_enc` columns are kept but no longer read. Your stored Gmail refresh +token is not used again and is cleared the first time you press **Clear credentials**. -- 2.49.1