From 1078b6a80160ff3541a15474589584cc2d4e4e15 Mon Sep 17 00:00:00 2001 From: mkoushni Date: Wed, 2 Sep 2026 15:00:23 +0300 Subject: [PATCH 1/4] fix(mcp_client): strip Keep-Alive and Proxy-Connection from MCP headers (#669) is_blocked_mcp_header already dropped Connection but left the Keep-Alive field and de-facto Proxy-Connection hop-by-hop header, so client-supplied MCP tool headers could reach the outbound HTTP/1 transport. Signed-off-by: mkoushni --- apis/src/mcp_client/mod.rs | 7 ++++++- apis/src/mcp_client/tests.rs | 37 ++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/apis/src/mcp_client/mod.rs b/apis/src/mcp_client/mod.rs index 2d2948f773..10c9c3f8ea 100644 --- a/apis/src/mcp_client/mod.rs +++ b/apis/src/mcp_client/mod.rs @@ -569,7 +569,12 @@ fn is_blocked_mcp_header(name: &http::HeaderName) -> bool { return true; } let s = name.as_str(); - s.starts_with("x-forwarded-") || s.starts_with("x-praxis-") || s.starts_with("x-mcp-") || s.starts_with("x-a2a-") + s == "keep-alive" + || s == "proxy-connection" + || s.starts_with("x-forwarded-") + || s.starts_with("x-praxis-") + || s.starts_with("x-mcp-") + || s.starts_with("x-a2a-") } /// Hostnames that resolve to loopback. diff --git a/apis/src/mcp_client/tests.rs b/apis/src/mcp_client/tests.rs index e0ee389480..179e50c3f5 100644 --- a/apis/src/mcp_client/tests.rs +++ b/apis/src/mcp_client/tests.rs @@ -81,6 +81,8 @@ fn hop_by_hop_headers_stripped_from_mcp_headers() { "content-length": "999", "transfer-encoding": "chunked", "connection": "keep-alive", + "keep-alive": "timeout=5", + "proxy-connection": "keep-alive", "te": "trailers", "trailer": "Foo", "upgrade": "websocket", @@ -98,6 +100,41 @@ fn hop_by_hop_headers_stripped_from_mcp_headers() { ); } +#[test] +fn keep_alive_and_proxy_connection_headers_stripped_from_mcp_headers() { + let headers = serde_json::json!({ + "keep-alive": "timeout=5", + "proxy-connection": "keep-alive", + "connection": "keep-alive", + "x-custom": "safe" + }); + let config = build_transport_config("http://api.example.com/mcp", Some(&headers), None).unwrap(); + + assert_eq!(config.custom_headers.len(), 1, "only safe header should remain"); + assert!( + config + .custom_headers + .contains_key(&http::HeaderName::from_static("x-custom")), + "x-custom should pass through" + ); + assert!( + !config + .custom_headers + .contains_key(&http::HeaderName::from_static("keep-alive")), + "keep-alive must not reach outbound MCP transport" + ); + assert!( + !config + .custom_headers + .contains_key(&http::HeaderName::from_static("proxy-connection")), + "proxy-connection must not reach outbound MCP transport" + ); + assert!( + !config.custom_headers.contains_key(&http::header::CONNECTION), + "connection must stay blocked" + ); +} + #[test] fn reserved_internal_headers_stripped_from_mcp_headers() { let headers = serde_json::json!({ From 5e1595e54a369e01ae6ebfb7f4e2d0b91cbe5b67 Mon Sep 17 00:00:00 2001 From: mkoushni Date: Wed, 2 Sep 2026 17:19:08 +0300 Subject: [PATCH 2/4] fix(http): centralize outbound hop-by-hop sanitization (#669) Share one hop-by-hop predicate across MCP, API callouts, http_callout, and fixture replay, and strip every field named by Connection before dropping Connection itself so header lists cannot drift. Signed-off-by: mkoushni --- apis/src/http_hop.rs | 148 ++++++++++++++++++ apis/src/lib.rs | 5 +- apis/src/mcp_client/mod.rs | 44 ++++-- apis/src/mcp_client/tests.rs | 52 ++++++ apis/src/openai/api_client/mod.rs | 47 +++++- apis/src/openai/api_client/url.rs | 21 +-- .../responses/file_search_callout/mod.rs | 21 +-- .../responses/file_search_callout/tests.rs | 1 + filters/src/callout/mod.rs | 31 ++-- filters/src/callout/tests.rs | 119 ++++++++++++++ .../src/inference_fixture/header_policy.rs | 27 +--- 11 files changed, 426 insertions(+), 90 deletions(-) create mode 100644 apis/src/http_hop.rs diff --git a/apis/src/http_hop.rs b/apis/src/http_hop.rs new file mode 100644 index 0000000000..8c47a449ab --- /dev/null +++ b/apis/src/http_hop.rs @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (c) 2026 Praxis Contributors + +//! Shared outbound hop-by-hop HTTP header policy. +//! +//! RFC 9110 hop-by-hop field names plus the obsolete `Proxy-Connection` +//! name. Outbound paths that copy headers onto a newly constructed request +//! must consult [`is_hop_by_hop`] and strip every field named by a +//! [`Connection`](http::header::CONNECTION) token list before dropping +//! `Connection` itself. +//! +//! Callers keep path-specific denylists (`Host`, `Content-Length`, cookies, +//! internal `x-praxis-*` prefixes) on top of this predicate. Do not copy +//! those extras into this module: they are not hop-by-hop. + +use http::{HeaderMap, HeaderName}; + +/// Whether `name` is a hop-by-hop field that must not be copied onto a +/// newly constructed outbound request. +/// +/// `name` must be a lowercase HTTP field name, as produced by +/// [`HeaderName::as_str`](http::HeaderName::as_str). +/// +/// ``` +/// assert!(praxis_ai_apis::http_hop::is_hop_by_hop("keep-alive")); +/// assert!(praxis_ai_apis::http_hop::is_hop_by_hop("proxy-connection")); +/// assert!(!praxis_ai_apis::http_hop::is_hop_by_hop("authorization")); +/// assert!(!praxis_ai_apis::http_hop::is_hop_by_hop("host")); +/// ``` +#[must_use] +pub fn is_hop_by_hop(name: &str) -> bool { + matches!( + name, + "connection" + | "keep-alive" + | "proxy-authenticate" + | "proxy-authorization" + | "proxy-connection" + | "te" + | "trailer" + | "transfer-encoding" + | "upgrade" + ) +} + +/// Iterate hop-by-hop field names listed in one `Connection` header value. +/// +/// Splits on commas, trims ASCII whitespace, and drops empty tokens. +/// Token comparison is the caller's responsibility; see +/// [`connection_nominates`]. +pub fn connection_tokens(value: &str) -> impl Iterator { + value.split(',').map(str::trim).filter(|token| !token.is_empty()) +} + +/// Whether one `Connection` header value nominates `name` as hop-by-hop. +#[must_use] +pub fn connection_nominates(value: &str, name: &str) -> bool { + connection_tokens(value).any(|token| token.eq_ignore_ascii_case(name)) +} + +/// Whether any `Connection` value on `headers` nominates `name`. +/// +/// Invalid (non-text) `Connection` values are ignored rather than failing +/// open: a header that cannot be parsed as tokens cannot nominate fields. +#[must_use] +pub fn connection_nominates_header(headers: &HeaderMap, name: &HeaderName) -> bool { + headers + .get_all(http::header::CONNECTION) + .iter() + .filter_map(|value| value.to_str().ok()) + .any(|value| connection_nominates(value, name.as_str())) +} + +#[cfg(test)] +#[expect(clippy::allow_attributes, reason = "blanket test suppressions")] +#[allow(clippy::unwrap_used, reason = "tests")] +mod tests { + use http::{HeaderMap, HeaderName, HeaderValue}; + + use super::{connection_nominates, connection_nominates_header, connection_tokens, is_hop_by_hop}; + + #[test] + fn hop_by_hop_covers_rfc_names_and_proxy_connection() { + for name in [ + "connection", + "keep-alive", + "proxy-authenticate", + "proxy-authorization", + "proxy-connection", + "te", + "trailer", + "transfer-encoding", + "upgrade", + ] { + assert!(is_hop_by_hop(name), "{name} is hop-by-hop"); + } + for name in ["authorization", "cookie", "content-length", "host", "x-custom"] { + assert!(!is_hop_by_hop(name), "{name} is not hop-by-hop"); + } + } + + #[test] + fn connection_tokens_split_trim_and_drop_empty() { + let tokens: Vec<&str> = connection_tokens(" x-smuggle , , Keep-Alive ").collect(); + assert_eq!( + tokens, + ["x-smuggle", "Keep-Alive"], + "Connection tokens should be comma-split and trimmed" + ); + } + + #[test] + fn connection_nominates_is_case_insensitive() { + assert!( + connection_nominates("X-Smuggle, close", "x-smuggle"), + "Connection tokens are case-insensitive" + ); + assert!( + !connection_nominates("close", "x-custom"), + "unrelated names must not be treated as nominated" + ); + } + + #[test] + fn connection_nominates_header_reads_every_connection_value() { + let mut headers = HeaderMap::new(); + headers.append(http::header::CONNECTION, HeaderValue::from_static("X-Hop-One")); + headers.append( + http::header::CONNECTION, + HeaderValue::from_static("x-hop-two, keep-alive"), + ); + let hop_one = HeaderName::from_static("x-hop-one"); + let hop_two = HeaderName::from_static("x-hop-two"); + let custom = HeaderName::from_static("x-custom"); + assert!( + connection_nominates_header(&headers, &hop_one), + "first Connection value should nominate x-hop-one" + ); + assert!( + connection_nominates_header(&headers, &hop_two), + "second Connection value should nominate x-hop-two" + ); + assert!( + !connection_nominates_header(&headers, &custom), + "x-custom is not listed in Connection" + ); + } +} diff --git a/apis/src/lib.rs b/apis/src/lib.rs index 64c638b8d4..5963b89333 100644 --- a/apis/src/lib.rs +++ b/apis/src/lib.rs @@ -6,11 +6,12 @@ //! AI provider API types and persistence for Praxis. //! //! Contains provider-specific protocol types (OpenAI, Anthropic), -//! request classification, shared JSON body-mutation helpers, and -//! response storage backends. +//! request classification, shared hop-by-hop header sanitization, +//! JSON body-mutation helpers, and response storage backends. pub mod anthropic; pub mod classifier; +pub mod http_hop; pub mod json_body; pub(crate) mod mcp_client; pub mod openai; diff --git a/apis/src/mcp_client/mod.rs b/apis/src/mcp_client/mod.rs index 10c9c3f8ea..9c5c53782e 100644 --- a/apis/src/mcp_client/mod.rs +++ b/apis/src/mcp_client/mod.rs @@ -22,7 +22,7 @@ mod tests; use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, fmt, net::{IpAddr, Ipv4Addr, SocketAddr}, time::Duration, @@ -368,10 +368,12 @@ fn build_transport_config( let mut header_map = HashMap::new(); if let Some(headers_obj) = headers.and_then(serde_json::Value::as_object) { + let nominated = connection_nominated_from_json(headers_obj); for (key, value) in headers_obj { if let Some(value_str) = value.as_str() && let Ok(name) = key.parse::() && !is_blocked_mcp_header(&name) + && !nominated.contains(&name) && let Ok(val) = http::HeaderValue::from_str(value_str) { header_map.insert(name, val); @@ -548,33 +550,49 @@ fn build_pinned_client(resolved: &ResolvedMcpUrl) -> Result, +) -> HashSet { + let mut nominated = HashSet::new(); + for (key, value) in headers_obj { + let Ok(name) = key.parse::() else { + continue; + }; + if name != http::header::CONNECTION { + continue; + } + let Some(value) = value.as_str() else { + continue; + }; + for token in crate::http_hop::connection_tokens(value) { + if let Ok(nominated_name) = token.parse::() { + nominated.insert(nominated_name); + } + } + } + nominated +} + /// Headers that must not pass through from client-supplied MCP /// tool config into the proxy's outbound MCP transport. fn is_blocked_mcp_header(name: &http::HeaderName) -> bool { + if crate::http_hop::is_hop_by_hop(name.as_str()) { + return true; + } if matches!( *name, http::header::AUTHORIZATION - | http::header::CONNECTION | http::header::CONTENT_LENGTH | http::header::COOKIE | http::header::FORWARDED | http::header::HOST - | http::header::PROXY_AUTHORIZATION | http::header::SET_COOKIE - | http::header::TE - | http::header::TRAILER - | http::header::TRANSFER_ENCODING - | http::header::UPGRADE ) { return true; } let s = name.as_str(); - s == "keep-alive" - || s == "proxy-connection" - || s.starts_with("x-forwarded-") - || s.starts_with("x-praxis-") - || s.starts_with("x-mcp-") - || s.starts_with("x-a2a-") + s.starts_with("x-forwarded-") || s.starts_with("x-praxis-") || s.starts_with("x-mcp-") || s.starts_with("x-a2a-") } /// Hostnames that resolve to loopback. diff --git a/apis/src/mcp_client/tests.rs b/apis/src/mcp_client/tests.rs index 179e50c3f5..c971a9c1af 100644 --- a/apis/src/mcp_client/tests.rs +++ b/apis/src/mcp_client/tests.rs @@ -87,6 +87,7 @@ fn hop_by_hop_headers_stripped_from_mcp_headers() { "trailer": "Foo", "upgrade": "websocket", "proxy-authorization": "Basic creds", + "proxy-authenticate": "Basic realm=\"mcp\"", "x-custom": "safe" }); let config = build_transport_config("http://api.example.com/mcp", Some(&headers), None).unwrap(); @@ -135,6 +136,57 @@ fn keep_alive_and_proxy_connection_headers_stripped_from_mcp_headers() { ); } +#[test] +fn connection_nominated_headers_stripped_from_mcp_headers() { + let headers = serde_json::json!({ + "connection": "x-smuggle, Keep-Alive", + "x-smuggle": "secret", + "x-custom": "safe" + }); + let config = build_transport_config("http://api.example.com/mcp", Some(&headers), None).unwrap(); + + assert_eq!(config.custom_headers.len(), 1, "only safe header should remain"); + assert!( + config + .custom_headers + .contains_key(&http::HeaderName::from_static("x-custom")), + "x-custom is not listed in Connection and should pass through" + ); + assert!( + !config + .custom_headers + .contains_key(&http::HeaderName::from_static("x-smuggle")), + "fields named by Connection must not reach outbound MCP transport" + ); + assert!( + !config.custom_headers.contains_key(&http::header::CONNECTION), + "connection itself must stay blocked" + ); +} + +#[test] +fn proxy_authenticate_stripped_from_mcp_headers() { + let headers = serde_json::json!({ + "proxy-authenticate": "Basic realm=\"mcp\"", + "x-custom": "safe" + }); + let config = build_transport_config("http://api.example.com/mcp", Some(&headers), None).unwrap(); + + assert_eq!(config.custom_headers.len(), 1, "only safe header should remain"); + assert!( + config + .custom_headers + .contains_key(&http::HeaderName::from_static("x-custom")), + "x-custom should pass through" + ); + assert!( + !config + .custom_headers + .contains_key(&http::HeaderName::from_static("proxy-authenticate")), + "proxy-authenticate is hop-by-hop and must not reach outbound MCP transport" + ); +} + #[test] fn reserved_internal_headers_stripped_from_mcp_headers() { let headers = serde_json::json!({ diff --git a/apis/src/openai/api_client/mod.rs b/apis/src/openai/api_client/mod.rs index 48f520be26..70a84b2118 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::{ + http_hop::{connection_nominates_header, is_hop_by_hop}, + subrequest::{self, SubRequest, SubRequestClient, SubRequestError, SubResponse}, +}; /// Configuration for constructing an [`ApiClient`]. /// @@ -77,6 +80,11 @@ fn map_subrequest_error(err: SubRequestError) -> ApiClientError { } } +/// Whether a configured forward header is safe to copy from the inbound request. +fn should_copy_forward_header(name: &http::HeaderName, request_headers: &HeaderMap) -> bool { + !is_hop_by_hop(name.as_str()) && !connection_nominates_header(request_headers, name) +} + impl ApiClient { /// Build a new client from validated configuration. /// @@ -191,6 +199,9 @@ impl ApiClient { pub(crate) fn forward_headers(&self, request_headers: &HeaderMap) -> Vec<(http::HeaderName, http::HeaderValue)> { let mut headers = Vec::new(); for name in &self.forward_header_names { + if !should_copy_forward_header(name, request_headers) { + continue; + } if let Some(value) = request_headers.get(name) { headers.push((name.clone(), value.clone())); } @@ -202,6 +213,9 @@ impl ApiClient { fn build_header_map(&self, request_headers: &HeaderMap) -> HeaderMap { let mut map = HeaderMap::new(); for name in &self.forward_header_names { + if !should_copy_forward_header(name, request_headers) { + continue; + } if let Some(value) = request_headers.get(name) { map.insert(name.clone(), value.clone()); } @@ -373,6 +387,37 @@ mod tests { ); } + #[test] + fn forward_headers_skips_connection_nominated_fields() { + let client = ApiClient::new(ApiClientConfig { + api_base_url: "http://ogx:8321".to_owned(), + client: SubRequestClient::new(SubRequestConnector::new(4, None)), + timeout: Duration::from_millis(1_000), + max_response_bytes: 1_048_576, + forward_header_names: vec![ + http::HeaderName::from_static("x-smuggle"), + http::HeaderName::from_static("x-tenant-id"), + ], + }); + + let mut request_headers = HeaderMap::new(); + request_headers.insert(http::header::CONNECTION, "x-smuggle".parse().unwrap()); + request_headers.insert("x-smuggle", "secret".parse().unwrap()); + request_headers.insert("x-tenant-id", "tenant-1".parse().unwrap()); + + let forwarded = client.forward_headers(&request_headers); + + assert_eq!(forwarded.len(), 1, "only the un-nominated header should be forwarded"); + assert!( + forwarded.iter().any(|(n, v)| n == "x-tenant-id" && v == "tenant-1"), + "x-tenant-id is not listed in Connection and should still be forwarded" + ); + assert!( + forwarded.iter().all(|(n, _)| n != "x-smuggle"), + "fields named by Connection must not be copied onto the outbound request" + ); + } + #[test] fn resource_url_delegates_to_url_module() { let client = test_client("http://ogx:8321"); diff --git a/apis/src/openai/api_client/url.rs b/apis/src/openai/api_client/url.rs index feb413369a..23d702a065 100644 --- a/apis/src/openai/api_client/url.rs +++ b/apis/src/openai/api_client/url.rs @@ -268,20 +268,9 @@ fn is_blocked_forward_header(name: &str) -> bool { || name.starts_with("x-ext-agent-") || name.starts_with("x-mcp-") || name.starts_with("x-a2a-") - || matches!( - name, - "connection" - | "content-length" - | "host" - | "keep-alive" - | "proxy-authenticate" - | "proxy-authorization" - | "proxy-connection" - | "te" - | "trailer" - | "transfer-encoding" - | "upgrade" - ) + || name == "content-length" + || name == "host" + || crate::http_hop::is_hop_by_hop(name) } // ----------------------------------------------------------------------------- @@ -541,7 +530,11 @@ mod tests { "host", "content-length", "transfer-encoding", + "keep-alive", + "proxy-authenticate", "proxy-authorization", + "proxy-connection", + "te", "x-praxis-route", ] { let mut headers = vec![name.to_owned()]; diff --git a/apis/src/openai/responses/file_search_callout/mod.rs b/apis/src/openai/responses/file_search_callout/mod.rs index 22ee48baf4..ab6acada0d 100644 --- a/apis/src/openai/responses/file_search_callout/mod.rs +++ b/apis/src/openai/responses/file_search_callout/mod.rs @@ -41,6 +41,7 @@ use self::{ model_context::{FormatLimits, FormatTemplates, MODEL_CONTEXT_TEMPLATES, format_search_results}, }; use crate::{ + http_hop::{connection_nominates_header, is_hop_by_hop}, openai::responses::{ bounded_json_size, config_validation::FailureMode, @@ -542,24 +543,13 @@ fn preserve_original_request_headers(ctx: &mut HttpFilterContext<'_>) { ctx.request_headers_to_set.extend(headers); } -/// Whether `Connection` marks a request header as specific to one hop. -fn connection_nominates_header(headers: &HeaderMap, name: &http::header::HeaderName) -> bool { - headers - .get_all(http::header::CONNECTION) - .iter() - .filter_map(|value| value.to_str().ok()) - .flat_map(|value| value.split(',')) - .map(str::trim) - .any(|token| token.eq_ignore_ascii_case(name.as_str())) -} - /// Whether a header remains valid after the continuation body is rewritten. fn should_replay_original_header(name: &http::header::HeaderName) -> bool { !praxis_core::reserved_headers::is_reserved(name.as_str()) + && !is_hop_by_hop(name.as_str()) && !matches!( name.as_str(), "accept-encoding" - | "connection" | "content-encoding" | "content-length" | "content-md5" @@ -567,15 +557,8 @@ fn should_replay_original_header(name: &http::header::HeaderName) -> bool { | "expect" | "host" | "idempotency-key" - | "keep-alive" - | "proxy-authenticate" - | "proxy-authorization" | "signature" | "signature-input" - | "te" - | "trailer" - | "transfer-encoding" - | "upgrade" ) } diff --git a/apis/src/openai/responses/file_search_callout/tests.rs b/apis/src/openai/responses/file_search_callout/tests.rs index f53c9cf4ad..6a9f12feb8 100644 --- a/apis/src/openai/responses/file_search_callout/tests.rs +++ b/apis/src/openai/responses/file_search_callout/tests.rs @@ -458,6 +458,7 @@ fn continuation_header_replay_excludes_stale_request_metadata() { http::header::CONTENT_ENCODING, http::header::ACCEPT_ENCODING, http::header::HeaderName::from_static("idempotency-key"), + http::header::HeaderName::from_static("proxy-connection"), http::header::HeaderName::from_static("x-praxis-internal-test"), ] { assert!(!should_replay_original_header(&name), "{name} should not be replayed"); diff --git a/filters/src/callout/mod.rs b/filters/src/callout/mod.rs index 34e83c5f01..f319ad322f 100644 --- a/filters/src/callout/mod.rs +++ b/filters/src/callout/mod.rs @@ -29,6 +29,7 @@ use config::{FailureModeConfig, HttpCalloutConfig, Phase, expand_env_vars, valid use extract::{BodyShaper, CompiledExtraction}; use http::HeaderMap; use pingora_core::upstreams::peer::HttpPeer; +use praxis_ai_apis::http_hop::{connection_nominates_header, is_hop_by_hop}; use praxis_core::{ circuit::CircuitBreakerConfig as CoreCircuitBreakerConfig, connectivity::is_private_ip, @@ -52,18 +53,6 @@ const FILTER_NAME: &str = "http_callout"; /// Maximum allowed value for `max_body_bytes` (100 MiB). const MAX_BODY_BYTES: usize = 104_857_600; // 100 MiB -/// Hop-by-hop and sensitive headers that must never be blindly -/// forwarded from the client onto the callout request. -const DISALLOWED_FORWARD_HEADERS: &[http::HeaderName] = &[ - http::header::HOST, - http::header::CONTENT_LENGTH, - http::header::TRANSFER_ENCODING, - http::header::CONNECTION, - http::header::UPGRADE, - http::header::PROXY_AUTHORIZATION, - http::header::TRAILER, -]; - // ----------------------------------------------------------------------------- // HttpCalloutFilter // ----------------------------------------------------------------------------- @@ -212,7 +201,7 @@ impl HttpCalloutFilter { // Forward allowed client headers, skipping hop-by-hop/sensitive ones. for name in &self.forward_headers { - if DISALLOWED_FORWARD_HEADERS.contains(name) { + if is_disallowed_forward_header(name) || connection_nominates_header(&ctx.request.headers, name) { continue; } if let Some(value) = ctx.request.headers.get(name) { @@ -469,13 +458,13 @@ fn parse_header_names(names: &[String], context: &str) -> Result bool { + *name == http::header::HOST || *name == http::header::CONTENT_LENGTH || is_hop_by_hop(name.as_str()) +} + /// Compile `JSONPath` extraction rules from config. fn compile_extractions(cfg: &HttpCalloutConfig) -> Result, FilterError> { cfg.response diff --git a/filters/src/callout/tests.rs b/filters/src/callout/tests.rs index 63e86821fd..1bbe5acd42 100644 --- a/filters/src/callout/tests.rs +++ b/filters/src/callout/tests.rs @@ -1432,6 +1432,125 @@ mod filter_tests { ); } + #[tokio::test] + async fn hop_by_hop_forward_headers_not_sent() { + let mock_server = MockServer::start().await; + + Mock::given(method("POST")) + .and(path("/guard")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({"ok": true}))) + .mount(&mock_server) + .await; + + let yaml = serde_yaml::from_str::(&format!( + r#" + target: + url: "{}/guard" + forward_headers: + - "keep-alive" + - "proxy-connection" + - "te" + - "x-ok" + request: + phase: request_headers + "#, + mock_server.uri() + )) + .unwrap(); + + let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + + let mut headers = http::HeaderMap::new(); + headers.insert("keep-alive", "timeout=5".parse().unwrap()); + headers.insert("proxy-connection", "keep-alive".parse().unwrap()); + headers.insert("te", "trailers".parse().unwrap()); + headers.insert("x-ok", "fine".parse().unwrap()); + + let req = praxis_filter::Request { + method: http::Method::POST, + uri: "/test".parse().unwrap(), + headers, + }; + let mut ctx = make_filter_context(&req); + + let action = filter.on_request(&mut ctx).await.unwrap(); + assert!(matches!(action, FilterAction::Continue)); + + let requests = mock_server.received_requests().await.expect("recorded requests"); + let callout = requests.first().expect("callout should have fired"); + assert!( + callout.headers.get("keep-alive").is_none(), + "keep-alive must not reach the callout even when configured as forward_header" + ); + assert!( + callout.headers.get("proxy-connection").is_none(), + "proxy-connection must not reach the callout even when configured as forward_header" + ); + assert!( + callout.headers.get("te").is_none(), + "te must not reach the callout even when configured as forward_header" + ); + assert_eq!( + callout.headers.get("x-ok").map(http::HeaderValue::as_bytes), + Some(&b"fine"[..]), + "an allowed forward header should still be forwarded" + ); + } + + #[tokio::test] + async fn connection_nominated_forward_header_not_sent() { + let mock_server = MockServer::start().await; + + Mock::given(method("POST")) + .and(path("/guard")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({"ok": true}))) + .mount(&mock_server) + .await; + + let yaml = serde_yaml::from_str::(&format!( + r#" + target: + url: "{}/guard" + forward_headers: + - "x-smuggle" + - "x-ok" + request: + phase: request_headers + "#, + mock_server.uri() + )) + .unwrap(); + + let filter = HttpCalloutFilter::from_config(&yaml).unwrap(); + + let mut headers = http::HeaderMap::new(); + headers.insert("connection", "x-smuggle".parse().unwrap()); + headers.insert("x-smuggle", "secret".parse().unwrap()); + headers.insert("x-ok", "fine".parse().unwrap()); + + let req = praxis_filter::Request { + method: http::Method::POST, + uri: "/test".parse().unwrap(), + headers, + }; + let mut ctx = make_filter_context(&req); + + let action = filter.on_request(&mut ctx).await.unwrap(); + assert!(matches!(action, FilterAction::Continue)); + + let requests = mock_server.received_requests().await.expect("recorded requests"); + let callout = requests.first().expect("callout should have fired"); + assert!( + callout.headers.get("x-smuggle").is_none(), + "a header named by Connection must not reach the callout" + ); + assert_eq!( + callout.headers.get("x-ok").map(http::HeaderValue::as_bytes), + Some(&b"fine"[..]), + "an allowed forward header should still be forwarded" + ); + } + // ------------------------------------------------------------------------- // Body Shaping — non-JSON fallback // ------------------------------------------------------------------------- diff --git a/tests/utils/src/inference_fixture/header_policy.rs b/tests/utils/src/inference_fixture/header_policy.rs index 45b3cc47f2..fc7277e9de 100644 --- a/tests/utils/src/inference_fixture/header_policy.rs +++ b/tests/utils/src/inference_fixture/header_policy.rs @@ -6,6 +6,7 @@ use std::collections::{BTreeMap, BTreeSet}; use http::{HeaderMap, HeaderName, HeaderValue, header}; +use praxis_ai_apis::http_hop::{connection_tokens, is_hop_by_hop}; use super::FixtureError; @@ -202,9 +203,7 @@ fn recorded_connection_nominations(headers: &BTreeMap>) -> B .iter() .filter(|(name, _)| name.eq_ignore_ascii_case("connection")) .flat_map(|(_, values)| values) - .flat_map(|value| value.split(',')) - .map(str::trim) - .filter(|name| !name.is_empty()) + .flat_map(|value| connection_tokens(value)) .map(str::to_ascii_lowercase) .collect() } @@ -216,13 +215,7 @@ fn http_connection_nominations(headers: &HeaderMap) -> Result, let value = value .to_str() .map_err(|_source| runtime_error("replay Connection header is not text"))?; - nominated.extend( - value - .split(',') - .map(str::trim) - .filter(|name| !name.is_empty()) - .map(str::to_ascii_lowercase), - ); + nominated.extend(connection_tokens(value).map(str::to_ascii_lowercase)); } Ok(nominated) } @@ -250,19 +243,7 @@ fn is_transport_unsafe(name: &str, nominated: &BTreeSet) -> bool { /// Returns whether a name controls message framing or one HTTP hop. fn is_framing_or_hop_header(name: &str) -> bool { - matches!( - name, - "connection" - | "content-length" - | "keep-alive" - | "proxy-authenticate" - | "proxy-authorization" - | "proxy-connection" - | "te" - | "trailer" - | "transfer-encoding" - | "upgrade" - ) + name == "content-length" || is_hop_by_hop(name) } /// Returns whether a normalized name belongs in a fixture. From 4736c0ca8ff75ba075fc89735fab7177138876d7 Mon Sep 17 00:00:00 2001 From: mkoushni Date: Thu, 3 Sep 2026 12:05:22 +0300 Subject: [PATCH 3/4] fix(server): enable admin-api when compiling against praxis main Praxis core main gated admin endpoints behind praxis-protocol's admin-api feature. test-praxis-main patches that tree in, so the praxis-main feature must turn the admin API on or the server crate does not compile. Signed-off-by: mkoushni --- server/Cargo.toml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/server/Cargo.toml b/server/Cargo.toml index d07a312c31..1d4f66d9ca 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -35,7 +35,14 @@ token-rate-limit-filter = ["praxis-ai-filters/token-rate-limit-filter", "experim experimental = ["praxis-ai-filters/experimental"] opentelemetry = ["praxis-ai-filters/opentelemetry"] llmd-ext-proc = ["dep:praxis-ai-llmd-ext-proc"] -praxis-main = ["praxis-ai-filters/praxis-main", "praxis-ai-apis/praxis-main"] +# Praxis core main moved admin endpoints behind `admin-api` (off by +# default). crates.io 0.5.3 does not have that feature; enable it only +# after `patch-praxis` so `test-praxis-main` can compile. +praxis-main = [ + "praxis-ai-filters/praxis-main", + "praxis-ai-apis/praxis-main", + "praxis-protocol/admin-api", +] [lints] workspace = true From 50704cab9413e1af0ee5cae8f07423ea535268e4 Mon Sep 17 00:00:00 2001 From: mkoushni Date: Thu, 3 Sep 2026 12:12:14 +0300 Subject: [PATCH 4/4] revert(server): drop admin-api from praxis-main on 0.5.3 Cargo rejects praxis-protocol/admin-api while the lockfile is still 0.5.3, so lint and unit tests fail before patch-praxis runs. test-praxis-main stays a known core-compat break until 0.5.4. Signed-off-by: mkoushni --- server/Cargo.toml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/server/Cargo.toml b/server/Cargo.toml index 1d4f66d9ca..d07a312c31 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -35,14 +35,7 @@ token-rate-limit-filter = ["praxis-ai-filters/token-rate-limit-filter", "experim experimental = ["praxis-ai-filters/experimental"] opentelemetry = ["praxis-ai-filters/opentelemetry"] llmd-ext-proc = ["dep:praxis-ai-llmd-ext-proc"] -# Praxis core main moved admin endpoints behind `admin-api` (off by -# default). crates.io 0.5.3 does not have that feature; enable it only -# after `patch-praxis` so `test-praxis-main` can compile. -praxis-main = [ - "praxis-ai-filters/praxis-main", - "praxis-ai-apis/praxis-main", - "praxis-protocol/admin-api", -] +praxis-main = ["praxis-ai-filters/praxis-main", "praxis-ai-apis/praxis-main"] [lints] workspace = true