Harden network proxy MITM authorization (#37211)

## Why

MITM hooks authorize requests before the upstream server parses them. Paths that
can be decoded or normalized to a different resource must not match an allowed
path, and hosts that require MITM inspection must not bypass it through the
plain HTTP proxy path.

## What changed

- Reject ambiguous hook paths, including traversal segments, backslashes,
  malformed percent encodings, and encoded separators or percent signs.
- Block plain HTTP proxy requests for hosts whose policy always requires MITM,
  recording the decision as `mitm_required`.

## Testing

- Cover safe and ambiguous path forms, encoded traversal through repository
  allowlists, and absolute-form HTTPS requests sent to the HTTP proxy.

GitOrigin-RevId: 8812a980ac64a97cbac3a237376d29be5ded9220

Author: kevinlin-openai

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

@@ -0,0 +1,64 @@
+/// Returns whether `path` has an unambiguous interpretation for authorization.
+///
+/// MITM hooks authorize the request before the upstream server parses it. Reject
+/// path forms that common upstreams may decode or normalize into a different
+/// resource after a hook has matched.
+pub(crate) fn is_safe_for_authorization(path: &str) -> bool {
+    path.split('/').all(is_safe_segment_for_authorization)
+}
+
+fn is_safe_segment_for_authorization(segment: &str) -> bool {
+    let bytes = segment.as_bytes();
+    let mut index = 0;
+    let mut decoded_dots = 0;
+    let mut has_non_dot = false;
+    while index < bytes.len() {
+        match bytes[index] {
+            b'.' => {
+                decoded_dots += 1;
+                index += 1;
+            }
+            b'\\' => return false,
+            b'%' => {
+                let Some(high) = bytes
+                    .get(index + 1)
+                    .and_then(|byte| decode_hex_digit(*byte))
+                else {
+                    return false;
+                };
+                let Some(low) = bytes
+                    .get(index + 2)
+                    .and_then(|byte| decode_hex_digit(*byte))
+                else {
+                    return false;
+                };
+                let decoded = high << 4 | low;
+                match decoded {
+                    b'%' | b'/' | b'\\' => return false,
+                    b'.' => decoded_dots += 1,
+                    _ => has_non_dot = true,
+                }
+                index += 3;
+            }
+            _ => {
+                has_non_dot = true;
+                index += 1;
+            }
+        }
+    }
+
+    has_non_dot || !matches!(decoded_dots, 1 | 2)
+}
+
+fn decode_hex_digit(byte: u8) -> Option<u8> {
+    match byte {
+        b'0'..=b'9' => Some(byte - b'0'),
+        b'a'..=b'f' => Some(byte - b'a' + 10),
+        b'A'..=b'F' => Some(byte - b'A' + 10),
+        _ => None,
+    }
+}
+
+#[cfg(test)]
+#[path = "authorization_path_tests.rs"]
+mod tests;

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

@@ -0,0 +1,46 @@
+use super::is_safe_for_authorization;
+use pretty_assertions::assert_eq;
+
+#[test]
+fn accepts_unambiguous_paths() {
+    let paths = [
+        "/openai/openai",
+        "/openai/openai/issues/123",
+        "/openai/openai/a..b",
+        "/openai/openai/%20space",
+        "/openai/openai/%E2%9C%93",
+        "/openai/openai/contents/%2Egitignore",
+        "/openai/openai/contents/.%2Egithub",
+        "/openai/openai/%2e%2efoo",
+        "/openai/openai/%2e%2e%2e",
+    ];
+
+    assert_eq!(
+        paths.map(is_safe_for_authorization),
+        [true, true, true, true, true, true, true, true, true]
+    );
+}
+
+#[test]
+fn rejects_paths_with_ambiguous_segments_or_encodings() {
+    let paths = [
+        "/openai/openai/../codex",
+        "/openai/openai/./issues",
+        "/openai/openai\\..\\codex",
+        "/openai/openai/%2e%2e/codex",
+        "/openai/openai/%2E%2E/codex",
+        "/openai/openai/%2f..%2fcodex",
+        "/openai/openai/%5c..%5ccodex",
+        "/openai/openai/%252e%252e/codex",
+        "/openai/openai/%",
+        "/openai/openai/%2",
+        "/openai/openai/%zz",
+    ];
+
+    assert_eq!(
+        paths.map(is_safe_for_authorization),
+        [
+            false, false, false, false, false, false, false, false, false, false, false
+        ]
+    );
+}

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

@@ -758,6 +758,54 @@ async fn http_plain_proxy(
         }
     }
 
