mirror of
https://github.com/openai/codex.git
synced 2026-04-28 00:25:56 +00:00
feat(core): persist network approvals in execpolicy (#12357)
## Summary Persist network approval allow/deny decisions as `network_rule(...)` entries in execpolicy (not proxy config) It adds `network_rule` parsing + append support in `codex-execpolicy`, including `decision="prompt"` (parse-only; not compiled into proxy allow/deny lists) - compile execpolicy network rules into proxy allow/deny lists and update the live proxy state on approval - preserve requirements execpolicy `network_rule(...)` entries when merging with file-based execpolicy - reject broad wildcard hosts (for example `*`) for persisted `network_rule(...)`
This commit is contained in:
@@ -2,8 +2,17 @@
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_core::config::Constrained;
|
||||
use codex_core::config_loader::ConfigLayerStack;
|
||||
use codex_core::config_loader::ConfigLayerStackOrdering;
|
||||
use codex_core::config_loader::NetworkConstraints;
|
||||
use codex_core::config_loader::NetworkRequirementsToml;
|
||||
use codex_core::config_loader::RequirementSource;
|
||||
use codex_core::config_loader::Sourced;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::sandboxing::SandboxPermissions;
|
||||
use codex_protocol::approvals::NetworkApprovalProtocol;
|
||||
use codex_protocol::approvals::NetworkPolicyAmendment;
|
||||
use codex_protocol::approvals::NetworkPolicyRuleAction;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::protocol::ApplyPatchApprovalRequestEvent;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -26,6 +35,7 @@ use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use core_test_support::wait_for_event_with_timeout;
|
||||
use pretty_assertions::assert_eq;
|
||||
use regex_lite::Regex;
|
||||
use serde_json::Value;
|
||||
@@ -33,6 +43,8 @@ use serde_json::json;
|
||||
use std::env;
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use tempfile::TempDir;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::ResponseTemplate;
|
||||
@@ -2106,6 +2118,305 @@ async fn approving_fallback_rule_for_compound_command_works() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn denying_network_policy_amendment_persists_policy_and_skips_future_network_prompt()
|
||||
-> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let home = Arc::new(TempDir::new()?);
|
||||
fs::write(
|
||||
home.path().join("config.toml"),
|
||||
r#"[permissions.network]
|
||||
enabled = true
|
||||
mode = "limited"
|
||||
allow_local_binding = true
|
||||
"#,
|
||||
)?;
|
||||
let approval_policy = AskForApproval::OnFailure;
|
||||
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
read_only_access: Default::default(),
|
||||
network_access: true,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
};
|
||||
let sandbox_policy_for_config = sandbox_policy.clone();
|
||||
let mut builder = test_codex().with_home(home).with_config(move |config| {
|
||||
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
|
||||
config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
|
||||
let layers = config
|
||||
.config_layer_stack
|
||||
.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true)
|
||||
.into_iter()
|
||||
.cloned()
|
||||
.collect();
|
||||
let mut requirements = config.config_layer_stack.requirements().clone();
|
||||
requirements.network = Some(Sourced::new(
|
||||
NetworkConstraints {
|
||||
enabled: Some(true),
|
||||
allow_local_binding: Some(true),
|
||||
..Default::default()
|
||||
},
|
||||
RequirementSource::CloudRequirements,
|
||||
));
|
||||
let mut requirements_toml = config.config_layer_stack.requirements_toml().clone();
|
||||
requirements_toml.network = Some(NetworkRequirementsToml {
|
||||
enabled: Some(true),
|
||||
allow_local_binding: Some(true),
|
||||
..Default::default()
|
||||
});
|
||||
config.config_layer_stack = ConfigLayerStack::new(layers, requirements, requirements_toml)
|
||||
.expect("rebuild config layer stack with network requirements");
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
assert!(
|
||||
test.config.managed_network_requirements_enabled(),
|
||||
"expected managed network requirements to be enabled"
|
||||
);
|
||||
assert!(
|
||||
test.config.permissions.network.is_some(),
|
||||
"expected managed network proxy config to be present"
|
||||
);
|
||||
let runtime_proxy = test
|
||||
.session_configured
|
||||
.network_proxy
|
||||
.as_ref()
|
||||
.expect("expected runtime managed network proxy addresses");
|
||||
let proxy_addr = runtime_proxy.http_addr.as_str();
|
||||
|
||||
let call_id_first = "allow-network-first";
|
||||
// Use the same urllib-based pattern as the other network integration tests,
|
||||
// but point it at the runtime proxy directly so the blocked host reliably
|
||||
// produces a network approval request without relying on curl.
|
||||
let fetch_command = format!(
|
||||
"python3 -c \"import urllib.request; proxy = urllib.request.ProxyHandler({{'http': 'http://{proxy_addr}'}}); opener = urllib.request.build_opener(proxy); print('OK:' + opener.open('http://codex-network-test.invalid', timeout=30).read().decode(errors='replace'))\""
|
||||
);
|
||||
let first_event = shell_event(
|
||||
call_id_first,
|
||||
&fetch_command,
|
||||
30_000,
|
||||
SandboxPermissions::UseDefault,
|
||||
)?;
|
||||
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-allow-network-1"),
|
||||
first_event,
|
||||
ev_completed("resp-allow-network-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let first_results = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-allow-network-1", "done"),
|
||||
ev_completed("resp-allow-network-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn(
|
||||
&test,
|
||||
"allow-network-first",
|
||||
approval_policy,
|
||||
sandbox_policy.clone(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30);
|
||||
let approval = loop {
|
||||
let remaining = deadline
|
||||
.checked_duration_since(std::time::Instant::now())
|
||||
.expect("timed out waiting for network approval request");
|
||||
let event = wait_for_event_with_timeout(
|
||||
&test.codex,
|
||||
|event| {
|
||||
matches!(
|
||||
event,
|
||||
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
|
||||
)
|
||||
},
|
||||
remaining,
|
||||
)
|
||||
.await;
|
||||
match event {
|
||||
EventMsg::ExecApprovalRequest(approval) => {
|
||||
if approval.command.first().map(std::string::String::as_str)
|
||||
== Some("network-access")
|
||||
{
|
||||
break approval;
|
||||
}
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::Approved,
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
EventMsg::TurnComplete(_) => {
|
||||
panic!("expected network approval request before completion");
|
||||
}
|
||||
other => panic!("unexpected event: {other:?}"),
|
||||
}
|
||||
};
|
||||
let network_context = approval
|
||||
.network_approval_context
|
||||
.clone()
|
||||
.expect("expected network approval context");
|
||||
assert_eq!(network_context.protocol, NetworkApprovalProtocol::Http);
|
||||
let expected_network_amendments = vec![
|
||||
NetworkPolicyAmendment {
|
||||
host: network_context.host.clone(),
|
||||
action: NetworkPolicyRuleAction::Allow,
|
||||
},
|
||||
NetworkPolicyAmendment {
|
||||
host: network_context.host.clone(),
|
||||
action: NetworkPolicyRuleAction::Deny,
|
||||
},
|
||||
];
|
||||
assert_eq!(
|
||||
approval.proposed_network_policy_amendments,
|
||||
Some(expected_network_amendments.clone())
|
||||
);
|
||||
let deny_network_amendment = expected_network_amendments
|
||||
.into_iter()
|
||||
.find(|amendment| amendment.action == NetworkPolicyRuleAction::Deny)
|
||||
.expect("expected deny network policy amendment");
|
||||
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::NetworkPolicyAmendment {
|
||||
network_policy_amendment: deny_network_amendment.clone(),
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
wait_for_completion(&test).await;
|
||||
|
||||
let policy_path = test.home.path().join("rules").join("default.rules");
|
||||
let policy_contents = fs::read_to_string(&policy_path)?;
|
||||
let expected_rule = format!(
|
||||
r#"network_rule(host="{}", protocol="{}", decision="deny", justification="Deny {} access to {}")"#,
|
||||
deny_network_amendment.host,
|
||||
match network_context.protocol {
|
||||
NetworkApprovalProtocol::Http => "http",
|
||||
NetworkApprovalProtocol::Https => "https_connect",
|
||||
NetworkApprovalProtocol::Socks5Tcp => "socks5_tcp",
|
||||
NetworkApprovalProtocol::Socks5Udp => "socks5_udp",
|
||||
},
|
||||
match network_context.protocol {
|
||||
NetworkApprovalProtocol::Http => "http",
|
||||
NetworkApprovalProtocol::Https => "https_connect",
|
||||
NetworkApprovalProtocol::Socks5Tcp => "socks5_tcp",
|
||||
NetworkApprovalProtocol::Socks5Udp => "socks5_udp",
|
||||
},
|
||||
deny_network_amendment.host
|
||||
);
|
||||
assert!(
|
||||
policy_contents.contains(&expected_rule),
|
||||
"unexpected policy contents: {policy_contents}"
|
||||
);
|
||||
|
||||
let first_output = parse_result(
|
||||
&first_results
|
||||
.single_request()
|
||||
.function_call_output(call_id_first),
|
||||
);
|
||||
Expectation::CommandFailure {
|
||||
output_contains: "",
|
||||
}
|
||||
.verify(&test, &first_output)?;
|
||||
|
||||
let call_id_second = "allow-network-second";
|
||||
let second_event = shell_event(
|
||||
call_id_second,
|
||||
&fetch_command,
|
||||
30_000,
|
||||
SandboxPermissions::UseDefault,
|
||||
)?;
|
||||
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-allow-network-3"),
|
||||
second_event,
|
||||
ev_completed("resp-allow-network-3"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let second_results = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-allow-network-2", "done"),
|
||||
ev_completed("resp-allow-network-4"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn(
|
||||
&test,
|
||||
"allow-network-second",
|
||||
approval_policy,
|
||||
sandbox_policy.clone(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30);
|
||||
loop {
|
||||
let remaining = deadline
|
||||
.checked_duration_since(std::time::Instant::now())
|
||||
.expect("timed out waiting for second turn completion");
|
||||
let event = wait_for_event_with_timeout(
|
||||
&test.codex,
|
||||
|event| {
|
||||
matches!(
|
||||
event,
|
||||
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
|
||||
)
|
||||
},
|
||||
remaining,
|
||||
)
|
||||
.await;
|
||||
match event {
|
||||
EventMsg::ExecApprovalRequest(approval) => {
|
||||
if approval.command.first().map(std::string::String::as_str)
|
||||
== Some("network-access")
|
||||
{
|
||||
panic!(
|
||||
"unexpected network approval request: {:?}",
|
||||
approval.command
|
||||
);
|
||||
}
|
||||
test.codex
|
||||
.submit(Op::ExecApproval {
|
||||
id: approval.effective_approval_id(),
|
||||
turn_id: None,
|
||||
decision: ReviewDecision::Approved,
|
||||
})
|
||||
.await?;
|
||||
}
|
||||
EventMsg::TurnComplete(_) => break,
|
||||
other => panic!("unexpected event: {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
let second_output = parse_result(
|
||||
&second_results
|
||||
.single_request()
|
||||
.function_call_output(call_id_second),
|
||||
);
|
||||
Expectation::CommandFailure {
|
||||
output_contains: "",
|
||||
}
|
||||
.verify(&test, &second_output)?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
// todo(dylan) add ScenarioSpec support for rules
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[cfg(unix)]
|
||||
|
||||
Reference in New Issue
Block a user