From fd01018c6e31a09e35449d01fc3c909a4573cc3b Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 15 Sep 2026 13:45:04 -0700 Subject: [PATCH 1/2] fix(agent): authorize workspace attachments through execution delegation --- .../handlers/agent/agent-handler.test.ts | 46 ++++ .../executor/handlers/agent/agent-handler.ts | 73 ++++--- .../agent/memory-harness.postgres.test.ts | 202 +++++++++++++++++- .../file/materialization-context.test.ts | 110 ++++++++++ .../internal/file/materialization-context.ts | 33 +++ .../providers/file-attachments.server.test.ts | 121 ++++++++++- apps/sim/providers/file-attachments.server.ts | 47 ++-- apps/sim/providers/index.test.ts | 24 ++- apps/sim/providers/index.ts | 4 +- 9 files changed, 606 insertions(+), 54 deletions(-) create mode 100644 apps/sim/lib/internal/file/materialization-context.test.ts create mode 100644 apps/sim/lib/internal/file/materialization-context.ts diff --git a/apps/sim/executor/handlers/agent/agent-handler.test.ts b/apps/sim/executor/handlers/agent/agent-handler.test.ts index b60638e6953..edd09b45fd2 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.test.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.test.ts @@ -953,6 +953,52 @@ describe('AgentBlockHandler', () => { expect(inputs).toEqual(rawInputs) }) + it.each([ + 'url/https://example.com/image.png', + '', + 'provider-file-id', + 'profile-pictures/avatar.png', + ])('preserves inline bytes for an actorless request with key %s', async (key) => { + mockGetProviderFromModel.mockReturnValue('openai') + await handler.execute( + { + ...mockContext, + principal: { + kind: 'system', + serviceId: 'chat', + workspaceId: 'test-workspace', + workflowId: 'test-workflow', + }, + executorDelegationOrigin: undefined, + }, + mockBlock, + { + model: 'gpt-4o', + messages: [ + { + role: 'user', + content: 'Analyze this image', + files: [ + { + id: 'file-1', + key, + name: 'image.png', + url: 'https://example.com/image.png', + size: 5, + type: 'image/png', + base64: 'aW1hZ2U=', + }, + ], + }, + ], + apiKey: 'test-api-key', + } + ) + expect(mockExecuteProviderRequest.mock.calls[0][1].messages[0].files).toEqual([ + expect.objectContaining({ base64: 'aW1hZ2U=' }), + ]) + }) + it('normalizes the persisted workspace-picker shape before provider execution', async () => { const key = 'workspace/ws-1/example.png' const hydrationSpy = vi diff --git a/apps/sim/executor/handlers/agent/agent-handler.ts b/apps/sim/executor/handlers/agent/agent-handler.ts index a2d1295facc..f33ff7b8ea1 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.ts @@ -9,6 +9,7 @@ import { selectModelSchemaInputPaths, } from '@/lib/execution/model-input-provenance' import { readAvailableCustomToolByIdOrTitleAsExecutor } from '@/lib/internal/custom-tools/read-available-by-id-or-title' +import { resolveExecutorFileMaterializationContext } from '@/lib/internal/file/materialization-context' import { discoverMcpServerToolsAsExecutor } from '@/lib/internal/mcp/discover-tools' import { readWorkflowInputFieldsForTool, @@ -31,6 +32,7 @@ import { MODEL_SUPPORTED_IMAGE_MIME_TYPES, processFilesToUserFiles, type RawFileInput, + tryInferContextFromKey, } from '@/lib/uploads/utils/file-utils' import { selectModelBoundFileInputPaths } from '@/lib/uploads/utils/model-input' import { hydrateUserFilesWithBase64 } from '@/lib/uploads/utils/user-file-base64.server' @@ -63,7 +65,7 @@ import type { ToolInput, } from '@/executor/handlers/agent/types' import { parseResponseFormat } from '@/executor/handlers/shared/response-format' -import type { BlockHandler, ExecutionContext, StreamingExecution } from '@/executor/types' +import type { BlockHandler, ExecutionContext, StreamingExecution, UserFile } from '@/executor/types' import { collectBlockData } from '@/executor/utils/block-data' import { stringifyJSON } from '@/executor/utils/json' import { projectResolvedSecretDiagnosticContent } from '@/executor/utils/resolved-secret-content-projection' @@ -1481,7 +1483,6 @@ export class AgentBlockHandler implements BlockHandler { throw new Error(`File attachments are not supported for provider "${providerId}"`) } - const requestId = ctx.executionId || ctx.workflowId || 'agent-files' const nextMessages = [...messages] const inlineMaxBytes = getInlineHydrationMaxBytes(providerId) @@ -1493,36 +1494,46 @@ export class AgentBlockHandler implements BlockHandler { } const unsafeGeneratedDocumentFiles = new Set() - const hydratedFiles = await hydrateUserFilesWithBase64(message.files, { - requestId, - workspaceId: ctx.workspaceId, - workflowId: ctx.workflowId, - executionId: ctx.executionId, - largeValueExecutionIds: ctx.largeValueExecutionIds, - largeValueKeys: ctx.largeValueKeys, - fileKeys: ctx.fileKeys, - allowLargeValueWorkflowScope: ctx.allowLargeValueWorkflowScope, - userId: ctx.userId, - principal: ctx.principal, - logger, - maxBytes: inlineMaxBytes, - onServableFileContributors: async (file, contributors) => { - if (!ctx.workspaceId) return - for (const identity of contributors) { - const safe = await importWorkspaceFileSecretProvenanceForModelView({ - workspaceId: ctx.workspaceId, - identity, - registry: ctx.resolvedSecretTraceRegistry, - view: 'opaque', - ...(ctx.userId ? { actorUserId: ctx.userId } : {}), - }) - if (!safe) { - unsafeGeneratedDocumentFiles.add(`${file.key}:${file.id}`) - return - } - } - }, + const groups = new Map>() + message.files.forEach((file, index) => { + const workspaceFile = + ctx.principal?.kind === 'system' && tryInferContextFromKey(file.key) === 'workspace' + const group = groups.get(workspaceFile) ?? [] + group.push({ file, index }) + groups.set(workspaceFile, group) }) + const hydratedFiles = [...message.files] + await Promise.all( + [...groups.values()].map(async (group) => { + const hydrated = await hydrateUserFilesWithBase64( + group.map(({ file }) => file), + { + ...(await resolveExecutorFileMaterializationContext(ctx, group[0].file)), + logger, + maxBytes: inlineMaxBytes, + onServableFileContributors: async (file, contributors) => { + if (!ctx.workspaceId) return + for (const identity of contributors) { + const safe = await importWorkspaceFileSecretProvenanceForModelView({ + workspaceId: ctx.workspaceId, + identity, + registry: ctx.resolvedSecretTraceRegistry, + view: 'opaque', + ...(ctx.userId ? { actorUserId: ctx.userId } : {}), + }) + if (!safe) { + unsafeGeneratedDocumentFiles.add(`${file.key}:${file.id}`) + return + } + } + }, + } + ) + group.forEach(({ index }, fileIndex) => { + hydratedFiles[index] = hydrated[fileIndex] + }) + }) + ) const modelSafeHydratedFiles = hydratedFiles.flatMap((file, fileIndex) => { if (unsafeGeneratedDocumentFiles.has(`${file.key}:${file.id}`)) return [] 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 7c9ebf1a3f5..b13d1b7f773 100644 --- a/apps/sim/executor/handlers/agent/memory-harness.postgres.test.ts +++ b/apps/sim/executor/handlers/agent/memory-harness.postgres.test.ts @@ -64,13 +64,22 @@ vi.mock('@/lib/api-key/byok', () => ({ import { memory, memorySecretProvenance, + resumeQueue, + workflow, + workflowExecutionLogs, + workspace, workspaceFileSecretProvenance, workspaceFiles, } from '@sim/db/schema' import { hashDurableSecretProvenanceValue } from '@/lib/execution/durable-secret-provenance' import { uploadExecutionFile } from '@/lib/uploads/contexts/execution/execution-file-manager' -import { EXACT_EMPTY_WORKSPACE_FILE_SECRET_PROVENANCE } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' -import { deleteFile } from '@/lib/uploads/core/storage-service' +import { + EXACT_EMPTY_WORKSPACE_FILE_SECRET_PROVENANCE, + initializeWorkspaceFileSecretProvenanceInTx, +} from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' +import { deleteFile, uploadFile } from '@/lib/uploads/core/storage-service' +import { insertImmutableFileMetadata } from '@/lib/uploads/server/metadata' +import { readWorkspaceFileRecordByKey } from '@/lib/workspace-files/application/read-workspace-file-content-by-key' 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' @@ -176,7 +185,7 @@ async function executeTurn(ctx: ExecutionContext, inputs: AgentInputs): Promise< return drained.answerText } -/** Generate only the four production tables exercised here; unrelated application FKs are omitted. */ +/** Generate the production tables exercised here; unrelated application FKs are omitted. */ async function createTable(table: PgTable): Promise { if (!connection) throw new Error('Missing harness database') const dialect = new PgDialect() @@ -190,7 +199,9 @@ async function createTable(table: PgTable): Promise { column.default === undefined ? '' : ` DEFAULT ${dialect.sqlToQuery(defaultValue.inlineParams()).sql}` - return `"${column.name}" ${column.getSQLType()}${column.primary ? ' PRIMARY KEY' : ''}${column.notNull ? ' NOT NULL' : ''}${defaultSql}` + const type = + 'enumValues' in column && Array.isArray(column.enumValues) ? 'text' : column.getSQLType() + return `"${column.name}" ${type}${column.primary ? ' PRIMARY KEY' : ''}${column.notNull ? ' NOT NULL' : ''}${defaultSql}` }) await connection.unsafe(`CREATE TABLE "${config.name}" (${columns.join(', ')})`) } @@ -318,8 +329,27 @@ describe.skipIf(!databaseUrl)( memorySecretProvenance, workspaceFiles, workspaceFileSecretProvenance, + workspace, + workflow, + workflowExecutionLogs, + resumeQueue, ]) await createTable(table) + await fixture.database.insert(workspace).values({ + id: scope.workspaceId, + name: 'Attachment harness', + ownerId: scope.userId, + billedAccountUserId: scope.userId, + }) + await fixture.database.insert(workflow).values({ + id: scope.workflowId, + workspaceId: scope.workspaceId, + userId: scope.userId, + name: 'Attachment harness', + createdAt: new Date(), + updatedAt: new Date(), + lastSynced: new Date(), + }) await connection.unsafe( `CREATE UNIQUE INDEX memory_workspace_key_idx ON memory(workspace_id, key)` ) @@ -351,6 +381,170 @@ describe.skipIf(!databaseUrl)( } }) + it.each( + (['openai', 'anthropic'] as const).flatMap((provider) => + [false, true].map((streaming) => ({ provider, streaming })) + ) + )( + 'deployed chat reads remembered workspace files with $provider, streaming=$streaming', + async ({ provider, streaming }) => { + if (!fixture.database || !connection) throw new Error('Missing harness database') + vi.stubGlobal('fetch', interceptFetch) + outbound = [] + const conversationId = generateId() + const pdf = await PDFDocument.create() + pdf + .addPage() + .drawText('A workspace image-edit result can be recalled without a new upload.') + const buffer = Buffer.from(await pdf.save()) + const key = `workspace/${scope.workspaceId}/${generateId()}/result.pdf` + await uploadFile({ + file: buffer, + fileName: 'result.pdf', + contentType: 'application/pdf', + context: 'workspace', + preserveKey: true, + customKey: key, + persistMetadata: false, + }) + const record = await fixture.database.transaction(async (tx) => { + const record = await insertImmutableFileMetadata( + { + id: generateId(), + key, + userId: scope.userId, + workspaceId: scope.workspaceId, + context: 'workspace', + originalName: 'result.pdf', + contentType: 'application/pdf', + size: buffer.length, + }, + tx + ) + await initializeWorkspaceFileSecretProvenanceInTx( + tx, + record.id, + record.contentUpdatedAt, + EXACT_EMPTY_WORKSPACE_FILE_SECRET_PROVENANCE + ) + return record + }) + const file: UserFile = { + id: record.id, + name: 'result.pdf', + key, + url: '', + size: buffer.length, + type: 'application/pdf', + context: 'workspace', + } + const createChatContext = async () => { + const ctx = context(streaming) + ctx.principal = { + kind: 'system', + serviceId: 'chat', + workspaceId: scope.workspaceId, + workflowId: scope.workflowId, + } + const deploymentVersionId = generateId() + ctx.executorDelegationOrigin = { + workflowId: scope.workflowId, + executionId: ctx.executionId, + principal: ctx.principal, + currentWorkflow: { + workflowId: scope.workflowId, + mode: 'deployment', + deploymentVersionId, + }, + } + await fixture.database!.insert(workflowExecutionLogs).values({ + id: generateId(), + workflowId: scope.workflowId, + workspaceId: scope.workspaceId, + executionId: ctx.executionId!, + deploymentVersionId, + stateSnapshotId: generateId(), + level: 'info', + status: 'running', + trigger: 'chat', + startedAt: new Date(), + }) + return ctx + } + const inputs: AgentInputs = { + model: models[provider], + apiKey: apiKey(provider), + maxTokens: '128', + memoryType: 'conversation', + conversationId, + userPrompt: 'Read the attached PDF and reply exactly READY.', + } + transportReply = 'READY' + const firstContext = await createChatContext() + await expect( + readWorkspaceFileRecordByKey.execute({ + principal: firstContext.principal!, + input: { key, assertedWorkspaceId: scope.workspaceId }, + }) + ).rejects.toThrow('Principal kind system') + expect(await executeTurn(firstContext, { ...inputs, files: [file] })).toBe('READY') + expect(requestFiles(outbound[0])).toEqual([buffer.toString('base64')]) + const stored = await readConversation(conversationId) + expect(stored.data[0].files).toEqual([file]) + expect(JSON.stringify(stored)).not.toContain('base64') + + expect( + await executeTurn(await createChatContext(), { + ...inputs, + userPrompt: 'Read the earlier PDF again and reply exactly READY.', + }) + ).toBe('READY') + expect(requestFiles(outbound[1])).toEqual([buffer.toString('base64')]) + + const missingOrigin = await createChatContext() + missingOrigin.executorDelegationOrigin = undefined + await expect(executeTurn(missingOrigin, inputs)).rejects.toThrow() + const otherWorkspace = await createChatContext() + otherWorkspace.workspaceId = generateId() + await expect( + executeTurn(otherWorkspace, { ...inputs, files: [file], memoryType: 'none' }) + ).rejects.toThrow('could not be read') + const terminalRun = await createChatContext() + await connection`UPDATE workflow_execution_logs SET status = 'completed' WHERE execution_id = ${terminalRun.executionId!}` + await expect(executeTurn(terminalRun, inputs)).rejects.toThrow('active workflow execution') + const mismatchedDeployment = await createChatContext() + mismatchedDeployment.executorDelegationOrigin!.currentWorkflow = { + workflowId: scope.workflowId, + mode: 'deployment', + deploymentVersionId: generateId(), + } + await expect(executeTurn(mismatchedDeployment, inputs)).rejects.toThrow( + 'active workflow execution' + ) + await connection`UPDATE workspace_files SET deleted_at = NOW() WHERE id = ${file.id}` + await expect(executeTurn(await createChatContext(), inputs)).rejects.toThrow( + 'could not be read' + ) + expect(outbound).toHaveLength(2) + report.push({ + provider, + streaming, + workspaceAttachment: true, + stored, + controls: { + missingOrigin: 'blocked before HTTP', + differentWorkspace: 'blocked before HTTP', + terminalRun: 'blocked before HTTP', + mismatchedDeployment: 'blocked before HTTP', + deletedFile: 'blocked before HTTP', + }, + passed: true, + }) + await deleteFile({ key, context: 'workspace' }) + }, + 150_000 + ) + it.each( ( [ diff --git a/apps/sim/lib/internal/file/materialization-context.test.ts b/apps/sim/lib/internal/file/materialization-context.test.ts new file mode 100644 index 00000000000..881f3463dc4 --- /dev/null +++ b/apps/sim/lib/internal/file/materialization-context.test.ts @@ -0,0 +1,110 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { ExecutionContext } from '@/executor/types' + +const { bindDelegation } = vi.hoisted(() => ({ bindDelegation: vi.fn() })) + +vi.mock('@/lib/auth/internal-delegation', () => ({ + bindInternalExecutorDelegation: bindDelegation, +})) + +import { resolveExecutorFileMaterializationContext } from '@/lib/internal/file/materialization-context' + +const workspaceFile = { key: 'workspace/workspace-1/image.png' } +const systemPrincipal = { + kind: 'system', + serviceId: 'chat', + workspaceId: 'workspace-1', + workflowId: 'workflow-1', +} as const + +function context(): ExecutionContext { + return { + workflowId: 'workflow-1', + workspaceId: 'workspace-1', + executionId: 'execution-1', + userId: 'billing-owner', + principal: systemPrincipal, + executorDelegationOrigin: { + workflowId: 'workflow-1', + executionId: 'execution-1', + principal: systemPrincipal, + currentWorkflow: { + workflowId: 'workflow-1', + mode: 'deployment', + deploymentVersionId: 'deployment-1', + }, + }, + fileKeys: ['execution/workspace-1/workflow-1/prior/image.png'], + } as ExecutionContext +} + +describe('executor file materialization context', () => { + beforeEach(() => { + vi.clearAllMocks() + bindDelegation.mockResolvedValue({ kind: 'delegated', serviceId: 'executor' }) + }) + + it('binds actorless workspace reads to the current deployment without inventing a subject', async () => { + const ctx = context() + const result = await resolveExecutorFileMaterializationContext(ctx, workspaceFile) + expect(bindDelegation).toHaveBeenCalledWith( + expect.objectContaining({ + principal: systemPrincipal, + workflowId: 'workflow-1', + executionId: 'execution-1', + currentWorkflow: ctx.executorDelegationOrigin?.currentWorkflow, + }), + { audience: 'sim:workspace-files', compatibilityActorUserId: 'billing-owner' } + ) + expect(bindDelegation.mock.calls[0][0].subjectUserId).toBeUndefined() + expect(result.principal).toEqual({ kind: 'delegated', serviceId: 'executor' }) + expect(result.userId).toBeUndefined() + expect(result.fileKeys).toBe(ctx.fileKeys) + expect(ctx.principal).toBe(systemPrincipal) + }) + + it.each([ + { kind: 'session', userId: 'reader', sessionId: 'session-1' }, + { kind: 'personal_api_key', userId: 'reader', keyId: 'key-1' }, + { kind: 'workspace_api_key', workspaceId: 'workspace-1', keyId: 'key-1' }, + ] as const)('preserves the existing $kind workspace authority', async (principal) => { + const ctx = { ...context(), principal } + expect((await resolveExecutorFileMaterializationContext(ctx, workspaceFile)).principal).toBe( + principal + ) + expect(bindDelegation).not.toHaveBeenCalled() + }) + + it.each([ + 'execution/workspace-1/workflow-1/execution-1/image.png', + 'knowledge-base/document.png', + 'url/https://example.com/image.png', + '', + 'provider-file-id', + 'profile-pictures/avatar.png', + ])('does not replace the original identity for %s', async (key) => { + const ctx = context() + expect((await resolveExecutorFileMaterializationContext(ctx, { key })).principal).toBe( + systemPrincipal + ) + expect(bindDelegation).not.toHaveBeenCalled() + }) + + it('fails closed without a trusted executor origin', async () => { + const ctx = context() + ctx.executorDelegationOrigin = undefined + await expect(resolveExecutorFileMaterializationContext(ctx, workspaceFile)).rejects.toThrow() + expect(bindDelegation).not.toHaveBeenCalled() + }) + + it('propagates a failed current workflow binding without an owner fallback', async () => { + bindDelegation.mockRejectedValueOnce(new Error('Workflow binding invalid')) + await expect( + resolveExecutorFileMaterializationContext(context(), workspaceFile) + ).rejects.toThrow('Workflow binding invalid') + expect(bindDelegation).toHaveBeenCalledTimes(1) + }) +}) diff --git a/apps/sim/lib/internal/file/materialization-context.ts b/apps/sim/lib/internal/file/materialization-context.ts new file mode 100644 index 00000000000..8c19a647522 --- /dev/null +++ b/apps/sim/lib/internal/file/materialization-context.ts @@ -0,0 +1,33 @@ +import type { ExecutionMaterializationContext } from '@/lib/execution/payloads/materialization.server' +import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor' +import { tryInferContextFromKey } from '@/lib/uploads/utils/file-utils' +import { WORKSPACE_FILES_DELEGATION_AUDIENCE } from '@/lib/workspace-files/application/authorization' +import type { ExecutionContext, UserFile } from '@/executor/types' + +/** Binds actorless workspace reads to the same trusted execution authority as File tools. */ +export async function resolveExecutorFileMaterializationContext( + context: ExecutionContext, + file: Pick +): Promise { + const requiresDelegation = + context.principal?.kind === 'system' && tryInferContextFromKey(file.key) === 'workspace' + const principal = requiresDelegation + ? await createExecutorPrincipalFromExecutionContext({ + context, + audience: WORKSPACE_FILES_DELEGATION_AUDIENCE, + }) + : context.principal + + return { + principal, + workspaceId: context.workspaceId, + workflowId: context.workflowId, + executionId: context.executionId, + largeValueExecutionIds: context.largeValueExecutionIds, + largeValueKeys: context.largeValueKeys, + fileKeys: context.fileKeys, + allowLargeValueWorkflowScope: context.allowLargeValueWorkflowScope, + userId: requiresDelegation ? undefined : context.userId, + requestId: context.executionId || context.workflowId || 'agent-files', + } +} diff --git a/apps/sim/providers/file-attachments.server.test.ts b/apps/sim/providers/file-attachments.server.test.ts index f88a10bf5f3..1e36a22f4b4 100644 --- a/apps/sim/providers/file-attachments.server.test.ts +++ b/apps/sim/providers/file-attachments.server.test.ts @@ -2,9 +2,11 @@ * @vitest-environment node */ import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { ExecutionContext } from '@/executor/types' import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' import { buildOpenAIMessageContent, + getProviderFileStrategy, INLINE_ATTACHMENT_THRESHOLD_BYTES, LARGE_FILE_PATH_THRESHOLD_BYTES, } from '@/providers/attachments' @@ -13,6 +15,7 @@ import { getInlineHydrationMaxBytes, uploadLargeFilesToProvider, } from '@/providers/file-attachments.server' +import { PROVIDER_DEFINITIONS } from '@/providers/models' import { runWithProviderRuntimeContext } from '@/providers/runtime-context' import type { ProviderRequest } from '@/providers/types' @@ -21,16 +24,32 @@ const { mockGeneratePresignedDownloadUrl, mockHasCloudStorage, mockVerifyFileAccess, + mockCreateExecutorPrincipal, + mockAssertUserFileContentAccess, + mockGoogleUpload, } = vi.hoisted(() => ({ mockDownloadServableFileFromStorage: vi.fn(), mockGeneratePresignedDownloadUrl: vi.fn(), mockHasCloudStorage: vi.fn(), mockVerifyFileAccess: vi.fn(), + mockCreateExecutorPrincipal: vi.fn(), + mockAssertUserFileContentAccess: vi.fn(), + mockGoogleUpload: vi.fn(), +})) + +vi.mock('@/lib/internal/principals/executor', () => ({ + createExecutorPrincipalFromExecutionContext: mockCreateExecutorPrincipal, +})) + +vi.mock('@/lib/execution/payloads/materialization.server', () => ({ + assertUserFileContentAccess: mockAssertUserFileContentAccess, })) vi.mock('@google/genai', () => ({ FileState: { PROCESSING: 'PROCESSING', FAILED: 'FAILED' }, - GoogleGenAI: class {}, + GoogleGenAI: class { + files = { upload: mockGoogleUpload } + }, })) vi.mock('@/lib/uploads', () => ({ @@ -82,6 +101,17 @@ describe('OpenAI large-file attachment lifecycle', () => { vi.clearAllMocks() mockHasCloudStorage.mockReturnValue(true) mockVerifyFileAccess.mockResolvedValue(true) + mockCreateExecutorPrincipal.mockResolvedValue({ + kind: 'delegated', + serviceId: 'executor', + workspaceId: 'workspace-1', + }) + mockAssertUserFileContentAccess.mockResolvedValue(undefined) + mockGoogleUpload.mockResolvedValue({ + name: 'files/harness', + uri: 'https://generativelanguage.googleapis.com/files/harness', + state: 'ACTIVE', + }) mockGeneratePresignedDownloadUrl.mockResolvedValue('https://storage.example.com/signed') mockDownloadServableFileFromStorage.mockResolvedValue({ buffer: Buffer.alloc(CSV_BYTES, 0x61), @@ -185,4 +215,93 @@ describe('OpenAI large-file attachment lifecycle', () => { expect(file?.remoteUrl).toBeUndefined() expect(file?.providerFileId).toBeUndefined() }) + + const executionContext = { + workflowId: 'workflow-1', + workspaceId: 'workspace-1', + executionId: 'execution-1', + userId: 'billing-owner', + principal: { + kind: 'system', + serviceId: 'chat', + workflowId: 'workflow-1', + workspaceId: 'workspace-1', + }, + executorDelegationOrigin: { workflowId: 'workflow-1', executionId: 'execution-1' }, + } as ExecutionContext + + it.each(Object.keys(PROVIDER_DEFINITIONS))( + 'preserves %s attachment strategy while authorizing remote bytes as the execution', + async (provider) => { + const request = makeRequest(INLINE_ATTACHMENT_THRESHOLD_BYTES + 1) + await attachLargeFileRemoteUrls(request, provider, executionContext) + const largeFile = getProviderFileStrategy(provider) !== 'inline' + expect(mockCreateExecutorPrincipal).toHaveBeenCalledTimes(largeFile ? 1 : 0) + expect(mockAssertUserFileContentAccess).toHaveBeenCalledTimes(largeFile ? 1 : 0) + expect(mockGeneratePresignedDownloadUrl).toHaveBeenCalledTimes(largeFile ? 1 : 0) + expect(mockVerifyFileAccess).not.toHaveBeenCalled() + if (largeFile) { + expect(mockCreateExecutorPrincipal).toHaveBeenCalledWith({ + context: executionContext, + audience: 'sim:workspace-files', + }) + expect(mockAssertUserFileContentAccess).toHaveBeenCalledWith( + request.messages?.[0].files?.[0], + expect.objectContaining({ + principal: { kind: 'delegated', serviceId: 'executor', workspaceId: 'workspace-1' }, + userId: undefined, + executionId: 'execution-1', + }) + ) + } + } + ) + + it('rechecks current access before a Files API upload and does not fall back to the billing owner', async () => { + const request = makeRequest(CSV_BYTES) + await attachLargeFileRemoteUrls(request, 'openai', executionContext) + mockAssertUserFileContentAccess.mockRejectedValueOnce(new Error('Access revoked')) + await expect(uploadLargeFilesToProvider(request, 'openai', executionContext)).rejects.toThrow( + 'Access revoked' + ) + expect(mockCreateExecutorPrincipal).toHaveBeenCalledTimes(2) + expect(mockDownloadServableFileFromStorage).not.toHaveBeenCalled() + expect(mockVerifyFileAccess).not.toHaveBeenCalled() + expect(fetch).not.toHaveBeenCalled() + }) + + it.each(['openai', 'google'])( + 'uploads an authorized actorless workspace file through %s', + async (provider) => { + const request = makeRequest(CSV_BYTES) + await attachLargeFileRemoteUrls(request, provider, executionContext) + await uploadLargeFilesToProvider(request, provider, executionContext) + expect(mockCreateExecutorPrincipal).toHaveBeenCalledTimes(2) + expect(mockAssertUserFileContentAccess).toHaveBeenCalledTimes(2) + expect(mockVerifyFileAccess).not.toHaveBeenCalled() + const file = request.messages?.[0].files?.[0] + if (provider === 'openai') expect(file?.providerFileId).toBe('file-abc') + else + expect(file?.providerFileUri).toBe( + 'https://generativelanguage.googleapis.com/files/harness' + ) + } + ) + + it('does not mint a remote URL after execution authorization fails', async () => { + mockCreateExecutorPrincipal.mockRejectedValueOnce(new Error('Run no longer active')) + await expect( + attachLargeFileRemoteUrls(makeRequest(CSV_BYTES), 'openai', executionContext) + ).rejects.toThrow('Run no longer active') + expect(mockGeneratePresignedDownloadUrl).not.toHaveBeenCalled() + expect(mockVerifyFileAccess).not.toHaveBeenCalled() + }) + + it('ignores forged execution authority on an ordinary provider request', async () => { + const request = { ...makeRequest(CSV_BYTES), executionContext } + mockVerifyFileAccess.mockResolvedValueOnce(false) + await expect(attachLargeFileRemoteUrls(request, 'openai')).rejects.toThrow('not accessible') + expect(mockCreateExecutorPrincipal).not.toHaveBeenCalled() + expect(mockGeneratePresignedDownloadUrl).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/providers/file-attachments.server.ts b/apps/sim/providers/file-attachments.server.ts index 39fcbf9267e..6fad9893e81 100644 --- a/apps/sim/providers/file-attachments.server.ts +++ b/apps/sim/providers/file-attachments.server.ts @@ -2,11 +2,13 @@ import { FileState, GoogleGenAI } from '@google/genai' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { sleep } from '@sim/utils/helpers' +import { assertUserFileContentAccess } from '@/lib/execution/payloads/materialization.server' +import { resolveExecutorFileMaterializationContext } from '@/lib/internal/file/materialization-context' import { StorageService } from '@/lib/uploads' import { resolveTrustedFileContext } from '@/lib/uploads/utils/file-utils' import { downloadServableFileFromStorage } from '@/lib/uploads/utils/file-utils.server' import { verifyFileAccess } from '@/app/api/files/authorization' -import type { UserFile } from '@/executor/types' +import type { ExecutionContext, UserFile } from '@/executor/types' import { formatAttachmentSizes, getProviderAttachmentMaxBytes, @@ -72,7 +74,8 @@ export function canUseProviderLargeFilePath(providerId: ProviderId | string): bo */ export async function attachLargeFileRemoteUrls( request: ProviderRequest, - providerId: ProviderId | string + providerId: ProviderId | string, + executionContext?: ExecutionContext ): Promise { for (const file of iterateRequestFiles(request.messages)) { file.providerFileId = undefined @@ -102,16 +105,21 @@ export async function attachLargeFileRemoteUrls( continue } - if (!request.userId) { - throw new Error( - `File "${file.name}" requires an authenticated user for provider "${providerId}"` - ) - } - - const context = resolveTrustedFileContext(file.key, file.context) - const hasAccess = await verifyFileAccess(file.key, request.userId, undefined, context, false) - if (!hasAccess) { - throw new Error(`File "${file.name}" is not accessible for provider "${providerId}"`) + let context: ReturnType + if (executionContext) { + context = resolveTrustedFileContext(file.key, file.context) + await assertFileAccessForUpload(file, request.userId, executionContext) + } else { + if (!request.userId) { + throw new Error( + `File "${file.name}" requires an authenticated user for provider "${providerId}"` + ) + } + context = resolveTrustedFileContext(file.key, file.context) + const hasAccess = await verifyFileAccess(file.key, request.userId, undefined, context, false) + if (!hasAccess) { + throw new Error(`File "${file.name}" is not accessible for provider "${providerId}"`) + } } file.remoteUrl = await StorageService.generatePresignedDownloadUrl( @@ -130,7 +138,8 @@ export async function attachLargeFileRemoteUrls( */ export async function uploadLargeFilesToProvider( request: ProviderRequest, - providerId: ProviderId | string + providerId: ProviderId | string, + executionContext?: ExecutionContext ): Promise { if (getProviderFileStrategy(providerId) !== 'files-api') return @@ -142,7 +151,7 @@ export async function uploadLargeFilesToProvider( for (const group of groups) { const [representative] = group - await assertFileAccessForUpload(representative, request.userId) + await assertFileAccessForUpload(representative, request.userId, executionContext) if (providerId === 'openai') { await uploadOpenAIFile(representative, request.apiKey, maxBytes, request.abortSignal) } else if (ai) { @@ -162,11 +171,19 @@ export async function uploadLargeFilesToProvider( */ async function assertFileAccessForUpload( file: UserFile, - userId: string | undefined + userId: string | undefined, + executionContext?: ExecutionContext ): Promise { if (!file.key) { throw new Error(`File "${file.name}" has no storage key`) } + if (executionContext) { + await assertUserFileContentAccess( + file, + await resolveExecutorFileMaterializationContext(executionContext, file) + ) + return + } if (!userId) { throw new Error(`File "${file.name}" requires an authenticated user to upload`) } diff --git a/apps/sim/providers/index.test.ts b/apps/sim/providers/index.test.ts index e7219714432..5594b448d99 100644 --- a/apps/sim/providers/index.test.ts +++ b/apps/sim/providers/index.test.ts @@ -45,7 +45,7 @@ vi.mock('@/tools', () => ({ executeTool: (...args: unknown[]) => mockExecuteTool(...args), })) -import type { NormalizedBlockOutput, StreamingExecution } from '@/executor/types' +import type { ExecutionContext, NormalizedBlockOutput, StreamingExecution } from '@/executor/types' import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' import { executeProviderRequest } from '@/providers' import { executeProviderTool } from '@/providers/runtime-context' @@ -114,6 +114,28 @@ describe('executeProviderRequest — tool identities', () => { vi.clearAllMocks() }) + it('passes trusted execution context to both attachment authorization stages without serializing it', async () => { + const executionContext = { + workflowId: 'workflow-1', + workspaceId: 'workspace-1', + executionId: 'execution-1', + } as ExecutionContext + mockExecuteRequest.mockResolvedValueOnce({ content: 'ready', model: 'test-model' }) + await executeProviderRequest('anthropic', { model: 'test-model' }, { executionContext }) + expect(mockAttachLargeFileRemoteUrls).toHaveBeenCalledWith( + expect.objectContaining({ model: 'test-model' }), + 'anthropic', + executionContext + ) + expect(mockUploadLargeFilesToProvider).toHaveBeenCalledWith( + expect.objectContaining({ model: 'test-model' }), + 'anthropic', + executionContext + ) + expect(mockExecuteRequest.mock.calls[0][0]).not.toHaveProperty('executionContext') + expect(mockExecuteRequest.mock.calls[0][0]).not.toHaveProperty('principal') + }) + it('sends unique opaque ids and projects provider aliases out of the response', async () => { const tools = [ makeProviderTool('gmail_send', 'credential-a'), diff --git a/apps/sim/providers/index.ts b/apps/sim/providers/index.ts index 8c7b315f677..5d1f5819a9d 100644 --- a/apps/sim/providers/index.ts +++ b/apps/sim/providers/index.ts @@ -265,8 +265,8 @@ export async function executeProviderRequest( } const response = await runWithProviderRuntimeContext(requestRuntimeContext, async () => { - await attachLargeFileRemoteUrls(modelSafeRequest, providerId) - await uploadLargeFilesToProvider(modelSafeRequest, providerId) + await attachLargeFileRemoteUrls(modelSafeRequest, providerId, runtimeContext?.executionContext) + await uploadLargeFilesToProvider(modelSafeRequest, providerId, runtimeContext?.executionContext) return provider.executeRequest(modelSafeRequest) }) From 95310455b2bc28b2b0ad7e1a4d5031476e3801a9 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 15 Sep 2026 13:59:19 -0700 Subject: [PATCH 2/2] test(agent): preserve rejection of unprefixed attachment keys --- .../file-attachments-authorization.test.ts | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 apps/sim/providers/file-attachments-authorization.test.ts diff --git a/apps/sim/providers/file-attachments-authorization.test.ts b/apps/sim/providers/file-attachments-authorization.test.ts new file mode 100644 index 00000000000..78e8260fcc9 --- /dev/null +++ b/apps/sim/providers/file-attachments-authorization.test.ts @@ -0,0 +1,97 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { ExecutionContext, UserFile } from '@/executor/types' + +const { presign, download, metadata, permission } = vi.hoisted(() => ({ + presign: vi.fn(), + download: vi.fn(), + metadata: vi.fn(), + permission: vi.fn(), +})) + +vi.mock('@/lib/uploads', () => ({ + StorageService: { hasCloudStorage: () => true, generatePresignedDownloadUrl: presign }, + getFileMetadata: metadata, +})) + +vi.mock('@/lib/uploads/utils/file-utils.server', () => ({ + downloadServableFileFromStorage: download, +})) + +vi.mock('@/lib/uploads/server/metadata', () => ({ + getFileMetadataByKey: metadata, +})) + +vi.mock('@/lib/workspaces/permissions/utils', () => ({ + getUserEntityPermissions: permission, +})) + +import { resolveTrustedFileContext } from '@/lib/uploads/utils/file-utils' +import { + attachLargeFileRemoteUrls, + uploadLargeFilesToProvider, +} from '@/providers/file-attachments.server' +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() + }) + + it.each( + (['workspace', 'execution', 'chat', 'copilot', 'knowledge-base'] as const).flatMap((context) => + (['standalone', 'session', 'system'] as const).map((caller) => ({ context, caller })) + ) + )( + 'rejects unprefixed $context keys for $caller before reading or signing bytes', + async ({ context, caller }) => { + const file: UserFile = { + id: 'file-1', + name: 'document.pdf', + key: 'legacy-file-id/document.pdf', + url: '', + size: 10 * 1024 * 1024, + type: 'application/pdf', + context, + } + const request: ProviderRequest = { + model: 'gpt-4.1', + userId: 'billing-owner', + messages: [{ role: 'user', content: 'Read this file', files: [file] }], + } + const executionContext = + caller === 'standalone' + ? undefined + : ({ + workflowId: 'workflow-1', + workspaceId: 'workspace-1', + executionId: 'execution-1', + userId: 'billing-owner', + principal: + caller === 'session' + ? { kind: 'session', userId: 'acting-user', sessionId: 'session-1' } + : { + kind: 'system', + serviceId: 'chat', + workspaceId: 'workspace-1', + workflowId: 'workflow-1', + }, + } as ExecutionContext) + + expect(resolveTrustedFileContext(file.key, file.context)).toBe(context) + await expect(attachLargeFileRemoteUrls(request, 'openai', executionContext)).rejects.toThrow() + expect(presign).not.toHaveBeenCalled() + + file.remoteUrl = 'https://storage.example.com/forged' + await expect( + uploadLargeFilesToProvider(request, 'openai', executionContext) + ).rejects.toThrow() + expect(download).not.toHaveBeenCalled() + expect(metadata).not.toHaveBeenCalled() + expect(permission).not.toHaveBeenCalled() + } + ) +})