fix: persist future network host approvals across sessions (#14619)
## Summary - apply persisted execpolicy network rules when booting the managed network proxy - pass the current execpolicy into managed proxy startup so host approvals selected with "allow this host in the future" survive new sessions
This commit is contained in:
parent
bbd329a812
commit
6dc04df5e6
3 changed files with 142 additions and 0 deletions
|
|
@ -1163,12 +1163,22 @@ impl Session {
|
|||
|
||||
async fn start_managed_network_proxy(
|
||||
spec: &crate::config::NetworkProxySpec,
|
||||
exec_policy: &codex_execpolicy::Policy,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
network_policy_decider: Option<Arc<dyn codex_network_proxy::NetworkPolicyDecider>>,
|
||||
blocked_request_observer: Option<Arc<dyn codex_network_proxy::BlockedRequestObserver>>,
|
||||
managed_network_requirements_enabled: bool,
|
||||
audit_metadata: NetworkProxyAuditMetadata,
|
||||
) -> anyhow::Result<(StartedNetworkProxy, SessionNetworkProxyRuntime)> {
|
||||
let spec = spec
|
||||
.with_exec_policy_network_rules(exec_policy)
|
||||
.map_err(|err| {
|
||||
tracing::warn!(
|
||||
"failed to apply execpolicy network rules to managed proxy; continuing with configured network policy: {err}"
|
||||
);
|
||||
err
|
||||
})
|
||||
.unwrap_or_else(|_| spec.clone());
|
||||
let network_proxy = spec
|
||||
.start_proxy(
|
||||
sandbox_policy,
|
||||
|
|
@ -1692,8 +1702,10 @@ impl Session {
|
|||
});
|
||||
let (network_proxy, session_network_proxy) =
|
||||
if let Some(spec) = config.permissions.network.as_ref() {
|
||||
let current_exec_policy = exec_policy.current();
|
||||
let (network_proxy, session_network_proxy) = Self::start_managed_network_proxy(
|
||||
spec,
|
||||
current_exec_policy.as_ref(),
|
||||
config.permissions.sandbox_policy.get(),
|
||||
network_policy_decider.as_ref().map(Arc::clone),
|
||||
blocked_request_observer.as_ref().map(Arc::clone),
|
||||
|
|
|
|||
|
|
@ -56,6 +56,10 @@ use crate::tools::registry::ToolHandler;
|
|||
use crate::tools::router::ToolCallSource;
|
||||
use crate::turn_diff_tracker::TurnDiffTracker;
|
||||
use codex_app_server_protocol::AppInfo;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::NetworkRuleProtocol;
|
||||
use codex_execpolicy::Policy;
|
||||
use codex_network_proxy::NetworkProxyConfig;
|
||||
use codex_otel::TelemetryAuthMode;
|
||||
use codex_protocol::models::BaseInstructions;
|
||||
use codex_protocol::models::ContentItem;
|
||||
|
|
@ -321,6 +325,79 @@ fn validated_network_policy_amendment_host_rejects_mismatch() {
|
|||
assert!(message.contains("does not match approved host"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn start_managed_network_proxy_applies_execpolicy_network_rules() -> anyhow::Result<()> {
|
||||
let spec = crate::config::NetworkProxySpec::from_config_and_constraints(
|
||||
NetworkProxyConfig::default(),
|
||||
None,
|
||||
&SandboxPolicy::new_workspace_write_policy(),
|
||||
)?;
|
||||
let mut exec_policy = Policy::empty();
|
||||
exec_policy.add_network_rule(
|
||||
"example.com",
|
||||
NetworkRuleProtocol::Https,
|
||||
Decision::Allow,
|
||||
None,
|
||||
)?;
|
||||
|
||||
let (started_proxy, _) = Session::start_managed_network_proxy(
|
||||
&spec,
|
||||
&exec_policy,
|
||||
&SandboxPolicy::new_workspace_write_policy(),
|
||||
None,
|
||||
None,
|
||||
false,
|
||||
crate::config::NetworkProxyAuditMetadata::default(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let current_cfg = started_proxy.proxy().current_cfg().await?;
|
||||
assert_eq!(
|
||||
current_cfg.network.allowed_domains,
|
||||
vec!["example.com".to_string()]
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn start_managed_network_proxy_ignores_invalid_execpolicy_network_rules() -> anyhow::Result<()>
|
||||
{
|
||||
let spec = crate::config::NetworkProxySpec::from_config_and_constraints(
|
||||
NetworkProxyConfig::default(),
|
||||
Some(NetworkConstraints {
|
||||
allowed_domains: Some(vec!["managed.example.com".to_string()]),
|
||||
managed_allowed_domains_only: Some(true),
|
||||
..Default::default()
|
||||
}),
|
||||
&SandboxPolicy::new_workspace_write_policy(),
|
||||
)?;
|
||||
let mut exec_policy = Policy::empty();
|
||||
exec_policy.add_network_rule(
|
||||
"example.com",
|
||||
NetworkRuleProtocol::Https,
|
||||
Decision::Allow,
|
||||
None,
|
||||
)?;
|
||||
|
||||
let (started_proxy, _) = Session::start_managed_network_proxy(
|
||||
&spec,
|
||||
&exec_policy,
|
||||
&SandboxPolicy::new_workspace_write_policy(),
|
||||
None,
|
||||
None,
|
||||
false,
|
||||
crate::config::NetworkProxyAuditMetadata::default(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let current_cfg = started_proxy.proxy().current_cfg().await?;
|
||||
assert_eq!(
|
||||
current_cfg.network.allowed_domains,
|
||||
vec!["managed.example.com".to_string()]
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn get_base_instructions_no_user_content() {
|
||||
let prompt_with_apply_patch_instructions =
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
use crate::config_loader::NetworkConstraints;
|
||||
use async_trait::async_trait;
|
||||
use codex_execpolicy::Policy;
|
||||
use codex_network_proxy::BlockedRequestObserver;
|
||||
use codex_network_proxy::ConfigReloader;
|
||||
use codex_network_proxy::ConfigState;
|
||||
|
|
@ -13,8 +14,10 @@ use codex_network_proxy::NetworkProxyHandle;
|
|||
use codex_network_proxy::NetworkProxyState;
|
||||
use codex_network_proxy::build_config_state;
|
||||
use codex_network_proxy::host_and_port_from_network_addr;
|
||||
use codex_network_proxy::normalize_host;
|
||||
use codex_network_proxy::validate_policy_against_constraints;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
|
|
@ -151,6 +154,21 @@ impl NetworkProxySpec {
|
|||
Ok(StartedNetworkProxy::new(proxy, handle))
|
||||
}
|
||||
|
||||
pub(crate) fn with_exec_policy_network_rules(
|
||||
&self,
|
||||
exec_policy: &Policy,
|
||||
) -> std::io::Result<Self> {
|
||||
let mut spec = self.clone();
|
||||
apply_exec_policy_network_rules(&mut spec.config, exec_policy);
|
||||
validate_policy_against_constraints(&spec.config, &spec.constraints).map_err(|err| {
|
||||
std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
format!("network proxy constraints are invalid: {err}"),
|
||||
)
|
||||
})?;
|
||||
Ok(spec)
|
||||
}
|
||||
|
||||
fn build_state_with_audit_metadata(
|
||||
&self,
|
||||
audit_metadata: NetworkProxyAuditMetadata,
|
||||
|
|
@ -279,6 +297,41 @@ impl NetworkProxySpec {
|
|||
}
|
||||
}
|
||||
|
||||
fn apply_exec_policy_network_rules(config: &mut NetworkProxyConfig, exec_policy: &Policy) {
|
||||
let (allowed_domains, denied_domains) = exec_policy.compiled_network_domains();
|
||||
upsert_network_domains(
|
||||
&mut config.network.allowed_domains,
|
||||
&mut config.network.denied_domains,
|
||||
allowed_domains,
|
||||
);
|
||||
upsert_network_domains(
|
||||
&mut config.network.denied_domains,
|
||||
&mut config.network.allowed_domains,
|
||||
denied_domains,
|
||||
);
|
||||
}
|
||||
|
||||
fn upsert_network_domains(
|
||||
target: &mut Vec<String>,
|
||||
opposite: &mut Vec<String>,
|
||||
hosts: Vec<String>,
|
||||
) {
|
||||
let mut incoming = HashSet::new();
|
||||
let mut deduped_hosts = Vec::new();
|
||||
for host in hosts {
|
||||
if incoming.insert(host.clone()) {
|
||||
deduped_hosts.push(host);
|
||||
}
|
||||
}
|
||||
if incoming.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
opposite.retain(|entry| !incoming.contains(&normalize_host(entry)));
|
||||
target.retain(|entry| !incoming.contains(&normalize_host(entry)));
|
||||
target.extend(deduped_hosts);
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "network_proxy_spec_tests.rs"]
|
||||
mod tests;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue