fix(ibac): Bypass MCP session-terminate DELETE as transport_stream

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 {

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,