diff --git a/server/src/db/migrations.ts b/server/src/db/migrations.ts index 94f8fdf..b4919be 100644 --- a/server/src/db/migrations.ts +++ b/server/src/db/migrations.ts @@ -563,8 +563,11 @@ function runMigrations(db: Database.Database): void { CREATE INDEX IF NOT EXISTS idx_ncp_user ON notification_channel_preferences(user_id); `); - // Migrate data from old notification_preferences table - const oldPrefs = db.prepare('SELECT * FROM notification_preferences').all() as Array>; + // Migrate data from old notification_preferences table (may not exist on fresh installs) + const tableExists = (db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name='notification_preferences'").get() as { name: string } | undefined) != null; + const oldPrefs: Array> = tableExists + ? db.prepare('SELECT * FROM notification_preferences').all() as Array> + : []; const eventCols: Record = { trip_invite: 'notify_trip_invite', booking_change: 'notify_booking_change', @@ -602,6 +605,10 @@ function runMigrations(db: Database.Database): void { SELECT 'notification_channels', value FROM app_settings WHERE key = 'notification_channel'; `); }, + // Migration 70: Drop the old notification_preferences table (data migrated to notification_channel_preferences in migration 69) + () => { + db.exec('DROP TABLE IF EXISTS notification_preferences;'); + }, ]; if (currentVersion < migrations.length) { diff --git a/server/src/services/authService.ts b/server/src/services/authService.ts index 358ea13..75495c4 100644 --- a/server/src/services/authService.ts +++ b/server/src/services/authService.ts @@ -716,6 +716,7 @@ export function updateAppSettings( } if (key === 'smtp_pass' && val === '••••••••') continue; if (key === 'smtp_pass') val = encrypt_api_key(val); + if (key === 'admin_webhook_url' && val) val = maybe_encrypt_api_key(val) ?? val; db.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)").run(key, val); } } diff --git a/server/src/services/notificationPreferencesService.ts b/server/src/services/notificationPreferencesService.ts index e005e0d..f8380fd 100644 --- a/server/src/services/notificationPreferencesService.ts +++ b/server/src/services/notificationPreferencesService.ts @@ -173,6 +173,27 @@ function setAdminGlobalPref(event: NotifEventType, channel: 'email' | 'webhook', // ── Preferences update ───────────────────────────────────────────────────── +// ── Shared helper for per-user channel preference upserts ───────────────── + +function applyUserChannelPrefs( + userId: number, + prefs: Partial>>>, + upsert: ReturnType, + del: ReturnType +): void { + for (const [eventType, channels] of Object.entries(prefs)) { + if (!channels) continue; + for (const [channel, enabled] of Object.entries(channels)) { + if (enabled) { + // Remove explicit row — default is enabled + del.run(userId, eventType, channel); + } else { + upsert.run(userId, eventType, channel, 0); + } + } + } +} + /** * Bulk-update preferences from the matrix UI. * Inserts disabled rows (enabled=0) and removes rows that are enabled (default). @@ -187,20 +208,7 @@ export function setPreferences( const del = db.prepare( 'DELETE FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?' ); - - db.transaction(() => { - for (const [eventType, channels] of Object.entries(prefs)) { - if (!channels) continue; - for (const [channel, enabled] of Object.entries(channels)) { - if (enabled) { - // Remove explicit row — default is enabled - del.run(userId, eventType, channel); - } else { - upsert.run(userId, eventType, channel, 0); - } - } - } - })(); + db.transaction(() => applyUserChannelPrefs(userId, prefs, upsert, del))(); } /** @@ -219,24 +227,33 @@ export function setAdminPreferences( 'DELETE FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?' ); - db.transaction(() => { - for (const [eventType, channels] of Object.entries(prefs)) { - if (!channels) continue; - for (const [channel, enabled] of Object.entries(channels)) { - if (ADMIN_GLOBAL_CHANNELS.includes(channel as NotifChannel)) { - // Global setting — stored in app_settings - setAdminGlobalPref(eventType as NotifEventType, channel as 'email' | 'webhook', enabled); - } else { - // Per-user (inapp) - if (enabled) { - del.run(userId, eventType, channel); - } else { - upsert.run(userId, eventType, channel, 0); - } - } + // Split global (email/webhook) from per-user (inapp) prefs + const globalPrefs: Partial>>> = {}; + const userPrefs: Partial>>> = {}; + + for (const [eventType, channels] of Object.entries(prefs)) { + if (!channels) continue; + for (const [channel, enabled] of Object.entries(channels)) { + if (ADMIN_GLOBAL_CHANNELS.includes(channel as NotifChannel)) { + if (!globalPrefs[eventType]) globalPrefs[eventType] = {}; + globalPrefs[eventType]![channel] = enabled; + } else { + if (!userPrefs[eventType]) userPrefs[eventType] = {}; + userPrefs[eventType]![channel] = enabled; } } - })(); + } + + // Apply global prefs outside the transaction (they write to app_settings) + for (const [eventType, channels] of Object.entries(globalPrefs)) { + if (!channels) continue; + for (const [channel, enabled] of Object.entries(channels)) { + setAdminGlobalPref(eventType as NotifEventType, channel as 'email' | 'webhook', enabled); + } + } + + // Apply per-user (inapp) prefs in a transaction + db.transaction(() => applyUserChannelPrefs(userId, userPrefs, upsert, del))(); } // ── SMTP availability helper (for authService) ───────────────────────────── diff --git a/server/src/services/notificationService.ts b/server/src/services/notificationService.ts index 76603c4..6fdc1ee 100644 --- a/server/src/services/notificationService.ts +++ b/server/src/services/notificationService.ts @@ -1,5 +1,5 @@ import { db } from '../db/database'; -import { logDebug } from './auditLog'; +import { logDebug, logError } from './auditLog'; import { getActiveChannels, isEnabledForEvent, @@ -264,7 +264,12 @@ export async function send(payload: NotificationPayload): Promise { } } - await Promise.allSettled(promises); + const results = await Promise.allSettled(promises); + for (const result of results) { + if (result.status === 'rejected') { + logError(`notificationService.send channel dispatch failed event=${event} recipient=${recipientId}: ${result.reason}`); + } + } })); // ── Admin webhook (scope: admin) — global, respects global pref ────── @@ -272,7 +277,9 @@ export async function send(payload: NotificationPayload): Promise { const adminWebhookUrl = getAdminWebhookUrl(); if (adminWebhookUrl) { const { title, body } = getEventText('en', event, params); - sendWebhook(adminWebhookUrl, { event, title, body, link: fullLink }).catch(() => {}); + await sendWebhook(adminWebhookUrl, { event, title, body, link: fullLink }).catch((err: unknown) => { + logError(`notificationService.send admin webhook failed event=${event}: ${err instanceof Error ? err.message : err}`); + }); } } } diff --git a/server/src/services/notifications.ts b/server/src/services/notifications.ts index 9f3bc6a..c4f0a16 100644 --- a/server/src/services/notifications.ts +++ b/server/src/services/notifications.ts @@ -3,6 +3,7 @@ import fetch from 'node-fetch'; import { db } from '../db/database'; import { decrypt_api_key } from './apiKeyCrypto'; import { logInfo, logDebug, logError } from './auditLog'; +import { checkSsrf, createPinnedAgent } from '../utils/ssrfGuard'; // ── Types ────────────────────────────────────────────────────────────────── @@ -17,6 +18,17 @@ interface SmtpConfig { secure: boolean; } +// ── HTML escaping ────────────────────────────────────────────────────────── + +function escapeHtml(str: string): string { + return str + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +} + // ── Settings helpers ─────────────────────────────────────────────────────── function getAppSetting(key: string): string | null { @@ -54,11 +66,13 @@ export function getUserLanguage(userId: number): string { } export function getUserWebhookUrl(userId: number): string | null { - return (db.prepare("SELECT value FROM settings WHERE user_id = ? AND key = 'webhook_url'").get(userId) as { value: string } | undefined)?.value || null; + const value = (db.prepare("SELECT value FROM settings WHERE user_id = ? AND key = 'webhook_url'").get(userId) as { value: string } | undefined)?.value || null; + return value ? decrypt_api_key(value) : null; } export function getAdminWebhookUrl(): string | null { - return getAppSetting('admin_webhook_url') || null; + const value = getAppSetting('admin_webhook_url') || null; + return value ? decrypt_api_key(value) : null; } // ── Email i18n strings ───────────────────────────────────────────────────── @@ -226,7 +240,9 @@ export function getEventText(lang: string, event: NotifEventType, params: Record export function buildEmailHtml(subject: string, body: string, lang: string, navigateTarget?: string): string { const s = I18N[lang] || I18N.en; const appUrl = getAppUrl(); - const ctaHref = navigateTarget ? `${appUrl}${navigateTarget}` : (appUrl || ''); + const ctaHref = escapeHtml(navigateTarget ? `${appUrl}${navigateTarget}` : (appUrl || '')); + const safeSubject = escapeHtml(subject); + const safeBody = escapeHtml(body); return ` @@ -243,9 +259,9 @@ export function buildEmailHtml(subject: string, body: string, lang: string, navi -

