From bcb8b2e2e2df7be227ed5550558f287d8229065c Mon Sep 17 00:00:00 2001 From: Alex Barclay Date: Mon, 7 Sep 2026 12:27:25 -0600 Subject: [PATCH] fix(fleetd): serve CORS so the cockpit can reach the daemon (D-3) fleetd served no CORS headers at all - OPTIONS /missions answered 405 and no response carried an Access-Control-Allow-Origin. Every browser fetch from the cockpit failed, which does not merely degrade the FLEET ops grid: store .reconnect() calls listUnits() over HTTP before opening any per-unit stream, so a missing layer blocks discovery entirely and the grid renders empty. Filed as a pre-existing main defect on 2026-08-16, blocking smoke items 1.2, 1.4a and 1.7; re-verified still real on 2026-09-07. The origin list is an allowlist, deliberately never Any. fleetd binds loopback but takes commands - POST /missions, POST /units/:id/commands - and loopback is reachable from every website the operator's browser visits. The bind address is not a security boundary against a browser, so a permissive origin would let any page drive the daemon. Defaults cover the Tauri webview on both platforms (the origin differs) and the Vite dev server; FLEETD_ALLOWED_ORIGINS overrides. Credentials stay disabled: the daemon has no cookie or Authorization auth, so allowing them would only widen what a permitted origin can do. tower-http is the one crate this adds to the graph - tower 0.5.3 was already resolved via axum, and the lock diff confirms tower-http is the only new entry. Hand-rolling the headers was the alternative and was rejected: Vary: Origin is easy to omit and a missing one is a cache-poisoning bug, and preflight has enough corners to be worth a maintained implementation. The tower dev-dependency for ServiceExt::oneshot adds nothing, being already in the lock. Three tests, driven red in both directions rather than one: layer removed (the D-3 state) -> preflight and allowed-origin FAIL, while the unlisted-origin test correctly passes, since no headers at all is right there allowlist widened to Any -> the unlisted-origin test FAILS The second is the one worth keeping: it is what stops a future "just make CORS work" change from handing every website on the internet a command channel. fleetd 87 -> 90 tests. Workspace check clean. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.lock | 16 ++++ crates/fleetd/Cargo.toml | 7 ++ crates/fleetd/src/server.rs | 142 +++++++++++++++++++++++++++++++++++- 3 files changed, 164 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 2de8c27..310848a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -231,6 +231,8 @@ dependencies = [ "thiserror 2.0.18", "tokio", "tokio-tungstenite", + "tower", + "tower-http", ] [[package]] @@ -853,6 +855,20 @@ dependencies = [ "tracing", ] +[[package]] +name = "tower-http" +version = "0.6.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4cfcf7e2740e6fc6d4d688b4ef00650406bb94adf4731e43c096c3a19fe40840" +dependencies = [ + "bitflags", + "bytes", + "http", + "pin-project-lite", + "tower-layer", + "tower-service", +] + [[package]] name = "tower-layer" version = "0.3.3" diff --git a/crates/fleetd/Cargo.toml b/crates/fleetd/Cargo.toml index 4dd9c7f..687867a 100644 --- a/crates/fleetd/Cargo.toml +++ b/crates/fleetd/Cargo.toml @@ -14,6 +14,10 @@ async-trait = { workspace = true } axum = { workspace = true } dotenvy = { workspace = true } rusqlite = { workspace = true } +# CORS. The one crate this adds to the graph (`tower` 0.5.3 is already resolved via +# axum). Hand-rolling the headers is where `Vary: Origin` cache-poisoning bugs live, +# and preflight is fiddly enough to be worth a maintained implementation. +tower-http = { version = "0.6", features = ["cors"] } [dev-dependencies] tokio = { version = "1", features = ["test-util"] } @@ -21,3 +25,6 @@ tokio = { version = "1", features = ["test-util"] } # already resolves, so no new crates enter the lock graph). tokio-tungstenite = "0.24" futures-util = "0.3" +# `ServiceExt::oneshot` for router-level request tests. Already in the lock graph +# via axum, so this adds no new crate. +tower = { version = "0.5", features = ["util"] } diff --git a/crates/fleetd/src/server.rs b/crates/fleetd/src/server.rs index 146461a..b9eda4f 100644 --- a/crates/fleetd/src/server.rs +++ b/crates/fleetd/src/server.rs @@ -29,11 +29,12 @@ use axum::{ ws::{Message, WebSocket, WebSocketUpgrade}, Path, Query, State, }, - http::StatusCode, + http::{header, HeaderValue, Method, StatusCode}, response::IntoResponse, routing::{get, post}, Json, Router, }; +use tower_http::cors::{AllowOrigin, CorsLayer}; use fleet_core::{Command, Event, GateConfig, Phase, Tier}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; @@ -138,6 +139,53 @@ impl Default for AppState { } } +/// Origins permitted to call the daemon from a browser context. +/// +/// **Deliberately an allowlist, never `Any`.** fleetd binds loopback but exposes +/// command endpoints (`POST /missions`, `POST /units/:id/commands`); with a +/// permissive origin, any page the operator happened to visit could drive the +/// daemon, because "localhost" is reachable from every website in the browser. +/// The bind address is not a security boundary against a browser. +/// +/// Defaults cover the Tauri webview (whose origin differs by platform) and the +/// Vite dev server. `FLEETD_ALLOWED_ORIGINS` overrides with a comma-separated +/// list, for a cockpit served from somewhere else. +fn allowed_origins() -> Vec { + const DEFAULTS: &[&str] = &[ + "tauri://localhost", // Tauri v2, macOS/Linux + "http://tauri.localhost", // Tauri v2, Windows + "http://localhost:5173", // Vite dev + "http://127.0.0.1:5173", + ]; + + match std::env::var("FLEETD_ALLOWED_ORIGINS") { + Ok(raw) if !raw.trim().is_empty() => raw + .split(',') + .map(str::trim) + .filter(|o| !o.is_empty()) + .filter_map(|o| HeaderValue::from_str(o).ok()) + .collect(), + _ => DEFAULTS + .iter() + .filter_map(|o| HeaderValue::from_str(o).ok()) + .collect(), + } +} + +/// D-3. Without this every browser `fetch` from the cockpit fails and the FLEET +/// ops grid is empty — `store.reconnect()` calls `listUnits()` over HTTP before +/// opening any stream, so a missing CORS layer blocks discovery entirely rather +/// than degrading it. +/// +/// Credentials are NOT allowed: the daemon has no cookie or Authorization auth, +/// so enabling them would only widen what a permitted origin can do. +fn cors_layer() -> CorsLayer { + CorsLayer::new() + .allow_origin(AllowOrigin::list(allowed_origins())) + .allow_methods([Method::GET, Method::POST, Method::OPTIONS]) + .allow_headers([header::CONTENT_TYPE]) +} + /// Build the router. pub fn router(state: AppState) -> Router { Router::new() @@ -151,6 +199,7 @@ pub fn router(state: AppState) -> Router { .route("/swarms", post(create_swarm).get(list_swarms)) .route("/swarms/:id", get(get_swarm)) .with_state(state) + .layer(cors_layer()) } #[derive(Deserialize)] @@ -2223,4 +2272,95 @@ mod tests { "nothing launched" ); } + + // ── D-3 · CORS ────────────────────────────────────────────────────────── + // Without these headers the cockpit cannot reach the daemon at all: the FLEET + // ops grid calls `listUnits()` over HTTP before opening any stream, so a + // missing layer blocks discovery rather than degrading it. The negative case + // matters as much as the positive one — fleetd takes commands on loopback, + // and loopback is reachable from every website in the browser. + + use axum::body::Body; + use axum::http::Request; + use tower::ServiceExt; // `oneshot` + + const ALLOWED: &str = "http://localhost:5173"; + const HOSTILE: &str = "https://evil.example"; + + #[tokio::test] + async fn preflight_on_a_command_route_is_answered_not_405() { + // The measured D-3 symptom was `OPTIONS /missions` → 405. + let res = router(AppState::default()) + .oneshot( + Request::builder() + .method(Method::OPTIONS) + .uri("/missions") + .header("origin", ALLOWED) + .header("access-control-request-method", "POST") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert!( + res.status().is_success(), + "preflight must be answered, got {}", + res.status() + ); + assert_eq!( + res.headers() + .get(header::ACCESS_CONTROL_ALLOW_ORIGIN) + .map(|v| v.to_str().unwrap()), + Some(ALLOWED), + ); + let methods = res + .headers() + .get(header::ACCESS_CONTROL_ALLOW_METHODS) + .map(|v| v.to_str().unwrap().to_string()) + .unwrap_or_default(); + assert!(methods.contains("POST"), "POST must be allowed, got {methods:?}"); + } + + #[tokio::test] + async fn an_allowed_origin_gets_the_header_on_a_real_response() { + let res = router(AppState::default()) + .oneshot( + Request::builder() + .uri("/health") + .header("origin", ALLOWED) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!( + res.headers() + .get(header::ACCESS_CONTROL_ALLOW_ORIGIN) + .map(|v| v.to_str().unwrap()), + Some(ALLOWED), + ); + } + + #[tokio::test] + async fn an_unlisted_origin_gets_no_allow_origin_header() { + // The check that makes the allowlist mean something. If this ever passes a + // header back, any page the operator visits can POST /missions. + let res = router(AppState::default()) + .oneshot( + Request::builder() + .uri("/health") + .header("origin", HOSTILE) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert!( + res.headers().get(header::ACCESS_CONTROL_ALLOW_ORIGIN).is_none(), + "an unlisted origin must not be granted access", + ); + } }