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..8a227e1592 --- /dev/null +++ b/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(" { + const input = + '' + + "" + + '
label
' + + 'link' + + 'split-link' + + 'invalid-codepoint' + + '' + + '' + + ""; + + const sanitized = sanitizeMermaidSvg(input); + + expect(sanitized).not.toBeNull(); + expect(sanitized).not.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) ? "" : decodeNumericCodePoint(parsed); + } + + if (entityBody.startsWith("#")) { + const parsed = Number.parseInt(entityBody.slice(1), 10); + return Number.isNaN(parsed) ? "" : decodeNumericCodePoint(parsed); + } + + const named: Record = { + 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"); + + 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 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]); 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). */}