From 02de49f7183d30cc72bdb83b64816392c29a908c Mon Sep 17 00:00:00 2001 From: jif Date: Thu, 20 Aug 2026 11:47:49 +0000 Subject: [PATCH] Harden Seatbelt writable root path binding (#39706) ## Why Resolving attacker-mutable path components while preparing a Seatbelt profile can let a writable root be rebound to a different location before the sandbox is applied. File roots also need to remain confined to the file itself rather than granting access to descendants after replacement. ## What changed - Preserve mutable components of writable-root paths until Seatbelt binds them, while still normalizing trusted top-level aliases such as `/tmp`. - Use literal grants for existing file and device roots, and subpath grants for directories and missing roots. - Exclude both logical and resolved forms of protected subpaths so symlinked metadata directories remain read-only. ## Testing Add coverage for ancestor rebinding, file-root symlink and directory replacement, missing directory roots, top-level aliases, and symlinked metadata carveouts. GitOrigin-RevId: 63c00e44dd30766e873b8acdad09d6657657fad9 --- codex-rs/protocol/src/permissions.rs | 162 ++++++++++++- codex-rs/sandboxing/src/seatbelt.rs | 148 +++++++++--- codex-rs/sandboxing/src/seatbelt_tests.rs | 277 +++++++++++++++++++++- 3 files changed, 540 insertions(+), 47 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 11ead20443..bde83ad18b 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -238,6 +238,21 @@ pub struct FileSystemSandboxPolicy { pub entries: Vec, } +#[derive(Clone, Copy)] +enum WritableRootPathResolution { + Effective, + PreserveMutableComponents, +} + +impl WritableRootPathResolution { + fn resolve(self, path: AbsolutePathBuf) -> AbsolutePathBuf { + match self { + Self::Effective => normalize_effective_absolute_path(path), + Self::PreserveMutableComponents => normalize_trusted_top_level_alias(path), + } + } +} + /// Serialized filesystem policy used at legacy string-based seams. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] #[schemars(rename = "FileSystemSandboxPolicy")] @@ -1149,6 +1164,30 @@ impl FileSystemSandboxPolicy { /// Returns the writable roots together with read-only carveouts resolved /// against the provided cwd. pub fn get_writable_roots_with_cwd(&self, cwd: &Path) -> Vec { + self.get_writable_roots_with_cwd_impl(cwd, WritableRootPathResolution::Effective) + } + + /// Returns writable roots without following attacker-mutable path components. + /// + /// Trusted top-level aliases such as `/tmp -> /private/tmp` are still + /// normalized so roots and carveouts are compared in the same namespace. + /// Deeper components remain exactly as configured until the platform + /// sandbox binds them. + pub fn get_writable_roots_with_cwd_preserving_mutable_paths( + &self, + cwd: &Path, + ) -> Vec { + self.get_writable_roots_with_cwd_impl( + cwd, + WritableRootPathResolution::PreserveMutableComponents, + ) + } + + fn get_writable_roots_with_cwd_impl( + &self, + cwd: &Path, + path_resolution: WritableRootPathResolution, + ) -> Vec { if self.has_full_disk_write_access() { return Vec::new(); } @@ -1162,8 +1201,12 @@ impl FileSystemSandboxPolicy { .collect(); dedup_absolute_paths( - writable_entries.clone(), - /*normalize_effective_paths*/ true, + writable_entries + .iter() + .cloned() + .map(|root| path_resolution.resolve(root)) + .collect(), + /*normalize_effective_paths*/ false, ) .into_iter() .map(|root| { @@ -1177,13 +1220,13 @@ impl FileSystemSandboxPolicy { let preserve_raw_carveout_paths = root.as_path().parent().is_some(); let raw_writable_roots: Vec<&AbsolutePathBuf> = writable_entries .iter() - .filter(|path| normalize_effective_absolute_path((*path).clone()) == root) + .filter(|path| path_resolution.resolve((*path).clone()) == root) .collect(); let protected_metadata_names = protected_metadata_names_for_writable_root(self, &root, &raw_writable_roots, cwd); let protect_missing_dot_codex = AbsolutePathBuf::from_absolute_path(cwd) .ok() - .is_some_and(|cwd| normalize_effective_absolute_path(cwd) == root); + .is_some_and(|cwd| path_resolution.resolve(cwd) == root); let mut read_only_subpaths: Vec = default_read_only_subpaths_for_writable_root(&root, protect_missing_dot_codex) .into_iter() @@ -1202,7 +1245,7 @@ impl FileSystemSandboxPolicy { .filter(|entry| !entry.access.can_write()) .filter(|entry| !self.can_write_path_with_cwd(entry.path.as_path(), cwd)) .filter_map(|entry| { - let effective_path = normalize_effective_absolute_path(entry.path.clone()); + let effective_path = path_resolution.resolve(entry.path.clone()); // Preserve the literal in-root path whenever the // carveout itself lives under this writable root, even // if following symlinks would resolve back to the root @@ -1754,6 +1797,27 @@ fn normalize_effective_absolute_path(path: AbsolutePathBuf) -> AbsolutePathBuf { path } +fn normalize_trusted_top_level_alias(path: AbsolutePathBuf) -> AbsolutePathBuf { + let Some(top_level) = path.as_path().ancestors().find(|ancestor| { + ancestor.parent().is_some() && ancestor.parent().and_then(Path::parent).is_none() + }) else { + return path; + }; + let Ok(metadata) = std::fs::symlink_metadata(top_level) else { + return path; + }; + if !metadata.file_type().is_symlink() { + return path; + } + let Ok(canonical_top_level) = top_level.canonicalize() else { + return path; + }; + let Ok(suffix) = path.as_path().strip_prefix(top_level) else { + return path; + }; + AbsolutePathBuf::from_absolute_path(canonical_top_level.join(suffix)).unwrap_or(path) +} + pub(crate) fn default_read_only_subpaths_for_writable_root( writable_root: &AbsolutePathBuf, protect_missing_dot_codex: bool, @@ -2296,6 +2360,94 @@ mod tests { ); } + #[cfg(target_os = "macos")] + #[test] + fn preserving_mutable_paths_normalizes_top_level_aliases_consistently() { + let root = TempDir::new_in("/tmp").expect("tempdir under /tmp"); + let logical_root = + AbsolutePathBuf::from_absolute_path(root.path()).expect("absolute logical root"); + let canonical_root = AbsolutePathBuf::from_absolute_path( + root.path().canonicalize().expect("canonicalize root"), + ) + .expect("absolute canonical root"); + let protected = canonical_root.join("protected"); + fs::create_dir(&protected).expect("create protected path"); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new(logical_root.into(), FileSystemAccessMode::Write), + FileSystemSandboxEntry::new(protected.clone().into(), FileSystemAccessMode::Read), + ]); + + let roots = policy.get_writable_roots_with_cwd_preserving_mutable_paths(root.path()); + + assert_eq!(roots.len(), 1); + assert_eq!(roots[0].root, canonical_root); + assert!(roots[0].read_only_subpaths.contains(&protected)); + } + + #[cfg(unix)] + #[test] + fn preserving_writable_roots_cannot_be_rebound_during_projection() { + use std::sync::Arc; + use std::sync::atomic::AtomicBool; + use std::sync::atomic::AtomicUsize; + use std::sync::atomic::Ordering; + use std::thread; + + let tmp = TempDir::new().expect("tempdir"); + let active_ancestor = tmp.path().join("active"); + let parked_ancestor = tmp.path().join("parked"); + let outside_ancestor = tmp.path().join("outside"); + let writable_root = active_ancestor.join("workspace"); + let outside_root = outside_ancestor.join("workspace"); + fs::create_dir_all(&writable_root).expect("create writable root"); + fs::create_dir_all(&outside_root).expect("create outside root"); + let writable_root = + AbsolutePathBuf::from_absolute_path(writable_root).expect("absolute writable root"); + let outside_root = + AbsolutePathBuf::from_absolute_path(outside_root).expect("absolute outside root"); + let expected_writable_root = normalize_trusted_top_level_alias(writable_root.clone()); + let expected_outside_root = normalize_trusted_top_level_alias(outside_root); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new( + writable_root.into(), + FileSystemAccessMode::Write, + )]); + + let stop = Arc::new(AtomicBool::new(false)); + let swaps = Arc::new(AtomicUsize::new(0)); + let racer_stop = Arc::clone(&stop); + let racer_swaps = Arc::clone(&swaps); + let racer = thread::spawn(move || { + while !racer_stop.load(Ordering::Relaxed) { + if fs::rename(&active_ancestor, &parked_ancestor).is_err() { + thread::yield_now(); + continue; + } + if symlink_dir(&outside_ancestor, &active_ancestor).is_ok() { + racer_swaps.fetch_add(1, Ordering::Relaxed); + thread::yield_now(); + let _ = fs::remove_file(&active_ancestor); + } + fs::rename(&parked_ancestor, &active_ancestor).expect("restore writable ancestor"); + } + }); + + let mut rebound_root = None; + for _ in 0..2_000 { + let roots = policy.get_writable_roots_with_cwd_preserving_mutable_paths(tmp.path()); + if roots.len() != 1 || roots[0].root != expected_writable_root { + rebound_root = roots.first().map(|root| root.root.clone()); + break; + } + assert_ne!(roots[0].root, expected_outside_root); + thread::yield_now(); + } + stop.store(true, Ordering::Relaxed); + racer.join().expect("join path racer"); + + assert!(swaps.load(Ordering::Relaxed) > 0, "racer did not run"); + assert_eq!(rebound_root, None); + } + #[test] fn legacy_workspace_write_projection_preserves_symbolic_project_root() { let policy = SandboxPolicy::WorkspaceWrite { diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs index a684d0914f..037bf6c57f 100644 --- a/codex-rs/sandboxing/src/seatbelt.rs +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -9,7 +9,6 @@ use codex_protocol::permissions::PROTECTED_METADATA_PATH_NAMES; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::WritableRoot; use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_absolute_path::canonicalize_preserving_symlinks; use std::collections::BTreeMap; use std::collections::BTreeSet; use std::collections::HashMap; @@ -381,6 +380,18 @@ enum SeatbeltAccessKind { Write, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum SeatbeltPathMatch { + Literal, + Subpath, +} + +#[derive(Debug)] +enum NormalizedWritableRoot { + Subpath(AbsolutePathBuf), + Literal(AbsolutePathBuf), +} + fn nested_symlink_component(path: &Path) -> Option<&Path> { // Keep top-level macOS aliases such as `/tmp -> /private/tmp` compatible, // but reject symlinks in user-controlled path components. @@ -392,9 +403,43 @@ fn nested_symlink_component(path: &Path) -> Option<&Path> { }) } +fn normalize_top_level_alias_for_sandbox( + path: AbsolutePathBuf, +) -> Result { + let Some(top_level) = path.as_path().ancestors().find(|ancestor| { + ancestor.parent().is_some() && ancestor.parent().and_then(Path::parent).is_none() + }) else { + return Ok(path); + }; + if !std::fs::symlink_metadata(top_level).is_ok_and(|metadata| metadata.file_type().is_symlink()) + { + return Ok(path); + } + + let canonical_top_level = top_level.canonicalize().map_err(|err| { + SeatbeltPreparationError::FileSystem(format!( + "failed to normalize top-level alias {} for Seatbelt: {err}", + top_level.display() + )) + })?; + let suffix = path.as_path().strip_prefix(top_level).map_err(|err| { + SeatbeltPreparationError::FileSystem(format!( + "failed to preserve path {} after normalizing {}: {err}", + path.display(), + top_level.display() + )) + })?; + AbsolutePathBuf::from_absolute_path(canonical_top_level.join(suffix)).map_err(|err| { + SeatbeltPreparationError::FileSystem(format!( + "failed to normalize top-level alias for path {}: {err}", + path.display() + )) + }) +} + fn normalize_writable_root_for_sandbox( root: AbsolutePathBuf, -) -> Result { +) -> Result { if let Some(symlink) = nested_symlink_component(root.as_path()) { return Err(SeatbeltPreparationError::FileSystem(format!( "writable root {} contains symlink component {}; symlinked writable roots are not supported", @@ -403,18 +448,28 @@ fn normalize_writable_root_for_sandbox( ))); } - let normalized = canonicalize_preserving_symlinks(root.as_path()).map_err(|err| { - SeatbeltPreparationError::FileSystem(format!( - "failed to normalize writable root {} for Seatbelt: {err}", - root.display() - )) - })?; - AbsolutePathBuf::from_absolute_path(normalized).map_err(|err| { - SeatbeltPreparationError::FileSystem(format!( - "failed to normalize writable root {} for Seatbelt: {err}", - root.display() - )) - }) + // Resolve only top-level system aliases such as `/tmp -> /private/tmp`. + // Deeper components can be mutated by an already-running sandboxed process, + // so following them here would turn a path check into a new authority grant. + let normalized = normalize_top_level_alias_for_sandbox(root)?; + + let metadata = match std::fs::symlink_metadata(normalized.as_path()) { + Ok(metadata) => metadata, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + return Ok(NormalizedWritableRoot::Subpath(normalized)); + } + Err(err) => { + return Err(SeatbeltPreparationError::FileSystem(format!( + "failed to inspect Seatbelt writable root {}: {err}", + normalized.display() + ))); + } + }; + if metadata.is_dir() { + return Ok(NormalizedWritableRoot::Subpath(normalized)); + } + + Ok(NormalizedWritableRoot::Literal(normalized)) } fn build_seatbelt_access_policy( @@ -430,11 +485,18 @@ fn build_seatbelt_access_policy( }; for (index, access_root) in roots.into_iter().enumerate() { - let root = match access_kind { + let (root, path_match) = match access_kind { SeatbeltAccessKind::Read => { - normalize_path_for_sandbox(access_root.root.as_path()).unwrap_or(access_root.root) + let root = normalize_path_for_sandbox(access_root.root.as_path()) + .unwrap_or(access_root.root); + (root, SeatbeltPathMatch::Subpath) + } + SeatbeltAccessKind::Write => { + match normalize_writable_root_for_sandbox(access_root.root)? { + NormalizedWritableRoot::Subpath(root) => (root, SeatbeltPathMatch::Subpath), + NormalizedWritableRoot::Literal(root) => (root, SeatbeltPathMatch::Literal), + } } - SeatbeltAccessKind::Write => normalize_writable_root_for_sandbox(access_root.root)?, }; let root_param = format!("{param_prefix}_{index}"); params.push((root_param.clone(), root.clone().into_path_buf())); @@ -445,31 +507,52 @@ fn build_seatbelt_access_policy( "(deny file-write-unlink (require-all (literal (param \"{root_param}\")) (vnode-type DIRECTORY)))" )); } + let root_filter = match path_match { + SeatbeltPathMatch::Literal => format!("(literal (param \"{root_param}\"))"), + SeatbeltPathMatch::Subpath => format!("(subpath (param \"{root_param}\"))"), + }; if access_root.excluded_subpaths.is_empty() && access_root.protected_metadata_names.is_empty() { - policy_components.push(format!("(subpath (param \"{root_param}\"))")); + policy_components.push(root_filter); continue; } - let mut require_parts = vec![format!("(subpath (param \"{root_param}\"))")]; + let mut require_parts = vec![root_filter]; for (excluded_index, excluded_subpath) in access_root.excluded_subpaths.into_iter().enumerate() { - let excluded_subpath = - normalize_path_for_sandbox(excluded_subpath.as_path()).unwrap_or(excluded_subpath); let excluded_param = format!("{param_prefix}_{index}_EXCLUDED_{excluded_index}"); - params.push((excluded_param.clone(), excluded_subpath.into_path_buf())); - // Exclude both the exact protected path and anything beneath it. - // `subpath` alone leaves a gap for first-time creation of the - // protected directory itself, such as `mkdir .codex`. - require_parts.push(format!( - "(require-not (literal (param \"{excluded_param}\")))" - )); - require_parts.push(format!( - "(require-not (subpath (param \"{excluded_param}\")))" - )); + let excluded_subpaths = match access_kind { + SeatbeltAccessKind::Read => vec![( + excluded_param.clone(), + normalize_path_for_sandbox(excluded_subpath.as_path()) + .unwrap_or(excluded_subpath), + )], + SeatbeltAccessKind::Write => { + let logical = normalize_top_level_alias_for_sandbox(excluded_subpath)?; + let resolved = normalize_path_for_sandbox(logical.as_path()) + .filter(|resolved| resolved != &logical); + let mut paths = vec![(excluded_param.clone(), logical)]; + if let Some(resolved) = resolved { + paths.push((format!("{excluded_param}_RESOLVED"), resolved)); + } + paths + } + }; + for (excluded_param, excluded_subpath) in excluded_subpaths { + params.push((excluded_param.clone(), excluded_subpath.into_path_buf())); + // Exclude both the exact protected path and anything beneath it. + // `subpath` alone leaves a gap for first-time creation of the + // protected directory itself, such as `mkdir .codex`. + require_parts.push(format!( + "(require-not (literal (param \"{excluded_param}\")))" + )); + require_parts.push(format!( + "(require-not (subpath (param \"{excluded_param}\")))" + )); + } } for metadata_name in access_root.protected_metadata_names { let regex = @@ -775,7 +858,8 @@ 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.get_writable_roots_with_cwd(sandbox_policy_cwd); + let writable_roots = file_system_sandbox_policy + .get_writable_roots_with_cwd_preserving_mutable_paths(sandbox_policy_cwd); // Protect ancestors of read-only paths so renaming a writable directory // cannot move its descendants outside their policy carveouts. let mut protected_ancestors = BTreeSet::new(); diff --git a/codex-rs/sandboxing/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs index 415358a221..4a8c195eb0 100644 --- a/codex-rs/sandboxing/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -65,7 +65,7 @@ fn seatbelt_policy_arg(args: &[String]) -> &str { .expect("seatbelt args should include policy text") } -#[cfg(target_os = "macos")] +#[cfg(unix)] fn restricted_write_policy(paths: &[&Path]) -> FileSystemSandboxPolicy { let mut entries = vec![FileSystemSandboxEntry::new( FileSystemPath::Special { @@ -1553,6 +1553,51 @@ fn seatbelt_prevents_writable_root_replacement() { ); } +#[cfg(unix)] +#[test] +fn create_seatbelt_args_uses_literal_non_directory_writable_roots() { + let tmp = TempDir::new().expect("tempdir"); + let target = tmp.path().join("target.txt"); + fs::write(&target, "contents").expect("write target"); + let policy = restricted_write_policy(&[target.as_path(), Path::new("/dev/null")]); + let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { + command: vec!["/usr/bin/true".to_string()], + file_system_sandbox_policy: &policy, + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox_policy_cwd: tmp.path(), + enforce_managed_network: false, + managed_network: None, + environment_id: None, + network: None, + extra_allow_unix_sockets: &[], + }) + .expect("file writable root should be supported"); + let policy_text = seatbelt_policy_arg(&args); + + assert!( + policy_text.contains("(literal (param \"WRITABLE_ROOT_0\"))"), + "expected literal file grant in policy:\n{policy_text}" + ); + assert!( + !policy_text.contains("(subpath (param \"WRITABLE_ROOT_0\"))"), + "file grant should not include descendants:\n{policy_text}" + ); + assert!( + policy_text.contains("(literal (param \"WRITABLE_ROOT_1\"))"), + "expected literal device grant in policy:\n{policy_text}" + ); + assert!( + !policy_text.contains("(subpath (param \"WRITABLE_ROOT_1\"))"), + "device grant should not include descendants:\n{policy_text}" + ); + assert!( + policy_text.contains( + "(deny file-write-unlink (require-all (literal (param \"WRITABLE_ROOT_0\")) (vnode-type DIRECTORY)))" + ), + "file grant should protect the path if it becomes a directory:\n{policy_text}" + ); +} + #[cfg(target_os = "macos")] #[test] fn seatbelt_allows_file_root_replacement_and_deletion() { @@ -1595,6 +1640,159 @@ fn seatbelt_allows_file_root_replacement_and_deletion() { ); assert!(!target.exists(), "target should have been deleted"); assert!(!replacement.exists(), "replacement should have been moved"); + + create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { + command: vec!["/usr/bin/true".to_string()], + file_system_sandbox_policy: &policy, + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox_policy_cwd: tmp.path(), + enforce_managed_network: false, + managed_network: None, + environment_id: None, + network: None, + extra_allow_unix_sockets: &[], + }) + .expect("deleted file roots should remain usable by later commands"); +} + +#[cfg(target_os = "macos")] +#[test] +fn seatbelt_file_root_does_not_follow_replacement_symlink() { + use std::os::unix::fs::symlink; + + let tmp = TempDir::new().expect("tempdir"); + let writable_file = tmp.path().join("writable.txt"); + let outside_file = tmp.path().join("outside.txt"); + fs::write(&writable_file, "writable").expect("write writable file"); + fs::write(&outside_file, "outside").expect("write outside file"); + let policy = restricted_write_policy(&[writable_file.as_path()]); + let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { + command: vec![ + "/bin/sh".to_string(), + "-c".to_string(), + "printf escaped > \"$1\"".to_string(), + "sh".to_string(), + writable_file.display().to_string(), + ], + file_system_sandbox_policy: &policy, + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox_policy_cwd: tmp.path(), + enforce_managed_network: false, + managed_network: None, + environment_id: None, + network: None, + extra_allow_unix_sockets: &[], + }) + .expect("build seatbelt command"); + fs::remove_file(&writable_file).expect("remove writable file"); + symlink(&outside_file, &writable_file).expect("replace writable file with symlink"); + + let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) + .args(&args) + .current_dir(tmp.path()) + .output() + .expect("execute seatbelt command"); + + assert!( + !output.status.success(), + "replacement symlink should not grant access to its target" + ); + assert_eq!( + fs::read_to_string(&outside_file).expect("read outside file"), + "outside" + ); +} + +#[cfg(target_os = "macos")] +#[test] +fn seatbelt_protects_file_root_replaced_with_directory() { + let tmp = TempDir::new().expect("tempdir"); + let writable_file = tmp.path().join("writable"); + fs::write(&writable_file, "contents").expect("write writable file"); + let policy = restricted_write_policy(&[writable_file.as_path()]); + let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { + command: vec![ + "/bin/rmdir".to_string(), + writable_file.display().to_string(), + ], + file_system_sandbox_policy: &policy, + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox_policy_cwd: tmp.path(), + enforce_managed_network: false, + managed_network: None, + environment_id: None, + network: None, + extra_allow_unix_sockets: &[], + }) + .expect("build seatbelt command"); + fs::remove_file(&writable_file).expect("remove writable file"); + fs::create_dir(&writable_file).expect("replace writable file with directory"); + + let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) + .args(&args) + .current_dir(tmp.path()) + .output() + .expect("execute seatbelt command"); + + assert!( + !output.status.success(), + "directory replacement should remain anchored" + ); + assert!( + writable_file.is_dir(), + "replacement directory should remain in place" + ); +} + +#[cfg(target_os = "macos")] +#[test] +fn seatbelt_does_not_follow_rebound_writable_root_ancestor() { + use std::os::unix::fs::symlink; + + let tmp = TempDir::new().expect("tempdir"); + let ancestor = tmp.path().join("ancestor"); + let original_ancestor = tmp.path().join("ancestor-original"); + let writable_root = ancestor.join("workspace"); + let outside_ancestor = tmp.path().join("outside"); + let outside_root = outside_ancestor.join("workspace"); + let logical_file = writable_root.join("escaped.txt"); + let escaped_file = outside_root.join("escaped.txt"); + fs::create_dir_all(&writable_root).expect("create writable root"); + fs::create_dir_all(&outside_root).expect("create outside root"); + let policy = restricted_write_policy(&[writable_root.as_path()]); + let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { + command: vec![ + "/bin/sh".to_string(), + "-c".to_string(), + "printf escaped > \"$1\"".to_string(), + "sh".to_string(), + logical_file.display().to_string(), + ], + file_system_sandbox_policy: &policy, + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox_policy_cwd: tmp.path(), + enforce_managed_network: false, + managed_network: None, + environment_id: None, + network: None, + extra_allow_unix_sockets: &[], + }) + .expect("build seatbelt command"); + + fs::rename(&ancestor, &original_ancestor).expect("move writable ancestor"); + symlink(&outside_ancestor, &ancestor).expect("rebind writable ancestor"); + + let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) + .args(&args) + .current_dir(tmp.path()) + .output() + .expect("execute seatbelt command"); + + assert!( + !output.status.success(), + "rebound ancestor should not grant the outside root" + ); + assert!(!escaped_file.exists(), "outside file should not be created"); } #[cfg(target_os = "macos")] @@ -1603,13 +1801,15 @@ fn seatbelt_prevents_writable_directory_root_rename() { let tmp = TempDir::new().expect("tempdir"); let source = tmp.path().join("source"); let destination = tmp.path().join("destination"); + let renamed = destination.join("renamed"); fs::create_dir(&source).expect("create source"); + fs::create_dir(&destination).expect("create destination"); let policy = restricted_write_policy(&[source.as_path(), destination.as_path()]); let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { command: vec![ "/bin/mv".to_string(), source.display().to_string(), - destination.display().to_string(), + renamed.display().to_string(), ], file_system_sandbox_policy: &policy, network_sandbox_policy: NetworkSandboxPolicy::Restricted, @@ -1634,8 +1834,8 @@ fn seatbelt_prevents_writable_directory_root_rename() { ); assert!(source.is_dir(), "source directory should remain in place"); assert!( - !destination.exists(), - "destination should not have been created" + !renamed.exists(), + "renamed directory should not have been created" ); } @@ -1644,14 +1844,17 @@ fn seatbelt_prevents_writable_directory_root_rename() { fn seatbelt_protects_writable_root_created_as_directory() { let tmp = TempDir::new().expect("tempdir"); let writable_root = tmp.path().join("writable-root"); - let policy = restricted_write_policy(&[writable_root.as_path()]); + let progress = tmp.path().join("progress.txt"); + fs::write(&progress, "pending").expect("write progress file"); + let policy = restricted_write_policy(&[writable_root.as_path(), progress.as_path()]); let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { command: vec![ "/bin/sh".to_string(), "-c".to_string(), - "mkdir \"$1\" && rmdir \"$1\"".to_string(), + "mkdir \"$1\" && touch \"$1/file\" && printf ok > \"$2\" && rm \"$1/file\" && rmdir \"$1\"".to_string(), "sh".to_string(), writable_root.display().to_string(), + progress.display().to_string(), ], file_system_sandbox_policy: &policy, network_sandbox_policy: NetworkSandboxPolicy::Restricted, @@ -1678,6 +1881,60 @@ fn seatbelt_protects_writable_root_created_as_directory() { writable_root.is_dir(), "newly created directory root should remain protected" ); + assert_eq!( + fs::read_to_string(&progress).expect("read progress file"), + "ok", + "missing writable root should allow descendant writes" + ); +} + +#[cfg(target_os = "macos")] +#[test] +fn seatbelt_protects_resolved_target_of_symlinked_metadata_directory() { + use std::os::unix::fs::symlink; + + let tmp = TempDir::new().expect("tempdir"); + let writable_root = tmp.path().join("workspace"); + let actual_config = writable_root.join("actual-config"); + let dot_codex = writable_root.join(".codex"); + let config_toml = actual_config.join("config.toml"); + fs::create_dir_all(&actual_config).expect("create actual config directory"); + fs::write(&config_toml, "original").expect("write config"); + symlink(&actual_config, &dot_codex).expect("create .codex symlink"); + let policy = restricted_write_policy(&[writable_root.as_path()]); + let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { + command: vec![ + "/bin/sh".to_string(), + "-c".to_string(), + "printf escaped > \"$1\"".to_string(), + "sh".to_string(), + config_toml.display().to_string(), + ], + file_system_sandbox_policy: &policy, + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox_policy_cwd: &writable_root, + enforce_managed_network: false, + managed_network: None, + environment_id: None, + network: None, + extra_allow_unix_sockets: &[], + }) + .expect("build seatbelt command"); + + let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) + .args(&args) + .current_dir(&writable_root) + .output() + .expect("execute seatbelt command"); + + assert!( + !output.status.success(), + "resolved .codex target should remain read-only" + ); + assert_eq!( + fs::read_to_string(&config_toml).expect("read config"), + "original" + ); } #[test] @@ -1927,13 +2184,13 @@ fn create_seatbelt_args_for_cwd_as_git_repo() { args.contains(&expected_dot_codex), "missing {expected_dot_codex}: {args:#?}" ); - let unexpected_dot_agents = format!( - "-DWRITABLE_ROOT_0_EXCLUDED_1={}", + let expected_dot_agents = format!( + "-DWRITABLE_ROOT_0_EXCLUDED_2={}", dot_agents_canonical.to_string_lossy() ); assert!( - !args.contains(&unexpected_dot_agents), - "missing .agents should be handled by regex rather than materialized as a path param: {args:#?}" + args.contains(&expected_dot_agents), + "missing {expected_dot_agents}: {args:#?}" ); let expected_slash_tmp = format!("-DWRITABLE_ROOT_1={}", slash_tmp.to_string_lossy()); assert!(