Protect workspace .agents directory in Windows sandbox (#11970)
The Mac and Linux implementations of the sandbox recently added write protections for `.codex` and `.agents` subdirectories in all writable roots. When adding documentation for this, I noticed that this change was never made for the Windows sandbox. Summary - make compute_allow_paths treat .codex/.agents as protected alongside .git, and cover their behavior in new tests - wire protect_workspace_agents_dir through the sandbox lib and setup path to apply deny ACEs when `.agents` exists - factor shared ACL logic for workspace subdirectories
This commit is contained in:
parent
31906cdb4d
commit
5296e06b61
4 changed files with 85 additions and 11 deletions
|
|
@ -52,9 +52,11 @@ pub fn compute_allow_paths(
|
|||
let canonical = canonicalize(&candidate).unwrap_or(candidate);
|
||||
add_allow(canonical.clone());
|
||||
|
||||
let git_entry = canonical.join(".git");
|
||||
if git_entry.exists() {
|
||||
add_deny(git_entry);
|
||||
for protected_subdir in [".git", ".codex", ".agents"] {
|
||||
let protected_entry = canonical.join(protected_subdir);
|
||||
if protected_entry.exists() {
|
||||
add_deny(protected_entry);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
|
|
@ -210,7 +212,39 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn skips_git_dir_when_missing() {
|
||||
fn denies_codex_and_agents_inside_writable_root() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let command_cwd = tmp.path().join("workspace");
|
||||
let codex_dir = command_cwd.join(".codex");
|
||||
let agents_dir = command_cwd.join(".agents");
|
||||
let _ = fs::create_dir_all(&codex_dir);
|
||||
let _ = fs::create_dir_all(&agents_dir);
|
||||
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
read_only_access: Default::default(),
|
||||
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();
|
||||
let expected_deny: HashSet<PathBuf> = [
|
||||
dunce::canonicalize(&codex_dir).unwrap(),
|
||||
dunce::canonicalize(&agents_dir).unwrap(),
|
||||
]
|
||||
.into_iter()
|
||||
.collect();
|
||||
|
||||
assert_eq!(expected_allow, paths.allow);
|
||||
assert_eq!(expected_deny, paths.deny);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skips_protected_subdirs_when_missing() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let command_cwd = tmp.path().join("workspace");
|
||||
let _ = fs::create_dir_all(&command_cwd);
|
||||
|
|
@ -225,6 +259,6 @@ mod tests {
|
|||
|
||||
let paths = compute_allow_paths(&policy, &command_cwd, &command_cwd, &HashMap::new());
|
||||
assert_eq!(paths.allow.len(), 1);
|
||||
assert!(paths.deny.is_empty(), "no deny when .git is absent");
|
||||
assert!(paths.deny.is_empty(), "no deny when protected dirs are absent");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -127,6 +127,8 @@ pub use winutil::to_wide;
|
|||
#[cfg(target_os = "windows")]
|
||||
pub use workspace_acl::is_command_cwd_root;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use workspace_acl::protect_workspace_agents_dir;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use workspace_acl::protect_workspace_codex_dir;
|
||||
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
|
|
@ -165,6 +167,7 @@ mod windows_impl {
|
|||
use super::winutil::quote_windows_arg;
|
||||
use super::winutil::to_wide;
|
||||
use super::workspace_acl::is_command_cwd_root;
|
||||
use super::workspace_acl::protect_workspace_agents_dir;
|
||||
use super::workspace_acl::protect_workspace_codex_dir;
|
||||
use anyhow::Result;
|
||||
use std::collections::HashMap;
|
||||
|
|
@ -344,6 +347,7 @@ mod windows_impl {
|
|||
if let Some(psid) = psid_workspace {
|
||||
allow_null_device(psid);
|
||||
let _ = protect_workspace_codex_dir(¤t_dir, psid);
|
||||
let _ = protect_workspace_agents_dir(¤t_dir, psid);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -552,6 +556,7 @@ mod windows_impl {
|
|||
allow_null_device(psid_generic);
|
||||
allow_null_device(psid_workspace);
|
||||
let _ = protect_workspace_codex_dir(¤t_dir, psid_workspace);
|
||||
let _ = protect_workspace_agents_dir(¤t_dir, psid_workspace);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ use codex_windows_sandbox::is_command_cwd_root;
|
|||
use codex_windows_sandbox::load_or_create_cap_sids;
|
||||
use codex_windows_sandbox::log_note;
|
||||
use codex_windows_sandbox::path_mask_allows;
|
||||
use codex_windows_sandbox::protect_workspace_agents_dir;
|
||||
use codex_windows_sandbox::protect_workspace_codex_dir;
|
||||
use codex_windows_sandbox::sandbox_dir;
|
||||
use codex_windows_sandbox::sandbox_secrets_dir;
|
||||
|
|
@ -788,9 +789,9 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
|
|||
}
|
||||
}
|
||||
|
||||
// Protect the current workspace's `.codex` directory from tampering (write/delete) by using a
|
||||
// workspace-specific capability SID. If `.codex` doesn't exist yet, skip it (it will be picked
|
||||
// up on the next refresh).
|
||||
// Protect the current workspace's `.codex` and `.agents` directories from tampering
|
||||
// (write/delete) by using a workspace-specific capability SID. If a directory doesn't exist
|
||||
// yet, skip it (it will be picked up on the next refresh).
|
||||
match unsafe { protect_workspace_codex_dir(&payload.command_cwd, workspace_psid) } {
|
||||
Ok(true) => {
|
||||
let cwd_codex = payload.command_cwd.join(".codex");
|
||||
|
|
@ -812,6 +813,30 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
|
|||
)?;
|
||||
}
|
||||
}
|
||||
match unsafe { protect_workspace_agents_dir(&payload.command_cwd, workspace_psid) } {
|
||||
Ok(true) => {
|
||||
let cwd_agents = payload.command_cwd.join(".agents");
|
||||
log_line(
|
||||
log,
|
||||
&format!(
|
||||
"applied deny ACE to protect workspace .agents {}",
|
||||
cwd_agents.display()
|
||||
),
|
||||
)?;
|
||||
}
|
||||
Ok(false) => {}
|
||||
Err(err) => {
|
||||
let cwd_agents = payload.command_cwd.join(".agents");
|
||||
refresh_errors.push(format!(
|
||||
"deny ACE failed on {}: {err}",
|
||||
cwd_agents.display()
|
||||
));
|
||||
log_line(
|
||||
log,
|
||||
&format!("deny ACE failed on {}: {err}", cwd_agents.display()),
|
||||
)?;
|
||||
}
|
||||
}
|
||||
unsafe {
|
||||
if !sandbox_group_psid.is_null() {
|
||||
LocalFree(sandbox_group_psid as HLOCAL);
|
||||
|
|
|
|||
|
|
@ -11,9 +11,19 @@ pub fn is_command_cwd_root(root: &Path, canonical_command_cwd: &Path) -> bool {
|
|||
/// # Safety
|
||||
/// Caller must ensure `psid` is a valid SID pointer.
|
||||
pub unsafe fn protect_workspace_codex_dir(cwd: &Path, psid: *mut c_void) -> Result<bool> {
|
||||
let cwd_codex = cwd.join(".codex");
|
||||
if cwd_codex.is_dir() {
|
||||
add_deny_write_ace(&cwd_codex, psid)
|
||||
protect_workspace_subdir(cwd, psid, ".codex")
|
||||
}
|
||||
|
||||
/// # Safety
|
||||
/// Caller must ensure `psid` is a valid SID pointer.
|
||||
pub unsafe fn protect_workspace_agents_dir(cwd: &Path, psid: *mut c_void) -> Result<bool> {
|
||||
protect_workspace_subdir(cwd, psid, ".agents")
|
||||
}
|
||||
|
||||
unsafe fn protect_workspace_subdir(cwd: &Path, psid: *mut c_void, subdir: &str) -> Result<bool> {
|
||||
let path = cwd.join(subdir);
|
||||
if path.is_dir() {
|
||||
add_deny_write_ace(&path, psid)
|
||||
} else {
|
||||
Ok(false)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue