mirror of
https://github.com/openai/codex.git
synced 2026-11-13 22:47:20 +00:00
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
This commit is contained in:
1 parent
21facf2273
commit
969efa5470
3 files changed
+54
-9
No files matched your search
@@ -102,11 +102,9 @@ fn patch_rejection_reason(
|
|||||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||||
cwd: &PathUri,
|
cwd: &PathUri,
|
||||||
) -> &'static str {
|
) -> &'static str {
|
||||||
let has_no_writable_roots = cwd.to_abs_path().is_ok_and(|cwd| {
|
let has_no_writable_roots = cwd
|
||||||
file_system_sandbox_policy
|
.to_abs_path()
|
||||||
.get_writable_roots_with_cwd(cwd.as_path())
|
.is_ok_and(|cwd| !file_system_sandbox_policy.has_writable_roots_with_cwd(cwd.as_path()));
|
||||||
.is_empty()
|
|
||||||
});
|
|
||||||
match permission_profile {
|
match permission_profile {
|
||||||
PermissionProfile::Managed { .. }
|
PermissionProfile::Managed { .. }
|
||||||
if !file_system_sandbox_policy.has_full_disk_write_access()
|
if !file_system_sandbox_policy.has_full_disk_write_access()
|
||||||
|
|||||||
@@ -48,10 +48,7 @@ pub(crate) fn permission_profile_policy_tag(
|
|||||||
let file_system_policy = profile.file_system_sandbox_policy();
|
let file_system_policy = profile.file_system_sandbox_policy();
|
||||||
if file_system_policy.has_full_disk_write_access() {
|
if file_system_policy.has_full_disk_write_access() {
|
||||||
"danger-full-access"
|
"danger-full-access"
|
||||||
} else if file_system_policy
|
} else if !file_system_policy.has_writable_roots_with_cwd(cwd) {
|
||||||
.get_writable_roots_with_cwd(cwd)
|
|
||||||
.is_empty()
|
|
||||||
{
|
|
||||||
"read-only"
|
"read-only"
|
||||||
} else {
|
} else {
|
||||||
"workspace-write"
|
"workspace-write"
|
||||||
|
|||||||
@@ -1167,6 +1167,18 @@ impl FileSystemSandboxPolicy {
|
|||||||
self.get_writable_roots_with_cwd_impl(cwd, WritableRootPathResolution::Effective)
|
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.
|
/// Returns writable roots without following attacker-mutable path components.
|
||||||
///
|
///
|
||||||
/// Trusted top-level aliases such as `/tmp -> /private/tmp` are still
|
/// 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)]
|
#[cfg(windows)]
|
||||||
#[test]
|
#[test]
|
||||||
fn permission_paths_preserve_native_slash_unc_strings() {
|
fn permission_paths_preserve_native_slash_unc_strings() {
|
||||||
|
|||||||
Reference in new issue
Block a user