Implement extra-config option
This commit is contained in:
@@ -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.
|
`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 = "<absolute path>"` 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
|
## Build and test
|
||||||
|
|
||||||
- `cargo fmt` and `cargo clippy` must pass before every commit.
|
- `cargo fmt` and `cargo clippy` must pass before every commit.
|
||||||
|
|||||||
@@ -1,6 +1,12 @@
|
|||||||
# Layered settings: CLI > active profile > globals. `--profile` selects the
|
# Layered settings: CLI > active profile > globals. `--profile` selects the
|
||||||
# profile, otherwise `default-profile` below is used. Vec fields append across
|
# profile, otherwise `default-profile` below is used. Vec fields append across
|
||||||
# layers; scalars replace.
|
# 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
|
whitelist = true
|
||||||
# blacklist = true
|
# blacklist = true
|
||||||
|
|||||||
+283
@@ -292,6 +292,8 @@ pub struct FileConfig {
|
|||||||
pub profile: HashMap<String, Options>,
|
pub profile: HashMap<String, Options>,
|
||||||
#[serde(rename = "default-profile", default)]
|
#[serde(rename = "default-profile", default)]
|
||||||
pub default_profile: Option<String>,
|
pub default_profile: Option<String>,
|
||||||
|
#[serde(rename = "extra-config", default)]
|
||||||
|
pub extra_config: Option<PathBuf>,
|
||||||
// Collects unrecognized keys; deny_unknown_fields is incompatible with flatten.
|
// Collects unrecognized keys; deny_unknown_fields is incompatible with flatten.
|
||||||
#[serde(flatten)]
|
#[serde(flatten)]
|
||||||
_unknown: HashMap<String, toml::Value>,
|
_unknown: HashMap<String, toml::Value>,
|
||||||
@@ -299,6 +301,30 @@ pub struct FileConfig {
|
|||||||
|
|
||||||
impl FileConfig {
|
impl FileConfig {
|
||||||
pub fn load(path: &Path) -> Result<Self, SandboxError> {
|
pub fn load(path: &Path) -> Result<Self, SandboxError> {
|
||||||
|
Self::load_file(path)?.load_extra()
|
||||||
|
}
|
||||||
|
|
||||||
|
fn load_extra(self) -> Result<Self, SandboxError> {
|
||||||
|
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<Self, SandboxError> {
|
||||||
let contents = std::fs::read_to_string(path).map_err(|e| SandboxError::ConfigRead {
|
let contents = std::fs::read_to_string(path).map_err(|e| SandboxError::ConfigRead {
|
||||||
path: path.to_path_buf(),
|
path: path.to_path_buf(),
|
||||||
source: e,
|
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<Self, SandboxError> {
|
fn parse(contents: &str) -> Result<Self, SandboxError> {
|
||||||
let config: Self = toml::from_str(contents).map_err(|e| SandboxError::ConfigParse {
|
let config: Self = toml::from_str(contents).map_err(|e| SandboxError::ConfigParse {
|
||||||
path: PathBuf::new(),
|
path: PathBuf::new(),
|
||||||
@@ -363,6 +407,28 @@ pub struct Options {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl 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> {
|
fn validate_paths(&mut self) -> Result<(), SandboxError> {
|
||||||
if let Some(ref chdir) = self.chdir {
|
if let Some(ref chdir) = self.chdir {
|
||||||
self.chdir = Some(expand_and_canonicalize(chdir)?);
|
self.chdir = Some(expand_and_canonicalize(chdir)?);
|
||||||
@@ -374,6 +440,19 @@ impl Options {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn append<T>(mut base: Vec<T>, extra: Vec<T>) -> Vec<T> {
|
||||||
|
base.extend(extra);
|
||||||
|
base
|
||||||
|
}
|
||||||
|
|
||||||
|
fn pick_mode_flags(base: &Options, extra: &Options) -> (Option<bool>, Option<bool>) {
|
||||||
|
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<PathBuf, SandboxError> {
|
fn expand_and_require_existing(path: &Path) -> Result<PathBuf, SandboxError> {
|
||||||
let expanded = expand_and_require_absolute(path)?;
|
let expanded = expand_and_require_absolute(path)?;
|
||||||
if !expanded.exists() {
|
if !expanded.exists() {
|
||||||
@@ -1485,6 +1564,210 @@ mod tests {
|
|||||||
assert!(matches!(result, Err(SandboxError::NoCommand)));
|
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]) {
|
fn assert_paths(actual: &[String], expected: &[&str]) {
|
||||||
let expected: Vec<String> = expected.iter().map(|s| s.to_string()).collect();
|
let expected: Vec<String> = expected.iter().map(|s| s.to_string()).collect();
|
||||||
assert_eq!(actual, &expected);
|
assert_eq!(actual, &expected);
|
||||||
|
|||||||
@@ -25,6 +25,7 @@ pub enum SandboxError {
|
|||||||
ConflictingEnvKey(String),
|
ConflictingEnvKey(String),
|
||||||
InvalidEnvEntry(String),
|
InvalidEnvEntry(String),
|
||||||
UnknownConfigKey(String),
|
UnknownConfigKey(String),
|
||||||
|
NestedExtraConfig(PathBuf),
|
||||||
ConfigPathNotAbsolute(PathBuf),
|
ConfigPathNotAbsolute(PathBuf),
|
||||||
InvalidBwrapArg(String),
|
InvalidBwrapArg(String),
|
||||||
InvalidBindSpec(String),
|
InvalidBindSpec(String),
|
||||||
@@ -75,6 +76,11 @@ impl std::fmt::Display for SandboxError {
|
|||||||
write!(f, "invalid env entry (expected KEY or KEY=VALUE): {raw:?}")
|
write!(f, "invalid env entry (expected KEY or KEY=VALUE): {raw:?}")
|
||||||
}
|
}
|
||||||
Self::UnknownConfigKey(key) => write!(f, "unknown config key: {key}"),
|
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) => {
|
Self::ConfigPathNotAbsolute(p) => {
|
||||||
write!(f, "config path is not absolute: {}", p.display())
|
write!(f, "config path is not absolute: {}", p.display())
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user