From d4fb73d614f526c7d52d90e8820997a198873bfb Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 9 Apr 2026 19:03:51 -0400 Subject: [PATCH] feat(server): fail-closed auth posture per R52/R53 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resolve_auth_mode_with_lookup` now returns `anyhow::Result` and refuses to return success when `server.auth` resolves to zero enabled strategies. Startup propagates the error via `?` and aborts with a descriptive message pointing at the three configuration escape hatches. Previously the resolver logged a warning and returned `AuthMode::Strategies(empty)`, which meant an unconfigured server would start and then reject every request — accidental misconfigurations produced a silently-broken process rather than a clean startup failure. The new behavior matches the implementation plan's explicit guidance: "if `server.auth` is absent or resolves to no enabled API or web auth configuration, normal server startup must refuse to start. Demo and test helpers may continue to inject explicit insecure settings, but insecure startup must be opt-in rather than accidental." The single opt-in path is the `FABRO_LOCAL_NO_AUTH` env var set to the literal string `"1"`, now hoisted into a module-level `FABRO_LOCAL_NO_AUTH_ENV` constant. `fabro server start --bind ` already sets this implicitly in `start.rs:232-234`, so local daemon usage is unchanged. TCP binds now require either real auth config or an explicit `FABRO_LOCAL_NO_AUTH=1` — arguably a security improvement for TCP. Detailed error message lists the three configuration options: Configure at least one of the following in `[server.auth]`: - `[server.auth.api.jwt]` (requires `FABRO_JWT_PUBLIC_KEY` env) - `[server.auth.api.mtls]` (requires `[server.listen.tls]` ...) - `SESSION_SECRET` env (enables cookie-based web auth) Adds six new unit tests covering the full decision matrix: - `fail_closed_when_server_auth_absent` - `fail_closed_when_all_strategies_disabled` - `opt_in_insecure_startup_via_env` - `insecure_startup_flag_any_other_value_still_fails_closed` - `cookie_strategy_alone_unlocks_startup` - `mtls_strategy_resolves_when_enabled_with_listen_tls` Also adds `#[derive(Debug)]` to `AuthMode` and `AuthStrategy` so the tests can `expect_err()` on the resolver result. Two existing `fabro-cli` integration tests for TCP bind resolution (`start_with_tcp_host_only_bind_resolves_to_host_and_port` and `start_with_tcp_host_only_bind_warns_and_falls_back_when_default_port_is_unavailable`) now set `FABRO_LOCAL_NO_AUTH=1` in the test environment. They were exercising bind-address resolution, not auth, so opting into insecure startup explicitly keeps their focus narrow. 3,764 workspace tests pass (was 3,758, +6 new). `cargo fmt --check --all` and `cargo clippy --workspace -- -D warnings` are clean. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../fabro-cli/tests/it/cmd/server_start.rs | 8 + lib/crates/fabro-server/src/jwt_auth.rs | 171 +++++++++++++++--- lib/crates/fabro-server/src/serve.rs | 2 +- 3 files changed, 159 insertions(+), 22 deletions(-) diff --git a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs index 76a4b94d3..2987f887f 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs @@ -143,8 +143,12 @@ fn start_with_tcp_host_only_bind_resolves_to_host_and_port() { let storage_root = isolated_storage_dir(); let storage_dir = storage_root.path().join("storage"); + // TCP binds don't auto-enable `FABRO_LOCAL_NO_AUTH`; the test is + // exercising bind resolution, not auth, so opt into insecure + // startup explicitly. let mut cmd = context.command(); cmd.env("FABRO_STORAGE_DIR", &storage_dir); + cmd.env("FABRO_LOCAL_NO_AUTH", "1"); cmd.args(["server", "start", "--dry-run", "--bind", "127.0.0.1"]); let output = cmd.output().expect("server start command should run"); assert!( @@ -202,8 +206,12 @@ fn start_with_tcp_host_only_bind_warns_and_falls_back_when_default_port_is_unava filters.push((r"pid \d+".to_string(), "pid [PID]".to_string())); filters.push((r"127\.0\.0\.1:\d+".to_string(), "[TCP_BIND]".to_string())); + // TCP binds don't auto-enable `FABRO_LOCAL_NO_AUTH`; the test is + // exercising bind resolution, not auth, so opt into insecure + // startup explicitly. let mut cmd = context.command(); cmd.env("FABRO_STORAGE_DIR", &storage_dir); + cmd.env("FABRO_LOCAL_NO_AUTH", "1"); cmd.args(["server", "start", "--dry-run", "--bind", "127.0.0.1"]); fabro_snapshot!(filters, cmd, @" success: true diff --git a/lib/crates/fabro-server/src/jwt_auth.rs b/lib/crates/fabro-server/src/jwt_auth.rs index e930f488e..b2eb439a8 100644 --- a/lib/crates/fabro-server/src/jwt_auth.rs +++ b/lib/crates/fabro-server/src/jwt_auth.rs @@ -1,5 +1,6 @@ use std::sync::Arc; +use anyhow::{Result, anyhow}; use axum::extract::FromRequestParts; use axum::http::request::Parts; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; @@ -14,6 +15,15 @@ use crate::web_auth::SessionCookie; use fabro_types::RunAuthMethod; use fabro_types::settings::SettingsFile; +/// Env var that explicitly opts the server into unauthenticated startup. +/// +/// When set to `"1"`, [`resolve_auth_mode_with_lookup`] returns +/// [`AuthMode::Disabled`] regardless of what `server.auth` says. This is the +/// only escape hatch for running the server without configured +/// authentication; it is off by default, so accidental misconfigurations +/// fail closed. +pub const FABRO_LOCAL_NO_AUTH_ENV: &str = "FABRO_LOCAL_NO_AUTH"; + /// JWT claims for service-to-service authentication. #[derive(Debug, Deserialize)] struct Claims { @@ -27,7 +37,7 @@ struct Claims { } /// A single authentication strategy resolved at startup. -#[derive(Clone)] +#[derive(Clone, Debug)] pub enum AuthStrategy { Jwt { key: Arc, @@ -46,7 +56,7 @@ pub fn jwt_validation() -> Validation { } /// Authentication mode resolved at startup. -#[derive(Clone)] +#[derive(Clone, Debug)] pub enum AuthMode { /// One or more strategies to try in order. Strategies(Vec), @@ -71,11 +81,19 @@ pub fn decode_pem_env(name: &str, value: &str) -> String { /// Resolve the authentication mode from a [`SettingsFile`]. /// -/// Call this once at startup before serving requests. Panics if the -/// configuration is invalid (JWT strategy but no public key, or mTLS without -/// TLS config). Walks the v2 `server.auth.api.{jwt,mtls}` subtree and +/// Call this once at startup before serving requests. Returns +/// [`AuthMode::Disabled`] when [`FABRO_LOCAL_NO_AUTH_ENV`] is set to `"1"` +/// (explicit insecure-startup opt-in). Returns `AuthMode::Strategies(...)` +/// when `server.auth` resolves to at least one enabled strategy. +/// +/// Fails closed when `server.auth` is absent or resolves to zero enabled +/// strategies: startup refuses rather than silently accepting every +/// request. Panics if a configured strategy is missing its required +/// material (JWT public key, mTLS TLS config). +/// +/// Walks the v2 `server.auth.api.{jwt,mtls}` subtree and /// `server.auth.web.allowed_usernames`. -pub fn resolve_auth_mode(settings: &SettingsFile) -> AuthMode { +pub fn resolve_auth_mode(settings: &SettingsFile) -> Result { resolve_auth_mode_with_lookup(settings, |name| std::env::var(name).ok()) } @@ -116,10 +134,18 @@ fn resolve_auth_strategies(settings: &SettingsFile) -> ResolvedAuthStrategies { } } -pub fn resolve_auth_mode_with_lookup(settings: &SettingsFile, lookup: F) -> AuthMode +pub fn resolve_auth_mode_with_lookup(settings: &SettingsFile, lookup: F) -> Result where F: Fn(&str) -> Option, { + if lookup(FABRO_LOCAL_NO_AUTH_ENV).as_deref() == Some("1") { + warn!( + "{FABRO_LOCAL_NO_AUTH_ENV}=1 set; allowing unauthenticated local daemon access. \ + Do not use this flag outside local development or demo environments." + ); + return Ok(AuthMode::Disabled); + } + let ResolvedAuthStrategies { jwt_enabled, mtls_enabled, @@ -127,19 +153,6 @@ where allowed_usernames, } = resolve_auth_strategies(settings); - let any_strategy = jwt_enabled || mtls_enabled; - - if !any_strategy && std::env::var("FABRO_LOCAL_NO_AUTH").ok().as_deref() == Some("1") { - warn!( - "No authentication strategies configured; allowing unauthenticated local daemon access" - ); - return AuthMode::Disabled; - } - - if !any_strategy { - warn!("No authentication strategies configured; all requests will be rejected"); - } - let mut strategies = Vec::new(); if lookup("SESSION_SECRET").is_some() { strategies.push(AuthStrategy::Cookie); @@ -170,7 +183,21 @@ where strategies.push(AuthStrategy::Mtls); } - AuthMode::Strategies(strategies) + if strategies.is_empty() { + return Err(anyhow!( + "Fabro server refuses to start: no authentication strategies are configured.\n\ + \n\ + Configure at least one of the following in `[server.auth]`:\n\ + - `[server.auth.api.jwt]` (requires `FABRO_JWT_PUBLIC_KEY` env)\n\ + - `[server.auth.api.mtls]` (requires `[server.listen.tls]` cert/key/ca)\n\ + - `SESSION_SECRET` env (enables cookie-based web auth)\n\ + \n\ + Or set `{FABRO_LOCAL_NO_AUTH_ENV}=1` to explicitly opt in to \ + unauthenticated local daemon access." + )); + } + + Ok(AuthMode::Strategies(strategies)) } /// Extract the login from JWT claims. @@ -417,10 +444,112 @@ mod tests { use axum::http::{Request, StatusCode}; use axum::response::IntoResponse; use axum::routing::get; + use fabro_config::ConfigLayer; use tower::ServiceExt; use crate::web_auth::SessionCookie; + // --- Fail-closed resolver tests (R52/R53) ----------------------------------- + + fn settings(source: &str) -> SettingsFile { + ConfigLayer::parse(source) + .expect("fixture should parse") + .into() + } + + /// Lookup closure that returns nothing — every env var is absent. + fn empty_lookup(_name: &str) -> Option { + None + } + + #[test] + fn fail_closed_when_server_auth_absent() { + let file = settings("_version = 1\n"); + let err = + resolve_auth_mode_with_lookup(&file, empty_lookup).expect_err("should refuse startup"); + assert!(err.to_string().contains("refuses to start")); + assert!(err.to_string().contains("FABRO_LOCAL_NO_AUTH")); + } + + #[test] + fn fail_closed_when_all_strategies_disabled() { + let file = settings( + r#" +_version = 1 + +[server.auth.api.jwt] +enabled = false + +[server.auth.api.mtls] +enabled = false +"#, + ); + let err = + resolve_auth_mode_with_lookup(&file, empty_lookup).expect_err("should refuse startup"); + assert!(err.to_string().contains("no authentication strategies")); + } + + #[test] + fn opt_in_insecure_startup_via_env() { + let file = settings("_version = 1\n"); + let mode = resolve_auth_mode_with_lookup(&file, |name| { + (name == FABRO_LOCAL_NO_AUTH_ENV).then(|| "1".to_string()) + }) + .expect("FABRO_LOCAL_NO_AUTH=1 should allow startup"); + assert!(matches!(mode, AuthMode::Disabled)); + } + + #[test] + fn insecure_startup_flag_any_other_value_still_fails_closed() { + let file = settings("_version = 1\n"); + let err = resolve_auth_mode_with_lookup(&file, |name| { + (name == FABRO_LOCAL_NO_AUTH_ENV).then(|| "true".to_string()) + }) + .expect_err("only the literal string \"1\" opts in"); + assert!(err.to_string().contains("refuses to start")); + } + + #[test] + fn cookie_strategy_alone_unlocks_startup() { + let file = settings("_version = 1\n"); + let mode = resolve_auth_mode_with_lookup(&file, |name| { + (name == "SESSION_SECRET").then(|| "deadbeef".to_string()) + }) + .expect("SESSION_SECRET alone should unlock startup"); + let AuthMode::Strategies(strategies) = mode else { + panic!("expected Strategies, got Disabled"); + }; + assert_eq!(strategies.len(), 1); + assert!(matches!(strategies[0], AuthStrategy::Cookie)); + } + + #[test] + fn mtls_strategy_resolves_when_enabled_with_listen_tls() { + let file = settings( + r#" +_version = 1 + +[server.auth.api.mtls] +enabled = true + +[server.listen] +type = "tcp" +address = "127.0.0.1:3000" + +[server.listen.tls] +cert = "/etc/fabro/tls/cert.pem" +key = "/etc/fabro/tls/key.pem" +ca = "/etc/fabro/tls/ca.pem" +"#, + ); + let mode = + resolve_auth_mode_with_lookup(&file, empty_lookup).expect("mTLS config should resolve"); + let AuthMode::Strategies(strategies) = mode else { + panic!("expected Strategies, got Disabled"); + }; + assert!(strategies.iter().any(|s| matches!(s, AuthStrategy::Mtls))); + } + async fn protected_handler(_auth: AuthenticatedService) -> impl IntoResponse { "ok" } diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index 6fceb88cf..db33402bd 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -293,7 +293,7 @@ where .get(name) .cloned() .or_else(|| std::env::var(name).ok()) - }); + })?; let tls_present = TlsSettings::from_settings(&cfg_file).is_some(); let client_auth = tls_present.then(|| client_auth_from_mode(&auth_mode)); let max_concurrent_runs = args