Skip to content

Commit 26d6ff5

Browse files
fix(provider-utils): scope download credentials and pin per-URL, not per-config
Two trust-boundary gaps in the SSRF guard, both reachable: 1. The first request sent caller headers (which may carry the provider API key) to whatever public host the response body named; cross-origin clearing only ran after a redirect. Now credentials are sent only when the target is on the configured base_url's origin or a same-scheme sibling under its parent domain (api.us1.bfl.ai for api.bfl.ai), gated before the first request and derived from config, never the response. 2. proxy_in_use() was a global boolean, so a request that a proxy would actually send DIRECT (NO_PROXY match, or HTTP-only proxy for an HTTPS URL) skipped DNS pinning and reopened the rebinding hole. The pinned client now applies the proxy config and lets reqwest decide per URL: proxied requests are resolved by that trusted transport, direct ones stay pinned to the validated addresses. Also allow clippy 1.98's chunks_exact_to_as_chunks in hash.rs and openai/embedding.rs, and result_large_err in ws.rs (suggested APIs exceed the 1.85 MSRV / boxing an immediately-consumed internal error).
1 parent cb7de4c commit 26d6ff5

5 files changed

Lines changed: 120 additions & 34 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1818
connection pinned to the validated addresses (defeating TTL-0 DNS
1919
rebinding), per-hop re-validation of redirects, rejection of redirects to
2020
non-HTTP(S) schemes, and sanitization of hop-by-hop/forwarding/metadata
21-
headers. URLs same-origin with the configured `base_url` are exempt so
22-
self-hosted deployments keep working; when a proxy is configured the
23-
proxied client is used (pinning is impossible through a proxy) while URL
24-
and DNS validation still apply. Wired into every response-body-URL fetch:
21+
headers. Caller headers (which may carry the provider API key) are sent
22+
only when the target is on the configured `base_url`'s origin or a sibling
23+
host under its parent domain, so a response cannot point credentials at an
24+
arbitrary public host. URLs same-origin with the configured `base_url` are
25+
exempt from address validation so self-hosted deployments keep working;
26+
proxy routing is decided per URL by reqwest itself — requests a proxy
27+
carries are resolved by that trusted transport, while requests that go
28+
direct (NO_PROXY match, or no proxy for the scheme) stay pinned to the
29+
validated addresses. Wired into every response-body-URL fetch:
2530
Black Forest Labs (polling + download), Gladia (result polling), Luma,
2631
Recraft, Replicate, and fal image downloads, and the Google Files upload
2732
URL.

‎aimux-provider-utils/src/download_guard.rs‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,44 @@ pub(crate) fn hop_trusted_origin<'a>(
3333
trusted_origin.filter(|origin| same_origin(current_url, origin))
3434
}
3535

