feat(notifications): keep what the relay answered on a failed send

The webhook and ntfy senders now carry up to 200 characters of a non-2xx response body on the send outcome, with control and formatting characters removed. The body is dropped when it looks like a credential or echoes the header value, auth token, topic or a URL token. Nothing stores it; the ledger row and channel state keep the reason code as before.

Transport faults without a status now say whether they were a timeout or a refused connection, and the email sender maps SMTP failures into the same vocabulary with the reply code. Both exist so the test routes can answer with the cause instead of a bare 500.

Refs #947

src/lib/notifications/__tests__/upstream-body.test.ts

@@ -0,0 +1,110 @@
+import { describe, expect, it } from "vitest";
+
+import {
+  readUpstreamBody,
+  secretsInHeaderValue,
+  secretsInUrl,
+  UPSTREAM_BODY_MAX_CHARS,
+} from "@/lib/notifications/upstream-body";
+
+function res(body: string, status = 400): Response {
+  return new Response(body, { status });
+}
+
+const RIGHT_TO_LEFT_OVERRIDE = String.fromCodePoint(0x202e);
+const ZERO_WIDTH_SPACE = String.fromCodePoint(0x200b);
+const BELL = String.fromCodePoint(0x07);
+
+describe("readUpstreamBody", () => {
+  it("returns a short JSON error body as it arrived", async () => {
+    const body =
+      '{"error":"Bad Request","errorCode":400,"errorDescription":"priority must be an integer"}';
+    await expect(readUpstreamBody(res(body), [])).resolves.toBe(body);
+  });
+
+  it("removes control and bidirectional characters and collapses whitespace", async () => {
+    const body = `line one\r\n\tline two ${RIGHT_TO_LEFT_OVERRIDE}evil${ZERO_WIDTH_SPACE}${BELL}`;
+    await expect(readUpstreamBody(res(body), [])).resolves.toBe(
+      "line one line two evil",
+    );
+  });
+
+  it("cuts a long body to the cap, marked with an ellipsis", async () => {
+    const result = await readUpstreamBody(res("x".repeat(5000)), []);
+    expect(Array.from(result ?? "")).toHaveLength(UPSTREAM_BODY_MAX_CHARS);
+    expect(result?.endsWith("…")).toBe(true);
+  });
+
+  it("reads at most a bounded prefix of an endless body", async () => {
+    let pulled = 0;
+    const stream = new ReadableStream<Uint8Array>({
+      pull(controller) {
+        pulled += 1;
+        controller.enqueue(new TextEncoder().encode("y".repeat(1024)));
+      },
+    });
+    const result = await readUpstreamBody(
+      new Response(stream, { status: 502 }),
+      [],
+    );
+    expect(result?.startsWith("yyyy")).toBe(true);
+    // 4 KB in 1 KB chunks: the body is abandoned after a few pulls.
+    expect(pulled).toBeLessThan(10);
+  });
+
+  it("refuses a secret-shaped body", async () => {
+    await expect(
+      readUpstreamBody(res('{"error":"bad key sk-ant-api03-abcdefgh"}'), []),
+    ).resolves.toBeUndefined();
+  });
+
+  it("refuses a body that echoes a configured secret", async () => {
+    await expect(
+      readUpstreamBody(res("token AbCdEf123 is not valid"), [
+        undefined,
+        "AbCdEf123",
+      ]),
+    ).resolves.toBeUndefined();
+  });
+
+  it("ignores a configured value too short to be a secret", async () => {
+    await expect(
+      readUpstreamBody(res("priority 5 rejected"), ["5"]),
+    ).resolves.toBe("priority 5 rejected");
+  });
+
+  it("returns nothing for an empty or unreadable body", async () => {
+    await expect(readUpstreamBody(res("   \n"), [])).resolves.toBeUndefined();
+    await expect(
+      readUpstreamBody({ ok: false, status: 400 } as Response, []),
+    ).resolves.toBeUndefined();
+    await expect(
+      readUpstreamBody(
+        {
+          text: () => Promise.reject(new Error("aborted")),
+        } as unknown as Response,
+        [],
+      ),
+    ).resolves.toBeUndefined();
+  });
+});
+
+describe("secret extraction", () => {
+  it("finds query values and userinfo in a URL", () => {
+    expect(
+      secretsInUrl(
+        "https://user:pa%20ss@gotify.example.com/message?token=Abc123",
+      ),
+    ).toEqual(["Abc123", "user", "pa ss"]);
+    expect(secretsInUrl("not a url")).toEqual([]);
+  });
+
+  it("keeps the token of a scheme-prefixed header value", () => {
+    expect(secretsInHeaderValue("Bearer tok123")).toEqual([
+      "Bearer tok123",
+      "tok123",
+    ]);
+    expect(secretsInHeaderValue("AppToken")).toEqual(["AppToken"]);
+    expect(secretsInHeaderValue(undefined)).toEqual([]);
+  });
+});

