clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-29 17:06:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

linux-sandbox: honor split filesystem policies in bwrap (#13453)

Browse Source 
## Why

After `#13449`, the Linux helper could receive split filesystem and
network policies, but the bubblewrap mount builder still reconstructed
filesystem access from the legacy `SandboxPolicy`.

That loses explicit unreadable carveouts under writable roots, and it
also mishandles `Root` read access paired with explicit deny carveouts.
In those cases bubblewrap could still expose paths that the split
filesystem policy intentionally blocked.

## What changed

- switched bubblewrap mount generation to consume
`FileSystemSandboxPolicy` directly at the implementation boundary;
legacy `SandboxPolicy` configs still flow through the existing
`FileSystemSandboxPolicy::from(&sandbox_policy)` bridge before reaching
bwrap
- kept the Linux helper and preflight path on the split filesystem
policy all the way into bwrap
- re-applied explicit unreadable carveouts after readable and writable
mounts so blocked subpaths still win under bubblewrap
- masked denied directories with `--tmpfs` plus `--remount-ro` and
denied files with `--ro-bind-data`, preserving the backing fd until exec
- added comments in the unreadable-root masking block to explain why the
mount order and directory/file split are intentional
- updated Linux helper call sites and tests for the split-policy bwrap
path

## Verification

- added protocol coverage for root carveouts staying scoped
- added core coverage that root-write plus deny carveouts still requires
a platform sandbox
- added bwrap unit coverage for reapplying blocked carveouts after
writable binds
- added Linux integration coverage for explicit split-policy carveouts
under bubblewrap
- validated the final branch state with `cargo test -p
codex-linux-sandbox`, `cargo clippy -p codex-linux-sandbox --all-targets
-- -D warnings`, and the PR CI reruns

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/13453).
* __->__ #13453
* #13452
* #13451
* #13449
* #13448
* #13445
* #13440
* #13439

---------

