From 11fd9821bf5dca198777f9706dc6065c3e0218fe Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 07:23:15 -0500 Subject: [PATCH] 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