diff --git a/bin/dev/check-boundary.sh b/bin/dev/check-boundary.sh index 7771d58ed..974dc4442 100755 --- a/bin/dev/check-boundary.sh +++ b/bin/dev/check-boundary.sh @@ -3,8 +3,14 @@ set -euo pipefail cd "$(dirname "$0")/../.." -symbol_allowlist=( +server_symbol_allowlist=( "lib/crates/fabro-cli/src/local_server.rs" + "lib/crates/fabro-cli/src/commands/run/runner.rs" + "lib/crates/fabro-cli/src/commands/pr/mod.rs" + "lib/crates/fabro-cli/src/commands/pr/create.rs" +) + +storage_allowlist=( "lib/crates/fabro-cli/src/commands/install.rs" "lib/crates/fabro-cli/src/commands/uninstall.rs" "lib/crates/fabro-cli/src/commands/run/runner.rs" @@ -50,11 +56,19 @@ fail=0 while IFS= read -r path; do [[ -z "$path" ]] && continue - if ! in_array "$path" "${symbol_allowlist[@]}"; then + if ! in_array "$path" "${server_symbol_allowlist[@]}"; then echo "boundary check failed: gated server symbol used outside allowlist: $path" >&2 fail=1 fi -done < <(find_matches 'fabro_config::resolve_server_from_file|fabro_config::resolve_server\b|fabro_config::ServerSettings::from_layer\b|fabro_config::ServerSettings::resolve\b|ServerSettings::from_layer\b|ServerSettings::resolve\b|Storage::new') +done < <(find_matches 'fabro_config::resolve_server_from_file|fabro_config::resolve_server\b|fabro_config::ServerSettings::from_layer\b|fabro_config::ServerSettings::resolve\b|ServerSettings::from_layer\b|ServerSettings::resolve\b') + +while IFS= read -r path; do + [[ -z "$path" ]] && continue + if ! in_array "$path" "${storage_allowlist[@]}"; then + echo "boundary check failed: Storage::new used outside allowlist: $path" >&2 + fail=1 + fi +done < <(find_matches 'Storage::new') while IFS= read -r path; do [[ -z "$path" ]] && continue diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 886524266..fa9d26f73 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -1277,8 +1277,7 @@ async fn write_artifact_store_metadata( settings: &SettingsLayer, fabro_version: &str, ) -> Result<()> { - let resolved = - fabro_config::ServerSettings::from_layer(settings).map_err(anyhow::Error::from)?; + let resolved = local_server::server_settings(settings)?; let (object_store, prefix) = serve::build_artifact_object_store(&resolved.server)?; let artifact_store = ArtifactStore::new(object_store, prefix); artifact_store.write_metadata(fabro_version).await?; @@ -1797,7 +1796,7 @@ async fn run_install_inner( .context("failed to parse generated settings.toml")?, args.storage_dir.as_deref(), ); - fabro_config::ServerSettings::from_layer(&install_settings).map_err(anyhow::Error::from)?; + local_server::server_settings(&install_settings)?; // Secrets and auth material { diff --git a/lib/crates/fabro-cli/src/local_server.rs b/lib/crates/fabro-cli/src/local_server.rs index cdba3b556..5128e70da 100644 --- a/lib/crates/fabro-cli/src/local_server.rs +++ b/lib/crates/fabro-cli/src/local_server.rs @@ -26,8 +26,12 @@ pub(crate) fn bind_request( resolve_bind_request_from_settings(settings, cli_override) } +pub(crate) fn server_settings(settings: &SettingsLayer) -> Result { + fabro_config::ServerSettings::from_layer(settings).map_err(anyhow::Error::from) +} + pub(crate) fn auth_methods(settings: &SettingsLayer) -> Vec { - fabro_config::ServerSettings::from_layer(settings) + server_settings(settings) .map(|resolved| resolved.server.auth.methods) .unwrap_or_default() } diff --git a/lib/crates/fabro-cli/tests/it/scenario/smoke.rs b/lib/crates/fabro-cli/tests/it/scenario/smoke.rs index 13160b955..cf1db65fb 100644 --- a/lib/crates/fabro-cli/tests/it/scenario/smoke.rs +++ b/lib/crates/fabro-cli/tests/it/scenario/smoke.rs @@ -134,16 +134,12 @@ fn help_smoke_covers_high_cost_commands() { ----- stdout ----- Inspect effective settings - Usage: fabro settings [OPTIONS] [WORKFLOW] - - Arguments: - [WORKFLOW] Optional workflow name, .fabro path, or .toml run config to overlay + Usage: fabro settings [OPTIONS] Options: --json Output as JSON [env: FABRO_JSON=] --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] - --local Show only locally resolved settings and skip the server call --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] --quiet Suppress non-essential output [env: FABRO_QUIET=] --verbose Enable verbose output [env: FABRO_VERBOSE=] diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index 49ae4d959..0055eb108 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -901,6 +901,7 @@ mod tests { resolve_server_settings, resolve_startup_github_webhook_ip_allowlist, router_web_enabled, server_bind_title, server_title, }; + use crate::server::create_app_state_with_options; fn parse_settings(source: &str) -> SettingsLayer { let mut layer = parse_settings_layer(source).expect("v2 fixture should parse"); @@ -935,6 +936,42 @@ mod tests { assert_eq!(storage_root.as_deref(), Some("/srv/fabro-storage")); } + #[test] + fn app_state_server_settings_use_effective_runtime_layer_storage_override() { + let base = parse_settings( + r#" +_version = 1 + +[server.storage] +root = "/srv/from-disk" +"#, + ); + let args = ServeArgs { + bind: None, + model: None, + provider: None, + sandbox: None, + web: false, + no_web: false, + max_concurrent_runs: None, + config: None, + #[cfg(debug_assertions)] + watch_web: false, + }; + + let effective = apply_runtime_settings(&base, &args, &PathBuf::from("/srv/from-runtime")); + let state = create_app_state_with_options(effective, 5); + + assert_eq!( + state.server_settings().server.storage.root.as_source(), + "/srv/from-runtime" + ); + assert_eq!( + state.server_storage_dir(), + PathBuf::from("/srv/from-runtime") + ); + } + #[test] fn apply_runtime_settings_enables_web_from_cli_flag() { let base = parse_settings( diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index fa97ac90e..6d2526cb1 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -7397,6 +7397,65 @@ url = "{url}" } } + #[test] + fn replace_settings_updates_layer_and_typed_server_settings() { + let state = create_app_state_with_options( + fabro_config::parse_settings_layer( + r#" +_version = 1 + +[server.auth] +methods = ["dev-token"] + +[server.web] +url = "http://old.example.com" + +[server.storage] +root = "/srv/old" +"#, + ) + .expect("settings fixture should parse"), + 5, + ); + + let updated = fabro_config::parse_settings_layer( + r#" +_version = 1 + +[server.auth] +methods = ["dev-token"] + +[server.web] +url = "http://new.example.com" + +[server.storage] +root = "/srv/new" +"#, + ) + .expect("settings fixture should parse"); + + state + .replace_settings(updated) + .expect("valid settings should replace current state"); + + assert_eq!(state.canonical_origin().unwrap(), "http://new.example.com"); + assert_eq!( + state.server_settings().server.storage.root.as_source(), + "/srv/new" + ); + + let layer_root = state + .settings + .read() + .expect("settings lock poisoned") + .server + .as_ref() + .and_then(|server| server.storage.as_ref()) + .and_then(|storage| storage.root.as_ref()) + .map(InterpString::as_source); + assert_eq!(layer_root.as_deref(), Some("/srv/new")); + } + #[tokio::test] async fn create_secret_stores_file_secret_and_excludes_it_from_snapshot() { let state = create_app_state();