mirror of
https://github.com/openai/codex.git
synced 2026-11-13 22:47:20 +00:00
Ignore symbolic slash-tmp permissions on Windows (#36237)
## Why The `:slash_tmp` filesystem entry represents the Unix `/tmp` directory and should not affect Windows sandbox policy decisions. ## What changed - Ignore symbolic `SlashTmp` entries when resolving Windows filesystem access, converting permission profiles, and intersecting requested permissions. - Continue treating a literal `/tmp` entry as a regular filesystem path. ## Testing Add Windows-specific coverage for policy conversion, access checks, and permission-profile intersection. GitOrigin-RevId: f4cdd37ea41abc9debfe8a0d3a456b70da5e534c
This commit is contained in:
1 parent
3016671bb0
commit
413492cd6c
3 files changed
+144
-9
No files matched your search
@@ -462,10 +462,15 @@ impl FileSystemSandboxPolicy {
|
|||||||
|
|
||||||
pub fn has_denied_read_restrictions(&self) -> bool {
|
pub fn has_denied_read_restrictions(&self) -> bool {
|
||||||
matches!(self.kind, FileSystemSandboxKind::Restricted)
|
matches!(self.kind, FileSystemSandboxKind::Restricted)
|
||||||
&& self
|
&& self.entries.iter().any(|entry| {
|
||||||
.entries
|
entry.access == FileSystemAccessMode::Deny
|
||||||
.iter()
|
&& !matches!(
|
||||||
.any(|entry| entry.access == FileSystemAccessMode::Deny)
|
&entry.path,
|
||||||
|
FileSystemPath::Special {
|
||||||
|
value: FileSystemSpecialPath::SlashTmp,
|
||||||
|
} if !cfg!(unix)
|
||||||
|
)
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn from_legacy_sandbox_policy_preserving_deny_entries(
|
pub fn from_legacy_sandbox_policy_preserving_deny_entries(
|
||||||
@@ -546,6 +551,7 @@ impl FileSystemSandboxPolicy {
|
|||||||
FileSystemPath::GlobPattern { .. } => true,
|
FileSystemPath::GlobPattern { .. } => true,
|
||||||
FileSystemPath::Special { value } => match value {
|
FileSystemPath::Special { value } => match value {
|
||||||
FileSystemSpecialPath::Root => entry.access == FileSystemAccessMode::Deny,
|
FileSystemSpecialPath::Root => entry.access == FileSystemAccessMode::Deny,
|
||||||
|
FileSystemSpecialPath::SlashTmp if !cfg!(unix) => false,
|
||||||
FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => {
|
FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => {
|
||||||
false
|
false
|
||||||
}
|
}
|
||||||
@@ -1249,7 +1255,7 @@ impl FileSystemSandboxPolicy {
|
|||||||
} else if unbridgeable_root_write
|
} else if unbridgeable_root_write
|
||||||
|| !writable_roots.is_empty()
|
|| !writable_roots.is_empty()
|
||||||
|| tmpdir_writable
|
|| tmpdir_writable
|
||||||
|| slash_tmp_writable
|
|| (cfg!(unix) && slash_tmp_writable)
|
||||||
{
|
{
|
||||||
return Err(io::Error::new(
|
return Err(io::Error::new(
|
||||||
io::ErrorKind::InvalidInput,
|
io::ErrorKind::InvalidInput,
|
||||||
@@ -1523,6 +1529,9 @@ fn resolve_file_system_special_path(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
FileSystemSpecialPath::SlashTmp => {
|
FileSystemSpecialPath::SlashTmp => {
|
||||||
|
if !cfg!(unix) {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
#[allow(clippy::expect_used)]
|
#[allow(clippy::expect_used)]
|
||||||
let slash_tmp = AbsolutePathBuf::from_absolute_path("/tmp").expect("/tmp is absolute");
|
let slash_tmp = AbsolutePathBuf::from_absolute_path("/tmp").expect("/tmp is absolute");
|
||||||
if !slash_tmp.as_path().is_dir() {
|
if !slash_tmp.as_path().is_dir() {
|
||||||
@@ -1959,6 +1968,86 @@ mod tests {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(windows)]
|
||||||
|
#[test]
|
||||||
|
fn workspace_write_ignores_slash_tmp_special_path_but_preserves_literal_path() {
|
||||||
|
let cwd = TempDir::new().expect("tempdir");
|
||||||
|
let slash_tmp = AbsolutePathBuf::from_absolute_path("/tmp").expect("absolute tmp path");
|
||||||
|
let slash_tmp_only_policy = FileSystemSandboxPolicy::restricted(vec![
|
||||||
|
FileSystemSandboxEntry::new(
|
||||||
|
FileSystemPath::Special {
|
||||||
|
value: FileSystemSpecialPath::Root,
|
||||||
|
},
|
||||||
|
FileSystemAccessMode::Read,
|
||||||
|
),
|
||||||
|
FileSystemSandboxEntry::new(
|
||||||
|
FileSystemPath::Special {
|
||||||
|
value: FileSystemSpecialPath::SlashTmp,
|
||||||
|
},
|
||||||
|
FileSystemAccessMode::Write,
|
||||||
|
),
|
||||||
|
]);
|
||||||
|
assert_eq!(
|
||||||
|
slash_tmp_only_policy
|
||||||
|
.to_legacy_sandbox_policy(NetworkSandboxPolicy::Restricted, cwd.path())
|
||||||
|
.expect("legacy sandbox policy"),
|
||||||
|
SandboxPolicy::ReadOnly {
|
||||||
|
network_access: false,
|
||||||
|
}
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!slash_tmp_only_policy
|
||||||
|
.needs_direct_runtime_enforcement(NetworkSandboxPolicy::Restricted, cwd.path())
|
||||||
|
);
|
||||||
|
|
||||||
|
for access in [FileSystemAccessMode::Read, FileSystemAccessMode::Deny] {
|
||||||
|
let policy = FileSystemSandboxPolicy::restricted(vec![
|
||||||
|
FileSystemSandboxEntry::new(
|
||||||
|
FileSystemPath::Special {
|
||||||
|
value: FileSystemSpecialPath::Root,
|
||||||
|
},
|
||||||
|
FileSystemAccessMode::Write,
|
||||||
|
),
|
||||||
|
FileSystemSandboxEntry::new(
|
||||||
|
FileSystemPath::Special {
|
||||||
|
value: FileSystemSpecialPath::SlashTmp,
|
||||||
|
},
|
||||||
|
access,
|
||||||
|
),
|
||||||
|
]);
|
||||||
|
assert!(policy.has_full_disk_write_access());
|
||||||
|
assert!(policy.has_full_disk_read_access());
|
||||||
|
}
|
||||||
|
|
||||||
|
let legacy_policy = SandboxPolicy::new_workspace_write_policy();
|
||||||
|
assert_eq!(
|
||||||
|
FileSystemSandboxPolicy::from(&legacy_policy)
|
||||||
|
.to_legacy_sandbox_policy(NetworkSandboxPolicy::Restricted, cwd.path())
|
||||||
|
.expect("legacy workspace-write policy"),
|
||||||
|
legacy_policy
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
FileSystemSandboxPolicy::from(&legacy_policy)
|
||||||
|
.get_writable_roots_with_cwd(cwd.path())
|
||||||
|
.into_iter()
|
||||||
|
.map(|root| root.root)
|
||||||
|
.collect::<Vec<_>>(),
|
||||||
|
legacy_policy
|
||||||
|
.get_writable_roots_with_cwd(cwd.path())
|
||||||
|
.into_iter()
|
||||||
|
.map(|root| normalize_effective_absolute_path(root.root))
|
||||||
|
.collect::<Vec<_>>()
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
FileSystemSandboxPolicy::workspace_write(
|
||||||
|
std::slice::from_ref(&slash_tmp),
|
||||||
|
/*exclude_tmpdir_env_var*/ true,
|
||||||
|
/*exclude_slash_tmp*/ false,
|
||||||
|
)
|
||||||
|
.can_write_path_with_cwd(slash_tmp.as_path(), cwd.path())
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(unix)]
|
#[cfg(unix)]
|
||||||
#[test]
|
#[test]
|
||||||
fn writable_roots_proactively_protect_missing_dot_codex() {
|
fn writable_roots_proactively_protect_missing_dot_codex() {
|
||||||
|
|||||||
@@ -244,7 +244,14 @@ fn granted_file_system_entry_within_request(
|
|||||||
granted_entry: &FileSystemSandboxEntry,
|
granted_entry: &FileSystemSandboxEntry,
|
||||||
cwd: &Path,
|
cwd: &Path,
|
||||||
) -> bool {
|
) -> bool {
|
||||||
if !granted_entry.access.can_read() {
|
if !granted_entry.access.can_read()
|
||||||
|
|| matches!(
|
||||||
|
&granted_entry.path,
|
||||||
|
FileSystemPath::Special {
|
||||||
|
value: FileSystemSpecialPath::SlashTmp,
|
||||||
|
} if !cfg!(unix)
|
||||||
|
)
|
||||||
|
{
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -398,10 +405,14 @@ fn resolve_permission_path(path: &FileSystemPath, cwd: &Path) -> Option<Absolute
|
|||||||
AbsolutePathBuf::from_absolute_path(PathBuf::from(tmpdir)).ok()
|
AbsolutePathBuf::from_absolute_path(PathBuf::from(tmpdir)).ok()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
FileSystemSpecialPath::SlashTmp => AbsolutePathBuf::from_absolute_path("/tmp")
|
FileSystemSpecialPath::SlashTmp if cfg!(unix) => {
|
||||||
|
AbsolutePathBuf::from_absolute_path("/tmp")
|
||||||
.ok()
|
.ok()
|
||||||
.filter(|path| path.as_path().is_dir()),
|
.filter(|path| path.as_path().is_dir())
|
||||||
FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => None,
|
}
|
||||||
|
FileSystemSpecialPath::SlashTmp
|
||||||
|
| FileSystemSpecialPath::Minimal
|
||||||
|
| FileSystemSpecialPath::Unknown { .. } => None,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -294,6 +294,41 @@ fn intersect_permission_profiles_drops_explicit_empty_reads_without_grant() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(windows)]
|
||||||
|
#[test]
|
||||||
|
fn intersect_permission_profiles_rejects_symbolic_slash_tmp_grants() {
|
||||||
|
let cwd = TempDir::new().expect("tempdir");
|
||||||
|
let slash_tmp = AbsolutePathBuf::from_absolute_path("/tmp").expect("absolute tmp path");
|
||||||
|
let requested = PermissionProfile {
|
||||||
|
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||||
|
/*read*/ None,
|
||||||
|
Some(vec![slash_tmp]),
|
||||||
|
)),
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
let granted = PermissionProfile {
|
||||||
|
file_system: Some(FileSystemPermissions {
|
||||||
|
entries: vec![FileSystemSandboxEntry::new(
|
||||||
|
FileSystemPath::Special {
|
||||||
|
value: FileSystemSpecialPath::SlashTmp,
|
||||||
|
},
|
||||||
|
FileSystemAccessMode::Write,
|
||||||
|
)],
|
||||||
|
glob_scan_max_depth: None,
|
||||||
|
}),
|
||||||
|
..Default::default()
|
||||||
|
};
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
intersect_permission_profiles(requested, granted.clone(), cwd.path()),
|
||||||
|
PermissionProfile::default()
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
intersect_permission_profiles(granted.clone(), granted, cwd.path()),
|
||||||
|
PermissionProfile::default()
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn intersect_permission_profiles_accepts_child_path_granted_for_requested_cwd() {
|
fn intersect_permission_profiles_accepts_child_path_granted_for_requested_cwd() {
|
||||||
let temp_dir = TempDir::new().expect("create temp dir");
|
let temp_dir = TempDir::new().expect("create temp dir");
|
||||||
|
|||||||
Reference in new issue
Block a user