protocol: keep root carveouts sandboxed

codex-rs/core/src/exec.rs

@@ -32,6 +32,7 @@ use crate::sandboxing::CommandSpec;
 use crate::sandboxing::ExecRequest;
 use crate::sandboxing::SandboxManager;
 use crate::sandboxing::SandboxPermissions;
+use crate::sandboxing::should_require_platform_sandbox;
 use crate::spawn::SpawnChildRequest;
 use crate::spawn::StdioPolicy;
 use crate::spawn::spawn_child_async;
@@ -165,22 +166,17 @@ pub async fn process_exec_tool_call(
 ) -> Result<ExecToolCallOutput> {
     let windows_sandbox_level = params.windows_sandbox_level;
     let enforce_managed_network = params.network.is_some();
-    let sandbox_type = match file_system_sandbox_policy.kind {
-        FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => {
-            if enforce_managed_network {
-                get_platform_sandbox(
-                    windows_sandbox_level
-                        != codex_protocol::config_types::WindowsSandboxLevel::Disabled,
-                )
-                .unwrap_or(SandboxType::None)
-            } else {
-                SandboxType::None
-            }
-        }
-        _ => get_platform_sandbox(
+    let sandbox_type = if should_require_platform_sandbox(
+        file_system_sandbox_policy,
+        network_sandbox_policy,
+        enforce_managed_network,
+    ) {
+        get_platform_sandbox(
             windows_sandbox_level != codex_protocol::config_types::WindowsSandboxLevel::Disabled,
         )
-        .unwrap_or(SandboxType::None),
+        .unwrap_or(SandboxType::None)
+    } else {
+        SandboxType::None
     };
     tracing::debug!("Sandbox type: {sandbox_type:?}");
 

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

@@ -13,6 +13,9 @@ use crate::exec::StdoutStream;
 use crate::exec::execute_exec_env;
 use crate::landlock::allow_network_for_proxy;
 use crate::landlock::create_linux_sandbox_command_args_for_policies;
+use crate::protocol::FileSystemAccessMode;
+use crate::protocol::FileSystemPath;
+use crate::protocol::FileSystemSandboxEntry;
 use crate::protocol::FileSystemSandboxKind;
 use crate::protocol::FileSystemSandboxPolicy;
 use crate::protocol::NetworkSandboxPolicy;
@@ -177,6 +180,40 @@ fn additional_permission_roots(
     )
 }
 
+fn merge_file_system_policy_with_additional_permissions(
+    file_system_policy: &FileSystemSandboxPolicy,
+    extra_reads: Vec<AbsolutePathBuf>,
+    extra_writes: Vec<AbsolutePathBuf>,
+) -> FileSystemSandboxPolicy {
+    match file_system_policy.kind {
+        FileSystemSandboxKind::Restricted => {
+            let mut merged_policy = file_system_policy.clone();
+            for path in extra_reads {
+                let entry = FileSystemSandboxEntry {
+                    path: FileSystemPath::Path { path },
+                    access: FileSystemAccessMode::Read,
+                };
+                if !merged_policy.entries.contains(&entry) {
+                    merged_policy.entries.push(entry);
+                }
+            }
+            for path in extra_writes {
+                let entry = FileSystemSandboxEntry {
+                    path: FileSystemPath::Path { path },
+                    access: FileSystemAccessMode::Write,
+                };
+                if !merged_policy.entries.contains(&entry) {
+                    merged_policy.entries.push(entry);
+                }
+            }
+            merged_policy
+        }
+        FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => {
+            file_system_policy.clone()
+        }
+    }
+}
+
 fn merge_read_only_access_with_additional_reads(
     read_only_access: &ReadOnlyAccess,
     extra_reads: Vec<AbsolutePathBuf>,
@@ -281,6 +318,28 @@ fn sandbox_policy_with_additional_permissions(
     Ok(policy)
 }
 
+pub(crate) fn should_require_platform_sandbox(
+    file_system_policy: &FileSystemSandboxPolicy,
+    network_policy: NetworkSandboxPolicy,
+    has_managed_network_requirements: bool,
+) -> bool {
+    if has_managed_network_requirements {
+        return true;
+    }
+
+    if !network_policy.is_enabled() {
+        return !matches!(
+            file_system_policy.kind,
+            FileSystemSandboxKind::ExternalSandbox
+        );
+    }
+
+    match file_system_policy.kind {
+        FileSystemSandboxKind::Restricted => !file_system_policy.has_full_disk_write_access(),
+        FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => false,
+    }
+}
+
 #[derive(Default)]
 pub struct SandboxManager;
 
@@ -307,22 +366,20 @@ impl SandboxManager {
                 )
                 .unwrap_or(SandboxType::None)
             }
-            SandboxablePreference::Auto => match file_system_policy.kind {
-                FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => {
-                    if has_managed_network_requirements {
-                        crate::safety::get_platform_sandbox(
-                            windows_sandbox_level != WindowsSandboxLevel::Disabled,
-                        )
-                        .unwrap_or(SandboxType::None)
-                    } else {
-                        SandboxType::None
-                    }
+            SandboxablePreference::Auto => {
+                if should_require_platform_sandbox(
+                    file_system_policy,
+                    network_policy,
+                    has_managed_network_requirements,
+                ) {
+                    crate::safety::get_platform_sandbox(
+                        windows_sandbox_level != WindowsSandboxLevel::Disabled,
+                    )
+                    .unwrap_or(SandboxType::None)
+                } else {
+                    SandboxType::None
                 }
-                _ => crate::safety::get_platform_sandbox(
-                    windows_sandbox_level != WindowsSandboxLevel::Disabled,
-                )
-                .unwrap_or(SandboxType::None),
-            },
+            }
         }
     }
 
@@ -354,13 +411,11 @@ impl SandboxManager {
             let file_system_sandbox_policy = if extra_reads.is_empty() && extra_writes.is_empty() {
                 file_system_policy.clone()
             } else {
-                match file_system_policy.kind {
-                    FileSystemSandboxKind::Restricted => {
-                        FileSystemSandboxPolicy::from(&effective_policy)
-                    }
-                    FileSystemSandboxKind::Unrestricted
-                    | FileSystemSandboxKind::ExternalSandbox => file_system_policy.clone(),
-                }
+                merge_file_system_policy_with_additional_permissions(
+                    file_system_policy,
+                    extra_reads,
+                    extra_writes,
+                )
             };
             let network_sandbox_policy = NetworkSandboxPolicy::from(&effective_policy);
             (
@@ -473,8 +528,10 @@ pub async fn execute_env(
 #[cfg(test)]
 mod tests {
     use super::SandboxManager;
+    use super::merge_file_system_policy_with_additional_permissions;
     use super::normalize_additional_permissions;
     use super::sandbox_policy_with_additional_permissions;
+    use super::should_require_platform_sandbox;
     use crate::exec::SandboxType;
     use crate::protocol::FileSystemAccessMode;
     use crate::protocol::FileSystemPath;
@@ -564,6 +621,35 @@ mod tests {
         );
     }
 
+    #[test]
+    fn root_write_policy_with_carveouts_still_uses_platform_sandbox() {
+        let policy = FileSystemSandboxPolicy::restricted(vec![
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Special {
+                    value: FileSystemSpecialPath {
+                        kind: FileSystemSpecialPathKind::Root,
+                        subpath: None,
+                    },
+                },
+                access: FileSystemAccessMode::Write,
+            },
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Special {
+                    value: FileSystemSpecialPath {
+                        kind: FileSystemSpecialPathKind::CurrentWorkingDirectory,
+                        subpath: Some("blocked".into()),
+                    },
+                },
+                access: FileSystemAccessMode::None,
+            },
+        ]);
+
+        assert_eq!(
+            should_require_platform_sandbox(&policy, NetworkSandboxPolicy::Enabled, false),
+            true
+        );
+    }
+
     #[test]
     fn full_access_restricted_policy_still_uses_platform_sandbox_for_restricted_network() {
         let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {

codex-rs/protocol/src/protocol.rs

@@ -645,6 +645,25 @@ impl Default for FileSystemSandboxPolicy {
 }
 
 impl FileSystemSandboxPolicy {
+    fn has_root_access(&self, predicate: impl Fn(FileSystemAccessMode) -> bool) -> bool {
+        matches!(self.kind, FileSystemSandboxKind::Restricted)
+            && self.entries.iter().any(|entry| {
+                matches!(
+                    &entry.path,
+                    FileSystemPath::Special { value }
+                        if value.kind == FileSystemSpecialPathKind::Root && predicate(entry.access)
+                )
+            })
+    }
+
+    fn has_explicit_deny_entries(&self) -> bool {
+        matches!(self.kind, FileSystemSandboxKind::Restricted)
+            && self
+                .entries
+                .iter()
+                .any(|entry| entry.access == FileSystemAccessMode::None)
+    }
+
     pub fn unrestricted() -> Self {
         Self {
             kind: FileSystemSandboxKind::Unrestricted,
@@ -670,13 +689,10 @@ impl FileSystemSandboxPolicy {
     pub fn has_full_disk_read_access(&self) -> bool {
         match self.kind {
             FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => true,
-            FileSystemSandboxKind::Restricted => self.entries.iter().any(|entry| {
-                matches!(
-                    &entry.path,
-                    FileSystemPath::Special { value }
-                        if value.kind == FileSystemSpecialPathKind::Root && entry.access.can_read()
-                )
-            }),
+            FileSystemSandboxKind::Restricted => {
+                self.has_root_access(FileSystemAccessMode::can_read)
+                    && !self.has_explicit_deny_entries()
+            }
         }
     }
 
@@ -684,13 +700,10 @@ impl FileSystemSandboxPolicy {
     pub fn has_full_disk_write_access(&self) -> bool {
         match self.kind {
             FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => true,
-            FileSystemSandboxKind::Restricted => self.entries.iter().any(|entry| {
-                matches!(
-                    &entry.path,
-                    FileSystemPath::Special { value }
-                        if value.kind == FileSystemSpecialPathKind::Root && entry.access.can_write()
-                )
-            }),
+            FileSystemSandboxKind::Restricted => {
+                self.has_root_access(FileSystemAccessMode::can_write)
+                    && !self.has_explicit_deny_entries()
+            }
         }
     }
 
@@ -715,11 +728,24 @@ impl FileSystemSandboxPolicy {
         }
 
         let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
+        let mut readable_roots = Vec::new();
+        if self.has_root_access(FileSystemAccessMode::can_read)
+            && let Some(cwd_absolute) = cwd_absolute.as_ref()
+        {
+            readable_roots.push(absolute_root_path_for_cwd(cwd_absolute));
+        }
+
         dedup_absolute_paths(
-            self.entries
-                .iter()
-                .filter(|entry| entry.access.can_read())
-                .filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref()))
+            readable_roots
+                .into_iter()
+                .chain(
+                    self.entries
+                        .iter()
+                        .filter(|entry| entry.access.can_read())
+                        .filter_map(|entry| {
+                            resolve_file_system_path(&entry.path, cwd_absolute.as_ref())
+                        }),
+                )
                 .collect(),
         )
     }
@@ -733,11 +759,24 @@ impl FileSystemSandboxPolicy {
 
         let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
         let unreadable_roots = self.get_unreadable_roots_with_cwd(cwd);
+        let mut writable_roots = Vec::new();
+        if self.has_root_access(FileSystemAccessMode::can_write)
+            && let Some(cwd_absolute) = cwd_absolute.as_ref()
+        {
+            writable_roots.push(absolute_root_path_for_cwd(cwd_absolute));
+        }
+
         dedup_absolute_paths(
-            self.entries
-                .iter()
-                .filter(|entry| entry.access.can_write())
-                .filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref()))
+            writable_roots
+                .into_iter()
+                .chain(
+                    self.entries
+                        .iter()
+                        .filter(|entry| entry.access.can_write())
+                        .filter_map(|entry| {
+                            resolve_file_system_path(&entry.path, cwd_absolute.as_ref())
+                        }),
+                )
                 .collect(),
         )
         .into_iter()
@@ -1435,6 +1474,16 @@ impl From<&SandboxPolicy> for FileSystemSandboxPolicy {
     }
 }
 
+fn absolute_root_path_for_cwd(cwd: &AbsolutePathBuf) -> AbsolutePathBuf {
+    let root = cwd
+        .as_path()
+        .ancestors()
+        .last()
+        .unwrap_or_else(|| panic!("cwd must have a filesystem root"));
+    AbsolutePathBuf::from_absolute_path(root)
+        .unwrap_or_else(|err| panic!("cwd root must be an absolute path: {err}"))
+}
+
 fn resolve_file_system_path(
     path: &FileSystemPath,
     cwd: Option<&AbsolutePathBuf>,
@@ -3845,6 +3894,57 @@ mod tests {
         assert!(writable.has_full_disk_write_access());
     }
 
+    #[test]
+    fn restricted_file_system_policy_treats_root_with_carveouts_as_scoped_access() {
+        let cwd = TempDir::new().expect("tempdir");
+        let cwd_absolute =
+            AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute tempdir");
+        let root = absolute_root_path_for_cwd(&cwd_absolute);
+        let blocked = AbsolutePathBuf::resolve_path_against_base("blocked", cwd.path())
+            .expect("resolve blocked");
+        let policy = FileSystemSandboxPolicy::restricted(vec![
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Special {
+                    value: FileSystemSpecialPath {
+                        kind: FileSystemSpecialPathKind::Root,
+                        subpath: None,
+                    },
+                },
+                access: FileSystemAccessMode::Write,
+            },
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Special {
+                    value: FileSystemSpecialPath {
+                        kind: FileSystemSpecialPathKind::CurrentWorkingDirectory,
+                        subpath: Some(PathBuf::from("blocked")),
+                    },
+                },
+                access: FileSystemAccessMode::None,
+            },
+        ]);
+
+        assert!(!policy.has_full_disk_read_access());
+        assert!(!policy.has_full_disk_write_access());
+        assert_eq!(
+            policy.get_readable_roots_with_cwd(cwd.path()),
+            vec![root.clone()]
+        );
+        assert_eq!(
+            policy.get_unreadable_roots_with_cwd(cwd.path()),
+            vec![blocked.clone()]
+        );
+
+        let writable_roots = policy.get_writable_roots_with_cwd(cwd.path());
+        assert_eq!(writable_roots.len(), 1);
+        assert_eq!(writable_roots[0].root, root);
+        assert!(
+            writable_roots[0]
+                .read_only_subpaths
+                .iter()
+                .any(|path| path.as_path() == blocked.as_path())
+        );
+    }
+
     #[test]
     fn restricted_file_system_policy_derives_effective_paths() {
         let cwd = TempDir::new().expect("tempdir");