From 00c06439f1aee876d3e52033d4dcafb40c19e964 Mon Sep 17 00:00:00 2001
From: "Aaron K. Clark" <akclark@thenetwerk.net>
Date: Tue, 19 May 2026 00:52:56 -0500
Subject: [PATCH] fix(redact-url): handle malformed percent-encoding without
 throwing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`decodeURIComponent` raises URIError on invalid percent sequences
(incomplete UTF-8 like `%FF`, or non-hex like `%ZZ`). pino-http
invokes `redactUrl` from its request serializer once per request,
so an unhandled URIError here would either skip the log line
entirely or — depending on pino's serializer-error fallback path —
log the raw URL, leaking the very `authkey=…` / `token=…` /
`password=…` value we're meant to redact.

Wrap the `decodeURIComponent(rawName)` call: on URIError, fall back
to lowercasing the raw (still-encoded) name. The raw bytes are
preserved either way in the output, so no value is lost; a
percent-malformed param name is almost certainly not a real
sensitive-list entry anyway.

Regression test verifies:
- malformed-name URLs don't throw (the bug)
- a sensitive param following a malformed one is still redacted
  (the loop must keep going after recovery, not abort)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 app/middleware/redact-url.js  | 15 ++++++++++++++-
 tests/unit/redact-url.test.js | 15 +++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/app/middleware/redact-url.js b/app/middleware/redact-url.js
index e1f214b..8555216 100644
--- a/app/middleware/redact-url.js
+++ b/app/middleware/redact-url.js
@@ -44,7 +44,20 @@ function redactUrl(url) {
         if (part.length === 0) continue;
         const eq = part.indexOf('=');
         const rawName = eq === -1 ? part : part.slice(0, eq);
-        const name = decodeURIComponent(rawName).toLowerCase();
+        // decodeURIComponent throws URIError on malformed percent-encoding
+        // (e.g. `?%FF=v` or `?%ZZ=v`). pino-http calls this serializer for
+        // every request, so an unhandled throw here would either skip the
+        // log line entirely or fall back to logging the raw URL — leaking
+        // the very value we're trying to redact. Wrap and treat
+        // undecodable names as non-sensitive (the raw bytes are kept in
+        // the output, so no value is lost; a malformed name almost
+        // certainly isn't a real `authkey=…` anyway).
+        let name;
+        try {
+            name = decodeURIComponent(rawName).toLowerCase();
+        } catch (_err) {
+            name = rawName.toLowerCase();
+        }
         if (SENSITIVE_PARAM_NAMES.has(name)) {
             out.push(`${rawName}=<REDACTED>`);
         } else {
diff --git a/tests/unit/redact-url.test.js b/tests/unit/redact-url.test.js
index 6ee0112..ac91e51 100644
--- a/tests/unit/redact-url.test.js
+++ b/tests/unit/redact-url.test.js
@@ -69,4 +69,19 @@ describe('redactUrl', () => {
         expect(redactUrl(null)).toBe(null);
         expect(redactUrl(42)).toBe(42);
     });
+
+    test('does not throw on malformed percent-encoding in param name', () => {
+        // decodeURIComponent throws URIError on `%FF` (incomplete UTF-8
+        // sequence) or `%ZZ` (invalid hex). pino-http would invoke this
+        // serializer once per request, so an unhandled URIError here would
+        // either skip the log line or fall back to logging the raw URL —
+        // leaking the very value we're meant to redact. Wrap + recover.
+        expect(() => redactUrl('/x?%FF=bar')).not.toThrow();
+        expect(() => redactUrl('/x?%ZZ=bar&authkey=secret')).not.toThrow();
+        // And the sensitive param that follows the malformed one must
+        // still be redacted — recovery doesn't abort the loop.
+        const out = redactUrl('/x?%ZZ=bar&authkey=secret');
+        expect(out).not.toContain('secret');
+        expect(out).toContain('authkey=<REDACTED>');
+    });
 });