diff --git a/server/src/nest/notifications/mailer/mailer.service.ts b/server/src/nest/notifications/mailer/mailer.service.ts index 870bdf40f..f15937cb2 100644 --- a/server/src/nest/notifications/mailer/mailer.service.ts +++ b/server/src/nest/notifications/mailer/mailer.service.ts @@ -2,7 +2,7 @@ import { Injectable } from '@nestjs/common'; import nodemailer from 'nodemailer'; import { PASSWORD_RESET_I18N } from '@trek/shared/i18n/externalNotifications'; import { readEnv } from '../../../app-config'; -import { logError, logInfo, logDebug } from '../../audit/audit-log.logger'; +import { logError, logInfo, logDebug, logWarn } from '../../audit/audit-log.logger'; import { decrypt_api_key } from '../../common/crypto/apiKeyCrypto'; import { DatabaseService } from '../../database/database.service'; import { buildEmailHtml, buildPasswordResetHtml } from './email-html'; @@ -28,6 +28,8 @@ interface SmtpConfig { */ @Injectable() export class MailerService { + private skippedTlsWarned = false; + constructor(private readonly db: DatabaseService) {} private getAppSetting(key: string): string | null { @@ -61,15 +63,35 @@ export class MailerService { */ private createTransport(config: SmtpConfig) { const skipTls = readEnv().smtp.skipTlsVerify || this.getAppSetting('smtp_skip_tls_verify') === 'true'; + if (skipTls) this.warnOnceAboutSkippedTls(config); return nodemailer.createTransport({ host: config.host, port: config.port, secure: config.secure, auth: config.user ? { user: config.user, pass: config.pass } : undefined, + // The operator's opt-out for a relay with a self-signed certificate, off by + // default and reachable only through SMTP_SKIP_TLS_VERIFY or the matching + // admin setting. It stays because self-hosted installs behind an internal + // relay depend on it; warnOnceAboutSkippedTls is what keeps it from being a + // silent downgrade. ...(skipTls ? { tls: { rejectUnauthorized: false } } : {}), }); } + /** + * Once per process, not once per mail — a transport is built on every send, and + * a line on every notification would train the operator to scroll past it. + */ + private warnOnceAboutSkippedTls(config: SmtpConfig): void { + if (this.skippedTlsWarned) return; + this.skippedTlsWarned = true; + logWarn( + `SECURITY: SMTP certificate verification is disabled for ${config.host}:${config.port}. ` + + 'Mail — including password-reset links and the SMTP credentials — travels over a channel an ' + + 'active attacker can impersonate. Keep this on only for a relay you control on a trusted network.', + ); + } + /** Is SMTP configured at the instance level? (Independent of any one user's address.) */ isSmtpConfigured(): boolean { return !!(readEnv().smtp.host || this.getAppSetting('smtp_host')); diff --git a/server/tests/unit/nest/mailer.service.test.ts b/server/tests/unit/nest/mailer.service.test.ts new file mode 100644 index 000000000..e50a489cf --- /dev/null +++ b/server/tests/unit/nest/mailer.service.test.ts @@ -0,0 +1,146 @@ +/** + * mailer.service.test.ts + * + * Covers the SMTP transport options MailerService hands to nodemailer, and in + * particular the skip-TLS opt-out: it must stay reachable for operators behind an + * internal relay, and it must announce itself instead of downgrading quietly. + * Constructed directly (no TestingModule, repo convention). + */ + +const { testDb, dbMock } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + canAccessTrip: () => undefined, + isOwner: () => false, + }; + return { testDb: db, dbMock: mock }; +}); + +const { sendMail, createTransport } = vi.hoisted(() => { + const send = vi.fn().mockResolvedValue({ messageId: 'test' }); + return { + sendMail: send, + createTransport: vi.fn((_options: Record) => ({ sendMail: send })), + }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('nodemailer', () => ({ default: { createTransport } })); +vi.mock('../../../src/nest/audit/audit-log.logger', () => ({ + logInfo: vi.fn(), + logDebug: vi.fn(), + logError: vi.fn(), + logWarn: vi.fn(), +})); + +import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { logWarn } from '../../../src/nest/audit/audit-log.logger'; + +function setAppSetting(key: string, value: string): void { + testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run(key, value); +} + +/** The minimum that makes getSmtpConfig() return a config instead of null. */ +function configureSmtp(): void { + setAppSetting('smtp_host', 'mail.internal.example'); + setAppSetting('smtp_port', '587'); + setAppSetting('smtp_from', 'trek@example.com'); +} + +function newMailer(): MailerService { + return new MailerService(new DatabaseService(testDb)); +} + +/** The options object of the most recent nodemailer.createTransport() call. */ +function lastTransportOptions(): Record { + const calls = createTransport.mock.calls; + return calls[calls.length - 1][0] as Record; +} + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + resetTestDb(testDb); + vi.clearAllMocks(); +}); + +afterAll(() => { + testDb.close(); +}); + +describe('MailerService TLS options', () => { + it('MAILER-001: leaves certificate verification on by default', async () => { + configureSmtp(); + + expect(await newMailer().sendEmail('someone@example.com', 'Subject', 'Body')).toBe(true); + + expect(lastTransportOptions()).not.toHaveProperty('tls'); + expect(logWarn).not.toHaveBeenCalled(); + }); + + it('MAILER-002: the smtp_skip_tls_verify setting turns verification off', async () => { + configureSmtp(); + setAppSetting('smtp_skip_tls_verify', 'true'); + + await newMailer().sendEmail('someone@example.com', 'Subject', 'Body'); + + expect(lastTransportOptions().tls).toEqual({ rejectUnauthorized: false }); + }); + + it('MAILER-003: anything other than "true" leaves verification on', async () => { + configureSmtp(); + setAppSetting('smtp_skip_tls_verify', 'false'); + + await newMailer().sendEmail('someone@example.com', 'Subject', 'Body'); + + expect(lastTransportOptions()).not.toHaveProperty('tls'); + expect(logWarn).not.toHaveBeenCalled(); + }); + + it('MAILER-004: skipping verification is announced, and names the host it applies to', async () => { + configureSmtp(); + setAppSetting('smtp_skip_tls_verify', 'true'); + + await newMailer().sendEmail('someone@example.com', 'Subject', 'Body'); + + expect(logWarn).toHaveBeenCalledTimes(1); + const warning = vi.mocked(logWarn).mock.calls[0][0]; + expect(warning).toContain('mail.internal.example:587'); + expect(warning).toContain('SECURITY'); + }); + + it('MAILER-005: the warning is logged once per process, not once per mail', async () => { + configureSmtp(); + setAppSetting('smtp_skip_tls_verify', 'true'); + const mailer = newMailer(); + + await mailer.sendEmail('first@example.com', 'One', 'Body'); + await mailer.sendEmail('second@example.com', 'Two', 'Body'); + await mailer.sendPasswordResetEmail('third@example.com', 'https://trek.example/reset', null); + + expect(createTransport).toHaveBeenCalledTimes(3); + expect(logWarn).toHaveBeenCalledTimes(1); + }); + + it('MAILER-006: a send with no SMTP configured builds no transport at all', async () => { + expect(await newMailer().sendEmail('someone@example.com', 'Subject', 'Body')).toBe(false); + + expect(createTransport).not.toHaveBeenCalled(); + expect(sendMail).not.toHaveBeenCalled(); + }); +});