From 13f612b111373cc86a070a2bc69fabb6fba6b308 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 18 Apr 2026 15:59:32 -0400 Subject: [PATCH] feat(server): emit baseline HTTP security headers on every response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fabro-server previously sent no security headers beyond content-type and cache-control. Add a tower middleware that fills in a conservative default set on every response, preserving any header the handler already set so routes can still override. Always applied: - X-Content-Type-Options: nosniff - X-Frame-Options: DENY - Referrer-Policy: strict-origin-when-cross-origin - Cross-Origin-Opener-Policy: same-origin - Cross-Origin-Resource-Policy: same-origin - Permissions-Policy: (deny sensor/payment/xr APIs) - X-Download-Options: noopen - X-Permitted-Cross-Domain-Policies: none - X-XSS-Protection: 0 (current OWASP guidance — the legacy filter has known bypasses; CSP is the proper replacement) - Cache-Control: no-store (default; asset routes keep their own) - Pragma: no-cache - Vary: Accept-Encoding Applied only when the request reached an HTTPS edge (direct TLS or X-Forwarded-Proto: https from a reverse proxy): - Strict-Transport-Security: max-age=63072000; includeSubDomains CSP is deliberately not included — it needs a dedicated audit of the SPA's script/style/font/connect sources and isn't a drop-in header. Filed as a separate follow-up. Tests cover each applied header, non-override behavior against the static-file cache-control, HSTS gating on X-Forwarded-Proto (including the chained "https, http" leftmost-wins case), and an integration test against a live router confirming both API and SPA responses carry the headers. --- lib/crates/fabro-server/src/lib.rs | 1 + .../fabro-server/src/security_headers.rs | 220 ++++++++++++++++++ lib/crates/fabro-server/src/server.rs | 8 +- .../fabro-server/tests/it/api/routing.rs | 81 +++++++ 4 files changed, 308 insertions(+), 2 deletions(-) create mode 100644 lib/crates/fabro-server/src/security_headers.rs diff --git a/lib/crates/fabro-server/src/lib.rs b/lib/crates/fabro-server/src/lib.rs index 6a755feea..6301a7069 100644 --- a/lib/crates/fabro-server/src/lib.rs +++ b/lib/crates/fabro-server/src/lib.rs @@ -11,6 +11,7 @@ pub mod error; pub mod github_webhooks; pub mod jwt_auth; mod run_manifest; +pub mod security_headers; pub mod serve; pub mod server; mod server_secrets; diff --git a/lib/crates/fabro-server/src/security_headers.rs b/lib/crates/fabro-server/src/security_headers.rs new file mode 100644 index 000000000..9b7c0c72c --- /dev/null +++ b/lib/crates/fabro-server/src/security_headers.rs @@ -0,0 +1,220 @@ +//! Response-wide HTTP security headers. +//! +//! Applied as a tower layer outside every other middleware so inner handlers +//! can still override any header by setting their own value first — the +//! defaults here only fill in what's missing. + +use axum::extract::Request; +use axum::http::{HeaderMap, HeaderName, HeaderValue, header}; +use axum::middleware::Next; +use axum::response::Response; + +pub async fn layer(req: Request, next: Next) -> Response { + let is_https = request_is_https(&req); + let mut response = next.run(req).await; + apply_defaults(response.headers_mut(), is_https); + response +} + +fn apply_defaults(headers: &mut HeaderMap, is_https: bool) { + // Always-on security posture. Static values, no per-request logic. + set_default(headers, header::X_CONTENT_TYPE_OPTIONS, "nosniff"); + set_default(headers, header::X_FRAME_OPTIONS, "DENY"); + set_default( + headers, + header::REFERRER_POLICY, + "strict-origin-when-cross-origin", + ); + set_default( + headers, + HeaderName::from_static("cross-origin-opener-policy"), + "same-origin", + ); + set_default( + headers, + HeaderName::from_static("cross-origin-resource-policy"), + "same-origin", + ); + set_default( + headers, + HeaderName::from_static("permissions-policy"), + PERMISSIONS_POLICY, + ); + set_default( + headers, + HeaderName::from_static("x-download-options"), + "noopen", + ); + set_default( + headers, + HeaderName::from_static("x-permitted-cross-domain-policies"), + "none", + ); + // Legacy header; current OWASP guidance is to disable the reflected-XSS + // filter (it has known bypasses and CSP is the proper replacement). + set_default(headers, HeaderName::from_static("x-xss-protection"), "0"); + + // Conservative cache defaults. Routes that deliberately want to cache + // (hashed static assets, public GETs) set their own Cache-Control before + // this middleware runs, which prevents the default from being applied. + set_default(headers, header::CACHE_CONTROL, "no-store"); + set_default(headers, header::PRAGMA, "no-cache"); + set_default(headers, header::VARY, "Accept-Encoding"); + + // HSTS is a no-op over plain HTTP per RFC 6797, but only emit it on + // connections we can actually verify came in over TLS — direct HTTPS or + // a reverse proxy that honored X-Forwarded-Proto. Prevents a misconfigured + // proxy from accidentally shipping an HSTS header for a host that isn't + // actually HTTPS-terminated. + if is_https { + set_default( + headers, + HeaderName::from_static("strict-transport-security"), + "max-age=63072000; includeSubDomains", + ); + } +} + +const PERMISSIONS_POLICY: &str = "\ +accelerometer=(), \ +autoplay=(), \ +camera=(), \ +display-capture=(), \ +encrypted-media=(), \ +fullscreen=(), \ +geolocation=(), \ +gyroscope=(), \ +magnetometer=(), \ +microphone=(), \ +midi=(), \ +payment=(), \ +picture-in-picture=(), \ +publickey-credentials-get=(), \ +screen-wake-lock=(), \ +usb=(), \ +web-share=(), \ +xr-spatial-tracking=()\ +"; + +fn set_default(headers: &mut HeaderMap, name: HeaderName, value: &'static str) { + if !headers.contains_key(&name) { + headers.insert(name, HeaderValue::from_static(value)); + } +} + +fn request_is_https(req: &Request) -> bool { + if let Some(proto) = req + .headers() + .get("x-forwarded-proto") + .and_then(|v| v.to_str().ok()) + { + // X-Forwarded-Proto may be a comma-separated list if the request went + // through multiple proxies; the leftmost value reflects the origin. + let first = proto.split(',').next().unwrap_or(proto).trim(); + if first.eq_ignore_ascii_case("https") { + return true; + } + } + req.uri().scheme_str() == Some("https") +} + +#[cfg(test)] +mod tests { + use axum::body::Body; + use axum::http::{Request as HttpRequest, Response}; + + use super::*; + + fn req(uri: &str, extra_headers: &[(&str, &str)]) -> Request { + let mut builder = HttpRequest::builder().uri(uri).method("GET"); + for (k, v) in extra_headers { + builder = builder.header(*k, *v); + } + builder.body(Body::empty()).unwrap() + } + + fn headers_after(req: &Request, seeded: &[(&str, &str)]) -> HeaderMap { + let is_https = request_is_https(req); + let mut response: Response = Response::new(Body::empty()); + for (k, v) in seeded { + response.headers_mut().insert( + HeaderName::from_bytes(k.as_bytes()).unwrap(), + HeaderValue::from_str(v).unwrap(), + ); + } + apply_defaults(response.headers_mut(), is_https); + response.into_parts().0.headers + } + + #[test] + fn core_headers_are_applied() { + let headers = headers_after(&req("/", &[]), &[]); + assert_eq!(headers.get("x-content-type-options").unwrap(), "nosniff"); + assert_eq!(headers.get("x-frame-options").unwrap(), "DENY"); + assert_eq!( + headers.get("referrer-policy").unwrap(), + "strict-origin-when-cross-origin" + ); + assert_eq!( + headers.get("cross-origin-opener-policy").unwrap(), + "same-origin" + ); + assert_eq!( + headers.get("cross-origin-resource-policy").unwrap(), + "same-origin" + ); + assert!(headers.contains_key("permissions-policy")); + assert_eq!(headers.get("x-download-options").unwrap(), "noopen"); + assert_eq!( + headers.get("x-permitted-cross-domain-policies").unwrap(), + "none" + ); + assert_eq!(headers.get("x-xss-protection").unwrap(), "0"); + assert_eq!(headers.get("cache-control").unwrap(), "no-store"); + assert_eq!(headers.get("pragma").unwrap(), "no-cache"); + assert_eq!(headers.get("vary").unwrap(), "Accept-Encoding"); + } + + #[test] + fn existing_cache_control_is_not_overridden() { + // Static assets set their own cache-control with long immutability. + // The middleware default must not clobber it. + let headers = headers_after(&req("/assets/app-abc.js", &[]), &[( + "cache-control", + "public, max-age=31536000, immutable", + )]); + assert_eq!( + headers.get("cache-control").unwrap(), + "public, max-age=31536000, immutable" + ); + } + + #[test] + fn hsts_is_added_when_x_forwarded_proto_is_https() { + let headers = headers_after(&req("/", &[("x-forwarded-proto", "https")]), &[]); + assert_eq!( + headers.get("strict-transport-security").unwrap(), + "max-age=63072000; includeSubDomains" + ); + } + + #[test] + fn hsts_is_skipped_on_plain_http() { + let headers = headers_after(&req("/", &[]), &[]); + assert!(!headers.contains_key("strict-transport-security")); + + let headers = headers_after(&req("/", &[("x-forwarded-proto", "http")]), &[]); + assert!(!headers.contains_key("strict-transport-security")); + } + + #[test] + fn hsts_reads_leftmost_value_of_chained_x_forwarded_proto() { + // When a request flows through multiple proxies, the leftmost value + // represents the original client → edge connection. + let headers = headers_after(&req("/", &[("x-forwarded-proto", "https, http")]), &[]); + assert!(headers.contains_key("strict-transport-security")); + + let headers = headers_after(&req("/", &[("x-forwarded-proto", "http, https")]), &[]); + assert!(!headers.contains_key("strict-transport-security")); + } +} diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index b97af49dd..824978703 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -116,7 +116,9 @@ use crate::jwt_auth::{ use crate::server_secrets::{ LlmClientResult, ProviderCredentials, ServerSecrets, auth_issue_message, }; -use crate::{demo, diagnostics, run_manifest, settings_view, static_files, web_auth}; +use crate::{ + demo, diagnostics, run_manifest, security_headers, settings_view, static_files, web_auth, +}; pub(crate) type EnvLookup = Arc Option + Send + Sync>; @@ -975,7 +977,9 @@ pub fn build_router_with_options( )); } - router.layer(trace_layer) + router + .layer(middleware::from_fn(security_headers::layer)) + .layer(trace_layer) } fn demo_routes() -> Router> { diff --git a/lib/crates/fabro-server/tests/it/api/routing.rs b/lib/crates/fabro-server/tests/it/api/routing.rs index e59c49c88..2055d5f6b 100644 --- a/lib/crates/fabro-server/tests/it/api/routing.rs +++ b/lib/crates/fabro-server/tests/it/api/routing.rs @@ -163,6 +163,87 @@ async fn web_enabled_serves_web_only_routes() { assert_eq!(api_miss_response.status(), StatusCode::NOT_FOUND); } +#[tokio::test] +async fn security_headers_are_applied_to_all_responses() { + let app = build_router(create_app_state(), AuthMode::Disabled); + + // Plain HTTP: HSTS must NOT be present. + let api_response = app + .clone() + .oneshot( + Request::builder() + .method("GET") + .uri("/health") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + let headers = api_response.headers(); + assert_eq!(headers.get("x-content-type-options").unwrap(), "nosniff"); + assert_eq!(headers.get("x-frame-options").unwrap(), "DENY"); + assert_eq!( + headers.get("referrer-policy").unwrap(), + "strict-origin-when-cross-origin" + ); + assert_eq!( + headers.get("cross-origin-opener-policy").unwrap(), + "same-origin" + ); + assert!(headers.contains_key("permissions-policy")); + assert_eq!(headers.get("x-xss-protection").unwrap(), "0"); + assert_eq!(headers.get("pragma").unwrap(), "no-cache"); + assert!( + !headers.contains_key("strict-transport-security"), + "HSTS must not be emitted over plain HTTP" + ); + + // X-Forwarded-Proto: https signals the request reached an HTTPS edge. + let https_response = app + .clone() + .oneshot( + Request::builder() + .method("GET") + .uri("/health") + .header("x-forwarded-proto", "https") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!( + https_response + .headers() + .get("strict-transport-security") + .unwrap(), + "max-age=63072000; includeSubDomains" + ); + + // SPA fallback path must also get the headers. + let spa_response = app + .oneshot( + Request::builder() + .method("GET") + .uri("/runs/abc123") + .header("accept", "text/html") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(spa_response.status(), StatusCode::OK); + assert_eq!( + spa_response.headers().get("x-frame-options").unwrap(), + "DENY" + ); + // Static files set their own cache-control (no-cache for index.html); + // the middleware default must not stomp on it. + assert_eq!( + spa_response.headers().get("cache-control").unwrap(), + "no-cache" + ); +} + #[tokio::test] async fn web_disabled_returns_404_for_web_routes_and_keeps_machine_api() { let settings: SettingsLayer = parse_settings_layer(