openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-22 17:58:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add Windows missing metadata deny sentinel

Browse Source
This commit is contained in:
Eva Wong
2026-05-06 15:33:40 -07:00
parent 0dddc354d8
commit 4c24c5ec58
7 changed files with 204 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/windows-sandbox-rs/src/elevated_impl.rs
+3 -2
View File
@@ -82,8 +82,9 @@ mod windows_impl {
let logs_base_dir: Option<&Path> = Some(sandbox_base.as_path());
log_start(&command, logs_base_dir);
let protected_metadata_guard =
prepare_protected_metadata_targets(protected_metadata_targets);
let mut protected_metadata_guard =
prepare_protected_metadata_targets(protected_metadata_targets)?;
protected_metadata_guard.arm_sentinel_cleanup()?;
let sandbox_creds = require_logon_sandbox_creds(
&policy,
sandbox_policy_cwd,
codex-rs/windows-sandbox-rs/src/lib.rs
+5 -2
View File
@@ -172,6 +172,8 @@ pub use process::read_handle_loop;
#[cfg(target_os = "windows")]
pub use process::spawn_process_with_pipes;
#[cfg(target_os = "windows")]
pub use protected_metadata::ensure_missing_deny_sentinel;
#[cfg(target_os = "windows")]
pub use protected_metadata::protected_metadata_existing_deny_paths;
#[cfg(target_os = "windows")]
pub use session::spawn_windows_sandbox_session_elevated;
@@ -451,8 +453,8 @@ mod windows_impl {
compute_allow_paths(&policy, sandbox_policy_cwd, &current_dir, &env_map);
let read_roots = legacy_session_executable_read_roots(&env_map, &command);
let direct_read_paths = legacy_session_direct_read_paths(&env_map);
let protected_metadata_guard =
prepare_protected_metadata_targets(protected_metadata_targets);
let mut protected_metadata_guard =
prepare_protected_metadata_targets(protected_metadata_targets)?;
for path in protected_metadata_guard.deny_paths() {
deny.insert(path.clone());
}
@@ -461,6 +463,7 @@ mod windows_impl {
deny.insert(path.clone());
}
}
protected_metadata_guard.arm_sentinel_cleanup()?;
let canonical_cwd = canonicalize_path(&current_dir);
let mut guards: Vec<(PathBuf, *mut c_void)> = Vec::new();
let read_execute_mask = FILE_GENERIC_READ | FILE_GENERIC_EXECUTE;
codex-rs/windows-sandbox-rs/src/protected_metadata.rs
+168 -15
View File
@@ -23,24 +23,35 @@ use windows_sys::Win32::Foundation::TRUE;
use windows_sys::Win32::Foundation::WAIT_FAILED;
use windows_sys::Win32::Foundation::WAIT_OBJECT_0;
use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT;
use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS;
use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_DELETE_ON_CLOSE;
use windows_sys::Win32::Storage::FileSystem::FILE_NOTIFY_CHANGE_CREATION;
use windows_sys::Win32::Storage::FileSystem::FILE_NOTIFY_CHANGE_DIR_NAME;
use windows_sys::Win32::Storage::FileSystem::FILE_NOTIFY_CHANGE_FILE_NAME;
use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE;
use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ;
use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE;
use windows_sys::Win32::Storage::FileSystem::CreateFileW;
use windows_sys::Win32::Storage::FileSystem::DELETE;
use windows_sys::Win32::Storage::FileSystem::FindCloseChangeNotification;
use windows_sys::Win32::Storage::FileSystem::FindFirstChangeNotificationW;
use windows_sys::Win32::Storage::FileSystem::FindNextChangeNotification;
use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING;
use windows_sys::Win32::System::Threading::CreateEventW;
use windows_sys::Win32::System::Threading::INFINITE;
use windows_sys::Win32::System::Threading::SetEvent;
use windows_sys::Win32::System::Threading::WaitForMultipleObjects;
/// Layer: Windows enforcement layer. Existing metadata objects can be protected
/// with ACLs; missing names are monitored and removed if the sandbox creates
/// them.
/// with ACLs. Missing names are materialized as empty deny sentinels when the
/// caller needs pre-command creation denial, or monitored and removed after
/// creation when the caller explicitly requests reactive cleanup.
#[derive(Debug)]
pub(crate) struct ProtectedMetadataGuard {
deny_paths: Vec<PathBuf>,
monitored_paths: Vec<PathBuf>,
sentinel_paths: Vec<PathBuf>,
sentinel_handles: Vec<SentinelHandle>,
}
impl ProtectedMetadataGuard {
@@ -48,6 +59,14 @@ impl ProtectedMetadataGuard {
self.deny_paths.iter()
}
pub(crate) fn arm_sentinel_cleanup(&mut self) -> Result<()> {
for path in &self.sentinel_paths {
self.sentinel_handles
.push(open_delete_on_close_directory(path)?);
}
Ok(())
}
pub(crate) fn into_runtime(self) -> Result<ProtectedMetadataRuntime> {
let monitor = MissingCreationMonitor::start(&self.monitored_paths)?;
Ok(ProtectedMetadataRuntime {
@@ -56,9 +75,10 @@ impl ProtectedMetadataGuard {
})
}
pub(crate) fn cleanup_created_monitored_paths(&self) -> Result<Vec<PathBuf>> {
pub(crate) fn cleanup_created_paths(&mut self) -> Result<Vec<PathBuf>> {
self.sentinel_handles.clear();
let mut removed = Vec::new();
for path in &self.monitored_paths {
for path in self.monitored_paths.iter().chain(self.sentinel_paths.iter()) {
let Some(existing_path) = existing_metadata_path(path)? else {
continue;
};
@@ -70,6 +90,15 @@ impl ProtectedMetadataGuard {
}
}
impl Drop for ProtectedMetadataGuard {
fn drop(&mut self) {
self.sentinel_handles.clear();
for path in &self.sentinel_paths {
let _ = remove_metadata_path(path);
}
}
}
/// Layer: Windows enforcement runtime. Owns the prepared guard plus any active
/// OS change listeners for the lifetime of one sandboxed command, then reports
/// whether protected metadata was created during that command.
@@ -80,9 +109,35 @@ pub(crate) struct ProtectedMetadataRuntime {
impl ProtectedMetadataRuntime {
pub(crate) fn finish(mut self) -> Result<Vec<PathBuf>> {
let mut removed = self.monitor.finish()?;
removed.extend(self.guard.cleanup_created_monitored_paths()?);
Ok(unique_paths(removed))
let monitor_result = self.monitor.finish();
let cleanup_result = self.guard.cleanup_created_paths();
match (monitor_result, cleanup_result) {
(Ok(mut removed), Ok(cleaned)) => {
removed.extend(cleaned);
Ok(unique_paths(removed))
}
(Err(err), Ok(_)) | (Ok(_), Err(err)) => Err(err),
(Err(monitor_err), Err(cleanup_err)) => Err(anyhow!(
"protected metadata monitor failed: {monitor_err:#}; cleanup also failed: {cleanup_err:#}"
)),
}
}
}
/// Layer: Windows sentinel cleanup handle. Holds a delete-on-close directory
/// handle for one Codex-created sentinel so forced parent-process termination
/// does not leave a protected metadata artifact behind.
#[derive(Debug)]
struct SentinelHandle(HANDLE);
impl Drop for SentinelHandle {
fn drop(&mut self) {
if self.0 != 0 && self.0 != INVALID_HANDLE_VALUE {
unsafe {
CloseHandle(self.0);
}
self.0 = 0;
}
}
}
@@ -342,9 +397,10 @@ fn record_monitor_error(errors: &Arc<Mutex<Vec<String>>>, message: String) {
pub(crate) fn prepare_protected_metadata_targets(
targets: &[ProtectedMetadataTarget],
) -> ProtectedMetadataGuard {
) -> Result<ProtectedMetadataGuard> {
let mut deny_paths = Vec::new();
let mut monitored_paths = Vec::new();
let mut sentinel_paths = Vec::new();
for target in targets {
match target.mode {
ProtectedMetadataMode::ExistingDeny => {
@@ -353,12 +409,26 @@ pub(crate) fn prepare_protected_metadata_targets(
ProtectedMetadataMode::MissingCreationMonitor => {
monitored_paths.push(target.path.clone());
}
ProtectedMetadataMode::MissingDenySentinel => {
let created = ensure_missing_deny_sentinel(&target.path)?;
let existing_deny_paths = protected_metadata_existing_deny_paths(&target.path);
if existing_deny_paths.is_empty() {
deny_paths.push(target.path.clone());
} else {
deny_paths.extend(existing_deny_paths);
}
if created {
sentinel_paths.push(target.path.clone());
}
}
}
}
ProtectedMetadataGuard {
Ok(ProtectedMetadataGuard {
deny_paths,
monitored_paths,
}
sentinel_paths,
sentinel_handles: Vec::new(),
})
}
pub fn protected_metadata_existing_deny_paths(path: &Path) -> Vec<PathBuf> {
@@ -478,6 +548,47 @@ fn remove_metadata_path(path: &Path) -> Result<()> {
Ok(())
}
/// Creates an empty sentinel directory for a missing protected metadata name.
///
/// Returns true when this call created the sentinel. If the target already
/// exists by the time enforcement prepares, callers should still deny it, but
/// must not claim it for cleanup as a Codex-created sentinel.
pub fn ensure_missing_deny_sentinel(path: &Path) -> Result<bool> {
if existing_metadata_path(path)?.is_some() {
return Ok(false);
}
match std::fs::create_dir(path) {
Ok(()) => Ok(true),
Err(err) if err.kind() == io::ErrorKind::AlreadyExists => Ok(false),
Err(err) => Err(err)
.with_context(|| format!("failed to create protected metadata sentinel {}", path.display())),
}
}
fn open_delete_on_close_directory(path: &Path) -> Result<SentinelHandle> {
let path_wide = to_wide(path);
let handle = unsafe {
CreateFileW(
path_wide.as_ptr(),
DELETE,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
std::ptr::null_mut(),
OPEN_EXISTING,
FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_DELETE_ON_CLOSE,
0,
)
};
if handle == INVALID_HANDLE_VALUE {
return Err(anyhow!(
"failed to arm protected metadata sentinel cleanup for {}: {}",
path.display(),
io::Error::last_os_error()
));
}
Ok(SentinelHandle(handle))
}
fn is_directory_reparse_point(metadata: &Metadata) -> bool {
metadata.is_dir() && (metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT) != 0
}
@@ -491,17 +602,18 @@ mod tests {
use std::time::Instant;
#[test]
fn cleanup_created_monitored_paths_removes_case_variant() {
fn cleanup_created_paths_removes_case_variant() {
let temp_dir = tempfile::TempDir::new().expect("tempdir");
let target = temp_dir.path().join(".git");
let created = temp_dir.path().join(".GIT");
std::fs::create_dir_all(&created).expect("create metadata");
let guard = prepare_protected_metadata_targets(&[ProtectedMetadataTarget {
let mut guard = prepare_protected_metadata_targets(&[ProtectedMetadataTarget {
path: target.clone(),
mode: ProtectedMetadataMode::MissingCreationMonitor,
}]);
}])
.expect("guard");
let removed = guard.cleanup_created_monitored_paths().expect("cleanup");
let removed = guard.cleanup_created_paths().expect("cleanup");
assert_eq!(removed.len(), 1);
assert!(
removed[0]
@@ -523,6 +635,7 @@ mod tests {
path: target,
mode: ProtectedMetadataMode::MissingCreationMonitor,
}])
.expect("guard")
.into_runtime()
.expect("runtime");
@@ -559,7 +672,8 @@ mod tests {
let guard = prepare_protected_metadata_targets(&[ProtectedMetadataTarget {
path: symlink_dir.clone(),
mode: ProtectedMetadataMode::ExistingDeny,
}]);
}])
.expect("guard");
let deny_paths: Vec<PathBuf> = guard.deny_paths().cloned().collect();
let canonical_target = dunce::canonicalize(&target_dir).expect("canonical target");
@@ -576,4 +690,43 @@ mod tests {
"deny paths should include symlink target: {deny_paths:?}"
);
}
#[test]
fn missing_deny_sentinel_creates_and_cleans_path() {
let temp_dir = tempfile::TempDir::new().expect("tempdir");
let target = temp_dir.path().join(".git");
let mut guard = prepare_protected_metadata_targets(&[ProtectedMetadataTarget {
path: target.clone(),
mode: ProtectedMetadataMode::MissingDenySentinel,
}])
.expect("guard");
assert!(target.is_dir(), "sentinel directory should be created");
assert!(
guard.deny_paths().any(|path| path_text_key(path) == path_text_key(&target)),
"sentinel should be deny-listed"
);
let removed = guard.cleanup_created_paths().expect("cleanup");
assert_eq!(removed, vec![target.clone()]);
assert!(!target.exists(), "sentinel directory should be removed");
}
#[test]
fn missing_deny_sentinel_does_not_cleanup_preexisting_path() {
let temp_dir = tempfile::TempDir::new().expect("tempdir");
let target = temp_dir.path().join(".git");
std::fs::create_dir_all(&target).expect("create metadata");
let mut guard = prepare_protected_metadata_targets(&[ProtectedMetadataTarget {
path: target.clone(),
mode: ProtectedMetadataMode::MissingDenySentinel,
}])
.expect("guard");
let removed = guard.cleanup_created_paths().expect("cleanup");
assert!(removed.is_empty(), "pre-existing metadata is not Codex-owned cleanup");
assert!(target.exists(), "pre-existing metadata should not be removed");
}
}
codex-rs/windows-sandbox-rs/src/setup_main_win.rs
+11 -4
View File
@@ -19,6 +19,7 @@ use codex_windows_sandbox::canonicalize_path;
use codex_windows_sandbox::convert_string_sid_to_sid;
use codex_windows_sandbox::ensure_allow_mask_aces_with_inheritance;
use codex_windows_sandbox::ensure_allow_write_aces;
use codex_windows_sandbox::ensure_missing_deny_sentinel;
use codex_windows_sandbox::extract_setup_failure;
use codex_windows_sandbox::hide_newly_created_users;
use codex_windows_sandbox::install_wfp_filters;
@@ -824,10 +825,16 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
}
for target in &payload.protected_metadata_targets {
if !matches!(target.mode, ProtectedMetadataMode::ExistingDeny) {
continue;
}
let deny_paths = protected_metadata_existing_deny_paths(&target.path);
let deny_paths = match target.mode {
ProtectedMetadataMode::ExistingDeny => {
protected_metadata_existing_deny_paths(&target.path)
}
ProtectedMetadataMode::MissingCreationMonitor => continue,
ProtectedMetadataMode::MissingDenySentinel => {
ensure_missing_deny_sentinel(&target.path)?;
protected_metadata_existing_deny_paths(&target.path)
}
};
if deny_paths.is_empty() {
log_line(
log,
codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs
+10 -2
View File
@@ -35,7 +35,7 @@ use windows_sys::Win32::Security::CheckTokenMembership;
use windows_sys::Win32::Security::FreeSid;
use windows_sys::Win32::Security::SECURITY_NT_AUTHORITY;
pub const SETUP_VERSION: u32 = 5;
pub const SETUP_VERSION: u32 = 6;
pub const OFFLINE_USERNAME: &str = "CodexSandboxOffline";
pub const ONLINE_USERNAME: &str = "CodexSandboxOnline";
const ERROR_CANCELLED: u32 = 1223;
@@ -112,12 +112,20 @@ pub struct ProtectedMetadataTarget {
}
/// Layer: Windows enforcement request boundary. The helper must distinguish
/// existing metadata objects from missing names that need create monitoring.
/// existing metadata objects from missing names that need pre-command denial or
/// reactive cleanup.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum ProtectedMetadataMode {
/// Protect an existing metadata object and any canonical reparse target by
/// applying deny-write ACLs before the sandboxed command starts.
ExistingDeny,
/// Watch for a missing metadata object during the command and remove it if
/// a caller intentionally requests reactive cleanup behavior.
MissingCreationMonitor,
/// Create a temporary deny-listed sentinel before the command starts so the
/// sandbox cannot create the metadata object during execution.
MissingDenySentinel,
}
pub fn run_setup_refresh(
codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs
+4 -1
View File
@@ -34,6 +34,10 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated(
protected_metadata_targets: &[ProtectedMetadataTarget],
use_private_desktop: bool,
) -> Result<SpawnedProcess> {
let mut protected_metadata_guard =
prepare_protected_metadata_targets(protected_metadata_targets)?;
protected_metadata_guard.arm_sentinel_cleanup()?;
let elevated = prepare_elevated_spawn_context(
policy_json_or_preset,
sandbox_policy_cwd,
@@ -44,7 +48,6 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated(
protected_metadata_targets,
)?;
let protected_metadata_guard = prepare_protected_metadata_targets(protected_metadata_targets);
let protected_metadata_runtime = protected_metadata_guard.into_runtime()?;
let spawn_request = SpawnRequest {
command: command.clone(),
codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs
+3 -1
View File
@@ -328,9 +328,11 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy(
allow_null_device_for_workspace_write(common.is_workspace_write);
let persist_aces = common.is_workspace_write;
let protected_metadata_guard = prepare_protected_metadata_targets(protected_metadata_targets);
let mut protected_metadata_guard =
prepare_protected_metadata_targets(protected_metadata_targets)?;
let additional_deny_write_paths: Vec<PathBuf> =
protected_metadata_guard.deny_paths().cloned().collect();
protected_metadata_guard.arm_sentinel_cleanup()?;
let guards = apply_legacy_session_acl_rules(
&common.policy,
sandbox_policy_cwd,