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 5928480b4b8..0a1bf029446 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 @@ -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('Chat page')) + 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) => { 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 1da92cb8959..1a2c3b318ea 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 @@ -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 { @@ -88,11 +95,10 @@ async function triggerDownload(file: ChatFile): Promise { 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 { } 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() diff --git a/apps/sim/executor/handlers/agent/memory-harness.postgres.test.ts b/apps/sim/executor/handlers/agent/memory-harness.postgres.test.ts index b13d1b7f773..a55c6f45dfa 100644 --- a/apps/sim/executor/handlers/agent/memory-harness.postgres.test.ts +++ b/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', diff --git a/apps/sim/lib/execution/payloads/file-secret-provenance.test.ts b/apps/sim/lib/execution/payloads/file-secret-provenance.test.ts index 0afa418d05b..046c1f7cb83 100644 --- a/apps/sim/lib/execution/payloads/file-secret-provenance.test.ts +++ b/apps/sim/lib/execution/payloads/file-secret-provenance.test.ts @@ -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' diff --git a/apps/sim/lib/execution/payloads/materialization.server.test.ts b/apps/sim/lib/execution/payloads/materialization.server.test.ts index c7208738e04..42a5df4c621 100644 --- a/apps/sim/lib/execution/payloads/materialization.server.test.ts +++ b/apps/sim/lib/execution/payloads/materialization.server.test.ts @@ -19,14 +19,23 @@ 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', + async (importOriginal) => ({ + ...(await importOriginal< + typeof import('@/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' +import { StoredWorkspaceFileUnavailableError } from '@/lib/workspace-files/application/read-stored-workspace-file-record-by-key' import type { UserFile } from '@/executor/types' const PDF_SOURCE = Buffer.from('from reportlab.pdfgen import canvas') @@ -41,6 +50,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 +284,144 @@ 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( + ([undefined, 'workspace', 'mothership'] as const).flatMap((context) => + [ + { kind: 'session' as const, userId: 'reader', sessionId: 'session-1' }, + delegatedReader, + { ...delegatedReader, resourceScope: { fileId: 'file-1', chatId: 'chat-1' } }, + ].map((principal) => ({ context, principal })) + ) + )( + 'never falls back or reads bytes for a known unavailable binding: %j', + async ({ context, principal }) => { + const unavailable = new StoredWorkspaceFileUnavailableError() + mockReadWorkspaceFileByKey.mockRejectedValue(unavailable) + await expect( + readUserFileContent( + { ...generatedPdf, key: 'workspace/workspace-1/upload.png', context }, + { principal, workspaceId: 'workspace-1', userId: 'reader', encoding: 'base64' } + ) + ).rejects.toBe(unavailable) + expect(mockVerifyFileAccess).not.toHaveBeenCalled() + expect(mockDownloadServableFileFromStorage).not.toHaveBeenCalled() + } + ) + + 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() + }) }) diff --git a/apps/sim/lib/execution/payloads/materialization.server.ts b/apps/sim/lib/execution/payloads/materialization.server.ts index b0e6eb3483f..8e8035a90f7 100644 --- a/apps/sim/lib/execution/payloads/materialization.server.ts +++ b/apps/sim/lib/execution/payloads/materialization.server.ts @@ -29,7 +29,10 @@ import { } from '@/lib/uploads/utils/file-utils' import { downloadServableFileFromStorage } from '@/lib/uploads/utils/file-utils.server' import { rebindWorkspaceFileDelegatedPrincipal } from '@/lib/workspace-files/application/delegated-principal' -import { readWorkspaceFileRecordByKey } from '@/lib/workspace-files/application/read-workspace-file-content-by-key' +import { + readStoredWorkspaceFileRecordByKey, + StoredWorkspaceFileUnavailableError, +} from '@/lib/workspace-files/application/read-stored-workspace-file-record-by-key' import type { UserFile } from '@/executor/types' const logger = createLogger('ExecutionPayloadMaterialization') @@ -266,7 +269,11 @@ function getVerifiedStorageContext(file: Pick): Sto } const inferredContext = inferContextFromKey(file.key) - if (file.context && file.context !== inferredContext) { + if ( + file.context && + file.context !== inferredContext && + !(inferredContext === 'workspace' && file.context === 'mothership') + ) { throw new Error('File context does not match its storage key.') } @@ -305,7 +312,7 @@ export async function assertUserFileContentAccess( }) : options.principal try { - await readWorkspaceFileRecordByKey.execute({ + await readStoredWorkspaceFileRecordByKey.execute({ principal, input: { key: file.key, @@ -315,9 +322,22 @@ export async function assertUserFileContentAccess( return } catch (error) { if (!(error instanceof OrchestrationError && error.code === 'not_found')) throw error + if (error instanceof StoredWorkspaceFileUnavailableError) throw error + /** Legacy storage metadata cannot prove a delegated file or chat identity. */ + if ( + options.principal.kind === 'delegated' && + (options.principal.resourceScope?.fileId !== undefined || + options.principal.resourceScope?.chatId !== undefined) + ) { + throw error + } } } + if (context === 'workspace' && file.context === 'mothership') { + throw new Error('Chat upload access requires canonical file authorization.') + } + if (!options.userId) { throw new Error('File access requires an authenticated user.') } diff --git a/apps/sim/lib/function-execution/sandbox-mounts.test.ts b/apps/sim/lib/function-execution/sandbox-mounts.test.ts index 6557a35cb0d..f988bd4ef2f 100644 --- a/apps/sim/lib/function-execution/sandbox-mounts.test.ts +++ b/apps/sim/lib/function-execution/sandbox-mounts.test.ts @@ -32,9 +32,15 @@ vi.mock('@/lib/uploads/utils/file-utils.server', () => ({ downloadServableFileFromStorage: mockDownloadServableFileFromStorage, })) -vi.mock('@/lib/workspace-files/application/read-workspace-file-content-by-key', () => ({ - readWorkspaceFileRecordByKey: { execute: mockReadWorkspaceFileRecordByKey }, -})) +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: mockReadWorkspaceFileRecordByKey }, + }) +) vi.mock('@/lib/uploads/server/metadata', () => ({ getFileMetadataByKey: mockGetFileMetadataByKey, diff --git a/apps/sim/lib/internal/file/operations.test.ts b/apps/sim/lib/internal/file/operations.test.ts index 3cffe984456..8230dcc4aec 100644 --- a/apps/sim/lib/internal/file/operations.test.ts +++ b/apps/sim/lib/internal/file/operations.test.ts @@ -1019,6 +1019,7 @@ describe('file manage folder wiring', () => { describe('file manage operations', () => { beforeEach(() => { vi.clearAllMocks() + mockGetFileMetadataByKey.mockResolvedValue(null) hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({ success: true, userId: 'user-1', @@ -1150,16 +1151,31 @@ describe('file manage operations', () => { }, ] - beforeEach(() => { - mockResolveWorkspaceFileReference.mockResolvedValue(workspaceFile('document')) - mockGetFileMetadataByKey.mockResolvedValue({ - id: contributor.fileId, - key: contributor.key, - context: contributor.context, + function mockContributorMetadata(overrides: { userId?: string; workspaceId?: string } = {}) { + const document = { + id: 'document', + key: 'workspace/workspace-1/document.txt', + context: 'workspace', workspaceId: 'workspace-1', userId: 'user-1', contentUpdatedAt: CONTENT_UPDATED_AT, - }) + } + const image = { + ...document, + id: contributor.fileId, + key: contributor.key, + ...overrides, + } + const records = new Map([ + [document.key, document], + [image.key, image], + ]) + mockGetFileMetadataByKey.mockImplementation(async (key: string) => records.get(key) ?? null) + } + + beforeEach(() => { + mockResolveWorkspaceFileReference.mockResolvedValue(workspaceFile('document')) + mockContributorMetadata() mockDownloadServableFileFromStorage.mockResolvedValue({ buffer: Buffer.from('rendered image content'), contentType: 'text/plain', @@ -1301,14 +1317,7 @@ describe('file manage operations', () => { it.each(['write', 'compress'] as const)( '%s retains the secret owner guard for rendered contributors', async (operation) => { - mockGetFileMetadataByKey.mockResolvedValue({ - id: contributor.fileId, - key: contributor.key, - context: contributor.context, - workspaceId: 'workspace-1', - userId: 'other-user', - contentUpdatedAt: CONTENT_UPDATED_AT, - }) + mockContributorMetadata({ userId: 'other-user' }) mockGetBoundWorkspaceFileSecretProvenance.mockImplementation( async (_workspaceId: string, identity: { fileId: string }) => ({ status: 'exact', @@ -1326,14 +1335,7 @@ describe('file manage operations', () => { ) it('refuses a rendered contributor whose canonical scope differs', async () => { - mockGetFileMetadataByKey.mockResolvedValue({ - id: contributor.fileId, - key: contributor.key, - context: contributor.context, - workspaceId: 'other-workspace', - userId: 'user-1', - contentUpdatedAt: CONTENT_UPDATED_AT, - }) + mockContributorMetadata({ workspaceId: 'other-workspace' }) mockGetBoundWorkspaceFileSecretProvenance.mockResolvedValue({ status: 'exact', entries: [] }) const response = await POST(renderedRequest('write')) diff --git a/apps/sim/lib/workspace-files/application/read-stored-workspace-file-record-by-key.test.ts b/apps/sim/lib/workspace-files/application/read-stored-workspace-file-record-by-key.test.ts new file mode 100644 index 00000000000..8f67639c8d4 --- /dev/null +++ b/apps/sim/lib/workspace-files/application/read-stored-workspace-file-record-by-key.test.ts @@ -0,0 +1,284 @@ +/** @vitest-environment node */ +import type { DelegatedPrincipal, Principal } from '@sim/auth/principal' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + metadata: vi.fn(), + workspace: vi.fn(), + permission: vi.fn(), +})) + +vi.mock('@/lib/uploads/server/metadata', () => ({ getFileMetadataByKey: mocks.metadata })) +vi.mock('@/lib/uploads/contexts/workspace', () => ({ loadActiveWorkspaceContext: mocks.workspace })) +vi.mock('@sim/platform-authz/workspace', () => ({ + permissionSatisfies: (permission: string) => ['read', 'write', 'admin'].includes(permission), + resolveEffectiveWorkspacePermission: mocks.permission, +})) + +import { + readStoredWorkspaceFileRecordByKey, + StoredWorkspaceFileUnavailableError, +} from '@/lib/workspace-files/application/read-stored-workspace-file-record-by-key' + +const input = { + key: 'workspace/workspace-1/upload.png', + assertedWorkspaceId: 'workspace-1', +} +const workspace = { + workspaceId: 'workspace-1', + workspaceOrganizationId: null, + allowPersonalApiKeys: true, + billedAccountUserId: 'billing-owner', +} +const file = { + id: 'file-1', + key: input.key, + context: 'mothership', + workspaceId: 'workspace-1', + organizationId: null, + chatId: 'chat-1', + deletedAt: null, + userId: 'uploader', +} +const session = { kind: 'session', userId: 'reader', sessionId: 'session-1' } as const + +function executor(overrides: Partial = {}): DelegatedPrincipal { + return { + kind: 'delegated', + serviceId: 'executor', + workspaceId: workspace.workspaceId, + delegationId: 'delegation-1', + audience: 'sim:workspace-files', + issuedAt: new Date(Date.now() - 1_000), + expiresAt: new Date(Date.now() + 60_000), + delegationContext: { + kind: 'workflow_execution', + workflowId: 'workflow-1', + executionId: 'execution-1', + principal: { + kind: 'system', + serviceId: 'chat', + workspaceId: workspace.workspaceId, + workflowId: 'workflow-1', + }, + currentWorkflow: { + workflowId: 'workflow-1', + mode: 'deployment', + deploymentVersionId: 'deployment-1', + }, + }, + ...overrides, + } +} + +describe('readStoredWorkspaceFileRecordByKey', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.metadata.mockResolvedValue(file) + mocks.workspace.mockResolvedValue(workspace) + mocks.permission.mockResolvedValue('read') + }) + + it.each(['workspace', 'mothership'] as const)( + 'authorizes canonical %s bytes with the actual current workspace member', + async (context) => { + const record = { ...file, context } + mocks.metadata.mockResolvedValue(record) + + await expect( + readStoredWorkspaceFileRecordByKey.execute({ principal: session, input }) + ).resolves.toEqual({ file: record }) + expect(mocks.metadata).toHaveBeenCalledWith(input.key) + expect(mocks.metadata).toHaveBeenNthCalledWith(1, input.key, undefined, { + includeDeleted: true, + }) + expect(mocks.metadata).toHaveBeenCalledTimes(2) + expect(mocks.permission).toHaveBeenCalledWith('reader', 'workspace-1', null, undefined, { + forUpdate: undefined, + }) + } + ) + + it.each([ + { kind: 'personal_api_key', userId: 'reader', keyId: 'key-1' }, + { kind: 'workspace_api_key', workspaceId: 'workspace-1', keyId: 'key-1' }, + ])( + 'preserves the $kind authority without substituting the uploader or billing owner', + async (principal) => { + await expect( + readStoredWorkspaceFileRecordByKey.execute({ principal, input }) + ).resolves.toEqual({ + file, + }) + if (principal.kind === 'personal_api_key') { + expect(mocks.permission).toHaveBeenCalledWith('reader', 'workspace-1', null, undefined, { + forUpdate: undefined, + }) + } else { + expect(mocks.permission).not.toHaveBeenCalled() + } + } + ) + + it('admits a deployment executor through workspace authority without consulting a human', async () => { + await expect( + readStoredWorkspaceFileRecordByKey.execute({ principal: executor(), input }) + ).resolves.toEqual({ file }) + expect(mocks.permission).not.toHaveBeenCalled() + }) + + it.each([{ fileId: 'file-1' }, { chatId: 'chat-1' }, { fileId: 'file-1', chatId: 'chat-1' }])( + 'retains a matching narrower delegation: %j', + async (resourceScope) => { + await expect( + readStoredWorkspaceFileRecordByKey.execute({ + principal: executor({ resourceScope }), + input, + }) + ).resolves.toEqual({ file }) + } + ) + + it('keeps workspace files available to a chat-scoped delegate', async () => { + mocks.metadata.mockResolvedValue({ ...file, context: 'workspace', chatId: null }) + await expect( + readStoredWorkspaceFileRecordByKey.execute({ + principal: executor({ resourceScope: { chatId: 'chat-1' } }), + input, + }) + ).resolves.toMatchObject({ file: { context: 'workspace' } }) + }) + + it.each>([ + { resourceScope: { fileId: 'other-file' } }, + { resourceScope: { chatId: 'other-chat' } }, + { resourceScope: { fileId: 'file-1', chatId: 'other-chat' } }, + { workspaceId: 'other-workspace' }, + { audience: 'other-audience' }, + { expiresAt: new Date(0) }, + { delegationContext: undefined }, + ])('rejects invalid delegation before reading content metadata: %j', async (overrides) => { + await expect( + readStoredWorkspaceFileRecordByKey.execute({ principal: executor(overrides), input }) + ).rejects.toMatchObject({ code: 'forbidden' }) + expect(mocks.metadata).toHaveBeenCalledTimes(1) + expect(mocks.permission).not.toHaveBeenCalled() + }) + + it('does not broaden a chat-scoped delegation when an attachment has no chat binding', async () => { + mocks.metadata.mockResolvedValue({ ...file, chatId: null }) + await expect( + readStoredWorkspaceFileRecordByKey.execute({ + principal: executor({ resourceScope: { chatId: 'chat-1' } }), + input, + }) + ).rejects.toMatchObject({ code: 'forbidden' }) + }) + + it('rechecks the real human behind an executor and denies revoked membership', async () => { + mocks.permission.mockResolvedValue(null) + const principal = executor({ + subjectUserId: 'reader', + delegationContext: { + ...executor().delegationContext!, + principal: session, + }, + }) + await expect( + readStoredWorkspaceFileRecordByKey.execute({ principal, input }) + ).rejects.toMatchObject({ + code: 'forbidden', + }) + expect(mocks.permission).toHaveBeenCalledWith('reader', 'workspace-1', null, undefined, { + forUpdate: undefined, + }) + expect(mocks.metadata).toHaveBeenCalledTimes(1) + }) + + it('rejects raw system identity before loading metadata', async () => { + await expect( + readStoredWorkspaceFileRecordByKey.execute({ + principal: { + kind: 'system', + serviceId: 'chat', + workspaceId: 'workspace-1', + workflowId: 'workflow-1', + }, + input, + }) + ).rejects.toMatchObject({ code: 'forbidden' }) + expect(mocks.metadata).not.toHaveBeenCalled() + }) + + it('keeps absent initial metadata distinguishable for legacy access', async () => { + mocks.metadata.mockResolvedValue(null) + const read = readStoredWorkspaceFileRecordByKey.execute({ principal: session, input }) + await expect(read).rejects.toMatchObject({ code: 'not_found' }) + await expect(read).rejects.not.toBeInstanceOf(StoredWorkspaceFileUnavailableError) + expect(mocks.workspace).not.toHaveBeenCalled() + expect(mocks.permission).not.toHaveBeenCalled() + }) + + it('conceals canonical unavailability as not_found', () => { + expect(new StoredWorkspaceFileUnavailableError()).toMatchObject({ + code: 'not_found', + message: 'File not found', + }) + }) + + it.each([ + { ...file, deletedAt: new Date() }, + { ...file, workspaceId: 'other-workspace' }, + { ...file, workspaceId: null }, + { ...file, organizationId: 'organization-1' }, + { ...file, context: 'execution' }, + { ...file, context: 'profile-pictures' }, + { ...file, key: 'workspace/workspace-1/different.png' }, + ])('refuses legacy fallback for invalid canonical metadata: %j', async (record) => { + mocks.metadata.mockResolvedValue(record) + await expect( + readStoredWorkspaceFileRecordByKey.execute({ principal: session, input }) + ).rejects.toBeInstanceOf(StoredWorkspaceFileUnavailableError) + expect(mocks.workspace).not.toHaveBeenCalled() + expect(mocks.permission).not.toHaveBeenCalled() + }) + + it.each([null, { ...workspace, workspaceId: 'other-workspace' }])( + 'rejects an inactive or changed workspace: %j', + async (record) => { + mocks.workspace.mockResolvedValue(record) + await expect( + readStoredWorkspaceFileRecordByKey.execute({ principal: session, input }) + ).rejects.toBeInstanceOf(StoredWorkspaceFileUnavailableError) + expect(mocks.permission).not.toHaveBeenCalled() + } + ) + + it.each([ + null, + { ...file, id: 'replacement-file' }, + { ...file, key: 'workspace/workspace-1/new.png' }, + { ...file, deletedAt: new Date() }, + { ...file, workspaceId: 'other-workspace' }, + { ...file, context: 'workspace' }, + { ...file, chatId: 'other-chat' }, + ])('rejects an identity changed during authorization: %j', async (record) => { + mocks.metadata.mockResolvedValueOnce(file).mockResolvedValueOnce(record) + await expect( + readStoredWorkspaceFileRecordByKey.execute({ principal: session, input }) + ).rejects.toBeInstanceOf(StoredWorkspaceFileUnavailableError) + expect(mocks.permission).toHaveBeenCalledOnce() + }) + + it.each([ + '', + 'legacy-unprefixed.png', + 'assistant/org-1/file.png', + 'execution/workspace-1/file.png', + ])('rejects a storage key outside the shared workspace bucket: %s', async (key) => { + await expect( + readStoredWorkspaceFileRecordByKey.execute({ principal: session, input: { ...input, key } }) + ).rejects.toMatchObject({ code: 'not_found' }) + expect(mocks.metadata).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/workspace-files/application/read-stored-workspace-file-record-by-key.ts b/apps/sim/lib/workspace-files/application/read-stored-workspace-file-record-by-key.ts new file mode 100644 index 00000000000..acdb813194e --- /dev/null +++ b/apps/sim/lib/workspace-files/application/read-stored-workspace-file-record-by-key.ts @@ -0,0 +1,89 @@ +import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { + type ActiveWorkspaceContext, + loadActiveWorkspaceContext, +} from '@/lib/uploads/contexts/workspace' +import { type FileMetadataRecord, getFileMetadataByKey } from '@/lib/uploads/server/metadata' +import { isWorkspaceScopedContext } from '@/lib/uploads/shared/types' +import { tryInferContextFromKey } from '@/lib/uploads/utils/file-utils' +import { workspaceFileDelegationPolicy } from '@/lib/workspace-files/application/authorization' +import { fileOperations } from '@/lib/workspace-files/application/operations' + +interface ReadStoredWorkspaceFileByKeyInput { + key: string + assertedWorkspaceId: string +} + +interface StoredWorkspaceFileContext extends ActiveWorkspaceContext { + fileId: string + file: FileMetadataRecord +} + +/** Conceals a known unavailable binding without permitting legacy storage authorization. */ +export class StoredWorkspaceFileUnavailableError extends OrchestrationError { + constructor() { + super('not_found', 'File not found') + this.name = 'StoredWorkspaceFileUnavailableError' + } +} + +/** + * Authorizes stored bytes under the shared workspace tenancy of files and chat uploads. + * Canonical metadata determines ownership; workspace-file CRUD remains workspace-only. + */ +export const readStoredWorkspaceFileRecordByKey = defineAuthorizedWorkspaceUseCase({ + operation: fileOperations.readContent, + async resolveContext({ + input, + }: { + input: ReadStoredWorkspaceFileByKeyInput + }): Promise { + if (tryInferContextFromKey(input.key) !== 'workspace') { + throw new OrchestrationError('not_found', 'File not found') + } + const file = await getFileMetadataByKey(input.key, undefined, { includeDeleted: true }) + if (!file) throw new OrchestrationError('not_found', 'File not found') + if ( + !file.workspaceId || + file.deletedAt || + file.organizationId || + file.key !== input.key || + !isWorkspaceScopedContext(file.context) || + file.workspaceId !== input.assertedWorkspaceId + ) { + throw new StoredWorkspaceFileUnavailableError() + } + const workspace = await loadActiveWorkspaceContext(file.workspaceId) + if (!workspace || workspace.workspaceId !== file.workspaceId) { + throw new StoredWorkspaceFileUnavailableError() + } + return { ...workspace, fileId: file.id, file } + }, + authorizationOptions: { + delegation: { + audience: workspaceFileDelegationPolicy.audience, + isWithinScope: (principal, context) => + workspaceFileDelegationPolicy.isWithinScope(principal, context) && + (context.file.context !== 'mothership' || + principal.resourceScope?.chatId === undefined || + principal.resourceScope.chatId === context.file.chatId), + }, + }, + async execute({ input, context }): Promise<{ file: FileMetadataRecord }> { + const file = await getFileMetadataByKey(input.key) + if ( + !file || + file.deletedAt || + file.organizationId || + file.id !== context.fileId || + file.key !== input.key || + file.workspaceId !== context.workspaceId || + file.context !== context.file.context || + file.chatId !== context.file.chatId + ) { + throw new StoredWorkspaceFileUnavailableError() + } + return { file } + }, +}) diff --git a/apps/sim/providers/file-attachments-authorization.test.ts b/apps/sim/providers/file-attachments-authorization.test.ts index 78e8260fcc9..ebbfd6e45e2 100644 --- a/apps/sim/providers/file-attachments-authorization.test.ts +++ b/apps/sim/providers/file-attachments-authorization.test.ts @@ -38,7 +38,7 @@ import type { ProviderRequest } from '@/providers/types' /** Authorization and key inference are real: mocking either hid this pre-existing refusal. */ describe('provider attachment storage-key authorization', () => { beforeEach(() => { - vi.clearAllMocks() + vi.resetAllMocks() }) it.each( @@ -94,4 +94,39 @@ describe('provider attachment storage-key authorization', () => { expect(permission).not.toHaveBeenCalled() } ) + + it('allows standalone provider reads of authorized mothership attachments in workspace storage', async () => { + const file: UserFile = { + id: 'attachment-1', + name: 'document.pdf', + key: 'workspace/workspace-1/attachment-1/document.pdf', + url: '', + size: 10 * 1024 * 1024, + type: 'application/pdf', + context: 'workspace', + } + metadata.mockResolvedValue({ + id: file.id, + key: file.key, + workspaceId: 'workspace-1', + userId: 'uploader', + context: 'mothership', + deletedAt: null, + }) + permission.mockResolvedValue('read') + presign.mockResolvedValue('https://storage.example.com/signed') + + await attachLargeFileRemoteUrls( + { + model: 'gpt-4.1', + userId: 'reader', + messages: [{ role: 'user', content: 'Read the attachment', files: [file] }], + }, + 'openai' + ) + + expect(permission).toHaveBeenCalledWith('reader', 'workspace', 'workspace-1') + expect(presign).toHaveBeenCalledWith(file.key, 'workspace', 3600) + expect(file.remoteUrl).toBe('https://storage.example.com/signed') + }) })