fix(agents): bound body-less MCP HTTP text responses by Pick-cat · Pull Request #98143 · openclaw/openclaw

Pick-cat · 2026-06-30T12:11:37Z

What Problem This Solves

The MCP HTTP wrapper converted body-less foreign Response implementations by calling text(). That API materializes the complete remote body before OpenClaw can enforce a byte limit, so a hostile or compressed OAuth error response could exhaust memory.

Closes #97521.

Why This Change Was Made

This takes the issue's fail-closed policy option. A body-less foreign response has no genuinely bounded reader: Content-Length is remote-controlled and can describe compressed wire bytes rather than decoded text. The wrapper now preserves status and headers but never calls text() or arrayBuffer() for that shape. Normal responses with readable body streams are unchanged.

The test rewrite removes repetitive response classes and covers missing, plausible, oversized, and dishonest Content-Length values. MCP SDK source was checked directly: parseErrorResponse reads the global Response.text(), so the synthesized empty body still produces a useful HTTP-status diagnostic without touching the foreign unbounded method.

User Impact

Malformed/nonconforming body-less MCP OAuth responses now report a generic status-based OAuth error instead of preserving their raw error text. This is intentional: it removes the memory-exhaustion path while retaining status and headers. Standards-compliant streamed responses retain full error parsing.

Evidence

Blacksmith Testbox tbx_01kwt6qx4r4ptdbg4ps3yhnj55: src/agents/mcp-http-fetch.test.ts, 13/13 passed.
Remote oxfmt --check passed for the changed source, tests, and changelog.
Fresh full-diff autoreview after the fail-closed rewrite: no actionable findings, confidence 0.94.
git diff --check passed.

No docs change is needed; the Unreleased changelog entry is included.

clawsweeper · 2026-06-30T12:12:57Z

Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 7:42 PM ET / 23:42 UTC.

Summary
The PR changes MCP HTTP fetch response normalization to stop reading body-less foreign response text bodies, adds focused MCP OAuth tests, and adds an Unreleased changelog entry.

PR surface: Source -2, Tests +27, Docs +1. Total +26 across 3 files.

Reproducibility: yes. source-reproducible: current main reaches await response.text() for a body-less non-204/205/304 MCP response, and the existing compatibility test exercises that text-only foreign-response shape. I did not run a live repro in this read-only review.

Review metrics: 1 noteworthy metric.

Fallback behavior changed: 1 fallback removed. The only runtime change is compatibility-sensitive because it removes the body-less text fallback for foreign MCP Response objects.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #97521
Summary: This PR is the canonical candidate fix for the open MCP body-less foreign-response policy issue; an older 1 MiB variant was already closed as duplicate of this PR.

Members:

canonical: MCP HTTP fetch: bodyless foreign response needs explicit policy decision (text-cap vs fail-closed) #97521 - The issue tracks the exact body-less MCP HTTP response compatibility/security policy that this PR implements.
duplicate: fix(mcp): bound body-less MCP HTTP text responses at 1 MiB #99293 - The closed PR targeted the same MCP body-less HTTP response problem and was closed in favor of this PR as the stronger canonical landing path.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P2] Wait for exact-current-head required checks to finish green before merge.

Risk before merge

[P2] This intentionally removes the old text-only body-less foreign-response fallback, so nonconforming custom MCP transports lose raw OAuth error-body parsing and get a generic status-based diagnostic instead.

Maintainer options:

Accept the fail-closed policy (recommended)
Land the current head with the intentional tradeoff that body-less foreign OAuth error bodies no longer preserve raw error text.
Preserve text-only diagnostics instead
Replace the fail-closed branch with a separate compatibility-preserving design only if maintainers decide raw diagnostics are worth the residual allocation risk.

Next step before merge

No automated repair is needed; the remaining action is ordinary maintainer landing after current-head gates and compatibility-risk acceptance.

Security
Cleared: The diff narrows an availability-sensitive MCP transport path and does not add dependencies, secret handling, workflow execution, or supply-chain changes.

Review details

Best possible solution:

Land the fail-closed MCP transport fix after exact-head gates, keeping raw OAuth body parsing only for standards-compliant streamed responses.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible: current main reaches await response.text() for a body-less non-204/205/304 MCP response, and the existing compatibility test exercises that text-only foreign-response shape. I did not run a live repro in this read-only review.

Is this the best way to solve the issue?

Yes, this is the best available fix shape for the availability risk: the owner boundary is the MCP fetch Response normalizer, and the alternative Content-Length/text-cap designs still call an unbounded text() implementation before any cap can be enforced.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 2ded26a5d6be.

