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 index b386babcad2..5928480b4b8 100644 --- 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 @@ -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,13 +21,39 @@ const imageFile: ChatFile = { base64: 'YWJj', } +const fetchMock = vi.fn() +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()) + act(() => + root.render( + Array.isArray(file) ? : + ) + ) mounts.push(() => act(() => root.unmount())) return container } @@ -31,6 +61,7 @@ function renderFile(file: ChatFile): HTMLDivElement { afterEach(() => { while (mounts.length) mounts.pop()?.() vi.restoreAllMocks() + vi.unstubAllGlobals() }) describe('ChatFileDownload', () => { @@ -67,3 +98,173 @@ describe('ChatFileDownload', () => { expect(container.querySelector('button')?.textContent).toContain('report.pdf') }) }) + +async function clickDownload(container: HTMLDivElement): Promise { + 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.each(['url/external', 'result-123', ''])( + 'keeps external URL files with key "%s" on their existing URL path', + async (key) => { + fetchMock.mockResolvedValue(new Response('external bytes')) + await clickDownload(renderFile({ ...imageFile, base64: undefined, key })) + 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.each([false, true])( + 'offers a safe browser download when an external host blocks CORS (stored=%s)', + async (stored) => { + if (stored) fetchMock.mockResolvedValueOnce(new Response(null, { status: 401 })) + fetchMock.mockRejectedValueOnce(new TypeError('Failed to fetch')) + const container = renderFile({ + ...imageFile, + base64: undefined, + key: stored ? imageFile.key : 'result-123', + }) + await clickDownload(container) + const link = container.querySelector('a')! + expect(link.href).toBe(imageFile.url) + expect(link.download).toBe(imageFile.name) + expect(link.rel).toBe('noopener noreferrer') + expect(link.target).toBe('_blank') + expect(window.open).not.toHaveBeenCalled() + } + ) + + it('cancels both discarded authentication and failed download response bodies', async () => { + const cancelAuthentication = vi.fn() + const cancelDownload = vi.fn() + fetchMock.mockResolvedValueOnce( + new Response(new ReadableStream({ cancel: cancelAuthentication }), { status: 401 }) + ) + fetchMock.mockResolvedValueOnce( + new Response(new ReadableStream({ cancel: cancelDownload }), { status: 403 }) + ) + const container = renderFile({ ...imageFile, base64: undefined }) + await clickDownload(container) + expect(cancelAuthentication).toHaveBeenCalledTimes(1) + expect(cancelDownload).toHaveBeenCalledTimes(1) + expect(container.querySelector('a')).toBeNull() + }) + + it('shows download errors without opening an expired storage error page', async () => { + fetchMock + .mockResolvedValueOnce(new Response(null, { status: 401 })) + .mockResolvedValueOnce(new Response('Request has expired', { 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 })) + const container = renderFile({ ...imageFile, base64: undefined }) + await clickDownload(container) + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(downloadedNames).toEqual([]) + expect(container.querySelector('a')).toBeNull() + } + ) + + 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('reports partial bulk failures, continues the batch, and clears the alert after a successful retry', async () => { + fetchMock.mockResolvedValue(new Response(null, { status: 403 })) + const stored = { ...imageFile, id: 'stored', name: 'stored.png', base64: undefined } + const container = renderFile([stored, imageFile]) + await clickDownload(container) + expect(downloadedNames).toEqual(['generated.png']) + expect(container.querySelector('[role="alert"]')?.textContent).toContain( + 'Unable to download 1 file' + ) + fetchMock.mockResolvedValue(new Response('stored bytes')) + await act(async () => { + container.querySelector('button')!.click() + await vi.waitFor(() => + expect(downloadedNames).toEqual(['generated.png', 'stored.png', 'generated.png']) + ) + }) + expect(container.querySelector('[role="alert"]')).toBeNull() + }) + + it('uses the recognized key context when metadata omits it', async () => { + fetchMock.mockResolvedValue(new Response('workspace bytes')) + await clickDownload( + renderFile({ ...imageFile, base64: undefined, key: 'workspace/id/file.png' }) + ) + expect(fetchMock).toHaveBeenCalledExactlyOnceWith( + '/api/files/serve/workspace%2Fid%2Ffile.png?context=workspace', + { cache: 'no-store' } + ) + }) +}) + +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([]) +}) 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 f9005b8b7e5..1da92cb8959 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 @@ -7,6 +7,8 @@ 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 { tryInferContextFromKey } from '@/lib/uploads/utils/file-utils' import type { ChatFile } from '@/app/(interfaces)/chat/components/message/message' const logger = createLogger('ChatFileDownload') @@ -19,6 +21,22 @@ interface ChatFileDownloadAllProps { files: ChatFile[] } +class DirectDownloadRequiredError extends Error { + constructor(readonly url: string) { + super('This file must be downloaded directly in the browser.') + } +} + +async function fetchExternalFile(url: string): Promise { + try { + // boundary-raw-fetch: external binary download from an already validated HTTP URL + return await fetch(url, { cache: 'no-store' }) + } catch { + /** A navigation can download external files whose hosts do not allow CORS reads. */ + throw new DirectDownloadRequiredError(url) + } +} + function formatFileSize(bytes: number): string { if (bytes === 0) return '0 B' const k = 1024 @@ -56,28 +74,51 @@ function getFileUrl(file: ChatFile): string { return `/api/files/serve/${encodeURIComponent(file.key)}?context=${file.context || 'execution'}` } -async function triggerDownload(url: string, filename: string): Promise { - 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 { + 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 storageContext = tryInferContextFromKey(file.key) + const hasStorageKey = storageContext !== null + const url = hasStorageKey + ? `/api/files/serve/${encodeURIComponent(file.key)}?context=${encodeURIComponent(storageContext)}` + : isSafeHttpUrl(file.url) + ? file.url + : null + if (!url) throw new Error('File has no download URL') - const link = document.createElement('a') - link.href = blobUrl - link.download = filename - document.body.appendChild(link) - link.click() - document.body.removeChild(link) + /** The same serve route as execution logs resolves current storage access on each click. */ + let response: Response + if (hasStorageKey) { + // boundary-raw-fetch: binary file download through the authorized serve route + response = await fetch(url, { cache: 'no-store' }) + } else { + response = await fetchExternalFile(url) + } + if (hasStorageKey && response.status === 401 && isSafeHttpUrl(file.url)) { + await response.body?.cancel() + /** Public chat visitors may only have the file access already delivered in the response. */ + response = await fetchExternalFile(file.url) + } + if (!response.ok) { + await response.body?.cancel() + 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 [downloadError, setDownloadError] = useState<{ directUrl?: string } | null>(null) const [failedPreviewUrl, setFailedPreviewUrl] = useState(null) const fileUrl = getFileUrl(file) @@ -85,16 +126,14 @@ export function ChatFileDownload({ file }: ChatFileDownloadProps) { if (isDownloading) return setIsDownloading(true) + setDownloadError(null) 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') - } + setDownloadError(error instanceof DirectDownloadRequiredError ? { directUrl: error.url } : {}) } finally { setIsDownloading(false) } @@ -142,12 +181,33 @@ export function ChatFileDownload({ file }: ChatFileDownloadProps) { )} + {downloadError && ( +

+ {downloadError.directUrl ? ( + <> + Unable to download automatically.{' '} + + Download directly + + + ) : ( + 'Unable to download this file. Please try again or request a new copy.' + )} +

+ )} ) } export function ChatFileDownloadAll({ files }: ChatFileDownloadAllProps) { const [isDownloading, setIsDownloading] = useState(false) + const [failedCount, setFailedCount] = useState(0) if (!files || files.length === 0) return null @@ -155,6 +215,8 @@ export function ChatFileDownloadAll({ files }: ChatFileDownloadAllProps) { if (isDownloading) return setIsDownloading(true) + setFailedCount(0) + let failures = 0 try { logger.info(`Initiating download for ${files.length} files`) @@ -162,8 +224,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) { @@ -171,25 +232,35 @@ export function ChatFileDownloadAll({ files }: ChatFileDownloadAllProps) { } } catch (error) { logger.error(`Failed to download file ${file.name}:`, error) + failures++ } } } finally { + setFailedCount(failures) setIsDownloading(false) } } return ( - + {failedCount > 0 && ( +

+ Unable to download {failedCount} {failedCount === 1 ? 'file' : 'files'}. Please try + downloading them individually. +

)} - + ) } diff --git a/apps/sim/app/api/files/authorization.test.ts b/apps/sim/app/api/files/authorization.test.ts index a8738342a8d..e9771609833 100644 --- a/apps/sim/app/api/files/authorization.test.ts +++ b/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() + }) +}) diff --git a/apps/sim/app/api/files/serve/[...path]/route.test.ts b/apps/sim/app/api/files/serve/[...path]/route.test.ts index e3937b9e78d..38c3cfcf73d 100644 --- a/apps/sim/app/api/files/serve/[...path]/route.test.ts +++ b/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')