diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 6fe7f003..a7d7103c 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -64,7 +64,6 @@ async fn run_command_under_sandbox( sandbox_type: SandboxType, ) -> anyhow::Result<()> { let sandbox_mode = create_sandbox_mode(full_auto); - let cwd = std::env::current_dir()?; let config = Config::load_with_cli_overrides( config_overrides .parse_overrides() @@ -75,13 +74,29 @@ async fn run_command_under_sandbox( ..Default::default() }, )?; + + // In practice, this should be `std::env::current_dir()` because this CLI + // does not support `--cwd`, but let's use the config value for consistency. + let cwd = config.cwd.clone(); + // For now, we always use the same cwd for both the command and the + // sandbox policy. In the future, we could add a CLI option to set them + // separately. + let sandbox_policy_cwd = cwd.clone(); + let stdio_policy = StdioPolicy::Inherit; let env = create_env(&config.shell_environment_policy); let mut child = match sandbox_type { SandboxType::Seatbelt => { - spawn_command_under_seatbelt(command, &config.sandbox_policy, cwd, stdio_policy, env) - .await? + spawn_command_under_seatbelt( + command, + cwd, + &config.sandbox_policy, + sandbox_policy_cwd.as_path(), + stdio_policy, + env, + ) + .await? } SandboxType::Landlock => { #[expect(clippy::expect_used)] @@ -91,8 +106,9 @@ async fn run_command_under_sandbox( spawn_command_under_linux_sandbox( codex_linux_sandbox_exe, command, - &config.sandbox_policy, cwd, + &config.sandbox_policy, + sandbox_policy_cwd.as_path(), stdio_policy, env, ) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 8caa2e8e..e9f52b1c 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1,6 +1,7 @@ use std::borrow::Cow; use std::collections::HashMap; use std::collections::HashSet; +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::AtomicU64; @@ -898,6 +899,7 @@ impl Session { exec_args.params, exec_args.sandbox_type, exec_args.sandbox_policy, + exec_args.sandbox_cwd, exec_args.codex_linux_sandbox_exe, exec_args.stdout_stream, ) @@ -2691,6 +2693,7 @@ pub struct ExecInvokeArgs<'a> { pub params: ExecParams, pub sandbox_type: SandboxType, pub sandbox_policy: &'a SandboxPolicy, + pub sandbox_cwd: &'a Path, pub codex_linux_sandbox_exe: &'a Option, pub stdout_stream: Option, } @@ -2882,6 +2885,7 @@ async fn handle_container_exec_with_params( params: params.clone(), sandbox_type, sandbox_policy: &turn_context.sandbox_policy, + sandbox_cwd: &turn_context.cwd, codex_linux_sandbox_exe: &sess.codex_linux_sandbox_exe, stdout_stream: if exec_command_context.apply_patch.is_some() { None @@ -3016,6 +3020,7 @@ async fn handle_sandbox_error( params, sandbox_type: SandboxType::None, sandbox_policy: &turn_context.sandbox_policy, + sandbox_cwd: &turn_context.cwd, codex_linux_sandbox_exe: &sess.codex_linux_sandbox_exe, stdout_stream: if exec_command_context.apply_patch.is_some() { None diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 8fecb16f..9e11604c 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -3,6 +3,7 @@ use std::os::unix::process::ExitStatusExt; use std::collections::HashMap; use std::io; +use std::path::Path; use std::path::PathBuf; use std::process::ExitStatus; use std::time::Duration; @@ -82,6 +83,7 @@ pub async fn process_exec_tool_call( params: ExecParams, sandbox_type: SandboxType, sandbox_policy: &SandboxPolicy, + sandbox_cwd: &Path, codex_linux_sandbox_exe: &Option, stdout_stream: Option, ) -> Result { @@ -94,12 +96,16 @@ pub async fn process_exec_tool_call( SandboxType::None => exec(params, sandbox_policy, stdout_stream.clone()).await, SandboxType::MacosSeatbelt => { let ExecParams { - command, cwd, env, .. + command, + cwd: command_cwd, + env, + .. } = params; let child = spawn_command_under_seatbelt( command, + command_cwd, sandbox_policy, - cwd, + sandbox_cwd, StdioPolicy::RedirectForShellTool, env, ) @@ -108,7 +114,10 @@ pub async fn process_exec_tool_call( } SandboxType::LinuxSeccomp => { let ExecParams { - command, cwd, env, .. + command, + cwd: command_cwd, + env, + .. } = params; let codex_linux_sandbox_exe = codex_linux_sandbox_exe @@ -117,8 +126,9 @@ pub async fn process_exec_tool_call( let child = spawn_command_under_linux_sandbox( codex_linux_sandbox_exe, command, + command_cwd, sandbox_policy, - cwd, + sandbox_cwd, StdioPolicy::RedirectForShellTool, env, ) diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 8c8840c2..264ea747 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -16,21 +16,22 @@ use tokio::process::Child; pub async fn spawn_command_under_linux_sandbox

( codex_linux_sandbox_exe: P, command: Vec, + command_cwd: PathBuf, sandbox_policy: &SandboxPolicy, - cwd: PathBuf, + sandbox_policy_cwd: &Path, stdio_policy: StdioPolicy, env: HashMap, ) -> std::io::Result where P: AsRef, { - let args = create_linux_sandbox_command_args(command, sandbox_policy, &cwd); + let args = create_linux_sandbox_command_args(command, sandbox_policy, sandbox_policy_cwd); let arg0 = Some("codex-linux-sandbox"); spawn_child_async( codex_linux_sandbox_exe.as_ref().to_path_buf(), args, arg0, - cwd, + command_cwd, sandbox_policy, stdio_policy, env, @@ -42,10 +43,13 @@ where fn create_linux_sandbox_command_args( command: Vec, sandbox_policy: &SandboxPolicy, - cwd: &Path, + sandbox_policy_cwd: &Path, ) -> Vec { #[expect(clippy::expect_used)] - let sandbox_policy_cwd = cwd.to_str().expect("cwd must be valid UTF-8").to_string(); + let sandbox_policy_cwd = sandbox_policy_cwd + .to_str() + .expect("cwd must be valid UTF-8") + .to_string(); #[expect(clippy::expect_used)] let sandbox_policy_json = diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index 285e5d56..09e93668 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -18,19 +18,20 @@ const MACOS_PATH_TO_SEATBELT_EXECUTABLE: &str = "/usr/bin/sandbox-exec"; pub async fn spawn_command_under_seatbelt( command: Vec, + command_cwd: PathBuf, sandbox_policy: &SandboxPolicy, - cwd: PathBuf, + sandbox_policy_cwd: &Path, stdio_policy: StdioPolicy, mut env: HashMap, ) -> std::io::Result { - let args = create_seatbelt_command_args(command, sandbox_policy, &cwd); + let args = create_seatbelt_command_args(command, sandbox_policy, sandbox_policy_cwd); let arg0 = None; env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string()); spawn_child_async( PathBuf::from(MACOS_PATH_TO_SEATBELT_EXECUTABLE), args, arg0, - cwd, + command_cwd, sandbox_policy, stdio_policy, env, @@ -41,7 +42,7 @@ pub async fn spawn_command_under_seatbelt( fn create_seatbelt_command_args( command: Vec, sandbox_policy: &SandboxPolicy, - cwd: &Path, + sandbox_policy_cwd: &Path, ) -> Vec { let (file_write_policy, extra_cli_args) = { if sandbox_policy.has_full_disk_write_access() { @@ -51,7 +52,7 @@ fn create_seatbelt_command_args( Vec::::new(), ) } else { - let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); + let writable_roots = sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd); let mut writable_folder_policies: Vec = Vec::new(); let mut cli_args: Vec = Vec::new(); diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index 28cf61dd..3bfef7e1 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -349,6 +349,7 @@ mod tests { }, SandboxType::None, &SandboxPolicy::DangerFullAccess, + temp_home.path(), &None, None, ) @@ -455,6 +456,7 @@ mod macos_tests { }, SandboxType::None, &SandboxPolicy::DangerFullAccess, + temp_home.path(), &None, None, ) diff --git a/codex-rs/core/tests/suite/exec.rs b/codex-rs/core/tests/suite/exec.rs index 9e0cffe6..280917b5 100644 --- a/codex-rs/core/tests/suite/exec.rs +++ b/codex-rs/core/tests/suite/exec.rs @@ -39,7 +39,7 @@ async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>) -> Result&2".to_string(), ]; + let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let params = ExecParams { command: cmd, - cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + cwd: cwd.clone(), timeout_ms: Some(5_000), env: HashMap::new(), with_escalated_permissions: None, @@ -114,6 +117,7 @@ async fn test_exec_stderr_stream_events_echo() { params, SandboxType::None, &policy, + cwd.as_path(), &None, Some(stdout_stream), ) @@ -152,9 +156,10 @@ async fn test_aggregated_output_interleaves_in_order() { "printf 'O1\\n'; sleep 0.01; printf 'E1\\n' 1>&2; sleep 0.01; printf 'O2\\n'; sleep 0.01; printf 'E2\\n' 1>&2".to_string(), ]; + let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let params = ExecParams { command: cmd, - cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + cwd: cwd.clone(), timeout_ms: Some(5_000), env: HashMap::new(), with_escalated_permissions: None, @@ -163,9 +168,16 @@ async fn test_aggregated_output_interleaves_in_order() { let policy = SandboxPolicy::new_read_only_policy(); - let result = process_exec_tool_call(params, SandboxType::None, &policy, &None, None) - .await - .expect("process_exec_tool_call"); + let result = process_exec_tool_call( + params, + SandboxType::None, + &policy, + cwd.as_path(), + &None, + None, + ) + .await + .expect("process_exec_tool_call"); assert_eq!(result.exit_code, 0); assert_eq!(result.stdout.text, "O1\nO2\n"); @@ -182,9 +194,10 @@ async fn test_exec_timeout_returns_partial_output() { "printf 'before\\n'; sleep 2; printf 'after\\n'".to_string(), ]; + let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let params = ExecParams { command: cmd, - cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + cwd: cwd.clone(), timeout_ms: Some(200), env: HashMap::new(), with_escalated_permissions: None, @@ -193,7 +206,15 @@ async fn test_exec_timeout_returns_partial_output() { let policy = SandboxPolicy::new_read_only_policy(); - let result = process_exec_tool_call(params, SandboxType::None, &policy, &None, None).await; + let result = process_exec_tool_call( + params, + SandboxType::None, + &policy, + cwd.as_path(), + &None, + None, + ) + .await; let Err(CodexErr::Sandbox(SandboxErr::Timeout { output })) = result else { panic!("expected timeout error"); diff --git a/codex-rs/core/tests/suite/seatbelt.rs b/codex-rs/core/tests/suite/seatbelt.rs index 69f1d890..78f599d4 100644 --- a/codex-rs/core/tests/suite/seatbelt.rs +++ b/codex-rs/core/tests/suite/seatbelt.rs @@ -171,6 +171,8 @@ async fn python_getpwuid_works_under_seatbelt() { // ReadOnly is sufficient here since we are only exercising user lookup. let policy = SandboxPolicy::ReadOnly; + let command_cwd = std::env::current_dir().expect("getcwd"); + let sandbox_cwd = command_cwd.clone(); let mut child = spawn_command_under_seatbelt( vec![ @@ -179,8 +181,9 @@ async fn python_getpwuid_works_under_seatbelt() { // Print the passwd struct; success implies lookup worked. "import pwd, os; print(pwd.getpwuid(os.getuid()))".to_string(), ], + command_cwd, &policy, - std::env::current_dir().expect("should be able to get current dir"), + sandbox_cwd.as_path(), StdioPolicy::RedirectForShellTool, HashMap::new(), ) @@ -216,13 +219,16 @@ fn create_test_scenario(tmp: &TempDir) -> TestScenario { /// Note that `path` must be absolute. async fn touch(path: &Path, policy: &SandboxPolicy) -> bool { assert!(path.is_absolute(), "Path must be absolute: {path:?}"); + let command_cwd = std::env::current_dir().expect("getcwd"); + let sandbox_cwd = command_cwd.clone(); let mut child = spawn_command_under_seatbelt( vec![ "/usr/bin/touch".to_string(), path.to_string_lossy().to_string(), ], + command_cwd, policy, - std::env::current_dir().expect("should be able to get current dir"), + sandbox_cwd.as_path(), StdioPolicy::RedirectForShellTool, HashMap::new(), ) diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index a98e0a97..5355a0b2 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -4,27 +4,39 @@ use codex_core::spawn::StdioPolicy; use std::collections::HashMap; use std::future::Future; use std::io; +use std::path::Path; use std::path::PathBuf; use std::process::ExitStatus; +use tokio::fs::create_dir_all; use tokio::process::Child; #[cfg(target_os = "macos")] async fn spawn_command_under_sandbox( command: Vec, + command_cwd: PathBuf, sandbox_policy: &SandboxPolicy, - cwd: PathBuf, + sandbox_cwd: &Path, stdio_policy: StdioPolicy, env: HashMap, ) -> std::io::Result { use codex_core::seatbelt::spawn_command_under_seatbelt; - spawn_command_under_seatbelt(command, sandbox_policy, cwd, stdio_policy, env).await + spawn_command_under_seatbelt( + command, + command_cwd, + sandbox_policy, + sandbox_cwd, + stdio_policy, + env, + ) + .await } #[cfg(target_os = "linux")] async fn spawn_command_under_sandbox( command: Vec, + command_cwd: PathBuf, sandbox_policy: &SandboxPolicy, - cwd: PathBuf, + sandbox_cwd: &Path, stdio_policy: StdioPolicy, env: HashMap, ) -> std::io::Result { @@ -33,8 +45,9 @@ async fn spawn_command_under_sandbox( spawn_command_under_linux_sandbox( codex_linux_sandbox_exe, command, + command_cwd, sandbox_policy, - cwd, + sandbox_cwd, stdio_policy, env, ) @@ -74,14 +87,17 @@ if __name__ == '__main__': p.join() "#; + let command_cwd = std::env::current_dir().expect("should be able to get current dir"); + let sandbox_cwd = command_cwd.clone(); let mut child = spawn_command_under_sandbox( vec![ "python3".to_string(), "-c".to_string(), python_code.to_string(), ], + command_cwd, &policy, - std::env::current_dir().expect("should be able to get current dir"), + sandbox_cwd.as_path(), StdioPolicy::Inherit, HashMap::new(), ) @@ -92,6 +108,88 @@ if __name__ == '__main__': assert!(status.success(), "python exited with {status:?}"); } +#[tokio::test] +async fn sandbox_distinguishes_command_and_policy_cwds() { + let temp = tempfile::tempdir().expect("should be able to create temp dir"); + let sandbox_root = temp.path().join("sandbox"); + let command_root = temp.path().join("command"); + create_dir_all(&sandbox_root).await.expect("mkdir"); + create_dir_all(&command_root).await.expect("mkdir"); + let canonical_sandbox_root = tokio::fs::canonicalize(&sandbox_root) + .await + .expect("canonicalize sandbox root"); + let canonical_allowed_path = canonical_sandbox_root.join("allowed.txt"); + + let disallowed_path = command_root.join("forbidden.txt"); + + // Note writable_roots is empty: verify that `canonical_allowed_path` is + // writable only because it is under the sandbox policy cwd, not because it + // is under a writable root. + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + // Attempt to write inside the command cwd, which is outside of the sandbox policy cwd. + let mut child = spawn_command_under_sandbox( + vec![ + "bash".to_string(), + "-lc".to_string(), + "echo forbidden > forbidden.txt".to_string(), + ], + command_root.clone(), + &policy, + canonical_sandbox_root.as_path(), + StdioPolicy::Inherit, + HashMap::new(), + ) + .await + .expect("should spawn command writing to forbidden path"); + + let status = child + .wait() + .await + .expect("should wait for forbidden command"); + assert!( + !status.success(), + "sandbox unexpectedly allowed writing to command cwd: {status:?}" + ); + let forbidden_exists = tokio::fs::try_exists(&disallowed_path) + .await + .expect("try_exists failed"); + assert!( + !forbidden_exists, + "forbidden path should not have been created" + ); + + // Writing to the sandbox policy cwd after changing directories into it should succeed. + let mut child = spawn_command_under_sandbox( + vec![ + "/usr/bin/touch".to_string(), + canonical_allowed_path.to_string_lossy().into_owned(), + ], + command_root, + &policy, + canonical_sandbox_root.as_path(), + StdioPolicy::Inherit, + HashMap::new(), + ) + .await + .expect("should spawn command writing to sandbox root"); + + let status = child.wait().await.expect("should wait for allowed command"); + assert!( + status.success(), + "sandbox blocked allowed write: {status:?}" + ); + let allowed_exists = tokio::fs::try_exists(&canonical_allowed_path) + .await + .expect("try_exists allowed failed"); + assert!(allowed_exists, "allowed path should exist"); +} + fn unix_sock_body() { unsafe { let mut fds = [0i32; 2]; @@ -200,10 +298,13 @@ where cmds.push(test_selector.into()); // Your existing launcher: + let command_cwd = std::env::current_dir().expect("should be able to get current dir"); + let sandbox_cwd = command_cwd.clone(); let mut child = spawn_command_under_sandbox( cmds, + command_cwd, policy, - std::env::current_dir().expect("should be able to get current dir"), + sandbox_cwd.as_path(), stdio_policy, HashMap::from([("IN_SANDBOX".into(), "1".into())]), ) diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index fddb43c3..ea4930fa 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -35,9 +35,11 @@ fn create_env_from_core_vars() -> HashMap { #[expect(clippy::print_stdout, clippy::expect_used, clippy::unwrap_used)] async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) { + let cwd = std::env::current_dir().expect("cwd should exist"); + let sandbox_cwd = cwd.clone(); let params = ExecParams { command: cmd.iter().map(|elm| elm.to_string()).collect(), - cwd: std::env::current_dir().expect("cwd should exist"), + cwd, timeout_ms: Some(timeout_ms), env: create_env_from_core_vars(), with_escalated_permissions: None, @@ -59,6 +61,7 @@ async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) { params, SandboxType::LinuxSeccomp, &sandbox_policy, + sandbox_cwd.as_path(), &codex_linux_sandbox_exe, None, ) @@ -133,6 +136,7 @@ async fn test_timeout() { #[expect(clippy::expect_used)] async fn assert_network_blocked(cmd: &[&str]) { let cwd = std::env::current_dir().expect("cwd should exist"); + let sandbox_cwd = cwd.clone(); let params = ExecParams { command: cmd.iter().map(|s| s.to_string()).collect(), cwd, @@ -151,6 +155,7 @@ async fn assert_network_blocked(cmd: &[&str]) { params, SandboxType::LinuxSeccomp, &sandbox_policy, + sandbox_cwd.as_path(), &codex_linux_sandbox_exe, None, ) diff --git a/codex-rs/mcp-server/src/codex_message_processor.rs b/codex-rs/mcp-server/src/codex_message_processor.rs index af0ffefe..1d4a1b0f 100644 --- a/codex-rs/mcp-server/src/codex_message_processor.rs +++ b/codex-rs/mcp-server/src/codex_message_processor.rs @@ -589,12 +589,14 @@ impl CodexMessageProcessor { let codex_linux_sandbox_exe = self.config.codex_linux_sandbox_exe.clone(); let outgoing = self.outgoing.clone(); let req_id = request_id; + let sandbox_cwd = self.config.cwd.clone(); tokio::spawn(async move { match codex_core::exec::process_exec_tool_call( exec_params, sandbox_type, &effective_policy, + sandbox_cwd.as_path(), &codex_linux_sandbox_exe, None, )