fix(web-search): resolve the SearXNG base URL host instead of prefix-matching it

crates/genie-core/src/security/sandbox.rs

@@ -226,14 +226,32 @@ fn find_secret_matches(text: &str, pattern: &SecretPattern) -> Vec<String> {
     matches
 }
 
-fn extract_host(url: &str) -> String {
+/// Extract the lowercased host from a URL: strip the scheme, take the
+/// authority, drop any `user[:pass]@` userinfo, and keep IPv6 literals bracketed.
+///
+/// `pub(crate)` so every on-device-only guard in this crate classifies a URL the
+/// same way. `tools::web_search` used to prefix-match its base URL instead, which
+/// let `http://localhost.attacker.example` and `http://127.0.0.1@evil.com` pass
+/// as loopback.
+pub(crate) fn extract_host(url: &str) -> String {
     let url = url.trim();
     let stripped = url
         .strip_prefix("http://")
         .or_else(|| url.strip_prefix("https://"))
         .unwrap_or(url);
 
-    let authority = stripped.split('/').next().unwrap_or(stripped);
+    // The authority ends at the first '/', '?', '#', or '\'. Splitting on '/'
+    // alone left a query or fragment attached, and the userinfo strip below then
+    // read *into* it: "http://evil.com#@127.0.0.1" and "http://evil.com?@127.0.0.1"
+    // both yielded the host "127.0.0.1", so a remote endpoint passed both the
+    // inference-route SSRF check and the web-search egress guard. A backslash is
+    // included because WHATWG treats it as a path separator for http(s), which is
+    // how reqwest's URL parser resolves it — "http://127.0.0.1\@evil.com" really
+    // does connect to 127.0.0.1, and the old split reported "evil.com".
+    let authority = stripped
+        .split(['/', '?', '#', '\\'])
+        .next()
+        .unwrap_or(stripped);
     let host_port = authority.rsplit('@').next().unwrap_or(authority);
     let host = if let Some(rest) = host_port.strip_prefix('[') {
         rest.find(']')
@@ -247,7 +265,10 @@ fn extract_host(url: &str) -> String {
 
 /// True when `host` is a literal loopback target (not a hostname that merely
 /// starts with a loopback-looking prefix).
-fn is_loopback_host(host: &str) -> bool {
+///
+/// `pub(crate)` alongside [`extract_host`] so sibling guards reuse this parse
+/// rather than growing another prefix-matching copy of it.
+pub(crate) fn is_loopback_host(host: &str) -> bool {
     let host = host.trim();
     if host.is_empty() {
         return false;
@@ -285,6 +306,30 @@ mod tests {
         assert!(validate_inference_route("http://localhost.evil.com:8080/v1").is_err());
     }
 
+    #[test]
+    fn reject_remote_host_hidden_behind_a_query_fragment_or_backslash() {
+        // The authority used to be split on '/' only, so a query or fragment
+        // stayed attached and the userinfo strip read into it — the '@' inside
+        // the fragment made "127.0.0.1" look like the host. Both the SSRF check
+        // here and the web-search egress guard accepted these.
+        assert!(validate_inference_route("http://evil.com#@127.0.0.1").is_err());
+        assert!(validate_inference_route("http://evil.com?@127.0.0.1").is_err());
+        assert!(validate_inference_route("http://evil.com\\@127.0.0.1").is_err());
+        assert!(validate_inference_route("http://evil.com?next=@localhost").is_err());
+        assert_eq!(extract_host("http://evil.com#@127.0.0.1"), "evil.com");
+
+        // A backslash is a path separator for http(s) under WHATWG, which is how
+        // reqwest resolves it: this really does connect to 127.0.0.1, so it must
+        // be accepted rather than misread as "evil.com" and rejected.
+        assert!(validate_inference_route("http://127.0.0.1\\@evil.com").is_ok());
+        assert_eq!(extract_host("http://127.0.0.1\\@evil.com"), "127.0.0.1");
+
+        // Genuine userinfo in the authority still resolves to the real host.
+        assert!(validate_inference_route("http://user:pass@[::1]:8080").is_ok());
+        assert!(validate_inference_route("http://user@localhost").is_ok());
+        assert!(validate_inference_route("http://localhost:1@evil.com/v1").is_err());
+    }
+
     #[test]
     fn reject_remote_routes() {
         assert!(validate_inference_route("http://api.openai.com/v1").is_err());

crates/genie-core/src/tools/web_search.rs

@@ -399,14 +399,31 @@ fn searxng_search_url(base_url: &str) -> String {
     }
 }
 
+/// True when the configured SearXNG base URL points at this device.
+///
+/// This gates `allow_remote_base_url`: when the user has not opted in, every
+/// household search query must stay on-device. The check therefore has to
+/// resolve the URL's *host*, not the shape of its first few characters.
+///
+/// It used to prefix-match (`starts_with("http://localhost")`, `"http://127."`),
+/// which was wrong in both directions:
+///
+/// - Remote hosts passed as local, so queries left the device with the opt-in
+///   still off — `http://localhost.attacker.example`, `http://127.evil.example`,
+///   `https://127.0.0.1.nip.io`, and `http://127.0.0.1@evil.com`, where the
+///   loopback text is userinfo and the real host is `evil.com`.
+/// - Genuine loopback URLs were rejected, so a legitimate local SearXNG behind
+///   credentials (`http://user@localhost:8888`, `http://user:pass@[::1]:8888`)
+///   could not be used at all.
+///
+/// `security::sandbox` already parses hosts correctly for `validate_inference_route`
+/// and documents itself as matching "a literal loopback target (not a hostname
+/// that merely starts with a loopback-looking prefix)". Delegate to it instead of
+/// keeping a third, weaker copy of the same decision — `genie-common::config`
+/// hardened its own copy after this exact class regressed once already (#327).
 fn is_local_base_url(base_url: &str) -> bool {
-    let lower = base_url.trim().to_lowercase();
-    lower.starts_with("http://127.")
-        || lower.starts_with("http://localhost")
-        || lower.starts_with("http://[::1]")
-        || lower.starts_with("https://127.")
-        || lower.starts_with("https://localhost")
-        || lower.starts_with("https://[::1]")
+    let host = crate::security::sandbox::extract_host(base_url);
+    crate::security::sandbox::is_loopback_host(&host)
 }
 
 pub(crate) fn format_results(query: &str, body: &str, limit: usize) -> Result<String> {
@@ -724,6 +741,48 @@ mod tests {
         assert!(!is_local_base_url("https://searx.example.com"));
     }
 
+    #[test]
+    fn local_base_url_rejects_hosts_that_merely_look_loopback() {
+        // The prefix match classified all of these as on-device, so household
+        // search queries were sent to a remote SearXNG with
+        // allow_remote_base_url still false. The last one is the userinfo case:
+        // the loopback text is credentials, the real host is evil.com.
+        for url in [
+            "http://localhost.attacker.example/search",
+            "http://localhost-evil.example",
+            "http://127.evil.example/",
+            "https://127.0.0.1.nip.io/",
+            "http://127.0.0.1@evil.com/search",
+            "http://127.0.0.1:8888@evil.com",
+            "https://searx.example.com",
+            // The loopback text sits in a query, fragment, or backslash-delimited
+            // path — the authority is still evil.com.
+            "http://evil.com#@127.0.0.1",
+            "http://evil.com?@127.0.0.1",
+            "http://evil.com\\@127.0.0.1",
+        ] {
+            assert!(!is_local_base_url(url), "{url:?} must not count as local");
+        }
+    }
+
+    #[test]
+    fn local_base_url_accepts_loopback_behind_userinfo_and_the_whole_127_range() {
+        // The prefix match rejected these, so a legitimate on-device SearXNG
+        // behind credentials could not be used at all — the guard failed closed
+        // on the very configuration it exists to permit.
+        for url in [
+            "http://user@localhost:8888",
+            "http://user:pass@[::1]:8888",
+            // 127.0.0.0/8 is entirely loopback, not just 127.0.0.1 (#327).
+            "http://127.0.0.2:8888",
+            "http://127.1.2.3:9090/search",
+            // Casing and a trailing path must not matter.
+            "http://LocalHost:8888/search",
+        ] {
+            assert!(is_local_base_url(url), "{url:?} must count as local");
+        }
+    }
+
     #[test]
     fn cache_query_normalization_is_stable() {
         assert_eq!(