fix(moderation): log the moderation the audit trail claimed it already had

apps/community/src/server/moderation-actions.test.ts

@@ -31,7 +31,20 @@ vi.mock('next/navigation', () => ({
 const actorRef: { current: Actor | null } = { current: null }
 vi.mock('./context', () => ({ getActor: async () => actorRef.current }))
 
-const { moderateQueueAction } = await import('./moderation-actions')
+const adminCalls: Array<{ action: string; detail: unknown }> = []
+vi.mock('./admin', () => ({
+  recordAdminAction: async (input: { action: string; detail?: unknown }) => {
+    adminCalls.push({ action: input.action, detail: input.detail })
+  },
+}))
+
+const avatarLocks: Array<{ userId: number; locked: boolean; reason: string }> = []
+const avatarServiceRef: { current: unknown } = { current: null }
+vi.mock('./avatars', () => ({ avatarService: () => avatarServiceRef.current }))
+
+const { moderateQueueAction, setAvatarLockAction, setSignatureLockAction } = await import(
+  './moderation-actions'
+)
 const { EMPTY_STATE } = await import('./auth-form-state')
 const { SEED_BOARD, SEED_FORUM, SEED_GROUP } = await import('./seed-board')
 
@@ -103,9 +116,24 @@ const SELECTION: Array<[string, string]> = [
   ['item', 'post:20'],
 ]
 
+const signatureLocks: Array<{ userId: number; locked: boolean; reason: string | null }> = []
+
+async function warnerActor(userId: number): Promise<Actor> {
+  const actor = await actorFor(SEED_GROUP.superModerators, userId)
+  return { ...actor, global: { ...actor.global, canWarnUsers: true } }
+}
+
 beforeEach(async () => {
   queue = new FakeQueue()
   actorRef.current = await actorFor(SEED_GROUP.superModerators, 2)
+  adminCalls.length = 0
+  signatureLocks.length = 0
+  avatarLocks.length = 0
+  avatarServiceRef.current = {
+    setLock: async (input: { userId: number; locked: boolean; reason: string }) => {
+      avatarLocks.push(input)
+    },
+  }
   installContainer()
 })
 
@@ -211,3 +239,89 @@ describe('moderateQueueAction', () => {
     expect(queue.applied).toHaveLength(0)
   })
 })
+
+describe('locking what a member displays', () => {
+  beforeEach(async () => {
+    actorRef.current = await warnerActor(2)
+    installContainer({
+      signatures: {
+        setLocked: async (input: {
+          userId: number
+          locked: boolean
+          reason: string | null
+        }) => {
+          signatureLocks.push(input)
+        },
+      },
+    })
+  })
+
+  it('records a signature lock against the member it was applied to', async () => {
+    await redirectOf(
+      setSignatureLockAction(
+        EMPTY_STATE,
+        form([['userId', '7'], ['locked', '1'], ['reason', 'advertising']]),
+      ),
+    )
+
+    expect(signatureLocks[0]).toMatchObject({ userId: 7, locked: true })
+    expect(adminCalls).toEqual([{ action: 'signature.lock', detail: { userId: 7 } }])
+  })
+
+  it('records the release as its own action, not as the lock again', async () => {
+    await redirectOf(
+      setSignatureLockAction(EMPTY_STATE, form([['userId', '7'], ['locked', '0']])),
+    )
+
+    expect(adminCalls).toEqual([{ action: 'signature.unlock', detail: { userId: 7 } }])
+  })
+
+  it('records an avatar lock and its release the same way', async () => {
+    await redirectOf(
+      setAvatarLockAction(
+        EMPTY_STATE,
+        form([['userId', '7'], ['locked', '1'], ['reason', 'obscene']]),
+      ),
+    )
+    await redirectOf(
+      setAvatarLockAction(EMPTY_STATE, form([['userId', '7'], ['locked', '0']])),
+    )
+
+    expect(avatarLocks).toHaveLength(2)
+    expect(adminCalls.map((call) => call.action)).toEqual(['avatar.lock', 'avatar.unlock'])
+  })
+
+  it('keeps the reason the member is shown out of the log', async () => {
+    await redirectOf(
+      setSignatureLockAction(
+        EMPTY_STATE,
+        form([['userId', '7'], ['locked', '1'], ['reason', 'advertising a rival board']]),
+      ),
+    )
+
+    expect(JSON.stringify(adminCalls)).not.toContain('rival board')
+  })
+
+  it('logs nothing when the lock was refused', async () => {
+    actorRef.current = await actorFor(SEED_GROUP.registered, 3)
+
+    const state = await setSignatureLockAction(
+      EMPTY_STATE,
+      form([['userId', '7'], ['locked', '1'], ['reason', 'advertising']]),
+    )
+
+    expect(state.error).toBeTruthy()
+    expect(signatureLocks).toEqual([])
+    expect(adminCalls).toEqual([])
+  })
+
+  it('logs nothing when the lock was rejected for having no reason', async () => {
+    const state = await setSignatureLockAction(
+      EMPTY_STATE,
+      form([['userId', '7'], ['locked', '1']]),
+    )
+
+    expect(state.error).toMatch(/say why/i)
+    expect(adminCalls).toEqual([])
+  })
+})

apps/community/src/server/moderation-actions.ts

@@ -5,6 +5,7 @@ import { redirect } from 'next/navigation'
 import { ForbiddenError, ValidationError } from '@meith/core'
 import { ModerationQueue, parseSelection } from '@meith/moderation'
 
+import { recordAdminAction } from './admin'
 import { avatarService } from './avatars'
 import { getActor } from './context'
 import { getContainer } from './container'
@@ -106,6 +107,10 @@ export async function setSignatureLockAction(
     }
 
     await store.setLocked({ userId, locked, reason: locked ? reason : null })
+    await recordAdminAction({
+      action: locked ? 'signature.lock' : 'signature.unlock',
+      detail: { userId },
+    })
   } catch (err) {
     return toFormState(err)
   }
