From 673701c91b28897d4164b880c6942a5dfccbea28 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Sat, 31 Oct 2026 00:23:18 -0500 Subject: [PATCH] fix(safety): team notifications skip members who block the author #233 left this open: the forum hid a blocked author's posts from a member, but the member was still pushed, put in the inbox and emailed about them. - teamNotify.forumPost excludes every member who blocks the post's author, or the member who started the thread (the forum 404s that thread for them). One recipient list, so push, the inbox and email skip the same people. userSafety.blockersOf answers it, and a staff author is blocked by nobody (S8). A failed lookup notifies without it. - The daily digest applies each recipient's forumFilter at send time: blocked authors, blocked thread starters, hidden threads and posts. digestPostsSince now also returns the thread's created_by. - A reply's notify payload carries threadAuthorUserId. The Team activity feed needed nothing: forum posts are never written to it (membership, renames and module game events only). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01E14m6SuuY6i1vASFeGDBeY --- server/src/model/safety/userSafety.db.js | 14 +++++ server/src/model/safety/userSafety.model.js | 20 +++++++ server/src/model/teams/teamForum.model.js | 9 ++- server/src/model/teams/teamNotify.db.js | 2 +- .../router/v1/player/teamForum.controller.js | 1 + server/src/utils/teamDigestWorker.js | 13 +++- server/src/utils/teamNotify.js | 33 +++++++++- server/test/displayAuthor.test.js | 5 ++ server/test/teamBridge.test.js | 2 + server/test/teamNotifyDispatch.test.js | 60 +++++++++++++++++++ server/test/userSafety.test.js | 14 +++++ 11 files changed, 167 insertions(+), 6 deletions(-) diff --git a/server/src/model/safety/userSafety.db.js b/server/src/model/safety/userSafety.db.js index c790821..c9ce65e 100644 --- a/server/src/model/safety/userSafety.db.js +++ b/server/src/model/safety/userSafety.db.js @@ -86,6 +86,19 @@ async function convertToAccountBlock(id, blockedUserId) { } } +/** + * Everyone who has blocked this account, as account blocks. The reverse of + * `blocksFor`, for the notification fan-out: a post goes out to a Team, and the + * members who block its author are the ones it must skip. + */ +async function blockersOf(blockedUserId) { + const rows = await query( + 'SELECT blocker_user_id FROM user_blocks WHERE blocked_user_id = ?', + [blockedUserId], + ) + return rows.map((r) => Number(r.blocker_user_id)) +} + /** Undo one of this member's blocks. Scoped to the blocker, so an id is not a capability. */ async function deleteBlock(blockerUserId, id) { const res = await query('DELETE FROM user_blocks WHERE id = ? AND blocker_user_id = ?', [id, blockerUserId]) @@ -152,6 +165,7 @@ module.exports = { insertAccountBlock, insertGameBlock, convertToAccountBlock, + blockersOf, deleteBlock, staffAmong, userById, diff --git a/server/src/model/safety/userSafety.model.js b/server/src/model/safety/userSafety.model.js index 3cf1218..e4962b3 100644 --- a/server/src/model/safety/userSafety.model.js +++ b/server/src/model/safety/userSafety.model.js @@ -280,6 +280,25 @@ async function forModuleViewer(moduleId, userId) { return out } +/** + * The members whose blocks cover this author, for a notification about something + * the author wrote: a forum post is pushed, mailed and put in the inbox of every + * member of its Team, and a member who blocks the author must not be told about it + * (the forum itself already leaves the post out for them). + * + * S8 applies here too: a staff author is blocked by nobody, so this answers empty + * and the post reaches everyone. Answers account blocks only. A game block on the + * author's Steam id that S14 has not yet rewritten counts from the next time its + * owner reads the forum or their blocks, which is when the rewrite happens. + */ +async function blockersOf(authorUserId) { + const id = Number(authorUserId) + if (!Number.isInteger(id) || id <= 0) return [] + const author = await db.userById(id) + if (!author || isStaffRole(author.role)) return [] + return db.blockersOf(id) +} + /** `ctx.safety.staffAmong` (S13): which of these accounts are staff. */ async function staffAmong(userIds) { const ids = [...new Set((Array.isArray(userIds) ? userIds : []) @@ -301,6 +320,7 @@ module.exports = { clearHidden, forumFilter, forModuleViewer, + blockersOf, staffAmong, publicBlock, } diff --git a/server/src/model/teams/teamForum.model.js b/server/src/model/teams/teamForum.model.js index 76d2678..44d0b0d 100644 --- a/server/src/model/teams/teamForum.model.js +++ b/server/src/model/teams/teamForum.model.js @@ -318,7 +318,14 @@ async function createPost({ team, threadId, actor, body }) { // what a recipient needs to know is which conversation moved. `type` is always // 'discussion' here (an announcement takes no replies) and is carried anyway so // the controller has one shape to hand the fan-out from both routes. - return { ok: true, threadId, postId, notify: { threadId, title: thread.title, type: thread.type, bodyHtml: cleaned } } + return { + ok: true, + threadId, + postId, + // `threadAuthorUserId`: a member who blocks whoever started the thread cannot + // open it (leftOutThread), so they are not told about replies in it either. + notify: { threadId, title: thread.title, type: thread.type, bodyHtml: cleaned, threadAuthorUserId: thread.created_by ?? null }, + } } /** diff --git a/server/src/model/teams/teamNotify.db.js b/server/src/model/teams/teamNotify.db.js index 77755f9..8061415 100644 --- a/server/src/model/teams/teamNotify.db.js +++ b/server/src/model/teams/teamNotify.db.js @@ -198,7 +198,7 @@ async function teamsWithForumActivitySince(since) { async function digestPostsSince(teamId, since, limit = 20) { return query( `SELECT po.id, po.thread_id, po.body_html, po.created_at, po.author_user_id, po.author_username, - th.title, th.type + th.title, th.type, th.created_by AS thread_created_by FROM team_forum_posts po JOIN team_forum_threads th ON th.id = po.thread_id WHERE th.team_id = ? diff --git a/server/src/router/v1/player/teamForum.controller.js b/server/src/router/v1/player/teamForum.controller.js index 50c51d0..405a935 100644 --- a/server/src/router/v1/player/teamForum.controller.js +++ b/server/src/router/v1/player/teamForum.controller.js @@ -114,6 +114,7 @@ async function announce(ctx, actor, notify) { threadTitle: notify.title, type: notify.type, authorUserId: actor.id, + threadAuthorUserId: notify.threadAuthorUserId ?? null, authorName: actor.username, bodyHtml: notify.bodyHtml, }) diff --git a/server/src/utils/teamDigestWorker.js b/server/src/utils/teamDigestWorker.js index 43f88b5..72ae47c 100644 --- a/server/src/utils/teamDigestWorker.js +++ b/server/src/utils/teamDigestWorker.js @@ -42,6 +42,7 @@ const templates = require('../engagement/templates') const emailChannel = require('../engagement/emailChannel') const mailer = require('./mailer') const notify = require('./teamNotify') +const safety = require('../model/safety/userSafety.model') const { DELETED_PLAYER } = require('./displayAuthor') const log = require('./logger')('team-digest') @@ -114,7 +115,17 @@ async function digestRule() { async function sendOne(team, recipient, rule, now) { const scopeKey = notify.scopeKey(team) const since = clampSince(recipient.last_digest_at, now) - const posts = await teamNotify.digestPostsSince(team.id, since, MAX_ITEMS) + const all = await teamNotify.digestPostsSince(team.id, since, MAX_ITEMS) + // What the forum would leave out for THIS recipient stays out of their mail too: + // posts by an account they block, posts in a thread whose starter they block, and + // anything they hid (USER_SAFETY.md §4.1). Same filter, read at send time like + // the rest of the digest. + const filter = await safety.forumFilter(recipient.user_id) + const posts = all.filter((p) => + !filter.hidden.team_forum_post.has(Number(p.id)) + && !filter.hidden.team_forum_thread.has(Number(p.thread_id)) + && !(p.author_user_id != null && filter.blockedUserIds.has(Number(p.author_user_id))) + && !(p.thread_created_by != null && filter.blockedUserIds.has(Number(p.thread_created_by)))) // Nothing new for THIS recipient — which is not the same as nothing new for the // Team, because each recipient has their own `since`. No mail, and no stamp: a // stamp here would move the window past posts they have not been told about. diff --git a/server/src/utils/teamNotify.js b/server/src/utils/teamNotify.js index 8e47ea0..ebe4a1e 100644 --- a/server/src/utils/teamNotify.js +++ b/server/src/utils/teamNotify.js @@ -50,6 +50,7 @@ const pushDispatch = require('./pushDispatch') const teamBridge = require('./teamBridge') const teamNotify = require('../model/teams/teamNotify.model') const forumSettings = require('../model/teams/teamForumSettings.model') +const safety = require('../model/safety/userSafety.model') const engagementEmit = require('./engagementEmit') const registries = require('../modules/registries') const log = require('./logger')('team-notify') @@ -131,6 +132,25 @@ function excerpt(html) { return text.length > EXCERPT_CHARS ? `${text.slice(0, EXCERPT_CHARS - 1)}…` : text } +/** + * The members a forum post must skip because they block its author, or the + * author of the thread it is in (USER_SAFETY.md §4.1): the forum leaves both out + * of their view, so a notification would point at something they cannot open. + * + * A failed lookup answers empty and is logged. A notification that reaches one + * member too many is the lesser failure than a forum post that tells nobody. + */ +async function blockedRecipients(...authorUserIds) { + try { + const ids = [...new Set(authorUserIds.filter((id) => id != null).map(Number))] + const lists = await Promise.all(ids.map((id) => safety.blockersOf(id))) + return [...new Set(lists.flat())] + } catch (err) { + log.warn('block lookup failed; notifying without it', { message: err.message }) + return [] + } +} + /** The push half. Resolves recipients, honours mutes and subscriptions, never throws. */ async function tickle(streamId, team, { ref, exclude = [] } = {}) { const userIds = await teamNotify.recipientIds(team.id, { exclude }) @@ -238,7 +258,9 @@ async function leadershipChanged(team, { names = [] } = {}) { * * The author is excluded from the tickle and from the emitted audience. Not as a * nicety — a forum that emails you your own post is the first thing anyone turns - * off, and turning it off costs the deployment every other notification too. + * off, and turning it off costs the deployment every other notification too. So + * is every member who blocks the author or the thread's starter: the forum hides + * the post from them, and the notification follows the forum. * * @returns {{push: number, emitted: boolean, bridged: boolean}} `emitted` says the * event reached the engine, NOT that anybody was mailed. Whether a mail goes out @@ -246,7 +268,7 @@ async function leadershipChanged(team, { names = [] } = {}) { * that reported "3 emails sent" from here would be reporting a decision that has * not been taken yet. The old `emails` count is gone for that reason. */ -async function forumPost({ team, threadId, threadTitle, type, authorUserId, authorName, bodyHtml }) { +async function forumPost({ team, threadId, threadTitle, type, authorUserId, threadAuthorUserId = null, authorName, bodyHtml }) { try { // Belt and braces with the routes, which already 404 when forums are off. The // digest worker has no route in front of it, so the check has to live here as @@ -256,7 +278,12 @@ async function forumPost({ team, threadId, threadTitle, type, authorUserId, auth const announcement = type === 'announcement' const stream = announcement ? STREAMS.ANNOUNCEMENT : STREAMS.FORUM_POST - const exclude = authorUserId ? [authorUserId] : [] + // The author, and every member who blocks the author or the thread's starter. + // One list, so push, the inbox and email all skip the same people. + const exclude = [ + ...(authorUserId ? [authorUserId] : []), + ...(await blockedRecipients(authorUserId, threadAuthorUserId)), + ] const recipientIds = await teamNotify.recipientIds(team.id, { exclude }) const push = await tickle(stream, team, { ref: `team:${team.id}:thread:${threadId}`, exclude }) diff --git a/server/test/displayAuthor.test.js b/server/test/displayAuthor.test.js index d566a73..1fa9b32 100644 --- a/server/test/displayAuthor.test.js +++ b/server/test/displayAuthor.test.js @@ -145,7 +145,12 @@ test('the digest mail names a deleted author "[deleted player]"', async () => { const worker = require('../src/utils/teamDigestWorker') const teamNotify = require('../src/model/teams/teamNotify.model') const templates = require('../src/engagement/templates') + const safety = require('../src/model/safety/userSafety.model') let items = null + patch(safety, 'forumFilter', async () => ({ + blockedUserIds: new Set(), + hidden: { team_forum_thread: new Set(), team_forum_post: new Set() }, + })) patch(teamNotify, 'digestPostsSince', async () => [ { id: 1, thread_id: 5, body_html: '

