fix(net): harden the loopback transport guard across all HTTP clients

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"] }

src/api/client.rs

@@ -45,10 +45,10 @@ impl ActualApiClient {
         timeout: Duration,
         connect_timeout: Duration,
     ) -> Result<Self, ActualError> {
-        // 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). \

src/auth/oauth.rs

@@ -140,12 +140,10 @@ 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`]).
 fn build_http_client(base_url: &str) -> Result<reqwest::Client, ActualError> {
-    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."
@@ -154,7 +152,7 @@ fn build_http_client(base_url: &str) -> Result<reqwest::Client, ActualError> {
     }
     reqwest::Client::builder()
         .user_agent(USER_AGENT_VALUE)
-        .https_only(!is_localhost)
+        .https_only(!is_loopback)
         .timeout(Duration::from_secs(30))
         .connect_timeout(Duration::from_secs(10))
         .build()
@@ -439,6 +437,37 @@ mod tests {
         }
     }
 
+    // --- Transport guard: the auth client must refuse to relax HTTPS for any
+    // non-loopback host. The exact-host predicate itself is unit-tested in
+    // `crate::net`; these cover this client's use of it. ---
+
+    #[test]
+    fn test_build_http_client_rejects_bypass_urls() {
+        // Each bypass URL must be refused outright — the HTTPS guard stays on,
+        // so no clear-text client is ever built for a non-loopback host.
+        for url in [
+            "http://localhost.evil.com",
+            "http://127.0.0.1.evil.com",
+            "http://127.0.0.1@evil.com",
+            "http://evil.com",
+        ] {
+            let err = build_http_client(url).unwrap_err();
+            assert!(
+                matches!(err, ActualError::ConfigError(ref m) if m.contains("HTTPS")),
+                "expected HTTPS-guard rejection for {url}, got: {err:?}"
+            );
+        }
+    }
+
+    #[test]
+    fn test_build_http_client_allows_loopback_and_https() {
+        // Genuine loopback dev endpoints and any https URL still build fine.
+        assert!(build_http_client("http://localhost:4000").is_ok());
+        assert!(build_http_client("http://127.0.0.1:4000").is_ok());
+        assert!(build_http_client("http://[::1]:4000").is_ok());
+        assert!(build_http_client("https://app.actual.ai").is_ok());
+    }
+
     #[test]
     fn test_build_authorize_url_contains_required_params() {
         let cfg = test_cfg("http://localhost:4000");
@@ -921,10 +950,11 @@ mod tests {
 
     #[tokio::test]
     async fn test_login_with_authorize_url_error() {
-        // base_url passes the loopback HTTP-client guard (localhost) but fails
-        // URL parsing (invalid port) → exercises the build_authorize_url `?`
+        // base_url passes the HTTP-client guard (an `https://` URL is accepted
+        // by scheme prefix, without a full parse) but fails `build_authorize_url`'s
+        // URL parse (invalid port 99999) → exercises the build_authorize_url `?`
         // error path inside login_with.
-        let cfg = test_cfg("http://127.0.0.1:99999");
+        let cfg = test_cfg("https://127.0.0.1:99999");
         let server = LoopbackServer::bind().await.unwrap();
         let opener = |_: &str| {};
         let err = login_with(

src/lib.rs

@@ -7,6 +7,7 @@ pub mod config;
 pub mod error;
 pub mod generation;
 pub mod model_cache;
+pub mod net;
 pub mod runner;
 pub mod tailoring;
 #[cfg(feature = "telemetry")]

src/model_cache.rs

@@ -153,8 +153,7 @@ pub(crate) async fn fetch_openai_models_async(
 ) -> Result<Vec<String>, 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<Vec<String>, 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)

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"));
+    }
+}

src/runner/anthropic_api.rs

@@ -81,9 +81,7 @@ impl AnthropicApiRunner {
         base_url: String,
         max_tokens: u32,
     ) -> Result<Self, ActualError> {
-        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)

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)