From 11fd9821bf5dca198777f9706dc6065c3e0218fe Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 07:23:15 -0500 Subject: [PATCH 1/6] feat(teams): the forum schema, the operator's two switches, and the ack gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- server/db/schema.sql | 112 ++++++++++++++ server/src/model/settings/settings.db.js | 16 +- server/src/model/settings/settings.model.js | 12 ++ .../model/teams/teamForumSettings.model.js | 143 ++++++++++++++++++ .../src/router/v1/admin/admin.controller.js | 43 ++++++ 5 files changed, 325 insertions(+), 1 deletion(-) create mode 100644 server/src/model/teams/teamForumSettings.model.js diff --git a/server/db/schema.sql b/server/db/schema.sql index 2fd7ec4..db13dec 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -1022,6 +1022,118 @@ CREATE TABLE IF NOT EXISTS team_forum_grants ( INDEX idx_tfg_user (user_id) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +-- ── Team forums (TEAMS.md Part 5, phase 4 "5a") ──────────────────────────── +-- +-- The WHOLE forum schema lands here, in 5a, including the columns only 5b uses. +-- That is §5.1's split-by-layer: 5a ships the access model and announcements, 5b +-- enables discussion by opening paths rather than by migrating data. `type`, +-- `locked`, `pinned` and the whole post table exist from day one so that the +-- second half adds no ALTER. +-- +-- Every table here is guarded by `teams_forums_enabled` at the ROUTE level and +-- never at the data level (§5.5.1). Switching the forum off must not delete a +-- thread, revoke a grant or clear a subscription, because the operator will +-- switch it back on and expects what they had. +CREATE TABLE IF NOT EXISTS team_forum_threads ( + id INT AUTO_INCREMENT PRIMARY KEY, + team_id INT NOT NULL, + type ENUM('announcement','discussion') NOT NULL DEFAULT 'discussion', + title VARCHAR(200) NOT NULL, + created_by INT NULL, -- SET NULL: the body survives the account (§2.10) + created_username VARCHAR(32) NULL, -- snapshot, so a deleted author still reads + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + last_post_at DATETIME NULL, + post_count INT NOT NULL DEFAULT 0, + pinned TINYINT(1) NOT NULL DEFAULT 0, + locked TINYINT(1) NOT NULL DEFAULT 0, + status ENUM('visible','hidden','deleted') NOT NULL DEFAULT 'visible', + CONSTRAINT fk_tft_team FOREIGN KEY (team_id) REFERENCES teams(id) ON DELETE CASCADE, + CONSTRAINT fk_tft_user FOREIGN KEY (created_by) REFERENCES users(id) ON DELETE SET NULL, + INDEX idx_tft_team_feed (team_id, status, pinned, last_post_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- `body_html` is sanitised ON WRITE and served without re-sanitising, the same +-- contract the wiki and the CMS already follow — but through the FORUM's own +-- profile (utils/forumHtml.js), not the shared one. The shared profile allows +-- `` from any host, which would make `teams_forum_images` unenforceable: +-- every post could hotlink in every mode and the setting would be decoration. +-- No stored body ever contains an ``; core's renderer emits those at read +-- time from the URLs the author wrote (§5.5.3), which is why flipping the policy +-- back to `disabled` un-renders every image on every existing post with no +-- migration at all. +CREATE TABLE IF NOT EXISTS team_forum_posts ( + id BIGINT AUTO_INCREMENT PRIMARY KEY, + thread_id INT NOT NULL, + author_user_id INT NULL, + author_username VARCHAR(32) NULL, -- snapshot; renders as "[deleted account]" when both are gone + body_html MEDIUMTEXT NOT NULL, -- sanitised on write via utils/forumHtml.js + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + edited_at DATETIME NULL, + edited_by INT NULL, + status ENUM('visible','hidden','deleted') NOT NULL DEFAULT 'visible', + CONSTRAINT fk_tfp_thread FOREIGN KEY (thread_id) REFERENCES team_forum_threads(id) ON DELETE CASCADE, + CONSTRAINT fk_tfp_user FOREIGN KEY (author_user_id) REFERENCES users(id) ON DELETE SET NULL, + CONSTRAINT fk_tfp_editor FOREIGN KEY (edited_by) REFERENCES users(id) ON DELETE SET NULL, + INDEX idx_tfp_thread (thread_id, status, created_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- Append-only. Never updated, never deleted. +-- +-- Deliberately NOT merged into the site's mod_actions/appeals pair (§5.3), which +-- is Discord-sanction-shaped and bot-owned: routing a guild leader locking a +-- thread through it would make ordinary housekeeping an appealable sanction with +-- a reversal path into the bot. The two are cross-referenced instead — every +-- STAFF-exercised action here additionally writes an activity_log row, so the +-- site's staff-accountability trail sees it; a LEADER-exercised one writes only +-- this ledger. `actor_role` records WHICH authority was exercised, which is the +-- column that makes that distinction auditable after the fact. +CREATE TABLE IF NOT EXISTS team_forum_moderation ( + id BIGINT AUTO_INCREMENT PRIMARY KEY, + team_id INT NOT NULL, + target_type ENUM('thread','post') NOT NULL, + target_id BIGINT NOT NULL, + action ENUM('pin','unpin','lock','unlock','hide','unhide','delete','restore') NOT NULL, + actor_user_id INT NULL, + actor_username VARCHAR(32) NULL, -- snapshot (§2.10) + actor_role ENUM('leader','staff') NOT NULL, + reason VARCHAR(255) NULL, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + CONSTRAINT fk_tfm_team FOREIGN KEY (team_id) REFERENCES teams(id) ON DELETE CASCADE, + CONSTRAINT fk_tfm_actor FOREIGN KEY (actor_user_id) REFERENCES users(id) ON DELETE SET NULL, + INDEX idx_tfm_target (target_type, target_id), + INDEX idx_tfm_team (team_id, created_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- Upload attribution (§5.2a, §5.5.4). Not bookkeeping: the acknowledgement an +-- operator gives before enabling uploads is meaningless if "who uploaded this" +-- cannot be answered afterwards, and the deletion sweep needs a row to sweep. +-- +-- `post_id` is NULL between the upload and the post that embeds it — the composer +-- uploads first and references the URL in the body — and that is exactly the state +-- the orphan sweep looks for. `deleted_at` is a soft delete: the file survives a +-- retention window so a mis-click is recoverable, then the nightly sweep removes +-- the bytes. +CREATE TABLE IF NOT EXISTS team_forum_uploads ( + id BIGINT AUTO_INCREMENT PRIMARY KEY, + team_id INT NOT NULL, + post_id BIGINT NULL, + uploader_user_id INT NULL, + uploader_username VARCHAR(32) NULL, -- snapshot: attribution must survive the account + filename VARCHAR(255) NOT NULL, -- the STORED name, never originalname + mimetype VARCHAR(64) NOT NULL, -- the SNIFFED type, never the client's header + byte_size INT NOT NULL, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + deleted_at DATETIME NULL, + deleted_by INT NULL, + CONSTRAINT fk_tfu_team FOREIGN KEY (team_id) REFERENCES teams(id) ON DELETE CASCADE, + CONSTRAINT fk_tfu_post FOREIGN KEY (post_id) REFERENCES team_forum_posts(id) ON DELETE SET NULL, + CONSTRAINT fk_tfu_user FOREIGN KEY (uploader_user_id) REFERENCES users(id) ON DELETE SET NULL, + CONSTRAINT fk_tfu_deleter FOREIGN KEY (deleted_by) REFERENCES users(id) ON DELETE SET NULL, + UNIQUE KEY uq_tfu_filename (filename), + INDEX idx_tfu_uploader (uploader_user_id, created_at), + INDEX idx_tfu_sweep (deleted_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + -- The §2.9 approval queue. A MODERATOR performing one of the three actions that -- publish untrusted game-sourced strings creates a pending row here; an ADMIN -- performing one applies it immediately. Rows are kept after a decision — "a diff --git a/server/src/model/settings/settings.db.js b/server/src/model/settings/settings.db.js index 07c4547..290b6f4 100644 --- a/server/src/model/settings/settings.db.js +++ b/server/src/model/settings/settings.db.js @@ -17,6 +17,20 @@ async function set(key, value, updatedBy = null) { ) } +// One row WITH its provenance. `updated_by`/`updated_at` are already stored for +// every key; this is the only reader that needs them, because TEAMS.md §5.5.5 +// makes the uploads acknowledgement a RECORDED consent rather than a displayed +// one, and "which admin accepted it, and when" is the question that has to be +// answerable afterwards. +async function getRow(key) { + const rows = await query( + 'SELECT s.`key`, s.value, s.updated_by, s.updated_at, u.username AS updated_by_username ' + + 'FROM settings s LEFT JOIN users u ON u.id = s.updated_by WHERE s.`key` = ? LIMIT 1', + [key], + ) + return rows[0] || null +} + // Insert a default only if the key does not already exist. async function seedDefault(key, value) { await query('INSERT IGNORE INTO settings (`key`, value) VALUES (?, ?)', [key, value]) @@ -30,4 +44,4 @@ async function remove(key) { await query('DELETE FROM settings WHERE `key` = ?', [key]) } -module.exports = { getAll, get, set, seedDefault, remove } +module.exports = { getAll, get, getRow, set, seedDefault, remove } diff --git a/server/src/model/settings/settings.model.js b/server/src/model/settings/settings.model.js index 53f10a9..ded35a8 100644 --- a/server/src/model/settings/settings.model.js +++ b/server/src/model/settings/settings.model.js @@ -16,6 +16,18 @@ const PUBLIC_KEYS = [ 'theme_visual', // preset/custom colors, fonts, radii (JSON). See THEMING_AND_NAV.md §6.1. 'brand_assets', // uploaded logo/hero/favicon overrides (JSON). §6.3. 'nav_public', // public site nav overrides (JSON). §6.4. + // The two Team-forum controls (TEAMS.md §5.5.6). The client needs the first to + // know whether to render the forum panel at all, and the second to decide which + // composer to show — an upload control that 404s is worse than no control. + // Neither is sensitive. + // + // `teams_forum_uploads_ack` is deliberately NOT here: who accepted a liability + // notice is operator detail, exactly as `failure_reason` is in MODULE_API.md + // §2.9. And publishing the mode does not move the DECISION client-side — the + // server still resolves what renders (§5.5.3); the client is only told which + // composer to draw. + 'teams_forums_enabled', + 'teams_forum_images', ] // Admin-configurable theming & navigation (docs/website/THEMING_AND_NAV.md). diff --git a/server/src/model/teams/teamForumSettings.model.js b/server/src/model/teams/teamForumSettings.model.js new file mode 100644 index 0000000..b3d69dc --- /dev/null +++ b/server/src/model/teams/teamForumSettings.model.js @@ -0,0 +1,143 @@ +// ── The operator's two forum controls, and the acknowledgement gate ──────── +// +// TEAMS.md §5.5. Three `settings` keys, and the reason they live in their own +// file rather than in settings.model.js is that only one of them is an ordinary +// key: `teams_forum_images` has a server-side precondition, and a precondition +// buried in the generic setMany() loop is one nobody reading that loop would +// know about. +// +// teams_forums_enabled '0' | '1' default '0' — off +// teams_forum_images 'disabled' | 'remote' | 'uploads' default 'disabled' +// teams_forum_uploads_ack the acknowledged TEXT VERSION absent until given +// +// **Both reads fail closed.** A DB fault reports the forum off and images +// disabled, because the alternative is a transient error opening a feature the +// operator turned off, or rendering third-party images on a site whose operator +// chose not to. The cost of failing closed here is a forum that 404s for a minute; +// the cost of failing open is a policy that is not a policy. + +const settingsDb = require('../settings/settings.db') + +const ENABLED_KEY = 'teams_forums_enabled' +const IMAGES_KEY = 'teams_forum_images' +const ACK_KEY = 'teams_forum_uploads_ack' + +const IMAGE_MODES = ['disabled', 'remote', 'uploads'] + +// The version of the §5.5.5 warning text currently in force. Bumping this is what +// makes every stored acknowledgement stale — see `ackState` below for what that +// then does, which is deliberately NOT "turn uploads off". +const ACK_VERSION = '1' + +/** Is the forum switched on? Fail closed. */ +async function forumsEnabled() { + try { + return String(await settingsDb.get(ENABLED_KEY)) === '1' + } catch { + return false + } +} + +/** + * The image policy. Fail closed, and coerce any unexpected stored value back to + * 'disabled' — a hand-edited row must not be able to widen the policy by being + * unreadable. + */ +async function imageMode() { + try { + const value = await settingsDb.get(IMAGES_KEY) + return IMAGE_MODES.includes(value) ? value : 'disabled' + } catch { + return 'disabled' + } +} + +/** Are uploads accepted? The one mode where files come to rest on the operator's disk. */ +async function uploadsEnabled() { + return (await imageMode()) === 'uploads' +} + +/** + * The acknowledgement's state, for the admin surface. + * + * `stale` is the case §5.5.5 spends its longest paragraph on: the text was + * reworded after an operator accepted it. Neither obvious answer is right — + * silently downgrading a live feature because a legal text changed strands users + * mid-conversation, and honouring an old acceptance forever defeats versioning. + * So uploads keep working, `stale` drives a persistent banner, and + * `assertSettingsWritable` below refuses every other forum setting until it is + * re-given. Non-destructive, and impossible to ignore. + */ +async function ackState() { + const stored = await settingsDb.get(ACK_KEY) + const row = await settingsDb.getRow(ACK_KEY) + return { + version: ACK_VERSION, + acknowledgedVersion: stored ?? null, + given: stored != null, + stale: stored != null && String(stored) !== ACK_VERSION, + ...(row ? { acknowledgedBy: row.updated_by_username ?? null, acknowledgedAt: row.updated_at } : {}), + } +} + +/** + * The gate. `PUT teams_forum_images = 'uploads'` is rejected 400 unless the SAME + * request carries `acknowledge: `. + * + * The checkbox in the admin UI is not the gate — it is how the gate is presented. + * That distinction is the whole reason this function exists on the server: an + * acknowledgement a client could skip is not an acknowledgement. + * + * Returns `{ ok }` or `{ ok: false, error, status }`, matching the model result + * shape the Teams controllers already translate. + */ +function assertAcknowledged(nextMode, acknowledge) { + if (nextMode !== 'uploads') return { ok: true } + if (String(acknowledge ?? '') !== ACK_VERSION) { + return { + ok: false, + status: 400, + error: `Enabling uploads requires acknowledging the current notice (version ${ACK_VERSION}).`, + } + } + return { ok: true } +} + +/** + * The stale-acknowledgement lock: while an acknowledgement is stale, NO forum + * setting may be saved until it is re-given. Not "uploads are disabled" — see + * `ackState`. The re-acknowledgement itself is exempt, or the lock would have no + * key. + */ +async function assertSettingsWritable(keys, acknowledge) { + const touchesForum = keys.some((k) => k === ENABLED_KEY || k === IMAGES_KEY) + if (!touchesForum) return { ok: true } + const state = await ackState() + if (!state.stale) return { ok: true } + if (String(acknowledge ?? '') === ACK_VERSION) return { ok: true } + return { + ok: false, + status: 400, + error: 'The image-upload notice has changed. Re-acknowledge it before saving forum settings.', + } +} + +/** Record the acknowledgement. `updated_by`/`updated_at` come free from the settings schema. */ +async function recordAck(adminUserId) { + await settingsDb.set(ACK_KEY, ACK_VERSION, adminUserId) +} + +module.exports = { + ENABLED_KEY, + IMAGES_KEY, + ACK_KEY, + IMAGE_MODES, + ACK_VERSION, + forumsEnabled, + imageMode, + uploadsEnabled, + ackState, + assertAcknowledged, + assertSettingsWritable, + recordAck, +} diff --git a/server/src/router/v1/admin/admin.controller.js b/server/src/router/v1/admin/admin.controller.js index b1da58d..4c6602e 100644 --- a/server/src/router/v1/admin/admin.controller.js +++ b/server/src/router/v1/admin/admin.controller.js @@ -9,6 +9,7 @@ const trustedDevices = require('../../../model/trustedDevices/trustedDevices.mod const recoveryCodes = require('../../../model/recoveryCodes/recoveryCodes.model') const registries = require('../../../modules/registries') const announceJobs = require('../../../model/announceJobs/announceJobs.model') +const forumSettings = require('../../../model/teams/teamForumSettings.model') const pushDispatch = require('../../../utils/pushDispatch') const { cleanBody } = require('../../../utils/sanitizeHtml') const { parseJsonSetting } = require('../../../utils/settingsJson') @@ -589,8 +590,50 @@ async function updateSettings(req, res) { if (!check.ok) return res.status(400).json({ message: check.message }) updates[key] = JSON.stringify(resolveNavOverrides(parsed, key)) } + // The Team-forum controls (TEAMS.md §5.5). Two enum keys and one PRECONDITION — + // the only key on this endpoint whose write depends on something other than its + // own value. `acknowledge` is a request field, not a setting: it is consumed + // here and never stored, because what gets stored is the text VERSION the + // operator accepted, written by recordAck() below. + if (forumSettings.ENABLED_KEY in updates) { + const v = updates[forumSettings.ENABLED_KEY] + if (v !== '0' && v !== '1' && v !== true && v !== false) { + return res.status(400).json({ message: 'Invalid teams_forums_enabled value' }) + } + updates[forumSettings.ENABLED_KEY] = v === true || v === '1' ? '1' : '0' + } + const nextImageMode = updates[forumSettings.IMAGES_KEY] + if (forumSettings.IMAGES_KEY in updates) { + if (!forumSettings.IMAGE_MODES.includes(nextImageMode)) { + return res.status(400).json({ message: 'Invalid teams_forum_images value' }) + } + // THE GATE (§5.5.5). Server-side, and rejected 400 with the admin UI's + // checkbox bypassed — a checkbox is how the gate is presented, never the gate. + const gate = forumSettings.assertAcknowledged(nextImageMode, req.body.acknowledge) + if (!gate.ok) return res.status(gate.status).json({ message: gate.error }) + } + { + // The stale-acknowledgement lock: a reworded notice freezes the forum + // settings until it is re-given, and does NOT turn uploads off (§5.5.5). + const writable = await forumSettings.assertSettingsWritable(Object.keys(updates), req.body.acknowledge) + if (!writable.ok) return res.status(writable.status).json({ message: writable.error }) + } + const acknowledging = String(req.body.acknowledge ?? '') === forumSettings.ACK_VERSION + delete updates.acknowledge try { await settings.setMany(updates, req.user.id) + if (acknowledging && (nextImageMode === 'uploads' || forumSettings.IMAGES_KEY in updates)) { + // Recorded, not merely displayed: `updated_by`/`updated_at` come from the + // settings schema, and the activity_log row puts it in the staff audit trail + // with the acting admin's IP alongside every other consequential action. + await forumSettings.recordAck(req.user.id) + await activity.log({ + req, + action: 'team.forum.uploads.acknowledged', + detail: `${req.user.username} (#${req.user.id}) acknowledged the image-upload notice ` + + `(version ${forumSettings.ACK_VERSION})`, + }) + } // The HTML shell is templated from brand_assets and theme_visual, and is // cached per process (utils/htmlShell.js) — a write that can change it has // to say so, or the favicon an admin just uploaded appears only after the -- 2.49.1 From fb70013adfb43208a2863f6e042090cc3ba1e5b9 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 07:23:32 -0500 Subject: [PATCH 2/6] feat(teams): the forum's own HTML profile, and core's image renderer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The load-bearing decision of the whole forum design, and deliberately not how the rest of the site works. Core's shared sanitizer allows 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 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 --- server/src/utils/forumHtml.js | 193 ++++++++++++++++++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 server/src/utils/forumHtml.js diff --git a/server/src/utils/forumHtml.js b/server/src/utils/forumHtml.js new file mode 100644 index 0000000..d6e0b4c --- /dev/null +++ b/server/src/utils/forumHtml.js @@ -0,0 +1,193 @@ +// ── The forum's own HTML profile, and core's image renderer ──────────────── +// +// TEAMS.md §5.5.3, which is the load-bearing decision of the whole forum design +// and is deliberately NOT how the rest of the site works. +// +// **The author never writes an `` tag.** Core's shared sanitizer +// (utils/sanitizeHtml.js) allows `` from any http/https host — it is tuned +// for rich text from the ADMIN editor, where the author is already trusted. +// Handing that profile 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: +// +// author types: https://example.com/banner.png +// stored HTML: https://… +// rendered: that link, and — in `remote`/`uploads` mode only — a +// core-generated beneath it +// +// Five properties fall out, and they are the reason for the design: +// +// 1. The policy is ENFORCEABLE, because the only code that can emit an +// is this file. +// 2. Flipping the setting back to `disabled` retroactively un-renders every +// image on every existing post, with NO data migration — the images were +// never in the stored HTML. +// 3. No attribute smuggling: no author-supplied srcset, onerror, width=99999 +// or style. Core emits a fixed attribute set. +// 4. The link always survives. A blocked, dead or 404ing image degrades to the +// URL the author actually wrote, which is what the reader wanted anyway. +// 5. It matches how forums conventionally behave. +// +// **Never proxy or cache a remote image server-side.** The moment the server +// fetches a user-supplied URL it is an SSRF vector, and an allow-set is useless +// here because the whole point is arbitrary hosts. The browser fetches; the +// server never does. Written down so nobody adds a proxy "for performance". + +const sanitizeHtml = require('sanitize-html') + +// Derived from the shared profile with the image family removed. `figure` and +// `figcaption` go with `img` rather than surviving it: without an image inside, +// a figure is an empty box, and leaving them would let an author build a caption +// for a picture core decided not to render. +const FORUM_OPTIONS = { + allowedTags: [ + 'h3', 'h4', 'h5', 'h6', + 'p', 'br', 'hr', 'blockquote', 'pre', 'code', + 'ul', 'ol', 'li', + 'strong', 'b', 'em', 'i', 'u', 's', 'sup', 'sub', 'mark', 'span', + 'a', + 'table', 'thead', 'tbody', 'tr', 'th', 'td', + ], + allowedAttributes: { + // `rel` is allowed only so the transform below can WRITE it — an author's own + // rel is overwritten, not merged. Without it here, sanitize-html strips the + // very attribute the transform just added and every link ships without + // noopener. + a: ['href', 'title', 'rel'], + th: ['colspan', 'rowspan'], + td: ['colspan', 'rowspan'], + }, + // No `style` at all, and therefore no allowedStyles. The shared profile permits + // text-align for the admin editor's block alignment; a forum post has no such + // editor and every style attribute a player could send is one more thing to + // reason about. + allowedSchemes: ['http', 'https', 'mailto'], + allowProtocolRelative: false, + transformTags: { + a: sanitizeHtml.simpleTransform('a', { rel: 'noopener noreferrer nofollow' }, true), + }, + disallowedTagsMode: 'discard', +} + +// What may become a picture. Conservative on purpose: guessing wrong renders an +// pointed at something that is not an image, which reads as a broken site. +const IMAGE_EXTENSIONS = ['.png', '.jpg', '.jpeg', '.gif', '.webp', '.avif'] + +// Tags whose text is left alone by the linkifier. Inside an anchor because +// nesting one is invalid; inside code/pre because a URL in a code sample is +// being shown, not offered. +const NO_LINKIFY = new Set(['a', 'code', 'pre']) + +const BARE_URL = /\bhttps?:\/\/[^\s<>"']+/g + +/** + * Sanitise a forum post body. Runs on WRITE; the stored value is already safe and + * is served without re-sanitising — the same contract the wiki and the CMS follow. + */ +function cleanForumBody(html) { + if (html == null || html === '') return html + return linkify(sanitizeHtml(String(html), FORUM_OPTIONS)) +} + +/** + * Turn bare URLs in text into anchors. + * + * Runs AFTER sanitising, over the sanitiser's own output, and only on text + * outside tags. That ordering is what makes it safe: every text node has already + * been HTML-escaped, so the matched URL can go into both the href and the link + * text unchanged — `&` is already `&`, which is what an attribute wants. + */ +function linkify(html) { + const tokens = String(html).split(/(<[^>]+>)/) + const openStack = [] + return tokens + .map((token) => { + if (token.startsWith('<')) { + const match = /^<\s*(\/?)\s*([a-zA-Z0-9]+)/.exec(token) + if (match) { + const [, closing, name] = match + const tag = name.toLowerCase() + if (closing) { + const at = openStack.lastIndexOf(tag) + if (at !== -1) openStack.splice(at, 1) + } else if (!token.endsWith('/>')) { + openStack.push(tag) + } + } + return token + } + if (openStack.some((tag) => NO_LINKIFY.has(tag))) return token + return token.replace(BARE_URL, (url) => { + // Trailing punctuation is far more likely to be the sentence's than the + // URL's — "see https://example.com." should not link the full stop. + const trimmed = url.replace(/[.,;:!?)\]]+$/, '') + const tail = url.slice(trimmed.length) + return `${trimmed}${tail}` + }) + }) + .join('') +} + +/** + * May this URL become a picture? + * + * `https:` only, because the CSP is `img-src 'self' data: https:` (config/csp.js) + * — an `http:` image is blocked by the browser and renders as a broken picture, + * so an `http:` URL stays a plain link. This is a real mismatch with the SHARED + * sanitizer, which permits `http` for `img`, and it is exactly the sort of thing + * that presents as "images are broken on my forum" with nothing in any log. + * + * Same-origin `/uploads/…` paths are embeddable too — that is where `uploads` + * mode puts a file, and `'self'` covers them under the same CSP. + */ +function isEmbeddableImageUrl(href) { + if (typeof href !== 'string' || href === '') return false + const decoded = href.replace(/&/g, '&') + let pathname + if (decoded.startsWith('/uploads/')) { + pathname = decoded.split(/[?#]/)[0] + } else { + let url + try { + url = new URL(decoded) + } catch { + return false + } + if (url.protocol !== 'https:') return false + pathname = url.pathname + } + const lower = pathname.toLowerCase() + return IMAGE_EXTENSIONS.some((ext) => lower.endsWith(ext)) +} + +/** + * Render a stored body for one viewer under one image policy. + * + * `disabled` returns the stored HTML byte-for-byte. The other two append a core- + * generated after each anchor whose href looks like an image — which is why + * the stored HTML is identical between the three modes, the property this whole + * design exists to give. + */ +function renderForumBody(storedHtml, mode) { + if (storedHtml == null || storedHtml === '') return storedHtml + if (mode !== 'remote' && mode !== 'uploads') return storedHtml + return String(storedHtml).replace(/]*href="([^"]*)"[^>]*>.*?<\/a>/gi, (anchor, href) => { + if (!isEmbeddableImageUrl(href)) return anchor + // A fixed attribute set, every time. `no-referrer` limits what leaks to the + // third-party host — it cannot prevent the request itself, which is the + // privacy cost stated in the admin help text rather than hidden. + return `${anchor}` + }) +} + +module.exports = { + cleanForumBody, + renderForumBody, + isEmbeddableImageUrl, + IMAGE_EXTENSIONS, + FORUM_OPTIONS, +} -- 2.49.1 From e27c3682342dcc1f95808b352db15106de1b8406 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 07:23:47 -0500 Subject: [PATCH 3/6] feat(teams): the grant flow, announcements, and the routes behind both guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- server/src/model/teams/teamAccess.db.js | 46 +++ server/src/model/teams/teamForum.db.js | 236 ++++++++++++++ server/src/model/teams/teamForum.model.js | 193 ++++++++++++ server/src/model/teams/teamGrants.model.js | 165 ++++++++++ .../src/router/v1/admin/teams.controller.js | 66 ++++ server/src/router/v1/admin/teams.router.js | 45 +++ server/src/router/v1/player/index.js | 5 + .../router/v1/player/teamForum.controller.js | 288 ++++++++++++++++++ .../src/router/v1/player/teamForum.router.js | 198 ++++++++++++ 9 files changed, 1242 insertions(+) create mode 100644 server/src/model/teams/teamForum.db.js create mode 100644 server/src/model/teams/teamForum.model.js create mode 100644 server/src/model/teams/teamGrants.model.js create mode 100644 server/src/router/v1/player/teamForum.controller.js create mode 100644 server/src/router/v1/player/teamForum.router.js diff --git a/server/src/model/teams/teamAccess.db.js b/server/src/model/teams/teamAccess.db.js index e21fe9a..6ae8496 100644 --- a/server/src/model/teams/teamAccess.db.js +++ b/server/src/model/teams/teamAccess.db.js @@ -41,6 +41,49 @@ async function activeGrants(teamId) { ) } +/** How many active grants a team currently holds — the §2.5 per-Team cap reads this. */ +async function activeGrantCount(teamId) { + const rows = await query( + 'SELECT COUNT(*) AS n FROM team_forum_grants WHERE team_id = ? AND revoked_at IS NULL', + [teamId], + ) + return Number(rows[0]?.n || 0) +} + +/** + * Issue a grant. + * + * Writes nothing but this table — that is the non-contamination invariant, and it + * is a property of this function being the ONLY writer on the grant path rather + * than of anyone remembering it at the call site. The username snapshots are + * taken here so the ledger still reads after either account is deleted (§2.10). + */ +async function insertGrant({ teamId, userId, username, grantedBy, grantedUsername, reason }) { + const res = await query( + `INSERT INTO team_forum_grants (team_id, user_id, username, granted_by, granted_username, reason) + VALUES (?, ?, ?, ?, ?, ?)`, + [teamId, userId, username, grantedBy, grantedUsername, reason ?? null], + ) + return res.insertId +} + +/** + * Revoke the active grant, if there is one. + * + * An UPDATE of the existing row rather than a delete: the table is a ledger as + * well as the current state, and `revoked_at` is what moves a row out of the + * unique key (the generated `active_marker` goes NULL) while keeping the history. + */ +async function revokeGrant({ teamId, userId, revokedBy, revokedUsername, reason }) { + const res = await query( + `UPDATE team_forum_grants + SET revoked_at = NOW(), revoked_by = ?, revoked_username = ?, revoke_reason = ? + WHERE team_id = ? AND user_id = ? AND revoked_at IS NULL`, + [revokedBy, revokedUsername, reason ?? null, teamId, userId], + ) + return res.affectedRows > 0 +} + // ── team_leader_overrides (§2.5.1) ───────────────────────────────────────── const OVERRIDE_COLUMNS = 'team_id, member_key, effect, actor_user_id, actor_username, reason, created_at' @@ -87,6 +130,9 @@ module.exports = { activeGrant, grantLedger, activeGrants, + activeGrantCount, + insertGrant, + revokeGrant, overridesForTeam, overrideFor, setOverride, diff --git a/server/src/model/teams/teamForum.db.js b/server/src/model/teams/teamForum.db.js new file mode 100644 index 0000000..0647f30 --- /dev/null +++ b/server/src/model/teams/teamForum.db.js @@ -0,0 +1,236 @@ +// SQL for the four forum tables (TEAMS.md §5.2, §5.2a). +// +// Kept apart from teamAccess.db.js for the same reason that file is kept apart +// from teams.db.js: forum CONTENT and forum ACCESS are different questions, and a +// query here that read `team_members` to decide who may see a thread would be the +// exact collapse §2.5 forbids. Nothing in this file resolves access; callers hand +// it a decision the resolver already made. + +const { query } = require('../../utils/db') + +const THREAD_COLUMNS = ` + id, team_id, type, title, created_by, created_username, created_at, + last_post_at, post_count, pinned, locked, status` + +const POST_COLUMNS = ` + id, thread_id, author_user_id, author_username, body_html, created_at, + edited_at, edited_by, status` + +// ── threads ──────────────────────────────────────────────────────────────── + +/** + * A Team's threads, newest activity first with pinned rows on top. + * + * `includeHidden` is the staff/leader view. Hidden is not deleted: a hidden + * thread stays in the ledger and comes back with `unhide`, which is why the + * status filter is a parameter rather than a WHERE clause everyone remembers. + */ +async function threadsByTeam(teamId, { includeHidden = false, limit = 50, offset = 0 } = {}) { + const statuses = includeHidden ? "('visible','hidden')" : "('visible')" + return query( + `SELECT ${THREAD_COLUMNS} FROM team_forum_threads + WHERE team_id = ? AND status IN ${statuses} + ORDER BY pinned DESC, COALESCE(last_post_at, created_at) DESC, id DESC + LIMIT ? OFFSET ?`, + [teamId, limit, offset], + ) +} + +async function threadById(id) { + const rows = await query(`SELECT ${THREAD_COLUMNS} FROM team_forum_threads WHERE id = ? LIMIT 1`, [id]) + return rows[0] || null +} + +async function insertThread({ teamId, type, title, createdBy, createdUsername }) { + const res = await query( + `INSERT INTO team_forum_threads (team_id, type, title, created_by, created_username, last_post_at, post_count) + VALUES (?, ?, ?, ?, ?, NOW(), 0)`, + [teamId, type, title, createdBy, createdUsername], + ) + return res.insertId +} + +/** Apply one moderation action's effect. The LEDGER row is written separately. */ +async function setThreadFlags(id, { pinned, locked, status }) { + const sets = [] + const args = [] + if (pinned !== undefined) { sets.push('pinned = ?'); args.push(pinned ? 1 : 0) } + if (locked !== undefined) { sets.push('locked = ?'); args.push(locked ? 1 : 0) } + if (status !== undefined) { sets.push('status = ?'); args.push(status) } + if (!sets.length) return false + args.push(id) + const res = await query(`UPDATE team_forum_threads SET ${sets.join(', ')} WHERE id = ?`, args) + return res.affectedRows > 0 +} + +// ── posts ────────────────────────────────────────────────────────────────── + +async function postsByThread(threadId, { includeHidden = false } = {}) { + const statuses = includeHidden ? "('visible','hidden')" : "('visible')" + return query( + `SELECT ${POST_COLUMNS} FROM team_forum_posts + WHERE thread_id = ? AND status IN ${statuses} ORDER BY created_at, id`, + [threadId], + ) +} + +async function postById(id) { + const rows = await query(`SELECT ${POST_COLUMNS} FROM team_forum_posts WHERE id = ? LIMIT 1`, [id]) + return rows[0] || null +} + +/** + * Append a post and move the thread's counters in the same breath. + * + * Two statements rather than a trigger: the counters are a denormalisation for + * the thread list, and a trigger would put half the write in the schema where + * nobody reading this file would find it. + */ +async function insertPost({ threadId, authorUserId, authorUsername, bodyHtml }) { + const res = await query( + `INSERT INTO team_forum_posts (thread_id, author_user_id, author_username, body_html) + VALUES (?, ?, ?, ?)`, + [threadId, authorUserId, authorUsername, bodyHtml], + ) + await query( + 'UPDATE team_forum_threads SET post_count = post_count + 1, last_post_at = NOW() WHERE id = ?', + [threadId], + ) + return res.insertId +} + +async function setPostStatus(id, status) { + const res = await query('UPDATE team_forum_posts SET status = ? WHERE id = ?', [status, id]) + return res.affectedRows > 0 +} + +// ── the moderation ledger (append-only) ──────────────────────────────────── + +async function insertModeration({ teamId, targetType, targetId, action, actorUserId, actorUsername, actorRole, reason }) { + await query( + `INSERT INTO team_forum_moderation + (team_id, target_type, target_id, action, actor_user_id, actor_username, actor_role, reason) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + [teamId, targetType, targetId, action, actorUserId, actorUsername, actorRole, reason ?? null], + ) +} + +async function moderationForTeam(teamId, { limit = 100, offset = 0 } = {}) { + return query( + `SELECT id, team_id, target_type, target_id, action, actor_user_id, actor_username, + actor_role, reason, created_at + FROM team_forum_moderation WHERE team_id = ? + ORDER BY created_at DESC, id DESC LIMIT ? OFFSET ?`, + [teamId, limit, offset], + ) +} + +// ── uploads (§5.2a) ──────────────────────────────────────────────────────── + +const UPLOAD_COLUMNS = ` + id, team_id, post_id, uploader_user_id, uploader_username, filename, mimetype, + byte_size, created_at, deleted_at, deleted_by` + +async function insertUpload({ teamId, postId, uploaderUserId, uploaderUsername, filename, mimetype, byteSize }) { + const res = await query( + `INSERT INTO team_forum_uploads + (team_id, post_id, uploader_user_id, uploader_username, filename, mimetype, byte_size) + VALUES (?, ?, ?, ?, ?, ?, ?)`, + [teamId, postId ?? null, uploaderUserId, uploaderUsername, filename, mimetype, byteSize], + ) + return res.insertId +} + +async function uploadById(id) { + const rows = await query(`SELECT ${UPLOAD_COLUMNS} FROM team_forum_uploads WHERE id = ? LIMIT 1`, [id]) + return rows[0] || null +} + +/** Bytes this account has uploaded in the trailing window — the §5.5.4 daily quota. */ +async function bytesUploadedSince(userId, sinceHours) { + const rows = await query( + `SELECT COALESCE(SUM(byte_size), 0) AS bytes FROM team_forum_uploads + WHERE uploader_user_id = ? AND created_at > (NOW() - INTERVAL ? HOUR)`, + [userId, sinceHours], + ) + return Number(rows[0]?.bytes || 0) +} + +/** The admin attribution view: who uploaded what, when, how much, and where. */ +async function listUploads({ limit = 100, offset = 0, includeDeleted = false } = {}) { + return query( + `SELECT u.id, u.team_id, u.post_id, u.uploader_user_id, u.uploader_username, + u.filename, u.mimetype, u.byte_size, u.created_at, u.deleted_at, u.deleted_by, + t.name AS team_name, t.slug AS team_slug + FROM team_forum_uploads u JOIN teams t ON t.id = u.team_id + ${includeDeleted ? '' : 'WHERE u.deleted_at IS NULL'} + ORDER BY u.created_at DESC, u.id DESC LIMIT ? OFFSET ?`, + [limit, offset], + ) +} + +async function softDeleteUpload(id, deletedBy) { + const res = await query( + 'UPDATE team_forum_uploads SET deleted_at = NOW(), deleted_by = ? WHERE id = ? AND deleted_at IS NULL', + [deletedBy, id], + ) + return res.affectedRows > 0 +} + +/** Soft-delete every upload attached to a post — the lifecycle half of §5.5.4. */ +async function softDeleteUploadsForPost(postId, deletedBy) { + await query( + 'UPDATE team_forum_uploads SET deleted_at = NOW(), deleted_by = ? WHERE post_id = ? AND deleted_at IS NULL', + [deletedBy, postId], + ) +} + +/** Rows soft-deleted longer ago than the retention window — the sweep's worklist. */ +async function sweepableUploads(retentionDays) { + return query( + `SELECT id, filename FROM team_forum_uploads + WHERE deleted_at IS NOT NULL AND deleted_at < (NOW() - INTERVAL ? DAY)`, + [retentionDays], + ) +} + +/** Never-referenced uploads older than the grace period — a composer opened and abandoned. */ +async function orphanedUploads(graceHours) { + return query( + `SELECT id, filename FROM team_forum_uploads + WHERE post_id IS NULL AND deleted_at IS NULL AND created_at < (NOW() - INTERVAL ? HOUR)`, + [graceHours], + ) +} + +async function deleteUploadRows(ids) { + if (!ids.length) return 0 + const res = await query( + `DELETE FROM team_forum_uploads WHERE id IN (${ids.map(() => '?').join(',')})`, + ids, + ) + return res.affectedRows +} + + +module.exports = { + threadsByTeam, + threadById, + insertThread, + setThreadFlags, + postsByThread, + postById, + insertPost, + setPostStatus, + insertModeration, + moderationForTeam, + insertUpload, + uploadById, + bytesUploadedSince, + listUploads, + softDeleteUpload, + softDeleteUploadsForPost, + sweepableUploads, + orphanedUploads, + deleteUploadRows, +} diff --git a/server/src/model/teams/teamForum.model.js b/server/src/model/teams/teamForum.model.js new file mode 100644 index 0000000..2f8316f --- /dev/null +++ b/server/src/model/teams/teamForum.model.js @@ -0,0 +1,193 @@ +// ── The forum, phase 4 ("5a": access + announcements) ────────────────────── +// +// TEAMS.md §5.1's split is BY LAYER, not by feature: 5a ships the whole access +// model and a single announcements stream per Team; 5b opens discussion threads, +// replies and editing. The schema for all of it landed together, so 5b enables +// paths here rather than migrating data — which is why `type` is a parameter +// below and not a constant, and why `locked` is honoured on a thread nothing can +// reply to yet. +// +// **Every function here takes an already-resolved access decision.** Nothing in +// this file reads `team_members` or `team_forum_grants`; the caller asks +// teamAccess.forumAccess() once and hands the answer down. That is §5.4's "never +// by checking membership directly, which is how paths 1 and 3 would drift back +// together", made structural. +// +// **The read path is where the image policy is applied**, once, in `renderPost`. +// Not in the controller and never in the client: the client is TOLD the mode so it +// can draw the right composer, and is never the thing that decides whether an +// image appears (§5.5.6). + +const forumDb = require('./teamForum.db') +const forumSettings = require('./teamForumSettings.model') +const { cleanForumBody, renderForumBody } = require('../../utils/forumHtml') + +// Announcements are leader-authored and replies are disabled; 5b's discussion +// threads are member-authored and take replies. Both types exist in the enum from +// day one — this is the list of what 5a will CREATE. +const CREATABLE_TYPES_5A = ['announcement'] + +const DELETED_AUTHOR = '[deleted account]' + +/** + * Moderation actions, and what each one does to the row. + * + * A table rather than a switch because the ledger and the effect have to stay in + * step: every entry here writes one row of `team_forum_moderation` naming the + * authority that was exercised, and an action with an effect but no ledger entry + * would be a moderation nobody can audit. + */ +const THREAD_ACTIONS = { + pin: { pinned: true }, + unpin: { pinned: false }, + lock: { locked: true }, + unlock: { locked: false }, + hide: { status: 'hidden' }, + unhide: { status: 'visible' }, + delete: { status: 'deleted' }, + restore: { status: 'visible' }, +} + +function publicThread(row) { + return { + id: row.id, + type: row.type, + title: row.title, + author: row.created_username || DELETED_AUTHOR, + authorDeleted: row.created_by == null, + createdAt: row.created_at, + lastPostAt: row.last_post_at, + postCount: row.post_count, + pinned: Boolean(row.pinned), + locked: Boolean(row.locked), + status: row.status, + } +} + +/** + * One post, rendered for one image policy. + * + * `body` is what the reader gets and `mode` decides whether it carries images. + * The STORED html is never modified — flipping the policy changes this function's + * output and nothing on disk, which is the property §5.5.3 exists to give and the + * one acceptance criterion 3 measures. + */ +function renderPost(row, mode) { + return { + id: row.id, + author: row.author_username || DELETED_AUTHOR, + authorDeleted: row.author_user_id == null, + body: renderForumBody(row.body_html, mode), + createdAt: row.created_at, + editedAt: row.edited_at, + status: row.status, + } +} + +/** + * The thread list for one viewer. + * + * `canModerate` widens what is returned, not just what is offered: a hidden + * thread is visible to the people who can unhide it and to nobody else, so the + * same call answers both audiences without a second endpoint that could disagree + * with this one. + */ +async function listThreads(teamId, { canModerate = false, limit = 50, offset = 0 } = {}) { + const rows = await forumDb.threadsByTeam(teamId, { includeHidden: canModerate, limit, offset }) + return rows.map(publicThread) +} + +/** One thread with its posts, rendered under the current image policy. */ +async function getThread(teamId, threadId, { canModerate = false } = {}) { + const thread = await forumDb.threadById(threadId) + // The team check is here rather than in the SQL so a thread id from another + // Team reads as "not found" and not as "found, but not yours" — a forum is a + // private room and the existence of a thread in it is itself private. + if (!thread || thread.team_id !== teamId) return null + if (thread.status === 'deleted' && !canModerate) return null + if (thread.status === 'hidden' && !canModerate) return null + + const mode = await forumSettings.imageMode() + const posts = await forumDb.postsByThread(threadId, { includeHidden: canModerate }) + return { ...publicThread(thread), posts: posts.map((p) => renderPost(p, mode)) } +} + +/** + * Post an announcement: a thread and its first post, in one call. + * + * An announcement is a degenerate thread rather than its own thing (§5.1) — which + * is why this writes the ordinary tables and 5b adds no migration. `locked` is + * left false: replies are refused because the TYPE takes none, not because the + * thread was closed, and conflating the two would make "unlock" look like it + * would open replies on an announcement. + */ +async function createThread({ team, actor, type, title, body }) { + if (!CREATABLE_TYPES_5A.includes(type)) { + return { ok: false, status: 400, error: 'Only announcements can be posted yet' } + } + const cleaned = cleanForumBody(body) + if (!cleaned || !cleaned.replace(/<[^>]*>/g, '').trim()) { + return { ok: false, status: 400, error: 'An announcement needs a body' } + } + const threadId = await forumDb.insertThread({ + teamId: team.id, + type, + title, + createdBy: actor.id, + createdUsername: actor.username, + }) + await forumDb.insertPost({ + threadId, + authorUserId: actor.id, + authorUsername: actor.username, + bodyHtml: cleaned, + }) + return { ok: true, threadId } +} + +/** + * Apply a moderation action to a thread, and record WHICH authority did it. + * + * `actorRole` is 'leader' or 'staff' — the column that makes a leader's ordinary + * housekeeping distinguishable from a staff intervention after the fact (§5.3). + * The caller resolves it; this function records it and never infers it, because + * an actor who is both would otherwise be recorded as whichever the code checked + * first. + */ +async function moderateThread({ team, threadId, action, actor, actorRole, reason }) { + const effect = THREAD_ACTIONS[action] + if (!effect) return { ok: false, status: 400, error: 'Unknown moderation action' } + + const thread = await forumDb.threadById(threadId) + if (!thread || thread.team_id !== team.id) return { ok: false, status: 404, error: 'Thread not found' } + + await forumDb.setThreadFlags(threadId, effect) + await forumDb.insertModeration({ + teamId: team.id, + targetType: 'thread', + targetId: threadId, + action, + actorUserId: actor.id, + actorUsername: actor.username, + actorRole, + reason, + }) + return { ok: true, action, threadId } +} + +/** The ledger for the admin Team page. Staff-only by its route, not by this function. */ +async function moderationLedger(teamId, opts) { + return forumDb.moderationForTeam(teamId, opts) +} + +module.exports = { + CREATABLE_TYPES_5A, + THREAD_ACTIONS, + listThreads, + getThread, + createThread, + moderateThread, + moderationLedger, + publicThread, + renderPost, +} diff --git a/server/src/model/teams/teamGrants.model.js b/server/src/model/teams/teamGrants.model.js new file mode 100644 index 0000000..a1adf4b --- /dev/null +++ b/server/src/model/teams/teamGrants.model.js @@ -0,0 +1,165 @@ +// ── The grant/revoke flow (TEAMS.md §2.5 path 3) ─────────────────────────── +// +// The RESOLVER lives in teamAccess.model.js and answers "may this account use the +// forum". This file is the WRITE half: who may hand that access out, to whom, and +// what stops a leader turning a Team forum into open hosting on the operator's +// site. +// +// **Two authorities, and they are not the same authority with different reach.** +// +// staff (admin | moderator) — any Team, no cap, may revoke anything +// leader (path 2, on THIS Team) — own Team, capped, may not revoke a staff grant +// +// The last clause is the one worth stating: a leader who could revoke a +// staff-issued grant could undo a moderation decision, which is the whole reason +// `granted_by` is retained rather than collapsed into a boolean. +// +// **Nothing here writes `team_members`, in either direction, ever.** A grant is +// not a membership: it may name any Runic Gateway account, including one with no +// linked game identity at all — that is the point of it, since letting an unlinked +// guildmate into the forum must not be a staff ticket. `teams.model.js` keeps such +// an account off the roster and out of every membership count, and path 4 keeps it +// off external platforms. + +const accessDb = require('./teamAccess.db') +const teamsDb = require('./teams.db') +const access = require('./teamAccess.model') +const usersDb = require('../users/users.db') +const settingsDb = require('../settings/settings.db') + +// The per-Team ceiling on ACTIVE leader-issued grants. A leader admitting +// unlimited arbitrary accounts to a private space on the operator's host is a +// quiet way to turn a Team forum into free hosting; the cap is what makes it a +// decision the operator made rather than one a leader made for them. +const CAP_KEY = 'teams_max_grants_per_team' +const DEFAULT_CAP = 50 + +const STAFF_ROLES = ['admin', 'moderator'] + +async function grantCap() { + const raw = await settingsDb.get(CAP_KEY) + const n = Number.parseInt(raw, 10) + return Number.isFinite(n) && n > 0 ? n : DEFAULT_CAP +} + +const isStaff = (actor) => STAFF_ROLES.includes(actor?.role) + +/** + * What may this actor do with grants on this Team? + * + * Resolved once and returned whole, so the controller asks a question rather than + * assembling the answer from three booleans — the shape that lets a leader check + * and a staff check drift apart. + */ +async function authorityFor(teamId, actor) { + if (isStaff(actor)) return { may: true, as: 'staff' } + const leads = await access.isLeaderByUser(teamId, actor?.id) + return { may: leads, as: leads ? 'leader' : null } +} + +/** + * Issue a grant. Returns the model result shape the Teams controllers translate: + * `{ ok }` or `{ ok: false, status, error }`. + * + * `warning` on a staff grant past the cap is deliberate and is not an error: + * staff are exempt, and silently exceeding a ceiling the operator configured is + * worth saying out loud on the way past. + */ +async function grant({ team, actor, userId, username, reason }) { + const authority = await authorityFor(team.id, actor) + if (!authority.may) return { ok: false, status: 403, error: 'Not a leader of this Team' } + + const target = userId + ? await usersDb.findById(userId) + : await usersDb.findByUsername(username) + if (!target) return { ok: false, status: 404, error: 'No such account' } + + const existing = await accessDb.activeGrant(team.id, target.id) + if (existing) return { ok: false, status: 409, error: 'That account already has an active grant' } + + const cap = await grantCap() + const count = await accessDb.activeGrantCount(team.id) + let warning = null + if (count >= cap) { + if (authority.as === 'leader') { + return { ok: false, status: 409, error: `This Team has reached its limit of ${cap} forum guests` } + } + warning = `This Team is past the configured limit of ${cap} forum guests` + } + + await accessDb.insertGrant({ + teamId: team.id, + userId: target.id, + username: target.username, + grantedBy: actor.id, + grantedUsername: actor.username, + reason, + }) + return { ok: true, as: authority.as, grantee: target.username, ...(warning ? { warning } : {}) } +} + +/** + * Revoke a grant. + * + * The one asymmetry with `grant`: a leader may not revoke what staff issued. + * Checked against `granted_by`'s role AT REVOKE TIME rather than against a stored + * flag, so an account that has since lost its staff role stops protecting the + * grants it made — which is the behaviour an operator demoting someone expects. + */ +async function revoke({ team, actor, userId, reason }) { + const authority = await authorityFor(team.id, actor) + if (!authority.may) return { ok: false, status: 403, error: 'Not a leader of this Team' } + + const existing = await accessDb.activeGrant(team.id, userId) + if (!existing) return { ok: false, status: 404, error: 'No active grant for that account' } + + if (authority.as === 'leader' && existing.granted_by) { + const issuer = await usersDb.findById(existing.granted_by) + if (isStaff(issuer)) { + return { ok: false, status: 403, error: 'That access was granted by staff and only staff may revoke it' } + } + } + + await accessDb.revokeGrant({ + teamId: team.id, + userId, + revokedBy: actor.id, + revokedUsername: actor.username, + reason, + }) + return { ok: true, as: authority.as, grantee: existing.username } +} + +/** + * The Team's forum guests — active grants for accounts that are NOT members. + * + * The subtraction is the §3.2 "Forum guests" list: someone who is both a member + * and a grantee is a member, listed on the roster, and appears here not at all. + * Both facts stay true in the ledger; only the presentation picks one. + */ +async function forumGuests(teamId) { + const [grants, members] = await Promise.all([ + accessDb.activeGrants(teamId), + teamsDb.membersByTeam(teamId, { includeDeparted: false }), + ]) + const memberUserIds = new Set(members.map((m) => m.user_id).filter((id) => id != null)) + return grants + .filter((g) => g.user_id == null || !memberUserIds.has(g.user_id)) + .map((g) => ({ + userId: g.user_id, + username: g.username, + grantedBy: g.granted_username, + grantedAt: g.granted_at, + reason: g.reason, + })) +} + +module.exports = { + CAP_KEY, + DEFAULT_CAP, + grantCap, + authorityFor, + grant, + revoke, + forumGuests, +} diff --git a/server/src/router/v1/admin/teams.controller.js b/server/src/router/v1/admin/teams.controller.js index 00627f1..f9799ba 100644 --- a/server/src/router/v1/admin/teams.controller.js +++ b/server/src/router/v1/admin/teams.controller.js @@ -12,6 +12,10 @@ const access = require('../../../model/teams/teamAccess.model') const teamSync = require('../../../model/teams/teamSync.model') const teamsDb = require('../../../model/teams/teams.db') const activity = require('../../../model/activity/activity.model') +const forum = require('../../../model/teams/teamForum.model') +const forumDb = require('../../../model/teams/teamForum.db') +const forumUploadsModel = require('../../../model/teams/teamForumUploads.model') +const forumSettings = require('../../../model/teams/teamForumSettings.model') const log = require('../../../utils/logger')('teams') @@ -85,6 +89,65 @@ async function grants(req, res) { } } +// ── Forum: the ledger and the upload attribution view (§5.4) ────────────── + +/** + * A Team's forum moderation ledger. + * + * Served whether or not the forum is switched on, unlike every /player forum + * route. The switch guards the forum as a FEATURE — what members can read and + * write — and an operator who turned it off to deal with a problem is precisely + * the operator who needs to see what was moderated (§5.5.1: no data is deleted). + */ +async function forumModeration(req, res) { + try { + const id = Number(req.params.id) + const team = await teamsDb.findById(id) + if (!team) return res.status(404).json({ message: 'Team not found' }) + return res.json({ entries: await forum.moderationLedger(id, { limit: 200 }) }) + } catch (err) { + return fail(res, err, 'forum moderation') + } +} + +/** + * Who uploaded what, when, and how much — across every Team. + * + * This view is the reason §5.5.4 added an attribution table at all: the + * acknowledgement an operator gives before enabling uploads is meaningless if the + * question it makes them responsible for cannot be answered afterwards. + */ +async function forumUploads(req, res) { + try { + return res.json({ + uploads: await forumDb.listUploads({ + limit: Number(req.query.limit) || 100, + offset: Number(req.query.offset) || 0, + includeDeleted: req.query.deleted === '1', + }), + quota: { + dailyBytes: forumUploadsModel.DAILY_QUOTA_BYTES, + retentionDays: forumUploadsModel.RETENTION_DAYS, + }, + }) + } catch (err) { + return fail(res, err, 'forum uploads') + } +} + +/** The forum settings' own state — the acknowledgement, which is not a public key. */ +async function forumSettingsState(req, res) { + try { + return res.json({ + enabled: await forumSettings.forumsEnabled(), + imageMode: await forumSettings.imageMode(), + acknowledgement: await forumSettings.ackState(), + }) + } catch (err) { + return fail(res, err, 'forum settings') + } +} + // ── Leadership overrides (§2.5.1) — NOT gated ───────────────────────────── async function setLeaderOverride(req, res) { @@ -195,6 +258,9 @@ async function decideRequest(req, res) { } module.exports = { + forumModeration, + forumUploads, + forumSettingsState, listTeams, getTeam, resync, diff --git a/server/src/router/v1/admin/teams.router.js b/server/src/router/v1/admin/teams.router.js index 33d2715..dde13af 100644 --- a/server/src/router/v1/admin/teams.router.js +++ b/server/src/router/v1/admin/teams.router.js @@ -92,6 +92,37 @@ teamsRouter.post( // ── :id paths ────────────────────────────────────────────────────────────── +// Both literal, and both under '/forum' rather than '/:id/forum', so they cannot +// be captured by the '/:id' lookup below — 'forum' is not an integer, but relying +// on the validator to reject it would mean the route table's meaning depended on +// a param check three lines further down. +teamsRouter.get( + '/forum/uploads', + // #swagger.tags = ['Admin · Teams'] + // #swagger.summary = 'Upload attribution across every Team forum' + // #swagger.description = 'Who uploaded what, when and how much. This view is why an attribution table exists at all: the liability an operator accepts before enabling uploads is meaningless if "who uploaded this" cannot be answered afterwards. Deleted rows are excluded unless `deleted=1` — a soft-deleted upload still has bytes on disk until the sweep runs.' + // #swagger.parameters['limit'] = { in: 'query', required: false, schema: { type: 'integer' }, description: 'Page size (default 100).' } + // #swagger.parameters['offset'] = { in: 'query', required: false, schema: { type: 'integer' }, description: 'Rows to skip (default 0).' } + // #swagger.parameters['deleted'] = { in: 'query', required: false, schema: { type: 'string', enum: ['0','1'] }, description: 'Include soft-deleted uploads.' } + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'Uploads with their attribution', content: { "application/json": { schema: { $ref: "#/components/schemas/TeamForumUploadList" } } } } */ + query('limit').optional().isInt({ min: 1, max: 500 }).toInt(), + query('offset').optional().isInt({ min: 0 }).toInt(), + query('deleted').optional().isIn(['0', '1']), + validate, + ctrl.forumUploads, +) + +teamsRouter.get( + '/forum/settings', + // #swagger.tags = ['Admin · Teams'] + // #swagger.summary = 'The forum switch, the image policy, and the acknowledgement’s state' + // #swagger.description = 'The two settings themselves ride the ordinary admin settings endpoint and are published to every client; this route adds the one thing that is NOT public — whether the uploads acknowledgement has been given, by whom, and whether the notice has been reworded since. A stale acknowledgement does not disable uploads: it raises a banner and freezes every other forum setting until it is re-given.' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'Forum settings state', content: { "application/json": { schema: { $ref: "#/components/schemas/TeamForumSettingsState" } } } } */ + ctrl.forumSettingsState, +) + teamsRouter.get( '/:id', // #swagger.tags = ['Admin · Teams'] @@ -119,6 +150,20 @@ teamsRouter.get( ctrl.grants, ) +teamsRouter.get( + '/:id/forum/moderation', + // #swagger.tags = ['Admin · Teams'] + // #swagger.summary = 'A Team’s forum moderation ledger' + // #swagger.description = 'Append-only, and deliberately separate from the site’s mod_actions/appeals pair (§5.3): that one is Discord-sanction-shaped and bot-owned, and routing a guild leader locking a thread through it would make ordinary housekeeping an appealable sanction. `actorRole` records which authority was exercised — a leader’s action appears only here, a staffer’s appears here AND in activity_log. Answers whether or not the forum is switched on.' + // #swagger.parameters['id'] = { in: 'path', required: true, schema: { type: 'integer' }, description: 'The Team id.' } + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'The ledger, newest first', content: { "application/json": { schema: { $ref: "#/components/schemas/TeamForumModerationLedger" } } } } */ + /* #swagger.responses[404] = { description: 'No such Team', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + param('id').isInt({ min: 1 }).toInt(), + validate, + ctrl.forumModeration, +) + teamsRouter.post( '/:id/archive', // #swagger.tags = ['Admin · Teams'] diff --git a/server/src/router/v1/player/index.js b/server/src/router/v1/player/index.js index ffe8298..ce972f4 100644 --- a/server/src/router/v1/player/index.js +++ b/server/src/router/v1/player/index.js @@ -27,6 +27,7 @@ const noindex = require('../../../middleware/noindex') const accountRouter = require('./account.router') const appealsRouter = require('./appeals.router') const teamsRouter = require('./teams.router') +const teamForumRouter = require('./teamForum.router') const playerRouter = express.Router() @@ -41,5 +42,9 @@ playerRouter.use(noindex, requireAuth) playerRouter.use('/account', accountRouter) playerRouter.use('/appeals', appealsRouter) playerRouter.use('/teams', teamsRouter) +// Same prefix, second router. The forum and the leader-exercised grant flow are a +// different capability from "the caller's own Teams", and splitting them keeps +// each file about one thing; no path in the two collides. +playerRouter.use('/teams', teamForumRouter) module.exports = playerRouter diff --git a/server/src/router/v1/player/teamForum.controller.js b/server/src/router/v1/player/teamForum.controller.js new file mode 100644 index 0000000..d480f2a --- /dev/null +++ b/server/src/router/v1/player/teamForum.controller.js @@ -0,0 +1,288 @@ +// Player · Team forums — the participant surface (TEAMS.md §5.4). +// +// Under `/player` rather than `/admin` for the reason §2.11 gives: a forum +// participant may be a plain player, 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. The leader check is a per-handler +// question on top of the tier's `requireAuth`. +// +// **Two guards run before anything else in this file, in this order:** +// +// 1. `teams_forums_enabled` — off means every route here answers 404, not 403. +// A 403 says "this exists and you may not have it", which advertises a +// feature the operator deliberately turned off; 404 says "not a thing on +// this site", which is the true statement (§5.5.1). +// 2. the §2.5 access resolver — and never a membership check. Both a member and +// a granted non-member reach the forum, and asking `team_members` directly +// here is precisely how paths 1 and 3 drift back together. +// +// Both live in `resolveForum` below so a handler cannot forget either. + +const teamsDb = require('../../../model/teams/teams.db') +const access = require('../../../model/teams/teamAccess.model') +const grants = require('../../../model/teams/teamGrants.model') +const forum = require('../../../model/teams/teamForum.model') +const forumSettings = require('../../../model/teams/teamForumSettings.model') +const uploads = require('../../../model/teams/teamForumUploads.model') +const activity = require('../../../model/activity/activity.model') + +const log = require('../../../utils/logger')('teams') + +const STAFF_ROLES = ['admin', 'moderator'] +const isStaff = (user) => STAFF_ROLES.includes(user?.role) + +const fail = (res, err, what) => { + log.error(`player team forum: ${what} failed`, { message: err.message }) + return res.status(500).json({ message: 'Internal Server Error' }) +} + +const send = (res, result, body = { ok: true }) => + (result.ok ? res.json({ ...body, ...result }) : res.status(result.status || 400).json({ message: result.error })) + +/** + * The two guards, plus the Team, plus what this caller may do in it. + * + * Returns null when the caller should see a 404 — which covers three different + * situations on purpose: the forum is switched off, the Team does not exist, and + * the caller has no access to it. A private room's contents and its existence are + * the same secret. + */ +async function resolveForum(req) { + if (!(await forumSettings.forumsEnabled())) return null + const team = await teamsDb.findBySlug(req.params.slug) + if (!team) return null + + const resolved = await access.forumAccess(team.id, req.user.id) + const staff = isStaff(req.user) + if (!resolved.allowed && !staff) return null + + return { + team, + access: resolved, + // Staff moderate anywhere; a leader moderates their own Team. `actorRole` + // records WHICH of the two was exercised, and leadership wins when both are + // true: a leader who is also a moderator acting on their own Team is doing + // ordinary housekeeping, and logging it as a staff intervention would put a + // guild's day-to-day tidying into the site's staff-accountability trail. + canModerate: resolved.isLeader || staff, + actorRole: resolved.isLeader ? 'leader' : 'staff', + } +} + +// ── threads ──────────────────────────────────────────────────────────────── + +async function listThreads(req, res) { + try { + const ctx = await resolveForum(req) + if (!ctx) return res.status(404).json({ message: 'Not found' }) + return res.json({ + threads: await forum.listThreads(ctx.team.id, { canModerate: ctx.canModerate }), + canPost: ctx.canModerate, + canModerate: ctx.canModerate, + imageMode: await forumSettings.imageMode(), + }) + } catch (err) { + return fail(res, err, 'list threads') + } +} + +async function getThread(req, res) { + try { + const ctx = await resolveForum(req) + if (!ctx) return res.status(404).json({ message: 'Not found' }) + const thread = await forum.getThread(ctx.team.id, Number(req.params.id), { canModerate: ctx.canModerate }) + if (!thread) return res.status(404).json({ message: 'Not found' }) + return res.json({ ...thread, canModerate: ctx.canModerate }) + } catch (err) { + return fail(res, err, 'get thread') + } +} + +/** + * Post an announcement. 5a: leaders (and staff) only, replies disabled. + * + * The `canModerate` gate is doing double duty here and that is deliberate for one + * phase only: in 5a the only creatable type is an announcement, whose author must + * be a leader. 5b adds `type: 'discussion'`, which any member may create — at + * which point the check splits by type rather than being widened. + */ +async function createThread(req, res) { + try { + const ctx = await resolveForum(req) + if (!ctx) return res.status(404).json({ message: 'Not found' }) + if (!ctx.canModerate) return res.status(403).json({ message: 'Only Team leaders may post announcements' }) + + const result = await forum.createThread({ + team: ctx.team, + actor: req.user, + type: req.body.type || 'announcement', + title: req.body.title, + body: req.body.body, + }) + return send(res, result) + } catch (err) { + return fail(res, err, 'create thread') + } +} + +/** + * Pin / lock / hide / delete a thread, and its opposites. + * + * A staff-exercised action ALSO writes `activity_log`; a leader-exercised one + * writes only the forum ledger (§5.3). That asymmetry is the whole reason the two + * ledgers are cross-referenced rather than merged: routing a guild leader locking + * a thread into the site's sanction pipeline would make ordinary housekeeping an + * appealable staff action. + */ +async function moderateThread(req, res) { + try { + const ctx = await resolveForum(req) + if (!ctx) return res.status(404).json({ message: 'Not found' }) + if (!ctx.canModerate) return res.status(403).json({ message: 'Not a leader of this Team' }) + + const result = await forum.moderateThread({ + team: ctx.team, + threadId: Number(req.params.id), + action: req.body.action, + actor: req.user, + actorRole: ctx.actorRole, + reason: req.body.reason, + }) + if (result.ok && ctx.actorRole === 'staff') { + await activity.log({ + req, + action: 'team.forum.moderate', + detail: `${req.user.username} (#${req.user.id}) ${req.body.action} thread #${req.params.id} ` + + `on team "${ctx.team.name}" (#${ctx.team.id})` + + `${req.body.reason ? `: "${req.body.reason}"` : ''}`, + }) + } + return send(res, result) + } catch (err) { + return fail(res, err, 'moderate thread') + } +} + +// ── grants (§2.5 path 3, leader-exercised) ───────────────────────────────── + +/** + * The grant surface is reachable whether or not the FORUM is on. + * + * Not an oversight: §5.5.1 says a toggle-off revokes no grant and that the rows + * stay authoritative, so a leader must still be able to see and manage them — + * they simply have nothing to grant access to for the moment. What the switch + * guards is the forum's CONTENT, not its access list. + */ +async function listGrants(req, res) { + try { + const team = await teamsDb.findBySlug(req.params.slug) + if (!team) return res.status(404).json({ message: 'Team not found' }) + const authority = await grants.authorityFor(team.id, req.user) + if (!authority.may) return res.status(403).json({ message: 'Not a leader of this Team' }) + return res.json({ + guests: await grants.forumGuests(team.id), + cap: await grants.grantCap(), + as: authority.as, + }) + } catch (err) { + return fail(res, err, 'list grants') + } +} + +async function createGrant(req, res) { + try { + const team = await teamsDb.findBySlug(req.params.slug) + if (!team) return res.status(404).json({ message: 'Team not found' }) + const result = await grants.grant({ + team, + actor: req.user, + userId: req.body.userId, + username: req.body.username, + reason: req.body.reason, + }) + if (result.ok && result.as === 'staff') { + await activity.log({ + req, + action: 'team.forum.grant', + detail: `${req.user.username} (#${req.user.id}) granted forum access to ${result.grantee} ` + + `on team "${team.name}" (#${team.id})`, + }) + } + return send(res, result) + } catch (err) { + return fail(res, err, 'create grant') + } +} + +async function revokeGrant(req, res) { + try { + const team = await teamsDb.findBySlug(req.params.slug) + if (!team) return res.status(404).json({ message: 'Team not found' }) + const result = await grants.revoke({ + team, + actor: req.user, + userId: Number(req.params.userId), + reason: req.body.reason, + }) + if (result.ok && result.as === 'staff') { + await activity.log({ + req, + action: 'team.forum.revoke', + detail: `${req.user.username} (#${req.user.id}) revoked forum access from ${result.grantee} ` + + `on team "${team.name}" (#${team.id})`, + }) + } + return send(res, result) + } catch (err) { + return fail(res, err, 'revoke grant') + } +} + +// ── uploads (§5.5.4) ─────────────────────────────────────────────────────── + +/** + * The same 404 guard, applied at a second level: these routes answer 404 in any + * image mode but `uploads`, for the same reason the forum's do when the switch is + * off. An upload control the client offers and the server refuses is worse than + * no control, which is why the mode is published (§5.5.6) — but the SERVER is + * still what enforces it. + */ +async function createUpload(req, res) { + try { + if (!(await forumSettings.uploadsEnabled())) return res.status(404).json({ message: 'Not found' }) + const ctx = await resolveForum(req) + if (!ctx) return res.status(404).json({ message: 'Not found' }) + if (!req.file) return res.status(400).json({ message: 'No file uploaded' }) + + return send(res, await uploads.accept({ team: ctx.team, actor: req.user, file: req.file })) + } catch (err) { + return fail(res, err, 'upload') + } +} + +async function deleteUpload(req, res) { + try { + if (!(await forumSettings.uploadsEnabled())) return res.status(404).json({ message: 'Not found' }) + const ctx = await resolveForum(req) + if (!ctx) return res.status(404).json({ message: 'Not found' }) + return send(res, await uploads.remove({ + id: Number(req.params.id), + actor: req.user, + isStaff: isStaff(req.user), + })) + } catch (err) { + return fail(res, err, 'delete upload') + } +} + +module.exports = { + listThreads, + getThread, + createThread, + moderateThread, + listGrants, + createGrant, + revokeGrant, + createUpload, + deleteUpload, +} diff --git a/server/src/router/v1/player/teamForum.router.js b/server/src/router/v1/player/teamForum.router.js new file mode 100644 index 0000000..19aecd7 --- /dev/null +++ b/server/src/router/v1/player/teamForum.router.js @@ -0,0 +1,198 @@ +// Player · Team forums (TEAMS.md §5.4) and the leader-exercised grant flow (§2.11). +// +// Mounted at /api/v1/player/teams by player/index.js — the SAME prefix as +// teams.router.js, which is why this file exists separately rather than being +// merged into it: that router is the caller's own Team reads, this one is the +// forum and the grants. Express walks both in mount order and no path collides +// ('/:slug/access' vs '/:slug/forum/*' and '/:slug/grants'). +// +// Every forum route here 404s while `teams_forums_enabled` is off, and the upload +// routes 404 in any image mode but `uploads`. Both guards are in the controller +// rather than in middleware here, because both need the resolved Team and the +// caller's access to decide, and a guard that answers before those are known +// would have to answer 403 — which is the thing §5.5.1 says not to say. + +const express = require('express') +const { body, param } = require('express-validator') + +const ctrl = require('./teamForum.controller') +const validate = require('../../../middleware/validate') +const { makeLimiter } = require('../../../middleware/rateLimit') +const { upload } = require('../admin/imageUpload') + +const forumRouter = express.Router() + +// Writes are rate-limited, reads are not. The caps are per IP and generous enough +// that a Team having a busy afternoon never meets them; what they stop is a script. +const postLimiter = makeLimiter({ + windowMs: 10 * 60 * 1000, + max: 20, + label: 'team-forum-post', + message: 'Too many forum posts. Please slow down.', +}) + +// Tighter than posting, and for a different reason: §2.5 caps how many active +// grants a Team may hold, and this caps how fast a leader may approach that cap. +const grantLimiter = makeLimiter({ + windowMs: 10 * 60 * 1000, + max: 15, + label: 'team-forum-grant', + message: 'Too many grant changes. Please slow down.', +}) + +// Bytes, not requests: the per-account daily quota lives in the uploads model, +// and this is the per-IP flood guard in front of it. +const uploadLimiter = makeLimiter({ + windowMs: 10 * 60 * 1000, + max: 30, + label: 'team-forum-upload', + message: 'Too many uploads. Please slow down.', +}) + +forumRouter.get( + '/:slug/forum/threads', + // #swagger.tags = ['Player · Teams'] + // #swagger.summary = 'List a Team forum’s threads' + // #swagger.description = 'Reachable by a member (path 1) OR a granted account (path 3) — a forum guest with no linked game identity reads exactly as a member does. Answers 404 while `teams_forums_enabled` 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. Hidden threads are included for a leader or staff and for nobody else.' + // #swagger.parameters['slug'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The Team slug.' } + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'The thread list, with what this caller may do', content: { "application/json": { schema: { $ref: "#/components/schemas/TeamForumThreadList" } } } } */ + /* #swagger.responses[404] = { description: 'Forum off, no such Team, or no access', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + ctrl.listThreads, +) + +forumRouter.post( + '/:slug/forum/threads', + // #swagger.tags = ['Player · Teams'] + // #swagger.summary = 'Post an announcement' + // #swagger.description = 'Phase 4 ships a single announcements stream per Team: leader-authored, replies disabled. An announcement is a degenerate thread rather than its own kind of object, so phase 5’s discussion threads add no migration. The body is sanitised with the FORUM’s own profile, in which `img` is never allowed — an author writes a URL and core decides at render time whether it becomes a picture.' + // #swagger.parameters['slug'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The Team slug.' } + /* #swagger.requestBody = { required: true, content: { "application/json": { schema: { type: 'object', required: ['title','body'], properties: { type: { type: 'string', enum: ['announcement'] }, title: { type: 'string', maxLength: 200 }, body: { type: 'string' } } } } } } */ + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'Posted', content: { "application/json": { schema: { type: 'object', properties: { ok: { type: 'boolean' }, threadId: { type: 'integer' } } } } } } */ + /* #swagger.responses[403] = { description: 'Not a leader of this Team', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + postLimiter, + param('slug').isString().trim().isLength({ min: 1, max: 191 }), + body('type').optional().isIn(['announcement']), + body('title').isString().trim().isLength({ min: 1, max: 200 }), + body('body').isString().isLength({ min: 1, max: 40000 }), + validate, + ctrl.createThread, +) + +forumRouter.get( + '/:slug/forum/threads/:id', + // #swagger.tags = ['Player · Teams'] + // #swagger.summary = 'Read one thread and its posts' + // #swagger.description = 'Post bodies are rendered under the CURRENT image policy: `disabled` serves the stored HTML unchanged, `remote` and `uploads` add a core-generated beneath each link that names an image. The stored HTML is identical in all three — flipping the policy back to disabled un-renders every image on every existing post with no data migration.' + // #swagger.parameters['slug'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The Team slug.' } + // #swagger.parameters['id'] = { in: 'path', required: true, schema: { type: 'integer' }, description: 'The thread id.' } + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'The thread', content: { "application/json": { schema: { $ref: "#/components/schemas/TeamForumThread" } } } } */ + /* #swagger.responses[404] = { description: 'Forum off, no such thread, or no access', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + param('id').isInt({ min: 1 }).toInt(), + validate, + ctrl.getThread, +) + +forumRouter.post( + '/:slug/forum/threads/:id/moderate', + // #swagger.tags = ['Player · Teams'] + // #swagger.summary = 'Pin, lock, hide or delete a thread' + // #swagger.description = 'Leader or staff. Every action writes the Team’s own append-only moderation ledger recording WHICH authority was exercised; a staff-exercised one additionally writes activity_log, so the site’s staff-accountability trail sees it while a leader’s ordinary housekeeping stays out of it. Deliberately not routed through the site’s mod_actions/appeals pair, which is Discord-sanction-shaped.' + // #swagger.parameters['slug'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The Team slug.' } + // #swagger.parameters['id'] = { in: 'path', required: true, schema: { type: 'integer' }, description: 'The thread id.' } + /* #swagger.requestBody = { required: true, content: { "application/json": { schema: { type: 'object', required: ['action'], properties: { action: { type: 'string', enum: ['pin','unpin','lock','unlock','hide','unhide','delete','restore'] }, reason: { type: 'string', maxLength: 255 } } } } } } */ + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'Applied', content: { "application/json": { schema: { type: 'object', properties: { ok: { type: 'boolean' }, action: { type: 'string' }, threadId: { type: 'integer' } } } } } } */ + /* #swagger.responses[403] = { description: 'Not a leader of this Team', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + param('id').isInt({ min: 1 }).toInt(), + body('action').isIn(['pin', 'unpin', 'lock', 'unlock', 'hide', 'unhide', 'delete', 'restore']), + body('reason').optional().isString().trim().isLength({ max: 255 }), + validate, + ctrl.moderateThread, +) + +// ── grants ───────────────────────────────────────────────────────────────── + +forumRouter.get( + '/:slug/grants', + // #swagger.tags = ['Player · Teams'] + // #swagger.summary = 'The Team’s forum guests, and the per-Team cap' + // #swagger.description = 'Leader or staff. Lists ACTIVE grants for accounts that are not members — someone who is both is a member, appears on the roster, and is absent here. Answers regardless of whether the forum is switched on: a toggle-off revokes no grant, so the access list stays manageable while there is temporarily nothing to grant access to.' + // #swagger.parameters['slug'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The Team slug.' } + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'Forum guests', content: { "application/json": { schema: { $ref: "#/components/schemas/TeamForumGuestList" } } } } */ + /* #swagger.responses[403] = { description: 'Not a leader of this Team', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + ctrl.listGrants, +) + +forumRouter.post( + '/:slug/grants', + // #swagger.tags = ['Player · Teams'] + // #swagger.summary = 'Grant forum access to an account' + // #swagger.description = 'A grant may name ANY Runic Gateway account, including one with no linked game identity — that is the point of it, since letting an unlinked guildmate into the forum must not be a staff ticket. It never writes team_members: the grantee stays off the roster, out of every membership count, and ineligible for external-platform access. A leader is capped at `teams_max_grants_per_team` active grants (default 50) and rate-limited; staff are exempt and are warned on the way past.' + // #swagger.parameters['slug'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The Team slug.' } + /* #swagger.requestBody = { required: true, content: { "application/json": { schema: { type: 'object', properties: { userId: { type: 'integer' }, username: { type: 'string' }, reason: { type: 'string', maxLength: 255 } } } } } } */ + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'Granted', content: { "application/json": { schema: { type: 'object', properties: { ok: { type: 'boolean' }, grantee: { type: 'string' }, warning: { type: 'string' } } } } } } */ + /* #swagger.responses[409] = { description: 'Already granted, or the Team is at its cap', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + grantLimiter, + body('userId').optional().isInt({ min: 1 }).toInt(), + body('username').optional().isString().trim().isLength({ min: 1, max: 32 }), + body('reason').optional().isString().trim().isLength({ max: 255 }), + validate, + ctrl.createGrant, +) + +forumRouter.delete( + '/:slug/grants/:userId', + // #swagger.tags = ['Player · Teams'] + // #swagger.summary = 'Revoke forum access' + // #swagger.description = 'The grant row is updated rather than deleted — the table is the audit ledger as well as the current state. A leader 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, so an account that has since lost its staff role stops protecting the grants it made.' + // #swagger.parameters['slug'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The Team slug.' } + // #swagger.parameters['userId'] = { in: 'path', required: true, schema: { type: 'integer' }, description: 'The grantee’s account id.' } + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'Revoked', content: { "application/json": { schema: { type: 'object', properties: { ok: { type: 'boolean' }, grantee: { type: 'string' } } } } } } */ + /* #swagger.responses[403] = { description: 'Not a leader, or the grant was staff-issued', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + grantLimiter, + param('userId').isInt({ min: 1 }).toInt(), + body('reason').optional().isString().trim().isLength({ max: 255 }), + validate, + ctrl.revokeGrant, +) + +// ── uploads ──────────────────────────────────────────────────────────────── + +forumRouter.post( + '/:slug/forum/uploads', + // #swagger.tags = ['Player · Teams'] + // #swagger.summary = 'Upload an image to a Team forum' + // #swagger.description = 'Multipart. Answers 404 in any image mode but `uploads`. Beyond the admin upload path’s 8 MB cap, mimetype allowlist and random filename, this one assumes a hostile uploader: the leading bytes are sniffed and a mismatch with the declared type is rejected (a client’s Content-Type header is a claim, not a fact), a rolling per-account byte quota applies, and every accepted file gets an attribution row naming who uploaded it.' + // #swagger.parameters['slug'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The Team slug.' } + /* #swagger.requestBody = { required: true, content: { "multipart/form-data": { schema: { type: 'object', properties: { image: { type: 'string', format: 'binary' } } } } } } */ + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'Stored', content: { "application/json": { schema: { type: 'object', properties: { ok: { type: 'boolean' }, id: { type: 'integer' }, url: { type: 'string' }, bytes: { type: 'integer' } } } } } } */ + /* #swagger.responses[400] = { description: 'Not the image type it claims to be', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + /* #swagger.responses[429] = { description: 'Daily upload quota reached', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + uploadLimiter, + upload.single('image'), + ctrl.createUpload, +) + +forumRouter.delete( + '/:slug/forum/uploads/:id', + // #swagger.tags = ['Player · Teams'] + // #swagger.summary = 'Remove an uploaded image' + // #swagger.description = 'The uploader or staff. Soft: the row is marked and the bytes go with the nightly sweep after a retention window, so a mis-click is recoverable. Note that disabling uploads later stops new files being accepted and does not remove files already uploaded — that is what this route is for.' + // #swagger.parameters['slug'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The Team slug.' } + // #swagger.parameters['id'] = { in: 'path', required: true, schema: { type: 'integer' }, description: 'The upload id.' } + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'Removed', content: { "application/json": { schema: { type: 'object', properties: { ok: { type: 'boolean' } } } } } } */ + /* #swagger.responses[403] = { description: 'Not your upload', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + param('id').isInt({ min: 1 }).toInt(), + validate, + ctrl.deleteUpload, +) + +module.exports = forumRouter -- 2.49.1 From 4ac353684aea4c0fa3d81ac569fb0cae4947ae48 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 07:24:02 -0500 Subject: [PATCH 4/6] feat(teams): harden the upload path for an uploader who is not an admin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../src/model/teams/teamForumUploads.model.js | 177 ++++++++++++++++++ server/src/server.js | 3 + server/src/utils/teamForumUploadSweep.js | 66 +++++++ 3 files changed, 246 insertions(+) create mode 100644 server/src/model/teams/teamForumUploads.model.js create mode 100644 server/src/utils/teamForumUploadSweep.js diff --git a/server/src/model/teams/teamForumUploads.model.js b/server/src/model/teams/teamForumUploads.model.js new file mode 100644 index 0000000..c006bbf --- /dev/null +++ b/server/src/model/teams/teamForumUploads.model.js @@ -0,0 +1,177 @@ +// ── `uploads` mode, and what had to harden first (TEAMS.md §5.5.4) ───────── +// +// The existing admin upload path (router/v1/admin/imageUpload.js) 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 +// `X-Content-Type-Options: nosniff` forced on serve. All of that is kept and this +// file adds the four things that path never needed, because until now it has never +// had a hostile uploader. +// +// 1. MAGIC-BYTE SNIFFING. `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`. Trusted from an admin, not from a player. +// 2. QUOTAS. A per-post attachment cap and a per-account daily byte quota. +// Community uploads with no ceiling is disk exhaustion on the operator's own +// host. (The per-request RATE limit is core's rateLimit middleware, applied +// at the route.) +// 3. ATTRIBUTION. Every accepted file gets a `team_forum_uploads` row. Not +// bookkeeping: the acknowledgement in §5.5.5 is meaningless if "who uploaded +// this" cannot be answered afterwards. +// 4. LIFECYCLE. Deleting a post soft-deletes its uploads; the sweep removes the +// bytes after a retention window, and files with no row at all. The admin +// upload path never deletes anything, which is fine at admin volume and is +// not fine here. + +const fs = require('fs/promises') +const path = require('path') + +const forumDb = require('./teamForum.db') +const { UPLOAD_DIR } = require('../../router/v1/admin/imageUpload') + +// Leading bytes → the type they actually are. Deliberately not a library: five +// signatures, checked exactly, is less surface than a dependency that accepts +// hundreds of formats when the allowlist only wants these. +// +// WebP and AVIF are container formats, so both need a second check past the first +// four bytes — RIFF alone is also .wav, and the `ftyp` box also fronts .mp4. +const SIGNATURES = [ + { mime: 'image/png', test: (b) => b.subarray(0, 8).equals(Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])) }, + { mime: 'image/jpeg', test: (b) => b[0] === 0xff && b[1] === 0xd8 && b[2] === 0xff }, + { mime: 'image/gif', test: (b) => b.subarray(0, 6).toString('latin1').match(/^GIF8[79]a$/) != null }, + { + mime: 'image/webp', + test: (b) => b.subarray(0, 4).toString('latin1') === 'RIFF' && b.subarray(8, 12).toString('latin1') === 'WEBP', + }, + { + mime: 'image/avif', + test: (b) => b.subarray(4, 8).toString('latin1') === 'ftyp' + && ['avif', 'avis'].includes(b.subarray(8, 12).toString('latin1')), + }, +] + +// Per-post attachment cap and per-account rolling byte quota. +const MAX_ATTACHMENTS_PER_POST = 6 +const DAILY_QUOTA_BYTES = 25 * 1024 * 1024 +const QUOTA_WINDOW_HOURS = 24 + +// Lifecycle windows. A soft-deleted file survives long enough for a mis-click to +// be recoverable; an orphan is one uploaded into a composer that was never +// submitted, which is a normal thing to do and so gets a generous grace. +const RETENTION_DAYS = 30 +const ORPHAN_GRACE_HOURS = 48 + +/** + * What do these bytes actually claim to be? + * + * Returns the sniffed mimetype, or null when nothing matches. Null is a rejection + * and never a "trust the header instead" — an unrecognised file is exactly the + * case this check exists for. + */ +function sniff(buffer) { + if (!Buffer.isBuffer(buffer) || buffer.length < 12) return null + return SIGNATURES.find((s) => s.test(buffer))?.mime || null +} + +/** + * Accept a file multer has already written to disk. + * + * The file is on disk before it can be sniffed — multer streams it there — so the + * rejection path has to REMOVE it. A rejected upload that stays on disk is exactly + * the disk-exhaustion vector the quota exists to close, reached by a different + * route. + */ +async function accept({ team, actor, file }) { + const stored = path.join(UPLOAD_DIR, file.filename) + const discard = async () => { await fs.rm(stored, { force: true }) } + + let head + try { + const handle = await fs.open(stored, 'r') + try { + head = Buffer.alloc(16) + await handle.read(head, 0, 16, 0) + } finally { + await handle.close() + } + } catch { + await discard() + return { ok: false, status: 400, error: 'Could not read the uploaded file' } + } + + const sniffed = sniff(head) + if (!sniffed || sniffed !== file.mimetype) { + await discard() + return { ok: false, status: 400, error: 'That file is not the image type it claims to be' } + } + + const used = await forumDb.bytesUploadedSince(actor.id, QUOTA_WINDOW_HOURS) + if (used + file.size > DAILY_QUOTA_BYTES) { + await discard() + return { ok: false, status: 429, error: 'Daily upload limit reached. Try again tomorrow.' } + } + + const id = await forumDb.insertUpload({ + teamId: team.id, + postId: null, // attached when the post that embeds it is written + uploaderUserId: actor.id, + uploaderUsername: actor.username, + filename: file.filename, + mimetype: sniffed, // the SNIFFED type, never the client's header + byteSize: file.size, + }) + return { ok: true, id, url: `/uploads/${file.filename}`, bytes: file.size } +} + +/** + * Remove an upload. The uploader may, within the edit window; staff may at any + * time. Soft — the bytes go with the sweep, not with the button. + */ +async function remove({ id, actor, isStaff }) { + const row = await forumDb.uploadById(id) + if (!row || row.deleted_at) return { ok: false, status: 404, error: 'No such upload' } + if (!isStaff && row.uploader_user_id !== actor.id) { + return { ok: false, status: 403, error: 'Not your upload' } + } + await forumDb.softDeleteUpload(id, actor.id) + return { ok: true } +} + +/** + * The nightly sweep: bytes for soft-deleted rows past retention, plus files on + * disk with no row at all. + * + * The orphan half deliberately only considers files whose names match the upload + * naming scheme AND appear in no row. UPLOAD_DIR is shared with the admin upload + * path, whose files have no row here and must never be swept — so the sweep works + * from the FORUM's own rows outward and never from the directory listing inward. + */ +async function sweep({ retentionDays = RETENTION_DAYS, orphanGraceHours = ORPHAN_GRACE_HOURS } = {}) { + const expired = await forumDb.sweepableUploads(retentionDays) + const orphans = await forumDb.orphanedUploads(orphanGraceHours) + const doomed = [...expired, ...orphans] + const cleared = [] + for (const row of doomed) { + try { + await fs.rm(path.join(UPLOAD_DIR, row.filename), { force: true }) + cleared.push(row.id) + } catch { + // Leave the ROW as well as the file. A file we could not delete is one the + // next run should try again, and dropping its row would lose the only + // record that the bytes are still there. + } + } + await forumDb.deleteUploadRows(cleared) + return { swept: doomed.length, filesRemoved: cleared.length } +} + +module.exports = { + MAX_ATTACHMENTS_PER_POST, + DAILY_QUOTA_BYTES, + QUOTA_WINDOW_HOURS, + RETENTION_DAYS, + ORPHAN_GRACE_HOURS, + sniff, + accept, + remove, + sweep, +} diff --git a/server/src/server.js b/server/src/server.js index 4474214..9b4dbce 100644 --- a/server/src/server.js +++ b/server/src/server.js @@ -10,6 +10,7 @@ const http = require('http') const botScore = require('./middleware/botScore') const announceWorker = require('./utils/announceWorker') const teamActivityPrune = require('./utils/teamActivityPrune') +const teamForumUploadSweep = require('./utils/teamForumUploadSweep') const { ensureSchema, close } = require('./utils/db') const { seedDefaults, createInitialAdminFromEnv } = require('../db/seed') const settings = require('./model/settings/settings.model') @@ -155,6 +156,7 @@ async function start() { // is the obvious unbounded-growth failure, so retention starts with the feed // rather than after someone notices. No-op on a deployment with no Teams. teamActivityPrune.start() + teamForumUploadSweep.start() setupShutdown(server, internalServer) } @@ -174,6 +176,7 @@ function setupShutdown(server, internalServer) { botScore.stopSweeper() // stop the bot-store cleanup interval announceWorker.stop() // stop the news-announcement dispatcher poller teamActivityPrune.stop() // stop the Team activity retention timer + teamForumUploadSweep.stop() // stop the forum upload sweep server.close(() => log.info('http server closed')) if (internalServer) internalServer.close(() => log.info('internal http server closed')) try { diff --git a/server/src/utils/teamForumUploadSweep.js b/server/src/utils/teamForumUploadSweep.js new file mode 100644 index 0000000..5d6e886 --- /dev/null +++ b/server/src/utils/teamForumUploadSweep.js @@ -0,0 +1,66 @@ +// ── Team forum upload sweep ──────────────────────────────────────────────── +// +// TEAMS.md §5.5.4's lifecycle half: soft-deleted uploads lose their bytes after a +// retention window, and files uploaded into a composer that was never submitted +// lose theirs after a grace period. The existing admin upload path never deletes +// anything, which is fine at admin volume and is not fine once a community can +// upload. +// +// Same in-process shape as utils/teamActivityPrune — setInterval + unref + stop(), +// wired into server.js start/shutdown. There is no cron in this stack. +// +// **It runs whether or not `teams_forum_images` is `uploads`, and that is the +// point.** An operator who turns uploads off after a problem has files already on +// disk; a sweep that switched itself off with the setting would strand exactly the +// bytes they were trying to be rid of. The admin help text says the same thing in +// the other direction — disabling uploads stops new files, it does not delete old +// ones — and this is the only thing that eventually does. + +const uploads = require('../model/teams/teamForumUploads.model') +const log = require('./logger')('teams') + +const INTERVAL_MS = Number(process.env.TEAM_FORUM_SWEEP_MS) || 24 * 60 * 60 * 1000 +// Later than the activity prune's five minutes, so two table-walking jobs do not +// land on the same boot at the same moment. +const FIRST_RUN_MS = Number(process.env.TEAM_FORUM_SWEEP_DELAY_MS) || 10 * 60 * 1000 + +let timer = null +let firstRun = null + +/** One sweep. Never throws — it runs on a timer with nobody to catch it. */ +async function tick() { + try { + const result = await uploads.sweep() + if (result.swept) log.info('team forum upload sweep', result) + return result + } catch (err) { + log.error('team forum upload sweep failed', { message: err.message }) + return null + } +} + +function start() { + if (timer || firstRun) return timer + firstRun = setTimeout(() => { + firstRun = null + tick() + timer = setInterval(() => { tick() }, INTERVAL_MS) + if (timer.unref) timer.unref() + }, FIRST_RUN_MS) + if (firstRun.unref) firstRun.unref() + log.info('team forum upload sweep started', { intervalMs: INTERVAL_MS, firstRunMs: FIRST_RUN_MS }) + return timer +} + +function stop() { + if (firstRun) { + clearTimeout(firstRun) + firstRun = null + } + if (timer) { + clearInterval(timer) + timer = null + } +} + +module.exports = { start, stop, tick, INTERVAL_MS, FIRST_RUN_MS } -- 2.49.1 From cbb7339a3a73b3052aa1360e4f3161d0becdce51 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 07:24:24 -0500 Subject: [PATCH 5/6] feat(teams): the forum panel core fills, and the operator's controls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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 --- client/src/api/client.js | 27 ++ client/src/main.jsx | 9 + client/src/modules/TeamForumPanel.jsx | 382 ++++++++++++++++++ .../src/routes/admin/views/SettingsAdmin.jsx | 3 + .../routes/admin/views/TeamForumSettings.jsx | 248 ++++++++++++ 5 files changed, 669 insertions(+) create mode 100644 client/src/modules/TeamForumPanel.jsx create mode 100644 client/src/routes/admin/views/TeamForumSettings.jsx diff --git a/client/src/api/client.js b/client/src/api/client.js index d35a67e..447803d 100644 --- a/client/src/api/client.js +++ b/client/src/api/client.js @@ -152,6 +152,26 @@ export const api = { if (opts.offset != null) qs.set('offset', String(opts.offset)) return req(`/public/teams/${encodeURIComponent(slug)}/activity${withQs(qs.toString())}`) }, + // The Team FORUM, under /player because a participant may be a plain player and + // a leader is a player (TEAMS.md §2.11). Core's, for the same reason the feed is + // core's: only core resolves whether this viewer is inside the Team, and the + // member/guest split is a security boundary. The module renders the PLACE. + teamForumThreads: (slug) => req(`/player/teams/${encodeURIComponent(slug)}/forum/threads`), + teamForumThread: (slug, id) => req(`/player/teams/${encodeURIComponent(slug)}/forum/threads/${id}`), + teamForumPost: (slug, body) => + req(`/player/teams/${encodeURIComponent(slug)}/forum/threads`, { method: 'POST', body }), + teamForumModerate: (slug, id, body) => + req(`/player/teams/${encodeURIComponent(slug)}/forum/threads/${id}/moderate`, { method: 'POST', body }), + teamForumUpload: (slug, file) => { + const fd = new FormData() + fd.append('image', file) + return req(`/player/teams/${encodeURIComponent(slug)}/forum/uploads`, { method: 'POST', body: fd, raw: true }) + }, + teamGrantList: (slug) => req(`/player/teams/${encodeURIComponent(slug)}/grants`), + teamGrantAdd: (slug, body) => + req(`/player/teams/${encodeURIComponent(slug)}/grants`, { method: 'POST', body }), + teamGrantRevoke: (slug, userId) => + req(`/player/teams/${encodeURIComponent(slug)}/grants/${userId}`, { method: 'DELETE' }), wikiTags: () => req('/public/wiki/tags'), wikiPage: (slug) => req(`/public/wiki/${slug}`), // CMS pages (block-based). Published-only for the public; a draft-preview link @@ -284,6 +304,13 @@ export const api = { req(`/admin/teams/${id}/leader-override`, { method: 'POST', body }), clearTeamLeaderOverride: (id, memberKey) => req(`/admin/teams/${id}/leader-override/${encodeURIComponent(memberKey)}`, { method: 'DELETE' }), + teamForumSettings: () => req('/admin/teams/forum/settings'), + teamForumUploads: (opts = {}) => { + const qs = new URLSearchParams() + if (opts.deleted) qs.set('deleted', '1') + return req(`/admin/teams/forum/uploads${withQs(qs.toString())}`) + }, + teamForumModeration: (id) => req(`/admin/teams/${id}/forum/moderation`), teamReviewQueue: () => req('/admin/teams/review'), teamRequests: (status) => req(`/admin/teams/requests${status ? `?status=${status}` : ''}`), decideTeamRequest: (id, status, note) => diff --git a/client/src/main.jsx b/client/src/main.jsx index 258fbbd..0d26693 100644 --- a/client/src/main.jsx +++ b/client/src/main.jsx @@ -5,6 +5,7 @@ import App from './App.jsx' import { publishSharedDependencies } from './modules/shared.js' import { declareSlot, applyCoreFills, fillModuleSlot } from './modules/registry.js' import TeamActivityFeed from './modules/TeamActivityFeed.jsx' +import TeamForumPanel from './modules/TeamForumPanel.jsx' import './styles/theme.css' // Publish window.__rg BEFORE rendering and before any module chunk evaluates. @@ -75,6 +76,14 @@ declareSlot('player.invite.accepted') // unfilled slot rendering nothing. fillModuleSlot('uo.guild.detail', TeamActivityFeed) +// The forum is core's for the same reason and goes in a SECOND place the module +// declares, rather than joining the feed in the first: a slot takes one component +// (first fill wins), and stacking two unrelated panels into one fill would make +// the module unable to place them separately on its own page. It also keeps the +// two independent — a deployment with the forum switched off renders the feed +// exactly as before. +fillModuleSlot('uo.guild.forum', TeamForumPanel) + // Render on DOMContentLoaded rather than immediately, and that is the one line // of core's boot the module system changes. // diff --git a/client/src/modules/TeamForumPanel.jsx b/client/src/modules/TeamForumPanel.jsx new file mode 100644 index 0000000..d961654 --- /dev/null +++ b/client/src/modules/TeamForumPanel.jsx @@ -0,0 +1,382 @@ +import { useCallback, useEffect, useState } from 'react' +import { useSearchParams } from 'react-router-dom' +import { api } from '../api/client.js' +import { useAuth } from '../contexts/AuthContext.jsx' +import { useSite } from '../contexts/SiteContext.jsx' + +// Core's Team forum, rendered into a second slot a MODULE declares +// (TEAMS.md Part 5, and the phase 3 amendment to §3.4). +// +// **Why the forum is core's content on a module's page.** Everything that decides +// who may read a thread is core's — the §2.5 resolver, the grants ledger, the +// member/guest distinction — and none of it is a module's to reimplement. But +// core does not own the word for a Team, so it publishes no Team page: the module +// that says "guild" owns the page and declares a place on it, and core fills the +// place. Same direction as the activity feed, same reason. +// +// **It is a whole forum inside one slot, and navigates by SEARCH PARAM.** A +// thread needs to be linkable, and core cannot mount a route for it — the route +// belongs to the module's page. `?thread=12` gives a shareable URL that works +// under whatever path the module chose, with no route of core's anywhere in it, +// and the browser's back button behaves. That is the whole reason this component +// holds a list view and a detail view rather than being two components. +// +// **The image mode is published so this can draw the right composer — never to +// decide what renders.** Post bodies arrive already rendered by the server under +// the current policy (§5.5.3); the mode is read here only to show or hide an +// upload control that would otherwise 404. If the two ever disagree, the server +// is right. +// +// Like the feed, everything here degrades to rendering nothing. A 404 from the +// thread list is the ordinary case — the forum is switched off, or this viewer +// has no access — and putting an error box on a page core does not own would be +// core reporting its own absence as a defect on someone else's surface. + +export default function TeamForumPanel({ externalId, moduleId }) { + const { user } = useAuth() + const { settings } = useSite() + const [params, setParams] = useSearchParams() + const [team, setTeam] = useState(null) + const [state, setState] = useState({ loading: true, forum: null }) + const [thread, setThread] = useState(null) + const [composing, setComposing] = useState(false) + + const openThreadId = params.get('thread') + const imageMode = settings?.teams_forum_images || 'disabled' + const forumsEnabled = String(settings?.teams_forums_enabled ?? '0') === '1' + + const loadThreads = useCallback(async (slug) => { + try { + setState({ loading: false, forum: await api.teamForumThreads(slug) }) + } catch { + setState({ loading: false, forum: null }) + } + }, []) + + useEffect(() => { + let active = true + // An anonymous visitor has no forum by definition — every route is behind + // requireAuth — so skip the two calls rather than provoking a 401 per page. + if (!externalId || !moduleId || !user || !forumsEnabled) { + setState({ loading: false, forum: null }) + return undefined + } + // The module names the Team its own way; core resolves that to a slug. Same + // two-call shape as the activity feed, and for the same reason: a module + // never has to hold core's identifiers. + api.teamByExternalId(moduleId, externalId) + .then(async (found) => { + if (!active) return + setTeam(found) + await loadThreads(found.slug) + }) + .catch(() => { if (active) setState({ loading: false, forum: null }) }) + return () => { active = false } + }, [externalId, moduleId, user, forumsEnabled, loadThreads]) + + useEffect(() => { + let active = true + if (!team || !openThreadId) { + setThread(null) + return undefined + } + api.teamForumThread(team.slug, openThreadId) + .then((t) => { if (active) setThread(t) }) + .catch(() => { if (active) setThread(null) }) + return () => { active = false } + }, [team, openThreadId]) + + const openThread = (id) => { + const next = new URLSearchParams(params) + if (id == null) next.delete('thread') + else next.set('thread', String(id)) + setParams(next) + } + + const { loading, forum } = state + if (loading || !forum) return null + + if (openThreadId && thread) { + return ( + openThread(null)} + onModerate={async (action) => { + await api.teamForumModerate(team.slug, thread.id, { action }) + await loadThreads(team.slug) + openThread(null) + }} + /> + ) + } + + return ( +
+
+

+ Announcements +

+ {forum.canPost && !composing && ( + + )} +
+ + {composing && ( + setComposing(false)} + onPosted={async () => { + setComposing(false) + await loadThreads(team.slug) + }} + /> + )} + + {forum.threads.length === 0 && !composing && ( +

+ Nothing has been announced here yet. +

+ )} + + {forum.canModerate && } + +
    + {forum.threads.map((t) => ( +
  • + +
    • + ))} +
+
+ ) +} + +/** + * The leader's grant control — §2.5 path 3, exercised by a leader rather than by + * staff. + * + * Worth being explicit about what this admits someone to and what it does not: a + * grant may name ANY account, including one with no linked game character, and it + * writes nothing but the grants ledger. A guest here never appears on the roster, + * never counts towards the Team's membership, and never becomes eligible for a + * Discord role — an integration cannot verify that an unlinked account is a real + * game member, so it must not hand that account a privilege somewhere + * impersonation has consequences. + * + * A leader is capped; staff are not. The cap is shown rather than only enforced, + * because a leader who hits a limit they were never told about reads it as a bug. + */ +function GuestManager({ slug }) { + const [open, setOpen] = useState(false) + const [data, setData] = useState(null) + const [username, setUsername] = useState('') + const [error, setError] = useState(null) + + const load = useCallback(async () => { + try { + setData(await api.teamGrantList(slug)) + } catch { + setData(null) + } + }, [slug]) + + useEffect(() => { if (open) load() }, [open, load]) + + const add = async (event) => { + event.preventDefault() + setError(null) + try { + await api.teamGrantAdd(slug, { username }) + setUsername('') + await load() + } catch (err) { + setError(err.message || 'Could not grant access') + } + } + + const revoke = async (userId) => { + setError(null) + try { + await api.teamGrantRevoke(slug, userId) + await load() + } catch (err) { + setError(err.message || 'Could not revoke that') + } + } + + if (!open) { + return ( + + ) + } + + return ( +
+
+

Forum guests

+ +
+

+ Guests read and post in this forum without being members of the Team. They do not appear on the + roster and are not counted as members. + {data?.cap ? ` Up to ${data.cap} at a time.` : ''} +

+ +
    + {(data?.guests || []).map((g) => ( +
  • + {g.username} + +
    • + ))} + {data && data.guests.length === 0 && ( +
  • No guests yet.
    • + )} +
+ +
+ setUsername(e.target.value)} + placeholder="Account name" + maxLength={32} + required + /> + +
+ {error &&

{error}

} +
+ ) +} + +function ThreadView({ thread, canModerate, onBack, onModerate }) { + return ( +
+ +

+ {thread.title} +

+

+ {thread.author} + {thread.authorDeleted && ' (account removed)'} +

+ + {thread.posts.map((post) => ( +
+ {/* + Rendered server-side under the operator's image policy, which is why + this is dangerouslySetInnerHTML and not a sanitizer call here. The body + was sanitised on write with the forum's own profile — one in which + `img` is never allowed — and any 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. + */} + {/* eslint-disable-next-line react/no-danger */} +
+
+ ))} + + {canModerate && ( +
+ + +
+ )} +
+ ) +} + +function Composer({ slug, imageMode, onCancel, onPosted }) { + const [title, setTitle] = useState('') + const [body, setBody] = useState('') + const [error, setError] = useState(null) + const [busy, setBusy] = useState(false) + + const submit = async (event) => { + event.preventDefault() + setBusy(true) + setError(null) + try { + await api.teamForumPost(slug, { type: 'announcement', title, body }) + await onPosted() + } catch (err) { + setError(err.message || 'Could not post that') + } finally { + setBusy(false) + } + } + + const attach = async (event) => { + const file = event.target.files?.[0] + if (!file) return + try { + const { url } = await api.teamForumUpload(slug, file) + // The URL goes into the BODY as text, not as an tag. The author never + // writes markup here — core decides at render time whether a URL becomes a + // picture, which is what makes the operator's image policy enforceable + // rather than decorative. + setBody((current) => `${current}${current ? '\n\n' : ''}${url}`) + } catch (err) { + setError(err.message || 'Could not upload that') + } + } + + return ( +
+ setTitle(e.target.value)} + placeholder="Title" + maxLength={200} + required + /> +