a

', created_at: new Date(), author_user_id: null, author_username: 'Ravenna', title: 'Raid night', type: 'discussion' }, { id: 2, thread_id: 5, body_html: '

b

', created_at: new Date(), author_user_id: 4, author_username: 'Thorne', title: 'Raid night', type: 'discussion' }, diff --git a/server/test/teamBridge.test.js b/server/test/teamBridge.test.js index 2392cab..c1e2380 100644 --- a/server/test/teamBridge.test.js +++ b/server/test/teamBridge.test.js @@ -28,6 +28,7 @@ const notifyDb = require('../src/model/teams/teamNotify.db') const forumSettings = require('../src/model/teams/teamForumSettings.model') const pushDispatch = require('../src/utils/pushDispatch') const mailer = require('../src/utils/mailer') +const safety = require('../src/model/safety/userSafety.model') const saved = new Map() @@ -59,6 +60,7 @@ beforeEach(() => { patch(notifyDb, 'emailRecipients', async () => []) patch(forumSettings, 'forumsEnabled', async () => true) patch(mailer, 'isConfigured', async () => false) + patch(safety, 'blockersOf', async () => []) }) afterEach(restore) diff --git a/server/test/teamNotifyDispatch.test.js b/server/test/teamNotifyDispatch.test.js index f753a96..f82abab 100644 --- a/server/test/teamNotifyDispatch.test.js +++ b/server/test/teamNotifyDispatch.test.js @@ -34,6 +34,7 @@ const rulesDb = require('../src/model/engagement/engagementRules.db') const sendsDb = require('../src/model/engagement/engagementSends.db') const templates = require('../src/engagement/templates') const registries = require('../src/modules/registries') +const safety = require('../src/model/safety/userSafety.model') const saved = new Map() @@ -71,6 +72,8 @@ function stub({ recipients = [10, 11], emailRows = [], sendOk = true, + blockers = {}, // authorUserId → the members who block them + readerFilter = {}, // userId → { blocked: [...], hiddenPosts: [...], hiddenThreads: [...] } } = {}) { sent = [] emitted = [] @@ -108,6 +111,14 @@ function stub({ // No module registered: the default in most tests, so the link-building ones // have to opt in and the absence is exercised rather than assumed. patch(registries, 'registeredTeamProvider', () => null) + patch(safety, 'blockersOf', async (authorId) => blockers[authorId] || []) + patch(safety, 'forumFilter', async (userId) => { + const f = readerFilter[userId] || {} + return { + blockedUserIds: new Set(f.blocked || []), + hidden: { team_forum_thread: new Set(f.hiddenThreads || []), team_forum_post: new Set(f.hiddenPosts || []) }, + } + }) } beforeEach(() => stub()) @@ -142,6 +153,32 @@ test('the author is not among the tickled, nor among the emitted audience', asyn assert.deepEqual(emitted[0].recipientUserIds, [11]) }) +// USER_SAFETY.md §4.1: the forum leaves a blocked author's posts out of a reader's +// view, so the notification must not point that reader at one. +test('a member who blocks the author is not tickled or told', async () => { + stub({ recipients: [10, 11, 12], blockers: { 10: [12] } }) + await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) + assert.deepEqual(sent[0].userIds, [11]) + assert.deepEqual(emitted[0].recipientUserIds, [11]) +}) + +test('a member who blocks the thread’s starter is not told about replies in it', async () => { + // A reply by 11 in a thread 10 started; 12 blocks 10, so cannot open the thread. + stub({ recipients: [10, 11, 12], blockers: { 10: [12] } }) + await notify.forumPost({ + team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 11, threadAuthorUserId: 10, + }) + assert.deepEqual(sent[0].userIds, [10]) + assert.deepEqual(emitted[0].recipientUserIds, [10]) +}) + +test('a block lookup that fails still notifies everyone else', async () => { + patch(safety, 'blockersOf', async () => { throw new Error('database is on fire') }) + const res = await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 }) + assert.equal(res.push, 1) + assert.deepEqual(emitted[0].recipientUserIds, [11]) +}) + test('roster events fire one tickle for the run, not one per member', async () => { await notify.memberJoined(TEAM) await notify.leadershipChanged(TEAM) @@ -312,6 +349,29 @@ test('the digest renders the rule’s digest template, not its instant one', asy assert.match(mails[0].rendered.subject, /^\[notify\.digest\]/) }) +test('a digest leaves out what the forum leaves out for that reader', async () => { + stub({ readerFilter: { 11: { blocked: [40, 50], hiddenPosts: [3], hiddenThreads: [9] } } }) + stubDigest({ + posts: [ + { id: 1, thread_id: 7, title: 'Raid', body_html: '

