diff --git a/lib/crates/fabro-cli/src/commands/server/start.rs b/lib/crates/fabro-cli/src/commands/server/start.rs index 7c9658945..5f9c0ea09 100644 --- a/lib/crates/fabro-cli/src/commands/server/start.rs +++ b/lib/crates/fabro-cli/src/commands/server/start.rs @@ -7,6 +7,7 @@ use chrono::Utc; use fabro_config::Storage; use fabro_config::user::default_socket_path; use fabro_server::bind::{Bind, BindRequest}; +use fabro_server::jwt_auth::FABRO_LOCAL_NO_AUTH_ENV; use fabro_server::serve; use fabro_server::serve::{DEFAULT_TCP_PORT, ServeArgs}; use fabro_util::terminal::Styles; @@ -230,7 +231,7 @@ fn execute_daemon( cmd.arg("--storage-dir").arg(storage_dir); if matches!(bind, BindRequest::Unix(_)) { - cmd.env("FABRO_LOCAL_NO_AUTH", "1"); + cmd.env(FABRO_LOCAL_NO_AUTH_ENV, "1"); } cmd.env_remove("FABRO_JSON"); 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 2987f887f..c4c344186 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs @@ -1,3 +1,4 @@ +use fabro_server::jwt_auth::FABRO_LOCAL_NO_AUTH_ENV; use fabro_test::{fabro_snapshot, test_context}; use std::process::Stdio; use std::sync::{Arc, Barrier}; @@ -148,7 +149,7 @@ fn start_with_tcp_host_only_bind_resolves_to_host_and_port() { // startup explicitly. let mut cmd = context.command(); cmd.env("FABRO_STORAGE_DIR", &storage_dir); - cmd.env("FABRO_LOCAL_NO_AUTH", "1"); + cmd.env(FABRO_LOCAL_NO_AUTH_ENV, "1"); cmd.args(["server", "start", "--dry-run", "--bind", "127.0.0.1"]); let output = cmd.output().expect("server start command should run"); assert!( @@ -211,7 +212,7 @@ fn start_with_tcp_host_only_bind_warns_and_falls_back_when_default_port_is_unava // startup explicitly. let mut cmd = context.command(); cmd.env("FABRO_STORAGE_DIR", &storage_dir); - cmd.env("FABRO_LOCAL_NO_AUTH", "1"); + cmd.env(FABRO_LOCAL_NO_AUTH_ENV, "1"); cmd.args(["server", "start", "--dry-run", "--bind", "127.0.0.1"]); fabro_snapshot!(filters, cmd, @" success: true diff --git a/lib/crates/fabro-config/src/config.rs b/lib/crates/fabro-config/src/config.rs index 15295cb43..b9c066ccb 100644 --- a/lib/crates/fabro-config/src/config.rs +++ b/lib/crates/fabro-config/src/config.rs @@ -13,6 +13,7 @@ use std::path::Path; use anyhow::Context; +use fabro_types::settings::accessors::resolve_goal_file_path; use fabro_types::settings::interp::InterpString; use fabro_types::settings::run::RunGoalLayer; use fabro_types::settings::{SettingsFile, parse_settings_file as parse_v2_settings_file}; @@ -42,11 +43,10 @@ fn resolve_goal_file_paths(file: &mut SettingsFile, base_dir: &Path) { return; } let literal = goal_file.as_source(); - let path = Path::new(&literal); - if path.is_absolute() { + if Path::new(&literal).is_absolute() { return; } - let absolute = base_dir.join(path); + let absolute = resolve_goal_file_path(&literal, base_dir); *goal_file = InterpString::parse(&absolute.to_string_lossy()); } diff --git a/lib/crates/fabro-server/src/tls_config.rs b/lib/crates/fabro-server/src/tls_config.rs index 13955639c..84341813e 100644 --- a/lib/crates/fabro-server/src/tls_config.rs +++ b/lib/crates/fabro-server/src/tls_config.rs @@ -1,11 +1,8 @@ //! Resolved TLS material extracted from `[server.listen.tls]`. //! -//! This module 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. It lives outside `jwt_auth.rs` because TLS material -//! is a listen-side concern, not an authentication strategy — the auth -//! resolver only cares about *whether* TLS is present (for mTLS support), -//! not about its contents. +//! 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; diff --git a/lib/crates/fabro-server/src/web_auth.rs b/lib/crates/fabro-server/src/web_auth.rs index 5ae2d3d9a..b13d5d5fb 100644 --- a/lib/crates/fabro-server/src/web_auth.rs +++ b/lib/crates/fabro-server/src/web_auth.rs @@ -563,7 +563,13 @@ async fn setup_register( // preserves existing comments, whitespace, and key ordering. The value- // tree parser (`toml::Value`) would strip all of that on round-trip. if let Some(parent) = settings_path.parent() { - let _ = std::fs::create_dir_all(parent); + if let Err(err) = std::fs::create_dir_all(parent) { + error!(error = %err, path = %parent.display(), "Setup register failed: could not create settings parent directory"); + return json_response( + StatusCode::INTERNAL_SERVER_ERROR, + json!({"error": format!("Failed to create settings directory: {err}")}), + ); + } } let existing = std::fs::read_to_string(&settings_path).unwrap_or_default(); let mut doc: toml_edit::DocumentMut = if existing.is_empty() { @@ -622,12 +628,20 @@ async fn setup_register( } // Re-parse the freshly-written settings file and swap it into the - // in-memory state. Stage 6.6 may split this differently when the web - // setup flow is reworked, but for now a round-trip through - // `ConfigLayer::load` keeps the live state consistent with disk. - if let Ok(reloaded) = fabro_config::ConfigLayer::load(&settings_path) { - let mut shared = state.settings.write().expect("settings lock poisoned"); - *shared = reloaded.into(); + // 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(); + } + Err(err) => { + error!(error = %err, path = %settings_path.display(), "Setup register failed: could not reload written settings config"); + return json_response( + StatusCode::INTERNAL_SERVER_ERROR, + json!({"error": format!("Failed to reload settings config after write: {err}")}), + ); + } } info!(slug = %data.slug, app_id = %data.id, "GitHub App registered successfully"); diff --git a/lib/crates/fabro-types/src/settings/accessors.rs b/lib/crates/fabro-types/src/settings/accessors.rs index 347128d46..aef8b2bbb 100644 --- a/lib/crates/fabro-types/src/settings/accessors.rs +++ b/lib/crates/fabro-types/src/settings/accessors.rs @@ -465,7 +465,8 @@ impl SettingsFile { /// Resolve a goal-file path string against `base_dir`. Absolute paths are /// used as-is; relative paths are joined onto `base_dir`. -fn resolve_goal_file_path(path_str: &str, base_dir: &Path) -> PathBuf { +#[must_use] +pub fn resolve_goal_file_path(path_str: &str, base_dir: &Path) -> PathBuf { let path = Path::new(path_str); if path.is_absolute() { path.to_path_buf() @@ -494,11 +495,9 @@ impl std::fmt::Display for ResolveGoalError { f, "failed to resolve run.goal.file: env var {var:?} referenced by ${{env.{var}}} is not set" ), - Self::Io { path, source } => write!( - f, - "failed to read run.goal.file at {}: {source}", - path.display() - ), + Self::Io { path, .. } => { + write!(f, "failed to read run.goal.file at {}", path.display()) + } } } } diff --git a/lib/crates/fabro-workflow/src/operations/source.rs b/lib/crates/fabro-workflow/src/operations/source.rs index e70f34d44..93c1e5321 100644 --- a/lib/crates/fabro-workflow/src/operations/source.rs +++ b/lib/crates/fabro-workflow/src/operations/source.rs @@ -140,7 +140,7 @@ fn resolve_goal_override( settings .resolve_run_goal(working_directory) .map(|opt| opt.map(|resolved| resolved.text)) - .map_err(|err| anyhow::anyhow!(err)) + .map_err(anyhow::Error::from) } #[cfg(test)]