Remove the network proxy config wrapper (#31767)

## Why

`NetworkProxyConfig` only wrapped `NetworkProxySettings` in a single
`network` field. That extra level made runtime callers repeat `.network`
everywhere without representing a real boundary.

## What changed

- move the managed-network fields directly onto `NetworkProxyConfig`
- collapse the matching partial-config wrapper
- update runtime callers and tests to use the direct fields
- keep the user-facing permissions/profile TOML layout unchanged

The internal serialized shape now matches the runtime type itself. This
does not change managed-network behavior or the `config.toml` shape.
This commit is contained in:
jif authored and GitHub committed 2026-07-09 17:33:37 +01:00
1 parent f46b53bd85
commit cf6d8ad921
25 files changed
+428 -632

No files matched your search

+9 -9
View File
@@ -1182,16 +1182,16 @@ fn permissions_profile_network_to_proxy_config_preserves_mitm_hooks() {
let config = network.to_network_proxy_config();
assert_eq!(config.network.mode, NetworkMode::Full);
assert!(config.network.mitm);
assert_eq!(config.network.mitm_hooks.len(), 1);
assert_eq!(config.network.mitm_hooks[0].host, "api.github.com");
assert_eq!(config.mode, NetworkMode::Full);
assert!(config.mitm);
assert_eq!(config.mitm_hooks.len(), 1);
assert_eq!(config.mitm_hooks[0].host, "api.github.com");
assert_eq!(
config.network.mitm_hooks[0].matcher.methods,
config.mitm_hooks[0].matcher.methods,
vec!["POST".to_string()]
);
assert_eq!(
config.network.mitm_hooks[0].actions.strip_request_headers,
config.mitm_hooks[0].actions.strip_request_headers,
vec!["authorization".to_string()]
);
}
@@ -1228,13 +1228,13 @@ action = ["noop"]
let config = network.to_network_proxy_config();
assert_eq!(config.network.mitm_hooks.len(), 2);
assert_eq!(config.mitm_hooks.len(), 2);
assert_eq!(
config.network.mitm_hooks[0].matcher.path_prefixes,
config.mitm_hooks[0].matcher.path_prefixes,
vec!["/repos/openai/".to_string()]
);
assert_eq!(
config.network.mitm_hooks[1].matcher.path_prefixes,
config.mitm_hooks[1].matcher.path_prefixes,
vec!["/repos/".to_string()]
);
}
+2 -2
View File
@@ -3368,7 +3368,7 @@ impl Config {
network_proxy,
);
}
configured_network_proxy_config.network.enabled = true;
configured_network_proxy_config.enabled = true;
}
let approval_policy_was_explicit =
approval_policy_override.is_some() || cfg.approval_policy.is_some();
@@ -4128,7 +4128,7 @@ impl Config {
network_proxy,
);
}
configured_network_proxy_config.network.enabled = true;
configured_network_proxy_config.enabled = true;
}
configured_network_proxy_config
} else {
+16 -21
View File
@@ -74,15 +74,15 @@ impl ConfigReloader for StaticNetworkProxyReloader {
impl NetworkProxySpec {
pub(crate) fn enabled(&self) -> bool {
self.config.network.enabled
self.config.enabled
}
pub fn proxy_host_and_port(&self) -> String {
host_and_port_from_network_addr(&self.config.network.proxy_url, /*default_port*/ 3128)
host_and_port_from_network_addr(&self.config.proxy_url, /*default_port*/ 3128)
}
pub fn socks_enabled(&self) -> bool {
self.config.network.enable_socks5
self.config.enable_socks5
}
pub(crate) fn from_config_and_constraints(
@@ -221,31 +221,30 @@ impl NetworkProxySpec {
let denylist_expansion_enabled = Self::denylist_expansion_enabled(permission_profile);
if let Some(enabled) = requirements.enabled {
config.network.enabled = enabled;
config.enabled = enabled;
constraints.enabled = Some(enabled);
}
if let Some(http_port) = requirements.http_port {
config.network.proxy_url = format!("http://127.0.0.1:{http_port}");
config.proxy_url = format!("http://127.0.0.1:{http_port}");
}
if let Some(socks_port) = requirements.socks_port {
config.network.socks_url = format!("http://127.0.0.1:{socks_port}");
config.socks_url = format!("http://127.0.0.1:{socks_port}");
}
if let Some(allow_upstream_proxy) = requirements.allow_upstream_proxy {
config.network.allow_upstream_proxy = allow_upstream_proxy;
config.allow_upstream_proxy = allow_upstream_proxy;
constraints.allow_upstream_proxy = Some(allow_upstream_proxy);
}
if let Some(dangerously_allow_non_loopback_proxy) =
requirements.dangerously_allow_non_loopback_proxy
{
config.network.dangerously_allow_non_loopback_proxy =
dangerously_allow_non_loopback_proxy;
config.dangerously_allow_non_loopback_proxy = dangerously_allow_non_loopback_proxy;
constraints.dangerously_allow_non_loopback_proxy =
Some(dangerously_allow_non_loopback_proxy);
}
if let Some(dangerously_allow_all_unix_sockets) =
requirements.dangerously_allow_all_unix_sockets
{
config.network.dangerously_allow_all_unix_sockets = dangerously_allow_all_unix_sockets;
config.dangerously_allow_all_unix_sockets = dangerously_allow_all_unix_sockets;
constraints.dangerously_allow_all_unix_sockets =
Some(dangerously_allow_all_unix_sockets);
}
@@ -270,14 +269,12 @@ impl NetworkProxySpec {
let effective_allowed_domains = if allowlist_expansion_enabled {
Self::merge_domain_lists(
managed_allowed_domains.clone(),
config.network.allowed_domains().as_deref().unwrap_or(&[]),
config.allowed_domains().as_deref().unwrap_or(&[]),
)
} else {
managed_allowed_domains.clone()
};
config
.network
.set_allowed_domains(effective_allowed_domains);
config.set_allowed_domains(effective_allowed_domains);
constraints.allowed_domains = Some(managed_allowed_domains);
constraints.allowlist_expansion_enabled = Some(allowlist_expansion_enabled);
}
@@ -289,12 +286,12 @@ impl NetworkProxySpec {
let effective_denied_domains = if denylist_expansion_enabled {
Self::merge_domain_lists(
managed_denied_domains.clone(),
config.network.denied_domains().as_deref().unwrap_or(&[]),
config.denied_domains().as_deref().unwrap_or(&[]),
)
} else {
managed_denied_domains.clone()
};
config.network.set_denied_domains(effective_denied_domains);
config.set_denied_domains(effective_denied_domains);
constraints.denied_domains = Some(managed_denied_domains);
constraints.denylist_expansion_enabled = Some(denylist_expansion_enabled);
}
@@ -304,13 +301,11 @@ impl NetworkProxySpec {
.as_ref()
.map(codex_config::NetworkUnixSocketPermissionsToml::allow_unix_sockets)
.unwrap_or_default();
config
.network
.set_allow_unix_sockets(allow_unix_sockets.clone());
config.set_allow_unix_sockets(allow_unix_sockets.clone());
constraints.allow_unix_sockets = Some(allow_unix_sockets);
}
if let Some(allow_local_binding) = requirements.allow_local_binding {
config.network.allow_local_binding = allow_local_binding;
config.allow_local_binding = allow_local_binding;
constraints.allow_local_binding = Some(allow_local_binding);
}
@@ -359,7 +354,7 @@ fn upsert_network_domains(config: &mut NetworkProxyConfig, hosts: Vec<String>, a
let mut incoming = HashSet::new();
for host in hosts {
if incoming.insert(host.clone()) {
config.network.upsert_domain_permission(
config.upsert_domain_permission(
host,
if allow {
codex_network_proxy::NetworkDomainPermission::Allow
@@ -43,9 +43,7 @@ fn build_state_with_audit_metadata_threads_metadata_to_state() {
#[test]
fn requirements_allowed_domains_are_a_baseline_for_user_allowlist() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_allowed_domains(vec!["api.example.com".to_string()]);
config.set_allowed_domains(vec!["api.example.com".to_string()]);
let requirements = NetworkConstraints {
domains: Some(domain_permissions([(
"*.example.com",
@@ -62,7 +60,7 @@ fn requirements_allowed_domains_are_a_baseline_for_user_allowlist() {
.expect("config should stay within the managed allowlist");
assert_eq!(
spec.config.network.allowed_domains(),
spec.config.allowed_domains(),
Some(vec![
"*.example.com".to_string(),
"api.example.com".to_string()
@@ -78,9 +76,7 @@ fn requirements_allowed_domains_are_a_baseline_for_user_allowlist() {
#[test]
fn requirements_allowed_domains_do_not_override_user_denies_for_same_pattern() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_denied_domains(vec!["api.example.com".to_string()]);
config.set_denied_domains(vec!["api.example.com".to_string()]);
let requirements = NetworkConstraints {
domains: Some(domain_permissions([(
"api.example.com",
@@ -96,9 +92,9 @@ fn requirements_allowed_domains_do_not_override_user_denies_for_same_pattern() {
)
.expect("managed allowlist should not erase a user deny");
assert_eq!(spec.config.network.allowed_domains(), None);
assert_eq!(spec.config.allowed_domains(), None);
assert_eq!(
spec.config.network.denied_domains(),
spec.config.denied_domains(),
Some(vec!["api.example.com".to_string()])
);
assert_eq!(
@@ -110,9 +106,7 @@ fn requirements_allowed_domains_do_not_override_user_denies_for_same_pattern() {
#[test]
fn requirements_allowlist_expansion_keeps_user_entries_mutable() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_allowed_domains(vec!["api.example.com".to_string()]);
config.set_allowed_domains(vec!["api.example.com".to_string()]);
let requirements = NetworkConstraints {
domains: Some(domain_permissions([(
"*.example.com",
@@ -129,18 +123,18 @@ fn requirements_allowlist_expansion_keeps_user_entries_mutable() {
.expect("managed baseline should still allow user edits");
let mut candidate = spec.config.clone();
candidate.network.upsert_domain_permission(
candidate.upsert_domain_permission(
"api.example.com".to_string(),
NetworkDomainPermission::Deny,
normalize_host,
);
assert_eq!(
candidate.network.allowed_domains(),
candidate.allowed_domains(),
Some(vec!["*.example.com".to_string()])
);
assert_eq!(
candidate.network.denied_domains(),
candidate.denied_domains(),
Some(vec!["api.example.com".to_string()])
);
validate_policy_against_constraints(&candidate, &spec.constraints)
@@ -150,9 +144,7 @@ fn requirements_allowlist_expansion_keeps_user_entries_mutable() {
#[test]
fn managed_unrestricted_profile_allows_domain_expansion() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_allowed_domains(vec!["api.example.com".to_string()]);
config.set_allowed_domains(vec!["api.example.com".to_string()]);
let requirements = NetworkConstraints {
domains: Some(domain_permissions([(
"*.example.com",
@@ -173,7 +165,7 @@ fn managed_unrestricted_profile_allows_domain_expansion() {
.expect("managed unrestricted filesystem should still use managed network constraints");
assert_eq!(
spec.config.network.allowed_domains(),
spec.config.allowed_domains(),
Some(vec![
"*.example.com".to_string(),
"api.example.com".to_string()
@@ -185,12 +177,8 @@ fn managed_unrestricted_profile_allows_domain_expansion() {
#[test]
fn danger_full_access_keeps_managed_allowlist_and_denylist_fixed() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_allowed_domains(vec!["evil.com".to_string()]);
config
.network
.set_denied_domains(vec!["more-blocked.example.com".to_string()]);
config.set_allowed_domains(vec!["evil.com".to_string()]);
config.set_denied_domains(vec!["more-blocked.example.com".to_string()]);
let requirements = NetworkConstraints {
domains: Some(domain_permissions([
("*.example.com", NetworkDomainPermissionToml::Allow),
@@ -207,11 +195,11 @@ fn danger_full_access_keeps_managed_allowlist_and_denylist_fixed() {
.expect("yolo mode should pin the effective policy to the managed baseline");
assert_eq!(
spec.config.network.allowed_domains(),
spec.config.allowed_domains(),
Some(vec!["*.example.com".to_string()])
);
assert_eq!(
spec.config.network.denied_domains(),
spec.config.denied_domains(),
Some(vec!["blocked.example.com".to_string()])
);
assert_eq!(spec.constraints.allowlist_expansion_enabled, Some(false));
@@ -221,9 +209,7 @@ fn danger_full_access_keeps_managed_allowlist_and_denylist_fixed() {
#[test]
fn managed_allowed_domains_only_disables_default_mode_allowlist_expansion() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_allowed_domains(vec!["api.example.com".to_string()]);
config.set_allowed_domains(vec!["api.example.com".to_string()]);
let requirements = NetworkConstraints {
domains: Some(domain_permissions([(
"*.example.com",
@@ -241,7 +227,7 @@ fn managed_allowed_domains_only_disables_default_mode_allowlist_expansion() {
.expect("managed baseline should still load");
assert_eq!(
spec.config.network.allowed_domains(),
spec.config.allowed_domains(),
Some(vec!["*.example.com".to_string()])
);
assert_eq!(spec.constraints.allowlist_expansion_enabled, Some(false));
@@ -250,9 +236,7 @@ fn managed_allowed_domains_only_disables_default_mode_allowlist_expansion() {
#[test]
fn managed_allowed_domains_only_ignores_user_allowlist_and_hard_denies_misses() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_allowed_domains(vec!["api.example.com".to_string()]);
config.set_allowed_domains(vec!["api.example.com".to_string()]);
let requirements = NetworkConstraints {
domains: Some(domain_permissions([(
"managed.example.com",
@@ -270,7 +254,7 @@ fn managed_allowed_domains_only_ignores_user_allowlist_and_hard_denies_misses()
.expect("managed-only allowlist should still load");
assert_eq!(
spec.config.network.allowed_domains(),
spec.config.allowed_domains(),
Some(vec!["managed.example.com".to_string()])
);
assert_eq!(
@@ -284,9 +268,7 @@ fn managed_allowed_domains_only_ignores_user_allowlist_and_hard_denies_misses()
#[test]
fn managed_allowed_domains_only_without_managed_allowlist_blocks_all_user_domains() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_allowed_domains(vec!["api.example.com".to_string()]);
config.set_allowed_domains(vec!["api.example.com".to_string()]);
let requirements = NetworkConstraints {
managed_allowed_domains_only: Some(true),
..Default::default()
@@ -299,7 +281,7 @@ fn managed_allowed_domains_only_without_managed_allowlist_blocks_all_user_domain
)
.expect("managed-only mode should treat missing managed allowlist as empty");
assert_eq!(spec.config.network.allowed_domains(), None);
assert_eq!(spec.config.allowed_domains(), None);
assert_eq!(spec.constraints.allowed_domains, Some(Vec::new()));
assert_eq!(spec.constraints.allowlist_expansion_enabled, Some(false));
assert!(spec.hard_deny_allowlist_misses);
@@ -308,9 +290,7 @@ fn managed_allowed_domains_only_without_managed_allowlist_blocks_all_user_domain
#[test]
fn managed_allowed_domains_only_blocks_all_user_domains_in_full_access_without_managed_list() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_allowed_domains(vec!["api.example.com".to_string()]);
config.set_allowed_domains(vec!["api.example.com".to_string()]);
let requirements = NetworkConstraints {
managed_allowed_domains_only: Some(true),
..Default::default()
@@ -323,7 +303,7 @@ fn managed_allowed_domains_only_blocks_all_user_domains_in_full_access_without_m
)
.expect("managed-only mode should treat missing managed allowlist as empty");
assert_eq!(spec.config.network.allowed_domains(), None);
assert_eq!(spec.config.allowed_domains(), None);
assert_eq!(spec.constraints.allowed_domains, Some(Vec::new()));
assert_eq!(spec.constraints.allowlist_expansion_enabled, Some(false));
assert!(spec.hard_deny_allowlist_misses);
@@ -332,9 +312,7 @@ fn managed_allowed_domains_only_blocks_all_user_domains_in_full_access_without_m
#[test]
fn deny_only_requirements_do_not_create_allow_constraints_in_full_access() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_allowed_domains(vec!["api.example.com".to_string()]);
config.set_allowed_domains(vec!["api.example.com".to_string()]);
let requirements = NetworkConstraints {
domains: Some(domain_permissions([(
"managed-blocked.example.com",
@@ -351,13 +329,13 @@ fn deny_only_requirements_do_not_create_allow_constraints_in_full_access() {
.expect("deny-only requirements should not constrain the allowlist");
assert_eq!(
spec.config.network.allowed_domains(),
spec.config.allowed_domains(),
Some(vec!["api.example.com".to_string()])
);
assert_eq!(spec.constraints.allowed_domains, None);
assert_eq!(spec.constraints.allowlist_expansion_enabled, None);
assert_eq!(
spec.config.network.denied_domains(),
spec.config.denied_domains(),
Some(vec!["managed-blocked.example.com".to_string()])
);
}
@@ -365,9 +343,7 @@ fn deny_only_requirements_do_not_create_allow_constraints_in_full_access() {
#[test]
fn allow_only_requirements_do_not_create_deny_constraints_in_full_access() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_denied_domains(vec!["blocked.example.com".to_string()]);
config.set_denied_domains(vec!["blocked.example.com".to_string()]);
let requirements = NetworkConstraints {
domains: Some(domain_permissions([(
"managed.example.com",
@@ -384,11 +360,11 @@ fn allow_only_requirements_do_not_create_deny_constraints_in_full_access() {
.expect("allow-only requirements should not constrain the denylist");
assert_eq!(
spec.config.network.allowed_domains(),
spec.config.allowed_domains(),
Some(vec!["managed.example.com".to_string()])
);
assert_eq!(
spec.config.network.denied_domains(),
spec.config.denied_domains(),
Some(vec!["blocked.example.com".to_string()])
);
assert_eq!(spec.constraints.denied_domains, None);
@@ -398,9 +374,7 @@ fn allow_only_requirements_do_not_create_deny_constraints_in_full_access() {
#[test]
fn requirements_denied_domains_are_a_baseline_for_default_mode() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_denied_domains(vec!["blocked.example.com".to_string()]);
config.set_denied_domains(vec!["blocked.example.com".to_string()]);
let requirements = NetworkConstraints {
domains: Some(domain_permissions([(
"managed-blocked.example.com",
@@ -417,7 +391,7 @@ fn requirements_denied_domains_are_a_baseline_for_default_mode() {
.expect("default mode should merge managed and user deny entries");
assert_eq!(
spec.config.network.denied_domains(),
spec.config.denied_domains(),
Some(vec![
"managed-blocked.example.com".to_string(),
"blocked.example.com".to_string()
@@ -433,9 +407,7 @@ fn requirements_denied_domains_are_a_baseline_for_default_mode() {
#[test]
fn requirements_denylist_expansion_keeps_user_entries_mutable() {
let mut config = NetworkProxyConfig::default();
config
.network
.set_denied_domains(vec!["blocked.example.com".to_string()]);
config.set_denied_domains(vec!["blocked.example.com".to_string()]);
let requirements = NetworkConstraints {
domains: Some(domain_permissions([(
"managed-blocked.example.com",
@@ -452,18 +424,18 @@ fn requirements_denylist_expansion_keeps_user_entries_mutable() {
.expect("managed baseline should still allow user edits");
let mut candidate = spec.config.clone();
candidate.network.upsert_domain_permission(
candidate.upsert_domain_permission(
"blocked.example.com".to_string(),
NetworkDomainPermission::Allow,
normalize_host,
);
assert_eq!(
candidate.network.allowed_domains(),
candidate.allowed_domains(),
Some(vec!["blocked.example.com".to_string()])
);
assert_eq!(
candidate.network.denied_domains(),
candidate.denied_domains(),
Some(vec!["managed-blocked.example.com".to_string()])
);
validate_policy_against_constraints(&candidate, &spec.constraints)
+1 -1
View File
@@ -127,7 +127,7 @@ pub(crate) fn network_proxy_config_from_profile_network(
// Profile `network.enabled` controls sandbox network access. Profiles may
// provide proxy settings for the feature gate to consume when that network
// access is enabled, but they do not start the managed proxy on their own.
config.network.enabled = false;
config.enabled = false;
config
}
@@ -220,7 +220,7 @@ fn network_toml_overlays_unix_socket_permissions_by_path() {
.apply_to_network_proxy_config(&mut config);
assert_eq!(
config.network.unix_sockets,
config.unix_sockets,
Some(codex_network_proxy::NetworkUnixSocketPermissions {
entries: BTreeMap::from([
(
@@ -399,7 +399,7 @@ fn profile_network_proxy_config_keeps_proxy_disabled_for_bare_network_access() {
..Default::default()
}));
assert!(!config.network.enabled);
assert!(!config.enabled);
}
#[test]
@@ -417,11 +417,11 @@ fn profile_network_proxy_config_keeps_proxy_disabled_for_proxy_policy() {
..Default::default()
}));
assert!(!config.network.enabled);
assert_eq!(config.network.proxy_url, "http://127.0.0.1:43128");
assert!(!config.network.enable_socks5);
assert!(!config.enabled);
assert_eq!(config.proxy_url, "http://127.0.0.1:43128");
assert!(!config.enable_socks5);
assert_eq!(
config.network.domains,
config.domains,
Some(codex_network_proxy::NetworkDomainPermissions {
entries: vec![codex_network_proxy::NetworkDomainPermissionEntry {
pattern: "openai.com".to_string(),