From 21facf227366ae68589cf7567db917a3ba2dbd9a Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Thu, 20 Aug 2026 23:08:57 +0000 Subject: [PATCH] Restrict macOS preference reads to full-disk policies (#39811) ## Why The macOS preferences service can expose data outside a sandbox's allowed filesystem read roots. ## What changed - Move the Seatbelt preference and `cfprefsd` grants into a separate policy section that is included only when filesystem reads are unrestricted. - Remove the equivalent grants from the restricted platform defaults. ## Testing - Verify both Seatbelt profiles include preference grants only for full-disk read policies, including policies with denied paths or globs. - Verify a restricted sandbox cannot retrieve a preference whose plist is denied, while an unrestricted read policy can. GitOrigin-RevId: 90388366a7302bca1830ad0439544be8babc7321 --- codex-rs/sandboxing/BUILD.bazel | 1 + ...estricted_read_only_platform_defaults.sbpl | 5 +- codex-rs/sandboxing/src/seatbelt.rs | 4 + .../sandboxing/src/seatbelt_base_policy.sbpl | 8 - .../src/seatbelt_preferences_policy.sbpl | 8 + codex-rs/sandboxing/src/seatbelt_tests.rs | 236 +++++++++++++++++- 6 files changed, 238 insertions(+), 24 deletions(-) create mode 100644 codex-rs/sandboxing/src/seatbelt_preferences_policy.sbpl diff --git a/codex-rs/sandboxing/BUILD.bazel b/codex-rs/sandboxing/BUILD.bazel index 69e8561282..b9117606d3 100644 --- a/codex-rs/sandboxing/BUILD.bazel +++ b/codex-rs/sandboxing/BUILD.bazel @@ -6,6 +6,7 @@ codex_rust_crate( "src/restricted_read_only_platform_defaults.sbpl", "src/seatbelt_base_policy.sbpl", "src/seatbelt_network_policy.sbpl", + "src/seatbelt_preferences_policy.sbpl", ], crate_name = "codex_sandboxing", test_tags = ["no-sandbox"], diff --git a/codex-rs/sandboxing/src/restricted_read_only_platform_defaults.sbpl b/codex-rs/sandboxing/src/restricted_read_only_platform_defaults.sbpl index f9a40c9524..2a65f9d6d9 100644 --- a/codex-rs/sandboxing/src/restricted_read_only_platform_defaults.sbpl +++ b/codex-rs/sandboxing/src/restricted_read_only_platform_defaults.sbpl @@ -122,8 +122,6 @@ (global-name "com.apple.analyticsd.messagetracer") (global-name "com.apple.appsleep") (global-name "com.apple.bsd.dirhelper") - (global-name "com.apple.cfprefsd.agent") - (global-name "com.apple.cfprefsd.daemon") (global-name "com.apple.diagnosticd") (global-name "com.apple.dt.automationmode.reader") (global-name "com.apple.espd") @@ -137,8 +135,7 @@ (global-name "com.apple.system.opendirectoryd.membership") (global-name "com.apple.trustd") (global-name "com.apple.trustd.agent") - (global-name "com.apple.xpc.activity.unmanaged") - (local-name "com.apple.cfprefsd.agent")) + (global-name "com.apple.xpc.activity.unmanaged")) ; Allow IPC to the syslog socket for logging. (allow network-outbound (literal "/private/var/run/syslog")) diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs index 037bf6c57f..46197b2527 100644 --- a/codex-rs/sandboxing/src/seatbelt.rs +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -20,6 +20,7 @@ use url::Url; const MACOS_SEATBELT_BASE_POLICY: &str = include_str!("seatbelt_base_policy.sbpl"); const MACOS_SEATBELT_NETWORK_POLICY: &str = include_str!("seatbelt_network_policy.sbpl"); +const MACOS_SEATBELT_PREFERENCES_POLICY: &str = include_str!("seatbelt_preferences_policy.sbpl"); const MACOS_RESTRICTED_READ_ONLY_PLATFORM_DEFAULTS: &str = include_str!("restricted_read_only_platform_defaults.sbpl"); const MACOS_PROCESS_APPLICATIONS_READ_POLICY: &str = @@ -987,6 +988,9 @@ pub(crate) fn create_seatbelt_command_args_with_profile( file_write_policy, network_policy, ]; + 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 { diff --git a/codex-rs/sandboxing/src/seatbelt_base_policy.sbpl b/codex-rs/sandboxing/src/seatbelt_base_policy.sbpl index 99f43e42e3..ba3140aa44 100644 --- a/codex-rs/sandboxing/src/seatbelt_base_policy.sbpl +++ b/codex-rs/sandboxing/src/seatbelt_base_policy.sbpl @@ -112,11 +112,3 @@ ; PTYs created before entering seatbelt may lack the extension; allow ioctl ; on those slave ttys so interactive shells detect a TTY and remain functional. (allow file-ioctl (regex #"^/dev/ttys[0-9]+")) - -; allow readonly user preferences -(allow ipc-posix-shm-read* (ipc-posix-name-prefix "apple.cfprefs.")) -(allow mach-lookup - (global-name "com.apple.cfprefsd.daemon") - (global-name "com.apple.cfprefsd.agent") - (local-name "com.apple.cfprefsd.agent")) -(allow user-preference-read) diff --git a/codex-rs/sandboxing/src/seatbelt_preferences_policy.sbpl b/codex-rs/sandboxing/src/seatbelt_preferences_policy.sbpl new file mode 100644 index 0000000000..45599d1b5e --- /dev/null +++ b/codex-rs/sandboxing/src/seatbelt_preferences_policy.sbpl @@ -0,0 +1,8 @@ +; Preferences IPC can expose data outside the filesystem read roots. +; Include this policy only when filesystem reads are unrestricted. +(allow ipc-posix-shm-read* (ipc-posix-name-prefix "apple.cfprefs.")) +(allow mach-lookup + (global-name "com.apple.cfprefsd.daemon") + (global-name "com.apple.cfprefsd.agent") + (local-name "com.apple.cfprefsd.agent")) +(allow user-preference-read) diff --git a/codex-rs/sandboxing/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs index 4a8c195eb0..e9e42f054a 100644 --- a/codex-rs/sandboxing/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -35,7 +35,11 @@ use codex_protocol::permissions::PROTECTED_METADATA_PATH_NAMES; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; +use std::ffi::CStr; +use std::ffi::OsStr; use std::fs; +use std::mem::MaybeUninit; +use std::os::unix::ffi::OsStrExt; use std::path::Path; use std::path::PathBuf; use std::process::Command; @@ -762,19 +766,227 @@ fn unreadable_glob_policy_includes_canonicalized_static_prefix() { } #[test] -fn seatbelt_args_without_extension_profile_keep_legacy_preferences_read_access() { - let cwd = std::env::temp_dir(); - let args = create_seatbelt_command_args_for_legacy_policy( - vec!["echo".to_string(), "ok".to_string()], +fn preferences_access_requires_unrestricted_reads() { + let cwd = Path::new("/tmp"); + let full_read = FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( + &SandboxPolicy::new_workspace_write_policy(), + cwd, + ); + let minimal = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Minimal, + }, + FileSystemAccessMode::Read, + )]); + let workspace_only = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new( + absolute_path("/tmp").into(), + FileSystemAccessMode::Read, + )]); + let mut denied_path = full_read.clone(); + denied_path.entries.push(FileSystemSandboxEntry::new( + absolute_path("/tmp/codex-private").into(), + FileSystemAccessMode::Deny, + )); + let mut denied_glob = full_read.clone(); + denied_glob.entries.push(FileSystemSandboxEntry::new( + FileSystemPath::GlobPattern { + pattern: "/tmp/**/*.private".to_string(), + }, + FileSystemAccessMode::Deny, + )); + + for (name, file_system_policy, allow_preferences) in [ + ("legacy workspace-write", full_read, true), + ("minimal", minimal, false), + ("workspace only", workspace_only, false), + ("denied path", denied_path, false), + ("denied glob", denied_glob, false), + ] { + for profile in [ + MacosSeatbeltProfile::Process, + MacosSeatbeltProfile::FileSystemHelper, + ] { + let args = create_seatbelt_command_args_with_profile( + CreateSeatbeltCommandArgsParams { + command: vec!["/usr/bin/true".to_string()], + file_system_sandbox_policy: &file_system_policy, + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox_policy_cwd: cwd, + enforce_managed_network: false, + managed_network: None, + environment_id: None, + network: None, + extra_allow_unix_sockets: &[], + }, + profile, + ) + .expect("build seatbelt policy"); + let policy = seatbelt_policy_arg(&args); + for grant in [ + "apple.cfprefs.", + "com.apple.cfprefsd.daemon", + "com.apple.cfprefsd.agent", + "(allow user-preference-read)", + ] { + assert_eq!( + policy.contains(grant), + allow_preferences, + "unexpected {grant} permission for {name} ({profile:?})" + ); + } + assert!(!policy.contains("(allow user-preference-write)")); + } + } +} + +#[test] +fn restricted_reads_cannot_read_preferences_outside_allowed_roots() { + struct PreferenceDomain(String); + + impl Drop for PreferenceDomain { + fn drop(&mut self) { + let _ = Command::new("/usr/bin/defaults") + .args(["delete", &self.0]) + .output(); + } + } + + let workspace = tempfile::Builder::new() + .prefix("codex-prefs-") + .tempdir() + .expect("temp workspace"); + let domain = PreferenceDomain(format!( + "com.openai.codex.{}", + workspace + .path() + .file_name() + .expect("workspace name") + .to_string_lossy() + )); + let marker = "codex-preferences-read-canary"; + // Bazel gives tests a temporary HOME, but preferences use the account home. + // Use caller-owned storage because tests can query the account concurrently. + let mut passwd = MaybeUninit::::uninit(); + let mut buffer = vec![0_u8; 16 * 1024]; + let mut result = std::ptr::null_mut(); + let status = unsafe { + libc::getpwuid_r( + libc::getuid(), + passwd.as_mut_ptr(), + buffer.as_mut_ptr().cast(), + buffer.len(), + &mut result, + ) + }; + assert_eq!(status, 0, "look up current account"); + assert!(!result.is_null(), "current account was not found"); + // SAFETY: getpwuid_r succeeded and initialized passwd; buffer is still alive. + let passwd = unsafe { passwd.assume_init_ref() }; + assert!(!passwd.pw_dir.is_null(), "current account has no home"); + let account_home = unsafe { CStr::from_ptr(passwd.pw_dir) }; + let plist = PathBuf::from(OsStr::from_bytes(account_home.to_bytes())) + .join("Library/Preferences") + .join(format!("{}.plist", domain.0)); + let written = Command::new("/usr/bin/defaults") + .args(["write", &domain.0, "canary", "-string", marker]) + .output() + .expect("write test preference"); + assert!( + written.status.success(), + "write test preference: {}", + String::from_utf8_lossy(&written.stderr) + ); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while !plist.is_file() && std::time::Instant::now() < deadline { + std::thread::sleep(std::time::Duration::from_millis(100)); + } + assert!( + plist.is_file(), + "test preference was not persisted at {}", + plist.display() + ); + + let workspace_root = + AbsolutePathBuf::from_absolute_path(workspace.path()).expect("absolute workspace"); + let plist_path = AbsolutePathBuf::from_absolute_path(&plist).expect("absolute plist path"); + let restricted = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Minimal, + }, + FileSystemAccessMode::Read, + ), + FileSystemSandboxEntry::new(workspace_root.into(), FileSystemAccessMode::Read), + FileSystemSandboxEntry::new(plist_path.into(), FileSystemAccessMode::Deny), + ]); + let full_read = FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( &SandboxPolicy::new_read_only_policy(), - cwd.as_path(), - /*enforce_managed_network*/ false, - /*network*/ None, - ) - .unwrap(); - let policy = &args[1]; - assert!(policy.contains("(allow user-preference-read)")); - assert!(!policy.contains("(allow user-preference-write)")); + workspace.path(), + ); + let run = |policy: &FileSystemSandboxPolicy, command: Vec| { + let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { + command, + file_system_sandbox_policy: policy, + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox_policy_cwd: workspace.path(), + enforce_managed_network: false, + managed_network: None, + environment_id: None, + network: None, + extra_allow_unix_sockets: &[], + }) + .expect("build seatbelt command"); + Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) + .args(args) + .current_dir(workspace.path()) + .output() + .expect("run seatbelt command") + }; + let allowed_file = workspace.path().join("allowed.txt"); + fs::write(&allowed_file, "allowed").expect("write allowed file"); + let control = run( + &restricted, + vec!["/bin/cat".to_string(), allowed_file.display().to_string()], + ); + let control_stderr = String::from_utf8_lossy(&control.stderr); + if !control.status.success() + && control_stderr.contains("sandbox-exec: sandbox_apply: Operation not permitted") + { + return; + } + assert!(control.status.success(), "control failed: {control_stderr}"); + assert_eq!(control.stdout, b"allowed"); + + let direct = run( + &restricted, + vec!["/bin/cat".to_string(), plist.display().to_string()], + ); + assert!( + !direct.status.success() + && String::from_utf8_lossy(&direct.stderr).contains("Operation not permitted"), + "direct plist read should be denied: {direct:?}" + ); + + let read_preference = || { + vec![ + "/usr/bin/defaults".to_string(), + "read".to_string(), + domain.0.clone(), + "canary".to_string(), + ] + }; + for _ in 0..2 { + let denied = run(&restricted, read_preference()); + assert!( + !denied.status.success() && !String::from_utf8_lossy(&denied.stdout).contains(marker), + "restricted preferences read returned protected data: {denied:?}" + ); + + // The full-read control also warms the cache before the next attempt. + let allowed = run(&full_read, read_preference()); + assert!(allowed.status.success(), "full-read control: {allowed:?}"); + assert_eq!(String::from_utf8_lossy(&allowed.stdout).trim(), marker); + } } #[test]