mirror of
https://github.com/openai/codex.git
synced 2026-10-25 18:28:38 +00:00
Validate remote unified exec shells
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::function_tool::FunctionCallError;
|
||||
@@ -139,7 +140,7 @@ impl ExecCommandHandler {
|
||||
);
|
||||
let environment = Arc::clone(&turn_environment.environment);
|
||||
let fs = environment.get_filesystem();
|
||||
let args: ExecCommandArgs = parse_arguments_with_base_path(&arguments, &cwd)?;
|
||||
let mut args: ExecCommandArgs = parse_arguments_with_base_path(&arguments, &cwd)?;
|
||||
let hook_command = args.cmd.clone();
|
||||
maybe_emit_implicit_skill_invocation(
|
||||
session.as_ref(),
|
||||
@@ -151,13 +152,28 @@ impl ExecCommandHandler {
|
||||
let process_id = manager.allocate_process_id().await;
|
||||
let shell_mode =
|
||||
shell_mode_for_environment(&turn.unified_exec_shell_mode, environment.as_ref());
|
||||
// Remote environments may use a different OS and must build commands with their native
|
||||
// shell; fall back to the session shell when the environment did not report one.
|
||||
let shell = turn_environment
|
||||
.shell
|
||||
.clone()
|
||||
.map(Arc::new)
|
||||
.unwrap_or_else(|| session.user_shell());
|
||||
let shell = if environment.is_remote() {
|
||||
let environment_shell = turn_environment.shell.clone().ok_or_else(|| {
|
||||
FunctionCallError::RespondToModel(format!(
|
||||
"shell is unavailable in environment `{}`",
|
||||
turn_environment.environment_id
|
||||
))
|
||||
})?;
|
||||
if let Some(provided_shell) = args.shell.as_deref() {
|
||||
let matches_environment = provided_shell == environment_shell.name()
|
||||
|| Path::new(provided_shell) == environment_shell.shell_path;
|
||||
if !matches_environment {
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"`shell` must match the selected remote environment shell `{}`",
|
||||
environment_shell.shell_path.display()
|
||||
)));
|
||||
}
|
||||
args.shell = None;
|
||||
}
|
||||
Arc::new(environment_shell)
|
||||
} else {
|
||||
session.user_shell()
|
||||
};
|
||||
let resolved_command = get_command(
|
||||
&args,
|
||||
shell,
|
||||
|
||||
@@ -366,18 +366,43 @@ async fn exec_command_routes_to_selected_remote_environment() -> Result<()> {
|
||||
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
|
||||
cwd: PathUri::from_abs_path(&remote_cwd),
|
||||
};
|
||||
let selections = vec![local_selection, remote_selection];
|
||||
let mismatched_shell_output = exec_command_routing_output(
|
||||
&test,
|
||||
&server,
|
||||
"call-remote-mismatched-shell",
|
||||
json!({
|
||||
"shell": "/definitely/not/the/remote/shell",
|
||||
"cmd": "echo should-not-run",
|
||||
"environment_id": REMOTE_ENVIRONMENT_ID,
|
||||
}),
|
||||
Some(selections.clone()),
|
||||
)
|
||||
.await?;
|
||||
assert!(
|
||||
mismatched_shell_output
|
||||
.contains("`shell` must match the selected remote environment shell"),
|
||||
"unexpected mismatched shell output: {mismatched_shell_output}",
|
||||
);
|
||||
let remote_shell_path = test
|
||||
.executor_environment()
|
||||
.environment()
|
||||
.info()
|
||||
.await?
|
||||
.shell
|
||||
.path;
|
||||
let multi_env_output = exec_command_routing_output(
|
||||
&test,
|
||||
&server,
|
||||
"call-multi-env",
|
||||
json!({
|
||||
"shell": "/bin/sh",
|
||||
"shell": remote_shell_path,
|
||||
"cmd": format!("cat {remote_marker_name}"),
|
||||
"login": false,
|
||||
"yield_time_ms": 1_000,
|
||||
"environment_id": REMOTE_ENVIRONMENT_ID,
|
||||
}),
|
||||
Some(vec![local_selection, remote_selection]),
|
||||
Some(selections),
|
||||
)
|
||||
.await?;
|
||||
assert!(
|
||||
@@ -496,7 +521,6 @@ async fn remote_request_permissions_grant_unblocks_later_remote_exec() -> Result
|
||||
"exec-call",
|
||||
"exec_command",
|
||||
&json!({
|
||||
"shell": "/bin/sh",
|
||||
"cmd": command,
|
||||
"login": false,
|
||||
"yield_time_ms": 1_000,
|
||||
@@ -925,7 +949,6 @@ async fn apply_patch_intercepted_exec_command_routes_to_selected_remote_environm
|
||||
call_id,
|
||||
"exec_command",
|
||||
&serde_json::to_string(&json!({
|
||||
"shell": "/bin/sh",
|
||||
"cmd": command,
|
||||
"login": false,
|
||||
"yield_time_ms": 5_000,
|
||||
|
||||
@@ -405,10 +405,16 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
|
||||
});
|
||||
let test = builder.build_with_remote_env(&server).await?;
|
||||
let cwd = test.config.cwd.to_path_buf();
|
||||
let shell_path = test
|
||||
.executor_environment()
|
||||
.environment()
|
||||
.info()
|
||||
.await?
|
||||
.shell
|
||||
.path;
|
||||
|
||||
let call_id = "uexec-begin-event";
|
||||
let args = json!({
|
||||
"shell": "bash".to_string(),
|
||||
"cmd": "/bin/echo hello unified exec".to_string(),
|
||||
"yield_time_ms": 250,
|
||||
});
|
||||
@@ -435,7 +441,12 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
|
||||
})
|
||||
.await;
|
||||
|
||||
assert_command(&begin_event.command, "-lc", "/bin/echo hello unified exec");
|
||||
assert_command(
|
||||
&begin_event.command,
|
||||
&shell_path,
|
||||
"-lc",
|
||||
"/bin/echo hello unified exec",
|
||||
);
|
||||
|
||||
assert_eq!(begin_event.cwd.as_path(), cwd.as_path());
|
||||
|
||||
@@ -1300,10 +1311,16 @@ async fn unified_exec_emits_one_begin_and_one_end_event() -> Result<()> {
|
||||
.expect("test config should allow feature update");
|
||||
});
|
||||
let test = builder.build_with_remote_env(&server).await?;
|
||||
let shell_path = test
|
||||
.executor_environment()
|
||||
.environment()
|
||||
.info()
|
||||
.await?
|
||||
.shell
|
||||
.path;
|
||||
|
||||
let open_call_id = "uexec-open-session";
|
||||
let open_args = json!({
|
||||
"shell": "bash".to_string(),
|
||||
"cmd": "sleep 0.1".to_string(),
|
||||
"yield_time_ms": 10,
|
||||
});
|
||||
@@ -1393,7 +1410,7 @@ async fn unified_exec_emits_one_begin_and_one_end_event() -> Result<()> {
|
||||
|
||||
let open_event = &begin_events[0];
|
||||
|
||||
assert_command(&open_event.command, "-lc", "sleep 0.1");
|
||||
assert_command(&open_event.command, &shell_path, "-lc", "sleep 0.1");
|
||||
|
||||
assert!(
|
||||
open_event.interaction_input.is_none(),
|
||||
@@ -2204,7 +2221,6 @@ async fn write_stdin_ctrl_c_reports_unsupported_interrupt_to_model_on_windows()
|
||||
let interrupt_call_id = "uexec-windows-interrupt";
|
||||
|
||||
let start_args = serde_json::json!({
|
||||
"shell": "cmd",
|
||||
"cmd": "echo READY && ping -n 30 127.0.0.1 >NUL",
|
||||
"yield_time_ms": 250,
|
||||
"tty": false,
|
||||
@@ -3552,16 +3568,14 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn assert_command(command: &[String], expected_args: &str, expected_cmd: &str) {
|
||||
fn assert_command(
|
||||
command: &[String],
|
||||
expected_shell: &str,
|
||||
expected_args: &str,
|
||||
expected_cmd: &str,
|
||||
) {
|
||||
assert_eq!(command.len(), 3);
|
||||
let shell_path = &command[0];
|
||||
assert!(
|
||||
shell_path == "/bin/bash"
|
||||
|| shell_path == "/usr/bin/bash"
|
||||
|| shell_path == "/usr/local/bin/bash"
|
||||
|| shell_path.ends_with("/bash"),
|
||||
"unexpected bash path: {shell_path}"
|
||||
);
|
||||
assert_eq!(command[0], expected_shell);
|
||||
assert_eq!(command[1], expected_args);
|
||||
assert_eq!(command[2], expected_cmd);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user