diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 13e60ecb71..4b1024d683 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -1181,6 +1181,52 @@ exclude_slash_tmp = true Ok(()) } +#[test] +fn missing_project_dot_codex_warning_condition_matches_workspace_policy() -> std::io::Result<()> { + let cwd = TempDir::new()?; + let workspace_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy( + &SandboxPolicy::new_workspace_write_policy(), + cwd.path(), + ); + + assert!(should_warn_missing_project_dot_codex_for_linux_bwrap( + &workspace_policy, + cwd.path() + )); + + let read_only_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy( + &SandboxPolicy::new_read_only_policy(), + cwd.path(), + ); + assert!(!should_warn_missing_project_dot_codex_for_linux_bwrap( + &read_only_policy, + cwd.path() + )); + + std::fs::create_dir(cwd.path().join(".codex"))?; + assert!(!should_warn_missing_project_dot_codex_for_linux_bwrap( + &workspace_policy, + cwd.path() + )); + + #[cfg(unix)] + { + let cwd = TempDir::new()?; + let workspace_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy( + &SandboxPolicy::new_workspace_write_policy(), + cwd.path(), + ); + std::os::unix::fs::symlink(cwd.path().join("missing-target"), cwd.path().join(".codex"))?; + + assert!(!should_warn_missing_project_dot_codex_for_linux_bwrap( + &workspace_policy, + cwd.path() + )); + } + + Ok(()) +} + #[test] fn filter_mcp_servers_by_allowlist_enforces_identity_rules() { const MISMATCHED_COMMAND_SERVER: &str = "mismatched-command-should-disable"; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index d83fc87371..20eb99570a 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -148,6 +148,7 @@ pub(crate) const DEFAULT_AGENT_MAX_DEPTH: i32 = 1; pub(crate) const DEFAULT_AGENT_JOB_MAX_RUNTIME_SECONDS: Option = None; pub const CONFIG_TOML_FILE: &str = "config.toml"; +const PROJECT_LOCAL_CODEX_DIR: &str = ".codex"; const RESERVED_MODEL_PROVIDER_IDS: [&str; 3] = [ OPENAI_PROVIDER_ID, OLLAMA_OSS_PROVIDER_ID, @@ -168,6 +169,42 @@ fn resolve_sqlite_home_env(resolved_cwd: &Path) -> Option { } } +fn maybe_warn_missing_project_dot_codex_for_linux_bwrap( + startup_warnings: &mut Vec, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + cwd: &Path, +) { + if !cfg!(target_os = "linux") + || !should_warn_missing_project_dot_codex_for_linux_bwrap(file_system_sandbox_policy, cwd) + { + return; + } + + let dot_codex = cwd.join(PROJECT_LOCAL_CODEX_DIR); + let dot_codex_display = dot_codex.display(); + let warning = format!( + "Project-local `.codex` does not exist at `{dot_codex_display}`. On Linux, Codex will not create a sandbox mount target for it, so shell commands may create `.codex` without approval until it exists; existing `.codex` paths and `apply_patch` edits remain protected." + ); + tracing::warn!("{warning}"); + startup_warnings.push(warning); +} + +fn should_warn_missing_project_dot_codex_for_linux_bwrap( + file_system_sandbox_policy: &FileSystemSandboxPolicy, + cwd: &Path, +) -> bool { + let dot_codex = cwd.join(PROJECT_LOCAL_CODEX_DIR); + let dot_codex_is_missing = match dot_codex.symlink_metadata() { + Ok(_) => false, + Err(err) if err.kind() == ErrorKind::NotFound => true, + Err(_) => false, + }; + + dot_codex_is_missing + && file_system_sandbox_policy.can_write_path_with_cwd(cwd, cwd) + && !file_system_sandbox_policy.can_write_path_with_cwd(&dot_codex, cwd) +} + #[cfg(test)] pub(crate) fn test_config() -> Config { let codex_home = tempfile::tempdir().expect("create temp dir"); @@ -2642,6 +2679,11 @@ impl Config { } else { NetworkSandboxPolicy::from(&effective_sandbox_policy) }; + maybe_warn_missing_project_dot_codex_for_linux_bwrap( + &mut startup_warnings, + &effective_file_system_sandbox_policy, + resolved_cwd.as_path(), + ); let config = Self { model, service_tier, diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index 70f5197e82..4c4b0a6f17 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -313,15 +313,10 @@ async fn sandbox_distinguishes_command_and_policy_cwds() { assert!(allowed_exists, "allowed path should exist"); } +#[cfg(target_os = "macos")] #[tokio::test] async fn sandbox_blocks_first_time_dot_codex_creation() { core_test_support::skip_if_sandbox!(); - #[cfg(target_os = "linux")] - let sandbox_env = match linux_sandbox_test_env().await { - Some(env) => env, - None => return, - }; - #[cfg(not(target_os = "linux"))] let sandbox_env = HashMap::new(); let temp = tempfile::tempdir().expect("should be able to create temp dir"); diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 0709324f64..28d368b570 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -344,7 +344,12 @@ fn create_filesystem_args( .collect(); 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 args, + &mut preserved_files, + &subpath, + &allowed_write_paths, + )?; } let mut nested_unreadable_roots: Vec = unreadable_roots .iter() @@ -424,25 +429,20 @@ 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, subpath: &Path, allowed_write_paths: &[PathBuf], -) { +) -> Result<()> { if let Some(symlink_path) = find_symlink_in_path(subpath, allowed_write_paths) { - args.push("--ro-bind".to_string()); - args.push("/dev/null".to_string()); - args.push(path_to_string(&symlink_path)); - return; + append_empty_file_read_only_bind_args(args, preserved_files, &symlink_path)?; + 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)); - } - return; + // Bubblewrap must create bind targets before mounting over them. If the + // missing path lives under a writable host bind, that target creation + // leaks a real host-side placeholder or fails before command startup. + return Ok(()); } if is_within_allowed_write_paths(subpath, allowed_write_paths) { @@ -450,6 +450,8 @@ fn append_read_only_subpath_args( args.push(path_to_string(subpath)); args.push(path_to_string(subpath)); } + + Ok(()) } fn append_unreadable_root_args( @@ -459,9 +461,7 @@ fn append_unreadable_root_args( allowed_write_paths: &[PathBuf], ) -> Result<()> { if let Some(symlink_path) = find_symlink_in_path(unreadable_root, allowed_write_paths) { - args.push("--ro-bind".to_string()); - args.push("/dev/null".to_string()); - args.push(path_to_string(&symlink_path)); + append_empty_file_read_only_bind_args(args, preserved_files, &symlink_path)?; return Ok(()); } @@ -469,9 +469,7 @@ 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_empty_file_read_only_bind_args(args, preserved_files, &first_missing_component)?; } return Ok(()); } @@ -506,15 +504,24 @@ fn append_unreadable_root_args( return Ok(()); } + args.push("--perms".to_string()); + args.push("000".to_string()); + append_empty_file_read_only_bind_args(args, preserved_files, unreadable_root)?; + Ok(()) +} + +fn append_empty_file_read_only_bind_args( + args: &mut Vec, + preserved_files: &mut Vec, + mount_target: &Path, +) -> Result<()> { 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)); + args.push(path_to_string(mount_target)); Ok(()) } @@ -529,7 +536,7 @@ fn is_within_allowed_write_paths(path: &Path, allowed_write_paths: &[PathBuf]) - /// /// This blocks symlink replacement attacks where a protected path is a symlink /// inside a writable root (e.g., `.codex -> ./decoy`). In that case we mount -/// `/dev/null` on the symlink itself to prevent rewiring it. +/// an inherited empty file on the symlink itself to prevent rewiring it. fn find_symlink_in_path(target_path: &Path, allowed_write_paths: &[PathBuf]) -> Option { let mut current = PathBuf::new(); @@ -566,8 +573,8 @@ fn find_symlink_in_path(target_path: &Path, allowed_write_paths: &[PathBuf]) -> /// Find the first missing path component while walking `target_path`. /// -/// Mounting `/dev/null` on the first missing component prevents the sandboxed -/// process from creating the protected path hierarchy. +/// Mounting an inherited empty file on the first missing component prevents the +/// sandboxed process from creating the protected path hierarchy. fn find_first_non_existent_component(target_path: &Path) -> Option { let mut current = PathBuf::new(); @@ -767,33 +774,35 @@ mod tests { Path::new("/"), ) .expect("bwrap fs args"); + assert_eq!(args.preserved_files.len(), 0); assert_eq!( - args.args, - vec![ + &args.args[..8], + [ // Start from a read-only view of the full filesystem. - "--ro-bind".to_string(), - "/".to_string(), - "/".to_string(), + "--ro-bind", + "/", + "/", // Recreate a writable /dev inside the sandbox. - "--dev".to_string(), - "/dev".to_string(), + "--dev", + "/dev", // Make the writable root itself writable again. - "--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 as `/.codex`. - "--ro-bind".to_string(), - "/dev/null".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(), + "--bind", + "/", + "/", ] ); + assert!( + !args.args.iter().any(|arg| arg == "/.codex"), + "missing protected .codex should not be masked under bwrap: {:#?}", + args.args + ); + assert!( + args.args + .windows(3) + .any(|window| window == ["--bind", "/dev", "/dev"]), + "expected /dev to be rebound after the writable root: {:#?}", + args.args + ); } #[test] diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index ffe0d9e57d..df5f64387f 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -21,6 +21,7 @@ use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::collections::HashMap; +use std::path::Path; use std::path::PathBuf; use tempfile::NamedTempFile; @@ -81,6 +82,26 @@ async fn run_cmd_result_with_writable_roots( timeout_ms: u64, use_legacy_landlock: bool, network_access: bool, +) -> Result { + let cwd = std::env::current_dir().expect("cwd should exist"); + run_cmd_result_with_writable_roots_in_cwd( + cmd, + writable_roots, + cwd.as_path(), + timeout_ms, + use_legacy_landlock, + network_access, + ) + .await +} + +async fn run_cmd_result_with_writable_roots_in_cwd( + cmd: &[&str], + writable_roots: &[PathBuf], + cwd: &Path, + timeout_ms: u64, + use_legacy_landlock: bool, + network_access: bool, ) -> Result { let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: writable_roots @@ -97,11 +118,12 @@ async fn run_cmd_result_with_writable_roots( }; let file_system_sandbox_policy = FileSystemSandboxPolicy::from(&sandbox_policy); let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); - run_cmd_result_with_policies( + run_cmd_result_with_policies_in_cwd( cmd, sandbox_policy, file_system_sandbox_policy, network_sandbox_policy, + cwd, timeout_ms, use_legacy_landlock, ) @@ -118,6 +140,29 @@ async fn run_cmd_result_with_policies( use_legacy_landlock: bool, ) -> Result { let cwd = std::env::current_dir().expect("cwd should exist"); + run_cmd_result_with_policies_in_cwd( + cmd, + sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, + cwd.as_path(), + timeout_ms, + use_legacy_landlock, + ) + .await +} + +#[expect(clippy::expect_used)] +async fn run_cmd_result_with_policies_in_cwd( + cmd: &[&str], + sandbox_policy: SandboxPolicy, + file_system_sandbox_policy: FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + cwd: &Path, + timeout_ms: u64, + use_legacy_landlock: bool, +) -> Result { + let cwd = cwd.to_path_buf(); let sandbox_cwd = cwd.clone(); let params = ExecParams { command: cmd.iter().copied().map(str::to_owned).collect(), @@ -258,6 +303,40 @@ async fn bwrap_populates_minimal_dev_nodes() { assert_eq!(output.exit_code, 0); } +#[tokio::test] +async fn bwrap_dev_nodes_work_when_workspace_dot_codex_is_missing() { + if should_skip_bwrap_tests().await { + eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable"); + return; + } + + let tmpdir = tempfile::tempdir().expect("tempdir"); + let output = run_cmd_result_with_writable_roots_in_cwd( + &[ + "bash", + "-lc", + ": >/dev/null && head -c 8 /dev/zero | od -An -tx1", + ], + &[], + tmpdir.path(), + LONG_TIMEOUT_MS, + false, + true, + ) + .await + .expect("sandboxed command should execute"); + + assert_eq!(output.exit_code, 0); + assert_eq!( + output.stdout.text.split_whitespace().collect::>(), + vec!["00", "00", "00", "00", "00", "00", "00", "00"] + ); + assert!( + !tmpdir.path().join(".codex").exists(), + "missing workspace .codex should not be materialized by bwrap startup" + ); +} + #[tokio::test] async fn bwrap_preserves_writable_dev_shm_bind_mount() { if should_skip_bwrap_tests().await {