core/core-agent-ide
8
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

chore: introduce *Args types for new() methods (#10009)

Browse source 
Constructors with long param lists can be hard to reason about when a
number of the args are `None`, in practice. Introducing a struct to use
as the args type helps make things more self-documenting.
This commit is contained in:
Michael Bolin 2026-01-27 11:15:38 -08:00 • committed by GitHub
parent c40ad65bd8
commit 700a29e157
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 189 additions and 162 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

110
codex-rs/network-proxy/src/http_proxy.rs
View file

@ -2,6 +2,7 @@ use crate::config::NetworkMode;
use crate::network_policy::NetworkDecision;
use crate::network_policy::NetworkPolicyDecider;
use crate::network_policy::NetworkPolicyRequest;
use crate::network_policy::NetworkPolicyRequestArgs;
use crate::network_policy::NetworkProtocol;
use crate::network_policy::evaluate_host_policy;
use crate::policy::normalize_host;
@ -12,6 +13,7 @@ use crate::responses::blocked_header_value;
use crate::responses::json_response;
use crate::runtime::unix_socket_permissions_supported;
use crate::state::BlockedRequest;
use crate::state::BlockedRequestArgs;
use crate::state::NetworkProxyState;
use crate::upstream::UpstreamClient;
use crate::upstream::proxy_for_connect;
@ -146,27 +148,27 @@ async fn http_connect_accept(
.await);
}
let request = NetworkPolicyRequest::new(
NetworkProtocol::HttpsConnect,
host.clone(),
authority.port,
client.clone(),
Some("CONNECT".to_string()),
None,
None,
);
let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs {
protocol: NetworkProtocol::HttpsConnect,
host: host.clone(),
port: authority.port,
client_addr: client.clone(),
method: Some("CONNECT".to_string()),
command: None,
exec_policy_hint: None,
});
match evaluate_host_policy(&app_state, policy_decider.as_ref(), &request).await {
Ok(NetworkDecision::Deny { reason }) => {
let _ = app_state
.record_blocked(BlockedRequest::new(
host.clone(),
reason.clone(),
client.clone(),
Some("CONNECT".to_string()),
None,
"http-connect".to_string(),
))
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host: host.clone(),
reason: reason.clone(),
client: client.clone(),
method: Some("CONNECT".to_string()),
mode: None,
protocol: "http-connect".to_string(),
}))
.await;
let client = client.as_deref().unwrap_or_default();
warn!("CONNECT blocked (client={client}, host={host}, reason={reason})");
@ -189,14 +191,14 @@ async fn http_connect_accept(
if mode == NetworkMode::Limited {
let _ = app_state
.record_blocked(BlockedRequest::new(
host.clone(),
REASON_METHOD_NOT_ALLOWED.to_string(),
client.clone(),
Some("CONNECT".to_string()),
Some(NetworkMode::Limited),
"http-connect".to_string(),
))
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host: host.clone(),
reason: REASON_METHOD_NOT_ALLOWED.to_string(),
client: client.clone(),
method: Some("CONNECT".to_string()),
mode: Some(NetworkMode::Limited),
protocol: "http-connect".to_string(),
}))
.await;
let client = client.as_deref().unwrap_or_default();
warn!("CONNECT blocked by method policy (client={client}, host={host}, mode=limited)");
@ -425,27 +427,27 @@ async fn http_plain_proxy(
.await);
}
let request = NetworkPolicyRequest::new(
NetworkProtocol::Http,
host.clone(),
let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs {
protocol: NetworkProtocol::Http,
host: host.clone(),
port,
client.clone(),
Some(req.method().as_str().to_string()),
None,
None,
);
client_addr: client.clone(),
method: Some(req.method().as_str().to_string()),
command: None,
exec_policy_hint: None,
});
match evaluate_host_policy(&app_state, policy_decider.as_ref(), &request).await {
Ok(NetworkDecision::Deny { reason }) => {
let _ = app_state
.record_blocked(BlockedRequest::new(
host.clone(),
reason.clone(),
client.clone(),
Some(req.method().as_str().to_string()),
None,
"http".to_string(),
))
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host: host.clone(),
reason: reason.clone(),
client: client.clone(),
method: Some(req.method().as_str().to_string()),
mode: None,
protocol: "http".to_string(),
}))
.await;
let client = client.as_deref().unwrap_or_default();
warn!("request blocked (client={client}, host={host}, reason={reason})");
@ -460,14 +462,14 @@ async fn http_plain_proxy(
if !method_allowed {
let _ = app_state
.record_blocked(BlockedRequest::new(
host.clone(),
REASON_METHOD_NOT_ALLOWED.to_string(),
client.clone(),
Some(req.method().as_str().to_string()),
Some(NetworkMode::Limited),
"http".to_string(),
))
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host: host.clone(),
reason: REASON_METHOD_NOT_ALLOWED.to_string(),
client: client.clone(),
method: Some(req.method().as_str().to_string()),
mode: Some(NetworkMode::Limited),
protocol: "http".to_string(),
}))
.await;
let client = client.as_deref().unwrap_or_default();
let method = req.method();
@ -565,14 +567,14 @@ async fn proxy_disabled_response(
protocol: &str,
) -> Response {
let _ = app_state
.record_blocked(BlockedRequest::new(
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host,
REASON_PROXY_DISABLED.to_string(),
reason: REASON_PROXY_DISABLED.to_string(),
client,
method,
None,
protocol.to_string(),
))
mode: None,
protocol: protocol.to_string(),
}))
.await;
text_response(StatusCode::SERVICE_UNAVAILABLE, "proxy disabled")
}

