refactor: centralize filesystem permissions precedence

codex-rs/protocol/src/permissions.rs

@@ -121,6 +121,22 @@ pub struct FileSystemSandboxPolicy {
     pub entries: Vec<FileSystemSandboxEntry>,
 }
 
+#[derive(Debug, Clone, PartialEq, Eq)]
+struct ResolvedFileSystemEntry {
+    path: AbsolutePathBuf,
+    access: FileSystemAccessMode,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+struct FileSystemSemanticSignature {
+    has_full_disk_read_access: bool,
+    has_full_disk_write_access: bool,
+    include_platform_defaults: bool,
+    readable_roots: Vec<AbsolutePathBuf>,
+    writable_roots: Vec<WritableRoot>,
+    unreadable_roots: Vec<AbsolutePathBuf>,
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)]
 #[serde(tag = "type", rename_all = "snake_case")]
 #[ts(tag = "type")]
@@ -163,6 +179,25 @@ impl FileSystemSandboxPolicy {
                 .any(|entry| entry.access == FileSystemAccessMode::None)
     }
 
+    fn has_write_narrowing_entries(&self) -> bool {
+        matches!(self.kind, FileSystemSandboxKind::Restricted)
+            && self.entries.iter().any(|entry| {
+                if entry.access.can_write() {
+                    return false;
+                }
+
+                match &entry.path {
+                    FileSystemPath::Path { .. } => true,
+                    FileSystemPath::Special { value } => !matches!(
+                        value,
+                        FileSystemSpecialPath::Root
+                            | FileSystemSpecialPath::Minimal
+                            | FileSystemSpecialPath::Unknown { .. }
+                    ),
+                }
+            })
+    }
+
     pub fn unrestricted() -> Self {
         Self {
             kind: FileSystemSandboxKind::Unrestricted,
@@ -229,7 +264,7 @@ impl FileSystemSandboxPolicy {
             FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => true,
             FileSystemSandboxKind::Restricted => {
                 self.has_root_access(FileSystemAccessMode::can_write)
-                    && !self.has_explicit_deny_entries()
+                    && !self.has_write_narrowing_entries()
             }
         }
     }
@@ -248,31 +283,64 @@ impl FileSystemSandboxPolicy {
             })
     }
 
+    pub fn resolve_access_with_cwd(&self, path: &Path, cwd: &Path) -> FileSystemAccessMode {
+        match self.kind {
+            FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => {
+                return FileSystemAccessMode::Write;
+            }
+            FileSystemSandboxKind::Restricted => {}
+        }
+
+        let Some(path) = resolve_candidate_path(path, cwd) else {
+            return FileSystemAccessMode::None;
+        };
+
+        self.resolved_entries_with_cwd(cwd)
+            .into_iter()
+            .filter(|entry| path.as_path().starts_with(entry.path.as_path()))
+            .max_by_key(resolved_entry_precedence)
+            .map(|entry| entry.access)
+            .unwrap_or(FileSystemAccessMode::None)
+    }
+
+    pub fn can_read_path_with_cwd(&self, path: &Path, cwd: &Path) -> bool {
+        self.resolve_access_with_cwd(path, cwd).can_read()
+    }
+
+    pub fn can_write_path_with_cwd(&self, path: &Path, cwd: &Path) -> bool {
+        self.resolve_access_with_cwd(path, cwd).can_write()
+    }
+
+    pub fn needs_direct_runtime_enforcement(
+        &self,
+        network_policy: NetworkSandboxPolicy,
+        cwd: &Path,
+    ) -> bool {
+        if !matches!(self.kind, FileSystemSandboxKind::Restricted) {
+            return false;
+        }
+
+        let Ok(legacy_policy) = self.to_legacy_sandbox_policy(network_policy, cwd) else {
+            return true;
+        };
+
+        self.semantic_signature(cwd)
+            != FileSystemSandboxPolicy::from_legacy_sandbox_policy(&legacy_policy, cwd)
+                .semantic_signature(cwd)
+    }
+
     /// Returns the explicit readable roots resolved against the provided cwd.
     pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec<AbsolutePathBuf> {
         if self.has_full_disk_read_access() {
             return Vec::new();
         }
 
-        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(
-            readable_roots
+            self.resolved_entries_with_cwd(cwd)
                 .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())
-                        }),
-                )
+                .filter(|entry| entry.access.can_read())
+                .filter(|entry| self.can_read_path_with_cwd(entry.path.as_path(), cwd))
+                .map(|entry| entry.path)
                 .collect(),
         )
     }
