diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index ff40f3663d0b..22e7d72531a2 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -3,8 +3,8 @@ //! This module mirrors the semantics used by the macOS Seatbelt sandbox: //! - the filesystem is read-only by default, //! - explicit writable roots are layered on top, and -//! - sensitive subpaths such as `.git` and `.codex` remain read-only even when -//! their parent root is writable. +//! - sensitive subpaths such as `.git`, `.agents`, and `.codex` remain +//! read-only even when their parent root is writable. //! //! The overall Linux sandbox is composed of: //! - seccomp + `PR_SET_NO_NEW_PRIVS` applied in-process, and @@ -15,15 +15,18 @@ use std::collections::HashSet; use std::ffi::OsString; use std::fs; use std::fs::File; +use std::fs::Metadata; use std::io; use std::os::fd::AsRawFd; use std::os::unix::ffi::OsStringExt; +use std::os::unix::fs::MetadataExt; use std::path::Path; use std::path::PathBuf; use std::process::Command; use codex_protocol::error::CodexErr; use codex_protocol::error::Result; +use codex_protocol::permissions::is_protected_metadata_name; use codex_protocol::protocol::FileSystemAccessMode; use codex_protocol::protocol::FileSystemPath; use codex_protocol::protocol::FileSystemSandboxPolicy; @@ -104,6 +107,121 @@ impl BwrapNetworkMode { pub(crate) struct BwrapArgs { pub args: Vec, pub preserved_files: Vec, + pub synthetic_mount_targets: Vec, + pub protected_create_targets: Vec, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct FileIdentity { + dev: u64, + ino: u64, +} + +impl FileIdentity { + fn from_metadata(metadata: &Metadata) -> Self { + Self { + dev: metadata.dev(), + ino: metadata.ino(), + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum SyntheticMountTargetKind { + EmptyFile, + EmptyDirectory, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct SyntheticMountTarget { + path: PathBuf, + kind: SyntheticMountTargetKind, + // If an empty metadata path was already present, remember its inode so + // cleanup does not delete a real pre-existing file or directory. + pre_existing_path: Option, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ProtectedCreateTarget { + path: PathBuf, +} + +impl ProtectedCreateTarget { + pub(crate) fn missing(path: &Path) -> Self { + Self { + path: path.to_path_buf(), + } + } + + pub(crate) fn path(&self) -> &Path { + &self.path + } +} + +impl SyntheticMountTarget { + pub(crate) fn missing(path: &Path) -> Self { + Self { + path: path.to_path_buf(), + kind: SyntheticMountTargetKind::EmptyFile, + pre_existing_path: None, + } + } + + pub(crate) fn missing_empty_directory(path: &Path) -> Self { + Self { + path: path.to_path_buf(), + kind: SyntheticMountTargetKind::EmptyDirectory, + pre_existing_path: None, + } + } + + pub(crate) fn existing_empty_file(path: &Path, metadata: &Metadata) -> Self { + Self { + path: path.to_path_buf(), + kind: SyntheticMountTargetKind::EmptyFile, + pre_existing_path: Some(FileIdentity::from_metadata(metadata)), + } + } + + fn existing_empty_directory(path: &Path, metadata: &Metadata) -> Self { + Self { + path: path.to_path_buf(), + kind: SyntheticMountTargetKind::EmptyDirectory, + pre_existing_path: Some(FileIdentity::from_metadata(metadata)), + } + } + + pub(crate) fn preserves_pre_existing_path(&self) -> bool { + self.pre_existing_path.is_some() + } + + pub(crate) fn path(&self) -> &Path { + &self.path + } + + pub(crate) fn kind(&self) -> SyntheticMountTargetKind { + self.kind + } + + pub(crate) fn should_remove_after_bwrap(&self, metadata: &Metadata) -> bool { + match self.kind { + SyntheticMountTargetKind::EmptyFile => { + if !metadata.file_type().is_file() || metadata.len() != 0 { + return false; + } + } + SyntheticMountTargetKind::EmptyDirectory => { + if !metadata.file_type().is_dir() { + return false; + } + } + } + + match self.pre_existing_path { + Some(pre_existing_path) => pre_existing_path != FileIdentity::from_metadata(metadata), + None => true, + } + } } /// Wrap a command with bubblewrap so the filesystem is read-only by default, @@ -129,6 +247,8 @@ pub(crate) fn create_bwrap_command_args( Ok(BwrapArgs { args: command, preserved_files: Vec::new(), + synthetic_mount_targets: Vec::new(), + protected_create_targets: Vec::new(), }) } else { Ok(create_bwrap_flags_full_filesystem(command, options)) @@ -168,6 +288,8 @@ fn create_bwrap_flags_full_filesystem(command: Vec, options: BwrapOption BwrapArgs { args, preserved_files: Vec::new(), + synthetic_mount_targets: Vec::new(), + protected_create_targets: Vec::new(), } } @@ -182,6 +304,8 @@ fn create_bwrap_flags( let BwrapArgs { args: filesystem_args, preserved_files, + synthetic_mount_targets, + protected_create_targets, } = create_filesystem_args( file_system_sandbox_policy, sandbox_policy_cwd, @@ -219,6 +343,8 @@ fn create_bwrap_flags( Ok(BwrapArgs { args, preserved_files, + synthetic_mount_targets, + protected_create_targets, }) } @@ -277,14 +403,18 @@ fn create_filesystem_args( else { return None; }; - // Missing `.codex` remains protected so first-time project config - // creation still goes through the protected-path approval flow. - // Only the automatic repo-metadata read masks are skipped here: - // user-authored `read` rules for other subpaths and `none` rules - // should keep their normal bwrap behavior, which can mask the - // first missing component to prevent creation under writable roots. + // Automatic repo-metadata read masks are skipped here so the + // metadata handling below can apply the root-scoped + // protection consistently for `.git`, `.agents`, and `.codex`. + // User-authored `read` rules for other subpaths and `none` + // rules should keep their normal bwrap behavior, which can mask + // the first missing component to prevent creation under writable + // roots. let project_subpath = subpath.as_path(); - if project_subpath != Path::new(".git") && project_subpath != Path::new(".agents") { + if project_subpath != Path::new(".git") + && project_subpath != Path::new(".agents") + && project_subpath != Path::new(".codex") + { return None; } let resolved = AbsolutePathBuf::resolve_path_against_base(subpath, cwd); @@ -307,7 +437,7 @@ fn create_filesystem_args( unreadable_roots.sort(); unreadable_roots.dedup(); - let mut args = if file_system_sandbox_policy.has_full_disk_read_access() { + let args = if file_system_sandbox_policy.has_full_disk_read_access() { // Read-only root, then mount a minimal device tree. // In bubblewrap (`bubblewrap.c`, `SETUP_MOUNT_DEV`), `--dev /dev` // creates the standard minimal nodes: null, zero, full, random, @@ -380,7 +510,12 @@ fn create_filesystem_args( args }; - let mut preserved_files = Vec::new(); + let mut bwrap_args = BwrapArgs { + args, + preserved_files: Vec::new(), + synthetic_mount_targets: Vec::new(), + protected_create_targets: Vec::new(), + }; let mut allowed_write_paths = Vec::with_capacity(writable_roots.len()); for writable_root in &writable_roots { let root = writable_root.root.as_path(); @@ -411,12 +546,7 @@ fn create_filesystem_args( unreadable_ancestors_of_writable_roots.sort_by_key(|path| path_depth(path)); for unreadable_root in &unreadable_ancestors_of_writable_roots { - append_unreadable_root_args( - &mut args, - &mut preserved_files, - unreadable_root, - &allowed_write_paths, - )?; + append_unreadable_root_args(&mut bwrap_args, unreadable_root, &allowed_write_paths)?; } for writable_root in &sorted_writable_roots { @@ -430,13 +560,13 @@ fn create_filesystem_args( .filter(|unreadable_root| root.starts_with(unreadable_root)) .max_by_key(|unreadable_root| path_depth(unreadable_root)) { - append_mount_target_parent_dir_args(&mut args, root, masking_root); + append_mount_target_parent_dir_args(&mut bwrap_args.args, root, masking_root); } let mount_root = symlink_target.as_deref().unwrap_or(root); - args.push("--bind".to_string()); - args.push(path_to_string(mount_root)); - args.push(path_to_string(mount_root)); + bwrap_args.args.push("--bind".to_string()); + bwrap_args.args.push(path_to_string(mount_root)); + bwrap_args.args.push(path_to_string(mount_root)); let mut read_only_subpaths: Vec = writable_root .read_only_subpaths @@ -445,12 +575,26 @@ fn create_filesystem_args( .filter(|path| !unreadable_paths.contains(path)) .filter(|path| !missing_auto_metadata_read_only_project_root_subpaths.contains(path)) .collect(); + let protected_metadata_names = writable_root.protected_metadata_names.clone(); + append_metadata_path_masks_for_writable_root( + &mut read_only_subpaths, + root, + mount_root, + &protected_metadata_names, + ); if let Some(target) = &symlink_target { read_only_subpaths = remap_paths_for_symlink_target(read_only_subpaths, root, target); } + append_protected_create_targets_for_writable_root( + &mut bwrap_args, + &protected_metadata_names, + root, + symlink_target.as_deref(), + &read_only_subpaths, + ); read_only_subpaths.sort_by_key(|path| path_depth(path)); for subpath in read_only_subpaths { - append_read_only_subpath_args(&mut args, &subpath, &allowed_write_paths)?; + append_read_only_subpath_args(&mut bwrap_args, &subpath, &allowed_write_paths)?; } let mut nested_unreadable_roots: Vec = unreadable_roots .iter() @@ -463,12 +607,7 @@ fn create_filesystem_args( } nested_unreadable_roots.sort_by_key(|path| path_depth(path)); for unreadable_root in nested_unreadable_roots { - append_unreadable_root_args( - &mut args, - &mut preserved_files, - &unreadable_root, - &allowed_write_paths, - )?; + append_unreadable_root_args(&mut bwrap_args, &unreadable_root, &allowed_write_paths)?; } } @@ -484,18 +623,78 @@ fn create_filesystem_args( .collect(); rootless_unreadable_roots.sort_by_key(|path| path_depth(path)); for unreadable_root in rootless_unreadable_roots { - append_unreadable_root_args( - &mut args, - &mut preserved_files, - &unreadable_root, - &allowed_write_paths, - )?; + append_unreadable_root_args(&mut bwrap_args, &unreadable_root, &allowed_write_paths)?; } - Ok(BwrapArgs { - args, - preserved_files, - }) + Ok(bwrap_args) +} + +fn append_protected_create_targets_for_writable_root( + bwrap_args: &mut BwrapArgs, + protected_metadata_names: &[String], + root: &Path, + symlink_target: Option<&Path>, + read_only_subpaths: &[PathBuf], +) { + for name in protected_metadata_names { + let mut path = root.join(name); + if let Some(target) = symlink_target + && let Ok(relative_path) = path.strip_prefix(root) + { + path = target.join(relative_path); + } + if read_only_subpaths.iter().any(|subpath| subpath == &path) || path.exists() { + continue; + } + bwrap_args + .protected_create_targets + .push(ProtectedCreateTarget::missing(&path)); + } +} + +fn append_metadata_path_masks_for_writable_root( + read_only_subpaths: &mut Vec, + root: &Path, + mount_root: &Path, + protected_metadata_names: &[String], +) { + for name in protected_metadata_names { + let path = root.join(name); + if should_leave_missing_git_for_parent_repo_discovery(mount_root, name) { + continue; + } + if !read_only_subpaths.iter().any(|subpath| subpath == &path) { + read_only_subpaths.push(path); + } + } +} + +fn should_leave_missing_git_for_parent_repo_discovery(mount_root: &Path, name: &str) -> bool { + let path = mount_root.join(name); + name == ".git" + && matches!( + path.symlink_metadata(), + Err(err) if err.kind() == io::ErrorKind::NotFound + ) + && mount_root + .ancestors() + .skip(1) + .any(ancestor_has_git_metadata) +} + +fn ancestor_has_git_metadata(ancestor: &Path) -> bool { + let git_path = ancestor.join(".git"); + let Ok(metadata) = git_path.symlink_metadata() else { + return false; + }; + if metadata.is_dir() { + return git_path.join("HEAD").symlink_metadata().is_ok(); + } + if metadata.is_file() { + return fs::read_to_string(git_path) + .is_ok_and(|contents| contents.trim_start().starts_with("gitdir:")); + } + false } fn expand_unreadable_globs_with_ripgrep( @@ -820,7 +1019,7 @@ fn append_mount_target_parent_dir_args(args: &mut Vec, mount_target: &Pa } fn append_read_only_subpath_args( - args: &mut Vec, + bwrap_args: &mut BwrapArgs, subpath: &Path, allowed_write_paths: &[PathBuf], ) -> Result<()> { @@ -838,28 +1037,107 @@ fn append_read_only_subpath_args( ))); } + if let Some(metadata) = transient_empty_metadata_path(subpath) + && is_within_allowed_write_paths(subpath, allowed_write_paths) + { + // Another concurrent bwrap setup can leave an empty mount target at + // a missing metadata path. Treat it like the missing case instead of + // binding that transient host path as the stable source. + match metadata { + EmptyProtectedMetadataPath::File(metadata) => { + append_existing_empty_file_bind_data_args(bwrap_args, subpath, &metadata)?; + } + EmptyProtectedMetadataPath::Directory(metadata) => { + append_existing_empty_directory_args(bwrap_args, subpath, &metadata); + } + } + return Ok(()); + } + if !subpath.exists() { if let Some(first_missing_component) = find_first_non_existent_component(subpath) && is_within_allowed_write_paths(&first_missing_component, allowed_write_paths) { - args.push("--ro-bind".to_string()); - args.push("/dev/null".to_string()); - args.push(path_to_string(&first_missing_component)); + append_missing_read_only_subpath_args(bwrap_args, &first_missing_component)?; } return Ok(()); } if is_within_allowed_write_paths(subpath, allowed_write_paths) { - args.push("--ro-bind".to_string()); - args.push(path_to_string(subpath)); - args.push(path_to_string(subpath)); + bwrap_args.args.push("--ro-bind".to_string()); + bwrap_args.args.push(path_to_string(subpath)); + bwrap_args.args.push(path_to_string(subpath)); + } + Ok(()) +} + +fn append_empty_file_bind_data_args(bwrap_args: &mut BwrapArgs, path: &Path) -> Result<()> { + if bwrap_args.preserved_files.is_empty() { + bwrap_args.preserved_files.push(File::open("/dev/null")?); } + let null_fd = bwrap_args.preserved_files[0].as_raw_fd().to_string(); + bwrap_args.args.push("--ro-bind-data".to_string()); + bwrap_args.args.push(null_fd); + bwrap_args.args.push(path_to_string(path)); Ok(()) } +fn append_empty_directory_args(bwrap_args: &mut BwrapArgs, path: &Path) { + bwrap_args.args.push("--perms".to_string()); + bwrap_args.args.push("555".to_string()); + bwrap_args.args.push("--tmpfs".to_string()); + bwrap_args.args.push(path_to_string(path)); + bwrap_args.args.push("--remount-ro".to_string()); + bwrap_args.args.push(path_to_string(path)); +} + +fn append_missing_read_only_subpath_args(bwrap_args: &mut BwrapArgs, path: &Path) -> Result<()> { + if path.file_name().is_some_and(is_protected_metadata_name) { + append_empty_directory_args(bwrap_args, path); + bwrap_args + .synthetic_mount_targets + .push(SyntheticMountTarget::missing_empty_directory(path)); + return Ok(()); + } + + append_missing_empty_file_bind_data_args(bwrap_args, path) +} + +fn append_missing_empty_file_bind_data_args(bwrap_args: &mut BwrapArgs, path: &Path) -> Result<()> { + append_empty_file_bind_data_args(bwrap_args, path)?; + bwrap_args + .synthetic_mount_targets + .push(SyntheticMountTarget::missing(path)); + Ok(()) +} + +fn append_existing_empty_file_bind_data_args( + bwrap_args: &mut BwrapArgs, + path: &Path, + metadata: &Metadata, +) -> Result<()> { + append_empty_file_bind_data_args(bwrap_args, path)?; + bwrap_args + .synthetic_mount_targets + .push(SyntheticMountTarget::existing_empty_file(path, metadata)); + Ok(()) +} + +fn append_existing_empty_directory_args( + bwrap_args: &mut BwrapArgs, + path: &Path, + metadata: &Metadata, +) { + append_empty_directory_args(bwrap_args, path); + bwrap_args + .synthetic_mount_targets + .push(SyntheticMountTarget::existing_empty_directory( + path, metadata, + )); +} + fn append_unreadable_root_args( - args: &mut Vec, - preserved_files: &mut Vec, + bwrap_args: &mut BwrapArgs, unreadable_root: &Path, allowed_write_paths: &[PathBuf], ) -> Result<()> { @@ -884,24 +1162,16 @@ fn append_unreadable_root_args( if let Some(first_missing_component) = find_first_non_existent_component(unreadable_root) && is_within_allowed_write_paths(&first_missing_component, allowed_write_paths) { - args.push("--ro-bind".to_string()); - args.push("/dev/null".to_string()); - args.push(path_to_string(&first_missing_component)); + append_missing_empty_file_bind_data_args(bwrap_args, &first_missing_component)?; } return Ok(()); } - append_existing_unreadable_path_args( - args, - preserved_files, - unreadable_root, - allowed_write_paths, - ) + append_existing_unreadable_path_args(bwrap_args, unreadable_root, allowed_write_paths) } fn append_existing_unreadable_path_args( - args: &mut Vec, - preserved_files: &mut Vec, + bwrap_args: &mut BwrapArgs, unreadable_root: &Path, allowed_write_paths: &[PathBuf], ) -> Result<()> { @@ -911,40 +1181,37 @@ fn append_existing_unreadable_path_args( .map(PathBuf::as_path) .filter(|path| *path != unreadable_root && path.starts_with(unreadable_root)) .collect(); - args.push("--perms".to_string()); + bwrap_args.args.push("--perms".to_string()); // Execute-only perms let the process traverse into explicitly // re-opened writable descendants while still hiding the denied // directory contents. Plain denied directories with no writable child // mounts stay at `000`. - args.push(if writable_descendants.is_empty() { + bwrap_args.args.push(if writable_descendants.is_empty() { "000".to_string() } else { "111".to_string() }); - args.push("--tmpfs".to_string()); - args.push(path_to_string(unreadable_root)); + bwrap_args.args.push("--tmpfs".to_string()); + bwrap_args.args.push(path_to_string(unreadable_root)); // Recreate any writable descendants inside the tmpfs before remounting // the denied parent read-only. Otherwise bubblewrap cannot mkdir the // nested mount targets after the parent has been frozen. writable_descendants.sort_by_key(|path| path_depth(path)); for writable_descendant in writable_descendants { - append_mount_target_parent_dir_args(args, writable_descendant, unreadable_root); + append_mount_target_parent_dir_args( + &mut bwrap_args.args, + writable_descendant, + unreadable_root, + ); } - args.push("--remount-ro".to_string()); - args.push(path_to_string(unreadable_root)); + bwrap_args.args.push("--remount-ro".to_string()); + bwrap_args.args.push(path_to_string(unreadable_root)); return Ok(()); } - if preserved_files.is_empty() { - preserved_files.push(File::open("/dev/null")?); - } - let null_fd = preserved_files[0].as_raw_fd().to_string(); - args.push("--perms".to_string()); - args.push("000".to_string()); - args.push("--ro-bind-data".to_string()); - args.push(null_fd); - args.push(path_to_string(unreadable_root)); - Ok(()) + bwrap_args.args.push("--perms".to_string()); + bwrap_args.args.push("000".to_string()); + append_empty_file_bind_data_args(bwrap_args, unreadable_root) } /// Returns true when `path` is under any allowed writable root. @@ -954,6 +1221,35 @@ fn is_within_allowed_write_paths(path: &Path, allowed_write_paths: &[PathBuf]) - .any(|root| path.starts_with(root)) } +enum EmptyProtectedMetadataPath { + File(Metadata), + Directory(Metadata), +} + +fn transient_empty_metadata_path(path: &Path) -> Option { + if !path.file_name().is_some_and(is_protected_metadata_name) { + return None; + } + + let metadata = fs::symlink_metadata(path).ok()?; + if metadata.file_type().is_file() && metadata.len() == 0 { + return Some(EmptyProtectedMetadataPath::File(metadata)); + } + + if metadata.file_type().is_dir() && directory_is_empty(path) { + return Some(EmptyProtectedMetadataPath::Directory(metadata)); + } + + None +} + +fn directory_is_empty(path: &Path) -> bool { + let Ok(mut entries) = fs::read_dir(path) else { + return false; + }; + entries.next().is_none() +} + fn first_writable_symlink_component_in_path( target_path: &Path, allowed_write_paths: &[PathBuf], @@ -1031,6 +1327,7 @@ fn find_first_non_existent_component(target_path: &Path) -> Option { #[cfg(test)] mod tests { use super::*; + use codex_protocol::protocol::FileSystemAccessMode; use codex_protocol::protocol::FileSystemPath; use codex_protocol::protocol::FileSystemSandboxEntry; @@ -1391,6 +1688,189 @@ mod tests { assert!(message.contains(&real_linked_private_str), "{message}"); } + #[test] + fn missing_read_only_subpath_uses_empty_file_bind_data() { + let temp_dir = TempDir::new().expect("temp dir"); + let workspace = temp_dir.path().join("workspace"); + let blocked = workspace.join("blocked"); + std::fs::create_dir_all(&workspace).expect("create workspace"); + + let workspace_root = + AbsolutePathBuf::from_absolute_path(&workspace).expect("absolute workspace"); + let blocked_root = AbsolutePathBuf::from_absolute_path(&blocked).expect("absolute blocked"); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: workspace_root, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: blocked_root }, + access: FileSystemAccessMode::Read, + }, + ]); + + let args = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); + + assert_empty_file_bound_without_perms(&args.args, &blocked); + assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".git")); + assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".agents")); + assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".codex")); + assert_eq!(args.preserved_files.len(), 1); + assert_eq!( + synthetic_mount_target_paths(&args), + vec![ + blocked.clone(), + workspace.join(".git"), + workspace.join(".agents"), + workspace.join(".codex"), + ] + ); + assert!( + !blocked.exists(), + "missing path mask should not materialize host-side metadata paths at arg construction time", + ); + } + + #[test] + fn transient_empty_preserved_file_uses_empty_file_bind_data() { + let temp_dir = TempDir::new().expect("temp dir"); + let workspace = temp_dir.path().join("workspace"); + let dot_git = workspace.join(".git"); + std::fs::create_dir_all(&workspace).expect("create workspace"); + File::create(&dot_git).expect("create empty .git file"); + + let workspace_root = + AbsolutePathBuf::from_absolute_path(&workspace).expect("absolute workspace"); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: workspace_root, + }, + access: FileSystemAccessMode::Write, + }]); + + let args = + create_filesystem_args(&policy, temp_dir.path(), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); + let dot_git_str = path_to_string(&dot_git); + + assert_empty_file_bound_without_perms(&args.args, &dot_git); + assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".agents")); + assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".codex")); + assert_eq!( + synthetic_mount_target_paths(&args), + vec![ + dot_git.clone(), + workspace.join(".agents"), + workspace.join(".codex"), + ] + ); + assert!( + !args + .args + .windows(3) + .any(|window| window == ["--ro-bind", dot_git_str.as_str(), dot_git_str.as_str()]), + "transient empty preserved file should not be treated as a stable bind source", + ); + let metadata = std::fs::symlink_metadata(&dot_git).expect("stat .git"); + assert!( + !args.synthetic_mount_targets[0].should_remove_after_bwrap(&metadata), + "pre-existing empty preserved files must not be cleaned up as synthetic targets", + ); + } + + #[test] + fn missing_child_git_under_parent_repo_uses_protected_create_target() { + let temp_dir = TempDir::new().expect("temp dir"); + let repo = temp_dir.path().join("repo"); + let workspace = repo.join("workspace"); + let dot_git = workspace.join(".git"); + std::fs::create_dir_all(repo.join(".git")).expect("create parent .git"); + std::fs::write(repo.join(".git/HEAD"), "ref: refs/heads/main\n").expect("write HEAD"); + std::fs::create_dir_all(&workspace).expect("create workspace"); + + let workspace_root = + AbsolutePathBuf::from_absolute_path(&workspace).expect("absolute workspace"); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: workspace_root, + }, + access: FileSystemAccessMode::Write, + }]); + + let args = create_filesystem_args(&policy, &workspace, NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); + assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".agents")); + assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".codex")); + let dot_git_str = path_to_string(&dot_git); + assert!( + !args + .args + .windows(4) + .any(|window| window == ["--perms", "555", "--tmpfs", dot_git_str.as_str()]), + "missing child .git should not shadow parent repo discovery", + ); + assert!( + !synthetic_mount_target_paths(&args).contains(&dot_git), + "missing child .git should not be a transient mount target", + ); + assert_eq!( + protected_create_target_paths(&args), + vec![dot_git], + "missing child .git should fail through protected create cleanup", + ); + } + + #[cfg(unix)] + #[test] + fn symlinked_missing_child_git_under_parent_repo_uses_effective_mount_root() { + let temp_dir = TempDir::new().expect("temp dir"); + let repo = temp_dir.path().join("repo"); + let workspace = repo.join("workspace"); + let link_repo = temp_dir.path().join("link-repo"); + let link_workspace = link_repo.join("workspace"); + let dot_git = workspace.join(".git"); + std::fs::create_dir_all(repo.join(".git")).expect("create parent .git"); + std::fs::write(repo.join(".git/HEAD"), "ref: refs/heads/main\n").expect("write HEAD"); + std::fs::create_dir_all(&workspace).expect("create workspace"); + std::os::unix::fs::symlink(&repo, &link_repo).expect("create symlinked repo"); + + let link_workspace_root = AbsolutePathBuf::from_absolute_path(&link_workspace) + .expect("absolute symlinked workspace"); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: link_workspace_root, + }, + access: FileSystemAccessMode::Write, + }]); + + let args = + create_filesystem_args(&policy, &link_workspace, NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH) + .expect("filesystem args"); + assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".agents")); + assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".codex")); + let dot_git_str = path_to_string(&dot_git); + assert!( + !args + .args + .windows(4) + .any(|window| window == ["--perms", "555", "--tmpfs", dot_git_str.as_str()]), + "symlinked missing child .git should not shadow parent repo discovery", + ); + assert!( + !synthetic_mount_target_paths(&args).contains(&dot_git), + "symlinked missing child .git should not be a transient mount target", + ); + assert_eq!( + protected_create_target_paths(&args), + vec![dot_git], + "symlinked missing child .git should fail through protected create cleanup", + ); + } + #[test] fn ignores_missing_writable_roots() { let temp_dir = TempDir::new().expect("temp dir"); @@ -1426,7 +1906,7 @@ mod tests { } #[test] - fn skips_missing_project_root_metadata_carveouts_except_codex() { + fn missing_project_root_metadata_carveouts_use_metadata_path_masks() { let temp_dir = TempDir::new().expect("temp dir"); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { @@ -1468,12 +1948,18 @@ mod tests { let dot_agents = path_to_string(&temp_dir.path().join(".agents")); let dot_codex = path_to_string(&temp_dir.path().join(".codex")); - assert!(!args.args.iter().any(|arg| arg == &dot_git)); - assert!(!args.args.iter().any(|arg| arg == &dot_agents)); - assert!( - args.args - .windows(3) - .any(|window| { window == ["--ro-bind", "/dev/null", dot_codex.as_str()] }) + assert_empty_directory_mounted_read_only(&args.args, Path::new(&dot_git)); + assert_empty_directory_mounted_read_only(&args.args, Path::new(&dot_agents)); + assert_empty_directory_mounted_read_only(&args.args, Path::new(&dot_codex)); + assert!(args.preserved_files.is_empty()); + let synthetic_targets = synthetic_mount_target_paths(&args); + assert!(synthetic_targets.contains(&PathBuf::from(&dot_git))); + assert!(synthetic_targets.contains(&PathBuf::from(&dot_agents))); + assert!(synthetic_targets.contains(&PathBuf::from(&dot_codex))); + assert_eq!( + protected_create_target_paths(&args), + Vec::::new(), + "missing protected metadata paths should fail at creation time through read-only mounts", ); } @@ -1513,16 +1999,8 @@ mod tests { let dot_vscode = path_to_string(&temp_dir.path().join(".vscode")); let dot_secrets = path_to_string(&temp_dir.path().join(".secrets")); - assert!( - args.args - .windows(3) - .any(|window| { window == ["--ro-bind", "/dev/null", dot_vscode.as_str()] }) - ); - assert!( - args.args - .windows(3) - .any(|window| { window == ["--ro-bind", "/dev/null", dot_secrets.as_str()] }) - ); + assert_empty_file_bound_without_perms(&args.args, Path::new(&dot_vscode)); + assert_empty_file_bound_without_perms(&args.args, Path::new(&dot_secrets)); } #[test] @@ -1539,6 +2017,18 @@ mod tests { NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH, ) .expect("bwrap fs args"); + assert!(args.preserved_files.is_empty()); + assert_eq!( + synthetic_mount_target_paths(&args), + vec![ + PathBuf::from("/.git"), + PathBuf::from("/.agents"), + PathBuf::from("/.codex"), + PathBuf::from("/dev/.git"), + PathBuf::from("/dev/.agents"), + PathBuf::from("/dev/.codex"), + ] + ); assert_eq!( args.args, vec![ @@ -1553,17 +2043,52 @@ mod tests { "--bind".to_string(), "/".to_string(), "/".to_string(), - // Mask the default protected .codex subpath under that writable - // root. Because the root is `/` in this test, the carveout path - // appears at the filesystem root. - "--ro-bind".to_string(), - "/dev/null".to_string(), + // Mask the default metadata path names under the writable root. + // Because the root is `/` in this test, these carveout paths + // appear directly below `/`. + "--perms".to_string(), + "555".to_string(), + "--tmpfs".to_string(), + "/.git".to_string(), + "--remount-ro".to_string(), + "/.git".to_string(), + "--perms".to_string(), + "555".to_string(), + "--tmpfs".to_string(), + "/.agents".to_string(), + "--remount-ro".to_string(), + "/.agents".to_string(), + "--perms".to_string(), + "555".to_string(), + "--tmpfs".to_string(), + "/.codex".to_string(), + "--remount-ro".to_string(), "/.codex".to_string(), // Rebind /dev after the root bind so device nodes remain // writable/usable inside the writable root. "--bind".to_string(), "/dev".to_string(), "/dev".to_string(), + // Then mask the metadata names that would otherwise be + // creatable below the writable /dev bind. + "--perms".to_string(), + "555".to_string(), + "--tmpfs".to_string(), + "/dev/.git".to_string(), + "--remount-ro".to_string(), + "/dev/.git".to_string(), + "--perms".to_string(), + "555".to_string(), + "--tmpfs".to_string(), + "/dev/.agents".to_string(), + "--remount-ro".to_string(), + "/dev/.agents".to_string(), + "--perms".to_string(), + "555".to_string(), + "--tmpfs".to_string(), + "/dev/.codex".to_string(), + "--remount-ro".to_string(), + "/dev/.codex".to_string(), ] ); } @@ -2027,6 +2552,7 @@ mod tests { let blocked_file_str = path_to_string(blocked_file.as_path()); assert_eq!(args.preserved_files.len(), 1); + assert!(args.synthetic_mount_targets.is_empty()); assert!(args.args.windows(5).any(|window| { window[0] == "--perms" && window[1] == "000" @@ -2130,4 +2656,52 @@ mod tests { "expected file mask for {path}: {args:#?}" ); } + + /// Assert that `path` is backed by an fd-supplied empty file without + /// changing the next mount operation's permissions. + fn assert_empty_file_bound_without_perms(args: &[String], path: &Path) { + let path = path_to_string(path); + assert!( + args.windows(3) + .any(|window| { window[0] == "--ro-bind-data" && window[2] == path }), + "expected empty file bind for {path}: {args:#?}" + ); + assert!( + !args.windows(5).any(|window| { + window[0] == "--perms" + && window[1] == "000" + && window[2] == "--ro-bind-data" + && window[4] == path + }), + "missing path bind should not set explicit file perms for {path}: {args:#?}" + ); + } + + fn assert_empty_directory_mounted_read_only(args: &[String], path: &Path) { + let path = path_to_string(path); + assert!( + args.windows(4) + .any(|window| window == ["--perms", "555", "--tmpfs", path.as_str()]), + "expected empty directory mount for {path}: {args:#?}" + ); + assert!( + args.windows(2) + .any(|window| window == ["--remount-ro", path.as_str()]), + "expected read-only remount for {path}: {args:#?}" + ); + } + + fn synthetic_mount_target_paths(args: &BwrapArgs) -> Vec { + args.synthetic_mount_targets + .iter() + .map(|target| target.path().to_path_buf()) + .collect() + } + + fn protected_create_target_paths(args: &BwrapArgs) -> Vec { + args.protected_create_targets + .iter() + .map(|target| target.path().to_path_buf()) + .collect() + } } diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 21fbbe8261af..c88a28b3245a 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -1,11 +1,21 @@ use clap::Parser; use std::ffi::CString; use std::fmt; +use std::fs; use std::fs::File; +use std::fs::OpenOptions; use std::io::Read; +use std::os::fd::AsRawFd; use std::os::fd::FromRawFd; +use std::os::unix::ffi::OsStrExt; use std::path::Path; use std::path::PathBuf; +use std::sync::Arc; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::AtomicI32; +use std::sync::atomic::Ordering; +use std::thread; +use std::time::Duration; use crate::bwrap::BwrapNetworkMode; use crate::bwrap::BwrapOptions; @@ -20,6 +30,46 @@ use codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::NetworkSandboxPolicy; use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; +static BWRAP_CHILD_PID: AtomicI32 = AtomicI32::new(0); +static PENDING_FORWARDED_SIGNAL: AtomicI32 = AtomicI32::new(0); + +const FORWARDED_SIGNALS: &[libc::c_int] = + &[libc::SIGHUP, libc::SIGINT, libc::SIGQUIT, libc::SIGTERM]; +const SYNTHETIC_MOUNT_MARKER_SYNTHETIC: &[u8] = b"synthetic\n"; +const SYNTHETIC_MOUNT_MARKER_EXISTING: &[u8] = b"existing\n"; +const PROTECTED_CREATE_MARKER: &[u8] = b"protected-create\n"; + +#[derive(Debug)] +struct SyntheticMountTargetRegistration { + target: crate::bwrap::SyntheticMountTarget, + marker_file: PathBuf, + marker_dir: PathBuf, +} + +#[derive(Debug)] +struct ProtectedCreateTargetRegistration { + target: crate::bwrap::ProtectedCreateTarget, + marker_file: PathBuf, + marker_dir: PathBuf, +} + +struct ProtectedCreateMonitor { + stop: Arc, + violation: Arc, + handle: thread::JoinHandle<()>, +} + +struct ProtectedCreateWatcher { + fd: libc::c_int, + _watches: Vec, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum ProtectedCreateRemoval { + Directory, + Other, +} + #[derive(Debug, Parser)] /// CLI surface for the Linux sandbox helper. /// @@ -302,7 +352,7 @@ fn run_bwrap_with_proc_fallback( options, ); apply_inner_command_argv0(&mut bwrap_args.args); - exec_bwrap(bwrap_args.args, bwrap_args.preserved_files); + run_or_exec_bwrap(bwrap_args); } fn bwrap_network_mode( @@ -339,6 +389,8 @@ fn build_bwrap_argv( crate::bwrap::BwrapArgs { args: argv, preserved_files: bwrap_args.preserved_files, + synthetic_mount_targets: bwrap_args.synthetic_mount_targets, + protected_create_targets: bwrap_args.protected_create_targets, } } @@ -427,6 +479,802 @@ fn resolve_true_command() -> String { "true".to_string() } +fn run_or_exec_bwrap(bwrap_args: crate::bwrap::BwrapArgs) -> ! { + if bwrap_args.synthetic_mount_targets.is_empty() + && bwrap_args.protected_create_targets.is_empty() + { + exec_bwrap(bwrap_args.args, bwrap_args.preserved_files); + } + run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args); +} + +fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::BwrapArgs) -> ! { + let crate::bwrap::BwrapArgs { + args, + preserved_files, + synthetic_mount_targets, + protected_create_targets, + } = bwrap_args; + let setup_signal_mask = ForwardedSignalMask::block(); + let synthetic_mount_registrations = register_synthetic_mount_targets(&synthetic_mount_targets); + let protected_create_registrations = + register_protected_create_targets(&protected_create_targets); + let exec_start_pipe = create_exec_start_pipe(!protected_create_targets.is_empty()); + let parent_pid = unsafe { libc::getpid() }; + let pid = unsafe { libc::fork() }; + if pid < 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to fork for bubblewrap: {err}"); + } + + if pid == 0 { + reset_forwarded_signal_handlers_to_default(); + setup_signal_mask.restore(); + let setpgid_res = unsafe { libc::setpgid(0, 0) }; + if setpgid_res < 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to place bubblewrap child in its own process group: {err}"); + } + terminate_with_parent(parent_pid); + wait_for_parent_exec_start(exec_start_pipe[0], exec_start_pipe[1]); + exec_bwrap(args, preserved_files); + } + + close_child_exec_start_read(exec_start_pipe[0]); + let protected_create_monitor = ProtectedCreateMonitor::start(&protected_create_targets); + let signal_forwarders = install_bwrap_signal_forwarders(pid); + release_child_exec_start(exec_start_pipe[1]); + setup_signal_mask.restore(); + let status = wait_for_bwrap_child(pid); + let cleanup_signal_mask = ForwardedSignalMask::block(); + BWRAP_CHILD_PID.store(0, Ordering::SeqCst); + let protected_create_monitor_violation = protected_create_monitor + .map(ProtectedCreateMonitor::stop) + .unwrap_or(false); + cleanup_synthetic_mount_targets(&synthetic_mount_registrations); + let protected_create_violation = protected_create_monitor_violation + || cleanup_protected_create_targets(&protected_create_registrations); + signal_forwarders.restore(); + cleanup_signal_mask.restore(); + exit_with_wait_status_or_policy_violation(status, protected_create_violation); +} + +impl ProtectedCreateMonitor { + fn start(targets: &[crate::bwrap::ProtectedCreateTarget]) -> Option { + if targets.is_empty() { + return None; + } + + let targets = targets.to_vec(); + let stop = Arc::new(AtomicBool::new(false)); + let violation = Arc::new(AtomicBool::new(false)); + let monitor_stop = Arc::clone(&stop); + let monitor_violation = Arc::clone(&violation); + let handle = thread::spawn(move || { + let watcher = ProtectedCreateWatcher::new(&targets); + while !monitor_stop.load(Ordering::SeqCst) { + for target in &targets { + if remove_protected_create_target_best_effort(target).is_some() { + monitor_violation.store(true, Ordering::SeqCst); + } + } + if let Some(watcher) = &watcher { + watcher.wait_for_create_event(&monitor_stop); + } else { + thread::sleep(Duration::from_millis(1)); + } + } + }); + + Some(Self { + stop, + violation, + handle, + }) + } + + fn stop(self) -> bool { + self.stop.store(true, Ordering::SeqCst); + self.handle + .join() + .unwrap_or_else(|_| panic!("protected create monitor thread panicked")); + self.violation.load(Ordering::SeqCst) + } +} + +impl ProtectedCreateWatcher { + fn new(targets: &[crate::bwrap::ProtectedCreateTarget]) -> Option { + let fd = unsafe { libc::inotify_init1(libc::IN_NONBLOCK | libc::IN_CLOEXEC) }; + if fd < 0 { + return None; + } + + let mut watched_parents = Vec::::new(); + let mut watches = Vec::new(); + for target in targets { + let Some(parent) = target.path().parent() else { + continue; + }; + if watched_parents.iter().any(|watched| watched == parent) { + continue; + } + watched_parents.push(parent.to_path_buf()); + let Ok(parent_cstr) = CString::new(parent.as_os_str().as_bytes()) else { + continue; + }; + let mask = + libc::IN_CREATE | libc::IN_MOVED_TO | libc::IN_DELETE_SELF | libc::IN_MOVE_SELF; + let watch = unsafe { libc::inotify_add_watch(fd, parent_cstr.as_ptr(), mask) }; + if watch >= 0 { + watches.push(watch); + } + } + + if watches.is_empty() { + unsafe { + libc::close(fd); + } + return None; + } + + Some(Self { + fd, + _watches: watches, + }) + } + + fn wait_for_create_event(&self, stop: &AtomicBool) { + let mut poll_fd = libc::pollfd { + fd: self.fd, + events: libc::POLLIN, + revents: 0, + }; + while !stop.load(Ordering::SeqCst) { + let res = unsafe { libc::poll(&mut poll_fd, 1, 10) }; + if res > 0 { + self.drain_events(); + return; + } + if res == 0 { + return; + } + let err = std::io::Error::last_os_error(); + if err.kind() == std::io::ErrorKind::Interrupted { + continue; + } + return; + } + } + + fn drain_events(&self) { + let mut buf = [0_u8; 4096]; + loop { + let read = unsafe { libc::read(self.fd, buf.as_mut_ptr().cast(), buf.len()) }; + if read > 0 { + continue; + } + if read == 0 { + return; + } + let err = std::io::Error::last_os_error(); + if err.kind() == std::io::ErrorKind::Interrupted { + continue; + } + return; + } + } +} + +impl Drop for ProtectedCreateWatcher { + fn drop(&mut self) { + unsafe { + libc::close(self.fd); + } + } +} + +fn create_exec_start_pipe(enabled: bool) -> [libc::c_int; 2] { + if !enabled { + return [-1, -1]; + } + let mut pipe = [-1, -1]; + if unsafe { libc::pipe2(pipe.as_mut_ptr(), libc::O_CLOEXEC) } < 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to create bubblewrap exec start pipe: {err}"); + } + pipe +} + +fn wait_for_parent_exec_start(read_fd: libc::c_int, write_fd: libc::c_int) { + if write_fd >= 0 { + unsafe { + libc::close(write_fd); + } + } + if read_fd < 0 { + return; + } + + let mut byte = [0_u8; 1]; + loop { + let read = unsafe { libc::read(read_fd, byte.as_mut_ptr().cast(), byte.len()) }; + if read >= 0 { + break; + } + let err = std::io::Error::last_os_error(); + if err.kind() != std::io::ErrorKind::Interrupted { + break; + } + } + unsafe { + libc::close(read_fd); + } +} + +fn close_child_exec_start_read(read_fd: libc::c_int) { + if read_fd >= 0 { + unsafe { + libc::close(read_fd); + } + } +} + +fn release_child_exec_start(write_fd: libc::c_int) { + if write_fd < 0 { + return; + } + let byte = [0_u8; 1]; + unsafe { + libc::write(write_fd, byte.as_ptr().cast(), byte.len()); + libc::close(write_fd); + } +} + +struct ForwardedSignalMask { + previous: libc::sigset_t, +} + +struct ForwardedSignalHandlers { + previous: Vec<(libc::c_int, libc::sigaction)>, +} + +impl ForwardedSignalMask { + fn block() -> Self { + let mut blocked: libc::sigset_t = unsafe { std::mem::zeroed() }; + let mut previous: libc::sigset_t = unsafe { std::mem::zeroed() }; + unsafe { + libc::sigemptyset(&mut blocked); + for signal in FORWARDED_SIGNALS { + libc::sigaddset(&mut blocked, *signal); + } + if libc::sigprocmask(libc::SIG_BLOCK, &blocked, &mut previous) < 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to block bubblewrap forwarded signals: {err}"); + } + } + Self { previous } + } + + fn restore(&self) { + let mut restored = self.previous; + unsafe { + for signal in FORWARDED_SIGNALS { + libc::sigdelset(&mut restored, *signal); + } + if libc::sigprocmask(libc::SIG_SETMASK, &restored, std::ptr::null_mut()) < 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to restore bubblewrap forwarded signals: {err}"); + } + } + } +} + +fn terminate_with_parent(parent_pid: libc::pid_t) { + let res = unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) }; + if res < 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to set bubblewrap child parent-death signal: {err}"); + } + if unsafe { libc::getppid() } != parent_pid { + unsafe { + libc::raise(libc::SIGTERM); + } + } +} + +impl ForwardedSignalHandlers { + fn restore(self) { + BWRAP_CHILD_PID.store(0, Ordering::SeqCst); + PENDING_FORWARDED_SIGNAL.store(0, Ordering::SeqCst); + for (signal, previous_action) in self.previous { + unsafe { + if libc::sigaction(signal, &previous_action, std::ptr::null_mut()) < 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to restore bubblewrap signal handler for {signal}: {err}"); + } + } + } + } +} + +fn install_bwrap_signal_forwarders(pid: libc::pid_t) -> ForwardedSignalHandlers { + BWRAP_CHILD_PID.store(pid, Ordering::SeqCst); + let mut previous = Vec::with_capacity(FORWARDED_SIGNALS.len()); + for signal in FORWARDED_SIGNALS { + let mut action: libc::sigaction = unsafe { std::mem::zeroed() }; + let mut previous_action: libc::sigaction = unsafe { std::mem::zeroed() }; + action.sa_sigaction = forward_signal_to_bwrap_child as *const () as libc::sighandler_t; + unsafe { + libc::sigemptyset(&mut action.sa_mask); + if libc::sigaction(*signal, &action, &mut previous_action) < 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to install bubblewrap signal forwarder for {signal}: {err}"); + } + } + previous.push((*signal, previous_action)); + } + replay_pending_forwarded_signal(pid); + ForwardedSignalHandlers { previous } +} + +extern "C" fn forward_signal_to_bwrap_child(signal: libc::c_int) { + PENDING_FORWARDED_SIGNAL.store(signal, Ordering::SeqCst); + let pid = BWRAP_CHILD_PID.load(Ordering::SeqCst); + if pid > 0 { + send_signal_to_bwrap_child(pid, signal); + } +} + +fn replay_pending_forwarded_signal(pid: libc::pid_t) { + let signal = PENDING_FORWARDED_SIGNAL.swap(0, Ordering::SeqCst); + if signal > 0 { + send_signal_to_bwrap_child(pid, signal); + } +} + +fn send_signal_to_bwrap_child(pid: libc::pid_t, signal: libc::c_int) { + unsafe { + libc::kill(-pid, signal); + libc::kill(pid, signal); + } +} + +fn reset_forwarded_signal_handlers_to_default() { + for signal in FORWARDED_SIGNALS { + unsafe { + if libc::signal(*signal, libc::SIG_DFL) == libc::SIG_ERR { + let err = std::io::Error::last_os_error(); + panic!("failed to reset bubblewrap signal handler for {signal}: {err}"); + } + } + } +} + +fn wait_for_bwrap_child(pid: libc::pid_t) -> libc::c_int { + loop { + let mut status: libc::c_int = 0; + let wait_res = unsafe { libc::waitpid(pid, &mut status as *mut libc::c_int, 0) }; + if wait_res >= 0 { + return status; + } + let err = std::io::Error::last_os_error(); + if err.raw_os_error() == Some(libc::EINTR) { + continue; + } + panic!("waitpid failed for bubblewrap child: {err}"); + } +} + +fn register_synthetic_mount_targets( + targets: &[crate::bwrap::SyntheticMountTarget], +) -> Vec { + with_synthetic_mount_registry_lock(|| { + targets + .iter() + .map(|target| { + let marker_dir = synthetic_mount_marker_dir(target.path()); + fs::create_dir_all(&marker_dir).unwrap_or_else(|err| { + panic!( + "failed to create synthetic bubblewrap mount marker directory {}: {err}", + marker_dir.display() + ) + }); + let target = if target.preserves_pre_existing_path() + && synthetic_mount_marker_dir_has_active_synthetic_owner(&marker_dir) + { + match target.kind() { + crate::bwrap::SyntheticMountTargetKind::EmptyFile => { + crate::bwrap::SyntheticMountTarget::missing(target.path()) + } + crate::bwrap::SyntheticMountTargetKind::EmptyDirectory => { + crate::bwrap::SyntheticMountTarget::missing_empty_directory( + target.path(), + ) + } + } + } else { + target.clone() + }; + let marker_file = marker_dir.join(std::process::id().to_string()); + fs::write(&marker_file, synthetic_mount_marker_contents(&target)).unwrap_or_else( + |err| { + panic!( + "failed to register synthetic bubblewrap mount target {}: {err}", + target.path().display() + ) + }, + ); + SyntheticMountTargetRegistration { + target, + marker_file, + marker_dir, + } + }) + .collect() + }) +} + +fn register_protected_create_targets( + targets: &[crate::bwrap::ProtectedCreateTarget], +) -> Vec { + with_synthetic_mount_registry_lock(|| { + targets + .iter() + .map(|target| { + let marker_dir = synthetic_mount_marker_dir(target.path()); + fs::create_dir_all(&marker_dir).unwrap_or_else(|err| { + panic!( + "failed to create protected create marker directory {}: {err}", + marker_dir.display() + ) + }); + let marker_file = marker_dir.join(std::process::id().to_string()); + fs::write(&marker_file, PROTECTED_CREATE_MARKER).unwrap_or_else(|err| { + panic!( + "failed to register protected create target {}: {err}", + target.path().display() + ) + }); + ProtectedCreateTargetRegistration { + target: target.clone(), + marker_file, + marker_dir, + } + }) + .collect() + }) +} + +fn synthetic_mount_marker_contents(target: &crate::bwrap::SyntheticMountTarget) -> &'static [u8] { + if target.preserves_pre_existing_path() { + SYNTHETIC_MOUNT_MARKER_EXISTING + } else { + SYNTHETIC_MOUNT_MARKER_SYNTHETIC + } +} + +fn synthetic_mount_marker_dir_has_active_synthetic_owner(marker_dir: &Path) -> bool { + synthetic_mount_marker_dir_has_active_process_matching(marker_dir, |path| { + match fs::read(path) { + Ok(contents) => contents == SYNTHETIC_MOUNT_MARKER_SYNTHETIC, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => false, + Err(err) => panic!( + "failed to read synthetic bubblewrap mount marker {}: {err}", + path.display() + ), + } + }) +} + +fn synthetic_mount_marker_dir_has_active_process(marker_dir: &Path) -> bool { + synthetic_mount_marker_dir_has_active_process_matching(marker_dir, |_| true) +} + +fn synthetic_mount_marker_dir_has_active_process_matching( + marker_dir: &Path, + matches_marker: impl Fn(&Path) -> bool, +) -> bool { + let entries = match fs::read_dir(marker_dir) { + Ok(entries) => entries, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return false, + Err(err) => panic!( + "failed to read synthetic bubblewrap mount marker directory {}: {err}", + marker_dir.display() + ), + }; + for entry in entries { + let entry = entry.unwrap_or_else(|err| { + panic!( + "failed to read synthetic bubblewrap mount marker in {}: {err}", + marker_dir.display() + ) + }); + let path = entry.path(); + let Some(pid) = path + .file_name() + .and_then(|name| name.to_str()) + .and_then(|name| name.parse::().ok()) + else { + continue; + }; + if !process_is_active(pid) { + match fs::remove_file(&path) { + Ok(()) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) => panic!( + "failed to remove stale synthetic bubblewrap mount marker {}: {err}", + path.display() + ), + } + continue; + } + let matches_marker = matches_marker(&path); + if matches_marker { + return true; + } + } + false +} + +fn cleanup_synthetic_mount_targets(targets: &[SyntheticMountTargetRegistration]) { + with_synthetic_mount_registry_lock(|| { + for target in targets.iter().rev() { + match fs::remove_file(&target.marker_file) { + Ok(()) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) => panic!( + "failed to unregister synthetic bubblewrap mount target {}: {err}", + target.target.path().display() + ), + } + } + + for target in targets.iter().rev() { + if synthetic_mount_marker_dir_has_active_process(&target.marker_dir) { + continue; + } + remove_synthetic_mount_target(&target.target); + match fs::remove_dir(&target.marker_dir) { + Ok(()) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) if err.kind() == std::io::ErrorKind::DirectoryNotEmpty => {} + Err(err) => panic!( + "failed to remove synthetic bubblewrap mount marker directory {}: {err}", + target.marker_dir.display() + ), + } + } + }); +} + +fn cleanup_protected_create_targets(targets: &[ProtectedCreateTargetRegistration]) -> bool { + with_synthetic_mount_registry_lock(|| { + for target in targets.iter().rev() { + match fs::remove_file(&target.marker_file) { + Ok(()) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) => panic!( + "failed to unregister protected create target {}: {err}", + target.target.path().display() + ), + } + } + + let mut violation = false; + for target in targets.iter().rev() { + if synthetic_mount_marker_dir_has_active_process(&target.marker_dir) { + if target.target.path().exists() { + violation = true; + } + continue; + } + violation |= remove_protected_create_target(&target.target); + match fs::remove_dir(&target.marker_dir) { + Ok(()) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) if err.kind() == std::io::ErrorKind::DirectoryNotEmpty => {} + Err(err) => panic!( + "failed to remove protected create marker directory {}: {err}", + target.marker_dir.display() + ), + } + } + violation + }) +} + +fn remove_protected_create_target(target: &crate::bwrap::ProtectedCreateTarget) -> bool { + for attempt in 0..100 { + match try_remove_protected_create_target(target) { + Ok(removal) => return removal.is_some(), + Err(err) if err.kind() == std::io::ErrorKind::DirectoryNotEmpty && attempt < 99 => { + thread::sleep(Duration::from_millis(1)); + } + Err(err) => { + panic!( + "failed to remove protected create target {}: {err}", + target.path().display() + ); + } + } + } + unreachable!("protected create removal retry loop should return or panic") +} + +fn remove_protected_create_target_best_effort( + target: &crate::bwrap::ProtectedCreateTarget, +) -> Option { + for _ in 0..100 { + match try_remove_protected_create_target(target) { + Ok(removal) => return removal, + Err(err) if err.kind() == std::io::ErrorKind::DirectoryNotEmpty => { + thread::sleep(Duration::from_millis(1)); + } + Err(_) => return Some(ProtectedCreateRemoval::Other), + } + } + Some(ProtectedCreateRemoval::Other) +} + +fn try_remove_protected_create_target( + target: &crate::bwrap::ProtectedCreateTarget, +) -> std::io::Result> { + let path = target.path(); + let metadata = match fs::symlink_metadata(path) { + Ok(metadata) => metadata, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(err) => return Err(err), + }; + + let removal = if metadata.is_dir() { + ProtectedCreateRemoval::Directory + } else { + ProtectedCreateRemoval::Other + }; + let result = if removal == ProtectedCreateRemoval::Directory { + fs::remove_dir_all(path) + } else { + fs::remove_file(path) + }; + match result { + Ok(()) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(err) => return Err(err), + } + eprintln!( + "sandbox blocked creation of protected workspace metadata path {}", + path.display() + ); + Ok(Some(removal)) +} + +fn remove_synthetic_mount_target(target: &crate::bwrap::SyntheticMountTarget) { + let path = target.path(); + let metadata = match fs::symlink_metadata(path) { + Ok(metadata) => metadata, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return, + Err(err) => panic!( + "failed to inspect synthetic bubblewrap mount target {}: {err}", + path.display() + ), + }; + if !target.should_remove_after_bwrap(&metadata) { + return; + } + match target.kind() { + crate::bwrap::SyntheticMountTargetKind::EmptyFile => match fs::remove_file(path) { + Ok(()) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) => panic!( + "failed to remove synthetic bubblewrap mount target {}: {err}", + path.display() + ), + }, + crate::bwrap::SyntheticMountTargetKind::EmptyDirectory => match fs::remove_dir(path) { + Ok(()) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) if err.kind() == std::io::ErrorKind::DirectoryNotEmpty => {} + Err(err) => panic!( + "failed to remove synthetic bubblewrap mount target {}: {err}", + path.display() + ), + }, + } +} + +fn process_is_active(pid: libc::pid_t) -> bool { + let result = unsafe { libc::kill(pid, 0) }; + if result == 0 { + return true; + } + let err = std::io::Error::last_os_error(); + !matches!(err.raw_os_error(), Some(libc::ESRCH)) +} + +fn with_synthetic_mount_registry_lock(f: impl FnOnce() -> T) -> T { + let registry_root = synthetic_mount_registry_root(); + fs::create_dir_all(®istry_root).unwrap_or_else(|err| { + panic!( + "failed to create synthetic bubblewrap mount registry {}: {err}", + registry_root.display() + ) + }); + let lock_path = registry_root.join("lock"); + let lock_file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + .unwrap_or_else(|err| { + panic!( + "failed to open synthetic bubblewrap mount registry lock {}: {err}", + lock_path.display() + ) + }); + if unsafe { libc::flock(lock_file.as_raw_fd(), libc::LOCK_EX) } < 0 { + let err = std::io::Error::last_os_error(); + panic!( + "failed to lock synthetic bubblewrap mount registry {}: {err}", + lock_path.display() + ); + } + let result = f(); + if unsafe { libc::flock(lock_file.as_raw_fd(), libc::LOCK_UN) } < 0 { + let err = std::io::Error::last_os_error(); + panic!( + "failed to unlock synthetic bubblewrap mount registry {}: {err}", + lock_path.display() + ); + } + result +} + +fn synthetic_mount_marker_dir(path: &Path) -> PathBuf { + synthetic_mount_registry_root().join(format!("{:016x}", hash_path(path))) +} + +fn synthetic_mount_registry_root() -> PathBuf { + std::env::temp_dir().join("codex-bwrap-synthetic-mount-targets") +} + +fn hash_path(path: &Path) -> u64 { + let mut hash = 0xcbf29ce484222325u64; + for byte in path.as_os_str().as_bytes() { + hash ^= u64::from(*byte); + hash = hash.wrapping_mul(0x100000001b3); + } + hash +} + +fn exit_with_wait_status(status: libc::c_int) -> ! { + if libc::WIFEXITED(status) { + std::process::exit(libc::WEXITSTATUS(status)); + } + + if libc::WIFSIGNALED(status) { + let signal = libc::WTERMSIG(status); + unsafe { + libc::signal(signal, libc::SIG_DFL); + libc::kill(libc::getpid(), signal); + } + std::process::exit(128 + signal); + } + + std::process::exit(1); +} + +fn exit_with_wait_status_or_policy_violation( + status: libc::c_int, + protected_create_violation: bool, +) -> ! { + if protected_create_violation && libc::WIFEXITED(status) && libc::WEXITSTATUS(status) == 0 { + std::process::exit(1); + } + + exit_with_wait_status(status); +} + /// Run a short-lived bubblewrap preflight in a child process and capture stderr. /// /// Strategy: @@ -440,6 +1288,16 @@ fn resolve_true_command() -> String { /// command, and reads are bounded to a fixed max size. fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> String { const MAX_PREFLIGHT_STDERR_BYTES: u64 = 64 * 1024; + let crate::bwrap::BwrapArgs { + args, + preserved_files, + synthetic_mount_targets, + protected_create_targets, + } = bwrap_args; + let setup_signal_mask = ForwardedSignalMask::block(); + let synthetic_mount_registrations = register_synthetic_mount_targets(&synthetic_mount_targets); + let protected_create_registrations = + register_protected_create_targets(&protected_create_targets); let mut pipe_fds = [0; 2]; let pipe_res = unsafe { libc::pipe2(pipe_fds.as_mut_ptr(), libc::O_CLOEXEC) }; @@ -457,6 +1315,8 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str } if pid == 0 { + reset_forwarded_signal_handlers_to_default(); + setup_signal_mask.restore(); // Child: redirect stderr to the pipe, then run bubblewrap. unsafe { close_fd_or_panic(read_fd, "close read end in bubblewrap child"); @@ -467,9 +1327,11 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str close_fd_or_panic(write_fd, "close write end in bubblewrap child"); } - exec_bwrap(bwrap_args.args, bwrap_args.preserved_files); + exec_bwrap(args, preserved_files); } + let signal_forwarders = install_bwrap_signal_forwarders(pid); + setup_signal_mask.restore(); // Parent: close the write end and read stderr while the child runs. close_fd_or_panic(write_fd, "close write end in bubblewrap parent"); @@ -481,11 +1343,15 @@ fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> Str panic!("failed to read bubblewrap stderr: {err}"); } - let mut status: libc::c_int = 0; - let wait_res = unsafe { libc::waitpid(pid, &mut status as *mut libc::c_int, 0) }; - if wait_res < 0 { - let err = std::io::Error::last_os_error(); - panic!("waitpid failed for bubblewrap child: {err}"); + let status = wait_for_bwrap_child(pid); + let cleanup_signal_mask = ForwardedSignalMask::block(); + BWRAP_CHILD_PID.store(0, Ordering::SeqCst); + cleanup_synthetic_mount_targets(&synthetic_mount_registrations); + cleanup_protected_create_targets(&protected_create_registrations); + signal_forwarders.restore(); + cleanup_signal_mask.restore(); + if libc::WIFSIGNALED(status) { + exit_with_wait_status(status); } String::from_utf8_lossy(&stderr_bytes).into_owned() diff --git a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs index 2b3f963557ff..228cea6e5da4 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -1,6 +1,10 @@ #[cfg(test)] use super::*; #[cfg(test)] +use crate::linux_run_main::install_bwrap_signal_forwarders; +#[cfg(test)] +use crate::linux_run_main::wait_for_bwrap_child; +#[cfg(test)] use codex_protocol::models::PermissionProfile; #[cfg(test)] use codex_protocol::protocol::FileSystemSandboxPolicy; @@ -265,6 +269,180 @@ fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() { assert!(argv.iter().any(|arg| arg == "--")); } +#[test] +fn cleanup_synthetic_mount_targets_removes_only_empty_mount_targets() { + let temp_dir = tempfile::TempDir::new().expect("tempdir"); + let empty_file = temp_dir.path().join(".git"); + let empty_dir = temp_dir.path().join(".agents"); + let non_empty_file = temp_dir.path().join("non-empty"); + let missing_file = temp_dir.path().join(".missing"); + std::fs::write(&empty_file, "").expect("write empty file"); + std::fs::create_dir(&empty_dir).expect("create empty dir"); + std::fs::write(&non_empty_file, "keep").expect("write nonempty file"); + + let registrations = register_synthetic_mount_targets(&[ + crate::bwrap::SyntheticMountTarget::missing(&empty_file), + crate::bwrap::SyntheticMountTarget::missing_empty_directory(&empty_dir), + crate::bwrap::SyntheticMountTarget::missing(&non_empty_file), + crate::bwrap::SyntheticMountTarget::missing(&missing_file), + ]); + cleanup_synthetic_mount_targets(®istrations); + + assert!(!empty_file.exists()); + assert!(!empty_dir.exists()); + assert_eq!( + std::fs::read_to_string(&non_empty_file).expect("read nonempty file"), + "keep" + ); + assert!(!missing_file.exists()); +} + +#[test] +fn cleanup_synthetic_mount_targets_waits_for_other_active_registrations() { + let temp_dir = tempfile::TempDir::new().expect("tempdir"); + let empty_file = temp_dir.path().join(".git"); + std::fs::write(&empty_file, "").expect("write empty file"); + let target = crate::bwrap::SyntheticMountTarget::missing(&empty_file); + + let registrations = register_synthetic_mount_targets(std::slice::from_ref(&target)); + let active_marker = registrations[0].marker_dir.join("1"); + std::fs::write(&active_marker, "").expect("write active marker"); + + cleanup_synthetic_mount_targets(®istrations); + assert!(empty_file.exists()); + + std::fs::remove_file(active_marker).expect("remove active marker"); + let registrations = register_synthetic_mount_targets(std::slice::from_ref(&target)); + cleanup_synthetic_mount_targets(®istrations); + + assert!(!empty_file.exists()); +} + +#[test] +fn cleanup_synthetic_mount_targets_removes_transient_file_after_concurrent_owner_exits() { + let temp_dir = tempfile::TempDir::new().expect("tempdir"); + let empty_file = temp_dir.path().join(".git"); + let first_target = crate::bwrap::SyntheticMountTarget::missing(&empty_file); + + let first_registrations = register_synthetic_mount_targets(&[first_target]); + std::fs::write(&empty_file, "").expect("write transient empty file"); + let active_marker = first_registrations[0].marker_dir.join("1"); + std::fs::write(&active_marker, SYNTHETIC_MOUNT_MARKER_SYNTHETIC).expect("write active marker"); + let metadata = std::fs::symlink_metadata(&empty_file).expect("stat empty file"); + let second_target = + crate::bwrap::SyntheticMountTarget::existing_empty_file(&empty_file, &metadata); + let second_registrations = register_synthetic_mount_targets(&[second_target]); + + cleanup_synthetic_mount_targets(&first_registrations); + assert!(empty_file.exists()); + + std::fs::remove_file(active_marker).expect("remove active marker"); + cleanup_synthetic_mount_targets(&second_registrations); + + assert!(!empty_file.exists()); +} + +#[test] +fn cleanup_synthetic_mount_targets_preserves_real_pre_existing_empty_file() { + let temp_dir = tempfile::TempDir::new().expect("tempdir"); + let empty_file = temp_dir.path().join(".git"); + std::fs::write(&empty_file, "").expect("write pre-existing empty file"); + let metadata = std::fs::symlink_metadata(&empty_file).expect("stat empty file"); + let first_target = + crate::bwrap::SyntheticMountTarget::existing_empty_file(&empty_file, &metadata); + let second_target = + crate::bwrap::SyntheticMountTarget::existing_empty_file(&empty_file, &metadata); + + let first_registrations = register_synthetic_mount_targets(&[first_target]); + let second_registrations = register_synthetic_mount_targets(&[second_target]); + + cleanup_synthetic_mount_targets(&first_registrations); + cleanup_synthetic_mount_targets(&second_registrations); + + assert!(empty_file.exists()); +} + +#[test] +fn cleanup_protected_create_targets_removes_created_path_and_reports_violation() { + let temp_dir = tempfile::TempDir::new().expect("tempdir"); + let dot_git = temp_dir.path().join(".git"); + let target = crate::bwrap::ProtectedCreateTarget::missing(&dot_git); + + let registrations = register_protected_create_targets(&[target]); + std::fs::create_dir(&dot_git).expect("create protected path"); + let violation = cleanup_protected_create_targets(®istrations); + + assert!(violation); + assert!(!dot_git.exists()); +} + +#[test] +fn cleanup_protected_create_targets_waits_for_other_active_registrations() { + let temp_dir = tempfile::TempDir::new().expect("tempdir"); + let dot_git = temp_dir.path().join(".git"); + let target = crate::bwrap::ProtectedCreateTarget::missing(&dot_git); + + let registrations = register_protected_create_targets(std::slice::from_ref(&target)); + let active_marker = registrations[0].marker_dir.join("1"); + std::fs::write(&active_marker, PROTECTED_CREATE_MARKER).expect("write active marker"); + std::fs::write(&dot_git, "").expect("create protected path"); + + let violation = cleanup_protected_create_targets(®istrations); + assert!(violation); + assert!(dot_git.exists()); + + std::fs::remove_file(active_marker).expect("remove active marker"); + let registrations = register_protected_create_targets(std::slice::from_ref(&target)); + let violation = cleanup_protected_create_targets(®istrations); + + assert!(violation); + assert!(!dot_git.exists()); +} + +#[test] +fn bwrap_signal_forwarder_terminates_child_and_keeps_parent_alive() { + let supervisor_pid = unsafe { libc::fork() }; + assert!(supervisor_pid >= 0, "failed to fork supervisor"); + + if supervisor_pid == 0 { + run_bwrap_signal_forwarder_test_supervisor(); + } + + let status = wait_for_bwrap_child(supervisor_pid); + assert!(libc::WIFEXITED(status), "supervisor status: {status}"); + assert_eq!(libc::WEXITSTATUS(status), 0); +} + +#[cfg(test)] +fn run_bwrap_signal_forwarder_test_supervisor() -> ! { + let child_pid = unsafe { libc::fork() }; + if child_pid < 0 { + unsafe { + libc::_exit(2); + } + } + + if child_pid == 0 { + loop { + unsafe { + libc::pause(); + } + } + } + + install_bwrap_signal_forwarders(child_pid); + unsafe { + libc::raise(libc::SIGTERM); + } + + let status = wait_for_bwrap_child(child_pid); + let child_terminated_by_sigterm = + libc::WIFSIGNALED(status) && libc::WTERMSIG(status) == libc::SIGTERM; + unsafe { + libc::_exit(if child_terminated_by_sigterm { 0 } else { 1 }); + } +} + #[test] fn managed_proxy_inner_command_includes_route_spec() { let permission_profile = read_only_permission_profile(); diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index 3b97e933bb26..efbcd0b4868a 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -26,17 +26,17 @@ use tempfile::NamedTempFile; // At least on GitHub CI, the arm64 tests appear to need longer timeouts. #[cfg(not(target_arch = "aarch64"))] -const SHORT_TIMEOUT_MS: u64 = 200; +const SHORT_TIMEOUT_MS: u64 = 5_000; #[cfg(target_arch = "aarch64")] const SHORT_TIMEOUT_MS: u64 = 5_000; #[cfg(not(target_arch = "aarch64"))] -const LONG_TIMEOUT_MS: u64 = 1_000; +const LONG_TIMEOUT_MS: u64 = 5_000; #[cfg(target_arch = "aarch64")] const LONG_TIMEOUT_MS: u64 = 5_000; #[cfg(not(target_arch = "aarch64"))] -const NETWORK_TIMEOUT_MS: u64 = 2_000; +const NETWORK_TIMEOUT_MS: u64 = 10_000; #[cfg(target_arch = "aarch64")] const NETWORK_TIMEOUT_MS: u64 = 10_000; @@ -47,6 +47,14 @@ fn create_env_from_core_vars() -> HashMap { create_env(&policy, /*thread_id*/ None) } +fn codex_linux_sandbox_exe() -> PathBuf { + let sandbox_program = PathBuf::from(env!("CARGO_BIN_EXE_codex-linux-sandbox")); + match sandbox_program.canonicalize() { + Ok(path) => path, + Err(_) => sandbox_program, + } +} + #[expect(clippy::print_stdout)] async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) { let output = run_cmd_output(cmd, writable_roots, timeout_ms).await; @@ -111,6 +119,57 @@ async fn run_cmd_result_with_permission_profile( use_legacy_landlock: bool, ) -> Result { let cwd = AbsolutePathBuf::current_dir().expect("cwd should exist"); + run_cmd_result_with_permission_profile_for_cwd( + cmd, + cwd, + permission_profile, + timeout_ms, + use_legacy_landlock, + ) + .await +} + +#[expect(clippy::expect_used)] +async fn run_cmd_result_with_cwd_and_writable_roots( + cmd: &[&str], + cwd: &std::path::Path, + writable_roots: &[PathBuf], + timeout_ms: u64, + use_legacy_landlock: bool, + network_access: bool, +) -> Result { + let writable_roots = writable_roots + .iter() + .map(|path| AbsolutePathBuf::try_from(path.as_path()).unwrap()) + .collect::>(); + let permission_profile = PermissionProfile::workspace_write_with( + &writable_roots, + if network_access { + NetworkSandboxPolicy::Enabled + } else { + NetworkSandboxPolicy::Restricted + }, + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ); + let cwd = AbsolutePathBuf::try_from(cwd).expect("cwd should be absolute"); + run_cmd_result_with_permission_profile_for_cwd( + cmd, + cwd, + permission_profile, + timeout_ms, + use_legacy_landlock, + ) + .await +} + +async fn run_cmd_result_with_permission_profile_for_cwd( + cmd: &[&str], + cwd: AbsolutePathBuf, + permission_profile: PermissionProfile, + timeout_ms: u64, + use_legacy_landlock: bool, +) -> Result { let sandbox_cwd = cwd.clone(); let params = ExecParams { command: cmd.iter().copied().map(str::to_owned).collect(), @@ -125,8 +184,7 @@ async fn run_cmd_result_with_permission_profile( justification: None, arg0: None, }; - let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox"); - let codex_linux_sandbox_exe = Some(PathBuf::from(sandbox_program)); + let codex_linux_sandbox_exe = Some(codex_linux_sandbox_exe()); process_exec_tool_call( params, @@ -384,8 +442,7 @@ async fn assert_network_blocked(cmd: &[&str]) { arg0: None, }; - let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox"); - let codex_linux_sandbox_exe: Option = Some(PathBuf::from(sandbox_program)); + let codex_linux_sandbox_exe: Option = Some(codex_linux_sandbox_exe()); let permission_profile = PermissionProfile::read_only(); let result = process_exec_tool_call( params, @@ -530,6 +587,129 @@ async fn sandbox_blocks_codex_symlink_replacement_attack() { assert_ne!(codex_output.exit_code, 0); } +#[tokio::test] +async fn sandbox_keeps_parent_repo_discovery_while_blocking_child_metadata() { + if should_skip_bwrap_tests().await { + eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable"); + return; + } + + let git_available = std::process::Command::new("git") + .arg("--version") + .status() + .is_ok_and(|status| status.success()); + let python_available = std::process::Command::new("python3") + .arg("--version") + .status() + .is_ok_and(|status| status.success()); + if !git_available || !python_available { + eprintln!("skipping bwrap test: git or python3 is unavailable"); + return; + } + + let tmpdir = tempfile::tempdir().expect("tempdir"); + let repo = tmpdir.path().join("repo"); + let subdir = repo.join("sub"); + std::fs::create_dir_all(&subdir).expect("create nested workspace"); + assert!( + std::process::Command::new("git") + .arg("init") + .arg("-q") + .arg(&repo) + .status() + .expect("git init should run") + .success(), + "git init should create parent repo" + ); + + let repo = repo.to_string_lossy(); + let script = format!( + r#"set -e +test "$(git rev-parse --show-toplevel)" = '{repo}' +git status --short > status.before +if grep -E '(^|[[:space:]])\.(git|codex|agents)(/|$)' status.before; then + cat status.before + exit 21 +fi +"#, + ); + + let output = run_cmd_result_with_cwd_and_writable_roots( + &["bash", "-lc", &script], + &subdir, + std::slice::from_ref(&subdir), + LONG_TIMEOUT_MS, + /*use_legacy_landlock*/ false, + /*network_access*/ true, + ) + .await + .expect("sandboxed command should execute"); + + assert_eq!( + output.exit_code, 0, + "stdout:\n{}\nstderr:\n{}", + output.stdout.text, output.stderr.text + ); + + let git_init_output = expect_denied( + run_cmd_result_with_cwd_and_writable_roots( + &["git", "init", "-q"], + &subdir, + std::slice::from_ref(&subdir), + LONG_TIMEOUT_MS, + /*use_legacy_landlock*/ false, + /*network_access*/ true, + ) + .await, + "child git init should be denied", + ); + assert_ne!(git_init_output.exit_code, 0); + assert!(!subdir.join(".git").exists()); + + let mkdir_codex_output = expect_denied( + run_cmd_result_with_cwd_and_writable_roots( + &["mkdir", ".codex"], + &subdir, + std::slice::from_ref(&subdir), + LONG_TIMEOUT_MS, + /*use_legacy_landlock*/ false, + /*network_access*/ true, + ) + .await, + "child .codex directory creation should be denied", + ); + assert_ne!(mkdir_codex_output.exit_code, 0); + assert!(!subdir.join(".codex").exists()); + + let script = format!( + r#"set -e +test "$(git rev-parse --show-toplevel)" = '{repo}' +printf '%s\n' 'import json, sys' 'for line in sys.stdin:' ' obj = json.loads(line)' ' print(obj.get("message", obj))' > jsonl_viewer.py +printf '%s\n' '{{"message":"ok"}}' | python3 jsonl_viewer.py | grep -q ok +"#, + ); + let output = run_cmd_result_with_cwd_and_writable_roots( + &["bash", "-lc", &script], + &subdir, + std::slice::from_ref(&subdir), + LONG_TIMEOUT_MS, + /*use_legacy_landlock*/ false, + /*network_access*/ true, + ) + .await + .expect("sandboxed command should execute"); + assert_eq!( + output.exit_code, 0, + "stdout:\n{}\nstderr:\n{}", + output.stdout.text, output.stderr.text + ); + + assert!(subdir.join("jsonl_viewer.py").is_file()); + assert!(!subdir.join(".git").exists()); + assert!(!subdir.join(".codex").exists()); + assert!(!subdir.join(".agents").exists()); +} + #[tokio::test] async fn sandbox_blocks_explicit_split_policy_carveouts_under_bwrap() { if should_skip_bwrap_tests().await { @@ -543,7 +723,7 @@ async fn sandbox_blocks_explicit_split_policy_carveouts_under_bwrap() { let blocked_target = blocked.join("secret.txt"); // These tests bypass the usual legacy-policy bridge, so explicitly keep // the sandbox helper binary and minimal runtime paths readable. - let sandbox_helper_dir = PathBuf::from(env!("CARGO_BIN_EXE_codex-linux-sandbox")) + let sandbox_helper_dir = codex_linux_sandbox_exe() .parent() .expect("sandbox helper should have a parent") .to_path_buf(); @@ -611,7 +791,7 @@ async fn sandbox_reenables_writable_subpaths_under_unreadable_parents() { let allowed_target = allowed.join("note.txt"); // These tests bypass the usual legacy-policy bridge, so explicitly keep // the sandbox helper binary and minimal runtime paths readable. - let sandbox_helper_dir = PathBuf::from(env!("CARGO_BIN_EXE_codex-linux-sandbox")) + let sandbox_helper_dir = codex_linux_sandbox_exe() .parent() .expect("sandbox helper should have a parent") .to_path_buf();