From eebbcbb23ab1fcb75fcc64de3500755164951f1f Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 13 Mar 2026 23:58:23 -0700 Subject: [PATCH 1/9] fix(sandbox): normalize effective runtime roots --- codex-rs/core/src/seatbelt_tests.rs | 14 +++- codex-rs/protocol/src/permissions.rs | 116 +++++++++++++++++++++++++-- codex-rs/protocol/src/protocol.rs | 63 +++++++++------ 3 files changed, 157 insertions(+), 36 deletions(-) diff --git a/codex-rs/core/src/seatbelt_tests.rs b/codex-rs/core/src/seatbelt_tests.rs index 9ac5eaa7b02..bab1362017e 100644 --- a/codex-rs/core/src/seatbelt_tests.rs +++ b/codex-rs/core/src/seatbelt_tests.rs @@ -126,6 +126,8 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access() ); let policy = seatbelt_policy_arg(&args); + let unreadable_roots = file_system_policy.get_unreadable_roots_with_cwd(Path::new("/")); + let unreadable_root = unreadable_roots.first().expect("expected unreadable root"); assert!( policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_RO_0\")))"), "expected read carveout in policy:\n{policy}" @@ -136,12 +138,12 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access() ); assert!( args.iter() - .any(|arg| arg == "-DREADABLE_ROOT_0_RO_0=/tmp/codex-unreadable"), + .any(|arg| arg == &format!("-DREADABLE_ROOT_0_RO_0={}", unreadable_root.display())), "expected read carveout parameter in args: {args:#?}" ); assert!( args.iter() - .any(|arg| arg == "-DWRITABLE_ROOT_0_RO_0=/tmp/codex-unreadable"), + .any(|arg| arg == &format!("-DWRITABLE_ROOT_0_RO_0={}", unreadable_root.display())), "expected write carveout parameter in args: {args:#?}" ); } @@ -172,18 +174,22 @@ fn explicit_unreadable_paths_are_excluded_from_readable_roots() { ); let policy = seatbelt_policy_arg(&args); + let readable_roots = file_system_policy.get_readable_roots_with_cwd(Path::new("/")); + let readable_root = readable_roots.first().expect("expected readable root"); + let unreadable_roots = file_system_policy.get_unreadable_roots_with_cwd(Path::new("/")); + let unreadable_root = unreadable_roots.first().expect("expected unreadable root"); assert!( policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_RO_0\")))"), "expected read carveout in policy:\n{policy}" ); assert!( args.iter() - .any(|arg| arg == "-DREADABLE_ROOT_0=/tmp/codex-readable"), + .any(|arg| arg == &format!("-DREADABLE_ROOT_0={}", readable_root.display())), "expected readable root parameter in args: {args:#?}" ); assert!( args.iter() - .any(|arg| arg == "-DREADABLE_ROOT_0_RO_0=/tmp/codex-readable/private"), + .any(|arg| arg == &format!("-DREADABLE_ROOT_0_RO_0={}", unreadable_root.display())), "expected read carveout parameter in args: {args:#?}" ); } diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 6c2e7d336fb..4eb91de29af 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -371,7 +371,7 @@ impl FileSystemSandboxPolicy { return Vec::new(); } - dedup_absolute_paths( + dedup_normalized_absolute_paths( self.resolved_entries_with_cwd(cwd) .into_iter() .filter(|entry| entry.access.can_read()) @@ -389,7 +389,7 @@ impl FileSystemSandboxPolicy { } let resolved_entries = self.resolved_entries_with_cwd(cwd); - let read_only_roots = dedup_absolute_paths( + let read_only_roots = dedup_normalized_absolute_paths( resolved_entries .iter() .filter(|entry| !entry.access.can_write()) @@ -398,7 +398,7 @@ impl FileSystemSandboxPolicy { .collect(), ); - dedup_absolute_paths( + dedup_normalized_absolute_paths( resolved_entries .into_iter() .filter(|entry| entry.access.can_write()) @@ -421,7 +421,7 @@ impl FileSystemSandboxPolicy { ); WritableRoot { root, - read_only_subpaths: dedup_absolute_paths(read_only_subpaths), + read_only_subpaths: dedup_normalized_absolute_paths(read_only_subpaths), } }) .collect() @@ -437,7 +437,7 @@ impl FileSystemSandboxPolicy { .ok() .map(|cwd| absolute_root_path_for_cwd(&cwd)); - dedup_absolute_paths( + dedup_normalized_absolute_paths( self.resolved_entries_with_cwd(cwd) .iter() .filter(|entry| entry.access == FileSystemAccessMode::None) @@ -933,6 +933,33 @@ fn dedup_absolute_paths(paths: Vec) -> Vec { deduped } +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 { + continue; + }; + let Ok(suffix) = raw_path.strip_prefix(ancestor) else { + continue; + }; + if let Ok(normalized_path) = + AbsolutePathBuf::from_absolute_path(canonical_ancestor.join(suffix)) + { + return normalized_path; + } + } + path +} + +fn dedup_normalized_absolute_paths(paths: Vec) -> Vec { + dedup_absolute_paths( + paths + .into_iter() + .map(normalize_effective_absolute_path) + .collect(), + ) +} + fn default_read_only_subpaths_for_writable_root( writable_root: &AbsolutePathBuf, ) -> Vec { @@ -1038,8 +1065,15 @@ fn resolve_gitdir_from_file(dot_git: &AbsolutePathBuf) -> Option std::io::Result<()> { + std::os::unix::fs::symlink(original, link) + } + #[test] fn unknown_special_paths_are_ignored_by_legacy_bridge() -> std::io::Result<()> { let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { @@ -1067,6 +1101,66 @@ mod tests { Ok(()) } + #[cfg(unix)] + #[test] + fn effective_runtime_roots_canonicalize_symlinked_paths() { + let cwd = TempDir::new().expect("tempdir"); + let real_root = cwd.path().join("real"); + let link_root = cwd.path().join("link"); + let blocked = real_root.join("blocked"); + let codex_dir = real_root.join(".codex"); + + fs::create_dir_all(&blocked).expect("create blocked"); + fs::create_dir_all(&codex_dir).expect("create .codex"); + symlink_dir(&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 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 policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: link_root }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: link_blocked }, + access: FileSystemAccessMode::None, + }, + ]); + + assert_eq!( + policy.get_unreadable_roots_with_cwd(cwd.path()), + vec![expected_blocked.clone()] + ); + + let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); + assert_eq!(writable_roots.len(), 1); + assert_eq!(writable_roots[0].root, expected_root); + assert!( + writable_roots[0] + .read_only_subpaths + .contains(&expected_blocked) + ); + assert!( + writable_roots[0] + .read_only_subpaths + .contains(&expected_codex) + ); + } + #[test] fn resolve_access_with_cwd_uses_most_specific_entry() { let cwd = TempDir::new().expect("tempdir"); @@ -1183,6 +1277,13 @@ mod tests { let cwd = TempDir::new().expect("tempdir"); let docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); + let expected_docs = AbsolutePathBuf::from_absolute_path( + cwd.path() + .canonicalize() + .expect("canonicalize cwd") + .join("docs"), + ) + .expect("canonical docs"); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -1200,7 +1301,10 @@ mod tests { policy.resolve_access_with_cwd(docs.as_path(), cwd.path()), FileSystemAccessMode::Read ); - assert_eq!(policy.get_readable_roots_with_cwd(cwd.path()), vec![docs]); + assert_eq!( + policy.get_readable_roots_with_cwd(cwd.path()), + vec![expected_docs] + ); assert!(policy.get_unreadable_roots_with_cwd(cwd.path()).is_empty()); } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 8b5d490a2ba..f48e67fb163 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -3685,6 +3685,13 @@ mod tests { .expect("filesystem root"); let blocked = AbsolutePathBuf::resolve_path_against_base("blocked", cwd.path()) .expect("resolve blocked"); + let expected_blocked = AbsolutePathBuf::from_absolute_path( + cwd.path() + .canonicalize() + .expect("canonicalize cwd") + .join("blocked"), + ) + .expect("canonical blocked"); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -3693,9 +3700,7 @@ mod tests { access: FileSystemAccessMode::Write, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { - path: blocked.clone(), - }, + path: FileSystemPath::Path { path: blocked }, access: FileSystemAccessMode::None, }, ]); @@ -3708,7 +3713,7 @@ mod tests { ); assert_eq!( policy.get_unreadable_roots_with_cwd(cwd.path()), - vec![blocked.clone()] + vec![expected_blocked.clone()] ); let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); @@ -3718,7 +3723,7 @@ mod tests { writable_roots[0] .read_only_subpaths .iter() - .any(|path| path.as_path() == blocked.as_path()) + .any(|path| path.as_path() == expected_blocked.as_path()) ); } @@ -3727,14 +3732,17 @@ 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 cwd_absolute = - AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute tempdir"); + AbsolutePathBuf::from_absolute_path(&canonical_cwd).expect("absolute tempdir"); let secret = AbsolutePathBuf::resolve_path_against_base("secret", cwd.path()) .expect("resolve unreadable path"); - let agents = AbsolutePathBuf::resolve_path_against_base(".agents", cwd.path()) - .expect("resolve .agents"); - let codex = AbsolutePathBuf::resolve_path_against_base(".codex", cwd.path()) - .expect("resolve .codex"); + let expected_secret = AbsolutePathBuf::from_absolute_path(canonical_cwd.join("secret")) + .expect("canonical secret"); + let expected_agents = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".agents")) + .expect("canonical .agents"); + let expected_codex = AbsolutePathBuf::from_absolute_path(canonical_cwd.join(".codex")) + .expect("canonical .codex"); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -3749,9 +3757,7 @@ mod tests { access: FileSystemAccessMode::Write, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { - path: secret.clone(), - }, + path: FileSystemPath::Path { path: secret }, access: FileSystemAccessMode::None, }, ]); @@ -3761,43 +3767,49 @@ mod tests { assert!(policy.include_platform_defaults()); assert_eq!( policy.get_readable_roots_with_cwd(cwd.path()), - vec![cwd_absolute] + vec![cwd_absolute.clone()] ); assert_eq!( policy.get_unreadable_roots_with_cwd(cwd.path()), - vec![secret.clone()] + vec![expected_secret.clone()] ); let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); assert_eq!(writable_roots.len(), 1); - assert_eq!(writable_roots[0].root.as_path(), cwd.path()); + assert_eq!(writable_roots[0].root, cwd_absolute); assert!( writable_roots[0] .read_only_subpaths .iter() - .any(|path| path.as_path() == secret.as_path()) + .any(|path| path.as_path() == expected_secret.as_path()) ); assert!( writable_roots[0] .read_only_subpaths .iter() - .any(|path| path.as_path() == agents.as_path()) + .any(|path| path.as_path() == expected_agents.as_path()) ); assert!( writable_roots[0] .read_only_subpaths .iter() - .any(|path| path.as_path() == codex.as_path()) + .any(|path| path.as_path() == expected_codex.as_path()) ); } #[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 docs = AbsolutePathBuf::resolve_path_against_base("docs", cwd.path()).expect("resolve docs"); let docs_public = AbsolutePathBuf::resolve_path_against_base("docs/public", cwd.path()) .expect("resolve docs/public"); + let expected_docs = AbsolutePathBuf::from_absolute_path(canonical_cwd.join("docs")) + .expect("canonical docs"); + let expected_docs_public = + AbsolutePathBuf::from_absolute_path(canonical_cwd.join("docs/public")) + .expect("canonical docs/public"); let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -3806,13 +3818,11 @@ mod tests { access: FileSystemAccessMode::Write, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: docs.clone() }, + path: FileSystemPath::Path { path: docs }, access: FileSystemAccessMode::Read, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { - path: docs_public.clone(), - }, + path: FileSystemPath::Path { path: docs_public }, access: FileSystemAccessMode::Write, }, ]); @@ -3821,8 +3831,8 @@ mod tests { assert_eq!( sorted_writable_roots(policy.get_writable_roots_with_cwd(cwd.path())), vec![ - (cwd.path().to_path_buf(), vec![docs.to_path_buf()]), - (docs_public.to_path_buf(), Vec::new()), + (canonical_cwd, vec![expected_docs.to_path_buf()]), + (expected_docs_public.to_path_buf(), Vec::new()), ] ); } @@ -3832,6 +3842,7 @@ 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 policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], read_only_access: ReadOnlyAccess::Restricted { @@ -3848,7 +3859,7 @@ mod tests { FileSystemSandboxPolicy::from_legacy_sandbox_policy(&policy, cwd.path()) .get_writable_roots_with_cwd(cwd.path()) ), - vec![(cwd.path().to_path_buf(), Vec::new())] + vec![(canonical_cwd, Vec::new())] ); } From d7559c626575f3f5ac630a5e2d857fc510d96779 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sat, 14 Mar 2026 00:26:25 -0700 Subject: [PATCH 2/9] fix(sandbox): preserve protected symlink carveouts --- codex-rs/protocol/src/permissions.rs | 80 ++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 17 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 4eb91de29af..e2665d363f6 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -371,7 +371,7 @@ impl FileSystemSandboxPolicy { return Vec::new(); } - dedup_normalized_absolute_paths( + dedup_absolute_paths( self.resolved_entries_with_cwd(cwd) .into_iter() .filter(|entry| entry.access.can_read()) @@ -389,7 +389,7 @@ impl FileSystemSandboxPolicy { } let resolved_entries = self.resolved_entries_with_cwd(cwd); - let read_only_roots = dedup_normalized_absolute_paths( + let read_only_roots = dedup_absolute_paths( resolved_entries .iter() .filter(|entry| !entry.access.can_write()) @@ -398,7 +398,7 @@ impl FileSystemSandboxPolicy { .collect(), ); - dedup_normalized_absolute_paths( + dedup_absolute_paths( resolved_entries .into_iter() .filter(|entry| entry.access.can_write()) @@ -421,7 +421,10 @@ impl FileSystemSandboxPolicy { ); WritableRoot { root, - read_only_subpaths: dedup_normalized_absolute_paths(read_only_subpaths), + // Preserve literal in-root protected paths like `.git` and + // `.codex` so downstream sandboxes can still detect and mask + // the symlink itself instead of only its resolved target. + read_only_subpaths: dedup_absolute_paths_preserving_symlinks(read_only_subpaths), } }) .collect() @@ -437,7 +440,7 @@ impl FileSystemSandboxPolicy { .ok() .map(|cwd| absolute_root_path_for_cwd(&cwd)); - dedup_normalized_absolute_paths( + dedup_absolute_paths( self.resolved_entries_with_cwd(cwd) .iter() .filter(|entry| entry.access == FileSystemAccessMode::None) @@ -580,13 +583,13 @@ impl FileSystemSandboxPolicy { } else { ReadOnlyAccess::Restricted { include_platform_defaults, - readable_roots: dedup_absolute_paths(readable_roots), + readable_roots: dedup_absolute_paths_preserving_symlinks(readable_roots), } }; if workspace_root_writable { SandboxPolicy::WorkspaceWrite { - writable_roots: dedup_absolute_paths(writable_roots), + writable_roots: dedup_absolute_paths_preserving_symlinks(writable_roots), read_only_access, network_access: network_policy.is_enabled(), exclude_tmpdir_env_var: !tmpdir_writable, @@ -923,6 +926,18 @@ fn resolve_file_system_special_path( } fn dedup_absolute_paths(paths: Vec) -> Vec { + let mut deduped = Vec::with_capacity(paths.len()); + let mut seen = HashSet::new(); + for path in paths { + let normalized_path = normalize_effective_absolute_path(path); + if seen.insert(normalized_path.to_path_buf()) { + deduped.push(normalized_path); + } + } + deduped +} + +fn dedup_absolute_paths_preserving_symlinks(paths: Vec) -> Vec { let mut deduped = Vec::with_capacity(paths.len()); let mut seen = HashSet::new(); for path in paths { @@ -951,15 +966,6 @@ fn normalize_effective_absolute_path(path: AbsolutePathBuf) -> AbsolutePathBuf { path } -fn dedup_normalized_absolute_paths(paths: Vec) -> Vec { - dedup_absolute_paths( - paths - .into_iter() - .map(normalize_effective_absolute_path) - .collect(), - ) -} - fn default_read_only_subpaths_for_writable_root( writable_root: &AbsolutePathBuf, ) -> Vec { @@ -993,7 +999,7 @@ fn default_read_only_subpaths_for_writable_root( } } - dedup_absolute_paths(subpaths) + dedup_absolute_paths_preserving_symlinks(subpaths) } fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool { @@ -1161,6 +1167,46 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn writable_roots_preserve_symlinked_protected_subpaths() { + let cwd = TempDir::new().expect("tempdir"); + let root = cwd.path().join("root"); + let decoy = root.join("decoy-codex"); + let dot_codex = root.join(".codex"); + fs::create_dir_all(&decoy).expect("create decoy"); + symlink_dir(&decoy, &dot_codex).expect("create .codex symlink"); + + let root = AbsolutePathBuf::from_absolute_path(&root).expect("absolute root"); + let expected_dot_codex = AbsolutePathBuf::from_absolute_path( + root.as_path() + .canonicalize() + .expect("canonicalize root") + .join(".codex"), + ) + .expect("absolute .codex symlink"); + let unexpected_decoy = + AbsolutePathBuf::from_absolute_path(decoy.canonicalize().expect("canonicalize decoy")) + .expect("absolute canonical decoy"); + + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Path { path: root }, + access: FileSystemAccessMode::Write, + }]); + + let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); + assert_eq!(writable_roots.len(), 1); + assert_eq!( + writable_roots[0].read_only_subpaths, + vec![expected_dot_codex] + ); + assert!( + !writable_roots[0] + .read_only_subpaths + .contains(&unexpected_decoy) + ); + } + #[test] fn resolve_access_with_cwd_uses_most_specific_entry() { let cwd = TempDir::new().expect("tempdir"); From 16d18520dfbc1ef0db94109f0bc6238fc7a9fdff Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sat, 14 Mar 2026 00:34:20 -0700 Subject: [PATCH 3/9] fix(sandbox): keep symlink-aware root dedup unified --- codex-rs/protocol/src/permissions.rs | 38 ++++++++++++++-------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index e2665d363f6..507ca0ac0e2 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -378,6 +378,7 @@ impl FileSystemSandboxPolicy { .filter(|entry| self.can_read_path_with_cwd(entry.path.as_path(), cwd)) .map(|entry| entry.path) .collect(), + true, ) } @@ -396,6 +397,7 @@ impl FileSystemSandboxPolicy { .filter(|entry| !self.can_write_path_with_cwd(entry.path.as_path(), cwd)) .map(|entry| entry.path.clone()) .collect(), + true, ); dedup_absolute_paths( @@ -405,6 +407,7 @@ impl FileSystemSandboxPolicy { .filter(|entry| self.can_write_path_with_cwd(entry.path.as_path(), cwd)) .map(|entry| entry.path) .collect(), + true, ) .into_iter() .map(|root| { @@ -424,7 +427,7 @@ impl FileSystemSandboxPolicy { // Preserve literal in-root protected paths like `.git` and // `.codex` so downstream sandboxes can still detect and mask // the symlink itself instead of only its resolved target. - read_only_subpaths: dedup_absolute_paths_preserving_symlinks(read_only_subpaths), + read_only_subpaths: dedup_absolute_paths(read_only_subpaths, false), } }) .collect() @@ -451,6 +454,7 @@ impl FileSystemSandboxPolicy { .filter(|entry| root.as_ref() != Some(&entry.path)) .map(|entry| entry.path.clone()) .collect(), + true, ) } @@ -583,13 +587,13 @@ impl FileSystemSandboxPolicy { } else { ReadOnlyAccess::Restricted { include_platform_defaults, - readable_roots: dedup_absolute_paths_preserving_symlinks(readable_roots), + readable_roots: dedup_absolute_paths(readable_roots, false), } }; if workspace_root_writable { SandboxPolicy::WorkspaceWrite { - writable_roots: dedup_absolute_paths_preserving_symlinks(writable_roots), + writable_roots: dedup_absolute_paths(writable_roots, false), read_only_access, network_access: network_policy.is_enabled(), exclude_tmpdir_env_var: !tmpdir_writable, @@ -925,24 +929,20 @@ fn resolve_file_system_special_path( } } -fn dedup_absolute_paths(paths: Vec) -> Vec { - let mut deduped = Vec::with_capacity(paths.len()); - let mut seen = HashSet::new(); - for path in paths { - let normalized_path = normalize_effective_absolute_path(path); - if seen.insert(normalized_path.to_path_buf()) { - deduped.push(normalized_path); - } - } - deduped -} - -fn dedup_absolute_paths_preserving_symlinks(paths: Vec) -> Vec { +fn dedup_absolute_paths( + paths: Vec, + normalize_effective_paths: bool, +) -> Vec { let mut deduped = Vec::with_capacity(paths.len()); let mut seen = HashSet::new(); for path in paths { - if seen.insert(path.to_path_buf()) { - deduped.push(path); + let dedup_path = if normalize_effective_paths { + normalize_effective_absolute_path(path) + } else { + path + }; + if seen.insert(dedup_path.to_path_buf()) { + deduped.push(dedup_path); } } deduped @@ -999,7 +999,7 @@ fn default_read_only_subpaths_for_writable_root( } } - dedup_absolute_paths_preserving_symlinks(subpaths) + dedup_absolute_paths(subpaths, false) } fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool { From 82011cd8a94c4468bd61a0f6f73507f84795a30d Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sat, 14 Mar 2026 00:51:53 -0700 Subject: [PATCH 4/9] fix(protocol): gate unix-only test import --- codex-rs/protocol/src/permissions.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 507ca0ac0e2..eb34607e456 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -1071,6 +1071,7 @@ fn resolve_gitdir_from_file(dot_git: &AbsolutePathBuf) -> Option Date: Sat, 14 Mar 2026 01:20:56 -0700 Subject: [PATCH 5/9] test(protocol): add symlinked tmpdir regression --- codex-rs/protocol/src/permissions.rs | 87 ++++++++++++++++++++++++++++ codex-rs/protocol/src/protocol.rs | 6 +- 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index eb34607e456..2c579d0bdf2 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -1076,6 +1076,9 @@ mod tests { use std::path::Path; use tempfile::TempDir; + #[cfg(unix)] + const SYMLINKED_TMPDIR_TEST_ENV: &str = "CODEX_PROTOCOL_TEST_SYMLINKED_TMPDIR"; + #[cfg(unix)] fn symlink_dir(original: &Path, link: &Path) -> std::io::Result<()> { std::os::unix::fs::symlink(original, link) @@ -1208,6 +1211,90 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn tmpdir_special_path_canonicalizes_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") + .output() + .expect("run tmpdir subprocess test"); + + assert!( + output.status.success(), + "tmpdir subprocess test failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + return; + } + + let cwd = TempDir::new().expect("tempdir"); + let real_tmpdir = cwd.path().join("real-tmpdir"); + let link_tmpdir = cwd.path().join("link-tmpdir"); + let blocked = real_tmpdir.join("blocked"); + let codex_dir = real_tmpdir.join(".codex"); + + fs::create_dir_all(&blocked).expect("create blocked"); + fs::create_dir_all(&codex_dir).expect("create .codex"); + symlink_dir(&real_tmpdir, &link_tmpdir).expect("create symlinked tmpdir"); + + 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"); + + unsafe { + std::env::set_var("TMPDIR", &link_tmpdir); + } + + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: link_blocked }, + access: FileSystemAccessMode::None, + }, + ]); + + assert_eq!( + policy.get_unreadable_roots_with_cwd(cwd.path()), + vec![expected_blocked.clone()] + ); + + let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); + assert_eq!(writable_roots.len(), 1); + assert_eq!(writable_roots[0].root, expected_root); + assert!( + writable_roots[0] + .read_only_subpaths + .contains(&expected_blocked) + ); + assert!( + writable_roots[0] + .read_only_subpaths + .contains(&expected_codex) + ); + } + #[test] fn resolve_access_with_cwd_uses_most_specific_entry() { let cwd = TempDir::new().expect("tempdir"); diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index f48e67fb163..c827045f1e0 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -3675,9 +3675,9 @@ mod tests { #[test] fn restricted_file_system_policy_treats_root_with_carveouts_as_scoped_access() { let cwd = TempDir::new().expect("tempdir"); - let cwd_absolute = - AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute tempdir"); - let root = cwd_absolute + let canonical_cwd = cwd.path().canonicalize().expect("canonicalize cwd"); + let root = AbsolutePathBuf::from_absolute_path(&canonical_cwd) + .expect("absolute canonical tempdir") .as_path() .ancestors() .last() From fc453b2d07b6ab166487d31f836373be2290c76d Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sat, 14 Mar 2026 12:26:49 -0700 Subject: [PATCH 6/9] style(protocol): annotate normalization boolean args --- codex-rs/protocol/src/permissions.rs | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 2c579d0bdf2..57f370cdec5 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -378,7 +378,7 @@ impl FileSystemSandboxPolicy { .filter(|entry| self.can_read_path_with_cwd(entry.path.as_path(), cwd)) .map(|entry| entry.path) .collect(), - true, + /*normalize_effective_paths*/ true, ) } @@ -397,7 +397,7 @@ impl FileSystemSandboxPolicy { .filter(|entry| !self.can_write_path_with_cwd(entry.path.as_path(), cwd)) .map(|entry| entry.path.clone()) .collect(), - true, + /*normalize_effective_paths*/ true, ); dedup_absolute_paths( @@ -407,7 +407,7 @@ impl FileSystemSandboxPolicy { .filter(|entry| self.can_write_path_with_cwd(entry.path.as_path(), cwd)) .map(|entry| entry.path) .collect(), - true, + /*normalize_effective_paths*/ true, ) .into_iter() .map(|root| { @@ -427,7 +427,10 @@ impl FileSystemSandboxPolicy { // Preserve literal in-root protected paths like `.git` and // `.codex` so downstream sandboxes can still detect and mask // the symlink itself instead of only its resolved target. - read_only_subpaths: dedup_absolute_paths(read_only_subpaths, false), + read_only_subpaths: dedup_absolute_paths( + read_only_subpaths, + /*normalize_effective_paths*/ false, + ), } }) .collect() @@ -454,7 +457,7 @@ impl FileSystemSandboxPolicy { .filter(|entry| root.as_ref() != Some(&entry.path)) .map(|entry| entry.path.clone()) .collect(), - true, + /*normalize_effective_paths*/ true, ) } @@ -587,13 +590,19 @@ impl FileSystemSandboxPolicy { } else { ReadOnlyAccess::Restricted { include_platform_defaults, - readable_roots: dedup_absolute_paths(readable_roots, false), + readable_roots: dedup_absolute_paths( + readable_roots, + /*normalize_effective_paths*/ false, + ), } }; if workspace_root_writable { SandboxPolicy::WorkspaceWrite { - writable_roots: dedup_absolute_paths(writable_roots, false), + writable_roots: dedup_absolute_paths( + writable_roots, + /*normalize_effective_paths*/ false, + ), read_only_access, network_access: network_policy.is_enabled(), exclude_tmpdir_env_var: !tmpdir_writable, @@ -999,7 +1008,7 @@ fn default_read_only_subpaths_for_writable_root( } } - dedup_absolute_paths(subpaths, false) + dedup_absolute_paths(subpaths, /*normalize_effective_paths*/ false) } fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool { From 08fee1d3a1e2727e5bc99faf9d4bd09d3275d771 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sat, 14 Mar 2026 12:42:37 -0700 Subject: [PATCH 7/9] fix(protocol): preserve explicit symlink carveouts --- codex-rs/protocol/src/permissions.rs | 122 ++++++++++++++++++++++----- 1 file changed, 103 insertions(+), 19 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 57f370cdec5..74c9c37faa0 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -390,37 +390,68 @@ impl FileSystemSandboxPolicy { } let resolved_entries = self.resolved_entries_with_cwd(cwd); - let read_only_roots = dedup_absolute_paths( - resolved_entries - .iter() - .filter(|entry| !entry.access.can_write()) - .filter(|entry| !self.can_write_path_with_cwd(entry.path.as_path(), cwd)) - .map(|entry| entry.path.clone()) - .collect(), - /*normalize_effective_paths*/ true, - ); + let writable_entries: Vec = resolved_entries + .iter() + .filter(|entry| entry.access.can_write()) + .filter(|entry| self.can_write_path_with_cwd(entry.path.as_path(), cwd)) + .map(|entry| entry.path.clone()) + .collect(); dedup_absolute_paths( - resolved_entries - .into_iter() - .filter(|entry| entry.access.can_write()) - .filter(|entry| self.can_write_path_with_cwd(entry.path.as_path(), cwd)) - .map(|entry| entry.path) - .collect(), + writable_entries.clone(), /*normalize_effective_paths*/ true, ) .into_iter() .map(|root| { + let preserve_raw_carveout_paths = root.as_path().parent().is_some(); + let raw_writable_roots: Vec<&AbsolutePathBuf> = writable_entries + .iter() + .filter(|path| normalize_effective_absolute_path((*path).clone()) == root) + .collect(); let mut read_only_subpaths = default_read_only_subpaths_for_writable_root(&root); // Narrower explicit non-write entries carve out broader writable roots. // More specific write entries still remain writable because they appear // as separate WritableRoot values and are checked independently. + // Preserve symlink path components that live under the writable root + // so downstream sandboxes can still mask the symlink inode itself. read_only_subpaths.extend( - read_only_roots + resolved_entries .iter() - .filter(|path| path.as_path() != root.as_path()) - .filter(|path| path.as_path().starts_with(root.as_path())) - .cloned(), + .filter(|entry| !entry.access.can_write()) + .filter(|entry| !self.can_write_path_with_cwd(entry.path.as_path(), cwd)) + .filter_map(|entry| { + let effective_path = normalize_effective_absolute_path(entry.path.clone()); + if effective_path == root + || !effective_path.as_path().starts_with(root.as_path()) + { + return None; + } + + if !preserve_raw_carveout_paths { + return Some(effective_path); + } + + if entry.path.as_path().starts_with(root.as_path()) { + return Some(entry.path.clone()); + } + + Some( + raw_writable_roots + .iter() + .find_map(|raw_root| { + let suffix = entry + .path + .as_path() + .strip_prefix(raw_root.as_path()) + .ok()?; + let candidate = root.join(suffix).ok()?; + (normalize_effective_absolute_path(candidate.clone()) + == effective_path) + .then_some(candidate) + }) + .unwrap_or(effective_path), + ) + }), ); WritableRoot { root, @@ -1220,6 +1251,59 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn writable_roots_preserve_explicit_symlinked_carveouts_under_symlinked_roots() { + let cwd = TempDir::new().expect("tempdir"); + let real_root = cwd.path().join("real"); + let link_root = cwd.path().join("link"); + let decoy = real_root.join("decoy-private"); + let linked_private = real_root.join("linked-private"); + fs::create_dir_all(&decoy).expect("create decoy"); + symlink_dir(&real_root, &link_root).expect("create symlinked root"); + symlink_dir(&decoy, &linked_private).expect("create linked-private 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 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 unexpected_decoy = + AbsolutePathBuf::from_absolute_path(decoy.canonicalize().expect("canonicalize decoy")) + .expect("absolute canonical decoy"); + + 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 writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); + assert_eq!(writable_roots.len(), 1); + assert_eq!(writable_roots[0].root, expected_root); + assert_eq!( + writable_roots[0].read_only_subpaths, + vec![expected_linked_private] + ); + assert!( + !writable_roots[0] + .read_only_subpaths + .contains(&unexpected_decoy) + ); + } + #[cfg(unix)] #[test] fn tmpdir_special_path_canonicalizes_symlinked_tmpdir() { From 80a142bde32c2651d1bf1e83149dbcda17ca4f65 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sat, 14 Mar 2026 12:50:44 -0700 Subject: [PATCH 8/9] fix(protocol): harden symlinked carveouts --- codex-rs/protocol/src/permissions.rs | 148 ++++++++++++++++++++++----- 1 file changed, 124 insertions(+), 24 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 74c9c37faa0..b26340fe027 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -403,6 +403,8 @@ impl FileSystemSandboxPolicy { ) .into_iter() .map(|root| { + // Filesystem-root policies stay in their effective canonical form + // so root-wide aliases do not create duplicate top-level masks. let preserve_raw_carveout_paths = root.as_path().parent().is_some(); let raw_writable_roots: Vec<&AbsolutePathBuf> = writable_entries .iter() @@ -421,36 +423,44 @@ impl FileSystemSandboxPolicy { .filter(|entry| !self.can_write_path_with_cwd(entry.path.as_path(), cwd)) .filter_map(|entry| { let effective_path = normalize_effective_absolute_path(entry.path.clone()); - if effective_path == root - || !effective_path.as_path().starts_with(root.as_path()) - { - return None; - } - - if !preserve_raw_carveout_paths { - return Some(effective_path); - } - - if entry.path.as_path().starts_with(root.as_path()) { - return Some(entry.path.clone()); - } - - Some( - raw_writable_roots - .iter() - .find_map(|raw_root| { + // Preserve the literal in-root path whenever the + // carveout itself lives under this writable root, even + // if following symlinks would resolve back to the root + // or escape outside it. Downstream sandboxes need that + // raw path so they can mask the symlink inode itself. + let raw_carveout_path = if preserve_raw_carveout_paths { + if entry.path == root { + None + } else if entry.path.as_path().starts_with(root.as_path()) { + Some(entry.path.clone()) + } else { + raw_writable_roots.iter().find_map(|raw_root| { let suffix = entry .path .as_path() .strip_prefix(raw_root.as_path()) .ok()?; - let candidate = root.join(suffix).ok()?; - (normalize_effective_absolute_path(candidate.clone()) - == effective_path) - .then_some(candidate) + if suffix.as_os_str().is_empty() { + return None; + } + root.join(suffix).ok() }) - .unwrap_or(effective_path), - ) + } + } else { + None + }; + + if let Some(raw_carveout_path) = raw_carveout_path { + return Some(raw_carveout_path); + } + + if effective_path == root + || !effective_path.as_path().starts_with(root.as_path()) + { + return None; + } + + Some(effective_path) }), ); WritableRoot { @@ -1304,6 +1314,96 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn writable_roots_preserve_explicit_symlinked_carveouts_that_escape_root() { + let cwd = TempDir::new().expect("tempdir"); + let real_root = cwd.path().join("real"); + let link_root = cwd.path().join("link"); + let decoy = cwd.path().join("outside-private"); + let linked_private = real_root.join("linked-private"); + fs::create_dir_all(&decoy).expect("create decoy"); + fs::create_dir_all(&real_root).expect("create real root"); + symlink_dir(&real_root, &link_root).expect("create symlinked root"); + symlink_dir(&decoy, &linked_private).expect("create linked-private 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 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 unexpected_decoy = + AbsolutePathBuf::from_absolute_path(decoy.canonicalize().expect("canonicalize decoy")) + .expect("absolute canonical decoy"); + + 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 writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); + assert_eq!(writable_roots.len(), 1); + assert_eq!(writable_roots[0].root, expected_root); + assert_eq!( + writable_roots[0].read_only_subpaths, + vec![expected_linked_private] + ); + assert!( + !writable_roots[0] + .read_only_subpaths + .contains(&unexpected_decoy) + ); + } + + #[cfg(unix)] + #[test] + fn writable_roots_preserve_explicit_symlinked_carveouts_that_alias_root() { + let cwd = TempDir::new().expect("tempdir"); + let root = cwd.path().join("root"); + let alias = root.join("alias-root"); + fs::create_dir_all(&root).expect("create root"); + symlink_dir(&root, &alias).expect("create alias symlink"); + + let root = AbsolutePathBuf::from_absolute_path(&root).expect("absolute root"); + let alias = root.join("alias-root").expect("alias root path"); + let expected_root = AbsolutePathBuf::from_absolute_path( + root.as_path().canonicalize().expect("canonicalize root"), + ) + .expect("absolute canonical root"); + let expected_alias = expected_root + .join("alias-root") + .expect("expected alias path"); + + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: root }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: alias }, + access: FileSystemAccessMode::None, + }, + ]); + + let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); + assert_eq!(writable_roots.len(), 1); + assert_eq!(writable_roots[0].root, expected_root); + assert_eq!(writable_roots[0].read_only_subpaths, vec![expected_alias]); + } + #[cfg(unix)] #[test] fn tmpdir_special_path_canonicalizes_symlinked_tmpdir() { From c025ae5ec26f2b8219af59eecc6fbe187a6fb447 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sat, 14 Mar 2026 12:53:44 -0700 Subject: [PATCH 9/9] docs(protocol): add carveout path examples --- codex-rs/protocol/src/permissions.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index b26340fe027..a485a9c16f9 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -405,6 +405,8 @@ impl FileSystemSandboxPolicy { .map(|root| { // Filesystem-root policies stay in their effective canonical form // so root-wide aliases do not create duplicate top-level masks. + // Example: keep `/var/...` normalized under `/` instead of + // materializing both `/var/...` and `/private/var/...`. let preserve_raw_carveout_paths = root.as_path().parent().is_some(); let raw_writable_roots: Vec<&AbsolutePathBuf> = writable_entries .iter() @@ -416,6 +418,8 @@ impl FileSystemSandboxPolicy { // as separate WritableRoot values and are checked independently. // Preserve symlink path components that live under the writable root // so downstream sandboxes can still mask the symlink inode itself. + // Example: if `/.codex -> /decoy`, bwrap must still see + // `/.codex`, not only the resolved `/decoy`. read_only_subpaths.extend( resolved_entries .iter() @@ -428,6 +432,10 @@ impl FileSystemSandboxPolicy { // if following symlinks would resolve back to the root // or escape outside it. Downstream sandboxes need that // raw path so they can mask the symlink inode itself. + // Examples: + // - `/linked-private -> /decoy-private` + // - `/linked-private -> /tmp/outside-private` + // - `/alias-root -> ` let raw_carveout_path = if preserve_raw_carveout_paths { if entry.path == root { None