Add telemetry for the Windows system config namespace (#44284)

## What changed

Probe the default Windows system config namespace at session start and record `codex.windows_system_config.namespace_squatting_probe` once per process when metrics are available.

When `config.toml` or `requirements.toml` exists, check the containing directories for expected ownership and broad standard-user mutation allow ACEs, including permissions inherited by child files. Report coarse results for missing files, expected directories, unexpected types or owners, mutation permissions, and check errors.

The probe is observational: it does not change configuration loading or enforce a trust decision.

GitOrigin-RevId: e1c79dca5548e16e83c7b84b44ab0de4e2643d0d
This commit is contained in:
iceweasel-oai
2026-09-09 19:31:12 +00:00
committed by copyberry
parent c1840dc55e
commit bb71d758cd
11 changed files with 254 additions and 2 deletions
+1
View File
@@ -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",
+1
View File
@@ -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",
+6
View File
@@ -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";
+68
View File
@@ -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<bool>,
) -> 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,
}
}
+10
View File
@@ -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();
}
+2
View File
@@ -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;
+2
View File
@@ -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;
+1 -1
View File
@@ -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())
@@ -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)],
);
}
+122 -1
View File
@@ -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<bool> {
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<bool> {
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<bool> {
// 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<bool> {
// 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<bool> {
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],
+6
View File
@@ -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;