Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 73 additions & 58 deletions codex-rs/windows-sandbox-rs/src/allow.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::policy::SandboxPolicy;
use crate::resolved_permissions::ResolvedWindowsSandboxPermissions;
use dunce::canonicalize;
use std::collections::HashMap;
use std::collections::HashSet;
Expand All @@ -11,9 +12,19 @@ pub struct AllowDenyPaths {
pub deny: HashSet<PathBuf>,
}

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<String, String>,
) -> 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<String, String>,
) -> AllowDenyPaths {
Expand All @@ -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 }
}

Expand Down Expand Up @@ -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);

Expand All @@ -162,6 +124,59 @@ 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<PathBuf> = [
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 ignores_unix_slash_tmp_for_windows_allow_roots() {
let tmp = TempDir::new().expect("tempdir");
let command_cwd = tmp.path().join("workspace");
let _ = fs::create_dir_all(&command_cwd);

let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: false,
};

let paths = compute_allow_paths(&policy, &command_cwd, &command_cwd, &HashMap::new());
let expected_allow: HashSet<PathBuf> = [dunce::canonicalize(&command_cwd).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");
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/windows-sandbox-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
116 changes: 116 additions & 0 deletions codex-rs/windows-sandbox-rs/src/resolved_permissions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
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<PathBuf>,
}

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<String, String>,
) -> Vec<WindowsWritableRoot> {
let mut file_system = self.file_system.clone();
file_system
.entries
.retain(|FileSystemSandboxEntry { path, .. }| {
!matches!(
path,
FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::Tmpdir
| codex_protocol::permissions::FileSystemSpecialPath::SlashTmp,
}
)
});

let mut roots = file_system
.get_writable_roots_with_cwd(cwd)
Comment thread
bolinfest marked this conversation as resolved.
Comment thread
bolinfest marked this conversation as resolved.
.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::<Vec<_>>();

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<String, String>) -> Vec<PathBuf> {
["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()
}
7 changes: 1 addition & 6 deletions codex-rs/windows-sandbox-rs/src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,14 +395,9 @@ pub(crate) fn gather_write_roots(
command_cwd: &Path,
env_map: &HashMap<String, String>,
) -> Vec<PathBuf> {
let mut roots: Vec<PathBuf> = 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<PathBuf> = allow.into_iter().collect();
let mut dedup: HashSet<PathBuf> = HashSet::new();
let mut out: Vec<PathBuf> = Vec::new();
for r in canonical_existing(&roots) {
Expand Down
3 changes: 2 additions & 1 deletion codex-rs/windows-sandbox-rs/src/spawn_prep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,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;
Expand Down Expand Up @@ -73,7 +74,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(
Expand Down
Loading