Skip to content

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

Open

Description

@dezren39

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) in OpenAIProvider.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 in OpenAICompatibleProvider.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, against httptest servers):
    • internal/model: server returns HTTP 500 with a 33,554,432-byte body of x → Generate returns 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 → Complete returns 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.HTTPErrorBodyLimit for 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.HTTPErrorBodyLimit in 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/model and internal/scripting: an httptest provider 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=1 passes (package-scoped per pkgs/mcpx/internal/AGENTS.md).

Activity

  1. added
    kind/bugSomething isn't working (prefixed form of the classic bug label)
    task/triageQueue packet kind: verify, reproduce, and route this item
    source/creatorFiled by the new-issue-creator job
    priority/normalOrdinary priority (the default)
    queue/queuedHas a queued task in the Supabase agent queue
    app/mcpxPackage/app: mcpx (pkgs/ or apps/ in this repo)
    effort/sSmall: expected under 2 hours of work
    on Nov 2, 2026
  2. dezren39 commented on Nov 2, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · filed

  3. dezren39 commented on Nov 2, 2026

    @dezren39
    MemberAuthor

    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 named defaults cap 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).

  4. added
    work/assignedQueue worker lifecycle state
    and removed
    queue/queuedHas a queued task in the Supabase agent queue
    on Nov 2, 2026
  5. 53 remaining items

  6. dezren39 commented on Nov 4, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · parked

  7. added
    queue/parkedQueue task is parked (waiting on CI, time, or an event)
    status/parkedDeliberately paused with a recorded resume condition
    queue/claimedQueue worker lifecycle state
    queue/queuedHas a queued task in the Supabase agent queue
    and removed
    queue/claimedQueue worker lifecycle state
    status/parkedDeliberately paused with a recorded resume condition
    queue/parkedQueue task is parked (waiting on CI, time, or an event)
    work/assignedQueue worker lifecycle state
    on Nov 4, 2026
  8. dezren39 commented on Nov 5, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · handed off

    • Agent: fleet monitor
    • Session / ID: task 8783 (repo-fleet-monitor run 2026-11-05T15:45:48Z)
    • Found via: monitor reconciliation (Duty 1 claim/label divergence repair)
    • Task: Claim evaporated: task 8783 is queued and unclaimed in repo-health-code-pilot, but this issue still carried claimed labels. Labels reset to the truth (queue/queued + status/needs-triage). Checkpoint stage at evaporation: handed_off.
    • Expectation: The next lane claim picks task 8783 up from the queued state and posts a fresh pickup note.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

app/mcpxPackage/app: mcpx (pkgs/ or apps/ in this repo)effort/sSmall: expected under 2 hours of workkind/bugSomething isn't working (prefixed form of the classic bug label)priority/normalOrdinary priority (the default)queue/queuedHas a queued task in the Supabase agent queuesource/creatorFiled by the new-issue-creator jobsource/design-architectFiled by the repo design architect jobstatus/needs-triageNot yet triaged; needs a first looktask/triageQueue packet kind: verify, reproduce, and route this item

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions