feat(teams): phase 4 — the forum access model, announcements and the operator's controls #153

wtclaude · 2026-08-18T12:27:00Z

wtclaude commented

2026-08-18 12:27:00 +00:00

What & why

Teams phase 4 — Forum 5a: the access model, announcements, and the operator's
controls. docs/website/TEAMS.md Part 5, plus §2.5's grant flow. Targets edge
alongside Module-uo#12 and docs#157.

This ships the permission model everything downstream depends on, behind a small
surface: a single announcements stream per Team, plus the two switches an operator
needs to run it.

One structural correction, and it is why this PR is not website-only. TEAMS.md
§3.1 gave the forum a CORE page and phase 3 deleted every core Team page — Teams is
a contract primitive and core does not own the word for one. The routes were
unaffected (all /player and /admin), but the participant SURFACE had no home.
Settled by the org lead the way phase 3 settled the activity feed: module-uo
declares a second place on its guild page (uo.guild.forum) and core fills it.

What is in it

The whole forum schema at once — threads, posts, the moderation ledger, and
upload attribution — including the columns only phase 5's discussion threads use.
That is §5.1's split by LAYER: phase 5 opens paths rather than migrating data.
The grant flow. Staff act anywhere, uncapped. A leader grants and revokes
ordinary access on their own Team, capped and rate-limited, and cannot revoke a
staff-issued grant. Nothing on this path writes team_members, in either
direction.
The forum's own sanitiser profile, in which img is never allowed. An author
writes a URL; core's renderer decides at read time whether it becomes a picture.
uploads mode, hardened for an uploader who is not an admin: magic-byte
sniffing, a rolling per-account quota, an attribution row per file, and a nightly
sweep.
The versioned acknowledgement, enforced server-side with the admin checkbox
bypassed, and recorded rather than merely displayed.

Three things worth a reviewer's attention

The 404, not 403. Every forum route answers 404 while the switch is off, and
404 to a caller with no access. A 403 says "this exists and you may not have it",
which advertises both a feature the operator turned off and the existence of a
private room to someone outside it.

The stored HTML never changes with the image policy. That is the property the
renderer-owned design exists to give, and it is what makes flipping the setting
back to disabled un-render every image on every existing post with no migration.
Asserted directly (acceptance 3).

A stale acknowledgement does not disable uploads. It raises a banner and
freezes every other forum setting until re-given. Silently downgrading a live
feature because a legal text changed would strand users mid-conversation.

How it was tested

npm test in server/ — 972 pass, 0 fail, including 23 new tests in
test/teamForum.test.js and 5 new route-level tests in test/teamRoutes.test.js.
npm test in client/ — 224 pass. npm run build in client/ — clean.
npm run swagger and npm run routes:manifest regenerated and committed; the
12 new routes are all annotated and in the spec.
All four of the phase's acceptance criteria are tests, named acceptance N:
1. a granted, unlinked account reads the forum, is absent from the member rows
  (asserted byte-identical across a grant), and is still refused external
  eligibility;
2. with the switch off every forum route 404s and nothing is read or written
  on the way there;
3. the stored HTML is byte-identical between disabled and remote;
4. uploads without a matching acknowledgement is refused 400 server-side.

Not yet run on the live rig — worth doing before the edge → main cutover, since
the panel is core content inside a module's page and that seam is exactly what unit
tests cannot see.

Checklist

I have read CONTRIBUTING.md.
The change builds and existing tests/checks pass locally.
I have added or updated tests/docs where it makes sense.
My commits are reasonably scoped with clear messages.

AI-assisted contributions (required)

AI tools were used. Tool(s): Claude Code. I have reviewed and understand
every change, and take responsibility for it. AI-authored commits are
marked with a Co-Authored-By trailer.

License

I agree that my contribution is licensed under this project's license
(GNU GPL v3.0 or later), and I have the right to contribute it.