+    let host_mitm_requirement = match app_state.host_mitm_requirement(&host).await {
+        Ok(requirement) => requirement,
+        Err(err) => {
+            return Ok(internal_error("failed to inspect MITM requirements", err));
+        }
+    };
+    if host_mitm_requirement == HostMitmRequirement::Always {
+        emit_http_block_decision_audit_event(
+            &app_state,
+            BlockDecisionAuditEventArgs {
+                source: NetworkDecisionSource::ModeGuard,
+                reason: REASON_MITM_REQUIRED,
+                protocol: NetworkProtocol::Http,
+                server_address: host.as_str(),
+                server_port: port,
+                method: Some(req.method().as_str()),
+                client_addr: client.as_deref(),
+            },
+        );
+        let details = PolicyDecisionDetails {
+            decision: NetworkPolicyDecision::Deny,
+            reason: REASON_MITM_REQUIRED,
+            source: NetworkDecisionSource::ModeGuard,
+            protocol: NetworkProtocol::Http,
+            host: &host,
+            port,
+        };
+        let _ = app_state
+            .record_blocked(BlockedRequest::new(BlockedRequestArgs {
+                host: host.clone(),
+                reason: REASON_MITM_REQUIRED.to_string(),
+                client: client.clone(),
+                method: Some(req.method().as_str().to_string()),
+                mode: None,
+                protocol: "http".to_string(),
+                decision: Some(details.decision.as_str().to_string()),
+                source: Some(details.source.as_str().to_string()),
+                port: Some(port),
+            }))
+            .await;
+        let client = client.as_deref().unwrap_or_default();
+        warn!(
+            "request blocked; MITM required to enforce host policy (client={client}, host={host}, method={})",
+            req.method()
+        );
+        return Ok(json_blocked(&host, REASON_MITM_REQUIRED, Some(&details)));
+    }
+
     if !method_allowed {
         emit_http_block_decision_audit_event(
             &app_state,
@@ -1418,6 +1466,87 @@ mod tests {
         target_task.await.expect("target task should finish");
     }
 
+    #[tokio::test]
+    async fn http_proxy_blocks_absolute_form_https_for_hooked_host() {
+        let target_listener = TokioTcpListener::bind((Ipv4Addr::LOCALHOST, 0))
+            .await
+            .expect("target listener should bind");
+        let target_addr = target_listener
+            .local_addr()
+            .expect("target listener should expose local addr");
+        let target_task = tokio::spawn(async move {
+            timeout(Duration::from_secs(1), target_listener.accept())
+                .await
+                .is_ok()
+        });
+
+        let state = Arc::new(network_proxy_state_for_policy({
+            let mut network = NetworkProxyConfig {
+                allow_local_binding: true,
+                mitm: true,
+                mitm_hooks: vec![crate::mitm_hook::MitmHookConfig {
+                    host: "127.0.0.1".to_string(),
+                    matcher: crate::mitm_hook::MitmHookMatchConfig {
+                        methods: vec!["GET".to_string()],
+                        path_prefixes: vec!["/repos/openai/ALLOWED".to_string()],
+                        ..crate::mitm_hook::MitmHookMatchConfig::default()
+                    },
+                    actions: crate::mitm_hook::MitmHookActionsConfig::default(),
+                }],
+                ..NetworkProxyConfig::default()
+            };
+            network.set_allowed_domains(vec!["127.0.0.1".to_string()]);
+            network
+        }));
+        let listener =
+            StdTcpListener::bind((Ipv4Addr::LOCALHOST, 0)).expect("proxy listener should bind");
+        let proxy_addr = listener
+            .local_addr()
+            .expect("proxy listener should expose local addr");
+        let proxy_task = tokio::spawn(run_http_proxy_with_std_listener(
+            state.clone(),
+            listener,
+            /*policy_decider*/ None,
+            /*environment_id*/ None,
+        ));
+
+        let mut stream = tokio::net::TcpStream::connect(proxy_addr)
+            .await
+            .expect("client should connect to proxy");
+        let request = format!(
+            "GET https://127.0.0.1:{port}/repos/openai/UNAUTHORIZED HTTP/1.1\r\nHost: 127.0.0.1:{port}\r\nConnection: close\r\n\r\n",
+            port = target_addr.port()
+        );
+        stream
+            .write_all(request.as_bytes())
+            .await
+            .expect("client should write absolute-form HTTPS request");
+
+        let mut buf = [0_u8; 512];
+        let bytes_read = timeout(Duration::from_secs(2), stream.read(&mut buf))
+            .await
+            .expect("proxy should respond before timeout")
+            .expect("client should read proxy response");
+        let response = String::from_utf8_lossy(&buf[..bytes_read]);
+        assert!(
+            response.starts_with("HTTP/1.1 403 Forbidden\r\n"),
+            "unexpected proxy response: {response:?}"
+        );
+        assert!(response.contains("x-proxy-error: blocked-by-mitm-required\r\n"));
+        assert!(
+            !target_task.await.expect("target task should finish"),
+            "blocked request must not reach upstream"
+        );
+
+        let blocked = state.drain_blocked().await.unwrap();
+        assert_eq!(blocked.len(), 1);
+        assert_eq!(blocked[0].reason, REASON_MITM_REQUIRED);
+
+        drop(stream);
+        proxy_task.abort();
+        let _ = proxy_task.await;
+    }
+
     #[tokio::test(flavor = "current_thread")]
     async fn http_plain_proxy_blocks_unix_socket_when_method_not_allowed() {
         let state = Arc::new(network_proxy_state_for_policy(NetworkProxyConfig::default()));

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

@@ -1,6 +1,7 @@
 #![deny(clippy::print_stdout, clippy::print_stderr)]
 
 mod attribution;
+mod authorization_path;
 mod certs;
 mod config;
 mod connect_policy;

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

@@ -1,5 +1,6 @@
 #![cfg_attr(not(test), allow(dead_code))]
 
+use crate::authorization_path::is_safe_for_authorization;
 use crate::config::NetworkProxyConfig;
 use crate::policy::normalize_host;
 use anyhow::Context as _;
@@ -392,7 +393,7 @@ fn hook_matches(hook: &MitmHook, req: &Request) -> bool {
     }
 
     let path = req.uri().path();
-    if !path_matches(&hook.matcher.path_prefixes, path) {
+    if !is_safe_for_authorization(path) || !path_matches(&hook.matcher.path_prefixes, path) {
         return false;
     }
 
@@ -918,6 +919,46 @@ mod tests {
         );
     }
 
+    #[test]
+    fn evaluate_rejects_paths_that_upstream_may_normalize() {
+        let mut config = base_config();
+        let mut hook = github_hook();
+        hook.matcher.methods = vec!["GET".to_string()];
+        hook.matcher.path_prefixes = vec!["pattern:/openai/openai/**".to_string()];
+        config.mitm_hooks = vec![hook];
+
+        let hooks = compile_mitm_hooks_with_resolvers(
+            &config,
+            |_| Some("abc".to_string()),
+            |_| Err(anyhow!("unexpected file lookup")),
+        )
+        .unwrap();
+        let paths = [
+            "/openai/openai/../codex",
+            "/openai/openai/%2e%2e/codex",
+            "/openai/openai/%2E%2E/codex",
+            "/openai/openai/.%2e/codex",
+            "/openai/openai/%2e./codex",
+            "/openai/openai/%252e%252e/codex",
+            "/openai/openai/%2f..%2fcodex",
+            "/openai/openai/%5c..%5ccodex",
+            "/openai/openai/%2e%2e/%2e%2e/microsoft/vscode",
+        ];
+        let actual = paths
+            .iter()
+            .map(|path| {
+                let req = Request::builder()
+                    .method(Method::GET)
+                    .uri(*path)
+                    .body(Body::empty())
+                    .unwrap();
+                evaluate_mitm_hooks(&hooks, "api.github.com", &req)
+            })
+            .collect::<Vec<_>>();
+
+        assert_eq!(actual, vec![HookEvaluation::HookedHostNoMatch; paths.len()]);
+    }
+
     #[test]
     fn evaluate_treats_glob_metacharacters_as_literal_without_glob_prefix() {
         let mut config = base_config();