diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index 48af4667dd..fc11c561a7 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -39,6 +39,7 @@ mod windows_impl { use crate::logging::log_success; use crate::policy::SandboxPolicy; use crate::policy::parse_policy; + use crate::protected_metadata::prepare_protected_metadata_targets; use crate::runner_client::spawn_runner_transport; use crate::sandbox_utils::ensure_codex_home_exists; use crate::sandbox_utils::inject_git_safe_directory; @@ -80,6 +81,8 @@ 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 sandbox_creds = require_logon_sandbox_creds( &policy, sandbox_policy_cwd, @@ -154,7 +157,7 @@ mod windows_impl { let mut stdout = Vec::new(); let mut stderr = Vec::new(); - let (exit_code, timed_out) = loop { + let (mut exit_code, timed_out) = loop { let msg = read_frame(&mut pipe_read)? .ok_or_else(|| anyhow::anyhow!("runner pipe closed before exit"))?; match msg.message { @@ -178,6 +181,12 @@ mod windows_impl { } }; + let protected_metadata_violations = + protected_metadata_guard.cleanup_created_monitored_paths()?; + if !protected_metadata_violations.is_empty() && exit_code == 0 { + exit_code = 1; + } + if exit_code == 0 { log_success(&command, logs_base_dir); } else { diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 687e4abf64..8d780f652e 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -170,6 +170,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::protected_metadata_existing_deny_paths; +#[cfg(target_os = "windows")] pub use session::spawn_windows_sandbox_session_elevated; #[cfg(target_os = "windows")] pub use session::spawn_windows_sandbox_session_legacy; @@ -272,6 +274,7 @@ mod windows_impl { use super::path_normalization::canonicalize_path; use super::policy::SandboxPolicy; use super::process::create_process_as_user; + use super::protected_metadata::prepare_protected_metadata_targets; use super::sandbox_utils::ensure_codex_home_exists; use super::spawn_prep::prepare_legacy_spawn_context; use super::token::convert_string_sid_to_sid; @@ -366,7 +369,7 @@ mod windows_impl { mut env_map: HashMap, timeout_ms: Option, additional_deny_write_paths: &[PathBuf], - _protected_metadata_targets: &[ProtectedMetadataTarget], + protected_metadata_targets: &[ProtectedMetadataTarget], use_private_desktop: bool, ) -> Result { let common = prepare_legacy_spawn_context( @@ -436,6 +439,11 @@ mod windows_impl { let persist_aces = is_workspace_write; let AllowDenyPaths { allow, mut deny } = compute_allow_paths(&policy, sandbox_policy_cwd, ¤t_dir, &env_map); + let protected_metadata_guard = + prepare_protected_metadata_targets(protected_metadata_targets); + for path in protected_metadata_guard.deny_paths() { + deny.insert(path.clone()); + } for path in additional_deny_write_paths { if path.exists() { deny.insert(path.clone()); @@ -586,11 +594,16 @@ mod windows_impl { let _ = t_err.join(); let stdout = rx_out.recv().unwrap_or_default(); let stderr = rx_err.recv().unwrap_or_default(); - let exit_code = if timed_out { + let mut exit_code = if timed_out { 128 + 64 } else { exit_code_u32 as i32 }; + let protected_metadata_violations = + protected_metadata_guard.cleanup_created_monitored_paths()?; + if !protected_metadata_violations.is_empty() && exit_code == 0 { + exit_code = 1; + } if exit_code == 0 { log_success(&command, logs_base_dir); diff --git a/codex-rs/windows-sandbox-rs/src/protected_metadata.rs b/codex-rs/windows-sandbox-rs/src/protected_metadata.rs index 40e3d2512d..3a066590ab 100644 --- a/codex-rs/windows-sandbox-rs/src/protected_metadata.rs +++ b/codex-rs/windows-sandbox-rs/src/protected_metadata.rs @@ -1,5 +1,3 @@ -#![allow(dead_code)] - use crate::setup::ProtectedMetadataMode; use crate::setup::ProtectedMetadataTarget; use anyhow::Context; diff --git a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs index 6e0f19ac4a..ad571ba169 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs @@ -8,6 +8,7 @@ use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64; use codex_otel::StatsigMetricsSettings; use codex_windows_sandbox::LOG_FILE_NAME; +use codex_windows_sandbox::ProtectedMetadataMode; use codex_windows_sandbox::ProtectedMetadataTarget; use codex_windows_sandbox::SETUP_VERSION; use codex_windows_sandbox::SetupErrorCode; @@ -25,6 +26,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::protected_metadata_existing_deny_paths; use codex_windows_sandbox::sandbox_bin_dir; use codex_windows_sandbox::sandbox_dir; use codex_windows_sandbox::sandbox_secrets_dir; @@ -88,7 +90,6 @@ struct Payload { write_roots: Vec, #[serde(default)] deny_write_paths: Vec, - #[allow(dead_code)] #[serde(default)] protected_metadata_targets: Vec, proxy_ports: Vec, @@ -820,6 +821,66 @@ 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); + if deny_paths.is_empty() { + log_line( + log, + &format!( + "protected metadata {} missing during setup; skipping", + target.path.display() + ), + )?; + continue; + } + + for path in deny_paths { + if !seen_deny_paths.insert(path.clone()) { + continue; + } + if std::fs::symlink_metadata(&path).is_err() { + log_line( + log, + &format!( + "protected metadata {} missing during setup; skipping", + path.display() + ), + )?; + continue; + } + + let canonical_path = canonicalize_path(&path); + let deny_psid = if canonical_path.starts_with(&canonical_command_cwd) { + workspace_psid + } else { + cap_psid + }; + + match unsafe { add_deny_write_ace(&path, deny_psid) } { + Ok(true) => { + log_line( + log, + &format!("applied deny ACE to protect metadata {}", path.display()), + )?; + } + Ok(false) => {} + Err(err) => { + refresh_errors.push(format!( + "metadata deny ACE failed on {}: {err}", + path.display() + )); + log_line( + log, + &format!("metadata deny ACE failed on {}: {err}", path.display()), + )?; + } + } + } + } + lock_sandbox_dir( &sandbox_bin_dir(&payload.codex_home), &payload.real_user, diff --git a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs index 022c219922..bb753362a4 100644 --- a/codex-rs/windows-sandbox-rs/src/spawn_prep.rs +++ b/codex-rs/windows-sandbox-rs/src/spawn_prep.rs @@ -212,6 +212,7 @@ pub(crate) fn allow_null_device_for_workspace_write(is_workspace_write: bool) { } } +#[allow(clippy::too_many_arguments)] pub(crate) fn apply_legacy_session_acl_rules( policy: &SandboxPolicy, sandbox_policy_cwd: &Path, @@ -220,9 +221,11 @@ pub(crate) fn apply_legacy_session_acl_rules( psid_generic: &LocalSid, psid_workspace: Option<&LocalSid>, persist_aces: bool, + additional_deny_paths: &[PathBuf], ) -> Vec { - let AllowDenyPaths { allow, deny } = + let AllowDenyPaths { allow, mut deny } = compute_allow_paths(policy, sandbox_policy_cwd, current_dir, env_map); + deny.extend(additional_deny_paths.iter().cloned()); let mut guards: Vec = Vec::new(); let canonical_cwd = canonicalize_path(current_dir); unsafe { diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs index 3ef4828d1b..1e30b76b7b 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/elevated.rs @@ -7,6 +7,7 @@ use crate::ipc_framed::EmptyPayload; use crate::ipc_framed::FramedMessage; use crate::ipc_framed::Message; use crate::ipc_framed::SpawnRequest; +use crate::protected_metadata::prepare_protected_metadata_targets; use crate::runner_client::spawn_runner_transport; use crate::setup::ProtectedMetadataTarget; use crate::spawn_prep::prepare_elevated_spawn_context; @@ -43,6 +44,7 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated( protected_metadata_targets, )?; + let protected_metadata_guard = prepare_protected_metadata_targets(protected_metadata_targets); let spawn_request = SpawnRequest { command: command.clone(), cwd: cwd.to_path_buf(), @@ -102,6 +104,7 @@ pub(crate) async fn spawn_windows_sandbox_session_elevated( stdout_tx, stderr_rx.as_ref().map(|(tx, _rx)| tx.clone()), exit_tx, + Some(protected_metadata_guard), ); Ok(finish_driver_spawn( diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs index 911c218440..f9e2ba4e68 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/legacy.rs @@ -10,6 +10,8 @@ use crate::process::StderrMode; use crate::process::StdinMode; use crate::process::read_handle_loop; use crate::process::spawn_process_with_pipes; +use crate::protected_metadata::ProtectedMetadataGuard; +use crate::protected_metadata::prepare_protected_metadata_targets; use crate::setup::ProtectedMetadataTarget; use crate::spawn_prep::LocalSid; use crate::spawn_prep::allow_null_device_for_workspace_write; @@ -206,10 +208,11 @@ fn finalize_exit( output_join: std::thread::JoinHandle<()>, guards: Vec, cap_sid: Option, + protected_metadata_guard: ProtectedMetadataGuard, logs_base_dir: Option<&Path>, command: Vec, ) { - let exit_code = { + let mut exit_code = { let mut raw_exit = 1u32; if let Ok(guard) = process_handle.lock() && let Some(handle) = guard.as_ref() @@ -223,6 +226,21 @@ fn finalize_exit( }; let _ = output_join.join(); + let protected_metadata_failure = + match protected_metadata_guard.cleanup_created_monitored_paths() { + Ok(paths) => { + if !paths.is_empty() && exit_code == 0 { + exit_code = 1; + } + None + } + Err(err) => { + if exit_code == 0 { + exit_code = 1; + } + Some(format!("protected metadata cleanup failed: {err:#}")) + } + }; let _ = exit_tx.send(exit_code); unsafe { @@ -236,7 +254,9 @@ fn finalize_exit( } } - if exit_code == 0 { + if let Some(message) = protected_metadata_failure { + log_failure(&command, &message, logs_base_dir); + } else if exit_code == 0 { log_success(&command, logs_base_dir); } else { log_failure(&command, &format!("exit code {exit_code}"), logs_base_dir); @@ -290,7 +310,7 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy( timeout_ms: Option, tty: bool, stdin_open: bool, - _protected_metadata_targets: &[ProtectedMetadataTarget], + protected_metadata_targets: &[ProtectedMetadataTarget], use_private_desktop: bool, ) -> Result { let common = prepare_legacy_spawn_context( @@ -309,6 +329,9 @@ 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 additional_deny_write_paths: Vec = + protected_metadata_guard.deny_paths().cloned().collect(); let guards = apply_legacy_session_acl_rules( &common.policy, sandbox_policy_cwd, @@ -317,6 +340,7 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy( &security.psid_generic, security.psid_workspace.as_ref(), persist_aces, + &additional_deny_write_paths, ); let (writer_tx, writer_rx) = mpsc::channel::>(128); @@ -408,6 +432,7 @@ pub(crate) async fn spawn_windows_sandbox_session_legacy( output_join, guards_for_wait, cap_sid_for_wait, + protected_metadata_guard, common.logs_base_dir.as_deref(), command_for_wait, ); diff --git a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/windows_common.rs b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/windows_common.rs index 3396b4ed98..f46a01ec8a 100644 --- a/codex-rs/windows-sandbox-rs/src/unified_exec/backends/windows_common.rs +++ b/codex-rs/windows-sandbox-rs/src/unified_exec/backends/windows_common.rs @@ -6,6 +6,7 @@ use crate::ipc_framed::ResizePayload; use crate::ipc_framed::StdinPayload; use crate::ipc_framed::decode_bytes; use crate::ipc_framed::encode_bytes; +use crate::protected_metadata::ProtectedMetadataGuard; use anyhow::Result; use codex_utils_pty::ProcessDriver; use codex_utils_pty::SpawnedProcess; @@ -97,6 +98,7 @@ pub(crate) fn start_runner_stdout_reader( stdout_tx: broadcast::Sender>, stderr_tx: Option>>, exit_tx: oneshot::Sender, + protected_metadata_guard: Option, ) { std::thread::spawn(move || { loop { @@ -140,7 +142,27 @@ pub(crate) fn start_runner_stdout_reader( } } Message::Exit { payload } => { - let _ = exit_tx.send(payload.exit_code); + let mut exit_code = payload.exit_code; + if let Some(protected_metadata_guard) = protected_metadata_guard { + match protected_metadata_guard.cleanup_created_monitored_paths() { + Ok(paths) => { + if !paths.is_empty() && exit_code == 0 { + exit_code = 1; + } + } + Err(err) => { + send_runner_error( + &format!("protected metadata cleanup failed: {err:#}"), + &stdout_tx, + stderr_tx.as_ref(), + ); + if exit_code == 0 { + exit_code = 1; + } + } + } + } + let _ = exit_tx.send(exit_code); break; } Message::Error { payload } => {