fix(desktop): admit browser download saves before reading the body

PUT /api/desktop/tool/file buffered up to 100 MiB of the request body
before the use case checked that the tool call exists, is a running
browser_save_download claimed by the desktop, and belongs to the caller's
run, so any signed-in user could force large reads with bogus toolCallIds.

Add admitBrowserDownloadSave (same operation and binding resolution as
saveBrowserDownload, no claim or audit), following the existing
admitCreateWorkspaceFile pattern, and run it before the body read. The
save use case still re-validates and claims atomically after the read,
so a failed or oversized upload never burns the single-use claim.

Author: waleedlatif1

apps/sim/app/api/desktop/tool/file/route.test.ts

@@ -6,9 +6,14 @@ import { NextRequest } from 'next/server'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
 
-const { mockRead, mockSave } = vi.hoisted(() => ({ mockRead: vi.fn(), mockSave: vi.fn() }))
+const { mockAdmit, mockRead, mockSave } = vi.hoisted(() => ({
+  mockAdmit: vi.fn(),
+  mockRead: vi.fn(),
+  mockSave: vi.fn(),
+}))
 
 vi.mock('@/lib/browser-agent/application/browser-file-transfer', () => ({
+  admitBrowserDownloadSave: mockAdmit,
   readBrowserUploadFile: {
     operation: { id: 'files.read_content', minimumRole: 'read', workspaceApiKey: 'allow' },
     execute: mockRead,
@@ -43,6 +48,7 @@ describe('/api/desktop/tool/file', () => {
   beforeEach(() => {
     vi.clearAllMocks()
     mockGetSession.mockResolvedValue(session)
+    mockAdmit.mockResolvedValue(undefined)
     mockRead.mockResolvedValue({ file: { name: 'Q3 plan.pdf' }, content: Buffer.from('%PDF') })
     mockSave.mockResolvedValue({
       file: { name: 'report.csv', size: 3, folderPath: null, vfsNamespace: 'files' },
@@ -92,6 +98,21 @@ describe('/api/desktop/tool/file', () => {
       put('toolCallId=c&name=a.csv', new Uint8Array(1), { 'content-length': String(2 ** 31) })
     )
     expect(oversized.status).toBe(413)
+    expect(mockAdmit).not.toHaveBeenCalled()
+    expect(mockSave).not.toHaveBeenCalled()
+  })
+
+  it('refuses an unadmitted save without reading its body', async () => {
+    mockAdmit.mockRejectedValueOnce(
+      new OrchestrationError('not_found', 'Browser file transfer not found')
+    )
+    const request = put('toolCallId=unknown&name=a.csv', new Uint8Array([1, 2, 3]))
+
+    const res = await PUT(request)
+
+    expect(res.status).toBe(404)
+    expect(mockAdmit).toHaveBeenCalledWith(principal, { toolCallId: 'unknown', name: 'a.csv' })
+    expect(request.bodyUsed).toBe(false)
     expect(mockSave).not.toHaveBeenCalled()
   })
 

apps/sim/app/api/desktop/tool/file/route.ts

@@ -13,6 +13,7 @@ import {
 } from '@/lib/api/server/routes'
 import { withRequestId } from '@/lib/api/server/routes/request-id'
 import {
+  admitBrowserDownloadSave,
   readBrowserUploadFile,
   saveBrowserDownload,
 } from '@/lib/browser-agent/application/browser-file-transfer'
@@ -52,8 +53,9 @@ export const POST = defineInternalBinaryRoute({
  * PUT /api/desktop/tool/file?toolCallId=…&name=…
  *
  * Raw `withRouteHandler`: the body is the download's bytes, so it is admitted by declared length
- * and read under a byte ceiling only after the session is authenticated, then handed to the
- * application use case that binds it to its claimed `browser_save_download` call.
+ * and read under a byte ceiling only after the session is authenticated and the application has
+ * admitted the caller's claimed `browser_save_download` call, then handed to the use case that
+ * re-validates and claims that call.
  */
 export const PUT = withRouteHandler(async (request: NextRequest) => {
   let principal
@@ -73,6 +75,13 @@ export const PUT = withRouteHandler(async (request: NextRequest) => {
   }
   const parsed = await parseRequest(saveBrowserDownloadContract, request, {})
   if (!parsed.success) return parsed.response
+  const { toolCallId, name } = parsed.data.query
+
+  try {
+    await admitBrowserDownloadSave(principal, { toolCallId, name })
+  } catch (error) {
+    return saveErrorResponse(error)
+  }
 
   let content: Buffer
   try {
@@ -93,16 +102,21 @@ export const PUT = withRouteHandler(async (request: NextRequest) => {
   try {
     const { file } = await saveBrowserDownload.execute({
       principal,
-      input: { toolCallId: parsed.data.query.toolCallId, name: parsed.data.query.name, content },
+      input: { toolCallId, name, content },
       request,
     })
     return NextResponse.json({ path: workspaceFileVfsPath(file), name: file.name, size: file.size })
   } catch (error) {
-    const response = internalFileErrorPolicies.concealContentAuthorization.project(error)
-    if (!response) throw error
-    return NextResponse.json(withRequestId(response.body), {
-      status: response.status,
-      headers: response.headers,
-    })
+    return saveErrorResponse(error)
   }
 })
+
+/** Projects a typed download-save failure, rethrowing anything the policy does not classify. */
+function saveErrorResponse(error: unknown): NextResponse {
+  const response = internalFileErrorPolicies.concealContentAuthorization.project(error)
+  if (!response) throw error
+  return NextResponse.json(withRequestId(response.body), {
+    status: response.status,
+    headers: response.headers,
+  })
+}

apps/sim/lib/browser-agent/application/browser-file-transfer.test.ts

@@ -47,6 +47,7 @@ vi.mock('@/lib/workspace-files/application/create-workspace-file', () => ({
 }))
 
 import {
+  admitBrowserDownloadSave,
   readBrowserUploadFile,
   saveBrowserDownload,
 } from '@/lib/browser-agent/application/browser-file-transfer'
@@ -231,6 +232,28 @@ describe('browser file transfer use cases', () => {
     expect(mocks.createFile).not.toHaveBeenCalled()
   })
 
+  it('admits a download save without claiming, creating, or auditing, and refuses a foreign run', async () => {
+    mocks.getAsyncToolCall.mockResolvedValue(
+      claimedCall('browser_save_download', { downloadId: 'd1' })
+    )
+    await expect(
+      admitBrowserDownloadSave(principal, { toolCallId: 'call-1', name: 'report.csv' })
+    ).resolves.toBeUndefined()
+
+    mocks.getRunSegment.mockResolvedValue({
+      id: 'run-1',
+      userId: 'someone-else',
+      workspaceId: 'workspace-1',
+      chatId: 'chat-1',
+    })
+    await expect(
+      admitBrowserDownloadSave(principal, { toolCallId: 'call-1', name: 'report.csv' })
+    ).rejects.toMatchObject({ code: 'not_found' })
+    expect(mocks.claimDownload).not.toHaveBeenCalled()
+    expect(mocks.createFile).not.toHaveBeenCalled()
+    expect(mocks.recordAudit).not.toHaveBeenCalled()
+  })
+
   it('falls back to the downloaded file name and never saves for an upload call', async () => {
     mocks.getAsyncToolCall.mockResolvedValueOnce(
       claimedCall('browser_save_download', { downloadId: 'd1' })

apps/sim/lib/browser-agent/application/browser-file-transfer.ts

@@ -103,39 +103,66 @@ export const readBrowserUploadFile = defineAuthorizedWorkspaceFileUseCase({
   },
 })
 
-export interface SaveBrowserDownloadInput {
+export interface AdmitBrowserDownloadSaveInput {
   toolCallId: string
   /** The download's file name on this machine; the call's own `name` argument wins. */
   name: string
+}
+
+export interface SaveBrowserDownloadInput extends AdmitBrowserDownloadSaveInput {
   content: Buffer
 }
 
+async function resolveBrowserDownloadSaveContext({
+  principal,
+  input,
+}: {
+  principal: Principal
+  input: AdmitBrowserDownloadSaveInput
+}) {
+  const binding = await loadBrowserFileTransferBinding(
+    principal,
+    input.toolCallId,
+    'browser_save_download'
+  )
+  const workspace = await loadActiveWorkspaceContext(binding.workspaceId)
+  if (!workspace) throw new OrchestrationError('not_found', 'Workspace not found')
+  const requestedName = binding.args.name
+  return {
+    ...workspace,
+    name:
+      typeof requestedName === 'string' && requestedName.trim() ? requestedName.trim() : input.name,
+  }
+}
+
+const admitBrowserDownloadSaveUseCase = defineAuthorizedWorkspaceFileUseCase({
+  operation: fileOperations.create,
+  resolveContext: resolveBrowserDownloadSaveContext,
+  async execute() {},
+})
+
+/**
+ * Admits a `browser_save_download` upload before its body is read, so a caller without a claimed,
+ * running call of its own is refused without buffering the download. {@link saveBrowserDownload}
+ * re-validates and claims the call atomically after the read.
+ */
+export async function admitBrowserDownloadSave(
+  principal: Principal,
+  input: AdmitBrowserDownloadSaveInput
+): Promise<void> {
+  await admitBrowserDownloadSaveUseCase.execute({ principal, input })
+}
+
 /** Stores a completed browser download as a workspace file for a claimed `browser_save_download` call. */
 export const saveBrowserDownload = defineAuthorizedWorkspaceFileUseCase({
   operation: fileOperations.create,
-  async resolveContext({
+  resolveContext: ({
     principal,
     input,
   }: {
     principal: Principal
     input: SaveBrowserDownloadInput
-  }) {
-    const binding = await loadBrowserFileTransferBinding(
-      principal,
-      input.toolCallId,
-      'browser_save_download'
-    )
-    const workspace = await loadActiveWorkspaceContext(binding.workspaceId)
-    if (!workspace) throw new OrchestrationError('not_found', 'Workspace not found')
-    const requestedName = binding.args.name
-    return {
-      ...workspace,
-      name:
-        typeof requestedName === 'string' && requestedName.trim()
-          ? requestedName.trim()
-          : input.name,
-    }
-  },
+  }) => resolveBrowserDownloadSaveContext({ principal, input }),
   async execute({ principal, input, context }) {
     if (!(await claimBrowserDownloadSave(input.toolCallId))) {
       throw new OrchestrationError(