diff --git a/authbridge/authlib/plugins/ibac/plugin.go b/authbridge/authlib/plugins/ibac/plugin.go index a3c6236ca..8ae6c951f 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} } @@ -572,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 @@ -591,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 @@ -614,6 +637,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..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", @@ -540,7 +544,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 +567,90 @@ 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) + } +} + +// 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,