mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Add an --approve-for-me CLI flag (#36373)
## What changed - Add `--approve-for-me` to interactive and exec commands to route approval requests through automatic review. - Configure the mode with `approval_policy="on-request"` and the `workspace-write` sandbox. - Propagate the option across root, `exec`, `resume`, and `fork` argument handling while preserving later subcommand permission overrides. ## Testing - Cover parsing, permission conflicts, root/subcommand precedence, resume handling, and the effective exec approval and sandbox modes. GitOrigin-RevId: ae969e8c18f925f943049fefff56255f10b25659
This commit is contained in:
+188
-6
@@ -990,6 +990,9 @@ async fn cli_main(
|
||||
let root_remote = remote.remote;
|
||||
let root_remote_auth_token_env = remote.remote_auth_token_env;
|
||||
let root_strict_config = interactive.strict_config;
|
||||
interactive
|
||||
.shared
|
||||
.take_auto_review_config_overrides(&mut root_config_overrides);
|
||||
reject_root_strict_config_for_subcommand(root_strict_config, &subcommand)?;
|
||||
if let Some(subcommand) = subcommand.as_ref() {
|
||||
profile_v2_for_subcommand(&interactive, subcommand)?;
|
||||
@@ -2570,13 +2573,19 @@ fn merge_interactive_cli_flags(interactive: &mut TuiCli, subcommand_cli: TuiCli)
|
||||
approval_policy,
|
||||
web_search,
|
||||
prompt,
|
||||
config_overrides,
|
||||
mut config_overrides,
|
||||
..
|
||||
} = subcommand_cli;
|
||||
let subcommand_auto_review = shared.auto_review;
|
||||
interactive
|
||||
.shared
|
||||
.apply_subcommand_overrides(shared.into_inner());
|
||||
if let Some(approval) = approval_policy {
|
||||
interactive
|
||||
.shared
|
||||
.take_auto_review_config_overrides(&mut config_overrides);
|
||||
if subcommand_auto_review {
|
||||
interactive.approval_policy = None;
|
||||
} else if let Some(approval) = approval_policy {
|
||||
interactive.approval_policy = Some(approval);
|
||||
}
|
||||
if web_search {
|
||||
@@ -2698,12 +2707,15 @@ mod tests {
|
||||
fn finalize_resume_from_args(args: &[&str]) -> TuiCli {
|
||||
let cli = MultitoolCli::try_parse_from(args).expect("parse");
|
||||
let MultitoolCli {
|
||||
interactive,
|
||||
config_overrides: root_overrides,
|
||||
mut interactive,
|
||||
config_overrides: mut root_overrides,
|
||||
subcommand,
|
||||
feature_toggles: _,
|
||||
remote: _,
|
||||
} = cli;
|
||||
interactive
|
||||
.shared
|
||||
.take_auto_review_config_overrides(&mut root_overrides);
|
||||
|
||||
let Subcommand::Resume(ResumeCommand {
|
||||
session_id,
|
||||
@@ -2732,12 +2744,15 @@ mod tests {
|
||||
fn finalize_fork_from_args(args: &[&str]) -> TuiCli {
|
||||
let cli = MultitoolCli::try_parse_from(args).expect("parse");
|
||||
let MultitoolCli {
|
||||
interactive,
|
||||
config_overrides: root_overrides,
|
||||
mut interactive,
|
||||
config_overrides: mut root_overrides,
|
||||
subcommand,
|
||||
feature_toggles: _,
|
||||
remote: _,
|
||||
} = cli;
|
||||
interactive
|
||||
.shared
|
||||
.take_auto_review_config_overrides(&mut root_overrides);
|
||||
|
||||
let Subcommand::Fork(ForkCommand {
|
||||
session_id,
|
||||
@@ -2754,6 +2769,22 @@ mod tests {
|
||||
finalize_fork_interactive(interactive, root_overrides, session_id, last, all, fork_cli)
|
||||
}
|
||||
|
||||
fn finalize_exec_from_args(args: &[&str]) -> ExecCli {
|
||||
let mut cli = MultitoolCli::try_parse_from(args).expect("parse");
|
||||
cli.interactive
|
||||
.shared
|
||||
.take_auto_review_config_overrides(&mut cli.config_overrides);
|
||||
let Some(Subcommand::Exec(mut exec)) = cli.subcommand else {
|
||||
panic!("expected exec subcommand");
|
||||
};
|
||||
exec.shared
|
||||
.inherit_exec_root_options(&cli.interactive.shared);
|
||||
prepend_config_flags(&mut exec.config_overrides, cli.config_overrides);
|
||||
exec.shared
|
||||
.take_auto_review_config_overrides(&mut exec.config_overrides);
|
||||
exec
|
||||
}
|
||||
|
||||
fn finalize_archive_from_args(args: &[&str]) -> (String, TuiCli, InteractiveRemoteOptions) {
|
||||
let cli = MultitoolCli::try_parse_from(args).expect("parse");
|
||||
let MultitoolCli {
|
||||
@@ -2921,6 +2952,157 @@ mod tests {
|
||||
assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approve_for_me_configures_interactive_mode() {
|
||||
for flag in ["--approve-for-me", "--not-so-yolo"] {
|
||||
let mut cli = MultitoolCli::try_parse_from(["codex", flag]).expect("parse flag");
|
||||
|
||||
assert!(cli.interactive.auto_review);
|
||||
cli.interactive
|
||||
.shared
|
||||
.take_auto_review_config_overrides(&mut cli.interactive.config_overrides);
|
||||
assert_eq!(
|
||||
cli.interactive.config_overrides.raw_overrides,
|
||||
vec![
|
||||
r#"approvals_reviewer="auto_review""#.to_string(),
|
||||
r#"approval_policy="on-request""#.to_string(),
|
||||
r#"sandbox_mode="workspace-write""#.to_string(),
|
||||
]
|
||||
);
|
||||
assert!(!cli.interactive.auto_review);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn not_so_yolo_alias_is_hidden_from_help() {
|
||||
for args in [&["codex", "--help"][..], &["codex", "exec", "--help"][..]] {
|
||||
let help = help_from_args(args);
|
||||
|
||||
assert!(!help.contains("--not-so-yolo"), "{help}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approve_for_me_defaults_propagate_from_root_to_exec() {
|
||||
let exec = finalize_exec_from_args(&["codex", "--approve-for-me", "exec", "summarize"]);
|
||||
|
||||
assert_eq!(
|
||||
exec.config_overrides.raw_overrides,
|
||||
vec![
|
||||
r#"approvals_reviewer="auto_review""#.to_string(),
|
||||
r#"approval_policy="on-request""#.to_string(),
|
||||
r#"sandbox_mode="workspace-write""#.to_string(),
|
||||
]
|
||||
);
|
||||
assert!(exec.sandbox_mode.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn later_exec_sandbox_partially_overrides_approve_for_me() {
|
||||
let exec = finalize_exec_from_args(&[
|
||||
"codex",
|
||||
"--approve-for-me",
|
||||
"exec",
|
||||
"--sandbox",
|
||||
"read-only",
|
||||
]);
|
||||
|
||||
assert_matches!(
|
||||
exec.sandbox_mode,
|
||||
Some(codex_utils_cli::SandboxModeCliArg::ReadOnly)
|
||||
);
|
||||
assert_eq!(
|
||||
exec.config_overrides.raw_overrides,
|
||||
vec![
|
||||
r#"approvals_reviewer="auto_review""#.to_string(),
|
||||
r#"approval_policy="on-request""#.to_string(),
|
||||
r#"sandbox_mode="workspace-write""#.to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn later_approve_for_me_overrides_root_exec_sandbox() {
|
||||
let exec = finalize_exec_from_args(&[
|
||||
"codex",
|
||||
"--sandbox",
|
||||
"read-only",
|
||||
"exec",
|
||||
"--approve-for-me",
|
||||
]);
|
||||
|
||||
assert!(exec.sandbox_mode.is_none());
|
||||
assert_eq!(
|
||||
exec.config_overrides.raw_overrides,
|
||||
vec![
|
||||
r#"approvals_reviewer="auto_review""#.to_string(),
|
||||
r#"approval_policy="on-request""#.to_string(),
|
||||
r#"sandbox_mode="workspace-write""#.to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn later_resume_approval_policy_partially_overrides_approve_for_me() {
|
||||
let interactive = finalize_resume_from_args(&[
|
||||
"codex",
|
||||
"--approve-for-me",
|
||||
"resume",
|
||||
"--ask-for-approval",
|
||||
"never",
|
||||
]);
|
||||
|
||||
assert_matches!(
|
||||
interactive.approval_policy,
|
||||
Some(codex_utils_cli::ApprovalModeCliArg::Never)
|
||||
);
|
||||
assert_eq!(
|
||||
interactive.config_overrides.raw_overrides,
|
||||
vec![
|
||||
r#"approvals_reviewer="auto_review""#.to_string(),
|
||||
r#"approval_policy="on-request""#.to_string(),
|
||||
r#"sandbox_mode="workspace-write""#.to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn later_approve_for_me_overrides_root_tui_approval_policy() {
|
||||
let interactive = finalize_resume_from_args(&[
|
||||
"codex",
|
||||
"--ask-for-approval",
|
||||
"never",
|
||||
"resume",
|
||||
"--approve-for-me",
|
||||
]);
|
||||
|
||||
assert!(interactive.approval_policy.is_none());
|
||||
assert_eq!(
|
||||
interactive.config_overrides.raw_overrides,
|
||||
vec![
|
||||
r#"approvals_reviewer="auto_review""#.to_string(),
|
||||
r#"approval_policy="on-request""#.to_string(),
|
||||
r#"sandbox_mode="workspace-write""#.to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approve_for_me_conflicts_with_explicit_interactive_permissions() {
|
||||
for conflicting_args in [
|
||||
vec!["--sandbox", "read-only"],
|
||||
vec!["--ask-for-approval", "on-request"],
|
||||
vec!["--dangerously-bypass-approvals-and-sandbox"],
|
||||
] {
|
||||
let mut args = vec!["codex", "--approve-for-me"];
|
||||
args.extend(conflicting_args);
|
||||
|
||||
let error =
|
||||
MultitoolCli::try_parse_from(args).expect_err("permission flags should conflict");
|
||||
assert_eq!(error.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 {
|
||||
|
||||
@@ -73,3 +73,27 @@ fn parses_config_isolation_flags() {
|
||||
assert!(cli.ignore_user_config);
|
||||
assert!(cli.ignore_rules);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approve_for_me_flag_applies_to_resume_when_passed_at_exec_root() {
|
||||
for flag in ["--approve-for-me", "--not-so-yolo"] {
|
||||
let cli = Cli::parse_from(["codex-exec", flag, "resume", "--last"]);
|
||||
|
||||
assert!(cli.auto_review);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approve_for_me_flag_conflicts_with_other_sandbox_modes() {
|
||||
for conflicting_args in [
|
||||
vec!["--sandbox", "read-only"],
|
||||
vec!["--dangerously-bypass-approvals-and-sandbox"],
|
||||
] {
|
||||
let mut args = vec!["codex-exec", "--approve-for-me"];
|
||||
args.extend(conflicting_args);
|
||||
args.push("summarize");
|
||||
|
||||
let error = Cli::try_parse_from(args).expect_err("flags should conflict");
|
||||
assert_eq!(error.kind(), clap::error::ErrorKind::ArgumentConflict);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -255,9 +255,10 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
json: json_mode,
|
||||
prompt,
|
||||
output_schema: output_schema_path,
|
||||
config_overrides,
|
||||
mut config_overrides,
|
||||
} = cli;
|
||||
let shared = shared.into_inner();
|
||||
let mut shared = shared.into_inner();
|
||||
shared.take_auto_review_config_overrides(&mut config_overrides);
|
||||
let SharedCliOptions {
|
||||
images,
|
||||
model: model_cli_arg,
|
||||
@@ -265,6 +266,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
oss_provider,
|
||||
config_profile_v2,
|
||||
sandbox_mode: sandbox_mode_cli_arg,
|
||||
auto_review: _,
|
||||
dangerously_bypass_approvals_and_sandbox,
|
||||
bypass_hook_trust,
|
||||
cwd,
|
||||
|
||||
@@ -4,15 +4,9 @@
|
||||
use core_test_support::responses;
|
||||
use core_test_support::test_codex_exec::test_codex_exec;
|
||||
|
||||
async fn run_exec_with_auto_review_config(extra_args: &[&str]) -> anyhow::Result<String> {
|
||||
async fn run_exec_with_config(config_toml: &str, extra_args: &[&str]) -> anyhow::Result<String> {
|
||||
let test = test_codex_exec();
|
||||
std::fs::write(
|
||||
test.home_path().join("config.toml"),
|
||||
r#"
|
||||
approval_policy = "on-request"
|
||||
approvals_reviewer = "auto_review"
|
||||
"#,
|
||||
)?;
|
||||
std::fs::write(test.home_path().join("config.toml"), config_toml)?;
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let body = responses::sse(vec![
|
||||
@@ -34,6 +28,17 @@ approvals_reviewer = "auto_review"
|
||||
Ok(String::from_utf8(output.stderr)?)
|
||||
}
|
||||
|
||||
async fn run_exec_with_auto_review_config(extra_args: &[&str]) -> anyhow::Result<String> {
|
||||
run_exec_with_config(
|
||||
r#"
|
||||
approval_policy = "on-request"
|
||||
approvals_reviewer = "auto_review"
|
||||
"#,
|
||||
extra_args,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_preserves_on_request_for_auto_review_config() -> anyhow::Result<()> {
|
||||
let stderr = run_exec_with_auto_review_config(&[]).await?;
|
||||
@@ -45,6 +50,21 @@ async fn exec_preserves_on_request_for_auto_review_config() -> anyhow::Result<()
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_approve_for_me_flag_sets_approval_mode_and_sandbox() -> anyhow::Result<()> {
|
||||
let stderr = run_exec_with_config("", &["--approve-for-me"]).await?;
|
||||
assert!(
|
||||
stderr.contains("approval: on-request"),
|
||||
"stderr missing --approve-for-me approval mode: {stderr}"
|
||||
);
|
||||
assert!(
|
||||
stderr.contains("sandbox: workspace-write"),
|
||||
"stderr missing --approve-for-me sandbox mode: {stderr}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_bypass_preserves_never_for_auto_review_config() -> anyhow::Result<()> {
|
||||
let stderr =
|
||||
|
||||
@@ -136,4 +136,5 @@ fn mark_tui_args(cmd: clap::Command) -> clap::Command {
|
||||
cmd.mut_arg("dangerously_bypass_approvals_and_sandbox", |arg| {
|
||||
arg.conflicts_with("approval_policy")
|
||||
})
|
||||
.mut_arg("auto_review", |arg| arg.conflicts_with("approval_policy"))
|
||||
}
|
||||
|
||||
@@ -924,6 +924,9 @@ pub async fn run_main(
|
||||
)
|
||||
};
|
||||
|
||||
cli.shared
|
||||
.take_auto_review_config_overrides(&mut cli.config_overrides);
|
||||
|
||||
// Map the legacy --search flag to the canonical web_search mode.
|
||||
if cli.web_search {
|
||||
cli.config_overrides
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
//! Shared command-line flags used by both interactive and non-interactive Codex entry points.
|
||||
|
||||
use crate::CliConfigOverrides;
|
||||
use crate::SandboxModeCliArg;
|
||||
use clap::Args;
|
||||
use codex_protocol::config_types::ProfileV2Name;
|
||||
@@ -39,6 +40,15 @@ pub struct SharedCliOptions {
|
||||
#[arg(long = "sandbox", short = 's')]
|
||||
pub sandbox_mode: Option<SandboxModeCliArg>,
|
||||
|
||||
/// Route approval requests through automatic review using the workspace-write sandbox.
|
||||
#[arg(
|
||||
long = "approve-for-me",
|
||||
alias = "not-so-yolo",
|
||||
default_value_t = false,
|
||||
conflicts_with_all = ["sandbox_mode", "dangerously_bypass_approvals_and_sandbox"]
|
||||
)]
|
||||
pub auto_review: bool,
|
||||
|
||||
/// Skip all confirmation prompts and execute commands without sandboxing.
|
||||
/// EXTREMELY DANGEROUS. Intended solely for running in environments that are externally sandboxed.
|
||||
#[arg(
|
||||
@@ -63,9 +73,25 @@ pub struct SharedCliOptions {
|
||||
}
|
||||
|
||||
impl SharedCliOptions {
|
||||
pub fn take_auto_review_config_overrides(&mut self, overrides: &mut CliConfigOverrides) {
|
||||
if self.auto_review {
|
||||
overrides
|
||||
.raw_overrides
|
||||
.push(r#"approvals_reviewer="auto_review""#.to_string());
|
||||
overrides
|
||||
.raw_overrides
|
||||
.push(r#"approval_policy="on-request""#.to_string());
|
||||
overrides
|
||||
.raw_overrides
|
||||
.push(r#"sandbox_mode="workspace-write""#.to_string());
|
||||
self.auto_review = false;
|
||||
}
|
||||
}
|
||||
|
||||
pub fn inherit_exec_root_options(&mut self, root: &Self) {
|
||||
let self_selected_sandbox_mode =
|
||||
self.sandbox_mode.is_some() || self.dangerously_bypass_approvals_and_sandbox;
|
||||
let self_selected_sandbox_mode = self.sandbox_mode.is_some()
|
||||
|| self.auto_review
|
||||
|| self.dangerously_bypass_approvals_and_sandbox;
|
||||
let Self {
|
||||
images,
|
||||
model,
|
||||
@@ -73,6 +99,7 @@ impl SharedCliOptions {
|
||||
oss_provider,
|
||||
config_profile_v2,
|
||||
sandbox_mode,
|
||||
auto_review,
|
||||
dangerously_bypass_approvals_and_sandbox,
|
||||
bypass_hook_trust,
|
||||
cwd,
|
||||
@@ -85,6 +112,7 @@ impl SharedCliOptions {
|
||||
oss_provider: root_oss_provider,
|
||||
config_profile_v2: root_config_profile_v2,
|
||||
sandbox_mode: root_sandbox_mode,
|
||||
auto_review: root_auto_review,
|
||||
dangerously_bypass_approvals_and_sandbox: root_dangerously_bypass_approvals_and_sandbox,
|
||||
bypass_hook_trust: root_bypass_hook_trust,
|
||||
cwd: root_cwd,
|
||||
@@ -103,10 +131,9 @@ impl SharedCliOptions {
|
||||
if config_profile_v2.is_none() {
|
||||
config_profile_v2.clone_from(root_config_profile_v2);
|
||||
}
|
||||
if sandbox_mode.is_none() {
|
||||
*sandbox_mode = *root_sandbox_mode;
|
||||
}
|
||||
if !self_selected_sandbox_mode {
|
||||
*sandbox_mode = *root_sandbox_mode;
|
||||
*auto_review = *root_auto_review;
|
||||
*dangerously_bypass_approvals_and_sandbox =
|
||||
*root_dangerously_bypass_approvals_and_sandbox;
|
||||
}
|
||||
@@ -130,6 +157,7 @@ impl SharedCliOptions {
|
||||
|
||||
pub fn apply_subcommand_overrides(&mut self, subcommand: Self) {
|
||||
let subcommand_selected_sandbox_mode = subcommand.sandbox_mode.is_some()
|
||||
|| subcommand.auto_review
|
||||
|| subcommand.dangerously_bypass_approvals_and_sandbox;
|
||||
let Self {
|
||||
images,
|
||||
@@ -138,6 +166,7 @@ impl SharedCliOptions {
|
||||
oss_provider,
|
||||
config_profile_v2,
|
||||
sandbox_mode,
|
||||
auto_review,
|
||||
dangerously_bypass_approvals_and_sandbox,
|
||||
bypass_hook_trust,
|
||||
cwd,
|
||||
@@ -158,6 +187,7 @@ impl SharedCliOptions {
|
||||
}
|
||||
if subcommand_selected_sandbox_mode {
|
||||
self.sandbox_mode = sandbox_mode;
|
||||
self.auto_review = auto_review;
|
||||
self.dangerously_bypass_approvals_and_sandbox =
|
||||
dangerously_bypass_approvals_and_sandbox;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user