From 0dc8cf86c9276948321e330d4bf50aa0a965e8fe Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 27 May 2026 17:41:02 -0700 Subject: [PATCH] refactor(permissions): lower Seatbelt from effective filesystem permissions Co-authored-by: Codex noreply@openai.com --- codex-rs/cli/src/debug_sandbox.rs | 20 ++++-- codex-rs/sandboxing/src/manager.rs | 3 +- codex-rs/sandboxing/src/seatbelt.rs | 78 +++++++++++++---------- codex-rs/sandboxing/src/seatbelt_tests.rs | 48 ++++++++++---- 4 files changed, 100 insertions(+), 49 deletions(-) diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 36c5e683b2..5f8083f00a 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -17,6 +17,10 @@ use codex_core::spawn::CODEX_SANDBOX_ENV_VAR; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use codex_protocol::config_types::SandboxMode; use codex_protocol::permissions::NetworkSandboxPolicy; +#[cfg(target_os = "macos")] +use codex_sandboxing::EffectiveFilesystemPermissions; +#[cfg(target_os = "macos")] +use codex_sandboxing::FilesystemPermissionsContext; use codex_sandboxing::landlock::allow_network_for_proxy; use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_permission_profile; #[cfg(target_os = "macos")] @@ -260,13 +264,21 @@ async fn run_command_under_sandbox( let mut child = match sandbox_type { #[cfg(target_os = "macos")] SandboxType::Seatbelt => { - let file_system_sandbox_policy = config.permissions.file_system_sandbox_policy(); - let network_sandbox_policy = config.permissions.network_sandbox_policy(); + let permission_profile = config.permissions.effective_permission_profile(); + let network_sandbox_policy = permission_profile.network_sandbox_policy(); + let effective_filesystem_permissions = EffectiveFilesystemPermissions::from_profile( + &permission_profile, + FilesystemPermissionsContext { + policy_evaluation_cwd: &permission_profile_cwd, + }, + ) + .map_err(|err| { + anyhow::anyhow!("failed to derive effective filesystem permissions: {err}") + })?; let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { command, - file_system_sandbox_policy: &file_system_sandbox_policy, + effective_filesystem_permissions: &effective_filesystem_permissions, network_sandbox_policy, - sandbox_policy_cwd: permission_profile_cwd.as_path(), enforce_managed_network: false, network: network.as_ref(), extra_allow_unix_sockets: allow_unix_sockets, diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index 8dc909f751..7296955a49 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -217,9 +217,8 @@ impl SandboxManager { let mut args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { command: os_argv_to_strings(argv), - file_system_sandbox_policy: &effective_file_system_policy, + effective_filesystem_permissions: &effective_filesystem_permissions, network_sandbox_policy: effective_network_policy, - sandbox_policy_cwd, enforce_managed_network, network, extra_allow_unix_sockets: &[], diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs index 7057ec46b9..1bd33dd249 100644 --- a/codex-rs/sandboxing/src/seatbelt.rs +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -1,7 +1,10 @@ +use crate::EffectiveFilesystemPermissions; +use crate::FilesystemPermissionsContext; use codex_network_proxy::NetworkProxy; use codex_network_proxy::PROXY_URL_ENV_KEYS; use codex_network_proxy::has_proxy_url_env_vars; use codex_network_proxy::proxy_url_env_value; +use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::permissions::PROTECTED_METADATA_PATH_NAMES; @@ -404,9 +407,8 @@ fn seatbelt_protected_metadata_name_regex(root: &AbsolutePathBuf, name: &str) -> } fn protected_metadata_names_for_writable_root( - file_system_sandbox_policy: &FileSystemSandboxPolicy, + effective_filesystem_permissions: &EffectiveFilesystemPermissions, writable_root: &WritableRoot, - cwd: &Path, ) -> Vec { let mut names = writable_root.protected_metadata_names.clone(); for name in PROTECTED_METADATA_PATH_NAMES { @@ -414,7 +416,7 @@ fn protected_metadata_names_for_writable_root( continue; } let path = writable_root.root.join(*name); - if !file_system_sandbox_policy.can_write_path_with_cwd(path.as_path(), cwd) { + if !effective_filesystem_permissions.can_write(path.as_path()) { names.push((*name).to_string()); } } @@ -422,25 +424,24 @@ fn protected_metadata_names_for_writable_root( } fn build_seatbelt_unreadable_glob_policy( - file_system_sandbox_policy: &FileSystemSandboxPolicy, - cwd: &Path, + effective_filesystem_permissions: &EffectiveFilesystemPermissions, ) -> String { // Seatbelt does not understand the filesystem policy's glob syntax directly. // Convert each unreadable pattern into an anchored regex deny rule and apply // it to both reads and unlink-style writes so a denied path cannot be probed // through destructive filesystem operations. - let unreadable_globs = file_system_sandbox_policy.get_unreadable_globs_with_cwd(cwd); - if unreadable_globs.is_empty() { + if effective_filesystem_permissions.unreadable_globs.is_empty() { return String::new(); } let mut policy_components = Vec::new(); - for pattern in unreadable_globs { + for glob in &effective_filesystem_permissions.unreadable_globs { + let pattern = glob.pattern(); let mut regexes = BTreeSet::new(); - if let Some(regex) = seatbelt_regex_for_unreadable_glob(&pattern) { + if let Some(regex) = seatbelt_regex_for_unreadable_glob(pattern) { regexes.insert(regex); } - if let Some(pattern) = canonicalize_glob_static_prefix_for_sandbox(&pattern) + if let Some(pattern) = canonicalize_glob_static_prefix_for_sandbox(pattern) && let Some(regex) = seatbelt_regex_for_unreadable_glob(&pattern) { regexes.insert(regex); @@ -573,15 +574,31 @@ fn create_seatbelt_command_args_for_legacy_policy( enforce_managed_network: bool, network: Option<&NetworkProxy>, ) -> Vec { + let legacy_workspace_root = AbsolutePathBuf::from_absolute_path(sandbox_policy_cwd) + .unwrap_or_else(|err| panic!("sandbox policy cwd should be absolute: {err}")); let file_system_sandbox_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( sandbox_policy, sandbox_policy_cwd, + ) + .materialize_project_roots_with_workspace_roots(std::slice::from_ref(&legacy_workspace_root)); + let network_sandbox_policy = NetworkSandboxPolicy::from(sandbox_policy); + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + network_sandbox_policy, ); + let effective_filesystem_permissions = EffectiveFilesystemPermissions::from_profile( + &permission_profile, + FilesystemPermissionsContext { + policy_evaluation_cwd: &legacy_workspace_root, + }, + ) + .unwrap_or_else(|err| { + panic!("legacy filesystem policy should yield effective permissions: {err}") + }); create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { command, - file_system_sandbox_policy: &file_system_sandbox_policy, - network_sandbox_policy: NetworkSandboxPolicy::from(sandbox_policy), - sandbox_policy_cwd, + effective_filesystem_permissions: &effective_filesystem_permissions, + network_sandbox_policy, enforce_managed_network, network, extra_allow_unix_sockets: &[], @@ -591,9 +608,8 @@ fn create_seatbelt_command_args_for_legacy_policy( #[derive(Debug)] pub struct CreateSeatbeltCommandArgsParams<'a> { pub command: Vec, - pub file_system_sandbox_policy: &'a FileSystemSandboxPolicy, + pub effective_filesystem_permissions: &'a EffectiveFilesystemPermissions, pub network_sandbox_policy: NetworkSandboxPolicy, - pub sandbox_policy_cwd: &'a Path, pub enforce_managed_network: bool, pub network: Option<&'a NetworkProxy>, pub extra_allow_unix_sockets: &'a [AbsolutePathBuf], @@ -602,18 +618,16 @@ pub struct CreateSeatbeltCommandArgsParams<'a> { pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -> Vec { let CreateSeatbeltCommandArgsParams { command, - file_system_sandbox_policy, + effective_filesystem_permissions, network_sandbox_policy, - sandbox_policy_cwd, enforce_managed_network, network, extra_allow_unix_sockets, } = args; - let unreadable_roots = - file_system_sandbox_policy.get_unreadable_roots_with_cwd(sandbox_policy_cwd); + let unreadable_roots = effective_filesystem_permissions.unreadable_roots.clone(); let (file_write_policy, file_write_dir_params) = - if file_system_sandbox_policy.has_full_disk_write_access() { + if effective_filesystem_permissions.has_full_disk_write_access() { if unreadable_roots.is_empty() { // Allegedly, this is more permissive than `(allow file-write*)`. ( @@ -635,14 +649,14 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) - build_seatbelt_access_policy( "file-write*", "WRITABLE_ROOT", - file_system_sandbox_policy - .get_writable_roots_with_cwd(sandbox_policy_cwd) - .into_iter() + effective_filesystem_permissions + .writable_roots + .iter() + .cloned() .map(|root| SeatbeltAccessRoot { protected_metadata_names: protected_metadata_names_for_writable_root( - file_system_sandbox_policy, + effective_filesystem_permissions, &root, - sandbox_policy_cwd, ), root: root.root, excluded_subpaths: root.read_only_subpaths, @@ -652,7 +666,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) - }; let (file_read_policy, file_read_dir_params) = - if file_system_sandbox_policy.has_full_disk_read_access() { + if effective_filesystem_permissions.has_full_disk_read_access() { if unreadable_roots.is_empty() { ( "; allow read-only file operations\n(allow file-read*)".to_string(), @@ -677,9 +691,10 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) - let (policy, params) = build_seatbelt_access_policy( "file-read*", "READABLE_ROOT", - file_system_sandbox_policy - .get_readable_roots_with_cwd(sandbox_policy_cwd) - .into_iter() + effective_filesystem_permissions + .readable_roots + .iter() + .cloned() .map(|root| SeatbeltAccessRoot { excluded_subpaths: unreadable_roots .iter() @@ -705,9 +720,8 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) - let network_policy = dynamic_network_policy_for_network(network_sandbox_policy, enforce_managed_network, &proxy); - let include_platform_defaults = file_system_sandbox_policy.include_platform_defaults(); - let deny_read_policy = - build_seatbelt_unreadable_glob_policy(file_system_sandbox_policy, sandbox_policy_cwd); + let include_platform_defaults = effective_filesystem_permissions.include_platform_defaults; + let deny_read_policy = build_seatbelt_unreadable_glob_policy(effective_filesystem_permissions); let mut policy_sections = vec![ MACOS_SEATBELT_BASE_POLICY.to_string(), file_read_policy, diff --git a/codex-rs/sandboxing/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs index d905d0adcf..1807368ef1 100644 --- a/codex-rs/sandboxing/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -19,6 +19,7 @@ use codex_network_proxy::NetworkProxyConfig; use codex_network_proxy::NetworkProxyConstraints; use codex_network_proxy::NetworkProxyState; use codex_network_proxy::build_config_state; +use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; @@ -50,6 +51,30 @@ fn absolute_path(path: &str) -> AbsolutePathBuf { AbsolutePathBuf::from_absolute_path(Path::new(path)).expect("absolute path") } +fn effective_permissions( + file_system_policy: &FileSystemSandboxPolicy, + cwd: &Path, +) -> crate::EffectiveFilesystemPermissions { + let policy_evaluation_cwd = + AbsolutePathBuf::from_absolute_path(cwd).expect("absolute sandbox policy cwd"); + let file_system_policy = file_system_policy + .clone() + .materialize_project_roots_with_workspace_roots(std::slice::from_ref( + &policy_evaluation_cwd, + )); + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system_policy, + NetworkSandboxPolicy::Restricted, + ); + crate::EffectiveFilesystemPermissions::from_profile( + &permission_profile, + crate::FilesystemPermissionsContext { + policy_evaluation_cwd: &policy_evaluation_cwd, + }, + ) + .expect("derive effective filesystem permissions") +} + fn seatbelt_policy_arg(args: &[String]) -> &str { let policy_index = args .iter() @@ -198,12 +223,12 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access() access: FileSystemAccessMode::Deny, }, ]); + let effective_permissions = effective_permissions(&file_system_policy, Path::new("/")); let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { command: vec!["/bin/true".to_string()], - file_system_sandbox_policy: &file_system_policy, + effective_filesystem_permissions: &effective_permissions, network_sandbox_policy: NetworkSandboxPolicy::Restricted, - sandbox_policy_cwd: Path::new("/"), enforce_managed_network: false, network: None, extra_allow_unix_sockets: &[], @@ -270,12 +295,12 @@ fn explicit_unreadable_paths_are_excluded_from_readable_roots() { access: FileSystemAccessMode::Deny, }, ]); + let effective_permissions = effective_permissions(&file_system_policy, Path::new("/")); let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { command: vec!["/bin/true".to_string()], - file_system_sandbox_policy: &file_system_policy, + effective_filesystem_permissions: &effective_permissions, network_sandbox_policy: NetworkSandboxPolicy::Restricted, - sandbox_policy_cwd: Path::new("/"), enforce_managed_network: false, network: None, extra_allow_unix_sockets: &[], @@ -374,8 +399,9 @@ fn unreadable_glob_policy_includes_canonicalized_static_prefix() { path: FileSystemPath::GlobPattern { pattern }, access: FileSystemAccessMode::Deny, }); + let effective_permissions = effective_permissions(&policy, temp_dir.path()); - let seatbelt_policy = build_seatbelt_unreadable_glob_policy(&policy, temp_dir.path()); + let seatbelt_policy = build_seatbelt_unreadable_glob_policy(&effective_permissions); assert!( seatbelt_policy.contains(&format!(r#"(deny file-read* (regex #"{expected_regex}"))"#)), @@ -573,12 +599,12 @@ fn create_seatbelt_args_allowlists_explicit_unix_socket_paths_without_proxy() { &SandboxPolicy::new_read_only_policy(), cwd.path(), ); + let effective_permissions = effective_permissions(&file_system_policy, cwd.path()); let extra_allow_unix_sockets = vec![absolute_path("/tmp/codex-browser-use")]; let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { command: vec!["/usr/bin/true".to_string()], - file_system_sandbox_policy: &file_system_policy, + effective_filesystem_permissions: &effective_permissions, network_sandbox_policy: NetworkSandboxPolicy::Restricted, - sandbox_policy_cwd: cwd.path(), enforce_managed_network: false, network: None, extra_allow_unix_sockets: &extra_allow_unix_sockets, @@ -613,6 +639,7 @@ async fn create_seatbelt_args_merges_proxy_and_explicit_unix_socket_paths() -> a &SandboxPolicy::new_read_only_policy(), cwd.path(), ); + let effective_permissions = effective_permissions(&file_system_policy, cwd.path()); let network_socket = "/tmp/codex-proxy-use"; let explicit_socket = "/tmp/codex-browser-use"; let mut network_config = NetworkProxyConfig::default(); @@ -634,9 +661,8 @@ async fn create_seatbelt_args_merges_proxy_and_explicit_unix_socket_paths() -> a let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { command: vec!["/usr/bin/true".to_string()], - file_system_sandbox_policy: &file_system_policy, + effective_filesystem_permissions: &effective_permissions, network_sandbox_policy: NetworkSandboxPolicy::Restricted, - sandbox_policy_cwd: cwd.path(), enforce_managed_network: false, network: Some(&network_proxy), extra_allow_unix_sockets: &extra_allow_unix_sockets, @@ -672,12 +698,12 @@ fn create_seatbelt_args_preserves_full_network_with_explicit_unix_socket_paths() &SandboxPolicy::new_read_only_policy(), cwd.path(), ); + let effective_permissions = effective_permissions(&file_system_policy, cwd.path()); let extra_allow_unix_sockets = vec![absolute_path("/tmp/codex-browser-use")]; let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { command: vec!["/usr/bin/true".to_string()], - file_system_sandbox_policy: &file_system_policy, + effective_filesystem_permissions: &effective_permissions, network_sandbox_policy: NetworkSandboxPolicy::Enabled, - sandbox_policy_cwd: cwd.path(), enforce_managed_network: false, network: None, extra_allow_unix_sockets: &extra_allow_unix_sockets,