mirror of
https://github.com/openai/codex.git
synced 2026-04-28 16:45:54 +00:00
chore: refactor network permissions to use explicit domain and unix socket rule maps (#15120)
## Summary This PR replaces the legacy network allow/deny list model with explicit rule maps for domains and unix sockets across managed requirements, permissions profiles, the network proxy config, and the app server protocol. Concretely, it: - introduces typed domain (`allow` / `deny`) and unix socket permission (`allow` / `none`) entries instead of separate `allowed_domains`, `denied_domains`, and `allow_unix_sockets` lists - updates config loading, managed requirements merging, and exec-policy overlays to read and upsert rule entries consistently - exposes the new shape through protocol/schema outputs, debug surfaces, and app-server config APIs - rejects the legacy list-based keys and updates docs/tests to reflect the new config format ## Why The previous representation split related network policy across multiple parallel lists, which made merging and overriding rules harder to reason about. Moving to explicit keyed permission maps gives us a single source of truth per host/socket entry, makes allow/deny precedence clearer, and gives protocol consumers access to the full rule state instead of derived projections only. ## Backward Compatibility ### Backward compatible - Managed requirements still accept the legacy `experimental_network.allowed_domains`, `experimental_network.denied_domains`, and `experimental_network.allow_unix_sockets` fields. They are normalized into the new canonical `domains` and `unix_sockets` maps internally. - App-server v2 still deserializes legacy `allowedDomains`, `deniedDomains`, and `allowUnixSockets` payloads, so older clients can continue reading managed network requirements. - App-server v2 responses still populate `allowedDomains`, `deniedDomains`, and `allowUnixSockets` as legacy compatibility views derived from the canonical maps. - `managed_allowed_domains_only` keeps the same behavior after normalization. Legacy managed allowlists still participate in the same enforcement path as canonical `domains` entries. ### Not backward compatible - Permissions profiles under `[permissions.<profile>.network]` no longer accept the legacy list-based keys. Those configs must use the canonical `[domains]` and `[unix_sockets]` tables instead of `allowed_domains`, `denied_domains`, or `allow_unix_sockets`. - Managed `experimental_network` config cannot mix canonical and legacy forms in the same block. For example, `domains` cannot be combined with `allowed_domains` or `denied_domains`, and `unix_sockets` cannot be combined with `allow_unix_sockets`. - The canonical format can express explicit `"none"` entries for unix sockets, but those entries do not round-trip through the legacy compatibility fields because the legacy fields only represent allow/deny lists. ## Testing `/target/debug/codex sandbox macos --log-denials /bin/zsh -c 'curl https://www.example.com' ` gives 200 with config ``` [permissions.workspace.network.domains] "www.example.com" = "allow" ``` and fails when set to deny: `curl: (56) CONNECT tunnel failed, response 403`. Also tested backward compatibility path by verifying that adding the following to `/etc/codex/requirements.toml` works: ``` [experimental_network] allowed_domains = ["www.example.com"] ```
This commit is contained in:
@@ -3,7 +3,10 @@ use anyhow::Result;
|
||||
use anyhow::bail;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use serde::Deserialize;
|
||||
use serde::Deserializer;
|
||||
use serde::Serialize;
|
||||
use serde::Serializer;
|
||||
use std::collections::BTreeMap;
|
||||
use std::net::IpAddr;
|
||||
use std::net::SocketAddr;
|
||||
use std::path::Path;
|
||||
@@ -16,6 +19,101 @@ pub struct NetworkProxyConfig {
|
||||
pub network: NetworkProxySettings,
|
||||
}
|
||||
|
||||
/// Variant order encodes effective precedence for duplicate patterns:
|
||||
/// `None < Allow < Deny`, so deny wins over allow when entries conflict.
|
||||
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
pub enum NetworkDomainPermission {
|
||||
None,
|
||||
Allow,
|
||||
Deny,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct NetworkDomainPermissionEntry {
|
||||
pub pattern: String,
|
||||
pub permission: NetworkDomainPermission,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq)]
|
||||
pub struct NetworkDomainPermissions {
|
||||
pub entries: Vec<NetworkDomainPermissionEntry>,
|
||||
}
|
||||
|
||||
impl Serialize for NetworkDomainPermissions {
|
||||
fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
|
||||
where
|
||||
S: Serializer,
|
||||
{
|
||||
self.effective_entries()
|
||||
.into_iter()
|
||||
.map(|entry| (entry.pattern, entry.permission))
|
||||
.collect::<BTreeMap<_, _>>()
|
||||
.serialize(serializer)
|
||||
}
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for NetworkDomainPermissions {
|
||||
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
|
||||
where
|
||||
D: Deserializer<'de>,
|
||||
{
|
||||
let entries = BTreeMap::<String, NetworkDomainPermission>::deserialize(deserializer)?
|
||||
.into_iter()
|
||||
.map(|(pattern, permission)| NetworkDomainPermissionEntry {
|
||||
pattern,
|
||||
permission,
|
||||
})
|
||||
.collect();
|
||||
Ok(Self { entries })
|
||||
}
|
||||
}
|
||||
|
||||
impl NetworkDomainPermissions {
|
||||
fn effective_entries(&self) -> Vec<NetworkDomainPermissionEntry> {
|
||||
let mut order = Vec::new();
|
||||
let mut effective_permissions = BTreeMap::new();
|
||||
|
||||
for entry in &self.entries {
|
||||
if !effective_permissions.contains_key(&entry.pattern) {
|
||||
order.push(entry.pattern.clone());
|
||||
}
|
||||
|
||||
let permission = effective_permissions
|
||||
.entry(entry.pattern.clone())
|
||||
.or_insert(entry.permission);
|
||||
if entry.permission > *permission {
|
||||
*permission = entry.permission;
|
||||
}
|
||||
}
|
||||
|
||||
order
|
||||
.into_iter()
|
||||
.filter_map(|pattern| {
|
||||
effective_permissions.remove(&pattern).map(|permission| {
|
||||
NetworkDomainPermissionEntry {
|
||||
pattern,
|
||||
permission,
|
||||
}
|
||||
})
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
pub enum NetworkUnixSocketPermission {
|
||||
Allow,
|
||||
None,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq, Eq)]
|
||||
pub struct NetworkUnixSocketPermissions {
|
||||
#[serde(flatten)]
|
||||
pub entries: BTreeMap<String, NetworkUnixSocketPermission>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
|
||||
#[serde(default)]
|
||||
pub struct NetworkProxySettings {
|
||||
@@ -35,11 +133,9 @@ pub struct NetworkProxySettings {
|
||||
#[serde(default)]
|
||||
pub mode: NetworkMode,
|
||||
#[serde(default)]
|
||||
pub allowed_domains: Vec<String>,
|
||||
pub domains: Option<NetworkDomainPermissions>,
|
||||
#[serde(default)]
|
||||
pub denied_domains: Vec<String>,
|
||||
#[serde(default)]
|
||||
pub allow_unix_sockets: Vec<String>,
|
||||
pub unix_sockets: Option<NetworkUnixSocketPermissions>,
|
||||
pub allow_local_binding: bool,
|
||||
#[serde(default)]
|
||||
pub mitm: bool,
|
||||
@@ -57,15 +153,119 @@ impl Default for NetworkProxySettings {
|
||||
dangerously_allow_non_loopback_proxy: false,
|
||||
dangerously_allow_all_unix_sockets: false,
|
||||
mode: NetworkMode::default(),
|
||||
allowed_domains: Vec::new(),
|
||||
denied_domains: Vec::new(),
|
||||
allow_unix_sockets: Vec::new(),
|
||||
domains: None,
|
||||
unix_sockets: None,
|
||||
allow_local_binding: false,
|
||||
mitm: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl NetworkProxySettings {
|
||||
pub fn allowed_domains(&self) -> Option<Vec<String>> {
|
||||
self.domain_entries(NetworkDomainPermission::Allow)
|
||||
}
|
||||
|
||||
pub fn denied_domains(&self) -> Option<Vec<String>> {
|
||||
self.domain_entries(NetworkDomainPermission::Deny)
|
||||
}
|
||||
|
||||
fn domain_entries(&self, permission: NetworkDomainPermission) -> Option<Vec<String>> {
|
||||
self.domains
|
||||
.as_ref()
|
||||
.map(|domains| {
|
||||
domains
|
||||
.effective_entries()
|
||||
.iter()
|
||||
.filter(|entry| entry.permission == permission)
|
||||
.map(|entry| entry.pattern.clone())
|
||||
.collect()
|
||||
})
|
||||
.filter(|entries: &Vec<String>| !entries.is_empty())
|
||||
}
|
||||
|
||||
pub fn allow_unix_sockets(&self) -> Vec<String> {
|
||||
self.unix_sockets
|
||||
.as_ref()
|
||||
.map(|unix_sockets| {
|
||||
unix_sockets
|
||||
.entries
|
||||
.iter()
|
||||
.filter(|(_, permission)| {
|
||||
matches!(permission, NetworkUnixSocketPermission::Allow)
|
||||
})
|
||||
.map(|(path, _)| path.clone())
|
||||
.collect()
|
||||
})
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
pub fn set_allowed_domains(&mut self, allowed_domains: Vec<String>) {
|
||||
self.set_domain_entries(allowed_domains, NetworkDomainPermission::Allow);
|
||||
}
|
||||
|
||||
pub fn set_denied_domains(&mut self, denied_domains: Vec<String>) {
|
||||
self.set_domain_entries(denied_domains, NetworkDomainPermission::Deny);
|
||||
}
|
||||
|
||||
pub fn upsert_domain_permission(
|
||||
&mut self,
|
||||
host: String,
|
||||
permission: NetworkDomainPermission,
|
||||
normalize: impl Fn(&str) -> String,
|
||||
) {
|
||||
let mut domains = self.domains.take().unwrap_or_default();
|
||||
let normalized_host = normalize(&host);
|
||||
domains
|
||||
.entries
|
||||
.retain(|entry| normalize(&entry.pattern) != normalized_host);
|
||||
domains.entries.push(NetworkDomainPermissionEntry {
|
||||
pattern: host,
|
||||
permission,
|
||||
});
|
||||
self.domains = (!domains.entries.is_empty()).then_some(domains);
|
||||
}
|
||||
|
||||
pub fn set_allow_unix_sockets(&mut self, allow_unix_sockets: Vec<String>) {
|
||||
self.set_unix_socket_entries(allow_unix_sockets, NetworkUnixSocketPermission::Allow);
|
||||
}
|
||||
|
||||
fn set_domain_entries(&mut self, entries: Vec<String>, permission: NetworkDomainPermission) {
|
||||
let mut domains = self.domains.take().unwrap_or_default();
|
||||
domains
|
||||
.entries
|
||||
.retain(|entry| entry.permission != permission);
|
||||
for entry in entries {
|
||||
if !domains
|
||||
.entries
|
||||
.iter()
|
||||
.any(|existing| existing.pattern == entry && existing.permission == permission)
|
||||
{
|
||||
domains.entries.push(NetworkDomainPermissionEntry {
|
||||
pattern: entry,
|
||||
permission,
|
||||
});
|
||||
}
|
||||
}
|
||||
self.domains = (!domains.entries.is_empty()).then_some(domains);
|
||||
}
|
||||
|
||||
fn set_unix_socket_entries(
|
||||
&mut self,
|
||||
entries: Vec<String>,
|
||||
permission: NetworkUnixSocketPermission,
|
||||
) {
|
||||
let mut unix_sockets = self.unix_sockets.take().unwrap_or_default();
|
||||
unix_sockets
|
||||
.entries
|
||||
.retain(|_, existing| *existing != permission);
|
||||
for entry in entries {
|
||||
unix_sockets.entries.insert(entry, permission);
|
||||
}
|
||||
self.unix_sockets = (!unix_sockets.entries.is_empty()).then_some(unix_sockets);
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
pub enum NetworkMode {
|
||||
@@ -136,7 +336,7 @@ pub(crate) fn clamp_bind_addrs(
|
||||
"SOCKS5 proxy",
|
||||
"dangerously_allow_non_loopback_proxy",
|
||||
);
|
||||
if cfg.allow_unix_sockets.is_empty() && !cfg.dangerously_allow_all_unix_sockets {
|
||||
if cfg.allow_unix_sockets().is_empty() && !cfg.dangerously_allow_all_unix_sockets {
|
||||
return (http_addr, socks_addr);
|
||||
}
|
||||
|
||||
@@ -198,7 +398,7 @@ impl ValidatedUnixSocketPath {
|
||||
}
|
||||
|
||||
pub(crate) fn validate_unix_socket_allowlist_paths(cfg: &NetworkProxyConfig) -> Result<()> {
|
||||
for (index, socket_path) in cfg.network.allow_unix_sockets.iter().enumerate() {
|
||||
for (index, socket_path) in cfg.network.allow_unix_sockets().iter().enumerate() {
|
||||
ValidatedUnixSocketPath::parse(socket_path)
|
||||
.with_context(|| format!("invalid network.allow_unix_sockets[{index}]"))?;
|
||||
}
|
||||
@@ -357,6 +557,19 @@ mod tests {
|
||||
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn settings_with_unix_sockets(unix_sockets: &[&str]) -> NetworkProxySettings {
|
||||
let mut settings = NetworkProxySettings::default();
|
||||
if !unix_sockets.is_empty() {
|
||||
settings.set_allow_unix_sockets(
|
||||
unix_sockets
|
||||
.iter()
|
||||
.map(|path| (*path).to_string())
|
||||
.collect(),
|
||||
);
|
||||
}
|
||||
settings
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn network_proxy_settings_default_matches_local_use_baseline() {
|
||||
assert_eq!(
|
||||
@@ -371,9 +584,8 @@ mod tests {
|
||||
dangerously_allow_non_loopback_proxy: false,
|
||||
dangerously_allow_all_unix_sockets: false,
|
||||
mode: NetworkMode::Full,
|
||||
allowed_domains: Vec::new(),
|
||||
denied_domains: Vec::new(),
|
||||
allow_unix_sockets: Vec::new(),
|
||||
domains: None,
|
||||
unix_sockets: None,
|
||||
allow_local_binding: false,
|
||||
mitm: false,
|
||||
}
|
||||
@@ -398,6 +610,53 @@ mod tests {
|
||||
assert_eq!(config.network, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn set_allowed_domains_preserves_existing_deny_for_same_pattern() {
|
||||
let mut settings = NetworkProxySettings::default();
|
||||
settings.set_denied_domains(vec!["example.com".to_string()]);
|
||||
|
||||
settings.set_allowed_domains(vec!["example.com".to_string()]);
|
||||
|
||||
assert_eq!(settings.allowed_domains(), None);
|
||||
assert_eq!(
|
||||
settings.denied_domains(),
|
||||
Some(vec!["example.com".to_string()])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn network_domain_permissions_serialize_to_effective_map_shape() {
|
||||
let mut settings = NetworkProxySettings::default();
|
||||
settings.set_denied_domains(vec!["example.com".to_string()]);
|
||||
settings.set_allowed_domains(vec!["example.com".to_string()]);
|
||||
let config = NetworkProxyConfig { network: settings };
|
||||
|
||||
let value = serde_json::to_value(&config).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
value,
|
||||
serde_json::json!({
|
||||
"network": {
|
||||
"enabled": false,
|
||||
"proxy_url": "http://127.0.0.1:3128",
|
||||
"enable_socks5": true,
|
||||
"socks_url": "http://127.0.0.1:8081",
|
||||
"enable_socks5_udp": true,
|
||||
"allow_upstream_proxy": true,
|
||||
"dangerously_allow_non_loopback_proxy": false,
|
||||
"dangerously_allow_all_unix_sockets": false,
|
||||
"mode": "full",
|
||||
"domains": {
|
||||
"example.com": "deny",
|
||||
},
|
||||
"unix_sockets": null,
|
||||
"allow_local_binding": false,
|
||||
"mitm": false,
|
||||
}
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_host_port_defaults_for_empty_string() {
|
||||
assert!(parse_host_port("", 1234).is_err());
|
||||
@@ -536,10 +795,10 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn clamp_bind_addrs_forces_loopback_when_unix_sockets_enabled() {
|
||||
let cfg = NetworkProxySettings {
|
||||
dangerously_allow_non_loopback_proxy: true,
|
||||
allow_unix_sockets: vec!["/tmp/docker.sock".to_string()],
|
||||
..Default::default()
|
||||
let cfg = {
|
||||
let mut settings = settings_with_unix_sockets(&["/tmp/docker.sock"]);
|
||||
settings.dangerously_allow_non_loopback_proxy = true;
|
||||
settings
|
||||
};
|
||||
let http_addr = "0.0.0.0:3128".parse::<SocketAddr>().unwrap();
|
||||
let socks_addr = "0.0.0.0:8081".parse::<SocketAddr>().unwrap();
|
||||
@@ -569,10 +828,7 @@ mod tests {
|
||||
#[test]
|
||||
fn resolve_runtime_rejects_relative_allow_unix_sockets_entries() {
|
||||
let cfg = NetworkProxyConfig {
|
||||
network: NetworkProxySettings {
|
||||
allow_unix_sockets: vec!["relative.sock".to_string()],
|
||||
..NetworkProxySettings::default()
|
||||
},
|
||||
network: settings_with_unix_sockets(&["relative.sock"]),
|
||||
};
|
||||
|
||||
let err = match resolve_runtime(&cfg) {
|
||||
@@ -591,10 +847,7 @@ mod tests {
|
||||
#[test]
|
||||
fn resolve_runtime_accepts_unix_style_absolute_allow_unix_sockets_entries() {
|
||||
let cfg = NetworkProxyConfig {
|
||||
network: NetworkProxySettings {
|
||||
allow_unix_sockets: vec!["/private/tmp/example.sock".to_string()],
|
||||
..NetworkProxySettings::default()
|
||||
},
|
||||
network: settings_with_unix_sockets(&["/private/tmp/example.sock"]),
|
||||
};
|
||||
|
||||
assert!(
|
||||
|
||||
Reference in New Issue
Block a user