mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
chore(cli) deprecate --full-auto (#20133)
## Summary Starts the process of getting rid of `--full-auto`, with some concessions: 1. Fully removes the command from the tui, since it just resolves to the default permissions there, and encourages users to use the one-time trust flow if they're not in a trusted repo. 2. Marks the command as deprecated in `codex exec`, in case users are actively relying on this. We'll remove in an upcoming n+X release. 3. Cleans up some of the `codex sandbox` cli logic, to keep supporting legacy sandbox policies for now. This isn't the cleanest setup, but I think it is worthwhile to warn users for one release before hard-removing it. ## Testing - [x] Updated unit tests
This commit is contained in:
committed by
GitHub
Unverified
parent
e1ec9e63a0
commit
3d10ba9f36
@@ -42,14 +42,12 @@ pub async fn run_command_under_seatbelt(
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
) -> anyhow::Result<()> {
|
||||
let SeatbeltCommand {
|
||||
full_auto,
|
||||
allow_unix_sockets,
|
||||
log_denials,
|
||||
config_overrides,
|
||||
command,
|
||||
} = command;
|
||||
run_command_under_sandbox(
|
||||
full_auto,
|
||||
command,
|
||||
config_overrides,
|
||||
codex_linux_sandbox_exe,
|
||||
@@ -73,12 +71,10 @@ pub async fn run_command_under_landlock(
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
) -> anyhow::Result<()> {
|
||||
let LandlockCommand {
|
||||
full_auto,
|
||||
config_overrides,
|
||||
command,
|
||||
} = command;
|
||||
run_command_under_sandbox(
|
||||
full_auto,
|
||||
command,
|
||||
config_overrides,
|
||||
codex_linux_sandbox_exe,
|
||||
@@ -94,12 +90,10 @@ pub async fn run_command_under_windows(
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
) -> anyhow::Result<()> {
|
||||
let WindowsCommand {
|
||||
full_auto,
|
||||
config_overrides,
|
||||
command,
|
||||
} = command;
|
||||
run_command_under_sandbox(
|
||||
full_auto,
|
||||
command,
|
||||
config_overrides,
|
||||
codex_linux_sandbox_exe,
|
||||
@@ -118,7 +112,6 @@ enum SandboxType {
|
||||
}
|
||||
|
||||
async fn run_command_under_sandbox(
|
||||
full_auto: bool,
|
||||
command: Vec<String>,
|
||||
config_overrides: CliConfigOverrides,
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
@@ -132,7 +125,6 @@ async fn run_command_under_sandbox(
|
||||
.parse_overrides()
|
||||
.map_err(anyhow::Error::msg)?,
|
||||
codex_linux_sandbox_exe,
|
||||
full_auto,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -402,14 +394,6 @@ async fn run_command_under_windows_session(
|
||||
std::process::exit(exit_code);
|
||||
}
|
||||
|
||||
pub fn create_sandbox_mode(full_auto: bool) -> SandboxMode {
|
||||
if full_auto {
|
||||
SandboxMode::WorkspaceWrite
|
||||
} else {
|
||||
SandboxMode::ReadOnly
|
||||
}
|
||||
}
|
||||
|
||||
async fn spawn_debug_sandbox_child(
|
||||
program: PathBuf,
|
||||
args: Vec<String>,
|
||||
@@ -579,12 +563,10 @@ mod windows_stdio_bridge {
|
||||
async fn load_debug_sandbox_config(
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
full_auto: bool,
|
||||
) -> anyhow::Result<Config> {
|
||||
load_debug_sandbox_config_with_codex_home(
|
||||
cli_overrides,
|
||||
codex_linux_sandbox_exe,
|
||||
full_auto,
|
||||
/*codex_home*/ None,
|
||||
)
|
||||
.await
|
||||
@@ -593,9 +575,14 @@ async fn load_debug_sandbox_config(
|
||||
async fn load_debug_sandbox_config_with_codex_home(
|
||||
cli_overrides: Vec<(String, TomlValue)>,
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
full_auto: bool,
|
||||
codex_home: Option<PathBuf>,
|
||||
) -> anyhow::Result<Config> {
|
||||
// For legacy configs, `codex sandbox` historically defaulted to read-only
|
||||
// instead of inheriting ambient `sandbox_mode` settings from user/system
|
||||
// config. Keep that behavior unless this invocation explicitly passes a
|
||||
// legacy `sandbox_mode` CLI override, which is now the documented writable
|
||||
// replacement for the removed `--full-auto` flag.
|
||||
let uses_legacy_sandbox_mode_override = cli_overrides_use_legacy_sandbox_mode(&cli_overrides);
|
||||
let config = build_debug_sandbox_config(
|
||||
cli_overrides.clone(),
|
||||
ConfigOverrides {
|
||||
@@ -606,19 +593,14 @@ async fn load_debug_sandbox_config_with_codex_home(
|
||||
)
|
||||
.await?;
|
||||
|
||||
if config_uses_permission_profiles(&config) {
|
||||
if full_auto {
|
||||
anyhow::bail!(
|
||||
"`codex sandbox --full-auto` is only supported for legacy `sandbox_mode` configs; choose a writable `[permissions]` profile instead"
|
||||
);
|
||||
}
|
||||
if config_uses_permission_profiles(&config) || uses_legacy_sandbox_mode_override {
|
||||
return Ok(config);
|
||||
}
|
||||
|
||||
build_debug_sandbox_config(
|
||||
cli_overrides,
|
||||
ConfigOverrides {
|
||||
sandbox_mode: Some(create_sandbox_mode(full_auto)),
|
||||
sandbox_mode: Some(SandboxMode::ReadOnly),
|
||||
codex_linux_sandbox_exe,
|
||||
..Default::default()
|
||||
},
|
||||
@@ -652,9 +634,14 @@ fn config_uses_permission_profiles(config: &Config) -> bool {
|
||||
.is_some()
|
||||
}
|
||||
|
||||
fn cli_overrides_use_legacy_sandbox_mode(cli_overrides: &[(String, TomlValue)]) -> bool {
|
||||
cli_overrides.iter().any(|(key, _)| key == "sandbox_mode")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn escape_toml_path(path: &std::path::Path) -> String {
|
||||
@@ -701,7 +688,7 @@ mod tests {
|
||||
let legacy_config = build_debug_sandbox_config(
|
||||
Vec::new(),
|
||||
ConfigOverrides {
|
||||
sandbox_mode: Some(create_sandbox_mode(/*full_auto*/ false)),
|
||||
sandbox_mode: Some(SandboxMode::ReadOnly),
|
||||
..Default::default()
|
||||
},
|
||||
Some(codex_home_path.clone()),
|
||||
@@ -711,7 +698,6 @@ mod tests {
|
||||
let config = load_debug_sandbox_config_with_codex_home(
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
/*full_auto*/ false,
|
||||
Some(codex_home_path),
|
||||
)
|
||||
.await?;
|
||||
@@ -735,25 +721,90 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn debug_sandbox_rejects_full_auto_for_permission_profiles() -> anyhow::Result<()> {
|
||||
async fn debug_sandbox_honors_explicit_legacy_sandbox_mode() -> anyhow::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let sandbox_paths = TempDir::new()?;
|
||||
let docs = sandbox_paths.path().join("docs");
|
||||
let private = docs.join("private");
|
||||
write_permissions_profile_config(&codex_home, &docs, &private)?;
|
||||
let codex_home_path = codex_home.path().to_path_buf();
|
||||
let cli_overrides = vec![(
|
||||
"sandbox_mode".to_string(),
|
||||
TomlValue::String("workspace-write".to_string()),
|
||||
)];
|
||||
|
||||
let err = load_debug_sandbox_config_with_codex_home(
|
||||
let workspace_write_config = build_debug_sandbox_config(
|
||||
cli_overrides.clone(),
|
||||
ConfigOverrides::default(),
|
||||
Some(codex_home_path.clone()),
|
||||
)
|
||||
.await?;
|
||||
let read_only_config = build_debug_sandbox_config(
|
||||
Vec::new(),
|
||||
ConfigOverrides {
|
||||
sandbox_mode: Some(SandboxMode::ReadOnly),
|
||||
..Default::default()
|
||||
},
|
||||
Some(codex_home_path.clone()),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let config = load_debug_sandbox_config_with_codex_home(
|
||||
cli_overrides,
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
Some(codex_home_path),
|
||||
)
|
||||
.await?;
|
||||
|
||||
if cfg!(target_os = "windows") {
|
||||
assert_eq!(
|
||||
workspace_write_config
|
||||
.permissions
|
||||
.file_system_sandbox_policy(),
|
||||
read_only_config.permissions.file_system_sandbox_policy(),
|
||||
"workspace-write downgrades to read-only when the Windows sandbox is disabled"
|
||||
);
|
||||
} else {
|
||||
assert_ne!(
|
||||
workspace_write_config
|
||||
.permissions
|
||||
.file_system_sandbox_policy(),
|
||||
read_only_config.permissions.file_system_sandbox_policy(),
|
||||
"test fixture should distinguish explicit workspace-write from read-only"
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy(),
|
||||
workspace_write_config
|
||||
.permissions
|
||||
.file_system_sandbox_policy(),
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn debug_sandbox_defaults_legacy_configs_to_read_only() -> anyhow::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let codex_home_path = codex_home.path().to_path_buf();
|
||||
|
||||
let read_only_config = build_debug_sandbox_config(
|
||||
Vec::new(),
|
||||
ConfigOverrides {
|
||||
sandbox_mode: Some(SandboxMode::ReadOnly),
|
||||
..Default::default()
|
||||
},
|
||||
Some(codex_home_path.clone()),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let config = load_debug_sandbox_config_with_codex_home(
|
||||
Vec::new(),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
/*full_auto*/ true,
|
||||
Some(codex_home.path().to_path_buf()),
|
||||
Some(codex_home_path),
|
||||
)
|
||||
.await
|
||||
.expect_err("full-auto should be rejected for active permission profiles");
|
||||
.await?;
|
||||
|
||||
assert!(
|
||||
err.to_string().contains("--full-auto"),
|
||||
"unexpected error: {err}"
|
||||
assert!(!config_uses_permission_profiles(&config));
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy(),
|
||||
read_only_config.permissions.file_system_sandbox_policy(),
|
||||
);
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -21,10 +21,6 @@ pub use login::run_logout;
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
pub struct SeatbeltCommand {
|
||||
/// Convenience alias for low-friction sandboxed automatic execution (network-disabled sandbox that can write to cwd and TMPDIR)
|
||||
#[arg(long = "full-auto", default_value_t = false)]
|
||||
pub full_auto: bool,
|
||||
|
||||
/// Allow the sandboxed command to bind/connect AF_UNIX sockets rooted at this path. Relative paths are resolved against the current directory. Repeat to allow multiple paths.
|
||||
#[arg(long = "allow-unix-socket", value_parser = parse_allow_unix_socket_path)]
|
||||
pub allow_unix_sockets: Vec<AbsolutePathBuf>,
|
||||
@@ -48,10 +44,6 @@ fn parse_allow_unix_socket_path(raw: &str) -> Result<AbsolutePathBuf, String> {
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
pub struct LandlockCommand {
|
||||
/// Convenience alias for low-friction sandboxed automatic execution (network-disabled sandbox that can write to cwd and TMPDIR)
|
||||
#[arg(long = "full-auto", default_value_t = false)]
|
||||
pub full_auto: bool,
|
||||
|
||||
#[clap(skip)]
|
||||
pub config_overrides: CliConfigOverrides,
|
||||
|
||||
@@ -62,10 +54,6 @@ pub struct LandlockCommand {
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
pub struct WindowsCommand {
|
||||
/// Convenience alias for low-friction sandboxed automatic execution (network-disabled sandbox that can write to cwd and TMPDIR)
|
||||
#[arg(long = "full-auto", default_value_t = false)]
|
||||
pub full_auto: bool,
|
||||
|
||||
#[clap(skip)]
|
||||
pub config_overrides: CliConfigOverrides,
|
||||
|
||||
|
||||
@@ -1352,16 +1352,12 @@ async fn run_debug_prompt_input_command(
|
||||
));
|
||||
}
|
||||
|
||||
let approval_policy = if shared.full_auto {
|
||||
Some(AskForApproval::OnRequest)
|
||||
} else if shared.dangerously_bypass_approvals_and_sandbox {
|
||||
let approval_policy = if shared.dangerously_bypass_approvals_and_sandbox {
|
||||
Some(AskForApproval::Never)
|
||||
} else {
|
||||
interactive.approval_policy.map(Into::into)
|
||||
};
|
||||
let sandbox_mode = if shared.full_auto {
|
||||
Some(codex_protocol::config_types::SandboxMode::WorkspaceWrite)
|
||||
} else if shared.dangerously_bypass_approvals_and_sandbox {
|
||||
let sandbox_mode = if shared.dangerously_bypass_approvals_and_sandbox {
|
||||
Some(codex_protocol::config_types::SandboxMode::DangerFullAccess)
|
||||
} else {
|
||||
shared.sandbox_mode.map(Into::into)
|
||||
@@ -1950,6 +1946,35 @@ mod tests {
|
||||
assert!(remove_result.is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn full_auto_no_longer_parses_at_top_level() {
|
||||
let result = MultitoolCli::try_parse_from(["codex", "--full-auto"]);
|
||||
|
||||
assert!(result.is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_full_auto_reports_migration_path() {
|
||||
let cli = MultitoolCli::try_parse_from(["codex", "exec", "--full-auto", "summarize"])
|
||||
.expect("exec should accept removed flag long enough to report a migration path");
|
||||
let Some(Subcommand::Exec(exec)) = cli.subcommand else {
|
||||
panic!("expected exec subcommand");
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
exec.removed_full_auto_warning(),
|
||||
Some("warning: `--full-auto` is deprecated; use `--sandbox workspace-write` instead.")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sandbox_full_auto_no_longer_parses() {
|
||||
let result =
|
||||
MultitoolCli::try_parse_from(["codex", "sandbox", "linux", "--full-auto", "--"]);
|
||||
|
||||
assert!(result.is_err());
|
||||
}
|
||||
|
||||
fn sample_exit_info(conversation_id: Option<&str>, thread_name: Option<&str>) -> AppExitInfo {
|
||||
let token_usage = TokenUsage {
|
||||
output_tokens: 2,
|
||||
@@ -2080,14 +2105,13 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resume_merges_option_flags_and_full_auto() {
|
||||
fn resume_merges_option_flags() {
|
||||
let interactive = finalize_resume_from_args(
|
||||
[
|
||||
"codex",
|
||||
"resume",
|
||||
"sid",
|
||||
"--oss",
|
||||
"--full-auto",
|
||||
"--search",
|
||||
"--sandbox",
|
||||
"workspace-write",
|
||||
@@ -2116,7 +2140,6 @@ mod tests {
|
||||
interactive.approval_policy,
|
||||
Some(codex_utils_cli::ApprovalModeCliArg::OnRequest)
|
||||
);
|
||||
assert!(interactive.full_auto);
|
||||
assert_eq!(
|
||||
interactive.cwd.as_deref(),
|
||||
Some(std::path::Path::new("/tmp"))
|
||||
|
||||
Reference in New Issue
Block a user