mirror of
https://github.com/openai/codex.git
synced 2026-09-05 15:18:41 +00:00
refactor(permissions): lower Windows enforcement from effective filesystem permissions
Co-authored-by: Codex noreply@openai.com
This commit is contained in:
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -4148,6 +4148,7 @@ dependencies = [
|
||||
"chrono",
|
||||
"codex-otel",
|
||||
"codex-protocol",
|
||||
"codex-sandboxing",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-pty",
|
||||
"codex-utils-string",
|
||||
|
||||
@@ -31,6 +31,7 @@ codex-utils-pty = { workspace = true }
|
||||
codex-utils-absolute-path = { workspace = true }
|
||||
codex-utils-string = { workspace = true }
|
||||
codex-otel = { workspace = true }
|
||||
codex-sandboxing = { workspace = true }
|
||||
dunce = "1.0"
|
||||
glob = { workspace = true }
|
||||
serde = { version = "1.0", features = ["derive"] }
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::permissions::ReadDenyMatcher;
|
||||
use codex_sandboxing::EffectiveFilesystemPermissions;
|
||||
use codex_sandboxing::FilesystemPermissionsContext;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
@@ -23,15 +27,41 @@ struct GlobScanPlan {
|
||||
pub fn resolve_windows_deny_read_paths(
|
||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
cwd: &AbsolutePathBuf,
|
||||
) -> Result<Vec<AbsolutePathBuf>, String> {
|
||||
let file_system_sandbox_policy = file_system_sandbox_policy
|
||||
.clone()
|
||||
.materialize_project_roots_with_workspace_roots(std::slice::from_ref(cwd));
|
||||
let permission_profile = PermissionProfile::from_runtime_permissions(
|
||||
&file_system_sandbox_policy,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
);
|
||||
let effective_file_system = EffectiveFilesystemPermissions::from_profile(
|
||||
&permission_profile,
|
||||
FilesystemPermissionsContext {
|
||||
policy_evaluation_cwd: cwd,
|
||||
},
|
||||
)
|
||||
.map_err(|err| err.to_string())?;
|
||||
resolve_windows_deny_read_paths_from_effective_permissions(&effective_file_system, cwd)
|
||||
}
|
||||
|
||||
/// Resolves effective read-deny entries into concrete Windows ACL targets.
|
||||
pub fn resolve_windows_deny_read_paths_from_effective_permissions(
|
||||
effective_file_system: &EffectiveFilesystemPermissions,
|
||||
cwd: &AbsolutePathBuf,
|
||||
) -> Result<Vec<AbsolutePathBuf>, String> {
|
||||
let mut paths = Vec::new();
|
||||
let mut seen = HashSet::new();
|
||||
|
||||
for path in file_system_sandbox_policy.get_unreadable_roots_with_cwd(cwd.as_path()) {
|
||||
push_absolute_path(&mut paths, &mut seen, path.into_path_buf())?;
|
||||
for path in &effective_file_system.unreadable_roots {
|
||||
push_absolute_path(&mut paths, &mut seen, path.to_path_buf())?;
|
||||
}
|
||||
|
||||
let unreadable_globs = file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd.as_path());
|
||||
let unreadable_globs = effective_file_system
|
||||
.unreadable_globs
|
||||
.iter()
|
||||
.map(|glob| glob.pattern().to_string())
|
||||
.collect::<Vec<_>>();
|
||||
if unreadable_globs.is_empty() {
|
||||
return Ok(paths);
|
||||
}
|
||||
@@ -53,7 +83,7 @@ pub fn resolve_windows_deny_read_paths(
|
||||
|
||||
for pattern in unreadable_globs {
|
||||
let mut seen_scan_dirs = HashSet::new();
|
||||
let scan_plan = glob_scan_plan(&pattern, file_system_sandbox_policy.glob_scan_max_depth);
|
||||
let scan_plan = glob_scan_plan(&pattern, effective_file_system.glob_scan_max_depth);
|
||||
collect_existing_glob_matches(
|
||||
&scan_plan.root,
|
||||
&matcher,
|
||||
|
||||
@@ -124,6 +124,7 @@ pub use deny_read_acl::apply_deny_read_acls;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use deny_read_acl::plan_deny_read_acl_paths;
|
||||
pub use deny_read_resolver::resolve_windows_deny_read_paths;
|
||||
pub use deny_read_resolver::resolve_windows_deny_read_paths_from_effective_permissions;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use deny_read_state::sync_persistent_deny_read_acls;
|
||||
#[cfg(target_os = "windows")]
|
||||
|
||||
@@ -1,10 +1,14 @@
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxKind;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::WritableRoot;
|
||||
use codex_sandboxing::EffectiveFilesystemPermissions;
|
||||
use codex_sandboxing::FilesystemPermissionsContext;
|
||||
use codex_sandboxing::FilesystemPermissionsMode;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
@@ -15,9 +19,11 @@ use std::path::PathBuf;
|
||||
/// Most Windows sandbox code needs resolved runtime permissions plus a few
|
||||
/// Windows-specific path conventions, not the user/config-facing
|
||||
/// `PermissionProfile` enum itself.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
#[derive(Debug)]
|
||||
pub struct ResolvedWindowsSandboxPermissions {
|
||||
file_system: FileSystemSandboxPolicy,
|
||||
effective_file_system: EffectiveFilesystemPermissions,
|
||||
writable_roots: Vec<WritableRoot>,
|
||||
has_writable_tmpdir_entry: bool,
|
||||
network: NetworkSandboxPolicy,
|
||||
}
|
||||
|
||||
@@ -44,7 +50,10 @@ pub fn token_mode_for_permission_profile(
|
||||
permission_profile,
|
||||
cwd,
|
||||
)?;
|
||||
if permissions.file_system.has_full_disk_write_access() {
|
||||
if permissions
|
||||
.effective_file_system
|
||||
.has_full_disk_write_access()
|
||||
{
|
||||
anyhow::bail!(
|
||||
"permission profile requests full-disk filesystem writes, which cannot be enforced by the Windows sandbox"
|
||||
);
|
||||
@@ -57,37 +66,81 @@ pub fn token_mode_for_permission_profile(
|
||||
}
|
||||
|
||||
impl ResolvedWindowsSandboxPermissions {
|
||||
pub fn try_from_permission_profile(permission_profile: &PermissionProfile) -> Result<Self> {
|
||||
/// Resolves a managed permission profile for the Windows compatibility boundary.
|
||||
///
|
||||
/// Normal runtime callers provide already-materialized workspace roots. For
|
||||
/// callers that still provide symbolic `:workspace_roots`, this adapter
|
||||
/// explicitly binds them to the permission root supplied by the caller.
|
||||
pub fn try_from_permission_profile_for_cwd(
|
||||
permission_profile: &PermissionProfile,
|
||||
cwd: &Path,
|
||||
) -> Result<Self> {
|
||||
if !matches!(permission_profile, PermissionProfile::Managed { .. }) {
|
||||
anyhow::bail!(
|
||||
"only managed permission profiles can be enforced by the Windows sandbox"
|
||||
);
|
||||
}
|
||||
let permission_profile_cwd = AbsolutePathBuf::from_absolute_path(cwd)
|
||||
.context("permission profile cwd must be absolute for the Windows sandbox")?;
|
||||
let permission_profile = permission_profile
|
||||
.clone()
|
||||
.materialize_project_roots_with_workspace_roots(std::slice::from_ref(
|
||||
&permission_profile_cwd,
|
||||
));
|
||||
let (file_system, network) = permission_profile.to_runtime_permissions();
|
||||
if !matches!(file_system.kind, FileSystemSandboxKind::Restricted) {
|
||||
anyhow::bail!(
|
||||
"only restricted managed filesystem permissions can be enforced by the Windows sandbox"
|
||||
);
|
||||
}
|
||||
let effective_file_system = EffectiveFilesystemPermissions::from_profile(
|
||||
&permission_profile,
|
||||
FilesystemPermissionsContext {
|
||||
policy_evaluation_cwd: &permission_profile_cwd,
|
||||
},
|
||||
)?;
|
||||
let has_writable_tmpdir_entry =
|
||||
file_system
|
||||
.entries
|
||||
.iter()
|
||||
.any(|FileSystemSandboxEntry { path, access }| {
|
||||
matches!(
|
||||
path,
|
||||
FileSystemPath::Special {
|
||||
value: codex_protocol::permissions::FileSystemSpecialPath::Tmpdir,
|
||||
}
|
||||
) && access.can_write()
|
||||
});
|
||||
let mut windows_writable_file_system = file_system;
|
||||
windows_writable_file_system
|
||||
.entries
|
||||
.retain(|FileSystemSandboxEntry { path, .. }| {
|
||||
!matches!(
|
||||
path,
|
||||
FileSystemPath::Special {
|
||||
value: codex_protocol::permissions::FileSystemSpecialPath::Tmpdir
|
||||
| codex_protocol::permissions::FileSystemSpecialPath::SlashTmp,
|
||||
}
|
||||
)
|
||||
});
|
||||
let windows_writable_profile =
|
||||
PermissionProfile::from_runtime_permissions(&windows_writable_file_system, network);
|
||||
let writable_roots = EffectiveFilesystemPermissions::from_profile(
|
||||
&windows_writable_profile,
|
||||
FilesystemPermissionsContext {
|
||||
policy_evaluation_cwd: &permission_profile_cwd,
|
||||
},
|
||||
)?
|
||||
.writable_roots;
|
||||
|
||||
Ok(Self {
|
||||
file_system,
|
||||
effective_file_system,
|
||||
writable_roots,
|
||||
has_writable_tmpdir_entry,
|
||||
network,
|
||||
})
|
||||
}
|
||||
|
||||
/// Resolves a managed permission profile and binds symbolic `:workspace_roots`
|
||||
/// entries to the permission root supplied by the caller.
|
||||
pub fn try_from_permission_profile_for_cwd(
|
||||
permission_profile: &PermissionProfile,
|
||||
cwd: &Path,
|
||||
) -> Result<Self> {
|
||||
let mut permissions = Self::try_from_permission_profile(permission_profile)?;
|
||||
permissions.file_system = permissions
|
||||
.file_system
|
||||
.materialize_project_roots_with_cwd(cwd);
|
||||
Ok(permissions)
|
||||
}
|
||||
|
||||
pub(crate) fn should_apply_network_block(&self) -> bool {
|
||||
!self.network.is_enabled()
|
||||
}
|
||||
@@ -97,21 +150,25 @@ impl ResolvedWindowsSandboxPermissions {
|
||||
}
|
||||
|
||||
pub(crate) fn is_enforceable_by_windows_sandbox(&self) -> bool {
|
||||
matches!(self.file_system.kind, FileSystemSandboxKind::Restricted)
|
||||
matches!(
|
||||
self.effective_file_system.mode,
|
||||
FilesystemPermissionsMode::Restricted
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn has_full_disk_read_access(&self) -> bool {
|
||||
self.file_system.has_full_disk_read_access()
|
||||
self.effective_file_system.has_full_disk_read_access()
|
||||
}
|
||||
|
||||
pub(crate) fn include_platform_defaults(&self) -> bool {
|
||||
self.file_system.include_platform_defaults()
|
||||
self.effective_file_system.include_platform_defaults
|
||||
}
|
||||
|
||||
pub(crate) fn readable_roots_for_cwd(&self, cwd: &Path) -> Vec<PathBuf> {
|
||||
self.file_system
|
||||
.get_readable_roots_with_cwd(cwd)
|
||||
.into_iter()
|
||||
pub(crate) fn readable_roots_for_cwd(&self, _cwd: &Path) -> Vec<PathBuf> {
|
||||
self.effective_file_system
|
||||
.readable_roots
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(AbsolutePathBuf::into_path_buf)
|
||||
.collect()
|
||||
}
|
||||
@@ -126,25 +183,13 @@ impl ResolvedWindowsSandboxPermissions {
|
||||
|
||||
pub(crate) fn writable_roots_for_cwd(
|
||||
&self,
|
||||
cwd: &Path,
|
||||
_cwd: &Path,
|
||||
env_map: &HashMap<String, String>,
|
||||
) -> Vec<WindowsWritableRoot> {
|
||||
let mut file_system = self.file_system.clone();
|
||||
file_system
|
||||
.entries
|
||||
.retain(|FileSystemSandboxEntry { path, .. }| {
|
||||
!matches!(
|
||||
path,
|
||||
FileSystemPath::Special {
|
||||
value: codex_protocol::permissions::FileSystemSpecialPath::Tmpdir
|
||||
| codex_protocol::permissions::FileSystemSpecialPath::SlashTmp,
|
||||
}
|
||||
)
|
||||
});
|
||||
|
||||
let mut roots = file_system
|
||||
.get_writable_roots_with_cwd(cwd)
|
||||
.into_iter()
|
||||
let mut roots = self
|
||||
.writable_roots
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(|root| WindowsWritableRoot {
|
||||
root: root.root.into_path_buf(),
|
||||
read_only_subpaths: root
|
||||
@@ -155,7 +200,7 @@ impl ResolvedWindowsSandboxPermissions {
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
if self.has_writable_tmpdir_entry() {
|
||||
if self.has_writable_tmpdir_entry {
|
||||
roots.extend(windows_temp_env_roots(env_map).into_iter().map(|root| {
|
||||
WindowsWritableRoot {
|
||||
root,
|
||||
@@ -166,20 +211,6 @@ impl ResolvedWindowsSandboxPermissions {
|
||||
|
||||
roots
|
||||
}
|
||||
|
||||
fn has_writable_tmpdir_entry(&self) -> bool {
|
||||
self.file_system
|
||||
.entries
|
||||
.iter()
|
||||
.any(|FileSystemSandboxEntry { path, access }| {
|
||||
matches!(
|
||||
path,
|
||||
FileSystemPath::Special {
|
||||
value: codex_protocol::permissions::FileSystemSpecialPath::Tmpdir,
|
||||
}
|
||||
) && access.can_write()
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
fn windows_temp_env_roots(env_map: &HashMap<String, String>) -> Vec<PathBuf> {
|
||||
@@ -217,8 +248,9 @@ mod tests {
|
||||
env_map.insert("TEMP".to_string(), temp_dir.to_string_lossy().to_string());
|
||||
env_map.insert("TMP".to_string(), temp_dir.to_string_lossy().to_string());
|
||||
|
||||
let permissions = ResolvedWindowsSandboxPermissions::try_from_permission_profile(
|
||||
let permissions = ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_cwd(
|
||||
&PermissionProfile::workspace_write(),
|
||||
&cwd,
|
||||
)
|
||||
.expect("managed permission profile");
|
||||
let roots = permissions
|
||||
@@ -308,8 +340,9 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn permission_profile_rejects_disabled_profiles() {
|
||||
let err = ResolvedWindowsSandboxPermissions::try_from_permission_profile(
|
||||
let err = ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_cwd(
|
||||
&PermissionProfile::Disabled,
|
||||
Path::new("/"),
|
||||
)
|
||||
.expect_err("disabled profile should not resolve for sandbox enforcement");
|
||||
|
||||
@@ -326,9 +359,11 @@ mod tests {
|
||||
network: NetworkSandboxPolicy::Restricted,
|
||||
};
|
||||
|
||||
let err =
|
||||
ResolvedWindowsSandboxPermissions::try_from_permission_profile(&permission_profile)
|
||||
.expect_err("unrestricted profile should not resolve for sandbox enforcement");
|
||||
let err = ResolvedWindowsSandboxPermissions::try_from_permission_profile_for_cwd(
|
||||
&permission_profile,
|
||||
Path::new("/"),
|
||||
)
|
||||
.expect_err("unrestricted profile should not resolve for sandbox enforcement");
|
||||
|
||||
assert!(
|
||||
err.to_string()
|
||||
|
||||
Reference in New Issue
Block a user