fix(admin): address Greptile moderation follow-ups from #941

.changeset/admin-moderation-greptile-941.md

@@ -0,0 +1,7 @@
+---
+"nostream": patch
+---
+
+fix(admin): Greptile follow-ups for moderation limits and activity stats
+
+Moderation limit PATCH and ban/unban actions could interleave read–update–save across workers and drop updates. Those paths now run under an async settings write lock that yields while waiting and reclaims stale locks without deleting a fresh holder’s file. Activity stats windows filter on `first_seen` (when the relay first saw the event), not `event_created_at`, with partial indexes on live rows for those queries. IPv6 addresses on the rate-limit bypass list are normalized the same way as connection IPs so bypass rules cannot be skipped by formatting, including when removing entries stored in another spelling. PATCH rejects kind ranges with `start` greater than `end`. PATCH and moderation actions return the normalized limits read back from settings after save.

migrations/20261025_120000_add_moderation_stats_first_seen_indexes.js

@@ -0,0 +1,59 @@
+/**
+ * Supports admin moderation activity stats (queryTopPublishingPubkeys /
+ * queryTopClientIps), which filter live events by relay receipt time:
+ *
+ *   WHERE deleted_at IS NULL AND first_seen >= ?
+ *
+ * Partial indexes keep the working set small on relays where most rows are
+ * soft-deleted history. CREATE INDEX CONCURRENTLY cannot run inside a
+ * transaction.
+ */
+
+exports.config = { transaction: false }
+
+const INDEX_NAMES = [
+  'events_live_first_seen_pubkey_idx',
+  'events_live_first_seen_remote_address_idx',
+]
+
+// A failed concurrent build leaves the index behind as invalid. `IF NOT EXISTS`
+// would then skip it on retry, leaving the migration successful but the index
+// unusable. Drop any invalid leftover before recreating it.
+const dropInvalidIndex = async (knex, indexName) => {
+  const result = await knex.raw(
+    `
+      SELECT 1
+      FROM pg_index
+      WHERE indexrelid = to_regclass(?)
+        AND NOT indisvalid
+    `,
+    [indexName],
+  )
+
+  if (result.rows.length > 0) {
+    await knex.raw('DROP INDEX CONCURRENTLY IF EXISTS ??', [indexName])
+  }
+}
+
+exports.up = async function (knex) {
+  for (const indexName of INDEX_NAMES) {
+    await dropInvalidIndex(knex, indexName)
+  }
+
+  await knex.raw(`
+    CREATE INDEX CONCURRENTLY IF NOT EXISTS events_live_first_seen_pubkey_idx
+    ON events (first_seen, event_pubkey)
+    WHERE deleted_at IS NULL
+  `)
+
+  await knex.raw(`
+    CREATE INDEX CONCURRENTLY IF NOT EXISTS events_live_first_seen_remote_address_idx
+    ON events (first_seen, remote_address)
+    WHERE deleted_at IS NULL AND remote_address IS NOT NULL
+  `)
+}
+
+exports.down = async function (knex) {
+  await knex.raw('DROP INDEX CONCURRENTLY IF EXISTS events_live_first_seen_remote_address_idx')
+  await knex.raw('DROP INDEX CONCURRENTLY IF EXISTS events_live_first_seen_pubkey_idx')
+}

src/controllers/admin/patch-moderation-limits-controller.ts

@@ -21,11 +21,9 @@ export class PatchAdminModerationLimitsController implements IController {
       return
     }
 
-    const next = mergeAdminModerationLimitsPatch(readAdminModerationLimits(), validation.value)
-
     try {
       await saveModerationLimitsFromRequest(
-        next,
+        (current) => mergeAdminModerationLimitsPatch(current, validation.value),
         request,
         this.notificationOutboxRepository,
         'moderation.limits.updated',
@@ -47,6 +45,9 @@ export class PatchAdminModerationLimitsController implements IController {
       throw error
     }
 
-    response.status(200).setHeader('content-type', 'application/json').send({ ok: true, limits: next })
+    response
+      .status(200)
+      .setHeader('content-type', 'application/json')
+      .send({ ok: true, limits: readAdminModerationLimits() })
   }
 }

src/controllers/admin/post-moderation-actions-controller.ts

@@ -22,10 +22,9 @@ export class PostAdminModerationActionsController implements IController {
     }
 
     const { action, pubkey, ip } = validation.value
