fix: pass apply_patch payload over stdin

This commit is contained in:
David Wiesen
2026-04-10 10:52:55 -07:00
parent 87328976f6
commit f1e19d39db
23 changed files with 150 additions and 18 deletions
+1
View File
@@ -5597,6 +5597,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
]
},
cwd: turn_context.cwd.clone(),
stdin: None,
expiration: timeout_ms.into(),
capture_policy: ExecCapturePolicy::ShellTool,
env: HashMap::new(),
@@ -123,6 +123,7 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
]
},
cwd: turn_context.cwd.clone(),
stdin: None,
expiration: expiration_ms.into(),
capture_policy: ExecCapturePolicy::ShellTool,
env: HashMap::new(),
+10
View File
@@ -83,6 +83,7 @@ pub const IO_DRAIN_TIMEOUT_MS: u64 = 2_000; // 2 s should be plenty for local pi
pub struct ExecParams {
pub command: Vec<String>,
pub cwd: AbsolutePathBuf,
pub stdin: Option<Vec<u8>>,
pub expiration: ExecExpiration,
pub capture_policy: ExecCapturePolicy,
pub env: HashMap<String, String>,
@@ -265,6 +266,7 @@ pub fn build_exec_request(
command,
cwd,
mut env,
stdin,
expiration,
capture_policy,
network,
@@ -291,6 +293,7 @@ pub fn build_exec_request(
cwd,
env,
additional_permissions: None,
stdin,
};
let options = ExecOptions {
expiration,
@@ -349,6 +352,7 @@ pub(crate) async fn execute_exec_request(
command,
cwd,
env,
stdin,
network,
expiration,
capture_policy,
@@ -365,6 +369,7 @@ pub(crate) async fn execute_exec_request(
let params = ExecParams {
command,
cwd,
stdin,
expiration,
capture_policy,
env,
@@ -472,6 +477,7 @@ async fn exec_windows_sandbox(
command,
cwd,
mut env,
stdin,
network,
expiration,
capture_policy,
@@ -538,6 +544,7 @@ async fn exec_windows_sandbox(
command,
cwd: &cwd,
env_map: env,
stdin,
timeout_ms,
use_private_desktop: windows_sandbox_private_desktop,
proxy_enforced,
@@ -554,6 +561,7 @@ async fn exec_windows_sandbox(
command,
&cwd,
env,
stdin,
timeout_ms,
&additional_deny_write_paths,
windows_sandbox_private_desktop,
@@ -822,6 +830,7 @@ async fn exec(
command,
cwd,
mut env,
stdin,
network,
arg0,
expiration,
@@ -852,6 +861,7 @@ async fn exec(
network: None,
stdio_policy: StdioPolicy::RedirectForShellTool,
env,
stdin,
})
.await?;
if let Some(after_spawn) = after_spawn {
+5
View File
@@ -265,6 +265,7 @@ async fn exec_full_buffer_capture_ignores_expiration() -> Result<()> {
ExecParams {
command,
cwd: codex_utils_absolute_path::AbsolutePathBuf::current_dir()?,
stdin: None,
expiration: 1.into(),
capture_policy: ExecCapturePolicy::FullBuffer,
env,
@@ -305,6 +306,7 @@ async fn exec_full_buffer_capture_keeps_io_drain_timeout_when_descendant_holds_p
"printf hello; sleep 30 &".to_string(),
],
cwd: codex_utils_absolute_path::AbsolutePathBuf::current_dir()?,
stdin: None,
expiration: 1.into(),
capture_policy: ExecCapturePolicy::FullBuffer,
env: std::env::vars().collect(),
@@ -356,6 +358,7 @@ async fn process_exec_tool_call_preserves_full_buffer_capture_policy() -> Result
ExecParams {
command,
cwd: cwd.clone(),
stdin: None,
expiration: 1.into(),
capture_policy: ExecCapturePolicy::FullBuffer,
env: std::env::vars().collect(),
@@ -926,6 +929,7 @@ async fn kill_child_process_group_kills_grandchildren_on_timeout() -> Result<()>
let params = ExecParams {
command,
cwd,
stdin: None,
expiration: 500.into(),
capture_policy: ExecCapturePolicy::ShellTool,
env,
@@ -985,6 +989,7 @@ async fn process_exec_tool_call_respects_cancellation_token() -> Result<()> {
let params = ExecParams {
command,
cwd: cwd.clone(),
stdin: None,
expiration: ExecExpiration::Cancellation(cancel_token),
capture_policy: ExecCapturePolicy::ShellTool,
env,
+1
View File
@@ -72,6 +72,7 @@ where
network,
stdio_policy,
env,
stdin: None,
})
.await
}
+5
View File
@@ -38,6 +38,7 @@ pub struct ExecRequest {
pub command: Vec<String>,
pub cwd: AbsolutePathBuf,
pub env: HashMap<String, String>,
pub stdin: Option<Vec<u8>>,
pub network: Option<NetworkProxy>,
pub expiration: ExecExpiration,
pub capture_policy: ExecCapturePolicy,
@@ -57,6 +58,7 @@ impl ExecRequest {
command: Vec<String>,
cwd: AbsolutePathBuf,
env: HashMap<String, String>,
stdin: Option<Vec<u8>>,
network: Option<NetworkProxy>,
expiration: ExecExpiration,
capture_policy: ExecCapturePolicy,
@@ -72,6 +74,7 @@ impl ExecRequest {
command,
cwd,
env,
stdin,
network,
expiration,
capture_policy,
@@ -94,6 +97,7 @@ impl ExecRequest {
command,
cwd,
mut env,
stdin,
network,
sandbox,
windows_sandbox_level,
@@ -121,6 +125,7 @@ impl ExecRequest {
command,
cwd,
env,
stdin,
network,
expiration,
capture_policy,
+1
View File
@@ -43,6 +43,7 @@ pub async fn spawn_command_under_seatbelt(
network,
stdio_policy,
env,
stdin: None,
})
.await
}
+23 -6
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: Option<Vec<u8>>,
}
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,
} = 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.is_some() {
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());
}
@@ -121,5 +127,16 @@ pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io
}
}
cmd.kill_on_drop(true).spawn()
let mut child = cmd.kill_on_drop(true).spawn()?;
if let Some(stdin) = stdin {
let Some(mut child_stdin) = child.stdin.take() else {
return Err(std::io::Error::other("child stdin was not piped"));
};
tokio::spawn(async move {
use tokio::io::AsyncWriteExt as _;
let _ = child_stdin.write_all(&stdin).await;
let _ = child_stdin.shutdown().await;
});
}
Ok(child)
}
+1
View File
@@ -162,6 +162,7 @@ pub(crate) async fn execute_user_shell_command(
command: exec_command.clone(),
cwd: cwd.clone(),
env: exec_env_map,
stdin: None,
network: turn_context.network.clone(),
// TODO(zhao-oai): Now that we have ExecExpiration::Cancellation, we
// should use that instead of an "arbitrarily large" timeout here.
@@ -96,6 +96,7 @@ impl ShellHandler {
ExecParams {
command: params.command.clone(),
cwd: turn_context.resolve_path(params.workdir.clone()),
stdin: None,
expiration: params.timeout_ms.into(),
capture_policy: ExecCapturePolicy::ShellTool,
env: create_env(&turn_context.shell_environment_policy, Some(thread_id)),
@@ -151,6 +152,7 @@ impl ShellCommandHandler {
Ok(ExecParams {
command,
cwd: turn_context.resolve_path(params.workdir.clone()),
stdin: None,
expiration: params.timeout_ms.into(),
capture_policy: ExecCapturePolicy::ShellTool,
env: create_env(&turn_context.shell_environment_policy, Some(thread_id)),
+1
View File
@@ -1053,6 +1053,7 @@ impl JsReplManager {
cwd: turn.cwd.clone(),
env,
additional_permissions: None,
stdin: None,
};
let options = ExecOptions {
expiration: ExecExpiration::DefaultTimeout,
@@ -29,6 +29,8 @@ use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::FileChange;
use codex_protocol::protocol::ReviewDecision;
use codex_sandboxing::SandboxCommand;
#[cfg(target_os = "windows")]
use codex_sandboxing::SandboxType;
use codex_sandboxing::SandboxablePreference;
use codex_utils_absolute_path::AbsolutePathBuf;
use futures::future::BoxFuture;
@@ -75,6 +77,7 @@ impl ApplyPatchRuntime {
Ok(Self::build_sandbox_command_with_program(
req,
codex_windows_sandbox::resolve_current_exe_for_launch(codex_home, "codex.exe"),
/*pass_patch_via_stdin*/ true,
))
}
@@ -84,7 +87,9 @@ impl ApplyPatchRuntime {
codex_self_exe: Option<&PathBuf>,
) -> Result<SandboxCommand, ToolError> {
let exe = Self::resolve_apply_patch_program(codex_self_exe)?;
Ok(Self::build_sandbox_command_with_program(req, exe))
Ok(Self::build_sandbox_command_with_program(
req, exe, /*pass_patch_via_stdin*/ true,
))
}
#[cfg(not(target_os = "windows"))]
@@ -97,17 +102,33 @@ impl ApplyPatchRuntime {
.map_err(|e| ToolError::Rejected(format!("failed to determine codex exe: {e}")))
}
fn build_sandbox_command_with_program(req: &ApplyPatchRequest, exe: PathBuf) -> SandboxCommand {
fn build_sandbox_command_with_program(
req: &ApplyPatchRequest,
exe: PathBuf,
pass_patch_via_stdin: bool,
) -> SandboxCommand {
let (args, stdin) = if pass_patch_via_stdin {
(
vec![CODEX_CORE_APPLY_PATCH_ARG1.to_string()],
Some(req.action.patch.clone().into_bytes()),
)
} else {
(
vec![
CODEX_CORE_APPLY_PATCH_ARG1.to_string(),
req.action.patch.clone(),
],
None,
)
};
SandboxCommand {
program: exe.into_os_string(),
args: vec![
CODEX_CORE_APPLY_PATCH_ARG1.to_string(),
req.action.patch.clone(),
],
cwd: req.action.cwd.clone(),
args,
cwd: req.action.cwd.to_path_buf(),
// Run apply_patch with a minimal environment for determinism and to avoid leaks.
env: HashMap::new(),
additional_permissions: req.additional_permissions.clone(),
stdin,
}
}
@@ -241,7 +262,17 @@ impl ToolRuntime<ApplyPatchRequest, ExecToolCallOutput> for ApplyPatchRuntime {
}
#[cfg(target_os = "windows")]
let command = Self::build_sandbox_command(req, &ctx.turn.config.codex_home)?;
let command = {
let exe = codex_windows_sandbox::resolve_current_exe_for_launch(
&ctx.turn.config.codex_home,
"codex.exe",
);
Self::build_sandbox_command_with_program(
req,
exe,
matches!(attempt.sandbox, SandboxType::WindowsRestrictedToken),
)
};
#[cfg(not(target_os = "windows"))]
let command = Self::build_sandbox_command(req, ctx.turn.codex_self_exe.as_ref())?;
let options = ExecOptions {
+1
View File
@@ -35,6 +35,7 @@ pub(crate) fn build_sandbox_command(
cwd: cwd.clone(),
env: env.clone(),
additional_permissions,
stdin: None,
})
}
@@ -124,6 +124,7 @@ pub(super) async fn try_run_zsh_fork(
command,
cwd: sandbox_cwd,
env: sandbox_env,
stdin: _stdin,
network: sandbox_network,
expiration: _sandbox_expiration,
capture_policy: _capture_policy,
@@ -713,6 +714,7 @@ impl ShellCommandExecutor for CoreShellCommandExecutor {
command: self.command.clone(),
cwd: self.cwd.clone(),
env: exec_env,
stdin: None,
network: self.network.clone(),
expiration: ExecExpiration::Cancellation(cancel_rx),
capture_policy: ExecCapturePolicy::ShellTool,
@@ -837,6 +839,7 @@ impl CoreShellCommandExecutor {
cwd: workdir.clone(),
env,
additional_permissions,
stdin: None,
};
let options = ExecOptions {
expiration: ExecExpiration::DefaultTimeout,
@@ -64,6 +64,7 @@ fn test_exec_request(
command,
cwd,
env,
None,
network,
ExecExpiration::DefaultTimeout,
ExecCapturePolicy::ShellTool,