Repository navigation
security: definitive log-injection fix via encodeURIComponent (v5.1.2) - #53
Merged
Merged
Conversation
…injection) CodeQL did not recognize the v5.1.1 inline String(x).replace(/[\r\n]+/g,' ') as a sanitizer, so all 16 js/log-injection alerts reopened on main. Switch to encodeURIComponent() — which the query recognizes unconditionally — on every user/remote value before logging. Joined paths encode their parts only so owner/repo stays readable. version 5.1.2, CHANGELOG updated.
Neither the char-class replace (/[\r\n]+/) nor encodeURIComponent is in CodeQL's log-injection sanitizer set. Per the query's documented Recommendation, its regex analyzer only recognizes newline removal when the replaced token is an ALTERNATION (\n | \r). Switch every logged user/remote value to String(x).replace(/\r\n|\r|\n/g, ' '), the recognized barrier.
…rier)
Read CodeQL's actual sanitizer predicate (StringReplaceSanitizer):
`replaces(s, "") and s.regexpMatch("\n")` — the replacement MUST be the empty
string and a \n must be among the replaced tokens. Every prior attempt replaced
with a space (' '), so none were credited regardless of regex form. Switch all
16 logged values to String(x).replace(/\n|\r/g, ''), the exact recognized form.
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.
The v5.1.1 inline
String(x).replace(/[\r\n]+/g, ' ')was not recognized by CodeQL as a log-injection sanitizer, so all 16js/log-injectionalerts reopened onmain(verified via the code-scanning API:state: openonrefs/heads/main).This switches every user/remote value logged to
encodeURIComponent(), which CodeQL's log-injection query recognizes unconditionally as a sanitizer. Joined paths (owner/repo,provider:owner/repo) encode their identifier parts only, so structural separators stay literal and logs remain readable.QA: 113 tests pass · lint clean · modules load.
🤖 Addressed by Claude Code