Skip to content

Commit 00c0643

Browse files
Aaron K. Clarkclaude
andcommitted
fix(redact-url): handle malformed percent-encoding without throwing
`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>
1 parent d40ecb6 commit 00c0643

2 files changed

Lines changed: 29 additions & 1 deletion

File tree

app/middleware/redact-url.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,20 @@ function redactUrl(url) {
4444
if (part.length === 0) continue;
4545
const eq = part.indexOf('=');
4646
const rawName = eq === -1 ? part : part.slice(0, eq);
47-
const name = decodeURIComponent(rawName).toLowerCase();
47+
// decodeURIComponent throws URIError on malformed percent-encoding
48+
// (e.g. `?%FF=v` or `?%ZZ=v`). pino-http calls this serializer for
49+
// every request, so an unhandled throw here would either skip the
50+
// log line entirely or fall back to logging the raw URL — leaking
51+
// the very value we're trying to redact. Wrap and treat
52+
// undecodable names as non-sensitive (the raw bytes are kept in
53+
// the output, so no value is lost; a malformed name almost
54+
// certainly isn't a real `authkey=…` anyway).
55+
let name;
56+
try {
57+
name = decodeURIComponent(rawName).toLowerCase();
58+
} catch (_err) {
59+
name = rawName.toLowerCase();
60+
}
4861
if (SENSITIVE_PARAM_NAMES.has(name)) {
4962
out.push(`${rawName}=<REDACTED>`);
5063
} else {

tests/unit/redact-url.test.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,19 @@ describe('redactUrl', () => {
6969
expect(redactUrl(null)).toBe(null);
7070
expect(redactUrl(42)).toBe(42);
7171
});
72+
73+
test('does not throw on malformed percent-encoding in param name', () => {
74+
// decodeURIComponent throws URIError on `%FF` (incomplete UTF-8
75+
// sequence) or `%ZZ` (invalid hex). pino-http would invoke this
76+
// serializer once per request, so an unhandled URIError here would
77+
// either skip the log line or fall back to logging the raw URL —
78+
// leaking the very value we're meant to redact. Wrap + recover.
79+
expect(() => redactUrl('/x?%FF=bar')).not.toThrow();
80+
expect(() => redactUrl('/x?%ZZ=bar&authkey=secret')).not.toThrow();
81+
// And the sensitive param that follows the malformed one must
82+
// still be redacted — recovery doesn't abort the loop.
83+
const out = redactUrl('/x?%ZZ=bar&authkey=secret');
84+
expect(out).not.toContain('secret');
85+
expect(out).toContain('authkey=<REDACTED>');
86+
});
7287
});

0 commit comments

Comments
 (0)