Skip to content

Commit 6c6e04d

Browse files
committed
fix(chat): read a sandbox copy body once under the cap before routing it
Parsing a clone of the live stream hung on an oversized body, because cancelling one teed branch waits for the unread other. The copy body is now buffered once at the entry, and both the protocol check and either copy path read that in-memory request.
1 parent 8e64726 commit 6c6e04d

2 files changed

Lines changed: 66 additions & 35 deletions

File tree

‎apps/sim/lib/mothership/tools/sandbox-resource-transport.test.ts‎

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -467,30 +467,32 @@ it('validates a copy body itself once the lease negotiated the file-owner protoc
467467
expect(fetcher).not.toHaveBeenCalled()
468468
})
469469

470-
it('leaves an oversized copy body to the workspace route without reading it past the cap', async () => {
471-
readScope.mockResolvedValue({ ...scope, fileOwnerProtocolVersion: undefined })
472-
routeMatcher.mockReturnValue({ params: {}, load: async () => ({ POST: fetcher }) })
473-
fetcher.mockResolvedValue(Response.json({ error: 'Request body is too large' }, { status: 413 }))
474-
const projectCopy = new TextEncoder().encode(
475-
JSON.stringify({
476-
source: { owner: workspaceOwner, fileIds: ['file'] },
477-
destination: { owner: { entityType: 'project', entityId: 'project' } },
470+
it.each([undefined, 1 as const])(
471+
'refuses an oversized streamed copy body at the cap without dispatch (protocol %s)',
472+
async (fileOwnerProtocolVersion) => {
473+
readScope.mockResolvedValue({ ...scope, fileOwnerProtocolVersion })
474+
routeMatcher.mockReturnValue({ params: {}, load: async () => ({ POST: fetcher }) })
475+
const projectCopy = new TextEncoder().encode(
476+
JSON.stringify({
477+
source: { owner: workspaceOwner, fileIds: ['file'] },
478+
destination: { owner: { entityType: 'project', entityId: 'project' } },
479+
})
480+
)
481+
const padding = new Uint8Array(1024 * 1024).fill(0x20)
482+
const paddingChunks = Math.ceil((2 * DEFAULT_MAX_JSON_BODY_BYTES) / padding.byteLength)
483+
let pulledChunks = 0
484+
const body = new ReadableStream<Uint8Array>({
485+
pull(controller) {
486+
if (pulledChunks === 0) controller.enqueue(projectCopy)
487+
else if (pulledChunks <= paddingChunks) controller.enqueue(padding)
488+
else controller.close()
489+
pulledChunks++
490+
},
478491
})
479-
)
480-
const padding = new Uint8Array(1024 * 1024).fill(0x20)
481-
const paddingChunks = Math.ceil((2 * DEFAULT_MAX_JSON_BODY_BYTES) / padding.byteLength)
482-
let pulledChunks = 0
483-
const body = new ReadableStream<Uint8Array>({
484-
pull(controller) {
485-
if (pulledChunks === 0) controller.enqueue(projectCopy)
486-
else if (pulledChunks <= paddingChunks) controller.enqueue(padding)
487-
else controller.close()
488-
pulledChunks++
489-
},
490-
})
491-
const init: RequestInit & { duplex: 'half' } = { method: 'POST', body, duplex: 'half' }
492-
const response = await proxySandboxResourceRequest(request('/api/v2/files/copy', init), token)
493-
expect(response.status).toBe(413)
494-
expect(fetcher).toHaveBeenCalledTimes(1)
495-
expect(pulledChunks).toBeLessThan(paddingChunks)
496-
})
492+
const init: RequestInit & { duplex: 'half' } = { method: 'POST', body, duplex: 'half' }
493+
const response = await proxySandboxResourceRequest(request('/api/v2/files/copy', init), token)
494+
expect(response.status).toBe(413)
495+
expect(fetcher).not.toHaveBeenCalled()
496+
expect(pulledChunks).toBeLessThan(paddingChunks)
497+
}
498+
)

‎apps/sim/lib/mothership/tools/sandbox-resource-transport.ts‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,14 @@ import { v2CopyFileItemsContract } from '@/lib/api/contracts/v2/file-copy'
55
import { markCopilotRequest } from '@/lib/api/server/routes/copilot-request'
66
import { matchV2Route } from '@/lib/api/server/routes/in-process-transport'
77
import { V2_PARSE_DEFAULTS } from '@/lib/api/server/routes/v2-json-route'
8-
import { parseRequest } from '@/lib/api/server/validation'
8+
import { DEFAULT_MAX_JSON_BODY_BYTES, parseRequest } from '@/lib/api/server/validation'
99
import { withWorkspaceInvocationScope } from '@/lib/core/application/workspace-invocation-scope'
1010
import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types'
11+
import {
12+
assertContentLengthWithinLimit,
13+
isPayloadSizeLimitError,
14+
readStreamToBufferWithLimit,
15+
} from '@/lib/core/utils/stream-limits'
1116
import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
1217
import { createResourceEffectTransport } from '@/lib/mothership/agent-cli/resource-effects'
1318
import { resolveInvocationWorkspace } from '@/lib/mothership/application/workspace-target'
@@ -28,9 +33,10 @@ const logger = createLogger('MothershipSandboxResourceTransport')
2833

2934
/** Private callback observes the real authenticated v2 request without changing its body or API contract. */
3035
export async function proxySandboxResourceRequest(
31-
request: Request,
36+
callback: Request,
3237
token: string
3338
): Promise<Response> {
39+
let request = callback
3440
const url = new URL(request.url)
3541
const prefix = `/api/mothership/sandbox/${token}`
3642
const path = url.pathname.slice(prefix.length)
@@ -52,11 +58,13 @@ export async function proxySandboxResourceRequest(
5258
* owner is refused there; workspace copies and invalid bodies keep the workspace route and its
5359
* v2 validation response.
5460
*/
55-
if (
56-
path === v2CopyFileItemsContract.path &&
57-
(scope.fileOwnerProtocolVersion === 1 || (await copiesBeyondWorkspaces(request)))
58-
)
59-
return proxySandboxFileCopyRequest(request, token, scope)
61+
if (path === v2CopyFileItemsContract.path) {
62+
const buffered = await bufferCopyRequest(request)
63+
if (buffered instanceof Response) return buffered
64+
request = buffered
65+
if (scope.fileOwnerProtocolVersion === 1 || (await copiesBeyondWorkspaces(request)))
66+
return proxySandboxFileCopyRequest(request, token, scope)
67+
}
6068
const ownerHeader = request.headers.get('x-mothership-file-owner')
6169
let owner: FileOperationOwner | undefined
6270
if (ownerHeader !== null) {
@@ -204,10 +212,31 @@ async function proxyAuthorizedSandboxRequest(
204212
})
205213
}
206214

215+
/**
216+
* Reads a copy body once, under the v2 body cap, so the protocol check and whichever copy path
217+
* handles it each read an in-memory copy. Parsing a clone of the live stream instead would hang on
218+
* an oversized body: cancelling one branch of a teed stream waits for the unread other branch.
219+
*/
220+
async function bufferCopyRequest(request: Request): Promise<Request | Response> {
221+
if (!request.body) return request
222+
try {
223+
assertContentLengthWithinLimit(request.headers, DEFAULT_MAX_JSON_BODY_BYTES, 'Request body')
224+
const body = await readStreamToBufferWithLimit(request.body, {
225+
maxBytes: DEFAULT_MAX_JSON_BODY_BYTES,
226+
label: 'Request body',
227+
signal: request.signal,
228+
})
229+
return new Request(request, { body: new Uint8Array(body) })
230+
} catch (error) {
231+
if (isPayloadSizeLimitError(error)) return V2_PARSE_DEFAULTS.payloadTooLargeResponse()
232+
throw error
233+
}
234+
}
235+
207236
/**
208237
* A valid copy naming a non-workspace source or destination needs the file-owner protocol. Parsed
209-
* as the copy route parses it, under the same body cap: an oversized or invalid body stays on the
210-
* workspace route, which returns its own 413 or 400.
238+
* as the copy route parses it: an invalid body stays on the workspace route, which returns its own
239+
* 400.
211240
*/
212241
async function copiesBeyondWorkspaces(request: Request): Promise<boolean> {
213242
const parsed = await parseRequest(

0 commit comments

Comments
 (0)