diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 2837dd03..e12a3a60 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1371,7 +1371,7 @@ async fn handle_container_exec_with_params( } } Err(CodexErr::Sandbox(error)) => { - handle_sanbox_error(error, sandbox_type, params, sess, sub_id, call_id).await + handle_sandbox_error(error, sandbox_type, params, sess, sub_id, call_id).await } Err(e) => { // Handle non-sandbox errors @@ -1386,7 +1386,7 @@ async fn handle_container_exec_with_params( } } -async fn handle_sanbox_error( +async fn handle_sandbox_error( error: SandboxErr, sandbox_type: SandboxType, params: ExecParams, @@ -1408,7 +1408,14 @@ async fn handle_sanbox_error( }; } - // Ask the user to retry without sandbox + // Note that when `error` is `SandboxErr::Denied`, it could be a false + // positive. That is, it may have exited with a non-zero exit code, not + // because the sandbox denied it, but because that is its expected behavior, + // i.e., a grep command that did not match anything. Ideally we would + // include additional metadata on the command to indicate whether non-zero + // exit codes merit a retry. + + // For now, we categorically ask the user to retry without sandbox. sess.notify_background_event(&sub_id, format!("Execution failed: {error}")) .await; diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index df7da6e1..d4aa7698 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -231,12 +231,6 @@ impl SandboxPolicy { } } } - - // TODO(mbolin): This conflates sandbox policy and approval policy and - // should go away. - pub fn is_unrestricted(&self) -> bool { - matches!(self, SandboxPolicy::DangerFullAccess) - } } /// User input diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index bda16228..6a3ff299 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -63,40 +63,71 @@ pub fn assess_patch_safety( } } +/// For a command to be run _without_ a sandbox, one of the following must be +/// true: +/// +/// - the user has explicitly approved the command +/// - the command is on the "known safe" list +/// - `DangerFullAccess` was specified and `UnlessTrusted` was not pub fn assess_command_safety( command: &[String], approval_policy: AskForApproval, sandbox_policy: &SandboxPolicy, approved: &HashSet>, ) -> SafetyCheck { - let approve_without_sandbox = || SafetyCheck::AutoApprove { - sandbox_type: SandboxType::None, - }; + use AskForApproval::*; + use SandboxPolicy::*; - // Previously approved or allow-listed commands - // All approval modes allow these commands to continue without sandboxing + // A command is "trusted" because either: + // - it belongs to a set of commands we consider "safe" by default, or + // - the user has explicitly approved the command for this session + // + // Currently, whether a command is "trusted" is a simple boolean, but we + // should include more metadata on this command test to indicate whether it + // should be run inside a sandbox or not. (This could be something the user + // defines as part of `execpolicy`.) + // + // For example, when `is_known_safe_command(command)` returns `true`, it + // would probably be fine to run the command in a sandbox, but when + // `approved.contains(command)` is `true`, the user may have approved it for + // the session _because_ they know it needs to run outside a sandbox. if is_known_safe_command(command) || approved.contains(command) { - // TODO(ragona): I think we should consider running even these inside the sandbox, but it's - // a change in behavior so I'm keeping it at parity with upstream for now. - return approve_without_sandbox(); + return SafetyCheck::AutoApprove { + sandbox_type: SandboxType::None, + }; } - // Command was not known-safe or allow-listed - if sandbox_policy.is_unrestricted() { - approve_without_sandbox() - } else { - match get_platform_sandbox() { - // We have a sandbox, so we can approve the command in all modes - Some(sandbox_type) => SafetyCheck::AutoApprove { sandbox_type }, - None => { - // We do not have a sandbox, so we need to consider the approval policy - match approval_policy { - // Never is our "non-interactive" mode; it must automatically reject - AskForApproval::Never => SafetyCheck::Reject { - reason: "auto-rejected by user approval settings".to_string(), - }, - // Otherwise, we ask the user for approval - _ => SafetyCheck::AskUser, + match (approval_policy, sandbox_policy) { + (UnlessTrusted, _) => { + // Even though the user may have opted into DangerFullAccess, + // they also requested that we ask for approval for untrusted + // commands. + SafetyCheck::AskUser + } + (OnFailure, DangerFullAccess) | (Never, DangerFullAccess) => SafetyCheck::AutoApprove { + sandbox_type: SandboxType::None, + }, + (Never, ReadOnly) + | (Never, WorkspaceWrite { .. }) + | (OnFailure, ReadOnly) + | (OnFailure, WorkspaceWrite { .. }) => { + match get_platform_sandbox() { + Some(sandbox_type) => SafetyCheck::AutoApprove { sandbox_type }, + None => { + if matches!(approval_policy, OnFailure) { + // Since the command is not trusted, even though the + // user has requested to only ask for approval on + // failure, we will ask the user because no sandbox is + // available. + SafetyCheck::AskUser + } else { + // We are in non-interactive mode and lack approval, so + // all we can do is reject the command. + SafetyCheck::Reject { + reason: "auto-rejected because command is not on trusted list" + .to_string(), + } + } } } } diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index 7f356337..d9d577eb 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -22,6 +22,15 @@ pub struct Cli { #[arg(long = "full-auto", default_value_t = false)] pub full_auto: bool, + /// Skip all confirmation prompts and execute commands without sandboxing. + /// EXTREMELY DANGEROUS. Intended solely for running in environments that are externally sandboxed. + #[arg( + long = "dangerously-bypass-approvals-and-sandbox", + default_value_t = false, + conflicts_with = "full_auto" + )] + pub dangerously_bypass_approvals_and_sandbox: bool, + /// Tell the agent to use the specified directory as its working root. #[clap(long = "cd", short = 'C', value_name = "DIR")] pub cwd: Option, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index bd59e117..8603a753 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -31,6 +31,7 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any model, config_profile, full_auto, + dangerously_bypass_approvals_and_sandbox, cwd, skip_git_repo_check, color, @@ -85,6 +86,8 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any let sandbox_policy = if full_auto { Some(SandboxPolicy::new_workspace_write_policy()) + } else if dangerously_bypass_approvals_and_sandbox { + Some(SandboxPolicy::DangerFullAccess) } else { None }; diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index 7e5a8175..cb6bb923 100644 --- a/codex-rs/tui/src/cli.rs +++ b/codex-rs/tui/src/cli.rs @@ -29,6 +29,15 @@ pub struct Cli { #[arg(long = "full-auto", default_value_t = false)] pub full_auto: bool, + /// Skip all confirmation prompts and execute commands without sandboxing. + /// EXTREMELY DANGEROUS. Intended solely for running in environments that are externally sandboxed. + #[arg( + long = "dangerously-bypass-approvals-and-sandbox", + default_value_t = false, + conflicts_with_all = ["approval_policy", "full_auto"] + )] + pub dangerously_bypass_approvals_and_sandbox: bool, + /// Tell the agent to use the specified directory as its working root. #[clap(long = "cd", short = 'C', value_name = "DIR")] pub cwd: Option, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 78fc2834..156951ff 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -51,6 +51,11 @@ pub fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> std::io:: Some(SandboxPolicy::new_workspace_write_policy()), Some(AskForApproval::OnFailure), ) + } else if cli.dangerously_bypass_approvals_and_sandbox { + ( + Some(SandboxPolicy::DangerFullAccess), + Some(AskForApproval::Never), + ) } else { let sandbox_policy = None; (sandbox_policy, cli.approval_policy.map(Into::into))