From 889ee018e7dd5d8bf9861ddae56082d86cebe5c7 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 13 May 2026 09:08:05 -0700 Subject: [PATCH] config: add strict config parsing (#20559) ## Why Codex intentionally ignores unknown `config.toml` fields by default so older and newer config files keep working across versions. That leniency also makes typo detection hard because misspelled or misplaced keys disappear silently. This change adds an opt-in strict config mode so users and tooling can fail fast on unrecognized config fields without changing the default permissive behavior. This feature is possible because `serde_ignored` exposes the exact signal Codex needs: it lets Codex run ordinary Serde deserialization while recording fields Serde would otherwise ignore. That avoids requiring `#[serde(deny_unknown_fields)]` across every config type and keeps strict validation opt-in around the existing config model. ## What Changed ### Added strict config validation - Added `serde_ignored`-based validation for `ConfigToml` in `codex-rs/config/src/strict_config.rs`. - Combined `serde_ignored` with `serde_path_to_error` so strict mode preserves typed config error paths while also collecting fields Serde would otherwise ignore. - Added strict-mode validation for unknown `[features]` keys, including keys that would otherwise be accepted by `FeaturesToml`'s flattened boolean map. - Kept typed config errors ahead of ignored-field reporting, so malformed known fields are reported before unknown-field diagnostics. - Added source-range diagnostics for top-level and nested unknown config fields, including non-file managed preference source names. ### Kept parsing single-pass per source - Reworked file and managed-config loading so strict validation reuses the already parsed `TomlValue` for that source. - For actual config files and managed config strings, the loader now reads once, parses once, and validates that same parsed value instead of deserializing multiple times. - Validated `-c` / `--config` override layers with the same base-directory context used for normal relative-path resolution, so unknown override keys are still reported when another override contains a relative path. ### Scoped `--strict-config` to config-heavy entry points - Added support for `--strict-config` on the main config-loading entry points where it is most useful: - `codex` - `codex resume` - `codex fork` - `codex exec` - `codex review` - `codex mcp-server` - `codex app-server` when running the server itself - the standalone `codex-app-server` binary - the standalone `codex-exec` binary - Commands outside that set now reject `--strict-config` early with targeted errors instead of accepting it everywhere through shared CLI plumbing. - `codex app-server` subcommands such as `proxy`, `daemon`, and `generate-*` are intentionally excluded from the first rollout. - When app-server strict mode sees invalid config, app-server exits with the config error instead of logging a warning and continuing with defaults. - Introduced a dedicated `ReviewCommand` wrapper in `codex-rs/cli` instead of extending shared `ReviewArgs`, so `--strict-config` stays on the outer config-loading command surface and does not become part of the reusable review payload used by `codex exec review`. ### Coverage - Added tests for top-level and nested unknown config fields, unknown `[features]` keys, typed-error precedence, source-location reporting, and non-file managed preference source names. - Added CLI coverage showing invalid `--enable`, invalid `--disable`, and unknown `-c` overrides still error when `--strict-config` is present, including compound-looking feature names such as `multi_agent_v2.subagent_usage_hint_text`. - Added integration coverage showing both `codex app-server --strict-config` and standalone `codex-app-server --strict-config` exit with an error for unknown config fields instead of starting with fallback defaults. - Added coverage showing unsupported command surfaces reject `--strict-config` with explicit errors. ## Example Usage Run Codex with strict config validation enabled: ```shell codex --strict-config ``` Strict config mode is also available on the supported config-heavy subcommands: ```shell codex --strict-config exec "explain this repository" codex review --strict-config --uncommitted codex mcp-server --strict-config codex app-server --strict-config --listen off codex-app-server --strict-config --listen off ``` For example, if `~/.codex/config.toml` contains a typo in a key name: ```toml model = "gpt-5" approval_polic = "on-request" ``` then `codex --strict-config` reports the misspelled key instead of silently ignoring it. The path is shortened to `~` here for readability: ```text $ codex --strict-config Error loading config.toml: ~/.codex/config.toml:2:1: unknown configuration field `approval_polic` | 2 | approval_polic = "on-request" | ^^^^^^^^^^^^^^ ``` Without `--strict-config`, Codex keeps the existing permissive behavior and ignores the unknown key. Strict config mode also validates ad-hoc `-c` / `--config` overrides: ```text $ codex --strict-config -c foo=bar Error: unknown configuration field `foo` in -c/--config override $ codex --strict-config -c features.foo=true Error: unknown configuration field `features.foo` in -c/--config override ``` Invalid feature toggles are rejected too, including values that look like nested config paths: ```text $ codex --strict-config --enable does_not_exist Error: Unknown feature flag: does_not_exist $ codex --strict-config --disable does_not_exist Error: Unknown feature flag: does_not_exist $ codex --strict-config --enable multi_agent_v2.subagent_usage_hint_text Error: Unknown feature flag: multi_agent_v2.subagent_usage_hint_text ``` Unsupported commands reject the flag explicitly: ```text $ codex --strict-config cloud list Error: `--strict-config` is not supported for `codex cloud` ``` ## Verification The `codex-cli` `strict_config` tests cover invalid `--enable`, invalid `--disable`, the compound `multi_agent_v2.subagent_usage_hint_text` case, unknown `-c` overrides, app-server strict startup failure through `codex app-server`, and rejection for unsupported commands such as `codex cloud`, `codex mcp`, `codex remote-control`, and `codex app-server proxy`. The config and config-loader tests cover unknown top-level fields, unknown nested fields, unknown `[features]` keys, source-location reporting, non-file managed config sources, and `-c` validation for keys such as `features.foo`. The app-server test suite covers standalone `codex-app-server --strict-config` startup failure for an unknown config field. ## Documentation The Codex CLI docs on developers.openai.com/codex should mention `--strict-config` as an opt-in validation mode for supported config-heavy entry points once this ships. --- MODULE.bazel.lock | 1 + codex-rs/Cargo.lock | 13 +- codex-rs/Cargo.toml | 1 + codex-rs/app-server-client/src/lib.rs | 6 + codex-rs/app-server/src/config_manager.rs | 10 +- codex-rs/app-server/src/in_process.rs | 4 + codex-rs/app-server/src/lib.rs | 35 +-- codex-rs/app-server/src/main.rs | 24 +- codex-rs/app-server/src/mcp_refresh.rs | 1 + .../src/message_processor_tracing_tests.rs | 1 + .../thread_processor_tests.rs | 1 + .../tests/suite/conversation_summary.rs | 1 + codex-rs/app-server/tests/suite/mod.rs | 1 + .../app-server/tests/suite/strict_config.rs | 33 ++ .../app-server/tests/suite/v2/mcp_resource.rs | 1 + .../tests/suite/v2/remote_thread_store.rs | 1 + .../app-server/tests/suite/v2/thread_read.rs | 3 + .../tests/suite/v2/thread_unarchive.rs | 1 + codex-rs/cli/src/debug_sandbox.rs | 27 +- codex-rs/cli/src/main.rs | 289 ++++++++++++++++-- codex-rs/cli/tests/app_server.rs | 30 ++ codex-rs/cli/tests/features.rs | 28 ++ codex-rs/config/Cargo.toml | 1 + codex-rs/config/src/diagnostics.rs | 92 +++++- codex-rs/config/src/lib.rs | 3 + codex-rs/config/src/loader/README.md | 3 +- codex-rs/config/src/loader/layer_io.rs | 68 ++++- codex-rs/config/src/loader/macos.rs | 86 +++++- codex-rs/config/src/loader/mod.rs | 104 ++++++- codex-rs/config/src/state.rs | 16 + codex-rs/config/src/strict_config.rs | 201 ++++++++++++ codex-rs/config/src/strict_config_tests.rs | 112 +++++++ .../core/src/config/config_loader_tests.rs | 238 +++++++++++++-- codex-rs/core/src/config/mod.rs | 71 ++--- codex-rs/exec/src/cli.rs | 6 +- codex-rs/exec/src/lib.rs | 14 +- codex-rs/exec/src/main.rs | 3 +- codex-rs/exec/src/main_tests.rs | 12 +- codex-rs/mcp-server/src/codex_tool_config.rs | 8 +- codex-rs/mcp-server/src/lib.rs | 8 +- codex-rs/mcp-server/src/main.rs | 7 +- codex-rs/tui/src/cli.rs | 4 + codex-rs/tui/src/lib.rs | 41 ++- codex-rs/tui/src/onboarding/auth.rs | 1 + codex-rs/utils/cli/src/config_override.rs | 25 ++ 45 files changed, 1458 insertions(+), 178 deletions(-) create mode 100644 codex-rs/app-server/tests/suite/strict_config.rs create mode 100644 codex-rs/cli/tests/app_server.rs create mode 100644 codex-rs/config/src/strict_config.rs create mode 100644 codex-rs/config/src/strict_config_tests.rs diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index ce362f0978..87434e1c68 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 4a23797cc3..25827331b2 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2432,6 +2432,7 @@ dependencies = [ "prost 0.14.3", "schemars 0.8.22", "serde", + "serde_ignored", "serde_json", "serde_path_to_error", "sha2", @@ -5454,7 +5455,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]] @@ -11641,6 +11642,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" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index bd0bcec66a..f5cfda7296 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -352,6 +352,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-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index cd5dcf6649..49a7a3b800 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -336,6 +336,8 @@ pub struct InProcessClientStartArgs { pub cli_overrides: Vec<(String, TomlValue)>, /// Loader override knobs used by config API paths. pub loader_overrides: LoaderOverrides, + /// Whether config API paths should reject unknown config fields. + pub strict_config: bool, /// Preloaded cloud requirements provider. pub cloud_requirements: CloudRequirementsLoader, /// Feedback sink used by app-server/core telemetry and logs. @@ -402,6 +404,7 @@ impl InProcessClientStartArgs { config: self.config, cli_overrides: self.cli_overrides, loader_overrides: self.loader_overrides, + strict_config: self.strict_config, cloud_requirements: self.cloud_requirements, thread_config_loader, feedback: self.feedback, @@ -1030,6 +1033,7 @@ mod tests { config, cli_overrides: Vec::new(), loader_overrides: LoaderOverrides::default(), + strict_config: false, cloud_requirements: CloudRequirementsLoader::default(), feedback: CodexFeedback::new(), log_db: None, @@ -2188,6 +2192,7 @@ mod tests { config: config.clone(), cli_overrides: Vec::new(), loader_overrides: LoaderOverrides::default(), + strict_config: false, cloud_requirements: CloudRequirementsLoader::default(), feedback: CodexFeedback::new(), log_db: None, @@ -2228,6 +2233,7 @@ mod tests { config: Arc::new(config), cli_overrides: Vec::new(), loader_overrides: LoaderOverrides::default(), + strict_config: false, cloud_requirements: CloudRequirementsLoader::default(), feedback: CodexFeedback::new(), log_db: None, diff --git a/codex-rs/app-server/src/config_manager.rs b/codex-rs/app-server/src/config_manager.rs index 030829fa4b..628a4cc8ea 100644 --- a/codex-rs/app-server/src/config_manager.rs +++ b/codex-rs/app-server/src/config_manager.rs @@ -30,6 +30,7 @@ pub(crate) struct ConfigManager { cli_overrides: Arc>>, runtime_feature_enablement: Arc>>, loader_overrides: LoaderOverrides, + strict_config: bool, cloud_requirements: Arc>, arg0_paths: Arg0DispatchPaths, thread_config_loader: Arc>>, @@ -40,6 +41,7 @@ impl ConfigManager { codex_home: PathBuf, cli_overrides: Vec<(String, TomlValue)>, loader_overrides: LoaderOverrides, + strict_config: bool, cloud_requirements: CloudRequirementsLoader, arg0_paths: Arg0DispatchPaths, thread_config_loader: Arc, @@ -49,6 +51,7 @@ impl ConfigManager { cli_overrides: Arc::new(RwLock::new(cli_overrides)), runtime_feature_enablement: Arc::new(RwLock::new(BTreeMap::new())), loader_overrides, + strict_config, cloud_requirements: Arc::new(RwLock::new(cloud_requirements)), arg0_paths, thread_config_loader: Arc::new(RwLock::new(thread_config_loader)), @@ -217,6 +220,7 @@ impl ConfigManager { .codex_home(self.codex_home.clone()) .cli_overrides(merged_cli_overrides) .loader_overrides(self.loader_overrides.clone()) + .strict_config(self.strict_config) .harness_overrides(typesafe_overrides) .fallback_cwd(fallback_cwd) .cloud_requirements(self.current_cloud_requirements()) @@ -245,7 +249,10 @@ impl ConfigManager { &self.codex_home, cwd, &self.current_cli_overrides(), - self.loader_overrides.clone(), + codex_config::ConfigLoadOptions { + loader_overrides: self.loader_overrides.clone(), + strict_config: self.strict_config, + }, self.current_cloud_requirements(), thread_config_loader.as_ref(), ) @@ -280,6 +287,7 @@ impl ConfigManager { codex_home, cli_overrides, loader_overrides, + /*strict_config*/ false, cloud_requirements, Arg0DispatchPaths::default(), Arc::new(codex_config::NoopThreadConfigLoader), diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 29c0dae45a..d4fc50fea7 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -119,6 +119,8 @@ pub struct InProcessStartArgs { pub cli_overrides: Vec<(String, TomlValue)>, /// Loader override knobs used by config API paths. pub loader_overrides: LoaderOverrides, + /// Whether config API paths should reject unknown config fields. + pub strict_config: bool, /// Preloaded cloud requirements provider. pub cloud_requirements: CloudRequirementsLoader, /// Loader used to fetch typed thread config sources before a thread starts. @@ -409,6 +411,7 @@ async fn start_uninitialized(args: InProcessStartArgs) -> IoResult Arc IoResult<()> { - run_main_with_transport( + run_main_with_transport_options( arg0_paths, cli_config_overrides, loader_overrides, + strict_config, default_analytics_enabled, AppServerTransport::Stdio, SessionSource::VSCode, AppServerWebsocketAuthSettings::default(), + AppServerRuntimeOptions::default(), ) .await } @@ -409,33 +412,12 @@ impl Default for AppServerRuntimeOptions { } } -pub async fn run_main_with_transport( - arg0_paths: Arg0DispatchPaths, - cli_config_overrides: CliConfigOverrides, - loader_overrides: LoaderOverrides, - default_analytics_enabled: bool, - transport: AppServerTransport, - session_source: SessionSource, - auth: AppServerWebsocketAuthSettings, -) -> IoResult<()> { - run_main_with_transport_options( - arg0_paths, - cli_config_overrides, - loader_overrides, - default_analytics_enabled, - transport, - session_source, - auth, - AppServerRuntimeOptions::default(), - ) - .await -} - #[allow(clippy::too_many_arguments)] pub async fn run_main_with_transport_options( arg0_paths: Arg0DispatchPaths, cli_config_overrides: CliConfigOverrides, loader_overrides: LoaderOverrides, + strict_config: bool, default_analytics_enabled: bool, transport: AppServerTransport, session_source: SessionSource, @@ -472,6 +454,7 @@ pub async fn run_main_with_transport_options( codex_home.to_path_buf(), cli_kv_overrides.clone(), loader_overrides, + strict_config, Default::default(), arg0_paths.clone(), Arc::new(NoopThreadConfigLoader), @@ -500,6 +483,10 @@ pub async fn run_main_with_transport_options( { Ok(config) => (config, true), Err(err) => { + if strict_config { + return Err(err); + } + let message = config_warning_from_error("Invalid configuration; using defaults.", &err); config_warnings.push(message); ( diff --git a/codex-rs/app-server/src/main.rs b/codex-rs/app-server/src/main.rs index 31a017d17c..6f5ecabf90 100644 --- a/codex-rs/app-server/src/main.rs +++ b/codex-rs/app-server/src/main.rs @@ -39,6 +39,10 @@ struct AppServerArgs { #[command(flatten)] auth: AppServerWebsocketAuthArgs, + /// Fail if config.toml contains unknown configuration fields. + #[arg(long = "strict-config", default_value_t = false)] + strict_config: bool, + /// Hidden debug-only test hook used by integration tests that spawn the /// production app-server binary. #[cfg(debug_assertions)] @@ -52,7 +56,15 @@ struct AppServerArgs { fn main() -> anyhow::Result<()> { arg0_dispatch_or_else(|arg0_paths: Arg0DispatchPaths| async move { - let args = AppServerArgs::parse(); + let AppServerArgs { + listen, + session_source, + auth, + strict_config, + #[cfg(debug_assertions)] + disable_plugin_startup_tasks_for_tests, + remote_control, + } = AppServerArgs::parse(); let loader_overrides = if disable_managed_config_from_debug_env() { LoaderOverrides::without_managed_config_for_tests() } else { @@ -60,20 +72,20 @@ fn main() -> anyhow::Result<()> { .map(LoaderOverrides::with_managed_config_path_for_tests) .unwrap_or_default() }; - let transport = args.listen; - let session_source = args.session_source; - let auth = args.auth.try_into_settings()?; + let transport = listen; + let auth = auth.try_into_settings()?; let mut runtime_options = AppServerRuntimeOptions::default(); #[cfg(debug_assertions)] - if args.disable_plugin_startup_tasks_for_tests { + if disable_plugin_startup_tasks_for_tests { runtime_options.plugin_startup_tasks = PluginStartupTasks::Skip; } - runtime_options.remote_control_enabled = args.remote_control; + runtime_options.remote_control_enabled = remote_control; run_main_with_transport_options( arg0_paths, CliConfigOverrides::default(), loader_overrides, + strict_config, /*default_analytics_enabled*/ false, transport, session_source, diff --git a/codex-rs/app-server/src/mcp_refresh.rs b/codex-rs/app-server/src/mcp_refresh.rs index 31a6f4afd8..f7d32b2ea8 100644 --- a/codex-rs/app-server/src/mcp_refresh.rs +++ b/codex-rs/app-server/src/mcp_refresh.rs @@ -207,6 +207,7 @@ mod tests { temp_dir.path().to_path_buf(), Vec::new(), LoaderOverrides::without_managed_config_for_tests(), + /*strict_config*/ false, CloudRequirementsLoader::default(), Arg0DispatchPaths::default(), loader.clone(), diff --git a/codex-rs/app-server/src/message_processor_tracing_tests.rs b/codex-rs/app-server/src/message_processor_tracing_tests.rs index 452991270a..3daeeeb5c9 100644 --- a/codex-rs/app-server/src/message_processor_tracing_tests.rs +++ b/codex-rs/app-server/src/message_processor_tracing_tests.rs @@ -239,6 +239,7 @@ async fn build_test_processor( config.codex_home.to_path_buf(), Vec::new(), LoaderOverrides::default(), + /*strict_config*/ false, CloudRequirementsLoader::default(), Arg0DispatchPaths::default(), Arc::new(codex_config::NoopThreadConfigLoader), diff --git a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs index 61a31f9c4d..8bda80d6cf 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs @@ -582,6 +582,7 @@ mod thread_processor_behavior_tests { temp_dir.path().to_path_buf(), Vec::new(), LoaderOverrides::default(), + /*strict_config*/ false, CloudRequirementsLoader::default(), Arg0DispatchPaths::default(), Arc::new(StaticThreadConfigLoader::new(vec![ diff --git a/codex-rs/app-server/tests/suite/conversation_summary.rs b/codex-rs/app-server/tests/suite/conversation_summary.rs index 754d1f9467..5253850107 100644 --- a/codex-rs/app-server/tests/suite/conversation_summary.rs +++ b/codex-rs/app-server/tests/suite/conversation_summary.rs @@ -149,6 +149,7 @@ async fn get_conversation_summary_by_thread_id_reads_pathless_store_thread() -> config: Arc::new(config), cli_overrides: Vec::new(), loader_overrides, + strict_config: false, cloud_requirements: CloudRequirementsLoader::default(), thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader), feedback: CodexFeedback::new(), diff --git a/codex-rs/app-server/tests/suite/mod.rs b/codex-rs/app-server/tests/suite/mod.rs index c60f4a32a1..bac309a930 100644 --- a/codex-rs/app-server/tests/suite/mod.rs +++ b/codex-rs/app-server/tests/suite/mod.rs @@ -1,4 +1,5 @@ mod auth; mod conversation_summary; mod fuzzy_file_search; +mod strict_config; mod v2; diff --git a/codex-rs/app-server/tests/suite/strict_config.rs b/codex-rs/app-server/tests/suite/strict_config.rs new file mode 100644 index 0000000000..93784c9752 --- /dev/null +++ b/codex-rs/app-server/tests/suite/strict_config.rs @@ -0,0 +1,33 @@ +use std::process::Command; + +use anyhow::Result; +use tempfile::TempDir; + +#[test] +fn strict_config_rejects_unknown_config_fields_for_standalone_app_server() -> Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join("config.toml"), + r#" +foo = "bar" +"#, + )?; + + let output = Command::new(codex_utils_cargo_bin::cargo_bin("codex-app-server")?) + .env("CODEX_HOME", codex_home.path()) + .env( + "CODEX_APP_SERVER_MANAGED_CONFIG_PATH", + codex_home.path().join("managed_config.toml"), + ) + .args(["--strict-config", "--listen", "off"]) + .output()?; + + assert!(!output.status.success()); + let stderr = String::from_utf8(output.stderr)?; + assert!( + stderr.contains("unknown configuration field `foo`"), + "expected strict config error in stderr, got: {stderr}" + ); + + Ok(()) +} diff --git a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs index a51f4bbd4e..bddf57f666 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs @@ -200,6 +200,7 @@ async fn mcp_resource_read_returns_error_for_unknown_thread() -> Result<()> { config: Arc::new(config), cli_overrides: Vec::new(), loader_overrides, + strict_config: false, cloud_requirements: CloudRequirementsLoader::default(), thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader), feedback: CodexFeedback::new(), diff --git a/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs b/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs index e5c0b2c53f..9787868f5c 100644 --- a/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs +++ b/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs @@ -75,6 +75,7 @@ async fn thread_start_with_non_local_thread_store_does_not_create_local_persiste config: Arc::new(config), cli_overrides: Vec::new(), loader_overrides, + strict_config: false, cloud_requirements: CloudRequirementsLoader::default(), thread_config_loader: Arc::new(NoopThreadConfigLoader), feedback: CodexFeedback::new(), diff --git a/codex-rs/app-server/tests/suite/v2/thread_read.rs b/codex-rs/app-server/tests/suite/v2/thread_read.rs index d6267b8fc0..fa143254a5 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_read.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_read.rs @@ -375,6 +375,7 @@ async fn thread_turns_list_reads_store_history_without_rollout_path() -> Result< config: Arc::new(config), cli_overrides: Vec::new(), loader_overrides, + strict_config: false, cloud_requirements: CloudRequirementsLoader::default(), thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader), feedback: CodexFeedback::new(), @@ -440,6 +441,7 @@ async fn thread_read_loaded_include_turns_reads_store_history_without_rollout_pa config: Arc::new(config), cli_overrides: Vec::new(), loader_overrides, + strict_config: false, cloud_requirements: CloudRequirementsLoader::default(), thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader), feedback: CodexFeedback::new(), @@ -525,6 +527,7 @@ async fn thread_list_includes_store_thread_without_rollout_path() -> Result<()> config: Arc::new(config), cli_overrides: Vec::new(), loader_overrides, + strict_config: false, cloud_requirements: CloudRequirementsLoader::default(), thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader), feedback: CodexFeedback::new(), diff --git a/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs b/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs index dfc79e9d48..d2f8f268a2 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs @@ -245,6 +245,7 @@ async fn thread_unarchive_preserves_pathless_store_metadata() -> Result<()> { config: Arc::new(config), cli_overrides: Vec::new(), loader_overrides, + strict_config: false, cloud_requirements: CloudRequirementsLoader::default(), thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader), feedback: CodexFeedback::new(), diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 2722f69849..30c60d7b95 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, + /*strict_config*/ false, ) .await?; @@ -636,12 +637,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 } @@ -651,6 +654,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, @@ -680,6 +684,7 @@ async fn load_debug_sandbox_config_with_codex_home( }, codex_home.clone(), managed_requirements_mode, + strict_config, ) .await?; @@ -697,6 +702,7 @@ async fn load_debug_sandbox_config_with_codex_home( }, codex_home, managed_requirements_mode, + strict_config, ) .await .map_err(Into::into) @@ -707,14 +713,16 @@ 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 { + .harness_overrides(harness_overrides) + .strict_config(strict_config); + if matches!(managed_requirements_mode, ManagedRequirementsMode::Ignore) { builder = builder.loader_overrides(LoaderOverrides { ignore_managed_requirements: true, - ..Default::default() + ..LoaderOverrides::default() }); } if let Some(codex_home) = codex_home { @@ -783,6 +791,7 @@ mod tests { ConfigOverrides::default(), Some(codex_home_path.clone()), ManagedRequirementsMode::Include, + /*strict_config*/ false, ) .await?; let legacy_config = build_debug_sandbox_config( @@ -793,6 +802,7 @@ mod tests { }, Some(codex_home_path.clone()), ManagedRequirementsMode::Include, + /*strict_config*/ false, ) .await?; @@ -805,6 +815,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Include, }, Some(codex_home_path), + /*strict_config*/ false, ) .await?; @@ -840,6 +851,7 @@ mod tests { ConfigOverrides::default(), Some(codex_home_path.clone()), ManagedRequirementsMode::Include, + /*strict_config*/ false, ) .await?; let read_only_config = build_debug_sandbox_config( @@ -850,6 +862,7 @@ mod tests { }, Some(codex_home_path.clone()), ManagedRequirementsMode::Include, + /*strict_config*/ false, ) .await?; @@ -862,6 +875,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Include, }, Some(codex_home_path), + /*strict_config*/ false, ) .await?; @@ -905,6 +919,7 @@ mod tests { }, Some(codex_home_path.clone()), ManagedRequirementsMode::Include, + /*strict_config*/ false, ) .await?; @@ -917,6 +932,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Include, }, Some(codex_home_path), + /*strict_config*/ false, ) .await?; @@ -942,6 +958,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Ignore, }, Some(codex_home.path().to_path_buf()), + /*strict_config*/ false, ) .await?; @@ -975,6 +992,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Ignore, }, Some(codex_home.path().to_path_buf()), + /*strict_config*/ false, ) .await?; @@ -1004,6 +1022,7 @@ mod tests { managed_requirements_mode: ManagedRequirementsMode::Ignore, }, Some(codex_home.path().to_path_buf()), + /*strict_config*/ false, ) .await?; @@ -1015,6 +1034,7 @@ mod tests { ConfigOverrides::default(), Some(codex_home.path().to_path_buf()), ManagedRequirementsMode::Include, + /*strict_config*/ false, ) .await?; @@ -1040,6 +1060,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/main.rs b/codex-rs/cli/src/main.rs index 36ce876b88..df3a9ab44e 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -55,7 +55,7 @@ use crate::marketplace_cmd::MarketplaceCli; 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; @@ -109,7 +109,7 @@ enum Subcommand { Exec(ExecCli), /// Run a code review non-interactively. - Review(ReviewArgs), + Review(ReviewCommand), /// Manage login. Login(LoginCommand), @@ -124,7 +124,7 @@ enum Subcommand { Plugin(PluginCli), /// Start Codex as an MCP server (stdio). - McpServer, + McpServer(McpServerCommand), /// [experimental] Run the app server or related tooling. AppServer(AppServerCommand), @@ -266,6 +266,23 @@ struct DebugModelsCommand { bundled: bool, } +#[derive(Debug, Parser)] +struct ReviewCommand { + /// Error out when config.toml contains fields that are not recognized by this version of Codex. + #[arg(long = "strict-config", default_value_t = false)] + strict_config: bool, + + #[clap(flatten)] + args: ReviewArgs, +} + +#[derive(Debug, Parser)] +struct McpServerCommand { + /// Error out when config.toml contains fields that are not recognized by this version of Codex. + #[arg(long = "strict-config", default_value_t = false)] + strict_config: bool, +} + #[derive(Debug, Parser)] struct DebugTraceReduceCommand { /// Trace bundle directory containing manifest.json and trace.jsonl. @@ -419,6 +436,10 @@ struct AppServerCommand { #[command(subcommand)] subcommand: Option, + /// Error out when config.toml contains fields that are not recognized by this version of Codex. + #[arg(long = "strict-config", default_value_t = false)] + strict_config: bool, + /// Transport endpoint URL. Supported values: `stdio://` (default), /// `unix://`, `unix://PATH`, `ws://IP:PORT`, `off`. #[arg( @@ -810,6 +831,8 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { root_config_overrides.raw_overrides.extend(toggle_overrides); let root_remote = remote.remote; let root_remote_auth_token_env = remote.remote_auth_token_env; + let root_strict_config = interactive.strict_config; + reject_root_strict_config_for_subcommand(root_strict_config, &subcommand)?; match subcommand { None => { @@ -835,13 +858,17 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { exec_cli .shared .inherit_exec_root_options(&interactive.shared); + exec_cli.strict_config |= root_strict_config; prepend_config_flags( &mut exec_cli.config_overrides, root_config_overrides.clone(), ); codex_exec::run_main(exec_cli, arg0_paths.clone()).await?; } - Some(Subcommand::Review(review_args)) => { + Some(Subcommand::Review(ReviewCommand { + strict_config, + args: review_args, + })) => { reject_remote_mode_for_subcommand( root_remote.as_deref(), root_remote_auth_token_env.as_deref(), @@ -849,19 +876,25 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { )?; let mut exec_cli = ExecCli::try_parse_from(["codex", "exec"])?; exec_cli.command = Some(ExecCommand::Review(review_args)); + exec_cli.strict_config = strict_config || root_strict_config; prepend_config_flags( &mut exec_cli.config_overrides, root_config_overrides.clone(), ); codex_exec::run_main(exec_cli, arg0_paths.clone()).await?; } - Some(Subcommand::McpServer) => { + Some(Subcommand::McpServer(McpServerCommand { strict_config })) => { reject_remote_mode_for_subcommand( root_remote.as_deref(), root_remote_auth_token_env.as_deref(), "mcp-server", )?; - codex_mcp_server::run_main(arg0_paths.clone(), root_config_overrides).await?; + codex_mcp_server::run_main( + arg0_paths.clone(), + root_config_overrides, + strict_config || root_strict_config, + ) + .await?; } Some(Subcommand::Mcp(mut mcp_cli)) => { reject_remote_mode_for_subcommand( @@ -894,11 +927,14 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { Some(Subcommand::AppServer(app_server_cli)) => { let AppServerCommand { subcommand, + strict_config: app_server_strict_config, listen, remote_control, analytics_default_enabled, auth, } = app_server_cli; + let strict_config = app_server_strict_config || root_strict_config; + reject_strict_config_for_app_server_subcommand(strict_config, subcommand.as_ref())?; reject_remote_mode_for_app_server_subcommand( root_remote.as_deref(), root_remote_auth_token_env.as_deref(), @@ -916,6 +952,7 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { arg0_paths.clone(), root_config_overrides, codex_config::LoaderOverrides::default(), + strict_config, analytics_default_enabled, transport, codex_protocol::protocol::SessionSource::VSCode, @@ -1320,11 +1357,11 @@ 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) + .build() + .await?; let mut rows = Vec::with_capacity(FEATURES.len()); let mut name_width = 0; let mut stage_width = 0; @@ -1500,8 +1537,11 @@ 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) + .build() + .await?; let mut input = shared .images @@ -1532,7 +1572,10 @@ 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 = ConfigBuilder::default() + .cli_overrides(cli_overrides) + .build() + .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); @@ -1557,8 +1600,11 @@ 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) + .build() + .await?; let state_path = state_db_path(config.sqlite_home.as_path()); let mut cleared_state_db = false; @@ -1593,9 +1639,7 @@ fn prepend_config_flags( subcommand_config_overrides: &mut CliConfigOverrides, cli_config_overrides: CliConfigOverrides, ) { - subcommand_config_overrides - .raw_overrides - .splice(0..0, cli_config_overrides.raw_overrides); + subcommand_config_overrides.prepend_root_overrides(cli_config_overrides); } fn reject_remote_mode_for_subcommand( @@ -1616,12 +1660,103 @@ fn reject_remote_mode_for_subcommand( Ok(()) } +fn reject_root_strict_config_for_subcommand( + strict_config: bool, + subcommand: &Option, +) -> anyhow::Result<()> { + if !strict_config { + return Ok(()); + } + + match unsupported_subcommand_name_for_strict_config(subcommand) { + Some(subcommand_name) => { + reject_strict_config_for_unsupported_subcommand(strict_config, subcommand_name) + } + None => Ok(()), + } +} + +/// Return the selected subcommand name when a root-level `--strict-config` +/// flag should be rejected after parsing. +/// +/// `--strict-config` is parsed on the root interactive CLI so commands like +/// `codex --strict-config` continue to work for the TUI and for wrappers that +/// forward root options into another command shape. Clap will still accept that +/// root flag before the dispatcher knows which subcommand the user selected, so +/// unsupported subcommands need an explicit post-parse reject path. +/// +/// `Some(...)` returns the user-facing command name fragment to embed in the +/// rejection error, such as `cloud` or `app-server proxy`. `None` means the +/// selected command is allowed to inherit root `--strict-config`. +fn unsupported_subcommand_name_for_strict_config( + subcommand: &Option, +) -> Option<&'static str> { + match subcommand { + None + | Some(Subcommand::Exec(_)) + | Some(Subcommand::Review(_)) + | Some(Subcommand::McpServer(_)) + | Some(Subcommand::Resume(_)) + | Some(Subcommand::Fork(_)) => None, + Some(Subcommand::AppServer(app_server)) if app_server.subcommand.is_none() => None, + Some(Subcommand::AppServer(app_server)) => { + Some(app_server_subcommand_name(app_server.subcommand.as_ref())) + } + Some(Subcommand::RemoteControl) => Some("remote-control"), + Some(Subcommand::Mcp(_)) => Some("mcp"), + Some(Subcommand::Plugin(_)) => Some("plugin"), + #[cfg(any(target_os = "macos", target_os = "windows"))] + Some(Subcommand::App(_)) => Some("app"), + Some(Subcommand::Login(_)) => Some("login"), + Some(Subcommand::Logout(_)) => Some("logout"), + Some(Subcommand::Completion(_)) => Some("completion"), + Some(Subcommand::Update) => Some("update"), + Some(Subcommand::Cloud(_)) => Some("cloud"), + Some(Subcommand::Sandbox(_)) => Some("sandbox"), + Some(Subcommand::Debug(_)) => Some("debug"), + Some(Subcommand::Execpolicy(_)) => Some("execpolicy"), + Some(Subcommand::Apply(_)) => Some("apply"), + Some(Subcommand::ResponsesApiProxy(_)) => Some("responses-api-proxy"), + Some(Subcommand::StdioToUds(_)) => Some("stdio-to-uds"), + Some(Subcommand::ExecServer(_)) => Some("exec-server"), + Some(Subcommand::Features(_)) => Some("features"), + } +} + +fn reject_strict_config_for_app_server_subcommand( + strict_config: bool, + subcommand: Option<&AppServerSubcommand>, +) -> anyhow::Result<()> { + if subcommand.is_none() { + return Ok(()); + } + reject_strict_config_for_unsupported_subcommand( + strict_config, + app_server_subcommand_name(subcommand), + ) +} + +fn reject_strict_config_for_unsupported_subcommand( + strict_config: bool, + subcommand: &str, +) -> anyhow::Result<()> { + if strict_config { + anyhow::bail!("`--strict-config` is not supported for `codex {subcommand}`"); + } + Ok(()) +} + fn reject_remote_mode_for_app_server_subcommand( remote: Option<&str>, remote_auth_token_env: Option<&str>, subcommand: Option<&AppServerSubcommand>, ) -> anyhow::Result<()> { - let subcommand_name = match subcommand { + let subcommand_name = app_server_subcommand_name(subcommand); + reject_remote_mode_for_subcommand(remote, remote_auth_token_env, subcommand_name) +} + +fn app_server_subcommand_name(subcommand: Option<&AppServerSubcommand>) -> &'static str { + match subcommand { None => "app-server", Some(AppServerSubcommand::Daemon(daemon)) => match daemon.subcommand { AppServerDaemonSubcommand::Bootstrap(_) => "app-server daemon bootstrap", @@ -1643,8 +1778,7 @@ fn reject_remote_mode_for_app_server_subcommand( Some(AppServerSubcommand::GenerateInternalJsonSchema(_)) => { "app-server generate-internal-json-schema" } - }; - reject_remote_mode_for_subcommand(remote, remote_auth_token_env, subcommand_name) + } } async fn print_app_server_daemon_output(command: AppServerLifecycleCommand) -> anyhow::Result<()> { @@ -1816,6 +1950,7 @@ fn finalize_fork_interactive( fn merge_interactive_cli_flags(interactive: &mut TuiCli, subcommand_cli: TuiCli) { let TuiCli { shared, + strict_config, approval_policy, web_search, prompt, @@ -1831,6 +1966,9 @@ fn merge_interactive_cli_flags(interactive: &mut TuiCli, subcommand_cli: TuiCli) if web_search { interactive.web_search = true; } + if strict_config { + interactive.strict_config = true; + } if let Some(prompt) = prompt { // Normalize CRLF/CR to LF so CLI-provided text can't leak `\r` into TUI state. interactive.prompt = Some(prompt.replace("\r\n", "\n").replace('\r', "\n")); @@ -2318,6 +2456,7 @@ mod tests { "my-profile", "-C", "/tmp", + "--strict-config", "-i", "/tmp/a.png,/tmp/b.png", ] @@ -2340,6 +2479,7 @@ mod tests { Some(std::path::Path::new("/tmp")) ); assert!(interactive.web_search); + assert!(interactive.strict_config); let has_a = interactive .images .iter() @@ -2434,6 +2574,77 @@ mod tests { assert!(app_server.analytics_default_enabled); } + #[test] + fn strict_config_parses_for_supported_commands() { + let cli = MultitoolCli::try_parse_from(["codex", "--strict-config"]).expect("parse"); + assert!(cli.interactive.strict_config); + + let cli = MultitoolCli::try_parse_from(["codex", "mcp-server", "--strict-config"]) + .expect("parse"); + assert_matches!( + cli.subcommand, + Some(Subcommand::McpServer(McpServerCommand { + strict_config: true, + })) + ); + + let cli = + MultitoolCli::try_parse_from(["codex", "review", "--strict-config", "--uncommitted"]) + .expect("parse"); + assert_matches!( + cli.subcommand, + Some(Subcommand::Review(ReviewCommand { + strict_config: true, + .. + })) + ); + } + + #[test] + fn root_strict_config_is_rejected_for_unsupported_subcommands() { + let cli = MultitoolCli::try_parse_from(["codex", "--strict-config", "mcp", "list"]) + .expect("parse"); + let err = reject_root_strict_config_for_subcommand( + cli.interactive.strict_config, + &cli.subcommand, + ) + .expect_err("mcp should not support root --strict-config"); + + assert_eq!( + err.to_string(), + "`--strict-config` is not supported for `codex mcp`" + ); + + let cli = MultitoolCli::try_parse_from(["codex", "--strict-config", "remote-control"]) + .expect("parse"); + let err = reject_root_strict_config_for_subcommand( + cli.interactive.strict_config, + &cli.subcommand, + ) + .expect_err("remote-control should not support root --strict-config"); + + assert_eq!( + err.to_string(), + "`--strict-config` is not supported for `codex remote-control`" + ); + } + + #[test] + fn app_server_subcommands_reject_strict_config() { + let app_server = + app_server_from_args(["codex", "app-server", "--strict-config", "proxy"].as_ref()); + let err = reject_strict_config_for_app_server_subcommand( + app_server.strict_config, + app_server.subcommand.as_ref(), + ) + .expect_err("app-server proxy should not support --strict-config"); + + assert_eq!( + err.to_string(), + "`--strict-config` is not supported for `codex app-server proxy`" + ); + } + #[test] fn reject_remote_flag_for_remote_control() { let cli = MultitoolCli::try_parse_from(["codex", "--remote", "unix://", "remote-control"]) @@ -2880,4 +3091,38 @@ mod tests { .expect_err("feature should be rejected"); assert_eq!(err.to_string(), "Unknown feature flag: does_not_exist"); } + + #[test] + fn strict_config_with_unknown_enable_errors() { + let err = strict_config_feature_toggle_error(["--enable", "does_not_exist"].as_ref()); + assert_eq!(err.to_string(), "Unknown feature flag: does_not_exist"); + } + + #[test] + fn strict_config_with_unknown_disable_errors() { + let err = strict_config_feature_toggle_error(["--disable", "does_not_exist"].as_ref()); + assert_eq!(err.to_string(), "Unknown feature flag: does_not_exist"); + } + + #[test] + fn strict_config_with_compound_enable_errors() { + let err = strict_config_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_config_feature_toggle_error(args: &[&str]) -> anyhow::Error { + let cli_args = std::iter::once("codex") + .chain(std::iter::once("--strict-config")) + .chain(args.iter().copied()); + let cli = MultitoolCli::try_parse_from(cli_args).expect("parse should succeed"); + assert!(cli.interactive.strict_config); + cli.feature_toggles + .to_overrides() + .expect_err("feature should be rejected") + } } diff --git a/codex-rs/cli/tests/app_server.rs b/codex-rs/cli/tests/app_server.rs new file mode 100644 index 0000000000..4a2642379d --- /dev/null +++ b/codex-rs/cli/tests/app_server.rs @@ -0,0 +1,30 @@ +use std::path::Path; + +use anyhow::Result; +use predicates::str::contains; +use tempfile::TempDir; + +fn codex_command(codex_home: &Path) -> Result { + let mut cmd = assert_cmd::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?); + cmd.env("CODEX_HOME", codex_home); + Ok(cmd) +} + +#[test] +fn strict_config_rejects_unknown_config_fields_for_app_server() -> Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join("config.toml"), + r#" +foo = "bar" +"#, + )?; + + let mut cmd = codex_command(codex_home.path())?; + cmd.args(["app-server", "--strict-config", "--listen", "off"]) + .assert() + .failure() + .stderr(contains("unknown configuration field")); + + Ok(()) +} diff --git a/codex-rs/cli/tests/features.rs b/codex-rs/cli/tests/features.rs index 17a7eff679..596cfaa8f8 100644 --- a/codex-rs/cli/tests/features.rs +++ b/codex-rs/cli/tests/features.rs @@ -11,6 +11,34 @@ fn codex_command(codex_home: &Path) -> Result { Ok(cmd) } +#[test] +fn strict_config_rejects_unknown_config_override() -> Result<()> { + let codex_home = TempDir::new()?; + + let mut cmd = codex_command(codex_home.path())?; + cmd.args(["--strict-config", "-c", "foo=bar", "mcp-server"]) + .assert() + .failure() + .stderr(contains("unknown configuration field")); + + Ok(()) +} + +#[test] +fn strict_config_is_not_supported_for_cloud_command() -> Result<()> { + let codex_home = TempDir::new()?; + + let mut cmd = codex_command(codex_home.path())?; + cmd.args(["--strict-config", "-c", "foo=bar", "cloud", "list"]) + .assert() + .failure() + .stderr(contains( + "`--strict-config` is not supported for `codex cloud`", + )); + + Ok(()) +} + #[tokio::test] async fn features_enable_writes_feature_flag_to_config() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/config/Cargo.toml b/codex-rs/config/Cargo.toml index 9583a57c62..c345f31c6e 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..69549dc207 100644 --- a/codex-rs/config/src/diagnostics.rs +++ b/codex-rs/config/src/diagnostics.rs @@ -86,6 +86,23 @@ impl std::error::Error for ConfigLoadError { } } +#[derive(Clone, Copy)] +pub(crate) enum ConfigDiagnosticSource<'a> { + Path(&'a Path), + #[cfg(any(target_os = "macos", test))] + DisplayName(&'a str), +} + +impl ConfigDiagnosticSource<'_> { + pub(crate) fn to_path_buf(self) -> PathBuf { + match self { + ConfigDiagnosticSource::Path(path) => path.to_path_buf(), + #[cfg(any(target_os = "macos", test))] + ConfigDiagnosticSource::DisplayName(name) => PathBuf::from(name), + } + } +} + pub fn io_error_from_config_error( kind: io::ErrorKind, error: ConfigError, @@ -98,21 +115,39 @@ pub fn config_error_from_toml( path: impl AsRef, contents: &str, err: toml::de::Error, +) -> ConfigError { + config_error_from_toml_for_source(ConfigDiagnosticSource::Path(path.as_ref()), contents, err) +} + +pub(crate) fn config_error_from_toml_for_source( + source: ConfigDiagnosticSource<'_>, + contents: &str, + err: toml::de::Error, ) -> ConfigError { let range = err .span() .map(|span| text_range_from_span(contents, span)) .unwrap_or_else(default_range); - ConfigError::new(path.as_ref().to_path_buf(), range, err.message()) + ConfigError::new(source.to_path_buf(), range, err.message()) } pub fn config_error_from_typed_toml( path: impl AsRef, contents: &str, +) -> Option { + config_error_from_typed_toml_for_source::( + ConfigDiagnosticSource::Path(path.as_ref()), + contents, + ) +} + +fn config_error_from_typed_toml_for_source( + source: ConfigDiagnosticSource<'_>, + contents: &str, ) -> Option { let deserializer = match toml::de::Deserializer::parse(contents) { Ok(deserializer) => deserializer, - Err(err) => return Some(config_error_from_toml(path, contents, err)), + Err(err) => return Some(config_error_from_toml_for_source(source, contents, err)), }; let result: Result = serde_path_to_error::deserialize(deserializer); @@ -126,7 +161,7 @@ pub fn config_error_from_typed_toml( .map(|span| text_range_from_span(contents, span)) .unwrap_or_else(default_range); Some(ConfigError::new( - path.as_ref().to_path_buf(), + source.to_path_buf(), range, toml_err.message(), )) @@ -205,7 +240,7 @@ fn config_path_for_layer(layer: &ConfigLayerEntry, config_toml_file: &str) -> Op } } -fn text_range_from_span(contents: &str, span: std::ops::Range) -> TextRange { +pub(crate) fn text_range_from_span(contents: &str, span: std::ops::Range) -> TextRange { let start = position_for_offset(contents, span.start); let end_index = if span.end > span.start { span.end - 1 @@ -290,7 +325,7 @@ fn position_for_offset(contents: &str, index: usize) -> TextPosition { } } -fn default_range() -> TextRange { +pub(crate) fn default_range() -> TextRange { let position = TextPosition { line: 1, column: 1 }; TextRange { start: position, @@ -314,7 +349,10 @@ fn span_for_path(contents: &str, path: &SerdePath) -> Option Option> { +pub(crate) fn span_for_config_path( + contents: &str, + path: &SerdePath, +) -> Option> { if is_features_table_path(path) && let Some(span) = span_for_features_value(contents) { @@ -323,6 +361,48 @@ 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 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 2d4c62013e..e48dcedac3 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -21,6 +21,7 @@ mod requirements_exec_policy; pub mod schema; mod skills_config; mod state; +mod strict_config; mod thread_config; mod tui_keymap; pub mod types; @@ -116,7 +117,9 @@ pub use skills_config::SkillsConfig; pub use state::ConfigLayerEntry; pub use state::ConfigLayerStack; pub use state::ConfigLayerStackOrdering; +pub use state::ConfigLoadOptions; pub use state::LoaderOverrides; +pub use strict_config::config_error_from_ignored_toml_fields; pub use thread_config::NoopThreadConfigLoader; pub use thread_config::RemoteThreadConfigLoader; pub use thread_config::SessionThreadConfig; diff --git a/codex-rs/config/src/loader/README.md b/codex-rs/config/src/loader/README.md index 28750c4929..e004cea87c 100644 --- a/codex-rs/config/src/loader/README.md +++ b/codex-rs/config/src/loader/README.md @@ -10,13 +10,14 @@ This module is the canonical place to **load and describe Codex configuration la Exported from `codex_config::loader`: -- `load_config_layers_state(fs, codex_home, cwd_opt, cli_overrides, overrides, cloud_requirements, thread_config_loader) -> ConfigLayerStack` +- `load_config_layers_state(fs, codex_home, cwd_opt, cli_overrides, options, cloud_requirements, thread_config_loader) -> ConfigLayerStack` - `ConfigLayerStack` - `effective_config() -> toml::Value` - `origins() -> HashMap` - `layers_high_to_low() -> Vec` - `with_user_config(user_config) -> ConfigLayerStack` - `ConfigLayerEntry` (one layer’s `{name, config, version, disabled_reason}`; `name` carries source metadata) +- `ConfigLoadOptions` (user-facing load behavior such as strict config validation) - `LoaderOverrides` (test/override hooks for managed config sources) - `merge_toml_values(base, overlay)` (public helper used elsewhere) diff --git a/codex-rs/config/src/loader/layer_io.rs b/codex-rs/config/src/loader/layer_io.rs index 9c15df7271..415d82e405 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_toml; use crate::diagnostics::io_error_from_config_error; use crate::state::LoaderOverrides; +use crate::strict_config::config_error_from_ignored_toml_value_fields; 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; @@ -39,6 +42,7 @@ pub(super) async fn load_config_layers_internal( fs: &dyn ExecutorFileSystem, codex_home: &Path, overrides: LoaderOverrides, + strict_config: bool, ) -> io::Result { #[cfg(target_os = "macos")] let LoaderOverrides { @@ -57,19 +61,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 +104,16 @@ 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) => { + if strict_config { + validate_config_toml_strictly(path, &contents, &value)?; + } + 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 +139,33 @@ pub(super) async fn read_config_from_path( } } +fn validate_config_toml_strictly( + path: &AbsolutePathBuf, + contents: &str, + value: &TomlValue, +) -> io::Result<()> { + 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_value_fields::( + path.as_path(), + contents, + value.clone(), + ) { + 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..199e7f6daa 100644 --- a/codex-rs/config/src/loader/macos.rs +++ b/codex-rs/config/src/loader/macos.rs @@ -2,13 +2,20 @@ 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::ConfigDiagnosticSource; +use crate::diagnostics::config_error_from_toml_for_source; +use crate::diagnostics::io_error_from_config_error; +use crate::strict_config::config_error_from_ignored_toml_value_fields_for_source_name; 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 tokio::task; use toml::Value as TomlValue; @@ -31,17 +38,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 +64,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,24 +147,73 @@ 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)?; - match toml::from_str::(&raw_toml) { - Ok(TomlValue::Table(parsed)) => Ok(ManagedAdminConfigLayer { + let source_name = + format!("{MANAGED_PREFERENCES_APPLICATION_ID}:{MANAGED_PREFERENCES_CONFIG_KEY}"); + let parsed = toml::from_str::(&raw_toml).map_err(|err| { + tracing::error!("Failed to parse managed config TOML: {err}"); + if strict_config { + let config_error = config_error_from_toml_for_source( + ConfigDiagnosticSource::DisplayName(&source_name), + &raw_toml, + err.clone(), + ); + io_error_from_config_error(io::ErrorKind::InvalidData, config_error, Some(err)) + } else { + io::Error::new(io::ErrorKind::InvalidData, err) + } + })?; + + validate_managed_config_toml_strictly_if_requested( + strict_config, + &source_name, + &raw_toml, + &parsed, + base_dir, + )?; + match parsed { + TomlValue::Table(parsed) => Ok(ManagedAdminConfigLayer { config: TomlValue::Table(parsed), raw_toml, }), - Ok(other) => { + other => { tracing::error!("Managed config TOML must have a table at the root, found {other:?}",); Err(io::Error::new( io::ErrorKind::InvalidData, "managed config root must be a table", )) } - Err(err) => { - tracing::error!("Failed to parse managed config TOML: {err}"); - Err(io::Error::new(io::ErrorKind::InvalidData, err)) - } + } +} + +fn validate_managed_config_toml_strictly_if_requested( + strict_config: bool, + source_name: &str, + raw_toml: &str, + parsed: &TomlValue, + 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_value_fields_for_source_name::< + ConfigToml, + >(source_name, raw_toml, parsed.clone()) + { + Err(io_error_from_config_error( + io::ErrorKind::InvalidData, + config_error, + /*source*/ None, + )) + } else { + Ok(()) } } diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index 6e98691465..1bf70c4d05 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -21,7 +21,11 @@ use crate::project_root_markers::default_project_root_markers; use crate::project_root_markers::project_root_markers_from_config; use crate::state::ConfigLayerEntry; use crate::state::ConfigLayerStack; +use crate::state::ConfigLoadOptions; use crate::state::LoaderOverrides; +use crate::strict_config::config_error_from_ignored_toml_value_fields; +use crate::strict_config::ignored_toml_value_field; +use crate::strict_config::unknown_feature_toml_value_field; use crate::thread_config::ThreadConfigContext; use crate::thread_config::ThreadConfigLoader; use codex_app_server_protocol::ConfigLayerSource; @@ -104,10 +108,14 @@ pub async fn load_config_layers_state( codex_home: &Path, cwd: Option, cli_overrides: &[(String, TomlValue)], - overrides: LoaderOverrides, + options: impl Into, cloud_requirements: CloudRequirementsLoader, thread_config_loader: &dyn ThreadConfigLoader, ) -> io::Result { + let ConfigLoadOptions { + loader_overrides: overrides, + strict_config, + } = options.into(); let ignore_managed_requirements = overrides.ignore_managed_requirements; let ignore_user_config = overrides.ignore_user_config; let ignore_user_and_project_exec_policy_rules = @@ -140,7 +148,8 @@ pub async fn load_config_layers_state( // Make a best-effort to support the legacy `managed_config.toml` as a // requirements specification. let loaded_config_layers = - layer_io::load_config_layers_internal(fs, codex_home, overrides.clone()).await?; + layer_io::load_config_layers_internal(fs, codex_home, overrides.clone(), strict_config) + .await?; if !ignore_managed_requirements { load_requirements_from_legacy_scheme( &mut config_requirements_toml, @@ -168,6 +177,9 @@ pub async fn load_config_layers_state( .as_ref() .map(AbsolutePathBuf::as_path) .unwrap_or(codex_home); + if strict_config { + validate_cli_overrides_strictly(&cli_overrides_layer, base_dir)?; + } Some(resolve_relative_paths_in_config_toml( cli_overrides_layer, base_dir, @@ -177,16 +189,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 @@ -201,7 +217,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(), @@ -268,6 +284,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); @@ -359,15 +376,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, @@ -377,6 +390,19 @@ 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)) + })?; + if strict_config { + validate_config_toml_strictly( + toml_file.as_path(), + &contents, + &config, + config_parent, + )?; + } resolve_relative_paths_in_config_toml(config, config_parent) } Err(e) => { @@ -397,6 +423,51 @@ async fn load_config_toml_for_required_layer( Ok(create_entry(toml_value)) } +fn validate_config_toml_strictly( + toml_file: &Path, + contents: &str, + value: &TomlValue, + base_dir: &Path, +) -> io::Result<()> { + let _guard = AbsolutePathBufGuard::new(base_dir); + if let Some(config_error) = config_error_from_ignored_toml_value_fields::( + toml_file, + contents, + value.clone(), + ) { + Err(io_error_from_config_error( + io::ErrorKind::InvalidData, + config_error, + /*source*/ None, + )) + } else { + Ok(()) + } +} + +fn validate_cli_overrides_strictly( + cli_overrides_layer: &TomlValue, + base_dir: &Path, +) -> io::Result<()> { + let _guard = AbsolutePathBufGuard::new(base_dir); + if let Some(ignored_path) = ignored_toml_value_field::(cli_overrides_layer.clone()) + { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("unknown configuration field `{ignored_path}` in -c/--config override"), + )); + } + + if let Some(ignored_path) = unknown_feature_toml_value_field(cli_overrides_layer) { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("unknown configuration field `{ignored_path}` in -c/--config override"), + )); + } + + Ok(()) +} + /// If available, apply requirements from the platform system /// `requirements.toml` location to `config_requirements_toml` by filling in /// any unset fields. @@ -998,6 +1069,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 = @@ -1063,6 +1135,14 @@ async fn load_project_layers( } }; let mut config = config; + if disabled_reason.is_none() && strict_config { + validate_config_toml_strictly( + config_file.as_path(), + &contents, + &config, + 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 e718ecae67..015cd5d239 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -14,6 +14,22 @@ use std::collections::HashMap; use std::path::PathBuf; use toml::Value as TomlValue; +/// User-facing config loading behavior that is not part of the config document. +#[derive(Debug, Default, Clone)] +pub struct ConfigLoadOptions { + pub loader_overrides: LoaderOverrides, + pub strict_config: bool, +} + +impl From for ConfigLoadOptions { + fn from(loader_overrides: LoaderOverrides) -> Self { + Self { + loader_overrides, + strict_config: false, + } + } +} + /// LoaderOverrides overrides managed configuration inputs (primarily for tests). #[derive(Debug, Default, Clone)] pub struct LoaderOverrides { diff --git a/codex-rs/config/src/strict_config.rs b/codex-rs/config/src/strict_config.rs new file mode 100644 index 0000000000..fb64458e07 --- /dev/null +++ b/codex-rs/config/src/strict_config.rs @@ -0,0 +1,201 @@ +//! Strict config validation built on top of serde's ignored-field tracking. + +use crate::diagnostics::ConfigDiagnosticSource; +use crate::diagnostics::ConfigError; +use crate::diagnostics::config_error_from_toml_for_source; +use crate::diagnostics::default_range; +use crate::diagnostics::span_for_config_path; +use crate::diagnostics::span_for_toml_key_path; +use crate::diagnostics::text_range_from_span; +use codex_features::is_known_feature_key; +use serde::de::DeserializeOwned; +use std::path::Path; +use toml::Value as TomlValue; + +pub fn config_error_from_ignored_toml_fields( + path: impl AsRef, + contents: &str, +) -> Option { + let source = ConfigDiagnosticSource::Path(path.as_ref()); + match toml::from_str::(contents) { + Ok(value) => { + config_error_from_ignored_toml_value_fields_for_source::(source, contents, value) + } + Err(err) => Some(config_error_from_toml_for_source(source, contents, err)), + } +} + +pub(crate) fn config_error_from_ignored_toml_value_fields( + path: impl AsRef, + contents: &str, + value: TomlValue, +) -> Option { + config_error_from_ignored_toml_value_fields_for_source::( + ConfigDiagnosticSource::Path(path.as_ref()), + contents, + value, + ) +} + +#[cfg(any(target_os = "macos", test))] +pub(crate) fn config_error_from_ignored_toml_value_fields_for_source_name( + source_name: &str, + contents: &str, + value: TomlValue, +) -> Option { + config_error_from_ignored_toml_value_fields_for_source::( + ConfigDiagnosticSource::DisplayName(source_name), + contents, + value, + ) +} + +fn config_error_from_ignored_toml_value_fields_for_source( + source: ConfigDiagnosticSource<'_>, + contents: &str, + value: TomlValue, +) -> Option { + let unknown_feature_paths = unknown_feature_toml_value_path(&value); + let mut ignored_paths = Vec::new(); + let mut ignored_callback = |ignored_path: serde_ignored::Path<'_>| { + let path_segments = ignored_path_segments(&ignored_path); + if !path_segments.is_empty() { + ignored_paths.push(path_segments); + } + }; + let deserializer = serde_ignored::Deserializer::new(value, &mut ignored_callback); + let result: Result = serde_path_to_error::deserialize(deserializer); + + match result { + Ok(_) => unknown_field_error_from_paths(source, contents, ignored_paths) + .or_else(|| unknown_field_error_from_paths(source, contents, unknown_feature_paths)), + Err(err) => { + let path_hint = err.path().clone(); + let toml_err = err.into_inner(); + let range = span_for_config_path(contents, &path_hint) + .or_else(|| toml_err.span()) + .map(|span| text_range_from_span(contents, span)) + .unwrap_or_else(default_range); + Some(ConfigError::new( + source.to_path_buf(), + range, + toml_err.message(), + )) + } + } +} + +pub(crate) fn ignored_toml_value_field(value: TomlValue) -> Option { + let mut ignored_paths = Vec::new(); + let result: Result = serde_ignored::deserialize(value, |ignored_path| { + let path_segments = ignored_path_segments(&ignored_path); + if !path_segments.is_empty() { + ignored_paths.push(path_segments); + } + }); + if result.is_err() { + return None; + } + + ignored_paths + .into_iter() + .next() + .map(|path_segments| path_segments.join(".")) +} + +pub(crate) fn unknown_feature_toml_value_field(value: &TomlValue) -> Option { + unknown_feature_toml_value_path(value) + .into_iter() + .next() + .map(|path_segments| path_segments.join(".")) +} + +fn unknown_field_error_from_paths( + source: ConfigDiagnosticSource<'_>, + contents: &str, + ignored_paths: Vec>, +) -> Option { + let path_segments = ignored_paths.into_iter().next()?; + let ignored_path = path_segments.join("."); + let range = span_for_toml_key_path(contents, &path_segments) + .map(|span| text_range_from_span(contents, span)) + .unwrap_or_else(default_range); + Some(ConfigError::new( + source.to_path_buf(), + range, + format!("unknown configuration field `{ignored_path}`"), + )) +} + +fn unknown_feature_toml_value_path(value: &TomlValue) -> Vec> { + let Some(root) = value.as_table() else { + return Vec::new(); + }; + + let mut paths = Vec::new(); + push_unknown_feature_paths(&mut paths, &["features"], root.get("features")); + + if let Some(profiles) = root.get("profiles").and_then(TomlValue::as_table) { + for (profile_name, profile) in profiles { + let prefix = ["profiles", profile_name.as_str(), "features"]; + let features = profile + .as_table() + .and_then(|profile| profile.get("features")); + push_unknown_feature_paths(&mut paths, &prefix, features); + } + } + + paths +} + +fn push_unknown_feature_paths( + paths: &mut Vec>, + prefix: &[&str], + features: Option<&TomlValue>, +) { + let Some(features) = features.and_then(TomlValue::as_table) else { + return; + }; + + for feature_key in features + .keys() + .map(String::as_str) + .filter(|key| !is_known_feature_key(key)) + { + let mut path = prefix + .iter() + .map(|segment| (*segment).to_string()) + .collect::>(); + path.push(feature_key.to_string()); + paths.push(path); + } +} + +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); + } + } +} + +#[cfg(test)] +#[path = "strict_config_tests.rs"] +mod tests; diff --git a/codex-rs/config/src/strict_config_tests.rs b/codex-rs/config/src/strict_config_tests.rs new file mode 100644 index 0000000000..4621b6b2e5 --- /dev/null +++ b/codex-rs/config/src/strict_config_tests.rs @@ -0,0 +1,112 @@ +use super::*; +use crate::config_toml::ConfigToml; +use crate::diagnostics::TextPosition; +use crate::diagnostics::TextRange; +use pretty_assertions::assert_eq; +use std::path::PathBuf; + +#[test] +fn ignored_toml_field_errors_accept_non_file_source_names() { + let source_name = "com.openai.codex:config_toml_base64"; + let contents = r#" +model = "gpt-5" +unknown_key = true"#; + + let value = toml::from_str::(contents).expect("valid TOML"); + let error = config_error_from_ignored_toml_value_fields_for_source_name::( + source_name, + contents, + value, + ) + .expect("unknown field error"); + + assert_eq!( + error, + ConfigError::new( + PathBuf::from(source_name), + TextRange { + start: TextPosition { line: 3, column: 1 }, + end: TextPosition { + line: 3, + column: 11, + }, + }, + "unknown configuration field `unknown_key`", + ) + ); +} + +#[test] +fn type_errors_take_precedence_over_ignored_fields() { + let path = Path::new("/tmp/config.toml"); + let contents = r#" +model_context_window = "wide" +unknown_key = true"#; + + let error = + config_error_from_ignored_toml_fields::(path, contents).expect("type error"); + + assert_eq!( + error, + ConfigError::new( + path.to_path_buf(), + TextRange { + start: TextPosition { + line: 2, + column: 24, + }, + end: TextPosition { + line: 2, + column: 29, + }, + }, + "invalid type: string \"wide\", expected i64", + ) + ); +} + +#[test] +fn strict_config_rejects_unknown_feature_key() { + let path = Path::new("/tmp/config.toml"); + let contents = r#" +[features] +foo = true"#; + + let error = config_error_from_ignored_toml_fields::(path, contents) + .expect("unknown feature error"); + + assert_eq!( + error, + ConfigError::new( + path.to_path_buf(), + TextRange { + start: TextPosition { line: 3, column: 1 }, + end: TextPosition { line: 3, column: 3 }, + }, + "unknown configuration field `features.foo`", + ) + ); +} + +#[test] +fn strict_config_rejects_unknown_profile_feature_key() { + let path = Path::new("/tmp/config.toml"); + let contents = r#" +[profiles.work.features] +foo = true"#; + + let error = config_error_from_ignored_toml_fields::(path, contents) + .expect("unknown feature error"); + + assert_eq!( + error, + ConfigError::new( + path.to_path_buf(), + TextRange { + start: TextPosition { line: 3, column: 1 }, + end: TextPosition { line: 3, column: 3 }, + }, + "unknown configuration field `profiles.work.features.foo`", + ) + ); +} diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index 9eec97773d..136a422849 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; @@ -133,7 +134,8 @@ async fn cli_overrides_resolve_relative_paths_against_cwd() -> std::io::Result<( #[tokio::test] async fn returns_config_error_for_invalid_user_config_toml() { let tmp = tempdir().expect("tempdir"); - let contents = "model = \"gpt-4\"\ninvalid = ["; + let contents = r#"model = "gpt-4" +invalid = ["#; let config_path = tmp.path().join(CONFIG_TOML_FILE); std::fs::write(&config_path, contents).expect("write config"); @@ -161,7 +163,8 @@ async fn ignore_user_config_keeps_empty_user_layer() -> std::io::Result<()> { let tmp = tempdir().expect("tempdir"); std::fs::write( tmp.path().join(CONFIG_TOML_FILE), - "model = \"from-user-config\"\ninvalid = [", + r#"model = "from-user-config" +invalid = ["#, ) .expect("write config"); @@ -219,7 +222,8 @@ async fn ignore_rules_marks_config_stack_for_exec_policy_rule_skip() -> std::io: async fn returns_config_error_for_invalid_managed_config_toml() { let tmp = tempdir().expect("tempdir"); let managed_path = tmp.path().join("managed_config.toml"); - let contents = "model = \"gpt-4\"\ninvalid = ["; + let contents = r#"model = "gpt-4" +invalid = ["#; std::fs::write(&managed_path, contents).expect("write managed config"); let overrides = LoaderOverrides::with_managed_config_path_for_tests(managed_path.clone()); @@ -336,10 +340,151 @@ command = "python3 /tmp/user-hook.py" Ok(()) } +#[tokio::test] +async fn strict_config_rejects_unknown_user_config_key() { + let tmp = tempdir().expect("tempdir"); + let contents = r#"model = "gpt-5" +unknown_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::without_managed_config_for_tests()) + .strict_config(/*strict_config*/ true) + .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); +} + +#[tokio::test] +async fn strict_config_rejects_unknown_cli_override_key() { + let tmp = tempdir().expect("tempdir"); + + let err = ConfigBuilder::default() + .codex_home(tmp.path().to_path_buf()) + .fallback_cwd(Some(tmp.path().to_path_buf())) + .loader_overrides(LoaderOverrides::without_managed_config_for_tests()) + .cli_overrides(vec![( + "foo".to_string(), + TomlValue::String("bar".to_string()), + )]) + .strict_config(/*strict_config*/ true) + .build() + .await + .expect_err("expected error"); + + assert_eq!( + err.to_string(), + "unknown configuration field `foo` in -c/--config override" + ); +} + +#[tokio::test] +async fn strict_config_rejects_unknown_cli_override_key_with_relative_path_override() { + let tmp = tempdir().expect("tempdir"); + let instructions_path = tmp.path().join("instructions.md"); + std::fs::write(&instructions_path, "instructions").expect("write instructions"); + + let err = ConfigBuilder::default() + .codex_home(tmp.path().to_path_buf()) + .fallback_cwd(Some(tmp.path().to_path_buf())) + .loader_overrides(LoaderOverrides::without_managed_config_for_tests()) + .cli_overrides(vec![ + ( + "model_instructions_file".to_string(), + TomlValue::String("instructions.md".to_string()), + ), + ("foo".to_string(), TomlValue::String("bar".to_string())), + ]) + .strict_config(/*strict_config*/ true) + .build() + .await + .expect_err("expected error"); + + assert_eq!( + err.to_string(), + "unknown configuration field `foo` in -c/--config override" + ); +} + +#[tokio::test] +async fn strict_config_rejects_unknown_feature_cli_override_key() { + let tmp = tempdir().expect("tempdir"); + + let err = ConfigBuilder::default() + .codex_home(tmp.path().to_path_buf()) + .fallback_cwd(Some(tmp.path().to_path_buf())) + .loader_overrides(LoaderOverrides::without_managed_config_for_tests()) + .cli_overrides(vec![("features.foo".to_string(), TomlValue::Boolean(true))]) + .strict_config(/*strict_config*/ true) + .build() + .await + .expect_err("expected error"); + + assert_eq!( + err.to_string(), + "unknown configuration field `features.foo` in -c/--config override" + ); +} + +#[tokio::test] +async fn strict_config_rejects_unknown_feature_user_config_key() { + let tmp = tempdir().expect("tempdir"); + let contents = r#"[features] +foo = 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::without_managed_config_for_tests()) + .strict_config(/*strict_config*/ true) + .build() + .await + .expect_err("expected error"); + + let config_error = config_error_from_io(&err); + assert_eq!( + config_error.message, + "unknown configuration field `features.foo`" + ); + assert_eq!(config_error.range.start.line, 2); + assert_eq!(config_error.range.start.column, 1); +} + +#[test] +fn strict_config_points_to_unknown_nested_key() { + let tmp = tempdir().expect("tempdir"); + let contents = r#"[mcp_servers.local] +command = "echo" +unknown_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"); - let contents = "[features]\ncollaboration_modes = \"true\""; + let contents = r#"[features] +collaboration_modes = "true""#; let config_path = tmp.path().join(CONFIG_TOML_FILE); std::fs::write(&config_path, contents).expect("write config"); @@ -716,7 +861,12 @@ async fn managed_preferences_requirements_take_precedence() -> anyhow::Result<() let tmp = tempdir()?; let managed_path = tmp.path().join("managed_config.toml"); - tokio::fs::write(&managed_path, "approval_policy = \"on-request\"\n").await?; + tokio::fs::write( + &managed_path, + r#"approval_policy = "on-request" +"#, + ) + .await?; let mut loader_overrides = LoaderOverrides::with_managed_config_path_for_tests(managed_path); loader_overrides.macos_managed_config_requirements_base64 = Some( @@ -1201,11 +1351,17 @@ async fn load_config_layers_can_ignore_managed_requirements() -> anyhow::Result< let cwd = AbsolutePathBuf::from_absolute_path(tmp.path())?; let managed_config_path = tmp.path().join("managed_config.toml"); - tokio::fs::write(&managed_config_path, "approval_policy = \"never\"\n").await?; + tokio::fs::write( + &managed_config_path, + r#"approval_policy = "never" +"#, + ) + .await?; let system_requirements_path = tmp.path().join("requirements.toml"); tokio::fs::write( &system_requirements_path, - "allowed_sandbox_modes = [\"read-only\"]\n", + r#"allowed_sandbox_modes = ["read-only"] +"#, ) .await?; @@ -1391,12 +1547,14 @@ async fn project_layers_prefer_closest_cwd() -> std::io::Result<()> { tokio::fs::write( project_root.join(".codex").join(CONFIG_TOML_FILE), - "foo = \"root\"\n", + r#"foo = "root" +"#, ) .await?; tokio::fs::write( nested.join(".codex").join(CONFIG_TOML_FILE), - "foo = \"child\"\n", + r#"foo = "child" +"#, ) .await?; @@ -1867,7 +2025,12 @@ async fn codex_home_is_not_loaded_as_project_layer_from_home_dir() -> std::io::R let home_dir = tmp.path().join("home"); let codex_home = home_dir.join(".codex"); tokio::fs::create_dir_all(&codex_home).await?; - tokio::fs::write(codex_home.join(CONFIG_TOML_FILE), "foo = \"user\"\n").await?; + tokio::fs::write( + codex_home.join(CONFIG_TOML_FILE), + r#"foo = "user" +"#, + ) + .await?; let cwd = AbsolutePathBuf::from_absolute_path(&home_dir)?; let layers = load_config_layers_state( @@ -1909,7 +2072,12 @@ async fn codex_home_within_project_tree_is_not_double_loaded() -> std::io::Resul tokio::fs::create_dir_all(&nested_dot_codex).await?; tokio::fs::create_dir_all(project_root.join(".git")).await?; - tokio::fs::write(nested_dot_codex.join(CONFIG_TOML_FILE), "foo = \"child\"\n").await?; + tokio::fs::write( + nested_dot_codex.join(CONFIG_TOML_FILE), + r#"foo = "child" +"#, + ) + .await?; tokio::fs::create_dir_all(&project_dot_codex).await?; make_config_for_test( @@ -1923,7 +2091,10 @@ async fn codex_home_within_project_tree_is_not_double_loaded() -> std::io::Resul let user_config_contents = tokio::fs::read_to_string(&user_config_path).await?; tokio::fs::write( &user_config_path, - format!("foo = \"user\"\n{user_config_contents}"), + format!( + r#"foo = "user" +{user_config_contents}"# + ), ) .await?; @@ -1948,7 +2119,11 @@ async fn codex_home_within_project_tree_is_not_double_loaded() -> std::io::Resul .filter(|layer| matches!(layer.name, ConfigLayerSource::Project { .. })) .collect(); - let child_config: TomlValue = toml::from_str("foo = \"child\"\n").expect("parse child config"); + let child_config: TomlValue = toml::from_str( + r#"foo = "child" +"#, + ) + .expect("parse child config"); let expected_project_layer = ConfigLayerEntry::new( ConfigLayerSource::Project { dot_codex_folder: AbsolutePathBuf::from_absolute_path(&nested_dot_codex)?, @@ -1972,7 +2147,9 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result< tokio::fs::create_dir_all(nested.join(".codex")).await?; tokio::fs::write( nested.join(".codex").join(CONFIG_TOML_FILE), - "foo = \"child\"\nprofile = \"ignored\"\n", + r#"foo = "child" +profile = "ignored" +"#, ) .await?; @@ -1991,7 +2168,10 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result< let untrusted_config_contents = tokio::fs::read_to_string(&untrusted_config_path).await?; tokio::fs::write( &untrusted_config_path, - format!("foo = \"user\"\n{untrusted_config_contents}"), + format!( + r#"foo = "user" +{untrusted_config_contents}"# + ), ) .await?; @@ -2037,7 +2217,8 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result< tokio::fs::create_dir_all(&codex_home_unknown).await?; tokio::fs::write( codex_home_unknown.join(CONFIG_TOML_FILE), - "foo = \"user\"\n", + r#"foo = "user" +"#, ) .await?; @@ -2207,7 +2388,8 @@ async fn project_trust_does_not_match_configured_alias_for_canonical_cwd() -> st tokio::fs::write(project_root.join(".git"), "gitdir: here").await?; tokio::fs::write( project_root.join(".codex").join(CONFIG_TOML_FILE), - "foo = \"project\"\n", + r#"foo = "project" +"#, ) .await?; std::os::unix::fs::symlink(&project_root, &alias_root)?; @@ -2378,9 +2560,21 @@ async fn invalid_project_config_ignored_when_untrusted_or_unknown() -> std::io:: ) .await?; let config_contents = tokio::fs::read_to_string(&config_path).await?; - tokio::fs::write(&config_path, format!("foo = \"user\"\n{config_contents}")).await?; + tokio::fs::write( + &config_path, + format!( + r#"foo = "user" +{config_contents}"# + ), + ) + .await?; } else { - tokio::fs::write(&config_path, "foo = \"user\"\n").await?; + tokio::fs::write( + &config_path, + r#"foo = "user" +"#, + ) + .await?; } let layers = load_config_layers_state( @@ -2537,12 +2731,14 @@ async fn project_root_markers_supports_alternate_markers() -> std::io::Result<() tokio::fs::write(project_root.join(".hg"), "hg").await?; tokio::fs::write( project_root.join(".codex").join(CONFIG_TOML_FILE), - "foo = \"root\"\n", + r#"foo = "root" +"#, ) .await?; tokio::fs::write( nested.join(".codex").join(CONFIG_TOML_FILE), - "foo = \"child\"\n", + r#"foo = "child" +"#, ) .await?; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index d74f83102c..3b42a660e8 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; @@ -136,9 +135,11 @@ mod otel; mod permissions; #[cfg(test)] mod schema; +pub use codex_config::ConfigLoadOptions; 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; @@ -902,6 +903,7 @@ pub struct ConfigBuilder { cli_overrides: Option>, harness_overrides: Option, loader_overrides: Option, + strict_config: bool, cloud_requirements: CloudRequirementsLoader, thread_config_loader: Option>, fallback_cwd: Option, @@ -928,6 +930,11 @@ impl ConfigBuilder { self } + pub fn strict_config(mut self, strict_config: bool) -> Self { + self.strict_config = strict_config; + self + } + pub fn cloud_requirements(mut self, cloud_requirements: CloudRequirementsLoader) -> Self { self.cloud_requirements = cloud_requirements; self @@ -957,6 +964,7 @@ impl ConfigBuilder { cli_overrides, harness_overrides, loader_overrides, + strict_config, cloud_requirements, thread_config_loader, fallback_cwd, @@ -979,7 +987,10 @@ impl ConfigBuilder { &codex_home, Some(cwd), &cli_overrides, - loader_overrides, + ConfigLoadOptions { + loader_overrides, + strict_config, + }, cloud_requirements, thread_config_loader .as_deref() @@ -1260,56 +1271,38 @@ impl Config { ) .await } - - /// This is a secondary way of creating [Config], which is appropriate when - /// the harness is meant to be used with a specific configuration that - /// ignores user settings. For example, the `codex exec` subcommand is - /// designed to use [AskForApproval::Never] exclusively. - /// - /// Further, [ConfigOverrides] contains some options that are not supported - /// in [ConfigToml], such as `cwd`, `codex_self_exe`, `codex_linux_sandbox_exe`, and - /// `main_execve_wrapper_exe`. - pub async fn load_with_cli_overrides_and_harness_overrides( - cli_overrides: Vec<(String, TomlValue)>, - harness_overrides: ConfigOverrides, - ) -> std::io::Result { - ConfigBuilder::default() - .cli_overrides(cli_overrides) - .harness_overrides(harness_overrides) - .build() - .await - } -} - -/// DEPRECATED: Use [Config::load_with_cli_overrides()] instead because working -/// with [ConfigToml] directly means that [ConfigRequirements] have not been -/// applied yet, which risks failing to enforce required constraints. -pub async fn load_config_as_toml_with_cli_overrides( - codex_home: &Path, - cwd: Option<&AbsolutePathBuf>, - cli_overrides: Vec<(String, TomlValue)>, -) -> std::io::Result { - load_config_as_toml_with_cli_and_loader_overrides( - codex_home, - cwd, - cli_overrides, - LoaderOverrides::default(), - ) - .await } +/// DEPRECATED for most callers: prefer [Config::load_with_cli_overrides()] or +/// [ConfigBuilder] because working with [ConfigToml] directly means +/// [ConfigRequirements] have not been applied yet, which risks skipping +/// required constraints. pub async fn load_config_as_toml_with_cli_and_loader_overrides( codex_home: &Path, cwd: Option<&AbsolutePathBuf>, cli_overrides: Vec<(String, TomlValue)>, loader_overrides: LoaderOverrides, +) -> std::io::Result { + load_config_as_toml_with_cli_and_load_options(codex_home, cwd, cli_overrides, loader_overrides) + .await +} + +/// DEPRECATED for most callers: prefer [Config::load_with_cli_overrides()] or +/// [ConfigBuilder] because working with [ConfigToml] directly means +/// [ConfigRequirements] have not been applied yet, which risks skipping +/// required constraints. +pub async fn load_config_as_toml_with_cli_and_load_options( + codex_home: &Path, + cwd: Option<&AbsolutePathBuf>, + cli_overrides: Vec<(String, TomlValue)>, + options: impl Into, ) -> std::io::Result { let config_layer_stack = load_config_layers_state( LOCAL_FS.as_ref(), codex_home, cwd.cloned(), &cli_overrides, - loader_overrides, + options, CloudRequirementsLoader::default(), &codex_config::NoopThreadConfigLoader, ) diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index 5a94e815ae..3a5ebfd1ba 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -16,6 +16,10 @@ pub struct Cli { #[command(subcommand)] pub command: Option, + /// Error out when config.toml contains fields that are not recognized by this version of Codex. + #[arg(long = "strict-config", global = true, default_value_t = false)] + pub strict_config: bool, + #[clap(flatten)] pub shared: ExecSharedCliOptions, @@ -258,7 +262,7 @@ impl FromArgMatches for ResumeArgs { } } -#[derive(Parser, Debug)] +#[derive(Args, Debug)] pub struct ReviewArgs { /// Review staged, unstaged, and untracked changes. #[arg( diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index e7a960c02e..7825d163c9 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -54,6 +54,7 @@ use codex_app_server_protocol::TurnStartedNotification; use codex_arg0::Arg0DispatchPaths; use codex_cloud_requirements::cloud_requirements_loader_for_storage; use codex_config::ConfigLoadError; +use codex_config::ConfigLoadOptions; use codex_config::LoaderOverrides; use codex_config::format_config_error_with_source; use codex_core::StateDbHandle; @@ -62,7 +63,7 @@ use codex_core::config::Config; use codex_core::config::ConfigBuilder; use codex_core::config::ConfigOverrides; use codex_core::config::find_codex_home; -use codex_core::config::load_config_as_toml_with_cli_and_loader_overrides; +use codex_core::config::load_config_as_toml_with_cli_and_load_options; use codex_core::config::resolve_oss_provider; use codex_core::find_thread_meta_by_name_str; use codex_core::format_exec_policy_error_with_source; @@ -242,6 +243,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result let Cli { command, + strict_config, shared, skip_git_repo_check, ephemeral, @@ -318,18 +320,20 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result } }; - #[allow(clippy::print_stderr)] let loader_overrides = LoaderOverrides { ignore_user_config, ignore_user_and_project_exec_policy_rules: ignore_rules, ..Default::default() }; - let config_toml = match load_config_as_toml_with_cli_and_loader_overrides( + let config_toml = match load_config_as_toml_with_cli_and_load_options( &codex_home, Some(&config_cwd), cli_kv_overrides.clone(), - loader_overrides.clone(), + ConfigLoadOptions { + loader_overrides: loader_overrides.clone(), + strict_config, + }, ) .await { @@ -431,6 +435,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result .cli_overrides(cli_kv_overrides) .harness_overrides(overrides) .loader_overrides(loader_overrides) + .strict_config(strict_config) .cloud_requirements(cloud_requirements) .build() .await?; @@ -522,6 +527,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result config: std::sync::Arc::new(config.clone()), cli_overrides: run_cli_overrides, loader_overrides: run_loader_overrides, + strict_config, cloud_requirements: run_cloud_requirements, feedback: CodexFeedback::new(), log_db: None, diff --git a/codex-rs/exec/src/main.rs b/codex-rs/exec/src/main.rs index 79a681b146..61eaecdd0a 100644 --- a/codex-rs/exec/src/main.rs +++ b/codex-rs/exec/src/main.rs @@ -32,8 +32,7 @@ fn main() -> anyhow::Result<()> { let mut inner = top_cli.inner; inner .config_overrides - .raw_overrides - .splice(0..0, top_cli.config_overrides.raw_overrides); + .prepend_root_overrides(top_cli.config_overrides); run_main(inner, arg0_paths).await?; Ok(()) diff --git a/codex-rs/exec/src/main_tests.rs b/codex-rs/exec/src/main_tests.rs index a9cb0ec633..5c0a8a3bfc 100644 --- a/codex-rs/exec/src/main_tests.rs +++ b/codex-rs/exec/src/main_tests.rs @@ -7,6 +7,7 @@ fn top_cli_parses_resume_prompt_after_config_flag() { let cli = TopCli::parse_from([ "codex-exec", "resume", + "--strict-config", "--last", "--json", "--model", @@ -17,8 +18,12 @@ fn top_cli_parses_resume_prompt_after_config_flag() { "--skip-git-repo-check", PROMPT, ]); + let mut inner = cli.inner; + inner + .config_overrides + .prepend_root_overrides(cli.config_overrides); - let Some(codex_exec::Command::Resume(args)) = cli.inner.command else { + let Some(codex_exec::Command::Resume(args)) = inner.command.as_ref() else { panic!("expected resume command"); }; let effective_prompt = args.prompt.clone().or_else(|| { @@ -29,9 +34,10 @@ fn top_cli_parses_resume_prompt_after_config_flag() { } }); assert_eq!(effective_prompt.as_deref(), Some(PROMPT)); - assert_eq!(cli.config_overrides.raw_overrides.len(), 1); + assert_eq!(inner.config_overrides.raw_overrides.len(), 1); assert_eq!( - cli.config_overrides.raw_overrides[0], + inner.config_overrides.raw_overrides[0], "reasoning_level=xhigh" ); + assert!(inner.strict_config); } diff --git a/codex-rs/mcp-server/src/codex_tool_config.rs b/codex-rs/mcp-server/src/codex_tool_config.rs index f83fd4fd5a..2f9f354277 100644 --- a/codex-rs/mcp-server/src/codex_tool_config.rs +++ b/codex-rs/mcp-server/src/codex_tool_config.rs @@ -2,6 +2,7 @@ use codex_arg0::Arg0DispatchPaths; use codex_core::config::Config; +use codex_core::config::ConfigBuilder; use codex_core::config::ConfigOverrides; use codex_protocol::ThreadId; use codex_protocol::config_types::SandboxMode; @@ -191,8 +192,11 @@ impl CodexToolCallParam { .map(|(k, v)| (k, json_to_toml(v))) .collect(); - let cfg = - Config::load_with_cli_overrides_and_harness_overrides(cli_overrides, overrides).await?; + let cfg = ConfigBuilder::default() + .cli_overrides(cli_overrides) + .harness_overrides(overrides) + .build() + .await?; Ok((prompt, cfg)) } diff --git a/codex-rs/mcp-server/src/lib.rs b/codex-rs/mcp-server/src/lib.rs index 45daf99a01..d82de48f9e 100644 --- a/codex-rs/mcp-server/src/lib.rs +++ b/codex-rs/mcp-server/src/lib.rs @@ -6,7 +6,7 @@ use std::io::Result as IoResult; use std::sync::Arc; use codex_arg0::Arg0DispatchPaths; -use codex_core::config::Config; +use codex_core::config::ConfigBuilder; use codex_core::resolve_installation_id; use codex_exec_server::EnvironmentManager; use codex_exec_server::ExecServerRuntimePaths; @@ -59,6 +59,7 @@ type IncomingMessage = JsonRpcMessage; pub async fn run_main( arg0_paths: Arg0DispatchPaths, cli_config_overrides: CliConfigOverrides, + strict_config: bool, ) -> IoResult<()> { // Parse CLI overrides once and derive the base Config eagerly so later // components do not need to work with raw TOML values. @@ -68,7 +69,10 @@ pub async fn run_main( format!("error parsing -c overrides: {e}"), ) })?; - let config = Config::load_with_cli_overrides(cli_kv_overrides) + let config = ConfigBuilder::default() + .cli_overrides(cli_kv_overrides) + .strict_config(strict_config) + .build() .await .map_err(|e| { std::io::Error::new(ErrorKind::InvalidData, format!("error loading config: {e}")) diff --git a/codex-rs/mcp-server/src/main.rs b/codex-rs/mcp-server/src/main.rs index ce61fd04d1..220507446a 100644 --- a/codex-rs/mcp-server/src/main.rs +++ b/codex-rs/mcp-server/src/main.rs @@ -5,7 +5,12 @@ use codex_utils_cli::CliConfigOverrides; fn main() -> anyhow::Result<()> { arg0_dispatch_or_else(|arg0_paths: Arg0DispatchPaths| async move { - run_main(arg0_paths, CliConfigOverrides::default()).await?; + run_main( + arg0_paths, + CliConfigOverrides::default(), + /*strict_config*/ false, + ) + .await?; Ok(()) }) } diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index d6001c60d8..4ec69bf26f 100644 --- a/codex-rs/tui/src/cli.rs +++ b/codex-rs/tui/src/cli.rs @@ -12,6 +12,10 @@ pub struct Cli { #[arg(value_name = "PROMPT", value_hint = clap::ValueHint::Other)] pub prompt: Option, + /// Error out when config.toml contains fields that are not recognized by this version of Codex. + #[arg(long = "strict-config", default_value_t = false)] + pub strict_config: bool, + // Internal controls set by the top-level `codex resume` subcommand. // These are not exposed as user flags on the base `codex` command. #[clap(skip)] diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 97c7d023f5..8880935d06 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_and_loader_overrides; +use crate::legacy_core::config::load_config_as_toml_with_cli_and_load_options; 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; @@ -282,6 +282,7 @@ async fn start_embedded_app_server( config: Config, cli_kv_overrides: Vec<(String, toml::Value)>, loader_overrides: LoaderOverrides, + strict_config: bool, cloud_requirements: CloudRequirementsLoader, feedback: codex_feedback::CodexFeedback, log_db: Option, @@ -293,6 +294,7 @@ async fn start_embedded_app_server( config, cli_kv_overrides, loader_overrides, + strict_config, cloud_requirements, feedback, log_db, @@ -453,6 +455,7 @@ async fn start_app_server( config: Config, cli_kv_overrides: Vec<(String, toml::Value)>, loader_overrides: LoaderOverrides, + strict_config: bool, cloud_requirements: CloudRequirementsLoader, feedback: codex_feedback::CodexFeedback, log_db: Option, @@ -465,6 +468,7 @@ async fn start_app_server( config, cli_kv_overrides, loader_overrides, + strict_config, cloud_requirements, feedback, log_db, @@ -489,6 +493,7 @@ pub(crate) async fn start_app_server_for_picker( config.clone(), Vec::new(), LoaderOverrides::default(), + /*strict_config*/ false, CloudRequirementsLoader::default(), codex_feedback::CodexFeedback::new(), /*log_db*/ None, @@ -519,6 +524,7 @@ async fn start_embedded_app_server_with( config: Config, cli_kv_overrides: Vec<(String, toml::Value)>, loader_overrides: LoaderOverrides, + strict_config: bool, cloud_requirements: CloudRequirementsLoader, feedback: codex_feedback::CodexFeedback, log_db: Option, @@ -545,6 +551,7 @@ where config: Arc::new(config), cli_overrides: cli_kv_overrides, loader_overrides, + strict_config, cloud_requirements, feedback, log_db, @@ -759,6 +766,7 @@ pub async fn run_main( loader_overrides: LoaderOverrides, explicit_remote_endpoint: Option, ) -> std::io::Result { + let strict_config = cli.strict_config; let (sandbox_mode, approval_policy) = if cli.dangerously_bypass_approvals_and_sandbox { ( Some(SandboxMode::DangerFullAccess), @@ -836,11 +844,14 @@ 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_and_loader_overrides( + let config_toml = match load_config_as_toml_with_cli_and_load_options( &codex_home, config_cwd.as_ref(), cli_kv_overrides.clone(), - loader_overrides.clone(), + codex_config::ConfigLoadOptions { + loader_overrides: loader_overrides.clone(), + strict_config, + }, ) .await { @@ -936,6 +947,8 @@ pub async fn run_main( cli_kv_overrides.clone(), overrides.clone(), cloud_requirements.clone(), + loader_overrides.clone(), + strict_config, ) .await; @@ -991,6 +1004,8 @@ pub async fn run_main( cli_kv_overrides.clone(), overrides.clone(), cloud_requirements.clone(), + loader_overrides.clone(), + strict_config, ) .await; } @@ -1133,6 +1148,7 @@ pub async fn run_main( cli, arg0_paths, loader_overrides, + strict_config, app_server_target, remote_cwd_override, config, @@ -1154,6 +1170,7 @@ async fn run_ratatui_app( cli: Cli, arg0_paths: Arg0DispatchPaths, loader_overrides: LoaderOverrides, + strict_config: bool, app_server_target: AppServerTarget, remote_cwd_override: Option, initial_config: Config, @@ -1217,6 +1234,7 @@ async fn run_ratatui_app( initial_config.clone(), cli_kv_overrides.clone(), loader_overrides.clone(), + strict_config, cloud_requirements.clone(), feedback.clone(), log_db.clone(), @@ -1304,6 +1322,8 @@ async fn run_ratatui_app( cli_kv_overrides.clone(), overrides.clone(), cloud_requirements.clone(), + loader_overrides.clone(), + strict_config, ) .await } else { @@ -1506,6 +1526,8 @@ async fn run_ratatui_app( cli_kv_overrides.clone(), overrides.clone(), cloud_requirements.clone(), + loader_overrides.clone(), + strict_config, fallback_cwd, ) .await @@ -1515,6 +1537,8 @@ async fn run_ratatui_app( cli_kv_overrides.clone(), overrides.clone(), cloud_requirements.clone(), + loader_overrides.clone(), + strict_config, ) .await } @@ -1556,6 +1580,7 @@ async fn run_ratatui_app( config.clone(), cli_kv_overrides.clone(), loader_overrides, + strict_config, cloud_requirements.clone(), feedback.clone(), log_db.clone(), @@ -1697,11 +1722,15 @@ async fn load_config_or_exit( cli_kv_overrides: Vec<(String, toml::Value)>, overrides: ConfigOverrides, cloud_requirements: CloudRequirementsLoader, + loader_overrides: LoaderOverrides, + strict_config: bool, ) -> Config { load_config_or_exit_with_fallback_cwd( cli_kv_overrides, overrides, cloud_requirements, + loader_overrides, + strict_config, /*fallback_cwd*/ None, ) .await @@ -1711,12 +1740,16 @@ async fn load_config_or_exit_with_fallback_cwd( cli_kv_overrides: Vec<(String, toml::Value)>, overrides: ConfigOverrides, cloud_requirements: CloudRequirementsLoader, + loader_overrides: LoaderOverrides, + strict_config: bool, fallback_cwd: Option, ) -> Config { #[allow(clippy::print_stderr)] match ConfigBuilder::default() .cli_overrides(cli_kv_overrides) .harness_overrides(overrides) + .loader_overrides(loader_overrides) + .strict_config(strict_config) .cloud_requirements(cloud_requirements) .fallback_cwd(fallback_cwd) .build() @@ -1788,6 +1821,7 @@ mod tests { config, Vec::new(), LoaderOverrides::default(), + /*strict_config*/ false, CloudRequirementsLoader::default(), codex_feedback::CodexFeedback::new(), /*log_db*/ None, @@ -2353,6 +2387,7 @@ mod tests { config, Vec::new(), LoaderOverrides::default(), + /*strict_config*/ false, CloudRequirementsLoader::default(), codex_feedback::CodexFeedback::new(), /*log_db*/ None, diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 91362bf5a5..8486de32b4 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -1056,6 +1056,7 @@ mod tests { config: Arc::new(config), cli_overrides: Vec::new(), loader_overrides: Default::default(), + strict_config: false, cloud_requirements: cloud_requirements_loader_for_storage( codex_home_path.clone(), /*enable_codex_api_key_env*/ false, diff --git a/codex-rs/utils/cli/src/config_override.rs b/codex-rs/utils/cli/src/config_override.rs index 41be3ca6b6..8368b10f89 100644 --- a/codex-rs/utils/cli/src/config_override.rs +++ b/codex-rs/utils/cli/src/config_override.rs @@ -37,6 +37,13 @@ pub struct CliConfigOverrides { } impl CliConfigOverrides { + /// Prepend root-level config flags so they have lower precedence than + /// command-specific flags parsed after a subcommand. + pub fn prepend_root_overrides(&mut self, root_overrides: Self) { + self.raw_overrides + .splice(0..0, root_overrides.raw_overrides); + } + /// Parse the raw strings captured from the CLI into a list of `(path, /// value)` tuples where `value` is a `serde_json::Value`. pub fn parse_overrides(&self) -> Result, String> { @@ -190,6 +197,24 @@ mod tests { assert_eq!(parsed[0].1.as_bool(), Some(true)); } + #[test] + fn prepends_root_overrides() { + let mut subcommand_overrides = CliConfigOverrides { + raw_overrides: vec![r#"model="gpt-5.2""#.to_string()], + }; + subcommand_overrides.prepend_root_overrides(CliConfigOverrides { + raw_overrides: vec![r#"model="gpt-5.1""#.to_string()], + }); + + assert_eq!( + subcommand_overrides.raw_overrides, + vec![ + r#"model="gpt-5.1""#.to_string(), + r#"model="gpt-5.2""#.to_string(), + ] + ); + } + #[test] fn parses_inline_table() { let v = parse_toml_value("{a = 1, b = 2}").expect("parse");