diff --git a/apps/sim/app/api/files/public/[token]/content/route.test.ts b/apps/sim/app/api/files/public/[token]/content/route.test.ts index 54d7ce0d3ad..46c666d6d10 100644 --- a/apps/sim/app/api/files/public/[token]/content/route.test.ts +++ b/apps/sim/app/api/files/public/[token]/content/route.test.ts @@ -3,6 +3,8 @@ */ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits' +import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types' const { mockResolveActiveShareByToken, @@ -78,13 +80,47 @@ describe('GET /api/files/public/[token]/content', () => { expect(mockDownloadFile).not.toHaveBeenCalled() }) - it('serves the bytes once authorized', async () => { + it('serves the bytes once authorized, bounded by the shared transfer ceiling', async () => { mockValidateDeploymentAuth.mockResolvedValueOnce({ authorized: true }) const res = await GET(request(), params()) expect(res.status).toBe(200) + // The ceiling matters most here: this is the only surface that reads a workspace + // object for a caller with no session, and the object is admitted at 5 GB. expect(mockDownloadFile).toHaveBeenCalledWith({ key: passwordShare.file.key, context: 'workspace', + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, }) }) + + it('413s when a compiled artifact outgrows the ceiling its source fit inside', async () => { + mockValidateDeploymentAuth.mockResolvedValueOnce({ authorized: true }) + // The source read is bounded, but the artifact is fetched separately — a small + // generation source can resolve to a document far larger than the source ever was. + mockDownloadFile.mockResolvedValueOnce(Buffer.from('generation source')) + mockResolveServableDoc.mockResolvedValueOnce({ + kind: 'artifact', + buffer: Buffer.alloc(MAX_BUFFERED_TRANSFER_BYTES + 1), + contentType: 'application/pdf', + }) + + const res = await GET(request(), params()) + + expect(res.status).toBe(413) + }) + + it('answers 413 rather than 500 when the shared file is too large to serve resident', async () => { + mockValidateDeploymentAuth.mockResolvedValueOnce({ authorized: true }) + mockDownloadFile.mockRejectedValueOnce( + new PayloadSizeLimitError({ + label: 'storage download', + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, + observedBytes: 5 * 1024 * 1024 * 1024, + }) + ) + + const res = await GET(request(), params()) + + expect(res.status).toBe(413) + }) }) diff --git a/apps/sim/app/api/files/public/[token]/content/route.ts b/apps/sim/app/api/files/public/[token]/content/route.ts index 6c47668fba0..f5ca997c573 100644 --- a/apps/sim/app/api/files/public/[token]/content/route.ts +++ b/apps/sim/app/api/files/public/[token]/content/route.ts @@ -7,11 +7,13 @@ import { parseRequest } from '@/lib/api/server' import { resolveServableDoc } from '@/lib/copilot/tools/server/files/doc-compile' import { validateDeploymentAuth } from '@/lib/core/security/deployment-auth' import { generateRequestId } from '@/lib/core/utils/request' +import { assertKnownSizeWithinLimit } from '@/lib/core/utils/stream-limits' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { enforcePublicFileRateLimit } from '@/lib/public-shares/rate-limit' import { resolveActiveShareByToken } from '@/lib/public-shares/share-manager' import { downloadFile } from '@/lib/uploads/core/storage-service' import { resolveServableImageBytes } from '@/lib/uploads/server/image-derivative' +import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types' import { isSimPageSource, SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/page-compile' import { renderSimPageDocumentWithAssets } from '@/lib/workspace-files/page-document.server' import { @@ -69,7 +71,14 @@ export const GET = withRouteHandler( } const { file } = resolved - const raw = await downloadFile({ key: file.key, context: 'workspace' }) + // The same ceiling the authenticated serve route reads this object under + // (`fetchWorkspaceFileBuffer`). Without it a share link is the one way to ask + // an unauthenticated caller's request to hold a 5 GB workspace file resident. + const raw = await downloadFile({ + key: file.key, + context: 'workspace', + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, + }) const servable = file.workspaceId ? await resolveServableDoc(file.workspaceId, raw, file.originalName) @@ -116,6 +125,12 @@ export const GET = withRouteHandler( if (image) ({ buffer, contentType } = image) } + // Bounding the source read does not bound the response: each branch above can + // replace it with bytes fetched or produced separately — a compiled artifact, a + // page with its images inlined, a transcoded derivative. This is an anonymous + // route, so the bytes it actually returns are what has to fit. + assertKnownSizeWithinLimit(buffer.length, MAX_BUFFERED_TRANSFER_BYTES, 'served file response') + logger.info('Public shared file served', { token, key: file.key, size: buffer.length }) // Anonymous access: null actor (owner-as-actor would misread as a self-download). diff --git a/apps/sim/app/api/files/public/[token]/inline/route.test.ts b/apps/sim/app/api/files/public/[token]/inline/route.test.ts index 5d3e7871d06..f84e1298093 100644 --- a/apps/sim/app/api/files/public/[token]/inline/route.test.ts +++ b/apps/sim/app/api/files/public/[token]/inline/route.test.ts @@ -3,6 +3,8 @@ */ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits' +import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types' const { mockResolveShare, mockRateLimit, mockValidateAuth, mockDownloadFile, mockResolveImage } = vi.hoisted(() => ({ @@ -122,4 +124,37 @@ describe('GET /api/files/public/[token]/inline', () => { expect(res.status).toBe(404) expect(mockDownloadFile).not.toHaveBeenCalled() }) + + it('bounds both reads: the doc scan tightly, the served image at the transfer ceiling', async () => { + await GET(req(`fileId=${FILE_ID}`), params) + + const [docRead, imageRead] = mockDownloadFile.mock.calls.map(([args]) => args) + // The doc is scanned and discarded (and decoded to UTF-16 on top of the buffer), + // so it must not inherit the ceiling of a file this route actually serves. + expect(docRead.key).toBe(DOC_KEY) + expect(docRead.maxBytes).toBeGreaterThan(0) + expect(docRead.maxBytes).toBeLessThan(MAX_BUFFERED_TRANSFER_BYTES) + expect(imageRead.key).toBe(IMG_KEY) + expect(imageRead.maxBytes).toBe(MAX_BUFFERED_TRANSFER_BYTES) + }) + + it('fails the referenced-by-doc gate closed when the document is too large to scan', async () => { + mockDownloadFile.mockImplementation(({ key }: { key: string }) => + key === DOC_KEY + ? Promise.reject( + new PayloadSizeLimitError({ + label: 'storage download', + maxBytes: 10 * 1024 * 1024, + observedBytes: 5 * 1024 * 1024 * 1024, + }) + ) + : Promise.resolve(PNG) + ) + + const res = await GET(req(`fileId=${FILE_ID}`), params) + + expect(res.status).toBe(404) + // The gate could not be verified, so the image must never be read at all. + expect(mockDownloadFile).toHaveBeenCalledTimes(1) + }) }) diff --git a/apps/sim/app/api/files/public/[token]/inline/route.ts b/apps/sim/app/api/files/public/[token]/inline/route.ts index 80926733a67..69c09db12b1 100644 --- a/apps/sim/app/api/files/public/[token]/inline/route.ts +++ b/apps/sim/app/api/files/public/[token]/inline/route.ts @@ -6,6 +6,7 @@ import { getPublicInlineFileContract } from '@/lib/api/contracts/public-shares' import { parseRequest } from '@/lib/api/server' import { validateDeploymentAuth } from '@/lib/core/security/deployment-auth' import { generateRequestId } from '@/lib/core/utils/request' +import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { enforcePublicFileRateLimit } from '@/lib/public-shares/rate-limit' import { resolveActiveShareByToken } from '@/lib/public-shares/share-manager' @@ -20,6 +21,17 @@ export const dynamic = 'force-dynamic' const logger = createLogger('PublicInlineFileAPI') +/** + * Ceiling on the shared document read for the referenced-by-doc gate below. + * + * Far tighter than the ceiling on a file this route SERVES, because these bytes are + * never served — they are scanned for image references and discarded, and scanning + * decodes them to UTF-16 on top of the buffer, so the resident cost is roughly double + * the read. A share can point at any workspace file, admitted at 5 GB, and this route + * is anonymous; nothing a person writes as a document approaches even this bound. + */ +const MAX_INLINE_REF_SCAN_BYTES = 10 * 1024 * 1024 + /** * GET /api/files/public/[token]/inline?key=|fileId= * @@ -72,7 +84,21 @@ export const GET = withRouteHandler( } // Referenced-by-doc gate: the share grants exactly the images the document embeds. - const docText = (await downloadFile({ key: doc.key, context: 'workspace' })).toString('utf-8') + // A document too large to scan fails the gate like any other unverifiable + // reference — the grant cannot be extended to an embed we were unable to confirm. + let docText: string + try { + const docBuffer = await downloadFile({ + key: doc.key, + context: 'workspace', + maxBytes: MAX_INLINE_REF_SCAN_BYTES, + }) + docText = docBuffer.toString('utf-8') + } catch (error) { + if (!isPayloadSizeLimitError(error)) throw error + logger.info('Shared document too large to scan for embedded references', { token }) + throw new FileNotFoundError('Not found') + } const { keys, ids } = extractEmbeddedFileRefs(docText) const referenced = ref.fileId ? ids.some((id) => storedFileId(id) === ref.fileId) diff --git a/apps/sim/app/api/files/serve-inline-image.ts b/apps/sim/app/api/files/serve-inline-image.ts index 162685e8682..f984413fd52 100644 --- a/apps/sim/app/api/files/serve-inline-image.ts +++ b/apps/sim/app/api/files/serve-inline-image.ts @@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger' import type { NextResponse } from 'next/server' import { downloadFile } from '@/lib/uploads/core/storage-service' import type { ResolvedInlineImage } from '@/lib/uploads/server/inline-image' +import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types' import { sniffImageContentType } from '@/lib/uploads/utils/validation' import { createFileResponse, FileNotFoundError } from '@/app/api/files/utils' @@ -25,7 +26,11 @@ export async function serveInlineImage( image: ResolvedInlineImage, { sniff }: { sniff: boolean } ): Promise { - const buffer = await downloadFile({ key: image.key, context: 'workspace' }) + const buffer = await downloadFile({ + key: image.key, + context: 'workspace', + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, + }) let contentType = image.contentType if (sniff) { diff --git a/apps/sim/app/api/files/serve/[...path]/route.test.ts b/apps/sim/app/api/files/serve/[...path]/route.test.ts index 27a8d39ce37..6bdae38ab55 100644 --- a/apps/sim/app/api/files/serve/[...path]/route.test.ts +++ b/apps/sim/app/api/files/serve/[...path]/route.test.ts @@ -6,6 +6,7 @@ import { hybridAuthMockFns, storageServiceMock, storageServiceMockFns } from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types' vi.mock('@sim/logger', () => ({ createLogger: vi.fn(() => serveLogger), @@ -27,6 +28,7 @@ const { mockResolveServableDocBytes, mockGetContentType, mockFindLocalFile, + mockReadLocalFileWithinLimit, mockCreateFileResponse, mockCreateErrorResponse, FileNotFoundError, @@ -52,6 +54,7 @@ const { mockResolveServableDocBytes: vi.fn(), mockGetContentType: vi.fn(), mockFindLocalFile: vi.fn(), + mockReadLocalFileWithinLimit: vi.fn(), mockCreateFileResponse: vi.fn(), mockCreateErrorResponse: vi.fn(), FileNotFoundError: FileNotFoundErrorClass, @@ -119,6 +122,7 @@ vi.mock('@/app/api/files/utils', () => ({ extractStorageKey: vi.fn().mockImplementation((path: string) => path.split('/').pop()), extractFilename: vi.fn().mockImplementation((path: string) => path.split('/').pop()), findLocalFile: mockFindLocalFile, + readLocalFileWithinLimit: mockReadLocalFileWithinLimit, })) import { GET } from '@/app/api/files/serve/[...path]/route' @@ -162,6 +166,9 @@ describe('File Serve API Route', () => { ) mockGetContentType.mockReturnValue('text/plain') mockFindLocalFile.mockReturnValue('/test/uploads/test-file.txt') + mockReadLocalFileWithinLimit.mockImplementation(async (filePath: string) => + mockReadFile(filePath) + ) mockCreateFileResponse.mockImplementation( (file: { buffer: Buffer; contentType: string; filename: string }) => { return new Response(file.buffer, { @@ -181,6 +188,82 @@ describe('File Serve API Route', () => { }) }) + it('bounds every buffered read at the shared transfer ceiling', async () => { + mockIsUsingCloudStorage.mockReturnValue(true) + mockResolveStoredFileContext.mockResolvedValue('copilot') + mockInferContextFromKey.mockReturnValue('copilot') + mockDownloadCopilotFile.mockResolvedValue(Buffer.from('bytes')) + + await GET(new NextRequest('http://localhost:3000/api/files/serve/copilot/doc.txt'), { + params: Promise.resolve({ path: ['copilot', 'doc.txt'] }), + }) + + expect(mockDownloadCopilotFile).toHaveBeenCalledWith('copilot/doc.txt', { + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, + }) + }) + + it('bounds the local read rather than trusting the stored size', async () => { + await GET(new NextRequest('http://localhost:3000/api/files/serve/workspace/ws/test-file.txt'), { + params: Promise.resolve({ path: ['workspace', 'ws', 'test-file.txt'] }), + }) + + expect(mockReadLocalFileWithinLimit).toHaveBeenCalledWith( + '/test/uploads/test-file.txt', + MAX_BUFFERED_TRANSFER_BYTES, + expect.any(String) + ) + }) + + it('413s when a resolved document outgrows the ceiling its source fit inside', async () => { + // The stored source is a small generation script; the compiled artifact it + // resolves to is fetched separately and is what the response would carry. + mockResolveServableDocBytes.mockResolvedValue({ + buffer: Buffer.alloc(MAX_BUFFERED_TRANSFER_BYTES + 1), + contentType: 'application/pdf', + }) + mockCreateErrorResponse.mockImplementation( + (error: Error) => + new Response(JSON.stringify({ error: error.name }), { + status: error.name === 'PayloadSizeLimitError' ? 413 : 500, + }) + ) + + const response = await GET( + new NextRequest('http://localhost:3000/api/files/serve/workspace/ws/report.pdf'), + { params: Promise.resolve({ path: ['workspace', 'ws', 'report.pdf'] }) } + ) + + expect(response.status).toBe(413) + }) + + it('answers 413 rather than 500 when a file is too large to serve resident', async () => { + const { PayloadSizeLimitError } = await import('@/lib/core/utils/stream-limits') + mockReadLocalFileWithinLimit.mockRejectedValue( + new PayloadSizeLimitError({ + label: 'served file', + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, + observedBytes: MAX_BUFFERED_TRANSFER_BYTES + 1, + }) + ) + // The real createErrorResponse owns the status mapping; mirror it here so the + // route's own error path is what decides, not the mock's default 500. + mockCreateErrorResponse.mockImplementation( + (error: Error) => + new Response(JSON.stringify({ error: error.name }), { + status: error.name === 'PayloadSizeLimitError' ? 413 : 500, + }) + ) + + const response = await GET( + new NextRequest('http://localhost:3000/api/files/serve/workspace/ws/huge.bin'), + { params: Promise.resolve({ path: ['workspace', 'ws', 'huge.bin'] }) } + ) + + expect(response.status).toBe(413) + expect(serveLogger.error).not.toHaveBeenCalled() + }) + it('should serve local file successfully', async () => { const req = new NextRequest( 'http://localhost:3000/api/files/serve/workspace/test-workspace-id/test-file.txt' @@ -232,6 +315,7 @@ describe('File Serve API Route', () => { expect(storageServiceMockFns.mockDownloadFile).toHaveBeenCalledWith({ key: 'workspace/test-workspace-id/1234567890-image.png', context: 'mothership', + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, }) }) @@ -318,6 +402,7 @@ describe('File Serve API Route', () => { expect(storageServiceMockFns.mockDownloadFile).toHaveBeenCalledWith({ key: 'workspace/test-workspace-id/1234567890-photo.png', context: 'mothership', + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, }) }) diff --git a/apps/sim/app/api/files/serve/[...path]/route.ts b/apps/sim/app/api/files/serve/[...path]/route.ts index 0e18cfd04ad..71fdb72a46f 100644 --- a/apps/sim/app/api/files/serve/[...path]/route.ts +++ b/apps/sim/app/api/files/serve/[...path]/route.ts @@ -1,6 +1,6 @@ -import { readFile } from 'fs/promises' import { type Principal, requirePrincipalSubjectUserId } from '@sim/auth/principal' import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' import type { NextRequest } from 'next/server' import { NextResponse } from 'next/server' import { fileServeParamsSchema, fileServeQuerySchema } from '@/lib/api/contracts/storage-transfer' @@ -12,6 +12,7 @@ import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { resolveServableDocBytes } from '@/lib/copilot/tools/server/files/doc-compile' import { DocCompileUserError } from '@/lib/copilot/tools/server/files/doc-compile-error' import { asOrchestrationError } from '@/lib/core/orchestration/types' +import { assertKnownSizeWithinLimit, isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { CopilotFiles, isUsingCloudStorage } from '@/lib/uploads' import type { StorageContext } from '@/lib/uploads/config' @@ -19,6 +20,7 @@ import { parseWorkspaceFileKey } from '@/lib/uploads/contexts/workspace/workspac import { downloadFile } from '@/lib/uploads/core/storage-service' import { resolveServableImageBytes } from '@/lib/uploads/server/image-derivative' import { resolveStoredFileContext } from '@/lib/uploads/server/metadata' +import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types' import { inferContextFromKey } from '@/lib/uploads/utils/file-utils' import { internalWorkspaceFileServeAuth } from '@/lib/workspace-files/api' import { readWorkspaceFileContentByKey } from '@/lib/workspace-files/application/read-workspace-file-content-by-key' @@ -31,6 +33,7 @@ import { FileNotFoundError, findLocalFile, getContentType, + readLocalFileWithinLimit, } from '@/app/api/files/utils' const logger = createLogger('FilesServeAPI') @@ -42,11 +45,13 @@ const logger = createLogger('FilesServeAPI') * workspace file is rewritten under a new key on every content update, so a reader * holding the previous key lands here routinely and correctly receives a 404. Each * handler rethrows into the outer one, so logging those at `error` reports the same - * expected 404 twice and buries the failures that do warrant attention. + * expected 404 twice and buries the failures that do warrant attention. A file too + * large to serve resident is the same kind of answer — a 413 the caller cannot retry + * its way out of, not something on call needs to look at. */ function logServeFailure(message: string, error: unknown): void { - if (error instanceof FileNotFoundError) { - logger.info(message, { reason: error.message }) + if (error instanceof FileNotFoundError || isPayloadSizeLimitError(error)) { + logger.info(message, { reason: getErrorMessage(error) }) return } logger.error(message, error) @@ -69,6 +74,13 @@ interface ServeOptions { * routes through here. An image derivative is the opposite — the stored bytes are * the file — so it is served only when the caller asked to preview, never when it * asked to download. + * + * Every branch that replaces the source bytes is re-checked against the transfer + * ceiling on the way out. Bounding the read alone does not bound the response: a + * page inlines its images, a generated document resolves to a compiled artifact + * fetched separately, and a derivative is transcoded here — so each can turn a + * source under the ceiling into a response over it. One check where the branches + * converge is what makes that impossible to miss when a branch is added. */ async function resolveServableBytes(params: { buffer: Buffer @@ -81,6 +93,31 @@ async function resolveServableBytes(params: { /** The stored record's content type, where the caller has the record. */ fileType?: string signal: AbortSignal | undefined +}): Promise<{ buffer: Buffer; contentType: string }> { + // `raw` is the stored source, already bounded by the read that produced it, but it + // goes through the same check so the ceiling holds for everything this returns + // rather than for every branch someone remembered to cover. + const resolved = params.options.raw + ? { buffer: params.buffer, contentType: getContentType(params.filename) } + : await resolveTransformedBytes(params) + assertKnownSizeWithinLimit( + resolved.buffer.length, + MAX_BUFFERED_TRANSFER_BYTES, + 'served file response' + ) + return resolved +} + +async function resolveTransformedBytes(params: { + buffer: Buffer + filename: string + storageKey: string + workspaceId: string | undefined + options: ServeOptions + ownerKey: string | undefined + filePrincipal?: Principal + fileType?: string + signal: AbortSignal | undefined }): Promise<{ buffer: Buffer; contentType: string }> { const { buffer, @@ -93,7 +130,6 @@ async function resolveServableBytes(params: { fileType, signal, } = params - if (options.raw) return { buffer, contentType: getContentType(filename) } // The pdf model for pages: a page file stores its SOURCE (frontmatter + // markdown + sim: fences) and serving compiles it to the rendered document, @@ -104,10 +140,11 @@ async function resolveServableBytes(params: { if (fileType === SIM_PAGE_CONTENT_TYPE || filename.toLowerCase().endsWith('.html')) { const text = buffer.toString('utf8') if (isSimPageSource(text)) { - return { - buffer: Buffer.from(await renderSimPageDocumentWithAssets(text, { workspaceId }), 'utf8'), - contentType: 'text/html', - } + const rendered = Buffer.from( + await renderSimPageDocumentWithAssets(text, { workspaceId }), + 'utf8' + ) + return { buffer: rendered, contentType: 'text/html' } } } @@ -345,7 +382,11 @@ async function handleLocalFile( throw new FileNotFoundError(`File not found: ${filename}`) } - const rawBuffer = await readFile(filePath) + const rawBuffer = await readLocalFileWithinLimit( + filePath, + MAX_BUFFERED_TRANSFER_BYTES, + 'served file' + ) const segment = filename.split('/').pop() || filename const displayName = stripStorageKeyPrefix(segment) const workspaceId = getWorkspaceIdForCompile(filename) @@ -400,11 +441,14 @@ async function handleCloudProxy( let rawBuffer: Buffer if (context === 'copilot') { - rawBuffer = await CopilotFiles.downloadCopilotFile(cloudKey) + rawBuffer = await CopilotFiles.downloadCopilotFile(cloudKey, { + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, + }) } else { rawBuffer = await downloadFile({ key: cloudKey, context, + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, }) } @@ -448,11 +492,14 @@ async function handleCloudProxyPublic( let fileBuffer: Buffer if (context === 'copilot') { - fileBuffer = await CopilotFiles.downloadCopilotFile(cloudKey) + fileBuffer = await CopilotFiles.downloadCopilotFile(cloudKey, { + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, + }) } else { fileBuffer = await downloadFile({ key: cloudKey, context, + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, }) } @@ -485,7 +532,11 @@ async function handleLocalFilePublic(filename: string): Promise { throw new FileNotFoundError(`File not found: ${filename}`) } - const fileBuffer = await readFile(filePath) + const fileBuffer = await readLocalFileWithinLimit( + filePath, + MAX_BUFFERED_TRANSFER_BYTES, + 'served file' + ) const contentType = getContentType(filename) logger.info('Public local file served', { filename, size: fileBuffer.length }) diff --git a/apps/sim/app/api/files/utils.ts b/apps/sim/app/api/files/utils.ts index 0de6ed4b3ba..c74679197b0 100644 --- a/apps/sim/app/api/files/utils.ts +++ b/apps/sim/app/api/files/utils.ts @@ -1,5 +1,9 @@ import { createLogger } from '@sim/logger' import { NextResponse } from 'next/server' +import { + isPayloadSizeLimitError, + readNodeStreamToBufferWithLimit, +} from '@/lib/core/utils/stream-limits' import { sanitizeFileKey } from '@/lib/uploads/utils/file-utils' const logger = createLogger('FilesUtils') @@ -268,7 +272,16 @@ export function createFileResponse(file: FileResponse): NextResponse { export function createErrorResponse(error: Error, status = 500): NextResponse { const statusCode = - error instanceof FileNotFoundError ? 404 : error instanceof InvalidRequestError ? 400 : status + error instanceof FileNotFoundError + ? 404 + : error instanceof InvalidRequestError + ? 400 + : // A file too large to hold resident is the caller asking for something this + // route will not do, not a server fault — 413 keeps it out of the 5xx alarms + // and tells the client retrying is pointless. + isPayloadSizeLimitError(error) + ? 413 + : status return NextResponse.json( { @@ -279,6 +292,33 @@ export function createErrorResponse(error: Error, status = 500): NextResponse { ) } +/** + * Reads a local upload into memory under a hard byte ceiling. + * + * The self-hosted mirror of the `maxBytes` every cloud provider download takes: + * a bare `readFile` inherits the 5 GB admission ceiling workspace files are stored + * under and allocates all of it inside the shared app process. + * + * The limit is enforced on the bytes as they arrive, through the same bounded-stream + * reader the S3/Blob/GCS downloads use, rather than by checking `stat` and then + * reading. A declared size only describes the file at the moment it was measured, so + * a stat-then-read pair admits whatever the file becomes in between — the cloud + * providers check `ContentLength` too, but never trust it as the only bound. + */ +export async function readLocalFileWithinLimit( + filePath: string, + maxBytes: number, + label: string +): Promise { + const { createReadStream } = await import('fs') + const stream = createReadStream(filePath) + try { + return await readNodeStreamToBufferWithLimit(stream, { maxBytes, label }) + } finally { + stream.destroy() + } +} + export function createSuccessResponse(data: ApiSuccessResponse): NextResponse { return NextResponse.json(data) } diff --git a/apps/sim/lib/copilot/tools/server/files/doc-compiled-store.test.ts b/apps/sim/lib/copilot/tools/server/files/doc-compiled-store.test.ts index 3c9a6e6d3ab..5fbaa13dbfb 100644 --- a/apps/sim/lib/copilot/tools/server/files/doc-compiled-store.test.ts +++ b/apps/sim/lib/copilot/tools/server/files/doc-compiled-store.test.ts @@ -19,6 +19,8 @@ import { loadPublishedCompiledDoc, storeCompiledDoc, } from '@/lib/copilot/tools/server/files/doc-compiled-store' +import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits' +import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types' describe('compiled document publication', () => { beforeEach(() => { @@ -67,6 +69,54 @@ describe('compiled document publication', () => { ) }) + it('bounds the artifact read so an oversized artifact is never materialized', async () => { + mockHeadObject.mockResolvedValue({ size: 1 }) + mockDownloadFile.mockResolvedValueOnce( + Buffer.from(JSON.stringify({ version: 1, referencedInputIdentity: 'dependency-identity' })) + ) + mockDownloadFile.mockResolvedValueOnce(Buffer.from('%PDF-artifact')) + + await loadPublishedCompiledDoc('workspace-1', 'source', 'pdf') + + // The artifact is fetched separately from the source that names it, so a source + // that cleared its own ceiling says nothing about how large this is. + expect(mockDownloadFile.mock.calls[1]?.[0]).toEqual( + expect.objectContaining({ maxBytes: MAX_BUFFERED_TRANSFER_BYTES }) + ) + }) + + it('surfaces an oversized artifact instead of reporting it as not yet built', async () => { + // `null` means "still compiling", which callers answer with a retry — an artifact + // that is too large would sit behind that answer forever. + mockHeadObject.mockResolvedValue({ size: 1 }) + mockDownloadFile.mockResolvedValueOnce( + Buffer.from(JSON.stringify({ version: 1, referencedInputIdentity: 'dependency-identity' })) + ) + mockDownloadFile.mockRejectedValueOnce( + new PayloadSizeLimitError({ + label: 'storage download', + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, + observedBytes: MAX_BUFFERED_TRANSFER_BYTES + 1, + }) + ) + + await expect(loadPublishedCompiledDoc('workspace-1', 'source', 'pdf')).rejects.toThrow( + PayloadSizeLimitError + ) + }) + + it('still reports a missing artifact as not yet built', async () => { + mockHeadObject.mockResolvedValue({ size: 1 }) + mockDownloadFile.mockResolvedValueOnce( + Buffer.from(JSON.stringify({ version: 1, referencedInputIdentity: 'dependency-identity' })) + ) + mockDownloadFile.mockRejectedValueOnce(new Error('NoSuchKey')) + + await expect(loadPublishedCompiledDoc('workspace-1', 'source', 'pdf')).rejects.toThrow( + 'Published compiled document artifact is missing' + ) + }) + it('fails fast on a malformed published pointer', async () => { mockHeadObject.mockResolvedValue({ size: 1 }) mockDownloadFile.mockResolvedValueOnce(Buffer.from('{not-json')) diff --git a/apps/sim/lib/copilot/tools/server/files/doc-compiled-store.ts b/apps/sim/lib/copilot/tools/server/files/doc-compiled-store.ts index dc57f549f0b..a35cb3f10b2 100644 --- a/apps/sim/lib/copilot/tools/server/files/doc-compiled-store.ts +++ b/apps/sim/lib/copilot/tools/server/files/doc-compiled-store.ts @@ -1,7 +1,9 @@ import { createHash } from 'node:crypto' import { createLogger } from '@sim/logger' import { getErrorMessage, toError } from '@sim/utils/errors' +import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import { downloadFile, headObject, uploadFile } from '@/lib/uploads/core/storage-service' +import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types' const logger = createLogger('CopilotDocCompiledStore') @@ -65,7 +67,24 @@ async function loadPublishedArtifactPointer(key: string): Promise { const key = compiledArtifactKey(workspaceId, source, ext, referencedInputIdentity) try { - return await downloadFile({ key, context: 'copilot' }) - } catch { + return await downloadFile({ key, context: 'copilot', maxBytes: MAX_BUFFERED_TRANSFER_BYTES }) + } catch (error) { + if (isPayloadSizeLimitError(error)) throw error return null } } diff --git a/apps/sim/lib/file-parsers/yaml-limits.test.ts b/apps/sim/lib/file-parsers/yaml-limits.test.ts new file mode 100644 index 00000000000..b03dc85edd0 --- /dev/null +++ b/apps/sim/lib/file-parsers/yaml-limits.test.ts @@ -0,0 +1,139 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + createYamlExpansionBudget, + isYamlExpansionBudgetExhausted, + measureYamlExpansion, + type YamlExpansionLimits, +} from '@/lib/file-parsers/yaml-limits' + +const LIMITS: YamlExpansionLimits = { + maxNodes: 1000, + maxSerializedBytes: 64 * 1024, + maxDepth: 10, +} + +const limits = (overrides: Partial = {}): YamlExpansionLimits => ({ + ...LIMITS, + ...overrides, +}) + +describe('measureYamlExpansion', () => { + it('reports the depth of the expanded tree', () => { + expect(measureYamlExpansion('scalar', LIMITS)).toEqual({ within: true, depth: 0 }) + expect(measureYamlExpansion([1, 2, 3], LIMITS)).toEqual({ within: true, depth: 1 }) + expect(measureYamlExpansion({ a: { b: { c: 1 } } }, LIMITS)).toEqual({ within: true, depth: 3 }) + }) + + it('counts an empty container as a level', () => { + expect(measureYamlExpansion([], LIMITS)).toEqual({ within: true, depth: 1 }) + expect(measureYamlExpansion({ a: {} }, LIMITS)).toEqual({ within: true, depth: 2 }) + }) + + it('charges an aliased subtree once per path that reaches it', () => { + const shared = [1, 2, 3, 4, 5] + const aliased = { a: shared, b: shared, c: shared } + + // 19 nodes when every reach is charged (root + 3 refs + 3x5 elements); 9 if the + // shared array were counted once, which is what makes an alias bomb invisible. + expect(measureYamlExpansion(aliased, limits({ maxNodes: 19 }))).toEqual({ + within: true, + depth: 2, + }) + expect(measureYamlExpansion(aliased, limits({ maxNodes: 18 })).within).toBe(false) + }) + + it('terminates on a self-referential anchor instead of recursing forever', () => { + const cyclic: Record = {} + cyclic.self = cyclic + + const measured = measureYamlExpansion(cyclic, LIMITS) + + expect(measured.within).toBe(false) + if (!measured.within) expect(measured.reason).toContain('nesting depth') + }) + + it('rejects a wide fan-out of containers without enumerating it first', () => { + // The traversal holds one frame per level, not one per pending node, so a node + // whose fan-out dwarfs the budget trips the cap part way through rather than + // after building a frame for every sibling. + const wide = Array.from({ length: 100_000 }, () => ({ a: 1 })) + + const measured = measureYamlExpansion(wide, limits({ maxNodes: 50 })) + + expect(measured.within).toBe(false) + if (!measured.within) expect(measured.reason).toContain('expanded nodes') + }) + + it('charges a long number its serialized length, not the flat allowance', () => { + // Each serializes to 24 characters; the flat non-string allowance is 16. + const longNumbers = Array.from({ length: 200 }, () => -1.2345678901234567e-308) + const shortNumbers = Array.from({ length: 200 }, () => 1) + + // Sits between what 200 short numbers cost (~4.4 KB) and what 200 long ones do + // (~6 KB); under the flat allowance both would land on the same side of it. + const byteCap = limits({ maxSerializedBytes: 5000 }) + + expect(measureYamlExpansion(shortNumbers, byteCap).within).toBe(true) + expect(measureYamlExpansion(longNumbers, byteCap).within).toBe(false) + }) + + it('charges a Date its quoted ISO length, not the flat allowance', () => { + // The default js-yaml schema turns `!!timestamp` into a Date, and JSON.stringify + // emits it as a 26-character quoted string — an aliased list of them would + // otherwise be charged 16 apiece and slip past the byte cap. + const dates = Array.from({ length: 200 }, () => new Date('2026-08-31T00:00:00.000Z')) + const booleans = Array.from({ length: 200 }, () => true) + + const byteCap = limits({ maxSerializedBytes: 5000 }) + + expect(measureYamlExpansion(booleans, byteCap).within).toBe(true) + expect(measureYamlExpansion(dates, byteCap).within).toBe(false) + }) + + it('charges object keys, so an aliased object with long keys cannot bypass the cap', () => { + const key = 'k'.repeat(500) + const shared = { [key]: 1 } + const aliased = Array.from({ length: 50 }, () => shared) + + const measured = measureYamlExpansion(aliased, limits({ maxSerializedBytes: 10_000 })) + + expect(measured.within).toBe(false) + if (!measured.within) expect(measured.reason).toContain('serialized size') + }) + + it('draws several documents down one shared budget', () => { + const budget = createYamlExpansionBudget(limits({ maxNodes: 30 })) + const doc = Array.from({ length: 10 }, (_, i) => i) + + expect(measureYamlExpansion(doc, limits({ maxNodes: 30 }), budget).within).toBe(true) + expect(isYamlExpansionBudgetExhausted(budget)).toBe(false) + expect(measureYamlExpansion(doc, limits({ maxNodes: 30 }), budget).within).toBe(true) + // The third pass runs out: 3 x 11 nodes exceeds the 30 the budget was created with. + expect(measureYamlExpansion(doc, limits({ maxNodes: 30 }), budget).within).toBe(false) + expect(isYamlExpansionBudgetExhausted(budget)).toBe(true) + }) + + it('leaves a shared budget usable after a depth rejection', () => { + // Depth costs only its own nesting, so one over-deep document must not bankrupt + // the documents that share its budget. + const budget = createYamlExpansionBudget(limits({ maxDepth: 2 })) + const deep = { a: { b: { c: { d: 1 } } } } + + expect(measureYamlExpansion(deep, limits({ maxDepth: 2 }), budget).within).toBe(false) + expect(isYamlExpansionBudgetExhausted(budget)).toBe(false) + expect(measureYamlExpansion({ ok: 1 }, limits({ maxDepth: 2 }), budget).within).toBe(true) + }) + + it('ignores inherited properties when walking an object', () => { + const parent = { inherited: 'x'.repeat(5000) } + const child = Object.create(parent) as Record + child.own = 1 + + const measured = measureYamlExpansion(child, limits({ maxSerializedBytes: 200 })) + + expect(measured).toEqual({ within: true, depth: 1 }) + }) +}) diff --git a/apps/sim/lib/file-parsers/yaml-limits.ts b/apps/sim/lib/file-parsers/yaml-limits.ts new file mode 100644 index 00000000000..a7822949640 --- /dev/null +++ b/apps/sim/lib/file-parsers/yaml-limits.ts @@ -0,0 +1,232 @@ +/** + * Bounded traversal of a parsed YAML value, shared by every consumer that walks + * one as a tree. + * + * `yaml.load` resolves aliases into shared references, so the parsed value is a + * compact DAG that costs whatever the source cost. The amplification happens + * afterwards, in whatever expands that DAG back into a tree — `JSON.stringify` + * in the file parser, the fence renderers in the page compiler. A sub-kilobyte + * source can carry millions of expanded nodes, so the expansion has to be + * measured and rejected before anything materializes it. + * + * Repeated (aliased) references are intentionally charged on every reach, which + * is what makes the amplification visible here rather than at materialization + * time. Charging on reach also terminates on self-referential anchors. + */ + +/** Ceilings for one traversal. Callers pick values matched to what they render. */ +export interface YamlExpansionLimits { + /** Expanded nodes — every value reached, aliases counted once per path. */ + maxNodes: number + /** Estimated pretty-printed JSON size of the expanded tree. */ + maxSerializedBytes: number + /** Nesting depth, which also bounds the traversal's own working set. */ + maxDepth: number +} + +/** + * Allowance remaining across every traversal that shares one unit of work — a + * page compile parses its frontmatter and each `sim:` fence separately, and it + * is their SUM that a request pays for, so they draw down one budget rather than + * each getting the full limits. + */ +export interface YamlExpansionBudget { + nodes: number + bytes: number +} + +export function createYamlExpansionBudget(limits: YamlExpansionLimits): YamlExpansionBudget { + return { nodes: limits.maxNodes, bytes: limits.maxSerializedBytes } +} + +/** True once a budget has nothing left, so callers can skip parsing entirely. */ +export function isYamlExpansionBudgetExhausted(budget: YamlExpansionBudget): boolean { + return budget.nodes <= 0 || budget.bytes <= 0 +} + +export type YamlExpansionResult = + | { within: true; depth: number } + | { within: false; reason: string } + +/** + * Exact serialized length (in UTF-16 code units — the unit V8 allocates for the + * resulting string) that `JSON.stringify` produces for a string, accounting for + * the escape expansion of quotes, backslashes, control characters, and lone + * surrogates. Computed precisely rather than with a flat multiplier so plain + * text is charged its true size (no false rejection of large legitimate + * documents) while escape-heavy strings are charged their real, larger cost + * (no cap bypass). + */ +function serializedStringLength(value: string): number { + let length = 2 // surrounding quotes + for (let i = 0; i < value.length; i++) { + const code = value.charCodeAt(i) + if (code === 0x22 /* " */ || code === 0x5c /* \ */) { + length += 2 + } else if (code < 0x20) { + // \b \t \n \f \r use two-char escapes; other control chars use \uXXXX (six) + length += + code === 0x08 || code === 0x09 || code === 0x0a || code === 0x0c || code === 0x0d ? 2 : 6 + } else if (code >= 0xd800 && code <= 0xdfff) { + // Well-formed JSON.stringify emits a valid high+low surrogate pair as-is + // (two code units) but escapes a lone surrogate to \uXXXX (six). + const next = i + 1 < value.length ? value.charCodeAt(i + 1) : 0 + if (code <= 0xdbff && next >= 0xdc00 && next <= 0xdfff) { + length += 2 + i++ + } else { + length += 6 + } + } else { + length += 1 + } + } + return length +} + +/** + * Flat allowance for a value whose serialized form is bounded by its own kind: + * `true`, `false`, `null`, and the punctuation a container contributes on its own + * line all fit well inside it. + */ +const NON_STRING_NODE_BYTES = 16 + +/** `"2026-08-31T00:00:00.000Z"` — 24 characters of ISO 8601 plus its two quotes. */ +const SERIALIZED_DATE_BYTES = 26 + +/** + * Estimate the pretty-printed (`JSON.stringify(value, null, 2)`) size a single + * value node contributes, including the indentation/newline overhead that + * dominates deeply nested alias bombs and the exact escape expansion of strings. + */ +function estimateNodeBytes(value: unknown, depth: number): number { + const indentOverhead = depth * 2 + 4 + if (typeof value === 'string') return indentOverhead + serializedStringLength(value) + // Two non-string values outgrow the flat allowance, and charging them the allowance + // would let a document of them exceed the byte cap by half again: a double serializes + // to as many as 24 characters (`-1.2345678901234567e-308`), and a `Date` — which the + // default js-yaml schema produces for a `!!timestamp`, so the file parser sees them — + // serializes to a 26-character quoted ISO string. Taking the larger of the two never + // charges less than the flat allowance did. + if (typeof value === 'number') { + return indentOverhead + Math.max(NON_STRING_NODE_BYTES, String(value).length) + } + if (value instanceof Date) { + return indentOverhead + Math.max(NON_STRING_NODE_BYTES, SERIALIZED_DATE_BYTES) + } + return indentOverhead + NON_STRING_NODE_BYTES +} + +/** + * Estimate the serialized size of an object key (`"key": `). Keys are re-emitted + * on every alias expansion of their parent object, so an aliased object with a + * long key amplifies just like an aliased value — this must be charged or the + * size cap is trivially bypassed. + */ +function estimateKeyBytes(key: string): number { + return serializedStringLength(key) + 2 // ": " +} + +function isContainer(value: unknown): value is object { + return value !== null && typeof value === 'object' +} + +/** One child of a container, with the serialized cost of naming it. */ +interface YamlChild { + keyBytes: number + value: unknown +} + +/** + * Yields a container's children one at a time. + * + * Lazily, and via `for...in` rather than `Object.entries`, because this runs on + * untrusted input: eagerly building the child list would let a single wide node + * allocate an array proportional to its fan-out *before* the first byte of it is + * charged, which is the allocation the guard exists to prevent. + */ +function* childrenOf(container: object): Generator { + if (Array.isArray(container)) { + for (const value of container) yield { keyBytes: 0, value } + return + } + for (const key in container) { + if (Object.hasOwn(container, key)) { + yield { keyBytes: estimateKeyBytes(key), value: (container as Record)[key] } + } + } +} + +/** + * Iteratively walk the parsed value, charging every reached node against + * `budget`, and return the document depth. + * + * Each node is charged as it is reached, before any of its own children are, so a + * pathologically wide fan-out (an array of millions of aliases) trips a limit part + * way through that node rather than after enumerating it. The traversal holds one + * frame per level of nesting rather than one per pending node, so its own working + * set is bounded by `maxDepth` and not by the document's width — a guard that + * allocated in proportion to the fan-out it is meant to reject would be its own + * exhaustion path. + * + * A size or node rejection leaves the budget spent, because reaching it means the + * allowance ran out mid-walk — a shared budget therefore short-circuits every + * later document instead of paying for a full walk each time. A depth rejection + * costs only its own nesting, so it does not draw the budget down further and + * later documents sharing it still get measured. + */ +export function measureYamlExpansion( + root: unknown, + limits: YamlExpansionLimits, + budget: YamlExpansionBudget = createYamlExpansionBudget(limits) +): YamlExpansionResult { + let maxDepth = 0 + + /** Draws the node down the budget and returns a rejection reason, or null when it fits. */ + const charge = (bytes: number): string | null => { + if (--budget.nodes < 0) { + return `YAML document exceeds the maximum of ${limits.maxNodes} expanded nodes (possible alias-expansion bomb)` + } + budget.bytes -= bytes + if (budget.bytes < 0) { + return `YAML document expands beyond the maximum serialized size of ${limits.maxSerializedBytes} bytes (possible alias-expansion bomb)` + } + return null + } + + const tooDeep: YamlExpansionResult = { + within: false, + reason: `YAML document exceeds the maximum nesting depth of ${limits.maxDepth}`, + } + + const rootOverflow = charge(estimateNodeBytes(root, 0)) + if (rootOverflow) return { within: false, reason: rootOverflow } + + /** One frame per level of nesting; `depth` is the depth of the children it yields. */ + const stack: Array<{ children: Generator; depth: number }> = [] + + const descend = (container: object, depth: number): boolean => { + if (depth > maxDepth) maxDepth = depth + if (depth > limits.maxDepth) return false + stack.push({ children: childrenOf(container), depth }) + return true + } + + if (isContainer(root) && !descend(root, 1)) return tooDeep + + while (stack.length > 0) { + const frame = stack[stack.length - 1] + const next = frame.children.next() + if (next.done) { + stack.pop() + continue + } + + const { keyBytes, value } = next.value + const overflow = charge(keyBytes + estimateNodeBytes(value, frame.depth)) + if (overflow) return { within: false, reason: overflow } + if (isContainer(value) && !descend(value, frame.depth + 1)) return tooDeep + } + + return { within: true, depth: maxDepth } +} diff --git a/apps/sim/lib/file-parsers/yaml-parser.ts b/apps/sim/lib/file-parsers/yaml-parser.ts index 339f5a86853..c8ed21517cd 100644 --- a/apps/sim/lib/file-parsers/yaml-parser.ts +++ b/apps/sim/lib/file-parsers/yaml-parser.ts @@ -2,33 +2,19 @@ import { getErrorMessage } from '@sim/utils/errors' import * as yaml from 'js-yaml' import { FileParserError } from '@/lib/file-parsers/errors' import type { FileParseResult } from '@/lib/file-parsers/types' +import { measureYamlExpansion, type YamlExpansionLimits } from '@/lib/file-parsers/yaml-limits' /** - * Hard cap on the number of expanded nodes visited while validating a parsed - * YAML document. `yaml.load` resolves aliases into shared references, so the - * in-memory value is a compact DAG, but `JSON.stringify` expands that DAG into - * a full tree — duplicating every shared node. A tiny "billion laughs" alias - * bomb therefore expands to millions/billions of nodes at serialize time. This - * cap (and the byte cap below) bound the traversal so the amplification is - * detected and rejected before it ever reaches `JSON.stringify`. It also stops - * traversal of self-referential (cyclic) YAML anchors. + * What a parsed YAML file may expand to once `JSON.stringify` walks its alias + * DAG as a tree. The node cap also stops traversal of self-referential anchors; + * the byte cap bounds output a sub-1 KB input can inflate to hundreds of MB; + * the depth cap bounds the traversal's own working set. */ -const MAX_YAML_EXPANDED_NODES = 5_000_000 - -/** - * Cap on the estimated serialized (pretty-printed JSON) size of the document. - * Alias expansion inflates output far beyond the input size — a sub-1 KB input - * can serialize to hundreds of MB — so we estimate output bytes during the - * bounded traversal and abort past this limit rather than allocating them. - */ -const MAX_YAML_SERIALIZED_BYTES = 64 * 1024 * 1024 - -/** - * Cap on nesting depth. Guards the depth computation (previously an unbounded - * recursion that also spread large arrays into `Math.max(...array)`, risking a - * stack overflow) and rejects pathologically deep documents. - */ -const MAX_YAML_DEPTH = 500 +const FILE_PARSER_YAML_LIMITS: YamlExpansionLimits = { + maxNodes: 5_000_000, + maxSerializedBytes: 64 * 1024 * 1024, + maxDepth: 500, +} /** * Raised when a parsed YAML document exceeds the complexity limits above. @@ -51,128 +37,15 @@ export function isYamlComplexityError(error: unknown): error is YamlComplexityEr } /** - * Exact serialized length (in UTF-16 code units — the unit V8 allocates for the - * resulting string) that `JSON.stringify` produces for a string, accounting for - * the escape expansion of quotes, backslashes, control characters, and lone - * surrogates. Computed precisely rather than with a flat multiplier so plain - * text is charged its true size (no false rejection of large legitimate - * documents) while escape-heavy strings are charged their real, larger cost - * (no cap bypass). - */ -function serializedStringLength(value: string): number { - let length = 2 // surrounding quotes - for (let i = 0; i < value.length; i++) { - const code = value.charCodeAt(i) - if (code === 0x22 /* " */ || code === 0x5c /* \ */) { - length += 2 - } else if (code < 0x20) { - // \b \t \n \f \r use two-char escapes; other control chars use \uXXXX (six) - length += - code === 0x08 || code === 0x09 || code === 0x0a || code === 0x0c || code === 0x0d ? 2 : 6 - } else if (code >= 0xd800 && code <= 0xdfff) { - // Well-formed JSON.stringify emits a valid high+low surrogate pair as-is - // (two code units) but escapes a lone surrogate to \uXXXX (six). - const next = i + 1 < value.length ? value.charCodeAt(i + 1) : 0 - if (code <= 0xdbff && next >= 0xdc00 && next <= 0xdfff) { - length += 2 - i++ - } else { - length += 6 - } - } else { - length += 1 - } - } - return length -} - -/** - * Estimate the pretty-printed (`JSON.stringify(value, null, 2)`) size a single - * value node contributes, including the indentation/newline overhead that - * dominates deeply nested alias bombs and the exact escape expansion of strings. - */ -function estimateNodeBytes(value: unknown, depth: number): number { - const indentOverhead = depth * 2 + 4 - if (typeof value === 'string') return indentOverhead + serializedStringLength(value) - return indentOverhead + 16 -} - -/** - * Estimate the serialized size of an object key (`"key": `). Keys are re-emitted - * on every alias expansion of their parent object, so an aliased object with a - * long key amplifies just like an aliased value — this must be charged or the - * size cap is trivially bypassed. - */ -function estimateKeyBytes(key: string): number { - return serializedStringLength(key) + 2 // ": " -} - -/** - * Iteratively walk the parsed YAML value with strict node-count, output-size, - * and depth limits, returning the document depth. Repeated (aliased) references - * are intentionally counted each time they are reached, mirroring the way - * `JSON.stringify` expands them — this is what makes the alias-expansion bomb - * detectable before serialization. - * - * Each node is charged against the caps as it is *enqueued*, before its own - * children are pushed, and only container nodes are pushed onto the traversal - * stack. A pathologically wide fan-out (e.g. an array of millions of aliases) - * therefore trips a cap during the enqueue loop instead of first materializing - * millions of stack entries and exhausting memory inside the guard itself. + * Validate that a parsed YAML value stays within the file parser's expansion + * limits, returning the document depth. * * @throws {YamlComplexityError} when any limit is exceeded */ export function assertYamlWithinLimits(root: unknown): number { - let visited = 0 - let estimatedBytes = 0 - let maxDepth = 0 - - const charge = (bytes: number): void => { - if (++visited > MAX_YAML_EXPANDED_NODES) { - throw new YamlComplexityError( - `YAML document exceeds the maximum of ${MAX_YAML_EXPANDED_NODES} expanded nodes (possible alias-expansion bomb)` - ) - } - estimatedBytes += bytes - if (estimatedBytes > MAX_YAML_SERIALIZED_BYTES) { - throw new YamlComplexityError( - `YAML document expands beyond the maximum serialized size of ${MAX_YAML_SERIALIZED_BYTES} bytes (possible alias-expansion bomb)` - ) - } - } - - const isContainer = (value: unknown): value is object => - value !== null && typeof value === 'object' - - charge(estimateNodeBytes(root, 0)) - const stack: Array<{ value: object; depth: number }> = [] - if (isContainer(root)) stack.push({ value: root, depth: 0 }) - - while (stack.length > 0) { - const { value, depth } = stack.pop()! - const childDepth = depth + 1 - - if (childDepth > maxDepth) maxDepth = childDepth - if (childDepth > MAX_YAML_DEPTH) { - throw new YamlComplexityError( - `YAML document exceeds the maximum nesting depth of ${MAX_YAML_DEPTH}` - ) - } - - if (Array.isArray(value)) { - for (const child of value) { - charge(estimateNodeBytes(child, childDepth)) - if (isContainer(child)) stack.push({ value: child, depth: childDepth }) - } - } else { - for (const [key, child] of Object.entries(value as Record)) { - charge(estimateKeyBytes(key) + estimateNodeBytes(child, childDepth)) - if (isContainer(child)) stack.push({ value: child, depth: childDepth }) - } - } - } - - return maxDepth + const measured = measureYamlExpansion(root, FILE_PARSER_YAML_LIMITS) + if (!measured.within) throw new YamlComplexityError(measured.reason) + return measured.depth } /** diff --git a/apps/sim/lib/uploads/contexts/copilot/copilot-file-manager.ts b/apps/sim/lib/uploads/contexts/copilot/copilot-file-manager.ts index ceb7eacc49b..9f182f09eb3 100644 --- a/apps/sim/lib/uploads/contexts/copilot/copilot-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/copilot/copilot-file-manager.ts @@ -91,15 +91,24 @@ export async function uploadCopilotFile(options: { * Uses the unified storage service with explicit copilot context. * Handles S3, Azure Blob, and local storage automatically. * + * `maxBytes` is required for the same reason it is on `fetchWorkspaceFileBuffer`: + * the stored object is admitted far above what one request may hold resident, so a + * caller that omits a ceiling inherits "unbounded" inside the shared app process. + * * @param key File storage key + * @param options.maxBytes Hard ceiling; throws `PayloadSizeLimitError` when exceeded * @returns File buffer * @throws Error if file not found or download fails */ -export async function downloadCopilotFile(key: string): Promise { +export async function downloadCopilotFile( + key: string, + options: { maxBytes: number } +): Promise { try { const fileBuffer = await downloadFile({ key, context: 'copilot', + maxBytes: options.maxBytes, }) logger.info(`Successfully downloaded copilot file: ${key}`, { diff --git a/apps/sim/lib/uploads/core/storage-service.local-download.test.ts b/apps/sim/lib/uploads/core/storage-service.local-download.test.ts new file mode 100644 index 00000000000..c9f46f3869d --- /dev/null +++ b/apps/sim/lib/uploads/core/storage-service.local-download.test.ts @@ -0,0 +1,85 @@ +/** + * @vitest-environment node + */ +import { Readable } from 'node:stream' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockCreateReadStream, mockReadFile, mockStat } = vi.hoisted(() => ({ + mockCreateReadStream: vi.fn(), + mockReadFile: vi.fn(), + mockStat: vi.fn(), +})) + +vi.mock('fs', () => ({ createReadStream: mockCreateReadStream })) +vi.mock('fs/promises', () => ({ readFile: mockReadFile, stat: mockStat })) + +vi.mock('@/lib/uploads/config', () => ({ + USE_S3_STORAGE: false, + USE_BLOB_STORAGE: false, + USE_GCS_STORAGE: false, + getStorageConfig: () => ({ bucket: 'b', region: 'r' }), +})) + +vi.mock('@/lib/uploads/core/setup.server', () => ({ UPLOAD_DIR_SERVER: '/uploads' })) + +vi.mock('@/lib/uploads/server/metadata', () => ({ insertFileMetadata: vi.fn() })) + +import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' +import { downloadFile } from '@/lib/uploads/core/storage-service' + +/** A stream that delivers `bytes`, whatever a prior `stat` would have claimed. */ +function streamOf(bytes: number) { + const stream = Readable.from([Buffer.alloc(bytes)]) as Readable & { destroy: () => void } + vi.spyOn(stream, 'destroy') + return stream +} + +describe('downloadFile on local storage', () => { + beforeEach(() => { + vi.clearAllMocks() + mockReadFile.mockResolvedValue(Buffer.alloc(10)) + }) + + it('reads without a ceiling when the caller asks for none', async () => { + const buffer = await downloadFile({ key: 'workspace/ws/file.bin', context: 'workspace' }) + + expect(buffer.length).toBe(10) + expect(mockReadFile).toHaveBeenCalled() + expect(mockCreateReadStream).not.toHaveBeenCalled() + }) + + it('enforces the ceiling on the bytes as they arrive, not on a prior stat', async () => { + // The file grew (or was replaced) after any size a caller could have measured: + // the stream delivers more than the ceiling allows, and a stat-then-read + // implementation would have admitted it. + mockCreateReadStream.mockReturnValue(streamOf(500)) + + await expect( + downloadFile({ key: 'workspace/ws/file.bin', context: 'workspace', maxBytes: 100 }) + ).rejects.toSatisfy(isPayloadSizeLimitError) + + expect(mockStat).not.toHaveBeenCalled() + expect(mockReadFile).not.toHaveBeenCalled() + }) + + it('returns the bytes when they fit the ceiling', async () => { + mockCreateReadStream.mockReturnValue(streamOf(50)) + + const buffer = await downloadFile({ + key: 'workspace/ws/file.bin', + context: 'workspace', + maxBytes: 100, + }) + + expect(buffer.length).toBe(50) + }) + + it('destroys the stream once the read settles', async () => { + const stream = streamOf(50) + mockCreateReadStream.mockReturnValue(stream) + + await downloadFile({ key: 'workspace/ws/file.bin', context: 'workspace', maxBytes: 100 }) + + expect(stream.destroy).toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/uploads/core/storage-service.ts b/apps/sim/lib/uploads/core/storage-service.ts index 6a035a0ec4a..f1409a620ad 100644 --- a/apps/sim/lib/uploads/core/storage-service.ts +++ b/apps/sim/lib/uploads/core/storage-service.ts @@ -1,7 +1,7 @@ import type { Readable } from 'node:stream' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' -import { assertKnownSizeWithinLimit } from '@/lib/core/utils/stream-limits' +import { readNodeStreamToBufferWithLimit } from '@/lib/core/utils/stream-limits' import { getStorageConfig, USE_BLOB_STORAGE, @@ -507,7 +507,7 @@ export async function downloadFile(options: DownloadFileOptions): Promise { try { - return await downloadFile({ key: derivativeKey(storageKey), context: 'copilot' }) + return await downloadFile({ + key: derivativeKey(storageKey), + context: 'copilot', + maxBytes: MAX_BUFFERED_TRANSFER_BYTES, + }) } catch { return null } diff --git a/apps/sim/lib/workspace-files/page-compile-limits.test.ts b/apps/sim/lib/workspace-files/page-compile-limits.test.ts new file mode 100644 index 00000000000..7afcf241adc --- /dev/null +++ b/apps/sim/lib/workspace-files/page-compile-limits.test.ts @@ -0,0 +1,135 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + collectSimPageDiagnostics, + compileSimPage, + isSimPageSource, +} from '@/lib/workspace-files/page-compile' + +/** + * A `sim:table` payload whose cell count is the PRODUCT of two alias lists while + * its source length is their SUM: `columns` is anchored once and every row is an + * alias to it, so `n` rows over `n` columns cost ~13n source bytes and render + * n² cells. This is the shape that makes the fence renderers amplify — a deeply + * nested alias chain does not, because every payload schema is at most + * `array of array of scalar` and rejects depth 3 without descending. + */ +function aliasedTable(n: number): string { + const columns = Array.from({ length: n }, () => ' - x').join('\n') + const rows = Array.from({ length: n }, () => ' - *c').join('\n') + return `columns: &c\n${columns}\nrows:\n${rows}\n` +} + +/** A `sim:steps` payload that aliases one large markdown body `n` times. */ +function aliasedSteps(n: number, markdownBytes: number): string { + const markdown = 'lorem ipsum dolor sit amet '.repeat(Math.ceil(markdownBytes / 27)) + const repeats = Array.from({ length: n - 1 }, () => '- *s').join('\n') + return `- &s\n title: T\n markdown: "${markdown.slice(0, markdownBytes)}"\n${repeats}\n` +} + +function page(kind: string, payload: string): string { + return `---\ntitle: T\n---\n\`\`\`sim:${kind}\n${payload}\`\`\`\n` +} + +describe('page compile YAML expansion limits', () => { + it('renders a table whose expanded size is within the budget', () => { + const html = compileSimPage(page('table', aliasedTable(40))) + expect(html).toContain('') + expect(collectSimPageDiagnostics(page('table', aliasedTable(40)))).toEqual([]) + }) + + it('skips a table whose aliases expand past the budget', () => { + const source = page('table', aliasedTable(400)) + const html = compileSimPage(source) + + expect(html).not.toContain('
') + expect(collectSimPageDiagnostics(source)).toEqual([ + expect.stringContaining('sim:table block starting "columns: &c" skipped:'), + ]) + expect(collectSimPageDiagnostics(source)[0]).toContain('too large to render') + }) + + it('bounds the compile cost of an alias bomb that would otherwise be quadratic', () => { + // 2000 x 2000 renders 4M cells through marked.parseInline — seconds of CPU + // and tens of MB of HTML per request, from 25 KB of source. + const source = page('table', aliasedTable(2000)) + + const started = performance.now() + const html = compileSimPage(source) + const elapsed = performance.now() - started + + expect(html).not.toContain('') + expect(html).toContain('
    ') + expect(collectSimPageDiagnostics(source)).toEqual([ + 'sim:table block starting "columns: nope" skipped: its payload did not match the expected shape', + ]) + }) + + it('still reports a malformed payload as a syntax error, not a size error', () => { + const source = '---\ntitle: T\n---\n```sim:table\ncolumns: [a\n```' + expect(collectSimPageDiagnostics(source)).toEqual([ + expect.stringContaining('its payload is not valid YAML/JSON —'), + ]) + }) +}) diff --git a/apps/sim/lib/workspace-files/page-compile.ts b/apps/sim/lib/workspace-files/page-compile.ts index 0bf693acef1..7cc398c06e6 100644 --- a/apps/sim/lib/workspace-files/page-compile.ts +++ b/apps/sim/lib/workspace-files/page-compile.ts @@ -3,6 +3,13 @@ import { truncate } from '@sim/utils/string' import { JSON_SCHEMA, load } from 'js-yaml' import { marked } from 'marked' import { z } from 'zod' +import { + createYamlExpansionBudget, + isYamlExpansionBudgetExhausted, + measureYamlExpansion, + type YamlExpansionBudget, + type YamlExpansionLimits, +} from '@/lib/file-parsers/yaml-limits' /** * Compiler for agent-authored `.html` pages. @@ -246,8 +253,53 @@ export function resolveSimResourceLinks(html: string, workspaceId: string, baseU ) } -function loadYaml(body: string): unknown { - return load(body, { schema: JSON_SCHEMA }) +/** + * What ONE compile may expand its YAML to, counted across the frontmatter and + * every `sim:` fence together — a request pays for their sum, so they share one + * budget rather than each getting the full allowance. + * + * The ceiling has to sit on the EXPANDED value, not on the source: `load` + * resolves aliases into shared references, so a payload of a few kilobytes + * (`columns: &c [...]` plus a row list of aliases to it) parses into a small DAG + * that the renderers then walk as a tree, one `marked.parseInline` per cell. + * Cells grow with the product of the two alias lists while the source grows with + * their sum, so bounding source length bounds nothing. These values clear a + * 100x100 table — far past any page a person writes — and cap a compile's + * rendering work at roughly a tenth of a second. + */ +const PAGE_YAML_LIMITS: YamlExpansionLimits = { + maxNodes: 50_000, + maxSerializedBytes: 2 * 1024 * 1024, + maxDepth: 64, +} + +type PageYamlResult = { ok: true; value: unknown } | { ok: false; reason: string } + +/** + * Parses one YAML region of a page — the frontmatter or a `sim:` fence payload — + * and charges its expanded size to the compile's budget. On failure `reason` is + * the clause a block-skipped diagnostic appends; the frontmatter callers only + * read `ok`. + */ +function loadYaml(body: string, budget: YamlExpansionBudget): PageYamlResult { + if (isYamlExpansionBudgetExhausted(budget)) { + return { + ok: false, + reason: 'the page spent its whole structured-block budget on earlier blocks', + } + } + let value: unknown + try { + value = load(body, { schema: JSON_SCHEMA }) + } catch (err) { + const message = truncate(getErrorMessage(err, 'invalid YAML'), 160) + return { ok: false, reason: `its payload is not valid YAML/JSON — ${message}` } + } + const measured = measureYamlExpansion(value, PAGE_YAML_LIMITS, budget) + if (!measured.within) { + return { ok: false, reason: `its payload is too large to render — ${measured.reason}` } + } + return { ok: true, value } } type FenceRenderer = (payload: unknown) => string | null @@ -360,11 +412,8 @@ export function isSimPageSource(content: string): boolean { if (!trimmed.startsWith('---\n')) return false const end = trimmed.indexOf('\n---', 3) if (end === -1) return false - try { - return frontmatterSchema.safeParse(loadYaml(trimmed.slice(4, end)) ?? {}).success - } catch { - return false - } + const parsed = loadYaml(trimmed.slice(4, end), createYamlExpansionBudget(PAGE_YAML_LIMITS)) + return parsed.ok && frontmatterSchema.safeParse(parsed.value ?? {}).success } /** @@ -373,7 +422,7 @@ export function isSimPageSource(content: string): boolean { * page helps nobody); the skip is reported through `diagnostics` instead, * which the file-editing tool surfaces back to the authoring agent. */ -function compileBody(source: string, diagnostics?: string[]): string { +function compileBody(source: string, budget: YamlExpansionBudget, diagnostics?: string[]): string { const lines = source.split('\n') const html: string[] = [] let prose: string[] = [] @@ -407,16 +456,12 @@ function compileBody(source: string, diagnostics?: string[]): string { } } else { const renderer = FENCE_RENDERERS[kind] - let payload: unknown let rendered: string | null = null - let parseError: string | null = null + let loadError: string | null = null if (renderer) { - try { - payload = loadYaml(body) - } catch (err) { - parseError = getErrorMessage(err, 'invalid YAML') - } - if (parseError === null) rendered = renderer(payload) + const parsed = loadYaml(body, budget) + if (parsed.ok) rendered = renderer(parsed.value) + else loadError = parsed.reason } if (rendered !== null) { html.push(rendered) @@ -425,9 +470,7 @@ function compileBody(source: string, diagnostics?: string[]): string { // of the same kind, and a bare "a table is malformed" sends the // fixing agent hunting through all of them. const preview = truncate(body.trim().split('\n')[0] ?? '', 80) - const reason = parseError - ? `its payload is not valid YAML/JSON — ${truncate(parseError, 160)}` - : 'its payload did not match the expected shape' + const reason = loadError ?? 'its payload did not match the expected shape' diagnostics?.push(`sim:${kind} block starting "${preview}" skipped: ${reason}`) } } @@ -486,12 +529,17 @@ function compileSimPageDocument(source: string, diagnostics?: string[]): string const end = trimmed.indexOf('\n---', 3) const frontmatterText = trimmed.slice(4, end) const rest = trimmed.slice(end + 4).replace(/^-*\n?/, '') + // One budget for the whole document: the frontmatter and every fence draw + // from it, so a page cannot buy more rendering by splitting across blocks. + const budget = createYamlExpansionBudget(PAGE_YAML_LIMITS) + const frontmatter = loadYaml(frontmatterText, budget) + // isSimPageSource gates on parseable frontmatter; this is a safety net. + if (!frontmatter.ok) return compileBody(source, budget, diagnostics) let meta: z.infer try { - meta = frontmatterSchema.parse(loadYaml(frontmatterText) ?? {}) + meta = frontmatterSchema.parse(frontmatter.value ?? {}) } catch { - // isSimPageSource gates on parseable frontmatter; this is a safety net. - return compileBody(source, diagnostics) + return compileBody(source, budget, diagnostics) } // Two or more top-level `# ` headings turn the body into IN-DOCUMENT tabs: @@ -517,14 +565,14 @@ function compileSimPageDocument(source: string, diagnostics?: string[]): string ...(meta.lede ? [`

    ${escapeHtml(meta.lede)}

    `] : []), ...(multiTab ? [ - ...(intro.trim() ? [compileBody(intro, diagnostics)] : []), + ...(intro.trim() ? [compileBody(intro, budget, diagnostics)] : []), ...docTabs.map( (tab, i) => - `
    ${compileBody(tab.body, diagnostics)}
    ` + `
    ${compileBody(tab.body, budget, diagnostics)}
    ` ), DOC_TABS_SCRIPT, ] - : [compileBody(rest, diagnostics)]), + : [compileBody(rest, budget, diagnostics)]), '', '', '',
') + expect(html.length).toBeLessThan(64 * 1024) + expect(elapsed).toBeLessThan(1000) + }) + + it('charges aliased strings by their expanded bytes, not their node count', () => { + // Only ~2000 nodes, but 2000 x 4 KB of markdown reaches the renderer. + const source = page('steps', aliasedSteps(2000, 4096)) + const html = compileSimPage(source) + + expect(html).not.toContain('
    ') + expect(collectSimPageDiagnostics(source)[0]).toContain('maximum serialized size') + }) + + it('shares one budget across every block, so splitting buys no extra rendering', () => { + const oneBlock = page('table', aliasedTable(150)) + expect(collectSimPageDiagnostics(oneBlock)).toEqual([]) + + const manyBlocks = `---\ntitle: T\n---\n${Array.from( + { length: 6 }, + () => `\`\`\`sim:table\n${aliasedTable(150)}\`\`\`\n` + ).join('\n')}` + const diagnostics = collectSimPageDiagnostics(manyBlocks) + + expect(diagnostics.length).toBeGreaterThan(0) + expect(diagnostics.length).toBeLessThan(6) + expect(diagnostics.at(-1)).toContain('spent its whole structured-block budget') + }) + + it('refuses to recognize page source whose frontmatter expands past the budget', () => { + const nav = Array.from({ length: 400 }, () => ' - *g').join('\n') + const pages = Array.from({ length: 400 }, () => ' - "[A](sim:file/a)"').join('\n') + const source = `---\ntitle: T\nnav:\n - &g\n pages:\n${pages}\n${nav}\n---\nBody.\n` + + expect(isSimPageSource(source)).toBe(false) + }) + + it('leaves an ordinary page and its diagnostics untouched', () => { + const source = [ + '---', + 'title: Report', + '---', + 'Intro prose.', + '```sim:table', + 'columns: [Name, Count:num]', + 'rows:', + ' - [alpha, 1]', + ' - [beta, 2]', + '```', + '```sim:kv', + '- key: Owner', + ' value: Ops', + '```', + '```sim:table', + 'columns: nope', + '```', + ].join('\n') + + const html = compileSimPage(source) + expect(html).toContain('
alpha