1
codex-rs/network-proxy/src/lib.rs
View file

@ -17,6 +17,7 @@ use anyhow::Result;
pub use network_policy::NetworkDecision;
pub use network_policy::NetworkPolicyDecider;
pub use network_policy::NetworkPolicyRequest;
pub use network_policy::NetworkPolicyRequestArgs;
pub use network_policy::NetworkProtocol;
pub use proxy::Args;
pub use proxy::NetworkProxy;

83
codex-rs/network-proxy/src/network_policy.rs
View file

@ -26,16 +26,27 @@ pub struct NetworkPolicyRequest {
pub exec_policy_hint: Option<String>,
}
pub struct NetworkPolicyRequestArgs {
pub protocol: NetworkProtocol,
pub host: String,
pub port: u16,
pub client_addr: Option<String>,
pub method: Option<String>,
pub command: Option<String>,
pub exec_policy_hint: Option<String>,
}
impl NetworkPolicyRequest {
pub fn new(
protocol: NetworkProtocol,
host: String,
port: u16,
client_addr: Option<String>,
method: Option<String>,
command: Option<String>,
exec_policy_hint: Option<String>,
) -> Self {
pub fn new(args: NetworkPolicyRequestArgs) -> Self {
let NetworkPolicyRequestArgs {
protocol,
host,
port,
client_addr,
method,
command,
exec_policy_hint,
} = args;
Self {
protocol,
host,
@ -139,15 +150,15 @@ mod tests {
}
});
let request = NetworkPolicyRequest::new(
NetworkProtocol::Http,
"example.com".to_string(),
80,
None,
Some("GET".to_string()),
None,
None,
);
let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs {
protocol: NetworkProtocol::Http,
host: "example.com".to_string(),
port: 80,
client_addr: None,
method: Some("GET".to_string()),
command: None,
exec_policy_hint: None,
});
let decision = evaluate_host_policy(&state, Some(&decider), &request)
.await
@ -172,15 +183,15 @@ mod tests {
}
});
let request = NetworkPolicyRequest::new(
NetworkProtocol::Http,
"blocked.com".to_string(),
80,
None,
Some("GET".to_string()),
None,
None,
);
let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs {
protocol: NetworkProtocol::Http,
host: "blocked.com".to_string(),
port: 80,
client_addr: None,
method: Some("GET".to_string()),
command: None,
exec_policy_hint: None,
});
let decision = evaluate_host_policy(&state, Some(&decider), &request)
.await
@ -210,15 +221,15 @@ mod tests {
}
});
let request = NetworkPolicyRequest::new(
NetworkProtocol::Http,
"127.0.0.1".to_string(),
80,
None,
Some("GET".to_string()),
None,
None,
);
let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs {
protocol: NetworkProtocol::Http,
host: "127.0.0.1".to_string(),
port: 80,
client_addr: None,
method: Some("GET".to_string()),
command: None,
exec_policy_hint: None,
});
let decision = evaluate_host_policy(&state, Some(&decider), &request)
.await

26
codex-rs/network-proxy/src/runtime.rs
View file

