From 8681eefeed15dcc1e99cc130cf721b2ea68407a8 Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 12:48:36 +0100 Subject: [PATCH 01/13] refactor(callouts): standardize target and credential policy Signed-off-by: Eoin Fennessy rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED --- apis/src/callout_target.rs | 245 ++++++++++++++++++ apis/src/lib.rs | 1 + apis/src/openai/api_client/mod.rs | 63 ++++- apis/src/openai/api_client/url.rs | 148 +---------- apis/src/openai/responses/compact/config.rs | 19 +- apis/src/openai/responses/compact/mod.rs | 1 + apis/src/openai/responses/compact/tests.rs | 13 +- .../openai/responses/file_resolve/config.rs | 8 +- apis/src/openai/responses/file_resolve/mod.rs | 3 + .../openai/responses/file_resolve/resolve.rs | 1 + .../openai/responses/file_resolve/tests.rs | 1 + .../responses/file_search_callout/config.rs | 11 +- .../responses/file_search_callout/tests.rs | 8 +- apis/src/subrequest.rs | 167 ++++++++++-- apis/src/web_search/config.rs | 23 +- apis/src/web_search/provider.rs | 28 +- docs/README.md | 1 + docs/architecture/outbound-callouts.md | 60 +++++ docs/filters/ai_guardrails.md | 1 + docs/filters/anthropic_web_search.md | 2 +- docs/filters/azure_ad.md | 1 + docs/filters/gcp_adc.md | 2 + docs/filters/http_callout.md | 2 +- docs/filters/openai_file_resolve.md | 2 +- docs/filters/openai_file_search_callout.md | 2 +- docs/filters/openai_responses_compact.md | 1 + docs/filters/openai_web_search.md | 2 +- examples/configs/azure-ad.yaml | 1 + examples/configs/gcp-adc.yaml | 2 + .../inference/fallback-with-translation.yaml | 4 +- examples/configs/lakera-guard.yaml | 5 +- examples/configs/nemo-guardrails.yaml | 3 + .../configs/openai/responses/compact.yaml | 3 + .../configs/openai/responses/web-search.yaml | 12 +- filters/src/azure/azure_ad.rs | 76 +++++- filters/src/callout/config.rs | 14 +- filters/src/callout/mod.rs | 187 ++----------- filters/src/callout/tests.rs | 156 ++++++----- filters/src/gcp/filter.rs | 21 +- filters/src/gcp/token.rs | 42 +++ filters/src/guardrails/filter.rs | 2 + filters/src/guardrails/providers/nemo.rs | 28 +- filters/src/guardrails/tests.rs | 18 ++ .../tests/suite/examples/azure_ad.rs | 2 +- .../tests/suite/examples/lakera_guard.rs | 20 +- 45 files changed, 917 insertions(+), 495 deletions(-) create mode 100644 apis/src/callout_target.rs create mode 100644 docs/architecture/outbound-callouts.md diff --git a/apis/src/callout_target.rs b/apis/src/callout_target.rs new file mode 100644 index 0000000000..a0ffc084a8 --- /dev/null +++ b/apis/src/callout_target.rs @@ -0,0 +1,245 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (c) 2026 Praxis Contributors + +//! Shared target and address policy for outbound HTTP callouts. +//! +//! Operator-configured targets are syntax-checked when a pipeline is built +//! and their address policy is enforced again after DNS resolution, directly +//! before the validated socket addresses are handed to the transport. This +//! closes the DNS-rebinding gap left by startup-only URL validation. + +use std::net::{IpAddr, SocketAddr}; + +use praxis_core::connectivity::normalize_mapped_ipv4; +use praxis_filter::FilterError; + +use crate::openai::url_security::is_non_public_ip; + +/// Whether a configured callout may connect to non-public addresses. +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub enum AddressPolicy { + /// Only publicly routable addresses are accepted. + #[default] + PublicOnly, + /// Private, loopback, link-local, and other non-public addresses are + /// accepted because the operator explicitly opted in. + AllowPrivate, +} + +impl AddressPolicy { + /// Build a policy from a filter's explicit private-target opt-in. + #[must_use] + pub const fn from_allow_private(allow_private: bool) -> Self { + if allow_private { + Self::AllowPrivate + } else { + Self::PublicOnly + } + } + + /// Return whether non-public addresses are allowed. + #[must_use] + pub const fn allows_private(self) -> bool { + matches!(self, Self::AllowPrivate) + } +} + +/// Validate an operator-configured HTTP target's URL structure. +/// +/// Address classification is deliberately deferred until immediately after +/// DNS resolution. Embedded credentials are always rejected, regardless of +/// address policy. +/// +/// # Errors +/// +/// Returns [`FilterError`] when the URL is malformed, is not HTTP(S), lacks a +/// host, contains userinfo, or contains a fragment. +pub fn validate_http_target(filter_name: &str, raw: &str) -> Result { + let parsed = url::Url::parse(raw) + .map_err(|error| -> FilterError { format!("{filter_name}: target URL is not valid: {error}").into() })?; + if !matches!(parsed.scheme(), "http" | "https") { + return Err(format!("{filter_name}: target URL must use http or https").into()); + } + if parsed.host().is_none() { + return Err(format!("{filter_name}: target URL must include a host").into()); + } + if !parsed.username().is_empty() || parsed.password().is_some() { + return Err(format!("{filter_name}: target URL must not contain embedded credentials").into()); + } + if parsed.fragment().is_some() { + return Err(format!("{filter_name}: target URL must not contain a fragment").into()); + } + Ok(parsed) +} + +/// Validate a configured target, including address literals and localhost +/// aliases that can be classified without DNS. +/// +/// # Errors +/// +/// Returns [`FilterError`] for an invalid HTTP target or when a statically +/// classifiable host violates `policy`. +pub fn validate_configured_http_target( + filter_name: &str, + raw: &str, + policy: AddressPolicy, +) -> Result { + let parsed = validate_http_target(filter_name, raw)?; + let host = parsed.host_str().unwrap_or_default(); + let unbracketed = host + .strip_prefix('[') + .and_then(|host| host.strip_suffix(']')) + .unwrap_or(host); + if !policy.allows_private() && unbracketed.trim_end_matches('.').eq_ignore_ascii_case("localhost") { + return Err( + format!("{filter_name}: target URL targets localhost; enable the private-target opt-in to allow").into(), + ); + } + if let Ok(ip) = unbracketed.parse::() { + validate_ip(filter_name, ip, policy)?; + } + Ok(parsed) +} + +/// Validate every address returned by one DNS lookup. +/// +/// The complete answer set is rejected when any address is disallowed. This +/// prevents a mixed public/private DNS response from becoming an address- +/// selection bypass. +/// +/// # Errors +/// +/// Returns [`FilterError`] when the set is empty or any address violates +/// `policy`. +pub fn validate_resolved_addrs( + filter_name: &str, + addrs: &[SocketAddr], + policy: AddressPolicy, +) -> Result, FilterError> { + if addrs.is_empty() { + return Err(format!("{filter_name}: DNS returned no addresses").into()); + } + + let mut validated = Vec::with_capacity(addrs.len()); + let mut seen = std::collections::HashSet::with_capacity(addrs.len()); + for addr in addrs { + let ip = normalize_mapped_ipv4(addr.ip()); + validate_ip(filter_name, ip, policy)?; + let normalized = SocketAddr::new(ip, addr.port()); + if seen.insert(normalized) { + validated.push(normalized); + } + } + Ok(validated) +} + +/// Validate one literal or DNS-resolved address. +/// +/// # Errors +/// +/// Returns [`FilterError`] when `ip` is non-public under +/// [`AddressPolicy::PublicOnly`]. +pub fn validate_ip(filter_name: &str, ip: IpAddr, policy: AddressPolicy) -> Result<(), FilterError> { + let ip = normalize_mapped_ipv4(ip); + if !policy.allows_private() && is_non_public_ip(&ip) { + return Err(format!( + "{filter_name}: target resolved to blocked non-public address {ip}; set the filter's private-target opt-in to true to allow" + ) + .into()); + } + Ok(()) +} + +/// Build a redirect-free, proxy-free `reqwest` client pinned to the address +/// set that passed the shared connect-time policy. +/// +/// This adapter exists for protocol clients that cannot use +/// [`SubRequestClient`](praxis_core::subrequest::SubRequestClient), notably +/// cloud credential endpoints. +/// +/// # Errors +/// +/// Returns [`FilterError`] for invalid targets, failed or timed-out DNS, +/// disallowed resolved addresses, or client construction failure. +#[expect( + clippy::too_many_lines, + reason = "validation, one-time resolution, pinning, and client hardening are one security boundary" +)] +pub async fn build_pinned_reqwest_client( + filter_name: &str, + target: &str, + policy: AddressPolicy, + timeout: std::time::Duration, +) -> Result { + let started = std::time::Instant::now(); + let parsed = validate_configured_http_target(filter_name, target, policy)?; + let host = parsed + .host_str() + .ok_or_else(|| -> FilterError { format!("{filter_name}: target URL must include a host").into() })?; + let port = parsed + .port_or_known_default() + .ok_or_else(|| -> FilterError { format!("{filter_name}: target URL has no usable port").into() })?; + + let mut builder = reqwest::Client::builder() + .no_proxy() + .redirect(reqwest::redirect::Policy::none()); + + let unbracketed = host + .strip_prefix('[') + .and_then(|host| host.strip_suffix(']')) + .unwrap_or(host); + if let Ok(ip) = unbracketed.parse::() { + validate_ip(filter_name, ip, policy)?; + } else { + let resolved = tokio::time::timeout(timeout, tokio::net::lookup_host((host, port))) + .await + .map_err(|_elapsed| -> FilterError { format!("{filter_name}: DNS resolution timed out").into() })? + .map_err(|error| -> FilterError { + format!("{filter_name}: DNS resolution failed for {host}: {error}").into() + })? + .collect::>(); + let validated = validate_resolved_addrs(filter_name, &resolved, policy)?; + builder = builder.resolve_to_addrs(host, &validated); + } + + let remaining = timeout.checked_sub(started.elapsed()).ok_or_else(|| -> FilterError { + format!("{filter_name}: callout deadline exceeded during target resolution").into() + })?; + builder + .timeout(remaining) + .build() + .map_err(|error| format!("{filter_name}: failed to build HTTP client: {error}").into()) +} + +#[cfg(test)] +#[expect(clippy::allow_attributes, reason = "blanket test suppressions")] +#[allow(clippy::unwrap_used, reason = "tests")] +mod tests { + use super::*; + + #[test] + fn target_rejects_userinfo_for_both_policies() { + assert!(validate_http_target("test", "https://user:pass@example.com/path").is_err()); + } + + #[test] + fn mixed_dns_answer_is_rejected() { + let addrs = ["8.8.8.8:443".parse().unwrap(), "169.254.169.254:443".parse().unwrap()]; + assert!(validate_resolved_addrs("test", &addrs, AddressPolicy::PublicOnly).is_err()); + } + + #[test] + fn private_opt_in_accepts_private_answers() { + let addrs = ["127.0.0.1:8080".parse().unwrap(), "10.0.0.1:8080".parse().unwrap()]; + assert_eq!( + validate_resolved_addrs("test", &addrs, AddressPolicy::AllowPrivate).unwrap(), + addrs + ); + } + + #[test] + fn mapped_loopback_is_rejected() { + let addrs = ["[::ffff:127.0.0.1]:80".parse().unwrap()]; + assert!(validate_resolved_addrs("test", &addrs, AddressPolicy::PublicOnly).is_err()); + } +} diff --git a/apis/src/lib.rs b/apis/src/lib.rs index 484eb9e74b..4251e9a49a 100644 --- a/apis/src/lib.rs +++ b/apis/src/lib.rs @@ -11,6 +11,7 @@ pub mod anthropic; pub mod callout_policy; +pub mod callout_target; pub mod classifier; pub mod json_body; pub(crate) mod mcp_client; diff --git a/apis/src/openai/api_client/mod.rs b/apis/src/openai/api_client/mod.rs index 48f520be26..9fa2ce62c7 100644 --- a/apis/src/openai/api_client/mod.rs +++ b/apis/src/openai/api_client/mod.rs @@ -29,7 +29,10 @@ pub(crate) use self::{ error::ApiClientError, url::{resource_url, validate_base_url, validate_forward_headers}, }; -use crate::subrequest::{self, SubRequest, SubRequestClient, SubRequestError, SubResponse}; +use crate::{ + callout_target::AddressPolicy, + subrequest::{self, SubRequest, SubRequestClient, SubRequestError, SubResponse}, +}; /// Configuration for constructing an [`ApiClient`]. /// @@ -46,6 +49,8 @@ pub(crate) struct ApiClientConfig { pub max_response_bytes: usize, /// Header names to forward from the original request. pub forward_header_names: Vec, + /// Connect-time policy for the configured API target. + pub address_policy: AddressPolicy, } /// Shared HTTP client for OpenAI-compatible API callouts. @@ -58,6 +63,8 @@ pub(crate) struct ApiClientConfig { pub(crate) struct ApiClient { /// Base URL of the API endpoint (trailing slash stripped). api_base_url: String, + /// Normalized origin to which forwarded credentials are bound. + target_origin: Option, /// Sub-request client for bounded execution. client: SubRequestClient, /// Per-request timeout. @@ -67,6 +74,8 @@ pub(crate) struct ApiClient { /// Header names to forward from the original downstream /// request. forward_header_names: Vec, + /// Connect-time policy for the configured API target. + address_policy: AddressPolicy, } /// Map a [`SubRequestError`] to an [`ApiClientError`]. @@ -89,14 +98,20 @@ impl ApiClient { timeout, max_response_bytes, forward_header_names, + address_policy, } = config; + let target_origin = ::url::Url::parse(&api_base_url) + .ok() + .map(|url| url.origin().ascii_serialization()); Self { api_base_url: api_base_url.trim_end_matches('/').to_owned(), + target_origin, client, timeout, max_response_bytes, forward_header_names, + address_policy, } } @@ -223,6 +238,16 @@ impl ApiClient { body: Bytes, max_response_bytes: usize, ) -> Result { + let candidate_origin = ::url::Url::parse(url) + .ok() + .map(|url| url.origin().ascii_serialization()); + if candidate_origin.is_none() || candidate_origin != self.target_origin { + return Err(ApiClientError::Transport { + source: SubRequestError::InvalidRequest( + "callout URL changed the configured credential origin".to_owned(), + ), + }); + } let request = SubRequest { method, uri: http::Uri::default(), @@ -230,9 +255,16 @@ impl ApiClient { body, }; - let mut response = subrequest::execute_url(&self.client, url, request, max_response_bytes, self.timeout) - .await - .map_err(map_subrequest_error)?; + let mut response = subrequest::execute_url( + &self.client, + url, + request, + max_response_bytes, + self.timeout, + self.address_policy, + ) + .await + .map_err(map_subrequest_error)?; sanitize_response_headers(&mut response.headers); Ok(response) } @@ -331,6 +363,7 @@ mod tests { timeout: Duration::from_millis(1_000), max_response_bytes: 1_048_576, forward_header_names: Vec::new(), + address_policy: AddressPolicy::AllowPrivate, }) } @@ -351,6 +384,7 @@ mod tests { http::header::AUTHORIZATION, http::HeaderName::from_static("x-tenant-id"), ], + address_policy: AddressPolicy::AllowPrivate, }); let mut request_headers = HeaderMap::new(); @@ -380,6 +414,25 @@ mod tests { assert_eq!(url, "http://ogx:8321/v1/files/file-abc/content"); } + #[tokio::test] + async fn forwarded_credentials_are_bound_to_configured_origin() { + let client = test_client("https://api.example.com"); + let mut headers = HeaderMap::new(); + headers.insert(http::header::AUTHORIZATION, "Bearer secret".parse().unwrap()); + + let error = client + .get("https://attacker.example/v1/files", &headers, 1024) + .await + .expect_err("a derived URL on another origin must be rejected before I/O"); + + assert!(matches!( + error, + ApiClientError::Transport { + source: SubRequestError::InvalidRequest(detail), + } if detail.contains("configured credential origin") + )); + } + #[tokio::test] async fn get_bytes_preserves_redirect_without_following_it() { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); @@ -594,6 +647,7 @@ mod tests { timeout: Duration::from_millis(1_000), max_response_bytes: 1_048_576, forward_header_names: vec![http::header::CONTENT_TYPE], + address_policy: AddressPolicy::AllowPrivate, }); let mut headers = HeaderMap::new(); @@ -824,6 +878,7 @@ mod tests { timeout: Duration::from_millis(50), max_response_bytes: 1_048_576, forward_header_names: Vec::new(), + address_policy: AddressPolicy::AllowPrivate, }); let err = client diff --git a/apis/src/openai/api_client/url.rs b/apis/src/openai/api_client/url.rs index feb413369a..86e8aaca1b 100644 --- a/apis/src/openai/api_client/url.rs +++ b/apis/src/openai/api_client/url.rs @@ -9,17 +9,13 @@ //! Each filter calls the validation helpers during its own config //! validation phase, passing its filter name for error messages. -use std::{ - collections::HashSet, - net::{IpAddr, Ipv4Addr}, -}; +use std::collections::HashSet; use percent_encoding::{AsciiSet, CONTROLS, utf8_percent_encode}; -use praxis_core::connectivity::normalize_mapped_ipv4; use praxis_filter::FilterError; use super::error::ApiClientError; -use crate::openai::url_security::is_non_public_ip; +use crate::callout_target::{AddressPolicy, validate_configured_http_target}; /// Characters that could let a client-supplied resource ID escape /// its single URL path segment. @@ -86,145 +82,19 @@ pub(crate) fn resource_url( /// Validate a base URL against SSRF-sensitive targets. /// /// Checks scheme, embedded credentials, query strings, fragments, -/// and host address. When `allow_private` is `false`, private, -/// loopback, link-local, CGNAT, and DNS-name hosts are rejected. +/// and literal host address. DNS names are accepted because the shared +/// transport validates and pins every resolved address at connect time. /// /// `filter_name` is used as a prefix in error messages so each /// consuming filter reports its own name. pub(crate) fn validate_base_url(filter_name: &str, url: &str, allow_private: bool) -> Result<(), FilterError> { - if url.contains('#') { - return Err(format!("{filter_name}: base URL must not contain a fragment").into()); - } - - let uri: http::Uri = url.parse().map_err(|e: http::uri::InvalidUri| -> FilterError { - format!("{filter_name}: base URL is not valid: {e}").into() - })?; - - match uri.scheme_str() { - Some("http" | "https") => {}, - _ => { - return Err(format!("{filter_name}: base URL must use http or https scheme").into()); - }, - } - - if uri - .authority() - .is_some_and(|authority| authority.as_str().contains('@')) - { - return Err(format!("{filter_name}: base URL must not contain embedded credentials").into()); - } - - if uri.query().is_some() { + let parsed = validate_configured_http_target(filter_name, url, AddressPolicy::from_allow_private(allow_private))?; + if parsed.query().is_some() { return Err(format!("{filter_name}: base URL must not contain a query string").into()); } - - let host = uri - .host() - .ok_or_else(|| -> FilterError { format!("{filter_name}: base URL must include a host").into() })?; - - validate_host(filter_name, host, allow_private) -} - -/// Validate a host value against SSRF-sensitive targets. -fn validate_host(filter_name: &str, host: &str, allow_private: bool) -> Result<(), FilterError> { - if !allow_private && is_localhost_name(host) { - return Err(format!( - "{filter_name}: base URL targets localhost; \ - set the allow-private option to true to allow" - ) - .into()); - } - - let ip_host = host - .strip_prefix('[') - .and_then(|value| value.strip_suffix(']')) - .unwrap_or(host); - if let Ok(ip) = ip_host.parse::() { - validate_ip(filter_name, ip, allow_private)?; - } else if let Some(ip) = parse_legacy_ipv4_host(host) { - validate_ip(filter_name, IpAddr::V4(ip), allow_private)?; - } else { - validate_dns(filter_name, host, allow_private)?; - } - Ok(()) } -/// Validate an IP target against SSRF-sensitive ranges. -fn validate_ip(filter_name: &str, ip: IpAddr, allow_private: bool) -> Result<(), FilterError> { - let ip = normalize_mapped_ipv4(ip); - if !allow_private && is_non_public_ip(&ip) { - return Err(format!( - "{filter_name}: base URL targets a local-sensitive address; \ - set the allow-private option to true to allow" - ) - .into()); - } - Ok(()) -} - -/// Reject DNS hostnames unless private targets are opted in. -fn validate_dns(filter_name: &str, host: &str, allow_private: bool) -> Result<(), FilterError> { - if allow_private { - return Ok(()); - } - Err(format!( - "{filter_name}: base URL host '{host}' is a DNS name; DNS targets are unsupported in \ - protected mode. Use a public IP literal, or set the allow-private option to true, which \ - also permits DNS results resolving to local-sensitive addresses" - ) - .into()) -} - -/// Return whether a host name is a localhost alias. -fn is_localhost_name(host: &str) -> bool { - host.trim_end_matches('.').eq_ignore_ascii_case("localhost") -} - -/// Parse legacy IPv4 literals accepted by common libc resolvers. -fn parse_legacy_ipv4_host(host: &str) -> Option { - let host = host.trim_end_matches('.'); - let parts: Vec<_> = host.split('.').collect(); - if parts.is_empty() || parts.len() > 4 || parts.iter().any(|part| part.is_empty()) { - return None; - } - - let mut numbers = Vec::with_capacity(parts.len()); - for part in parts { - numbers.push(parse_legacy_ipv4_number(part)?); - } - - let addr = match numbers.as_slice() { - [a] => *a, - [a, b] if *a <= 0xFF && *b <= 0x00FF_FFFF => (*a << 24) | *b, - [a, b, c] if *a <= 0xFF && *b <= 0xFF && *c <= 0xFFFF => (*a << 24) | (*b << 16) | *c, - [a, b, c, d] if numbers.iter().all(|part| *part <= 0xFF) => (*a << 24) | (*b << 16) | (*c << 8) | *d, - _ => return None, - }; - - Some(Ipv4Addr::from(addr)) -} - -/// Parse a decimal, octal, or hexadecimal legacy IPv4 component. -fn parse_legacy_ipv4_number(part: &str) -> Option { - let (digits, radix) = part.strip_prefix("0x").or_else(|| part.strip_prefix("0X")).map_or_else( - || { - if part.len() > 1 && part.starts_with('0') { - (part.get(1..).unwrap_or_default(), 8) - } else { - (part, 10) - } - }, - |digits| (digits, 16), - ); - - if digits.is_empty() || !digits.chars().all(|c| c.is_digit(radix)) { - return None; - } - - u32::from_str_radix(digits, radix).ok() -} - // ----------------------------------------------------------------------------- // Forward-header validation // ----------------------------------------------------------------------------- @@ -409,10 +279,10 @@ mod tests { } #[test] - fn ssrf_rejects_dns_name() { + fn ssrf_allows_dns_name_for_connect_time_validation() { assert!( - validate_base_url("test", "http://ogx:8321", false).is_err(), - "DNS name should be rejected without allow_private" + validate_base_url("test", "http://ogx:8321", false).is_ok(), + "DNS names are classified and pinned by the transport at connect time" ); } diff --git a/apis/src/openai/responses/compact/config.rs b/apis/src/openai/responses/compact/config.rs index b6cbff472d..e20d0c55a7 100644 --- a/apis/src/openai/responses/compact/config.rs +++ b/apis/src/openai/responses/compact/config.rs @@ -6,7 +6,10 @@ use praxis_filter::FilterError; use serde::Deserialize; -use crate::callout_policy::{self, CalloutSettings, OnFailure}; +use crate::{ + callout_policy::{self, CalloutSettings, OnFailure}, + callout_target::{AddressPolicy, validate_configured_http_target}, +}; /// Default callout timeout (30 seconds — summarization can be slow). const DEFAULT_TIMEOUT_MS: u64 = 30_000; @@ -35,6 +38,10 @@ pub(super) struct CompactFilterConfig { /// E.g., `"http://localhost:11434/v1/chat/completions"` pub inference_url: String, + /// Allow the inference target to resolve to non-public addresses. + #[serde(default)] + pub allow_private_inference_url: bool, + /// Default model for summarization when not overridden /// in the request's `context_management`. #[serde(default = "default_model")] @@ -78,6 +85,9 @@ pub(super) struct ValidatedConfig { /// URL of the inference backend for summarization calls. pub inference_url: String, + /// Connect-time policy for the inference target. + pub address_policy: AddressPolicy, + /// Default model for summarization. pub default_model: String, @@ -99,11 +109,17 @@ const SUPPORTED_ENCODINGS: &[&str] = &["cl100k_base", "o200k_base"]; /// `true`, `inference_url` is empty, `tiktoken_encoding` is not a /// supported encoding name, `timeout_ms` is zero, or /// `status_on_error` is out of range. +#[expect( + clippy::too_many_lines, + reason = "the validation flow keeps all compact callout invariants together" +)] pub(super) fn build_config(raw: &CompactFilterConfig) -> Result { validate_pre_security_callout(raw)?; if raw.inference_url.is_empty() { return Err(FilterError::from("openai_responses_compact: inference_url is empty")); } + let address_policy = AddressPolicy::from_allow_private(raw.allow_private_inference_url); + validate_configured_http_target("openai_responses_compact", &raw.inference_url, address_policy)?; if !SUPPORTED_ENCODINGS.contains(&raw.tiktoken_encoding.as_str()) { return Err(FilterError::from(format!( @@ -124,6 +140,7 @@ pub(super) fn build_config(raw: &CompactFilterConfig) -> Result CompactFilterConfig { CompactFilterConfig { + allow_private_inference_url: true, allow_pre_security_callout: true, inference_url: "http://localhost:11434/v1/chat/completions".to_owned(), default_model: "gpt-4o-mini".to_owned(), @@ -60,7 +61,7 @@ fn from_config_missing_pre_security_ack() { #[test] fn from_config_accepts_pre_security_ack() { let yaml = serde_yaml::from_str::( - "allow_pre_security_callout: true\ninference_url: http://localhost/v1/chat/completions", + "allow_pre_security_callout: true\ninference_url: http://localhost/v1/chat/completions\nallow_private_inference_url: true", ) .unwrap(); assert!( @@ -76,6 +77,14 @@ fn build_config_rejects_empty_inference_url() { assert!(build_config(&cfg).is_err()); } +#[test] +fn private_inference_target_requires_explicit_opt_in() { + let mut cfg = base_config(); + cfg.allow_private_inference_url = false; + let error = build_config(&cfg).expect_err("loopback inference target must require opt-in"); + assert!(error.to_string().contains("localhost"), "unexpected error: {error}"); +} + #[test] fn build_config_rejects_zero_timeout() { let mut cfg = base_config(); @@ -469,7 +478,7 @@ fn conversation_text_skips_empty_compaction_summary() { fn make_filter(on_failure: &str) -> CompactFilter { let yaml = serde_yaml::from_str::(&format!( - "allow_pre_security_callout: true\ninference_url: http://localhost/v1/chat/completions\non_failure: {on_failure}" + "allow_pre_security_callout: true\ninference_url: http://localhost/v1/chat/completions\nallow_private_inference_url: true\non_failure: {on_failure}" )) .unwrap(); let cfg: CompactFilterConfig = serde_yaml::from_value(yaml).unwrap(); diff --git a/apis/src/openai/responses/file_resolve/config.rs b/apis/src/openai/responses/file_resolve/config.rs index 232c1afff7..6dc7582dfe 100644 --- a/apis/src/openai/responses/file_resolve/config.rs +++ b/apis/src/openai/responses/file_resolve/config.rs @@ -39,10 +39,10 @@ pub(crate) enum FileUrlMode { #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] pub(crate) struct FileResolveConfig { - /// Allow `files_api_url` to target private, loopback, link-local, - /// or DNS-name hosts. Default `false` rejects SSRF-sensitive - /// targets; set to `true` in development or when the Files API is - /// an internal service on a private network. + /// Allow `files_api_url` to resolve to private, loopback, link-local, + /// or otherwise non-public addresses. Default `false` permits DNS + /// names only when every connect-time result is public. Set to `true` + /// only when the Files API is a trusted private service. #[serde(default)] pub allow_private_files_api_url: bool, diff --git a/apis/src/openai/responses/file_resolve/mod.rs b/apis/src/openai/responses/file_resolve/mod.rs index 0535046435..1ea14261f0 100644 --- a/apis/src/openai/responses/file_resolve/mod.rs +++ b/apis/src/openai/responses/file_resolve/mod.rs @@ -203,6 +203,9 @@ impl FileResolveFilter { timeout: std::time::Duration::from_millis(validated.timeout_ms), max_response_bytes: 1_048_576, forward_header_names, + address_policy: crate::callout_target::AddressPolicy::from_allow_private( + validated.allow_private_files_api_url, + ), }); let client = FilesApiClient::new( diff --git a/apis/src/openai/responses/file_resolve/resolve.rs b/apis/src/openai/responses/file_resolve/resolve.rs index 020efdc700..cdf6c079ed 100644 --- a/apis/src/openai/responses/file_resolve/resolve.rs +++ b/apis/src/openai/responses/file_resolve/resolve.rs @@ -999,6 +999,7 @@ mod tests { timeout: std::time::Duration::from_millis(timeout_ms), max_response_bytes: 1_048_576, forward_header_names: Vec::new(), + address_policy: crate::callout_target::AddressPolicy::AllowPrivate, }) } diff --git a/apis/src/openai/responses/file_resolve/tests.rs b/apis/src/openai/responses/file_resolve/tests.rs index c1454b87f2..00cf6a2c04 100644 --- a/apis/src/openai/responses/file_resolve/tests.rs +++ b/apis/src/openai/responses/file_resolve/tests.rs @@ -790,6 +790,7 @@ fn make_client_for_url_with_max(files_api_url: &str, max_resolved_bytes: usize) timeout: Duration::from_secs(5), max_response_bytes: 1_048_576, forward_header_names: vec![], + address_policy: crate::callout_target::AddressPolicy::AllowPrivate, }); FilesApiClient::new( api, diff --git a/apis/src/openai/responses/file_search_callout/config.rs b/apis/src/openai/responses/file_search_callout/config.rs index 10db8583e8..68a59600a9 100644 --- a/apis/src/openai/responses/file_search_callout/config.rs +++ b/apis/src/openai/responses/file_search_callout/config.rs @@ -51,8 +51,8 @@ const DEFAULT_TIMEOUT_MS: u64 = 5_000; pub(crate) struct FileSearchFilterConfig { /// Allow URLs that target local-sensitive addresses. /// - /// DNS names are rejected unless this is enabled because validation - /// cannot pin the address that the HTTP client will eventually dial. + /// DNS names are allowed by default when every connect-time result is + /// public. Enable this only for a trusted private vector-store service. #[serde(default)] pub allow_private_url: bool, @@ -125,6 +125,7 @@ pub(crate) fn build_config_with_client( &forward_headers, max_response_bytes, timeout_ms, + cfg.allow_private_url, ); Ok(ValidatedConfig { @@ -162,12 +163,17 @@ fn validated_state_limit(configured: Option) -> Result ApiClient { ApiClient::new(ApiClientConfig { api_base_url: vector_store_url.as_str().to_owned(), @@ -178,6 +184,7 @@ fn build_api_client( .iter() .filter_map(|name| http::HeaderName::from_bytes(name.as_bytes()).ok()) .collect(), + address_policy: crate::callout_target::AddressPolicy::from_allow_private(allow_private), }) } diff --git a/apis/src/openai/responses/file_search_callout/tests.rs b/apis/src/openai/responses/file_search_callout/tests.rs index 39a66c9c3c..310d9ecd7c 100644 --- a/apis/src/openai/responses/file_search_callout/tests.rs +++ b/apis/src/openai/responses/file_search_callout/tests.rs @@ -79,10 +79,9 @@ fn config_rejects_ambiguous_or_invalid_urls() { } #[test] -fn config_rejects_dns_and_sensitive_ip_targets_by_default() { +fn config_rejects_sensitive_ip_targets_by_default() { for url in [ "http://localhost:8001", - "http://vector-store.internal:8001", "http://127.0.0.1:8001", "http://10.0.0.1:8001", "http://169.254.169.254:8001", @@ -93,6 +92,11 @@ fn config_rejects_dns_and_sensitive_ip_targets_by_default() { ] { assert!(parse_config(&format!("vector_store_url: '{url}'\n")).is_err(), "{url}"); } + + assert!( + parse_config("vector_store_url: 'http://vector-store.example:8001'\n").is_ok(), + "DNS targets are validated and pinned at connect time" + ); } #[test] diff --git a/apis/src/subrequest.rs b/apis/src/subrequest.rs index e50a20d777..11db752750 100644 --- a/apis/src/subrequest.rs +++ b/apis/src/subrequest.rs @@ -13,9 +13,11 @@ use std::{future::Future, net::SocketAddr, time::Duration}; use pingora_core::upstreams::peer::HttpPeer; -pub use praxis_core::subrequest::{SubRequest, SubRequestClient, SubRequestError, SubResponse}; +pub use praxis_core::subrequest::{FrameworkHeaders, SubRequest, SubRequestClient, SubRequestError, SubResponse}; use tracing::debug; +use crate::callout_target::{AddressPolicy, validate_http_target, validate_resolved_addrs}; + /// Parsed URL components needed to resolve and execute a request. #[derive(Debug)] struct ParsedUrl { @@ -111,57 +113,121 @@ async fn with_deadline( /// resolution or connect fails, the deadline is exceeded, admission /// or circuit breaking rejects the call, the response exceeds /// `max_response_bytes`, or I/O fails during the exchange. +#[expect( + clippy::too_many_arguments, + reason = "the request's transport policy and execution bounds remain explicit" +)] pub async fn execute_url( client: &SubRequestClient, url: &str, request: SubRequest, max_response_bytes: usize, timeout: Duration, + address_policy: AddressPolicy, +) -> Result { + execute_url_with_framework(client, url, request, max_response_bytes, timeout, address_policy, None).await +} + +/// Parse and execute a full-URL sub-request carrying framework headers. +/// +/// This is used by the generic callout filter to retain depth propagation +/// while sharing the same DNS pinning and address-policy enforcement as the +/// provider-specific clients. +/// +/// # Errors +/// +/// Returns [`SubRequestError`] for invalid URLs, resolution or connect +/// failures, policy violations, timeouts, bounded-read failures, or I/O. +#[expect( + clippy::too_many_arguments, + reason = "framework metadata is an additional explicit transport input" +)] +pub async fn execute_url_with_framework( + client: &SubRequestClient, + url: &str, + request: SubRequest, + max_response_bytes: usize, + timeout: Duration, + address_policy: AddressPolicy, + framework_headers: Option<&FrameworkHeaders>, ) -> Result { with_deadline( timeout, - Box::pin(execute_url_inner(client, url, request, max_response_bytes, timeout)), + Box::pin(resolve_and_execute_url( + client, + url, + request, + max_response_bytes, + timeout, + address_policy, + framework_headers, + )), ) .await } -/// Resolve DNS and execute under the deadline enforced by [`execute_url`]. -async fn execute_url_inner( +/// Resolve DNS and execute the request against the validated addresses. +#[expect( + clippy::too_many_arguments, + reason = "the resolution and execution inputs remain explicit" +)] +async fn resolve_and_execute_url( client: &SubRequestClient, url: &str, request: SubRequest, max_response_bytes: usize, timeout: Duration, + address_policy: AddressPolicy, + framework_headers: Option<&FrameworkHeaders>, ) -> Result { + validate_http_target("sub-request", url).map_err(|error| SubRequestError::InvalidRequest(error.to_string()))?; let parsed = parse_url_components(url)?; let addrs = resolve_addrs(&parsed.host, parsed.port).await?; - execute_resolved_url(client, parsed, request, &addrs, max_response_bytes, timeout).await + execute_with_addresses( + client, + parsed, + request, + max_response_bytes, + timeout, + address_policy, + framework_headers, + addrs, + ) + .await } -/// Try each resolved address until one connects successfully. +/// Validate addresses and try each one until the request succeeds. +/// +/// Keeping this boundary separate gives tests a controlled DNS result set +/// without changing production resolution behavior. #[expect( clippy::too_many_arguments, - reason = "internal helper requires parsed target and execution limits" + reason = "the subrequest transport inputs remain explicit" )] -async fn execute_resolved_url( +async fn execute_with_addresses( client: &SubRequestClient, parsed: ParsedUrl, - mut request: SubRequest, - addrs: &[SocketAddr], + request: SubRequest, max_response_bytes: usize, timeout: Duration, + address_policy: AddressPolicy, + framework_headers: Option<&FrameworkHeaders>, + addrs: Vec, ) -> Result { + let addrs = validate_resolved_addrs("sub-request", &addrs, address_policy) + .map_err(|error| SubRequestError::Connect(error.to_string()))?; + let mut request = request; request.uri = parsed.uri; if !request.headers.contains_key(http::header::HOST) { request.headers.insert(http::header::HOST, parsed.authority); } let mut last_connect_error = None; - for addr in addrs { + for addr in &addrs { let peer = HttpPeer::new(*addr, parsed.tls, parsed.sni.clone()); debug!(host = %parsed.host, %addr, "sub-request: trying resolved address"); - match Box::pin(client.execute(&peer, &request, max_response_bytes, timeout, None)).await { + match Box::pin(client.execute(&peer, &request, max_response_bytes, timeout, framework_headers)).await { Ok(response) => return Ok(response), Err(SubRequestError::Connect(error)) => { debug!(host = %parsed.host, %addr, %error, "sub-request: connect failed, trying next address"); @@ -177,6 +243,37 @@ async fn execute_resolved_url( ))) } +/// Test-only entry point that substitutes controlled DNS results. +#[cfg(test)] +#[expect( + clippy::too_many_arguments, + reason = "the test seam mirrors the production subrequest transport inputs" +)] +async fn execute_url_with_test_addresses( + client: &SubRequestClient, + url: &str, + request: SubRequest, + max_response_bytes: usize, + timeout: Duration, + address_policy: AddressPolicy, + framework_headers: Option<&FrameworkHeaders>, + addrs: Vec, +) -> Result { + validate_http_target("sub-request", url).map_err(|error| SubRequestError::InvalidRequest(error.to_string()))?; + let parsed = parse_url_components(url)?; + execute_with_addresses( + client, + parsed, + request, + max_response_bytes, + timeout, + address_policy, + framework_headers, + addrs, + ) + .await +} + #[cfg(test)] #[expect(clippy::allow_attributes, reason = "blanket test suppressions")] #[allow( @@ -303,14 +400,16 @@ mod tests { let good_addr = good_listener.local_addr().unwrap(); let captured = capture_raw_request(good_listener); - let parsed = parse_url_components(&format!("http://example.test:{}/test", good_addr.port())).unwrap(); - let response = Box::pin(execute_resolved_url( + let url = format!("http://example.test:{}/test", good_addr.port()); + let response = Box::pin(execute_url_with_test_addresses( &test_client(), - parsed, + &url, empty_request(), - &[bad_addr, good_addr], 1024, Duration::from_secs(5), + AddressPolicy::AllowPrivate, + None, + vec![bad_addr, good_addr], )) .await .unwrap(); @@ -319,6 +418,36 @@ mod tests { let _request = captured.join().unwrap(); } + #[tokio::test] + async fn execute_rejects_mixed_resolved_addresses_before_dialing() { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + listener.set_nonblocking(true).unwrap(); + let private_addr = listener.local_addr().unwrap(); + let public_addr = "8.8.8.8:443".parse().unwrap(); + let url = format!("http://example.test:{}/test", private_addr.port()); + + let result = Box::pin(execute_url_with_test_addresses( + &test_client(), + &url, + empty_request(), + 1024, + Duration::from_secs(5), + AddressPolicy::PublicOnly, + None, + vec![public_addr, private_addr], + )) + .await; + + assert!( + matches!(&result, Err(SubRequestError::Connect(detail)) if detail.contains("blocked non-public address")), + "mixed public/private answers must be rejected before transport: {result:?}" + ); + assert!( + matches!(listener.accept(), Err(error) if error.kind() == std::io::ErrorKind::WouldBlock), + "a rejected address set must not be dialed" + ); + } + #[tokio::test] async fn execute_sends_original_authority_as_host_header() { let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); @@ -327,13 +456,15 @@ mod tests { let authority = format!("my-virtual-host.example.com:{}", addr.port()); let parsed = parse_url_components(&format!("http://{authority}/test")).unwrap(); - Box::pin(execute_resolved_url( + Box::pin(execute_with_addresses( &test_client(), parsed, empty_request(), - &[addr], 1024, Duration::from_secs(5), + AddressPolicy::AllowPrivate, + None, + vec![addr], )) .await .unwrap(); diff --git a/apis/src/web_search/config.rs b/apis/src/web_search/config.rs index d0c4ab3354..3cc7a7cd83 100644 --- a/apis/src/web_search/config.rs +++ b/apis/src/web_search/config.rs @@ -119,13 +119,10 @@ pub(crate) struct WebSearchFilterConfig { /// Allow a `base_url` that targets local-sensitive addresses. /// - /// DNS targets are unsupported in protected mode (the default): - /// validation cannot pin the address the HTTP client will eventually - /// dial, so a `base_url` host must be a public IP literal. Enabling - /// `allow_private_base_url` also permits DNS results resolving to - /// local-sensitive addresses, so a hostile or rebound resolution can - /// send the provider credential to a loopback, private, or - /// cloud-metadata endpoint. + /// DNS names are resolved once per request and every result is checked + /// immediately before the transport connects. By default, any private, + /// loopback, link-local, or otherwise non-public result rejects the + /// callout. Enable this only for a trusted private provider endpoint. #[serde(default)] pub(crate) allow_private_base_url: bool, } @@ -154,6 +151,9 @@ pub(crate) struct ValidatedConfig { /// Override the provider's default API base URL. pub base_url: Option, + + /// Connect-time private-address policy for the provider target. + pub allow_private_base_url: bool, } impl std::fmt::Debug for ValidatedConfig { @@ -165,6 +165,7 @@ impl std::fmt::Debug for ValidatedConfig { .field("timeout_ms", &self.timeout_ms) .field("max_body_bytes", &self.max_body_bytes) .field("base_url", &self.base_url) + .field("allow_private_base_url", &self.allow_private_base_url) .finish() } } @@ -202,6 +203,7 @@ fn build_validated_config( timeout_ms: callout_policy::validate_timeout_ms(filter_name, raw.timeout_ms, DEFAULT_TIMEOUT_MS)?, max_body_bytes: validate_max_body_bytes_field(filter_name, raw.max_body_bytes)?, base_url: raw.base_url.clone(), + allow_private_base_url: raw.allow_private_base_url, }) } @@ -375,13 +377,10 @@ mod tests { } #[test] - fn build_config_rejects_dns_base_url_without_opt_in() { + fn build_config_accepts_dns_base_url_for_connect_time_validation() { let mut cfg = base_config(); cfg.base_url = Some("http://internal.search.example:8080".into()); - assert!( - build_config("openai_web_search", &cfg).is_err(), - "DNS base_url must be rejected without allow_private_base_url because the dialed address cannot be pinned" - ); + assert!(build_config("openai_web_search", &cfg).is_ok()); } #[test] diff --git a/apis/src/web_search/provider.rs b/apis/src/web_search/provider.rs index fa8ad3650b..54abef8c35 100644 --- a/apis/src/web_search/provider.rs +++ b/apis/src/web_search/provider.rs @@ -21,7 +21,10 @@ use super::{ ValidatedConfig, config::{SearchContextSize, SearchProvider}, }; -use crate::subrequest::{self, SubRequest, SubRequestClient, SubRequestError, SubResponse}; +use crate::{ + callout_target::AddressPolicy, + subrequest::{self, SubRequest, SubRequestClient, SubRequestError, SubResponse}, +}; /// Response body cap for search callouts (1 MiB). Distinct from /// `max_body_bytes` which governs inbound request buffering. @@ -77,6 +80,8 @@ pub(crate) struct SearchClient { default_context_size: SearchContextSize, /// Override the provider's default API base URL. base_url: Option, + /// Connect-time private-address policy for the provider target. + address_policy: AddressPolicy, } impl std::fmt::Debug for SearchClient { @@ -88,6 +93,7 @@ impl std::fmt::Debug for SearchClient { .field("api_key", &"[REDACTED]") .field("default_context_size", &self.default_context_size) .field("base_url", &self.base_url) + .field("address_policy", &self.address_policy) .finish() } } @@ -113,6 +119,7 @@ impl SearchClient { api_key: config.api_key.clone(), default_context_size: config.default_context_size, base_url: config.base_url.clone(), + address_policy: AddressPolicy::from_allow_private(config.allow_private_base_url), }) } @@ -137,7 +144,15 @@ impl SearchClient { /// Execute a search request and map the result to a /// [`SearchOutcome`]. async fn execute_search(&self, url: &str, request: SubRequest) -> SearchOutcome { - let result = subrequest::execute_url(&self.client, url, request, MAX_SEARCH_RESPONSE_BYTES, self.timeout).await; + let result = subrequest::execute_url( + &self.client, + url, + request, + MAX_SEARCH_RESPONSE_BYTES, + self.timeout, + self.address_policy, + ) + .await; self.map_search_result(result) } @@ -475,6 +490,7 @@ mod tests { timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, base_url: None, + allow_private_base_url: false, }; let client = SearchClient::from_config("test", &config, test_subrequest_client()).unwrap(); @@ -528,6 +544,7 @@ mod tests { timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, base_url: None, + allow_private_base_url: false, }; let client = SearchClient::from_config("test", &config, test_subrequest_client()); assert!(client.is_ok()); @@ -542,6 +559,7 @@ mod tests { timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, base_url: None, + allow_private_base_url: false, }; let error = SearchClient::from_config("anthropic_web_search", &config, test_subrequest_client()).unwrap_err(); @@ -563,6 +581,7 @@ mod tests { timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, base_url: Some("http://localhost:9999".into()), + allow_private_base_url: true, }; let client = SearchClient::from_config("test", &config, test_subrequest_client()).unwrap(); let (url, _) = client.build_brave_request("test query", 5); @@ -581,6 +600,7 @@ mod tests { timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, base_url: Some("http://localhost:9999".into()), + allow_private_base_url: true, }; let client = SearchClient::from_config("test", &config, test_subrequest_client()).unwrap(); let (url, _) = client.build_tavily_request("test query", SearchContextSize::Medium); @@ -599,6 +619,7 @@ mod tests { timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, base_url: Some("http://localhost:9999".into()), + allow_private_base_url: true, }; let client = SearchClient::from_config("test", &config, test_subrequest_client()).unwrap(); let (url, _) = client.build_you_request("test query", 5); @@ -617,6 +638,7 @@ mod tests { timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, base_url: None, + allow_private_base_url: false, }; let client = SearchClient::from_config("test", &config, test_subrequest_client()).unwrap(); let outcome = client.parse_response(b"not json"); @@ -635,6 +657,7 @@ mod tests { timeout_ms: 5000, max_body_bytes: 64 * 1024 * 1024, base_url: None, + allow_private_base_url: false, }; let client = SearchClient::from_config("test", &config, test_subrequest_client()).unwrap(); let outcome = client.parse_response(br#"{"web":{"results":[]}}"#); @@ -652,6 +675,7 @@ mod tests { timeout_ms: 1000, max_body_bytes: 64 * 1024 * 1024, base_url: None, + allow_private_base_url: true, }; SearchClient::from_config("test", &config, test_subrequest_client()).unwrap() } diff --git a/docs/README.md b/docs/README.md index e186ba8989..04ffa97759 100644 --- a/docs/README.md +++ b/docs/README.md @@ -15,6 +15,7 @@ provider API integrations on top of [Praxis](https://github.com/praxis-proxy/pra - [AI inference](architecture/ai-inference.md) - [Agentic protocols](architecture/agentic-protocols.md) - [Response store](architecture/response-store.md) +- [Outbound callout security](architecture/outbound-callouts.md) ## Provider guides diff --git a/docs/architecture/outbound-callouts.md b/docs/architecture/outbound-callouts.md new file mode 100644 index 0000000000..af22f2e61d --- /dev/null +++ b/docs/architecture/outbound-callouts.md @@ -0,0 +1,60 @@ +# Outbound callout security + +Praxis AI treats an outbound target, its resolved socket addresses, and the +credentials attached to the request as one trust decision. Operator-configured +HTTP targets are public-only by default. Each request resolves its target once, +checks every returned address immediately before connecting, and gives those +same addresses to the transport. A single private, loopback, link-local, or +otherwise non-public result rejects the whole callout. + +Private targets require a filter-specific, explicit opt-in. HTTP proxies and +redirect following are disabled for direct callouts. URL userinfo is rejected, +and forwarded or configured credentials remain bound to the validated target +origin. + +## Callout inventory + +`No-follow` means a redirect response is returned to the caller and its +`Location` is never requested. + +| Callout | Target source | Private opt-in | Redirect policy | Authentication mode | +| --- | --- | --- | --- | --- | +| `openai_web_search`, `anthropic_web_search` | Provider default or configured `base_url` | `allow_private_base_url` | No-follow | Configured provider API key; validated origin only | +| `openai_file_resolve` Files API | Configured `files_api_url` | `allow_private_files_api_url` | No-follow | Only headers named by `forward_headers` | +| `openai_file_resolve` `file_url` fetch | Request-derived URL | Exact `allowed_file_url_origins` | No-follow | Anonymous; no downstream headers | +| `openai_file_search_callout` | Configured `vector_store_url` | `allow_private_url` | No-follow | Only headers named by `forward_headers` | +| `openai_responses_compact` | Configured `inference_url` | `allow_private_inference_url` | No-follow | Anonymous; no downstream or cluster headers | +| `ai_guardrails` with NeMo | Configured `endpoint` | `allow_private_endpoint` | No-follow | Anonymous; no downstream headers | +| `http_callout` | Configured `target.url` | `allow_private_addresses` | No-follow | Configured static headers plus allowed `forward_headers` | +| MCP client | Request-derived server URL or configured connector | `allow_loopback` for loopback only | No-follow | Sanitized request-provided MCP authorization/headers | +| `azure_ad` token fetch | Configured authority plus tenant | `allow_private_authority` | No-follow | Client secret in the token POST body | +| `gcp_adc` metadata fetch | Protocol-owned metadata endpoint | Intrinsic to metadata mode | No-follow | `Metadata-Flavor` protocol header; returned token is not forwarded back to metadata | + +The request-derived `file_url` and MCP transports retain stricter policies: +they pin one validated resolution set, do not follow redirects, and allow +private access only through their narrow origin/loopback controls. GCP metadata +also uses pinned resolution, no redirects, and no ambient proxy, but intentionally +allows its protocol-owned private destination. + +Upstream cluster connections are not direct callouts. They use the core Praxis +endpoint and TLS policy instead of these filter-level controls. + +## Adding a callout + +A new filter that opens an outbound HTTP connection must: + +1. Classify the target as operator-configured, request-derived, or + protocol-owned. +2. Reuse the shared target/address policy, or document why a stricter dedicated + policy is required. +3. Default to public addresses and expose a narrowly named private-address + opt-in only when the use case requires it. +4. Resolve once per connection attempt, reject the complete result set if any + address violates policy, and connect only to that validated set. +5. Disable ambient proxies and redirects unless their security behavior is + explicitly designed and tested. +6. Reject URL userinfo and bind every credential or forwarded header to the + validated origin. +7. Document the authentication mode and cover loopback/private/link-local, + mixed DNS answers, redirects, userinfo, and credential non-disclosure in + tests. diff --git a/docs/filters/ai_guardrails.md b/docs/filters/ai_guardrails.md index 618a2c7ea1..9b959ad379 100644 --- a/docs/filters/ai_guardrails.md +++ b/docs/filters/ai_guardrails.md @@ -22,6 +22,7 @@ filter: ai_guardrails provider: type: nemo endpoint: "http://nemo:8000/v1/guardrail/checks" + allow_private_endpoint: true timeout_ms: 5000 phase: request: true diff --git a/docs/filters/anthropic_web_search.md b/docs/filters/anthropic_web_search.md index d0136dc7f3..ddef6075a4 100644 --- a/docs/filters/anthropic_web_search.md +++ b/docs/filters/anthropic_web_search.md @@ -15,7 +15,7 @@ Executes server-owned `WebSearch` tool calls in an Anthropic Messages loop. | `timeout_ms` | integer | no | Callout timeout in milliseconds. | | `max_body_bytes` | integer | no | Maximum request body bytes to buffer. | | `base_url` | string | no | Override the provider's default API base URL. | -| `allow_private_base_url` | bool | no | Allow a `base_url` that targets local-sensitive addresses. DNS targets are unsupported in protected mode (the default): validation cannot pin the address the HTTP client will eventually dial, so a `base_url` host must be a public IP literal. Enabling `allow_private_base_url` also permits DNS results resolving to local-sensitive addresses, so a hostile or rebound resolution can send the provider credential to a loopback, private, or cloud-metadata endpoint. | +| `allow_private_base_url` | bool | no | Allow a `base_url` that targets local-sensitive addresses. DNS names are resolved once per request and every result is checked immediately before the transport connects. By default, any private, loopback, link-local, or otherwise non-public result rejects the callout. Enable this only for a trusted private provider endpoint. | ## Examples diff --git a/docs/filters/azure_ad.md b/docs/filters/azure_ad.md index 3bc951376d..3674e1377b 100644 --- a/docs/filters/azure_ad.md +++ b/docs/filters/azure_ad.md @@ -20,4 +20,5 @@ client_id: 11111111-1111-1111-1111-111111111111 scope: https://cognitiveservices.azure.com/.default client_secret_env_var: AZURE_CLIENT_SECRET authority_host: login.microsoftonline.com # optional, for sovereign clouds +allow_private_authority: false # opt in only for a trusted private authority ``` diff --git a/docs/filters/gcp_adc.md b/docs/filters/gcp_adc.md index 3745dee50c..c0b7dc206d 100644 --- a/docs/filters/gcp_adc.md +++ b/docs/filters/gcp_adc.md @@ -19,6 +19,8 @@ This filter only injects `Authorization`. Pointing the request at the correct Ve Whenever no valid token can be produced — none cached and the inline fetch fails — the request is rejected with `503` rather than forwarded unauthenticated. +Metadata requests use a proxy-free, redirect-free client pinned to the complete validated DNS result set. Metadata mode intentionally permits the protocol-owned private endpoint; arbitrary private hosts remain invalid configuration. + ## Configuration | Field | Type | Required | Description | diff --git a/docs/filters/http_callout.md b/docs/filters/http_callout.md index c12d3ecb87..9675e64e64 100644 --- a/docs/filters/http_callout.md +++ b/docs/filters/http_callout.md @@ -17,7 +17,7 @@ Makes an outbound HTTP request during request processing, optionally forwarding |-------|------|---------|-------------| | `target` | TargetConfig | yes | Callout target configuration. | | `target.url` | string | yes | Absolute HTTP(S) URL to call. | -| `target.allow_private_addresses` | bool | no | Allow the target to resolve to a private, loopback, or link-local address. Defaults to `true`, which preserves the permissive behavior of pointing a callout at a loopback/sidecar guard service (only a warning is emitted). Set to `false` to harden against SSRF and DNS-rebinding: the callout is then rejected at request time if the resolved peer address is private/loopback/link-local — including a hostname that resolves to such an address (e.g. cloud metadata at `169.254.169.254`). | +| `target.allow_private_addresses` | bool | no | Allow the target to resolve to a private, loopback, or link-local address. Defaults to `false`. Set to `true` explicitly when a trusted loopback/sidecar or private service is the intended destination. When disabled, the callout is rejected at request time if any resolved peer address is private/loopback/link-local — including a hostname that resolves to such an address (e.g. cloud metadata at `169.254.169.254`). | | `target.timeout` | Duration | no | Request timeout (e.g. `"2s"`, `"500ms"`). | | `target.headers` | HeaderEntry[] | no | Static headers to send with every callout. | | `target.headers[].name` | string | yes | Header name. | diff --git a/docs/filters/openai_file_resolve.md b/docs/filters/openai_file_resolve.md index 02cfabc4ef..bc9d6d15ba 100644 --- a/docs/filters/openai_file_resolve.md +++ b/docs/filters/openai_file_resolve.md @@ -15,7 +15,7 @@ This filter resolves references inside Responses requests; it does not proxy cli | Field | Type | Required | Description | |-------|------|---------|-------------| -| `allow_private_files_api_url` | bool | no | Allow `files_api_url` to target private, loopback, link-local, or DNS-name hosts. Default `false` rejects SSRF-sensitive targets; set to `true` in development or when the Files API is an internal service on a private network. | +| `allow_private_files_api_url` | bool | no | Allow `files_api_url` to resolve to private, loopback, link-local, or otherwise non-public addresses. Default `false` permits DNS names only when every connect-time result is public. Set to `true` only when the Files API is a trusted private service. | | `allow_pre_security_callout` | bool | no | Allow Files API callouts from the `StreamBuffer` pre-read phase, before header-phase security filters execute. This must be explicitly enabled only when an outer trust boundary authenticates and authorizes requests before they reach this listener. Forwarded headers are the original downstream values, not mutations from request filters. | | `files_api_url` | string | yes | Base URL of the Files API endpoint. Example: `http://files-api:8321` | | `forward_headers` | string[] | no | Headers to forward from the original request to the Files API for authentication and tenant isolation. No downstream headers are forwarded by default. | diff --git a/docs/filters/openai_file_search_callout.md b/docs/filters/openai_file_search_callout.md index 24ff764c88..f48b01cb72 100644 --- a/docs/filters/openai_file_search_callout.md +++ b/docs/filters/openai_file_search_callout.md @@ -13,7 +13,7 @@ The enclosing iterative router owns model re-entry. Streaming requests are rejec | Field | Type | Required | Description | |-------|------|---------|-------------| -| `allow_private_url` | bool | no | Allow URLs that target local-sensitive addresses. DNS names are rejected unless this is enabled because validation cannot pin the address that the HTTP client will eventually dial. | +| `allow_private_url` | bool | no | Allow URLs that target local-sensitive addresses. DNS names are allowed by default when every connect-time result is public. Enable this only for a trusted private vector-store service. | | `on_failure` | `closed` \| `open` | no | Behaviour when a vector-store callout fails. | | `forward_headers` | string[] | no | Headers to forward from the original request to the vector store API for authentication and tenant isolation. No downstream headers are forwarded by default. | | `max_response_bytes` | integer | no | Maximum response body size in bytes per callout. | diff --git a/docs/filters/openai_responses_compact.md b/docs/filters/openai_responses_compact.md index 77951fbe00..e9adf9418c 100644 --- a/docs/filters/openai_responses_compact.md +++ b/docs/filters/openai_responses_compact.md @@ -19,6 +19,7 @@ Praxis runs `StreamBuffer` body hooks before header-phase request filters. This |-------|------|---------|-------------| | `allow_pre_security_callout` | bool | no | Allow summarization callouts from the `StreamBuffer` pre-read phase, before header-phase security filters execute. This must be explicitly enabled only when an outer trust boundary authenticates and authorizes requests before they reach this listener. | | `inference_url` | string | yes | URL of the inference backend for summarization calls. E.g., `"http://localhost:11434/v1/chat/completions"` | +| `allow_private_inference_url` | bool | no | Allow the inference target to resolve to non-public addresses. | | `default_model` | string | no | Default model for summarization when not overridden in the request's `context_management`. | | `tiktoken_encoding` | string | no | Tiktoken encoding name for local token estimation of the conversation text. | | `timeout_ms` | integer | no | Callout timeout in milliseconds. | diff --git a/docs/filters/openai_web_search.md b/docs/filters/openai_web_search.md index e504e5fc40..648419b847 100644 --- a/docs/filters/openai_web_search.md +++ b/docs/filters/openai_web_search.md @@ -19,7 +19,7 @@ Detects pending web search calls in the response phase and executes them on re-e | `timeout_ms` | integer | no | Callout timeout in milliseconds. | | `max_body_bytes` | integer | no | Maximum request body bytes to buffer. | | `base_url` | string | no | Override the provider's default API base URL. | -| `allow_private_base_url` | bool | no | Allow a `base_url` that targets local-sensitive addresses. DNS targets are unsupported in protected mode (the default): validation cannot pin the address the HTTP client will eventually dial, so a `base_url` host must be a public IP literal. Enabling `allow_private_base_url` also permits DNS results resolving to local-sensitive addresses, so a hostile or rebound resolution can send the provider credential to a loopback, private, or cloud-metadata endpoint. | +| `allow_private_base_url` | bool | no | Allow a `base_url` that targets local-sensitive addresses. DNS names are resolved once per request and every result is checked immediately before the transport connects. By default, any private, loopback, link-local, or otherwise non-public result rejects the callout. Enable this only for a trusted private provider endpoint. | ## Examples diff --git a/examples/configs/azure-ad.yaml b/examples/configs/azure-ad.yaml index 75fc8066db..fd496f8b47 100644 --- a/examples/configs/azure-ad.yaml +++ b/examples/configs/azure-ad.yaml @@ -40,6 +40,7 @@ filter_chains: scope: https://cognitiveservices.azure.com/.default client_secret_env_var: AZURE_CLIENT_SECRET # authority_host: login.microsoftonline.com # optional, for sovereign clouds + # allow_private_authority: false # opt in only for a trusted private authority insecure_options: allow_private_endpoints: true # example proxies to a local backend diff --git a/examples/configs/gcp-adc.yaml b/examples/configs/gcp-adc.yaml index 72f21bcde3..41c69c647f 100644 --- a/examples/configs/gcp-adc.yaml +++ b/examples/configs/gcp-adc.yaml @@ -43,6 +43,8 @@ filter_chains: - filter: gcp_adc # source: adc # default: GOOGLE_APPLICATION_CREDENTIALS, else GKE metadata # scope: https://www.googleapis.com/auth/cloud-platform + # Metadata authentication uses only the fixed metadata endpoint; + # redirects and ambient HTTP proxies are disabled. insecure_options: allow_private_endpoints: true # example proxies to a local backend diff --git a/examples/configs/inference/fallback-with-translation.yaml b/examples/configs/inference/fallback-with-translation.yaml index a04d4f4125..4ccd17689b 100644 --- a/examples/configs/inference/fallback-with-translation.yaml +++ b/examples/configs/inference/fallback-with-translation.yaml @@ -83,7 +83,7 @@ filter_chains: - filter: openai_responses_format - filter: openai_responses_validate - filter: responses_to_chat_completions - max_body_bytes: 67108864 + max_rewritten_body_bytes: 67108864 - filter: path_rewrite replace: pattern: "^/v1/responses/?$" @@ -119,7 +119,7 @@ filter_chains: - filter: openai_responses_format - filter: openai_responses_validate - filter: responses_to_chat_completions - max_body_bytes: 67108864 + max_rewritten_body_bytes: 67108864 - filter: path_rewrite replace: pattern: "^/v1/responses/?$" diff --git a/examples/configs/lakera-guard.yaml b/examples/configs/lakera-guard.yaml index b8b6b542b2..af9f0dedcc 100644 --- a/examples/configs/lakera-guard.yaml +++ b/examples/configs/lakera-guard.yaml @@ -41,9 +41,8 @@ filter_chains: url: "https://api.lakera.ai/v2/guard" # This callout targets a public API. Reject it at request time # if the host ever resolves to a private/loopback/link-local - # address (SSRF / DNS-rebinding hardening). Leave unset (default - # true) only when deliberately pointing at a loopback/sidecar - # guard service. + # address (SSRF / DNS-rebinding hardening). Set this to true + # only when deliberately pointing at a trusted private service. allow_private_addresses: false timeout: "2s" headers: diff --git a/examples/configs/nemo-guardrails.yaml b/examples/configs/nemo-guardrails.yaml index d68782fa2b..2b0c8713ac 100644 --- a/examples/configs/nemo-guardrails.yaml +++ b/examples/configs/nemo-guardrails.yaml @@ -33,7 +33,10 @@ filter_chains: - filter: ai_guardrails provider: type: nemo + # NeMo callouts are anonymous; no downstream authentication + # headers are forwarded to this endpoint. endpoint: "http://127.0.0.1:3001/v1/guardrail/checks" + allow_private_endpoint: true timeout_ms: 5000 phase: request: true diff --git a/examples/configs/openai/responses/compact.yaml b/examples/configs/openai/responses/compact.yaml index 7c930ae407..d9834997ac 100644 --- a/examples/configs/openai/responses/compact.yaml +++ b/examples/configs/openai/responses/compact.yaml @@ -56,8 +56,11 @@ filter_chains: - filter: openai_responses_rehydrate - filter: openai_responses_compact + # This direct summarization callout is anonymous; no downstream or + # cluster authentication headers are forwarded. allow_pre_security_callout: true inference_url: "http://localhost:11434/v1/chat/completions" + allow_private_inference_url: true default_model: llama3.2:1b timeout_ms: 60000 diff --git a/examples/configs/openai/responses/web-search.yaml b/examples/configs/openai/responses/web-search.yaml index b68f68a843..3cd84f9076 100644 --- a/examples/configs/openai/responses/web-search.yaml +++ b/examples/configs/openai/responses/web-search.yaml @@ -12,14 +12,12 @@ # api_key: Provider API key (supports ${ENV_VAR} syntax) # default_context_size: How many results to return (low/medium/high) # timeout_ms: Callout timeout in milliseconds -# base_url: Override the provider API base URL. Must be a -# public IP literal unless allow_private_base_url -# is set (SSRF/credential-disclosure guard). +# base_url: Override the provider API base URL. DNS is pinned +# and all resolved addresses must be public by default. # allow_private_base_url: Permit base_url targets on local-sensitive -# addresses and DNS names (default false). DNS -# targets are unsupported in protected mode; -# enabling this also permits DNS results resolving -# to local-sensitive addresses. +# addresses (default false). Enable only for a trusted +# private provider. The configured api_key is sent only +# to this validated origin; redirects are not followed. listeners: - name: ai-gateway diff --git a/filters/src/azure/azure_ad.rs b/filters/src/azure/azure_ad.rs index 347ec8bc38..2e66d64d1b 100644 --- a/filters/src/azure/azure_ad.rs +++ b/filters/src/azure/azure_ad.rs @@ -64,6 +64,7 @@ //! scope: https://cognitiveservices.azure.com/.default //! client_secret_env_var: AZURE_CLIENT_SECRET //! authority_host: login.microsoftonline.com # optional, for sovereign clouds +//! allow_private_authority: false # opt in only for a trusted private authority //! ``` use std::{ @@ -178,13 +179,12 @@ pub struct AzureAdFilter { /// [`praxis_ai_apis::token_cache`]. cache: TokenCache, - /// HTTP client used for token-endpoint requests, built once with - /// [`TOKEN_REQUEST_TIMEOUT`]. - client: reqwest::Client, - /// Fully-formed token endpoint URL. token_url: String, + /// Connect-time address policy for the authority endpoint. + address_policy: praxis_ai_apis::callout_target::AddressPolicy, + /// Application (client) ID. client_id: String, @@ -223,15 +223,13 @@ impl AzureAdFilter { "https://{}/{}/oauth2/v2.0/token", config.authority_host, config.tenant_id ); - let client = reqwest::Client::builder() - .timeout(TOKEN_REQUEST_TIMEOUT) - .build() - .map_err(|e| FilterError::from(format!("azure_ad: failed to build HTTP client: {e}")))?; - + let address_policy = + praxis_ai_apis::callout_target::AddressPolicy::from_allow_private(config.allow_private_authority); + praxis_ai_apis::callout_target::validate_configured_http_target("azure_ad", &token_url, address_policy)?; Ok(Self { cache: TokenCache::new(EXPIRY_SKEW), - client, token_url, + address_policy, client_id: config.client_id, client_secret, scope: config.scope, @@ -265,20 +263,32 @@ impl praxis_filter::HttpFilter for AzureAdFilter { praxis_filter::BodyMode::Stream } + #[expect( + clippy::too_many_lines, + reason = "token refresh and fail-closed state transitions stay together" + )] async fn on_request( &self, ctx: &mut praxis_filter::HttpFilterContext<'_>, ) -> Result { let fetched = self .cache - .get_or_refresh(|| { + .get_or_refresh(|| async { + let client = praxis_ai_apis::callout_target::build_pinned_reqwest_client( + "azure_ad", + &self.token_url, + self.address_policy, + TOKEN_REQUEST_TIMEOUT, + ) + .await?; fetch_token( - &self.client, + &client, &self.token_url, &self.client_id, &self.client_secret, &self.scope, ) + .await }) .await; match fetched { @@ -330,6 +340,10 @@ pub(crate) struct AzureAdConfig { /// `login.microsoftonline.us`). #[serde(default = "default_authority_host")] pub(crate) authority_host: String, + + /// Allow a private authority host for a trusted internal identity service. + #[serde(default)] + pub(crate) allow_private_authority: bool, } /// Validate the config fields [`AzureAdFilter::new`] relies on before it @@ -510,6 +524,7 @@ mod tests { scope: "s".to_owned(), client_secret_env_var: "AZURE_TEST_UNSET_SECRET".to_owned(), authority_host: "login.microsoftonline.com@evil.com".to_owned(), + allow_private_authority: false, }; match AzureAdFilter::new(cfg) { Ok(_) => panic!("malicious authority_host must be rejected"), @@ -579,6 +594,40 @@ mod tests { server.join().unwrap(); } + #[tokio::test] + async fn token_redirect_does_not_disclose_client_secret() { + let redirect_target = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + redirect_target.set_nonblocking(true).unwrap(); + let target_addr = redirect_target.local_addr().unwrap(); + + let redirector = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let redirector_addr = redirector.local_addr().unwrap(); + let server = std::thread::spawn(move || { + let (mut stream, _) = redirector.accept().unwrap(); + let mut request = [0_u8; 4096]; + let _ = stream.read(&mut request).unwrap(); + let response = format!( + "HTTP/1.1 307 Temporary Redirect\r\nLocation: http://{target_addr}/steal\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" + ); + stream.write_all(response.as_bytes()).unwrap(); + }); + + let filter = filter_at(&format!("http://{redirector_addr}/token")); + let request = make_request(Method::POST, "/openai/deployments/gpt-4o/chat/completions"); + let mut ctx = make_filter_context(&request); + let action = filter.on_request(&mut ctx).await.unwrap(); + + assert!( + matches!(action, FilterAction::Reject(_)), + "redirected token fetch must fail closed" + ); + server.join().unwrap(); + assert!( + matches!(redirect_target.accept(), Err(error) if error.kind() == std::io::ErrorKind::WouldBlock), + "redirect target must not receive the secret-bearing token request" + ); + } + // -- on_request: cache-through end to end ---------------------------------- /// Build a filter with a real, always-set secret env var (so @@ -591,7 +640,8 @@ mod tests { "tenant_id: tid\n\ client_id: cid\n\ scope: scope\n\ - client_secret_env_var: CARGO_PKG_NAME\n", + client_secret_env_var: CARGO_PKG_NAME\n\ + allow_private_authority: true\n", )) .expect("test config must parse"); let mut filter = diff --git a/filters/src/callout/config.rs b/filters/src/callout/config.rs index 798165a020..0e44bd8452 100644 --- a/filters/src/callout/config.rs +++ b/filters/src/callout/config.rs @@ -64,14 +64,13 @@ pub(crate) struct TargetConfig { /// Allow the target to resolve to a private, loopback, or /// link-local address. /// - /// Defaults to `true`, which preserves the permissive behavior of - /// pointing a callout at a loopback/sidecar guard service (only a - /// warning is emitted). Set to `false` to harden against SSRF and - /// DNS-rebinding: the callout is then rejected at request time if the + /// Defaults to `false`. Set to `true` explicitly when a trusted + /// loopback/sidecar or private service is the intended destination. + /// When disabled, the callout is rejected at request time if any /// resolved peer address is private/loopback/link-local — including a /// hostname that resolves to such an address (e.g. cloud metadata at /// `169.254.169.254`). - #[serde(default = "default_allow_private_addresses")] + #[serde(default)] pub allow_private_addresses: bool, /// Request timeout (e.g. `"2s"`, `"500ms"`). @@ -215,11 +214,6 @@ fn default_max_body_bytes() -> usize { 1_048_576 // 1 MiB } -/// Default: allow private/loopback targets (permissive, warning only). -fn default_allow_private_addresses() -> bool { - true -} - // ----------------------------------------------------------------------------- // Duration Parsing // ----------------------------------------------------------------------------- diff --git a/filters/src/callout/mod.rs b/filters/src/callout/mod.rs index 9104cb5383..0f8aa7c63e 100644 --- a/filters/src/callout/mod.rs +++ b/filters/src/callout/mod.rs @@ -28,11 +28,13 @@ use bytes::Bytes; use config::{HttpCalloutConfig, Phase, expand_env_vars, validate_callout_url}; use extract::{BodyShaper, CompiledExtraction}; use http::HeaderMap; -use pingora_core::upstreams::peer::HttpPeer; -use praxis_ai_apis::callout_policy::{OnFailure, validate_status_on_error}; +use praxis_ai_apis::{ + callout_policy::{OnFailure, validate_status_on_error}, + callout_target::{AddressPolicy, validate_configured_http_target}, + subrequest, +}; use praxis_core::{ circuit::CircuitBreakerConfig as CoreCircuitBreakerConfig, - connectivity::is_private_ip, subrequest::{ DEPTH_HEADER, FrameworkHeaders, SubRequest, SubRequestClient, SubRequestConnector, SubRequestConnectorOptions, SubResponse, @@ -125,9 +127,6 @@ pub struct HttpCalloutFilter { /// HTTP status code returned when rejecting on failure. status_on_error: u16, - /// Parsed target (host, port, TLS, SNI, authority, request URI). - target: CalloutTarget, - /// Request timeout covering DNS, connect, and I/O. timeout: Duration, @@ -143,6 +142,10 @@ impl HttpCalloutFilter { /// Returns [`FilterError`] if config parsing, SSRF validation, /// env-var expansion, `JSONPath` compilation, or client /// construction fails. + #[expect( + clippy::too_many_lines, + reason = "construction validates the complete callout security policy in one place" + )] pub fn from_config(config: &serde_yaml::Value) -> Result, FilterError> { let cfg: HttpCalloutConfig = parse_filter_config(FILTER_NAME, config)?; @@ -157,7 +160,11 @@ impl HttpCalloutFilter { let extractions = compile_extractions(&cfg)?; let inject_headers = parse_header_names(&cfg.response.inject_headers, "inject_header")?; - let target = CalloutTarget::parse(&cfg.target.url)?; + validate_configured_http_target( + FILTER_NAME, + &cfg.target.url, + AddressPolicy::from_allow_private(cfg.target.allow_private_addresses), + )?; let client = build_subrequest_client(&cfg); Ok(Box::new(Self { @@ -173,7 +180,6 @@ impl HttpCalloutFilter { max_depth: cfg.max_depth.unwrap_or(1), phase: cfg.request.phase, status_on_error, - target, timeout: cfg.target.timeout, url: cfg.target.url, })) @@ -196,7 +202,7 @@ impl HttpCalloutFilter { let request = SubRequest { method: http::Method::POST, - uri: self.target.request_uri.clone(), + uri: http::Uri::default(), headers, body: body.map_or(Bytes::new(), Bytes::from), }; @@ -224,11 +230,7 @@ impl HttpCalloutFilter { } } - // Enforce the configured target authority on the Host header. - if let Ok(value) = self.target.authority.parse() { - headers.insert(http::header::HOST, value); - } - + // The shared URL executor supplies the configured authority as Host. headers } @@ -283,39 +285,6 @@ impl HttpCalloutFilter { } } - /// Resolve DNS for the target and construct an [`HttpPeer`]. - /// - /// When `allow_private_addresses` is `false`, the resolved peer address - /// is validated *after* resolution against the shared classifier - /// [`praxis_core::connectivity::is_private_ip`], so a hostname that - /// resolves to a private/loopback/link-local address (or rebinds to one - /// after config time) is rejected rather than connected to. Deferring to - /// core's predicate keeps this check consistent with the rest of the - /// proxy instead of adding another hand-rolled range list (see - /// praxis-proxy/ai#771). - async fn resolve_peer(&self) -> Result { - let addr = tokio::net::lookup_host((self.target.host.as_str(), self.target.port)) - .await - .map_err(|e| format!("DNS resolution failed for {}: {e}", self.target.host))? - .next() - .ok_or_else(|| format!("no addresses resolved for {}", self.target.host))?; - - if !self.allow_private_addresses && is_private_ip(&addr.ip()) { - return Err(format!( - "{} resolved to a blocked private/loopback address {} \ - (allow_private_addresses is false)", - self.target.host, - addr.ip() - )); - } - - Ok(HttpPeer::new( - addr.to_string(), - self.target.tls, - self.target.sni.clone(), - )) - } - /// The action to take when the callout itself fails (DNS, connect, /// I/O), per the configured failure mode. fn failure_action(&self) -> FilterAction { @@ -341,18 +310,15 @@ impl HttpCalloutFilter { /// Returns the response on success, or `None` when the callout /// itself failed (DNS/connect/I/O) and the caller should apply /// [`Self::failure_action`]. - async fn perform_callout(&self, request: &SubRequest, fw: &FrameworkHeaders) -> Option { - let peer = match self.resolve_peer().await { - Ok(p) => p, - Err(e) => { - warn!(url = %self.url, error = e, "callout failed"); - return None; - }, - }; - - match Box::pin( - self.client - .execute(&peer, request, self.max_body_bytes, self.timeout, Some(fw)), + async fn perform_callout(&self, request: SubRequest, fw: &FrameworkHeaders) -> Option { + match subrequest::execute_url_with_framework( + &self.client, + &self.url, + request, + self.max_body_bytes, + self.timeout, + AddressPolicy::from_allow_private(self.allow_private_addresses), + Some(fw), ) .await { @@ -383,7 +349,7 @@ impl HttpCalloutFilter { let (request, fw) = self.build_request(ctx, callout_body, depth); - let action = match Box::pin(self.perform_callout(&request, &fw)).await { + let action = match Box::pin(self.perform_callout(request, &fw)).await { Some(response) => self.handle_response(&response, ctx), None => self.failure_action(), }; @@ -473,107 +439,6 @@ fn compile_extractions(cfg: &HttpCalloutConfig) -> Result Result { - let parsed: http::Uri = url - .parse() - .map_err(|e| -> FilterError { format!("http_callout: invalid URL '{url}': {e}").into() })?; - - let tls = parse_scheme_tls(&parsed, url)?; - let host = parse_host(&parsed, url)?; - - let default_port = if tls { 443 } else { 80 }; - let port = parsed - .authority() - .and_then(http::uri::Authority::port_u16) - .unwrap_or(default_port); - let sni = if tls { host.clone() } else { String::new() }; - let authority = if port == default_port { - host.clone() - } else { - format!("{host}:{port}") - }; - - let path_and_query = parsed.path_and_query().map_or("/", |pq| pq.as_str()); - let request_uri: http::Uri = path_and_query - .parse() - .map_err(|e| -> FilterError { format!("http_callout: bad path in URL: {e}").into() })?; - - Ok(Self { - authority, - host, - port, - request_uri, - sni, - tls, - }) - } -} - -/// Determine whether the target scheme enables TLS (https) or not (http). -fn parse_scheme_tls(parsed: &http::Uri, url: &str) -> Result { - match parsed.scheme_str() { - Some("https") => Ok(true), - Some("http") => Ok(false), - _ => Err(format!("http_callout: scheme must be http or https in '{url}'").into()), - } -} - -/// Extract and validate the host from a parsed target URL. -fn parse_host(parsed: &http::Uri, url: &str) -> Result { - let authority = parsed - .authority() - .ok_or_else(|| -> FilterError { format!("http_callout: URL missing host: {url}").into() })?; - - // Reject userinfo (e.g. user:pass@host) to prevent credential leakage. - if url.contains('@') { - return Err(format!("http_callout: userinfo in URL is not allowed: {url}").into()); - } - - let host = authority - .host() - .trim_start_matches('[') - .trim_end_matches(']') - .to_owned(); - - if host.is_empty() { - return Err(format!("http_callout: empty host in URL: {url}").into()); - } - - Ok(host) -} - /// Build a [`SubRequestClient`] from parsed config. fn build_subrequest_client(cfg: &HttpCalloutConfig) -> SubRequestClient { let circuit_breaker = cfg.circuit_breaker.as_ref().map(|cb| CoreCircuitBreakerConfig { diff --git a/filters/src/callout/tests.rs b/filters/src/callout/tests.rs index fc19b5d737..752112c936 100644 --- a/filters/src/callout/tests.rs +++ b/filters/src/callout/tests.rs @@ -18,7 +18,7 @@ mod filter_tests { use std::time::Duration; - use praxis_filter::{BodyAccess, BodyMode, FilterAction}; + use praxis_filter::{BodyAccess, BodyMode, FilterAction, FilterError, HttpFilter}; use wiremock::{ Mock, MockServer, ResponseTemplate, matchers::{method, path}, @@ -26,6 +26,28 @@ mod filter_tests { use crate::{callout::HttpCalloutFilter, test_utils::make_filter_context}; + /// Build a test filter, explicitly opting local mock endpoints into the + /// private-address policy while leaving public-target fixtures unchanged. + fn test_filter(config: &serde_yaml::Value) -> Result, FilterError> { + let mut config = config.clone(); + let needs_private_opt_in = config + .get("target") + .and_then(|target| target.get("url")) + .and_then(serde_yaml::Value::as_str) + .is_some_and(|url| { + praxis_ai_apis::callout_target::validate_configured_http_target( + "http_callout test", + url, + praxis_ai_apis::callout_target::AddressPolicy::PublicOnly, + ) + .is_err() + }); + if needs_private_opt_in { + config["target"]["allow_private_addresses"] = serde_yaml::Value::Bool(true); + } + HttpCalloutFilter::from_config(&config) + } + // ------------------------------------------------------------------------- // Config Parsing // ------------------------------------------------------------------------- @@ -40,7 +62,7 @@ mod filter_tests { ) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); assert_eq!(filter.name(), "http_callout"); } @@ -277,15 +299,14 @@ mod filter_tests { .err() .unwrap_or_else(|| panic!("URL with userinfo should be rejected: {bad}")); assert!( - err.to_string().contains("userinfo"), - "error should mention userinfo for {bad}: {err}" + err.to_string().contains("embedded credentials"), + "error should mention embedded credentials for {bad}: {err}" ); } } #[test] - fn config_warns_on_private_ip_url() { - // Private/loopback URLs should succeed (warning only). + fn config_rejects_private_ip_url_without_opt_in() { let yaml = serde_yaml::from_str::( r#" target: @@ -295,7 +316,7 @@ mod filter_tests { .unwrap(); let filter = HttpCalloutFilter::from_config(&yaml); - assert!(filter.is_ok(), "private/loopback URL should succeed with a warning"); + assert!(filter.is_err(), "private/loopback URL should require explicit opt-in"); } #[test] @@ -326,43 +347,28 @@ mod filter_tests { // ------------------------------------------------------------------------- #[test] - fn target_parse_https_enables_tls_sni_and_default_port() { - // An https URL without an explicit port must enable TLS, set the - // SNI to the host, default the port to 443, and omit the port from - // the Host authority (since 443 is the default for the scheme). - let target = crate::callout::CalloutTarget::parse("https://example.com/api").unwrap(); - - assert!(target.tls, "https should enable TLS"); - assert_eq!(target.port, 443, "https should default to port 443"); - assert_eq!(target.sni, "example.com", "SNI should be the host"); - assert_eq!(target.host, "example.com"); - assert_eq!(target.authority, "example.com", "default port omitted from authority"); - assert_eq!(target.request_uri, "/api"); + fn target_parse_https_preserves_target_components() { + let target = + praxis_ai_apis::callout_target::validate_http_target("http_callout", "https://example.com:8443/api") + .unwrap(); + + assert_eq!(target.scheme(), "https"); + assert_eq!(target.host_str(), Some("example.com")); + assert_eq!(target.port(), Some(8443)); + assert_eq!(target.path(), "/api"); } #[test] - fn target_parse_https_explicit_nondefault_port_in_authority() { - // A non-default https port must appear in the Host authority. - let target = crate::callout::CalloutTarget::parse("https://example.com:8443/api").unwrap(); - - assert!(target.tls); - assert_eq!(target.port, 8443); - assert_eq!(target.sni, "example.com", "SNI is the host, not host:port"); - assert_eq!( - target.authority, "example.com:8443", - "non-default port kept in authority" + fn target_parse_rejects_userinfo() { + assert!( + praxis_ai_apis::callout_target::validate_http_target("http_callout", "https://user:pass@example.com/api",) + .is_err() ); } #[test] - fn target_parse_http_disables_tls_and_leaves_sni_empty() { - // An http URL defaults to port 80, disables TLS, and has no SNI. - let target = crate::callout::CalloutTarget::parse("http://example.com/api").unwrap(); - - assert!(!target.tls, "http should disable TLS"); - assert_eq!(target.port, 80, "http should default to port 80"); - assert!(target.sni.is_empty(), "no SNI without TLS"); - assert_eq!(target.authority, "example.com", "default port omitted from authority"); + fn target_parse_rejects_non_http_scheme() { + assert!(praxis_ai_apis::callout_target::validate_http_target("http_callout", "file:///tmp/secret").is_err()); } // ------------------------------------------------------------------------- @@ -381,7 +387,7 @@ mod filter_tests { ) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); assert_eq!(filter.request_body_access(), BodyAccess::None); assert_eq!(filter.request_body_mode(), BodyMode::Stream); } @@ -398,7 +404,7 @@ mod filter_tests { ) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); assert_eq!(filter.request_body_access(), BodyAccess::ReadOnly); assert!( matches!( @@ -422,7 +428,7 @@ mod filter_tests { ) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); assert_eq!(filter.name(), crate::callout::FILTER_NAME); assert_eq!(filter.name(), "http_callout"); } @@ -437,7 +443,7 @@ mod filter_tests { ) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); assert!(filter.needs_request_context()); } @@ -475,7 +481,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -522,7 +528,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -568,7 +574,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -613,7 +619,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -647,7 +653,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -693,7 +699,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -714,7 +720,7 @@ mod filter_tests { // ------------------------------------------------------------------------- #[tokio::test] - async fn private_target_blocked_when_addresses_disallowed() { + async fn private_target_requires_explicit_opt_in() { // A live server on loopback that would answer 200 if reached. With // allow_private_addresses: false, resolve_peer must reject the // loopback peer *before* any request, so the callout fails and @@ -740,27 +746,14 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); - - let req = praxis_filter::Request { - method: http::Method::POST, - uri: "/test".parse().unwrap(), - headers: http::HeaderMap::new(), - }; - let mut ctx = make_filter_context(&req); - - let action = filter.on_request(&mut ctx).await.unwrap(); assert!( - matches!(action, FilterAction::Reject(r) if r.status == 502), - "a resolved private/loopback peer must be blocked when disallowed" + HttpCalloutFilter::from_config(&yaml).is_err(), + "a literal private target must fail configuration without the opt-in" ); } #[tokio::test] - async fn private_target_allowed_by_default() { - // The default (allow_private_addresses omitted -> true) preserves the - // permissive loopback-guard use case: a callout to a loopback server - // succeeds and continues the request. + async fn private_target_allowed_with_explicit_opt_in() { let mock_server = MockServer::start().await; Mock::given(method("POST")) .and(path("/guard")) @@ -772,6 +765,7 @@ mod filter_tests { r#" target: url: "{}/guard" + allow_private_addresses: true request: phase: request_headers on_failure: closed @@ -780,7 +774,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -792,7 +786,7 @@ mod filter_tests { let action = filter.on_request(&mut ctx).await.unwrap(); assert!( matches!(action, FilterAction::Continue), - "a loopback target must be reachable under the permissive default" + "a loopback target must be reachable after explicit opt-in" ); } @@ -824,7 +818,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -868,7 +862,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let mut headers = http::HeaderMap::new(); headers.insert("x-praxis-iterative-depth", "1".parse().unwrap()); @@ -915,7 +909,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let mut headers = http::HeaderMap::new(); headers.insert("x-custom", "my-value".parse().unwrap()); @@ -963,7 +957,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -1009,7 +1003,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -1044,7 +1038,7 @@ mod filter_tests { ) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -1085,7 +1079,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -1114,7 +1108,7 @@ mod filter_tests { ) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -1166,7 +1160,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -1213,7 +1207,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); // Fire enough requests to trip the breaker via connect failures. for _ in 0..3 { @@ -1270,7 +1264,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -1309,7 +1303,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, @@ -1354,7 +1348,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); // Downstream request omits the configured forward header. let req = praxis_filter::Request { @@ -1402,7 +1396,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); // The client supplies BOTH headers downstream. let mut headers = http::HeaderMap::new(); @@ -1466,7 +1460,7 @@ mod filter_tests { )) .unwrap(); - let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + let filter = test_filter(&yaml).unwrap(); let req = praxis_filter::Request { method: http::Method::POST, diff --git a/filters/src/gcp/filter.rs b/filters/src/gcp/filter.rs index bcec5bfa15..268c071455 100644 --- a/filters/src/gcp/filter.rs +++ b/filters/src/gcp/filter.rs @@ -68,6 +68,11 @@ const TOKEN_REQUEST_TIMEOUT: Duration = Duration::from_secs(30); /// fetch fails — the request is rejected with `503` rather than /// forwarded unauthenticated. /// +/// Metadata requests use a proxy-free, redirect-free client pinned to the +/// complete validated DNS result set. Metadata mode intentionally permits +/// the protocol-owned private endpoint; arbitrary private hosts remain +/// invalid configuration. +/// /// # YAML configuration /// /// ```yaml @@ -80,10 +85,6 @@ pub struct GcpAdcFilter { /// [`praxis_ai_apis::token_cache`]. cache: TokenCache, - /// HTTP client used for metadata-server requests, built once with - /// [`TOKEN_REQUEST_TIMEOUT`]. - client: reqwest::Client, - /// Resolved credential source. source: TokenSource, @@ -106,18 +107,12 @@ impl GcpAdcFilter { /// /// Returns [`FilterError`] if `service_account` or `metadata_host` /// are structurally unsafe, a field is set that its `source` does - /// not use, ADC file resolution fails, or the HTTP client fails to - /// build. + /// not use, or ADC file resolution fails. fn new(config: &GcpAdcConfig, application_credentials: Option<&std::path::Path>) -> Result { validate_config(config)?; let source = resolve_token_source(config, application_credentials)?; - let client = reqwest::Client::builder() - .timeout(TOKEN_REQUEST_TIMEOUT) - .build() - .map_err(|e| FilterError::from(format!("gcp_adc: failed to build HTTP client: {e}")))?; Ok(Self { cache: TokenCache::new(EXPIRY_SKEW), - client, source, scope: config.scope.clone(), metadata_host: config.metadata_host.clone(), @@ -162,7 +157,9 @@ impl praxis_filter::HttpFilter for GcpAdcFilter { ) -> Result { let fetched = self .cache - .get_or_refresh(|| token::fetch(&self.client, &self.source, &self.metadata_host, &self.scope)) + .get_or_refresh(|| { + token::fetch_pinned(&self.source, &self.metadata_host, &self.scope, TOKEN_REQUEST_TIMEOUT) + }) .await; match fetched { Ok(authorization) => { diff --git a/filters/src/gcp/token.rs b/filters/src/gcp/token.rs index 80f8fb055b..3b1bea70d7 100644 --- a/filters/src/gcp/token.rs +++ b/filters/src/gcp/token.rs @@ -65,6 +65,7 @@ struct MetadataTokenResponse { /// Returns [`FilterError`] if the metadata request fails, returns a /// non-success status, or its body cannot be parsed; or, for /// [`TokenSource::ServiceAccountKey`], always (not implemented). +#[cfg(test)] pub(super) async fn fetch( client: &reqwest::Client, source: &TokenSource, @@ -82,17 +83,58 @@ pub(super) async fn fetch( } } +/// Acquire a token through a proxy-free, redirect-free client pinned to +/// every validated metadata-server address returned by one DNS lookup. +/// +/// The metadata protocol intentionally targets a private endpoint. The +/// configured host is separately restricted to Google's metadata hostname +/// or a literal loopback test host. +pub(super) async fn fetch_pinned( + source: &TokenSource, + metadata_host: &str, + scope: &str, + timeout: Duration, +) -> Result<(HeaderValue, Duration), FilterError> { + let TokenSource::Metadata { service_account } = source else { + return Err(FilterError::from( + "gcp_adc: token fetch for source key_file is not implemented yet (requires JWT signing); \ + use source: adc or source: metadata on a GCE/GKE instance instead", + )); + }; + + let url = metadata_token_url(metadata_host, service_account, scope); + let client = praxis_ai_apis::callout_target::build_pinned_reqwest_client( + "gcp_adc", + &url, + praxis_ai_apis::callout_target::AddressPolicy::AllowPrivate, + timeout, + ) + .await?; + fetch_metadata_token_url(&client, &url).await +} + /// Acquire a token from the GCE/GKE metadata server. +#[cfg(test)] async fn fetch_metadata_token( client: &reqwest::Client, metadata_host: &str, service_account: &str, scope: &str, ) -> Result<(HeaderValue, Duration), FilterError> { + let url = metadata_token_url(metadata_host, service_account, scope); + fetch_metadata_token_url(client, &url).await +} + +/// Build the metadata token URL from already validated components. +fn metadata_token_url(metadata_host: &str, service_account: &str, scope: &str) -> String { let mut url = format!("http://{metadata_host}/computeMetadata/v1/instance/service-accounts/{service_account}/token?scopes="); url::form_urlencoded::byte_serialize(scope.as_bytes()).for_each(|piece| url.push_str(piece)); + url +} +/// Send one metadata token request with a caller-configured client. +async fn fetch_metadata_token_url(client: &reqwest::Client, url: &str) -> Result<(HeaderValue, Duration), FilterError> { let response = client .get(url) .header("Metadata-Flavor", "Google") diff --git a/filters/src/guardrails/filter.rs b/filters/src/guardrails/filter.rs index c741675d86..693369e1df 100644 --- a/filters/src/guardrails/filter.rs +++ b/filters/src/guardrails/filter.rs @@ -33,6 +33,7 @@ const DEFAULT_MAX_BODY_BYTES: usize = 1_048_576; /// provider: /// type: nemo /// endpoint: "http://nemo:8000/v1/guardrail/checks" +/// allow_private_endpoint: true /// timeout_ms: 5000 /// phase: /// request: true @@ -49,6 +50,7 @@ const DEFAULT_MAX_BODY_BYTES: usize = 1_048_576; /// provider: /// type: nemo /// endpoint: "http://nemo:8000/v1/guardrail/checks" +/// allow_private_endpoint: true /// "#, /// ) /// .unwrap(); diff --git a/filters/src/guardrails/providers/nemo.rs b/filters/src/guardrails/providers/nemo.rs index 5b5dad1890..83f82fbca1 100644 --- a/filters/src/guardrails/providers/nemo.rs +++ b/filters/src/guardrails/providers/nemo.rs @@ -28,6 +28,10 @@ struct NemoConfig { /// `NeMo` endpoint URL. endpoint: String, + /// Allow the endpoint to resolve to non-public addresses. + #[serde(default)] + allow_private_endpoint: bool, + /// Model name sent in each request. Defaults to `""` when omitted. #[serde(default)] model: String, @@ -80,6 +84,9 @@ pub(in crate::guardrails) struct NemoProvider { /// Per-request deadline covering admission, connect, and I/O. timeout: Duration, + + /// Connect-time policy for the configured endpoint. + address_policy: praxis_ai_apis::callout_target::AddressPolicy, } impl NemoProvider { @@ -99,6 +106,13 @@ impl NemoProvider { if cfg.endpoint.is_empty() { return Err("ai_guardrails (nemo): 'endpoint' must not be empty".into()); } + let address_policy = + praxis_ai_apis::callout_target::AddressPolicy::from_allow_private(cfg.allow_private_endpoint); + praxis_ai_apis::callout_target::validate_configured_http_target( + "ai_guardrails (nemo)", + &cfg.endpoint, + address_policy, + )?; if cfg.timeout_ms == 0 { return Err("ai_guardrails (nemo): 'timeout_ms' must be greater than zero".into()); } @@ -108,6 +122,7 @@ impl NemoProvider { endpoint: cfg.endpoint, model: cfg.model, timeout: Duration::from_millis(cfg.timeout_ms), + address_policy, }) } } @@ -116,9 +131,16 @@ impl NemoProvider { impl GuardProvider for NemoProvider { async fn evaluate(&self, messages: Vec, _phase: GuardPhase) -> Result { let request = build_request(&self.model, messages)?; - let response = subrequest::execute_url(&self.client, &self.endpoint, request, MAX_RESPONSE_SIZE, self.timeout) - .await - .map_err(|error| map_subrequest_error(&error))?; + let response = subrequest::execute_url( + &self.client, + &self.endpoint, + request, + MAX_RESPONSE_SIZE, + self.timeout, + self.address_policy, + ) + .await + .map_err(|error| map_subrequest_error(&error))?; ensure_success_status(&response)?; let nemo_response: NemoResponse = serde_json::from_slice(&response.body) .map_err(|e| -> FilterError { format!("ai_guardrails (nemo): failed to parse response: {e}").into() })?; diff --git a/filters/src/guardrails/tests.rs b/filters/src/guardrails/tests.rs index 2b4b243577..9fc300c128 100644 --- a/filters/src/guardrails/tests.rs +++ b/filters/src/guardrails/tests.rs @@ -18,6 +18,7 @@ fn nemo_filter(endpoint: &str) -> Box { provider: type: nemo endpoint: "{endpoint}" + allow_private_endpoint: true "#, )) .unwrap(); @@ -76,6 +77,23 @@ phase: assert_eq!(filter.name(), "ai_guardrails"); } +#[test] +fn nemo_private_endpoint_requires_explicit_opt_in() { + let yaml: serde_yaml::Value = serde_yaml::from_str( + r#" +provider: + type: nemo + endpoint: "http://127.0.0.1:8000/v1/guardrail/checks" +"#, + ) + .unwrap(); + + assert!( + AiGuardrailsFilter::from_config(&yaml).is_err(), + "loopback NeMo endpoint must require allow_private_endpoint" + ); +} + #[test] fn phase_response_true_rejected() { let yaml: serde_yaml::Value = serde_yaml::from_str( diff --git a/tests/integration/tests/suite/examples/azure_ad.rs b/tests/integration/tests/suite/examples/azure_ad.rs index 09f410d684..074d441025 100644 --- a/tests/integration/tests/suite/examples/azure_ad.rs +++ b/tests/integration/tests/suite/examples/azure_ad.rs @@ -48,7 +48,7 @@ fn azure_ad_fails_closed_without_token() { // fetch can never acquire a token (deterministic fail-closed). let patched = patched.replace( " client_secret_env_var: AZURE_CLIENT_SECRET", - " client_secret_env_var: CARGO_PKG_NAME\n authority_host: 127.0.0.1:1", + " client_secret_env_var: CARGO_PKG_NAME\n authority_host: 127.0.0.1:1\n allow_private_authority: true", ); let config = praxis_core::config::Config::from_yaml(&patched).unwrap_or_else(|e| panic!("parse azure-ad.yaml: {e}")); diff --git a/tests/integration/tests/suite/examples/lakera_guard.rs b/tests/integration/tests/suite/examples/lakera_guard.rs index eb9bb89774..eab7634dc2 100644 --- a/tests/integration/tests/suite/examples/lakera_guard.rs +++ b/tests/integration/tests/suite/examples/lakera_guard.rs @@ -104,23 +104,21 @@ fn load_lakera_config(proxy_port: u16, lakera_port: u16, backend_port: u16) -> p let path = example_config_path("lakera-guard.yaml"); let yaml = std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("read {path}: {e}")); - // Replace the real Lakera API URL with our mock and remove - // the Authorization header that references an env var. - // - // The example hardens against SSRF with `allow_private_addresses: - // false`, but the mock Lakera backend runs on loopback, so drop that - // line for the test harness — otherwise resolve_peer would (correctly) - // block the loopback callout. + // Replace the real public API with the trusted loopback mock, explicitly + // opt into that private target, and remove the real credential header. let yaml = yaml .replace( - "https://api.lakera.ai/v2/guard", - &format!("http://127.0.0.1:{lakera_port}/v2/guard"), + " url: \"https://api.lakera.ai/v2/guard\"", + &format!(" url: \"http://127.0.0.1:{lakera_port}/v2/guard\""), + ) + .replace( + " allow_private_addresses: false", + " allow_private_addresses: true", ) .replace( " - name: \"Authorization\"\n value: \"Bearer ${LAKERA_API_KEY}\"\n", "", - ) - .replace(" allow_private_addresses: false\n", ""); + ); let patched = patch_yaml(&yaml, proxy_port, &HashMap::from([("127.0.0.1:3000", backend_port)])); praxis_core::config::Config::from_yaml(&patched).unwrap_or_else(|e| panic!("parse lakera-guard.yaml: {e}")) From a5f6c2f08c3b1dbe1d660b76a9a2eb6105e21c66 Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 13:06:35 +0100 Subject: [PATCH 02/13] fix: remove unused pingora-core dependency Signed-off-by: Eoin Fennessy --- Cargo.lock | 1 - filters/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cd3e8dd3a8..722395070f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2734,7 +2734,6 @@ dependencies = [ "praxis-ai-apis", "praxis-proxy-core", "praxis-proxy-filter", - "quixotic-plecostomus-core", "rand 0.10.2", "redis", "reqwest", diff --git a/filters/Cargo.toml b/filters/Cargo.toml index 81ddecc72b..c710358c05 100644 --- a/filters/Cargo.toml +++ b/filters/Cargo.toml @@ -55,7 +55,6 @@ dashmap = { workspace = true } http = { workspace = true } metrics = { workspace = true } notify = { workspace = true } -pingora-core.workspace = true praxis-ai-apis = { workspace = true } praxis-core = { workspace = true } praxis-filter = { workspace = true } From a09165f89172d2d2f5f594554923dca118944416 Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 14:15:56 +0100 Subject: [PATCH 03/13] fix(callouts): reject legacy IPv4 targets Signed-off-by: Eoin Fennessy --- apis/src/callout_target.rs | 59 +++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/apis/src/callout_target.rs b/apis/src/callout_target.rs index a0ffc084a8..d38d5b7e5d 100644 --- a/apis/src/callout_target.rs +++ b/apis/src/callout_target.rs @@ -8,7 +8,7 @@ //! before the validated socket addresses are handed to the transport. This //! closes the DNS-rebinding gap left by startup-only URL validation. -use std::net::{IpAddr, SocketAddr}; +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use praxis_core::connectivity::normalize_mapped_ipv4; use praxis_filter::FilterError; @@ -97,10 +97,56 @@ pub fn validate_configured_http_target( } if let Ok(ip) = unbracketed.parse::() { validate_ip(filter_name, ip, policy)?; + } else if let Some(ip) = parse_legacy_ipv4_host(unbracketed) { + validate_ip(filter_name, IpAddr::V4(ip), policy)?; } Ok(parsed) } +/// Parse legacy IPv4 literals accepted by common libc resolvers. +fn parse_legacy_ipv4_host(host: &str) -> Option { + let host = host.trim_end_matches('.'); + let parts: Vec<_> = host.split('.').collect(); + if parts.is_empty() || parts.len() > 4 || parts.iter().any(|part| part.is_empty()) { + return None; + } + + let mut numbers = Vec::with_capacity(parts.len()); + for part in parts { + numbers.push(parse_legacy_ipv4_number(part)?); + } + + let addr = match numbers.as_slice() { + [a] => *a, + [a, b] if *a <= 0xFF && *b <= 0x00FF_FFFF => (*a << 24) | *b, + [a, b, c] if *a <= 0xFF && *b <= 0xFF && *c <= 0xFFFF => (*a << 24) | (*b << 16) | *c, + [a, b, c, d] if numbers.iter().all(|part| *part <= 0xFF) => (*a << 24) | (*b << 16) | (*c << 8) | *d, + _ => return None, + }; + + Some(Ipv4Addr::from(addr)) +} + +/// Parse a decimal, octal, or hexadecimal legacy IPv4 component. +fn parse_legacy_ipv4_number(part: &str) -> Option { + let (digits, radix) = part.strip_prefix("0x").or_else(|| part.strip_prefix("0X")).map_or_else( + || { + if part.len() > 1 && part.starts_with('0') { + (part.get(1..).unwrap_or_default(), 8) + } else { + (part, 10) + } + }, + |digits| (digits, 16), + ); + + if digits.is_empty() || !digits.chars().all(|c| c.is_digit(radix)) { + return None; + } + + u32::from_str_radix(digits, radix).ok() +} + /// Validate every address returned by one DNS lookup. /// /// The complete answer set is rejected when any address is disallowed. This @@ -222,6 +268,17 @@ mod tests { assert!(validate_http_target("test", "https://user:pass@example.com/path").is_err()); } + #[test] + fn public_only_rejects_legacy_ipv4_literals() { + for host in ["127.1", "2130706433", "0x7f.0.0.1", "0177.0.0.1", "0x7f000001"] { + assert!( + validate_configured_http_target("test", &format!("http://{host}:8080"), AddressPolicy::PublicOnly) + .is_err(), + "legacy IPv4 literal {host} should be rejected" + ); + } + } + #[test] fn mixed_dns_answer_is_rejected() { let addrs = ["8.8.8.8:443".parse().unwrap(), "169.254.169.254:443".parse().unwrap()]; From 0214cfc238153876f009a1cb337e6c5004a85b0d Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 14:17:51 +0100 Subject: [PATCH 04/13] refactor(callouts): remove dead IPv6 bracket handling Signed-off-by: Eoin Fennessy --- apis/src/callout_target.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/apis/src/callout_target.rs b/apis/src/callout_target.rs index d38d5b7e5d..529ebeb1d1 100644 --- a/apis/src/callout_target.rs +++ b/apis/src/callout_target.rs @@ -86,18 +86,14 @@ pub fn validate_configured_http_target( ) -> Result { let parsed = validate_http_target(filter_name, raw)?; let host = parsed.host_str().unwrap_or_default(); - let unbracketed = host - .strip_prefix('[') - .and_then(|host| host.strip_suffix(']')) - .unwrap_or(host); - if !policy.allows_private() && unbracketed.trim_end_matches('.').eq_ignore_ascii_case("localhost") { + if !policy.allows_private() && host.trim_end_matches('.').eq_ignore_ascii_case("localhost") { return Err( format!("{filter_name}: target URL targets localhost; enable the private-target opt-in to allow").into(), ); } - if let Ok(ip) = unbracketed.parse::() { + if let Ok(ip) = host.parse::() { validate_ip(filter_name, ip, policy)?; - } else if let Some(ip) = parse_legacy_ipv4_host(unbracketed) { + } else if let Some(ip) = parse_legacy_ipv4_host(host) { validate_ip(filter_name, IpAddr::V4(ip), policy)?; } Ok(parsed) @@ -230,11 +226,7 @@ pub async fn build_pinned_reqwest_client( .no_proxy() .redirect(reqwest::redirect::Policy::none()); - let unbracketed = host - .strip_prefix('[') - .and_then(|host| host.strip_suffix(']')) - .unwrap_or(host); - if let Ok(ip) = unbracketed.parse::() { + if let Ok(ip) = host.parse::() { validate_ip(filter_name, ip, policy)?; } else { let resolved = tokio::time::timeout(timeout, tokio::net::lookup_host((host, port))) From a74a23ed46290d4b7ddaadddc41c1649eed06657 Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 14:18:35 +0100 Subject: [PATCH 05/13] refactor(callouts): avoid duplicate literal validation Signed-off-by: Eoin Fennessy --- apis/src/callout_target.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apis/src/callout_target.rs b/apis/src/callout_target.rs index 529ebeb1d1..974caedab2 100644 --- a/apis/src/callout_target.rs +++ b/apis/src/callout_target.rs @@ -226,9 +226,7 @@ pub async fn build_pinned_reqwest_client( .no_proxy() .redirect(reqwest::redirect::Policy::none()); - if let Ok(ip) = host.parse::() { - validate_ip(filter_name, ip, policy)?; - } else { + if host.parse::().is_err() { let resolved = tokio::time::timeout(timeout, tokio::net::lookup_host((host, port))) .await .map_err(|_elapsed| -> FilterError { format!("{filter_name}: DNS resolution timed out").into() })? From a19caa7a5c509f3585abee243cb2fcf6273068de Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 14:19:15 +0100 Subject: [PATCH 06/13] fix(callouts): report malformed credential URLs Signed-off-by: Eoin Fennessy --- apis/src/openai/api_client/mod.rs | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/apis/src/openai/api_client/mod.rs b/apis/src/openai/api_client/mod.rs index 9fa2ce62c7..0e07c4ede1 100644 --- a/apis/src/openai/api_client/mod.rs +++ b/apis/src/openai/api_client/mod.rs @@ -239,9 +239,12 @@ impl ApiClient { max_response_bytes: usize, ) -> Result { let candidate_origin = ::url::Url::parse(url) - .ok() - .map(|url| url.origin().ascii_serialization()); - if candidate_origin.is_none() || candidate_origin != self.target_origin { + .map_err(|_error| ApiClientError::Transport { + source: SubRequestError::InvalidRequest("malformed callout URL".to_owned()), + })? + .origin() + .ascii_serialization(); + if Some(candidate_origin) != self.target_origin { return Err(ApiClientError::Transport { source: SubRequestError::InvalidRequest( "callout URL changed the configured credential origin".to_owned(), @@ -433,6 +436,23 @@ mod tests { )); } + #[tokio::test] + async fn malformed_callout_url_is_rejected_before_origin_comparison() { + let client = test_client("https://api.example.com"); + + let error = client + .get("not a valid URL", &HeaderMap::new(), 1024) + .await + .expect_err("a malformed callout URL must be rejected before I/O"); + + assert!(matches!( + error, + ApiClientError::Transport { + source: SubRequestError::InvalidRequest(detail), + } if detail == "malformed callout URL" + )); + } + #[tokio::test] async fn get_bytes_preserves_redirect_without_following_it() { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); From 3f7f54be87e6de5bd9301782164299086e011814 Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 14:20:51 +0100 Subject: [PATCH 07/13] chore(callouts): document credential validation boundary Signed-off-by: Eoin Fennessy --- apis/src/openai/api_client/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apis/src/openai/api_client/mod.rs b/apis/src/openai/api_client/mod.rs index 0e07c4ede1..91a7ec97f3 100644 --- a/apis/src/openai/api_client/mod.rs +++ b/apis/src/openai/api_client/mod.rs @@ -228,7 +228,8 @@ impl ApiClient { /// the caller's response-size limit. #[expect( clippy::too_many_arguments, - reason = "the request's independently owned transport fields stay explicit" + clippy::too_many_lines, + reason = "the request's independently owned transport fields and credential-origin binding stay explicit" )] async fn execute_url( &self, From f7fbcba321019e30fd317c90a13a18832cdd1a18 Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 14:22:44 +0100 Subject: [PATCH 08/13] docs(web-search): describe pinned DNS validation Signed-off-by: Eoin Fennessy --- apis/src/web_search/config.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/apis/src/web_search/config.rs b/apis/src/web_search/config.rs index d44aecd4ca..676081aaf7 100644 --- a/apis/src/web_search/config.rs +++ b/apis/src/web_search/config.rs @@ -163,13 +163,10 @@ pub(crate) struct OpenAiWebSearchConfig { /// Allow a `base_url` that targets local-sensitive addresses. /// - /// DNS targets are unsupported in protected mode (the default): - /// validation cannot pin the address the HTTP client will eventually - /// dial, so a `base_url` host must be a public IP literal. Enabling - /// `allow_private_base_url` also permits DNS results resolving to - /// local-sensitive addresses, so a hostile or rebound resolution can - /// send the provider credential to a loopback, private, or - /// cloud-metadata endpoint. + /// DNS names are resolved once per request and every result is checked + /// immediately before the transport connects. By default, any private, + /// loopback, link-local, or otherwise non-public result rejects the + /// callout. Enable this only for a trusted private provider endpoint. #[serde(default)] allow_private_base_url: bool, } From 3bab1ca606616595775aa9c0500a8c7231eeb960 Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 14:40:51 +0100 Subject: [PATCH 09/13] fix(callouts): validate bracketed IPv6 targets Signed-off-by: Eoin Fennessy --- apis/src/callout_target.rs | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/apis/src/callout_target.rs b/apis/src/callout_target.rs index 974caedab2..cd087590f0 100644 --- a/apis/src/callout_target.rs +++ b/apis/src/callout_target.rs @@ -86,14 +86,22 @@ pub fn validate_configured_http_target( ) -> Result { let parsed = validate_http_target(filter_name, raw)?; let host = parsed.host_str().unwrap_or_default(); - if !policy.allows_private() && host.trim_end_matches('.').eq_ignore_ascii_case("localhost") { + let host_without_brackets = host + .strip_prefix('[') + .and_then(|host| host.strip_suffix(']')) + .unwrap_or(host); + if !policy.allows_private() + && host_without_brackets + .trim_end_matches('.') + .eq_ignore_ascii_case("localhost") + { return Err( format!("{filter_name}: target URL targets localhost; enable the private-target opt-in to allow").into(), ); } - if let Ok(ip) = host.parse::() { + if let Ok(ip) = host_without_brackets.parse::() { validate_ip(filter_name, ip, policy)?; - } else if let Some(ip) = parse_legacy_ipv4_host(host) { + } else if let Some(ip) = parse_legacy_ipv4_host(host_without_brackets) { validate_ip(filter_name, IpAddr::V4(ip), policy)?; } Ok(parsed) @@ -218,6 +226,12 @@ pub async fn build_pinned_reqwest_client( let host = parsed .host_str() .ok_or_else(|| -> FilterError { format!("{filter_name}: target URL must include a host").into() })?; + // `Url::host_str()` serializes IPv6 hosts with brackets; remove them + // before passing the host to `IpAddr` parsing or DNS resolution. + let host_without_brackets = host + .strip_prefix('[') + .and_then(|host| host.strip_suffix(']')) + .unwrap_or(host); let port = parsed .port_or_known_default() .ok_or_else(|| -> FilterError { format!("{filter_name}: target URL has no usable port").into() })?; @@ -226,8 +240,8 @@ pub async fn build_pinned_reqwest_client( .no_proxy() .redirect(reqwest::redirect::Policy::none()); - if host.parse::().is_err() { - let resolved = tokio::time::timeout(timeout, tokio::net::lookup_host((host, port))) + if host_without_brackets.parse::().is_err() { + let resolved = tokio::time::timeout(timeout, tokio::net::lookup_host((host_without_brackets, port))) .await .map_err(|_elapsed| -> FilterError { format!("{filter_name}: DNS resolution timed out").into() })? .map_err(|error| -> FilterError { From e01788cd45ec37edac5eada3ffd91e2f9c2c191e Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 21:31:58 +0100 Subject: [PATCH 10/13] fix(callouts): bind Host to target authority Signed-off-by: Eoin Fennessy --- filters/src/callout/mod.rs | 23 ++++++++++++++++++++-- filters/src/callout/tests.rs | 38 ++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/filters/src/callout/mod.rs b/filters/src/callout/mod.rs index 0f8aa7c63e..1f19de9cbf 100644 --- a/filters/src/callout/mod.rs +++ b/filters/src/callout/mod.rs @@ -111,6 +111,9 @@ pub struct HttpCalloutFilter { /// Static headers to send with every callout. headers: Vec<(http::HeaderName, http::HeaderValue)>, + /// Authority from the configured URL, used as the callout `Host`. + target_authority: http::HeaderValue, + /// Callout response headers to inject into the upstream /// request on success. inject_headers: Vec, @@ -155,6 +158,7 @@ impl HttpCalloutFilter { let body_shaper = BodyShaper::compile(&cfg.target.body)?; let headers = parse_static_headers(&cfg)?; + let target_authority = parse_target_authority(&cfg.target.url)?; let forward_headers = parse_header_names(&cfg.target.forward_headers, "forward_header")?; warn_on_disallowed_forward_headers(&forward_headers); let extractions = compile_extractions(&cfg)?; @@ -180,6 +184,7 @@ impl HttpCalloutFilter { max_depth: cfg.max_depth.unwrap_or(1), phase: cfg.request.phase, status_on_error, + target_authority, timeout: cfg.target.timeout, url: cfg.target.url, })) @@ -211,7 +216,7 @@ impl HttpCalloutFilter { } /// Assemble the callout request headers: static configured headers, - /// safely-forwarded client headers, and the enforced `Host`. + /// safely-forwarded client headers, and the target-bound `Host`. fn build_callout_headers(&self, ctx: &HttpFilterContext<'_>) -> HeaderMap { let mut headers = HeaderMap::new(); @@ -230,7 +235,9 @@ impl HttpCalloutFilter { } } - // The shared URL executor supplies the configured authority as Host. + // Host is security-sensitive: the shared executor preserves an + // explicitly supplied value, so bind it here to the target URL. + headers.insert(http::header::HOST, self.target_authority.clone()); headers } @@ -401,6 +408,18 @@ fn parse_static_headers(cfg: &HttpCalloutConfig) -> Result Result { + let uri: http::Uri = url + .parse() + .map_err(|e| -> FilterError { format!("http_callout: invalid target URL '{url}': {e}").into() })?; + let authority = uri + .authority() + .ok_or_else(|| FilterError::from(format!("http_callout: target URL has no authority: {url}")))?; + http::HeaderValue::from_str(authority.as_str()) + .map_err(|e| format!("http_callout: invalid target authority '{}': {e}", authority.as_str()).into()) +} + /// Parse a list of header name strings. fn parse_header_names(names: &[String], context: &str) -> Result, FilterError> { names diff --git a/filters/src/callout/tests.rs b/filters/src/callout/tests.rs index e806e64901..339e7a106e 100644 --- a/filters/src/callout/tests.rs +++ b/filters/src/callout/tests.rs @@ -925,6 +925,44 @@ mod filter_tests { assert!(matches!(action, FilterAction::Continue), "forward_headers should work"); } + #[tokio::test] + async fn static_host_is_overwritten_with_target_authority_on_wire() { + let mock_server = MockServer::start().await; + let authority = mock_server.address().to_string(); + + Mock::given(method("POST")) + .and(path("/guard")) + .and(wiremock::matchers::header("host", authority)) + .respond_with(ResponseTemplate::new(200)) + .mount(&mock_server) + .await; + + let yaml = serde_yaml::from_str::(&format!( + r#" + target: + url: "{}/guard" + headers: + - name: "Host" + value: "attacker.example" + request: + phase: request_headers + "#, + mock_server.uri() + )) + .unwrap(); + + let filter = test_filter(&yaml).unwrap(); + let req = praxis_filter::Request { + method: http::Method::POST, + uri: "/test".parse().unwrap(), + headers: http::HeaderMap::new(), + }; + let mut ctx = make_filter_context(&req); + + let action = filter.on_request(&mut ctx).await.unwrap(); + assert!(matches!(action, FilterAction::Continue)); + } + // ------------------------------------------------------------------------- // Inject Headers // ------------------------------------------------------------------------- From e5cbaa0f4e80cfb64e6bad2e042742dffcfc1d3a Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 21:37:43 +0100 Subject: [PATCH 11/13] fix(apis): reject empty URL userinfo Signed-off-by: Eoin Fennessy --- apis/src/callout_target.rs | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/apis/src/callout_target.rs b/apis/src/callout_target.rs index cd087590f0..0201cd64b5 100644 --- a/apis/src/callout_target.rs +++ b/apis/src/callout_target.rs @@ -63,7 +63,7 @@ pub fn validate_http_target(filter_name: &str, raw: &str) -> Result Result bool { + let Some((_, authority_and_rest)) = raw.split_once("://") else { + return false; + }; + let authority = authority_and_rest.split(['/', '?', '#']).next().unwrap_or_default(); + authority.contains('@') +} + /// Validate a configured target, including address literals and localhost /// aliases that can be classified without DNS. /// @@ -269,7 +283,26 @@ mod tests { #[test] fn target_rejects_userinfo_for_both_policies() { - assert!(validate_http_target("test", "https://user:pass@example.com/path").is_err()); + for target in [ + "https://user:pass@example.com/path", + "http://@example.com", + "http://:@example.com", + ] { + assert!( + validate_http_target("test", target).is_err(), + "{target} should be rejected" + ); + } + } + + #[test] + fn target_allows_at_sign_outside_authority() { + for target in ["https://example.com/@path", "https://example.com/?q=@value"] { + assert!( + validate_http_target("test", target).is_ok(), + "{target} should be accepted" + ); + } } #[test] From e7e4d311f9c5998ae66afe243a37bfaa13eb8126 Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 21:40:26 +0100 Subject: [PATCH 12/13] fix(docs): allow private compact inference examples Signed-off-by: Eoin Fennessy --- apis/src/openai/responses/compact/mod.rs | 2 ++ docs/filters/openai_responses_compact.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/apis/src/openai/responses/compact/mod.rs b/apis/src/openai/responses/compact/mod.rs index 431bf81d7e..ec08164761 100644 --- a/apis/src/openai/responses/compact/mod.rs +++ b/apis/src/openai/responses/compact/mod.rs @@ -113,6 +113,7 @@ struct CompactionParams { /// filter: openai_responses_compact /// allow_pre_security_callout: true /// inference_url: "http://localhost:11434/v1/chat/completions" +/// allow_private_inference_url: true /// default_model: llama3.2:1b /// ``` /// @@ -122,6 +123,7 @@ struct CompactionParams { /// filter: openai_responses_compact /// allow_pre_security_callout: true /// inference_url: "http://localhost:11434/v1/chat/completions" +/// allow_private_inference_url: true /// default_model: gpt-4o-mini /// tiktoken_encoding: cl100k_base /// timeout_ms: 30000 diff --git a/docs/filters/openai_responses_compact.md b/docs/filters/openai_responses_compact.md index dbc0152f3b..7fe7e7987d 100644 --- a/docs/filters/openai_responses_compact.md +++ b/docs/filters/openai_responses_compact.md @@ -34,6 +34,7 @@ Praxis runs `StreamBuffer` body hooks before header-phase request filters. This filter: openai_responses_compact allow_pre_security_callout: true inference_url: "http://localhost:11434/v1/chat/completions" +allow_private_inference_url: true default_model: llama3.2:1b ``` @@ -43,6 +44,7 @@ default_model: llama3.2:1b filter: openai_responses_compact allow_pre_security_callout: true inference_url: "http://localhost:11434/v1/chat/completions" +allow_private_inference_url: true default_model: gpt-4o-mini tiktoken_encoding: cl100k_base timeout_ms: 30000 From dd015db6db1bba64212940fe4193605cf09a10fa Mon Sep 17 00:00:00 2001 From: Eoin Fennessy Date: Thu, 3 Sep 2026 21:44:30 +0100 Subject: [PATCH 13/13] test(apis): add assertion diagnostics Signed-off-by: Eoin Fennessy --- apis/src/callout_target.rs | 10 ++- apis/src/openai/api_client/mod.rs | 120 +++++++++++++++++++----------- apis/src/web_search/provider.rs | 27 +++++-- 3 files changed, 104 insertions(+), 53 deletions(-) diff --git a/apis/src/callout_target.rs b/apis/src/callout_target.rs index 0201cd64b5..f9b6db8c73 100644 --- a/apis/src/callout_target.rs +++ b/apis/src/callout_target.rs @@ -319,7 +319,10 @@ mod tests { #[test] fn mixed_dns_answer_is_rejected() { let addrs = ["8.8.8.8:443".parse().unwrap(), "169.254.169.254:443".parse().unwrap()]; - assert!(validate_resolved_addrs("test", &addrs, AddressPolicy::PublicOnly).is_err()); + assert!( + validate_resolved_addrs("test", &addrs, AddressPolicy::PublicOnly).is_err(), + "mixed public and private DNS answers should be rejected" + ); } #[test] @@ -334,6 +337,9 @@ mod tests { #[test] fn mapped_loopback_is_rejected() { let addrs = ["[::ffff:127.0.0.1]:80".parse().unwrap()]; - assert!(validate_resolved_addrs("test", &addrs, AddressPolicy::PublicOnly).is_err()); + assert!( + validate_resolved_addrs("test", &addrs, AddressPolicy::PublicOnly).is_err(), + "IPv4-mapped loopback answers should be rejected" + ); } } diff --git a/apis/src/openai/api_client/mod.rs b/apis/src/openai/api_client/mod.rs index 91a7ec97f3..503cd4372b 100644 --- a/apis/src/openai/api_client/mod.rs +++ b/apis/src/openai/api_client/mod.rs @@ -429,12 +429,15 @@ mod tests { .await .expect_err("a derived URL on another origin must be rejected before I/O"); - assert!(matches!( - error, - ApiClientError::Transport { - source: SubRequestError::InvalidRequest(detail), - } if detail.contains("configured credential origin") - )); + assert!( + matches!( + error, + ApiClientError::Transport { + source: SubRequestError::InvalidRequest(detail), + } if detail.contains("configured credential origin") + ), + "cross-origin derived URLs should be rejected as invalid requests" + ); } #[tokio::test] @@ -446,12 +449,15 @@ mod tests { .await .expect_err("a malformed callout URL must be rejected before I/O"); - assert!(matches!( - error, - ApiClientError::Transport { - source: SubRequestError::InvalidRequest(detail), - } if detail == "malformed callout URL" - )); + assert!( + matches!( + error, + ApiClientError::Transport { + source: SubRequestError::InvalidRequest(detail), + } if detail == "malformed callout URL" + ), + "malformed callout URLs should be rejected as invalid requests" + ); } #[tokio::test] @@ -786,49 +792,73 @@ mod tests { )] fn transport_errors_preserve_kind_without_rendering_source_details() { let connect = map_subrequest_error(SubRequestError::Connect("attacker-controlled".to_owned())); - assert!(matches!( - connect, - ApiClientError::Transport { - source: SubRequestError::Connect(_) - } - )); - assert!(!connect.to_string().contains("attacker-controlled")); + assert!( + matches!( + connect, + ApiClientError::Transport { + source: SubRequestError::Connect(_) + } + ), + "connect failures should preserve their typed transport variant" + ); + assert!( + !connect.to_string().contains("attacker-controlled"), + "connect failure details should not expose attacker-controlled text" + ); let io = map_subrequest_error(SubRequestError::Io("attacker-controlled".to_owned())); - assert!(matches!( - io, - ApiClientError::Transport { - source: SubRequestError::Io(_) - } - )); - assert!(!io.to_string().contains("attacker-controlled")); + assert!( + matches!( + io, + ApiClientError::Transport { + source: SubRequestError::Io(_) + } + ), + "I/O failures should preserve their typed transport variant" + ); + assert!( + !io.to_string().contains("attacker-controlled"), + "I/O failure details should not expose attacker-controlled text" + ); let admission = map_subrequest_error(SubRequestError::AdmissionTimeout { max_connections: 1 }); - assert!(matches!( - admission, - ApiClientError::Transport { - source: SubRequestError::AdmissionTimeout { .. } - } - )); + assert!( + matches!( + admission, + ApiClientError::Transport { + source: SubRequestError::AdmissionTimeout { .. } + } + ), + "admission timeouts should preserve their typed transport variant" + ); let circuit = map_subrequest_error(SubRequestError::CircuitOpen { peer: "attacker-controlled".to_owned(), }); - assert!(matches!( - circuit, - ApiClientError::Transport { - source: SubRequestError::CircuitOpen { .. } - } - )); - assert!(!circuit.to_string().contains("attacker-controlled")); + assert!( + matches!( + circuit, + ApiClientError::Transport { + source: SubRequestError::CircuitOpen { .. } + } + ), + "circuit-open errors should preserve their typed transport variant" + ); + assert!( + !circuit.to_string().contains("attacker-controlled"), + "circuit-open details should not expose attacker-controlled text" + ); let deadline = map_subrequest_error(SubRequestError::DeadlineExceeded); - assert!(matches!( - deadline, - ApiClientError::Transport { - source: SubRequestError::DeadlineExceeded - } - )); + assert!( + matches!( + deadline, + ApiClientError::Transport { + source: SubRequestError::DeadlineExceeded + } + ), + "deadline errors should preserve their typed transport variant" + ); } #[test] diff --git a/apis/src/web_search/provider.rs b/apis/src/web_search/provider.rs index 54abef8c35..183fbd32bc 100644 --- a/apis/src/web_search/provider.rs +++ b/apis/src/web_search/provider.rs @@ -419,13 +419,19 @@ mod tests { #[test] fn parse_brave_results_empty() { let json = json!({"web": {"results": []}}); - assert!(parse_brave_results(&json).is_empty()); + assert!( + parse_brave_results(&json).is_empty(), + "empty Brave results should parse as empty" + ); } #[test] fn parse_brave_results_missing_web() { let json = json!({"query": "test"}); - assert!(parse_brave_results(&json).is_empty()); + assert!( + parse_brave_results(&json).is_empty(), + "missing Brave web results should parse as empty" + ); } #[test] @@ -460,13 +466,19 @@ mod tests { #[test] fn parse_tavily_results_empty() { let json = json!({"results": []}); - assert!(parse_tavily_results(&json).is_empty()); + assert!( + parse_tavily_results(&json).is_empty(), + "empty Tavily results should parse as empty" + ); } #[test] fn parse_tavily_results_missing_results() { let json = json!({"answer": "some answer"}); - assert!(parse_tavily_results(&json).is_empty()); + assert!( + parse_tavily_results(&json).is_empty(), + "missing Tavily results should parse as empty" + ); } #[test] @@ -532,7 +544,10 @@ mod tests { #[test] fn parse_you_results_handles_missing_sections() { - assert!(parse_you_results(&json!({"results": {}})).is_empty()); + assert!( + parse_you_results(&json!({"results": {}})).is_empty(), + "missing You.com result sections should parse as empty" + ); } #[test] @@ -547,7 +562,7 @@ mod tests { allow_private_base_url: false, }; let client = SearchClient::from_config("test", &config, test_subrequest_client()); - assert!(client.is_ok()); + assert!(client.is_ok(), "a valid search configuration should build a client"); } #[test]