From 55bda1a0f205cd35bd2385056f4b9b7be30dc2e1 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 15 Jan 2026 09:47:57 -0800 Subject: [PATCH] revert: remove pre-Landlock bind mounts apply (#9300) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Description** This removes the pre‑Landlock read‑only bind‑mount step from the Linux sandbox so filesystem restrictions rely solely on Landlock again. `mounts.rs` is kept in place but left unused. The linux‑sandbox README is updated to match the new behavior and manual test expectations. --- codex-rs/linux-sandbox/README.md | 49 ----- codex-rs/linux-sandbox/src/landlock.rs | 6 - codex-rs/linux-sandbox/src/mounts.rs | 2 + .../linux-sandbox/tests/suite/landlock.rs | 174 ------------------ 4 files changed, 2 insertions(+), 229 deletions(-) diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index 4d45b06e8d..676f234954 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -6,52 +6,3 @@ This crate is responsible for producing: - a lib crate that exposes the business logic of the executable as `run_main()` so that - the `codex-exec` CLI can check if its arg0 is `codex-linux-sandbox` and, if so, execute as if it were `codex-linux-sandbox` - this should also be true of the `codex` multitool CLI - -## Git safety mounts (Linux) - -When the sandbox policy allows workspace writes, the Linux sandbox uses a user -namespace plus a mount namespace to bind-mount sensitive subpaths read-only -before applying Landlock rules. This keeps Git and Codex metadata immutable -while still allowing writes to other workspace files, including worktree setups -where `.git` is a pointer file. - -Protected subpaths under each writable root include: - -- `.git` (directory or pointer file) -- the resolved `gitdir:` target when `.git` is a pointer file -- `.codex` when present - -### How this plays with Landlock - -Mount permissions and Landlock intersect: if a bind mount is read-only, writes -are denied even if Landlock would allow them. For that reason, the sandbox sets -up the read-only mounts *before* calling `landlock_restrict_self()` and then -applies Landlock rules on top. - -### Quick manual test - -Run the sandbox directly with a workspace-write policy (from a Git repository -root): - -```bash -codex-linux-sandbox \ - --sandbox-policy-cwd "$PWD" \ - --sandbox-policy '{"type":"workspace-write"}' \ - -- bash -lc ' -set -euo pipefail - -echo "should fail" > .git/config && exit 1 || true -echo "should fail" > .git/hooks/pre-commit && exit 1 || true -echo "should fail" > .git/index.lock && exit 1 || true -echo "should fail" > .codex/config.toml && exit 1 || true -echo "ok" > sandbox-write-test.txt -' -``` - -Expected behavior: - -- Writes to `.git/config` fail with `Read-only file system`. -- Creating or modifying files under `.git/hooks/` fails. -- Writing `.git/index.lock` fails (since `.git` is read-only). -- Writes under `.codex/` fail when the directory exists. -- Writing a normal repo file succeeds. diff --git a/codex-rs/linux-sandbox/src/landlock.rs b/codex-rs/linux-sandbox/src/landlock.rs index dab5a6bfce..772176fc90 100644 --- a/codex-rs/linux-sandbox/src/landlock.rs +++ b/codex-rs/linux-sandbox/src/landlock.rs @@ -7,8 +7,6 @@ use codex_core::error::SandboxErr; use codex_core::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; -use crate::mounts::apply_read_only_mounts; - use landlock::ABI; use landlock::Access; use landlock::AccessFs; @@ -33,10 +31,6 @@ pub(crate) fn apply_sandbox_policy_to_current_thread( sandbox_policy: &SandboxPolicy, cwd: &Path, ) -> Result<()> { - if !sandbox_policy.has_full_disk_write_access() { - apply_read_only_mounts(sandbox_policy, cwd)?; - } - if !sandbox_policy.has_full_disk_write_access() || !sandbox_policy.has_full_network_access() { set_no_new_privs()?; } diff --git a/codex-rs/linux-sandbox/src/mounts.rs b/codex-rs/linux-sandbox/src/mounts.rs index bed732da14..04f3651dff 100644 --- a/codex-rs/linux-sandbox/src/mounts.rs +++ b/codex-rs/linux-sandbox/src/mounts.rs @@ -1,3 +1,5 @@ +#![allow(dead_code)] + use std::ffi::CString; use std::os::unix::ffi::OsStrExt; use std::path::Path; diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index cca049e049..663f3cbe21 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -13,7 +13,6 @@ use pretty_assertions::assert_eq; use std::collections::HashMap; use std::path::PathBuf; use tempfile::NamedTempFile; -use tokio::process::Command; // At least on GitHub CI, the arm64 tests appear to need longer timeouts. @@ -91,51 +90,6 @@ async fn run_cmd_output( .unwrap() } -#[expect(clippy::expect_used)] -async fn assert_write_blocked(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) { - let cwd = std::env::current_dir().expect("cwd should exist"); - let sandbox_cwd = cwd.clone(); - let params = ExecParams { - command: cmd.iter().copied().map(str::to_owned).collect(), - cwd, - expiration: timeout_ms.into(), - env: create_env_from_core_vars(), - sandbox_permissions: SandboxPermissions::UseDefault, - justification: None, - arg0: None, - }; - - let sandbox_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: writable_roots - .iter() - .map(|p| AbsolutePathBuf::try_from(p.as_path()).unwrap()) - .collect(), - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }; - let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox"); - let codex_linux_sandbox_exe = Some(PathBuf::from(sandbox_program)); - let result = process_exec_tool_call( - params, - &sandbox_policy, - sandbox_cwd.as_path(), - &codex_linux_sandbox_exe, - None, - ) - .await; - - match result { - Ok(output) => { - if output.exit_code == 0 { - panic!("expected command to fail, but exit code was 0"); - } - } - Err(CodexErr::Sandbox(SandboxErr::Denied { .. })) => {} - Err(err) => panic!("expected sandbox denial, got: {err:?}"), - } -} - #[tokio::test] async fn test_root_read() { run_cmd(&["ls", "-l", "/bin"], &[], SHORT_TIMEOUT_MS).await; @@ -201,134 +155,6 @@ async fn test_no_new_privs_is_enabled() { assert_eq!(line.trim(), "NoNewPrivs:\t1"); } -#[tokio::test] -async fn test_git_dir_write_blocked() { - let tmpdir = tempfile::tempdir().unwrap(); - let repo_root = tmpdir.path(); - Command::new("git") - .arg("init") - .arg(".") - .current_dir(repo_root) - .output() - .await - .expect("git init ."); - - let git_config = repo_root.join(".git").join("config"); - let git_index_lock = repo_root.join(".git").join("index.lock"); - - assert_write_blocked( - &[ - "bash", - "-lc", - &format!("echo pwned > {}", git_config.to_string_lossy()), - ], - &[repo_root.to_path_buf()], - LONG_TIMEOUT_MS, - ) - .await; - - assert_write_blocked( - &[ - "bash", - "-lc", - &format!("echo pwned > {}", git_index_lock.to_string_lossy()), - ], - &[repo_root.to_path_buf()], - LONG_TIMEOUT_MS, - ) - .await; -} - -#[tokio::test] -async fn test_git_dir_move_blocked() { - let tmpdir = tempfile::tempdir().unwrap(); - let repo_root = tmpdir.path(); - Command::new("git") - .arg("init") - .arg(".") - .current_dir(repo_root) - .output() - .await - .expect("git init ."); - - let git_dir = repo_root.join(".git"); - let git_dir_backup = repo_root.join(".git.bak"); - - assert_write_blocked( - &[ - "bash", - "-lc", - &format!( - "mv {} {}", - git_dir.to_string_lossy(), - git_dir_backup.to_string_lossy() - ), - ], - &[repo_root.to_path_buf()], - LONG_TIMEOUT_MS, - ) - .await; -} - -#[tokio::test] -async fn test_codex_dir_write_blocked() { - let tmpdir = tempfile::tempdir().unwrap(); - let repo_root = tmpdir.path(); - std::fs::create_dir_all(repo_root.join(".codex")).unwrap(); - - let codex_config = repo_root.join(".codex").join("config.toml"); - - assert_write_blocked( - &[ - "bash", - "-lc", - &format!("echo pwned > {}", codex_config.to_string_lossy()), - ], - &[repo_root.to_path_buf()], - LONG_TIMEOUT_MS, - ) - .await; -} - -#[tokio::test] -async fn test_git_pointer_file_blocks_gitdir_writes() { - let tmpdir = tempfile::tempdir().unwrap(); - let repo_root = tmpdir.path(); - let gitdir = repo_root.join("actual-gitdir"); - std::fs::create_dir_all(&gitdir).unwrap(); - - let gitdir_config = gitdir.join("config"); - std::fs::write(&gitdir_config, "[core]\n\trepositoryformatversion = 0\n").unwrap(); - - std::fs::write( - repo_root.join(".git"), - format!("gitdir: {}\n", gitdir.to_string_lossy()), - ) - .unwrap(); - - assert_write_blocked( - &[ - "bash", - "-lc", - &format!("echo pwned > {}", gitdir_config.to_string_lossy()), - ], - &[repo_root.to_path_buf()], - LONG_TIMEOUT_MS, - ) - .await; - - assert_write_blocked( - &[ - "bash", - "-lc", - &format!("echo pwned > {}", repo_root.join(".git").to_string_lossy()), - ], - &[repo_root.to_path_buf()], - LONG_TIMEOUT_MS, - ) - .await; -} - #[tokio::test] #[should_panic(expected = "Sandbox(Timeout")] async fn test_timeout() {