From 11da6b9edcbbf1cffdb9b7686970b5b4d5af9c83 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Wed, 7 Oct 2026 20:35:56 +0000 Subject: [PATCH] Add policy-based file contents integrity checks (#51828) ## What changed Export `FileContentsChecker` and structured integrity findings from `codex-rs/sandboxing` to identify existing containment dependency files whose contents are writable under a prepared filesystem policy. Findings include the requested target, its resolved path, and the write roots that permit modification. Resolve symlinks in targets, write roots, and read-only carveouts, and respect protected metadata paths and deny globs matching resolved contents. Missing targets produce no finding; non-file targets return an error. Callers remain responsible for dependency inventory, reporting, enforcement, and handling full-disk write access. ## Testing Add unit tests covering write grants, read-only and denied carveouts, overlapping grants, deny globs, symlink resolution, protected metadata, and missing targets. GitOrigin-RevId: 4f52faed822a1bbb4ed33a9975af0718a5f85662 --- codex-rs/sandboxing/src/lib.rs | 4 + .../src/sandbox_integrity/contents.rs | 99 ++++++++++++ .../src/sandbox_integrity/contents_tests.rs | 143 ++++++++++++++++++ .../src/sandbox_integrity/finding.rs | 18 +++ .../sandboxing/src/sandbox_integrity/mod.rs | 7 + .../src/sandbox_integrity/policy.rs | 31 ++++ 6 files changed, 302 insertions(+) create mode 100644 codex-rs/sandboxing/src/sandbox_integrity/contents.rs create mode 100644 codex-rs/sandboxing/src/sandbox_integrity/contents_tests.rs create mode 100644 codex-rs/sandboxing/src/sandbox_integrity/finding.rs create mode 100644 codex-rs/sandboxing/src/sandbox_integrity/mod.rs create mode 100644 codex-rs/sandboxing/src/sandbox_integrity/policy.rs diff --git a/codex-rs/sandboxing/src/lib.rs b/codex-rs/sandboxing/src/lib.rs index e65ead4d5e..cad86f1467 100644 --- a/codex-rs/sandboxing/src/lib.rs +++ b/codex-rs/sandboxing/src/lib.rs @@ -5,6 +5,7 @@ pub mod landlock; mod linux_pid_namespace; mod manager; pub mod policy_transforms; +mod sandbox_integrity; #[cfg(target_os = "macos")] pub mod seatbelt; mod spawn; @@ -38,6 +39,9 @@ pub use manager::SandboxablePreference; pub use manager::compatibility_sandbox_policy_for_permission_profile; pub use manager::get_platform_sandbox; pub use manager::with_managed_mitm_ca_readable_root; +pub use sandbox_integrity::FileContentsChecker; +pub use sandbox_integrity::IntegrityFinding; +pub use sandbox_integrity::IntegrityFindingDetails; pub use spawn::SpawnRequest; pub use spawn::WindowsSandboxSpawnRequest; pub use spawn::spawn_process; diff --git a/codex-rs/sandboxing/src/sandbox_integrity/contents.rs b/codex-rs/sandboxing/src/sandbox_integrity/contents.rs new file mode 100644 index 0000000000..219c71e815 --- /dev/null +++ b/codex-rs/sandboxing/src/sandbox_integrity/contents.rs @@ -0,0 +1,99 @@ +//! Check declared policy access to an existing containment dependency's contents. +//! Callers own the dependency inventory, reporting, and enforcement. Filesystem +//! inspection resolves current objects; it does not assess ownership or OS ACLs. + +use super::finding::IntegrityFinding; +use super::finding::IntegrityFindingDetails; +use super::policy::PreparedPolicy; +use super::policy::prepare_policy; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_utils_absolute_path::AbsolutePathBuf; +use std::fs; +use std::io; + +/// Rebuild for each effective policy and executor filesystem snapshot. +pub struct FileContentsChecker { + policy: PreparedPolicy, +} + +impl FileContentsChecker { + /// Takes the prepared executor policy: symbolic roots must be materialized, + /// and backend snapshot expansion of globs (including scan limits) must + /// already be reflected in its entries. Remaining globs use policy matching. + /// Callers must handle full-disk write access separately. + pub fn new(policy: &FileSystemSandboxPolicy, cwd: &AbsolutePathBuf) -> io::Result { + let mut policy = prepare_policy(policy, cwd)?; + let mut roots = Vec::new(); + for mut root in policy.roots { + let Some(resolved_root) = resolve_existing(&root.root)? else { + continue; + }; + if !fs::metadata(&resolved_root)?.is_dir() { + root.protected_metadata_names.clear(); + } + // Resolve existing carveouts and protected metadata aliases while + // retaining missing paths unchanged. + let mut exclusions = Vec::new(); + for path in root.read_only_subpaths.iter().cloned().chain( + root.protected_metadata_names + .iter() + .map(|name| root.root.join(name)), + ) { + exclusions.push(resolve_existing(&path)?.unwrap_or(path)); + } + root.root = resolved_root; + root.read_only_subpaths = exclusions; + roots.push(root); + } + policy.roots = roots; + Ok(Self { policy }) + } + + pub fn check(&self, target: &AbsolutePathBuf) -> io::Result> { + match fs::metadata(target) { + Ok(metadata) if metadata.is_file() => {} + Ok(_) => { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "containment dependency is not a file", + )); + } + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(error), + } + let resolved_target = target.canonicalize()?; + // Denying an alias does not establish protection of the resolved contents. + if self + .policy + .glob_denials + .as_ref() + .is_some_and(|matcher| matcher.is_local_path_read_denied(resolved_target.as_path())) + { + return Ok(None); + } + let write_roots: Vec<_> = self + .policy + .roots + .iter() + .filter(|root| root.is_path_writable(resolved_target.as_path())) + .map(|root| root.root.clone()) + .collect(); + Ok((!write_roots.is_empty()).then(|| IntegrityFinding { + target: target.clone(), + write_roots, + details: IntegrityFindingDetails::Contents { resolved_target }, + })) + } +} + +fn resolve_existing(path: &AbsolutePathBuf) -> io::Result> { + match path.canonicalize() { + Ok(path) => Ok(Some(path)), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(None), + Err(error) => Err(error), + } +} + +#[cfg(test)] +#[path = "contents_tests.rs"] +mod tests; diff --git a/codex-rs/sandboxing/src/sandbox_integrity/contents_tests.rs b/codex-rs/sandboxing/src/sandbox_integrity/contents_tests.rs new file mode 100644 index 0000000000..cd1e2df0f1 --- /dev/null +++ b/codex-rs/sandboxing/src/sandbox_integrity/contents_tests.rs @@ -0,0 +1,143 @@ +//! Policy grants, denials, and current file identities used by the contents checker. + +use super::*; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemAccessMode::Deny; +use codex_protocol::permissions::FileSystemAccessMode::Read; +use codex_protocol::permissions::FileSystemAccessMode::Write; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry; +use pretty_assertions::assert_eq; + +fn policy(entries: &[(&AbsolutePathBuf, FileSystemAccessMode)]) -> FileSystemSandboxPolicy { + FileSystemSandboxPolicy::restricted( + entries + .iter() + .map(|(path, access)| FileSystemSandboxEntry::new((*path).clone().into(), *access)) + .collect(), + ) +} + +#[test] +fn contents_respect_write_grants_and_carveouts() -> io::Result<()> { + let directory = tempfile::tempdir()?; + let root = AbsolutePathBuf::from_absolute_path(directory.path())?.canonicalize()?; + let target = root.join("sandbox-helper"); + fs::write(&target, "fixture")?; + for (entries, write_roots) in [ + (vec![(&root, Write)], Some(vec![root.clone()])), + (vec![(&root, Write), (&target, Read)], None), + ( + vec![(&root, Read), (&target, Write)], + Some(vec![target.clone()]), + ), + (vec![(&root, Write), (&target, Deny)], None), + ( + vec![(&root, Write), (&target, Write)], + Some(vec![root.clone(), target.clone()]), + ), + ] { + let checker = FileContentsChecker::new(&policy(&entries), &root)?; + assert_eq!( + checker.check(&target)?, + write_roots.map(|write_roots| IntegrityFinding { + target: target.clone(), + write_roots, + details: IntegrityFindingDetails::Contents { + resolved_target: target.clone(), + }, + }) + ); + } + Ok(()) +} + +#[test] +fn deny_read_globs_protect_matching_contents_without_disabling_other_checks() -> io::Result<()> { + let directory = tempfile::tempdir()?; + let root = AbsolutePathBuf::from_absolute_path(directory.path())?.canonicalize()?; + let target = root.join("sandbox-helper"); + let unrelated = root.join("other-helper"); + fs::write(&target, "fixture")?; + fs::write(&unrelated, "fixture")?; + let mut policy = policy(&[(&root, Write)]); + policy.entries.push(FileSystemSandboxEntry::new( + FileSystemPath::GlobPattern { + pattern: format!("{}/sandbox-*", root.display()), + }, + Deny, + )); + let checker = FileContentsChecker::new(&policy, &root)?; + assert_eq!(checker.check(&target)?, None); + assert_eq!( + checker.check(&unrelated)?, + Some(IntegrityFinding { + target: unrelated.clone(), + write_roots: vec![root], + details: IntegrityFindingDetails::Contents { + resolved_target: unrelated, + }, + }) + ); + Ok(()) +} + +#[cfg(unix)] +#[test] +fn contents_follow_links_and_resolved_write_grants() -> io::Result<()> { + let directory = tempfile::tempdir()?; + let root = AbsolutePathBuf::from_absolute_path(directory.path())?.canonicalize()?; + let writable = root.join("workspace"); + let destination = root.join("sandbox-helper"); + let link = writable.join("helper-link"); + fs::create_dir(&writable)?; + fs::write(&destination, "fixture")?; + std::os::unix::fs::symlink(&destination, &link)?; + let mut alias_denied = policy(&[(&root, Write)]); + alias_denied.entries.push(FileSystemSandboxEntry::new( + FileSystemPath::GlobPattern { + pattern: format!("{}/helper-*", writable.display()), + }, + Deny, + )); + for (policy, expected) in [ + (policy(&[(&writable, Write)]), None), + (policy(&[(&link, Write)]), Some(destination.clone())), + (policy(&[(&root, Write), (&link, Read)]), None), + (alias_denied, Some(root.clone())), + ] { + let checker = FileContentsChecker::new(&policy, &root)?; + assert_eq!( + checker.check(&link)?, + expected.map(|write_root| IntegrityFinding { + target: link.clone(), + write_roots: vec![write_root], + details: IntegrityFindingDetails::Contents { + resolved_target: destination.clone(), + }, + }) + ); + } + Ok(()) +} + +#[test] +fn protected_metadata_contents_remain_read_only() -> io::Result<()> { + let directory = tempfile::tempdir()?; + let root = AbsolutePathBuf::from_absolute_path(directory.path())?.canonicalize()?; + let target = root.join(".codex/config.toml"); + fs::create_dir(root.join(".codex"))?; + fs::write(&target, "fixture")?; + let checker = FileContentsChecker::new(&policy(&[(&root, Write)]), &root)?; + assert_eq!(checker.check(&target)?, None); + Ok(()) +} + +#[test] +fn missing_target_has_no_contents_to_mutate() -> io::Result<()> { + let directory = tempfile::tempdir()?; + let root = AbsolutePathBuf::from_absolute_path(directory.path())?.canonicalize()?; + let checker = FileContentsChecker::new(&policy(&[(&root, Write)]), &root)?; + assert_eq!(checker.check(&root.join("missing"))?, None); + Ok(()) +} diff --git a/codex-rs/sandboxing/src/sandbox_integrity/finding.rs b/codex-rs/sandboxing/src/sandbox_integrity/finding.rs new file mode 100644 index 0000000000..ea2945b900 --- /dev/null +++ b/codex-rs/sandboxing/src/sandbox_integrity/finding.rs @@ -0,0 +1,18 @@ +//! Shared findings for policy-based containment integrity checks. + +use codex_utils_absolute_path::AbsolutePathBuf; + +/// A containment dependency and the effective write grants responsible for a finding. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct IntegrityFinding { + pub target: AbsolutePathBuf, + pub write_roots: Vec, + pub details: IntegrityFindingDetails, +} + +/// Evidence specific to the kind of containment weakness detected. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum IntegrityFindingDetails { + /// The policy permits modifying an existing file's contents. + Contents { resolved_target: AbsolutePathBuf }, +} diff --git a/codex-rs/sandboxing/src/sandbox_integrity/mod.rs b/codex-rs/sandboxing/src/sandbox_integrity/mod.rs new file mode 100644 index 0000000000..063465855a --- /dev/null +++ b/codex-rs/sandboxing/src/sandbox_integrity/mod.rs @@ -0,0 +1,7 @@ +mod contents; +mod finding; +mod policy; + +pub use contents::FileContentsChecker; +pub use finding::IntegrityFinding; +pub use finding::IntegrityFindingDetails; diff --git a/codex-rs/sandboxing/src/sandbox_integrity/policy.rs b/codex-rs/sandboxing/src/sandbox_integrity/policy.rs new file mode 100644 index 0000000000..ebf9c9adc1 --- /dev/null +++ b/codex-rs/sandboxing/src/sandbox_integrity/policy.rs @@ -0,0 +1,31 @@ +//! Prepare policy grants and glob matching shared by containment integrity checks. +//! Each checker owns further filesystem resolution of these roots and carveouts. + +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::ReadDenyMatcher; +use codex_protocol::protocol::WritableRoot; +use codex_utils_absolute_path::AbsolutePathBuf; +use std::io; + +pub(super) struct PreparedPolicy { + pub roots: Vec, + pub glob_denials: Option, +} + +pub(super) fn prepare_policy( + policy: &FileSystemSandboxPolicy, + cwd: &AbsolutePathBuf, +) -> io::Result { + // Writable roots already resolve precedence for exact-path rules. + let mut glob_policy = policy.clone(); + glob_policy + .entries + .retain(|entry| matches!(entry.path, FileSystemPath::GlobPattern { .. })); + let glob_denials = ReadDenyMatcher::try_new_for_local_paths(&glob_policy, cwd.as_path()) + .map_err(|error| io::Error::new(io::ErrorKind::InvalidInput, error))?; + Ok(PreparedPolicy { + roots: policy.get_writable_roots_with_cwd(cwd.as_path()), + glob_denials, + }) +}