From ce254df05a3162a93d8f3357ff4dd86582c534b7 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Tue, 8 Sep 2026 16:26:59 +0000 Subject: [PATCH] Add canonical permission translation for MXC execution requests (#43853) ## What changed Add `policy::build_request` to `codex-rs/mxc-sandbox` to translate `MxcCommand` permissions into native MXC execution requests. Preserve filesystem denies, read-only carveouts, and protected metadata paths, and resolve overlapping grants using canonical path identities. Map network permissions to ingress, egress, and loopback policies; quote arguments for Windows process creation; and disable DACL mutation fallback. Reject unsupported symbolic roots and deny globs, empty commands, relative working directories, and non-Unicode request paths. ## Testing Enable unit tests and add coverage for filesystem grants and carveouts, disabled DACL mutation, deny-glob rejection, relative working directories, and non-Unicode paths. GitOrigin-RevId: 3e647ed5fa932ab9d528eb44a49dd9283ef50481 --- codex-rs/Cargo.lock | 5 + codex-rs/mxc-sandbox/Cargo.toml | 6 +- codex-rs/mxc-sandbox/src/lib.rs | 6 + codex-rs/mxc-sandbox/src/policy.rs | 193 +++++++++++++++++++++++ codex-rs/mxc-sandbox/src/policy_tests.rs | 169 ++++++++++++++++++++ 5 files changed, 378 insertions(+), 1 deletion(-) create mode 100644 codex-rs/mxc-sandbox/src/policy.rs create mode 100644 codex-rs/mxc-sandbox/src/policy_tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index fa30609770..d57f141bf5 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4035,7 +4035,12 @@ dependencies = [ "anyhow", "appcontainer_common", "codex-protocol", + "codex-utils-absolute-path", + "codex-utils-path-uri", "learning_mode_windows", + "pretty_assertions", + "tempfile", + "thiserror 2.0.18", "tracelogging", "wxc_common", ] diff --git a/codex-rs/mxc-sandbox/Cargo.toml b/codex-rs/mxc-sandbox/Cargo.toml index 33bb833ee3..cf38f09e07 100644 --- a/codex-rs/mxc-sandbox/Cargo.toml +++ b/codex-rs/mxc-sandbox/Cargo.toml @@ -5,7 +5,6 @@ edition.workspace = true license.workspace = true [lib] -test = false doctest = false [lints] @@ -18,6 +17,9 @@ ignored = ["tracelogging"] [dependencies] anyhow = { workspace = true } codex-protocol = { workspace = true } +codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } +thiserror = { workspace = true } wxc_common = { workspace = true } [target.'cfg(windows)'.dependencies] @@ -27,3 +29,5 @@ learning_mode_windows = { workspace = true } tracelogging = "=1.2.3" [dev-dependencies] +pretty_assertions = { workspace = true } +tempfile = { workspace = true } diff --git a/codex-rs/mxc-sandbox/src/lib.rs b/codex-rs/mxc-sandbox/src/lib.rs index 6072925828..cbf7123d64 100644 --- a/codex-rs/mxc-sandbox/src/lib.rs +++ b/codex-rs/mxc-sandbox/src/lib.rs @@ -13,6 +13,8 @@ pub struct MxcCommand { pub command: Vec, } +pub mod policy; + /// Whether the executor can create a native MXC process security environment. /// This deliberately excludes MXC's older AppContainer fallback backends. pub fn is_available() -> bool { @@ -25,3 +27,7 @@ pub fn is_available() -> bool { false } } + +#[cfg(test)] +#[path = "policy_tests.rs"] +mod tests; diff --git a/codex-rs/mxc-sandbox/src/policy.rs b/codex-rs/mxc-sandbox/src/policy.rs new file mode 100644 index 0000000000..9011a341d3 --- /dev/null +++ b/codex-rs/mxc-sandbox/src/policy.rs @@ -0,0 +1,193 @@ +//! Translate canonical permissions into native MXC grants without expanding +//! access. Denies and read-only carveouts remain separate kernel policy lists. + +use std::collections::HashMap; +use std::path::Path; +use std::path::PathBuf; + +use codex_protocol::protocol::FileSystemPath; +use codex_protocol::protocol::FileSystemSpecialPath; +use codex_utils_path_uri::PathUri; +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::models::ContainerPolicy; +use wxc_common::models::ExecutionRequest; +use wxc_common::models::FallbackPolicy; +use wxc_common::models::NetworkAction; +use wxc_common::models::NetworkEgressPolicy; +use wxc_common::models::NetworkIngressPolicy; +use wxc_common::models::NetworkPolicy; + +use crate::MxcCommand; + +// Use the same Windows case/URI identity as the canonical permission model, +// while retaining native spellings for the OS API. +type NativePaths = HashMap; + +/// Invalid inputs or unsupported permissions encountered during policy translation. +#[derive(Debug, Error)] +pub enum PolicyError { + #[error("MXC command must not be empty")] + EmptyCommand, + #[error("MXC requires an absolute policy working directory")] + RelativePolicyCwd, + #[error("MXC requires an absolute command working directory")] + 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(transparent)] + Path(#[from] std::io::Error), + #[error(transparent)] + CommandLine(#[from] CommandLineError), +} + +pub fn build_request( + command: &MxcCommand, + command_cwd: &Path, + env: Vec, +) -> Result { + if command.command.is_empty() { + return Err(PolicyError::EmptyCommand); + } + let permissions = &command.permissions; + let cwd = &command.sandbox_policy_cwd; + if !cwd.is_absolute() { + return Err(PolicyError::RelativePolicyCwd); + } + if !command_cwd.is_absolute() { + return Err(PolicyError::RelativeCommandCwd); + } + let fs = permissions.file_system_sandbox_policy(); + if fs.has_full_disk_write_access() + || fs.entries.iter().any(|entry| match &entry.path { + FileSystemPath::Path { .. } | FileSystemPath::GlobPattern { .. } => false, + FileSystemPath::Special { value } => { + !matches!(value, FileSystemSpecialPath::Unknown { .. }) + } + }) + { + 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( + fs.get_readable_roots_with_cwd(cwd) + .into_iter() + .filter(|path| !fs.can_write_local_path_with_cwd(path.as_path(), cwd)) + .map(|path| path.to_path_buf()), + )?; + let deny = collect_paths( + fs.get_unreadable_roots_with_cwd(cwd) + .into_iter() + .map(|path| path.to_path_buf()), + )?; + let carveouts = collect_paths(roots.into_iter().flat_map(|root| { + let protected = root + .protected_metadata_names + .into_iter() + .map(|name| root.root.join(name).to_path_buf()); + root.read_only_subpaths + .into_iter() + .map(|path| path.to_path_buf()) + .chain(protected) + .collect::>() + }))?; + // Write restrictions must not grant otherwise forbidden reads. + read.extend( + carveouts + .iter() + .filter(|(_, path)| fs.can_read_local_path_with_cwd(path.as_path(), cwd)) + .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)); + // 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)); + let network_enabled = permissions.network_sandbox_policy().is_enabled(); + let egress_default = if network_enabled { + NetworkAction::Allow + } else { + NetworkAction::Deny + }; + let ingress_default = if network_enabled { + NetworkAction::Allow + } else { + NetworkAction::Deny + }; + let egress = NetworkEgressPolicy { + default: egress_default, + ..Default::default() + }; + Ok(ExecutionRequest { + script_code: cmdline_from_argv_for_context( + &command.command, + CommandLineContext::WindowsCreateProcess, + )?, + working_directory: command_cwd + .to_str() + .ok_or(PolicyError::NonUnicodeCommandCwd)? + .to_owned(), + env, + policy: ContainerPolicy { + capabilities: vec!["registryRead".to_owned()], + readwrite_paths: unicode_paths(write)?, + readonly_paths: unicode_paths(read)?, + denied_paths: unicode_paths(deny)?, + fallback: FallbackPolicy { + allow_dacl_mutation: false, + }, + default_network_policy: if egress_default == NetworkAction::Allow { + NetworkPolicy::Allow + } else { + NetworkPolicy::Block + }, + allow_local_network: ingress_default == NetworkAction::Allow, + network_egress: Some(egress), + network_ingress: Some(NetworkIngressPolicy { + default: ingress_default, + host_loopback: if network_enabled { + NetworkAction::Allow + } else { + NetworkAction::Deny + }, + }), + network_specified: true, + network_mode_specified: true, + ..Default::default() + }, + ..Default::default() + }) +} + +fn collect_paths(paths: impl IntoIterator) -> Result { + paths + .into_iter() + .map(|path| Ok((PathUri::from_host_native_path(&path)?, path))) + .collect() +} + +fn unicode_paths(paths: NativePaths) -> Result, PolicyError> { + let mut paths = paths + .into_values() + .map(|path| { + path.into_os_string() + .into_string() + .map_err(|_| PolicyError::NonUnicodePolicyPath) + }) + .collect::, _>>()?; + paths.sort(); + Ok(paths) +} diff --git a/codex-rs/mxc-sandbox/src/policy_tests.rs b/codex-rs/mxc-sandbox/src/policy_tests.rs new file mode 100644 index 0000000000..83b091197f --- /dev/null +++ b/codex-rs/mxc-sandbox/src/policy_tests.rs @@ -0,0 +1,169 @@ +//! Portable policy and launcher regressions, with Windows identity coverage. + +use std::path::Path; +use std::path::PathBuf; + +use anyhow::Result; +use codex_protocol::models::PermissionProfile; +use codex_protocol::protocol::FileSystemAccessMode; +use codex_protocol::protocol::FileSystemPath; +use codex_protocol::protocol::FileSystemSandboxEntry; +use codex_protocol::protocol::FileSystemSandboxPolicy; +use codex_protocol::protocol::NetworkSandboxPolicy; +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; +use pretty_assertions::assert_eq; + +use crate::MxcCommand; +use crate::policy::build_request; + +fn canonical_root(temp: &tempfile::TempDir) -> Result { + Ok( + PathUri::from_host_native_path(std::fs::canonicalize(temp.path())?)? + .to_abs_path()? + .into_path_buf(), + ) +} + +fn entry(path: &Path, access: FileSystemAccessMode) -> Result { + Ok(FileSystemSandboxEntry::new( + AbsolutePathBuf::from_absolute_path(path)?.into(), + access, + )) +} + +fn command(permissions: &PermissionProfile, cwd: &Path) -> MxcCommand { + MxcCommand { + permissions: permissions.clone(), + sandbox_policy_cwd: cwd.to_owned(), + command: vec!["program.exe".to_owned(), "--arg".to_owned()], + } +} + +#[test] +fn native_grants_preserve_denies_and_read_only_carveouts() -> Result<()> { + let temp = tempfile::tempdir()?; + let canonical = canonical_root(&temp)?; + let root = canonical.as_path(); + let gitdir = tempfile::tempdir()?; + std::fs::write( + root.join(".git"), + format!("gitdir: {}", gitdir.path().display()), + )?; + let readonly = root.join("readonly"); + let writable_child = readonly.join("writable"); + let denied = root.join("secret"); + let fs = FileSystemSandboxPolicy::restricted(vec![ + entry(root, FileSystemAccessMode::Write)?, + entry(&readonly, FileSystemAccessMode::Read)?, + entry(&writable_child, 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())?; + let mut expected_read = [ + root.join(".agents"), + root.join(".codex"), + root.join(".git"), + readonly, + writable_child.join(".agents"), + writable_child.join(".codex"), + writable_child.join(".git"), + ]; + expected_read.sort(); + assert_eq!( + ( + request.policy.readwrite_paths, + request.policy.readonly_paths, + request.policy.denied_paths + ), + ( + vec![ + root.to_str().unwrap().to_owned(), + writable_child.to_str().unwrap().to_owned() + ], + expected_read + .iter() + .map(|path| path.to_str().unwrap().to_owned()) + .collect::>(), + vec![denied.to_str().unwrap().to_owned()], + ) + ); + assert!(!request.policy.fallback.allow_dacl_mutation); + Ok(()) +} + +#[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, + )]); + let profile = + PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); + let error = + build_request(&command(&profile, root.path()), root.path(), Vec::new()).unwrap_err(); + assert_eq!( + error.to_string(), + "MXC does not support filesystem deny glob patterns" + ); + Ok(()) +} + +#[test] +fn relative_working_directories_fail_before_launch() -> Result<()> { + let root = tempfile::tempdir()?; + let profile = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(Vec::new()), + NetworkSandboxPolicy::Restricted, + ); + for (policy_cwd, command_cwd, kind) in [ + (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(); + assert_eq!( + error.to_string(), + format!("MXC requires an absolute {kind} working directory") + ); + } + Ok(()) +} + +#[test] +fn non_unicode_paths_fail_before_launch() -> Result<()> { + #[cfg(unix)] + let name = ::from_vec(vec![0xff]); + #[cfg(windows)] + let name = ::from_wide(&[0xd800]); + let temp = tempfile::tempdir()?; + let root = canonical_root(&temp)?; + let invalid = root.join(name); + for (policy_path, command_cwd, expected) in [ + ( + root.as_path(), + invalid.as_path(), + "MXC requires a Unicode command working directory", + ), + ( + invalid.as_path(), + root.as_path(), + "MXC requires Unicode filesystem policy paths", + ), + ] { + let fs = FileSystemSandboxPolicy::restricted(vec![entry( + policy_path, + FileSystemAccessMode::Deny, + )?]); + let profile = + PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted); + let error = build_request(&command(&profile, &root), command_cwd, Vec::new()).unwrap_err(); + assert_eq!(error.to_string(), expected); + } + Ok(()) +}