From a75b2c77d4c38e8befa096ebd7c7ec2fd798faaf Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 31 Oct 2026 17:09:33 -0700 Subject: [PATCH] fix(editor): render formatted Mermaid labels safely (#25554) * fix(editor): render formatted Mermaid labels safely Enable Mermaid HTML labels everywhere the desktop renders diagrams and sanitize with foreignObject XHTML kept, form controls and SVG filters removed. * fix(editor): copy HTML-label diagrams as PNG Chromium taints a canvas drawn from a blob-URL SVG that holds foreignObject labels; the viewer now loads the diagram from a data URL. * test(editor): share one Chromium across the Mermaid sanitizer suite instead of per-test timeouts --- .../editor/DiagramViewerModal.test.tsx | 5 +- .../components/editor/DiagramViewerModal.tsx | 7 +- .../components/editor/MermaidBlock.test.tsx | 5 +- .../src/components/editor/MermaidBlock.tsx | 10 +- .../src/components/editor/MermaidViewer.tsx | 2 - .../editor/RichMarkdownCodeBlock.tsx | 3 +- .../components/editor/mermaid-config.test.ts | 2 +- .../src/components/editor/mermaid-config.ts | 4 +- .../editor/mermaid-sanitize.chromium.test.ts | 136 ++++++++++++++++++ .../src/components/editor/mermaid-sanitize.ts | 28 ++++ .../use-markdown-preview-components.tsx | 1 - .../sidebar/CommentMermaidBlock.tsx | 5 +- 12 files changed, 183 insertions(+), 25 deletions(-) create mode 100644 src/renderer/src/components/editor/mermaid-sanitize.chromium.test.ts create mode 100644 src/renderer/src/components/editor/mermaid-sanitize.ts diff --git a/src/renderer/src/components/editor/DiagramViewerModal.test.tsx b/src/renderer/src/components/editor/DiagramViewerModal.test.tsx index 530011dc85c..eeb6485ddb4 100644 --- a/src/renderer/src/components/editor/DiagramViewerModal.test.tsx +++ b/src/renderer/src/components/editor/DiagramViewerModal.test.tsx @@ -53,8 +53,6 @@ beforeEach(() => { } } ) - URL.createObjectURL = vi.fn(() => 'blob:diagram') - URL.revokeObjectURL = vi.fn() }) afterEach(() => { @@ -73,7 +71,7 @@ describe('DiagramViewerModal', () => { expect(screen.getByRole('dialog', { name: 'Diagram' })).toBeInTheDocument() sizeCanvas() const image = screen.getByRole('img', { name: 'Diagram' }) - expect(image).toHaveAttribute('src', 'blob:diagram') + expect(image.getAttribute('src')).toMatch(/^data:image\/svg\+xml/) expect(image.style.width).toBe('400px') expect(image.style.transform).toBe('translate(300px, 200px)') expect(screen.getByRole('button', { name: 'Fit to screen' })).toBeDisabled() @@ -81,7 +79,6 @@ describe('DiagramViewerModal', () => { await user.keyboard('{Escape}') await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) await waitFor(() => expect(document.activeElement).toBe(opener)) - expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:diagram') }) it('zooms from the toolbar, pans on drag, and fits again', async () => { diff --git a/src/renderer/src/components/editor/DiagramViewerModal.tsx b/src/renderer/src/components/editor/DiagramViewerModal.tsx index 05745877f9e..ff20b55eb60 100644 --- a/src/renderer/src/components/editor/DiagramViewerModal.tsx +++ b/src/renderer/src/components/editor/DiagramViewerModal.tsx @@ -3,7 +3,6 @@ import { translate } from '@/i18n/i18n' import { useAppStore } from '@/store' import type { PanZoomSize } from '../../../../shared/pan-zoom-view' import { PanZoomImageDialog } from './PanZoomImageDialog' -import { useSvgObjectUrl } from './use-svg-object-url' function readSvgImage(svg: string): { xml: string; size: PanZoomSize } | null { // HTML-serialized sanitized SVG can contain  , which XML image documents reject. @@ -27,7 +26,11 @@ export default function DiagramViewerModal(): React.JSX.Element { const svg = typeof modalData.svg === 'string' ? modalData.svg : '' const background = typeof modalData.background === 'string' ? modalData.background : undefined const image = useMemo(() => readSvgImage(svg), [svg]) - const url = useSvgObjectUrl(image?.xml ?? null) + // Chromium taints a canvas drawn from a blob-URL SVG with HTML labels, which breaks Copy as PNG; a data URL does not. + const url = useMemo( + () => (image ? `data:image/svg+xml;charset=utf-8,${encodeURIComponent(image.xml)}` : null), + [image] + ) return ( ({ initialize: vi.fn(), render: vi.fn<(id: string, content: string) => Promise<{ svg: string }>>(), - sanitize: vi.fn<(svg: string) => string>() + sanitize: vi.fn<(svg: string, config?: unknown) => string>() })) vi.mock('mermaid', () => ({ default: mermaid })) vi.mock('dompurify', () => ({ default: { sanitize: mermaid.sanitize } })) import MermaidBlock from './MermaidBlock' +import { mermaidSvgSanitizeConfig } from './mermaid-sanitize' const partial = 'flowchart TD\n A["Sync with main ' const complete = `${partial}once"]` @@ -133,7 +134,7 @@ describe('MermaidBlock rendering', () => { mermaid.sanitize.mockReturnValueOnce('Safe diagram') const { container } = render() await screen.findByText('Safe diagram') - expect(mermaid.sanitize).toHaveBeenCalledWith(unsafeSvg, { USE_PROFILES: { svg: true } }) + expect(mermaid.sanitize).toHaveBeenCalledWith(unsafeSvg, mermaidSvgSanitizeConfig) expect(container.querySelector('script')).toBeNull() expect(container.querySelector('svg')).not.toHaveAttribute('onload') }) diff --git a/src/renderer/src/components/editor/MermaidBlock.tsx b/src/renderer/src/components/editor/MermaidBlock.tsx index 006458bfaa4..cd7efa9e453 100644 --- a/src/renderer/src/components/editor/MermaidBlock.tsx +++ b/src/renderer/src/components/editor/MermaidBlock.tsx @@ -1,9 +1,9 @@ import React, { useEffect, useId, useLayoutEffect, useRef, useState } from 'react' import { Maximize2 } from 'lucide-react' import type mermaidNamespace from 'mermaid' -import DOMPurify from 'dompurify' import { readableMermaidFlowchartSvg } from '../../../../shared/mermaid-flowchart-contrast' import { getMermaidConfig } from './mermaid-config' +import { sanitizeMermaidSvg } from './mermaid-sanitize' import { readMermaidThemeColors } from './mermaid-theme-colors' import { translate } from '@/i18n/i18n' @@ -24,7 +24,6 @@ export type RenderedMermaidDiagram = { svg: string; background?: string } type MermaidBlockProps = { content: string isDark: boolean - htmlLabels?: boolean className?: string pendingContent?: React.ReactNode renderEnabled?: boolean @@ -49,7 +48,6 @@ function enqueueRender(fn: () => Promise): void { export default function MermaidBlock({ content, isDark, - htmlLabels = false, className, pendingContent, renderEnabled = true, @@ -81,11 +79,11 @@ export default function MermaidBlock({ } // Initialize inside the serialized task because global Mermaid config can otherwise change between initialization and render. const colors = readMermaidThemeColors() - mermaid.initialize(getMermaidConfig(isDark, htmlLabels, isDark ? colors : undefined)) + mermaid.initialize(getMermaidConfig(isDark, isDark ? colors : undefined)) const { svg } = await mermaid.render(`mermaid-${id}`, content) if (!cancelled) { // Sanitize explicitly against XSS even if Mermaid’s internal sanitization changes. - const sanitized = DOMPurify.sanitize(svg, { USE_PROFILES: { svg: true } }) + const sanitized = sanitizeMermaidSvg(svg) setRendered({ content, result: { @@ -121,7 +119,7 @@ export default function MermaidBlock({ return () => { cancelled = true } - }, [content, htmlLabels, isDark, id, renderEnabled]) + }, [content, isDark, id, renderEnabled]) useLayoutEffect(() => { if (renderEnabled && rendered?.content === content) { diff --git a/src/renderer/src/components/editor/MermaidViewer.tsx b/src/renderer/src/components/editor/MermaidViewer.tsx index 0a148e70021..f9f9d07ffad 100644 --- a/src/renderer/src/components/editor/MermaidViewer.tsx +++ b/src/renderer/src/components/editor/MermaidViewer.tsx @@ -105,11 +105,9 @@ export default function MermaidViewer({ return (
- {/* DOMPurify strips foreignObject HTML labels; request native SVG text so labels survive sanitization. */} diff --git a/src/renderer/src/components/editor/RichMarkdownCodeBlock.tsx b/src/renderer/src/components/editor/RichMarkdownCodeBlock.tsx index 0438d31c7ee..fa9a6cd5e5e 100644 --- a/src/renderer/src/components/editor/RichMarkdownCodeBlock.tsx +++ b/src/renderer/src/components/editor/RichMarkdownCodeBlock.tsx @@ -138,13 +138,12 @@ export function RichMarkdownCodeBlock({ )} as="pre" /> - {/* Keep Mermaid source editable; native SVG labels survive the preview’s sanitization. */} + {/* Keep Mermaid source editable; the diagram below is read-only output. */} {isMermaid && node.textContent.trim() && (
diff --git a/src/renderer/src/components/editor/mermaid-config.test.ts b/src/renderer/src/components/editor/mermaid-config.test.ts index 545053d91b0..298a95f63c7 100644 --- a/src/renderer/src/components/editor/mermaid-config.test.ts +++ b/src/renderer/src/components/editor/mermaid-config.test.ts @@ -31,7 +31,7 @@ function luminance(color: string | boolean | undefined): number { function resolvedDarkTheme(): NonNullable< ReturnType['themeVariables'] > { - mermaid.initialize(getMermaidConfig(true, false, darkColors)) + mermaid.initialize(getMermaidConfig(true, darkColors)) const variables = mermaid.mermaidAPI.getConfig().themeVariables if (!variables) { throw new Error('Mermaid did not resolve the theme') diff --git a/src/renderer/src/components/editor/mermaid-config.ts b/src/renderer/src/components/editor/mermaid-config.ts index 3e5a24dcbbb..c727ad087f2 100644 --- a/src/renderer/src/components/editor/mermaid-config.ts +++ b/src/renderer/src/components/editor/mermaid-config.ts @@ -11,7 +11,6 @@ export type MermaidThemeColors = { export function getMermaidConfig( isDark: boolean, - htmlLabels = false, colors?: MermaidThemeColors ): Parameters[0] { return { @@ -47,6 +46,7 @@ export function getMermaidConfig( } } : {}), - htmlLabels + // Strict mode sanitizes label HTML; sanitizeMermaidSvg keeps the foreignObject labels it emits. + htmlLabels: true } } diff --git a/src/renderer/src/components/editor/mermaid-sanitize.chromium.test.ts b/src/renderer/src/components/editor/mermaid-sanitize.chromium.test.ts new file mode 100644 index 00000000000..ae0cd1a8cc8 --- /dev/null +++ b/src/renderer/src/components/editor/mermaid-sanitize.chromium.test.ts @@ -0,0 +1,136 @@ +// Why: happy-dom does not exercise DOMPurify's Chromium namespace rules for +// foreignObject XHTML; this suite is the security regression gate for #12414. +// Unit CI does not install Playwright browsers — probe once and skip honestly +// (not a silent pass) when Chromium is unavailable. +import { createRequire } from 'node:module' +import { readFileSync } from 'node:fs' +import { chromium, type Browser } from '@playwright/test' +import { afterAll, describe, expect, it } from 'vitest' + +import { mermaidSvgSanitizeConfig } from './mermaid-sanitize' + +declare global { + var DOMPurify: { sanitize: (dirty: string, config: unknown) => string } | undefined +} + +const require = createRequire(import.meta.url) + +// One browser for the suite keeps each case well inside the default test timeout. +const browser: Browser | null = await chromium.launch({ headless: true }).catch(() => null) +const chromiumAvailable = browser !== null + +afterAll(async () => { + await browser?.close() +}) + +async function sanitizeInChromium(svg: string): Promise { + const purifyPath = require.resolve('dompurify/dist/purify.min.js') + const purifySrc = readFileSync(purifyPath, 'utf8') + if (!browser) { + throw new Error('Chromium is unavailable') + } + const page = await browser.newPage() + try { + await page.route('https://orca-mermaid.invalid/**', (route) => route.abort()) + await page.setContent('') + await page.addScriptTag({ content: purifySrc }) + return await page.evaluate( + ({ svg, cfg }) => { + if (!globalThis.DOMPurify) { + throw new Error('DOMPurify not loaded') + } + const clean = globalThis.DOMPurify.sanitize(svg, cfg) + const root = document.createElement('div') + document.body.append(root) + root.innerHTML = clean + for (const element of root.querySelectorAll('*')) { + for (const type of ['click', 'error', 'load', 'focus']) { + element.dispatchEvent(new Event(type)) + } + } + if (document.body.dataset.mermaidXss) { + throw new Error('A sanitized label executed an event handler') + } + return root.innerHTML + }, + { svg, cfg: mermaidSvgSanitizeConfig } + ) + } finally { + await page.close() + } +} + +describe.skipIf(!chromiumAvailable)('sanitizeMermaidSvg (Chromium)', () => { + it('keeps mermaid HTML label formatting tags inside foreignObject', async () => { + const svg = `
Bold and italic
` + const out = await sanitizeInChromium(svg) + expect(out).toContain('foreignObject') + expect(out).toMatch(/]/i) + expect(out).toMatch(/]/i) + expect(out).toContain('Bold') + }) + + it('strips script, event handlers, and javascript: URLs while keeping label text', async () => { + const svg = `
Boldx
` + const out = await sanitizeInChromium(svg) + expect(out).toContain('Bold') + expect(out).toMatch(/]/i) + expect(out.toLowerCase()).not.toContain(' { + const svg = `
Safe
` + const out = await sanitizeInChromium(svg) + expect(out).toContain('Safe') + expect(out).not.toMatch(/<(?:iframe|object|embed)\b|srcdoc|javascript:/i) + }) + + it('removes interactive form controls while preserving formatting and images', async () => { + const svg = `
Group
Result
SafeImage labelx
` + const out = await sanitizeInChromium(svg) + expect(out).not.toMatch( + /<(?:form|input|button|select|option|optgroup|textarea|datalist|fieldset|legend|label|output)\b/i + ) + expect(out).toContain('Safe') + expect(out).toContain(' { + const svg = `` + const out = await sanitizeInChromium(svg) + expect(out).toContain('Safe') + expect(out).toContain('first') + expect(out).not.toMatch(/\bonclick\s*=|\bonerror\s*=|\bhref\s*=/i) + }) + + it('keeps encoded markup as label text through browser reparsing', async () => { + const svg = `
<img src=x onerror="document.body.dataset.mermaidXss='encoded'">
` + const out = await sanitizeInChromium(svg) + expect(out).toContain('<img') + expect(out).not.toContain(' { + const svg = `
Safe
nested
` + const out = await sanitizeInChromium(svg) + expect(out).toContain('Safe') + expect(out).not.toMatch(/\bonerror\s*=|\bonclick\s*=| { + const svg = `
Safex2
` + const out = await sanitizeInChromium(svg) + expect(out).toContain('font-weight: 700') + expect(out).toContain('Safe') + expect(out).toContain('') + expect(out).not.toMatch(/ ) diff --git a/src/renderer/src/components/sidebar/CommentMermaidBlock.tsx b/src/renderer/src/components/sidebar/CommentMermaidBlock.tsx index a7ec09c259b..910fa5971d7 100644 --- a/src/renderer/src/components/sidebar/CommentMermaidBlock.tsx +++ b/src/renderer/src/components/sidebar/CommentMermaidBlock.tsx @@ -4,7 +4,7 @@ import { useMermaidDiagramExpand } from '@/components/editor/use-mermaid-diagram import { useDocumentDarkTheme } from '@/hooks/use-document-dark-theme' import { cn } from '@/lib/utils' -// Resolve live document theme here; disable HTML labels because sanitized SVG foreignObjects disappear on some platforms. +// Resolve live document theme here; comment markdown components have no theme access. export default function CommentMermaidBlock({ content, className, @@ -22,7 +22,7 @@ export default function CommentMermaidBlock({ if (pendingContent === undefined) { return (
- +
) } @@ -31,7 +31,6 @@ export default function CommentMermaidBlock({