From e5208d8c68bd018042f8e30ea4e2ae7a2866db22 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Sat, 30 May 2026 07:57:20 -0400 Subject: [PATCH 1/3] fix(ibac): Bypass MCP session-terminate DELETE as transport_stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After PR #450 landed the body-less GET/HEAD/OPTIONS bypass, one class of MCP traffic still reaches the judge and gets denied: the MCP Streamable HTTP session-termination signal — DELETE to the MCP endpoint with the Mcp-Session-Id header. The MCP client SDK fires this at end-of-conversation to release server-side session state. Symptom seen on a real agent (exgentic-a2a-tool-calling-gsm8k): after the user's task completes successfully, the agent issues DELETE /mcp with Mcp-Session-Id; the judge sees the bare request line (no body) and reasonably answers something like "Action involves deleting data, which is not strictly necessary for user intent." 403 on routine protocol cleanup. Extend the transport_stream bypass to cover this shape. The Mcp-Session-Id header is set by the client SDK (never user input), so it's a precise distinguisher between transport cleanup and a real "DELETE /api/users/42" action call — the latter has no MCP session header and still reaches the judge. Refactor: replace the single-condition isTransportRetrieval check at step 5b with a wider isTransportShaped helper that wraps both patterns. Keeps the existing isTransportRetrieval(method) helper for testability and readability. Tests: - TestOnRequest_TransportStream_MCPSessionTerminate (positive: body-less DELETE + Mcp-Session-Id → skip/transport_stream) - TestOnRequest_TransportStream_BodylessDELETEWithoutHeaderIsJudged (negative: body-less DELETE without the header → judged, the header is the load-bearing distinguisher) - Existing TestOnRequest_TransportStreamBypass_BodylessPOSTIsJudged still validates body-less DELETE without Mcp-Session-Id reaches the judge — comment cross-references the new test. Threat model unchanged: an attacker can't smuggle a payload through a body-less request, and the Mcp-Session-Id header is set by the client SDK rather than user input, so the bypass condition isn't attacker-controllable in any way that bypasses other action shapes. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/plugins/ibac/plugin.go | 68 +++++++++++++++---- .../authlib/plugins/ibac/plugin_test.go | 65 +++++++++++++++++- 2 files changed, 118 insertions(+), 15 deletions(-) diff --git a/authbridge/authlib/plugins/ibac/plugin.go b/authbridge/authlib/plugins/ibac/plugin.go index a3c6236ca..70509cd9e 100644 --- a/authbridge/authlib/plugins/ibac/plugin.go +++ b/authbridge/authlib/plugins/ibac/plugin.go @@ -303,20 +303,33 @@ func (p *IBAC) OnRequest(ctx context.Context, pctx *pipeline.Context) pipeline.A return pipeline.Action{Type: pipeline.Continue} } - // 5b. Transport-stream bypass. Body-less retrieval-shaped requests - // carry no action payload to evaluate — typical patterns: MCP - // Streamable HTTP's server→client SSE channel (GET), agent- - // card fetches (GET), OAuth metadata probes (GET), CORS - // preflights (OPTIONS), HEAD probes for cache validation / - // existence checks. Sending these to the judge is a category - // error: there's nothing to judge, and the LLM either denies - // for lack of context or returns a non-deterministic verdict - // that depends on noise in the prompt. + // 5b. Transport-stream bypass. Two patterns of requests carry no + // user-meaningful action and must skip the judge: // - // Threat model: an attacker can't smuggle a payload through a - // body-less GET/HEAD/OPTIONS — there's no request body to put - // the action in. Side-effect HTTP methods (POST/PUT/DELETE/ - // PATCH) always reach the judge, body-having or not. + // - Body-less GET/HEAD/OPTIONS: retrieval-shaped calls + // (MCP Streamable HTTP server→client SSE channel-open, + // agent-card fetches, OAuth metadata probes, CORS + // preflights, HEAD existence/cache-validation probes). + // + // - Body-less DELETE with the Mcp-Session-Id header: MCP + // Streamable HTTP session termination, sent by the MCP + // client SDK at end-of-conversation to release server- + // side session state. The header is set by the SDK, not + // user input, so it's a precise distinguisher from a + // "DELETE /api/users/42" real-action call. + // + // Sending either to the judge is a category error: there's + // nothing to judge, and the LLM either denies for lack of + // context or misinterprets the verb (e.g. correctly noting + // "DELETE involves deleting data" without knowing it's + // protocol session cleanup). + // + // Threat model: an attacker can't smuggle a payload through + // a body-less request — there's no body to put it in. Side- + // effect HTTP methods (POST/PUT/DELETE/PATCH) always reach + // the judge when they carry a body. Body-less DELETE without + // the Mcp-Session-Id header — a real "delete this resource" + // call by URL path — also reaches the judge. // // Caveat: servers that handle side-effect operations through // GET query strings (e.g. ?action=delete&id=42) violate REST @@ -325,7 +338,7 @@ func (p *IBAC) OnRequest(ctx context.Context, pctx *pipeline.Context) pipeline.A // plugin trusts HTTP method semantics as a proxy for "is this // an action?", and a server that breaks that convention is // making its own authorization promises that IBAC can't see. - if isTransportRetrieval(pctx.Method) && len(pctx.Body) == 0 { + if isTransportShaped(pctx) { pctx.Skip("transport_stream") return pipeline.Action{Type: pipeline.Continue} } @@ -614,6 +627,33 @@ func isTransportRetrieval(method string) bool { return false } +// isTransportShaped reports whether a request matches a transport- +// layer pattern that carries no user-meaningful action — should be +// skipped before the intent check. Combines two body-less shapes: +// +// - retrieval (GET / HEAD / OPTIONS): see isTransportRetrieval. +// - MCP Streamable HTTP session termination: DELETE with the +// Mcp-Session-Id header. The MCP spec defines this as the way +// a client releases server-side session state at end-of- +// conversation; the header is set by the client SDK, not user +// input, so it's a precise distinguisher from a real +// "DELETE /api/resource" action call. +// +// A request with a non-empty body is always treated as an action, +// regardless of method. +func isTransportShaped(pctx *pipeline.Context) bool { + if len(pctx.Body) > 0 { + return false + } + if isTransportRetrieval(pctx.Method) { + return true + } + if pctx.Method == "DELETE" && pctx.Headers.Get("Mcp-Session-Id") != "" { + return true + } + return false +} + // extractMCPToolName pulls the tool name from a tools/call request's // params. Returns "" for non-tools/call methods or missing field. func extractMCPToolName(mcp *pipeline.MCPExtension) string { diff --git a/authbridge/authlib/plugins/ibac/plugin_test.go b/authbridge/authlib/plugins/ibac/plugin_test.go index 0f429f698..c071ae738 100644 --- a/authbridge/authlib/plugins/ibac/plugin_test.go +++ b/authbridge/authlib/plugins/ibac/plugin_test.go @@ -540,7 +540,9 @@ func TestOnRequest_TransportStreamBypass_GETWithBodyIsJudged(t *testing.T) { // Body-less POST/PUT/DELETE/PATCH must NOT bypass — the absence of // body alone isn't enough; the method must signal "retrieval" too. // A body-less POST is unusual but legal (semantic "do action") and -// IBAC should still judge it. +// IBAC should still judge it. (Body-less DELETE *with* the +// Mcp-Session-Id header is its own special case — see +// TestOnRequest_TransportStream_MCPSessionTerminate.) func TestOnRequest_TransportStreamBypass_BodylessPOSTIsJudged(t *testing.T) { for _, method := range []string{"POST", "PUT", "DELETE", "PATCH"} { t.Run(method, func(t *testing.T) { @@ -561,6 +563,67 @@ func TestOnRequest_TransportStreamBypass_BodylessPOSTIsJudged(t *testing.T) { } } +// MCP Streamable HTTP session termination: DELETE to the MCP +// endpoint with the Mcp-Session-Id header. Per spec, MCP clients +// send this at end-of-conversation to release server-side session +// state. It carries no payload and no user-actionable intent — +// must bypass the judge with reason "transport_stream". +// +// Without this bypass, the judge sees "DELETE http://...mcp" with +// an empty body and reasonably (but wrongly) responds along the +// lines of "Action involves deleting data, which is not strictly +// necessary for user intent" — a 403 on routine protocol cleanup +// at end-of-conversation. +func TestOnRequest_TransportStream_MCPSessionTerminate(t *testing.T) { + fj := &fakeJudge{} + p := newConfiguredIBAC(t, fj) + + pctx := makePCtx(t) + pctx.Method = "DELETE" + pctx.Host = "exgentic-mcp-gsm8k-mcp:8000" + pctx.Path = "/mcp" + pctx.Body = nil + pctx.Headers.Set("Mcp-Session-Id", "abc-123") + action := invokeOnRequest(p, pctx) + + if action.Type != pipeline.Continue { + t.Errorf("got %v, want Continue for MCP session terminate", action.Type) + } + if fj.calls != 0 { + t.Errorf("judge calls = %d, want 0 (session terminate must not reach judge)", fj.calls) + } + inv := lastInvocation(t, pctx) + if inv.Action != pipeline.ActionSkip { + t.Errorf("Invocation action = %v, want ActionSkip", inv.Action) + } + if inv.Reason != "transport_stream" { + t.Errorf("Invocation reason = %q, want 'transport_stream'", inv.Reason) + } +} + +// Body-less DELETE WITHOUT the Mcp-Session-Id header must still be +// judged. A real "delete this resource" call (e.g. DELETE /api/ +// users/42) carries no MCP session header and is exactly the kind +// of side-effect action IBAC exists to evaluate. The header is the +// load-bearing distinguisher between transport cleanup and a user- +// meaningful delete. +func TestOnRequest_TransportStream_BodylessDELETEWithoutHeaderIsJudged(t *testing.T) { + fj := &fakeJudge{verdict: "allow"} + p := newConfiguredIBAC(t, fj) + + pctx := makePCtx(t) + pctx.Method = "DELETE" + pctx.Host = "api.example.com" + pctx.Path = "/api/users/42" + pctx.Body = nil + // no Mcp-Session-Id header + _ = invokeOnRequest(p, pctx) + + if fj.calls != 1 { + t.Errorf("judge calls = %d, want 1 (real DELETE without MCP header must be judged)", fj.calls) + } +} + // describeAction's first non-empty token must be the HTTP method, with // no leading whitespace. This locks in the listener-side pctx.Method // wiring contract: if any listener regresses to leaving Method empty, From 3336818d991655899e2c3bc21b4913b5bc378a93 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Sat, 30 May 2026 08:02:48 -0400 Subject: [PATCH 2/3] test(ibac): Lock in body-first guard for MCP session terminate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer flagged that the test suite covers DELETE+Mcp-Session-Id without a body (bypass) and DELETE without the header (judge), but not DELETE+Mcp-Session-Id WITH a body — the third corner that the body-first guard in isTransportShaped quietly handles. Mirrors how GET-with-body has its own case alongside body-less GET. Add TestOnRequest_TransportStream_MCPSessionTerminate_WithBodyIsJudged asserting the invariant: even with the right header, a non-empty body routes the request to the judge. Pure test addition; no production-code change. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- .../authlib/plugins/ibac/plugin_test.go | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/authbridge/authlib/plugins/ibac/plugin_test.go b/authbridge/authlib/plugins/ibac/plugin_test.go index c071ae738..eb4a48e1a 100644 --- a/authbridge/authlib/plugins/ibac/plugin_test.go +++ b/authbridge/authlib/plugins/ibac/plugin_test.go @@ -624,6 +624,29 @@ func TestOnRequest_TransportStream_BodylessDELETEWithoutHeaderIsJudged(t *testin } } +// DELETE + Mcp-Session-Id header + non-empty body must still be +// judged. The header alone isn't sufficient — the body-first guard +// in isTransportShaped is what keeps an attacker (or a misbehaving +// SDK) from smuggling action payload through what looks like +// transport cleanup. Locks in the same "header + body → not +// bypassed" invariant that body-having GETs already cover. +func TestOnRequest_TransportStream_MCPSessionTerminate_WithBodyIsJudged(t *testing.T) { + fj := &fakeJudge{verdict: "allow"} + p := newConfiguredIBAC(t, fj) + + pctx := makePCtx(t) + pctx.Method = "DELETE" + pctx.Host = "exgentic-mcp-gsm8k-mcp:8000" + pctx.Path = "/mcp" + pctx.Body = []byte(`{"unexpected":"payload"}`) + pctx.Headers.Set("Mcp-Session-Id", "abc-123") + _ = invokeOnRequest(p, pctx) + + if fj.calls != 1 { + t.Errorf("judge calls = %d, want 1 (DELETE+Mcp-Session-Id with body must be judged, not bypassed)", fj.calls) + } +} + // describeAction's first non-empty token must be the HTTP method, with // no leading whitespace. This locks in the listener-side pctx.Method // wiring contract: if any listener regresses to leaving Method empty, From 4255dd53d31e7af1d64ef68dfbc4a878fa73686a Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Sat, 30 May 2026 08:11:53 -0400 Subject: [PATCH 3/3] fix(ibac): Bypass resources/subscribe + resources/unsubscribe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Auditing MCP protocol coverage of the housekeeping bypass turned up two methods that fit the same shape as the existing list (initialize, *list, etc.) but go through a body-having JSON-RPC POST so the new transport_stream bypass doesn't apply: - resources/subscribe: client tells the server to push notifications when a resource changes - resources/unsubscribe: pair to the above; release the subscription Both are subscription-state bookkeeping — protocol mechanics, not user-meaningful actions. An agent that maintains resource subscriptions mid-conversation would see these calls hit the judge today; the judge might happen to allow them since they don't look like exfiltration, but it's wasted judge calls and a missed bypass shape. Add both to the isMCPHousekeeping switch list and extend the existing TestOnRequest_MCPHousekeepingBypass table to cover them. Doc-comment on the helper expanded to mention subscription management alongside connection setup and capability discovery. Side-effect MCP methods (tools/call, prompts/get, resources/read) still reach the judge — those are exactly what IBAC exists for. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/plugins/ibac/plugin.go | 24 +++++++++++++------ .../authlib/plugins/ibac/plugin_test.go | 10 +++++--- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/authbridge/authlib/plugins/ibac/plugin.go b/authbridge/authlib/plugins/ibac/plugin.go index 70509cd9e..8ae6c951f 100644 --- a/authbridge/authlib/plugins/ibac/plugin.go +++ b/authbridge/authlib/plugins/ibac/plugin.go @@ -585,13 +585,21 @@ func formatBodyExcerpt(body []byte, n int) string { return fmt.Sprintf("%q", string(body)) } -// isMCPHousekeeping reports whether an MCP method is connection-setup -// or capability-discovery traffic that carries no user-actionable -// intent. These methods fire before any user turn (e.g. an agent's -// startup `initialize` against its MCP tool server) and judging them -// would either panic on a nil session or deny on no_intent — both -// break agents that open MCP connections at startup. Only side-effect -// methods (tools/call, prompts/get, resources/read) reach the judge. +// isMCPHousekeeping reports whether an MCP method is connection-setup, +// capability-discovery, or subscription-management traffic that +// carries no user-actionable intent. These methods fire before, after, +// or alongside user turns as protocol mechanics — judging them would +// either panic on a nil session or deny on no_intent, breaking agents +// that open MCP connections at startup or maintain resource +// subscriptions. Only side-effect methods (tools/call, prompts/get, +// resources/read) reach the judge. +// +// resources/subscribe and resources/unsubscribe are subscription- +// state management — the client tells the server "notify me when +// this resource changes" / "stop notifying me." They go through POST +// with a JSON-RPC body (so the body-less transport_stream bypass +// doesn't apply), but they're conceptually identical to *list calls: +// protocol bookkeeping, not user-meaningful actions. // // JSON-RPC notifications (any method starting with `notifications/`) // are also bypassed: they're one-way protocol signals, never tied to @@ -604,6 +612,8 @@ func isMCPHousekeeping(method string) bool { "prompts/list", "resources/list", "resources/templates/list", + "resources/subscribe", + "resources/unsubscribe", "completion/complete", "logging/setLevel": return true diff --git a/authbridge/authlib/plugins/ibac/plugin_test.go b/authbridge/authlib/plugins/ibac/plugin_test.go index eb4a48e1a..516b65d2a 100644 --- a/authbridge/authlib/plugins/ibac/plugin_test.go +++ b/authbridge/authlib/plugins/ibac/plugin_test.go @@ -420,9 +420,11 @@ func TestOnRequest_NilSession_PolicyAllow_Bypasses(t *testing.T) { } // MCP protocol-housekeeping methods carry no user-actionable intent -// (they're connection setup or capability discovery, not side -// effects). IBAC must skip them; otherwise every agent that opens an -// MCP connection at startup gets blocked before any user turn. +// (they're connection setup, capability discovery, or subscription +// management — not side effects). IBAC must skip them; otherwise +// every agent that opens an MCP connection at startup gets blocked +// before any user turn, and any agent that maintains resource +// subscriptions sees its bookkeeping calls denied mid-conversation. func TestOnRequest_MCPHousekeepingBypass(t *testing.T) { cases := []string{ "initialize", @@ -431,6 +433,8 @@ func TestOnRequest_MCPHousekeepingBypass(t *testing.T) { "prompts/list", "resources/list", "resources/templates/list", + "resources/subscribe", + "resources/unsubscribe", "completion/complete", "logging/setLevel", "notifications/initialized",