diff --git a/src/lib/notifications/__tests__/upstream-body.test.ts b/src/lib/notifications/__tests__/upstream-body.test.ts new file mode 100644 index 000000000..021c6c1a0 --- /dev/null +++ b/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({ + 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([]); + }); +}); diff --git a/src/lib/notifications/retry-policy.ts b/src/lib/notifications/retry-policy.ts index 6dbdfe8d8..958024563 100644 --- a/src/lib/notifications/retry-policy.ts +++ b/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; } /** diff --git a/src/lib/notifications/senders/__tests__/email.test.ts b/src/lib/notifications/senders/__tests__/email.test.ts index 998ec15f6..83f12cec3 100644 --- a/src/lib/notifications/senders/__tests__/email.test.ts +++ b/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(); + }); +}); diff --git a/src/lib/notifications/senders/__tests__/ntfy.test.ts b/src/lib/notifications/senders/__tests__/ntfy.test.ts index 9bb4c17be..e38efa619 100644 --- a/src/lib/notifications/senders/__tests__/ntfy.test.ts +++ b/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"); + }); +}); diff --git a/src/lib/notifications/senders/__tests__/webhook.test.ts b/src/lib/notifications/senders/__tests__/webhook.test.ts index 255f1705e..6bec91585 100644 --- a/src/lib/notifications/senders/__tests__/webhook.test.ts +++ b/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(); + }); +}); diff --git a/src/lib/notifications/senders/email.ts b/src/lib/notifications/senders/email.ts index 86afb748e..2b518638d 100644 --- a/src/lib/notifications/senders/email.ts +++ b/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 } + : {}), }; } } diff --git a/src/lib/notifications/senders/ntfy.ts b/src/lib/notifications/senders/ntfy.ts index 1692112b8..a2fe63401 100644 --- a/src/lib/notifications/senders/ntfy.ts +++ b/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 } : {}), }; } } diff --git a/src/lib/notifications/senders/webhook.ts b/src/lib/notifications/senders/webhook.ts index c9d480a7e..baad20ccc 100644 --- a/src/lib/notifications/senders/webhook.ts +++ b/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 } : {}), }; } } diff --git a/src/lib/notifications/upstream-body.ts b/src/lib/notifications/upstream-body.ts new file mode 100644 index 000000000..dbf57bad2 --- /dev/null +++ b/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 { + 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, +): Promise { + 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 ` ` 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]; +}