diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index f8f8743419..a5c06c9896 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -1,3 +1,4 @@ +use std::cmp::Ordering; use std::collections::HashSet; use std::ffi::OsStr; use std::io; @@ -34,6 +35,11 @@ impl NetworkSandboxPolicy { } } +/// Access mode for a filesystem entry. +/// +/// When two equally specific entries target the same path, we compare these by +/// conflict precedence rather than by capability breadth: `none` beats +/// `write`, and `write` beats `read`. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Display, JsonSchema, TS)] #[serde(rename_all = "lowercase")] #[strum(serialize_all = "lowercase")] @@ -44,6 +50,14 @@ pub enum FileSystemAccessMode { } impl FileSystemAccessMode { + fn precedence_rank(self) -> u8 { + match self { + FileSystemAccessMode::Read => 0, + FileSystemAccessMode::Write => 1, + FileSystemAccessMode::None => 2, + } + } + pub fn can_read(self) -> bool { !matches!(self, FileSystemAccessMode::None) } @@ -53,6 +67,18 @@ impl FileSystemAccessMode { } } +impl PartialOrd for FileSystemAccessMode { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for FileSystemAccessMode { + fn cmp(&self, other: &Self) -> Ordering { + self.precedence_rank().cmp(&other.precedence_rank()) + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] #[serde(tag = "kind", rename_all = "snake_case")] #[ts(tag = "kind")] @@ -211,7 +237,7 @@ impl FileSystemSandboxPolicy { fn has_same_target_write_override(&self, entry: &FileSystemSandboxEntry) -> bool { self.entries.iter().any(|candidate| { candidate.access.can_write() - && access_priority(candidate.access) > access_priority(entry.access) + && candidate.access > entry.access && file_system_paths_share_target(&candidate.path, &entry.path) }) } @@ -771,16 +797,6 @@ fn resolve_candidate_path(path: &Path, cwd: &Path) -> Option { } } -/// Mirrors the tie-breaker used by `resolve_access_with_cwd`: for equally -/// specific entries, `none` beats `write`, and `write` beats `read`. -fn access_priority(access: FileSystemAccessMode) -> u8 { - match access { - FileSystemAccessMode::Read => 0, - FileSystemAccessMode::Write => 1, - FileSystemAccessMode::None => 2, - } -} - /// Returns true when two config paths refer to the same exact target before /// any prefix matching is applied. /// @@ -857,10 +873,10 @@ fn special_path_matches_absolute_path( } /// Orders resolved entries so the most specific path wins first, then applies -/// the access tie-breaker from `access_priority`. -fn resolved_entry_precedence(entry: &ResolvedFileSystemEntry) -> (usize, u8) { +/// the access tie-breaker from [`FileSystemAccessMode`]. +fn resolved_entry_precedence(entry: &ResolvedFileSystemEntry) -> (usize, FileSystemAccessMode) { let specificity = entry.path.as_path().components().count(); - (specificity, access_priority(entry.access)) + (specificity, entry.access) } fn absolute_root_path_for_cwd(cwd: &AbsolutePathBuf) -> AbsolutePathBuf { @@ -1252,4 +1268,10 @@ mod tests { FileSystemAccessMode::Write ); } + + #[test] + fn file_system_access_mode_orders_by_conflict_precedence() { + assert!(FileSystemAccessMode::Write > FileSystemAccessMode::Read); + assert!(FileSystemAccessMode::None > FileSystemAccessMode::Write); + } }