Keep env values off the bwrap command line

AGENTS.md

@@ -23,3 +23,7 @@ Later bwrap arguments override earlier ones for the same path. This has caused m
 - User `--rw`/`--ro` escape hatches must come **after** mode setup so they can override sandbox restrictions.
 
 Take extreme care when reordering any arguments in `sandbox.rs` or refactor things and test thoroughly.
+
+### Env values must never become bwrap arguments
+
+`/proc/<pid>/cmdline` is world-readable, and the same string is exposed inside every sandbox at `/run/agent-sandbox/bwrap-args`. `--setenv KEY VALUE` therefore publishes every secret the user puts in `env = [...]`. `set_sandbox_env` in `sandbox.rs` sets the whole child environment on the bwrap process instead, and bwrap passes its own environment on. Do not "simplify" it back to `--setenv`; `tests/e2e/env.rs` guards this.

README.md

@@ -33,6 +33,8 @@ Both modes clamp the environment the child sees so prompt-injected agents can't
 
 Disable the built-in policy entirely with `--no-env-filter` (or `env-filter = false` in the config file) to pass the parent env through unchanged. User `--setenv`/`--unsetenv` escape hatches still apply.
 
+Env values never become `bwrap` arguments. The final environment is set on the `bwrap` process itself, so a secret passed via `env = ["KEY=VALUE"]` stays out of the world-readable `/proc/<pid>/cmdline`, out of `ps` output, and out of the exposed command line below.
+
 ## Seccomp
 
 Both modes apply a seccomp-BPF syscall allowlist derived from Podman's default profile. Dangerous syscalls (`mount`, `unshare`, `ptrace`, `bpf`, `perf_event_open`, `io_uring_*`, `keyctl`, `kexec_*`, …) return `ENOSYS`. Disable with `--no-seccomp` or `seccomp = false` in the config file.

src/env.rs

