diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index f4664c16a..8e8825e9f 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -581,6 +581,17 @@ jobs: tool: nextest version: 0.9.103 + - name: Enable unprivileged user namespaces (Linux) + if: runner.os == 'Linux' + run: | + # Required for bubblewrap to work on Linux CI runners. + sudo sysctl -w kernel.unprivileged_userns_clone=1 + # Ubuntu 24.04+ can additionally gate unprivileged user namespaces + # behind AppArmor. + if sudo sysctl -a 2>/dev/null | grep -q '^kernel.apparmor_restrict_unprivileged_userns'; then + sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 + fi + - name: tests id: test run: cargo nextest run --all-features --no-fail-fast --target ${{ matrix.target }} --cargo-profile ci-test --timings diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 7ff7acf5e..71704e729 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -135,11 +135,12 @@ fn create_bwrap_flags( /// /// The mount order is important: /// 1. `--ro-bind / /` makes the entire filesystem read-only. -/// 2. `--bind ` re-enables writes for allowed roots. -/// 3. `--ro-bind ` re-applies read-only protections under +/// 2. `--dev /dev` mounts a minimal writable `/dev` with standard device nodes +/// (including `/dev/urandom`) even under a read-only root. +/// 3. `--bind ` re-enables writes for allowed roots, including +/// writable subpaths under `/dev` (for example, `/dev/shm`). +/// 4. `--ro-bind ` re-applies read-only protections under /// those writable roots so protected subpaths win. -/// 4. `--dev-bind /dev/null /dev/null` preserves the common sink even under a -/// read-only root. fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result> { if !sandbox_policy.has_full_disk_read_access() { return Err(CodexErr::UnsupportedOperation( @@ -151,12 +152,18 @@ fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result< let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); ensure_mount_targets_exist(&writable_roots)?; - let mut args = Vec::new(); - - // Read-only root, then selectively re-enable writes. - args.push("--ro-bind".to_string()); - args.push("/".to_string()); - args.push("/".to_string()); + // 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, urandom, and tty. + // `/dev` must be mounted before writable roots so explicit `/dev/*` + // writable binds remain visible. + let mut args = vec![ + "--ro-bind".to_string(), + "/".to_string(), + "/".to_string(), + "--dev".to_string(), + "/dev".to_string(), + ]; for writable_root in &writable_roots { let root = writable_root.root.as_path(); @@ -180,12 +187,15 @@ fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result< } if !subpath.exists() { - if let Some(first_missing) = find_first_non_existent_component(&subpath) - && is_within_allowed_write_paths(&first_missing, &allowed_write_paths) + // Keep this in the per-subpath loop: each protected subpath can have + // a different first missing component that must be blocked + // independently (for example, `/repo/.git` vs `/repo/.codex`). + 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)); + args.push(path_to_string(&first_missing_component)); } continue; } @@ -197,11 +207,6 @@ fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result< } } - // Ensure `/dev/null` remains usable regardless of the root bind. - args.push("--dev-bind".to_string()); - args.push("/dev/null".to_string()); - args.push("/dev/null".to_string()); - Ok(args) } diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 1df9a5443..c2a6a4d31 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -307,7 +307,9 @@ fn close_fd_or_panic(fd: libc::c_int, context: &str) { fn is_proc_mount_failure(stderr: &str) -> bool { stderr.contains("Can't mount proc") && stderr.contains("/newroot/proc") - && stderr.contains("Invalid argument") + && (stderr.contains("Invalid argument") + || stderr.contains("Operation not permitted") + || stderr.contains("Permission denied")) } /// Build the inner command that applies seccomp after bubblewrap. @@ -381,6 +383,18 @@ mod tests { assert_eq!(is_proc_mount_failure(stderr), true); } + #[test] + fn detects_proc_mount_operation_not_permitted_failure() { + let stderr = "bwrap: Can't mount proc on /newroot/proc: Operation not permitted"; + assert_eq!(is_proc_mount_failure(stderr), true); + } + + #[test] + fn detects_proc_mount_permission_denied_failure() { + let stderr = "bwrap: Can't mount proc on /newroot/proc: Permission denied"; + assert_eq!(is_proc_mount_failure(stderr), true); + } + #[test] fn ignores_non_proc_mount_errors() { let stderr = "bwrap: Can't bind mount /dev/null: Operation not permitted"; @@ -407,9 +421,8 @@ mod tests { "--ro-bind".to_string(), "/".to_string(), "/".to_string(), - "--dev-bind".to_string(), - "/dev/null".to_string(), - "/dev/null".to_string(), + "--dev".to_string(), + "/dev".to_string(), "--unshare-pid".to_string(), "--proc".to_string(), "/proc".to_string(), diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index fa26a8b96..668c65e9c 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -56,7 +56,7 @@ async fn run_cmd_output( writable_roots: &[PathBuf], timeout_ms: u64, ) -> codex_core::exec::ExecToolCallOutput { - run_cmd_result_with_writable_roots(cmd, writable_roots, timeout_ms, false) + run_cmd_result_with_writable_roots(cmd, writable_roots, timeout_ms, false, false) .await .expect("sandboxed command should execute") } @@ -67,6 +67,7 @@ async fn run_cmd_result_with_writable_roots( writable_roots: &[PathBuf], timeout_ms: u64, use_bwrap_sandbox: bool, + network_access: bool, ) -> Result { let cwd = std::env::current_dir().expect("cwd should exist"); let sandbox_cwd = cwd.clone(); @@ -89,7 +90,7 @@ async fn run_cmd_result_with_writable_roots( .map(|p| AbsolutePathBuf::try_from(p.as_path()).unwrap()) .collect(), read_only_access: Default::default(), - network_access: false, + network_access, // Exclude tmp-related folders from writable roots because we need a // folder that is writable by tests but that we intentionally disallow // writing to in the sandbox. @@ -112,6 +113,13 @@ async fn run_cmd_result_with_writable_roots( fn is_bwrap_unavailable_output(output: &codex_core::exec::ExecToolCallOutput) -> bool { output.stderr.text.contains(BWRAP_UNAVAILABLE_ERR) + || (output + .stderr + .text + .contains("Can't mount proc on /newroot/proc") + && (output.stderr.text.contains("Operation not permitted") + || output.stderr.text.contains("Permission denied") + || output.stderr.text.contains("Invalid argument"))) } async fn should_skip_bwrap_tests() -> bool { @@ -120,6 +128,7 @@ async fn should_skip_bwrap_tests() -> bool { &[], NETWORK_TIMEOUT_MS, true, + true, ) .await { @@ -168,14 +177,90 @@ async fn test_root_write() { #[tokio::test] async fn test_dev_null_write() { - run_cmd( + if should_skip_bwrap_tests().await { + eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable"); + return; + } + + let output = run_cmd_result_with_writable_roots( &["bash", "-lc", "echo blah > /dev/null"], &[], // We have seen timeouts when running this test in CI on GitHub, // so we are using a generous timeout until we can diagnose further. LONG_TIMEOUT_MS, + true, + true, ) - .await; + .await + .expect("sandboxed command should execute"); + + assert_eq!(output.exit_code, 0); +} + +#[tokio::test] +async fn bwrap_populates_minimal_dev_nodes() { + if should_skip_bwrap_tests().await { + eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable"); + return; + } + + let output = run_cmd_result_with_writable_roots( + &[ + "bash", + "-lc", + "for node in null zero full random urandom tty; do [ -c \"/dev/$node\" ] || { echo \"missing /dev/$node\" >&2; exit 1; }; done", + ], + &[], + LONG_TIMEOUT_MS, + true, + true, + ) + .await + .expect("sandboxed command should execute"); + + assert_eq!(output.exit_code, 0); +} + +#[tokio::test] +async fn bwrap_preserves_writable_dev_shm_bind_mount() { + if should_skip_bwrap_tests().await { + eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable"); + return; + } + if !std::path::Path::new("/dev/shm").exists() { + eprintln!("skipping bwrap test: /dev/shm is unavailable in this environment"); + return; + } + + let target_file = match NamedTempFile::new_in("/dev/shm") { + Ok(file) => file, + Err(err) => { + eprintln!("skipping bwrap test: failed to create /dev/shm temp file: {err}"); + return; + } + }; + let target_path = target_file.path().to_path_buf(); + std::fs::write(&target_path, "host-before").expect("seed /dev/shm file"); + + let output = run_cmd_result_with_writable_roots( + &[ + "bash", + "-lc", + &format!("printf sandbox-after > {}", target_path.to_string_lossy()), + ], + &[PathBuf::from("/dev/shm")], + LONG_TIMEOUT_MS, + true, + true, + ) + .await + .expect("sandboxed command should execute"); + + assert_eq!(output.exit_code, 0); + assert_eq!( + std::fs::read_to_string(&target_path).expect("read /dev/shm file"), + "sandbox-after" + ); } #[tokio::test] @@ -306,7 +391,7 @@ async fn sandbox_blocks_nc() { #[tokio::test] async fn sandbox_blocks_git_and_codex_writes_inside_writable_root() { if should_skip_bwrap_tests().await { - eprintln!("skipping bwrap test: vendored bwrap was not built in this environment"); + eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable"); return; } @@ -329,6 +414,7 @@ async fn sandbox_blocks_git_and_codex_writes_inside_writable_root() { &[tmpdir.path().to_path_buf()], LONG_TIMEOUT_MS, true, + true, ) .await, ".git write should be denied under bubblewrap", @@ -344,6 +430,7 @@ async fn sandbox_blocks_git_and_codex_writes_inside_writable_root() { &[tmpdir.path().to_path_buf()], LONG_TIMEOUT_MS, true, + true, ) .await, ".codex write should be denied under bubblewrap", @@ -355,7 +442,7 @@ async fn sandbox_blocks_git_and_codex_writes_inside_writable_root() { #[tokio::test] async fn sandbox_blocks_codex_symlink_replacement_attack() { if should_skip_bwrap_tests().await { - eprintln!("skipping bwrap test: vendored bwrap was not built in this environment"); + eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable"); return; } @@ -380,6 +467,7 @@ async fn sandbox_blocks_codex_symlink_replacement_attack() { &[tmpdir.path().to_path_buf()], LONG_TIMEOUT_MS, true, + true, ) .await, ".codex symlink replacement should be denied",