From 6de61689dff987643f98ad8fd1f65c14d2b285ff Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Fri, 17 Jul 2026 01:17:08 +0800 Subject: [PATCH] fix(web): attach bridge token to the mcp server settings fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Task 7 token audit missed a daemon-bound network call: `agent_settings_mcp_server.rs::request_mcp_server_update` POSTs `/api/mcp/server` via `window.fetch` (Reflect::get) and attached no `X-OpenPencil-Token`. In managed mode (VS Code webview) the daemon's auth gate rejects the request (fails closed), silently breaking the MCP server start/stop toggle in the settings modal. Fix: build the request URL from `daemon_base()` (absolute, matching every other daemon call site in the crate) instead of a bare relative path, and attach the token via a new shared `live_sync::daemon_token_for` helper — factored out of `attach_daemon_headers` so XHR and non-XHR call sites can't drift on the leak-guard policy (token only when set AND the URL targets the daemon). Corrected audit (grep -rn "XmlHttpRequest|fetch|Request::new" crates/op-host-web/src, network-issuing sites only): 5 sites total — the 4 live_sync XHR helpers + web_model_catalog + web_ai_transport + iconify_web's fetch_text all already tokened via attach_daemon_headers (iconify_web additionally verified to withhold the token from the public Iconify CDN target); agent_settings_mcp_server's window.fetch was the sole untokened site and is now fixed. No other window.fetch / Request::new call sites exist in the crate. Verification: cargo check --target wasm32-unknown-unknown -p op-host-web --no-default-features --features canvaskit (pass, 1 pre-existing unrelated warning) / --features web (pass, clean); cargo clippy -p op-host-web --all-targets -- -D warnings (pass, clean). --- crates/op-host-web/src/live_sync.rs | 21 +++++++++++++++---- .../widget_host/agent_settings_mcp_server.rs | 21 ++++++++++++++++++- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/crates/op-host-web/src/live_sync.rs b/crates/op-host-web/src/live_sync.rs index 58bc3dbd3..3429a97cb 100644 --- a/crates/op-host-web/src/live_sync.rs +++ b/crates/op-host-web/src/live_sync.rs @@ -40,16 +40,29 @@ pub fn bridge_token() -> Option { BRIDGE_TOKEN.with(|t| t.borrow().clone()) } +/// The managed-daemon auth token to send for a request targeting `url`, if +/// any. `Some` only when BOTH a bridge token is stored AND `url` targets the +/// daemon (`daemon_base()` prefix) — public requests (e.g. the Iconify CDN) +/// MUST NEVER carry the token, so the prefix check is the leak guard: a URL +/// that is not the daemon origin never yields a token even when one is set. +/// This is the single policy every request helper (XHR-based and `fetch`-based +/// alike) must funnel through so the leak guard can't drift between call sites. +pub fn daemon_token_for(url: &str) -> Option { + let token = bridge_token()?; + if url.starts_with(&crate::daemon_base::daemon_base()) { + Some(token) + } else { + None + } +} + /// Attach the managed-daemon auth header to `req` — but ONLY when `url` targets /// the daemon (`daemon_base()` prefix). Public requests (e.g. the Iconify CDN) /// MUST NEVER carry the token, so the prefix check is the leak guard: a URL that /// is not the daemon origin never receives the header even when a token is set. /// Call AFTER `open` and BEFORE `send` (request headers require an open XHR). pub fn attach_daemon_headers(req: &web_sys::XmlHttpRequest, url: &str) { - let Some(token) = bridge_token() else { - return; - }; - if url.starts_with(&crate::daemon_base::daemon_base()) { + if let Some(token) = daemon_token_for(url) { let _ = req.set_request_header("X-OpenPencil-Token", &token); } } diff --git a/crates/op-host-web/src/widget_host/agent_settings_mcp_server.rs b/crates/op-host-web/src/widget_host/agent_settings_mcp_server.rs index 74b835545..e30da9e26 100644 --- a/crates/op-host-web/src/widget_host/agent_settings_mcp_server.rs +++ b/crates/op-host-web/src/widget_host/agent_settings_mcp_server.rs @@ -44,12 +44,31 @@ pub(in crate::widget_host) fn request_mcp_server_update(request: McpServerReques return; }; + // Build an absolute daemon URL (not a bare `/api/mcp/server` relative + // path) so the daemon-token gate below is checking the actual request + // target rather than assuming same-origin — same convention as every + // other daemon call site (`web_chat.rs`, `dom_io.rs`, …). + let base = crate::daemon_base::daemon_base(); + let url = format!("{base}/api/mcp/server"); + let headers = Object::new(); let _ = Reflect::set( &headers, &JsValue::from_str("Content-Type"), &JsValue::from_str("application/json"), ); + // Managed mode (VS Code webview) gates `/api/*` behind the bridge token; + // this `fetch` bypasses `live_sync`'s XHR helpers, so it must attach the + // header itself under the exact same policy as + // `live_sync::attach_daemon_headers` — daemon-targeted URL AND a token + // present — via the shared `live_sync::daemon_token_for` decision. + if let Some(token) = crate::live_sync::daemon_token_for(&url) { + let _ = Reflect::set( + &headers, + &JsValue::from_str("X-OpenPencil-Token"), + &JsValue::from_str(&token), + ); + } let init = Object::new(); let _ = Reflect::set( @@ -65,7 +84,7 @@ pub(in crate::widget_host) fn request_mcp_server_update(request: McpServerReques let _ = Reflect::set(&init, &JsValue::from_str("body"), &JsValue::from_str(&body)); let args = Array::new(); - args.push(&JsValue::from_str("/api/mcp/server")); + args.push(&JsValue::from_str(&url)); args.push(&init); let _ = fetch.apply(window.as_ref(), &args); }