Compare commits

...

1 Commits

Author SHA1 Message Date
Michael Bolin
3aaee0f061 core: add stdin-backed sandboxed fs writes 2026-03-20 15:32:27 -07:00
20 changed files with 236 additions and 11 deletions

View File

@@ -170,6 +170,7 @@ async fn run_command_under_sandbox(
command_vec,
&cwd_clone,
env_map,
/*stdin_bytes*/ None,
/*timeout_ms*/ None,
config.permissions.windows_sandbox_private_desktop,
)
@@ -181,6 +182,7 @@ async fn run_command_under_sandbox(
command_vec,
&cwd_clone,
env_map,
/*stdin_bytes*/ None,
/*timeout_ms*/ None,
config.permissions.windows_sandbox_private_desktop,
)

View File

@@ -4798,6 +4798,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
capture_policy: ExecCapturePolicy::ShellTool,
env: HashMap::new(),
network: None,
stdin: crate::exec::ExecStdin::Closed,
sandbox_permissions,
windows_sandbox_level: turn_context.windows_sandbox_level,
windows_sandbox_private_desktop: turn_context
@@ -4816,6 +4817,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
capture_policy: ExecCapturePolicy::ShellTool,
env: HashMap::new(),
network: None,
stdin: crate::exec::ExecStdin::Closed,
windows_sandbox_level: turn_context.windows_sandbox_level,
windows_sandbox_private_desktop: turn_context
.config

View File

@@ -128,6 +128,7 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
capture_policy: ExecCapturePolicy::ShellTool,
env: HashMap::new(),
network: None,
stdin: crate::exec::ExecStdin::Closed,
sandbox_permissions: SandboxPermissions::WithAdditionalPermissions,
windows_sandbox_level: turn_context.windows_sandbox_level,
windows_sandbox_private_desktop: turn_context

View File

