fix: fix symlinked writable roots in sandbox policies (#14674)

## Summary
- normalize effective readable, writable, and unreadable sandbox roots
after resolving special paths so symlinked roots use canonical runtime
paths
- add a protocol regression test for a symlinked writable root with a
denied child and update protocol expectations to canonicalized effective
paths
- update macOS seatbelt tests to assert against effective normalized
roots produced by the shared policy helpers

## Testing
- just fmt
- cargo test -p codex-protocol
- cargo test -p codex-core explicit_unreadable_paths_are_excluded_
- cargo clippy -p codex-protocol -p codex-core --tests -- -D warnings

## Notes
- This is intended to fix the symlinked TMPDIR bind failure in
bubblewrap described in #14672.
Fixes #14672
This commit is contained in:
viyatb-oai
2026-03-14 13:24:43 -07:00
committed by GitHub
parent 4b31848f5b
commit 9060dc7557
3 changed files with 515 additions and 59 deletions

View File

@@ -3681,9 +3681,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()
@@ -3691,6 +3691,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 {
@@ -3699,9 +3706,7 @@ mod tests {
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: blocked.clone(),
},
path: FileSystemPath::Path { path: blocked },
access: FileSystemAccessMode::None,
},
]);
@@ -3714,7 +3719,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());
@@ -3724,7 +3729,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())
);
}
@@ -3733,14 +3738,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 {
@@ -3755,9 +3763,7 @@ mod tests {
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: secret.clone(),
},
path: FileSystemPath::Path { path: secret },
access: FileSystemAccessMode::None,
},
]);
@@ -3767,43 +3773,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 {
@@ -3812,13 +3824,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,
},
]);
@@ -3827,8 +3837,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()),
]
);
}
@@ -3838,6 +3848,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 {
@@ -3854,7 +3865,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())]
);
}