Skip to content

fix: escape quotes in notebook viewer esc(); strip nested Java generics (CodeQL) - #178

Merged
AkashGoenka merged 2 commits into
mainfrom
fix/codeql-esc-attrs-and-java-generics
Sep 29, 2026
Merged

AkashGoenka merged 2 commits into
mainfrom
fix/codeql-esc-attrs-and-java-generics

Conversation

@AkashGoenka

Copy link
Copy Markdown
Owner

Addresses the open CodeQL alerts. I read the code behind all 13; two root causes are worth fixing, two are false positives.

Fixed here

Alerts Fix
#12–20 js/incomplete-html-attribute-sanitization esc() in src/kb/view-template.ts escaped only & < > but feeds double-quoted attributes (data-dir="${esc(path)}"). A file path or note title containing " could close the attribute. Now also escapes " and '. Site copy site-astro/src/notebook-page.html regenerated (CodeQL scans that copy).
#3 js/incomplete-multi-character-sanitization (java.ts) False positive as a security issue (Java type names, never rendered), but a real bug: stripGenerics('Map<String, List<String>>') returned Map>. Now strips innermost <...> repeatedly, so Outer<T>.Inner is still Outer.Inner.
#5 js/bad-tag-filter (parser.ts) </script > (space before >) is a valid end tag; now matched.

Java resolution, before -> after (scripts/probe-repos.mjs, Java repos only)

repo edges java resolved java ratio
kafka 48,881 -> 49,034 (+153) 48,376 -> 48,529 49.7% -> 50.9%
jmri 34,229 -> 34,230 (+1) 34,212 -> 34,213 37.7% -> 37.8%
  • Consistent with the mechanism: kafka has ~314 extends/implements lines with nested generics, jmri has 4.
  • Most of the ratio movement is the denominator: unresolved fell by 2,116 (kafka) and 285 (jmri) because corrupted names like Map> no longer count as unresolved. The real new edges are the +153 / +1.
  • Not decomposed: the exact split of that unresolved drop between extends/implements and field/param types (both use stripGenerics).
  • Counts only, not an edge-by-edge diff. No count went down, and the change can only turn a name that could never match into one that can.

Not fixed (to dismiss on GitHub after review)

  • #4 (parser.ts): extracts <script> bodies for the parser, never rendered. False positive.
  • #6 (tools.ts --match /regex/): user-supplied regex is the documented feature; worst case is a hung local process from a pathological pattern. Won't fix.

Notes

  • The site page regeneration also refreshes the baked notebook data (135 -> 139 notes); that is the repo's own notebook, not new content.
  • Tests: 3 new tests fail on the old code and pass on the new (stripGenerics nested, extends/implements names, esc quotes). Full suite 873/873, tsc clean.

🤖 Generated with Claude Code

…generics (CodeQL)

- kb view template: esc() escaped only & < >, but feeds double-quoted attributes
  (data-dir="${esc(path)}"), so a path or note title containing a quote could
  close the attribute. Now also escapes " and '. Site copy regenerated.
  Clears code-scanning alerts #12-20 (js/incomplete-html-attribute-sanitization).
- java.ts stripGenerics: one `<[^>]*>` pass turned `Map<String, List<String>>`
  into `Map>`, a name that matches no class. Strip innermost `<...>` repeatedly.
  Not a security issue (alert #3 is a false positive) but a real resolution bug.
- parser.ts extractSfcScripts: accept `</script >` (space before `>`), alert #5.

Java probe (scripts/probe-repos.mjs), before -> after:
  kafka: edges 48881 -> 49034 (+153), java resolved 48376 -> 48529,
         java ratio 49.7% -> 50.9%
  jmri:  edges 34229 -> 34230 (+1),   java resolved 34212 -> 34213,
         java ratio 37.7% -> 37.8%

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Comment thread src/indexer/parser.ts Fixed
…js/bad-tag-filter)

HTML parsers allow whitespace and junk attributes in an end tag, so
</script\t\n bar> closes a block. The end-tag regex now matches that too.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@AkashGoenka
AkashGoenka merged commit ebc0769 into main Sep 29, 2026
9 of 10 checks passed
@AkashGoenka
AkashGoenka deleted the fix/codeql-esc-attrs-and-java-generics branch September 29, 2026 17:11
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