Skip to content
10 changes: 0 additions & 10 deletions codex-rs/core/src/codex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1075,16 +1075,6 @@ impl Session {
.map_err(FunctionCallError::RespondToModel)
}

pub(crate) async fn run_unified_exec_request(
&self,
request: crate::unified_exec::UnifiedExecRequest<'_>,
) -> Result<crate::unified_exec::UnifiedExecResult, crate::unified_exec::UnifiedExecError> {
self.services
.unified_exec_manager
.handle_request(request)
.await
}

pub async fn interrupt_task(self: &Arc<Self>) {
info!("interrupt received: abort current task, if any");
self.abort_all_tasks(TurnAbortReason::Interrupted).await;
Expand Down
6 changes: 6 additions & 0 deletions codex-rs/core/src/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ pub(crate) use runner::ExecutionRequest;
pub(crate) use runner::Executor;
pub(crate) use runner::ExecutorConfig;
pub(crate) use runner::normalize_exec_result;
pub(crate) use sandbox::RetrySandboxContext;
pub(crate) use sandbox::SandboxLaunch;
pub(crate) use sandbox::SandboxLaunchError;
pub(crate) use sandbox::build_launch_for_sandbox;
pub(crate) use sandbox::request_retry_without_sandbox;
pub(crate) use sandbox::select_sandbox;

pub(crate) mod linkers {
use crate::exec::ExecParams;
Expand Down
92 changes: 45 additions & 47 deletions codex-rs/core/src/executor/runner.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::HashSet;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::RwLock;
Expand All @@ -17,14 +18,14 @@ use crate::exec::StdoutStream;
use crate::exec::StreamOutput;
use crate::exec::process_exec_tool_call;
use crate::executor::errors::ExecError;
use crate::executor::sandbox::RetrySandboxContext;
use crate::executor::sandbox::request_retry_without_sandbox;
use crate::executor::sandbox::select_sandbox;
use crate::function_tool::FunctionCallError;
use crate::protocol::AskForApproval;
use crate::protocol::ReviewDecision;
use crate::protocol::SandboxPolicy;
use crate::shell;
use crate::tools::context::ExecCommandContext;
use codex_otel::otel_event_manager::ToolDecisionSource;

#[derive(Clone, Debug)]
pub(crate) struct ExecutorConfig {
Expand All @@ -45,6 +46,10 @@ impl ExecutorConfig {
codex_linux_sandbox_exe,
}
}

pub(crate) fn codex_linux_sandbox_exe(&self) -> Option<PathBuf> {
self.codex_linux_sandbox_exe.clone()
}
}

/// Coordinates sandbox selection, backend-specific preparation, and command
Expand All @@ -62,6 +67,18 @@ impl Executor {
}
}

pub(crate) fn approval_cache_snapshot(&self) -> HashSet<Vec<String>> {
self.approval_cache.snapshot()
}

pub(crate) fn record_session_approval(&self, command: Vec<String>) {
self.approval_cache.insert(command);
}

pub(crate) fn config_snapshot(&self) -> Option<ExecutorConfig> {
self.config.read().ok().map(|cfg| cfg.clone())
}

