fix(sandbox): cap the Go daemon's MCP JSON-RPC response body - #6162
Merged
Conversation
The plain-JSON branch of readRPCBody read a downstream MCP endpoint's response with unbounded io.ReadAll, while the SSE branch next to it already caps the frame at 8MB via the scanner buffer. A misbehaving or malicious MCP connection returning application/json could stream an arbitrarily large body straight into daemon memory during catalog fetch. Cap the plain-JSON path at the same 8MB via LimitReader, matching the file's existing threat model (see maxCatalogPages above it).
pedrofrxncx
enabled auto-merge (squash)
August 18, 2026 14:09
pedrofrxncx
added a commit
that referenced
this pull request
Aug 18, 2026
decodeBody (used by every /fs/* route: read, write, edit, mkdir, unlink, mv, list, write_from_url, upload_to_url) read the request body with an unbounded io.ReadAll. A caller sending an oversized body could buffer it entirely into the daemon's memory and crash it, tearing down the sandbox pod on the next missed health probe. Cap it at maxTransferBytes (500MB), the same bound the daemon already uses for its other file-transfer path, following the same LimitReader pattern used to cap the JSON-RPC response body in #6162. Test: TestDecodeBodyRejectsOversizedRequest drives decodeBody with an infinite reader and asserts it rejects instead of buffering forever.
pedrofrxncx
added a commit
that referenced
this pull request
Aug 18, 2026
decodeBody (used by every /fs/* route: read, write, edit, mkdir, unlink, mv, list, write_from_url, upload_to_url) read the request body with an unbounded io.ReadAll. A caller sending an oversized body could buffer it entirely into the daemon's memory and crash it, tearing down the sandbox pod on the next missed health probe. Cap it at maxTransferBytes (500MB), the same bound the daemon already uses for its other file-transfer path, following the same LimitReader pattern used to cap the JSON-RPC response body in #6162. Test: TestDecodeBodyRejectsOversizedRequest drives decodeBody with an infinite reader and asserts it rejects instead of buffering forever.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug found while auditing
packages/sandbox/daemon-go/internal/toolscatalog/mcp.gofor resource-leak / oversized-payload gaps per the file's own stated threat model.readRPCBody's plain-JSON branch calledio.ReadAll(res.Body)with no size limit, while the SSE branch right next to it already caps a frame at 8MB via the scanner buffer (sc.Buffer(make([]byte, 0, 64*1024), 8*1024*1024)). The file already documents this exact threat model for pagination (maxCatalogPages's comment: "a misbehaving or malicious endpoint") — the same endpoint answeringapplication/jsoninstead of SSE could stream an unbounded body straight into the daemon's memory during a tools catalog fetch, with no cap at all.Fix: cap the plain-JSON path at the same 8MB via
io.LimitReader, returning an error when exceeded — matching the SSE branch's existing bound instead of adding a new one.Failure scenario before the fix: a malicious or buggy downstream MCP connection responding with
Content-Type: application/jsonand an arbitrarily large body could exhaust the daemon pod's memory on every catalog fetch against that connection.Regression test:
TestReadRPCBodyBoundsPlainJSONinmcp_test.go— a test server writesmaxRPCResponseBytes+1bytes asapplication/json; assertsreadRPCBodyrejects it with an "exceeded" error instead of buffering it.Reviewer check:
cd packages/sandbox/daemon-go && go test ./internal/toolscatalog/...Locally ran:
go build ./...,go test ./internal/toolscatalog/...(both existing and new tests pass),gofmt -landgo veton the touched files (clean). Full CI validates the rest of the daemon-go suite.Summary by cubic
Caps the Go daemon’s MCP JSON‑RPC response body at 8MB to prevent unbounded memory use. Previously,
readRPCBodyusedio.ReadAllforapplication/jsonresponses with no limit; now it usesio.LimitReaderand returns an error when the body exceeds 8MB, matching the SSE branch’s per‑frame cap.Review notes
maxRPCResponseBytes; oversized responses now fail with a “exceeded” error. SSE handling is unchanged.TestReadRPCBodyBoundsPlainJSONto assert rejection of oversized bodies.packages/sandbox/daemon-go, rungo test ./internal/toolscatalog/....Written for commit 469d346. Summary will update on new commits.