fix(files): hydrate chat attachments and reject blank download URLs

Author: icecrasher321

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

@@ -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')))
@@ -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) => {

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

@@ -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> {
@@ -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. */
@@ -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()

apps/sim/executor/handlers/agent/memory-harness.postgres.test.ts

@@ -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,
@@ -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
@@ -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 = []
@@ -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,
@@ -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)
@@ -529,7 +589,7 @@ describe.skipIf(!databaseUrl)(
         report.push({
           provider,
           streaming,
-          workspaceAttachment: true,
+          storageContext,
           stored,
           controls: {
             missingOrigin: 'blocked before HTTP',

apps/sim/lib/execution/payloads/file-secret-provenance.test.ts

@@ -7,8 +7,8 @@ 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', () => ({
+  readStoredWorkspaceFileRecordByKey: { execute: readWorkspaceFile },
 }))
 
 import { resolveStoredFileProvenanceSource } from '@/lib/execution/payloads/file-secret-provenance'

apps/sim/lib/execution/payloads/materialization.server.test.ts

@@ -19,11 +19,13 @@ vi.mock('@/app/api/files/authorization', () => ({
   verifyFileAccess: mockVerifyFileAccess,
 }))
 
-vi.mock('@/lib/workspace-files/application/read-workspace-file-content-by-key', () => ({
-  readWorkspaceFileRecordByKey: { execute: mockReadWorkspaceFileByKey },
+vi.mock('@/lib/workspace-files/application/read-stored-workspace-file-record-by-key', () => ({
+  readStoredWorkspaceFileRecordByKey: { execute: mockReadWorkspaceFileByKey },
 }))
 
+import { OrchestrationError } from '@/lib/core/orchestration/types'
 import {
+  assertUserFileContentAccess,
   readUserFileContent,
   readUserFileContentWithContributors,
 } from '@/lib/execution/payloads/materialization.server'
@@ -41,6 +43,17 @@ const generatedPdf: UserFile = {
   key: 'workspace/2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f/1700000000000-abc1234-report.pdf',
 }
 
+const delegatedReader = {
+  kind: 'delegated' as const,
+  serviceId: 'copilot' as const,
+  subjectUserId: 'reader',
+  workspaceId: 'workspace-1',
+  delegationId: 'read-1',
+  audience: 'sim:function-executions',
+  issuedAt: new Date(Date.now() - 1_000),
+  expiresAt: new Date(Date.now() + 60_000),
+}
+
 describe('readUserFileContent', () => {
   beforeEach(() => {
     vi.clearAllMocks()
@@ -264,4 +277,120 @@ describe('readUserFileContent', () => {
       },
     })
   })
+
+  it.each([undefined, 'workspace', 'mothership'] as const)(
+    'resolves chat-upload ownership canonically with descriptor context %s',
+    async (context) => {
+      const principal = {
+        kind: 'workspace_api_key' as const,
+        workspaceId: 'workspace-1',
+        keyId: 'key-1',
+      }
+      await assertUserFileContentAccess(
+        { key: 'workspace/workspace-1/upload.png', context },
+        { principal, workspaceId: 'workspace-1' }
+      )
+
+      expect(mockReadWorkspaceFileByKey).toHaveBeenCalledWith({
+        principal,
+        input: {
+          key: 'workspace/workspace-1/upload.png',
+          assertedWorkspaceId: 'workspace-1',
+        },
+      })
+      expect(mockVerifyFileAccess).not.toHaveBeenCalled()
+    }
+  )
+
+  it.each([
+    { userId: 'billing-owner' },
+    {
+      userId: 'billing-owner',
+      workspaceId: 'workspace-1',
+      principal: { kind: 'workspace_api_key' as const, workspaceId: 'workspace-1', keyId: 'key-1' },
+    },
+  ])('never uses a user fallback to accept a mothership context alias', async (options) => {
+    mockReadWorkspaceFileByKey.mockRejectedValue(
+      new OrchestrationError('not_found', 'File not found')
+    )
+
+    await expect(
+      assertUserFileContentAccess(
+        { key: 'workspace/workspace-1/upload.png', context: 'mothership' },
+        options
+      )
+    ).rejects.toThrow('Chat upload access requires canonical file authorization.')
+    expect(mockVerifyFileAccess).not.toHaveBeenCalled()
+    expect(mockDownloadServableFileFromStorage).not.toHaveBeenCalled()
+  })
+
+  it.each(
+    ([undefined, 'workspace', 'mothership'] as const).flatMap((context) =>
+      (['forbidden', 'not_found'] as const).flatMap((code) =>
+        [{ fileId: 'file-1' }, { chatId: 'chat-1' }, { fileId: 'file-1', chatId: 'chat-1' }].map(
+          (resourceScope) => ({ context, code, resourceScope })
+        )
+      )
+    )
+  )(
+    'preserves delegated file/chat limits after canonical rejection: %j',
+    async ({ context, code, resourceScope }) => {
+      const principal = { ...delegatedReader, resourceScope }
+      mockReadWorkspaceFileByKey.mockRejectedValue(new OrchestrationError(code, 'Denied'))
+
+      await expect(
+        assertUserFileContentAccess(
+          { key: 'workspace/workspace-1/upload.png', context },
+          { principal, workspaceId: 'workspace-1', userId: 'billing-owner' }
+        )
+      ).rejects.toMatchObject({ code })
+      expect(mockReadWorkspaceFileByKey).toHaveBeenCalledWith(
+        expect.objectContaining({
+          principal: expect.objectContaining({ resourceScope: principal.resourceScope }),
+        })
+      )
+      expect(mockVerifyFileAccess).not.toHaveBeenCalled()
+    }
+  )
+
+  it.each(
+    ([undefined, 'workspace'] as const).flatMap((context) =>
+      [{ kind: 'session' as const, userId: 'reader', sessionId: 'session-1' }, delegatedReader].map(
+        (principal) => ({ context, principal })
+      )
+    )
+  )(
+    'retains legacy authorization for an unscoped caller with absent metadata: %j',
+    async ({ context, principal }) => {
+      mockReadWorkspaceFileByKey.mockRejectedValue(
+        new OrchestrationError('not_found', 'File not found')
+      )
+      await expect(
+        assertUserFileContentAccess(
+          { key: 'workspace/workspace-1/legacy.png', context },
+          { principal, workspaceId: 'workspace-1', userId: 'reader' }
+        )
+      ).resolves.toBeUndefined()
+      expect(mockVerifyFileAccess).toHaveBeenCalledWith(
+        'workspace/workspace-1/legacy.png',
+        'reader',
+        undefined,
+        'workspace',
+        false,
+        { knowledgeAccess: undefined }
+      )
+    }
+  )
+
+  it.each([
+    'execution/workspace-1/workflow-1/run-1/file.png',
+    'profile-pictures/file.png',
+    'assistant/org-1/file.png',
+  ])('does not extend the mothership alias to %s', async (key) => {
+    await expect(
+      assertUserFileContentAccess({ key, context: 'mothership' }, { workspaceId: 'workspace-1' })
+    ).rejects.toThrow()
+    expect(mockReadWorkspaceFileByKey).not.toHaveBeenCalled()
+    expect(mockVerifyFileAccess).not.toHaveBeenCalled()
+  })
 })