@@ -12,6 +12,7 @@ use std::time::Instant;
use async_channel::Sender;
use tokio::io::AsyncRead;
use tokio::io::AsyncReadExt;
use tokio::io::AsyncWriteExt;
use tokio::io::BufReader;
use tokio::process::Child;
use tokio_util::sync::CancellationToken;
@@ -81,6 +82,7 @@ pub struct ExecParams {
pub capture_policy: ExecCapturePolicy,
pub env: HashMap<String, String>,
pub network: Option<NetworkProxy>,
pub stdin: ExecStdin,
pub sandbox_permissions: SandboxPermissions,
pub windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel,
pub windows_sandbox_private_desktop: bool,
@@ -97,7 +99,12 @@ pub enum ExecCapturePolicy {
/// without the shell-oriented output cap or exec-expiration behavior.
FullBuffer,
}
#[derive(Clone, Debug, Default)]
pub enum ExecStdin {
#[default]
Closed,
Bytes(Vec<u8>),
}
fn select_process_exec_tool_sandbox_type(
file_system_sandbox_policy: &FileSystemSandboxPolicy,
network_sandbox_policy: NetworkSandboxPolicy,
@@ -263,6 +270,7 @@ pub fn build_exec_request(
expiration,
capture_policy,
network,
stdin: _stdin,
sandbox_permissions,
windows_sandbox_level,
windows_sandbox_private_desktop,
@@ -380,6 +388,7 @@ fn prepare_exec_request(exec_request: ExecRequest) -> PreparedExecRequest {
cwd,
env,
network,
stdin,
expiration,
capture_policy,
sandbox,
@@ -402,6 +411,7 @@ fn prepare_exec_request(exec_request: ExecRequest) -> PreparedExecRequest {
capture_policy,
env,
network,
stdin,
sandbox_permissions,
windows_sandbox_level,
windows_sandbox_private_desktop,
@@ -495,6 +505,7 @@ async fn exec_windows_sandbox(
cwd,
mut env,
network,
stdin,
expiration,
capture_policy,
windows_sandbox_level,
@@ -536,6 +547,10 @@ async fn exec_windows_sandbox(
command,
&cwd,
env,
match stdin {
ExecStdin::Closed => None,
ExecStdin::Bytes(bytes) => Some(bytes),
},
timeout_ms,
windows_sandbox_private_desktop,
)
@@ -547,6 +562,10 @@ async fn exec_windows_sandbox(
command,
&cwd,
env,
match stdin {
ExecStdin::Closed => None,
ExecStdin::Bytes(bytes) => Some(bytes),
},
timeout_ms,
windows_sandbox_private_desktop,
)
@@ -913,6 +932,7 @@ async fn exec(
mut env,
network,
arg0,
stdin,
expiration,
capture_policy,
windows_sandbox_level: _,
@@ -941,12 +961,13 @@ async fn exec(
network: None,
stdio_policy: StdioPolicy::RedirectForShellTool,
env,
stdin_open: matches!(stdin, ExecStdin::Bytes(_)),
})
.await?;
if let Some(after_spawn) = after_spawn {
after_spawn();
}
consume_output(child, expiration, capture_policy, stdout_stream).await
consume_output(child, stdin, expiration, capture_policy, stdout_stream).await
}
#[cfg_attr(not(target_os = "windows"), allow(dead_code))]
@@ -1004,10 +1025,19 @@ fn windows_restricted_token_sandbox_support(
/// policy.
async fn consume_output(
mut child: Child,
stdin: ExecStdin,
expiration: ExecExpiration,
capture_policy: ExecCapturePolicy,
stdout_stream: Option<StdoutStream>,
) -> Result<RawExecToolCallOutput> {
let stdin_task = match (child.stdin.take(), stdin) {
(Some(mut child_stdin), ExecStdin::Bytes(bytes)) => Some(tokio::spawn(async move {
child_stdin.write_all(&bytes).await?;
child_stdin.shutdown().await
})),
_ => None,
};
// Both stdout and stderr were configured with `Stdio::piped()`
// above, therefore `take()` should normally return `Some`. If it doesn't
// we treat it as an exceptional I/O error
@@ -1090,6 +1120,13 @@ async fn consume_output(
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?;
if let Some(stdin_task) = stdin_task {
match stdin_task.await {
Ok(Ok(())) => {}
Ok(Err(err)) => return Err(CodexErr::Io(err)),
Err(join_err) => return Err(CodexErr::Io(io::Error::other(join_err))),
}
}
let aggregated_output = aggregate_output(&stdout, &stderr, retained_bytes_cap);
Ok(RawExecToolCallOutput {

View File

@@ -105,6 +105,56 @@ async fn read_output_limits_retained_bytes_for_shell_capture() {
assert_eq!(out.text.len(), EXEC_OUTPUT_MAX_BYTES);
}
#[tokio::test]
async fn exec_passes_stdin_bytes_to_child() -> Result<()> {
let command = if cfg!(windows) {
vec![
"cmd.exe".to_string(),
"/Q".to_string(),
"/D".to_string(),
"/C".to_string(),
"more".to_string(),
]
} else {
vec!["/bin/cat".to_string()]
};
let params = ExecParams {
command,
cwd: std::env::current_dir()?,
expiration: 1_000.into(),
env: std::env::vars().collect(),
network: None,
stdin: ExecStdin::Bytes(b"hello from stdin\n".to_vec()),
sandbox_permissions: SandboxPermissions::UseDefault,
windows_sandbox_level: WindowsSandboxLevel::Disabled,
windows_sandbox_private_desktop: false,
justification: None,
arg0: None,
};
let output = exec(
params,
SandboxType::None,
&SandboxPolicy::DangerFullAccess,
&FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess),
NetworkSandboxPolicy::Enabled,
None,
None,
)
.await?;
let expected_stdout = if cfg!(windows) {
"hello from stdin\r\n"
} else {
"hello from stdin\n"
};
assert_eq!(output.exit_status.code(), Some(0));
assert_eq!(output.stdout.from_utf8_lossy().text, expected_stdout);
assert_eq!(output.stderr.from_utf8_lossy().text, "");
Ok(())
}
#[test]
fn aggregate_output_prefers_stderr_on_contention() {
let stdout = StreamOutput {
@@ -588,6 +638,7 @@ async fn kill_child_process_group_kills_grandchildren_on_timeout() -> Result<()>
capture_policy: ExecCapturePolicy::ShellTool,
env,
network: None,
stdin: ExecStdin::Closed,
sandbox_permissions: SandboxPermissions::UseDefault,
windows_sandbox_level: WindowsSandboxLevel::Disabled,
windows_sandbox_private_desktop: false,
@@ -646,6 +697,7 @@ async fn process_exec_tool_call_respects_cancellation_token() -> Result<()> {
capture_policy: ExecCapturePolicy::ShellTool,
env,
network: None,
stdin: ExecStdin::Closed,
sandbox_permissions: SandboxPermissions::UseDefault,
windows_sandbox_level: WindowsSandboxLevel::Disabled,
windows_sandbox_private_desktop: false,

View File

@@ -56,6 +56,7 @@ where
network,
stdio_policy,
env,
stdin_open: false,
})
.await
}

View File

@@ -2,6 +2,7 @@ use crate::codex::Session;
use crate::codex::TurnContext;
use crate::exec::ExecCapturePolicy;
use crate::exec::ExecExpiration;
use crate::exec::ExecStdin;
use crate::exec::ExecToolCallRawOutput;
use crate::exec::execute_exec_request_raw_output;
use crate::sandboxing::CommandSpec;
@@ -104,6 +105,7 @@ async fn perform_operation(
exit_code: -1,
message: error.to_string(),
})?;
exec_request.stdin = stdin;
let effective_policy = exec_request.sandbox_policy.clone();
let output = execute_exec_request_raw_output(

View File

@@ -10,6 +10,7 @@ pub(crate) mod macos_permissions;
use crate::exec::ExecCapturePolicy;
use crate::exec::ExecExpiration;
use crate::exec::ExecStdin;
use crate::exec::ExecToolCallOutput;
use crate::exec::SandboxType;
use crate::exec::StdoutStream;
@@ -68,6 +69,7 @@ pub struct ExecRequest {
pub cwd: PathBuf,
pub env: HashMap<String, String>,
pub network: Option<NetworkProxy>,
pub stdin: ExecStdin,
pub expiration: ExecExpiration,
pub capture_policy: ExecCapturePolicy,
pub sandbox: SandboxType,
@@ -709,6 +711,7 @@ impl SandboxManager {
cwd: spec.cwd,
env,
network: network.cloned(),
stdin: ExecStdin::Closed,
expiration: spec.expiration,
capture_policy: spec.capture_policy,
sandbox,

View File

@@ -63,6 +63,7 @@ pub async fn spawn_command_under_seatbelt(
network,
stdio_policy,
env,
stdin_open: false,
})
.await
}

View File

@@ -45,6 +45,7 @@ pub(crate) struct SpawnChildRequest<'a> {
pub network: Option<&'a NetworkProxy>,
pub stdio_policy: StdioPolicy,
pub env: HashMap<String, String>,
pub stdin_open: bool,
}
pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io::Result<Child> {
@@ -57,6 +58,7 @@ pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io
network,
stdio_policy,
mut env,
stdin_open,
} = request;
trace!(
@@ -105,11 +107,15 @@ pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io
match stdio_policy {
StdioPolicy::RedirectForShellTool => {
// Do not create a file descriptor for stdin because otherwise some
// commands may hang forever waiting for input. For example, ripgrep has
// a heuristic where it may try to read from stdin as explained here:
// https://github.com/BurntSushi/ripgrep/blob/e2362d4d5185d02fa857bf381e7bd52e66fafc73/crates/core/flags/hiargs.rs#L1101-L1103
cmd.stdin(Stdio::null());
if stdin_open {
cmd.stdin(Stdio::piped());
} else {
// Do not create a file descriptor for stdin because otherwise some
// commands may hang forever waiting for input. For example, ripgrep has
// a heuristic where it may try to read from stdin as explained here:
// https://github.com/BurntSushi/ripgrep/blob/e2362d4d5185d02fa857bf381e7bd52e66fafc73/crates/core/flags/hiargs.rs#L1101-L1103
cmd.stdin(Stdio::null());
}
cmd.stdout(Stdio::piped()).stderr(Stdio::piped());
}

View File

@@ -11,6 +11,7 @@ use uuid::Uuid;
use crate::codex::TurnContext;
use crate::exec::ExecCapturePolicy;
use crate::exec::ExecStdin;
use crate::exec::ExecToolCallOutput;
use crate::exec::SandboxType;
use crate::exec::StdoutStream;
@@ -163,6 +164,7 @@ pub(crate) async fn execute_user_shell_command(
Some(session.conversation_id),
),
network: turn_context.network.clone(),
stdin: ExecStdin::Closed,
// 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(),

View File

@@ -7,6 +7,7 @@ use std::sync::Arc;
use crate::codex::TurnContext;
use crate::exec::ExecCapturePolicy;
use crate::exec::ExecParams;
use crate::exec::ExecStdin;
use crate::exec_env::create_env;
use crate::exec_policy::ExecApprovalRequest;
use crate::function_tool::FunctionCallError;
@@ -74,6 +75,7 @@ impl ShellHandler {
capture_policy: ExecCapturePolicy::ShellTool,
env: create_env(&turn_context.shell_environment_policy, Some(thread_id)),
network: turn_context.network.clone(),
stdin: ExecStdin::Closed,
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
windows_sandbox_level: turn_context.windows_sandbox_level,
windows_sandbox_private_desktop: turn_context
@@ -129,6 +131,7 @@ impl ShellCommandHandler {
capture_policy: ExecCapturePolicy::ShellTool,
env: create_env(&turn_context.shell_environment_policy, Some(thread_id)),
network: turn_context.network.clone(),
stdin: ExecStdin::Closed,
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
windows_sandbox_level: turn_context.windows_sandbox_level,
windows_sandbox_private_desktop: turn_context

View File

@@ -124,6 +124,7 @@ pub(super) async fn try_run_zsh_fork(
cwd: sandbox_cwd,
env: sandbox_env,
network: sandbox_network,
stdin: _stdin,
expiration: _sandbox_expiration,
capture_policy: _capture_policy,
sandbox,
@@ -904,6 +905,7 @@ impl ShellCommandExecutor for CoreShellCommandExecutor {
cwd: self.cwd.clone(),
env: exec_env,
network: self.network.clone(),
stdin: crate::exec::ExecStdin::Closed,
expiration: ExecExpiration::Cancellation(cancel_rx),
capture_policy: ExecCapturePolicy::ShellTool,
sandbox: self.sandbox,

View File

@@ -41,6 +41,7 @@ async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>) -> Result<ExecToolCallOutput
capture_policy: ExecCapturePolicy::ShellTool,
env: HashMap::new(),
network: None,
stdin: codex_core::exec::ExecStdin::Closed,
sandbox_permissions: SandboxPermissions::UseDefault,
windows_sandbox_level: WindowsSandboxLevel::Disabled,
windows_sandbox_private_desktop: false,

View File

@@ -5,6 +5,7 @@ use std::path::PathBuf;
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum FsCommand {
ReadFile { path: PathBuf },
WriteFile { path: PathBuf },
}
pub fn parse_command_from_args(
@@ -28,7 +29,10 @@ pub fn parse_command_from_args(
let path = PathBuf::from(path);
match operation {
READ_FILE_OPERATION_ARG2 => Ok(FsCommand::ReadFile { path }),
_ => Err(format!("unsupported filesystem operation `{operation}`")),
"write" => Ok(FsCommand::WriteFile { path }),
_ => Err(format!(
"unsupported filesystem operation `{operation}`; expected `read` or `write`"
)),
}
}

View File

@@ -19,3 +19,17 @@ fn parse_read_command() {
}
);
}
#[test]
fn parse_write_command() {
let command =
parse_command_from_args(["write", "/tmp/example.png"].into_iter().map(Into::into))
.expect("command should parse");
assert_eq!(
command,
FsCommand::WriteFile {
path: "/tmp/example.png".into(),
}
);
}

View File

@@ -36,7 +36,7 @@ fn run_from_args(
fn try_run_from_args(
args: impl Iterator<Item = OsString>,
_stdin: &mut impl Read,
stdin: &mut impl Read,
stdout: &mut impl Write,
) -> std::io::Result<()> {
let command = parse_command_from_args(args)
@@ -58,6 +58,12 @@ fn try_run_from_args(
std::io::copy(&mut file, stdout).map(|_| ())
}
FsCommand::WriteFile { path } => {
let mut file = std::fs::File::create(path).map_err(FsError::from)?;
std::io::copy(stdin, &mut file)
.map(|_| ())
.map_err(FsError::from)
}
}
}

View File

@@ -1,6 +1,7 @@
use super::run_from_args;
use crate::READ_FILE_OPERATION_ARG2;
use pretty_assertions::assert_eq;
use std::io::Cursor;
use tempfile::tempdir;
#[test]
@@ -112,3 +113,48 @@ fn run_from_args_serializes_errors_to_stderr() {
assert!(result.is_err(), "missing file should fail");
assert_eq!(stdout, Vec::<u8>::new());
}
#[test]
fn run_from_args_streams_stdin_bytes_to_file() {
let tempdir = tempdir().expect("tempdir");
let path = tempdir.path().join("image.bin");
let expected = b"hello\x00world".to_vec();
let mut stdin = Cursor::new(expected.clone());
let mut stdout = Vec::new();
let mut stderr = Vec::new();
run_from_args(
["write", path.to_str().expect("utf-8 test path")]
.into_iter()
.map(Into::into),
&mut stdin,
&mut stdout,
&mut stderr,
)
.expect("write should succeed");
assert_eq!(std::fs::read(&path).expect("read test file"), expected);
assert_eq!(stdout, Vec::<u8>::new());
assert_eq!(stderr, Vec::<u8>::new());
}
#[test]
fn write_reports_directory_error() {
let tempdir = tempdir().expect("tempdir");
let mut stdin = Cursor::new(b"hello world".to_vec());
let mut stdout = Vec::new();
let error = execute(
FsCommand::WriteFile {
path: tempdir.path().to_path_buf(),
},
&mut stdin,
&mut stdout,
)
.expect_err("writing to a directory should fail");
#[cfg(target_os = "windows")]
assert_eq!(error.kind, FsErrorKind::PermissionDenied);
#[cfg(not(target_os = "windows"))]
assert_eq!(error.kind, FsErrorKind::IsADirectory);
}

View File

@@ -209,6 +209,7 @@ mod windows_impl {
command: Vec<String>,
cwd: &Path,
mut env_map: HashMap<String, String>,
stdin_bytes: Option<Vec<u8>>,
timeout_ms: Option<u64>,
use_private_desktop: bool,
) -> Result<CaptureResult> {
@@ -383,13 +384,26 @@ mod windows_impl {
cap_sids,
timeout_ms,
tty: false,
stdin_open: false,
stdin_open: stdin_bytes.is_some(),
use_private_desktop,
}),
},
};
write_frame(&mut pipe_write, &spawn_request)?;
read_spawn_ready(&mut pipe_read)?;
if let Some(stdin_bytes) = stdin_bytes {
write_frame(
&mut pipe_write,
&FramedMessage {
version: 1,
message: Message::Stdin {
payload: crate::ipc_framed::StdinPayload {
data_b64: crate::ipc_framed::encode_bytes(&stdin_bytes),
},
},
},
)?;
}
drop(pipe_write);
let mut stdout = Vec::new();
@@ -503,6 +517,7 @@ mod stub {
_command: Vec<String>,
_cwd: &Path,
_env_map: HashMap<String, String>,
_stdin_bytes: Option<Vec<u8>>,
_timeout_ms: Option<u64>,
_use_private_desktop: bool,
) -> Result<CaptureResult> {

View File

@@ -265,6 +265,7 @@ mod windows_impl {
command: Vec<String>,
cwd: &Path,
mut env_map: HashMap<String, String>,
stdin_bytes: Option<Vec<u8>>,
timeout_ms: Option<u64>,
use_private_desktop: bool,
) -> Result<CaptureResult> {
@@ -406,7 +407,30 @@ mod windows_impl {
unsafe {
CloseHandle(in_r);
// Close the parent's stdin write end so the child sees EOF immediately.
if let Some(stdin_bytes) = stdin_bytes {
let mut offset = 0;
while offset < stdin_bytes.len() {
let remaining = stdin_bytes.len() - offset;
let chunk_len = remaining.min(u32::MAX as usize);
let mut written: u32 = 0;
let ok = windows_sys::Win32::Storage::FileSystem::WriteFile(
in_w,
stdin_bytes[offset..offset + chunk_len].as_ptr(),
chunk_len as u32,
&mut written,
std::ptr::null_mut(),
);
if ok == 0 {
CloseHandle(in_w);
CloseHandle(out_w);
CloseHandle(err_w);
CloseHandle(pi.hThread);
CloseHandle(pi.hProcess);
return Err(std::io::Error::from_raw_os_error(GetLastError() as i32).into());
}
offset += written as usize;
}
}
CloseHandle(in_w);
CloseHandle(out_w);
CloseHandle(err_w);
@@ -615,6 +639,7 @@ mod stub {
_command: Vec<String>,
_cwd: &Path,
_env_map: HashMap<String, String>,
_stdin_bytes: Option<Vec<u8>>,
_timeout_ms: Option<u64>,
_use_private_desktop: bool,
) -> Result<CaptureResult> {