changes

changes

Update codex-rs/core/src/config/mod.rs

Co-authored-by: Michael Bolin <mbolin@openai.com>

changes

codex-rs/core/src/config/mod.rs

@@ -97,6 +97,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;
@@ -1917,29 +1918,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 {
@@ -2292,10 +2270,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)?;
             }
@@ -2646,6 +2622,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 {
@@ -2656,6 +2637,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

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<AbsolutePathBuf> {
+    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;

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(())
+}

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);