mirror of
https://github.com/openai/codex.git
synced 2026-11-13 06:45:07 +00:00
Honor the configured Windows sandbox level for managed networking (#37875)
## Why Managed networking implicitly selected the elevated Windows sandbox backend, even when the sandbox was configured to use a restricted token. ## What changed - Select the Windows sandbox backend solely from `WindowsSandboxLevel`. - Reject managed networking with a restricted-token sandbox before spawning a process, since managed networking requires the elevated backend. - Cover the rejection through sandbox preparation, unified exec, and Windows sandbox session tests. GitOrigin-RevId: 308858652d7b629af623d22896dafde3a23d3758
This commit is contained in:
1 parent
46f5d6eba9
commit
a603d7ca5c
9 files changed
+129
-51
No files matched your search
@@ -406,10 +406,8 @@ pub fn build_exec_request(
|
|||||||
)
|
)
|
||||||
})
|
})
|
||||||
.map_err(CodexErr::from)?;
|
.map_err(CodexErr::from)?;
|
||||||
let use_windows_elevated_backend = windows_sandbox_uses_elevated_backend(
|
let use_windows_elevated_backend =
|
||||||
exec_req.windows_sandbox_level,
|
windows_sandbox_uses_elevated_backend(exec_req.windows_sandbox_level);
|
||||||
exec_req.network.is_some(),
|
|
||||||
);
|
|
||||||
exec_req.windows_sandbox_filesystem_overrides = if use_windows_elevated_backend {
|
exec_req.windows_sandbox_filesystem_overrides = if use_windows_elevated_backend {
|
||||||
resolve_windows_elevated_filesystem_overrides(
|
resolve_windows_elevated_filesystem_overrides(
|
||||||
exec_req.sandbox,
|
exec_req.sandbox,
|
||||||
@@ -665,7 +663,7 @@ async fn exec_windows_sandbox(
|
|||||||
let command_path = command.first().cloned();
|
let command_path = command.first().cloned();
|
||||||
let sandbox_level = windows_sandbox_level;
|
let sandbox_level = windows_sandbox_level;
|
||||||
let proxy_enforced = network.is_some();
|
let proxy_enforced = network.is_some();
|
||||||
let use_elevated = windows_sandbox_uses_elevated_backend(sandbox_level, proxy_enforced);
|
let use_elevated = windows_sandbox_uses_elevated_backend(sandbox_level);
|
||||||
let additional_deny_write_paths = windows_sandbox_filesystem_overrides
|
let additional_deny_write_paths = windows_sandbox_filesystem_overrides
|
||||||
.map(|overrides| overrides.additional_deny_write_paths.clone())
|
.map(|overrides| overrides.additional_deny_write_paths.clone())
|
||||||
.unwrap_or_default();
|
.unwrap_or_default();
|
||||||
|
|||||||
@@ -398,18 +398,12 @@ fn windows_restricted_token_supports_read_only_profiles() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn windows_proxy_enforcement_uses_elevated_backend() {
|
fn windows_sandbox_backend_honors_unelevated_configuration() {
|
||||||
assert!(!windows_sandbox_uses_elevated_backend(
|
assert!(!windows_sandbox_uses_elevated_backend(
|
||||||
WindowsSandboxLevel::RestrictedToken,
|
WindowsSandboxLevel::RestrictedToken
|
||||||
/*proxy_enforced*/ false,
|
|
||||||
));
|
));
|
||||||
assert!(windows_sandbox_uses_elevated_backend(
|
assert!(windows_sandbox_uses_elevated_backend(
|
||||||
WindowsSandboxLevel::RestrictedToken,
|
WindowsSandboxLevel::Elevated
|
||||||
/*proxy_enforced*/ true,
|
|
||||||
));
|
|
||||||
assert!(windows_sandbox_uses_elevated_backend(
|
|
||||||
WindowsSandboxLevel::Elevated,
|
|
||||||
/*proxy_enforced*/ false,
|
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1028,6 +1028,43 @@ async fn unified_exec_short_lived_network_denial_emits_failed_end_event() -> Res
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(windows)]
|
||||||
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
|
async fn unified_exec_rejects_unelevated_windows_sandbox_with_managed_network() -> Result<()> {
|
||||||
|
let server = start_mock_server().await;
|
||||||
|
let (test, permission_profile) = unified_exec_network_denial_test(&server).await?;
|
||||||
|
let call_id = "uexec-unelevated-managed-network";
|
||||||
|
let args = json!({
|
||||||
|
"cmd": "echo should not run",
|
||||||
|
"yield_time_ms": 1_000,
|
||||||
|
});
|
||||||
|
let responses = mount_unified_exec_network_denial_responses(&server, call_id, &args).await?;
|
||||||
|
|
||||||
|
submit_unified_exec_turn(
|
||||||
|
&test,
|
||||||
|
"run an unelevated managed-network command",
|
||||||
|
permission_profile,
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
wait_for_event(&test.codex, |event| {
|
||||||
|
matches!(event, EventMsg::TurnComplete(_))
|
||||||
|
})
|
||||||
|
.await;
|
||||||
|
|
||||||
|
let output_item = responses
|
||||||
|
.last_request()
|
||||||
|
.expect("model should receive the rejected tool call output")
|
||||||
|
.function_call_output(call_id);
|
||||||
|
let output = extract_output_text(&output_item)
|
||||||
|
.expect("rejected tool call should include model-visible text");
|
||||||
|
assert!(
|
||||||
|
output.contains("managed networking requires the elevated Windows sandbox backend"),
|
||||||
|
"unexpected output: {output}"
|
||||||
|
);
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
async fn unified_exec_network_denial_test(
|
async fn unified_exec_network_denial_test(
|
||||||
server: &wiremock::MockServer,
|
server: &wiremock::MockServer,
|
||||||
) -> Result<(TestCodex, PermissionProfile)> {
|
) -> Result<(TestCodex, PermissionProfile)> {
|
||||||
@@ -1070,6 +1107,8 @@ allow_local_binding = true
|
|||||||
.permissions
|
.permissions
|
||||||
.set_permission_profile(permission_profile_for_config)
|
.set_permission_profile(permission_profile_for_config)
|
||||||
.expect("set permission profile");
|
.expect("set permission profile");
|
||||||
|
#[cfg(windows)]
|
||||||
|
config.set_windows_sandbox_enabled(/*value*/ true);
|
||||||
});
|
});
|
||||||
let test = builder.build_with_auto_env(server).await?;
|
let test = builder.build_with_auto_env(server).await?;
|
||||||
assert!(
|
assert!(
|
||||||
|
|||||||
@@ -248,10 +248,8 @@ pub(crate) async fn prepare_exec_request(
|
|||||||
let windows_sandbox = if sandbox == SandboxType::WindowsRestrictedToken {
|
let windows_sandbox = if sandbox == SandboxType::WindowsRestrictedToken {
|
||||||
request.arg0 = params.arg0.clone();
|
request.arg0 = params.arg0.clone();
|
||||||
let proxy_enforced = params.enforce_managed_network;
|
let proxy_enforced = params.enforce_managed_network;
|
||||||
let use_elevated = windows_sandbox_uses_elevated_backend(
|
let use_elevated =
|
||||||
sandbox_context.windows_sandbox_level,
|
windows_sandbox_uses_elevated_backend(sandbox_context.windows_sandbox_level);
|
||||||
proxy_enforced,
|
|
||||||
);
|
|
||||||
let filesystem_overrides = if use_elevated {
|
let filesystem_overrides = if use_elevated {
|
||||||
resolve_windows_elevated_filesystem_overrides(
|
resolve_windows_elevated_filesystem_overrides(
|
||||||
sandbox,
|
sandbox,
|
||||||
|
|||||||
@@ -17,6 +17,8 @@ use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0;
|
|||||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||||
use codex_utils_path_uri::PathUri;
|
use codex_utils_path_uri::PathUri;
|
||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
|
#[cfg(windows)]
|
||||||
|
use test_case::test_case;
|
||||||
use tokio::io::AsyncReadExt;
|
use tokio::io::AsyncReadExt;
|
||||||
use tokio::io::AsyncWriteExt;
|
use tokio::io::AsyncWriteExt;
|
||||||
use tokio::time::timeout;
|
use tokio::time::timeout;
|
||||||
@@ -379,8 +381,10 @@ async fn disabled_remote_proxy_config_is_rejected_before_exporting_ports() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(windows)]
|
#[cfg(windows)]
|
||||||
|
#[test_case(WindowsSandboxLevel::RestrictedToken ; "unelevated is rejected")]
|
||||||
|
#[test_case(WindowsSandboxLevel::Elevated ; "elevated is accepted")]
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn managed_network_selects_elevated_windows_spawn() {
|
async fn managed_network_honors_windows_sandbox_level(windows_sandbox_level: WindowsSandboxLevel) {
|
||||||
let cwd: AbsolutePathBuf = std::env::current_dir()
|
let cwd: AbsolutePathBuf = std::env::current_dir()
|
||||||
.expect("current directory")
|
.expect("current directory")
|
||||||
.try_into()
|
.try_into()
|
||||||
@@ -393,7 +397,7 @@ async fn managed_network_selects_elevated_windows_spawn() {
|
|||||||
permissions.clone(),
|
permissions.clone(),
|
||||||
cwd_uri.clone(),
|
cwd_uri.clone(),
|
||||||
);
|
);
|
||||||
sandbox.windows_sandbox_level = WindowsSandboxLevel::RestrictedToken;
|
sandbox.windows_sandbox_level = windows_sandbox_level;
|
||||||
sandbox.windows_sandbox_proxy_settings_mode =
|
sandbox.windows_sandbox_proxy_settings_mode =
|
||||||
Some(codex_sandboxing::WindowsSandboxProxySettingsMode::Preserve);
|
Some(codex_sandboxing::WindowsSandboxProxySettingsMode::Preserve);
|
||||||
let proxy_config = RemoteNetworkProxyConfig::from_effective_config(&NetworkProxyConfig {
|
let proxy_config = RemoteNetworkProxyConfig::from_effective_config(&NetworkProxyConfig {
|
||||||
@@ -417,32 +421,34 @@ async fn managed_network_selects_elevated_windows_spawn() {
|
|||||||
network_proxy: Some(RemoteNetworkProxyLaunchConfig::new(proxy_config)),
|
network_proxy: Some(RemoteNetworkProxyLaunchConfig::new(proxy_config)),
|
||||||
};
|
};
|
||||||
|
|
||||||
let mut prepared = prepare_exec_request(
|
let prepared = prepare_exec_request(
|
||||||
¶ms,
|
¶ms,
|
||||||
HashMap::new(),
|
HashMap::new(),
|
||||||
Some(&runtime_paths),
|
Some(&runtime_paths),
|
||||||
/*network_policy_decider*/ None,
|
/*network_policy_decider*/ None,
|
||||||
)
|
)
|
||||||
.await
|
.await;
|
||||||
.expect("prepare sandboxed request");
|
|
||||||
{
|
if windows_sandbox_level == WindowsSandboxLevel::RestrictedToken {
|
||||||
|
let error = prepared
|
||||||
|
.err()
|
||||||
|
.expect("managed networking must reject an unelevated Windows sandbox");
|
||||||
|
assert_eq!(error.code, -32602);
|
||||||
|
assert!(
|
||||||
|
error
|
||||||
|
.message
|
||||||
|
.contains("managed networking requires the elevated Windows sandbox backend")
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
let mut prepared = prepared.expect("managed networking accepts an elevated Windows sandbox");
|
||||||
let spawn = prepared
|
let spawn = prepared
|
||||||
.windows_sandbox_spawn_request()
|
.windows_sandbox_spawn_request()
|
||||||
.expect("Windows sandbox spawn request");
|
.expect("Windows sandbox spawn request");
|
||||||
|
assert_eq!(spawn.windows_sandbox_level, WindowsSandboxLevel::Elevated);
|
||||||
assert_eq!(
|
|
||||||
spawn.windows_sandbox_level,
|
|
||||||
WindowsSandboxLevel::RestrictedToken
|
|
||||||
);
|
|
||||||
assert!(spawn.proxy_enforced);
|
assert!(spawn.proxy_enforced);
|
||||||
assert!(spawn.network_proxy_restricting_sid.is_some());
|
assert!(spawn.network_proxy_restricting_sid.is_some());
|
||||||
assert_eq!(
|
|
||||||
spawn.proxy_settings_mode,
|
|
||||||
codex_sandboxing::WindowsSandboxProxySettingsMode::Preserve
|
|
||||||
);
|
|
||||||
assert_eq!(spawn.permission_profile, &permissions);
|
|
||||||
assert_eq!(spawn.workspace_roots, std::slice::from_ref(&cwd));
|
|
||||||
}
|
|
||||||
prepared
|
prepared
|
||||||
.network_proxy_handle
|
.network_proxy_handle
|
||||||
.take()
|
.take()
|
||||||
|
|||||||
@@ -404,11 +404,20 @@ impl SandboxManager {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
#[cfg(target_os = "windows")]
|
#[cfg(target_os = "windows")]
|
||||||
SandboxType::WindowsRestrictedToken => (
|
SandboxType::WindowsRestrictedToken => {
|
||||||
|
if enforce_managed_network && windows_sandbox_level != WindowsSandboxLevel::Elevated
|
||||||
|
{
|
||||||
|
return Err(SandboxTransformError::WindowsSandboxPreparation(
|
||||||
|
"managed networking requires the elevated Windows sandbox backend"
|
||||||
|
.to_string(),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
(
|
||||||
os_argv_to_strings(argv),
|
os_argv_to_strings(argv),
|
||||||
None,
|
None,
|
||||||
Some(pending_sandboxed_request?),
|
Some(pending_sandboxed_request?),
|
||||||
),
|
)
|
||||||
|
}
|
||||||
#[cfg(not(target_os = "windows"))]
|
#[cfg(not(target_os = "windows"))]
|
||||||
SandboxType::WindowsRestrictedToken => (
|
SandboxType::WindowsRestrictedToken => (
|
||||||
os_argv_to_strings(argv),
|
os_argv_to_strings(argv),
|
||||||
@@ -524,8 +533,7 @@ fn wrap_windows_sandbox_exec_request_for_direct_spawn(
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
.transpose()?;
|
.transpose()?;
|
||||||
let use_elevated =
|
let use_elevated = windows_sandbox_uses_elevated_backend(request.windows_sandbox_level);
|
||||||
windows_sandbox_uses_elevated_backend(request.windows_sandbox_level, proxy_enforced);
|
|
||||||
let overrides = if use_elevated {
|
let overrides = if use_elevated {
|
||||||
resolve_windows_elevated_filesystem_overrides(
|
resolve_windows_elevated_filesystem_overrides(
|
||||||
request.sandbox,
|
request.sandbox,
|
||||||
|
|||||||
@@ -29,14 +29,8 @@ pub struct WindowsSandboxFilesystemOverrides {
|
|||||||
pub additional_deny_write_paths: Vec<AbsolutePathBuf>,
|
pub additional_deny_write_paths: Vec<AbsolutePathBuf>,
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn windows_sandbox_uses_elevated_backend(
|
pub fn windows_sandbox_uses_elevated_backend(sandbox_level: WindowsSandboxLevel) -> bool {
|
||||||
sandbox_level: WindowsSandboxLevel,
|
matches!(sandbox_level, WindowsSandboxLevel::Elevated)
|
||||||
proxy_enforced: bool,
|
|
||||||
) -> bool {
|
|
||||||
// Windows firewall enforcement is tied to the logon-user sandbox identities, so
|
|
||||||
// proxy-enforced sessions must use that backend even when the configured mode is
|
|
||||||
// the default restricted-token sandbox.
|
|
||||||
proxy_enforced || matches!(sandbox_level, WindowsSandboxLevel::Elevated)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn permission_profile_supports_windows_restricted_token_sandbox(
|
pub fn permission_profile_supports_windows_restricted_token_sandbox(
|
||||||
|
|||||||
@@ -48,9 +48,7 @@ pub struct WindowsSandboxSessionRequest<'a> {
|
|||||||
pub async fn spawn_windows_sandbox_session_for_level(
|
pub async fn spawn_windows_sandbox_session_for_level(
|
||||||
request: WindowsSandboxSessionRequest<'_>,
|
request: WindowsSandboxSessionRequest<'_>,
|
||||||
) -> Result<SpawnedProcess> {
|
) -> Result<SpawnedProcess> {
|
||||||
if request.proxy_enforced
|
if matches!(request.windows_sandbox_level, WindowsSandboxLevel::Elevated) {
|
||||||
|| matches!(request.windows_sandbox_level, WindowsSandboxLevel::Elevated)
|
|
||||||
{
|
|
||||||
backends::elevated::spawn_windows_sandbox_session_elevated_for_permission_profile(
|
backends::elevated::spawn_windows_sandbox_session_elevated_for_permission_profile(
|
||||||
request.permission_profile,
|
request.permission_profile,
|
||||||
request.workspace_roots,
|
request.workspace_roots,
|
||||||
@@ -73,6 +71,9 @@ pub async fn spawn_windows_sandbox_session_for_level(
|
|||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
} else {
|
} else {
|
||||||
|
if request.proxy_enforced {
|
||||||
|
bail!("managed networking requires the elevated Windows sandbox backend");
|
||||||
|
}
|
||||||
if request.network_proxy_restricting_sid.is_some() {
|
if request.network_proxy_restricting_sid.is_some() {
|
||||||
bail!("network proxy restricting SID requires the elevated Windows sandbox backend");
|
bail!("network proxy restricting SID requires the elevated Windows sandbox backend");
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
#![cfg(target_os = "windows")]
|
#![cfg(target_os = "windows")]
|
||||||
|
|
||||||
|
use super::WindowsSandboxSessionRequest;
|
||||||
use super::spawn_windows_sandbox_session_elevated_for_permission_profile;
|
use super::spawn_windows_sandbox_session_elevated_for_permission_profile;
|
||||||
|
use super::spawn_windows_sandbox_session_for_level;
|
||||||
use super::spawn_windows_sandbox_session_legacy;
|
use super::spawn_windows_sandbox_session_legacy;
|
||||||
use crate::WindowsSandboxCancellationToken;
|
use crate::WindowsSandboxCancellationToken;
|
||||||
use crate::ipc_framed::Message;
|
use crate::ipc_framed::Message;
|
||||||
@@ -9,6 +11,7 @@ use crate::ipc_framed::read_frame;
|
|||||||
use crate::run_windows_sandbox_capture;
|
use crate::run_windows_sandbox_capture;
|
||||||
use base64::Engine;
|
use base64::Engine;
|
||||||
use base64::engine::general_purpose::STANDARD as BASE64;
|
use base64::engine::general_purpose::STANDARD as BASE64;
|
||||||
|
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||||
use codex_protocol::models::PermissionProfile;
|
use codex_protocol::models::PermissionProfile;
|
||||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||||
use codex_utils_pty::ProcessDriver;
|
use codex_utils_pty::ProcessDriver;
|
||||||
@@ -221,6 +224,43 @@ async fn collect_stdout_and_exit(
|
|||||||
(stdout, exit_code)
|
(stdout, exit_code)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn restricted_token_rejects_managed_network_before_spawn() {
|
||||||
|
current_thread_runtime().block_on(async {
|
||||||
|
let cwd = sandbox_cwd();
|
||||||
|
let codex_home = sandbox_home("restricted-token-managed-network");
|
||||||
|
let permission_profile = PermissionProfile::workspace_write();
|
||||||
|
let error = spawn_windows_sandbox_session_for_level(WindowsSandboxSessionRequest {
|
||||||
|
permission_profile: &permission_profile,
|
||||||
|
workspace_roots: &[],
|
||||||
|
codex_home: codex_home.path(),
|
||||||
|
command: Vec::new(),
|
||||||
|
cwd: cwd.as_path(),
|
||||||
|
env_map: HashMap::new(),
|
||||||
|
windows_sandbox_level: WindowsSandboxLevel::RestrictedToken,
|
||||||
|
proxy_enforced: true,
|
||||||
|
network_proxy_restricting_sid: None,
|
||||||
|
proxy_settings_mode: crate::WindowsSandboxProxySettingsMode::Preserve,
|
||||||
|
timeout_ms: None,
|
||||||
|
read_roots_override: None,
|
||||||
|
read_roots_include_platform_defaults: false,
|
||||||
|
write_roots_override: None,
|
||||||
|
deny_read_paths_override: &[],
|
||||||
|
deny_write_paths_override: &[],
|
||||||
|
tty: false,
|
||||||
|
stdin_open: false,
|
||||||
|
use_private_desktop: false,
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
.expect_err("managed networking must fail before spawning an unelevated sandbox");
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
error.to_string(),
|
||||||
|
"managed networking requires the elevated Windows sandbox backend"
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn legacy_non_tty_cmd_emits_output() {
|
fn legacy_non_tty_cmd_emits_output() {
|
||||||
let _guard = legacy_process_test_guard();
|
let _guard = legacy_process_test_guard();
|
||||||
|
|||||||
Reference in new issue
Block a user