From 810b43ffeba00efe74a66ff6c3f2016a19098450 Mon Sep 17 00:00:00 2001 From: chaodu-agent <274062505+chaodu-agent@users.noreply.github.com> Date: Tue, 29 Sep 2026 19:47:53 -0400 Subject: [PATCH] feat(tools): session-independent POST /mcp with a bearer session key A workspace MCP config is shared by every session in a pod, so a URL that names one session routes all of them to that session's computer (seen 2026-09-29 on kiro-1040: agent-black, agent-rp1 and mac all answered from macmini). Sessions now also get OPENAB_TOOLS_MCP_ENDPOINT (http://127.0.0.1:/mcp, identical everywhere) and OPENAB_TOOLS_MCP_TOKEN (the same key as in the URL). POST /mcp + Authorization: Bearer routes by key, with a constant-time non-short-circuit scan; missing, malformed and unknown keys are one 401. /mcp/{session}/{key} is unchanged. The contract documents the one-file config: Kiro expands ${VAR} in headers, not in url. --- runtime/CLIENT-CONTRACT.md | 44 ++++++++++++-- runtime/src/server.rs | 60 ++++++++++++++++++- runtime/src/session.rs | 18 +++++- runtime/src/tools.rs | 19 ++++++ runtime/tests/tools_e2e.rs | 115 +++++++++++++++++++++++++++++++++++-- 5 files changed, 242 insertions(+), 14 deletions(-) diff --git a/runtime/CLIENT-CONTRACT.md b/runtime/CLIENT-CONTRACT.md index 032694d..5463657 100644 --- a/runtime/CLIENT-CONTRACT.md +++ b/runtime/CLIENT-CONTRACT.md @@ -376,10 +376,23 @@ Every session child is spawned with ``` OPENAB_TOOLS_MCP_URL=http://127.0.0.1:/mcp//<64-hex key> +OPENAB_TOOLS_MCP_ENDPOINT=http://127.0.0.1:/mcp +OPENAB_TOOLS_MCP_TOKEN= ``` -Point the CLI's MCP config at it as a Streamable HTTP server. The runtime does not -edit any CLI's config; whatever installs the CLI does that. Properties: +Two equivalent ways in, to the same session's tools: + +- `POST $OPENAB_TOOLS_MCP_URL` — the session is in the path. +- `POST $OPENAB_TOOLS_MCP_ENDPOINT` with `Authorization: Bearer $OPENAB_TOOLS_MCP_TOKEN` — + the endpoint is **identical in every session**; the key selects the session. + Missing, malformed or unknown key → `401` (`WWW-Authenticate: Bearer`), + indistinguishable. Use this form whenever the CLI's MCP config is a file shared + by several sessions (a workspace `.kiro/settings/mcp.json`, a repo `.mcp.json`): + such a file cannot name one session in its URL, and a URL pinned to one session + silently routes every session to that session's computer. + +Point the CLI's MCP config at one of them as a Streamable HTTP server. The runtime +does not edit any CLI's config; whatever installs the CLI does that. Properties: - `POST` one JSON-RPC object → one JSON response, `200`. A notification → `202`, empty body. `GET` → `405`: there is no server-to-client stream, and saying so @@ -408,8 +421,29 @@ edit any CLI's config; whatever installs the CLI does that. Properties: #### Wiring the URL into the coding CLI -The runtime sets the env var; it does **not** edit the CLI's config. Whatever owns -the session's workspace does that. For `kiro-cli` (2.13+), inside the session shell: +The runtime sets the env vars; it does **not** edit the CLI's config. Whatever owns +the session's workspace does that. + +**Preferred — one config for every session.** The file never changes, across +sessions, restarts or re-lends, as long as the CLI expands environment variables in +HTTP headers. `kiro-cli` does (`${VAR}` in `headers`; it does **not** expand `url`): + +```json +{ + "mcpServers": { + "computer": { + "url": "http://127.0.0.1:/mcp", + "headers": { "Authorization": "Bearer ${OPENAB_TOOLS_MCP_TOKEN}" } + } + } +} +``` + +The port is fixed by `PTY_TOOLS_LISTEN`, so the URL is a constant. Each CLI process +expands `${OPENAB_TOOLS_MCP_TOKEN}` from its own session's environment. + +**Per session — only when the config is private to one session.** For `kiro-cli` +(2.13+), inside the session shell: ```sh kiro-cli mcp add --name computer --url "$OPENAB_TOOLS_MCP_URL" --scope global @@ -450,7 +484,7 @@ kiro-cli mcp add --name computer --url "$OPENAB_TOOLS_MCP_URL" --scope global Update `@mac/*` entries in an agent's `allowedTools` to `@computer/*` at the same time; keeping both aliases duplicates every served tool. -**Caveat — the key rotates.** The `` in `$OPENAB_TOOLS_MCP_URL` is per session +**Caveat — the key rotates** (this is what the header form above avoids). The `` in `$OPENAB_TOOLS_MCP_URL` is per session *generation*: a restart-in-place, or tearing down and re-lending a Mac, mints a new URL, and any config that hard-codes the old one (both `mcp.json` and the agent's `allowedTools` server entry) must be updated. Re-run `mcp add`, or read diff --git a/runtime/src/server.rs b/runtime/src/server.rs index 138611a..ff6b452 100644 --- a/runtime/src/server.rs +++ b/runtime/src/server.rs @@ -568,6 +568,12 @@ pub fn tools_router(state: Arc) -> Router { "/mcp/{session}/{key}", post(tools_loopback).get(tools_loopback_no_stream), ) + // Session-independent form: the session is whoever owns the bearer key. + // One static MCP config then serves every session in a shared workspace. + .route( + "/mcp", + post(tools_loopback_bearer).get(tools_loopback_no_stream), + ) .layer(DefaultBodyLimit::max(tools::MAX_LOOPBACK_BODY_BYTES)) .with_state(state) } @@ -1355,7 +1361,55 @@ async fn tools_loopback( if state.manager.get(&name).is_none() || !state.tools.check_loopback_key(&name, &key) { return StatusCode::NOT_FOUND.into_response(); } - let request: Value = match serde_json::from_slice(&body) { + serve_loopback(&state, &name, &body).await +} + +/// `POST /mcp` with `Authorization: Bearer ` — the same session +/// key as in `$OPENAB_TOOLS_MCP_URL`, delivered as `$OPENAB_TOOLS_MCP_TOKEN`. +/// Missing, malformed and unknown keys are all one `401`, so a same-pod caller +/// learns nothing about which sessions exist. +async fn tools_loopback_bearer( + State(state): State>, + headers: HeaderMap, + body: axum::body::Bytes, +) -> Response { + let unauthorized = || { + ( + StatusCode::UNAUTHORIZED, + [("www-authenticate", "Bearer")], + "send Authorization: Bearer $OPENAB_TOOLS_MCP_TOKEN", + ) + .into_response() + }; + let Some(key) = bearer_token(&headers) else { + return unauthorized(); + }; + let Some(name) = state.tools.session_for_loopback_key(key) else { + return unauthorized(); + }; + if state.manager.get(&name).is_none() { + return unauthorized(); + } + serve_loopback(&state, &name, &body).await +} + +/// `Authorization: Bearer `; the scheme is case-insensitive (RFC 7235). +fn bearer_token(headers: &HeaderMap) -> Option<&str> { + let value = headers + .get(axum::http::header::AUTHORIZATION)? + .to_str() + .ok()?; + let (scheme, token) = value.trim().split_once(' ')?; + if !scheme.eq_ignore_ascii_case("bearer") { + return None; + } + let token = token.trim(); + (!token.is_empty()).then_some(token) +} + +/// One JSON-RPC message for an authenticated session, shared by both routes. +async fn serve_loopback(state: &Arc, name: &SessionName, body: &[u8]) -> Response { + let request: Value = match serde_json::from_slice(body) { Ok(Value::Object(map)) => Value::Object(map), Ok(_) => { return ( @@ -1380,7 +1434,7 @@ async fn tools_loopback( .into_response() } }; - match state.tools.handle_loopback(&name, request).await { + match state.tools.handle_loopback(name, request).await { Some(response) => Json(response).into_response(), None => StatusCode::ACCEPTED.into_response(), } @@ -1474,7 +1528,7 @@ where }); tracing::info!( tools_listen = %local, - "tools plane enabled: loopback POST /mcp/{{session}}/{{key}}; Macs dial in on \ + "tools plane enabled: loopback POST /mcp/{{session}}/{{key}} or /mcp + Bearer; Macs dial in on \ GET /tools/attach/{{session}}" ); let router = tools_router(state.clone()); diff --git a/runtime/src/session.rs b/runtime/src/session.rs index 927ed37..85db00f 100644 --- a/runtime/src/session.rs +++ b/runtime/src/session.rs @@ -1004,6 +1004,15 @@ pub struct ToolsEndpoint { /// Environment variable that carries the session's tools MCP URL to the CLI. pub const TOOLS_URL_ENV: &str = "OPENAB_TOOLS_MCP_URL"; +/// The same session key on its own, for `Authorization: Bearer` at the +/// session-independent endpoint below. A workspace MCP config is shared by +/// every session in the pod, so it cannot name a session in its URL; it can +/// reference this variable in a header, which each CLI expands per process. +pub const TOOLS_TOKEN_ENV: &str = "OPENAB_TOOLS_MCP_TOKEN"; + +/// `http://127.0.0.1:/mcp` — identical in every session. +pub const TOOLS_ENDPOINT_ENV: &str = "OPENAB_TOOLS_MCP_ENDPOINT"; + impl SessionManager { pub fn new( config: PtyConfig, @@ -1193,11 +1202,18 @@ impl SessionManager { // restart-in-place rotates it with the shell. if let Some(endpoint) = self.tools.lock().as_ref() { let key = endpoint.hub.issue_loopback_key(&name); - env.retain(|(k, _)| k != TOOLS_URL_ENV); + env.retain(|(k, _)| { + k != TOOLS_URL_ENV && k != TOOLS_TOKEN_ENV && k != TOOLS_ENDPOINT_ENV + }); env.push(( TOOLS_URL_ENV.to_string(), format!("{}/mcp/{}/{}", endpoint.base_url, name.as_str(), key), )); + env.push((TOOLS_TOKEN_ENV.to_string(), key)); + env.push(( + TOOLS_ENDPOINT_ENV.to_string(), + format!("{}/mcp", endpoint.base_url), + )); } let request = SpawnRequest { session: name.clone(), diff --git a/runtime/src/tools.rs b/runtime/src/tools.rs index 3590d05..447e1d8 100644 --- a/runtime/src/tools.rs +++ b/runtime/src/tools.rs @@ -438,6 +438,25 @@ impl ToolsHub { bytes.len() == expected.len() && bool::from(bytes.as_slice().ct_eq(&expected)) } + /// The session whose current loopback key is `presented`, for the + /// session-independent `POST /mcp` + `Authorization: Bearer` route. Every + /// key is compared in constant time and the scan never exits early, so the + /// answer's timing does not depend on which session (if any) matched. + pub fn session_for_loopback_key(&self, presented: &str) -> Option { + let bytes = hex::decode(presented).ok()?; + if bytes.len() != 32 { + return None; + } + let keys = self.loopback_keys.lock(); + let mut found: Option = None; + for (session, expected) in keys.iter() { + if bool::from(bytes.as_slice().ct_eq(expected)) { + found = Some(session.clone()); + } + } + found + } + pub fn forget_session(&self, session: &SessionName) { self.loopback_keys.lock().remove(session); self.revoke(session, close_code::SESSION_ENDED); diff --git a/runtime/tests/tools_e2e.rs b/runtime/tests/tools_e2e.rs index 548b1c3..067d8f6 100644 --- a/runtime/tests/tools_e2e.rs +++ b/runtime/tests/tools_e2e.rs @@ -12,7 +12,10 @@ use openab_pty::close_code; use openab_pty::config; use openab_pty::killdomain::{KillDomain, TrackingLimits}; use openab_pty::server::{self, AppState, ServerConfig}; -use openab_pty::session::{PortablePtySpawner, SessionManager, SessionPolicy, TOOLS_URL_ENV}; +use openab_pty::session::{ + PortablePtySpawner, SessionManager, SessionPolicy, TOOLS_ENDPOINT_ENV, TOOLS_TOKEN_ENV, + TOOLS_URL_ENV, +}; use openab_pty::token::TokenStore; use serde_json::{json, Value}; use std::net::SocketAddr; @@ -159,6 +162,11 @@ tools_attach_ttl = "1h" /// is what makes the tests below honest: the URL under test is the one the /// child process was actually handed, not one the test computed. async fn shell_tools_url(&self, name: &str, token: &str) -> String { + self.shell_env(name, token, TOOLS_URL_ENV).await + } + + /// Ask the session's shell for one environment variable's value. + async fn shell_env(&self, name: &str, token: &str, var: &str) -> String { let mut socket = ws_connect( &format!("ws://{}/pty/{name}", self.addr), ("authorization", format!("Bearer {token}")), @@ -169,9 +177,7 @@ tools_attach_ttl = "1h" let _ = next_text(&mut socket).await; socket .send(tungstenite::Message::Binary( - format!("echo TOOLS=${TOOLS_URL_ENV}=END\n") - .into_bytes() - .into(), + format!("echo TOOLS=${var}=END\n").into_bytes().into(), )) .await .unwrap(); @@ -187,7 +193,10 @@ tools_attach_ttl = "1h" let text = String::from_utf8_lossy(&collected).to_string(); // Skip the echoed command line: take the last match, which is // the expansion. - if let Some(start) = text.rfind("TOOLS=http") { + if let Some(start) = text + .rfind("TOOLS=") + .filter(|i| !text[*i..].starts_with("TOOLS=$")) + { if let Some(end) = text[start..].find("=END") { break text[start + "TOOLS=".len()..start + end].to_string(); } @@ -264,6 +273,22 @@ fn dechunk(body: &str) -> String { out } +/// POST one JSON-RPC message with `Authorization: Bearer ` to a full URL. +async fn mcp_post_bearer(url: &str, key: Option<&str>, message: Value) -> (u16, Value) { + let without_scheme = url.strip_prefix("http://").expect("http url"); + let (host, path) = without_scheme.split_once('/').expect("path"); + let addr: SocketAddr = host.parse().expect("socket addr in url"); + let (status, raw) = http_request( + addr, + "POST", + &format!("/{path}"), + key, + Some(&message.to_string()), + ) + .await; + (status, serde_json::from_str(&raw).unwrap_or(Value::Null)) +} + /// POST one JSON-RPC message to a full loopback URL (`http://host:port/path`). async fn mcp_post(url: &str, message: Value) -> (u16, Value) { let without_scheme = url.strip_prefix("http://").expect("http url"); @@ -702,3 +727,83 @@ tools_listen = "0.0.0.0:9000" "config must refuse a non-loopback tools_listen" ); } + +/// One static MCP config for a whole workspace: every session posts to the same +/// `$OPENAB_TOOLS_MCP_ENDPOINT` with its own `$OPENAB_TOOLS_MCP_TOKEN`, and each +/// reaches its own computer. This is the shape a shared `mcp.json` needs, since +/// it cannot name a session in its URL. Field finding 2026-09-29: three sessions +/// sharing a URL-pinned config all answered from one computer. +#[tokio::test] +#[ignore = "binds sockets"] +async fn one_static_endpoint_routes_each_session_by_its_bearer_key() { + let h = Harness::start(Duration::from_secs(60)).await; + let token_a = h.create("aaa").await; + let token_b = h.create("bbb").await; + + let endpoint_a = h.shell_env("aaa", &token_a, TOOLS_ENDPOINT_ENV).await; + let endpoint_b = h.shell_env("bbb", &token_b, TOOLS_ENDPOINT_ENV).await; + assert_eq!(endpoint_a, format!("http://{}/mcp", h.tools_addr)); + assert_eq!( + endpoint_a, endpoint_b, + "the endpoint is the same in every session" + ); + + let key_a = h.shell_env("aaa", &token_a, TOOLS_TOKEN_ENV).await; + let key_b = h.shell_env("bbb", &token_b, TOOLS_TOKEN_ENV).await; + assert_eq!(key_a.len(), 64); + assert_ne!(key_a, key_b); + let url_a = h.shell_tools_url("aaa", &token_a).await; + assert!( + url_a.ends_with(&format!("/mcp/aaa/{key_a}")), + "the token is the URL's key" + ); + + let secret_a = h.mint_tools("aaa").await; + let secret_b = h.mint_tools("bbb").await; + let mac_a = fake_mac(h.addr, "aaa", &secret_a, "A").await.unwrap(); + let mac_b = fake_mac(h.addr, "bbb", &secret_b, "B").await.unwrap(); + + let call = json!({"jsonrpc":"2.0","id":7,"method":"tools/call","params":{"name":"sys_info","arguments":{}}}); + let mut answers = (String::new(), String::new()); + for _ in 0..50 { + let (sa, ra) = mcp_post_bearer(&endpoint_a, Some(&key_a), call.clone()).await; + let (sb, rb) = mcp_post_bearer(&endpoint_a, Some(&key_b), call.clone()).await; + assert_eq!((sa, sb), (200, 200)); + answers = ( + ra["result"]["content"][0]["text"] + .as_str() + .unwrap_or_default() + .to_owned(), + rb["result"]["content"][0]["text"] + .as_str() + .unwrap_or_default() + .to_owned(), + ); + if answers.0 == "fake mac A" && answers.1 == "fake mac B" { + break; + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + assert_eq!(answers, ("fake mac A".to_owned(), "fake mac B".to_owned())); + + // Missing, malformed, unknown, wrong-scheme: all the same 401, nothing routed. + let probe = json!({"jsonrpc":"2.0","id":1,"method":"tools/list"}); + let (s1, _) = mcp_post_bearer(&endpoint_a, None, probe.clone()).await; + let (s2, _) = mcp_post_bearer(&endpoint_a, Some("not-hex"), probe.clone()).await; + let (s3, _) = mcp_post_bearer(&endpoint_a, Some(&"0".repeat(64)), probe.clone()).await; + let (s4, _) = mcp_post_bearer(&endpoint_a, Some(&key_a[..32]), probe.clone()).await; + assert_eq!((s1, s2, s3, s4), (401, 401, 401, 401)); + + // The key dies with the session: after deleting A, its token routes nowhere. + h.admin("DELETE", "/admin/sessions/aaa", None).await; + let (s5, _) = mcp_post_bearer(&endpoint_a, Some(&key_a), probe.clone()).await; + assert_eq!(s5, 401); + let (s6, _) = mcp_post_bearer(&endpoint_a, Some(&key_b), probe).await; + assert_eq!(s6, 200, "B is unaffected"); + + h.admin("DELETE", "/admin/sessions/bbb/tools-attach", None) + .await; + let _ = mac_a.await; + let _ = mac_b.await; + h.shutdown().await; +}