diff --git a/server/src/router/v1/player/teamForum.router.js b/server/src/router/v1/player/teamForum.router.js index ffc1f40..5038333 100644 --- a/server/src/router/v1/player/teamForum.router.js +++ b/server/src/router/v1/player/teamForum.router.js @@ -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, diff --git a/server/test/teamRoutes.test.js b/server/test/teamRoutes.test.js index 293fc83..f020e22 100644 --- a/server/test/teamRoutes.test.js +++ b/server/test/teamRoutes.test.js @@ -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) + }) +})