From 38695d7e89ec2e861e20f98daaaeb556d2a5c5d9 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp <19+brynary@users.noreply.github.com> Date: Tue, 26 May 2026 19:30:24 -0400 Subject: [PATCH] fix(server): normalize default ports in terminal origin check (#417) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary The terminal WebSocket origin check (`origin_allowed` in `handler/sandbox.rs`) rejected browser requests when the `Host` header omitted the default port for the scheme. Result: clicking the **Terminal** tab on `/runs//sandbox` returned **403 Forbidden** and the UI showed "Terminal WebSocket connection failed." Other tabs (Services, Filesystem, VNC) worked because their WebSockets either don't traverse the server (VNC connects directly to Daytona's signed preview URL) or aren't WebSocket upgrades. ## Root cause Browsers send `Origin: https://example.com` and `Host: example.com` (no `:443`) on default HTTPS. The previous logic always constructed the origin authority *with* the default port, then string-compared against the raw `Host` header: ```rust let origin_authority = match origin_url.port_or_known_default() { Some(port) => format!("{origin_host}:{port}"), None => origin_host.to_string(), }; origin_authority.eq_ignore_ascii_case(host) ``` So `"example.com:443"` got compared against `"example.com"` and never matched. Every browser-driven WS upgrade to a default-port HTTPS deployment failed. ## Fix Parse the `Host` header through the origin's scheme into another `Url`, then compare `host_str()` and `port_or_known_default()` on both sides. This normalizes default ports symmetrically. ```rust let Ok(host_url) = url::Url::parse(&format!("{}://{host}", origin_url.scheme())) else { return false; }; origin_url.host_str() == host_url.host_str() && origin_url.port_or_known_default() == host_url.port_or_known_default() ``` Reproduced in a production deployment of the nightly image behind Caddy doing TLS termination on a public IP. Before the fix the terminal WS handshake returned 403 every time; with the fix the handshake completes and the terminal session attaches. ## Tests Added four new cases alongside the existing two: - `origin_validation_allows_default_https_port_omitted_from_host` — the bug case (browser-style `Origin: https://host` + `Host: host`). - `origin_validation_allows_default_http_port_omitted_from_host` — same for plain HTTP. - `origin_validation_allows_explicit_default_port_in_host` — `Host: example.com:443` still matches `Origin: https://example.com`. - `origin_validation_rejects_scheme_mismatch_on_default_port` — `Origin: http://example.com` + `Host: example.com:443` is still rejected (different effective ports). All six `origin_validation_*` tests pass; the full `fabro-server` suite stays green (679/679). ## Test plan - [x] `cargo nextest run -p fabro-server origin_validation` — 6 passed - [x] `cargo nextest run -p fabro-server` — 679 passed - [x] `cargo +nightly-2026-04-14 fmt --check --all` - [x] `cargo +nightly-2026-04-14 clippy -p fabro-server --all-targets -- -D warnings` - [x] Manual: terminal tab in the SPA against a TLS-terminated default-port deployment 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .../src/server/handler/sandbox.rs | 54 ++++++++++++++++--- 1 file changed, 48 insertions(+), 6 deletions(-) diff --git a/lib/crates/fabro-server/src/server/handler/sandbox.rs b/lib/crates/fabro-server/src/server/handler/sandbox.rs index 3bad166d4..bfba9c242 100644 --- a/lib/crates/fabro-server/src/server/handler/sandbox.rs +++ b/lib/crates/fabro-server/src/server/handler/sandbox.rs @@ -186,14 +186,15 @@ fn origin_allowed(headers: &HeaderMap) -> bool { let Ok(origin_url) = url::Url::parse(origin) else { return false; }; - let Some(origin_host) = origin_url.host_str() else { + // Parse the Host header through the origin's scheme so default-port + // normalization is symmetric: browsers omit the port from Host for default + // scheme ports (e.g. `Host: example.com` on HTTPS) but always include + // scheme+host in Origin. + let Ok(host_url) = url::Url::parse(&format!("{}://{host}", origin_url.scheme())) else { return false; }; - let origin_authority = match origin_url.port_or_known_default() { - Some(port) => format!("{origin_host}:{port}"), - None => origin_host.to_string(), - }; - origin_authority.eq_ignore_ascii_case(host) + origin_url.host_str() == host_url.host_str() + && origin_url.port_or_known_default() == host_url.port_or_known_default() } async fn run_terminal( @@ -984,6 +985,37 @@ mod tests { assert!(origin_allowed(&headers)); } + #[test] + fn origin_validation_allows_default_https_port_omitted_from_host() { + // Browsers omit the port from Host when connecting on default scheme ports. + let mut headers = HeaderMap::new(); + headers.insert("host", HeaderValue::from_static("example.com")); + headers.insert("origin", HeaderValue::from_static("https://example.com")); + assert!(origin_allowed(&headers)); + + let mut headers = HeaderMap::new(); + headers.insert("host", HeaderValue::from_static("100.53.109.177")); + headers.insert("origin", HeaderValue::from_static("https://100.53.109.177")); + assert!(origin_allowed(&headers)); + } + + #[test] + fn origin_validation_allows_default_http_port_omitted_from_host() { + let mut headers = HeaderMap::new(); + headers.insert("host", HeaderValue::from_static("example.com")); + headers.insert("origin", HeaderValue::from_static("http://example.com")); + assert!(origin_allowed(&headers)); + } + + #[test] + fn origin_validation_allows_explicit_default_port_in_host() { + // RFC-legal but uncommon: client includes the default port explicitly. + let mut headers = HeaderMap::new(); + headers.insert("host", HeaderValue::from_static("example.com:443")); + headers.insert("origin", HeaderValue::from_static("https://example.com")); + assert!(origin_allowed(&headers)); + } + #[test] fn origin_validation_rejects_cross_origin_browser_origin() { let mut headers = HeaderMap::new(); @@ -992,6 +1024,16 @@ mod tests { assert!(!origin_allowed(&headers)); } + #[test] + fn origin_validation_rejects_scheme_mismatch_on_default_port() { + // Same hostname but Origin uses http (default port 80) while Host carries the + // HTTPS default port 443 — different effective ports must not match. + let mut headers = HeaderMap::new(); + headers.insert("host", HeaderValue::from_static("example.com:443")); + headers.insert("origin", HeaderValue::from_static("http://example.com")); + assert!(!origin_allowed(&headers)); + } + #[test] fn ss_parser_extracts_addresses_processes_and_preview_support() { let services = parse_ss_listening_services(