mirror of
https://github.com/openai/codex.git
synced 2026-11-03 19:53:25 +00:00
fix(core): refresh managed proxy across profiles
This commit is contained in:
@@ -997,6 +997,15 @@ impl Session {
|
||||
!matches!(permission_profile, PermissionProfile::Disabled)
|
||||
}
|
||||
|
||||
fn managed_network_proxy_refresh_required(
|
||||
previous: &SessionConfiguration,
|
||||
next: &SessionConfiguration,
|
||||
) -> bool {
|
||||
previous.permission_profile() != next.permission_profile()
|
||||
|| previous.original_config_do_not_use.permissions.network
|
||||
!= next.original_config_do_not_use.permissions.network
|
||||
}
|
||||
|
||||
/// Builds the `x-codex-beta-features` header value for this session.
|
||||
///
|
||||
/// `ModelClient` is session-scoped and intentionally does not depend on the full `Config`, so
|
||||
@@ -1080,7 +1089,7 @@ impl Session {
|
||||
.as_ref()
|
||||
.cloned()
|
||||
else {
|
||||
self.services.network_proxy.store(None);
|
||||
self.services.network_proxy.store(/*val*/ None);
|
||||
return;
|
||||
};
|
||||
|
||||
@@ -1090,7 +1099,7 @@ impl Session {
|
||||
Ok(spec) => spec,
|
||||
Err(err) => {
|
||||
warn!("failed to rebuild managed network proxy policy for sandbox change: {err}");
|
||||
self.services.network_proxy.store(None);
|
||||
self.services.network_proxy.store(/*val*/ None);
|
||||
return;
|
||||
}
|
||||
};
|
||||
@@ -1106,17 +1115,22 @@ impl Session {
|
||||
};
|
||||
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}");
|
||||
self.services.network_proxy.store(None);
|
||||
warn!(
|
||||
"failed to refresh managed network proxy for sandbox change; replacing it: {err}"
|
||||
);
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
match Self::start_managed_network_proxy(
|
||||
&spec,
|
||||
current_exec_policy.as_ref(),
|
||||
&session_configuration.permission_profile(),
|
||||
/*network_policy_decider*/ None,
|
||||
self.services
|
||||
.network_policy_decider
|
||||
.as_ref()
|
||||
.map(Arc::clone),
|
||||
self.services
|
||||
.managed_network_requirements_configured
|
||||
.then(|| {
|
||||
@@ -1134,6 +1148,7 @@ impl Session {
|
||||
}
|
||||
Err(err) => {
|
||||
warn!("failed to start managed network proxy for sandbox change: {err}");
|
||||
self.services.network_proxy.store(/*val*/ None);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1530,7 +1545,7 @@ impl Session {
|
||||
updates: SessionSettingsUpdate,
|
||||
) -> ConstraintResult<()> {
|
||||
let notify_config_contributors = !self.services.extensions.config_contributors().is_empty();
|
||||
let (previous_config, new_config, permission_profile_changed) = {
|
||||
let (previous_config, new_config, refresh_managed_network_proxy) = {
|
||||
let mut state = self.state.lock().await;
|
||||
let updated = match state.session_configuration.apply(&updates) {
|
||||
Ok(updated) => updated,
|
||||
@@ -1544,20 +1559,20 @@ impl Session {
|
||||
.then(|| Self::build_effective_session_config(&state.session_configuration));
|
||||
let new_config =
|
||||
notify_config_contributors.then(|| Self::build_effective_session_config(&updated));
|
||||
let previous_permission_profile = state.session_configuration.permission_profile();
|
||||
let updated_permission_profile = updated.permission_profile();
|
||||
let permission_profile_changed =
|
||||
previous_permission_profile != updated_permission_profile;
|
||||
let refresh_managed_network_proxy = Self::managed_network_proxy_refresh_required(
|
||||
&state.session_configuration,
|
||||
&updated,
|
||||
);
|
||||
if updates.environments.is_some() {
|
||||
self.services
|
||||
.turn_environments
|
||||
.update_selections(updated.environment_selections());
|
||||
}
|
||||
state.session_configuration = updated;
|
||||
(previous_config, new_config, permission_profile_changed)
|
||||
(previous_config, new_config, refresh_managed_network_proxy)
|
||||
};
|
||||
self.emit_config_changed_contributors(previous_config.as_ref(), new_config.as_ref());
|
||||
if permission_profile_changed {
|
||||
if refresh_managed_network_proxy {
|
||||
self.refresh_managed_network_proxy_for_current_permission_profile()
|
||||
.await;
|
||||
}
|
||||
|
||||
@@ -1086,6 +1086,7 @@ impl Session {
|
||||
),
|
||||
agent_control,
|
||||
network_proxy: arc_swap::ArcSwapOption::from(network_proxy.map(Arc::new)),
|
||||
network_policy_decider,
|
||||
network_proxy_audit_metadata,
|
||||
managed_network_requirements_configured,
|
||||
network_approval: Arc::clone(&network_approval),
|
||||
|
||||
@@ -4829,7 +4829,12 @@ async fn active_profile_update_rebuilds_network_proxy_config() -> std::io::Resul
|
||||
FilesystemPermissionToml::Access(FileSystemAccessMode::Read),
|
||||
)]),
|
||||
}),
|
||||
network: None,
|
||||
network: Some(NetworkToml {
|
||||
enabled: Some(true),
|
||||
proxy_url: Some("http://127.0.0.1:0".to_string()),
|
||||
enable_socks5: Some(true),
|
||||
..Default::default()
|
||||
}),
|
||||
},
|
||||
),
|
||||
(
|
||||
@@ -4847,7 +4852,7 @@ async fn active_profile_update_rebuilds_network_proxy_config() -> std::io::Resul
|
||||
}),
|
||||
network: Some(NetworkToml {
|
||||
enabled: Some(true),
|
||||
proxy_url: Some("http://127.0.0.1:43128".to_string()),
|
||||
proxy_url: Some("http://127.0.0.1:0".to_string()),
|
||||
enable_socks5: Some(false),
|
||||
..Default::default()
|
||||
}),
|
||||
@@ -4875,14 +4880,12 @@ async fn active_profile_update_rebuilds_network_proxy_config() -> std::io::Resul
|
||||
.build()
|
||||
.await?,
|
||||
);
|
||||
assert_ne!(
|
||||
assert!(
|
||||
locked_config
|
||||
.permissions
|
||||
.network
|
||||
.as_ref()
|
||||
.map(crate::config::NetworkProxySpec::proxy_host_and_port)
|
||||
.as_deref(),
|
||||
Some("127.0.0.1:43128")
|
||||
.is_some_and(crate::config::NetworkProxySpec::socks_enabled)
|
||||
);
|
||||
let selected_config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
@@ -4893,6 +4896,11 @@ async fn active_profile_update_rebuilds_network_proxy_config() -> std::io::Resul
|
||||
})
|
||||
.build()
|
||||
.await?;
|
||||
assert_eq!(
|
||||
locked_config.permissions.permission_profile(),
|
||||
selected_config.permissions.permission_profile(),
|
||||
"the profile switch should exercise proxy-spec-only refresh detection"
|
||||
);
|
||||
|
||||
let mut session_configuration = make_session_configuration_for_tests().await;
|
||||
session_configuration.permission_profile_state =
|
||||
@@ -4913,8 +4921,50 @@ async fn active_profile_update_rebuilds_network_proxy_config() -> std::io::Resul
|
||||
.network
|
||||
.as_ref()
|
||||
.expect("selected profile proxy should become the session proxy config");
|
||||
assert_eq!(network.proxy_host_and_port(), "127.0.0.1:43128");
|
||||
assert_eq!(network.proxy_host_and_port(), "127.0.0.1:0");
|
||||
assert!(!network.socks_enabled());
|
||||
|
||||
let session = make_session_with_config({
|
||||
let locked_config = Arc::clone(&locked_config);
|
||||
move |config| *config = (*locked_config).clone()
|
||||
})
|
||||
.await
|
||||
.map_err(std::io::Error::other)?;
|
||||
let initial_http_addr = session
|
||||
.services
|
||||
.network_proxy
|
||||
.load_full()
|
||||
.expect("initial managed network proxy should be present")
|
||||
.proxy()
|
||||
.http_addr();
|
||||
|
||||
let turn_context = session
|
||||
.new_turn_with_sub_id(
|
||||
"proxy-spec-only-profile-change".to_string(),
|
||||
SessionSettingsUpdate {
|
||||
permission_profile: Some(selected_config.permissions.permission_profile().clone()),
|
||||
active_permission_profile: selected_config.permissions.active_permission_profile(),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
.map_err(std::io::Error::other)?;
|
||||
assert!(turn_context.network.is_some());
|
||||
|
||||
let replacement_proxy = session
|
||||
.services
|
||||
.network_proxy
|
||||
.load_full()
|
||||
.expect("replacement managed network proxy should be present");
|
||||
let replacement_http_addr = replacement_proxy.proxy().http_addr();
|
||||
assert_ne!(replacement_http_addr, initial_http_addr);
|
||||
let replacement_config = replacement_proxy
|
||||
.proxy()
|
||||
.current_cfg()
|
||||
.await
|
||||
.map_err(std::io::Error::other)?;
|
||||
assert!(!replacement_config.network.enable_socks5);
|
||||
tokio::net::TcpStream::connect(replacement_http_addr).await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -5548,6 +5598,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
supports_openai_form_elicitation: std::sync::atomic::AtomicBool::new(false),
|
||||
agent_control,
|
||||
network_proxy: arc_swap::ArcSwapOption::from(None),
|
||||
network_policy_decider: None,
|
||||
network_proxy_audit_metadata: crate::config::NetworkProxyAuditMetadata::default(),
|
||||
managed_network_requirements_configured: false,
|
||||
network_approval: Arc::clone(&network_approval),
|
||||
@@ -7674,6 +7725,7 @@ where
|
||||
supports_openai_form_elicitation: std::sync::atomic::AtomicBool::new(false),
|
||||
agent_control,
|
||||
network_proxy: arc_swap::ArcSwapOption::from(None),
|
||||
network_policy_decider: None,
|
||||
network_proxy_audit_metadata: crate::config::NetworkProxyAuditMetadata::default(),
|
||||
managed_network_requirements_configured: false,
|
||||
network_approval: Arc::clone(&network_approval),
|
||||
|
||||
@@ -580,11 +580,11 @@ impl Session {
|
||||
let mut state = self.state.lock().await;
|
||||
match state.session_configuration.clone().apply(&updates) {
|
||||
Ok(next) => {
|
||||
let previous_permission_profile =
|
||||
state.session_configuration.permission_profile();
|
||||
let next_permission_profile = next.permission_profile();
|
||||
let permission_profile_changed =
|
||||
previous_permission_profile != next_permission_profile;
|
||||
let refresh_managed_network_proxy =
|
||||
Self::managed_network_proxy_refresh_required(
|
||||
&state.session_configuration,
|
||||
&next,
|
||||
);
|
||||
let previous_config = notify_config_contributors.then(|| {
|
||||
Self::build_effective_session_config(&state.session_configuration)
|
||||
});
|
||||
@@ -598,7 +598,7 @@ impl Session {
|
||||
state.session_configuration = next.clone();
|
||||
Ok((
|
||||
next,
|
||||
permission_profile_changed,
|
||||
refresh_managed_network_proxy,
|
||||
previous_config,
|
||||
new_config,
|
||||
))
|
||||
@@ -607,7 +607,7 @@ impl Session {
|
||||
}
|
||||
};
|
||||
|
||||
let (session_configuration, permission_profile_changed, previous_config, new_config) =
|
||||
let (session_configuration, refresh_managed_network_proxy, previous_config, new_config) =
|
||||
match update_result {
|
||||
Ok(update) => update,
|
||||
Err(err) => {
|
||||
@@ -625,7 +625,7 @@ impl Session {
|
||||
};
|
||||
self.emit_config_changed_contributors(previous_config.as_ref(), new_config.as_ref());
|
||||
|
||||
if permission_profile_changed {
|
||||
if refresh_managed_network_proxy {
|
||||
self.refresh_managed_network_proxy_for_current_permission_profile()
|
||||
.await;
|
||||
}
|
||||
|
||||
@@ -86,6 +86,7 @@ pub(crate) struct SessionServices {
|
||||
pub(crate) mcp_thread_init: ExtensionDataInit,
|
||||
pub(crate) agent_control: AgentControl,
|
||||
pub(crate) network_proxy: ArcSwapOption<StartedNetworkProxy>,
|
||||
pub(crate) network_policy_decider: Option<Arc<dyn codex_network_proxy::NetworkPolicyDecider>>,
|
||||
pub(crate) network_proxy_audit_metadata: NetworkProxyAuditMetadata,
|
||||
pub(crate) managed_network_requirements_configured: bool,
|
||||
pub(crate) network_approval: Arc<NetworkApprovalService>,
|
||||
|
||||
Reference in New Issue
Block a user