/// Updates the sandbox policy and working directory used for future
/// executions without recreating the executor.
pub(crate) fn update_environment(&self, sandbox_policy: SandboxPolicy, sandbox_cwd: PathBuf) {
Expand Down Expand Up @@ -167,51 +184,32 @@ impl Executor {
stdout_stream: Option<StdoutStream>,
sandbox_error: SandboxErr,
) -> Result<ExecToolCallOutput, ExecError> {
session
.notify_background_event(
&context.sub_id,
format!("Execution failed: {sandbox_error}"),
)
.await;
let decision = session
.request_command_approval(
context.sub_id.to_string(),
context.call_id.to_string(),
request.approval_command.clone(),
request.params.cwd.clone(),
Some("command failed; retry without sandbox?".to_string()),
)
.await;

context.otel_event_manager.tool_decision(
&context.tool_name,
&context.call_id,
decision,
ToolDecisionSource::User,
);
match decision {
ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {
if matches!(decision, ReviewDecision::ApprovedForSession) {
self.approval_cache.insert(request.approval_command.clone());
}
session
.notify_background_event(&context.sub_id, "retrying command without sandbox")
.await;

let retry_output = self
.spawn(
request.params.clone(),
SandboxType::None,
config,
stdout_stream,
)
.await?;

Ok(retry_output)
}
ReviewDecision::Denied | ReviewDecision::Abort => {
Err(ExecError::rejection("exec command rejected by user"))
}
let approval = request_retry_without_sandbox(
session,
format!("Execution failed: {sandbox_error}"),
&request.approval_command,
request.params.cwd.clone(),
RetrySandboxContext {
sub_id: &context.sub_id,
call_id: &context.call_id,
tool_name: &context.tool_name,
otel_event_manager: &context.otel_event_manager,
},
)
.await;
if approval.is_some() {
let retry_output = self
.spawn(
request.params.clone(),
SandboxType::None,
config,
stdout_stream,
)
.await?;

Ok(retry_output)
} else {
Err(ExecError::rejection("exec command rejected by user"))
}
}

Expand Down
132 changes: 132 additions & 0 deletions codex-rs/core/src/executor/sandbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,99 @@ use crate::executor::ExecutionMode;
use crate::executor::ExecutionRequest;
use crate::executor::ExecutorConfig;
use crate::executor::errors::ExecError;
use crate::landlock::create_linux_sandbox_command_args;
use crate::protocol::SandboxPolicy;
use crate::safety::SafetyCheck;
use crate::safety::assess_command_safety;
use crate::safety::assess_patch_safety;
use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE;
use crate::seatbelt::create_seatbelt_command_args;
use crate::spawn::CODEX_SANDBOX_ENV_VAR;
use crate::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR;
use codex_otel::otel_event_manager::OtelEventManager;
use codex_otel::otel_event_manager::ToolDecisionSource;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::Path;
use std::path::PathBuf;
use thiserror::Error;

#[derive(Debug)]
pub(crate) struct SandboxLaunch {
pub program: String,
pub args: Vec<String>,
pub env: HashMap<String, String>,
}

#[derive(Debug, Error)]
pub(crate) enum SandboxLaunchError {
#[error("missing command line for sandbox launch")]
MissingCommandLine,
#[error("missing codex-linux-sandbox executable path")]
MissingLinuxSandboxExecutable,
}

pub(crate) fn build_launch_for_sandbox(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we put this method on the main path of all exect commands. I really don't like that we are forking an already complicated logic.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in remove spawn_command_under_* and send all invocations via build_launch_for_sandbox + spawn?

sandbox: SandboxType,
command: &[String],
sandbox_policy: &SandboxPolicy,
sandbox_policy_cwd: &Path,
codex_linux_sandbox_exe: Option<&PathBuf>,
) -> Result<SandboxLaunch, SandboxLaunchError> {
let mut env = HashMap::new();
if !sandbox_policy.has_full_network_access() {
env.insert(
CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR.to_string(),
"1".to_string(),
);
}

match sandbox {
SandboxType::None => {
let (program, args) = command
.split_first()
.ok_or(SandboxLaunchError::MissingCommandLine)?;
Ok(SandboxLaunch {
program: program.clone(),
args: args.to_vec(),
env,
})
}
SandboxType::MacosSeatbelt => {
env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string());
let args =
create_seatbelt_command_args(command.to_vec(), sandbox_policy, sandbox_policy_cwd);
Ok(SandboxLaunch {
program: MACOS_PATH_TO_SEATBELT_EXECUTABLE.to_string(),
args,
env,
})
}
SandboxType::LinuxSeccomp => {
let exe =
codex_linux_sandbox_exe.ok_or(SandboxLaunchError::MissingLinuxSandboxExecutable)?;
let args = create_linux_sandbox_command_args(
command.to_vec(),
sandbox_policy,
sandbox_policy_cwd,
);
Ok(SandboxLaunch {
program: exe.to_string_lossy().to_string(),
args,
env,
})
}
}
}

pub(crate) struct RetrySandboxContext<'a> {
pub sub_id: &'a str,
pub call_id: &'a str,
pub tool_name: &'a str,
pub otel_event_manager: &'a OtelEventManager,
}

