diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 00c798517c..62f24f3d54 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -856,10 +856,7 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { root_remote_auth_token_env.as_deref(), "exec", )?; - exec_cli - .shared - .inherit_exec_root_options(&interactive.shared); - exec_cli.strict_config |= root_strict_config; + apply_exec_root_options(&mut exec_cli, &interactive)?; prepend_config_flags( &mut exec_cli.config_overrides, root_config_overrides.clone(), @@ -1652,6 +1649,22 @@ fn prepend_config_flags( subcommand_config_overrides.prepend_root_overrides(cli_config_overrides); } +fn apply_exec_root_options(exec_cli: &mut ExecCli, interactive: &TuiCli) -> anyhow::Result<()> { + exec_cli + .shared + .inherit_exec_root_options(&interactive.shared); + exec_cli.strict_config |= interactive.strict_config; + if exec_cli.approval_policy.is_none() { + exec_cli.approval_policy = interactive.approval_policy; + } + if exec_cli.dangerously_bypass_approvals_and_sandbox && exec_cli.approval_policy.is_some() { + anyhow::bail!( + "--dangerously-bypass-approvals-and-sandbox cannot be used with --ask-for-approval" + ); + } + Ok(()) +} + fn reject_remote_mode_for_subcommand( remote: Option<&str>, remote_auth_token_env: Option<&str>, @@ -2121,6 +2134,61 @@ mod tests { assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict); } + #[test] + fn exec_accepts_approval_policy_after_subcommand() { + let cli = MultitoolCli::try_parse_from(["codex", "exec", "-a", "untrusted", "hi"]) + .expect("parse should succeed"); + + let Some(Subcommand::Exec(exec)) = cli.subcommand else { + panic!("expected exec subcommand"); + }; + + assert_matches!( + exec.approval_policy, + Some(codex_utils_cli::ApprovalModeCliArg::Untrusted) + ); + } + + #[test] + fn exec_accepts_root_approval_policy() { + let cli = MultitoolCli::try_parse_from(["codex", "-a", "untrusted", "exec", "hi"]) + .expect("parse should succeed"); + let MultitoolCli { + interactive, + subcommand, + .. + } = cli; + let Some(Subcommand::Exec(mut exec)) = subcommand else { + panic!("expected exec subcommand"); + }; + + assert_matches!( + exec.approval_policy, + Some(codex_utils_cli::ApprovalModeCliArg::Untrusted) + ); + apply_exec_root_options(&mut exec, &interactive).expect("root options apply"); + + assert_matches!( + exec.approval_policy, + Some(codex_utils_cli::ApprovalModeCliArg::Untrusted) + ); + } + + #[test] + fn exec_rejects_subcommand_approval_policy_with_bypass() { + let err = MultitoolCli::try_parse_from([ + "codex", + "exec", + "--dangerously-bypass-approvals-and-sandbox", + "-a", + "untrusted", + "hi", + ]) + .expect_err("conflicting permission flags should be rejected"); + + assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict); + } + fn app_server_from_args(args: &[&str]) -> AppServerCommand { let cli = MultitoolCli::try_parse_from(args).expect("parse"); let Subcommand::AppServer(app_server) = cli.subcommand.expect("app-server present") else { diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index 3a5ebfd1ba..f317cf214c 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -2,6 +2,7 @@ use clap::Args; use clap::FromArgMatches; use clap::Parser; use clap::ValueEnum; +use codex_utils_cli::ApprovalModeCliArg; use codex_utils_cli::CliConfigOverrides; use codex_utils_cli::SharedCliOptions; use std::path::PathBuf; @@ -23,6 +24,15 @@ pub struct Cli { #[clap(flatten)] pub shared: ExecSharedCliOptions, + /// Configure when the model requires human approval before executing a command. + #[arg( + long = "ask-for-approval", + short = 'a', + global = true, + conflicts_with = "dangerously_bypass_approvals_and_sandbox" + )] + pub approval_policy: Option, + /// Allow running Codex outside a Git repository. #[arg(long = "skip-git-repo-check", global = true, default_value_t = false)] pub skip_git_repo_check: bool, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 7825d163c9..eb2a3df68b 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -94,6 +94,7 @@ use codex_protocol::protocol::SessionSource; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::canonicalize_existing_preserving_symlinks; +use codex_utils_cli::ApprovalModeCliArg; use codex_utils_cli::SharedCliOptions; use codex_utils_oss::ensure_oss_provider_ready; use codex_utils_oss::get_default_model_for_oss_provider; @@ -231,6 +232,28 @@ fn exec_stderr_env_filter() -> EnvFilter { .unwrap_or_else(|_| EnvFilter::new("error")) } +fn cli_overrides_include_approval_policy(cli_kv_overrides: &[(String, T)]) -> bool { + cli_kv_overrides + .iter() + .any(|(key, _)| key == "approval_policy") +} + +fn exec_approval_policy_override( + dangerously_bypass_approvals_and_sandbox: bool, + approval_policy: Option, + cli_kv_overrides: &[(String, T)], +) -> Option { + if dangerously_bypass_approvals_and_sandbox { + Some(AskForApproval::Never) + } else if let Some(approval_policy) = approval_policy { + Some(approval_policy.into()) + } else if cli_overrides_include_approval_policy(cli_kv_overrides) { + None + } else { + Some(AskForApproval::Never) + } +} + pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { #[allow(clippy::print_stderr)] if let Some(message) = cli.removed_full_auto_warning() { @@ -245,6 +268,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result command, strict_config, shared, + approval_policy, skip_git_repo_check, ephemeral, ignore_user_config, @@ -406,8 +430,13 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result model, review_model: None, config_profile, - // Default to never ask for approvals in headless mode. Feature flags can override. - approval_policy: Some(AskForApproval::Never), + // Default to never ask for approvals in headless mode unless the caller + // explicitly selected an approval policy. + approval_policy: exec_approval_policy_override( + dangerously_bypass_approvals_and_sandbox, + approval_policy, + &cli_kv_overrides, + ), approvals_reviewer: None, sandbox_mode, permission_profile: None, diff --git a/codex-rs/exec/src/lib_tests.rs b/codex-rs/exec/src/lib_tests.rs index c12b483e89..b07fd5ac52 100644 --- a/codex-rs/exec/src/lib_tests.rs +++ b/codex-rs/exec/src/lib_tests.rs @@ -81,6 +81,50 @@ fn exec_default_stderr_filter_suppresses_otel_self_diagnostics() { assert!(logs.contains("real exec error")); } +#[test] +fn exec_approval_policy_override_defaults_to_never() { + let cli_kv_overrides: Vec<(String, ())> = Vec::new(); + + assert_eq!( + exec_approval_policy_override(false, None, &cli_kv_overrides), + Some(AskForApproval::Never) + ); +} + +#[test] +fn exec_approval_policy_override_uses_cli_approval_policy() { + let cli_kv_overrides: Vec<(String, ())> = Vec::new(); + + assert_eq!( + exec_approval_policy_override( + false, + Some(ApprovalModeCliArg::Untrusted), + &cli_kv_overrides + ), + Some(AskForApproval::UnlessTrusted) + ); +} + +#[test] +fn exec_approval_policy_override_allows_config_approval_policy() { + let cli_kv_overrides = vec![("approval_policy".to_string(), ())]; + + assert_eq!( + exec_approval_policy_override(false, None, &cli_kv_overrides), + None + ); +} + +#[test] +fn exec_approval_policy_override_yolo_forces_never() { + let cli_kv_overrides = vec![("approval_policy".to_string(), ())]; + + assert_eq!( + exec_approval_policy_override(true, Some(ApprovalModeCliArg::Untrusted), &cli_kv_overrides), + Some(AskForApproval::Never) + ); +} + #[test] fn exec_root_span_can_be_parented_from_trace_context() { let subscriber = test_tracing_subscriber();