## What & why Teams **phase 4 — Forum 5a**: the access model, announcements, and the operator's controls. `docs/website/TEAMS.md` Part 5, plus §2.5's grant flow. Targets `edge` alongside **Module-uo#12** and **docs#157**. This ships the permission model everything downstream depends on, behind a small surface: a single announcements stream per Team, plus the two switches an operator needs to run it. **One structural correction, and it is why this PR is not website-only.** TEAMS.md §3.1 gave the forum a CORE page and phase 3 deleted every core Team page — Teams is a contract primitive and core does not own the word for one. The routes were unaffected (all `/player` and `/admin`), but the participant SURFACE had no home. Settled by the org lead the way phase 3 settled the activity feed: `module-uo` declares a second place on its guild page (`uo.guild.forum`) and core fills it. ### What is in it - **The whole forum schema at once** — threads, posts, the moderation ledger, and upload attribution — including the columns only phase 5's discussion threads use. That is §5.1's split by LAYER: phase 5 opens paths rather than migrating data. - **The grant flow.** Staff act anywhere, uncapped. A leader grants and revokes ordinary access on their own Team, capped and rate-limited, and cannot revoke a staff-issued grant. Nothing on this path writes `team_members`, in either direction. - **The forum's own sanitiser profile, in which `img` is never allowed.** An author writes a URL; core's renderer decides at read time whether it becomes a picture. - **`uploads` mode, hardened for an uploader who is not an admin**: magic-byte sniffing, a rolling per-account quota, an attribution row per file, and a nightly sweep. - **The versioned acknowledgement**, enforced server-side with the admin checkbox bypassed, and recorded rather than merely displayed. ### Three things worth a reviewer's attention **The 404, not 403.** Every forum route answers 404 while the switch is off, and 404 to a caller with no access. A 403 says "this exists and you may not have it", which advertises both a feature the operator turned off and the existence of a private room to someone outside it. **The stored HTML never changes with the image policy.** That is the property the renderer-owned design exists to give, and it is what makes flipping the setting back to `disabled` un-render every image on every existing post with no migration. Asserted directly (acceptance 3). **A stale acknowledgement does not disable uploads.** It raises a banner and freezes every other forum setting until re-given. Silently downgrading a live feature because a legal text changed would strand users mid-conversation. ## How it was tested - `npm test` in `server/` — **972 pass, 0 fail**, including 23 new tests in `test/teamForum.test.js` and 5 new route-level tests in `test/teamRoutes.test.js`. - `npm test` in `client/` — 224 pass. `npm run build` in `client/` — clean. - `npm run swagger` and `npm run routes:manifest` regenerated and committed; the 12 new routes are all annotated and in the spec. - **All four of the phase's acceptance criteria are tests**, named `acceptance N`: 1. a granted, unlinked account reads the forum, is absent from the member rows (asserted byte-identical across a grant), and is still refused external eligibility; 2. with the switch off every forum route 404s **and nothing is read or written** on the way there; 3. the stored HTML is byte-identical between `disabled` and `remote`; 4. `uploads` without a matching acknowledgement is refused 400 server-side. Not yet run on the live rig — worth doing before the `edge` → `main` cutover, since the panel is core content inside a module's page and that seam is exactly what unit tests cannot see. ## Checklist - [x] I have read [CONTRIBUTING.md](CONTRIBUTING.md). - [x] The change builds and existing tests/checks pass locally. - [x] I have added or updated tests/docs where it makes sense. - [x] My commits are reasonably scoped with clear messages. ## AI-assisted contributions (required) - [x] AI tools were used. Tool(s): `Claude Code`. I have reviewed and understand every change, and take responsibility for it. AI-authored commits are marked with a `Co-Authored-By` trailer. ## License - [x] I agree that my contribution is licensed under this project's license (**GNU GPL v3.0 or later**), and I have the right to contribute it.

wtclaude added 6 commits 2026-08-18 12:27:02 +00:00

feat(teams): the forum schema, the operator's two switches, and the ack gate 11fd9821bf

The whole forum schema lands at once — threads, posts, the moderation ledger and
upload attribution — including the columns only phase 5's discussion threads use.
That is TEAMS.md 5.1's split BY LAYER rather than by feature: phase 5 opens paths
instead of migrating data.

Three settings keys, and only one of them is ordinary. `teams_forums_enabled` and
`teams_forum_images` are enum keys on the existing admin settings endpoint;
`teams_forum_images` also carries a server-side PRECONDITION, which is why the
three live in their own model rather than in the generic setMany() loop where a
reader would never find it.

The gate is the server's. `PUT teams_forum_images = 'uploads'` is rejected 400
unless the same request carries the acknowledgement version — the admin checkbox
is how the gate is presented, never the gate. What is stored is the TEXT VERSION,
so "which wording did they agree to" is answerable later; settings already record
updated_by/updated_at, and an activity_log row puts it in the staff audit trail.

A reworded notice makes a stored acknowledgement stale, and neither obvious answer
is right: uploads KEEP WORKING, and no other forum setting may be saved until it is
re-given. Non-destructive, and impossible to ignore.

