diff --git a/lib/crates/fabro-cli/src/commands/config/mod.rs b/lib/crates/fabro-cli/src/commands/config/mod.rs index f4a182c27..d24baf870 100644 --- a/lib/crates/fabro-cli/src/commands/config/mod.rs +++ b/lib/crates/fabro-cli/src/commands/config/mod.rs @@ -11,7 +11,8 @@ use std::io::Write; use std::path::Path; use fabro_config::effective_settings::{EffectiveSettingsLayers, EffectiveSettingsMode}; -use fabro_config::{load_and_resolve, load_settings_project, project}; +use fabro_config::{load_settings_project, project}; +use fabro_types::settings::Settings; use fabro_types::settings::cli::{CliLayer, OutputFormat}; use fabro_types::settings::{CliSettings, SettingsLayer}; use fabro_util::printer::Printer; @@ -90,15 +91,59 @@ fn local_settings_value( ) -> anyhow::Result { let base_ctx = CommandContext::base(printer, cli.clone(), cli_layer)?; let layers = config_layers(&base_ctx, args.workflow.as_deref())?; - let mut value = serde_json::to_value(load_and_resolve( + let local_settings = fabro_config::effective_settings::materialize_settings_layer( layers, None, EffectiveSettingsMode::LocalOnly, - )?)?; + )?; + let mut value = serde_json::to_value(resolve_local_settings(&local_settings, printer)?)?; strip_nulls(&mut value); Ok(value) } +fn render_resolve_errors(errors: Vec) -> anyhow::Error { + anyhow::anyhow!( + "failed to resolve local settings:\n{}", + errors + .into_iter() + .map(|error| error.to_string()) + .collect::>() + .join("\n") + ) +} + +fn resolve_local_settings(file: &SettingsLayer, printer: Printer) -> anyhow::Result { + let file = fabro_config::apply_builtin_defaults(file.clone()); + + let project = fabro_config::resolve_project_from_file(&file).map_err(render_resolve_errors)?; + let workflow = fabro_config::resolve_workflow_from_file(&file).map_err(render_resolve_errors)?; + let run = fabro_config::resolve_run_from_file(&file).map_err(render_resolve_errors)?; + let cli = fabro_config::resolve_cli_from_file(&file).map_err(render_resolve_errors)?; + let features = fabro_config::resolve_features_from_file(&file).map_err(render_resolve_errors)?; + + let server = match fabro_config::resolve_server_from_file(&file) { + Ok(server) => server, + Err(errors) => { + for error in &errors { + fabro_util::printerr!(printer, "warning: server config: {error}"); + } + + let server_layer = file.server.clone().unwrap_or_default(); + let mut ignored = Vec::new(); + fabro_config::resolve_server(&server_layer, &mut ignored) + } + }; + + Ok(Settings { + project, + workflow, + run, + cli, + server, + features, + }) +} + async fn rendered_config( args: &SettingsArgs, cli: &CliSettings, diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 010078cae..a85b512e2 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -131,15 +131,6 @@ fn format_config_toml() -> String { toml::to_string_pretty(&doc).expect("default server config should serialize") } -fn settings_enable_dev_token_auth(settings_toml: &str) -> bool { - fabro_config::parse_settings_layer(settings_toml) - .ok() - .and_then(|layer| layer.server) - .and_then(|srv| srv.auth) - .and_then(|auth| auth.methods) - .is_some_and(|methods| methods.contains(&ServerAuthMethod::DevToken)) -} - // --------------------------------------------------------------------------- // Binary detection // --------------------------------------------------------------------------- @@ -1585,9 +1576,14 @@ async fn run_install_github_inner( .and_then(|srv| srv.auth) .and_then(|auth| auth.methods) .unwrap_or_default(); - let token = dev_token::read_dev_token_file( - &Storage::new(&storage_dir).server_state().dev_token_path(), - ); + let token = methods + .contains(&ServerAuthMethod::DevToken) + .then(|| { + dev_token::read_dev_token_file( + &Storage::new(&storage_dir).server_state().dev_token_path(), + ) + }) + .flatten(); print_auth_status(&methods, token.as_deref(), &s, printer); fabro_util::printerr!(printer, ""); } @@ -1802,6 +1798,13 @@ async fn run_install_inner( toml::to_string_pretty(&doc)? }; + let install_settings = user_config::apply_storage_dir_override( + fabro_config::parse_settings_layer(&settings_toml) + .context("failed to parse generated settings.toml")?, + args.storage_dir.as_deref(), + ); + fabro_config::resolve_server_from_file(&install_settings).map_err(render_server_resolve_errors)?; + // Secrets and auth material { let session_secret = session_secret::generate_session_secret(); @@ -1818,8 +1821,8 @@ async fn run_install_inner( s.green.apply_to("✔") ); - let dev_token = if settings_enable_dev_token_auth(&settings_toml) { - let token = dev_token::load_or_create_dev_token( + let dev_token = if fabro_config::dev_token_auth_enabled(&install_settings) { + let token = dev_token::read_or_mint_dev_token_for_install( &fabro_util::Home::from_env().dev_token_path(), )?; dev_token::write_dev_token( @@ -1862,11 +1865,6 @@ async fn run_install_inner( server_was_running, ) .await?; - let install_settings = user_config::apply_storage_dir_override( - fabro_config::parse_settings_layer(&settings_toml) - .context("failed to parse generated settings.toml")?, - args.storage_dir.as_deref(), - ); if let Err(err) = write_artifact_store_metadata(&install_settings, FABRO_VERSION).await { fabro_util::printerr!( printer, @@ -1910,9 +1908,14 @@ async fn run_install_inner( .and_then(|auth| auth.methods.as_ref()) .map(Vec::as_slice) .unwrap_or_default(); - let token = dev_token::read_dev_token_file( - &Storage::new(&storage_dir).server_state().dev_token_path(), - ); + let token = methods + .contains(&ServerAuthMethod::DevToken) + .then(|| { + dev_token::read_dev_token_file( + &Storage::new(&storage_dir).server_state().dev_token_path(), + ) + }) + .flatten(); print_auth_status(methods, token.as_deref(), &s, printer); fabro_util::printerr!(printer, ""); true @@ -2177,47 +2180,59 @@ name = "custom" ); } + fn parse_install_settings(source: &str) -> SettingsLayer { + fabro_config::parse_settings_layer(source).expect("install settings fixture should parse") + } + #[test] - fn settings_enable_dev_token_auth_when_methods_include_dev_token() { - let toml = r#" + fn dev_token_auth_enabled_when_methods_include_dev_token() { + let settings = parse_install_settings( + r#" _version = 1 [server.auth] methods = ["dev-token"] -"#; - assert!(settings_enable_dev_token_auth(toml)); +"#, + ); + assert!(fabro_config::dev_token_auth_enabled(&settings)); } #[test] - fn settings_enable_dev_token_auth_when_mixed_with_github() { - let toml = r#" + fn dev_token_auth_enabled_when_mixed_with_github() { + let settings = parse_install_settings( + r#" _version = 1 [server.auth] methods = ["dev-token", "github"] -"#; - assert!(settings_enable_dev_token_auth(toml)); +"#, + ); + assert!(fabro_config::dev_token_auth_enabled(&settings)); } #[test] - fn settings_enable_dev_token_auth_false_for_github_only() { - let toml = r#" + fn dev_token_auth_enabled_false_for_github_only() { + let settings = parse_install_settings( + r#" _version = 1 [server.auth] methods = ["github"] -"#; - assert!(!settings_enable_dev_token_auth(toml)); +"#, + ); + assert!(!fabro_config::dev_token_auth_enabled(&settings)); } #[test] - fn settings_enable_dev_token_auth_false_when_methods_absent() { - let toml = " + fn dev_token_auth_enabled_false_when_methods_absent() { + let settings = parse_install_settings( + " _version = 1 [server.auth] -"; - assert!(!settings_enable_dev_token_auth(toml)); +", + ); + assert!(!fabro_config::dev_token_auth_enabled(&settings)); } #[test] diff --git a/lib/crates/fabro-cli/src/commands/server/foreground.rs b/lib/crates/fabro-cli/src/commands/server/foreground.rs index f2b536281..0c2eb6988 100644 --- a/lib/crates/fabro-cli/src/commands/server/foreground.rs +++ b/lib/crates/fabro-cli/src/commands/server/foreground.rs @@ -36,7 +36,6 @@ pub(crate) async fn execute( }; let log_path = Storage::new(&storage_dir).server_state().log_path(); - let dev_token_path = std::env::var_os("FABRO_DEV_TOKEN_PATH").map(PathBuf::from); let pid = std::process::id(); Box::pin(serve::serve_command( @@ -48,7 +47,6 @@ pub(crate) async fn execute( pid, bind: resolved_bind.clone(), log_path: log_path.clone(), - dev_token_path: dev_token_path.clone(), started_at: Utc::now(), }) }, diff --git a/lib/crates/fabro-cli/src/commands/server/record.rs b/lib/crates/fabro-cli/src/commands/server/record.rs index 23481d8dd..a725c8702 100644 --- a/lib/crates/fabro-cli/src/commands/server/record.rs +++ b/lib/crates/fabro-cli/src/commands/server/record.rs @@ -15,12 +15,10 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, Serialize, Deserialize)] pub(crate) struct ServerRecord { - pub pid: u32, - pub bind: Bind, - pub log_path: PathBuf, - #[serde(skip_serializing_if = "Option::is_none")] - pub dev_token_path: Option, - pub started_at: DateTime, + pub pid: u32, + pub bind: Bind, + pub log_path: PathBuf, + pub started_at: DateTime, } #[derive(Debug, Clone)] @@ -129,10 +127,9 @@ mod tests { fn test_record(bind: Bind) -> ServerRecord { ServerRecord { - pid: std::process::id(), + pid: std::process::id(), bind, - log_path: PathBuf::from("/tmp/storage/logs/server.log"), - dev_token_path: None, + log_path: PathBuf::from("/tmp/storage/logs/server.log"), started_at: Utc::now(), } } diff --git a/lib/crates/fabro-cli/src/commands/server/start.rs b/lib/crates/fabro-cli/src/commands/server/start.rs index 31ebc0ce5..420e6b427 100644 --- a/lib/crates/fabro-cli/src/commands/server/start.rs +++ b/lib/crates/fabro-cli/src/commands/server/start.rs @@ -16,7 +16,7 @@ use fabro_server::serve; use fabro_server::serve::{DEFAULT_TCP_PORT, ServeArgs}; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; -use fabro_util::{Home, dev_token, session_secret}; +use fabro_util::session_secret; use tokio::net::{TcpStream, UnixStream}; use tokio::process::Command as TokioCommand; use tokio::task::spawn_blocking; @@ -214,51 +214,6 @@ fn server_max_concurrent_runs_override() -> Option { .filter(|value| *value > 0) } -fn load_or_create_local_dev_token(storage_dir: &Path, home: &Home) -> Result { - if let Some(token) = std::env::var("FABRO_DEV_TOKEN") - .ok() - .filter(|token| dev_token::validate_dev_token_format(token)) - { - return Ok(token); - } - - let storage = Storage::new(storage_dir); - let server_env_path = storage.server_state().env_path(); - if let Some(token) = envfile::read_env_file(&server_env_path) - .ok() - .and_then(|entries| entries.get("FABRO_DEV_TOKEN").cloned()) - .filter(|token| dev_token::validate_dev_token_format(token)) - { - dev_token::write_dev_token(&home.dev_token_path(), &token) - .with_context(|| format!("writing dev token to {}", home.dev_token_path().display()))?; - dev_token::write_dev_token(&storage.server_state().dev_token_path(), &token).with_context( - || { - format!( - "writing dev token to {}", - storage.server_state().dev_token_path().display() - ) - }, - )?; - return Ok(token); - } - - let token = dev_token::load_or_create_dev_token(&home.dev_token_path()).with_context(|| { - format!( - "loading or creating dev token at {}", - home.dev_token_path().display() - ) - })?; - dev_token::write_dev_token(&storage.server_state().dev_token_path(), &token).with_context( - || { - format!( - "writing dev token to {}", - storage.server_state().dev_token_path().display() - ) - }, - )?; - Ok(token) -} - fn valid_session_secret(secret: &str) -> bool { session_secret::validate_session_secret(secret).is_ok() } @@ -297,28 +252,17 @@ async fn execute_foreground( storage_dir: PathBuf, _log_bootstrap: ForegroundServerLogBootstrap, styles: &'static Styles, - printer: Printer, + _printer: Printer, ) -> Result<()> { - let home = Home::from_env(); - let token = load_or_create_local_dev_token(&storage_dir, &home)?; let session_secret = load_or_create_local_session_secret(&storage_dir)?; - let prior_token = std::env::var_os("FABRO_DEV_TOKEN"); let prior_session_secret = std::env::var_os("SESSION_SECRET"); - std::env::set_var("FABRO_DEV_TOKEN", &token); std::env::set_var("SESSION_SECRET", &session_secret); - let _env_guard = scopeguard::guard( - (prior_token, prior_session_secret), - |(prior_token, prior_session_secret)| { - match prior_token { - Some(value) => std::env::set_var("FABRO_DEV_TOKEN", value), - None => std::env::remove_var("FABRO_DEV_TOKEN"), - } - match prior_session_secret { - Some(value) => std::env::set_var("SESSION_SECRET", value), - None => std::env::remove_var("SESSION_SECRET"), - } - }, - ); + let _env_guard = scopeguard::guard(prior_session_secret, |prior_session_secret| { + match prior_session_secret { + Some(value) => std::env::set_var("SESSION_SECRET", value), + None => std::env::remove_var("SESSION_SECRET"), + } + }); let server_state = Storage::new(&storage_dir).server_state(); let record_path = server_state.record_path(); @@ -343,12 +287,10 @@ async fn execute_foreground( styles, Some(storage_dir), move |resolved_bind| { - print_dev_token(printer, &home, &token); record::write_server_record(&record_path, &record::ServerRecord { pid, bind: resolved_bind.clone(), log_path: log_path.clone(), - dev_token_path: Some(home.dev_token_path()), started_at: Utc::now(), }) }, @@ -430,12 +372,8 @@ async fn execute_daemon( cmd.arg("--watch-web"); } - let home = Home::from_env(); - let token = load_or_create_local_dev_token(storage_dir, &home)?; let session_secret = load_or_create_local_session_secret(storage_dir)?; cmd.arg("--storage-dir").arg(storage_dir); - cmd.env("FABRO_DEV_TOKEN", &token); - cmd.env("FABRO_DEV_TOKEN_PATH", home.dev_token_path()); cmd.env("SESSION_SECRET", &session_secret); cmd.env_remove("FABRO_JSON"); @@ -484,7 +422,6 @@ async fn execute_daemon( fabro_util::printerr!(printer, "Web UI: {styled}"); } print_auth_methods(printer, serve_args); - print_dev_token(printer, &home, &token); } return Ok(()); } @@ -524,11 +461,6 @@ fn print_auth_methods(printer: Printer, serve_args: &ServeArgs) { fabro_util::printerr!(printer, "Auth: {}", names.join(", ")); } -fn print_dev_token(printer: Printer, home: &Home, token: &str) { - fabro_util::printerr!(printer, "Dev token: {token}"); - fabro_util::printerr!(printer, "Token file: {}", home.dev_token_path().display()); -} - // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- diff --git a/lib/crates/fabro-cli/src/server_client.rs b/lib/crates/fabro-cli/src/server_client.rs index 337a34530..88158fc39 100644 --- a/lib/crates/fabro-cli/src/server_client.rs +++ b/lib/crates/fabro-cli/src/server_client.rs @@ -1,4 +1,5 @@ -use std::path::{Path, PathBuf}; +use std::net::IpAddr; +use std::path::Path; use std::sync::Arc; use std::time::Duration; @@ -8,7 +9,6 @@ use fabro_client::{ apply_bearer_token_auth, }; pub(crate) use fabro_client::{Client, RunEventStream}; -use fabro_config::Storage; use fabro_server::bind::Bind; pub(crate) use fabro_types::RunProjection; use fabro_types::settings::SettingsLayer; @@ -17,17 +17,15 @@ use fabro_util::{Home, dev_token}; use tokio::time::sleep; use crate::args::ServerTargetArgs; -use crate::commands::server::{record, start}; +use crate::commands::server::start; use crate::user_config::{self, cli_http_client_builder}; #[derive(Debug)] -struct CliDevTokenFallback { - storage_dir: Option, -} +struct CliDevTokenFallback; impl CredentialFallback for CliDevTokenFallback { fn resolve(&self) -> Option { - load_dev_token_if_available(self.storage_dir.as_deref()).map(Credential::DevToken) + load_cli_dev_token().map(Credential::DevToken) } } @@ -38,9 +36,7 @@ fn refreshable_oauth( if matches!(credential, Some(Credential::OAuth(_))) { let session = OAuthSession::new(target.clone(), AuthStore::default()); if local_dev_token_fallback(target) { - return Some( - session.with_fallback(Arc::new(CliDevTokenFallback { storage_dir: None })), - ); + return Some(session.with_fallback(Arc::new(CliDevTokenFallback))); } return Some(session); } @@ -86,23 +82,19 @@ async fn connect_managed_unix_socket_api_client_bundle( active_config_path: &Path, ) -> Result { let target = ServerTarget::unix_socket_path(path)?; - let credential = resolve_target_credential( - &target, - Some(storage_dir), - local_dev_token_fallback(&target), - )?; + let credential = resolve_target_credential(&target, local_dev_token_fallback(&target))?; let oauth_session = refreshable_oauth(&target, credential.as_ref()); let bearer_token = credential.as_ref().map(Credential::bearer_token); let http_client = if let Ok(http_client) = - try_connect_unix_socket_http_client(path, Some(storage_dir), bearer_token).await + try_connect_unix_socket_http_client(path, true, bearer_token).await { http_client } else { start::ensure_server_running_on_socket(path, active_config_path, storage_dir) .await .with_context(|| format!("Failed to start fabro server for {}", path.display()))?; - connect_unix_socket_http_client(path, Some(storage_dir), bearer_token) + connect_unix_socket_http_client(path, true, bearer_token) .await .with_context(|| format!("Failed to connect to fabro server at {}", path.display()))? }; @@ -125,16 +117,14 @@ async fn connect_local_api_client_bundle( .with_context(|| format!("Failed to start fabro server for {}", storage_dir.display()))?; match bind { Bind::Unix(path) => { - let http_client = - connect_unix_socket_http_client(&path, Some(storage_dir), None).await?; + let http_client = connect_unix_socket_http_client(&path, true, None).await?; Ok(Client::from_http_client("http://fabro", http_client)) } Bind::Tcp(addr) => { - let token = wait_for_local_dev_token(storage_dir).await?; - let builder = cli_http_client_builder().no_proxy(); - let http_client = apply_bearer_token_auth(builder, &token)?.build()?; - let base_url = format!("http://{addr}"); - Ok(Client::from_http_client(base_url, http_client)) + let target = ServerTarget::http_url(&format!("http://{addr}"))?; + let credential = resolve_local_tcp_credential(&target)?; + let oauth_session = refreshable_oauth(&target, credential.as_ref()); + build_client(target, credential, oauth_session, None).await } } } @@ -150,7 +140,7 @@ pub(crate) async fn connect_api_client(storage_dir: &Path) -> Result Result { - let credential = resolve_target_credential(target, None, local_dev_token_fallback(target))?; + let credential = resolve_target_credential(target, local_dev_token_fallback(target))?; let oauth_session = refreshable_oauth(target, credential.as_ref()); build_client(target.clone(), credential, oauth_session, None).await } @@ -189,6 +179,9 @@ fn connect_cli_target_transport( ) -> Result<(fabro_http::HttpClient, String)> { if let Some(api_url) = target.as_http_url() { let mut builder = cli_http_client_builder(); + if should_bypass_proxy_for_http_target(api_url) { + builder = builder.no_proxy(); + } builder = match bearer_token { Some(token) => apply_bearer_token_auth(builder, token)?, None => builder, @@ -215,13 +208,12 @@ fn local_dev_token_fallback(target: &ServerTarget) -> bool { target.is_unix_socket() } -fn load_dev_token_if_available(storage_dir: Option<&Path>) -> Option { +fn load_cli_dev_token() -> Option { let env_token = std::env::var("FABRO_DEV_TOKEN").ok(); - load_dev_token_if_available_from_sources(storage_dir, env_token.as_deref(), &Home::from_env()) + load_cli_dev_token_from_sources(env_token.as_deref(), &Home::from_env()) } -fn load_dev_token_if_available_from_sources( - storage_dir: Option<&Path>, +fn load_cli_dev_token_from_sources( env_token: Option<&str>, home: &Home, ) -> Option { @@ -229,51 +221,32 @@ fn load_dev_token_if_available_from_sources( return Some(token.to_owned()); } - if let Some(storage_dir) = storage_dir { - let storage_token_path = Storage::new(storage_dir).server_state().dev_token_path(); - if let Some(token) = dev_token::read_dev_token_file(&storage_token_path) { - return Some(token); - } - - let record_path = Storage::new(storage_dir).server_state().record_path(); - if let Some(token) = record::read_server_record(&record_path) - .and_then(|server| server.dev_token_path) - .as_deref() - .and_then(dev_token::read_dev_token_file) - { - return Some(token); - } - } - dev_token::read_dev_token_file(&home.dev_token_path()) } -async fn wait_for_local_dev_token(storage_dir: &Path) -> Result { +async fn wait_for_cli_dev_token() -> Result { let deadline = std::time::Instant::now() + Duration::from_secs(5); while std::time::Instant::now() < deadline { - if let Some(token) = load_dev_token_if_available(Some(storage_dir)) { + if let Some(token) = load_cli_dev_token() { return Ok(token); } sleep(Duration::from_millis(50)).await; } - bail!( - "local server dev token did not become available for {}", - storage_dir.display() - ); + bail!("local CLI dev token did not become available"); } async fn build_authed_unix_socket_http_client( path: &Path, - storage_dir: Option<&Path>, + wait_for_cli_dev_token_fallback: bool, bearer_token: Option<&str>, ) -> Result { let builder = cli_http_client_builder().unix_socket(path).no_proxy(); let builder = if let Some(token) = bearer_token { apply_bearer_token_auth(builder, token)? - } else if let Some(storage_dir) = storage_dir { - let token = wait_for_local_dev_token(storage_dir).await?; + } else if wait_for_cli_dev_token_fallback { + let token = wait_for_cli_dev_token().await?; apply_bearer_token_auth(builder, &token)? } else { builder @@ -294,49 +267,93 @@ fn build_unix_socket_probe_client(path: &Path) -> Result async fn try_connect_unix_socket_http_client( path: &Path, - storage_dir: Option<&Path>, + wait_for_cli_dev_token_fallback: bool, bearer_token: Option<&str>, ) -> Result { check_server_ready(&build_unix_socket_probe_client(path)?).await?; - build_authed_unix_socket_http_client(path, storage_dir, bearer_token).await + build_authed_unix_socket_http_client(path, wait_for_cli_dev_token_fallback, bearer_token).await } async fn connect_unix_socket_http_client( path: &Path, - storage_dir: Option<&Path>, + wait_for_cli_dev_token_fallback: bool, bearer_token: Option<&str>, ) -> Result { wait_for_server_ready(&build_unix_socket_probe_client(path)?).await?; - build_authed_unix_socket_http_client(path, storage_dir, bearer_token).await + build_authed_unix_socket_http_client(path, wait_for_cli_dev_token_fallback, bearer_token).await } -fn resolve_target_credential( +fn resolve_oauth_credential( target: &ServerTarget, - storage_dir: Option<&Path>, - allow_local_dev_token_fallback: bool, + store: &AuthStore, + now: chrono::DateTime, ) -> Result> { - if let Some(token) = std::env::var("FABRO_DEV_TOKEN") - .ok() - .filter(|token| validate_dev_token_format(token)) - { - return Ok(Some(Credential::DevToken(token))); - } - - let store = AuthStore::default(); if let Some(entry) = store.get(target)? { - let now = chrono::Utc::now(); if entry.access_token_expires_at > now || entry.refresh_token_expires_at > now { return Ok(Some(Credential::OAuth(entry))); } } + Ok(None) +} + +fn resolve_local_tcp_credential_with_store( + target: &ServerTarget, + env_token: Option<&str>, + store: &AuthStore, + now: chrono::DateTime, +) -> Result> { + if let Some(token) = env_token.filter(|token| validate_dev_token_format(token)) { + return Ok(Some(Credential::DevToken(token.to_owned()))); + } + + resolve_oauth_credential(target, store, now) +} + +fn resolve_local_tcp_credential(target: &ServerTarget) -> Result> { + let env_token = std::env::var("FABRO_DEV_TOKEN").ok(); + let store = AuthStore::default(); + resolve_local_tcp_credential_with_store(target, env_token.as_deref(), &store, chrono::Utc::now()) +} + +fn resolve_target_credential( + target: &ServerTarget, + allow_local_dev_token_fallback: bool, +) -> Result> { + let env_token = std::env::var("FABRO_DEV_TOKEN").ok(); + let store = AuthStore::default(); + if let Some(credential) = resolve_local_tcp_credential_with_store( + target, + env_token.as_deref(), + &store, + chrono::Utc::now(), + )? { + return Ok(Some(credential)); + } + if allow_local_dev_token_fallback { - return Ok(load_dev_token_if_available(storage_dir).map(Credential::DevToken)); + return Ok(load_cli_dev_token_from_sources(env_token.as_deref(), &Home::from_env()) + .map(Credential::DevToken)); } Ok(None) } +fn should_bypass_proxy_for_http_target(api_url: &str) -> bool { + let Ok(url) = fabro_http::Url::parse(api_url) else { + return false; + }; + let Some(host) = url.host_str() else { + return false; + }; + if host.eq_ignore_ascii_case("localhost") { + return true; + } + host.trim_matches(['[', ']']) + .parse::() + .is_ok_and(|ip| ip.is_loopback()) +} + async fn check_server_ready(http_client: &fabro_http::HttpClient) -> Result<()> { match http_client.get("http://fabro/health").send().await { Ok(response) if response.status().is_success() => Ok(()), @@ -366,12 +383,13 @@ async fn wait_for_server_ready(http_client: &fabro_http::HttpClient) -> Result<( reason = "server-client tests stage local dev-token fixtures with sync std::fs::write" )] mod tests { - use chrono::Utc; + use chrono::{Duration as ChronoDuration, Utc}; + use fabro_client::{AuthEntry, StoredSubject}; use super::*; #[test] - fn load_dev_token_if_available_prefers_env() { + fn load_cli_dev_token_prefers_env() { let temp_home = tempfile::tempdir().unwrap(); let token_path = temp_home.path().join("dev-token"); std::fs::write( @@ -380,8 +398,7 @@ mod tests { ) .unwrap(); - let token = load_dev_token_if_available_from_sources( - None, + let token = load_cli_dev_token_from_sources( Some("fabro_dev_cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd"), &Home::new(temp_home.path()), ); @@ -393,47 +410,121 @@ mod tests { } #[test] - fn load_dev_token_if_available_reads_file() { + fn load_cli_dev_token_reads_home_file() { let temp_home = tempfile::tempdir().unwrap(); let token = "fabro_dev_abababababababababababababababababababababababababababababababab"; std::fs::write(temp_home.path().join("dev-token"), token).unwrap(); - let loaded = - load_dev_token_if_available_from_sources(None, None, &Home::new(temp_home.path())); + let loaded = load_cli_dev_token_from_sources(None, &Home::new(temp_home.path())); assert_eq!(loaded.as_deref(), Some(token)); } #[test] - fn load_dev_token_if_available_reads_path_from_active_server_record() { - let temp_home = tempfile::tempdir().unwrap(); - let storage = tempfile::tempdir().unwrap(); - let token_dir = tempfile::tempdir().unwrap(); - let token = "fabro_dev_abababababababababababababababababababababababababababababababab"; - let token_path = token_dir.path().join("dev-token"); - std::fs::write(&token_path, token).unwrap(); + fn resolve_local_tcp_credential_prefers_valid_env_token() { + let target = ServerTarget::http_url("http://127.0.0.1:32276").unwrap(); + let store = AuthStore::new(tempfile::tempdir().unwrap().path().join("auth.json")); - let record_path = fabro_config::Storage::new(storage.path()) - .server_state() - .record_path(); - record::write_server_record(&record_path, &record::ServerRecord { - pid: std::process::id(), - bind: Bind::Unix(temp_home.path().join("fabro.sock")), - log_path: fabro_config::Storage::new(storage.path()) - .server_state() - .log_path(), - dev_token_path: Some(token_path), - started_at: Utc::now(), - }) + let credential = resolve_local_tcp_credential_with_store( + &target, + Some( + "fabro_dev_cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd", + ), + &store, + Utc::now(), + ) .unwrap(); - let loaded = load_dev_token_if_available_from_sources( - Some(storage.path()), - None, - &Home::new(temp_home.path()), - ); + assert!(matches!(credential, Some(Credential::DevToken(_)))); + } - assert_eq!(loaded.as_deref(), Some(token)); + #[test] + fn resolve_local_tcp_credential_uses_live_oauth_entry() { + let dir = tempfile::tempdir().unwrap(); + let target = ServerTarget::http_url("http://127.0.0.1:32276").unwrap(); + let store = AuthStore::new(dir.path().join("auth.json")); + let now = Utc::now(); + store + .put(&target, oauth_entry(now + ChronoDuration::minutes(5), now - ChronoDuration::minutes(1))) + .unwrap(); + + let credential = + resolve_local_tcp_credential_with_store(&target, None, &store, now).unwrap(); + + assert!(matches!(credential, Some(Credential::OAuth(_)))); + } + + #[test] + fn resolve_local_tcp_credential_uses_refreshable_oauth_entry() { + let dir = tempfile::tempdir().unwrap(); + let target = ServerTarget::http_url("http://127.0.0.1:32276").unwrap(); + let store = AuthStore::new(dir.path().join("auth.json")); + let now = Utc::now(); + store + .put( + &target, + oauth_entry( + now - ChronoDuration::minutes(1), + now + ChronoDuration::minutes(5), + ), + ) + .unwrap(); + + let credential = + resolve_local_tcp_credential_with_store(&target, None, &store, now).unwrap(); + + assert!(matches!(credential, Some(Credential::OAuth(_)))); + } + + #[test] + fn resolve_local_tcp_credential_ignores_expired_oauth_entry() { + let dir = tempfile::tempdir().unwrap(); + let target = ServerTarget::http_url("http://127.0.0.1:32276").unwrap(); + let store = AuthStore::new(dir.path().join("auth.json")); + let now = Utc::now(); + store + .put( + &target, + oauth_entry( + now - ChronoDuration::minutes(5), + now - ChronoDuration::minutes(1), + ), + ) + .unwrap(); + + let credential = + resolve_local_tcp_credential_with_store(&target, None, &store, now).unwrap(); + + assert!(credential.is_none()); + } + + #[test] + fn resolve_local_tcp_credential_does_not_fallback_to_home_dev_token() { + let temp_home = tempfile::tempdir().unwrap(); + std::fs::write( + temp_home.path().join("dev-token"), + "fabro_dev_abababababababababababababababababababababababababababababababab", + ) + .unwrap(); + let original_home = std::env::var_os("FABRO_HOME"); + std::env::set_var("FABRO_HOME", temp_home.path()); + let _guard = scopeguard::guard(original_home, |original_home| match original_home { + Some(value) => std::env::set_var("FABRO_HOME", value), + None => std::env::remove_var("FABRO_HOME"), + }); + + let target = ServerTarget::http_url("http://127.0.0.1:32276").unwrap(); + + assert!(resolve_local_tcp_credential(&target).unwrap().is_none()); + } + + #[test] + fn bypasses_proxy_for_loopback_http_targets() { + assert!(should_bypass_proxy_for_http_target("http://127.0.0.1:32276")); + assert!(should_bypass_proxy_for_http_target("http://[::1]:32276")); + assert!(should_bypass_proxy_for_http_target("http://localhost:32276")); + assert!(!should_bypass_proxy_for_http_target("https://fabro.example.com")); + assert!(!should_bypass_proxy_for_http_target("http://fabro.example.com")); } #[test] @@ -447,4 +538,24 @@ mod tests { let target = ServerTarget::unix_socket_path("/tmp/fabro.sock").unwrap(); assert!(local_dev_token_fallback(&target)); } + + fn oauth_entry( + access_token_expires_at: chrono::DateTime, + refresh_token_expires_at: chrono::DateTime, + ) -> AuthEntry { + AuthEntry { + access_token: "access-token".to_string(), + access_token_expires_at, + refresh_token: "refresh-token".to_string(), + refresh_token_expires_at, + subject: StoredSubject { + idp_issuer: "https://github.com/login/oauth".to_string(), + idp_subject: "subject-123".to_string(), + login: "octocat".to_string(), + name: "Octo Cat".to_string(), + email: "octocat@example.com".to_string(), + }, + logged_in_at: Utc::now(), + } + } } diff --git a/lib/crates/fabro-cli/src/user_config.rs b/lib/crates/fabro-cli/src/user_config.rs index 3fa3710da..378bb981b 100644 --- a/lib/crates/fabro-cli/src/user_config.rs +++ b/lib/crates/fabro-cli/src/user_config.rs @@ -84,25 +84,11 @@ pub(crate) fn default_server_target() -> ServerTarget { } pub(crate) fn storage_dir(settings: &SettingsLayer) -> anyhow::Result { - let resolved = fabro_config::resolve_server_from_file(settings).map_err(|errors| { - anyhow::anyhow!( - "failed to resolve server settings:\n{}", - errors - .into_iter() - .map(|error| error.to_string()) - .collect::>() - .join("\n") - ) - })?; - let resolved_root = resolved - .storage - .root + let storage_root = fabro_config::resolve_storage_root(settings); + let resolved_root = storage_root .resolve(|name| std::env::var(name).ok()) .map_err(|err| { - anyhow::anyhow!( - "failed to resolve {}: {err}", - resolved.storage.root.as_source() - ) + anyhow::anyhow!("failed to resolve {}: {err}", storage_root.as_source()) })?; Ok(PathBuf::from(resolved_root.value)) } @@ -259,4 +245,46 @@ url = "https://config.example.com" "server target must be an http(s) URL or absolute Unix socket path" ); } + + #[test] + fn storage_dir_defaults_without_server_auth_methods() { + let settings = SettingsLayer::default(); + + assert_eq!(storage_dir(&settings).unwrap(), fabro_config::user::default_storage_dir()); + } + + #[test] + fn storage_dir_uses_explicit_server_storage_root() { + let settings = parse_v2( + r#" +_version = 1 + +[server.storage] +root = "/srv/fabro" +"#, + ); + + assert_eq!(storage_dir(&settings).unwrap(), PathBuf::from("/srv/fabro")); + } + + #[test] + fn storage_dir_resolves_env_interpolated_root() { + let settings = parse_v2( + r#" +_version = 1 + +[server.storage] +root = "{{ env.FABRO_STORAGE_ROOT }}" +"#, + ); + let temp = tempfile::tempdir().unwrap(); + let original = std::env::var_os("FABRO_STORAGE_ROOT"); + std::env::set_var("FABRO_STORAGE_ROOT", temp.path()); + let _guard = scopeguard::guard(original, |original| match original { + Some(value) => std::env::set_var("FABRO_STORAGE_ROOT", value), + None => std::env::remove_var("FABRO_STORAGE_ROOT"), + }); + + assert_eq!(storage_dir(&settings).unwrap(), temp.path()); + } } diff --git a/lib/crates/fabro-cli/tests/it/cmd/config.rs b/lib/crates/fabro-cli/tests/it/cmd/config.rs index d80fa1169..b11f8c5a0 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/config.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/config.rs @@ -138,6 +138,9 @@ fn server_settings_layer_fixture() -> SettingsLayer { r#" _version = 1 +[server.auth] +methods = ["dev-token"] + [server.storage] root = "/srv/fabro-server" @@ -207,6 +210,9 @@ SHARED = "cli" [run.sandbox.daytona.labels] cli_only = "1" shared = "cli" + +[server.auth] +methods = ["dev-token"] "#, ); @@ -311,6 +317,9 @@ fn setup_external_workflow_fixture( r#" _version = 1 +[server.auth] +methods = ["dev-token"] + [server.storage] root = "{}" @@ -533,6 +542,7 @@ fn settings_local_explicit_workflow_path_uses_workflow_project_layers() { fn create_explicit_workflow_path_uses_project_config_relative_to_workflow() { let mut context = test_context!(); let (project, storage_dir) = setup_external_workflow_fixture(&mut context); + context.ensure_home_server_auth_methods(); let cwd = tempfile::tempdir().unwrap(); let workflow = project.path().join("workflow.toml"); let run_id = unique_run_id(); @@ -672,11 +682,9 @@ name = "legacy-model" .assert() .success(); - assert!( - assert.get_output().stderr.is_empty(), - "settings should not warn about legacy config files: {}", - String::from_utf8_lossy(&assert.get_output().stderr) - ); + let stderr = String::from_utf8_lossy(&assert.get_output().stderr); + assert!(stderr.contains("warning: server config:")); + assert!(stderr.contains("server.auth.methods")); let cfg = parse_settings(&assert.get_output().stdout); assert_eq!(cfg["cli"]["output"]["verbosity"].as_str(), Some("normal")); @@ -711,11 +719,9 @@ name = "legacy-model" .assert() .success(); - assert!( - assert.get_output().stderr.is_empty(), - "settings should not warn about legacy config files: {}", - String::from_utf8_lossy(&assert.get_output().stderr) - ); + let stderr = String::from_utf8_lossy(&assert.get_output().stderr); + assert!(stderr.contains("warning: server config:")); + assert!(stderr.contains("server.auth.methods")); let cfg = parse_settings(&assert.get_output().stdout); assert_eq!(cfg["cli"]["output"]["verbosity"].as_str(), Some("normal")); @@ -750,11 +756,9 @@ name = "legacy-model" .assert() .success(); - assert!( - assert.get_output().stderr.is_empty(), - "settings should not warn about legacy config files: {}", - String::from_utf8_lossy(&assert.get_output().stderr) - ); + let stderr = String::from_utf8_lossy(&assert.get_output().stderr); + assert!(stderr.contains("warning: server config:")); + assert!(stderr.contains("server.auth.methods")); let cfg = parse_settings(&assert.get_output().stdout); assert_eq!(cfg["cli"]["output"]["verbosity"].as_str(), Some("normal")); diff --git a/lib/crates/fabro-cli/tests/it/cmd/create.rs b/lib/crates/fabro-cli/tests/it/cmd/create.rs index 05386a24d..d09845eaa 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/create.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/create.rs @@ -199,6 +199,7 @@ fn create_cli_server_target_overrides_configured_server_target() { #[test] fn create_persists_directory_workflow_slug_and_cached_graph() { let context = test_context!(); + context.ensure_home_server_auth_methods(); let run_id = unique_run_id(); let workflow_path = context.temp_dir.join("sluggy/workflow.fabro"); @@ -255,6 +256,7 @@ digraph BarBaz { #[test] fn create_persists_file_stem_slug_for_standalone_file() { let context = test_context!(); + context.ensure_home_server_auth_methods(); let run_id = unique_run_id(); let workflow_path = context.temp_dir.join("alpha.fabro"); @@ -311,6 +313,7 @@ digraph FooWorkflow { #[test] fn create_persists_requested_overrides_into_store() { let context = test_context!(); + context.ensure_home_server_auth_methods(); let workflow = fixture("simple.fabro"); let mut cmd = context.command(); cmd.args([ @@ -411,6 +414,7 @@ fn create_persists_requested_overrides_into_store() { #[test] fn create_json_does_not_imply_auto_approve() { let context = test_context!(); + context.ensure_home_server_auth_methods(); let workflow = fixture("simple.fabro"); let output = context .command() diff --git a/lib/crates/fabro-cli/tests/it/cmd/ps.rs b/lib/crates/fabro-cli/tests/it/cmd/ps.rs index f9c9ad498..bbeacc839 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/ps.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/ps.rs @@ -1,10 +1,26 @@ +use fabro_config::{Storage, envfile}; use fabro_test::{fabro_snapshot, test_context}; +use fabro_util::dev_token; use httpmock::MockServer; use serde_json::Value; use super::support::{local_dev_token, setup_completed_fast_dry_run, setup_created_fast_dry_run}; use crate::support::unique_run_id; +const TEST_DEV_TOKEN: &str = + "fabro_dev_abababababababababababababababababababababababababababababababab"; + +fn provision_local_server_auth(context: &fabro_test::TestContext, storage_dir: &std::path::Path) { + context.ensure_home_server_auth_methods(); + let server_env_path = Storage::new(storage_dir).server_state().env_path(); + envfile::merge_env_file(&server_env_path, [("FABRO_DEV_TOKEN", TEST_DEV_TOKEN)]).unwrap(); + dev_token::write_dev_token( + &context.home_dir.join(".fabro").join("dev-token"), + TEST_DEV_TOKEN, + ) + .unwrap(); +} + #[test] fn help() { let context = test_context!(); @@ -41,6 +57,7 @@ fn ps_explicit_local_tcp_server_target_requires_explicit_auth() { let storage_root = tempfile::tempdir_in("/tmp").unwrap(); let storage_dir = storage_root.path().join("storage"); std::fs::create_dir_all(&storage_dir).unwrap(); + provision_local_server_auth(&context, &storage_dir); context .command() @@ -97,6 +114,7 @@ fn ps_explicit_local_tcp_server_target_accepts_explicit_dev_token() { let storage_root = tempfile::tempdir_in("/tmp").unwrap(); let storage_dir = storage_root.path().join("storage"); std::fs::create_dir_all(&storage_dir).unwrap(); + provision_local_server_auth(&context, &storage_dir); context .command() diff --git a/lib/crates/fabro-cli/tests/it/cmd/run.rs b/lib/crates/fabro-cli/tests/it/cmd/run.rs index ddcb9e400..f4b45bf1c 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/run.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/run.rs @@ -275,6 +275,7 @@ fn detach_uses_configured_server_target_without_server_flag() { #[test] fn run_uses_vault_credentials_for_worker_execution() { let mut context = test_context!(); + context.write_home(".fabro/settings.toml", "[server.auth]\nmethods = [\"dev-token\"]\n"); context.isolated_server(); let run_id = unique_run_id(); let llm_server = MockServer::start(); @@ -733,6 +734,7 @@ fn dry_run_rejects_goal_and_goal_file_together() { #[test] fn dry_run_persists_event_history_in_store() { let context = test_context!(); + context.ensure_home_server_auth_methods(); let run_id = unique_run_id(); let workflow = context.install_fixture("simple.fabro"); @@ -832,6 +834,7 @@ fn dry_run_persists_event_history_in_store() { #[test] fn run_id_passthrough_uses_provided_ulid() { let context = test_context!(); + context.ensure_home_server_auth_methods(); let run_id = unique_run_id(); let workflow = context.install_fixture("simple.fabro"); @@ -854,6 +857,7 @@ fn run_id_passthrough_uses_provided_ulid() { #[test] fn json_run_requires_manual_input_for_human_gates_without_auto_approve() { let context = test_context!(); + context.ensure_home_server_auth_methods(); let workflow = context.temp_dir.join("human-gate.fabro"); context.write_temp( "human-gate.fabro", diff --git a/lib/crates/fabro-cli/tests/it/cmd/runner.rs b/lib/crates/fabro-cli/tests/it/cmd/runner.rs index 2acafba9c..3adbb0e89 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/runner.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/runner.rs @@ -24,6 +24,12 @@ use crate::support::{fabro_json_snapshot, unique_run_id}; const SHARED_DAEMON_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); +fn auth_context() -> fabro_test::TestContext { + let context = test_context!(); + context.ensure_home_server_auth_methods(); + context +} + fn stored_worker_events(run_dir: &std::path::Path) -> Vec { run_events(run_dir).iter().map(run_event).collect() } @@ -180,7 +186,7 @@ fn help() { #[test] fn runner_uses_cached_graph_after_source_deleted() { - let context = test_context!(); + let context = auth_context(); let run_id = unique_run_id(); let workflow_path = context.temp_dir.join("workflow.fabro"); @@ -236,7 +242,7 @@ digraph CachedGraph { #[test] fn runner_uses_snapshotted_app_id_for_github_credentials() { - let context = test_context!(); + let context = auth_context(); let run_id = unique_run_id(); let workflow_path = context.temp_dir.join("workflow.fabro"); @@ -245,6 +251,9 @@ fn runner_uses_snapshotted_app_id_for_github_credentials() { "\ _version = 1 +[server.auth] +methods = [\"dev-token\"] + [server.integrations.github] app_id = \"snapshotted-app-id\" ", @@ -312,7 +321,7 @@ digraph GitHubApp { #[test] fn runner_runs_without_run_json_when_run_id_is_explicit() { - let context = test_context!(); + let context = auth_context(); let run_id = unique_run_id(); let workflow_path = context.temp_dir.join("workflow.fabro"); @@ -366,7 +375,7 @@ digraph DetachedStoreOnly { #[test] fn runner_resume_rejects_completed_run_without_mutating_it() { - let context = test_context!(); + let context = auth_context(); context.write_temp( "workflow.fabro", "\ @@ -466,7 +475,7 @@ digraph Test { #[test] fn runner_reports_missing_run_spec_without_prefetching_events() { - let context = test_context!(); + let context = auth_context(); let server = MockServer::start(); let run_id = unique_run_id(); let run_dir = tempfile::tempdir().expect("temp run dir should exist"); @@ -538,7 +547,7 @@ fn runner_reports_missing_run_spec_without_prefetching_events() { #[test] fn detached_run_answers_pending_question_without_interview_scratch_files() { - let context = test_context!(); + let context = auth_context(); let run_id = unique_run_id(); let workflow_path = context.temp_dir.join("human-gate.fabro"); @@ -631,7 +640,7 @@ fn detached_run_answers_pending_question_without_interview_scratch_files() { #[test] fn worker_exits_with_retro_enabled_even_when_stdin_stays_open() { - let context = test_context!(); + let context = auth_context(); let run_id = unique_run_id(); let workflow_path = context.temp_dir.join("retro-success.fabro"); @@ -703,7 +712,7 @@ retros = true #[cfg(unix)] #[test] fn worker_exits_after_sigterm_cancel_even_when_stdin_stays_open() { - let context = test_context!(); + let context = auth_context(); let run_id = unique_run_id(); let workflow_path = context.temp_dir.join("cancel-gated.fabro"); let _gate = write_gated_workflow(&workflow_path, "cancel_gated", "Wait for cancellation"); 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 369e1c664..6d1abebed 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs @@ -12,10 +12,33 @@ use std::process::Stdio; use std::sync::{Arc, Barrier}; use std::time::{Duration, Instant}; +use fabro_config::{Storage, envfile}; use fabro_test::{ apply_test_isolation, fabro_snapshot, isolated_storage_dir, server_log_files, stop_pid, test_context, wait_for_log_line, wait_for_path, }; +use fabro_util::dev_token; + +const TEST_DEV_TOKEN: &str = + "fabro_dev_abababababababababababababababababababababababababababababababab"; + +fn write_dev_token_server_settings(config_path: &std::path::Path, rest: &str) { + std::fs::write( + config_path, + format!("_version = 1\n\n[server.auth]\nmethods = [\"dev-token\"]\n\n{rest}"), + ) + .unwrap(); +} + +fn provision_dev_token_auth(home_dir: &std::path::Path, storage_dir: &std::path::Path) { + let server_env_path = Storage::new(storage_dir).server_state().env_path(); + envfile::merge_env_file(&server_env_path, [("FABRO_DEV_TOKEN", TEST_DEV_TOKEN)]).unwrap(); + dev_token::write_dev_token( + &home_dir.join(".fabro").join("dev-token"), + TEST_DEV_TOKEN, + ) + .unwrap(); +} #[test] fn help() { @@ -80,6 +103,8 @@ fn start_already_running_exits_with_error() { let context = test_context!(); let storage_root = isolated_storage_dir(); let storage_dir = storage_root.path().join("storage"); + context.write_home(".fabro/settings.toml", "[server.auth]\nmethods = [\"dev-token\"]\n"); + provision_dev_token_auth(&context.home_dir, &storage_dir); let sock_dir = tempfile::tempdir_in("/tmp").unwrap(); let bind_addr = sock_dir.path().join("test.sock"); @@ -170,7 +195,8 @@ fn foreground_start_writes_tracing_to_storage_server_log() { let socket_path = storage_root.path().join("foreground.sock"); let config_dir = tempfile::tempdir_in("/tmp").unwrap(); let config_path = config_dir.path().join("settings.toml"); - std::fs::write(&config_path, "_version = 1\n").unwrap(); + write_dev_token_server_settings(&config_path, ""); + provision_dev_token_auth(home_dir.path(), &storage_dir); let storage_log_path = storage_dir.join("logs").join("server.log"); std::fs::create_dir_all(storage_log_path.parent().unwrap()).unwrap(); std::fs::write(&storage_log_path, "stale pre-start log entry\n").unwrap(); @@ -277,7 +303,8 @@ fn daemon_start_writes_tracing_to_storage_server_log() { let socket_path = storage_root.path().join("daemon.sock"); let config_dir = tempfile::tempdir_in("/tmp").unwrap(); let config_path = config_dir.path().join("settings.toml"); - std::fs::write(&config_path, "_version = 1\n").unwrap(); + write_dev_token_server_settings(&config_path, ""); + provision_dev_token_auth(&context.home_dir, &storage_dir); let storage_log_path = storage_dir.join("logs").join("server.log"); std::fs::create_dir_all(storage_log_path.parent().unwrap()).unwrap(); std::fs::write(&storage_log_path, "stale pre-start log entry\n").unwrap(); @@ -351,7 +378,8 @@ fn start_errors_when_only_a_legacy_running_server_record_exists() { let socket_path = home_dir.path().join("legacy.sock"); let config_dir = tempfile::tempdir_in("/tmp").unwrap(); let config_path = config_dir.path().join("settings.toml"); - std::fs::write(&config_path, "_version = 1\n").unwrap(); + write_dev_token_server_settings(&config_path, ""); + provision_dev_token_auth(home_dir.path(), &storage_dir); let start_output = { let mut start = std::process::Command::new(env!("CARGO_BIN_EXE_fabro")); @@ -431,7 +459,8 @@ fn concurrent_foreground_start_does_not_retruncate_storage_server_log() { let second_socket_path = storage_root.path().join("foreground-second.sock"); let config_dir = tempfile::tempdir_in("/tmp").unwrap(); let config_path = config_dir.path().join("settings.toml"); - std::fs::write(&config_path, "_version = 1\n").unwrap(); + write_dev_token_server_settings(&config_path, ""); + provision_dev_token_auth(home_dir.path(), &storage_dir); let storage_log_path = storage_dir.join("logs").join("server.log"); std::fs::create_dir_all(storage_log_path.parent().unwrap()).unwrap(); @@ -670,6 +699,8 @@ fn start_without_bind_uses_home_socket_instead_of_storage_socket() { let context = test_context!(); let storage_root = isolated_storage_dir(); let storage_dir = storage_root.path().join("storage"); + context.write_home(".fabro/settings.toml", "[server.auth]\nmethods = [\"dev-token\"]\n"); + provision_dev_token_auth(&context.home_dir, &storage_dir); let expected_socket = context.home_dir.join(".fabro").join("fabro.sock"); let storage_socket = storage_dir.join("fabro.sock"); @@ -709,17 +740,14 @@ fn start_without_bind_uses_configured_tcp_listen_address() { let storage_dir = storage_root.path().join("storage"); let config_dir = tempfile::tempdir_in("/tmp").unwrap(); let config_path = config_dir.path().join("settings.toml"); - std::fs::write( + write_dev_token_server_settings( &config_path, - r#" -_version = 1 - -[server.listen] + r#"[server.listen] type = "tcp" address = "127.0.0.1:0" "#, - ) - .unwrap(); + ); + provision_dev_token_auth(&context.home_dir, &storage_dir); let mut cmd = context.command(); cmd.env("FABRO_STORAGE_DIR", &storage_dir); @@ -766,6 +794,8 @@ fn start_with_tcp_host_only_bind_resolves_to_host_and_port() { let context = test_context!(); let storage_root = isolated_storage_dir(); let storage_dir = storage_root.path().join("storage"); + context.write_home(".fabro/settings.toml", "[server.auth]\nmethods = [\"dev-token\"]\n"); + provision_dev_token_auth(&context.home_dir, &storage_dir); let mut cmd = context.command(); cmd.env("FABRO_STORAGE_DIR", &storage_dir); @@ -816,6 +846,7 @@ fn start_with_tcp_host_only_bind_warns_and_falls_back_when_default_port_is_unava let context = test_context!(); let storage_root = isolated_storage_dir(); let storage_dir = storage_root.path().join("storage"); + context.write_home(".fabro/settings.toml", "[server.auth]\nmethods = [\"dev-token\"]\n"); let occupied = match std::net::TcpListener::bind(("127.0.0.1", 32276)) { Ok(listener) => listener, Err(error) if error.kind() == std::io::ErrorKind::AddrInUse => { @@ -827,10 +858,7 @@ fn start_with_tcp_host_only_bind_warns_and_falls_back_when_default_port_is_unava let mut filters = context.filters(); 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())); - filters.push(( - r"fabro_dev_[0-9a-f]{64}".to_string(), - "fabro_dev_[DEV_TOKEN]".to_string(), - )); + provision_dev_token_auth(&context.home_dir, &storage_dir); let mut cmd = context.command(); cmd.env("FABRO_STORAGE_DIR", &storage_dir); @@ -844,8 +872,6 @@ fn start_with_tcp_host_only_bind_warns_and_falls_back_when_default_port_is_unava Server started (pid [PID]) on [TCP_BIND] Web UI: http://[TCP_BIND] Auth: dev-token - Dev token: fabro_dev_[DEV_TOKEN] - Token file: [HOME_DIR]/.fabro/dev-token "); let output = context @@ -928,6 +954,7 @@ fn default_test_context_server_keeps_object_store_off_disk() { #[test] fn isolated_server_switches_context_to_separate_daemon() { let mut context = test_context!(); + context.write_home(".fabro/settings.toml", "[server.auth]\nmethods = [\"dev-token\"]\n"); let shared_storage_dir = context.storage_dir.clone(); let shared_status = context .command() @@ -1004,7 +1031,7 @@ fn concurrent_autostart_converges_on_one_shared_daemon_and_cleans_up() { std::fs::write( &config_path, format!( - "_version = 1\n\n[server.storage]\nroot = \"{}\"\n\n[cli.target]\ntype = \"unix\"\npath = \"{}\"\n", + "_version = 1\n\n[server.auth]\nmethods = [\"dev-token\"]\n\n[server.storage]\nroot = \"{}\"\n\n[cli.target]\ntype = \"unix\"\npath = \"{}\"\n", storage_dir.display(), socket_path.display() ), @@ -1012,6 +1039,8 @@ fn concurrent_autostart_converges_on_one_shared_daemon_and_cleans_up() { .unwrap(); let home_a = tempfile::tempdir_in("/tmp").unwrap(); let home_b = tempfile::tempdir_in("/tmp").unwrap(); + provision_dev_token_auth(home_a.path(), &storage_dir); + provision_dev_token_auth(home_b.path(), &storage_dir); let temp_a = tempfile::tempdir_in("/tmp").unwrap(); let temp_b = tempfile::tempdir_in("/tmp").unwrap(); diff --git a/lib/crates/fabro-cli/tests/it/cmd/support.rs b/lib/crates/fabro-cli/tests/it/cmd/support.rs index 04b470048..0bc052f30 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/support.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/support.rs @@ -14,7 +14,7 @@ use std::path::{Path, PathBuf}; use std::process::Output; use std::time::{Duration, Instant}; -use fabro_config::Storage; +use fabro_config::{Storage, envfile}; use fabro_server::bind::Bind; use fabro_store::EventEnvelope; use fabro_test::{TestContext, expect_reqwest_status}; @@ -660,22 +660,16 @@ fn block_on(future: impl std::future::Future) -> T { #[derive(Debug, serde::Deserialize)] struct TestServerRecord { - bind: Bind, - #[serde(default)] - dev_token_path: Option, + bind: Bind, } pub(crate) fn local_dev_token(storage_dir: &Path) -> Option { let server_state = Storage::new(storage_dir).server_state(); - fabro_util::dev_token::read_dev_token_file(&server_state.dev_token_path()).or_else(|| { - std::fs::read_to_string(server_state.record_path()) - .ok() - .and_then(|content| serde_json::from_str::(&content).ok()) - .and_then(|record| record.dev_token_path) - .as_deref() - .and_then(fabro_util::dev_token::read_dev_token_file) - }) + envfile::read_env_file(&server_state.env_path()) + .ok() + .and_then(|entries| entries.get("FABRO_DEV_TOKEN").cloned()) + .or_else(|| fabro_util::dev_token::read_dev_token_file(&server_state.dev_token_path())) } pub(crate) fn server_endpoint(storage_dir: &Path) -> Option<(fabro_http::HttpClient, String)> { diff --git a/lib/crates/fabro-cli/tests/it/scenario/server_lifecycle.rs b/lib/crates/fabro-cli/tests/it/scenario/server_lifecycle.rs index 78872bc63..c40a0199c 100644 --- a/lib/crates/fabro-cli/tests/it/scenario/server_lifecycle.rs +++ b/lib/crates/fabro-cli/tests/it/scenario/server_lifecycle.rs @@ -6,6 +6,21 @@ fn start_status_stop_lifecycle() { let storage_root = tempfile::tempdir_in("/tmp").unwrap(); let storage_dir = storage_root.path().join("storage"); std::fs::create_dir_all(&storage_dir).unwrap(); + context.write_home(".fabro/settings.toml", "[server.auth]\nmethods = [\"dev-token\"]\n"); + let server_env_path = fabro_config::Storage::new(&storage_dir).server_state().env_path(); + fabro_config::envfile::merge_env_file( + &server_env_path, + [( + "FABRO_DEV_TOKEN", + "fabro_dev_abababababababababababababababababababababababababababababababab", + )], + ) + .unwrap(); + fabro_util::dev_token::write_dev_token( + &context.home_dir.join(".fabro").join("dev-token"), + "fabro_dev_abababababababababababababababababababababababababababababababab", + ) + .unwrap(); let sock_dir = tempfile::tempdir_in("/tmp").unwrap(); let bind_addr = sock_dir.path().join("test.sock"); @@ -33,8 +48,6 @@ fn start_status_stop_lifecycle() { ----- stderr ----- Server started (pid [PID]) on [SOCKET_PATH] Auth: dev-token - Dev token: fabro_dev_[DEV_TOKEN] - Token file: [HOME_DIR]/.fabro/dev-token "); let mut cmd = context.command(); diff --git a/lib/crates/fabro-config/src/defaults.toml b/lib/crates/fabro-config/src/defaults.toml index c83e3a7f7..e7aca7a25 100644 --- a/lib/crates/fabro-config/src/defaults.toml +++ b/lib/crates/fabro-config/src/defaults.toml @@ -38,9 +38,6 @@ check = true enabled = true url = "http://localhost:3000" -[server.auth] -methods = ["dev-token"] - [server.scheduler] max_concurrent_runs = 5 diff --git a/lib/crates/fabro-config/src/lib.rs b/lib/crates/fabro-config/src/lib.rs index 08f25ac21..f36b259d6 100644 --- a/lib/crates/fabro-config/src/lib.rs +++ b/lib/crates/fabro-config/src/lib.rs @@ -32,10 +32,10 @@ pub use load::{ }; pub use parse::{ParseError, parse_settings_layer}; pub use resolve::{ - ResolveError, resolve, resolve_cli, resolve_cli_from_file, resolve_features, - resolve_features_from_file, resolve_project, resolve_project_from_file, resolve_run, - resolve_run_from_file, resolve_server, resolve_server_from_file, resolve_workflow, - resolve_workflow_from_file, + ResolveError, dev_token_auth_enabled, resolve, resolve_cli, resolve_cli_from_file, + resolve_features, resolve_features_from_file, resolve_project, resolve_project_from_file, + resolve_run, resolve_run_from_file, resolve_server, resolve_server_from_file, + resolve_storage_root, resolve_workflow, resolve_workflow_from_file, }; use serde::de::DeserializeOwned; pub use storage::{RunScratch, ServerState, Storage}; diff --git a/lib/crates/fabro-config/src/resolve/mod.rs b/lib/crates/fabro-config/src/resolve/mod.rs index b0ac5c82b..9032464a0 100644 --- a/lib/crates/fabro-config/src/resolve/mod.rs +++ b/lib/crates/fabro-config/src/resolve/mod.rs @@ -15,7 +15,7 @@ use fabro_types::settings::{ pub use features::resolve_features; pub use project::resolve_project; pub use run::resolve_run; -pub use server::resolve_server; +pub use server::{dev_token_auth_enabled, resolve_server, resolve_storage_root}; pub use workflow::resolve_workflow; use crate::apply_builtin_defaults; @@ -47,33 +47,81 @@ pub fn resolve(file: &SettingsLayer) -> Result> { } pub fn resolve_cli_from_file(file: &SettingsLayer) -> Result> { - resolve(file).map(|settings| settings.cli) + let file = apply_builtin_defaults(file.clone()); + let mut errors = Vec::new(); + let cli_layer = file.cli.clone().unwrap_or_default(); + let cli = resolve_cli(&cli_layer, &mut errors); + if errors.is_empty() { + Ok(cli) + } else { + Err(errors) + } } pub fn resolve_server_from_file(file: &SettingsLayer) -> Result> { - resolve(file).map(|settings| settings.server) + let file = apply_builtin_defaults(file.clone()); + let mut errors = Vec::new(); + let server_layer = file.server.clone().unwrap_or_default(); + let server = resolve_server(&server_layer, &mut errors); + if errors.is_empty() { + Ok(server) + } else { + Err(errors) + } } pub fn resolve_project_from_file( file: &SettingsLayer, ) -> Result> { - resolve(file).map(|settings| settings.project) + let file = apply_builtin_defaults(file.clone()); + let mut errors = Vec::new(); + let project_layer = file.project.clone().unwrap_or_default(); + let project = resolve_project(&project_layer, &mut errors); + if errors.is_empty() { + Ok(project) + } else { + Err(errors) + } } pub fn resolve_features_from_file( file: &SettingsLayer, ) -> Result> { - resolve(file).map(|settings| settings.features) + let file = apply_builtin_defaults(file.clone()); + let mut errors = Vec::new(); + let features_layer = file.features.clone().unwrap_or_default(); + let features = resolve_features(&features_layer, &mut errors); + if errors.is_empty() { + Ok(features) + } else { + Err(errors) + } } pub fn resolve_run_from_file(file: &SettingsLayer) -> Result> { - resolve(file).map(|settings| settings.run) + let file = apply_builtin_defaults(file.clone()); + let mut errors = Vec::new(); + let run_layer = file.run.clone().unwrap_or_default(); + let run = resolve_run(&run_layer, &mut errors); + if errors.is_empty() { + Ok(run) + } else { + Err(errors) + } } pub fn resolve_workflow_from_file( file: &SettingsLayer, ) -> Result> { - resolve(file).map(|settings| settings.workflow) + let file = apply_builtin_defaults(file.clone()); + let mut errors = Vec::new(); + let workflow_layer = file.workflow.clone().unwrap_or_default(); + let workflow = resolve_workflow(&workflow_layer, &mut errors); + if errors.is_empty() { + Ok(workflow) + } else { + Err(errors) + } } pub(crate) fn require_interp( @@ -126,6 +174,9 @@ mod tests { r#" _version = 1 +[server.auth] +methods = ["dev-token"] + [run.agent.mcps.stdio] type = "stdio" command = ["fabro-mcp", "--stdio"] diff --git a/lib/crates/fabro-config/src/resolve/server.rs b/lib/crates/fabro-config/src/resolve/server.rs index 4084f8f9a..0b3a38452 100644 --- a/lib/crates/fabro-config/src/resolve/server.rs +++ b/lib/crates/fabro-config/src/resolve/server.rs @@ -17,6 +17,24 @@ use fabro_util::Home; use super::{ResolveError, default_interp, parse_socket_addr, require_interp}; use crate::user::default_storage_dir; +pub fn resolve_storage_root(file: &fabro_types::settings::SettingsLayer) -> InterpString { + let file = crate::apply_builtin_defaults(file.clone()); + file.server + .as_ref() + .and_then(|server| server.storage.as_ref()) + .and_then(|storage| storage.root.clone()) + .unwrap_or_else(|| default_interp(default_storage_dir())) +} + +pub fn dev_token_auth_enabled(layer: &fabro_types::settings::SettingsLayer) -> bool { + layer + .server + .as_ref() + .and_then(|server| server.auth.as_ref()) + .and_then(|auth| auth.methods.as_ref()) + .is_some_and(|methods| methods.contains(&ServerAuthMethod::DevToken)) +} + pub fn resolve_server(layer: &ServerLayer, errors: &mut Vec) -> ServerSettings { let storage = resolve_storage(layer.storage.as_ref()); let listen = resolve_listen(layer.listen.as_ref(), errors); @@ -106,16 +124,24 @@ fn resolve_auth( layer: Option<&ServerAuthLayer>, errors: &mut Vec, ) -> ServerAuthSettings { - let mut methods = layer - .and_then(|auth| auth.methods.clone()) - .unwrap_or_else(|| vec![ServerAuthMethod::DevToken]); - if methods.is_empty() { - errors.push(ResolveError::Invalid { - path: "server.auth.methods".to_string(), - reason: "must not be empty".to_string(), - }); - } - methods.dedup(); + let methods = match layer.and_then(|auth| auth.methods.clone()) { + Some(mut methods) => { + if methods.is_empty() { + errors.push(ResolveError::Invalid { + path: "server.auth.methods".to_string(), + reason: "must not be empty".to_string(), + }); + } + methods.dedup(); + methods + } + None => { + errors.push(ResolveError::Missing { + path: "server.auth.methods".to_string(), + }); + Vec::new() + } + }; let github = layer .and_then(|auth| auth.github.as_ref()) diff --git a/lib/crates/fabro-config/tests/defaults.rs b/lib/crates/fabro-config/tests/defaults.rs index cdeaf5a10..ff67dc914 100644 --- a/lib/crates/fabro-config/tests/defaults.rs +++ b/lib/crates/fabro-config/tests/defaults.rs @@ -90,12 +90,15 @@ fn apply_builtin_defaults_materializes_expected_layer() { } #[test] -fn resolve_empty_settings_still_produces_valid_defaults() { - let settings = resolve(&SettingsLayer::default()).expect("empty settings should resolve"); +fn resolve_empty_settings_requires_explicit_server_auth_methods() { + let errors = resolve(&SettingsLayer::default()).expect_err("empty settings should fail"); - assert_eq!(settings.project.directory, "."); - assert_eq!(settings.workflow.graph, "workflow.fabro"); - assert_eq!(settings.run.execution.mode, RunMode::Normal); + assert!(errors.iter().any(|error| { + matches!( + error, + fabro_config::ResolveError::Missing { path } if path == "server.auth.methods" + ) + })); } #[test] @@ -104,6 +107,9 @@ fn higher_precedence_values_override_builtin_defaults() { r#" _version = 1 +[server.auth] +methods = ["dev-token"] + [run.execution] mode = "dry_run" "#, diff --git a/lib/crates/fabro-config/tests/resolve_root.rs b/lib/crates/fabro-config/tests/resolve_root.rs index 0048eb4f3..b6176c6ad 100644 --- a/lib/crates/fabro-config/tests/resolve_root.rs +++ b/lib/crates/fabro-config/tests/resolve_root.rs @@ -7,16 +7,16 @@ fn parse(source: &str) -> SettingsLayer { } #[test] -fn resolves_root_settings_defaults() { - let settings = - fabro_config::resolve(&SettingsLayer::default()).expect("empty settings should resolve"); +fn resolves_root_settings_require_explicit_server_auth_methods() { + let errors = + fabro_config::resolve(&SettingsLayer::default()).expect_err("empty settings should fail"); - assert_eq!(settings.project.directory, "."); - assert_eq!(settings.workflow.graph, "workflow.fabro"); - assert!(settings.run.execution.retros); - assert!(settings.cli.updates.check); - assert_eq!(settings.server.scheduler.max_concurrent_runs, 5); - assert!(!settings.features.session_sandboxes); + assert!(errors.iter().any(|error| { + matches!( + error, + fabro_config::ResolveError::Missing { path } if path == "server.auth.methods" + ) + })); } #[test] @@ -80,6 +80,9 @@ _version = 1 [server.storage] root = "/srv/fabro" +[server.auth] +methods = ["dev-token"] + [run.model] provider = "openai" name = "gpt-5" diff --git a/lib/crates/fabro-config/tests/resolve_server.rs b/lib/crates/fabro-config/tests/resolve_server.rs index 72a670574..520b09ef5 100644 --- a/lib/crates/fabro-config/tests/resolve_server.rs +++ b/lib/crates/fabro-config/tests/resolve_server.rs @@ -2,18 +2,39 @@ use fabro_config::parse_settings_layer; use fabro_config::user::default_storage_dir; use fabro_types::settings::server::{ GithubIntegrationStrategy, IpAllowEntry, ObjectStoreSettings, ServerListenSettings, + ServerAuthMethod, }; use fabro_types::settings::{InterpString, SettingsLayer}; use fabro_util::Home; fn parse(source: &str) -> SettingsLayer { - parse_settings_layer(source).expect("fixture should parse") + let mut layer = parse_settings_layer(source).expect("fixture should parse"); + if layer + .server + .as_ref() + .and_then(|server| server.auth.as_ref()) + .and_then(|auth| auth.methods.as_ref()) + .is_none() + { + let server = layer.server.get_or_insert_with(Default::default); + let auth = server.auth.get_or_insert_with(Default::default); + auth.methods = Some(vec![ServerAuthMethod::DevToken]); + } + layer +} + +fn empty_settings_with_auth_methods() -> SettingsLayer { + parse( + r#" +_version = 1 +"#, + ) } #[test] fn resolves_server_defaults_from_empty_settings() { - let settings = fabro_config::resolve_server_from_file(&SettingsLayer::default()) - .expect("empty settings should resolve"); + let settings = fabro_config::resolve_server_from_file(&empty_settings_with_auth_methods()) + .expect("server settings should resolve"); assert_eq!( settings.storage.root.as_source(), @@ -207,8 +228,8 @@ disk_cache = true #[test] fn resolves_empty_ip_allowlist_by_default() { - let settings = fabro_config::resolve_server_from_file(&SettingsLayer::default()) - .expect("empty settings should resolve"); + let settings = fabro_config::resolve_server_from_file(&empty_settings_with_auth_methods()) + .expect("server settings should resolve"); assert!(settings.ip_allowlist.entries.is_empty()); assert_eq!(settings.ip_allowlist.trusted_proxy_count, 0); @@ -445,3 +466,75 @@ entries = ["github_meta_hooks"] rendered.contains("server.integrations.github.webhooks.ip_allowlist.trusted_proxy_count") ); } + +#[test] +fn resolve_storage_root_defaults_without_server_auth_methods() { + assert_eq!( + fabro_config::resolve_storage_root(&SettingsLayer::default()).as_source(), + default_storage_dir().to_string_lossy() + ); +} + +#[test] +fn resolve_storage_root_prefers_explicit_root() { + let file = parse( + r#" +_version = 1 + +[server.storage] +root = "/srv/fabro" +"#, + ); + + assert_eq!(fabro_config::resolve_storage_root(&file).as_source(), "/srv/fabro"); +} + +#[test] +fn resolve_storage_root_preserves_env_interpolation() { + let file = parse( + r#" +_version = 1 + +[server.storage] +root = "{{ env.FABRO_STORAGE_ROOT }}" +"#, + ); + + assert_eq!( + fabro_config::resolve_storage_root(&file), + InterpString::parse("{{ env.FABRO_STORAGE_ROOT }}") + ); +} + +#[test] +fn dev_token_auth_enabled_requires_explicit_dev_token_method() { + let dev_token_only = parse( + r#" +_version = 1 + +[server.auth] +methods = ["dev-token"] +"#, + ); + let github_only = parse( + r#" +_version = 1 + +[server.auth] +methods = ["github"] +"#, + ); + let both = parse( + r#" +_version = 1 + +[server.auth] +methods = ["dev-token", "github"] +"#, + ); + + assert!(fabro_config::dev_token_auth_enabled(&dev_token_only)); + assert!(!fabro_config::dev_token_auth_enabled(&github_only)); + assert!(fabro_config::dev_token_auth_enabled(&both)); + assert!(!fabro_config::dev_token_auth_enabled(&SettingsLayer::default())); +} diff --git a/lib/crates/fabro-server/src/install.rs b/lib/crates/fabro-server/src/install.rs index 9448181df..8390a42af 100644 --- a/lib/crates/fabro-server/src/install.rs +++ b/lib/crates/fabro-server/src/install.rs @@ -815,7 +815,8 @@ async fn post_install_finish( description: None, }); let home = state.home.clone().unwrap_or_else(Home::from_env); - let token = match dev_token::load_or_create_dev_token(&home.dev_token_path()) { + let token = match dev_token::read_or_mint_dev_token_for_install(&home.dev_token_path()) + { Ok(value) => value, Err(err) => { return install_error_response( diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 19ae6a41f..45ec40b47 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -62,7 +62,10 @@ use fabro_store::{ ArtifactStore, Database, EventEnvelope, EventPayload, PendingInterviewRecord, StageId, }; use fabro_types::settings::run::RunMode; -use fabro_types::settings::server::{GithubIntegrationSettings, GithubIntegrationStrategy}; +use fabro_types::settings::server::{ + GithubIntegrationSettings, GithubIntegrationStrategy, ServerAuthLayer, ServerAuthMethod, + ServerLayer, +}; use fabro_types::settings::{ InterpString, ServerSettings as ResolvedServerSettings, SettingsLayer, }; @@ -2546,6 +2549,7 @@ fn default_test_app_state_config( max_concurrent_runs: usize, env_lookup: EnvLookup, ) -> AppStateConfig { + ensure_test_auth_methods(&settings); let (store, artifact_store) = test_store_bundle(); let vault_path = test_secret_store_path(); let server_env_path = vault_path.with_file_name("server.env"); @@ -2563,6 +2567,21 @@ fn default_test_app_state_config( } } +fn ensure_test_auth_methods(settings: &Arc>) { + let mut settings = settings.write().expect("test settings lock poisoned"); + if settings + .server + .as_ref() + .and_then(|server| server.auth.as_ref()) + .and_then(|auth| auth.methods.as_ref()) + .is_none() + { + let server = settings.server.get_or_insert_with(ServerLayer::default); + let auth = server.auth.get_or_insert_with(ServerAuthLayer::default); + auth.methods = Some(vec![ServerAuthMethod::DevToken]); + } +} + pub fn create_app_state_with_store( settings: Arc>, max_concurrent_runs: usize, @@ -3785,8 +3804,16 @@ fn worker_command( .stderr(Stdio::piped()); cmd.env_remove("FABRO_JSON"); - if let Some(token) = std::env::var_os("FABRO_DEV_TOKEN") { - cmd.env("FABRO_DEV_TOKEN", token); + cmd.env_remove("FABRO_DEV_TOKEN"); + if state + .server_settings() + .auth + .methods + .contains(&ServerAuthMethod::DevToken) + { + if let Some(token) = state.server_secret("FABRO_DEV_TOKEN") { + cmd.env("FABRO_DEV_TOKEN", token); + } } #[cfg(unix)] @@ -7755,6 +7782,88 @@ type = "http" ); } + #[cfg(unix)] + #[test] + fn worker_command_injects_dev_token_only_when_enabled() { + let github_only = tempfile::tempdir().unwrap(); + let github_state = worker_command_test_state(github_only.path(), &["github"], Some(TEST_DEV_TOKEN)); + let github_cmd = worker_command( + github_state.as_ref(), + RunId::new(), + RunExecutionMode::Start, + github_only.path(), + ) + .unwrap(); + assert_eq!(command_env_value(&github_cmd, "FABRO_DEV_TOKEN"), Some(None)); + + let dev_token = tempfile::tempdir().unwrap(); + let dev_token_state = + worker_command_test_state(dev_token.path(), &["dev-token"], Some(TEST_DEV_TOKEN)); + let dev_token_cmd = worker_command( + dev_token_state.as_ref(), + RunId::new(), + RunExecutionMode::Start, + dev_token.path(), + ) + .unwrap(); + assert_eq!( + command_env_value(&dev_token_cmd, "FABRO_DEV_TOKEN"), + Some(Some(TEST_DEV_TOKEN.to_string())) + ); + } + + fn worker_command_test_state( + storage_dir: &Path, + methods: &[&str], + dev_token: Option<&str>, + ) -> Arc { + let dev_token = dev_token.map(str::to_owned); + std::fs::create_dir_all(storage_dir).unwrap(); + let settings = fabro_config::parse_settings_layer(&format!( + r#" +_version = 1 + +[server.storage] +root = "{}" + +[server.auth] +methods = [{}] + +[server.auth.github] +allowed_usernames = ["octocat"] +"#, + storage_dir.display(), + methods + .iter() + .map(|method| format!("\"{method}\"")) + .collect::>() + .join(", ") + )) + .unwrap(); + let record_path = Storage::new(storage_dir).server_state().record_path(); + std::fs::create_dir_all(record_path.parent().unwrap()).unwrap(); + std::fs::write( + &record_path, + serde_json::to_string(&json!({ + "bind": Bind::Tcp("127.0.0.1:32276".parse::().unwrap()), + })) + .unwrap(), + ) + .unwrap(); + + create_app_state_with_env_lookup(settings, 5, move |name| match name { + "FABRO_DEV_TOKEN" => dev_token.clone(), + _ => None, + }) + } + + #[cfg(unix)] + fn command_env_value(cmd: &Command, key: &str) -> Option> { + cmd.as_std().get_envs().find_map(|(name, value)| { + (name.to_str() == Some(key)).then(|| value.map(|value| value.to_string_lossy().into_owned())) + }) + } + #[test] fn provider_credentials_resolve_process_env_before_vault() { let dir = tempfile::tempdir().unwrap(); diff --git a/lib/crates/fabro-test/src/lib.rs b/lib/crates/fabro-test/src/lib.rs index b428cc482..d8719112b 100644 --- a/lib/crates/fabro-test/src/lib.rs +++ b/lib/crates/fabro-test/src/lib.rs @@ -13,7 +13,7 @@ use std::sync::{Mutex, OnceLock}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use assert_cmd::Command; -use fabro_config::Storage; +use fabro_config::{Storage, envfile}; use fabro_types::RunId; use regex::Regex; use serde_json::{Map, Value, json}; @@ -79,6 +79,8 @@ const TEST_IN_MEMORY_STORE_ENV: &str = "FABRO_TEST_IN_MEMORY_STORE"; const SESSION_LOCK_TIMEOUT: Duration = Duration::from_secs(20); const TEST_SESSION_SECRET: &str = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; +const TEST_DEV_TOKEN: &str = + "fabro_dev_abababababababababababababababababababababababababababababababab"; #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum TestMode { @@ -617,13 +619,29 @@ fn write_settings_file(path: &Path, storage_dir: &Path, rest: &str) { std::fs::write( path, format!( - "_version = 1\n\n[server.storage]\nroot = \"{}\"\n\n{rest}", + "_version = 1\n\n[server.storage]\nroot = \"{}\"\n\n[server.auth]\nmethods = [\"dev-token\"]\n\n{rest}", storage_dir.display() ), ) .unwrap_or_else(|err| panic!("failed to write {}: {err}", path.display())); } +fn write_test_server_dev_token(storage_dir: &Path) { + let server_env_path = Storage::new(storage_dir).server_state().env_path(); + envfile::merge_env_file(&server_env_path, [("FABRO_DEV_TOKEN", TEST_DEV_TOKEN)]) + .unwrap_or_else(|err| panic!("failed to write {}: {err}", server_env_path.display())); +} + +fn write_test_home_dev_token(settings_path: &Path) { + let home_dir = settings_path + .parent() + .unwrap_or_else(|| panic!("expected {} to have a parent", settings_path.display())); + let dev_token_path = home_dir.join("dev-token"); + ensure_parent_dir(&dev_token_path); + std::fs::write(&dev_token_path, TEST_DEV_TOKEN) + .unwrap_or_else(|err| panic!("failed to write {}: {err}", dev_token_path.display())); +} + fn parse_settings_table(contents: &str, source: &Path) -> TomlMap { let stripped = strip_managed_storage_settings(contents); let value = toml::from_str::(stripped) @@ -695,6 +713,8 @@ fn sync_home_settings( socket_path: &Path, force_server_target: bool, ) { + write_test_home_dev_token(settings_path); + let (mut table, had_explicit_storage, had_explicit_target) = match std::fs::read_to_string(settings_path) { Ok(contents) => { @@ -753,6 +773,64 @@ fn sync_home_settings( write_settings_table(settings_path, &table); } +fn has_explicit_server_auth_methods(table: &TomlMap) -> bool { + table + .get("server") + .and_then(TomlValue::as_table) + .and_then(|server| server.get("auth")) + .and_then(TomlValue::as_table) + .and_then(|auth| auth.get("methods")) + .is_some() +} + +fn set_server_auth_methods(table: &mut TomlMap, methods: &[&str]) { + let server_entry = table + .entry("server".to_string()) + .or_insert_with(|| TomlValue::Table(TomlMap::new())); + let Some(server_table) = server_entry.as_table_mut() else { + panic!("expected [server] to be a TOML table"); + }; + let auth_entry = server_table + .entry("auth".to_string()) + .or_insert_with(|| TomlValue::Table(TomlMap::new())); + let Some(auth_table) = auth_entry.as_table_mut() else { + panic!("expected [server.auth] to be a TOML table"); + }; + auth_table.insert( + "methods".to_string(), + TomlValue::Array( + methods + .iter() + .map(|method| TomlValue::String((*method).to_string())) + .collect(), + ), + ); +} + +fn ensure_home_server_auth_methods( + settings_path: &Path, + storage_dir: &Path, + socket_path: &Path, + force_server_target: bool, +) { + let mut table = match std::fs::read_to_string(settings_path) { + Ok(contents) => parse_settings_table(&contents, settings_path), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => TomlMap::new(), + Err(err) => panic!("failed to read {}: {err}", settings_path.display()), + }; + + if has_explicit_server_auth_methods(&table) { + return; + } + + table + .entry("_version".to_string()) + .or_insert(TomlValue::Integer(1)); + set_server_auth_methods(&mut table, &["dev-token"]); + write_settings_table(settings_path, &table); + sync_home_settings(settings_path, storage_dir, socket_path, force_server_target); +} + fn has_explicit_storage_root(table: &TomlMap) -> bool { table .get("server") @@ -850,6 +928,7 @@ fn ensure_server_running(fabro_bin: &Path, server: &ServerPaths, config_path: &P ensure_parent_dir(config_path); std::fs::create_dir_all(&server.storage_dir) .unwrap_or_else(|err| panic!("failed to create {}: {err}", server.storage_dir.display())); + write_test_server_dev_token(&server.storage_dir); let _ = std::fs::remove_file(server_record_path(&server.storage_dir)); let _ = std::fs::remove_file(&server.socket_path); @@ -1201,6 +1280,7 @@ impl TestContext { /// Build a `run` subcommand. pub fn run_cmd(&self) -> Command { + self.ensure_home_server_auth_methods(); let mut cmd = self.command(); cmd.arg("run"); self.append_test_labels(&mut cmd); @@ -1209,6 +1289,7 @@ impl TestContext { /// Build a `create` subcommand with per-test labels attached. pub fn create_cmd(&self) -> Command { + self.ensure_home_server_auth_methods(); let mut cmd = self.command(); cmd.arg("create"); self.append_test_labels(&mut cmd); @@ -1400,6 +1481,17 @@ impl TestContext { self } + pub fn ensure_home_server_auth_methods(&self) -> &Self { + let settings_path = home_settings_path(&self.home_dir); + ensure_home_server_auth_methods( + &settings_path, + &self.storage_dir, + &self.active_socket_path, + self.isolated_server.is_some(), + ); + self + } + pub fn server_target(&self) -> String { self.active_socket_path.display().to_string() } @@ -1435,6 +1527,7 @@ impl TestContext { pub fn manage_storage_dir(&mut self, path: impl AsRef) -> &mut Self { let path = path.as_ref().to_path_buf(); if path != self.storage_dir && !self.managed_storage_dirs.contains(&path) { + write_test_server_dev_token(&path); self.managed_storage_dirs.push(path); } self diff --git a/lib/crates/fabro-types/src/settings/server.rs b/lib/crates/fabro-types/src/settings/server.rs index a40aedaba..285b38002 100644 --- a/lib/crates/fabro-types/src/settings/server.rs +++ b/lib/crates/fabro-types/src/settings/server.rs @@ -71,21 +71,12 @@ impl Default for ServerWebSettings { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize)] pub struct ServerAuthSettings { pub methods: Vec, pub github: ServerAuthGithubSettings, } -impl Default for ServerAuthSettings { - fn default() -> Self { - Self { - methods: vec![ServerAuthMethod::DevToken], - github: ServerAuthGithubSettings::default(), - } - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub enum ServerAuthMethod { diff --git a/lib/crates/fabro-util/src/dev_token.rs b/lib/crates/fabro-util/src/dev_token.rs index c7d9e2ee2..ead3f20e6 100644 --- a/lib/crates/fabro-util/src/dev_token.rs +++ b/lib/crates/fabro-util/src/dev_token.rs @@ -52,7 +52,18 @@ pub fn read_dev_token_file(path: &Path) -> Option { .filter(|token| validate_dev_token_format(token)) } -pub fn load_or_create_dev_token(path: &Path) -> Result { +pub fn read_dev_token_or_err(path: &Path) -> Result { + let contents = fs::read_to_string(path) + .with_context(|| format!("read dev token {}", path.display()))?; + let token = contents.trim().to_string(); + if validate_dev_token_format(&token) { + Ok(token) + } else { + Err(anyhow!("invalid dev token format in {}", path.display())) + } +} + +pub fn read_or_mint_dev_token_for_install(path: &Path) -> Result { match fs::read_to_string(path) { Ok(contents) => { let token = contents.trim().to_string(); diff --git a/lib/crates/fabro-util/tests/dev_token.rs b/lib/crates/fabro-util/tests/dev_token.rs index 99124b232..032232d64 100644 --- a/lib/crates/fabro-util/tests/dev_token.rs +++ b/lib/crates/fabro-util/tests/dev_token.rs @@ -7,7 +7,8 @@ use std::fs; use fabro_util::Home; use fabro_util::dev_token::{ - DEV_TOKEN_PREFIX, generate_dev_token, load_or_create_dev_token, validate_dev_token_format, + DEV_TOKEN_PREFIX, generate_dev_token, read_dev_token_or_err, + read_or_mint_dev_token_for_install, validate_dev_token_format, }; #[test] @@ -53,11 +54,11 @@ fn validate_format_rejects_wrong_prefix() { } #[test] -fn load_or_create_creates_file() { +fn read_or_mint_for_install_creates_file() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("dev-token"); - let token = load_or_create_dev_token(&path).unwrap(); + let token = read_or_mint_dev_token_for_install(&path).unwrap(); assert!(validate_dev_token_format(&token)); assert_eq!(fs::read_to_string(&path).unwrap(), token); @@ -71,28 +72,38 @@ fn load_or_create_creates_file() { } #[test] -fn load_or_create_reads_existing() { +fn read_or_mint_for_install_reads_existing() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("dev-token"); let token = format!("{DEV_TOKEN_PREFIX}{}", "cd".repeat(32)); fs::write(&path, &token).unwrap(); - let loaded = load_or_create_dev_token(&path).unwrap(); + let loaded = read_or_mint_dev_token_for_install(&path).unwrap(); assert_eq!(loaded, token); } #[test] -fn load_or_create_rejects_malformed_file() { +fn read_dev_token_or_err_rejects_malformed_file() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("dev-token"); fs::write(&path, "not-a-token").unwrap(); - let error = load_or_create_dev_token(&path).unwrap_err(); + let error = read_dev_token_or_err(&path).unwrap_err(); assert!(error.to_string().contains("invalid")); } +#[test] +fn read_dev_token_or_err_reports_missing_file() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("missing-dev-token"); + + let error = read_dev_token_or_err(&path).unwrap_err(); + + assert!(error.to_string().contains("read dev token")); +} + #[test] fn home_dev_token_path_is_relative_to_root() { let home = Home::new("/tmp/fabro-home");