From 225079c850c71fce3cb8bed7b57dc76370a156b3 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 15 Sep 2026 16:34:54 -0700 Subject: [PATCH 1/3] fix(provenance): stop silently dropping model attachments --- .../handlers/agent/agent-handler.test.ts | 105 ++++++++++-------- .../executor/handlers/agent/agent-handler.ts | 37 +++--- .../lib/copilot/request/lifecycle/run.test.ts | 52 ++++++--- apps/sim/lib/copilot/request/lifecycle/run.ts | 44 +++----- apps/sim/providers/index.test.ts | 76 ++++++++----- apps/sim/providers/index.ts | 45 +++----- 6 files changed, 189 insertions(+), 170 deletions(-) diff --git a/apps/sim/executor/handlers/agent/agent-handler.test.ts b/apps/sim/executor/handlers/agent/agent-handler.test.ts index edd09b45fd2..8362a4bae76 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.test.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.test.ts @@ -53,6 +53,8 @@ vi.mock('@/lib/internal/mcp/discover-tools', () => ({ })) vi.mock('@/lib/uploads/contexts/workspace/workspace-file-secret-provenance', () => ({ + MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE: + 'File cannot be sent to a model because its secret provenance is unavailable', importWorkspaceFileSecretProvenanceForModelView: mockImportWorkspaceFileSecretProvenanceForModelView, })) @@ -1040,55 +1042,68 @@ describe('AgentBlockHandler', () => { } }) - it('omits only a generated document whose embedded contributor is not model-safe', async () => { - const key = 'workspace/ws-1/report.pdf' - mockContext.workspaceId = 'ws-1' - const hydrationSpy = vi - .spyOn(userFileBase64, 'hydrateUserFilesWithBase64') - .mockImplementationOnce(async (files, options) => { - await options.onServableFileContributors?.(files[0], [ - { - fileId: 'image-1', - key: 'workspace/ws-1/image-1.png', - context: 'workspace', - contentUpdatedAt: new Date('2026-08-06T00:00:00.000Z'), - }, - ]) - return files.map((file) => ({ ...file, base64: 'JVBERi0=' })) - }) - mockImportWorkspaceFileSecretProvenanceForModelView.mockResolvedValueOnce(false) + it.each([false, true])( + 'honors generated document contributor admission (safe=%s)', + async (safe) => { + const key = 'workspace/ws-1/report.pdf' + mockContext.workspaceId = 'ws-1' + const hydrationSpy = vi + .spyOn(userFileBase64, 'hydrateUserFilesWithBase64') + .mockImplementationOnce(async (files, options) => { + await options.onServableFileContributors?.(files[0], [ + { + fileId: 'image-1', + key: 'workspace/ws-1/image-1.png', + context: 'workspace', + contentUpdatedAt: new Date('2026-08-06T00:00:00.000Z'), + }, + ]) + return files.map((file) => ({ ...file, base64: 'JVBERi0=' })) + }) + mockImportWorkspaceFileSecretProvenanceForModelView.mockResolvedValueOnce(safe) - try { - mockGetProviderFromModel.mockReturnValue('openai') + try { + mockGetProviderFromModel.mockReturnValue('openai') - await handler.execute(mockContext, mockBlock, { - model: 'gpt-4o', - userPrompt: 'Analyze this document', - files: [ - { - id: 'file-1', - name: 'report.pdf', - path: `/api/files/serve/${encodeURIComponent(key)}?context=workspace`, - key, - size: 128, - type: 'text/x-python-pdf', - }, - ], - apiKey: 'test-api-key', - }) - - expect(mockExecuteProviderRequest.mock.calls[0][1].messages.at(-1)?.files).toEqual([]) - expect(mockImportWorkspaceFileSecretProvenanceForModelView).toHaveBeenCalledWith( - expect.objectContaining({ - workspaceId: mockContext.workspaceId, - view: 'opaque', - identity: expect.objectContaining({ fileId: 'image-1' }), + const execution = handler.execute(mockContext, mockBlock, { + model: 'gpt-4o', + userPrompt: 'Analyze this document', + files: [ + { + id: 'file-1', + name: 'report.pdf', + path: `/api/files/serve/${encodeURIComponent(key)}?context=workspace`, + key, + size: 128, + type: 'text/x-python-pdf', + }, + ], + apiKey: 'test-api-key', }) - ) - } finally { - hydrationSpy.mockRestore() + + if (safe) { + await execution + expect(mockExecuteProviderRequest.mock.calls[0][1].messages.at(-1)?.files).toEqual([ + expect.objectContaining({ key, base64: 'JVBERi0=' }), + ]) + } else { + await expect(execution).rejects.toThrow( + 'File cannot be sent to a model because its secret provenance is unavailable' + ) + expect(mockExecuteProviderRequest).not.toHaveBeenCalled() + } + expect(mockImportWorkspaceFileSecretProvenanceForModelView).toHaveBeenCalledWith( + expect.objectContaining({ + workspaceId: mockContext.workspaceId, + view: 'opaque', + identity: expect.objectContaining({ fileId: 'image-1' }), + }) + ) + } finally { + hydrationSpy.mockRestore() + } } - }) + ) it('should reject files for providers without attachment support', async () => { const inputs = { diff --git a/apps/sim/executor/handlers/agent/agent-handler.ts b/apps/sim/executor/handlers/agent/agent-handler.ts index f33ff7b8ea1..c1bb7d2f111 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.ts @@ -26,7 +26,10 @@ import { resolveAutoModel, SIM_AUTO_SYSTEM_PREAMBLE, } from '@/lib/model-router/resolve' -import { importWorkspaceFileSecretProvenanceForModelView } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' +import { + importWorkspaceFileSecretProvenanceForModelView, + MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE, +} from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' import { getFileExtension, MODEL_SUPPORTED_IMAGE_MIME_TYPES, @@ -1493,7 +1496,6 @@ export class AgentBlockHandler implements BlockHandler { continue } - const unsafeGeneratedDocumentFiles = new Set() const groups = new Map>() message.files.forEach((file, index) => { const workspaceFile = @@ -1511,7 +1513,7 @@ export class AgentBlockHandler implements BlockHandler { ...(await resolveExecutorFileMaterializationContext(ctx, group[0].file)), logger, maxBytes: inlineMaxBytes, - onServableFileContributors: async (file, contributors) => { + onServableFileContributors: async (_file, contributors) => { if (!ctx.workspaceId) return for (const identity of contributors) { const safe = await importWorkspaceFileSecretProvenanceForModelView({ @@ -1522,8 +1524,7 @@ export class AgentBlockHandler implements BlockHandler { ...(ctx.userId ? { actorUserId: ctx.userId } : {}), }) if (!safe) { - unsafeGeneratedDocumentFiles.add(`${file.key}:${file.id}`) - return + throw new Error(MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE) } } }, @@ -1535,9 +1536,7 @@ export class AgentBlockHandler implements BlockHandler { }) ) - const modelSafeHydratedFiles = hydratedFiles.flatMap((file, fileIndex) => { - if (unsafeGeneratedDocumentFiles.has(`${file.key}:${file.id}`)) return [] - + const modelSafeHydratedFiles = hydratedFiles.map((file, fileIndex) => { const sourceFile = message.files?.[fileIndex] const nameProjection = sourceFile ? projectedNameByFile.get(sourceFile) : undefined if ( @@ -1546,7 +1545,7 @@ export class AgentBlockHandler implements BlockHandler { largeFilePathAvailable: canUseProviderLargeFilePath(providerId), }) ) { - return [file] + return file } if (nameProjection.inputPath) modelBoundInputPaths.push(nameProjection.inputPath) @@ -1554,22 +1553,12 @@ export class AgentBlockHandler implements BlockHandler { const suffix = extension ? `.${extension}` : '' const keepsSuffix = suffix !== '' && nameProjection.name.toLowerCase().endsWith(suffix.toLowerCase()) - return [ - { - ...file, - name: - suffix !== '' && !keepsSuffix - ? `${nameProjection.name}${suffix}` - : nameProjection.name, - }, - ] + return { + ...file, + name: + suffix !== '' && !keepsSuffix ? `${nameProjection.name}${suffix}` : nameProjection.name, + } }) - if (modelSafeHydratedFiles.length !== hydratedFiles.length) { - logger.warn('Omitting generated document attachments with unsafe contributor provenance', { - omittedCount: hydratedFiles.length - modelSafeHydratedFiles.length, - attachmentCount: hydratedFiles.length, - }) - } const missingFile = modelSafeHydratedFiles.find( (file) => diff --git a/apps/sim/lib/copilot/request/lifecycle/run.test.ts b/apps/sim/lib/copilot/request/lifecycle/run.test.ts index 5f8f74b27a1..9a614aa9f30 100644 --- a/apps/sim/lib/copilot/request/lifecycle/run.test.ts +++ b/apps/sim/lib/copilot/request/lifecycle/run.test.ts @@ -50,6 +50,8 @@ vi.mock('@/lib/copilot/application/load-search-integrations', () => ({ })) vi.mock('@/lib/uploads/contexts/workspace/workspace-file-secret-provenance', () => ({ + MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE: + 'File cannot be sent to a model because its secret provenance is unavailable', filterModelSafeWorkspaceFileAttachments: (...args: unknown[]) => mockFilterModelSafeWorkspaceFileAttachments(...args), })) @@ -665,31 +667,45 @@ describe('runCopilotLifecycle', () => { expect(sent.fileAttachments).toEqual([{ name: 'TOKEN.txt', key: 'safe-key' }]) }) - it('omits only unsafe durable attachments before the initial Go request', async () => { - const unsafe = { id: 'wf-unsafe', name: 'unsafe.txt', key: 'workspace/ws-1/unsafe.txt' } - const safe = { id: 'wf-safe', name: 'safe.txt', key: 'workspace/ws-1/safe.txt' } - mockFilterModelSafeWorkspaceFileAttachments.mockResolvedValueOnce([safe]) - let capturedRequestBody = '' - mockRunStreamLoop.mockImplementationOnce(async (_url: string, request: RequestInit) => { - capturedRequestBody = String(request.body) - }) - - await runCopilotLifecycle( - { + it.each([ + { key: 'attachments', includeSafeFile: false }, + { key: 'attachments', includeSafeFile: true }, + { key: 'fileAttachments', includeSafeFile: false }, + { key: 'fileAttachments', includeSafeFile: true }, + ])( + 'rejects refused initial $key before the Go request (mixed=$includeSafeFile)', + async ({ key, includeSafeFile }) => { + const unsafe = { id: 'wf-unsafe', name: 'unsafe.txt', key: 'workspace/ws-1/unsafe.txt' } + const safe = { id: 'wf-safe', name: 'safe.txt', key: 'workspace/ws-1/safe.txt' } + const safeFiles = includeSafeFile ? [safe] : [] + mockFilterModelSafeWorkspaceFileAttachments.mockResolvedValueOnce(safeFiles) + const onError = vi.fn() + const payload = { message: 'Review files', - fileAttachments: [unsafe, safe], + [key]: [...safeFiles, unsafe], workspaceId: 'ws-1', messageId: 'stream-file-provenance', - }, - { + } + const originalPayload = structuredClone(payload) + + const result = await runCopilotLifecycle(payload, { userId: 'user-1', workspaceId: 'ws-1', executionContext: { userId: 'user-1', workflowId: '', workspaceId: 'ws-1' }, - } - ) + onError, + }) - expect(JSON.parse(capturedRequestBody).fileAttachments).toEqual([safe]) - }) + const message = 'File cannot be sent to a model because its secret provenance is unavailable' + expect(result).toMatchObject({ success: false, error: message }) + expect(onError).toHaveBeenCalledWith(expect.objectContaining({ message }), result) + expect(mockFilterModelSafeWorkspaceFileAttachments).toHaveBeenCalledWith( + [...safeFiles, unsafe], + { workspaceId: 'ws-1' } + ) + expect(payload).toEqual(originalPayload) + expect(mockRunStreamLoop).not.toHaveBeenCalled() + } + ) it('rejects when durable attachment provenance cannot be verified', async () => { mockFilterModelSafeWorkspaceFileAttachments.mockRejectedValueOnce(new Error('db unavailable')) diff --git a/apps/sim/lib/copilot/request/lifecycle/run.ts b/apps/sim/lib/copilot/request/lifecycle/run.ts index d9b090fd48a..281f1762421 100644 --- a/apps/sim/lib/copilot/request/lifecycle/run.ts +++ b/apps/sim/lib/copilot/request/lifecycle/run.ts @@ -4,7 +4,6 @@ import type { PermissionType } from '@sim/platform-authz/workspace' import { getErrorMessage, toError } from '@sim/utils/errors' import { interruptibleSleep, sleep } from '@sim/utils/helpers' import { generateId } from '@sim/utils/id' -import { omit } from '@sim/utils/object' import { workspaceSearchFiltersSchema } from '@/lib/api/contracts/knowledge/search' import { type AttributedBillingRequestEnvelope, @@ -71,7 +70,10 @@ import { prepareExecutionContext } from '@/lib/copilot/tools/handlers/context' import { env } from '@/lib/core/config/env' import { isCopilotToolPermissionsEnabled, isHosted } from '@/lib/core/config/env-flags' import { isWorkspaceCapabilityWithheld } from '@/lib/permission-groups/capability-assertions' -import { filterModelSafeWorkspaceFileAttachments } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' +import { + filterModelSafeWorkspaceFileAttachments, + MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE, +} from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' import type { ExecutorDelegationOrigin } from '@/executor/types' import { refuseResolvedSecretProjection } from '@/executor/utils/resolved-secret-projection-refusal' import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' @@ -95,14 +97,13 @@ class CopilotModelContentProjectionError extends Error { } } -async function omitUnsafeInitialCopilotAttachments( +async function assertModelSafeInitialCopilotAttachments( payload: Record, workspaceId?: string -): Promise> { - let projected = payload +): Promise { for (const key of ['attachments', 'fileAttachments'] as const) { - if (!Object.hasOwn(projected, key)) continue - const attachments = projected[key] + if (!Object.hasOwn(payload, key)) continue + const attachments = payload[key] if (!Array.isArray(attachments)) { refuseResolvedSecretProjection({ site: 'copilot.initialAttachmentsShape', @@ -128,22 +129,14 @@ async function omitUnsafeInitialCopilotAttachments( }) } - if (safeAttachments.length === attachments.length) continue - logger.warn('Omitting Copilot attachments with unsafe secret provenance', { - attachmentCount: attachments.length, - omittedCount: attachments.length - safeAttachments.length, - }) - projected = - safeAttachments.length > 0 ? { ...projected, [key]: safeAttachments } : omit(projected, [key]) + if (safeAttachments.length !== attachments.length) { + refuseResolvedSecretProjection({ + site: 'copilot.initialAttachmentsProvenance', + message: MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE, + inputPath: key, + }) + } } - return projected -} - -async function filterInitialCopilotAttachmentsForModel( - payload: Record, - workspaceId?: string -): Promise> { - return omitUnsafeInitialCopilotAttachments(payload, workspaceId) } async function ensureModelEgressRegistry( @@ -412,12 +405,9 @@ export async function runCopilotLifecycle( }), } } - const modelSafeRequestPayload = await filterInitialCopilotAttachmentsForModel( - requestPayload, - lifecycleOptions.workspaceId - ) + await assertModelSafeInitialCopilotAttachments(requestPayload, lifecycleOptions.workspaceId) await runCheckpointLoop( - modelSafeRequestPayload, + requestPayload, context, execContext, lifecycleOptions, diff --git a/apps/sim/providers/index.test.ts b/apps/sim/providers/index.test.ts index 5594b448d99..38bbc273e5f 100644 --- a/apps/sim/providers/index.test.ts +++ b/apps/sim/providers/index.test.ts @@ -37,6 +37,8 @@ vi.mock('@/providers/file-attachments.server', () => ({ })) vi.mock('@/lib/uploads/contexts/workspace/workspace-file-secret-provenance', () => ({ + MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE: + 'File cannot be sent to a model because its secret provenance is unavailable', filterModelSafeWorkspaceFileAttachments: (...args: unknown[]) => mockFilterModelSafeWorkspaceFileAttachments(...args), })) @@ -963,35 +965,53 @@ describe('executeProviderRequest — caller-prepared model input', () => { }) }) - it('omits only unsafe durable files before any provider attachment processing', async () => { - const unsafe = { - id: 'wf-unsafe', - name: 'unsafe.txt', - url: '/unsafe', - size: 10, - type: 'text/plain', - key: 'workspace/ws-1/unsafe.txt', - } - const safe = { - id: 'wf-safe', - name: 'safe.txt', - url: '/safe', - size: 10, - type: 'text/plain', - key: 'workspace/ws-1/safe.txt', - } - mockFilterModelSafeWorkspaceFileAttachments.mockResolvedValueOnce([safe]) - - await executeProviderRequest('openai', { - model: 'test-model', - workspaceId: 'ws-1', - messages: [{ role: 'user', content: 'Review files', files: [unsafe, safe] }], - }) + it.each([ + { stream: false, includeSafeFile: false }, + { stream: false, includeSafeFile: true }, + { stream: true, includeSafeFile: false }, + { stream: true, includeSafeFile: true }, + ])( + 'rejects refused attachments before provider processing (stream=$stream, mixed=$includeSafeFile)', + async ({ stream, includeSafeFile }) => { + const unsafe = { + id: 'wf-unsafe', + name: 'unsafe.txt', + url: '/unsafe', + size: 10, + type: 'text/plain', + key: 'workspace/ws-1/unsafe.txt', + } + const safe = { ...unsafe, id: 'wf-safe', key: 'workspace/ws-1/safe.txt' } + const safeFiles = includeSafeFile ? [safe] : [] + mockFilterModelSafeWorkspaceFileAttachments.mockResolvedValueOnce(safeFiles) + const messages = [ + { role: 'user' as const, content: 'Review files', files: safeFiles }, + { role: 'user' as const, content: 'Include this file too', files: [unsafe] }, + ] + const originalMessages = structuredClone(messages) + + await expect( + executeProviderRequest('openai', { + model: 'test-model', + workspaceId: 'ws-1', + userId: 'user-1', + stream, + messages, + }) + ).rejects.toThrow( + 'File cannot be sent to a model because its secret provenance is unavailable' + ) - expect(mockAttachLargeFileRemoteUrls.mock.calls[0][0].messages[0].files).toEqual([safe]) - expect(mockUploadLargeFilesToProvider.mock.calls[0][0].messages[0].files).toEqual([safe]) - expect(mockExecuteRequest.mock.calls[0][0].messages[0].files).toEqual([safe]) - }) + expect(mockFilterModelSafeWorkspaceFileAttachments).toHaveBeenCalledWith( + [...safeFiles, unsafe], + { workspaceId: 'ws-1', actorUserId: 'user-1' } + ) + expect(messages).toEqual(originalMessages) + expect(mockAttachLargeFileRemoteUrls).not.toHaveBeenCalled() + expect(mockUploadLargeFilesToProvider).not.toHaveBeenCalled() + expect(mockExecuteRequest).not.toHaveBeenCalled() + } + ) it('fails explicitly when file provenance lookup is unavailable', async () => { mockFilterModelSafeWorkspaceFileAttachments.mockRejectedValueOnce(new Error('db unavailable')) diff --git a/apps/sim/providers/index.ts b/apps/sim/providers/index.ts index 5d1f5819a9d..60109042208 100644 --- a/apps/sim/providers/index.ts +++ b/apps/sim/providers/index.ts @@ -2,7 +2,10 @@ import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { getApiKeyWithBYOK } from '@/lib/api-key/byok' import { env, envNumber } from '@/lib/core/config/env' -import { filterModelSafeWorkspaceFileAttachments } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' +import { + filterModelSafeWorkspaceFileAttachments, + MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE, +} from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' import type { StreamingExecution } from '@/executor/types' import { applyModelCostPolicy, @@ -41,11 +44,9 @@ import { const logger = createLogger('Providers') -async function omitUnsafeProviderFileAttachments( - request: ProviderRequest -): Promise { +async function assertModelSafeProviderFileAttachments(request: ProviderRequest): Promise { const attachments = (request.messages ?? []).flatMap((message) => message.files ?? []) - if (attachments.length === 0) return request + if (attachments.length === 0) return let safeAttachments: typeof attachments try { @@ -61,19 +62,8 @@ async function omitUnsafeProviderFileAttachments( throw new Error('File attachments could not be verified for model use') } - if (safeAttachments.length === attachments.length) return request - const safe = new Set(safeAttachments) - logger.warn('Omitting model attachments with unsafe secret provenance', { - attachmentCount: attachments.length, - omittedCount: attachments.length - safeAttachments.length, - }) - return { - ...request, - messages: request.messages?.map((message) => { - if (!message.files) return message - const files = message.files.filter((file) => safe.has(file)) - return { ...message, ...(files.length > 0 ? { files } : { files: undefined }) } - }), + if (safeAttachments.length !== attachments.length) { + throw new Error(MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE) } } @@ -236,9 +226,8 @@ export async function executeProviderRequest( sanitizedRequest.responseFormat = undefined } - const provenanceSafeRequest = await omitUnsafeProviderFileAttachments(sanitizedRequest) - const modelSafeRequest = provenanceSafeRequest - const toolIdentities = assignProviderToolIdentities(modelSafeRequest.tools) + await assertModelSafeProviderFileAttachments(sanitizedRequest) + const toolIdentities = assignProviderToolIdentities(sanitizedRequest.tools) const failedFunctionToolCost = { total: 0 } const requestRuntimeContext: ProviderRuntimeContext = { ...runtimeContext, @@ -253,21 +242,21 @@ export async function executeProviderRequest( : {}), } - if (modelSafeRequest.responseFormat) { + if (sanitizedRequest.responseFormat) { const structuredOutputInstructions = generateStructuredOutputInstructions( - modelSafeRequest.responseFormat + sanitizedRequest.responseFormat ) if (structuredOutputInstructions.trim()) { - const originalPrompt = modelSafeRequest.systemPrompt || '' - modelSafeRequest.systemPrompt = `${originalPrompt}\n\n${structuredOutputInstructions}`.trim() + const originalPrompt = sanitizedRequest.systemPrompt || '' + sanitizedRequest.systemPrompt = `${originalPrompt}\n\n${structuredOutputInstructions}`.trim() logger.info('Added structured output instructions to system prompt') } } const response = await runWithProviderRuntimeContext(requestRuntimeContext, async () => { - await attachLargeFileRemoteUrls(modelSafeRequest, providerId, runtimeContext?.executionContext) - await uploadLargeFilesToProvider(modelSafeRequest, providerId, runtimeContext?.executionContext) - return provider.executeRequest(modelSafeRequest) + await attachLargeFileRemoteUrls(sanitizedRequest, providerId, runtimeContext?.executionContext) + await uploadLargeFilesToProvider(sanitizedRequest, providerId, runtimeContext?.executionContext) + return provider.executeRequest(sanitizedRequest) }) if (isStreamingExecution(response)) { From e5e66c10b6dfb150bf586195b8f179dc8e1d6507 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 15 Sep 2026 16:48:27 -0700 Subject: [PATCH 2/3] fix(provenance): keep model turns running after attachment refusal --- .../handlers/agent/agent-handler.test.ts | 43 +++++++--- .../executor/handlers/agent/agent-handler.ts | 47 +++++++---- .../lib/copilot/request/lifecycle/run.test.ts | 79 +++++++++++++++++-- apps/sim/lib/copilot/request/lifecycle/run.ts | 67 ++++++++++++---- apps/sim/lib/uploads/utils/model-input.ts | 9 +++ apps/sim/providers/index.test.ts | 76 ++++++++++++------ apps/sim/providers/index.ts | 49 ++++++++---- 7 files changed, 279 insertions(+), 91 deletions(-) diff --git a/apps/sim/executor/handlers/agent/agent-handler.test.ts b/apps/sim/executor/handlers/agent/agent-handler.test.ts index 8362a4bae76..b5b61ec6c1d 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.test.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.test.ts @@ -53,8 +53,6 @@ vi.mock('@/lib/internal/mcp/discover-tools', () => ({ })) vi.mock('@/lib/uploads/contexts/workspace/workspace-file-secret-provenance', () => ({ - MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE: - 'File cannot be sent to a model because its secret provenance is unavailable', importWorkspaceFileSecretProvenanceForModelView: mockImportWorkspaceFileSecretProvenanceForModelView, })) @@ -1042,9 +1040,13 @@ describe('AgentBlockHandler', () => { } }) - it.each([false, true])( - 'honors generated document contributor admission (safe=%s)', - async (safe) => { + it.each([ + { safe: false, includeSafeFile: false }, + { safe: false, includeSafeFile: true }, + { safe: true, includeSafeFile: true }, + ])( + 'continues after document contributor admission (safe=$safe, mixed=$includeSafeFile)', + async ({ safe, includeSafeFile }) => { const key = 'workspace/ws-1/report.pdf' mockContext.workspaceId = 'ws-1' const hydrationSpy = vi @@ -1065,7 +1067,7 @@ describe('AgentBlockHandler', () => { try { mockGetProviderFromModel.mockReturnValue('openai') - const execution = handler.execute(mockContext, mockBlock, { + await handler.execute(mockContext, mockBlock, { model: 'gpt-4o', userPrompt: 'Analyze this document', files: [ @@ -1077,20 +1079,35 @@ describe('AgentBlockHandler', () => { size: 128, type: 'text/x-python-pdf', }, + ...(includeSafeFile + ? [ + { + id: 'file-2', + name: 'safe.pdf', + path: '/safe.pdf', + key: 'workspace/ws-1/safe.pdf', + size: 128, + type: 'application/pdf', + }, + ] + : []), ], apiKey: 'test-api-key', }) + expect(mockExecuteProviderRequest).toHaveBeenCalledOnce() + const sent = mockExecuteProviderRequest.mock.calls[0][1].messages.at(-1) + expect(sent.files.map((file: { id: string }) => file.id)).toEqual([ + ...(safe ? ['file-1'] : []), + ...(includeSafeFile ? ['file-2'] : []), + ]) if (safe) { - await execution - expect(mockExecuteProviderRequest.mock.calls[0][1].messages.at(-1)?.files).toEqual([ - expect.objectContaining({ key, base64: 'JVBERi0=' }), - ]) + expect(sent.content).toBe('Analyze this document') } else { - await expect(execution).rejects.toThrow( - 'File cannot be sent to a model because its secret provenance is unavailable' + expect(sent.content).toMatch( + /^Analyze this document\n\nAttachment error: 1 requested file attachment was not provided/ ) - expect(mockExecuteProviderRequest).not.toHaveBeenCalled() + expect(JSON.stringify(sent)).not.toContain(key) } expect(mockImportWorkspaceFileSecretProvenanceForModelView).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/apps/sim/executor/handlers/agent/agent-handler.ts b/apps/sim/executor/handlers/agent/agent-handler.ts index c1bb7d2f111..d96aeed8f74 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.ts @@ -26,10 +26,7 @@ import { resolveAutoModel, SIM_AUTO_SYSTEM_PREAMBLE, } from '@/lib/model-router/resolve' -import { - importWorkspaceFileSecretProvenanceForModelView, - MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE, -} from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' +import { importWorkspaceFileSecretProvenanceForModelView } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' import { getFileExtension, MODEL_SUPPORTED_IMAGE_MIME_TYPES, @@ -37,7 +34,10 @@ import { type RawFileInput, tryInferContextFromKey, } from '@/lib/uploads/utils/file-utils' -import { selectModelBoundFileInputPaths } from '@/lib/uploads/utils/model-input' +import { + appendUnavailableAttachmentNotice, + selectModelBoundFileInputPaths, +} from '@/lib/uploads/utils/model-input' import { hydrateUserFilesWithBase64 } from '@/lib/uploads/utils/user-file-base64.server' import { resolveCustomBlockToolBinding } from '@/lib/workflows/custom-blocks/operations' import { @@ -1496,6 +1496,7 @@ export class AgentBlockHandler implements BlockHandler { continue } + const unsafeGeneratedDocumentFiles = new Set() const groups = new Map>() message.files.forEach((file, index) => { const workspaceFile = @@ -1513,7 +1514,7 @@ export class AgentBlockHandler implements BlockHandler { ...(await resolveExecutorFileMaterializationContext(ctx, group[0].file)), logger, maxBytes: inlineMaxBytes, - onServableFileContributors: async (_file, contributors) => { + onServableFileContributors: async (file, contributors) => { if (!ctx.workspaceId) return for (const identity of contributors) { const safe = await importWorkspaceFileSecretProvenanceForModelView({ @@ -1524,7 +1525,8 @@ export class AgentBlockHandler implements BlockHandler { ...(ctx.userId ? { actorUserId: ctx.userId } : {}), }) if (!safe) { - throw new Error(MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE) + unsafeGeneratedDocumentFiles.add(`${file.key}:${file.id}`) + return } } }, @@ -1536,7 +1538,9 @@ export class AgentBlockHandler implements BlockHandler { }) ) - const modelSafeHydratedFiles = hydratedFiles.map((file, fileIndex) => { + const modelSafeHydratedFiles = hydratedFiles.flatMap((file, fileIndex) => { + if (unsafeGeneratedDocumentFiles.has(`${file.key}:${file.id}`)) return [] + const sourceFile = message.files?.[fileIndex] const nameProjection = sourceFile ? projectedNameByFile.get(sourceFile) : undefined if ( @@ -1545,7 +1549,7 @@ export class AgentBlockHandler implements BlockHandler { largeFilePathAvailable: canUseProviderLargeFilePath(providerId), }) ) { - return file + return [file] } if (nameProjection.inputPath) modelBoundInputPaths.push(nameProjection.inputPath) @@ -1553,12 +1557,22 @@ export class AgentBlockHandler implements BlockHandler { const suffix = extension ? `.${extension}` : '' const keepsSuffix = suffix !== '' && nameProjection.name.toLowerCase().endsWith(suffix.toLowerCase()) - return { - ...file, - name: - suffix !== '' && !keepsSuffix ? `${nameProjection.name}${suffix}` : nameProjection.name, - } + return [ + { + ...file, + name: + suffix !== '' && !keepsSuffix + ? `${nameProjection.name}${suffix}` + : nameProjection.name, + }, + ] }) + if (modelSafeHydratedFiles.length !== hydratedFiles.length) { + logger.warn('Omitting generated document attachments with unsafe contributor provenance', { + omittedCount: hydratedFiles.length - modelSafeHydratedFiles.length, + attachmentCount: hydratedFiles.length, + }) + } const missingFile = modelSafeHydratedFiles.find( (file) => @@ -1591,8 +1605,13 @@ export class AgentBlockHandler implements BlockHandler { ) } + const omittedCount = hydratedFiles.length - modelSafeHydratedFiles.length nextMessages[messageIndex] = { ...message, + content: + omittedCount > 0 + ? appendUnavailableAttachmentNotice(message.content, omittedCount) + : message.content, files: modelSafeHydratedFiles, } } diff --git a/apps/sim/lib/copilot/request/lifecycle/run.test.ts b/apps/sim/lib/copilot/request/lifecycle/run.test.ts index 9a614aa9f30..655067607cb 100644 --- a/apps/sim/lib/copilot/request/lifecycle/run.test.ts +++ b/apps/sim/lib/copilot/request/lifecycle/run.test.ts @@ -50,8 +50,6 @@ vi.mock('@/lib/copilot/application/load-search-integrations', () => ({ })) vi.mock('@/lib/uploads/contexts/workspace/workspace-file-secret-provenance', () => ({ - MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE: - 'File cannot be sent to a model because its secret provenance is unavailable', filterModelSafeWorkspaceFileAttachments: (...args: unknown[]) => mockFilterModelSafeWorkspaceFileAttachments(...args), })) @@ -673,13 +671,22 @@ describe('runCopilotLifecycle', () => { { key: 'fileAttachments', includeSafeFile: false }, { key: 'fileAttachments', includeSafeFile: true }, ])( - 'rejects refused initial $key before the Go request (mixed=$includeSafeFile)', + 'continues with an error notice for refused $key (mixed=$includeSafeFile)', async ({ key, includeSafeFile }) => { - const unsafe = { id: 'wf-unsafe', name: 'unsafe.txt', key: 'workspace/ws-1/unsafe.txt' } + const unsafe = { + id: 'wf-private', + name: 'private-filename.txt', + key: 'private-storage-key', + base64: 'private-bytes', + } const safe = { id: 'wf-safe', name: 'safe.txt', key: 'workspace/ws-1/safe.txt' } const safeFiles = includeSafeFile ? [safe] : [] mockFilterModelSafeWorkspaceFileAttachments.mockResolvedValueOnce(safeFiles) const onError = vi.fn() + mockRunStreamLoop.mockImplementationOnce(async (_url, _request, context) => { + context.accumulatedContent = 'I can continue with the available inputs.' + context.completionStatus = MothershipStreamV1CompletionStatus.complete + }) const payload = { message: 'Review files', [key]: [...safeFiles, unsafe], @@ -695,15 +702,71 @@ describe('runCopilotLifecycle', () => { onError, }) - const message = 'File cannot be sent to a model because its secret provenance is unavailable' - expect(result).toMatchObject({ success: false, error: message }) - expect(onError).toHaveBeenCalledWith(expect.objectContaining({ message }), result) + expect(result).toMatchObject({ + success: true, + content: 'I can continue with the available inputs.', + }) + expect(onError).not.toHaveBeenCalled() + expect(mockRunStreamLoop).toHaveBeenCalledOnce() + const sent = JSON.parse(String(mockRunStreamLoop.mock.calls[0][1].body)) + expect(sent.message).toMatch( + /^Review files\n\nAttachment error: 1 requested file attachment was not provided/ + ) + expect(sent[key] ?? []).toEqual(safeFiles) + expect(JSON.stringify(sent)).not.toContain('private-') expect(mockFilterModelSafeWorkspaceFileAttachments).toHaveBeenCalledWith( [...safeFiles, unsafe], { workspaceId: 'ws-1' } ) expect(payload).toEqual(originalPayload) - expect(mockRunStreamLoop).not.toHaveBeenCalled() + } + ) + + it.each(['messages', 'both', 'attachment-only', 'system-only'])( + 'reports combined attachment refusals in %s payloads without changing history', + async (shape) => { + const history = { + role: 'assistant', + content: 'Previous response', + tool_calls: [{ id: 'existing-call' }], + } + const messages = + shape === 'system-only' + ? [{ role: 'system', content: 'System context' }] + : [history, { role: 'user', content: 'Review files' }] + const payload = { + ...(shape === 'both' ? { message: 'Review files' } : {}), + ...(shape === 'attachment-only' ? {} : { messages }), + attachments: [{ key: 'private-first-file' }], + fileAttachments: [{ key: 'private-second-file' }], + } + const original = structuredClone(payload) + mockFilterModelSafeWorkspaceFileAttachments + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([]) + mockRunStreamLoop.mockResolvedValueOnce(undefined) + + const result = await runCopilotLifecycle(payload, { + userId: 'user-1', + workspaceId: 'ws-1', + executionContext: { userId: 'user-1', workflowId: '', workspaceId: 'ws-1' }, + }) + + expect(result.success).toBe(true) + const sent = JSON.parse(String(mockRunStreamLoop.mock.calls[0][1].body)) + const notice = 'Attachment error: 2 requested file attachments were not provided' + if (shape === 'both' || shape === 'attachment-only') expect(sent.message).toContain(notice) + if (shape !== 'attachment-only') { + expect(sent.messages[0]).toEqual(messages[0]) + expect(sent.messages.at(-1)).toMatchObject({ + role: 'user', + content: expect.stringContaining(notice), + }) + } + expect(sent).not.toHaveProperty('attachments') + expect(sent).not.toHaveProperty('fileAttachments') + expect(JSON.stringify(sent)).not.toContain('private-') + expect(payload).toEqual(original) } ) diff --git a/apps/sim/lib/copilot/request/lifecycle/run.ts b/apps/sim/lib/copilot/request/lifecycle/run.ts index 281f1762421..ba50ca1ef5e 100644 --- a/apps/sim/lib/copilot/request/lifecycle/run.ts +++ b/apps/sim/lib/copilot/request/lifecycle/run.ts @@ -4,6 +4,7 @@ import type { PermissionType } from '@sim/platform-authz/workspace' import { getErrorMessage, toError } from '@sim/utils/errors' import { interruptibleSleep, sleep } from '@sim/utils/helpers' import { generateId } from '@sim/utils/id' +import { isPlainRecord, omit } from '@sim/utils/object' import { workspaceSearchFiltersSchema } from '@/lib/api/contracts/knowledge/search' import { type AttributedBillingRequestEnvelope, @@ -70,10 +71,8 @@ import { prepareExecutionContext } from '@/lib/copilot/tools/handlers/context' import { env } from '@/lib/core/config/env' import { isCopilotToolPermissionsEnabled, isHosted } from '@/lib/core/config/env-flags' import { isWorkspaceCapabilityWithheld } from '@/lib/permission-groups/capability-assertions' -import { - filterModelSafeWorkspaceFileAttachments, - MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE, -} from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' +import { filterModelSafeWorkspaceFileAttachments } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' +import { appendUnavailableAttachmentNotice } from '@/lib/uploads/utils/model-input' import type { ExecutorDelegationOrigin } from '@/executor/types' import { refuseResolvedSecretProjection } from '@/executor/utils/resolved-secret-projection-refusal' import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' @@ -97,13 +96,15 @@ class CopilotModelContentProjectionError extends Error { } } -async function assertModelSafeInitialCopilotAttachments( +async function prepareInitialCopilotAttachmentsForModel( payload: Record, workspaceId?: string -): Promise { +): Promise> { + let projected = payload + let omittedCount = 0 for (const key of ['attachments', 'fileAttachments'] as const) { - if (!Object.hasOwn(payload, key)) continue - const attachments = payload[key] + if (!Object.hasOwn(projected, key)) continue + const attachments = projected[key] if (!Array.isArray(attachments)) { refuseResolvedSecretProjection({ site: 'copilot.initialAttachmentsShape', @@ -129,14 +130,45 @@ async function assertModelSafeInitialCopilotAttachments( }) } - if (safeAttachments.length !== attachments.length) { - refuseResolvedSecretProjection({ - site: 'copilot.initialAttachmentsProvenance', - message: MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE, - inputPath: key, - }) + if (safeAttachments.length === attachments.length) continue + omittedCount += attachments.length - safeAttachments.length + logger.warn('Omitting Copilot attachments with unsafe secret provenance', { + attachmentCount: attachments.length, + omittedCount: attachments.length - safeAttachments.length, + }) + projected = + safeAttachments.length > 0 ? { ...projected, [key]: safeAttachments } : omit(projected, [key]) + } + if (omittedCount === 0) return projected + + if (typeof projected.message === 'string') { + projected = { + ...projected, + message: appendUnavailableAttachmentNotice(projected.message, omittedCount), } } + if (Array.isArray(projected.messages)) { + const messages: unknown[] = [...projected.messages] + let notified = false + for (let index = messages.length - 1; index >= 0; index--) { + const message = messages[index] + if (!isPlainRecord(message) || message.role !== 'user' || typeof message.content !== 'string') + continue + messages[index] = { + ...message, + content: appendUnavailableAttachmentNotice(message.content, omittedCount), + } + notified = true + break + } + if (!notified) { + messages.push({ role: 'user', content: appendUnavailableAttachmentNotice('', omittedCount) }) + } + projected = { ...projected, messages } + } else if (typeof projected.message !== 'string') { + projected = { ...projected, message: appendUnavailableAttachmentNotice('', omittedCount) } + } + return projected } async function ensureModelEgressRegistry( @@ -405,9 +437,12 @@ export async function runCopilotLifecycle( }), } } - await assertModelSafeInitialCopilotAttachments(requestPayload, lifecycleOptions.workspaceId) - await runCheckpointLoop( + const modelSafeRequestPayload = await prepareInitialCopilotAttachmentsForModel( requestPayload, + lifecycleOptions.workspaceId + ) + await runCheckpointLoop( + modelSafeRequestPayload, context, execContext, lifecycleOptions, diff --git a/apps/sim/lib/uploads/utils/model-input.ts b/apps/sim/lib/uploads/utils/model-input.ts index 736de9c7eaf..214f0a3f459 100644 --- a/apps/sim/lib/uploads/utils/model-input.ts +++ b/apps/sim/lib/uploads/utils/model-input.ts @@ -149,3 +149,12 @@ export function applyProjectedModelVisibleFileNames( } return { ...original, name: projected.name } } + +/** Reports withheld attachments without exposing their unverified names, locators, or contents. */ +export function appendUnavailableAttachmentNotice( + content: string | null | undefined, + omittedCount: number +): string { + const notice = `Attachment error: ${omittedCount} requested file attachment${omittedCount === 1 ? ' was' : 's were'} not provided because file safety checks failed. Their contents are unavailable. Continue with the available inputs and explain any resulting limitation.` + return content ? `${content}\n\n${notice}` : notice +} diff --git a/apps/sim/providers/index.test.ts b/apps/sim/providers/index.test.ts index 38bbc273e5f..c254ef97f1c 100644 --- a/apps/sim/providers/index.test.ts +++ b/apps/sim/providers/index.test.ts @@ -37,8 +37,6 @@ vi.mock('@/providers/file-attachments.server', () => ({ })) vi.mock('@/lib/uploads/contexts/workspace/workspace-file-secret-provenance', () => ({ - MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE: - 'File cannot be sent to a model because its secret provenance is unavailable', filterModelSafeWorkspaceFileAttachments: (...args: unknown[]) => mockFilterModelSafeWorkspaceFileAttachments(...args), })) @@ -971,45 +969,77 @@ describe('executeProviderRequest — caller-prepared model input', () => { { stream: true, includeSafeFile: false }, { stream: true, includeSafeFile: true }, ])( - 'rejects refused attachments before provider processing (stream=$stream, mixed=$includeSafeFile)', + 'continues with an attachment error notice (stream=$stream, mixed=$includeSafeFile)', async ({ stream, includeSafeFile }) => { const unsafe = { id: 'wf-unsafe', - name: 'unsafe.txt', - url: '/unsafe', + name: 'private-filename.txt', + url: '/private-file-url', size: 10, type: 'text/plain', - key: 'workspace/ws-1/unsafe.txt', + key: 'workspace/ws-1/private-storage-key.txt', + base64: 'private-file-bytes', + } + const safe = { + ...unsafe, + id: 'wf-safe', + name: 'safe.txt', + url: '/safe', + key: 'safe-key', + base64: 'safe-bytes', } - const safe = { ...unsafe, id: 'wf-safe', key: 'workspace/ws-1/safe.txt' } const safeFiles = includeSafeFile ? [safe] : [] mockFilterModelSafeWorkspaceFileAttachments.mockResolvedValueOnce(safeFiles) const messages = [ - { role: 'user' as const, content: 'Review files', files: safeFiles }, - { role: 'user' as const, content: 'Include this file too', files: [unsafe] }, + { role: 'user' as const, content: 'Earlier context' }, + { + role: 'user' as const, + content: includeSafeFile ? 'Review files' : null, + files: [...safeFiles, unsafe], + }, ] const originalMessages = structuredClone(messages) - - await expect( - executeProviderRequest('openai', { - model: 'test-model', - workspaceId: 'ws-1', - userId: 'user-1', - stream, - messages, + if (stream) { + mockExecuteRequest.mockResolvedValueOnce({ + stream: new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('ok')) + controller.close() + }, + }), + execution: { success: true, output: { content: 'ok' } }, }) - ).rejects.toThrow( - 'File cannot be sent to a model because its secret provenance is unavailable' - ) + } + const response = await executeProviderRequest('openai', { + model: 'test-model', + workspaceId: 'ws-1', + userId: 'user-1', + stream, + messages, + }) + + if (stream) { + expect(await new Response((response as StreamingExecution).stream).text()).toBe('ok') + } else { + expect(response).toMatchObject({ content: 'ok' }) + } expect(mockFilterModelSafeWorkspaceFileAttachments).toHaveBeenCalledWith( [...safeFiles, unsafe], { workspaceId: 'ws-1', actorUserId: 'user-1' } ) + const sent = mockExecuteRequest.mock.calls[0][0] + expect(sent.messages[0]).toEqual(messages[0]) + expect(sent.messages[1].content).toContain( + 'Attachment error: 1 requested file attachment was not provided' + ) + expect(sent.messages[1].content).toContain('Continue with the available inputs') + if (includeSafeFile) expect(sent.messages[1].content).toMatch(/^Review files\n\n/) + expect(sent.messages[1].files ?? []).toEqual(safeFiles) + expect(JSON.stringify(sent)).not.toContain('private-') + expect(mockAttachLargeFileRemoteUrls.mock.calls[0][0]).toBe(sent) + expect(mockUploadLargeFilesToProvider.mock.calls[0][0]).toBe(sent) expect(messages).toEqual(originalMessages) - expect(mockAttachLargeFileRemoteUrls).not.toHaveBeenCalled() - expect(mockUploadLargeFilesToProvider).not.toHaveBeenCalled() - expect(mockExecuteRequest).not.toHaveBeenCalled() } ) diff --git a/apps/sim/providers/index.ts b/apps/sim/providers/index.ts index 60109042208..c7e5f8b2eb0 100644 --- a/apps/sim/providers/index.ts +++ b/apps/sim/providers/index.ts @@ -2,10 +2,8 @@ import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { getApiKeyWithBYOK } from '@/lib/api-key/byok' import { env, envNumber } from '@/lib/core/config/env' -import { - filterModelSafeWorkspaceFileAttachments, - MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE, -} from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' +import { filterModelSafeWorkspaceFileAttachments } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' +import { appendUnavailableAttachmentNotice } from '@/lib/uploads/utils/model-input' import type { StreamingExecution } from '@/executor/types' import { applyModelCostPolicy, @@ -44,9 +42,9 @@ import { const logger = createLogger('Providers') -async function assertModelSafeProviderFileAttachments(request: ProviderRequest): Promise { +async function prepareProviderFileAttachments(request: ProviderRequest): Promise { const attachments = (request.messages ?? []).flatMap((message) => message.files ?? []) - if (attachments.length === 0) return + if (attachments.length === 0) return request let safeAttachments: typeof attachments try { @@ -62,8 +60,25 @@ async function assertModelSafeProviderFileAttachments(request: ProviderRequest): throw new Error('File attachments could not be verified for model use') } - if (safeAttachments.length !== attachments.length) { - throw new Error(MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE) + if (safeAttachments.length === attachments.length) return request + const safe = new Set(safeAttachments) + logger.warn('Omitting model attachments with unsafe secret provenance', { + attachmentCount: attachments.length, + omittedCount: attachments.length - safeAttachments.length, + }) + return { + ...request, + messages: request.messages?.map((message) => { + if (!message.files) return message + const files = message.files.filter((file) => safe.has(file)) + const omittedCount = message.files.length - files.length + if (omittedCount === 0) return message + return { + ...message, + content: appendUnavailableAttachmentNotice(message.content, omittedCount), + files: files.length > 0 ? files : undefined, + } + }), } } @@ -226,8 +241,8 @@ export async function executeProviderRequest( sanitizedRequest.responseFormat = undefined } - await assertModelSafeProviderFileAttachments(sanitizedRequest) - const toolIdentities = assignProviderToolIdentities(sanitizedRequest.tools) + const modelSafeRequest = await prepareProviderFileAttachments(sanitizedRequest) + const toolIdentities = assignProviderToolIdentities(modelSafeRequest.tools) const failedFunctionToolCost = { total: 0 } const requestRuntimeContext: ProviderRuntimeContext = { ...runtimeContext, @@ -242,21 +257,21 @@ export async function executeProviderRequest( : {}), } - if (sanitizedRequest.responseFormat) { + if (modelSafeRequest.responseFormat) { const structuredOutputInstructions = generateStructuredOutputInstructions( - sanitizedRequest.responseFormat + modelSafeRequest.responseFormat ) if (structuredOutputInstructions.trim()) { - const originalPrompt = sanitizedRequest.systemPrompt || '' - sanitizedRequest.systemPrompt = `${originalPrompt}\n\n${structuredOutputInstructions}`.trim() + const originalPrompt = modelSafeRequest.systemPrompt || '' + modelSafeRequest.systemPrompt = `${originalPrompt}\n\n${structuredOutputInstructions}`.trim() logger.info('Added structured output instructions to system prompt') } } const response = await runWithProviderRuntimeContext(requestRuntimeContext, async () => { - await attachLargeFileRemoteUrls(sanitizedRequest, providerId, runtimeContext?.executionContext) - await uploadLargeFilesToProvider(sanitizedRequest, providerId, runtimeContext?.executionContext) - return provider.executeRequest(sanitizedRequest) + await attachLargeFileRemoteUrls(modelSafeRequest, providerId, runtimeContext?.executionContext) + await uploadLargeFilesToProvider(modelSafeRequest, providerId, runtimeContext?.executionContext) + return provider.executeRequest(modelSafeRequest) }) if (isStreamingExecution(response)) { From aa9f3882084e543f11c41e21f74e39b6bbd1b276 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 15 Sep 2026 17:01:16 -0700 Subject: [PATCH 3/3] fix(provenance): apply literal policy before file classification --- .../mounted-file-secret-provenance.test.ts | 46 +++++ .../mounted-file-secret-provenance.ts | 19 +- .../execute-request.test.ts | 166 +++++++++++++----- .../lib/function-execution/execute-request.ts | 1 + 4 files changed, 178 insertions(+), 54 deletions(-) diff --git a/apps/sim/lib/execution/mounted-file-secret-provenance.test.ts b/apps/sim/lib/execution/mounted-file-secret-provenance.test.ts index 77be6d392a0..8a4dccf5a4e 100644 --- a/apps/sim/lib/execution/mounted-file-secret-provenance.test.ts +++ b/apps/sim/lib/execution/mounted-file-secret-provenance.test.ts @@ -84,6 +84,52 @@ describe('mounted file output provenance scanner', () => { expect(scanner?.hasSecrets).toBe(true) }) + it.each(['false', 'hunter2', '""""'])( + 'excludes short plaintext %j before escaping it', + async (plaintext) => { + encryptionMockFns.mockDecryptSecret.mockResolvedValue({ decrypted: plaintext }) + + const scanner = await createMountedFileSecretProvenanceScanner({ + version: 1, + complete: true, + entries: [{ encryptedValue: 'encrypted-short' }], + scope: { userId: 'user-1', workspaceId: 'workspace-1' }, + }) + + expect(scanner?.scan(Buffer.from(JSON.stringify(plaintext)))).toEqual({ + status: 'exact', + entries: [], + }) + expect(scanner?.hasSecrets).toBe(false) + } + ) + + it('protects an eight-character literal alongside excluded short entries', async () => { + encryptionMockFns.mockDecryptSecret.mockImplementation(async (value: string) => ({ + decrypted: value === 'encrypted-short' ? 'false' : 'hunter22', + })) + + const scanner = await createMountedFileSecretProvenanceScanner({ + version: 1, + complete: true, + entries: [{ encryptedValue: 'encrypted-short' }, { encryptedValue: 'encrypted-boundary' }], + scope: { userId: 'user-1', workspaceId: 'workspace-1' }, + }) + + expect(scanner?.hasSecrets).toBe(true) + expect(scanner?.scan(Buffer.from('false hunter22'))).toEqual({ + status: 'exact', + entries: [ + { + name: 'MOUNTED_FILE_SECRET', + encryptedValue: 'encrypted-boundary', + sourceUserId: 'user-1', + sourceWorkspaceId: 'workspace-1', + }, + ], + }) + }) + it('classifies outputs unknown when authenticated mount provenance cannot be inspected', async () => { const incomplete = await createMountedFileSecretProvenanceScanner({ version: 1, diff --git a/apps/sim/lib/execution/mounted-file-secret-provenance.ts b/apps/sim/lib/execution/mounted-file-secret-provenance.ts index 44ddfddc870..15ec17b42fa 100644 --- a/apps/sim/lib/execution/mounted-file-secret-provenance.ts +++ b/apps/sim/lib/execution/mounted-file-secret-provenance.ts @@ -5,6 +5,7 @@ import { createResolvedSecretMatcher, scanResolvedSecretString, } from '@/executor/utils/resolved-secret-content-projection' +import { isNonIdentifyingSecretLiteral } from '@/executor/utils/resolved-secret-match-policy' import type { ResolvedSecretTraceProvenanceV1 } from '@/executor/utils/resolved-secret-trace-registry' const MAX_MOUNTED_FILE_SECRET_MATCH_EVENTS = 1_000_000 @@ -12,11 +13,10 @@ const ANONYMOUS_MOUNTED_FILE_SECRET_NAME = 'MOUNTED_FILE_SECRET' export interface MountedFileSecretProvenanceScanner { /** - * True when the envelope attested to any secret material, whether or not it could be turned into - * a scannable literal. False therefore means the mount carried nothing to leak — which lets - * callers classify content this scanner cannot soundly scan (binary bytes) instead of failing - * closed. Entries that fail to yield plaintext keep this true: losing the ability to scan them - * makes the mount less classifiable, not more. + * True when the envelope carries material protected by the shared literal policy, or an entry + * cannot be inspected. Successfully decrypted short values do not taint derived binary files. + * Entries that fail to yield plaintext keep this true: losing the ability to scan them makes + * the mount less classifiable, not more. */ hasSecrets: boolean scan(buffer: Buffer): WorkspaceFileSecretProvenance @@ -42,12 +42,17 @@ export async function createMountedFileSecretProvenanceScanner( if (!provenance.complete) return UNKNOWN_MOUNTED_FILE_SECRET_PROVENANCE_SCANNER if (!provenance.scope?.userId) return undefined - const hasSecrets = provenance.entries.length > 0 + let hasSecrets = false const entriesByScanLiteral = new Map>() try { for (const entry of provenance.entries) { const { decrypted: plaintext } = await decryptSecret(entry.encryptedValue) - if (!plaintext) continue + if (!plaintext) { + hasSecrets = true + continue + } + if (isNonIdentifyingSecretLiteral(plaintext)) continue + hasSecrets = true const fileEntry: WorkspaceFileSecretProvenanceEntry = { name: entry.name || ANONYMOUS_MOUNTED_FILE_SECRET_NAME, encryptedValue: entry.encryptedValue, diff --git a/apps/sim/lib/function-execution/execute-request.test.ts b/apps/sim/lib/function-execution/execute-request.test.ts index 36ceb0a0e59..30f47ef2387 100644 --- a/apps/sim/lib/function-execution/execute-request.test.ts +++ b/apps/sim/lib/function-execution/execute-request.test.ts @@ -1018,6 +1018,67 @@ describe('Function execution request', () => { ) }) + it('excludes short compiled plaintext before JSON escaping even with a protected secret in scope', async () => { + const shortValue = '""""' + envFlagsMock.isRemoteSandboxEnabled = true + mockExecuteInSandbox.mockResolvedValueOnce({ + result: 'done', + stdout: '', + sandboxId: 'sandbox-123', + exportedFiles: { + '/home/user/short.json': JSON.stringify({ value: shortValue }), + '/home/user/protected.txt': 'hunter22', + }, + }) + + const response = await POST( + createMockRequest('POST', { + code: 'print({{SHORT_VALUE}}, {{API_KEY}})', + language: 'python', + workspaceId: 'workspace-1', + envVars: { SHORT_VALUE: shortValue, API_KEY: 'hunter22' }, + outputs: { + files: [ + { + path: 'files/short.json', + sandboxPath: '/home/user/short.json', + mimeType: 'application/json', + }, + { + path: 'files/protected.txt', + sandboxPath: '/home/user/protected.txt', + mimeType: 'text/plain', + }, + ], + }, + }) + ) + + expect(response.status).toBe(200) + expect(mockWriteWorkspaceFileByPath).toHaveBeenCalledWith( + expect.objectContaining({ + target: expect.objectContaining({ path: 'files/short.json' }), + secretProvenance: { status: 'exact', entries: [] }, + }) + ) + expect(mockWriteWorkspaceFileByPath).toHaveBeenCalledWith( + expect.objectContaining({ + target: expect.objectContaining({ path: 'files/protected.txt' }), + secretProvenance: { + status: 'exact', + entries: [ + { + name: 'API_KEY', + encryptedValue: 'encrypted:hunter22', + sourceUserId: 'user-123', + sourceWorkspaceId: 'workspace-1', + }, + ], + }, + }) + ) + }) + it('classifies exports exact-empty when the only compiled secret is exempt, still reporting its name', async () => { envFlagsMock.isRemoteSandboxEnabled = true mockExecuteInSandbox.mockResolvedValueOnce({ @@ -1369,58 +1430,69 @@ describe('Function execution request', () => { ) }) - it('keeps a binary export unknown when a mounted input file carried a secret', async () => { - envFlagsMock.isRemoteSandboxEnabled = true - mockExecuteInSandbox.mockResolvedValueOnce({ - result: 'done', - stdout: '', - sandboxId: 'sandbox-123', - exportedFiles: { '/home/user/small.jpg': '/9j/4AAQ' }, - }) + it.each([ + { plaintext: 'mounted-secret', expectedStatus: 'unknown' }, + { plaintext: 'false', expectedStatus: 'exact' }, + { plaintext: '""""', expectedStatus: 'exact' }, + ])( + 'classifies binary exports $expectedStatus with mounted plaintext $plaintext', + async ({ plaintext, expectedStatus }) => { + mockDecryptSecret.mockResolvedValueOnce({ decrypted: plaintext }) + envFlagsMock.isRemoteSandboxEnabled = true + mockExecuteInSandbox.mockResolvedValueOnce({ + result: 'done', + stdout: '', + sandboxId: 'sandbox-123', + exportedFiles: { '/home/user/small.jpg': '/9j/4AAQ' }, + }) - const response = await POST( - createMockRequest( - 'POST', - { - code: 'print("done")', - language: 'python', - workspaceId: 'workspace-1', - outputs: { - files: [ - { - path: 'files/small.jpg', - sandboxPath: '/home/user/small.jpg', - mimeType: 'image/jpeg', - }, - ], - }, - [PRIVATE_SECRET_PROVENANCE_FIELD]: { - version: 1, - complete: true, - selections: [ - { - key: MOUNTED_WORKSPACE_FILES_PROVENANCE_KEY, - provenance: { - version: 1, - complete: true, - entries: [{ encryptedValue: 'encrypted:mounted-secret' }], - scope: { userId: 'user-123', workspaceId: 'workspace-1' }, + const response = await POST( + createMockRequest( + 'POST', + { + code: 'print("done")', + language: 'python', + workspaceId: 'workspace-1', + outputs: { + files: [ + { + path: 'files/small.jpg', + sandboxPath: '/home/user/small.jpg', + mimeType: 'image/jpeg', }, - }, - ], + ], + }, + [PRIVATE_SECRET_PROVENANCE_FIELD]: { + version: 1, + complete: true, + selections: [ + { + key: MOUNTED_WORKSPACE_FILES_PROVENANCE_KEY, + provenance: { + version: 1, + complete: true, + entries: [{ encryptedValue: 'encrypted:mounted-secret' }], + scope: { userId: 'user-123', workspaceId: 'workspace-1' }, + }, + }, + ], + }, }, - }, - { - [PRIVATE_SECRET_PROVENANCE_HEADER]: PRIVATE_SECRET_PROVENANCE_BUNDLE_V1, - } + { + [PRIVATE_SECRET_PROVENANCE_HEADER]: PRIVATE_SECRET_PROVENANCE_BUNDLE_V1, + } + ) ) - ) - expect(response.status).toBe(200) - expect(mockWriteWorkspaceFileByPath).toHaveBeenCalledWith( - expect.objectContaining({ secretProvenance: { status: 'unknown' } }) - ) - }) + expect(response.status).toBe(200) + expect(mockWriteWorkspaceFileByPath).toHaveBeenCalledWith( + expect.objectContaining({ + secretProvenance: + expectedStatus === 'exact' ? { status: 'exact', entries: [] } : { status: 'unknown' }, + }) + ) + } + ) it('marks binary exports unknown without failing the Function execution', async () => { envFlagsMock.isRemoteSandboxEnabled = true diff --git a/apps/sim/lib/function-execution/execute-request.ts b/apps/sim/lib/function-execution/execute-request.ts index 020b7b32f9d..de66f8dbfe9 100644 --- a/apps/sim/lib/function-execution/execute-request.ts +++ b/apps/sim/lib/function-execution/execute-request.ts @@ -2452,6 +2452,7 @@ export async function executeFunctionRequest( * owner's provenance and the file still locks. */ if (routeContext.unredactedSecretNames.has(name)) continue + if (isNonIdentifyingSecretLiteral(plaintext)) continue const scanLiterals = new Set([plaintext, JSON.stringify(plaintext).slice(1, -1)]) for (const scanLiteral of scanLiterals) { const names = routeContext.outputSecretNamesByScanLiteral.get(scanLiteral) ?? []