diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs index bc98097b29..8a96fe4a5e 100644 --- a/codex-rs/sandboxing/src/seatbelt.rs +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -23,15 +23,11 @@ const MACOS_SEATBELT_NETWORK_POLICY: &str = include_str!("seatbelt_network_polic const MACOS_SEATBELT_PREFERENCES_POLICY: &str = include_str!("seatbelt_preferences_policy.sbpl"); const MACOS_RESTRICTED_READ_ONLY_PLATFORM_DEFAULTS: &str = include_str!("seatbelt_read_only_platform_defaults.sbpl"); -// Ordinary processes need system scratch directories for compatibility, but filesystem helpers -// must not inherit scratch access beyond the paths their permission profile explicitly grants. -const MACOS_PROCESS_PLATFORM_DEFAULTS: &str = r#" -(allow file-read* (subpath "/Applications")) -(allow file-read* file-test-existence file-write* (subpath "/tmp")) -(allow file-read* file-write* (subpath "/private/tmp")) -(allow file-read* file-write* (subpath "/var/tmp")) -(allow file-read* file-write* (subpath "/private/var/tmp")) -"#; +#[path = "seatbelt_daemon.rs"] +mod daemon; + +#[path = "seatbelt_scratch.rs"] +mod scratch; #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] pub(crate) enum MacosSeatbeltProfile { @@ -903,8 +899,20 @@ pub(crate) fn create_seatbelt_command_args_with_profile( let unreadable_roots = file_system_sandbox_policy.get_unreadable_roots_with_cwd(sandbox_policy_cwd); - let writable_roots = file_system_sandbox_policy + let mut writable_roots = file_system_sandbox_policy .get_writable_roots_with_cwd_preserving_mutable_paths(sandbox_policy_cwd); + let include_platform_defaults = file_system_sandbox_policy.include_platform_defaults(); + let scratch_reads = if include_platform_defaults && profile == MacosSeatbeltProfile::Process { + let (reads, writes) = scratch::scratch_access_roots( + file_system_sandbox_policy, + sandbox_policy_cwd, + &writable_roots, + )?; + writable_roots.extend(writes); + reads + } else { + Vec::new() + }; let allowed_symlinked_codex_home = allowed_symlinked_codex_home .cloned() .map(normalize_top_level_alias_for_sandbox) @@ -915,16 +923,19 @@ pub(crate) fn create_seatbelt_command_args_with_profile( for writable_root in &writable_roots { let root = normalize_path_for_sandbox(writable_root.root.as_path()) .unwrap_or_else(|| writable_root.root.clone()); - for protected_directory in writable_root.read_only_subpaths.iter().filter_map(|path| { - normalize_path_for_sandbox(path.as_path()) - .unwrap_or_else(|| path.clone()) - .parent() - }) { - for ancestor in protected_directory.ancestors() { - if !ancestor.as_path().starts_with(root.as_path()) { - break; + for path in &writable_root.read_only_subpaths { + // Protect the logical entry's parents as well as its target's: + // moving a symlink's parent would otherwise bypass its exclusion. + let logical = normalize_top_level_alias_for_sandbox(path.clone())?; + let resolved = normalize_path_for_sandbox(logical.as_path()) + .filter(|resolved| resolved != &logical); + for protected_path in std::iter::once(logical).chain(resolved) { + for ancestor in protected_path.ancestors().skip(/*n*/ 1) { + if !ancestor.as_path().starts_with(root.as_path()) { + break; + } + protected_ancestors.insert(ancestor); } - protected_ancestors.insert(ancestor); } } } @@ -1008,6 +1019,7 @@ pub(crate) fn create_seatbelt_command_args_with_profile( protected_metadata_names: Vec::new(), root, }) + .chain(scratch_reads) .collect(), /*allowed_symlinked_codex_home*/ None, )?; @@ -1031,7 +1043,6 @@ pub(crate) fn create_seatbelt_command_args_with_profile( 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 mut policy_sections = vec![ @@ -1040,27 +1051,22 @@ pub(crate) fn create_seatbelt_command_args_with_profile( file_write_policy, network_policy, ]; - // Network grants and Unix-socket allowlists must never reopen the - // privileged app-server RPC transport to filesystem-restricted commands. - if !file_system_sandbox_policy.has_full_disk_write_access() { - let directory = codex_uds::shared_daemon_socket_directory() - .map_err(|error| SeatbeltPreparationError::FileSystem(error.to_string()))?; - let directory = serde_json::to_string(&directory.to_string_lossy()) - .map_err(|error| SeatbeltPreparationError::FileSystem(error.to_string()))?; - policy_sections.push(format!( - "(deny file-read* file-write* (subpath {directory}))\n\ - (deny network-outbound (remote unix-socket (subpath {directory})))" - )); - } if file_system_sandbox_policy.has_full_disk_read_access() { policy_sections.push(MACOS_SEATBELT_PREFERENCES_POLICY.to_string()); } if include_platform_defaults { policy_sections.push(MACOS_RESTRICTED_READ_ONLY_PLATFORM_DEFAULTS.to_string()); if profile == MacosSeatbeltProfile::Process { - policy_sections.push(MACOS_PROCESS_PLATFORM_DEFAULTS.to_string()); + policy_sections.push("(allow file-read* (subpath \"/Applications\"))".to_string()); } } + // Network grants and Unix-socket allowlists must never reopen the + // privileged app-server RPC transport to filesystem-restricted commands. + if !file_system_sandbox_policy.has_full_disk_write_access() { + let directory = codex_uds::shared_daemon_socket_directory() + .map_err(|error| SeatbeltPreparationError::FileSystem(error.to_string()))?; + policy_sections.push(daemon::protection_policy(&directory)?); + } policy_sections.push(deny_read_policy); // Renaming an allowed ancestor relocates its protected descendants past // their pathname carveouts. Keep these denies last so no broader allowance diff --git a/codex-rs/sandboxing/src/seatbelt_daemon.rs b/codex-rs/sandboxing/src/seatbelt_daemon.rs new file mode 100644 index 0000000000..964144cd88 --- /dev/null +++ b/codex-rs/sandboxing/src/seatbelt_daemon.rs @@ -0,0 +1,24 @@ +//! Mandatory pathname and network protections for privileged daemon sockets. +//! Emit these after all grants, including implicit platform access. + +use super::SeatbeltPreparationError; +use std::path::Path; + +pub(super) fn protection_policy(directory: &Path) -> Result { + let quoted = serde_json::to_string(&directory.to_string_lossy()) + .map_err(|error| SeatbeltPreparationError::FileSystem(error.to_string()))?; + let mut rules = vec![format!( + "(deny file-read* file-write* (literal {quoted}) (subpath {quoted}))\n\ + (deny network-outbound (remote unix-socket (subpath {quoted})))" + )]; + // Moving an ancestor would relocate the entire protected subtree beyond + // both pathname rules. Only unlink is denied; sibling writes still work. + for ancestor in directory.ancestors().skip(/*n*/ 1) { + let quoted = serde_json::to_string(&ancestor.to_string_lossy()) + .map_err(|error| SeatbeltPreparationError::FileSystem(error.to_string()))?; + rules.push(format!( + "(deny file-write-unlink (require-all (vnode-type DIRECTORY) (literal {quoted})))" + )); + } + Ok(rules.join("\n")) +} diff --git a/codex-rs/sandboxing/src/seatbelt_scratch.rs b/codex-rs/sandboxing/src/seatbelt_scratch.rs new file mode 100644 index 0000000000..6fb13192f4 --- /dev/null +++ b/codex-rs/sandboxing/src/seatbelt_scratch.rs @@ -0,0 +1,69 @@ +//! Lower implicit process scratch access through the normal Seatbelt exclusions. +//! Explicit restrictions and project metadata also constrain these broad grants. + +use super::SeatbeltAccessRoot; +use super::SeatbeltPreparationError; +use super::protected_metadata_names_for_writable_root; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemSandboxEntry; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::protocol::WritableRoot; +use codex_utils_absolute_path::AbsolutePathBuf; +use std::path::Path; + +pub(super) fn scratch_access_roots( + policy: &FileSystemSandboxPolicy, + cwd: &Path, + writable_roots: &[WritableRoot], +) -> Result<(Vec, Vec), SeatbeltPreparationError> { + let unreadable = policy.get_unreadable_roots_with_cwd(cwd); + let mut read_only = policy + .get_readable_roots_with_cwd(cwd) + .into_iter() + .filter(|path| !policy.can_write_local_path_with_cwd(path.as_path(), cwd)) + .collect::>(); + read_only.extend(unreadable.iter().cloned()); + for root in writable_roots { + read_only.extend(root.read_only_subpaths.iter().cloned()); + read_only.extend( + protected_metadata_names_for_writable_root(policy, root, cwd) + .iter() + .map(|name| root.root.join(name)), + ); + } + + // Use the physical paths: /tmp and /var are trusted system aliases, and + // the normal lowerer binds both roots and exclusions in that namespace. + let scratch_paths = ["/private/tmp", "/private/var/tmp"] + .into_iter() + .map(AbsolutePathBuf::from_absolute_path) + .collect::, _>>() + .map_err(|error| SeatbeltPreparationError::FileSystem(error.to_string()))?; + let mut scratch_policy = policy.clone(); + scratch_policy.entries.extend( + scratch_paths.iter().map(|path| { + FileSystemSandboxEntry::new(path.clone().into(), FileSystemAccessMode::Write) + }), + ); + let writes = scratch_policy + .get_writable_roots_with_cwd_preserving_mutable_paths(cwd) + .into_iter() + .filter(|root| scratch_paths.contains(&root.root)) + .map(|mut root| { + // Retain normal logical-path exclusions (including symlink inodes). + // Explicit restrictions at or above a scratch root, and metadata + // belonging to narrower project roots, also constrain the default. + root.read_only_subpaths.extend(read_only.iter().cloned()); + root + }) + .collect(); + let reads = scratch_paths + .into_iter() + .map(|root| SeatbeltAccessRoot { + root, + excluded_subpaths: unreadable.clone(), + protected_metadata_names: Vec::new(), + }) + .collect(); + Ok((reads, writes)) +} diff --git a/codex-rs/sandboxing/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs index eef7919213..90e7537c59 100644 --- a/codex-rs/sandboxing/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -51,6 +51,11 @@ fn assert_seatbelt_denied(stderr: &[u8], path: &Path) { let expected = format!("bash: {}: Operation not permitted\n", path.display()); assert!( stderr == expected + || stderr + == format!( + "bash: line 1: {}: Operation not permitted\n", + path.display() + ) || stderr.contains("sandbox-exec: sandbox_apply: Operation not permitted"), "unexpected stderr: {stderr}" ); @@ -303,46 +308,6 @@ fn process_platform_defaults_allow_scratch_without_granting_it_to_filesystem_hel .expect("build restricted seatbelt command") }; - let scratch_grants = [ - ( - "/tmp", - r#"(allow file-read* file-test-existence file-write* (subpath "/tmp"))"#, - ), - ( - "/private/tmp", - r#"(allow file-read* file-write* (subpath "/private/tmp"))"#, - ), - ( - "/var/tmp", - r#"(allow file-read* file-write* (subpath "/var/tmp"))"#, - ), - ( - "/private/var/tmp", - r#"(allow file-read* file-write* (subpath "/private/var/tmp"))"#, - ), - ]; - - for profile in [ - MacosSeatbeltProfile::Process, - MacosSeatbeltProfile::FileSystemHelper, - ] { - let args = sandboxed_args(vec!["/usr/bin/true".to_string()], profile); - let policy = seatbelt_policy_arg(&args); - - for (scratch_root, scratch_grant) in scratch_grants { - match profile { - MacosSeatbeltProfile::Process => assert!( - policy.contains(scratch_grant), - "processes should retain scratch read/write access to {scratch_root}" - ), - MacosSeatbeltProfile::FileSystemHelper => assert!( - !policy.contains(&format!(r#"(subpath "{scratch_root}")"#)), - "filesystem helpers should not inherit scratch access to {scratch_root}" - ), - } - } - } - let run_sandboxed = |command: Vec, profile: MacosSeatbeltProfile| { Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(sandboxed_args(command, profile)) @@ -371,9 +336,7 @@ fn process_platform_defaults_allow_scratch_without_granting_it_to_filesystem_hel if !process_result.status.success() && process_stderr.contains("sandbox-exec: sandbox_apply: Operation not permitted") { - eprintln!( - "nested Seatbelt is unavailable; generated policies verified every scratch path" - ); + eprintln!("nested Seatbelt is unavailable; scratch access behavior was not verified"); break; } assert!(