From beeb42547d05044f0b7458e16e475739445d8283 Mon Sep 17 00:00:00 2001 From: Bill Leoutsakos Date: Wed, 9 Sep 2026 11:01:09 -0700 Subject: [PATCH] fix(files): persist large Agiloft and Cursor downloads before serialization --- .agents/skills/add-integration/SKILL.md | 65 ++--- .../sim/executor/utils/file-tool-processor.ts | 33 +-- apps/sim/lib/api/contracts/tools/agiloft.ts | 10 +- .../lib/internal/agiloft/execute-tool.test.ts | 13 +- apps/sim/lib/internal/agiloft/execute-tool.ts | 3 + .../lib/internal/agiloft/operations.test.ts | 27 +- apps/sim/lib/internal/agiloft/operations.ts | 52 +++- .../lib/internal/cursor/execute-tool.test.ts | 2 +- apps/sim/lib/internal/cursor/execute-tool.ts | 10 + .../lib/internal/cursor/operations.test.ts | 4 + apps/sim/lib/internal/cursor/operations.ts | 55 ++++- .../file-output-boundary.test.ts | 231 ++++++++++++++++++ apps/sim/lib/uploads/utils/validation.ts | 32 +++ apps/sim/tools/agiloft/retrieve_attachment.ts | 15 +- apps/sim/tools/agiloft/types.ts | 8 +- apps/sim/tools/cursor/types.ts | 8 +- 16 files changed, 433 insertions(+), 135 deletions(-) create mode 100644 apps/sim/lib/internal/tool-operations/file-output-boundary.test.ts diff --git a/.agents/skills/add-integration/SKILL.md b/.agents/skills/add-integration/SKILL.md index 850571a9cef..17cb705d688 100644 --- a/.agents/skills/add-integration/SKILL.md +++ b/.agents/skills/add-integration/SKILL.md @@ -523,46 +523,31 @@ registry/direct-handler test. There is no HTTP fallback. ### File Output Pattern (Downloads) -For tools that return files, use `FileToolProcessor` to store files and return `UserFile` objects. - -#### In Tool transformResponse - -```typescript -import { FileToolProcessor } from '@/executor/utils/file-tool-processor' - -transformResponse: async (response, context) => { - const data = await response.json() - - // Process file outputs to UserFile objects - const fileProcessor = new FileToolProcessor(context) - const file = await fileProcessor.processFileData({ - data: data.content, // base64 or buffer - mimeType: data.mimeType, - filename: data.filename, - }) - - return { - success: true, - output: { file }, - } -} -``` - -#### In the operation handler (for complex file handling) - -```typescript -// Return file data that FileToolProcessor can handle. No API route is involved. -return Response.json({ - success: true, - output: { - file: { - data: base64Content, - mimeType: 'application/pdf', - filename: 'document.pdf', - }, - }, -}) -``` +Declare downloads as `file` / `file[]` outputs and return canonical `UserFile` objects. +Internal operation responses are capped at 10 MiB **before** `transformResponse` and +`FileToolProcessor` run. Inline base64 expands the bytes by roughly one third, so it +cannot carry a download near that limit. Persist downloads in the server operation +**before `Response.json`**, not in a response transform. + +Follow `executeQuickBooksDownloadDocument` or `executeAgiloftRetrieveAttachment`: + +- Derive storage scope only from trusted `request.context`, never tool parameters. + Use `uploadExecutionFile` for a complete workspace/workflow/execution scope; + otherwise use `uploadCopilotFile` with the trusted user identity. Reject missing + storage authority before downloading. Do not fabricate an `ExecutionContext`. +- Keep provider authentication, DNS-pinned downloads, byte caps and cancellation. + Normalize image metadata with `resolveStoredFileMetadata` before uploading. +- Return the stored file unchanged through the response schema and transform; use + `userFileSchema` / `UserFile` rather than rebuilding a base64-only shape. + `FileToolProcessor` passes stored files through; the executor records them for + execution consumers. Do not call its private `processFileData` method. +- Surface storage failures instead of falling back to an oversized inline payload. + Storage helpers do not promise rollback when later execution steps fail. + +Test provider bytes larger than the inline JSON budget through the actual handler, +bounded response reader, transform and file processor, mocking provider/storage +boundaries only. Preserve explicit legacy base64 outputs when they are a separate +versioned contract; do not silently convert those outputs or raise the global cap. ### Key Helpers Reference diff --git a/apps/sim/executor/utils/file-tool-processor.ts b/apps/sim/executor/utils/file-tool-processor.ts index d66dea14763..6c665368b9a 100644 --- a/apps/sim/executor/utils/file-tool-processor.ts +++ b/apps/sim/executor/utils/file-tool-processor.ts @@ -4,19 +4,12 @@ import { isCanonicalBase64 } from '@/lib/api/contracts/primitives' import { isUserFile } from '@/lib/core/utils/user-file' import { uploadExecutionFile, uploadFileFromRawData } from '@/lib/uploads/contexts/execution' import { downloadFileFromUrl } from '@/lib/uploads/utils/file-utils.server' -import { MAX_FILE_SIZE, sniffImageContentType } from '@/lib/uploads/utils/validation' +import { MAX_FILE_SIZE, resolveStoredFileMetadata } from '@/lib/uploads/utils/validation' import type { ExecutionContext, UserFile } from '@/executor/types' import type { ToolDefinition, ToolFileData } from '@/tools/types' const logger = createLogger('FileToolProcessor') -const IMAGE_FILE_EXTENSIONS: Record = { - 'image/gif': 'gif', - 'image/jpeg': 'jpg', - 'image/png': 'png', - 'image/webp': 'webp', -} - /** * Strip a base64 `data:` URI prefix, leaving the encoded payload. An empty payload is * a legitimate zero-byte file; a payload that only looks empty after normalization is @@ -43,30 +36,6 @@ function assertFileSize(size: number, fileName: string): void { } } -function resolveStoredFileMetadata( - fileName: string, - declaredMimeType: string, - buffer: Buffer -): { fileName: string; mimeType: string } { - if (!declaredMimeType.startsWith('image/')) { - return { fileName, mimeType: declaredMimeType } - } - - const mimeType = sniffImageContentType(buffer) - if (!mimeType) { - return { - fileName: `${fileName.replace(/\.[^.]+$/, '')}.bin`, - mimeType: 'application/octet-stream', - } - } - - const extension = IMAGE_FILE_EXTENSIONS[mimeType] - return { - fileName: extension ? `${fileName.replace(/\.[^.]+$/, '')}.${extension}` : fileName, - mimeType, - } -} - /** * Processes tool outputs and converts file-typed outputs to UserFile objects. * This enables tools to return file data that gets automatically stored in the diff --git a/apps/sim/lib/api/contracts/tools/agiloft.ts b/apps/sim/lib/api/contracts/tools/agiloft.ts index 720a1e252e6..f1088b41b23 100644 --- a/apps/sim/lib/api/contracts/tools/agiloft.ts +++ b/apps/sim/lib/api/contracts/tools/agiloft.ts @@ -1,4 +1,5 @@ import { z } from 'zod' +import { userFileSchema } from '@/lib/api/contracts/primitives' import type { ContractBody, ContractBodyInput, @@ -18,17 +19,10 @@ const optionalText = z .nullish() .transform((value) => value ?? undefined) -const agiloftFileOutputSchema = z.object({ - name: z.string(), - mimeType: z.string(), - data: z.string(), - size: z.number(), -}) - export const agiloftRetrieveResponseSchema = z.object({ success: z.literal(true), output: z.object({ - file: agiloftFileOutputSchema, + file: userFileSchema, }), }) diff --git a/apps/sim/lib/internal/agiloft/execute-tool.test.ts b/apps/sim/lib/internal/agiloft/execute-tool.test.ts index d0c98cc55f8..33d86888e91 100644 --- a/apps/sim/lib/internal/agiloft/execute-tool.test.ts +++ b/apps/sim/lib/internal/agiloft/execute-tool.test.ts @@ -185,11 +185,14 @@ describe('executeAgiloftTool', () => { }) ) - expect(operationMocks.executeAgiloftCreateRecord).toHaveBeenCalledWith(input, { - requestId: 'request-1', - userId: 'user-origin', - signal: controller.signal, - }) + expect(operationMocks.executeAgiloftCreateRecord).toHaveBeenCalledWith( + input, + expect.objectContaining({ + requestId: 'request-1', + userId: 'user-origin', + signal: controller.signal, + }) + ) }) it('preserves non-object input and canonical validation envelopes', async () => { diff --git a/apps/sim/lib/internal/agiloft/execute-tool.ts b/apps/sim/lib/internal/agiloft/execute-tool.ts index e59c8ba22ba..8a1aa72eb69 100644 --- a/apps/sim/lib/internal/agiloft/execute-tool.ts +++ b/apps/sim/lib/internal/agiloft/execute-tool.ts @@ -77,6 +77,9 @@ async function executeOperation( const result = await operation(parsed.data, { requestId: request.requestId, userId: request.context.executorDelegationOrigin?.subjectUserId ?? request.context.userId, + workspaceId: request.context.workspaceId, + workflowId: request.context.workflowId, + executionId: request.context.executionId, signal: request.signal, }) request.signal?.throwIfAborted() diff --git a/apps/sim/lib/internal/agiloft/operations.test.ts b/apps/sim/lib/internal/agiloft/operations.test.ts index a9e373f3244..be6bebda755 100644 --- a/apps/sim/lib/internal/agiloft/operations.test.ts +++ b/apps/sim/lib/internal/agiloft/operations.test.ts @@ -20,6 +20,7 @@ const providerMocks = vi.hoisted(() => ({ const fileMocks = vi.hoisted(() => ({ resolveAgiloftAttachmentFile: vi.fn(), + uploadCopilotFile: vi.fn(), })) vi.mock('@/lib/core/security/input-validation.server', () => ({ @@ -28,6 +29,8 @@ vi.mock('@/lib/core/security/input-validation.server', () => ({ })) vi.mock('@/lib/internal/agiloft/client', () => clientMocks) vi.mock('@/lib/internal/agiloft/file-input', () => fileMocks) +vi.mock('@/lib/uploads/contexts/copilot/copilot-file-manager', () => fileMocks) +vi.mock('@/lib/uploads/contexts/execution', () => ({ uploadExecutionFile: vi.fn() })) import { executeAgiloftCreateRecord, @@ -148,6 +151,15 @@ describe('Agiloft operations', () => { it('bounds attachment downloads and preserves binary metadata', async () => { const controller = new AbortController() + const storedFile = { + id: 'file-1', + key: 'copilot/user-1/file-1', + url: '/api/files/serve/file-1', + name: 'evidence.txt', + type: 'text/plain', + size: 5, + } + fileMocks.uploadCopilotFile.mockResolvedValue(storedFile) providerMocks.secureFetchWithPinnedIP.mockResolvedValue( createResponse({ bytes: new TextEncoder().encode('hello'), @@ -160,20 +172,21 @@ describe('Agiloft operations', () => { const result = await executeAgiloftRetrieveAttachment( { ...BASE, recordId: '1', fieldName: 'files', position: '0' }, - { requestId: 'request-1', signal: controller.signal } + { requestId: 'request-1', userId: 'user-1', signal: controller.signal } ) expect(result).toEqual({ success: true, output: { - file: { - name: 'evidence.txt', - mimeType: 'text/plain', - data: Buffer.from('hello').toString('base64'), - size: 5, - }, + file: storedFile, }, }) + expect(fileMocks.uploadCopilotFile).toHaveBeenCalledWith({ + buffer: Buffer.from('hello'), + fileName: 'evidence.txt', + contentType: 'text/plain', + userId: 'user-1', + }) expect(providerMocks.secureFetchWithPinnedIP).toHaveBeenCalledWith( expect.stringContaining('/ewws/EWRetrieve'), '203.0.113.10', diff --git a/apps/sim/lib/internal/agiloft/operations.ts b/apps/sim/lib/internal/agiloft/operations.ts index a346fd60ef8..78bd4ae2987 100644 --- a/apps/sim/lib/internal/agiloft/operations.ts +++ b/apps/sim/lib/internal/agiloft/operations.ts @@ -65,7 +65,10 @@ import { getLockHttpMethod, parseFieldList, } from '@/lib/internal/agiloft/urls' +import { uploadCopilotFile } from '@/lib/uploads/contexts/copilot/copilot-file-manager' +import { uploadExecutionFile } from '@/lib/uploads/contexts/execution' import { resolveEffectiveMimeType } from '@/lib/uploads/utils/file-utils' +import { resolveStoredFileMetadata } from '@/lib/uploads/utils/validation' import type { AgiloftAsyncStatusResponse, AgiloftAttachmentInfoResponse, @@ -88,6 +91,9 @@ import type { ToolResponse } from '@/tools/types' export interface AgiloftOperationContext { requestId: string userId?: string + workspaceId?: string + workflowId?: string + executionId?: string signal?: AbortSignal } @@ -894,6 +900,20 @@ export async function executeAgiloftRetrieveAttachment( input: AgiloftRetrieveBody, context: AgiloftOperationContext ): Promise { + const executionContext = + context.workspaceId && context.workflowId && context.executionId + ? { + workspaceId: context.workspaceId, + workflowId: context.workflowId, + executionId: context.executionId, + } + : null + if (!executionContext && !context.userId) { + throw new AgiloftOperationError(401, { + success: false, + error: 'User context is required to store attachments', + }) + } let resolvedIP: string try { resolvedIP = await resolveAgiloftInstance(input.instanceUrl, context.signal) @@ -930,15 +950,25 @@ export async function executeAgiloftRetrieveAttachment( error: `Agiloft error: ${buffer.toString('utf8').slice(0, 300)}`, }) } - return { - success: true, - output: { - file: { - name: fileName, - mimeType: resolveEffectiveMimeType(contentType, fileName), - data: buffer.toString('base64'), - size: buffer.length, - }, - }, - } + const metadata = resolveStoredFileMetadata( + fileName, + resolveEffectiveMimeType(contentType, fileName), + buffer + ) + const file = executionContext + ? await uploadExecutionFile( + executionContext, + buffer, + metadata.fileName, + metadata.mimeType, + context.userId + ) + : await uploadCopilotFile({ + buffer, + fileName: metadata.fileName, + contentType: metadata.mimeType, + userId: context.userId!, + }) + context.signal?.throwIfAborted() + return { success: true, output: { file } } } diff --git a/apps/sim/lib/internal/cursor/execute-tool.test.ts b/apps/sim/lib/internal/cursor/execute-tool.test.ts index a290d6a7f06..ca8784c078b 100644 --- a/apps/sim/lib/internal/cursor/execute-tool.test.ts +++ b/apps/sim/lib/internal/cursor/execute-tool.test.ts @@ -47,7 +47,7 @@ describe('executeCursorTool', () => { expect(response.status).toBe(200) expect(mocks.downloadCursorArtifact).toHaveBeenCalledWith( { apiKey: 'cursor-key', agentId: 'agent-1', path: '/src/index.ts' }, - { requestId: 'request-1', signal: controller.signal } + expect.objectContaining({ requestId: 'request-1', signal: controller.signal }) ) } ) diff --git a/apps/sim/lib/internal/cursor/execute-tool.ts b/apps/sim/lib/internal/cursor/execute-tool.ts index 9436106fdec..ffc801ab01c 100644 --- a/apps/sim/lib/internal/cursor/execute-tool.ts +++ b/apps/sim/lib/internal/cursor/execute-tool.ts @@ -33,6 +33,16 @@ export const executeCursorTool: InternalToolOperationHandler = async (request) = await downloadCursorArtifact(parsed.data, { requestId: request.requestId, signal: request.signal, + ...(request.toolId === 'cursor_download_artifact_v2' + ? { + persistFile: true, + userId: + request.context.executorDelegationOrigin?.subjectUserId ?? request.context.userId, + workspaceId: request.context.workspaceId, + workflowId: request.context.workflowId, + executionId: request.context.executionId, + } + : {}), }) ) } catch (error) { diff --git a/apps/sim/lib/internal/cursor/operations.test.ts b/apps/sim/lib/internal/cursor/operations.test.ts index 1eff865fe46..e88022d593d 100644 --- a/apps/sim/lib/internal/cursor/operations.test.ts +++ b/apps/sim/lib/internal/cursor/operations.test.ts @@ -12,6 +12,10 @@ vi.mock('@/lib/core/security/input-validation.server', () => ({ secureFetchWithPinnedIP: mocks.secureFetchWithPinnedIP, validateUrlWithDNS: mocks.validateUrlWithDNS, })) +vi.mock('@/lib/uploads/contexts/copilot/copilot-file-manager', () => ({ + uploadCopilotFile: vi.fn(), +})) +vi.mock('@/lib/uploads/contexts/execution', () => ({ uploadExecutionFile: vi.fn() })) import { downloadCursorArtifact } from '@/lib/internal/cursor/operations' diff --git a/apps/sim/lib/internal/cursor/operations.ts b/apps/sim/lib/internal/cursor/operations.ts index 3365cd7969d..49b3c3ef8b9 100644 --- a/apps/sim/lib/internal/cursor/operations.ts +++ b/apps/sim/lib/internal/cursor/operations.ts @@ -9,6 +9,10 @@ import { readResponseTextWithLimit, } from '@/lib/core/utils/stream-limits' import { CursorOperationError } from '@/lib/internal/cursor/errors' +import { uploadCopilotFile } from '@/lib/uploads/contexts/copilot/copilot-file-manager' +import { uploadExecutionFile } from '@/lib/uploads/contexts/execution' +import { resolveStoredFileMetadata } from '@/lib/uploads/utils/validation' +import type { UserFile } from '@/executor/types' import type { DownloadArtifactParams } from '@/tools/cursor/types' const logger = createLogger('CursorOperations') @@ -23,6 +27,11 @@ interface CursorArtifactLocation { export interface CursorOperationContext { requestId: string signal?: AbortSignal + persistFile?: boolean + userId?: string + workspaceId?: string + workflowId?: string + executionId?: string } export async function downloadCursorArtifact( @@ -30,9 +39,20 @@ export async function downloadCursorArtifact( context: CursorOperationContext ): Promise<{ success: true - output: { file: { name: string; mimeType: string; data: string; size: number } } + output: { file: UserFile | { name: string; mimeType: string; data: string; size: number } } }> { context.signal?.throwIfAborted() + const executionContext = + context.workspaceId && context.workflowId && context.executionId + ? { + workspaceId: context.workspaceId, + workflowId: context.workflowId, + executionId: context.executionId, + } + : null + if (context.persistFile && !executionContext && !context.userId) { + throw new CursorOperationError('User context is required to store artifacts', 401) + } const authHeader = `Basic ${Buffer.from(`${input.apiKey}:`).toString('base64')}` const artifactResponse = await fetch( `https://api.cursor.com/v0/agents/${encodeURIComponent(input.agentId)}/artifacts/download?path=${encodeURIComponent(input.path)}`, @@ -83,11 +103,34 @@ export async function downloadCursorArtifact( const fileBuffer = Buffer.from(await downloadResponse.arrayBuffer()) context.signal?.throwIfAborted() - const file = { - name: input.path.split('/').pop() || 'artifact', - mimeType: downloadResponse.headers.get('content-type') || 'application/octet-stream', - data: fileBuffer.toString('base64'), - size: fileBuffer.length, + const fileName = input.path.split('/').pop() || 'artifact' + const mimeType = downloadResponse.headers.get('content-type') || 'application/octet-stream' + let file: UserFile | { name: string; mimeType: string; data: string; size: number } + if (context.persistFile) { + const metadata = resolveStoredFileMetadata(fileName, mimeType, fileBuffer) + file = executionContext + ? await uploadExecutionFile( + executionContext, + fileBuffer, + metadata.fileName, + metadata.mimeType, + context.userId + ) + : await uploadCopilotFile({ + buffer: fileBuffer, + fileName: metadata.fileName, + contentType: metadata.mimeType, + userId: context.userId!, + }) + context.signal?.throwIfAborted() + } else { + // V1 exposes base64 metadata rather than a file-typed output. + file = { + name: fileName, + mimeType, + data: fileBuffer.toString('base64'), + size: fileBuffer.length, + } } logger.info(`[${context.requestId}] Cursor artifact downloaded`, { agentId: input.agentId, diff --git a/apps/sim/lib/internal/tool-operations/file-output-boundary.test.ts b/apps/sim/lib/internal/tool-operations/file-output-boundary.test.ts new file mode 100644 index 00000000000..a8907ab322f --- /dev/null +++ b/apps/sim/lib/internal/tool-operations/file-output-boundary.test.ts @@ -0,0 +1,231 @@ +/** + * @vitest-environment node + */ +import { createExecutionContext } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { InternalToolOperationCall } from '@/lib/internal/tool-operations/types' +import type { UserFile } from '@/executor/types' + +const mocks = vi.hoisted(() => ({ + secureFetchWithPinnedIP: vi.fn(), + validateUrlWithDNS: vi.fn(), + uploadExecutionFile: vi.fn(), + uploadCopilotFile: vi.fn(), + downloadFileFromUrl: vi.fn(), +})) + +vi.mock('@/lib/core/security/input-validation.server', () => ({ + MAX_JSON_API_RESPONSE_BYTES: 10 * 1024 * 1024, + secureFetchWithPinnedIP: mocks.secureFetchWithPinnedIP, + validateUrlWithDNS: mocks.validateUrlWithDNS, +})) +vi.mock('@/lib/uploads/contexts/execution', () => ({ + uploadExecutionFile: mocks.uploadExecutionFile, + uploadFileFromRawData: vi.fn(), +})) +vi.mock('@/lib/uploads/contexts/copilot/copilot-file-manager', () => ({ + uploadCopilotFile: mocks.uploadCopilotFile, +})) +vi.mock('@/lib/uploads/utils/file-utils.server', () => ({ + downloadFileFromUrl: mocks.downloadFileFromUrl, +})) +vi.mock('@/lib/internal/agiloft/file-input', () => ({ resolveAgiloftAttachmentFile: vi.fn() })) + +import { agiloftRetrieveResponseSchema } from '@/lib/api/contracts/tools/agiloft' +import { readResponseToBufferWithLimit } from '@/lib/core/utils/stream-limits' +import { executeAgiloftTool } from '@/lib/internal/agiloft/execute-tool' +import { executeCursorTool } from '@/lib/internal/cursor/execute-tool' +import { FileToolProcessor } from '@/executor/utils/file-tool-processor' +import { agiloftRetrieveAttachmentTool } from '@/tools/agiloft/retrieve_attachment' +import { downloadArtifactTool, downloadArtifactV2Tool } from '@/tools/cursor/download_artifact' + +const cases = [ + { + tool: agiloftRetrieveAttachmentTool, + handler: executeAgiloftTool, + input: { + instanceUrl: 'https://example.agiloft.com', + knowledgeBase: 'demo', + login: 'user', + password: 'test-password', + table: 'contracts', + recordId: '1', + fieldName: 'files', + position: '0', + }, + }, + { + tool: downloadArtifactV2Tool, + handler: executeCursorTool, + input: { apiKey: 'test-key', agentId: 'agent-1', path: '/files/evidence.bin' }, + }, +] + +const context = { + ...createExecutionContext({ workflowId: 'workflow-1', executionId: 'execution-1' }), + workspaceId: 'workspace-1', + userId: 'user-1', +} +const storedFile: UserFile = { + id: 'file-1', + key: 'execution/workspace-1/workflow-1/execution-1/file-1', + name: 'evidence.bin', + size: 8 * 1024 * 1024, + type: 'application/octet-stream', + url: '/api/files/serve/file-1', + context: 'execution', +} +const responseOptions = { maxBytes: 10 * 1024 * 1024, label: 'Tool response' } + +function request(toolId: string, input: unknown): InternalToolOperationCall { + return { toolId, input, context, requestId: 'request-1', headers: new Headers() } +} + +describe.each(cases)('$tool.id stored output boundary', ({ tool, handler, input }) => { + beforeEach(() => { + vi.resetAllMocks() + mocks.validateUrlWithDNS.mockResolvedValue({ isValid: true, resolvedIP: '203.0.113.1' }) + mocks.secureFetchWithPinnedIP.mockImplementation( + async () => + new Response(Buffer.alloc(8 * 1024 * 1024), { + headers: { + 'content-type': 'application/octet-stream', + 'content-disposition': 'attachment; filename="evidence.bin"', + }, + }) + ) + vi.stubGlobal( + 'fetch', + vi.fn(async () => Response.json({ url: 'https://download.example/artifact' })) + ) + mocks.uploadExecutionFile.mockResolvedValue(storedFile) + mocks.uploadCopilotFile.mockResolvedValue({ ...storedFile, context: 'copilot' }) + }) + + it('admits an 8 MiB download and preserves its stored identity through the real transform and processor', async () => { + const controller = new AbortController() + const call = request(tool.id, { + ...input, + workspaceId: 'forged-workspace', + userId: 'forged-user', + }) + call.signal = controller.signal + const response = await handler(call) + expect(response.status).toBe(200) + const bytes = await readResponseToBufferWithLimit(response, responseOptions) + expect(bytes.length).toBeLessThan(1024) + if (tool.id === 'agiloft_retrieve_attachment') { + expect(agiloftRetrieveResponseSchema.parse(JSON.parse(bytes.toString())).output.file).toEqual( + storedFile + ) + } + const transformed = await tool.transformResponse!(new Response(bytes)) + const output = await FileToolProcessor.processToolOutputs(transformed.output, tool, context) + expect(output.file).toEqual(storedFile) + expect(mocks.uploadExecutionFile).toHaveBeenCalledExactlyOnceWith( + { workspaceId: 'workspace-1', workflowId: 'workflow-1', executionId: 'execution-1' }, + expect.any(Buffer), + 'evidence.bin', + 'application/octet-stream', + 'user-1' + ) + expect(mocks.uploadExecutionFile.mock.calls[0][1].length).toBe(8 * 1024 * 1024) + expect(mocks.uploadCopilotFile).not.toHaveBeenCalled() + expect(mocks.downloadFileFromUrl).not.toHaveBeenCalled() + expect(mocks.secureFetchWithPinnedIP).toHaveBeenCalledWith( + expect.any(String), + '203.0.113.1', + expect.objectContaining({ signal: controller.signal }) + ) + }) + + it('uses the trusted delegated user for storage without an execution scope', async () => { + const call = request(tool.id, input) + call.context = { + workflowId: '', + userId: 'runtime-user', + executorDelegationOrigin: { + subjectUserId: 'origin-user', + workflowId: 'origin-workflow', + executionId: 'origin-execution', + }, + } + expect((await handler(call)).status).toBe(200) + expect(mocks.uploadCopilotFile).toHaveBeenCalledWith( + expect.objectContaining({ userId: 'origin-user' }) + ) + expect(mocks.uploadExecutionFile).not.toHaveBeenCalled() + }) + + it('allows a complete execution scope without a user identity', async () => { + const call = request(tool.id, input) + call.context = { ...context, userId: undefined } + expect((await handler(call)).status).toBe(200) + expect(mocks.uploadExecutionFile).toHaveBeenCalledWith( + { workspaceId: 'workspace-1', workflowId: 'workflow-1', executionId: 'execution-1' }, + expect.any(Buffer), + 'evidence.bin', + 'application/octet-stream', + undefined + ) + expect(mocks.uploadCopilotFile).not.toHaveBeenCalled() + }) + + it('rejects forged storage scope when trusted context has no owner', async () => { + const call = request(tool.id, { + ...input, + workspaceId: 'forged', + workflowId: 'forged', + executionId: 'forged', + userId: 'forged', + }) + call.context = { workflowId: '' } + expect((await handler(call)).status).toBe(401) + expect(mocks.secureFetchWithPinnedIP).not.toHaveBeenCalled() + expect(mocks.uploadExecutionFile).not.toHaveBeenCalled() + expect(mocks.uploadCopilotFile).not.toHaveBeenCalled() + }) + + it('surfaces storage failure without returning inline bytes', async () => { + mocks.uploadExecutionFile.mockRejectedValue(new Error('Storage unavailable')) + const response = await handler(request(tool.id, input)) + expect(response.status).toBe(500) + await expect(response.json()).resolves.toEqual({ success: false, error: 'Storage unavailable' }) + expect(mocks.uploadCopilotFile).not.toHaveBeenCalled() + }) +}) + +it('preserves Agiloft failure messages without manufacturing a file output', async () => { + await expect( + agiloftRetrieveAttachmentTool.transformResponse!( + Response.json({ success: false, error: 'Attachment unavailable' }) + ) + ).rejects.toThrow('Attachment unavailable') +}) + +it('keeps Cursor v1 metadata base64 through the real handler and transform', async () => { + vi.clearAllMocks() + vi.stubGlobal( + 'fetch', + vi.fn(async () => Response.json({ url: 'https://download.example/artifact' })) + ) + mocks.validateUrlWithDNS.mockResolvedValue({ isValid: true, resolvedIP: '203.0.113.1' }) + mocks.secureFetchWithPinnedIP.mockResolvedValue(new Response('legacy')) + const call = request(downloadArtifactTool.id, { + apiKey: 'test-key', + agentId: 'agent-1', + path: '/legacy.txt', + persistFile: true, + }) + call.context = { workflowId: '' } + const response = await executeCursorTool(call) + const transformed = await downloadArtifactTool.transformResponse!(response) + expect(transformed.output.metadata).toEqual({ + name: 'legacy.txt', + mimeType: 'text/plain;charset=UTF-8', + data: Buffer.from('legacy').toString('base64'), + size: 6, + }) + expect(mocks.uploadExecutionFile).not.toHaveBeenCalled() + expect(mocks.uploadCopilotFile).not.toHaveBeenCalled() +}) diff --git a/apps/sim/lib/uploads/utils/validation.ts b/apps/sim/lib/uploads/utils/validation.ts index a6bbd26681b..ed0804b5b25 100644 --- a/apps/sim/lib/uploads/utils/validation.ts +++ b/apps/sim/lib/uploads/utils/validation.ts @@ -14,6 +14,38 @@ function extractExtension(fileName: string): string { export const MAX_FILE_SIZE = 100 * 1024 * 1024 // 100MB +const IMAGE_FILE_EXTENSIONS: Record = { + 'image/gif': 'gif', + 'image/jpeg': 'jpg', + 'image/png': 'png', + 'image/webp': 'webp', +} + +/** Normalize tool-output image metadata before storing provider bytes. */ +export function resolveStoredFileMetadata( + fileName: string, + declaredMimeType: string, + buffer: Buffer +): { fileName: string; mimeType: string } { + if (!declaredMimeType.startsWith('image/')) { + return { fileName, mimeType: declaredMimeType } + } + + const mimeType = sniffImageContentType(buffer) + if (!mimeType) { + return { + fileName: `${fileName.replace(/\.[^.]+$/, '')}.bin`, + mimeType: 'application/octet-stream', + } + } + + const extension = IMAGE_FILE_EXTENSIONS[mimeType] + return { + fileName: extension ? `${fileName.replace(/\.[^.]+$/, '')}.${extension}` : fileName, + mimeType, + } +} + export const SUPPORTED_DOCUMENT_EXTENSIONS = [ 'pdf', 'csv', diff --git a/apps/sim/tools/agiloft/retrieve_attachment.ts b/apps/sim/tools/agiloft/retrieve_attachment.ts index ad2b2ab1953..517c0c63d54 100644 --- a/apps/sim/tools/agiloft/retrieve_attachment.ts +++ b/apps/sim/tools/agiloft/retrieve_attachment.ts @@ -81,24 +81,13 @@ export const agiloftRetrieveAttachmentTool: InternalToolConfig< const data = await response.json() if (!data.success) { - return { - success: false, - output: { - file: { name: '', mimeType: '', data: '', size: 0 }, - }, - error: data.error || 'Failed to retrieve attachment', - } + throw new Error(data.error || 'Failed to retrieve attachment') } return { success: true, output: { - file: { - name: data.output.file.name, - mimeType: data.output.file.mimeType, - data: data.output.file.data, - size: data.output.file.size, - }, + file: data.output.file, }, } }, diff --git a/apps/sim/tools/agiloft/types.ts b/apps/sim/tools/agiloft/types.ts index f21eb5c265c..835957126e7 100644 --- a/apps/sim/tools/agiloft/types.ts +++ b/apps/sim/tools/agiloft/types.ts @@ -1,3 +1,4 @@ +import type { UserFile } from '@/executor/types' import type { ToolResponse } from '@/tools/types' /** @@ -149,12 +150,7 @@ export interface AgiloftRetrieveAttachmentParams extends AgiloftBaseParams { export interface AgiloftRetrieveAttachmentResponse extends ToolResponse { output: { - file: { - name: string - mimeType: string - data: string - size: number - } + file: UserFile } } diff --git a/apps/sim/tools/cursor/types.ts b/apps/sim/tools/cursor/types.ts index 2dfcf38fa20..31159e509bd 100644 --- a/apps/sim/tools/cursor/types.ts +++ b/apps/sim/tools/cursor/types.ts @@ -1,3 +1,4 @@ +import type { UserFile } from '@/executor/types' import type { ToolResponse } from '@/tools/types' interface BaseCursorParams { @@ -218,12 +219,7 @@ export interface DownloadArtifactResponse extends ToolResponse { export interface DownloadArtifactV2Response extends ToolResponse { output: { - file: { - name: string - mimeType: string - data: string - size: number - } + file: UserFile } }