Skip to content

fix(app): escape mermaid content for the script element it lands in - #1836

Open
ethicnology-agent wants to merge 1 commit into
slopus:mainfrom
ethicnology-agent:fix/app-mermaid-script-element-escaping
Open

ethicnology-agent wants to merge 1 commit into
slopus:mainfrom
ethicnology-agent:fix/app-mermaid-script-element-escaping

Conversation

@ethicnology-agent

Copy link
Copy Markdown

Fixes the first defect reported in #1832.

Problem

MermaidRenderer.tsx:112 encodes the diagram body with JSON.stringify and line 157 interpolates the result into an inline <script> in the WebView document:

// Pass mermaid content via JSON to prevent XSS from HTML interpolation
const mermaidContent = JSON.stringify(props.content);
...
const content = ${mermaidContent};

JSON.stringify is a JavaScript-string encoder, not an HTML one. The HTML tokenizer ends a script element at the first case-insensitive </script, whatever JavaScript string state it appears in. A fence containing </script> therefore closes the element, and everything after it is parsed as markup and executed inside the WebView. The comment above the call states that this encoding prevents exactly that.

This content is not the user's own writing. It reaches the component from agent and tool output (MessageView.tsx:276), from artifacts, and from workstation files rendered in the app (FileViewPanel.tsx:419), via parseMarkdownBlock.ts:103. No CSP is set on the document.

What this changes

  • scriptEncoding.ts (new) — jsonForScriptElement, which escapes < so the closing sequence can never be produced. Mermaid receives an identical string, because < is < inside a JavaScript string literal. U+2028 and U+2029 are escaped alongside it, since JSON permits them raw where JavaScript historically does not.
  • MermaidRenderer.tsx — two lines: the import and the call.
  • scriptEncoding.test.ts — the property, asserted directly.

The encoder is its own module because the app's vitest project only collects sources/**/*.{spec,test}.ts, so a .tsx component cannot be covered. linkUtils.ts + linkUtils.test.ts in the same folder already follow this shape.

One of the cases asserts the defect itself rather than only the fix, so the reason this module exists stays visible:

it('is exactly what JSON.stringify alone fails to provide', () => {
    expect(JSON.stringify('</script>').toLowerCase()).toContain('</script');
});

tsc --noEmit is clean and sources/components/markdown/ passes 15/15.

What this deliberately does not change

  • The web and desktop path. It renders through dangerouslySetInnerHTML, but mermaid 11.12.2 defaults to securityLevel: "strict", which is in the non-overridable "secure" list, and render() runs DOMPurify before returning. I checked this before touching anything, and it is not affected.
  • The CDN script tag. The same WebView loads mermaid@11 from jsDelivr on a mutable major-version tag with no integrity attribute and no CSP. That is a separate defect, reported in App: a mermaid code fence escapes its inline <script> in the mobile WebView; session file RPCs follow symlinks out of the workspace #1832, and it needs a decision about bundling rather than a patch here.
  • The absent CSP on the WebView document. Worth adding, but it is a different change with its own blast radius.

MermaidRenderer encodes the diagram body with JSON.stringify and then
interpolates the result into an inline <script> in the WebView document.
JSON.stringify is a JavaScript-string encoder; the HTML tokenizer ends a
script element at the first case-insensitive `</script`, whatever the
JavaScript string state, so a fence containing that sequence closes the
element and the remainder is parsed as markup and executed. The comment
above the call claimed this encoding prevented exactly that.

The content is not the user's own writing: it reaches this component from
agent and tool output, from artifacts, and from workstation files rendered
in the app.

Escape `<` so the closing sequence can never be produced. The value mermaid
receives is unchanged, because `<` is `<` inside a JavaScript string
literal. U+2028 and U+2029 are escaped alongside it, since JSON permits them
raw where JavaScript historically does not.

The encoder lives in its own module because the app's vitest project only
collects .ts files, and this way the property is asserted directly rather
than inferred from the rendered template.

The web and desktop path is unaffected: it renders through mermaid's
non-overridable strict security level, which runs DOMPurify before returning.
@ethicnology-agent
ethicnology-agent force-pushed the fix/app-mermaid-script-element-escaping branch from 19550e2 to 0a408c7 Compare October 1, 2026 19:02
@ethicnology-agent

Copy link
Copy Markdown
Author

Rebased onto current main (4cf54d1). Clean rebase, diff unchanged.

This branch has not been deployed

No deployments
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