From 8b394d1706d19e5a3d15d1f6f710f4f1088be89f Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 27 Mar 2026 01:08:24 -0700 Subject: [PATCH 01/12] Fix symlinked writable roots in sandbox permissions --- codex-rs/exec/src/lib.rs | 3 +- codex-rs/linux-sandbox/src/bwrap.rs | 135 +++++++++++++++++- codex-rs/protocol/src/permissions.rs | 91 ++++-------- codex-rs/sandboxing/src/policy_transforms.rs | 4 +- .../sandboxing/src/policy_transforms_tests.rs | 12 +- codex-rs/tui/src/lib.rs | 3 +- codex-rs/tui_app_server/src/lib.rs | 3 +- codex-rs/utils/absolute-path/src/lib.rs | 52 +++++++ 8 files changed, 223 insertions(+), 80 deletions(-) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 0cc5ad28cabe..c32e513a61c2 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -80,6 +80,7 @@ use codex_protocol::protocol::SessionConfiguredEvent; use codex_protocol::protocol::SessionSource; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_absolute_path::canonicalize_preserving_symlinks; use codex_utils_oss::ensure_oss_provider_ready; use codex_utils_oss::get_default_model_for_oss_provider; use event_processor_with_human_output::EventProcessorWithHumanOutput; @@ -230,7 +231,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result let resolved_cwd = cwd.clone(); let config_cwd = match resolved_cwd.as_deref() { - Some(path) => AbsolutePathBuf::from_absolute_path(path.canonicalize()?)?, + Some(path) => AbsolutePathBuf::from_absolute_path(canonicalize_preserving_symlinks(path)?)?, None => AbsolutePathBuf::current_dir()?, }; diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index dfc5b1282ac1..d9d86630d118 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -11,6 +11,7 @@ //! - bubblewrap used to construct the filesystem view before exec. use std::collections::BTreeSet; use std::collections::HashSet; +use std::fs; use std::fs::File; use std::os::fd::AsRawFd; use std::path::Path; @@ -282,10 +283,14 @@ fn create_filesystem_args( args }; let mut preserved_files = Vec::new(); - let allowed_write_paths: Vec = writable_roots - .iter() - .map(|writable_root| writable_root.root.as_path().to_path_buf()) - .collect(); + let mut allowed_write_paths = Vec::with_capacity(writable_roots.len()); + for writable_root in &writable_roots { + let root = writable_root.root.as_path(); + allowed_write_paths.push(root.to_path_buf()); + if let Some(target) = symlink_target(root) { + allowed_write_paths.push(target); + } + } let unreadable_paths: HashSet = unreadable_roots .iter() .map(|path| path.as_path().to_path_buf()) @@ -321,6 +326,7 @@ fn create_filesystem_args( for writable_root in &sorted_writable_roots { let root = writable_root.root.as_path(); + let symlink_target = symlink_target(root); // 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 @@ -332,9 +338,10 @@ fn create_filesystem_args( append_mount_target_parent_dir_args(&mut args, root, masking_root); } + let mount_root = symlink_target.as_deref().unwrap_or(root); args.push("--bind".to_string()); - args.push(path_to_string(root)); - args.push(path_to_string(root)); + args.push(path_to_string(mount_root)); + args.push(path_to_string(mount_root)); let mut read_only_subpaths: Vec = writable_root .read_only_subpaths @@ -342,6 +349,9 @@ fn create_filesystem_args( .map(|path| path.as_path().to_path_buf()) .filter(|path| !unreadable_paths.contains(path)) .collect(); + if let Some(target) = &symlink_target { + read_only_subpaths = remap_paths_for_symlink_target(read_only_subpaths, root, target); + } read_only_subpaths.sort_by_key(|path| path_depth(path)); for subpath in read_only_subpaths { append_read_only_subpath_args(&mut args, &subpath, &allowed_write_paths); @@ -351,6 +361,10 @@ fn create_filesystem_args( .filter(|path| path.as_path().starts_with(root)) .map(|path| path.as_path().to_path_buf()) .collect(); + if let Some(target) = &symlink_target { + nested_unreadable_roots = + remap_paths_for_symlink_target(nested_unreadable_roots, root, target); + } nested_unreadable_roots.sort_by_key(|path| path_depth(path)); for unreadable_root in nested_unreadable_roots { append_unreadable_root_args( @@ -396,6 +410,31 @@ fn path_depth(path: &Path) -> usize { path.components().count() } +fn symlink_target(root: &Path) -> Option { + let meta = fs::symlink_metadata(root).ok()?; + if !meta.file_type().is_symlink() { + return None; + } + let target = fs::canonicalize(root).ok()?; + if target.as_path() == root { + return None; + } + Some(target) +} + +fn remap_paths_for_symlink_target(paths: Vec, root: &Path, target: &Path) -> Vec { + paths + .into_iter() + .map(|path| { + if let Ok(relative) = path.strip_prefix(root) { + target.join(relative) + } else { + path + } + }) + .collect() +} + fn normalize_command_cwd_for_bwrap(command_cwd: &Path) -> PathBuf { command_cwd .canonicalize() @@ -717,6 +756,90 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn symlinked_writable_roots_bind_real_target_and_remap_carveouts() { + let temp_dir = TempDir::new().expect("temp dir"); + let real_root = temp_dir.path().join("real"); + let link_root = temp_dir.path().join("link"); + let blocked = real_root.join("blocked"); + std::fs::create_dir_all(&blocked).expect("create blocked dir"); + std::os::unix::fs::symlink(&real_root, &link_root).expect("create symlinked root"); + + let link_root = + AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root"); + let link_blocked = link_root.join("blocked").expect("symlinked blocked"); + let real_root_str = path_to_string(&real_root); + let real_blocked_str = path_to_string(&blocked); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: link_root }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: link_blocked }, + access: FileSystemAccessMode::None, + }, + ]); + + let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + + assert!(args.args.windows(3).any(|window| { + window == ["--bind", real_root_str.as_str(), real_root_str.as_str()] + })); + assert!(args.args.windows(6).any(|window| { + window + == [ + "--perms", + "000", + "--tmpfs", + real_blocked_str.as_str(), + "--remount-ro", + real_blocked_str.as_str(), + ] + })); + } + + #[cfg(unix)] + #[test] + fn symlinked_writable_roots_mask_nested_symlink_escape_paths_without_binding_targets() { + let temp_dir = TempDir::new().expect("temp dir"); + let real_root = temp_dir.path().join("real"); + let link_root = temp_dir.path().join("link"); + let outside = temp_dir.path().join("outside-private"); + let linked_private = real_root.join("linked-private"); + std::fs::create_dir_all(&real_root).expect("create real root"); + std::fs::create_dir_all(&outside).expect("create outside dir"); + std::os::unix::fs::symlink(&real_root, &link_root).expect("create symlinked root"); + std::os::unix::fs::symlink(&outside, &linked_private) + .expect("create nested escape symlink"); + + let link_root = + AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root"); + let link_private = link_root + .join("linked-private") + .expect("symlinked linked-private"); + let real_linked_private_str = path_to_string(&linked_private); + let outside_str = path_to_string(&outside); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: link_root }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: link_private }, + access: FileSystemAccessMode::None, + }, + ]); + + let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + + assert!(args.args.windows(3).any(|window| { + window == ["--ro-bind", "/dev/null", real_linked_private_str.as_str()] + })); + assert!(!args.args.iter().any(|arg| arg == outside_str.as_str())); + } + #[test] fn ignores_missing_writable_roots() { let temp_dir = TempDir::new().expect("temp dir"); diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 978334f696ea..658746375914 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -5,6 +5,7 @@ use std::path::Path; use std::path::PathBuf; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_absolute_path::canonicalize_preserving_symlinks; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -471,6 +472,9 @@ impl FileSystemSandboxPolicy { // so root-wide aliases do not create duplicate top-level masks. // Example: keep `/var/...` normalized under `/` instead of // materializing both `/var/...` and `/private/var/...`. + // Nested symlink paths under a writable root stay logical so + // downstream sandboxes can still bind the real target while + // masking the user-visible symlink inode when needed. let preserve_raw_carveout_paths = root.as_path().parent().is_some(); let raw_writable_roots: Vec<&AbsolutePathBuf> = writable_entries .iter() @@ -1080,14 +1084,17 @@ fn dedup_absolute_paths( fn normalize_effective_absolute_path(path: AbsolutePathBuf) -> AbsolutePathBuf { let raw_path = path.to_path_buf(); for ancestor in raw_path.ancestors() { - let Ok(canonical_ancestor) = ancestor.canonicalize() else { + if std::fs::symlink_metadata(ancestor).is_err() { + continue; + } + let Ok(normalized_ancestor) = canonicalize_preserving_symlinks(ancestor) else { continue; }; let Ok(suffix) = raw_path.strip_prefix(ancestor) else { continue; }; if let Ok(normalized_path) = - AbsolutePathBuf::from_absolute_path(canonical_ancestor.join(suffix)) + AbsolutePathBuf::from_absolute_path(normalized_ancestor.join(suffix)) { return normalized_path; } @@ -1435,7 +1442,7 @@ mod tests { #[cfg(unix)] #[test] - fn effective_runtime_roots_canonicalize_symlinked_paths() { + fn effective_runtime_roots_preserve_symlinked_paths() { let cwd = TempDir::new().expect("tempdir"); let real_root = cwd.path().join("real"); let link_root = cwd.path().join("link"); @@ -1449,18 +1456,9 @@ mod tests { let link_root = AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root"); let link_blocked = link_root.join("blocked").expect("symlinked blocked path"); - let expected_root = AbsolutePathBuf::from_absolute_path( - real_root.canonicalize().expect("canonicalize real root"), - ) - .expect("absolute canonical root"); - let expected_blocked = AbsolutePathBuf::from_absolute_path( - blocked.canonicalize().expect("canonicalize blocked"), - ) - .expect("absolute canonical blocked"); - let expected_codex = AbsolutePathBuf::from_absolute_path( - codex_dir.canonicalize().expect("canonicalize .codex"), - ) - .expect("absolute canonical .codex"); + let expected_root = link_root.clone(); + let expected_blocked = link_blocked.clone(); + let expected_codex = link_root.join(".codex").expect("expected .codex"); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { @@ -1495,7 +1493,7 @@ mod tests { #[cfg(unix)] #[test] - fn current_working_directory_special_path_canonicalizes_symlinked_cwd() { + fn current_working_directory_special_path_preserves_symlinked_cwd() { let cwd = TempDir::new().expect("tempdir"); let real_root = cwd.path().join("real"); let link_root = cwd.path().join("link"); @@ -1510,22 +1508,11 @@ mod tests { let link_blocked = AbsolutePathBuf::from_absolute_path(link_root.join("blocked")).expect("link blocked"); - let expected_root = AbsolutePathBuf::from_absolute_path( - real_root.canonicalize().expect("canonicalize real root"), - ) - .expect("absolute canonical root"); - let expected_blocked = AbsolutePathBuf::from_absolute_path( - blocked.canonicalize().expect("canonicalize blocked"), - ) - .expect("absolute canonical blocked"); - let expected_agents = AbsolutePathBuf::from_absolute_path( - agents_dir.canonicalize().expect("canonicalize .agents"), - ) - .expect("absolute canonical .agents"); - let expected_codex = AbsolutePathBuf::from_absolute_path( - codex_dir.canonicalize().expect("canonicalize .codex"), - ) - .expect("absolute canonical .codex"); + let expected_root = + AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root"); + let expected_blocked = link_blocked.clone(); + let expected_agents = expected_root.join(".agents").expect("expected .agents"); + let expected_codex = expected_root.join(".codex").expect("expected .codex"); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { @@ -1632,13 +1619,8 @@ mod tests { let link_private = link_root .join("linked-private") .expect("symlinked linked-private path"); - let expected_root = AbsolutePathBuf::from_absolute_path( - real_root.canonicalize().expect("canonicalize real root"), - ) - .expect("absolute canonical root"); - let expected_linked_private = expected_root - .join("linked-private") - .expect("expected linked-private path"); + let expected_root = link_root.clone(); + let expected_linked_private = link_private.clone(); let unexpected_decoy = AbsolutePathBuf::from_absolute_path(decoy.canonicalize().expect("canonicalize decoy")) .expect("absolute canonical decoy"); @@ -1686,13 +1668,8 @@ mod tests { let link_private = link_root .join("linked-private") .expect("symlinked linked-private path"); - let expected_root = AbsolutePathBuf::from_absolute_path( - real_root.canonicalize().expect("canonicalize real root"), - ) - .expect("absolute canonical root"); - let expected_linked_private = expected_root - .join("linked-private") - .expect("expected linked-private path"); + let expected_root = link_root.clone(); + let expected_linked_private = link_private.clone(); let unexpected_decoy = AbsolutePathBuf::from_absolute_path(decoy.canonicalize().expect("canonicalize decoy")) .expect("absolute canonical decoy"); @@ -1760,12 +1737,12 @@ mod tests { #[cfg(unix)] #[test] - fn tmpdir_special_path_canonicalizes_symlinked_tmpdir() { + fn tmpdir_special_path_preserves_symlinked_tmpdir() { if std::env::var_os(SYMLINKED_TMPDIR_TEST_ENV).is_none() { let output = std::process::Command::new(std::env::current_exe().expect("test binary")) .env(SYMLINKED_TMPDIR_TEST_ENV, "1") .arg("--exact") - .arg("permissions::tests::tmpdir_special_path_canonicalizes_symlinked_tmpdir") + .arg("permissions::tests::tmpdir_special_path_preserves_symlinked_tmpdir") .output() .expect("run tmpdir subprocess test"); @@ -1790,20 +1767,10 @@ mod tests { let link_blocked = AbsolutePathBuf::from_absolute_path(link_tmpdir.join("blocked")).expect("link blocked"); - let expected_root = AbsolutePathBuf::from_absolute_path( - real_tmpdir - .canonicalize() - .expect("canonicalize real tmpdir"), - ) - .expect("absolute canonical tmpdir"); - let expected_blocked = AbsolutePathBuf::from_absolute_path( - blocked.canonicalize().expect("canonicalize blocked"), - ) - .expect("absolute canonical blocked"); - let expected_codex = AbsolutePathBuf::from_absolute_path( - codex_dir.canonicalize().expect("canonicalize .codex"), - ) - .expect("absolute canonical .codex"); + let expected_root = + AbsolutePathBuf::from_absolute_path(&link_tmpdir).expect("absolute symlinked tmpdir"); + let expected_blocked = link_blocked.clone(); + let expected_codex = expected_root.join(".codex").expect("expected .codex"); unsafe { std::env::set_var("TMPDIR", &link_tmpdir); diff --git a/codex-rs/sandboxing/src/policy_transforms.rs b/codex-rs/sandboxing/src/policy_transforms.rs index 25c79fcc9eb5..73ecf81c9bc6 100644 --- a/codex-rs/sandboxing/src/policy_transforms.rs +++ b/codex-rs/sandboxing/src/policy_transforms.rs @@ -11,7 +11,7 @@ use codex_protocol::protocol::NetworkAccess; use codex_protocol::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; -use dunce::canonicalize; +use codex_utils_absolute_path::canonicalize_preserving_symlinks; use std::collections::HashSet; #[derive(Debug, Clone, PartialEq, Eq)] @@ -171,7 +171,7 @@ fn normalize_permission_paths( let mut seen = HashSet::new(); for path in paths { - let canonicalized = canonicalize(path.as_path()) + let canonicalized = canonicalize_preserving_symlinks(path.as_path()) .ok() .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) .unwrap_or(path); diff --git a/codex-rs/sandboxing/src/policy_transforms_tests.rs b/codex-rs/sandboxing/src/policy_transforms_tests.rs index 30dbac924222..a70116cc3866 100644 --- a/codex-rs/sandboxing/src/policy_transforms_tests.rs +++ b/codex-rs/sandboxing/src/policy_transforms_tests.rs @@ -119,7 +119,7 @@ fn normalize_additional_permissions_preserves_network() { #[cfg(unix)] #[test] -fn normalize_additional_permissions_canonicalizes_symlinked_write_paths() { +fn normalize_additional_permissions_preserves_symlinked_write_paths() { let temp_dir = TempDir::new().expect("create temp dir"); let real_root = temp_dir.path().join("real"); let link_root = temp_dir.path().join("link"); @@ -129,11 +129,6 @@ fn normalize_additional_permissions_canonicalizes_symlinked_write_paths() { let link_write_dir = AbsolutePathBuf::from_absolute_path(link_root.join("write")).expect("link write dir"); - let expected_write_dir = AbsolutePathBuf::from_absolute_path( - write_dir.canonicalize().expect("canonicalize write dir"), - ) - .expect("absolute canonical write dir"); - let permissions = normalize_additional_permissions(PermissionProfile { file_system: Some(FileSystemPermissions { read: Some(vec![]), @@ -147,7 +142,10 @@ fn normalize_additional_permissions_canonicalizes_symlinked_write_paths() { permissions.file_system, Some(FileSystemPermissions { read: Some(vec![]), - write: Some(vec![expected_write_dir]), + write: Some(vec![ + AbsolutePathBuf::from_absolute_path(link_root.join("write")) + .expect("link write dir") + ]), }) ); } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index e291480d0bfb..5019c57f4f80 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -46,6 +46,7 @@ use codex_state::log_db; use codex_terminal_detection::Multiplexer; use codex_terminal_detection::terminal_info; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_absolute_path::canonicalize_preserving_symlinks; use codex_utils_oss::ensure_oss_provider_ready; use codex_utils_oss::get_default_model_for_oss_provider; use cwd_prompt::CwdPromptAction; @@ -321,7 +322,7 @@ pub async fn run_main( let cwd = cli.cwd.clone(); let config_cwd = match cwd.as_deref() { - Some(path) => AbsolutePathBuf::from_absolute_path(path.canonicalize()?)?, + Some(path) => AbsolutePathBuf::from_absolute_path(canonicalize_preserving_symlinks(path)?)?, None => AbsolutePathBuf::current_dir()?, }; diff --git a/codex-rs/tui_app_server/src/lib.rs b/codex-rs/tui_app_server/src/lib.rs index 22de8b38c994..d42c05b6c91e 100644 --- a/codex-rs/tui_app_server/src/lib.rs +++ b/codex-rs/tui_app_server/src/lib.rs @@ -52,6 +52,7 @@ use codex_state::log_db; use codex_terminal_detection::Multiplexer; use codex_terminal_detection::terminal_info; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_absolute_path::canonicalize_preserving_symlinks; use codex_utils_oss::ensure_oss_provider_ready; use codex_utils_oss::get_default_model_for_oss_provider; use color_eyre::eyre::WrapErr; @@ -684,7 +685,7 @@ pub async fn run_main( let cwd = cli.cwd.clone(); let config_cwd = match cwd.as_deref() { - Some(path) => AbsolutePathBuf::from_absolute_path(path.canonicalize()?)?, + Some(path) => AbsolutePathBuf::from_absolute_path(canonicalize_preserving_symlinks(path)?)?, None => AbsolutePathBuf::current_dir()?, }; diff --git a/codex-rs/utils/absolute-path/src/lib.rs b/codex-rs/utils/absolute-path/src/lib.rs index 0de0f4e023b0..b07ffc147e04 100644 --- a/codex-rs/utils/absolute-path/src/lib.rs +++ b/codex-rs/utils/absolute-path/src/lib.rs @@ -101,6 +101,27 @@ impl AbsolutePathBuf { } } +/// Canonicalize a path when possible, but preserve the logical absolute path +/// whenever canonicalization would rewrite it through a nested symlink. +/// +/// Top-level system aliases such as macOS `/var -> /private/var` still remain +/// canonicalized so existing runtime expectations around those paths stay +/// stable. +pub fn canonicalize_preserving_symlinks(path: &Path) -> std::io::Result { + let logical = AbsolutePathBuf::from_absolute_path(path)?.into_path_buf(); + let preserve_logical_path = logical.ancestors().any(|ancestor| { + let Ok(metadata) = std::fs::symlink_metadata(ancestor) else { + return false; + }; + metadata.file_type().is_symlink() && ancestor.parent().and_then(Path::parent).is_some() + }); + match std::fs::canonicalize(path) { + Ok(canonical) if preserve_logical_path && canonical != logical => Ok(logical), + Ok(canonical) => Ok(canonical), + Err(_) => Ok(logical), + } +} + impl AsRef for AbsolutePathBuf { fn as_ref(&self) -> &Path { &self.0 @@ -293,6 +314,37 @@ mod tests { assert_eq!(abs_path_buf.as_path(), home.join("code").as_path()); } + #[cfg(unix)] + #[test] + fn canonicalize_preserving_symlinks_keeps_logical_symlink_path() { + let temp_dir = tempdir().expect("temp dir"); + let real = temp_dir.path().join("real"); + let link = temp_dir.path().join("link"); + std::fs::create_dir_all(&real).expect("create real dir"); + std::os::unix::fs::symlink(&real, &link).expect("create symlink"); + + let canonicalized = + canonicalize_preserving_symlinks(&link).expect("canonicalize preserving symlinks"); + + assert_eq!(canonicalized, link); + } + + #[cfg(unix)] + #[test] + fn canonicalize_preserving_symlinks_keeps_logical_missing_child_under_symlink() { + let temp_dir = tempdir().expect("temp dir"); + let real = temp_dir.path().join("real"); + let link = temp_dir.path().join("link"); + std::fs::create_dir_all(&real).expect("create real dir"); + std::os::unix::fs::symlink(&real, &link).expect("create symlink"); + let missing = link.join("missing.txt"); + + let canonicalized = + canonicalize_preserving_symlinks(&missing).expect("canonicalize preserving symlinks"); + + assert_eq!(canonicalized, missing); + } + #[cfg(target_os = "windows")] #[test] fn home_directory_backslash_subpath_is_expanded_in_deserialization() { From 916b05eb751501e1f4c9565aecf58066f92e9f2d Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 27 Mar 2026 09:08:00 -0700 Subject: [PATCH 02/12] fix: preserve Windows path shape in symlink canonicalization Co-authored-by: Codex --- codex-rs/Cargo.lock | 1 + codex-rs/utils/absolute-path/Cargo.toml | 1 + codex-rs/utils/absolute-path/src/lib.rs | 20 +++++++++++++++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index c5846fbe1988..e1af9a6a35e5 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2832,6 +2832,7 @@ name = "codex-utils-absolute-path" version = "0.0.0" dependencies = [ "dirs", + "dunce", "path-absolutize", "pretty_assertions", "schemars 0.8.22", diff --git a/codex-rs/utils/absolute-path/Cargo.toml b/codex-rs/utils/absolute-path/Cargo.toml index f4fa8bf78907..c0b12ca67934 100644 --- a/codex-rs/utils/absolute-path/Cargo.toml +++ b/codex-rs/utils/absolute-path/Cargo.toml @@ -10,6 +10,7 @@ workspace = true [dependencies] dirs = { workspace = true } +dunce = { workspace = true } path-absolutize = { workspace = true } schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/codex-rs/utils/absolute-path/src/lib.rs b/codex-rs/utils/absolute-path/src/lib.rs index b07ffc147e04..2a6185e812ef 100644 --- a/codex-rs/utils/absolute-path/src/lib.rs +++ b/codex-rs/utils/absolute-path/src/lib.rs @@ -115,7 +115,7 @@ pub fn canonicalize_preserving_symlinks(path: &Path) -> std::io::Result }; metadata.file_type().is_symlink() && ancestor.parent().and_then(Path::parent).is_some() }); - match std::fs::canonicalize(path) { + match dunce::canonicalize(path) { Ok(canonical) if preserve_logical_path && canonical != logical => Ok(logical), Ok(canonical) => Ok(canonical), Err(_) => Ok(logical), @@ -360,4 +360,22 @@ mod tests { }; assert_eq!(abs_path_buf.as_path(), home.join("code").as_path()); } + + #[cfg(target_os = "windows")] + #[test] + fn canonicalize_preserving_symlinks_avoids_verbatim_prefixes() { + let temp_dir = tempdir().expect("temp dir"); + + let canonicalized = + canonicalize_preserving_symlinks(temp_dir.path()).expect("canonicalize"); + + assert_eq!( + canonicalized, + dunce::canonicalize(temp_dir.path()).expect("canonicalize temp dir") + ); + assert!( + !canonicalized.to_string_lossy().starts_with(r"\\?\"), + "expected a non-verbatim Windows path, got {canonicalized:?}" + ); + } } From 2ef215617bbe30d01d9e2a0cdda4d6f10fd7bffc Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 27 Mar 2026 13:59:27 -0700 Subject: [PATCH 03/12] test: normalize Windows protocol path expectations Co-authored-by: Codex --- codex-rs/protocol/src/permissions.rs | 3 +-- codex-rs/protocol/src/protocol.rs | 15 +++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 658746375914..3263b7e84c33 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -1928,8 +1928,7 @@ mod tests { let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); let expected_docs = AbsolutePathBuf::from_absolute_path( - cwd.path() - .canonicalize() + canonicalize_preserving_symlinks(cwd.path()) .expect("canonicalize cwd") .join("docs"), ) diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 8b38a5050fd5..970bb171fe1d 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -3982,7 +3982,8 @@ mod tests { #[test] fn restricted_file_system_policy_treats_root_with_carveouts_as_scoped_access() { let cwd = TempDir::new().expect("tempdir"); - let canonical_cwd = cwd.path().canonicalize().expect("canonicalize cwd"); + let canonical_cwd = codex_utils_absolute_path::canonicalize_preserving_symlinks(cwd.path()) + .expect("canonicalize cwd"); let root = AbsolutePathBuf::from_absolute_path(&canonical_cwd) .expect("absolute canonical tempdir") .as_path() @@ -3993,8 +3994,7 @@ mod tests { let blocked = AbsolutePathBuf::resolve_path_against_base("blocked", cwd.path()) .expect("resolve blocked"); let expected_blocked = AbsolutePathBuf::from_absolute_path( - cwd.path() - .canonicalize() + codex_utils_absolute_path::canonicalize_preserving_symlinks(cwd.path()) .expect("canonicalize cwd") .join("blocked"), ) @@ -4039,7 +4039,8 @@ mod tests { let cwd = TempDir::new().expect("tempdir"); std::fs::create_dir_all(cwd.path().join(".agents")).expect("create .agents"); std::fs::create_dir_all(cwd.path().join(".codex")).expect("create .codex"); - let canonical_cwd = cwd.path().canonicalize().expect("canonicalize cwd"); + let canonical_cwd = codex_utils_absolute_path::canonicalize_preserving_symlinks(cwd.path()) + .expect("canonicalize cwd"); let cwd_absolute = AbsolutePathBuf::from_absolute_path(&canonical_cwd).expect("absolute tempdir"); let secret = AbsolutePathBuf::resolve_path_against_base("secret", cwd.path()) @@ -4107,7 +4108,8 @@ mod tests { #[test] fn restricted_file_system_policy_treats_read_entries_as_read_only_subpaths() { let cwd = TempDir::new().expect("tempdir"); - let canonical_cwd = cwd.path().canonicalize().expect("canonicalize cwd"); + let canonical_cwd = codex_utils_absolute_path::canonicalize_preserving_symlinks(cwd.path()) + .expect("canonicalize cwd"); let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); let docs_public = AbsolutePathBuf::resolve_path_against_base("docs/public", cwd.path()) @@ -4157,7 +4159,8 @@ mod tests { let cwd = TempDir::new().expect("tempdir"); let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); - let canonical_cwd = cwd.path().canonicalize().expect("canonicalize cwd"); + let canonical_cwd = codex_utils_absolute_path::canonicalize_preserving_symlinks(cwd.path()) + .expect("canonicalize cwd"); let expected_dot_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex")) .expect("canonical .codex"); let policy = SandboxPolicy::WorkspaceWrite { From 4843065c98c1b5a5b3ead88f18ae8047927e655c Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 27 Mar 2026 15:31:22 -0700 Subject: [PATCH 04/12] test: update Windows restricted-token overlay expectation Co-authored-by: Codex --- codex-rs/core/src/exec_tests.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 07532e5a18e9..d8ffe960fea6 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -595,14 +595,8 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() { }, ]); - #[cfg(windows)] - let expected_deny_write_paths = vec![ - codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(cwd.join(".codex")) - .expect("absolute .codex"), - codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&docs) - .expect("absolute docs"), - ]; - #[cfg(not(windows))] + // The legacy workspace-write root already protects top-level `.codex`, so + // the restricted-token overlay only needs the extra read-only docs carveout. let expected_deny_write_paths = vec![ codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(&docs) .expect("absolute docs"), From b4c746fb8c6f1380933e8fd31fe46a9af5eb0b96 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 7 Apr 2026 10:07:19 -0700 Subject: [PATCH 05/12] fix: reject missing symlink-preserving cwd Co-authored-by: Codex noreply@openai.com --- codex-rs/exec/src/lib.rs | 6 ++- codex-rs/tui/src/lib.rs | 20 +++++++- codex-rs/utils/absolute-path/src/lib.rs | 61 ++++++++++++++++++++++--- 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 2b8f064c27e6..61e38fe6813f 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -82,7 +82,7 @@ use codex_protocol::protocol::SessionConfiguredEvent; use codex_protocol::protocol::SessionSource; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_absolute_path::canonicalize_preserving_symlinks; +use codex_utils_absolute_path::canonicalize_existing_preserving_symlinks; use codex_utils_oss::ensure_oss_provider_ready; use codex_utils_oss::get_default_model_for_oss_provider; use event_processor_with_human_output::EventProcessorWithHumanOutput; @@ -243,7 +243,9 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result let resolved_cwd = cwd.clone(); let config_cwd = match resolved_cwd.as_deref() { - Some(path) => AbsolutePathBuf::from_absolute_path(canonicalize_preserving_symlinks(path)?)?, + Some(path) => { + AbsolutePathBuf::from_absolute_path(canonicalize_existing_preserving_symlinks(path)?)? + } None => AbsolutePathBuf::current_dir()?, }; diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index a506a471033b..c1d487e6c37c 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -51,7 +51,7 @@ use codex_rollout::state_db::get_state_db; use codex_state::log_db; use codex_terminal_detection::terminal_info; use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_absolute_path::canonicalize_preserving_symlinks; +use codex_utils_absolute_path::canonicalize_existing_preserving_symlinks; use codex_utils_oss::ensure_oss_provider_ready; use codex_utils_oss::get_default_model_for_oss_provider; use color_eyre::eyre::WrapErr; @@ -592,7 +592,9 @@ fn config_cwd_for_app_server_target( } match cwd { - Some(path) => AbsolutePathBuf::from_absolute_path(canonicalize_preserving_symlinks(path)?), + Some(path) => { + AbsolutePathBuf::from_absolute_path(canonicalize_existing_preserving_symlinks(path)?) + } None => AbsolutePathBuf::current_dir(), } } @@ -1909,6 +1911,20 @@ mod tests { Ok(()) } + #[test] + fn config_cwd_for_app_server_target_errors_for_missing_embedded_cli_cwd() -> std::io::Result<()> + { + let temp_dir = TempDir::new()?; + let missing = temp_dir.path().join("missing"); + let target = AppServerTarget::Embedded; + + let err = config_cwd_for_app_server_target(Some(&missing), &target) + .expect_err("missing embedded cwd should fail"); + + assert_eq!(err.kind(), std::io::ErrorKind::NotFound); + Ok(()) + } + #[tokio::test] async fn read_session_cwd_returns_none_without_sqlite_or_rollout_path() -> std::io::Result<()> { let temp_dir = TempDir::new()?; diff --git a/codex-rs/utils/absolute-path/src/lib.rs b/codex-rs/utils/absolute-path/src/lib.rs index 2a6185e812ef..5815390550dc 100644 --- a/codex-rs/utils/absolute-path/src/lib.rs +++ b/codex-rs/utils/absolute-path/src/lib.rs @@ -106,15 +106,12 @@ impl AbsolutePathBuf { /// /// Top-level system aliases such as macOS `/var -> /private/var` still remain /// canonicalized so existing runtime expectations around those paths stay -/// stable. +/// stable. If the full path cannot be canonicalized, this returns the logical +/// absolute path; use [`canonicalize_existing_preserving_symlinks`] for paths +/// that must exist. pub fn canonicalize_preserving_symlinks(path: &Path) -> std::io::Result { let logical = AbsolutePathBuf::from_absolute_path(path)?.into_path_buf(); - let preserve_logical_path = logical.ancestors().any(|ancestor| { - let Ok(metadata) = std::fs::symlink_metadata(ancestor) else { - return false; - }; - metadata.file_type().is_symlink() && ancestor.parent().and_then(Path::parent).is_some() - }); + let preserve_logical_path = should_preserve_logical_path(&logical); match dunce::canonicalize(path) { Ok(canonical) if preserve_logical_path && canonical != logical => Ok(logical), Ok(canonical) => Ok(canonical), @@ -122,6 +119,30 @@ pub fn canonicalize_preserving_symlinks(path: &Path) -> std::io::Result } } +/// Canonicalize an existing path while preserving the logical absolute path +/// whenever canonicalization would rewrite it through a nested symlink. +/// +/// Unlike [`canonicalize_preserving_symlinks`], canonicalization failures are +/// propagated so callers can reject invalid working directories early. +pub fn canonicalize_existing_preserving_symlinks(path: &Path) -> std::io::Result { + let logical = AbsolutePathBuf::from_absolute_path(path)?.into_path_buf(); + let canonical = dunce::canonicalize(path)?; + if should_preserve_logical_path(&logical) && canonical != logical { + Ok(logical) + } else { + Ok(canonical) + } +} + +fn should_preserve_logical_path(logical: &Path) -> bool { + logical.ancestors().any(|ancestor| { + let Ok(metadata) = std::fs::symlink_metadata(ancestor) else { + return false; + }; + metadata.file_type().is_symlink() && ancestor.parent().and_then(Path::parent).is_some() + }) +} + impl AsRef for AbsolutePathBuf { fn as_ref(&self) -> &Path { &self.0 @@ -345,6 +366,32 @@ mod tests { assert_eq!(canonicalized, missing); } + #[test] + fn canonicalize_existing_preserving_symlinks_errors_for_missing_path() { + let temp_dir = tempdir().expect("temp dir"); + let missing = temp_dir.path().join("missing"); + + let err = canonicalize_existing_preserving_symlinks(&missing) + .expect_err("missing path should fail canonicalization"); + + assert_eq!(err.kind(), std::io::ErrorKind::NotFound); + } + + #[cfg(unix)] + #[test] + fn canonicalize_existing_preserving_symlinks_keeps_logical_symlink_path() { + let temp_dir = tempdir().expect("temp dir"); + let real = temp_dir.path().join("real"); + let link = temp_dir.path().join("link"); + std::fs::create_dir_all(&real).expect("create real dir"); + std::os::unix::fs::symlink(&real, &link).expect("create symlink"); + + let canonicalized = + canonicalize_existing_preserving_symlinks(&link).expect("canonicalize symlink"); + + assert_eq!(canonicalized, link); + } + #[cfg(target_os = "windows")] #[test] fn home_directory_backslash_subpath_is_expanded_in_deserialization() { From 78e49812641926e9bc31749f642eead6f9e08261 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 9 Apr 2026 17:05:09 -0700 Subject: [PATCH 06/12] fix: update linux sandbox symlink tests for infallible joins Co-authored-by: Codex noreply@openai.com --- codex-rs/linux-sandbox/src/bwrap.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 45805435f39b..481648c22f61 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -768,7 +768,7 @@ mod tests { let link_root = AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root"); - let link_blocked = link_root.join("blocked").expect("symlinked blocked"); + let link_blocked = link_root.join("blocked"); let real_root_str = path_to_string(&real_root); let real_blocked_str = path_to_string(&blocked); let policy = FileSystemSandboxPolicy::restricted(vec![ @@ -816,9 +816,7 @@ mod tests { let link_root = AbsolutePathBuf::from_absolute_path(&link_root).expect("absolute symlinked root"); - let link_private = link_root - .join("linked-private") - .expect("symlinked linked-private"); + let link_private = link_root.join("linked-private"); let real_linked_private_str = path_to_string(&linked_private); let outside_str = path_to_string(&outside); let policy = FileSystemSandboxPolicy::restricted(vec![ From 5dd22f89b185613c04e82694811efdef94bc3f10 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 9 Apr 2026 18:10:18 -0700 Subject: [PATCH 07/12] test: normalize tui cwd expectation on windows Co-authored-by: Codex noreply@openai.com --- codex-rs/tui/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 410b380ab237..765dd32eb808 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -1927,9 +1927,9 @@ mod tests { assert_eq!( config_cwd, - Some(AbsolutePathBuf::from_absolute_path( - temp_dir.path().canonicalize()? - )?) + Some(AbsolutePathBuf::from_absolute_path(dunce::canonicalize( + temp_dir.path() + )?)?) ); Ok(()) } From e64a32eb05e63c88e6d288ee706c7c645e2ee44f Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 9 Apr 2026 20:12:27 -0700 Subject: [PATCH 08/12] fix: handle symlinked ancestors in bwrap writable roots Co-authored-by: Codex noreply@openai.com --- codex-rs/linux-sandbox/src/bwrap.rs | 121 +++++++++++++++++++++++++--- 1 file changed, 110 insertions(+), 11 deletions(-) diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 481648c22f61..76c896ab9339 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -287,7 +287,7 @@ fn create_filesystem_args( for writable_root in &writable_roots { let root = writable_root.root.as_path(); allowed_write_paths.push(root.to_path_buf()); - if let Some(target) = symlink_target(root) { + if let Some(target) = canonical_target_if_symlinked_path(root) { allowed_write_paths.push(target); } } @@ -326,7 +326,7 @@ fn create_filesystem_args( for writable_root in &sorted_writable_roots { let root = writable_root.root.as_path(); - let symlink_target = symlink_target(root); + let symlink_target = canonical_target_if_symlinked_path(root); // 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 @@ -410,16 +410,37 @@ fn path_depth(path: &Path) -> usize { path.components().count() } -fn symlink_target(root: &Path) -> Option { - let meta = fs::symlink_metadata(root).ok()?; - if !meta.file_type().is_symlink() { - return None; - } - let target = fs::canonicalize(root).ok()?; - if target.as_path() == root { - return None; +fn canonical_target_if_symlinked_path(path: &Path) -> Option { + let mut current = PathBuf::new(); + for component in path.components() { + use std::path::Component; + match component { + Component::RootDir => { + current.push(Path::new("/")); + continue; + } + Component::CurDir => continue, + Component::ParentDir => { + current.pop(); + continue; + } + Component::Normal(part) => current.push(part), + Component::Prefix(_) => continue, + } + + let metadata = match fs::symlink_metadata(¤t) { + Ok(metadata) => metadata, + Err(_) => return None, + }; + if metadata.file_type().is_symlink() { + let target = fs::canonicalize(path).ok()?; + if target.as_path() == path { + return None; + } + return Some(target); + } } - Some(target) + None } fn remap_paths_for_symlink_target(paths: Vec, root: &Path, target: &Path) -> Vec { @@ -800,6 +821,84 @@ mod tests { })); } + #[cfg(unix)] + #[test] + fn writable_roots_under_symlinked_ancestors_bind_real_target() { + let temp_dir = TempDir::new().expect("temp dir"); + let logical_home = temp_dir.path().join("home"); + let real_codex = temp_dir.path().join("real-codex"); + let logical_codex = logical_home.join(".codex"); + let real_memories = real_codex.join("memories"); + let logical_memories = logical_codex.join("memories"); + std::fs::create_dir_all(&logical_home).expect("create logical home"); + std::fs::create_dir_all(&real_memories).expect("create memories dir"); + std::os::unix::fs::symlink(&real_codex, &logical_codex) + .expect("create symlinked codex home"); + + let logical_memories_root = + AbsolutePathBuf::from_absolute_path(&logical_memories).expect("absolute memories"); + let real_memories_str = path_to_string(&real_memories); + let logical_memories_str = path_to_string(&logical_memories); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: logical_memories_root, + }, + access: FileSystemAccessMode::Write, + }]); + + let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + + assert!(args.args.windows(3).any(|window| { + window + == [ + "--bind", + real_memories_str.as_str(), + real_memories_str.as_str(), + ] + })); + assert!(!args.args.windows(3).any(|window| { + window + == [ + "--bind", + logical_memories_str.as_str(), + logical_memories_str.as_str(), + ] + })); + } + + #[cfg(unix)] + #[test] + fn protected_symlinked_directory_subpaths_mask_symlink_inode() { + let temp_dir = TempDir::new().expect("temp dir"); + let root = temp_dir.path().join("root"); + let agents_target = root.join("agents-target"); + let agents_link = root.join(".agents"); + std::fs::create_dir_all(&agents_target).expect("create agents target"); + std::os::unix::fs::symlink(&agents_target, &agents_link).expect("create symlinked .agents"); + + let root = AbsolutePathBuf::from_absolute_path(&root).expect("absolute root"); + let agents_link_str = path_to_string(&agents_link); + let agents_target_str = path_to_string(&agents_target); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Path { path: root }, + access: FileSystemAccessMode::Write, + }]); + + let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + + assert!( + args.args + .windows(3) + .any(|window| { window == ["--ro-bind", "/dev/null", agents_link_str.as_str()] }) + ); + assert!( + !args + .args + .iter() + .any(|arg| arg == agents_target_str.as_str()) + ); + } + #[cfg(unix)] #[test] fn symlinked_writable_roots_mask_nested_symlink_escape_paths_without_binding_targets() { From 704782c037dd52aeba7a59a5bd9ac10b187f9916 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 9 Apr 2026 20:14:43 -0700 Subject: [PATCH 09/12] fix: handle symlinked protected bwrap targets Co-authored-by: Codex noreply@openai.com --- codex-rs/linux-sandbox/src/bwrap.rs | 86 +++++++++++++++++++---------- 1 file changed, 56 insertions(+), 30 deletions(-) diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 76c896ab9339..b96dcaba6feb 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -487,10 +487,10 @@ fn append_read_only_subpath_args( subpath: &Path, allowed_write_paths: &[PathBuf], ) { - if let Some(symlink_path) = find_symlink_in_path(subpath, allowed_write_paths) { + if let Some(target) = canonical_target_for_symlink_in_path(subpath, allowed_write_paths) { args.push("--ro-bind".to_string()); - args.push("/dev/null".to_string()); - args.push(path_to_string(&symlink_path)); + args.push(path_to_string(&target)); + args.push(path_to_string(&target)); return; } @@ -518,11 +518,14 @@ fn append_unreadable_root_args( unreadable_root: &Path, allowed_write_paths: &[PathBuf], ) -> Result<()> { - if let Some(symlink_path) = find_symlink_in_path(unreadable_root, allowed_write_paths) { - args.push("--ro-bind".to_string()); - args.push("/dev/null".to_string()); - args.push(path_to_string(&symlink_path)); - return Ok(()); + if let Some(target) = canonical_target_for_symlink_in_path(unreadable_root, allowed_write_paths) + { + return append_existing_unreadable_path_args( + args, + preserved_files, + &target, + allowed_write_paths, + ); } if !unreadable_root.exists() { @@ -536,6 +539,20 @@ fn append_unreadable_root_args( return Ok(()); } + append_existing_unreadable_path_args( + args, + preserved_files, + unreadable_root, + allowed_write_paths, + ) +} + +fn append_existing_unreadable_path_args( + args: &mut Vec, + preserved_files: &mut Vec, + unreadable_root: &Path, + allowed_write_paths: &[PathBuf], +) -> Result<()> { if unreadable_root.is_dir() { let mut writable_descendants: Vec<&Path> = allowed_write_paths .iter() @@ -585,12 +602,10 @@ fn is_within_allowed_write_paths(path: &Path, allowed_write_paths: &[PathBuf]) - .any(|root| path.starts_with(root)) } -/// Find the first symlink along `target_path` that is also under a writable root. -/// -/// This blocks symlink replacement attacks where a protected path is a symlink -/// inside a writable root (e.g., `.codex -> ./decoy`). In that case we mount -/// `/dev/null` on the symlink itself to prevent rewiring it. -fn find_symlink_in_path(target_path: &Path, allowed_write_paths: &[PathBuf]) -> Option { +fn canonical_target_for_symlink_in_path( + target_path: &Path, + allowed_write_paths: &[PathBuf], +) -> Option { let mut current = PathBuf::new(); for component in target_path.components() { @@ -617,7 +632,7 @@ fn find_symlink_in_path(target_path: &Path, allowed_write_paths: &[PathBuf]) -> if metadata.file_type().is_symlink() && is_within_allowed_write_paths(¤t, allowed_write_paths) { - return Some(current); + return fs::canonicalize(¤t).ok(); } } @@ -868,7 +883,7 @@ mod tests { #[cfg(unix)] #[test] - fn protected_symlinked_directory_subpaths_mask_symlink_inode() { + fn protected_symlinked_directory_subpaths_bind_target_read_only() { let temp_dir = TempDir::new().expect("temp dir"); let root = temp_dir.path().join("root"); let agents_target = root.join("agents-target"); @@ -886,17 +901,15 @@ mod tests { let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); - assert!( - args.args - .windows(3) - .any(|window| { window == ["--ro-bind", "/dev/null", agents_link_str.as_str()] }) - ); - assert!( - !args - .args - .iter() - .any(|arg| arg == agents_target_str.as_str()) - ); + assert!(args.args.windows(3).any(|window| { + window + == [ + "--ro-bind", + agents_target_str.as_str(), + agents_target_str.as_str(), + ] + })); + assert!(!args.args.iter().any(|arg| arg == agents_link_str.as_str())); } #[cfg(unix)] @@ -931,10 +944,23 @@ mod tests { let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); - assert!(args.args.windows(3).any(|window| { - window == ["--ro-bind", "/dev/null", real_linked_private_str.as_str()] + assert!(args.args.windows(6).any(|window| { + window + == [ + "--perms", + "000", + "--tmpfs", + outside_str.as_str(), + "--remount-ro", + outside_str.as_str(), + ] })); - assert!(!args.args.iter().any(|arg| arg == outside_str.as_str())); + assert!( + !args + .args + .iter() + .any(|arg| arg == real_linked_private_str.as_str()) + ); } #[test] From fb9e39caaa279a7133e6f73d39b4bc31614ae9eb Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 9 Apr 2026 20:21:23 -0700 Subject: [PATCH 10/12] docs: clarify symlinked bwrap bind operands Co-authored-by: Codex noreply@openai.com --- codex-rs/linux-sandbox/src/bwrap.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index b96dcaba6feb..2b7d3dc229c7 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -488,9 +488,14 @@ fn append_read_only_subpath_args( allowed_write_paths: &[PathBuf], ) { if let Some(target) = canonical_target_for_symlink_in_path(subpath, allowed_write_paths) { + // bwrap takes `--ro-bind `. Use the resolved target + // for both operands so a protected symlinked directory is remounted + // read-only in place instead of binding onto the symlink path itself. + let mount_source = path_to_string(&target); + let mount_destination = path_to_string(&target); args.push("--ro-bind".to_string()); - args.push(path_to_string(&target)); - args.push(path_to_string(&target)); + args.push(mount_source); + args.push(mount_destination); return; } @@ -520,6 +525,8 @@ fn append_unreadable_root_args( ) -> Result<()> { if let Some(target) = canonical_target_for_symlink_in_path(unreadable_root, allowed_write_paths) { + // Apply unreadable handling to the resolved symlink target, not the + // logical symlink path, to avoid file-vs-directory bind mismatches. return append_existing_unreadable_path_args( args, preserved_files, From 19185575bb0ead7feaea37a38b0b090c9a21206b Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 9 Apr 2026 20:30:55 -0700 Subject: [PATCH 11/12] fix: bind symlinked readable roots by target Co-authored-by: Codex noreply@openai.com --- codex-rs/linux-sandbox/src/bwrap.rs | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 2b7d3dc229c7..8f7b7d2894a1 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -274,9 +274,12 @@ fn create_filesystem_args( if !root.exists() { continue; } + // In restricted-read sandboxes the tmpfs root does not provide + // symlink aliases, so bind the real target just like writable roots. + let mount_root = canonical_target_if_symlinked_path(&root).unwrap_or(root); args.push("--ro-bind".to_string()); - args.push(path_to_string(&root)); - args.push(path_to_string(&root)); + args.push(path_to_string(&mount_root)); + args.push(path_to_string(&mount_root)); } } @@ -783,7 +786,13 @@ mod tests { BwrapOptions::default(), ) .expect("create bwrap args"); + let canonical_sandbox_cwd = path_to_string( + &real_root + .canonicalize() + .expect("canonicalize sandbox policy cwd"), + ); let canonical_command_cwd = path_to_string(&canonical_command_cwd); + let link_sandbox_cwd = path_to_string(&link_root); let link_command_cwd = path_to_string(&command_cwd); assert!( @@ -791,12 +800,28 @@ mod tests { .windows(2) .any(|window| { window == ["--chdir", canonical_command_cwd.as_str()] }) ); + assert!(args.args.windows(3).any(|window| { + window + == [ + "--ro-bind", + canonical_sandbox_cwd.as_str(), + canonical_sandbox_cwd.as_str(), + ] + })); assert!( !args .args .windows(2) .any(|window| { window == ["--chdir", link_command_cwd.as_str()] }) ); + assert!(!args.args.windows(3).any(|window| { + window + == [ + "--ro-bind", + link_sandbox_cwd.as_str(), + link_sandbox_cwd.as_str(), + ] + })); } #[cfg(unix)] From f6f6b61e7256063a8acd59e0644ab33b462a0ed5 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 9 Apr 2026 20:49:03 -0700 Subject: [PATCH 12/12] fix: keep read-only bwrap helper paths logical Co-authored-by: Codex noreply@openai.com --- codex-rs/linux-sandbox/src/bwrap.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 8f7b7d2894a1..91d66210291b 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -274,9 +274,18 @@ fn create_filesystem_args( if !root.exists() { continue; } - // In restricted-read sandboxes the tmpfs root does not provide - // symlink aliases, so bind the real target just like writable roots. - let mount_root = canonical_target_if_symlinked_path(&root).unwrap_or(root); + // Writable roots are rebound by real target below; mirror that + // for their restricted-read bootstrap mount. Plain read-only + // roots must stay logical because callers may execute those + // paths inside bwrap, such as Bazel runfiles helper binaries. + let mount_root = if writable_roots + .iter() + .any(|writable_root| root.starts_with(writable_root.root.as_path())) + { + canonical_target_if_symlinked_path(&root).unwrap_or(root) + } else { + root + }; args.push("--ro-bind".to_string()); args.push(path_to_string(&mount_root)); args.push(path_to_string(&mount_root));