@@ -284,32 +352,22 @@ impl FileSystemSandboxPolicy {
             return Vec::new();
         }
 
-        let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
+        let resolved_entries = self.resolved_entries_with_cwd(cwd);
         let read_only_roots = dedup_absolute_paths(
-            self.entries
+            resolved_entries
                 .iter()
                 .filter(|entry| !entry.access.can_write())
-                .filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref()))
+                .filter(|entry| !self.can_write_path_with_cwd(entry.path.as_path(), cwd))
+                .map(|entry| entry.path.clone())
                 .collect(),
         );
-        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(
-            writable_roots
+            resolved_entries
                 .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())
-                        }),
-                )
+                .filter(|entry| entry.access.can_write())
+                .filter(|entry| self.can_write_path_with_cwd(entry.path.as_path(), cwd))
+                .map(|entry| entry.path)
                 .collect(),
         )
         .into_iter()
@@ -339,12 +397,12 @@ impl FileSystemSandboxPolicy {
             return Vec::new();
         }
 
-        let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
         dedup_absolute_paths(
-            self.entries
+            self.resolved_entries_with_cwd(cwd)
                 .iter()
                 .filter(|entry| entry.access == FileSystemAccessMode::None)
-                .filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref()))
+                .filter(|entry| !self.can_read_path_with_cwd(entry.path.as_path(), cwd))
+                .map(|entry| entry.path.clone())
                 .collect(),
         )
     }
@@ -504,6 +562,32 @@ impl FileSystemSandboxPolicy {
             }
         })
     }
+
+    fn resolved_entries_with_cwd(&self, cwd: &Path) -> Vec<ResolvedFileSystemEntry> {
+        let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
+        self.entries
+            .iter()
+            .filter_map(|entry| {
+                resolve_entry_path(&entry.path, cwd_absolute.as_ref()).map(|path| {
+                    ResolvedFileSystemEntry {
+                        path,
+                        access: entry.access,
+                    }
+                })
+            })
+            .collect()
+    }
+
+    fn semantic_signature(&self, cwd: &Path) -> FileSystemSemanticSignature {
+        FileSystemSemanticSignature {
+            has_full_disk_read_access: self.has_full_disk_read_access(),
+            has_full_disk_write_access: self.has_full_disk_write_access(),
+            include_platform_defaults: self.include_platform_defaults(),
+            readable_roots: self.get_readable_roots_with_cwd(cwd),
+            writable_roots: self.get_writable_roots_with_cwd(cwd),
+            unreadable_roots: self.get_unreadable_roots_with_cwd(cwd),
+        }
+    }
 }
 
 impl From<&SandboxPolicy> for NetworkSandboxPolicy {
@@ -641,6 +725,36 @@ fn resolve_file_system_path(
     }
 }
 
