mirror of
https://github.com/openai/codex.git
synced 2026-11-08 20:40:33 +00:00
Carry prepared commands through unified execution (#57438)
## What changed Pass `PreparedExecCommand` through the process manager and runtime instead of copying command, shell, environment, and permission fields into separate request structs. Move approval requirement evaluation and approval action construction onto the prepared command, and use it for shell snapshots and launch failure events. GitOrigin-RevId: 3e9f87e0974218fea6fffc0f7740616d696b9800
This commit is contained in:
@@ -29,7 +29,6 @@ use std::sync::Arc;
|
||||
use std::sync::Weak;
|
||||
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_utils_output_truncation::TruncationPolicy;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use rand::Rng;
|
||||
@@ -37,12 +36,10 @@ use rand::rng;
|
||||
use tokio::sync::Mutex;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::step_context::StepContext;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::shell::ShellInvocation;
|
||||
use crate::tools::handlers::unified_exec::preparation::PreparedExecCommand;
|
||||
use crate::tools::network_approval::DeferredNetworkApproval;
|
||||
use codex_core_plugins::PluginMetricsSidecar;
|
||||
|
||||
@@ -108,22 +105,9 @@ impl UnifiedExecContext {
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(crate) struct ExecCommandRequest {
|
||||
pub command: Vec<String>,
|
||||
pub shell: ShellInvocation,
|
||||
pub hook_command: String,
|
||||
pub prepared: PreparedExecCommand,
|
||||
pub process_id: i32,
|
||||
pub yield_time_ms: u64,
|
||||
pub max_output_tokens: Option<usize>,
|
||||
pub cwd: PathUri,
|
||||
pub sandbox_cwd: PathUri,
|
||||
pub turn_environment: TurnEnvironment,
|
||||
pub network: Option<NetworkProxy>,
|
||||
pub tty: bool,
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
pub additional_permissions_preapproved: bool,
|
||||
pub justification: Option<String>,
|
||||
pub prefix_rule: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
/// The process and environment behind a terminal session ID at the time of lookup.
|
||||
|
||||
@@ -4,6 +4,7 @@ use crate::codex_thread::BackgroundTerminalInfo;
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecExpiration;
|
||||
use crate::sandboxing::ExecRequest;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::tests::make_session_and_context;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
|
||||
@@ -25,7 +25,6 @@ use crate::exec_env::inject_apply_patch_env;
|
||||
use crate::exec_env::inject_permission_profile_env;
|
||||
use crate::exec_env::inject_session_env;
|
||||
use crate::exec_env::set_tool_call_id_env_var;
|
||||
use crate::exec_policy::ExecApprovalRequest;
|
||||
use crate::guardian::GuardianReviewContext;
|
||||
use crate::plugins::metrics::finish_and_track_measurements;
|
||||
use crate::sandboxing::ExecOptions;
|
||||
@@ -71,7 +70,6 @@ use crate::unified_exec::process::UnifiedExecProcess;
|
||||
use crate::unified_exec::shell_snapshot::shell_snapshot_request;
|
||||
use crate::unified_exec::take_plugin_metrics_sidecar;
|
||||
use crate::unified_exec::trace_id;
|
||||
use crate::windows_sandbox::windows_sandbox_level_for_legacy_checks;
|
||||
use codex_core_plugins::PLUGIN_METRICS_OUTPUT_ENV_VAR;
|
||||
use codex_core_plugins::PluginCommandAttribution;
|
||||
use codex_core_plugins::PluginMetricsSidecar;
|
||||
@@ -87,7 +85,6 @@ use codex_protocol::protocol::ExecCommandSource;
|
||||
use codex_protocol::protocol::TerminalInteractionEvent;
|
||||
use codex_protocol::shell_environment::is_non_inheritable_env_var;
|
||||
use codex_sandboxing::SandboxCommand;
|
||||
use codex_shell_command::is_dangerous_command::DangerousCommandPlatform;
|
||||
use codex_tools::ToolName;
|
||||
use codex_utils_head_tail_buffer::HeadTailBuffer;
|
||||
use codex_utils_output_truncation::approx_tokens_from_byte_count;
|
||||
@@ -416,7 +413,7 @@ async fn emit_failed_initial_exec_end_if_unstored(
|
||||
Arc::clone(&context.step_context.turn),
|
||||
Arc::clone(&context.step_context.settings.model_info),
|
||||
context.call_id.clone(),
|
||||
request.command.clone(),
|
||||
request.prepared.resolved.command.clone(),
|
||||
cwd,
|
||||
Some(request.process_id.to_string()),
|
||||
plugin_attribution,
|
||||
@@ -543,10 +540,10 @@ impl UnifiedExecProcessManager {
|
||||
context: &UnifiedExecContext,
|
||||
mut completion: Option<&mut Completion<'_>>,
|
||||
) -> Result<ExecCommandToolOutput, UnifiedExecError> {
|
||||
let cwd = request.cwd.clone();
|
||||
let process = self
|
||||
.open_session_with_sandbox(&request, cwd.clone(), context)
|
||||
.await;
|
||||
let prepared = &request.prepared;
|
||||
let command = &prepared.resolved.command;
|
||||
let cwd = prepared.cwd.clone();
|
||||
let process = self.open_session_with_sandbox(&request, context).await;
|
||||
|
||||
let (attempt, mut deferred_network_approval) = match process {
|
||||
Ok((attempt, deferred_network_approval)) => (attempt, deferred_network_approval),
|
||||
@@ -582,27 +579,23 @@ impl UnifiedExecProcessManager {
|
||||
/*turn_diff_tracker*/ None,
|
||||
);
|
||||
event_ctx.model_context = Some(&model_context);
|
||||
let plugin_attribution = if request.turn_environment.environment.is_remote() {
|
||||
let file_system = request.turn_environment.environment.get_filesystem();
|
||||
let plugin_attribution = if prepared.turn_environment.environment.is_remote() {
|
||||
let file_system = prepared.turn_environment.environment.get_filesystem();
|
||||
context
|
||||
.step_context
|
||||
.turn
|
||||
.plugin_attribution_for_executor_command(
|
||||
&request.command,
|
||||
&cwd,
|
||||
file_system.as_ref(),
|
||||
)
|
||||
.plugin_attribution_for_executor_command(command, &cwd, file_system.as_ref())
|
||||
.await
|
||||
} else {
|
||||
cwd.to_abs_path().ok().and_then(|cwd| {
|
||||
context
|
||||
.step_context
|
||||
.turn
|
||||
.plugin_attribution_for_command(&request.command, &cwd)
|
||||
.plugin_attribution_for_command(command, &cwd)
|
||||
})
|
||||
};
|
||||
let emitter = ToolEmitter::unified_exec(
|
||||
&request.command,
|
||||
command,
|
||||
cwd.clone(),
|
||||
ExecCommandSource::UnifiedExecStartup,
|
||||
Some(request.process_id.to_string()),
|
||||
@@ -637,15 +630,15 @@ impl UnifiedExecProcessManager {
|
||||
self.store_process(
|
||||
Arc::clone(&process),
|
||||
context,
|
||||
&request.command,
|
||||
request.hook_command.clone(),
|
||||
command,
|
||||
prepared.hook_command.clone(),
|
||||
cwd.clone(),
|
||||
request.turn_environment.selection.environment_id.clone(),
|
||||
prepared.turn_environment.selection.environment_id.clone(),
|
||||
permissions,
|
||||
plugin_attribution.clone(),
|
||||
start,
|
||||
request.process_id,
|
||||
request.tty,
|
||||
prepared.tty,
|
||||
deferred_network_approval.clone(),
|
||||
network_denial_monitor,
|
||||
initial_exec_command_guard.metrics_sidecar.take(),
|
||||
@@ -655,7 +648,7 @@ impl UnifiedExecProcessManager {
|
||||
.await;
|
||||
}
|
||||
|
||||
let yield_time_ms = clamp_yield_time(request.yield_time_ms);
|
||||
let yield_time_ms = clamp_yield_time(prepared.yield_time_ms);
|
||||
// For the initial exec_command call, we both stream output to events
|
||||
// (via start_streaming_output above) and collect a snapshot here for
|
||||
// the tool response body.
|
||||
@@ -837,7 +830,7 @@ impl UnifiedExecProcessManager {
|
||||
Arc::clone(&context.step_context.turn),
|
||||
Arc::clone(&context.step_context.settings.model_info),
|
||||
context.call_id.clone(),
|
||||
request.command.clone(),
|
||||
command.clone(),
|
||||
cwd.clone(),
|
||||
Some(process_id.to_string()),
|
||||
plugin_attribution.clone(),
|
||||
@@ -870,13 +863,13 @@ impl UnifiedExecProcessManager {
|
||||
.model_info
|
||||
.truncation_policy
|
||||
.into(),
|
||||
max_output_tokens: request.max_output_tokens,
|
||||
max_output_tokens: prepared.max_output_tokens,
|
||||
process_id: response_process_id,
|
||||
exit_code,
|
||||
original_token_count: Some(original_token_count),
|
||||
hook_command: Some(request.hook_command.clone()),
|
||||
tool_cwd: (!request.turn_environment.environment.is_remote())
|
||||
.then(|| request.cwd.clone()),
|
||||
hook_command: Some(prepared.hook_command.clone()),
|
||||
tool_cwd: (!prepared.turn_environment.environment.is_remote())
|
||||
.then(|| prepared.cwd.clone()),
|
||||
};
|
||||
|
||||
Ok(response)
|
||||
@@ -1489,10 +1482,10 @@ impl UnifiedExecProcessManager {
|
||||
pub(super) async fn open_session_with_sandbox(
|
||||
&self,
|
||||
request: &ExecCommandRequest,
|
||||
cwd: PathUri,
|
||||
context: &UnifiedExecContext,
|
||||
) -> Result<(UnifiedExecAttempt, Option<DeferredNetworkApproval>), UnifiedExecError> {
|
||||
let shell_environment_policy = request.turn_environment.shell_environment_policy();
|
||||
let prepared = &request.prepared;
|
||||
let shell_environment_policy = prepared.turn_environment.shell_environment_policy();
|
||||
let local_policy_env = create_env(shell_environment_policy, /*thread_id*/ None);
|
||||
let mut env = local_policy_env.clone();
|
||||
#[cfg(windows)]
|
||||
@@ -1504,7 +1497,7 @@ impl UnifiedExecProcessManager {
|
||||
set_tool_call_id_env_var(&mut env, Some(&context.call_id));
|
||||
inject_session_env(&mut env, context.session.session_id());
|
||||
inject_apply_patch_env(&mut env);
|
||||
let active_permission_profile = request.turn_environment.active_permission_profile();
|
||||
let active_permission_profile = prepared.turn_environment.active_permission_profile();
|
||||
inject_permission_profile_env(&mut env, active_permission_profile.as_ref());
|
||||
let mut env = apply_unified_exec_env(env);
|
||||
strip_output_env(&mut env);
|
||||
@@ -1514,65 +1507,20 @@ impl UnifiedExecProcessManager {
|
||||
policy: exec_env_policy_from_shell_policy(shell_environment_policy),
|
||||
local_policy_env,
|
||||
};
|
||||
let shell_snapshot = shell_snapshot_request(request, &cwd, context);
|
||||
let shell_snapshot = shell_snapshot_request(prepared, context);
|
||||
let mut orchestrator = ToolOrchestrator::new();
|
||||
let mut runtime = UnifiedExecRuntime::new(self);
|
||||
let session_shell = context.session.user_shell();
|
||||
let configured_shell = request
|
||||
.turn_environment
|
||||
.shell
|
||||
.as_ref()
|
||||
.unwrap_or(session_shell.as_ref());
|
||||
// Legacy executors omit the platform. Unknown reported platforms use the
|
||||
// more conservative Windows rules rather than silently weakening checks.
|
||||
let command_platform = match request.turn_environment.executor_platform_os.as_deref() {
|
||||
Some("linux" | "macos") => DangerousCommandPlatform::Posix,
|
||||
Some("windows") | Some(_) => DangerousCommandPlatform::Windows,
|
||||
None => DangerousCommandPlatform::host(),
|
||||
};
|
||||
let exec_approval_requirement = context
|
||||
.session
|
||||
.services
|
||||
.exec_policy
|
||||
.create_exec_approval_requirement_for_shell(
|
||||
ExecApprovalRequest {
|
||||
command: &request.command,
|
||||
approval_policy: context.step_context.settings.approval_policy(),
|
||||
permission_profile: request.turn_environment.permission_profile().clone(),
|
||||
environment_policy: request.turn_environment.config().exec_policy.as_ref(),
|
||||
windows_sandbox_level: windows_sandbox_level_for_legacy_checks(
|
||||
request.turn_environment.config().windows_sandbox_type,
|
||||
request.turn_environment.config().windows_sandbox_level,
|
||||
),
|
||||
sandbox_permissions: if request.additional_permissions_preapproved {
|
||||
crate::sandboxing::SandboxPermissions::UseDefault
|
||||
} else {
|
||||
request.sandbox_permissions
|
||||
},
|
||||
prefix_rule: request.prefix_rule.clone(),
|
||||
allow_prefix_rules: context.step_context.turn.allow_prefix_rules(),
|
||||
},
|
||||
configured_shell,
|
||||
command_platform,
|
||||
)
|
||||
let exec_approval_requirement = prepared
|
||||
.approval_requirement(&context.session, &context.step_context)
|
||||
.await;
|
||||
let req = UnifiedExecToolRequest {
|
||||
command: request.command.clone(),
|
||||
shell: request.shell.clone(),
|
||||
hook_command: request.hook_command.clone(),
|
||||
prepared,
|
||||
process_id: request.process_id,
|
||||
cwd,
|
||||
sandbox_cwd: request.sandbox_cwd.clone(),
|
||||
turn_environment: request.turn_environment.clone(),
|
||||
env,
|
||||
exec_server_env_config: Some(exec_server_env_config),
|
||||
shell_snapshot,
|
||||
explicit_env_overrides,
|
||||
network: request.network.clone(),
|
||||
tty: request.tty,
|
||||
sandbox_permissions: request.sandbox_permissions,
|
||||
additional_permissions: request.additional_permissions.clone(),
|
||||
justification: request.justification.clone(),
|
||||
exec_approval_requirement,
|
||||
};
|
||||
let tool_ctx = ToolCtx {
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
use super::*;
|
||||
use crate::shell::Shell;
|
||||
use crate::shell::ShellInvocation;
|
||||
use crate::shell::ShellType;
|
||||
use crate::tools::handlers::unified_exec::preparation::ExecCommandLifetime;
|
||||
use crate::tools::handlers::unified_exec::preparation::ParsedExecCommand;
|
||||
use crate::unified_exec::clamp_yield_time;
|
||||
use codex_exec_server::ExecOutputStream;
|
||||
use codex_network_proxy::ManagedNetworkSandboxContext;
|
||||
@@ -464,39 +463,23 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() {
|
||||
tokio_util::sync::CancellationToken::new(),
|
||||
"call-unified-denied".to_string(),
|
||||
);
|
||||
let environment = turn
|
||||
.initial_environments
|
||||
.primary()
|
||||
.expect("primary environment");
|
||||
let prepared = ParsedExecCommand::parse(
|
||||
r#"{"cmd":"echo before"}"#,
|
||||
&session,
|
||||
&context.step_context,
|
||||
environment,
|
||||
)
|
||||
.expect("parse command")
|
||||
.prepare(ExecCommandLifetime::Interactive)
|
||||
.expect("prepare command");
|
||||
let request = ExecCommandRequest {
|
||||
command: vec![
|
||||
"sh".to_string(),
|
||||
"-lc".to_string(),
|
||||
"echo before".to_string(),
|
||||
],
|
||||
shell: ShellInvocation {
|
||||
shell: Shell {
|
||||
shell_type: ShellType::Sh,
|
||||
shell_path: "sh".into(),
|
||||
},
|
||||
use_login_shell: true,
|
||||
},
|
||||
hook_command: "echo before".to_string(),
|
||||
prepared,
|
||||
process_id: 123,
|
||||
yield_time_ms: 1000,
|
||||
max_output_tokens: None,
|
||||
#[allow(deprecated)]
|
||||
cwd: turn.cwd.clone().into(),
|
||||
#[allow(deprecated)]
|
||||
sandbox_cwd: turn.cwd.clone().into(),
|
||||
turn_environment: turn
|
||||
.initial_environments
|
||||
.primary()
|
||||
.cloned()
|
||||
.expect("primary environment"),
|
||||
network: None,
|
||||
tty: true,
|
||||
sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault,
|
||||
additional_permissions: None,
|
||||
additional_permissions_preapproved: false,
|
||||
justification: None,
|
||||
prefix_rule: None,
|
||||
};
|
||||
|
||||
let output_buffer = Arc::new(tokio::sync::Mutex::new(OutputBuffers::default()));
|
||||
@@ -510,8 +493,7 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() {
|
||||
Some(codex_protocol::sandbox::SandboxType::WindowsMxc),
|
||||
&context,
|
||||
&request,
|
||||
#[allow(deprecated)]
|
||||
turn.cwd.clone().into(),
|
||||
request.prepared.cwd.clone(),
|
||||
/*plugin_attribution*/ None,
|
||||
output_buffer,
|
||||
"PRE_DENIAL_MARKER".to_string(),
|
||||
|
||||
@@ -11,11 +11,9 @@ use codex_protocol::sandbox::SandboxOverride;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxablePreference;
|
||||
use codex_tools::ToolName;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use tokio_util::task::AbortOnDropHandle;
|
||||
use uuid::Uuid;
|
||||
|
||||
use super::ExecCommandRequest;
|
||||
use super::UnifiedExecContext;
|
||||
use super::process_manager::apply_unified_exec_env;
|
||||
use super::process_manager::exec_env_policy_from_shell_policy;
|
||||
@@ -27,6 +25,7 @@ use crate::exec_env::inject_session_env;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::step_context::StepContext;
|
||||
use crate::shell::ShellType;
|
||||
use crate::tools::handlers::unified_exec::preparation::PreparedExecCommand;
|
||||
use crate::tools::sandboxing::executor_windows_sandbox_selection;
|
||||
|
||||
impl Session {
|
||||
@@ -168,19 +167,18 @@ impl Session {
|
||||
}
|
||||
|
||||
pub(super) fn shell_snapshot_request(
|
||||
request: &ExecCommandRequest,
|
||||
cwd: &PathUri,
|
||||
request: &PreparedExecCommand,
|
||||
context: &UnifiedExecContext,
|
||||
) -> Option<ShellSnapshotRequest> {
|
||||
if !context.session.features().enabled(Feature::ShellSnapshotV2)
|
||||
|| !request.turn_environment.shell_snapshot_v2_supported
|
||||
|| request.turn_environment.selection.cwd != *cwd
|
||||
|| !request.shell.is_posix_login()
|
||||
|| request.turn_environment.selection.cwd != request.cwd
|
||||
|| !request.resolved.shell.is_posix_login()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
let shell = &request.shell.shell;
|
||||
let shell = &request.resolved.shell.shell;
|
||||
Some(ShellSnapshotRequest {
|
||||
scope_id: format!(
|
||||
"{}:{}",
|
||||
|
||||
Reference in New Issue
Block a user