diff --git a/.changeset/admin-moderation-greptile-941.md b/.changeset/admin-moderation-greptile-941.md new file mode 100644 index 000000000..1a37e8b40 --- /dev/null +++ b/.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. diff --git a/migrations/20261025_120000_add_moderation_stats_first_seen_indexes.js b/migrations/20261025_120000_add_moderation_stats_first_seen_indexes.js new file mode 100644 index 000000000..f6d7a3db5 --- /dev/null +++ b/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') +} diff --git a/src/controllers/admin/patch-moderation-limits-controller.ts b/src/controllers/admin/patch-moderation-limits-controller.ts index 415991af5..8d0c9384a 100644 --- a/src/controllers/admin/patch-moderation-limits-controller.ts +++ b/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() }) } } diff --git a/src/controllers/admin/post-moderation-actions-controller.ts b/src/controllers/admin/post-moderation-actions-controller.ts index f0af6d9c2..bee566dd2 100644 --- a/src/controllers/admin/post-moderation-actions-controller.ts +++ b/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() }) } } diff --git a/src/schemas/admin-moderation-schema.ts b/src/schemas/admin-moderation-schema.ts index fe51155fd..dc7212f17 100644 --- a/src/schemas/admin-moderation-schema.ts +++ b/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({ diff --git a/src/utils/admin-moderation-persist.ts b/src/utils/admin-moderation-persist.ts index ddfcc3dc5..625e86a4a 100644 --- a/src/utils/admin-moderation-persist.ts +++ b/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, ): Promise => { try { - persistAdminModerationLimits(limits) + await updateAdminModerationLimits(update) } catch (error) { if (error instanceof ValidationError) { throw error diff --git a/src/utils/admin-moderation-stats.ts b/src/utils/admin-moderation-stats.ts index 97cbed21e..a1fb30fec 100644 --- a/src/utils/admin-moderation-stats.ts +++ b/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) diff --git a/src/utils/admin-moderation.ts b/src/utils/admin-moderation.ts index 9020cb8fd..d55c1be51 100644 --- a/src/utils/admin-moderation.ts +++ b/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() 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 => { + await withSettingsWriteLock(() => { + const current = readAdminModerationLimits() + persistAdminModerationLimitsUnlocked(updater(current)) + }) +} + +export const persistAdminModerationLimits = async (next: AdminModerationLimits): Promise => { + await updateAdminModerationLimits(() => next) +} + export class ValidationError extends Error { public constructor( message: string, @@ -183,6 +212,15 @@ export const mergeAdminModerationLimitsPatch = ( const withoutValue = (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), }, } } diff --git a/src/utils/settings-config.ts b/src/utils/settings-config.ts index d80201792..c0421c398 100644 --- a/src/utils/settings-config.ts +++ b/src/utils/settings-config.ts @@ -1,4 +1,5 @@ import fs from 'fs' +import { setTimeout as delay } from 'node:timers/promises' import { join } from 'path' import yaml from 'js-yaml' import { isNil, mergeDeepRight } from 'ramda' @@ -14,6 +15,7 @@ import { getSettingsAuditLogPath, getSettingsBackupDir, getSettingsFilePath, + getSettingsWriteLockPath, } from './settings-paths' export { @@ -23,8 +25,149 @@ export { getSettingsAuditLogPath, getSettingsBackupDir, getSettingsFilePath, + getSettingsWriteLockPath, } from './settings-paths' +const SETTINGS_WRITE_LOCK_STALE_MS = 60_000 +const SETTINGS_WRITE_LOCK_RETRY_MS = 25 +const SETTINGS_WRITE_LOCK_MAX_WAIT_MS = 30_000 + +type SettingsWriteLockMetadata = { + pid: number + acquiredAt: number +} + +const readSettingsWriteLockMetadata = (lockPath: string): SettingsWriteLockMetadata | undefined => { + try { + const raw = fs.readFileSync(lockPath, 'utf-8') + const [pidLine, acquiredLine] = raw.split('\n') + const pid = Number(pidLine) + const acquiredAt = Number(acquiredLine) + if (!Number.isFinite(pid) || !Number.isFinite(acquiredAt)) { + return undefined + } + return { pid, acquiredAt } + } catch { + return undefined + } +} + +const isSettingsWriteLockStale = (metadata: SettingsWriteLockMetadata): boolean => { + if (Date.now() - metadata.acquiredAt > SETTINGS_WRITE_LOCK_STALE_MS) { + return true + } + + try { + process.kill(metadata.pid, 0) + return false + } catch (error) { + // Only ESRCH proves the owner is gone. EPERM means the process exists under + // another user, and any other error is treated as alive so a failed check + // can never steal a live lock; the age rule above remains the fallback. + return (error as NodeJS.ErrnoException).code === 'ESRCH' + } +} + +const reclaimSettingsWriteLock = (lockPath: string): void => { + const reclaimPath = `${lockPath}.reclaim.${process.pid}.${Date.now()}` + fs.renameSync(lockPath, reclaimPath) + fs.rmSync(reclaimPath, { force: true }) +} + +const attemptStealStaleSettingsWriteLock = ( + lockPath: string, + expected: SettingsWriteLockMetadata, +): boolean => { + try { + const current = readSettingsWriteLockMetadata(lockPath) + if ( + !current || + current.pid !== expected.pid || + current.acquiredAt !== expected.acquiredAt || + !isSettingsWriteLockStale(current) + ) { + return false + } + + reclaimSettingsWriteLock(lockPath) + return true + } catch { + return false + } +} + +// A writer holds the lock between openSync and writeFileSync, so a lock with +// missing or unparseable metadata is only abandoned once it has been untouched +// for the same grace period as a stale lock. Reclaiming it immediately would +// race a live writer. +const attemptStealAbandonedSettingsWriteLock = (lockPath: string): boolean => { + try { + const stats = fs.statSync(lockPath) + if (Date.now() - stats.mtimeMs <= SETTINGS_WRITE_LOCK_STALE_MS) { + return false + } + + reclaimSettingsWriteLock(lockPath) + return true + } catch { + return false + } +} + +export const withSettingsWriteLock = async (operation: () => T): Promise => { + ensureSettingsExists() + const lockPath = getSettingsWriteLockPath() + const deadline = Date.now() + SETTINGS_WRITE_LOCK_MAX_WAIT_MS + let lockFd: number | undefined + + while (Date.now() < deadline) { + try { + const lockFdLocal = fs.openSync(lockPath, 'wx') + try { + fs.writeFileSync(lockFdLocal, `${process.pid}\n${Date.now()}\n`) + } catch (error) { + // The lock exists before it has contents; leaving that empty file + // behind would block every later writer until someone removes it. + try { + fs.closeSync(lockFdLocal) + } finally { + fs.rmSync(lockPath, { force: true }) + } + throw error + } + lockFd = lockFdLocal + break + } catch (error) { + const errno = error as NodeJS.ErrnoException + if (errno.code !== 'EEXIST') { + throw error + } + + const metadata = readSettingsWriteLockMetadata(lockPath) + if (metadata) { + if (isSettingsWriteLockStale(metadata)) { + attemptStealStaleSettingsWriteLock(lockPath, metadata) + } + } else { + attemptStealAbandonedSettingsWriteLock(lockPath) + } + + await delay(SETTINGS_WRITE_LOCK_RETRY_MS) + } + } + + if (lockFd === undefined) { + throw new Error('Timed out waiting for settings write lock') + } + + try { + return operation() + } finally { + fs.closeSync(lockFd) + fs.rmSync(lockPath, { force: true }) + } +} + const logger = createLogger('settings-config') export type ValidationIssue = { diff --git a/src/utils/settings-paths.ts b/src/utils/settings-paths.ts index d55743195..49290bb4f 100644 --- a/src/utils/settings-paths.ts +++ b/src/utils/settings-paths.ts @@ -12,3 +12,5 @@ export const getDefaultSettingsFilePath = (): string => join(process.cwd(), 'res export const getSettingsBackupDir = (): string => join(getConfigBaseDir(), 'backups') export const getSettingsAuditLogPath = (): string => join(getConfigBaseDir(), 'settings-audit.jsonl') + +export const getSettingsWriteLockPath = (): string => join(getConfigBaseDir(), '.settings-write.lock') diff --git a/test/unit/controllers/admin/apply-moderation-actions-controller.spec.ts b/test/unit/controllers/admin/apply-moderation-actions-controller.spec.ts index 0f925cbc8..058851caf 100644 --- a/test/unit/controllers/admin/apply-moderation-actions-controller.spec.ts +++ b/test/unit/controllers/admin/apply-moderation-actions-controller.spec.ts @@ -83,6 +83,23 @@ describe('PostAdminModerationActionsController', () => { it('applies ban-pubkey and persists limits', async () => { const controller = new PostAdminModerationActionsController(outboxRepository as any) const pubkey = 'c'.repeat(64) + const savedLimits = { + ...baseLimits, + pubkey: { blacklist: [pubkey], whitelist: [] }, + } + sandbox.restore() + sandbox = Sinon.createSandbox() + outboxRepository = { enqueue: sandbox.stub().resolves() } + response = { + status: sandbox.stub().returnsThis(), + setHeader: sandbox.stub().returnsThis(), + send: sandbox.stub().returnsThis(), + } + sandbox.stub(adminModeration, 'readAdminModerationLimits') + ;(adminModeration.readAdminModerationLimits as Sinon.SinonStub).onFirstCall().returns(baseLimits) + ;(adminModeration.readAdminModerationLimits as Sinon.SinonStub).onSecondCall().returns(savedLimits) + sandbox.stub(adminModerationPersist, 'saveModerationLimitsFromRequest').resolves() + await controller.handleRequest( { body: { action: 'ban-pubkey', pubkey }, ip: '127.0.0.1' } as any, response as any, diff --git a/test/unit/controllers/admin/patch-moderation-limits-controller.spec.ts b/test/unit/controllers/admin/patch-moderation-limits-controller.spec.ts index 654061799..4d7a2c48f 100644 --- a/test/unit/controllers/admin/patch-moderation-limits-controller.spec.ts +++ b/test/unit/controllers/admin/patch-moderation-limits-controller.spec.ts @@ -74,6 +74,21 @@ describe('PatchAdminModerationLimitsController', () => { it('merges patches and persists limits', async () => { const controller = new PatchAdminModerationLimitsController(outboxRepository as any) const pubkey = 'c'.repeat(64) + const savedLimits = { + ...baseLimits, + pubkey: { blacklist: [pubkey], whitelist: [] }, + } + sandbox.restore() + sandbox = Sinon.createSandbox() + outboxRepository = { enqueue: sandbox.stub().resolves() } + response = { + status: sandbox.stub().returnsThis(), + setHeader: sandbox.stub().returnsThis(), + send: sandbox.stub().returnsThis(), + } + sandbox.stub(adminModeration, 'readAdminModerationLimits').returns(savedLimits) + sandbox.stub(adminModerationPersist, 'saveModerationLimitsFromRequest').resolves() + await controller.handleRequest( { body: { pubkey: { blacklist: [pubkey] } }, ip: '127.0.0.1' } as any, response as any, diff --git a/test/unit/migrations/moderation-stats-first-seen-indexes-migration.spec.ts b/test/unit/migrations/moderation-stats-first-seen-indexes-migration.spec.ts new file mode 100644 index 000000000..9816a4064 --- /dev/null +++ b/test/unit/migrations/moderation-stats-first-seen-indexes-migration.spec.ts @@ -0,0 +1,39 @@ +import { createRequire } from 'node:module' +import { join } from 'node:path' + +import { expect } from 'chai' +import Sinon from 'sinon' + +// createRequire works whether mocha loads this spec as CommonJS or as an ES module. +const requireFromRepo = createRequire(join(process.cwd(), 'package.json')) +const migration = requireFromRepo('./migrations/20261025_120000_add_moderation_stats_first_seen_indexes.js') + +describe('migrations/20261025_120000_add_moderation_stats_first_seen_indexes', () => { + it('opts out of knex transaction so CREATE INDEX CONCURRENTLY can run', () => { + expect(migration.config).to.deep.equal({ transaction: false }) + }) + + it('exports up and down', () => { + expect(migration.up).to.be.a('function') + expect(migration.down).to.be.a('function') + }) + + it('drops an invalid leftover index before recreating it', async () => { + const statements: string[] = [] + const raw = Sinon.stub().callsFake(async (sql: string, bindings?: unknown[]) => { + statements.push(sql) + if (sql.includes('SELECT 1') && bindings?.[0] === 'events_live_first_seen_pubkey_idx') { + return { rows: [{ '?column?': 1 }] } + } + return { rows: [] } + }) + + await migration.up({ raw }) + + expect(raw.calledWith('DROP INDEX CONCURRENTLY IF EXISTS ??', ['events_live_first_seen_pubkey_idx'])).to.equal(true) + expect(raw.calledWith('DROP INDEX CONCURRENTLY IF EXISTS ??', ['events_live_first_seen_remote_address_idx'])).to.equal( + false, + ) + expect(statements.filter((sql) => sql.includes('CREATE INDEX CONCURRENTLY IF NOT EXISTS')).length).to.equal(2) + }) +}) diff --git a/test/unit/schemas/admin-moderation-schema.spec.ts b/test/unit/schemas/admin-moderation-schema.spec.ts new file mode 100644 index 000000000..db38cbc0b --- /dev/null +++ b/test/unit/schemas/admin-moderation-schema.spec.ts @@ -0,0 +1,21 @@ +import { expect } from 'chai' + +import { adminModerationLimitsPatchSchema } from '../../../src/schemas/admin-moderation-schema' + +describe('admin-moderation-schema', () => { + it('accepts the same kinds event admission accepts', () => { + const result = adminModerationLimitsPatchSchema.safeParse({ + kind: { + blacklist: [65536, [65536, 65536]], + whitelist: [0, [0, 65535]], + }, + }) + + expect(result.success).to.equal(true) + }) + + it('rejects negative kinds and inverted ranges', () => { + expect(adminModerationLimitsPatchSchema.safeParse({ kind: { blacklist: [-1] } }).success).to.equal(false) + expect(adminModerationLimitsPatchSchema.safeParse({ kind: { blacklist: [[10, 1]] } }).success).to.equal(false) + }) +}) diff --git a/test/unit/utils/admin-moderation-persist.spec.ts b/test/unit/utils/admin-moderation-persist.spec.ts index 9ebd88906..837aed42d 100644 --- a/test/unit/utils/admin-moderation-persist.spec.ts +++ b/test/unit/utils/admin-moderation-persist.spec.ts @@ -1,11 +1,15 @@ +import { createRequire } from 'node:module' +import { join } from 'node:path' + import chai from 'chai' import Sinon from 'sinon' import sinonChai from 'sinon-chai' -import * as adminModeration from '../../../src/utils/admin-moderation' -import { saveModerationLimitsFromRequest } from '../../../src/utils/admin-moderation-persist' -import * as recordAdminAudit from '../../../src/utils/record-admin-audit' -import * as settingsConfig from '../../../src/utils/settings-config' +const loadFromRepo = createRequire(join(process.cwd(), 'package.json')) +const adminModeration = loadFromRepo('./src/utils/admin-moderation') as typeof import('../../../src/utils/admin-moderation') +const adminModerationPersist = loadFromRepo('./src/utils/admin-moderation-persist') as typeof import('../../../src/utils/admin-moderation-persist') +const recordAdminAudit = loadFromRepo('./src/utils/record-admin-audit') as typeof import('../../../src/utils/record-admin-audit') +const settingsConfig = loadFromRepo('./src/utils/settings-config') as typeof import('../../../src/utils/settings-config') chai.use(sinonChai) @@ -22,7 +26,7 @@ describe('saveModerationLimitsFromRequest', () => { beforeEach(() => { sandbox = Sinon.createSandbox() - sandbox.stub(adminModeration, 'persistAdminModerationLimits') + sandbox.stub(adminModeration, 'updateAdminModerationLimits').resolves() sandbox.stub(settingsConfig, 'appendSettingsAuditLog') sandbox.stub(recordAdminAudit, 'recordAdminAuditFromRequest').resolves() }) @@ -33,14 +37,15 @@ describe('saveModerationLimitsFromRequest', () => { it('persists limits, audits, and enqueues notification', async () => { const enqueue = sandbox.stub().resolves() - await saveModerationLimitsFromRequest( - limits, + await adminModerationPersist.saveModerationLimitsFromRequest( + () => limits, { ip: '127.0.0.1' } as any, { enqueue } as any, 'moderation.limits.updated', ) - expect(adminModeration.persistAdminModerationLimits).to.have.been.calledOnceWithExactly(limits) + expect(adminModeration.updateAdminModerationLimits).to.have.been.calledOnce + expect(adminModeration.updateAdminModerationLimits.firstCall.args[0]).to.be.a('function') expect(settingsConfig.appendSettingsAuditLog).to.have.been.calledOnce expect(recordAdminAudit.recordAdminAuditFromRequest).to.have.been.calledOnce expect(enqueue).to.have.been.calledOnce @@ -48,14 +53,14 @@ describe('saveModerationLimitsFromRequest', () => { it('still completes when notification enqueue fails', async () => { const enqueue = sandbox.stub().rejects(new Error('db unavailable')) - await saveModerationLimitsFromRequest( - limits, + await adminModerationPersist.saveModerationLimitsFromRequest( + () => limits, { ip: '127.0.0.1' } as any, { enqueue } as any, 'moderation.action', { action: 'ban-pubkey' }, ) - expect(adminModeration.persistAdminModerationLimits).to.have.been.calledOnce + expect(adminModeration.updateAdminModerationLimits).to.have.been.calledOnce }) }) diff --git a/test/unit/utils/admin-moderation-stats.spec.ts b/test/unit/utils/admin-moderation-stats.spec.ts index 86410a387..e01f6d5d1 100644 --- a/test/unit/utils/admin-moderation-stats.spec.ts +++ b/test/unit/utils/admin-moderation-stats.spec.ts @@ -40,22 +40,37 @@ describe('admin-moderation-stats queries', () => { it('maps top publishing pubkeys from the database', async () => { const chain = buildQueryChain([{ pubkey: 'e'.repeat(64), eventCount: '7' }]) const db = Sinon.stub().returns(chain) as any - db.raw = Sinon.stub().returns('raw') + const raw = Sinon.stub().callsFake((sql: string, bindings: unknown[]) => ({ sql, bindings })) + db.raw = raw const rows = await queryTopPublishingPubkeys(db, 1_700_000_000, 5) expect(rows).to.deep.equal([{ pubkey: 'e'.repeat(64), eventCount: 7 }]) expect(chain.limit.callCount).to.equal(1) + expect(raw.calledWith(`to_timestamp(?) at time zone 'UTC'`, [1_700_000_000])).to.equal(true) + expect( + chain.where.calledWith('first_seen', '>=', { + sql: `to_timestamp(?) at time zone 'UTC'`, + bindings: [1_700_000_000], + }), + ).to.equal(true) }) it('maps top client IPs from the database', async () => { const chain = buildQueryChain([{ ip: '203.0.113.4', eventCount: 2 }]) const db = Sinon.stub().returns(chain) as any - db.raw = Sinon.stub().returns('raw') + const raw = Sinon.stub().callsFake((sql: string, bindings: unknown[]) => ({ sql, bindings })) + db.raw = raw const rows = await queryTopClientIps(db, 1_700_000_000, 10) expect(rows).to.deep.equal([{ ip: '203.0.113.4', eventCount: 2 }]) expect(chain.whereNotNull.callCount).to.equal(1) + expect( + chain.where.calledWith('first_seen', '>=', { + sql: `to_timestamp(?) at time zone 'UTC'`, + bindings: [1_700_000_000], + }), + ).to.equal(true) }) }) diff --git a/test/unit/utils/admin-moderation.spec.ts b/test/unit/utils/admin-moderation.spec.ts index ee202b0f9..84dde463c 100644 --- a/test/unit/utils/admin-moderation.spec.ts +++ b/test/unit/utils/admin-moderation.spec.ts @@ -105,9 +105,30 @@ describe('admin-moderation', () => { expect(withoutIp.rateLimitBypass.ipAddresses).to.not.include('203.0.113.10') }) - it('persists normalized moderation limits to settings', () => { + it('normalizes IPv6 bypass addresses to match client socket spelling', () => { + const expanded = '2001:0db8:0:0:0:0:0:1' + const withIp = applyAdminModerationAction(baseLimits, 'whitelist-ip', { ip: expanded }) + expect(withIp.rateLimitBypass.ipAddresses).to.include('2001:db8::1') + + const withoutIp = applyAdminModerationAction(withIp, 'unwhitelist-ip', { ip: '2001:db8::1' }) + expect(withoutIp.rateLimitBypass.ipAddresses).to.not.include('2001:db8::1') + }) + + it('removes IPv6 bypass entries stored in alternate spelling', () => { + const limits = { + ...baseLimits, + rateLimitBypass: { + ...baseLimits.rateLimitBypass, + ipAddresses: ['2001:0db8:0:0:0:0:0:1'], + }, + } + const withoutIp = applyAdminModerationAction(limits, 'unwhitelist-ip', { ip: '2001:db8::1' }) + expect(withoutIp.rateLimitBypass.ipAddresses).to.be.empty + }) + + it('persists normalized moderation limits to settings', async () => { const pubkey = 'c'.repeat(64) - persistAdminModerationLimits({ + await persistAdminModerationLimits({ ...baseLimits, pubkey: { blacklist: [pubkey], whitelist: [] }, }) @@ -116,16 +137,14 @@ describe('admin-moderation', () => { expect(limits.pubkey.blacklist).to.include(pubkey) }) - it('persists when user settings only override part of info', () => { + it('persists when user settings only override part of info', async () => { settingsConfig.saveSettings({ info: { relay_url: 'wss://relay.example' } } as any) const pubkey = 'f'.repeat(64) - expect(() => - persistAdminModerationLimits({ - ...baseLimits, - pubkey: { blacklist: [pubkey], whitelist: [] }, - }), - ).to.not.throw() + await persistAdminModerationLimits({ + ...baseLimits, + pubkey: { blacklist: [pubkey], whitelist: [] }, + }) expect(readAdminModerationLimits().pubkey.blacklist).to.include(pubkey) }) diff --git a/test/unit/utils/settings-config.spec.ts b/test/unit/utils/settings-config.spec.ts index 320c02ea5..c16e80ff8 100644 --- a/test/unit/utils/settings-config.spec.ts +++ b/test/unit/utils/settings-config.spec.ts @@ -2,6 +2,7 @@ import { expect } from 'chai' import fs from 'fs' import os from 'os' import path from 'path' +import Sinon from 'sinon' import { ensureSettingsExists, @@ -15,7 +16,9 @@ import { setByPath, validatePathAgainstDefaults, validateSettings, + withSettingsWriteLock, } from '../../../src/utils/settings-config' +import { getSettingsWriteLockPath } from '../../../src/utils/settings-paths' import { getGuidedSettingCategory, getGuidedSettingField, @@ -881,3 +884,106 @@ describe('settings-guided-schema', () => { } }) }) + +describe('settings write lock', () => { + const originalConfigDir = process.env.NOSTR_CONFIG_DIR + let tmpDir: string + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'nostream-settings-lock-')) + process.env.NOSTR_CONFIG_DIR = tmpDir + }) + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }) + if (originalConfigDir === undefined) { + delete process.env.NOSTR_CONFIG_DIR + } else { + process.env.NOSTR_CONFIG_DIR = originalConfigDir + } + Sinon.restore() + }) + + it('removes the lock file when writing its metadata fails', async () => { + const lockPath = getSettingsWriteLockPath() + Sinon.stub(fs, 'writeFileSync').throws(Object.assign(new Error('no space left on device'), { code: 'ENOSPC' })) + + let error: Error | undefined + try { + await withSettingsWriteLock(() => 'unreachable') + } catch (err) { + error = err as Error + } + + expect(error?.message).to.equal('no space left on device') + expect(fs.existsSync(lockPath)).to.equal(false) + }) + + it('reclaims a lock without metadata once it is older than the grace period', async () => { + ensureSettingsExists() + const lockPath = getSettingsWriteLockPath() + fs.writeFileSync(lockPath, '') + const old = new Date(Date.now() - 61_000) + fs.utimesSync(lockPath, old, old) + + expect(await withSettingsWriteLock(() => 'ran')).to.equal('ran') + expect(fs.existsSync(lockPath)).to.equal(false) + }) + + it('reclaims a lock whose owner pid is gone (ESRCH)', async () => { + ensureSettingsExists() + const lockPath = getSettingsWriteLockPath() + fs.writeFileSync(lockPath, `${process.pid}\n${Date.now()}\n`) + Sinon.stub(process, 'kill').throws(Object.assign(new Error('no such process'), { code: 'ESRCH' })) + + expect(await withSettingsWriteLock(() => 'ran')).to.equal('ran') + expect(fs.existsSync(lockPath)).to.equal(false) + }) + + it('does not steal a live lock when the liveness check fails with EPERM', async () => { + ensureSettingsExists() + const lockPath = getSettingsWriteLockPath() + const acquiredAt = Date.now() + fs.writeFileSync(lockPath, `${process.pid}\n${acquiredAt}\n`) + Sinon.stub(process, 'kill').throws(Object.assign(new Error('not permitted'), { code: 'EPERM' })) + + let now = acquiredAt + Sinon.stub(Date, 'now').callsFake(() => now) + setTimeout(() => { + now = acquiredAt + 31_000 + }, 10) + + let error: Error | undefined + try { + await withSettingsWriteLock(() => 'ran') + } catch (err) { + error = err as Error + } + + expect(error?.message).to.equal('Timed out waiting for settings write lock') + expect(fs.existsSync(lockPath)).to.equal(true) + }) + + it('does not reclaim a fresh lock that has no metadata yet', async () => { + ensureSettingsExists() + const lockPath = getSettingsWriteLockPath() + fs.writeFileSync(lockPath, '') + + const acquiredAt = Date.now() + let now = acquiredAt + Sinon.stub(Date, 'now').callsFake(() => now) + setTimeout(() => { + now = acquiredAt + 31_000 + }, 10) + + let error: Error | undefined + try { + await withSettingsWriteLock(() => 'ran') + } catch (err) { + error = err as Error + } + + expect(error?.message).to.equal('Timed out waiting for settings write lock') + expect(fs.existsSync(lockPath)).to.equal(true) + }) +})