Co-authored-by: viyatb-oai <viyatb@openai.com>
This commit is contained in:
Michael Bolin
2026-03-07 23:46:52 -08:00
committed by GitHub
parent dc19e78962
commit dcc4d7b634
5 changed files with 456 additions and 88 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -10,12 +10,14 @@
//! - 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;
use codex_core::error::CodexErr;
use codex_core::error::Result;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::FileSystemSandboxPolicy;
use codex_protocol::protocol::WritableRoot;
/// Linux "platform defaults" that keep common system binaries and dynamic
@@ -76,6 +78,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.
///
@@ -85,22 +93,25 @@ impl BwrapNetworkMode {
/// namespace restrictions apply while preserving full filesystem access.
pub(crate) fn create_bwrap_command_args(
command: Vec<String>,
sandbox_policy: &SandboxPolicy,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
cwd: &Path,
options: BwrapOptions,
) -> Result<Vec<String>> {
if sandbox_policy.has_full_disk_write_access() {
) -> 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))
};
}
create_bwrap_flags(command, sandbox_policy, cwd, options)
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(),
@@ -121,20 +132,27 @@ 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]`).
fn create_bwrap_flags(
command: Vec<String>,
sandbox_policy: &SandboxPolicy,
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(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());
@@ -150,25 +168,35 @@ 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 sandbox policy.
/// 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.
fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result<Vec<String>> {
let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd);
/// 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<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 sandbox_policy.has_full_disk_read_access() {
let mut args = if file_system_sandbox_policy.has_full_disk_read_access() {
// 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,
@@ -191,12 +219,12 @@ fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result<
"/dev".to_string(),
];
let mut readable_roots: BTreeSet<PathBuf> = sandbox_policy
let mut readable_roots: BTreeSet<PathBuf> = file_system_sandbox_policy
.get_readable_roots_with_cwd(cwd)
.into_iter()
.map(PathBuf::from)
.collect();
if sandbox_policy.include_platform_defaults() {
if file_system_sandbox_policy.include_platform_defaults() {
readable_roots.extend(
LINUX_PLATFORM_DEFAULT_READ_ROOTS
.iter()
@@ -206,7 +234,8 @@ fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result<
}
// 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(),
@@ -228,6 +257,7 @@ fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result<
args
};
let mut preserved_files = Vec::new();
for writable_root in &writable_roots {
let root = writable_root.root.as_path();
@@ -271,7 +301,44 @@ fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result<
}
}
Ok(args)
if !unreadable_roots.is_empty() {
// Apply explicit deny carveouts after all readable and writable mounts
// so they win even when the broader baseline includes `/` or a writable
// parent path.
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() {
// Bubblewrap cannot bind `/dev/null` over a directory, so mask
// denied directories by overmounting them with an empty tmpfs
// and then remounting that tmpfs read-only.
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;
}
// For files, bind a stable null-file payload over the original path
// so later reads do not expose host contents. `--ro-bind-data`
// expects a live fd number, so keep the backing file open until we
// exec bubblewrap below.
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.
@@ -386,6 +453,11 @@ fn find_first_non_existent_component(target_path: &Path) -> Option<PathBuf> {
#[cfg(test)]
mod tests {
use super::*;
use codex_protocol::protocol::FileSystemAccessMode;
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;
@@ -397,7 +469,7 @@ mod tests {
let command = vec!["/bin/true".to_string()];
let args = create_bwrap_command_args(
command.clone(),
&SandboxPolicy::DangerFullAccess,
&FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess),
Path::new("/"),
BwrapOptions {
mount_proc: true,
@@ -406,7 +478,7 @@ mod tests {
)
.expect("create bwrap args");
assert_eq!(args, command);
assert_eq!(args.args, command);
}
#[test]
@@ -414,7 +486,7 @@ mod tests {
let command = vec!["/bin/true".to_string()];
let args = create_bwrap_command_args(
command,
&SandboxPolicy::DangerFullAccess,
&FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess),
Path::new("/"),
BwrapOptions {
mount_proc: true,
@@ -424,7 +496,7 @@ mod tests {
.expect("create bwrap args");
assert_eq!(
args,
args.args,
vec![
"--new-session".to_string(),
"--die-with-parent".to_string(),
@@ -452,9 +524,13 @@ mod tests {
exclude_slash_tmp: true,
};
let args = create_filesystem_args(&sandbox_policy, Path::new("/")).expect("bwrap fs args");
let args = create_filesystem_args(
&FileSystemSandboxPolicy::from(&sandbox_policy),
Path::new("/"),
)
.expect("bwrap fs args");
assert_eq!(
args,
args.args,
vec![
"--ro-bind".to_string(),
"/".to_string(),
@@ -462,11 +538,11 @@ mod tests {
"--dev".to_string(),
"/dev".to_string(),
"--bind".to_string(),
"/dev".to_string(),
"/dev".to_string(),
"/".to_string(),
"/".to_string(),
"--bind".to_string(),
"/".to_string(),
"/".to_string(),
"/dev".to_string(),
"/dev".to_string(),
]
);
}
@@ -488,12 +564,13 @@ mod tests {
network_access: false,
};
let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args");
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",
@@ -517,15 +594,138 @@ mod tests {
// `ReadOnlyAccess::Restricted` always includes `cwd` as a readable
// root. Using `"/"` here would intentionally collapse to broad read
// access, so use a non-root cwd to exercise the restricted path.
let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args");
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"])
);
}
}
#[test]
fn split_policy_reapplies_unreadable_carveouts_after_writable_binds() {
let temp_dir = TempDir::new().expect("temp dir");
let writable_root = temp_dir.path().join("workspace");
let blocked = writable_root.join("blocked");
std::fs::create_dir_all(&blocked).expect("create blocked dir");
let writable_root =
AbsolutePathBuf::from_absolute_path(&writable_root).expect("absolute writable root");
let blocked = AbsolutePathBuf::from_absolute_path(&blocked).expect("absolute blocked dir");
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: writable_root.clone(),
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: blocked.clone(),
},
access: FileSystemAccessMode::None,
},
]);
let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args");
let writable_root_str = path_to_string(writable_root.as_path());
let blocked_str = path_to_string(blocked.as_path());
assert!(args.args.windows(3).any(|window| {
window
== [
"--bind",
writable_root_str.as_str(),
writable_root_str.as_str(),
]
}));
assert!(
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
}));
}
}