From b87ded090cd8022aab4566e0561ce4326c38db0d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= <mbombeck@gmail.com>
Date: Mon, 15 Jun 2026 01:27:49 +0200
Subject: [PATCH] fix(notifications): bound the SMTP transport with explicit
 timeouts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The email sender built its nodemailer transport without connection,
greeting, or socket timeouts, so nodemailer's defaults applied — up to a
10-minute socket timeout. A mail server that accepts the TCP connection
then stalls (firewalled relay, overloaded Postfix, blackholed smarthost)
could hold a dispatch worker for minutes per notification, while every
other egress in the cascade is bounded. Pin connection/greeting at 10s
and socket at 20s so a stall maps to the transient send outcome the SMTP
classifier already retries, matching the webhook sender's ethos.
---
 .../senders/__tests__/email.test.ts           | 26 ++++++++++++++++---
 src/lib/notifications/senders/email.ts        |  8 ++++++
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/src/lib/notifications/senders/__tests__/email.test.ts b/src/lib/notifications/senders/__tests__/email.test.ts
index 53066c67a..e78403306 100644
--- a/src/lib/notifications/senders/__tests__/email.test.ts
+++ b/src/lib/notifications/senders/__tests__/email.test.ts
@@ -8,15 +8,16 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
 const sendMailMock = vi.fn();
-const createTransportMock = vi.fn((..._args: unknown[]) => ({
-  sendMail: sendMailMock,
-}));
+const createTransportMock = vi.fn((options: unknown) => {
+  void options;
+  return { sendMail: sendMailMock };
+});
 const loadEmailConfigMock = vi.fn();
 const recordPushAttemptMock = vi.fn();
 
 vi.mock("nodemailer", () => ({
   default: {
-    createTransport: (...args: unknown[]) => createTransportMock(...args),
+    createTransport: (options: unknown) => createTransportMock(options),
   },
 }));
 
@@ -86,6 +87,23 @@ describe("sendViaEmail", () => {
     );
   });
 
+  it("builds the transport with explicit SMTP timeouts (no unbounded hang)", async () => {
+    loadEmailConfigMock.mockReturnValue(transport);
+    sendMailMock.mockResolvedValue({ messageId: "1" });
+
+    await sendViaEmail({ recipient: "you@example.com" }, payload());
+
+    expect(createTransportMock).toHaveBeenCalledTimes(1);
+    const opts = createTransportMock.mock.calls[0][0] as {
+      connectionTimeout?: number;
+      greetingTimeout?: number;
+      socketTimeout?: number;
+    };
+    expect(opts.connectionTimeout).toBe(10_000);
+    expect(opts.greetingTimeout).toBe(10_000);
+    expect(opts.socketTimeout).toBe(20_000);
+  });
+
   it("soft-skips when SMTP is unconfigured (no channel burn)", async () => {
     loadEmailConfigMock.mockReturnValue(null);
 
diff --git a/src/lib/notifications/senders/email.ts b/src/lib/notifications/senders/email.ts
index 289c7490b..b072cb90b 100644
--- a/src/lib/notifications/senders/email.ts
+++ b/src/lib/notifications/senders/email.ts
@@ -55,6 +55,14 @@ function getTransporter(): { transporter: Transporter; from: string } | null {
     port: config.port,
     secure: config.secure,
     ...(config.auth ? { auth: config.auth } : {}),
+    // Bound every phase of the SMTP exchange. nodemailer's defaults (2 min
+    // connect, 30 s greeting, 10 min socket) would let a hung/blackholed relay
+    // pin a dispatch worker for minutes per send. These caps map a stall to the
+    // transient `SendOutcome` `classifySmtpError` already retries, matching the
+    // webhook sender's 5 s ethos.
+    connectionTimeout: 10_000,
+    greetingTimeout: 10_000,
+    socketTimeout: 20_000,
   });
   cachedTransportKey = key;
   return { transporter: cachedTransporter, from: config.from };