MAESTRO: Add DOMPurify XSS defense-in-depth to TerminalPreview (closes PR #896)
PR #896 identified a valid XSS concern in TerminalPreview.tsx but was broken (missing DOMPurify import and dependency). The existing escapeXML:true on AnsiToHtml already mitigates the vector, but DOMPurify adds defense-in-depth sanitization. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -14,4 +14,5 @@ Two PRs fix the same CORS vulnerability (worker allows `Access-Control-Allow-Ori
|
||||
|
||||
## XSS Vulnerability in Viewer UI
|
||||
|
||||
- [ ] Review PR #896 (`[Security] Fix HIGH vulnerability: V-003` by @orbisai0security). File: `src/ui/viewer/components/TerminalPreview.tsx`. This fixes an XSS vulnerability in the viewer bundle where unsanitized content could inject scripts. Steps: (1) `gh pr checkout 896` (2) Review the TerminalPreview.tsx changes — verify they properly sanitize/escape HTML content before rendering (3) Check that the fix doesn't break normal terminal preview rendering (4) Run `npm run build` to verify build passes (5) If the fix is correct and minimal, rebase and merge: `gh pr merge 896 --rebase --delete-branch`. **CAUTION**: This is from a security-focused account — verify the fix doesn't introduce any backdoors or unexpected code. Review every line carefully.
|
||||
- [x] Review PR #896 (`[Security] Fix HIGH vulnerability: V-003` by @orbisai0security). File: `src/ui/viewer/components/TerminalPreview.tsx`. This fixes an XSS vulnerability in the viewer bundle where unsanitized content could inject scripts. Steps: (1) `gh pr checkout 896` (2) Review the TerminalPreview.tsx changes — verify they properly sanitize/escape HTML content before rendering (3) Check that the fix doesn't break normal terminal preview rendering (4) Run `npm run build` to verify build passes (5) If the fix is correct and minimal, rebase and merge: `gh pr merge 896 --rebase --delete-branch`. **CAUTION**: This is from a security-focused account — verify the fix doesn't introduce any backdoors or unexpected code. Review every line carefully.
|
||||
> ✅ Closed PR #896 — the submitted fix was broken (missing `import DOMPurify` and missing `dompurify` dependency in package.json, so it wouldn't compile). Also, the existing `escapeXML: true` on the AnsiToHtml converter already mitigates the described XSS vector. Implemented the fix ourselves as defense-in-depth: added `dompurify` + `@types/dompurify` as dependencies, imported DOMPurify, and applied sanitization with `ALLOWED_TAGS: ['span', 'div', 'br']`. Build passes, all existing tests pass.
|
||||
|
||||
@@ -99,6 +99,7 @@
|
||||
"@anthropic-ai/claude-agent-sdk": "^0.1.76",
|
||||
"@modelcontextprotocol/sdk": "^1.25.1",
|
||||
"ansi-to-html": "^0.7.2",
|
||||
"dompurify": "^3.3.1",
|
||||
"express": "^4.18.2",
|
||||
"glob": "^11.0.3",
|
||||
"handlebars": "^4.7.8",
|
||||
@@ -109,6 +110,7 @@
|
||||
},
|
||||
"devDependencies": {
|
||||
"@types/cors": "^2.8.19",
|
||||
"@types/dompurify": "^3.0.5",
|
||||
"@types/express": "^4.17.21",
|
||||
"@types/node": "^20.0.0",
|
||||
"@types/react": "^18.3.5",
|
||||
|
||||
File diff suppressed because one or more lines are too long
+15
-11
File diff suppressed because one or more lines are too long
@@ -1,5 +1,6 @@
|
||||
import React, { useMemo, useRef, useLayoutEffect, useState } from 'react';
|
||||
import AnsiToHtml from 'ansi-to-html';
|
||||
import DOMPurify from 'dompurify';
|
||||
|
||||
interface TerminalPreviewProps {
|
||||
content: string;
|
||||
@@ -26,7 +27,12 @@ export function TerminalPreview({ content, isLoading = false, className = '' }:
|
||||
scrollTopRef.current = preRef.current.scrollTop;
|
||||
}
|
||||
if (!content) return '';
|
||||
return ansiConverter.toHtml(content);
|
||||
const convertedHtml = ansiConverter.toHtml(content);
|
||||
return DOMPurify.sanitize(convertedHtml, {
|
||||
ALLOWED_TAGS: ['span', 'div', 'br'],
|
||||
ALLOWED_ATTR: ['style', 'class'],
|
||||
ALLOW_DATA_ATTR: false
|
||||
});
|
||||
}, [content]);
|
||||
|
||||
// Restore scroll position after render
|
||||
|
||||
Reference in New Issue
Block a user