diff --git a/crates/acp-tunnel/src/lib.rs b/crates/acp-tunnel/src/lib.rs index 52e89ba..139edb8 100644 --- a/crates/acp-tunnel/src/lib.rs +++ b/crates/acp-tunnel/src/lib.rs @@ -33,6 +33,13 @@ pub const ACP_SUBPROTOCOL: &str = "acp.v1"; /// `openab.bearer., acp.v1` (tunnel contract §1). The server echoes only /// `acp.v1`. The token is a secret — this returns it for the transport to place /// in the handshake header; never log the result. +/// +/// ⚠️ Browser transport only. A native tokio-tungstenite (0.23) client must **not** +/// feed this as a single `Sec-WebSocket-Protocol` value: that transport parses the +/// offer with `split(",")` without trimming, so the ` acp.v1` element keeps a +/// leading space and never matches the server's echoed `acp.v1`. Native clients +/// send the bearer in the `Authorization` header and offer only [`ACP_SUBPROTOCOL`] +/// (see `src-tauri/src/remote.rs`). pub fn bearer_subprotocol(token: &str) -> String { format!("openab.bearer.{token}, {ACP_SUBPROTOCOL}") } @@ -343,6 +350,14 @@ mod tests { ); } + #[test] + fn acp_subprotocol_is_a_single_clean_token() { + // Native transports offer this verbatim as one `Sec-WebSocket-Protocol` + // value; tokio-tungstenite 0.23 splits on "," without trimming, so any + // comma or whitespace here would break the match against the server's echo. + assert!(!ACP_SUBPROTOCOL.contains([',', ' '])); + } + #[test] fn server_decl_wire_form_is_type_acp() { let d = oab_server("conn-uuid"); diff --git a/src-tauri/src/remote.rs b/src-tauri/src/remote.rs index b520c0e..a59faab 100644 --- a/src-tauri/src/remote.rs +++ b/src-tauri/src/remote.rs @@ -135,10 +135,24 @@ async fn run_once( .as_str() .into_client_request() .map_err(|e| format!("bad url: {e}"))?; - // Bearer rides the WS sub-protocol offer (contract §1). Never logged. - let proto = HeaderValue::from_str(&acp::bearer_subprotocol(&cfg.token)) - .map_err(|_| "invalid token in sub-protocol header".to_string())?; - req.headers_mut().insert("Sec-WebSocket-Protocol", proto); + // Native (non-browser) client: the bearer rides the `Authorization` header + // (the gateway reads `ws_bearer_token` from it), and the WS sub-protocol + // offers only `acp.v1`. This split matters: tokio-tungstenite 0.23 parses the + // request's `Sec-WebSocket-Protocol` with `split(",")` and does *not* trim, so + // a combined `openab.bearer., acp.v1` offer is stored as + // [`openab.bearer.`, ` acp.v1`] (note the leading space) and never + // matches the server's echoed `acp.v1` — `Server sent an invalid subprotocol`. + // `bearer_subprotocol()` remains the browser form (browsers can't set + // `Authorization` on a WS upgrade). Both values carry secrets — never logged. + req.headers_mut().insert( + "Authorization", + HeaderValue::from_str(&format!("Bearer {}", cfg.token)) + .map_err(|_| "invalid token in Authorization header".to_string())?, + ); + req.headers_mut().insert( + "Sec-WebSocket-Protocol", + HeaderValue::from_static(acp::ACP_SUBPROTOCOL), + ); let (ws, _resp) = tokio_tungstenite::connect_async(req) .await