From 38b2104c5178c4b396fa5df6e1017693667315f8 Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Thu, 23 Jul 2026 17:02:31 -0400 Subject: [PATCH] Respect standard HTTP proxy environment variables --- Cargo.lock | 74 +++++++++++- Cargo.toml | 3 +- src/api/client.rs | 35 +++--- src/auth/client.rs | 6 +- src/ci/gitlab.rs | 15 ++- src/commands/analyze/cube.rs | 6 +- src/daemon/telemetry_worker.rs | 6 +- src/http.rs | 60 ++++++---- tests/integration/http_proxy.rs | 160 ++++++++++++++++++++++++++ tests/integration/main.rs | 1 + tests/integration/tls_native_certs.rs | 2 +- 11 files changed, 307 insertions(+), 61 deletions(-) create mode 100644 tests/integration/http_proxy.rs diff --git a/Cargo.lock b/Cargo.lock index 0d96d6f3e5..3c1520183a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -143,6 +143,12 @@ version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "base64ct" +version = "1.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2af50177e190e07a26ab74f8b1efbfe2ef87da2116221318cb1c2e82baf7de06" + [[package]] name = "bit-set" version = "0.5.3" @@ -651,6 +657,16 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5729f5117e208430e437df2f4843f5e5952997175992d1414f94c57d61e270b4" +[[package]] +name = "der" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a69dedd701da44b0536442edf09c81a64b0ab97a7a4a5e3d1971f00027cbc63d" +dependencies = [ + "pem-rfc7468", + "zeroize", +] + [[package]] name = "deranged" version = "0.5.8" @@ -1074,7 +1090,6 @@ dependencies = [ "libc", "mockito", "named_pipe", - "native-tls", "once_cell", "openssl", "paste", @@ -2424,6 +2439,15 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" +[[package]] +name = "pem-rfc7468" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6305423e0e7738146434843d1694d621cce767262b2a86910beab705e4493d9" +dependencies = [ + "base64ct", +] + [[package]] name = "percent-encoding" version = "2.3.2" @@ -2954,6 +2978,15 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "rustls-pki-types" +version = "1.15.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f4925028c7eb5d1fcdaf196971378ed9d2c1c4efc7dc5d011256f76c99c0a96" +dependencies = [ + "zeroize", +] + [[package]] name = "rustversion" version = "1.0.22" @@ -3752,15 +3785,31 @@ checksum = "81e544489bf3d8ef66c953931f56617f423cd4b5494be343d9b9d3dda037b9a3" [[package]] name = "ureq" -version = "2.12.1" +version = "3.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "02d1a66277ed75f640d608235660df48c8e3c19f3b4edb6a263315626cc3c01d" +checksum = "dea7109cdcd5864d4eeb1b58a1648dc9bf520360d7af16ec26d0a9354bafcfc0" dependencies = [ "base64", + "der", "log", "native-tls", - "once_cell", - "url", + "percent-encoding", + "rustls-pki-types", + "ureq-proto", + "utf8-zero", + "webpki-root-certs", +] + +[[package]] +name = "ureq-proto" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e994ba84b0bd1b1b0cf92878b7ef898a5c1760108fe7b6010327e274917a808c" +dependencies = [ + "base64", + "http", + "httparse", + "log", ] [[package]] @@ -3775,6 +3824,12 @@ dependencies = [ "serde", ] +[[package]] +name = "utf8-zero" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8c0a043c9540bae7c578c88f91dda8bd82e59ae27c21baca69c8b191aaf5a6e" + [[package]] name = "utf8_iter" version = "1.0.4" @@ -3959,6 +4014,15 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "webpki-root-certs" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b96554aa2acc8ccdb7e1c9a58a7a68dd5d13bccc69cd124cb09406db612a1c9b" +dependencies = [ + "rustls-pki-types", +] + [[package]] name = "wezterm-bidi" version = "0.2.3" diff --git a/Cargo.toml b/Cargo.toml index 789bf11d2f..9c570137e1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,8 +22,7 @@ rusqlite = { version = "0.31", features = ["bundled"] } libc = "0.2" jsonc-parser = { version = "0.32", features = ["cst"] } dirs = "5.0" -ureq = { version = "2.12", default-features = false, features = ["native-tls"] } -native-tls = "0.2" +ureq = { version = "3.3", default-features = false, features = ["native-tls"] } url = "2.5" glob = "0.3" ignore = "0.4" diff --git a/src/api/client.rs b/src/api/client.rs index 40a4697b29..001148c503 100644 --- a/src/api/client.rs +++ b/src/api/client.rs @@ -188,30 +188,39 @@ impl ApiContext { /// Create a GET request with common headers (User-Agent, X-Distinct-ID) /// Use this for all HTTP GET requests to ensure consistent headers. /// The returned (Agent, Request) pair uses the system's native certificate store. - pub fn http_get(url: &str, timeout_secs: Option) -> (ureq::Agent, ureq::Request) { + pub fn http_get( + url: &str, + timeout_secs: Option, + ) -> ( + ureq::Agent, + ureq::RequestBuilder, + ) { let agent = http::build_agent(timeout_secs); let request = agent .get(url) - .set( + .header( "User-Agent", &format!("git-ai/{}", env!("CARGO_PKG_VERSION")), ) - .set("X-Distinct-ID", &config::get_or_create_distinct_id()); + .header("X-Distinct-ID", &config::get_or_create_distinct_id()); (agent, request) } /// Create a POST request with common headers (User-Agent, X-Distinct-ID) /// Use this for all HTTP POST requests to ensure consistent headers. /// The returned (Agent, Request) pair uses the system's native certificate store. - pub fn http_post(url: &str, timeout_secs: Option) -> (ureq::Agent, ureq::Request) { + pub fn http_post( + url: &str, + timeout_secs: Option, + ) -> (ureq::Agent, ureq::RequestBuilder) { let agent = http::build_agent(timeout_secs); let request = agent .post(url) - .set( + .header( "User-Agent", &format!("git-ai/{}", env!("CARGO_PKG_VERSION")), ) - .set("X-Distinct-ID", &config::get_or_create_distinct_id()); + .header("X-Distinct-ID", &config::get_or_create_distinct_id()); (agent, request) } @@ -313,16 +322,16 @@ impl ApiContext { let body_json = serde_json::to_string(body).map_err(GitAiError::JsonError)?; let (_agent, mut request) = Self::http_post(&url, self.timeout_secs); - request = request.set("Content-Type", "application/json"); + request = request.header("Content-Type", "application/json"); if let Some(api_key) = &self.api_key { - request = request.set("X-API-Key", api_key); + request = request.header("X-API-Key", api_key); if let Some(identity) = &self.author_identity { - request = request.set("X-Author-Identity", identity); + request = request.header("X-Author-Identity", identity); } } if let Some(token) = &self.auth_token { - request = request.set("Authorization", &format!("Bearer {}", token)); + request = request.header("Authorization", &format!("Bearer {}", token)); } http::send_with_body(request, &body_json) @@ -336,13 +345,13 @@ impl ApiContext { let (_agent, mut request) = Self::http_get(&url, self.timeout_secs); if let Some(api_key) = &self.api_key { - request = request.set("X-API-Key", api_key); + request = request.header("X-API-Key", api_key); if let Some(identity) = &self.author_identity { - request = request.set("X-Author-Identity", identity); + request = request.header("X-Author-Identity", identity); } } if let Some(token) = &self.auth_token { - request = request.set("Authorization", &format!("Bearer {}", token)); + request = request.header("Authorization", &format!("Bearer {}", token)); } http::send(request).map_err(|e| GitAiError::Generic(format!("HTTP request failed: {}", e))) diff --git a/src/auth/client.rs b/src/auth/client.rs index ae49b18c2a..8000f7d2b0 100644 --- a/src/auth/client.rs +++ b/src/auth/client.rs @@ -60,7 +60,7 @@ impl OAuthClient { let url = format!("{}/worker/oauth/token", self.base_url); let (_agent, request) = ApiContext::http_post(&url, Some(30)); - let request = request.set("Content-Type", "application/json"); + let request = request.header("Content-Type", "application/json"); let response = http::send_with_body(request, &body.to_string()) .map_err(|e| format!("Failed to connect to server: {}", e))?; @@ -98,7 +98,7 @@ impl OAuthClient { let url = format!("{}/worker/oauth/device/code", self.base_url); let (_agent, request) = ApiContext::http_post(&url, Some(30)); - let request = request.set("Content-Type", "application/json"); + let request = request.header("Content-Type", "application/json"); let response = http::send_with_body(request, "{}") .map_err(|e| format!("Failed to connect to server: {}", e))?; @@ -142,7 +142,7 @@ impl OAuthClient { }); let (_agent, request) = ApiContext::http_post(&url, Some(30)); - let request = request.set("Content-Type", "application/json"); + let request = request.header("Content-Type", "application/json"); let response = http::send_with_body(request, &body.to_string()) .map_err(|e| format!("Failed to connect to server: {}", e))?; diff --git a/src/ci/gitlab.rs b/src/ci/gitlab.rs index c8d83dd4b7..0f2af719f2 100644 --- a/src/ci/gitlab.rs +++ b/src/ci/gitlab.rs @@ -85,10 +85,13 @@ fn gitlab_api_get( auth_token: &str, ) -> Result { let agent = crate::http::build_agent(Some(30)); - let request = agent.get(endpoint).set(auth_header_name, auth_token).set( - "User-Agent", - &format!("git-ai/{}", env!("CARGO_PKG_VERSION")), - ); + let request = agent + .get(endpoint) + .header(auth_header_name, auth_token) + .header( + "User-Agent", + &format!("git-ai/{}", env!("CARGO_PKG_VERSION")), + ); crate::http::send(request) } @@ -300,8 +303,8 @@ pub fn get_gitlab_ci_context() -> Result, GitAiError> { let agent = crate::http::build_agent(Some(30)); let request = agent .get(&source_project_endpoint) - .set(auth_header_name, &auth_token) - .set( + .header(auth_header_name, &auth_token) + .header( "User-Agent", &format!("git-ai/{}", env!("CARGO_PKG_VERSION")), ); diff --git a/src/commands/analyze/cube.rs b/src/commands/analyze/cube.rs index 58ca7541b3..48647726bf 100644 --- a/src/commands/analyze/cube.rs +++ b/src/commands/analyze/cube.rs @@ -87,8 +87,8 @@ impl CubeClient { let request = self .agent .post(&self.url(path)) - .set("x-api-key", &self.api_key) - .set("Content-Type", "application/json"); + .header("x-api-key", &self.api_key) + .header("Content-Type", "application/json"); let body_str = serde_json::to_string(body).map_err(|e| CubeError::Json(e.to_string()))?; let response = http::send_with_body(request, &body_str).map_err(CubeError::Transport)?; Self::parse(response) @@ -99,7 +99,7 @@ impl CubeClient { let request = self .agent .get(&self.url(path)) - .set("x-api-key", &self.api_key); + .header("x-api-key", &self.api_key); let response = http::send(request).map_err(CubeError::Transport)?; Self::parse(response) } diff --git a/src/daemon/telemetry_worker.rs b/src/daemon/telemetry_worker.rs index 2e03e2efd4..01be00d42d 100644 --- a/src/daemon/telemetry_worker.rs +++ b/src/daemon/telemetry_worker.rs @@ -1247,7 +1247,7 @@ fn flush_sentry_and_posthog( let agent = crate::http::build_agent(Some(30)); let request = agent .post(&endpoint) - .set("Content-Type", "application/json"); + .header("Content-Type", "application/json"); let _ = crate::http::send_with_body( request, &serde_json::to_string(&ph_event).unwrap_or_default(), @@ -1499,8 +1499,8 @@ impl SentryClient { let agent = crate::http::build_agent(Some(30)); let request = agent .post(&self.endpoint) - .set("X-Sentry-Auth", &auth_header) - .set("Content-Type", "application/json"); + .header("X-Sentry-Auth", &auth_header) + .header("Content-Type", "application/json"); let response = crate::http::send_with_body(request, &body)?; let status = response.status_code; diff --git a/src/http.rs b/src/http.rs index f6f8b7f376..ffa189dfa4 100644 --- a/src/http.rs +++ b/src/http.rs @@ -1,22 +1,31 @@ -use std::io::Read; -use std::sync::Arc; use std::time::Duration; +use ureq::config::Config; +use ureq::tls::{RootCerts, TlsConfig, TlsProvider}; +use ureq::typestate::{WithBody, WithoutBody}; -/// Build a ureq Agent that uses the platform's native TLS library. +/// Build a ureq Agent that uses standard proxy environment variables and the +/// platform's native TLS library. /// /// Uses OpenSSL on Linux, Secure Transport on macOS, and SChannel on /// Windows — the same TLS implementations that curl uses. This ensures /// certificates trusted by the OS (including custom CA certs added to /// the system trust store) are handled identically to curl and browsers. +/// +/// Proxy configuration is read from `ALL_PROXY`, `HTTPS_PROXY`, `HTTP_PROXY`, +/// and their lowercase variants. `NO_PROXY`/`no_proxy` bypasses matching hosts. pub fn build_agent(timeout_secs: Option) -> ureq::Agent { - let tls = native_tls::TlsConnector::new().expect("failed to create TLS connector"); - let mut builder = ureq::AgentBuilder::new().tls_connector(Arc::new(tls)); + let mut builder = Config::builder().http_status_as_error(false).tls_config( + TlsConfig::builder() + .provider(TlsProvider::NativeTls) + .root_certs(RootCerts::PlatformVerifier) + .build(), + ); if let Some(secs) = timeout_secs { - builder = builder.timeout(Duration::from_secs(secs)); + builder = builder.timeout_global(Some(Duration::from_secs(secs))); } - builder.build() + builder.build().new_agent() } /// HTTP response wrapper that normalizes ureq's error handling. @@ -41,31 +50,32 @@ impl Response { } } -fn read_ureq_response(response: ureq::Response) -> Result { - let status_code = response.status(); - let mut body = Vec::new(); - response - .into_reader() - .read_to_end(&mut body) +fn read_ureq_response(mut response: ureq::http::Response) -> Result { + let status_code = response.status().as_u16(); + let body = response + .body_mut() + .with_config() + .read_to_vec() .map_err(|e| format!("Failed to read response body: {}", e))?; Ok(Response { status_code, body }) } /// Execute a ureq request, normalizing errors so that HTTP error status codes /// are returned as Ok(Response) rather than Err. -pub fn send(request: ureq::Request) -> Result { - match request.call() { - Ok(response) => read_ureq_response(response), - Err(ureq::Error::Status(_code, response)) => read_ureq_response(response), - Err(ureq::Error::Transport(err)) => Err(err.to_string()), - } +pub fn send(request: ureq::RequestBuilder) -> Result { + request + .call() + .map_err(|err| err.to_string()) + .and_then(read_ureq_response) } /// Execute a ureq request with a string body. -pub fn send_with_body(request: ureq::Request, body: &str) -> Result { - match request.send_string(body) { - Ok(response) => read_ureq_response(response), - Err(ureq::Error::Status(_code, response)) => read_ureq_response(response), - Err(ureq::Error::Transport(err)) => Err(err.to_string()), - } +pub fn send_with_body( + request: ureq::RequestBuilder, + body: &str, +) -> Result { + request + .send(body) + .map_err(|err| err.to_string()) + .and_then(read_ureq_response) } diff --git a/tests/integration/http_proxy.rs b/tests/integration/http_proxy.rs new file mode 100644 index 0000000000..4894fbdb8f --- /dev/null +++ b/tests/integration/http_proxy.rs @@ -0,0 +1,160 @@ +use crate::repos::test_repo::TestRepo; +use std::io::{Read, Write}; +use std::net::{SocketAddr, TcpListener}; +use std::sync::mpsc::{self, Receiver}; +use std::time::Duration; + +const RELEASES_RESPONSE: &str = + r#"{"channels":{"latest":{"version":"0.0.0","checksum":"unused"}}}"#; + +struct OneShotHttpServer { + address: SocketAddr, + request: Receiver, +} + +fn read_http_headers(stream: &mut std::net::TcpStream) -> Vec { + let mut bytes = Vec::new(); + let mut buffer = [0; 1024]; + while !bytes.windows(4).any(|window| window == b"\r\n\r\n") { + let count = stream.read(&mut buffer).unwrap(); + if count == 0 { + break; + } + bytes.extend_from_slice(&buffer[..count]); + } + bytes +} + +impl OneShotHttpServer { + fn start(response_body: impl AsRef<[u8]>) -> Self { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let address = listener.local_addr().unwrap(); + let (request_tx, request) = mpsc::channel(); + let response_body = response_body.as_ref().to_vec(); + + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().unwrap(); + stream + .set_read_timeout(Some(Duration::from_secs(5))) + .unwrap(); + + let mut bytes = read_http_headers(&mut stream); + if bytes.starts_with(b"CONNECT ") { + stream + .write_all(b"HTTP/1.1 200 Connection Established\r\n\r\n") + .unwrap(); + stream.flush().unwrap(); + bytes.extend_from_slice(&read_http_headers(&mut stream)); + } + + request_tx + .send(String::from_utf8_lossy(&bytes).into_owned()) + .unwrap(); + write!( + stream, + "HTTP/1.1 200 OK\r\nContent-Length: {}\r\nConnection: close\r\n\r\n", + response_body.len() + ) + .unwrap(); + stream.write_all(&response_body).unwrap(); + }); + + Self { address, request } + } + + fn url(&self) -> String { + format!("http://{}", self.address) + } + + fn received_request(&self) -> String { + self.request + .recv_timeout(Duration::from_secs(5)) + .expect("HTTP server did not receive a request") + } +} + +fn proxy_env<'a>(proxy_url: &'a str, no_proxy: &'a str) -> [(&'a str, &'a str); 10] { + // Windows environment variable names are case-insensitive, so setting the + // uppercase variant to empty would also clear the lowercase variant. + let (http_proxy, uppercase_http_proxy) = if cfg!(windows) { + ("", proxy_url) + } else { + (proxy_url, "") + }; + + [ + ("http_proxy", http_proxy), + ("HTTP_PROXY", uppercase_http_proxy), + ("https_proxy", ""), + ("HTTPS_PROXY", ""), + ("all_proxy", ""), + ("ALL_PROXY", ""), + ("no_proxy", no_proxy), + ("NO_PROXY", no_proxy), + ("GIT_AI_API_BASE_URL", "http://git-ai-proxy-test.invalid"), + ("GIT_AI_DISABLE_VERSION_CHECKS", "false"), + ] +} + +#[test] +fn upgrade_uses_http_proxy_from_environment() { + let repo = TestRepo::new(); + let proxy = OneShotHttpServer::start(RELEASES_RESPONSE); + let proxy_url = proxy.url(); + + let output = repo.git_ai_with_env(&["upgrade"], &proxy_env(&proxy_url, "")); + let request = proxy.received_request(); + + assert!( + output + .as_ref() + .is_ok_and(|output| output.contains("You are running a newer version")), + "upgrade output: {output:?}; proxy request: {request:?}" + ); + assert!( + request.starts_with("CONNECT git-ai-proxy-test.invalid:80 HTTP/1.1") + && request.contains("GET /worker/releases HTTP/1.1"), + "unexpected proxy request: {request:?}" + ); +} + +#[test] +fn upgrade_respects_no_proxy_from_environment() { + let repo = TestRepo::new(); + let target = OneShotHttpServer::start(RELEASES_RESPONSE); + let unavailable_proxy = TcpListener::bind("127.0.0.1:0").unwrap(); + let proxy_url = format!("http://{}", unavailable_proxy.local_addr().unwrap()); + drop(unavailable_proxy); + let target_url = target.url(); + let mut env = proxy_env(&proxy_url, "127.0.0.1"); + env[8] = ("GIT_AI_API_BASE_URL", &target_url); + + let output = repo.git_ai_with_env(&["upgrade"], &env).unwrap(); + + assert!(output.contains("You are running a newer version")); + assert!( + target + .received_request() + .starts_with("GET /worker/releases ") + ); +} + +#[test] +fn upgrade_accepts_response_larger_than_ureq_default_limit() { + let repo = TestRepo::new(); + let mut response = RELEASES_RESPONSE.as_bytes().to_vec(); + response.resize(11 * 1024 * 1024, b' '); + let target = OneShotHttpServer::start(response); + let target_url = target.url(); + let mut env = proxy_env("", ""); + env[8] = ("GIT_AI_API_BASE_URL", &target_url); + + let output = repo.git_ai_with_env(&["upgrade"], &env).unwrap(); + + assert!(output.contains("You are running a newer version")); + assert!( + target + .received_request() + .starts_with("GET /worker/releases ") + ); +} diff --git a/tests/integration/main.rs b/tests/integration/main.rs index 606e1a1683..82c798e750 100644 --- a/tests/integration/main.rs +++ b/tests/integration/main.rs @@ -74,6 +74,7 @@ mod github_copilot_tools; mod github_integration; mod gix_config_tests; mod graphite; +mod http_proxy; mod ignore_prompts; mod ignore_unit; mod initial_attributions; diff --git a/tests/integration/tls_native_certs.rs b/tests/integration/tls_native_certs.rs index be6360477e..ff8f4b7a25 100644 --- a/tests/integration/tls_native_certs.rs +++ b/tests/integration/tls_native_certs.rs @@ -23,7 +23,7 @@ fn test_https_request_uses_system_certs() { for url in URLS { for attempt in 1..=ATTEMPTS_PER_URL { let agent = git_ai::http::build_agent(Some(10)); - match git_ai::http::send(agent.get(url)) { + match git_ai::http::send(agent.get(*url)) { Ok(response) if (200..400).contains(&response.status_code) => return, Ok(response) => failures.push(format!( "{} attempt {} returned status {}",