From 814f205da0784c3eef1fe909b734cfb193707d38 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Tue, 1 Sep 2026 18:50:12 +0200 Subject: [PATCH] fix(feedback): make the "block" severity tier actually reject The block level is advertised as "comment is rejected immediately", but every non-approved comment was saved with is_approved = false instead of the submission being refused. moderateText now sets an explicit `blocked: true` on the blocking-violation branch -- branching on the reason string in the route would have been fragile -- and the route 400s with code COMMENT_BLOCKED and stores nothing. Everything else that is not approved (moderate/high, the spam and caps checks, and the "Moderation system error" fallback) deliberately omits the flag and keeps the held-for-moderation path, so a moderation failure still fails safe. Also fixes an adjacent defect that made the tier split unobservable: feedbackService.submitFeedback ignored feedbackData.is_approved entirely and hard-derived is_approved from moderate_comments. So a moderate/high word-filter hit on an event with moderation switched OFF was published immediately -- the route's `feedbackData.is_approved = false` was dead code. Now honoured one-directionally: a caller-supplied false is respected, but nothing a caller passes can RELAX the event's setting. That deliberately leaves the route's reputation.autoApprove -> is_approved = true branch inert rather than letting a trusted guest bypass an event's moderation setting. Refs testplan REPORT.md B11. (cherry picked from commit b1b57b1615aaf02fe76e789a86b7e11933288d77) --- .../routes/feedbackBlockSeverity.test.js | 119 ++++++++++++++++++ backend/src/routes/galleryFeedback.js | 15 +++ backend/src/services/feedbackModeration.js | 7 ++ backend/src/services/feedbackService.js | 9 +- 4 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 backend/__tests__/routes/feedbackBlockSeverity.test.js diff --git a/backend/__tests__/routes/feedbackBlockSeverity.test.js b/backend/__tests__/routes/feedbackBlockSeverity.test.js new file mode 100644 index 00000000..281a4c2a --- /dev/null +++ b/backend/__tests__/routes/feedbackBlockSeverity.test.js @@ -0,0 +1,119 @@ +/** + * Word-filter severity tiers, at the submission route. + * + * The Settings → Moderation UI advertises `block` as "comment is rejected + * immediately", but the submit route saved every non-approved comment with + * is_approved = false — identical handling to `moderate`/`high`. So the + * strongest tier stored the prohibited text anyway and only hid it from the + * public list. + * + * These three cases pin the tiers apart: + * block → 4xx, nothing written + * moderate / high → 201, stored held-for-moderation (is_approved = false) + * low → 201, stored approved (flag-only) + */ + +const request = require('supertest'); +const express = require('express'); +const cookieParser = require('cookie-parser'); +const jwt = require('jsonwebtoken'); + +const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb'); + +process.env.JWT_SECRET = process.env.JWT_SECRET || 'block-severity-secret'; + +const SLUG = 'block-severity'; + +describe('word-filter severity tiers at submission (#B11)', () => { + let db; let cleanup; let app; + let eventId; let photoId; + + const galleryToken = () => jwt.sign( + { eventId, eventSlug: SLUG, type: 'gallery' }, + process.env.JWT_SECRET, + { expiresIn: '1h', issuer: 'picpeak-auth' } + ); + + const comment = (text) => request(app) + .post(`/api/gallery/${SLUG}/photos/${photoId}/feedback`) + .set('Authorization', `Bearer ${galleryToken()}`) + .send({ feedback_type: 'comment', comment_text: text }); + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + await seedMinimal(db); + + const [ev] = await db('events').insert({ + slug: SLUG, + event_type: 'wedding', + event_name: 'Block Severity', + event_date: '2026-08-01', + host_email: 'h@example.com', + admin_email: 'a@example.com', + password_hash: 'x', + share_link: `/gallery/${SLUG}/share`, + share_token: 'block-severity-share', + expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(), + is_active: 1, is_archived: 0, is_draft: 0, + created_at: new Date().toISOString(), + }).returning('id'); + eventId = typeof ev === 'object' ? ev.id : ev; + + const [p] = await db('photos').insert({ + event_id: eventId, filename: 'shot.jpg', path: `events/${SLUG}/shot.jpg`, + type: 'individual', uploaded_at: new Date().toISOString(), + }).returning('id'); + photoId = typeof p === 'object' ? p.id : p; + + await db('event_feedback_settings').insert({ + event_id: eventId, feedback_enabled: true, allow_comments: true, + moderate_comments: false, require_name_email: false, + show_feedback_to_guests: true, + }); + + await db('feedback_word_filters').insert([ + { word: 'zzblocked', severity: 'block', is_active: true, created_at: new Date().toISOString() }, + { word: 'zzmoderated', severity: 'moderate', is_active: true, created_at: new Date().toISOString() }, + { word: 'zzmild', severity: 'low', is_active: true, created_at: new Date().toISOString() }, + ]); + require('../../src/services/feedbackModeration').clearCache(); + + app = express(); + app.use(express.json()); + app.use(cookieParser()); + app.use('/api/gallery', require('../../src/routes/galleryFeedback')); + }, 180000); + + afterAll(async () => { if (cleanup) await cleanup(); }); + + beforeEach(async () => { + await db('photo_feedback').where({ photo_id: photoId }).del(); + }); + + it('rejects a "block" match outright and stores nothing', async () => { + const res = await comment('this is zzblocked content'); + + expect(res.status).toBe(400); + expect(res.body.code).toBe('COMMENT_BLOCKED'); + expect(await db('photo_feedback').where({ photo_id: photoId })).toHaveLength(0); + }); + + it('still holds a "moderate" match for moderation', async () => { + const res = await comment('this is zzmoderated content'); + + expect(res.status).toBeLessThan(400); + const rows = await db('photo_feedback').where({ photo_id: photoId }); + expect(rows).toHaveLength(1); + expect([false, 0]).toContain(rows[0].is_approved); + expect(rows[0].comment_text).toContain('zzmoderated'); + }); + + it('lets a "low" match through approved (flag only)', async () => { + const res = await comment('this is zzmild content'); + + expect(res.status).toBeLessThan(400); + const rows = await db('photo_feedback').where({ photo_id: photoId }); + expect(rows).toHaveLength(1); + expect([true, 1]).toContain(rows[0].is_approved); + }); +}); diff --git a/backend/src/routes/galleryFeedback.js b/backend/src/routes/galleryFeedback.js index d756e816..ec4d6bff 100644 --- a/backend/src/routes/galleryFeedback.js +++ b/backend/src/routes/galleryFeedback.js @@ -295,6 +295,21 @@ router.post('/:slug/photos/:photoId/feedback', // Moderate the comment const moderationResult = await feedbackModeration.moderateText(req.body.comment_text); + if (moderationResult.blocked) { + // `block` severity means rejected outright — never stored, not even + // as a pending row for a moderator to see. Anything else that isn't + // approved falls through to the held-for-moderation branch below. + logger.warn('Comment rejected by word filter:', { + eventId: event.id, + reason: moderationResult.reason, + violations: moderationResult.violations + }); + return res.status(400).json({ + error: 'Your comment contains words that are not allowed here.', + code: 'COMMENT_BLOCKED' + }); + } + if (!moderationResult.approved) { // Still save but mark as not approved feedbackData.is_approved = false; diff --git a/backend/src/services/feedbackModeration.js b/backend/src/services/feedbackModeration.js index df9babb0..7c5cfc88 100644 --- a/backend/src/services/feedbackModeration.js +++ b/backend/src/services/feedbackModeration.js @@ -71,8 +71,15 @@ class FeedbackModerationService { // low/moderate/high/block levels — rows stored under it still apply. const isBlocking = (v) => v.severity === 'block' || v.severity === 'severe'; if (violations.some(isBlocking)) { + // `blocked` is the flag the submit route branches on: the `block` + // tier is advertised as "comment is rejected immediately", so it 4xxs + // the submission instead of storing it for a moderator. Every other + // not-approved outcome (moderate/high, spam checks, and the + // moderation-system-error fallback below) deliberately omits it and + // keeps the held-for-moderation behaviour. return { approved: false, + blocked: true, reason: 'Content contains prohibited words', violations: violations.filter(isBlocking) }; diff --git a/backend/src/services/feedbackService.js b/backend/src/services/feedbackService.js index 09859a94..d8269b3f 100644 --- a/backend/src/services/feedbackService.js +++ b/backend/src/services/feedbackService.js @@ -645,7 +645,14 @@ class FeedbackService { guest_id: guest_id || null, ip_address, user_agent, - is_approved: feedback_type !== 'comment' || !feedbackData.moderate_comments, + // The submit route can force a comment into moderation (a + // `moderate`/`high` word-filter hit) on an event whose + // moderate_comments is off — this line used to ignore that entirely, + // so those hits published straight away. A caller-supplied `false` is + // honoured; nothing a caller passes can RELAX the event's setting. + is_approved: feedbackData.is_approved === false + ? false + : (feedback_type !== 'comment' || !feedbackData.moderate_comments), created_at: new Date(), updated_at: new Date() }).returning('id');