fix(chat): download generated files without stale storage URLs

Author: icecrasher321

apps/sim/app/(interfaces)/chat/components/message/components/file-download.test.tsx

@@ -1,10 +1,14 @@
 /**
  * @vitest-environment jsdom
  */
+import { Blob as NodeBlob } from 'node:buffer'
 import { act } from 'react'
 import { createRoot } from 'react-dom/client'
-import { afterEach, describe, expect, it, vi } from 'vitest'
-import { ChatFileDownload } from '@/app/(interfaces)/chat/components/message/components/file-download'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import {
+  ChatFileDownload,
+  ChatFileDownloadAll,
+} from '@/app/(interfaces)/chat/components/message/components/file-download'
 import type { ChatFile } from '@/app/(interfaces)/chat/components/message/message'
 
 const imageFile: ChatFile = {
@@ -17,20 +21,47 @@ const imageFile: ChatFile = {
   base64: 'YWJj',
 }
 
+const fetchMock = vi.fn<typeof fetch>()
+const createObjectURL = vi.fn((_blob: Blob) => 'blob:download')
+const downloadedNames: string[] = []
+
+beforeEach(() => {
+  vi.clearAllMocks()
+  downloadedNames.length = 0
+  vi.stubGlobal('fetch', fetchMock)
+  vi.stubGlobal('Blob', NodeBlob)
+  vi.stubGlobal(
+    'URL',
+    class extends URL {
+      static createObjectURL = createObjectURL
+      static revokeObjectURL = vi.fn()
+    }
+  )
+  vi.spyOn(HTMLAnchorElement.prototype, 'click').mockImplementation(function () {
+    downloadedNames.push(this.download)
+  })
+  vi.spyOn(window, 'open').mockImplementation(() => null)
+})
+
 const mounts: Array<() => void> = []
 
-function renderFile(file: ChatFile): HTMLDivElement {
+function renderFile(file: ChatFile | ChatFile[]): HTMLDivElement {
   ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
   const container = document.createElement('div')
   const root = createRoot(container)
-  act(() => root.render(<ChatFileDownload file={file} />))
+  act(() =>
+    root.render(
+      Array.isArray(file) ? <ChatFileDownloadAll files={file} /> : <ChatFileDownload file={file} />
+    )
+  )
   mounts.push(() => act(() => root.unmount()))
   return container
 }
 
 afterEach(() => {
   while (mounts.length) mounts.pop()?.()
   vi.restoreAllMocks()
+  vi.unstubAllGlobals()
 })
 
 describe('ChatFileDownload', () => {
@@ -67,3 +98,102 @@ describe('ChatFileDownload', () => {
     expect(container.querySelector('button')?.textContent).toContain('report.pdf')
   })
 })
+
+async function clickDownload(container: HTMLDivElement): Promise<void> {
+  await act(async () => container.querySelector('button')!.click())
+}
+
+describe('chat file downloads', () => {
+  it('downloads exact inline bytes without fetching a data URL or needing a session', async () => {
+    fetchMock.mockRejectedValue(new TypeError('Blocked by connect-src'))
+    const container = renderFile({ ...imageFile, base64: 'AP9/gAE=' })
+    await clickDownload(container)
+    expect(fetchMock).not.toHaveBeenCalled()
+    const blob = createObjectURL.mock.calls[0]![0]
+    expect([...new Uint8Array(await blob.arrayBuffer())]).toEqual([0, 255, 127, 128, 1])
+    expect(blob.type).toBe('image/png')
+    expect(downloadedNames).toEqual(['generated.png'])
+    expect(window.open).not.toHaveBeenCalled()
+  })
+
+  it.each(['s3', 'blob', 'gcs', 'local'])(
+    'downloads stored %s files through the logs serve route instead of stale URLs',
+    async (provider) => {
+      fetchMock.mockResolvedValue(new Response('current stored bytes'))
+      const file = {
+        ...imageFile,
+        base64: undefined,
+        key: `execution/workspace/workflow/run/${provider}.png`,
+        url: 'https://files.example.com/expired?X-Amz-Expires=300',
+      }
+      await clickDownload(renderFile(file))
+      expect(fetchMock).toHaveBeenCalledExactlyOnceWith(
+        `/api/files/serve/${encodeURIComponent(file.key)}?context=execution`,
+        { cache: 'no-store' }
+      )
+      expect(await createObjectURL.mock.calls[0]![0].text()).toBe('current stored bytes')
+      expect(downloadedNames).toEqual(['generated.png'])
+    }
+  )
+
+  it('keeps external URL files on their existing URL path', async () => {
+    fetchMock.mockResolvedValue(new Response('external bytes'))
+    await clickDownload(renderFile({ ...imageFile, base64: undefined, key: 'url/external' }))
+    expect(fetchMock).toHaveBeenCalledExactlyOnceWith(imageFile.url, { cache: 'no-store' })
+  })
+
+  it('preserves delivered signed access for public visitors without a workspace session', async () => {
+    fetchMock
+      .mockResolvedValueOnce(new Response(null, { status: 401 }))
+      .mockResolvedValueOnce(new Response('publicly delivered bytes'))
+    await clickDownload(renderFile({ ...imageFile, base64: undefined }))
+    expect(fetchMock).toHaveBeenNthCalledWith(2, imageFile.url, { cache: 'no-store' })
+    expect(downloadedNames).toEqual(['generated.png'])
+  })
+
+  it('shows download errors without opening an expired storage error page', async () => {
+    fetchMock
+      .mockResolvedValueOnce(new Response(null, { status: 401 }))
+      .mockResolvedValueOnce(new Response('<Error>Request has expired</Error>', { status: 403 }))
+    const container = renderFile({ ...imageFile, base64: undefined })
+    await clickDownload(container)
+    expect(container.querySelector('[role="alert"]')?.textContent).toContain('Unable to download')
+    expect(downloadedNames).toEqual([])
+    expect(window.open).not.toHaveBeenCalled()
+    expect(container.querySelector('button')?.disabled).toBe(false)
+  })
+
+  it.each([403, 404])(
+    'does not retry denied or deleted stored files through their old URLs (%s)',
+    async (status) => {
+      fetchMock.mockResolvedValue(new Response(null, { status }))
+      await clickDownload(renderFile({ ...imageFile, base64: undefined }))
+      expect(fetchMock).toHaveBeenCalledTimes(1)
+      expect(downloadedNames).toEqual([])
+    }
+  )
+
+  it('uses the same inline and storage handling for download all', async () => {
+    fetchMock.mockResolvedValue(new Response('stored bytes'))
+    const stored = { ...imageFile, id: 'stored', name: 'stored.png', base64: undefined }
+    const container = renderFile([imageFile, stored])
+    await act(async () => {
+      container.querySelector('button')!.click()
+      await vi.waitFor(() => expect(downloadedNames).toEqual(['generated.png', 'stored.png']))
+    })
+    expect(fetchMock).toHaveBeenCalledTimes(1)
+  })
+})
+
+it('refuses unsafe external file URLs', async () => {
+  const container = renderFile({
+    ...imageFile,
+    base64: undefined,
+    key: 'url/external',
+    url: 'javascript:alert(1)',
+  })
+  await clickDownload(container)
+  expect(fetchMock).not.toHaveBeenCalled()
+  expect(window.open).not.toHaveBeenCalled()
+  expect(downloadedNames).toEqual([])
+})

apps/sim/app/(interfaces)/chat/components/message/components/file-download.tsx

@@ -7,6 +7,7 @@ import { createLogger } from '@sim/logger'
 import { sleep } from '@sim/utils/helpers'
 import { DefaultFileIcon, getDocumentIcon } from '@/components/icons/document-icons'
 import { isSafeHttpUrl } from '@/lib/core/utils/urls'
+import { saveBlob } from '@/lib/uploads/client/download'
 import type { ChatFile } from '@/app/(interfaces)/chat/components/message/message'
 
 const logger = createLogger('ChatFileDownload')
@@ -56,45 +57,58 @@ function getFileUrl(file: ChatFile): string {
   return `/api/files/serve/${encodeURIComponent(file.key)}?context=${file.context || 'execution'}`
 }
 
-async function triggerDownload(url: string, filename: string): Promise<void> {
-  const response = await fetch(url)
-  if (!response.ok) {
-    throw new Error(`Failed to fetch file: ${response.status} ${response.statusText}`)
+async function triggerDownload(file: ChatFile): Promise<void> {
+  if (file.base64) {
+    /** Decoding locally avoids a data-URL fetch, which connect-src does not allow. */
+    const decoded = atob(file.base64)
+    const bytes = new Uint8Array(decoded.length)
+    for (let index = 0; index < decoded.length; index++) {
+      bytes[index] = decoded.charCodeAt(index)
+    }
+    saveBlob(new Blob([bytes], { type: file.type }), file.name)
+    return
   }
 
-  const blob = await response.blob()
-  const blobUrl = URL.createObjectURL(blob)
-
-  const link = document.createElement('a')
-  link.href = blobUrl
-  link.download = filename
-  document.body.appendChild(link)
-  link.click()
-  document.body.removeChild(link)
+  const hasStorageKey = Boolean(file.key && !file.key.startsWith('url/'))
+  const url = hasStorageKey
+    ? `/api/files/serve/${encodeURIComponent(file.key)}?context=${encodeURIComponent(file.context || 'execution')}`
+    : isSafeHttpUrl(file.url)
+      ? file.url
+      : null
+  if (!url) throw new Error('File has no download URL')
+
+  /** The same serve route as execution logs resolves current storage access on each click. */
+  // boundary-raw-fetch: binary file download, including externally hosted file URLs
+  let response = await fetch(url, { cache: 'no-store' })
+  if (hasStorageKey && response.status === 401 && isSafeHttpUrl(file.url)) {
+    /** Public chat visitors may only have the file access already delivered in the response. */
+    response = await fetch(file.url, { cache: 'no-store' })
+  }
+  if (!response.ok) {
+    throw new Error('Unable to download this file. Please try again or request a new copy.')
+  }
 
-  URL.revokeObjectURL(blobUrl)
-  logger.info(`Downloaded: ${filename}`)
+  saveBlob(await response.blob(), file.name)
 }
 
 export function ChatFileDownload({ file }: ChatFileDownloadProps) {
   const [isDownloading, setIsDownloading] = useState(false)
+  const [downloadFailed, setDownloadFailed] = useState(false)
   const [failedPreviewUrl, setFailedPreviewUrl] = useState<string | null>(null)
   const fileUrl = getFileUrl(file)
 
   const handleDownload = async () => {
     if (isDownloading) return
 
     setIsDownloading(true)
+    setDownloadFailed(false)
 
     try {
       logger.info(`Initiating download for file: ${file.name}`)
-      const url = getFileUrl(file)
-      await triggerDownload(url, file.name)
+      await triggerDownload(file)
     } catch (error) {
       logger.error(`Failed to download file ${file.name}:`, error)
-      if (file.url && isSafeHttpUrl(file.url)) {
-        window.open(file.url, '_blank', 'noopener,noreferrer')
-      }
+      setDownloadFailed(true)
     } finally {
       setIsDownloading(false)
     }
@@ -142,6 +156,11 @@ export function ChatFileDownload({ file }: ChatFileDownloadProps) {
           )}
         </div>
       </Button>
+      {downloadFailed && (
+        <p role='alert' className='text-[var(--text-error)] text-xs'>
+          Unable to download this file. Please try again or request a new copy.
+        </p>
+      )}
     </div>
   )
 }
@@ -162,8 +181,7 @@ export function ChatFileDownloadAll({ files }: ChatFileDownloadAllProps) {
       for (let i = 0; i < files.length; i++) {
         const file = files[i]
         try {
-          const url = getFileUrl(file)
-          await triggerDownload(url, file.name)
+          await triggerDownload(file)
           logger.info(`Downloaded file ${i + 1}/${files.length}: ${file.name}`)
 
           if (i < files.length - 1) {

apps/sim/app/api/files/authorization.test.ts

@@ -491,3 +491,48 @@ describe('KB file live source authorization', () => {
     expect(get).not.toHaveBeenCalled()
   })
 })
+
+/** Execution downloads share the logs endpoint's current workspace permission check. */
+describe('execution file download authorization', () => {
+  const executionKey = 'execution/owner-workspace/workflow/run/image.png'
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('allows a current reader of the workspace named by the storage key', async () => {
+    mockGetUserEntityPermissions.mockResolvedValue('read')
+    await expect(verifyFileAccess(executionKey, USER_ID, undefined, 'execution')).resolves.toBe(
+      true
+    )
+    expect(mockGetUserEntityPermissions).toHaveBeenCalledExactlyOnceWith(
+      USER_ID,
+      'workspace',
+      'owner-workspace'
+    )
+  })
+
+  it('denies a caller without access to the file workspace', async () => {
+    mockGetUserEntityPermissions.mockResolvedValue(null)
+    await expect(verifyFileAccess(executionKey, USER_ID, undefined, 'execution')).resolves.toBe(
+      false
+    )
+  })
+
+  it('rechecks access after membership is revoked', async () => {
+    mockGetUserEntityPermissions.mockResolvedValueOnce('read').mockResolvedValueOnce(null)
+    await expect(verifyFileAccess(executionKey, USER_ID, undefined, 'execution')).resolves.toBe(
+      true
+    )
+    await expect(verifyFileAccess(executionKey, USER_ID, undefined, 'execution')).resolves.toBe(
+      false
+    )
+  })
+
+  it('denies a malformed execution key before looking up workspace access', async () => {
+    await expect(
+      verifyFileAccess('execution/image.png', USER_ID, undefined, 'execution')
+    ).resolves.toBe(false)
+    expect(mockGetUserEntityPermissions).not.toHaveBeenCalled()
+  })
+})

apps/sim/app/api/files/serve/[...path]/route.test.ts

@@ -200,6 +200,26 @@ describe('File Serve API Route', () => {
     })
   })
 
+  it('requires authentication for execution downloads before reading bytes', async () => {
+    mockResolveStoredFileContext.mockResolvedValue('execution')
+    hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({
+      success: false,
+      error: 'Unauthorized',
+    })
+    const response = await GET(
+      new NextRequest(
+        'http://localhost/api/files/serve/execution%2Fworkspace%2Fworkflow%2Frun%2Fimage.png?context=execution'
+      ),
+      {
+        params: Promise.resolve({ path: ['execution/workspace/workflow/run/image.png'] }),
+      }
+    )
+    expect(response.status).toBe(401)
+    expect(mockVerifyFileAccess).not.toHaveBeenCalled()
+    expect(mockReadFile).not.toHaveBeenCalled()
+    expect(storageServiceMockFns.mockDownloadFile).not.toHaveBeenCalled()
+  })
+
   it('bounds every buffered read at the shared transfer ceiling', async () => {
     mockIsUsingCloudStorage.mockReturnValue(true)
     mockResolveStoredFileContext.mockResolvedValue('copilot')