✅ TEST(#143): add categorical XSS guard across plugin renderers#151
Open
KyleKing wants to merge 5 commits into
Open
✅ TEST(#143): add categorical XSS guard across plugin renderers#151KyleKing wants to merge 5 commits into
KyleKing wants to merge 5 commits into
Conversation
…blebooks#143) Resolve the XSS advisories from executablebooks#143 where plugins interpolated attacker-controlled markdown into their default HTML render rules without escaping: - texmath: escape the math source in the default render() stub and the math_block_eqno equation number, across every delimiter flavor - dollarmath: escape the equation label in the id/href attributes (the content was already escaped) - myst_block: escape the target label in the <a> body and href Harden attrs/attrs_block: with no explicit allowed list, divert event-handler (on*) and style attributes to token.meta["insecure_attrs"] instead of emitting them, so the default configuration cannot inject script. The inline span path previously set token.attrs directly and bypassed the filter; route it through _add_attrs with the allowed set threaded in. Other attribute names are unchanged. Update the texmath/myst fixtures that encoded the old unescaped output, and add a categorical guard (tests/test_xss.py) that renders injection probes through each plugin to catch the next unescaped interpolation. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01L4mwfRr7ZkjiwPATm6Q4ue
Contributor
Author
|
FYI: @chrisjsewell, feel free to consider merging this change into the base PR (#150) to fully address all five findings and proactively catch them going forward |
KyleKing
force-pushed
the
kyle/xss-and-attrs-fixes-143
branch
from
July 21, 2026 16:56
e1e501a to
c977b60
Compare
KyleKing
commented
Jul 21, 2026
|
|
||
| def render(tex: str, displayMode: bool, macros: Any) -> str: | ||
| return tex | ||
| return escapeHtml(tex) |
Contributor
Author
There was a problem hiding this comment.
When the rendering logic below is implemented, we want the escaping to be owned here because doing so around render would break the valid HTML and reasonable to be owned by render
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
1 participant
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.
I started working on a fix locally, but it is the same net result with one addition a of categorical check to complement the per-plugin regression tests. The implementation follows the same pattern for test_redos in my other PR to proactively surface these types of security issues to PR authors rather than in code review