Skip to content

Commit 585e8f7

Browse files
fix(chat): download generated files without stale storage URLs (#7858)
* fix(chat): download generated files without stale storage URLs * fix(chat): preserve external downloads and report bulk failures * fix(chat): preserve direct downloads for CORS-restricted files * chore(chat): annotate binary download fetch boundaries
1 parent 3856d9e commit 585e8f7

4 files changed

Lines changed: 373 additions & 36 deletions

File tree

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

Lines changed: 205 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
/**
22
* @vitest-environment jsdom
33
*/
4+
import { Blob as NodeBlob } from 'node:buffer'
45
import { act } from 'react'
56
import { createRoot } from 'react-dom/client'
6-
import { afterEach, describe, expect, it, vi } from 'vitest'
7-
import { ChatFileDownload } from '@/app/(interfaces)/chat/components/message/components/file-download'
7+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
8+
import {
9+
ChatFileDownload,
10+
ChatFileDownloadAll,
11+
} from '@/app/(interfaces)/chat/components/message/components/file-download'
812
import type { ChatFile } from '@/app/(interfaces)/chat/components/message/message'
913

1014
const imageFile: ChatFile = {
@@ -17,20 +21,47 @@ const imageFile: ChatFile = {
1721
base64: 'YWJj',
1822
}
1923

24+
const fetchMock = vi.fn<typeof fetch>()
25+
const createObjectURL = vi.fn((_blob: Blob) => 'blob:download')
26+
const downloadedNames: string[] = []
27+
28+
beforeEach(() => {
29+
vi.clearAllMocks()
30+
downloadedNames.length = 0
31+
vi.stubGlobal('fetch', fetchMock)
32+
vi.stubGlobal('Blob', NodeBlob)
33+
vi.stubGlobal(
34+
'URL',
35+
class extends URL {
36+
static createObjectURL = createObjectURL
37+
static revokeObjectURL = vi.fn()
38+
}
39+
)
40+
vi.spyOn(HTMLAnchorElement.prototype, 'click').mockImplementation(function () {
41+
downloadedNames.push(this.download)
42+
})
43+
vi.spyOn(window, 'open').mockImplementation(() => null)
44+
})
45+
2046
const mounts: Array<() => void> = []
2147

22-
function renderFile(file: ChatFile): HTMLDivElement {
48+
function renderFile(file: ChatFile | ChatFile[]): HTMLDivElement {
2349
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
2450
const container = document.createElement('div')
2551
const root = createRoot(container)
26-
act(() => root.render(<ChatFileDownload file={file} />))
52+
act(() =>
53+
root.render(
54+
Array.isArray(file) ? <ChatFileDownloadAll files={file} /> : <ChatFileDownload file={file} />
55+
)
56+
)
2757
mounts.push(() => act(() => root.unmount()))
2858
return container
2959
}
3060

3161
afterEach(() => {
3262
while (mounts.length) mounts.pop()?.()
3363
vi.restoreAllMocks()
64+
vi.unstubAllGlobals()
3465
})
3566

3667
describe('ChatFileDownload', () => {
@@ -67,3 +98,173 @@ describe('ChatFileDownload', () => {
6798
expect(container.querySelector('button')?.textContent).toContain('report.pdf')
6899
})
69100
})
101+
102+
async function clickDownload(container: HTMLDivElement): Promise<void> {
103+
await act(async () => container.querySelector('button')!.click())
104+
}
105+
106+
describe('chat file downloads', () => {
107+
it('downloads exact inline bytes without fetching a data URL or needing a session', async () => {
108+
fetchMock.mockRejectedValue(new TypeError('Blocked by connect-src'))
109+
const container = renderFile({ ...imageFile, base64: 'AP9/gAE=' })
110+
await clickDownload(container)
111+
expect(fetchMock).not.toHaveBeenCalled()
112+
const blob = createObjectURL.mock.calls[0]![0]
113+
expect([...new Uint8Array(await blob.arrayBuffer())]).toEqual([0, 255, 127, 128, 1])
114+
expect(blob.type).toBe('image/png')
115+
expect(downloadedNames).toEqual(['generated.png'])
116+
expect(window.open).not.toHaveBeenCalled()
117+
})
118+
119+
it.each(['s3', 'blob', 'gcs', 'local'])(
120+
'downloads stored %s files through the logs serve route instead of stale URLs',
121+
async (provider) => {
122+
fetchMock.mockResolvedValue(new Response('current stored bytes'))
123+
const file = {
124+
...imageFile,
125+
base64: undefined,
126+
key: `execution/workspace/workflow/run/${provider}.png`,
127+
url: 'https://files.example.com/expired?X-Amz-Expires=300',
128+
}
129+
await clickDownload(renderFile(file))
130+
expect(fetchMock).toHaveBeenCalledExactlyOnceWith(
131+
`/api/files/serve/${encodeURIComponent(file.key)}?context=execution`,
132+
{ cache: 'no-store' }
133+
)
134+
expect(await createObjectURL.mock.calls[0]![0].text()).toBe('current stored bytes')
135+
expect(downloadedNames).toEqual(['generated.png'])
136+
}
137+
)
138+
139+
it.each(['url/external', 'result-123', ''])(
140+
'keeps external URL files with key "%s" on their existing URL path',
141+
async (key) => {
142+
fetchMock.mockResolvedValue(new Response('external bytes'))
143+
await clickDownload(renderFile({ ...imageFile, base64: undefined, key }))
144+
expect(fetchMock).toHaveBeenCalledExactlyOnceWith(imageFile.url, { cache: 'no-store' })
145+
}
146+
)
147+
148+
it('preserves delivered signed access for public visitors without a workspace session', async () => {
149+
fetchMock
150+
.mockResolvedValueOnce(new Response(null, { status: 401 }))
151+
.mockResolvedValueOnce(new Response('publicly delivered bytes'))
152+
await clickDownload(renderFile({ ...imageFile, base64: undefined }))
153+
expect(fetchMock).toHaveBeenNthCalledWith(2, imageFile.url, { cache: 'no-store' })
154+
expect(downloadedNames).toEqual(['generated.png'])
155+
})
156+
157+
it.each([false, true])(
158+
'offers a safe browser download when an external host blocks CORS (stored=%s)',
159+
async (stored) => {
160+
if (stored) fetchMock.mockResolvedValueOnce(new Response(null, { status: 401 }))
161+
fetchMock.mockRejectedValueOnce(new TypeError('Failed to fetch'))
162+
const container = renderFile({
163+
...imageFile,
164+
base64: undefined,
165+
key: stored ? imageFile.key : 'result-123',
166+
})
167+
await clickDownload(container)
168+
const link = container.querySelector('a')!
169+
expect(link.href).toBe(imageFile.url)
170+
expect(link.download).toBe(imageFile.name)
171+
expect(link.rel).toBe('noopener noreferrer')
172+
expect(link.target).toBe('_blank')
173+
expect(window.open).not.toHaveBeenCalled()
174+
}
175+
)
176+
177+
it('cancels both discarded authentication and failed download response bodies', async () => {
178+
const cancelAuthentication = vi.fn()
179+
const cancelDownload = vi.fn()
180+
fetchMock.mockResolvedValueOnce(
181+
new Response(new ReadableStream({ cancel: cancelAuthentication }), { status: 401 })
182+
)
183+
fetchMock.mockResolvedValueOnce(
184+
new Response(new ReadableStream({ cancel: cancelDownload }), { status: 403 })
185+
)
186+
const container = renderFile({ ...imageFile, base64: undefined })
187+
await clickDownload(container)
188+
expect(cancelAuthentication).toHaveBeenCalledTimes(1)
189+
expect(cancelDownload).toHaveBeenCalledTimes(1)
190+
expect(container.querySelector('a')).toBeNull()
191+
})
192+
193+
it('shows download errors without opening an expired storage error page', async () => {
194+
fetchMock
195+
.mockResolvedValueOnce(new Response(null, { status: 401 }))
196+
.mockResolvedValueOnce(new Response('<Error>Request has expired</Error>', { status: 403 }))
197+
const container = renderFile({ ...imageFile, base64: undefined })
198+
await clickDownload(container)
199+
expect(container.querySelector('[role="alert"]')?.textContent).toContain('Unable to download')
200+
expect(downloadedNames).toEqual([])
201+
expect(window.open).not.toHaveBeenCalled()
202+
expect(container.querySelector('button')?.disabled).toBe(false)
203+
})
204+
205+
it.each([403, 404])(
206+
'does not retry denied or deleted stored files through their old URLs (%s)',
207+
async (status) => {
208+
fetchMock.mockResolvedValue(new Response(null, { status }))
209+
const container = renderFile({ ...imageFile, base64: undefined })
210+
await clickDownload(container)
211+
expect(fetchMock).toHaveBeenCalledTimes(1)
212+
expect(downloadedNames).toEqual([])
213+
expect(container.querySelector('a')).toBeNull()
214+
}
215+
)
216+
217+
it('uses the same inline and storage handling for download all', async () => {
218+
fetchMock.mockResolvedValue(new Response('stored bytes'))
219+
const stored = { ...imageFile, id: 'stored', name: 'stored.png', base64: undefined }
220+
const container = renderFile([imageFile, stored])
221+
await act(async () => {
222+
container.querySelector('button')!.click()
223+
await vi.waitFor(() => expect(downloadedNames).toEqual(['generated.png', 'stored.png']))
224+
})
225+
expect(fetchMock).toHaveBeenCalledTimes(1)
226+
})
227+
228+
it('reports partial bulk failures, continues the batch, and clears the alert after a successful retry', async () => {
229+
fetchMock.mockResolvedValue(new Response(null, { status: 403 }))
230+
const stored = { ...imageFile, id: 'stored', name: 'stored.png', base64: undefined }
231+
const container = renderFile([stored, imageFile])
232+
await clickDownload(container)
233+
expect(downloadedNames).toEqual(['generated.png'])
234+
expect(container.querySelector('[role="alert"]')?.textContent).toContain(
235+
'Unable to download 1 file'
236+
)
237+
fetchMock.mockResolvedValue(new Response('stored bytes'))
238+
await act(async () => {
239+
container.querySelector('button')!.click()
240+
await vi.waitFor(() =>
241+
expect(downloadedNames).toEqual(['generated.png', 'stored.png', 'generated.png'])
242+
)
243+
})
244+
expect(container.querySelector('[role="alert"]')).toBeNull()
245+
})
246+
247+
it('uses the recognized key context when metadata omits it', async () => {
248+
fetchMock.mockResolvedValue(new Response('workspace bytes'))
249+
await clickDownload(
250+
renderFile({ ...imageFile, base64: undefined, key: 'workspace/id/file.png' })
251+
)
252+
expect(fetchMock).toHaveBeenCalledExactlyOnceWith(
253+
'/api/files/serve/workspace%2Fid%2Ffile.png?context=workspace',
254+
{ cache: 'no-store' }
255+
)
256+
})
257+
})
258+
259+
it('refuses unsafe external file URLs', async () => {
260+
const container = renderFile({
261+
...imageFile,
262+
base64: undefined,
263+
key: 'url/external',
264+
url: 'javascript:alert(1)',
265+
})
266+
await clickDownload(container)
267+
expect(fetchMock).not.toHaveBeenCalled()
268+
expect(window.open).not.toHaveBeenCalled()
269+
expect(downloadedNames).toEqual([])
270+
})

0 commit comments

Comments
 (0)