36+
/// Whether caller headers (which may carry credentials) may be sent to `url`:
37+
/// same origin as the trusted origin, or a same-scheme sibling host under the
38+
/// trusted origin's parent domain — providers routinely serve polling URLs
39+
/// from regional hosts (api.us1.bfl.ai for a base of api.bfl.ai). The parent
40+
/// must keep at least two labels so an apex base never widens the trust to a
41+
/// public suffix, and trust always derives from the configured origin, never
42+
/// from the response.
43+
pub(crate) fn credential_eligible(url: &str, trusted_origin: Option<&str>) -> bool {
44+
let Some(origin) = trusted_origin else {
45+
return false;
46+
};
47+
if same_origin(url, origin) {
48+
return true;
49+
}
50+
let (Ok(url), Ok(origin)) = (url::Url::parse(url), url::Url::parse(origin)) else {
51+
return false;
52+
};
53+
if url.scheme() != origin.scheme() {
54+
return false;
55+
}
56+
let (Some(host), Some(origin_host)) = (url.host_str(), origin.host_str()) else {
57+
return false;
58+
};
59+
// IP-literal origins have no domain family; only an exact origin match
60+
// (handled above) qualifies. Otherwise "127.0.0.1" would parse a bogus
61+
// "0.0.1" parent and treat every 127.* port as a sibling.
62+
if origin_host.parse::<IpAddr>().is_ok() {
63+
return false;
64+
}
65+
let Some((_, parent)) = origin_host.split_once('.') else {
66+
return false;
67+
};
68+
parent.contains('.')
69+
&& host.len() > parent.len()
70+
&& host.ends_with(parent)
71+
&& host.as_bytes()[host.len() - parent.len() - 1] == b'.'
72+
}
73+
3674
fn without_query(parsed: &url::Url) -> String {
3775
let mut redacted = parsed.clone();
3876
redacted.set_query(None);
@@ -406,6 +444,49 @@ mod tests {
406444
assert!(addresses.is_empty());
407445
}
408446

447+
#[test]
448+
fn credentials_stay_within_the_trusted_origin_family() {
449+
let base = Some("https://api.bfl.ai");
450+
// Same origin and same-parent-domain siblings are eligible.
451+
assert!(credential_eligible(
452+
"https://api.bfl.ai/v1/get_result",
453+
base
454+
));
455+
assert!(credential_eligible(
456+
"https://api.us1.bfl.ai/v1/get_result",
457+
base
458+
));
459+
// Foreign hosts, lookalike suffixes, scheme downgrades, and
460+
// response-chosen public hosts are not.
461+
assert!(!credential_eligible("https://attacker.example/poll", base));
462+
assert!(!credential_eligible("https://evil-bfl.ai/poll", base));
463+
assert!(!credential_eligible("http://api.us1.bfl.ai/poll", base));
464+
assert!(!credential_eligible("https://api.bfl.ai/x", None));
465+
// An apex base must not widen trust to the entire public suffix.
466+
assert!(!credential_eligible(
467+
"https://other.ai/poll",
468+
Some("https://bfl.ai")
469+
));
470+
// A deeper base widens only to its own organization's domain.
471+
assert!(credential_eligible(
472+
"https://cdn.example.co.uk/file",
473+
Some("https://api.example.co.uk")
474+
));
475+
assert!(!credential_eligible(
476+
"https://evil.co.uk/file",
477+
Some("https://api.example.co.uk")
478+
));
479+
// An IP-literal origin has no domain family: only the exact origin.
480+
assert!(credential_eligible(
481+
"http://127.0.0.1:8080/file",
482+
Some("http://127.0.0.1:8080")
483+
));
484+
assert!(!credential_eligible(
485+
"http://127.0.0.1:9090/file",
486+
Some("http://127.0.0.1:8080")
487+
));
488+
}
489+
409490
#[test]
410491
fn trusted_origin_does_not_extend_to_foreign_hops() {
411492
let trusted = Some("http://localhost:43123");

‎aimux-provider-utils/src/http.rs‎

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -270,10 +270,16 @@ fn pinned_client(url: &str, addresses: &[std::net::IpAddr]) -> Result<Client, Ai
270270
.ok_or_else(|| AiMuxError::Other("download url has no host".to_string()))?
271271
.to_string();
272272
let port = parsed.port_or_known_default().unwrap_or(80);
273-
let mut b = client_builder(&PoolConfig::default(), &TimeoutConfig::default(), false)
274-
// A proxy resolves and connects to the target independently, which
275-
// would bypass the validated address binding below.
276-
.no_proxy();
273+
// The proxy configuration is applied so reqwest makes the per-URL
274+
// routing decision itself: when a proxy carries the request the proxy
275+
// resolves the target (a trusted transport, the override below is
276+
// unused), and any request the proxy rules send DIRECT — a NO_PROXY
277+
// match, or no proxy configured for the URL's scheme — still connects
278+
// only through the validated, pinned addresses.
279+
let mut b = apply_proxy(
280+
client_builder(&PoolConfig::default(), &TimeoutConfig::default(), false),
281+
&global_proxy(),
282+
);
277283
let socket_addresses: Vec<_> = addresses
278284
.iter()
279285
.map(|address| std::net::SocketAddr::new(*address, port))
@@ -284,25 +290,6 @@ fn pinned_client(url: &str, addresses: &[std::net::IpAddr]) -> Result<Client, Ai
284290
})
285291
}
286292

