diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 1e132ad840..581d3564a9 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -118,7 +118,7 @@ impl FileSystemSandboxRunner { sandbox_context: &FileSystemSandboxContext, ) -> Result { let helper = &self.runtime_paths.codex_self_exe; - let sandbox_manager = SandboxManager::new(); + let sandbox_manager = SandboxManager::for_file_system_helpers(); let sandbox = sandbox_manager.select_initial( permission_profile, SandboxablePreference::Require, @@ -196,16 +196,11 @@ fn native_workspace_root(root: &PathUri) -> Result Vec { - let mut roots = Vec::new(); - for path in std::iter::once(runtime_paths.codex_self_exe.as_path()) - .chain(runtime_paths.codex_linux_sandbox_exe.as_deref()) + let mut roots = vec![runtime_paths.codex_self_exe.clone()]; + if let Some(path) = &runtime_paths.codex_linux_sandbox_exe + && !roots.contains(path) { - if let Some(parent) = path.parent() - && let Ok(root) = AbsolutePathBuf::from_absolute_path(parent) - && !roots.contains(&root) - { - roots.push(root); - } + roots.push(path.clone()); } roots } @@ -458,13 +453,7 @@ mod tests { writable.clone(), FileSystemAccessMode::Write, )]); - let readable = AbsolutePathBuf::from_absolute_path( - runtime_paths - .codex_self_exe - .parent() - .expect("current exe parent"), - ) - .expect("absolute readable path"); + let readable = runtime_paths.codex_self_exe.clone(); add_helper_runtime_permissions( &mut policy, @@ -680,7 +669,7 @@ mod tests { } #[test] - fn helper_permissions_include_helper_read_root_without_additional_permissions() { + fn helper_permissions_include_only_the_helper_executable() { let codex_self_exe = std::env::current_exe().expect("current exe"); let runtime_paths = ExecServerRuntimePaths::new(codex_self_exe, /*codex_linux_sandbox_exe*/ None) @@ -688,13 +677,11 @@ mod tests { let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) .expect("absolute cwd"); let mut policy = restricted_policy(Vec::new()); - let readable = AbsolutePathBuf::from_absolute_path( - runtime_paths - .codex_self_exe - .parent() - .expect("current exe parent"), - ) - .expect("absolute readable path"); + let parent = runtime_paths + .codex_self_exe + .parent() + .expect("current exe parent"); + let sibling = parent.join("credentials.json"); add_helper_runtime_permissions( &mut policy, @@ -702,11 +689,15 @@ mod tests { cwd.as_path(), ); - assert!(policy.can_read_path_with_cwd(readable.as_path(), cwd.as_path())); + assert!( + policy.can_read_path_with_cwd(runtime_paths.codex_self_exe.as_path(), cwd.as_path()) + ); + assert!(!policy.can_read_path_with_cwd(parent.as_path(), cwd.as_path())); + assert!(!policy.can_read_path_with_cwd(sibling.as_path(), cwd.as_path())); } #[test] - fn helper_permissions_include_linux_sandbox_alias_parent() { + fn helper_permissions_include_only_linux_sandbox_alias_executable() { let root = tempfile::tempdir().expect("temp dir"); let codex_self_exe = root.path().join("bin").join("codex"); let codex_linux_sandbox_exe = root.path().join("aliases").join("codex-linux-sandbox"); @@ -716,10 +707,12 @@ mod tests { let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) .expect("absolute cwd"); let mut policy = restricted_policy(Vec::new()); - let codex_parent = AbsolutePathBuf::from_absolute_path(root.path().join("bin")) - .expect("absolute codex parent"); - let alias_parent = AbsolutePathBuf::from_absolute_path(root.path().join("aliases")) - .expect("absolute alias parent"); + let codex_parent = runtime_paths.codex_self_exe.parent().expect("codex parent"); + let alias = runtime_paths + .codex_linux_sandbox_exe + .as_ref() + .expect("linux sandbox alias"); + let alias_parent = alias.parent().expect("alias parent"); add_helper_runtime_permissions( &mut policy, @@ -727,8 +720,12 @@ mod tests { cwd.as_path(), ); - assert!(policy.can_read_path_with_cwd(codex_parent.as_path(), cwd.as_path())); - assert!(policy.can_read_path_with_cwd(alias_parent.as_path(), cwd.as_path())); + assert!( + policy.can_read_path_with_cwd(runtime_paths.codex_self_exe.as_path(), cwd.as_path()) + ); + assert!(policy.can_read_path_with_cwd(alias.as_path(), cwd.as_path())); + assert!(!policy.can_read_path_with_cwd(codex_parent.as_path(), cwd.as_path())); + assert!(!policy.can_read_path_with_cwd(alias_parent.as_path(), cwd.as_path())); } fn restricted_policy(entries: Vec) -> FileSystemSandboxPolicy { diff --git a/codex-rs/exec-server/tests/file_system/shared.rs b/codex-rs/exec-server/tests/file_system/shared.rs index 7c4c2eef18..b62adf7009 100644 --- a/codex-rs/exec-server/tests/file_system/shared.rs +++ b/codex-rs/exec-server/tests/file_system/shared.rs @@ -2,8 +2,14 @@ use anyhow::Context; use anyhow::Result; use codex_exec_server::CopyOptions; use codex_exec_server::CreateDirectoryOptions; +#[cfg(unix)] +use codex_exec_server::ExecServerRuntimePaths; +#[cfg(unix)] +use codex_exec_server::ExecutorFileSystem; use codex_exec_server::FILE_READ_CHUNK_SIZE; use codex_exec_server::FileMetadata; +#[cfg(unix)] +use codex_exec_server::LocalFileSystem; use codex_exec_server::ReadDirectoryEntry; use codex_exec_server::RemoveOptions; use codex_exec_server::WalkEntry; @@ -703,6 +709,90 @@ async fn file_system_sandboxed_metadata_and_read_allow_readable_root( Ok(()) } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn sandboxed_file_operations_cannot_read_helper_siblings() -> Result<()> { + let helper_paths = crate::common::exec_server::test_codex_helper_paths()?; + let root = TempDir::new()?; + let runtime_dir = root.path().join("runtime"); + let workspace = root.path().join("workspace"); + std::fs::create_dir(&runtime_dir)?; + std::fs::create_dir(&workspace)?; + + let helper = runtime_dir.join("codex-test-helper"); + std::fs::hard_link(&helper_paths.codex_exe, &helper) + .or_else(|_| std::fs::copy(&helper_paths.codex_exe, &helper).map(|_| ()))?; + let linux_sandbox = if helper_paths.codex_linux_sandbox_exe.is_some() { + let alias = runtime_dir.join("codex-linux-sandbox"); + std::fs::hard_link(&helper, &alias) + .or_else(|_| std::fs::copy(&helper, &alias).map(|_| ()))?; + Some(alias) + } else { + None + }; + let file_system = + LocalFileSystem::with_runtime_paths(ExecServerRuntimePaths::new(helper, linux_sandbox)?); + + let sibling = runtime_dir.join("credentials.json"); + std::fs::write(&sibling, "secret")?; + let escaping_link = workspace.join("credentials-link"); + std::os::unix::fs::symlink(&sibling, &escaping_link)?; + let sandbox = workspace_write_sandbox(workspace.clone()); + let allowed_file = workspace.join("allowed.txt"); + std::fs::write(&allowed_file, b"allowed")?; + let allowed_contents = file_system + .read_file( + &PathUri::from_host_native_path(&allowed_file)?, + Some(&sandbox), + ) + .await?; + assert_eq!(allowed_contents, b"allowed"); + + #[cfg(target_os = "macos")] + assert!( + file_system + .read_directory( + &PathUri::from_host_native_path("/Applications")?, + Some(&sandbox) + ) + .await + .is_err(), + "filesystem helpers should not inherit the normal process sandbox's /Applications access" + ); + + let sibling_uri = PathUri::from_host_native_path(&sibling)?; + let destination = PathUri::from_host_native_path(workspace.join("copied.json"))?; + + for path in [sibling, escaping_link] { + let path = PathUri::from_host_native_path(path)?; + assert!( + file_system.read_file(&path, Some(&sandbox)).await.is_err(), + "sandboxed read unexpectedly accessed helper sibling {path}" + ); + assert!( + file_system + .read_file_stream(&path, Some(&sandbox)) + .await + .is_err(), + "sandboxed streaming unexpectedly accessed helper sibling {path}" + ); + } + assert!( + file_system + .copy( + &sibling_uri, + &destination, + CopyOptions { recursive: false }, + Some(&sandbox), + ) + .await + .is_err(), + "sandboxed copy unexpectedly accessed helper sibling {sibling_uri}" + ); + + Ok(()) +} + pub(crate) async fn assert_canonicalize_resolves_directory_alias( implementation: FileSystemImplementation, create_directory_alias: impl FnOnce(&Path, &Path) -> Result<()>, diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index 87c1e26a58..3a49a7d72c 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -11,6 +11,8 @@ use crate::policy_transforms::should_require_platform_sandbox; use crate::resolve_windows_elevated_filesystem_overrides; #[cfg(target_os = "windows")] use crate::resolve_windows_restricted_token_filesystem_overrides; +#[cfg(target_os = "macos")] +use crate::seatbelt::MacosSeatbeltProfile; #[cfg(target_os = "windows")] use crate::windows_sandbox_uses_elevated_backend; use codex_network_proxy::ManagedNetworkSandboxContext; @@ -262,11 +264,22 @@ impl std::error::Error for SandboxTransformError { } #[derive(Default)] -pub struct SandboxManager; +pub struct SandboxManager { + #[cfg(target_os = "macos")] + seatbelt_profile: MacosSeatbeltProfile, +} impl SandboxManager { pub fn new() -> Self { - Self + Self::default() + } + + /// Creates a manager that applies the narrower runtime profile required by filesystem helpers. + pub fn for_file_system_helpers() -> Self { + Self { + #[cfg(target_os = "macos")] + seatbelt_profile: MacosSeatbeltProfile::FileSystemHelper, + } } pub fn select_initial( @@ -347,23 +360,26 @@ impl SandboxManager { SandboxType::MacosSeatbelt => { use crate::seatbelt::CreateSeatbeltCommandArgsParams; use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; - use crate::seatbelt::create_seatbelt_command_args; + use crate::seatbelt::create_seatbelt_command_args_with_profile; let pending = pending_sandboxed_request?; let (file_system_sandbox_policy, network_sandbox_policy) = pending .effective_permission_profile .to_runtime_permissions(); - let mut args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { - command: os_argv_to_strings(argv), - file_system_sandbox_policy: &file_system_sandbox_policy, - network_sandbox_policy, - sandbox_policy_cwd: pending.native_sandbox_policy_cwd.as_path(), - enforce_managed_network, - managed_network, - environment_id, - network, - extra_allow_unix_sockets: &[], - }) + let mut args = create_seatbelt_command_args_with_profile( + CreateSeatbeltCommandArgsParams { + command: os_argv_to_strings(argv), + file_system_sandbox_policy: &file_system_sandbox_policy, + network_sandbox_policy, + sandbox_policy_cwd: pending.native_sandbox_policy_cwd.as_path(), + enforce_managed_network, + managed_network, + environment_id, + network, + extra_allow_unix_sockets: &[], + }, + self.seatbelt_profile, + ) .map_err(SandboxTransformError::EnvironmentNetworkProxy)?; let mut full_command = Vec::with_capacity(1 + args.len()); full_command.push(MACOS_PATH_TO_SEATBELT_EXECUTABLE.to_string()); 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 d3015bc58a..f9a40c9524 100644 --- a/codex-rs/sandboxing/src/restricted_read_only_platform_defaults.sbpl +++ b/codex-rs/sandboxing/src/restricted_read_only_platform_defaults.sbpl @@ -171,7 +171,6 @@ (allow file-read* (subpath "/Library/Preferences")) (allow file-read* (subpath "/opt/homebrew/lib")) (allow file-read* (subpath "/usr/local/lib")) -(allow file-read* (subpath "/Applications")) ; Terminal basics and device handles. (allow file-read* (regex "^/dev/fd/(0|1|2)$")) diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs index f53569de07..b3ef7e17d1 100644 --- a/codex-rs/sandboxing/src/seatbelt.rs +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -22,6 +22,15 @@ 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_RESTRICTED_READ_ONLY_PLATFORM_DEFAULTS: &str = include_str!("restricted_read_only_platform_defaults.sbpl"); +const MACOS_PROCESS_APPLICATIONS_READ_POLICY: &str = + r#"(allow file-read* (subpath "/Applications"))"#; + +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub(crate) enum MacosSeatbeltProfile { + #[default] + Process, + FileSystemHelper, +} /// When working with `sandbox-exec`, only consider `sandbox-exec` in `/usr/bin` /// to defend against an attacker trying to inject a malicious version on the @@ -622,6 +631,13 @@ pub struct CreateSeatbeltCommandArgsParams<'a> { pub fn create_seatbelt_command_args( args: CreateSeatbeltCommandArgsParams<'_>, +) -> Result, String> { + create_seatbelt_command_args_with_profile(args, MacosSeatbeltProfile::Process) +} + +pub(crate) fn create_seatbelt_command_args_with_profile( + args: CreateSeatbeltCommandArgsParams<'_>, + profile: MacosSeatbeltProfile, ) -> Result, String> { let CreateSeatbeltCommandArgsParams { command, @@ -747,6 +763,9 @@ pub fn create_seatbelt_command_args( ]; if include_platform_defaults { policy_sections.push(MACOS_RESTRICTED_READ_ONLY_PLATFORM_DEFAULTS.to_string()); + if profile == MacosSeatbeltProfile::Process { + policy_sections.push(MACOS_PROCESS_APPLICATIONS_READ_POLICY.to_string()); + } } let full_policy = policy_sections.join("\n"); diff --git a/codex-rs/sandboxing/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs index 656f3f05b3..bfd440c50e 100644 --- a/codex-rs/sandboxing/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -1,11 +1,13 @@ use super::CreateSeatbeltCommandArgsParams; use super::MACOS_PATH_TO_SEATBELT_EXECUTABLE; use super::MACOS_SEATBELT_BASE_POLICY; +use super::MacosSeatbeltProfile; use super::ProxyPolicyInputs; use super::UnixDomainSocketPolicy; use super::build_seatbelt_unreadable_glob_policy; use super::create_seatbelt_command_args; use super::create_seatbelt_command_args_for_legacy_policy; +use super::create_seatbelt_command_args_with_profile; use super::dynamic_network_policy; use super::normalize_path_for_sandbox; use super::seatbelt_regex_for_unreadable_glob; @@ -122,6 +124,80 @@ fn base_policy_allows_kmp_registration_shm_read_create_and_unlink() { ); } +#[test] +fn filesystem_helper_platform_defaults_do_not_grant_applications_directory() { + let workspace = TempDir::new().expect("temp workspace"); + let workspace_root = AbsolutePathBuf::from_absolute_path(workspace.path()) + .expect("workspace path should be absolute"); + let file_system_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new( + FileSystemPath::Path { + path: workspace_root, + }, + FileSystemAccessMode::Read, + ), + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Minimal, + }, + FileSystemAccessMode::Read, + ), + ]); + + let list_directory = |path: &Path, profile: MacosSeatbeltProfile| { + let args = create_seatbelt_command_args_with_profile( + CreateSeatbeltCommandArgsParams { + command: vec!["/bin/ls".to_string(), path.display().to_string()], + file_system_sandbox_policy: &file_system_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: &[], + }, + profile, + ) + .expect("build restricted seatbelt command"); + + Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) + .args(args) + .current_dir(workspace.path()) + .output() + .expect("run restricted seatbelt command") + }; + + let allowed = list_directory(workspace.path(), MacosSeatbeltProfile::FileSystemHelper); + let allowed_stderr = String::from_utf8_lossy(&allowed.stderr); + if !allowed.status.success() + && allowed_stderr.contains("sandbox-exec: sandbox_apply: Operation not permitted") + { + return; + } + assert!( + allowed.status.success(), + "the explicitly allowed workspace should remain readable: {allowed_stderr}" + ); + + let process_allowed = list_directory(Path::new("/Applications"), MacosSeatbeltProfile::Process); + let process_allowed_stderr = String::from_utf8_lossy(&process_allowed.stderr); + assert!( + process_allowed.status.success(), + "normal process sandboxes should preserve /Applications access: {process_allowed_stderr}" + ); + + let denied = list_directory( + Path::new("/Applications"), + MacosSeatbeltProfile::FileSystemHelper, + ); + let denied_stderr = String::from_utf8_lossy(&denied.stderr); + assert!( + !denied.status.success() && denied_stderr.contains("Operation not permitted"), + "filesystem helper platform defaults should not grant /Applications: {denied_stderr}" + ); +} + #[test] fn create_seatbelt_args_routes_network_through_proxy_ports() { let policy = dynamic_network_policy(