Skip to content

security: definitive log-injection fix via encodeURIComponent (v5.1.2) - #53

Merged
OmarRao merged 3 commits into
mainfrom
security/loginj-encode-v5.1.2
Sep 23, 2026
Merged

OmarRao merged 3 commits into
mainfrom
security/loginj-encode-v5.1.2

Conversation

@OmarRao

@OmarRao OmarRao commented Sep 23, 2026

Copy link
Copy Markdown
Owner

The v5.1.1 inline String(x).replace(/[\r\n]+/g, ' ') was not recognized by CodeQL as a log-injection sanitizer, so all 16 js/log-injection alerts reopened on main (verified via the code-scanning API: state: open on refs/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

…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.
Comment thread src/backup/gdrive.js Fixed
Comment thread src/backup/gdrive.js Fixed
Comment thread src/backup/index.js Fixed
Comment thread src/backup/index.js Fixed
Comment thread src/backup/index.js Fixed
Comment thread src/backup/storage/fanout.js Fixed
Comment thread src/restore/index.js Fixed
Comment thread src/restore/index.js Fixed
Comment thread src/restore/index.js Fixed
Comment thread src/restore/index.js Fixed
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.
Comment thread src/backup/gdrive.js Fixed
Comment thread src/backup/gdrive.js Fixed
Comment thread src/backup/index.js Fixed
Comment thread src/backup/index.js Fixed
Comment thread src/backup/index.js Fixed
Comment thread src/backup/storage/fanout.js Fixed
Comment thread src/restore/index.js Fixed
Comment thread src/restore/index.js Fixed
Comment thread src/restore/index.js Fixed
Comment thread src/restore/index.js Fixed
…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.
@OmarRao
OmarRao merged commit 1d7faab into main Sep 23, 2026
5 checks passed
@OmarRao
OmarRao deleted the security/loginj-encode-v5.1.2 branch September 23, 2026 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants