Repository navigation
Scanner hardening strategy: move SVG/MDX active-content scanning off per-bypass regex patching #1194
Description
Problem: the bypass class keeps recurring; per-bypass patching is losing
The import-pipeline content scanner (scripts/lib/svg-active-content.mjs, scripts/lib/architecture-content.mjs) accumulated roughly ten distinct bypass classes in one week of sec-check findings, each fixed by another regex:
- [sec-check] SVG active-content scanner is blind to payloads hidden in internal DTD entities, and every DOCTYPE-removal regex halves an internal subset #1005 internal DTD entities; [sec-check] SVG DOCTYPE strip fails open on an external identifier containing "[", leaving it in place or deleting live markup #1023 DOCTYPE external identifiers containing '['
- [sec-check] MDX active-content scanner bypass: multi-line inline code spans hide live MDX -> build-time code execution from issue submissions #1038 / [sec-check] findActiveContent misses live MDX hidden by a code span that crosses a block boundary #1121 MDX hidden by multi-line or block-crossing code spans
- [sec-check] CDATA-hidden </style> hides remote CSS fetches from findRemoteReferences #1043 CDATA-hidden
</style>; [sec-check] CSS escapes hide remote url()/@import targets from findRemoteReferences #1089 CSS escapes; [scanner] findRemoteReferences misses CSS comment- and escape-obfuscated url()/@import targets in style blocks #1091 CSS comments - [sec-check] remoteTarget() misses backslash authority prefixes, so an SVG can reference a remote host past findRemoteReferences() #1067 backslash authority prefixes; [sec-check] findRemoteReferences misses image-set() bare-string URLs, a spec-defined remote fetch with no url() token #1175
image-set()bare strings - [sec-check] activeScheme() only rejects two of the markup data: media types — data:application/xhtml+xml passes the SVG active-content gate #1167
data:markup media types; [sec-check] cleanMarkdown() leaves protocol-relative image references live, hot-linking every visitor to a third-party host #1180 protocol-relative image hosts
svg-active-content.mjs currently performs its analysis through 13 regex operations over text that is actually XML + CSS. Each finding above is the same root cause: regexes approximate grammars that have escaping, comments, and tokenizer states the regex cannot see. The historical rate (~10/week) predicts the next bypass; the per-bypass loop produces unbounded future work and a permanently uncertain gate.
Proposed direction (pick via discussion)
- Parse, do not pattern-match. Run SVG through a real XML parser (e.g.
@rgrove/parse-xmlorsax) and walk the element/attribute tree; tokenize embedded CSS with a real CSS tokenizer before looking for remote targets. Grammar-level analysis eliminates the escaping/comment/CDATA class wholesale. - Sanitize instead of scan-and-reject for the mirror path: project the SVG onto an element/attribute allowlist (DOMPurify-style, or rebuild from the parsed tree), so unknown constructs are dropped rather than needing to be anticipated.
- Defense in depth at serve time: the CSP currently ships only as a
<meta http-equiv>tag (docusaurus.config.js:148), which cannot cover everything response headers can on GitHub Pages — document this residual risk and the compensating controls in the content-security doc requested by [guide] Content-security rules enforced by architecture validators are documented nowhere — submitters hit 'active content'/'remote reference' CI failures with no doc to consult #1096.
Keep the existing regression test corpus (tests/svg-active-content.test.mjs) as the acceptance suite: every past bypass becomes a fixture the parser-based gate must still reject.
Completion criteria
- A decision (ADR or issue consensus) on parser-based scanning vs sanitization vs both.
- The chosen implementation passes the full historical-bypass fixture corpus.
- Residual risks documented alongside the submitter-facing rules from [guide] Content-security rules enforced by architecture validators are documented nowhere — submitters hit 'active content'/'remote reference' CI failures with no doc to consult #1096.
Refs #1096 (documentation counterpart).
Activity
mrbobbytables commentedon Oct 8, 2026
Two updates folding related work into this issue:
[scanner] svg-active-content.mjs: unterminated-string arms in #1176's two new CSS scanners have no unit coverage #1197 is consolidated here as acceptance fixtures. PR test: cover the CSS scanners' unterminated-string fallback arms #1200 lands the two unit tests it asked for (unterminated-string arms in the
image-set()walker andstripCssComments), closing [scanner] svg-active-content.mjs: unterminated-string arms in #1176's two new CSS scanners have no unit coverage #1197. Those cases now join the regression corpus intests/svg-active-content.test.mjsthat this issue designates as the acceptance suite — the parser/sanitizer replacement must still pass them. [scanner] svg-active-content.mjs: unterminated-string arms in #1176's two new CSS scanners have no unit coverage #1197 is also the latest data point for the trend this issue describes: every scanner hardening PR (fix(security): detect image-set() and comment-hidden CSS remote references #1176 most recently) grows the hand-rolled tokenizer surface and leaves new fallback arms to test, which is the per-bypass loop compounding.The documentation counterpart has landed. [guide] Content-security rules enforced by architecture validators are documented nowhere — submitters hit 'active content'/'remote reference' CI failures with no doc to consult #1096 is closed via docs: document the architecture content-security rules submitters hit in CI #1199 (
docs: document the architecture content-security rules submitters hit in CI), so the third completion criterion ('residual risks documented alongside the submitter-facing rules from [guide] Content-security rules enforced by architecture validators are documented nowhere — submitters hit 'active content'/'remote reference' CI failures with no doc to consult #1096') now has a concrete home: extend that doc rather than create one.Current state of the corpus for whoever implements the decision:
scripts/lib/svg-active-content.mjssits at 100% lines / 99.61% regions with only the documented-unreachable?? ''fallback uncovered, and the test file carries 121 passing cases including every historical bypass listed in this issue's body.mrbobbytables commentedon Oct 8, 2026
Maintainer direction recorded (@mrbobbytables): option 1 — parse, don't pattern-match — is the preferred approach for the scanner replacement.
Scope of that decision, to anchor whoever picks up the implementation:
scripts/lib/svg-active-content.mjswith grammar-level analysis: a real XML parser (e.g.@rgrove/parse-xmlorsax) walking the element/attribute tree, and a real CSS tokenizer for embedded style content before looking for remote targets. This eliminates the escaping/comment/CDATA bypass class wholesale rather than one regex at a time.tests/svg-active-content.test.mjs(121 cases, including every historical bypass listed in this issue plus the [scanner] svg-active-content.mjs: unterminated-string arms in #1176's two new CSS scanners have no unit coverage #1197 fixtures landed via test: cover the CSS scanners' unterminated-string fallback arms #1200) is the acceptance suite: the parser-based gate must reject every fixture the current scanner rejects.This settles the first completion criterion (direction decision via issue consensus). Remaining work is the implementation itself and the residual-risk documentation.
test: cover the CSS scanners' unterminated-string fallback arms (cncf…
fix(security): parser-based SVG/MDX scanning, replacing per-bypass re…