+fn resolve_entry_path(
+    path: &FileSystemPath,
+    cwd: Option<&AbsolutePathBuf>,
+) -> Option<AbsolutePathBuf> {
+    match path {
+        FileSystemPath::Special {
+            value: FileSystemSpecialPath::Root,
+        } => cwd.map(absolute_root_path_for_cwd),
+        _ => resolve_file_system_path(path, cwd),
+    }
+}
+
+fn resolve_candidate_path(path: &Path, cwd: &Path) -> Option<AbsolutePathBuf> {
+    if path.is_absolute() {
+        AbsolutePathBuf::from_absolute_path(path).ok()
+    } else {
+        AbsolutePathBuf::resolve_path_against_base(path, cwd).ok()
+    }
+}
+
+fn resolved_entry_precedence(entry: &ResolvedFileSystemEntry) -> (usize, u8) {
+    let specificity = entry.path.as_path().components().count();
+    let access_priority = match entry.access {
+        FileSystemAccessMode::Read => 0,
+        FileSystemAccessMode::Write => 1,
+        FileSystemAccessMode::None => 2,
+    };
+    (specificity, access_priority)
+}
+
 fn absolute_root_path_for_cwd(cwd: &AbsolutePathBuf) -> AbsolutePathBuf {
     let root = cwd
         .as_path()
@@ -808,6 +922,7 @@ fn resolve_gitdir_from_file(dot_git: &AbsolutePathBuf) -> Option<AbsolutePathBuf
 mod tests {
     use super::*;
     use pretty_assertions::assert_eq;
+    use tempfile::TempDir;
 
     #[test]
     fn unknown_special_paths_are_ignored_by_legacy_bridge() -> std::io::Result<()> {
@@ -835,4 +950,115 @@ mod tests {
         );
         Ok(())
     }
+
+    #[test]
+    fn resolve_access_with_cwd_uses_most_specific_entry() {
+        let cwd = TempDir::new().expect("tempdir");
+        let docs =
+            AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs");
+        let docs_private = AbsolutePathBuf::resolve_path_against_base("docs/private", cwd.path())
+            .expect("resolve docs/private");
+        let docs_private_public =
+            AbsolutePathBuf::resolve_path_against_base("docs/private/public", cwd.path())
+                .expect("resolve docs/private/public");
+        let policy = FileSystemSandboxPolicy::restricted(vec![
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Special {
+                    value: FileSystemSpecialPath::CurrentWorkingDirectory,
+                },
+                access: FileSystemAccessMode::Write,
+            },
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Path { path: docs.clone() },
+                access: FileSystemAccessMode::Read,
+            },
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Path {
+                    path: docs_private.clone(),
+                },
+                access: FileSystemAccessMode::None,
+            },
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Path {
+                    path: docs_private_public.clone(),
+                },
+                access: FileSystemAccessMode::Write,
+            },
+        ]);
+
+        assert_eq!(
+            policy.resolve_access_with_cwd(cwd.path(), cwd.path()),
+            FileSystemAccessMode::Write
+        );
+        assert_eq!(
+            policy.resolve_access_with_cwd(docs.as_path(), cwd.path()),
+            FileSystemAccessMode::Read
+        );
+        assert_eq!(
+            policy.resolve_access_with_cwd(docs_private.as_path(), cwd.path()),
+            FileSystemAccessMode::None
+        );
+        assert_eq!(
+            policy.resolve_access_with_cwd(docs_private_public.as_path(), cwd.path()),
+            FileSystemAccessMode::Write
+        );
+    }
+
+    #[test]
+    fn split_only_nested_carveouts_need_direct_runtime_enforcement() {
+        let cwd = TempDir::new().expect("tempdir");
+        let docs =
+            AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs");
+        let policy = FileSystemSandboxPolicy::restricted(vec![
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Special {
+                    value: FileSystemSpecialPath::CurrentWorkingDirectory,
+                },
+                access: FileSystemAccessMode::Write,
+            },
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Path { path: docs },
+                access: FileSystemAccessMode::Read,
+            },
+        ]);
+
+        assert!(
+            policy.needs_direct_runtime_enforcement(NetworkSandboxPolicy::Restricted, cwd.path(),)
+        );
+
+        let legacy_workspace_write =
+            FileSystemSandboxPolicy::from(&SandboxPolicy::new_workspace_write_policy());
+        assert!(
+            !legacy_workspace_write
+                .needs_direct_runtime_enforcement(NetworkSandboxPolicy::Restricted, cwd.path(),)
+        );
+    }
+
+    #[test]
+    fn root_write_with_read_only_child_is_not_full_disk_write() {
+        let cwd = TempDir::new().expect("tempdir");
+        let docs =
+            AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs");
+        let policy = FileSystemSandboxPolicy::restricted(vec![
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Special {
+                    value: FileSystemSpecialPath::Root,
+                },
+                access: FileSystemAccessMode::Write,
+            },
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Path { path: docs.clone() },
+                access: FileSystemAccessMode::Read,
+            },
+        ]);
+
+        assert!(!policy.has_full_disk_write_access());
+        assert_eq!(
+            policy.resolve_access_with_cwd(docs.as_path(), cwd.path()),
+            FileSystemAccessMode::Read
+        );
+        assert!(
+            policy.needs_direct_runtime_enforcement(NetworkSandboxPolicy::Restricted, cwd.path(),)
+        );
+    }
 }