Both reads fail closed. A DB fault reports the forum off and images disabled — a
forum that 404s for a minute is the cheap failure; a policy that is not a policy
is not.

Co-Authored-By: Claude <noreply@anthropic.com>

feat(teams): the forum's own HTML profile, and core's image renderer fb70013adf

The load-bearing decision of the whole forum design, and deliberately not how the
rest of the site works.

Core's shared sanitizer allows <img> from any host — it is tuned for rich text
from the ADMIN editor, where the author is already trusted. Handing that to
arbitrary players would make `teams_forum_images` unenforceable: every post could
hotlink in every mode and the setting would be decoration. So the forum derives
its own profile in which `img` is never an allowed tag, in any mode.

What an author writes is a URL. What decides whether it becomes a picture is this
file's renderer, at READ time. Four properties fall out: the policy cannot be
evaded, because the only code that can emit an <img> is core's; flipping the
setting back to `disabled` un-renders every image on every existing post with no
data migration, since the images were never stored; there is no author-supplied
srcset, onerror, width or style to smuggle anything through; and a blocked or dead
image degrades to the URL the author actually wrote.

Two details found while building it:

`rel` had to be ADDED to the allowed attributes to make links safer, not laxer.
The profile writes rel="noopener noreferrer nofollow" through a transform, and
sanitize-html strips any attribute not on the allowlist — including one its own
transform just added. Without the entry, every forum link shipped without noopener.

The bare-URL linkifier runs AFTER sanitising, over the sanitiser's own output and
only on text outside tags. That ordering is the security property: every text node
is HTML-escaped by then, so the matched URL is safe in both the href and the link
text. Running it first would be an injection point.

https: only, because the CSP is `img-src 'self' data: https:` — an http: image is
blocked by the browser and renders broken, which presents as "images are broken on
my forum" with nothing in any log. And the server never fetches a user-supplied
URL: that is an SSRF vector, and an allow-set is useless when the point is
arbitrary hosts.

Co-Authored-By: Claude <noreply@anthropic.com>

feat(teams): the grant flow, announcements, and the routes behind both guards e27c368234

Path 3's WRITE half. The resolver landed in phase 2; this is who may hand access
out, to whom, and what stops a leader turning a Team forum into open hosting on
the operator's site.

Two authorities, and not one authority with different reach. Staff may act on any
Team, uncapped, and may revoke anything. A leader may grant and revoke ordinary
access on their own Team, is capped at `teams_max_grants_per_team` (default 50),
is rate-limited, and may NOT revoke a staff-issued grant — which is what stops a
leader undoing a moderation decision. The issuer's role is checked at revoke time
rather than stored, so an account that has since lost its staff role stops
protecting the grants it made.

Nothing on this path writes team_members, in either direction. A grant may name any
account, including one with no linked game identity — that is the point of it — and
that account stays off the roster, out of every count, and ineligible for external
platforms.

Announcements are a degenerate thread rather than their own object, so phase 5 adds
no migration. Moderation records WHICH authority was exercised: a staff action also
writes activity_log, a leader's writes only the Team's own ledger. Merging the two
would make a guild leader locking a thread an appealable Discord sanction.

Every forum route answers 404 while the switch is off, and 404 — never 403 — to a
caller with no access: in a private room the contents and the existence are the
same secret. The grant routes deliberately answer even while the forum is OFF,
because a toggle-off revokes no grant and the access list has to stay manageable.

Under /player rather than /admin: a leader is a player, and the /admin tier gate is
requireRole('admin','editor','moderator') — putting a leader endpoint behind it
would mean widening that gate.

Co-Authored-By: Claude <noreply@anthropic.com>

feat(teams): harden the upload path for an uploader who is not an admin 4ac353684a

The existing admin upload path is already good for an admin: an 8 MB cap, a
mimetype allowlist, a random filename, an extension derived from the mimetype map
and never from originalname, and nosniff forced on serve. All of it is kept. What
it does not have is anything that assumes a hostile uploader, because until now it
has not had one.

Magic-byte sniffing, because `file.mimetype` is the client's own Content-Type
header — a player can send image/png with arbitrary bytes and land arbitrary
content under a .png. Unrecognised bytes are a rejection and never a fallback to
what the header claimed. The file is on disk before it can be sniffed, so the
rejection path removes it: a rejected upload left on disk is the same
disk-exhaustion vector reached another way.

A rolling per-account byte quota and a per-IP rate limit, because community uploads
with no ceiling is disk exhaustion on the operator's own host.

