diff --git a/frontend-modern/src/components/AI/__tests__/aiChatUtils.test.ts b/frontend-modern/src/components/AI/__tests__/aiChatUtils.test.ts index e96e89ae66..28b26d2f87 100644 --- a/frontend-modern/src/components/AI/__tests__/aiChatUtils.test.ts +++ b/frontend-modern/src/components/AI/__tests__/aiChatUtils.test.ts @@ -105,6 +105,42 @@ describe('aiChatUtils', () => { spy.mockRestore(); }); + + // Regression: drop `javascript:` href from LLM-supplied markdown links. + // Relies on the explicit ALLOWED_URI_REGEXP pin so the behaviour survives + // future DOMPurify upgrades that might relax internal scheme filtering. + it('strips javascript: hrefs from links', () => { + const output = utils.renderMarkdown('[click](javascript:alert(1))'); + expect(output).not.toContain('javascript:'); + expect(output).not.toContain('alert(1)'); + }); + + // Regression: data: URIs are not in the allowlist either; including SVG + // data URIs which some browsers will execute inline script from. + it('strips data: hrefs from links', () => { + const output = utils.renderMarkdown('[svg](data:image/svg+xml,)'); + expect(output).not.toContain('data:'); + }); + + // Regression: the LLM has no legitimate reason to apply arbitrary CSS + // classes — used to be ALLOWED_ATTR; allowing them opens a UI-redress + // surface (overlay attacks, hidden text, etc.). + it('strips class attribute from LLM-supplied markup', () => { + const output = utils.renderMarkdown( + '
overlay
', + ); + expect(output).not.toContain('class="fixed'); + expect(output).not.toContain('inset-0'); + }); + + // Regression: real http/https links still render and pick up the safe + // target/rel attributes from the afterSanitizeAttributes hook. + it('preserves https links and applies target/rel', () => { + const output = utils.renderMarkdown('[ok](https://example.com)'); + expect(output).toContain('href="https://example.com"'); + expect(output).toContain('target="_blank"'); + expect(output).toContain('rel="noopener noreferrer"'); + }); }); describe('Mention ID format contracts', () => { diff --git a/frontend-modern/src/components/AI/aiChatUtils.ts b/frontend-modern/src/components/AI/aiChatUtils.ts index da6bc964d1..e4c356c51e 100644 --- a/frontend-modern/src/components/AI/aiChatUtils.ts +++ b/frontend-modern/src/components/AI/aiChatUtils.ts @@ -18,7 +18,15 @@ export function groupModelsByProvider(models: ModelInfo[]): Map gfm: true, // GitHub Flavored Markdown @@ -59,7 +67,21 @@ export const renderMarkdown = (content: unknown): string => { try { configureDOMPurify(); const rawHtml = marked.parse(normalized) as string; - // Sanitize to prevent XSS from malicious LLM output or injected content + // Sanitize to prevent XSS from malicious LLM output or injected content. + // + // Hardening notes: + // - ALLOWED_URI_REGEXP pins href values to http/https/mailto. Without + // this we rely on DOMPurify's internal default URI scheme filter, + // which strips javascript:/data: in current versions but is library- + // version dependent; an explicit allowlist regex makes the intent + // survive future updates. + // - 'class' was previously in ALLOWED_ATTR. The LLM has no legitimate + // reason to apply utility classes, and allowing arbitrary classes + // opens a UI-redressing surface (e.g. injecting "fixed inset-0 z-50 + // bg-black" to overlay the page). Drop it. + // - 'target' and 'rel' are not in ALLOWED_ATTR because the + // afterSanitizeAttributes hook above sets them on every ; they + // don't need to be parsed in from the LLM-supplied HTML. return DOMPurify.sanitize(rawHtml, { // Allow common formatting tags but block scripts, iframes, etc. ALLOWED_TAGS: [ @@ -93,7 +115,8 @@ export const renderMarkdown = (content: unknown): string => { 'span', 'div', ], - ALLOWED_ATTR: ['href', 'target', 'rel', 'class'], + ALLOWED_ATTR: ['href'], + ALLOWED_URI_REGEXP: /^(?:https?:|mailto:)/i, // Force all links to open in new tab and prevent opener attacks ADD_ATTR: ['target', 'rel'], });