diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index ac41557e59..d735f833ad 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4065,11 +4065,13 @@ dependencies = [ "codex-protocol", "codex-utils-absolute-path", "codex-utils-path-uri", + "codex-windows-sandbox", "learning_mode_windows", "pretty_assertions", "tempfile", "thiserror 2.0.18", "tracelogging", + "windows-sys 0.61.2", "wxc_common", ] diff --git a/codex-rs/mxc-sandbox/Cargo.toml b/codex-rs/mxc-sandbox/Cargo.toml index cf38f09e07..7623d17a45 100644 --- a/codex-rs/mxc-sandbox/Cargo.toml +++ b/codex-rs/mxc-sandbox/Cargo.toml @@ -19,6 +19,7 @@ anyhow = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-path-uri = { workspace = true } +codex-windows-sandbox = { workspace = true } thiserror = { workspace = true } wxc_common = { workspace = true } @@ -27,6 +28,13 @@ appcontainer_common = { workspace = true } learning_mode_windows = { workspace = true } # 1.2.4 imports OneCore_apiset, which the GNU Windows toolchain does not ship. tracelogging = "=1.2.3" +windows-sys = { version = "0.61", features = [ + "Win32_Foundation", + "Win32_Storage_FileSystem", + "Win32_System_Com", + "Win32_System_SystemInformation", + "Win32_UI_Shell", +] } [dev-dependencies] pretty_assertions = { workspace = true } diff --git a/codex-rs/mxc-sandbox/src/policy.rs b/codex-rs/mxc-sandbox/src/policy.rs index 9011a341d3..5d989458fe 100644 --- a/codex-rs/mxc-sandbox/src/policy.rs +++ b/codex-rs/mxc-sandbox/src/policy.rs @@ -2,16 +2,24 @@ //! access. Denies and read-only carveouts remain separate kernel policy lists. use std::collections::HashMap; +use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; use codex_protocol::protocol::FileSystemPath; use codex_protocol::protocol::FileSystemSpecialPath; +use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; +use codex_windows_sandbox::resolve_windows_deny_read_paths; use thiserror::Error; use wxc_common::cmdline::CommandLineContext; use wxc_common::cmdline::CommandLineError; use wxc_common::cmdline::cmdline_from_argv_for_context; +use wxc_common::filesystem_object::ExistingObjectComparison; +use wxc_common::filesystem_object::compare_existing_filesystem_objects; +use wxc_common::filesystem_object::normalize_object_conflicts; +use wxc_common::logger::Logger; +use wxc_common::logger::Mode; use wxc_common::models::ContainerPolicy; use wxc_common::models::ExecutionRequest; use wxc_common::models::FallbackPolicy; @@ -37,12 +45,18 @@ pub enum PolicyError { RelativeCommandCwd, #[error("MXC symbolic filesystem roots are not implemented")] SymbolicRoots, - #[error("MXC does not support filesystem deny glob patterns")] - DenyGlobs, #[error("MXC requires a Unicode command working directory")] NonUnicodeCommandCwd, #[error("MXC requires Unicode filesystem policy paths")] NonUnicodePolicyPath, + #[error("{0}")] + PolicyResolution(String), + #[error("enumerate MXC volume {path}: {source}")] + EnumerateVolume { + path: PathBuf, + #[source] + source: std::io::Error, + }, #[error(transparent)] Path(#[from] std::io::Error), #[error(transparent)] @@ -53,6 +67,8 @@ pub fn build_request( command: &MxcCommand, command_cwd: &Path, env: Vec, + volume_roots: &[PathBuf], + _platform_read_roots: &[PathBuf], ) -> Result { if command.command.is_empty() { return Err(PolicyError::EmptyCommand); @@ -76,10 +92,6 @@ pub fn build_request( { return Err(PolicyError::SymbolicRoots); } - if !fs.get_unreadable_globs_with_cwd(cwd).is_empty() { - return Err(PolicyError::DenyGlobs); - } - let roots = fs.get_writable_roots_with_cwd_preserving_mutable_paths(cwd); let mut write = collect_paths(roots.iter().map(|root| root.root.to_path_buf()))?; let mut read = collect_paths( @@ -88,10 +100,13 @@ pub fn build_request( .filter(|path| !fs.can_write_local_path_with_cwd(path.as_path(), cwd)) .map(|path| path.to_path_buf()), )?; + let absolute_cwd = AbsolutePathBuf::from_absolute_path(cwd) + .map_err(|error| PolicyError::PolicyResolution(error.to_string()))?; let deny = collect_paths( - fs.get_unreadable_roots_with_cwd(cwd) + resolve_windows_deny_read_paths(&fs, &absolute_cwd) + .map_err(PolicyError::PolicyResolution)? .into_iter() - .map(|path| path.to_path_buf()), + .map(AbsolutePathBuf::into_path_buf), )?; let carveouts = collect_paths(roots.into_iter().flat_map(|root| { let protected = root @@ -112,7 +127,30 @@ pub fn build_request( .map(|(key, path)| (key.clone(), path.clone())), ); write.retain(|key, _| !carveouts.contains_key(key) && !deny.contains_key(key)); - read.retain(|key, _| !deny.contains_key(key)); + read.retain(|key, _| !write.contains_key(key) && !deny.contains_key(key)); + let volume_roots = volume_roots + .iter() + .map(PathUri::from_host_native_path) + .collect::>>()?; + prune_unavailable_volume_roots(&mut write, &volume_roots); + prune_unavailable_volume_roots(&mut read, &volume_roots); + // Normalize pre-expansion aliases so generated children inherit + // any tightened access applied to their volume root. + let normalized = normalize_policy(ContainerPolicy { + readwrite_paths: unicode_paths(write)?, + readonly_paths: unicode_paths(read)?, + denied_paths: unicode_paths(deny)?, + ..Default::default() + })?; + let write = collect_paths(normalized.readwrite_paths.into_iter().map(PathBuf::from))?; + let read = collect_paths(normalized.readonly_paths.into_iter().map(PathBuf::from))?; + let deny = collect_paths(normalized.denied_paths.into_iter().map(PathBuf::from))?; + // If expansion skips an unavailable writable volume, its generated read-only + // carveouts remain below. Pruning them requires retaining their root provenance, + // so we accept that rare normalization failure for now. + let mut write = expand_volume_roots(write, &volume_roots)?; + write.retain(|key, _| !carveouts.contains_key(key) && !deny.contains_key(key)); + let mut read = expand_volume_roots(read, &volume_roots)?; // Resolve equal-path write overrides before reaching MXC so the native // API receives one effective access mode for each path identity. read.retain(|key, _| !write.contains_key(key) && !deny.contains_key(key)); @@ -131,7 +169,7 @@ pub fn build_request( default: egress_default, ..Default::default() }; - Ok(ExecutionRequest { + let mut request = ExecutionRequest { script_code: cmdline_from_argv_for_context( &command.command, CommandLineContext::WindowsCreateProcess, @@ -169,7 +207,77 @@ pub fn build_request( ..Default::default() }, ..Default::default() - }) + }; + // Reconcile aliases introduced by expansion. MXC/PSEC separately enforces + // restrictions on junction-resolved targets at access time. + request.policy = normalize_policy(request.policy)?; + Ok(request) +} + +fn normalize_policy(mut policy: ContainerPolicy) -> Result { + let mut logger = Logger::new(Mode::Buffer); + if let Some(normalized) = + normalize_object_conflicts(&policy, &mut logger).map_err(PolicyError::PolicyResolution)? + { + policy = normalized; + } + Ok(policy) +} + +fn prune_unavailable_volume_roots(paths: &mut NativePaths, volume_roots: &HashSet) { + paths.retain(|key, path| { + if !volume_roots.contains(key) && path.parent().is_some() { + return true; + } + match std::fs::read_dir(path) { + Ok(_) => true, + Err(error) => !inaccessible_volume(&error), + } + }); +} + +// MXC volume-root grants are nonrecursive, so snapshot each root's current +// immediate children as recursive grants. A child created directly under the +// root after policy construction is not granted. MXC/PSEC still enforces +// restrictions on junction-resolved targets at access time. +fn expand_volume_roots( + paths: NativePaths, + volume_roots: &HashSet, +) -> Result { + let mut expanded = NativePaths::new(); + for (key, path) in paths { + if volume_roots.contains(&key) || path.parent().is_none() { + match std::fs::read_dir(&path).and_then(Iterator::collect::>>) { + Ok(entries) => { + for entry in entries { + let child = entry.path(); + if child.to_str().is_none() { + continue; + } + // A self-comparison is MXC's public object-identity probe. + // Generated grants can be skipped; explicit paths fail closed below. + if compare_existing_filesystem_objects(&child, &child) + != ExistingObjectComparison::Same + { + continue; + } + expanded.insert(PathUri::from_host_native_path(&child)?, child); + } + } + // GetLogicalDrives includes disconnected and empty removable + // drives. They must not prevent commands on available volumes. + Err(error) if inaccessible_volume(&error) => continue, + Err(error) => { + return Err(PolicyError::EnumerateVolume { + path, + source: error, + }); + } + } + } + expanded.insert(key, path); + } + Ok(expanded) } fn collect_paths(paths: impl IntoIterator) -> Result { @@ -179,6 +287,33 @@ fn collect_paths(paths: impl IntoIterator) -> Result bool { + if matches!( + error.kind(), + std::io::ErrorKind::PermissionDenied + | std::io::ErrorKind::NotFound + | std::io::ErrorKind::NetworkUnreachable + | std::io::ErrorKind::HostUnreachable + ) { + return true; + } + #[cfg(windows)] + { + use windows_sys::Win32::Foundation::ERROR_BAD_NETPATH; + use windows_sys::Win32::Foundation::ERROR_CONNECTION_UNAVAIL; + use windows_sys::Win32::Foundation::ERROR_DEVICE_NOT_CONNECTED; + use windows_sys::Win32::Foundation::ERROR_NOT_READY; + matches!( + error.raw_os_error().map(|code| code as u32), + Some(ERROR_NOT_READY | ERROR_DEVICE_NOT_CONNECTED | ERROR_BAD_NETPATH) + ) || error.raw_os_error() == Some(ERROR_CONNECTION_UNAVAIL as i32) + } + #[cfg(not(windows))] + { + false + } +} + fn unicode_paths(paths: NativePaths) -> Result, PolicyError> { let mut paths = paths .into_values() diff --git a/codex-rs/mxc-sandbox/src/policy_tests.rs b/codex-rs/mxc-sandbox/src/policy_tests.rs index 83b091197f..49d8093295 100644 --- a/codex-rs/mxc-sandbox/src/policy_tests.rs +++ b/codex-rs/mxc-sandbox/src/policy_tests.rs @@ -61,7 +61,7 @@ fn native_grants_preserve_denies_and_read_only_carveouts() -> Result<()> { ]); let profile = PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); - let request = build_request(&command(&profile, root), root, Vec::new())?; + let request = build_request(&command(&profile, root), root, Vec::new(), &[], &[])?; let mut expected_read = [ root.join(".agents"), root.join(".codex"), @@ -95,21 +95,171 @@ fn native_grants_preserve_denies_and_read_only_carveouts() -> Result<()> { } #[test] -fn unsupported_deny_globs_fail_before_launch() -> Result<()> { - let root = tempfile::tempdir()?; - let fs = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new( - FileSystemPath::GlobPattern { - pattern: "**/*.secret".to_owned(), - }, - FileSystemAccessMode::Deny, - )]); +fn volume_expansion_does_not_turn_read_only_child_writable() -> Result<()> { + let temp = tempfile::tempdir()?; + let canonical = canonical_root(&temp)?; + let root = canonical.as_path(); + let readonly = root.join("protected"); + let writable = root.join("work"); + std::fs::create_dir(&readonly)?; + std::fs::create_dir(&writable)?; + let fs = FileSystemSandboxPolicy::restricted(vec![ + entry(root, FileSystemAccessMode::Write)?, + entry(&readonly, FileSystemAccessMode::Read)?, + ]); let profile = PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); - let error = - build_request(&command(&profile, root.path()), root.path(), Vec::new()).unwrap_err(); + let request = build_request( + &command(&profile, root), + root, + Vec::new(), + &[root.to_owned()], + &[], + )?; assert_eq!( - error.to_string(), - "MXC does not support filesystem deny glob patterns" + request.policy.readwrite_paths, + vec![root.to_str().unwrap(), writable.to_str().unwrap()] + ); + assert_eq!( + request.policy.readonly_paths, + vec![ + root.join(".agents").to_str().unwrap(), + root.join(".codex").to_str().unwrap(), + root.join(".git").to_str().unwrap(), + readonly.to_str().unwrap() + ] + ); + Ok(()) +} + +#[test] +fn volume_expansion_does_not_turn_read_only_alias_writable() -> Result<()> { + let temp = tempfile::tempdir()?; + let root = canonical_root(&temp)?; + let readonly = root.join("protected"); + let alias = root.join("alias"); + std::fs::write(&readonly, "protected")?; + std::fs::hard_link(&readonly, &alias)?; + let fs = FileSystemSandboxPolicy::restricted(vec![ + entry(&root, FileSystemAccessMode::Write)?, + entry(&readonly, FileSystemAccessMode::Read)?, + ]); + let profile = + PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); + let request = build_request( + &command(&profile, &root), + &root, + Vec::new(), + std::slice::from_ref(&root), + &[], + )?; + let alias = alias.display().to_string(); + assert_eq!( + ( + request.policy.readwrite_paths.contains(&alias), + request.policy.readonly_paths.contains(&alias), + ), + (false, true) + ); + Ok(()) +} + +#[cfg(unix)] +#[test] +fn volume_expansion_uses_normalized_root_access() -> Result<()> { + let temp = tempfile::tempdir()?; + let root = canonical_root(&temp)?; + let volume = root.join("volume"); + let child = volume.join("child"); + let alias = root.join("alias"); + std::fs::create_dir_all(&child)?; + std::os::unix::fs::symlink(&volume, &alias)?; + let fs = FileSystemSandboxPolicy::restricted(vec![ + entry(&volume, FileSystemAccessMode::Write)?, + entry(&alias, FileSystemAccessMode::Read)?, + ]); + let profile = + PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); + let request = build_request( + &command(&profile, &root), + &root, + Vec::new(), + std::slice::from_ref(&volume), + &[], + )?; + let child = child.display().to_string(); + assert_eq!( + ( + request.policy.readwrite_paths.contains(&child), + request.policy.readonly_paths.contains(&child), + ), + (false, true) + ); + Ok(()) +} + +#[cfg(unix)] +#[test] +fn volume_expansion_skips_uninspectable_children() -> Result<()> { + let temp = tempfile::tempdir()?; + let root = canonical_root(&temp)?; + let uninspectable = root.join("loop"); + std::os::unix::fs::symlink(&uninspectable, &uninspectable)?; + let denied = root.join("secret"); + std::fs::write(&denied, "secret")?; + let fs = FileSystemSandboxPolicy::restricted(vec![ + entry(&root, FileSystemAccessMode::Write)?, + entry(&denied, FileSystemAccessMode::Deny)?, + ]); + let profile = + PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); + let request = build_request( + &command(&profile, &root), + &root, + Vec::new(), + std::slice::from_ref(&root), + &[], + )?; + assert_eq!( + (request.policy.readwrite_paths, request.policy.denied_paths), + ( + vec![root.to_str().unwrap().to_owned()], + vec![denied.to_str().unwrap().to_owned()], + ) + ); + Ok(()) +} + +#[test] +fn deny_globs_expand_files_and_directories_before_launch() -> Result<()> { + let temp = tempfile::tempdir()?; + let root = canonical_root(&temp)?; + let nested = root.join("nested"); + std::fs::create_dir(&nested)?; + let file = nested.join("file.secret"); + let directory = nested.join("directory.secret"); + std::fs::write(&file, "secret")?; + std::fs::create_dir(&directory)?; + std::fs::write(nested.join("allowed.txt"), "allowed")?; + let missing = root.join("explicit.secret"); + let fs = FileSystemSandboxPolicy::restricted(vec![ + entry(&root, FileSystemAccessMode::Write)?, + entry(&missing, FileSystemAccessMode::Deny)?, + FileSystemSandboxEntry::new( + FileSystemPath::GlobPattern { + pattern: "**/*.secret".to_owned(), + }, + FileSystemAccessMode::Deny, + ), + ]); + let profile = + PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); + let request = build_request(&command(&profile, &root), &nested, Vec::new(), &[], &[])?; + let mut expected = [file, directory, missing]; + expected.sort(); + assert_eq!( + request.policy.denied_paths, + expected.map(|path| path.to_str().unwrap().to_owned()) ); Ok(()) } @@ -125,8 +275,14 @@ fn relative_working_directories_fail_before_launch() -> Result<()> { (Path::new("relative"), root.path(), "policy"), (root.path(), Path::new("relative"), "command"), ] { - let error = - build_request(&command(&profile, policy_cwd), command_cwd, Vec::new()).unwrap_err(); + let error = build_request( + &command(&profile, policy_cwd), + command_cwd, + Vec::new(), + &[], + &[], + ) + .unwrap_err(); assert_eq!( error.to_string(), format!("MXC requires an absolute {kind} working directory") @@ -162,7 +318,8 @@ fn non_unicode_paths_fail_before_launch() -> Result<()> { )?]); let profile = PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); - let error = build_request(&command(&profile, &root), command_cwd, Vec::new()).unwrap_err(); + let error = build_request(&command(&profile, &root), command_cwd, Vec::new(), &[], &[]) + .unwrap_err(); assert_eq!(error.to_string(), expected); } Ok(())