diff --git a/codex-rs/core/README.md b/codex-rs/core/README.md index 09aadcfe9..6fddf2f87 100644 --- a/codex-rs/core/README.md +++ b/codex-rs/core/README.md @@ -48,6 +48,18 @@ Seatbelt also supports macOS permission-profile extensions layered on top of Expects the binary containing `codex-core` to run the equivalent of `codex sandbox linux` (legacy alias: `codex debug landlock`) when `arg0` is `codex-linux-sandbox`. See the `codex-arg0` crate for details. +Legacy `SandboxPolicy` / `sandbox_mode` configs are still supported on Linux. +They can continue to use the legacy Landlock path when the split filesystem +policy is sandbox-equivalent to the legacy model after `cwd` resolution. + +Split filesystem policies that need direct `FileSystemSandboxPolicy` +enforcement, such as read-only or denied carveouts under a broader writable +root, automatically route through bubblewrap. The legacy Landlock path is used +only when the split filesystem policy round-trips through the legacy +`SandboxPolicy` model without changing semantics. That includes overlapping +cases like `/repo = write`, `/repo/a = none`, `/repo/a/b = write`, where the +more specific writable child must reopen under a denied parent. + ### All Platforms Expects the binary containing `codex-core` to simulate the virtual `apply_patch` CLI when `arg1` is `--codex-run-as-apply-patch`. See the `codex-arg0` crate for details. diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index e7f835efe..b3f0c05b6 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -11,12 +11,15 @@ On Linux, the bubblewrap pipeline uses the vendored bubblewrap path compiled into this binary. **Current Behavior** +- Legacy `SandboxPolicy` / `sandbox_mode` configs remain supported. - Bubblewrap is the default filesystem sandbox pipeline and is standardized on the vendored path. - Legacy Landlock + mount protections remain available as an explicit legacy fallback path. - Set `features.use_legacy_landlock = true` (or CLI `-c use_legacy_landlock=true`) to force the legacy Landlock fallback. +- The legacy Landlock fallback is used only when the split filesystem policy is + sandbox-equivalent to the legacy model after `cwd` resolution. - Split-only filesystem policies that do not round-trip through the legacy `SandboxPolicy` model stay on bubblewrap so nested read-only or denied carveouts are preserved. @@ -27,9 +30,12 @@ into this binary. - When the default bubblewrap pipeline is active, protected subpaths under writable roots (for example `.git`, resolved `gitdir:`, and `.codex`) are re-applied as read-only via `--ro-bind`. -- When the default bubblewrap pipeline is active, overlapping split-policy entries are applied in - path-specificity order so narrower writable children can reopen broader - read-only parents while narrower denied subpaths still win. +- When the default bubblewrap pipeline is active, overlapping split-policy + entries are applied in path-specificity order so narrower writable children + can reopen broader read-only or denied parents while narrower denied subpaths + still win. For example, `/repo = write`, `/repo/a = none`, `/repo/a/b = write` + keeps `/repo` writable, denies `/repo/a`, and reopens `/repo/a/b` as + writable again. - When the default bubblewrap pipeline is active, symlink-in-path and non-existent protected paths inside writable roots are blocked by mounting `/dev/null` on the symlink or first missing component. diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 40c9d0570..e93bb80f1 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -20,6 +20,7 @@ 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 /// libraries readable when `ReadOnlyAccess::Restricted` requests them. @@ -183,14 +184,14 @@ fn create_bwrap_flags( /// `--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. Unreadable ancestors of writable roots are masked first so narrower -/// writable descendants can be rebound afterward. +/// 3. Unreadable ancestors of writable roots are masked before their child +/// mounts are rebound so nested writable carveouts can be reopened safely. /// 4. `--bind ` re-enables writes for allowed roots, including /// writable subpaths under `/dev` (for example, `/dev/shm`). /// 5. `--ro-bind ` re-applies read-only protections under /// those writable roots so protected subpaths win. -/// 6. Remaining explicit unreadable roots are masked last so deny carveouts -/// still win even when the readable baseline includes `/`. +/// 6. Nested unreadable carveouts under a writable root are masked after that +/// root is bound, and unrelated unreadable roots are masked afterward. fn create_filesystem_args( file_system_sandbox_policy: &FileSystemSandboxPolicy, cwd: &Path, @@ -265,13 +266,15 @@ fn create_filesystem_args( .iter() .map(|writable_root| writable_root.root.as_path().to_path_buf()) .collect(); - let unreadable_paths: HashSet = unreadable_roots .iter() .map(|path| path.as_path().to_path_buf()) .collect(); let mut sorted_writable_roots = writable_roots; sorted_writable_roots.sort_by_key(|writable_root| path_depth(writable_root.root.as_path())); + // Mask only the unreadable ancestors that sit outside every writable root. + // Unreadable paths nested under a broader writable root are applied after + // that broader root is bound, then reopened by any deeper writable child. let mut unreadable_ancestors_of_writable_roots: Vec = unreadable_roots .iter() .filter(|path| { @@ -286,6 +289,7 @@ fn create_filesystem_args( .map(|path| path.as_path().to_path_buf()) .collect(); unreadable_ancestors_of_writable_roots.sort_by_key(|path| path_depth(path)); + for unreadable_root in &unreadable_ancestors_of_writable_roots { append_unreadable_root_args( &mut args, @@ -297,13 +301,17 @@ fn create_filesystem_args( for writable_root in &sorted_writable_roots { let root = writable_root.root.as_path(); - if let Some(masking_root) = unreadable_ancestors_of_writable_roots + // If a denied ancestor was already masked, recreate any missing mount + // target parents before binding the narrower writable descendant. + if let Some(masking_root) = unreadable_roots .iter() + .map(AbsolutePathBuf::as_path) .filter(|unreadable_root| root.starts_with(unreadable_root)) .max_by_key(|unreadable_root| path_depth(unreadable_root)) { append_mount_target_parent_dir_args(&mut args, root, masking_root); } + args.push("--bind".to_string()); args.push(path_to_string(root)); args.push(path_to_string(root)); @@ -318,7 +326,6 @@ fn create_filesystem_args( for subpath in read_only_subpaths { append_read_only_subpath_args(&mut args, &subpath, &allowed_write_paths); } - let mut nested_unreadable_roots: Vec = unreadable_roots .iter() .filter(|path| path.as_path().starts_with(root)) @@ -389,11 +396,10 @@ fn path_depth(path: &Path) -> usize { fn append_mount_target_parent_dir_args(args: &mut Vec, mount_target: &Path, anchor: &Path) { let mount_target_dir = if mount_target.is_dir() { mount_target + } else if let Some(parent) = mount_target.parent() { + parent } else { - match mount_target.parent() { - Some(parent) => parent, - None => return, - } + return; }; let mut mount_target_dirs: Vec = mount_target_dir .ancestors() @@ -462,10 +468,30 @@ fn append_unreadable_root_args( } if unreadable_root.is_dir() { + let mut writable_descendants: Vec<&Path> = allowed_write_paths + .iter() + .map(PathBuf::as_path) + .filter(|path| *path != unreadable_root && path.starts_with(unreadable_root)) + .collect(); args.push("--perms".to_string()); - args.push("000".to_string()); + // Execute-only perms let the process traverse into explicitly + // re-opened writable descendants while still hiding the denied + // directory contents. Plain denied directories with no writable child + // mounts stay at `000`. + args.push(if writable_descendants.is_empty() { + "000".to_string() + } else { + "111".to_string() + }); args.push("--tmpfs".to_string()); args.push(path_to_string(unreadable_root)); + // Recreate any writable descendants inside the tmpfs before remounting + // the denied parent read-only. Otherwise bubblewrap cannot mkdir the + // nested mount targets after the parent has been frozen. + writable_descendants.sort_by_key(|path| path_depth(path)); + for writable_descendant in writable_descendants { + append_mount_target_parent_dir_args(args, writable_descendant, unreadable_root); + } args.push("--remount-ro".to_string()); args.push(path_to_string(unreadable_root)); return Ok(()); @@ -730,24 +756,22 @@ mod tests { 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 writable_root_str = path_to_string(writable_root.as_path()); + let blocked_str = path_to_string(blocked.as_path()); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Path { - path: writable_root.clone(), + path: writable_root, }, access: FileSystemAccessMode::Write, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { - path: blocked.clone(), - }, + path: FileSystemPath::Path { path: blocked }, 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 @@ -882,13 +906,18 @@ mod tests { let blocked_none_index = args .args .windows(4) - .position(|window| window == ["--perms", "000", "--tmpfs", blocked_str.as_str()]) + .position(|window| window == ["--perms", "111", "--tmpfs", blocked_str.as_str()]) .expect("blocked should be masked first"); let allowed_dir_index = args .args .windows(2) .position(|window| window == ["--dir", allowed_str.as_str()]) .expect("allowed mount target should be recreated"); + let blocked_remount_ro_index = args + .args + .windows(2) + .position(|window| window == ["--remount-ro", blocked_str.as_str()]) + .expect("blocked directory should be remounted read-only"); let allowed_bind_index = args .args .windows(3) @@ -896,8 +925,10 @@ mod tests { .expect("allowed path should be rebound writable"); assert!( - blocked_none_index < allowed_dir_index && allowed_dir_index < allowed_bind_index, - "expected unreadable parent mask before recreating and rebinding writable child: {:#?}", + blocked_none_index < allowed_dir_index + && allowed_dir_index < blocked_remount_ro_index + && blocked_remount_ro_index < allowed_bind_index, + "expected writable child target recreation before remounting and rebinding under unreadable parent: {:#?}", args.args ); } @@ -959,7 +990,7 @@ mod tests { let blocked_none_index = args .args .windows(4) - .position(|window| window == ["--perms", "000", "--tmpfs", blocked_str.as_str()]) + .position(|window| window == ["--perms", "111", "--tmpfs", blocked_str.as_str()]) .expect("blocked should be masked first"); let allowed_bind_index = args .args @@ -981,6 +1012,60 @@ mod tests { ); } + #[test] + fn split_policy_reenables_nested_writable_roots_after_unreadable_parent() { + let temp_dir = TempDir::new().expect("temp dir"); + let writable_root = temp_dir.path().join("workspace"); + let blocked = writable_root.join("blocked"); + let allowed = blocked.join("allowed"); + std::fs::create_dir_all(&allowed).expect("create blocked/allowed 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 allowed = AbsolutePathBuf::from_absolute_path(&allowed).expect("absolute allowed dir"); + let blocked_str = path_to_string(blocked.as_path()); + let allowed_str = path_to_string(allowed.as_path()); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: writable_root, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: blocked }, + access: FileSystemAccessMode::None, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: allowed }, + access: FileSystemAccessMode::Write, + }, + ]); + + let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let blocked_none_index = args + .args + .windows(4) + .position(|window| window == ["--perms", "111", "--tmpfs", blocked_str.as_str()]) + .expect("blocked should be masked first"); + let allowed_dir_index = args + .args + .windows(2) + .position(|window| window == ["--dir", allowed_str.as_str()]) + .expect("allowed mount target should be recreated"); + let allowed_bind_index = args + .args + .windows(3) + .position(|window| window == ["--bind", allowed_str.as_str(), allowed_str.as_str()]) + .expect("allowed path should be rebound writable"); + + assert!( + blocked_none_index < allowed_dir_index && allowed_dir_index < allowed_bind_index, + "expected unreadable parent mask before recreating and rebinding writable child: {:#?}", + args.args + ); + } + #[test] fn split_policy_masks_root_read_directory_carveouts() { let temp_dir = TempDir::new().expect("temp dir"); diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index 774fb4f17..49898eb0e 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -515,6 +515,12 @@ async fn sandbox_blocks_explicit_split_policy_carveouts_under_bwrap() { let blocked = tmpdir.path().join("blocked"); std::fs::create_dir_all(&blocked).expect("create blocked dir"); let blocked_target = blocked.join("secret.txt"); + // These tests bypass the usual legacy-policy bridge, so explicitly keep + // the sandbox helper binary and minimal runtime paths readable. + let sandbox_helper_dir = PathBuf::from(env!("CARGO_BIN_EXE_codex-linux-sandbox")) + .parent() + .expect("sandbox helper should have a parent") + .to_path_buf(); let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![AbsolutePathBuf::try_from(tmpdir.path()).expect("absolute tempdir")], @@ -524,6 +530,19 @@ async fn sandbox_blocks_explicit_split_policy_carveouts_under_bwrap() { exclude_slash_tmp: true, }; let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Minimal, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: AbsolutePathBuf::try_from(sandbox_helper_dir.as_path()) + .expect("absolute helper dir"), + }, + access: FileSystemAccessMode::Read, + }, FileSystemSandboxEntry { path: FileSystemPath::Path { path: AbsolutePathBuf::try_from(tmpdir.path()).expect("absolute tempdir"), @@ -557,6 +576,88 @@ async fn sandbox_blocks_explicit_split_policy_carveouts_under_bwrap() { assert_ne!(output.exit_code, 0); } +#[tokio::test] +async fn sandbox_reenables_writable_subpaths_under_unreadable_parents() { + 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"); + let allowed = blocked.join("allowed"); + std::fs::create_dir_all(&allowed).expect("create blocked/allowed dir"); + let allowed_target = allowed.join("note.txt"); + // These tests bypass the usual legacy-policy bridge, so explicitly keep + // the sandbox helper binary and minimal runtime paths readable. + let sandbox_helper_dir = PathBuf::from(env!("CARGO_BIN_EXE_codex-linux-sandbox")) + .parent() + .expect("sandbox helper should have a parent") + .to_path_buf(); + + let sandbox_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![AbsolutePathBuf::try_from(tmpdir.path()).expect("absolute tempdir")], + read_only_access: Default::default(), + network_access: true, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Minimal, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: AbsolutePathBuf::try_from(sandbox_helper_dir.as_path()) + .expect("absolute helper dir"), + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: AbsolutePathBuf::try_from(tmpdir.path()).expect("absolute tempdir"), + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: AbsolutePathBuf::try_from(blocked.as_path()).expect("absolute blocked dir"), + }, + access: FileSystemAccessMode::None, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: AbsolutePathBuf::try_from(allowed.as_path()).expect("absolute allowed dir"), + }, + access: FileSystemAccessMode::Write, + }, + ]); + let output = run_cmd_result_with_policies( + &[ + "bash", + "-lc", + &format!( + "printf allowed > {} && cat {}", + allowed_target.to_string_lossy(), + allowed_target.to_string_lossy() + ), + ], + sandbox_policy, + file_system_sandbox_policy, + NetworkSandboxPolicy::Enabled, + LONG_TIMEOUT_MS, + false, + ) + .await + .expect("nested writable carveout should execute under bubblewrap"); + + assert_eq!(output.exit_code, 0); + assert_eq!(output.stdout.text.trim(), "allowed"); +} + #[tokio::test] async fn sandbox_blocks_root_read_carveouts_under_bwrap() { if should_skip_bwrap_tests().await {