openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-16 12:13:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(linux-sandbox): enforce root read carveouts in bwrap

Browse Source
This commit is contained in:
viyatb-oai
2026-03-07 22:14:56 -08:00
parent aec2a01f7a
commit e7eccd035a
5 changed files with 250 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

182
codex-rs/linux-sandbox/src/bwrap.rs
View File

@@ -10,6 +10,8 @@
//! - seccomp + `PR_SET_NO_NEW_PRIVS` applied in-process, and
//! - bubblewrap used to construct the filesystem view before exec.
use std::collections::BTreeSet;
use std::fs::File;
use std::os::fd::AsRawFd;
use std::path::Path;
use std::path::PathBuf;
@@ -77,6 +79,12 @@ impl BwrapNetworkMode {
}
}
#[derive(Debug)]
pub(crate) struct BwrapArgs {
pub args: Vec<String>,
pub preserved_files: Vec<File>,
}
/// Wrap a command with bubblewrap so the filesystem is read-only by default,
/// with explicit writable roots and read-only subpaths layered afterward.
///
@@ -84,15 +92,18 @@ impl BwrapNetworkMode {
/// returns `command` unchanged so we avoid unnecessary sandboxing overhead.
/// If network isolation is requested, we still wrap with bubblewrap so network
/// namespace restrictions apply while preserving full filesystem access.
pub(crate) fn create_bwrap_command_args_for_policy(
pub(crate) fn create_bwrap_command_args(
command: Vec<String>,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
cwd: &Path,
options: BwrapOptions,
) -> Result<Vec<String>> {
) -> Result<BwrapArgs> {
if file_system_sandbox_policy.has_full_disk_write_access() {
return if options.network_mode == BwrapNetworkMode::FullAccess {
Ok(command)
Ok(BwrapArgs {
args: command,
preserved_files: Vec::new(),
})
} else {
Ok(create_bwrap_flags_full_filesystem(command, options))
};
@@ -101,7 +112,7 @@ pub(crate) fn create_bwrap_command_args_for_policy(
create_bwrap_flags(command, file_system_sandbox_policy, cwd, options)
}
fn create_bwrap_flags_full_filesystem(command: Vec<String>, options: BwrapOptions) -> Vec<String> {
fn create_bwrap_flags_full_filesystem(command: Vec<String>, options: BwrapOptions) -> BwrapArgs {
let mut args = vec![
"--new-session".to_string(),
"--die-with-parent".to_string(),
@@ -122,7 +133,10 @@ fn create_bwrap_flags_full_filesystem(command: Vec<String>, options: BwrapOption
}
args.push("--".to_string());
args.extend(command);
args
BwrapArgs {
args,
preserved_files: Vec::new(),
}
}
/// Build the bubblewrap flags (everything after `argv[0]`).
@@ -131,11 +145,15 @@ fn create_bwrap_flags(
file_system_sandbox_policy: &FileSystemSandboxPolicy,
cwd: &Path,
options: BwrapOptions,
) -> Result<Vec<String>> {
) -> Result<BwrapArgs> {
let BwrapArgs {
args: filesystem_args,
preserved_files,
} = create_filesystem_args(file_system_sandbox_policy, cwd)?;
let mut args = Vec::new();
args.push("--new-session".to_string());
args.push("--die-with-parent".to_string());
args.extend(create_filesystem_args(file_system_sandbox_policy, cwd)?);
args.extend(filesystem_args);
// Request a user namespace explicitly rather than relying on bubblewrap's
// auto-enable behavior, which is skipped when the caller runs as uid 0.
args.push("--unshare-user".to_string());
@@ -151,25 +169,32 @@ fn create_bwrap_flags(
}
args.push("--".to_string());
args.extend(command);
Ok(args)
Ok(BwrapArgs {
args,
preserved_files,
})
}
/// Build the bubblewrap filesystem mounts for a given filesystem policy.
///
/// The mount order is important:
/// 1. Full-read policies use `--ro-bind / /`; restricted-read policies start
/// from `--tmpfs /` and layer scoped `--ro-bind` mounts.
/// 1. Full-read policies, and restricted policies that explicitly read `/`,
/// use `--ro-bind / /`; other restricted-read policies start from
/// `--tmpfs /` and layer scoped `--ro-bind` mounts.
/// 2. `--dev /dev` mounts a minimal writable `/dev` with standard device nodes
/// (including `/dev/urandom`) even under a read-only root.
/// 3. `--bind <root> <root>` re-enables writes for allowed roots, including
/// writable subpaths under `/dev` (for example, `/dev/shm`).
/// 4. `--ro-bind <subpath> <subpath>` re-applies read-only protections under
/// those writable roots so protected subpaths win.
/// 5. Explicit unreadable roots are masked last so deny carveouts still win
/// even when the readable baseline includes `/`.
fn create_filesystem_args(
file_system_sandbox_policy: &FileSystemSandboxPolicy,
cwd: &Path,
) -> Result<Vec<String>> {
) -> Result<BwrapArgs> {
let writable_roots = file_system_sandbox_policy.get_writable_roots_with_cwd(cwd);
let unreadable_roots = file_system_sandbox_policy.get_unreadable_roots_with_cwd(cwd);
ensure_mount_targets_exist(&writable_roots)?;
let mut args = if file_system_sandbox_policy.has_full_disk_read_access() {
@@ -210,7 +235,8 @@ fn create_filesystem_args(
}
// A restricted policy can still explicitly request `/`, which is
// semantically equivalent to broad read access.
// the broad read baseline. Explicit unreadable carveouts are
// re-applied later.
if readable_roots.iter().any(|root| root == Path::new("/")) {
args = vec![
"--ro-bind".to_string(),
@@ -232,6 +258,7 @@ fn create_filesystem_args(
args
};
let mut preserved_files = Vec::new();
for writable_root in &writable_roots {
let root = writable_root.root.as_path();
@@ -275,7 +302,34 @@ fn create_filesystem_args(
}
}
Ok(args)
if !unreadable_roots.is_empty() {
let null_file = File::open("/dev/null")?;
let null_fd = null_file.as_raw_fd().to_string();
for unreadable_root in unreadable_roots {
let unreadable_root = unreadable_root.as_path();
if unreadable_root.is_dir() {
args.push("--perms".to_string());
args.push("000".to_string());
args.push("--tmpfs".to_string());
args.push(path_to_string(unreadable_root));
args.push("--remount-ro".to_string());
args.push(path_to_string(unreadable_root));
continue;
}
args.push("--perms".to_string());
args.push("000".to_string());
args.push("--ro-bind-data".to_string());
args.push(null_fd.clone());
args.push(path_to_string(unreadable_root));
}
preserved_files.push(null_file);
}
Ok(BwrapArgs {
args,
preserved_files,
})
}
/// Collect unique read-only subpaths across all writable roots.
@@ -394,6 +448,7 @@ mod tests {
use codex_protocol::protocol::FileSystemPath;
use codex_protocol::protocol::FileSystemSandboxEntry;
use codex_protocol::protocol::FileSystemSandboxPolicy;
use codex_protocol::protocol::FileSystemSpecialPath;
use codex_protocol::protocol::ReadOnlyAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -403,7 +458,7 @@ mod tests {
#[test]
fn full_disk_write_full_network_returns_unwrapped_command() {
let command = vec!["/bin/true".to_string()];
let args = create_bwrap_command_args_for_policy(
let args = create_bwrap_command_args(
command.clone(),
&FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess),
Path::new("/"),
@@ -414,13 +469,13 @@ mod tests {
)
.expect("create bwrap args");
assert_eq!(args, command);
assert_eq!(args.args, command);
}
#[test]
fn full_disk_write_proxy_only_keeps_full_filesystem_but_unshares_network() {
let command = vec!["/bin/true".to_string()];
let args = create_bwrap_command_args_for_policy(
let args = create_bwrap_command_args(
command,
&FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess),
Path::new("/"),
@@ -432,7 +487,7 @@ mod tests {
.expect("create bwrap args");
assert_eq!(
args,
args.args,
vec![
"--new-session".to_string(),
"--die-with-parent".to_string(),
@@ -466,7 +521,7 @@ mod tests {
)
.expect("bwrap fs args");
assert_eq!(
args,
args.args,
vec![
"--ro-bind".to_string(),
"/".to_string(),
@@ -503,10 +558,10 @@ mod tests {
let args = create_filesystem_args(&FileSystemSandboxPolicy::from(&policy), temp_dir.path())
.expect("filesystem args");
assert_eq!(args[0..4], ["--tmpfs", "/", "--dev", "/dev"]);
assert_eq!(args.args[0..4], ["--tmpfs", "/", "--dev", "/dev"]);
let readable_root_str = path_to_string(&readable_root);
assert!(args.windows(3).any(|window| {
assert!(args.args.windows(3).any(|window| {
window
== [
"--ro-bind",
@@ -533,11 +588,15 @@ mod tests {
let args = create_filesystem_args(&FileSystemSandboxPolicy::from(&policy), temp_dir.path())
.expect("filesystem args");
assert!(args.starts_with(&["--tmpfs".to_string(), "/".to_string()]));
assert!(
args.args
.starts_with(&["--tmpfs".to_string(), "/".to_string()])
);
if Path::new("/usr").exists() {
assert!(
args.windows(3)
args.args
.windows(3)
.any(|window| window == ["--ro-bind", "/usr", "/usr"])
);
}
@@ -571,7 +630,7 @@ mod tests {
let writable_root_str = path_to_string(writable_root.as_path());
let blocked_str = path_to_string(blocked.as_path());
assert!(args.windows(3).any(|window| {
assert!(args.args.windows(3).any(|window| {
window
== [
"--bind",
@@ -580,9 +639,84 @@ mod tests {
]
}));
assert!(
args.windows(3).any(|window| {
args.args.windows(3).any(|window| {
window == ["--ro-bind", blocked_str.as_str(), blocked_str.as_str()]
})
);
}
#[test]
fn split_policy_masks_root_read_directory_carveouts() {
let temp_dir = TempDir::new().expect("temp dir");
let blocked = temp_dir.path().join("blocked");
std::fs::create_dir_all(&blocked).expect("create blocked dir");
let blocked = AbsolutePathBuf::from_absolute_path(&blocked).expect("absolute blocked dir");
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: blocked.clone(),
},
access: FileSystemAccessMode::None,
},
]);
let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args");
let blocked_str = path_to_string(blocked.as_path());
assert!(
args.args
.windows(3)
.any(|window| window == ["--ro-bind", "/", "/"])
);
assert!(
args.args
.windows(4)
.any(|window| { window == ["--perms", "000", "--tmpfs", blocked_str.as_str()] })
);
assert!(
args.args
.windows(2)
.any(|window| window == ["--remount-ro", blocked_str.as_str()])
);
}
#[test]
fn split_policy_masks_root_read_file_carveouts() {
let temp_dir = TempDir::new().expect("temp dir");
let blocked_file = temp_dir.path().join("blocked.txt");
std::fs::write(&blocked_file, "secret").expect("create blocked file");
let blocked_file =
AbsolutePathBuf::from_absolute_path(&blocked_file).expect("absolute blocked file");
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: blocked_file.clone(),
},
access: FileSystemAccessMode::None,
},
]);
let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args");
let blocked_file_str = path_to_string(blocked_file.as_path());
assert_eq!(args.preserved_files.len(), 1);
assert!(args.args.windows(5).any(|window| {
window[0] == "--perms"
&& window[1] == "000"
&& window[2] == "--ro-bind-data"
&& window[4] == blocked_file_str
}));
}
}

28
codex-rs/linux-sandbox/src/linux_run_main.rs
View File

@@ -8,7 +8,7 @@ use std::path::PathBuf;
use crate::bwrap::BwrapNetworkMode;
use crate::bwrap::BwrapOptions;
use crate::bwrap::create_bwrap_command_args_for_policy;
use crate::bwrap::create_bwrap_command_args;
use crate::landlock::apply_sandbox_policy_to_current_thread;
use crate::proxy_routing::activate_proxy_routes_in_netns;
use crate::proxy_routing::prepare_host_proxy_route_spec;
@@ -285,13 +285,13 @@ fn run_bwrap_with_proc_fallback(
mount_proc,
network_mode,
};
let argv = build_bwrap_argv(
let bwrap_args = build_bwrap_argv(
inner,
file_system_sandbox_policy,
sandbox_policy_cwd,
options,
);
exec_vendored_bwrap(argv);
exec_vendored_bwrap(bwrap_args.args, bwrap_args.preserved_files);
}
fn bwrap_network_mode(
@@ -312,8 +312,8 @@ fn build_bwrap_argv(
file_system_sandbox_policy: &FileSystemSandboxPolicy,
sandbox_policy_cwd: &Path,
options: BwrapOptions,
) -> Vec<String> {
let mut args = create_bwrap_command_args_for_policy(
) -> crate::bwrap::BwrapArgs {
let mut bwrap_args = create_bwrap_command_args(
inner,
file_system_sandbox_policy,
sandbox_policy_cwd,
@@ -321,18 +321,22 @@ fn build_bwrap_argv(
)
.unwrap_or_else(|err| panic!("error building bubblewrap command: {err:?}"));
let command_separator_index = args
let command_separator_index = bwrap_args
.args
.iter()
.position(|arg| arg == "--")
.unwrap_or_else(|| panic!("bubblewrap argv is missing command separator '--'"));
args.splice(
bwrap_args.args.splice(
command_separator_index..command_separator_index,
["--argv0".to_string(), "codex-linux-sandbox".to_string()],
);
let mut argv = vec!["bwrap".to_string()];
argv.extend(args);
argv
argv.extend(bwrap_args.args);
crate::bwrap::BwrapArgs {
args: argv,
preserved_files: bwrap_args.preserved_files,
}
}
fn preflight_proc_mount_support(
@@ -350,7 +354,7 @@ fn build_preflight_bwrap_argv(
sandbox_policy_cwd: &Path,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
network_mode: BwrapNetworkMode,
) -> Vec<String> {
) -> crate::bwrap::BwrapArgs {
let preflight_command = vec![resolve_true_command()];
build_bwrap_argv(
preflight_command,
@@ -383,7 +387,7 @@ fn resolve_true_command() -> String {
/// - We capture stderr from that preflight to match known mount-failure text.
/// We do not stream it because this is a one-shot probe with a trivial
/// command, and reads are bounded to a fixed max size.
fn run_bwrap_in_child_capture_stderr(argv: Vec<String>) -> String {
fn run_bwrap_in_child_capture_stderr(bwrap_args: crate::bwrap::BwrapArgs) -> String {
const MAX_PREFLIGHT_STDERR_BYTES: u64 = 64 * 1024;
let mut pipe_fds = [0; 2];
@@ -412,7 +416,7 @@ fn run_bwrap_in_child_capture_stderr(argv: Vec<String>) -> String {
close_fd_or_panic(write_fd, "close write end in bubblewrap child");
}
let exit_code = run_vendored_bwrap_main(&argv);
let exit_code = run_vendored_bwrap_main(&bwrap_args.args, &bwrap_args.preserved_files);
std::process::exit(exit_code);
}

12
codex-rs/linux-sandbox/src/linux_run_main_tests.rs
View File

@@ -44,7 +44,8 @@ fn inserts_bwrap_argv0_before_command_separator() {
mount_proc: true,
network_mode: BwrapNetworkMode::FullAccess,
},
);
)
.args;
assert_eq!(
argv,
vec![
@@ -79,7 +80,8 @@ fn inserts_unshare_net_when_network_isolation_requested() {
mount_proc: true,
network_mode: BwrapNetworkMode::Isolated,
},
);
)
.args;
assert!(argv.contains(&"--unshare-net".to_string()));
}
@@ -94,7 +96,8 @@ fn inserts_unshare_net_when_proxy_only_network_mode_requested() {
mount_proc: true,
network_mode: BwrapNetworkMode::ProxyOnly,
},
);
)
.args;
assert!(argv.contains(&"--unshare-net".to_string()));
}
@@ -111,7 +114,8 @@ fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() {
Path::new("/"),
&FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess),
mode,
);
)
.args;
assert!(argv.iter().any(|arg| arg == "--"));
}

21
codex-rs/linux-sandbox/src/vendored_bwrap.rs
View File

@@ -6,6 +6,7 @@
#[cfg(vendored_bwrap_available)]
mod imp {
use std::ffi::CString;
use std::fs::File;
use std::os::raw::c_char;
unsafe extern "C" {
@@ -27,7 +28,10 @@ mod imp {
///
/// On success, bubblewrap will `execve` into the target program and this
/// function will never return. A return value therefore implies failure.
pub(crate) fn run_vendored_bwrap_main(argv: &[String]) -> libc::c_int {
pub(crate) fn run_vendored_bwrap_main(
argv: &[String],
_preserved_files: &[File],
) -> libc::c_int {
let cstrings = argv_to_cstrings(argv);
let mut argv_ptrs: Vec<*const c_char> = cstrings.iter().map(|arg| arg.as_ptr()).collect();
@@ -39,16 +43,21 @@ mod imp {
}
/// Execute the build-time bubblewrap `main` function with the given argv.
pub(crate) fn exec_vendored_bwrap(argv: Vec<String>) -> ! {
let exit_code = run_vendored_bwrap_main(&argv);
pub(crate) fn exec_vendored_bwrap(argv: Vec<String>, preserved_files: Vec<File>) -> ! {
let exit_code = run_vendored_bwrap_main(&argv, &preserved_files);
std::process::exit(exit_code);
}
}
#[cfg(not(vendored_bwrap_available))]
mod imp {
use std::fs::File;
/// Panics with a clear error when the build-time bwrap path is not enabled.
pub(crate) fn run_vendored_bwrap_main(_argv: &[String]) -> libc::c_int {
pub(crate) fn run_vendored_bwrap_main(
_argv: &[String],
_preserved_files: &[File],
) -> libc::c_int {
panic!(
r#"build-time bubblewrap is not available in this build.
codex-linux-sandbox should always compile vendored bubblewrap on Linux targets.
@@ -60,8 +69,8 @@ Notes:
}
/// Panics with a clear error when the build-time bwrap path is not enabled.
pub(crate) fn exec_vendored_bwrap(_argv: Vec<String>) -> ! {
let _ = run_vendored_bwrap_main(&[]);
pub(crate) fn exec_vendored_bwrap(_argv: Vec<String>, _preserved_files: Vec<File>) -> ! {
let _ = run_vendored_bwrap_main(&[], &[]);
unreachable!("run_vendored_bwrap_main should always panic in this configuration")
}
}

53
codex-rs/linux-sandbox/tests/suite/landlock.rs
View File

@@ -13,7 +13,9 @@ use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::ReadOnlyAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
@@ -556,6 +558,57 @@ async fn sandbox_blocks_explicit_split_policy_carveouts_under_bwrap() {
assert_ne!(output.exit_code, 0);
}
#[tokio::test]
async fn sandbox_blocks_root_read_carveouts_under_bwrap() {
if should_skip_bwrap_tests().await {
eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable");
return;
}
let tmpdir = tempfile::tempdir().expect("tempdir");
let blocked = tmpdir.path().join("blocked");
std::fs::create_dir_all(&blocked).expect("create blocked dir");
let blocked_target = blocked.join("secret.txt");
std::fs::write(&blocked_target, "secret").expect("seed blocked file");
let sandbox_policy = SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::FullAccess,
network_access: true,
};
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: AbsolutePathBuf::try_from(blocked.as_path()).expect("absolute blocked dir"),
},
access: FileSystemAccessMode::None,
},
]);
let output = expect_denied(
run_cmd_result_with_policies(
&[
"bash",
"-lc",
&format!("cat {}", blocked_target.to_string_lossy()),
],
sandbox_policy,
file_system_sandbox_policy,
NetworkSandboxPolicy::Enabled,
LONG_TIMEOUT_MS,
true,
)
.await,
"root-read carveout should be denied under bubblewrap",
);
assert_ne!(output.exit_code, 0);
}
#[tokio::test]
async fn sandbox_blocks_ssh() {
// Force ssh to attempt a real TCP connection but fail quickly. `BatchMode`