-    let next
 
     try {
-      next = applyAdminModerationAction(readAdminModerationLimits(), action, { pubkey, ip })
+      applyAdminModerationAction(readAdminModerationLimits(), action, { pubkey, ip })
     } catch (error) {
       response
         .status(400)
@@ -35,11 +34,17 @@ export class PostAdminModerationActionsController implements IController {
     }
 
     try {
-      await saveModerationLimitsFromRequest(next, request, this.notificationOutboxRepository, 'moderation.action', {
-        action,
-        pubkey,
-        ip,
-      })
+      await saveModerationLimitsFromRequest(
+        (current) => applyAdminModerationAction(current, action, { pubkey, ip }),
+        request,
+        this.notificationOutboxRepository,
+        'moderation.action',
+        {
+          action,
+          pubkey,
+          ip,
+        },
+      )
     } catch (error) {
       if (error instanceof ValidationError) {
         response
@@ -57,6 +62,9 @@ export class PostAdminModerationActionsController implements IController {
       throw error
     }
 
-    response.status(200).setHeader('content-type', 'application/json').send({ ok: true, limits: next })
+    response
+      .status(200)
+      .setHeader('content-type', 'application/json')
+      .send({ ok: true, limits: readAdminModerationLimits() })
   }
 }

src/schemas/admin-moderation-schema.ts

@@ -1,6 +1,15 @@
 import { z } from 'zod'
 
-const kindRuleSchema = z.union([z.number().int().min(0).max(65535), z.tuple([z.number().int(), z.number().int()])])
+import { kindSchema } from './base-schema'
+
+// Match event admission (`kindSchema`), which accepts any non-negative integer,
+// so moderation rules can configure and round-trip every kind admission allows.
+const kindRuleSchema = z.union([
+  kindSchema,
+  z.tuple([kindSchema, kindSchema]).refine(([start, end]) => start <= end, {
+    message: 'Kind range start must not exceed its end',
+  }),
+])
 
 export const adminModerationLimitsPatchSchema = z
   .object({

src/utils/admin-moderation-persist.ts

@@ -5,7 +5,11 @@ import { INotificationOutboxRepository } from '../@types/repositories'
 import { createLogger } from '../factories/logger-factory'
 import { appendSettingsAuditLog } from './settings-config'
 import { recordAdminAuditFromRequest } from './record-admin-audit'
-import { AdminModerationLimits, persistAdminModerationLimits, ValidationError } from './admin-moderation'
+import {
+  AdminModerationLimitsUpdater,
+  updateAdminModerationLimits,
+  ValidationError,
+} from './admin-moderation'
 
 const logger = createLogger('admin-moderation-persist')
 
@@ -19,14 +23,14 @@ const MODERATION_SETTINGS_PATHS = [
 ] as const
 
 export const saveModerationLimitsFromRequest = async (
-  limits: AdminModerationLimits,
+  update: AdminModerationLimitsUpdater,
   request: Request,
   notificationOutboxRepository: INotificationOutboxRepository,
   auditAction: 'moderation.limits.updated' | 'moderation.action',
   auditDetail?: Record<string, unknown>,
 ): Promise<void> => {
   try {
-    persistAdminModerationLimits(limits)
+    await updateAdminModerationLimits(update)
   } catch (error) {
     if (error instanceof ValidationError) {
       throw error

src/utils/admin-moderation-stats.ts

@@ -42,6 +42,13 @@ export const parseModerationStatsLimit = (raw: unknown): number => {
   return limit
 }
 
+// `first_seen` is a timestamp without time zone written in UTC. Build the
+// cutoff from the unix timestamp in SQL: a JS Date is sent with the process's
+// local offset, which PostgreSQL ignores for timestamp-without-time-zone
+// comparisons, shifting the window by the difference between the two zones.
+const firstSeenCutoff = (db: Knex, sinceUnixSeconds: number) =>
+  db.raw(`to_timestamp(?) at time zone 'UTC'`, [sinceUnixSeconds])
+
 export const queryTopPublishingPubkeys = async (
   db: Knex,
   sinceUnixSeconds: number,
@@ -51,7 +58,7 @@ export const queryTopPublishingPubkeys = async (
     .select(db.raw(`encode(event_pubkey, 'hex') as pubkey`))
     .count('* as eventCount')
     .whereNull('deleted_at')
-    .where('event_created_at', '>=', sinceUnixSeconds)
+    .where('first_seen', '>=', firstSeenCutoff(db, sinceUnixSeconds))
     .groupBy('event_pubkey')
     .orderBy('eventCount', 'desc')
     .limit(limit)
@@ -73,7 +80,7 @@ export const queryTopClientIps = async (
     .count('* as eventCount')
     .whereNull('deleted_at')
     .whereNotNull('remote_address')
-    .where('event_created_at', '>=', sinceUnixSeconds)
+    .where('first_seen', '>=', firstSeenCutoff(db, sinceUnixSeconds))
     .groupBy('remote_address')
     .orderBy('eventCount', 'desc')
     .limit(limit)

src/utils/admin-moderation.ts

@@ -1,4 +1,4 @@
-import { isIP } from 'node:net'
+import { isIP, SocketAddress } from 'node:net'
 import { mergeDeepRight } from 'ramda'
 
 import { Settings } from '../@types/settings'
@@ -9,6 +9,7 @@ import {
   loadUserSettings,
   saveSettings,
   validateSettings,
+  withSettingsWriteLock,
 } from './settings-config'
 
 export interface AdminModerationLimits {
@@ -53,13 +54,28 @@ const normalizePubkeyList = (values: string[], label: string): string[] => {
   return normalized
 }
 
+export const normalizeModerationIpAddress = (raw: string): string => {
+  const ip = raw.trim()
+  if (!ip || isIP(ip) === 0) {
+    throw new Error('ip must be a valid IPv4 or IPv6 address')
+  }
+
+  const family = isIP(ip)
+  return new SocketAddress({
+    address: ip,
+    family: family === 6 ? 'ipv6' : 'ipv4',
+  }).address
+}
+
 const normalizeIpList = (values: string[], label: string): string[] => {
   const normalized: string[] = []
   const seen = new Set<string>()
 
   for (const raw of values) {
-    const ip = raw.trim()
-    if (!ip || isIP(ip) === 0) {
+    let ip: string
+    try {
+      ip = normalizeModerationIpAddress(raw)
+    } catch {
       throw new Error(`${label} must contain valid IPv4 or IPv6 addresses`)
     }
 
@@ -119,7 +135,9 @@ const applyLimitsToUserSettings = (userSettings: Settings, limits: AdminModerati
   }
 }
 
-export const persistAdminModerationLimits = (next: AdminModerationLimits): void => {
+export type AdminModerationLimitsUpdater = (current: AdminModerationLimits) => AdminModerationLimits
+
+const persistAdminModerationLimitsUnlocked = (next: AdminModerationLimits): void => {
   const normalized: AdminModerationLimits = {
     pubkey: {
       blacklist: normalizePubkeyList(next.pubkey.blacklist, 'pubkey blacklist'),
@@ -147,6 +165,17 @@ export const persistAdminModerationLimits = (next: AdminModerationLimits): void
   saveSettings(nextUserSettings)
 }
 
+export const updateAdminModerationLimits = async (updater: AdminModerationLimitsUpdater): Promise<void> => {
+  await withSettingsWriteLock(() => {
+    const current = readAdminModerationLimits()
+    persistAdminModerationLimitsUnlocked(updater(current))
+  })
+}
+
+export const persistAdminModerationLimits = async (next: AdminModerationLimits): Promise<void> => {
+  await updateAdminModerationLimits(() => next)
+}
+
 export class ValidationError extends Error {
   public constructor(
     message: string,
@@ -183,6 +212,15 @@ export const mergeAdminModerationLimitsPatch = (
 
 const withoutValue = <T>(list: T[], value: T): T[] => list.filter((entry) => entry !== value)
 
+const withoutNormalizedModerationIp = (list: string[], normalizedTarget: string): string[] =>
+  list.filter((stored) => {
+    try {
+      return normalizeModerationIpAddress(stored) !== normalizedTarget
+    } catch {
+      return stored.trim().toLowerCase() !== normalizedTarget.trim().toLowerCase()
+    }
+  })
+
 export const applyAdminModerationAction = (
   current: AdminModerationLimits,
   action: AdminModerationAction,
@@ -248,10 +286,11 @@ export const applyAdminModerationAction = (
       }
     }
     case 'whitelist-ip': {
-      const ip = fields.ip?.trim()
-      if (!ip || isIP(ip) === 0) {
+      const rawIp = fields.ip?.trim()
+      if (!rawIp) {
         throw new Error('ip is required and must be a valid IPv4 or IPv6 address')
       }
+      const ip = normalizeModerationIpAddress(rawIp)
       return {
         ...current,
         rateLimitBypass: {
@@ -261,15 +300,16 @@ export const applyAdminModerationAction = (
       }
     }
     case 'unwhitelist-ip': {
-      const ip = fields.ip?.trim()
-      if (!ip || isIP(ip) === 0) {
+      const rawIp = fields.ip?.trim()
+      if (!rawIp) {
         throw new Error('ip is required and must be a valid IPv4 or IPv6 address')
       }
+      const ip = normalizeModerationIpAddress(rawIp)
       return {
         ...current,
         rateLimitBypass: {
           ...current.rateLimitBypass,
-          ipAddresses: withoutValue(current.rateLimitBypass.ipAddresses, ip),
+          ipAddresses: withoutNormalizedModerationIp(current.rateLimitBypass.ipAddresses, ip),
         },
       }
     }