diff --git a/AGENTS.md b/AGENTS.md index de610ea..2cbedd6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,6 +4,8 @@ `config-example.toml` in the repo root is the canonical config file. It is symlinked into `$XDG_CONFIG_HOME/agent-sandbox/config.toml` on the host. When editing it, remember that changes take effect immediately for all sandbox invocations. +The config file may set `extra-config = ""` to layer a second file on top using the same merge rules (scalars replace, vecs append, profiles merge by name). A missing extra file is silently skipped; nesting is not supported (the extra file cannot itself set `extra-config`). + ## Build and test - `cargo fmt` and `cargo clippy` must pass before every commit. diff --git a/config-example.toml b/config-example.toml index 559bc4f..e2910b2 100644 --- a/config-example.toml +++ b/config-example.toml @@ -1,6 +1,12 @@ # Layered settings: CLI > active profile > globals. `--profile` selects the # profile, otherwise `default-profile` below is used. Vec fields append across # layers; scalars replace. +# +# `extra-config` optionally points to a second file layered on top of this one +# using the same rules (scalars replace, vecs append, profiles merge by name). +# Missing extra-config files are silently skipped; nesting is not supported. + +extra-config = "~/.config/agent-sandbox/extra.toml" whitelist = true # blacklist = true diff --git a/src/config.rs b/src/config.rs index 2f11141..158dfa8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -292,6 +292,8 @@ pub struct FileConfig { pub profile: HashMap, #[serde(rename = "default-profile", default)] pub default_profile: Option, + #[serde(rename = "extra-config", default)] + pub extra_config: Option, // Collects unrecognized keys; deny_unknown_fields is incompatible with flatten. #[serde(flatten)] _unknown: HashMap, @@ -299,6 +301,30 @@ pub struct FileConfig { impl FileConfig { pub fn load(path: &Path) -> Result { + Self::load_file(path)?.load_extra() + } + + fn load_extra(self) -> Result { + let Some(ref extra_path) = self.extra_config else { + return Ok(self); + }; + let resolved = expand_and_require_absolute(extra_path)?; + let extra = match Self::load_file(&resolved) { + Ok(c) => c, + Err(SandboxError::ConfigRead { source, .. }) + if source.kind() == std::io::ErrorKind::NotFound => + { + return Ok(self); + } + Err(e) => return Err(e), + }; + if extra.extra_config.is_some() { + return Err(SandboxError::NestedExtraConfig(resolved)); + } + Ok(self.merge_with(extra)) + } + + fn load_file(path: &Path) -> Result { let contents = std::fs::read_to_string(path).map_err(|e| SandboxError::ConfigRead { path: path.to_path_buf(), source: e, @@ -312,6 +338,24 @@ impl FileConfig { }) } + fn merge_with(self, extra: FileConfig) -> FileConfig { + let mut profile = self.profile; + for (profile_name, profile_options) in extra.profile { + let merged = match profile.remove(&profile_name) { + Some(existing) => existing.merge_with(profile_options), + None => profile_options, + }; + profile.insert(profile_name, merged); + } + FileConfig { + options: self.options.merge_with(extra.options), + profile, + default_profile: extra.default_profile.or(self.default_profile), + extra_config: None, + _unknown: HashMap::new(), + } + } + fn parse(contents: &str) -> Result { let config: Self = toml::from_str(contents).map_err(|e| SandboxError::ConfigParse { path: PathBuf::new(), @@ -363,6 +407,28 @@ pub struct Options { } impl Options { + fn merge_with(self, extra: Options) -> Options { + let (blacklist, whitelist) = pick_mode_flags(&self, &extra); + Options { + blacklist, + whitelist, + hardened: extra.hardened.or(self.hardened), + unshare_net: extra.unshare_net.or(self.unshare_net), + seccomp: extra.seccomp.or(self.seccomp), + env_filter: extra.env_filter.or(self.env_filter), + entrypoint: extra.entrypoint.or(self.entrypoint), + command: extra.command.or(self.command), + dry_run: extra.dry_run.or(self.dry_run), + chdir: extra.chdir.or(self.chdir), + rw: append(self.rw, extra.rw), + ro: append(self.ro, extra.ro), + mask: append(self.mask, extra.mask), + env: append(self.env, extra.env), + unsetenv: append(self.unsetenv, extra.unsetenv), + bwrap_args: append(self.bwrap_args, extra.bwrap_args), + } + } + fn validate_paths(&mut self) -> Result<(), SandboxError> { if let Some(ref chdir) = self.chdir { self.chdir = Some(expand_and_canonicalize(chdir)?); @@ -374,6 +440,19 @@ impl Options { } } +fn append(mut base: Vec, extra: Vec) -> Vec { + base.extend(extra); + base +} + +fn pick_mode_flags(base: &Options, extra: &Options) -> (Option, Option) { + if extra.blacklist.is_some() || extra.whitelist.is_some() { + (extra.blacklist, extra.whitelist) + } else { + (base.blacklist, base.whitelist) + } +} + fn expand_and_require_existing(path: &Path) -> Result { let expanded = expand_and_require_absolute(path)?; if !expanded.exists() { @@ -1485,6 +1564,210 @@ mod tests { assert!(matches!(result, Err(SandboxError::NoCommand))); } + #[test] + fn merge_options_scalar_extra_overrides_base() { + let base = Options { + hardened: Some(false), + ..Options::default() + }; + let extra = Options { + hardened: Some(true), + ..Options::default() + }; + assert_eq!(base.merge_with(extra).hardened, Some(true)); + } + + #[test] + fn merge_options_scalar_extra_unset_inherits_base() { + let base = Options { + hardened: Some(true), + ..Options::default() + }; + assert_eq!(base.merge_with(Options::default()).hardened, Some(true)); + } + + #[test] + fn merge_options_extra_mode_replaces_base_mode() { + let base = Options { + whitelist: Some(true), + ..Options::default() + }; + let extra = Options { + blacklist: Some(true), + ..Options::default() + }; + let merged = base.merge_with(extra); + assert_eq!(merged.blacklist, Some(true)); + assert_eq!(merged.whitelist, None); + } + + #[test] + fn merge_options_base_mode_inherited_when_extra_silent() { + let base = Options { + whitelist: Some(true), + ..Options::default() + }; + let merged = base.merge_with(Options::default()); + assert_eq!(merged.whitelist, Some(true)); + assert_eq!(merged.blacklist, None); + } + + #[test] + fn merge_options_lists_append_base_then_extra() { + let base = Options { + rw: vec!["/a".into()], + env: vec!["A=1".into()], + ..Options::default() + }; + let extra = Options { + rw: vec!["/b".into()], + env: vec!["B=2".into()], + ..Options::default() + }; + let merged = base.merge_with(extra); + assert_eq!(merged.rw, vec!["/a", "/b"]); + assert_eq!(merged.env, vec!["A=1", "B=2"]); + } + + #[test] + fn merge_file_config_profile_merged_by_name() { + let base = FileConfig { + profile: HashMap::from([( + "claude".into(), + Options { + rw: vec!["/a".into()], + hardened: Some(false), + ..Options::default() + }, + )]), + ..FileConfig::default() + }; + let extra = FileConfig { + profile: HashMap::from([ + ( + "claude".into(), + Options { + rw: vec!["/b".into()], + hardened: Some(true), + ..Options::default() + }, + ), + ( + "codex".into(), + Options { + unshare_net: Some(true), + ..Options::default() + }, + ), + ]), + ..FileConfig::default() + }; + let merged = base.merge_with(extra); + assert_eq!(merged.profile["claude"].hardened, Some(true)); + assert_eq!(merged.profile["claude"].rw, vec!["/a", "/b"]); + assert_eq!(merged.profile["codex"].unshare_net, Some(true)); + } + + #[test] + fn merge_file_config_default_profile_extra_overrides() { + let base = FileConfig { + default_profile: Some("claude".into()), + ..FileConfig::default() + }; + let extra = FileConfig { + default_profile: Some("codex".into()), + ..FileConfig::default() + }; + assert_eq!(base.merge_with(extra).default_profile, Some("codex".into())); + } + + #[test] + fn merge_file_config_default_profile_extra_unset_inherits() { + let base = FileConfig { + default_profile: Some("claude".into()), + ..FileConfig::default() + }; + assert_eq!( + base.merge_with(FileConfig::default()).default_profile, + Some("claude".into()) + ); + } + + #[test] + fn load_extra_applies_overlay() { + let dir = TempDir::new().unwrap(); + let base_path = dir.path().join("base.toml"); + let extra_path = dir.path().join("extra.toml"); + std::fs::write(&extra_path, "hardened = true\nrw = [\"/b\"]\n").unwrap(); + std::fs::write( + &base_path, + format!( + "hardened = false\nrw = [\"/a\"]\nextra-config = \"{}\"\n", + extra_path.display() + ), + ) + .unwrap(); + + let config = FileConfig::load(&base_path).unwrap(); + assert_eq!(config.options.hardened, Some(true)); + assert_eq!(config.options.rw, vec!["/a", "/b"]); + assert_eq!(config.extra_config, None); + } + + #[test] + fn load_extra_missing_file_is_skipped() { + let dir = TempDir::new().unwrap(); + let base_path = dir.path().join("base.toml"); + let extra_path = dir.path().join("does-not-exist.toml"); + std::fs::write( + &base_path, + format!( + "hardened = true\nextra-config = \"{}\"\n", + extra_path.display() + ), + ) + .unwrap(); + + let config = FileConfig::load(&base_path).unwrap(); + assert_eq!(config.options.hardened, Some(true)); + } + + #[test] + fn load_extra_nested_rejected() { + let dir = TempDir::new().unwrap(); + let base_path = dir.path().join("base.toml"); + let extra_path = dir.path().join("extra.toml"); + let deeper_path = dir.path().join("deeper.toml"); + std::fs::write(&deeper_path, "hardened = true\n").unwrap(); + std::fs::write( + &extra_path, + format!("extra-config = \"{}\"\n", deeper_path.display()), + ) + .unwrap(); + std::fs::write( + &base_path, + format!("extra-config = \"{}\"\n", extra_path.display()), + ) + .unwrap(); + + assert!(matches!( + FileConfig::load(&base_path), + Err(SandboxError::NestedExtraConfig(_)) + )); + } + + #[test] + fn load_extra_relative_path_rejected() { + let dir = TempDir::new().unwrap(); + let base_path = dir.path().join("base.toml"); + std::fs::write(&base_path, "extra-config = \"extra.toml\"\n").unwrap(); + + assert!(matches!( + FileConfig::load(&base_path), + Err(SandboxError::ConfigPathNotAbsolute(_)) + )); + } + fn assert_paths(actual: &[String], expected: &[&str]) { let expected: Vec = expected.iter().map(|s| s.to_string()).collect(); assert_eq!(actual, &expected); diff --git a/src/errors.rs b/src/errors.rs index 1ec908f..677fbf9 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -25,6 +25,7 @@ pub enum SandboxError { ConflictingEnvKey(String), InvalidEnvEntry(String), UnknownConfigKey(String), + NestedExtraConfig(PathBuf), ConfigPathNotAbsolute(PathBuf), InvalidBwrapArg(String), InvalidBindSpec(String), @@ -75,6 +76,11 @@ impl std::fmt::Display for SandboxError { write!(f, "invalid env entry (expected KEY or KEY=VALUE): {raw:?}") } Self::UnknownConfigKey(key) => write!(f, "unknown config key: {key}"), + Self::NestedExtraConfig(p) => write!( + f, + "extra-config file '{}' sets its own extra-config (nesting not supported)", + p.display() + ), Self::ConfigPathNotAbsolute(p) => { write!(f, "config path is not absolute: {}", p.display()) }