diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 19fa0c6633..9b3e7ba6a4 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -208,6 +208,7 @@ v2_enum_from_core!( } ); +// TODO(mbolin): Support in-repo layer. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(tag = "type", rename_all = "camelCase")] #[ts(tag = "type")] @@ -216,17 +217,59 @@ pub enum ConfigLayerSource { /// Managed preferences layer delivered by MDM (macOS only). #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] - Mdm { domain: String, key: String }, + Mdm { + domain: String, + key: String, + }, + /// Managed config layer from a file (usually `managed_config.toml`). #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] - System { file: AbsolutePathBuf }, - /// Session-layer overrides supplied via `-c`/`--config`. - SessionFlags, - /// User config layer from a file (usually `config.toml`). + System { + file: AbsolutePathBuf, + }, + + /// User config layer from $CODEX_HOME/config.toml. This layer is special + /// in that it is expected to be: + /// - writable by the user + /// - generally outside the workspace directory #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] - User { file: AbsolutePathBuf }, + User { + file: AbsolutePathBuf, + }, + + /// Session-layer overrides supplied via `-c`/`--config`. + SessionFlags, + + /// `managed_config.toml` was designed to be a config that was loaded + /// as the last layer on top of everything else. This scheme did not quite + /// work out as intended, but we keep this variant as a "best effort" while + /// we phase out `managed_config.toml` in favor of `requirements.toml`. + LegacyManagedConfigTomlFromFile { + file: AbsolutePathBuf, + }, + + LegacyManagedConfigTomlFromMdm, +} + +impl ConfigLayerSource { + pub fn precedence(&self) -> i16 { + match self { + ConfigLayerSource::Mdm { .. } => 0, + ConfigLayerSource::System { .. } => 10, + ConfigLayerSource::User { .. } => 20, + ConfigLayerSource::SessionFlags => 30, + ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. } => 40, + ConfigLayerSource::LegacyManagedConfigTomlFromMdm => 50, + } + } +} + +impl PartialOrd for ConfigLayerSource { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.precedence().cmp(&other.precedence())) + } } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema, TS)] @@ -344,7 +387,7 @@ pub struct ConfigWriteResponse { pub status: WriteStatus, pub version: String, /// Canonical path to the config file that was written. - pub file_path: String, + pub file_path: AbsolutePathBuf, pub overridden_metadata: Option, } @@ -357,6 +400,7 @@ pub enum ConfigWriteErrorCode { ConfigValidationError, ConfigPathNotFound, ConfigSchemaUnknownKey, + UserLayerNotFound, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] 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 d26f36fa3f..805601d599 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -73,9 +73,8 @@ sandbox_mode = "workspace-write" } ); let layers = layers.expect("layers present"); - assert_eq!(layers.len(), 2); - assert_eq!(layers[0].name, ConfigLayerSource::SessionFlags); - assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file }); + assert_eq!(layers.len(), 1); + assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file }); Ok(()) } @@ -137,9 +136,8 @@ view_image = false ); let layers = layers.expect("layers present"); - assert_eq!(layers.len(), 2); - assert_eq!(layers[0].name, ConfigLayerSource::SessionFlags); - assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file }); + assert_eq!(layers.len(), 1); + assert_eq!(layers[0].name, ConfigLayerSource::User { file: user_file }); Ok(()) } @@ -211,7 +209,7 @@ writable_roots = [{}] assert_eq!(config.model.as_deref(), Some("gpt-system")); assert_eq!( origins.get("model").expect("origin").name, - ConfigLayerSource::System { + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file.clone(), } ); @@ -219,7 +217,7 @@ writable_roots = [{}] assert_eq!(config.approval_policy, Some(AskForApproval::Never)); assert_eq!( origins.get("approval_policy").expect("origin").name, - ConfigLayerSource::System { + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file.clone(), } ); @@ -242,7 +240,7 @@ writable_roots = [{}] .get("sandbox_workspace_write.writable_roots.0") .expect("origin") .name, - ConfigLayerSource::System { + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file.clone(), } ); @@ -259,28 +257,28 @@ writable_roots = [{}] ); let layers = layers.expect("layers present"); - assert_eq!(layers.len(), 3); + assert_eq!(layers.len(), 2); assert_eq!( layers[0].name, - ConfigLayerSource::System { file: managed_file } + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file } ); - assert_eq!(layers[1].name, ConfigLayerSource::SessionFlags); - assert_eq!(layers[2].name, ConfigLayerSource::User { file: user_file }); + assert_eq!(layers[1].name, ConfigLayerSource::User { file: user_file }); Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn config_value_write_replaces_value() -> Result<()> { - let codex_home = TempDir::new()?; + let temp_dir = TempDir::new()?; + let codex_home = temp_dir.path().canonicalize()?; write_config( - &codex_home, + &temp_dir, r#" model = "gpt-old" "#, )?; - let mut mcp = McpProcess::new(codex_home.path()).await?; + let mut mcp = McpProcess::new(&codex_home).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let read_id = mcp @@ -311,13 +309,7 @@ model = "gpt-old" ) .await??; let write: ConfigWriteResponse = to_response(write_resp)?; - let expected_file_path = codex_home - .path() - .join("config.toml") - .canonicalize() - .unwrap() - .display() - .to_string(); + let expected_file_path = AbsolutePathBuf::resolve_path_against_base("config.toml", codex_home)?; assert_eq!(write.status, WriteStatus::Ok); assert_eq!(write.file_path, expected_file_path); @@ -380,16 +372,17 @@ model = "gpt-old" #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn config_batch_write_applies_multiple_edits() -> Result<()> { - let codex_home = TempDir::new()?; - write_config(&codex_home, "")?; + let tmp_dir = TempDir::new()?; + let codex_home = tmp_dir.path().canonicalize()?; + write_config(&tmp_dir, "")?; - let mut mcp = McpProcess::new(codex_home.path()).await?; + let mut mcp = McpProcess::new(&codex_home).await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let writable_root = test_tmp_path_buf(); let batch_id = mcp .send_config_batch_write_request(ConfigBatchWriteParams { - file_path: Some(codex_home.path().join("config.toml").display().to_string()), + file_path: Some(codex_home.join("config.toml").display().to_string()), edits: vec![ ConfigEdit { key_path: "sandbox_mode".to_string(), @@ -415,13 +408,7 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> { .await??; let batch_write: ConfigWriteResponse = to_response(batch_resp)?; assert_eq!(batch_write.status, WriteStatus::Ok); - let expected_file_path = codex_home - .path() - .join("config.toml") - .canonicalize() - .unwrap() - .display() - .to_string(); + let expected_file_path = AbsolutePathBuf::resolve_path_against_base("config.toml", codex_home)?; assert_eq!(batch_write.file_path, expected_file_path); let read_id = mcp diff --git a/codex-rs/core/src/config/constraint.rs b/codex-rs/core/src/config/constraint.rs index f10df09ddd..229a434781 100644 --- a/codex-rs/core/src/config/constraint.rs +++ b/codex-rs/core/src/config/constraint.rs @@ -57,6 +57,32 @@ impl Constrained { } } + pub fn allow_only(value: T) -> Self + where + T: PartialEq + Send + Sync + fmt::Debug + Clone + 'static, + { + #[expect(clippy::expect_used)] + Self::new(value.clone(), move |candidate| { + if *candidate == value { + Ok(()) + } else { + Err(ConstraintError::invalid_value( + format!("{candidate:?}"), + format!("{value:?}"), + )) + } + }) + .expect("initial value should always be valid") + } + + /// Allow any value of T, using T's Default as the initial value. + pub fn allow_any_from_default() -> Self + where + T: Default, + { + Self::allow_any(T::default()) + } + pub fn allow_values(initial_value: T, allowed: Vec) -> ConstraintResult where T: PartialEq + Send + Sync + fmt::Debug + 'static, @@ -129,6 +155,12 @@ mod tests { assert_eq!(constrained.value(), -10); } + #[test] + fn constrained_allow_any_default_uses_default_value() { + let constrained = Constrained::::allow_any_from_default(); + assert_eq!(constrained.value(), 0); + } + #[test] fn constrained_new_rejects_invalid_initial_value() { let result = Constrained::new(0, |value| { diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 64f1ca5c1f..438e441b5c 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -12,6 +12,7 @@ use crate::config::types::ShellEnvironmentPolicy; use crate::config::types::ShellEnvironmentPolicyToml; use crate::config::types::Tui; use crate::config::types::UriBasedFileOpener; +use crate::config_loader::ConfigRequirements; use crate::config_loader::LoaderOverrides; use crate::config_loader::load_config_layers_state; use crate::features::Feature; @@ -356,7 +357,12 @@ impl ConfigBuilder { let config_toml: ConfigToml = merged_toml .try_into() .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; - Config::load_from_base_config_with_overrides(config_toml, harness_overrides, codex_home) + Config::load_config_with_requirements( + config_toml, + harness_overrides, + codex_home, + config_layer_stack.requirements().clone(), + ) } } @@ -390,20 +396,18 @@ impl Config { } } -/// DEPRECATED: Use [Config::load_with_cli_overrides()] instead because this -/// codepath is not guaranteed to honor [ConfigRequirements]. +/// DEPRECATED: Use [Config::load_with_cli_overrides()] instead because working +/// with [ConfigToml] directly means that [ConfigRequirements] have not been +/// applied yet, which risks failing to enforce required constraints. pub async fn load_config_as_toml_with_cli_overrides( codex_home: &Path, cli_overrides: Vec<(String, TomlValue)>, ) -> std::io::Result { - let root_value = load_resolved_config( - codex_home, - cli_overrides, - crate::config_loader::LoaderOverrides::default(), - ) - .await?; + let config_layer_stack = + load_config_layers_state(codex_home, &cli_overrides, LoaderOverrides::default()).await?; - let cfg = deserialize_config_toml_with_base(root_value, codex_home).map_err(|e| { + let merged_toml = config_layer_stack.effective_config(); + let cfg = deserialize_config_toml_with_base(merged_toml, codex_home).map_err(|e| { tracing::error!("Failed to deserialize overridden config: {e}"); e })?; @@ -411,15 +415,6 @@ pub async fn load_config_as_toml_with_cli_overrides( Ok(cfg) } -async fn load_resolved_config( - codex_home: &Path, - cli_overrides: Vec<(String, TomlValue)>, - overrides: crate::config_loader::LoaderOverrides, -) -> std::io::Result { - let layers = load_config_layers_state(codex_home, &cli_overrides, overrides).await?; - Ok(layers.effective_config()) -} - fn deserialize_config_toml_with_base( root_value: TomlValue, config_base_dir: &Path, @@ -435,13 +430,18 @@ fn deserialize_config_toml_with_base( pub async fn load_global_mcp_servers( codex_home: &Path, ) -> std::io::Result> { - let root_value = load_resolved_config( - codex_home, - Vec::new(), - crate::config_loader::LoaderOverrides::default(), - ) - .await?; - let Some(servers_value) = root_value.get("mcp_servers") else { + // In general, Config::load_with_cli_overrides() should be used to load the + // full config with requirements.toml applied, but in this case, we need + // access to the raw TOML in order to warn the user about deprecated fields. + // + // Note that a more precise way to do this would be to audit the individual + // config layers for deprecated fields rather than reporting on the merged + // result. + let cli_overrides = Vec::<(String, TomlValue)>::new(); + let config_layer_stack = + load_config_layers_state(codex_home, &cli_overrides, LoaderOverrides::default()).await?; + let merged_toml = config_layer_stack.effective_config(); + let Some(servers_value) = merged_toml.get("mcp_servers") else { return Ok(BTreeMap::new()); }; @@ -999,6 +999,16 @@ impl Config { cfg: ConfigToml, overrides: ConfigOverrides, codex_home: PathBuf, + ) -> std::io::Result { + let requirements = ConfigRequirements::default(); + Self::load_config_with_requirements(cfg, overrides, codex_home, requirements) + } + + fn load_config_with_requirements( + cfg: ConfigToml, + overrides: ConfigOverrides, + codex_home: PathBuf, + requirements: ConfigRequirements, ) -> std::io::Result { let user_instructions = Self::load_instructions(Some(&codex_home)); @@ -1104,7 +1114,8 @@ impl Config { AskForApproval::default() } }); - let approval_policy = Constrained::allow_any(approval_policy); + // TODO(dylan): We should be able to leverage ConfigLayerStack so that + // we can reliably check this at every config level. let did_user_set_custom_approval_policy_or_sandbox_mode = approval_policy_override .is_some() || config_profile.approval_policy.is_some() @@ -1221,6 +1232,16 @@ impl Config { let check_for_update_on_startup = cfg.check_for_update_on_startup.unwrap_or(true); + // Ensure that every field of ConfigRequirements is applied to the final + // Config. + let ConfigRequirements { + approval_policy: mut constrained_approval_policy, + } = requirements; + + constrained_approval_policy + .set(approval_policy) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{e}")))?; + let config = Self { model, review_model, @@ -1229,7 +1250,7 @@ impl Config { model_provider_id, model_provider, cwd: resolved_cwd, - approval_policy, + approval_policy: constrained_approval_policy, sandbox_policy, did_user_set_custom_approval_policy_or_sandbox_mode, forced_auto_mode_downgraded_on_windows, @@ -1920,18 +1941,22 @@ trust_level = "trusted" std::fs::write(&config_path, "mcp_oauth_credentials_store = \"file\"\n")?; std::fs::write(&managed_path, "mcp_oauth_credentials_store = \"keyring\"\n")?; - let overrides = crate::config_loader::LoaderOverrides { + let overrides = LoaderOverrides { managed_config_path: Some(managed_path.clone()), #[cfg(target_os = "macos")] managed_preferences_base64: None, }; - let root_value = load_resolved_config(codex_home.path(), Vec::new(), overrides).await?; - let cfg = - deserialize_config_toml_with_base(root_value, codex_home.path()).map_err(|e| { - tracing::error!("Failed to deserialize overridden config: {e}"); - e - })?; + let config_layer_stack = + load_config_layers_state(codex_home.path(), &Vec::new(), overrides).await?; + let cfg = deserialize_config_toml_with_base( + config_layer_stack.effective_config(), + codex_home.path(), + ) + .map_err(|e| { + tracing::error!("Failed to deserialize overridden config: {e}"); + e + })?; assert_eq!( cfg.mcp_oauth_credentials_store, Some(OAuthCredentialsStoreMode::Keyring), @@ -2035,24 +2060,27 @@ trust_level = "trusted" )?; std::fs::write(&managed_path, "model = \"managed_config\"\n")?; - let overrides = crate::config_loader::LoaderOverrides { + let overrides = LoaderOverrides { managed_config_path: Some(managed_path), #[cfg(target_os = "macos")] managed_preferences_base64: None, }; - let root_value = load_resolved_config( + let config_layer_stack = load_config_layers_state( codex_home.path(), - vec![("model".to_string(), TomlValue::String("cli".to_string()))], + &[("model".to_string(), TomlValue::String("cli".to_string()))], overrides, ) .await?; - let cfg = - deserialize_config_toml_with_base(root_value, codex_home.path()).map_err(|e| { - tracing::error!("Failed to deserialize overridden config: {e}"); - e - })?; + let cfg = deserialize_config_toml_with_base( + config_layer_stack.effective_config(), + codex_home.path(), + ) + .map_err(|e| { + tracing::error!("Failed to deserialize overridden config: {e}"); + e + })?; assert_eq!(cfg.model.as_deref(), Some("managed_config")); Ok(()) diff --git a/codex-rs/core/src/config/service.rs b/codex-rs/core/src/config/service.rs index 398a74c0d7..a219c15bcd 100644 --- a/codex-rs/core/src/config/service.rs +++ b/codex-rs/core/src/config/service.rs @@ -2,7 +2,6 @@ use super::CONFIG_TOML_FILE; use super::ConfigToml; use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; -use crate::config_loader::ConfigLayerEntry; use crate::config_loader::ConfigLayerStack; use crate::config_loader::LoaderOverrides; use crate::config_loader::load_config_layers_state; @@ -20,6 +19,7 @@ use codex_app_server_protocol::ConfigWriteResponse; use codex_app_server_protocol::MergeStrategy; use codex_app_server_protocol::OverriddenMetadata; use codex_app_server_protocol::WriteStatus; +use codex_utils_absolute_path::AbsolutePathBuf; use serde_json::Value as JsonValue; use std::path::Path; use std::path::PathBuf; @@ -146,7 +146,13 @@ impl ConfigService { Ok(ConfigReadResponse { config, origins: layers.origins(), - layers: params.include_layers.then(|| layers.layers_high_to_low()), + layers: params.include_layers.then(|| { + layers + .layers_high_to_low() + .iter() + .map(|layer| layer.as_layer()) + .collect() + }), }) } @@ -194,11 +200,14 @@ impl ConfigService { expected_version: Option, edits: Vec<(String, JsonValue, MergeStrategy)>, ) -> Result { - let allowed_path = self.codex_home.join(CONFIG_TOML_FILE); - let provided_path = file_path - .as_ref() - .map(PathBuf::from) - .unwrap_or_else(|| allowed_path.clone()); + let allowed_path = + AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, &self.codex_home) + .map_err(|err| ConfigServiceError::io("failed to resolve user config path", err))?; + let provided_path = match file_path { + Some(path) => AbsolutePathBuf::from_absolute_path(PathBuf::from(path)) + .map_err(|err| ConfigServiceError::io("failed to resolve user config path", err))?, + None => allowed_path.clone(), + }; if !paths_match(&allowed_path, &provided_path) { return Err(ConfigServiceError::write( @@ -211,9 +220,16 @@ impl ConfigService { .load_layers_state() .await .map_err(|err| ConfigServiceError::io("failed to load configuration", err))?; + let user_layer = layers.get_user_layer().ok_or_else(|| { + // TODO(mbolin): Support creating the user layer if it does not exist. + ConfigServiceError::write( + ConfigWriteErrorCode::UserLayerNotFound, + "user layer not found and we do not support creating it yet", + ) + })?; if let Some(expected) = expected_version.as_deref() - && expected != layers.user.version + && expected != user_layer.version { return Err(ConfigServiceError::write( ConfigWriteErrorCode::ConfigVersionConflict, @@ -221,7 +237,7 @@ impl ConfigService { )); } - let mut user_config = layers.user.config.clone(); + let mut user_config = user_layer.config.clone(); let mut parsed_segments = Vec::new(); let mut config_edits = Vec::new(); @@ -273,7 +289,7 @@ impl ConfigService { ) })?; - let updated_layers = layers.with_user_config(user_config.clone()); + let updated_layers = layers.with_user_config(&provided_path, user_config.clone()); let effective = updated_layers.effective_config(); validate_config(&effective).map_err(|err| { ConfigServiceError::write( @@ -296,16 +312,19 @@ impl ConfigService { .map(|_| WriteStatus::OkOverridden) .unwrap_or(WriteStatus::Ok); - let file_path = provided_path - .canonicalize() - .unwrap_or(provided_path.clone()) - .display() - .to_string(); - Ok(ConfigWriteResponse { status, - version: updated_layers.user.version.clone(), - file_path, + version: updated_layers + .get_user_layer() + .ok_or_else(|| { + ConfigServiceError::write( + ConfigWriteErrorCode::UserLayerNotFound, + "user layer not found in updated layers", + ) + })? + .version + .clone(), + file_path: provided_path, overridden_metadata: overridden, }) } @@ -470,15 +489,15 @@ fn validate_config(value: &TomlValue) -> Result<(), toml::de::Error> { Ok(()) } -fn paths_match(expected: &Path, provided: &Path) -> bool { +fn paths_match, Q: AsRef>(expected: P, provided: Q) -> bool { if let (Ok(expanded_expected), Ok(expanded_provided)) = ( - path_utils::normalize_for_path_comparison(expected), - path_utils::normalize_for_path_comparison(provided), + path_utils::normalize_for_path_comparison(&expected), + path_utils::normalize_for_path_comparison(&provided), ) { - return expanded_expected == expanded_provided; + expanded_expected == expanded_provided + } else { + expected.as_ref() == provided.as_ref() } - - expected == provided } fn value_at_path<'a>(root: &'a TomlValue, segments: &[String]) -> Option<&'a TomlValue> { @@ -501,10 +520,25 @@ fn value_at_path<'a>(root: &'a TomlValue, segments: &[String]) -> Option<&'a Tom fn override_message(layer: &ConfigLayerSource) -> String { match layer { - ConfigLayerSource::Mdm { .. } => "Overridden by managed policy (mdm)".to_string(), - ConfigLayerSource::System { .. } => "Overridden by managed config (system)".to_string(), + ConfigLayerSource::Mdm { domain, key: _ } => { + format!("Overridden by managed policy (MDM): {domain}") + } + ConfigLayerSource::System { file } => { + format!("Overridden by managed config (system): {}", file.display()) + } ConfigLayerSource::SessionFlags => "Overridden by session flags".to_string(), - ConfigLayerSource::User { .. } => "Overridden by user config".to_string(), + ConfigLayerSource::User { file } => { + format!("Overridden by user config: {}", file.display()) + } + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file } => { + format!( + "Overridden by legacy managed_config.toml: {}", + file.display() + ) + } + ConfigLayerSource::LegacyManagedConfigTomlFromMdm => { + "Overridden by legacy managed configuration from MDM".to_string() + } } } @@ -513,7 +547,10 @@ fn compute_override_metadata( effective: &TomlValue, segments: &[String], ) -> Option { - let user_value = value_at_path(&layers.user.config, segments); + let user_value = match layers.get_user_layer() { + Some(user_layer) => value_at_path(&user_layer.config, segments), + None => return None, + }; let effective_value = value_at_path(effective, segments); if user_value.is_some() && user_value == effective_value { @@ -524,8 +561,7 @@ fn compute_override_metadata( return None; } - let effective_layer = find_effective_layer(layers, segments); - let overriding_layer = effective_layer.unwrap_or_else(|| layers.user.metadata()); + let overriding_layer = find_effective_layer(layers, segments)?; let message = override_message(&overriding_layer.name); Some(OverriddenMetadata { @@ -554,23 +590,13 @@ fn find_effective_layer( layers: &ConfigLayerStack, segments: &[String], ) -> Option { - let check = - |state: &ConfigLayerEntry| value_at_path(&state.config, segments).map(|_| state.metadata()); + for layer in layers.layers_high_to_low() { + if let Some(meta) = value_at_path(&layer.config, segments).map(|_| layer.metadata()) { + return Some(meta); + } + } - if let Some(mdm) = &layers.mdm - && let Some(meta) = check(mdm) - { - return Some(meta); - } - if let Some(system) = &layers.system - && let Some(meta) = check(system) - { - return Some(meta); - } - if let Some(meta) = check(&layers.session_flags) { - return Some(meta); - } - check(&layers.user) + None } #[cfg(test)] @@ -713,18 +739,18 @@ remote_compaction = true .get("approval_policy") .expect("origin") .name, - ConfigLayerSource::System { - file: managed_file.clone(), - } + ConfigLayerSource::LegacyManagedConfigTomlFromFile { + file: managed_file.clone() + }, ); let layers = response.layers.expect("layers present"); + assert_eq!(layers.len(), 2, "expected two layers"); assert_eq!( layers.first().unwrap().name, - ConfigLayerSource::System { file: managed_file } + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file } ); - assert_eq!(layers.get(1).unwrap().name, ConfigLayerSource::SessionFlags); assert_eq!( - layers.last().unwrap().name, + layers.get(1).unwrap().name, ConfigLayerSource::User { file: user_file } ); } @@ -779,7 +805,9 @@ remote_compaction = true .get("approval_policy") .expect("origin") .name, - ConfigLayerSource::System { file: managed_file } + ConfigLayerSource::LegacyManagedConfigTomlFromFile { + file: managed_file.clone() + } ); assert_eq!(result.status, WriteStatus::Ok); assert!(result.overridden_metadata.is_none()); @@ -909,14 +937,14 @@ remote_compaction = true assert_eq!(response.config.model.as_deref(), Some("system")); assert_eq!( response.origins.get("model").expect("origin").name, - ConfigLayerSource::System { - file: managed_file.clone(), - } + ConfigLayerSource::LegacyManagedConfigTomlFromFile { + file: managed_file.clone() + }, ); let layers = response.layers.expect("layers"); assert_eq!( layers.first().unwrap().name, - ConfigLayerSource::System { file: managed_file } + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file } ); assert_eq!(layers.get(1).unwrap().name, ConfigLayerSource::SessionFlags); assert_eq!( @@ -959,7 +987,7 @@ remote_compaction = true let overridden = result.overridden_metadata.expect("overridden metadata"); assert_eq!( overridden.overriding_layer.name, - ConfigLayerSource::System { file: managed_file } + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: managed_file } ); assert_eq!(overridden.effective_value, serde_json::json!("never")); } diff --git a/codex-rs/core/src/config_loader/config_requirements.rs b/codex-rs/core/src/config_loader/config_requirements.rs new file mode 100644 index 0000000000..3a44315e37 --- /dev/null +++ b/codex-rs/core/src/config_loader/config_requirements.rs @@ -0,0 +1,38 @@ +use codex_protocol::protocol::AskForApproval; +use serde::Deserialize; + +use crate::config::Constrained; +use crate::config::ConstraintError; + +/// Normalized version of [`ConfigRequirementsToml`] after deserialization and +/// normalization. +#[derive(Debug, Clone, PartialEq)] +pub struct ConfigRequirements { + pub approval_policy: Constrained, +} + +impl Default for ConfigRequirements { + fn default() -> Self { + Self { + approval_policy: Constrained::allow_any_from_default(), + } + } +} + +/// Base config deserialized from /etc/codex/requirements.toml or MDM. +#[derive(Deserialize, Debug, Clone, Default, PartialEq)] +pub struct ConfigRequirementsToml { + pub approval_policy: Option>, +} + +impl TryFrom for ConfigRequirements { + type Error = ConstraintError; + + fn try_from(toml: ConfigRequirementsToml) -> Result { + let approval_policy: Constrained = match toml.approval_policy { + Some(policies) => Constrained::allow_values(AskForApproval::default(), policies)?, + None => Constrained::allow_any_from_default(), + }; + Ok(ConfigRequirements { approval_policy }) + } +} diff --git a/codex-rs/core/src/config_loader/layer_io.rs b/codex-rs/core/src/config_loader/layer_io.rs index 1e9052f216..996d19981f 100644 --- a/codex-rs/core/src/config_loader/layer_io.rs +++ b/codex-rs/core/src/config_loader/layer_io.rs @@ -1,7 +1,7 @@ use super::LoaderOverrides; +#[cfg(target_os = "macos")] use super::macos::load_managed_admin_config_layer; -use super::overrides::default_empty_table; -use crate::config::CONFIG_TOML_FILE; +use codex_utils_absolute_path::AbsolutePathBuf; use std::io; use std::path::Path; use std::path::PathBuf; @@ -11,11 +11,18 @@ use toml::Value as TomlValue; #[cfg(unix)] const CODEX_MANAGED_CONFIG_SYSTEM_PATH: &str = "/etc/codex/managed_config.toml"; +#[derive(Debug, Clone)] +pub(super) struct MangedConfigFromFile { + pub managed_config: TomlValue, + pub file: AbsolutePathBuf, +} + #[derive(Debug, Clone)] pub(super) struct LoadedConfigLayers { - pub base: TomlValue, - pub managed_config: Option, - pub managed_preferences: Option, + /// If present, data read from a file such as `/etc/codex/managed_config.toml`. + pub managed_config: Option, + /// If present, data read from managed preferences (macOS only). + pub managed_config_from_mdm: Option, } pub(super) async fn load_config_layers_internal( @@ -33,49 +40,52 @@ pub(super) async fn load_config_layers_internal( managed_config_path, } = overrides; - let managed_config_path = - managed_config_path.unwrap_or_else(|| managed_config_default_path(codex_home)); + let managed_config_path = AbsolutePathBuf::from_absolute_path( + managed_config_path.unwrap_or_else(|| managed_config_default_path(codex_home)), + )?; - let user_config_path = codex_home.join(CONFIG_TOML_FILE); - let user_config = read_config_from_path(&user_config_path, true).await?; - let managed_config = read_config_from_path(&managed_config_path, false).await?; + let managed_config = read_config_from_path(&managed_config_path, false) + .await? + .map(|managed_config| MangedConfigFromFile { + managed_config, + file: managed_config_path.clone(), + }); #[cfg(target_os = "macos")] let managed_preferences = load_managed_admin_config_layer(managed_preferences_base64.as_deref()).await?; #[cfg(not(target_os = "macos"))] - let managed_preferences = load_managed_admin_config_layer(None).await?; + let managed_preferences = None; Ok(LoadedConfigLayers { - base: user_config.unwrap_or_else(default_empty_table), managed_config, - managed_preferences, + managed_config_from_mdm: managed_preferences, }) } -pub(super) async fn read_config_from_path( - path: &Path, +pub(super) async fn read_config_from_path>( + path: P, log_missing_as_info: bool, ) -> io::Result> { - match fs::read_to_string(path).await { + match fs::read_to_string(path.as_ref()).await { Ok(contents) => match toml::from_str::(&contents) { Ok(value) => Ok(Some(value)), Err(err) => { - tracing::error!("Failed to parse {}: {err}", path.display()); + tracing::error!("Failed to parse {}: {err}", path.as_ref().display()); Err(io::Error::new(io::ErrorKind::InvalidData, err)) } }, Err(err) if err.kind() == io::ErrorKind::NotFound => { if log_missing_as_info { - tracing::info!("{} not found, using defaults", path.display()); + tracing::info!("{} not found, using defaults", path.as_ref().display()); } else { - tracing::debug!("{} not found", path.display()); + tracing::debug!("{} not found", path.as_ref().display()); } Ok(None) } Err(err) => { - tracing::error!("Failed to read {}: {err}", path.display()); + tracing::error!("Failed to read {}: {err}", path.as_ref().display()); Err(err) } } diff --git a/codex-rs/core/src/config_loader/macos.rs b/codex-rs/core/src/config_loader/macos.rs index 036619e06a..4a80267b90 100644 --- a/codex-rs/core/src/config_loader/macos.rs +++ b/codex-rs/core/src/config_loader/macos.rs @@ -1,118 +1,100 @@ +use base64::Engine; +use base64::prelude::BASE64_STANDARD; +use core_foundation::base::TCFType; +use core_foundation::string::CFString; +use core_foundation::string::CFStringRef; +use std::ffi::c_void; use std::io; +use tokio::task; use toml::Value as TomlValue; -#[cfg(target_os = "macos")] -mod native { - use super::*; - use base64::Engine; - use base64::prelude::BASE64_STANDARD; - use core_foundation::base::TCFType; - use core_foundation::string::CFString; - use core_foundation::string::CFStringRef; - use std::ffi::c_void; - use tokio::task; +const MANAGED_PREFERENCES_APPLICATION_ID: &str = "com.openai.codex"; +const MANAGED_PREFERENCES_CONFIG_KEY: &str = "config_toml_base64"; - pub(crate) async fn load_managed_admin_config_layer( - override_base64: Option<&str>, - ) -> io::Result> { - if let Some(encoded) = override_base64 { - let trimmed = encoded.trim(); - return if trimmed.is_empty() { - Ok(None) - } else { - parse_managed_preferences_base64(trimmed).map(Some) - }; - } - - const LOAD_ERROR: &str = "Failed to load managed preferences configuration"; - - match task::spawn_blocking(load_managed_admin_config).await { - Ok(result) => result, - Err(join_err) => { - if join_err.is_cancelled() { - tracing::error!("Managed preferences load task was cancelled"); - } else { - tracing::error!("Managed preferences load task failed: {join_err}"); - } - Err(io::Error::other(LOAD_ERROR)) - } - } - } - - pub(super) fn load_managed_admin_config() -> io::Result> { - #[link(name = "CoreFoundation", kind = "framework")] - unsafe extern "C" { - fn CFPreferencesCopyAppValue( - key: CFStringRef, - application_id: CFStringRef, - ) -> *mut c_void; - } - - const MANAGED_PREFERENCES_APPLICATION_ID: &str = "com.openai.codex"; - const MANAGED_PREFERENCES_CONFIG_KEY: &str = "config_toml_base64"; - - let application_id = CFString::new(MANAGED_PREFERENCES_APPLICATION_ID); - let key = CFString::new(MANAGED_PREFERENCES_CONFIG_KEY); - - let value_ref = unsafe { - CFPreferencesCopyAppValue( - key.as_concrete_TypeRef(), - application_id.as_concrete_TypeRef(), - ) - }; - - if value_ref.is_null() { - tracing::debug!( - "Managed preferences for {} key {} not found", - MANAGED_PREFERENCES_APPLICATION_ID, - MANAGED_PREFERENCES_CONFIG_KEY - ); - return Ok(None); - } - - let value = unsafe { CFString::wrap_under_create_rule(value_ref as _) }; - let contents = value.to_string(); - let trimmed = contents.trim(); - - parse_managed_preferences_base64(trimmed).map(Some) - } - - pub(super) fn parse_managed_preferences_base64(encoded: &str) -> io::Result { - let decoded = BASE64_STANDARD.decode(encoded.as_bytes()).map_err(|err| { - tracing::error!("Failed to decode managed preferences as base64: {err}"); - io::Error::new(io::ErrorKind::InvalidData, err) - })?; - - let decoded_str = String::from_utf8(decoded).map_err(|err| { - tracing::error!("Managed preferences base64 contents were not valid UTF-8: {err}"); - io::Error::new(io::ErrorKind::InvalidData, err) - })?; - - match toml::from_str::(&decoded_str) { - Ok(TomlValue::Table(parsed)) => Ok(TomlValue::Table(parsed)), - Ok(other) => { - tracing::error!( - "Managed preferences TOML must have a table at the root, found {other:?}", - ); - Err(io::Error::new( - io::ErrorKind::InvalidData, - "managed preferences root must be a table", - )) - } - Err(err) => { - tracing::error!("Failed to parse managed preferences TOML: {err}"); - Err(io::Error::new(io::ErrorKind::InvalidData, err)) - } - } - } -} - -#[cfg(target_os = "macos")] -pub(crate) use native::load_managed_admin_config_layer; - -#[cfg(not(target_os = "macos"))] pub(crate) async fn load_managed_admin_config_layer( - _override_base64: Option<&str>, + override_base64: Option<&str>, ) -> io::Result> { - Ok(None) + if let Some(encoded) = override_base64 { + let trimmed = encoded.trim(); + return if trimmed.is_empty() { + Ok(None) + } else { + parse_managed_preferences_base64(trimmed).map(Some) + }; + } + + const LOAD_ERROR: &str = "Failed to load managed preferences configuration"; + + match task::spawn_blocking(load_managed_admin_config).await { + Ok(result) => result, + Err(join_err) => { + if join_err.is_cancelled() { + tracing::error!("Managed preferences load task was cancelled"); + } else { + tracing::error!("Managed preferences load task failed: {join_err}"); + } + Err(io::Error::other(LOAD_ERROR)) + } + } +} + +fn load_managed_admin_config() -> io::Result> { + #[link(name = "CoreFoundation", kind = "framework")] + unsafe extern "C" { + fn CFPreferencesCopyAppValue(key: CFStringRef, application_id: CFStringRef) -> *mut c_void; + } + + let application_id = CFString::new(MANAGED_PREFERENCES_APPLICATION_ID); + let key = CFString::new(MANAGED_PREFERENCES_CONFIG_KEY); + + let value_ref = unsafe { + CFPreferencesCopyAppValue( + key.as_concrete_TypeRef(), + application_id.as_concrete_TypeRef(), + ) + }; + + if value_ref.is_null() { + tracing::debug!( + "Managed preferences for {} key {} not found", + MANAGED_PREFERENCES_APPLICATION_ID, + MANAGED_PREFERENCES_CONFIG_KEY + ); + return Ok(None); + } + + let value = unsafe { CFString::wrap_under_create_rule(value_ref as _) }; + let contents = value.to_string(); + let trimmed = contents.trim(); + + parse_managed_preferences_base64(trimmed).map(Some) +} + +fn parse_managed_preferences_base64(encoded: &str) -> io::Result { + let decoded = BASE64_STANDARD.decode(encoded.as_bytes()).map_err(|err| { + tracing::error!("Failed to decode managed preferences as base64: {err}"); + io::Error::new(io::ErrorKind::InvalidData, err) + })?; + + let decoded_str = String::from_utf8(decoded).map_err(|err| { + tracing::error!("Managed preferences base64 contents were not valid UTF-8: {err}"); + io::Error::new(io::ErrorKind::InvalidData, err) + })?; + + match toml::from_str::(&decoded_str) { + Ok(TomlValue::Table(parsed)) => Ok(TomlValue::Table(parsed)), + Ok(other) => { + tracing::error!( + "Managed preferences TOML must have a table at the root, found {other:?}", + ); + Err(io::Error::new( + io::ErrorKind::InvalidData, + "managed preferences root must be a table", + )) + } + Err(err) => { + tracing::error!("Failed to parse managed preferences TOML: {err}"); + Err(io::Error::new(io::ErrorKind::InvalidData, err)) + } + } } diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 2c3634ea04..db281cd09f 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -1,5 +1,7 @@ +mod config_requirements; mod fingerprint; mod layer_io; +#[cfg(target_os = "macos")] mod macos; mod merge; mod overrides; @@ -9,74 +11,167 @@ mod state; mod tests; use crate::config::CONFIG_TOML_FILE; +use crate::config_loader::layer_io::LoadedConfigLayers; use codex_app_server_protocol::ConfigLayerSource; +use codex_protocol::protocol::AskForApproval; use codex_utils_absolute_path::AbsolutePathBuf; +use serde::Deserialize; use std::io; use std::path::Path; use toml::Value as TomlValue; +pub use config_requirements::ConfigRequirements; pub use merge::merge_toml_values; pub use state::ConfigLayerEntry; pub use state::ConfigLayerStack; pub use state::LoaderOverrides; -const MDM_PREFERENCES_DOMAIN: &str = "com.openai.codex"; -const MDM_PREFERENCES_KEY: &str = "config_toml_base64"; - -/// Configuration layering pipeline (top overrides bottom): +/// To build up the set of admin-enforced constraints, we build up from multiple +/// configuration layers in the following order, but a constraint defined in an +/// earlier layer cannot be overridden by a later layer: /// -/// +-------------------------+ -/// | Managed preferences (*) | -/// +-------------------------+ -/// ^ -/// | -/// +-------------------------+ -/// | managed_config.toml | -/// +-------------------------+ -/// ^ -/// | -/// +-------------------------+ -/// | config.toml (base) | -/// +-------------------------+ +/// - admin: managed preferences (*) +/// - system `/etc/codex/requirements.toml` +/// +/// For backwards compatibility, we also load from +/// `/etc/codex/managed_config.toml` and map it to +/// `/etc/codex/requirements.toml`. +/// +/// Configuration is built up from multiple layers in the following order: +/// +/// - admin: managed preferences (*) +/// - system `/etc/codex/config.toml` +/// - user `${CODEX_HOME}/config.toml` +/// - cwd `${PWD}/config.toml` +/// - tree parent directories up to root looking for `./.codex/config.toml` +/// - repo `$(git rev-parse --show-toplevel)/.codex/config.toml` +/// - runtime e.g., --config flags, model selector in UI /// /// (*) Only available on macOS via managed device profiles. +/// +/// See https://developers.openai.com/codex/security for details. pub async fn load_config_layers_state( codex_home: &Path, cli_overrides: &[(String, TomlValue)], overrides: LoaderOverrides, ) -> io::Result { - let managed_config_path = overrides - .managed_config_path - .clone() - .unwrap_or_else(|| layer_io::managed_config_default_path(codex_home)); + let loaded_config_layers = layer_io::load_config_layers_internal(codex_home, overrides).await?; + let requirements = load_requirements_from_legacy_scheme(loaded_config_layers.clone()).await?; - let layers = layer_io::load_config_layers_internal(codex_home, overrides).await?; - let cli_overrides_layer = overrides::build_cli_overrides_layer(cli_overrides); - let user_file = AbsolutePathBuf::from_absolute_path(codex_home.join(CONFIG_TOML_FILE))?; + // TODO(mbolin): Honor /etc/codex/requirements.toml. - let system = match layers.managed_config { - Some(cfg) => { - let system_file = AbsolutePathBuf::from_absolute_path(managed_config_path.clone())?; - Some(ConfigLayerEntry::new( - ConfigLayerSource::System { file: system_file }, - cfg, - )) + let mut layers = Vec::::new(); + + // TODO(mbolin): Honor managed preferences (macOS only). + // TODO(mbolin): Honor /etc/codex/config.toml. + + // 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 + // user. + let user_file = AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, codex_home)?; + match tokio::fs::read_to_string(&user_file).await { + Ok(contents) => { + let user_config: TomlValue = + toml::from_str(&contents).unwrap_or(TomlValue::Table(toml::map::Map::new())); + layers.push(ConfigLayerEntry::new( + ConfigLayerSource::User { file: user_file }, + user_config, + )); } - None => None, - }; + Err(e) => { + if e.kind() != io::ErrorKind::NotFound { + return Err(io::Error::new( + e.kind(), + format!( + "Failed to read user config file {}: {e}", + user_file.as_path().display(), + ), + )); + } + } + } - Ok(ConfigLayerStack { - user: ConfigLayerEntry::new(ConfigLayerSource::User { file: user_file }, layers.base), - session_flags: ConfigLayerEntry::new(ConfigLayerSource::SessionFlags, cli_overrides_layer), - system, - mdm: layers.managed_preferences.map(|cfg| { - ConfigLayerEntry::new( - ConfigLayerSource::Mdm { - domain: MDM_PREFERENCES_DOMAIN.to_string(), - key: MDM_PREFERENCES_KEY.to_string(), - }, - cfg, - ) - }), - }) + // TODO(mbolin): Add layers for cwd, tree, and repo config files. + + // Add a layer for runtime overrides from the CLI or UI, if any exist. + if !cli_overrides.is_empty() { + let cli_overrides_layer = overrides::build_cli_overrides_layer(cli_overrides); + layers.push(ConfigLayerEntry::new( + ConfigLayerSource::SessionFlags, + cli_overrides_layer, + )); + } + + // Make a best-effort to support the legacy `managed_config.toml` as + // a config layer on top of everything else. Note users can still + // override these values on a per-turn basis in the TUI and VS Code. + let LoadedConfigLayers { + managed_config, + managed_config_from_mdm, + } = loaded_config_layers; + if let Some(config) = managed_config { + layers.push(ConfigLayerEntry::new( + ConfigLayerSource::LegacyManagedConfigTomlFromFile { + file: config.file.clone(), + }, + config.managed_config, + )); + } + if let Some(config) = managed_config_from_mdm { + layers.push(ConfigLayerEntry::new( + ConfigLayerSource::LegacyManagedConfigTomlFromMdm, + config, + )); + } + + ConfigLayerStack::new(layers, requirements) +} + +async fn load_requirements_from_legacy_scheme( + loaded_config_layers: LoadedConfigLayers, +) -> io::Result { + let mut config_requirements = ConfigRequirements::default(); + + // In this implementation, later layers override earlier layers, so list + // managed_config_from_mdm last because it has the highest precedence. + let LoadedConfigLayers { + managed_config, + managed_config_from_mdm, + } = loaded_config_layers; + for config in [ + managed_config.map(|c| c.managed_config), + managed_config_from_mdm, + ] + .into_iter() + .flatten() + { + let legacy_config: LegacyManagedConfigToml = + config.try_into().map_err(|err: toml::de::Error| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("Failed to parse config requirements as TOML: {err}"), + ) + })?; + + let LegacyManagedConfigToml { approval_policy } = legacy_config; + if let Some(approval_policy) = approval_policy { + config_requirements.approval_policy = + crate::config::Constrained::allow_only(approval_policy); + } + } + + Ok(config_requirements) +} + +/// The legacy mechanism for specifying admin-enforced configuration is to read +/// from a file like `/etc/codex/managed_config.toml` that has the same +/// structure as `config.toml` where fields like `approval_policy` can specify +/// exactly one value rather than a list of allowed values. +/// +/// If present, re-interpret `managed_config.toml` as a `requirements.toml` +/// where each specified field is treated as a constraint allowing only that +/// value. +#[derive(Deserialize, Debug, Clone, Default, PartialEq)] +struct LegacyManagedConfigToml { + approval_policy: Option, } diff --git a/codex-rs/core/src/config_loader/state.rs b/codex-rs/core/src/config_loader/state.rs index 4ae235b796..f3df27f3a0 100644 --- a/codex-rs/core/src/config_loader/state.rs +++ b/codex-rs/core/src/config_loader/state.rs @@ -1,9 +1,12 @@ +use crate::config_loader::ConfigRequirements; + use super::fingerprint::record_origins; use super::fingerprint::version_for_toml; use super::merge::merge_toml_values; use codex_app_server_protocol::ConfigLayer; use codex_app_server_protocol::ConfigLayerMetadata; use codex_app_server_protocol::ConfigLayerSource; +use codex_utils_absolute_path::AbsolutePathBuf; use serde_json::Value as JsonValue; use std::collections::HashMap; use std::path::PathBuf; @@ -51,30 +54,90 @@ impl ConfigLayerEntry { #[derive(Debug, Clone)] pub struct ConfigLayerStack { - pub user: ConfigLayerEntry, - pub session_flags: ConfigLayerEntry, - pub system: Option, - pub mdm: Option, + /// Layers are listed from lowest precedence (base) to highest (top), so + /// later entries in the Vec override earlier ones. + layers: Vec, + + /// Index into [layers] of the user config layer, if any. + user_layer_index: Option, + + /// Constraints that must be enforced when deriving a [Config] from the + /// layers. + requirements: ConfigRequirements, } impl ConfigLayerStack { - pub fn with_user_config(&self, user_config: TomlValue) -> Self { - Self { - user: ConfigLayerEntry::new(self.user.name.clone(), user_config), - session_flags: self.session_flags.clone(), - system: self.system.clone(), - mdm: self.mdm.clone(), + pub fn new( + layers: Vec, + requirements: ConfigRequirements, + ) -> std::io::Result { + let user_layer_index = verify_layer_ordering(&layers)?; + Ok(Self { + layers, + user_layer_index, + requirements, + }) + } + + /// Returns the user config layer, if any. + pub fn get_user_layer(&self) -> Option<&ConfigLayerEntry> { + self.user_layer_index + .and_then(|index| self.layers.get(index)) + } + + pub fn requirements(&self) -> &ConfigRequirements { + &self.requirements + } + + /// Creates a new [ConfigLayerStack] using the specified values to inject a + /// "user layer" into the stack. If such a layer already exists, it is + /// replaced; otherwise, it is inserted into the stack at the appropriate + /// position based on precedence rules. + pub fn with_user_config(&self, config_toml: &AbsolutePathBuf, user_config: TomlValue) -> Self { + let user_layer = ConfigLayerEntry::new( + ConfigLayerSource::User { + file: config_toml.clone(), + }, + user_config, + ); + + let mut layers = self.layers.clone(); + match self.user_layer_index { + Some(index) => { + layers[index] = user_layer; + Self { + layers, + user_layer_index: self.user_layer_index, + requirements: self.requirements.clone(), + } + } + None => { + let user_layer_index = match layers + .iter() + .position(|layer| layer.name.precedence() > user_layer.name.precedence()) + { + Some(index) => { + layers.insert(index, user_layer); + index + } + None => { + layers.push(user_layer); + layers.len() - 1 + } + }; + Self { + layers, + user_layer_index: Some(user_layer_index), + requirements: self.requirements.clone(), + } + } } } pub fn effective_config(&self) -> TomlValue { - let mut merged = self.user.config.clone(); - merge_toml_values(&mut merged, &self.session_flags.config); - if let Some(system) = &self.system { - merge_toml_values(&mut merged, &system.config); - } - if let Some(mdm) = &self.mdm { - merge_toml_values(&mut merged, &mdm.config); + let mut merged = TomlValue::Table(toml::map::Map::new()); + for layer in &self.layers { + merge_toml_values(&mut merged, &layer.config); } merged } @@ -83,38 +146,44 @@ impl ConfigLayerStack { let mut origins = HashMap::new(); let mut path = Vec::new(); - record_origins( - &self.user.config, - &self.user.metadata(), - &mut path, - &mut origins, - ); - record_origins( - &self.session_flags.config, - &self.session_flags.metadata(), - &mut path, - &mut origins, - ); - if let Some(system) = &self.system { - record_origins(&system.config, &system.metadata(), &mut path, &mut origins); - } - if let Some(mdm) = &self.mdm { - record_origins(&mdm.config, &mdm.metadata(), &mut path, &mut origins); + for layer in &self.layers { + record_origins(&layer.config, &layer.metadata(), &mut path, &mut origins); } origins } - pub fn layers_high_to_low(&self) -> Vec { - let mut layers = Vec::new(); - if let Some(mdm) = &self.mdm { - layers.push(mdm.as_layer()); - } - if let Some(system) = &self.system { - layers.push(system.as_layer()); - } - layers.push(self.session_flags.as_layer()); - layers.push(self.user.as_layer()); - layers + /// Returns the highest-precedence to lowest-precedence layers, so + /// `ConfigLayerSource::SessionFlags` would be first, if present. + pub fn layers_high_to_low(&self) -> Vec<&ConfigLayerEntry> { + self.layers.iter().rev().collect() } } + +/// Ensures precedence ordering of config layers is correct. Returns the index +/// of the user config layer, if any (at most one should exist). +fn verify_layer_ordering(layers: &[ConfigLayerEntry]) -> std::io::Result> { + if !layers.iter().map(|layer| &layer.name).is_sorted() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "config layers are not in correct precedence order", + )); + } + + let mut user_layer_index: Option = None; + let mut levels = Vec::with_capacity(layers.len()); + for (index, layer) in layers.iter().enumerate() { + levels.push(layer.name.precedence()); + if matches!(layer.name, ConfigLayerSource::User { .. }) { + if user_layer_index.is_some() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "multiple user config layers found", + )); + } + user_layer_index = Some(index); + } + } + + Ok(user_layer_index) +} diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 121ef736dc..15d4578367 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -66,13 +66,24 @@ async fn returns_empty_when_all_layers_missing() { let layers = load_config_layers_state(tmp.path(), &[] as &[(String, TomlValue)], overrides) .await .expect("load layers"); - let base_table = layers.user.config.as_table().expect("base table expected"); + assert!( + layers.get_user_layer().is_none(), + "no user layer when CODEX_HOME/config.toml does not exist" + ); + + let binding = layers.effective_config(); + let base_table = binding.as_table().expect("base table expected"); assert!( base_table.is_empty(), "expected empty base layer when configs missing" ); - assert!( - layers.system.is_none(), + let num_system_layers = layers + .layers_high_to_low() + .iter() + .filter(|layer| matches!(layer.name, super::ConfigLayerSource::System { .. })) + .count(); + assert_eq!( + num_system_layers, 0, "managed config layer should be absent when file missing" ); diff --git a/codex-rs/core/src/path_utils.rs b/codex-rs/core/src/path_utils.rs index 9a7007e4f1..eaa6195b23 100644 --- a/codex-rs/core/src/path_utils.rs +++ b/codex-rs/core/src/path_utils.rs @@ -3,8 +3,8 @@ use std::path::PathBuf; use crate::env; -pub fn normalize_for_path_comparison(path: &Path) -> std::io::Result { - let canonical = path.canonicalize()?; +pub fn normalize_for_path_comparison>(path: P) -> std::io::Result { + let canonical = path.as_ref().canonicalize()?; Ok(normalize_for_wsl(canonical)) }