Skip to content

Commit 2bced81

Browse files
feat(network-proxy): structured policy signaling and attempt correlation to core (openai#11662)
## Summary When network requests were blocked, downstream code often had to infer ask vs deny from free-form response text. That was brittle and led to incorrect approval behavior. This PR fixes the proxy side so blocked decisions are structured and request metadata survives reliably. ## Description - Blocked proxy responses now carry consistent structured policy decision data. - Request attempt metadata is preserved across proxy env paths (including ALL_PROXY flows). - Header stripping was tightened so we still remove unsafe forwarding headers, but keep metadata needed for policy handling. - Block messages were clarified (for example, allowlist miss vs explicit deny). - Added unified violation log entries so policy failures can be inspected in one place. - Added/updated tests for these behaviors. --------- Co-authored-by: Codex <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
1 parent fca5629 commit 2bced81

File tree

14 files changed

+581
-83
lines changed

14 files changed

+581
-83
lines changed

.codespellignore

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
iTerm
22
iTerm2
3-
psuedo
3+
psuedo
4+
te
5+
TE

.codespellrc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@
33
skip = .git*,vendor,*-lock.yaml,*.lock,.codespellrc,*test.ts,*.jsonl,frame*.txt,*.snap,*.snap.new,*meriyah.umd.min.js
44
check-hidden = true
55
ignore-regex = ^\s*"image/\S+": ".*|\b(afterAll)\b
6-
ignore-words-list = ratatui,ser,iTerm,iterm2,iterm
6+
ignore-words-list = ratatui,ser,iTerm,iterm2,iterm,te,TE

codex-rs/Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

codex-rs/network-proxy/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ workspace = true
1414
[dependencies]
1515
anyhow = { workspace = true }
1616
async-trait = { workspace = true }
17+
base64 = { workspace = true }
1718
clap = { workspace = true, features = ["derive"] }
1819
codex-utils-absolute-path = { workspace = true }
1920
codex-utils-rustls-provider = { workspace = true }

codex-rs/network-proxy/src/admin.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ async fn handle_admin_request(
8989
text_response(StatusCode::INTERNAL_SERVER_ERROR, "error")
9090
}
9191
},
92-
("GET", "/blocked") => match state.drain_blocked().await {
92+
("GET", "/blocked") => match state.blocked_snapshot().await {
9393
Ok(blocked) => json_response(&BlockedResponse { blocked }),
9494
Err(err) => {
9595
error!("failed to read blocked queue: {err}");

codex-rs/network-proxy/src/http_proxy.rs

Lines changed: 155 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::config::NetworkMode;
2+
use crate::metadata::attempt_id_from_proxy_authorization;
23
use crate::network_policy::NetworkDecision;
34
use crate::network_policy::NetworkDecisionSource;
45
use crate::network_policy::NetworkPolicyDecider;
@@ -16,7 +17,6 @@ use crate::responses::blocked_header_value;
1617
use crate::responses::blocked_message_with_policy;
1718
use crate::responses::blocked_text_response_with_policy;
1819
use crate::responses::json_response;
19-
use crate::responses::policy_decision_prefix;
2020
use crate::runtime::unix_socket_permissions_supported;
2121
use crate::state::BlockedRequest;
2222
use crate::state::BlockedRequestArgs;
@@ -36,11 +36,13 @@ use rama_core::layer::AddInputExtensionLayer;
3636
use rama_core::rt::Executor;
3737
use rama_core::service::service_fn;
3838
use rama_http::Body;
39+
use rama_http::HeaderMap;
40+
use rama_http::HeaderName;
3941
use rama_http::HeaderValue;
4042
use rama_http::Request;
4143
use rama_http::Response;
4244
use rama_http::StatusCode;
43-
use rama_http::layer::remove_header::RemoveRequestHeaderLayer;
45+
use rama_http::header;
4446
use rama_http::layer::remove_header::RemoveResponseHeaderLayer;
4547
use rama_http::matcher::MethodMatcher;
4648
use rama_http_backend::client::proxy::layer::HttpProxyConnector;
@@ -119,7 +121,6 @@ async fn run_http_proxy_with_listener(
119121
service_fn(http_connect_proxy),
120122
),
121123
RemoveResponseHeaderLayer::hop_by_hop(),
122-
RemoveRequestHeaderLayer::hop_by_hop(),
123124
)
124125
.into_layer(service_fn({
125126
let policy_decider = policy_decider.clone();
@@ -159,6 +160,7 @@ async fn http_connect_accept(
159160
}
160161

161162
let client = client_addr(&req);
163+
let network_attempt_id = request_network_attempt_id(&req);
162164

163165
let enabled = app_state
164166
.enabled()
@@ -186,6 +188,7 @@ async fn http_connect_accept(
186188
method: Some("CONNECT".to_string()),
187189
command: None,
188190
exec_policy_hint: None,
191+
attempt_id: network_attempt_id.clone(),
189192
});
190193

191194
match evaluate_host_policy(&app_state, policy_decider.as_ref(), &request).await {
@@ -210,6 +213,10 @@ async fn http_connect_accept(
210213
method: Some("CONNECT".to_string()),
211214
mode: None,
212215
protocol: "http-connect".to_string(),
216+
attempt_id: network_attempt_id.clone(),
217+
decision: Some(details.decision.as_str().to_string()),
218+
source: Some(details.source.as_str().to_string()),
219+
port: Some(authority.port),
213220
}))
214221
.await;
215222
let client = client.as_deref().unwrap_or_default();
@@ -248,6 +255,10 @@ async fn http_connect_accept(
248255
method: Some("CONNECT".to_string()),
249256
mode: Some(NetworkMode::Limited),
250257
protocol: "http-connect".to_string(),
258+
attempt_id: network_attempt_id,
259+
decision: Some(details.decision.as_str().to_string()),
260+
source: Some(details.source.as_str().to_string()),
261+
port: Some(authority.port),
251262
}))
252263
.await;
253264
let client = client.as_deref().unwrap_or_default();
@@ -353,7 +364,7 @@ async fn forward_connect_tunnel(
353364

354365
async fn http_plain_proxy(
355366
policy_decider: Option<Arc<dyn NetworkPolicyDecider>>,
356-
req: Request,
367+
mut req: Request,
357368
) -> Result<Response, Infallible> {
358369
let app_state = match req.extensions().get::<Arc<NetworkProxyState>>().cloned() {
359370
Some(state) => state,
@@ -363,6 +374,7 @@ async fn http_plain_proxy(
363374
}
364375
};
365376
let client = client_addr(&req);
377+
let network_attempt_id = request_network_attempt_id(&req);
366378

367379
let method_allowed = match app_state
368380
.method_allowed(req.method().as_str())
@@ -492,6 +504,7 @@ async fn http_plain_proxy(
492504
method: Some(req.method().as_str().to_string()),
493505
command: None,
494506
exec_policy_hint: None,
507+
attempt_id: network_attempt_id.clone(),
495508
});
496509

497510
match evaluate_host_policy(&app_state, policy_decider.as_ref(), &request).await {
@@ -516,6 +529,10 @@ async fn http_plain_proxy(
516529
method: Some(req.method().as_str().to_string()),
517530
mode: None,
518531
protocol: "http".to_string(),
532+
attempt_id: network_attempt_id.clone(),
533+
decision: Some(details.decision.as_str().to_string()),
534+
source: Some(details.source.as_str().to_string()),
535+
port: Some(port),
519536
}))
520537
.await;
521538
let client = client.as_deref().unwrap_or_default();
@@ -546,6 +563,10 @@ async fn http_plain_proxy(
546563
method: Some(req.method().as_str().to_string()),
547564
mode: Some(NetworkMode::Limited),
548565
protocol: "http".to_string(),
566+
attempt_id: network_attempt_id,
567+
decision: Some(details.decision.as_str().to_string()),
568+
source: Some(details.source.as_str().to_string()),
569+
port: Some(port),
549570
}))
550571
.await;
551572
let client = client.as_deref().unwrap_or_default();
@@ -578,6 +599,8 @@ async fn http_plain_proxy(
578599
UpstreamClient::direct()
579600
};
580601

