From 969efa54708576f972285d7c9c55a0b4ffa4f1ec Mon Sep 17 00:00:00 2001 From: chess Date: Thu, 20 Aug 2026 23:10:13 +0000 Subject: [PATCH] Avoid materializing writable-root carveouts for presence checks (#39812) ## What changed - Add `FileSystemSandboxPolicy::has_writable_roots_with_cwd` to detect an effective writable root without constructing its read-only carveouts. - Use the helper when classifying permission profiles and selecting patch rejection reasons. ## Testing - Verify the presence check matches the existing materialized-root result across read-only, unrestricted, external, writable, denied, and unresolved special-path policies. GitOrigin-RevId: 17ee490aa3180e05732133203f68544d9de5268b --- codex-rs/core/src/safety.rs | 8 ++--- codex-rs/core/src/sandbox_tags.rs | 5 +-- codex-rs/protocol/src/permissions.rs | 50 ++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index 2e5441a078..7c93938c71 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -102,11 +102,9 @@ fn patch_rejection_reason( file_system_sandbox_policy: &FileSystemSandboxPolicy, cwd: &PathUri, ) -> &'static str { - let has_no_writable_roots = cwd.to_abs_path().is_ok_and(|cwd| { - file_system_sandbox_policy - .get_writable_roots_with_cwd(cwd.as_path()) - .is_empty() - }); + let has_no_writable_roots = cwd + .to_abs_path() + .is_ok_and(|cwd| !file_system_sandbox_policy.has_writable_roots_with_cwd(cwd.as_path())); match permission_profile { PermissionProfile::Managed { .. } if !file_system_sandbox_policy.has_full_disk_write_access() diff --git a/codex-rs/core/src/sandbox_tags.rs b/codex-rs/core/src/sandbox_tags.rs index 2973a5bf94..29d520e2d1 100644 --- a/codex-rs/core/src/sandbox_tags.rs +++ b/codex-rs/core/src/sandbox_tags.rs @@ -48,10 +48,7 @@ pub(crate) fn permission_profile_policy_tag( let file_system_policy = profile.file_system_sandbox_policy(); if file_system_policy.has_full_disk_write_access() { "danger-full-access" - } else if file_system_policy - .get_writable_roots_with_cwd(cwd) - .is_empty() - { + } else if !file_system_policy.has_writable_roots_with_cwd(cwd) { "read-only" } else { "workspace-write" diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index bde83ad18b..1537230e0a 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -1167,6 +1167,18 @@ impl FileSystemSandboxPolicy { self.get_writable_roots_with_cwd_impl(cwd, WritableRootPathResolution::Effective) } + /// Returns whether any effective writable root exists without materializing its carveouts. + pub fn has_writable_roots_with_cwd(&self, cwd: &Path) -> bool { + !self.has_full_disk_write_access() + && self + .resolved_entries_with_cwd(cwd) + .into_iter() + .any(|entry| { + entry.access.can_write() + && self.can_write_path_with_cwd(entry.path.as_path(), cwd) + }) + } + /// Returns writable roots without following attacker-mutable path components. /// /// Trusted top-level aliases such as `/tmp -> /private/tmp` are still @@ -2173,6 +2185,44 @@ mod tests { } } + #[test] + fn writable_root_presence_matches_materialized_roots() { + let cwd = TempDir::new().expect("tempdir"); + let writable_root = AbsolutePathBuf::resolve_path_against_base("work", cwd.path()); + let policies = [ + FileSystemSandboxPolicy::read_only(), + FileSystemSandboxPolicy::unrestricted(), + FileSystemSandboxPolicy::external_sandbox(), + FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new( + writable_root.clone().into(), + FileSystemAccessMode::Write, + )]), + FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new( + writable_root.clone().into(), + FileSystemAccessMode::Write, + ), + FileSystemSandboxEntry::new(writable_root.into(), FileSystemAccessMode::Deny), + ]), + FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::unknown( + ":future_special_path", + /*subpath*/ None, + ), + }, + FileSystemAccessMode::Write, + )]), + ]; + + for policy in policies { + assert_eq!( + policy.has_writable_roots_with_cwd(cwd.path()), + !policy.get_writable_roots_with_cwd(cwd.path()).is_empty() + ); + } + } + #[cfg(windows)] #[test] fn permission_paths_preserve_native_slash_unc_strings() {