src/lib/notifications/retry-policy.ts

@@ -42,6 +42,22 @@ export interface SendOutcome {
    * instead of a bare 500.
    */
   errorCode?: string;
+  /**
+   * Why a delivery failed when there is no HTTP status to say it: a timeout,
+   * a refused connection, an SMTP reply. The value is a code from the test
+   * card's vocabulary (`settings.testConnection.errors`), so a test route can
+   * forward it as `meta.errorCode` without translating. Absent when the
+   * sender cannot tell, which the test routes treat as an internal failure.
+   */
+  failureCode?: string;
+  /** SMTP reply code of a rejected send (email only). */
+  smtpCode?: number;
+  /**
+   * What the other end answered on a non-2xx, bounded and cleaned by
+   * `readUpstreamBody`, and absent when it might carry a secret. Only the
+   * test routes read it; neither the ledger nor the channel state stores it.
+   */
+  upstreamBody?: string;
 }
 
 /**

src/lib/notifications/senders/__tests__/email.test.ts

@@ -170,3 +170,70 @@ describe("sendViaEmail", () => {
     );
   });
 });
+
+describe("sendViaEmail — the failure names itself for the test button", () => {
+  it.each([
+    [
+      "an authentication refusal",
+      Object.assign(new Error("Invalid login"), {
+        code: "EAUTH",
+        responseCode: 535,
+      }),
+      "credentials_rejected",
+      535,
+    ],
+    [
+      "a timeout",
+      Object.assign(new Error("Connection timeout"), { code: "ETIMEDOUT" }),
+      "timeout",
+      undefined,
+    ],
+    [
+      "a refused connection",
+      Object.assign(new Error("connect ECONNREFUSED"), {
+        code: "ECONNECTION",
+      }),
+      "connection_failed",
+      undefined,
+    ],
+    [
+      "a permanent SMTP rejection",
+      Object.assign(new Error("550 mailbox unavailable"), {
+        responseCode: 550,
+      }),
+      "upstream_rejected",
+      550,
+    ],
+    [
+      "a transient SMTP rejection",
+      Object.assign(new Error("451 try later"), { responseCode: 451 }),
+      "upstream_error",
+      451,
+    ],
+  ])("%s", async (_label, err, failureCode, smtpCode) => {
+    loadEmailConfigMock.mockReturnValue(transport);
+    sendMailMock.mockRejectedValue(err);
+
+    const result = await sendViaEmail(
+      { recipient: "you@example.com" },
+      payload(),
+    );
+
+    expect(result.ok).toBe(false);
+    expect(result.failureCode).toBe(failureCode);
+    expect(result.smtpCode).toBe(smtpCode);
+  });
+
+  it("leaves an unrecognised fault without a code", async () => {
+    loadEmailConfigMock.mockReturnValue(transport);
+    sendMailMock.mockRejectedValue(new Error("something else"));
+
+    const result = await sendViaEmail(
+      { recipient: "you@example.com" },
+      payload(),
+    );
+
+    expect(result.failureCode).toBeUndefined();
+    expect(result.smtpCode).toBeUndefined();
+  });
+});

src/lib/notifications/senders/__tests__/ntfy.test.ts

@@ -233,3 +233,38 @@ describe("sendViaNtfy", () => {
     });
   });
 });
+
+describe("sendViaNtfy — what the server said (#947)", () => {
+  it("carries a short error body on a non-2xx", async () => {
+    safeFetchMock.mockResolvedValue(
+      new Response('{"code":40009,"http":400,"error":"invalid request"}', {
+        status: 400,
+      }),
+    );
+
+    const result = await sendViaNtfy(config, payload());
+
+    expect(result).toMatchObject({
+      ok: false,
+      statusCode: 400,
+      upstreamBody: '{"code":40009,"http":400,"error":"invalid request"}',
+    });
+  });
+
+  it("drops a body that echoes the auth token or the topic", async () => {
+    safeFetchMock.mockResolvedValueOnce(
+      new Response("token tk_secret rejected", { status: 403 }),
+    );
+    expect((await sendViaNtfy(config, payload())).upstreamBody).toBeUndefined();
+
+    safeFetchMock.mockResolvedValueOnce(
+      new Response("topic health is reserved", { status: 403 }),
+    );
+    expect((await sendViaNtfy(config, payload())).upstreamBody).toBeUndefined();
+  });
+
+  it("names a timeout", async () => {
+    safeFetchMock.mockRejectedValue(new SafeFetchError("timed out", "timeout"));
+    expect((await sendViaNtfy(config, payload())).failureCode).toBe("timeout");
+  });
+});

src/lib/notifications/senders/__tests__/webhook.test.ts

@@ -281,3 +281,62 @@ describe("sendViaWebhook", () => {
     });
   });
 });
+
+describe("sendViaWebhook — what the relay said (#947)", () => {
+  it("carries a short error body on a non-2xx", async () => {
+    safeFetchMock.mockResolvedValue(
+      new Response('{"error":"Bad Request","errorCode":400}', { status: 400 }),
+    );
+
+    const result = await sendViaWebhook(config, payload());
+
+    expect(result).toMatchObject({
+      ok: false,
+      statusCode: 400,
+      reason: "webhook_400",
+      upstreamBody: '{"error":"Bad Request","errorCode":400}',
+    });
+    // The ledger row keeps the reason only, never the body.
+    expect(recordPushAttemptMock).toHaveBeenCalledWith(
+      expect.not.objectContaining({ upstreamBody: expect.anything() }),
+    );
+  });
+
+  it("drops a body that echoes the header token or a query token", async () => {
+    safeFetchMock.mockResolvedValueOnce(
+      new Response("invalid token secret", { status: 401 }),
+    );
+    const viaHeader = await sendViaWebhook(config, payload());
+    expect(viaHeader.upstreamBody).toBeUndefined();
+
+    safeFetchMock.mockResolvedValueOnce(
+      new Response("unknown token Q1w2E3r4", { status: 401 }),
+    );
+    const viaQuery = await sendViaWebhook(
+      { url: "https://relay.example.com/message?token=Q1w2E3r4" },
+      payload(),
+    );
+    expect(viaQuery.upstreamBody).toBeUndefined();
+  });
+
+  it("names a timeout and a refused connection, and leaves an unknown fault uncoded", async () => {
+    safeFetchMock.mockRejectedValueOnce(
+      new SafeFetchError("timed out", "timeout"),
+    );
+    expect((await sendViaWebhook(config, payload())).failureCode).toBe(
+      "timeout",
+    );
+
+    safeFetchMock.mockRejectedValueOnce(
+      new SafeFetchError("ECONNREFUSED", "network"),
+    );
+    expect((await sendViaWebhook(config, payload())).failureCode).toBe(
+      "connection_failed",
+    );
+
+    safeFetchMock.mockRejectedValueOnce(new TypeError("boom"));
+    expect(
+      (await sendViaWebhook(config, payload())).failureCode,
+    ).toBeUndefined();
+  });
+});

src/lib/notifications/senders/email.ts

@@ -79,19 +79,48 @@ function classifySmtpError(err: unknown): {
   hardReject: boolean;
   reason: string;
   message: string;
+  failureCode?: string;
+  smtpCode?: number;
 } {
   const message = err instanceof Error ? err.message : "smtp_send_failed";
   // nodemailer attaches `responseCode` (the SMTP reply code) on protocol-level
-  // rejections. 5xx = permanent (bad mailbox, relay denied) → hard reject so
-  // the channel auto-disables. 4xx / connection errors = transient → retry.
+  // rejections and a `code` string on transport faults. 5xx = permanent (bad
+  // mailbox, relay denied) → hard reject so the channel auto-disables.
+  // 4xx / connection errors = transient → retry.
   const responseCode =
     typeof err === "object" && err !== null && "responseCode" in err
       ? (err as { responseCode?: number }).responseCode
       : undefined;
-  if (typeof responseCode === "number" && responseCode >= 500) {
-    return { hardReject: true, reason: "email_smtp_5xx", message };
+  const code =
+    typeof err === "object" && err !== null && "code" in err
+      ? (err as { code?: unknown }).code
+      : undefined;
+  const smtpCode = typeof responseCode === "number" ? responseCode : undefined;
+  // The test card's vocabulary, so the test route can say which it was.
+  // Anything unrecognised stays without a code.
+  const failureCode =
+    code === "EAUTH"
+      ? "credentials_rejected"
+      : code === "ETIMEDOUT"
+        ? "timeout"
+        : code === "ECONNECTION" ||
+            code === "ESOCKET" ||
+            code === "EDNS" ||
+            code === "ECONNREFUSED"
+          ? "connection_failed"
+          : smtpCode !== undefined && smtpCode >= 500
+            ? "upstream_rejected"
+            : smtpCode !== undefined && smtpCode >= 400
+              ? "upstream_error"
+              : undefined;
+  const extras = {
+    ...(failureCode ? { failureCode } : {}),
+    ...(smtpCode !== undefined ? { smtpCode } : {}),
+  };
+  if (smtpCode !== undefined && smtpCode >= 500) {
+    return { hardReject: true, reason: "email_smtp_5xx", message, ...extras };
   }
-  return { hardReject: false, reason: "email_smtp_error", message };
+  return { hardReject: false, reason: "email_smtp_error", message, ...extras };
 }
 
 export async function sendViaEmail(
@@ -169,6 +198,12 @@ export async function sendViaEmail(
       hardReject: classified.hardReject,
       reason: classified.reason,
       message: classified.message,
+      ...(classified.failureCode
+        ? { failureCode: classified.failureCode }
+        : {}),
+      ...(classified.smtpCode !== undefined
+        ? { smtpCode: classified.smtpCode }
+        : {}),
     };
   }
 }

src/lib/notifications/senders/ntfy.ts

@@ -16,6 +16,10 @@ import {
   PRIVATE_ORIGIN_NOT_APPROVED_CODE,
 } from "@/lib/notifications/egress-policy";
 import type { OriginReason } from "@/lib/private-origin-policy";
+import {
+  readUpstreamBody,
+  secretsInUrl,
+} from "@/lib/notifications/upstream-body";
 
 /**
  * Send notification via ntfy (simple HTTP POST).
@@ -129,11 +133,19 @@ export async function sendViaNtfy(
       result: "error",
       reason: classified.reason,
     });
+    // What the server said, for the test button. The topic is treated as a
+    // secret too: on a public ntfy server it is the only access control.
+    const upstreamBody = await readUpstreamBody(res, [
+      config.authToken,
+      config.topic,
+      ...secretsInUrl(config.serverUrl),
+    ]);
     return {
       ok: false,
       statusCode: res.status,
       hardReject: classified.hardReject,
       reason: classified.reason,
+      ...(upstreamBody ? { upstreamBody } : {}),
     };
   } catch (err) {
     const message = err instanceof Error ? err.message : "request_failed";
@@ -144,6 +156,12 @@ export async function sendViaNtfy(
     const reason = policyRefused
       ? "ntfy_private_origin_refused"
       : "ntfy_network_error";
+    const failureCode =
+      err instanceof SafeFetchError && !policyRefused
+        ? err.kind === "timeout"
+          ? "timeout"
+          : "connection_failed"
+        : undefined;
     getEvent()?.addExternalCall({
       service: "ntfy",
       method: "sendNotification",
@@ -165,6 +183,7 @@ export async function sendViaNtfy(
       ...(policyRefused
         ? { errorCode: policyReason ?? PRIVATE_ORIGIN_NOT_APPROVED_CODE }
         : {}),
+      ...(failureCode ? { failureCode } : {}),
     };
   }
 }

src/lib/notifications/senders/webhook.ts

@@ -15,6 +15,11 @@ import {
   PRIVATE_ORIGIN_NOT_APPROVED_CODE,
 } from "@/lib/notifications/egress-policy";
 import type { OriginReason } from "@/lib/private-origin-policy";
+import {
+  readUpstreamBody,
+  secretsInHeaderValue,
+  secretsInUrl,
+} from "@/lib/notifications/upstream-body";
 
 /**
  * Send a notification via a generic outbound webhook (v1.17.1).
@@ -130,11 +135,18 @@ export async function sendViaWebhook(
       result: "error",
       reason: classified.reason,
     });
+    // What the relay said, for the test button (Gotify names the field it
+    // could not bind). Refused when it echoes the header or a URL token.
+    const upstreamBody = await readUpstreamBody(res, [
+      ...secretsInHeaderValue(config.headerValue),
+      ...secretsInUrl(config.url),
+    ]);
     return {
       ok: false,
       statusCode: res.status,
       hardReject: classified.hardReject,
       reason: classified.reason,
+      ...(upstreamBody ? { upstreamBody } : {}),
     };
   } catch (err) {
     const message = err instanceof Error ? err.message : "request_failed";
@@ -147,6 +159,12 @@ export async function sendViaWebhook(
     const reason = policyRefused
       ? "webhook_private_origin_refused"
       : "webhook_network_error";
+    const failureCode =
+      err instanceof SafeFetchError && !policyRefused
+        ? err.kind === "timeout"
+          ? "timeout"
+          : "connection_failed"
+        : undefined;
     getEvent()?.addExternalCall({
       service: "webhook",
       method: "sendNotification",
@@ -168,6 +186,7 @@ export async function sendViaWebhook(
       ...(policyRefused
         ? { errorCode: policyReason ?? PRIVATE_ORIGIN_NOT_APPROVED_CODE }
         : {}),
+      ...(failureCode ? { failureCode } : {}),
     };
   }
 }

src/lib/notifications/upstream-body.ts

@@ -0,0 +1,121 @@
+import { looksSecretShaped } from "@/lib/secret-shape";
+
+/** What the test card shows at most. */
+export const UPSTREAM_BODY_MAX_CHARS = 200;
+/** What is read off the wire at most, before anything is cleaned or cut. */
+const UPSTREAM_BODY_MAX_READ_BYTES = 4096;
+/** A configured value shorter than this is not treated as a secret to hide. */
+const MIN_KNOWN_SECRET_LENGTH = 4;
+
+/**
+ * C0 and C1 control characters, plus the zero-width and bidirectional
+ * formatting marks that could make the rendered text read differently from
+ * what the relay sent.
+ */
+function isControlOrFormatting(codePoint: number): boolean {
+  return (
+    codePoint <= 0x1f ||
+    (codePoint >= 0x7f && codePoint <= 0x9f) ||
+    (codePoint >= 0x200b && codePoint <= 0x200f) ||
+    (codePoint >= 0x202a && codePoint <= 0x202e) ||
+    (codePoint >= 0x2066 && codePoint <= 0x2069)
+  );
+}
+
+async function readBounded(res: Response): Promise<string | undefined> {
+  const reader = res.body?.getReader?.();
+  if (reader) {
+    const chunks: Uint8Array[] = [];
+    let total = 0;
+    try {
+      while (total < UPSTREAM_BODY_MAX_READ_BYTES) {
+        const { done, value } = await reader.read();
+        if (done || !value) break;
+        chunks.push(value);
+        total += value.byteLength;
+      }
+    } finally {
+      // An oversized error page is abandoned, not drained.
+      await reader.cancel().catch(() => {});
+    }
+    const bytes = new Uint8Array(Math.min(total, UPSTREAM_BODY_MAX_READ_BYTES));
+    let offset = 0;
+    for (const chunk of chunks) {
+      const room = bytes.length - offset;
+      if (room <= 0) break;
+      bytes.set(chunk.subarray(0, room), offset);
+      offset += Math.min(chunk.byteLength, room);
+    }
+    return new TextDecoder("utf-8", { fatal: false }).decode(bytes);
+  }
+  if (typeof res.text === "function") return res.text();
+  return undefined;
+}
+
+/**
+ * Read what a relay answered on a non-2xx, in a form safe to hand back to
+ * the person who pressed the test button.
+ *
+ * Bounded twice: at most 4 KB is read off the wire, and at most 200
+ * characters survive. Control and formatting characters are removed and
+ * whitespace collapsed. The text is refused outright (undefined) when it is
+ * empty, unreadable, secret-shaped, or contains one of `knownSecrets` (the
+ * header value, the auth token, a query-string token): a relay that echoes
+ * the credential it rejected must not have that credential shown on screen.
+ *
+ * The result is for the test routes. Nothing stores it: the ledger and the
+ * channel state keep only the status and the reason code.
+ */
+export async function readUpstreamBody(
+  res: Response,
+  knownSecrets: ReadonlyArray<string | undefined>,
+): Promise<string | undefined> {
+  let raw: string | undefined;
+  try {
+    raw = await readBounded(res);
+  } catch {
+    return undefined;
+  }
+  if (!raw) return undefined;
+
+  const cleaned = Array.from(raw, (ch) =>
+    isControlOrFormatting(ch.codePointAt(0) ?? 0) ? " " : ch,
+  )
+    .join("")
+    .replace(/\s+/g, " ")
+    .trim();
+  if (!cleaned) return undefined;
+
+  if (looksSecretShaped(raw) || looksSecretShaped(cleaned)) return undefined;
+  for (const secret of knownSecrets) {
+    if (!secret || secret.length < MIN_KNOWN_SECRET_LENGTH) continue;
+    if (raw.includes(secret) || cleaned.includes(secret)) return undefined;
+  }
+
+  const characters = Array.from(cleaned);
+  if (characters.length <= UPSTREAM_BODY_MAX_CHARS) return cleaned;
+  return `${characters.slice(0, UPSTREAM_BODY_MAX_CHARS - 1).join("")}…`;
+}
+
+/**
+ * The secrets a URL can carry on its own: userinfo and every query value
+ * (Gotify's `?token=` form).
+ */
+export function secretsInUrl(url: string): string[] {
+  try {
+    const parsed = new URL(url);
+    const found = [...parsed.searchParams.values()];
+    if (parsed.username) found.push(decodeURIComponent(parsed.username));
+    if (parsed.password) found.push(decodeURIComponent(parsed.password));
+    return found;
+  } catch {
+    return [];
+  }
+}
+
+/** A header value and, for the `<scheme> <token>` form, the token alone. */
+export function secretsInHeaderValue(value: string | undefined): string[] {
+  if (!value) return [];
+  const parts = value.trim().split(/\s+/);
+  return parts.length > 1 ? [value, parts[parts.length - 1]] : [value];
+}