Label changes

Label changes:

add merge-risk: 🚨 compatibility: The PR intentionally stops preserving raw text from body-less foreign MCP OAuth error responses, changing behavior for nonconforming custom transports.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body and comments include Blacksmith Testbox output plus loopback/production-entry proof for the body-less MCP response path, with no private data shown.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body and comments include Blacksmith Testbox output plus loopback/production-entry proof for the body-less MCP response path, with no private data shown.

Label justifications:

P2: This is a focused MCP availability hardening bug with limited blast radius and no active outage evidence.
merge-risk: 🚨 compatibility: The PR intentionally stops preserving raw text from body-less foreign MCP OAuth error responses, changing behavior for nonconforming custom transports.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body and comments include Blacksmith Testbox output plus loopback/production-entry proof for the body-less MCP response path, with no private data shown.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body and comments include Blacksmith Testbox output plus loopback/production-entry proof for the body-less MCP response path, with no private data shown.

Evidence reviewed

PR surface:

Source -2, Tests +27, Docs +1. Total +26 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	2	4	-2
Tests	1	56	29	+27
Docs	1	1	0	+1
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	59	33	+26

What I checked:

Current main has the unbounded fallback: On current main, ensureGlobalFetchResponse calls await response.text() for body-less non-204/205/304 responses, which is the availability issue this PR targets. (src/agents/mcp-http-fetch.ts:64, 2ded26a5d6be)
PR head removes the unbounded read: At PR head, the same branch preserves status/statusText/headers but returns new Response(null, init) for body-less foreign responses without calling text() or arrayBuffer(). (src/agents/mcp-http-fetch.ts:64, 8240e5782a23)
Focused regression coverage at PR head: The new tests cover missing, plausible, oversized, and lying Content-Length body-less foreign OAuth responses and assert text() is not called while the SDK parser still reports HTTP 400 for the empty diagnostic body. (src/agents/mcp-http-fetch.test.ts:243, 8240e5782a23)
MCP SDK contract inspected: The pinned MCP SDK FetchLike returns a Promise<Response>, and parseErrorResponse awaits input.text(), so the OpenClaw wrapper is protecting an SDK-facing Response-normalization boundary. (modelcontextprotocol/typescript-sdk/src/client/auth.ts:379)
Related policy issue remains canonical: The linked issue is open and tracks the exact compatibility/security policy decision for body-less MCP HTTP responses; this PR is the candidate implementation path.
Maintainer review context: A current PR assignee posted land-ready review context for the fail-closed policy, Blacksmith Testbox proof, SDK inspection, formatting, hosted CI, and native review artifacts before the final branch rewrite to the same stricter policy. (8240e5782a23)

Likely related people:

LiuwqGit: Authored the merged MCP OAuth repair work that added the SDK OAuth error compatibility test and Response-normalization path this PR changes. (role: introduced behavior; confidence: high; commits: 7efaf4cf9ae4; files: src/agents/mcp-http-fetch.ts, src/agents/mcp-http-fetch.test.ts, src/agents/mcp-oauth.ts)
vincentkoc: Current-main blame on the normalized MCP HTTP fetch helper and its compatibility test points to recent main work by Vincent Koc, so this is a relevant current-source routing signal. (role: recent area contributor; confidence: medium; commits: 1408deede7cb; files: src/agents/mcp-http-fetch.ts, src/agents/mcp-http-fetch.test.ts)
pgondhi987: Authored earlier MCP HTTP redirect guard work around the same guarded fetch and response-normalization boundary. (role: adjacent owner; confidence: medium; commits: 3c6259ebb70c; files: src/agents/mcp-http-fetch.ts, src/agents/mcp-transport.ts, src/infra/net/fetch-guard.ts)
Pick-cat: Besides authoring this PR, Pick-cat has prior merged bounded provider response-read work in the adjacent provider HTTP error surface. (role: recent area contributor; confidence: medium; commits: eb5fb2aa69f4; files: src/agents/provider-http-errors.ts, src/agents/mcp-http-fetch.ts, src/agents/mcp-http-fetch.test.ts)
steipete: Assigned this PR, posted land-ready review context, and authored the current-head cleanup/fail-closed commits on the PR branch. (role: current PR reviewer and branch updater; confidence: medium; commits: f0c0fe0d89d2, 8240e5782a23; files: src/agents/mcp-http-fetch.ts, src/agents/mcp-http-fetch.test.ts, CHANGELOG.md)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (5 earlier review cycles)

