diff --git a/codex-rs/common/src/sandbox_summary.rs b/codex-rs/common/src/sandbox_summary.rs index e0e309a9d9..7c49bac59d 100644 --- a/codex-rs/common/src/sandbox_summary.rs +++ b/codex-rs/common/src/sandbox_summary.rs @@ -7,22 +7,25 @@ pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String { SandboxPolicy::WorkspaceWrite { writable_roots, network_access, - include_default_writable_roots, + exclude_tmpdir_env_var, + exclude_slash_tmp, } => { let mut summary = "workspace-write".to_string(); - if !writable_roots.is_empty() { - summary.push_str(&format!( - " [{}]", - writable_roots - .iter() - .map(|p| p.to_string_lossy()) - .collect::>() - .join(", ") - )); + + let mut writable_entries = Vec::::new(); + if !*exclude_slash_tmp { + writable_entries.push("/tmp".to_string()); } - if !*include_default_writable_roots { - summary.push_str(" (exact writable roots)"); + if !*exclude_tmpdir_env_var { + writable_entries.push("$TMPDIR".to_string()); } + writable_entries.extend( + writable_roots + .iter() + .map(|p| p.to_string_lossy().to_string()), + ); + + summary.push_str(&format!(" [{}]", writable_entries.join(", "))); if *network_access { summary.push_str(" (network access enabled)"); } diff --git a/codex-rs/config.md b/codex-rs/config.md index f93a35ebca..e044684426 100644 --- a/codex-rs/config.md +++ b/codex-rs/config.md @@ -275,9 +275,12 @@ sandbox_mode = "workspace-write" # Extra settings that only apply when `sandbox = "workspace-write"`. [sandbox_workspace_write] -# By default, only the cwd for the Codex session will be writable (and $TMPDIR -# on macOS), but you can specify additional writable folders in this array. -writable_roots = ["/tmp"] +# By default, the cwd for the Codex session will be writable as well as $TMPDIR +# if set) and /tmp (if it exists). Setting the respective options to `true` +# will override those defaults. +exclude_tmpdir_env_var = false +exclude_slash_tmp = false + # Allow the command being run inside the sandbox to make outbound network # requests. Disabled by default. network_access = false diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 63a2e5949f..a2e0618a7f 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -361,10 +361,16 @@ impl ConfigToml { match resolved_sandbox_mode { SandboxMode::ReadOnly => SandboxPolicy::new_read_only_policy(), SandboxMode::WorkspaceWrite => match self.sandbox_workspace_write.as_ref() { - Some(s) => SandboxPolicy::WorkspaceWrite { - writable_roots: s.writable_roots.clone(), - network_access: s.network_access, - include_default_writable_roots: true, + Some(SandboxWorkplaceWrite { + writable_roots, + network_access, + exclude_tmpdir_env_var, + exclude_slash_tmp, + }) => SandboxPolicy::WorkspaceWrite { + writable_roots: writable_roots.clone(), + network_access: *network_access, + exclude_tmpdir_env_var: *exclude_tmpdir_env_var, + exclude_slash_tmp: *exclude_slash_tmp, }, None => SandboxPolicy::new_workspace_write_policy(), }, @@ -745,8 +751,10 @@ sandbox_mode = "workspace-write" [sandbox_workspace_write] writable_roots = [ - "/tmp", + "/my/workspace", ] +exclude_tmpdir_env_var = true +exclude_slash_tmp = true "#; let sandbox_workspace_write_cfg = toml::from_str::(sandbox_workspace_write) @@ -754,9 +762,10 @@ writable_roots = [ let sandbox_mode_override = None; assert_eq!( SandboxPolicy::WorkspaceWrite { - writable_roots: vec![PathBuf::from("/tmp")], + writable_roots: vec![PathBuf::from("/my/workspace")], network_access: false, - include_default_writable_roots: true, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, }, sandbox_workspace_write_cfg.derive_sandbox_policy(sandbox_mode_override) ); diff --git a/codex-rs/core/src/config_types.rs b/codex-rs/core/src/config_types.rs index 9bf0d483e1..a81c20502b 100644 --- a/codex-rs/core/src/config_types.rs +++ b/codex-rs/core/src/config_types.rs @@ -98,6 +98,10 @@ pub struct SandboxWorkplaceWrite { pub writable_roots: Vec, #[serde(default)] pub network_access: bool, + #[serde(default)] + pub exclude_tmpdir_env_var: bool, + #[serde(default)] + pub exclude_slash_tmp: bool, } #[derive(Deserialize, Debug, Clone, PartialEq, Default)] diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index 052806dd97..f6088786b3 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -185,11 +185,16 @@ pub enum SandboxPolicy { #[serde(default)] network_access: bool, - /// When set to `true`, will include defaults like the current working - /// directory and TMPDIR (on macOS). When `false`, only `writable_roots` - /// are used. (Mainly used for testing.) - #[serde(default = "default_true")] - include_default_writable_roots: bool, + /// When set to `true`, will NOT include the per-user `TMPDIR` + /// environment variable among the default writable roots. Defaults to + /// `false`. + #[serde(default)] + exclude_tmpdir_env_var: bool, + + /// When set to `true`, will NOT include the `/tmp` among the default + /// writable roots on UNIX. Defaults to `false`. + #[serde(default)] + exclude_slash_tmp: bool, }, } @@ -203,10 +208,6 @@ pub struct WritableRoot { pub read_only_subpaths: Vec, } -fn default_true() -> bool { - true -} - impl FromStr for SandboxPolicy { type Err = serde_json::Error; @@ -228,7 +229,8 @@ impl SandboxPolicy { SandboxPolicy::WorkspaceWrite { writable_roots: vec![], network_access: false, - include_default_writable_roots: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, } } @@ -263,27 +265,40 @@ impl SandboxPolicy { SandboxPolicy::ReadOnly => Vec::new(), SandboxPolicy::WorkspaceWrite { writable_roots, - include_default_writable_roots, - .. + exclude_tmpdir_env_var, + exclude_slash_tmp, + network_access: _, } => { // Start from explicitly configured writable roots. let mut roots: Vec = writable_roots.clone(); - // Optionally include defaults (cwd and TMPDIR on macOS). - if *include_default_writable_roots { - roots.push(cwd.to_path_buf()); + // Always include defaults: cwd, /tmp (if present on Unix), and + // on macOS, the per-user TMPDIR unless explicitly excluded. + roots.push(cwd.to_path_buf()); - // Also include the per-user tmp dir on macOS. - // Note this is added dynamically rather than storing it in - // `writable_roots` because `writable_roots` contains only static - // values deserialized from the config file. - if cfg!(target_os = "macos") { - if let Some(tmpdir) = std::env::var_os("TMPDIR") { - roots.push(PathBuf::from(tmpdir)); - } + // Include /tmp on Unix unless explicitly excluded. + if cfg!(unix) && !exclude_slash_tmp { + let slash_tmp = PathBuf::from("/tmp"); + if slash_tmp.is_dir() { + roots.push(slash_tmp); } } + // Include $TMPDIR unless explicitly excluded. On macOS, TMPDIR + // is per-user on macOS, so writes to TMPDIR should not be + // readable by other users on the system. + // + // By comparison, TMPDIR is not guaranteed to be defined on + // Linux or Windows, but supporting it here gives users a way + // to provide the model with their own temporary directory + // without having to hardcode it in the config. + if !*exclude_tmpdir_env_var + && let Some(tmpdir) = std::env::var_os("TMPDIR") + && !tmpdir.is_empty() + { + roots.push(PathBuf::from(tmpdir)); + } + // For each root, compute subpaths that should remain read-only. roots .into_iter() diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index 0364840b1a..3ed352aa39 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -134,6 +134,11 @@ mod tests { #[test] fn create_seatbelt_args_with_read_only_git_subpath() { + if cfg!(target_os = "windows") { + // /tmp does not exist on Windows, so skip this test. + return; + } + // Create a temporary workspace with two writable roots: one containing // a top-level .git directory and one without it. let tmp = TempDir::new().expect("tempdir"); @@ -145,12 +150,13 @@ mod tests { root_without_git_canon, } = populate_tmpdir(tmp.path()); - // Build a policy that only includes the two test roots as writable and - // does not automatically include defaults like cwd or TMPDIR. + // Build a policy that includes the two test roots as writable and also + // includes default writable roots (cwd, /tmp, and possibly TMPDIR). let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![root_with_git.clone(), root_without_git.clone()], network_access: false, - include_default_writable_roots: false, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, }; let args = create_seatbelt_command_args( @@ -159,22 +165,50 @@ mod tests { tmp.path(), ); + // Compute dynamic expected bits for defaults. + let cwd_canon = tmp + .path() + .canonicalize() + .expect("canonicalize tmp cwd") + .to_string_lossy() + .to_string(); + let slash_tmp_canon = PathBuf::from("/tmp") + .canonicalize() + .expect("canonicalize /tmp") + .to_string_lossy() + .to_string(); + let tmpdir_env_var = if cfg!(target_os = "macos") { + std::env::var("TMPDIR") + .ok() + .map(PathBuf::from) + .and_then(|p| p.canonicalize().ok()) + .map(|p| p.to_string_lossy().to_string()) + } else { + None + }; + let tmpdir_policy_entry = if tmpdir_env_var.is_some() { + " (subpath (param \"WRITABLE_ROOT_4\"))" + } else { + "" + }; + // Build the expected policy text using a raw string for readability. // Note that the policy includes: // - the base policy, // - read-only access to the filesystem, - // - write access to WRITABLE_ROOT_0 (but not its .git) and WRITABLE_ROOT_1. + // - write access to WRITABLE_ROOT_0 (but not its .git), WRITABLE_ROOT_1, + // WRITABLE_ROOT_2 (cwd), WRITABLE_ROOT_3 (/tmp), and optionally WRITABLE_ROOT_4 (TMPDIR). let expected_policy = format!( r#"{MACOS_SEATBELT_BASE_POLICY} ; allow read-only file operations (allow file-read*) (allow file-write* -(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) ) (subpath (param "WRITABLE_ROOT_1")) +(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) ) (subpath (param "WRITABLE_ROOT_1")) (subpath (param "WRITABLE_ROOT_2")) (subpath (param "WRITABLE_ROOT_3")){tmpdir_policy_entry} ) "#, ); - let expected_args = vec![ + let mut expected_args = vec![ "-p".to_string(), expected_policy, format!( @@ -189,16 +223,30 @@ mod tests { "-DWRITABLE_ROOT_1={}", root_without_git_canon.to_string_lossy() ), + format!("-DWRITABLE_ROOT_2={cwd_canon}"), + format!("-DWRITABLE_ROOT_3={slash_tmp_canon}"), + ]; + + if let Some(p) = tmpdir_env_var { + expected_args.push(format!("-DWRITABLE_ROOT_4={p}")); + } + + expected_args.extend(vec![ "--".to_string(), "/bin/echo".to_string(), "hello".to_string(), - ]; + ]); - assert_eq!(args, expected_args); + assert_eq!(expected_args, args); } #[test] fn create_seatbelt_args_for_cwd_as_git_repo() { + if cfg!(target_os = "windows") { + // /tmp does not exist on Windows, so skip this test. + return; + } + // Create a temporary workspace with two writable roots: one containing // a top-level .git directory and one without it. let tmp = TempDir::new().expect("tempdir"); @@ -215,7 +263,8 @@ mod tests { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], network_access: false, - include_default_writable_roots: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, }; let args = create_seatbelt_command_args( @@ -233,8 +282,9 @@ mod tests { } else { None }; + let tempdir_policy_entry = if tmpdir_env_var.is_some() { - " (subpath (param \"WRITABLE_ROOT_1\"))" + " (subpath (param \"WRITABLE_ROOT_2\"))" } else { "" }; @@ -249,7 +299,7 @@ mod tests { ; allow read-only file operations (allow file-read*) (allow file-write* -(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) ){tempdir_policy_entry} +(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) ) (subpath (param "WRITABLE_ROOT_1")){tempdir_policy_entry} ) "#, ); @@ -265,10 +315,17 @@ mod tests { "-DWRITABLE_ROOT_0_RO_0={}", root_with_git_git_canon.to_string_lossy() ), + format!( + "-DWRITABLE_ROOT_1={}", + PathBuf::from("/tmp") + .canonicalize() + .expect("canonicalize /tmp") + .to_string_lossy() + ), ]; if let Some(p) = tmpdir_env_var { - expected_args.push(format!("-DWRITABLE_ROOT_1={p}")); + expected_args.push(format!("-DWRITABLE_ROOT_2={p}")); } expected_args.extend(vec![ @@ -277,7 +334,7 @@ mod tests { "hello".to_string(), ]); - assert_eq!(args, expected_args); + assert_eq!(expected_args, args); } struct PopulatedTmp { diff --git a/codex-rs/core/tests/sandbox.rs b/codex-rs/core/tests/sandbox.rs index e85156bf05..ae5bdc44a6 100644 --- a/codex-rs/core/tests/sandbox.rs +++ b/codex-rs/core/tests/sandbox.rs @@ -76,7 +76,8 @@ async fn if_parent_of_repo_is_writable_then_dot_git_folder_is_writable() { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![test_scenario.repo_parent.clone()], network_access: false, - include_default_writable_roots: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, }; test_scenario @@ -101,7 +102,8 @@ async fn if_git_repo_is_writable_root_then_dot_git_folder_is_read_only() { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![test_scenario.repo_root.clone()], network_access: false, - include_default_writable_roots: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, }; test_scenario diff --git a/codex-rs/linux-sandbox/tests/landlock.rs b/codex-rs/linux-sandbox/tests/landlock.rs index 041e64e208..96298c6563 100644 --- a/codex-rs/linux-sandbox/tests/landlock.rs +++ b/codex-rs/linux-sandbox/tests/landlock.rs @@ -51,7 +51,11 @@ async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) { let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: writable_roots.to_vec(), network_access: false, - include_default_writable_roots: true, + // Exclude tmp-related folders from writable roots because we need a + // folder that is writable by tests but that we intentionally disallow + // writing to in the sandbox. + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, }; let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox"); let codex_linux_sandbox_exe = Some(PathBuf::from(sandbox_program));