diff --git a/lib/crates/fabro-cli/src/commands/config/mod.rs b/lib/crates/fabro-cli/src/commands/config/mod.rs index 414d43ae5..f81fd3c7b 100644 --- a/lib/crates/fabro-cli/src/commands/config/mod.rs +++ b/lib/crates/fabro-cli/src/commands/config/mod.rs @@ -9,7 +9,7 @@ use fabro_config::ConfigLayer; use fabro_config::effective_settings; use fabro_config::effective_settings::{EffectiveSettingsLayers, EffectiveSettingsMode}; use fabro_config::project; -use fabro_types::settings::v2::SettingsFile; +use fabro_types::settings::SettingsFile; fn config_layers( ctx: &CommandContext, @@ -70,11 +70,7 @@ async fn merged_config(args: &SettingsArgs) -> anyhow::Result { let ctx = CommandContext::for_target(&args.target)?; let target = user_config::resolve_server_target(&args.target, ctx.machine_settings())?; - // `retrieve_server_settings` currently returns a legacy flat `Settings`; - // route it through the v2 bridge shim for the consumer-side call. - // Stage 6.6 rewrites the API client to return v2 types directly. - let legacy_server = ctx.server().await?.retrieve_server_settings().await?; - let server_settings = legacy_settings_to_v2(&legacy_server); + let server_settings = ctx.server().await?.retrieve_server_settings().await?; let mode = match target { user_config::ServerTarget::HttpUrl { .. } => EffectiveSettingsMode::RemoteServer, user_config::ServerTarget::UnixSocket(_) => EffectiveSettingsMode::LocalDaemon, @@ -83,84 +79,6 @@ async fn merged_config(args: &SettingsArgs) -> anyhow::Result { effective_settings::resolve_settings(layers, Some(&server_settings), mode) } -/// Stopgap reverse bridge from the legacy flat `Settings` to a v2 -/// `SettingsFile`. `retrieve_server_settings` still returns the legacy -/// shape across the wire; the v2 resolver needs server-settings in v2 -/// shape. This reverse-maps the fields that matter for server-side -/// defaults (storage, scheduler, integrations, verbose, run model). -/// Stage 6.6 rewrites the API client to return v2 types directly and -/// deletes this helper. -fn legacy_settings_to_v2(legacy: &fabro_types::Settings) -> SettingsFile { - use fabro_types::settings::v2::cli::{CliLayer, CliOutputLayer, OutputVerbosity}; - use fabro_types::settings::v2::interp::InterpString; - use fabro_types::settings::v2::run::{RunLayer, RunModelLayer}; - use fabro_types::settings::v2::server::{ - GithubIntegrationLayer, ServerIntegrationsLayer, ServerLayer, ServerSchedulerLayer, - ServerStorageLayer, SlackIntegrationLayer, - }; - - let mut file = SettingsFile::default(); - - if let Some(storage_dir) = legacy.storage_dir.as_ref() { - let server = file.server.get_or_insert_with(ServerLayer::default); - server.storage = Some(ServerStorageLayer { - root: Some(InterpString::parse(&storage_dir.to_string_lossy())), - }); - } - if let Some(max_concurrent) = legacy.max_concurrent_runs { - let server = file.server.get_or_insert_with(ServerLayer::default); - server.scheduler = Some(ServerSchedulerLayer { - max_concurrent_runs: Some(max_concurrent), - }); - } - if let Some(git) = legacy.git.as_ref() { - let server = file.server.get_or_insert_with(ServerLayer::default); - let integrations = server - .integrations - .get_or_insert_with(ServerIntegrationsLayer::default); - let github = integrations - .github - .get_or_insert_with(GithubIntegrationLayer::default); - github.app_id = git.app_id.as_deref().map(InterpString::parse); - github.client_id = git.client_id.as_deref().map(InterpString::parse); - github.slug = git.slug.as_deref().map(InterpString::parse); - } - if let Some(slack) = legacy.slack.as_ref() { - let server = file.server.get_or_insert_with(ServerLayer::default); - let integrations = server - .integrations - .get_or_insert_with(ServerIntegrationsLayer::default); - integrations.slack = Some(SlackIntegrationLayer { - enabled: None, - default_channel: slack.default_channel.as_deref().map(InterpString::parse), - }); - } - if let Some(llm) = legacy.llm.as_ref() { - let run = file.run.get_or_insert_with(RunLayer::default); - run.model = Some(RunModelLayer { - provider: llm.provider.as_deref().map(InterpString::parse), - name: llm.model.as_deref().map(InterpString::parse), - fallbacks: Vec::new(), - }); - } - if let Some(vars) = legacy.vars.as_ref() { - let run = file.run.get_or_insert_with(RunLayer::default); - run.inputs = Some( - vars.iter() - .map(|(k, v)| (k.clone(), toml::Value::String(v.clone()))) - .collect(), - ); - } - if let Some(true) = legacy.verbose { - let cli = file.cli.get_or_insert_with(CliLayer::default); - cli.output = Some(CliOutputLayer { - verbosity: Some(OutputVerbosity::Verbose), - ..CliOutputLayer::default() - }); - } - file -} - pub(crate) async fn execute(args: &SettingsArgs, globals: &GlobalArgs) -> anyhow::Result<()> { let config = Box::pin(merged_config(args)).await?; if globals.json { diff --git a/lib/crates/fabro-cli/src/server_client.rs b/lib/crates/fabro-cli/src/server_client.rs index 5d83052fe..fe07f607b 100644 --- a/lib/crates/fabro-cli/src/server_client.rs +++ b/lib/crates/fabro-cli/src/server_client.rs @@ -8,8 +8,7 @@ use bytes::Bytes; use fabro_api::types; use fabro_server::bind::Bind; use fabro_store::{EventEnvelope, RunSummary, StageId}; -use fabro_types::Settings; -use fabro_types::settings::v2::SettingsFile; +use fabro_types::settings::SettingsFile; use fabro_types::{RunBlobId, RunEvent, RunId}; use fabro_workflow::artifact_snapshot::CapturedArtifactInfo; use futures::StreamExt; @@ -279,14 +278,16 @@ impl ServerStoreClient { &self.base_url } - pub(crate) async fn retrieve_server_settings(&self) -> Result { + pub(crate) async fn retrieve_server_settings(&self) -> Result { let response = self .client .retrieve_server_settings() .send() .await .map_err(map_api_error)?; - convert_type(response.into_inner()) + let raw = serde_json::Value::Object(response.into_inner().into()); + serde_json::from_value::(raw) + .context("server returned a settings payload that does not match the v2 schema") } pub(crate) async fn create_run_from_manifest( diff --git a/lib/crates/fabro-cli/tests/it/cmd/config.rs b/lib/crates/fabro-cli/tests/it/cmd/config.rs index fe90025c4..985c411c5 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/config.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/config.rs @@ -1,8 +1,8 @@ use std::path::PathBuf; +use fabro_config::ConfigLayer; use fabro_test::{fabro_snapshot, test_context}; -use fabro_types::Settings; -use fabro_types::settings::v2::SettingsFile; +use fabro_types::settings::SettingsFile; use httpmock::MockServer; use predicates::prelude::*; @@ -35,45 +35,29 @@ fn parse_settings(stdout: &[u8]) -> SettingsFile { serde_yaml::from_slice(stdout).expect("stdout should be valid YAML SettingsFile") } -fn server_settings_fixture() -> Settings { - toml::from_str( +fn server_settings_fixture() -> SettingsFile { + ConfigLayer::parse( r#" -storage_dir = "/srv/fabro-server" -verbose = false +_version = 1 -[llm] -model = "server-model" +[server.storage] +root = "/srv/fabro-server" + +[run.model] +name = "server-model" provider = "openai" -[vars] +[run.inputs] server_only = "1" shared = "server" "#, ) .expect("server settings fixture should parse") + .into() } -fn server_settings_body(settings: &Settings) -> String { - fn strip_nulls(value: &mut serde_json::Value) { - match value { - serde_json::Value::Object(map) => { - for child in map.values_mut() { - strip_nulls(child); - } - map.retain(|_, child| !child.is_null()); - } - serde_json::Value::Array(values) => { - for child in values { - strip_nulls(child); - } - } - _ => {} - } - } - - let mut value = serde_json::to_value(settings).expect("settings fixture should serialize"); - strip_nulls(&mut value); - serde_json::to_string(&value).expect("settings payload should serialize") +fn server_settings_body(settings: &SettingsFile) -> String { + serde_json::to_string(settings).expect("settings payload should serialize") } /// Set up home config and project config for settings command tests. diff --git a/lib/crates/fabro-server/src/lib.rs b/lib/crates/fabro-server/src/lib.rs index 85e49717a..a2465afdb 100644 --- a/lib/crates/fabro-server/src/lib.rs +++ b/lib/crates/fabro-server/src/lib.rs @@ -14,6 +14,7 @@ mod run_manifest; pub mod secret_store; pub mod serve; pub mod server; +mod settings_view; pub mod static_files; pub mod server_config { pub use fabro_types::Settings; diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 9cad3a01e..14e69da33 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -76,6 +76,7 @@ use crate::jwt_auth::{ }; use crate::run_manifest; use crate::secret_store::{SecretStore, SecretStoreError}; +use crate::settings_view; use crate::static_files; use crate::web_auth; use fabro_interview::{ @@ -1009,7 +1010,7 @@ fn real_routes() -> Router> { get(get_stage_artifact), ) .route("/runs/{id}/billing", get(get_run_billing)) - .route("/runs/{id}/settings", get(not_implemented)) + .route("/runs/{id}/settings", get(get_run_settings)) .route("/runs/{id}/steer", post(not_implemented)) .route("/runs/{id}/preview", post(generate_preview_url)) .route("/runs/{id}/ssh", post(create_ssh_access)) @@ -1064,13 +1065,8 @@ async fn get_server_settings( State(state): State>, ) -> Response { let settings = state.settings.read().unwrap().clone(); - // Stage 6.6 TODO: replace this with an explicit allow-list DTO that - // reads directly from the v2 tree and redacts env-sourced values via - // `InterpString` provenance. For now we serialize the full v2 - // `SettingsFile` as JSON so the web UI still has a response body -- - // the legacy `ServerSettings` OpenAPI schema will be rewritten in - // 6.6 alongside the fabro-web DTO updates. - let mut value = match serde_json::to_value(&settings) { + let redacted = settings_view::redact_for_api(&settings); + let mut value = match serde_json::to_value(&redacted) { Ok(value) => value, Err(err) => { return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) @@ -4036,6 +4032,47 @@ async fn get_run_status( } } +async fn get_run_settings( + _auth: AuthenticatedService, + State(state): State>, + Path(id): Path, +) -> Response { + let id = match parse_run_id_path(&id) { + Ok(id) => id, + Err(response) => return response, + }; + let run_store = match state.store.open_run_reader(&id).await { + Ok(store) => store, + Err(fabro_store::StoreError::RunNotFound(_)) => { + return ApiError::not_found("Run not found.").into_response(); + } + Err(err) => { + return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) + .into_response(); + } + }; + let run_state = match run_store.state().await { + Ok(state) => state, + Err(err) => { + return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) + .into_response(); + } + }; + let Some(run_record) = run_state.run else { + return ApiError::not_found("Run not found.").into_response(); + }; + let redacted = settings_view::redact_for_api(&run_record.settings); + let mut value = match serde_json::to_value(&redacted) { + Ok(value) => value, + Err(err) => { + return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) + .into_response(); + } + }; + strip_nulls(&mut value); + (StatusCode::OK, Json(value)).into_response() +} + async fn get_questions( _auth: AuthenticatedService, State(state): State>, diff --git a/lib/crates/fabro-server/src/settings_view.rs b/lib/crates/fabro-server/src/settings_view.rs new file mode 100644 index 000000000..8c1d23f5e --- /dev/null +++ b/lib/crates/fabro-server/src/settings_view.rs @@ -0,0 +1,237 @@ +//! Outward-facing view of [`SettingsFile`] for API responses. +//! +//! `/api/v1/settings` and `/api/v1/runs/:id/settings` return the server's v2 +//! [`SettingsFile`] directly as JSON so authenticated clients (the `fabro +//! settings` CLI, the web UI) can see the effective configuration. Before +//! serialization, this module drops the handful of fields that would leak +//! operational secrets or host-specific filesystem layout. +//! +//! ## What gets dropped +//! +//! Per the requirements doc (R16, R52, R53, R79–R81) and the Stage 6.6 plan: +//! +//! - `server.listen` — the whole subtree. Bind address reveals network +//! topology; `[server.listen.tls]` cert/key/ca paths reveal the host +//! filesystem layout. +//! - `server.auth.api.jwt.issuer` and `jwt.audience` — auth topology. Keeps +//! `enabled` so clients can tell whether JWT auth is on. +//! - `server.auth.api.mtls.ca` — filesystem path to the CA bundle. Keeps +//! `enabled`. +//! - `server.auth.web.providers.github.client_secret` — explicit OAuth +//! secret. Keeps `enabled` and `client_id` (the latter is public in OAuth). +//! +//! ## Why that's all +//! +//! The rest of the v2 tree is either: +//! +//! - A literal non-secret value (storage root, scheduler limit, integration +//! slug, feature flag), OR +//! - An [`InterpString`] containing `${env.NAME}` tokens. `InterpString`'s +//! default serialization preserves the *unresolved* template form, so the +//! wire payload surfaces `"Bearer ${env.TOKEN}"` instead of the resolved +//! secret value. No additional redaction pass is needed. +//! +//! Any future field that carries a raw secret in-band (without env +//! interpolation) must be added to the drop list below. + +use fabro_types::settings::SettingsFile; + +/// Build a redacted clone of `settings` safe to serialize outward. +/// +/// See the module docs for the drop-list rationale. +#[must_use] +pub(crate) fn redact_for_api(settings: &SettingsFile) -> SettingsFile { + let mut out = settings.clone(); + + if let Some(server) = out.server.as_mut() { + // Bind address + TLS key/cert paths: host operational details. + server.listen = None; + + if let Some(auth) = server.auth.as_mut() { + if let Some(api) = auth.api.as_mut() { + if let Some(jwt) = api.jwt.as_mut() { + jwt.issuer = None; + jwt.audience = None; + } + if let Some(mtls) = api.mtls.as_mut() { + mtls.ca = None; + } + } + if let Some(web) = auth.web.as_mut() { + if let Some(providers) = web.providers.as_mut() { + if let Some(github) = providers.github.as_mut() { + github.client_secret = None; + } + } + } + } + } + + out +} + +#[cfg(test)] +mod tests { + use super::*; + use fabro_config::ConfigLayer; + + fn parse(source: &str) -> SettingsFile { + ConfigLayer::parse(source) + .expect("fixture should parse") + .into() + } + + #[test] + fn drops_server_listen_entirely() { + let settings = parse( + r#" +_version = 1 + +[server.listen] +type = "tcp" +address = "127.0.0.1:32276" + +[server.listen.tls] +cert = "/etc/fabro/tls/cert.pem" +key = "/etc/fabro/tls/key.pem" +ca = "/etc/fabro/tls/ca.pem" +"#, + ); + let redacted = redact_for_api(&settings); + assert!(redacted.server.unwrap().listen.is_none()); + } + + #[test] + fn drops_jwt_issuer_and_audience_but_keeps_enabled() { + let settings = parse( + r#" +_version = 1 + +[server.auth.api.jwt] +enabled = true +issuer = "https://auth.example.com" +audience = "fabro" +"#, + ); + let redacted = redact_for_api(&settings); + let jwt = redacted + .server + .unwrap() + .auth + .unwrap() + .api + .unwrap() + .jwt + .unwrap(); + assert_eq!(jwt.enabled, Some(true)); + assert!(jwt.issuer.is_none()); + assert!(jwt.audience.is_none()); + } + + #[test] + fn drops_mtls_ca_path_but_keeps_enabled() { + let settings = parse( + r#" +_version = 1 + +[server.auth.api.mtls] +enabled = true +ca = "/etc/fabro/tls/ca.pem" +"#, + ); + let redacted = redact_for_api(&settings); + let mtls = redacted + .server + .unwrap() + .auth + .unwrap() + .api + .unwrap() + .mtls + .unwrap(); + assert_eq!(mtls.enabled, Some(true)); + assert!(mtls.ca.is_none()); + } + + #[test] + fn drops_github_client_secret_but_keeps_client_id_and_enabled() { + let settings = parse( + r#" +_version = 1 + +[server.auth.web.providers.github] +enabled = true +client_id = "Iv1.abcdef" +client_secret = "${env.GITHUB_OAUTH_SECRET}" +"#, + ); + let redacted = redact_for_api(&settings); + let github = redacted + .server + .unwrap() + .auth + .unwrap() + .web + .unwrap() + .providers + .unwrap() + .github + .unwrap(); + assert_eq!(github.enabled, Some(true)); + assert!(github.client_id.is_some()); + assert!(github.client_secret.is_none()); + } + + #[test] + fn preserves_run_cli_project_and_features() { + let settings = parse( + r#" +_version = 1 + +[project] +name = "Fabro" + +[run] +goal = "ship it" + +[run.model] +provider = "anthropic" +name = "sonnet" + +[cli.output] +verbosity = "verbose" + +[features] +session_sandboxes = true + +[server.scheduler] +max_concurrent_runs = 9 + +[server.storage] +root = "/srv/fabro" + +[server.integrations.github] +app_id = "12345" +client_id = "Iv1.abcdef" +slug = "fabro-app" +"#, + ); + let redacted = redact_for_api(&settings); + assert!(redacted.project.is_some()); + let run = redacted.run.unwrap(); + assert!(run.goal.is_some()); + assert!(run.model.is_some()); + assert!(redacted.cli.is_some()); + assert!(redacted.features.is_some()); + let server = redacted.server.unwrap(); + assert_eq!( + server.scheduler.and_then(|s| s.max_concurrent_runs), + Some(9) + ); + assert!(server.storage.is_some()); + let github = server.integrations.unwrap().github.unwrap(); + assert!(github.app_id.is_some()); + assert!(github.client_id.is_some()); + assert!(github.slug.is_some()); + } +}