Lane 2: what the public pages read (#728) #752

Merged

claude-bot merged 4 commits from build/lane2-server into main 2026-09-22 16:33:11 +00:00

Conversation 0 Commits 4 Files changed 11 +838 -56

claude-bot commented 2026-09-22 16:00:26 +00:00

Collaborator

Copy link

Lane 2 of the v4.5.0 redesign (#728), the first PR of its series: the server change the public pages need, so the UI PRs that follow are UI only (build plan §7). Backend only.

The public pages are the only part of Quest Board that reads the server rather than a campaign, and until now nothing said anything about a server without a session.

No migration

Instance settings are a key/JSON store (app_settings), so "About this server" is a new key rather than a column. Worth saying plainly, because a lane that adds a settings concept usually means Alembic and this one does not.

GET /public/site — the front door

IS-384, IS-385, IS-388.

{ "server_name": "Amberwake" | null, "operator_name": "Dana" | null,
  "contact_email": "…" | null, "contact_url": null,
  "blurb": "for a few groups of friends" | null,
  "numbers": null | { "campaigns_running": 2, "sessions_played": 45, "running_since": "2026-01-14" | null } }

Every string is null rather than "" when unset, because the pages draw a different sentence for "not said yet" than for "said, and empty". numbers: null is the switch About this server (ADMIN-15, lane 9) turns off; zeros mean a server too new, which the page words itself — deciding when a number is worth saying is not the server's business.

• campaigns_running excludes soft-deleted and archived campaigns: an archived campaign is readable but is not running.
• sessions_played is completed, joined to the campaign, because a session has no deleted_at of its own and a trashed campaign's sessions would otherwise keep counting.
• running_since is min(users.created_at). Nothing records when an instance was installed; the first account is created at the first sign-in, which is the closest thing the database holds. Named as a definition rather than invented as a column.
• Counted at most once a minute. Three aggregates per hit on an unauthenticated endpoint is a small denial-of-service surface, and nobody watches these tick.

blurb is the administrator's one line about their own server. It exists because the drawing's "Amberwake is run by Dana, for a few groups of friends" had no field behind it — and the UI PR had duly hard-coded it, which would have had every self-hosted Quest Board telling visitors that its operator runs it for a few groups of friends.

GET /public/policies — privacy and licences

IS-387, IS-391, API-016, IS-388. The administrator's own words, the published notices address, and the retention settings read live rather than repeated — which is the whole point of IS-387: a privacy page that repeats its server's retention posture starts lying the day that posture changes.

API-016 already exists as GET /api/content-packs/notice-contact, behind get_current_user. It stays where it is for the admin UI; this is a second, public reading of the same setting, because D64 puts that address on a page that has to be readable without an account.

The invitation says two more things

IS-196, and the system line the drawing shows. InvitePreview gains game_system and records_sessions — the latter true only when the campaign has a Discord server connected and the instance has a bot configured, because anything looser would tell a stranger a table records when it has no way to.

Nothing else about the endpoint changes: the code is still the secret, and a dead invite is still one 404 with one sentence for all five reasons.

?next= now means what the sign-in page says it means

SH-032. The rewritten sign-in page says "Your session ran out. Sign in again and you will come back to where you were." _safe_next honoured exactly two shapes — /auth/… and an invite link — so that sentence was true for an invitation and for nothing else in the app.

It accepts any same-origin path now, and refuses everything else by rule rather than by allowlist, each rule with a test named after it: no leading /, a protocol-relative // or /\, a backslash anywhere, any control character (a newline in a Location header being the interesting one), a scheme or authority, and a .. segment.

Three cases the old tests asserted as hostile now pass, and they are worth a reviewer's eye:

• /dashboard — the point of the change.
• /join/x?redirect=https://evil.example — accepted. It resolves same-origin; the absolute URL is inside the query string, which this function treats as opaque on purpose. Its guarantee is about the destination, never about what the destination does with its own parameters — a page that followed a redirect= of its own would be just as exploitable from a link in an email.
• /join/../../admin — refused, although it cannot leave the origin. What makes it safe is dot-segment removal performed by the browser, not by us, and a non-browser client or an intermediary is not guaranteed to resolve it the same way. A path that says /join/… and lands on /admin is also a path that lies about where it goes.

The two existing branches are byte-for-byte unchanged, and a value reaching them always begins /a, so it can never be reclassified as protocol-relative.

The two Discord link pages

EX-047, EX-048. Restyled to the design system's tokens in light and dark, and reworded to the drawing. They stay a second, standalone implementation, as link_pages.py already explains: the backend renders them, outside the app, and sharing a component would mean shipping the app to draw two sentences.

One out-of-scope fix, called out rather than smuggled

tests/test_audio_tasks.py committed a campaign and a completed session through its own session — invisible to the rolled-back db fixture — and never deleted them. Nothing had ever run an unscoped count over campaigns or sessions, so the leak was silent; /public/site's numbers are exactly that query and it surfaced immediately, off by one. Fixed with the finally-cleanup its sibling tests already use, rather than by loosening the new tests to tolerate stray global state.

Checks

ruff format --check and ruff check over webapp/backend/ scripts/ webapp/frontend/e2e/ at the pinned 0.4.4: clean. Full backend suite: 3850 passed, 13 skipped.

🤖 Generated with Claude Code

Lane 2 of the v4.5.0 redesign (#728), the **first** PR of its series: the server change the public pages need, so the UI PRs that follow are UI only (build plan §7). Backend only. The public pages are the only part of Quest Board that reads the **server** rather than a campaign, and until now nothing said anything about a server without a session. ## No migration Instance settings are a key/JSON store (`app_settings`), so "About this server" is a new key rather than a column. Worth saying plainly, because a lane that adds a settings concept usually means Alembic and this one does not. ## `GET /public/site` — the front door `IS-384`, `IS-385`, `IS-388`. ```json { "server_name": "Amberwake" | null, "operator_name": "Dana" | null, "contact_email": "…" | null, "contact_url": null, "blurb": "for a few groups of friends" | null, "numbers": null | { "campaigns_running": 2, "sessions_played": 45, "running_since": "2026-01-14" | null } } ``` Every string is `null` rather than `""` when unset, because the pages draw a different sentence for "not said yet" than for "said, and empty". `numbers: null` is the switch About this server (ADMIN-15, lane 9) turns off; zeros mean a server too new, which the page words itself — deciding when a number is worth saying is not the server's business. - `campaigns_running` excludes soft-deleted **and** archived campaigns: an archived campaign is readable but is not running. - `sessions_played` is `completed`, joined to the campaign, because a session has no `deleted_at` of its own and a trashed campaign's sessions would otherwise keep counting. - `running_since` is `min(users.created_at)`. Nothing records when an instance was installed; the first account is created at the first sign-in, which is the closest thing the database holds. Named as a definition rather than invented as a column. - **Counted at most once a minute.** Three aggregates per hit on an unauthenticated endpoint is a small denial-of-service surface, and nobody watches these tick. `blurb` is the administrator's one line about their own server. It exists because the drawing's "Amberwake is run by Dana, **for a few groups of friends**" had no field behind it — and the UI PR had duly hard-coded it, which would have had every self-hosted Quest Board telling visitors that its operator runs it for a few groups of friends. ## `GET /public/policies` — privacy and licences `IS-387`, `IS-391`, `API-016`, `IS-388`. The administrator's own words, the published notices address, and **the retention settings read live** rather than repeated — which is the whole point of `IS-387`: a privacy page that repeats its server's retention posture starts lying the day that posture changes. `API-016` already exists as `GET /api/content-packs/notice-contact`, behind `get_current_user`. It stays where it is for the admin UI; this is a second, public reading of the same setting, because D64 puts that address on a page that has to be readable without an account. ## The invitation says two more things `IS-196`, and the system line the drawing shows. `InvitePreview` gains `game_system` and `records_sessions` — the latter true only when the campaign has a Discord server connected **and** the instance has a bot configured, because anything looser would tell a stranger a table records when it has no way to. Nothing else about the endpoint changes: the code is still the secret, and a dead invite is still one 404 with one sentence for all five reasons. ## `?next=` now means what the sign-in page says it means `SH-032`. The rewritten sign-in page says *"Your session ran out. Sign in again and you will come back to where you were."* `_safe_next` honoured exactly two shapes — `/auth/…` and an invite link — so that sentence was true for an invitation and for nothing else in the app. It accepts any same-origin path now, and refuses everything else **by rule rather than by allowlist**, each rule with a test named after it: no leading `/`, a protocol-relative `//` or `/\`, a backslash anywhere, any control character (a newline in a `Location` header being the interesting one), a scheme or authority, and a `..` segment. Three cases the old tests asserted as hostile now pass, and they are worth a reviewer's eye: - **`/dashboard`** — the point of the change. - **`/join/x?redirect=https://evil.example`** — accepted. It resolves same-origin; the absolute URL is inside the query string, which this function treats as opaque on purpose. Its guarantee is about the destination, never about what the destination does with its own parameters — a page that followed a `redirect=` of its own would be just as exploitable from a link in an email. - **`/join/../../admin`** — **refused**, although it cannot leave the origin. What makes it safe is dot-segment removal performed by the *browser*, not by us, and a non-browser client or an intermediary is not guaranteed to resolve it the same way. A path that says `/join/…` and lands on `/admin` is also a path that lies about where it goes. The two existing branches are byte-for-byte unchanged, and a value reaching them always begins `/a`, so it can never be reclassified as protocol-relative. ## The two Discord link pages `EX-047`, `EX-048`. Restyled to the design system's tokens in light and dark, and reworded to the drawing. They stay a second, standalone implementation, as `link_pages.py` already explains: the backend renders them, outside the app, and sharing a component would mean shipping the app to draw two sentences. ## One out-of-scope fix, called out rather than smuggled `tests/test_audio_tasks.py` committed a campaign and a completed session through its own session — invisible to the rolled-back `db` fixture — and never deleted them. Nothing had ever run an unscoped count over campaigns or sessions, so the leak was silent; `/public/site`'s numbers are exactly that query and it surfaced immediately, off by one. Fixed with the `finally`-cleanup its sibling tests already use, rather than by loosening the new tests to tolerate stray global state. ## Checks `ruff format --check` and `ruff check` over `webapp/backend/ scripts/ webapp/frontend/e2e/` at the pinned 0.4.4: clean. Full backend suite: **3850 passed, 13 skipped**. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 4 commits 2026-09-22 16:00:26 +00:00

feat(backend): the public pages' server reads for lane 2 (#728) ... d378a8fab8

Adds what the twelve public pages need from the server, so the UI PRs that
follow (build plan §7) are UI only:

- A new site_about instance setting (settings_service.SiteAbout / get/set),
  written by lane 9's Admin form later — never encrypted, since it is public
  by definition the moment it is set.
- GET /public/site (PUB-11): the front door's identity and its three
  numbers, with a 60-second in-process TTL on the counting so an
  unauthenticated endpoint cannot be used to run three aggregates per hit.
- GET /public/policies (PUB-12, PUB-6): the administrator's own words, the
  retention defaults read live (the whole point of IS-387 — the page cannot
  lie the day the settings change), and the published notice-contact
  address.
- InvitePreview gains game_system and records_sessions (PUB-3, IS-196).
  records_sessions is true only when the campaign has a connected Discord
  server *and* this instance has a bot token configured — anything looser
  would tell a stranger the table records when it has no way to.
- The two Discord link pages (PUB-9) restyled to the redesign's palette
  (tokens copied literally from webapp/frontend/src/ui/tokens.css, since
  this backend-rendered HTML has no stylesheet to import) and reworded to
  the drawing's copy, keeping their 400/409 status codes.

Also fixes a pre-existing test-isolation leak in test_audio_tasks.py: its one
test commits a real Campaign + completed Session (the task under test opens
its own task_session(), invisible to the per-test rolled-back db fixture) but
never cleaned them up, unlike its siblings in test_retention.py and
test_calendar_invites.py. Nothing before this PR ever ran an unscoped count
over campaigns/sessions, so the leak was silent; /public/site's numbers are
exactly that query, and surfaced it immediately.

(#728, #659)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

feat(backend): SiteAbout.blurb, and _safe_next accepts any same-origin path (#728) ... aa7cdc5b33

Two follow-ups from review of the lane 2 server change:

- SiteAbout gains blurb: the administrator's one-line description of their
  server (the drawing's "for a few groups of friends"), which had no field
  behind it — the UI PR being built in parallel had hard-coded that clause,
  which would have every self-hosted Quest Board claiming the same operator's
  words. Exposed on GET /public/site as "blurb", null when unset, same
  convention as the rest of the record.

- _safe_next (routers/auth.py) now accepts any same-origin path, not only
  /auth/... and /join/<code>. The redesign's sign-in page says "Sign in
  again and you will come back to where you were" (PUB-1, SH-032) — with the
  old two-shape allowlist that sentence was untrue for every other page in
  the app. Widened via a new _is_safe_relative_path: exactly one leading /,
  no //, no \ anywhere, no control character, and no scheme/authority
  anywhere in the value (checked explicitly via urlsplit, though the
  preceding checks already make it unreachable — kept as the actual
  property being asserted, not an accident of the other checks). The two
  existing allowlisted branches are untouched.

  This also flips three cases the existing tests treated as hostile into
  accepted ones, since none of them actually leaves this origin:
  /dashboard, /join/../../admin (a browser resolves the dot-segments per
  RFC 3986 before requesting it, landing on /admin — same-origin either
  way), and /join/x?redirect=https://evil.example (the embedded scheme is
  query data on our own origin, not the redirect target's). Flagged rather
  than decided quietly.

(#728, #659)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

fix(backend): _safe_next rejects a .. path segment (#728) ... 74c442b727

Coordinator ruling on the three flipped cases from the previous commit:
/dashboard and the redirect=https://... query string stay accepted (both
genuinely resolve same-origin, and the query string is data _safe_next
deliberately does not inspect), but /join/../../admin is refused.

Not because it can leave this origin — it cannot — but because what makes
that true is dot-segment removal performed by the browser (RFC 3986 §5),
not by this function, and a non-browser client or intermediary is not
guaranteed to resolve it the same way. A path that says /join/... and lands
on /admin is also a path that lies about its destination, which
_safe_next should refuse to vouch for regardless of where it actually ends
up. Segments are compared whole (split on "/", exact match on ".."), so a
filename like ..foo or x..y is not caught by accident.

(#728, #659)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docs(changelog): what the public pages read, and the two fixes under it (#728) ... 204ac5ac48

The lane's server PR had no entry. Three: the two public endpoints and the
invitation's two new fields under Added; the widened `?next=` and the restyled
Discord link pages under Fixed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot referenced this pull request 2026-09-22 16:22:10 +00:00

Lane 2: the public frame, the front door and sign in (#728) #751

claude-bot merged commit ed3b2916d7 into main 2026-09-22 16:33:11 +00:00

claude-bot referenced this pull request from a commit 2026-09-22 16:33:12 +00:00

Merge PR #752: lane 2, what the public pages read (#728)

claude-bot referenced this pull request 2026-09-22 17:37:04 +00:00

Build lane 2: the public pages (PUB-1 to PUB-12) #728

claude-bot referenced this pull request 2026-09-22 19:13:52 +00:00

Lane 2: the stops, the waits, and the four written pages (#728) #758

claude-bot referenced this pull request from a commit 2026-09-22 21:12:52 +00:00

docs(catalogue): the front door's own line is a row now — IS-438 (#728)

claude-bot referenced this pull request 2026-09-22 21:17:32 +00:00

Build lane 2: the public pages (PUB-1 to PUB-12) #728

claude-bot referenced this pull request 2026-09-22 21:42:05 +00:00

D86 to D90, and the front door's own line drawn on ADMIN-15 (#659, #727, #728) #761

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

backend

Issue related to the backend

  

bot

Issue related to the bot

  

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

frontend

Issue related to the frontend

  

help wanted

Need some help

  

invalid

Something is wrong

  

question

More information is needed

  

recording

Issue related to the recording system

  

security

Security hardening / vulnerability work

  

wontfix

This won't be fixed

No labels

backend

bot

bug

duplicate

enhancement

frontend

help wanted

invalid

question

recording

security

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

rbrooks/Quest-Board!752

No description provided.