fix: hold first posts for review, and promote into groups an operator made (#251)

Two bugs found while checking the deleted administrator reference against
the code for MEI-124. Both are configuration that silently does nothing.

`antispam.moderate_first_posts` asked whether the author bypasses
moderation with `authorizer.can(actor, 'content.viewUnapproved')` and no
target. That action is forum-scoped, so the call threw, the surrounding
catch logged a warning and returned false, and the post was not held. It
failed open for exactly the ordinary new members the setting exists to
catch: administrators and awaiting-activation members never reached the
throw. The line above it in reply-core already passed the scope.

The target is now required, so no caller can omit it — both call sites had
one in hand and the compiler found them. The catch that swallowed this is
split in two: a threshold it cannot read leaves the feature off, which is
what an unset threshold means anyway, while a bypass it cannot resolve
holds the post. Holding a post that did not need holding is undone by a
moderator; publishing spam is not.

Promotion rules ranked an unranked group 0 and treated a lower rank as a
demotion, so every rule into a group the operator created — Registered
ranks 2 — was refused. That is every rule a board would write, since the
seeded groups are guests, registered, moderators and administrators. The
guard now compares only ranks it knows, which loses nothing: a candidate
already in a protected group is skipped before the comparison, so
protectedGroupIds is what keeps staff safe, and a real demotion between
two ranked groups is still refused.

promotion.test.ts passed throughout because its fixture ranks the custom
group; the shipped guards cannot. The regression test is in
packages/runtime against `defaultPromotionGuards()` itself.

The antispam mock now throws for a forum-scoped action with no target, the
way the real authorizer does. Without that it agreed with the bug.

Co-authored-by: Claude <noreply@anthropic.com>

Author: jouwdan

apps/community/src/server/antispam.test.ts

@@ -3,21 +3,33 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 import type { Actor } from '@meith/authorization'
 import { emptyPermissionSet, type PermissionSet } from '@meith/core'
 
+const FORUM_SCOPED = new Set(['content.viewUnapproved'])
+
 const state = vi.hoisted(() => ({
   dataSource: 'postgres' as 'postgres' | 'fixture',
   bypassesFlood: false,
   counts: new Map<string, number>(),
   storeThrows: false,
+  firstPostThreshold: 0,
 }))
 
 vi.mock('./request-fingerprint', () => ({ countingPrefix: async () => null }))
-vi.mock('./settings', () => ({ getSettings: async () => ({ get: () => 0 }) }))
+vi.mock('./settings', () => ({
+  getSettings: async () => ({ get: () => state.firstPostThreshold }),
+}))
 
 vi.mock('./container', () => ({
   getContainer: () => ({
     dataSource: state.dataSource,
     authorizer: {
-      can: () => state.bypassesFlood,
+      can: (_actor: Actor, action: string, target?: unknown) => {
+        if (FORUM_SCOPED.has(action) && target === undefined) {
+          throw new Error(
+            `Forum-scoped action "${action}" requires target.forum (resolved matrix).`,
+          )
+        }
+        return state.bypassesFlood
+      },
       globalLimit: (actor: Actor, key: keyof PermissionSet) => {
         const value = actor.global[key]
         return typeof value === 'number' ? value : 0
@@ -51,7 +63,7 @@ vi.mock('@meith/db', () => ({
   },
 }))
 
-const { dailyLimitMessage, refused, spendDailyLimit } = await import('./antispam')
+const { dailyLimitMessage, holdsNewMember, refused, spendDailyLimit } = await import('./antispam')
 
 function member(over: Partial<PermissionSet>, userId: number | null = 7): Actor {
   return {
@@ -179,3 +191,48 @@ describe('dailyLimitMessage', () => {
     ).toBe('You have used your allowance of private messages for today. It resets in 2 hours.')
   })
 })
+
+describe('holding a new member for review', () => {
+  const FORUM_TARGET = { forum: { id: 1 } } as never
+
+  beforeEach(() => {
+    state.firstPostThreshold = 0
+    state.bypassesFlood = false
+  })
+
+  it('does nothing while the setting is off', async () => {
+    state.firstPostThreshold = 0
+    await expect(
+      holdsNewMember({ actor: member({}), postCount: 0, target: FORUM_TARGET }),
+    ).resolves.toBe(false)
+  })
+
+  it('holds a member under the threshold', async () => {
+    state.firstPostThreshold = 5
+    await expect(
+      holdsNewMember({ actor: member({}), postCount: 1, target: FORUM_TARGET }),
+    ).resolves.toBe(true)
+  })
+
+  it('lets a member past the threshold through', async () => {
+    state.firstPostThreshold = 5
+    await expect(
+      holdsNewMember({ actor: member({}), postCount: 9, target: FORUM_TARGET }),
+    ).resolves.toBe(false)
+  })
+
+  it('lets someone who may see unapproved content through', async () => {
+    state.firstPostThreshold = 5
+    state.bypassesFlood = true
+    await expect(
+      holdsNewMember({ actor: member({}), postCount: 1, target: FORUM_TARGET }),
+    ).resolves.toBe(false)
+  })
+
+  it('holds the post when it cannot tell who bypasses moderation', async () => {
+    state.firstPostThreshold = 5
+    await expect(
+      holdsNewMember({ actor: member({}), postCount: 1, target: undefined as never }),
+    ).resolves.toBe(true)
+  })
+})

apps/community/src/server/antispam.ts

@@ -16,7 +16,7 @@ import {
   type RateLimitOutcome,
   subjectFor,
 } from '@meith/antispam'
-import type { Actor, NumericGlobalPermission } from '@meith/authorization'
+import type { Actor, NumericGlobalPermission, Target } from '@meith/authorization'
 import { env, logger } from '@meith/core'
 import { getDb, PostgresCaptchaQuestionRepository, PostgresRateLimitBucketStore } from '@meith/db'
 import type { SettingsSnapshot } from '@meith/settings'
@@ -280,29 +280,42 @@ export async function verifyChallenge(form: FormData): Promise<ChallengeVerdict>
   }
 }
 
+const FIRST_POST_UNRESOLVED = 'could not resolve first-post moderation'
+
+function bypassesModeration(actor: Actor, target: Target): boolean {
+  try {
+    return getContainer().authorizer.can(actor, 'content.viewUnapproved', target)
+  } catch (error) {
+    logger().warn({ err: String(error), stage: 'bypass', held: true }, FIRST_POST_UNRESOLVED)
+    return false
+  }
+}
+
 export async function holdsNewMember(input: {
   readonly actor: Actor
   readonly postCount: number
+  readonly target: Target
   readonly settings?: SettingsSnapshot
 }): Promise<boolean> {
+  let threshold = 0
   try {
     const settings = input.settings ?? (await getSettings())
-    const threshold = Number(settings.get('antispam.moderate_first_posts') ?? 0)
-    if (threshold <= 0) return false
-
-    const { authorizer } = getContainer()
-    return holdsForReview(
-      {
-        userId: input.actor.userId,
-        postCount: input.postCount,
-        bypassesModeration: authorizer.can(input.actor, 'content.viewUnapproved'),
-      },
-      { threshold },
-    )
+    threshold = Number(settings.get('antispam.moderate_first_posts') ?? 0)
   } catch (error) {
-    logger().warn({ err: String(error) }, 'could not resolve first-post moderation')
+    logger().warn({ err: String(error), stage: 'threshold', held: false }, FIRST_POST_UNRESOLVED)
     return false
   }
+
+  if (threshold <= 0) return false
+
+  return holdsForReview(
+    {
+      userId: input.actor.userId,
+      postCount: input.postCount,
+      bypassesModeration: bypassesModeration(input.actor, input.target),
+    },
+    { threshold },
+  )
 }
 
 export function antispamAvailable(): boolean {

apps/community/src/server/reply-core.ts

@@ -131,6 +131,7 @@ export async function submitReply(
       heldAsNewMember: await holdsNewMember({
         actor,
         postCount: profile.postCount,
+        target: scope,
         settings,
       }),
       requiresApproval: scope.forum.requiresPostApproval === true,

apps/community/src/server/thread-core.ts

@@ -126,7 +126,12 @@ export async function submitThread(
       message: draft.body,
       prefixId: draft.prefixId,
       subscribe: input.subscribe ?? false,
-      heldAsNewMember: await holdsNewMember({ actor, postCount: profile.postCount, settings }),
+      heldAsNewMember: await holdsNewMember({
+        actor,
+        postCount: profile.postCount,
+        target: scope,
+        settings,
+      }),
       requiresApproval: scope.forum.requiresThreadApproval === true,
       ...(input.poll === undefined
         ? {}

packages/groups/src/promotion.ts

@@ -62,8 +62,8 @@ export function evaluatePromotions(
   now: Date = new Date(),
 ): PromotionOutcome[] {
   const protectedIds = new Set(guards.protectedGroupIds)
-  const rankOf = (groupId: number | null): number =>
-    groupId === null ? 0 : (guards.rank?.get(groupId) ?? 0)
+  const rankOf = (groupId: number | null): number | undefined =>
+    groupId === null ? undefined : guards.rank?.get(groupId)
 
   const active = [...rules]
     .filter((r) => r.enabled)
@@ -79,7 +79,9 @@ export function evaluatePromotions(
 
       if (user.primaryGroupId === rule.toPrimaryGroupId) break
 
-      if (rankOf(rule.toPrimaryGroupId) < rankOf(user.primaryGroupId)) break
+      const to = rankOf(rule.toPrimaryGroupId)
+      const from = rankOf(user.primaryGroupId)
+      if (to !== undefined && from !== undefined && to < from) break
 
       outcomes.push({
         userId: user.userId,

packages/runtime/src/task-workers.test.ts

@@ -2,9 +2,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 import { MemoryQueue } from '@meith/drivers'
 import type { OutboxReader, OutboxRecord } from '@meith/events'
+import type { PromotionCandidate, PromotionRule } from '@meith/groups'
+import { evaluatePromotions } from '@meith/groups'
 import type { MarketplaceFeed } from '@meith/marketplace'
 
 import { buildEventRegistry } from './event-handlers'
+import { SEED_GROUP } from './groups'
 import { defaultPromotionGuards, taskWorkers } from './task-workers'
 
 const NOT_ABORTED = new AbortController().signal
@@ -403,3 +406,58 @@ describe('the marketplace catalog refresh', () => {
     expect('refreshMarketplaceCatalog' in workers).toBe(false)
   })
 })
+
+describe('the guards a real board promotes under', () => {
+  const CUSTOM_GROUP = 42
+
+  function candidate(primaryGroupId: number | null): PromotionCandidate {
+    return {
+      userId: 1,
+      primaryGroupId,
+      postCount: 500,
+      reputation: 0,
+      registeredAt: new Date('2020-01-01T00:00:00Z'),
+    }
+  }
+
+  function ruleInto(toPrimaryGroupId: number): PromotionRule {
+    return {
+      id: 1,
+      title: 'Veteran',
+      enabled: true,
+      displayOrder: 0,
+      toPrimaryGroupId,
+      minPostCount: 100,
+    }
+  }
+
+  it('promotes a registered member into a group the operator made', () => {
+    const outcomes = evaluatePromotions(
+      [ruleInto(CUSTOM_GROUP)],
+      [candidate(SEED_GROUP.registered)],
+      defaultPromotionGuards(),
+    )
+
+    expect(outcomes.map((outcome) => outcome.toPrimaryGroupId)).toEqual([CUSTOM_GROUP])
+  })
+
+  it('still refuses a move down a rank it can see', () => {
+    const outcomes = evaluatePromotions(
+      [ruleInto(SEED_GROUP.guest)],
+      [candidate(SEED_GROUP.registered)],
+      defaultPromotionGuards(),
+    )
+
+    expect(outcomes).toEqual([])
+  })
+
+  it('still leaves a protected group alone', () => {
+    const outcomes = evaluatePromotions(
+      [ruleInto(CUSTOM_GROUP)],
+      [candidate(SEED_GROUP.administrators)],
+      defaultPromotionGuards(),
+    )
+
+    expect(outcomes).toEqual([])
+  })
+})