mirror of
https://github.com/openai/codex.git
synced 2026-11-06 20:20:34 +00:00
Record sandbox backends in command execution analytics (#47568)
## Why Command execution analytics lack the observed sandbox backend. Older execution servers may omit this information, so an unknown backend must remain distinguishable from an explicitly unsandboxed process. ## What changed - Carry optional `sandbox_type` metadata through command execution items and emit its metric tag as `sandbox_backend` in analytics. - Propagate the observed backend through unified execution completion and failure events, including asynchronous exits. - Preserve missing backend information as `None` and mark executed user shell commands as `Some(SandboxType::None)`. - Exclude the metadata from serialized command items and generated schemas and TypeScript types. ## Testing Extend tests for missing and explicit executor backends, backend propagation on failed execution, and analytics payloads, including platform-specific backend tags in app-server integration coverage. GitOrigin-RevId: 178fe46824a6194910802ec02168ed40ee0b2019
This commit is contained in:
@@ -202,6 +202,7 @@ pub(crate) fn spawn_exit_watcher(
|
||||
if let Some(message) = process.failure_message() {
|
||||
drop(plugin_metrics_sidecar);
|
||||
emit_failed_exec_end_for_unified_exec(
|
||||
process.sandbox_type(),
|
||||
session_ref,
|
||||
turn_ref,
|
||||
model_info,
|
||||
@@ -229,6 +230,7 @@ pub(crate) fn spawn_exit_watcher(
|
||||
)
|
||||
.await;
|
||||
emit_exec_end_for_unified_exec(
|
||||
process.sandbox_type(),
|
||||
session_ref,
|
||||
turn_ref,
|
||||
model_info,
|
||||
@@ -342,6 +344,7 @@ impl Emitter {
|
||||
/// text when the transcript is empty.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) async fn emit_exec_end_for_unified_exec(
|
||||
sandbox_type: Option<codex_protocol::sandbox::SandboxType>,
|
||||
session_ref: Arc<Session>,
|
||||
turn_ref: Arc<TurnContext>,
|
||||
model_info: Arc<ModelInfo>,
|
||||
@@ -365,13 +368,14 @@ pub(crate) async fn emit_exec_end_for_unified_exec(
|
||||
duration,
|
||||
timed_out,
|
||||
};
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
let mut event_ctx = ToolEventCtx::new(
|
||||
session_ref.as_ref(),
|
||||
turn_ref.as_ref(),
|
||||
&model_info,
|
||||
&call_id,
|
||||
/*turn_diff_tracker*/ None,
|
||||
);
|
||||
event_ctx.sandbox_type = sandbox_type;
|
||||
let emitter = ToolEmitter::unified_exec(
|
||||
&command,
|
||||
cwd,
|
||||
@@ -392,6 +396,7 @@ pub(crate) async fn emit_exec_end_for_unified_exec(
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) async fn emit_failed_exec_end_for_unified_exec(
|
||||
sandbox_type: Option<codex_protocol::sandbox::SandboxType>,
|
||||
session_ref: Arc<Session>,
|
||||
turn_ref: Arc<TurnContext>,
|
||||
model_info: Arc<ModelInfo>,
|
||||
@@ -423,13 +428,14 @@ pub(crate) async fn emit_failed_exec_end_for_unified_exec(
|
||||
duration,
|
||||
timed_out: false,
|
||||
};
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
let mut event_ctx = ToolEventCtx::new(
|
||||
session_ref.as_ref(),
|
||||
turn_ref.as_ref(),
|
||||
&model_info,
|
||||
&call_id,
|
||||
/*turn_diff_tracker*/ None,
|
||||
);
|
||||
event_ctx.sandbox_type = sandbox_type;
|
||||
let emitter = ToolEmitter::unified_exec(
|
||||
&command,
|
||||
cwd,
|
||||
|
||||
@@ -97,7 +97,7 @@ pub(crate) struct UnifiedExecProcess {
|
||||
state_tx: watch::Sender<ProcessState>,
|
||||
state_rx: watch::Receiver<ProcessState>,
|
||||
output_task: Option<JoinHandle<()>>,
|
||||
sandbox_type: SandboxType,
|
||||
sandbox_type: Option<SandboxType>,
|
||||
timed_out: AtomicBool,
|
||||
_spawn_lifecycle: Option<SpawnLifecycleHandle>,
|
||||
// The shell may still need to replay this file after process startup returns.
|
||||
@@ -117,7 +117,7 @@ impl std::fmt::Debug for UnifiedExecProcess {
|
||||
impl UnifiedExecProcess {
|
||||
fn new(
|
||||
process_handle: ProcessHandle,
|
||||
sandbox_type: SandboxType,
|
||||
sandbox_type: Option<SandboxType>,
|
||||
spawn_lifecycle: Option<SpawnLifecycleHandle>,
|
||||
) -> Self {
|
||||
let output = OutputHandles {
|
||||
@@ -281,7 +281,7 @@ impl UnifiedExecProcess {
|
||||
guard.to_bytes()
|
||||
}
|
||||
|
||||
pub(crate) fn sandbox_type(&self) -> SandboxType {
|
||||
pub(crate) fn sandbox_type(&self) -> Option<SandboxType> {
|
||||
self.sandbox_type
|
||||
}
|
||||
|
||||
@@ -309,7 +309,7 @@ impl UnifiedExecProcess {
|
||||
text: &str,
|
||||
) -> Result<(), UnifiedExecError> {
|
||||
let executor_reported_denial = self.state_rx.borrow().sandbox_denied;
|
||||
let sandbox_type = self.sandbox_type();
|
||||
let sandbox_type = self.sandbox_type().unwrap_or(SandboxType::None);
|
||||
if !self.has_exited() || (!executor_reported_denial && sandbox_type == SandboxType::None) {
|
||||
return Ok(());
|
||||
}
|
||||
@@ -354,7 +354,7 @@ impl UnifiedExecProcess {
|
||||
let output_rx = codex_utils_pty::combine_output_receivers(stdout_rx, stderr_rx);
|
||||
let mut managed = Self::new(
|
||||
ProcessHandle::Local(Box::new(process_handle)),
|
||||
sandbox_type,
|
||||
Some(sandbox_type),
|
||||
Some(spawn_lifecycle),
|
||||
);
|
||||
managed.output_task = Some(Self::spawn_local_output_task(
|
||||
@@ -403,7 +403,7 @@ impl UnifiedExecProcess {
|
||||
let process_handle = ProcessHandle::ExecServer(Arc::clone(&started.process));
|
||||
// Older peers do not report this field. In that case, skip local
|
||||
// classification rather than attributing a violation to a guessed backend.
|
||||
let sandbox_type = started.sandbox_type.unwrap_or(SandboxType::None);
|
||||
let sandbox_type = started.sandbox_type;
|
||||
let mut managed = Self::new(process_handle, sandbox_type, /*spawn_lifecycle*/ None);
|
||||
let output_handles = managed.output_handles().clone();
|
||||
managed.output_task = Some(Self::spawn_exec_server_output_task(
|
||||
|
||||
@@ -394,6 +394,7 @@ fn fail_process_with_message(process: &UnifiedExecProcess, message: String) -> U
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn emit_failed_initial_exec_end_if_unstored(
|
||||
process_started_alive: bool,
|
||||
sandbox_type: Option<codex_protocol::sandbox::SandboxType>,
|
||||
context: &UnifiedExecContext,
|
||||
request: &ExecCommandRequest,
|
||||
cwd: PathUri,
|
||||
@@ -408,6 +409,7 @@ async fn emit_failed_initial_exec_end_if_unstored(
|
||||
}
|
||||
|
||||
emit_failed_exec_end_for_unified_exec(
|
||||
sandbox_type,
|
||||
Arc::clone(&context.session),
|
||||
Arc::clone(&context.step_context.turn),
|
||||
Arc::clone(&context.step_context.settings.model_info),
|
||||
@@ -677,6 +679,7 @@ impl UnifiedExecProcessManager {
|
||||
.await;
|
||||
emit_failed_initial_exec_end_if_unstored(
|
||||
process_started_alive,
|
||||
process.sandbox_type(),
|
||||
context,
|
||||
&request,
|
||||
cwd.clone(),
|
||||
@@ -698,6 +701,7 @@ impl UnifiedExecProcessManager {
|
||||
.await;
|
||||
emit_failed_initial_exec_end_if_unstored(
|
||||
process_started_alive,
|
||||
process.sandbox_type(),
|
||||
context,
|
||||
&request,
|
||||
cwd.clone(),
|
||||
@@ -776,6 +780,7 @@ impl UnifiedExecProcessManager {
|
||||
if let Err(message) = finish_result {
|
||||
emit_failed_initial_exec_end_if_unstored(
|
||||
process_started_alive,
|
||||
process.sandbox_type(),
|
||||
context,
|
||||
&request,
|
||||
cwd.clone(),
|
||||
@@ -795,6 +800,7 @@ impl UnifiedExecProcessManager {
|
||||
.finish_plugin_metrics(context, exit)
|
||||
.await;
|
||||
emit_exec_end_for_unified_exec(
|
||||
process.sandbox_type(),
|
||||
Arc::clone(&context.session),
|
||||
Arc::clone(&context.step_context.turn),
|
||||
Arc::clone(&context.step_context.settings.model_info),
|
||||
|
||||
@@ -482,6 +482,7 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() {
|
||||
|
||||
emit_failed_initial_exec_end_if_unstored(
|
||||
/*process_started_alive*/ false,
|
||||
Some(codex_protocol::sandbox::SandboxType::WindowsMxc),
|
||||
&context,
|
||||
&request,
|
||||
#[allow(deprecated)]
|
||||
@@ -505,6 +506,10 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() {
|
||||
panic!("expected CommandExecution item");
|
||||
};
|
||||
assert_eq!(item.id, "call-unified-denied");
|
||||
assert_eq!(
|
||||
item.sandbox_type,
|
||||
Some(codex_protocol::sandbox::SandboxType::WindowsMxc)
|
||||
);
|
||||
assert_eq!(
|
||||
item.status,
|
||||
codex_protocol::items::CommandExecutionStatus::Failed
|
||||
|
||||
@@ -88,7 +88,7 @@ impl ExecProcess for MockExecProcess {
|
||||
pub(super) async fn remote_process(
|
||||
write_status: WriteStatus,
|
||||
terminate_error: Option<String>,
|
||||
sandbox_type: codex_sandboxing::SandboxType,
|
||||
sandbox_type: impl Into<Option<codex_sandboxing::SandboxType>>,
|
||||
) -> UnifiedExecProcess {
|
||||
let (wake_tx, _wake_rx) = watch::channel(0);
|
||||
let started = StartedExecProcess {
|
||||
@@ -101,7 +101,7 @@ pub(super) async fn remote_process(
|
||||
terminate_error,
|
||||
wake_tx,
|
||||
}),
|
||||
sandbox_type: Some(sandbox_type),
|
||||
sandbox_type: sandbox_type.into(),
|
||||
};
|
||||
|
||||
UnifiedExecProcess::from_exec_server_started(started)
|
||||
@@ -198,15 +198,18 @@ async fn remote_terminate_confirmed_updates_state_on_success_only() {
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_process_preserves_executor_sandbox_type() {
|
||||
let process = remote_process(
|
||||
WriteStatus::Accepted,
|
||||
/*terminate_error*/ None,
|
||||
codex_sandboxing::SandboxType::LinuxSeccomp,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
process.sandbox_type(),
|
||||
codex_sandboxing::SandboxType::LinuxSeccomp
|
||||
);
|
||||
use codex_sandboxing::SandboxType;
|
||||
for sandbox_type in [
|
||||
None,
|
||||
Some(SandboxType::None),
|
||||
Some(SandboxType::LinuxSeccomp),
|
||||
] {
|
||||
let process = remote_process(
|
||||
WriteStatus::Accepted,
|
||||
/*terminate_error*/ None,
|
||||
sandbox_type,
|
||||
)
|
||||
.await;
|
||||
assert_eq!(process.sandbox_type(), sandbox_type);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user