From bc73fa7c4ad8d71be1dbff171022b44e13e75106 Mon Sep 17 00:00:00 2001 From: ABHAY PANDEY Date: Sun, 25 Oct 2026 07:44:14 +0530 Subject: [PATCH 1/4] fix(admin): address Greptile moderation follow-ups from #941 Serialize settings updates across workers, count stats by first_seen, normalize IPv6 bypass addresses, reject reverd kind ranges, and return saved limits from PATCH/actions. --- .../patch-moderation-limits-controller.ts | 9 +-- .../post-moderation-actions-controller.ts | 24 +++++--- src/schemas/admin-moderation-schema.ts | 8 ++- src/utils/admin-moderation-persist.ts | 10 +++- src/utils/admin-moderation-stats.ts | 4 +- src/utils/admin-moderation.ts | 47 +++++++++++++--- src/utils/settings-config.ts | 56 +++++++++++++++++++ src/utils/settings-paths.ts | 2 + ...pply-moderation-actions-controller.spec.ts | 17 ++++++ ...patch-moderation-limits-controller.spec.ts | 15 +++++ .../utils/admin-moderation-persist.spec.ts | 26 +++++---- .../unit/utils/admin-moderation-stats.spec.ts | 9 ++- test/unit/utils/admin-moderation.spec.ts | 9 +++ 13 files changed, 197 insertions(+), 39 deletions(-) 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..81ff29d57 100644 --- a/src/schemas/admin-moderation-schema.ts +++ b/src/schemas/admin-moderation-schema.ts @@ -1,6 +1,12 @@ import { z } from 'zod' -const kindRuleSchema = z.union([z.number().int().min(0).max(65535), z.tuple([z.number().int(), z.number().int()])]) +const kindValueSchema = z.number().int().min(0).max(65535) +const kindRuleSchema = z.union([ + kindValueSchema, + z.tuple([kindValueSchema, kindValueSchema]).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..4d0106a8b 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) + 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..0188fce71 100644 --- a/src/utils/admin-moderation-stats.ts +++ b/src/utils/admin-moderation-stats.ts @@ -51,7 +51,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', '>=', new Date(sinceUnixSeconds * 1000)) .groupBy('event_pubkey') .orderBy('eventCount', 'desc') .limit(limit) @@ -73,7 +73,7 @@ export const queryTopClientIps = async ( .count('* as eventCount') .whereNull('deleted_at') .whereNotNull('remote_address') - .where('event_created_at', '>=', sinceUnixSeconds) + .where('first_seen', '>=', new Date(sinceUnixSeconds * 1000)) .groupBy('remote_address') .orderBy('eventCount', 'desc') .limit(limit) diff --git a/src/utils/admin-moderation.ts b/src/utils/admin-moderation.ts index 9020cb8fd..b99597d72 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 = (updater: AdminModerationLimitsUpdater): void => { + withSettingsWriteLock(() => { + const current = readAdminModerationLimits() + persistAdminModerationLimitsUnlocked(updater(current)) + }) +} + +export const persistAdminModerationLimits = (next: AdminModerationLimits): void => { + updateAdminModerationLimits(() => next) +} + export class ValidationError extends Error { public constructor( message: string, @@ -248,10 +277,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,10 +291,11 @@ 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: { diff --git a/src/utils/settings-config.ts b/src/utils/settings-config.ts index de7e94a9a..e87854636 100644 --- a/src/utils/settings-config.ts +++ b/src/utils/settings-config.ts @@ -14,6 +14,7 @@ import { getSettingsAuditLogPath, getSettingsBackupDir, getSettingsFilePath, + getSettingsWriteLockPath, } from './settings-paths' export { @@ -23,8 +24,63 @@ 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 + +const sleepSync = (milliseconds: number): void => { + const deadline = Date.now() + milliseconds + while (Date.now() < deadline) { + // Busy-wait so lock acquisition stays synchronous with saveSettings. + } +} + +export const withSettingsWriteLock = (operation: () => T): T => { + ensureSettingsExists() + const lockPath = getSettingsWriteLockPath() + const deadline = Date.now() + SETTINGS_WRITE_LOCK_MAX_WAIT_MS + let lockFd: number | undefined + + while (Date.now() < deadline) { + try { + lockFd = fs.openSync(lockPath, 'wx') + fs.writeFileSync(lockFd, `${process.pid}\n${Date.now()}\n`) + break + } catch (error) { + const errno = error as NodeJS.ErrnoException + if (errno.code !== 'EEXIST') { + throw error + } + + try { + const stat = fs.statSync(lockPath) + if (Date.now() - stat.mtimeMs > SETTINGS_WRITE_LOCK_STALE_MS) { + fs.rmSync(lockPath, { force: true }) + continue + } + } catch { + // Another worker may have released the lock. + } + + sleepSync(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/utils/admin-moderation-persist.spec.ts b/test/unit/utils/admin-moderation-persist.spec.ts index 9ebd88906..d90a612ab 100644 --- a/test/unit/utils/admin-moderation-persist.spec.ts +++ b/test/unit/utils/admin-moderation-persist.spec.ts @@ -1,11 +1,14 @@ +import { createRequire } from 'node:module' + 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 require = createRequire(import.meta.url) +const adminModeration = require('../../../src/utils/admin-moderation') as typeof import('../../../src/utils/admin-moderation') +const adminModerationPersist = require('../../../src/utils/admin-moderation-persist') as typeof import('../../../src/utils/admin-moderation-persist') +const recordAdminAudit = require('../../../src/utils/record-admin-audit') as typeof import('../../../src/utils/record-admin-audit') +const settingsConfig = require('../../../src/utils/settings-config') as typeof import('../../../src/utils/settings-config') chai.use(sinonChai) @@ -22,7 +25,7 @@ describe('saveModerationLimitsFromRequest', () => { beforeEach(() => { sandbox = Sinon.createSandbox() - sandbox.stub(adminModeration, 'persistAdminModerationLimits') + sandbox.stub(adminModeration, 'updateAdminModerationLimits') sandbox.stub(settingsConfig, 'appendSettingsAuditLog') sandbox.stub(recordAdminAudit, 'recordAdminAuditFromRequest').resolves() }) @@ -33,14 +36,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 +52,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..59f184a6c 100644 --- a/test/unit/utils/admin-moderation-stats.spec.ts +++ b/test/unit/utils/admin-moderation-stats.spec.ts @@ -1,12 +1,15 @@ +import { createRequire } from 'node:module' + import { expect } from 'chai' import Sinon from 'sinon' -import { +const require = createRequire(import.meta.url) +const { parseModerationStatsLimit, parseModerationStatsWindowHours, queryTopClientIps, queryTopPublishingPubkeys, -} from '../../../src/utils/admin-moderation-stats' +} = require('../../../src/utils/admin-moderation-stats') as typeof import('../../../src/utils/admin-moderation-stats') describe('admin-moderation-stats parsing', () => { it('parses windowHours', () => { @@ -46,6 +49,7 @@ describe('admin-moderation-stats queries', () => { expect(rows).to.deep.equal([{ pubkey: 'e'.repeat(64), eventCount: 7 }]) expect(chain.limit.callCount).to.equal(1) + expect(chain.where.calledWith('first_seen', '>=', new Date(1_700_000_000 * 1000))).to.equal(true) }) it('maps top client IPs from the database', async () => { @@ -57,5 +61,6 @@ describe('admin-moderation-stats queries', () => { 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', '>=', new Date(1_700_000_000 * 1000))).to.equal(true) }) }) diff --git a/test/unit/utils/admin-moderation.spec.ts b/test/unit/utils/admin-moderation.spec.ts index ee202b0f9..681d195d5 100644 --- a/test/unit/utils/admin-moderation.spec.ts +++ b/test/unit/utils/admin-moderation.spec.ts @@ -105,6 +105,15 @@ describe('admin-moderation', () => { expect(withoutIp.rateLimitBypass.ipAddresses).to.not.include('203.0.113.10') }) + 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('persists normalized moderation limits to settings', () => { const pubkey = 'c'.repeat(64) persistAdminModerationLimits({ From 3696f3e418063418e027cd2a5756dfe19fe8ad58 Mon Sep 17 00:00:00 2001 From: ABHAY PANDEY Date: Sun, 25 Oct 2026 07:49:14 +0530 Subject: [PATCH 2/4] chore(changeset): admin moderation Greptile follow-ups (#941) --- .changeset/admin-moderation-greptile-941.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/admin-moderation-greptile-941.md diff --git a/.changeset/admin-moderation-greptile-941.md b/.changeset/admin-moderation-greptile-941.md new file mode 100644 index 000000000..34c211a0e --- /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 a settings write lock. Activity stats windows filter on `first_seen` (when the relay first saw the event), not `event_created_at`. IPv6 addresses on the rate-limit bypass list are normalized the same way as connection IPs so bypass rules cannot be skipped by formatting. PATCH rejects kind ranges with `start` greater than `end`. PATCH and moderation actions return the normalized limits read back from settings after save. From 527b1afade5c1ae67a323694a96be7911ab636f6 Mon Sep 17 00:00:00 2001 From: ABHAY PANDEY Date: Sun, 25 Oct 2026 07:54:54 +0530 Subject: [PATCH 3/4] fix(admin): address Greptile review on moderation follow-ups Yield while waiting on the settings write lock, reclaim stale locks safely, normalize IPv6 bypass removal, index first_seen for stats queries, and fix persist unit test loading under ts-node. --- .changeset/admin-moderation-greptile-941.md | 2 +- ...add_moderation_stats_first_seen_indexes.js | 31 ++++++++ src/utils/admin-moderation-persist.ts | 2 +- src/utils/admin-moderation.ts | 19 +++-- src/utils/settings-config.ts | 74 +++++++++++++++---- ...stats-first-seen-indexes-migration.spec.ts | 17 +++++ .../utils/admin-moderation-persist.spec.ts | 13 ++-- .../unit/utils/admin-moderation-stats.spec.ts | 7 +- test/unit/utils/admin-moderation.spec.ts | 28 ++++--- 9 files changed, 152 insertions(+), 41 deletions(-) create mode 100644 migrations/20261025_120000_add_moderation_stats_first_seen_indexes.js create mode 100644 test/unit/migrations/moderation-stats-first-seen-indexes-migration.spec.ts diff --git a/.changeset/admin-moderation-greptile-941.md b/.changeset/admin-moderation-greptile-941.md index 34c211a0e..1a37e8b40 100644 --- a/.changeset/admin-moderation-greptile-941.md +++ b/.changeset/admin-moderation-greptile-941.md @@ -4,4 +4,4 @@ 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 a settings write lock. Activity stats windows filter on `first_seen` (when the relay first saw the event), not `event_created_at`. IPv6 addresses on the rate-limit bypass list are normalized the same way as connection IPs so bypass rules cannot be skipped by formatting. PATCH rejects kind ranges with `start` greater than `end`. PATCH and moderation actions return the normalized limits read back from settings after save. +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..73f0bef47 --- /dev/null +++ b/migrations/20261025_120000_add_moderation_stats_first_seen_indexes.js @@ -0,0 +1,31 @@ +/** + * 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 } + +exports.up = async function (knex) { + 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/utils/admin-moderation-persist.ts b/src/utils/admin-moderation-persist.ts index 4d0106a8b..625e86a4a 100644 --- a/src/utils/admin-moderation-persist.ts +++ b/src/utils/admin-moderation-persist.ts @@ -30,7 +30,7 @@ export const saveModerationLimitsFromRequest = async ( auditDetail?: Record, ): Promise => { try { - updateAdminModerationLimits(update) + await updateAdminModerationLimits(update) } catch (error) { if (error instanceof ValidationError) { throw error diff --git a/src/utils/admin-moderation.ts b/src/utils/admin-moderation.ts index b99597d72..d55c1be51 100644 --- a/src/utils/admin-moderation.ts +++ b/src/utils/admin-moderation.ts @@ -165,15 +165,15 @@ const persistAdminModerationLimitsUnlocked = (next: AdminModerationLimits): void saveSettings(nextUserSettings) } -export const updateAdminModerationLimits = (updater: AdminModerationLimitsUpdater): void => { - withSettingsWriteLock(() => { +export const updateAdminModerationLimits = async (updater: AdminModerationLimitsUpdater): Promise => { + await withSettingsWriteLock(() => { const current = readAdminModerationLimits() persistAdminModerationLimitsUnlocked(updater(current)) }) } -export const persistAdminModerationLimits = (next: AdminModerationLimits): void => { - updateAdminModerationLimits(() => next) +export const persistAdminModerationLimits = async (next: AdminModerationLimits): Promise => { + await updateAdminModerationLimits(() => next) } export class ValidationError extends Error { @@ -212,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, @@ -300,7 +309,7 @@ export const applyAdminModerationAction = ( ...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 e87854636..6ca808480 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' @@ -31,14 +32,64 @@ const SETTINGS_WRITE_LOCK_STALE_MS = 60_000 const SETTINGS_WRITE_LOCK_RETRY_MS = 25 const SETTINGS_WRITE_LOCK_MAX_WAIT_MS = 30_000 -const sleepSync = (milliseconds: number): void => { - const deadline = Date.now() + milliseconds - while (Date.now() < deadline) { - // Busy-wait so lock acquisition stays synchronous with saveSettings. +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 } } -export const withSettingsWriteLock = (operation: () => T): T => { +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 { + return 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 + } + + const reclaimPath = `${lockPath}.reclaim.${process.pid}.${Date.now()}` + fs.renameSync(lockPath, reclaimPath) + fs.rmSync(reclaimPath, { force: true }) + 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 @@ -55,17 +106,12 @@ export const withSettingsWriteLock = (operation: () => T): T => { throw error } - try { - const stat = fs.statSync(lockPath) - if (Date.now() - stat.mtimeMs > SETTINGS_WRITE_LOCK_STALE_MS) { - fs.rmSync(lockPath, { force: true }) - continue - } - } catch { - // Another worker may have released the lock. + const metadata = readSettingsWriteLockMetadata(lockPath) + if (metadata && isSettingsWriteLockStale(metadata)) { + attemptStealStaleSettingsWriteLock(lockPath, metadata) } - sleepSync(SETTINGS_WRITE_LOCK_RETRY_MS) + await delay(SETTINGS_WRITE_LOCK_RETRY_MS) } } 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..03e9649d4 --- /dev/null +++ b/test/unit/migrations/moderation-stats-first-seen-indexes-migration.spec.ts @@ -0,0 +1,17 @@ +import { createRequire } from 'node:module' + +import { expect } from 'chai' + +const requireFromHere = createRequire(__filename) +const migration = requireFromHere('../../../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') + }) +}) diff --git a/test/unit/utils/admin-moderation-persist.spec.ts b/test/unit/utils/admin-moderation-persist.spec.ts index d90a612ab..837aed42d 100644 --- a/test/unit/utils/admin-moderation-persist.spec.ts +++ b/test/unit/utils/admin-moderation-persist.spec.ts @@ -1,14 +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' -const require = createRequire(import.meta.url) -const adminModeration = require('../../../src/utils/admin-moderation') as typeof import('../../../src/utils/admin-moderation') -const adminModerationPersist = require('../../../src/utils/admin-moderation-persist') as typeof import('../../../src/utils/admin-moderation-persist') -const recordAdminAudit = require('../../../src/utils/record-admin-audit') as typeof import('../../../src/utils/record-admin-audit') -const settingsConfig = require('../../../src/utils/settings-config') as typeof import('../../../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) @@ -25,7 +26,7 @@ describe('saveModerationLimitsFromRequest', () => { beforeEach(() => { sandbox = Sinon.createSandbox() - sandbox.stub(adminModeration, 'updateAdminModerationLimits') + sandbox.stub(adminModeration, 'updateAdminModerationLimits').resolves() sandbox.stub(settingsConfig, 'appendSettingsAuditLog') sandbox.stub(recordAdminAudit, 'recordAdminAuditFromRequest').resolves() }) diff --git a/test/unit/utils/admin-moderation-stats.spec.ts b/test/unit/utils/admin-moderation-stats.spec.ts index 59f184a6c..89c79fad9 100644 --- a/test/unit/utils/admin-moderation-stats.spec.ts +++ b/test/unit/utils/admin-moderation-stats.spec.ts @@ -1,15 +1,12 @@ -import { createRequire } from 'node:module' - import { expect } from 'chai' import Sinon from 'sinon' -const require = createRequire(import.meta.url) -const { +import { parseModerationStatsLimit, parseModerationStatsWindowHours, queryTopClientIps, queryTopPublishingPubkeys, -} = require('../../../src/utils/admin-moderation-stats') as typeof import('../../../src/utils/admin-moderation-stats') +} from '../../../src/utils/admin-moderation-stats' describe('admin-moderation-stats parsing', () => { it('parses windowHours', () => { diff --git a/test/unit/utils/admin-moderation.spec.ts b/test/unit/utils/admin-moderation.spec.ts index 681d195d5..84dde463c 100644 --- a/test/unit/utils/admin-moderation.spec.ts +++ b/test/unit/utils/admin-moderation.spec.ts @@ -114,9 +114,21 @@ describe('admin-moderation', () => { expect(withoutIp.rateLimitBypass.ipAddresses).to.not.include('2001:db8::1') }) - it('persists normalized moderation limits to settings', () => { + 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: [] }, }) @@ -125,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) }) From f1c52bd6489d5f29550bbb2e6f3734f9838bba43 Mon Sep 17 00:00:00 2001 From: ABHAY PANDEY Date: Sun, 1 Nov 2026 23:44:06 +0530 Subject: [PATCH 4/4] fix(admin): harden settings lock and moderation stats --- ...add_moderation_stats_first_seen_indexes.js | 28 +++++ src/schemas/admin-moderation-schema.ts | 9 +- src/utils/admin-moderation-stats.ts | 11 +- src/utils/settings-config.ts | 59 ++++++++-- ...stats-first-seen-indexes-migration.spec.ts | 26 ++++- .../schemas/admin-moderation-schema.spec.ts | 21 ++++ .../unit/utils/admin-moderation-stats.spec.ts | 21 +++- test/unit/utils/settings-config.spec.ts | 106 ++++++++++++++++++ 8 files changed, 261 insertions(+), 20 deletions(-) create mode 100644 test/unit/schemas/admin-moderation-schema.spec.ts diff --git a/migrations/20261025_120000_add_moderation_stats_first_seen_indexes.js b/migrations/20261025_120000_add_moderation_stats_first_seen_indexes.js index 73f0bef47..f6d7a3db5 100644 --- a/migrations/20261025_120000_add_moderation_stats_first_seen_indexes.js +++ b/migrations/20261025_120000_add_moderation_stats_first_seen_indexes.js @@ -11,7 +11,35 @@ 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) diff --git a/src/schemas/admin-moderation-schema.ts b/src/schemas/admin-moderation-schema.ts index 81ff29d57..dc7212f17 100644 --- a/src/schemas/admin-moderation-schema.ts +++ b/src/schemas/admin-moderation-schema.ts @@ -1,9 +1,12 @@ import { z } from 'zod' -const kindValueSchema = z.number().int().min(0).max(65535) +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([ - kindValueSchema, - z.tuple([kindValueSchema, kindValueSchema]).refine(([start, end]) => start <= end, { + kindSchema, + z.tuple([kindSchema, kindSchema]).refine(([start, end]) => start <= end, { message: 'Kind range start must not exceed its end', }), ]) diff --git a/src/utils/admin-moderation-stats.ts b/src/utils/admin-moderation-stats.ts index 0188fce71..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('first_seen', '>=', new Date(sinceUnixSeconds * 1000)) + .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('first_seen', '>=', new Date(sinceUnixSeconds * 1000)) + .where('first_seen', '>=', firstSeenCutoff(db, sinceUnixSeconds)) .groupBy('remote_address') .orderBy('eventCount', 'desc') .limit(limit) diff --git a/src/utils/settings-config.ts b/src/utils/settings-config.ts index 5357aee32..c0421c398 100644 --- a/src/utils/settings-config.ts +++ b/src/utils/settings-config.ts @@ -60,11 +60,20 @@ const isSettingsWriteLockStale = (metadata: SettingsWriteLockMetadata): boolean try { process.kill(metadata.pid, 0) return false - } catch { - return true + } 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, @@ -80,9 +89,25 @@ const attemptStealStaleSettingsWriteLock = ( return false } - const reclaimPath = `${lockPath}.reclaim.${process.pid}.${Date.now()}` - fs.renameSync(lockPath, reclaimPath) - fs.rmSync(reclaimPath, { force: true }) + 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 @@ -97,8 +122,20 @@ export const withSettingsWriteLock = async (operation: () => T): Promise = while (Date.now() < deadline) { try { - lockFd = fs.openSync(lockPath, 'wx') - fs.writeFileSync(lockFd, `${process.pid}\n${Date.now()}\n`) + 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 @@ -107,8 +144,12 @@ export const withSettingsWriteLock = async (operation: () => T): Promise = } const metadata = readSettingsWriteLockMetadata(lockPath) - if (metadata && isSettingsWriteLockStale(metadata)) { - attemptStealStaleSettingsWriteLock(lockPath, metadata) + if (metadata) { + if (isSettingsWriteLockStale(metadata)) { + attemptStealStaleSettingsWriteLock(lockPath, metadata) + } + } else { + attemptStealAbandonedSettingsWriteLock(lockPath) } await delay(SETTINGS_WRITE_LOCK_RETRY_MS) 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 index 03e9649d4..9816a4064 100644 --- a/test/unit/migrations/moderation-stats-first-seen-indexes-migration.spec.ts +++ b/test/unit/migrations/moderation-stats-first-seen-indexes-migration.spec.ts @@ -1,9 +1,12 @@ import { createRequire } from 'node:module' +import { join } from 'node:path' import { expect } from 'chai' +import Sinon from 'sinon' -const requireFromHere = createRequire(__filename) -const migration = requireFromHere('../../../migrations/20261025_120000_add_moderation_stats_first_seen_indexes.js') +// 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', () => { @@ -14,4 +17,23 @@ describe('migrations/20261025_120000_add_moderation_stats_first_seen_indexes', ( 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-stats.spec.ts b/test/unit/utils/admin-moderation-stats.spec.ts index 89c79fad9..e01f6d5d1 100644 --- a/test/unit/utils/admin-moderation-stats.spec.ts +++ b/test/unit/utils/admin-moderation-stats.spec.ts @@ -40,24 +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(chain.where.calledWith('first_seen', '>=', new Date(1_700_000_000 * 1000))).to.equal(true) + 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', '>=', new Date(1_700_000_000 * 1000))).to.equal(true) + 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/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) + }) +})