mirror of
https://github.com/openai/codex.git
synced 2026-11-01 19:33:35 +00:00
Separate executor sandbox selection from Windows sandbox levels (#45730)
## Why MXC is a sandbox implementation, not a restricted-token sandbox level. Executor requests need to represent that choice separately from `WindowsSandboxLevel`. ## What changed - Introduce `WindowsSandboxSelection` for executor sandbox contexts and remove `Mxc` from `WindowsSandboxLevel`. - Preserve the `windowsSandboxLevel` wire field and its serialized values for compatibility. - Share sandbox selection between executor process launches and filesystem helpers, and use the new selection in capability discovery and skill reads. - Disable Windows sandbox selection for executor paths that do not use Windows path conventions. ## Testing Extend coverage for MXC wire serialization, Windows skill-read sandbox checks, and capability discovery with distinct permissions. Exercise remote filesystem write restrictions with both restricted-token and MXC sandboxes, including rejection when native MXC is unavailable. GitOrigin-RevId: 266211377bcb138a0dc75861e9ff2225fa37a53d
This commit is contained in:
@@ -12,20 +12,15 @@ use codex_exec_server::CapabilityRootsDiscoverResponse;
|
||||
use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_exec_server::InitializeParams;
|
||||
use codex_exec_server::InitializeResponse;
|
||||
use codex_exec_server::WindowsSandboxSelection;
|
||||
use codex_exec_server_protocol::CapabilityRootDiscoverRequest;
|
||||
use codex_exec_server_protocol::JSONRPCMessage;
|
||||
use codex_exec_server_protocol::JSONRPCResponse;
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
#[cfg(unix)]
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use common::exec_server::exec_server;
|
||||
@@ -188,10 +183,11 @@ async fn discovers_cursor_plugin_without_reading_default_mcp_for_inline_servers(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn sandboxed_discovery_batches_roots_without_combining_different_permissions()
|
||||
-> anyhow::Result<()> {
|
||||
#[cfg(windows)]
|
||||
crate::skip_if_mxc_unavailable!(Ok(()));
|
||||
let workspace = tempfile::tempdir()?;
|
||||
let first_root = workspace.path().join("first");
|
||||
let second_root = workspace.path().join("second");
|
||||
@@ -216,6 +212,7 @@ async fn sandboxed_discovery_batches_roots_without_combining_different_permissio
|
||||
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted),
|
||||
workspace_uri,
|
||||
);
|
||||
let shared_sandbox = with_native_sandbox(shared_sandbox);
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
let fake_bwrap_directory = tempfile::tempdir()?;
|
||||
@@ -338,6 +335,7 @@ async fn sandboxed_discovery_batches_roots_without_combining_different_permissio
|
||||
),
|
||||
first_uri.clone(),
|
||||
);
|
||||
let first_only_sandbox = with_native_sandbox(first_only_sandbox);
|
||||
let read_second_root = FileSystemSandboxEntry::new(
|
||||
AbsolutePathBuf::from_absolute_path(&second_root)?.into(),
|
||||
FileSystemAccessMode::Read,
|
||||
@@ -350,6 +348,7 @@ async fn sandboxed_discovery_batches_roots_without_combining_different_permissio
|
||||
),
|
||||
second_uri.clone(),
|
||||
);
|
||||
let second_only_sandbox = with_native_sandbox(second_only_sandbox);
|
||||
let response = discover_roots(
|
||||
&mut server,
|
||||
vec![
|
||||
@@ -546,3 +545,10 @@ fn write_file(path: &std::path::Path, contents: &str) -> anyhow::Result<()> {
|
||||
std::fs::write(path, contents)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn with_native_sandbox(mut sandbox: FileSystemSandboxContext) -> FileSystemSandboxContext {
|
||||
if cfg!(windows) {
|
||||
sandbox.windows_sandbox_selection = WindowsSandboxSelection::Mxc;
|
||||
}
|
||||
sandbox
|
||||
}
|
||||
|
||||
@@ -32,6 +32,16 @@ pub(crate) const SYSTEM_PROXY_URL_ENV: &str = "CODEX_EXEC_SERVER_TEST_SYSTEM_PRO
|
||||
const CODEX_WINDOWS_SANDBOX_ARG1: &str = "--run-as-windows-sandbox";
|
||||
const DELAYED_OUTPUT_AFTER_EXIT_CHILD_ARG: &str = "--codex-test-delayed-output-after-exit-child";
|
||||
|
||||
#[macro_export]
|
||||
macro_rules! skip_if_mxc_unavailable {
|
||||
($return_value:expr $(,)?) => {{
|
||||
if !codex_sandboxing::windows_mxc_available() {
|
||||
eprintln!("skipping test: native MXC is unavailable on this host");
|
||||
return $return_value;
|
||||
}
|
||||
}};
|
||||
}
|
||||
|
||||
#[ctor]
|
||||
pub static TEST_BINARY_DISPATCH_GUARD: Option<TestBinaryDispatchGuard> = {
|
||||
let guard = configure_test_binary_dispatch("codex-exec-server-tests", |exe_name, argv1| {
|
||||
|
||||
@@ -27,6 +27,8 @@ use codex_exec_server::ShellInfo;
|
||||
#[cfg(unix)]
|
||||
use codex_exec_server::ShellSnapshotRequest;
|
||||
use codex_exec_server::StartedExecProcess;
|
||||
#[cfg(any(unix, windows))]
|
||||
use codex_exec_server::WindowsSandboxSelection;
|
||||
use codex_exec_server::WriteStatus;
|
||||
#[cfg(unix)]
|
||||
use codex_network_proxy::NetworkProxyConfig;
|
||||
@@ -36,7 +38,6 @@ use codex_network_proxy::RemoteNetworkProxyConfig;
|
||||
use codex_network_proxy::RemoteNetworkProxyLaunchConfig;
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicyInherit;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
#[cfg(unix)]
|
||||
@@ -104,10 +105,6 @@ async fn create_process_context(use_remote: bool) -> Result<ProcessContext> {
|
||||
}
|
||||
}
|
||||
|
||||
fn selected_windows_sandbox_available(windows_sandbox_level: WindowsSandboxLevel) -> bool {
|
||||
windows_sandbox_level != WindowsSandboxLevel::Mxc || codex_sandboxing::windows_mxc_available()
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn codex_home_symlink_opt_out_respects_host_config_and_scope() -> Result<()> {
|
||||
@@ -1295,13 +1292,11 @@ async fn assert_exec_process_write_then_read_without_tty(use_remote: bool) -> Re
|
||||
}
|
||||
|
||||
async fn assert_remote_windows_sandbox_process_write(
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
expected_sandbox_type: codex_sandboxing::SandboxType,
|
||||
tty: bool,
|
||||
) -> Result<()> {
|
||||
if !selected_windows_sandbox_available(windows_sandbox_level) {
|
||||
eprintln!("skipping MXC enforcement test: native MXC is unavailable on this host");
|
||||
return Ok(());
|
||||
if expected_sandbox_type == codex_sandboxing::SandboxType::WindowsMxc {
|
||||
crate::skip_if_mxc_unavailable!(Ok(()));
|
||||
}
|
||||
let context = create_process_context(/*use_remote*/ true).await?;
|
||||
let workspace = TempDir::new()?;
|
||||
@@ -1311,7 +1306,19 @@ async fn assert_remote_windows_sandbox_process_write(
|
||||
SandboxPolicy::new_read_only_policy(),
|
||||
cwd.clone(),
|
||||
)?;
|
||||
sandbox.windows_sandbox_level = windows_sandbox_level;
|
||||
match expected_sandbox_type {
|
||||
codex_sandboxing::SandboxType::WindowsRestrictedToken => {
|
||||
sandbox.windows_sandbox_selection = WindowsSandboxSelection::RestrictedToken;
|
||||
}
|
||||
codex_sandboxing::SandboxType::WindowsMxc => {
|
||||
sandbox.windows_sandbox_selection = WindowsSandboxSelection::Mxc;
|
||||
}
|
||||
codex_sandboxing::SandboxType::None
|
||||
| codex_sandboxing::SandboxType::MacosSeatbelt
|
||||
| codex_sandboxing::SandboxType::LinuxSeccomp => {
|
||||
anyhow::bail!("expected a Windows sandbox type")
|
||||
}
|
||||
}
|
||||
|
||||
let session = match context
|
||||
.backend
|
||||
@@ -1767,19 +1774,16 @@ async fn exec_process_write_then_read_without_tty(use_remote: bool) -> Result<()
|
||||
}
|
||||
|
||||
#[test_case(
|
||||
WindowsSandboxLevel::RestrictedToken,
|
||||
codex_sandboxing::SandboxType::WindowsRestrictedToken,
|
||||
false;
|
||||
"restricted_token"
|
||||
)]
|
||||
#[test_case(
|
||||
WindowsSandboxLevel::Mxc,
|
||||
codex_sandboxing::SandboxType::WindowsMxc,
|
||||
false;
|
||||
"mxc_pipe"
|
||||
)]
|
||||
#[test_case(
|
||||
WindowsSandboxLevel::Mxc,
|
||||
codex_sandboxing::SandboxType::WindowsMxc,
|
||||
true;
|
||||
"mxc_conpty"
|
||||
@@ -1788,12 +1792,10 @@ async fn exec_process_write_then_read_without_tty(use_remote: bool) -> Result<()
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[serial_test::serial(remote_exec_server)]
|
||||
async fn remote_windows_sandbox_process_accepts_process_write(
|
||||
windows_sandbox_level: WindowsSandboxLevel,
|
||||
expected_sandbox_type: codex_sandboxing::SandboxType,
|
||||
tty: bool,
|
||||
) -> Result<()> {
|
||||
assert_remote_windows_sandbox_process_write(windows_sandbox_level, expected_sandbox_type, tty)
|
||||
.await
|
||||
assert_remote_windows_sandbox_process_write(expected_sandbox_type, tty).await
|
||||
}
|
||||
|
||||
#[test_case(false ; "local")]
|
||||
|
||||
@@ -14,10 +14,7 @@ use pretty_assertions::assert_eq;
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[serial_test::serial(remote_exec_server)]
|
||||
async fn mxc_tmpdir_uses_command_environment_over_rpc() -> Result<()> {
|
||||
if !selected_windows_sandbox_available(WindowsSandboxLevel::Mxc) {
|
||||
eprintln!("skipping MXC enforcement test: native MXC is unavailable on this host");
|
||||
return Ok(());
|
||||
}
|
||||
crate::skip_if_mxc_unavailable!(Ok(()));
|
||||
let root = TempDir::new()?;
|
||||
let command_temp = root.path().join("command temp");
|
||||
let server_temp = root.path().join("server temp");
|
||||
@@ -53,7 +50,7 @@ async fn mxc_tmpdir_uses_command_environment_over_rpc() -> Result<()> {
|
||||
PermissionProfile::from_runtime_permissions(&fs, NetworkSandboxPolicy::Restricted),
|
||||
cwd.clone(),
|
||||
);
|
||||
sandbox.windows_sandbox_level = WindowsSandboxLevel::Mxc;
|
||||
sandbox.windows_sandbox_selection = codex_exec_server::WindowsSandboxSelection::Mxc;
|
||||
let command_temp = command_temp.to_string_lossy().into_owned();
|
||||
let started = environment
|
||||
.get_exec_backend()
|
||||
|
||||
@@ -7,7 +7,7 @@ use codex_exec_server::ExecServerRuntimePaths;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_exec_server::LocalFileSystem;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_exec_server::WindowsSandboxSelection;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
@@ -141,7 +141,7 @@ pub(crate) fn workspace_write_sandbox(
|
||||
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted),
|
||||
PathUri::from_abs_path(&writable_root),
|
||||
);
|
||||
sandbox.windows_sandbox_level = WindowsSandboxLevel::RestrictedToken;
|
||||
sandbox.windows_sandbox_selection = WindowsSandboxSelection::RestrictedToken;
|
||||
sandbox
|
||||
}
|
||||
|
||||
@@ -163,7 +163,7 @@ fn sandbox_context(mut entries: Vec<FileSystemSandboxEntry>) -> FileSystemSandbo
|
||||
),
|
||||
);
|
||||
if cfg!(windows) {
|
||||
sandbox.windows_sandbox_level = WindowsSandboxLevel::RestrictedToken;
|
||||
sandbox.windows_sandbox_selection = WindowsSandboxSelection::RestrictedToken;
|
||||
}
|
||||
sandbox
|
||||
}
|
||||
|
||||
@@ -20,9 +20,10 @@ use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_exec_server::GetMetadataOptions;
|
||||
use codex_exec_server::ReadFileOptions;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_exec_server::WindowsSandboxSelection;
|
||||
use codex_exec_server::WriteFileOptions;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use futures::TryStreamExt;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -346,8 +347,12 @@ async fn file_system_no_follow_operations_reject_named_pipes(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(SandboxType::WindowsRestrictedToken; "restricted_token")]
|
||||
#[test_case(SandboxType::WindowsMxc; "mxc")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn file_system_remote_fs_helper_respects_windows_sandbox_write_policy() -> Result<()> {
|
||||
async fn file_system_remote_fs_helper_respects_windows_sandbox_write_policy(
|
||||
sandbox_type: SandboxType,
|
||||
) -> Result<()> {
|
||||
let context = create_file_system_context(FileSystemImplementation::Remote).await?;
|
||||
let file_system = context.file_system;
|
||||
let tmp = tempfile::TempDir::new()?;
|
||||
@@ -355,7 +360,39 @@ async fn file_system_remote_fs_helper_respects_windows_sandbox_write_policy() ->
|
||||
std::fs::create_dir_all(&readonly_dir)?;
|
||||
|
||||
let mut sandbox = read_only_sandbox_for_cwd(readonly_dir.clone())?;
|
||||
sandbox.windows_sandbox_level = WindowsSandboxLevel::RestrictedToken;
|
||||
match sandbox_type {
|
||||
SandboxType::WindowsRestrictedToken => {
|
||||
sandbox.windows_sandbox_selection = WindowsSandboxSelection::RestrictedToken;
|
||||
}
|
||||
SandboxType::WindowsMxc => {
|
||||
sandbox.windows_sandbox_selection = WindowsSandboxSelection::Mxc;
|
||||
}
|
||||
SandboxType::None | SandboxType::MacosSeatbelt | SandboxType::LinuxSeccomp => {
|
||||
anyhow::bail!("expected a Windows sandbox type")
|
||||
}
|
||||
}
|
||||
|
||||
let blocked_file = readonly_dir.join("blocked.txt");
|
||||
if sandbox_type == SandboxType::WindowsMxc && !codex_sandboxing::windows_mxc_available() {
|
||||
let error = file_system
|
||||
.write_file(
|
||||
&PathUri::from_host_native_path(&blocked_file)?,
|
||||
b"blocked".to_vec(),
|
||||
WriteFileOptions::default(),
|
||||
Some(&sandbox),
|
||||
)
|
||||
.await
|
||||
.expect_err("unavailable MXC must fail closed");
|
||||
assert_eq!(
|
||||
(error.kind(), error.to_string()),
|
||||
(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"failed to prepare fs sandbox: failed to prepare MXC sandbox: native MXC is unavailable on this executor".to_owned(),
|
||||
)
|
||||
);
|
||||
assert!(!blocked_file.exists());
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let readable_file = readonly_dir.join("readable.txt");
|
||||
std::fs::write(&readable_file, b"readable")?;
|
||||
@@ -369,12 +406,13 @@ async fn file_system_remote_fs_helper_respects_windows_sandbox_write_policy() ->
|
||||
// Some local Windows hosts cannot create restricted tokens. Reaching that
|
||||
// error still proves the remote fs helper went through the Windows sandbox
|
||||
// launcher; before the wrapper fix this read would have run unsandboxed.
|
||||
if is_unsupported_restricted_token_host(&read_result) {
|
||||
if sandbox_type == SandboxType::WindowsRestrictedToken
|
||||
&& is_unsupported_restricted_token_host(&read_result)
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
assert_eq!(read_result?, b"readable");
|
||||
|
||||
let blocked_file = readonly_dir.join("blocked.txt");
|
||||
let error = file_system
|
||||
.write_file(
|
||||
&PathUri::from_host_native_path(&blocked_file)?,
|
||||
@@ -450,7 +488,7 @@ async fn file_system_private_desktop_survives_helper_exits_and_separates_permiss
|
||||
}
|
||||
|
||||
let mut readonly = read_only_sandbox_for_cwd(tmp.path().to_path_buf())?;
|
||||
readonly.windows_sandbox_level = WindowsSandboxLevel::RestrictedToken;
|
||||
readonly.windows_sandbox_selection = WindowsSandboxSelection::RestrictedToken;
|
||||
readonly.windows_sandbox_private_desktop = true;
|
||||
assert_eq!(
|
||||
file_system
|
||||
|
||||
Reference in New Issue
Block a user