Skip to content

Reject JSON-RPC batches to close authz blind spot#5931

Merged
JAORMX merged 1 commit into
mainfrom
reject-jsonrpc-batches
Jul 23, 2026
Merged

Reject JSON-RPC batches to close authz blind spot#5931
JAORMX merged 1 commit into
mainfrom
reject-jsonrpc-batches

Conversation

@JAORMX

@JAORMX JAORMX commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

JSON-RPC batch requests (a top-level JSON array [{...},{...}]) were executed by the streamable HTTP proxy and forwarded raw by the transparent proxy, but the MCP parser middleware could not decode them — so authz (Cedar), audit, and tool filtering, which each inspect a single parsed request, never saw the nested calls. A batch could smuggle tool calls past Cedar policies, tool filtering, and audit logging: a request-smuggling / authz-audit blind spot.

Batching was removed from MCP in the 2025-06-18 revision. ToolHive serves only 2025-11-25 and 2026-07-28, so a batch can only originate from an unsupported pre-2025-06-18 client. Rather than teach every middleware to walk batch entries, this rejects batches outright (spec-conformant for the versions we serve; JSON-RPC -32600 "Invalid Request", id: null, HTTP 400).

  • Add mcp.IsBatchRequest (single shared detector, bytes.TrimSpace — matches encoding/json), BatchUnsupportedError (a CodedError), and the two JSON-RPC error writers WriteBatchUnsupportedError (ResponseWriter) / BatchUnsupportedResponse (RoundTripper), backed by one shared error-body/response builder.
  • Reject batches in three places so no path can slip through: ParsingMiddleware, the streamable proxy handlePost, and the transparent proxy RoundTrip. Both proxy checks run before session resolution/classification and independent of Content-Type, so a batch cannot reach a backend uninspected even under a non-JSON content type or if the parser middleware were absent/reordered.
  • Delete the now-unreachable streamable batch executor (handleBatchRequest, processSingleMessage, decodeBatch, resolveSessionForBatch, and the orphaned waitForResponse).

Fixes #5745

Type of change

  • Bug fix

Test plan

  • Unit tests (task test) — all affected packages pass (pkg/mcp, pkg/transport/proxy/streamable, pkg/transport/proxy/transparent, pkg/authz, pkg/audit, pkg/telemetry, pkg/vmcp, pkg/runner). The only failures in the full suite are pre-existing environmental ones in this sandbox (no container runtime, no kernel keyring) and are unrelated to this change.
  • Linting (task lint-fix) — 0 issues.
  • New regression tests assert rejection (400 / -32600 / id:null, backend never contacted) across both proxies, including the non-JSON content-type and leading-whitespace evasion cases; obsolete tests that asserted batch execution/forwarding were updated or removed.

Does this introduce a user-facing change?

Yes. A client that POSTs a JSON-RPC batch now receives an HTTP 400 with a JSON-RPC -32600 "Invalid Request" error instead of having the batch executed/forwarded. This only affects pre-2025-06-18 clients (the only revision where batching was valid); ToolHive serves 2025-11-25 and 2026-07-28, where batching does not exist.

Special notes for reviewers

  • Reviewed by a security + MCP-spec + Go-conventions panel. The security pass initially found that rejecting only in the parser was insufficient (the streamable executor and transparent forwarder run independent of Content-Type); the fix now rejects at both proxy executors. The MCP-spec pass confirmed unconditional rejection with -32600 / id:null / HTTP 400 is correct for the revisions ToolHive serves.
  • Batch rejection sits after the body-size limit read in both proxies, so it does not open an unbounded-read DoS.
  • Minor, non-blocking follow-up: parseRPCRequest's batch-scanning branch in the transparent proxy is now dead for the RoundTrip path and could be simplified in a later cleanup (harmless defense-in-depth left in place).

Generated with Claude Code

JSON-RPC batch requests (a top-level array) were executed by the
streamable HTTP proxy and forwarded by the transparent proxy, but the
parser middleware could not decode them, so authz, audit, and tool
filtering — which each inspect a single parsed request — never saw the
nested calls. This let a batch smuggle tool calls past Cedar policies,
tool filtering, and audit logging.

Batching was removed from MCP in the 2025-06-18 revision; ToolHive
serves only 2025-11-25 and 2026-07-28, so a batch can only come from an
unsupported client. Reject batches outright instead of executing them:

- Add mcp.IsBatchRequest plus BatchUnsupportedError and the JSON-RPC
  writers (WriteBatchUnsupportedError / BatchUnsupportedResponse),
  sharing one detector (bytes.TrimSpace) and one error-body builder.
- Reject in ParsingMiddleware, in the streamable proxy handlePost, and
  in the transparent proxy RoundTrip — the two proxy checks run before
  session resolution and independent of Content-Type, so a batch can
  never reach a backend uninspected regardless of middleware presence.
- Delete the now-unreachable streamable batch executor and its helpers.

Fixes #5745

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WG3mSjVGWNc8nfgbkdd79
@github-actions github-actions Bot added the size/L Large PR: 600-999 lines changed label Jul 23, 2026

@jhrozek jhrozek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the whole change - reads well and closes the batch smuggling gap cleanly.

The defense-in-depth is the right call: rejecting at the streamable executor and the transparent proxy's RoundTrip (not just ParsingMiddleware) makes rejection independent of middleware ordering and Content-Type, so a batch under a non-JSON content type can't slip past authz/audit/tool-filtering. Deleting the batch execution path, including the sessionless-token logic in resolveSessionForBatch, is a nice simplification on top. Tests cover the vectors that actually matter (non-JSON content type, leading vertical tab, stale session, empty batch).

One small thing, non-blocking: the IsBatchRequest doc comment (and a couple of test comments) say bytes.TrimSpace trims "the same Unicode whitespace as encoding/json" and that json ignores vertical tab / form feed. That's not accurate - encoding/json errors on \v and \f, it only skips space/tab/newline/CR. The behavior is still correct and actually safer than the comment implies: TrimSpace strips a superset of JSON whitespace, so detection can never under-trim relative to the decoder and can't miss a batch it would parse as an array. Worth rewording the comment to that stronger guarantee at some point.

Also FYI the telemetry middleware's if parsedMCP.IsBatch branch is now dead since IsBatch is always false - fine to leave given the field is kept for compatibility.

@JAORMX
JAORMX merged commit a18f17e into main Jul 23, 2026
26 of 28 checks passed
@JAORMX
JAORMX deleted the reject-jsonrpc-batches branch July 23, 2026 08:36
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.72%. Comparing base (e8d243d) to head (16e56bd).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5931      +/-   ##
==========================================
+ Coverage   71.70%   71.72%   +0.02%     
==========================================
  Files         701      702       +1     
  Lines       71980    71901      -79     
==========================================
- Hits        51612    51573      -39     
+ Misses      16673    16646      -27     
+ Partials     3695     3682      -13     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

JAORMX added a commit that referenced this pull request Jul 23, 2026
PR #5931 made the streamable-HTTP proxy reject JSON-RPC batches with
HTTP 400 / -32600 to close an authz/audit blind spot, and updated every
unit and integration test accordingly. This e2e test was missed: it
still POSTed a batch and asserted HTTP 200 with two responses, so the
"E2E Tests Core (proxy)" shard failed deterministically on every open
PR regardless of its content.

Assert the batch is rejected (400 / -32600 / id:null), matching the
merged proxy behavior. Batching was removed in MCP revision 2025-06-18;
ToolHive serves only 2025-11-25 and 2026-07-28, so rejection is
spec-conformant.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large PR: 600-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Close the authz/audit blind spot for batch JSON-RPC requests

2 participants