fix(server): normalize default ports in terminal origin check (#417)

## 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/<id>/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)
This commit is contained in:
Bryan Helmkamp 2026-05-26 19:30:24 -04:00 • committed by GitHub
parent 08cd80cac7
commit 38695d7e89
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -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(