@@ -136,10 +141,11 @@ export async function setAvatarLockAction(
       throw new ValidationError('No such member.')
     }
 
-    await service.setLock({
-      userId,
-      locked: form.get('locked') === '1',
-      reason: text('reason'),
+    const locked = form.get('locked') === '1'
+    await service.setLock({ userId, locked, reason: text('reason') })
+    await recordAdminAction({
+      action: locked ? 'avatar.lock' : 'avatar.unlock',
+      detail: { userId },
     })
   } catch (err) {
     return toFormState(err)

apps/community/src/server/user-admin-actions.test.ts

@@ -503,6 +503,19 @@ describe('mass mail', () => {
     expect(state.values?.queued).toBe('7')
   })
 
+  it('records every further batch, so a campaign is not one row and then silence', async () => {
+    claimResult.current = {
+      recipients: [{ userId: 3, email: 'c@example.test', username: 'cal' }],
+      finished: false,
+    }
+
+    await continueMassMailAction({}, form({ massMailId: '55' }))
+
+    expect(adminCalls).toEqual([
+      { action: 'user.mass_mail_continued', detail: { massMailId: 55, sent: 1, queued: 7 } },
+    ])
+  })
+
   it('refuses to continue a campaign that is not one', async () => {
     const state = await continueMassMailAction({}, form({ massMailId: 'latest' }))
     expect(state.error).toBeDefined()

apps/community/src/server/user-admin-actions.ts

@@ -273,7 +273,7 @@ export async function startMassMailAction(
       detail: { massMailId, targetGroupId },
     })
 
-    return queueMassMailBatch(bulk, massMailId)
+    return (await queueMassMailBatch(bulk, massMailId)).state
   } catch (err) {
     return toFormState(err)
   }
@@ -291,7 +291,13 @@ export async function continueMassMailAction(
       throw new ValidationError('No such message.')
     }
 
-    return queueMassMailBatch(requireUserBulk(), massMailId)
+    const batch = await queueMassMailBatch(requireUserBulk(), massMailId)
+    await recordAdminAction({
+      action: 'user.mass_mail_continued',
+      detail: { massMailId, sent: batch.sent, queued: batch.queued },
+    })
+
+    return batch.state
   } catch (err) {
     return toFormState(err)
   }
@@ -300,7 +306,7 @@ export async function continueMassMailAction(
 async function queueMassMailBatch(
   bulk: ReturnType<typeof requireUserBulk>,
   massMailId: number,
-): Promise<FormState> {
+): Promise<{ state: FormState; sent: number; queued: number }> {
   const chunk = await bulk.claimMassMailChunk(massMailId, MASS_MAIL_CHUNK)
 
   for (const recipient of chunk.recipients) {
@@ -314,8 +320,12 @@ async function queueMassMailBatch(
   const total = (await bulk.readMassMail(massMailId))?.queuedCount ?? 0
 
   return {
-    notice: chunk.finished ? 'sent' : 'more',
-    values: { massMailId: String(massMailId), queued: String(total) },
+    state: {
+      notice: chunk.finished ? 'sent' : 'more',
+      values: { massMailId: String(massMailId), queued: String(total) },
+    },
+    sent: chunk.recipients.length,
+    queued: total,
   }
 }
 

docs/mybb-parity.md

@@ -568,6 +568,49 @@ action into a moderator-visible disclosure the day somebody forgets to update
 it, whereas an allow-list turns a new moderation action into a missing row
 somebody notices.
 
+### Everything that changes something is logged, and nothing that does not
+
+**MyBB:** the moderator log records what the moderation tools do. Handling a
+report, deleting one post from the thread it is in, or editing somebody else's
+post leaves nothing behind in it.
+
+**Here:** every path that changes content, a member's presentation or a report
+writes a row, whichever screen it was reached from. Closing a report writes
+`report.resolve` or `report.reject`; deleting or restoring a single post from
+the postbit writes `post.delete` or `post.restore`; editing a post somebody else
+wrote writes `post.edit`; locking a signature or an avatar writes
+`signature.lock`/`signature.unlock` or `avatar.lock`/`avatar.unlock`. Copying a
+thread already wrote `thread.copy` and now appears in the ModCP, because the
+action is in the allow-list the reader filters by. Every further batch of a mass
+mail writes `user.mass_mail_continued`, so a campaign is not one row followed by
+silence for the next several thousand recipients.
+
+**Why:** `/admin/log` says it holds "every administrative and moderation action"
+and the ModCP offers itself as the record of a forum. A log that is only mostly
+complete is worse than one that admits a boundary, because the missing row reads
+as "it did not happen". The rows that concern a forum carry the same
+`forumIds`/`forumId` scope keys as every other moderation row, so they reach the
+moderators of the forum they happened in and no one else.
+
+Where the change is a database write, the row is written in the transaction that
+makes it — a moderation that rolls back leaves no row claiming it happened. The
+two that are not database writes of their own, the signature and avatar locks,
+go through the same helper as the rest of the control panel and so also record
+the address the moderator acted from.
+
+**The boundary is authorship, and it is deliberate.** A member deleting or
+editing their own post writes nothing: it is not moderation, and logging it
+would bury the moderation in it. The row is written when the actor is not the
+post's author, which is the same question the postbit asks to decide whether it
+is showing a moderator's button. Taking a report or putting it back is not
+logged either — it moves nothing, and the report's own timeline already shows
+who holds it.
+
+**Cost.** A forum whose moderators edit heavily has a longer log than MyBB's,
+and every entry names a post rather than only a thread. The moderator log has no
+retention policy, so `admin_log` grows with moderation rather than with
+administration alone.
+
 ### Every log row names the forums it concerns, at the moment it is written
 
 **MyBB:** the moderator log carries an `fid` column, and the ModCP scopes the

docs/operating.md

@@ -1340,6 +1340,22 @@ recorded, so every action the board has ever logged can be filtered on. The
 dropdown is what the log contains, not a fixed list — an action nobody has
 performed yet is not offered, and appears the first time it happens.
 
+### What reaches the admin log
+
+`/admin/log` is the whole table. Administrative actions and moderation actions
+share it, so the control panel's log is the superset and the ModCP's is the same
+rows filtered to moderation actions in the forums a moderator holds — see
+[Everything that changes something is logged](./mybb-parity.md#everything-that-changes-something-is-logged-and-nothing-that-does-not)
+for what qualifies.
+
+Two things are on the screen but not in the table, and knowing which is which
+saves an investigation. A member editing or deleting **their own** post writes
+no row; the row appears when somebody else does it to them. And a report's
+assignment — a moderator taking it, or putting it back — is on the report's own
+timeline rather than in the log, because it changes nothing about the board.
+Everything else a moderator or an administrator does leaves a row, including
+each 500-recipient batch of a mass mail.
+
 ## Migrations
 
 Migrations are **forward-only**. There is no down migration and there will not be

packages/db/src/modcp-repo.test.ts

@@ -244,6 +244,64 @@ describe('the moderator log', () => {
     ).toHaveLength(1)
   })
 
+  it('shows a copy to the moderators of the forum it came from and the one it landed in', async () => {
+    await logRow(
+      'thread.copy',
+      {
+        threadId: 7,
+        fromForumId: THEIRS,
+        toForumId: MINE,
+        forumIds: [THEIRS, MINE],
+        newThreadId: 9,
+        posts: 2,
+      },
+      OTHER_MOD,
+    )
+
+    expect(
+      (await repo.log({ forumIds: [MINE], actorUserId: MOD, limit: 10 })).entries,
+    ).toHaveLength(1)
+    expect(
+      (await repo.log({ forumIds: [THEIRS], actorUserId: IVAN, limit: 10 })).entries,
+    ).toHaveLength(1)
+  })
+
+  it('shows a closed report to everyone who moderates the forum it was filed in', async () => {
+    await logRow('report.resolve', { reportId: 3, forumId: MINE, forumIds: [MINE] }, OTHER_MOD)
+
+    const entry = (await repo.log({ forumIds: [MINE], actorUserId: MOD, limit: 10 }))
+      .entries[0]!
+    expect(entry.action).toBe('report.resolve')
+    expect(entry.forumTitle).toBe('Mine')
+    expect(entry.detail).toContainEqual({ label: 'Report', value: '3' })
+  })
+
+  it('shows a single-post deletion to the forum the post was in', async () => {
+    await logRow(
+      'post.delete',
+      { postId: 11, threadId: 7, forumId: MINE, forumIds: [MINE] },
+      OTHER_MOD,
+    )
+
+    expect(
+      (await repo.log({ forumIds: [MINE], actorUserId: MOD, limit: 10 })).entries,
+    ).toHaveLength(1)
+    expect(
+      (await repo.log({ forumIds: [THEIRS], actorUserId: IVAN, limit: 10 })).entries,
+    ).toEqual([])
+  })
+
+  it('shows a signature lock to the moderator who set it and to nobody else', async () => {
+    await logRow('signature.lock', { userId: IVAN }, OTHER_MOD)
+
+    expect((await repo.log({ forumIds: [MINE], actorUserId: MOD, limit: 10 })).entries).toEqual(
+      [],
+    )
+    const own = await repo.log({ forumIds: [], actorUserId: OTHER_MOD, limit: 10 })
+    expect(own.entries[0]).toMatchObject({ action: 'signature.lock' })
+    expect(own.entries[0]!.detail).toContainEqual({ label: 'Member', value: String(IVAN) })
+  })
+
   it('shows a forum-less entry only to the moderator who wrote it', async () => {
     await logRow('warning.issue', { userId: IVAN, points: 2 }, OTHER_MOD)