reviewed 2026-06-30T16:06:32.154Z sha 8bb30cd :: needs maintainer review before merge. :: none
reviewed 2026-07-05T14:32:52.866Z sha d80ac22 :: needs maintainer review before merge. :: none
reviewed 2026-07-05T14:56:16.544Z sha d80ac22 :: needs maintainer review before merge. :: none
reviewed 2026-07-05T23:08:06.823Z sha ce41bf0 :: needs maintainer review before merge. :: none
reviewed 2026-07-05T23:14:53.059Z sha ce41bf0 :: needs maintainer review before merge. :: none

Pick-cat · 2026-06-30T14:17:43Z

@clawsweeper re-review

Addressed the previous compatibility/availability finding by making the bodyless foreign-response policy explicit.

Policy now:

Preserve text-only bodyless OAuth error parsing only when the foreign response declares a safe Content-Length at or below the MCP text cap.
Fail closed for bodyless text responses with missing/invalid Content-Length before calling text().
Fail closed for declared oversized Content-Length before calling text().
Keep a post-read UTF-8 byte check as defense in depth if the declared length lies.

This means the no-length allocation path identified in the review no longer calls response.text() at all.

Local proof:

node scripts/run-vitest.mjs src/agents/mcp-http-fetch.test.ts --run passed: 12 tests.
git diff --check passed.
Real loopback proof through production buildMcpHttpFetch: a local http.createServer returned a 1 MiB+ body, and the test fetch adapter converted the real HTTP response into the MCP bodyless foreign-response shape. Both declared-oversized and no-length cases returned body=null with textCalls=0:

{"declared":{"status":400,"body":null},"unbounded":{"status":400,"body":null},"textCalls":0,"declaredTextCalls":0}

clawsweeper · 2026-06-30T14:17:47Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Pick-cat · 2026-06-30T14:20:00Z

@clawsweeper re-review

Updated the PR body with the latest loopback proof and the explicit bodyless foreign-response policy. The body now includes the real http.createServer proof output showing both declared-oversized and no-length bodyless responses return body=null with textCalls=0.

Pick-cat · 2026-06-30T14:54:34Z

@clawsweeper re-review

Updated the branch and PR body with declared-small/returned-large coverage. The focused test now covers a bodyless foreign response that declares Content-Length: 64 but returns >1 MiB from text(); the wrapper drops the body after the post-read UTF-8 byte check. The PR body also explicitly states the remaining policy tradeoff from #97521.

clawsweeper · 2026-06-30T14:54:37Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Pick-cat · 2026-07-05T14:42:15Z

@clawsweeper re-review
Rebased onto latest main, conflicts resolved, tests pass, lint clean.

clawsweeper · 2026-07-05T14:42:18Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

steipete · 2026-07-05T23:20:36Z

Land-ready at exact head 8240e5782a23296014d91bea7700774ca403af34.

Deep review replaced the original Content-Length precheck: that header cannot bound memory after another fetch implementation has already buffered or decompressed a response. The final fix handles only foreign response-like objects that expose neither text() nor arrayBuffer(), rebuilding an empty standards Response with the original status and headers; native/streamed responses pass through unchanged.

Proof:

inspected @modelcontextprotocol/sdk directly: its OAuth parser awaits input.text();
Blacksmith Testbox: 13 focused cases passed, including absent/plausible/oversized/lying length headers and the real SDK parse path;
remote formatting passed;
fresh rebased-tree autoreview: clean, confidence 0.96;
exact-head hosted CI: green;
native review artifacts: validated, zero findings.

The body-less malformed path intentionally yields the original HTTP 400 diagnostic without an eager body read.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Co-authored-by: Pick-cat <huang.ting3@xydigit.com>

steipete · 2026-07-05T23:47:44Z

Merged via squash.

Prepared head SHA: 8240e5782a23296014d91bea7700774ca403af34
Landed commit: f13ce2db96cae1cd34714a28ec4e710d652a5457

* fix(agents): bound body-less MCP HTTP text responses * fix(agents): fail closed on unbounded MCP text fallback * test(agents): cover lying MCP bodyless content length * fix(agents): reject malformed MCP content length * chore: drop unused FinalizationRegistry after rebase Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(mcp): consolidate bounded response cases Co-authored-by: Pick-cat <huang.ting3@xydigit.com> * fix(mcp): reject unbounded bodyless responses Co-authored-by: Pick-cat <huang.ting3@xydigit.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>