${subject}

+

${safeSubject}

-

${body}

+

${safeBody}

${appUrl ? ` @@ -328,12 +344,20 @@ export function buildWebhookBody(url: string, payload: { event: string; title: s export async function sendWebhook(url: string, payload: { event: string; title: string; body: string; tripName?: string; link?: string }): Promise { if (!url) return false; + const ssrf = await checkSsrf(url); + if (!ssrf.allowed) { + logError(`Webhook blocked by SSRF guard event=${payload.event} url=${url} reason=${ssrf.error}`); + return false; + } + try { + const agent = createPinnedAgent(ssrf.resolvedIp!, new URL(url).protocol); const res = await fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: buildWebhookBody(url, payload), signal: AbortSignal.timeout(10000), + agent, }); if (!res.ok) { diff --git a/server/src/services/settingsService.ts b/server/src/services/settingsService.ts index 7aa49d8..2fb5527 100644 --- a/server/src/services/settingsService.ts +++ b/server/src/services/settingsService.ts @@ -1,4 +1,7 @@ import { db } from '../db/database'; +import { maybe_encrypt_api_key } from './apiKeyCrypto'; + +const ENCRYPTED_SETTING_KEYS = new Set(['webhook_url']); export function getUserSettings(userId: number): Record { const rows = db.prepare('SELECT key, value FROM settings WHERE user_id = ?').all(userId) as { key: string; value: string }[]; @@ -13,12 +16,17 @@ export function getUserSettings(userId: number): Record { return settings; } +function serializeValue(key: string, value: unknown): string { + const raw = typeof value === 'object' ? JSON.stringify(value) : String(value !== undefined ? value : ''); + if (ENCRYPTED_SETTING_KEYS.has(key)) return maybe_encrypt_api_key(raw) ?? raw; + return raw; +} + export function upsertSetting(userId: number, key: string, value: unknown) { - const serialized = typeof value === 'object' ? JSON.stringify(value) : String(value !== undefined ? value : ''); db.prepare(` INSERT INTO settings (user_id, key, value) VALUES (?, ?, ?) ON CONFLICT(user_id, key) DO UPDATE SET value = excluded.value - `).run(userId, key, serialized); + `).run(userId, key, serializeValue(key, value)); } export function bulkUpsertSettings(userId: number, settings: Record) { @@ -29,8 +37,7 @@ export function bulkUpsertSettings(userId: number, settings: Record ({ vi.mock('node-fetch', () => ({ default: fetchMock })); vi.mock('../../../src/websocket', () => ({ broadcastToUser: broadcastMock })); +vi.mock('../../../src/utils/ssrfGuard', () => ({ + checkSsrf: vi.fn(async () => ({ allowed: true, isPrivate: false, resolvedIp: '1.2.3.4' })), + createPinnedAgent: vi.fn(() => ({})), +})); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; diff --git a/server/tests/unit/services/notifications.test.ts b/server/tests/unit/services/notifications.test.ts index 7af5706..70a9dfd 100644 --- a/server/tests/unit/services/notifications.test.ts +++ b/server/tests/unit/services/notifications.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, afterEach } from 'vitest'; +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; vi.mock('../../../src/db/database', () => ({ db: { prepare: () => ({ get: vi.fn(() => undefined), all: vi.fn(() => []) }) }, @@ -18,7 +18,15 @@ vi.mock('../../../src/services/auditLog', () => ({ vi.mock('nodemailer', () => ({ default: { createTransport: vi.fn(() => ({ sendMail: vi.fn() })) } })); vi.mock('node-fetch', () => ({ default: vi.fn() })); -import { getEventText, buildEmailHtml, buildWebhookBody } from '../../../src/services/notifications'; +// ssrfGuard is mocked per-test in the SSRF describe block; default passes all +vi.mock('../../../src/utils/ssrfGuard', () => ({ + checkSsrf: vi.fn(async () => ({ allowed: true, isPrivate: false, resolvedIp: '1.2.3.4' })), + createPinnedAgent: vi.fn(() => ({})), +})); + +import { getEventText, buildEmailHtml, buildWebhookBody, sendWebhook } from '../../../src/services/notifications'; +import { checkSsrf } from '../../../src/utils/ssrfGuard'; +import { logError } from '../../../src/services/auditLog'; afterEach(() => { vi.unstubAllEnvs(); @@ -193,3 +201,119 @@ describe('buildEmailHtml', () => { expect(unknown).toContain('notifications enabled in TREK'); }); }); + +// ── SEC: XSS escaping in buildEmailHtml ────────────────────────────────────── + +describe('buildEmailHtml XSS prevention (SEC-016)', () => { + it('escapes HTML special characters in subject', () => { + const html = buildEmailHtml('', 'Body', 'en'); + expect(html).not.toContain('', + }); + const html = buildEmailHtml('Subject', body, 'en'); + expect(html).not.toContain(''); + expect(html).not.toContain('