diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 094cd52d20..0dcd53a4e6 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4336,6 +4336,7 @@ dependencies = [ "icu_locale_core", "icu_provider", "landlock", + "libc", "pretty_assertions", "quick-xml 0.41.0", "schemars 0.8.22", diff --git a/codex-rs/core/tests/suite/apply_patch_system_aliases.rs b/codex-rs/core/tests/suite/apply_patch_system_aliases.rs index 2ba778a4a8..03482611a4 100644 --- a/codex-rs/core/tests/suite/apply_patch_system_aliases.rs +++ b/codex-rs/core/tests/suite/apply_patch_system_aliases.rs @@ -94,6 +94,16 @@ async fn case_variant_metadata_patch_cannot_write_without_approval(shell: bool) ("# changed\n".to_string(), "# original\n".to_string()), ); assert_eq!(responses.requests().len(), 3); + let request = responses.last_request().expect("protected patch response"); + let (output, _) = if shell { + request.function_call_output_content_and_success("protected") + } else { + request.custom_tool_call_output_content_and_success("protected") + } + .expect("protected patch output"); + assert!(output.expect("patch rejection text").contains( + "patch rejected: writing outside of the project; rejected by user approval settings" + )); Ok(()) } diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index 914ed5260c..8314592a7d 100644 --- a/codex-rs/protocol/Cargo.toml +++ b/codex-rs/protocol/Cargo.toml @@ -56,6 +56,9 @@ wildmatch = { workspace = true } landlock = { workspace = true } seccompiler = { workspace = true } +[target.'cfg(target_os = "macos")'.dependencies] +libc = { workspace = true } + [dev-dependencies] anyhow = { workspace = true } pretty_assertions = { workspace = true } diff --git a/codex-rs/protocol/src/permissions/local_aliases.rs b/codex-rs/protocol/src/permissions/local_aliases.rs index 83c8ba0dd5..c3badbb37b 100644 --- a/codex-rs/protocol/src/permissions/local_aliases.rs +++ b/codex-rs/protocol/src/permissions/local_aliases.rs @@ -1,5 +1,6 @@ //! Prepare local policy paths in the sandbox's trusted system-alias namespace. //! Normalize grants and restrictions once per operation without resolving mutable symlinks. +//! Check native macOS ASCII aliases before approving protected metadata writes. //! Borrow unchanged policies and return normalization failures to fallible callers. use super::FileSystemAccessMode; @@ -8,12 +9,22 @@ use super::FileSystemSandboxEntry; use super::FileSystemSandboxPolicy; use super::FileSystemSandboxPolicyContext; use super::FileSystemSpecialPath; +#[cfg(target_os = "macos")] +use super::PROTECTED_METADATA_PATH_NAMES; use super::local_temporary_directories; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use std::borrow::Cow; +#[cfg(target_os = "macos")] +use std::ffi::CString; use std::io; +#[cfg(target_os = "macos")] +use std::os::unix::ffi::OsStrExt; +#[cfg(target_os = "macos")] +use std::path::Component; use std::path::Path; +#[cfg(target_os = "macos")] +use std::path::PathBuf; /// Owns native environment bindings so one matcher can serve a whole root calculation. pub(super) struct LocalPolicyContext { @@ -43,7 +54,8 @@ impl LocalPolicyContext { /// An operation-scoped view of a configured policy with local roots resolved. /// Reuse it for each target in the operation; prepare again when policy or -/// environment bindings change. Only target paths are normalized during matching. +/// environment bindings change. Metadata checks also account for native ASCII +/// case aliases without changing executor-independent path comparisons. pub struct LocalFileSystemPolicyMatcher<'a> { policy: Cow<'a, FileSystemSandboxPolicy>, context: FileSystemSandboxPolicyContext<'a>, @@ -100,9 +112,45 @@ impl FileSystemSandboxPolicy { } impl LocalFileSystemPolicyMatcher<'_> { - /// Check write access, preserving any failure to normalize the target path. + /// Check write access, preserving failures to establish native path equivalence. pub fn can_write_path(&self, path: &PathUri) -> io::Result { - self.with_path(path, |path| self.policy.can_write_path(path, &self.context)) + self.with_path(path, |path| -> io::Result { + let writable = self.policy.can_write_path(path, &self.context); + #[cfg(target_os = "macos")] + if writable + && !self + .policy + .has_full_disk_write_access_with_context(&self.context) + { + let native_path = path.to_abs_path()?; + for (root, access) in self.policy.resolved_entries(&self.context) { + if !access.can_write() || root.is_opaque() { + continue; + } + let root = root.to_abs_path()?; + let Some(candidate) = normalize_metadata_case_alias(&native_path, &root)? + else { + continue; + }; + // This second policy view may only deny an already allowed write. + // Normalize both candidate and entries so explicit metadata grants + // and narrower restrictions keep their precedence. + let mut policy = self.policy.clone().into_owned(); + for entry in &mut policy.entries { + if let FileSystemPath::Path { path } = &mut entry.path + && let Some(normalized) = + normalize_metadata_case_alias(&path.to_abs_path()?, &root)? + { + *path = PathUri::from(normalized); + } + } + if !policy.can_write_path(&PathUri::from(candidate), &self.context) { + return Ok(false); + } + } + } + Ok(writable) + })? } pub(super) fn resolve_access(&self, path: &PathUri) -> FileSystemAccessMode { @@ -126,6 +174,87 @@ impl LocalFileSystemPolicyMatcher<'_> { } } +// Match only ASCII case aliases leading to protected metadata. Each differing +// component is checked against its containing filesystem, including ancestors +// above mount points. Mutable symlink targets are never substituted into paths. +#[cfg(target_os = "macos")] +fn normalize_metadata_case_alias( + path: &AbsolutePathBuf, + root: &AbsolutePathBuf, +) -> io::Result> { + let mut relative = path.components(); + let Some(Component::Normal(name)) = relative.nth(root.components().count()) else { + return Ok(None); + }; + let Some(metadata_name) = PROTECTED_METADATA_PATH_NAMES + .iter() + .find(|metadata| name.as_bytes().eq_ignore_ascii_case(metadata.as_bytes())) + else { + return Ok(None); + }; + let mut parent = PathBuf::new(); + for (expected, actual) in root.components().zip(path.components()) { + if expected != actual + && (!expected + .as_os_str() + .as_bytes() + .eq_ignore_ascii_case(actual.as_os_str().as_bytes()) + || !is_case_insensitive_filesystem(&parent)?) + { + return Ok(None); + } + parent.push(expected.as_os_str()); + } + // Exact metadata names still need root alias normalization even when the + // metadata's own filesystem is case-sensitive. + if name.as_bytes() != metadata_name.as_bytes() + && !is_case_insensitive_filesystem(root.as_path())? + { + return Ok(None); + } + Ok(Some(root.join(*metadata_name).join(relative.as_path()))) +} + +#[cfg(target_os = "macos")] +fn is_case_insensitive_filesystem(path: &Path) -> io::Result { + // Missing paths inherit their nearest existing ancestor's semantics. + // An unknown result must never authorize a protected metadata write. + for ancestor in path.ancestors() { + let path = CString::new(ancestor.as_os_str().as_bytes())?; + // SAFETY: path is NUL-terminated and __error points to this thread's + // errno. Reset it to distinguish an unavailable value from a failure. + let sensitivity = unsafe { + *libc::__error() = 0; + libc::pathconf(path.as_ptr(), libc::_PC_CASE_SENSITIVE) + }; + match sensitivity { + 0 => return Ok(true), + 1 => return Ok(false), + -1 => { + let error = io::Error::last_os_error(); + if error.raw_os_error() == Some(0) { + return Err(io::Error::new( + io::ErrorKind::Unsupported, + "filesystem case sensitivity is unavailable", + )); + } + if error.kind() != io::ErrorKind::NotFound { + return Err(error); + } + } + value => { + return Err(io::Error::other(format!( + "unexpected filesystem case sensitivity: {value}", + ))); + } + } + } + Err(io::Error::new( + io::ErrorKind::NotFound, + "no existing metadata parent", + )) +} + #[cfg(all(test, target_os = "macos"))] #[path = "local_aliases_tests.rs"] mod tests; diff --git a/codex-rs/protocol/src/permissions/local_aliases_tests.rs b/codex-rs/protocol/src/permissions/local_aliases_tests.rs index 2e06e016c3..6f7d47e101 100644 --- a/codex-rs/protocol/src/permissions/local_aliases_tests.rs +++ b/codex-rs/protocol/src/permissions/local_aliases_tests.rs @@ -63,3 +63,101 @@ fn equivalent_alias_entries_keep_restrictive_precedence() -> anyhow::Result<()> ); Ok(()) } + +#[test] +fn local_metadata_case_aliases_respect_filesystem_and_explicit_grants() -> anyhow::Result<()> { + let directory = tempfile::tempdir()?; + let parent = PathUri::from_host_native_path(directory.path().canonicalize()?)?; + let workspace = parent.join("workspace")?.to_abs_path()?; + std::fs::create_dir(&workspace)?; + std::fs::create_dir(workspace.join(".git"))?; + let case_insensitive = workspace.join(".GIT").exists(); + let root = PathUri::from_host_native_path(&workspace)?; + let context = FileSystemSandboxPolicyContext { + cwd: &root, + workspace_roots: std::slice::from_ref(&root), + user_home_dir: None, + temporary_directories: None, + }; + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new(parent.clone().into(), FileSystemAccessMode::Write), + FileSystemSandboxEntry::new(root.clone().into(), FileSystemAccessMode::Write), + ]); + let matching = policy.prepare_local_matching(&context)?; + for relative in [ + ".GIT/config", + ".CoDeX/config.toml", + ".AGENTS/new", + ".AWS/config", + ] { + let path = root.join(relative)?; + assert_eq!( + ( + matching.can_write_path(&path)?, + policy.can_write_path(&path, &context), + ), + (!case_insensitive, true), + "local aliases must not change executor-independent matching: {relative}", + ); + } + let root_alias = parent.join("WORKSPACE")?; + for relative in [".GIT/config", ".git/config", ".CoDeX/config.toml"] { + let path = root_alias.join(relative)?; + assert_eq!( + ( + matching.can_write_path(&path)?, + policy.can_write_path(&path, &context), + ), + (!case_insensitive, true), + "a broader grant must not bypass metadata protection via a root alias: {relative}", + ); + } + for name in [".git", ".GIT"] { + let mut explicitly_writable = policy.clone(); + explicitly_writable + .entries + .push(FileSystemSandboxEntry::new( + root.join(name)?.into(), + FileSystemAccessMode::Write, + )); + let matching = explicitly_writable.prepare_local_matching(&context)?; + for spelling in [&root, &root_alias] { + assert!( + matching.can_write_path(&spelling.join(".GIT/config")?)?, + "explicit metadata grant must remain effective: {name}", + ); + } + } + for (grant, restriction, candidate) in [ + (".git", ".GIT/private", ".git/private"), + (".GIT", ".git/private", ".GIT/private"), + ] { + for access in [FileSystemAccessMode::Read, FileSystemAccessMode::Deny] { + let mut restricted = policy.clone(); + restricted.entries.extend([ + FileSystemSandboxEntry::new(root.join(grant)?.into(), FileSystemAccessMode::Write), + FileSystemSandboxEntry::new(root.join(restriction)?.into(), access), + ]); + assert_eq!( + restricted + .prepare_local_matching(&context)? + .can_write_path(&root.join(candidate)?)?, + !case_insensitive, + "narrower metadata restrictions must retain precedence: {restriction}", + ); + } + } + let missing_root = root.join("missing")?; + let missing_policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new( + missing_root.clone().into(), + FileSystemAccessMode::Write, + )]); + assert_eq!( + missing_policy + .prepare_local_matching(&context)? + .can_write_path(&missing_root.join(".GIT/config")?)?, + !case_insensitive, + ); + assert!(matching.can_write_path(&root.join("nested/.GIT/config")?)?); + Ok(()) +}