An attribution row per accepted file. Not bookkeeping: the acknowledgement is
meaningless if "who uploaded this" cannot be answered afterwards, which is exactly
what the operator has just accepted responsibility for.

A nightly sweep for soft-deleted files past retention and for never-referenced
orphans, in the same in-process shape as the activity prune. It runs whether or not
`uploads` is the current mode, and that is the point — an operator who turns
uploads off after a problem still has the files, and a sweep that switched itself
off with the setting would strand exactly the bytes they were trying to be rid of.
It works from the forum's own rows outward and never from the directory listing
inward, because UPLOAD_DIR is shared with the admin upload path.

Co-Authored-By: Claude <noreply@anthropic.com>

feat(teams): the forum panel core fills, and the operator's controls cbb7339a3a

The forum had nowhere to live. TEAMS.md 3.1 gave it a CORE page, and phase 3
deleted every core Team page — Teams is a contract primitive and core does not own
the word for one. So the forum follows the activity feed: module-uo declares a
second place on its guild page and core fills it.

TWO slots rather than one, because a slot holds one component and the first fill
wins. Stacking the feed and the forum into a single fill would take from the module
the ability to place core's two contributions separately on its own page, which is
the whole point of the module owning it.

The panel navigates by SEARCH PARAM (?thread=12) rather than by route. A thread has
to be linkable and core cannot mount a route for one — the route belongs to the
module's page — so a search param gives a shareable URL under whatever path the
module chose, with the back button intact and no core route anywhere in it. That is
why the fill is one component holding both a list view and a detail view.

Post bodies arrive already rendered by the server under the current image policy,
which is why they are set as HTML here rather than sanitised again: the body was
cleaned on write with the forum's own profile, and any <img> in it was emitted by
core's own renderer with a fixed attribute set. A client-side sanitiser would have
to strip exactly the tag core just decided to add. The published image mode is read
only to decide which composer to draw — never what renders.

The composer puts an uploaded file's URL into the body as TEXT, not as a tag. The
author never writes markup, which is what keeps the operator's policy enforceable.

The admin panel carries both settings, the always-on help text, and the
confirmation dialog with its two checkboxes and one recorded acknowledgement — plus
the three additions the org lead settled: attribution and staff removal, the
warning that disabling later does not delete existing files, and who "users"
actually means. A stale acknowledgement raises a banner and freezes the settings;
it does not turn uploads off.

Co-Authored-By: Claude <noreply@anthropic.com>

test(teams): the four acceptance criteria, and regenerate the API artifacts

PR Checks / bot-install (pull_request) Successful in 17s

Details

PR Checks / client-build (pull_request) Successful in 27s

Details

PR Checks / server-tests (pull_request) Successful in 32s

Details

57286594e7

Four tests are named "acceptance" and are Phase 4's criteria verbatim. Each names
a property the code around it can lose without any screen looking different:

1. A granted, unlinked account reads the forum, is absent from the member rows, and
   is still refused external-platform eligibility. The membership projection is
   asserted byte-identical across a grant, which is what "non-contamination" means
   in practice.
2. With the switch off every forum route 404s AND nothing is read or written on the
   way there — a guard that 404s after loading the thread is one that still bumped
   a counter.
3. The stored HTML is byte-identical between `disabled` and `remote`; only the
   rendered output differs. That is the property the renderer-owned design exists
   to give, and it is what makes flipping the policy back a no-op rather than a
   migration.
4. Selecting `uploads` without a matching acknowledgement is refused server-side,
   with the admin checkbox bypassed.

Plus the ones that are not criteria but are the same kind of claim: an author
cannot smuggle an <img> or its attributes through in any mode, http and non-image
URLs stay plain links, a leader cannot revoke a staff-issued grant, a demoted
account stops protecting the grants it made, moderation records which authority was
exercised, and a RIFF container that is not WebP is not accepted as one.

Twelve new routes in the manifest, all annotated and in the OpenAPI spec.

Co-Authored-By: Claude <noreply@anthropic.com>

whitlocktech merged commit 16e31de087 into edge

2026-08-18 14:18:11 +00:00

whitlocktech deleted branch feature/teams-phase4-forum-access

2026-08-18 14:18:11 +00:00

whitlocktech referenced this issue from a commit

2026-08-18 14:18:12 +00:00

Merge pull request 'feat(teams): phase 4 — the forum access model, announcements and the operator's controls' (#153) from feature/teams-phase4-forum-access into edge

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: RunicGateway/website#153