test(notifications): harden and cover the email validator

docs/email-notifications.md

@@ -0,0 +1,26 @@
+# Email Notifications
+
+## Recipient email validation
+
+Before an email notification is dispatched, the recipient address is validated
+by `NotificationService.isValidEmail` (`src/services/notification.service.ts`).
+Because validated addresses flow into the SMTP/SES/SendGrid transports, the
+validator is intentionally strict to prevent header- and recipient-injection.
+
+A recipient address is **rejected** when it:
+
+- is empty;
+- contains any CR/LF or other control characters (`\x00`–`\x1f`, `\x7f`) —
+  classic SMTP header injection;
+- contains a comma or semicolon (multi-recipient smuggling, e.g.
+  `a@b.com,c@d.com`);
+- contains quoting or backslash forms (`"x"@y.com`, `a\b@c.com`);
+- contains angle brackets / display-name forms (`foo<bar>@example.com`);
+- does not have exactly one `@` separating a non-empty local part and a dotted
+  domain with a valid TLD (e.g. `user@example` is rejected for the missing TLD).
+
+A recipient address is **accepted** when it is a single, RFC-shaped address such
+as `user@example.com` or `first.last@sub.example.co.uk`.
+
+The validator keeps its boolean contract and is applied before any transport
+dispatch, so downstream transport work can rely on its guarantees.

src/services/notification.email-validation.test.ts

@@ -0,0 +1,64 @@
+/**
+ * Email validator hardening tests.
+ *
+ * `isValidEmail` is private, so it is exercised through the public `sendEmail`
+ * path. A stub transport that always succeeds lets us assert that valid
+ * addresses reach the transport (`success: true`) while invalid/injection forms
+ * are rejected before dispatch with `success: false` and the
+ * `Invalid email address` message.
+ */
+
+import { NotificationService } from './notification.service';
+import { NotificationTransport, NotificationResult } from './notification.transport';
+import { KeyEscrowEvent } from '../types/notification.types';
+
+describe('NotificationService email validation', () => {
+  let sendEmailSpy: jest.Mock;
+  let service: NotificationService;
+
+  beforeEach(() => {
+    sendEmailSpy = jest.fn(async (): Promise<NotificationResult> => ({ success: true }));
+    const transport: NotificationTransport = { sendEmail: sendEmailSpy };
+    // A no-op repo stub so the constructor does not open a real database.
+    const repo = { saveWebNotification: jest.fn(), findByUser: jest.fn() } as never;
+    service = new NotificationService({ emailTransport: transport, repo });
+  });
+
+  const event = 'KEY_REQUESTED' as unknown as KeyEscrowEvent;
+
+  const validAddresses = [
+    'user@example.com',
+    'first.last@sub.example.co.uk',
+    'a+tag@example.io',
+    'name_123@example-domain.com',
+  ];
+
+  const invalidAddresses = [
+    '', // empty
+    'plainaddress', // no @ / domain
+    'user@example', // missing TLD
+    'user@@example.com', // two @
+    'a@b,c@d.com', // multi-recipient (comma)
+    'a@b.com;c@d.com', // multi-recipient (semicolon)
+    '"x"@y.com', // quoted local part
+    'a\\b@c.com', // backslash
+    'user@exam ple.com', // whitespace
+    'foo<bar>@example.com', // angle brackets
+    'user@example.com\r\nBcc: evil@example.com', // CRLF header injection
+    'user@example.com\ninjected', // LF injection
+  ];
+
+  it.each(validAddresses)('accepts valid address %p', async (addr) => {
+    const res = await service.sendEmail(addr, event, { ok: true });
+    expect(res.success).toBe(true);
+    expect(sendEmailSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it.each(invalidAddresses)('rejects invalid/injection address %p', async (addr) => {
+    const res = await service.sendEmail(addr, event, { ok: true });
+    expect(res.success).toBe(false);
+    expect(res.message).toBe('Invalid email address');
+    // Validation must happen before any dispatch.
+    expect(sendEmailSpy).not.toHaveBeenCalled();
+  });
+});

src/services/notification.service.ts

@@ -70,11 +70,47 @@ export class NotificationService {
     this.repo = options?.repo ?? new NotificationRepository(getDb(process.env['DB_PATH'] ?? ':memory:'));
   }
 
+  /**
+   * Validates a single recipient email address before it is handed to an email
+   * transport.
+   *
+   * The check is intentionally strict because validated addresses flow into the
+   * (soon real) SMTP/SES/SendGrid transports, where permissive input enables
+   * header- and recipient-injection attacks. The rules are:
+   *
+   *  - Reject empty input and any CR/LF (header-injection) characters.
+   *  - Reject control characters and whitespace anywhere in the address.
+   *  - Reject comma/semicolon-separated multi-recipient strings
+   *    (e.g. `a@b.com,c@d.com`).
+   *  - Reject quoting/backslash forms (`"x"@y.com`, `a\b@c.com`) that SMTP can
+   *    misinterpret.
+   *  - Accept normal RFC-shaped single addresses with exactly one `@` and a
+   *    dotted domain with a TLD.
+   *
+   * The method keeps its boolean contract and signature unchanged so callers and
+   * the email transport can rely on deterministic behaviour.
+   *
+   * @param address The candidate recipient address.
+   * @returns `true` if the address is a safe, single, RFC-shaped recipient.
+   */
   private isValidEmail(address: string): boolean {
     if (!address) return false;
-    // Basic sanity check + header injection protection (no CR/LF)
-    if (/[\r\n]/.test(address)) return false;
-    const re = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
+    // Header-injection protection: reject CR/LF and other control characters.
+    // eslint-disable-next-line no-control-regex
+    if (/[\x00-\x1f\x7f]/.test(address)) return false;
+    // Reject multi-recipient separators that could smuggle extra recipients.
+    if (/[,;]/.test(address)) return false;
+    // Reject quoting/backslash forms that SMTP can misinterpret.
+    if (/["\\]/.test(address)) return false;
+    // Reject angle brackets / display-name forms.
+    if (/[<>()[\]]/.test(address)) return false;
+    // Exactly one '@' separating local part and domain.
+    const parts = address.split('@');
+    if (parts.length !== 2) return false;
+    const [local, domain] = parts;
+    if (!local || !domain) return false;
+    // Strict, single-address shape with a dotted domain and a TLD.
+    const re = /^[A-Za-z0-9!#$%&'*+/=?^_`{|}~.-]+@[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*\.[A-Za-z]{2,}$/;
     return re.test(address);
   }