From dc46620ef4da513081c312bc7bc019196cfbc104 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 9 Mar 2026 21:27:49 -0700 Subject: [PATCH 1/8] refactor: centralize filesystem permissions precedence --- codex-rs/protocol/src/permissions.rs | 304 +++++++++++++++++++++++---- 1 file changed, 265 insertions(+), 39 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 9624f36cd61f..66059e675f93 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -121,6 +121,22 @@ pub struct FileSystemSandboxPolicy { pub entries: Vec, } +#[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, + writable_roots: Vec, + unreadable_roots: Vec, +} + #[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 { 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 { + 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 { + 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 { + 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 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(),) + ); + } } From e1d9f12b7f44eccd1975e89e0aa39e0c9bd18395 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 9 Mar 2026 22:14:53 -0700 Subject: [PATCH 2/8] fix: preserve root deny carveouts in permissions --- codex-rs/protocol/src/permissions.rs | 75 +++++++++++++++++++++++++--- 1 file changed, 69 insertions(+), 6 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 66059e675f93..3fac7db00f95 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -188,12 +188,13 @@ impl FileSystemSandboxPolicy { match &entry.path { FileSystemPath::Path { .. } => true, - FileSystemPath::Special { value } => !matches!( - value, - FileSystemSpecialPath::Root - | FileSystemSpecialPath::Minimal - | FileSystemSpecialPath::Unknown { .. } - ), + FileSystemPath::Special { value } => match value { + FileSystemSpecialPath::Root => entry.access == FileSystemAccessMode::None, + FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => { + false + } + _ => true, + }, } }) } @@ -397,11 +398,19 @@ impl FileSystemSandboxPolicy { return Vec::new(); } + let root = AbsolutePathBuf::from_absolute_path(cwd) + .ok() + .map(|cwd| absolute_root_path_for_cwd(&cwd)); + dedup_absolute_paths( self.resolved_entries_with_cwd(cwd) .iter() .filter(|entry| entry.access == FileSystemAccessMode::None) .filter(|entry| !self.can_read_path_with_cwd(entry.path.as_path(), cwd)) + // Restricted policies already deny reads outside explicit allow roots, + // so materializing the filesystem root here would erase narrower + // readable carveouts when downstream sandboxes apply deny masks last. + .filter(|entry| root.as_ref() != Some(&entry.path)) .map(|entry| entry.path.clone()) .collect(), ) @@ -1061,4 +1070,58 @@ mod tests { policy.needs_direct_runtime_enforcement(NetworkSandboxPolicy::Restricted, cwd.path(),) ); } + + #[test] + fn root_deny_does_not_materialize_as_unreadable_root() { + 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::None, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: docs.clone() }, + access: FileSystemAccessMode::Read, + }, + ]); + + assert_eq!( + policy.resolve_access_with_cwd(docs.as_path(), cwd.path()), + FileSystemAccessMode::Read + ); + assert_eq!(policy.get_readable_roots_with_cwd(cwd.path()), vec![docs]); + assert!(policy.get_unreadable_roots_with_cwd(cwd.path()).is_empty()); + } + + #[test] + fn duplicate_root_deny_prevents_full_disk_write_access() { + let cwd = TempDir::new().expect("tempdir"); + let root = AbsolutePathBuf::from_absolute_path(cwd.path()) + .map(|cwd| absolute_root_path_for_cwd(&cwd)) + .expect("resolve filesystem root"); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::None, + }, + ]); + + assert!(!policy.has_full_disk_write_access()); + assert_eq!( + policy.resolve_access_with_cwd(root.as_path(), cwd.path()), + FileSystemAccessMode::None + ); + } } From fd65c49f71944001c1066857704915ea21733666 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 9 Mar 2026 22:58:16 -0700 Subject: [PATCH 3/8] fix: ignore overridden write narrowings in permissions --- codex-rs/protocol/src/permissions.rs | 116 +++++++++++++++++++++++++-- 1 file changed, 109 insertions(+), 7 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 3fac7db00f95..6e18777e0c10 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -187,18 +187,26 @@ impl FileSystemSandboxPolicy { } match &entry.path { - FileSystemPath::Path { .. } => true, + FileSystemPath::Path { .. } => !self.has_same_target_write_override(entry), FileSystemPath::Special { value } => match value { FileSystemSpecialPath::Root => entry.access == FileSystemAccessMode::None, FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => { false } - _ => true, + _ => !self.has_same_target_write_override(entry), }, } }) } + 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) + && file_system_paths_share_target(&candidate.path, &entry.path) + }) + } + pub fn unrestricted() -> Self { Self { kind: FileSystemSandboxKind::Unrestricted, @@ -754,14 +762,79 @@ fn resolve_candidate_path(path: &Path, cwd: &Path) -> Option { } } -fn resolved_entry_precedence(entry: &ResolvedFileSystemEntry) -> (usize, u8) { - let specificity = entry.path.as_path().components().count(); - let access_priority = match entry.access { +fn access_priority(access: FileSystemAccessMode) -> u8 { + match access { FileSystemAccessMode::Read => 0, FileSystemAccessMode::Write => 1, FileSystemAccessMode::None => 2, - }; - (specificity, access_priority) + } +} + +fn file_system_paths_share_target(left: &FileSystemPath, right: &FileSystemPath) -> bool { + match (left, right) { + (FileSystemPath::Path { path: left }, FileSystemPath::Path { path: right }) => { + left == right + } + (FileSystemPath::Special { value: left }, FileSystemPath::Special { value: right }) => { + special_paths_share_target(left, right) + } + (FileSystemPath::Path { path }, FileSystemPath::Special { value }) + | (FileSystemPath::Special { value }, FileSystemPath::Path { path }) => { + special_path_matches_absolute_path(value, path) + } + } +} + +fn special_paths_share_target(left: &FileSystemSpecialPath, right: &FileSystemSpecialPath) -> bool { + match (left, right) { + (FileSystemSpecialPath::Root, FileSystemSpecialPath::Root) + | (FileSystemSpecialPath::Minimal, FileSystemSpecialPath::Minimal) + | ( + FileSystemSpecialPath::CurrentWorkingDirectory, + FileSystemSpecialPath::CurrentWorkingDirectory, + ) + | (FileSystemSpecialPath::Tmpdir, FileSystemSpecialPath::Tmpdir) + | (FileSystemSpecialPath::SlashTmp, FileSystemSpecialPath::SlashTmp) => true, + ( + FileSystemSpecialPath::CurrentWorkingDirectory, + FileSystemSpecialPath::ProjectRoots { subpath: None }, + ) + | ( + FileSystemSpecialPath::ProjectRoots { subpath: None }, + FileSystemSpecialPath::CurrentWorkingDirectory, + ) => true, + ( + FileSystemSpecialPath::ProjectRoots { subpath: left }, + FileSystemSpecialPath::ProjectRoots { subpath: right }, + ) => left == right, + ( + FileSystemSpecialPath::Unknown { + path: left, + subpath: left_subpath, + }, + FileSystemSpecialPath::Unknown { + path: right, + subpath: right_subpath, + }, + ) => left == right && left_subpath == right_subpath, + _ => false, + } +} + +fn special_path_matches_absolute_path( + value: &FileSystemSpecialPath, + path: &AbsolutePathBuf, +) -> bool { + match value { + FileSystemSpecialPath::Root => path.as_path().parent().is_none(), + FileSystemSpecialPath::SlashTmp => path.as_path() == Path::new("/tmp"), + _ => false, + } +} + +fn resolved_entry_precedence(entry: &ResolvedFileSystemEntry) -> (usize, u8) { + let specificity = entry.path.as_path().components().count(); + (specificity, access_priority(entry.access)) } fn absolute_root_path_for_cwd(cwd: &AbsolutePathBuf) -> AbsolutePathBuf { @@ -1124,4 +1197,33 @@ mod tests { FileSystemAccessMode::None ); } + + #[test] + fn same_specificity_write_override_keeps_full_disk_write_access() { + 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, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: docs.clone() }, + access: FileSystemAccessMode::Write, + }, + ]); + + assert!(policy.has_full_disk_write_access()); + assert_eq!( + policy.resolve_access_with_cwd(docs.as_path(), cwd.path()), + FileSystemAccessMode::Write + ); + } } From 2f41cf7cd130e031733fcf3c063ef392da7b581e Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 9 Mar 2026 23:06:16 -0700 Subject: [PATCH 4/8] docs: explain permissions precedence helpers --- codex-rs/protocol/src/permissions.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 6e18777e0c10..f8f8743419f9 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -179,6 +179,13 @@ impl FileSystemSandboxPolicy { .any(|entry| entry.access == FileSystemAccessMode::None) } + /// Returns true when a restricted policy contains any entry that really + /// reduces a broader `:root = write` grant. + /// + /// Raw entry presence is not enough here: an equally specific `write` + /// entry for the same target wins under the normal precedence rules, so a + /// shadowed `read` entry must not downgrade the policy out of full-disk + /// write mode. fn has_write_narrowing_entries(&self) -> bool { matches!(self.kind, FileSystemSandboxKind::Restricted) && self.entries.iter().any(|entry| { @@ -199,6 +206,8 @@ impl FileSystemSandboxPolicy { }) } + /// Returns true when a higher-priority `write` entry targets the same + /// location as `entry`, so `entry` cannot narrow effective write access. fn has_same_target_write_override(&self, entry: &FileSystemSandboxEntry) -> bool { self.entries.iter().any(|candidate| { candidate.access.can_write() @@ -762,6 +771,8 @@ 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, @@ -770,6 +781,12 @@ fn access_priority(access: FileSystemAccessMode) -> u8 { } } +/// Returns true when two config paths refer to the same exact target before +/// any prefix matching is applied. +/// +/// This is intentionally narrower than full path resolution: it only answers +/// the "can one entry shadow another at the same specificity?" question used +/// by `has_write_narrowing_entries`. fn file_system_paths_share_target(left: &FileSystemPath, right: &FileSystemPath) -> bool { match (left, right) { (FileSystemPath::Path { path: left }, FileSystemPath::Path { path: right }) => { @@ -785,6 +802,8 @@ fn file_system_paths_share_target(left: &FileSystemPath, right: &FileSystemPath) } } +/// Compares special-path tokens that resolve to the same concrete target +/// without needing a cwd. fn special_paths_share_target(left: &FileSystemSpecialPath, right: &FileSystemSpecialPath) -> bool { match (left, right) { (FileSystemSpecialPath::Root, FileSystemSpecialPath::Root) @@ -821,6 +840,11 @@ fn special_paths_share_target(left: &FileSystemSpecialPath, right: &FileSystemSp } } +/// Matches cwd-independent special paths against absolute `Path` entries when +/// they name the same location. +/// +/// We intentionally only fold the special paths whose concrete meaning is +/// stable without a cwd, such as `/` and `/tmp`. fn special_path_matches_absolute_path( value: &FileSystemSpecialPath, path: &AbsolutePathBuf, @@ -832,6 +856,8 @@ 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) { let specificity = entry.path.as_path().components().count(); (specificity, access_priority(entry.access)) From 8f936430f360bddf61ebaf809a62b926716c4545 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 10 Mar 2026 00:21:02 -0700 Subject: [PATCH 5/8] fix: add request permissions scope defaults --- codex-rs/core/src/codex.rs | 2 ++ codex-rs/core/src/codex_tests.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index ed119aabffc8..b30b93cc18b5 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2851,6 +2851,7 @@ impl Session { AskForApproval::Never => { return Some(RequestPermissionsResponse { permissions: PermissionProfile::default(), + scope: PermissionGrantScope::Turn, }); } AskForApproval::Reject(reject_config) @@ -2858,6 +2859,7 @@ impl Session { { return Some(RequestPermissionsResponse { permissions: PermissionProfile::default(), + scope: PermissionGrantScope::Turn, }); } AskForApproval::OnFailure diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 66e6cd27b6b6..e457f873bdb3 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -2293,6 +2293,7 @@ async fn request_permissions_emits_event_when_reject_policy_allows_requests() { }), ..Default::default() }, + scope: PermissionGrantScope::Turn, }; let handle = tokio::spawn({ @@ -2377,6 +2378,7 @@ async fn request_permissions_returns_empty_grant_when_reject_policy_blocks_reque Some( codex_protocol::request_permissions::RequestPermissionsResponse { permissions: codex_protocol::models::PermissionProfile::default(), + scope: PermissionGrantScope::Turn, } ) ); From f32c85ca064a2d655fbdd65f7f17c44aa13cb1e5 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 11 Mar 2026 16:57:59 -0700 Subject: [PATCH 6/8] refactor: clarify filesystem access precedence ordering --- codex-rs/protocol/src/permissions.rs | 50 ++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index f8f8743419f9..a5c06c989699 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); + } } From f03a477d71821eb9001ea1a21c5fb2484d8cb838 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 11 Mar 2026 17:36:29 -0700 Subject: [PATCH 7/8] docs: refresh config schema fixture --- codex-rs/core/config.schema.json | 1 + 1 file changed, 1 insertion(+) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index a1ca57d7a25d..fbc774ebaf44 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -575,6 +575,7 @@ "type": "object" }, "FileSystemAccessMode": { + "description": "Access mode for a filesystem entry.\n\nWhen 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`.", "enum": [ "none", "read", From 94331f1d51d68e5b61071d5ccc17076b709a055c Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 11 Mar 2026 18:12:37 -0700 Subject: [PATCH 8/8] refactor: derive filesystem access mode ordering --- codex-rs/core/config.schema.json | 4 +-- codex-rs/protocol/src/permissions.rs | 38 +++++++++++----------------- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index fbc774ebaf44..8f862153c8aa 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -577,9 +577,9 @@ "FileSystemAccessMode": { "description": "Access mode for a filesystem entry.\n\nWhen 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`.", "enum": [ - "none", "read", - "write" + "write", + "none" ], "type": "string" }, diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index a5c06c989699..6c2e7d336fb0 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -1,4 +1,3 @@ -use std::cmp::Ordering; use std::collections::HashSet; use std::ffi::OsStr; use std::io; @@ -40,24 +39,29 @@ impl NetworkSandboxPolicy { /// 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)] +#[derive( + Debug, + Clone, + Copy, + PartialEq, + Eq, + PartialOrd, + Ord, + Serialize, + Deserialize, + Display, + JsonSchema, + TS, +)] #[serde(rename_all = "lowercase")] #[strum(serialize_all = "lowercase")] pub enum FileSystemAccessMode { - None, Read, Write, + None, } 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) } @@ -67,18 +71,6 @@ 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")]