diff --git a/README.md b/README.md index 53c2bac..3287dcb 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ Settings can be stored in a TOML config file at `$XDG_CONFIG_HOME/agent-sandbox/ ln -sf "$(pwd)/config-example.toml" "${XDG_CONFIG_HOME:-$HOME/.config}/agent-sandbox/config.toml" ``` -Top-level keys set defaults; `[profile.]` sections define named presets selectable with `--profile `. CLI flags always take highest precedence, followed by the active profiles, then top-level defaults. When a profile is active, list-valued fields (`ro`, `rw`, `mask`, `env`, `unsetenv`, `bwrap-args`) append to the globals; scalar fields (modes, flags, `entrypoint`, `command`, `chdir`) replace them. +Top-level keys set defaults; `[profile.]` sections define named presets selectable with `--profile `. CLI flags always take highest precedence, followed by the active profiles, then top-level defaults. When a profile is active, list-valued fields (`ro`, `rw`, `mask`, `env`, `unsetenv`, `bwrap-args`, `keep-fd`) append to the globals; scalar fields (modes, flags, `entrypoint`, `command`, `chdir`) replace them. Several profiles combine: `--profile claude,gpu` (or `--profile claude --profile gpu`, or `profile = ["claude", "gpu"]` in the config) merges them left to right under the same rules, so a later profile's scalars win. A CLI `--profile` replaces the config's list rather than extending it. A profile's own `profile = ...` names its parents, which merge before it; a profile reached twice merges once. @@ -84,11 +84,14 @@ A user `--rw`/`--ro` targeting `/run/agent-sandbox` overrides the directory, lik When the agent needs access to something the sandbox blocks, use `--rw` or `--ro` for paths and `--setenv`/`--unsetenv` for env vars. User overrides always win over the built-in policies. +Inherited file descriptors above stderr never reach `bwrap` or the sandbox, because an open directory descriptor would bypass every mount restriction. Pass one through with `--keep-fd FD` (or `keep-fd = [FD]`), for example to feed a `bwrap` option that reads a descriptor. + ```bash agent-sandbox --rw /var/run/docker.sock -- claude --dangerously-skip-permissions agent-sandbox --ro ~/.aws -- claude --dangerously-skip-permissions agent-sandbox --setenv DATABASE_URL=postgres://localhost/dev -- claude agent-sandbox --unsetenv HTTP_PROXY -- claude +agent-sandbox --keep-fd 3 --bwrap-arg '--file 3 /tmp/input' -- claude 3, + /// Pass an inherited file descriptor into the sandbox (repeatable). + /// All other inherited descriptors above stderr are closed. + #[arg(long = "keep-fd", value_name = "FD", action = clap::ArgAction::Append)] + pub keep_fd: Vec, + /// Run this binary with the trailing args as its arguments #[arg(long, value_name = "CMD")] pub entrypoint: Option, diff --git a/src/config.rs b/src/config.rs index b7e0840..2a472c5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,5 +1,6 @@ use std::collections::{HashMap, HashSet}; use std::ffi::OsString; +use std::os::fd::RawFd; use std::path::{Path, PathBuf}; use serde::Deserialize; @@ -93,6 +94,7 @@ pub fn build(args: Args, file_config: Option) -> Result, #[serde(default)] pub bwrap_args: Vec, + #[serde(default)] + pub keep_fd: Vec, } impl Options { @@ -551,6 +555,7 @@ impl Options { env: append(self.env, extra.env), unsetenv: append(self.unsetenv, extra.unsetenv), bwrap_args: append(self.bwrap_args, extra.bwrap_args), + keep_fd: append(self.keep_fd, extra.keep_fd), } } diff --git a/src/errors.rs b/src/errors.rs index b297c33..89dc742 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,3 +1,4 @@ +use std::os::fd::RawFd; use std::path::PathBuf; #[derive(Debug)] @@ -36,6 +37,7 @@ pub enum SandboxError { PersistentTmpDir(PathBuf, String), ProcChildrenMissing, InheritedFdsCloexec(std::io::Error), + KeepFd(RawFd, std::io::Error), } impl std::fmt::Display for SandboxError { @@ -117,6 +119,12 @@ impl std::fmt::Display for SandboxError { f, "the kernel does not provide /proc//task//children (CONFIG_PROC_CHILDREN); agent-sandbox needs it to find the processes it must clean up" ), + Self::KeepFd(fd, e) => { + write!( + f, + "cannot pass file descriptor {fd} into the sandbox (--keep-fd): {e}" + ) + } Self::InheritedFdsCloexec(e) => write!( f, "cannot keep inherited file descriptors out of the sandbox (close_range with CLOSE_RANGE_CLOEXEC needs Linux 5.11+): {e}" @@ -134,6 +142,7 @@ impl std::error::Error for SandboxError { Self::ConfigRead { source, .. } => Some(source), Self::ConfigParse { source, .. } => Some(source), Self::InheritedFdsCloexec(e) => Some(e), + Self::KeepFd(_, e) => Some(e), _ => None, } } diff --git a/src/inherited_fds.rs b/src/inherited_fds.rs index 6900de5..d5c30ff 100644 --- a/src/inherited_fds.rs +++ b/src/inherited_fds.rs @@ -1,10 +1,20 @@ use std::io; +use std::os::fd::RawFd; use crate::SandboxError; const FIRST_FD_AFTER_STDIO: libc::c_uint = 3; -pub fn keep_out_of_sandbox() -> Result<(), SandboxError> { +pub fn keep_out_of_sandbox(kept_fds: &[RawFd]) -> Result<(), SandboxError> { + mark_all_after_stdio_cloexec().map_err(SandboxError::InheritedFdsCloexec)?; + + for &fd in kept_fds { + clear_cloexec(fd).map_err(|e| SandboxError::KeepFd(fd, e))?; + } + Ok(()) +} + +fn mark_all_after_stdio_cloexec() -> io::Result<()> { let result = unsafe { libc::syscall( libc::SYS_close_range, @@ -14,7 +24,18 @@ pub fn keep_out_of_sandbox() -> Result<(), SandboxError> { ) }; if result < 0 { - return Err(SandboxError::InheritedFdsCloexec(io::Error::last_os_error())); + return Err(io::Error::last_os_error()); + } + Ok(()) +} + +pub fn clear_cloexec(fd: RawFd) -> io::Result<()> { + let flags = unsafe { libc::fcntl(fd, libc::F_GETFD) }; + if flags < 0 { + return Err(io::Error::last_os_error()); + } + if unsafe { libc::fcntl(fd, libc::F_SETFD, flags & !libc::FD_CLOEXEC) } < 0 { + return Err(io::Error::last_os_error()); } Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index 9e9f8c2..53d31f3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,6 +19,7 @@ pub use signals::exit_with_status; use std::ffi::OsString; use std::fs; +use std::os::fd::RawFd; use std::path::PathBuf; use std::process::ExitStatus; use std::time::Duration; @@ -67,6 +68,7 @@ pub struct SandboxConfig { pub env: Vec, pub unsetenv: Vec, pub bwrap_args: Vec, + pub keep_fds: Vec, pub command: PathBuf, pub command_args: Vec, pub chdir: PathBuf, @@ -100,7 +102,7 @@ fn resolve_run_user_from_proc() -> Option { } pub fn run(config: SandboxConfig) -> Result { - inherited_fds::keep_out_of_sandbox()?; + inherited_fds::keep_out_of_sandbox(&config.keep_fds)?; preflight::check(&config)?; let forwarded_signals = ForwardedSignals::detect(); diff --git a/src/persistent_tmp.rs b/src/persistent_tmp.rs index 13eb62d..39ef1ca 100644 --- a/src/persistent_tmp.rs +++ b/src/persistent_tmp.rs @@ -7,6 +7,7 @@ use std::process::Command; use crate::SandboxConfig; use crate::errors::SandboxError; +use crate::inherited_fds; pub struct PersistentTmpDirs { tmp: BindSource, @@ -80,7 +81,8 @@ impl PersistentTmpDir { .map_err(|e| self.error(format!("open: {e}")))?; self.verify_owner(&file)?; let fd: OwnedFd = file.into(); - clear_cloexec(&fd).map_err(|e| self.error(format!("clear FD_CLOEXEC: {e}")))?; + inherited_fds::clear_cloexec(fd.as_raw_fd()) + .map_err(|e| self.error(format!("clear FD_CLOEXEC: {e}")))?; Ok(fd) } @@ -108,18 +110,6 @@ impl PersistentTmpDir { } } -fn clear_cloexec(fd: &OwnedFd) -> io::Result<()> { - let raw = fd.as_raw_fd(); - let flags = unsafe { libc::fcntl(raw, libc::F_GETFD) }; - if flags < 0 { - return Err(io::Error::last_os_error()); - } - if unsafe { libc::fcntl(raw, libc::F_SETFD, flags & !libc::FD_CLOEXEC) } < 0 { - return Err(io::Error::last_os_error()); - } - Ok(()) -} - #[cfg(test)] mod tests { use super::*; diff --git a/tests/e2e/mounts.rs b/tests/e2e/mounts.rs index 0f7029a..79c341a 100644 --- a/tests/e2e/mounts.rs +++ b/tests/e2e/mounts.rs @@ -500,11 +500,9 @@ fn persistent_tmp_leaks_no_descriptor_into_host_tmp() { #[test] fn inherited_host_descriptor_does_not_reach_sandbox() { - const PASSED_FD: i32 = 9; let host_dir = TempDir::new().unwrap(); fs::write(host_dir.path().join("canary"), "host-secret").unwrap(); let host_dir_handle = fs::File::open(host_dir.path()).unwrap(); - let host_dir_fd = host_dir_handle.as_raw_fd(); let mut sandbox = Sandbox::new(&[]); sandbox.args([ @@ -513,12 +511,7 @@ fn inherited_host_descriptor_does_not_reach_sandbox() { "-c", &format!("cat /proc/self/fd/{PASSED_FD}/canary 2>/dev/null; true"), ]); - unsafe { - sandbox.pre_exec(move || match libc::dup2(host_dir_fd, PASSED_FD) { - -1 => Err(std::io::Error::last_os_error()), - _ => Ok(()), - }); - } + pass_as_fd(&mut sandbox, &host_dir_handle, PASSED_FD); let output = sandbox .output() .expect("agent-sandbox binary failed to execute"); @@ -534,6 +527,52 @@ fn inherited_host_descriptor_does_not_reach_sandbox() { ); } +#[test] +fn kept_descriptor_reaches_sandbox() { + let scratch = TempDir::new().unwrap(); + let kept_file = scratch.path().join("kept"); + fs::write(&kept_file, "kept-content").unwrap(); + let kept_handle = fs::File::open(&kept_file).unwrap(); + + let mut sandbox = Sandbox::new(&["--keep-fd", &PASSED_FD.to_string()]); + sandbox.args(["--", "bash", "-c", &format!("cat <&{PASSED_FD}")]); + pass_as_fd(&mut sandbox, &kept_handle, PASSED_FD); + let output = sandbox + .output() + .expect("agent-sandbox binary failed to execute"); + + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(String::from_utf8_lossy(&output.stdout), "kept-content"); +} + +#[test] +fn keep_fd_rejects_descriptor_that_is_not_open() { + let output = Sandbox::new(&["--keep-fd", "999", "--dry-run"]) + .args(["--", "true"]) + .output() + .expect("agent-sandbox binary failed to execute"); + + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("999"), "stderr: {stderr}"); +} + +const PASSED_FD: i32 = 9; + +fn pass_as_fd(sandbox: &mut Sandbox, file: &fs::File, target_fd: i32) { + let source_fd = file.as_raw_fd(); + unsafe { + sandbox.pre_exec(move || match libc::dup2(source_fd, target_fd) { + -1 => Err(std::io::Error::last_os_error()), + _ => Ok(()), + }); + } +} + #[test] fn persistent_tmp_is_noop_in_blacklist_mode() { let label = format!("e2e-bl-noop-{}-{}", std::process::id(), rand_suffix()); diff --git a/tests/unit/config.rs b/tests/unit/config.rs index 5212e00..353e769 100644 --- a/tests/unit/config.rs +++ b/tests/unit/config.rs @@ -1255,6 +1255,43 @@ fn build_mask_accumulates() { assert_eq!(config.mask.len(), 3); } +#[test] +fn build_keep_fd_accumulates_globals_then_profile_then_cli() { + let file_config = FileConfig { + options: Options { + keep_fd: vec![3], + ..Options::default() + }, + profiles: HashMap::from([( + "extra".into(), + Options { + keep_fd: vec![4], + ..Options::default() + }, + )]), + ..FileConfig::default() + }; + let args = Args { + profile: vec!["extra".into()], + keep_fd: vec![5], + ..args_with_command() + }; + let config = build(args, Some(file_config)).unwrap(); + assert_eq!(config.keep_fds, vec![3, 4, 5]); +} + +#[test] +fn keep_fd_parsed_from_config() { + let config = FileConfig::parse("keep-fd = [3, 4]").unwrap(); + assert_eq!(config.options.keep_fd, vec![3, 4]); +} + +#[test] +fn keep_fd_parsed_from_repeated_cli_flag() { + let args = Args::try_parse_from(["agent-sandbox", "--keep-fd", "3", "--keep-fd", "4"]).unwrap(); + assert_eq!(args.keep_fd, vec![3, 4]); +} + #[test] fn unknown_option_rejected() { let toml = r#" @@ -1460,16 +1497,19 @@ fn merge_options_lists_append_base_then_extra() { let base = Options { rw: vec!["/a".into()], env: vec!["A=1".into()], + keep_fd: vec![3], ..Options::default() }; let extra = Options { rw: vec!["/b".into()], env: vec!["B=2".into()], + keep_fd: vec![4], ..Options::default() }; let merged = base.merge_with(extra); assert_eq!(merged.rw, vec!["/a", "/b"]); assert_eq!(merged.env, vec!["A=1", "B=2"]); + assert_eq!(merged.keep_fd, vec![3, 4]); } #[test]