kept

', author_user_id: 20, author_username: 'twenty' }, + { id: 2, thread_id: 7, title: 'Raid', body_html: '

blocked author

', author_user_id: 40, author_username: 'forty' }, + { id: 3, thread_id: 7, title: 'Raid', body_html: '

hidden post

', author_user_id: 20, author_username: 'twenty' }, + { id: 4, thread_id: 9, title: 'Hidden', body_html: '

hidden thread

', author_user_id: 20, author_username: 'twenty' }, + { id: 5, thread_id: 8, title: 'Theirs', body_html: '

blocked starter

', author_user_id: 20, author_username: 'twenty', thread_created_by: 50 }, + ], + }) + await digest.tick(new Date()) + assert.deepEqual(mails[0].rendered.values.items.map((i) => i.excerpt), ['kept']) +}) + +test('a digest whose every post is filtered out sends nothing and stamps nothing', async () => { + stub({ readerFilter: { 11: { blocked: [40] } } }) + stubDigest({ posts: [{ id: 2, thread_id: 7, title: 'Raid', body_html: '

x

', author_user_id: 40, author_username: 'forty' }] }) + const res = await digest.tick(new Date()) + assert.equal(res.sent, 0) + assert.equal(world.stamped.length, 0) +}) + test('a recipient with nothing new gets no mail and no stamp', async () => { stubDigest({ posts: [] }) const res = await digest.tick(new Date()) diff --git a/server/test/userSafety.test.js b/server/test/userSafety.test.js index ec79d10..56fea2f 100644 --- a/server/test/userSafety.test.js +++ b/server/test/userSafety.test.js @@ -201,6 +201,20 @@ test('a signed-out reader has no blocks and no hides', async () => { assert.equal((await safety.forumFilter(null)).blockedUserIds.size, 0) }) +test('blockersOf answers who blocks an author, and nobody for a staff author (S8)', async () => { + patch(safetyDb, 'blockersOf', async () => [21, 22]) + patch(safetyDb, 'userById', async (id) => ({ id, username: 'rusty', role: 'player' })) + assert.deepEqual(await safety.blockersOf(12), [21, 22]) + + patch(safetyDb, 'userById', async (id) => ({ id, username: 'mod', role: 'moderator' })) + assert.deepEqual(await safety.blockersOf(2), []) + + // A deleted account or no author at all: nothing to look up. + patch(safetyDb, 'userById', async () => null) + assert.deepEqual(await safety.blockersOf(99), []) + assert.deepEqual(await safety.blockersOf(null), []) +}) + test('staffAmong takes only positive integer ids', async () => { let asked patch(safetyDb, 'staffAmong', async (ids) => { asked = ids; return [] }) -- 2.49.1