mirror of
https://github.com/openai/codex.git
synced 2026-02-01 22:47:52 +00:00
revert: remove pre-Landlock bind mounts apply (#9300)
**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.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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()?;
|
||||
}
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
#![allow(dead_code)]
|
||||
|
||||
use std::ffi::CString;
|
||||
use std::os::unix::ffi::OsStrExt;
|
||||
use std::path::Path;
|
||||
|
||||
@@ -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() {
|
||||
|
||||
Reference in New Issue
Block a user