fix(notifications): refuse encoded and path-borne secrets in replies

Userinfo and each query value are decoded on their own, so one malformed escape no longer discards every other secret in the URL. A reply is compared with each secret in literal, JSON-escaped and percent-encoded form, and URL path segments of 16 or more characters count as secrets, which covers the token in a Discord or Slack webhook URL and an n8n webhook id.

Refs #947

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();
+  });
+});

src/lib/notifications/upstream-body.ts

@@ -52,6 +52,36 @@ async function readBounded(res: Response): Promise<string | undefined> {
   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 `<scheme> <token>` form, the token alone. */