mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
core: add a full-buffer exec capture policy (#15254)
This commit is contained in:
committed by
GitHub
Unverified
parent
0a344e4fab
commit
a3e59e9e85
@@ -203,6 +203,7 @@ use codex_core::config_loader::CloudRequirementsLoader;
|
||||
use codex_core::default_client::set_default_client_residency_requirement;
|
||||
use codex_core::error::CodexErr;
|
||||
use codex_core::error::Result as CodexResult;
|
||||
use codex_core::exec::ExecCapturePolicy;
|
||||
use codex_core::exec::ExecExpiration;
|
||||
use codex_core::exec::ExecParams;
|
||||
use codex_core::exec_env::create_env;
|
||||
@@ -1674,11 +1675,17 @@ impl CodexMessageProcessor {
|
||||
None => ExecExpiration::DefaultTimeout,
|
||||
}
|
||||
};
|
||||
let capture_policy = if disable_output_cap {
|
||||
ExecCapturePolicy::FullBuffer
|
||||
} else {
|
||||
ExecCapturePolicy::ShellTool
|
||||
};
|
||||
let sandbox_cwd = self.config.cwd.clone();
|
||||
let exec_params = ExecParams {
|
||||
command,
|
||||
cwd: cwd.clone(),
|
||||
expiration,
|
||||
capture_policy,
|
||||
env,
|
||||
network: started_network_proxy
|
||||
.as_ref()
|
||||
|
||||
@@ -733,6 +733,7 @@ mod tests {
|
||||
env: HashMap::new(),
|
||||
network: None,
|
||||
expiration: ExecExpiration::DefaultTimeout,
|
||||
capture_policy: codex_core::exec::ExecCapturePolicy::ShellTool,
|
||||
sandbox: SandboxType::WindowsRestrictedToken,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
@@ -845,6 +846,7 @@ mod tests {
|
||||
env: HashMap::new(),
|
||||
network: None,
|
||||
expiration: ExecExpiration::Cancellation(CancellationToken::new()),
|
||||
capture_policy: codex_core::exec::ExecCapturePolicy::ShellTool,
|
||||
sandbox: SandboxType::None,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
|
||||
@@ -7,6 +7,7 @@ use crate::config_loader::ConfigLayerStackOrdering;
|
||||
use crate::config_loader::NetworkConstraints;
|
||||
use crate::config_loader::RequirementSource;
|
||||
use crate::config_loader::Sourced;
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecToolCallOutput;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::mcp_connection_manager::ToolInfo;
|
||||
@@ -4788,6 +4789,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
},
|
||||
cwd: turn_context.cwd.clone(),
|
||||
expiration: timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env: HashMap::new(),
|
||||
network: None,
|
||||
sandbox_permissions,
|
||||
@@ -4805,6 +4807,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
|
||||
command: params.command.clone(),
|
||||
cwd: params.cwd.clone(),
|
||||
expiration: timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env: HashMap::new(),
|
||||
network: None,
|
||||
windows_sandbox_level: turn_context.windows_sandbox_level,
|
||||
|
||||
@@ -3,6 +3,7 @@ use crate::compact::InitialContextInjection;
|
||||
use crate::config_loader::ConfigLayerEntry;
|
||||
use crate::config_loader::ConfigRequirements;
|
||||
use crate::config_loader::ConfigRequirementsToml;
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecParams;
|
||||
use crate::exec_policy::ExecPolicyManager;
|
||||
use crate::features::Feature;
|
||||
@@ -124,6 +125,7 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
|
||||
},
|
||||
cwd: turn_context.cwd.clone(),
|
||||
expiration: expiration_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env: HashMap::new(),
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
|
||||
|
||||
+98
-29
@@ -78,6 +78,7 @@ pub struct ExecParams {
|
||||
pub command: Vec<String>,
|
||||
pub cwd: PathBuf,
|
||||
pub expiration: ExecExpiration,
|
||||
pub capture_policy: ExecCapturePolicy,
|
||||
pub env: HashMap<String, String>,
|
||||
pub network: Option<NetworkProxy>,
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
@@ -87,6 +88,16 @@ pub struct ExecParams {
|
||||
pub arg0: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
|
||||
pub enum ExecCapturePolicy {
|
||||
/// Shell-like execs keep the historical output cap and timeout behavior.
|
||||
#[default]
|
||||
ShellTool,
|
||||
/// Trusted internal helpers can buffer the full child output in memory
|
||||
/// without the shell-oriented output cap or exec-expiration behavior.
|
||||
FullBuffer,
|
||||
}
|
||||
|
||||
fn select_process_exec_tool_sandbox_type(
|
||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
network_sandbox_policy: NetworkSandboxPolicy,
|
||||
@@ -147,6 +158,26 @@ impl ExecExpiration {
|
||||
}
|
||||
}
|
||||
|
||||
impl ExecCapturePolicy {
|
||||
fn retained_bytes_cap(self) -> Option<usize> {
|
||||
match self {
|
||||
Self::ShellTool => Some(EXEC_OUTPUT_MAX_BYTES),
|
||||
Self::FullBuffer => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn io_drain_timeout(self) -> Duration {
|
||||
Duration::from_millis(IO_DRAIN_TIMEOUT_MS)
|
||||
}
|
||||
|
||||
fn uses_expiration(self) -> bool {
|
||||
match self {
|
||||
Self::ShellTool => true,
|
||||
Self::FullBuffer => false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq)]
|
||||
pub enum SandboxType {
|
||||
None,
|
||||
@@ -230,6 +261,7 @@ pub fn build_exec_request(
|
||||
cwd,
|
||||
mut env,
|
||||
expiration,
|
||||
capture_policy,
|
||||
network,
|
||||
sandbox_permissions,
|
||||
windows_sandbox_level,
|
||||
@@ -253,6 +285,7 @@ pub fn build_exec_request(
|
||||
cwd,
|
||||
env,
|
||||
expiration,
|
||||
capture_policy,
|
||||
sandbox_permissions,
|
||||
additional_permissions: None,
|
||||
justification,
|
||||
@@ -292,6 +325,7 @@ pub(crate) async fn execute_exec_request(
|
||||
env,
|
||||
network,
|
||||
expiration,
|
||||
capture_policy,
|
||||
sandbox,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
@@ -308,6 +342,7 @@ pub(crate) async fn execute_exec_request(
|
||||
command,
|
||||
cwd,
|
||||
expiration,
|
||||
capture_policy,
|
||||
env,
|
||||
network: network.clone(),
|
||||
sandbox_permissions,
|
||||
@@ -414,6 +449,7 @@ async fn exec_windows_sandbox(
|
||||
mut env,
|
||||
network,
|
||||
expiration,
|
||||
capture_policy,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
..
|
||||
@@ -424,7 +460,11 @@ async fn exec_windows_sandbox(
|
||||
|
||||
// TODO(iceweasel-oai): run_windows_sandbox_capture should support all
|
||||
// variants of ExecExpiration, not just timeout.
|
||||
let timeout_ms = expiration.timeout_ms();
|
||||
let timeout_ms = if capture_policy.uses_expiration() {
|
||||
expiration.timeout_ms()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
let policy_str = serde_json::to_string(sandbox_policy).map_err(|err| {
|
||||
CodexErr::Io(io::Error::other(format!(
|
||||
@@ -488,12 +528,16 @@ async fn exec_windows_sandbox(
|
||||
|
||||
let exit_status = synthetic_exit_status(capture.exit_code);
|
||||
let mut stdout_text = capture.stdout;
|
||||
if stdout_text.len() > EXEC_OUTPUT_MAX_BYTES {
|
||||
stdout_text.truncate(EXEC_OUTPUT_MAX_BYTES);
|
||||
if let Some(max_bytes) = capture_policy.retained_bytes_cap()
|
||||
&& stdout_text.len() > max_bytes
|
||||
{
|
||||
stdout_text.truncate(max_bytes);
|
||||
}
|
||||
let mut stderr_text = capture.stderr;
|
||||
if stderr_text.len() > EXEC_OUTPUT_MAX_BYTES {
|
||||
stderr_text.truncate(EXEC_OUTPUT_MAX_BYTES);
|
||||
if let Some(max_bytes) = capture_policy.retained_bytes_cap()
|
||||
&& stderr_text.len() > max_bytes
|
||||
{
|
||||
stderr_text.truncate(max_bytes);
|
||||
}
|
||||
let stdout = StreamOutput {
|
||||
text: stdout_text,
|
||||
@@ -503,7 +547,7 @@ async fn exec_windows_sandbox(
|
||||
text: stderr_text,
|
||||
truncated_after_lines: None,
|
||||
};
|
||||
let aggregated_output = aggregate_output(&stdout, &stderr);
|
||||
let aggregated_output = aggregate_output(&stdout, &stderr, capture_policy.retained_bytes_cap());
|
||||
|
||||
Ok(RawExecToolCallOutput {
|
||||
exit_status,
|
||||
@@ -701,9 +745,20 @@ fn append_capped(dst: &mut Vec<u8>, src: &[u8], max_bytes: usize) {
|
||||
fn aggregate_output(
|
||||
stdout: &StreamOutput<Vec<u8>>,
|
||||
stderr: &StreamOutput<Vec<u8>>,
|
||||
max_bytes: Option<usize>,
|
||||
) -> StreamOutput<Vec<u8>> {
|
||||
let Some(max_bytes) = max_bytes else {
|
||||
let total_len = stdout.text.len().saturating_add(stderr.text.len());
|
||||
let mut aggregated = Vec::with_capacity(total_len);
|
||||
aggregated.extend_from_slice(&stdout.text);
|
||||
aggregated.extend_from_slice(&stderr.text);
|
||||
return StreamOutput {
|
||||
text: aggregated,
|
||||
truncated_after_lines: None,
|
||||
};
|
||||
};
|
||||
|
||||
let total_len = stdout.text.len().saturating_add(stderr.text.len());
|
||||
let max_bytes = EXEC_OUTPUT_MAX_BYTES;
|
||||
let mut aggregated = Vec::with_capacity(total_len.min(max_bytes));
|
||||
|
||||
if total_len <= max_bytes {
|
||||
@@ -785,6 +840,7 @@ async fn exec(
|
||||
network,
|
||||
arg0,
|
||||
expiration,
|
||||
capture_policy,
|
||||
windows_sandbox_level: _,
|
||||
..
|
||||
} = params;
|
||||
@@ -816,7 +872,7 @@ async fn exec(
|
||||
if let Some(after_spawn) = after_spawn {
|
||||
after_spawn();
|
||||
}
|
||||
consume_truncated_output(child, expiration, stdout_stream).await
|
||||
consume_output(child, expiration, capture_policy, stdout_stream).await
|
||||
}
|
||||
|
||||
#[cfg_attr(not(target_os = "windows"), allow(dead_code))]
|
||||
@@ -870,11 +926,12 @@ fn windows_restricted_token_sandbox_support(
|
||||
}
|
||||
}
|
||||
|
||||
/// Consumes the output of a child process, truncating it so it is suitable for
|
||||
/// use as the output of a `shell` tool call. Also enforces specified timeout.
|
||||
async fn consume_truncated_output(
|
||||
/// Consumes the output of a child process according to the configured capture
|
||||
/// policy.
|
||||
async fn consume_output(
|
||||
mut child: Child,
|
||||
expiration: ExecExpiration,
|
||||
capture_policy: ExecCapturePolicy,
|
||||
stdout_stream: Option<StdoutStream>,
|
||||
) -> Result<RawExecToolCallOutput> {
|
||||
// Both stdout and stderr were configured with `Stdio::piped()`
|
||||
@@ -892,23 +949,34 @@ async fn consume_truncated_output(
|
||||
))
|
||||
})?;
|
||||
|
||||
let stdout_handle = tokio::spawn(read_capped(
|
||||
let retained_bytes_cap = capture_policy.retained_bytes_cap();
|
||||
let stdout_handle = tokio::spawn(read_output(
|
||||
BufReader::new(stdout_reader),
|
||||
stdout_stream.clone(),
|
||||
/*is_stderr*/ false,
|
||||
retained_bytes_cap,
|
||||
));
|
||||
let stderr_handle = tokio::spawn(read_capped(
|
||||
let stderr_handle = tokio::spawn(read_output(
|
||||
BufReader::new(stderr_reader),
|
||||
stdout_stream.clone(),
|
||||
/*is_stderr*/ true,
|
||||
retained_bytes_cap,
|
||||
));
|
||||
|
||||
let expiration_wait = async {
|
||||
if capture_policy.uses_expiration() {
|
||||
expiration.wait().await;
|
||||
} else {
|
||||
std::future::pending::<()>().await;
|
||||
}
|
||||
};
|
||||
tokio::pin!(expiration_wait);
|
||||
let (exit_status, timed_out) = tokio::select! {
|
||||
status_result = child.wait() => {
|
||||
let exit_status = status_result?;
|
||||
(exit_status, false)
|
||||
}
|
||||
_ = expiration.wait() => {
|
||||
_ = &mut expiration_wait => {
|
||||
kill_child_process_group(&mut child)?;
|
||||
child.start_kill()?;
|
||||
(synthetic_exit_status(EXIT_CODE_SIGNAL_BASE + TIMEOUT_CODE), true)
|
||||
@@ -923,7 +991,7 @@ async fn consume_truncated_output(
|
||||
// We need mutable bindings so we can `abort()` them on timeout.
|
||||
use tokio::task::JoinHandle;
|
||||
|
||||
async fn await_with_timeout(
|
||||
async fn await_output(
|
||||
handle: &mut JoinHandle<std::io::Result<StreamOutput<Vec<u8>>>>,
|
||||
timeout: Duration,
|
||||
) -> std::io::Result<StreamOutput<Vec<u8>>> {
|
||||
@@ -946,17 +1014,9 @@ async fn consume_truncated_output(
|
||||
let mut stdout_handle = stdout_handle;
|
||||
let mut stderr_handle = stderr_handle;
|
||||
|
||||
let stdout = await_with_timeout(
|
||||
&mut stdout_handle,
|
||||
Duration::from_millis(IO_DRAIN_TIMEOUT_MS),
|
||||
)
|
||||
.await?;
|
||||
let stderr = await_with_timeout(
|
||||
&mut stderr_handle,
|
||||
Duration::from_millis(IO_DRAIN_TIMEOUT_MS),
|
||||
)
|
||||
.await?;
|
||||
let aggregated_output = aggregate_output(&stdout, &stderr);
|
||||
let stdout = await_output(&mut stdout_handle, capture_policy.io_drain_timeout()).await?;
|
||||
let stderr = await_output(&mut stderr_handle, capture_policy.io_drain_timeout()).await?;
|
||||
let aggregated_output = aggregate_output(&stdout, &stderr, retained_bytes_cap);
|
||||
|
||||
Ok(RawExecToolCallOutput {
|
||||
exit_status,
|
||||
@@ -967,12 +1027,17 @@ async fn consume_truncated_output(
|
||||
})
|
||||
}
|
||||
|
||||
async fn read_capped<R: AsyncRead + Unpin + Send + 'static>(
|
||||
async fn read_output<R: AsyncRead + Unpin + Send + 'static>(
|
||||
mut reader: R,
|
||||
stream: Option<StdoutStream>,
|
||||
is_stderr: bool,
|
||||
max_bytes: Option<usize>,
|
||||
) -> io::Result<StreamOutput<Vec<u8>>> {
|
||||
let mut buf = Vec::with_capacity(AGGREGATE_BUFFER_INITIAL_CAPACITY.min(EXEC_OUTPUT_MAX_BYTES));
|
||||
let mut buf = Vec::with_capacity(
|
||||
max_bytes.map_or(AGGREGATE_BUFFER_INITIAL_CAPACITY, |max_bytes| {
|
||||
AGGREGATE_BUFFER_INITIAL_CAPACITY.min(max_bytes)
|
||||
}),
|
||||
);
|
||||
let mut tmp = [0u8; READ_CHUNK_SIZE];
|
||||
let mut emitted_deltas: usize = 0;
|
||||
|
||||
@@ -1004,7 +1069,11 @@ async fn read_capped<R: AsyncRead + Unpin + Send + 'static>(
|
||||
emitted_deltas += 1;
|
||||
}
|
||||
|
||||
append_capped(&mut buf, &tmp[..n], EXEC_OUTPUT_MAX_BYTES);
|
||||
if let Some(max_bytes) = max_bytes {
|
||||
append_capped(&mut buf, &tmp[..n], max_bytes);
|
||||
} else {
|
||||
buf.extend_from_slice(&tmp[..n]);
|
||||
}
|
||||
// Continue reading to EOF to avoid back-pressure
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use super::*;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::HashMap;
|
||||
use std::time::Duration;
|
||||
use tokio::io::AsyncWriteExt;
|
||||
|
||||
@@ -91,14 +92,16 @@ fn sandbox_detection_ignores_network_policy_text_with_zero_exit_code() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_capped_limits_retained_bytes() {
|
||||
async fn read_output_limits_retained_bytes_for_shell_capture() {
|
||||
let (mut writer, reader) = tokio::io::duplex(1024);
|
||||
let bytes = vec![b'a'; EXEC_OUTPUT_MAX_BYTES.saturating_add(128 * 1024)];
|
||||
tokio::spawn(async move {
|
||||
writer.write_all(&bytes).await.expect("write");
|
||||
});
|
||||
|
||||
let out = read_capped(reader, None, false).await.expect("read");
|
||||
let out = read_output(reader, None, false, Some(EXEC_OUTPUT_MAX_BYTES))
|
||||
.await
|
||||
.expect("read");
|
||||
assert_eq!(out.text.len(), EXEC_OUTPUT_MAX_BYTES);
|
||||
}
|
||||
|
||||
@@ -113,7 +116,7 @@ fn aggregate_output_prefers_stderr_on_contention() {
|
||||
truncated_after_lines: None,
|
||||
};
|
||||
|
||||
let aggregated = aggregate_output(&stdout, &stderr);
|
||||
let aggregated = aggregate_output(&stdout, &stderr, Some(EXEC_OUTPUT_MAX_BYTES));
|
||||
let stdout_cap = EXEC_OUTPUT_MAX_BYTES / 3;
|
||||
let stderr_cap = EXEC_OUTPUT_MAX_BYTES.saturating_sub(stdout_cap);
|
||||
|
||||
@@ -134,7 +137,7 @@ fn aggregate_output_fills_remaining_capacity_with_stderr() {
|
||||
truncated_after_lines: None,
|
||||
};
|
||||
|
||||
let aggregated = aggregate_output(&stdout, &stderr);
|
||||
let aggregated = aggregate_output(&stdout, &stderr, Some(EXEC_OUTPUT_MAX_BYTES));
|
||||
let stderr_cap = EXEC_OUTPUT_MAX_BYTES.saturating_sub(stdout_len);
|
||||
|
||||
assert_eq!(aggregated.text.len(), EXEC_OUTPUT_MAX_BYTES);
|
||||
@@ -153,7 +156,7 @@ fn aggregate_output_rebalances_when_stderr_is_small() {
|
||||
truncated_after_lines: None,
|
||||
};
|
||||
|
||||
let aggregated = aggregate_output(&stdout, &stderr);
|
||||
let aggregated = aggregate_output(&stdout, &stderr, Some(EXEC_OUTPUT_MAX_BYTES));
|
||||
let stdout_len = EXEC_OUTPUT_MAX_BYTES.saturating_sub(1);
|
||||
|
||||
assert_eq!(aggregated.text.len(), EXEC_OUTPUT_MAX_BYTES);
|
||||
@@ -172,7 +175,7 @@ fn aggregate_output_keeps_stdout_then_stderr_when_under_cap() {
|
||||
truncated_after_lines: None,
|
||||
};
|
||||
|
||||
let aggregated = aggregate_output(&stdout, &stderr);
|
||||
let aggregated = aggregate_output(&stdout, &stderr, Some(EXEC_OUTPUT_MAX_BYTES));
|
||||
let mut expected = Vec::new();
|
||||
expected.extend_from_slice(&stdout.text);
|
||||
expected.extend_from_slice(&stderr.text);
|
||||
@@ -181,6 +184,192 @@ fn aggregate_output_keeps_stdout_then_stderr_when_under_cap() {
|
||||
assert_eq!(aggregated.truncated_after_lines, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_output_retains_all_bytes_for_full_buffer_capture() {
|
||||
let (mut writer, reader) = tokio::io::duplex(1024);
|
||||
let bytes = vec![b'a'; EXEC_OUTPUT_MAX_BYTES.saturating_add(128 * 1024)];
|
||||
let expected_len = bytes.len();
|
||||
// The duplex pipe is smaller than `bytes`, so the writer must run concurrently
|
||||
// with `read_output()` or `write_all()` will block once the buffer fills up.
|
||||
tokio::spawn(async move {
|
||||
writer.write_all(&bytes).await.expect("write");
|
||||
});
|
||||
|
||||
let out = read_output(reader, None, false, None).await.expect("read");
|
||||
assert_eq!(out.text.len(), expected_len);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn aggregate_output_keeps_all_bytes_when_uncapped() {
|
||||
let stdout = StreamOutput {
|
||||
text: vec![b'a'; EXEC_OUTPUT_MAX_BYTES],
|
||||
truncated_after_lines: None,
|
||||
};
|
||||
let stderr = StreamOutput {
|
||||
text: vec![b'b'; EXEC_OUTPUT_MAX_BYTES],
|
||||
truncated_after_lines: None,
|
||||
};
|
||||
|
||||
let aggregated = aggregate_output(&stdout, &stderr, None);
|
||||
|
||||
assert_eq!(aggregated.text.len(), EXEC_OUTPUT_MAX_BYTES * 2);
|
||||
assert_eq!(
|
||||
aggregated.text[..EXEC_OUTPUT_MAX_BYTES],
|
||||
vec![b'a'; EXEC_OUTPUT_MAX_BYTES]
|
||||
);
|
||||
assert_eq!(
|
||||
aggregated.text[EXEC_OUTPUT_MAX_BYTES..],
|
||||
vec![b'b'; EXEC_OUTPUT_MAX_BYTES]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn full_buffer_capture_policy_disables_caps_and_exec_expiration() {
|
||||
assert_eq!(ExecCapturePolicy::FullBuffer.retained_bytes_cap(), None);
|
||||
assert_eq!(
|
||||
ExecCapturePolicy::FullBuffer.io_drain_timeout(),
|
||||
Duration::from_millis(IO_DRAIN_TIMEOUT_MS)
|
||||
);
|
||||
assert!(!ExecCapturePolicy::FullBuffer.uses_expiration());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_full_buffer_capture_ignores_expiration() -> Result<()> {
|
||||
#[cfg(windows)]
|
||||
let command = vec![
|
||||
"powershell.exe".to_string(),
|
||||
"-NonInteractive".to_string(),
|
||||
"-NoLogo".to_string(),
|
||||
"-Command".to_string(),
|
||||
"Start-Sleep -Milliseconds 50; [Console]::Out.Write('hello')".to_string(),
|
||||
];
|
||||
#[cfg(not(windows))]
|
||||
let command = vec![
|
||||
"/bin/sh".to_string(),
|
||||
"-c".to_string(),
|
||||
"sleep 0.05; printf hello".to_string(),
|
||||
];
|
||||
|
||||
let env: HashMap<String, String> = std::env::vars().collect();
|
||||
let output = exec(
|
||||
ExecParams {
|
||||
command,
|
||||
cwd: std::env::current_dir()?,
|
||||
expiration: 1.into(),
|
||||
capture_policy: ExecCapturePolicy::FullBuffer,
|
||||
env,
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
},
|
||||
SandboxType::None,
|
||||
&SandboxPolicy::DangerFullAccess,
|
||||
&FileSystemSandboxPolicy::unrestricted(),
|
||||
NetworkSandboxPolicy::Enabled,
|
||||
/*stdout_stream*/ None,
|
||||
/*after_spawn*/ None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(output.stdout.from_utf8_lossy().text.trim(), "hello");
|
||||
assert!(!output.timed_out);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn exec_full_buffer_capture_keeps_io_drain_timeout_when_descendant_holds_pipe_open()
|
||||
-> Result<()> {
|
||||
let output = tokio::time::timeout(
|
||||
Duration::from_millis(IO_DRAIN_TIMEOUT_MS * 3),
|
||||
exec(
|
||||
ExecParams {
|
||||
command: vec![
|
||||
"/bin/sh".to_string(),
|
||||
"-c".to_string(),
|
||||
"printf hello; sleep 30 &".to_string(),
|
||||
],
|
||||
cwd: std::env::current_dir()?,
|
||||
expiration: 1.into(),
|
||||
capture_policy: ExecCapturePolicy::FullBuffer,
|
||||
env: std::env::vars().collect(),
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
},
|
||||
SandboxType::None,
|
||||
&SandboxPolicy::DangerFullAccess,
|
||||
&FileSystemSandboxPolicy::unrestricted(),
|
||||
NetworkSandboxPolicy::Enabled,
|
||||
/*stdout_stream*/ None,
|
||||
/*after_spawn*/ None,
|
||||
),
|
||||
)
|
||||
.await
|
||||
.expect("full-buffer exec should return once the I/O drain guard fires")?;
|
||||
|
||||
assert!(!output.timed_out);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn process_exec_tool_call_preserves_full_buffer_capture_policy() -> Result<()> {
|
||||
let byte_count = EXEC_OUTPUT_MAX_BYTES.saturating_add(128 * 1024);
|
||||
#[cfg(windows)]
|
||||
let command = vec![
|
||||
"powershell.exe".to_string(),
|
||||
"-NonInteractive".to_string(),
|
||||
"-NoLogo".to_string(),
|
||||
"-Command".to_string(),
|
||||
format!("Start-Sleep -Milliseconds 50; [Console]::Out.Write('a' * {byte_count})"),
|
||||
];
|
||||
#[cfg(not(windows))]
|
||||
let command = vec![
|
||||
"/bin/sh".to_string(),
|
||||
"-c".to_string(),
|
||||
format!("sleep 0.05; head -c {byte_count} /dev/zero | tr '\\0' 'a'"),
|
||||
];
|
||||
|
||||
let cwd = std::env::current_dir()?;
|
||||
let sandbox_policy = SandboxPolicy::DangerFullAccess;
|
||||
let output = process_exec_tool_call(
|
||||
ExecParams {
|
||||
command,
|
||||
cwd: cwd.clone(),
|
||||
expiration: 1.into(),
|
||||
capture_policy: ExecCapturePolicy::FullBuffer,
|
||||
env: std::env::vars().collect(),
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
},
|
||||
&sandbox_policy,
|
||||
&FileSystemSandboxPolicy::from(&sandbox_policy),
|
||||
NetworkSandboxPolicy::Enabled,
|
||||
cwd.as_path(),
|
||||
&None,
|
||||
false,
|
||||
None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(!output.timed_out);
|
||||
assert_eq!(output.stdout.text.len(), byte_count);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn windows_restricted_token_skips_external_sandbox_policies() {
|
||||
let policy = SandboxPolicy::ExternalSandbox {
|
||||
@@ -396,6 +585,7 @@ async fn kill_child_process_group_kills_grandchildren_on_timeout() -> Result<()>
|
||||
command,
|
||||
cwd: std::env::current_dir()?,
|
||||
expiration: 500.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env,
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
@@ -453,6 +643,7 @@ async fn process_exec_tool_call_respects_cancellation_token() -> Result<()> {
|
||||
command,
|
||||
cwd: cwd.clone(),
|
||||
expiration: ExecExpiration::Cancellation(cancel_token),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env,
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
|
||||
@@ -8,6 +8,7 @@ ready‑to‑spawn environment.
|
||||
|
||||
pub(crate) mod macos_permissions;
|
||||
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecExpiration;
|
||||
use crate::exec::ExecToolCallOutput;
|
||||
use crate::exec::SandboxType;
|
||||
@@ -55,6 +56,7 @@ pub struct CommandSpec {
|
||||
pub cwd: PathBuf,
|
||||
pub env: HashMap<String, String>,
|
||||
pub expiration: ExecExpiration,
|
||||
pub capture_policy: ExecCapturePolicy,
|
||||
pub sandbox_permissions: SandboxPermissions,
|
||||
pub additional_permissions: Option<PermissionProfile>,
|
||||
pub justification: Option<String>,
|
||||
@@ -67,6 +69,7 @@ pub struct ExecRequest {
|
||||
pub env: HashMap<String, String>,
|
||||
pub network: Option<NetworkProxy>,
|
||||
pub expiration: ExecExpiration,
|
||||
pub capture_policy: ExecCapturePolicy,
|
||||
pub sandbox: SandboxType,
|
||||
pub windows_sandbox_level: WindowsSandboxLevel,
|
||||
pub windows_sandbox_private_desktop: bool,
|
||||
@@ -707,6 +710,7 @@ impl SandboxManager {
|
||||
env,
|
||||
network: network.cloned(),
|
||||
expiration: spec.expiration,
|
||||
capture_policy: spec.capture_policy,
|
||||
sandbox,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop,
|
||||
|
||||
@@ -158,6 +158,7 @@ fn transform_preserves_unrestricted_file_system_policy_for_restricted_network()
|
||||
cwd: cwd.clone(),
|
||||
env: HashMap::new(),
|
||||
expiration: crate::exec::ExecExpiration::DefaultTimeout,
|
||||
capture_policy: crate::exec::ExecCapturePolicy::ShellTool,
|
||||
sandbox_permissions: super::SandboxPermissions::UseDefault,
|
||||
additional_permissions: None,
|
||||
justification: None,
|
||||
@@ -518,6 +519,7 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() {
|
||||
cwd: cwd.clone(),
|
||||
env: HashMap::new(),
|
||||
expiration: crate::exec::ExecExpiration::DefaultTimeout,
|
||||
capture_policy: crate::exec::ExecCapturePolicy::ShellTool,
|
||||
sandbox_permissions: super::SandboxPermissions::WithAdditionalPermissions,
|
||||
additional_permissions: Some(PermissionProfile {
|
||||
network: Some(NetworkPermissions {
|
||||
@@ -580,6 +582,7 @@ fn transform_additional_permissions_preserves_denied_entries() {
|
||||
cwd: cwd.clone(),
|
||||
env: HashMap::new(),
|
||||
expiration: crate::exec::ExecExpiration::DefaultTimeout,
|
||||
capture_policy: crate::exec::ExecCapturePolicy::ShellTool,
|
||||
sandbox_permissions: super::SandboxPermissions::WithAdditionalPermissions,
|
||||
additional_permissions: Some(PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
|
||||
@@ -10,6 +10,7 @@ use tracing::error;
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::codex::TurnContext;
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecToolCallOutput;
|
||||
use crate::exec::SandboxType;
|
||||
use crate::exec::StdoutStream;
|
||||
@@ -165,6 +166,7 @@ pub(crate) async fn execute_user_shell_command(
|
||||
// TODO(zhao-oai): Now that we have ExecExpiration::Cancellation, we
|
||||
// should use that instead of an "arbitrarily large" timeout here.
|
||||
expiration: USER_SHELL_TIMEOUT_MS.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
sandbox: SandboxType::None,
|
||||
windows_sandbox_level: turn_context.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: turn_context
|
||||
|
||||
@@ -5,6 +5,7 @@ use codex_protocol::models::ShellToolCallParams;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::codex::TurnContext;
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecParams;
|
||||
use crate::exec_env::create_env;
|
||||
use crate::exec_policy::ExecApprovalRequest;
|
||||
@@ -70,6 +71,7 @@ impl ShellHandler {
|
||||
command: params.command.clone(),
|
||||
cwd: turn_context.resolve_path(params.workdir.clone()),
|
||||
expiration: params.timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env: create_env(&turn_context.shell_environment_policy, Some(thread_id)),
|
||||
network: turn_context.network.clone(),
|
||||
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
|
||||
@@ -124,6 +126,7 @@ impl ShellCommandHandler {
|
||||
command,
|
||||
cwd: turn_context.resolve_path(params.workdir.clone()),
|
||||
expiration: params.timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env: create_env(&turn_context.shell_environment_policy, Some(thread_id)),
|
||||
network: turn_context.network.clone(),
|
||||
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
|
||||
|
||||
@@ -34,6 +34,7 @@ use uuid::Uuid;
|
||||
use crate::client_common::tools::ToolSpec;
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecExpiration;
|
||||
use crate::exec_env::create_env;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
@@ -1037,6 +1038,7 @@ impl JsReplManager {
|
||||
cwd: turn.cwd.clone(),
|
||||
env,
|
||||
expiration: ExecExpiration::DefaultTimeout,
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
additional_permissions: None,
|
||||
justification: None,
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
//! decision to avoid re-prompting, builds the self-invocation command for
|
||||
//! `codex --codex-run-as-apply-patch`, and runs under the current
|
||||
//! `SandboxAttempt` with a minimal environment.
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecToolCallOutput;
|
||||
use crate::guardian::GuardianApprovalRequest;
|
||||
use crate::guardian::review_approval_request;
|
||||
@@ -93,6 +94,7 @@ impl ApplyPatchRuntime {
|
||||
],
|
||||
cwd: req.action.cwd.clone(),
|
||||
expiration: req.timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
// Run apply_patch with a minimal environment for determinism and to avoid leaks.
|
||||
env: HashMap::new(),
|
||||
sandbox_permissions: req.sandbox_permissions,
|
||||
|
||||
@@ -4,6 +4,7 @@ Module: runtimes
|
||||
Concrete ToolRuntime implementations for specific tools. Each runtime stays
|
||||
small and focused and reuses the orchestrator for approvals + sandbox + retry.
|
||||
*/
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecExpiration;
|
||||
use crate::path_utils;
|
||||
use crate::sandboxing::CommandSpec;
|
||||
@@ -47,6 +48,7 @@ pub(crate) fn build_command_spec(
|
||||
cwd: cwd.to_path_buf(),
|
||||
env: env.clone(),
|
||||
expiration,
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
sandbox_permissions,
|
||||
additional_permissions,
|
||||
justification,
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use super::ShellRequest;
|
||||
use crate::error::CodexErr;
|
||||
use crate::error::SandboxErr;
|
||||
use crate::exec::ExecCapturePolicy;
|
||||
use crate::exec::ExecExpiration;
|
||||
use crate::exec::ExecToolCallOutput;
|
||||
use crate::exec::SandboxType;
|
||||
@@ -124,6 +125,7 @@ pub(super) async fn try_run_zsh_fork(
|
||||
env: sandbox_env,
|
||||
network: sandbox_network,
|
||||
expiration: _sandbox_expiration,
|
||||
capture_policy: _capture_policy,
|
||||
sandbox,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: _windows_sandbox_private_desktop,
|
||||
@@ -903,6 +905,7 @@ impl ShellCommandExecutor for CoreShellCommandExecutor {
|
||||
env: exec_env,
|
||||
network: self.network.clone(),
|
||||
expiration: ExecExpiration::Cancellation(cancel_rx),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
sandbox: self.sandbox,
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: false,
|
||||
@@ -1042,6 +1045,7 @@ impl CoreShellCommandExecutor {
|
||||
cwd: workdir.to_path_buf(),
|
||||
env,
|
||||
expiration: ExecExpiration::DefaultTimeout,
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
sandbox_permissions: if additional_permissions.is_some() {
|
||||
SandboxPermissions::WithAdditionalPermissions
|
||||
} else {
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
use std::collections::HashMap;
|
||||
use std::string::ToString;
|
||||
|
||||
use codex_core::exec::ExecCapturePolicy;
|
||||
use codex_core::exec::ExecParams;
|
||||
use codex_core::exec::ExecToolCallOutput;
|
||||
use codex_core::exec::SandboxType;
|
||||
@@ -37,6 +38,7 @@ async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>) -> Result<ExecToolCallOutput
|
||||
command: cmd.iter().map(ToString::to_string).collect(),
|
||||
cwd: tmp.path().to_path_buf(),
|
||||
expiration: 1000.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env: HashMap::new(),
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
|
||||
@@ -4,6 +4,7 @@ use codex_core::config::types::ShellEnvironmentPolicy;
|
||||
use codex_core::error::CodexErr;
|
||||
use codex_core::error::Result;
|
||||
use codex_core::error::SandboxErr;
|
||||
use codex_core::exec::ExecCapturePolicy;
|
||||
use codex_core::exec::ExecParams;
|
||||
use codex_core::exec::process_exec_tool_call;
|
||||
use codex_core::exec_env::create_env;
|
||||
@@ -116,6 +117,7 @@ async fn run_cmd_result_with_policies(
|
||||
command: cmd.iter().copied().map(str::to_owned).collect(),
|
||||
cwd,
|
||||
expiration: timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env: create_env_from_core_vars(),
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
@@ -375,6 +377,7 @@ async fn assert_network_blocked(cmd: &[&str]) {
|
||||
// Give the tool a generous 2-second timeout so even slow DNS timeouts
|
||||
// do not stall the suite.
|
||||
expiration: NETWORK_TIMEOUT_MS.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env: create_env_from_core_vars(),
|
||||
network: None,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
|
||||
Reference in New Issue
Block a user