From 2d2418aab9977882582d9578a350f63582c52a1b Mon Sep 17 00:00:00 2001 From: Pip Build Date: Sun, 1 Mar 2026 23:48:28 -0500 Subject: [PATCH 01/21] feat(notifications): webhook notification system for important bot events (#130) - Add webhookNotifier module with HMAC-SHA256 signing, retry with exponential backoff (3 attempts: 1s/3s/9s), and delivery logging - Add webhook_delivery_log DB migration (005) - Add notifications.webhooks[] config section - Add CRUD + test + delivery log API routes under /guilds/:id/notifications/ - Register notificationsRouter in API index - Hook moderation.action into createCase (moderation.js) - Hook bot.disconnected/reconnected/error into shard events (index.js) - Hook config.changed into config change listeners (config-listeners.js) - Hook member.flagged into triage moderate flow (triage-respond.js) - Add health.degraded periodic check with memory/event-loop-lag thresholds - Add getAllGuildIds() to config module for bot-level event broadcasting - Add WEBHOOK_EVENTS constants, signPayload, testEndpoint, getDeliveryLog exports - Comprehensive test suite: 51 tests across module + route layers --- feat-issue-164 | 1 + migrations/005_webhook_notifications.cjs | 42 +++ src/api/index.js | 4 + src/api/routes/notifications.js | 311 ++++++++++++++++ src/config-listeners.js | 10 + src/index.js | 48 ++- src/modules/config.js | 11 + src/modules/moderation.js | 12 + src/modules/triage-respond.js | 10 + src/modules/webhookNotifier.js | 303 ++++++++++++++++ src/utils/health.js | 25 ++ tests/api/routes/notifications.test.js | 355 ++++++++++++++++++ tests/modules/webhookNotifier.test.js | 438 +++++++++++++++++++++++ 13 files changed, 1568 insertions(+), 2 deletions(-) create mode 160000 feat-issue-164 create mode 100644 migrations/005_webhook_notifications.cjs create mode 100644 src/api/routes/notifications.js create mode 100644 src/modules/webhookNotifier.js create mode 100644 tests/api/routes/notifications.test.js create mode 100644 tests/modules/webhookNotifier.test.js diff --git a/feat-issue-164 b/feat-issue-164 new file mode 160000 index 000000000..b9b1db37d --- /dev/null +++ b/feat-issue-164 @@ -0,0 +1 @@ +Subproject commit b9b1db37dcf8b2d5c3838d9a80eabc385f42aa59 diff --git a/migrations/005_webhook_notifications.cjs b/migrations/005_webhook_notifications.cjs new file mode 100644 index 000000000..17fa72638 --- /dev/null +++ b/migrations/005_webhook_notifications.cjs @@ -0,0 +1,42 @@ +/* eslint-disable */ +'use strict'; + +/** + * Migration 005: Webhook notifications delivery log + * + * Creates webhook_delivery_log table to store delivery attempts + * per webhook endpoint. Endpoint configs live in the per-guild config JSON. + */ + +/** @param {import('node-pg-migrate').MigrationBuilder} pgm */ +exports.up = (pgm) => { + pgm.sql(` + CREATE TABLE IF NOT EXISTS webhook_delivery_log ( + id SERIAL PRIMARY KEY, + guild_id TEXT NOT NULL, + endpoint_id TEXT NOT NULL, + event_type TEXT NOT NULL, + payload JSONB NOT NULL, + status TEXT NOT NULL CHECK (status IN ('success', 'failed', 'pending')), + response_code INTEGER, + response_body TEXT, + attempt INTEGER NOT NULL DEFAULT 1, + delivered_at TIMESTAMPTZ DEFAULT NOW() + ); + + CREATE INDEX IF NOT EXISTS idx_webhook_delivery_log_guild + ON webhook_delivery_log (guild_id, delivered_at DESC); + + CREATE INDEX IF NOT EXISTS idx_webhook_delivery_log_endpoint + ON webhook_delivery_log (endpoint_id, delivered_at DESC); + `); +}; + +/** @param {import('node-pg-migrate').MigrationBuilder} pgm */ +exports.down = (pgm) => { + pgm.sql(` + DROP INDEX IF EXISTS idx_webhook_delivery_log_endpoint; + DROP INDEX IF EXISTS idx_webhook_delivery_log_guild; + DROP TABLE IF EXISTS webhook_delivery_log; + `); +}; diff --git a/src/api/index.js b/src/api/index.js index 0a69c678d..edf599437 100644 --- a/src/api/index.js +++ b/src/api/index.js @@ -17,6 +17,7 @@ import healthRouter from './routes/health.js'; import membersRouter from './routes/members.js'; import moderationRouter from './routes/moderation.js'; import ticketsRouter from './routes/tickets.js'; +import notificationsRouter from './routes/notifications.js'; import webhooksRouter from './routes/webhooks.js'; const router = Router(); @@ -58,6 +59,9 @@ router.use('/moderation', requireAuth(), auditLogMiddleware(), moderationRouter) // GET-only; no audit middleware needed (reads are not mutating actions) router.use('/guilds', requireAuth(), auditLogRouter); +// Notification webhook management routes — require API secret or OAuth2 JWT +router.use('/guilds', requireAuth(), auditLogMiddleware(), notificationsRouter); + // Webhook routes — require API secret or OAuth2 JWT (endpoint further restricts to api-secret) router.use('/webhooks', requireAuth(), webhooksRouter); diff --git a/src/api/routes/notifications.js b/src/api/routes/notifications.js new file mode 100644 index 000000000..7703de08c --- /dev/null +++ b/src/api/routes/notifications.js @@ -0,0 +1,311 @@ +/** + * Notification Webhook Routes + * + * Endpoints for managing outbound webhook notification endpoints per guild + * and viewing the delivery log. Webhook secrets are write-only — they are + * never returned in GET responses. + */ + +import { randomUUID } from 'node:crypto'; +import { Router } from 'express'; +import { error as logError, info } from '../../logger.js'; +import { getConfig, setConfigValue } from '../../modules/config.js'; +import { getDeliveryLog, testEndpoint, WEBHOOK_EVENTS } from '../../modules/webhookNotifier.js'; + +const router = Router(); + +/** + * Mask the secret field from an endpoint object for safe GET responses. + * + * @param {Object} ep - Endpoint config + * @returns {Object} Endpoint with secret replaced by a mask indicator + */ +function maskEndpoint(ep) { + const { secret: _secret, ...rest } = ep; + return { ...rest, hasSecret: Boolean(_secret) }; +} + +/** + * @openapi + * /guilds/{id}/notifications/webhooks: + * get: + * tags: + * - Notifications + * summary: List webhook endpoints for a guild + * description: Returns all configured outbound webhook endpoints. Secrets are never included. + * security: + * - ApiKeyAuth: [] + * - BearerAuth: [] + * parameters: + * - in: path + * name: id + * required: true + * schema: + * type: string + * description: Guild ID + * responses: + * "200": + * description: List of webhook endpoints (secrets masked) + * "401": + * $ref: "#/components/responses/Unauthorized" + */ +router.get('/:guildId/notifications/webhooks', async (req, res) => { + const { guildId } = req.params; + + try { + const cfg = getConfig(guildId); + const webhooks = Array.isArray(cfg?.notifications?.webhooks) + ? cfg.notifications.webhooks.map(maskEndpoint) + : []; + return res.json(webhooks); + } catch (err) { + logError('Failed to list webhook endpoints', { guildId, error: err.message }); + return res.status(500).json({ error: 'Failed to retrieve webhook endpoints' }); + } +}); + +/** + * @openapi + * /guilds/{id}/notifications/webhooks: + * post: + * tags: + * - Notifications + * summary: Add a webhook endpoint + * security: + * - ApiKeyAuth: [] + * - BearerAuth: [] + * parameters: + * - in: path + * name: id + * required: true + * schema: + * type: string + * requestBody: + * required: true + * content: + * application/json: + * schema: + * type: object + * required: + * - url + * - events + * properties: + * url: + * type: string + * description: HTTPS delivery URL + * events: + * type: array + * items: + * type: string + * description: Event types to subscribe to + * secret: + * type: string + * description: Optional HMAC signing secret + * enabled: + * type: boolean + * default: true + * responses: + * "201": + * description: Created endpoint (secret masked) + * "400": + * $ref: "#/components/responses/BadRequest" + * "401": + * $ref: "#/components/responses/Unauthorized" + */ +router.post('/:guildId/notifications/webhooks', async (req, res) => { + const { guildId } = req.params; + const { url, events, secret, enabled = true } = req.body || {}; + + if (!url || typeof url !== 'string') { + return res.status(400).json({ error: 'Missing or invalid "url"' }); + } + + if (!/^https?:\/\/.+/.test(url)) { + return res.status(400).json({ error: '"url" must be a valid HTTP/HTTPS URL' }); + } + + if (!Array.isArray(events) || events.length === 0) { + return res.status(400).json({ error: '"events" must be a non-empty array' }); + } + + const invalidEvents = events.filter((e) => !WEBHOOK_EVENTS.includes(e)); + if (invalidEvents.length > 0) { + return res.status(400).json({ + error: `Invalid event types: ${invalidEvents.join(', ')}`, + validEvents: WEBHOOK_EVENTS, + }); + } + + try { + const cfg = getConfig(guildId); + const existing = Array.isArray(cfg?.notifications?.webhooks) ? cfg.notifications.webhooks : []; + + if (existing.length >= 20) { + return res.status(400).json({ error: 'Maximum of 20 webhook endpoints per guild' }); + } + + const newEndpoint = { + id: randomUUID(), + url, + events, + enabled: Boolean(enabled), + ...(secret ? { secret } : {}), + }; + + const updated = [...existing, newEndpoint]; + await setConfigValue('notifications.webhooks', updated, guildId); + + info('Webhook endpoint added', { guildId, endpointId: newEndpoint.id, url }); + return res.status(201).json(maskEndpoint(newEndpoint)); + } catch (err) { + logError('Failed to add webhook endpoint', { guildId, error: err.message }); + return res.status(500).json({ error: 'Failed to add webhook endpoint' }); + } +}); + +/** + * @openapi + * /guilds/{id}/notifications/webhooks/{endpointId}: + * delete: + * tags: + * - Notifications + * summary: Remove a webhook endpoint + * security: + * - ApiKeyAuth: [] + * - BearerAuth: [] + * parameters: + * - in: path + * name: id + * required: true + * schema: + * type: string + * - in: path + * name: endpointId + * required: true + * schema: + * type: string + * responses: + * "204": + * description: Endpoint removed + * "404": + * $ref: "#/components/responses/NotFound" + * "401": + * $ref: "#/components/responses/Unauthorized" + */ +router.delete('/:guildId/notifications/webhooks/:endpointId', async (req, res) => { + const { guildId, endpointId } = req.params; + + try { + const cfg = getConfig(guildId); + const existing = Array.isArray(cfg?.notifications?.webhooks) ? cfg.notifications.webhooks : []; + + const updated = existing.filter((ep) => ep.id !== endpointId); + if (updated.length === existing.length) { + return res.status(404).json({ error: 'Webhook endpoint not found' }); + } + + await setConfigValue('notifications.webhooks', updated, guildId); + info('Webhook endpoint removed', { guildId, endpointId }); + return res.status(204).end(); + } catch (err) { + logError('Failed to remove webhook endpoint', { guildId, error: err.message }); + return res.status(500).json({ error: 'Failed to remove webhook endpoint' }); + } +}); + +/** + * @openapi + * /guilds/{id}/notifications/webhooks/{endpointId}/test: + * post: + * tags: + * - Notifications + * summary: Send a test event to a webhook endpoint + * security: + * - ApiKeyAuth: [] + * - BearerAuth: [] + * parameters: + * - in: path + * name: id + * required: true + * schema: + * type: string + * - in: path + * name: endpointId + * required: true + * schema: + * type: string + * responses: + * "200": + * description: Test result with status code and response body + * "404": + * $ref: "#/components/responses/NotFound" + * "401": + * $ref: "#/components/responses/Unauthorized" + */ +router.post('/:guildId/notifications/webhooks/:endpointId/test', async (req, res) => { + const { guildId, endpointId } = req.params; + + try { + const cfg = getConfig(guildId); + const existing = Array.isArray(cfg?.notifications?.webhooks) ? cfg.notifications.webhooks : []; + const endpoint = existing.find((ep) => ep.id === endpointId); + + if (!endpoint) { + return res.status(404).json({ error: 'Webhook endpoint not found' }); + } + + const result = await testEndpoint(guildId, endpoint); + return res.json({ + ok: result.ok, + status: result.status, + body: result.text?.slice(0, 500), + }); + } catch (err) { + logError('Failed to test webhook endpoint', { guildId, error: err.message }); + return res.status(500).json({ error: 'Failed to test webhook endpoint' }); + } +}); + +/** + * @openapi + * /guilds/{id}/notifications/deliveries: + * get: + * tags: + * - Notifications + * summary: Get webhook delivery log for a guild + * security: + * - ApiKeyAuth: [] + * - BearerAuth: [] + * parameters: + * - in: path + * name: id + * required: true + * schema: + * type: string + * - in: query + * name: limit + * schema: + * type: integer + * default: 50 + * maximum: 100 + * description: Max entries to return + * responses: + * "200": + * description: Delivery log entries, newest first + * "401": + * $ref: "#/components/responses/Unauthorized" + */ +router.get('/:guildId/notifications/deliveries', async (req, res) => { + const { guildId } = req.params; + const limit = Math.min(parseInt(req.query.limit, 10) || 50, 100); + + try { + const log = await getDeliveryLog(guildId, limit); + return res.json(log); + } catch (err) { + logError('Failed to fetch delivery log', { guildId, error: err.message }); + return res.status(500).json({ error: 'Failed to fetch delivery log' }); + } +}); + +export default router; diff --git a/src/config-listeners.js b/src/config-listeners.js index c95091701..5aabc3378 100644 --- a/src/config-listeners.js +++ b/src/config-listeners.js @@ -9,6 +9,7 @@ */ import { addPostgresTransport, error, info, removePostgresTransport } from './logger.js'; +import { fireEvent } from './modules/webhookNotifier.js'; import { onConfigChange } from './modules/config.js'; import { cacheDelPattern } from './utils/cache.js'; @@ -109,6 +110,15 @@ export function registerConfigListeners({ dbPool, config }) { await cacheDelPattern(`reputation:${guildId}:*`).catch(() => {}); } }); + + // ── Webhook notifications for config changes ───────────────────────── + onConfigChange('*', async (newValue, _oldValue, path, guildId) => { + // Skip internal/logging changes and notification webhook updates (avoid recursion) + if (path.startsWith('logging.') || path.startsWith('notifications.')) return; + const targetGuildId = guildId && guildId !== 'global' ? guildId : null; + if (!targetGuildId) return; + await fireEvent('config.changed', targetGuildId, { path }).catch(() => {}); + }); } /** diff --git a/src/index.js b/src/index.js index 22197c30c..2150584a1 100644 --- a/src/index.js +++ b/src/index.js @@ -54,11 +54,12 @@ import { startTriage, stopTriage } from './modules/triage.js'; import { closeRedisClient as closeRedis, initRedis } from './redis.js'; import { pruneOldLogs } from './transports/postgres.js'; import { stopCacheCleanup } from './utils/cache.js'; -import { HealthMonitor } from './utils/health.js'; +import { HealthMonitor, MEMORY_DEGRADED_THRESHOLD, EVENT_LOOP_LAG_THRESHOLD_MS, measureEventLoopLag } from './utils/health.js'; import { loadCommandsFromDirectory } from './utils/loadCommands.js'; import { getPermissionError, hasPermission } from './utils/permissions.js'; import { registerCommands } from './utils/registerCommands.js'; import { recordRestart, updateUptimeOnShutdown } from './utils/restartTracker.js'; +import { fireEventAllGuilds } from './modules/webhookNotifier.js'; import { safeFollowUp, safeReply } from './utils/safeSend.js'; // ES module dirname equivalent @@ -119,6 +120,33 @@ client.commands = new Collection(); // Initialize health monitor const healthMonitor = HealthMonitor.getInstance(); +/** @type {ReturnType | null} Health degraded check interval */ +let healthCheckInterval = null; + +/** + * Start the periodic health degraded check. + * Fires health.degraded webhook when memory >80% or event loop lag >100ms. + * + * @param {string[]} guildIds - Guild IDs to notify + */ +function startHealthDegradedCheck() { + if (healthCheckInterval) return; + healthCheckInterval = setInterval(async () => { + const mem = process.memoryUsage(); + const memRatio = mem.heapTotal > 0 ? mem.heapUsed / mem.heapTotal : 0; + const lag = await measureEventLoopLag(); + const degraded = memRatio > MEMORY_DEGRADED_THRESHOLD || lag > EVENT_LOOP_LAG_THRESHOLD_MS; + if (degraded) { + fireEventAllGuilds('health.degraded', { + memoryUsedMb: Math.round(mem.heapUsed / 1024 / 1024), + memoryTotalMb: Math.round(mem.heapTotal / 1024 / 1024), + memoryRatio: Math.round(memRatio * 100), + eventLoopLagMs: lag, + }).catch(() => {}); + } + }, 60_000).unref(); +} + /** * Save conversation history to disk */ @@ -327,7 +355,13 @@ async function gracefulShutdown(signal) { info('Disconnecting from Discord'); client.destroy(); - // 7. Log clean exit + // 7. Stop health check interval + if (healthCheckInterval) { + clearInterval(healthCheckInterval); + healthCheckInterval = null; + } + + // 8. Log clean exit info('Shutdown complete'); process.exit(0); } @@ -344,14 +378,21 @@ client.on('error', (err) => { code: err.code, source: 'discord_client', }); + fireEventAllGuilds('bot.error', { message: err.message, code: err.code }).catch(() => {}); }); client.on('shardDisconnect', (event, shardId) => { if (event.code !== 1000) { warn('Shard disconnected unexpectedly', { shardId, code: event.code, source: 'discord_shard' }); + fireEventAllGuilds('bot.disconnected', { shardId, code: event.code }).catch(() => {}); } }); +client.on('shardResume', (shardId, replayedEvents) => { + info('Shard reconnected', { shardId, replayedEvents, source: 'discord_shard' }); + fireEventAllGuilds('bot.reconnected', { shardId, replayedEvents }).catch(() => {}); +}); + // Start bot const token = process.env.DISCORD_TOKEN; if (!token) { @@ -486,6 +527,9 @@ async function startup() { }) .catch(() => {}); + // Start periodic health degraded check (fires webhooks on threshold breach) + startHealthDegradedCheck(); + // Start REST API server with WebSocket log streaming (non-fatal — bot continues without it) { let wsTransport = null; diff --git a/src/modules/config.js b/src/modules/config.js index d80e322a0..928440cd9 100644 --- a/src/modules/config.js +++ b/src/modules/config.js @@ -350,6 +350,17 @@ function getNestedValue(obj, pathParts) { * @param {string} pathOrPrefix - Dot-notation path or prefix with wildcard * @param {Function} callback - Called with (newValue, oldValue, fullPath, guildId) */ +/** + * Get all guild IDs that have config entries (including 'global'). + * Used by webhook notifier to fire bot-level events to all guilds. + * + * @returns {string[]} Array of guild IDs (excluding 'global') + */ +export function getAllGuildIds() { + return [...configCache.keys()].filter((id) => id !== 'global'); +} + + export function onConfigChange(pathOrPrefix, callback) { listeners.push({ path: pathOrPrefix, callback }); } diff --git a/src/modules/moderation.js b/src/modules/moderation.js index 610c184b6..654de71ce 100644 --- a/src/modules/moderation.js +++ b/src/modules/moderation.js @@ -11,6 +11,7 @@ import { fetchChannelCached } from '../utils/discordCache.js'; import { parseDuration } from '../utils/duration.js'; import { safeSend } from '../utils/safeSend.js'; import { getConfig } from './config.js'; +import { fireEvent } from './webhookNotifier.js'; /** * Color map for mod log embeds by action type. @@ -147,6 +148,17 @@ export async function createCase(guildId, data) { moderator: data.moderatorTag, }); + // Fire webhook notification — fire-and-forget, don't block case creation + fireEvent('moderation.action', guildId, { + action: data.action, + caseNumber: createdCase.case_number, + targetId: data.targetId, + targetTag: data.targetTag, + moderatorId: data.moderatorId, + moderatorTag: data.moderatorTag, + reason: data.reason || null, + }).catch(() => {}); + return createdCase; } catch (err) { await client.query('ROLLBACK').catch(() => {}); diff --git a/src/modules/triage-respond.js b/src/modules/triage-respond.js index 96bb88a68..a1594cce8 100644 --- a/src/modules/triage-respond.js +++ b/src/modules/triage-respond.js @@ -10,6 +10,7 @@ import { fetchChannelCached } from '../utils/discordCache.js'; import { safeSend } from '../utils/safeSend.js'; import { splitMessage } from '../utils/splitMessage.js'; import { addToHistory } from './ai.js'; +import { fireEvent } from './webhookNotifier.js'; import { FEEDBACK_EMOJI, registerAiMessage } from './aiFeedback.js'; import { isProtectedTarget } from './moderation.js'; import { resolveMessageId, sanitizeText } from './triage-filter.js'; @@ -245,6 +246,15 @@ export async function sendResponses( if (type === 'moderate') { warn('Moderation flagged', { channelId, reasoning: classification.reasoning }); + // Fire member.flagged webhook notification + const guildId = channel?.guild?.id; + if (guildId) { + fireEvent('member.flagged', guildId, { + channelId, + reasoning: classification.reasoning?.slice(0, 500), + flaggedUsers: classification.flaggedUsers?.map((u) => u.userId || u) || [], + }).catch(() => {}); + } if (triageConfig.moderationResponse !== false && responses.length > 0) { for (const r of responses) { diff --git a/src/modules/webhookNotifier.js b/src/modules/webhookNotifier.js new file mode 100644 index 000000000..a490149cb --- /dev/null +++ b/src/modules/webhookNotifier.js @@ -0,0 +1,303 @@ +/** + * Webhook Notifier Module + * + * Delivers outbound webhook notifications to configured endpoints when + * important bot events occur. Supports HMAC-SHA256 signing, per-guild + * endpoint configuration, exponential backoff retry, and delivery logging. + * + * Event types: + * bot.disconnected - Discord gateway disconnection + * bot.reconnected - Successful reconnection + * bot.error - Unhandled error + * moderation.action - Warning/ban/kick issued + * health.degraded - Memory >80% or event loop lag >100ms + * config.changed - Config updated via dashboard + * member.flagged - AI flagged a member's message + */ + +import { createHmac } from 'node:crypto'; +import { getPool } from '../db.js'; +import { error as logError, info, warn } from '../logger.js'; +import { getConfig } from './config.js'; + +/** @type {string[]} All supported event types */ +export const WEBHOOK_EVENTS = [ + 'bot.disconnected', + 'bot.reconnected', + 'bot.error', + 'moderation.action', + 'health.degraded', + 'config.changed', + 'member.flagged', +]; + +/** Retry delays in ms (3 attempts: 1s, 3s, 9s) */ +const RETRY_DELAYS_MS = [1000, 3000, 9000]; + +/** Max delivery log entries per guild */ +const MAX_LOG_ENTRIES = 100; + +/** Fetch timeout per attempt (ms) */ +const FETCH_TIMEOUT_MS = 10000; + +/** + * Sign a payload with HMAC-SHA256 using the endpoint secret. + * + * @param {string} secret - Shared secret for the endpoint + * @param {string} body - Serialised JSON payload string + * @returns {string} Hex digest prefixed with "sha256=" + */ +export function signPayload(secret, body) { + return `sha256=${createHmac('sha256', secret).update(body, 'utf8').digest('hex')}`; +} + +/** + * Perform a single HTTP delivery attempt. + * + * @param {string} url - Endpoint URL + * @param {string} secret - HMAC secret (empty string = no signature header) + * @param {string} body - Serialised JSON payload + * @returns {Promise<{ok: boolean, status: number, text: string}>} + */ +async function attemptDelivery(url, secret, body) { + const headers = { + 'Content-Type': 'application/json', + 'User-Agent': 'VolvoxBot-Webhooks/1.0', + }; + + if (secret) { + headers['X-Signature-256'] = signPayload(secret, body); + } + + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS); + + try { + const res = await fetch(url, { + method: 'POST', + headers, + body, + signal: controller.signal, + }); + + const text = await res.text().catch(() => ''); + return { ok: res.ok, status: res.status, text }; + } catch (err) { + // Network error or timeout + return { ok: false, status: 0, text: err.message }; + } finally { + clearTimeout(timer); + } +} + +/** + * Sleep for ms milliseconds. + * @param {number} ms + * @returns {Promise} + */ +function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +/** + * Deliver a webhook payload to a single endpoint with exponential backoff retry. + * Records each attempt in the delivery log. + * + * @param {string} guildId - Guild that owns this endpoint + * @param {Object} endpoint - Endpoint config from notifications.webhooks[] + * @param {string} endpoint.id - Unique identifier for the endpoint + * @param {string} endpoint.url - Delivery URL + * @param {string} [endpoint.secret] - HMAC secret + * @param {Object} payload - Event payload object + * @returns {Promise} True if any attempt succeeded + */ +export async function deliverToEndpoint(guildId, endpoint, payload) { + const body = JSON.stringify(payload); + const pool = getPool(); + + for (let attempt = 1; attempt <= RETRY_DELAYS_MS.length + 1; attempt++) { + const result = await attemptDelivery(endpoint.url, endpoint.secret || '', body); + + // Log this attempt + if (pool) { + try { + await pool.query( + `INSERT INTO webhook_delivery_log + (guild_id, endpoint_id, event_type, payload, status, response_code, response_body, attempt) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8)`, + [ + guildId, + endpoint.id, + payload.event, + payload, + result.ok ? 'success' : 'failed', + result.status || null, + result.text?.slice(0, 2000) || null, + attempt, + ], + ); + + // Prune old log entries for this guild (keep most recent MAX_LOG_ENTRIES) + await pool.query( + `DELETE FROM webhook_delivery_log + WHERE guild_id = $1 + AND id NOT IN ( + SELECT id FROM webhook_delivery_log + WHERE guild_id = $1 + ORDER BY delivered_at DESC + LIMIT $2 + )`, + [guildId, MAX_LOG_ENTRIES], + ); + } catch (dbErr) { + warn('Failed to log webhook delivery', { error: dbErr.message }); + } + } + + if (result.ok) { + info('Webhook delivered', { + guildId, + endpointId: endpoint.id, + event: payload.event, + attempt, + status: result.status, + }); + return true; + } + + const isLastAttempt = attempt > RETRY_DELAYS_MS.length; + if (isLastAttempt) { + logError('Webhook delivery failed after all retries', { + guildId, + endpointId: endpoint.id, + event: payload.event, + status: result.status, + body: result.text?.slice(0, 500), + }); + return false; + } + + const delay = RETRY_DELAYS_MS[attempt - 1]; + warn('Webhook delivery failed, retrying', { + guildId, + endpointId: endpoint.id, + event: payload.event, + attempt, + nextRetryMs: delay, + status: result.status, + }); + await sleep(delay); + } + + return false; +} + +/** + * Fire a webhook event to all configured endpoints for a guild that subscribe + * to this event type. Deliveries run in parallel and are fire-and-forget + * (errors don't propagate to callers). + * + * @param {string} eventType - One of the WEBHOOK_EVENTS constants + * @param {string} guildId - Guild ID ('global' for bot-level events) + * @param {Object} data - Event-specific data to include in payload + * @returns {Promise} + */ +export async function fireEvent(eventType, guildId, data = {}) { + let endpoints; + try { + const cfg = getConfig(guildId); + endpoints = cfg?.notifications?.webhooks; + } catch { + // Config not loaded for this guild — skip + return; + } + + if (!Array.isArray(endpoints) || endpoints.length === 0) return; + + // Filter to endpoints that subscribe to this event + const targets = endpoints.filter( + (ep) => + ep && + ep.url && + ep.enabled !== false && + (Array.isArray(ep.events) ? ep.events.includes(eventType) : true), + ); + + if (targets.length === 0) return; + + const payload = { + event: eventType, + timestamp: new Date().toISOString(), + guild_id: guildId, + data, + }; + + // Fire-and-forget — parallel delivery, don't block caller + Promise.all(targets.map((ep) => deliverToEndpoint(guildId, ep, payload))).catch((err) => { + logError('Unexpected error in webhook delivery batch', { error: err.message }); + }); +} + +/** + * Get the delivery log for a guild. + * + * @param {string} guildId - Guild ID + * @param {number} [limit=100] - Max entries to return + * @returns {Promise} Delivery log entries, newest first + */ +export async function getDeliveryLog(guildId, limit = 50) { + const pool = getPool(); + if (!pool) return []; + + const { rows } = await pool.query( + `SELECT id, endpoint_id, event_type, status, response_code, response_body, attempt, delivered_at + FROM webhook_delivery_log + WHERE guild_id = $1 + ORDER BY delivered_at DESC + LIMIT $2`, + [guildId, Math.min(limit, MAX_LOG_ENTRIES)], + ); + + return rows; +} + +/** + * Send a test event to a specific endpoint. Used by the dashboard. + * + * @param {string} guildId - Guild ID + * @param {Object} endpoint - Endpoint config + * @returns {Promise<{ok: boolean, status: number, text: string}>} + */ +export async function testEndpoint(guildId, endpoint) { + const payload = { + event: 'test', + timestamp: new Date().toISOString(), + guild_id: guildId, + data: { message: 'This is a test webhook from VolvoxBot.' }, + }; + const body = JSON.stringify(payload); + return attemptDelivery(endpoint.url, endpoint.secret || '', body); +} + +/** + * Fire a webhook event for all configured guilds that subscribe to this event. + * Use this for bot-level events (disconnect, error) that aren't guild-specific. + * + * @param {string} eventType - One of the WEBHOOK_EVENTS constants + * @param {Object} data - Event-specific data to include in payload + * @param {string[]} [guildIds] - Guild IDs to fire for (defaults to all guilds with webhook configs) + * @returns {Promise} + */ +export async function fireEventAllGuilds(eventType, data = {}, guildIds) { + let targets = guildIds; + + if (!targets) { + // Import here to avoid circular dependency at module load time + const { getAllGuildIds } = await import('./config.js'); + targets = getAllGuildIds ? getAllGuildIds() : []; + } + + await Promise.all(targets.map((gid) => fireEvent(eventType, gid, data))).catch((err) => { + logError('Unexpected error in fireEventAllGuilds', { error: err.message }); + }); +} diff --git a/src/utils/health.js b/src/utils/health.js index 8d419c496..80cc9a9b3 100644 --- a/src/utils/health.js +++ b/src/utils/health.js @@ -157,3 +157,28 @@ class HealthMonitor { } export { HealthMonitor }; + +/** + * Memory usage threshold (%) above which health.degraded is fired. + * heapUsed / heapTotal > this fraction triggers the event. + */ +export const MEMORY_DEGRADED_THRESHOLD = 0.8; + +/** + * Event loop lag threshold in ms above which health.degraded is fired. + */ +export const EVENT_LOOP_LAG_THRESHOLD_MS = 100; + +/** + * Measure approximate event loop lag in milliseconds. + * Schedules a setImmediate and measures how long it took. + * + * @returns {Promise} Lag in milliseconds + */ +export function measureEventLoopLag() { + return new Promise((resolve) => { + const start = Date.now(); + setImmediate(() => resolve(Date.now() - start)); + }); +} + diff --git a/tests/api/routes/notifications.test.js b/tests/api/routes/notifications.test.js new file mode 100644 index 000000000..827cb30b4 --- /dev/null +++ b/tests/api/routes/notifications.test.js @@ -0,0 +1,355 @@ +import jwt from 'jsonwebtoken'; +import request from 'supertest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../../../src/logger.js', () => ({ + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), +})); + +vi.mock('../../../src/modules/config.js', () => ({ + getConfig: vi.fn(), + setConfigValue: vi.fn().mockResolvedValue({}), +})); + +vi.mock('../../../src/modules/webhookNotifier.js', () => ({ + WEBHOOK_EVENTS: [ + 'bot.disconnected', + 'bot.reconnected', + 'bot.error', + 'moderation.action', + 'health.degraded', + 'config.changed', + 'member.flagged', + ], + getDeliveryLog: vi.fn().mockResolvedValue([]), + testEndpoint: vi.fn().mockResolvedValue({ ok: true, status: 200, text: 'OK' }), +})); + +import { _resetSecretCache } from '../../../src/api/middleware/verifyJwt.js'; +import { createApp } from '../../../src/api/server.js'; +import { guildCache } from '../../../src/api/utils/discordApi.js'; +import { sessionStore } from '../../../src/api/utils/sessionStore.js'; +import { getConfig, setConfigValue } from '../../../src/modules/config.js'; +import { getDeliveryLog, testEndpoint } from '../../../src/modules/webhookNotifier.js'; + +describe('notifications routes', () => { + let app; + const SECRET = 'test-secret'; + const GUILD_ID = 'guild123'; + + const baseWebhook = { + id: 'uuid-1', + url: 'https://example.com/hook', + events: ['bot.error'], + enabled: true, + }; + + beforeEach(() => { + vi.stubEnv('BOT_API_SECRET', SECRET); + + const client = { + guilds: { cache: new Map() }, + ws: { status: 0, ping: 42 }, + user: { tag: 'Bot#1234' }, + }; + + app = createApp(client, null); + + getConfig.mockReturnValue({ + notifications: { webhooks: [baseWebhook] }, + }); + }); + + afterEach(() => { + sessionStore.clear(); + guildCache.clear(); + _resetSecretCache(); + vi.clearAllMocks(); + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + }); + + function authHeaders() { + return { 'x-api-secret': SECRET }; + } + + // ── GET /guilds/:guildId/notifications/webhooks ─────────────────────────── + + describe('GET /guilds/:guildId/notifications/webhooks', () => { + it('should list webhooks with secrets masked', async () => { + getConfig.mockReturnValue({ + notifications: { + webhooks: [{ ...baseWebhook, secret: 'mysecret' }], + }, + }); + + const res = await request(app) + .get(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()); + + expect(res.status).toBe(200); + expect(Array.isArray(res.body)).toBe(true); + expect(res.body[0].secret).toBeUndefined(); + expect(res.body[0].hasSecret).toBe(true); + expect(res.body[0].url).toBe('https://example.com/hook'); + }); + + it('should return empty array when no webhooks configured', async () => { + getConfig.mockReturnValue({ notifications: { webhooks: [] } }); + + const res = await request(app) + .get(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()); + + expect(res.status).toBe(200); + expect(res.body).toEqual([]); + }); + + it('should return empty array when notifications not in config', async () => { + getConfig.mockReturnValue({}); + + const res = await request(app) + .get(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()); + + expect(res.status).toBe(200); + expect(res.body).toEqual([]); + }); + + it('should return 401 without auth', async () => { + const res = await request(app) + .get(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`); + + expect(res.status).toBe(401); + }); + + it('should indicate hasSecret=false when no secret', async () => { + getConfig.mockReturnValue({ + notifications: { webhooks: [baseWebhook] }, + }); + + const res = await request(app) + .get(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()); + + expect(res.body[0].hasSecret).toBe(false); + }); + }); + + // ── POST /guilds/:guildId/notifications/webhooks ────────────────────────── + + describe('POST /guilds/:guildId/notifications/webhooks', () => { + beforeEach(() => { + // Start fresh with no webhooks + getConfig.mockReturnValue({ notifications: { webhooks: [] } }); + }); + + it('should add a webhook endpoint', async () => { + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()) + .send({ url: 'https://example.com/hook', events: ['bot.error'] }); + + expect(res.status).toBe(201); + expect(res.body.url).toBe('https://example.com/hook'); + expect(res.body.events).toEqual(['bot.error']); + expect(res.body.id).toBeTruthy(); + expect(res.body.secret).toBeUndefined(); + expect(setConfigValue).toHaveBeenCalledWith( + 'notifications.webhooks', + expect.arrayContaining([expect.objectContaining({ url: 'https://example.com/hook' })]), + GUILD_ID, + ); + }); + + it('should return 400 when url is missing', async () => { + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()) + .send({ events: ['bot.error'] }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('url'); + }); + + it('should return 400 for invalid URL format', async () => { + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()) + .send({ url: 'not-a-url', events: ['bot.error'] }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('URL'); + }); + + it('should return 400 when events is missing', async () => { + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()) + .send({ url: 'https://example.com/hook' }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('events'); + }); + + it('should return 400 when events is empty array', async () => { + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()) + .send({ url: 'https://example.com/hook', events: [] }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('events'); + }); + + it('should return 400 for invalid event types', async () => { + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()) + .send({ url: 'https://example.com/hook', events: ['not.a.real.event'] }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('Invalid event types'); + expect(res.body.validEvents).toBeTruthy(); + }); + + it('should return 400 when exceeding 20 endpoints', async () => { + const existing = Array.from({ length: 20 }, (_, i) => ({ id: `ep${i}`, url: 'https://x.com', events: ['bot.error'] })); + getConfig.mockReturnValue({ notifications: { webhooks: existing } }); + + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()) + .send({ url: 'https://example.com/hook', events: ['bot.error'] }); + + expect(res.status).toBe(400); + expect(res.body.error).toContain('Maximum'); + }); + + it('should accept optional secret', async () => { + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()) + .send({ url: 'https://example.com/hook', events: ['bot.error'], secret: 'topsecret' }); + + expect(res.status).toBe(201); + expect(res.body.secret).toBeUndefined(); + expect(res.body.hasSecret).toBe(true); + + // Secret should be saved in the config + const saved = setConfigValue.mock.calls[0][1][0]; + expect(saved.secret).toBe('topsecret'); + }); + + it('should return 401 without auth', async () => { + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .send({ url: 'https://example.com/hook', events: ['bot.error'] }); + + expect(res.status).toBe(401); + }); + }); + + // ── DELETE /guilds/:guildId/notifications/webhooks/:endpointId ──────────── + + describe('DELETE /guilds/:guildId/notifications/webhooks/:endpointId', () => { + it('should remove an existing endpoint', async () => { + const res = await request(app) + .delete(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks/uuid-1`) + .set(authHeaders()); + + expect(res.status).toBe(204); + expect(setConfigValue).toHaveBeenCalledWith( + 'notifications.webhooks', + [], + GUILD_ID, + ); + }); + + it('should return 404 when endpoint not found', async () => { + const res = await request(app) + .delete(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks/nonexistent-id`) + .set(authHeaders()); + + expect(res.status).toBe(404); + expect(res.body.error).toContain('not found'); + }); + + it('should return 401 without auth', async () => { + const res = await request(app) + .delete(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks/uuid-1`); + + expect(res.status).toBe(401); + }); + }); + + // ── POST /guilds/:guildId/notifications/webhooks/:endpointId/test ───────── + + describe('POST /guilds/:guildId/notifications/webhooks/:endpointId/test', () => { + it('should send a test event and return result', async () => { + testEndpoint.mockResolvedValueOnce({ ok: true, status: 200, text: 'received' }); + + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks/uuid-1/test`) + .set(authHeaders()); + + expect(res.status).toBe(200); + expect(res.body.ok).toBe(true); + expect(res.body.status).toBe(200); + expect(testEndpoint).toHaveBeenCalledWith(GUILD_ID, baseWebhook); + }); + + it('should return 404 when endpoint not found', async () => { + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks/bad-id/test`) + .set(authHeaders()); + + expect(res.status).toBe(404); + }); + + it('should return 401 without auth', async () => { + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks/uuid-1/test`); + + expect(res.status).toBe(401); + }); + }); + + // ── GET /guilds/:guildId/notifications/deliveries ───────────────────────── + + describe('GET /guilds/:guildId/notifications/deliveries', () => { + it('should return delivery log', async () => { + const rows = [ + { id: 1, endpoint_id: 'ep1', event_type: 'bot.error', status: 'success', attempt: 1, delivered_at: '2026-01-01' }, + ]; + getDeliveryLog.mockResolvedValueOnce(rows); + + const res = await request(app) + .get(`/api/v1/guilds/${GUILD_ID}/notifications/deliveries`) + .set(authHeaders()); + + expect(res.status).toBe(200); + expect(res.body).toEqual(rows); + expect(getDeliveryLog).toHaveBeenCalledWith(GUILD_ID, 50); + }); + + it('should accept limit query param (capped at 100)', async () => { + getDeliveryLog.mockResolvedValueOnce([]); + + await request(app) + .get(`/api/v1/guilds/${GUILD_ID}/notifications/deliveries?limit=200`) + .set(authHeaders()); + + expect(getDeliveryLog).toHaveBeenCalledWith(GUILD_ID, 100); + }); + + it('should return 401 without auth', async () => { + const res = await request(app) + .get(`/api/v1/guilds/${GUILD_ID}/notifications/deliveries`); + + expect(res.status).toBe(401); + }); + }); +}); diff --git a/tests/modules/webhookNotifier.test.js b/tests/modules/webhookNotifier.test.js new file mode 100644 index 000000000..a65ff76be --- /dev/null +++ b/tests/modules/webhookNotifier.test.js @@ -0,0 +1,438 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Mocks must be hoisted before imports +vi.mock('../../src/db.js', () => ({ + getPool: vi.fn(), +})); + +vi.mock('../../src/logger.js', () => ({ + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), +})); + +vi.mock('../../src/modules/config.js', () => ({ + getConfig: vi.fn(), +})); + +import { getPool } from '../../src/db.js'; +import { error as logError, info, warn } from '../../src/logger.js'; +import { getConfig } from '../../src/modules/config.js'; +import { + deliverToEndpoint, + fireEvent, + getDeliveryLog, + signPayload, + testEndpoint, + WEBHOOK_EVENTS, +} from '../../src/modules/webhookNotifier.js'; + +describe('webhookNotifier', () => { + let mockFetch; + let mockPool; + + beforeEach(() => { + vi.clearAllMocks(); + + mockPool = { + query: vi.fn().mockResolvedValue({ rows: [] }), + }; + getPool.mockReturnValue(mockPool); + + // Default: no webhooks configured + getConfig.mockReturnValue({ notifications: { webhooks: [] } }); + + // Mock global fetch + mockFetch = vi.fn(); + global.fetch = mockFetch; + }); + + afterEach(() => { + vi.restoreAllMocks(); + delete global.fetch; + }); + + // ── signPayload ────────────────────────────────────────────────────────── + + describe('signPayload', () => { + it('should return sha256= prefixed HMAC hex', () => { + const sig = signPayload('mysecret', '{"hello":"world"}'); + expect(sig).toMatch(/^sha256=[a-f0-9]{64}$/); + }); + + it('should produce consistent signatures for same inputs', () => { + const body = '{"event":"test"}'; + expect(signPayload('secret', body)).toBe(signPayload('secret', body)); + }); + + it('should produce different signatures for different secrets', () => { + const body = '{"event":"test"}'; + expect(signPayload('secret1', body)).not.toBe(signPayload('secret2', body)); + }); + }); + + // ── WEBHOOK_EVENTS ─────────────────────────────────────────────────────── + + describe('WEBHOOK_EVENTS', () => { + it('should include all required event types', () => { + const required = [ + 'bot.disconnected', + 'bot.reconnected', + 'bot.error', + 'moderation.action', + 'health.degraded', + 'config.changed', + 'member.flagged', + ]; + for (const evt of required) { + expect(WEBHOOK_EVENTS).toContain(evt); + } + }); + }); + + // ── deliverToEndpoint ──────────────────────────────────────────────────── + + describe('deliverToEndpoint', () => { + const endpoint = { + id: 'ep1', + url: 'https://example.com/hook', + secret: 'mysecret', + }; + const payload = { event: 'test', timestamp: '2026-01-01T00:00:00Z', guild_id: 'guild1', data: {} }; + + it('should deliver successfully on first attempt', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + text: async () => 'OK', + }); + + const result = await deliverToEndpoint('guild1', endpoint, payload); + expect(result).toBe(true); + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(info).toHaveBeenCalledWith('Webhook delivered', expect.objectContaining({ + guildId: 'guild1', + endpointId: 'ep1', + attempt: 1, + })); + }); + + it('should include HMAC signature header', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + text: async () => 'OK', + }); + + await deliverToEndpoint('guild1', endpoint, payload); + + const [url, opts] = mockFetch.mock.calls[0]; + expect(url).toBe('https://example.com/hook'); + expect(opts.headers['X-Signature-256']).toMatch(/^sha256=[a-f0-9]{64}$/); + }); + + it('should omit signature header when no secret', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + text: async () => 'OK', + }); + + const epNoSecret = { id: 'ep2', url: 'https://example.com/hook' }; + await deliverToEndpoint('guild1', epNoSecret, payload); + + const [, opts] = mockFetch.mock.calls[0]; + expect(opts.headers['X-Signature-256']).toBeUndefined(); + }); + + it('should retry on failure with exponential backoff', async () => { + vi.useFakeTimers(); + + // Fail twice, succeed on 3rd attempt + mockFetch + .mockResolvedValueOnce({ ok: false, status: 500, text: async () => 'Error' }) + .mockResolvedValueOnce({ ok: false, status: 503, text: async () => 'Unavailable' }) + .mockResolvedValueOnce({ ok: true, status: 200, text: async () => 'OK' }); + + const deliverPromise = deliverToEndpoint('guild1', endpoint, payload); + + // Advance through retry delays + await vi.advanceTimersByTimeAsync(1000); + await vi.advanceTimersByTimeAsync(3000); + + const result = await deliverPromise; + vi.useRealTimers(); + + expect(result).toBe(true); + expect(mockFetch).toHaveBeenCalledTimes(3); + }); + + it('should return false after all retries exhausted', async () => { + vi.useFakeTimers(); + + mockFetch.mockResolvedValue({ ok: false, status: 500, text: async () => 'Error' }); + + const deliverPromise = deliverToEndpoint('guild1', endpoint, payload); + + await vi.advanceTimersByTimeAsync(1000); + await vi.advanceTimersByTimeAsync(3000); + await vi.advanceTimersByTimeAsync(9000); + + const result = await deliverPromise; + vi.useRealTimers(); + + expect(result).toBe(false); + expect(logError).toHaveBeenCalledWith('Webhook delivery failed after all retries', expect.any(Object)); + }); + + it('should handle network errors (fetch throws)', async () => { + vi.useFakeTimers(); + + mockFetch.mockRejectedValue(new Error('ECONNREFUSED')); + + const deliverPromise = deliverToEndpoint('guild1', endpoint, payload); + + await vi.advanceTimersByTimeAsync(1000); + await vi.advanceTimersByTimeAsync(3000); + await vi.advanceTimersByTimeAsync(9000); + + const result = await deliverPromise; + vi.useRealTimers(); + + expect(result).toBe(false); + }); + + it('should log each attempt to delivery log', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + text: async () => 'OK', + }); + + await deliverToEndpoint('guild1', endpoint, payload); + + expect(mockPool.query).toHaveBeenCalledWith( + expect.stringContaining('INSERT INTO webhook_delivery_log'), + expect.arrayContaining(['guild1', 'ep1', 'test']), + ); + }); + + it('should prune old log entries after each delivery', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + text: async () => 'OK', + }); + + await deliverToEndpoint('guild1', endpoint, payload); + + expect(mockPool.query).toHaveBeenCalledWith( + expect.stringContaining('DELETE FROM webhook_delivery_log'), + ['guild1', 100], + ); + }); + + it('should work without a DB pool (no pool configured)', async () => { + getPool.mockReturnValue(null); + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + text: async () => 'OK', + }); + + const result = await deliverToEndpoint('guild1', endpoint, payload); + expect(result).toBe(true); + }); + + it('should handle DB logging errors gracefully', async () => { + mockPool.query.mockRejectedValueOnce(new Error('DB error')); + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + text: async () => 'OK', + }); + + const result = await deliverToEndpoint('guild1', endpoint, payload); + expect(result).toBe(true); + expect(warn).toHaveBeenCalledWith('Failed to log webhook delivery', expect.any(Object)); + }); + }); + + // ── fireEvent ──────────────────────────────────────────────────────────── + + describe('fireEvent', () => { + it('should not fire when no webhooks configured', async () => { + getConfig.mockReturnValue({ notifications: { webhooks: [] } }); + await fireEvent('bot.error', 'guild1', { message: 'oops' }); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('should not fire when notifications not in config', async () => { + getConfig.mockReturnValue({}); + await fireEvent('bot.error', 'guild1', {}); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('should fire to endpoints subscribed to the event', async () => { + getConfig.mockReturnValue({ + notifications: { + webhooks: [ + { id: 'ep1', url: 'https://example.com/hook', events: ['bot.error'], enabled: true }, + ], + }, + }); + mockFetch.mockResolvedValue({ ok: true, status: 200, text: async () => 'OK' }); + + await fireEvent('bot.error', 'guild1', { message: 'crash' }); + // Give microtasks time to settle + await new Promise((r) => setTimeout(r, 0)); + + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it('should not fire to endpoints not subscribed to the event', async () => { + getConfig.mockReturnValue({ + notifications: { + webhooks: [ + { id: 'ep1', url: 'https://example.com/hook', events: ['moderation.action'], enabled: true }, + ], + }, + }); + + await fireEvent('bot.error', 'guild1', {}); + await new Promise((r) => setTimeout(r, 0)); + + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('should not fire to disabled endpoints', async () => { + getConfig.mockReturnValue({ + notifications: { + webhooks: [ + { id: 'ep1', url: 'https://example.com/hook', events: ['bot.error'], enabled: false }, + ], + }, + }); + + await fireEvent('bot.error', 'guild1', {}); + await new Promise((r) => setTimeout(r, 0)); + + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('should fire to multiple subscribed endpoints', async () => { + getConfig.mockReturnValue({ + notifications: { + webhooks: [ + { id: 'ep1', url: 'https://a.com/hook', events: ['bot.error'], enabled: true }, + { id: 'ep2', url: 'https://b.com/hook', events: ['bot.error'], enabled: true }, + ], + }, + }); + mockFetch.mockResolvedValue({ ok: true, status: 200, text: async () => 'OK' }); + + await fireEvent('bot.error', 'guild1', {}); + await new Promise((r) => setTimeout(r, 0)); + + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('should include structured payload with event/timestamp/guild_id', async () => { + getConfig.mockReturnValue({ + notifications: { + webhooks: [ + { id: 'ep1', url: 'https://example.com/hook', events: ['moderation.action'], enabled: true }, + ], + }, + }); + mockFetch.mockResolvedValue({ ok: true, status: 200, text: async () => 'OK' }); + + await fireEvent('moderation.action', 'guild1', { action: 'ban' }); + await new Promise((r) => setTimeout(r, 0)); + + const [, opts] = mockFetch.mock.calls[0]; + const body = JSON.parse(opts.body); + expect(body.event).toBe('moderation.action'); + expect(body.guild_id).toBe('guild1'); + expect(body.data.action).toBe('ban'); + expect(body.timestamp).toBeTruthy(); + }); + + it('should handle config errors gracefully (returns without firing)', async () => { + getConfig.mockImplementation(() => { throw new Error('config not loaded'); }); + await expect(fireEvent('bot.error', 'guild1', {})).resolves.not.toThrow(); + expect(mockFetch).not.toHaveBeenCalled(); + }); + }); + + // ── getDeliveryLog ─────────────────────────────────────────────────────── + + describe('getDeliveryLog', () => { + it('should query the delivery log for a guild', async () => { + const rows = [ + { id: 1, endpoint_id: 'ep1', event_type: 'bot.error', status: 'success', attempt: 1, delivered_at: '2026-01-01' }, + ]; + mockPool.query.mockResolvedValueOnce({ rows }); + + const result = await getDeliveryLog('guild1'); + expect(result).toEqual(rows); + expect(mockPool.query).toHaveBeenCalledWith( + expect.stringContaining('SELECT'), + ['guild1', 50], + ); + }); + + it('should return empty array when no pool', async () => { + getPool.mockReturnValue(null); + const result = await getDeliveryLog('guild1'); + expect(result).toEqual([]); + }); + + it('should cap limit at 100', async () => { + mockPool.query.mockResolvedValueOnce({ rows: [] }); + await getDeliveryLog('guild1', 9999); + expect(mockPool.query).toHaveBeenCalledWith( + expect.any(String), + ['guild1', 100], + ); + }); + + it('should default limit to 50', async () => { + mockPool.query.mockResolvedValueOnce({ rows: [] }); + await getDeliveryLog('guild1'); + expect(mockPool.query).toHaveBeenCalledWith(expect.any(String), ['guild1', 50]); + }); + }); + + // ── testEndpoint ───────────────────────────────────────────────────────── + + describe('testEndpoint', () => { + it('should send a test payload and return result', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + text: async () => 'received', + }); + + const endpoint = { id: 'ep1', url: 'https://example.com/hook', secret: 'sec' }; + const result = await testEndpoint('guild1', endpoint); + + expect(result.ok).toBe(true); + expect(result.status).toBe(200); + expect(result.text).toBe('received'); + + const [, opts] = mockFetch.mock.calls[0]; + const body = JSON.parse(opts.body); + expect(body.event).toBe('test'); + expect(body.data.message).toBeTruthy(); + }); + + it('should return failure result on network error', async () => { + mockFetch.mockRejectedValueOnce(new Error('connection refused')); + const endpoint = { id: 'ep1', url: 'https://example.com/hook' }; + const result = await testEndpoint('guild1', endpoint); + expect(result.ok).toBe(false); + expect(result.status).toBe(0); + }); + }); +}); From f750a4e4534da816a7c66210b785d79a1c4e0d4e Mon Sep 17 00:00:00 2001 From: Pip Build Date: Sun, 1 Mar 2026 23:50:54 -0500 Subject: [PATCH 02/21] test(config-listeners): update listener count to 12 for webhook config.changed listener --- tests/config-listeners.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/config-listeners.test.js b/tests/config-listeners.test.js index e89452d1f..3101d6944 100644 --- a/tests/config-listeners.test.js +++ b/tests/config-listeners.test.js @@ -94,10 +94,10 @@ describe('config-listeners', () => { expect(registeredKeys).toContain('reputation.*'); }); - it('registers exactly 11 listeners', () => { + it('registers exactly 12 listeners', () => { const config = { logging: { database: { enabled: false } } }; registerConfigListeners({ dbPool: {}, config }); - expect(onConfigChange).toHaveBeenCalledTimes(11); + expect(onConfigChange).toHaveBeenCalledTimes(12); }); }); From 1c854c64a2967dd627c79b79d12b1240c7ae0a77 Mon Sep 17 00:00:00 2001 From: Pip Build Date: Mon, 2 Mar 2026 00:16:26 -0500 Subject: [PATCH 03/21] fix(webhooks): restrict delivery URL validation to https only --- src/api/routes/notifications.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/api/routes/notifications.js b/src/api/routes/notifications.js index 7703de08c..d2c34a130 100644 --- a/src/api/routes/notifications.js +++ b/src/api/routes/notifications.js @@ -120,8 +120,8 @@ router.post('/:guildId/notifications/webhooks', async (req, res) => { return res.status(400).json({ error: 'Missing or invalid "url"' }); } - if (!/^https?:\/\/.+/.test(url)) { - return res.status(400).json({ error: '"url" must be a valid HTTP/HTTPS URL' }); + if (!/^https:\/\/.+/.test(url)) { + return res.status(400).json({ error: '"url" must be a valid HTTPS URL' }); } if (!Array.isArray(events) || events.length === 0) { From 1f619ecc3443df82949483de39292b9d1f52bc16 Mon Sep 17 00:00:00 2001 From: Pip Build Date: Mon, 2 Mar 2026 00:16:34 -0500 Subject: [PATCH 04/21] chore: remove accidentally committed feat-issue-164 submodule --- feat-issue-164 | 1 - 1 file changed, 1 deletion(-) delete mode 160000 feat-issue-164 diff --git a/feat-issue-164 b/feat-issue-164 deleted file mode 160000 index b9b1db37d..000000000 --- a/feat-issue-164 +++ /dev/null @@ -1 +0,0 @@ -Subproject commit b9b1db37dcf8b2d5c3838d9a80eabc385f42aa59 From 5a6860ee3ef1bd3ac331e024b69d579ec23e07bb Mon Sep 17 00:00:00 2001 From: Pip Build Date: Mon, 2 Mar 2026 04:21:20 -0500 Subject: [PATCH 05/21] chore: fix lint errors (biome auto-fix + exclude stray worktree dir) --- biome.json | 3 +- src/api/index.js | 2 +- src/api/routes/notifications.js | 2 +- src/config-listeners.js | 2 +- src/index.js | 9 +++- src/modules/config.js | 1 - src/modules/triage-respond.js | 2 +- src/modules/webhookNotifier.js | 2 +- src/utils/health.js | 1 - tests/api/routes/notifications.test.js | 37 ++++++++------ tests/modules/webhookNotifier.test.js | 67 ++++++++++++++++++-------- 11 files changed, 83 insertions(+), 45 deletions(-) diff --git a/biome.json b/biome.json index 0f4c96575..84aba3023 100644 --- a/biome.json +++ b/biome.json @@ -8,7 +8,8 @@ "!node_modules", "!coverage", "!logs", - "!data" + "!data", + "!feat-issue-164" ] }, "linter": { diff --git a/src/api/index.js b/src/api/index.js index edf599437..7ed27a07c 100644 --- a/src/api/index.js +++ b/src/api/index.js @@ -16,8 +16,8 @@ import guildsRouter from './routes/guilds.js'; import healthRouter from './routes/health.js'; import membersRouter from './routes/members.js'; import moderationRouter from './routes/moderation.js'; -import ticketsRouter from './routes/tickets.js'; import notificationsRouter from './routes/notifications.js'; +import ticketsRouter from './routes/tickets.js'; import webhooksRouter from './routes/webhooks.js'; const router = Router(); diff --git a/src/api/routes/notifications.js b/src/api/routes/notifications.js index d2c34a130..68765c767 100644 --- a/src/api/routes/notifications.js +++ b/src/api/routes/notifications.js @@ -8,7 +8,7 @@ import { randomUUID } from 'node:crypto'; import { Router } from 'express'; -import { error as logError, info } from '../../logger.js'; +import { info, error as logError } from '../../logger.js'; import { getConfig, setConfigValue } from '../../modules/config.js'; import { getDeliveryLog, testEndpoint, WEBHOOK_EVENTS } from '../../modules/webhookNotifier.js'; diff --git a/src/config-listeners.js b/src/config-listeners.js index 5aabc3378..2596c7038 100644 --- a/src/config-listeners.js +++ b/src/config-listeners.js @@ -9,8 +9,8 @@ */ import { addPostgresTransport, error, info, removePostgresTransport } from './logger.js'; -import { fireEvent } from './modules/webhookNotifier.js'; import { onConfigChange } from './modules/config.js'; +import { fireEvent } from './modules/webhookNotifier.js'; import { cacheDelPattern } from './utils/cache.js'; /** @type {import('winston').transport | null} */ diff --git a/src/index.js b/src/index.js index 2150584a1..1f0ad290a 100644 --- a/src/index.js +++ b/src/index.js @@ -51,15 +51,20 @@ import { startTempbanScheduler, stopTempbanScheduler } from './modules/moderatio import { loadOptOuts } from './modules/optout.js'; import { startScheduler, stopScheduler } from './modules/scheduler.js'; import { startTriage, stopTriage } from './modules/triage.js'; +import { fireEventAllGuilds } from './modules/webhookNotifier.js'; import { closeRedisClient as closeRedis, initRedis } from './redis.js'; import { pruneOldLogs } from './transports/postgres.js'; import { stopCacheCleanup } from './utils/cache.js'; -import { HealthMonitor, MEMORY_DEGRADED_THRESHOLD, EVENT_LOOP_LAG_THRESHOLD_MS, measureEventLoopLag } from './utils/health.js'; +import { + EVENT_LOOP_LAG_THRESHOLD_MS, + HealthMonitor, + MEMORY_DEGRADED_THRESHOLD, + measureEventLoopLag, +} from './utils/health.js'; import { loadCommandsFromDirectory } from './utils/loadCommands.js'; import { getPermissionError, hasPermission } from './utils/permissions.js'; import { registerCommands } from './utils/registerCommands.js'; import { recordRestart, updateUptimeOnShutdown } from './utils/restartTracker.js'; -import { fireEventAllGuilds } from './modules/webhookNotifier.js'; import { safeFollowUp, safeReply } from './utils/safeSend.js'; // ES module dirname equivalent diff --git a/src/modules/config.js b/src/modules/config.js index 928440cd9..0b43ff2d2 100644 --- a/src/modules/config.js +++ b/src/modules/config.js @@ -360,7 +360,6 @@ export function getAllGuildIds() { return [...configCache.keys()].filter((id) => id !== 'global'); } - export function onConfigChange(pathOrPrefix, callback) { listeners.push({ path: pathOrPrefix, callback }); } diff --git a/src/modules/triage-respond.js b/src/modules/triage-respond.js index a1594cce8..27bad20f5 100644 --- a/src/modules/triage-respond.js +++ b/src/modules/triage-respond.js @@ -10,10 +10,10 @@ import { fetchChannelCached } from '../utils/discordCache.js'; import { safeSend } from '../utils/safeSend.js'; import { splitMessage } from '../utils/splitMessage.js'; import { addToHistory } from './ai.js'; -import { fireEvent } from './webhookNotifier.js'; import { FEEDBACK_EMOJI, registerAiMessage } from './aiFeedback.js'; import { isProtectedTarget } from './moderation.js'; import { resolveMessageId, sanitizeText } from './triage-filter.js'; +import { fireEvent } from './webhookNotifier.js'; /** Maximum characters to keep from fetched context messages. */ const CONTEXT_MESSAGE_CHAR_LIMIT = 500; diff --git a/src/modules/webhookNotifier.js b/src/modules/webhookNotifier.js index a490149cb..01846d5b8 100644 --- a/src/modules/webhookNotifier.js +++ b/src/modules/webhookNotifier.js @@ -17,7 +17,7 @@ import { createHmac } from 'node:crypto'; import { getPool } from '../db.js'; -import { error as logError, info, warn } from '../logger.js'; +import { info, error as logError, warn } from '../logger.js'; import { getConfig } from './config.js'; /** @type {string[]} All supported event types */ diff --git a/src/utils/health.js b/src/utils/health.js index 80cc9a9b3..185eefd04 100644 --- a/src/utils/health.js +++ b/src/utils/health.js @@ -181,4 +181,3 @@ export function measureEventLoopLag() { setImmediate(() => resolve(Date.now() - start)); }); } - diff --git a/tests/api/routes/notifications.test.js b/tests/api/routes/notifications.test.js index 827cb30b4..b2c3bd4b3 100644 --- a/tests/api/routes/notifications.test.js +++ b/tests/api/routes/notifications.test.js @@ -119,8 +119,7 @@ describe('notifications routes', () => { }); it('should return 401 without auth', async () => { - const res = await request(app) - .get(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`); + const res = await request(app).get(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`); expect(res.status).toBe(401); }); @@ -216,7 +215,11 @@ describe('notifications routes', () => { }); it('should return 400 when exceeding 20 endpoints', async () => { - const existing = Array.from({ length: 20 }, (_, i) => ({ id: `ep${i}`, url: 'https://x.com', events: ['bot.error'] })); + const existing = Array.from({ length: 20 }, (_, i) => ({ + id: `ep${i}`, + url: 'https://x.com', + events: ['bot.error'], + })); getConfig.mockReturnValue({ notifications: { webhooks: existing } }); const res = await request(app) @@ -261,11 +264,7 @@ describe('notifications routes', () => { .set(authHeaders()); expect(res.status).toBe(204); - expect(setConfigValue).toHaveBeenCalledWith( - 'notifications.webhooks', - [], - GUILD_ID, - ); + expect(setConfigValue).toHaveBeenCalledWith('notifications.webhooks', [], GUILD_ID); }); it('should return 404 when endpoint not found', async () => { @@ -278,8 +277,9 @@ describe('notifications routes', () => { }); it('should return 401 without auth', async () => { - const res = await request(app) - .delete(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks/uuid-1`); + const res = await request(app).delete( + `/api/v1/guilds/${GUILD_ID}/notifications/webhooks/uuid-1`, + ); expect(res.status).toBe(401); }); @@ -310,8 +310,9 @@ describe('notifications routes', () => { }); it('should return 401 without auth', async () => { - const res = await request(app) - .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks/uuid-1/test`); + const res = await request(app).post( + `/api/v1/guilds/${GUILD_ID}/notifications/webhooks/uuid-1/test`, + ); expect(res.status).toBe(401); }); @@ -322,7 +323,14 @@ describe('notifications routes', () => { describe('GET /guilds/:guildId/notifications/deliveries', () => { it('should return delivery log', async () => { const rows = [ - { id: 1, endpoint_id: 'ep1', event_type: 'bot.error', status: 'success', attempt: 1, delivered_at: '2026-01-01' }, + { + id: 1, + endpoint_id: 'ep1', + event_type: 'bot.error', + status: 'success', + attempt: 1, + delivered_at: '2026-01-01', + }, ]; getDeliveryLog.mockResolvedValueOnce(rows); @@ -346,8 +354,7 @@ describe('notifications routes', () => { }); it('should return 401 without auth', async () => { - const res = await request(app) - .get(`/api/v1/guilds/${GUILD_ID}/notifications/deliveries`); + const res = await request(app).get(`/api/v1/guilds/${GUILD_ID}/notifications/deliveries`); expect(res.status).toBe(401); }); diff --git a/tests/modules/webhookNotifier.test.js b/tests/modules/webhookNotifier.test.js index a65ff76be..6f35af0f1 100644 --- a/tests/modules/webhookNotifier.test.js +++ b/tests/modules/webhookNotifier.test.js @@ -16,7 +16,7 @@ vi.mock('../../src/modules/config.js', () => ({ })); import { getPool } from '../../src/db.js'; -import { error as logError, info, warn } from '../../src/logger.js'; +import { info, error as logError, warn } from '../../src/logger.js'; import { getConfig } from '../../src/modules/config.js'; import { deliverToEndpoint, @@ -98,7 +98,12 @@ describe('webhookNotifier', () => { url: 'https://example.com/hook', secret: 'mysecret', }; - const payload = { event: 'test', timestamp: '2026-01-01T00:00:00Z', guild_id: 'guild1', data: {} }; + const payload = { + event: 'test', + timestamp: '2026-01-01T00:00:00Z', + guild_id: 'guild1', + data: {}, + }; it('should deliver successfully on first attempt', async () => { mockFetch.mockResolvedValueOnce({ @@ -110,11 +115,14 @@ describe('webhookNotifier', () => { const result = await deliverToEndpoint('guild1', endpoint, payload); expect(result).toBe(true); expect(mockFetch).toHaveBeenCalledTimes(1); - expect(info).toHaveBeenCalledWith('Webhook delivered', expect.objectContaining({ - guildId: 'guild1', - endpointId: 'ep1', - attempt: 1, - })); + expect(info).toHaveBeenCalledWith( + 'Webhook delivered', + expect.objectContaining({ + guildId: 'guild1', + endpointId: 'ep1', + attempt: 1, + }), + ); }); it('should include HMAC signature header', async () => { @@ -182,7 +190,10 @@ describe('webhookNotifier', () => { vi.useRealTimers(); expect(result).toBe(false); - expect(logError).toHaveBeenCalledWith('Webhook delivery failed after all retries', expect.any(Object)); + expect(logError).toHaveBeenCalledWith( + 'Webhook delivery failed after all retries', + expect.any(Object), + ); }); it('should handle network errors (fetch throws)', async () => { @@ -294,7 +305,12 @@ describe('webhookNotifier', () => { getConfig.mockReturnValue({ notifications: { webhooks: [ - { id: 'ep1', url: 'https://example.com/hook', events: ['moderation.action'], enabled: true }, + { + id: 'ep1', + url: 'https://example.com/hook', + events: ['moderation.action'], + enabled: true, + }, ], }, }); @@ -341,7 +357,12 @@ describe('webhookNotifier', () => { getConfig.mockReturnValue({ notifications: { webhooks: [ - { id: 'ep1', url: 'https://example.com/hook', events: ['moderation.action'], enabled: true }, + { + id: 'ep1', + url: 'https://example.com/hook', + events: ['moderation.action'], + enabled: true, + }, ], }, }); @@ -359,7 +380,9 @@ describe('webhookNotifier', () => { }); it('should handle config errors gracefully (returns without firing)', async () => { - getConfig.mockImplementation(() => { throw new Error('config not loaded'); }); + getConfig.mockImplementation(() => { + throw new Error('config not loaded'); + }); await expect(fireEvent('bot.error', 'guild1', {})).resolves.not.toThrow(); expect(mockFetch).not.toHaveBeenCalled(); }); @@ -370,16 +393,23 @@ describe('webhookNotifier', () => { describe('getDeliveryLog', () => { it('should query the delivery log for a guild', async () => { const rows = [ - { id: 1, endpoint_id: 'ep1', event_type: 'bot.error', status: 'success', attempt: 1, delivered_at: '2026-01-01' }, + { + id: 1, + endpoint_id: 'ep1', + event_type: 'bot.error', + status: 'success', + attempt: 1, + delivered_at: '2026-01-01', + }, ]; mockPool.query.mockResolvedValueOnce({ rows }); const result = await getDeliveryLog('guild1'); expect(result).toEqual(rows); - expect(mockPool.query).toHaveBeenCalledWith( - expect.stringContaining('SELECT'), - ['guild1', 50], - ); + expect(mockPool.query).toHaveBeenCalledWith(expect.stringContaining('SELECT'), [ + 'guild1', + 50, + ]); }); it('should return empty array when no pool', async () => { @@ -391,10 +421,7 @@ describe('webhookNotifier', () => { it('should cap limit at 100', async () => { mockPool.query.mockResolvedValueOnce({ rows: [] }); await getDeliveryLog('guild1', 9999); - expect(mockPool.query).toHaveBeenCalledWith( - expect.any(String), - ['guild1', 100], - ); + expect(mockPool.query).toHaveBeenCalledWith(expect.any(String), ['guild1', 100]); }); it('should default limit to 50', async () => { From dc5f9909ba87be708809c630bb3d19d2eb73b38e Mon Sep 17 00:00:00 2001 From: Pip Build Date: Mon, 2 Mar 2026 04:24:54 -0500 Subject: [PATCH 06/21] fix: correct useGuildSelection usage in AiFeedbackStats component The hook returns string | null (guild ID), not an object. Also fix percent possibly-undefined in Pie label. --- web/src/components/dashboard/ai-feedback-stats.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/web/src/components/dashboard/ai-feedback-stats.tsx b/web/src/components/dashboard/ai-feedback-stats.tsx index 3a1aabd5c..5a15c5b23 100644 --- a/web/src/components/dashboard/ai-feedback-stats.tsx +++ b/web/src/components/dashboard/ai-feedback-stats.tsx @@ -37,19 +37,19 @@ const PIE_COLORS = ['#22C55E', '#EF4444']; * Shows 👍/👎 aggregate counts, approval ratio, and daily trend. */ export function AiFeedbackStats() { - const { selectedGuild, apiBase } = useGuildSelection(); + const guildId = useGuildSelection(); const [stats, setStats] = useState(null); const [loading, setLoading] = useState(false); const [error, setError] = useState(null); const fetchStats = useCallback(async () => { - if (!selectedGuild || !apiBase) return; + if (!guildId) return; setLoading(true); setError(null); try { - const res = await fetch(`${apiBase}/guilds/${selectedGuild.id}/ai-feedback/stats?days=30`, { + const res = await fetch(`/api/guilds/${guildId}/ai-feedback/stats?days=30`, { credentials: 'include', }); @@ -64,13 +64,13 @@ export function AiFeedbackStats() { } finally { setLoading(false); } - }, [selectedGuild, apiBase]); + }, [guildId]); useEffect(() => { void fetchStats(); }, [fetchStats]); - if (!selectedGuild) return null; + if (!guildId) return null; const pieData = stats && stats.total > 0 @@ -141,7 +141,7 @@ export function AiFeedbackStats() { innerRadius={50} outerRadius={75} dataKey="value" - label={({ name, percent }) => `${name} ${(percent * 100).toFixed(0)}%`} + label={({ name, percent }) => `${name} ${((percent ?? 0) * 100).toFixed(0)}%`} labelLine={false} > {pieData.map((_, index) => ( From 44cd6ba741a86e41c9f6e49f271227d82c42cbfc Mon Sep 17 00:00:00 2001 From: Pip Build Date: Mon, 2 Mar 2026 04:25:52 -0500 Subject: [PATCH 07/21] feat: add Next.js proxy routes for ai-feedback stats and recent endpoints --- .../[guildId]/ai-feedback/recent/route.ts | 49 +++++++++++++++++++ .../[guildId]/ai-feedback/stats/route.ts | 49 +++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 web/src/app/api/guilds/[guildId]/ai-feedback/recent/route.ts create mode 100644 web/src/app/api/guilds/[guildId]/ai-feedback/stats/route.ts diff --git a/web/src/app/api/guilds/[guildId]/ai-feedback/recent/route.ts b/web/src/app/api/guilds/[guildId]/ai-feedback/recent/route.ts new file mode 100644 index 000000000..2b4ed67fe --- /dev/null +++ b/web/src/app/api/guilds/[guildId]/ai-feedback/recent/route.ts @@ -0,0 +1,49 @@ +import type { NextRequest } from 'next/server'; +import { NextResponse } from 'next/server'; +import { + authorizeGuildAdmin, + buildUpstreamUrl, + getBotApiConfig, + proxyToBotApi, +} from '@/lib/bot-api-proxy'; + +export const dynamic = 'force-dynamic'; + +export async function GET( + request: NextRequest, + { params }: { params: Promise<{ guildId: string }> }, +) { + const { guildId } = await params; + if (!guildId) { + return NextResponse.json({ error: 'Missing guildId' }, { status: 400 }); + } + + const authError = await authorizeGuildAdmin( + request, + guildId, + '[api/guilds/:guildId/ai-feedback/recent]', + ); + if (authError) return authError; + + const config = getBotApiConfig('[api/guilds/:guildId/ai-feedback/recent]'); + if (config instanceof NextResponse) return config; + + const upstreamUrl = buildUpstreamUrl( + config.baseUrl, + `/guilds/${encodeURIComponent(guildId)}/ai-feedback/recent`, + '[api/guilds/:guildId/ai-feedback/recent]', + ); + if (upstreamUrl instanceof NextResponse) return upstreamUrl; + + const limit = request.nextUrl.searchParams.get('limit'); + if (limit !== null) { + upstreamUrl.searchParams.set('limit', limit); + } + + return proxyToBotApi( + upstreamUrl, + config.secret, + '[api/guilds/:guildId/ai-feedback/recent]', + 'Failed to fetch recent AI feedback', + ); +} diff --git a/web/src/app/api/guilds/[guildId]/ai-feedback/stats/route.ts b/web/src/app/api/guilds/[guildId]/ai-feedback/stats/route.ts new file mode 100644 index 000000000..fbe95b485 --- /dev/null +++ b/web/src/app/api/guilds/[guildId]/ai-feedback/stats/route.ts @@ -0,0 +1,49 @@ +import type { NextRequest } from 'next/server'; +import { NextResponse } from 'next/server'; +import { + authorizeGuildAdmin, + buildUpstreamUrl, + getBotApiConfig, + proxyToBotApi, +} from '@/lib/bot-api-proxy'; + +export const dynamic = 'force-dynamic'; + +export async function GET( + request: NextRequest, + { params }: { params: Promise<{ guildId: string }> }, +) { + const { guildId } = await params; + if (!guildId) { + return NextResponse.json({ error: 'Missing guildId' }, { status: 400 }); + } + + const authError = await authorizeGuildAdmin( + request, + guildId, + '[api/guilds/:guildId/ai-feedback/stats]', + ); + if (authError) return authError; + + const config = getBotApiConfig('[api/guilds/:guildId/ai-feedback/stats]'); + if (config instanceof NextResponse) return config; + + const upstreamUrl = buildUpstreamUrl( + config.baseUrl, + `/guilds/${encodeURIComponent(guildId)}/ai-feedback/stats`, + '[api/guilds/:guildId/ai-feedback/stats]', + ); + if (upstreamUrl instanceof NextResponse) return upstreamUrl; + + const days = request.nextUrl.searchParams.get('days'); + if (days !== null) { + upstreamUrl.searchParams.set('days', days); + } + + return proxyToBotApi( + upstreamUrl, + config.secret, + '[api/guilds/:guildId/ai-feedback/stats]', + 'Failed to fetch AI feedback stats', + ); +} From 4561f7ce234c47a4819abecdffff89efecfccf59 Mon Sep 17 00:00:00 2001 From: Pip Build Date: Mon, 2 Mar 2026 04:27:57 -0500 Subject: [PATCH 08/21] chore: fix remaining lint errors (biome format + unsafe fixes) --- src/config-listeners.js | 2 +- src/modules/webhookNotifier.js | 3 +-- tests/api/routes/notifications.test.js | 1 - web/src/components/dashboard/ai-feedback-stats.tsx | 4 +++- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/config-listeners.js b/src/config-listeners.js index 2596c7038..05cdcc3eb 100644 --- a/src/config-listeners.js +++ b/src/config-listeners.js @@ -112,7 +112,7 @@ export function registerConfigListeners({ dbPool, config }) { }); // ── Webhook notifications for config changes ───────────────────────── - onConfigChange('*', async (newValue, _oldValue, path, guildId) => { + onConfigChange('*', async (_newValue, _oldValue, path, guildId) => { // Skip internal/logging changes and notification webhook updates (avoid recursion) if (path.startsWith('logging.') || path.startsWith('notifications.')) return; const targetGuildId = guildId && guildId !== 'global' ? guildId : null; diff --git a/src/modules/webhookNotifier.js b/src/modules/webhookNotifier.js index 01846d5b8..d9aad7066 100644 --- a/src/modules/webhookNotifier.js +++ b/src/modules/webhookNotifier.js @@ -217,8 +217,7 @@ export async function fireEvent(eventType, guildId, data = {}) { // Filter to endpoints that subscribe to this event const targets = endpoints.filter( (ep) => - ep && - ep.url && + ep?.url && ep.enabled !== false && (Array.isArray(ep.events) ? ep.events.includes(eventType) : true), ); diff --git a/tests/api/routes/notifications.test.js b/tests/api/routes/notifications.test.js index b2c3bd4b3..6d5e65925 100644 --- a/tests/api/routes/notifications.test.js +++ b/tests/api/routes/notifications.test.js @@ -1,4 +1,3 @@ -import jwt from 'jsonwebtoken'; import request from 'supertest'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; diff --git a/web/src/components/dashboard/ai-feedback-stats.tsx b/web/src/components/dashboard/ai-feedback-stats.tsx index 5a15c5b23..998567fb6 100644 --- a/web/src/components/dashboard/ai-feedback-stats.tsx +++ b/web/src/components/dashboard/ai-feedback-stats.tsx @@ -141,7 +141,9 @@ export function AiFeedbackStats() { innerRadius={50} outerRadius={75} dataKey="value" - label={({ name, percent }) => `${name} ${((percent ?? 0) * 100).toFixed(0)}%`} + label={({ name, percent }) => + `${name} ${((percent ?? 0) * 100).toFixed(0)}%` + } labelLine={false} > {pieData.map((_, index) => ( From ca04d36848c73e0312eaf0e6c08752fa743d717c Mon Sep 17 00:00:00 2001 From: Pip Build Date: Mon, 2 Mar 2026 04:33:05 -0500 Subject: [PATCH 09/21] ci: trigger CI From f04130268d70932532717ed29798d71d2cf1b060 Mon Sep 17 00:00:00 2001 From: Pip Build Date: Mon, 2 Mar 2026 04:43:11 -0500 Subject: [PATCH 10/21] fix: ai-feedback route/module - pool injection, camelCase mapping, error handling --- src/api/routes/ai-feedback.js | 41 ++++++++++++++++++++++++++++++----- src/modules/aiFeedback.js | 30 ++++++++++++------------- 2 files changed, 51 insertions(+), 20 deletions(-) diff --git a/src/api/routes/ai-feedback.js b/src/api/routes/ai-feedback.js index 268a16c29..7079a97d3 100644 --- a/src/api/routes/ai-feedback.js +++ b/src/api/routes/ai-feedback.js @@ -92,6 +92,11 @@ router.get( async (req, res, next) => { try { const guildId = req.params.id; + const pool = req.app.locals.dbPool; + + if (!pool) { + return res.status(503).json({ error: 'Database unavailable' }); + } let days = 30; if (req.query.days !== undefined) { @@ -101,10 +106,15 @@ router.get( } } - const [stats, trend] = await Promise.all([ - getFeedbackStats(guildId), - getFeedbackTrend(guildId, days), - ]); + let stats, trend; + try { + [stats, trend] = await Promise.all([ + getFeedbackStats(guildId, pool), + getFeedbackTrend(guildId, days, pool), + ]); + } catch (_err) { + return res.status(500).json({ error: 'Failed to fetch AI feedback stats' }); + } res.json({ ...stats, @@ -188,6 +198,11 @@ router.get( async (req, res, next) => { try { const guildId = req.params.id; + const pool = req.app.locals.dbPool; + + if (!pool) { + return res.status(503).json({ error: 'Database unavailable' }); + } let limit = 25; if (req.query.limit !== undefined) { @@ -197,7 +212,23 @@ router.get( } } - const feedback = await getRecentFeedback(guildId, limit); + let rawFeedback; + try { + rawFeedback = await getRecentFeedback(guildId, limit, pool); + } catch (_err) { + return res.status(500).json({ error: 'Failed to fetch recent AI feedback' }); + } + + // Normalize DB row keys to camelCase (handles both raw SQL and aliased results) + const feedback = rawFeedback.map((row) => ({ + id: row.id, + messageId: row.messageId ?? row.message_id, + channelId: row.channelId ?? row.channel_id, + userId: row.userId ?? row.user_id, + feedbackType: row.feedbackType ?? row.feedback_type, + createdAt: row.createdAt ?? row.created_at, + })); + res.json({ feedback }); } catch (err) { next(err); diff --git a/src/modules/aiFeedback.js b/src/modules/aiFeedback.js index 1d031edb8..fb8efd3c2 100644 --- a/src/modules/aiFeedback.js +++ b/src/modules/aiFeedback.js @@ -159,12 +159,12 @@ export async function deleteFeedback({ messageId, userId }) { * @param {string} guildId * @returns {Promise<{positive: number, negative: number, total: number, ratio: number|null}>} */ -export async function getFeedbackStats(guildId) { - const pool = getPool(); - if (!pool) return { positive: 0, negative: 0, total: 0, ratio: null }; +export async function getFeedbackStats(guildId, pool) { + const resolvedPool = pool ?? getPool(); + if (!resolvedPool) return { positive: 0, negative: 0, total: 0, ratio: null }; try { - const result = await pool.query( + const result = await resolvedPool.query( `SELECT COUNT(*) FILTER (WHERE feedback_type = 'positive')::int AS positive, COUNT(*) FILTER (WHERE feedback_type = 'negative')::int AS negative, @@ -183,7 +183,7 @@ export async function getFeedbackStats(guildId) { return { positive, negative, total, ratio }; } catch (err) { logError('Failed to fetch AI feedback stats', { guildId, error: err.message }); - return { positive: 0, negative: 0, total: 0, ratio: null }; + throw err; } } @@ -193,12 +193,12 @@ export async function getFeedbackStats(guildId) { * @param {number} days - Number of days to look back (default 30) * @returns {Promise>} */ -export async function getFeedbackTrend(guildId, days = 30) { - const pool = getPool(); - if (!pool) return []; +export async function getFeedbackTrend(guildId, days = 30, pool) { + const resolvedPool = pool ?? getPool(); + if (!resolvedPool) return []; try { - const result = await pool.query( + const result = await resolvedPool.query( `SELECT DATE(created_at) AS date, COUNT(*) FILTER (WHERE feedback_type = 'positive')::int AS positive, @@ -218,7 +218,7 @@ export async function getFeedbackTrend(guildId, days = 30) { })); } catch (err) { logError('Failed to fetch AI feedback trend', { guildId, days, error: err.message }); - return []; + throw err; } } @@ -228,12 +228,12 @@ export async function getFeedbackTrend(guildId, days = 30) { * @param {number} limit - Max entries to return (default 50) * @returns {Promise>} */ -export async function getRecentFeedback(guildId, limit = 50) { - const pool = getPool(); - if (!pool) return []; +export async function getRecentFeedback(guildId, limit = 50, pool) { + const resolvedPool = pool ?? getPool(); + if (!resolvedPool) return []; try { - const result = await pool.query( + const result = await resolvedPool.query( `SELECT id, message_id AS "messageId", @@ -251,6 +251,6 @@ export async function getRecentFeedback(guildId, limit = 50) { return result.rows; } catch (err) { logError('Failed to fetch recent AI feedback', { guildId, limit, error: err.message }); - return []; + throw err; } } From 14bcf2c3f16aed0dcd46df9ac8882b16c7b74121 Mon Sep 17 00:00:00 2001 From: Pip Build Date: Mon, 2 Mar 2026 04:44:25 -0500 Subject: [PATCH 11/21] fix(webhookNotifier): use nullish coalescing to preserve status=0 for network errors result.status || null evaluated to null when status was 0 (network errors/ timeouts), losing the distinction between 'no HTTP status' and 'network-level failure'. Switch to result.status ?? null so 0 is stored correctly in the DB. Fixes review thread PRRT_kwDORICdSM5xgDsx --- src/modules/webhookNotifier.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/webhookNotifier.js b/src/modules/webhookNotifier.js index d9aad7066..8e68a3d6e 100644 --- a/src/modules/webhookNotifier.js +++ b/src/modules/webhookNotifier.js @@ -131,7 +131,7 @@ export async function deliverToEndpoint(guildId, endpoint, payload) { payload.event, payload, result.ok ? 'success' : 'failed', - result.status || null, + result.status ?? null, result.text?.slice(0, 2000) || null, attempt, ], From 4051e0b76c6213d0d429bbff870dcb2bd9cf41b8 Mon Sep 17 00:00:00 2001 From: Pip Build Date: Mon, 2 Mar 2026 04:44:25 -0500 Subject: [PATCH 12/21] fix(config): support '*' wildcard in onConfigChange for global change listeners emitConfigChangeEvents only matched exact paths and prefix.* patterns. The '*' wildcard registered in config-listeners.js for webhook config.changed events never fired, silently breaking that entire notification path. Add isWildcard = listener.path === '*' check so a bare '*' subscription receives every config change event. Fixes review thread PRRT_kwDORICdSM5xgDs8 --- src/modules/config.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/config.js b/src/modules/config.js index 0b43ff2d2..a1be1b387 100644 --- a/src/modules/config.js +++ b/src/modules/config.js @@ -396,7 +396,8 @@ async function emitConfigChangeEvents(fullPath, newValue, oldValue, guildId) { !isExact && listener.path.endsWith('.*') && fullPath.startsWith(listener.path.replace(/\.\*$/, '.')); - if (isExact || isPrefix) { + const isWildcard = listener.path === '*'; + if (isExact || isPrefix || isWildcard) { try { const result = listener.callback(newValue, oldValue, fullPath, guildId); if (result && typeof result.then === 'function') { From 1fe44d753368e10b08bb428f9c098a0ac4fd3b5c Mon Sep 17 00:00:00 2001 From: Pip Build Date: Mon, 2 Mar 2026 04:50:26 -0500 Subject: [PATCH 13/21] fix: redis test - use class mock for ioredis constructor --- tests/redis.test.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/redis.test.js b/tests/redis.test.js index 53fbd96ad..bc94f912a 100644 --- a/tests/redis.test.js +++ b/tests/redis.test.js @@ -48,7 +48,11 @@ describe('redis.js', () => { quit: vi.fn().mockResolvedValue('OK'), }; vi.doMock('ioredis', () => ({ - default: vi.fn().mockImplementation(() => mockClient), + default: class MockRedis { + constructor() { + Object.assign(this, mockClient); + } + }, })); const freshRedis = await import('../src/redis.js'); From 51fd8c4e2e0203fdc368332569a76d64894bcc9a Mon Sep 17 00:00:00 2001 From: Bill Date: Mon, 2 Mar 2026 05:09:50 -0500 Subject: [PATCH 14/21] fix(security): SSRF vulnerability in webhook URL validation The previous URL validation regex `/^https?:\/\/.+/` was too permissive and allowed requests to internal/private network addresses, including: - localhost - 127.0.0.1 and other loopback addresses - 169.254.169.254 (AWS metadata endpoint) - Private IP ranges (10.x, 172.16-31.x, 192.168.x) Changes: - Add src/api/utils/ssrfProtection.js with comprehensive URL validation - Block private/internal IP ranges, link-local addresses, and localhost variants - Add DNS resolution check to prevent DNS rebinding attacks (async version) - Update notifications.js route to use SSRF-safe validation - Add extensive tests for blocked and allowed URLs Fixes security vulnerability in webhook notification feature (PR #219). --- src/api/routes/notifications.js | 7 +- src/api/utils/ssrfProtection.js | 300 +++++++++++++++++++++++++ tests/api/routes/notifications.test.js | 46 ++++ tests/api/utils/ssrfProtection.test.js | 268 ++++++++++++++++++++++ 4 files changed, 619 insertions(+), 2 deletions(-) create mode 100644 src/api/utils/ssrfProtection.js create mode 100644 tests/api/utils/ssrfProtection.test.js diff --git a/src/api/routes/notifications.js b/src/api/routes/notifications.js index 68765c767..d52312ac4 100644 --- a/src/api/routes/notifications.js +++ b/src/api/routes/notifications.js @@ -11,6 +11,7 @@ import { Router } from 'express'; import { info, error as logError } from '../../logger.js'; import { getConfig, setConfigValue } from '../../modules/config.js'; import { getDeliveryLog, testEndpoint, WEBHOOK_EVENTS } from '../../modules/webhookNotifier.js'; +import { validateUrlForSsrfSync } from '../utils/ssrfProtection.js'; const router = Router(); @@ -120,8 +121,10 @@ router.post('/:guildId/notifications/webhooks', async (req, res) => { return res.status(400).json({ error: 'Missing or invalid "url"' }); } - if (!/^https:\/\/.+/.test(url)) { - return res.status(400).json({ error: '"url" must be a valid HTTPS URL' }); + // SSRF-safe URL validation - require HTTPS to prevent DNS rebinding attacks + const urlValidation = validateUrlForSsrfSync(url); + if (!urlValidation.valid) { + return res.status(400).json({ error: urlValidation.error }); } if (!Array.isArray(events) || events.length === 0) { diff --git a/src/api/utils/ssrfProtection.js b/src/api/utils/ssrfProtection.js new file mode 100644 index 000000000..974d6180b --- /dev/null +++ b/src/api/utils/ssrfProtection.js @@ -0,0 +1,300 @@ +/** + * SSRF Protection Utilities + * + * Validates URLs to prevent Server-Side Request Forgery attacks by blocking + * requests to internal/private network addresses. + */ + +/** + * Check if a hostname resolves to a blocked IP address. + * This handles DNS rebinding attacks by checking the resolved IP. + * + * @param {string} hostname - The hostname to check + * @returns {Promise} The blocked IP if found, null if safe + */ +async function resolveAndCheckIp(hostname) { + // Only perform DNS resolution in Node.js runtime + if (typeof process === 'undefined') return null; + + const dns = await import('node:dns').catch(() => null); + if (!dns) return null; + + return new Promise((resolve) => { + dns.lookup(hostname, { all: true }, (err, addresses) => { + if (err || !addresses) { + resolve(null); + return; + } + + for (const addr of addresses) { + if (isBlockedIp(addr.address)) { + resolve(addr.address); + return; + } + } + resolve(null); + }); + }); +} + +/** + * Check if an IP address is in a blocked range. + * Blocks: + * - Loopback (127.0.0.0/8) + * - Link-local (169.254.0.0/16) - includes AWS metadata at 169.254.169.254 + * - Private ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16) + * - Localhost IPv6 (::1) + * - IPv6 link-local (fe80::/10) + * + * @param {string} ip - The IP address to check + * @returns {boolean} True if the IP is blocked + */ +export function isBlockedIp(ip) { + // Normalize IPv6 addresses + const normalizedIp = ip.toLowerCase().trim(); + + // IPv6 loopback + if (normalizedIp === '::1' || normalizedIp === '0:0:0:0:0:0:0:1') { + return true; + } + + // IPv6 link-local (fe80::/10) + if (normalizedIp.startsWith('fe80:')) { + return true; + } + + // IPv4-mapped IPv6 addresses (::ffff:192.168.1.1) + const ipv4MappedMatch = normalizedIp.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/i); + if (ipv4MappedMatch) { + return isBlockedIp(ipv4MappedMatch[1]); + } + + // IPv4 checks + const parts = normalizedIp.split('.'); + if (parts.length !== 4) { + // Not a valid IPv4, let it pass (will fail elsewhere) + return false; + } + + const octets = parts.map((p) => { + const num = parseInt(p, 10); + return Number.isNaN(num) ? -1 : num; + }); + + // Invalid octets + if (octets.some((o) => o < 0 || o > 255)) { + return false; + } + + const [first, second] = octets; + + // Loopback: 127.0.0.0/8 + if (first === 127) { + return true; + } + + // Link-local: 169.254.0.0/16 (includes AWS metadata endpoint) + if (first === 169 && second === 254) { + return true; + } + + // Private: 10.0.0.0/8 + if (first === 10) { + return true; + } + + // Private: 172.16.0.0/12 (172.16.0.0 - 172.31.255.255) + if (first === 172 && second >= 16 && second <= 31) { + return true; + } + + // Private: 192.168.0.0/16 + if (first === 192 && second === 168) { + return true; + } + + // 0.0.0.0/8 - "this network" + if (first === 0) { + return true; + } + + return false; +} + +/** + * Check if a hostname is a blocked literal (like "localhost") + * + * @param {string} hostname - The hostname to check + * @returns {boolean} True if the hostname is blocked + */ +function isBlockedHostname(hostname) { + const normalized = hostname.toLowerCase().trim(); + + // Block localhost variants + const blockedHostnames = [ + 'localhost', + 'localhost.localdomain', + 'ip6-localhost', + 'ip6-loopback', + 'ip6-localnet', + 'ip6-mcastprefix', + ]; + + if (blockedHostnames.includes(normalized)) { + return true; + } + + // Block hostnames that end with .local, .localhost, .internal, .localdomain + const blockedSuffixes = ['.local', '.localhost', '.internal', '.localdomain', '.home', '.home.arpa']; + if (blockedSuffixes.some((suffix) => normalized.endsWith(suffix))) { + return true; + } + + // Block if the hostname is a raw IP address that's blocked + // IPv4 check + if (/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(normalized)) { + return isBlockedIp(normalized); + } + + // IPv6 check (basic patterns) + if (normalized.includes(':') && (normalized.startsWith('::1') || normalized.startsWith('fe80:'))) { + return true; + } + + return false; +} + +/** + * Validation result for SSRF-safe URL check + * + * @typedef {Object} UrlValidationResult + * @property {boolean} valid - Whether the URL is safe to use + * @property {string} [error] - Error message if invalid + * @property {string} [blockedIp] - The blocked IP address if found during DNS resolution + */ + +/** + * Validate a URL for SSRF safety. + * Checks both the hostname literal and performs DNS resolution to prevent + * DNS rebinding attacks. + * + * @param {string} urlString - The URL to validate + * @param {Object} [options] - Validation options + * @param {boolean} [options.allowHttp=false] - Allow HTTP (not just HTTPS) + * @param {boolean} [options.checkDns=true] - Perform DNS resolution check + * @returns {Promise} Validation result + */ +export async function validateUrlForSsrf(urlString, options = {}) { + const { allowHttp = false, checkDns = true } = options; + + // Basic URL parsing + let url; + try { + url = new URL(urlString); + } catch { + return { valid: false, error: 'Invalid URL format' }; + } + + // Protocol check + const allowedProtocols = allowHttp ? ['https:', 'http:'] : ['https:']; + if (!allowedProtocols.includes(url.protocol)) { + return { + valid: false, + error: allowHttp + ? 'URL must use HTTP or HTTPS protocol' + : 'URL must use HTTPS protocol', + }; + } + + const hostname = url.hostname; + + // Check for blocked hostnames (localhost, etc.) + if (isBlockedHostname(hostname)) { + return { + valid: false, + error: 'URL hostname is not allowed (private/internal addresses are blocked)', + }; + } + + // Check if hostname is already an IP and if it's blocked + if (/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(hostname)) { + if (isBlockedIp(hostname)) { + return { + valid: false, + error: 'URL resolves to a blocked IP address (private/internal ranges are not allowed)', + }; + } + } else if (checkDns) { + // Perform DNS resolution to prevent DNS rebinding + const blockedIp = await resolveAndCheckIp(hostname); + if (blockedIp) { + return { + valid: false, + error: `URL hostname resolves to blocked IP address ${blockedIp} (private/internal ranges are not allowed)`, + blockedIp, + }; + } + } + + return { valid: true }; +} + +/** + * Synchronous version of SSRF validation for cases where DNS resolution + * is not possible or desired. Use the async version when possible. + * + * @param {string} urlString - The URL to validate + * @param {Object} [options] - Validation options + * @param {boolean} [options.allowHttp=false] - Allow HTTP (not just HTTPS) + * @returns {UrlValidationResult} Validation result + */ +export function validateUrlForSsrfSync(urlString, options = {}) { + const { allowHttp = false } = options; + + // Basic URL parsing + let url; + try { + url = new URL(urlString); + } catch { + return { valid: false, error: 'Invalid URL format' }; + } + + // Protocol check + const allowedProtocols = allowHttp ? ['https:', 'http:'] : ['https:']; + if (!allowedProtocols.includes(url.protocol)) { + return { + valid: false, + error: allowHttp + ? 'URL must use HTTP or HTTPS protocol' + : 'URL must use HTTPS protocol', + }; + } + + const hostname = url.hostname; + + // Check for blocked hostnames + if (isBlockedHostname(hostname)) { + return { + valid: false, + error: 'URL hostname is not allowed (private/internal addresses are blocked)', + }; + } + + // Check if hostname is a raw IP and if it's blocked + if (/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(hostname)) { + if (isBlockedIp(hostname)) { + return { + valid: false, + error: 'URL points to a blocked IP address (private/internal ranges are not allowed)', + }; + } + } + + return { valid: true }; +} + +export default { + validateUrlForSsrf, + validateUrlForSsrfSync, + isBlockedIp, +}; diff --git a/tests/api/routes/notifications.test.js b/tests/api/routes/notifications.test.js index 6d5e65925..05cff29fe 100644 --- a/tests/api/routes/notifications.test.js +++ b/tests/api/routes/notifications.test.js @@ -252,6 +252,52 @@ describe('notifications routes', () => { expect(res.status).toBe(401); }); + + // ── SSRF Protection ───────────────────────────────────────────────────── + + const blockedUrls = [ + { url: 'https://localhost/webhook', desc: 'localhost' }, + { url: 'https://localhost:8080/webhook', desc: 'localhost with port' }, + { url: 'https://127.0.0.1/webhook', desc: '127.0.0.1 loopback' }, + { url: 'https://127.0.0.1:3000/webhook', desc: '127.0.0.1 with port' }, + { url: 'https://169.254.169.254/latest/meta-data/', desc: 'AWS metadata endpoint' }, + { url: 'https://10.0.0.1/webhook', desc: '10.x private range' }, + { url: 'https://10.255.255.255/webhook', desc: '10.x upper bound' }, + { url: 'https://172.16.0.1/webhook', desc: '172.16.x private range' }, + { url: 'https://172.31.255.255/webhook', desc: '172.31.x private range upper' }, + { url: 'https://192.168.0.1/webhook', desc: '192.168.x private range' }, + { url: 'https://192.168.255.255/webhook', desc: '192.168.x upper bound' }, + { url: 'https://0.0.0.0/webhook', desc: '0.0.0.0 this-network' }, + { url: 'https://myserver.local/webhook', desc: 'local domain' }, + { url: 'https://api.internal/webhook', desc: 'internal domain' }, + { url: 'https://app.localhost/webhook', desc: 'localhost domain' }, + ]; + + it.each(blockedUrls)('should reject $desc', async ({ url }) => { + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()) + .send({ url, events: ['bot.error'] }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/not allowed|blocked|private|internal/i); + }); + + const allowedUrls = [ + { url: 'https://example.com/webhook', desc: 'public domain' }, + { url: 'https://api.example.com/v1/webhook', desc: 'public domain with path' }, + { url: 'https://example.com:8443/webhook', desc: 'public domain with port' }, + { url: 'https://example.com/webhook?token=abc', desc: 'public domain with query' }, + ]; + + it.each(allowedUrls)('should accept $desc', async ({ url }) => { + const res = await request(app) + .post(`/api/v1/guilds/${GUILD_ID}/notifications/webhooks`) + .set(authHeaders()) + .send({ url, events: ['bot.error'] }); + + expect(res.status).toBe(201); + }); }); // ── DELETE /guilds/:guildId/notifications/webhooks/:endpointId ──────────── diff --git a/tests/api/utils/ssrfProtection.test.js b/tests/api/utils/ssrfProtection.test.js new file mode 100644 index 000000000..5edacfd95 --- /dev/null +++ b/tests/api/utils/ssrfProtection.test.js @@ -0,0 +1,268 @@ +import { describe, expect, it, vi } from 'vitest'; +import { isBlockedIp, validateUrlForSsrfSync, validateUrlForSsrf } from '../../../src/api/utils/ssrfProtection.js'; + +describe('isBlockedIp', () => { + describe('loopback addresses', () => { + it('should block 127.0.0.1', () => { + expect(isBlockedIp('127.0.0.1')).toBe(true); + }); + + it('should block 127.0.0.0', () => { + expect(isBlockedIp('127.0.0.0')).toBe(true); + }); + + it('should block 127.255.255.255', () => { + expect(isBlockedIp('127.255.255.255')).toBe(true); + }); + + it('should block 127.123.45.67', () => { + expect(isBlockedIp('127.123.45.67')).toBe(true); + }); + }); + + describe('link-local addresses (AWS metadata)', () => { + it('should block 169.254.169.254 (AWS metadata)', () => { + expect(isBlockedIp('169.254.169.254')).toBe(true); + }); + + it('should block 169.254.0.1', () => { + expect(isBlockedIp('169.254.0.1')).toBe(true); + }); + + it('should block 169.254.255.255', () => { + expect(isBlockedIp('169.254.255.255')).toBe(true); + }); + }); + + describe('private ranges', () => { + it('should block 10.0.0.1 (10.0.0.0/8)', () => { + expect(isBlockedIp('10.0.0.1')).toBe(true); + }); + + it('should block 10.255.255.255', () => { + expect(isBlockedIp('10.255.255.255')).toBe(true); + }); + + it('should block 172.16.0.1 (172.16.0.0/12)', () => { + expect(isBlockedIp('172.16.0.1')).toBe(true); + }); + + it('should block 172.31.255.255', () => { + expect(isBlockedIp('172.31.255.255')).toBe(true); + }); + + it('should NOT block 172.15.255.255 (below range)', () => { + expect(isBlockedIp('172.15.255.255')).toBe(false); + }); + + it('should NOT block 172.32.0.1 (above range)', () => { + expect(isBlockedIp('172.32.0.1')).toBe(false); + }); + + it('should block 192.168.0.1 (192.168.0.0/16)', () => { + expect(isBlockedIp('192.168.0.1')).toBe(true); + }); + + it('should block 192.168.255.255', () => { + expect(isBlockedIp('192.168.255.255')).toBe(true); + }); + }); + + describe('this-network', () => { + it('should block 0.0.0.0', () => { + expect(isBlockedIp('0.0.0.0')).toBe(true); + }); + + it('should block 0.0.0.1', () => { + expect(isBlockedIp('0.0.0.1')).toBe(true); + }); + }); + + describe('IPv6', () => { + it('should block ::1 (IPv6 loopback)', () => { + expect(isBlockedIp('::1')).toBe(true); + }); + + it('should block fe80::1 (IPv6 link-local)', () => { + expect(isBlockedIp('fe80::1')).toBe(true); + }); + + it('should block IPv4-mapped IPv6 private addresses', () => { + expect(isBlockedIp('::ffff:192.168.1.1')).toBe(true); + expect(isBlockedIp('::ffff:127.0.0.1')).toBe(true); + expect(isBlockedIp('::ffff:10.0.0.1')).toBe(true); + }); + }); + + describe('valid public IPs', () => { + it('should NOT block 8.8.8.8 (Google DNS)', () => { + expect(isBlockedIp('8.8.8.8')).toBe(false); + }); + + it('should NOT block 1.1.1.1 (Cloudflare DNS)', () => { + expect(isBlockedIp('1.1.1.1')).toBe(false); + }); + + it('should NOT block 172.15.0.1 (not in private range)', () => { + expect(isBlockedIp('172.15.0.1')).toBe(false); + }); + }); +}); + +describe('validateUrlForSsrfSync', () => { + describe('invalid URL format', () => { + it('should reject malformed URLs', () => { + const result = validateUrlForSsrfSync('not-a-url'); + expect(result.valid).toBe(false); + expect(result.error).toContain('Invalid URL format'); + }); + + it('should reject empty string', () => { + const result = validateUrlForSsrfSync(''); + expect(result.valid).toBe(false); + }); + }); + + describe('protocol validation', () => { + it('should accept HTTPS URLs by default', () => { + const result = validateUrlForSsrfSync('https://example.com/webhook'); + expect(result.valid).toBe(true); + }); + + it('should reject HTTP URLs by default', () => { + const result = validateUrlForSsrfSync('http://example.com/webhook'); + expect(result.valid).toBe(false); + expect(result.error).toContain('HTTPS'); + }); + + it('should accept HTTP URLs when allowHttp is true', () => { + const result = validateUrlForSsrfSync('http://example.com/webhook', { allowHttp: true }); + expect(result.valid).toBe(true); + }); + + it('should reject ftp:// protocol', () => { + const result = validateUrlForSsrfSync('ftp://example.com/file'); + expect(result.valid).toBe(false); + }); + }); + + describe('blocked hostnames', () => { + it('should reject localhost', () => { + const result = validateUrlForSsrfSync('https://localhost/webhook'); + expect(result.valid).toBe(false); + expect(result.error).toContain('not allowed'); + }); + + it('should reject localhost with port', () => { + const result = validateUrlForSsrfSync('https://localhost:8080/webhook'); + expect(result.valid).toBe(false); + }); + + it('should reject .local domains', () => { + const result = validateUrlForSsrfSync('https://myserver.local/webhook'); + expect(result.valid).toBe(false); + }); + + it('should reject .internal domains', () => { + const result = validateUrlForSsrfSync('https://api.internal/webhook'); + expect(result.valid).toBe(false); + }); + + it('should reject .localhost domains', () => { + const result = validateUrlForSsrfSync('https://app.localhost/webhook'); + expect(result.valid).toBe(false); + }); + }); + + describe('blocked IP addresses', () => { + it('should reject 127.0.0.1', () => { + const result = validateUrlForSsrfSync('https://127.0.0.1/webhook'); + expect(result.valid).toBe(false); + expect(result.error).toMatch(/blocked|private|internal/i); + }); + + it('should reject 127.0.0.1 with port', () => { + const result = validateUrlForSsrfSync('https://127.0.0.1:3000/webhook'); + expect(result.valid).toBe(false); + }); + + it('should reject AWS metadata endpoint', () => { + const result = validateUrlForSsrfSync('https://169.254.169.254/latest/meta-data/'); + expect(result.valid).toBe(false); + }); + + it('should reject 10.x.x.x addresses', () => { + const result = validateUrlForSsrfSync('https://10.0.0.1/webhook'); + expect(result.valid).toBe(false); + }); + + it('should reject 172.16.x.x addresses', () => { + const result = validateUrlForSsrfSync('https://172.16.0.1/webhook'); + expect(result.valid).toBe(false); + }); + + it('should reject 192.168.x.x addresses', () => { + const result = validateUrlForSsrfSync('https://192.168.1.1/webhook'); + expect(result.valid).toBe(false); + }); + + it('should reject 0.0.0.0', () => { + const result = validateUrlForSsrfSync('https://0.0.0.0/webhook'); + expect(result.valid).toBe(false); + }); + }); + + describe('valid public URLs', () => { + it('should accept https://example.com/webhook', () => { + const result = validateUrlForSsrfSync('https://example.com/webhook'); + expect(result.valid).toBe(true); + }); + + it('should accept URLs with paths', () => { + const result = validateUrlForSsrfSync('https://api.example.com/v1/webhooks/endpoint'); + expect(result.valid).toBe(true); + }); + + it('should accept URLs with query strings', () => { + const result = validateUrlForSsrfSync('https://example.com/webhook?token=abc123'); + expect(result.valid).toBe(true); + }); + + it('should accept URLs with ports', () => { + const result = validateUrlForSsrfSync('https://example.com:8443/webhook'); + expect(result.valid).toBe(true); + }); + }); +}); + +describe('validateUrlForSsrf (async)', () => { + describe('basic validation', () => { + it('should accept valid HTTPS URLs', async () => { + const result = await validateUrlForSsrf('https://example.com/webhook'); + expect(result.valid).toBe(true); + }); + + it('should reject invalid URLs', async () => { + const result = await validateUrlForSsrf('not-a-url'); + expect(result.valid).toBe(false); + }); + + it('should reject localhost', async () => { + const result = await validateUrlForSsrf('https://localhost/webhook'); + expect(result.valid).toBe(false); + }); + + it('should reject blocked IPs', async () => { + const result = await validateUrlForSsrf('https://127.0.0.1/webhook'); + expect(result.valid).toBe(false); + }); + }); + + describe('DNS resolution check', () => { + it('should skip DNS check when checkDns is false', async () => { + // This would normally do a DNS lookup + const result = await validateUrlForSsrf('https://example.com/webhook', { checkDns: false }); + expect(result.valid).toBe(true); + }); + }); +}); From a715c9b5b28ad5d17103bc5fe794f9dae25c9add Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Mon, 2 Mar 2026 06:11:58 -0500 Subject: [PATCH 15/21] fix(webhook): add idempotency key for delivery retries --- src/modules/webhookNotifier.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/modules/webhookNotifier.js b/src/modules/webhookNotifier.js index 8e68a3d6e..0d5aba333 100644 --- a/src/modules/webhookNotifier.js +++ b/src/modules/webhookNotifier.js @@ -15,7 +15,7 @@ * member.flagged - AI flagged a member's message */ -import { createHmac } from 'node:crypto'; +import { createHmac, randomUUID } from 'node:crypto'; import { getPool } from '../db.js'; import { info, error as logError, warn } from '../logger.js'; import { getConfig } from './config.js'; @@ -57,12 +57,14 @@ export function signPayload(secret, body) { * @param {string} url - Endpoint URL * @param {string} secret - HMAC secret (empty string = no signature header) * @param {string} body - Serialised JSON payload + * @param {string} deliveryId - Unique identifier for this delivery (used for idempotency) * @returns {Promise<{ok: boolean, status: number, text: string}>} */ -async function attemptDelivery(url, secret, body) { +async function attemptDelivery(url, secret, body, deliveryId) { const headers = { 'Content-Type': 'application/json', 'User-Agent': 'VolvoxBot-Webhooks/1.0', + 'X-Delivery-Id': deliveryId, }; if (secret) { @@ -113,10 +115,11 @@ function sleep(ms) { */ export async function deliverToEndpoint(guildId, endpoint, payload) { const body = JSON.stringify(payload); + const deliveryId = randomUUID(); const pool = getPool(); for (let attempt = 1; attempt <= RETRY_DELAYS_MS.length + 1; attempt++) { - const result = await attemptDelivery(endpoint.url, endpoint.secret || '', body); + const result = await attemptDelivery(endpoint.url, endpoint.secret || '', body, deliveryId); // Log this attempt if (pool) { @@ -275,7 +278,8 @@ export async function testEndpoint(guildId, endpoint) { data: { message: 'This is a test webhook from VolvoxBot.' }, }; const body = JSON.stringify(payload); - return attemptDelivery(endpoint.url, endpoint.secret || '', body); + const deliveryId = randomUUID(); + return attemptDelivery(endpoint.url, endpoint.secret || '', body, deliveryId); } /** From 618898470dad5aafed25efb9762f79f3d686f349 Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Mon, 2 Mar 2026 06:21:06 -0500 Subject: [PATCH 16/21] fix: remove swallowed errors in ai-feedback.js catch blocks - Removed inner catch blocks that swallowed errors with catch (_err) - Now errors bubble up to outer catch block and use next(err) - Follows API error handling conventions --- src/api/routes/ai-feedback.js | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/api/routes/ai-feedback.js b/src/api/routes/ai-feedback.js index 7079a97d3..6e16bce98 100644 --- a/src/api/routes/ai-feedback.js +++ b/src/api/routes/ai-feedback.js @@ -106,15 +106,11 @@ router.get( } } - let stats, trend; - try { - [stats, trend] = await Promise.all([ - getFeedbackStats(guildId, pool), - getFeedbackTrend(guildId, days, pool), - ]); - } catch (_err) { - return res.status(500).json({ error: 'Failed to fetch AI feedback stats' }); - } + // Let errors bubble up to the outer catch block + const [stats, trend] = await Promise.all([ + getFeedbackStats(guildId, pool), + getFeedbackTrend(guildId, days, pool), + ]); res.json({ ...stats, @@ -212,12 +208,8 @@ router.get( } } - let rawFeedback; - try { - rawFeedback = await getRecentFeedback(guildId, limit, pool); - } catch (_err) { - return res.status(500).json({ error: 'Failed to fetch recent AI feedback' }); - } + // Let errors bubble up to the outer catch block + const rawFeedback = await getRecentFeedback(guildId, limit, pool); // Normalize DB row keys to camelCase (handles both raw SQL and aliased results) const feedback = rawFeedback.map((row) => ({ From 496e6e7b82cd22f689dab59e58116c56a3833b98 Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Mon, 2 Mar 2026 06:21:13 -0500 Subject: [PATCH 17/21] fix: align notifications.js error handling with API conventions - Use next(err) instead of inline 500 responses in all routes - Add redactUrl() helper to avoid leaking secrets in logs - Validate secret and enabled types before persisting - Clamp limit to positive range (1-100) to prevent DB errors --- src/api/routes/notifications.js | 87 ++++++++++++++++++++++++--------- 1 file changed, 63 insertions(+), 24 deletions(-) diff --git a/src/api/routes/notifications.js b/src/api/routes/notifications.js index d52312ac4..0f6e4057c 100644 --- a/src/api/routes/notifications.js +++ b/src/api/routes/notifications.js @@ -8,13 +8,34 @@ import { randomUUID } from 'node:crypto'; import { Router } from 'express'; -import { info, error as logError } from '../../logger.js'; +import { info } from '../../logger.js'; import { getConfig, setConfigValue } from '../../modules/config.js'; import { getDeliveryLog, testEndpoint, WEBHOOK_EVENTS } from '../../modules/webhookNotifier.js'; import { validateUrlForSsrfSync } from '../utils/ssrfProtection.js'; const router = Router(); +/** + * Redact a URL for safe logging by replacing any query string or credentials. + * @param {string} url - URL to redact + * @returns {string} URL with query string replaced by [REDACTED] + */ +function redactUrl(url) { + try { + const parsed = new URL(url); + // Redact query string (may contain secrets) + parsed.search = '?[REDACTED]'; + // Redact password in userinfo if present + if (parsed.password) { + parsed.password = '[REDACTED]'; + } + return parsed.toString(); + } catch { + // If URL parsing fails, return a safe placeholder + return '[INVALID URL]'; + } +} + /** * Mask the secret field from an endpoint object for safe GET responses. * @@ -50,7 +71,7 @@ function maskEndpoint(ep) { * "401": * $ref: "#/components/responses/Unauthorized" */ -router.get('/:guildId/notifications/webhooks', async (req, res) => { +router.get('/:guildId/notifications/webhooks', async (req, res, next) => { const { guildId } = req.params; try { @@ -60,8 +81,7 @@ router.get('/:guildId/notifications/webhooks', async (req, res) => { : []; return res.json(webhooks); } catch (err) { - logError('Failed to list webhook endpoints', { guildId, error: err.message }); - return res.status(500).json({ error: 'Failed to retrieve webhook endpoints' }); + next(err); } }); @@ -113,7 +133,7 @@ router.get('/:guildId/notifications/webhooks', async (req, res) => { * "401": * $ref: "#/components/responses/Unauthorized" */ -router.post('/:guildId/notifications/webhooks', async (req, res) => { +router.post('/:guildId/notifications/webhooks', async (req, res, next) => { const { guildId } = req.params; const { url, events, secret, enabled = true } = req.body || {}; @@ -121,10 +141,15 @@ router.post('/:guildId/notifications/webhooks', async (req, res) => { return res.status(400).json({ error: 'Missing or invalid "url"' }); } - // SSRF-safe URL validation - require HTTPS to prevent DNS rebinding attacks - const urlValidation = validateUrlForSsrfSync(url); - if (!urlValidation.valid) { - return res.status(400).json({ error: urlValidation.error }); + if (!/^https:\/\/.+/.test(url)) { + return res.status(400).json({ error: '"url" must be a valid HTTPS URL' }); + } + + // Validate URL against SSRF + try { + validateUrlForSsrfSync(url); + } catch (err) { + return res.status(400).json({ error: err.message }); } if (!Array.isArray(events) || events.length === 0) { @@ -139,6 +164,16 @@ router.post('/:guildId/notifications/webhooks', async (req, res) => { }); } + // Validate secret type before persisting + if (secret !== undefined && typeof secret !== 'string') { + return res.status(400).json({ error: '"secret" must be a string' }); + } + + // Validate enabled type before persisting + if (enabled !== undefined && typeof enabled !== 'boolean') { + return res.status(400).json({ error: '"enabled" must be a boolean' }); + } + try { const cfg = getConfig(guildId); const existing = Array.isArray(cfg?.notifications?.webhooks) ? cfg.notifications.webhooks : []; @@ -151,18 +186,22 @@ router.post('/:guildId/notifications/webhooks', async (req, res) => { id: randomUUID(), url, events, - enabled: Boolean(enabled), - ...(secret ? { secret } : {}), + enabled: typeof enabled === 'boolean' ? enabled : true, + ...(secret && typeof secret === 'string' ? { secret } : {}), }; const updated = [...existing, newEndpoint]; await setConfigValue('notifications.webhooks', updated, guildId); - info('Webhook endpoint added', { guildId, endpointId: newEndpoint.id, url }); + // Use redacted URL for logging to avoid leaking secrets in query params + info('Webhook endpoint added', { + guildId, + endpointId: newEndpoint.id, + url: redactUrl(url), + }); return res.status(201).json(maskEndpoint(newEndpoint)); } catch (err) { - logError('Failed to add webhook endpoint', { guildId, error: err.message }); - return res.status(500).json({ error: 'Failed to add webhook endpoint' }); + next(err); } }); @@ -195,7 +234,7 @@ router.post('/:guildId/notifications/webhooks', async (req, res) => { * "401": * $ref: "#/components/responses/Unauthorized" */ -router.delete('/:guildId/notifications/webhooks/:endpointId', async (req, res) => { +router.delete('/:guildId/notifications/webhooks/:endpointId', async (req, res, next) => { const { guildId, endpointId } = req.params; try { @@ -211,8 +250,7 @@ router.delete('/:guildId/notifications/webhooks/:endpointId', async (req, res) = info('Webhook endpoint removed', { guildId, endpointId }); return res.status(204).end(); } catch (err) { - logError('Failed to remove webhook endpoint', { guildId, error: err.message }); - return res.status(500).json({ error: 'Failed to remove webhook endpoint' }); + next(err); } }); @@ -245,7 +283,7 @@ router.delete('/:guildId/notifications/webhooks/:endpointId', async (req, res) = * "401": * $ref: "#/components/responses/Unauthorized" */ -router.post('/:guildId/notifications/webhooks/:endpointId/test', async (req, res) => { +router.post('/:guildId/notifications/webhooks/:endpointId/test', async (req, res, next) => { const { guildId, endpointId } = req.params; try { @@ -264,8 +302,7 @@ router.post('/:guildId/notifications/webhooks/:endpointId/test', async (req, res body: result.text?.slice(0, 500), }); } catch (err) { - logError('Failed to test webhook endpoint', { guildId, error: err.message }); - return res.status(500).json({ error: 'Failed to test webhook endpoint' }); + next(err); } }); @@ -298,16 +335,18 @@ router.post('/:guildId/notifications/webhooks/:endpointId/test', async (req, res * "401": * $ref: "#/components/responses/Unauthorized" */ -router.get('/:guildId/notifications/deliveries', async (req, res) => { +router.get('/:guildId/notifications/deliveries', async (req, res, next) => { const { guildId } = req.params; - const limit = Math.min(parseInt(req.query.limit, 10) || 50, 100); + + // Clamp limit to positive range (1-100) to prevent DB errors from negative values + const rawLimit = parseInt(req.query.limit, 10) || 50; + const limit = Math.max(1, Math.min(rawLimit, 100)); try { const log = await getDeliveryLog(guildId, limit); return res.json(log); } catch (err) { - logError('Failed to fetch delivery log', { guildId, error: err.message }); - return res.status(500).json({ error: 'Failed to fetch delivery log' }); + next(err); } }); From 1e58a025733fa8cc9883c0336062c5d2dd137042 Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Mon, 2 Mar 2026 06:21:22 -0500 Subject: [PATCH 18/21] fix: add defensive pool access in webhookNotifier.js - Add safeGetPool() wrapper to handle getPool() exceptions - Use safeGetPool() in both deliverToEndpoint and getDeliveryLog - Prevents crashes when DB pool is unavailable --- src/modules/webhookNotifier.js | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/modules/webhookNotifier.js b/src/modules/webhookNotifier.js index 0d5aba333..771d7b5e8 100644 --- a/src/modules/webhookNotifier.js +++ b/src/modules/webhookNotifier.js @@ -40,6 +40,18 @@ const MAX_LOG_ENTRIES = 100; /** Fetch timeout per attempt (ms) */ const FETCH_TIMEOUT_MS = 10000; +/** + * Safely get the database pool, returning null if not available or on error. + * @returns {import('pg').Pool | null} + */ +function safeGetPool() { + try { + return getPool(); + } catch { + return null; + } +} + /** * Sign a payload with HMAC-SHA256 using the endpoint secret. * @@ -116,7 +128,8 @@ function sleep(ms) { export async function deliverToEndpoint(guildId, endpoint, payload) { const body = JSON.stringify(payload); const deliveryId = randomUUID(); - const pool = getPool(); + // Use defensive pool access to handle potential exceptions + const pool = safeGetPool(); for (let attempt = 1; attempt <= RETRY_DELAYS_MS.length + 1; attempt++) { const result = await attemptDelivery(endpoint.url, endpoint.secret || '', body, deliveryId); @@ -134,7 +147,7 @@ export async function deliverToEndpoint(guildId, endpoint, payload) { payload.event, payload, result.ok ? 'success' : 'failed', - result.status ?? null, + result.status || null, result.text?.slice(0, 2000) || null, attempt, ], @@ -248,7 +261,8 @@ export async function fireEvent(eventType, guildId, data = {}) { * @returns {Promise} Delivery log entries, newest first */ export async function getDeliveryLog(guildId, limit = 50) { - const pool = getPool(); + // Use defensive pool access + const pool = safeGetPool(); if (!pool) return []; const { rows } = await pool.query( From 82f6775e39819216ead1b4dead0fab0ea16ba20e Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Mon, 2 Mar 2026 06:21:28 -0500 Subject: [PATCH 19/21] fix: restore global.fetch in test instead of deleting - Save original fetch in beforeEach and restore in afterEach - Properly restores global.fetch to avoid polluting global state - Improves test isolation --- tests/modules/webhookNotifier.test.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/modules/webhookNotifier.test.js b/tests/modules/webhookNotifier.test.js index 6f35af0f1..873f949fd 100644 --- a/tests/modules/webhookNotifier.test.js +++ b/tests/modules/webhookNotifier.test.js @@ -30,10 +30,14 @@ import { describe('webhookNotifier', () => { let mockFetch; let mockPool; + let originalFetch; beforeEach(() => { vi.clearAllMocks(); + // Save original fetch before any mocks + originalFetch = global.fetch; + mockPool = { query: vi.fn().mockResolvedValue({ rows: [] }), }; @@ -49,7 +53,8 @@ describe('webhookNotifier', () => { afterEach(() => { vi.restoreAllMocks(); - delete global.fetch; + // Restore original fetch instead of deleting + global.fetch = originalFetch; }); // ── signPayload ────────────────────────────────────────────────────────── From 9b69aba6e4e8951367ab59db47b8a7bc32dea0d5 Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Mon, 2 Mar 2026 06:38:51 -0500 Subject: [PATCH 20/21] fix: properly check SSRF validation result --- src/api/routes/notifications.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/api/routes/notifications.js b/src/api/routes/notifications.js index 0f6e4057c..760b317e8 100644 --- a/src/api/routes/notifications.js +++ b/src/api/routes/notifications.js @@ -146,10 +146,9 @@ router.post('/:guildId/notifications/webhooks', async (req, res, next) => { } // Validate URL against SSRF - try { - validateUrlForSsrfSync(url); - } catch (err) { - return res.status(400).json({ error: err.message }); + const ssrfResult = validateUrlForSsrfSync(url); + if (!ssrfResult.valid) { + return res.status(400).json({ error: ssrfResult.error }); } if (!Array.isArray(events) || events.length === 0) { From 2b3939a7617e82ea57b8e40134851255c4e34f81 Mon Sep 17 00:00:00 2001 From: Bill Chirico Date: Mon, 2 Mar 2026 06:39:27 -0500 Subject: [PATCH 21/21] style: fix lint formatting --- src/api/utils/ssrfProtection.js | 22 ++++++++++++++-------- src/index.js | 2 +- tests/api/utils/ssrfProtection.test.js | 6 +++++- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/api/utils/ssrfProtection.js b/src/api/utils/ssrfProtection.js index 974d6180b..8e4d0949c 100644 --- a/src/api/utils/ssrfProtection.js +++ b/src/api/utils/ssrfProtection.js @@ -145,7 +145,14 @@ function isBlockedHostname(hostname) { } // Block hostnames that end with .local, .localhost, .internal, .localdomain - const blockedSuffixes = ['.local', '.localhost', '.internal', '.localdomain', '.home', '.home.arpa']; + const blockedSuffixes = [ + '.local', + '.localhost', + '.internal', + '.localdomain', + '.home', + '.home.arpa', + ]; if (blockedSuffixes.some((suffix) => normalized.endsWith(suffix))) { return true; } @@ -157,7 +164,10 @@ function isBlockedHostname(hostname) { } // IPv6 check (basic patterns) - if (normalized.includes(':') && (normalized.startsWith('::1') || normalized.startsWith('fe80:'))) { + if ( + normalized.includes(':') && + (normalized.startsWith('::1') || normalized.startsWith('fe80:')) + ) { return true; } @@ -200,9 +210,7 @@ export async function validateUrlForSsrf(urlString, options = {}) { if (!allowedProtocols.includes(url.protocol)) { return { valid: false, - error: allowHttp - ? 'URL must use HTTP or HTTPS protocol' - : 'URL must use HTTPS protocol', + error: allowHttp ? 'URL must use HTTP or HTTPS protocol' : 'URL must use HTTPS protocol', }; } @@ -264,9 +272,7 @@ export function validateUrlForSsrfSync(urlString, options = {}) { if (!allowedProtocols.includes(url.protocol)) { return { valid: false, - error: allowHttp - ? 'URL must use HTTP or HTTPS protocol' - : 'URL must use HTTPS protocol', + error: allowHttp ? 'URL must use HTTP or HTTPS protocol' : 'URL must use HTTPS protocol', }; } diff --git a/src/index.js b/src/index.js index ba964b238..3d04bd9b0 100644 --- a/src/index.js +++ b/src/index.js @@ -51,8 +51,8 @@ import { startTempbanScheduler, stopTempbanScheduler } from './modules/moderatio import { loadOptOuts } from './modules/optout.js'; import { startScheduler, stopScheduler } from './modules/scheduler.js'; import { startTriage, stopTriage } from './modules/triage.js'; -import { fireEventAllGuilds } from './modules/webhookNotifier.js'; import { startVoiceFlush, stopVoiceFlush } from './modules/voice.js'; +import { fireEventAllGuilds } from './modules/webhookNotifier.js'; import { closeRedisClient as closeRedis, initRedis } from './redis.js'; import { pruneOldLogs } from './transports/postgres.js'; import { stopCacheCleanup } from './utils/cache.js'; diff --git a/tests/api/utils/ssrfProtection.test.js b/tests/api/utils/ssrfProtection.test.js index 5edacfd95..2f024ec52 100644 --- a/tests/api/utils/ssrfProtection.test.js +++ b/tests/api/utils/ssrfProtection.test.js @@ -1,5 +1,9 @@ import { describe, expect, it, vi } from 'vitest'; -import { isBlockedIp, validateUrlForSsrfSync, validateUrlForSsrf } from '../../../src/api/utils/ssrfProtection.js'; +import { + isBlockedIp, + validateUrlForSsrf, + validateUrlForSsrfSync, +} from '../../../src/api/utils/ssrfProtection.js'; describe('isBlockedIp', () => { describe('loopback addresses', () => {