diff --git a/Cargo.lock b/Cargo.lock index b023b8c87..c3595a3e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1633,6 +1633,7 @@ dependencies = [ "serde_json", "strsim 0.11.1", "tempfile", + "thiserror 2.0.18", "toml 0.8.23", "tracing", ] diff --git a/lib/crates/fabro-config/Cargo.toml b/lib/crates/fabro-config/Cargo.toml index fb35c7db0..f9882ecab 100644 --- a/lib/crates/fabro-config/Cargo.toml +++ b/lib/crates/fabro-config/Cargo.toml @@ -28,6 +28,7 @@ serde_json.workspace = true strsim = "0.11" toml.workspace = true tracing.workspace = true +thiserror.workspace = true [dev-dependencies] tempfile = "3" diff --git a/lib/crates/fabro-config/src/lib.rs b/lib/crates/fabro-config/src/lib.rs index c3242e773..f9a60f070 100644 --- a/lib/crates/fabro-config/src/lib.rs +++ b/lib/crates/fabro-config/src/lib.rs @@ -6,6 +6,7 @@ pub mod home; pub mod legacy_env; pub mod merge; pub mod project; +pub mod resolve; pub mod run; pub mod storage; pub mod user; @@ -13,6 +14,7 @@ pub mod user; pub use config::ConfigLayer; pub use fabro_util::path::expand_tilde; pub use home::Home; +pub use resolve::{ResolveError, resolve_server, resolve_server_from_file}; pub use storage::{RunScratch, ServerState, Storage}; use std::path::{Path, PathBuf}; diff --git a/lib/crates/fabro-config/src/resolve/error.rs b/lib/crates/fabro-config/src/resolve/error.rs new file mode 100644 index 000000000..7b275111a --- /dev/null +++ b/lib/crates/fabro-config/src/resolve/error.rs @@ -0,0 +1,11 @@ +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum ResolveError { + #[error("{path}: field is required")] + Missing { path: String }, + + #[error("{path}: invalid value - {reason}")] + Invalid { path: String, reason: String }, + + #[error("{path}: parse failure - {reason}")] + ParseFailure { path: String, reason: String }, +} diff --git a/lib/crates/fabro-config/src/resolve/mod.rs b/lib/crates/fabro-config/src/resolve/mod.rs new file mode 100644 index 000000000..ea00dbe8a --- /dev/null +++ b/lib/crates/fabro-config/src/resolve/mod.rs @@ -0,0 +1,55 @@ +mod error; +mod server; + +use fabro_types::settings::{ServerSettings, SettingsFile}; + +pub use error::ResolveError; +pub use server::resolve_server; + +pub fn resolve_server_from_file(file: &SettingsFile) -> Result> { + let mut errors = Vec::new(); + let layer = file.server.as_ref().cloned().unwrap_or_default(); + let resolved = resolve_server(&layer, &mut errors); + if errors.is_empty() { + Ok(resolved) + } else { + Err(errors) + } +} + +pub(crate) fn require_interp( + value: Option<&fabro_types::settings::InterpString>, + path: &str, + errors: &mut Vec, +) -> fabro_types::settings::InterpString { + value.cloned().unwrap_or_else(|| { + errors.push(ResolveError::Missing { + path: path.to_string(), + }); + fabro_types::settings::InterpString::parse("") + }) +} + +pub(crate) fn parse_socket_addr( + value: &fabro_types::settings::InterpString, + path: &str, + errors: &mut Vec, +) -> std::net::SocketAddr { + let source = value.as_source(); + match source.parse::() { + Ok(address) => address, + Err(err) => { + errors.push(ResolveError::ParseFailure { + path: path.to_string(), + reason: err.to_string(), + }); + std::net::SocketAddr::from(([127, 0, 0, 1], 0)) + } + } +} + +pub(crate) fn default_interp( + path: impl AsRef, +) -> fabro_types::settings::InterpString { + fabro_types::settings::InterpString::parse(&path.as_ref().to_string_lossy()) +} diff --git a/lib/crates/fabro-config/src/resolve/server.rs b/lib/crates/fabro-config/src/resolve/server.rs new file mode 100644 index 000000000..7db2d8947 --- /dev/null +++ b/lib/crates/fabro-config/src/resolve/server.rs @@ -0,0 +1,298 @@ +use std::time::Duration; + +use fabro_types::settings::InterpString; +use fabro_types::settings::server::{ + DiscordIntegrationSettings, GithubIntegrationSettings, GithubOauthSettings, + IntegrationWebhooksSettings, ObjectStoreProvider, ObjectStoreSettings, ServerApiLayer, + ServerApiSettings, ServerArtifactsLayer, ServerArtifactsSettings, ServerAuthApiJwtSettings, + ServerAuthApiMtlsSettings, ServerAuthApiSettings, ServerAuthLayer, ServerAuthSettings, + ServerAuthWebGithubLayer, ServerAuthWebProvidersSettings, ServerAuthWebSettings, + ServerIntegrationsLayer, ServerIntegrationsSettings, ServerLayer, ServerListenLayer, + ServerListenSettings, ServerListenTlsLayer, ServerLoggingSettings, ServerSchedulerSettings, + ServerSettings, ServerSlateDbLayer, ServerSlateDbSettings, ServerStorageSettings, + ServerWebSettings, SlackIntegrationSettings, TeamsIntegrationSettings, TlsConfig, +}; +use fabro_util::Home; + +use super::{ResolveError, default_interp, parse_socket_addr, require_interp}; + +pub fn resolve_server(layer: &ServerLayer, errors: &mut Vec) -> ServerSettings { + let storage = resolve_storage(layer.storage.as_ref()); + let (listen, valid_tls) = resolve_listen(layer.listen.as_ref(), errors); + let web = resolve_web(layer.api.as_ref(), layer.web.as_ref()); + let auth = resolve_auth(layer.auth.as_ref(), valid_tls, errors); + + ServerSettings { + listen, + api: ServerApiSettings { + url: layer.api.as_ref().and_then(|api| api.url.clone()), + }, + web, + auth, + storage: storage.clone(), + artifacts: resolve_artifacts(layer.artifacts.as_ref(), &storage.root, errors), + slatedb: resolve_slatedb(layer.slatedb.as_ref(), &storage.root, errors), + scheduler: ServerSchedulerSettings { + max_concurrent_runs: layer + .scheduler + .as_ref() + .and_then(|scheduler| scheduler.max_concurrent_runs) + .unwrap_or(5), + }, + logging: ServerLoggingSettings { + level: layer + .logging + .as_ref() + .and_then(|logging| logging.level.clone()), + }, + integrations: resolve_integrations(layer.integrations.as_ref()), + } +} + +fn resolve_storage( + layer: Option<&fabro_types::settings::server::ServerStorageLayer>, +) -> ServerStorageSettings { + ServerStorageSettings { + root: layer + .and_then(|storage| storage.root.clone()) + .unwrap_or_else(|| default_interp(Home::from_env().storage_dir())), + } +} + +fn resolve_listen( + layer: Option<&ServerListenLayer>, + errors: &mut Vec, +) -> (ServerListenSettings, bool) { + match layer { + None => ( + ServerListenSettings::Unix { + path: default_interp(Home::from_env().socket_path()), + }, + false, + ), + Some(ServerListenLayer::Unix { path }) => ( + ServerListenSettings::Unix { + path: path + .clone() + .unwrap_or_else(|| default_interp(Home::from_env().socket_path())), + }, + false, + ), + Some(ServerListenLayer::Tcp { address, tls }) => { + let address = parse_socket_addr( + &require_interp(address.as_ref(), "server.listen.address", errors), + "server.listen.address", + errors, + ); + let (tls, valid_tls) = resolve_tls(tls.as_ref(), errors); + (ServerListenSettings::Tcp { address, tls }, valid_tls) + } + } +} + +fn resolve_tls( + layer: Option<&ServerListenTlsLayer>, + errors: &mut Vec, +) -> (Option, bool) { + let Some(layer) = layer else { + return (None, false); + }; + + let cert = require_interp(layer.cert.as_ref(), "server.listen.tls.cert", errors); + let key = require_interp(layer.key.as_ref(), "server.listen.tls.key", errors); + let ca = require_interp(layer.ca.as_ref(), "server.listen.tls.ca", errors); + let valid = layer.cert.is_some() && layer.key.is_some() && layer.ca.is_some(); + + (Some(TlsConfig { cert, key, ca }), valid) +} + +fn resolve_web( + _api: Option<&ServerApiLayer>, + layer: Option<&fabro_types::settings::server::ServerWebLayer>, +) -> ServerWebSettings { + ServerWebSettings { + enabled: layer.and_then(|web| web.enabled).unwrap_or(true), + url: layer + .and_then(|web| web.url.clone()) + .unwrap_or_else(|| InterpString::parse("http://localhost:3000")), + } +} + +fn resolve_auth( + layer: Option<&ServerAuthLayer>, + valid_tls: bool, + errors: &mut Vec, +) -> ServerAuthSettings { + let api = layer.and_then(|auth| auth.api.as_ref()); + let web = layer.and_then(|auth| auth.web.as_ref()); + + let jwt = api.and_then(|api| { + api.jwt.as_ref().map(|jwt| ServerAuthApiJwtSettings { + enabled: jwt.enabled.unwrap_or(true), + issuer: jwt.issuer.clone(), + audience: jwt.audience.clone(), + }) + }); + let mtls = api.and_then(|api| { + api.mtls.as_ref().map(|mtls| ServerAuthApiMtlsSettings { + enabled: mtls.enabled.unwrap_or(true), + ca: mtls.ca.clone(), + }) + }); + if mtls.as_ref().is_some_and(|mtls| mtls.enabled) && !valid_tls { + errors.push(ResolveError::Invalid { + path: "server.auth.api.mtls".to_string(), + reason: "requires tcp listen with tls cert, key, and ca configured".to_string(), + }); + } + + ServerAuthSettings { + api: ServerAuthApiSettings { jwt, mtls }, + web: ServerAuthWebSettings { + allowed_usernames: web + .map(|web| web.allowed_usernames.clone()) + .unwrap_or_default(), + providers: ServerAuthWebProvidersSettings { + github: web + .and_then(|web| web.providers.as_ref()) + .and_then(|providers| providers.github.as_ref()) + .map(resolve_web_github), + }, + }, + } +} + +fn resolve_web_github(layer: &ServerAuthWebGithubLayer) -> GithubOauthSettings { + GithubOauthSettings { + enabled: layer.enabled.unwrap_or(true), + client_id: layer.client_id.clone(), + client_secret: layer.client_secret.clone(), + } +} + +fn resolve_artifacts( + layer: Option<&ServerArtifactsLayer>, + storage_root: &InterpString, + errors: &mut Vec, +) -> ServerArtifactsSettings { + let provider = layer + .and_then(|artifacts| artifacts.provider) + .unwrap_or(ObjectStoreProvider::Local); + + ServerArtifactsSettings { + prefix: layer + .and_then(|artifacts| artifacts.prefix.clone()) + .unwrap_or_else(|| InterpString::parse("artifacts")), + store: resolve_object_store( + provider, + layer.and_then(|artifacts| artifacts.local.as_ref()), + layer.and_then(|artifacts| artifacts.s3.as_ref()), + storage_root, + "server.artifacts", + errors, + ), + } +} + +fn resolve_slatedb( + layer: Option<&ServerSlateDbLayer>, + storage_root: &InterpString, + errors: &mut Vec, +) -> ServerSlateDbSettings { + let provider = layer + .and_then(|slatedb| slatedb.provider) + .unwrap_or(ObjectStoreProvider::Local); + + ServerSlateDbSettings { + prefix: layer + .and_then(|slatedb| slatedb.prefix.clone()) + .unwrap_or_else(|| InterpString::parse("")), + store: resolve_object_store( + provider, + layer.and_then(|slatedb| slatedb.local.as_ref()), + layer.and_then(|slatedb| slatedb.s3.as_ref()), + storage_root, + "server.slatedb", + errors, + ), + flush_interval: layer + .and_then(|slatedb| slatedb.flush_interval) + .map(|duration| duration.as_std()) + .unwrap_or_else(|| Duration::from_millis(1)), + } +} + +fn resolve_object_store( + provider: ObjectStoreProvider, + local: Option<&fabro_types::settings::server::ObjectStoreLocalLayer>, + s3: Option<&fabro_types::settings::server::ObjectStoreS3Layer>, + storage_root: &InterpString, + path_prefix: &str, + errors: &mut Vec, +) -> ObjectStoreSettings { + match provider { + ObjectStoreProvider::Local => ObjectStoreSettings::Local { + root: local + .and_then(|local| local.root.clone()) + .unwrap_or_else(|| storage_root.clone()), + }, + ObjectStoreProvider::S3 => { + let bucket = require_interp( + s3.and_then(|s3| s3.bucket.as_ref()), + &format!("{path_prefix}.s3.bucket"), + errors, + ); + let region = require_interp( + s3.and_then(|s3| s3.region.as_ref()), + &format!("{path_prefix}.s3.region"), + errors, + ); + ObjectStoreSettings::S3 { + bucket, + region, + endpoint: s3.and_then(|s3| s3.endpoint.clone()), + path_style: s3.and_then(|s3| s3.path_style).unwrap_or(false), + } + } + } +} + +fn resolve_integrations(layer: Option<&ServerIntegrationsLayer>) -> ServerIntegrationsSettings { + ServerIntegrationsSettings { + github: layer + .and_then(|integrations| integrations.github.as_ref()) + .map(|github| GithubIntegrationSettings { + enabled: github.enabled.unwrap_or(true), + app_id: github.app_id.clone(), + client_id: github.client_id.clone(), + slug: github.slug.clone(), + permissions: github.permissions.clone(), + webhooks: github + .webhooks + .as_ref() + .map(|webhooks| IntegrationWebhooksSettings { + strategy: webhooks.strategy, + }), + }) + .unwrap_or_default(), + slack: layer + .and_then(|integrations| integrations.slack.as_ref()) + .map(|slack| SlackIntegrationSettings { + enabled: slack.enabled.unwrap_or(true), + default_channel: slack.default_channel.clone(), + }) + .unwrap_or_default(), + discord: layer + .and_then(|integrations| integrations.discord.as_ref()) + .map(|discord| DiscordIntegrationSettings { + enabled: discord.enabled.unwrap_or(true), + }) + .unwrap_or_default(), + teams: layer + .and_then(|integrations| integrations.teams.as_ref()) + .map(|teams| TeamsIntegrationSettings { + enabled: teams.enabled.unwrap_or(true), + }) + .unwrap_or_default(), + } +} diff --git a/lib/crates/fabro-config/tests/resolve_server.rs b/lib/crates/fabro-config/tests/resolve_server.rs new file mode 100644 index 000000000..f4ec7833f --- /dev/null +++ b/lib/crates/fabro-config/tests/resolve_server.rs @@ -0,0 +1,143 @@ +use fabro_config::ConfigLayer; +use fabro_types::settings::server::{ObjectStoreSettings, ServerListenSettings}; +use fabro_types::settings::{InterpString, SettingsFile}; +use fabro_util::Home; + +fn parse(source: &str) -> SettingsFile { + ConfigLayer::parse(source) + .expect("fixture should parse") + .into() +} + +#[test] +fn resolves_server_defaults_from_empty_settings() { + let settings = fabro_config::resolve_server_from_file(&SettingsFile::default()) + .expect("empty settings should resolve"); + + assert_eq!( + settings.storage.root.as_source(), + Home::from_env().storage_dir().to_string_lossy() + ); + assert_eq!(settings.web.enabled, true); + assert_eq!(settings.web.url.as_source(), "http://localhost:3000"); + assert_eq!(settings.scheduler.max_concurrent_runs, 5); + + match settings.listen { + ServerListenSettings::Unix { path } => { + assert_eq!( + path.as_source(), + Home::from_env().socket_path().to_string_lossy() + ); + } + ServerListenSettings::Tcp { .. } => panic!("expected default listen transport to be unix"), + } + + match settings.artifacts.store { + ObjectStoreSettings::Local { root } => { + assert_eq!( + root.as_source(), + Home::from_env().storage_dir().to_string_lossy() + ); + } + ObjectStoreSettings::S3 { .. } => panic!("expected local artifact store by default"), + } + assert_eq!(settings.artifacts.prefix.as_source(), "artifacts"); +} + +#[test] +fn reports_tls_shape_errors_and_requires_valid_tls_for_mtls() { + let file = parse( + r#" +_version = 1 + +[server.listen] +type = "tcp" +address = "127.0.0.1:32276" + +[server.listen.tls] +cert = "/etc/fabro/server.pem" + +[server.auth.api.mtls] +enabled = true +"#, + ); + + let errors = fabro_config::resolve_server_from_file(&file) + .expect_err("incomplete tls config should fail"); + let rendered = errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n"); + + assert!(rendered.contains("server.listen.tls.key")); + assert!(rendered.contains("server.listen.tls.ca")); + assert!(rendered.contains("server.auth.api.mtls")); +} + +#[test] +fn reports_s3_shape_errors() { + let file = parse( + r#" +_version = 1 + +[server.artifacts] +provider = "s3" + +[server.artifacts.s3] +endpoint = "${env.S3_ENDPOINT}" +"#, + ); + + let errors = fabro_config::resolve_server_from_file(&file) + .expect_err("s3 config without bucket/region should fail"); + let rendered = errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n"); + + assert!(rendered.contains("server.artifacts.s3.bucket")); + assert!(rendered.contains("server.artifacts.s3.region")); +} + +#[test] +fn preserves_interp_strings_in_resolved_server_settings() { + let file = parse( + r#" +_version = 1 + +[server.listen] +type = "unix" +path = "${env.FABRO_SOCKET}" + +[server.integrations.github] +app_id = "${env.GITHUB_APP_ID}" +client_id = "${env.GITHUB_CLIENT_ID}" +slug = "fabro-app" +"#, + ); + + let settings = + fabro_config::resolve_server_from_file(&file).expect("server settings should resolve"); + + match settings.listen { + ServerListenSettings::Unix { path } => { + assert_eq!(path, InterpString::parse("${env.FABRO_SOCKET}")); + } + ServerListenSettings::Tcp { .. } => panic!("expected unix listen transport"), + } + + assert_eq!( + settings.integrations.github.app_id, + Some(InterpString::parse("${env.GITHUB_APP_ID}")) + ); + assert_eq!( + settings.integrations.github.client_id, + Some(InterpString::parse("${env.GITHUB_CLIENT_ID}")) + ); + assert_eq!( + settings.integrations.github.slug, + Some(InterpString::parse("fabro-app")) + ); +} diff --git a/lib/crates/fabro-server/src/diagnostics.rs b/lib/crates/fabro-server/src/diagnostics.rs index 79161f08f..c9f81a4e3 100644 --- a/lib/crates/fabro-server/src/diagnostics.rs +++ b/lib/crates/fabro-server/src/diagnostics.rs @@ -8,6 +8,7 @@ use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use fabro_llm::client::Client as LlmClient; use fabro_llm::types::{Message, Request}; use fabro_model::{Catalog, Provider}; +use fabro_types::settings::InterpString; use fabro_util::check_report::{CheckDetail, CheckResult, CheckSection, CheckStatus}; use fabro_util::version::FABRO_VERSION; use regex::Regex; @@ -288,15 +289,21 @@ async fn probe_llm_provider(client: &LlmClient, provider: Provider) -> Result<() } async fn check_github_app(state: &AppState) -> CheckResult { - let settings = state - .settings - .read() - .expect("settings lock poisoned") - .clone(); - let app_id = settings.github_app_id_str(); - let slug = settings.github_slug_str(); + let settings = state.server_settings(); + let app_id = settings + .integrations + .github + .app_id + .as_ref() + .map(InterpString::as_source); + let slug = settings + .integrations + .github + .slug + .as_ref() + .map(InterpString::as_source); let private_key_raw = state.secret_or_env("GITHUB_APP_PRIVATE_KEY"); - let client_id = settings.github_client_id_str().is_some(); + let client_id = settings.integrations.github.client_id.is_some(); let client_secret = state.secret_or_env("GITHUB_APP_CLIENT_SECRET").is_some(); let webhook_secret = state.secret_or_env("GITHUB_APP_WEBHOOK_SECRET").is_some(); diff --git a/lib/crates/fabro-server/src/jwt_auth.rs b/lib/crates/fabro-server/src/jwt_auth.rs index 7b45fff8c..01c880fb0 100644 --- a/lib/crates/fabro-server/src/jwt_auth.rs +++ b/lib/crates/fabro-server/src/jwt_auth.rs @@ -10,10 +10,9 @@ use serde::Deserialize; use tracing::warn; use crate::error::ApiError; -use crate::tls_config::TlsSettings; use crate::web_auth::SessionCookie; use fabro_types::RunAuthMethod; -use fabro_types::settings::SettingsFile; +use fabro_types::settings::{ServerListenSettings, ServerSettings as ResolvedServerSettings}; /// Env var that explicitly opts the server into unauthenticated startup. /// @@ -78,7 +77,7 @@ pub fn decode_pem_env(name: &str, value: &str) -> Result { String::from_utf8(bytes).map_err(|e| anyhow!("{name} base64 decoded to invalid UTF-8: {e}")) } -/// Resolve the authentication mode from a [`SettingsFile`]. +/// Resolve the authentication mode from resolved server settings. /// /// Call this once at startup before serving requests. Returns /// [`AuthMode::Disabled`] when [`FABRO_LOCAL_NO_AUTH_ENV`] is set to `"1"` @@ -92,11 +91,11 @@ pub fn decode_pem_env(name: &str, value: &str) -> Result { /// /// Walks the v2 `server.auth.api.{jwt,mtls}` subtree and /// `server.auth.web.allowed_usernames`. -pub fn resolve_auth_mode(settings: &SettingsFile) -> Result { +pub fn resolve_auth_mode(settings: &ResolvedServerSettings) -> Result { resolve_auth_mode_with_lookup(settings, |name| std::env::var(name).ok()) } -/// Describes which API auth strategies are enabled in a `SettingsFile`. +/// Describes which API auth strategies are enabled in resolved server settings. struct ResolvedAuthStrategies { jwt_enabled: bool, mtls_enabled: bool, @@ -104,26 +103,26 @@ struct ResolvedAuthStrategies { allowed_usernames: Vec, } -fn resolve_auth_strategies(settings: &SettingsFile) -> ResolvedAuthStrategies { - let server = settings.server.as_ref(); - let auth = server.and_then(|s| s.auth.as_ref()); - let auth_api = auth.and_then(|a| a.api.as_ref()); +fn resolve_auth_strategies(settings: &ResolvedServerSettings) -> ResolvedAuthStrategies { + let jwt_enabled = settings + .auth + .api + .jwt + .as_ref() + .is_some_and(|jwt| jwt.enabled); + let mtls_enabled = settings + .auth + .api + .mtls + .as_ref() + .is_some_and(|mtls| mtls.enabled); - // Strategies: a subtree with `enabled = false` is explicitly off. - // Presence of the subtree with `enabled` unset counts as on. - let jwt_enabled = auth_api - .and_then(|api| api.jwt.as_ref()) - .is_some_and(|jwt| jwt.enabled.unwrap_or(true)); - let mtls_enabled = auth_api - .and_then(|api| api.mtls.as_ref()) - .is_some_and(|mtls| mtls.enabled.unwrap_or(true)); + let tls_present = matches!( + settings.listen, + ServerListenSettings::Tcp { ref tls, .. } if tls.is_some() + ); - let tls_present = TlsSettings::from_settings(settings).is_some(); - - let allowed_usernames = auth - .and_then(|a| a.web.as_ref()) - .map(|w| w.allowed_usernames.clone()) - .unwrap_or_default(); + let allowed_usernames = settings.auth.web.allowed_usernames.clone(); ResolvedAuthStrategies { jwt_enabled, @@ -133,7 +132,10 @@ fn resolve_auth_strategies(settings: &SettingsFile) -> ResolvedAuthStrategies { } } -pub fn resolve_auth_mode_with_lookup(settings: &SettingsFile, lookup: F) -> Result +pub fn resolve_auth_mode_with_lookup( + settings: &ResolvedServerSettings, + lookup: F, +) -> Result where F: Fn(&str) -> Option, { @@ -451,16 +453,18 @@ mod tests { use axum::response::IntoResponse; use axum::routing::get; use fabro_config::ConfigLayer; + use fabro_config::resolve_server_from_file; use tower::ServiceExt; use crate::web_auth::SessionCookie; // --- Fail-closed resolver tests (R52/R53) ----------------------------------- - fn settings(source: &str) -> SettingsFile { - ConfigLayer::parse(source) + fn settings(source: &str) -> ResolvedServerSettings { + let file = ConfigLayer::parse(source) .expect("fixture should parse") - .into() + .into(); + resolve_server_from_file(&file).expect("fixture should resolve") } /// Lookup closure that returns nothing — every env var is absent. @@ -590,21 +594,6 @@ enabled = true assert!(err.to_string().contains("invalid")); } - #[test] - fn fail_closed_when_mtls_enabled_without_listen_tls() { - let file = settings( - r" -_version = 1 - -[server.auth.api.mtls] -enabled = true -", - ); - let err = resolve_auth_mode_with_lookup(&file, empty_lookup) - .expect_err("mTLS without [server.listen.tls] should refuse startup"); - assert!(err.to_string().contains("server.listen.tls")); - } - async fn protected_handler(_auth: AuthenticatedService) -> impl IntoResponse { "ok" } diff --git a/lib/crates/fabro-server/src/lib.rs b/lib/crates/fabro-server/src/lib.rs index e11d07cdd..507c77f2a 100644 --- a/lib/crates/fabro-server/src/lib.rs +++ b/lib/crates/fabro-server/src/lib.rs @@ -17,5 +17,4 @@ pub mod server; mod settings_view; pub mod static_files; pub mod tls; -pub mod tls_config; pub mod web_auth; diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index db33402bd..42d161693 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -2,8 +2,9 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, RwLock}; use std::time::Duration; +use anyhow::Context; use fabro_config::Storage; -use fabro_config::resolve_storage_dir; +use fabro_config::resolve_server_from_file; use fabro_config::user::{active_settings_path, load_settings_config}; use fabro_util::terminal::Styles; use object_store::ObjectStore; @@ -17,7 +18,10 @@ use tracing::{error, info, warn}; use clap::Args; -use fabro_types::settings::SettingsFile; +use fabro_types::settings::{ + InterpString, ObjectStoreSettings, ServerListenSettings, + ServerSettings as ResolvedServerSettings, SettingsFile, +}; use crate::bind::{self, Bind, BindRequest}; use crate::github_webhooks::WebhookManager; @@ -28,7 +32,6 @@ use crate::server::{ reconcile_incomplete_runs_on_startup, shutdown_active_workers, spawn_scheduler, }; use crate::tls::{ClientAuth, build_rustls_config, serve_tls_with_shutdown}; -use crate::tls_config::TlsSettings; use fabro_llm::client::Client as LlmClient; use fabro_sandbox::SandboxProvider; @@ -171,53 +174,58 @@ fn build_object_store(store_path: &Path) -> anyhow::Result> build_object_store_with_preference(store_path, use_in_memory_store()) } -fn build_artifact_object_store( - settings: &SettingsFile, - storage: &Storage, -) -> anyhow::Result<(Arc, String)> { - use fabro_types::settings::interp::InterpString; - use fabro_types::settings::server::ObjectStoreProvider; +fn resolve_server_settings(file: &SettingsFile) -> anyhow::Result { + resolve_server_from_file(file).map_err(|errors| { + anyhow::anyhow!( + "failed to resolve server settings:\n{}", + errors + .into_iter() + .map(|error| error.to_string()) + .collect::>() + .join("\n") + ) + }) +} - let artifacts = settings.server_artifacts(); - let prefix = artifacts - .and_then(|a| a.prefix.as_ref()) - .map_or_else(|| "artifacts".to_string(), InterpString::as_source); +fn resolve_interp(value: &InterpString) -> anyhow::Result { + value + .resolve(|name| std::env::var(name).ok()) + .map(|resolved| resolved.value) + .with_context(|| format!("failed to resolve {}", value.as_source())) +} + +fn resolve_interp_path(value: &InterpString) -> anyhow::Result { + Ok(PathBuf::from(resolve_interp(value)?)) +} + +fn build_artifact_object_store( + settings: &ResolvedServerSettings, +) -> anyhow::Result<(Arc, String)> { + let prefix = resolve_interp(&settings.artifacts.prefix)?; if use_in_memory_store() { return Ok((Arc::new(InMemory::new()), prefix)); } - let provider = artifacts - .and_then(|a| a.provider) - .unwrap_or(ObjectStoreProvider::Local); - - let s3_cfg = artifacts.and_then(|a| a.s3.as_ref()); - match provider { - ObjectStoreProvider::Local => { - std::fs::create_dir_all(storage.artifact_store_dir())?; - let object_store = Arc::new(LocalFileSystem::new_with_prefix(storage.root())?); + match &settings.artifacts.store { + ObjectStoreSettings::Local { root } => { + let root = resolve_interp_path(root)?; + std::fs::create_dir_all(&root)?; + let object_store = Arc::new(LocalFileSystem::new_with_prefix(&root)?); Ok((object_store, prefix)) } - ObjectStoreProvider::S3 => { - let s3 = s3_cfg.ok_or_else(|| { - anyhow::anyhow!("server.artifacts.s3 is required for provider = 's3'") - })?; - let bucket = s3 - .bucket - .as_ref() - .map(InterpString::as_source) - .ok_or_else(|| anyhow::anyhow!("server.artifacts.s3.bucket is required"))?; - let region = s3 - .region - .as_ref() - .map(InterpString::as_source) - .ok_or_else(|| anyhow::anyhow!("server.artifacts.s3.region is required"))?; + ObjectStoreSettings::S3 { + bucket, + region, + endpoint, + path_style, + } => { let mut builder = AmazonS3Builder::from_env() - .with_bucket_name(bucket) - .with_region(region) - .with_virtual_hosted_style_request(!s3.path_style.unwrap_or(false)); - if let Some(endpoint) = s3.endpoint.as_ref().map(InterpString::as_source) { - builder = builder.with_endpoint(endpoint); + .with_bucket_name(resolve_interp(bucket)?) + .with_region(resolve_interp(region)?) + .with_virtual_hosted_style_request(!*path_style); + if let Some(endpoint) = endpoint.as_ref() { + builder = builder.with_endpoint(resolve_interp(endpoint)?); } let object_store = Arc::new(builder.build()?); Ok((object_store, prefix)) @@ -245,8 +253,12 @@ where let config_path = args.config.clone(); let disk_settings = load_settings(config_path.as_deref())?; + let disk_server_settings = resolve_server_settings(&disk_settings)?; let active_config_path = resolved_config_path(config_path.as_deref()); - let data_dir = storage_dir_override.unwrap_or_else(|| resolve_storage_dir(&disk_settings)); + let data_dir = match storage_dir_override { + Some(path) => path, + None => resolve_interp_path(&disk_server_settings.storage.root)?, + }; let storage = Storage::new(&data_dir); let secret_store_path = storage.secrets_path(); let secret_store = SecretStore::load(secret_store_path.clone())?; @@ -284,31 +296,27 @@ where // Shared config for live reloading let effective_settings = apply_runtime_settings(&disk_settings, &args, dry_run_mode, &data_dir); + let resolved_server_settings = resolve_server_settings(&effective_settings)?; let shared_settings = Arc::new(RwLock::new(effective_settings)); std::fs::create_dir_all(&data_dir)?; let (auth_mode, client_auth, max_concurrent_runs) = { - let cfg_file = shared_settings.read().expect("config lock poisoned"); - let auth_mode = resolve_auth_mode_with_lookup(&cfg_file, |name| { + let auth_mode = resolve_auth_mode_with_lookup(&resolved_server_settings, |name| { secret_snapshot .get(name) .cloned() .or_else(|| std::env::var(name).ok()) })?; - let tls_present = TlsSettings::from_settings(&cfg_file).is_some(); + let tls_present = matches!( + resolved_server_settings.listen, + ServerListenSettings::Tcp { ref tls, .. } if tls.is_some() + ); let client_auth = tls_present.then(|| client_auth_from_mode(&auth_mode)); let max_concurrent_runs = args .max_concurrent_runs - .or_else(|| cfg_file.max_concurrent_runs()) - .unwrap_or(5); + .unwrap_or(resolved_server_settings.scheduler.max_concurrent_runs); (auth_mode, client_auth, max_concurrent_runs) }; - let web_enabled = { - let cfg_file = shared_settings.read().expect("config lock poisoned"); - cfg_file - .server_web() - .and_then(|w| w.enabled) - .unwrap_or(true) - }; + let web_enabled = resolved_server_settings.web.enabled; let store_path = storage.store_dir(); let object_store = build_object_store(&store_path)?; @@ -317,10 +325,8 @@ where "", Duration::from_millis(1), )); - let (artifact_object_store, artifact_prefix) = build_artifact_object_store( - &shared_settings.read().expect("config lock poisoned"), - &storage, - )?; + let (artifact_object_store, artifact_prefix) = + build_artifact_object_store(&resolved_server_settings)?; let artifact_store = fabro_store::ArtifactStore::new(artifact_object_store, artifact_prefix); let state = build_app_state_with_path( Arc::clone(&shared_settings), @@ -345,19 +351,21 @@ where let bind_request = match args.bind { Some(ref s) => bind::parse_bind(s)?, - None => BindRequest::Tcp("127.0.0.1:3000".parse().unwrap()), + None => match &resolved_server_settings.listen { + ServerListenSettings::Unix { path } => BindRequest::Unix(resolve_interp_path(path)?), + ServerListenSettings::Tcp { address, .. } => BindRequest::Tcp(*address), + }, }; // Optionally start webhook listener - let webhook_app_id = { - use fabro_types::settings::InterpString; - let cfg_file = shared_settings.read().expect("config lock poisoned"); - cfg_file - .server_integrations_github() - .filter(|github| github.webhooks.is_some()) - .and_then(|github| github.app_id.as_ref()) - .map(InterpString::as_source) - }; + let webhook_app_id = resolved_server_settings + .integrations + .github + .webhooks + .as_ref() + .and_then(|_| resolved_server_settings.integrations.github.app_id.as_ref()) + .map(resolve_interp) + .transpose()?; let webhook_manager = match webhook_app_id { Some(app_id) => { let secret = secret_snapshot @@ -410,7 +418,7 @@ where }); // Spawn config polling task - let settings_for_poll = Arc::clone(&shared_settings); + let state_for_poll = Arc::clone(&state); let config_path_for_poll = config_path.clone(); let args_for_poll = args.clone(); let data_dir_for_poll = data_dir.clone(); @@ -428,13 +436,20 @@ where &data_dir_for_poll, ); let changed = { - let cfg = settings_for_poll.read().expect("config lock poisoned"); + let cfg = state_for_poll + .settings + .read() + .expect("config lock poisoned"); *cfg != effective }; if changed { - let mut cfg = settings_for_poll.write().expect("config lock poisoned"); - *cfg = effective; - info!("Server config reloaded"); + match state_for_poll.replace_settings(effective) { + Ok(()) => info!("Server config reloaded"), + Err(error) => warn!( + error = %error, + "Failed to resolve reloaded server config, keeping previous" + ), + } } } Err(e) => { @@ -445,9 +460,9 @@ where }); // Branch: TLS, plain TCP, or Unix socket - let tls_settings = { - let cfg_file = shared_settings.read().expect("config lock poisoned"); - TlsSettings::from_settings(&cfg_file) + let tls_settings = match &resolved_server_settings.listen { + ServerListenSettings::Tcp { tls, .. } => tls.clone(), + ServerListenSettings::Unix { .. } => None, }; let bound_listener = bind_listener(&bind_request).await?; @@ -483,7 +498,7 @@ where BoundListener::Tcp(listener) => { if let Some(ref tls_settings) = tls_settings { let client_auth = client_auth.unwrap(); - let rustls_config = build_rustls_config(tls_settings, client_auth); + let rustls_config = build_rustls_config(tls_settings, client_auth)?; let tls_acceptor = tokio_rustls::TlsAcceptor::from(rustls_config); info!("TLS enabled"); diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 3ff4205f8..58b020839 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -22,6 +22,7 @@ use base64::Engine as _; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use bytes::Bytes; use fabro_config::Storage; +use fabro_config::resolve_server_from_file; use fabro_llm::client::Client as LlmClient; use fabro_llm::generate::{GenerateParams, generate_object}; use fabro_llm::model_test::{ModelTestMode, run_model_test_with_client}; @@ -33,7 +34,7 @@ use fabro_model::{BilledModelUsage, BilledTokenCounts}; use fabro_store::{ ArtifactStore, Database, EventEnvelope, EventPayload, PendingInterviewRecord, StageId, }; -use fabro_types::settings::{InterpString, SettingsFile}; +use fabro_types::settings::{InterpString, ServerSettings as ResolvedServerSettings, SettingsFile}; use fabro_types::{ ActorRef, EventBody, InterviewQuestionRecord, InterviewQuestionType, RunBlobId, RunClientProvenance, RunControlAction, RunEvent, RunId, RunProvenance, RunServerProvenance, @@ -523,6 +524,7 @@ pub struct AppState { pub(crate) secret_store: AsyncRwLock, pub(crate) settings: Arc>, + pub(crate) server_settings: RwLock>, pub(crate) config_path: PathBuf, pub(crate) local_daemon_mode: bool, shutting_down: AtomicBool, @@ -575,6 +577,22 @@ fn accumulate_model_billing(entry: &mut ModelBillingTotals, usage: &BilledModelU } impl AppState { + pub(crate) fn server_settings(&self) -> Arc { + Arc::clone( + &self + .server_settings + .read() + .expect("server settings lock poisoned"), + ) + } + + pub(crate) fn server_storage_dir(&self) -> PathBuf { + PathBuf::from( + resolve_interp_string(&self.server_settings().storage.root) + .expect("server storage root should be resolved at startup"), + ) + } + pub(crate) fn dry_run(&self) -> bool { self.settings.read().unwrap().dry_run_enabled() } @@ -668,6 +686,31 @@ impl AppState { fn is_shutting_down(&self) -> bool { self.shutting_down.load(Ordering::Relaxed) } + + pub(crate) fn replace_settings(&self, settings: SettingsFile) -> anyhow::Result<()> { + let resolved = Arc::new(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") + ) + })?); + + *self.settings.write().expect("settings lock poisoned") = settings; + *self + .server_settings + .write() + .expect("server settings lock poisoned") = resolved; + Ok(()) + } + + pub(crate) fn reload_settings_from_disk(&self) -> anyhow::Result<()> { + let reloaded: SettingsFile = fabro_config::ConfigLayer::load(&self.config_path)?.into(); + self.replace_settings(reloaded) + } } fn artifact_upload_token_keys() -> ArtifactUploadTokenKeys { @@ -740,6 +783,13 @@ fn decode_secret_pem(name: &str, raw: &str) -> Result { .map_err(|err| format!("{name} base64 decoded to invalid UTF-8: {err}")) } +fn resolve_interp_string(value: &InterpString) -> anyhow::Result { + value + .resolve(|name| std::env::var(name).ok()) + .map(|resolved| resolved.value) + .map_err(anyhow::Error::from) +} + fn start_optional_slack_service(state: &Arc) { let Some(service) = state.slack_service.clone() else { return; @@ -1124,7 +1174,7 @@ async fn get_system_info( os: Some(std::env::consts::OS.to_string()), arch: Some(std::env::consts::ARCH.to_string()), storage_engine: Some("slatedb".to_string()), - storage_dir: Some(settings.storage_dir().display().to_string()), + storage_dir: Some(state.server_storage_dir().display().to_string()), uptime_secs: Some(to_i64(state.started_at.elapsed().as_secs())), runs: Some(SystemRunCounts { total: Some(to_i64(total_runs)), @@ -1140,7 +1190,7 @@ async fn get_system_df( State(state): State>, Query(params): Query, ) -> Response { - let storage_dir = state.settings.read().unwrap().storage_dir(); + let storage_dir = state.server_storage_dir(); let summaries = match state .store .list_runs(&fabro_store::ListRunsQuery::default()) @@ -1177,7 +1227,7 @@ async fn prune_runs( State(state): State>, Json(body): Json, ) -> Response { - let storage_dir = state.settings.read().unwrap().storage_dir(); + let storage_dir = state.server_storage_dir(); let summaries = match state .store .list_runs(&fabro_store::ListRunsQuery::default()) @@ -1606,18 +1656,20 @@ async fn get_github_repo( State(state): State>, Path((owner, name)): Path<(String, String)>, ) -> Response { - let settings = state - .settings - .read() - .expect("settings lock poisoned") - .clone(); - let Some(app_id) = settings.github_app_id_str() else { + let settings = state.server_settings(); + let Some(app_id) = settings.integrations.github.app_id.as_ref() else { return ApiError::new( StatusCode::SERVICE_UNAVAILABLE, "server.integrations.github.app_id is not configured", ) .into_response(); }; + let app_id = match resolve_interp_string(app_id) { + Ok(app_id) => app_id, + Err(err) => { + return ApiError::new(StatusCode::SERVICE_UNAVAILABLE, err.to_string()).into_response(); + } + }; let creds = match state.github_app_credentials(Some(&app_id)).await { Ok(Some(creds)) => creds, @@ -1642,10 +1694,16 @@ async fn get_github_repo( let base_url = fabro_github::github_api_base_url(); let client = reqwest::Client::new(); - let install_url = settings.github_slug_str().map_or_else( - || format!("https://github.com/organizations/{owner}/settings/installations"), - |slug| format!("https://github.com/apps/{slug}/installations/new"), - ); + let install_url = match settings.integrations.github.slug.as_ref() { + Some(slug) => match resolve_interp_string(slug) { + Ok(slug) => format!("https://github.com/apps/{slug}/installations/new"), + Err(err) => { + return ApiError::new(StatusCode::SERVICE_UNAVAILABLE, err.to_string()) + .into_response(); + } + }, + None => format!("https://github.com/organizations/{owner}/settings/installations"), + }; let installed = match fabro_github::check_app_installed(&client, &jwt, &owner, &name, &base_url).await { @@ -2013,11 +2071,32 @@ pub(crate) fn build_app_state_with_path( ) -> anyhow::Result> { let secret_store = SecretStore::load(secret_store_path)?; let (global_event_tx, _) = broadcast::channel(4096); - let slack_service = { + let resolved_server_settings = { let settings = settings.read().expect("settings lock poisoned"); - settings - .server_integrations_slack() - .and_then(|slack| slack.default_channel.as_ref().map(InterpString::as_source)) + Arc::new(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 slack_service = { + resolved_server_settings + .integrations + .slack + .default_channel + .as_ref() + .map(|value| { + value + .resolve(|name| std::env::var(name).ok()) + .map(|resolved| resolved.value) + .map_err(anyhow::Error::from) + }) + .transpose()? .and_then(|default_channel| { resolve_slack_credentials().map(|credentials| { Arc::new(SlackService::new( @@ -2040,6 +2119,7 @@ pub(crate) fn build_app_state_with_path( global_event_tx, secret_store: AsyncRwLock::new(secret_store), settings, + server_settings: RwLock::new(resolved_server_settings), config_path, local_daemon_mode, shutting_down: AtomicBool::new(false), @@ -2210,7 +2290,7 @@ async fn delete_run_internal(state: &Arc, id: RunId) -> Result<(), Res })?; } } else { - let storage = Storage::new(state.settings.read().unwrap().storage_dir()); + let storage = Storage::new(state.server_storage_dir()); let run_dir = storage.run_scratch(&id).root().to_path_buf(); remove_run_dir(&run_dir).map_err(|err| { ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response() @@ -2890,11 +2970,7 @@ fn worker_command( ) -> anyhow::Result { let exe = std::env::var_os("CARGO_BIN_EXE_fabro").map_or(std::env::current_exe()?, PathBuf::from); - let storage_dir = state - .settings - .read() - .expect("settings lock poisoned") - .storage_dir(); + let storage_dir = state.server_storage_dir(); let server_target = current_server_target(&storage_dir)?; let artifact_upload_token = state .issue_artifact_upload_token(&run_id) diff --git a/lib/crates/fabro-server/src/settings_view.rs b/lib/crates/fabro-server/src/settings_view.rs index 8c1d23f5e..4fbbfb753 100644 --- a/lib/crates/fabro-server/src/settings_view.rs +++ b/lib/crates/fabro-server/src/settings_view.rs @@ -234,4 +234,39 @@ slug = "fabro-app" assert!(github.client_id.is_some()); assert!(github.slug.is_some()); } + + #[test] + fn preserves_env_templates_for_non_redacted_fields() { + let settings = parse( + r#" +_version = 1 + +[server.storage] +root = "${env.FABRO_STORAGE_ROOT}" + +[server.integrations.slack] +default_channel = "${env.SLACK_CHANNEL}" +"#, + ); + + let redacted = redact_for_api(&settings); + let server = redacted + .server + .expect("server config should remain present"); + assert_eq!( + server + .storage + .and_then(|storage| storage.root) + .map(|value| value.as_source()), + Some("${env.FABRO_STORAGE_ROOT}".to_string()) + ); + assert_eq!( + server + .integrations + .and_then(|integrations| integrations.slack) + .and_then(|slack| slack.default_channel) + .map(|value| value.as_source()), + Some("${env.SLACK_CHANNEL}".to_string()) + ); + } } diff --git a/lib/crates/fabro-server/src/tls.rs b/lib/crates/fabro-server/src/tls.rs index a1b226cd1..a6f908556 100644 --- a/lib/crates/fabro-server/src/tls.rs +++ b/lib/crates/fabro-server/src/tls.rs @@ -2,6 +2,9 @@ use std::path::Path; use std::sync::Arc; use std::{future::Future, pin::Pin}; +use anyhow::Context; +use fabro_types::settings::InterpString; +use fabro_types::settings::TlsConfig; use rustls::ServerConfig; use rustls::server::WebPkiClientVerifier; use rustls_pki_types::{CertificateDer, PrivateKeyDer}; @@ -9,7 +12,6 @@ use tokio::net::TcpListener; use tracing::error; use crate::jwt_auth::PeerCertificates; -use crate::tls_config::TlsSettings; /// How client certificates should be verified. #[derive(Clone, Copy)] @@ -24,11 +26,14 @@ pub enum ClientAuth { /// Build a rustls `ServerConfig` from the `[api.tls]` configuration. pub fn build_rustls_config( - tls_settings: &TlsSettings, + tls_settings: &TlsConfig, client_auth: ClientAuth, -) -> Arc { - let certs = load_certs(&tls_settings.cert); - let key = load_private_key(&tls_settings.key); +) -> anyhow::Result> { + let cert = resolve_path(&tls_settings.cert)?; + let key_path = resolve_path(&tls_settings.key)?; + + let certs = load_certs(&cert); + let key = load_private_key(&key_path); let config = match client_auth { ClientAuth::None => ServerConfig::builder() @@ -36,7 +41,8 @@ pub fn build_rustls_config( .with_single_cert(certs, key) .expect("invalid server certificate or key"), ClientAuth::Required | ClientAuth::Optional => { - let ca_certs = load_certs(&tls_settings.ca); + let ca_path = resolve_path(&tls_settings.ca)?; + let ca_certs = load_certs(&ca_path); let mut root_store = rustls::RootCertStore::empty(); for cert in ca_certs { root_store @@ -60,7 +66,7 @@ pub fn build_rustls_config( } }; - Arc::new(config) + Ok(Arc::new(config)) } /// Serve requests over TLS, extracting peer certificates into request extensions. @@ -136,6 +142,13 @@ where pub use fabro_config::expand_tilde; +fn resolve_path(value: &InterpString) -> anyhow::Result { + let resolved = value + .resolve(|name| std::env::var(name).ok()) + .with_context(|| format!("failed to resolve {}", value.as_source()))?; + Ok(expand_tilde(Path::new(&resolved.value))) +} + fn load_certs(path: &Path) -> Vec> { let path = expand_tilde(path); let file = std::fs::File::open(&path) diff --git a/lib/crates/fabro-server/src/tls_config.rs b/lib/crates/fabro-server/src/tls_config.rs deleted file mode 100644 index 84341813e..000000000 --- a/lib/crates/fabro-server/src/tls_config.rs +++ /dev/null @@ -1,43 +0,0 @@ -//! Resolved TLS material extracted from `[server.listen.tls]`. -//! -//! Owns the `(cert, key, ca)` triple that the rustls config builder in -//! [`crate::tls`] consumes when the server is listening on TCP with mTLS -//! enabled. - -use std::path::PathBuf; - -use fabro_types::settings::SettingsFile; -use fabro_types::settings::interp::InterpString; -use fabro_types::settings::server::ServerListenLayer; - -/// Resolved TLS material used by the rustls config builder in -/// [`crate::tls`] when the server is listening on TCP with -/// `[server.listen.tls]` set. -#[derive(Debug, Clone, PartialEq)] -pub struct TlsSettings { - pub cert: PathBuf, - pub key: PathBuf, - pub ca: PathBuf, -} - -impl TlsSettings { - /// Extract the `[server.listen.tls]` subtree out of a `SettingsFile`. - /// Returns `None` when the server is on Unix sockets, TLS is unset, or - /// any of the three fields is missing. - #[must_use] - pub fn from_settings(file: &SettingsFile) -> Option { - let listen = file.server.as_ref()?.listen.as_ref()?; - let tls = match listen { - ServerListenLayer::Tcp { tls, .. } => tls.as_ref()?, - ServerListenLayer::Unix { .. } => return None, - }; - let cert = tls.cert.as_ref().map(InterpString::as_source)?; - let key = tls.key.as_ref().map(InterpString::as_source)?; - let ca = tls.ca.as_ref().map(InterpString::as_source)?; - Some(Self { - cert: cert.into(), - key: key.into(), - ca: ca.into(), - }) - } -} diff --git a/lib/crates/fabro-server/src/web_auth.rs b/lib/crates/fabro-server/src/web_auth.rs index b13d5d5fb..d522de14e 100644 --- a/lib/crates/fabro-server/src/web_auth.rs +++ b/lib/crates/fabro-server/src/web_auth.rs @@ -161,30 +161,49 @@ fn features_json(settings: &SettingsFile) -> serde_json::Value { }) } +fn resolve_interp(value: &InterpString) -> anyhow::Result { + value + .resolve(|name| std::env::var(name).ok()) + .map(|resolved| resolved.value) + .map_err(anyhow::Error::from) +} + async fn login_github(State(state): State>) -> Response { - let settings = state - .settings - .read() - .expect("settings lock poisoned") - .clone(); - let Some(client_id) = settings.github_client_id_str() else { + let settings = state.server_settings(); + let Some(client_id) = settings.integrations.github.client_id.as_ref() else { warn!("OAuth login failed: client_id not configured"); return json_response( StatusCode::CONFLICT, json!({"error": "GitHub App client_id is not configured"}), ); }; - let Some(web_url) = settings - .server_web() - .and_then(|w| w.url.as_ref()) - .map(InterpString::as_source) - else { + let client_id = match resolve_interp(client_id) { + Ok(client_id) => client_id, + Err(err) => { + warn!(error = %err, "OAuth login failed: client_id could not be resolved"); + return json_response( + StatusCode::CONFLICT, + json!({"error": format!("GitHub App client_id could not be resolved: {err}")}), + ); + } + }; + let web_url = match resolve_interp(&settings.web.url) { + Ok(web_url) => web_url, + Err(err) => { + warn!(error = %err, "OAuth login failed: server.web.url could not be resolved"); + return json_response( + StatusCode::CONFLICT, + json!({"error": format!("server.web.url could not be resolved: {err}")}), + ); + } + }; + if web_url.is_empty() { warn!("OAuth login failed: server.web.url not configured"); return json_response( StatusCode::CONFLICT, json!({"error": "server.web.url is not configured"}), ); - }; + } let state_token = format!("fabro-{}", ulid::Ulid::new()); let authorize_url = reqwest::Url::parse_with_params( @@ -226,11 +245,7 @@ async fn callback_github( json!({"error": "SESSION_SECRET is not configured"}), ); }; - let settings = state - .settings - .read() - .expect("settings lock poisoned") - .clone(); + let settings = state.server_settings(); let cookie_jar = parse_cookie_header(&headers); let stored_state = cookie_jar.get(OAUTH_STATE_COOKIE_NAME).map(Cookie::value); if stored_state != Some(params.state.as_str()) { @@ -238,13 +253,23 @@ async fn callback_github( return Redirect::to("/login").into_response(); } - let Some(client_id) = settings.github_client_id_str() else { + let Some(client_id) = settings.integrations.github.client_id.as_ref() else { error!("OAuth callback failed: client_id not configured"); return json_response( StatusCode::CONFLICT, json!({"error": "GitHub App client_id is not configured"}), ); }; + let client_id = match resolve_interp(client_id) { + Ok(client_id) => client_id, + Err(err) => { + error!(error = %err, "OAuth callback failed: client_id could not be resolved"); + return json_response( + StatusCode::CONFLICT, + json!({"error": format!("GitHub App client_id could not be resolved: {err}")}), + ); + } + }; let Some(client_secret) = state.secret_or_env("GITHUB_APP_CLIENT_SECRET") else { error!("OAuth callback failed: GITHUB_APP_CLIENT_SECRET not configured"); return json_response( @@ -252,13 +277,16 @@ async fn callback_github( json!({"error": "GITHUB_APP_CLIENT_SECRET is not configured"}), ); }; - let web_url = settings - .server_web() - .and_then(|w| w.url.as_ref()) - .map_or_else( - || "http://localhost:3000".to_string(), - InterpString::as_source, - ); + let web_url = match resolve_interp(&settings.web.url) { + Ok(web_url) => web_url, + Err(err) => { + error!(error = %err, "OAuth callback failed: server.web.url could not be resolved"); + return json_response( + StatusCode::CONFLICT, + json!({"error": format!("server.web.url could not be resolved: {err}")}), + ); + } + }; let http = reqwest::Client::new(); let token = match http @@ -356,13 +384,7 @@ async fn callback_github( _ => Vec::new(), }; - let allowed_usernames = settings - .server - .as_ref() - .and_then(|s| s.auth.as_ref()) - .and_then(|a| a.web.as_ref()) - .map(|w| w.allowed_usernames.clone()) - .unwrap_or_default(); + let allowed_usernames = settings.auth.web.allowed_usernames.clone(); if !allowed_usernames.is_empty() && !allowed_usernames.iter().any(|user| user == &profile.login) { warn!(login = %profile.login, "OAuth callback denied: username not in allowlist"); @@ -470,12 +492,12 @@ async fn auth_me(State(state): State>, headers: HeaderMap) -> Resp } async fn setup_status(State(state): State>) -> Response { - let settings = state - .settings - .read() - .expect("settings lock poisoned") - .clone(); - let configured = settings.github_client_id_str().is_some(); + let configured = state + .server_settings() + .integrations + .github + .client_id + .is_some(); Json(SetupStatusResponse { configured }).into_response() } @@ -630,11 +652,8 @@ async fn setup_register( // Re-parse the freshly-written settings file and swap it into the // in-memory state so subsequent OAuth requests see the new GitHub // App credentials without a server restart. - match fabro_config::ConfigLayer::load(&settings_path) { - Ok(reloaded) => { - let mut shared = state.settings.write().expect("settings lock poisoned"); - *shared = reloaded.into(); - } + match state.reload_settings_from_disk() { + Ok(()) => {} Err(err) => { error!(error = %err, path = %settings_path.display(), "Setup register failed: could not reload written settings config"); return json_response( diff --git a/lib/crates/fabro-types/src/settings/mod.rs b/lib/crates/fabro-types/src/settings/mod.rs index c1ff000b9..ac4d584ab 100644 --- a/lib/crates/fabro-types/src/settings/mod.rs +++ b/lib/crates/fabro-types/src/settings/mod.rs @@ -33,7 +33,15 @@ pub use model_ref::{ }; pub use project::ProjectLayer; pub use run::RunLayer; -pub use server::ServerLayer; +pub use server::{ + DiscordIntegrationSettings, GithubIntegrationSettings, GithubOauthSettings, + IntegrationWebhooksSettings, ObjectStoreSettings, ServerApiSettings, ServerArtifactsSettings, + ServerAuthApiJwtSettings, ServerAuthApiMtlsSettings, ServerAuthApiSettings, ServerAuthSettings, + ServerAuthWebProvidersSettings, ServerAuthWebSettings, ServerIntegrationsSettings, ServerLayer, + ServerListenSettings, ServerLoggingSettings, ServerSchedulerSettings, ServerSettings, + ServerSlateDbSettings, ServerStorageSettings, ServerWebSettings, SlackIntegrationSettings, + TeamsIntegrationSettings, TlsConfig, +}; pub use size::{ParseSizeError, Size}; pub use splice_array::{SPLICE_MARKER, SpliceArray, SpliceArrayError}; pub use tree::{ParseError, SettingsFile, parse_settings_file}; diff --git a/lib/crates/fabro-types/src/settings/server.rs b/lib/crates/fabro-types/src/settings/server.rs index c1931c620..34b4aff5e 100644 --- a/lib/crates/fabro-types/src/settings/server.rs +++ b/lib/crates/fabro-types/src/settings/server.rs @@ -6,12 +6,260 @@ //! use the same schema. use std::collections::HashMap; +use std::net::SocketAddr; +use std::time::Duration as StdDuration; use serde::{Deserialize, Serialize}; -use super::duration::Duration; +use super::duration::Duration as DurationLayer; use super::interp::InterpString; +/// A structurally resolved `[server]` view for consumers. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ServerSettings { + pub listen: ServerListenSettings, + pub api: ServerApiSettings, + pub web: ServerWebSettings, + pub auth: ServerAuthSettings, + pub storage: ServerStorageSettings, + pub artifacts: ServerArtifactsSettings, + pub slatedb: ServerSlateDbSettings, + pub scheduler: ServerSchedulerSettings, + pub logging: ServerLoggingSettings, + pub integrations: ServerIntegrationsSettings, +} + +impl Default for ServerSettings { + fn default() -> Self { + Self { + listen: ServerListenSettings::default(), + api: ServerApiSettings::default(), + web: ServerWebSettings::default(), + auth: ServerAuthSettings::default(), + storage: ServerStorageSettings::default(), + artifacts: ServerArtifactsSettings::default(), + slatedb: ServerSlateDbSettings::default(), + scheduler: ServerSchedulerSettings::default(), + logging: ServerLoggingSettings::default(), + integrations: ServerIntegrationsSettings::default(), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ServerListenSettings { + Tcp { + address: SocketAddr, + tls: Option, + }, + Unix { + path: InterpString, + }, +} + +impl Default for ServerListenSettings { + fn default() -> Self { + Self::Unix { + path: InterpString::parse(""), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TlsConfig { + pub cert: InterpString, + pub key: InterpString, + pub ca: InterpString, +} + +impl Default for TlsConfig { + fn default() -> Self { + Self { + cert: InterpString::parse(""), + key: InterpString::parse(""), + ca: InterpString::parse(""), + } + } +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ServerApiSettings { + pub url: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ServerWebSettings { + pub enabled: bool, + pub url: InterpString, +} + +impl Default for ServerWebSettings { + fn default() -> Self { + Self { + enabled: false, + url: InterpString::parse(""), + } + } +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ServerAuthSettings { + pub api: ServerAuthApiSettings, + pub web: ServerAuthWebSettings, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ServerAuthApiSettings { + pub jwt: Option, + pub mtls: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ServerAuthApiJwtSettings { + pub enabled: bool, + pub issuer: Option, + pub audience: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ServerAuthApiMtlsSettings { + pub enabled: bool, + pub ca: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ServerAuthWebSettings { + pub allowed_usernames: Vec, + pub providers: ServerAuthWebProvidersSettings, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ServerAuthWebProvidersSettings { + pub github: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct GithubOauthSettings { + pub enabled: bool, + pub client_id: Option, + pub client_secret: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ServerStorageSettings { + pub root: InterpString, +} + +impl Default for ServerStorageSettings { + fn default() -> Self { + Self { + root: InterpString::parse(""), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ServerArtifactsSettings { + pub prefix: InterpString, + pub store: ObjectStoreSettings, +} + +impl Default for ServerArtifactsSettings { + fn default() -> Self { + Self { + prefix: InterpString::parse(""), + store: ObjectStoreSettings::default(), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ServerSlateDbSettings { + pub prefix: InterpString, + pub store: ObjectStoreSettings, + pub flush_interval: StdDuration, +} + +impl Default for ServerSlateDbSettings { + fn default() -> Self { + Self { + prefix: InterpString::parse(""), + store: ObjectStoreSettings::default(), + flush_interval: StdDuration::ZERO, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ObjectStoreSettings { + Local { + root: InterpString, + }, + S3 { + bucket: InterpString, + region: InterpString, + endpoint: Option, + path_style: bool, + }, +} + +impl Default for ObjectStoreSettings { + fn default() -> Self { + Self::Local { + root: InterpString::parse(""), + } + } +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ServerSchedulerSettings { + pub max_concurrent_runs: usize, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ServerLoggingSettings { + pub level: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ServerIntegrationsSettings { + pub github: GithubIntegrationSettings, + pub slack: SlackIntegrationSettings, + pub discord: DiscordIntegrationSettings, + pub teams: TeamsIntegrationSettings, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct GithubIntegrationSettings { + pub enabled: bool, + pub app_id: Option, + pub client_id: Option, + pub slug: Option, + pub permissions: HashMap, + pub webhooks: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct SlackIntegrationSettings { + pub enabled: bool, + pub default_channel: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct DiscordIntegrationSettings { + pub enabled: bool, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct TeamsIntegrationSettings { + pub enabled: bool, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct IntegrationWebhooksSettings { + pub strategy: Option, +} + /// A sparse `[server]` layer as it appears in a single settings file. #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] @@ -194,7 +442,7 @@ pub struct ServerSlateDbLayer { #[serde(default, skip_serializing_if = "Option::is_none")] pub prefix: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub flush_interval: Option, + pub flush_interval: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub local: Option, #[serde(default, skip_serializing_if = "Option::is_none")]