Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ describe('ChatFileDownload', () => {
)
})

it.each(['', ' \t\n'])('uses the serve route to preview files with a blank URL (%j)', (url) => {
const container = renderFile({ ...imageFile, base64: undefined, url })
expect(container.querySelector('img')?.getAttribute('src')).toBe(
'/api/files/serve/execution%2Fgenerated.png?context=execution'
)
})

it('keeps a download available when an image preview fails', () => {
const container = renderFile(imageFile)
act(() => container.querySelector('img')!.dispatchEvent(new Event('error')))
Expand Down Expand Up @@ -154,6 +161,26 @@ describe('chat file downloads', () => {
expect(downloadedNames).toEqual(['generated.png'])
})

it.each(['', ' \t\n'].flatMap((url) => [false, true].map((stored) => ({ url, stored }))))(
'never downloads the chat page for a blank URL (%j)',
async ({ url, stored }) => {
if (stored) fetchMock.mockResolvedValueOnce(new Response(null, { status: 401 }))
fetchMock.mockResolvedValue(new Response('<html>Chat page</html>'))
const container = renderFile({
...imageFile,
base64: undefined,
key: stored ? imageFile.key : 'url/external',
url,
})
await clickDownload(container)
expect(fetchMock).toHaveBeenCalledTimes(stored ? 1 : 0)
expect(createObjectURL).not.toHaveBeenCalled()
expect(downloadedNames).toEqual([])
expect(container.querySelector('a')).toBeNull()
expect(container.querySelector('[role="alert"]')?.textContent).toContain('Unable to download')
}
)

it.each([false, true])(
'offers a safe browser download when an external host blocks CORS (stored=%s)',
async (stored) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,17 @@ function isImageFile(mimeType: string): boolean {
return mimeType.startsWith('image/')
}

function getExternalFileUrl(file: ChatFile): string | null {
const url = file.url?.trim()
return url && isSafeHttpUrl(url) ? url : null
}

function getFileUrl(file: ChatFile): string {
if (file.base64) return `data:${file.type};base64,${file.base64}`
if (isSafeHttpUrl(file.url)) return file.url
return `/api/files/serve/${encodeURIComponent(file.key)}?context=${file.context || 'execution'}`
return (
getExternalFileUrl(file) ??
`/api/files/serve/${encodeURIComponent(file.key)}?context=${file.context || 'execution'}`
)
}

async function triggerDownload(file: ChatFile): Promise<void> {
Expand All @@ -88,11 +95,10 @@ async function triggerDownload(file: ChatFile): Promise<void> {

const storageContext = tryInferContextFromKey(file.key)
const hasStorageKey = storageContext !== null
const externalUrl = getExternalFileUrl(file)
const url = hasStorageKey
? `/api/files/serve/${encodeURIComponent(file.key)}?context=${encodeURIComponent(storageContext)}`
: isSafeHttpUrl(file.url)
? file.url
: null
: externalUrl
if (!url) throw new Error('File has no download URL')

/** The same serve route as execution logs resolves current storage access on each click. */
Expand All @@ -103,10 +109,10 @@ async function triggerDownload(file: ChatFile): Promise<void> {
} else {
response = await fetchExternalFile(url)
}
if (hasStorageKey && response.status === 401 && isSafeHttpUrl(file.url)) {
if (hasStorageKey && response.status === 401 && externalUrl) {
await response.body?.cancel()
/** Public chat visitors may only have the file access already delivered in the response. */
response = await fetchExternalFile(file.url)
response = await fetchExternalFile(externalUrl)
}
if (!response.ok) {
await response.body?.cancel()
Expand Down
72 changes: 66 additions & 6 deletions apps/sim/executor/handlers/agent/memory-harness.postgres.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import {
workspaceFiles,
} from '@sim/db/schema'
import { hashDurableSecretProvenanceValue } from '@/lib/execution/durable-secret-provenance'
import { StorageService } from '@/lib/uploads'
import { uploadExecutionFile } from '@/lib/uploads/contexts/execution/execution-file-manager'
import {
EXACT_EMPTY_WORKSPACE_FILE_SECRET_PROVENANCE,
Expand All @@ -84,7 +85,13 @@ import { AgentBlockHandler } from '@/executor/handlers/agent/agent-handler'
import type { AgentInputs, Message } from '@/executor/handlers/agent/types'
import type { ExecutionContext, StreamingExecution, UserFile } from '@/executor/types'
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
import { INLINE_ATTACHMENT_THRESHOLD_BYTES } from '@/providers/attachments'
import {
attachLargeFileRemoteUrls,
uploadLargeFilesToProvider,
} from '@/providers/file-attachments.server'
import { createAgentStreamPump } from '@/providers/stream-pump'
import type { ProviderRequest } from '@/providers/types'
import type { SerializedBlock } from '@/serializer/types'

const databaseUrl = process.env.AGENT_MEMORY_TEST_DATABASE_URL
Expand Down Expand Up @@ -382,12 +389,14 @@ describe.skipIf(!databaseUrl)(
})

it.each(
(['openai', 'anthropic'] as const).flatMap((provider) =>
[false, true].map((streaming) => ({ provider, streaming }))
(['workspace', 'mothership'] as const).flatMap((storageContext) =>
(['openai', 'anthropic'] as const).flatMap((provider) =>
[false, true].map((streaming) => ({ storageContext, provider, streaming }))
)
)
)(
'deployed chat reads remembered workspace files with $provider, streaming=$streaming',
async ({ provider, streaming }) => {
'deployed chat reads remembered $storageContext files with $provider, streaming=$streaming',
async ({ storageContext, provider, streaming }) => {
if (!fixture.database || !connection) throw new Error('Missing harness database')
vi.stubGlobal('fetch', interceptFetch)
outbound = []
Expand All @@ -414,7 +423,7 @@ describe.skipIf(!databaseUrl)(
key,
userId: scope.userId,
workspaceId: scope.workspaceId,
context: 'workspace',
context: storageContext,
originalName: 'result.pdf',
contentType: 'application/pdf',
size: buffer.length,
Expand Down Expand Up @@ -487,6 +496,57 @@ describe.skipIf(!databaseUrl)(
input: { key, assertedWorkspaceId: scope.workspaceId },
})
).rejects.toThrow('Principal kind system')

const strictWorkspaceRead = readWorkspaceFileRecordByKey.execute({
principal: {
kind: 'workspace_api_key',
workspaceId: scope.workspaceId,
keyId: 'harness-key',
},
input: { key, assertedWorkspaceId: scope.workspaceId },
})
if (storageContext === 'mothership') {
await expect(strictWorkspaceRead).rejects.toMatchObject({ code: 'not_found' })
} else {
await expect(strictWorkspaceRead).resolves.toMatchObject({ file: { id: record.id } })
}

/** Exercise large-file authorization with real metadata and delegation before model dispatch. */
const largeFile = { ...file, size: INLINE_ATTACHMENT_THRESHOLD_BYTES + 1 }
const largeRequest: ProviderRequest = {
model: models[provider],
apiKey: apiKey(provider),
userId: scope.userId,
messages: [{ role: 'user', content: 'Read the attachment', files: [largeFile] }],
}
const cloudStorage = vi.spyOn(StorageService, 'hasCloudStorage').mockReturnValue(true)
const presign = vi
.spyOn(StorageService, 'generatePresignedDownloadUrl')
.mockResolvedValue('https://storage.example.com/signed')
try {
await attachLargeFileRemoteUrls(largeRequest, provider, firstContext)
expect(presign).toHaveBeenCalledWith(key, 'workspace', 3600)
expect(largeFile.remoteUrl).toBe('https://storage.example.com/signed')
if (provider === 'openai') {
const upload = vi.fn(async (url: string, init?: RequestInit) => {
expect(url).toBe('https://api.openai.com/v1/files')
expect(init?.body).toBeInstanceOf(FormData)
const body = init!.body as FormData
const uploaded = body.get('file') as File
expect(Buffer.from(await uploaded.arrayBuffer())).toEqual(buffer)
return Response.json({ id: 'file-harness' })
})
vi.stubGlobal('fetch', upload)
await uploadLargeFilesToProvider(largeRequest, provider, firstContext)
expect(upload).toHaveBeenCalledOnce()
expect(largeFile.providerFileId).toBe('file-harness')
}
} finally {
cloudStorage.mockRestore()
presign.mockRestore()
vi.stubGlobal('fetch', interceptFetch)
}

expect(await executeTurn(firstContext, { ...inputs, files: [file] })).toBe('READY')
expect(requestFiles(outbound[0])).toEqual([buffer.toString('base64')])
const stored = await readConversation(conversationId)
Expand Down Expand Up @@ -529,7 +589,7 @@ describe.skipIf(!databaseUrl)(
report.push({
provider,
streaming,
workspaceAttachment: true,
storageContext,
stored,
controls: {
missingOrigin: 'blocked before HTTP',
Expand Down
12 changes: 9 additions & 3 deletions apps/sim/lib/execution/payloads/file-secret-provenance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,15 @@ const { metadata, readWorkspaceFile } = vi.hoisted(() => ({
}))

vi.mock('@/lib/uploads/server/metadata', () => ({ getFileMetadataByKey: metadata }))
vi.mock('@/lib/workspace-files/application/read-workspace-file-content-by-key', () => ({
readWorkspaceFileRecordByKey: { execute: readWorkspaceFile },
}))
vi.mock(
'@/lib/workspace-files/application/read-stored-workspace-file-record-by-key',
async (importOriginal) => ({
...(await importOriginal<
typeof import('@/lib/workspace-files/application/read-stored-workspace-file-record-by-key')
>()),
readStoredWorkspaceFileRecordByKey: { execute: readWorkspaceFile },
})
)

import { resolveStoredFileProvenanceSource } from '@/lib/execution/payloads/file-secret-provenance'

Expand Down
Loading
Loading