* fix(agents): guard null response body in proxy stream reader The proxy transport used response.body! (non-null assertion) to enter the SSE read loop without checking whether the body is present. response.ok does not guarantee response.body is non-null — empty 200 responses and prematurely-closed connections leave body=null while ok=true. Sibling transports in the same repository already guard this case: - provider-transport-fetch.ts:148,385,541 - anthropic-transport-stream.ts:847 - openai-completions-transport.ts:272 - mcp-http-fetch.ts:60,76 (added in #98143) Add the same guard before getReader() so the caller receives a business- level diagnostic ("Proxy returned an empty response body") instead of the misleading JS runtime error "Cannot read properties of null (reading 'getReader')". The try-catch in streamProxy already catches the error and pushes it as a stream event, so this does not introduce a new crash — it replaces an internal TypeError with a meaningful error. * test(agents): cover null response body in proxy stream reader

…w#107325) * fix(agents): guard null response body in proxy stream reader The proxy transport used response.body! (non-null assertion) to enter the SSE read loop without checking whether the body is present. response.ok does not guarantee response.body is non-null — empty 200 responses and prematurely-closed connections leave body=null while ok=true. Sibling transports in the same repository already guard this case: - provider-transport-fetch.ts:148,385,541 - anthropic-transport-stream.ts:847 - openai-completions-transport.ts:272 - mcp-http-fetch.ts:60,76 (added in openclaw#98143) Add the same guard before getReader() so the caller receives a business- level diagnostic ("Proxy returned an empty response body") instead of the misleading JS runtime error "Cannot read properties of null (reading 'getReader')". The try-catch in streamProxy already catches the error and pushes it as a stream event, so this does not introduce a new crash — it replaces an internal TypeError with a meaningful error. * test(agents): cover null response body in proxy stream reader

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS labels Jun 30, 2026

openclaw-barnacle Bot added size: S and removed size: XS labels Jun 30, 2026

steipete self-assigned this Jul 5, 2026

steipete force-pushed the fix/mcp-http-fetch-bound-text-read branch from d80ac22 to ce41bf0 Compare July 5, 2026 22:56

steipete force-pushed the fix/mcp-http-fetch-bound-text-read branch from ce41bf0 to e3ac680 Compare July 5, 2026 23:21

Pick-cat and others added 7 commits July 6, 2026 00:35

fix(agents): bound body-less MCP HTTP text responses

dd8f780

fix(agents): fail closed on unbounded MCP text fallback

12ecb60

test(agents): cover lying MCP bodyless content length

c85c561

fix(agents): reject malformed MCP content length

e97aa9c

chore: drop unused FinalizationRegistry after rebase

38a9159

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

test(mcp): consolidate bounded response cases

f0c0fe0

Co-authored-by: Pick-cat <huang.ting3@xydigit.com>

fix(mcp): reject unbounded bodyless responses

8240e57

Co-authored-by: Pick-cat <huang.ting3@xydigit.com>

steipete force-pushed the fix/mcp-http-fetch-bound-text-read branch from e3ac680 to 8240e57 Compare July 5, 2026 23:35

steipete merged commit f13ce2d into openclaw:main Jul 5, 2026
132 of 144 checks passed

maweibin mentioned this pull request Jul 14, 2026

fix(agents): guard null response body in proxy stream reader #107325

Merged

clawsweeper Bot mentioned this pull request Jul 17, 2026

Bound HTTP error body snippets to readable streams #109245

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(agents): bound body-less MCP HTTP text responses#98143

fix(agents): bound body-less MCP HTTP text responses#98143
steipete merged 7 commits into
openclaw:mainfrom
Pick-cat:fix/mcp-http-fetch-bound-text-read

Pick-cat commented Jun 30, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

Pick-cat commented Jun 30, 2026

Uh oh!

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

Pick-cat commented Jun 30, 2026

Uh oh!

Pick-cat commented Jun 30, 2026

Uh oh!

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

Pick-cat commented Jul 5, 2026

Uh oh!

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

Pick-cat commented Jun 30, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pick-cat commented Jun 30, 2026

Uh oh!

clawsweeper Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pick-cat commented Jun 30, 2026

Uh oh!

Pick-cat commented Jun 30, 2026

Uh oh!

clawsweeper Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pick-cat commented Jul 5, 2026

Uh oh!

clawsweeper Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Pick-cat commented Jun 30, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

clawsweeper Bot commented Jul 5, 2026 •

edited

Loading

steipete commented Jul 5, 2026 •

edited

Loading