diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 464b7d72a2..8ff4c59742 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2122,6 +2122,7 @@ dependencies = [ "codex-rmcp-client", "codex-rollout-trace", "codex-sandboxing", + "codex-shell-command", "codex-state", "codex-stdio-to-uds", "codex-terminal-detection", diff --git a/codex-rs/cli/Cargo.toml b/codex-rs/cli/Cargo.toml index 5f4f3aee10..b1af3e7bbd 100644 --- a/codex-rs/cli/Cargo.toml +++ b/codex-rs/cli/Cargo.toml @@ -43,6 +43,7 @@ codex-responses-api-proxy = { workspace = true } codex-rmcp-client = { workspace = true } codex-rollout-trace = { workspace = true } codex-sandboxing = { workspace = true } +codex-shell-command = { workspace = true } codex-state = { workspace = true } codex-stdio-to-uds = { workspace = true } codex-terminal-detection = { workspace = true } diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index c85da0f5f2..698d058ef0 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -3,6 +3,7 @@ mod pid_tracker; #[cfg(target_os = "macos")] mod seatbelt; +use std::path::Path; use std::path::PathBuf; use std::process::Stdio; @@ -15,7 +16,9 @@ use codex_core::exec_env::create_env; use codex_core::spawn::CODEX_SANDBOX_ENV_VAR; use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use codex_protocol::config_types::SandboxMode; +use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::permissions::forbidden_agent_preserved_path_write; use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_policies; #[cfg(target_os = "macos")] use codex_sandboxing::seatbelt::CreateSeatbeltCommandArgsParams; @@ -142,6 +145,13 @@ async fn run_command_under_sandbox( // sandbox policy. In the future, we could add a CLI option to set them // separately. let sandbox_policy_cwd = cwd.clone(); + if let Some(reason) = preserved_path_write_forbidden_reason( + &command, + cwd.as_path(), + &config.permissions.file_system_sandbox_policy, + ) { + anyhow::bail!("{reason}"); + } let env = create_env( &config.permissions.shell_environment_policy, @@ -279,6 +289,126 @@ async fn run_command_under_sandbox( handle_exit_status(status); } +fn preserved_path_write_forbidden_reason( + command: &[String], + cwd: &Path, + file_system_sandbox_policy: &FileSystemSandboxPolicy, +) -> Option { + let commands = codex_shell_command::bash::parse_shell_lc_plain_commands(command) + .or_else(|| codex_shell_command::bash::parse_shell_lc_command_word_prefixes(command)) + .unwrap_or_else(|| vec![command.to_vec()]); + + for simple_command in commands { + if let Some(name) = + simple_command_preserved_path_write(&simple_command, cwd, file_system_sandbox_policy) + { + return Some(preserved_path_write_reason(name)); + } + } + + if let Some(targets) = + codex_shell_command::bash::parse_shell_lc_write_redirection_targets(command) + { + for target in targets { + if let Some(name) = forbidden_agent_preserved_path_write( + Path::new(&target), + cwd, + file_system_sandbox_policy, + ) { + return Some(preserved_path_write_reason(name)); + } + } + } + None +} + +fn preserved_path_write_reason(name: &str) -> String { + format!("command targets preserved workspace metadata path `{name}`") +} + +fn simple_command_preserved_path_write( + command: &[String], + cwd: &Path, + file_system_sandbox_policy: &FileSystemSandboxPolicy, +) -> Option<&'static str> { + let program = command.first().map(|program| { + Path::new(program) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or(program) + })?; + + match program { + "git" => git_init_preserved_path_write(command, cwd, file_system_sandbox_policy), + "touch" | "mkdir" | "rm" | "rmdir" | "ln" | "mv" | "cp" | "install" => command + .iter() + .skip(1) + .filter(|arg| !arg.starts_with('-')) + .find_map(|arg| { + forbidden_agent_preserved_path_write( + Path::new(arg), + cwd, + file_system_sandbox_policy, + ) + }), + _ => None, + } +} + +fn git_init_preserved_path_write( + command: &[String], + cwd: &Path, + file_system_sandbox_policy: &FileSystemSandboxPolicy, +) -> Option<&'static str> { + let mut git_cwd = PathBuf::from(cwd); + let mut index = 1; + + while index < command.len() { + match command[index].as_str() { + "-C" => { + let next = command.get(index + 1)?; + git_cwd = resolve_shell_operand(Path::new(next), &git_cwd); + index += 2; + } + "--" => { + index += 1; + break; + } + arg if arg.starts_with('-') => { + index += 1; + } + _ => break, + } + } + + if command.get(index).map(String::as_str) != Some("init") { + return None; + } + + let init_target = command + .iter() + .skip(index + 1) + .find(|arg| !arg.starts_with('-')) + .map_or_else( + || git_cwd.clone(), + |arg| resolve_shell_operand(Path::new(arg), &git_cwd), + ); + + forbidden_agent_preserved_path_write( + init_target.join(".git").as_path(), + &git_cwd, + file_system_sandbox_policy, + ) +} + +fn resolve_shell_operand(path: &Path, cwd: &Path) -> PathBuf { + if path.is_absolute() { + path.to_path_buf() + } else { + cwd.join(path) + } +} + #[cfg(target_os = "windows")] async fn run_command_under_windows_session( config: &Config, @@ -438,6 +568,25 @@ async fn spawn_debug_sandbox_child( cmd.env(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR, "1"); } + #[cfg(target_os = "linux")] + { + let parent_pid = unsafe { libc::getpid() }; + // SAFETY: `pre_exec` runs in the child immediately before exec. The + // closure only installs a parent-death signal and checks the inherited + // parent pid to close the fork/exec race. + unsafe { + cmd.pre_exec(move || { + if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) == -1 { + return Err(std::io::Error::last_os_error()); + } + if libc::getppid() != parent_pid { + libc::raise(libc::SIGTERM); + } + Ok(()) + }); + } + } + cmd.stdin(Stdio::inherit()) .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) @@ -762,4 +911,111 @@ mod tests { Ok(()) } + + fn legacy_workspace_write_policy(cwd: &std::path::Path) -> FileSystemSandboxPolicy { + let policy = codex_protocol::protocol::SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: codex_protocol::protocol::ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: vec![], + }, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, cwd) + } + + #[test] + fn debug_sandbox_preserved_path_guard_blocks_git_init_under_parent_repo() { + let repo = TempDir::new().expect("tempdir"); + std::fs::create_dir(repo.path().join(".git")).expect("create parent .git"); + let cwd = repo.path().join("sub"); + std::fs::create_dir(&cwd).expect("create cwd"); + let policy = legacy_workspace_write_policy(&cwd); + + let reason = preserved_path_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "git init".to_string(), + ], + &cwd, + &policy, + ); + + assert_eq!( + reason, + Some("command targets preserved workspace metadata path `.git`".to_string()) + ); + } + + #[test] + fn debug_sandbox_preserved_path_guard_allows_normal_git_under_parent_repo() { + let repo = TempDir::new().expect("tempdir"); + std::fs::create_dir(repo.path().join(".git")).expect("create parent .git"); + let cwd = repo.path().join("sub"); + std::fs::create_dir(&cwd).expect("create cwd"); + let policy = legacy_workspace_write_policy(&cwd); + + let reason = preserved_path_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "git status --short".to_string(), + ], + &cwd, + &policy, + ); + + assert_eq!(reason, None); + } + + #[test] + fn debug_sandbox_preserved_path_guard_blocks_preserved_path_redirections() { + let repo = TempDir::new().expect("tempdir"); + std::fs::create_dir(repo.path().join(".git")).expect("create parent .git"); + let cwd = repo.path().join("sub"); + std::fs::create_dir(&cwd).expect("create cwd"); + let policy = legacy_workspace_write_policy(&cwd); + + let reason = preserved_path_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "printf pwned > .git".to_string(), + ], + &cwd, + &policy, + ); + + assert_eq!( + reason, + Some("command targets preserved workspace metadata path `.git`".to_string()) + ); + } + + #[test] + fn debug_sandbox_preserved_path_guard_blocks_git_init_inside_complex_script() { + let repo = TempDir::new().expect("tempdir"); + std::fs::create_dir(repo.path().join(".git")).expect("create parent .git"); + let cwd = repo.path().join("sub"); + std::fs::create_dir(&cwd).expect("create cwd"); + let policy = legacy_workspace_write_policy(&cwd); + + let reason = preserved_path_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "set -e\nif git init -q; then\n exit 22\nfi".to_string(), + ], + &cwd, + &policy, + ); + + assert_eq!( + reason, + Some("command targets preserved workspace metadata path `.git`".to_string()) + ); + } } diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index b7512b7076..0cdb697886 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -1,7 +1,11 @@ use codex_protocol::ThreadId; use codex_protocol::models::ShellCommandToolCallParams; use codex_protocol::models::ShellToolCallParams; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::forbidden_agent_preserved_path_write; use serde_json::Value as JsonValue; +use std::path::Path; +use std::path::PathBuf; use std::sync::Arc; use crate::exec::ExecCapturePolicy; @@ -89,6 +93,126 @@ struct RunExecLikeArgs { shell_runtime_backend: ShellRuntimeBackend, } +fn preserved_path_write_forbidden_reason( + command: &[String], + cwd: &Path, + file_system_sandbox_policy: &FileSystemSandboxPolicy, +) -> Option { + let commands = codex_shell_command::bash::parse_shell_lc_plain_commands(command) + .or_else(|| codex_shell_command::bash::parse_shell_lc_command_word_prefixes(command)) + .unwrap_or_else(|| vec![command.to_vec()]); + + for simple_command in commands { + if let Some(name) = + simple_command_preserved_path_write(&simple_command, cwd, file_system_sandbox_policy) + { + return Some(preserved_path_write_reason(name)); + } + } + + if let Some(targets) = + codex_shell_command::bash::parse_shell_lc_write_redirection_targets(command) + { + for target in targets { + if let Some(name) = forbidden_agent_preserved_path_write( + Path::new(&target), + cwd, + file_system_sandbox_policy, + ) { + return Some(preserved_path_write_reason(name)); + } + } + } + None +} + +fn preserved_path_write_reason(name: &str) -> String { + format!("command targets preserved workspace metadata path `{name}`") +} + +fn simple_command_preserved_path_write( + command: &[String], + cwd: &Path, + file_system_sandbox_policy: &FileSystemSandboxPolicy, +) -> Option<&'static str> { + let program = command.first().map(|program| { + Path::new(program) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or(program) + })?; + + match program { + "git" => git_init_preserved_path_write(command, cwd, file_system_sandbox_policy), + "touch" | "mkdir" | "rm" | "rmdir" | "ln" | "mv" | "cp" | "install" => command + .iter() + .skip(1) + .filter(|arg| !arg.starts_with('-')) + .find_map(|arg| { + forbidden_agent_preserved_path_write( + Path::new(arg), + cwd, + file_system_sandbox_policy, + ) + }), + _ => None, + } +} + +fn git_init_preserved_path_write( + command: &[String], + cwd: &Path, + file_system_sandbox_policy: &FileSystemSandboxPolicy, +) -> Option<&'static str> { + let mut git_cwd = PathBuf::from(cwd); + let mut index = 1; + + while index < command.len() { + match command[index].as_str() { + "-C" => { + let next = command.get(index + 1)?; + git_cwd = resolve_shell_operand(Path::new(next), &git_cwd); + index += 2; + } + "--" => { + index += 1; + break; + } + arg if arg.starts_with('-') => { + index += 1; + } + _ => break, + } + } + + if command.get(index).map(String::as_str) != Some("init") { + return None; + } + + let init_target = command + .iter() + .skip(index + 1) + .find(|arg| !arg.starts_with('-')) + .map_or_else( + || git_cwd.clone(), + |arg| resolve_shell_operand(Path::new(arg), &git_cwd), + ); + + forbidden_agent_preserved_path_write( + init_target.join(".git").as_path(), + &git_cwd, + file_system_sandbox_policy, + ) +} + +fn resolve_shell_operand(path: &Path, cwd: &Path) -> PathBuf { + if path.is_absolute() { + path.to_path_buf() + } else { + cwd.join(path) + } +} + impl ShellHandler { fn to_exec_params( params: &ShellToolCallParams, @@ -531,6 +655,14 @@ impl ShellHandler { prefix_rule, }) .await; + let exec_approval_requirement = preserved_path_write_forbidden_reason( + &exec_params.command, + &exec_params.cwd, + &turn.file_system_sandbox_policy, + ) + .map_or(exec_approval_requirement, |reason| { + crate::tools::sandboxing::ExecApprovalRequirement::Forbidden { reason } + }); let req = ShellRequest { command: exec_params.command.clone(), diff --git a/codex-rs/core/src/tools/handlers/shell_tests.rs b/codex-rs/core/src/tools/handlers/shell_tests.rs index 49e2cf8f75..1a46b1dfa9 100644 --- a/codex-rs/core/src/tools/handlers/shell_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_tests.rs @@ -2,6 +2,9 @@ use std::path::PathBuf; use std::sync::Arc; use codex_protocol::models::ShellCommandToolCallParams; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::protocol::ReadOnlyAccess; +use codex_protocol::protocol::SandboxPolicy; use core_test_support::PathBufExt; use core_test_support::test_path_buf; use pretty_assertions::assert_eq; @@ -25,6 +28,7 @@ use codex_shell_command::is_safe_command::is_known_safe_command; use codex_shell_command::powershell::try_find_powershell_executable_blocking; use codex_shell_command::powershell::try_find_pwsh_executable_blocking; use serde_json::json; +use tempfile::TempDir; use tokio::sync::Mutex; use tokio::sync::watch; @@ -75,6 +79,134 @@ fn assert_safe(shell: &Shell, command: &str) { ))); } +fn legacy_workspace_write_policy(cwd: &std::path::Path) -> FileSystemSandboxPolicy { + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: vec![], + }, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, cwd) +} + +#[test] +fn preserved_path_detector_blocks_git_init_under_parent_repo() { + let repo = TempDir::new().expect("tempdir"); + std::fs::create_dir(repo.path().join(".git")).expect("create parent .git"); + let cwd = repo.path().join("sub"); + std::fs::create_dir(&cwd).expect("create cwd"); + let policy = legacy_workspace_write_policy(&cwd); + + let reason = super::preserved_path_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "git init".to_string(), + ], + &cwd, + &policy, + ); + + assert_eq!( + reason, + Some("command targets preserved workspace metadata path `.git`".to_string()) + ); +} + +#[test] +fn preserved_path_detector_allows_normal_git_under_parent_repo() { + let repo = TempDir::new().expect("tempdir"); + std::fs::create_dir(repo.path().join(".git")).expect("create parent .git"); + let cwd = repo.path().join("sub"); + std::fs::create_dir(&cwd).expect("create cwd"); + let policy = legacy_workspace_write_policy(&cwd); + + let reason = super::preserved_path_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "git status --short".to_string(), + ], + &cwd, + &policy, + ); + + assert_eq!(reason, None); +} + +#[test] +fn preserved_path_detector_blocks_direct_preserved_path_writes() { + let cwd = TempDir::new().expect("tempdir"); + let policy = legacy_workspace_write_policy(cwd.path()); + + let reason = super::preserved_path_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "touch .git && mkdir -p .codex".to_string(), + ], + cwd.path(), + &policy, + ); + + assert_eq!( + reason, + Some("command targets preserved workspace metadata path `.git`".to_string()) + ); +} + +#[test] +fn preserved_path_detector_blocks_preserved_path_redirections() { + let repo = TempDir::new().expect("tempdir"); + std::fs::create_dir(repo.path().join(".git")).expect("create parent .git"); + let cwd = repo.path().join("sub"); + std::fs::create_dir(&cwd).expect("create cwd"); + let policy = legacy_workspace_write_policy(&cwd); + + let reason = super::preserved_path_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "printf pwned > .git".to_string(), + ], + &cwd, + &policy, + ); + + assert_eq!( + reason, + Some("command targets preserved workspace metadata path `.git`".to_string()) + ); +} + +#[test] +fn preserved_path_detector_blocks_git_init_inside_complex_script() { + let repo = TempDir::new().expect("tempdir"); + std::fs::create_dir(repo.path().join(".git")).expect("create parent .git"); + let cwd = repo.path().join("sub"); + std::fs::create_dir(&cwd).expect("create cwd"); + let policy = legacy_workspace_write_policy(&cwd); + + let reason = super::preserved_path_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "set -e\nif git init -q; then\n exit 22\nfi".to_string(), + ], + &cwd, + &policy, + ); + + assert_eq!( + reason, + Some("command targets preserved workspace metadata path `.git`".to_string()) + ); +} + #[tokio::test] async fn shell_command_handler_to_exec_params_uses_session_shell_and_turn_context() { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 6cbe76cff5..4e49e4e9dc 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -26,6 +26,7 @@ use std::process::Command; use codex_protocol::error::CodexErr; use codex_protocol::error::Result; +use codex_protocol::permissions::is_preserved_path_name; use codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::WritableRoot; use codex_utils_absolute_path::AbsolutePathBuf; @@ -137,13 +138,17 @@ impl SyntheticMountTarget { } } - fn existing_empty_file(path: &Path, metadata: &Metadata) -> Self { + pub(crate) fn existing_empty_file(path: &Path, metadata: &Metadata) -> Self { Self { path: path.to_path_buf(), pre_existing_file: Some(FileIdentity::from_metadata(metadata)), } } + pub(crate) fn preserves_pre_existing_file(&self) -> bool { + self.pre_existing_file.is_some() + } + pub(crate) fn path(&self) -> &Path { &self.path } @@ -335,7 +340,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, @@ -408,8 +413,11 @@ fn create_filesystem_args( args }; - let mut preserved_files = Vec::new(); - let mut synthetic_mount_targets = Vec::new(); + let mut bwrap_args = BwrapArgs { + args, + preserved_files: Vec::new(), + synthetic_mount_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(); @@ -440,13 +448,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, - &mut synthetic_mount_targets, - unreadable_root, - &allowed_write_paths, - )?; + append_unreadable_root_args(&mut bwrap_args, unreadable_root, &allowed_write_paths)?; } for writable_root in &sorted_writable_roots { @@ -460,13 +462,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 @@ -479,13 +481,7 @@ fn create_filesystem_args( } read_only_subpaths.sort_by_key(|path| path_depth(path)); for subpath in read_only_subpaths { - append_read_only_subpath_args( - &mut args, - &mut preserved_files, - &mut synthetic_mount_targets, - &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() @@ -498,13 +494,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, - &mut synthetic_mount_targets, - &unreadable_root, - &allowed_write_paths, - )?; + append_unreadable_root_args(&mut bwrap_args, &unreadable_root, &allowed_write_paths)?; } } @@ -520,20 +510,10 @@ 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, - &mut synthetic_mount_targets, - &unreadable_root, - &allowed_write_paths, - )?; + append_unreadable_root_args(&mut bwrap_args, &unreadable_root, &allowed_write_paths)?; } - Ok(BwrapArgs { - args, - preserved_files, - synthetic_mount_targets, - }) + Ok(bwrap_args) } fn expand_unreadable_globs_with_ripgrep( @@ -858,9 +838,7 @@ fn append_mount_target_parent_dir_args(args: &mut Vec, mount_target: &Pa } fn append_read_only_subpath_args( - args: &mut Vec, - preserved_files: &mut Vec, - synthetic_mount_targets: &mut Vec, + bwrap_args: &mut BwrapArgs, subpath: &Path, allowed_write_paths: &[PathBuf], ) -> Result<()> { @@ -884,13 +862,7 @@ fn append_read_only_subpath_args( // Another concurrent bwrap setup can leave a zero-byte mount target at // a missing preserved path. Treat it like the missing case instead of // binding that transient host path as the stable source. - append_existing_empty_file_bind_data_args( - args, - preserved_files, - synthetic_mount_targets, - subpath, - &metadata, - )?; + append_existing_empty_file_bind_data_args(bwrap_args, subpath, &metadata)?; return Ok(()); } @@ -898,66 +870,52 @@ fn append_read_only_subpath_args( if let Some(first_missing_component) = find_first_non_existent_component(subpath) && is_within_allowed_write_paths(&first_missing_component, allowed_write_paths) { - append_missing_empty_file_bind_data_args( - args, - preserved_files, - synthetic_mount_targets, - &first_missing_component, - )?; + append_missing_empty_file_bind_data_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( - args: &mut Vec, - preserved_files: &mut Vec, - path: &Path, -) -> Result<()> { - if preserved_files.is_empty() { - preserved_files.push(File::open("/dev/null")?); +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 = preserved_files[0].as_raw_fd().to_string(); - args.push("--ro-bind-data".to_string()); - args.push(null_fd); - args.push(path_to_string(path)); + 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_missing_empty_file_bind_data_args( - args: &mut Vec, - preserved_files: &mut Vec, - synthetic_mount_targets: &mut Vec, - path: &Path, -) -> Result<()> { - append_empty_file_bind_data_args(args, preserved_files, path)?; - synthetic_mount_targets.push(SyntheticMountTarget::missing(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( - args: &mut Vec, - preserved_files: &mut Vec, - synthetic_mount_targets: &mut Vec, + bwrap_args: &mut BwrapArgs, path: &Path, metadata: &Metadata, ) -> Result<()> { - append_empty_file_bind_data_args(args, preserved_files, path)?; - synthetic_mount_targets.push(SyntheticMountTarget::existing_empty_file(path, metadata)); + append_empty_file_bind_data_args(bwrap_args, path)?; + bwrap_args + .synthetic_mount_targets + .push(SyntheticMountTarget::existing_empty_file(path, metadata)); Ok(()) } fn append_unreadable_root_args( - args: &mut Vec, - preserved_files: &mut Vec, - synthetic_mount_targets: &mut Vec, + bwrap_args: &mut BwrapArgs, unreadable_root: &Path, allowed_write_paths: &[PathBuf], ) -> Result<()> { @@ -982,27 +940,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) { - append_missing_empty_file_bind_data_args( - args, - preserved_files, - synthetic_mount_targets, - &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<()> { @@ -1012,33 +959,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(()); } - args.push("--perms".to_string()); - args.push("000".to_string()); - append_empty_file_bind_data_args(args, preserved_files, unreadable_root) + 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. @@ -1049,7 +1000,7 @@ fn is_within_allowed_write_paths(path: &Path, allowed_write_paths: &[PathBuf]) - } fn transient_empty_preserved_file_metadata(path: &Path) -> Option { - if !has_preserved_path_name(path) { + if !path.file_name().is_some_and(is_preserved_path_name) { return None; } @@ -1061,11 +1012,6 @@ fn transient_empty_preserved_file_metadata(path: &Path) -> Option { } } -fn has_preserved_path_name(path: &Path) -> bool { - path.file_name() - .is_some_and(|name| name == ".git" || name == ".agents" || name == ".codex") -} - fn first_writable_symlink_component_in_path( target_path: &Path, allowed_write_paths: &[PathBuf], @@ -1143,6 +1089,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; @@ -1533,7 +1480,15 @@ mod tests { assert_empty_file_bound_without_perms(&args.args, &blocked); assert_eq!(args.preserved_files.len(), 1); - assert_eq!(synthetic_mount_target_paths(&args), vec![blocked.clone()]); + assert_eq!( + synthetic_mount_target_paths(&args), + vec![ + workspace.join(".git"), + workspace.join(".agents"), + workspace.join(".codex"), + blocked.clone(), + ] + ); assert!( !blocked.exists(), "missing path mask should not materialize host-side preserved paths at arg construction time", @@ -1563,7 +1518,14 @@ mod tests { let dot_git_str = path_to_string(&dot_git); assert_empty_file_bound_without_perms(&args.args, &dot_git); - assert_eq!(synthetic_mount_target_paths(&args), vec![dot_git.clone()]); + assert_eq!( + synthetic_mount_target_paths(&args), + vec![ + dot_git.clone(), + workspace.join(".agents"), + workspace.join(".codex"), + ] + ); assert!( !args .args @@ -1578,6 +1540,36 @@ mod tests { ); } + #[test] + fn missing_child_git_under_parent_repo_stays_absent() { + 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::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!( + !args.args.iter().any(|arg| arg == &path_to_string(&dot_git)), + "missing child .git under an existing parent repo must not be materialized", + ); + assert!( + !synthetic_mount_target_paths(&args).contains(&dot_git), + "missing child .git should not be tracked for post-bwrap cleanup", + ); + } + #[test] fn ignores_missing_writable_roots() { let temp_dir = TempDir::new().expect("temp dir"); @@ -1638,6 +1630,9 @@ mod tests { PathBuf::from("/.git"), PathBuf::from("/.agents"), PathBuf::from("/.codex"), + PathBuf::from("/dev/.git"), + PathBuf::from("/dev/.agents"), + PathBuf::from("/dev/.codex"), ] ); let null_fd = args.preserved_files[0].as_raw_fd().to_string(); @@ -1665,13 +1660,22 @@ mod tests { null_fd.clone(), "/.agents".to_string(), "--ro-bind-data".to_string(), - null_fd, + null_fd.clone(), "/.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(), + "--ro-bind-data".to_string(), + null_fd.clone(), + "/dev/.git".to_string(), + "--ro-bind-data".to_string(), + null_fd.clone(), + "/dev/.agents".to_string(), + "--ro-bind-data".to_string(), + null_fd, + "/dev/.codex".to_string(), ] ); } diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index c3d833875d..9c4826b818 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -3,10 +3,15 @@ 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::atomic::AtomicI32; +use std::sync::atomic::Ordering; use crate::bwrap::BwrapNetworkMode; use crate::bwrap::BwrapOptions; @@ -21,6 +26,20 @@ use codex_protocol::protocol::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; +static BWRAP_CHILD_PID: 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"; + +#[derive(Debug)] +struct SyntheticMountTargetRegistration { + target: crate::bwrap::SyntheticMountTarget, + marker_file: PathBuf, + marker_dir: PathBuf, +} + #[derive(Debug, Parser)] /// CLI surface for the Linux sandbox helper. /// @@ -578,7 +597,13 @@ fn run_or_exec_bwrap(bwrap_args: crate::bwrap::BwrapArgs) -> ! { } fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::BwrapArgs) -> ! { - let synthetic_mount_targets = bwrap_args.synthetic_mount_targets.clone(); + let crate::bwrap::BwrapArgs { + args, + preserved_files, + synthetic_mount_targets, + } = bwrap_args; + let synthetic_mount_registrations = register_synthetic_mount_targets(&synthetic_mount_targets); + let parent_pid = unsafe { libc::getpid() }; let pid = unsafe { libc::fork() }; if pid < 0 { let err = std::io::Error::last_os_error(); @@ -586,45 +611,305 @@ fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::Bwr } if pid == 0 { - exec_bwrap(bwrap_args.args, bwrap_args.preserved_files); + 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); + exec_bwrap(args, preserved_files); } - 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}"); - } - - cleanup_synthetic_mount_targets(&synthetic_mount_targets); + install_bwrap_signal_forwarders(pid); + let status = wait_for_bwrap_child(pid); + BWRAP_CHILD_PID.store(0, Ordering::SeqCst); + cleanup_synthetic_mount_targets(&synthetic_mount_registrations); exit_with_wait_status(status); } -fn cleanup_synthetic_mount_targets(targets: &[crate::bwrap::SyntheticMountTarget]) { - for target in targets.iter().rev() { - let path = target.path(); - let metadata = match fs::symlink_metadata(path) { - Ok(metadata) => metadata, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue, - Err(err) => panic!( - "failed to inspect synthetic bubblewrap mount target {}: {err}", - path.display() - ), - }; - if !target.should_remove_after_bwrap(&metadata) { - continue; - } - 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() - ), +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); } } } +fn install_bwrap_signal_forwarders(pid: libc::pid_t) { + BWRAP_CHILD_PID.store(pid, Ordering::SeqCst); + for signal in FORWARDED_SIGNALS { + let mut 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, std::ptr::null_mut()) < 0 { + let err = std::io::Error::last_os_error(); + panic!("failed to install bubblewrap signal forwarder for {signal}: {err}"); + } + } + } +} + +extern "C" fn forward_signal_to_bwrap_child(signal: libc::c_int) { + let pid = BWRAP_CHILD_PID.load(Ordering::SeqCst); + if pid > 0 { + unsafe { + libc::kill(-pid, signal); + libc::kill(pid, signal); + } + } +} + +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_file() + && synthetic_mount_marker_dir_has_active_synthetic_owner(&marker_dir) + { + crate::bwrap::SyntheticMountTarget::missing(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 synthetic_mount_marker_contents(target: &crate::bwrap::SyntheticMountTarget) -> &'static [u8] { + if target.preserves_pre_existing_file() { + 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 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 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() + ), + } +} + +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)); @@ -655,7 +940,12 @@ fn exit_with_wait_status(status: libc::c_int) -> ! { /// 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 synthetic_mount_targets = bwrap_args.synthetic_mount_targets.clone(); + let crate::bwrap::BwrapArgs { + args, + preserved_files, + synthetic_mount_targets, + } = bwrap_args; + let synthetic_mount_registrations = register_synthetic_mount_targets(&synthetic_mount_targets); let mut pipe_fds = [0; 2]; let pipe_res = unsafe { libc::pipe2(pipe_fds.as_mut_ptr(), libc::O_CLOEXEC) }; @@ -683,7 +973,7 @@ 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); } // Parent: close the write end and read stderr while the child runs. @@ -697,13 +987,8 @@ 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}"); - } - cleanup_synthetic_mount_targets(&synthetic_mount_targets); + wait_for_bwrap_child(pid); + cleanup_synthetic_mount_targets(&synthetic_mount_registrations); 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 4d54b4988f..a436983d5a 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::protocol::FileSystemSandboxPolicy; #[cfg(test)] use codex_protocol::protocol::NetworkSandboxPolicy; @@ -264,11 +268,12 @@ fn cleanup_synthetic_mount_targets_removes_only_empty_files() { std::fs::write(&empty_file, "").expect("write empty file"); std::fs::write(&non_empty_file, "keep").expect("write nonempty file"); - cleanup_synthetic_mount_targets(&[ + let registrations = register_synthetic_mount_targets(&[ crate::bwrap::SyntheticMountTarget::missing(&empty_file), crate::bwrap::SyntheticMountTarget::missing(&non_empty_file), crate::bwrap::SyntheticMountTarget::missing(&missing_file), ]); + cleanup_synthetic_mount_targets(®istrations); assert!(!empty_file.exists()); assert_eq!( @@ -278,6 +283,115 @@ fn cleanup_synthetic_mount_targets_removes_only_empty_files() { 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 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 sandbox_policy = SandboxPolicy::new_read_only_policy(); diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 9040649c47..295f801728 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -19,6 +19,61 @@ use crate::protocol::NetworkAccess; use crate::protocol::SandboxPolicy; use crate::protocol::WritableRoot; +const PRESERVED_GIT_PATH_NAME: &str = ".git"; +const PRESERVED_AGENTS_PATH_NAME: &str = ".agents"; +const PRESERVED_CODEX_PATH_NAME: &str = ".codex"; + +/// Top-level workspace metadata paths that stay protected under writable roots. +pub const PRESERVED_PATH_NAMES: &[&str] = &[ + PRESERVED_GIT_PATH_NAME, + PRESERVED_AGENTS_PATH_NAME, + PRESERVED_CODEX_PATH_NAME, +]; + +/// Returns true when a path basename is one of the preserved workspace metadata names. +pub fn is_preserved_path_name(name: &OsStr) -> bool { + PRESERVED_PATH_NAMES + .iter() + .any(|preserved| name == OsStr::new(preserved)) +} + +/// Returns the preserved workspace metadata name when an agent write to `path` +/// should be blocked before execution. +pub fn forbidden_agent_preserved_path_write( + path: &Path, + cwd: &Path, + file_system_sandbox_policy: &FileSystemSandboxPolicy, +) -> Option<&'static str> { + if !matches!( + file_system_sandbox_policy.kind, + FileSystemSandboxKind::Restricted + ) { + return None; + } + + let target = resolve_candidate_path(path, cwd)?; + let (preserved_path, preserved_name) = first_preserved_component(target.as_path())?; + if has_explicit_write_entry_for_path(file_system_sandbox_policy, &preserved_path, cwd) { + return None; + } + + if !file_system_sandbox_policy.can_write_path_with_cwd(preserved_path.as_path(), cwd) { + return Some(preserved_name); + } + + // A child directory of an existing Git repository must keep normal Git + // discovery working, so the sandbox layer intentionally does not + // materialize a missing child `.git`. Block that creation at command time. + if preserved_name == PRESERVED_GIT_PATH_NAME + && !preserved_path.as_path().exists() + && has_ancestor_git_metadata(preserved_path.as_path()) + { + return Some(preserved_name); + } + + None +} + #[derive( Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Display, Default, JsonSchema, TS, )] @@ -474,27 +529,17 @@ impl FileSystemSandboxPolicy { /// into split filesystem policy. pub fn from_legacy_sandbox_policy_for_cwd(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Self { let mut file_system_policy = Self::from(sandbox_policy); - if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = sandbox_policy { + if let SandboxPolicy::WorkspaceWrite { .. } = sandbox_policy { let legacy_writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); prune_read_entries_under_writable_roots( &mut file_system_policy.entries, &legacy_writable_roots, ); - if let Ok(cwd_root) = AbsolutePathBuf::from_absolute_path(cwd) { + for writable_root in legacy_writable_roots { for protected_path in default_read_only_subpaths_for_writable_root( - &cwd_root, /*protect_missing_preserved_paths*/ true, - ) { - append_default_read_only_path_if_no_explicit_rule( - &mut file_system_policy.entries, - protected_path, - ); - } - } - for writable_root in writable_roots { - for protected_path in default_read_only_subpaths_for_writable_root( - writable_root, - /*protect_missing_preserved_paths*/ false, + &writable_root.root, + /*protect_missing_preserved_paths*/ true, ) { append_default_read_only_path_if_no_explicit_rule( &mut file_system_policy.entries, @@ -687,13 +732,9 @@ impl FileSystemSandboxPolicy { .iter() .filter(|path| normalize_effective_absolute_path((*path).clone()) == root) .collect(); - let protect_missing_preserved_paths = AbsolutePathBuf::from_absolute_path(cwd) - .ok() - .is_some_and(|cwd| normalize_effective_absolute_path(cwd) == root); let mut read_only_subpaths: Vec = default_read_only_subpaths_for_writable_root( - &root, - protect_missing_preserved_paths, + &root, /*protect_missing_preserved_paths*/ true, ) .into_iter() .filter(|path| !has_explicit_resolved_path_entry(&resolved_entries, path)) @@ -1301,18 +1342,21 @@ fn normalize_effective_absolute_path(path: AbsolutePathBuf) -> AbsolutePathBuf { path } -fn default_read_only_subpaths_for_writable_root( +pub(crate) fn default_read_only_subpaths_for_writable_root( writable_root: &AbsolutePathBuf, protect_missing_preserved_paths: bool, ) -> Vec { let mut subpaths: Vec = Vec::new(); - let top_level_git = writable_root.join(".git"); + let top_level_git = writable_root.join(PRESERVED_GIT_PATH_NAME); // This applies to typical repos (directory .git), worktrees/submodules // (file .git with gitdir pointer), and bare repos when the gitdir is the // writable root itself. let top_level_git_is_file = top_level_git.as_path().is_file(); let top_level_git_is_dir = top_level_git.as_path().is_dir(); - if top_level_git_is_dir || top_level_git_is_file || protect_missing_preserved_paths { + let should_protect_top_level_git = top_level_git_is_dir + || top_level_git_is_file + || (protect_missing_preserved_paths && !has_ancestor_git_metadata(writable_root.as_path())); + if should_protect_top_level_git { if top_level_git_is_file && is_git_pointer_file(&top_level_git) && let Some(gitdir) = resolve_gitdir_from_file(&top_level_git) @@ -1322,7 +1366,7 @@ fn default_read_only_subpaths_for_writable_root( subpaths.push(top_level_git); } - let top_level_agents = writable_root.join(".agents"); + let top_level_agents = writable_root.join(PRESERVED_AGENTS_PATH_NAME); if protect_missing_preserved_paths || top_level_agents.as_path().is_dir() { subpaths.push(top_level_agents); } @@ -1331,7 +1375,7 @@ fn default_read_only_subpaths_for_writable_root( // default. For the workspace root itself, protect it even before the // directory exists so first-time creation still goes through the // preserved path approval flow. - let top_level_codex = writable_root.join(".codex"); + let top_level_codex = writable_root.join(PRESERVED_CODEX_PATH_NAME); if protect_missing_preserved_paths || top_level_codex.as_path().is_dir() { subpaths.push(top_level_codex); } @@ -1434,8 +1478,67 @@ fn has_explicit_resolved_path_entry( entries.iter().any(|entry| &entry.path == path) } +fn first_preserved_component(path: &Path) -> Option<(AbsolutePathBuf, &'static str)> { + let mut candidate = PathBuf::new(); + for component in path.components() { + candidate.push(component.as_os_str()); + if let Some(preserved_name) = preserved_path_name(component.as_os_str()) { + let absolute = AbsolutePathBuf::from_absolute_path(candidate).ok()?; + return Some((absolute, preserved_name)); + } + } + None +} + +fn preserved_path_name(name: &OsStr) -> Option<&'static str> { + PRESERVED_PATH_NAMES + .iter() + .copied() + .find(|preserved| name == OsStr::new(preserved)) +} + +fn has_explicit_write_entry_for_path( + policy: &FileSystemSandboxPolicy, + path: &AbsolutePathBuf, + cwd: &Path, +) -> bool { + policy + .resolved_entries_with_cwd(cwd) + .iter() + .any(|entry| entry.access.can_write() && &entry.path == path) +} + +fn has_ancestor_git_metadata(path: &Path) -> bool { + let Some(parent) = path.parent() else { + return false; + }; + parent + .ancestors() + .any(|ancestor| git_metadata_dir(&ancestor.join(PRESERVED_GIT_PATH_NAME)).is_some()) +} + +fn git_metadata_dir(path: &Path) -> Option { + let Ok(metadata) = std::fs::symlink_metadata(path) else { + return None; + }; + if metadata.is_dir() { + return AbsolutePathBuf::from_absolute_path(path).ok(); + } + if !metadata.is_file() { + return None; + } + let Ok(dot_git) = AbsolutePathBuf::from_absolute_path(path) else { + return None; + }; + if !is_git_pointer_file(&dot_git) { + return None; + } + resolve_gitdir_from_file(&dot_git) +} + fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool { - path.as_path().is_file() && path.as_path().file_name() == Some(OsStr::new(".git")) + path.as_path().is_file() + && path.as_path().file_name() == Some(OsStr::new(PRESERVED_GIT_PATH_NAME)) } fn resolve_gitdir_from_file(dot_git: &AbsolutePathBuf) -> Option { @@ -1452,7 +1555,14 @@ fn resolve_gitdir_from_file(dot_git: &AbsolutePathBuf) -> Option parts, + Some((prefix, gitdir_raw)) if prefix.trim() == "gitdir" => (prefix, gitdir_raw), + Some(_) => { + error!( + "Expected {path} to contain a gitdir pointer, but it did not match `gitdir: `.", + path = dot_git.as_path().display() + ); + return None; + } None => { error!( "Expected {path} to contain a gitdir pointer, but it did not match `gitdir: `.", @@ -1626,6 +1736,71 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn writable_roots_keep_missing_git_absent_under_parent_git_repo() { + let repo = TempDir::new().expect("tempdir"); + fs::create_dir(repo.path().join(".git")).expect("create parent .git"); + let cwd = repo.path().join("sub"); + fs::create_dir(&cwd).expect("create subdir"); + let expected_root = + AbsolutePathBuf::from_absolute_path(cwd.canonicalize().expect("canonicalize cwd")) + .expect("absolute canonical root"); + let expected_dot_git = expected_root.join(".git"); + let expected_dot_agents = expected_root.join(".agents"); + let expected_dot_codex = expected_root.join(".codex"); + + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }]); + + let writable_roots = policy.get_writable_roots_with_cwd(&cwd); + assert_eq!(writable_roots.len(), 1); + assert_eq!(writable_roots[0].root, expected_root); + assert!( + !writable_roots[0] + .read_only_subpaths + .contains(&expected_dot_git), + "missing child .git under an existing parent repo is enforced by command policy so Git discovery still works" + ); + assert!( + writable_roots[0] + .read_only_subpaths + .contains(&expected_dot_agents) + ); + assert!( + writable_roots[0] + .read_only_subpaths + .contains(&expected_dot_codex) + ); + } + + #[cfg(unix)] + #[test] + fn writable_roots_protect_missing_git_when_parent_git_metadata_is_invalid() { + let repo = TempDir::new().expect("tempdir"); + fs::create_dir(repo.path().join("real_git_dir")).expect("create real git dir"); + fs::write(repo.path().join(".git"), "notgitdir: real_git_dir").expect("write parent .git"); + let cwd = repo.path().join("sub"); + fs::create_dir(&cwd).expect("create subdir"); + let expected_root = + AbsolutePathBuf::from_absolute_path(cwd.canonicalize().expect("canonicalize cwd")) + .expect("absolute canonical root"); + let expected_dot_git = expected_root.join(".git"); + + assert!( + default_read_only_subpaths_for_writable_root( + &expected_root, + /*protect_missing_preserved_paths*/ true, + ) + .contains(&expected_dot_git), + "invalid parent .git metadata should not suppress child .git protection" + ); + } + #[cfg(unix)] #[test] fn writable_roots_skip_default_preserved_paths_when_explicit_user_rule_exists() { @@ -1700,6 +1875,7 @@ mod tests { fn legacy_workspace_write_projection_blocks_missing_preserved_path_writes() { let cwd = TempDir::new().expect("tempdir"); let dot_git_config = cwd.path().join(".git").join("config"); + let dot_agents_config = cwd.path().join(".agents").join("config"); let dot_codex_config = cwd.path().join(".codex").join("config.toml"); let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], @@ -1712,33 +1888,53 @@ mod tests { FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, cwd.path()); assert!(!file_system_policy.can_write_path_with_cwd(&dot_git_config, cwd.path())); + assert!(!file_system_policy.can_write_path_with_cwd(&dot_agents_config, cwd.path())); assert!(!file_system_policy.can_write_path_with_cwd(&dot_codex_config, cwd.path())); } + #[test] + fn legacy_workspace_write_projection_blocks_missing_preserved_paths_under_extra_writable_root() + { + let cwd = TempDir::new().expect("tempdir"); + let extra = TempDir::new().expect("extra writable root"); + let extra_root = AbsolutePathBuf::from_absolute_path(extra.path()).expect("absolute extra"); + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![extra_root], + read_only_access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: vec![], + }, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + let file_system_policy = + FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, cwd.path()); + + assert!( + !file_system_policy + .can_write_path_with_cwd(extra.path().join(".git/config").as_path(), cwd.path()) + ); + assert!( + !file_system_policy + .can_write_path_with_cwd(extra.path().join(".agents/config").as_path(), cwd.path()) + ); + assert!(!file_system_policy.can_write_path_with_cwd( + extra.path().join(".codex/config.toml").as_path(), + cwd.path() + )); + } + #[test] fn legacy_workspace_write_projection_accepts_relative_cwd() { let relative_cwd = Path::new("workspace"); - let expected_dot_git = AbsolutePathBuf::from_absolute_path( + let expected_root = AbsolutePathBuf::from_absolute_path( std::env::current_dir() .expect("current dir") - .join(relative_cwd) - .join(".git"), + .join(relative_cwd), ) - .expect("absolute dot git"); - let expected_dot_agents = AbsolutePathBuf::from_absolute_path( - std::env::current_dir() - .expect("current dir") - .join(relative_cwd) - .join(".agents"), - ) - .expect("absolute dot agents"); - let expected_dot_codex = AbsolutePathBuf::from_absolute_path( - std::env::current_dir() - .expect("current dir") - .join(relative_cwd) - .join(".codex"), - ) - .expect("absolute dot codex"); + .expect("absolute root"); let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], network_access: false, @@ -1749,43 +1945,35 @@ mod tests { let file_system_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, relative_cwd); + let mut expected_entries = vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }]; + expected_entries.extend( + default_read_only_subpaths_for_writable_root( + &expected_root, + /*protect_missing_preserved_paths*/ true, + ) + .into_iter() + .map(|path| FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access: FileSystemAccessMode::Read, + }), + ); + assert_eq!( file_system_policy, - FileSystemSandboxPolicy::restricted(vec![ - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Root, - }, - access: FileSystemAccessMode::Read, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::CurrentWorkingDirectory, - }, - access: FileSystemAccessMode::Write, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Path { - path: expected_dot_git, - }, - access: FileSystemAccessMode::Read, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Path { - path: expected_dot_agents, - }, - access: FileSystemAccessMode::Read, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Path { - path: expected_dot_codex, - }, - access: FileSystemAccessMode::Read, - }, - ]) + FileSystemSandboxPolicy::restricted(expected_entries) ); - assert!( - !file_system_policy.can_write_path_with_cwd(Path::new(".git/config"), relative_cwd,) + assert_eq!( + forbidden_agent_preserved_path_write( + Path::new(".git/config"), + relative_cwd, + &file_system_policy, + ), + Some(".git") ); assert!( !file_system_policy @@ -1939,6 +2127,20 @@ mod tests { .join(".codex"), ) .expect("absolute .codex symlink"); + let expected_dot_git = AbsolutePathBuf::from_absolute_path( + root.as_path() + .canonicalize() + .expect("canonicalize root") + .join(".git"), + ) + .expect("absolute .git"); + let expected_dot_agents = AbsolutePathBuf::from_absolute_path( + root.as_path() + .canonicalize() + .expect("canonicalize root") + .join(".agents"), + ) + .expect("absolute .agents"); let unexpected_decoy = AbsolutePathBuf::from_absolute_path(decoy.canonicalize().expect("canonicalize decoy")) .expect("absolute canonical decoy"); @@ -1952,7 +2154,7 @@ mod tests { assert_eq!(writable_roots.len(), 1); assert_eq!( writable_roots[0].read_only_subpaths, - vec![expected_dot_codex] + vec![expected_dot_git, expected_dot_agents, expected_dot_codex] ); assert!( !writable_roots[0] @@ -1977,6 +2179,9 @@ mod tests { AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root"); let link_private = link_root.join("linked-private"); let expected_root = link_root.clone(); + let expected_dot_git = expected_root.join(".git"); + let expected_dot_agents = expected_root.join(".agents"); + let expected_dot_codex = expected_root.join(".codex"); let expected_linked_private = link_private.clone(); let unexpected_decoy = AbsolutePathBuf::from_absolute_path(decoy.canonicalize().expect("canonicalize decoy")) @@ -1998,7 +2203,12 @@ mod tests { assert_eq!(writable_roots[0].root, expected_root); assert_eq!( writable_roots[0].read_only_subpaths, - vec![expected_linked_private] + vec![ + expected_dot_git, + expected_dot_agents, + expected_dot_codex, + expected_linked_private + ] ); assert!( !writable_roots[0] @@ -2024,6 +2234,9 @@ mod tests { AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root"); let link_private = link_root.join("linked-private"); let expected_root = link_root.clone(); + let expected_dot_git = expected_root.join(".git"); + let expected_dot_agents = expected_root.join(".agents"); + let expected_dot_codex = expected_root.join(".codex"); let expected_linked_private = link_private.clone(); let unexpected_decoy = AbsolutePathBuf::from_absolute_path(decoy.canonicalize().expect("canonicalize decoy")) @@ -2045,7 +2258,12 @@ mod tests { assert_eq!(writable_roots[0].root, expected_root); assert_eq!( writable_roots[0].read_only_subpaths, - vec![expected_linked_private] + vec![ + expected_dot_git, + expected_dot_agents, + expected_dot_codex, + expected_linked_private + ] ); assert!( !writable_roots[0] @@ -2069,6 +2287,9 @@ mod tests { root.as_path().canonicalize().expect("canonicalize root"), ) .expect("absolute canonical root"); + let expected_dot_git = expected_root.join(".git"); + let expected_dot_agents = expected_root.join(".agents"); + let expected_dot_codex = expected_root.join(".codex"); let expected_alias = expected_root.join("alias-root"); let policy = FileSystemSandboxPolicy::restricted(vec![ @@ -2085,7 +2306,15 @@ mod tests { let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); assert_eq!(writable_roots.len(), 1); assert_eq!(writable_roots[0].root, expected_root); - assert_eq!(writable_roots[0].read_only_subpaths, vec![expected_alias]); + assert_eq!( + writable_roots[0].read_only_subpaths, + vec![ + expected_dot_git, + expected_dot_agents, + expected_dot_codex, + expected_alias + ] + ); } #[cfg(unix)] diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 1328e6cc1b..67bad250b9 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -5,7 +5,6 @@ use std::collections::HashMap; use std::collections::HashSet; -use std::ffi::OsStr; use std::fmt; use std::ops::Mul; use std::path::Path; @@ -84,6 +83,7 @@ pub use crate::permissions::FileSystemSandboxKind; pub use crate::permissions::FileSystemSandboxPolicy; pub use crate::permissions::FileSystemSpecialPath; pub use crate::permissions::NetworkSandboxPolicy; +use crate::permissions::default_read_only_subpaths_for_writable_root; pub use crate::request_permissions::RequestPermissionsArgs; pub use crate::request_user_input::RequestUserInputEvent; @@ -1249,17 +1249,13 @@ impl SandboxPolicy { } // For each root, compute subpaths that should remain read-only. - let cwd_root = AbsolutePathBuf::from_absolute_path(cwd).ok(); roots .into_iter() .map(|writable_root| { - let protect_missing_preserved_paths = cwd_root - .as_ref() - .is_some_and(|cwd_root| cwd_root == &writable_root); WritableRoot { read_only_subpaths: default_read_only_subpaths_for_writable_root( &writable_root, - protect_missing_preserved_paths, + /*protect_missing_preserved_paths*/ true, ), root: writable_root, } @@ -1270,107 +1266,6 @@ impl SandboxPolicy { } } -fn default_read_only_subpaths_for_writable_root( - writable_root: &AbsolutePathBuf, - protect_missing_preserved_paths: bool, -) -> Vec { - let mut subpaths: Vec = Vec::new(); - let top_level_git = writable_root.join(".git"); - // This applies to typical repos (directory .git), worktrees/submodules - // (file .git with gitdir pointer), and bare repos when the gitdir is the - // writable root itself. - let top_level_git_is_file = top_level_git.as_path().is_file(); - let top_level_git_is_dir = top_level_git.as_path().is_dir(); - if top_level_git_is_dir || top_level_git_is_file || protect_missing_preserved_paths { - if top_level_git_is_file - && is_git_pointer_file(&top_level_git) - && let Some(gitdir) = resolve_gitdir_from_file(&top_level_git) - { - subpaths.push(gitdir); - } - subpaths.push(top_level_git); - } - - let top_level_agents = writable_root.join(".agents"); - if protect_missing_preserved_paths || top_level_agents.as_path().is_dir() { - subpaths.push(top_level_agents); - } - - // Keep top-level preserved paths under .codex read-only to the agent by - // default. For the workspace root itself, protect it even before the - // directory exists so first-time creation still goes through the - // preserved path approval flow. - let top_level_codex = writable_root.join(".codex"); - if protect_missing_preserved_paths || top_level_codex.as_path().is_dir() { - subpaths.push(top_level_codex); - } - - let mut deduped = Vec::with_capacity(subpaths.len()); - let mut seen = HashSet::new(); - for path in subpaths { - if seen.insert(path.to_path_buf()) { - deduped.push(path); - } - } - deduped -} - -fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool { - path.as_path().is_file() && path.as_path().file_name() == Some(OsStr::new(".git")) -} - -fn resolve_gitdir_from_file(dot_git: &AbsolutePathBuf) -> Option { - let contents = match std::fs::read_to_string(dot_git.as_path()) { - Ok(contents) => contents, - Err(err) => { - error!( - "Failed to read {path} for gitdir pointer: {err}", - path = dot_git.as_path().display() - ); - return None; - } - }; - - let trimmed = contents.trim(); - let (_, gitdir_raw) = match trimmed.split_once(':') { - Some(parts) => parts, - None => { - error!( - "Expected {path} to contain a gitdir pointer, but it did not match `gitdir: `.", - path = dot_git.as_path().display() - ); - return None; - } - }; - let gitdir_raw = gitdir_raw.trim(); - if gitdir_raw.is_empty() { - error!( - "Expected {path} to contain a gitdir pointer, but it was empty.", - path = dot_git.as_path().display() - ); - return None; - } - let base = match dot_git.as_path().parent() { - Some(base) => base, - None => { - error!( - "Unable to resolve parent directory for {path}.", - path = dot_git.as_path().display() - ); - return None; - } - }; - let gitdir_path = AbsolutePathBuf::resolve_path_against_base(gitdir_raw, base); - if !gitdir_path.as_path().exists() { - error!( - "Resolved gitdir path {path} does not exist.", - path = gitdir_path.as_path().display() - ); - return None; - } - Some(gitdir_path) -} - /// Event Queue Entry - events from agent #[derive(Debug, Clone, Deserialize, Serialize)] pub struct Event { @@ -4453,6 +4348,15 @@ mod tests { let expected_docs_public = AbsolutePathBuf::from_absolute_path(canonical_cwd.join("docs/public")) .expect("canonical docs/public"); + let expected_docs_public_dot_agents = + AbsolutePathBuf::from_absolute_path(canonical_cwd.join("docs/public/.agents")) + .expect("canonical docs/public/.agents"); + let expected_docs_public_dot_codex = + AbsolutePathBuf::from_absolute_path(canonical_cwd.join("docs/public/.codex")) + .expect("canonical docs/public/.codex"); + let expected_docs_public_dot_git = + AbsolutePathBuf::from_absolute_path(canonical_cwd.join("docs/public/.git")) + .expect("canonical docs/public/.git"); let expected_dot_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex")) .expect("canonical .codex"); let expected_dot_git = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".git")) @@ -4490,7 +4394,92 @@ mod tests { expected_docs.to_path_buf() ], ), - (expected_docs_public.to_path_buf(), Vec::new()), + ( + expected_docs_public.to_path_buf(), + vec![ + expected_docs_public_dot_agents.to_path_buf(), + expected_docs_public_dot_codex.to_path_buf(), + expected_docs_public_dot_git.to_path_buf(), + ], + ), + ] + ); + } + + #[test] + fn workspace_write_keeps_missing_git_absent_under_parent_repo() { + let repo = TempDir::new().expect("tempdir"); + std::fs::create_dir(repo.path().join(".git")).expect("create parent .git"); + let cwd = repo.path().join("sub"); + std::fs::create_dir(&cwd).expect("create subdir"); + let expected_root = AbsolutePathBuf::from_absolute_path(&cwd).expect("absolute cwd"); + let expected_dot_codex = + AbsolutePathBuf::from_absolute_path(cwd.join(".codex")).expect("canonical .codex"); + let expected_dot_agents = + AbsolutePathBuf::from_absolute_path(cwd.join(".agents")).expect("canonical .agents"); + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: vec![], + }, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + assert_eq!( + sorted_writable_roots(policy.get_writable_roots_with_cwd(&cwd)), + vec![( + expected_root.to_path_buf(), + vec![ + expected_dot_agents.to_path_buf(), + expected_dot_codex.to_path_buf() + ], + )] + ); + } + + #[test] + fn workspace_write_reserves_missing_preserved_paths_under_configured_writable_roots() { + let root = TempDir::new().expect("tempdir"); + let cwd = root.path().join("cwd"); + let extra = root.path().join("extra"); + std::fs::create_dir_all(&cwd).expect("create cwd"); + std::fs::create_dir_all(&extra).expect("create extra writable root"); + let expected_cwd = AbsolutePathBuf::from_absolute_path(&cwd).expect("absolute cwd"); + let expected_extra = + AbsolutePathBuf::from_absolute_path(&extra).expect("absolute extra root"); + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![expected_extra.clone()], + read_only_access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: vec![], + }, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + assert_eq!( + sorted_writable_roots(policy.get_writable_roots_with_cwd(&cwd)), + vec![ + ( + expected_cwd.to_path_buf(), + vec![ + expected_cwd.join(".agents").to_path_buf(), + expected_cwd.join(".codex").to_path_buf(), + expected_cwd.join(".git").to_path_buf() + ], + ), + ( + expected_extra.to_path_buf(), + vec![ + expected_extra.join(".agents").to_path_buf(), + expected_extra.join(".codex").to_path_buf(), + expected_extra.join(".git").to_path_buf() + ], + ), ] ); } diff --git a/codex-rs/sandboxing/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs index b51cbcf81c..cf50baa2bd 100644 --- a/codex-rs/sandboxing/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -933,12 +933,28 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() { ), format!( "-DWRITABLE_ROOT_1_EXCLUDED_1={}", + vulnerable_root_canonical.join(".agents").to_string_lossy() + ), + format!( + "-DWRITABLE_ROOT_1_EXCLUDED_2={}", dot_codex_canonical.to_string_lossy() ), format!( "-DWRITABLE_ROOT_2={}", empty_root_canonical.to_string_lossy() ), + format!( + "-DWRITABLE_ROOT_2_EXCLUDED_0={}", + empty_root_canonical.join(".git").to_string_lossy() + ), + format!( + "-DWRITABLE_ROOT_2_EXCLUDED_1={}", + empty_root_canonical.join(".agents").to_string_lossy() + ), + format!( + "-DWRITABLE_ROOT_2_EXCLUDED_2={}", + empty_root_canonical.join(".codex").to_string_lossy() + ), ]; let writable_definitions: Vec = args .iter() @@ -1264,7 +1280,7 @@ fn create_seatbelt_args_for_cwd_as_git_repo() { .map(|p| p.to_string_lossy().to_string()); let tempdir_policy_entry = if tmpdir_env_var.is_some() { - r#" (require-all (subpath (param "WRITABLE_ROOT_2")) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_2"))) )"# + r#" (require-all (subpath (param "WRITABLE_ROOT_2")) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_2"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_3"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_3"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_4"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_4"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_5"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_5"))) )"# } else { "" }; @@ -1273,13 +1289,14 @@ fn create_seatbelt_args_for_cwd_as_git_repo() { // Note that the policy includes: // - the base policy, // - read-only access to the filesystem, - // - write access to WRITABLE_ROOT_0 (but not its preserved paths), WRITABLE_ROOT_1, and cwd as WRITABLE_ROOT_2. + // - write access to WRITABLE_ROOT_0, WRITABLE_ROOT_1, and cwd as + // WRITABLE_ROOT_2, each with preserved path carveouts. let expected_policy = format!( r#"{MACOS_SEATBELT_BASE_POLICY} ; allow read-only file operations (allow file-read*) (allow file-write* -(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_2"))) ) (subpath (param "WRITABLE_ROOT_1")){tempdir_policy_entry} +(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_2"))) ) (require-all (subpath (param "WRITABLE_ROOT_1")) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_2"))) ){tempdir_policy_entry} ) "#, @@ -1311,20 +1328,47 @@ fn create_seatbelt_args_for_cwd_as_git_repo() { .expect("canonicalize /tmp") .to_string_lossy() ), + format!( + "-DWRITABLE_ROOT_1_EXCLUDED_0={}", + PathBuf::from("/tmp") + .canonicalize() + .expect("canonicalize /tmp") + .join(".git") + .to_string_lossy() + ), + format!( + "-DWRITABLE_ROOT_1_EXCLUDED_1={}", + PathBuf::from("/tmp") + .canonicalize() + .expect("canonicalize /tmp") + .join(".agents") + .to_string_lossy() + ), + format!( + "-DWRITABLE_ROOT_1_EXCLUDED_2={}", + PathBuf::from("/tmp") + .canonicalize() + .expect("canonicalize /tmp") + .join(".codex") + .to_string_lossy() + ), ]; if let Some(p) = tmpdir_env_var { expected_args.push(format!("-DWRITABLE_ROOT_2={p}")); + expected_args.push(format!("-DWRITABLE_ROOT_2_EXCLUDED_0={p}/.git")); + expected_args.push(format!("-DWRITABLE_ROOT_2_EXCLUDED_1={p}/.agents")); + expected_args.push(format!("-DWRITABLE_ROOT_2_EXCLUDED_2={p}/.codex")); expected_args.push(format!( - "-DWRITABLE_ROOT_2_EXCLUDED_0={}", + "-DWRITABLE_ROOT_2_EXCLUDED_3={}", dot_git_canonical.to_string_lossy() )); expected_args.push(format!( - "-DWRITABLE_ROOT_2_EXCLUDED_1={}", + "-DWRITABLE_ROOT_2_EXCLUDED_4={}", dot_agents_canonical.to_string_lossy() )); expected_args.push(format!( - "-DWRITABLE_ROOT_2_EXCLUDED_2={}", + "-DWRITABLE_ROOT_2_EXCLUDED_5={}", dot_codex_canonical.to_string_lossy() )); } diff --git a/codex-rs/shell-command/src/bash.rs b/codex-rs/shell-command/src/bash.rs index 2fe299108c..9a10ba458b 100644 --- a/codex-rs/shell-command/src/bash.rs +++ b/codex-rs/shell-command/src/bash.rs @@ -119,6 +119,55 @@ pub fn parse_shell_lc_plain_commands(command: &[String]) -> Option Option>> { + let (_, script) = extract_bash_command(command)?; + let tree = try_parse_shell(script)?; + let root = tree.root_node(); + if root.has_error() { + return None; + } + + let mut commands = Vec::new(); + for command_node in find_command_nodes(root) { + if let Some(words) = parse_command_word_prefix_from_node(command_node, script) + && !words.is_empty() + { + commands.push(words); + } + } + + (!commands.is_empty()).then_some(commands) +} + +/// Returns literal write redirection targets within a `bash -lc "..."` or +/// `zsh -lc "..."` invocation. +pub fn parse_shell_lc_write_redirection_targets(command: &[String]) -> Option> { + let (_, script) = extract_bash_command(command)?; + let tree = try_parse_shell(script)?; + let root = tree.root_node(); + if root.has_error() { + return None; + } + + let mut targets = Vec::new(); + for redirect_node in find_nodes_by_kind(root, "file_redirect") { + if file_redirect_uses_write_operator(redirect_node) + && let Some(target) = parse_redirection_target(redirect_node, script) + { + targets.push(target); + } + } + + (!targets.is_empty()).then_some(targets) +} + /// Returns the parsed argv for a single shell command in a here-doc style /// script (`<<`), as long as the script contains exactly one command node. pub fn parse_shell_lc_single_command_prefix(command: &[String]) -> Option> { @@ -194,6 +243,100 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option, src: &str) -> Option> { + if cmd.kind() != "command" { + return None; + } + let mut words = Vec::new(); + let mut cursor = cmd.walk(); + for child in cmd.named_children(&mut cursor) { + match child.kind() { + "command_name" => { + let word_node = child.named_child(0)?; + if !matches!(word_node.kind(), "word" | "number") { + return None; + } + words.push(word_node.utf8_text(src.as_bytes()).ok()?.to_owned()); + } + "word" | "number" => { + words.push(child.utf8_text(src.as_bytes()).ok()?.to_owned()); + } + "string" => { + let parsed = parse_double_quoted_string(child, src)?; + words.push(parsed); + } + "raw_string" => { + let parsed = parse_raw_string(child, src)?; + words.push(parsed); + } + "concatenation" => { + let parsed = parse_concatenation(child, src)?; + words.push(parsed); + } + "variable_assignment" | "comment" => {} + kind if is_allowed_heredoc_attachment_kind(kind) => {} + _ => {} + } + } + Some(words) +} + +fn file_redirect_uses_write_operator(node: Node<'_>) -> bool { + let mut cursor = node.walk(); + for child in node.children(&mut cursor) { + if !child.is_named() && child.kind().contains('>') { + return true; + } + } + false +} + +fn parse_redirection_target(node: Node<'_>, src: &str) -> Option { + let mut target = None; + let mut cursor = node.walk(); + for child in node.named_children(&mut cursor) { + if let Some(parsed) = parse_literal_shell_word(child, src) { + target = Some(parsed); + } + } + target +} + +fn parse_literal_shell_word(node: Node<'_>, src: &str) -> Option { + match node.kind() { + "word" | "number" => Some(node.utf8_text(src.as_bytes()).ok()?.to_owned()), + "string" => parse_double_quoted_string(node, src), + "raw_string" => parse_raw_string(node, src), + "concatenation" => parse_concatenation(node, src), + _ => None, + } +} + +fn parse_concatenation(node: Node<'_>, src: &str) -> Option { + let mut concatenated = String::new(); + let mut cursor = node.walk(); + for part in node.named_children(&mut cursor) { + match part.kind() { + "word" | "number" => { + concatenated.push_str(part.utf8_text(src.as_bytes()).ok()?.to_owned().as_str()); + } + "string" => { + let parsed = parse_double_quoted_string(part, src)?; + concatenated.push_str(&parsed); + } + "raw_string" => { + let parsed = parse_raw_string(part, src)?; + concatenated.push_str(&parsed); + } + _ => return None, + } + } + if concatenated.is_empty() { + return None; + } + Some(concatenated) +} + fn parse_heredoc_command_words(cmd: Node<'_>, src: &str) -> Option> { if cmd.kind() != "command" { return None; @@ -268,6 +411,29 @@ fn find_single_command_node(root: Node<'_>) -> Option> { single_command } +fn find_command_nodes(root: Node<'_>) -> Vec> { + let mut command_nodes = find_nodes_by_kind(root, "command"); + command_nodes.sort_by_key(Node::start_byte); + command_nodes +} + +fn find_nodes_by_kind<'a>(root: Node<'a>, kind: &str) -> Vec> { + let mut stack = vec![root]; + let mut matches = Vec::new(); + while let Some(node) = stack.pop() { + if node.kind() == kind { + matches.push(node); + } + + let mut cursor = node.walk(); + for child in node.named_children(&mut cursor) { + stack.push(child); + } + } + matches.sort_by_key(Node::start_byte); + matches +} + fn has_named_descendant_kind(node: Node<'_>, kind: &str) -> bool { let mut stack = vec![node]; while let Some(current) = stack.pop() { @@ -453,6 +619,64 @@ mod tests { assert_eq!(parsed, vec![vec!["ls".to_string()]]); } + #[test] + fn parse_shell_lc_command_word_prefixes_extracts_complex_script_commands() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + r#"set -e +top=$(git rev-parse --show-toplevel) +if git init -q; then + exit 22 +fi +if mkdir .codex; then + exit 23 +fi +printf pwned > .git/config +"# + .to_string(), + ]; + + let parsed = parse_shell_lc_command_word_prefixes(&command).expect("parse script"); + + assert!(parsed.contains(&vec![ + "git".to_string(), + "rev-parse".to_string(), + "--show-toplevel".to_string() + ])); + assert!(parsed.contains(&vec![ + "git".to_string(), + "init".to_string(), + "-q".to_string() + ])); + assert!(parsed.contains(&vec!["mkdir".to_string(), ".codex".to_string()])); + } + + #[test] + fn parse_shell_lc_write_redirection_targets_extracts_literal_writes() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + r#"printf pwned > .git +cat < .git/config +printf ok >> .codex/log +printf ok > ".agents/config" +"# + .to_string(), + ]; + + let parsed = parse_shell_lc_write_redirection_targets(&command).expect("parse redirects"); + + assert_eq!( + parsed, + vec![ + ".git".to_string(), + ".codex/log".to_string(), + ".agents/config".to_string() + ] + ); + } + #[test] fn accepts_concatenated_flag_and_value() { // Test case: -g"*.py" (flag directly concatenated with quoted value)