fix(files): bound YAML expansion and buffered reads on the file-serve path (#7319)

* fix(files): bound YAML expansion and buffered reads on the file-serve path

Two unbounded resource paths reachable from the anonymous public share routes.

YAML alias expansion: the page compiler parsed sim: fence payloads with no
ceiling on what the parsed value expands to. Aliases are shared references, so
`columns: &c [...]` plus a row list of aliases to it renders N^2 cells from ~13N
source bytes — 25 KB of source cost 3.6s of CPU and 38 MB of HTML per request.
The expansion guard already in the file parser is now a shared primitive taking
caller-supplied limits, and the compiler charges every fence and the frontmatter
to one per-compile budget so splitting across blocks buys no extra rendering.

Buffered reads: the Files-module serve path already capped reads at
MAX_BUFFERED_TRANSFER_BYTES via fetchWorkspaceFileBuffer, but five sibling paths
serving the same objects did not, including both unauthenticated share routes.
A workspace object is admitted at 5 GB, so a share link was the one way to make
an anonymous request hold gigabytes resident in the shared process.

* fix(files): check transformed bytes against the ceiling and bound the guard's own walk

Review round 1.

Bounding the source read did not bound the response: every branch that replaces
the source — a compiled document artifact fetched separately, a page with its
images inlined, a transcoded image derivative — could turn a source under the
ceiling into a response over it, on the anonymous share route included. The check
now sits where those branches converge rather than on the page branch alone.

The local read enforced its ceiling on a size measured before the read, so it
described the file only as of the stat. It now reads through the same bounded
stream reader the S3/Blob/GCS downloads use.

The expansion guard held one stack frame per pending node, so proving a wide
document too large allocated in proportion to the width it was rejecting. It now
holds one frame per level of nesting and consumes each container through a lazy
generator, bounding its own working set by depth. A double serializes to up to 24
characters, so numbers are charged their serialized length rather than the flat
16-byte allowance they could outgrow.

* fix(files): route the raw serve branch through the same response ceiling

The `raw=1` branch returned before the check, so the ceiling held for the
branches that transform bytes but not for the one that returns them unchanged.
Those bytes are already bounded by the read that produced them, so this changes
no behavior — it makes the guarantee hold for everything the resolver returns
rather than for every branch someone remembered to cover.

* fix(files): bound the artifact and local reads at the read, not at the response

Review round 2.

The response-level ceiling rejected an oversized compiled artifact only after the
whole thing was resident, which is the allocation it exists to prevent. The
artifact read is bounded at its own funnel instead, and a size breach is rethrown
rather than folded into the "not built yet" null — that answer tells callers to
retry, which an oversized artifact would never stop doing. The cached image
derivative is bounded the same way, but keeps swallowing the breach, because a
miss there re-transcodes from an already-bounded source.

The local storage branch enforced its ceiling with stat-then-read, so it described
the file only as of the stat. It now reads through the same bounded-stream reader
the cloud branches use — this is the anonymous share path on a self-hosted
deployment, and the route helper was already fixed while the storage service it
sits next to was not.

The default js-yaml schema turns a timestamp into a Date, which serializes to a
26-character quoted string; charging it the 16-byte flat allowance let a document
of them exceed the byte cap.

* fix(files): bound the shared artifact reads at the widest consumer ceiling

Review round 3.

The previous round bounded the compiled-artifact and cached-derivative reads at
the rendered-document ceiling, which is half what the serving routes will return.
That made the bound a policy rather than a backstop: an artifact between the two
figures was refused even though the route accepts a response that size, and a
derivative in that band read as a cache miss on every preview and re-transcoded
the original each time.

Both funnels now bound at the widest ceiling any of their consumers allows. A
consumer that permits less still enforces its own limit on what it got back — the
workspace download path continues to hold artifacts to the rendered-document
ceiling.

Author: icecrasher321

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)
+  })
 })

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).

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)
+  })
 })

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=<cloudKey>|fileId=<id>
  *
@@ -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)

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<NextResponse> {
-  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) {

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,
     })
   })