diff --git a/config-example.toml b/config-example.toml index e2910b2..05d743c 100644 --- a/config-example.toml +++ b/config-example.toml @@ -1,10 +1,14 @@ -# Layered settings: CLI > active profile > globals. `--profile` selects the -# profile, otherwise `default-profile` below is used. Vec fields append across -# layers; scalars replace. +# Merge rules (apply to every layering below): scalars replace, vec fields +# append, profiles merge by name. # -# `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. +# Layers, lowest precedence first: globals -> active profile (with its +# ancestors folded in) -> CLI flags. `--profile` selects the active profile; +# otherwise the top-level `profile` below is used. Profiles can themselves set +# `profile = "parent"` to inherit from another profile. +# +# `extra-config` optionally points to a second file layered on top of this +# one. Missing extra-config files are silently skipped; nesting is not +# supported. extra-config = "~/.config/agent-sandbox/extra.toml" @@ -41,13 +45,17 @@ env = [ # command = ["--model", "opus"] # default trailing args # bwrap-args = ["--tmpfs /opt/scratch"] # raw bwrap escape hatch -default-profile = "claude" +profile = "claude" -[profile.claude] +[profiles.claude] ro = ["~/.local/share/claude-code"] rw = ["~/.config/claude"] +entrypoint = ["claude", "--allowedTools", "Bash(*)", "WebSearch", "WebFetch(*)", "mcp__*"] + +[profiles.claude-yolo] +profile = "claude" entrypoint = ["claude", "--dangerously-skip-permissions"] -[profile.codex] +[profiles.codex] ro = ["~/.local/share/codex-cli"] entrypoint = ["codex", "--dangerously-bypass-approvals-and-sandbox"] diff --git a/src/config.rs b/src/config.rs index 97d632b..8ea7f3e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -289,9 +289,7 @@ pub struct FileConfig { #[serde(flatten)] pub options: Options, #[serde(default)] - pub profile: HashMap, - #[serde(rename = "default-profile", default)] - pub default_profile: Option, + pub profiles: HashMap, #[serde(rename = "extra-config", default)] pub extra_config: Option, // Collects unrecognized keys; deny_unknown_fields is incompatible with flatten. @@ -339,18 +337,17 @@ 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) { + let mut profiles = self.profiles; + for (profile_name, profile_options) in extra.profiles { + let merged = match profiles.remove(&profile_name) { Some(existing) => existing.merge_with(profile_options), None => profile_options, }; - profile.insert(profile_name, merged); + profiles.insert(profile_name, merged); } FileConfig { options: self.options.merge_with(extra.options), - profile, - default_profile: extra.default_profile.or(self.default_profile), + profiles, extra_config: None, _unknown: HashMap::new(), } @@ -367,21 +364,57 @@ impl FileConfig { Ok(config) } - fn resolve_profile(&self, name: Option<&str>) -> Result { - match name.or(self.default_profile.as_deref()) { - Some(n) => self - .profile - .get(n) - .cloned() - .ok_or_else(|| SandboxError::ProfileNotFound(n.to_string())), + fn resolve_profile(&self, selected: Option<&str>) -> Result { + match selected.or(self.options.profile.as_deref()) { + Some(leaf) => self.resolve_chain(leaf), None => Ok(Options::default()), } } + + fn resolve_chain(&self, leaf: &str) -> Result { + let chain = self.collect_chain(leaf)?; + + let merged = chain + .into_iter() + .rev() + .fold(Options::default(), |base, child| base.merge_with(child)); + + Ok(merged) + } + + fn collect_chain(&self, leaf: &str) -> Result, SandboxError> { + let mut visited: Vec = Vec::new(); + let mut chain: Vec = Vec::new(); + let mut current = leaf.to_string(); + + loop { + if visited.iter().any(|seen| seen == ¤t) { + visited.push(current); + return Err(SandboxError::ProfileCycle(visited)); + } + + let options = self + .profiles + .get(¤t) + .cloned() + .ok_or_else(|| SandboxError::ProfileNotFound(current.clone()))?; + + visited.push(current); + let parent = options.profile.clone(); + chain.push(options); + + match parent { + Some(p) => current = p, + None => return Ok(chain), + } + } + } } #[derive(Deserialize, Default, Clone)] #[serde(rename_all = "kebab-case", deny_unknown_fields)] pub struct Options { + pub profile: Option, pub blacklist: Option, pub whitelist: Option, pub hardened: Option, @@ -410,6 +443,7 @@ impl Options { fn merge_with(self, extra: Options) -> Options { let (blacklist, whitelist) = pick_mode_flags(&self, &extra); Options { + profile: extra.profile.or(self.profile), blacklist, whitelist, hardened: extra.hardened.or(self.hardened), diff --git a/src/errors.rs b/src/errors.rs index 677fbf9..daddc01 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -21,6 +21,7 @@ pub enum SandboxError { source: toml::de::Error, }, ProfileNotFound(String), + ProfileCycle(Vec), ConflictingMode, ConflictingEnvKey(String), InvalidEnvEntry(String), @@ -65,6 +66,13 @@ impl std::fmt::Display for SandboxError { write!(f, "cannot parse config file '{}': {source}", path.display()) } Self::ProfileNotFound(name) => write!(f, "profile not found in config: {name}"), + Self::ProfileCycle(chain) => { + write!( + f, + "profile inheritance cycle detected: {}", + chain.join(" -> ") + ) + } Self::ConflictingMode => write!( f, "config section sets both blacklist and whitelist to true" diff --git a/tests/e2e/cli.rs b/tests/e2e/cli.rs index 18d7384..c3a789d 100644 --- a/tests/e2e/cli.rs +++ b/tests/e2e/cli.rs @@ -118,7 +118,7 @@ fn bwrap_arg_setenv_passes_through() { fn config_entrypoint_appends_passthrough_args() { let cfg = ConfigFile::new( r#" - [profile.test] + [profiles.test] entrypoint = ["bash", "-c"] "#, ); @@ -139,7 +139,7 @@ fn config_entrypoint_appends_passthrough_args() { fn config_entrypoint_falls_back_to_command_defaults() { let cfg = ConfigFile::new( r#" - [profile.test] + [profiles.test] entrypoint = ["bash", "-c"] command = ["echo default-args"] "#, @@ -160,7 +160,7 @@ fn config_entrypoint_falls_back_to_command_defaults() { fn config_entrypoint_alone_without_command_or_passthrough() { let cfg = ConfigFile::new( r#" - [profile.test] + [profiles.test] entrypoint = ["bash", "-c", "echo entrypoint-only"] "#, ); @@ -214,7 +214,7 @@ fn cli_entrypoint_overrides_config_entrypoint() { fn config_command_alone_without_passthrough() { let cfg = ConfigFile::new( r#" - [profile.test] + [profiles.test] command = ["bash", "-c", "echo command-only"] "#, ); @@ -234,7 +234,7 @@ fn config_command_alone_without_passthrough() { fn config_command_replaced_by_passthrough() { let cfg = ConfigFile::new( r#" - [profile.test] + [profiles.test] command = ["bash", "-c", "echo should-not-see-this"] "#, ); diff --git a/tests/unit/config.rs b/tests/unit/config.rs index 689a1e4..f791587 100644 --- a/tests/unit/config.rs +++ b/tests/unit/config.rs @@ -11,13 +11,13 @@ const FULL_CONFIG_TOML: &str = r#" rw = ["/tmp/a", "/tmp/b"] command = "zsh" - [profile.claude] + [profiles.claude] rw = ["/home/user/.config/claude"] ro = ["/etc/claude", "/etc/shared"] entrypoint = ["claude", "--dangerously-skip-permissions"] command = ["bash", "-c", "echo hi"] - [profile.codex] + [profiles.codex] whitelist = true dry-run = true chdir = "/home/user/project" @@ -50,13 +50,13 @@ fn unset_vecs_default_empty() { #[test] fn unset_scalars_are_none() { - assert_eq!(CONFIG.profile["claude"].hardened, None); + assert_eq!(CONFIG.profiles["claude"].hardened, None); } #[test] fn profile_multi_element_vecs() { assert_paths( - &CONFIG.profile["claude"].ro, + &CONFIG.profiles["claude"].ro, &["/etc/claude", "/etc/shared"], ); } @@ -64,7 +64,7 @@ fn profile_multi_element_vecs() { #[test] fn profile_entrypoint_with_args() { assert_command( - &CONFIG.profile["claude"].entrypoint, + &CONFIG.profiles["claude"].entrypoint, &["claude", "--dangerously-skip-permissions"], ); } @@ -72,14 +72,14 @@ fn profile_entrypoint_with_args() { #[test] fn profile_command_with_args() { assert_command( - &CONFIG.profile["claude"].command, + &CONFIG.profiles["claude"].command, &["bash", "-c", "echo hi"], ); } #[test] fn profile_kebab_case_scalars() { - let codex = &CONFIG.profile["codex"]; + let codex = &CONFIG.profiles["codex"]; assert_eq!(codex.whitelist, Some(true)); assert_eq!(codex.dry_run, Some(true)); assert_eq!(codex.chdir, Some(PathBuf::from("/home/user/project"))); @@ -112,7 +112,7 @@ fn build_profile_overrides_globals() { hardened: Some(true), ..Options::default() }, - profile: HashMap::from([( + profiles: HashMap::from([( "relaxed".into(), Options { hardened: Some(false), @@ -132,7 +132,7 @@ fn build_profile_overrides_globals() { #[test] fn build_cli_flag_overrides_profile() { let file_config = FileConfig { - profile: HashMap::from([( + profiles: HashMap::from([( "nonet".into(), Options { unshare_net: Some(false), @@ -257,7 +257,7 @@ fn build_cli_no_dry_run_overrides_profile() { #[test] fn build_cli_mode_overrides_profile() { let file_config = FileConfig { - profile: HashMap::from([( + profiles: HashMap::from([( "wl".into(), Options { whitelist: Some(true), @@ -282,7 +282,7 @@ fn build_rw_paths_accumulate() { rw: vec!["/tmp".into()], ..Options::default() }, - profile: HashMap::from([( + profiles: HashMap::from([( "extra".into(), Options { rw: vec!["/usr".into()], @@ -303,7 +303,7 @@ fn build_rw_paths_accumulate() { #[test] fn build_command_from_profile() { let file_config = FileConfig { - profile: HashMap::from([( + profiles: HashMap::from([( "test".into(), Options { command: Some(CommandValue::WithArgs(vec![ @@ -327,7 +327,7 @@ fn build_command_from_profile() { #[test] fn build_cli_entrypoint_overrides_profile() { let file_config = FileConfig { - profile: HashMap::from([( + profiles: HashMap::from([( "test".into(), Options { entrypoint: Some(CommandValue::Simple("/usr/bin/false".into())), @@ -410,10 +410,13 @@ fn build_no_file_config() { } #[test] -fn build_default_profile_used_when_cli_absent() { +fn build_top_level_profile_used_when_cli_absent() { let file_config = FileConfig { - default_profile: Some("auto".into()), - profile: HashMap::from([( + options: Options { + profile: Some("auto".into()), + ..Options::default() + }, + profiles: HashMap::from([( "auto".into(), Options { hardened: Some(true), @@ -427,10 +430,13 @@ fn build_default_profile_used_when_cli_absent() { } #[test] -fn build_cli_profile_overrides_default_profile() { +fn build_cli_profile_overrides_top_level_profile() { let file_config = FileConfig { - default_profile: Some("auto".into()), - profile: HashMap::from([ + options: Options { + profile: Some("auto".into()), + ..Options::default() + }, + profiles: HashMap::from([ ( "auto".into(), Options { @@ -457,9 +463,12 @@ fn build_cli_profile_overrides_default_profile() { } #[test] -fn build_missing_default_profile_errors() { +fn build_missing_top_level_profile_errors() { let file_config = FileConfig { - default_profile: Some("nope".into()), + options: Options { + profile: Some("nope".into()), + ..Options::default() + }, ..FileConfig::default() }; assert!(matches!( @@ -480,6 +489,227 @@ fn build_missing_profile_errors() { )); } +#[test] +fn inheritance_merges_parent_into_child() { + let file_config = FileConfig { + profiles: HashMap::from([ + ( + "base".into(), + Options { + hardened: Some(true), + unshare_net: Some(true), + env: vec!["FROM_PARENT=1".into()], + ..Options::default() + }, + ), + ( + "child".into(), + Options { + profile: Some("base".into()), + hardened: Some(false), + seccomp: Some(false), + env: vec!["FROM_CHILD=1".into()], + ..Options::default() + }, + ), + ]), + ..FileConfig::default() + }; + let args = Args { + profile: Some("child".into()), + ..args_with_command() + }; + let config = build(args, Some(file_config)).unwrap(); + + assert!(!config.hardened); + assert!(config.unshare_net); + assert!(!config.seccomp); + assert_eq!( + config.env, + vec![ + EnvEntry::Set("FROM_PARENT".into(), "1".into()), + EnvEntry::Set("FROM_CHILD".into(), "1".into()), + ] + ); +} + +#[test] +fn inheritance_walks_multi_level_chain() { + let file_config = FileConfig { + profiles: HashMap::from([ + ( + "grand".into(), + Options { + hardened: Some(true), + env: vec!["FROM_GRAND=1".into()], + ..Options::default() + }, + ), + ( + "parent".into(), + Options { + profile: Some("grand".into()), + env: vec!["FROM_PARENT=1".into()], + ..Options::default() + }, + ), + ( + "child".into(), + Options { + profile: Some("parent".into()), + env: vec!["FROM_CHILD=1".into()], + ..Options::default() + }, + ), + ]), + ..FileConfig::default() + }; + let args = Args { + profile: Some("child".into()), + ..args_with_command() + }; + let config = build(args, Some(file_config)).unwrap(); + + assert!(config.hardened); + assert_eq!( + config.env, + vec![ + EnvEntry::Set("FROM_GRAND".into(), "1".into()), + EnvEntry::Set("FROM_PARENT".into(), "1".into()), + EnvEntry::Set("FROM_CHILD".into(), "1".into()), + ] + ); +} + +#[test] +fn top_level_profile_walks_chain() { + let file_config = FileConfig { + options: Options { + profile: Some("child".into()), + ..Options::default() + }, + profiles: HashMap::from([ + ( + "base".into(), + Options { + hardened: Some(true), + ..Options::default() + }, + ), + ( + "child".into(), + Options { + profile: Some("base".into()), + ..Options::default() + }, + ), + ]), + ..FileConfig::default() + }; + let config = build(args_with_command(), Some(file_config)).unwrap(); + assert!(config.hardened); +} + +#[test] +fn inheritance_cycle_errors() { + let file_config = FileConfig { + profiles: HashMap::from([ + ( + "a".into(), + Options { + profile: Some("b".into()), + ..Options::default() + }, + ), + ( + "b".into(), + Options { + profile: Some("a".into()), + ..Options::default() + }, + ), + ]), + ..FileConfig::default() + }; + let args = Args { + profile: Some("a".into()), + ..args_with_command() + }; + assert!(matches!( + build(args, Some(file_config)), + Err(SandboxError::ProfileCycle(_)) + )); +} + +#[test] +fn missing_parent_profile_errors() { + let file_config = FileConfig { + profiles: HashMap::from([( + "child".into(), + Options { + profile: Some("ghost".into()), + ..Options::default() + }, + )]), + ..FileConfig::default() + }; + let args = Args { + profile: Some("child".into()), + ..args_with_command() + }; + assert!(matches!( + build(args, Some(file_config)), + Err(SandboxError::ProfileNotFound(name)) if name == "ghost" + )); +} + +#[test] +fn inheritance_chain_spans_extra_config() { + 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, + r#" + [profiles.child] + profile = "base" + env = ["FROM_CHILD=1"] + "#, + ) + .unwrap(); + std::fs::write( + &base_path, + format!( + r#" + extra-config = "{}" + + [profiles.base] + hardened = true + env = ["FROM_PARENT=1"] + "#, + extra_path.display() + ), + ) + .unwrap(); + + let file_config = FileConfig::load(&base_path).unwrap(); + let args = Args { + profile: Some("child".into()), + ..args_with_command() + }; + let config = build(args, Some(file_config)).unwrap(); + + assert!(config.hardened); + assert_eq!( + config.env, + vec![ + EnvEntry::Set("FROM_PARENT".into(), "1".into()), + EnvEntry::Set("FROM_CHILD".into(), "1".into()), + ] + ); +} + #[test] fn build_conflicting_mode_errors() { let file_config = FileConfig { @@ -700,7 +930,7 @@ fn build_env_accumulates_disjoint_keys() { env: vec!["A=global".into(), "B".into()], ..Options::default() }, - profile: HashMap::from([( + profiles: HashMap::from([( "p".into(), Options { env: vec!["C=profile".into()], @@ -733,7 +963,7 @@ fn build_env_later_tier_overrides_earlier() { env: vec!["A=global".into()], ..Options::default() }, - profile: HashMap::from([( + profiles: HashMap::from([( "p".into(), Options { env: vec!["A=profile".into()], @@ -818,7 +1048,7 @@ fn build_unsetenv_accumulates() { unsetenv: vec!["G".into()], ..Options::default() }, - profile: HashMap::from([( + profiles: HashMap::from([( "p".into(), Options { unsetenv: vec!["P".into()], @@ -843,7 +1073,7 @@ fn build_mask_accumulates() { mask: vec![PathBuf::from("/tmp/a")], ..Options::default() }, - profile: HashMap::from([( + profiles: HashMap::from([( "extra".into(), Options { mask: vec![PathBuf::from("/tmp/b")], @@ -876,7 +1106,7 @@ fn unknown_option_rejected() { #[test] fn unknown_profile_option_rejected() { let toml = r#" - [profile.test] + [profiles.test] hardened = true frobnicate = 42 "#; @@ -1081,7 +1311,7 @@ fn merge_options_lists_append_base_then_extra() { #[test] fn merge_file_config_profile_merged_by_name() { let base = FileConfig { - profile: HashMap::from([( + profiles: HashMap::from([( "claude".into(), Options { rw: vec!["/a".into()], @@ -1092,7 +1322,7 @@ fn merge_file_config_profile_merged_by_name() { ..FileConfig::default() }; let extra = FileConfig { - profile: HashMap::from([ + profiles: HashMap::from([ ( "claude".into(), Options { @@ -1112,32 +1342,41 @@ fn merge_file_config_profile_merged_by_name() { ..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)); + assert_eq!(merged.profiles["claude"].hardened, Some(true)); + assert_eq!(merged.profiles["claude"].rw, vec!["/a", "/b"]); + assert_eq!(merged.profiles["codex"].unshare_net, Some(true)); } #[test] -fn merge_file_config_default_profile_extra_overrides() { +fn merge_file_config_top_level_profile_extra_overrides() { let base = FileConfig { - default_profile: Some("claude".into()), + options: Options { + profile: Some("claude".into()), + ..Options::default() + }, ..FileConfig::default() }; let extra = FileConfig { - default_profile: Some("codex".into()), + options: Options { + profile: Some("codex".into()), + ..Options::default() + }, ..FileConfig::default() }; - assert_eq!(base.merge_with(extra).default_profile, Some("codex".into())); + assert_eq!(base.merge_with(extra).options.profile, Some("codex".into())); } #[test] -fn merge_file_config_default_profile_extra_unset_inherits() { +fn merge_file_config_top_level_profile_extra_unset_inherits() { let base = FileConfig { - default_profile: Some("claude".into()), + options: Options { + profile: Some("claude".into()), + ..Options::default() + }, ..FileConfig::default() }; assert_eq!( - base.merge_with(FileConfig::default()).default_profile, + base.merge_with(FileConfig::default()).options.profile, Some("claude".into()) ); }