Warn on invalid config enum values

Allow config loading to continue when enum-valued settings contain invalid values. Invalid enum entries are removed from the layer before merging and surfaced through startup config warnings, while unrelated valid settings keep loading normally.

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Ahmed Ibrahim
2026-05-05 03:01:34 +03:00
parent 4950e7d8a6
commit 5e1dbff17e
4 changed files with 697 additions and 11 deletions

View File

@@ -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<T> {
Success(T),
Failure,
}
pub(crate) fn sanitize_config_toml_enums(
value: &mut TomlValue,
source: impl Into<String>,
) -> Vec<String> {
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<String, TomlValue>,
source: &str,
warnings: &mut Vec<String>,
) {
remove_invalid_enum::<AskForApproval>(
table,
"approval_policy",
"approval_policy",
source,
warnings,
);
remove_invalid_enum::<ApprovalsReviewer>(
table,
"approvals_reviewer",
"approvals_reviewer",
source,
warnings,
);
remove_invalid_enum::<SandboxMode>(table, "sandbox_mode", "sandbox_mode", source, warnings);
remove_invalid_enum::<ReasoningEffort>(
table,
"model_reasoning_effort",
"model_reasoning_effort",
source,
warnings,
);
remove_invalid_enum::<ReasoningEffort>(
table,
"plan_mode_reasoning_effort",
"plan_mode_reasoning_effort",
source,
warnings,
);
remove_invalid_enum::<ReasoningSummary>(
table,
"model_reasoning_summary",
"model_reasoning_summary",
source,
warnings,
);
remove_invalid_enum::<Verbosity>(
table,
"model_verbosity",
"model_verbosity",
source,
warnings,
);
remove_invalid_enum::<Personality>(table, "personality", "personality", source, warnings);
remove_invalid_enum::<ServiceTier>(table, "service_tier", "service_tier", source, warnings);
remove_invalid_enum::<ForcedLoginMethod>(
table,
"forced_login_method",
"forced_login_method",
source,
warnings,
);
remove_invalid_enum::<AuthCredentialsStoreMode>(
table,
"cli_auth_credentials_store",
"cli_auth_credentials_store",
source,
warnings,
);
remove_invalid_enum::<OAuthCredentialsStoreMode>(
table,
"mcp_oauth_credentials_store",
"mcp_oauth_credentials_store",
source,
warnings,
);
remove_invalid_enum::<UriBasedFileOpener>(
table,
"file_opener",
"file_opener",
source,
warnings,
);
remove_invalid_enum::<ThreadStoreProbe>(
table,
"experimental_thread_store",
"experimental_thread_store",
source,
warnings,
);
remove_invalid_enum::<WebSearchMode>(table, "web_search", "web_search", source, warnings);
if let Some(tui) = table.get_mut("tui").and_then(TomlValue::as_table_mut) {
remove_invalid_enum::<NotificationMethod>(
tui,
"notification_method",
"tui.notification_method",
source,
warnings,
);
remove_invalid_enum::<NotificationCondition>(
tui,
"notification_condition",
"tui.notification_condition",
source,
warnings,
);
remove_invalid_enum::<AltScreenMode>(
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::<ShellEnvironmentPolicyInherit>(
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::<WindowsSandboxModeToml>(
windows,
"sandbox",
"windows.sandbox",
source,
warnings,
);
}
if let Some(realtime) = table.get_mut("realtime").and_then(TomlValue::as_table_mut) {
remove_invalid_enum::<RealtimeWsVersion>(
realtime,
"version",
"realtime.version",
source,
warnings,
);
remove_invalid_enum::<RealtimeWsMode>(realtime, "type", "realtime.type", source, warnings);
remove_invalid_enum::<RealtimeTransport>(
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::<WebSearchContextSize>(
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::<WebSearchUserLocationType>(
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::<TrustLevel>(
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<String, TomlValue>,
source: &str,
warnings: &mut Vec<String>,
) {
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::<ServiceTier>(
profile,
"service_tier",
&format!("profiles.{name}.service_tier"),
source,
warnings,
);
remove_invalid_enum::<AskForApproval>(
profile,
"approval_policy",
&format!("profiles.{name}.approval_policy"),
source,
warnings,
);
remove_invalid_enum::<ApprovalsReviewer>(
profile,
"approvals_reviewer",
&format!("profiles.{name}.approvals_reviewer"),
source,
warnings,
);
remove_invalid_enum::<SandboxMode>(
profile,
"sandbox_mode",
&format!("profiles.{name}.sandbox_mode"),
source,
warnings,
);
remove_invalid_enum::<ReasoningEffort>(
profile,
"model_reasoning_effort",
&format!("profiles.{name}.model_reasoning_effort"),
source,
warnings,
);
remove_invalid_enum::<ReasoningEffort>(
profile,
"plan_mode_reasoning_effort",
&format!("profiles.{name}.plan_mode_reasoning_effort"),
source,
warnings,
);
remove_invalid_enum::<ReasoningSummary>(
profile,
"model_reasoning_summary",
&format!("profiles.{name}.model_reasoning_summary"),
source,
warnings,
);
remove_invalid_enum::<Verbosity>(
profile,
"model_verbosity",
&format!("profiles.{name}.model_verbosity"),
source,
warnings,
);
remove_invalid_enum::<Personality>(
profile,
"personality",
&format!("profiles.{name}.personality"),
source,
warnings,
);
remove_invalid_enum::<WebSearchMode>(
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::<WindowsSandboxModeToml>(
windows,
"sandbox",
&format!("profiles.{name}.windows.sandbox"),
source,
warnings,
);
}
}
}
fn sanitize_permissions(
table: &mut TomlMap<String, TomlValue>,
source: &str,
warnings: &mut Vec<String>,
) {
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::<NetworkDomainPermissionToml>(
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::<NetworkUnixSocketPermissionToml>(
unix_sockets,
&format!("permissions.{profile_name}.network.unix_sockets"),
source,
warnings,
);
}
}
}
fn sanitize_mcp_servers(
table: &mut TomlMap<String, TomlValue>,
source: &str,
warnings: &mut Vec<String>,
) {
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::<AppToolApproval>(
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::<AppToolApproval>(
tool,
"approval_mode",
&format!("mcp_servers.{server_name}.tools.{tool_name}.approval_mode"),
source,
warnings,
);
}
}
}
}
}
fn sanitize_plugins(
table: &mut TomlMap<String, TomlValue>,
source: &str,
warnings: &mut Vec<String>,
) {
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::<AppToolApproval>(
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::<AppToolApproval>(
tool,
"approval_mode",
&format!(
"plugins.{plugin_name}.mcp_servers.{server_name}.tools.{tool_name}.approval_mode"
),
source,
warnings,
);
}
}
}
}
}
}
fn sanitize_otel(table: &mut TomlMap<String, TomlValue>, source: &str, warnings: &mut Vec<String>) {
let Some(otel) = table.get_mut("otel").and_then(TomlValue::as_table_mut) else {
return;
};
remove_invalid_enum::<OtelHttpProtocol>(otel, "protocol", "otel.protocol", source, warnings);
remove_invalid_enum::<OtelExporterKind>(otel, "exporter", "otel.exporter", source, warnings);
remove_invalid_enum::<OtelExporterKind>(
otel,
"trace_exporter",
"otel.trace_exporter",
source,
warnings,
);
remove_invalid_enum::<OtelExporterKind>(
otel,
"metrics_exporter",
"otel.metrics_exporter",
source,
warnings,
);
}
fn remove_invalid_map_enums<T>(
table: &mut TomlMap<String, TomlValue>,
path: &str,
source: &str,
warnings: &mut Vec<String>,
) where
T: DeserializeOwned,
{
let invalid_keys = table
.iter()
.filter_map(|(key, value)| invalid_enum::<T>(value).then_some(key.clone()))
.collect::<Vec<_>>();
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<T>(
table: &mut TomlMap<String, TomlValue>,
key: &str,
path: &str,
source: &str,
warnings: &mut Vec<String>,
) where
T: DeserializeOwned,
{
let Some(value) = table.get(key) else {
return;
};
if !invalid_enum::<T>(value) {
return;
}
let Some(value) = table.remove(key) else {
return;
};
warnings.push(invalid_enum_warning(source, path, &value));
}
fn invalid_enum<T>(value: &TomlValue) -> bool
where
T: DeserializeOwned,
{
match value.clone().try_into::<Lenient<T>>() {
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 {},
}

View File

@@ -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;

View File

@@ -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<ConfigLayerEntry>,
startup_warnings: &mut Vec<String>,
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<ConfigLayerEntry>, 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() {

View File

@@ -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::<TomlValue>(
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");