Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 45 additions & 24 deletions codex-rs/linux-sandbox/src/bwrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@ 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::FileSystemSandboxPolicy;
use codex_protocol::protocol::WritableRoot;
use codex_utils_absolute_path::AbsolutePathBuf;

/// Linux "platform defaults" that keep common system binaries and dynamic
Expand All @@ -41,10 +39,10 @@ const LINUX_PLATFORM_DEFAULT_READ_ROOTS: &[&str] = &[
/// Options that control how bubblewrap is invoked.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) struct BwrapOptions {
/// Whether to mount a fresh `/proc` inside the PID namespace.
/// Whether to mount a fresh `/proc` inside the sandbox.
///
/// This is the secure default, but some restrictive container environments
/// deny `--proc /proc` even when PID namespaces are available.
/// deny `--proc /proc`.
pub mount_proc: bool,
/// How networking should be configured inside the bubblewrap sandbox.
pub network_mode: BwrapNetworkMode,
Expand Down Expand Up @@ -167,7 +165,6 @@ fn create_bwrap_flags(
// 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());
// Isolate the PID namespace.
args.push("--unshare-pid".to_string());
if options.network_mode.should_unshare_network() {
args.push("--unshare-net".to_string());
Expand Down Expand Up @@ -213,9 +210,15 @@ 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);
// Bubblewrap requires bind mount targets to exist. Skip missing writable
// roots so mixed-platform configs can keep harmless paths for other
// environments without breaking Linux command startup.
let writable_roots = file_system_sandbox_policy
.get_writable_roots_with_cwd(cwd)
.into_iter()
.filter(|writable_root| writable_root.root.as_path().exists())
.collect::<Vec<_>>();
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() {
// Read-only root, then mount a minimal device tree.
Expand Down Expand Up @@ -385,23 +388,6 @@ fn create_filesystem_args(
})
}

/// Validate that writable roots exist before constructing mounts.
///
/// Bubblewrap requires bind mount targets to exist. We fail fast with a clear
/// error so callers can present an actionable message.
fn ensure_mount_targets_exist(writable_roots: &[WritableRoot]) -> Result<()> {
for writable_root in writable_roots {
let root = writable_root.root.as_path();
if !root.exists() {
return Err(CodexErr::UnsupportedOperation(format!(
"Sandbox expected writable root {root}, but it does not exist.",
root = root.display()
)));
}
}
Ok(())
}

fn path_to_string(path: &Path) -> String {
path.to_string_lossy().to_string()
}
Expand Down Expand Up @@ -731,6 +717,41 @@ mod tests {
);
}

#[test]
fn ignores_missing_writable_roots() {
let temp_dir = TempDir::new().expect("temp dir");
let existing_root = temp_dir.path().join("existing");
let missing_root = temp_dir.path().join("missing");
std::fs::create_dir(&existing_root).expect("create existing root");

let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![
AbsolutePathBuf::try_from(existing_root.as_path()).expect("absolute existing root"),
AbsolutePathBuf::try_from(missing_root.as_path()).expect("absolute missing root"),
],
read_only_access: Default::default(),
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
};

let args = create_filesystem_args(&FileSystemSandboxPolicy::from(&policy), temp_dir.path())
.expect("filesystem args");
let existing_root = path_to_string(&existing_root);
let missing_root = path_to_string(&missing_root);

assert!(
args.args.windows(3).any(|window| {
window == ["--bind", existing_root.as_str(), existing_root.as_str()]
}),
"existing writable root should be rebound writable",
);
assert!(
!args.args.iter().any(|arg| arg == &missing_root),
"missing writable root should be skipped",
);
}

#[test]
fn mounts_dev_before_writable_dev_binds() {
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
Expand Down
26 changes: 26 additions & 0 deletions codex-rs/linux-sandbox/tests/suite/landlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,32 @@ async fn test_writable_root() {
.await;
}

#[tokio::test]
async fn sandbox_ignores_missing_writable_roots_under_bwrap() {
if should_skip_bwrap_tests().await {
eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable");
return;
}

let tempdir = tempfile::tempdir().expect("tempdir");
let existing_root = tempdir.path().join("existing");
let missing_root = tempdir.path().join("missing");
std::fs::create_dir(&existing_root).expect("create existing root");

let output = run_cmd_result_with_writable_roots(
&["bash", "-lc", "printf sandbox-ok"],
&[existing_root, missing_root],
LONG_TIMEOUT_MS,
false,
true,
)
.await
.expect("sandboxed command should execute");

assert_eq!(output.exit_code, 0);
assert_eq!(output.stdout.text, "sandbox-ok");
}

#[tokio::test]
async fn test_no_new_privs_is_enabled() {
let output = run_cmd_output(
Expand Down
Loading