Repository navigation
mcpx(mcpclient): a non-JSON 200 reply is buffered whole with no size cap — Send's fallback io.ReadAll is unbounded while the error path in the same function caps at 4 KB #4282
Description
Problem
pkgs/mcpx/internal/mcpclient/http.go HTTPTransport.Send handles a 2xx reply whose body is not SSE by decoding the first JSON value (http.go:204-209). When that decode fails — the body is not JSON at all — the fallback hands the body on whole "for push to judge":
rest, readErr := io.ReadAll(resp.Body) // http.go:218 — no cap
...
b := append(head.buf.Bytes(), rest...)
if len(bytes.TrimSpace(b)) > 0 {
t.push(b)
}io.ReadAll here has no size bound at all, and the bytes are also duplicated through the firstValue tee buffer on the way in. The same function's non-2xx path, a few lines earlier (http.go:167), is careful to cap its body read at defaults.HTTPErrorBodyLimit (4 KB, "enough for a JSON-RPC error, not enough for an HTML error page to matter") — the garbage path for a successful status has no equivalent bound. A remote MCP server (or a proxy/error page in front of one) that answers 200 with a large or endless non-JSON body makes mcpx buffer the entire body in memory and deliver it as a single incoming "message". The HTTP client's overall timeout bounds the time, not the bytes: a fast server can push gigabytes inside it.
Evidence (verified this run)
Laptop probe (full verbatim copy of pkgs/mcpx @ fa28f499; http.go md5 691386b8… identical in both clones; probe dir ~/git/fleet/probe-2026-11-02-laneB-mcpclientbody/, test internal/mcpclient/probe_http_test.go, go test on the laptop):
- Test server replies 200,
Content-Type: application/json, body ="this is not json "+ 32 MiB ofx.Send+Recvdeliver a pushed message of 33,554,449 bytes — the whole garbage body, uncapped. - Control: a valid single-JSON reply is pushed as-is (
{"jsonrpc":"2.0","id":1,"result":{}}) — the streaming decode path is unaffected.
Why it matters
mcpx talks to third-party MCP servers by design; their failure modes are not under our control. The one reply shape that is definitionally not a protocol message — a body whose first value isn't even JSON — is the one shape read without any limit, turning a misbehaving/proxied upstream into unbounded memory growth in the client. The house already picked the bound for exactly this situation (4 KB error-body cap in the same function); the fallback just never got it.
Acceptance criteria
- The non-JSON fallback read in
Sendis bounded (e.g.io.LimitReaderat a named limit —defaults.HTTPErrorBodyLimitor a new explicit one); a body past the limit is truncated or surfaced as an error, never buffered whole. - Regression test: an oversized non-JSON 200 body does not produce an oversized pushed message (probe above reproduces at 32 MiB today); a small non-JSON body keeps today's hand-on-whole behaviour; the valid-JSON and SSE paths are unchanged.
-
go test ./internal/mcpclient/passes.
Distinct from: #2114 (open — daemon chat-completions proxy buffering an upstream response; different package, direction, and code path), #2082 (closed — daemon request bodies bypassing http.*BodyLimit).
Activity
dezren39 commentedon Nov 2, 2026
🤖 Agent note · filed
dezren39 commentedon Nov 2, 2026
🤖 Agent note · picked up
HTTPTransport.Send(pkgs/mcpx/internal/mcpclient/http.go) on current main and route the fix.108 remaining items
dezren39 commentedon Nov 5, 2026
🤖 Agent note · handed off
dezren39 commentedon Nov 5, 2026
🤖 Agent note · handed off
dezren39 commentedon Nov 5, 2026
Picked up task 8392 on the OpenChamber scheduled lane. Fresh verification found issue #4282 open and PR #4417 open, authored by dezren39 (Drew), explicitly fixing this issue. No files touched; avoiding duplicate work on the Drew-owned PR.
dezren39 commentedon Nov 5, 2026
Task 8392 parked for 30 minutes because open Drew-authored PR #4417 owns the fix. Resume after the delay and freshly revalidate PR #4417 before any work. No branch or worktree created.
dezren39 commentedon Nov 5, 2026
🤖 Agent note · handed off
developing-today commentedon Nov 5, 2026
🤖 Agent note · handed off