diff --git a/src/lib/notifications/__tests__/upstream-body.test.ts b/src/lib/notifications/__tests__/upstream-body.test.ts index 021c6c1a0..c7ade0172 100644 --- a/src/lib/notifications/__tests__/upstream-body.test.ts +++ b/src/lib/notifications/__tests__/upstream-body.test.ts @@ -95,7 +95,7 @@ describe("secret extraction", () => { secretsInUrl( "https://user:pa%20ss@gotify.example.com/message?token=Abc123", ), - ).toEqual(["Abc123", "user", "pa ss"]); + ).toEqual(expect.arrayContaining(["Abc123", "user", "pa%20ss", "pa ss"])); expect(secretsInUrl("not a url")).toEqual([]); }); @@ -108,3 +108,65 @@ describe("secret extraction", () => { expect(secretsInHeaderValue(undefined)).toEqual([]); }); }); + +describe("secret matching survives encoding and malformed parts (review L4)", () => { + it("keeps the query token when the userinfo holds a malformed escape", () => { + const found = secretsInUrl( + "https://us%ZZer:pw@gotify.example.com/message?token=Abc12345", + ); + expect(found).toContain("Abc12345"); + expect(found).toContain("us%ZZer"); + }); + + it("keeps the other query values when one holds a malformed escape", () => { + const found = secretsInUrl( + "https://gotify.example.com/message?a=%ZZ&token=Abc12345", + ); + expect(found).toContain("Abc12345"); + expect(found).toContain("%ZZ"); + }); + + it("treats long path segments as secrets and short ones not", () => { + const found = secretsInUrl( + "https://discord.com/api/webhooks/123456789012345678/AbCdEfGhIjKlMnOpQrSt", + ); + expect(found).toContain("123456789012345678"); + expect(found).toContain("AbCdEfGhIjKlMnOpQrSt"); + expect(found).not.toContain("webhooks"); + expect(found).not.toContain("api"); + }); + + it("refuses a body echoing a path token", async () => { + await expect( + readUpstreamBody( + res('{"message":"Unknown Webhook AbCdEfGhIjKlMnOpQrSt"}', 404), + secretsInUrl( + "https://discord.com/api/webhooks/123/AbCdEfGhIjKlMnOpQrSt", + ), + ), + ).resolves.toBeUndefined(); + }); + + it("refuses a JSON-escaped echo with an escaped slash", async () => { + await expect( + readUpstreamBody(res('{"auth":"Basic dXNlcj\\/wYXNz"}'), [ + "dXNlcj/wYXNz", + ]), + ).resolves.toBeUndefined(); + }); + + it("refuses a JSON-escaped echo with an escaped quote", async () => { + await expect( + readUpstreamBody(res('{"got":"ab\\"cd1234"}'), ['ab"cd1234']), + ).resolves.toBeUndefined(); + }); + + it("refuses a percent-encoded echo, in either hex case", async () => { + await expect( + readUpstreamBody(res("bad token p%40ss%20word!"), ["p@ss word!"]), + ).resolves.toBeUndefined(); + await expect( + readUpstreamBody(res("bad token a%2fb%2Bc1234"), ["a/b+c1234"]), + ).resolves.toBeUndefined(); + }); +}); diff --git a/src/lib/notifications/upstream-body.ts b/src/lib/notifications/upstream-body.ts index dbf57bad2..60646c681 100644 --- a/src/lib/notifications/upstream-body.ts +++ b/src/lib/notifications/upstream-body.ts @@ -52,6 +52,36 @@ async function readBounded(res: Response): Promise { return undefined; } +/** + * The ways a relay can echo a secret back: literally, JSON-escaped (with and + * without the `\/` form some serialisers write for a slash, common inside a + * base64 value), and percent-encoded. + */ +function echoForms(secret: string): string[] { + const json = JSON.stringify(secret).slice(1, -1); + return [ + ...new Set([ + secret, + json, + json.replaceAll("/", "\\/"), + encodeURIComponent(secret), + ]), + ]; +} + +/** A value and its decoded form; a malformed escape keeps the raw value. */ +function withDecoded(part: string): string[] { + try { + const decoded = decodeURIComponent(part.replaceAll("+", " ")); + return decoded === part ? [part] : [part, decoded]; + } catch { + return [part]; + } +} + +/** Path segments this long are credentials in Discord, Slack and n8n URLs. */ +const MIN_SECRET_PATH_SEGMENT_LENGTH = 16; + /** * Read what a relay answered on a non-2xx, in a form safe to hand back to * the person who pressed the test button. @@ -87,9 +117,16 @@ export async function readUpstreamBody( if (!cleaned) return undefined; if (looksSecretShaped(raw) || looksSecretShaped(cleaned)) return undefined; + const rawLower = raw.toLowerCase(); for (const secret of knownSecrets) { if (!secret || secret.length < MIN_KNOWN_SECRET_LENGTH) continue; - if (raw.includes(secret) || cleaned.includes(secret)) return undefined; + for (const form of echoForms(secret)) { + if (raw.includes(form) || cleaned.includes(form)) return undefined; + } + // Percent-encoding is case-insensitive in its hex digits. + if (rawLower.includes(encodeURIComponent(secret).toLowerCase())) { + return undefined; + } } const characters = Array.from(cleaned); @@ -98,19 +135,33 @@ export async function readUpstreamBody( } /** - * The secrets a URL can carry on its own: userinfo and every query value - * (Gotify's `?token=` form). + * The secrets a URL can carry on its own: userinfo, every query value + * (Gotify's `?token=` form) and every path segment of 16 or more characters + * (a Discord or Slack webhook token, an n8n webhook id). Each part is decoded + * on its own, so one malformed escape cannot drop the others. */ export function secretsInUrl(url: string): string[] { + let parsed: URL; 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; + parsed = new URL(url); } catch { return []; } + const found: string[] = []; + if (parsed.username) found.push(...withDecoded(parsed.username)); + if (parsed.password) found.push(...withDecoded(parsed.password)); + for (const pair of parsed.search.replace(/^\?/, "").split("&")) { + const eq = pair.indexOf("="); + if (eq < 0) continue; + const value = pair.slice(eq + 1); + if (value) found.push(...withDecoded(value)); + } + for (const segment of parsed.pathname.split("/")) { + if (segment.length >= MIN_SECRET_PATH_SEGMENT_LENGTH) { + found.push(...withDecoded(segment)); + } + } + return [...new Set(found)]; } /** A header value and, for the ` ` form, the token alone. */