mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Route exec-server HTTP through configured proxy policy (#35023)
## Why Delegated HTTP requests need to honor the same outbound proxy policy as the Codex process that starts the exec server. ## What changed - Pass the configured `HttpClientFactory` through local and remote exec-server startup and use route-aware client pools for delegated HTTP and local MCP requests. - Preserve per-request timeouts and follow-or-stop redirect behavior while keeping request URLs and sensitive response headers out of diagnostics. ## Testing - Cover configured system-proxy routing across the exec-server transport. - Cover both redirect policies and verify that success and failure logs do not expose request or response secrets. GitOrigin-RevId: 4af6aec1d265c4db62dfcb6e1fb076fb31736137
This commit is contained in:
@@ -9,6 +9,8 @@ use std::time::Duration;
|
||||
use codex_exec_server::CODEX_ARG0_EXEC_HELPER_ARG1;
|
||||
use codex_exec_server::CODEX_FS_HELPER_ARG1;
|
||||
use codex_exec_server::ExecServerRuntimePaths;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_http_client::OutboundProxyPolicy;
|
||||
use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0;
|
||||
use codex_test_binary_support::TestBinaryDispatchGuard;
|
||||
use codex_test_binary_support::TestBinaryDispatchMode;
|
||||
@@ -19,6 +21,9 @@ pub(crate) mod exec_server;
|
||||
|
||||
pub(crate) const DELAYED_OUTPUT_AFTER_EXIT_PARENT_ARG: &str =
|
||||
"--codex-test-delayed-output-after-exit-parent";
|
||||
pub(crate) const SYSTEM_PROXY_REQUEST_URL_ENV: &str =
|
||||
"CODEX_EXEC_SERVER_TEST_SYSTEM_PROXY_REQUEST_URL";
|
||||
pub(crate) const SYSTEM_PROXY_URL_ENV: &str = "CODEX_EXEC_SERVER_TEST_SYSTEM_PROXY_URL";
|
||||
|
||||
const CODEX_WINDOWS_SANDBOX_ARG1: &str = "--run-as-windows-sandbox";
|
||||
const DELAYED_OUTPUT_AFTER_EXIT_CHILD_ARG: &str = "--codex-test-delayed-output-after-exit-child";
|
||||
@@ -188,8 +193,27 @@ fn maybe_run_exec_server_from_test_binary(guard: Option<&TestBinaryDispatchGuard
|
||||
std::process::exit(1);
|
||||
}
|
||||
};
|
||||
let exit_code = match runtime.block_on(codex_exec_server::run_main(&listen_url, runtime_paths))
|
||||
{
|
||||
let http_client_factory = match (
|
||||
env::var(SYSTEM_PROXY_REQUEST_URL_ENV),
|
||||
env::var(SYSTEM_PROXY_URL_ENV),
|
||||
) {
|
||||
(Ok(request_url), Ok(proxy_url)) => {
|
||||
codex_http_client::cache_system_proxy_route_for_test(&request_url, proxy_url);
|
||||
HttpClientFactory::new(OutboundProxyPolicy::RespectSystemProxy)
|
||||
}
|
||||
(Err(env::VarError::NotPresent), Err(env::VarError::NotPresent)) => {
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault)
|
||||
}
|
||||
_ => {
|
||||
eprintln!("system proxy test configuration requires both request and proxy URLs");
|
||||
std::process::exit(1);
|
||||
}
|
||||
};
|
||||
let exit_code = match runtime.block_on(codex_exec_server::run_main(
|
||||
&listen_url,
|
||||
runtime_paths,
|
||||
http_client_factory,
|
||||
)) {
|
||||
Ok(()) => 0,
|
||||
Err(error) => {
|
||||
eprintln!("exec-server failed: {error}");
|
||||
|
||||
@@ -17,8 +17,11 @@ use codex_exec_server_protocol::JSONRPCMessage;
|
||||
use codex_exec_server_protocol::JSONRPCNotification;
|
||||
use codex_exec_server_protocol::JSONRPCResponse;
|
||||
use codex_exec_server_protocol::RequestId;
|
||||
use common::SYSTEM_PROXY_REQUEST_URL_ENV;
|
||||
use common::SYSTEM_PROXY_URL_ENV;
|
||||
use common::exec_server::ExecServerHarness;
|
||||
use common::exec_server::exec_server;
|
||||
use common::exec_server::exec_server_with_env;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde::de::DeserializeOwned;
|
||||
use serde_json::Value;
|
||||
@@ -110,6 +113,61 @@ async fn exec_server_http_request_buffers_response_body() -> anyhow::Result<()>
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// What this tests: a configured system-proxy factory survives the complete
|
||||
/// executor transport, processor, and handler chain for delegated HTTP.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_server_http_request_uses_configured_system_proxy() -> anyhow::Result<()> {
|
||||
let proxy_listener = TcpListener::bind("127.0.0.1:0").await?;
|
||||
let proxy_url = format!("http://{}", proxy_listener.local_addr()?);
|
||||
let request_url = "http://exec-server-system-proxy.invalid/delegated?route=system";
|
||||
let mut server = exec_server_with_env([
|
||||
(SYSTEM_PROXY_REQUEST_URL_ENV, request_url),
|
||||
(SYSTEM_PROXY_URL_ENV, proxy_url.as_str()),
|
||||
("HTTP_PROXY", ""),
|
||||
("http_proxy", ""),
|
||||
("HTTPS_PROXY", ""),
|
||||
("https_proxy", ""),
|
||||
("ALL_PROXY", ""),
|
||||
("all_proxy", ""),
|
||||
("NO_PROXY", ""),
|
||||
("no_proxy", ""),
|
||||
])
|
||||
.await?;
|
||||
initialize_exec_server(&mut server).await?;
|
||||
|
||||
let http_request_id = server
|
||||
.send_request(
|
||||
"http/request",
|
||||
serde_json::to_value(HttpRequestParams {
|
||||
method: "GET".to_string(),
|
||||
url: request_url.to_string(),
|
||||
headers: Vec::new(),
|
||||
body: None,
|
||||
timeout_ms: Some(5_000),
|
||||
redirect_policy: HttpRedirectPolicy::Follow,
|
||||
request_id: "system-proxy-request".to_string(),
|
||||
stream_response: false,
|
||||
})?,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let captured = accept_http_request(&proxy_listener).await?;
|
||||
assert_eq!(
|
||||
captured.request_line,
|
||||
"GET http://exec-server-system-proxy.invalid/delegated?route=system HTTP/1.1"
|
||||
);
|
||||
respond_with_status_and_headers(captured.stream, "200 OK", &[], b"proxied-response").await?;
|
||||
|
||||
let response: HttpRequestResponse = wait_for_response(&mut server, http_request_id).await?;
|
||||
assert_eq!(
|
||||
(response.status, response.body.into_inner()),
|
||||
(200, b"proxied-response".to_vec())
|
||||
);
|
||||
|
||||
server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// What this tests: OAuth callers can inspect redirect responses without the
|
||||
/// executor following the Location header.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
@@ -165,6 +223,69 @@ async fn exec_server_http_request_can_stop_at_redirects() -> anyhow::Result<()>
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// What this tests: the executor follows redirects when the HTTP request
|
||||
/// explicitly selects the redirect-following client pool.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_server_http_request_can_follow_redirects() -> anyhow::Result<()> {
|
||||
let mut server = exec_server().await?;
|
||||
initialize_exec_server(&mut server).await?;
|
||||
|
||||
let listener = TcpListener::bind("127.0.0.1:0").await?;
|
||||
let base_url = format!("http://{}", listener.local_addr()?);
|
||||
let http_request_id = server
|
||||
.send_request(
|
||||
"http/request",
|
||||
serde_json::to_value(HttpRequestParams {
|
||||
method: "GET".to_string(),
|
||||
url: format!("{base_url}/redirect"),
|
||||
headers: Vec::new(),
|
||||
body: None,
|
||||
timeout_ms: Some(5_000),
|
||||
redirect_policy: HttpRedirectPolicy::Follow,
|
||||
request_id: "follow-redirect-request".to_string(),
|
||||
stream_response: false,
|
||||
})?,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let redirect_request = accept_http_request(&listener).await?;
|
||||
assert_eq!(redirect_request.request_line, "GET /redirect HTTP/1.1");
|
||||
respond_with_status_and_headers(
|
||||
redirect_request.stream,
|
||||
"302 Found",
|
||||
&[("location", &format!("{base_url}/final"))],
|
||||
b"redirect",
|
||||
)
|
||||
.await?;
|
||||
|
||||
let final_request = accept_http_request(&listener).await?;
|
||||
assert_eq!(final_request.request_line, "GET /final HTTP/1.1");
|
||||
respond_with_status_and_headers(
|
||||
final_request.stream,
|
||||
"200 OK",
|
||||
&[("x-mcp-test", "redirected")],
|
||||
b"final-response-body",
|
||||
)
|
||||
.await?;
|
||||
|
||||
let response: HttpRequestResponse = wait_for_response(&mut server, http_request_id).await?;
|
||||
assert_eq!(
|
||||
(
|
||||
response.status,
|
||||
response_header(&response.headers, "x-mcp-test"),
|
||||
response.body.into_inner(),
|
||||
),
|
||||
(
|
||||
200,
|
||||
Some("redirected".to_string()),
|
||||
b"final-response-body".to_vec(),
|
||||
)
|
||||
);
|
||||
|
||||
server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// What this tests: a real exec-server websocket `http/request` can return
|
||||
/// response headers immediately and stream the response body as ordered
|
||||
/// `http/request/bodyDelta` notifications.
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
use std::io::Write;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
|
||||
use codex_exec_server::HttpClient;
|
||||
use codex_exec_server::HttpRedirectPolicy;
|
||||
use codex_exec_server::HttpRequestParams;
|
||||
use codex_exec_server::ReqwestHttpClient;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_http_client::OutboundProxyPolicy;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio::io::AsyncBufReadExt;
|
||||
use tokio::io::AsyncWriteExt;
|
||||
use tokio::io::BufReader;
|
||||
use tokio::net::TcpListener;
|
||||
use tracing_subscriber::Layer;
|
||||
use tracing_subscriber::layer::SubscriberExt;
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn delegated_http_success_logs_do_not_expose_sensitive_request_or_response_data()
|
||||
-> anyhow::Result<()> {
|
||||
let log_buffer = Arc::new(Mutex::new(Vec::new()));
|
||||
let writer_buffer = Arc::clone(&log_buffer);
|
||||
let subscriber = tracing_subscriber::registry().with(
|
||||
tracing_subscriber::fmt::layer()
|
||||
.with_ansi(false)
|
||||
.with_writer(move || TestLogWriter(Arc::clone(&writer_buffer)))
|
||||
.with_filter(
|
||||
tracing_subscriber::filter::Targets::new()
|
||||
.with_target("codex_http_client", tracing::Level::TRACE)
|
||||
.with_target("codex_exec_server", tracing::Level::TRACE),
|
||||
),
|
||||
);
|
||||
let _guard = tracing::subscriber::set_default(subscriber);
|
||||
tracing::debug!(target: "codex_exec_server", "log capture sentinel");
|
||||
let client =
|
||||
ReqwestHttpClient::new(HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault));
|
||||
|
||||
for (redirect_policy, status, query_secret, cookie_secret, location_secret) in [
|
||||
(
|
||||
HttpRedirectPolicy::Follow,
|
||||
"200 OK",
|
||||
"follow-query-secret",
|
||||
"follow-cookie-secret",
|
||||
"follow-location-secret",
|
||||
),
|
||||
(
|
||||
HttpRedirectPolicy::Stop,
|
||||
"302 Found",
|
||||
"stop-query-secret",
|
||||
"stop-cookie-secret",
|
||||
"stop-location-secret",
|
||||
),
|
||||
] {
|
||||
let listener = TcpListener::bind(("127.0.0.1", 0)).await?;
|
||||
let address = listener.local_addr()?;
|
||||
let response = format!(
|
||||
"HTTP/1.1 {status}\r\nSet-Cookie: session={cookie_secret}\r\nLocation: http://127.0.0.1/private?token={location_secret}\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok"
|
||||
);
|
||||
let server = tokio::spawn(async move {
|
||||
let (stream, _) = listener.accept().await?;
|
||||
let mut reader = BufReader::new(stream);
|
||||
loop {
|
||||
let mut line = String::new();
|
||||
if reader.read_line(&mut line).await? == 0 {
|
||||
anyhow::bail!("HTTP client disconnected before completing request headers");
|
||||
}
|
||||
if line == "\r\n" {
|
||||
break;
|
||||
}
|
||||
}
|
||||
reader.get_mut().write_all(response.as_bytes()).await?;
|
||||
anyhow::Ok(())
|
||||
});
|
||||
|
||||
let response = client
|
||||
.http_request(HttpRequestParams {
|
||||
method: "GET".to_string(),
|
||||
url: format!("http://{address}/delegated?token={query_secret}"),
|
||||
headers: Vec::new(),
|
||||
body: None,
|
||||
timeout_ms: Some(5_000),
|
||||
redirect_policy,
|
||||
request_id: "sensitive-request".to_string(),
|
||||
stream_response: false,
|
||||
})
|
||||
.await?;
|
||||
let expected_status = match redirect_policy {
|
||||
HttpRedirectPolicy::Follow => 200,
|
||||
HttpRedirectPolicy::Stop => 302,
|
||||
};
|
||||
assert_eq!(response.status, expected_status);
|
||||
server.await??;
|
||||
}
|
||||
|
||||
let logs = String::from_utf8(log_buffer.lock().expect("log buffer lock").clone())?;
|
||||
assert!(logs.contains("log capture sentinel"));
|
||||
for secret in [
|
||||
"follow-query-secret",
|
||||
"follow-cookie-secret",
|
||||
"follow-location-secret",
|
||||
"stop-query-secret",
|
||||
"stop-cookie-secret",
|
||||
"stop-location-secret",
|
||||
] {
|
||||
assert!(!logs.contains(secret), "logs exposed {secret}:\n{logs}");
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn delegated_http_failure_warning_redacts_request_url() -> anyhow::Result<()> {
|
||||
let log_buffer = Arc::new(Mutex::new(Vec::new()));
|
||||
let writer_buffer = Arc::clone(&log_buffer);
|
||||
let subscriber = tracing_subscriber::registry().with(
|
||||
tracing_subscriber::fmt::layer()
|
||||
.with_ansi(false)
|
||||
.with_writer(move || TestLogWriter(Arc::clone(&writer_buffer)))
|
||||
.with_filter(
|
||||
tracing_subscriber::filter::Targets::new()
|
||||
.with_target("codex_http_client", tracing::Level::TRACE)
|
||||
.with_target("codex_exec_server", tracing::Level::TRACE),
|
||||
),
|
||||
);
|
||||
let _guard = tracing::subscriber::set_default(subscriber);
|
||||
let unavailable_server = std::net::TcpListener::bind(("127.0.0.1", 0))?;
|
||||
let unavailable_address = unavailable_server.local_addr()?;
|
||||
drop(unavailable_server);
|
||||
let client =
|
||||
ReqwestHttpClient::new(HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault));
|
||||
|
||||
let error = client
|
||||
.http_request(HttpRequestParams {
|
||||
method: "GET".to_string(),
|
||||
url: format!(
|
||||
"http://{unavailable_address}/private-path-secret?token=failure-query-secret"
|
||||
),
|
||||
headers: Vec::new(),
|
||||
body: None,
|
||||
timeout_ms: None,
|
||||
redirect_policy: HttpRedirectPolicy::Follow,
|
||||
request_id: "failed-sensitive-request".to_string(),
|
||||
stream_response: false,
|
||||
})
|
||||
.await;
|
||||
assert!(error.is_err(), "request to a closed port should fail");
|
||||
|
||||
let logs = String::from_utf8(log_buffer.lock().expect("log buffer lock").clone())?;
|
||||
assert!(logs.contains("http/request send failed"));
|
||||
assert!(logs.contains("error_is_connect=true"));
|
||||
for secret in ["private-path-secret", "failure-query-secret"] {
|
||||
assert!(!logs.contains(secret), "logs exposed {secret}:\n{logs}");
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct TestLogWriter(Arc<Mutex<Vec<u8>>>);
|
||||
|
||||
impl Write for TestLogWriter {
|
||||
fn write(&mut self, bytes: &[u8]) -> std::io::Result<usize> {
|
||||
self.0
|
||||
.lock()
|
||||
.map_err(|_| std::io::Error::other("log buffer lock"))?
|
||||
.extend_from_slice(bytes);
|
||||
Ok(bytes.len())
|
||||
}
|
||||
|
||||
fn flush(&mut self) -> std::io::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
@@ -31,6 +31,8 @@ use codex_exec_server::NoiseRendezvousConnectProvider;
|
||||
use codex_exec_server::ProcessId;
|
||||
use codex_exec_server::RemoteEnvironmentConfig;
|
||||
use codex_exec_server_test_support::environment_manager_without_environments;
|
||||
use codex_http_client::HttpClientFactory;
|
||||
use codex_http_client::OutboundProxyPolicy;
|
||||
use codex_protocol::capabilities::CapabilityRootLocation;
|
||||
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
@@ -144,6 +146,7 @@ async fn deferred_noise_environment_connects_and_reconnects_with_fresh_bundle()
|
||||
registry.uri(),
|
||||
ENVIRONMENT_ID.to_string(),
|
||||
static_registry_auth_provider(),
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
)?;
|
||||
let remote_environment = tokio::spawn(codex_exec_server::run_remote_environment(
|
||||
config,
|
||||
@@ -274,6 +277,7 @@ async fn remote_environment_routes_encrypted_exec_server_rpc() -> Result<()> {
|
||||
registry.uri(),
|
||||
ENVIRONMENT_ID.to_string(),
|
||||
static_registry_auth_provider(),
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
)?;
|
||||
let remote_environment = tokio::spawn(codex_exec_server::run_remote_environment(
|
||||
config,
|
||||
|
||||
Reference in New Issue
Block a user