Repository navigation
fix(mcp-broker): refuse a JSON-RPC batch and any message it cannot judge - #478
Merged
Merged
Conversation
…e upstream Four cases for the MCP broker, added before the fix so CI shows them red on the current code: - a JSON-RPC batch of two tools/call over HTTP is refused with 400 and -32600, nothing reaches the upstream and the PDP is never asked, with a policy that denies every tool; - the stdio transport refuses the same batch; - a bare string, a number, null, and an object whose method is not a string are refused the same way; - one request and a client's JSON-RPC response (id and result, no method) still reach the upstream, so the fix cannot refuse too much. Scenarios in features/a-batch-cannot-carry-a-call-past-the-gate.feature, each bound to its test. README badge and PROGRESS.md count 2418. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every check the broker makes keys on `method` read from one JSON object. A batch is an array, so its method read as empty and every tools/call check was skipped: the policy decision, the identity requirement, drift, the taint gate, the argument scans and the tool_call record, and a batched tools/list answer was never scanned. The array was forwarded as it came, over HTTP and stdio alike. process_call, which both transports reach, now refuses first, before an upstream is resolved or a secret read: a message that is not one JSON object, or whose method is present and not a string, gets JSON-RPC -32600 with a null id and HTTP 400. MCP removed batching in its 2025-06-18 revision. A JSON-RPC response (no method) is still carried. Invariant 142 in CLAUDE.md. The previous commit's tests failed on the unfixed broker in CI (run 40258757907: three refused-case tests red at the "nothing reached the upstream" assertion, the must-not-fire case green). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…visories rustls-webpki 0.103.15 -> 0.103.16 (RUSTSEC-2026-0382) and webpki-roots 1.0.8 -> 1.0.11 (RUSTSEC-2026-0381) in both lockfiles, the workspace and crates/cluster. wasmtime 49.0.3 -> 49.0.4 (RUSTSEC-2026-0378, RUSTSEC-2026-0379) in the workspace lockfile only, staying on 49.x; the cranelift and pulley crates move with it from 0.136.3 / 49.0.3 to 0.136.4 / 49.0.4. Lockfile-only: no manifest changes, no new crate. crypto-bigint 0.5.5 (RUSTSEC-2026-0387) is the fifth advisory and needs the p256 0.14 migration in the next commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKcmLyFYX5vcY81ze4tHQ1
…026-0387) p256 0.13 is pulled only by this workspace's own crates (cloud, delegation, dpop, gateway). Moving all six manifest lines to 0.14 brings elliptic-curve 0.14.3, ecdsa 0.17.0, pkcs8 0.11, sec1 0.8, signature 3.0 and rfc6979 0.6, and replaces crypto-bigint 0.5.5 with 0.7.9, so `cargo tree -i crypto-bigint --all-features --target all` no longer shows a 0.5.x. The code changes are two renames, nothing else: - `VerifyingKey::to_encoded_point(compress)` is `to_sec1_point(compress)` in ecdsa 0.17; `x()`, `y()` and `as_bytes()` on the result are unchanged. - `SigningKey::random(&mut OsRng)` is deprecated, and rand_core is 0.10 with no `OsRng`; the test fixtures that made a random key now call `<SigningKey as p256::elliptic_curve::Generate>::generate()`. Production key generation (gateway checkpoints) already drew bytes with getrandom 0.2 and `SigningKey::from_slice`, which is unchanged. `Signature::from_slice`, `to_bytes`, `VerifyingKey::from_sec1_bytes`, `SecretKey::from_pkcs8_pem` / `from_sec1_pem` / `from_pkcs8_der` / `from_sec1_der` keep their names and signatures. P-256 keeps `NORMALIZE_S = false`, so a high-S signature still verifies and signing still does not normalise s, as in 0.13. One rustfmt collapse in an audit_sign test follows the shorter name; the assertion is the same. The gateway manifest comment on hmac is corrected: hmac 0.12 reached the lockfile through p256 0.13's rfc6979, and since 0.14 (which uses hmac 0.13) the gateway's own line is its only edge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKcmLyFYX5vcY81ze4tHQ1
The external P-256 values the suite already checked were three keyryx thumbprints (measured from keyryx's Go code) and agent-passport's checkpoint vectors, which it verifies but does not produce. Nothing held a signature this code produces, a SEC1 public key, or a PKCS#8 / SEC1 private key parse to an external value, so a curve library that changed one of those could pass the suite as long as it agreed with itself. Three tests now hold the bytes to RFC 6979 appendix A.2.5 (a P-256 key, its point, its deterministic SHA-256 signatures), with the base64 forms, the RFC 7638 thumbprint and the PKCS#8 / SEC1 encodings written out by Python `cryptography` 50.0.1, never by p256: - gateway::checkpoints: the checkpoint key's JWK x and y, its kid, and the raw r||s over "sample" and "test". - cloud::devices: the RFC signature verifies under the uncompressed and compressed SEC1 key; with its halves swapped, or in DER, it is refused. - cloud::audit_sign: the RFC key loads from PKCS#8 PEM, SEC1 PEM, PKCS#8 DER, SEC1 DER and a raw scalar, and each signs the same empty-chain manifest to the same public key and signature bytes. Each went red against a deliberately broken encoding first (padded standard base64 for x and y; s before r; a DER-only signature parser; a compressed public key and a DER signature in the manifest) and green on the real code. All three also pass unchanged on main with p256 0.13.2, so 0.13 and 0.14 put the same bytes on the wire. Encoding the manifest's public key compressed passed every older test; only the new one caught it. Scenarios: features/the-p256-wire-format-is-unchanged.feature, five, each bound. The suite goes from 2414 to 2417; README.md and PROGRESS.md say so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKcmLyFYX5vcY81ze4tHQ1
`secretscan / tree` flagged the PKCS#8 header of the RFC 6979 appendix A.2.5 P-256 key that the audit-signing-key encoding test loads (crates/cloud/src/audit_sign.rs, fingerprint 3021d90eb9437b2d, the same header fingerprint already allowed for the test keys in crates/cloud/tests/it/oidc.rs and crates/dpop/src/lib.rs). The key is a published test vector and signs nothing. The SEC1 header in the same file was already covered by its existing entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKcmLyFYX5vcY81ze4tHQ1
…cp-broker-refuses-batches # Conflicts: # PROGRESS.md # README.md
TAIPANBOX
marked this pull request as ready for review
October 26, 2026 04:07
TAIPANBOX
enabled auto-merge
October 26, 2026 04:07
Owner
Author
Mutation runRun 40266586803 on the throwaway draft #479, at 81753da of this branch: one CI job per mutant, each planted in Baseline (no mutant): 4 of 4 passed.
Seven of seven killed; none survived. Red first: run 40258757907 on b2fc69a (tests only) failed the three refused-case tests at the "nothing reached the upstream" assertion, with the must-not-fire test green. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The MCP broker judges a call by reading
methodfrom one JSON-RPC object. A JSON-RPC batch is a JSON array, so it has nomethodof its own: every check keyed ontools/callwas skipped and the array was forwarded as it came. That skipped the policy decision, the identity check, the drift refusal, the taint gate, the raw-secret scan of the arguments and thetool_callrecord, and a batchedtools/listanswer was never scanned for poisoned descriptions. The HTTP and stdio transports both reachprocess_call, so both were affected.Fix
process_callrefuses, before anything is resolved or forwarded, a message that is not one JSON object, or whosemethodis present and not a string. The answer is JSON-RPC Invalid Request (-32600) with no id, and HTTP 400 on the HTTP transport. MCP removed JSON-RPC batching in its 2025-06-18 revision (spec changelog, PR #416), and Streamable HTTP carries one message per POST. A client's JSON-RPC response (anidand aresult, nomethod) is still forwarded.Recorded as invariant 142 in CLAUDE.md. Scenarios:
features/a-batch-cannot-carry-a-call-past-the-gate.feature, four, each bound to a test.How this was proven
Compatibility
A client that still sends batches (MCP 2025-03-26 allowed them) now gets
-32600and has to send each message on its own.🤖 Generated with Claude Code