mirror of
https://github.com/openai/codex.git
synced 2026-05-18 18:22:39 +00:00
## Why When a turn exposes multiple selected environments, shell-style tools need a model-facing way to identify the intended target environment and handlers need to resolve that target before parsing cwd-relative permission fields or launching processes. This PR scopes that rollout to process tools. Filesystem-oriented tools such as `apply_patch`, `view_image`, and `list_dir` are intentionally left for follow-up slices. ## What Changed - Adds an `include_environment_id` option to shell-style tool schema builders. - Exposes optional `environment_id` on `shell`, `shell_command`, and `exec_command` only when `ToolEnvironmentMode::Multiple` is active. - Adds a shared handler helper that parses `environment_id` and `workdir` from JSON function-call arguments and returns the selected `Environment` plus effective absolute cwd. - Uses that helper in `shell`, `shell_command`, and `exec_command` handling so process execution uses the selected environment filesystem and cwd. - Changes `ExecCommandRequest` to carry a required resolved `cwd`, removing the process-manager fallback to the primary turn cwd for new exec commands. - Leaves `write_stdin` unchanged because it targets an existing process id, not a new environment. ## Testing - Added unit coverage for process-tool schema exposure, selected environment resolution, primary fallback, no-environment handling, unknown environment ids, and resolving cwd-relative permission paths against the selected environment cwd. - Added a remote-suite e2e coverage case for `exec_command` routing across explicit zero environments, one local environment, and local+remote environments. - Ran `just fmt` and `git diff --check`. --------- Co-authored-by: Codex <noreply@openai.com>
492 lines
16 KiB
Rust
492 lines
16 KiB
Rust
use anyhow::Context;
|
|
use anyhow::Result;
|
|
use codex_exec_server::CopyOptions;
|
|
use codex_exec_server::CreateDirectoryOptions;
|
|
use codex_exec_server::FileSystemSandboxContext;
|
|
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
|
|
use codex_exec_server::REMOTE_ENVIRONMENT_ID;
|
|
use codex_exec_server::RemoveOptions;
|
|
use codex_features::Feature;
|
|
use codex_protocol::models::PermissionProfile;
|
|
use codex_protocol::permissions::FileSystemAccessMode;
|
|
use codex_protocol::permissions::FileSystemPath;
|
|
use codex_protocol::permissions::FileSystemSandboxEntry;
|
|
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
|
use codex_protocol::permissions::NetworkSandboxPolicy;
|
|
use codex_protocol::protocol::TurnEnvironmentSelection;
|
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
|
use core_test_support::PathBufExt;
|
|
use core_test_support::PathExt;
|
|
use core_test_support::get_remote_test_env;
|
|
use core_test_support::responses::ev_assistant_message;
|
|
use core_test_support::responses::ev_completed;
|
|
use core_test_support::responses::ev_function_call;
|
|
use core_test_support::responses::ev_response_created;
|
|
use core_test_support::responses::mount_sse_sequence;
|
|
use core_test_support::responses::sse;
|
|
use core_test_support::responses::start_mock_server;
|
|
use core_test_support::skip_if_no_network;
|
|
use core_test_support::test_codex::TestCodex;
|
|
use core_test_support::test_codex::test_codex;
|
|
use core_test_support::test_codex::test_env;
|
|
use pretty_assertions::assert_eq;
|
|
use serde_json::Value;
|
|
use serde_json::json;
|
|
use std::fs;
|
|
use std::path::PathBuf;
|
|
use std::process::Command;
|
|
use std::time::SystemTime;
|
|
use std::time::UNIX_EPOCH;
|
|
use tempfile::TempDir;
|
|
async fn unified_exec_test(server: &wiremock::MockServer) -> Result<TestCodex> {
|
|
let mut builder = test_codex().with_config(|config| {
|
|
config.use_experimental_unified_exec_tool = true;
|
|
let result = config.features.enable(Feature::UnifiedExec);
|
|
assert!(
|
|
result.is_ok(),
|
|
"unified exec should enable for test: {result:?}",
|
|
);
|
|
});
|
|
builder.build_remote_aware(server).await
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn remote_test_env_can_connect_and_use_filesystem() -> Result<()> {
|
|
let Some(_remote_env) = get_remote_test_env() else {
|
|
return Ok(());
|
|
};
|
|
|
|
let test_env = test_env().await?;
|
|
let file_system = test_env.environment().get_filesystem();
|
|
|
|
let file_path_abs = remote_test_file_path().abs();
|
|
let payload = b"remote-test-env-ok".to_vec();
|
|
|
|
file_system
|
|
.write_file(&file_path_abs, payload.clone(), /*sandbox*/ None)
|
|
.await?;
|
|
let actual = file_system
|
|
.read_file(&file_path_abs, /*sandbox*/ None)
|
|
.await?;
|
|
assert_eq!(actual, payload);
|
|
|
|
file_system
|
|
.remove(
|
|
&file_path_abs,
|
|
RemoveOptions {
|
|
recursive: false,
|
|
force: true,
|
|
},
|
|
/*sandbox*/ None,
|
|
)
|
|
.await?;
|
|
|
|
Ok(())
|
|
}
|
|
|
|
fn absolute_path(path: PathBuf) -> AbsolutePathBuf {
|
|
match AbsolutePathBuf::try_from(path) {
|
|
Ok(path) => path,
|
|
Err(error) => panic!("path should be absolute: {error}"),
|
|
}
|
|
}
|
|
|
|
fn read_only_sandbox(readable_root: PathBuf) -> FileSystemSandboxContext {
|
|
let readable_root = absolute_path(readable_root);
|
|
FileSystemSandboxContext::from_permission_profile(PermissionProfile::from_runtime_permissions(
|
|
&FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
|
path: FileSystemPath::Path {
|
|
path: readable_root,
|
|
},
|
|
access: FileSystemAccessMode::Read,
|
|
}]),
|
|
NetworkSandboxPolicy::Restricted,
|
|
))
|
|
}
|
|
|
|
fn workspace_write_sandbox(writable_root: PathBuf) -> FileSystemSandboxContext {
|
|
let writable_root = absolute_path(writable_root);
|
|
FileSystemSandboxContext::from_permission_profile(PermissionProfile::from_runtime_permissions(
|
|
&FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
|
path: FileSystemPath::Path {
|
|
path: writable_root,
|
|
},
|
|
access: FileSystemAccessMode::Write,
|
|
}]),
|
|
NetworkSandboxPolicy::Restricted,
|
|
))
|
|
}
|
|
|
|
fn assert_normalized_path_rejected(error: &std::io::Error) {
|
|
match error.kind() {
|
|
std::io::ErrorKind::NotFound => assert!(
|
|
error.to_string().contains("No such file or directory"),
|
|
"unexpected not-found message: {error}",
|
|
),
|
|
std::io::ErrorKind::InvalidInput | std::io::ErrorKind::PermissionDenied => {
|
|
let message = error.to_string();
|
|
assert!(
|
|
message.contains("is not permitted")
|
|
|| message.contains("Operation not permitted")
|
|
|| message.contains("Permission denied"),
|
|
"unexpected rejection message: {message}",
|
|
);
|
|
}
|
|
other => panic!("unexpected normalized-path error kind: {other:?}: {error:?}"),
|
|
}
|
|
}
|
|
|
|
fn remote_exec(script: &str) -> Result<()> {
|
|
let remote_env = get_remote_test_env().context("remote env should be configured")?;
|
|
let output = Command::new("docker")
|
|
.args(["exec", &remote_env.container_name, "sh", "-lc", script])
|
|
.output()?;
|
|
assert!(
|
|
output.status.success(),
|
|
"remote exec failed: stdout={} stderr={}",
|
|
String::from_utf8_lossy(&output.stdout).trim(),
|
|
String::from_utf8_lossy(&output.stderr).trim(),
|
|
);
|
|
Ok(())
|
|
}
|
|
|
|
async fn exec_command_routing_output(
|
|
test: &TestCodex,
|
|
server: &wiremock::MockServer,
|
|
call_id: &str,
|
|
arguments: Value,
|
|
environments: Option<Vec<TurnEnvironmentSelection>>,
|
|
) -> Result<String> {
|
|
let response_mock = mount_sse_sequence(
|
|
server,
|
|
vec![
|
|
sse(vec![
|
|
ev_response_created("resp-1"),
|
|
ev_function_call(call_id, "exec_command", &serde_json::to_string(&arguments)?),
|
|
ev_completed("resp-1"),
|
|
]),
|
|
sse(vec![
|
|
ev_response_created("resp-2"),
|
|
ev_assistant_message("msg-1", "done"),
|
|
ev_completed("resp-2"),
|
|
]),
|
|
],
|
|
)
|
|
.await;
|
|
|
|
test.submit_turn_with_environments("route exec command", environments)
|
|
.await?;
|
|
|
|
response_mock
|
|
.function_call_output_text(call_id)
|
|
.with_context(|| format!("missing function_call_output for {call_id}"))
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn exec_command_routes_to_selected_remote_environment() -> Result<()> {
|
|
skip_if_no_network!(Ok(()));
|
|
let Some(_remote_env) = get_remote_test_env() else {
|
|
return Ok(());
|
|
};
|
|
|
|
let server = start_mock_server().await;
|
|
let test = unified_exec_test(&server).await?;
|
|
let local_cwd = TempDir::new()?;
|
|
fs::write(local_cwd.path().join("marker.txt"), "local-routing")?;
|
|
let local_selection = TurnEnvironmentSelection {
|
|
environment_id: LOCAL_ENVIRONMENT_ID.to_string(),
|
|
cwd: local_cwd.path().abs(),
|
|
};
|
|
let remote_cwd = PathBuf::from(format!(
|
|
"/tmp/codex-remote-routing-{}",
|
|
SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis()
|
|
))
|
|
.abs();
|
|
let remote_marker_name = "marker.txt";
|
|
test.fs()
|
|
.create_directory(
|
|
&remote_cwd,
|
|
CreateDirectoryOptions { recursive: true },
|
|
/*sandbox*/ None,
|
|
)
|
|
.await?;
|
|
test.fs()
|
|
.write_file(
|
|
&remote_cwd.join(remote_marker_name),
|
|
b"remote-routing".to_vec(),
|
|
/*sandbox*/ None,
|
|
)
|
|
.await?;
|
|
let remote_selection = TurnEnvironmentSelection {
|
|
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
|
|
cwd: remote_cwd.clone(),
|
|
};
|
|
let multi_env_output = exec_command_routing_output(
|
|
&test,
|
|
&server,
|
|
"call-multi-env",
|
|
json!({
|
|
"shell": "/bin/sh",
|
|
"cmd": format!("cat {remote_marker_name}"),
|
|
"login": false,
|
|
"yield_time_ms": 1_000,
|
|
"environment_id": REMOTE_ENVIRONMENT_ID,
|
|
}),
|
|
Some(vec![local_selection, remote_selection]),
|
|
)
|
|
.await?;
|
|
assert!(
|
|
multi_env_output.contains("remote-routing"),
|
|
"unexpected multi-env output: {multi_env_output}",
|
|
);
|
|
assert!(
|
|
!multi_env_output.contains("local-routing"),
|
|
"multi-env command should not route to local: {multi_env_output}",
|
|
);
|
|
|
|
test.fs()
|
|
.remove(
|
|
&remote_cwd,
|
|
RemoveOptions {
|
|
recursive: true,
|
|
force: true,
|
|
},
|
|
/*sandbox*/ None,
|
|
)
|
|
.await?;
|
|
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn remote_test_env_sandboxed_read_allows_readable_root() -> Result<()> {
|
|
skip_if_no_network!(Ok(()));
|
|
let Some(_remote_env) = get_remote_test_env() else {
|
|
return Ok(());
|
|
};
|
|
|
|
let test_env = test_env().await?;
|
|
let file_system = test_env.environment().get_filesystem();
|
|
|
|
let allowed_dir = PathBuf::from(format!("/tmp/codex-remote-readable-{}", std::process::id()));
|
|
let file_path = allowed_dir.join("note.txt");
|
|
file_system
|
|
.create_directory(
|
|
&absolute_path(allowed_dir.clone()),
|
|
CreateDirectoryOptions { recursive: true },
|
|
/*sandbox*/ None,
|
|
)
|
|
.await?;
|
|
file_system
|
|
.write_file(
|
|
&absolute_path(file_path.clone()),
|
|
b"sandboxed hello".to_vec(),
|
|
/*sandbox*/ None,
|
|
)
|
|
.await?;
|
|
|
|
let sandbox = read_only_sandbox(allowed_dir.clone());
|
|
let contents = file_system
|
|
.read_file(&absolute_path(file_path.clone()), Some(&sandbox))
|
|
.await?;
|
|
assert_eq!(contents, b"sandboxed hello");
|
|
|
|
file_system
|
|
.remove(
|
|
&absolute_path(allowed_dir),
|
|
RemoveOptions {
|
|
recursive: true,
|
|
force: true,
|
|
},
|
|
/*sandbox*/ None,
|
|
)
|
|
.await?;
|
|
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn remote_test_env_sandboxed_read_rejects_symlink_parent_dotdot_escape() -> Result<()> {
|
|
skip_if_no_network!(Ok(()));
|
|
let Some(_remote_env) = get_remote_test_env() else {
|
|
return Ok(());
|
|
};
|
|
|
|
let test_env = test_env().await?;
|
|
let file_system = test_env.environment().get_filesystem();
|
|
|
|
let root = PathBuf::from(format!("/tmp/codex-remote-dotdot-{}", std::process::id()));
|
|
let allowed_dir = root.join("allowed");
|
|
let outside_dir = root.join("outside");
|
|
let secret_path = root.join("secret.txt");
|
|
remote_exec(&format!(
|
|
"rm -rf {root}; mkdir -p {allowed} {outside}; printf nope > {secret}; ln -s {outside} {allowed}/link",
|
|
root = root.display(),
|
|
allowed = allowed_dir.display(),
|
|
outside = outside_dir.display(),
|
|
secret = secret_path.display(),
|
|
))?;
|
|
|
|
let requested_path = absolute_path(allowed_dir.join("link").join("..").join("secret.txt"));
|
|
let sandbox = read_only_sandbox(allowed_dir.clone());
|
|
let error = match file_system.read_file(&requested_path, Some(&sandbox)).await {
|
|
Ok(_) => anyhow::bail!("read should fail after path normalization"),
|
|
Err(error) => error,
|
|
};
|
|
assert_normalized_path_rejected(&error);
|
|
|
|
remote_exec(&format!("rm -rf {}", root.display()))?;
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn remote_test_env_remove_removes_symlink_not_target() -> Result<()> {
|
|
skip_if_no_network!(Ok(()));
|
|
let Some(_remote_env) = get_remote_test_env() else {
|
|
return Ok(());
|
|
};
|
|
|
|
let test_env = test_env().await?;
|
|
let file_system = test_env.environment().get_filesystem();
|
|
|
|
let root = PathBuf::from(format!(
|
|
"/tmp/codex-remote-remove-link-{}",
|
|
std::process::id()
|
|
));
|
|
let allowed_dir = root.join("allowed");
|
|
let outside_file = root.join("outside").join("keep.txt");
|
|
let symlink_path = allowed_dir.join("link");
|
|
remote_exec(&format!(
|
|
"rm -rf {root}; mkdir -p {allowed} {outside_parent}; printf outside > {outside}; ln -s {outside} {symlink}",
|
|
root = root.display(),
|
|
allowed = allowed_dir.display(),
|
|
outside_parent = absolute_path(
|
|
outside_file
|
|
.parent()
|
|
.context("outside parent should exist")?
|
|
.to_path_buf(),
|
|
)
|
|
.display(),
|
|
outside = outside_file.display(),
|
|
symlink = symlink_path.display(),
|
|
))?;
|
|
|
|
let sandbox = workspace_write_sandbox(allowed_dir.clone());
|
|
file_system
|
|
.remove(
|
|
&absolute_path(symlink_path.clone()),
|
|
RemoveOptions {
|
|
recursive: false,
|
|
force: false,
|
|
},
|
|
Some(&sandbox),
|
|
)
|
|
.await?;
|
|
|
|
let symlink_exists = file_system
|
|
.get_metadata(&absolute_path(symlink_path), /*sandbox*/ None)
|
|
.await
|
|
.is_ok();
|
|
assert!(!symlink_exists);
|
|
let outside = file_system
|
|
.read_file_text(&absolute_path(outside_file.clone()), /*sandbox*/ None)
|
|
.await?;
|
|
assert_eq!(outside, "outside");
|
|
|
|
file_system
|
|
.remove(
|
|
&absolute_path(root),
|
|
RemoveOptions {
|
|
recursive: true,
|
|
force: true,
|
|
},
|
|
/*sandbox*/ None,
|
|
)
|
|
.await?;
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
|
async fn remote_test_env_copy_preserves_symlink_source() -> Result<()> {
|
|
skip_if_no_network!(Ok(()));
|
|
let Some(_remote_env) = get_remote_test_env() else {
|
|
return Ok(());
|
|
};
|
|
|
|
let test_env = test_env().await?;
|
|
let file_system = test_env.environment().get_filesystem();
|
|
|
|
let root = PathBuf::from(format!(
|
|
"/tmp/codex-remote-copy-link-{}",
|
|
std::process::id()
|
|
));
|
|
let allowed_dir = root.join("allowed");
|
|
let outside_file = root.join("outside").join("outside.txt");
|
|
let source_symlink = allowed_dir.join("link");
|
|
let copied_symlink = allowed_dir.join("copied-link");
|
|
remote_exec(&format!(
|
|
"rm -rf {root}; mkdir -p {allowed} {outside_parent}; printf outside > {outside}; ln -s {outside} {source}",
|
|
root = root.display(),
|
|
allowed = allowed_dir.display(),
|
|
outside_parent = outside_file.parent().expect("outside parent").display(),
|
|
outside = outside_file.display(),
|
|
source = source_symlink.display(),
|
|
))?;
|
|
|
|
let sandbox = workspace_write_sandbox(allowed_dir.clone());
|
|
file_system
|
|
.copy(
|
|
&absolute_path(source_symlink),
|
|
&absolute_path(copied_symlink.clone()),
|
|
CopyOptions { recursive: false },
|
|
Some(&sandbox),
|
|
)
|
|
.await?;
|
|
|
|
let link_target = Command::new("docker")
|
|
.args([
|
|
"exec",
|
|
&get_remote_test_env()
|
|
.context("remote env should still be configured")?
|
|
.container_name,
|
|
"readlink",
|
|
copied_symlink
|
|
.to_str()
|
|
.context("copied symlink path should be utf-8")?,
|
|
])
|
|
.output()?;
|
|
assert!(
|
|
link_target.status.success(),
|
|
"readlink failed: stdout={} stderr={}",
|
|
String::from_utf8_lossy(&link_target.stdout).trim(),
|
|
String::from_utf8_lossy(&link_target.stderr).trim(),
|
|
);
|
|
assert_eq!(
|
|
String::from_utf8_lossy(&link_target.stdout).trim(),
|
|
outside_file.to_string_lossy()
|
|
);
|
|
|
|
file_system
|
|
.remove(
|
|
&absolute_path(root),
|
|
RemoveOptions {
|
|
recursive: true,
|
|
force: true,
|
|
},
|
|
/*sandbox*/ None,
|
|
)
|
|
.await?;
|
|
Ok(())
|
|
}
|
|
|
|
fn remote_test_file_path() -> PathBuf {
|
|
let nanos = match SystemTime::now().duration_since(UNIX_EPOCH) {
|
|
Ok(duration) => duration.as_nanos(),
|
|
Err(_) => 0,
|
|
};
|
|
PathBuf::from(format!(
|
|
"/tmp/codex-remote-test-env-{}-{nanos}.txt",
|
|
std::process::id()
|
|
))
|
|
}
|