fix(redact-url): handle malformed percent-encoding without throwing#128
Merged
Conversation
`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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #127.
Summary
decodeURIComponent(rawName)on line 47 isn't try-wrapped. Invalid percent-encoding (%FF,%ZZ, etc.) raisesURIError: URI malformed. Because pino-http callsredactUrlonce per request, an unhandled URIError there would either skip the log line or fall back to logging the raw URL — leaking the veryauthkey=…/token=…/password=…value we're meant to redact.Wrap the decode in try/catch; on URIError fall back to lowercasing the raw (still-encoded) name. The raw bytes are preserved in the output either way, so no value is lost; a percent-malformed name almost certainly isn't a real entry in the sensitive-param allowlist.
Test plan
npm run lint— cleannpm test— 485 passed (was 484 + new regression test), 15 skippedProudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/