fix: keep loopback and LAN traffic off an ambient proxy
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.
This commit is contained in:
@@ -56,9 +56,7 @@ pub trait Client: Sync + Send {
|
|||||||
let mut builder = ReqwestClient::builder();
|
let mut builder = ReqwestClient::builder();
|
||||||
let extra = self.extra_config();
|
let extra = self.extra_config();
|
||||||
let timeout = extra.and_then(|v| v.connect_timeout).unwrap_or(10);
|
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 = apply_proxy(builder, extra.and_then(|v| v.proxy.as_deref()))?;
|
||||||
builder = set_proxy(builder, proxy)?;
|
|
||||||
}
|
|
||||||
if let Some(user_agent) = self.app_config().user_agent.as_ref() {
|
if let Some(user_agent) = self.app_config().user_agent.as_ref() {
|
||||||
builder = builder.user_agent(user_agent);
|
builder = builder.user_agent(user_agent);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
use crate::rag::provider::RagProvider;
|
use crate::rag::provider::RagProvider;
|
||||||
use crate::rag::{DocumentId, RagData};
|
use crate::rag::{DocumentId, RagData};
|
||||||
|
use crate::utils::apply_proxy;
|
||||||
|
|
||||||
use anyhow::{Context, Result, bail};
|
use anyhow::{Context, Result, bail};
|
||||||
use async_trait::async_trait;
|
use async_trait::async_trait;
|
||||||
@@ -191,9 +192,7 @@ impl QdrantProvider {
|
|||||||
value.set_sensitive(true);
|
value.set_sensitive(true);
|
||||||
headers.insert("api-key", value);
|
headers.insert("api-key", value);
|
||||||
}
|
}
|
||||||
Client::builder()
|
apply_proxy(Client::builder().default_headers(headers), None)?
|
||||||
.default_headers(headers)
|
|
||||||
.no_proxy()
|
|
||||||
.build()
|
.build()
|
||||||
.context("Failed to build reqwest client")
|
.context("Failed to build reqwest client")
|
||||||
}
|
}
|
||||||
|
|||||||
+111
-5
@@ -295,18 +295,83 @@ pub fn is_url(path: &str) -> bool {
|
|||||||
path.starts_with("http://") || path.starts_with("https://")
|
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,
|
mut builder: reqwest::ClientBuilder,
|
||||||
proxy: &str,
|
configured: Option<&str>,
|
||||||
) -> Result<reqwest::ClientBuilder> {
|
) -> Result<reqwest::ClientBuilder> {
|
||||||
|
// reqwest offers no way to add rules to the proxies it auto-detects, so
|
||||||
|
// detection is disabled and redone below.
|
||||||
builder = builder.no_proxy();
|
builder = builder.no_proxy();
|
||||||
if !proxy.is_empty() && proxy != "-" {
|
|
||||||
builder = builder
|
let configured = configured.map(str::trim).filter(|p| !p.is_empty());
|
||||||
.proxy(reqwest::Proxy::all(proxy).with_context(|| format!("Invalid proxy `{proxy}`"))?);
|
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)
|
Ok(builder)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn first_env(keys: &[&str]) -> Option<String> {
|
||||||
|
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<T: serde::de::DeserializeOwned>(data: &[u8]) -> Result<T> {
|
pub fn decode_bin<T: serde::de::DeserializeOwned>(data: &[u8]) -> Result<T> {
|
||||||
let (v, _) = bincode::serde::decode_from_slice(data, bincode::config::legacy())?;
|
let (v, _) = bincode::serde::decode_from_slice(data, bincode::config::legacy())?;
|
||||||
Ok(v)
|
Ok(v)
|
||||||
@@ -316,6 +381,47 @@ pub fn decode_bin<T: serde::de::DeserializeOwned>(data: &[u8]) -> Result<T> {
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
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]
|
#[test]
|
||||||
#[cfg(not(target_os = "windows"))]
|
#[cfg(not(target_os = "windows"))]
|
||||||
fn test_safe_join_path() {
|
fn test_safe_join_path() {
|
||||||
|
|||||||
Reference in New Issue
Block a user