diff --git a/codex-rs/common/src/sandbox_summary.rs b/codex-rs/common/src/sandbox_summary.rs index e0e309a9..66e00cd4 100644 --- a/codex-rs/common/src/sandbox_summary.rs +++ b/codex-rs/common/src/sandbox_summary.rs @@ -7,22 +7,26 @@ 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(); + writable_entries.push("workdir".to_string()); + 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 f93a35eb..e0446844 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 63a2e594..a2e0618a 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 9bf0d483..a81c2050 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 052806dd..e61fc0c3 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, 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 0364840b..ff9dbaa7 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"); @@ -144,19 +149,21 @@ mod tests { root_with_git_git_canon, root_without_git_canon, } = populate_tmpdir(tmp.path()); + let cwd = tmp.path().join("cwd"); // Build a policy that only includes the two test roots as writable and - // does not automatically include defaults like cwd or TMPDIR. + // does not automatically include defaults TMPDIR or /tmp. 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: true, + exclude_slash_tmp: true, }; let args = create_seatbelt_command_args( vec!["/bin/echo".to_string(), "hello".to_string()], &policy, - tmp.path(), + &cwd, ); // Build the expected policy text using a raw string for readability. @@ -169,12 +176,12 @@ 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"))) ) (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")) ) "#, ); - let expected_args = vec![ + let mut expected_args = vec![ "-p".to_string(), expected_policy, format!( @@ -189,16 +196,25 @@ mod tests { "-DWRITABLE_ROOT_1={}", root_without_git_canon.to_string_lossy() ), + format!("-DWRITABLE_ROOT_2={}", cwd.to_string_lossy()), + ]; + + 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 +231,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( @@ -224,17 +241,14 @@ mod tests { root_with_git.as_path(), ); - 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_env_var = std::env::var("TMPDIR") + .ok() + .map(PathBuf::from) + .and_then(|p| p.canonicalize().ok()) + .map(|p| p.to_string_lossy().to_string()); + let tempdir_policy_entry = if tmpdir_env_var.is_some() { - " (subpath (param \"WRITABLE_ROOT_1\"))" + r#" (subpath (param "WRITABLE_ROOT_2"))"# } else { "" }; @@ -249,7 +263,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 +279,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 +298,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 e85156bf..ae5bdc44 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 041e64e2..96298c65 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));