diff --git a/Cargo.lock b/Cargo.lock index 8315bf9f..53da9bad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -58,6 +58,7 @@ dependencies = [ "tree-sitter-swift", "tree-sitter-typescript", "tui-test", + "url", "uuid", "which", "zip", diff --git a/Cargo.toml b/Cargo.toml index 271c2083..5705f8b3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,9 @@ tokio = { version = "1", features = ["full"] } # HTTP client reqwest = { version = "0.12", features = ["json", "rustls-tls", "stream"], default-features = false } +# URL parsing (reqwest's own dependency) — used to match loopback hosts exactly +# rather than by string prefix, so the HTTPS transport guard cannot be bypassed. +url = "2" # Serialization serde = { version = "1", features = ["derive"] } diff --git a/src/api/client.rs b/src/api/client.rs index aec203ec..49f09c5d 100644 --- a/src/api/client.rs +++ b/src/api/client.rs @@ -46,10 +46,10 @@ impl ActualApiClient { timeout: Duration, connect_timeout: Duration, ) -> Result { - // Enforce HTTPS for all non-loopback URLs to protect credentials in transit. - let is_localhost = base_url.starts_with("http://localhost") - || base_url.starts_with("http://127.0.0.1") - || base_url.starts_with("http://[::1]"); + // Enforce HTTPS for all non-loopback URLs to protect credentials in + // transit — see `crate::net::is_loopback_http_url` for why the loopback + // check must be an exact host match, not a raw string prefix. + let is_localhost = crate::net::is_loopback_http_url(base_url); if !base_url.starts_with("https://") && !is_localhost { return Err(ActualError::ConfigError( "API URL must use HTTPS (got a non-HTTPS URL). \ diff --git a/src/auth/oauth.rs b/src/auth/oauth.rs index 6d0be745..58fb62f7 100644 --- a/src/auth/oauth.rs +++ b/src/auth/oauth.rs @@ -177,15 +177,13 @@ pub fn build_authorize_url( /// Build an HTTP client for the auth server. Enforces HTTPS for non-loopback /// URLs so tokens are never sent in clear text (loopback `http://` is allowed -/// for the local mock). +/// for the local mock — see [`crate::net::is_loopback_http_url`]). /// /// Shared with [`crate::auth::pat`], whose token-issuance call carries the same /// "never send a bearer over clear-text http" requirement. pub(crate) fn build_http_client(base_url: &str) -> Result { - let is_localhost = base_url.starts_with("http://localhost") - || base_url.starts_with("http://127.0.0.1") - || base_url.starts_with("http://[::1]"); - if !base_url.starts_with("https://") && !is_localhost { + let is_loopback = crate::net::is_loopback_http_url(base_url); + if !base_url.starts_with("https://") && !is_loopback { return Err(ActualError::ConfigError( "Auth server URL must use HTTPS (got a non-HTTPS, non-loopback URL). \ Use https:// to protect your credentials." @@ -194,7 +192,7 @@ pub(crate) fn build_http_client(base_url: &str) -> Result Result, crate::error::ActualError> { use crate::error::ActualError; - let is_local = - base_url.starts_with("http://localhost") || base_url.starts_with("http://127.0.0.1"); + let is_local = crate::net::is_loopback_http_url(base_url); let client = reqwest::Client::builder() .timeout(timeout) @@ -245,8 +244,7 @@ pub(crate) async fn fetch_anthropic_models_async( ) -> Result, crate::error::ActualError> { use crate::error::ActualError; - let is_local = - base_url.starts_with("http://localhost") || base_url.starts_with("http://127.0.0.1"); + let is_local = crate::net::is_loopback_http_url(base_url); let client = reqwest::Client::builder() .timeout(timeout) diff --git a/src/net.rs b/src/net.rs new file mode 100644 index 00000000..4e227c93 --- /dev/null +++ b/src/net.rs @@ -0,0 +1,89 @@ +//! Network transport helpers shared across the crate's HTTP clients. + +use std::net::{Ipv4Addr, Ipv6Addr}; + +use url::Host; + +/// Return `true` only when `base_url` is a genuine clear-text `http` loopback +/// endpoint for which it is safe to relax the HTTPS-only transport guard. +/// +/// Several HTTP clients in this crate allow clear-text `http` for a local dev +/// or mock server by disabling reqwest's `https_only`. The earlier checks did +/// this with `str::starts_with("http://localhost")` / `127.0.0.1` / `[::1]` on +/// the raw URL, which a non-loopback attacker host could satisfy: +/// `http://localhost.evil.com`, `http://127.0.0.1.evil.com`, and the userinfo +/// trick `http://127.0.0.1@evil.com` (the connection's real host is `evil.com`) +/// all matched, which disabled `https_only` and let credentials leave over +/// clear-text HTTP when the caller controlled the base URL. Parse the URL and +/// inspect the *actual* host instead, so only true loopback endpoints qualify: +/// +/// - the scheme must be `http` — an `https` URL never needs the guard relaxed; +/// - the URL must carry no userinfo, rejecting the `user@host` smuggle; +/// - the host must be exactly `localhost`, `127.0.0.1`, or `::1`. +pub fn is_loopback_http_url(base_url: &str) -> bool { + let url = match url::Url::parse(base_url) { + Ok(u) => u, + Err(_) => return false, + }; + // Any userinfo means the host after `@` is the real destination — reject so + // `http://127.0.0.1@evil.com` cannot masquerade as loopback. + if !url.username().is_empty() || url.password().is_some() { + return false; + } + // A URL with no host at all (e.g. a `data:` / `mailto:` URL) is not loopback. + let host = match url.host() { + Some(h) => h, + None => return false, + }; + // Relax the guard only for clear-text http whose host is *exactly* a + // loopback identity. Compare the parsed host, not the raw string, so a + // lookalike domain like `localhost.evil.com` cannot match; `Ipv4Addr` / + // `Ipv6Addr::LOCALHOST` are exactly `127.0.0.1` / `::1`, not the wider + // loopback ranges. + url.scheme() == "http" + && match host { + Host::Domain(host) => host.eq_ignore_ascii_case("localhost"), + Host::Ipv4(addr) => addr == Ipv4Addr::LOCALHOST, + Host::Ipv6(addr) => addr == Ipv6Addr::LOCALHOST, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn accepts_genuine_loopback() { + assert!(is_loopback_http_url("http://localhost:4000")); + assert!(is_loopback_http_url("http://localhost")); // no port + assert!(is_loopback_http_url("http://127.0.0.1:4000")); + assert!(is_loopback_http_url("http://[::1]:4000")); + assert!(is_loopback_http_url("http://LOCALHOST:4000")); // host is case-insensitive + } + + #[test] + fn rejects_lookalike_hosts() { + // A host that merely *starts with* the loopback literal but resolves to + // the attacker's domain must NOT be treated as loopback. + assert!(!is_loopback_http_url("http://localhost.evil.com")); + assert!(!is_loopback_http_url("http://127.0.0.1.evil.com")); + // The userinfo trick: the connection's real host is evil.com. + assert!(!is_loopback_http_url("http://127.0.0.1@evil.com")); + assert!(!is_loopback_http_url("http://localhost@evil.com")); + // Plain non-loopback host. + assert!(!is_loopback_http_url("http://evil.com")); + } + + #[test] + fn rejects_https_userinfo_and_garbage() { + // https never needs the guard relaxed (stays https_only). + assert!(!is_loopback_http_url("https://localhost:4000")); + // Even a genuine loopback host is rejected when it carries userinfo. + assert!(!is_loopback_http_url("http://user:pass@127.0.0.1:4000")); + // A URL with no host component (data:/mailto:) is not loopback. + assert!(!is_loopback_http_url("data:text/plain,hello")); + assert!(!is_loopback_http_url("mailto:dev@example.com")); + // Non-URL input is not loopback. + assert!(!is_loopback_http_url("not a url")); + } +} diff --git a/src/runner/anthropic_api.rs b/src/runner/anthropic_api.rs index e53a22c4..4cdec786 100644 --- a/src/runner/anthropic_api.rs +++ b/src/runner/anthropic_api.rs @@ -81,9 +81,7 @@ impl AnthropicApiRunner { base_url: String, max_tokens: u32, ) -> Result { - let is_localhost = base_url.starts_with("http://localhost") - || base_url.starts_with("http://127.0.0.1") - || base_url.starts_with("http://[::1]"); + let is_localhost = crate::net::is_loopback_http_url(&base_url); let client = reqwest::Client::builder() .timeout(timeout) .https_only(!is_localhost) diff --git a/src/runner/openai_api.rs b/src/runner/openai_api.rs index bc9d2706..61f4959d 100644 --- a/src/runner/openai_api.rs +++ b/src/runner/openai_api.rs @@ -149,9 +149,7 @@ impl OpenAiApiRunner { /// Rebuilds the inner `reqwest::Client` with `https_only` disabled when the /// URL is a loopback address so that mock servers work in tests. pub(crate) fn with_base_url(mut self, base_url: String) -> Self { - let is_localhost = base_url.starts_with("http://localhost") - || base_url.starts_with("http://127.0.0.1") - || base_url.starts_with("http://[::1]"); + let is_localhost = crate::net::is_loopback_http_url(&base_url); if is_localhost { self.client = reqwest::Client::builder() .timeout(self.timeout)