diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index c0fa80e7f0..ac41557e59 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2867,6 +2867,7 @@ dependencies = [ "codex-utils-path", "codex-utils-path-uri", "codex-utils-redacted-string", + "codex-windows-sandbox", "core-foundation 0.9.4", "dns-lookup", "dunce", diff --git a/codex-rs/config/Cargo.toml b/codex-rs/config/Cargo.toml index 37a1c32439..a6b06169a5 100644 --- a/codex-rs/config/Cargo.toml +++ b/codex-rs/config/Cargo.toml @@ -55,6 +55,7 @@ libc = { workspace = true } core-foundation = "0.9" [target.'cfg(target_os = "windows")'.dependencies] +codex-windows-sandbox = { workspace = true } winapi-util = { workspace = true } windows-sys = { version = "0.52", features = [ "Win32_Foundation", diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index 35f042a48a..82dbcc3d59 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -5,6 +5,8 @@ mod macos; mod project_discovery; #[cfg(test)] mod tests; +#[cfg(windows)] +mod windows; use self::layer_io::LoadedConfigLayers; use crate::CONFIG_TOML_FILE; @@ -58,6 +60,10 @@ pub use local::LocalConfigLayers; pub use local::LocalTomlLayer; pub use local::LocalTomlLayerStack; pub use local::load_local_config_layers; +#[cfg(windows)] +pub use windows::WindowsSystemConfigNamespaceProbe; +#[cfg(windows)] +pub use windows::probe_windows_system_config_namespace; #[cfg(unix)] const SYSTEM_CONFIG_TOML_FILE_UNIX: &str = "/etc/codex/config.toml"; diff --git a/codex-rs/config/src/loader/windows.rs b/codex-rs/config/src/loader/windows.rs new file mode 100644 index 0000000000..27375754ab --- /dev/null +++ b/codex-rs/config/src/loader/windows.rs @@ -0,0 +1,68 @@ +use super::windows_codex_system_dir; +use std::path::Path; + +/// Coarse compatibility result for the default Windows system-config namespace. +#[derive(Clone, Copy)] +pub enum WindowsSystemConfigNamespaceProbe { + Missing, + Expected, + UnexpectedType, + UnexpectedOwner, + StandardUserMutationAcl, + CheckError, +} + +/// Observes the common user-preclaimed-directory shape without changing config loading. +/// This is not a trust decision: it only checks directory owner and broad +/// standard-user mutation ACLs. +pub fn probe_windows_system_config_namespace() -> WindowsSystemConfigNamespaceProbe { + let codex_dir = windows_codex_system_dir(); + let has_system_file = match ( + codex_dir.join("config.toml").try_exists(), + codex_dir.join("requirements.toml").try_exists(), + ) { + (Ok(true), _) | (_, Ok(true)) => true, + (Ok(false), Ok(false)) => false, + _ => return WindowsSystemConfigNamespaceProbe::CheckError, + }; + if !has_system_file { + return WindowsSystemConfigNamespaceProbe::Missing; + } + let Some(openai_dir) = codex_dir.parent() else { + return WindowsSystemConfigNamespaceProbe::CheckError; + }; + let result = probe_directory( + openai_dir, + codex_windows_sandbox::path_has_standard_user_mutation_allow, + ); + if !matches!(result, WindowsSystemConfigNamespaceProbe::Expected) { + return result; + } + let result = probe_directory( + codex_dir.as_path(), + codex_windows_sandbox::path_or_child_file_has_standard_user_mutation_allow, + ); + if !matches!(result, WindowsSystemConfigNamespaceProbe::Expected) { + return result; + } + WindowsSystemConfigNamespaceProbe::Expected +} + +fn probe_directory( + path: &Path, + mutation_allow: fn(&Path) -> anyhow::Result, +) -> WindowsSystemConfigNamespaceProbe { + if !path.is_dir() { + return WindowsSystemConfigNamespaceProbe::UnexpectedType; + } + match codex_windows_sandbox::path_has_trusted_system_owner(path) { + Ok(true) => {} + Ok(false) => return WindowsSystemConfigNamespaceProbe::UnexpectedOwner, + Err(_) => return WindowsSystemConfigNamespaceProbe::CheckError, + } + match mutation_allow(path) { + Ok(false) => WindowsSystemConfigNamespaceProbe::Expected, + Ok(true) => WindowsSystemConfigNamespaceProbe::StandardUserMutationAcl, + Err(_) => WindowsSystemConfigNamespaceProbe::CheckError, + } +} diff --git a/codex-rs/core/src/config/metrics.rs b/codex-rs/core/src/config/metrics.rs new file mode 100644 index 0000000000..48d3d71215 --- /dev/null +++ b/codex-rs/core/src/config/metrics.rs @@ -0,0 +1,10 @@ +//! Metrics derived from loaded configuration at session start. + +use super::Config; +use codex_otel::SessionTelemetry; + +pub(crate) fn emit_session_start_metrics(config: &Config, telemetry: &SessionTelemetry) { + config.features.emit_metrics(telemetry); + #[cfg(windows)] + crate::windows_system_config::emit_namespace_squatting_probe(); +} diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index c4053ef20e..1d23a3dc1d 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -164,6 +164,7 @@ use toml_edit::DocumentMut; mod auth_keyring; pub mod edit; mod managed_features; +mod metrics; mod network_proxy_spec; mod otel; mod permission_profile_catalog; @@ -186,6 +187,7 @@ pub use codex_network_proxy::NetworkProxyAuditMetadata; use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile; pub use codex_sandboxing::system_bwrap_warning; pub use managed_features::ManagedFeatures; +pub(crate) use metrics::emit_session_start_metrics; pub use network_proxy_spec::NetworkProxySpec; pub use network_proxy_spec::StartedNetworkProxy; pub use permission_profile_catalog::PermissionProfileCatalogEntry; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 3d21c8ca04..55d1f474e5 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -111,6 +111,8 @@ mod stream_events_utils; pub mod test_support; mod unified_exec; pub mod windows_sandbox; +#[cfg(windows)] +mod windows_system_config; pub use client::X_RESPONSESAPI_INCLUDE_TIMING_METRICS_HEADER; pub use codex_protocol::config_types::ModelProviderAuthInfo; mod event_mapping; diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 2c3ea5fc9e..298ffb8759 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1143,7 +1143,7 @@ impl Session { model: Some(session_model.clone()), slug: Some(session_model), }; - config.features.emit_metrics(&session_telemetry); + crate::config::emit_session_start_metrics(config.as_ref(), &session_telemetry); let is_worktree = session_configuration.cwd().canonicalize().ok().and_then(|cwd| { codex_git_utils::repository_identity(&cwd).and_then(|_| { get_git_repo_root(&cwd).map(|root| root.join(".git").is_file()) diff --git a/codex-rs/core/src/windows_system_config.rs b/codex-rs/core/src/windows_system_config.rs new file mode 100644 index 0000000000..bca27d47d6 --- /dev/null +++ b/codex-rs/core/src/windows_system_config.rs @@ -0,0 +1,35 @@ +//! Telemetry for the default Windows system-config namespace. + +use codex_config::loader::WindowsSystemConfigNamespaceProbe as Probe; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; + +const NAMESPACE_SQUATTING_PROBE_METRIC: &str = + "codex.windows_system_config.namespace_squatting_probe"; +static NAMESPACE_SQUATTING_PROBE_RECORDED: AtomicBool = AtomicBool::new(false); + +/// Records the coarse system-config namespace probe once after metrics exist. +pub(crate) fn emit_namespace_squatting_probe() { + let Some(metrics) = codex_otel::global() else { + return; + }; + if NAMESPACE_SQUATTING_PROBE_RECORDED + .compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed) + .is_err() + { + return; + } + let result = match codex_config::loader::probe_windows_system_config_namespace() { + Probe::Missing => "missing", + Probe::Expected => "expected", + Probe::UnexpectedType => "unexpected_type", + Probe::UnexpectedOwner => "unexpected_owner", + Probe::StandardUserMutationAcl => "standard_user_acl", + Probe::CheckError => "check_error", + }; + let _ = metrics.counter( + NAMESPACE_SQUATTING_PROBE_METRIC, + /*inc*/ 1, + &[("result", result)], + ); +} diff --git a/codex-rs/windows-sandbox-rs/src/acl.rs b/codex-rs/windows-sandbox-rs/src/acl.rs index e35777ab78..4dbbb155ce 100644 --- a/codex-rs/windows-sandbox-rs/src/acl.rs +++ b/codex-rs/windows-sandbox-rs/src/acl.rs @@ -1,3 +1,5 @@ +use crate::winutil::resolve_sid; +use crate::winutil::sid_bytes_from_string; use crate::winutil::to_wide; use anyhow::Result; use anyhow::anyhow; @@ -28,7 +30,9 @@ use windows_sys::Win32::Security::EqualSid; use windows_sys::Win32::Security::GENERIC_MAPPING; use windows_sys::Win32::Security::GetAce; use windows_sys::Win32::Security::GetAclInformation; +use windows_sys::Win32::Security::IsValidAcl; use windows_sys::Win32::Security::MapGenericMask; +use windows_sys::Win32::Security::OWNER_SECURITY_INFORMATION; use windows_sys::Win32::Storage::FileSystem::CreateFileW; use windows_sys::Win32::Storage::FileSystem::DELETE; use windows_sys::Win32::Storage::FileSystem::FILE_ALL_ACCESS; @@ -47,7 +51,10 @@ use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_DATA; use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_EA; use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING; use windows_sys::Win32::Storage::FileSystem::READ_CONTROL; +use windows_sys::Win32::Storage::FileSystem::WRITE_DAC; +use windows_sys::Win32::Storage::FileSystem::WRITE_OWNER; const SE_KERNEL_OBJECT: u32 = 6; +const OBJECT_INHERIT_ACE_FLAG: u8 = 0x01; const INHERIT_ONLY_ACE: u8 = 0x08; const INHERITED_ACE: u8 = 0x10; const ACCESS_ALLOWED_ACE_TYPE: u8 = 0; @@ -55,6 +62,17 @@ const ACCESS_DENIED_ACE_TYPE: u8 = 1; const GENERIC_READ_MASK: u32 = 0x8000_0000; const GENERIC_WRITE_MASK: u32 = 0x4000_0000; const DENY_ACCESS: i32 = 3; +// TrustedInstaller is a deterministic service SID, not a machine-local account SID. +const TRUSTED_INSTALLER_SID: &str = + "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464"; +const STANDARD_USER_MUTATION_MASK: u32 = FILE_WRITE_DATA + | FILE_APPEND_DATA + | FILE_WRITE_EA + | FILE_WRITE_ATTRIBUTES + | DELETE + | FILE_DELETE_CHILD + | WRITE_DAC + | WRITE_OWNER; fn acl_api_result(path: &Path, operation: &str, code: u32) -> Result<()> { if code == ERROR_SUCCESS { @@ -126,6 +144,7 @@ pub unsafe fn dacl_mask_allows( enum AceScope { Effective, Explicit, + EffectiveOrChildFile, } unsafe fn dacl_mask_allows_with_scope( @@ -163,7 +182,10 @@ unsafe fn dacl_mask_allows_with_scope( if hdr.AceType != ACCESS_ALLOWED_ACE_TYPE { continue; // not ACCESS_ALLOWED } - if (hdr.AceFlags & INHERIT_ONLY_ACE) != 0 { + if (hdr.AceFlags & INHERIT_ONLY_ACE) != 0 + && (!matches!(scope, AceScope::EffectiveOrChildFile) + || (hdr.AceFlags & OBJECT_INHERIT_ACE_FLAG) == 0) + { continue; } // SET_ACCESS cannot replace an ACE inherited from an ancestor, so it cannot make @@ -212,6 +234,105 @@ pub fn path_mask_allows( ) } +/// Whether a broad standard-user principal has an allow ACE that can mutate +/// the path. This is intentionally conservative and does not evaluate deny +/// ACEs against a concrete token. +pub fn path_has_standard_user_mutation_allow(path: &Path) -> Result { + path_has_standard_user_mutation_allow_with_scope(path, AceScope::Effective) +} + +/// Whether a broad standard-user principal has an allow ACE that can mutate +/// the path or a directly inherited child file. +pub fn path_or_child_file_has_standard_user_mutation_allow(path: &Path) -> Result { + path_has_standard_user_mutation_allow_with_scope(path, AceScope::EffectiveOrChildFile) +} + +fn path_has_standard_user_mutation_allow_with_scope(path: &Path, scope: AceScope) -> Result { + // These canonical labels map to fixed SIDs before any localized account lookup. + let mut users = resolve_sid("Users")?; + let mut authenticated_users = resolve_sid("Authenticated Users")?; + let mut everyone = resolve_sid("Everyone")?; + let sids = [ + users.as_mut_ptr().cast(), + authenticated_users.as_mut_ptr().cast(), + everyone.as_mut_ptr().cast(), + ]; + unsafe { + let (dacl, descriptor) = fetch_dacl_handle(path)?; + let result = if dacl.is_null() { + Ok(true) + } else if IsValidAcl(dacl) == 0 { + Err(anyhow!("invalid DACL for {}", path.display())) + } else { + Ok(dacl_mask_allows_with_scope( + dacl, + &sids, + STANDARD_USER_MUTATION_MASK, + /*require_all_bits*/ false, + scope, + )) + }; + if !descriptor.is_null() { + LocalFree(descriptor as HLOCAL); + } + result + } +} + +/// Whether the path owner is one of the conservative administrator-controlled +/// principals used by the Windows system-config shadow check. +pub fn path_has_trusted_system_owner(path: &Path) -> Result { + // These canonical labels map to fixed SIDs before any localized account lookup. + let mut administrators = resolve_sid("Administrators")?; + let mut system = resolve_sid("SYSTEM")?; + let mut trusted_installer = sid_bytes_from_string(TRUSTED_INSTALLER_SID)?; + let trusted_sids = [ + administrators.as_mut_ptr().cast(), + system.as_mut_ptr().cast(), + trusted_installer.as_mut_ptr().cast(), + ]; + path_owner_matches(path, &trusted_sids) +} + +fn path_owner_matches(path: &Path, trusted_sids: &[*mut c_void]) -> Result { + let mut owner = std::ptr::null_mut(); + let mut descriptor = std::ptr::null_mut(); + let code = unsafe { + GetNamedSecurityInfoW( + to_wide(path).as_ptr(), + 1, // SE_FILE_OBJECT + OWNER_SECURITY_INFORMATION, + &mut owner, + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut descriptor, + ) + }; + if code != ERROR_SUCCESS { + if !descriptor.is_null() { + unsafe { LocalFree(descriptor as HLOCAL) }; + } + return Err(anyhow!( + "GetNamedSecurityInfoW failed for {}: {code}", + path.display() + )); + } + if owner.is_null() { + if !descriptor.is_null() { + unsafe { LocalFree(descriptor as HLOCAL) }; + } + return Err(anyhow!("missing owner for {}", path.display())); + } + let matches = trusted_sids + .iter() + .any(|trusted_sid| unsafe { EqualSid(owner, *trusted_sid) } != 0); + if !descriptor.is_null() { + unsafe { LocalFree(descriptor as HLOCAL) }; + } + Ok(matches) +} + fn path_mask_allows_with_scope( path: &Path, psids: &[*mut c_void], diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 1f6d236da5..e83a42a59f 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -170,8 +170,14 @@ pub use acl::ensure_allow_write_aces; #[cfg(target_os = "windows")] pub use acl::fetch_dacl_handle; #[cfg(target_os = "windows")] +pub use acl::path_has_standard_user_mutation_allow; +#[cfg(target_os = "windows")] +pub use acl::path_has_trusted_system_owner; +#[cfg(target_os = "windows")] pub use acl::path_mask_allows; #[cfg(target_os = "windows")] +pub use acl::path_or_child_file_has_standard_user_mutation_allow; +#[cfg(target_os = "windows")] pub use acl::path_write_aces_need_refresh; #[cfg(target_os = "windows")] pub use audit::apply_world_writable_scan_and_denies_for_permissions;