fix(ibac): Bypass MCP session-terminate DELETE as transport_stream#451
Conversation
After PR kagenti#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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
📝 WalkthroughWalkthroughThe PR refactors IBAC's transport-stream bypass into ChangesMCP Session Termination Transport-Stream Bypass
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/ibac/plugin.go`:
- Around line 644-655: The DELETE + Mcp-Session-Id header check in
isTransportShaped(pctx *pipeline.Context) uses a sessionful header that was
removed in the sessionless 2026-07-28/SEP-2567 revision; update
isTransportShaped to either remove the pctx.Method == "DELETE" &&
pctx.Headers.Get("Mcp-Session-Id") != "" branch entirely if this service targets
the sessionless spec, or replace it with the correct termination mechanism from
the intended spec (e.g., a different header or transport-revision flag). Locate
the isTransportShaped function and modify the DELETE conditional accordingly so
it no longer relies on the removed Mcp-Session-Id header, and add a comment
referencing the protocol revision used to avoid regressions.
🪄 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: 8844ae8d-2447-42f3-b045-d5d7935acf04
📒 Files selected for processing (2)
authbridge/authlib/plugins/ibac/plugin.goauthbridge/authlib/plugins/ibac/plugin_test.go
| 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 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
MCP Streamable HTTP Mcp-Session-Id header specification
💡 Result:
The Model Context Protocol (MCP) has undergone a significant architectural change regarding session management. As of the current date (May 2026), the Mcp-Session-Id header has been deprecated and removed from the protocol specification [1][2][3]. Historical Context and Specification: Prior to this change (notably in specifications from 2025 and early 2026), the Streamable HTTP transport utilized the Mcp-Session-Id header to maintain session state [2][3][4]. Under that previous model: 1. Initialization: The client performed an initialize request [2][3]. 2. Session Assignment: The server generated a unique session ID and returned it in the Mcp-Session-Id response header [5][6][7]. 3. Session Persistence: Clients were required to include this Mcp-Session-Id header in all subsequent HTTP requests [5][8][4]. The server used this identifier to associate requests with specific session state, capabilities, and negotiated protocol versions [3][7]. Current Status (Sessionless Architecture): The protocol has shifted to a stateless architecture [1][2]. The updated specification removes both the initialize/initialized handshake and the Mcp-Session-Id header entirely [2][3]. Requests are now designed to be independent and self-contained; any state required by the server must be managed explicitly by the client and included within request parameters (e.g., using explicit state handles) [1][3]. Consequently, any existing reliance on Mcp-Session-Id for routing or stateful behavior is no longer supported in the current MCP specification [2][3].
Citations:
- 1: https://modelcontextprotocol.io/seps/2567-sessionless-mcp
- 2: https://dev.to/rabinarayanpatra/why-mcp-2026-07-28-spec-drops-sessions-and-goes-stateless-1gd
- 3: https://dev.to/akaranjkar08/mcp-spec-ships-july-28-every-breaking-change-and-how-to-migrate-4co8
- 4: https://modelcontextprotocol.io/specification/2025-03-26/basic/transports
- 5: Streamable HTTP: header Mcp-Session-Id is not set for "notifications/initialized" request modelcontextprotocol/inspector#905
- 6: https://www.rapidevelopers.com/mcp-tutorial/how-to-use-streamable-http-transport
- 7: https://medium.com/the-ai-language/a-visual-guide-to-mcps-streamable-http-transport-6dc18fe751ad
- 8: https://sudoall.com/mcp-streamable-http-sse/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="authbridge/authlib/plugins/ibac/plugin.go"
# Print the exact lines around the helper for context
sed -n '620,675p' "$FILE"Repository: kagenti/kagenti-extensions
Length of output: 1940
🌐 Web query:
site:modelcontextprotocol.io Mcp-Session-Id streamable HTTP
💡 Result:
In the Model Context Protocol (MCP), Mcp-Session-Id is an HTTP header used specifically within the Streamable HTTP transport to manage session state between a client and server [1][2][3]. Key details regarding its usage include: Initialization: A server MAY assign a session ID during initialization by including the Mcp-Session-Id header in the HTTP response that contains the InitializeResult [1][3]. Client Requirements: If a server provides an Mcp-Session-Id during initialization, the client MUST include this header in all subsequent HTTP requests made to that server for the duration of the session [1][3]. Validation: Servers that require a session ID SHOULD return an HTTP 400 Bad Request status for any non-initialization requests that do not include the header [1][3]. If a server receives a request with an invalid or expired session ID, it MUST respond with HTTP 404 Not Found, prompting the client to re-initialize [1][3]. Termination: Clients can explicitly terminate a session by sending an HTTP DELETE request to the MCP endpoint, including the Mcp-Session-Id header [1][4][3]. This header facilitates stateful communication in environments where multiple clients may connect to the same server, allowing the server to maintain distinct sessions [5][6]. Note that in some newer protocol revisions, the Streamable HTTP transport architecture has evolved, and users should always refer to the specific version of the specification they are implementing [7].
Citations:
- 1: https://modelcontextprotocol.io/specification/2025-11-25/basic/transports
- 2: https://modelcontextprotocol.io/specification/2025-03-26/basic/transports
- 3: https://modelcontextprotocol.io/specification/2025-06-18/basic/transports
- 4: https://ts.sdk.modelcontextprotocol.io/v2/classes/_modelcontextprotocol_client.client_streamableHttp.StreamableHTTPClientTransport.html
- 5: https://ts.sdk.modelcontextprotocol.io/v2/classes/_modelcontextprotocol_node.streamableHttp.NodeStreamableHTTPServerTransport.html
- 6: https://csharp.sdk.modelcontextprotocol.io/api/ModelContextProtocol.McpSession.html
- 7: https://modelcontextprotocol.io/specification/draft/basic/transports
🌐 Web query:
modelcontextprotocol.io transports streamable HTTP sessionless Mcp-Session-Id
💡 Result:
In MCP Streamable HTTP, the header is indeed Mcp-Session-Id, but whether it’s required depends on which Streamable HTTP protocol revision you’re targeting. 1) Streamable HTTP revisions that use Mcp-Session-Id (stateful protocol layer) - In the older Streamable HTTP revisions (e.g., protocol version 2025-03-26), servers MAY assign a session ID by including an Mcp-Session-Id header on the HTTP response containing the InitializeResult. [1] - If the server returns an Mcp-Session-Id during initialization, clients using Streamable HTTP MUST include that same Mcp-Session-Id header on all subsequent HTTP requests to that server. [2][1] - Session termination/behavior is also defined: servers can terminate sessions; clients receiving 404 for a request with Mcp-Session-Id must start a new session by sending a new InitializeRequest without a session ID; and clients SHOULD send an HTTP DELETE with Mcp-Session-Id to explicitly terminate. [2][1] 2) 2026-07-28 revision (“stateless” protocol layer): Mcp-Session-Id removed - The MCP Transport documentation’s draft “Transports” page describes a 2026-07-28 change where Streamable HTTP behavior was revised and specifically notes removal of “protocol-level sessions” and that requests/streams resumability mechanisms from earlier revisions are no longer part of this revision. [3] - The MCP release candidate blog post for 2026-07-28 states both that the Mcp-Session-Id header and the protocol-level session are removed, and that with both gone, any MCP request can go to any server instance (i.e., no sticky routing / shared session store needed at the protocol layer). [4] - SEP-2567 (“Sessionless MCP via Explicit State Handles”) further explains the intent: it removes the protocol-level session concept and the Mcp-Session-Id header, making the protocol sessionless. [5] So, for “modelcontextprotocol.io transports streamable HTTP sessionless Mcp-Session-Id”: - If you’re building against the sessionful Streamable HTTP revision, you must propagate Mcp-Session-Id on subsequent requests after it’s returned with InitializeResult. [2][1] - If you’re building against the 2026-07-28 (sessionless) revision, the protocol removes Mcp-Session-Id; you should not rely on it for request routing or continuity—use explicit state handles (per SEP-2567) or whatever mechanism the newer spec provides for carrying state. [3][5][4]
Citations:
- 1: https://modelcontextprotocol.io/specification/2025-03-26/basic/transports
- 2: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/ff960c9e/docs/specification/2025-06-18/basic/transports.mdx
- 3: https://modelcontextprotocol.io/specification/draft/basic/transports
- 4: https://blog.modelcontextprotocol.io/posts/2026-07-28-release-candidate/
- 5: https://modelcontextprotocol.io/seps/2567-sessionless-mcp
Validate MCP Streamable HTTP spec revision for Mcp-Session-Id termination
In authbridge/authlib/plugins/ibac/plugin.go (lines 644-655), the header name Mcp-Session-Id matches the Streamable HTTP transport in the sessionful protocol revisions (used to scope session state and used for explicit termination via DELETE). If this code is meant to target the sessionless 2026-07-28/SEP-2567 revision, Mcp-Session-Id is removed from the protocol, so the DELETE + Mcp-Session-Id check should be updated accordingly.
🤖 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/ibac/plugin.go` around lines 644 - 655, The DELETE
+ Mcp-Session-Id header check in isTransportShaped(pctx *pipeline.Context) uses
a sessionful header that was removed in the sessionless 2026-07-28/SEP-2567
revision; update isTransportShaped to either remove the pctx.Method == "DELETE"
&& pctx.Headers.Get("Mcp-Session-Id") != "" branch entirely if this service
targets the sessionless spec, or replace it with the correct termination
mechanism from the intended spec (e.g., a different header or transport-revision
flag). Locate the isTransportShaped function and modify the DELETE conditional
accordingly so it no longer relies on the removed Mcp-Session-Id header, and add
a comment referencing the protocol revision used to avoid regressions.
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) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
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) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
authbridge/authlib/plugins/ibac/plugin.go (1)
640-664:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftScope the
Mcp-Session-IdDELETE bypass to confirmed MCP traffic.As written, any body-less
DELETEwith that header skips IBAC, including non-MCP endpoints. That makes a header value alone a policy-bypass signal for a common side-effect method. Please gate this branch on an MCP-only signal, not justMcp-Session-Id, and add a regression test for a non-MCPDELETEcarrying the header.🤖 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/ibac/plugin.go` around lines 640 - 664, The DELETE-with-Mcp-Session-Id check in isTransportShaped currently treats any body-less DELETE with that header as transport-shaped, allowing non-MCP endpoints to bypass IBAC; change the condition in isTransportShaped to require both the Mcp-Session-Id header and an MCP-specific signal on the request/context (e.g., an MCP protocol flag on pipeline.Context or a specific MCP-only path/header such as pctx.Headers.Get("Mcp-Protocol") == "mcp" or pctx.IsMCP(true)) so only confirmed MCP traffic is exempted (update/introduce the MCP detector helper if needed and ensure isTransportRetrieval logic is unchanged), and add a regression test that sends a body-less DELETE with Mcp-Session-Id to a non-MCP endpoint and asserts IBAC is not skipped.
🤖 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.
Outside diff comments:
In `@authbridge/authlib/plugins/ibac/plugin.go`:
- Around line 640-664: The DELETE-with-Mcp-Session-Id check in isTransportShaped
currently treats any body-less DELETE with that header as transport-shaped,
allowing non-MCP endpoints to bypass IBAC; change the condition in
isTransportShaped to require both the Mcp-Session-Id header and an MCP-specific
signal on the request/context (e.g., an MCP protocol flag on pipeline.Context or
a specific MCP-only path/header such as pctx.Headers.Get("Mcp-Protocol") ==
"mcp" or pctx.IsMCP(true)) so only confirmed MCP traffic is exempted
(update/introduce the MCP detector helper if needed and ensure
isTransportRetrieval logic is unchanged), and add a regression test that sends a
body-less DELETE with Mcp-Session-Id to a non-MCP endpoint and asserts IBAC is
not skipped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b093e938-7d6e-44af-9d56-5d235b3789df
📒 Files selected for processing (2)
authbridge/authlib/plugins/ibac/plugin.goauthbridge/authlib/plugins/ibac/plugin_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- authbridge/authlib/plugins/ibac/plugin_test.go
pdettori
left a comment
pdettori
left a comment
There was a problem hiding this comment.
Clean, well-scoped extension of the transport_stream bypass.
Reviewed: Go security logic, test coverage, commit conventions, CI status.
- Body-first guard in
isTransportShapedis the load-bearing security check — prevents payload smuggling regardless of headers. Well-tested. Mcp-Session-Idheader is spec-defined (MCP Streamable HTTP 2025-03-26) and SDK-set, not user-controllable in practice.- Three new tests lock in all three corners: bypass (DELETE+header+no-body), non-bypass (DELETE+no-header), non-bypass (DELETE+header+body).
resources/subscribe+resources/unsubscribeaddition to housekeeping list is correct per MCP spec — these are subscription bookkeeping, not side-effect methods.- All 3 commits DCO-signed, conventional commit format, clean history.
- CI: all 17 checks passing.
Re CodeRabbit's concern about "SEP-2567 / 2026-07-28 sessionless revision" — that spec revision does not exist; Mcp-Session-Id remains current in the MCP Streamable HTTP transport spec. The scoping concern (bypass only for MCP traffic) is addressed by the body-first guard: no body means no action payload, so the bypass is inert from a threat-model perspective even if triggered on non-MCP traffic.
LGTM.
github-project-automation
Bot
moved this from New /:ToDo
to Done
in Kagenti Issue Prioritization
3 participants
Summary
Extends the
transport_streambypass merged in #450 to cover one more pattern of body-less transport-layer call: MCP Streamable HTTP session termination —DELETEto the MCP endpoint with theMcp-Session-Idheader.Why
After #450 fixed the body-less GET/HEAD/OPTIONS denials, one symptom remained on the same agent (
exgentic-a2a-tool-calling-gsm8k): a 403 at end-of-conversation when the MCP client SDK fires the session-cleanup DELETE.From the live session timeline post-#450:
Note: the leading-space artifact is gone (listener
pctx.Methodfrom #450 is doing its job — the action description correctly showsDELETE http://...). The judge's reasoning is technically sound; it just doesn't know thatDELETE /mcpwith theMcp-Session-Idheader is protocol cleanup, not a user-actionable delete.The benchmark task already succeeded at event 14; this 403 is cosmetic noise on cleanup. But it's noise that compounds: every conversation leaves a trailing 403 in the IBAC log, drives up
judge_unavailablerate when the judge is loaded, and makes the abctl session timeline look like the agent is misbehaving when it isn't.What changes
authlib/plugins/ibac/plugin.go— replace the single-condition step-5b check with a wider helper:New helper:
The existing
isTransportRetrieval(method)helper stays — it's now an inner check called fromisTransportShaped. Keeping it makes the two patterns testable independently and documents the GET/HEAD/OPTIONS shape on its own.Why the
Mcp-Session-Idheader is the right distinguisherThe MCP Streamable HTTP spec defines the
Mcp-Session-Idheader as the way clients reference a server-issued session ID. The header is set by the MCP client SDK (e.g., the officialmcpPython package), never by user input. So:Mcp-Session-Id→ MCP transport cleanup. Bypass.Mcp-Session-Id→ real action (DELETE /api/users/42,DELETE /resources/abc). Judge.An attacker controlling the agent could in principle set the
Mcp-Session-Idheader on a non-MCP DELETE to trick the bypass — but they could also just put their attack in a POST body, which IBAC has always judged. The bypass doesn't open new attack surface; it just stops misjudging legitimate transport.Test plan
go test ./...inauthbridge/authlib— all 29 packages passTestOnRequest_TransportStream_MCPSessionTerminate— body-less DELETE withMcp-Session-Idheader →skip/transport_stream, judge not calledTestOnRequest_TransportStream_BodylessDELETEWithoutHeaderIsJudged— body-less DELETE without the header → judged (the header is the distinguisher)TestOnRequest_TransportStreamBypass_BodylessPOSTIsJudgedstill passes — body-less DELETE withoutMcp-Session-Idreaches the judgeskip/transport_streamentries instead ofdeny/blockedNotes
This is the third symptom in a chain we've worked through:
transport_streamfor retrieval methodsno_intent_policy: allowdefaultAfter this lands, all four denies in the original timeline (rows 3, 5, 9, 10 in the pre-#450 abctl listing, plus row 15 in the post-#450 listing) will skip cleanly. The agent's full conversation lifecycle from MCP handshake through user turn through session cleanup runs without a single false-positive IBAC denial.
Threat model is unchanged from #450: real action requests (anything with a body, or any side-effect HTTP method without an MCP session header) still reach the judge.
Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
Summary by CodeRabbit
Improvements
Tests