RunicGateway/website
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Activity

Merge pull request 'fix(teams): four defects the live rig found in the phase 4 forum' (#154) from fix/teams-phase4-live-rig into edge

Browse Source 
Reviewed-on: #154
This commit is contained in:
Colby Whitlock
2026-08-18 15:21:55 +00:00
parent 16e31de087 5baada08ef
commit 763de66ebb
6 changed files with 141 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
client/src/modules/TeamForumPanel.jsx
View File

@@ -1,5 +1,6 @@
import { useCallback, useEffect, useState } from 'react'
import { useSearchParams } from 'react-router-dom'
import DOMPurify from 'dompurify'
import { api } from '../api/client.js'
import { useAuth } from '../contexts/AuthContext.jsx'
import { useSite } from '../contexts/SiteContext.jsx'
@@ -118,7 +119,7 @@ export default function TeamForumPanel({ externalId, moduleId }) {
Announcements
</h2>
{forum.canPost && !composing && (
<button type="button" className="btn-ghost sans" onClick={() => setComposing(true)}>
<button type="button" className="pill" onClick={() => setComposing(true)}>
Post an announcement
</button>
)}
@@ -225,7 +226,7 @@ function GuestManager({ slug }) {
if (!open) {
return (
<button type="button" className="btn-ghost sans" onClick={() => setOpen(true)} style={{ marginTop: 10 }}>
<button type="button" className="pill" onClick={() => setOpen(true)} style={{ marginTop: 10 }}>
Forum guests
</button>
)
@@ -235,7 +236,7 @@ function GuestManager({ slug }) {
<section style={{ marginTop: 12, padding: 12, border: '1px solid var(--rule, #ccc)', borderRadius: 6 }}>
<header style={{ display: 'flex', justifyContent: 'space-between', alignItems: 'baseline' }}>
<h3 className="sans" style={{ margin: 0, fontSize: '0.95rem' }}>Forum guests</h3>
<button type="button" className="btn-ghost sans" onClick={() => setOpen(false)}>Close</button>
<button type="button" className="pill" onClick={() => setOpen(false)}>Close</button>
</header>
<p className="sans dim" style={{ fontSize: '0.8rem', margin: '6px 0 10px' }}>
Guests read and post in this forum without being members of the Team. They do not appear on the
@@ -247,7 +248,7 @@ function GuestManager({ slug }) {
{(data?.guests || []).map((g) => (
<li key={g.userId} className="sans" style={{ fontSize: '0.88rem', display: 'flex', gap: 8 }}>
<span>{g.username}</span>
<button type="button" className="btn-ghost sans" onClick={() => revoke(g.userId)}>Remove</button>
<button type="button" className="pill" onClick={() => revoke(g.userId)}>Remove</button>
</li>
))}
{data && data.guests.length === 0 && (
@@ -257,14 +258,14 @@ function GuestManager({ slug }) {
<form onSubmit={add} style={{ display: 'flex', gap: 8 }}>
<input
className="sans"
className="input"
value={username}
onChange={(e) => setUsername(e.target.value)}
placeholder="Account name"
maxLength={32}
required
/>
<button type="submit" className="btn sans">Add</button>
<button type="submit" className="btn btn-primary btn-sq">Add</button>
</form>
{error && <p className="sans" style={{ color: 'var(--danger, crimson)', fontSize: '0.85rem' }}>{error}</p>}
</section>
@@ -274,7 +275,7 @@ function GuestManager({ slug }) {
function ThreadView({ thread, canModerate, onBack, onModerate }) {
return (
<section style={{ marginTop: 26 }}>
<button type="button" className="btn-ghost sans" onClick={onBack} style={{ marginBottom: 10 }}>
<button type="button" className="pill" onClick={onBack} style={{ marginBottom: 10 }}>
← All announcements
</button>
<h2 className="display" style={{ fontSize: '1.15rem', color: 'var(--head)', margin: '0 0 4px' }}>
@@ -288,24 +289,33 @@ function ThreadView({ thread, canModerate, onBack, onModerate }) {
{thread.posts.map((post) => (
<article key={post.id} style={{ marginBottom: 16 }}>
{/*
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 <img> in it was emitted by core's own
renderer with a fixed attribute set. A client-side sanitiser would have
to strip exactly the tag core just decided to add.
Sanitised on write with the forum's own profile, rendered server-side
under the operator's image policy, and re-sanitised here — the same
defence-in-depth every other body-HTML surface on this site applies
(FiveOnFriday, NewsletterIssue, the rich-text block).
`ADD_ATTR: ['referrerpolicy']` is load-bearing and not a preference.
DOMPurify's default allowlist carries `loading` but NOT
`referrerpolicy`, so a plain sanitize() call silently strips the one
attribute that limits what a remote embed leaks to the host serving it
— the privacy property the admin help text promises an operator. The
<img> itself is core's own output with a fixed attribute set, so
nothing here is widening what an author can write.
*/}
{/* eslint-disable-next-line react/no-danger */}
<div className="serif" dangerouslySetInnerHTML={{ __html: post.body }} />
<div
className="prose"
dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(post.body || '', { ADD_ATTR: ['referrerpolicy'] }) }}
/>
</article>
))}
{canModerate && (
<div style={{ display: 'flex', gap: 8, marginTop: 12 }}>
<button type="button" className="btn-ghost sans" onClick={() => onModerate(thread.pinned ? 'unpin' : 'pin')}>
<button type="button" className="pill" onClick={() => onModerate(thread.pinned ? 'unpin' : 'pin')}>
{thread.pinned ? 'Unpin' : 'Pin'}
</button>
<button type="button" className="btn-ghost sans" onClick={() => onModerate(thread.status === 'hidden' ? 'unhide' : 'hide')}>
<button type="button" className="pill" onClick={() => onModerate(thread.status === 'hidden' ? 'unhide' : 'hide')}>
{thread.status === 'hidden' ? 'Unhide' : 'Hide'}
</button>
</div>
@@ -352,7 +362,7 @@ function Composer({ slug, imageMode, onCancel, onPosted }) {
return (
<form onSubmit={submit} style={{ display: 'grid', gap: 8, marginTop: 12 }}>
<input
className="sans"
className="input"
value={title}
onChange={(e) => setTitle(e.target.value)}
placeholder="Title"
@@ -360,7 +370,7 @@ function Composer({ slug, imageMode, onCancel, onPosted }) {
required
/>
<textarea
className="sans"
className="textarea"
value={body}
onChange={(e) => setBody(e.target.value)}
placeholder="Write your announcement. Paste an image URL on its own line to share a picture."
@@ -374,8 +384,8 @@ function Composer({ slug, imageMode, onCancel, onPosted }) {
)}
{error && <p className="sans" style={{ color: 'var(--danger, crimson)', fontSize: '0.85rem' }}>{error}</p>}
<div style={{ display: 'flex', gap: 8 }}>
<button type="submit" className="btn sans" disabled={busy}>Post</button>
<button type="button" className="btn-ghost sans" onClick={onCancel}>Cancel</button>
<button type="submit" className="btn btn-primary btn-sq" disabled={busy}>Post</button>
<button type="button" className="pill" onClick={onCancel}>Cancel</button>
</div>
</form>
)

12
client/src/styles/theme.css
View File

@@ -298,6 +298,18 @@ button[disabled] {
}
/* ===== Rich prose (wiki / newsletter body) ===== */
.forum-embed {
/* The image a Team-forum post's URL renders as, in `remote`/`uploads` mode.
Emitted by the server (utils/forumHtml.js), never by an author — which is
what makes the operator's image policy enforceable. Block, so it sits
beneath its link rather than beside it; capped, because a remote image is
whatever size its host decided and one post must not blow out the column. */
display: block;
margin-top: 8px;
max-width: 100%;
height: auto;
border-radius: var(--radius-input);
}
.prose {
color: var(--text);
font-size: 1.06rem;

33
server/src/model/teams/teamForumSettings.model.js
View File

@@ -91,16 +91,33 @@ async function ackState() {
* Returns `{ ok }` or `{ ok: false, error, status }`, matching the model result
* shape the Teams controllers already translate.
*/
function assertAcknowledged(nextMode, acknowledge) {
async 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}).`,
}
if (String(acknowledge ?? '') === ACK_VERSION) return { ok: true }
// **The gate is on SELECTING uploads, not on the value being present.**
//
// A settings form sends every field it owns, so once uploads is on, every later
// save re-sends `uploads` — turning the forum off, switching back to `remote`,
// any of it. Demanding a fresh acknowledgement for those would make the mode a
// one-way door: the operator could never change a forum setting again, and the
// one thing they would most want to do in a hurry (switch the forum off) would
// be the thing refused. Found on the live rig, where unticking "Enable Team
// forums" came back 400.
//
// So an acknowledgement already ON RECORD, for the version in force, while
// uploads is ALREADY the stored mode, is what this request needs — there is no
// new consent to take. A transition INTO uploads still needs the checkbox, and
// a stale acknowledgement is caught by assertSettingsWritable, which is the
// separate rule for a reworded notice.
const [state, current] = await Promise.all([ackState(), imageMode()])
if (current === 'uploads' && state.given && !state.stale) return { ok: true }
return {
ok: false,
status: 400,
error: `Enabling uploads requires acknowledging the current notice (version ${ACK_VERSION}).`,
}
return { ok: true }
}
/**

2
server/src/router/v1/admin/admin.controller.js
View File

@@ -609,7 +609,7 @@ async function updateSettings(req, res) {
}
// 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)
const gate = await forumSettings.assertAcknowledged(nextImageMode, req.body.acknowledge)
if (!gate.ok) return res.status(gate.status).json({ message: gate.error })
}
{

24
server/src/utils/forumHtml.js
View File

@@ -82,7 +82,19 @@ const IMAGE_EXTENSIONS = ['.png', '.jpg', '.jpeg', '.gif', '.webp', '.avif']
// being shown, not offered.
const NO_LINKIFY = new Set(['a', 'code', 'pre'])
const BARE_URL = /\bhttps?:\/\/[^\s<>"']+/g
// Absolute http(s) URLs, and root-relative `/uploads/…` paths.
//
// The second alternative is not a nicety. In `uploads` mode the composer hands
// the author a path like `/uploads/1787…-ab12.png`, puts it in the body as TEXT
// (the author never writes markup — that is the whole design), and the renderer
// only ever rewrites ANCHORS. Without this branch the write path cannot produce
// the anchor the read path looks for, so an uploaded image could never become a
// picture — even though `isEmbeddableImageUrl` was written to accept exactly
// these paths. The two halves disagreed, and only a real upload showed it.
//
// Deliberately narrow: `/uploads/` and nothing else, so ordinary prose that
// happens to contain a slash is left alone.
const BARE_URL = /\bhttps?:\/\/[^\s<>"']+|(?:^|(?<=[\s(]))\/uploads\/[A-Za-z0-9._~-]+(?:\/[A-Za-z0-9._~-]+)*/g
/**
* Sanitise a forum post body. Runs on WRITE; the stored value is already safe and
@@ -180,7 +192,15 @@ function renderForumBody(storedHtml, mode) {
// 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}<img src="${href}" loading="lazy" referrerpolicy="no-referrer" alt="">`
//
// `class` rather than an inline style, for two things the live rig showed:
// the embed has to sit BENEATH the link (§5.5.3) and an <img> is inline, so
// without it the picture lands beside the URL; and a remote image is any size
// its host chooses, so it needs a max-width or one post can blow the column
// out. Both live in core's stylesheet (`.forum-embed`) because a style
// attribute would then have to survive the client's DOMPurify pass, and its
// CSS sanitiser is a larger thing to reason about than one class name.
return `${anchor}<img class="forum-embed" src="${href}" loading="lazy" referrerpolicy="no-referrer" alt="">`
})
}

60
server/test/teamForum.test.js
View File

@@ -67,23 +67,45 @@ test('an unexpected stored image mode reads as disabled rather than as itself',
// ── the acknowledgement gate (§5.5.5) ──────────────────────────────────────
test('acceptance 4: uploads mode is rejected without a matching acknowledgement', () => {
test('acceptance 4: uploads mode is rejected without a matching acknowledgement', async () => {
// Server-side, with the admin UI's checkbox bypassed — a checkbox is how the
// gate is presented and never the gate.
const refused = forumSettings.assertAcknowledged('uploads', undefined)
// gate is presented and never the gate. Nothing is on record and the stored
// mode is not uploads, so this is a genuine transition INTO it.
const refused = await forumSettings.assertAcknowledged('uploads', undefined)
assert.equal(refused.ok, false)
assert.equal(refused.status, 400)
// A STALE version is not an acknowledgement either.
assert.equal(forumSettings.assertAcknowledged('uploads', '0').ok, false)
assert.equal(forumSettings.assertAcknowledged('uploads', forumSettings.ACK_VERSION).ok, true)
assert.equal((await forumSettings.assertAcknowledged('uploads', '0')).ok, false)
assert.equal((await forumSettings.assertAcknowledged('uploads', forumSettings.ACK_VERSION)).ok, true)
})
test('the other two image modes need no acknowledgement', () => {
test('the other two image modes need no acknowledgement', async () => {
// `remote` gets a non-blocking advisory instead: nothing comes to rest on the
// operator's disk, which is the thing the acknowledgement is about.
assert.equal(forumSettings.assertAcknowledged('remote', undefined).ok, true)
assert.equal(forumSettings.assertAcknowledged('disabled', undefined).ok, true)
assert.equal((await forumSettings.assertAcknowledged('remote', undefined)).ok, true)
assert.equal((await forumSettings.assertAcknowledged('disabled', undefined)).ok, true)
})
test('uploads is not a one-way door — a later save needs no fresh acknowledgement', async () => {
// Found on the live rig. A settings form sends every field it owns, so with
// uploads on, unticking "Enable Team forums" re-sends `uploads` and came back
// 400 — the operator could never change a forum setting again, least of all the
// one they would reach for in a hurry.
store.teams_forum_images = 'uploads'
store.teams_forum_uploads_ack = forumSettings.ACK_VERSION
assert.equal((await forumSettings.assertAcknowledged('uploads', undefined)).ok, true)
// Still a gate where consent is genuinely absent: a stale acknowledgement means
// the wording moved, and that DOES need re-consent.
store.teams_forum_uploads_ack = '0'
assert.equal((await forumSettings.assertAcknowledged('uploads', undefined)).ok, false)
// And a transition INTO uploads from another mode still asks.
store.teams_forum_images = 'remote'
store.teams_forum_uploads_ack = forumSettings.ACK_VERSION
assert.equal((await forumSettings.assertAcknowledged('uploads', undefined)).ok, false)
})
test('a reworded notice freezes forum settings but does NOT disable uploads', async () => {
@@ -127,9 +149,12 @@ test('acceptance 3: the stored HTML is identical in every image mode', () => {
const remote = renderForumBody(stored, 'remote')
assert.equal(disabled, stored) // byte-for-byte
assert.match(remote, /<img src="https:\/\/example\.com\/banner\.png"/)
assert.match(remote, /<img [^>]*src="https:\/\/example\.com\/banner\.png"/)
assert.match(remote, /loading="lazy"/)
assert.match(remote, /referrerpolicy="no-referrer"/)
// Carries core's own class, which is what puts the picture BENEATH its link
// (an <img> is inline) and caps it to the column. Found on the live rig.
assert.match(remote, /class="forum-embed"/)
// The link survives in both. A blocked or dead image degrades to the URL the
// author actually wrote.
assert.ok(remote.includes('<a href="https://example.com/banner.png"'))
@@ -159,6 +184,23 @@ test('http URLs and non-image URLs stay plain links', () => {
assert.ok(!notAnImage.includes('<img'))
})
test('an UPLOADED image becomes a picture — the composer’s own path', () => {
// Found on the live rig, not by any unit test here. `uploads` mode hands the
// composer a root-relative path, the composer puts it in the body as TEXT, and
// the renderer only rewrites ANCHORS — so the write path has to produce one, or
// an uploaded image can never render. isEmbeddableImageUrl accepted these paths
// from day one; nothing made an anchor out of them.
const stored = cleanForumBody('<p>/uploads/1787-abc.png</p>')
assert.match(stored, /<a href="\/uploads\/1787-abc\.png"/)
assert.match(renderForumBody(stored, 'uploads'), /<img [^>]*src="\/uploads\/1787-abc\.png"/)
assert.equal(renderForumBody(stored, 'disabled'), stored)
})
test('ordinary prose containing a slash is not turned into a link', () => {
// The upload branch is deliberately narrow — `/uploads/` and nothing else.
assert.ok(!cleanForumBody('<p>meet at /the docks tonight</p>').includes('<a href'))
})
test('a URL inside code or pre is shown, not offered', () => {
const stored = cleanForumBody('<pre>https://example.com/a.png</pre>')
assert.ok(!stored.includes('<a href'))