Fix: scope a2a/mcp parsers to their own JSON-RPC method namespace - #700
Conversation
📝 WalkthroughWalkthroughA2A and MCP parsers now validate JSON-RPC method namespaces before attaching protocol extensions or recording invocation observations. Regression tests cover non-JSON-RPC payloads and cross-namespace methods. ChangesProtocol namespace parser gating
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
5c89d05 to
9a31507
Compare
a2a-parser and mcp-parser both parse JSON-RPC 2.0 request bodies, and
each claimed a request whenever the body carried a non-empty "method".
But A2A and MCP both ride JSON-RPC 2.0 over HTTP, so "method is
non-empty" cannot tell them apart:
- a2a-parser attached an A2AExtension + a "matched_" observe to MCP
calls (initialize, tools/list, notifications/*, ...) and to inference
/v1/messages bodies (which unmarshal into JSONRPCRequest with an
empty method).
- mcp-parser symmetrically claimed A2A traffic (message/send, tasks/*).
In abctl this showed up as a phantom second plugin on every MCP and
inference request (e.g. an inference row labeled "2 plugins" =
inference-parser + a2a-parser).
The method namespace is the only in-body signal that distinguishes the
two protocols, and the namespaces are disjoint:
- A2A: message/*, tasks/*, agent/*
- MCP: initialize, ping, notifications/*, tools/*, resources/*,
prompts/*, completion/*, logging/*, sampling/*, roots/*,
elicitation/*
Gate each parser on its own namespace (isA2AMethod / isMCPMethod) instead
of "non-empty method". Each stays forward-compatible within its own
namespace design, and an empty method (non-JSON-RPC body) now matches
neither. mcp-parser's body-less transport detection ($transport/stream,
$transport/terminate) is unchanged.
IBAC classification interaction: for a JSON-RPC body whose method falls
outside both namespaces, Context.Classification() now reports
"unclassified" (no extension attached) instead of a recorded bypass.
Under the default unclassified_policy (passthrough) this is
observability-only — a skip/protocol_mechanics row is no longer
recorded. Under "judge" such traffic is now judged rather than silently
skipped, which matches that policy's "judge the unknowns" intent (the
old phantom bypass let unknown JSON-RPC skip the judge). The IBAC demo
is unaffected: its exfil body is text/plain and was already declined by
mcp-parser's pre-existing non-JSON-RPC guard.
Add regression tests: each parser declines the other's methods and
non-JSON-RPC bodies without attaching an extension or recording an
invocation, and Classification() reports unclassified in that case.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
9a31507 to
f345284
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@authbridge/authlib/plugins/a2aparser/plugin.go`:
- Around line 61-64: The A2A and MCP parsers accept namespace-matched methods
without validating the JSON-RPC version. In
authbridge/authlib/plugins/a2aparser/plugin.go lines 61-64 and
authbridge/authlib/plugins/mcpparser/plugin.go lines 226-228, require the parsed
jsonrpc discriminator to equal "2.0" before accepting the method; update
authbridge/authlib/plugins/a2aparser/plugin_test.go lines 333-368 and
authbridge/authlib/plugins/mcpparser/plugin_test.go lines 189-230 with
regression cases for method-bearing payloads missing or using an invalid jsonrpc
value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4d6273e0-e6f8-436b-bd21-6427ecb75be1
📒 Files selected for processing (4)
authbridge/authlib/plugins/a2aparser/plugin.goauthbridge/authlib/plugins/a2aparser/plugin_test.goauthbridge/authlib/plugins/mcpparser/plugin.goauthbridge/authlib/plugins/mcpparser/plugin_test.go
| if !isA2AMethod(rpc.Method) { | ||
| slog.Debug("a2a-parser: not an A2A-namespace method, skipping", "method", rpc.Method, "bodyLen", len(pctx.Body)) | ||
| return pipeline.Action{Type: pipeline.Continue} | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect whether JSONRPCRequest validates the JSON-RPC version during unmarshal.
rg -n -A30 -B3 'type JSONRPCRequest struct|UnmarshalJSON' authbridge
# Locate existing JSON-RPC-version checks and parser tests.
rg -n -A6 -B4 '"jsonrpc"|JSONRPC' authbridgeRepository: rossoctl/cortex
Length of output: 50374
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "=== parsercommon.go ==="
cat -n authbridge/authlib/plugins/internal/parsercommon/parsercommon.go | sed -n '1,180p'
echo "=== a2aparser plugin around entrypoint ==="
cat -n authbridge/authlib/plugins/a2aparser/plugin.go | sed -n '1,140p'
echo "=== mcpparser plugin around entrypoint ==="
cat -n authbridge/authlib/plugins/mcpparser/plugin.go | sed -n '200,250p'
echo "=== focused Unmarshal JSONRPC references ==="
rg -n -C 5 'JSONRPCRequest|jsonrpc' authbridge/authlib/plugins/internal/parsercommon authbridge/authlib/plugins/a2aparser authbridge/authlib/plugins/mcpparserRepository: rossoctl/cortex
Length of output: 50371
Validate jsonrpc: "2.0" before accepting namespace-matched methods.
JSONRPCRequest has no custom unmarshaler, so valid JSON like {"method":"message/send","id":1} or {"method":"tools/list","id":1} can currently skip the namespace gate before being accepted. Add a jsonrpc discriminator check in both parsers, and add regression cases for method-bearing non-JSON-RPC payloads to both test files.
📍 Affects 4 files
authbridge/authlib/plugins/a2aparser/plugin.go#L61-L64(this comment)authbridge/authlib/plugins/mcpparser/plugin.go#L226-L228authbridge/authlib/plugins/a2aparser/plugin_test.go#L333-L368authbridge/authlib/plugins/mcpparser/plugin_test.go#L189-L230
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@authbridge/authlib/plugins/a2aparser/plugin.go` around lines 61 - 64, The A2A
and MCP parsers accept namespace-matched methods without validating the JSON-RPC
version. In authbridge/authlib/plugins/a2aparser/plugin.go lines 61-64 and
authbridge/authlib/plugins/mcpparser/plugin.go lines 226-228, require the parsed
jsonrpc discriminator to equal "2.0" before accepting the method; update
authbridge/authlib/plugins/a2aparser/plugin_test.go lines 333-368 and
authbridge/authlib/plugins/mcpparser/plugin_test.go lines 189-230 with
regression cases for method-bearing payloads missing or using an invalid jsonrpc
value.
Source: Coding guidelines
esnible
left a comment
There was a problem hiding this comment.
Clean, tightly-scoped bug fix. a2a-parser and mcp-parser both parse JSON-RPC 2.0 bodies and previously claimed any request with a non-empty method, so each attached a phantom extension to the other's traffic (and to inference bodies). Gating each parser on its own method namespace via isA2AMethod/isMCPMethod is the right fix.
Verified during review:
- The two namespaces are provably disjoint (A2A:
message/,tasks/,agent/; MCP:initialize,ping, and 9 distinct slash-prefixes) — no method is double-claimed, and A2A/MCP have no bare verbs outside these sets. - Both
isA2AAction/isMCPActionsets are strict subsets of their gated namespaces, so no action-classified method is now dropped. - Guard placement is correct: after JSON-RPC parse, before extension attach.
- MCP's body-less transport detection (
len(Body)==0branch) is untouched — the new guard only runs on non-empty bodies. - The subtle bypass -> unclassified behavior change is accurately documented and locked by the new
Classification()assertions.
Regression tests on both parsers are assertive (Continue, nil extension, nil invocations, unclassified). All CI green, DCO signed off, valid Fix: title.
Author: huang195 (MEMBER — maintainer)
Areas reviewed: Go (authlib parser plugins), Tests
Agent/IDE config (.claude/.vscode): none
Commits: 1, all signed-off
CI status: passing
Assisted-By: Claude Code
Summary
a2a-parserandmcp-parserboth parse JSON-RPC 2.0 request bodies, and each claimed a request whenever the body carried a non-emptymethod. But A2A and MCP both ride JSON-RPC 2.0 over HTTP, so "method is non-empty" can't tell them apart — and inference bodies (/v1/messages) unmarshal into aJSONRPCRequestwith an empty method:a2a-parserattached anA2AExtension+matched_observe to MCP calls (initialize,tools/list,notifications/*, …) and to inference bodies.mcp-parsersymmetrically claimed A2A traffic (message/send,tasks/*).In
abctlthis surfaced as a phantom second plugin on every MCP/inference request — e.g. an inference row labeled "2 plugins" (inference-parser+a2a-parser).Change
The method namespace is the only in-body signal that distinguishes the two protocols, and the namespaces are disjoint:
message/*,tasks/*,agent/*initialize,ping,notifications/*,tools/*,resources/*,prompts/*,completion/*,logging/*,sampling/*,roots/*,elicitation/*Each parser now gates on its own namespace (
isA2AMethod/isMCPMethod) instead of "non-empty method":message/*,tasks/*,tools/*, … match with no code change).mcp-parser's body-less transport detection ($transport/stream,$transport/terminate) is unchanged.Regression tests added to both packages: each parser declines the other's methods and non-JSON-RPC bodies without attaching an extension or recording an invocation.
IBAC / Classification interaction (behavior note)
Scoping the parsers changes how one narrow traffic class is classified: a valid JSON-RPC body whose
methodis outside both namespaces. Previously such a body was claimed withIsAction=false, soContext.Classification()reported a bypass (→Skip("protocol_mechanics")). Now no extension is attached, soClassification()reports unclassified (anyAction=false, anyBypass=false).unclassified_policy: passthroughthis is observability-only — the traffic still passes; only theskip/protocol_mechanicsrow is no longer recorded.unclassified_policy: judge(non-default) such traffic is now judged instead of silently skipped. This matches that policy's "judge the unknowns" intent — the old phantom bypass actually let unknown JSON-RPC (and even real A2Amessage/sendin an mcp-only outbound chain) slip past the judge.The IBAC demo is unaffected: its exfil request is
text/plain, already declined bymcp-parser's pre-existing non-JSON-RPC guard (before and after this change), anda2a-parseris inbound-only there. New tests assertClassification()is unclassified after each parser declines a foreign-namespace method.Testing Instructions
Assisted-By: Claude Code
Summary by CodeRabbit
Bug Fixes
Tests