From e0c614b2da85368bddbfd76f59366e84ffed103f Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 15 May 2026 13:41:52 -0700 Subject: [PATCH] windows-sandbox: add resolved permissions helper --- codex-rs/windows-sandbox-rs/src/allow.rs | 109 ++++++++---------- codex-rs/windows-sandbox-rs/src/lib.rs | 2 + .../src/resolved_permissions.rs | 104 +++++++++++++++++ codex-rs/windows-sandbox-rs/src/setup.rs | 7 +- codex-rs/windows-sandbox-rs/src/spawn_prep.rs | 3 +- 5 files changed, 160 insertions(+), 65 deletions(-) create mode 100644 codex-rs/windows-sandbox-rs/src/resolved_permissions.rs diff --git a/codex-rs/windows-sandbox-rs/src/allow.rs b/codex-rs/windows-sandbox-rs/src/allow.rs index 5b6d0d617a..3746359ab9 100644 --- a/codex-rs/windows-sandbox-rs/src/allow.rs +++ b/codex-rs/windows-sandbox-rs/src/allow.rs @@ -1,4 +1,5 @@ use crate::policy::SandboxPolicy; +use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; use dunce::canonicalize; use std::collections::HashMap; use std::collections::HashSet; @@ -11,9 +12,19 @@ pub struct AllowDenyPaths { pub deny: HashSet, } -pub fn compute_allow_paths( +pub(crate) fn compute_allow_paths( policy: &SandboxPolicy, - policy_cwd: &Path, + _policy_cwd: &Path, + command_cwd: &Path, + env_map: &HashMap, +) -> AllowDenyPaths { + let permissions = + ResolvedWindowsSandboxPermissions::from_legacy_policy_for_cwd(policy, command_cwd); + compute_allow_paths_for_permissions(&permissions, command_cwd, env_map) +} + +pub(crate) fn compute_allow_paths_for_permissions( + permissions: &ResolvedWindowsSandboxPermissions, command_cwd: &Path, env_map: &HashMap, ) -> AllowDenyPaths { @@ -30,65 +41,15 @@ pub fn compute_allow_paths( deny.insert(p); } }; - let include_tmp_env_vars = matches!( - policy, - SandboxPolicy::WorkspaceWrite { - exclude_tmpdir_env_var: false, - .. - } - ); - if matches!(policy, SandboxPolicy::WorkspaceWrite { .. }) { - let add_writable_root = - |root: PathBuf, - policy_cwd: &Path, - add_allow: &mut dyn FnMut(PathBuf), - add_deny: &mut dyn FnMut(PathBuf)| { - let candidate = if root.is_absolute() { - root - } else { - policy_cwd.join(root) - }; - let canonical = canonicalize(&candidate).unwrap_or(candidate); - add_allow(canonical.clone()); - - for protected_subdir in [".git", ".codex", ".agents"] { - let protected_entry = canonical.join(protected_subdir); - if protected_entry.exists() { - add_deny(protected_entry); - } - } - }; - - add_writable_root( - command_cwd.to_path_buf(), - policy_cwd, - &mut add_allow_path, - &mut add_deny_path, - ); - - if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = policy { - for root in writable_roots { - add_writable_root( - root.clone().into(), - policy_cwd, - &mut add_allow_path, - &mut add_deny_path, - ); - } - } - } - if include_tmp_env_vars { - for key in ["TEMP", "TMP"] { - if let Some(v) = env_map.get(key) { - let abs = PathBuf::from(v); - add_allow_path(abs); - } else if let Ok(v) = std::env::var(key) { - let abs = PathBuf::from(v); - add_allow_path(abs); - } + for writable_root in permissions.writable_roots_for_cwd(command_cwd, env_map) { + let canonical = canonicalize(&writable_root.root).unwrap_or(writable_root.root); + add_allow_path(canonical); + for read_only_subpath in writable_root.read_only_subpaths { + add_deny_path(read_only_subpath); } } + AllowDenyPaths { allow, deny } } @@ -146,6 +107,7 @@ mod tests { }; let mut env_map = HashMap::new(); env_map.insert("TEMP".into(), temp_dir.to_string_lossy().to_string()); + env_map.insert("TMP".into(), temp_dir.to_string_lossy().to_string()); let paths = compute_allow_paths(&policy, &command_cwd, &command_cwd, &env_map); @@ -162,6 +124,37 @@ mod tests { assert!(paths.deny.is_empty(), "no deny paths expected"); } + #[test] + fn includes_tmp_env_vars_when_requested() { + let tmp = TempDir::new().expect("tempdir"); + let command_cwd = tmp.path().join("workspace"); + let temp_dir = tmp.path().join("temp"); + let _ = fs::create_dir_all(&command_cwd); + let _ = fs::create_dir_all(&temp_dir); + + let policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + network_access: false, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + }; + let mut env_map = HashMap::new(); + env_map.insert("TEMP".into(), temp_dir.to_string_lossy().to_string()); + env_map.insert("TMP".into(), temp_dir.to_string_lossy().to_string()); + + let paths = compute_allow_paths(&policy, &command_cwd, &command_cwd, &env_map); + + let expected_allow: HashSet = [ + dunce::canonicalize(&command_cwd).unwrap(), + dunce::canonicalize(&temp_dir).unwrap(), + ] + .into_iter() + .collect(); + + assert_eq!(expected_allow, paths.allow); + assert!(paths.deny.is_empty(), "no deny paths expected"); + } + #[test] fn denies_git_dir_inside_writable_root() { let tmp = TempDir::new().expect("tempdir"); diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index db958c9975..21504b2f81 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -38,6 +38,8 @@ mod policy; #[cfg(target_os = "windows")] mod process; #[cfg(target_os = "windows")] +mod resolved_permissions; +#[cfg(target_os = "windows")] mod token; #[cfg(target_os = "windows")] mod wfp; diff --git a/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs b/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs new file mode 100644 index 0000000000..9165940ff2 --- /dev/null +++ b/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs @@ -0,0 +1,104 @@ +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::protocol::SandboxPolicy; +use codex_utils_absolute_path::AbsolutePathBuf; +use std::collections::HashMap; +use std::path::Path; +use std::path::PathBuf; + +/// Windows-local view of the runtime permission profile. +/// +/// Most Windows sandbox code needs resolved runtime permissions plus a few +/// Windows-specific path conventions, not the user/config-facing +/// `PermissionProfile` enum itself. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ResolvedWindowsSandboxPermissions { + file_system: FileSystemSandboxPolicy, + network: NetworkSandboxPolicy, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct WindowsWritableRoot { + pub(crate) root: PathBuf, + pub(crate) read_only_subpaths: Vec, +} + +impl ResolvedWindowsSandboxPermissions { + pub(crate) fn from_legacy_policy(policy: &SandboxPolicy) -> Self { + Self { + file_system: FileSystemSandboxPolicy::from(policy), + network: NetworkSandboxPolicy::from(policy), + } + } + + pub(crate) fn from_legacy_policy_for_cwd(policy: &SandboxPolicy, cwd: &Path) -> Self { + Self { + file_system: FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(policy, cwd), + network: NetworkSandboxPolicy::from(policy), + } + } + + pub(crate) fn should_apply_network_block(&self) -> bool { + !self.network.is_enabled() + } + + pub(crate) fn writable_roots_for_cwd( + &self, + cwd: &Path, + env_map: &HashMap, + ) -> Vec { + let mut roots = self + .file_system + .get_writable_roots_with_cwd(cwd) + .into_iter() + .map(|root| WindowsWritableRoot { + root: root.root.into_path_buf(), + read_only_subpaths: root + .read_only_subpaths + .into_iter() + .map(AbsolutePathBuf::into_path_buf) + .collect(), + }) + .collect::>(); + + if self.has_writable_tmpdir_entry() { + roots.extend(windows_temp_env_roots(env_map).into_iter().map(|root| { + WindowsWritableRoot { + root, + read_only_subpaths: Vec::new(), + } + })); + } + + roots + } + + fn has_writable_tmpdir_entry(&self) -> bool { + self.file_system + .entries + .iter() + .any(|FileSystemSandboxEntry { path, access }| { + matches!( + path, + FileSystemPath::Special { + value: codex_protocol::permissions::FileSystemSpecialPath::Tmpdir, + } + ) && access.can_write() + }) + } +} + +fn windows_temp_env_roots(env_map: &HashMap) -> Vec { + ["TEMP", "TMP"] + .into_iter() + .filter_map(|key| { + env_map + .get(key) + .map(|value| PathBuf::from(value.as_str())) + .or_else(|| std::env::var_os(key).map(PathBuf::from)) + }) + .filter(|path| path.is_absolute()) + .collect() +} diff --git a/codex-rs/windows-sandbox-rs/src/setup.rs b/codex-rs/windows-sandbox-rs/src/setup.rs index 0de2c6e029..6f4dd86019 100644 --- a/codex-rs/windows-sandbox-rs/src/setup.rs +++ b/codex-rs/windows-sandbox-rs/src/setup.rs @@ -395,14 +395,9 @@ pub(crate) fn gather_write_roots( command_cwd: &Path, env_map: &HashMap, ) -> Vec { - let mut roots: Vec = Vec::new(); - // Always include the command CWD for workspace-write. - if matches!(policy, SandboxPolicy::WorkspaceWrite { .. }) { - roots.push(command_cwd.to_path_buf()); - } let AllowDenyPaths { allow, .. } = compute_allow_paths(policy, policy_cwd, command_cwd, env_map); - roots.extend(allow); + let roots: Vec = allow.into_iter().collect(); let mut dedup: HashSet = HashSet::new(); let mut out: Vec = Vec::new(); for r in canonical_existing(&roots) { diff --git a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs index 331df4f438..07881b54d4 100644 --- a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs +++ b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs @@ -20,6 +20,7 @@ use crate::logging::log_start; use crate::path_normalization::canonicalize_path; use crate::policy::SandboxPolicy; use crate::policy::parse_policy; +use crate::resolved_permissions::ResolvedWindowsSandboxPermissions; use crate::sandbox_utils::ensure_codex_home_exists; use crate::sandbox_utils::inject_git_safe_directory; use crate::setup::effective_write_roots_for_setup; @@ -74,7 +75,7 @@ pub(crate) struct LegacyAclSids<'a> { } pub(crate) fn should_apply_network_block(policy: &SandboxPolicy) -> bool { - !policy.has_full_network_access() + ResolvedWindowsSandboxPermissions::from_legacy_policy(policy).should_apply_network_block() } fn prepare_spawn_context_common(