diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 4c1dd7c6d6..fb07fc3a33 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -832,6 +832,7 @@ fn permissions_profiles_allow_tmpdir_write_with_read_only_legacy_fallback() -> s Ok(()) } +#[cfg(unix)] #[test] fn permissions_profiles_allow_slash_tmp_write_with_read_only_legacy_fallback() -> std::io::Result<()> { diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index d2ac6fcbef..5f01adc523 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -86,16 +86,16 @@ pub enum FileSystemSpecialPath { /// Config-facing `:tmpdir` special path. /// /// This is the broader restricted-policy temp bundle used by filesystem - /// permission profiles. On macOS it expands to `$TMPDIR`, `/tmp`, and - /// `/tmp`'s canonical target so callers can grant write access to the tmp - /// namespace without depending on a single alias. + /// permission profiles. On Unix it expands to the temp env roots plus + /// `/tmp` and `/tmp`'s canonical target. On Windows it expands to the + /// platform temp env roots. Tmpdir, - /// Legacy `$TMPDIR`-only special path used when bridging old + /// Legacy env-backed temp roots used when bridging old /// `SandboxPolicy::WorkspaceWrite` semantics into split filesystem policy. /// /// Keep this narrower than [`FileSystemSpecialPath::Tmpdir`] so existing /// `exclude_tmpdir_env_var = false` behavior does not silently widen to the - /// full temp bundle. + /// full temp bundle on Unix. TmpdirEnvVar, SlashTmp, /// WARNING: `:special_path` tokens are part of config compatibility. @@ -1061,7 +1061,7 @@ fn resolve_file_system_special_paths( } } FileSystemSpecialPath::Tmpdir => resolve_tmpdir_paths(), - FileSystemSpecialPath::TmpdirEnvVar => resolve_tmpdir_env_var_path().into_iter().collect(), + FileSystemSpecialPath::TmpdirEnvVar => resolve_temp_env_paths(), FileSystemSpecialPath::SlashTmp => resolve_slash_tmp_path().into_iter().collect(), } } @@ -1074,16 +1074,13 @@ fn resolve_cwd_independent_special_paths(value: &FileSystemSpecialPath) -> Vec Vec::new(), FileSystemSpecialPath::Tmpdir => resolve_tmpdir_paths(), - FileSystemSpecialPath::TmpdirEnvVar => resolve_tmpdir_env_var_path().into_iter().collect(), + FileSystemSpecialPath::TmpdirEnvVar => resolve_temp_env_paths(), FileSystemSpecialPath::SlashTmp => resolve_slash_tmp_path().into_iter().collect(), } } fn resolve_tmpdir_paths() -> Vec { - let mut paths = Vec::new(); - if let Some(tmpdir) = resolve_tmpdir_env_var_path() { - paths.push(tmpdir); - } + let mut paths = resolve_temp_env_paths(); if let Some(slash_tmp) = resolve_slash_tmp_path() { paths.push(slash_tmp.clone()); if let Ok(realpath) = slash_tmp.as_path().canonicalize() @@ -1095,22 +1092,50 @@ fn resolve_tmpdir_paths() -> Vec { dedup_absolute_paths(paths, /*normalize_effective_paths*/ false) } -fn resolve_tmpdir_env_var_path() -> Option { - let tmpdir = std::env::var_os("TMPDIR")?; - if tmpdir.is_empty() { +pub(crate) fn resolve_temp_env_paths() -> Vec { + dedup_absolute_paths( + temp_env_var_keys() + .iter() + .filter_map(|key| resolve_temp_env_path(key)) + .collect(), + /*normalize_effective_paths*/ false, + ) +} + +#[cfg(windows)] +const fn temp_env_var_keys() -> &'static [&'static str] { + &["TEMP", "TMP"] +} + +#[cfg(not(windows))] +const fn temp_env_var_keys() -> &'static [&'static str] { + &["TMPDIR"] +} + +fn resolve_temp_env_path(key: &str) -> Option { + let value = std::env::var_os(key)?; + if value.is_empty() { None } else { - AbsolutePathBuf::from_absolute_path(PathBuf::from(tmpdir)).ok() + AbsolutePathBuf::from_absolute_path(PathBuf::from(value)).ok() } } fn resolve_slash_tmp_path() -> Option { - #[allow(clippy::expect_used)] - let slash_tmp = AbsolutePathBuf::from_absolute_path("/tmp").expect("/tmp is absolute"); - if !slash_tmp.as_path().is_dir() { - return None; + #[cfg(not(unix))] + { + None + } + + #[cfg(unix)] + { + #[allow(clippy::expect_used)] + let slash_tmp = AbsolutePathBuf::from_absolute_path("/tmp").expect("/tmp is absolute"); + if !slash_tmp.as_path().is_dir() { + return None; + } + Some(slash_tmp) } - Some(slash_tmp) } fn dedup_absolute_paths( diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index c6818726b5..d8d11f6608 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -79,6 +79,7 @@ pub use crate::permissions::FileSystemSandboxKind; pub use crate::permissions::FileSystemSandboxPolicy; pub use crate::permissions::FileSystemSpecialPath; pub use crate::permissions::NetworkSandboxPolicy; +use crate::permissions::resolve_temp_env_paths; pub use crate::request_permissions::RequestPermissionsArgs; pub use crate::request_user_input::RequestUserInputEvent; @@ -1047,28 +1048,12 @@ impl SandboxPolicy { } } - // 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() - { - match AbsolutePathBuf::from_absolute_path(PathBuf::from(&tmpdir)) { - Ok(tmpdir_path) => { - roots.push(tmpdir_path); - } - Err(e) => { - error!( - "Ignoring invalid TMPDIR value {tmpdir:?} for sandbox writable root: {e}", - ); - } - } + // Include platform temp env roots unless explicitly excluded. + // On Unix, this keeps the legacy TMPDIR behavior. On Windows, + // this picks up TEMP/TMP so legacy workspace-write policies + // keep matching the host temp directory. + if !exclude_tmpdir_env_var { + roots.extend(resolve_temp_env_paths()); } // For each root, compute subpaths that should remain read-only.