@@ -1,25 +1,27 @@
-pub fn keepenv_args(keys: &[String], parent_env: &[(String, String)]) -> Vec<String> {
-    let mut args = Vec::new();
-    for key in keys {
-        if let Some(value) = parent_env.iter().find_map(|(k, v)| (k == key).then_some(v)) {
-            args.push("--setenv".to_string());
-            args.push(key.clone());
-            args.push(value.clone());
-        }
-    }
-    args
+use std::collections::BTreeMap;
+
+pub type SandboxEnv = BTreeMap<String, String>;
+
+pub fn find_parent_value(parent_env: &[(String, String)], key: &str) -> Option<String> {
+    parent_env
+        .iter()
+        .find_map(|(k, v)| (k == key).then(|| v.clone()))
 }
 
-pub fn whitelist_env_args(parent_env: &[(String, String)]) -> Vec<String> {
-    let mut args = vec!["--clearenv".to_string()];
-    for (key, value) in parent_env {
-        if whitelist_keeps(key) {
-            args.push("--setenv".to_string());
-            args.push(key.clone());
-            args.push(value.clone());
-        }
-    }
-    args
+pub fn copy_parent_env(parent_env: &[(String, String)]) -> SandboxEnv {
+    select_vars(parent_env, |_| true)
+}
+
+fn select_vars(parent_env: &[(String, String)], permits: impl Fn(&str) -> bool) -> SandboxEnv {
+    parent_env
+        .iter()
+        .filter(|(key, _)| permits(key))
+        .cloned()
+        .collect()
+}
+
+pub fn apply_whitelist(parent_env: &[(String, String)]) -> SandboxEnv {
+    select_vars(parent_env, whitelist_keeps)
 }
 
 fn whitelist_keeps(key: &str) -> bool {
@@ -96,15 +98,8 @@ const WHITELIST_KEEP_PREFIXES: &[&str] = &[
     "OTEL_",
 ];
 
-pub fn blacklist_env_args(parent_env: &[(String, String)]) -> Vec<String> {
-    let mut args = Vec::new();
-    for (key, _) in parent_env {
-        if blacklist_drops(key) {
-            args.push("--unsetenv".to_string());
-            args.push(key.clone());
-        }
-    }
-    args
+pub fn apply_blacklist(parent_env: &[(String, String)]) -> SandboxEnv {
+    select_vars(parent_env, |key| !blacklist_drops(key))
 }
 
 fn blacklist_drops(key: &str) -> bool {

src/sandbox.rs

@@ -71,8 +71,7 @@ fn build_command(config: &SandboxConfig) -> Result<(Bwrap, BwrapCommandLine), Sa
         add_rw_bind(&mut cmd, spec)?;
     }
 
-    add_env_policy(&mut cmd, config);
-    add_user_env_overrides(&mut cmd, config);
+    set_sandbox_env(&mut cmd, config);
 
     cmd.args(["--remount-ro", "/"]);
     cmd.arg("--die-with-parent");
@@ -97,36 +96,48 @@ fn build_command(config: &SandboxConfig) -> Result<(Bwrap, BwrapCommandLine), Sa
     Ok((bwrap, command_line))
 }
 
-fn add_env_policy(cmd: &mut Command, config: &SandboxConfig) {
-    if !config.env_filter {
-        return;
-    }
+/// bwrap passes its own environment on to the child. `--setenv` would instead
+/// publish every value in the world-readable `/proc/<pid>/cmdline`.
+fn set_sandbox_env(cmd: &mut Command, config: &SandboxConfig) {
     let parent_env: Vec<(String, String)> = std::env::vars().collect();
-    let args = match config.mode {
-        SandboxMode::Blacklist => env::blacklist_env_args(&parent_env),
-        SandboxMode::Whitelist => env::whitelist_env_args(&parent_env),
-    };
-    cmd.args(args);
+
+    let mut sandbox_env = apply_env_filter(&parent_env, config);
+    apply_user_env_overrides(&mut sandbox_env, &parent_env, config);
+
+    cmd.env_clear();
+    cmd.envs(sandbox_env);
 }
 
-fn add_user_env_overrides(cmd: &mut Command, config: &SandboxConfig) {
-    let mut keep_keys: Vec<String> = Vec::new();
+fn apply_env_filter(parent_env: &[(String, String)], config: &SandboxConfig) -> env::SandboxEnv {
+    if !config.env_filter {
+        return env::copy_parent_env(parent_env);
+    }
+    match config.mode {
+        SandboxMode::Blacklist => env::apply_blacklist(parent_env),
+        SandboxMode::Whitelist => env::apply_whitelist(parent_env),
+    }
+}
+
+fn apply_user_env_overrides(
+    sandbox_env: &mut env::SandboxEnv,
+    parent_env: &[(String, String)],
+    config: &SandboxConfig,
+) {
     for entry in &config.env {
         match entry {
             EnvEntry::Set(key, value) => {
-                cmd.arg("--setenv").arg(key).arg(value);
+                sandbox_env.insert(key.clone(), value.clone());
+            }
+            EnvEntry::Keep(key) => {
+                if let Some(value) = env::find_parent_value(parent_env, key) {
+                    sandbox_env.insert(key.clone(), value);
+                }
             }
-            EnvEntry::Keep(key) => keep_keys.push(key.clone()),
         }
     }
 
-    if !keep_keys.is_empty() {
-        let parent_env: Vec<(String, String)> = std::env::vars().collect();
-        cmd.args(env::keepenv_args(&keep_keys, &parent_env));
-    }
-
     for key in &config.unsetenv {
-        cmd.arg("--unsetenv").arg(key);
+        sandbox_env.remove(key);
     }
 }
 

tests/e2e/env.rs

@@ -16,6 +16,49 @@ fn printenv_inside(args: &[&str], vars: &[(&str, &str)], query: &[&str]) -> Stri
         .expect("agent-sandbox binary failed to execute");
     String::from_utf8_lossy(&output.stdout).into_owned()
 }
+#[test]
+fn user_env_value_never_reaches_the_command_line() {
+    let stdout = dry_run_command_line(&["--env", "MY_SECRET=hunter2"], &[]);
+    assert!(
+        !stdout.contains("hunter2"),
+        "user env value leaked into the bwrap command line: {stdout}"
+    );
+}
+
+fn dry_run_command_line(args: &[&str], vars: &[(&str, &str)]) -> String {
+    let mut cmd = Sandbox::new(args);
+    cmd.arg("--dry-run");
+    for (k, v) in vars {
+        cmd.env(k, v);
+    }
+    let output = cmd
+        .args(["--", "true"])
+        .output()
+        .expect("agent-sandbox binary failed to execute");
+    String::from_utf8_lossy(&output.stdout).into_owned()
+}
+
+#[test]
+fn kept_host_env_value_never_reaches_the_command_line() {
+    let stdout = dry_run_command_line(&[], &[("TERM", "xterm-canary")]);
+    assert!(
+        !stdout.contains("xterm-canary"),
+        "kept host env value leaked into the bwrap command line: {stdout}"
+    );
+}
+
+#[test]
+fn passed_through_env_value_never_reaches_the_command_line() {
+    let stdout = dry_run_command_line(
+        &["--env", "PASSED_THROUGH"],
+        &[("PASSED_THROUGH", "from-host-canary")],
+    );
+    assert!(
+        !stdout.contains("from-host-canary"),
+        "passed-through env value leaked into the bwrap command line: {stdout}"
+    );
+}
+
 #[test]
 fn whitelist_keeps_identity_and_terminal_vars() {
     let stdout = printenv_inside(

tests/unit/env.rs

@@ -1,37 +1,55 @@
 use super::*;
 
-#[test]
-fn keepenv_emits_setenv_for_present_key() {
-    let parent = vec![("XDG_RUNTIME_DIR".into(), "/run/user/1000".into())];
-    let args = keepenv_args(&["XDG_RUNTIME_DIR".into()], &parent);
-    assert_eq!(args, vec!["--setenv", "XDG_RUNTIME_DIR", "/run/user/1000"]);
+fn parent(pairs: &[(&str, &str)]) -> Vec<(String, String)> {
+    pairs
+        .iter()
+        .map(|(k, v)| (k.to_string(), v.to_string()))
+        .collect()
 }
 
 #[test]
-fn keepenv_skips_absent_keys() {
-    let parent = vec![("HOME".into(), "/home/me".into())];
-    let args = keepenv_args(&["XDG_RUNTIME_DIR".into()], &parent);
-    assert!(args.is_empty());
+fn copy_parent_env_retains_every_var() {
+    let sandbox_env = copy_parent_env(&parent(&[("GH_TOKEN", "secret"), ("XDG_SEAT", "seat0")]));
+    assert_eq!(sandbox_env.get("GH_TOKEN").unwrap(), "secret");
+    assert_eq!(sandbox_env.get("XDG_SEAT").unwrap(), "seat0");
 }
 
 #[test]
-fn keepenv_preserves_caller_key_order() {
-    let parent = vec![
-        ("B".into(), "2".into()),
-        ("A".into(), "1".into()),
-        ("C".into(), "3".into()),
-    ];
-    let args = keepenv_args(&["A".into(), "B".into(), "C".into()], &parent);
+fn whitelist_retains_allowed_var() {
+    let sandbox_env = apply_whitelist(&parent(&[("TERM", "xterm")]));
+    assert_eq!(sandbox_env.get("TERM").unwrap(), "xterm");
+}
+
+#[test]
+fn whitelist_removes_unlisted_var() {
+    let sandbox_env = apply_whitelist(&parent(&[("SOME_RANDOM_NOISE_VAR", "leak")]));
+    assert!(sandbox_env.is_empty());
+}
+
+#[test]
+fn blacklist_removes_secret_var() {
+    let sandbox_env = apply_blacklist(&parent(&[("GH_TOKEN", "secret"), ("MY_NICE_VAR", "hello")]));
+    assert!(!sandbox_env.contains_key("GH_TOKEN"));
+    assert_eq!(sandbox_env.get("MY_NICE_VAR").unwrap(), "hello");
+}
+
+#[test]
+fn blacklist_carves_out_vendor_prefix() {
+    let sandbox_env = apply_blacklist(&parent(&[("ANTHROPIC_API_KEY", "key")]));
+    assert_eq!(sandbox_env.get("ANTHROPIC_API_KEY").unwrap(), "key");
+}
+
+#[test]
+fn find_parent_value_returns_present_value() {
+    let parent_env = parent(&[("XDG_RUNTIME_DIR", "/run/user/1000")]);
     assert_eq!(
-        args,
-        vec![
-            "--setenv", "A", "1", "--setenv", "B", "2", "--setenv", "C", "3"
-        ]
+        find_parent_value(&parent_env, "XDG_RUNTIME_DIR").unwrap(),
+        "/run/user/1000"
     );
 }
 
 #[test]
-fn keepenv_empty_keys_yields_nothing() {
-    let parent = vec![("A".into(), "1".into())];
-    assert!(keepenv_args(&[], &parent).is_empty());
+fn find_parent_value_returns_none_for_absent_key() {
+    let parent_env = parent(&[("HOME", "/home/me")]);
+    assert!(find_parent_value(&parent_env, "XDG_RUNTIME_DIR").is_none());
 }