fix(teams): let a post-moderation mistake reach the model that explains it
All checks were successful
PR Checks / bot-install (pull_request) Successful in 19s
PR Checks / client-build (pull_request) Successful in 30s
PR Checks / server-tests (pull_request) Successful in 34s

Found on the live rig. `moderatePost` answers `pin` with «"pin" applies to a
thread, not to a post» and an invented action with "Unknown moderation action" —
the distinction exists because they are different mistakes and a caller who made
the first one has a bug worth naming precisely.

The route's validator listed only the four actions a post accepts, so `pin` never
got there: it came back as a generic "Validation failed". The precise message was
written, documented, unit-tested — and unreachable through the API, which is the
worst of both, because the branch reads as live code and is only exercised by its
own test.

The validator now lists all eight and lets the model discriminate. Both answers
are 400, neither is a security boundary, and widening the list is not removing it
— an action outside the enum still stops at the validator, which the added route
test asserts alongside the `pin` case.

Nothing else the walk exercised needed changing. The whole phase 5 surface was
driven against a real server, real MariaDB and real sessions across four
identities — an ordinary member, a granted non-member guest, a Team leader and a
staffer — plus a browser pass over the forum panel, the reports queue, the
per-Team forum ledger and the settings screen. Notably confirmed live: a locked
thread refuses replies from all four identities at 409; a hidden post renders for
the leader and staff with Unhide and **no Edit control for anyone**; the report
queue answers 200 to staff and 403 to the leader, the member and the guest alike;
and turning the edit window down to 0 stops the author while leaving staff
unbounded.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-08-18 13:25:21 -05:00
parent 3f7e61af1c
commit c970caee16
2 changed files with 34 additions and 1 deletions

View File

@@ -176,7 +176,15 @@ forumRouter.post(
/* #swagger.responses[400] = { description: 'An action that applies to a thread, not a post', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */
/* #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(['hide', 'unhide', 'delete', 'restore']),
// **Deliberately the FULL action list, not the four a post accepts.** The model
// answers `pin` with "that applies to a thread, not to a post" and an invented
// action with "unknown", and a validator that allowed only the four would turn
// the first of those into a generic "Validation failed" — leaving the precise
// message reachable only from a unit test. Found on the live rig, where `pin`
// came back as a validation error rather than as the sentence written for it.
// Both are 400 and neither is a security boundary; the difference is entirely
// whether the caller is told which mistake they made.
body('action').isIn(['pin', 'unpin', 'lock', 'unlock', 'hide', 'unhide', 'delete', 'restore']),
body('reason').optional().isString().trim().isLength({ max: 255 }),
validate,
ctrl.moderatePost,

View File

@@ -495,3 +495,28 @@ test('the grant routes answer even while the forum is switched off', async () =>
assert.equal((await get(app, '/api/v1/player/teams/a/grants')).status, 200)
})
})
test('pin on a POST reaches the model, so the caller is told which mistake they made', async () => {
// The route's validator deliberately accepts all eight actions. Narrowing it to
// the four a post takes would turn "that applies to a thread, not to a post"
// into a generic "Validation failed" — the precise message would exist, be
// unit-tested, and be unreachable through the API. Found on the live rig.
signInAs(admin)
patch(forumSettings, 'forumsEnabled', async () => true)
patch(teamsDbModule, 'findBySlug', async () => ({ id: 1, name: 'A' }))
patch(access, 'forumAccess', async () => ({ allowed: true, viaMembership: true, viaGrant: false, isLeader: false }))
patch(forum, 'moderatePost', async ({ action }) => ({
ok: false, status: 400, error: `"${action}" applies to a thread, not to a post`,
}))
await withApp('/api/v1/player', playerRouter, async (app) => {
const res = await post(app, '/api/v1/player/teams/a/forum/posts/1/moderate', { action: 'pin' })
assert.equal(res.status, 400)
assert.match((await res.json()).message, /applies to a thread/)
// An action that is not in the enum at all still stops at the validator —
// widening the list is not the same as removing it.
const nonsense = await post(app, '/api/v1/player/teams/a/forum/posts/1/moderate', { action: 'incinerate' })
assert.equal(nonsense.status, 400)
})
})