mirror of
https://github.com/openai/codex.git
synced 2026-10-21 09:54:56 +00:00
Defer environment network policy validation until after composition (#46331)
## Why Feature settings and managed requirements can replace domain and socket values. Validating the selected configuration first rejects invalid entries even when they would be replaced before use. ## What changed Keep listener removal and unsupported controller-field rejection in `project_environment_profile_network`, but defer domain and socket validation to `validate_environment_network_policy` for the composed policy. Remove the executor OS argument from the preparation step. ## Testing Update tests to verify that invalid domain patterns and socket paths survive preparation, fail final validation when retained, and pass when managed requirements replace them. GitOrigin-RevId: 866c633c14ff0b4664e8bf5cf79fef51db5d7ed6
This commit is contained in:
@@ -15,9 +15,7 @@ use codex_features::FeatureToml;
|
||||
use codex_features::FeaturesToml;
|
||||
use codex_network_proxy::EnvironmentNetworkPolicy;
|
||||
use codex_network_proxy::NetworkProxyConfig;
|
||||
use codex_network_proxy::NetworkProxyConstraints;
|
||||
use codex_network_proxy::NetworkProxyExecutorOs;
|
||||
use codex_network_proxy::build_config_state;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
|
||||
use super::NetworkProxySpec;
|
||||
@@ -27,16 +25,14 @@ use super::permissions::apply_network_proxy_feature_config;
|
||||
#[error("invalid portable environment network policy")]
|
||||
pub struct EnvironmentNetworkConfigError;
|
||||
|
||||
/// Drops listener addresses, which the executor chooses for each command, before
|
||||
/// validating an environment's selected network configuration. Use the returned
|
||||
/// value when deciding whether a policy is present and when preparing it.
|
||||
/// Drops listener addresses and rejects unsupported controller fields in an
|
||||
/// environment's selected network configuration. Use the returned value when
|
||||
/// deciding whether a policy is present and when preparing it.
|
||||
///
|
||||
/// Socket policy validation uses the supplied executor OS: Allow keys must be
|
||||
/// NUL-free and absolute for that OS; Deny keys are preserved unchanged.
|
||||
/// Path normalization and socket support are determined by the executor.
|
||||
/// Domain and socket values may be replaced by feature settings or requirements.
|
||||
/// Validate the composed policy with [`validate_environment_network_policy`].
|
||||
pub fn project_environment_profile_network(
|
||||
network: Option<NetworkToml>,
|
||||
executor_os: NetworkProxyExecutorOs,
|
||||
) -> Result<Option<NetworkToml>, EnvironmentNetworkConfigError> {
|
||||
let Some(mut network) = network else {
|
||||
return Ok(None);
|
||||
@@ -55,15 +51,6 @@ pub fn project_environment_profile_network(
|
||||
if network != supported {
|
||||
return Err(EnvironmentNetworkConfigError);
|
||||
}
|
||||
// Reuse shared, host-independent policy validation.
|
||||
// With MITM and credentials excluded, this checks patterns and allowed
|
||||
// socket paths without discovering executor files.
|
||||
build_config_state(
|
||||
network.to_network_proxy_config(),
|
||||
NetworkProxyConstraints::default(),
|
||||
executor_os,
|
||||
)
|
||||
.map_err(|_| EnvironmentNetworkConfigError)?;
|
||||
Ok(Some(network))
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
|
||||
use super::*;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_network_proxy::NetworkProxyConstraints;
|
||||
use codex_network_proxy::build_config_state;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
@@ -38,7 +40,7 @@ fn environment_policy_presence_keeps_selected_and_managed_denials() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn attachment_projection_uses_executor_os_and_drops_listener_addresses() {
|
||||
fn attachment_projection_preserves_policy_and_drops_listener_addresses() {
|
||||
for executor_os in [
|
||||
NetworkProxyExecutorOs::Linux,
|
||||
NetworkProxyExecutorOs::Macos,
|
||||
@@ -108,24 +110,20 @@ allow_local_binding = false
|
||||
..expected.clone()
|
||||
};
|
||||
assert_eq!(
|
||||
project_environment_profile_network(Some(network), executor_os),
|
||||
if accepted {
|
||||
Ok(Some(expected))
|
||||
} else {
|
||||
Err(EnvironmentNetworkConfigError)
|
||||
},
|
||||
project_environment_profile_network(Some(network)),
|
||||
Ok(Some(expected)),
|
||||
"projection: {executor_os:?}, {raw}"
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
project_environment_profile_network(/*network*/ None, executor_os),
|
||||
project_environment_profile_network(/*network*/ None),
|
||||
Ok(None)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn attachment_projection_rejects_unsupported_restrictions_and_invalid_policy() {
|
||||
fn attachment_projection_rejects_unsupported_restrictions_and_defers_policy_validation() {
|
||||
for raw in [
|
||||
"mode = 'full'",
|
||||
"mode = 'limited'",
|
||||
@@ -138,16 +136,76 @@ methods = ['GET']
|
||||
path_prefixes = ['/']
|
||||
action = ['redact']
|
||||
"#,
|
||||
] {
|
||||
let network: NetworkToml = toml::from_str(raw).unwrap();
|
||||
assert_eq!(
|
||||
project_environment_profile_network(Some(network)),
|
||||
Err(EnvironmentNetworkConfigError),
|
||||
"{raw}"
|
||||
);
|
||||
}
|
||||
|
||||
for raw in [
|
||||
"[unix_sockets]\n'relative.sock' = 'allow'",
|
||||
"[unix_sockets]\n'~/allowed.sock' = 'allow'",
|
||||
"[unix_sockets]\n\"/tmp/\\u0000allowed.sock\" = 'allow'",
|
||||
"[domains]\n'[' = 'allow'",
|
||||
] {
|
||||
let network: NetworkToml = toml::from_str(raw).unwrap();
|
||||
let projected = project_environment_profile_network(Some(network.clone()))
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert_eq!(projected, network);
|
||||
let configured_proxy = projected.to_network_proxy_config();
|
||||
let policy = EnvironmentNetworkPolicy::from_config(
|
||||
&configured_proxy,
|
||||
/*managed_allowed_domains_only*/ false,
|
||||
);
|
||||
assert_eq!(
|
||||
project_environment_profile_network(Some(network), NetworkProxyExecutorOs::Unknown),
|
||||
validate_environment_network_policy(
|
||||
&policy,
|
||||
&PermissionProfile::read_only(),
|
||||
NetworkProxyExecutorOs::Unknown,
|
||||
),
|
||||
Err(EnvironmentNetworkConfigError),
|
||||
"{raw}"
|
||||
);
|
||||
|
||||
// Managed requirements replace the invalid Allow entries before validation.
|
||||
let policy = PreparedNetworkConfig { configured_proxy }
|
||||
.build_environment_policy(
|
||||
Some(Sourced::new(
|
||||
NetworkConstraints {
|
||||
enabled: Some(true),
|
||||
managed_allowed_domains_only: Some(true),
|
||||
unix_sockets: Some(Default::default()),
|
||||
..Default::default()
|
||||
},
|
||||
RequirementSource::Unknown,
|
||||
)),
|
||||
&PermissionProfile::read_only(),
|
||||
/*has_selected_policy*/ true,
|
||||
)
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
policy,
|
||||
EnvironmentNetworkPolicy::from_config(
|
||||
&NetworkProxyConfig {
|
||||
unix_sockets: Some(Default::default()),
|
||||
..Default::default()
|
||||
},
|
||||
/*managed_allowed_domains_only*/ true,
|
||||
)
|
||||
);
|
||||
assert_eq!(
|
||||
validate_environment_network_policy(
|
||||
&policy,
|
||||
&PermissionProfile::read_only(),
|
||||
NetworkProxyExecutorOs::Unknown,
|
||||
),
|
||||
Ok(()),
|
||||
"{raw}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user