mirror of
https://github.com/openai/codex.git
synced 2026-04-28 02:11:08 +03:00
chore: update unified exec sandboxing detection (#7541)
No integration test for now because it would make them flaky. Tracking it in my todos to add some once we have a clock based system for integration tests
This commit is contained in:
@@ -485,6 +485,19 @@ pub struct ExecToolCallOutput {
|
||||
pub timed_out: bool,
|
||||
}
|
||||
|
||||
impl Default for ExecToolCallOutput {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
exit_code: 0,
|
||||
stdout: StreamOutput::new(String::new()),
|
||||
stderr: StreamOutput::new(String::new()),
|
||||
aggregated_output: StreamOutput::new(String::new()),
|
||||
duration: Duration::ZERO,
|
||||
timed_out: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg_attr(not(target_os = "windows"), allow(unused_variables))]
|
||||
async fn exec(
|
||||
params: ExecParams,
|
||||
|
||||
@@ -590,7 +590,7 @@ mod tests {
|
||||
assert_eq!(entries.len(), 1);
|
||||
assert_eq!(entries[0].text, long_entry);
|
||||
|
||||
let pruned_len = std::fs::metadata(&history_path).expect("metadata").len() as u64;
|
||||
let pruned_len = std::fs::metadata(&history_path).expect("metadata").len();
|
||||
let max_bytes = config
|
||||
.history
|
||||
.max_bytes
|
||||
|
||||
@@ -149,7 +149,7 @@ impl UnifiedExecSession {
|
||||
guard.snapshot()
|
||||
}
|
||||
|
||||
fn sandbox_type(&self) -> SandboxType {
|
||||
pub(crate) fn sandbox_type(&self) -> SandboxType {
|
||||
self.sandbox_type
|
||||
}
|
||||
|
||||
@@ -172,10 +172,8 @@ impl UnifiedExecSession {
|
||||
let exec_output = ExecToolCallOutput {
|
||||
exit_code,
|
||||
stdout: StreamOutput::new(aggregated_text.clone()),
|
||||
stderr: StreamOutput::new(String::new()),
|
||||
aggregated_output: StreamOutput::new(aggregated_text.clone()),
|
||||
duration: Duration::ZERO,
|
||||
timed_out: false,
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
if is_likely_sandbox_denied(self.sandbox_type(), &exec_output) {
|
||||
@@ -184,7 +182,7 @@ impl UnifiedExecSession {
|
||||
TruncationPolicy::Tokens(UNIFIED_EXEC_OUTPUT_MAX_TOKENS),
|
||||
);
|
||||
let message = if snippet.is_empty() {
|
||||
format!("exit code {exit_code}")
|
||||
format!("Session creation failed with exit code {exit_code}")
|
||||
} else {
|
||||
snippet
|
||||
};
|
||||
@@ -205,10 +203,7 @@ impl UnifiedExecSession {
|
||||
} = spawned;
|
||||
let managed = Self::new(session, output_rx, sandbox_type);
|
||||
|
||||
let exit_ready = match exit_rx.try_recv() {
|
||||
Ok(_) | Err(TryRecvError::Closed) => true,
|
||||
Err(TryRecvError::Empty) => false,
|
||||
};
|
||||
let exit_ready = matches!(exit_rx.try_recv(), Ok(_) | Err(TryRecvError::Closed));
|
||||
|
||||
if exit_ready {
|
||||
managed.signal_exit();
|
||||
@@ -216,7 +211,7 @@ impl UnifiedExecSession {
|
||||
return Ok(managed);
|
||||
}
|
||||
|
||||
if tokio::time::timeout(Duration::from_millis(50), &mut exit_rx)
|
||||
if tokio::time::timeout(Duration::from_millis(150), &mut exit_rx)
|
||||
.await
|
||||
.is_ok()
|
||||
{
|
||||
|
||||
@@ -153,6 +153,7 @@ impl UnifiedExecSessionManager {
|
||||
let output = formatted_truncate_text(&text, TruncationPolicy::Tokens(max_tokens));
|
||||
let has_exited = session.has_exited();
|
||||
let exit_code = session.exit_code();
|
||||
let sandbox_type = session.sandbox_type();
|
||||
let chunk_id = generate_chunk_id();
|
||||
let process_id = if has_exited {
|
||||
None
|
||||
@@ -201,6 +202,9 @@ impl UnifiedExecSessionManager {
|
||||
Some(request.process_id),
|
||||
)
|
||||
.await;
|
||||
|
||||
// Exit code should always be Some
|
||||
sandboxing::check_sandboxing(sandbox_type, &text, exit_code.unwrap_or_default())?;
|
||||
}
|
||||
|
||||
Ok(response)
|
||||
@@ -703,6 +707,39 @@ impl UnifiedExecSessionManager {
|
||||
}
|
||||
}
|
||||
|
||||
mod sandboxing {
|
||||
use super::*;
|
||||
use crate::exec::SandboxType;
|
||||
use crate::exec::is_likely_sandbox_denied;
|
||||
use crate::unified_exec::UNIFIED_EXEC_OUTPUT_MAX_TOKENS;
|
||||
|
||||
pub(crate) fn check_sandboxing(
|
||||
sandbox_type: SandboxType,
|
||||
text: &str,
|
||||
exit_code: i32,
|
||||
) -> Result<(), UnifiedExecError> {
|
||||
let exec_output = ExecToolCallOutput {
|
||||
exit_code,
|
||||
stderr: StreamOutput::new(text.to_string()),
|
||||
aggregated_output: StreamOutput::new(text.to_string()),
|
||||
..Default::default()
|
||||
};
|
||||
if is_likely_sandbox_denied(sandbox_type, &exec_output) {
|
||||
let snippet = formatted_truncate_text(
|
||||
text,
|
||||
TruncationPolicy::Tokens(UNIFIED_EXEC_OUTPUT_MAX_TOKENS),
|
||||
);
|
||||
let message = if snippet.is_empty() {
|
||||
format!("Session exited with code {exit_code}")
|
||||
} else {
|
||||
snippet
|
||||
};
|
||||
return Err(UnifiedExecError::sandbox_denied(message, exec_output));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
enum SessionStatus {
|
||||
Alive {
|
||||
exit_code: Option<i32>,
|
||||
|
||||
Reference in New Issue
Block a user