From 0cd4c8ba43915c76b0a1e3c7a142db04c78e1eff Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Thu, 19 Feb 2026 12:59:22 +0000 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=A4=96=20fix:=20harden=20renderer=20H?= =?UTF-8?q?TML=20sinks=20against=20XSS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace attacker-controlled filename HTML rendering with safe React nodes, harden Mermaid SVG rendering with strict mode + sanitization, and document renderer XSS guardrails in AGENTS. --- _Generated with `mux` • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$0.01`_ --- docs/AGENTS.md | 7 ++ .../components/Messages/CodeBlockSSR.tsx | 1 + .../Messages/Mermaid.sanitization.test.ts | 33 +++++++ src/browser/components/Messages/Mermaid.tsx | 59 ++++++++++++- .../RightSidebar/CodeReview/HunkViewer.tsx | 85 +++++++++++++++---- .../components/shared/DiffRenderer.tsx | 4 + 6 files changed, 169 insertions(+), 20 deletions(-) create mode 100644 src/browser/components/Messages/Mermaid.sanitization.test.ts diff --git a/docs/AGENTS.md b/docs/AGENTS.md index e1b6a39f21..f602ac5c1c 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -108,6 +108,13 @@ HistoryService is pure local disk I/O with a single dependency (`getSessionDir`) - If a new emoji appears in tool output, extend `EmojiIcon` to map it to an SVG icon. - Colors defined in `src/browser/styles/globals.css` (`:root @theme` block). Reference via CSS variables (e.g., `var(--color-plan-mode)`), never hardcode hex values. +## Security: Renderer HTML & XSS + +- Treat repo-controlled strings (file paths, diff content, branch names, commit messages) as attacker-controlled input. +- Never render attacker-controlled data through `dangerouslySetInnerHTML`, `innerHTML`, `outerHTML`, or `insertAdjacentHTML`. +- Prefer React element trees for highlighting (split + `` nodes) so React escaping stays in effect. +- If raw HTML/SVG rendering is unavoidable (e.g., Shiki/Mermaid), require explicit sanitization/hardening and document the trust boundary with a `SECURITY AUDIT` comment at the sink. + ## TypeScript Discipline - Ban `as any`; rely on discriminated unions, type guards, or authored interfaces. diff --git a/src/browser/components/Messages/CodeBlockSSR.tsx b/src/browser/components/Messages/CodeBlockSSR.tsx index 5f3a7ff951..6a71aa8641 100644 --- a/src/browser/components/Messages/CodeBlockSSR.tsx +++ b/src/browser/components/Messages/CodeBlockSSR.tsx @@ -19,6 +19,7 @@ export function CodeBlockSSR({ code, highlightedLines }: CodeBlockSSRProps) { {highlightedLines.map((lineHtml, idx) => (
{idx + 1}
+ {/* SECURITY AUDIT: lineHtml is pre-tokenized Shiki output from docs generation. */}
))} diff --git a/src/browser/components/Messages/Mermaid.sanitization.test.ts b/src/browser/components/Messages/Mermaid.sanitization.test.ts new file mode 100644 index 0000000000..8a6210869e --- /dev/null +++ b/src/browser/components/Messages/Mermaid.sanitization.test.ts @@ -0,0 +1,33 @@ +import { sanitizeMermaidSvg } from "./Mermaid"; + +import { Window } from "happy-dom"; + +const testWindow = new Window(); +globalThis.DOMParser = testWindow.DOMParser as unknown as typeof DOMParser; + +describe("sanitizeMermaidSvg", () => { + it("returns null for malformed SVG", () => { + expect(sanitizeMermaidSvg(" { + const input = + '' + + "" + + "
unsafe
" + + 'link' + + '' + + ""; + + const sanitized = sanitizeMermaidSvg(input); + + expect(sanitized).not.toBeNull(); + expect(sanitized).not.toContain(" { + node.remove(); + }); + + const linkAttributes = new Set(["href", "xlink:href"]); + doc.querySelectorAll("*").forEach((element) => { + for (const attribute of Array.from(element.attributes)) { + const attributeName = attribute.name.toLowerCase(); + const attributeValue = attribute.value.trim().toLowerCase(); + + if (attributeName.startsWith("on")) { + element.removeAttribute(attribute.name); + continue; + } + + if ( + linkAttributes.has(attributeName) && + (attributeValue.startsWith("javascript:") || attributeValue.startsWith("data:text/html")) + ) { + element.removeAttribute(attribute.name); + } + } + }); + + return svgRoot.outerHTML; +} + // Common button styles const getButtonStyle = (disabled = false): CSSProperties => ({ background: disabled ? "rgba(255, 255, 255, 0.05)" : "rgba(255, 255, 255, 0.1)", @@ -162,11 +205,18 @@ export const Mermaid: React.FC<{ chart: string }> = ({ chart }) => { const { svg: renderedSvg } = await mermaid.render(id, debouncedChart); if (cancelled) return; - lastValidSvgRef.current = renderedSvg; - setSvg(renderedSvg); + const sanitizedSvg = sanitizeMermaidSvg(renderedSvg); + if (!sanitizedSvg) { + throw new Error("Mermaid returned invalid SVG output"); + } + + lastValidSvgRef.current = sanitizedSvg; + setSvg(sanitizedSvg); setError(null); if (containerRef.current) { - containerRef.current.innerHTML = renderedSvg; + // SECURITY AUDIT: sanitizedSvg is produced by sanitizeMermaidSvg(), + // which strips active SVG/HTML content before insertion. + containerRef.current.innerHTML = sanitizedSvg; } } catch (err) { if (cancelled) return; @@ -189,6 +239,7 @@ export const Mermaid: React.FC<{ chart: string }> = ({ chart }) => { // Update modal container when opened useEffect(() => { if (isModalOpen && modalContainerRef.current && svg) { + // SECURITY AUDIT: svg state only stores sanitizeMermaidSvg() output. modalContainerRef.current.innerHTML = svg; } }, [isModalOpen, svg]); diff --git a/src/browser/components/RightSidebar/CodeReview/HunkViewer.tsx b/src/browser/components/RightSidebar/CodeReview/HunkViewer.tsx index 643c6758ad..6832c65dc8 100644 --- a/src/browser/components/RightSidebar/CodeReview/HunkViewer.tsx +++ b/src/browser/components/RightSidebar/CodeReview/HunkViewer.tsx @@ -8,10 +8,7 @@ import { Check, Circle } from "lucide-react"; import type { DiffHunk, Review, ReviewNoteData } from "@/common/types/review"; import { SelectableDiffRenderer } from "../../shared/DiffRenderer"; import type { ReviewActionCallbacks } from "../../shared/InlineReviewNote"; -import { - type SearchHighlightConfig, - highlightSearchInText, -} from "@/browser/utils/highlighting/highlightSearchTerms"; +import { type SearchHighlightConfig } from "@/browser/utils/highlighting/highlightSearchTerms"; import { Tooltip, TooltipTrigger, TooltipContent } from "../../ui/tooltip"; import { usePersistedState } from "@/browser/hooks/usePersistedState"; import { getReviewExpandStateKey } from "@/common/constants/storage"; @@ -48,6 +45,67 @@ interface HunkViewerProps { onOpenFile?: (relativePath: string) => void; } +function escapeRegexForHighlight(term: string): string { + return term.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +function renderHighlightedFilePath( + filePath: string, + searchConfig?: SearchHighlightConfig +): React.ReactNode { + if (!searchConfig?.searchTerm.trim()) { + return filePath; + } + + const flags = searchConfig.matchCase ? "g" : "gi"; + let pattern: RegExp; + try { + pattern = searchConfig.useRegex + ? new RegExp(searchConfig.searchTerm, flags) + : new RegExp(escapeRegexForHighlight(searchConfig.searchTerm), flags); + } catch { + return filePath; + } + + const highlightedSegments: React.ReactNode[] = []; + let lastIndex = 0; + pattern.lastIndex = 0; + + let match: RegExpExecArray | null; + while ((match = pattern.exec(filePath)) !== null) { + if (match.index > lastIndex) { + highlightedSegments.push(filePath.slice(lastIndex, match.index)); + } + + highlightedSegments.push( + + {match[0]} + + ); + + lastIndex = match.index + match[0].length; + + // Prevent infinite loops when matching zero-length regex patterns + if (match[0].length === 0) { + pattern.lastIndex++; + } + } + + if (highlightedSegments.length === 0) { + return filePath; + } + + if (lastIndex < filePath.length) { + highlightedSegments.push(filePath.slice(lastIndex)); + } + + return highlightedSegments; +} + export const HunkViewer = React.memo( ({ hunk, @@ -124,13 +182,11 @@ export const HunkViewer = React.memo( }; }, [hunk.content]); - // Highlight filePath if search is active - const highlightedFilePath = useMemo(() => { - if (!searchConfig) { - return hunk.filePath; - } - return highlightSearchInText(hunk.filePath, searchConfig); - }, [hunk.filePath, searchConfig]); + // Keep file path highlighting in React nodes so file names are always escaped. + const highlightedFilePath = useMemo( + () => renderHighlightedFilePath(hunk.filePath, searchConfig), + [hunk.filePath, searchConfig] + ); const handleOpenFile = React.useCallback( (event: React.MouseEvent) => { @@ -288,13 +344,10 @@ export const HunkViewer = React.memo( title={hunk.filePath} aria-label={`Open ${hunk.filePath} in new tab`} > - + {highlightedFilePath} ) : ( -
+
{highlightedFilePath}
)}
{!isPureRename && ( diff --git a/src/browser/components/shared/DiffRenderer.tsx b/src/browser/components/shared/DiffRenderer.tsx index 30c3ba9e6b..b0364cf3f4 100644 --- a/src/browser/components/shared/DiffRenderer.tsx +++ b/src/browser/components/shared/DiffRenderer.tsx @@ -607,6 +607,8 @@ export const DiffRenderer: React.FC = ({ lineNumberWidths={lineNumberWidths} /> + {/* SECURITY AUDIT: line.html comes from Shiki token output or escapeHtml fallback. + User/repo text is escaped before insertion and search highlighting only wraps text nodes. */} ( ) } /> + {/* SECURITY AUDIT: lineInfo.html is derived from Shiki/escapeHtml output + (optionally transformed by text-node-only search highlighting). */} Date: Thu, 19 Feb 2026 13:09:58 +0000 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=A4=96=20fix:=20preserve=20Mermaid=20?= =?UTF-8?q?foreignObject=20labels=20during=20sanitization?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep Mermaid label rendering intact by preserving foreignObject nodes while continuing to strip active content and dangerous URL schemes. --- _Generated with `mux` • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$0.01`_ --- .../Messages/Mermaid.sanitization.test.ts | 8 +++++--- src/browser/components/Messages/Mermaid.tsx | 13 ++++++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/browser/components/Messages/Mermaid.sanitization.test.ts b/src/browser/components/Messages/Mermaid.sanitization.test.ts index 8a6210869e..c7cb295f23 100644 --- a/src/browser/components/Messages/Mermaid.sanitization.test.ts +++ b/src/browser/components/Messages/Mermaid.sanitization.test.ts @@ -10,12 +10,13 @@ describe("sanitizeMermaidSvg", () => { expect(sanitizeMermaidSvg(" { + it("removes active content/unsafe attributes while preserving foreignObject labels", () => { const input = '' + "" + - "
unsafe
" + + '
label
' + 'link' + + '' + '' + ""; @@ -23,11 +24,12 @@ describe("sanitizeMermaidSvg", () => { expect(sanitized).not.toBeNull(); expect(sanitized).not.toContain(" { + // Defense in depth: remove active content containers and sanitize remaining attributes. + // NOTE: Keep because Mermaid uses it for wrapped node labels. + doc.querySelectorAll("script,iframe,object,embed").forEach((node) => { node.remove(); }); - const linkAttributes = new Set(["href", "xlink:href"]); + const urlAttributes = new Set(["href", "xlink:href", "src", "action", "formaction"]); + const blockedSchemes = ["javascript:", "vbscript:", "data:text/html"]; + doc.querySelectorAll("*").forEach((element) => { for (const attribute of Array.from(element.attributes)) { const attributeName = attribute.name.toLowerCase(); @@ -60,8 +63,8 @@ export function sanitizeMermaidSvg(svg: string): string | null { } if ( - linkAttributes.has(attributeName) && - (attributeValue.startsWith("javascript:") || attributeValue.startsWith("data:text/html")) + urlAttributes.has(attributeName) && + blockedSchemes.some((scheme) => attributeValue.startsWith(scheme)) ) { element.removeAttribute(attribute.name); } From 442e5e4b534f65cffa3de320e671f5da539dcd84 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Thu, 19 Feb 2026 13:26:58 +0000 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=A4=96=20fix:=20canonicalize=20encode?= =?UTF-8?q?d=20Mermaid=20URL=20schemes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decode encoded URL forms before blocked-scheme checks so obfuscated payloads like `java script:` and `java%0Ascript:` are stripped during Mermaid SVG sanitization. --- _Generated with `mux` • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$0.01`_ --- .../Messages/Mermaid.sanitization.test.ts | 3 ++ src/browser/components/Messages/Mermaid.tsx | 53 ++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/browser/components/Messages/Mermaid.sanitization.test.ts b/src/browser/components/Messages/Mermaid.sanitization.test.ts index c7cb295f23..d69c20607b 100644 --- a/src/browser/components/Messages/Mermaid.sanitization.test.ts +++ b/src/browser/components/Messages/Mermaid.sanitization.test.ts @@ -16,6 +16,7 @@ describe("sanitizeMermaidSvg", () => { "" + '
label
' + 'link' + + 'split-link' + '' + '' + ""; @@ -27,6 +28,8 @@ describe("sanitizeMermaidSvg", () => { expect(sanitized).not.toContain("onload="); expect(sanitized).not.toContain("onclick="); expect(sanitized).not.toContain("javascript:"); + expect(sanitized).not.toContain("java script:"); + expect(sanitized).not.toContain("xlink:href="); expect(sanitized).toContain(" = { + tab: "\t", + newline: "\n", + colon: ":", + nbsp: " ", + }; + + return named[entityBody.toLowerCase()] ?? null; +} + +function canonicalizeUrlForSchemeCheck(value: string): string { + // Normalize encoded and obfuscated schemes before comparison so payloads like + // java script:, java%0Ascript:, or java\nscript: reduce to javascript:. + let normalized = value.toLowerCase(); + + normalized = normalized.replace(/&([^;\s]+);?/g, (fullMatch, entityBody: string) => { + return decodeEntity(entityBody) ?? fullMatch; + }); + + normalized = normalized.replace(/%([0-9a-f]{2})/gi, (fullMatch, hex: string) => { + const parsed = Number.parseInt(hex, 16); + return Number.isNaN(parsed) ? fullMatch : String.fromCodePoint(parsed); + }); + + let canonical = ""; + for (const char of normalized) { + const charCode = char.charCodeAt(0); + const isAsciiControl = charCode <= 0x1f || charCode === 0x7f; + if (isAsciiControl || /\s/.test(char)) { + continue; + } + + canonical += char; + } + + return canonical; +} + export function sanitizeMermaidSvg(svg: string): string | null { const parser = new DOMParser(); const doc = parser.parseFromString(svg, "image/svg+xml"); @@ -55,7 +104,7 @@ export function sanitizeMermaidSvg(svg: string): string | null { doc.querySelectorAll("*").forEach((element) => { for (const attribute of Array.from(element.attributes)) { const attributeName = attribute.name.toLowerCase(); - const attributeValue = attribute.value.trim().toLowerCase(); + const canonicalUrlValue = canonicalizeUrlForSchemeCheck(attribute.value.trim()); if (attributeName.startsWith("on")) { element.removeAttribute(attribute.name); @@ -64,7 +113,7 @@ export function sanitizeMermaidSvg(svg: string): string | null { if ( urlAttributes.has(attributeName) && - blockedSchemes.some((scheme) => attributeValue.startsWith(scheme)) + blockedSchemes.some((scheme) => canonicalUrlValue.startsWith(scheme)) ) { element.removeAttribute(attribute.name); } From 851db89cd2499720fbc393fadcc658eb13db244e Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Thu, 19 Feb 2026 13:36:03 +0000 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=A4=96=20fix:=20handle=20invalid=20nu?= =?UTF-8?q?meric=20entities=20in=20Mermaid=20URL=20sanitizer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Guard numeric entity decoding against invalid Unicode code points so sanitizer canonicalization cannot throw on crafted values. --- _Generated with `mux` • Model: `openai:gpt-5.3-codex` • Thinking: `xhigh` • Cost: `$0.01`_ --- .../Messages/Mermaid.sanitization.test.ts | 3 +++ src/browser/components/Messages/Mermaid.tsx | 18 ++++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/browser/components/Messages/Mermaid.sanitization.test.ts b/src/browser/components/Messages/Mermaid.sanitization.test.ts index d69c20607b..8a227e1592 100644 --- a/src/browser/components/Messages/Mermaid.sanitization.test.ts +++ b/src/browser/components/Messages/Mermaid.sanitization.test.ts @@ -17,6 +17,7 @@ describe("sanitizeMermaidSvg", () => { '
label
' + 'link' + 'split-link' + + 'invalid-codepoint' + '' + '' + ""; @@ -29,7 +30,9 @@ describe("sanitizeMermaidSvg", () => { expect(sanitized).not.toContain("onclick="); expect(sanitized).not.toContain("javascript:"); expect(sanitized).not.toContain("java script:"); + expect(sanitized).not.toContain("�"); expect(sanitized).not.toContain("xlink:href="); + expect(sanitized).not.toContain("href="); expect(sanitized).toContain(" 0x10ffff) { + // Treat invalid numeric entities as empty content to avoid parser crashes + // and collapse obfuscated scheme separators. + return ""; + } + + try { + return String.fromCodePoint(codePoint); + } catch { + return ""; + } +} + function decodeEntity(entityBody: string): string | null { if (entityBody.startsWith("#x") || entityBody.startsWith("#X")) { const parsed = Number.parseInt(entityBody.slice(2), 16); - return Number.isNaN(parsed) ? null : String.fromCodePoint(parsed); + return Number.isNaN(parsed) ? "" : decodeNumericCodePoint(parsed); } if (entityBody.startsWith("#")) { const parsed = Number.parseInt(entityBody.slice(1), 10); - return Number.isNaN(parsed) ? null : String.fromCodePoint(parsed); + return Number.isNaN(parsed) ? "" : decodeNumericCodePoint(parsed); } const named: Record = {