diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 48bde3f17..17abe55c4 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -98,6 +98,7 @@ use std::path::Path; use std::path::PathBuf; use crate::config::permissions::compile_permission_profile; +use crate::config::permissions::get_readable_roots_required_for_codex_runtime; use crate::config::permissions::network_proxy_config_from_profile_network; use crate::config::profile::ConfigProfile; use codex_network_proxy::NetworkProxyConfig; @@ -1918,29 +1919,6 @@ fn resolve_permission_config_syntax( }) } -fn add_additional_file_system_writes( - file_system_sandbox_policy: &mut FileSystemSandboxPolicy, - additional_writable_roots: &[AbsolutePathBuf], -) { - for path in additional_writable_roots { - let exists = file_system_sandbox_policy.entries.iter().any(|entry| { - matches!( - &entry.path, - codex_protocol::permissions::FileSystemPath::Path { path: existing } - if existing == path && entry.access == codex_protocol::permissions::FileSystemAccessMode::Write - ) - }); - if !exists { - file_system_sandbox_policy.entries.push( - codex_protocol::permissions::FileSystemSandboxEntry { - path: codex_protocol::permissions::FileSystemPath::Path { path: path.clone() }, - access: codex_protocol::permissions::FileSystemAccessMode::Write, - }, - ); - } - } -} - /// Optional overrides for user configuration (e.g., from CLI flags). #[derive(Default, Debug, Clone)] pub struct ConfigOverrides { @@ -2309,10 +2287,8 @@ impl Config { let mut sandbox_policy = file_system_sandbox_policy .to_legacy_sandbox_policy(network_sandbox_policy, &resolved_cwd)?; if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) { - add_additional_file_system_writes( - &mut file_system_sandbox_policy, - &additional_writable_roots, - ); + file_system_sandbox_policy = file_system_sandbox_policy + .with_additional_writable_roots(&resolved_cwd, &additional_writable_roots); sandbox_policy = file_system_sandbox_policy .to_legacy_sandbox_policy(network_sandbox_policy, &resolved_cwd)?; } @@ -2663,6 +2639,11 @@ impl Config { } else { network.enabled().then_some(network) }; + let helper_readable_roots = get_readable_roots_required_for_codex_runtime( + &codex_home, + zsh_path.as_ref(), + main_execve_wrapper_exe.as_ref(), + ); let effective_sandbox_policy = constrained_sandbox_policy.value.get().clone(); let effective_file_system_sandbox_policy = if effective_sandbox_policy == original_sandbox_policy { @@ -2673,6 +2654,8 @@ impl Config { &resolved_cwd, ) }; + let effective_file_system_sandbox_policy = effective_file_system_sandbox_policy + .with_additional_readable_roots(&resolved_cwd, &helper_readable_roots); let effective_network_sandbox_policy = if effective_sandbox_policy == original_sandbox_policy { network_sandbox_policy diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index 4d1027efe..759c269b7 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -190,6 +190,37 @@ pub(crate) fn compile_permission_profile( )) } +/// Returns a list of paths that must be readable by shell tools in order +/// for Codex to function. These should always be added to the +/// `FileSystemSandboxPolicy` for a thread. +pub(crate) fn get_readable_roots_required_for_codex_runtime( + codex_home: &Path, + zsh_path: Option<&PathBuf>, + main_execve_wrapper_exe: Option<&PathBuf>, +) -> Vec { + let arg0_root = AbsolutePathBuf::from_absolute_path(codex_home.join("tmp").join("arg0")).ok(); + let zsh_path = zsh_path.and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()); + let execve_wrapper_root = main_execve_wrapper_exe.and_then(|path| { + let path = AbsolutePathBuf::from_absolute_path(path).ok()?; + if let Some(arg0_root) = arg0_root.as_ref() + && path.as_path().starts_with(arg0_root.as_path()) + { + path.parent() + } else { + Some(path) + } + }); + + let mut readable_roots = Vec::new(); + if let Some(zsh_path) = zsh_path { + readable_roots.push(zsh_path); + } + if let Some(execve_wrapper_root) = execve_wrapper_root { + readable_roots.push(execve_wrapper_root); + } + readable_roots +} + fn compile_network_sandbox_policy(network: Option<&NetworkToml>) -> NetworkSandboxPolicy { let Some(network) = network else { return NetworkSandboxPolicy::Restricted; diff --git a/codex-rs/core/src/config/permissions_tests.rs b/codex-rs/core/src/config/permissions_tests.rs index 036c8450c..b90b903e8 100644 --- a/codex-rs/core/src/config/permissions_tests.rs +++ b/codex-rs/core/src/config/permissions_tests.rs @@ -1,5 +1,11 @@ use super::*; +use crate::config::Config; +use crate::config::ConfigOverrides; +use crate::config::ConfigToml; +use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; +use std::collections::BTreeMap; +use tempfile::TempDir; #[test] fn normalize_absolute_path_for_platform_simplifies_windows_verbatim_paths() { @@ -7,3 +13,66 @@ fn normalize_absolute_path_for_platform_simplifies_windows_verbatim_paths() { normalize_absolute_path_for_platform(r"\\?\D:\c\x\worktrees\2508\swift-base", true); assert_eq!(parsed, PathBuf::from(r"D:\c\x\worktrees\2508\swift-base")); } + +#[test] +fn restricted_read_implicitly_allows_helper_executables() -> std::io::Result<()> { + let temp_dir = TempDir::new()?; + let cwd = temp_dir.path().join("workspace"); + let codex_home = temp_dir.path().join(".codex"); + let zsh_path = temp_dir.path().join("runtime").join("zsh"); + let arg0_root = codex_home.join("tmp").join("arg0"); + let allowed_arg0_dir = arg0_root.join("codex-arg0-session"); + let sibling_arg0_dir = arg0_root.join("codex-arg0-other-session"); + let execve_wrapper = allowed_arg0_dir.join("codex-execve-wrapper"); + std::fs::create_dir_all(&cwd)?; + std::fs::create_dir_all(zsh_path.parent().expect("zsh path should have parent"))?; + std::fs::create_dir_all(&allowed_arg0_dir)?; + std::fs::create_dir_all(&sibling_arg0_dir)?; + std::fs::write(&zsh_path, "")?; + std::fs::write(&execve_wrapper, "")?; + + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + default_permissions: Some("workspace".to_string()), + permissions: Some(PermissionsToml { + entries: BTreeMap::from([( + "workspace".to_string(), + PermissionProfileToml { + filesystem: Some(FilesystemPermissionsToml { + entries: BTreeMap::new(), + }), + network: None, + }, + )]), + }), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.clone()), + zsh_path: Some(zsh_path.clone()), + main_execve_wrapper_exe: Some(execve_wrapper), + ..Default::default() + }, + codex_home, + )?; + + let expected_zsh = AbsolutePathBuf::try_from(zsh_path)?; + let expected_allowed_arg0_dir = AbsolutePathBuf::try_from(allowed_arg0_dir)?; + let expected_sibling_arg0_dir = AbsolutePathBuf::try_from(sibling_arg0_dir)?; + let policy = &config.permissions.file_system_sandbox_policy; + + assert!( + policy.can_read_path_with_cwd(expected_zsh.as_path(), &cwd), + "expected zsh helper path to be readable, policy: {policy:?}" + ); + assert!( + policy.can_read_path_with_cwd(expected_allowed_arg0_dir.as_path(), &cwd), + "expected active arg0 helper dir to be readable, policy: {policy:?}" + ); + assert!( + !policy.can_read_path_with_cwd(expected_sibling_arg0_dir.as_path(), &cwd), + "expected sibling arg0 helper dir to remain unreadable, policy: {policy:?}" + ); + + Ok(()) +} diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 590fbd1fe..0a5b33702 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -347,6 +347,48 @@ impl FileSystemSandboxPolicy { self.resolve_access_with_cwd(path, cwd).can_write() } + pub fn with_additional_readable_roots( + mut self, + cwd: &Path, + additional_readable_roots: &[AbsolutePathBuf], + ) -> Self { + if self.has_full_disk_read_access() { + return self; + } + + for path in additional_readable_roots { + if self.can_read_path_with_cwd(path.as_path(), cwd) { + continue; + } + + self.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { path: path.clone() }, + access: FileSystemAccessMode::Read, + }); + } + + self + } + + pub fn with_additional_writable_roots( + mut self, + cwd: &Path, + additional_writable_roots: &[AbsolutePathBuf], + ) -> Self { + for path in additional_writable_roots { + if self.can_write_path_with_cwd(path.as_path(), cwd) { + continue; + } + + self.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { path: path.clone() }, + access: FileSystemAccessMode::Write, + }); + } + + self + } + pub fn needs_direct_runtime_enforcement( &self, network_policy: NetworkSandboxPolicy, @@ -1782,6 +1824,74 @@ mod tests { ); } + #[test] + fn with_additional_readable_roots_skips_existing_effective_access() { + let cwd = TempDir::new().expect("tempdir"); + let cwd_root = AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute cwd"); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Read, + }]); + + let actual = policy + .clone() + .with_additional_readable_roots(cwd.path(), std::slice::from_ref(&cwd_root)); + + assert_eq!(actual, policy); + } + + #[test] + fn with_additional_writable_roots_skips_existing_effective_access() { + let cwd = TempDir::new().expect("tempdir"); + let cwd_root = AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute cwd"); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }]); + + let actual = policy + .clone() + .with_additional_writable_roots(cwd.path(), std::slice::from_ref(&cwd_root)); + + assert_eq!(actual, policy); + } + + #[test] + fn with_additional_writable_roots_adds_new_root() { + let temp_dir = TempDir::new().expect("tempdir"); + let cwd = temp_dir.path().join("workspace"); + let extra = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("extra")) + .expect("resolve extra root"); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }]); + + let actual = policy.with_additional_writable_roots(&cwd, std::slice::from_ref(&extra)); + + assert_eq!( + actual, + FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: extra }, + access: FileSystemAccessMode::Write, + }, + ]) + ); + } + #[test] fn file_system_access_mode_orders_by_conflict_precedence() { assert!(FileSystemAccessMode::Write > FileSystemAccessMode::Read);