/// Sandbox placement options selected for an execution run, including whether
/// to escalate after failures and whether approvals should persist.
Expand Down Expand Up @@ -50,6 +135,53 @@ fn should_escalate_on_failure(approval: AskForApproval, sandbox: SandboxType) ->
)
}

pub(crate) async fn request_retry_without_sandbox(
session: &Session,
failure_message: impl Into<String>,
command: &[String],
cwd: PathBuf,
ctx: RetrySandboxContext<'_>,
) -> Option<ReviewDecision> {
session
.notify_background_event(ctx.sub_id, failure_message.into())
.await;

let approval_command = command.to_vec();
let decision = session
.request_command_approval(
ctx.sub_id.to_string(),
ctx.call_id.to_string(),
approval_command.clone(),
cwd,
Some("command failed; retry without sandbox?".to_string()),
)
.await;

ctx.otel_event_manager.tool_decision(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we push this log ionto request_command_approval ?

ctx.tool_name,
ctx.call_id,
decision,
ToolDecisionSource::User,
);

match decision {
ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {
if matches!(decision, ReviewDecision::ApprovedForSession) {
session
.services
.executor
.record_session_approval(approval_command);
}

session
.notify_background_event(ctx.sub_id, "retrying command without sandbox")
.await;
Some(decision)
}
ReviewDecision::Denied | ReviewDecision::Abort => None,
}
}

/// Determines how a command should be sandboxed, prompting the user when
/// policy requires explicit approval.
#[allow(clippy::too_many_arguments)]
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/landlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ where
}

/// Converts the sandbox policy into the CLI invocation for `codex-linux-sandbox`.
fn create_linux_sandbox_command_args(
pub(crate) fn create_linux_sandbox_command_args(
command: Vec<String>,
sandbox_policy: &SandboxPolicy,
sandbox_policy_cwd: &Path,
Expand Down
4 changes: 2 additions & 2 deletions codex-rs/core/src/seatbelt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const MACOS_SEATBELT_BASE_POLICY: &str = include_str!("seatbelt_base_policy.sbpl
/// to defend against an attacker trying to inject a malicious version on the
/// PATH. If /usr/bin/sandbox-exec has been tampered with, then the attacker
/// already has root access.
const MACOS_PATH_TO_SEATBELT_EXECUTABLE: &str = "/usr/bin/sandbox-exec";
pub(crate) const MACOS_PATH_TO_SEATBELT_EXECUTABLE: &str = "/usr/bin/sandbox-exec";

pub async fn spawn_command_under_seatbelt(
command: Vec<String>,
Expand All @@ -39,7 +39,7 @@ pub async fn spawn_command_under_seatbelt(
.await
}

fn create_seatbelt_command_args(
pub(crate) fn create_seatbelt_command_args(
command: Vec<String>,
sandbox_policy: &SandboxPolicy,
sandbox_policy_cwd: &Path,
Expand Down
21 changes: 19 additions & 2 deletions codex-rs/core/src/tools/handlers/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,13 @@ impl ToolHandler for UnifiedExecHandler {

async fn handle(&self, invocation: ToolInvocation) -> Result<ToolOutput, FunctionCallError> {
let ToolInvocation {
session, payload, ..
session,
turn,
sub_id,
call_id,
tool_name,
payload,
..
} = invocation;

let args = match payload {
Expand Down Expand Up @@ -79,7 +85,18 @@ impl ToolHandler for UnifiedExecHandler {
};

let value = session
.run_unified_exec_request(request)
.services
.unified_exec_manager
.handle_request(
request,
crate::unified_exec::UnifiedExecContext {
session: &session,
turn: turn.as_ref(),
sub_id: &sub_id,
call_id: &call_id,
tool_name: &tool_name,
},
)
.await
.map_err(|err| {
FunctionCallError::RespondToModel(format!("unified exec failed: {err:?}"))
Expand Down
Loading
Loading