@ -73,15 +73,25 @@ pub struct BlockedRequest {
pub timestamp: i64,
}
pub struct BlockedRequestArgs {
pub host: String,
pub reason: String,
pub client: Option<String>,
pub method: Option<String>,
pub mode: Option<NetworkMode>,
pub protocol: String,
}
impl BlockedRequest {
pub fn new(
host: String,
reason: String,
client: Option<String>,
method: Option<String>,
mode: Option<NetworkMode>,
protocol: String,
) -> Self {
pub fn new(args: BlockedRequestArgs) -> Self {
let BlockedRequestArgs {
host,
reason,
client,
method,
mode,
protocol,
} = args;
Self {
host,
reason,

130
codex-rs/network-proxy/src/socks5.rs
View file

@ -2,12 +2,14 @@ use crate::config::NetworkMode;
use crate::network_policy::NetworkDecision;
use crate::network_policy::NetworkPolicyDecider;
use crate::network_policy::NetworkPolicyRequest;
use crate::network_policy::NetworkPolicyRequestArgs;
use crate::network_policy::NetworkProtocol;
use crate::network_policy::evaluate_host_policy;
use crate::policy::normalize_host;
use crate::reasons::REASON_METHOD_NOT_ALLOWED;
use crate::reasons::REASON_PROXY_DISABLED;
use crate::state::BlockedRequest;
use crate::state::BlockedRequestArgs;
use crate::state::NetworkProxyState;
use anyhow::Context as _;
use anyhow::Result;
@ -122,14 +124,14 @@ async fn handle_socks5_tcp(
Ok(true) => {}
Ok(false) => {
let _ = app_state
.record_blocked(BlockedRequest::new(
host.clone(),
REASON_PROXY_DISABLED.to_string(),
client.clone(),
None,
None,
"socks5".to_string(),
))
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host: host.clone(),
reason: REASON_PROXY_DISABLED.to_string(),
client: client.clone(),
method: None,
mode: None,
protocol: "socks5".to_string(),
}))
.await;
let client = client.as_deref().unwrap_or_default();
warn!("SOCKS blocked; proxy disabled (client={client}, host={host})");
@ -144,14 +146,14 @@ async fn handle_socks5_tcp(
match app_state.network_mode().await {
Ok(NetworkMode::Limited) => {
let _ = app_state
.record_blocked(BlockedRequest::new(
host.clone(),
REASON_METHOD_NOT_ALLOWED.to_string(),
client.clone(),
None,
Some(NetworkMode::Limited),
"socks5".to_string(),
))
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host: host.clone(),
reason: REASON_METHOD_NOT_ALLOWED.to_string(),
client: client.clone(),
method: None,
mode: Some(NetworkMode::Limited),
protocol: "socks5".to_string(),
}))
.await;
let client = client.as_deref().unwrap_or_default();
warn!(
@ -166,27 +168,27 @@ async fn handle_socks5_tcp(
}
}
let request = NetworkPolicyRequest::new(
NetworkProtocol::Socks5Tcp,
host.clone(),
let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs {
protocol: NetworkProtocol::Socks5Tcp,
host: host.clone(),
port,
client.clone(),
None,
None,
None,
);
client_addr: client.clone(),
method: None,
command: None,
exec_policy_hint: None,
});
match evaluate_host_policy(&app_state, policy_decider.as_ref(), &request).await {
Ok(NetworkDecision::Deny { reason }) => {
let _ = app_state
.record_blocked(BlockedRequest::new(
host.clone(),
reason.clone(),
client.clone(),
None,
None,
"socks5".to_string(),
))
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host: host.clone(),
reason: reason.clone(),
client: client.clone(),
method: None,
mode: None,
protocol: "socks5".to_string(),
}))
.await;
let client = client.as_deref().unwrap_or_default();
warn!("SOCKS blocked (client={client}, host={host}, reason={reason})");
@ -231,14 +233,14 @@ async fn inspect_socks5_udp(
Ok(true) => {}
Ok(false) => {
let _ = state
.record_blocked(BlockedRequest::new(
host.clone(),
REASON_PROXY_DISABLED.to_string(),
client.clone(),
None,
None,
"socks5-udp".to_string(),
))
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host: host.clone(),
reason: REASON_PROXY_DISABLED.to_string(),
client: client.clone(),
method: None,
mode: None,
protocol: "socks5-udp".to_string(),
}))
.await;
let client = client.as_deref().unwrap_or_default();
warn!("SOCKS UDP blocked; proxy disabled (client={client}, host={host})");
@ -256,14 +258,14 @@ async fn inspect_socks5_udp(
match state.network_mode().await {
Ok(NetworkMode::Limited) => {
let _ = state
.record_blocked(BlockedRequest::new(
host.clone(),
REASON_METHOD_NOT_ALLOWED.to_string(),
client.clone(),
None,
Some(NetworkMode::Limited),
"socks5-udp".to_string(),
))
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host: host.clone(),
reason: REASON_METHOD_NOT_ALLOWED.to_string(),
client: client.clone(),
method: None,
mode: Some(NetworkMode::Limited),
protocol: "socks5-udp".to_string(),
}))
.await;
return Ok(RelayResponse {
maybe_payload: None,
@ -277,27 +279,27 @@ async fn inspect_socks5_udp(
}
}
let request = NetworkPolicyRequest::new(
NetworkProtocol::Socks5Udp,
host.clone(),
let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs {
protocol: NetworkProtocol::Socks5Udp,
host: host.clone(),
port,
client.clone(),
None,
None,
None,
);
client_addr: client.clone(),
method: None,
command: None,
exec_policy_hint: None,
});
match evaluate_host_policy(&state, policy_decider.as_ref(), &request).await {
Ok(NetworkDecision::Deny { reason }) => {
let _ = state
.record_blocked(BlockedRequest::new(
host.clone(),
reason.clone(),
client.clone(),
None,
None,
"socks5-udp".to_string(),
))
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host: host.clone(),
reason: reason.clone(),
client: client.clone(),
method: None,
mode: None,
protocol: "socks5-udp".to_string(),
}))
.await;
let client = client.as_deref().unwrap_or_default();
warn!("SOCKS UDP blocked (client={client}, host={host}, reason={reason})");

1
codex-rs/network-proxy/src/state.rs
View file

@ -20,6 +20,7 @@ use serde::Deserialize;
use std::collections::HashSet;
pub use crate::runtime::BlockedRequest;
pub use crate::runtime::BlockedRequestArgs;
pub use crate::runtime::NetworkProxyState;
#[cfg(test)]
pub(crate) use crate::runtime::network_proxy_state_for_policy;