Skip to content

codeql: close js/incomplete-sanitization alerts #234 #235 #241-#246 - #57

Merged
natechadwick-intsof merged 1 commit into
mainfrom
codeql/js-incomplete-sanitization
Aug 15, 2026
Merged

codeql: close js/incomplete-sanitization alerts #234 #235 #241-#246#57
natechadwick-intsof merged 1 commit into
mainfrom
codeql/js-incomplete-sanitization

Conversation

@natechadwick-intsof

Copy link
Copy Markdown
Collaborator

Summary

Two fixes for the js/incomplete-sanitization cluster (8 alerts):

  1. Path-ignore vendored docs JSsystem/Docs/** carries the
    dhtml_search.js files in Rhythmyx_*_Help and
    Percussion_Package_Manager_Help. AuthorIT generated these in
    2009; the single-replace regex escaping at line 49/57 is in the
    generated file, not regenerated by Maven. Closes #241-#246.

  2. Code fix in 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 /g makes
    escaping global. Closes #234 and #235.

Validation

  • python3 scripts/verify-suppressions.py — PASS
  • python3 scripts/verify-triage-inventory.py — PASS (79 rows; 9 false-positive, 70 fix)
  • python3 scripts/verify-valid-fixes.py — PASS

JDK 1.8.0 compatible. No CHANGELOG.md entry per AGENTS.md.

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
natechadwick-intsof force-pushed the codeql/js-incomplete-sanitization branch from eb0a715 to a6ab54f Compare August 15, 2026 21:07
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
natechadwick-intsof force-pushed the codeql/js-incomplete-sanitization branch from a6ab54f to 0202d20 Compare August 15, 2026 21:08
@natechadwick-intsof
natechadwick-intsof merged commit f6e14aa into main Aug 15, 2026
2 checks passed
@natechadwick-intsof
natechadwick-intsof deleted the codeql/js-incomplete-sanitization branch August 15, 2026 21:09
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.
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