mirror of
https://github.com/openai/codex.git
synced 2026-09-20 12:47:38 +00:00
Protect first-time project .codex creation in Windows sandbox
Reserve missing writable-root .codex directories before applying deny ACLs so first-time creation cannot bypass Windows sandbox protection. Keep .git and .agents behavior unchanged and add Windows-focused regression coverage for reserved .codex handling.
This commit is contained in:
@@ -26,9 +26,7 @@ pub fn compute_allow_paths(
|
||||
}
|
||||
};
|
||||
let mut add_deny_path = |p: PathBuf| {
|
||||
if p.exists() {
|
||||
deny.insert(p);
|
||||
}
|
||||
deny.insert(p);
|
||||
};
|
||||
let include_tmp_env_vars = matches!(
|
||||
policy,
|
||||
@@ -52,12 +50,14 @@ pub fn compute_allow_paths(
|
||||
let canonical = canonicalize(&candidate).unwrap_or(candidate);
|
||||
add_allow(canonical.clone());
|
||||
|
||||
for protected_subdir in [".git", ".codex", ".agents"] {
|
||||
for protected_subdir in [".git", ".agents"] {
|
||||
let protected_entry = canonical.join(protected_subdir);
|
||||
if protected_entry.exists() {
|
||||
add_deny(protected_entry);
|
||||
}
|
||||
}
|
||||
// Reserve project `.codex` even before it exists so first creation stays protected.
|
||||
add_deny(canonical.join(".codex"));
|
||||
};
|
||||
|
||||
add_writable_root(
|
||||
@@ -124,7 +124,14 @@ mod tests {
|
||||
assert!(paths
|
||||
.allow
|
||||
.contains(&dunce::canonicalize(&extra_root).unwrap()));
|
||||
assert!(paths.deny.is_empty(), "no deny paths expected");
|
||||
let expected_deny: HashSet<PathBuf> = [
|
||||
dunce::canonicalize(&command_cwd).unwrap().join(".codex"),
|
||||
dunce::canonicalize(&extra_root).unwrap().join(".codex"),
|
||||
]
|
||||
.into_iter()
|
||||
.collect();
|
||||
|
||||
assert_eq!(expected_deny, paths.deny);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -153,7 +160,13 @@ mod tests {
|
||||
assert!(!paths
|
||||
.allow
|
||||
.contains(&dunce::canonicalize(&temp_dir).unwrap()));
|
||||
assert!(paths.deny.is_empty(), "no deny paths expected");
|
||||
let expected_deny: HashSet<PathBuf> = [dunce::canonicalize(&command_cwd)
|
||||
.unwrap()
|
||||
.join(".codex")]
|
||||
.into_iter()
|
||||
.collect();
|
||||
|
||||
assert_eq!(expected_deny, paths.deny);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -175,9 +188,12 @@ mod tests {
|
||||
let expected_allow: HashSet<PathBuf> = [dunce::canonicalize(&command_cwd).unwrap()]
|
||||
.into_iter()
|
||||
.collect();
|
||||
let expected_deny: HashSet<PathBuf> = [dunce::canonicalize(&git_dir).unwrap()]
|
||||
.into_iter()
|
||||
.collect();
|
||||
let expected_deny: HashSet<PathBuf> = [
|
||||
dunce::canonicalize(&git_dir).unwrap(),
|
||||
dunce::canonicalize(&command_cwd).unwrap().join(".codex"),
|
||||
]
|
||||
.into_iter()
|
||||
.collect();
|
||||
|
||||
assert_eq!(expected_allow, paths.allow);
|
||||
assert_eq!(expected_deny, paths.deny);
|
||||
@@ -203,9 +219,12 @@ mod tests {
|
||||
let expected_allow: HashSet<PathBuf> = [dunce::canonicalize(&command_cwd).unwrap()]
|
||||
.into_iter()
|
||||
.collect();
|
||||
let expected_deny: HashSet<PathBuf> = [dunce::canonicalize(&git_file).unwrap()]
|
||||
.into_iter()
|
||||
.collect();
|
||||
let expected_deny: HashSet<PathBuf> = [
|
||||
dunce::canonicalize(&git_file).unwrap(),
|
||||
dunce::canonicalize(&command_cwd).unwrap().join(".codex"),
|
||||
]
|
||||
.into_iter()
|
||||
.collect();
|
||||
|
||||
assert_eq!(expected_allow, paths.allow);
|
||||
assert_eq!(expected_deny, paths.deny);
|
||||
@@ -244,7 +263,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skips_protected_subdirs_when_missing() {
|
||||
fn reserves_missing_codex_dir_but_not_other_missing_protected_subdirs() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let command_cwd = tmp.path().join("workspace");
|
||||
let _ = fs::create_dir_all(&command_cwd);
|
||||
@@ -259,6 +278,12 @@ 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 protected dirs are absent");
|
||||
let expected_deny: HashSet<PathBuf> = [dunce::canonicalize(&command_cwd)
|
||||
.unwrap()
|
||||
.join(".codex")]
|
||||
.into_iter()
|
||||
.collect();
|
||||
|
||||
assert_eq!(expected_deny, paths.deny);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -215,6 +215,11 @@ mod windows_impl {
|
||||
|
||||
type PipeHandles = ((HANDLE, HANDLE), (HANDLE, HANDLE), (HANDLE, HANDLE));
|
||||
|
||||
fn is_reserved_codex_dir(path: &Path) -> bool {
|
||||
path.file_name()
|
||||
.is_some_and(|file_name| file_name == ".codex")
|
||||
}
|
||||
|
||||
fn should_apply_network_block(policy: &SandboxPolicy) -> bool {
|
||||
!policy.has_full_network_access()
|
||||
}
|
||||
@@ -391,9 +396,24 @@ mod windows_impl {
|
||||
}
|
||||
}
|
||||
for p in &deny {
|
||||
if let Ok(added) = add_deny_write_ace(p, psid_generic) {
|
||||
let deny_sid = if is_workspace_write
|
||||
&& p.parent()
|
||||
.is_some_and(|parent| is_command_cwd_root(parent, &canonical_cwd))
|
||||
{
|
||||
psid_workspace.unwrap_or(psid_generic)
|
||||
} else {
|
||||
psid_generic
|
||||
};
|
||||
let deny_result = if is_reserved_codex_dir(p) {
|
||||
p.parent().map_or(Ok(false), |root| {
|
||||
protect_workspace_codex_dir(root, deny_sid)
|
||||
})
|
||||
} else {
|
||||
add_deny_write_ace(p, deny_sid)
|
||||
};
|
||||
if let Ok(added) = deny_result {
|
||||
if added && !persist_aces {
|
||||
guards.push((p.clone(), psid_generic));
|
||||
guards.push((p.clone(), deny_sid));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -578,7 +598,21 @@ mod windows_impl {
|
||||
let _ = add_allow_ace(p, psid);
|
||||
}
|
||||
for p in &deny {
|
||||
let _ = add_deny_write_ace(p, psid_generic);
|
||||
let deny_sid = if p
|
||||
.parent()
|
||||
.is_some_and(|parent| is_command_cwd_root(parent, &canonical_cwd))
|
||||
{
|
||||
psid_workspace
|
||||
} else {
|
||||
psid_generic
|
||||
};
|
||||
if is_reserved_codex_dir(p) {
|
||||
if let Some(root) = p.parent() {
|
||||
let _ = protect_workspace_codex_dir(root, deny_sid);
|
||||
}
|
||||
} else {
|
||||
let _ = add_deny_write_ace(p, deny_sid);
|
||||
}
|
||||
}
|
||||
allow_null_device(psid_generic);
|
||||
allow_null_device(psid_workspace);
|
||||
|
||||
@@ -812,30 +812,38 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
|
||||
}
|
||||
}
|
||||
|
||||
// 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");
|
||||
log_line(
|
||||
log,
|
||||
&format!(
|
||||
"applied deny ACE to protect workspace .codex {}",
|
||||
cwd_codex.display()
|
||||
),
|
||||
)?;
|
||||
}
|
||||
Ok(false) => {}
|
||||
Err(err) => {
|
||||
let cwd_codex = payload.command_cwd.join(".codex");
|
||||
refresh_errors.push(format!("deny ACE failed on {}: {err}", cwd_codex.display()));
|
||||
log_line(
|
||||
log,
|
||||
&format!("deny ACE failed on {}: {err}", cwd_codex.display()),
|
||||
)?;
|
||||
// Protect top-level `.codex` inside every writable root. Windows deny ACEs require an
|
||||
// existing path, so reserve the directory when it has not been created yet.
|
||||
for root in &payload.write_roots {
|
||||
let deny_sid = if is_command_cwd_root(root, &canonical_command_cwd) {
|
||||
workspace_psid
|
||||
} else {
|
||||
cap_psid
|
||||
};
|
||||
match unsafe { protect_workspace_codex_dir(root, deny_sid) } {
|
||||
Ok(true) => {
|
||||
let codex_dir = root.join(".codex");
|
||||
log_line(
|
||||
log,
|
||||
&format!(
|
||||
"applied deny ACE to protect writable-root .codex {}",
|
||||
codex_dir.display()
|
||||
),
|
||||
)?;
|
||||
}
|
||||
Ok(false) => {}
|
||||
Err(err) => {
|
||||
let codex_dir = root.join(".codex");
|
||||
refresh_errors.push(format!("deny ACE failed on {}: {err}", codex_dir.display()));
|
||||
log_line(
|
||||
log,
|
||||
&format!("deny ACE failed on {}: {err}", codex_dir.display()),
|
||||
)?;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Protect the current workspace's `.agents` directory from tampering when it already exists.
|
||||
match unsafe { protect_workspace_agents_dir(&payload.command_cwd, workspace_psid) } {
|
||||
Ok(true) => {
|
||||
let cwd_agents = payload.command_cwd.join(".agents");
|
||||
|
||||
@@ -3,6 +3,7 @@ use crate::path_normalization::canonicalize_path;
|
||||
use anyhow::Result;
|
||||
use std::ffi::c_void;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
pub fn is_command_cwd_root(root: &Path, canonical_command_cwd: &Path) -> bool {
|
||||
canonicalize_path(root) == canonical_command_cwd
|
||||
@@ -11,20 +12,97 @@ 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> {
|
||||
protect_workspace_subdir(cwd, psid, ".codex")
|
||||
protect_root_subdir(cwd, psid, ".codex", MissingPathPolicy::CreateDir)
|
||||
}
|
||||
|
||||
/// # 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")
|
||||
protect_root_subdir(cwd, psid, ".agents", MissingPathPolicy::ExistingOnly)
|
||||
}
|
||||
|
||||
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)
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
enum MissingPathPolicy {
|
||||
ExistingOnly,
|
||||
CreateDir,
|
||||
}
|
||||
|
||||
fn prepare_root_subdir(
|
||||
root: &Path,
|
||||
subdir: &str,
|
||||
missing_path_policy: MissingPathPolicy,
|
||||
) -> Result<Option<PathBuf>> {
|
||||
let path = root.join(subdir);
|
||||
if path.exists() {
|
||||
return Ok(Some(path));
|
||||
}
|
||||
|
||||
match missing_path_policy {
|
||||
MissingPathPolicy::ExistingOnly => Ok(None),
|
||||
MissingPathPolicy::CreateDir => {
|
||||
// Windows deny ACEs require an existing path, so reserve `.codex` eagerly.
|
||||
std::fs::create_dir_all(&path)?;
|
||||
Ok(Some(path))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
unsafe fn protect_root_subdir(
|
||||
root: &Path,
|
||||
psid: *mut c_void,
|
||||
subdir: &str,
|
||||
missing_path_policy: MissingPathPolicy,
|
||||
) -> Result<bool> {
|
||||
let Some(path) = prepare_root_subdir(root, subdir, missing_path_policy)? else {
|
||||
return Ok(false);
|
||||
};
|
||||
add_deny_write_ace(&path, psid)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::MissingPathPolicy;
|
||||
use super::prepare_root_subdir;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[test]
|
||||
fn reserves_missing_codex_dir_for_protection() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let root = tmp.path().join("workspace");
|
||||
std::fs::create_dir_all(&root).expect("workspace root");
|
||||
|
||||
let prepared = prepare_root_subdir(&root, ".codex", MissingPathPolicy::CreateDir)
|
||||
.expect("prepare path");
|
||||
let expected = Some(root.join(".codex"));
|
||||
|
||||
assert_eq!(expected, prepared);
|
||||
assert!(root.join(".codex").is_dir());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skips_missing_agents_dir_when_not_reserved() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let root = tmp.path().join("workspace");
|
||||
std::fs::create_dir_all(&root).expect("workspace root");
|
||||
|
||||
let prepared = prepare_root_subdir(&root, ".agents", MissingPathPolicy::ExistingOnly)
|
||||
.expect("prepare path");
|
||||
|
||||
assert_eq!(None, prepared);
|
||||
assert!(!root.join(".agents").exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_existing_protected_path_without_recreating_it() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let root = tmp.path().join("workspace");
|
||||
let codex_dir = root.join(".codex");
|
||||
std::fs::create_dir_all(&codex_dir).expect("codex dir");
|
||||
|
||||
let prepared = prepare_root_subdir(&root, ".codex", MissingPathPolicy::CreateDir)
|
||||
.expect("prepare path");
|
||||
|
||||
assert_eq!(Some(codex_dir), prepared);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user