From 54685be9a22df582e4dac14a3ffe1468d87f0400 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Wed, 12 Aug 2026 10:33:03 -0600 Subject: [PATCH] fix: keep loopback and LAN traffic off an ambient proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-existing on main, not introduced by the driver work, but it makes a local RAG backend unusable so it belongs with this change. build_client only called set_proxy when a client had configured one of its own. With nothing configured, reqwest's own detection applied, which sends every request through a *_PROXY variable including ones bound for 127.0.0.1 or a LAN address. A proxy cannot usefully forward those, and anything that intercepts proxied traffic answers on behalf of a service that is running perfectly well, so the error names the proxy rather than the store and reads as a Coyote fault. Concretely, an installed Socket Firewall exports HTTP_PROXY to the processes it wraps and rejects hosts outside its allow list. That turned a healthy Ollama on the LAN into 'error decoding response body: expected value at line 2 column 1' — its HTML refusal page parsed as JSON — and a loopback Qdrant into an HTTP 405. Proxy handling is now always applied and always exempts loopback and private ranges, with NO_PROXY merged in since replacing reqwest's detection also replaces its handling of that variable. HTTP_PROXY and HTTPS_PROXY are kept separate because they are allowed to differ. An explicitly configured proxy still wins, and '-' still means none. This also supersedes the unconditional no_proxy() added to the Qdrant client in af9622d: that made it the only client to ignore a proxy outright, on a justification I got wrong. It now shares this path, so a remote store behind a real proxy keeps working. --- src/client/common.rs | 4 +- src/rag/providers/qdrant.rs | 5 +- src/utils/mod.rs | 118 ++++++++++++++++++++++++++++++++++-- 3 files changed, 115 insertions(+), 12 deletions(-) diff --git a/src/client/common.rs b/src/client/common.rs index d927440..8b4fe84 100644 --- a/src/client/common.rs +++ b/src/client/common.rs @@ -56,9 +56,7 @@ pub trait Client: Sync + Send { let mut builder = ReqwestClient::builder(); let extra = self.extra_config(); let timeout = extra.and_then(|v| v.connect_timeout).unwrap_or(10); - if let Some(proxy) = extra.and_then(|v| v.proxy.as_deref()) { - builder = set_proxy(builder, proxy)?; - } + builder = apply_proxy(builder, extra.and_then(|v| v.proxy.as_deref()))?; if let Some(user_agent) = self.app_config().user_agent.as_ref() { builder = builder.user_agent(user_agent); } diff --git a/src/rag/providers/qdrant.rs b/src/rag/providers/qdrant.rs index 7dac07e..29f5690 100644 --- a/src/rag/providers/qdrant.rs +++ b/src/rag/providers/qdrant.rs @@ -1,5 +1,6 @@ use crate::rag::provider::RagProvider; use crate::rag::{DocumentId, RagData}; +use crate::utils::apply_proxy; use anyhow::{Context, Result, bail}; use async_trait::async_trait; @@ -191,9 +192,7 @@ impl QdrantProvider { value.set_sensitive(true); headers.insert("api-key", value); } - Client::builder() - .default_headers(headers) - .no_proxy() + apply_proxy(Client::builder().default_headers(headers), None)? .build() .context("Failed to build reqwest client") } diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 428e882..2e8dc5b 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -295,18 +295,83 @@ pub fn is_url(path: &str) -> bool { path.starts_with("http://") || path.starts_with("https://") } -pub fn set_proxy( +/// 127.0.0.1 means something different to a proxy than it does to us, so a tool +/// that intercepts proxied traffic answers for a local service that is running +/// fine, and the failure reads as a fault in Coyote or in that service. +const LOCAL_NO_PROXY: &str = + "localhost,127.0.0.0/8,::1,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,.local"; + +/// Applies proxy settings, keeping local traffic direct. `configured` is a +/// client's own `extra.proxy`, where `"-"` means no proxy at all; with nothing +/// configured an ambient `*_PROXY` is still honoured for public hosts. +pub fn apply_proxy( mut builder: reqwest::ClientBuilder, - proxy: &str, + configured: Option<&str>, ) -> Result { + // reqwest offers no way to add rules to the proxies it auto-detects, so + // detection is disabled and redone below. builder = builder.no_proxy(); - if !proxy.is_empty() && proxy != "-" { - builder = builder - .proxy(reqwest::Proxy::all(proxy).with_context(|| format!("Invalid proxy `{proxy}`"))?); - }; + + let configured = configured.map(str::trim).filter(|p| !p.is_empty()); + if configured == Some("-") { + return Ok(builder); + } + let exempt = no_proxy_rules(); + if let Some(url) = configured { + let proxy = reqwest::Proxy::all(url) + .with_context(|| format!("Invalid proxy `{url}`"))? + .no_proxy(reqwest::NoProxy::from_string(&exempt)); + return Ok(builder.proxy(proxy)); + } + + // Split per scheme, because HTTP_PROXY and HTTPS_PROXY are allowed to differ. + for (is_https, keys) in [ + ( + true, + ["HTTPS_PROXY", "https_proxy", "ALL_PROXY", "all_proxy"], + ), + ( + false, + ["HTTP_PROXY", "http_proxy", "ALL_PROXY", "all_proxy"], + ), + ] { + let Some(url) = first_env(&keys) else { + continue; + }; + let proxy = if is_https { + reqwest::Proxy::https(&url) + } else { + reqwest::Proxy::http(&url) + }; + let proxy = proxy + .with_context(|| format!("Invalid proxy `{url}`"))? + .no_proxy(reqwest::NoProxy::from_string(&exempt)); + builder = builder.proxy(proxy); + } Ok(builder) } +fn first_env(keys: &[&str]) -> Option { + keys.iter() + .find_map(|key| env::var(key).ok()) + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) +} + +/// Replacing reqwest's auto-detection loses its `NO_PROXY` handling, so that is +/// merged back in here. +fn no_proxy_rules() -> String { + let mut rules = LOCAL_NO_PROXY.to_string(); + let extra = env::var("NO_PROXY") + .or_else(|_| env::var("no_proxy")) + .unwrap_or_default(); + if !extra.trim().is_empty() { + rules.push(','); + rules.push_str(extra.trim()); + } + rules +} + pub fn decode_bin(data: &[u8]) -> Result { let (v, _) = bincode::serde::decode_from_slice(data, bincode::config::legacy())?; Ok(v) @@ -316,6 +381,47 @@ pub fn decode_bin(data: &[u8]) -> Result { mod tests { use super::*; + /// Invalid rule syntax makes `from_string` return `None`, which silently drops + /// every exemption and sends local traffic back through the proxy. + #[test] + fn the_local_no_proxy_rules_are_valid() { + assert!( + reqwest::NoProxy::from_string(LOCAL_NO_PROXY).is_some(), + "reqwest rejected LOCAL_NO_PROXY, so nothing would be exempt" + ); + } + + #[test] + fn local_rules_cover_loopback_and_private_ranges() { + for host in [ + "localhost", + "127.0.0.0/8", + "10.0.0.0/8", + "172.16.0.0/12", + "192.168.0.0/16", + ] { + assert!( + LOCAL_NO_PROXY.contains(host), + "{host} must stay exempt from proxying" + ); + } + } + + #[test] + fn a_dash_means_no_proxy_at_all() { + assert!(apply_proxy(reqwest::ClientBuilder::new(), Some("-")).is_ok()); + assert!(apply_proxy(reqwest::ClientBuilder::new(), Some(" - ")).is_ok()); + } + + #[test] + fn an_unparseable_proxy_is_reported() { + let err = apply_proxy(reqwest::ClientBuilder::new(), Some("not a url")) + .unwrap_err() + .to_string(); + + assert!(err.contains("Invalid proxy"), "got: {err}"); + } + #[test] #[cfg(not(target_os = "windows"))] fn test_safe_join_path() {