From 49efb68ba2639a232b6b992d509d30e909a39612 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 30 Apr 2026 21:28:07 -0700 Subject: [PATCH] config: add strict config parsing --- MODULE.bazel.lock | 1 + codex-rs/Cargo.lock | 19 +++- codex-rs/Cargo.toml | 1 + codex-rs/app-server-test-client/src/lib.rs | 1 + codex-rs/app-server/src/lib.rs | 3 +- codex-rs/chatgpt/src/apply_command.rs | 7 +- codex-rs/cli/src/debug_sandbox.rs | 28 +++++- codex-rs/cli/src/login.rs | 8 +- codex-rs/cli/src/main.rs | 87 +++++++++++++--- codex-rs/cli/src/marketplace_cmd.rs | 11 ++- codex-rs/cli/src/mcp_cmd.rs | 49 ++++------ codex-rs/config/Cargo.toml | 1 + codex-rs/config/src/diagnostics.rs | 98 +++++++++++++++++++ codex-rs/config/src/lib.rs | 1 + codex-rs/config/src/loader/layer_io.rs | 69 ++++++++++--- codex-rs/config/src/loader/macos.rs | 52 +++++++++- codex-rs/config/src/loader/mod.rs | 67 +++++++++++-- codex-rs/config/src/state.rs | 2 + .../core/src/config/config_loader_tests.rs | 44 +++++++++ codex-rs/core/src/config/mod.rs | 13 ++- codex-rs/exec/src/lib.rs | 1 + codex-rs/mcp-server/src/lib.rs | 15 ++- codex-rs/tui/src/lib.rs | 17 +++- codex-rs/tui/src/main.rs | 7 +- codex-rs/utils/cli/src/config_override.rs | 10 ++ 25 files changed, 521 insertions(+), 91 deletions(-) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index e079e3af0e..3a47131120 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -1482,6 +1482,7 @@ "serde_derive_1.0.228": "{\"dependencies\":[{\"default_features\":false,\"features\":[\"proc-macro\"],\"name\":\"proc-macro2\",\"req\":\"^1.0.74\"},{\"default_features\":false,\"features\":[\"proc-macro\"],\"name\":\"quote\",\"req\":\"^1.0.35\"},{\"kind\":\"dev\",\"name\":\"serde\",\"req\":\"^1\"},{\"default_features\":false,\"features\":[\"clone-impls\",\"derive\",\"parsing\",\"printing\",\"proc-macro\"],\"name\":\"syn\",\"req\":\"^2.0.81\"}],\"features\":{\"default\":[],\"deserialize_in_place\":[]}}", "serde_derive_internals_0.29.1": "{\"dependencies\":[{\"default_features\":false,\"name\":\"proc-macro2\",\"req\":\"^1.0.74\"},{\"default_features\":false,\"name\":\"quote\",\"req\":\"^1.0.35\"},{\"default_features\":false,\"features\":[\"clone-impls\",\"derive\",\"parsing\",\"printing\"],\"name\":\"syn\",\"req\":\"^2.0.46\"}],\"features\":{}}", "serde_html_form_0.3.2": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"assert_matches2\",\"req\":\"^0.1.0\"},{\"kind\":\"dev\",\"name\":\"divan\",\"req\":\"^0.1.11\"},{\"default_features\":false,\"features\":[\"alloc\"],\"name\":\"form_urlencoded\",\"req\":\"^1.0.1\"},{\"default_features\":false,\"name\":\"indexmap\",\"req\":\"^2.0.0\"},{\"kind\":\"dev\",\"name\":\"insta\",\"req\":\"^1.45.0\"},{\"name\":\"itoa\",\"req\":\"^1.0.1\"},{\"name\":\"ryu\",\"optional\":true,\"req\":\"^1.0.9\"},{\"features\":[\"derive\"],\"kind\":\"dev\",\"name\":\"serde\",\"req\":\"^1.0.221\"},{\"default_features\":false,\"features\":[\"alloc\"],\"name\":\"serde_core\",\"req\":\"^1.0.221\"},{\"kind\":\"dev\",\"name\":\"serde_urlencoded\",\"req\":\"^0.7.1\"}],\"features\":{\"default\":[\"ryu\",\"std\"],\"std\":[]}}", + "serde_ignored_0.1.14": "{\"dependencies\":[{\"default_features\":false,\"name\":\"serde\",\"req\":\"^1.0.220\",\"target\":\"cfg(any())\"},{\"kind\":\"dev\",\"name\":\"serde\",\"req\":\"^1.0.220\"},{\"default_features\":false,\"features\":[\"alloc\"],\"name\":\"serde_core\",\"req\":\"^1.0.220\"},{\"kind\":\"dev\",\"name\":\"serde_derive\",\"req\":\"^1.0.220\"},{\"kind\":\"dev\",\"name\":\"serde_json\",\"req\":\"^1.0.110\"}],\"features\":{}}", "serde_json_1.0.149": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"automod\",\"req\":\"^1.0.11\"},{\"name\":\"indexmap\",\"optional\":true,\"req\":\"^2.2.3\"},{\"kind\":\"dev\",\"name\":\"indoc\",\"req\":\"^2.0.2\"},{\"name\":\"itoa\",\"req\":\"^1.0\"},{\"default_features\":false,\"name\":\"memchr\",\"req\":\"^2\"},{\"kind\":\"dev\",\"name\":\"ref-cast\",\"req\":\"^1.0.18\"},{\"kind\":\"dev\",\"name\":\"rustversion\",\"req\":\"^1.0.13\"},{\"default_features\":false,\"name\":\"serde\",\"req\":\"^1.0.220\",\"target\":\"cfg(any())\"},{\"features\":[\"derive\"],\"kind\":\"dev\",\"name\":\"serde\",\"req\":\"^1.0.194\"},{\"kind\":\"dev\",\"name\":\"serde_bytes\",\"req\":\"^0.11.10\"},{\"default_features\":false,\"name\":\"serde_core\",\"req\":\"^1.0.220\"},{\"kind\":\"dev\",\"name\":\"serde_derive\",\"req\":\"^1.0.166\"},{\"kind\":\"dev\",\"name\":\"serde_stacker\",\"req\":\"^0.1.8\"},{\"features\":[\"diff\"],\"kind\":\"dev\",\"name\":\"trybuild\",\"req\":\"^1.0.108\"},{\"name\":\"zmij\",\"req\":\"^1.0\"}],\"features\":{\"alloc\":[\"serde_core/alloc\"],\"arbitrary_precision\":[],\"default\":[\"std\"],\"float_roundtrip\":[],\"preserve_order\":[\"indexmap\",\"std\"],\"raw_value\":[],\"std\":[\"memchr/std\",\"serde_core/std\"],\"unbounded_depth\":[]}}", "serde_path_to_error_0.1.20": "{\"dependencies\":[{\"name\":\"itoa\",\"req\":\"^1.0\"},{\"default_features\":false,\"name\":\"serde\",\"req\":\"^1.0.220\",\"target\":\"cfg(any())\"},{\"kind\":\"dev\",\"name\":\"serde\",\"req\":\"^1.0.220\"},{\"default_features\":false,\"features\":[\"alloc\"],\"name\":\"serde_core\",\"req\":\"^1.0.220\"},{\"kind\":\"dev\",\"name\":\"serde_derive\",\"req\":\"^1.0.220\"},{\"kind\":\"dev\",\"name\":\"serde_json\",\"req\":\"^1.0.100\"}],\"features\":{}}", "serde_repr_0.1.20": "{\"dependencies\":[{\"name\":\"proc-macro2\",\"req\":\"^1.0.74\"},{\"name\":\"quote\",\"req\":\"^1.0.35\"},{\"kind\":\"dev\",\"name\":\"rustversion\",\"req\":\"^1.0.13\"},{\"kind\":\"dev\",\"name\":\"serde\",\"req\":\"^1.0.166\"},{\"kind\":\"dev\",\"name\":\"serde_json\",\"req\":\"^1.0.100\"},{\"name\":\"syn\",\"req\":\"^2.0.46\"},{\"features\":[\"diff\"],\"kind\":\"dev\",\"name\":\"trybuild\",\"req\":\"^1.0.81\"}],\"features\":{}}", diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 056bae4062..2d27e081cf 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2333,6 +2333,7 @@ dependencies = [ "prost 0.14.3", "schemars 0.8.22", "serde", + "serde_ignored", "serde_json", "serde_path_to_error", "sha2", @@ -5272,7 +5273,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -8820,7 +8821,7 @@ version = "5.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "51e219e79014df21a225b1860a479e2dcd7cbd9130f4defd4bd0e191ea31d67d" dependencies = [ - "base64 0.21.7", + "base64 0.22.1", "chrono", "getrandom 0.2.17", "http 1.4.0", @@ -9288,7 +9289,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7d8fae84b431384b68627d0f9b3b1245fcf9f46f6c0e3dc902e9dce64edd1967" dependencies = [ "libc", - "windows-sys 0.45.0", + "windows-sys 0.61.2", ] [[package]] @@ -11459,6 +11460,16 @@ dependencies = [ "serde_core", ] +[[package]] +name = "serde_ignored" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "115dffd5f3853e06e746965a20dcbae6ee747ae30b543d91b0e089668bb07798" +dependencies = [ + "serde", + "serde_core", +] + [[package]] name = "serde_json" version = "1.0.149" @@ -13932,7 +13943,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 79d932c8be..b95dba9ee1 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -336,6 +336,7 @@ seccompiler = "0.5.0" semver = "1.0" sentry = "0.46.0" serde = "1" +serde_ignored = "0.1.14" serde_json = "1" serde_path_to_error = "0.1.20" serde_with = "3.17" diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index edea431c61..5772a02b60 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -2125,6 +2125,7 @@ impl TestClientTracing { async fn initialize(config_overrides: &[String]) -> Result { let cli_kv_overrides = CliConfigOverrides { raw_overrides: config_overrides.to_vec(), + ..Default::default() } .parse_overrides() .map_err(|e| anyhow::anyhow!("error parsing -c overrides: {e}"))?; diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 4df869551e..61c572447a 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -413,13 +413,14 @@ pub async fn run_main_with_transport( pub async fn run_main_with_transport_options( arg0_paths: Arg0DispatchPaths, cli_config_overrides: CliConfigOverrides, - loader_overrides: LoaderOverrides, + mut loader_overrides: LoaderOverrides, default_analytics_enabled: bool, transport: AppServerTransport, session_source: SessionSource, auth: AppServerWebsocketAuthSettings, runtime_options: AppServerRuntimeOptions, ) -> IoResult<()> { + loader_overrides.strict_config |= cli_config_overrides.strict_config; let environment_manager = Arc::new( EnvironmentManager::new(EnvironmentManagerArgs::new( ExecServerRuntimePaths::from_optional_paths( diff --git a/codex-rs/chatgpt/src/apply_command.rs b/codex-rs/chatgpt/src/apply_command.rs index 70fe4481db..66f627cc6b 100644 --- a/codex-rs/chatgpt/src/apply_command.rs +++ b/codex-rs/chatgpt/src/apply_command.rs @@ -23,11 +23,16 @@ pub async fn run_apply_command( apply_cli: ApplyCommand, cwd: Option, ) -> anyhow::Result<()> { - let config = Config::load_with_cli_overrides( + let loader_overrides = codex_core::config::LoaderOverrides { + strict_config: apply_cli.config_overrides.strict_config, + ..Default::default() + }; + let config = Config::load_with_cli_overrides_and_loader_overrides( apply_cli .config_overrides .parse_overrides() .map_err(anyhow::Error::msg)?, + loader_overrides, ) .await?; diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index e9bc6a046e..2bcbed46b7 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -190,6 +190,7 @@ async fn run_command_under_sandbox( .map_err(anyhow::Error::msg)?, codex_linux_sandbox_exe, config_options, + config_overrides.strict_config, ) .await?; @@ -629,12 +630,14 @@ async fn load_debug_sandbox_config( cli_overrides: Vec<(String, TomlValue)>, codex_linux_sandbox_exe: Option, options: DebugSandboxConfigOptions, + strict_config: bool, ) -> anyhow::Result { load_debug_sandbox_config_with_codex_home( cli_overrides, codex_linux_sandbox_exe, options, /*codex_home*/ None, + strict_config, ) .await } @@ -644,6 +647,7 @@ async fn load_debug_sandbox_config_with_codex_home( codex_linux_sandbox_exe: Option, options: DebugSandboxConfigOptions, codex_home: Option, + strict_config: bool, ) -> anyhow::Result { let DebugSandboxConfigOptions { permissions_profile, @@ -673,6 +677,7 @@ async fn load_debug_sandbox_config_with_codex_home( }, codex_home.clone(), managed_requirements_mode, + strict_config, ) .await?; @@ -690,6 +695,7 @@ async fn load_debug_sandbox_config_with_codex_home( }, codex_home, managed_requirements_mode, + strict_config, ) .await .map_err(Into::into) @@ -700,13 +706,18 @@ async fn build_debug_sandbox_config( harness_overrides: ConfigOverrides, codex_home: Option, managed_requirements_mode: ManagedRequirementsMode, + strict_config: bool, ) -> std::io::Result { let mut builder = ConfigBuilder::default() .cli_overrides(cli_overrides) .harness_overrides(harness_overrides); - if let ManagedRequirementsMode::Ignore = managed_requirements_mode { + if strict_config || matches!(managed_requirements_mode, ManagedRequirementsMode::Ignore) { builder = builder.loader_overrides(LoaderOverrides { - ignore_managed_requirements: true, + strict_config, + ignore_managed_requirements: matches!( + managed_requirements_mode, + ManagedRequirementsMode::Ignore + ), ..Default::default() }); } @@ -776,6 +787,7 @@ mod tests { ConfigOverrides::default(), Some(codex_home_path.clone()), ManagedRequirementsMode::Include, + /*strict_config*/ false, ) .await?; let legacy_config = build_debug_sandbox_config( @@ -786,6 +798,7 @@ mod tests { }, Some(codex_home_path.clone()), ManagedRequirementsMode::Include, + /*strict_config*/ false, ) .await?; @@ -798,6 +811,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Include, }, Some(codex_home_path), + /*strict_config*/ false, ) .await?; @@ -833,6 +847,7 @@ mod tests { ConfigOverrides::default(), Some(codex_home_path.clone()), ManagedRequirementsMode::Include, + /*strict_config*/ false, ) .await?; let read_only_config = build_debug_sandbox_config( @@ -843,6 +858,7 @@ mod tests { }, Some(codex_home_path.clone()), ManagedRequirementsMode::Include, + /*strict_config*/ false, ) .await?; @@ -855,6 +871,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Include, }, Some(codex_home_path), + /*strict_config*/ false, ) .await?; @@ -898,6 +915,7 @@ mod tests { }, Some(codex_home_path.clone()), ManagedRequirementsMode::Include, + /*strict_config*/ false, ) .await?; @@ -910,6 +928,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Include, }, Some(codex_home_path), + /*strict_config*/ false, ) .await?; @@ -935,6 +954,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Ignore, }, Some(codex_home.path().to_path_buf()), + /*strict_config*/ false, ) .await?; @@ -968,6 +988,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Ignore, }, Some(codex_home.path().to_path_buf()), + /*strict_config*/ false, ) .await?; @@ -997,6 +1018,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Ignore, }, Some(codex_home.path().to_path_buf()), + /*strict_config*/ false, ) .await?; @@ -1008,6 +1030,7 @@ mod tests { ConfigOverrides::default(), Some(codex_home.path().to_path_buf()), ManagedRequirementsMode::Include, + /*strict_config*/ false, ) .await?; @@ -1033,6 +1056,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Ignore, }, Some(codex_home.path().to_path_buf()), + /*strict_config*/ false, ) .await?; diff --git a/codex-rs/cli/src/login.rs b/codex-rs/cli/src/login.rs index 1baa344b8d..19b7254d55 100644 --- a/codex-rs/cli/src/login.rs +++ b/codex-rs/cli/src/login.rs @@ -431,7 +431,13 @@ async fn load_config_or_exit(cli_config_overrides: CliConfigOverrides) -> Config } }; - match Config::load_with_cli_overrides(cli_overrides).await { + let loader_overrides = codex_config::LoaderOverrides { + strict_config: cli_config_overrides.strict_config, + ..Default::default() + }; + match Config::load_with_cli_overrides_and_loader_overrides(cli_overrides, loader_overrides) + .await + { Ok(config) => config, Err(e) => { eprintln!("Error loading configuration: {e}"); diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 7b6e7448d4..bc19489636 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -53,6 +53,7 @@ use crate::mcp_cmd::McpCli; use codex_core::build_models_manager; use codex_core::config::Config; +use codex_core::config::ConfigBuilder; use codex_core::config::ConfigOverrides; use codex_core::config::edit::ConfigEditsBuilder; use codex_core::config::find_codex_home; @@ -845,10 +846,11 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { None => { let transport = listen; let auth = auth.try_into_settings()?; + let loader_overrides = loader_overrides_from_cli(&root_config_overrides); codex_app_server::run_main_with_transport( arg0_paths.clone(), root_config_overrides, - codex_config::LoaderOverrides::default(), + loader_overrides, analytics_default_enabled, transport, codex_protocol::protocol::SessionSource::VSCode, @@ -1209,11 +1211,12 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { ..Default::default() }; - let config = Config::load_with_cli_overrides_and_harness_overrides( - cli_kv_overrides, - overrides, - ) - .await?; + let config = ConfigBuilder::default() + .cli_overrides(cli_kv_overrides) + .harness_overrides(overrides) + .loader_overrides(loader_overrides_from_cli(&root_config_overrides)) + .build() + .await?; let mut rows = Vec::with_capacity(FEATURES.len()); let mut name_width = 0; let mut stage_width = 0; @@ -1372,8 +1375,12 @@ async fn run_debug_prompt_input_command( additional_writable_roots: shared.add_dir, ..Default::default() }; - let config = - Config::load_with_cli_overrides_and_harness_overrides(cli_kv_overrides, overrides).await?; + let config = ConfigBuilder::default() + .cli_overrides(cli_kv_overrides) + .harness_overrides(overrides) + .loader_overrides(loader_overrides_from_cli(&root_config_overrides)) + .build() + .await?; let mut input = shared .images @@ -1404,7 +1411,11 @@ async fn run_debug_models_command( let cli_overrides = root_config_overrides .parse_overrides() .map_err(anyhow::Error::msg)?; - let config = Config::load_with_cli_overrides(cli_overrides).await?; + let config = Config::load_with_cli_overrides_and_loader_overrides( + cli_overrides, + loader_overrides_from_cli(&root_config_overrides), + ) + .await?; let auth_manager = AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ true).await; let models_manager = build_models_manager(&config, auth_manager); @@ -1429,8 +1440,12 @@ async fn run_debug_clear_memories_command( config_profile: interactive.config_profile.clone(), ..Default::default() }; - let config = - Config::load_with_cli_overrides_and_harness_overrides(cli_kv_overrides, overrides).await?; + let config = ConfigBuilder::default() + .cli_overrides(cli_kv_overrides) + .harness_overrides(overrides) + .loader_overrides(loader_overrides_from_cli(root_config_overrides)) + .build() + .await?; let state_path = state_db_path(config.sqlite_home.as_path()); let mut cleared_state_db = false; @@ -1465,11 +1480,21 @@ fn prepend_config_flags( subcommand_config_overrides: &mut CliConfigOverrides, cli_config_overrides: CliConfigOverrides, ) { + subcommand_config_overrides.strict_config |= cli_config_overrides.strict_config; subcommand_config_overrides .raw_overrides .splice(0..0, cli_config_overrides.raw_overrides); } +fn loader_overrides_from_cli( + config_overrides: &CliConfigOverrides, +) -> codex_config::LoaderOverrides { + codex_config::LoaderOverrides { + strict_config: config_overrides.strict_config, + ..Default::default() + } +} + fn reject_remote_mode_for_subcommand( remote: Option<&str>, remote_auth_token_env: Option<&str>, @@ -1569,10 +1594,11 @@ async fn run_interactive_tui( .map(read_remote_auth_token_from_env_var) .transpose() .map_err(std::io::Error::other)?; + let loader_overrides = loader_overrides_from_cli(&interactive.config_overrides); codex_tui::run_main( interactive, arg0_paths, - codex_config::LoaderOverrides::default(), + loader_overrides, normalized_remote, remote_auth_token, ) @@ -1672,6 +1698,7 @@ fn merge_interactive_cli_flags(interactive: &mut TuiCli, subcommand_cli: TuiCli) .config_overrides .raw_overrides .extend(config_overrides.raw_overrides); + interactive.config_overrides.strict_config |= config_overrides.strict_config; } fn print_completion(cmd: CompletionCommand) { @@ -2150,6 +2177,7 @@ mod tests { "my-profile", "-C", "/tmp", + "--strict-check", "-i", "/tmp/a.png,/tmp/b.png", ] @@ -2172,6 +2200,7 @@ mod tests { Some(std::path::Path::new("/tmp")) ); assert!(interactive.web_search); + assert!(interactive.config_overrides.strict_config); let has_a = interactive .images .iter() @@ -2605,4 +2634,38 @@ mod tests { .expect_err("feature should be rejected"); assert_eq!(err.to_string(), "Unknown feature flag: does_not_exist"); } + + #[test] + fn strict_check_with_unknown_enable_errors() { + let err = strict_check_feature_toggle_error(["--enable", "does_not_exist"].as_ref()); + assert_eq!(err.to_string(), "Unknown feature flag: does_not_exist"); + } + + #[test] + fn strict_check_with_unknown_disable_errors() { + let err = strict_check_feature_toggle_error(["--disable", "does_not_exist"].as_ref()); + assert_eq!(err.to_string(), "Unknown feature flag: does_not_exist"); + } + + #[test] + fn strict_check_with_compound_enable_errors() { + let err = strict_check_feature_toggle_error( + ["--enable", "multi_agent_v2.subagent_usage_hint_text"].as_ref(), + ); + assert_eq!( + err.to_string(), + "Unknown feature flag: multi_agent_v2.subagent_usage_hint_text" + ); + } + + fn strict_check_feature_toggle_error(args: &[&str]) -> anyhow::Error { + let cli_args = std::iter::once("codex") + .chain(std::iter::once("--strict-check")) + .chain(args.iter().copied()); + let cli = MultitoolCli::try_parse_from(cli_args).expect("parse should succeed"); + assert!(cli.config_overrides.strict_config); + cli.feature_toggles + .to_overrides() + .expect_err("feature should be rejected") + } } diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index fcd9049d59..747abb7169 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -70,10 +70,16 @@ impl MarketplaceCli { let overrides = config_overrides .parse_overrides() .map_err(anyhow::Error::msg)?; + let loader_overrides = codex_config::LoaderOverrides { + strict_config: config_overrides.strict_config, + ..Default::default() + }; match subcommand { MarketplaceSubcommand::Add(args) => run_add(args).await?, - MarketplaceSubcommand::Upgrade(args) => run_upgrade(overrides, args).await?, + MarketplaceSubcommand::Upgrade(args) => { + run_upgrade(overrides, loader_overrides, args).await? + } MarketplaceSubcommand::Remove(args) => run_remove(args).await?, } @@ -120,10 +126,11 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { async fn run_upgrade( overrides: Vec<(String, toml::Value)>, + loader_overrides: codex_config::LoaderOverrides, args: UpgradeMarketplaceArgs, ) -> Result<()> { let UpgradeMarketplaceArgs { marketplace_name } = args; - let config = Config::load_with_cli_overrides(overrides) + let config = Config::load_with_cli_overrides_and_loader_overrides(overrides, loader_overrides) .await .context("failed to load configuration")?; let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?; diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 858ef442ae..f31b455bf3 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -188,6 +188,19 @@ impl McpCli { } } +async fn load_config(config_overrides: &CliConfigOverrides) -> Result { + let overrides = config_overrides + .parse_overrides() + .map_err(anyhow::Error::msg)?; + let loader_overrides = codex_config::LoaderOverrides { + strict_config: config_overrides.strict_config, + ..Default::default() + }; + Config::load_with_cli_overrides_and_loader_overrides(overrides, loader_overrides) + .await + .context("failed to load configuration") +} + /// Preserve compatibility with servers that still expect the legacy empty-scope /// OAuth request. If a discovered-scope request is rejected by the provider, /// retry the login flow once without scopes. @@ -237,13 +250,7 @@ async fn perform_oauth_login_retry_without_scopes( } async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Result<()> { - // Validate any provided overrides even though they are not currently applied. - let overrides = config_overrides - .parse_overrides() - .map_err(anyhow::Error::msg)?; - let config = Config::load_with_cli_overrides(overrides) - .await - .context("failed to load configuration")?; + let config = load_config(config_overrides).await?; let AddArgs { name, @@ -388,12 +395,7 @@ async fn run_remove(config_overrides: &CliConfigOverrides, remove_args: RemoveAr } async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) -> Result<()> { - let overrides = config_overrides - .parse_overrides() - .map_err(anyhow::Error::msg)?; - let config = Config::load_with_cli_overrides(overrides) - .await - .context("failed to load configuration")?; + let config = load_config(config_overrides).await?; let mcp_manager = McpManager::new(Arc::new(PluginsManager::new( config.codex_home.to_path_buf(), ))); @@ -441,12 +443,7 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) } async fn run_logout(config_overrides: &CliConfigOverrides, logout_args: LogoutArgs) -> Result<()> { - let overrides = config_overrides - .parse_overrides() - .map_err(anyhow::Error::msg)?; - let config = Config::load_with_cli_overrides(overrides) - .await - .context("failed to load configuration")?; + let config = load_config(config_overrides).await?; let mcp_manager = McpManager::new(Arc::new(PluginsManager::new( config.codex_home.to_path_buf(), ))); @@ -473,12 +470,7 @@ async fn run_logout(config_overrides: &CliConfigOverrides, logout_args: LogoutAr } async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> Result<()> { - let overrides = config_overrides - .parse_overrides() - .map_err(anyhow::Error::msg)?; - let config = Config::load_with_cli_overrides(overrides) - .await - .context("failed to load configuration")?; + let config = load_config(config_overrides).await?; let mcp_manager = McpManager::new(Arc::new(PluginsManager::new( config.codex_home.to_path_buf(), ))); @@ -728,12 +720,7 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> } async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Result<()> { - let overrides = config_overrides - .parse_overrides() - .map_err(anyhow::Error::msg)?; - let config = Config::load_with_cli_overrides(overrides) - .await - .context("failed to load configuration")?; + let config = load_config(config_overrides).await?; let mcp_manager = McpManager::new(Arc::new(PluginsManager::new( config.codex_home.to_path_buf(), ))); diff --git a/codex-rs/config/Cargo.toml b/codex-rs/config/Cargo.toml index 8cef4070c9..f78ffb23ed 100644 --- a/codex-rs/config/Cargo.toml +++ b/codex-rs/config/Cargo.toml @@ -32,6 +32,7 @@ multimap = { workspace = true } prost = "0.14.3" schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } +serde_ignored = { workspace = true } serde_json = { workspace = true } serde_path_to_error = { workspace = true } sha2 = { workspace = true } diff --git a/codex-rs/config/src/diagnostics.rs b/codex-rs/config/src/diagnostics.rs index 899114d6d7..4efd24d795 100644 --- a/codex-rs/config/src/diagnostics.rs +++ b/codex-rs/config/src/diagnostics.rs @@ -134,6 +134,40 @@ pub fn config_error_from_typed_toml( } } +pub fn config_error_from_ignored_toml_fields( + path: impl AsRef, + contents: &str, +) -> Option { + let path = path.as_ref(); + if let Some(error) = config_error_from_typed_toml::(path, contents) { + return Some(error); + } + + let deserializer = toml::de::Deserializer::parse(contents).ok()?; + + let mut ignored_paths = Vec::new(); + let result: Result = serde_ignored::deserialize(deserializer, |ignored_path| { + let path_segments = ignored_path_segments(&ignored_path); + if !path_segments.is_empty() { + ignored_paths.push(path_segments); + } + }); + if let Err(err) = result { + return Some(config_error_from_toml(path, contents, err)); + } + + let path_segments = ignored_paths.into_iter().next()?; + let ignored_path = path_segments.join("."); + let range = span_for_ignored_path(contents, &path_segments) + .map(|span| text_range_from_span(contents, span)) + .unwrap_or_else(default_range); + Some(ConfigError::new( + path.to_path_buf(), + range, + format!("unknown configuration field `{ignored_path}`"), + )) +} + pub async fn first_layer_config_error( layers: &ConfigLayerStack, config_toml_file: &str, @@ -323,6 +357,70 @@ fn span_for_config_path(contents: &str, path: &SerdePath) -> Option Option> { + let doc = contents.parse::>().ok()?; + let mut node = TomlNode::Item(doc.as_item()); + for (index, segment) in path.iter().enumerate() { + if index + 1 == path.len() { + let key_span = match &node { + TomlNode::Item(item) => item + .as_table_like() + .and_then(|table| table.get_key_value(segment)) + .and_then(|(key, _)| key.span()), + TomlNode::Table(table) => { + table.get_key_value(segment).and_then(|(key, _)| key.span()) + } + TomlNode::Value(Value::InlineTable(table)) => { + table.get_key_value(segment).and_then(|(key, _)| key.span()) + } + _ => None, + }; + if key_span.is_some() { + return key_span; + } + } + + if let Some(next) = map_child(&node, segment) { + node = next; + continue; + } + + let index = segment.parse::().ok()?; + node = seq_child(&node, index)?; + } + + match node { + TomlNode::Item(item) => item.span(), + TomlNode::Table(table) => table.span(), + TomlNode::Value(value) => value.span(), + } +} + +fn ignored_path_segments(path: &serde_ignored::Path<'_>) -> Vec { + let mut segments = Vec::new(); + push_ignored_path_segments(path, &mut segments); + segments +} + +fn push_ignored_path_segments(path: &serde_ignored::Path<'_>, segments: &mut Vec) { + match path { + serde_ignored::Path::Root => {} + serde_ignored::Path::Seq { parent, index } => { + push_ignored_path_segments(parent, segments); + segments.push(index.to_string()); + } + serde_ignored::Path::Map { parent, key } => { + push_ignored_path_segments(parent, segments); + segments.push(key.clone()); + } + serde_ignored::Path::Some { parent } + | serde_ignored::Path::NewtypeStruct { parent } + | serde_ignored::Path::NewtypeVariant { parent } => { + push_ignored_path_segments(parent, segments); + } + } +} + fn is_features_table_path(path: &SerdePath) -> bool { let mut segments = path.iter(); matches!(segments.next(), Some(SerdeSegment::Map { key }) if key == "features") diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index e88c736db0..fbbe3bcea1 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -64,6 +64,7 @@ pub use diagnostics::ConfigError; pub use diagnostics::ConfigLoadError; pub use diagnostics::TextPosition; pub use diagnostics::TextRange; +pub use diagnostics::config_error_from_ignored_toml_fields; pub use diagnostics::config_error_from_toml; pub use diagnostics::config_error_from_typed_toml; pub use diagnostics::first_layer_config_error; diff --git a/codex-rs/config/src/loader/layer_io.rs b/codex-rs/config/src/loader/layer_io.rs index 9c15df7271..665fd31d21 100644 --- a/codex-rs/config/src/loader/layer_io.rs +++ b/codex-rs/config/src/loader/layer_io.rs @@ -2,11 +2,14 @@ use super::macos::ManagedAdminConfigLayer; #[cfg(target_os = "macos")] use super::macos::load_managed_admin_config_layer; +use crate::config_toml::ConfigToml; +use crate::diagnostics::config_error_from_ignored_toml_fields; use crate::diagnostics::config_error_from_toml; use crate::diagnostics::io_error_from_config_error; use crate::state::LoaderOverrides; use codex_file_system::ExecutorFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_absolute_path::AbsolutePathBufGuard; use std::io; use std::path::Path; use std::path::PathBuf; @@ -44,12 +47,14 @@ pub(super) async fn load_config_layers_internal( let LoaderOverrides { managed_config_path, managed_preferences_base64, + strict_config, .. } = overrides; #[cfg(not(target_os = "macos"))] let LoaderOverrides { managed_config_path, + strict_config, .. } = overrides; @@ -57,19 +62,26 @@ pub(super) async fn load_config_layers_internal( managed_config_path.unwrap_or_else(|| managed_config_default_path(codex_home)), )?; - let managed_config = - read_config_from_path(fs, &managed_config_path, /*log_missing_as_info*/ false) - .await? - .map(|managed_config| MangedConfigFromFile { - managed_config, - file: managed_config_path.clone(), - }); + let managed_config = read_config_from_path( + fs, + &managed_config_path, + /*log_missing_as_info*/ false, + strict_config, + ) + .await? + .map(|loaded| MangedConfigFromFile { + managed_config: loaded, + file: managed_config_path.clone(), + }); #[cfg(target_os = "macos")] - let managed_preferences = - load_managed_admin_config_layer(managed_preferences_base64.as_deref()) - .await? - .map(map_managed_admin_layer); + let managed_preferences = load_managed_admin_config_layer( + managed_preferences_base64.as_deref(), + strict_config, + codex_home, + ) + .await? + .map(map_managed_admin_layer); #[cfg(not(target_os = "macos"))] let managed_preferences = None; @@ -93,10 +105,14 @@ pub(super) async fn read_config_from_path( fs: &dyn ExecutorFileSystem, path: &AbsolutePathBuf, log_missing_as_info: bool, + strict_config: bool, ) -> io::Result> { match fs.read_file_text(path, /*sandbox*/ None).await { Ok(contents) => match toml::from_str::(&contents) { - Ok(value) => Ok(Some(value)), + Ok(value) => { + validate_config_toml_strictly_if_requested(strict_config, path, &contents)?; + Ok(Some(value)) + } Err(err) => { tracing::error!("Failed to parse {}: {err}", path.as_path().display()); let config_error = config_error_from_toml(path.as_path(), &contents, err.clone()); @@ -122,6 +138,35 @@ pub(super) async fn read_config_from_path( } } +fn validate_config_toml_strictly_if_requested( + strict_config: bool, + path: &AbsolutePathBuf, + contents: &str, +) -> io::Result<()> { + if !strict_config { + return Ok(()); + } + + let Some(base_dir) = path.as_path().parent() else { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("Config file {} has no parent directory", path.display()), + )); + }; + let _guard = AbsolutePathBufGuard::new(base_dir); + if let Some(config_error) = + config_error_from_ignored_toml_fields::(path.as_path(), contents) + { + return Err(io_error_from_config_error( + io::ErrorKind::InvalidData, + config_error, + /*source*/ None, + )); + } + + Ok(()) +} + /// Return the default managed config path. pub(super) fn managed_config_default_path(codex_home: &Path) -> PathBuf { #[cfg(unix)] diff --git a/codex-rs/config/src/loader/macos.rs b/codex-rs/config/src/loader/macos.rs index 3a9fc3a0ea..9143d6ed69 100644 --- a/codex-rs/config/src/loader/macos.rs +++ b/codex-rs/config/src/loader/macos.rs @@ -2,13 +2,19 @@ use super::merge_requirements_with_remote_sandbox_config; use crate::config_requirements::ConfigRequirementsToml; use crate::config_requirements::ConfigRequirementsWithSources; use crate::config_requirements::RequirementSource; +use crate::config_toml::ConfigToml; +use crate::diagnostics::config_error_from_ignored_toml_fields; +use crate::diagnostics::io_error_from_config_error; use base64::Engine; use base64::prelude::BASE64_STANDARD; +use codex_utils_absolute_path::AbsolutePathBufGuard; use core_foundation::base::TCFType; use core_foundation::string::CFString; use core_foundation::string::CFStringRef; use std::ffi::c_void; use std::io; +use std::path::Path; +use std::path::PathBuf; use tokio::task; use toml::Value as TomlValue; @@ -31,17 +37,20 @@ pub(super) fn managed_preferences_requirements_source() -> RequirementSource { pub(crate) async fn load_managed_admin_config_layer( override_base64: Option<&str>, + strict_config: bool, + base_dir: &Path, ) -> io::Result> { if let Some(encoded) = override_base64 { let trimmed = encoded.trim(); return if trimmed.is_empty() { Ok(None) } else { - parse_managed_config_base64(trimmed).map(Some) + parse_managed_config_base64(trimmed, strict_config, base_dir).map(Some) }; } - match task::spawn_blocking(load_managed_admin_config).await { + let base_dir = base_dir.to_path_buf(); + match task::spawn_blocking(move || load_managed_admin_config(strict_config, &base_dir)).await { Ok(result) => result, Err(join_err) => { if join_err.is_cancelled() { @@ -54,11 +63,14 @@ pub(crate) async fn load_managed_admin_config_layer( } } -fn load_managed_admin_config() -> io::Result> { +fn load_managed_admin_config( + strict_config: bool, + base_dir: &Path, +) -> io::Result> { load_managed_preference(MANAGED_PREFERENCES_CONFIG_KEY)? .as_deref() .map(str::trim) - .map(parse_managed_config_base64) + .map(|encoded| parse_managed_config_base64(encoded, strict_config, base_dir)) .transpose() } @@ -134,8 +146,13 @@ fn load_managed_preference(key_name: &str) -> io::Result> { Ok(Some(value)) } -fn parse_managed_config_base64(encoded: &str) -> io::Result { +fn parse_managed_config_base64( + encoded: &str, + strict_config: bool, + base_dir: &Path, +) -> io::Result { let raw_toml = decode_managed_preferences_base64(encoded)?; + validate_managed_config_toml_strictly_if_requested(strict_config, &raw_toml, base_dir)?; match toml::from_str::(&raw_toml) { Ok(TomlValue::Table(parsed)) => Ok(ManagedAdminConfigLayer { config: TomlValue::Table(parsed), @@ -155,6 +172,31 @@ fn parse_managed_config_base64(encoded: &str) -> io::Result io::Result<()> { + if !strict_config { + return Ok(()); + } + + let _guard = AbsolutePathBufGuard::new(base_dir); + let path = PathBuf::from(format!( + "{MANAGED_PREFERENCES_APPLICATION_ID}:{MANAGED_PREFERENCES_CONFIG_KEY}" + )); + if let Some(config_error) = config_error_from_ignored_toml_fields::(&path, raw_toml) + { + return Err(io_error_from_config_error( + io::ErrorKind::InvalidData, + config_error, + /*source*/ None, + )); + } + + Ok(()) +} + fn parse_managed_requirements_base64(encoded: &str) -> io::Result { toml::from_str::(&decode_managed_preferences_base64(encoded)?).map_err( |err| { diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index f5f8ec44e5..f0797e0e88 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -12,6 +12,7 @@ use crate::config_requirements::SandboxModeRequirement; use crate::config_toml::ConfigToml; use crate::config_toml::ProjectConfig; use crate::diagnostics::ConfigError; +use crate::diagnostics::config_error_from_ignored_toml_fields; 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; @@ -109,6 +110,7 @@ pub async fn load_config_layers_state( ) -> io::Result { let ignore_managed_requirements = overrides.ignore_managed_requirements; let ignore_user_config = overrides.ignore_user_config; + let strict_config = overrides.strict_config; let ignore_user_and_project_exec_policy_rules = overrides.ignore_user_and_project_exec_policy_rules; let mut config_requirements_toml = ConfigRequirementsWithSources::default(); @@ -176,16 +178,20 @@ 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 = system_config_toml_file_with_overrides(&overrides)?; - let system_layer = - load_config_toml_for_required_layer(fs, &system_config_toml_file, |config_toml| { + let system_layer = load_config_toml_for_required_layer( + fs, + &system_config_toml_file, + strict_config, + |config_toml| { ConfigLayerEntry::new( ConfigLayerSource::System { file: system_config_toml_file.clone(), }, config_toml, ) - }) - .await?; + }, + ) + .await?; layers.push(system_layer); // Add a layer for $CODEX_HOME/config.toml so folder-derived resources such @@ -200,7 +206,7 @@ pub async fn load_config_layers_state( TomlValue::Table(toml::map::Map::new()), ) } else { - load_config_toml_for_required_layer(fs, &user_file, |config_toml| { + load_config_toml_for_required_layer(fs, &user_file, strict_config, |config_toml| { ConfigLayerEntry::new( ConfigLayerSource::User { file: user_file.clone(), @@ -267,6 +273,7 @@ pub async fn load_config_layers_state( &project_trust_context.project_root, &project_trust_context, codex_home, + strict_config, ) .await?; layers.extend(project_layers.layers); @@ -358,15 +365,11 @@ fn insert_layer_by_precedence(layers: &mut Vec, layer: ConfigL async fn load_config_toml_for_required_layer( fs: &dyn ExecutorFileSystem, toml_file: &AbsolutePathBuf, + strict_config: bool, create_entry: impl FnOnce(TomlValue) -> ConfigLayerEntry, ) -> io::Result { let toml_value = match fs.read_file_text(toml_file, /*sandbox*/ None).await { Ok(contents) => { - let config: TomlValue = toml::from_str(&contents).map_err(|err| { - let config_error = - config_error_from_toml(toml_file.as_path(), &contents, err.clone()); - io_error_from_config_error(io::ErrorKind::InvalidData, config_error, Some(err)) - })?; let config_parent = toml_file.as_path().parent().ok_or_else(|| { io::Error::new( io::ErrorKind::InvalidData, @@ -376,6 +379,17 @@ async fn load_config_toml_for_required_layer( ), ) })?; + let config: TomlValue = toml::from_str(&contents).map_err(|err| { + let config_error = + config_error_from_toml(toml_file.as_path(), &contents, err.clone()); + io_error_from_config_error(io::ErrorKind::InvalidData, config_error, Some(err)) + })?; + validate_config_toml_strictly_if_requested( + strict_config, + toml_file.as_path(), + &contents, + config_parent, + )?; resolve_relative_paths_in_config_toml(config, config_parent) } Err(e) => { @@ -396,6 +410,30 @@ async fn load_config_toml_for_required_layer( Ok(create_entry(toml_value)) } +fn validate_config_toml_strictly_if_requested( + strict_config: bool, + toml_file: &Path, + contents: &str, + base_dir: &Path, +) -> io::Result<()> { + if !strict_config { + return Ok(()); + } + + let _guard = AbsolutePathBufGuard::new(base_dir); + if let Some(config_error) = + config_error_from_ignored_toml_fields::(toml_file, contents) + { + return Err(io_error_from_config_error( + io::ErrorKind::InvalidData, + config_error, + /*source*/ None, + )); + } + + Ok(()) +} + /// If available, apply requirements from the platform system /// `requirements.toml` location to `config_requirements_toml` by filling in /// any unset fields. @@ -960,6 +998,7 @@ async fn load_project_layers( project_root: &AbsolutePathBuf, trust_context: &ProjectTrustContext, codex_home: &Path, + strict_config: bool, ) -> io::Result { let codex_home_abs = AbsolutePathBuf::from_absolute_path(codex_home)?; let codex_home_normalized = @@ -1023,6 +1062,14 @@ async fn load_project_layers( } }; let mut config = config; + if disabled_reason.is_none() { + validate_config_toml_strictly_if_requested( + strict_config, + config_file.as_path(), + &contents, + dot_codex_abs.as_path(), + )?; + } let ignored_project_config_keys = sanitize_project_config(&mut config); let config = resolve_relative_paths_in_config_toml(config, dot_codex_abs.as_path())?; diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index fc5ec79971..d4bb84b9f4 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -20,6 +20,7 @@ pub struct LoaderOverrides { pub managed_config_path: Option, pub system_config_path: Option, pub system_requirements_path: Option, + pub strict_config: bool, pub ignore_managed_requirements: bool, pub ignore_user_config: bool, pub ignore_user_and_project_exec_policy_rules: bool, @@ -39,6 +40,7 @@ impl LoaderOverrides { managed_config_path: Some(base.join("managed_config.toml")), system_config_path: Some(base.join("config.toml")), system_requirements_path: Some(base.join("requirements.toml")), + strict_config: false, ignore_managed_requirements: false, ignore_user_config: false, ignore_user_and_project_exec_policy_rules: false, diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index 1f6e145cd1..d83b029411 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -18,6 +18,7 @@ use codex_config::RequirementSource; use codex_config::SessionThreadConfig; use codex_config::StaticThreadConfigLoader; use codex_config::ThreadConfigSource; +use codex_config::config_error_from_ignored_toml_fields; use codex_config::config_error_from_toml; use codex_config::config_toml::ConfigToml; use codex_config::config_toml::ProjectConfig; @@ -228,6 +229,49 @@ async fn returns_config_error_for_schema_error_in_user_config() { assert_eq!(config_error, &expected_config_error); } +#[tokio::test] +async fn strict_config_rejects_unknown_user_config_key() { + let tmp = tempdir().expect("tempdir"); + let contents = "model = \"gpt-5\"\nunknown_key = true"; + let config_path = tmp.path().join(CONFIG_TOML_FILE); + std::fs::write(&config_path, contents).expect("write config"); + + let err = ConfigBuilder::default() + .codex_home(tmp.path().to_path_buf()) + .fallback_cwd(Some(tmp.path().to_path_buf())) + .loader_overrides(LoaderOverrides { + strict_config: true, + ..LoaderOverrides::without_managed_config_for_tests() + }) + .build() + .await + .expect_err("expected error"); + + let config_error = config_error_from_io(&err); + let expected_config_error = + config_error_from_ignored_toml_fields::(&config_path, contents) + .expect("unknown field error"); + assert_eq!(config_error, &expected_config_error); +} + +#[test] +fn strict_config_points_to_unknown_nested_key() { + let tmp = tempdir().expect("tempdir"); + let contents = "[mcp_servers.local]\ncommand = \"echo\"\nunknown_key = true"; + let config_path = tmp.path().join(CONFIG_TOML_FILE); + std::fs::write(&config_path, contents).expect("write config"); + + let error = config_error_from_ignored_toml_fields::(&config_path, contents) + .expect("unknown field error"); + + assert_eq!( + error.message, + "unknown configuration field `mcp_servers.local.unknown_key`" + ); + assert_eq!(error.range.start.line, 3); + assert_eq!(error.range.start.column, 1); +} + #[test] fn schema_error_points_to_feature_value() { let tmp = tempdir().expect("tempdir"); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 29f158058b..688657e5d3 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -15,7 +15,6 @@ use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; use codex_config::ConstrainedWithSource; use codex_config::FeatureRequirementsToml; -use codex_config::LoaderOverrides; use codex_config::McpServerIdentity; use codex_config::McpServerRequirement; use codex_config::PluginRequirementsToml; @@ -129,6 +128,7 @@ mod schema; pub use codex_config::Constrained; pub use codex_config::ConstraintError; pub use codex_config::ConstraintResult; +pub use codex_config::LoaderOverrides; pub use codex_network_proxy::NetworkProxyAuditMetadata; use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile; pub use codex_sandboxing::system_bwrap_warning; @@ -1068,6 +1068,17 @@ impl Config { .await } + pub async fn load_with_cli_overrides_and_loader_overrides( + cli_overrides: Vec<(String, TomlValue)>, + loader_overrides: LoaderOverrides, + ) -> std::io::Result { + ConfigBuilder::default() + .cli_overrides(cli_overrides) + .loader_overrides(loader_overrides) + .build() + .await + } + /// Load a default configuration when user config files are invalid. pub async fn load_default_with_cli_overrides( cli_overrides: Vec<(String, TomlValue)>, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index f2f0ed030b..81ad6211d0 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -315,6 +315,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result #[allow(clippy::print_stderr)] let loader_overrides = LoaderOverrides { + strict_config: config_overrides.strict_config, ignore_user_config, ignore_user_and_project_exec_policy_rules: ignore_rules, ..Default::default() diff --git a/codex-rs/mcp-server/src/lib.rs b/codex-rs/mcp-server/src/lib.rs index bb54ffcc53..d2a20c783c 100644 --- a/codex-rs/mcp-server/src/lib.rs +++ b/codex-rs/mcp-server/src/lib.rs @@ -77,11 +77,16 @@ pub async fn run_main( format!("error parsing -c overrides: {e}"), ) })?; - let config = Config::load_with_cli_overrides(cli_kv_overrides) - .await - .map_err(|e| { - std::io::Error::new(ErrorKind::InvalidData, format!("error loading config: {e}")) - })?; + let loader_overrides = codex_config::LoaderOverrides { + strict_config: cli_config_overrides.strict_config, + ..Default::default() + }; + let config = + Config::load_with_cli_overrides_and_loader_overrides(cli_kv_overrides, loader_overrides) + .await + .map_err(|e| { + std::io::Error::new(ErrorKind::InvalidData, format!("error loading config: {e}")) + })?; set_default_client_residency_requirement(config.enforce_residency.value()); let otel = codex_core::otel_init::build_provider( diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 334e412c0a..b43fd6a4d8 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -8,7 +8,7 @@ use crate::legacy_core::config::Config; use crate::legacy_core::config::ConfigBuilder; use crate::legacy_core::config::ConfigOverrides; use crate::legacy_core::config::find_codex_home; -use crate::legacy_core::config::load_config_as_toml_with_cli_overrides; +use crate::legacy_core::config::load_config_as_toml_with_cli_and_loader_overrides; use crate::legacy_core::config::resolve_oss_provider; use crate::legacy_core::format_exec_policy_error_with_source; use crate::legacy_core::windows_sandbox::WindowsSandboxLevelExt; @@ -724,7 +724,10 @@ pub async fn run_main( // gpt-oss:20b) and ensure it is present locally. Also, force the built‑in let raw_overrides = cli.config_overrides.raw_overrides.clone(); // `oss` model provider. - let overrides_cli = codex_utils_cli::CliConfigOverrides { raw_overrides }; + let overrides_cli = codex_utils_cli::CliConfigOverrides { + raw_overrides, + ..Default::default() + }; let cli_kv_overrides = match overrides_cli.parse_overrides() { // Parse `-c` overrides from the CLI. Ok(v) => v, @@ -759,10 +762,11 @@ pub async fn run_main( config_cwd_for_app_server_target(cwd.as_deref(), &app_server_target, &environment_manager)?; #[allow(clippy::print_stderr)] - let config_toml = match load_config_as_toml_with_cli_overrides( + let config_toml = match load_config_as_toml_with_cli_and_loader_overrides( &codex_home, config_cwd.as_ref(), cli_kv_overrides.clone(), + loader_overrides.clone(), ) .await { @@ -866,6 +870,7 @@ pub async fn run_main( cli_kv_overrides.clone(), overrides.clone(), cloud_requirements.clone(), + loader_overrides.clone(), ) .await; @@ -1189,6 +1194,7 @@ async fn run_ratatui_app( cli_kv_overrides.clone(), overrides.clone(), cloud_requirements.clone(), + loader_overrides.clone(), ) .await } else { @@ -1390,6 +1396,7 @@ async fn run_ratatui_app( cli_kv_overrides.clone(), overrides.clone(), cloud_requirements.clone(), + loader_overrides.clone(), fallback_cwd, ) .await @@ -1584,11 +1591,13 @@ async fn load_config_or_exit( cli_kv_overrides: Vec<(String, toml::Value)>, overrides: ConfigOverrides, cloud_requirements: CloudRequirementsLoader, + loader_overrides: LoaderOverrides, ) -> Config { load_config_or_exit_with_fallback_cwd( cli_kv_overrides, overrides, cloud_requirements, + loader_overrides, /*fallback_cwd*/ None, ) .await @@ -1598,12 +1607,14 @@ async fn load_config_or_exit_with_fallback_cwd( cli_kv_overrides: Vec<(String, toml::Value)>, overrides: ConfigOverrides, cloud_requirements: CloudRequirementsLoader, + loader_overrides: LoaderOverrides, fallback_cwd: Option, ) -> Config { #[allow(clippy::print_stderr)] match ConfigBuilder::default() .cli_overrides(cli_kv_overrides) .harness_overrides(overrides) + .loader_overrides(loader_overrides) .cloud_requirements(cloud_requirements) .fallback_cwd(fallback_cwd) .build() diff --git a/codex-rs/tui/src/main.rs b/codex-rs/tui/src/main.rs index 8b66e5368c..a332231887 100644 --- a/codex-rs/tui/src/main.rs +++ b/codex-rs/tui/src/main.rs @@ -49,14 +49,19 @@ fn main() -> anyhow::Result<()> { arg0_dispatch_or_else(|arg0_paths: Arg0DispatchPaths| async move { let top_cli = TopCli::parse(); let mut inner = top_cli.inner; + inner.config_overrides.strict_config |= top_cli.config_overrides.strict_config; inner .config_overrides .raw_overrides .splice(0..0, top_cli.config_overrides.raw_overrides); + let loader_overrides = LoaderOverrides { + strict_config: inner.config_overrides.strict_config, + ..Default::default() + }; let exit_info = run_main( inner, arg0_paths, - LoaderOverrides::default(), + loader_overrides, /*remote*/ None, /*remote_auth_token*/ None, ) diff --git a/codex-rs/utils/cli/src/config_override.rs b/codex-rs/utils/cli/src/config_override.rs index 41be3ca6b6..b361558119 100644 --- a/codex-rs/utils/cli/src/config_override.rs +++ b/codex-rs/utils/cli/src/config_override.rs @@ -17,6 +17,15 @@ use toml::Value; /// calling code can decide how to interpret the right-hand side. #[derive(Parser, Debug, Default, Clone)] pub struct CliConfigOverrides { + /// Error out when config.toml contains fields that are not recognized by this version of Codex. + #[arg( + long = "strict-check", + alias = "strict-config", + global = true, + default_value_t = false + )] + pub strict_config: bool, + /// Override a configuration value that would otherwise be loaded from /// `~/.codex/config.toml`. Use a dotted path (`foo.bar.baz`) to override /// nested values. The `value` portion is parsed as TOML. If it fails to @@ -184,6 +193,7 @@ mod tests { fn canonicalizes_use_legacy_landlock_alias() { let overrides = CliConfigOverrides { raw_overrides: vec!["use_legacy_landlock=true".to_string()], + ..Default::default() }; let parsed = overrides.parse_overrides().expect("parse_overrides"); assert_eq!(parsed[0].0.as_str(), "features.use_legacy_landlock");