From 9fe925b15a55071f39f49f8026e7dd4fc7e500d1 Mon Sep 17 00:00:00 2001 From: gt-oai Date: Mon, 9 Feb 2026 16:05:38 +0000 Subject: [PATCH] Load requirements on windows (#10770) We support requirements on Unix, loading from `/etc/codex/requirements.toml`. On MacOS, we also support MDM. Now, on Windows, we'll load requirements from `%ProgramData%\OpenAI\Codex\requirements.toml` --- codex-rs/Cargo.lock | 1 + .../app-server/tests/suite/v2/config_rpc.rs | 43 +--- codex-rs/core/Cargo.toml | 5 + codex-rs/core/src/config/service.rs | 52 ++--- .../src/config_loader/config_requirements.rs | 13 +- codex-rs/core/src/config_loader/mod.rs | 205 ++++++++++++++---- codex-rs/core/src/config_loader/tests.rs | 5 +- codex-rs/core/src/skills/loader.rs | 6 +- codex-rs/tui/src/debug_config.rs | 2 +- 9 files changed, 209 insertions(+), 123 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 90d711792..e6e9cc22c 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1643,6 +1643,7 @@ dependencies = [ "walkdir", "which", "wildmatch", + "windows-sys 0.52.0", "wiremock", "zip", "zstd", diff --git a/codex-rs/app-server/tests/suite/v2/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index fe4d2c449..4129564b1 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -22,7 +22,6 @@ use codex_app_server_protocol::SandboxMode; use codex_app_server_protocol::ToolsV2; use codex_app_server_protocol::WriteStatus; use codex_core::config::set_project_trust_level; -use codex_core::config_loader::SYSTEM_CONFIG_TOML_FILE_UNIX; use codex_protocol::config_types::TrustLevel; use codex_protocol::openai_models::ReasoningEffort; use codex_utils_absolute_path::AbsolutePathBuf; @@ -561,18 +560,9 @@ fn assert_layers_user_then_optional_system( layers: &[codex_app_server_protocol::ConfigLayer], user_file: AbsolutePathBuf, ) -> Result<()> { - if cfg!(unix) { - let system_file = AbsolutePathBuf::from_absolute_path(SYSTEM_CONFIG_TOML_FILE_UNIX)?; - assert_eq!(layers.len(), 2); - assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file }); - assert_eq!( - layers[1].name, - ConfigLayerSource::System { file: system_file } - ); - } else { - assert_eq!(layers.len(), 1); - assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file }); - } + assert_eq!(layers.len(), 2); + assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file }); + assert!(matches!(layers[1].name, ConfigLayerSource::System { .. })); Ok(()) } @@ -581,25 +571,12 @@ fn assert_layers_managed_user_then_optional_system( managed_file: AbsolutePathBuf, user_file: AbsolutePathBuf, ) -> Result<()> { - if cfg!(unix) { - let system_file = AbsolutePathBuf::from_absolute_path(SYSTEM_CONFIG_TOML_FILE_UNIX)?; - assert_eq!(layers.len(), 3); - assert_eq!( - layers[0].name, - ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file } - ); - assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file }); - assert_eq!( - layers[2].name, - ConfigLayerSource::System { file: system_file } - ); - } else { - assert_eq!(layers.len(), 2); - assert_eq!( - layers[0].name, - ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file } - ); - assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file }); - } + assert_eq!(layers.len(), 3); + assert_eq!( + layers[0].name, + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file } + ); + assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file }); + assert!(matches!(layers[2].name, ConfigLayerSource::System { .. })); Ok(()) } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index b38846315..5a8b07ce8 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -135,6 +135,11 @@ openssl-sys = { workspace = true, features = ["vendored"] } [target.'cfg(target_os = "windows")'.dependencies] keyring = { workspace = true, features = ["windows-native"] } +windows-sys = { version = "0.52", features = [ + "Win32_Foundation", + "Win32_System_Com", + "Win32_UI_Shell", +] } [target.'cfg(any(target_os = "freebsd", target_os = "openbsd"))'.dependencies] keyring = { workspace = true, features = ["sync-secret-service"] } diff --git a/codex-rs/core/src/config/service.rs b/codex-rs/core/src/config/service.rs index 36c638063..fa3768377 100644 --- a/codex-rs/core/src/config/service.rs +++ b/codex-rs/core/src/config/service.rs @@ -903,41 +903,23 @@ remote_models = true }, ); let layers = response.layers.expect("layers present"); - if cfg!(unix) { - let system_file = AbsolutePathBuf::from_absolute_path( - crate::config_loader::SYSTEM_CONFIG_TOML_FILE_UNIX, - ) - .expect("system file"); - assert_eq!(layers.len(), 3, "expected three layers on unix"); - assert_eq!( - layers.first().unwrap().name, - ConfigLayerSource::LegacyManagedConfigTomlFromFile { - file: managed_file.clone() - } - ); - assert_eq!( - layers.get(1).unwrap().name, - ConfigLayerSource::User { - file: user_file.clone() - } - ); - assert_eq!( - layers.get(2).unwrap().name, - ConfigLayerSource::System { file: system_file } - ); - } else { - assert_eq!(layers.len(), 2, "expected two layers"); - assert_eq!( - layers.first().unwrap().name, - ConfigLayerSource::LegacyManagedConfigTomlFromFile { - file: managed_file.clone() - } - ); - assert_eq!( - layers.get(1).unwrap().name, - ConfigLayerSource::User { file: user_file } - ); - } + assert_eq!(layers.len(), 3, "expected three layers"); + assert_eq!( + layers.first().unwrap().name, + ConfigLayerSource::LegacyManagedConfigTomlFromFile { + file: managed_file.clone() + } + ); + assert_eq!( + layers.get(1).unwrap().name, + ConfigLayerSource::User { + file: user_file.clone() + } + ); + assert!(matches!( + layers.get(2).unwrap().name, + ConfigLayerSource::System { .. } + )); } #[tokio::test] diff --git a/codex-rs/core/src/config_loader/config_requirements.rs b/codex-rs/core/src/config_loader/config_requirements.rs index c0810ff30..6ba68afeb 100644 --- a/codex-rs/core/src/config_loader/config_requirements.rs +++ b/codex-rs/core/src/config_loader/config_requirements.rs @@ -223,7 +223,7 @@ impl fmt::Display for WebSearchModeRequirement { } } -/// Base config deserialized from /etc/codex/requirements.toml or MDM. +/// Base config deserialized from system `requirements.toml` or MDM. #[derive(Deserialize, Debug, Clone, Default, PartialEq)] pub struct ConfigRequirementsToml { pub allowed_approval_policies: Option>, @@ -560,6 +560,7 @@ impl TryFrom for ConfigRequirements { #[cfg(test)] mod tests { use super::*; + use crate::config_loader::system_requirements_toml_file; use anyhow::Result; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; @@ -731,12 +732,7 @@ mod tests { "#, )?; - let requirements_toml_file = if cfg!(windows) { - "C:\\etc\\codex\\requirements.toml" - } else { - "/etc/codex/requirements.toml" - }; - let requirements_toml_file = AbsolutePathBuf::from_absolute_path(requirements_toml_file)?; + let requirements_toml_file = system_requirements_toml_file()?; let source_location = RequirementSource::SystemRequirementsToml { file: requirements_toml_file, }; @@ -1153,8 +1149,7 @@ mod tests { ] "#; let config: ConfigRequirementsToml = from_str(toml_str)?; - let requirements_toml_file = - AbsolutePathBuf::from_absolute_path("/etc/codex/requirements.toml")?; + let requirements_toml_file = system_requirements_toml_file()?; let source_location = RequirementSource::SystemRequirementsToml { file: requirements_toml_file, }; diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 0966ac874..a38baf7d4 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -29,6 +29,8 @@ use dunce::canonicalize as normalize_path; use serde::Deserialize; use std::io; use std::path::Path; +#[cfg(windows)] +use std::path::PathBuf; use toml::Value as TomlValue; pub use cloud_requirements::CloudRequirementsLoader; @@ -61,14 +63,14 @@ pub use state::ConfigLayerStack; pub use state::ConfigLayerStackOrdering; pub use state::LoaderOverrides; -/// On Unix systems, load requirements from this file path, if present. -const DEFAULT_REQUIREMENTS_TOML_FILE_UNIX: &str = "/etc/codex/requirements.toml"; - /// On Unix systems, load default settings from this file path, if present. /// Note that /etc/codex/ is treated as a "config folder," so subfolders such /// as skills/ and rules/ will also be honored. pub const SYSTEM_CONFIG_TOML_FILE_UNIX: &str = "/etc/codex/config.toml"; +#[cfg(windows)] +const DEFAULT_PROGRAM_DATA_DIR_WINDOWS: &str = r"C:\ProgramData"; + const DEFAULT_PROJECT_ROOT_MARKERS: &[&str] = &[".git"]; /// To build up the set of admin-enforced constraints, we build up from multiple @@ -77,16 +79,17 @@ const DEFAULT_PROJECT_ROOT_MARKERS: &[&str] = &[".git"]; /// /// - cloud: managed cloud requirements /// - admin: managed preferences (*) -/// - system `/etc/codex/requirements.toml` +/// - system `/etc/codex/requirements.toml` (Unix) or +/// `%ProgramData%\OpenAI\Codex\requirements.toml` (Windows) /// /// For backwards compatibility, we also load from -/// `/etc/codex/managed_config.toml` and map it to -/// `/etc/codex/requirements.toml`. +/// `managed_config.toml` and map it to `requirements.toml`. /// /// Configuration is built up from multiple layers in the following order: /// /// - admin: managed preferences (*) -/// - system `/etc/codex/config.toml` +/// - system `/etc/codex/config.toml` (Unix) or +/// `%ProgramData%\OpenAI\Codex\config.toml` (Windows) /// - user `${CODEX_HOME}/config.toml` /// - cwd `${PWD}/config.toml` (loaded but disabled when the directory is untrusted) /// - tree parent directories up to root looking for `./.codex/config.toml` (loaded but disabled when untrusted) @@ -124,14 +127,9 @@ pub async fn load_config_layers_state( ) .await?; - // Honor /etc/codex/requirements.toml. - if cfg!(unix) { - load_requirements_toml( - &mut config_requirements_toml, - DEFAULT_REQUIREMENTS_TOML_FILE_UNIX, - ) - .await?; - } + // Honor the system requirements.toml location. + let requirements_toml_file = system_requirements_toml_file()?; + load_requirements_toml(&mut config_requirements_toml, requirements_toml_file).await?; // Make a best-effort to support the legacy `managed_config.toml` as a // requirements specification. @@ -160,27 +158,18 @@ pub async fn load_config_layers_state( // Include an entry for the "system" config folder, loading its config.toml, // if it exists. - let system_config_toml_file = if cfg!(unix) { - Some(AbsolutePathBuf::from_absolute_path( - SYSTEM_CONFIG_TOML_FILE_UNIX, - )?) - } else { - // TODO(gt): Determine the path to load on Windows. - None - }; - if let Some(system_config_toml_file) = system_config_toml_file { - let system_layer = - load_config_toml_for_required_layer(&system_config_toml_file, |config_toml| { - ConfigLayerEntry::new( - ConfigLayerSource::System { - file: system_config_toml_file.clone(), - }, - config_toml, - ) - }) - .await?; - layers.push(system_layer); - } + let system_config_toml_file = system_config_toml_file()?; + let system_layer = + load_config_toml_for_required_layer(&system_config_toml_file, |config_toml| { + ConfigLayerEntry::new( + ConfigLayerSource::System { + file: system_config_toml_file.clone(), + }, + config_toml, + ) + }) + .await?; + layers.push(system_layer); // Add a layer for $CODEX_HOME/config.toml if it exists. Note if the file // exists, but is malformed, then this error should be propagated to the @@ -346,8 +335,9 @@ async fn load_config_toml_for_required_layer( Ok(create_entry(toml_value)) } -/// If available, apply requirements from `/etc/codex/requirements.toml` to -/// `config_requirements_toml` by filling in any unset fields. +/// If available, apply requirements from the platform system +/// `requirements.toml` location to `config_requirements_toml` by filling in +/// any unset fields. async fn load_requirements_toml( config_requirements_toml: &mut ConfigRequirementsWithSources, requirements_toml_file: impl AsRef, @@ -389,6 +379,99 @@ async fn load_requirements_toml( Ok(()) } +#[cfg(unix)] +fn system_requirements_toml_file() -> io::Result { + AbsolutePathBuf::from_absolute_path(Path::new("/etc/codex/requirements.toml")) +} + +#[cfg(windows)] +fn system_requirements_toml_file() -> io::Result { + windows_system_requirements_toml_file() +} + +#[cfg(unix)] +fn system_config_toml_file() -> io::Result { + AbsolutePathBuf::from_absolute_path(Path::new(SYSTEM_CONFIG_TOML_FILE_UNIX)) +} + +#[cfg(windows)] +fn system_config_toml_file() -> io::Result { + windows_system_config_toml_file() +} + +#[cfg(windows)] +fn windows_codex_system_dir() -> PathBuf { + let program_data = windows_program_data_dir_from_known_folder().unwrap_or_else(|err| { + tracing::warn!( + error = %err, + "Failed to resolve ProgramData known folder; using default path" + ); + PathBuf::from(DEFAULT_PROGRAM_DATA_DIR_WINDOWS) + }); + program_data.join("OpenAI").join("Codex") +} + +#[cfg(windows)] +fn windows_system_requirements_toml_file() -> io::Result { + let requirements_toml_file = windows_codex_system_dir().join("requirements.toml"); + AbsolutePathBuf::try_from(requirements_toml_file) +} + +#[cfg(windows)] +fn windows_system_config_toml_file() -> io::Result { + let config_toml_file = windows_codex_system_dir().join("config.toml"); + AbsolutePathBuf::try_from(config_toml_file) +} + +#[cfg(windows)] +fn windows_program_data_dir_from_known_folder() -> io::Result { + use std::ffi::OsString; + use std::os::windows::ffi::OsStringExt; + use windows_sys::Win32::System::Com::CoTaskMemFree; + use windows_sys::Win32::UI::Shell::FOLDERID_ProgramData; + use windows_sys::Win32::UI::Shell::KF_FLAG_DEFAULT; + use windows_sys::Win32::UI::Shell::SHGetKnownFolderPath; + + let mut path_ptr = std::ptr::null_mut::(); + let known_folder_flags = u32::try_from(KF_FLAG_DEFAULT).map_err(|_| { + io::Error::other(format!( + "KF_FLAG_DEFAULT did not fit in u32: {KF_FLAG_DEFAULT}" + )) + })?; + // Known folder IDs reference: + // https://learn.microsoft.com/en-us/windows/win32/shell/knownfolderid + // SAFETY: SHGetKnownFolderPath initializes path_ptr with a CoTaskMem-allocated, + // null-terminated UTF-16 string on success. + let hr = unsafe { + SHGetKnownFolderPath(&FOLDERID_ProgramData, known_folder_flags, 0, &mut path_ptr) + }; + if hr != 0 { + return Err(io::Error::other(format!( + "SHGetKnownFolderPath(FOLDERID_ProgramData) failed with HRESULT {hr:#010x}" + ))); + } + if path_ptr.is_null() { + return Err(io::Error::other( + "SHGetKnownFolderPath(FOLDERID_ProgramData) returned a null pointer", + )); + } + + // SAFETY: path_ptr is a valid null-terminated UTF-16 string allocated by + // SHGetKnownFolderPath and must be freed with CoTaskMemFree. + let path = unsafe { + let mut len = 0usize; + while *path_ptr.add(len) != 0 { + len += 1; + } + let wide = std::slice::from_raw_parts(path_ptr, len); + let path = PathBuf::from(OsString::from_wide(wide)); + CoTaskMemFree(path_ptr.cast()); + path + }; + + Ok(path) +} + async fn load_requirements_from_legacy_scheme( config_requirements_toml: &mut ConfigRequirementsWithSources, loaded_config_layers: LoadedConfigLayers, @@ -826,6 +909,8 @@ impl From for ConfigRequirementsToml { #[cfg(test)] mod unit_tests { use super::*; + #[cfg(windows)] + use std::path::Path; use tempfile::tempdir; #[test] @@ -882,4 +967,48 @@ foo = "xyzzy" ]) ); } + + #[cfg(windows)] + #[test] + fn windows_system_requirements_toml_file_uses_expected_suffix() { + let expected = windows_program_data_dir_from_known_folder() + .unwrap_or_else(|_| PathBuf::from(DEFAULT_PROGRAM_DATA_DIR_WINDOWS)) + .join("OpenAI") + .join("Codex") + .join("requirements.toml"); + assert_eq!( + windows_system_requirements_toml_file() + .expect("requirements.toml path") + .as_path(), + expected.as_path() + ); + assert!( + windows_system_requirements_toml_file() + .expect("requirements.toml path") + .as_path() + .ends_with(Path::new("OpenAI").join("Codex").join("requirements.toml")) + ); + } + + #[cfg(windows)] + #[test] + fn windows_system_config_toml_file_uses_expected_suffix() { + let expected = windows_program_data_dir_from_known_folder() + .unwrap_or_else(|_| PathBuf::from(DEFAULT_PROGRAM_DATA_DIR_WINDOWS)) + .join("OpenAI") + .join("Codex") + .join("config.toml"); + assert_eq!( + windows_system_config_toml_file() + .expect("config.toml path") + .as_path(), + expected.as_path() + ); + assert!( + windows_system_config_toml_file() + .expect("config.toml path") + .as_path() + .ends_with(Path::new("OpenAI").join("Codex").join("config.toml")) + ); + } } diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 1b5125840..0a6f67d85 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -286,10 +286,9 @@ async fn returns_empty_when_all_layers_missing() { .iter() .filter(|layer| matches!(layer.name, super::ConfigLayerSource::System { .. })) .count(); - let expected_system_layers = if cfg!(unix) { 1 } else { 0 }; assert_eq!( - num_system_layers, expected_system_layers, - "system layer should be present only on unix" + num_system_layers, 1, + "system layer should always be present" ); #[cfg(not(target_os = "macos"))] diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index a04f3089b..44a503888 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -2200,7 +2200,7 @@ interface: } #[tokio::test] - async fn skill_roots_include_admin_with_lowest_priority_on_unix() { + async fn skill_roots_include_admin_with_lowest_priority() { let codex_home = tempfile::tempdir().expect("tempdir"); let cfg = make_config(&codex_home).await; @@ -2212,9 +2212,7 @@ interface: if home_dir().is_some() { expected.insert(1, SkillScope::User); } - if cfg!(unix) { - expected.push(SkillScope::Admin); - } + expected.push(SkillScope::Admin); assert_eq!(scopes, expected); } } diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index 8a6a849b9..908ee1d24 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -356,7 +356,7 @@ mod tests { #[test] fn debug_config_output_lists_requirement_sources() { let requirements_file = if cfg!(windows) { - absolute_path("C:\\etc\\codex\\requirements.toml") + absolute_path("C:\\ProgramData\\OpenAI\\Codex\\requirements.toml") } else { absolute_path("/etc/codex/requirements.toml") };