From aebe75cd04ca0b3416befb58f8ebb2a3440eada1 Mon Sep 17 00:00:00 2001 From: Huskies Agent Date: Thu, 16 Jul 2026 17:14:48 +0000 Subject: [PATCH] =?UTF-8?q?huskies:=20merge=201179=20story=20Token-authent?= =?UTF-8?q?icated=20sled=E2=86=92gateway=20WS=20uplink=20(remote-gateway?= =?UTF-8?q?=20ready)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- server/src/http/gateway/websocket.rs | 259 ++++++++++++++++++++++++--- server/src/main.rs | 27 ++- server/src/sled_uplink.rs | 137 +++++++++++++- 3 files changed, 390 insertions(+), 33 deletions(-) diff --git a/server/src/http/gateway/websocket.rs b/server/src/http/gateway/websocket.rs index 9302a700..59c62250 100644 --- a/server/src/http/gateway/websocket.rs +++ b/server/src/http/gateway/websocket.rs @@ -151,18 +151,35 @@ pub async fn gateway_crdt_sync_handler( /// Query parameters accepted on the `/api/sled-uplink` WebSocket upgrade. #[derive(Deserialize)] struct SledUplinkParams { - /// Shared-secret token identifying the connecting sled (from `[sled_tokens]` in `projects.toml`). + /// Legacy shared-secret token, sent as a query parameter (story 899). + /// Superseded by the `Authorization: Bearer` header (story 1179 AC 1) but + /// still honoured when no header is present, for backwards compatibility. token: Option, + /// Project name the connecting sled represents, declared upfront so a + /// rejected connection's target project can still be logged even when + /// the supplied token doesn't match (story 1179 AC 2). + project: Option, } /// `GET /api/sled-uplink` — gateway-side WebSocket endpoint for sled uplinks. /// /// # Authentication /// -/// The connecting sled must supply a valid shared-secret token via the `token` -/// query parameter. Tokens are configured either as per-project `auth_token` -/// fields under `[projects.]` in `projects.toml` (preferred, story 899) -/// or, for backwards compatibility, in the deprecated `[sled_tokens]` table. +/// The connecting sled declares which project it represents via the +/// `project` query parameter and supplies its shared-secret token as an +/// `Authorization: Bearer ` header (preferred, story 1179 AC 1) or, +/// for backwards compatibility, a `token` query parameter. Tokens are +/// configured either as per-project `auth_token` fields under +/// `[projects.]` in `projects.toml` (preferred, story 899) or, for +/// backwards compatibility, in the deprecated `[sled_tokens]` table. +/// +/// When no token is configured anywhere in `projects.toml` (i.e. +/// [`GatewayState::sled_tokens`] is empty), the connection is accepted +/// unconditionally and a warning is logged that the uplink is unauthenticated +/// (story 1179 AC 3). Otherwise, a project with no token of its own is +/// rejected — auth being "on" anywhere means every project must be +/// explicitly configured, rather than silently falling back to open access. +/// Token comparison is constant-time (story 1179 AC 5). /// /// # Protocol /// @@ -184,23 +201,47 @@ pub async fn gateway_sled_uplink_handler( ws: WebSocket, state: Data<&Arc>, Query(params): Query, + req: &poem::Request, ) -> poem::Response { - let token = match params.token { - Some(t) if !t.is_empty() => t, - _ => { - return poem::Response::builder() - .status(StatusCode::UNAUTHORIZED) - .body("token query parameter required"); - } - }; + let project = params.project.filter(|p| !p.is_empty()); + let bearer_token = req + .header("Authorization") + .and_then(|v| v.strip_prefix("Bearer ")) + .filter(|t| !t.is_empty()) + .map(str::to_string); + let token = bearer_token.or_else(|| params.token.filter(|t| !t.is_empty())); - let sled_id = match state.sled_tokens.get(&token) { - Some(id) => id.clone(), - None => { + let sled_id = if state.sled_tokens.is_empty() { + // No tokens configured anywhere — accept as today, but this is the + // one case where the sled operates entirely unauthenticated. + let name = project.unwrap_or_else(|| "".to_string()); + crate::slog_warn!( + "[gateway/sled-uplink] no sled tokens configured; accepting uplink for '{name}' unauthenticated" + ); + name + } else { + let name = match project { + Some(p) => p, + None => { + return poem::Response::builder() + .status(StatusCode::UNAUTHORIZED) + .body("project query parameter required"); + } + }; + let expected = expected_token_for_project(&state, &name).await; + let authorized = match (&expected, &token) { + (Some(expected_token), Some(given)) => tokens_match(given, expected_token), + _ => false, + }; + if !authorized { + crate::slog!( + "[gateway/sled-uplink] rejected uplink for project '{name}': missing or invalid token" + ); return poem::Response::builder() .status(StatusCode::UNAUTHORIZED) - .body("invalid token"); + .body("invalid or missing token"); } + name }; use poem::IntoResponse as _; @@ -212,6 +253,35 @@ pub async fn gateway_sled_uplink_handler( .into_response() } +/// Resolve the configured auth token for `project`, checking the per-project +/// `auth_token` field first (preferred, story 899) and falling back to the +/// legacy `[sled_tokens]` table (keyed by sled_id, reversed onto +/// [`GatewayState::sled_tokens`]) for backwards compatibility. +async fn expected_token_for_project(state: &GatewayState, project: &str) -> Option { + if let Some(token) = state + .projects + .read() + .await + .get(project) + .and_then(|p| p.auth_token.clone()) + { + return Some(token); + } + state + .sled_tokens + .iter() + .find(|(_, sled_id)| sled_id.as_str() == project) + .map(|(token, _)| token.clone()) +} + +/// Constant-time comparison of a supplied token against the expected one +/// (story 1179 AC 5), preventing a timing attack from leaking the configured +/// secret byte-by-byte. +fn tokens_match(given: &str, expected: &str) -> bool { + use subtle::ConstantTimeEq; + given.as_bytes().ct_eq(expected.as_bytes()).into() +} + /// Run a single connected sled's request/response flow until it disconnects. /// /// Performs the identity handshake, registers a [`SledConnection`] in @@ -220,7 +290,7 @@ pub async fn gateway_sled_uplink_handler( async fn run_sled_uplink_session( socket: poem::web::websocket::WebSocketStream, state: Arc, - token_sled_id: String, + sled_id: String, perm_tx: tokio::sync::mpsc::UnboundedSender, ) { use crate::service::gateway::SledConnection; @@ -241,18 +311,19 @@ async fn run_sled_uplink_session( _ => { crate::slog!( "[gateway/sled-uplink] '{}' missing identity frame; closing", - token_sled_id + sled_id ); return; } }; - // Project name in the identity must match the project resolved from the - // auth token; otherwise the sled is claiming to be a different project. - if identity != token_sled_id { + // Project name in the identity must match the project declared (and + // authenticated) pre-upgrade; otherwise the sled is claiming to be a + // different project than the one its token was checked against. + if identity != sled_id { crate::slog!( - "[gateway/sled-uplink] identity mismatch (token says '{}', sled claims '{}'); closing", - token_sled_id, + "[gateway/sled-uplink] identity mismatch (auth says '{}', sled claims '{}'); closing", + sled_id, identity ); return; @@ -524,3 +595,143 @@ pub async fn gateway_event_push_handler( }) .into_response() } + +// ── Tests ──────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + use super::*; + use crate::service::gateway::{GatewayConfig, ProjectEntry}; + use poem::EndpointExt as _; + use poem::listener::TcpAcceptor; + use std::collections::BTreeMap; + use std::path::PathBuf; + use tokio::net::TcpListener; + use tokio_tungstenite::tungstenite::client::IntoClientRequest; + use tokio_tungstenite::tungstenite::http::HeaderValue; + use tokio_tungstenite::tungstenite::http::header::AUTHORIZATION; + + #[test] + fn tokens_match_accepts_equal_tokens() { + assert!(tokens_match("secret", "secret")); + } + + #[test] + fn tokens_match_rejects_different_tokens() { + assert!(!tokens_match("secret", "wrong")); + } + + #[test] + fn tokens_match_rejects_different_length_tokens() { + assert!(!tokens_match("short", "much-longer-token")); + } + + /// Spin up a real poem server exposing only the sled-uplink handler on an + /// ephemeral loopback port, returning its base `ws://` URL and the state + /// (so callers can inspect e.g. `sled_connections` after connecting). + async fn start_test_gateway(config: GatewayConfig) -> (String, Arc) { + let state = Arc::new(GatewayState::new(config, PathBuf::new(), 3000).unwrap()); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let route = poem::Route::new() + .at("/api/sled-uplink", poem::get(gateway_sled_uplink_handler)) + .data(state.clone()); + tokio::spawn(async move { + let acceptor = TcpAcceptor::from_tokio(listener).unwrap(); + let _ = poem::Server::new_with_acceptor(acceptor).run(route).await; + }); + (format!("ws://{addr}"), state) + } + + /// Attempt a WS upgrade with an optional `Authorization: Bearer` header. + /// Returns `Ok(())` on a successful upgrade (101) or `Err(status)` for a + /// rejected pre-upgrade response. + async fn connect_with_bearer(url: &str, token: Option<&str>) -> Result<(), u16> { + let mut request = url.into_client_request().unwrap(); + if let Some(t) = token { + request.headers_mut().insert( + AUTHORIZATION, + HeaderValue::from_str(&format!("Bearer {t}")).unwrap(), + ); + } + match tokio_tungstenite::connect_async(request).await { + Ok(_) => Ok(()), + Err(tokio_tungstenite::tungstenite::Error::Http(resp)) => Err(resp.status().as_u16()), + Err(e) => panic!("unexpected connect error: {e}"), + } + } + + fn config_with_project_token(project: &str, token: &str) -> GatewayConfig { + let mut projects = BTreeMap::new(); + projects.insert( + project.to_string(), + ProjectEntry { + url: None, + auth_token: Some(token.to_string()), + ssh_port: None, + host_path: None, + expected_node_id: None, + }, + ); + GatewayConfig { + projects, + sled_tokens: BTreeMap::new(), + release_channels: BTreeMap::new(), + } + } + + #[tokio::test] + async fn valid_token_via_header_is_accepted() { + let config = config_with_project_token("myproj", "secret-token"); + let (base_url, _state) = start_test_gateway(config).await; + let url = format!("{base_url}/api/sled-uplink?project=myproj"); + assert!( + connect_with_bearer(&url, Some("secret-token")) + .await + .is_ok(), + "valid bearer token must be accepted" + ); + } + + #[tokio::test] + async fn wrong_token_is_rejected_with_401() { + let config = config_with_project_token("myproj", "secret-token"); + let (base_url, _state) = start_test_gateway(config).await; + let url = format!("{base_url}/api/sled-uplink?project=myproj"); + let result = connect_with_bearer(&url, Some("wrong-token")).await; + assert_eq!(result, Err(401), "wrong token must be rejected with 401"); + } + + #[tokio::test] + async fn missing_token_is_rejected_when_configured() { + let config = config_with_project_token("myproj", "secret-token"); + let (base_url, _state) = start_test_gateway(config).await; + let url = format!("{base_url}/api/sled-uplink?project=myproj"); + let result = connect_with_bearer(&url, None).await; + assert_eq!( + result, + Err(401), + "missing token must be rejected when a token is configured for the project" + ); + } + + #[tokio::test] + async fn no_sled_tokens_configured_is_accepted_unauthenticated() { + let mut projects = BTreeMap::new(); + projects.insert( + "myproj".to_string(), + ProjectEntry::with_url("http://myproj:3001"), + ); + let config = GatewayConfig { + projects, + sled_tokens: BTreeMap::new(), + release_channels: BTreeMap::new(), + }; + let (base_url, _state) = start_test_gateway(config).await; + let url = format!("{base_url}/api/sled-uplink?project=myproj"); + assert!( + connect_with_bearer(&url, None).await.is_ok(), + "with no sled tokens configured anywhere, the uplink must accept unauthenticated" + ); + } +} diff --git a/server/src/main.rs b/server/src/main.rs index 7419527b..76c033cf 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -270,13 +270,9 @@ async fn main() -> Result<(), std::io::Error> { crate::crdt_state::write_llm_session(&services.bot_name.to_lowercase(), "all"); // Sled uplink: forward permission requests to an upstream gateway when configured. - let upstream_gateway = cli - .upstream_gateway - .clone() - .or_else(|| std::env::var("HUSKIES_UPSTREAM_GATEWAY").ok()) - .unwrap_or_default(); - // Project name for the identity frame (story 899 AC 5). Env-var override - // wins; otherwise derive from the project_root basename. + // Project name for the identity frame (story 899 AC 5) and for the + // derived uplink URL below (story 1179 AC 4). Env-var override wins; + // otherwise derive from the project_root basename. let project_name = std::env::var("HUSKIES_PROJECT_NAME").unwrap_or_else(|_| { services .project_root @@ -285,12 +281,29 @@ async fn main() -> Result<(), std::io::Error> { .unwrap_or("unknown") .to_string() }); + // Explicit `--upstream-gateway`/`HUSKIES_UPSTREAM_GATEWAY` (a full WS URL, + // possibly with a legacy `?token=`) takes precedence; otherwise derive the + // uplink URL from `HUSKIES_GATEWAY_URL` (story 1179 AC 4). + let upstream_gateway = cli + .upstream_gateway + .clone() + .or_else(|| std::env::var("HUSKIES_UPSTREAM_GATEWAY").ok()) + .or_else(|| { + std::env::var("HUSKIES_GATEWAY_URL") + .ok() + .map(|gateway_url| sled_uplink::build_uplink_url(&gateway_url, &project_name)) + }) + .unwrap_or_default(); + // Shared-secret sent as `Authorization: Bearer ` on the uplink + // upgrade request (story 1179 AC 1). + let sled_auth_token = std::env::var("HUSKIES_SLED_TOKEN").ok(); let local_mcp_url = format!("http://127.0.0.1:{port}/mcp"); sled_uplink::spawn_uplink_task( sled_uplink::UplinkConfig { upstream_url: upstream_gateway, project_name, local_mcp_url, + auth_token: sled_auth_token, }, Arc::clone(&services), ); diff --git a/server/src/sled_uplink.rs b/server/src/sled_uplink.rs index 47fa9b62..8f732a25 100644 --- a/server/src/sled_uplink.rs +++ b/server/src/sled_uplink.rs @@ -38,7 +38,8 @@ use tokio_tungstenite::tungstenite::Message as WsMessage; /// without proliferating positional arguments. pub struct UplinkConfig { /// WebSocket URL of the upstream gateway's `/api/sled-uplink` endpoint. - /// Includes the `?token=` query parameter for auth. + /// May include a legacy `?token=` query parameter for auth; see + /// [`build_uplink_url`] for the preferred derivation from `HUSKIES_GATEWAY_URL`. pub upstream_url: String, /// Project name this sled identifies as. Sent in the `identity` frame /// after WS connect (story 899 AC 5). @@ -47,6 +48,34 @@ pub struct UplinkConfig { /// `http://127.0.0.1:3001/mcp`). Used to replay `mcp_request` frames /// received from the gateway against the local MCP handler. pub local_mcp_url: String, + /// Shared-secret token sent as `Authorization: Bearer ` on the WS + /// upgrade request (story 1179 AC 1). Read from `HUSKIES_SLED_TOKEN`. + /// When `None`, no `Authorization` header is sent (open/unauthenticated + /// gateway configurations continue to work unchanged). + pub auth_token: Option, +} + +/// Convert an `http://` or `https://` base URL to its `ws://` / `wss://` +/// equivalent (story 1179 AC 4). Returns the input unchanged if it does not +/// start with `http`. +pub fn to_ws_url(base: &str) -> String { + if let Some(rest) = base.strip_prefix("https://") { + format!("wss://{rest}") + } else if let Some(rest) = base.strip_prefix("http://") { + format!("ws://{rest}") + } else { + base.to_string() + } +} + +/// Derive the sled uplink WebSocket URL from a `HUSKIES_GATEWAY_URL`-style +/// base URL and this sled's project name (story 1179 AC 4). +/// +/// The token itself is no longer embedded in the URL — it travels as an +/// `Authorization: Bearer` header instead (see [`UplinkConfig::auth_token`]). +pub fn build_uplink_url(gateway_url: &str, project_name: &str) -> String { + let ws_base = to_ws_url(gateway_url.trim_end_matches('/')); + format!("{ws_base}/api/sled-uplink?project={project_name}") } // ── Back-off constants ──────────────────────────────────────────────────────── @@ -98,6 +127,7 @@ pub fn spawn_uplink_task(config: UplinkConfig, services: Arc) { upstream_url, project_name, local_mcp_url, + auth_token, } = config; slog!("[uplink] Spawning sled uplink task (gateway={upstream_url}, project={project_name})"); tokio::spawn(async move { @@ -114,6 +144,7 @@ pub fn spawn_uplink_task(config: UplinkConfig, services: Arc) { loop { match run_uplink_session( &upstream_url, + auth_token.as_deref(), &project_name, &local_mcp_url, &http, @@ -138,17 +169,44 @@ pub fn spawn_uplink_task(config: UplinkConfig, services: Arc) { // ── Private helpers ─────────────────────────────────────────────────────────── +/// Build the WebSocket upgrade request for `url`, attaching an +/// `Authorization: Bearer ` header when `auth_token` is set (story +/// 1179 AC 1). With no token, this is equivalent to connecting with the bare +/// URL string. +fn build_connect_request( + url: &str, + auth_token: Option<&str>, +) -> Result, String> { + use tokio_tungstenite::tungstenite::client::IntoClientRequest; + + let mut request = url + .into_client_request() + .map_err(|e| format!("build request for {url}: {e}"))?; + if let Some(token) = auth_token.filter(|t| !t.is_empty()) { + let value = + tokio_tungstenite::tungstenite::http::HeaderValue::from_str(&format!("Bearer {token}")) + .map_err(|e| format!("invalid auth token header value: {e}"))?; + request.headers_mut().insert( + tokio_tungstenite::tungstenite::http::header::AUTHORIZATION, + value, + ); + } + Ok(request) +} + /// Run a single uplink session: connect, send identity frame, pump messages /// bidirectionally until disconnect or channel close, then fail-close any /// in-flight requests. async fn run_uplink_session( url: &str, + auth_token: Option<&str>, project_name: &str, local_mcp_url: &str, http: &reqwest::Client, perm_rx: &mut tokio::sync::mpsc::Receiver, ) -> Result<(), String> { - let (ws_stream, _) = tokio_tungstenite::connect_async(url) + let request = build_connect_request(url, auth_token)?; + let (ws_stream, _) = tokio_tungstenite::connect_async(request) .await .map_err(|e| format!("WS connect to {url}: {e}"))?; slog!("[uplink] Connected to gateway uplink endpoint"); @@ -512,6 +570,78 @@ mod tests { assert_eq!(rx2.blocking_recv().unwrap(), PermissionDecision::Deny); } + // ── AC 4: URL scheme mapping ────────────────────────────────────── + + #[test] + fn to_ws_url_converts_https_to_wss() { + assert_eq!(to_ws_url("https://gateway:3000"), "wss://gateway:3000"); + } + + #[test] + fn to_ws_url_converts_http_to_ws() { + assert_eq!(to_ws_url("http://gateway:3000"), "ws://gateway:3000"); + } + + #[test] + fn to_ws_url_passes_through_non_http_scheme() { + assert_eq!(to_ws_url("ws://already:3000"), "ws://already:3000"); + } + + #[test] + fn build_uplink_url_from_https_gateway() { + assert_eq!( + build_uplink_url("https://gateway.example.com", "myproj"), + "wss://gateway.example.com/api/sled-uplink?project=myproj" + ); + } + + #[test] + fn build_uplink_url_from_http_gateway_trims_trailing_slash() { + assert_eq!( + build_uplink_url("http://gateway:3000/", "myproj"), + "ws://gateway:3000/api/sled-uplink?project=myproj" + ); + } + + // ── AC 1: Authorization header on connect ───────────────────────── + + #[test] + fn build_connect_request_attaches_bearer_header_when_token_set() { + let request = build_connect_request("ws://gateway:3001/api/sled-uplink", Some("secret")) + .expect("must build request"); + assert_eq!( + request + .headers() + .get(tokio_tungstenite::tungstenite::http::header::AUTHORIZATION) + .and_then(|v| v.to_str().ok()), + Some("Bearer secret") + ); + } + + #[test] + fn build_connect_request_omits_header_when_token_absent() { + let request = build_connect_request("ws://gateway:3001/api/sled-uplink", None) + .expect("must build request"); + assert!( + request + .headers() + .get(tokio_tungstenite::tungstenite::http::header::AUTHORIZATION) + .is_none() + ); + } + + #[test] + fn build_connect_request_omits_header_when_token_empty() { + let request = build_connect_request("ws://gateway:3001/api/sled-uplink", Some("")) + .expect("must build request"); + assert!( + request + .headers() + .get(tokio_tungstenite::tungstenite::http::header::AUTHORIZATION) + .is_none() + ); + } + #[test] fn on_gateway_text_ignores_unknown_type() { let mut in_flight: HashMap> = HashMap::new(); @@ -549,6 +679,7 @@ mod tests { upstream_url: String::new(), project_name: "test".to_string(), local_mcp_url: "http://127.0.0.1:0/mcp".to_string(), + auth_token: None, }, services, ); @@ -608,6 +739,7 @@ mod tests { upstream_url: url, project_name: "test-proj".to_string(), local_mcp_url: "http://127.0.0.1:0/mcp".to_string(), + auth_token: None, }, Arc::clone(&services), ); @@ -691,6 +823,7 @@ mod tests { upstream_url: url, project_name: "test-proj".to_string(), local_mcp_url: "http://127.0.0.1:0/mcp".to_string(), + auth_token: None, }, Arc::clone(&services), );