From 74f06dcdfbb9f0c9f93ca0814ee873043e78faa4 Mon Sep 17 00:00:00 2001 From: evawong-oai Date: Wed, 29 Apr 2026 16:14:14 -0700 Subject: [PATCH] Enforce workspace metadata protections in Linux sandbox (#19852) ## Summary Enforce FileSystemSandboxPolicy protected metadata names in the Linux bubblewrap adapter so `.git`, `.agents`, and `.codex` remain read only inside writable workspace roots unless the policy grants an explicit write carveout. ## Scope 1. Translate protected metadata names from FileSystemSandboxPolicy into bubblewrap masks for existing metadata paths. 2. Represent missing protected metadata paths as guarded mount targets so agents cannot create `.git`, `.agents`, or `.codex` under writable roots. 3. Preserve normal git discovery for existing repos, worktrees, and parent repos. 4. Keep explicit user write grants working when policy allows a protected metadata path directly. ## Not in scope 1. No shell preflight UX. 2. No TUI runtime profile propagation. 3. No macOS Seatbelt changes in this PR. ## Reviewer focus 1. This should be reviewed as the Linux enforcement adapter for the policy primitive from PR 19846. 2. macOS enforcement already landed in PR 19847. 3. The important invariant is that `FileSystemSandboxPolicy` is the source of truth for `.git`, `.agents`, and `.codex`. ## Validation 1. `git diff` whitespace check passed. 2. `cargo fmt` check passed with the existing stable rustfmt warning about `imports_granularity`. 3. Full Linux sandbox Cargo test suite passed on the devbox. 4. Devbox forty six case suite passed at head `012accb703c13bd28df5b40079a9bf183036336a`. 5. Devbox summary: pass 46, fail 0. 6. The devbox suite was run through `just c sandbox linux`. 7. Focused repo test for Viyat parent repo case passed on the devbox. --- codex-rs/linux-sandbox/src/bwrap.rs | 768 +++++++++++++-- codex-rs/linux-sandbox/src/linux_run_main.rs | 880 +++++++++++++++++- .../linux-sandbox/src/linux_run_main_tests.rs | 178 ++++ .../linux-sandbox/tests/suite/landlock.rs | 198 +++- 4 files changed, 1911 insertions(+), 113 deletions(-) diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index ff40f3663..22e7d7253 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 21fbbe826..c88a28b32 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 2b3f96355..228cea6e5 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 3b97e933b..efbcd0b48 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();