mirror of
https://github.com/openai/codex.git
synced 2026-02-01 22:47:52 +00:00
fix: fallback to Landlock-only when user namespaces unavailable and set PR_SET_NO_NEW_PRIVS early (#9250)
fixes https://github.com/openai/codex/issues/9236 ### Motivation - Prevent sandbox setup from failing when unprivileged user namespaces are denied so Landlock-only protections can still be applied. - Ensure `PR_SET_NO_NEW_PRIVS` is set before installing seccomp and Landlock restrictions to avoid kernel `EPERM`/`LandlockRestrict` ordering issues. ### Description - Add `is_permission_denied` helper that detects `EPERM` / `PermissionDenied` from `CodexErr` to drive fallback logic. - In `apply_read_only_mounts` skip read-only bind-mount setup and return `Ok(())` when `unshare_user_and_mount_namespaces()` fails with permission-denied so Landlock rules can still be installed. - Add `set_no_new_privs()` and call it from `apply_sandbox_policy_to_current_thread` before installing seccomp filters and Landlock rules when disk or network access is restricted.
This commit is contained in:
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -1524,6 +1524,7 @@ dependencies = [
|
||||
"codex-utils-absolute-path",
|
||||
"landlock",
|
||||
"libc",
|
||||
"pretty_assertions",
|
||||
"seccompiler",
|
||||
"tempfile",
|
||||
"tokio",
|
||||
|
||||
@@ -24,6 +24,7 @@ libc = { workspace = true }
|
||||
seccompiler = { workspace = true }
|
||||
|
||||
[target.'cfg(target_os = "linux")'.dev-dependencies]
|
||||
pretty_assertions = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
tokio = { workspace = true, features = [
|
||||
"io-std",
|
||||
|
||||
@@ -37,6 +37,10 @@ pub(crate) fn apply_sandbox_policy_to_current_thread(
|
||||
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()?;
|
||||
}
|
||||
|
||||
if !sandbox_policy.has_full_network_access() {
|
||||
install_network_seccomp_filter_on_current_thread()?;
|
||||
}
|
||||
@@ -56,6 +60,14 @@ pub(crate) fn apply_sandbox_policy_to_current_thread(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn set_no_new_privs() -> Result<()> {
|
||||
let result = unsafe { libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) };
|
||||
if result != 0 {
|
||||
return Err(std::io::Error::last_os_error().into());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Installs Landlock file-system rules on the current thread allowing read
|
||||
/// access to the entire file-system while restricting write access to
|
||||
/// `/dev/null` and the provided list of `writable_roots`.
|
||||
|
||||
@@ -253,3 +253,85 @@ fn bind_mount_read_only(path: &Path) -> Result<()> {
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn collect_read_only_mount_targets_errors_on_missing_path() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let missing = AbsolutePathBuf::try_from(tempdir.path().join("missing").as_path())
|
||||
.expect("missing path");
|
||||
let root = AbsolutePathBuf::try_from(tempdir.path()).expect("root");
|
||||
let writable_root = WritableRoot {
|
||||
root,
|
||||
read_only_subpaths: vec![missing],
|
||||
};
|
||||
|
||||
let err = collect_read_only_mount_targets(&[writable_root])
|
||||
.expect_err("expected missing path error");
|
||||
let message = match err {
|
||||
CodexErr::UnsupportedOperation(message) => message,
|
||||
other => panic!("unexpected error: {other:?}"),
|
||||
};
|
||||
assert_eq!(
|
||||
message,
|
||||
format!(
|
||||
"Sandbox expected to protect {path}, but it does not exist. Ensure the repository contains this path or create it before running Codex.",
|
||||
path = tempdir.path().join("missing").display()
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_read_only_mount_targets_adds_gitdir_for_pointer_file() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let gitdir = tempdir.path().join("actual-gitdir");
|
||||
std::fs::create_dir_all(&gitdir).expect("create gitdir");
|
||||
let dot_git = tempdir.path().join(".git");
|
||||
std::fs::write(&dot_git, format!("gitdir: {}\n", gitdir.display()))
|
||||
.expect("write gitdir pointer");
|
||||
let root = AbsolutePathBuf::try_from(tempdir.path()).expect("root");
|
||||
let writable_root = WritableRoot {
|
||||
root,
|
||||
read_only_subpaths: vec![
|
||||
AbsolutePathBuf::try_from(dot_git.as_path()).expect("dot git"),
|
||||
],
|
||||
};
|
||||
|
||||
let targets = collect_read_only_mount_targets(&[writable_root]).expect("collect targets");
|
||||
assert_eq!(targets.len(), 2);
|
||||
assert_eq!(targets[0].as_path(), dot_git.as_path());
|
||||
assert_eq!(targets[1].as_path(), gitdir.as_path());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_read_only_mount_targets_errors_on_invalid_gitdir_pointer() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let dot_git = tempdir.path().join(".git");
|
||||
std::fs::write(&dot_git, "not-a-pointer\n").expect("write invalid pointer");
|
||||
let root = AbsolutePathBuf::try_from(tempdir.path()).expect("root");
|
||||
let writable_root = WritableRoot {
|
||||
root,
|
||||
read_only_subpaths: vec![
|
||||
AbsolutePathBuf::try_from(dot_git.as_path()).expect("dot git"),
|
||||
],
|
||||
};
|
||||
|
||||
let err = collect_read_only_mount_targets(&[writable_root])
|
||||
.expect_err("expected invalid pointer error");
|
||||
let message = match err {
|
||||
CodexErr::UnsupportedOperation(message) => message,
|
||||
other => panic!("unexpected error: {other:?}"),
|
||||
};
|
||||
assert_eq!(
|
||||
message,
|
||||
format!(
|
||||
"Expected {path} to contain a gitdir pointer, but it did not match `gitdir: <path>`.",
|
||||
path = dot_git.display()
|
||||
)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ use codex_core::exec_env::create_env;
|
||||
use codex_core::protocol::SandboxPolicy;
|
||||
use codex_core::sandboxing::SandboxPermissions;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use tempfile::NamedTempFile;
|
||||
@@ -36,8 +37,22 @@ fn create_env_from_core_vars() -> HashMap<String, String> {
|
||||
create_env(&policy)
|
||||
}
|
||||
|
||||
#[expect(clippy::print_stdout, clippy::expect_used, clippy::unwrap_used)]
|
||||
#[expect(clippy::print_stdout)]
|
||||
async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) {
|
||||
let output = run_cmd_output(cmd, writable_roots, timeout_ms).await;
|
||||
if output.exit_code != 0 {
|
||||
println!("stdout:\n{}", output.stdout.text);
|
||||
println!("stderr:\n{}", output.stderr.text);
|
||||
panic!("exit code: {}", output.exit_code);
|
||||
}
|
||||
}
|
||||
|
||||
#[expect(clippy::expect_used, clippy::unwrap_used)]
|
||||
async fn run_cmd_output(
|
||||
cmd: &[&str],
|
||||
writable_roots: &[PathBuf],
|
||||
timeout_ms: u64,
|
||||
) -> codex_core::exec::ExecToolCallOutput {
|
||||
let cwd = std::env::current_dir().expect("cwd should exist");
|
||||
let sandbox_cwd = cwd.clone();
|
||||
let params = ExecParams {
|
||||
@@ -64,7 +79,8 @@ async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) {
|
||||
};
|
||||
let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox");
|
||||
let codex_linux_sandbox_exe = Some(PathBuf::from(sandbox_program));
|
||||
let res = process_exec_tool_call(
|
||||
|
||||
process_exec_tool_call(
|
||||
params,
|
||||
&sandbox_policy,
|
||||
sandbox_cwd.as_path(),
|
||||
@@ -72,13 +88,7 @@ async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) {
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
if res.exit_code != 0 {
|
||||
println!("stdout:\n{}", res.stdout.text);
|
||||
println!("stderr:\n{}", res.stderr.text);
|
||||
panic!("exit code: {}", res.exit_code);
|
||||
}
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
@@ -174,6 +184,23 @@ async fn test_writable_root() {
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_no_new_privs_is_enabled() {
|
||||
let output = run_cmd_output(
|
||||
&["bash", "-lc", "grep '^NoNewPrivs:' /proc/self/status"],
|
||||
&[],
|
||||
SHORT_TIMEOUT_MS,
|
||||
)
|
||||
.await;
|
||||
let line = output
|
||||
.stdout
|
||||
.text
|
||||
.lines()
|
||||
.find(|line| line.starts_with("NoNewPrivs:"))
|
||||
.unwrap_or("");
|
||||
assert_eq!(line.trim(), "NoNewPrivs:\t1");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_git_dir_write_blocked() {
|
||||
let tmpdir = tempfile::tempdir().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user