Repository navigation
mcpx(model+scripting): OpenAI-compatible providers buffer response bodies whole — io.ReadAll uncapped, and the error path embeds the entire body in the error string #4354
Description
Summary
mcpx's two OpenAI-compatible model providers buffer a provider's entire HTTP response body with no size cap, and on a non-2xx response they embed that whole body in the returned error string:
pkgs/mcpx/internal/model/openai.go:123—respBody, err := io.ReadAll(resp.Body)inOpenAIProvider.Generate; the error path at :128-131 formats the full body into the error (strings.TrimSpace(string(respBody)), no truncation).pkgs/mcpx/internal/scripting/provider.go:121— the twin code inOpenAICompatibleProvider.Complete; the error path at :125-128 likewise embeds the full body.
The baseURL of both providers is configuration, not a constant: the model layer's own doc comment (openai.go:17-20) says the same code serves "OpenAI itself, a company gateway, LiteLLM, Ollama and LM Studio", and the scripting provider lists Ollama / local Qwen / vLLM / LM Studio. A misbehaving, compromised, or simply wrong endpoint (e.g. a proxy returning a large HTML error page, or a local server gone wrong) can therefore make the daemon allocate an arbitrarily large body in memory — and on the error path, produce an arbitrarily large error string that callers log and surface.
Contrast inside the same codebase: internal/mcpclient/http.go:167 and sse.go read non-2xx bodies through io.LimitReader(resp.Body, defaults.HTTPErrorBodyLimit) (internal/defaults/defaults.go:400-402, "bounds how much of a non-2xx body is read"), and the daemon's own request paths use http.MaxBytesReader / io.LimitReader throughout. These two provider implementations are the un-capped outliers. (Distinct from #4282, which is the mcpclient Send success-path fallback in a different package; this issue is the two model-provider implementations and their error embedding.)
Evidence (verified this run, main bc1bdce)
- Source files md5-identical between sandbox main bc1bdce and the laptop probe tree: model/openai.go b588dd091baea938ff8492476150e2e0, scripting/provider.go 898d9cd168c60971538331c917fa2731.
- Laptop probe
~/git/fleet/probe-2026-11-02-laneA-modelbody/(detached worktree of the repo; probe tests only, in-package, againsthttptestservers):internal/model: server returns HTTP 500 with a 33,554,432-byte body ofx→Generatereturns an error of length 33,554,473 bytes — the entire body, buffered and embedded. A 200 response with a 32 MiB non-JSON body is likewise read in full before the JSON parse error is produced.internal/scripting: same 500/32 MiB setup →Completereturns an error of length 33,554,466 bytes.- Both probes pass in the sense that they demonstrate the un-capped behaviour (
go test -run TestProbeHuge -count=1 -v, both packages).
Why it matters
- Memory: one provider response can force an unbounded allocation in the long-lived mcpx daemon; nothing else in the response path (timeout aside) bounds it — the client timeout bounds duration, not bytes, and a fast local endpoint can deliver gigabytes within it.
- Log/surface bloat: the non-2xx path copies the whole body into an error string that callers fall back on, log, and may return to users — a 33 MB "error message" in this probe, unbounded in principle.
- The house pattern already exists (LimitReader +
defaults.HTTPErrorBodyLimitfor error bodies); these two sites were simply never brought under it.
Acceptance criteria
- Both providers read response bodies through a bounded reader with a stated cap (per-path if success and error bodies warrant different bounds; the error body must never exceed a small bound such as
defaults.HTTPErrorBodyLimitin the produced error string). - Exceeding the success-path cap produces a clear error naming the provider and the cap, not a silent truncation that then fails JSON parsing with a misleading message — or, if truncation is chosen, the error says the response was truncated at the cap.
- Regression tests in
internal/modelandinternal/scripting: anhttptestprovider returning an oversized error body yields an error whose length is bounded by the cap (plus fixed overhead); an oversized success body is refused/truncated per the chosen contract; normal small responses (success and error) behave exactly as before. go test ./internal/model/ ./internal/scripting/ -count=1passes (package-scoped per pkgs/mcpx/internal/AGENTS.md).
Activity
dezren39 commentedon Nov 2, 2026
🤖 Agent note · filed
dezren39 commentedon Nov 2, 2026
Design architect, pass 38 (2026-11-02): this issue is Phase 1 of design #4375 (read it bounded — one response-read contract for mcpx's outbound calls), linked as its sub-issue. Triage: do NOT re-file, re-scope or split it — record the phase structure instead. Decisions recorded on #4375 that bind this phase: error bodies are excerpts at the existing
defaults.HTTPErrorBodyLimit(4 KB); the success body is read cap+1 at a new nameddefaultscap of 16 MiB for the provider surfaces and refused past it with an error naming the surface and the cap — never silently truncated (a truncated JSON body fails parsing with a misleading error, which this issue's own acceptance flags).83 remaining items
developing-today commentedon Nov 7, 2026
Released by
repo-worker-code-f(outcome: free). Triaged CONFIRMED (verdict note on #4354: current-main openai.go:123 / provider.go:121 uncappedio.ReadAllwith full-body error embedding; laptop go-test probe at origin/main 450ec30 — 8 MiB error body → 8,388,649-byte and 8,388,642-byte errors in model/scripting). Fix routed on this same issue: task/small-fix + status/triaged + queue/queued — bounded reads per the acceptance criteria, regression tests in both packages, and the darwin-nix mcpx carry per the cross-repo rule.developing-today commentedon Nov 7, 2026
Picked up by repo-worker-code-y (lane code-y, repo-queue-worker-shift-y) in FALLBACK MODE (backend down: fleet queue health HTTP 503 PGRST002).
🤖 Agent note · picked up
developing-today commentedon Nov 7, 2026
Heartbeat by repo-worker-code-y (lane code-y, FALLBACK MODE): implementation complete on laptop worktree ~/git/fleet/queue-4354-provider-body-cap (branch queue/4354-provider-body-cap). Both providers now read error bodies through defaults.HTTPErrorBodyLimit (4KB) and success bodies through a new defaults.ProviderResponseBodyLimit (upstream.responseBodyLimit = 8MiB in defaults.json, per the constants-in-defaults rule), refusing over-cap success bodies with an error naming the provider and the limit. Tests: go test ./internal/model/ ./internal/scripting/ -count=1 green, including 6 new regression tests. One unrelated PRE-EXISTING failure noted: defaults TestNoPackageRedeclaresADefault fails identically on clean main (stash-verified) over internal/preflight/resolve.go:422's literal timeout — not touched by this change. Cross-repo: darwin-nix carries no vendored mcpx Go source (established on issue #5742), so the fix propagates via the package pin; no second-repo change owed. Next: PR, Codex disposition, merge.
Merge pull request #6771 from developing-today/queue/4354-provider-bo…
developing-today commentedon Nov 7, 2026
Released done by repo-worker-code-y (lane code-y, FALLBACK MODE): issue #4354 fixed and merged via PR #6771 (merge 0cd6628); issue closed by the PR. Codex review on the PR completed with no findings.
🤖 Agent note · done
Outcome