From fc2ded7425387d097e59819dda55a90580a2c8a6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 28 Jul 2026 10:26:45 -0700 Subject: [PATCH 1/4] fix(files): count the document body against the export limit The 250 MB export cap measured only the embedded assets' declared sizes. The markdown body was downloaded with no limit and never counted, so a large document with modest attachments cleared the check and still produced a zip well over the stated limit, materialized whole in memory. The body is the largest single entry in most bundles, so excluding it left the limit unenforced against the item most able to exceed it. It is now capped on read and counted alongside its assets, and the message names both. Follow-up to #5995, which introduced the asset caps without extending them to the body. --- .../app/api/files/export/[id]/route.test.ts | 20 +++++++++++++++++++ apps/sim/app/api/files/export/[id]/route.ts | 13 +++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/apps/sim/app/api/files/export/[id]/route.test.ts b/apps/sim/app/api/files/export/[id]/route.test.ts index 1056fd61e05..eac05dd1ebf 100644 --- a/apps/sim/app/api/files/export/[id]/route.test.ts +++ b/apps/sim/app/api/files/export/[id]/route.test.ts @@ -116,6 +116,26 @@ describe('markdown export bundling', () => { expect(mockDownloadFile).toHaveBeenCalledTimes(1) }) + it('counts the document body against the export limit, not just its assets', async () => { + // Assets alone sit under the cap; the body is what carries the bundle over it. + mockExtractEmbeddedImageIds.mockReturnValue(['a']) + mockDownloadFile.mockResolvedValue(Buffer.alloc(250 * MB)) + + const response = await GET(request(), context) + + expect(response.status).toBe(400) + expect((await response.json()).error).toContain('document and its embedded files') + }) + + it('caps the document body read rather than loading it unbounded', async () => { + mockExtractEmbeddedImageIds.mockReturnValue([]) + + await GET(request(), context) + + const bodyCall = mockDownloadFile.mock.calls.find(([options]) => options.key.endsWith('doc.md')) + expect(bodyCall?.[0].maxBytes).toBe(250 * MB) + }) + it('caps each asset download rather than trusting its declared size', async () => { mockExtractEmbeddedImageIds.mockReturnValue(['a']) diff --git a/apps/sim/app/api/files/export/[id]/route.ts b/apps/sim/app/api/files/export/[id]/route.ts index 6b217f9c6c1..78f714417ea 100644 --- a/apps/sim/app/api/files/export/[id]/route.ts +++ b/apps/sim/app/api/files/export/[id]/route.ts @@ -128,9 +128,13 @@ export const GET = withRouteHandler( return NextResponse.redirect(new URL(servePath, request.url), { status: 302 }) } + // Capped like everything else in the bundle: the document body is usually the + // largest single entry, so leaving it unbounded left the export limit unenforced + // against the one item most able to exceed it. const mdBuffer = await downloadFile({ key: record.key, context: record.context as StorageContext, + maxBytes: MAX_EXPORT_TOTAL_BYTES, }) let mdContent = mdBuffer.toString('utf-8') @@ -180,11 +184,14 @@ export const GET = withRouteHandler( }) ).filter((target): target is NonNullable => target !== null) - const declaredAssetBytes = assetTargets.reduce((sum, target) => sum + target.record.size, 0) - if (declaredAssetBytes > MAX_EXPORT_TOTAL_BYTES) { + // The body counts against the same budget as its assets — the zip holds both, so a + // limit that measured only the attachments would not describe the archive produced. + const bundleBytes = + mdBuffer.length + assetTargets.reduce((sum, target) => sum + target.record.size, 0) + if (bundleBytes > MAX_EXPORT_TOTAL_BYTES) { return NextResponse.json( { - error: `Embedded files total ${formatFileSize(declaredAssetBytes)}, which exceeds the ${formatFileSize(MAX_EXPORT_TOTAL_BYTES)} export limit.`, + error: `This document and its embedded files total ${formatFileSize(bundleBytes)}, which exceeds the ${formatFileSize(MAX_EXPORT_TOTAL_BYTES)} export limit.`, }, { status: 400 } ) From f16e140ea33d52498794be4ef6cceadc026b9b52 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 28 Jul 2026 10:30:14 -0700 Subject: [PATCH 2/4] test(v1): cover the file download's rendered-bytes behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The route had no tests, and #5995 changed what it serves: rendered bytes, the resolved content type rather than the record's source MIME, Content-Length from the rendered length, and a retryable 409 while an artifact compiles. One test pins the filename/content-type relationship. A review flagged the download as naming a rendered file with a source extension, but the renderer picks its output format from the file name — getE2BDocFormat and COMPILABLE_FORMATS both key on it — so a .docx renders to a docx and the two cannot disagree. The test makes that argument executable rather than a comment. --- .../app/api/v1/files/[fileId]/route.test.ts | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 apps/sim/app/api/v1/files/[fileId]/route.test.ts diff --git a/apps/sim/app/api/v1/files/[fileId]/route.test.ts b/apps/sim/app/api/v1/files/[fileId]/route.test.ts new file mode 100644 index 00000000000..9a85c408bb1 --- /dev/null +++ b/apps/sim/app/api/v1/files/[fileId]/route.test.ts @@ -0,0 +1,135 @@ +/** + * @vitest-environment node + */ +import { createMockRequest } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockCheckRateLimit, + mockValidateWorkspaceAccess, + mockGetWorkspaceFile, + mockFetchServableWorkspaceFileBuffer, +} = vi.hoisted(() => ({ + mockCheckRateLimit: vi.fn(), + mockValidateWorkspaceAccess: vi.fn(), + mockGetWorkspaceFile: vi.fn(), + mockFetchServableWorkspaceFileBuffer: vi.fn(), +})) + +vi.mock('@/app/api/v1/middleware', () => ({ + checkRateLimit: mockCheckRateLimit, + createRateLimitResponse: () => new Response('rate limited', { status: 429 }), + validateWorkspaceAccess: mockValidateWorkspaceAccess, +})) +vi.mock('@/lib/uploads/contexts/workspace', () => ({ + getWorkspaceFile: mockGetWorkspaceFile, + fetchServableWorkspaceFileBuffer: mockFetchServableWorkspaceFileBuffer, +})) +vi.mock('@/lib/workspace-files/orchestration', () => ({ + performDeleteWorkspaceFileItems: vi.fn(), +})) +vi.mock('@sim/audit', () => ({ + recordAudit: vi.fn(), + AuditAction: { FILE_DOWNLOADED: 'file.downloaded', FILE_DELETED: 'file.deleted' }, + AuditResourceType: { FILE: 'file' }, +})) +vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: vi.fn() })) + +import { DocCompileUserError } from '@/lib/copilot/tools/server/files/doc-compile' +import { GET } from '@/app/api/v1/files/[fileId]/route' + +const WORKSPACE_ID = 'ws-1' +const FILE_ID = 'file-1' +const context = { params: Promise.resolve({ fileId: FILE_ID }) } + +const DOCX_MIME = 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' + +function request() { + return createMockRequest( + 'GET', + undefined, + {}, + `http://localhost:3000/api/v1/files/${FILE_ID}?workspaceId=${WORKSPACE_ID}` + ) +} + +/** A generated document: the name carries the target extension, the type the source. */ +function generatedDocument(name = 'report.docx') { + return { + id: FILE_ID, + workspaceId: WORKSPACE_ID, + name, + key: `workspace/${WORKSPACE_ID}/${FILE_ID}`, + path: `/serve/${FILE_ID}`, + size: 6_242, + type: 'text/x-docxjs', + uploadedBy: 'user-1', + uploadedAt: new Date('2026-01-01'), + updatedAt: new Date('2026-01-01'), + } +} + +describe('v1 file download', () => { + beforeEach(() => { + vi.clearAllMocks() + mockCheckRateLimit.mockResolvedValue({ allowed: true, userId: 'user-1' }) + mockValidateWorkspaceAccess.mockResolvedValue(null) + mockGetWorkspaceFile.mockResolvedValue(generatedDocument()) + mockFetchServableWorkspaceFileBuffer.mockResolvedValue({ + buffer: Buffer.from('PKrendered'), + contentType: DOCX_MIME, + }) + }) + + it('serves the rendered bytes and the rendered content type', async () => { + const response = await GET(request(), context) + + expect(response.status).toBe(200) + // Not the record's `text/x-docxjs`, which describes the stored source. + expect(response.headers.get('Content-Type')).toBe(DOCX_MIME) + expect(Buffer.from(await response.arrayBuffer()).toString()).toContain('rendered') + }) + + it('names the download with an extension matching the served content type', async () => { + // The renderer picks its output format from the file name, so the two cannot + // disagree: a `.docx` renders to a docx. This pins that invariant. + const response = await GET(request(), context) + + const disposition = response.headers.get('Content-Disposition') ?? '' + expect(disposition).toContain('report.docx') + expect(response.headers.get('Content-Type')).toBe(DOCX_MIME) + }) + + it('reports Content-Length from the rendered bytes, not the declared source size', async () => { + const rendered = Buffer.alloc(50_000) + mockFetchServableWorkspaceFileBuffer.mockResolvedValue({ + buffer: rendered, + contentType: DOCX_MIME, + }) + + const response = await GET(request(), context) + + expect(response.headers.get('Content-Length')).toBe(String(rendered.length)) + }) + + it('returns a retryable 409 while the artifact is still compiling', async () => { + mockFetchServableWorkspaceFileBuffer.mockRejectedValue( + new DocCompileUserError('Document is still being generated') + ) + + const response = await GET(request(), context) + + // A 500 would give the caller no reason to try again. + expect(response.status).toBe(409) + expect((await response.json()).error).toContain('still being generated') + }) + + it('404s a file that does not exist', async () => { + mockGetWorkspaceFile.mockResolvedValue(null) + + const response = await GET(request(), context) + + expect(response.status).toBe(404) + expect(mockFetchServableWorkspaceFileBuffer).not.toHaveBeenCalled() + }) +}) From b7f6acd15b8a607a0052063ebc1f2aa380694168 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 28 Jul 2026 10:36:39 -0700 Subject: [PATCH 3/4] fix(files): report an oversized document body as a size rejection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Capping the body read meant an oversized document threw PayloadSizeLimitError, which nothing caught, so the caller got a generic 500 — hiding the very limit message the cap was added to produce. It now returns the same 400 as the bundle check, naming the export limit. --- .../app/api/files/export/[id]/route.test.ts | 14 +++++++++++ apps/sim/app/api/files/export/[id]/route.ts | 25 ++++++++++++++----- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/apps/sim/app/api/files/export/[id]/route.test.ts b/apps/sim/app/api/files/export/[id]/route.test.ts index eac05dd1ebf..e147df871bf 100644 --- a/apps/sim/app/api/files/export/[id]/route.test.ts +++ b/apps/sim/app/api/files/export/[id]/route.test.ts @@ -4,6 +4,7 @@ import { createMockRequest } from '@sim/testing' import JSZip from 'jszip' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits' const { mockCheckAuth, @@ -136,6 +137,19 @@ describe('markdown export bundling', () => { expect(bodyCall?.[0].maxBytes).toBe(250 * MB) }) + it('reports an oversized body as a size rejection, not a server error', async () => { + mockExtractEmbeddedImageIds.mockReturnValue([]) + mockDownloadFile.mockRejectedValue( + new PayloadSizeLimitError({ label: 'storage file download', maxBytes: 1 }) + ) + + const response = await GET(request(), context) + + // The cap exists to produce a clear limit message; a 500 would hide it. + expect(response.status).toBe(400) + expect((await response.json()).error).toContain('export limit') + }) + it('caps each asset download rather than trusting its declared size', async () => { mockExtractEmbeddedImageIds.mockReturnValue(['a']) diff --git a/apps/sim/app/api/files/export/[id]/route.ts b/apps/sim/app/api/files/export/[id]/route.ts index 78f714417ea..fc01c48e055 100644 --- a/apps/sim/app/api/files/export/[id]/route.ts +++ b/apps/sim/app/api/files/export/[id]/route.ts @@ -10,6 +10,7 @@ import { parseRequest } from '@/lib/api/server' import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { extractEmbeddedImageIds } from '@/lib/copilot/tools/server/files/embedded-image-refs' import { MATERIALIZE_CONCURRENCY, mapWithConcurrency } from '@/lib/core/utils/concurrency' +import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { captureServerEvent } from '@/lib/posthog/server' import type { StorageContext } from '@/lib/uploads/config' @@ -130,12 +131,24 @@ export const GET = withRouteHandler( // Capped like everything else in the bundle: the document body is usually the // largest single entry, so leaving it unbounded left the export limit unenforced - // against the one item most able to exceed it. - const mdBuffer = await downloadFile({ - key: record.key, - context: record.context as StorageContext, - maxBytes: MAX_EXPORT_TOTAL_BYTES, - }) + // against the one item most able to exceed it. A body that alone exceeds the limit + // is a size rejection, so it reports as one rather than as a server error. + let mdBuffer: Buffer + try { + mdBuffer = await downloadFile({ + key: record.key, + context: record.context as StorageContext, + maxBytes: MAX_EXPORT_TOTAL_BYTES, + }) + } catch (error) { + if (!isPayloadSizeLimitError(error)) throw error + return NextResponse.json( + { + error: `This document exceeds the ${formatFileSize(MAX_EXPORT_TOTAL_BYTES)} export limit.`, + }, + { status: 400 } + ) + } let mdContent = mdBuffer.toString('utf-8') const imageIds = extractEmbeddedImageIds(mdContent) From 6950e22032ae22966933b98e4b4a72b63fa84de5 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 28 Jul 2026 10:55:02 -0700 Subject: [PATCH 4/4] chore(files): drop the unreachable export asset-count cap extractEmbeddedFileRefs stops collecting at MAX_EMBEDDED_IMAGES, so the list this route receives is already bounded before it arrives and the count check could never fire. Its test only passed because it mocked the extractor, so it asserted a branch production cannot reach. The byte ceilings are the real bound and stay. A comment records where the count is actually enforced, so the next reader does not add a second one. --- .../app/api/files/export/[id]/route.test.ts | 13 ----------- apps/sim/app/api/files/export/[id]/route.ts | 23 +++++-------------- 2 files changed, 6 insertions(+), 30 deletions(-) diff --git a/apps/sim/app/api/files/export/[id]/route.test.ts b/apps/sim/app/api/files/export/[id]/route.test.ts index e147df871bf..069e1fc30a9 100644 --- a/apps/sim/app/api/files/export/[id]/route.test.ts +++ b/apps/sim/app/api/files/export/[id]/route.test.ts @@ -80,19 +80,6 @@ describe('markdown export bundling', () => { mockExtractEmbeddedImageIds.mockReturnValue([]) }) - it('rejects a document embedding more assets than an export may bundle', async () => { - mockExtractEmbeddedImageIds.mockReturnValue( - Array.from({ length: 501 }, (_, index) => `img-${index}`) - ) - - const response = await GET(request(), context) - - expect(response.status).toBe(400) - expect((await response.json()).error).toContain('501') - // Rejected on the embed count alone: nothing was resolved or downloaded. - expect(mockGetFileMetadataById).toHaveBeenCalledTimes(1) - }) - it('rejects on declared asset bytes before downloading any of them', async () => { mockExtractEmbeddedImageIds.mockReturnValue(['a', 'b', 'c']) mockGetFileMetadataById.mockImplementation(async (id: string) => diff --git a/apps/sim/app/api/files/export/[id]/route.ts b/apps/sim/app/api/files/export/[id]/route.ts index fc01c48e055..0e578ada87b 100644 --- a/apps/sim/app/api/files/export/[id]/route.ts +++ b/apps/sim/app/api/files/export/[id]/route.ts @@ -24,16 +24,14 @@ import { encodeFilenameForHeader } from '@/app/api/files/utils' const logger = createLogger('FilesExportAPI') /** - * Bundling caps. The embed list comes from scanning the document body, so its length - * and the bytes behind it are whatever the author put there — without these the export - * would materialize an unbounded number of unbounded assets in one request. + * Byte ceilings for a bundled export. The bytes behind an embed list are whatever the + * author put there, so without these the export would materialize unbounded assets in + * one request. They match the bulk-download route, so the two export surfaces reject at + * the same size. * - * The byte ceilings are the real bound and match the bulk-download route, so the two - * export surfaces reject at the same size. The count is only a guard on the metadata - * lookups that precede the byte check, so it sits far above any hand-authored document - * rather than at a number a screenshot-heavy doc could plausibly reach. + * There is deliberately no count cap here: `extractEmbeddedFileRefs` already stops at + * `MAX_EMBEDDED_IMAGES`, so the list this route receives is bounded before it arrives. */ -const MAX_EXPORT_ASSETS = 500 const MAX_EXPORT_ASSET_BYTES = 25 * 1024 * 1024 const MAX_EXPORT_TOTAL_BYTES = 250 * 1024 * 1024 @@ -169,15 +167,6 @@ export const GET = withRouteHandler( }) } - if (imageIds.length > MAX_EXPORT_ASSETS) { - return NextResponse.json( - { - error: `This document embeds ${imageIds.length} files, more than the ${MAX_EXPORT_ASSETS} an export can bundle.`, - }, - { status: 400 } - ) - } - // Metadata first: declared sizes bound the download before a byte is read, and the // authorization check costs nothing to run here. const assetTargets = (