Repository navigation
fix(web): show math labels in Mermaid diagrams - #16198
NikitaMGrimm wants to merge 1 commit into
Conversation
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — The PR is a focused Mermaid rendering fix with targeted tests, but it broadens DOMPurify's handling of markup derived from untrusted diagram content. That sanitizer-policy change has security implications and warrants human review. You can add or adjust custom eligibility rules. Learn more. |
d1cf6bc to
f1cc8f7
Compare
|
Note Written by Hi! We are cleaning up open PRs, and this one does not say which model or harness was used to create it. If this change is really important, we recommend rebuilding the PR with a newer model and noting the model and harness in the PR description. |
|
Note Written by Reopening, this was closed by mistake. Sorry for the noise! |
Problem
Math in Mermaid diagram labels, such as
Note over F: $$f(x)=x^2+3x$$in a sequence diagram, renders as an empty box. Mermaid draws these labels as KaTeX MathML inside<foreignObject>, and our final DOMPurify pass has no MathML allowlist. DOMPurify then drops<math>and its children, text included.Change
The final sanitizer now accepts only the MathML that KaTeX emits, instead of DOMPurify's whole MathML profile:
math,mrow,mi,mfrac,msup,mtable, and so on).semanticsis still unwrapped with its content kept.annotation,annotation-xml,maction, andmglyphstay removed. Mermaid already stripsannotation, andmglyphis KaTeX's trusted-input image.mathvariant,stretchy,fence,columnalign, ...), accepted only on those MathML elements.href,xlink:href,src, andsrcsetstay forbidden everywhere, and script and event handlers are still removed.<math>is only accepted under the HTML label inside<foreignObject>, and each child must be in the MathML namespace.The expanded diagram is a standalone SVG image, so it now also carries the page text color the way it already carries the background. Otherwise, math in dark mode would render black on dark notes. That image is parsed as XML. KaTeX writes spaces in math (
\text{total cost},\quad) as no-break spaces, which HTML serialization outputs as , an entity XML does not define. So the sanitized SVG now uses instead, which keeps the expanded image from breaking on those labels.Mermaid's configuration is unchanged, including
htmlLabels: false. With that setting, flowchart labels still show$$...$$as plain text, so this PR does not change flowcharts.Scope and approval
This is a focused fix for an obvious bug: Mermaid already produces these labels, and our sanitizer erases them. It touches only the shared Mermaid renderer used by web and desktop (chat messages, plans, file and PR markdown previews). Mobile does not render Mermaid. No prior maintainer approval is claimed.
Verification
vp test run apps/web/src/components/chat/MermaidDiagram.test.tsx: 4 passed. Against main's sanitizer, the two retention tests fail and the guardrail test passes. The spaced-label test fails without the replacement. The tests run the real sanitizer on markup captured from Mermaid 11.17.2 and check that:href,onclick,src,maction,mglyph,annotation-xml,semantics, andannotationare removed;<math>placed directly in SVG is removed;tsc --noEmitforapps/web, plus targeted lint and format checks: clean.MermaidDiagramcomponent in an isolated Vite harness in the T3 Browser, light and dark. Main shows empty notes and an empty message label. With this change, the formulas render inline and in the expanded image. In Chrome, real Mermaid output for$$\text{total cost} = a\quad b$$parses as an SVG image after sanitizing.Native Electron and mobile were not run. The browser check used a standalone harness, not the full T3 app.
Note
🤖 Agent assistance: Opus 5.5 (claude-opus-5-5) via Claude Code