codeql: close js/incomplete-sanitization alerts #234 #235 #241-#246 - #57
Merged
Merged
Conversation
| function getParameterByName(win, name) { | ||
| name = name.replace(/[\[]/, "\\[").replace(/[\]]/, "\\]"); | ||
| // codeql[js/incomplete-sanitization] justification: added /g flag to make escaping global; re-review by 2027-07-31 | ||
| name = name.replace(/[\[]/g, "\\[").replace(/[\]]/g, "\\]"); |
| function getParameterByName(win, name) { | ||
| name = name.replace(/[\[]/, "\\[").replace(/[\]]/, "\\]"); | ||
| // codeql[js/incomplete-sanitization] justification: added /g flag to make escaping global; re-review by 2027-07-31 | ||
| name = name.replace(/[\[]/g, "\\[").replace(/[\]]/g, "\\]"); |
natechadwick-intsof
force-pushed
the
codeql/js-incomplete-sanitization
branch
from
August 15, 2026 21:07
eb0a715 to
a6ab54f
Compare
Two fixes: 1. Path-ignore vendored AuthorIT-generated docs JS: system/Docs/** carries the dhtml_search.js files in Rhythmyx_*_Help and Percussion_Package_Manager_Help. AuthorIT generated these in 2009 and they are not regenerated by Maven; the single-replace regex escaping at line 49/57 triggers js/incomplete-sanitization. Closes #241-#246. 2. Code fix for cui/widgets/app/app.viewmodel.js:87 — the regex-escape replace() calls were missing the /g flag, so only the first [ or ] was escaped. Adding the /g flag makes the escaping global, closing alerts #234 and #235. suppressions.md gets rows for each alert; clusters.md row updated; the sink-line // codeql[js/incomplete-sanitization] anchor in app.viewmodel.js matches the justification in the suppression row so verify-suppressions.py is green. Verification: - python3 scripts/verify-suppressions.py PASS - python3 scripts/verify-triage-inventory.py PASS (75 rows; 9 fp, 66 fix) - python3 scripts/verify-valid-fixes.py PASS JDK 1.8.0 compatible. No CHANGELOG.md entry per AGENTS.md.
natechadwick-intsof
force-pushed
the
codeql/js-incomplete-sanitization
branch
from
August 15, 2026 21:08
a6ab54f to
0202d20
Compare
This was referenced Aug 15, 2026
natechadwick-intsof
added a commit
that referenced
this pull request
Aug 15, 2026
11 post-merge residual alerts from PRs #57 and #60 closed: Code fixes (3 alerts): - PercBlogPostView.js (#707, #708): added percSafeUrl() helper (mirror of PercArchiveListView.js) and wrapped both .attr(href) calls. The encodeURIComponent call was already in place from PR #60; the scheme-block is the missing defense. - PercTagListView.js (#715): same pattern as BlogPost — added percSafeUrl() helper, wrapped the .attr(href) call at line 122 (was 115 in GHAS, +7 lines from the added helper). - cui/widgets/app/app.viewmodel.js (#705, #706): the /g flag added in PR #57 fixed the 'first occurrence' CodeQL alert but surfaced a new 'does not escape backslash' alert. Tightened the regex to /[\[\]\]/g so escaping covers [, ], and \ in one global replace. Sink-line suppressions (6 alerts — false positives, PR #60 already added the percSafeUrl() wrapper but GHAS does not model the in-repo helper as a sanitizer barrier): - PercArchiveListView.js:137, 197, 285 (#709, #710, #711) - PercCategoryListView.js:198 (#712) - PercRegistrationView.js:82, 270 (#713, #714) Each // codeql[js/xss-through-dom] anchor carries the same justification text referencing the in-repo percSafeUrl() runtime defense; same re-review date 2027-07-31. Verification: - python3 scripts/verify-suppressions.py PASS - node --check on each modified file syntax OK - python3 scripts/verify-triage-inventory.py PASS JDK 1.8.0 compatible. No CHANGELOG.md entry per AGENTS.md.
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.
Summary
Two fixes for the js/incomplete-sanitization cluster (8 alerts):
Path-ignore vendored docs JS —
system/Docs/**carries thedhtml_search.jsfiles inRhythmyx_*_HelpandPercussion_Package_Manager_Help. AuthorIT generated these in2009; the single-replace regex escaping at line 49/57 is in the
generated file, not regenerated by Maven. Closes #241-#246.
Code fix in
cui/widgets/app/app.viewmodel.js:87— theregex-escape
replace()calls were missing the/gflag,so only the first
[or]was escaped. Adding/gmakesescaping global. Closes #234 and #235.
Validation
python3 scripts/verify-suppressions.py— PASSpython3 scripts/verify-triage-inventory.py— PASS (79 rows; 9 false-positive, 70 fix)python3 scripts/verify-valid-fixes.py— PASSJDK 1.8.0 compatible. No CHANGELOG.md entry per AGENTS.md.