602+
// Strip hop-by-hop headers only after extracting metadata used for policy correlation.
603+
remove_hop_by_hop_request_headers(req.headers_mut());
581604
match client.serve(req).await {
582605
Ok(resp) => Ok(resp),
583606
Err(err) => {
@@ -602,6 +625,7 @@ async fn proxy_via_unix_socket(req: Request, socket_path: &str) -> Result<Respon
602625
.parse()
603626
.with_context(|| format!("invalid unix socket request path: {path}"))?;
604627
parts.headers.remove("x-unix-socket");
628+
remove_hop_by_hop_request_headers(&mut parts.headers);
605629

606630
let req = Request::from_parts(parts, body);
607631
client.serve(req).await.map_err(anyhow::Error::from)
@@ -621,20 +645,67 @@ fn client_addr<T: ExtensionsRef>(input: &T) -> Option<String> {
621645
.map(|info| info.peer_addr().to_string())
622646
}
623647

648+
fn request_network_attempt_id(req: &Request) -> Option<String> {
649+
// Some HTTP stacks normalize proxy credentials into `authorization`; accept both.
650+
attempt_id_from_proxy_authorization(req.headers().get("proxy-authorization"))
651+
.or_else(|| attempt_id_from_proxy_authorization(req.headers().get("authorization")))
652+
}
653+
654+
fn remove_hop_by_hop_request_headers(headers: &mut HeaderMap) {
655+
while let Some(raw_connection) = headers.get(header::CONNECTION).cloned() {
656+
headers.remove(header::CONNECTION);
657+
if let Ok(raw_connection) = raw_connection.to_str() {
658+
let connection_headers: Vec<String> = raw_connection
659+
.split(',')
660+
.map(str::trim)
661+
.filter(|token| !token.is_empty())
662+
.map(ToOwned::to_owned)
663+
.collect();
664+
for token in connection_headers {
665+
if let Ok(name) = HeaderName::from_bytes(token.as_bytes()) {
666+
headers.remove(name);
667+
}
668+
}
669+
}
670+
}
671+
for name in [
672+
&header::KEEP_ALIVE,
673+
&header::PROXY_CONNECTION,
674+
&header::PROXY_AUTHORIZATION,
675+
&header::TRAILER,
676+
&header::TRANSFER_ENCODING,
677+
&header::UPGRADE,
678+
] {
679+
headers.remove(name);
680+
}
681+
682+
// codespell:ignore te,TE
683+
// 0x74,0x65 is ASCII "te" (the HTTP TE hop-by-hop header).
684+
if let Ok(short_hop_header_name) = HeaderName::from_bytes(&[0x74, 0x65]) {
685+
headers.remove(short_hop_header_name);
686+
}
687+
}
688+
624689
fn json_blocked(host: &str, reason: &str, details: Option<&PolicyDecisionDetails<'_>>) -> Response {
625-
let (policy_decision_prefix, message) = details
690+
let (message, decision, source, protocol, port) = details
626691
.map(|details| {
627692
(
628-
Some(policy_decision_prefix(details)),
629693
Some(blocked_message_with_policy(reason, details)),
694+
Some(details.decision.as_str()),
695+
Some(details.source.as_str()),
696+
Some(details.protocol.as_policy_protocol()),
697+
Some(details.port),
630698
)
631699
})
632-
.unwrap_or((None, None));
700+
.unwrap_or((None, None, None, None, None));
633701
let response = BlockedResponse {
634702
status: "blocked",
635703
host,
636704
reason,
637-
policy_decision_prefix,
705+
decision,
706+
source,
707+
protocol,
708+
port,
638709
message,
639710
};
640711
let mut resp = json_response(&response);
@@ -667,6 +738,10 @@ async fn proxy_disabled_response(
667738
method,
668739
mode: None,
669740
protocol: protocol.as_policy_protocol().to_string(),
741+
attempt_id: None,
742+
decision: Some("deny".to_string()),
743+
source: Some("proxy_state".to_string()),
744+
port: Some(port),
670745
}))
671746
.await;
672747

@@ -703,7 +778,13 @@ struct BlockedResponse<'a> {
703778
host: &'a str,
704779
reason: &'a str,
705780
#[serde(skip_serializing_if = "Option::is_none")]
706-
policy_decision_prefix: Option<String>,
781+
decision: Option<&'static str>,
782+
#[serde(skip_serializing_if = "Option::is_none")]
783+
source: Option<&'static str>,
784+
#[serde(skip_serializing_if = "Option::is_none")]
785+
protocol: Option<&'static str>,
786+
#[serde(skip_serializing_if = "Option::is_none")]
787+
port: Option<u16>,
707788
#[serde(skip_serializing_if = "Option::is_none")]
708789
message: Option<String>,
709790
}
@@ -715,6 +796,8 @@ mod tests {
715796
use crate::config::NetworkMode;
716797
use crate::config::NetworkProxySettings;
717798
use crate::runtime::network_proxy_state_for_policy;
799+
use base64::Engine;
800+
use base64::engine::general_purpose::STANDARD;
718801
use pretty_assertions::assert_eq;
719802
use rama_http::Method;
720803
use rama_http::Request;
@@ -744,4 +827,67 @@ mod tests {
744827
"blocked-by-method-policy"
745828
);
746829
}
830+
831+
#[test]
832+
fn request_network_attempt_id_reads_proxy_authorization_header() {
833+
let encoded = STANDARD.encode("codex-net-attempt-attempt-1:");
834+
let req = Request::builder()
835+
.method(Method::GET)
836+
.uri("http://example.com")
837+
.header("proxy-authorization", format!("Basic {encoded}"))
838+
.body(Body::empty())
839+
.unwrap();
840+
assert_eq!(
841+
request_network_attempt_id(&req),
842+
Some("attempt-1".to_string())
843+
);
844+
}
845+
846+
#[test]
847+
fn request_network_attempt_id_reads_authorization_header_fallback() {
848+
let encoded = STANDARD.encode("codex-net-attempt-attempt-2:");
849+
let req = Request::builder()
850+
.method(Method::GET)
851+
.uri("http://example.com")
852+
.header("authorization", format!("Basic {encoded}"))
853+
.body(Body::empty())
854+
.unwrap();
855+
assert_eq!(
856+
request_network_attempt_id(&req),
857+
Some("attempt-2".to_string())
858+
);
859+
}
860+
861+
#[test]
862+
fn remove_hop_by_hop_request_headers_keeps_forwarding_headers() {
863+
let mut headers = HeaderMap::new();
864+
headers.insert(
865+
header::CONNECTION,
866+
HeaderValue::from_static("x-hop, keep-alive"),
867+
);
868+
headers.insert("x-hop", HeaderValue::from_static("1"));
869+
headers.insert(
870+
header::PROXY_AUTHORIZATION,
871+
HeaderValue::from_static("Basic abc"),
872+
);
873+
headers.insert(
874+
&header::X_FORWARDED_FOR,
875+
HeaderValue::from_static("127.0.0.1"),
876+
);
877+
headers.insert(header::HOST, HeaderValue::from_static("example.com"));
878+
879+
remove_hop_by_hop_request_headers(&mut headers);
880+
881+
assert_eq!(headers.get(header::CONNECTION), None);
882+
assert_eq!(headers.get("x-hop"), None);
883+
assert_eq!(headers.get(header::PROXY_AUTHORIZATION), None);
884+
assert_eq!(
885+
headers.get(&header::X_FORWARDED_FOR),
886+
Some(&HeaderValue::from_static("127.0.0.1"))
887+
);
888+
assert_eq!(
889+
headers.get(header::HOST),
890+
Some(&HeaderValue::from_static("example.com"))
891+
);
892+
}
747893
}

codex-rs/network-proxy/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
mod admin;
44
mod config;
55
mod http_proxy;
6+
mod metadata;
67
mod network_policy;
78
mod policy;
89
mod proxy;
@@ -32,6 +33,7 @@ pub use proxy::NetworkProxyHandle;
3233
pub use proxy::PROXY_URL_ENV_KEYS;
3334
pub use proxy::has_proxy_url_env_vars;
3435
pub use proxy::proxy_url_env_value;
36+
pub use runtime::BlockedRequest;
3537
pub use runtime::ConfigReloader;
3638
pub use runtime::ConfigState;
3739
pub use runtime::NetworkProxyState;

0 commit comments

Comments
 (0)