diff --git a/codex-rs/config/src/lenient.rs b/codex-rs/config/src/lenient.rs new file mode 100644 index 0000000000..0c60e79677 --- /dev/null +++ b/codex-rs/config/src/lenient.rs @@ -0,0 +1,563 @@ +use crate::config_toml::RealtimeTransport; +use crate::config_toml::RealtimeWsMode; +use crate::config_toml::RealtimeWsVersion; +use crate::permissions_toml::NetworkDomainPermissionToml; +use crate::permissions_toml::NetworkUnixSocketPermissionToml; +use crate::types::AppToolApproval; +use crate::types::AuthCredentialsStoreMode; +use crate::types::NotificationCondition; +use crate::types::NotificationMethod; +use crate::types::OAuthCredentialsStoreMode; +use crate::types::OtelExporterKind; +use crate::types::OtelHttpProtocol; +use crate::types::UriBasedFileOpener; +use crate::types::WindowsSandboxModeToml; +use codex_protocol::config_types::AltScreenMode; +use codex_protocol::config_types::ApprovalsReviewer; +use codex_protocol::config_types::ForcedLoginMethod; +use codex_protocol::config_types::Personality; +use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::config_types::SandboxMode; +use codex_protocol::config_types::ServiceTier; +use codex_protocol::config_types::ShellEnvironmentPolicyInherit; +use codex_protocol::config_types::TrustLevel; +use codex_protocol::config_types::Verbosity; +use codex_protocol::config_types::WebSearchContextSize; +use codex_protocol::config_types::WebSearchMode; +use codex_protocol::config_types::WebSearchUserLocationType; +use codex_protocol::openai_models::ReasoningEffort; +use codex_protocol::protocol::AskForApproval; +use serde::Deserialize; +use serde::de::DeserializeOwned; +use toml::Value as TomlValue; +use toml::map::Map as TomlMap; + +#[derive(Deserialize)] +#[serde(untagged)] +enum Lenient { + Success(T), + Failure, +} + +pub(crate) fn sanitize_config_toml_enums( + value: &mut TomlValue, + source: impl Into, +) -> Vec { + let source = source.into(); + let mut warnings = Vec::new(); + let Some(table) = value.as_table_mut() else { + return warnings; + }; + + sanitize_config_table(table, &source, &mut warnings); + warnings +} + +fn sanitize_config_table( + table: &mut TomlMap, + source: &str, + warnings: &mut Vec, +) { + remove_invalid_enum::( + table, + "approval_policy", + "approval_policy", + source, + warnings, + ); + remove_invalid_enum::( + table, + "approvals_reviewer", + "approvals_reviewer", + source, + warnings, + ); + remove_invalid_enum::(table, "sandbox_mode", "sandbox_mode", source, warnings); + remove_invalid_enum::( + table, + "model_reasoning_effort", + "model_reasoning_effort", + source, + warnings, + ); + remove_invalid_enum::( + table, + "plan_mode_reasoning_effort", + "plan_mode_reasoning_effort", + source, + warnings, + ); + remove_invalid_enum::( + table, + "model_reasoning_summary", + "model_reasoning_summary", + source, + warnings, + ); + remove_invalid_enum::( + table, + "model_verbosity", + "model_verbosity", + source, + warnings, + ); + remove_invalid_enum::(table, "personality", "personality", source, warnings); + remove_invalid_enum::(table, "service_tier", "service_tier", source, warnings); + remove_invalid_enum::( + table, + "forced_login_method", + "forced_login_method", + source, + warnings, + ); + remove_invalid_enum::( + table, + "cli_auth_credentials_store", + "cli_auth_credentials_store", + source, + warnings, + ); + remove_invalid_enum::( + table, + "mcp_oauth_credentials_store", + "mcp_oauth_credentials_store", + source, + warnings, + ); + remove_invalid_enum::( + table, + "file_opener", + "file_opener", + source, + warnings, + ); + remove_invalid_enum::( + table, + "experimental_thread_store", + "experimental_thread_store", + source, + warnings, + ); + remove_invalid_enum::(table, "web_search", "web_search", source, warnings); + if let Some(tui) = table.get_mut("tui").and_then(TomlValue::as_table_mut) { + remove_invalid_enum::( + tui, + "notification_method", + "tui.notification_method", + source, + warnings, + ); + remove_invalid_enum::( + tui, + "notification_condition", + "tui.notification_condition", + source, + warnings, + ); + remove_invalid_enum::( + tui, + "alternate_screen", + "tui.alternate_screen", + source, + warnings, + ); + } + + if let Some(shell) = table + .get_mut("shell_environment_policy") + .and_then(TomlValue::as_table_mut) + { + remove_invalid_enum::( + shell, + "inherit", + "shell_environment_policy.inherit", + source, + warnings, + ); + } + + if let Some(windows) = table.get_mut("windows").and_then(TomlValue::as_table_mut) { + remove_invalid_enum::( + windows, + "sandbox", + "windows.sandbox", + source, + warnings, + ); + } + + if let Some(realtime) = table.get_mut("realtime").and_then(TomlValue::as_table_mut) { + remove_invalid_enum::( + realtime, + "version", + "realtime.version", + source, + warnings, + ); + remove_invalid_enum::(realtime, "type", "realtime.type", source, warnings); + remove_invalid_enum::( + realtime, + "transport", + "realtime.transport", + source, + warnings, + ); + } + + if let Some(web_search_tool) = table + .get_mut("tools") + .and_then(TomlValue::as_table_mut) + .and_then(|tools| tools.get_mut("web_search")) + .and_then(TomlValue::as_table_mut) + { + remove_invalid_enum::( + web_search_tool, + "search_context_size", + "tools.web_search.search_context_size", + source, + warnings, + ); + if let Some(user_location) = web_search_tool + .get_mut("user_location") + .and_then(TomlValue::as_table_mut) + { + remove_invalid_enum::( + user_location, + "type", + "tools.web_search.user_location.type", + source, + warnings, + ); + } + } + + if let Some(projects) = table.get_mut("projects").and_then(TomlValue::as_table_mut) { + for (name, project) in projects { + if let Some(project) = project.as_table_mut() { + remove_invalid_enum::( + project, + "trust_level", + &format!("projects.{name}.trust_level"), + source, + warnings, + ); + } + } + } + + sanitize_profiles(table, source, warnings); + sanitize_permissions(table, source, warnings); + sanitize_mcp_servers(table, source, warnings); + sanitize_plugins(table, source, warnings); + sanitize_otel(table, source, warnings); +} + +fn sanitize_profiles( + table: &mut TomlMap, + source: &str, + warnings: &mut Vec, +) { + let Some(profiles) = table.get_mut("profiles").and_then(TomlValue::as_table_mut) else { + return; + }; + for (name, profile) in profiles { + let Some(profile) = profile.as_table_mut() else { + continue; + }; + remove_invalid_enum::( + profile, + "service_tier", + &format!("profiles.{name}.service_tier"), + source, + warnings, + ); + remove_invalid_enum::( + profile, + "approval_policy", + &format!("profiles.{name}.approval_policy"), + source, + warnings, + ); + remove_invalid_enum::( + profile, + "approvals_reviewer", + &format!("profiles.{name}.approvals_reviewer"), + source, + warnings, + ); + remove_invalid_enum::( + profile, + "sandbox_mode", + &format!("profiles.{name}.sandbox_mode"), + source, + warnings, + ); + remove_invalid_enum::( + profile, + "model_reasoning_effort", + &format!("profiles.{name}.model_reasoning_effort"), + source, + warnings, + ); + remove_invalid_enum::( + profile, + "plan_mode_reasoning_effort", + &format!("profiles.{name}.plan_mode_reasoning_effort"), + source, + warnings, + ); + remove_invalid_enum::( + profile, + "model_reasoning_summary", + &format!("profiles.{name}.model_reasoning_summary"), + source, + warnings, + ); + remove_invalid_enum::( + profile, + "model_verbosity", + &format!("profiles.{name}.model_verbosity"), + source, + warnings, + ); + remove_invalid_enum::( + profile, + "personality", + &format!("profiles.{name}.personality"), + source, + warnings, + ); + remove_invalid_enum::( + profile, + "web_search", + &format!("profiles.{name}.web_search"), + source, + warnings, + ); + if let Some(windows) = profile.get_mut("windows").and_then(TomlValue::as_table_mut) { + remove_invalid_enum::( + windows, + "sandbox", + &format!("profiles.{name}.windows.sandbox"), + source, + warnings, + ); + } + } +} + +fn sanitize_permissions( + table: &mut TomlMap, + source: &str, + warnings: &mut Vec, +) { + let Some(permissions) = table + .get_mut("permissions") + .and_then(TomlValue::as_table_mut) + else { + return; + }; + for (profile_name, profile) in permissions { + let Some(network) = profile + .as_table_mut() + .and_then(|profile| profile.get_mut("network")) + .and_then(TomlValue::as_table_mut) + else { + continue; + }; + if let Some(domains) = network.get_mut("domains").and_then(TomlValue::as_table_mut) { + remove_invalid_map_enums::( + domains, + &format!("permissions.{profile_name}.network.domains"), + source, + warnings, + ); + } + if let Some(unix_sockets) = network + .get_mut("unix_sockets") + .and_then(TomlValue::as_table_mut) + { + remove_invalid_map_enums::( + unix_sockets, + &format!("permissions.{profile_name}.network.unix_sockets"), + source, + warnings, + ); + } + } +} + +fn sanitize_mcp_servers( + table: &mut TomlMap, + source: &str, + warnings: &mut Vec, +) { + let Some(mcp_servers) = table + .get_mut("mcp_servers") + .and_then(TomlValue::as_table_mut) + else { + return; + }; + for (server_name, server) in mcp_servers { + let Some(server) = server.as_table_mut() else { + continue; + }; + remove_invalid_enum::( + server, + "default_tools_approval_mode", + &format!("mcp_servers.{server_name}.default_tools_approval_mode"), + source, + warnings, + ); + if let Some(tools) = server.get_mut("tools").and_then(TomlValue::as_table_mut) { + for (tool_name, tool) in tools { + if let Some(tool) = tool.as_table_mut() { + remove_invalid_enum::( + tool, + "approval_mode", + &format!("mcp_servers.{server_name}.tools.{tool_name}.approval_mode"), + source, + warnings, + ); + } + } + } + } +} + +fn sanitize_plugins( + table: &mut TomlMap, + source: &str, + warnings: &mut Vec, +) { + let Some(plugins) = table.get_mut("plugins").and_then(TomlValue::as_table_mut) else { + return; + }; + for (plugin_name, plugin) in plugins { + let Some(mcp_servers) = plugin + .as_table_mut() + .and_then(|plugin| plugin.get_mut("mcp_servers")) + .and_then(TomlValue::as_table_mut) + else { + continue; + }; + for (server_name, server) in mcp_servers { + let Some(server) = server.as_table_mut() else { + continue; + }; + remove_invalid_enum::( + server, + "default_tools_approval_mode", + &format!( + "plugins.{plugin_name}.mcp_servers.{server_name}.default_tools_approval_mode" + ), + source, + warnings, + ); + if let Some(tools) = server.get_mut("tools").and_then(TomlValue::as_table_mut) { + for (tool_name, tool) in tools { + if let Some(tool) = tool.as_table_mut() { + remove_invalid_enum::( + tool, + "approval_mode", + &format!( + "plugins.{plugin_name}.mcp_servers.{server_name}.tools.{tool_name}.approval_mode" + ), + source, + warnings, + ); + } + } + } + } + } +} + +fn sanitize_otel(table: &mut TomlMap, source: &str, warnings: &mut Vec) { + let Some(otel) = table.get_mut("otel").and_then(TomlValue::as_table_mut) else { + return; + }; + remove_invalid_enum::(otel, "protocol", "otel.protocol", source, warnings); + remove_invalid_enum::(otel, "exporter", "otel.exporter", source, warnings); + remove_invalid_enum::( + otel, + "trace_exporter", + "otel.trace_exporter", + source, + warnings, + ); + remove_invalid_enum::( + otel, + "metrics_exporter", + "otel.metrics_exporter", + source, + warnings, + ); +} + +fn remove_invalid_map_enums( + table: &mut TomlMap, + path: &str, + source: &str, + warnings: &mut Vec, +) where + T: DeserializeOwned, +{ + let invalid_keys = table + .iter() + .filter_map(|(key, value)| invalid_enum::(value).then_some(key.clone())) + .collect::>(); + for key in invalid_keys { + let Some(value) = table.remove(&key) else { + continue; + }; + warnings.push(invalid_enum_warning( + source, + &format!("{path}.{key}"), + &value, + )); + } +} + +fn remove_invalid_enum( + table: &mut TomlMap, + key: &str, + path: &str, + source: &str, + warnings: &mut Vec, +) where + T: DeserializeOwned, +{ + let Some(value) = table.get(key) else { + return; + }; + if !invalid_enum::(value) { + return; + } + let Some(value) = table.remove(key) else { + return; + }; + warnings.push(invalid_enum_warning(source, path, &value)); +} + +fn invalid_enum(value: &TomlValue) -> bool +where + T: DeserializeOwned, +{ + match value.clone().try_into::>() { + Ok(Lenient::Success(_)) => false, + Ok(Lenient::Failure) | Err(_) => true, + } +} + +fn invalid_enum_warning(source: &str, path: &str, value: &TomlValue) -> String { + format!("Ignoring invalid config value in {source} at {path}: {value}") +} + +#[derive(serde::Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +enum ThreadStoreProbe { + Local {}, + Remote {}, + InMemory {}, +} diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index e88c736db0..7cc97a7063 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -7,6 +7,7 @@ mod fingerprint; mod hook_config; mod host_name; mod key_aliases; +mod lenient; pub mod loader; mod marketplace_edit; mod mcp_edit; diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index f5f8ec44e5..b0a6e5a102 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -15,6 +15,7 @@ use crate::diagnostics::ConfigError; use crate::diagnostics::config_error_from_toml; use crate::diagnostics::first_layer_config_error_from_entries as typed_first_layer_config_error_from_entries; use crate::diagnostics::io_error_from_config_error; +use crate::lenient::sanitize_config_toml_enums; use crate::merge::merge_toml_values; use crate::overrides::build_cli_overrides_layer; use crate::project_root_markers::default_project_root_markers; @@ -173,6 +174,8 @@ pub async fn load_config_layers_state( )?) }; + let mut startup_warnings = Vec::new(); + // Include an entry for the "system" config folder, loading its config.toml, // if it exists. let system_config_toml_file = system_config_toml_file_with_overrides(&overrides)?; @@ -186,7 +189,12 @@ pub async fn load_config_layers_state( ) }) .await?; - layers.push(system_layer); + push_config_layer_with_enum_warnings( + system_layer, + &mut layers, + &mut startup_warnings, + CONFIG_TOML_FILE, + ); // Add a layer for $CODEX_HOME/config.toml so folder-derived resources such // as rules/ can still be discovered. When user config is ignored, preserve @@ -210,9 +218,14 @@ pub async fn load_config_layers_state( }) .await? }; - layers.push(user_layer); + push_config_layer_with_enum_warnings( + user_layer, + &mut layers, + &mut startup_warnings, + CONFIG_TOML_FILE, + ); - let mut startup_warnings = None; + let checked_stack_warnings = cwd.is_some(); if let Some(cwd) = cwd { let mut merged_so_far = TomlValue::Table(toml::map::Map::new()); for layer in &layers { @@ -270,7 +283,7 @@ pub async fn load_config_layers_state( ) .await?; layers.extend(project_layers.layers); - startup_warnings = Some(project_layers.startup_warnings); + startup_warnings.extend(project_layers.startup_warnings); } // Add a layer for runtime overrides from the CLI or UI, if any exist. @@ -306,10 +319,16 @@ pub async fn load_config_layers_state( })?; let managed_config = resolve_relative_paths_in_config_toml(config.managed_config, managed_parent)?; - layers.push(ConfigLayerEntry::new( + let managed_layer = ConfigLayerEntry::new( ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: config.file }, managed_config, - )); + ); + push_config_layer_with_enum_warnings( + managed_layer, + &mut layers, + &mut startup_warnings, + CONFIG_TOML_FILE, + ); } if let Some(config) = managed_config_from_mdm { // As a general rule, config from MDM should _not_ include relative @@ -319,11 +338,17 @@ pub async fn load_config_layers_state( // value for base_dir, so codex_home is as good a value as any. let managed_config = resolve_relative_paths_in_config_toml(config.managed_config, codex_home)?; - layers.push(ConfigLayerEntry::new_with_raw_toml( + let managed_layer = ConfigLayerEntry::new_with_raw_toml( ConfigLayerSource::LegacyManagedConfigTomlFromMdm, managed_config, config.raw_toml, - )); + ); + push_config_layer_with_enum_warnings( + managed_layer, + &mut layers, + &mut startup_warnings, + CONFIG_TOML_FILE, + ); } let config_layer_stack = ConfigLayerStack::new( @@ -332,12 +357,48 @@ pub async fn load_config_layers_state( config_requirements_toml.into_toml(), )? .with_user_and_project_exec_policy_rules_ignored(ignore_user_and_project_exec_policy_rules); - Ok(match startup_warnings { - Some(startup_warnings) => config_layer_stack.with_startup_warnings(startup_warnings), - None => config_layer_stack, + Ok(if checked_stack_warnings || !startup_warnings.is_empty() { + config_layer_stack.with_startup_warnings(startup_warnings) + } else { + config_layer_stack }) } +fn push_config_layer_with_enum_warnings( + mut layer: ConfigLayerEntry, + layers: &mut Vec, + startup_warnings: &mut Vec, + config_toml_file: &str, +) { + startup_warnings.extend(sanitize_config_toml_enums( + &mut layer.config, + config_warning_source(&layer.name, config_toml_file), + )); + layers.push(layer); +} + +fn config_warning_source(layer: &ConfigLayerSource, config_toml_file: &str) -> String { + match layer { + ConfigLayerSource::System { file } => { + format!("system config {}", file.as_path().display()) + } + ConfigLayerSource::User { file } => { + format!("user config {}", file.as_path().display()) + } + ConfigLayerSource::Project { dot_codex_folder } => format!( + "project config {}", + dot_codex_folder.as_path().join(config_toml_file).display() + ), + ConfigLayerSource::SessionFlags => "session config overrides".to_string(), + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file } => { + format!("managed config {}", file.as_path().display()) + } + ConfigLayerSource::Mdm { .. } | ConfigLayerSource::LegacyManagedConfigTomlFromMdm => { + "managed device config".to_string() + } + } +} + fn insert_layer_by_precedence(layers: &mut Vec, layer: ConfigLayerEntry) { match layers .iter() @@ -1024,6 +1085,12 @@ async fn load_project_layers( }; let mut config = config; let ignored_project_config_keys = sanitize_project_config(&mut config); + if disabled_reason.is_none() { + startup_warnings.extend(sanitize_config_toml_enums( + &mut config, + format!("project config {}", config_file.as_path().display()), + )); + } let config = resolve_relative_paths_in_config_toml(config, dot_codex_abs.as_path())?; if disabled_reason.is_none() && !ignored_project_config_keys.is_empty() { diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index 6fcd5f872d..8e7a2ed382 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -118,6 +118,61 @@ async fn returns_config_error_for_invalid_user_config_toml() { assert_eq!(config_error, &expected_config_error); } +#[tokio::test] +async fn invalid_enum_values_emit_warnings_without_poisoning_config() -> std::io::Result<()> { + let tmp = tempdir().expect("tempdir"); + let contents = r#" +model = "gpt-5-codex" +sandbox_mode = "make-it-so" + +[tui] +notification_method = "loudly" +"#; + let config_path = tmp.path().join(CONFIG_TOML_FILE); + std::fs::write(&config_path, contents).expect("write config"); + + let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd"); + let layers = load_config_layers_state( + LOCAL_FS.as_ref(), + tmp.path(), + Some(cwd), + &[] as &[(String, TomlValue)], + LoaderOverrides::default(), + CloudRequirementsLoader::default(), + &codex_config::NoopThreadConfigLoader, + ) + .await?; + + let effective_config = layers.effective_config(); + let expected_config = toml::from_str::( + r#" +model = "gpt-5-codex" + +[tui] +"#, + ) + .expect("expected config should parse"); + let expected_startup_warnings = vec![ + format!( + "Ignoring invalid config value in user config {} at sandbox_mode: \"make-it-so\"", + config_path.display() + ), + format!( + "Ignoring invalid config value in user config {} at tui.notification_method: \"loudly\"", + config_path.display() + ), + ]; + + assert_eq!( + ( + effective_config, + layers.startup_warnings().unwrap_or_default().to_vec() + ), + (expected_config, expected_startup_warnings) + ); + Ok(()) +} + #[tokio::test] async fn ignore_user_config_keeps_empty_user_layer() -> std::io::Result<()> { let tmp = tempdir().expect("tempdir");