Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions codex-rs/cli/src/debug_sandbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ async fn run_command_under_sandbox(
&cwd_clone,
env_map,
None,
None,
config.permissions.windows_sandbox_private_desktop,
)
} else {
Expand All @@ -182,6 +183,7 @@ async fn run_command_under_sandbox(
&cwd_clone,
env_map,
None,
None,
config.permissions.windows_sandbox_private_desktop,
)
}
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/core/src/codex_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4764,6 +4764,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
expiration: timeout_ms.into(),
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
Expand All @@ -4781,6 +4782,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
expiration: timeout_ms.into(),
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
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/src/codex_tests_guardian.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
expiration: expiration_ms.into(),
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
Expand Down
43 changes: 42 additions & 1 deletion codex-rs/core/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -80,13 +81,21 @@ pub struct ExecParams {
pub expiration: ExecExpiration,
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,
pub justification: Option<String>,
pub arg0: Option<String>,
}

#[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,
Expand Down Expand Up @@ -231,6 +240,7 @@ pub fn build_exec_request(
mut env,
expiration,
network,
stdin: _stdin,
Comment on lines 240 to +243
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve stdin when building sandboxed exec requests

build_exec_request() drops the new ExecParams.stdin field here, and SandboxManager::transform() still initializes ExecRequest.stdin to ExecStdin::Closed in core/src/sandboxing/mod.rs. That means any caller that goes through process_exec_tool_call/build_exec_request will always send EOF, so the stdin support added in this commit only works for the direct exec() path covered by the new unit test.

Useful? React with 👍 / 👎.

sandbox_permissions,
windows_sandbox_level,
windows_sandbox_private_desktop,
Expand Down Expand Up @@ -291,6 +301,7 @@ pub(crate) async fn execute_exec_request(
cwd,
env,
network,
stdin,
expiration,
sandbox,
windows_sandbox_level,
Expand All @@ -310,6 +321,7 @@ pub(crate) async fn execute_exec_request(
expiration,
env,
network: network.clone(),
stdin,
sandbox_permissions,
windows_sandbox_level,
windows_sandbox_private_desktop,
Expand Down Expand Up @@ -343,6 +355,7 @@ pub(crate) async fn execute_exec_request_raw_output(
cwd,
env,
network,
stdin,
expiration,
sandbox,
windows_sandbox_level,
Expand All @@ -362,6 +375,7 @@ pub(crate) async fn execute_exec_request_raw_output(
expiration,
env,
network: network.clone(),
stdin,
sandbox_permissions,
windows_sandbox_level,
windows_sandbox_private_desktop,
Expand Down Expand Up @@ -465,6 +479,7 @@ async fn exec_windows_sandbox(
cwd,
mut env,
network,
stdin,
expiration,
windows_sandbox_level,
windows_sandbox_private_desktop,
Expand Down Expand Up @@ -501,6 +516,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,
)
Expand All @@ -512,6 +531,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,
)
Expand Down Expand Up @@ -917,6 +940,7 @@ async fn exec(
mut env,
network,
arg0,
stdin,
expiration,
windows_sandbox_level: _,
..
Expand Down Expand Up @@ -944,12 +968,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_truncated_output(child, expiration, stdout_stream).await
consume_truncated_output(child, stdin, expiration, stdout_stream).await
}

#[cfg_attr(not(target_os = "windows"), allow(dead_code))]
Expand Down Expand Up @@ -1007,9 +1032,18 @@ fn windows_restricted_token_sandbox_support(
/// use as the output of a `shell` tool call. Also enforces specified timeout.
async fn consume_truncated_output(
mut child: Child,
stdin: ExecStdin,
expiration: ExecExpiration,
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
Expand Down Expand Up @@ -1089,6 +1123,13 @@ async fn consume_truncated_output(
Duration::from_millis(IO_DRAIN_TIMEOUT_MS),
)
.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))),
Comment on lines +1126 to +1130
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Don't fail exec when the child closes stdin early

When ExecStdin::Bytes is used with the direct exec() path, commands that stop reading before draining all input (for example head -c 1, parsers that only consume a header, or a child that exits immediately) now come back as I/O errors even if the process itself exits successfully. In that case child_stdin.write_all() returns BrokenPipe, and this block converts it into CodexErr::Io after we've already collected the child's exit status, so partial stdin consumers become unusable through the new API.

Useful? React with 👍 / 👎.

}
}
let aggregated_output = aggregate_output(&stdout, &stderr);

Ok(RawExecToolCallOutput {
Expand Down
52 changes: 52 additions & 0 deletions codex-rs/core/src/exec_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,56 @@ async fn read_capped_limits_retained_bytes() {
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 {
Expand Down Expand Up @@ -398,6 +448,7 @@ async fn kill_child_process_group_kills_grandchildren_on_timeout() -> Result<()>
expiration: 500.into(),
env,
network: None,
stdin: ExecStdin::Closed,
sandbox_permissions: SandboxPermissions::UseDefault,
windows_sandbox_level: WindowsSandboxLevel::Disabled,
windows_sandbox_private_desktop: false,
Expand Down Expand Up @@ -455,6 +506,7 @@ async fn process_exec_tool_call_respects_cancellation_token() -> Result<()> {
expiration: ExecExpiration::Cancellation(cancel_token),
env,
network: None,
stdin: ExecStdin::Closed,
sandbox_permissions: SandboxPermissions::UseDefault,
windows_sandbox_level: WindowsSandboxLevel::Disabled,
windows_sandbox_private_desktop: false,
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/src/landlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ where
network,
stdio_policy,
env,
stdin_open: false,
})
.await
}
Expand Down
28 changes: 25 additions & 3 deletions codex-rs/core/src/sandboxed_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::codex::TurnContext;
use crate::error::CodexErr;
use crate::error::SandboxErr;
use crate::exec::ExecExpiration;
use crate::exec::ExecStdin;
use crate::exec::ExecToolCallRawOutput;
use crate::sandboxing::CommandSpec;
use crate::sandboxing::SandboxPermissions;
Expand All @@ -27,14 +28,34 @@ pub(crate) async fn read_file(
turn: &Arc<TurnContext>,
path: &Path,
) -> Result<Vec<u8>, SandboxedFsError> {
let output = run_request(session, turn, path).await?;
let output = run_request(session, turn, path, "read", ExecStdin::Closed).await?;
Ok(output.stdout.text)
}

#[allow(dead_code)]
pub(crate) async fn write_file(
session: &Arc<Session>,
turn: &Arc<TurnContext>,
path: &Path,
contents: &[u8],
) -> Result<(), SandboxedFsError> {
run_request(
session,
turn,
path,
"write",
ExecStdin::Bytes(contents.to_vec()),
)
.await?;
Ok(())
}

async fn run_request(
session: &Arc<Session>,
turn: &Arc<TurnContext>,
path: &Path,
operation: &str,
stdin: ExecStdin,
) -> Result<ExecToolCallRawOutput, SandboxedFsError> {
let exe = std::env::current_exe().map_err(|error| SandboxedFsError::ResolveExe {
message: error.to_string(),
Expand All @@ -60,13 +81,13 @@ async fn run_request(
windows_sandbox_level: turn.windows_sandbox_level,
windows_sandbox_private_desktop: turn.config.permissions.windows_sandbox_private_desktop,
};
let exec_request = attempt
let mut exec_request = attempt
.env_for(
CommandSpec {
program: exe.to_string_lossy().to_string(),
args: vec![
CODEX_CORE_FS_OPS_ARG1.to_string(),
"read".to_string(),
operation.to_string(),
path.to_string_lossy().to_string(),
],
cwd: turn.cwd.clone(),
Expand All @@ -83,6 +104,7 @@ async fn run_request(
exit_code: -1,
message: error.to_string(),
})?;
exec_request.stdin = stdin;

let output = execute_env_raw_output(exec_request, /*stdout_stream*/ None)
.await
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/core/src/sandboxing/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ ready‑to‑spawn environment.
pub(crate) mod macos_permissions;

use crate::exec::ExecExpiration;
use crate::exec::ExecStdin;
use crate::exec::ExecToolCallOutput;
use crate::exec::ExecToolCallRawOutput;
use crate::exec::SandboxType;
Expand Down Expand Up @@ -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 sandbox: SandboxType,
pub windows_sandbox_level: WindowsSandboxLevel,
Expand Down Expand Up @@ -708,6 +710,7 @@ impl SandboxManager {
cwd: spec.cwd,
env,
network: network.cloned(),
stdin: ExecStdin::Closed,
expiration: spec.expiration,
sandbox,
windows_sandbox_level,
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/src/seatbelt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ pub async fn spawn_command_under_seatbelt(
network,
stdio_policy,
env,
stdin_open: false,
})
.await
}
Expand Down
Loading
Loading