mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-23 04:06:21 +00:00
Tighten DOMPurify config in LLM markdown renderer
Two hardening adjustments to renderMarkdown in aiChatUtils.ts:
- Pin ALLOWED_URI_REGEXP to ^(?:https?:|mailto:). The previous
config left href scheme filtering to DOMPurify's library defaults,
which strips javascript:/data: in current versions but is version-
dependent. Making the allowlist explicit means the safety property
survives a future DOMPurify upgrade that loosens the internal
default.
- Drop 'class' from ALLOWED_ATTR. The LLM has no legitimate reason
to apply arbitrary CSS classes, and allowing them on sanitized
markup is a UI-redress surface: an attacker who can influence the
chat output (prompt injection, compromised model endpoint) could
inject something like <div class="fixed inset-0 z-50 bg-black">
to overlay the page or hide content.
Also expanded the marked.setOptions comment to document that the
sanitizer is the real XSS gate but marked options are kept tight as
defence-in-depth: a future DOMPurify-config loosening shouldn't mean
script tags from the LLM hit the DOM.
Regression tests added in aiChatUtils.test.ts:
- javascript: hrefs stripped
- data: hrefs stripped
- class attribute stripped from <div>
- https links preserved with target=_blank rel=noopener noreferrer
This commit is contained in:
@@ -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,<svg/>)');
|
||||
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(
|
||||
'<div class="fixed inset-0 z-50 bg-black">overlay</div>',
|
||||
);
|
||||
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', () => {
|
||||
|
||||
@@ -18,7 +18,15 @@ export function groupModelsByProvider(models: ModelInfo[]): Map<string, ModelInf
|
||||
return grouped;
|
||||
}
|
||||
|
||||
// Configure marked for safe rendering
|
||||
// Configure marked for safe rendering.
|
||||
//
|
||||
// DOMPurify runs downstream of marked.parse and is the actual XSS gate, but
|
||||
// we set marked options here to keep its output predictable rather than
|
||||
// relying solely on the downstream sanitizer. Raw HTML coming out of an LLM
|
||||
// would otherwise pass through marked unchanged (marked allows raw HTML by
|
||||
// default since v9) and rely on DOMPurify to clean it. Defence-in-depth: a
|
||||
// future DOMPurify-config edit that accidentally loosens the allowlist
|
||||
// shouldn't suddenly mean script tags from the LLM hit the DOM.
|
||||
marked.setOptions({
|
||||
breaks: true, // Convert \n to <br>
|
||||
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 <a>; 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'],
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user