mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
Honor disabled managed network configuration (#40489)
## Why Managed network requirements can remain present when `enabled = false`, but a disabled configuration should not create or expose an active proxy runtime. ## What changed - Start the managed network proxy only when its effective specification is enabled. - Clear an existing proxy when a permission-profile refresh disables managed networking. - Keep remote executor launches free of managed-network proxy settings when the requirements explicitly disable the feature. ## Testing Added session and unified-exec coverage for initial startup and refresh with disabled managed networking. GitOrigin-RevId: f76699802e079eb746d5b1ea6547d19a4bfb7715
This commit is contained in:
@@ -1128,6 +1128,12 @@ impl Session {
|
||||
spec
|
||||
}
|
||||
};
|
||||
// Disabled specs still carry managed requirements and constraints, but they do not have
|
||||
// listeners and must not be exposed as active managed proxy runtimes.
|
||||
if !spec.enabled() {
|
||||
self.services.network_proxy.store(None);
|
||||
return;
|
||||
}
|
||||
if let Some(started_proxy) = self.services.network_proxy.load_full() {
|
||||
if let Err(err) = spec.apply_to_started_proxy(started_proxy.as_ref()).await {
|
||||
warn!("failed to refresh managed network proxy for sandbox change: {err}");
|
||||
|
||||
@@ -1274,7 +1274,12 @@ impl Session {
|
||||
)
|
||||
});
|
||||
let (network_proxy, session_network_proxy) =
|
||||
if let Some(spec) = config.permissions.network.as_ref() {
|
||||
if let Some(spec) = config
|
||||
.permissions
|
||||
.network
|
||||
.as_ref()
|
||||
.filter(|spec| spec.enabled())
|
||||
{
|
||||
let current_exec_policy = exec_policy.current();
|
||||
let (network_proxy, session_network_proxy) = Self::start_managed_network_proxy(
|
||||
spec,
|
||||
|
||||
@@ -1158,6 +1158,7 @@ async fn new_turn_refreshes_managed_network_proxy_for_sandbox_change() -> anyhow
|
||||
let mut network_config = NetworkProxyConfig::default();
|
||||
network_config.set_allowed_domains(vec!["evil.com".to_string()]);
|
||||
let requirements = NetworkConstraints {
|
||||
enabled: Some(true),
|
||||
domains: Some(NetworkDomainPermissionsToml {
|
||||
entries: std::collections::BTreeMap::from([(
|
||||
"*.example.com".to_string(),
|
||||
@@ -1228,6 +1229,51 @@ async fn new_turn_refreshes_managed_network_proxy_for_sandbox_change() -> anyhow
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn refresh_clears_disabled_managed_network_proxy() -> anyhow::Result<()> {
|
||||
let permission_profile = PermissionProfile::workspace_write();
|
||||
let enabled_spec = crate::config::NetworkProxySpec::from_config_and_constraints(
|
||||
NetworkProxyConfig::default(),
|
||||
Some(NetworkConstraints {
|
||||
enabled: Some(true),
|
||||
..Default::default()
|
||||
}),
|
||||
&permission_profile,
|
||||
)?;
|
||||
let disabled_spec = crate::config::NetworkProxySpec::from_config_and_constraints(
|
||||
NetworkProxyConfig::default(),
|
||||
Some(NetworkConstraints {
|
||||
enabled: Some(false),
|
||||
..Default::default()
|
||||
}),
|
||||
&permission_profile,
|
||||
)?;
|
||||
let session = make_session_with_config(move |config| {
|
||||
config
|
||||
.permissions
|
||||
.set_permission_profile(permission_profile)
|
||||
.expect("test setup should allow permission profile");
|
||||
config.permissions.network = Some(enabled_spec);
|
||||
})
|
||||
.await?;
|
||||
assert!(session.services.network_proxy.load_full().is_some());
|
||||
|
||||
{
|
||||
let mut state = session.state.lock().await;
|
||||
let mut config = (*state.session_configuration.original_config_do_not_use).clone();
|
||||
config.permissions.network = Some(disabled_spec);
|
||||
state.session_configuration.original_config_do_not_use = Arc::new(config);
|
||||
}
|
||||
|
||||
session
|
||||
.refresh_managed_network_proxy_for_current_permission_profile()
|
||||
.await;
|
||||
|
||||
assert!(session.services.network_proxy.load_full().is_none());
|
||||
assert!(session.new_default_turn().await.network.is_none());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn danger_full_access_turns_do_not_expose_managed_network_proxy() -> anyhow::Result<()> {
|
||||
let network_spec = crate::config::NetworkProxySpec::from_config_and_constraints(
|
||||
@@ -1400,6 +1446,41 @@ async fn workspace_write_turns_continue_to_expose_managed_network_proxy() -> any
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn disabled_managed_network_does_not_start_or_expose_proxy() -> anyhow::Result<()> {
|
||||
let permission_profile = PermissionProfile::workspace_write();
|
||||
let network_spec = crate::config::NetworkProxySpec::from_config_and_constraints(
|
||||
NetworkProxyConfig::default(),
|
||||
Some(NetworkConstraints {
|
||||
enabled: Some(false),
|
||||
..Default::default()
|
||||
}),
|
||||
&permission_profile,
|
||||
)?;
|
||||
|
||||
let (session, rx) = make_session_with_config_and_rx(move |config| {
|
||||
config
|
||||
.permissions
|
||||
.set_permission_profile(permission_profile)
|
||||
.expect("test setup should allow permission profile");
|
||||
config.permissions.network = Some(network_spec);
|
||||
})
|
||||
.await?;
|
||||
|
||||
assert!(session.services.network_proxy.load_full().is_none());
|
||||
assert!(session.new_default_turn().await.network.is_none());
|
||||
|
||||
loop {
|
||||
let event = rx.recv().await.expect("channel open");
|
||||
if let EventMsg::SessionConfigured(event) = event.msg {
|
||||
assert!(event.network_proxy.is_none());
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn user_shell_commands_do_not_inherit_managed_network_proxy() -> anyhow::Result<()> {
|
||||
let permission_profile = PermissionProfile::workspace_write();
|
||||
|
||||
@@ -74,6 +74,13 @@ enum PushedExecScenario {
|
||||
ReplayGap,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum ManagedNetworkScenario {
|
||||
None,
|
||||
Enabled { policy_callbacks: bool },
|
||||
Disabled,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct PushedExecServerResult {
|
||||
process_read_requests: usize,
|
||||
@@ -532,28 +539,36 @@ async fn serve_exec_with_pushed_events(
|
||||
}
|
||||
}
|
||||
|
||||
#[test_case(PushedExecScenario::Complete, false, false, false ; "complete_event_stream")]
|
||||
#[test_case(PushedExecScenario::DirectDenied, false, false, false ; "direct_sandbox_denial")]
|
||||
#[test_case(PushedExecScenario::LegacyExit, false, false, false ; "legacy_exit_metadata")]
|
||||
#[test_case(PushedExecScenario::ReplayGap, false, false, false ; "truncated_event_replay")]
|
||||
#[test_case(PushedExecScenario::Complete, true, true, false ; "managed_network_uses_executor_proxy_launch")]
|
||||
#[test_case(PushedExecScenario::Complete, true, false, false ; "strict_managed_allowlist_omits_policy_callbacks")]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::Complete, true, true, true ; "foreign_windows_managed_network_preserves_approval_registration"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::Complete, false, false, true ; "foreign_windows_workspace_sandbox"))]
|
||||
#[test_case(PushedExecScenario::ElevatedPowerShell, false, false, true ; "windows_elevated_powershell_disables_profile")]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::SandboxedInterceptedPatch, false, false, true ; "foreign_windows_intercepted_patch_is_sandboxed"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::SandboxedDirectPatch, false, false, true ; "foreign_windows_direct_patch_is_sandboxed"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::SandboxedDirectPatchDenied, false, false, true ; "foreign_windows_direct_patch_denial_requests_approval"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::SandboxedDirectPatchRetry, false, false, true ; "foreign_windows_direct_patch_denial_approval_retries_unsandboxed"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::UnsandboxedInterceptedPatch, false, false, true ; "foreign_windows_unsandboxed_intercepted_patch_succeeds"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::FullDiskInterceptedPatch, false, false, true ; "foreign_windows_full_disk_intercepted_patch_succeeds"))]
|
||||
#[test_case(PushedExecScenario::Complete, ManagedNetworkScenario::None, false ; "complete_event_stream")]
|
||||
#[test_case(PushedExecScenario::DirectDenied, ManagedNetworkScenario::None, false ; "direct_sandbox_denial")]
|
||||
#[test_case(PushedExecScenario::LegacyExit, ManagedNetworkScenario::None, false ; "legacy_exit_metadata")]
|
||||
#[test_case(PushedExecScenario::ReplayGap, ManagedNetworkScenario::None, false ; "truncated_event_replay")]
|
||||
#[test_case(PushedExecScenario::Complete, ManagedNetworkScenario::Enabled { policy_callbacks: true }, false ; "managed_network_uses_executor_proxy_launch")]
|
||||
#[test_case(PushedExecScenario::Complete, ManagedNetworkScenario::Enabled { policy_callbacks: false }, false ; "strict_managed_allowlist_omits_policy_callbacks")]
|
||||
#[test_case(PushedExecScenario::Complete, ManagedNetworkScenario::Disabled, false ; "disabled_managed_network_omits_executor_proxy_launch")]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::Complete, ManagedNetworkScenario::Enabled { policy_callbacks: true }, true ; "foreign_windows_managed_network_preserves_approval_registration"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::Complete, ManagedNetworkScenario::None, true ; "foreign_windows_workspace_sandbox"))]
|
||||
#[test_case(PushedExecScenario::ElevatedPowerShell, ManagedNetworkScenario::None, true ; "windows_elevated_powershell_disables_profile")]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::SandboxedInterceptedPatch, ManagedNetworkScenario::None, true ; "foreign_windows_intercepted_patch_is_sandboxed"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::SandboxedDirectPatch, ManagedNetworkScenario::None, true ; "foreign_windows_direct_patch_is_sandboxed"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::SandboxedDirectPatchDenied, ManagedNetworkScenario::None, true ; "foreign_windows_direct_patch_denial_requests_approval"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::SandboxedDirectPatchRetry, ManagedNetworkScenario::None, true ; "foreign_windows_direct_patch_denial_approval_retries_unsandboxed"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::UnsandboxedInterceptedPatch, ManagedNetworkScenario::None, true ; "foreign_windows_unsandboxed_intercepted_patch_succeeds"))]
|
||||
#[cfg_attr(not(windows), test_case(PushedExecScenario::FullDiskInterceptedPatch, ManagedNetworkScenario::None, true ; "foreign_windows_full_disk_intercepted_patch_succeeds"))]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_command_consumes_pushed_remote_process_events(
|
||||
scenario: PushedExecScenario,
|
||||
managed_network: bool,
|
||||
policy_callbacks: bool,
|
||||
managed_network: ManagedNetworkScenario,
|
||||
foreign_cwd: bool,
|
||||
) -> Result<()> {
|
||||
let managed_network_configured = !matches!(managed_network, ManagedNetworkScenario::None);
|
||||
let managed_network_enabled = matches!(managed_network, ManagedNetworkScenario::Enabled { .. });
|
||||
let policy_callbacks = matches!(
|
||||
managed_network,
|
||||
ManagedNetworkScenario::Enabled {
|
||||
policy_callbacks: true
|
||||
}
|
||||
);
|
||||
let listener = TcpListener::bind("127.0.0.1:0").await?;
|
||||
let server = start_mock_server().await;
|
||||
let tool_call = match scenario {
|
||||
@@ -601,11 +616,14 @@ async fn exec_command_consumes_pushed_remote_process_events(
|
||||
let exec_server_url = format!("ws://{}", listener.local_addr()?);
|
||||
let exec_server = tokio::spawn(serve_exec_with_pushed_events(listener, scenario));
|
||||
let mut builder = test_codex().with_exec_server_url(exec_server_url);
|
||||
if managed_network {
|
||||
let cloud_config_bundle = if policy_callbacks {
|
||||
managed_network_requirements_loader()
|
||||
} else {
|
||||
CloudConfigBundleFixture::loader_with_enterprise_requirement(
|
||||
if managed_network_configured {
|
||||
let cloud_config_bundle = match managed_network {
|
||||
ManagedNetworkScenario::Enabled {
|
||||
policy_callbacks: true,
|
||||
} => managed_network_requirements_loader(),
|
||||
ManagedNetworkScenario::Enabled {
|
||||
policy_callbacks: false,
|
||||
} => CloudConfigBundleFixture::loader_with_enterprise_requirement(
|
||||
r#"
|
||||
[experimental_network]
|
||||
enabled = true
|
||||
@@ -615,7 +633,16 @@ managed_allowed_domains_only = true
|
||||
[experimental_network.domains]
|
||||
"allowed.example.com" = "allow"
|
||||
"#,
|
||||
)
|
||||
),
|
||||
ManagedNetworkScenario::Disabled => {
|
||||
CloudConfigBundleFixture::loader_with_enterprise_requirement(
|
||||
r#"
|
||||
[experimental_network]
|
||||
enabled = false
|
||||
"#,
|
||||
)
|
||||
}
|
||||
ManagedNetworkScenario::None => unreachable!("managed network is not configured"),
|
||||
};
|
||||
builder = builder
|
||||
.with_cloud_config_bundle(cloud_config_bundle)
|
||||
@@ -653,7 +680,7 @@ timeout = 900
|
||||
if matches!(scenario, PushedExecScenario::ElevatedPowerShell) {
|
||||
config.set_windows_elevated_sandbox_enabled(/*value*/ true);
|
||||
}
|
||||
if managed_network {
|
||||
if managed_network_configured {
|
||||
#[cfg(windows)]
|
||||
config.set_windows_sandbox_enabled(/*value*/ true);
|
||||
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
|
||||
@@ -664,7 +691,7 @@ timeout = 900
|
||||
.await
|
||||
.context("thread startup should connect to the fake exec-server")??;
|
||||
|
||||
let turn_permission_profile = if managed_network {
|
||||
let turn_permission_profile = if managed_network_configured {
|
||||
test.session_configured.permission_profile.clone()
|
||||
} else if matches!(scenario, PushedExecScenario::FullDiskInterceptedPatch) {
|
||||
PermissionProfile::from_runtime_permissions(
|
||||
@@ -707,7 +734,7 @@ timeout = 900
|
||||
)
|
||||
}),
|
||||
approval_policy: Some(
|
||||
if managed_network
|
||||
if managed_network_configured
|
||||
|| matches!(
|
||||
scenario,
|
||||
PushedExecScenario::SandboxedDirectPatchDenied
|
||||
@@ -735,7 +762,7 @@ timeout = 900
|
||||
.await?;
|
||||
let mut saw_exec_command_begin = false;
|
||||
let mut saw_patch_denial_approval = false;
|
||||
if !managed_network {
|
||||
if !managed_network_enabled {
|
||||
loop {
|
||||
let event = timeout(Duration::from_secs(5), test.codex.next_event())
|
||||
.await
|
||||
@@ -891,7 +918,7 @@ timeout = 900
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
let cleanup_timeout = if managed_network {
|
||||
let cleanup_timeout = if managed_network_enabled {
|
||||
Duration::from_secs(15)
|
||||
} else {
|
||||
Duration::from_secs(5)
|
||||
@@ -919,7 +946,7 @@ timeout = 900
|
||||
assert_eq!(params["sandbox"]["windowsSandboxLevel"], "restricted-token");
|
||||
}
|
||||
}
|
||||
if managed_network {
|
||||
if managed_network_enabled {
|
||||
let params = &exec_server_result.process_start["params"];
|
||||
assert_eq!(params["enforceManagedNetwork"], true);
|
||||
assert_eq!(params["managedNetwork"], Value::Null);
|
||||
@@ -941,6 +968,13 @@ timeout = 900
|
||||
.context("model should receive the remote exec output")?;
|
||||
return Ok(());
|
||||
}
|
||||
if matches!(managed_network, ManagedNetworkScenario::Disabled) {
|
||||
let params = &exec_server_result.process_start["params"];
|
||||
assert_eq!(params["enforceManagedNetwork"], false);
|
||||
assert_eq!(params["managedNetwork"], Value::Null);
|
||||
assert_eq!(params["networkProxy"], Value::Null);
|
||||
assert_eq!(params["env"]["HTTP_PROXY"], Value::Null);
|
||||
}
|
||||
let request = response_mock
|
||||
.last_request()
|
||||
.context("model should receive the exec_command output")?;
|
||||
|
||||
Reference in New Issue
Block a user