config: add strict config parsing

This commit is contained in:
Michael Bolin
2026-05-13 08:32:31 -07:00
parent 68e045a631
commit 731b5d92d8
45 changed files with 1458 additions and 178 deletions

View File

@@ -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<PathBuf>,
options: DebugSandboxConfigOptions,
strict_config: bool,
) -> anyhow::Result<Config> {
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<PathBuf>,
options: DebugSandboxConfigOptions,
codex_home: Option<PathBuf>,
strict_config: bool,
) -> anyhow::Result<Config> {
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<PathBuf>,
managed_requirements_mode: ManagedRequirementsMode,
strict_config: bool,
) -> std::io::Result<Config> {
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?;

View File

@@ -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<AppServerSubcommand>,
/// 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<Subcommand>,
) -> 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<Subcommand>,
) -> 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")
}
}