diff --git a/apps/sim/lib/copilot/tools/server/files/doc-compile.ts b/apps/sim/lib/copilot/tools/server/files/doc-compile.ts index 01fce21d370..4b26ebe4c7d 100644 --- a/apps/sim/lib/copilot/tools/server/files/doc-compile.ts +++ b/apps/sim/lib/copilot/tools/server/files/doc-compile.ts @@ -541,8 +541,9 @@ export async function resolveServableDocBytes(args: { } } - // Reaches here only for xlsx, which has no isolated-vm fallback. - if (!format) return { buffer: rawBuffer, contentType: getContentType(fileName) } + // Reaches here only for xlsx, which has no isolated-vm fallback. Returning these + // bytes would expose generation source as a spreadsheet. + if (!format) throw new DocCompileUserError('Document is still being generated') const cacheKey = sha256Hex(`${ext}${source}${workspaceId ?? ''}`) const cached = compiledDocCache.get(cacheKey) diff --git a/apps/sim/lib/copilot/tools/server/files/doc-servable.test.ts b/apps/sim/lib/copilot/tools/server/files/doc-servable.test.ts index 93e17d8f21c..17d4ec964af 100644 --- a/apps/sim/lib/copilot/tools/server/files/doc-servable.test.ts +++ b/apps/sim/lib/copilot/tools/server/files/doc-servable.test.ts @@ -158,14 +158,30 @@ describe('resolveServableDocBytes', () => { expect(mockRunSandboxTask).not.toHaveBeenCalled() }) - it('returns raw XLSX source when there is no workspaceId (xlsx has no isolated-vm path)', async () => { - const result = await resolveServableDocBytes({ - rawBuffer: XLSX_SOURCE, - fileName: 'sheet.xlsx', - workspaceId: undefined, - }) + it('throws instead of returning XLSX source when E2B is disabled', async () => { + mockLoadCompiledDoc.mockResolvedValue(null) + setEnvFlags({ isDocSandboxEnabled: false }) + + await expect( + resolveServableDocBytes({ + rawBuffer: XLSX_SOURCE, + fileName: 'sheet.xlsx', + workspaceId: WORKSPACE_ID, + }) + ).rejects.toBeInstanceOf(DocCompileUserError) + + expect(mockRunSandboxTask).not.toHaveBeenCalled() + }) + + it('throws instead of returning XLSX source when there is no workspaceId', async () => { + await expect( + resolveServableDocBytes({ + rawBuffer: XLSX_SOURCE, + fileName: 'sheet.xlsx', + workspaceId: undefined, + }) + ).rejects.toBeInstanceOf(DocCompileUserError) - expect(result.buffer).toBe(XLSX_SOURCE) expect(mockLoadCompiledDoc).not.toHaveBeenCalled() expect(mockRunSandboxTask).not.toHaveBeenCalled() }) diff --git a/apps/sim/lib/execution/payloads/materialization.server.test.ts b/apps/sim/lib/execution/payloads/materialization.server.test.ts new file mode 100644 index 00000000000..c2a649e2e9d --- /dev/null +++ b/apps/sim/lib/execution/payloads/materialization.server.test.ts @@ -0,0 +1,56 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockDownloadServableFileFromStorage, mockVerifyFileAccess } = vi.hoisted(() => ({ + mockDownloadServableFileFromStorage: vi.fn(), + mockVerifyFileAccess: vi.fn(), +})) + +vi.mock('@/lib/uploads/utils/file-utils.server', () => ({ + downloadServableFileFromStorage: mockDownloadServableFileFromStorage, +})) + +vi.mock('@/app/api/files/authorization', () => ({ + verifyFileAccess: mockVerifyFileAccess, +})) + +import { readUserFileContent } from '@/lib/execution/payloads/materialization.server' +import type { UserFile } from '@/executor/types' + +const PDF_SOURCE = Buffer.from('from reportlab.pdfgen import canvas') +const PDF_BYTES = Buffer.from('%PDF-1.4 rendered bytes') + +const generatedPdf: UserFile = { + id: 'file-1', + name: 'report.pdf', + url: '', + size: PDF_SOURCE.length, + type: 'text/x-python-pdf', + key: 'workspace/2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f/1700000000000-abc1234-report.pdf', +} + +describe('readUserFileContent', () => { + beforeEach(() => { + vi.clearAllMocks() + generatedPdf.size = PDF_SOURCE.length + mockVerifyFileAccess.mockResolvedValue(true) + mockDownloadServableFileFromStorage.mockResolvedValue({ + buffer: PDF_BYTES, + contentType: 'application/pdf', + }) + }) + + it('returns the compiled artifact instead of the stored generation source', async () => { + const content = await readUserFileContent(generatedPdf, { + userId: 'user-1', + encoding: 'base64', + }) + + expect(mockDownloadServableFileFromStorage).toHaveBeenCalledOnce() + expect(content).toBe(PDF_BYTES.toString('base64')) + expect(content).not.toBe(PDF_SOURCE.toString('base64')) + expect(generatedPdf.size).toBe(PDF_BYTES.length) + }) +}) diff --git a/apps/sim/lib/execution/payloads/materialization.server.ts b/apps/sim/lib/execution/payloads/materialization.server.ts index 58093bed715..e929d2c9cbf 100644 --- a/apps/sim/lib/execution/payloads/materialization.server.ts +++ b/apps/sim/lib/execution/payloads/materialization.server.ts @@ -10,8 +10,12 @@ import { } from '@/lib/execution/payloads/large-value-ref' import { ExecutionResourceLimitError } from '@/lib/execution/resource-errors' import type { StorageContext } from '@/lib/uploads' -import { bufferToBase64, inferContextFromKey } from '@/lib/uploads/utils/file-utils' -import { downloadFileFromStorage } from '@/lib/uploads/utils/file-utils.server' +import { + bufferToBase64, + inferContextFromKey, + isGeneratedDocumentSourceType, +} from '@/lib/uploads/utils/file-utils' +import { downloadServableFileFromStorage } from '@/lib/uploads/utils/file-utils.server' import type { UserFile } from '@/executor/types' const logger = createLogger('ExecutionPayloadMaterialization') @@ -267,6 +271,11 @@ export async function assertUserFileContentAccess( } } +/** + * Reads the bytes a consumer should receive. For generated documents, updates the + * file's size to the rendered artifact size so downstream attachment routing does + * not make decisions from the smaller generation-source size. + */ export async function readUserFileContent( file: unknown, options: ReadUserFileContentOptions @@ -291,9 +300,14 @@ export async function readUserFileContent( const requestId = options.requestId ?? 'unknown' try { - buffer = await downloadFileFromStorage(file, requestId, log, { maxBytes: maxSourceBytes }) + buffer = ( + await downloadServableFileFromStorage(file, requestId, log, { maxBytes: maxSourceBytes }) + ).buffer } catch (error) { if (isPayloadSizeLimitError(error)) { + if (isGeneratedDocumentSourceType(file.type) && error.observedBytes !== undefined) { + file.size = error.observedBytes + } throw new ExecutionResourceLimitError({ resource: 'execution_payload_bytes', attemptedBytes: error.observedBytes ?? maxSourceBytes + 1, @@ -306,6 +320,9 @@ export async function readUserFileContent( if (!buffer) { throw new Error(`File content for ${file.name} is unavailable.`) } + if (isGeneratedDocumentSourceType(file.type)) { + file.size = buffer.length + } if (buffer.length > maxSourceBytes) { throw new ExecutionResourceLimitError({ resource: 'execution_payload_bytes', diff --git a/apps/sim/lib/uploads/utils/file-utils.server.test.ts b/apps/sim/lib/uploads/utils/file-utils.server.test.ts index 40e9ec97044..19f8a4dea71 100644 --- a/apps/sim/lib/uploads/utils/file-utils.server.test.ts +++ b/apps/sim/lib/uploads/utils/file-utils.server.test.ts @@ -3,27 +3,51 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockDownloadFile } = vi.hoisted(() => ({ - mockDownloadFile: vi.fn(), -})) +const { mockDownloadFile, mockParseWorkspaceFileKey, mockResolveServableDocBytes } = vi.hoisted( + () => ({ + mockDownloadFile: vi.fn(), + mockParseWorkspaceFileKey: vi.fn(), + mockResolveServableDocBytes: vi.fn(), + }) +) vi.mock('@/lib/uploads/core/storage-service', () => ({ downloadFile: mockDownloadFile, hasCloudStorage: vi.fn(() => true), })) +vi.mock('@/lib/uploads/contexts/execution/execution-file-manager', () => ({ + downloadExecutionFile: mockDownloadFile, +})) + +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ + parseWorkspaceFileKey: mockParseWorkspaceFileKey, +})) + +vi.mock('@/lib/copilot/tools/server/files/doc-compile', () => ({ + resolveServableDocBytes: mockResolveServableDocBytes, +})) + vi.mock('@/app/api/files/authorization', () => ({ verifyFileAccess: vi.fn(), })) import { createLogger } from '@sim/logger' -import { downloadFileFromStorage } from '@/lib/uploads/utils/file-utils.server' +import { + downloadFileFromStorage, + downloadServableFileFromStorage, +} from '@/lib/uploads/utils/file-utils.server' import type { UserFile } from '@/executor/types' describe('downloadFileFromStorage context derivation', () => { beforeEach(() => { vi.clearAllMocks() mockDownloadFile.mockResolvedValue(Buffer.from('bytes')) + mockParseWorkspaceFileKey.mockReturnValue(null) + mockResolveServableDocBytes.mockImplementation(async ({ rawBuffer }) => ({ + buffer: rawBuffer, + contentType: 'application/pdf', + })) }) it('downloads with the key-derived context, ignoring a caller-supplied public context', async () => { @@ -44,4 +68,23 @@ describe('downloadFileFromStorage context derivation', () => { expect.objectContaining({ key: userFile.key, context: 'workspace' }) ) }) + + it('uses the workspace ID embedded in an execution key to resolve generated artifacts', async () => { + const workspaceId = '2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f' + const userFile: UserFile = { + id: 'f1', + name: 'report.pdf', + url: '', + size: 5, + type: 'text/x-python-pdf', + key: `execution/${workspaceId}/3f2e9d4c-6a7b-4d8e-9f0a-1b2c3d4e5f6a/4a3b2c1d-7e8f-4a9b-8c0d-1e2f3a4b5c6d/report.pdf`, + context: 'execution', + } + + await downloadServableFileFromStorage(userFile, 'req-1', createLogger('test')) + + expect(mockResolveServableDocBytes).toHaveBeenCalledWith( + expect.objectContaining({ workspaceId }) + ) + }) }) diff --git a/apps/sim/lib/uploads/utils/file-utils.server.ts b/apps/sim/lib/uploads/utils/file-utils.server.ts index 97013439e77..da6ddf1aaef 100644 --- a/apps/sim/lib/uploads/utils/file-utils.server.ts +++ b/apps/sim/lib/uploads/utils/file-utils.server.ts @@ -16,6 +16,7 @@ import { StorageService } from '@/lib/uploads' import { isExecutionFile } from '@/lib/uploads/contexts/execution/utils' import { extractStorageKey, + extractWorkspaceIdFromExecutionKey, getFileExtension, getMimeTypeFromExtension, inferContextFromKey, @@ -384,7 +385,11 @@ export async function downloadServableFileFromStorage( const { parseWorkspaceFileKey } = await import( '@/lib/uploads/contexts/workspace/workspace-file-manager' ) - const workspaceId = userFile.key ? (parseWorkspaceFileKey(userFile.key) ?? undefined) : undefined + const workspaceId = userFile.key + ? (parseWorkspaceFileKey(userFile.key) ?? + extractWorkspaceIdFromExecutionKey(userFile.key) ?? + undefined) + : undefined const { resolveServableDocBytes } = await import('@/lib/copilot/tools/server/files/doc-compile') const resolved = await resolveServableDocBytes({ diff --git a/apps/sim/lib/uploads/utils/user-file-base64.server.test.ts b/apps/sim/lib/uploads/utils/user-file-base64.server.test.ts index 6c6cc70611a..6e62f378da9 100644 --- a/apps/sim/lib/uploads/utils/user-file-base64.server.test.ts +++ b/apps/sim/lib/uploads/utils/user-file-base64.server.test.ts @@ -9,24 +9,26 @@ import { } from '@/lib/uploads/utils/user-file-base64.server' import type { UserFile } from '@/executor/types' -const { mockDownloadFile, mockRedis, mockVerifyFileAccess } = vi.hoisted(() => { - const mockRedis = { - get: vi.fn(), - set: vi.fn(), - hget: vi.fn(), - hset: vi.fn(), - hgetall: vi.fn(), - expire: vi.fn(), - scan: vi.fn(), - del: vi.fn(), - eval: vi.fn(), - } - return { - mockDownloadFile: vi.fn(), - mockRedis, - mockVerifyFileAccess: vi.fn(), - } -}) +const { mockDownloadFile, mockDownloadServableFileFromStorage, mockRedis, mockVerifyFileAccess } = + vi.hoisted(() => { + const mockRedis = { + get: vi.fn(), + set: vi.fn(), + hget: vi.fn(), + hset: vi.fn(), + hgetall: vi.fn(), + expire: vi.fn(), + scan: vi.fn(), + del: vi.fn(), + eval: vi.fn(), + } + return { + mockDownloadFile: vi.fn(), + mockDownloadServableFileFromStorage: vi.fn(), + mockRedis, + mockVerifyFileAccess: vi.fn(), + } + }) const mockGetRedisClient = redisConfigMockFns.mockGetRedisClient @@ -44,6 +46,7 @@ vi.mock('@/lib/uploads/contexts/execution/execution-file-manager', () => ({ vi.mock('@/lib/uploads/utils/file-utils.server', () => ({ downloadFileFromStorage: mockDownloadFile, + downloadServableFileFromStorage: mockDownloadServableFileFromStorage, })) vi.mock('@/app/api/files/authorization', () => ({ @@ -64,6 +67,10 @@ describe('hydrateUserFilesWithBase64', () => { mockRedis.del.mockResolvedValue(1) mockRedis.eval.mockResolvedValue([1, 'ok', 0, 0]) mockVerifyFileAccess.mockResolvedValue(true) + mockDownloadServableFileFromStorage.mockImplementation(async (file: unknown) => ({ + buffer: await mockDownloadFile(file), + contentType: 'application/octet-stream', + })) }) it('strips existing base64 when it exceeds maxBytes', async () => { @@ -101,6 +108,84 @@ describe('hydrateUserFilesWithBase64', () => { expect(hydrated.file.base64).toBe(base64) }) + it('uses rendered size when generated source metadata exceeds the inline limit', async () => { + const rendered = Buffer.from('%PDF') + mockDownloadServableFileFromStorage.mockResolvedValueOnce({ + buffer: rendered, + contentType: 'application/pdf', + }) + const file: UserFile = { + id: 'file-1', + name: 'report.pdf', + key: 'workspace/2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f/report.pdf', + url: '', + size: 11, + type: 'text/x-python-pdf', + } + + const hydrated = await hydrateUserFilesWithBase64({ file }, { maxBytes: 10, userId: 'user-1' }) + + expect(hydrated.file.base64).toBe(rendered.toString('base64')) + expect(hydrated.file.size).toBe(rendered.length) + }) + + it('records rendered size when a generated document must use a provider upload path', async () => { + mockDownloadServableFileFromStorage.mockResolvedValueOnce({ + buffer: Buffer.alloc(11), + contentType: 'application/pdf', + }) + const file: UserFile = { + id: 'file-1', + name: 'report.pdf', + key: 'workspace/2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f/report.pdf', + url: '', + size: 1, + type: 'text/x-python-pdf', + } + + const hydrated = await hydrateUserFilesWithBase64({ file }, { maxBytes: 10, userId: 'user-1' }) + + expect(hydrated.file).not.toHaveProperty('base64') + expect(hydrated.file.size).toBe(11) + }) + + it('records cached rendered size when a generated document must use a provider upload path', async () => { + mockGetRedisClient.mockReturnValue(mockRedis) + mockRedis.get.mockResolvedValueOnce(Buffer.alloc(11).toString('base64')) + const file: UserFile = { + id: 'file-1', + name: 'report.pdf', + key: 'workspace/2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f/report.pdf', + url: '', + size: 1, + type: 'text/x-python-pdf', + } + + const hydrated = await hydrateUserFilesWithBase64({ file }, { maxBytes: 10, userId: 'user-1' }) + + expect(hydrated.file).not.toHaveProperty('base64') + expect(hydrated.file.size).toBe(11) + expect(mockDownloadServableFileFromStorage).not.toHaveBeenCalled() + }) + + it('propagates generated documents that are still compiling', async () => { + const notReady = new Error('Document is still being generated') + notReady.name = 'DocCompileUserError' + mockDownloadServableFileFromStorage.mockRejectedValueOnce(notReady) + const file: UserFile = { + id: 'file-1', + name: 'report.pdf', + key: 'workspace/2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f/report.pdf', + url: '', + size: 1, + type: 'text/x-python-pdf', + } + + await expect( + hydrateUserFilesWithBase64({ file }, { maxBytes: 10, userId: 'user-1' }) + ).rejects.toBe(notReady) + }) + it('does not hydrate URL-only internal file objects', async () => { const file: UserFile = { id: 'file-1', diff --git a/apps/sim/lib/uploads/utils/user-file-base64.server.ts b/apps/sim/lib/uploads/utils/user-file-base64.server.ts index 2aa3abda185..ba0407dd766 100644 --- a/apps/sim/lib/uploads/utils/user-file-base64.server.ts +++ b/apps/sim/lib/uploads/utils/user-file-base64.server.ts @@ -27,6 +27,7 @@ import { ExecutionResourceLimitError, isExecutionResourceLimitError, } from '@/lib/execution/resource-errors' +import { isGeneratedDocumentSourceType } from '@/lib/uploads/utils/file-utils' import type { UserFile } from '@/executor/types' const INLINE_BASE64_JSON_OVERHEAD_BYTES = 512 * 1024 @@ -391,7 +392,11 @@ async function resolveBase64( const allowUnknownSize = options.allowUnknownSize ?? false const hasStableStorageKey = Boolean(file.key) - if (Number.isFinite(file.size) && file.size > maxBytes) { + if ( + !isGeneratedDocumentSourceType(file.type) && + Number.isFinite(file.size) && + file.size > maxBytes + ) { logger.warn( `[${options.requestId}] Skipping base64 for ${file.name} (size ${file.size} exceeds ${maxBytes})` ) @@ -420,9 +425,11 @@ async function resolveBase64( userId: options.userId, encoding: 'base64', maxBytes, - maxSourceBytes: maxBytes, }) } catch (error) { + if (error instanceof Error && error.name === 'DocCompileUserError') { + throw error + } logger.warn(`[${requestId}] Failed to hydrate base64 for ${file.name}`, error) return null } @@ -456,7 +463,11 @@ async function hydrateUserFile( const cached = await state.cache.get(file) if (cached) { const maxBytes = options.maxBytes ?? DEFAULT_MAX_BASE64_BYTES - if (Buffer.byteLength(cached, 'base64') > maxBytes) { + const cachedBytes = Buffer.byteLength(cached, 'base64') + if (isGeneratedDocumentSourceType(file.type)) { + file.size = cachedBytes + } + if (cachedBytes > maxBytes) { return stripBase64(file) } return { ...file, base64: cached } diff --git a/apps/sim/providers/attachments.test.ts b/apps/sim/providers/attachments.test.ts index e141a292505..6ac38dc2b76 100644 --- a/apps/sim/providers/attachments.test.ts +++ b/apps/sim/providers/attachments.test.ts @@ -59,6 +59,15 @@ describe('provider attachments', () => { ).toBe('image/png') }) + it('infers MIME type from filename when file type is a generated-doc source marker', () => { + expect( + inferAttachmentMimeType({ + ...pdfFile, + type: 'text/x-python-pdf', + }) + ).toBe('application/pdf') + }) + it('formats OpenAI Responses content with text, image, and file parts', () => { const content = buildOpenAIMessageContent( 'Analyze these files', @@ -301,6 +310,16 @@ describe('provider large-file capability', () => { expect(shouldUseLargeFilePath(large, 'bedrock')).toBe(false) }) + it('does not expose generated source through a remote-url large-file path', () => { + const generated = { + ...pdfFile, + size: INLINE_ATTACHMENT_THRESHOLD_BYTES + 1, + type: 'text/x-python-pdf', + } + expect(shouldUseLargeFilePath(generated, 'openai')).toBe(true) + expect(shouldUseLargeFilePath(generated, 'anthropic')).toBe(false) + }) + it('references uploaded OpenAI files by file_id instead of inlining base64', () => { const content = buildOpenAIMessageContent( 'Analyze', diff --git a/apps/sim/providers/attachments.ts b/apps/sim/providers/attachments.ts index 71ead60cd0a..0cc4ebc5d6b 100644 --- a/apps/sim/providers/attachments.ts +++ b/apps/sim/providers/attachments.ts @@ -6,9 +6,10 @@ import { getContentType, getExtensionFromMimeType, getFileExtension, - getMimeTypeFromExtension, + isGeneratedDocumentSourceType, MIME_TYPE_MAPPING, MODEL_SUPPORTED_IMAGE_MIME_TYPES, + resolveFileType, } from '@/lib/uploads/utils/file-utils' import type { UserFile } from '@/executor/types' import { @@ -88,12 +89,18 @@ export function getProviderFileStrategy(providerId: ProviderId | string): Provid return getProviderFileAttachment(providerId).strategy } -/** True when a file exceeds the inline threshold and the provider has a large-file path. */ +/** + * True when an oversized file has a safe provider path. Remote URLs point at the + * primary storage object, so source-backed documents can only use artifact-aware + * Files API uploads. + */ export function shouldUseLargeFilePath( - file: Pick, + file: Pick, providerId: ProviderId | string ): boolean { - if (getProviderFileAttachment(providerId).strategy === 'inline') return false + const strategy = getProviderFileAttachment(providerId).strategy + if (strategy === 'inline') return false + if (strategy === 'remote-url' && isGeneratedDocumentSourceType(file.type)) return false return Number.isFinite(file.size) && file.size > INLINE_ATTACHMENT_THRESHOLD_BYTES } @@ -197,12 +204,10 @@ export function getProviderAttachmentMaxBytes(providerId: ProviderId | string): export function inferAttachmentMimeType(file: UserFile): string { const explicitType = file.type?.trim().toLowerCase() - if (explicitType && explicitType !== 'application/octet-stream') { - return explicitType - } - - const inferred = getMimeTypeFromExtension(getFileExtension(file.name)) - return inferred.toLowerCase() + return resolveFileType({ + name: file.name, + type: isGeneratedDocumentSourceType(explicitType) ? '' : (explicitType ?? ''), + }).toLowerCase() } function isTextDocumentMimeType(mimeType: string): boolean {