🤖 fix: harden renderer HTML sinks against XSS

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 + `<mark>` 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.

src/browser/components/Messages/CodeBlockSSR.tsx

@@ -19,6 +19,7 @@ export function CodeBlockSSR({ code, highlightedLines }: CodeBlockSSRProps) {
         {highlightedLines.map((lineHtml, idx) => (
           <React.Fragment key={idx}>
             <div className="line-number">{idx + 1}</div>
+            {/* SECURITY AUDIT: lineHtml is pre-tokenized Shiki output from docs generation. */}
             <div className="code-line" dangerouslySetInnerHTML={{ __html: lineHtml }} />
           </React.Fragment>
         ))}

src/browser/components/Messages/Mermaid.sanitization.test.ts

@@ -0,0 +1,41 @@
+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("<svg><g></svg")).toBeNull();
+  });
+
+  it("removes active content/unsafe attributes while preserving foreignObject labels", () => {
+    const input =
+      '<svg xmlns="http://www.w3.org/2000/svg" onload="alert(1)">' +
+      "<script>alert(1)</script>" +
+      '<foreignObject><div onclick="evil()">label</div></foreignObject>' +
+      '<a href="javascript:alert(1)"><text>link</text></a>' +
+      '<a xlink:href="java&#10;script:alert(3)"><text>split-link</text></a>' +
+      '<a href="java&#1114112;script:alert(4)"><text>invalid-codepoint</text></a>' +
+      '<image src="javascript:alert(2)" />' +
+      '<rect width="10" height="10" onclick="steal()" />' +
+      "</svg>";
+
+    const sanitized = sanitizeMermaidSvg(input);
+
+    expect(sanitized).not.toBeNull();
+    expect(sanitized).not.toContain("<script");
+    expect(sanitized).not.toContain("onload=");
+    expect(sanitized).not.toContain("onclick=");
+    expect(sanitized).not.toContain("javascript:");
+    expect(sanitized).not.toContain("java&#10;script:");
+    expect(sanitized).not.toContain("&#1114112;");
+    expect(sanitized).not.toContain("xlink:href=");
+    expect(sanitized).not.toContain("href=");
+    expect(sanitized).toContain("<svg");
+    expect(sanitized).toContain("<rect");
+    expect(sanitized).toContain("<foreignObject");
+    expect(sanitized).toContain("label");
+  });
+});

src/browser/components/Messages/Mermaid.tsx

@@ -12,7 +12,9 @@ mermaid.initialize({
   startOnLoad: false,
   theme: "dark",
   layout: "elk",
-  securityLevel: "loose",
+  // Security hardening: Mermaid content is untrusted (comes from markdown),
+  // so keep Mermaid's strict sanitization and disallow scriptable links.
+  securityLevel: "strict",
   fontFamily: "var(--font-monospace)",
   darkMode: true,
   elk: {
@@ -28,6 +30,113 @@ mermaid.initialize({
   },
 });
 
+function decodeNumericCodePoint(codePoint: number): string {
+  if (!Number.isInteger(codePoint) || codePoint < 0 || codePoint > 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) ? "" : decodeNumericCodePoint(parsed);
+  }
+
+  if (entityBody.startsWith("#")) {
+    const parsed = Number.parseInt(entityBody.slice(1), 10);
+    return Number.isNaN(parsed) ? "" : decodeNumericCodePoint(parsed);
+  }
+
+  const named: Record<string, string> = {
+    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&#10;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");
+
+  if (doc.querySelector("parsererror")) {
+    return null;
+  }
+
+  const svgRoot = doc.documentElement;
+  if (svgRoot.tagName.toLowerCase() !== "svg") {
+    return null;
+  }
+
+  // Defense in depth: remove active content containers and sanitize remaining attributes.
+  // NOTE: Keep <foreignObject> because Mermaid uses it for wrapped node labels.
+  doc.querySelectorAll("script,iframe,object,embed").forEach((node) => {
+    node.remove();
+  });
+
+  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();
+      const canonicalUrlValue = canonicalizeUrlForSchemeCheck(attribute.value.trim());
+
+      if (attributeName.startsWith("on")) {
+        element.removeAttribute(attribute.name);
+        continue;
+      }
+
+      if (
+        urlAttributes.has(attributeName) &&
+        blockedSchemes.some((scheme) => canonicalUrlValue.startsWith(scheme))
+      ) {
+        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 +271,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 +305,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]);

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(
+      <mark
+        // Keep filename highlighting safe: render React nodes instead of HTML strings
+        key={`file-path-match-${match.index}-${match[0]}-${highlightedSegments.length}`}
+        className="search-highlight"
+      >
+        {match[0]}
+      </mark>
+    );
+
+    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<HunkViewerProps>(
   ({
     hunk,
@@ -124,13 +182,11 @@ export const HunkViewer = React.memo<HunkViewerProps>(
       };
     }, [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<HTMLButtonElement>) => {
@@ -288,13 +344,10 @@ export const HunkViewer = React.memo<HunkViewerProps>(
               title={hunk.filePath}
               aria-label={`Open ${hunk.filePath} in new tab`}
             >
-              <span dangerouslySetInnerHTML={{ __html: highlightedFilePath }} />
+              <span>{highlightedFilePath}</span>
             </button>
           ) : (
-            <div
-              className="text-foreground min-w-0 truncate"
-              dangerouslySetInnerHTML={{ __html: highlightedFilePath }}
-            />
+            <div className="text-foreground min-w-0 truncate">{highlightedFilePath}</div>
           )}
           <div className="text-muted ml-auto flex shrink-0 items-center gap-1.5 whitespace-nowrap">
             {!isPureRename && (

src/browser/components/shared/DiffRenderer.tsx

@@ -607,6 +607,8 @@ export const DiffRenderer: React.FC<DiffRendererProps> = ({
                 lineNumberWidths={lineNumberWidths}
               />
               <DiffIndicator type={chunk.type} background={codeBg} />
+              {/* 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. */}
               <span
                 className="min-w-0 whitespace-pre [&_span:not(.search-highlight)]:!bg-transparent"
                 style={{
@@ -1243,6 +1245,8 @@ export const SelectableDiffRenderer = React.memo<SelectableDiffRendererProps>(
                     )
                   }
                 />
+                {/* SECURITY AUDIT: lineInfo.html is derived from Shiki/escapeHtml output
+                    (optionally transformed by text-node-only search highlighting). */}
                 <span
                   className="min-w-0 whitespace-pre [&_span:not(.search-highlight)]:!bg-transparent"
                   style={{