287-
/// Whether any proxy could carry outbound traffic: an explicit `init_proxy`
288-
/// configuration, or the environment variables reqwest reads by default.
289-
fn proxy_in_use() -> bool {
290-
let proxy = global_proxy();
291-
proxy.http_url.is_some()
292-
|| proxy.https_url.is_some()
293-
|| proxy.all_url.is_some()
294-
|| [
295-
"HTTPS_PROXY",
296-
"https_proxy",
297-
"HTTP_PROXY",
298-
"http_proxy",
299-
"ALL_PROXY",
300-
"all_proxy",
301-
]
302-
.iter()
303-
.any(|name| std::env::var_os(name).is_some_and(|value| !value.is_empty()))
304-
}
305-
306293
/// Apply proxy configuration to a reqwest client builder (by-value chain).
307294
fn apply_proxy(b: reqwest::ClientBuilder, proxy: &ProxyConfig) -> reqwest::ClientBuilder {
308295
use reqwest::Proxy as ReqwestProxy;
@@ -1517,19 +1504,25 @@ async fn send_validated_redirects(
15171504
) -> Result<reqwest::Response, AiMuxError> {
15181505
let mut current = request.clone();
15191506
crate::download_guard::sanitize_download_headers(&mut current.headers);
1507+
// Caller headers may carry credentials (providers poll response-supplied
1508+
// URLs with their API key); a response must not be able to point them at
1509+
// an arbitrary public host. Trust derives from the configured origin, so
1510+
// anything outside its domain gets only a User-Agent.
1511+
if !crate::download_guard::credential_eligible(&current.url, trusted_origin) {
1512+
crate::download_guard::retain_user_agent(&mut current.headers);
1513+
}
15201514
let mut pinned =
15211515
crate::download_guard::validate_download_target(&current.url, trusted_origin).await?;
15221516
for redirect_count in 0..=10 {
15231517
let hop_client;
1524-
let client: &Client = if pinned.is_empty() || proxy_in_use() {
1525-
// Empty pins mean the hop is on the trusted origin. A proxy
1526-
// resolves and connects to the target itself, so pinning is
1527-
// structurally impossible through one; the URL and its DNS
1528-
// answers were still validated above.
1518+
let client: &Client = if pinned.is_empty() {
1519+
// Empty pins mean the hop is on the trusted origin.
15291520
download_client()?
15301521
} else {
1531-
// DNS answers were validated; pin them so the actual connection
1532-
// can only reach the addresses that passed the guard.
1522+
// DNS answers were validated; pin them so a direct connection
1523+
// can only reach the addresses that passed the guard. When a
1524+
// configured proxy carries the request instead, the proxy
1525+
// resolves the target and the pin is deliberately unused.
15331526
hop_client = pinned_client(&current.url, &pinned)?;
15341527
&hop_client
15351528
};

‎aimux-provider-utils/src/ws.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ enum ConnectError {
8686
Tungstenite(tokio_tungstenite::tungstenite::Error),
8787
}
8888

89+
// Clippy 1.98 flags the 136-byte tungstenite error variant; this internal
90+
// error is consumed immediately by the one caller, so boxing buys nothing.
91+
#[allow(clippy::result_large_err)]
8992
async fn connect_with_timeout(
9093
request: tokio_tungstenite::tungstenite::http::Request<()>,
9194
timeout: Option<std::time::Duration>,

‎aimux-providers/src/openai/embedding.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,10 @@ fn decode_base64_embedding(s: &str) -> Option<Vec<f32>> {
220220
if bytes.len() % 4 != 0 {
221221
return None;
222222
}
223+
// Clippy 1.98 suggests `as_chunks::<4>()`, which is stable only since
224+
// Rust 1.88; the workspace MSRV is 1.85. `unknown_lints` keeps this
225+
// buildable on toolchains older than the lint itself.
226+
#[allow(unknown_lints, clippy::chunks_exact_to_as_chunks)]
223227
Some(
224228
bytes
225229
.chunks_exact(4)

0 commit comments

Comments
 (0)