diff --git a/crates/openab-mcp/src/mcp/facade.rs b/crates/openab-mcp/src/mcp/facade.rs index a18dd088f..9bf832068 100644 --- a/crates/openab-mcp/src/mcp/facade.rs +++ b/crates/openab-mcp/src/mcp/facade.rs @@ -478,21 +478,36 @@ pub async fn serve_http(addr: &str) -> Result<()> { serve_http_with(addr, Vec::new(), SessionTokens::new()).await } -/// [`serve_http`] plus in-process capability sources and the broker-shared -/// session-token registry (see the `sources` module). The broker hands the -/// same `tokens` handle to its session pool so per-agent-session mint/revoke -/// is visible here per request. /// The facade's axum router — factored out of [`serve_http_with`] so tests /// can drive the full HTTP path (including rmcp's injection of the request /// `Parts` into extensions, which the session-token resolution depends on) /// without binding a port. +/// +/// **Stateless transport.** The facade never issues an `Mcp-Session-Id`: every +/// POST is served on its own, and a stale `Mcp-Session-Id` header is ignored +/// rather than rejected. Nothing here needs a protocol-level session — identity +/// is the per-request `Authorization` bearer, the two tools are plain +/// request/response, and the facade never pushes notifications or sends +/// server→client requests. A stateful session only added state that can expire +/// (after 5 idle minutes, and every session on restart) plus the client-side +/// failure that expiry triggers — see `docs/oab-mcp-facade.md`, "Stateless +/// transport". This also aligns with the MCP 2026-07-28 revision's removal of +/// protocol-level sessions (the facade still speaks the 2025 `initialize` +/// handshake). +/// +/// Constraint for future facade tools: they must not rely on server→client +/// requests (sampling/elicitation/roots) or the standalone GET SSE stream — +/// both need session routing. Client cancellation is not available either: +/// stateless mode drops `notifications/cancelled`, so `RequestContext.ct` +/// never fires on a client cancel. A tool that needs any of these means +/// revisiting this choice. pub(crate) fn build_router( manager: McpRuntimeManager, sources: Vec>, tokens: SessionTokens, ) -> axum::Router { use rmcp::transport::streamable_http_server::{ - session::local::LocalSessionManager, StreamableHttpService, + session::never::NeverSessionManager, StreamableHttpServerConfig, StreamableHttpService, }; let sources = Arc::new(sources); let service = StreamableHttpService::new( @@ -503,12 +518,19 @@ pub(crate) fn build_router( tokens: tokens.clone(), }) }, - LocalSessionManager::default().into(), - Default::default(), + // Deliberately redundant with `with_stateful_mode(false)`: if stateful + // mode is ever flipped back on, `initialize` fails loudly instead of + // silently re-enabling sessions. + NeverSessionManager::default().into(), + StreamableHttpServerConfig::default().with_stateful_mode(false), ); axum::Router::new().nest_service("/mcp", service) } +/// [`serve_http`] plus in-process capability sources and the broker-shared +/// session-token registry (see the `sources` module). The broker hands the +/// same `tokens` handle to its session pool so per-agent-session mint/revoke +/// is visible here per request. pub async fn serve_http_with( addr: &str, sources: Vec>, @@ -646,7 +668,7 @@ mod tests { assert_eq!(out["channel"], ""); } - /// Full-HTTP-path proof of the session mechanism: a real request through + /// Full-HTTP-path proof of the session-token (bearer) mechanism: a real request through /// the router (rmcp StreamableHttpService) must surface the /// `Authorization` header to the handler via request extensions, and the /// same request without the header must fall back to the anonymous view. @@ -668,55 +690,20 @@ mod tests { tokens, ); - let post = |body: String, bearer: Option, session: Option| { - let mut b = axum::http::Request::builder() - .method("POST") - .uri("/mcp") - .header("content-type", "application/json") - .header("accept", "application/json, text/event-stream") - // tower::oneshot bypasses hyper, which normally supplies Host. - .header("host", "127.0.0.1"); - if let Some(t) = bearer { - b = b.header("authorization", format!("Bearer {t}")); - } - if let Some(s) = session { - b = b.header("mcp-session-id", s); - } - b.body(axum::body::Body::from(body)).unwrap() - }; - let init_body = serde_json::json!({ - "jsonrpc": "2.0", "id": 0, "method": "initialize", - "params": { "protocolVersion": "2025-06-18", "capabilities": {}, - "clientInfo": { "name": "t", "version": "0" } } - }) - .to_string(); let search_body = serde_json::json!({ "jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": { "name": "search_capabilities", "arguments": {} } }) .to_string(); - // One MCP session per identity variant (initialize → session id → call). + // The facade is stateless: identity rides on each request's bearer, so + // a bare tools/call (no initialize, no session id) is the whole exchange. let run = |bearer: Option| { let router = router.clone(); - let init_body = init_body.clone(); let search_body = search_body.clone(); async move { let resp = router - .clone() - .oneshot(post(init_body, bearer.clone(), None)) - .await - .unwrap(); - assert_eq!(resp.status(), 200, "initialize must succeed"); - let sid = resp - .headers() - .get("mcp-session-id") - .expect("session id header") - .to_str() - .unwrap() - .to_string(); - let resp = router - .oneshot(post(search_body, bearer, Some(sid))) + .oneshot(mcp_post(search_body, bearer.as_deref(), None)) .await .unwrap(); assert_eq!(resp.status(), 200); @@ -742,6 +729,134 @@ mod tests { ); } + /// A Streamable HTTP POST to the facade router, shaped like an MCP client's. + fn mcp_post( + body: String, + bearer: Option<&str>, + session: Option<&str>, + ) -> axum::http::Request { + let mut b = axum::http::Request::builder() + .method("POST") + .uri("/mcp") + .header("content-type", "application/json") + .header("accept", "application/json, text/event-stream") + // tower::oneshot bypasses hyper, which normally supplies Host. + .header("host", "127.0.0.1"); + if let Some(t) = bearer { + b = b.header("authorization", format!("Bearer {t}")); + } + if let Some(s) = session { + b = b.header("mcp-session-id", s); + } + b.body(axum::body::Body::from(body)).unwrap() + } + + /// Regression for "session expired, but the write already landed": on the + /// stateful transport an idle-retired (or pre-restart) session id got + /// `404 Session not found`, which sent Claude Code into a recovery path that + /// re-ran the call downstream and then reported it as failed. Stateless: + /// initialize returns a result and issues no session id, a notification-only + /// POST is 202, a `tools/call` carrying a stale session id is served (not + /// 404'd), and GET (the standalone SSE stream) / DELETE (session + /// termination) — both session features — are 405 with `Allow: POST`. + #[tokio::test] + async fn http_stateless_issues_no_session_and_never_404s_a_stale_one() { + use http_body_util::BodyExt as _; + use tower::ServiceExt as _; + + // Bounded so a regression that leaves the SSE stream open fails + // instead of hanging the test. + async fn sse_body(resp: axum::response::Response) -> axum::body::Bytes { + tokio::time::timeout(std::time::Duration::from_secs(10), resp.into_body().collect()) + .await + .expect("SSE response body must end") + .unwrap() + .to_bytes() + } + + let router = super::build_router( + McpRuntimeManager::from_config(McpConfig::default()), + Vec::new(), + super::SessionTokens::new(), + ); + + let init_body = serde_json::json!({ + "jsonrpc": "2.0", "id": 0, "method": "initialize", + "params": { "protocolVersion": "2025-06-18", "capabilities": {}, + "clientInfo": { "name": "t", "version": "0" } } + }) + .to_string(); + let resp = router + .clone() + .oneshot(mcp_post(init_body, None, None)) + .await + .unwrap(); + assert_eq!(resp.status(), 200, "initialize must succeed"); + assert!( + resp.headers().get("mcp-session-id").is_none(), + "the facade must not issue a session id" + ); + // A JSON-RPC error also arrives as HTTP 200 over SSE, so check the body. + let bytes = sse_body(resp).await; + let body = String::from_utf8_lossy(&bytes); + assert!( + body.contains(r#""result""#) + && body.contains("protocolVersion") + && !body.contains(r#""error""#), + "initialize must return a result, not an error: {body}" + ); + + let initialized = serde_json::json!({ + "jsonrpc": "2.0", "method": "notifications/initialized" + }) + .to_string(); + let resp = router + .clone() + .oneshot(mcp_post(initialized, None, None)) + .await + .unwrap(); + assert_eq!(resp.status(), 202, "a notification-only POST must be 202"); + + let search_body = serde_json::json!({ + "jsonrpc": "2.0", "id": 1, "method": "tools/call", + "params": { "name": "search_capabilities", "arguments": {} } + }) + .to_string(); + let resp = router + .clone() + .oneshot(mcp_post(search_body, None, Some("retired-by-idle-or-restart"))) + .await + .unwrap(); + assert_eq!(resp.status(), 200, "a stale session id must not be 404'd"); + let bytes = sse_body(resp).await; + let body = String::from_utf8_lossy(&bytes); + assert!( + body.contains(r#""id":1"#) + && body.contains(r#""result""#) + && body.contains("unavailable") + && body.contains(r#""isError":false"#), + "tools/call must be served a search result: {body}" + ); + + for method in ["GET", "DELETE"] { + let req = axum::http::Request::builder() + .method(method) + .uri("/mcp") + .header("accept", "text/event-stream") + .header("host", "127.0.0.1") + .header("mcp-session-id", "retired-by-idle-or-restart") + .body(axum::body::Body::empty()) + .unwrap(); + let resp = router.clone().oneshot(req).await.unwrap(); + assert_eq!(resp.status(), 405, "{method} /mcp must be 405"); + assert_eq!( + resp.headers().get("allow").and_then(|v| v.to_str().ok()), + Some("POST"), + "{method} /mcp must advertise Allow: POST" + ); + } + } + #[tokio::test] async fn host_level_source_works_anonymously_and_validates_args() { let facade = facade_with_source(false); diff --git a/crates/openab-mcp/src/native/gmail.rs b/crates/openab-mcp/src/native/gmail.rs index 0549b0935..cc6bfa466 100644 --- a/crates/openab-mcp/src/native/gmail.rs +++ b/crates/openab-mcp/src/native/gmail.rs @@ -100,7 +100,8 @@ impl OauthApp { } } -/// The native Gmail MCP server. One instance per `serve` process. +/// The native Gmail MCP server. One instance per stdio `serve` process; over +/// HTTP (`serve_http`), one per MCP session. pub struct GmailNative { http: reqwest::Client, store: McpCredentialStore, @@ -702,12 +703,20 @@ impl ServerHandler for GmailNative { } } -/// Serve the adapter over **loopback Streamable HTTP** at `/mcp`, mirroring -/// the OAB MCP Facade's transport (ADR §6.2 trust model: loopback-only, no +/// Serve the adapter over **loopback Streamable HTTP** at `/mcp`, with the +/// same trust model as the OAB MCP Facade (ADR §6.2: loopback-only, no /// authentication layer — the host/pod boundary is the trust boundary; a /// non-loopback bind is refused at startup). Register in `mcp.json` as a /// `"type": "http"` entry pointing at `http:///mcp`. /// +/// Unlike the facade, this transport is still **stateful**: rmcp retires a +/// session after 5 idle minutes (and all of them on restart), so clients can +/// hit the same `404 Session not found` recovery race the facade's stateless +/// transport removed. Going stateless here first needs the `reqwest::Client` +/// and refresh lock shared outside the per-session factory +/// (`GmailNative::new` builds fresh ones), or every request would lose +/// connection pooling and the refresh lock would cover a single call. +/// /// One handler is constructed per MCP session, all sharing the same /// credential store file; a cross-session refresh race is benign (each /// refresh yields a valid token and the stored refresh token is preserved). diff --git a/docs/adr/oab-mcp-adapter.md b/docs/adr/oab-mcp-adapter.md index 451a39a40..1252dba4f 100644 --- a/docs/adr/oab-mcp-adapter.md +++ b/docs/adr/oab-mcp-adapter.md @@ -430,11 +430,16 @@ listen = "127.0.0.1:8848" # loopback only; this is the default - **Security posture:** the listener refuses to bind any non-loopback address; the endpoint itself carries no authentication layer, so the host/pod boundary is the trust boundary for every capability that - does not require a session. Deployments that colocate untrusted processes + does not require an agent session. Deployments that colocate untrusted processes must not enable `[mcp]`. Session-bound sources are the exception: the - per-session bearer shipped in #1447, so the facade hides and refuses them - unless the request resolves to a session. A socket-permission scheme for - the remaining session-free capabilities is still a documented follow-up. + per-agent-session bearer shipped in #1447, so the facade hides and refuses them + unless the request resolves to an agent session. A socket-permission scheme for + the remaining agent-session-free capabilities is still a documented follow-up. +- **Stateless transport:** the facade serves Streamable HTTP without + protocol-level sessions (no `Mcp-Session-Id`; `GET`/`DELETE /mcp` → `405`); + identity stays the per-request bearer above. The native Gmail adapter's HTTP + transport remains stateful pending unification. See + [`oab-mcp-facade.md`](../oab-mcp-facade.md#stateless-transport). - **Native `openab-agent`:** the dispatcher is invoked in-process via the existing `mcp` meta-tool. No HTTP hop is required because the facade contract and policy checks are implemented by the same dispatcher component diff --git a/docs/oab-mcp-facade.md b/docs/oab-mcp-facade.md index e3292303a..7ea90c906 100644 --- a/docs/oab-mcp-facade.md +++ b/docs/oab-mcp-facade.md @@ -125,12 +125,43 @@ Claude Code: claude mcp add --transport http oab http://127.0.0.1:8848/mcp ``` +## Stateless transport + +The facade serves Streamable HTTP in **stateless** mode: it never issues an +`Mcp-Session-Id`, every POST is handled on its own, and `GET`/`DELETE` on +`/mcp` return `405`. Nothing it does needs a protocol-level session — +identity is the per-request `Authorization` bearer, both tools are plain +request/response, and it never pushes notifications or sends server→client +requests. + +This removes a client-visible failure mode. With sessions, an idle-retired +(5 minutes) or pre-restart session id got `404 Session not found`; Claude +Code then retried the call on a fresh session while its own background +reconnect closed that connection about a second later, so the retried call +**ran downstream** (a write landed) yet the agent saw +`MCP server "…" session expired`. Stateless, there is no session to expire. +A stale `Mcp-Session-Id` header is ignored rather than rejected, so clients +still holding one after an idle gap or a restart keep working. This also +aligns with the MCP 2026-07-28 revision's removal of protocol-level sessions +(the facade still speaks the 2025 `initialize` handshake). + +Constraint for future facade tools: they must not rely on server→client +requests (sampling/elicitation/roots) or the standalone GET SSE stream — both +need session routing. Client cancellation is not available either: stateless +mode drops `notifications/cancelled`, so a tool's request cancellation token +never fires on a client cancel. A tool that needs any of these means +revisiting this choice. + ## Trust model +"Session" below means the **agent session** behind the per-request +`Authorization` bearer — application-level, unrelated to the MCP protocol +session the facade no longer issues (see Stateless transport). + Loopback-only, **no authentication layer** — the host/pod boundary is the -trust boundary. Any process on the host can invoke every capability that does **not** require a session. -Session-bound sources are different: since the per-session bearer shipped, the facade filters -them out unless the request resolves to a session, so a host process without that token can +trust boundary. Any process on the host can invoke every capability that does **not** require an agent session. +Session-bound sources are different: since the per-agent-session bearer shipped, the facade filters +them out unless the request's bearer resolves to an agent session, so a host process without that token can neither discover nor call them. Do not enable it on hosts that colocate untrusted processes. Least-privilege is enforced per provider with `tool_filter` include/exclude lists in `mcp.json`; excluded tools are