diff --git a/Cargo.lock b/Cargo.lock index 183795d8..8b4b9330 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1627,8 +1627,10 @@ dependencies = [ name = "firma-config" version = "0.1.1" dependencies = [ + "anyhow", "clap", "dirs", + "fs-err", "pretty_assertions", "serde", "tempfile", @@ -1877,6 +1879,15 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs-err" +version = "3.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73fde052dbfc920003cfd2c8e2c6e6d4cc7c1091538c3a24226cec0665ab08c0" +dependencies = [ + "autocfg", +] + [[package]] name = "fs_extra" version = "1.3.0" diff --git a/Cargo.toml b/Cargo.toml index 91704454..7e2ea967 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,6 +53,7 @@ firma-protobuf = "0.1.1" firma-run = { path = "crates/firma-run" } firma-sidecar = { path = "crates/firma-sidecar" } firma-stack = { path = "crates/firma-stack" } +fs-err = "3" governor = "0.10" hex = "0.4" http-body = "1" diff --git a/crates/firma-config/Cargo.toml b/crates/firma-config/Cargo.toml index 9e8376a6..3abdeee6 100644 --- a/crates/firma-config/Cargo.toml +++ b/crates/firma-config/Cargo.toml @@ -13,8 +13,10 @@ workspace = true clap = ["dep:clap"] [dependencies] +anyhow = { workspace = true } clap = { workspace = true, optional = true, features = ["derive"] } dirs = { workspace = true } +fs-err = { workspace = true } serde = { workspace = true } thiserror = { workspace = true } toml = { workspace = true } diff --git a/crates/firma-config/src/lib.rs b/crates/firma-config/src/lib.rs index 0c44b2bb..b0a4e9d0 100644 --- a/crates/firma-config/src/lib.rs +++ b/crates/firma-config/src/lib.rs @@ -5,16 +5,12 @@ //! paths. Fail-closed: no silent fallback to an empty config. mod profile; -mod provider; mod resolver; mod schema; /// Canonical config file name shared by every binary. pub const CONFIG_FILE_NAME: &str = "firma.toml"; -/// Application subdir under a platform config root (e.g. `~/.config/firma`). -pub const CONFIG_SUBDIR: &str = "firma"; pub use profile::AgentProfile; -pub use provider::{DirProvider, SystemDirs}; -pub use resolver::{ConfigResolveError, ConfigSource, ResolvedConfig, resolve_config}; +pub use resolver::{ConfigResolveError, ConfigSource, ResolvedConfig, SystemDirs}; pub use schema::{FirmaConfig, load_section}; diff --git a/crates/firma-config/src/provider.rs b/crates/firma-config/src/provider.rs deleted file mode 100644 index 036e2155..00000000 --- a/crates/firma-config/src/provider.rs +++ /dev/null @@ -1,59 +0,0 @@ -//! Directory sources for config discovery. -//! -//! The trait is injected so resolution is unit-testable without -//! mutating the real environment or the filesystem. - -use std::path::PathBuf; - -/// Supplies the candidate config locations. -pub trait DirProvider { - /// `$FIRMA_CONFIG` if set — direct path to the config file. - fn env_config_file(&self) -> Option; - /// Current working directory for the `.firma/firma.toml` walk-up. - fn cwd(&self) -> Option; -} - -/// Production [`DirProvider`] backed by the real environment. -#[derive(Debug, Default, Clone, Copy)] -pub struct SystemDirs; - -impl DirProvider for SystemDirs { - fn env_config_file(&self) -> Option { - std::env::var_os("FIRMA_CONFIG") - .filter(|v| !v.is_empty()) - .map(PathBuf::from) - } - - fn cwd(&self) -> Option { - std::env::current_dir().ok() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - struct Fake { - env: Option, - cwd: Option, - } - - impl DirProvider for Fake { - fn env_config_file(&self) -> Option { - self.env.clone() - } - fn cwd(&self) -> Option { - self.cwd.clone() - } - } - - #[test] - fn fake_provider_exposes_tiers() { - let f = Fake { - env: Some(PathBuf::from("/env/firma.toml")), - cwd: Some(PathBuf::from("/cwd")), - }; - assert_eq!(f.env_config_file(), Some(PathBuf::from("/env/firma.toml"))); - assert_eq!(f.cwd(), Some(PathBuf::from("/cwd"))); - } -} diff --git a/crates/firma-config/src/resolver.rs b/crates/firma-config/src/resolver.rs index e5145f1f..bf1a60d1 100644 --- a/crates/firma-config/src/resolver.rs +++ b/crates/firma-config/src/resolver.rs @@ -1,8 +1,41 @@ -//! Config-file discovery: fixed precedence, first existing file wins. +//! Config-file discovery: fixed precedence, first selected file wins. -use std::path::{Path, PathBuf}; +use std::{ + io::ErrorKind, + path::{Path, PathBuf}, +}; -use crate::provider::DirProvider; +use fs_err as fs; + +use crate::FirmaConfig; + +/// Config discovery inputs backed by the real process environment. +#[derive(Debug, Default, Clone)] +pub struct SystemDirs { + walk_ceiling: Option, +} + +impl SystemDirs { + /// Create a provider backed by the real process environment. + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// Set the inclusive directory where project-local walk-up stops. + #[must_use] + pub fn walk_up_to(mut self, ceiling: impl Into) -> Self { + self.walk_ceiling = Some(ceiling.into()); + self + } + + /// `$FIRMA_CONFIG` if set — direct path to the config file. + fn env_config_file() -> Option { + std::env::var_os("FIRMA_CONFIG") + .filter(|v| !v.is_empty()) + .map(PathBuf::from) + } +} /// Where the resolved config came from. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -16,221 +49,119 @@ pub enum ConfigSource { } /// Resolved config location plus the dir to re-base defaults against. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone)] pub struct ResolvedConfig { - /// The `firma.toml` that won. - pub config_file: PathBuf, - /// Its parent — base for unset resource paths. - pub config_dir: PathBuf, /// Provenance, for startup logs. pub source: ConfigSource, + /// Config content loaded during resolution. + pub config: FirmaConfig, } -/// Resolution failure. Fail-closed; lists every directory searched. -#[derive(Debug, thiserror::Error)] -pub enum ConfigResolveError { - #[error( - "no firma.toml found for `{subcommand}`; searched:\n{}", - searched.iter().map(|p| format!(" - {}", p.display())) - .collect::>().join("\n") - )] - NotFound { - subcommand: String, - searched: Vec, - }, -} - -use crate::CONFIG_FILE_NAME as FILE_NAME; - -fn dir_of(path: &Path) -> PathBuf { - path.parent() - .filter(|p| !p.as_os_str().is_empty()) - .map_or_else(|| PathBuf::from("."), Path::to_path_buf) -} - -/// Resolve the config file for `subcommand`. -/// -/// Priority: -/// 1. `cli_override` (`--config` flag) — always wins. -/// 2. `$FIRMA_CONFIG` env var — direct path to the config file. -/// 3. Walk up from cwd looking for `.firma/firma.toml`. -/// -/// # Errors -/// -/// Returns [`ConfigResolveError::NotFound`] (listing every searched path) -/// when no file exists in any tier. -pub fn resolve_config( - subcommand: &str, - cli_override: Option<&Path>, - provider: &dyn DirProvider, -) -> Result { - if let Some(path) = cli_override { - return Ok(ResolvedConfig { - config_file: path.to_path_buf(), - config_dir: dir_of(path), - source: ConfigSource::Flag, - }); +impl ResolvedConfig { + fn new(source: ConfigSource, config: FirmaConfig) -> Self { + Self { source, config } } - if let Some(env_path) = provider.env_config_file() { - return Ok(ResolvedConfig { - config_dir: dir_of(&env_path), - config_file: env_path, - source: ConfigSource::EnvVar, - }); + /// The `firma.toml` that won. + #[must_use] + pub fn config_file(&self) -> &Path { + self.config.origin() } - let mut searched: Vec = Vec::new(); - if let Some(cwd) = provider.cwd() { - for dir in cwd.ancestors() { - let candidate = dir.join(".firma").join(FILE_NAME); - if candidate.is_file() { - return Ok(ResolvedConfig { - config_dir: dir_of(&candidate), - config_file: candidate, - source: ConfigSource::ProjectLocal, - }); - } - searched.push(candidate); - } + /// The resolved config file's parent, used to re-base unset resource paths. + #[must_use] + pub fn config_dir(&self) -> PathBuf { + self.config_file() + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .map_or_else(|| PathBuf::from("."), Path::to_path_buf) } +} - Err(ConfigResolveError::NotFound { - subcommand: subcommand.to_string(), - searched, - }) +/// Resolution failure. Fail-closed; a selected config could not be loaded. +#[derive(Debug, thiserror::Error)] +#[error("failed to load `{path}` from {config_source:?}: {reason:#}")] +pub struct ConfigResolveError { + pub config_source: ConfigSource, + pub path: PathBuf, + pub reason: anyhow::Error, } -#[cfg(test)] -mod tests { - use super::*; - use tempfile::tempdir; +use crate::CONFIG_FILE_NAME as FILE_NAME; - struct Fake { - env: Option, - cwd: Option, - } +fn load_path(path: &Path, source: ConfigSource) -> Result { + FirmaConfig::load(path) + .map(|config| ResolvedConfig::new(source, config)) + .map_err(|reason| ConfigResolveError { + config_source: source, + path: path.to_path_buf(), + reason, + }) +} - impl DirProvider for Fake { - fn env_config_file(&self) -> Option { - self.env.clone() +impl SystemDirs { + /// Resolve and load the config file. + /// + /// Priority: + /// 1. `cli_override` (`--config` flag) — always wins. + /// 2. `$FIRMA_CONFIG` env var — direct path to the config file. + /// 3. Walk up from cwd looking for `.firma/firma.toml`. + /// + /// # Errors + /// + /// Returns `Ok(None)` when no file exists in any discovery tier. Returns + /// [`ConfigResolveError`] when a selected file cannot be read or parsed. + pub fn resolve_config( + &self, + cli_override: Option<&Path>, + ) -> Result, ConfigResolveError> { + if let Some(path) = cli_override { + return load_path(path, ConfigSource::Flag).map(Some); } - fn cwd(&self) -> Option { - self.cwd.clone() - } - } - fn empty() -> Fake { - Fake { - env: None, - cwd: None, + if let Some(env_path) = Self::env_config_file() { + return load_path(&env_path, ConfigSource::EnvVar).map(Some); } - } - - fn touch_project_local(dir: &Path) -> PathBuf { - let d = dir.join(".firma"); - std::fs::create_dir_all(&d).unwrap(); - let f = d.join(FILE_NAME); - std::fs::write(&f, "").unwrap(); - f - } - - #[test] - fn flag_wins_over_everything() { - let tmp = tempdir().unwrap(); - let flag = tmp.path().join("explicit.toml"); - std::fs::write(&flag, "").unwrap(); - let r = resolve_config("sidecar", Some(&flag), &empty()).unwrap(); - assert_eq!(r.source, ConfigSource::Flag); - assert_eq!(r.config_file, flag); - assert_eq!(r.config_dir, tmp.path()); - } - - #[test] - fn env_var_beats_project_local() { - let tmp = tempdir().unwrap(); - let env_file = tmp.path().join("env.toml"); - std::fs::write(&env_file, "").unwrap(); - let cwd = tmp.path().join("project"); - std::fs::create_dir_all(&cwd).unwrap(); - touch_project_local(&cwd); - let f = Fake { - env: Some(env_file.clone()), - cwd: Some(cwd), - }; - let r = resolve_config("sidecar", None, &f).unwrap(); - assert_eq!(r.source, ConfigSource::EnvVar); - assert_eq!(r.config_file, env_file); - } - - #[test] - fn project_local_found_in_cwd() { - let tmp = tempdir().unwrap(); - let project_file = touch_project_local(tmp.path()); - let f = Fake { - env: None, - cwd: Some(tmp.path().to_path_buf()), - }; - let r = resolve_config("sidecar", None, &f).unwrap(); - assert_eq!(r.source, ConfigSource::ProjectLocal); - assert_eq!(r.config_file, project_file); - } - #[test] - fn project_local_walks_up_to_ancestor() { - let tmp = tempdir().unwrap(); - let project = tmp.path().join("project"); - let nested = project.join("sub").join("deep"); - std::fs::create_dir_all(&nested).unwrap(); - let project_file = touch_project_local(&project); - let f = Fake { - env: None, - cwd: Some(nested), - }; - let r = resolve_config("sidecar", None, &f).unwrap(); - assert_eq!(r.source, ConfigSource::ProjectLocal); - assert_eq!(r.config_file, project_file); - } - - #[test] - fn project_local_closest_ancestor_wins() { - let tmp = tempdir().unwrap(); - let project = tmp.path().join("project"); - let nested = project.join("sub").join("deep"); - std::fs::create_dir_all(&nested).unwrap(); - touch_project_local(&project); - let nested_file = touch_project_local(project.join("sub").as_path()); - let f = Fake { - env: None, - cwd: Some(nested), - }; - let r = resolve_config("sidecar", None, &f).unwrap(); - assert_eq!(r.source, ConfigSource::ProjectLocal); - assert_eq!(r.config_file, nested_file); - } - - #[test] - fn nothing_found_lists_searched_paths() { - let tmp = tempdir().unwrap(); - let cwd = tmp.path().join("project"); - std::fs::create_dir_all(&cwd).unwrap(); - let f = Fake { - env: None, - cwd: Some(cwd.clone()), - }; - let err = resolve_config("sidecar", None, &f).unwrap_err(); - let ConfigResolveError::NotFound { - subcommand, - searched, - } = err; - assert_eq!(subcommand, "sidecar"); - assert!(searched.iter().any(|p| p.starts_with(&cwd))); - } + 'walk_up: { + let Some(cwd) = std::env::current_dir().ok() else { + break 'walk_up; + }; + let walk_ceiling = self.walk_ceiling.as_deref(); + if walk_ceiling.is_some_and(|ceiling| !cwd.starts_with(ceiling)) { + break 'walk_up; + } + for dir in cwd.ancestors() { + let candidate = dir.join(".firma").join(FILE_NAME); + match fs::read_to_string(&candidate) { + Ok(text) => { + let config = FirmaConfig::parse(&candidate, &text).map_err(|reason| { + ConfigResolveError { + config_source: ConfigSource::ProjectLocal, + path: candidate.clone(), + reason, + } + })?; + return Ok(Some(ResolvedConfig::new( + ConfigSource::ProjectLocal, + config, + ))); + } + Err(error) if error.kind() == ErrorKind::NotFound => {} + Err(error) => { + return Err(ConfigResolveError { + config_source: ConfigSource::ProjectLocal, + path: candidate, + reason: error.into(), + }); + } + } + if walk_ceiling.is_some_and(|ceiling| dir == ceiling) { + break; + } + } + } - #[test] - fn nothing_found_when_cwd_none() { - let err = resolve_config("sidecar", None, &empty()).unwrap_err(); - let ConfigResolveError::NotFound { searched, .. } = err; - assert!(searched.is_empty()); + Ok(None) } } diff --git a/crates/firma-config/src/schema.rs b/crates/firma-config/src/schema.rs index a259fa62..d96818a3 100644 --- a/crates/firma-config/src/schema.rs +++ b/crates/firma-config/src/schema.rs @@ -1,6 +1,9 @@ //! Section extraction so one `firma.toml` serves every subcommand. -use std::path::Path; +use std::path::{Path, PathBuf}; + +use anyhow::{Context as _, anyhow, bail}; +use fs_err as fs; /// A `firma.toml` parsed once. Use when more than one `[section]` is /// needed from the same file (e.g. `doctor` reads both `[authority]` @@ -8,7 +11,7 @@ use std::path::Path; #[derive(Debug, Clone)] pub struct FirmaConfig { table: toml::Table, - origin: String, + origin: PathBuf, } impl FirmaConfig { @@ -16,16 +19,24 @@ impl FirmaConfig { /// /// # Errors /// - /// Returns the read or parse error as a string (caller wraps it). - pub fn load(path: &Path) -> Result { - let text = std::fs::read_to_string(path).map_err(|e| format!("{}: {e}", path.display()))?; + /// Returns the read or parse error. + pub fn load(path: &Path) -> anyhow::Result { + let text = fs::read_to_string(path).with_context(|| format!("read {}", path.display()))?; + Self::parse(path, &text) + } + + pub(crate) fn parse(origin: impl Into, text: &str) -> anyhow::Result { + let origin = origin.into(); let table: toml::Table = text .parse() - .map_err(|e: toml::de::Error| format!("{}: {e}", path.display()))?; - Ok(Self { - table, - origin: path.display().to_string(), - }) + .with_context(|| format!("parse {}", origin.display()))?; + Ok(Self { table, origin }) + } + + /// Path where this config was originally loaded from. + #[must_use] + pub fn origin(&self) -> &Path { + &self.origin } /// The named section body re-serialized as standalone TOML. @@ -36,30 +47,30 @@ impl FirmaConfig { /// /// # Errors /// - /// Returns a "missing section" or serialization error string. - pub fn section(&self, section: &str) -> Result { - let parts: Vec<&str> = section.split('.').collect(); + /// Returns a "missing section" or serialization error. + pub fn section(&self, section_path: &str) -> anyhow::Result { + let parts: Vec<&str> = section_path.split('.').collect(); let mut current = &self.table; for &part in &parts[..parts.len() - 1] { match current.get(part) { Some(toml::Value::Table(t)) => current = t, _ => { - return Err(format!( - "{}: missing required `[{section}]` section", - self.origin - )); + bail!( + "{}: missing required `[{section_path}]` section", + self.origin.display() + ); } } } - let last = parts.last().copied().unwrap_or(section); + let last = parts.last().copied().unwrap_or(section_path); match current.get(last) { Some(toml::Value::Table(sub)) => { - toml::to_string(sub).map_err(|e| format!("{}: {e}", self.origin)) + toml::to_string(sub).map_err(|e| anyhow!("{}: {e}", self.origin.display())) } - _ => Err(format!( - "{}: missing required `[{section}]` section", - self.origin - )), + _ => bail!( + "{}: missing required `[{section_path}]` section", + self.origin.display() + ), } } } @@ -71,82 +82,7 @@ impl FirmaConfig { /// /// # Errors /// -/// Returns the read/parse error, or a "missing section" error, as a -/// string (caller wraps it). -pub fn load_section(path: &Path, section: &str) -> Result { +/// Returns the read/parse error, or a "missing section" error. +pub fn load_section(path: &Path, section: &str) -> anyhow::Result { FirmaConfig::load(path)?.section(section) } - -#[cfg(test)] -mod tests { - use super::*; - use tempfile::tempdir; - - #[test] - fn extracts_named_section() { - let tmp = tempdir().unwrap(); - let p = tmp.path().join("firma.toml"); - std::fs::write(&p, "[sidecar]\nfoo = 1\n[authority]\nbar = 2\n").unwrap(); - let out = load_section(&p, "sidecar").unwrap(); - let t: toml::Table = out.parse().unwrap(); - assert_eq!(t.get("foo").and_then(toml::Value::as_integer), Some(1)); - assert!(t.get("bar").is_none()); - } - - #[test] - fn nested_subtables_are_preserved() { - let tmp = tempdir().unwrap(); - let p = tmp.path().join("firma.toml"); - std::fs::write( - &p, - "[sidecar.interceptor]\nmode = \"http_proxy\"\n[sidecar.authority]\nurl = \"http://x\"\n", - ) - .unwrap(); - let out = load_section(&p, "sidecar").unwrap(); - let t: toml::Table = out.parse().unwrap(); - assert!( - t.get("interceptor") - .and_then(toml::Value::as_table) - .is_some() - ); - assert!(t.get("authority").and_then(toml::Value::as_table).is_some()); - } - - #[test] - fn missing_section_is_an_error() { - let tmp = tempdir().unwrap(); - let p = tmp.path().join("firma.toml"); - std::fs::write(&p, "[authority]\nbar = 2\n").unwrap(); - let err = load_section(&p, "sidecar").unwrap_err(); - assert!(err.contains("sidecar"), "error names the section: {err}"); - } - - #[test] - fn dotted_path_extracts_nested_section() { - let tmp = tempdir().unwrap(); - let p = tmp.path().join("firma.toml"); - std::fs::write( - &p, - "[sidecar.policy]\ndir = \".\"\n[sidecar.authority]\nurl = \"https://x\"\n", - ) - .unwrap(); - let policy = load_section(&p, "sidecar.policy").unwrap(); - let t: toml::Table = policy.parse().unwrap(); - assert_eq!(t.get("dir").and_then(toml::Value::as_str), Some(".")); - let connect = load_section(&p, "sidecar.authority").unwrap(); - let t2: toml::Table = connect.parse().unwrap(); - assert_eq!( - t2.get("url").and_then(toml::Value::as_str), - Some("https://x") - ); - } - - #[test] - fn dotted_path_missing_is_an_error() { - let tmp = tempdir().unwrap(); - let p = tmp.path().join("firma.toml"); - std::fs::write(&p, "[sidecar.policy]\ndir = \".\"\n").unwrap(); - let err = load_section(&p, "sidecar.authority").unwrap_err(); - assert!(err.contains("sidecar.authority"), "error: {err}"); - } -} diff --git a/crates/firma-config/tests/integration/helper.rs b/crates/firma-config/tests/integration/helper.rs new file mode 100644 index 00000000..475adfa7 --- /dev/null +++ b/crates/firma-config/tests/integration/helper.rs @@ -0,0 +1,76 @@ +//! Shared integration-test helpers. + +use std::{ffi::OsString, path::Path}; + +use fs_err as fs; + +/// Run `test` in an isolated process, using a temporary working tree. +/// +/// This helper requires `cargo nextest` (`NEXTEST=1`) because it mutates +/// process-global state: it clears every `FIRMA_*` environment variable and +/// sets the process cwd. Nextest runs each test in its own process, so those +/// mutations cannot race with other tests. +/// +/// The helper creates a temp directory, changes cwd to that temp root, +/// canonicalizes it, and passes the canonical root to `test`. The test body may +/// create additional directories and move cwd again; that remains safe under the +/// nextest process-isolation guard. The temp directory is removed after `test` +/// returns. +pub fn run_isolated(test: impl FnOnce(&Path)) { + run_isolated_with_env(|_| Vec::new(), test); +} + +/// Run `test` in an isolated process with root-derived environment variables. +/// +/// This behaves like [`run_isolated`], then calls `env` with the canonical temp +/// root and sets each returned environment variable before running `test`. +/// Existing `FIRMA_*` variables are always cleared before any requested env vars +/// are set. +pub fn run_isolated_with_env( + env: impl FnOnce(&Path) -> Vec<(OsString, OsString)>, + test: impl FnOnce(&Path), +) { + assert_eq!( + std::env::var("NEXTEST").as_deref(), + Ok("1"), + "run_isolated mutates process cwd/env and must run under cargo nextest" + ); + clear_firma_env(); + + let tmp = tempfile::tempdir().expect("create isolated temporary directory"); + let root = fs::canonicalize(tmp.path()).expect("canonicalize isolated temporary directory"); + std::env::set_current_dir(&root).expect("set cwd to isolated temporary directory"); + set_env_vars(env(&root)); + + test(&root); +} + +#[expect( + unsafe_code, + reason = "nextest runs one process per test; env is cleared before test work starts" +)] +/// Clears all `FIRMA_*` environment variables for the current process. +fn clear_firma_env() { + for (key, _) in std::env::vars_os() { + let Some(key) = key.as_os_str().to_str() else { + continue; + }; + if key.starts_with("FIRMA_") { + // SAFETY: guarded by `NEXTEST=1`; nextest executes each test in + // its own process, and this helper runs before the test body. + unsafe { std::env::remove_var(key) }; + } + } +} + +#[expect( + unsafe_code, + reason = "nextest runs one process per test; env is set before test work starts" +)] +fn set_env_vars(vars: Vec<(OsString, OsString)>) { + for (key, value) in vars { + // SAFETY: guarded by `NEXTEST=1`; nextest executes each test in its own + // process, and this helper sets env before invoking the test body. + unsafe { std::env::set_var(key, value) }; + } +} diff --git a/crates/firma-config/tests/integration/main.rs b/crates/firma-config/tests/integration/main.rs new file mode 100644 index 00000000..4c8aea83 --- /dev/null +++ b/crates/firma-config/tests/integration/main.rs @@ -0,0 +1,5 @@ +#![allow(clippy::expect_used)] +mod helper; +mod resolution; +mod resolver; +mod schema; diff --git a/crates/firma-config/tests/integration/resolution.rs b/crates/firma-config/tests/integration/resolution.rs new file mode 100644 index 00000000..108f239c --- /dev/null +++ b/crates/firma-config/tests/integration/resolution.rs @@ -0,0 +1,28 @@ +//! End-to-end: a sectioned firma.toml resolves and the sidecar section +//! parses through `load_section`. + +use firma_config::{ConfigSource, SystemDirs}; +use fs_err as fs; + +#[test] +fn explicit_flag_sectioned_file_round_trips() { + let tmp = tempfile::tempdir().expect("tmpdir"); + let path = tmp.path().join("firma.toml"); + fs::write( + &path, + "[sidecar.policy]\ndir = \"x\"\n[authority]\nbar = 1\n", + ) + .expect("write config"); + let resolved = SystemDirs::default() + .resolve_config(Some(&path)) + .expect("resolve config") + .expect("config should be found"); + assert_eq!(resolved.source, ConfigSource::Flag); + let body = resolved + .config + .section("sidecar") + .expect("load sidecar section"); + let table: toml::Table = body.parse().expect("parse sidecar section"); + assert!(table.get("policy").is_some()); + assert!(table.get("bar").is_none()); +} diff --git a/crates/firma-config/tests/integration/resolver.rs b/crates/firma-config/tests/integration/resolver.rs new file mode 100644 index 00000000..97c15ec9 --- /dev/null +++ b/crates/firma-config/tests/integration/resolver.rs @@ -0,0 +1,296 @@ +//! Config discovery behavior through the public resolver API. + +use std::path::{Path, PathBuf}; + +use firma_config::{ConfigResolveError, ConfigSource, SystemDirs}; +use fs_err as fs; + +use crate::helper; + +fn unwrap_resolved( + result: Result, ConfigResolveError>, +) -> firma_config::ResolvedConfig { + result + .expect("resolve config") + .expect("config should be found") +} + +fn touch_project_local(dir: &Path) -> PathBuf { + let d = dir.join(".firma"); + fs::create_dir_all(&d).expect("create .firma dir"); + let f = d.join(firma_config::CONFIG_FILE_NAME); + fs::write(&f, "").expect("write config file"); + f +} + +fn enter_nested_project(root: &Path) -> PathBuf { + let ceiling = root.join("project"); + let nested = ceiling.join("sub").join("deep"); + fs::create_dir_all(&nested).expect("create nested project cwd"); + std::env::set_current_dir(&nested).expect("set cwd to nested project directory"); + ceiling +} + +fn firma_config_env(root: &Path, file_name: &str) -> Vec<(std::ffi::OsString, std::ffi::OsString)> { + vec![( + std::ffi::OsString::from("FIRMA_CONFIG"), + root.join(file_name).into_os_string(), + )] +} + +#[test] +fn flag_wins_over_everything() { + helper::run_isolated(|root| { + let flag = root.join("explicit.toml"); + fs::write(&flag, "").expect("write explicit config"); + + let resolved = unwrap_resolved(SystemDirs::default().resolve_config(Some(&flag))); + + assert_eq!(resolved.source, ConfigSource::Flag); + assert_eq!(resolved.config_file(), flag.as_path()); + assert_eq!(resolved.config_dir(), root); + }); +} + +#[test] +fn explicit_missing_config_is_an_error() { + helper::run_isolated(|root| { + let missing = root.join("missing.toml"); + let error = SystemDirs::default() + .resolve_config(Some(&missing)) + .expect_err("resolution should fail"); + assert!(matches!( + error, + ConfigResolveError { + config_source: ConfigSource::Flag, + .. + } + )); + }); +} + +#[test] +fn explicit_invalid_toml_is_an_error() { + helper::run_isolated(|root| { + let invalid = root.join("invalid.toml"); + fs::write(&invalid, "=").expect("write invalid config"); + + let error = SystemDirs::default() + .resolve_config(Some(&invalid)) + .expect_err("resolution should fail"); + assert!( + matches!(error, ConfigResolveError { config_source: ConfigSource::Flag, path, .. } if path == invalid) + ); + }); +} + +#[test] +fn env_var_beats_project_local() { + helper::run_isolated_with_env( + |root| firma_config_env(root, "env.toml"), + |root| { + let env_file = root.join("env.toml"); + fs::write(&env_file, "").expect("write env config"); + let project = enter_nested_project(root); + touch_project_local(&project); + + let resolved = unwrap_resolved(SystemDirs::default().resolve_config(None)); + + assert_eq!(resolved.source, ConfigSource::EnvVar); + assert_eq!(resolved.config_file(), env_file.as_path()); + }, + ); +} + +#[test] +fn env_missing_config_is_an_error() { + helper::run_isolated_with_env( + |root| firma_config_env(root, "missing.toml"), + |_| { + let error = SystemDirs::default() + .resolve_config(None) + .expect_err("resolution should fail"); + assert!(matches!( + error, + ConfigResolveError { + config_source: ConfigSource::EnvVar, + .. + } + )); + }, + ); +} + +#[test] +fn env_invalid_toml_is_an_error() { + helper::run_isolated_with_env( + |root| firma_config_env(root, "invalid.toml"), + |root| { + let invalid = root.join("invalid.toml"); + fs::write(&invalid, "=").expect("write invalid config"); + + let error = SystemDirs::default() + .resolve_config(None) + .expect_err("resolution should fail"); + assert!( + matches!(error, ConfigResolveError { config_source: ConfigSource::EnvVar, path, .. } if path == invalid) + ); + }, + ); +} + +#[test] +fn project_local_found_in_cwd() { + helper::run_isolated(|root| { + let project_file = touch_project_local(root); + let provider = SystemDirs::default().walk_up_to(root); + + let resolved = unwrap_resolved(provider.resolve_config(None)); + + assert_eq!(resolved.source, ConfigSource::ProjectLocal); + assert_eq!(resolved.config_file(), project_file.as_path()); + }); +} + +#[test] +fn project_local_walks_up_to_ancestor() { + helper::run_isolated(|root| { + let project = enter_nested_project(root); + let project_file = touch_project_local(&project); + + let resolved = unwrap_resolved(SystemDirs::default().resolve_config(None)); + + assert_eq!(resolved.source, ConfigSource::ProjectLocal); + assert_eq!(resolved.config_file(), project_file.as_path()); + }); +} + +#[test] +fn project_local_closest_ancestor_wins() { + helper::run_isolated(|root| { + let project = enter_nested_project(root); + touch_project_local(&project); + let nested_file = touch_project_local(project.join("sub").as_path()); + + let resolved = unwrap_resolved(SystemDirs::default().resolve_config(None)); + + assert_eq!(resolved.source, ConfigSource::ProjectLocal); + assert_eq!(resolved.config_file(), nested_file.as_path()); + }); +} + +#[test] +fn project_local_invalid_toml_fails_closed_without_continuing_to_parent() { + helper::run_isolated(|root| { + let project = enter_nested_project(root); + let nested = project.join("sub").join("deep"); + fs::create_dir_all(nested.join(".firma")).expect("create nested .firma dir"); + touch_project_local(&project); + let invalid = nested.join(".firma").join(firma_config::CONFIG_FILE_NAME); + fs::write(&invalid, "=").expect("write invalid config"); + + let error = SystemDirs::default() + .resolve_config(None) + .expect_err("resolution should fail"); + assert!( + matches!(error, ConfigResolveError { config_source: ConfigSource::ProjectLocal, path, .. } if path == invalid) + ); + }); +} + +#[cfg(unix)] +#[test] +fn project_local_unreadable_file_fails_closed_without_continuing_to_parent() { + use std::os::unix::fs::PermissionsExt as _; + + fn set_mode(path: &Path, mode: u32) { + let mut permissions = fs::metadata(path) + .expect("read config metadata for permission update") + .permissions(); + permissions.set_mode(mode); + fs::set_permissions(path, permissions).expect("update config permissions"); + } + + helper::run_isolated(|root| { + let project = enter_nested_project(root); + let nested = project.join("sub").join("deep"); + fs::create_dir_all(nested.join(".firma")).expect("create nested .firma dir"); + touch_project_local(&project); + let unreadable = nested.join(".firma").join(firma_config::CONFIG_FILE_NAME); + fs::write(&unreadable, "").expect("write unreadable config"); + set_mode(&unreadable, 0o000); + if fs::read_to_string(&unreadable).is_ok() { + set_mode(&unreadable, 0o600); + return; + } + + let error = SystemDirs::default() + .resolve_config(None) + .expect_err("resolution should fail"); + assert!( + matches!(error, ConfigResolveError { config_source: ConfigSource::ProjectLocal, path, .. } if path == unreadable) + ); + + set_mode(&unreadable, 0o600); + }); +} + +#[test] +fn system_dirs_walks_up_to_project_local_with_ceiling() { + helper::run_isolated(|root| { + let ceiling = enter_nested_project(root); + let project_file = touch_project_local(&ceiling); + let provider = SystemDirs::default().walk_up_to(&ceiling); + + let resolved = unwrap_resolved(provider.resolve_config(None)); + + assert_eq!(resolved.source, ConfigSource::ProjectLocal); + assert_eq!(resolved.config_file(), project_file.as_path()); + }); +} + +#[test] +fn system_dirs_stops_walk_at_ceiling() { + helper::run_isolated(|root| { + let ceiling = enter_nested_project(root); + touch_project_local(root); + let provider = SystemDirs::default().walk_up_to(&ceiling); + + assert!( + provider + .resolve_config(None) + .expect("resolve config") + .is_none() + ); + }); +} + +#[test] +fn cwd_outside_ceiling_stops_before_searching() { + helper::run_isolated(|root| { + let ceiling = root.join("project"); + touch_project_local(root); + let provider = SystemDirs::default().walk_up_to(&ceiling); + + assert!( + provider + .resolve_config(None) + .expect("resolve config") + .is_none() + ); + }); +} + +#[test] +fn nothing_found_returns_none() { + helper::run_isolated(|root| { + let provider = SystemDirs::default().walk_up_to(root); + + assert!( + provider + .resolve_config(None) + .expect("resolve config") + .is_none() + ); + }); +} diff --git a/crates/firma-config/tests/integration/schema.rs b/crates/firma-config/tests/integration/schema.rs new file mode 100644 index 00000000..bbcf3c93 --- /dev/null +++ b/crates/firma-config/tests/integration/schema.rs @@ -0,0 +1,115 @@ +//! Section extraction behavior through the public schema API. + +use firma_config::{FirmaConfig, load_section}; +use fs_err as fs; + +#[test] +fn extracts_named_section() { + let tmp = tempfile::tempdir().expect("tmpdir"); + let path = tmp.path().join("firma.toml"); + fs::write(&path, "[sidecar]\nfoo = 1\n[authority]\nbar = 2\n").expect("write config"); + let out = load_section(&path, "sidecar").expect("load section"); + let table: toml::Table = out.parse().expect("parse section"); + assert_eq!(table.get("foo").and_then(toml::Value::as_integer), Some(1)); + assert!(table.get("bar").is_none()); +} + +#[test] +fn nested_subtables_are_preserved() { + let tmp = tempfile::tempdir().expect("tmpdir"); + let path = tmp.path().join("firma.toml"); + fs::write( + &path, + "[sidecar.interceptor]\nmode = \"http_proxy\"\n[sidecar.authority]\nurl = \"http://x\"\n", + ) + .expect("write config"); + let out = load_section(&path, "sidecar").expect("load section"); + let table: toml::Table = out.parse().expect("parse section"); + assert!( + table + .get("interceptor") + .and_then(toml::Value::as_table) + .is_some() + ); + assert!( + table + .get("authority") + .and_then(toml::Value::as_table) + .is_some() + ); +} + +#[test] +fn missing_section_is_an_error() { + let tmp = tempfile::tempdir().expect("tmpdir"); + let path = tmp.path().join("firma.toml"); + fs::write(&path, "[authority]\nbar = 2\n").expect("write config"); + let error = load_section(&path, "sidecar").expect_err("section should be missing"); + assert!( + error.to_string().contains("sidecar"), + "error names the section: {error}" + ); +} + +#[test] +fn dotted_path_extracts_nested_section() { + let tmp = tempfile::tempdir().expect("tmpdir"); + let path = tmp.path().join("firma.toml"); + fs::write( + &path, + "[sidecar.policy]\ndir = \".\"\n[sidecar.authority]\nurl = \"https://x\"\n", + ) + .expect("write config"); + let policy = load_section(&path, "sidecar.policy").expect("load policy section"); + let policy_table: toml::Table = policy.parse().expect("parse policy section"); + assert_eq!( + policy_table.get("dir").and_then(toml::Value::as_str), + Some(".") + ); + let connect = load_section(&path, "sidecar.authority").expect("load authority section"); + let authority_table: toml::Table = connect.parse().expect("parse authority section"); + assert_eq!( + authority_table.get("url").and_then(toml::Value::as_str), + Some("https://x") + ); +} + +#[test] +fn dotted_path_missing_is_an_error() { + let tmp = tempfile::tempdir().expect("tmpdir"); + let path = tmp.path().join("firma.toml"); + fs::write(&path, "[sidecar.policy]\ndir = \".\"\n").expect("write config"); + let error = load_section(&path, "sidecar.authority").expect_err("section should be missing"); + assert!( + error.to_string().contains("sidecar.authority"), + "error: {error}" + ); +} + +#[test] +fn firma_config_reuses_a_single_parse_for_multiple_sections() { + let tmp = tempfile::tempdir().expect("tmpdir"); + let path = tmp.path().join("firma.toml"); + fs::write(&path, "[sidecar]\nfoo = 1\n[authority]\nbar = 2\n").expect("write config"); + + let config = FirmaConfig::load(&path).expect("load config"); + let sidecar: toml::Table = config + .section("sidecar") + .expect("load sidecar section") + .parse() + .expect("parse sidecar section"); + let authority: toml::Table = config + .section("authority") + .expect("load authority section") + .parse() + .expect("parse authority section"); + + assert_eq!( + sidecar.get("foo").and_then(toml::Value::as_integer), + Some(1) + ); + assert_eq!( + authority.get("bar").and_then(toml::Value::as_integer), + Some(2) + ); +} diff --git a/crates/firma-config/tests/resolution.rs b/crates/firma-config/tests/resolution.rs deleted file mode 100644 index fcfec35d..00000000 --- a/crates/firma-config/tests/resolution.rs +++ /dev/null @@ -1,18 +0,0 @@ -//! End-to-end: a sectioned firma.toml resolves and the sidecar section -//! parses through `load_section`. - -use firma_config::{ConfigSource, SystemDirs, load_section, resolve_config}; -use tempfile::tempdir; - -#[test] -fn explicit_flag_sectioned_file_round_trips() { - let tmp = tempdir().unwrap(); - let p = tmp.path().join("firma.toml"); - std::fs::write(&p, "[sidecar.policy]\ndir = \"x\"\n[authority]\nbar = 1\n").unwrap(); - let r = resolve_config("sidecar", Some(&p), &SystemDirs).unwrap(); - assert_eq!(r.source, ConfigSource::Flag); - let body = load_section(&r.config_file, "sidecar").unwrap(); - let t: toml::Table = body.parse().unwrap(); - assert!(t.get("policy").is_some()); - assert!(t.get("bar").is_none()); -} diff --git a/crates/firma-run/src/config.rs b/crates/firma-run/src/config.rs index f8c2991f..8bb18e91 100644 --- a/crates/firma-run/src/config.rs +++ b/crates/firma-run/src/config.rs @@ -963,6 +963,7 @@ fn read_config(path: &Path, profile: &str) -> Result { // load_section prefixes the path; strip it to avoid doubling in the // RunError::ConfigParse display ("{path}: {reason}"). let prefix = format!("{}: ", path.display()); + let reason = reason.to_string(); let reason = reason.strip_prefix(&prefix).unwrap_or(&reason).to_string(); let hint = if reason.contains("[run]") { "; run `firma config` to add a [run] section" @@ -993,6 +994,7 @@ fn read_config(path: &Path, profile: &str) -> Result { pub fn read_configured_profile(path: &Path) -> Result, RunError> { let section = firma_config::load_section(path, "run").map_err(|reason| { let prefix = format!("{}: ", path.display()); + let reason = reason.to_string(); let reason = reason.strip_prefix(&prefix).unwrap_or(&reason).to_string(); RunError::ConfigParse { path: path.to_path_buf(), diff --git a/crates/firma-run/src/runtime.rs b/crates/firma-run/src/runtime.rs index 8a5fd620..f8bd235e 100644 --- a/crates/firma-run/src/runtime.rs +++ b/crates/firma-run/src/runtime.rs @@ -147,12 +147,15 @@ pub fn execute_run(args: &RunInput) -> Result { // Resolve firma.toml: explicit CLI path > env var > walk up from // cwd for `.firma/firma.toml`. `None` means no config — zero-config // defaults kick in downstream. - let resolved_user_config = firma_config::resolve_config( - "run", - args.user_config_path.as_deref(), - &firma_config::SystemDirs, - ) - .ok(); + let resolved_user_config = firma_config::SystemDirs::default() + .resolve_config(args.user_config_path.as_deref()) + .map_err(|error| RunError::ConfigParse { + path: args + .user_config_path + .clone() + .unwrap_or_else(|| PathBuf::from("firma.toml")), + reason: error.to_string(), + })?; let user_config_path: Option = resolved_user_config.as_ref().map_or_else( || { tracing::info!("no firma.toml found; using zero-config defaults"); @@ -160,16 +163,16 @@ pub fn execute_run(args: &RunInput) -> Result { }, |resolved| { tracing::info!( - path = %resolved.config_file.display(), + path = %resolved.config_file().display(), source = ?resolved.source, "loaded firma.toml" ); - Some(resolved.config_file.clone()) + Some(resolved.config_file().to_path_buf()) }, ); let user_config_dir = resolved_user_config .as_ref() - .map(|resolved| resolved.config_dir.as_path()); + .map(firma_config::ResolvedConfig::config_dir); let sidecar_template_path = resolve_sidecar_template_path(args, user_config_path.as_deref()); let flags = AutostartFlags { @@ -199,7 +202,7 @@ pub fn execute_run(args: &RunInput) -> Result { &args.authority_cli, &args.authority_profile, user_config_path.as_deref(), - user_config_dir, + user_config_dir.as_deref(), &firma_exe, &mut prompt, )?; diff --git a/crates/firma-stack/src/config.rs b/crates/firma-stack/src/config.rs index 9a172437..63fa52a1 100644 --- a/crates/firma-stack/src/config.rs +++ b/crates/firma-stack/src/config.rs @@ -35,15 +35,20 @@ pub struct StackConfig { /// Returns [`StackError::ConfigRead`] when no `firma.toml` can be /// resolved. pub fn resolve_stack_config(cli_override: Option<&Path>) -> Result { - let resolved = firma_config::resolve_config("stack", cli_override, &firma_config::SystemDirs) + let resolved = firma_config::SystemDirs::default() + .resolve_config(cli_override) .map_err(|error| StackError::ConfigRead { - path: cli_override.map_or_else(|| PathBuf::from("firma.toml"), Path::to_path_buf), - source: std::io::Error::new(std::io::ErrorKind::NotFound, error.to_string()), - })?; - debug!(config = %resolved.config_file.display(), "resolved unified firma.toml"); + path: cli_override.map_or_else(|| PathBuf::from("firma.toml"), Path::to_path_buf), + source: std::io::Error::new(std::io::ErrorKind::NotFound, error.to_string()), + })? + .ok_or_else(|| StackError::ConfigRead { + path: cli_override.map_or_else(|| PathBuf::from("firma.toml"), Path::to_path_buf), + source: std::io::Error::new(std::io::ErrorKind::NotFound, "no firma.toml found"), + })?; + debug!(config = %resolved.config_file().display(), "resolved unified firma.toml"); Ok(StackConfig { state_dir: None, - config_file: resolved.config_file, + config_file: resolved.config_file().to_path_buf(), firma_bin: None, }) } @@ -66,7 +71,6 @@ mod tests { #[test] fn unresolvable_is_error() { let missing = Path::new("/definitely/not/here/firma.toml"); - let cfg = resolve_stack_config(Some(missing)).unwrap(); - assert_eq!(cfg.config_file, missing); + assert!(resolve_stack_config(Some(missing)).is_err()); } } diff --git a/crates/firma-stack/tests/config_parse.rs b/crates/firma-stack/tests/config_parse.rs index 51c8d148..0d9b186f 100644 --- a/crates/firma-stack/tests/config_parse.rs +++ b/crates/firma-stack/tests/config_parse.rs @@ -19,10 +19,7 @@ fn resolves_explicit_override() { } #[test] -fn explicit_override_round_trips_even_if_missing() { - // resolve_config trusts an explicit --config flag (Flag source); the - // not-found branch only triggers during discovery. +fn explicit_override_errors_if_missing() { let cfg_path = std::path::Path::new("/definitely/not/here/firma.toml"); - let cfg = resolve_stack_config(Some(cfg_path)).expect("resolve"); - assert_eq!(cfg.config_file, cfg_path); + assert!(resolve_stack_config(Some(cfg_path)).is_err()); } diff --git a/crates/firma/README.md b/crates/firma/README.md index fb58a011..37859343 100644 --- a/crates/firma/README.md +++ b/crates/firma/README.md @@ -55,8 +55,8 @@ firma sidecar --config /etc/firma/firma.toml | `--health-bind-addr` | `FIRMA_SIDECAR_HEALTH_BIND_ADDR` | `127.0.0.1:9000` | When `--config` is omitted, a shared `firma.toml` is discovered from -platform-standard directories — see the Config Discovery section in -`docs/cli.md`. +`$FIRMA_CONFIG` or by walking up to `.firma/firma.toml` — see the Config +Discovery section in `docs/cli.md`. Config schema: see `crates/firma-sidecar/src/config.rs`. diff --git a/crates/firma/src/doctor/config_parse.rs b/crates/firma/src/doctor/config_parse.rs index 3c263b41..6bed8f7f 100644 --- a/crates/firma/src/doctor/config_parse.rs +++ b/crates/firma/src/doctor/config_parse.rs @@ -11,16 +11,25 @@ use crate::doctor::report::Check; /// Validate the resolved `firma.toml`: both `[authority]` and `[sidecar]` /// sections must be present and parse into their component config types. #[must_use] +#[cfg(test)] pub fn check(firma_toml: &Path) -> Check { let display = firma_toml.display().to_string(); let parsed = match firma_config::FirmaConfig::load(firma_toml) { Ok(parsed) => parsed, Err(error) => { - return Check::fail("config parsed", error).with_detail("path", display); + return Check::fail("config parsed", error.to_string()).with_detail("path", display); } }; + check_loaded(firma_toml, &parsed) +} + +/// Validate an already-loaded `firma.toml`. +#[must_use] +pub fn check_loaded(firma_toml: &Path, parsed: &firma_config::FirmaConfig) -> Check { + let display = firma_toml.display().to_string(); + // [authority] is optional — agent-remote configs have no server section. if let Ok(body) = parsed.section("authority") && let Err(error) = toml::from_str::(&body) diff --git a/crates/firma/src/services/authority.rs b/crates/firma/src/services/authority.rs index 7e06fa93..ac91403a 100644 --- a/crates/firma/src/services/authority.rs +++ b/crates/firma/src/services/authority.rs @@ -43,22 +43,22 @@ pub async fn run(args: Args) -> Result { return Ok(ExitCode::SUCCESS); } - let resolved = firma_config::resolve_config( - "authority", - args.config.as_deref(), - &firma_config::SystemDirs, - )?; + let resolved = firma_config::SystemDirs::default() + .resolve_config(args.config.as_deref())? + .ok_or_else(|| anyhow::anyhow!("no firma.toml found for `authority`"))?; tracing::info!( - path = %resolved.config_file.display(), + path = %resolved.config_file().display(), source = ?resolved.source, "config resolved" ); - let body = firma_config::load_section(&resolved.config_file, "authority") + let body = resolved + .config + .section("authority") .map_err(|e| anyhow::anyhow!("failed to load authority configuration: {e}"))?; let tmp_dir = tempfile::tempdir().context("tempdir for authority config")?; let tmp_cfg = tmp_dir.path().join("authority.toml"); std::fs::write(&tmp_cfg, body).context("stage authority config")?; - let config = AuthorityConfig::load_resolved(&tmp_cfg, &resolved.config_dir) + let config = AuthorityConfig::load_resolved(&tmp_cfg, &resolved.config_dir()) .context("failed to load authority configuration")?; match args.command { diff --git a/crates/firma/src/services/config.rs b/crates/firma/src/services/config.rs index bc39f38f..67b9c428 100644 --- a/crates/firma/src/services/config.rs +++ b/crates/firma/src/services/config.rs @@ -445,8 +445,9 @@ fn resolve_config_dir( return std::path::absolute(p).with_context(|| format!("resolve path {}", p.display())); } - let default = firma_config::resolve_config("config", None, &firma_config::SystemDirs) - .map_or_else(|_| default_output_dir(), |resolved| resolved.config_dir); + let default = firma_config::SystemDirs::default() + .resolve_config(None)? + .map_or_else(default_output_dir, |resolved| resolved.config_dir()); if interactive { let s: String = dialoguer::Input::with_theme(theme) @@ -1094,24 +1095,23 @@ pub fn resolve_audit_log_path(state_dir_flag: Option<&PathBuf>) -> Result RenderedReport { // uses, then validates both sections. Runs early so the // reachability probes can reuse the resolved path. let resolved_config = - firma_config::resolve_config("doctor", args.config.as_deref(), &firma_config::SystemDirs); - let config_path = match &resolved_config { - Ok(resolved) => { - let p = resolved.config_file.clone(); - report.push(config_parse::check(&p)); - Some(p) + firma_config::SystemDirs::default().resolve_config(args.config.as_deref()); + let parsed_config = match &resolved_config { + Ok(Some(resolved)) => { + report.push(config_parse::check_loaded( + resolved.config_file(), + &resolved.config, + )); + Some(resolved.config.clone()) + } + Ok(None) => { + report.push(Check::fail( + "config parsed", + "could not resolve firma.toml: no config found", + )); + None } Err(error) => { report.push(Check::fail( @@ -119,18 +128,6 @@ async fn build_report(args: Args) -> RenderedReport { } }; - // Parse the resolved file once; reachability probes 4 and 5 reuse it. - let parsed_config = - config_path - .as_deref() - .and_then(|p| match firma_config::FirmaConfig::load(p) { - Ok(parsed) => Some(parsed), - Err(error) => { - warn!(?error, "could not parse firma.toml"); - None - } - }); - // State dir doubles as the runtime dir on every supported platform, so the // per-run sidecar markers live under it. Resolve it now: checks 4 and 5 // cross-check live per-run instances against the configured daemon probe. @@ -141,13 +138,17 @@ async fn build_report(args: Args) -> RenderedReport { // 4. sidecar mode + reachability let parsed_sidecar: Option = parsed_config.as_ref().and_then(|c| { - match c.section("sidecar").and_then(|body| { - toml::from_str::(&body) - .map_err(|e| e.to_string()) - }) { - Ok(sc) => Some(sc), + let body = match c.section("sidecar") { + Ok(body) => body, Err(error) => { warn!(?error, "could not load sidecar config"); + return None; + } + }; + match toml::from_str::(&body) { + Ok(sc) => Some(sc), + Err(error) => { + warn!(?error, "could not parse sidecar config"); None } } diff --git a/crates/firma/src/services/run.rs b/crates/firma/src/services/run.rs index a7507222..6ddd3fb8 100644 --- a/crates/firma/src/services/run.rs +++ b/crates/firma/src/services/run.rs @@ -41,12 +41,12 @@ pub fn run(args: RunArgs) -> anyhow::Result { maybe_implicit_init(&args)?; // Auto-discover firma.toml when --config is not set. - let run_config = args.config.clone().or_else(|| { - firma_config::resolve_config("run", None, &firma_config::SystemDirs) - .ok() - .map(|r| r.config_dir.join(firma_config::CONFIG_FILE_NAME)) - .filter(|p| p.is_file()) - }); + let run_config = match &args.config { + Some(config) => Some(config.clone()), + None => firma_config::SystemDirs::default() + .resolve_config(None)? + .map(|resolved| resolved.config_file().to_path_buf()), + }; let profile = resolve_profile_name( args.profile.as_deref(), @@ -97,7 +97,10 @@ fn maybe_implicit_init(args: &RunArgs) -> anyhow::Result<()> { // Spec §4 step 1 + §5: walk-up `./.firma/firma.toml` is the project-local // tier, picked up by `firma_config::resolve_config`. If anything in the // search path resolves, skip implicit init. - if firma_config::resolve_config("run", None, &firma_config::SystemDirs).is_ok() { + if firma_config::SystemDirs::default() + .resolve_config(None)? + .is_some() + { return Ok(()); } diff --git a/crates/firma/src/services/sidecar.rs b/crates/firma/src/services/sidecar.rs index d6f5207b..fc2abf00 100644 --- a/crates/firma/src/services/sidecar.rs +++ b/crates/firma/src/services/sidecar.rs @@ -85,10 +85,11 @@ fn fail(msg: &str) -> ExitCode { async fn serve(args: crate::args::sidecar::ServeArgs) -> anyhow::Result { debug!("firma sidecar starting"); - let resolved = - firma_config::resolve_config("sidecar", args.config.as_deref(), &firma_config::SystemDirs)?; + let resolved = firma_config::SystemDirs::default() + .resolve_config(args.config.as_deref())? + .ok_or_else(|| anyhow::anyhow!("no firma.toml found for `sidecar`"))?; info!( - path = %resolved.config_file.display(), + path = %resolved.config_file().display(), source = ?resolved.source, "config resolved" ); @@ -135,7 +136,7 @@ async fn serve(args: crate::args::sidecar::ServeArgs) -> anyhow::Result anyhow::Result { - let body = firma_config::load_section(&resolved.config_file, "sidecar") + let body = resolved + .config + .section("sidecar") .map_err(|e| anyhow::anyhow!("invalid configuration: {e}"))?; let mut config: config::SidecarConfig = toml::from_str(&body)?; - config.rebase_defaults(&resolved.config_dir); + config.rebase_defaults(&resolved.config_dir()); config .validate() .map_err(|e| anyhow::anyhow!("invalid configuration: {e}"))?; diff --git a/docs-site/public/llms.txt b/docs-site/public/llms.txt index 1057a4f7..f6fa172c 100644 --- a/docs-site/public/llms.txt +++ b/docs-site/public/llms.txt @@ -21,5 +21,6 @@ OpenFirma docs highlights for LLM-based retrieval: - Capability token non-exposure applies when capabilities are pre-seeded into the Sidecar; current `firma run --capability-file` exports capability material into the wrapped process environment for compatibility. - Runtime logs for non-structural backends use "backend compatibility proof" with `mode=proxy_only enforced=false` instead of "backend network enforcement proof". - `firma config` reads existing target `firma.toml` values as defaults; CLI flags override only supplied fields. +- Config discovery order is explicit `--config`, then `$FIRMA_CONFIG`, then walk-up to `.firma/firma.toml`; a selected unreadable or invalid file fails closed instead of falling through. - Sidecar-internal ABORT is distinct from DENY: DENY rejects request shape/token/scope/policy before execution; ABORT blocks an already-allowed in-flight call and leaves the capability token active. - V1 ABORT reason codes are `CONNECTOR_TIMEOUT`, `CONNECTOR_FAILURE`, `CONNECTOR_INVALID_REQUEST`, and `CREDENTIAL_INJECTION_FAILED`; HTTP-facing interceptors return 504 with `{"aborted": true, "reason": "...", "detail": "..."}`. diff --git a/docs-site/src/content/docs/guides/firma-doctor.md b/docs-site/src/content/docs/guides/firma-doctor.md index 6ef2ef60..7e57c19e 100644 --- a/docs-site/src/content/docs/guides/firma-doctor.md +++ b/docs-site/src/content/docs/guides/firma-doctor.md @@ -39,7 +39,7 @@ firma doctor --timeout-ms 1500 # slower network probe (ms) | Flag | Env | Default | Description | | -------------- | ----------------- | ---------- | -------------------------------------------------------------- | -| `--config` | `FIRMA_STACK_CONFIG` | discovered | Unified `firma.toml`. When unset, auto-discovery uses `$FIRMA_CONFIG` or walk-up to `/.firma/firma.toml`. | +| `--config` | `FIRMA_STACK_CONFIG` | discovered | Unified `firma.toml`. When unset, auto-discovery uses `$FIRMA_CONFIG` or walk-up to `/.firma/firma.toml`; selected files must load successfully. | | `--state-dir` | `FIRMA_STATE_DIR` | resolved | Override the runtime state directory. | | `--json` | — | _off_ | Emit a single JSON object instead of pretty text. | | `--timeout-ms` | — | `500` | Per-probe network timeout (TCP / UDS connect) in milliseconds. | diff --git a/docs/cli.md b/docs/cli.md index 90b1fca0..16480aa4 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -349,27 +349,22 @@ FIR-104. When `--config` is **omitted**, every subcommand — `sidecar` (`start`/`stop`/`status`), `authority`, `run`, `config`, `monitor`, and -`doctor` — discovers the same single shared `firma.toml` from -platform-standard directories. That one file holds top-level +`doctor` — discovers the same single shared `firma.toml`. That one file holds top-level `[project]` / `[sidecar]` / `[authority]` / `[run]` sections; each -subcommand reads only its own section. The first existing file wins: +subcommand reads only its own section. The first selected file wins: 1. `--config ` flag — always wins. It only relocates the file; the file still uses the sectioned schema. -2. **Project-local `.firma/firma.toml`**, found by walking up from +2. `$FIRMA_CONFIG` env var if set — overrides walk, points directly to file. +3. **Project-local `.firma/firma.toml`**, found by walking up from `cwd`. The closest ancestor with a `.firma/firma.toml` wins; the walk stops at the filesystem root. This is what `firma config` writes. -3. `$FIRMA_CONFIG` env var if set — overrides walk, points directly to file. -4. None found and config is required → exit non-zero with a message listing - every directory searched. - -On macOS the user tier is a dual path: `$XDG_CONFIG_HOME/firma` is tried -first, then `~/.config/firma` (CLI/dev-tool convention, preferred), then -`~/Library/Application Support/firma`. On Linux `~/.config/firma` and the -platform default coincide and are de-duplicated. On Windows the -home-convention tier is the bare `%USERPROFILE%\.firma` dotdir (matching -agent/dev CLIs such as `.claude` and `.codex`), distinct from the -`%APPDATA%\Roaming\firma` platform tier tried after it. +4. None found and config is required → exit non-zero. Optional flows such as + zero-config `firma run` may continue without a config. + +A selected file must be readable and valid TOML. An explicit `--config`, +`$FIRMA_CONFIG`, or discovered project-local file that cannot be loaded fails +closed instead of falling through to another tier or to zero-config defaults. The resolved path and its source are emitted on startup as a single `config resolved` INFO line (with `path` and `source` fields) so operators diff --git a/docs/configuration.md b/docs/configuration.md index 72e31069..28e15ed3 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -8,10 +8,10 @@ and `[run]`. Each subcommand reads only its own section; section extraction is fail-closed (a missing required section is a hard error). When `--config` is **omitted**, that `firma.toml` is discovered from -platform-standard directories (see +`$FIRMA_CONFIG` or by walking up from `cwd` to `.firma/firma.toml` (see [Config Discovery](cli.md#config-discovery)). An explicit `--config ` only overrides the file location — the file still uses the same sectioned -shape. +shape and must be readable and valid TOML. Configuration is validated at startup. Invalid fields cause the affected binary to exit before accepting requests.