From 15f18b54fca2a5fa48a9ba32fb35b450294f2019 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Wed, 2 Sep 2026 14:33:47 +0000 Subject: [PATCH 1/3] fix(headers): complete the exact-name rule, and carry the forward on /v1/realtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four follow-ups to the one auth-independent client-header forward. The exact-name rule for credential slots was implemented against a fixed list of names. That list is complete on `/v1/*` and MCP, where the caller's gateway credential always arrives in `Authorization` or `x-api-key`. It is not complete on a passthrough route, where the operator names the slot: under `auth_mode: header_key` the credential arrives in the route's `auth_header_name`, and route validation guarantees that name is never one of the fixed ones. `forward_client_headers: ["x-*"]` beside `auth_header_name: x-gw-key` therefore relayed every caller's gateway key to the upstream, where it can be replayed. `identity_header` had the same shape, against a field whose description promises it is stripped before forwarding. A route's own `auth_header_name` and `identity_header` now join the exact-name set for that route. A glob no longer reaches either; naming one in full still forwards it, unchanged. No existing configuration changes meaning except one that was relaying a credential it never named. `/v1/realtime` never carried the setting at all: it builds its upstream WebSocket handshake by hand rather than through the shared pipeline, so an operator who declared that an upstream reads the caller's own credential got it on every other `/v1/*` endpoint and not there. It now applies the same forward with the same precedence, refusing the handshake slots this surface owns — the caller's `sec-websocket-key` / `-version` / `-extensions` describe the connection the caller opened, and the browser flow puts the caller's own gateway key in `sec-websocket-protocol`. `request.default_headers` remains unapplied on this face, as before. The `ensemble` and `semantic` sub-dispatch paths were audited and already correct: a panel member, a judge (buffered and streamed) and a semantic route target each carry the caller's forwardable headers, resolved against the ProviderKey of the upstream actually being called rather than the entry's — the ensemble entry has no ProviderKey of its own. The semantic router's embedding lookup deliberately forwards nothing: it is a gateway classification call the caller never addressed. Covered by e2e so a future sub-dispatch cannot drop it silently. Finally, the `forward_client_headers` description said an AWS Bedrock upstream refuses four names; the code refuses six. `x-amz-target` and `x-amzn-bedrock-accept` were missing from the prose, and the reserved list is now pinned by a test so the two cannot drift again. Behavior changes since 0.13.0, both from the preceding change and stated here because release notes are built from commits: - `default_headers` filtering widened. 0.13.0 filtered the block only against credential names, so gateway-namespace headers passed through; the filter now also excludes the `x-aisix-` prefix. An existing `default_headers: {"x-aisix-foo": "..."}` stops being delivered, and because the control plane re-validates the merged block, that provider key is rejected on edit until the header is removed. The error is explicit and the configuration is dead on the new data plane either way, so the guard stays. - The reserved-upstream-header list is gone as a permission gate. Credential and cookie names are forwardable when a pattern names them exactly, and settable as `default_headers`; only protocol-breaking names and the gateway's own namespace remain blocked outright. --- crates/aisix-core/src/forwarded_headers.rs | 76 +++++- crates/aisix-core/src/lib.rs | 8 +- .../src/models/passthrough_route.rs | 8 + crates/aisix-core/src/models/provider_key.rs | 16 +- crates/aisix-gateway/src/bridge.rs | 3 + crates/aisix-gateway/src/upstream_headers.rs | 14 +- crates/aisix-provider-bedrock/src/wire.rs | 23 ++ crates/aisix-proxy/src/passthrough_route.rs | 199 +++++++++++++- crates/aisix-proxy/src/realtime.rs | 244 ++++++++++++++++-- .../resources/passthrough_route.schema.json | 2 +- schemas/resources/provider_key.schema.json | 2 +- .../cases/forward-client-headers-e2e.test.ts | 165 +++++++++++- 12 files changed, 723 insertions(+), 37 deletions(-) diff --git a/crates/aisix-core/src/forwarded_headers.rs b/crates/aisix-core/src/forwarded_headers.rs index 19f75f060..3062fdd55 100644 --- a/crates/aisix-core/src/forwarded_headers.rs +++ b/crates/aisix-core/src/forwarded_headers.rs @@ -135,10 +135,34 @@ pub const TRACE_CONTEXT_HEADERS: &[&str] = &["traceparent", "tracestate"]; /// `forward_client_headers: ["x-*"]` does not start relaying the caller's /// `x-api-key` — which on `/v1/*` is the caller's own gateway key — the /// day it upgrades. +/// +/// The list here is the one every surface shares, and it is complete only +/// where the caller's credential always arrives in a slot it names — on +/// `/v1/*` and MCP the gateway reads `authorization` or `x-api-key` and +/// nothing else. A surface where the OPERATOR chooses the slot has to add +/// its own; see [`exact_match_only_with`]. pub fn exact_match_only(name: &str) -> bool { CREDENTIAL_SLOT_HEADERS.contains(&name) || TRACE_CONTEXT_HEADERS.contains(&name) } +/// [`exact_match_only`] plus the slots one surface names for itself. +/// +/// A `passthrough_route` picks its own header names: under `auth_mode: +/// header_key` the gateway credential arrives in the route's +/// `auth_header_name`, and `identity_header` carries an end-user identity +/// the route promises to record and strip. Nothing constrains an operator +/// to choose a name from the shared list — the route schema in fact +/// forbids the credential ones — so without this a `["x-*"]` pattern +/// sweeps in exactly the header the gateway just consumed to authenticate +/// the caller and relays it upstream, where it can be replayed against +/// this gateway. +/// +/// The rule is unchanged, only its input: a glob does not reach these +/// names, and a pattern that spells one out still forwards it. +pub fn exact_match_only_with(name: &str, surface_slots: &[&str]) -> bool { + exact_match_only(name) || surface_slots.iter().any(|s| s.eq_ignore_ascii_case(name)) +} + /// Whether a forwarded value may DISPLACE a header the gateway already /// placed on the outbound request. /// @@ -177,7 +201,17 @@ pub fn client_header_forwardable(name: &str) -> bool { /// snapshot. [`exact_match_only`] names the exception: those headers need /// a pattern that spells them out. pub fn forward_pattern_admits(patterns: &[String], name: &str) -> bool { - if exact_match_only(name) { + forward_pattern_admits_with(patterns, name, &[]) +} + +/// [`forward_pattern_admits`] for a surface that names credential slots of +/// its own — see [`exact_match_only_with`]. +pub fn forward_pattern_admits_with( + patterns: &[String], + name: &str, + surface_slots: &[&str], +) -> bool { + if exact_match_only_with(name, surface_slots) { return patterns.iter().any(|p| p.eq_ignore_ascii_case(name)); } patterns @@ -341,6 +375,46 @@ mod tests { } } + /// The shared list cannot know the name a `passthrough_route` picked + /// for its own gateway credential, and the route schema guarantees it + /// is NOT one of the shared names. So a glob would sweep in exactly + /// the header the gateway consumed to authenticate this caller. + #[test] + fn a_surface_slot_needs_its_own_name_not_a_glob() { + let slots = ["x-gw-key", "x-end-user"]; + for name in slots { + assert!(!exact_match_only(name), "{name}"); + assert!(exact_match_only_with(name, &slots), "{name}"); + assert!(!forward_pattern_admits_with(&["*".into()], name, &slots)); + assert!(!forward_pattern_admits_with(&["x-*".into()], name, &slots)); + // Naming it in full is still consent, on this face as on + // every other. + assert!(forward_pattern_admits_with( + &[name.to_string()], + name, + &slots + )); + assert!(forward_pattern_admits_with( + &[name.to_uppercase()], + name, + &slots + )); + } + // Only the named slots move: an ordinary header on the same + // surface still answers to the glob. + assert!(forward_pattern_admits_with( + &["x-*".into()], + "x-trace-id", + &slots + )); + // And a surface that names none behaves exactly as before. + assert!(forward_pattern_admits_with( + &["x-*".into()], + "x-gw-key", + &[] + )); + } + #[test] fn an_empty_allowlist_forwards_nothing() { let client = map(&[("authorization", "Bearer caller"), ("x-trace-id", "t")]); diff --git a/crates/aisix-core/src/lib.rs b/crates/aisix-core/src/lib.rs index 7b1b6585f..bc2af5729 100644 --- a/crates/aisix-core/src/lib.rs +++ b/crates/aisix-core/src/lib.rs @@ -44,10 +44,10 @@ pub use error::{ AdminError, AdminErrorEnvelope, BootstrapError, ProxyError, ProxyErrorEnvelope, RateLimitScope, }; pub use forwarded_headers::{ - client_header_forwardable, displaces_a_gateway_header, exact_match_only, - forward_pattern_admits, header_forward_blocked, resolve_forwarded_client_headers, - CREDENTIAL_SLOT_HEADERS, GATEWAY_HEADER_PREFIX, NEVER_FORWARD_FROM_CLIENT, - NEVER_FORWARD_FROM_CLIENT_PREFIXES, NON_FORWARDABLE_HEADERS, + client_header_forwardable, displaces_a_gateway_header, exact_match_only, exact_match_only_with, + forward_pattern_admits, forward_pattern_admits_with, header_forward_blocked, + resolve_forwarded_client_headers, CREDENTIAL_SLOT_HEADERS, GATEWAY_HEADER_PREFIX, + NEVER_FORWARD_FROM_CLIENT, NEVER_FORWARD_FROM_CLIENT_PREFIXES, NON_FORWARDABLE_HEADERS, }; pub use header_template::{render_header_template, HeaderVars, HEADER_TEMPLATE_VARS}; pub use models::{ diff --git a/crates/aisix-core/src/models/passthrough_route.rs b/crates/aisix-core/src/models/passthrough_route.rs index 8d141c615..26093bedf 100644 --- a/crates/aisix-core/src/models/passthrough_route.rs +++ b/crates/aisix-core/src/models/passthrough_route.rs @@ -149,6 +149,14 @@ pub struct PassthroughRoute { /// party's telemetry, so a broad pattern overrides the rest of the /// strip set and leaves those alone. /// + /// This route's own `auth_header_name` and `identity_header` are read + /// the same way. Both are slots this route chose rather than ones the + /// gateway owns — under `auth_mode: header_key` the first carries the + /// gateway credential the caller authenticated with, and the second + /// carries an end-user identity this route records and strips — so a + /// glob does not sweep either, and a pattern that names one in full + /// forwards it. + /// /// Headers whose forwarding would break the exchange rather than /// change who it comes from are stripped whatever the patterns say: /// `host`, `content-length`, the hop-by-hop headers that describe the diff --git a/crates/aisix-core/src/models/provider_key.rs b/crates/aisix-core/src/models/provider_key.rs index 7e2a046b0..fac0e521c 100644 --- a/crates/aisix-core/src/models/provider_key.rs +++ b/crates/aisix-core/src/models/provider_key.rs @@ -389,11 +389,17 @@ pub struct RequestOverrides { /// Two cases where a named header still does not reach the upstream: /// a `default_headers` entry of the same name wins it, since both are /// operator configuration and the static one is the more specific - /// choice; and on an AWS Bedrock provider the SigV4-signed names - /// (`authorization`, `x-amz-date`, `x-amz-content-sha256`, - /// `x-amz-security-token`) are refused from either source, because - /// the request signer derives them and a supplied value would break - /// the signature rather than authenticate anyone. + /// choice; and on an AWS Bedrock provider six names are refused from + /// either source. Five of them (`authorization`, `x-amz-date`, + /// `x-amz-content-sha256`, `x-amz-security-token`, `x-amz-target`) + /// are SigV4 inputs the request signer derives, so a supplied value + /// would break the signature rather than authenticate anyone; the + /// sixth, `x-amzn-bedrock-accept`, selects the response wire shape + /// the gateway then decodes. + /// + /// Naming a credential slot needs a data plane new enough to honor + /// it; an older one refuses those names outright, so the pattern has + /// no effect there rather than a different one. /// /// Headers whose forwarding would break the exchange rather than /// change who it comes from are never forwarded whatever the patterns diff --git a/crates/aisix-gateway/src/bridge.rs b/crates/aisix-gateway/src/bridge.rs index b5b23539c..3a21f1021 100644 --- a/crates/aisix-gateway/src/bridge.rs +++ b/crates/aisix-gateway/src/bridge.rs @@ -215,6 +215,9 @@ impl BridgeContext { provider_key_name: Some(&self.provider_key.display_name), }, client_headers: self.client_headers.as_deref(), + // A Bridge speaks HTTP to its upstream and owns no header + // name the shared lists do not already cover. + surface_blocked: &[], } } } diff --git a/crates/aisix-gateway/src/upstream_headers.rs b/crates/aisix-gateway/src/upstream_headers.rs index 4e4fc7067..f3f3462de 100644 --- a/crates/aisix-gateway/src/upstream_headers.rs +++ b/crates/aisix-gateway/src/upstream_headers.rs @@ -105,6 +105,11 @@ pub struct UpstreamHeaderContext<'a> { /// poll of an async job, a semantic-routing embedding lookup) — those /// requests forward nothing. pub client_headers: Option<&'a HeaderMap>, + /// Header names THIS surface owns, which the shared lists cannot know + /// about. `/v1/realtime` opens its own WebSocket, so the caller's + /// `sec-websocket-*` slots describe the connection the caller opened + /// and one of them carries the caller's own gateway key. + pub surface_blocked: &'a [&'a str], } impl<'a> UpstreamHeaderContext<'a> { @@ -126,6 +131,13 @@ impl<'a> UpstreamHeaderContext<'a> { self.client_headers = Some(headers); self } + + /// Names this surface refuses on top of the shared lists — see + /// [`UpstreamHeaderContext::surface_blocked`]. + pub fn with_surface_blocked(mut self, blocked: &'a [&'a str]) -> Self { + self.surface_blocked = blocked; + self + } } /// Resolve the operator-configured headers for one upstream call: rendered @@ -221,7 +233,7 @@ impl ForwardedClientHeaders { entries: aisix_core::resolve_forwarded_client_headers( &r.forward_client_headers, client, - &[], + ctx.surface_blocked, ), } } diff --git a/crates/aisix-provider-bedrock/src/wire.rs b/crates/aisix-provider-bedrock/src/wire.rs index a0a7d755e..cd054350d 100644 --- a/crates/aisix-provider-bedrock/src/wire.rs +++ b/crates/aisix-provider-bedrock/src/wire.rs @@ -57,5 +57,28 @@ mod tests { // honest about Bedrock specifics, not just generic SigV4. let reserved = reserved_sigv4_headers(); assert!(reserved.contains(&"x-amzn-bedrock-accept")); + assert!(reserved.contains(&"x-amz-target")); + } + + /// The `request.forward_client_headers` description in + /// `cp-admin.yaml` and `schemas/resources/provider_key.schema.json` + /// SPELLS OUT this list — it is what a user reads to know which names + /// a Bedrock upstream will refuse. Prose is a second definition and + /// drifts silently: it named four of these for a release while the + /// code refused six. Adding or removing an entry here fails this + /// assertion, which is the reminder to re-word the description. + #[test] + fn the_reserved_list_is_the_one_the_public_description_spells_out() { + assert_eq!( + reserved_sigv4_headers(), + [ + "authorization", + "x-amz-date", + "x-amz-content-sha256", + "x-amz-security-token", + "x-amz-target", + "x-amzn-bedrock-accept", + ] + ); } } diff --git a/crates/aisix-proxy/src/passthrough_route.rs b/crates/aisix-proxy/src/passthrough_route.rs index 9c794030a..b795c63e4 100644 --- a/crates/aisix-proxy/src/passthrough_route.rs +++ b/crates/aisix-proxy/src/passthrough_route.rs @@ -720,8 +720,25 @@ async fn dispatch( // it is handed, but hyper honours a caller-set value verbatim instead, // so a relayed copy is a request-framing bug waiting for the first // body this route rewrites. + // + // The exact-name rule is per ROUTE here. `/v1/*` and MCP read the + // caller's credential out of `authorization` or `x-api-key`, both on + // the shared list, but a route names its own slots: under `auth_mode: + // header_key` the gateway credential arrives in `auth_header_name`, + // and `identity_header` is one the route promises to strip. The route + // schema forbids the shared credential names for both, so without + // this a `["x-*"]` pattern would relay the very header this gateway + // authenticated the caller with. Naming either in full still forwards + // it — the rule is unchanged, only its input. + let route_slots: Vec<&str> = [ + route.auth_header_name.as_deref(), + route.identity_header.as_deref(), + ] + .into_iter() + .flatten() + .collect(); let forwards = |name: &str| { - aisix_core::forward_pattern_admits(&route.forward_client_headers, name) + aisix_core::forward_pattern_admits_with(&route.forward_client_headers, name, &route_slots) && !aisix_core::header_forward_blocked(name) && name != "content-length" }; @@ -3205,6 +3222,186 @@ mod tests { assert_eq!(rid.len(), 1); } + /// A `header_key` route names the slot its gateway credential + /// arrives in, and the route schema forbids every name on the shared + /// credential list — so the shared list can never cover it. A glob + /// must not sweep it upstream, where the caller's AISIX key would be + /// replayable against this gateway. + #[tokio::test] + async fn a_glob_never_sweeps_the_route_s_own_auth_header() { + let (upstream, snap) = slot_route_fixture(serde_json::json!({ + "auth_mode": "header_key", + "auth_header_name": "x-gw-key", + "forward_client_headers": ["x-*"] + })) + .await; + + let resp = build_app(snap) + .oneshot(slot_request(&[("x-gw-key", "sk-caller")])) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + + let received = &upstream.received_requests().await.unwrap()[0]; + assert!( + received.headers.get("x-gw-key").is_none(), + "`x-*` must not relay the slot this route authenticated the caller with" + ); + // The glob still does its job for everything else, so the + // assertion above is the rule firing rather than a pattern that + // never matched. + assert_eq!(received.headers.get("x-other").unwrap(), "ok"); + } + + /// Naming it in full is still consent — the rule narrows how a slot + /// is reached, never whether it can be. + #[tokio::test] + async fn the_route_s_own_auth_header_forwards_when_named_in_full() { + let (upstream, snap) = slot_route_fixture(serde_json::json!({ + "auth_mode": "header_key", + "auth_header_name": "x-gw-key", + "forward_client_headers": ["x-gw-key"] + })) + .await; + + let resp = build_app(snap) + .oneshot(slot_request(&[("x-gw-key", "sk-caller")])) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + + let received = &upstream.received_requests().await.unwrap()[0]; + assert_eq!(received.headers.get("x-gw-key").unwrap(), "sk-caller"); + } + + /// `identity_header`'s whole contract is that its value is recorded + /// on the usage event and stripped before forwarding — a glob that + /// put it back would make the promise false. + #[tokio::test] + async fn a_glob_never_sweeps_the_route_s_identity_header() { + let (upstream, snap) = slot_route_fixture(serde_json::json!({ + "identity_header": "x-end-user", + "forward_client_headers": ["x-*"] + })) + .await; + + let resp = build_app(snap) + .oneshot(slot_request(&[ + ("authorization", "Bearer sk-caller"), + ("x-end-user", "alice@example.com"), + ])) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + + let received = &upstream.received_requests().await.unwrap()[0]; + assert!(received.headers.get("x-end-user").is_none()); + assert_eq!(received.headers.get("x-other").unwrap(), "ok"); + } + + #[tokio::test] + async fn the_route_s_identity_header_forwards_when_named_in_full() { + let (upstream, snap) = slot_route_fixture(serde_json::json!({ + "identity_header": "x-end-user", + "forward_client_headers": ["x-end-user"] + })) + .await; + + let resp = build_app(snap) + .oneshot(slot_request(&[ + ("authorization", "Bearer sk-caller"), + ("x-end-user", "alice@example.com"), + ])) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + + let received = &upstream.received_requests().await.unwrap()[0]; + assert_eq!( + received.headers.get("x-end-user").unwrap(), + "alice@example.com" + ); + } + + /// `gateway_key` names no slot of its own — the schema forbids + /// `auth_header_name` outside `header_key` — so nothing joins the + /// exact-name set and a glob keeps meaning exactly what it did. + /// (`anonymous` is the same shape and is covered end-to-end, where a + /// real peer address can satisfy its `source_cidrs` gate.) + #[tokio::test] + async fn a_gateway_key_route_keeps_the_shared_rule_and_nothing_more() { + let (upstream, snap) = slot_route_fixture(serde_json::json!({ + "auth_mode": "gateway_key", + "forward_client_headers": ["x-*"] + })) + .await; + + let resp = build_app(snap) + .oneshot(slot_request(&[ + ("authorization", "Bearer sk-caller"), + ("x-gw-key", "not-a-slot-here"), + ])) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + + let received = &upstream.received_requests().await.unwrap()[0]; + assert_eq!(received.headers.get("x-other").unwrap(), "ok"); + // No slot was declared, so `x-gw-key` is an ordinary header the + // glob carries — the narrowing is per route, not global. + assert_eq!(received.headers.get("x-gw-key").unwrap(), "not-a-slot-here"); + // And the shared rule is untouched: `x-*` never reached + // `authorization`, so the ProviderKey's credential still rides + // alone. + let auths: Vec<_> = received.headers.get_all("authorization").iter().collect(); + assert_eq!(auths.len(), 1); + assert_eq!(auths[0], "Bearer sk-upstream"); + } + + /// An `inject` route with the given overrides merged onto it. The + /// upstream always answers `/v1/models`, and every request through + /// [`slot_request`] carries an ordinary `x-other`, so each test above + /// can tell "the rule fired" from "the pattern never matched". + async fn slot_route_fixture(overrides: serde_json::Value) -> (MockServer, AisixSnapshot) { + let upstream = MockServer::start().await; + Mock::given(wm_method("GET")) + .and(wm_path("/v1/models")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({"ok": true}))) + .mount(&upstream) + .await; + + let mut json = serde_json::json!({ + "name": "slot-route", + "path_prefix": "/passthrough/openai", + "target_url": upstream.uri(), + "provider_key_id": PK_ID + }); + let map = json.as_object_mut().unwrap(); + for (k, v) in overrides.as_object().unwrap() { + map.insert(k.clone(), v.clone()); + } + + let snap = AisixSnapshot::new(); + snap.provider_keys + .insert(provider_key_entry("http://unused")); + snap.apikeys.insert(apikey_entry("sk-caller", Some(&["*"]))); + snap.passthrough_routes + .insert(route_entry("route-slot", json)); + (upstream, snap) + } + + /// A caller request carrying `headers` plus one ordinary `x-` header. + fn slot_request(headers: &[(&str, &str)]) -> Request { + let mut b = Request::builder() + .method("GET") + .uri("/passthrough/openai/v1/models") + .header("x-other", "ok"); + for (k, v) in headers { + b = b.header(*k, *v); + } + b.body(axum::body::Body::empty()).unwrap() + } + #[test] fn copy_safe_headers_preserves_repeated_values() { let mut src = HeaderMap::new(); diff --git a/crates/aisix-proxy/src/realtime.rs b/crates/aisix-proxy/src/realtime.rs index 2c56e4928..d6452bc11 100644 --- a/crates/aisix-proxy/src/realtime.rs +++ b/crates/aisix-proxy/src/realtime.rs @@ -41,6 +41,16 @@ //! envelope) rather than accept-then-close-1008: same enforcement point, //! observable to every WS client as a failed handshake. //! +//! ## Forwarded client headers +//! +//! The ProviderKey's `request.forward_client_headers` applies here as on +//! every other `/v1/*` face: the named headers ride the upstream +//! handshake, and a named credential slot displaces the ProviderKey's own +//! rather than joining it. The handshake slots this surface owns are the +//! one addition to the shared refusals — see [`REALTIME_HANDSHAKE_SLOTS`]. +//! `request.default_headers` is a separate feature that this face has +//! never applied. +//! //! ## Usage //! //! The relay harvests `response.done` usage frames (and @@ -84,6 +94,29 @@ const HEADER_BETA_VALUE: &str = "realtime=v1"; /// `realtime=v1` is spelled `realtime-v1` here). const SUBPROTOCOL_BETA_ITEM: &str = "openai-beta.realtime-v1"; +/// Handshake slots this surface owns, refused to `forward_client_headers` +/// on top of the shared lists. +/// +/// Every other `/v1/*` face rebuilds an HTTP request; this one performs a +/// second WebSocket handshake, and these headers describe the handshake +/// the CALLER made rather than the request's content. Relaying +/// `sec-websocket-key` / `-version` / `-extensions` breaks the upstream +/// handshake outright — the gateway's client generated its own key and +/// negotiates its own extensions, and the accept value is computed +/// against them. `sec-websocket-protocol` is worse than broken: the +/// documented browser flow puts the caller's own AISIX key in it +/// (`openai-insecure-api-key.`), so relaying it would hand the +/// provider the credential this gateway authenticates with. It also +/// selects the subprotocol the gateway echoes back to the client, which +/// is the gateway's answer to make, not the upstream's. +const REALTIME_HANDSHAKE_SLOTS: &[&str] = &[ + "sec-websocket-accept", + "sec-websocket-extensions", + "sec-websocket-key", + "sec-websocket-protocol", + "sec-websocket-version", +]; + /// Is `needle` one of the comma-separated items across every value of /// `name`? Both headers are list-valued and may repeat, so a first-value /// substring test would both miss a repeat and accept `realtime=v10`. @@ -311,7 +344,19 @@ async fn prepare( let secret = crate::dispatch::require_api_key(&pk_entry.value, model)?.to_string(); let upstream_model = crate::dispatch::require_upstream_model(model)?.to_string(); - let upstream_request = match pk_entry.value.adapter { + // The ProviderKey's `request.forward_client_headers`, resolved against + // the caller's own handshake. This face builds its upstream request by + // hand instead of through the shared bridge pipeline, which is exactly + // where a per-request mechanism goes silently missing — an operator + // who declared that this upstream reads the caller's credential got it + // on every other `/v1/*` endpoint and not here. + let forwarded = aisix_gateway::ForwardedClientHeaders::resolve( + &aisix_gateway::UpstreamHeaderContext::from_overrides(pk_entry.value.request.as_ref()) + .with_client_headers(headers) + .with_surface_blocked(REALTIME_HANDSHAKE_SLOTS), + ); + + let mut upstream_request = match pk_entry.value.adapter { Some(Adapter::Openai) => { let base = crate::dispatch::resolve_base_url(&pk_entry.value)?; let url = crate::dispatch::build_openai_url(&base, "/realtime"); @@ -381,6 +426,13 @@ async fn prepare( ))); } }; + // AFTER the per-adapter build, which is what lets a credential slot + // the operator named displace the ProviderKey's own — the ordering + // `apply` documents and every other face follows. It leaves any other + // header the arms above set alone: those select how the exchange + // works, not who it is from. + forwarded.apply(upstream_request.headers_mut()); + let reservation = crate::quota::enforce( state, snapshot, @@ -1188,6 +1240,20 @@ mod tests { snap } + /// [`snapshot`] whose ProviderKey opted into forwarding `forward`. + fn snapshot_forwarding(api_base: &str, forward: &[&str]) -> AisixSnapshot { + let snap = snapshot(api_base, "openai", "openai"); + let pk_json = format!( + r#"{{"display_name":"rt-pk","secret":"sk-up","api_base":"{api_base}", + "provider":"openai","adapter":"openai", + "request":{{"forward_client_headers":{}}}}}"#, + serde_json::to_string(forward).unwrap() + ); + let pk: aisix_core::ProviderKey = serde_json::from_str(&pk_json).unwrap(); + snap.provider_keys.insert(ResourceEntry::new(PK_ID, pk, 1)); + snap + } + /// Bind the full proxy router on a real TCP port (WS handshakes need a /// live connection; `oneshot` can't upgrade). async fn serve( @@ -1214,10 +1280,20 @@ mod tests { (addr, state, rx) } + /// One header's value as a string, `""` when absent — the mock + /// records the whole map, and every assertion below reads one name. + fn header_str(headers: &HeaderMap, name: &str) -> String { + headers + .get(name) + .and_then(|v| v.to_str().ok()) + .unwrap_or("") + .to_string() + } + /// Scripted mock upstream: accepts ONE WebSocket, records the request - /// path + auth header + any `openai-beta` header, waits for one text - /// frame, replies with a `response.done` usage frame, then closes. - type SeenHandshake = Option<(String, String, Option)>; + /// path and its FULL header map, waits for one text frame, replies + /// with a `response.done` usage frame, then closes. + type SeenHandshake = Option<(String, HeaderMap)>; async fn spawn_upstream() -> ( std::net::SocketAddr, @@ -1237,18 +1313,7 @@ mod tests { stream, move |req: &tokio_tungstenite::tungstenite::handshake::server::Request, resp: tokio_tungstenite::tungstenite::handshake::server::Response| { - let auth = req - .headers() - .get("authorization") - .and_then(|v| v.to_str().ok()) - .unwrap_or("") - .to_string(); - let beta = req - .headers() - .get("openai-beta") - .and_then(|v| v.to_str().ok()) - .map(str::to_string); - *hs2.lock().unwrap() = Some((req.uri().to_string(), auth, beta)); + *hs2.lock().unwrap() = Some((req.uri().to_string(), req.headers().clone())); Ok(resp) }, ) @@ -1318,11 +1383,13 @@ mod tests { // Upstream saw the relayed client frame + the gateway's provider auth. assert_eq!(frames.lock().unwrap().len(), 1); assert!(frames.lock().unwrap()[0].contains("session.update")); - let (uri, auth, beta) = handshake + let (uri, seen) = handshake .lock() .unwrap() .clone() .expect("handshake recorded"); + let auth = header_str(&seen, "authorization"); + let beta = seen.get("openai-beta").map(|_| ()); assert!( uri.contains("/v1/realtime") && uri.contains("model=gpt-realtime"), "upstream URI must be the realtime path with the UPSTREAM model id, got {uri}" @@ -1347,6 +1414,139 @@ mod tests { assert_eq!(ev.api_key_id, "k-1"); } + /// `/v1/realtime` builds its upstream handshake by hand rather than + /// through the shared bridge pipeline, which is exactly where a + /// per-request mechanism goes silently missing: the operator + /// configured `forward_client_headers` on this ProviderKey and every + /// other `/v1/*` endpoint honoured it. + #[tokio::test] + async fn a_named_client_header_rides_the_upstream_handshake() { + let (up_addr, handshake, _frames) = spawn_upstream().await; + let snap = snapshot_forwarding( + &format!("http://{up_addr}/v1"), + &["x-user-jwt", "sec-websocket-extensions", "x-*"], + ); + let (addr, _state, _rx) = serve(snap).await; + + let mut req = format!("ws://{addr}/v1/realtime?model=rt-model") + .into_client_request() + .unwrap(); + req.headers_mut() + .insert("authorization", "Bearer sk-caller".parse().unwrap()); + req.headers_mut() + .insert("x-user-jwt", "eyJraw".parse().unwrap()); + // A handshake slot the gateway's own client did NOT set, so + // nothing would decline it on the way out — only the surface + // list stops it. + req.headers_mut().insert( + "sec-websocket-extensions", + "permessage-deflate".parse().unwrap(), + ); + let (ws, _) = tokio_tungstenite::connect_async(req) + .await + .expect("handshake"); + let (mut tx, mut client_rx) = ws.split(); + tx.send(TgMessage::Text("{\"type\":\"session.update\"}".into())) + .await + .unwrap(); + let _ = tokio::time::timeout(Duration::from_secs(3), client_rx.next()).await; + + let (_uri, seen) = handshake + .lock() + .unwrap() + .clone() + .expect("handshake recorded"); + assert_eq!(header_str(&seen, "x-user-jwt"), "eyJraw"); + // The caller did not name `authorization`, so the gateway's own + // provider credential still authenticates the session. + assert_eq!(header_str(&seen, "authorization"), "Bearer sk-up"); + // A `"*"`-shaped pattern is not consent to relay the handshake + // this surface owns. `permessage-deflate` at the upstream would + // enable a compression the gateway's own codec never negotiated, + // so the relay would decode garbage. + assert_eq!( + header_str(&seen, "sec-websocket-extensions"), + "", + "the caller's own handshake negotiation must not reach the upstream" + ); + } + + /// The credential collision, on this face as on every other: the + /// operator declared that this upstream reads the caller's own + /// credential, so the ProviderKey's stands aside rather than joining + /// it on the wire. + #[tokio::test] + async fn a_named_credential_slot_displaces_the_provider_key_s_own() { + let (up_addr, handshake, _frames) = spawn_upstream().await; + let snap = snapshot_forwarding(&format!("http://{up_addr}/v1"), &["authorization"]); + let (addr, _state, _rx) = serve(snap).await; + + let mut req = format!("ws://{addr}/v1/realtime?model=rt-model") + .into_client_request() + .unwrap(); + req.headers_mut() + .insert("authorization", "Bearer sk-caller".parse().unwrap()); + let (ws, _) = tokio_tungstenite::connect_async(req) + .await + .expect("handshake"); + let (mut tx, mut client_rx) = ws.split(); + tx.send(TgMessage::Text("{\"type\":\"session.update\"}".into())) + .await + .unwrap(); + let _ = tokio::time::timeout(Duration::from_secs(3), client_rx.next()).await; + + let (_uri, seen) = handshake + .lock() + .unwrap() + .clone() + .expect("handshake recorded"); + assert_eq!(header_str(&seen, "authorization"), "Bearer sk-caller"); + // And alone: a second value would let the upstream pick. + assert_eq!(seen.get_all("authorization").iter().count(), 1); + } + + /// The browser flow puts the caller's own AISIX key in + /// `sec-websocket-protocol`. Relaying that list would hand the + /// provider the credential this gateway authenticates with, so no + /// pattern reaches it — not even one naming it in full. + #[tokio::test] + async fn the_browser_credential_never_rides_the_upstream_handshake() { + let (up_addr, handshake, _frames) = spawn_upstream().await; + let snap = snapshot_forwarding( + &format!("http://{up_addr}/v1"), + &["sec-websocket-protocol", "*"], + ); + let (addr, _state, _rx) = serve(snap).await; + + let mut req = format!("ws://{addr}/v1/realtime?model=rt-model") + .into_client_request() + .unwrap(); + req.headers_mut().insert( + "sec-websocket-protocol", + "realtime, openai-insecure-api-key.sk-caller" + .parse() + .unwrap(), + ); + let (ws, _) = tokio_tungstenite::connect_async(req) + .await + .expect("subprotocol auth must be accepted"); + let (mut tx, mut client_rx) = ws.split(); + tx.send(TgMessage::Text("{\"type\":\"session.update\"}".into())) + .await + .unwrap(); + let _ = tokio::time::timeout(Duration::from_secs(3), client_rx.next()).await; + + let (_uri, seen) = handshake + .lock() + .unwrap() + .clone() + .expect("handshake recorded"); + assert!( + !header_str(&seen, "sec-websocket-protocol").contains("sk-caller"), + "the caller's gateway key must not reach the provider" + ); + } + #[tokio::test] async fn subprotocol_key_authenticates_and_realtime_is_echoed() { let (up_addr, _handshake, _frames) = spawn_upstream().await; @@ -1408,12 +1608,12 @@ mod tests { let _ = tokio::time::timeout(Duration::from_secs(3), client_rx.next()).await; // The opt-in rode the same later field, so it reaches upstream. - let (_uri, _auth, beta) = handshake + let (_uri, seen) = handshake .lock() .unwrap() .clone() .expect("handshake recorded"); - assert_eq!(beta.as_deref(), Some("realtime=v1")); + assert_eq!(header_str(&seen, "openai-beta"), "realtime=v1"); } /// The predicate itself: neither channel set => GA (no header). @@ -1501,14 +1701,14 @@ mod tests { .unwrap(); let _ = tokio::time::timeout(Duration::from_secs(3), client_rx.next()).await; - let (_uri, _auth, beta) = handshake + let (_uri, seen) = handshake .lock() .unwrap() .clone() .expect("handshake recorded"); assert_eq!( - beta.as_deref(), - Some("realtime=v1"), + header_str(&seen, "openai-beta"), + "realtime=v1", "an explicit client opt-in must reach the upstream" ); } diff --git a/schemas/resources/passthrough_route.schema.json b/schemas/resources/passthrough_route.schema.json index 90082cfce..264efa280 100644 --- a/schemas/resources/passthrough_route.schema.json +++ b/schemas/resources/passthrough_route.schema.json @@ -485,7 +485,7 @@ "type": "boolean" }, "forward_client_headers": { - "description": "Inbound client headers forwarded to the upstream even when this route would otherwise strip them, as single-`*` glob patterns matched case-insensitively against the header name (`\"authorization\"`, `\"x-trace-*\"`). Empty — the default — overrides no stripping.\n\nA route forwards the caller's headers by default, so this field only matters for the ones it removes: the ProviderKey's `strip_headers` under `credential_mode: inject`, and the slot the gateway consumed to authenticate the caller. Naming `authorization` under `auth_mode: gateway_key` therefore puts the caller's own credential back on the upstream request in place of the one this route would inject, never both — which is what lets an internal service that already authorizes on the end user's `Authorization` keep doing so unchanged.\n\nA credential slot — `authorization`, `proxy-authorization`, `x-api-key`, `api-key`, `x-goog-api-key`, `cookie` — and `traceparent` / `tracestate` are forwarded only when a pattern names them exactly. A glob such as `\"*\"` or `\"x-*\"` is a statement about the operator's own headers, not consent to hand a third party the caller's credential or to graft the caller's trace onto that party's telemetry, so a broad pattern overrides the rest of the strip set and leaves those alone.\n\nHeaders whose forwarding would break the exchange rather than change who it comes from are stripped whatever the patterns say: `host`, `content-length`, the hop-by-hop headers that describe the caller's own connection, and the gateway's `x-aisix-*` namespace.", + "description": "Inbound client headers forwarded to the upstream even when this route would otherwise strip them, as single-`*` glob patterns matched case-insensitively against the header name (`\"authorization\"`, `\"x-trace-*\"`). Empty — the default — overrides no stripping.\n\nA route forwards the caller's headers by default, so this field only matters for the ones it removes: the ProviderKey's `strip_headers` under `credential_mode: inject`, and the slot the gateway consumed to authenticate the caller. Naming `authorization` under `auth_mode: gateway_key` therefore puts the caller's own credential back on the upstream request in place of the one this route would inject, never both — which is what lets an internal service that already authorizes on the end user's `Authorization` keep doing so unchanged.\n\nA credential slot — `authorization`, `proxy-authorization`, `x-api-key`, `api-key`, `x-goog-api-key`, `cookie` — and `traceparent` / `tracestate` are forwarded only when a pattern names them exactly. A glob such as `\"*\"` or `\"x-*\"` is a statement about the operator's own headers, not consent to hand a third party the caller's credential or to graft the caller's trace onto that party's telemetry, so a broad pattern overrides the rest of the strip set and leaves those alone.\n\nThis route's own `auth_header_name` and `identity_header` are read the same way. Both are slots this route chose rather than ones the gateway owns — under `auth_mode: header_key` the first carries the gateway credential the caller authenticated with, and the second carries an end-user identity this route records and strips — so a glob does not sweep either, and a pattern that names one in full forwards it.\n\nHeaders whose forwarding would break the exchange rather than change who it comes from are stripped whatever the patterns say: `host`, `content-length`, the hop-by-hop headers that describe the caller's own connection, and the gateway's `x-aisix-*` namespace.", "items": { "type": "string" }, diff --git a/schemas/resources/provider_key.schema.json b/schemas/resources/provider_key.schema.json index 0308e6cca..8479e9602 100644 --- a/schemas/resources/provider_key.schema.json +++ b/schemas/resources/provider_key.schema.json @@ -129,7 +129,7 @@ "type": "object" }, "forward_client_headers": { - "description": "Inbound client headers forwarded to the upstream, as single-`*` glob patterns matched case-insensitively against the header name (`\"anthropic-beta\"`, `\"x-trace-*\"`, `\"authorization\"`). Empty — the default — forwards nothing, which is the behavior of every standard-protocol endpoint before AISIX-Cloud#1167.\n\nA header named here reaches the upstream whatever the gateway would otherwise do with it. Naming a credential slot — `authorization`, `proxy-authorization`, `x-api-key`, `api-key`, `x-goog-api-key`, `cookie` — hands the upstream the caller's own credential in place of the one this ProviderKey would inject there, never both. That is what lets an internal service that already authorizes on the end user's `Authorization` keep doing so unchanged. Any OTHER header the gateway had already set is left alone: it selects how the exchange works, not who it is from.\n\nA credential slot, and `traceparent` / `tracestate`, are forwarded only when a pattern names them exactly — a glob such as `\"*\"` or `\"x-*\"` is a statement about the operator's own headers, not consent to hand a third party the caller's credential or to graft the caller's trace onto that party's telemetry.\n\nTwo cases where a named header still does not reach the upstream: a `default_headers` entry of the same name wins it, since both are operator configuration and the static one is the more specific choice; and on an AWS Bedrock provider the SigV4-signed names (`authorization`, `x-amz-date`, `x-amz-content-sha256`, `x-amz-security-token`) are refused from either source, because the request signer derives them and a supplied value would break the signature rather than authenticate anyone.\n\nHeaders whose forwarding would break the exchange rather than change who it comes from are never forwarded whatever the patterns say: `host`, the hop-by-hop headers that describe the caller's own connection, and the gateway's `x-aisix-*` namespace. The headers describing a body this gateway re-serializes or a response shape it parses (`content-type`, `content-length`, `accept`, `anthropic-version`, `x-stainless-*`) are excluded for the same reason. `traceparent` and `tracestate` are forwarded only when a pattern names them exactly — a glob is not read as consent to graft the caller's trace onto the upstream's telemetry.", + "description": "Inbound client headers forwarded to the upstream, as single-`*` glob patterns matched case-insensitively against the header name (`\"anthropic-beta\"`, `\"x-trace-*\"`, `\"authorization\"`). Empty — the default — forwards nothing, which is the behavior of every standard-protocol endpoint before AISIX-Cloud#1167.\n\nA header named here reaches the upstream whatever the gateway would otherwise do with it. Naming a credential slot — `authorization`, `proxy-authorization`, `x-api-key`, `api-key`, `x-goog-api-key`, `cookie` — hands the upstream the caller's own credential in place of the one this ProviderKey would inject there, never both. That is what lets an internal service that already authorizes on the end user's `Authorization` keep doing so unchanged. Any OTHER header the gateway had already set is left alone: it selects how the exchange works, not who it is from.\n\nA credential slot, and `traceparent` / `tracestate`, are forwarded only when a pattern names them exactly — a glob such as `\"*\"` or `\"x-*\"` is a statement about the operator's own headers, not consent to hand a third party the caller's credential or to graft the caller's trace onto that party's telemetry.\n\nTwo cases where a named header still does not reach the upstream: a `default_headers` entry of the same name wins it, since both are operator configuration and the static one is the more specific choice; and on an AWS Bedrock provider six names are refused from either source. Five of them (`authorization`, `x-amz-date`, `x-amz-content-sha256`, `x-amz-security-token`, `x-amz-target`) are SigV4 inputs the request signer derives, so a supplied value would break the signature rather than authenticate anyone; the sixth, `x-amzn-bedrock-accept`, selects the response wire shape the gateway then decodes.\n\nNaming a credential slot needs a data plane new enough to honor it; an older one refuses those names outright, so the pattern has no effect there rather than a different one.\n\nHeaders whose forwarding would break the exchange rather than change who it comes from are never forwarded whatever the patterns say: `host`, the hop-by-hop headers that describe the caller's own connection, and the gateway's `x-aisix-*` namespace. The headers describing a body this gateway re-serializes or a response shape it parses (`content-type`, `content-length`, `accept`, `anthropic-version`, `x-stainless-*`) are excluded for the same reason. `traceparent` and `tracestate` are forwarded only when a pattern names them exactly — a glob is not read as consent to graft the caller's trace onto the upstream's telemetry.", "items": { "type": "string" }, diff --git a/tests/e2e/src/cases/forward-client-headers-e2e.test.ts b/tests/e2e/src/cases/forward-client-headers-e2e.test.ts index 2f92eb1a6..c500a938e 100644 --- a/tests/e2e/src/cases/forward-client-headers-e2e.test.ts +++ b/tests/e2e/src/cases/forward-client-headers-e2e.test.ts @@ -36,6 +36,12 @@ import { // default is the opposite (forward everything) // and the list OVERRIDES a strip. // +// And beside them, the sub-dispatch paths of the model kinds that do NOT +// go through the one convergence point the faces above share: an +// ensemble's panel members and its judge each reach a DIFFERENT upstream, +// so each obeys ITS OWN ProviderKey's list rather than the ensemble +// entry's — the entry has no ProviderKey at all. +// // The credential collision is asserted on every face: a header the // operator named beats the credential the gateway would otherwise inject // into that slot, and rides ALONE — two credentials on the wire would let @@ -49,7 +55,12 @@ const CALLER_HASH = createHash("sha256").update(CALLER_KEY).digest("hex"); const FORWARD_MODEL = "fwd-openai-model"; const PLAIN_MODEL = "fwd-plain-model"; const ANTHROPIC_MODEL = "fwd-anthropic-model"; +const ENSEMBLE_MODEL = "fwd-ensemble-model"; +const ENSEMBLE_MEMBER_A = "fwd-ensemble-member-a"; +const ENSEMBLE_MEMBER_B = "fwd-ensemble-member-b"; +const ENSEMBLE_JUDGE = "fwd-ensemble-judge"; const ROUTE_PREFIX = "/passthrough/fwd"; +const GATEWAY_KEY_HEADER = "x-gw-key"; const CUSTOM_HEADER = "x-user-jwt"; const CALLER_CREDENTIAL = "Bearer callers-own-credential"; const GATEWAY_SECRET = "sk-mock-provider-secret"; @@ -215,6 +226,28 @@ describe("forward_client_headers e2e: one capability across every proxy face", ( provider_key_id: plainPk.id, }); + // An ensemble whose panel members and judge all sit behind the + // opted-in ProviderKey. `ProxyModelCaller::call` and the streaming + // judge build their own dispatch contexts instead of reusing the + // entry's, which is the shape this repo's model-kind rule calls the + // most-repeated silent gap. + for (const member of [ENSEMBLE_MEMBER_A, ENSEMBLE_MEMBER_B, ENSEMBLE_JUDGE]) { + await seed.createModel({ + display_name: member, + provider: "openai", + model_name: "gpt-4o-mini", + provider_key_id: forwardingPk.id, + }); + } + await seed.createModel({ + display_name: ENSEMBLE_MODEL, + ensemble: { + panel: [{ model: ENSEMBLE_MEMBER_A }, { model: ENSEMBLE_MEMBER_B }], + judge: { model: ENSEMBLE_JUDGE }, + min_responses: 2, + }, + }); + // The Anthropic bridge assembles its upstream request by hand rather // than through the shared pipeline — the classic place a per-request // mechanism goes silently missing. Its credential slot is `x-api-key`. @@ -295,13 +328,39 @@ describe("forward_client_headers e2e: one capability across every proxy face", ( forward_client_headers: [CUSTOM_HEADER, "*"], }); - await seed.createApiKey({ + const callerKey = await seed.createApiKey({ key_hash: CALLER_HASH, allowed_models: ["*"], allowed_routes: ["*"], mcp_access: { allow: ["*"] }, }); + // A route that names its OWN gateway-credential slot, and one that + // names none. `x-*` is the same pattern on both: what differs is + // whether the route declared a slot for it to have to leave alone. + await seed.createPassthroughRoute({ + name: "fwd-route-headerkey", + path_prefix: `${ROUTE_PREFIX}-headerkey`, + target_url: upstream.baseUrl, + auth_mode: "header_key", + auth_header_name: GATEWAY_KEY_HEADER, + credential_mode: "inject", + provider_key_id: routePk.id, + identity_header: "x-end-user", + forward_client_headers: ["x-*"], + }); + await seed.createPassthroughRoute({ + name: "fwd-route-anon", + path_prefix: `${ROUTE_PREFIX}-anon`, + target_url: upstream.baseUrl, + auth_mode: "anonymous", + anonymous_key_id: callerKey.id, + source_cidrs: ["127.0.0.0/8", "::1/128"], + credential_mode: "inject", + provider_key_id: routePk.id, + forward_client_headers: ["x-*"], + }); + await waitConfigPropagation(async () => { const res = await chat(FORWARD_MODEL); await res.text(); @@ -422,6 +481,60 @@ describe("forward_client_headers e2e: one capability across every proxy face", ( expect(occurrences(forwarded, "authorization")).toBe(1); }); + test("a glob never sweeps the slots a route named for itself", async () => { + if (!etcdReachable) return; + const mark = upstream!.receivedRequests.length; + + // `header_key`: the caller's AISIX key arrives in the route's own + // `x-gw-key`, and the end-user identity in its own `x-end-user`. + // Both match `x-*`, and neither is on the credential list every + // surface shares — the route schema in fact forbids those names — + // so only the per-route rule keeps them off the wire. + await fetch(`${app!.proxyUrl}${ROUTE_PREFIX}-headerkey/v1/chat/completions`, { + method: "POST", + headers: { + "content-type": "application/json", + [GATEWAY_KEY_HEADER]: CALLER_KEY, + "x-end-user": "alice@example.com", + "x-allowed": "yes", + }, + body: JSON.stringify({ model: "gpt-4o-mini", messages: [] }), + }).then((r) => bodyOf(r, "the header_key route")); + + const seen = since(mark).at(-1)!; + expect(seen.headers[GATEWAY_KEY_HEADER]).toBeUndefined(); + expect(seen.headers["x-end-user"]).toBeUndefined(); + // The same `x-*` still carries an ordinary header, so the two + // assertions above are the rule firing rather than a dead pattern. + expect(seen.headers["x-allowed"]).toBe("yes"); + }); + + test("a route that names no slot of its own is unchanged", async () => { + if (!etcdReachable) return; + const mark = upstream!.receivedRequests.length; + + // `anonymous` reads no inbound credential at all, so nothing joins + // the exact-name set and `x-*` means exactly what it always did — + // including for a header another route would have treated as a slot. + await fetch(`${app!.proxyUrl}${ROUTE_PREFIX}-anon/v1/chat/completions`, { + method: "POST", + headers: { + "content-type": "application/json", + [GATEWAY_KEY_HEADER]: "not-a-slot-on-this-route", + "x-allowed": "yes", + }, + body: JSON.stringify({ model: "gpt-4o-mini", messages: [] }), + }).then((r) => bodyOf(r, "the anonymous route")); + + const seen = since(mark).at(-1)!; + expect(seen.headers[GATEWAY_KEY_HEADER]).toBe("not-a-slot-on-this-route"); + expect(seen.headers["x-allowed"]).toBe("yes"); + // And the shared rule is untouched: the ProviderKey's credential + // still rides alone. + expect(seen.headers.authorization).toBe(`Bearer ${GATEWAY_SECRET}`); + expect(occurrences(seen, "authorization")).toBe(1); + }); + test("a passthrough route never doubles a header it injects", async () => { if (!etcdReachable) return; const mark = upstream!.receivedRequests.length; @@ -491,6 +604,56 @@ describe("forward_client_headers e2e: one capability across every proxy face", ( } }); + test("an ensemble's panel and judge each forward on their own upstream", async () => { + if (!etcdReachable) return; + const mark = upstream!.receivedRequests.length; + + await chat(ENSEMBLE_MODEL, { [CUSTOM_HEADER]: "carried.verbatim" }).then((r) => + bodyOf(r, "the ensemble entry"), + ); + + // Two panel members plus the judge, each a separate upstream call + // built by `ProxyModelCaller::call` rather than by the single + // dispatch chokepoint. + const calls = since(mark); + expect(calls.length, "panel of two plus a judge").toBe(3); + for (const call of calls) { + expect(call.headers[CUSTOM_HEADER]).toBe("carried.verbatim"); + // Each sub-call reads its OWN ProviderKey's list, which here also + // names the credential slot — the ensemble entry has no + // ProviderKey to inherit one from. + expect(call.headers.authorization).toBe(`Bearer ${CALLER_KEY}`); + } + }); + + test("a streamed ensemble judge forwards too", async () => { + if (!etcdReachable) return; + const mark = upstream!.receivedRequests.length; + + // The streaming judge is dispatched from a different call site than + // the buffered one, and only this path exercises it. + const res = await fetch(`${app!.proxyUrl}/v1/chat/completions`, { + method: "POST", + headers: { + authorization: `Bearer ${CALLER_KEY}`, + "content-type": "application/json", + [CUSTOM_HEADER]: "carried.verbatim", + }, + body: JSON.stringify({ + model: ENSEMBLE_MODEL, + messages: [{ role: "user", content: "probe" }], + stream: true, + }), + }); + await bodyOf(res, "the streamed ensemble"); + + const calls = since(mark); + expect(calls.length).toBe(3); + for (const call of calls) { + expect(call.headers[CUSTOM_HEADER]).toBe("carried.verbatim"); + } + }); + test("the gateway's own namespace and trace context resist a glob", async () => { if (!etcdReachable) return; const mark = upstream!.receivedRequests.length; From c6ae9f8f65fd97f2dd4429af2a0bff9cfc8bc2b0 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Wed, 2 Sep 2026 14:43:19 +0000 Subject: [PATCH 2/3] fix(headers): keep the route's slot list off the heap, and say what the realtime block list really stops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups, no behavior change. The passthrough route's slot list was a collected `Vec` built per request for at most two entries. It is a fixed array now, with `""` standing for an unset slot — a header name is never empty, on the wire or in the schema, so the sentinel matches nothing and needs no filtering out. Pinned by a test, since the code now depends on it. The `REALTIME_HANDSHAKE_SLOTS` comment claimed relaying `sec-websocket-key` or `-version` would break the upstream handshake. Delivery already declines a non-credential name the outbound request carries, and it carries both, so they are listed for completeness rather than because a pattern can reach them today. `sec-websocket-protocol` and `-extensions` are the two that genuinely are reachable — nothing else declines them — and the comment now says which is which. --- crates/aisix-core/src/forwarded_headers.rs | 13 +++++++++++- crates/aisix-proxy/src/passthrough_route.rs | 16 ++++++++------- crates/aisix-proxy/src/realtime.rs | 22 ++++++++++++++------- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/crates/aisix-core/src/forwarded_headers.rs b/crates/aisix-core/src/forwarded_headers.rs index 3062fdd55..3a464dc7a 100644 --- a/crates/aisix-core/src/forwarded_headers.rs +++ b/crates/aisix-core/src/forwarded_headers.rs @@ -407,12 +407,23 @@ mod tests { "x-trace-id", &slots )); - // And a surface that names none behaves exactly as before. + // And a surface that names none behaves exactly as before — + // whether it passes an empty list, or (as the passthrough call + // site does, to keep a two-element list off the heap) a fixed + // array with `""` standing for an unset slot. A header name is + // never empty, on the wire or in the schema, so the sentinel + // matches nothing and needs no filtering out. assert!(forward_pattern_admits_with( &["x-*".into()], "x-gw-key", &[] )); + assert!(!exact_match_only_with("x-gw-key", &["", ""])); + assert!(forward_pattern_admits_with( + &["x-*".into()], + "x-gw-key", + &["", ""] + )); } #[test] diff --git a/crates/aisix-proxy/src/passthrough_route.rs b/crates/aisix-proxy/src/passthrough_route.rs index b795c63e4..c3f36a259 100644 --- a/crates/aisix-proxy/src/passthrough_route.rs +++ b/crates/aisix-proxy/src/passthrough_route.rs @@ -730,13 +730,15 @@ async fn dispatch( // this a `["x-*"]` pattern would relay the very header this gateway // authenticated the caller with. Naming either in full still forwards // it — the rule is unchanged, only its input. - let route_slots: Vec<&str> = [ - route.auth_header_name.as_deref(), - route.identity_header.as_deref(), - ] - .into_iter() - .flatten() - .collect(); + // + // A fixed array rather than a collected `Vec`: there are at most two, + // on a per-request path. An unset slot stands as `""`, which matches + // nothing — a header name is never empty, on the wire or in the + // schema, so the empty entry needs no filtering out. + let route_slots = [ + route.auth_header_name.as_deref().unwrap_or_default(), + route.identity_header.as_deref().unwrap_or_default(), + ]; let forwards = |name: &str| { aisix_core::forward_pattern_admits_with(&route.forward_client_headers, name, &route_slots) && !aisix_core::header_forward_blocked(name) diff --git a/crates/aisix-proxy/src/realtime.rs b/crates/aisix-proxy/src/realtime.rs index d6452bc11..79367075d 100644 --- a/crates/aisix-proxy/src/realtime.rs +++ b/crates/aisix-proxy/src/realtime.rs @@ -99,16 +99,24 @@ const SUBPROTOCOL_BETA_ITEM: &str = "openai-beta.realtime-v1"; /// /// Every other `/v1/*` face rebuilds an HTTP request; this one performs a /// second WebSocket handshake, and these headers describe the handshake -/// the CALLER made rather than the request's content. Relaying -/// `sec-websocket-key` / `-version` / `-extensions` breaks the upstream -/// handshake outright — the gateway's client generated its own key and -/// negotiates its own extensions, and the accept value is computed -/// against them. `sec-websocket-protocol` is worse than broken: the -/// documented browser flow puts the caller's own AISIX key in it +/// the CALLER made rather than the request's content. +/// +/// `sec-websocket-protocol` is the dangerous one: the documented browser +/// flow puts the caller's own AISIX key in it /// (`openai-insecure-api-key.`), so relaying it would hand the /// provider the credential this gateway authenticates with. It also /// selects the subprotocol the gateway echoes back to the client, which -/// is the gateway's answer to make, not the upstream's. +/// is the gateway's answer to make, not the upstream's. Nothing else +/// declines it — the outbound handshake does not carry one, so a matching +/// pattern would insert the caller's list verbatim. `-extensions` is the +/// same shape and enables a compression the gateway's own codec never +/// negotiated. +/// +/// `sec-websocket-key` and `-version` are listed for completeness rather +/// than because they are reachable today: the outbound request already +/// carries both, and a non-credential name already present is declined on +/// delivery. Naming them here keeps that from being the only thing +/// standing between a caller's key and the accept value computed from it. const REALTIME_HANDSHAKE_SLOTS: &[&str] = &[ "sec-websocket-accept", "sec-websocket-extensions", From 38ec07ae530b707b10554200f1c5baaeafb621f3 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Wed, 2 Sep 2026 15:12:35 +0000 Subject: [PATCH 3/3] fix(realtime): drop a header value the handshake cannot render, and make the controls real MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round. One behavior fix, the rest are tests and prose. `/v1/realtime` renders its upstream handshake as text and refuses a header value it cannot read as a string, failing the whole connection rather than that one header. Header values may legally carry 0x80-0xFF and hyper accepts them inbound, so once this face started forwarding client headers a caller sending `x-user-name: José` could no longer open a realtime session at all — while the same header is forwarded byte-for-byte on every other face. Such an entry is now dropped, and the session opens without it. Reproduced over a raw socket, because the WebSocket client used in tests renders the outbound handshake as text too and cannot send the header in the first place. The control assertions in the passthrough tests were checking a header the route never strips. A passthrough route relays whatever it does not strip, so those lines held whether or not a pattern matched — they could not show what their comments claimed they showed. Every control is now a header the ProviderKey strips and the glob recovers, which also makes the `gateway_key` case pin what it was written for: `x-gw-key` IS in that route's strip set, so the pattern is genuinely asked about it and answers yes because THIS route declared no slot. Widen the narrowing to a global list and that test fails. `/v1/realtime` also had no coverage at the real-binary-plus-etcd layer; the existing case now asserts a named header reaches the upstream handshake while the browser flow's `sec-websocket-protocol` does not, even though the ProviderKey names it exactly. Two prose corrections. `x-amz-target` is not derived by the SigV4 signer — it is the operation selector the signature covers — so the description now says the signer OWNS all six names and drops any supplied value, matching how the control plane already words it. And the claim that the route schema forbids every shared credential name for `auth_header_name` / `identity_header` was wrong: it forbids five of them, and the two it allows are on the shared list already, which is what makes the union complete. --- crates/aisix-core/src/forwarded_headers.rs | 10 ++- crates/aisix-core/src/models/provider_key.rs | 12 ++- crates/aisix-gateway/src/upstream_headers.rs | 20 +++++ crates/aisix-proxy/src/passthrough_route.rs | 60 ++++++++++---- crates/aisix-proxy/src/realtime.rs | 75 ++++++++++++++++- schemas/resources/provider_key.schema.json | 2 +- .../cases/forward-client-headers-e2e.test.ts | 37 +++++++-- tests/e2e/src/cases/realtime-ws-e2e.test.ts | 80 ++++++++++++++++++- 8 files changed, 257 insertions(+), 39 deletions(-) diff --git a/crates/aisix-core/src/forwarded_headers.rs b/crates/aisix-core/src/forwarded_headers.rs index 3a464dc7a..9d12232d7 100644 --- a/crates/aisix-core/src/forwarded_headers.rs +++ b/crates/aisix-core/src/forwarded_headers.rs @@ -150,10 +150,12 @@ pub fn exact_match_only(name: &str) -> bool { /// A `passthrough_route` picks its own header names: under `auth_mode: /// header_key` the gateway credential arrives in the route's /// `auth_header_name`, and `identity_header` carries an end-user identity -/// the route promises to record and strip. Nothing constrains an operator -/// to choose a name from the shared list — the route schema in fact -/// forbids the credential ones — so without this a `["x-*"]` pattern -/// sweeps in exactly the header the gateway just consumed to authenticate +/// the route promises to record and strip. The route schema forbids MOST +/// of the shared credential names for those two fields, and the two it +/// does allow (`api-key`, `x-goog-api-key`) are on the shared list +/// already — so the union below is what makes the rule complete, and +/// without it a `["x-*"]` pattern sweeps in exactly the header the +/// gateway just consumed to authenticate /// the caller and relays it upstream, where it can be replayed against /// this gateway. /// diff --git a/crates/aisix-core/src/models/provider_key.rs b/crates/aisix-core/src/models/provider_key.rs index fac0e521c..3ae3ca08b 100644 --- a/crates/aisix-core/src/models/provider_key.rs +++ b/crates/aisix-core/src/models/provider_key.rs @@ -389,13 +389,11 @@ pub struct RequestOverrides { /// Two cases where a named header still does not reach the upstream: /// a `default_headers` entry of the same name wins it, since both are /// operator configuration and the static one is the more specific - /// choice; and on an AWS Bedrock provider six names are refused from - /// either source. Five of them (`authorization`, `x-amz-date`, - /// `x-amz-content-sha256`, `x-amz-security-token`, `x-amz-target`) - /// are SigV4 inputs the request signer derives, so a supplied value - /// would break the signature rather than authenticate anyone; the - /// sixth, `x-amzn-bedrock-accept`, selects the response wire shape - /// the gateway then decodes. + /// choice; and on an AWS Bedrock provider the request signer owns + /// `authorization`, `x-amz-date`, `x-amz-content-sha256`, + /// `x-amz-security-token`, `x-amz-target` and `x-amzn-bedrock-accept`, + /// and drops any supplied value. A value there would not authenticate + /// anyone: it either loses to the signer or breaks the signature. /// /// Naming a credential slot needs a data plane new enough to honor /// it; an older one refuses those names outright, so the pattern has diff --git a/crates/aisix-gateway/src/upstream_headers.rs b/crates/aisix-gateway/src/upstream_headers.rs index f3f3462de..a8c36981a 100644 --- a/crates/aisix-gateway/src/upstream_headers.rs +++ b/crates/aisix-gateway/src/upstream_headers.rs @@ -262,6 +262,26 @@ impl ForwardedClientHeaders { /// put two values on the wire and let the upstream pick between them, /// which on a credential slot is the #411 shape. /// + /// Drop the entries whose value is not ASCII, returning how many went. + /// + /// Only `/v1/realtime` calls this. Its WebSocket client writes the + /// upstream handshake as text and calls `HeaderValue::to_str` on every + /// header it was given, so a single obs-text byte (0x80-0xFF — legal + /// in a header value, and accepted on the way in) fails the whole + /// upstream connection rather than that one header. Every other face + /// hands the bytes to reqwest, which writes them out unexamined, so a + /// caller sending `x-user-name: José` is forwarded there and would + /// otherwise be unable to open a realtime session at all. + /// + /// Dropping rather than failing matches how this pipeline treats an + /// entry it cannot turn into a header anywhere else: the exchange goes + /// ahead without it. + pub fn drop_non_ascii_values(&mut self) -> usize { + let before = self.entries.len(); + self.entries.retain(|(_, v)| v.to_str().is_ok()); + before - self.entries.len() + } + /// Every surface delivers through here, including the ones that /// resolve once and reuse across several round-trips (jobs, videos) — /// the precedence is a property of this type, not of each call site. diff --git a/crates/aisix-proxy/src/passthrough_route.rs b/crates/aisix-proxy/src/passthrough_route.rs index c3f36a259..55c7b1903 100644 --- a/crates/aisix-proxy/src/passthrough_route.rs +++ b/crates/aisix-proxy/src/passthrough_route.rs @@ -725,11 +725,13 @@ async fn dispatch( // caller's credential out of `authorization` or `x-api-key`, both on // the shared list, but a route names its own slots: under `auth_mode: // header_key` the gateway credential arrives in `auth_header_name`, - // and `identity_header` is one the route promises to strip. The route - // schema forbids the shared credential names for both, so without - // this a `["x-*"]` pattern would relay the very header this gateway - // authenticated the caller with. Naming either in full still forwards - // it — the rule is unchanged, only its input. + // and `identity_header` is one the route promises to strip. Neither + // can be a name the shared list already covers in any way that helps: + // the route schema rejects most of them outright, and the two it + // permits are on that list anyway. So without this a `["x-*"]` + // pattern would relay the very header this gateway authenticated the + // caller with. Naming either in full still forwards it — the rule is + // unchanged, only its input. // // A fixed array rather than a collected `Vec`: there are at most two, // on a per-request path. An unset slot stands as `""`, which matches @@ -3249,10 +3251,13 @@ mod tests { received.headers.get("x-gw-key").is_none(), "`x-*` must not relay the slot this route authenticated the caller with" ); - // The glob still does its job for everything else, so the - // assertion above is the rule firing rather than a pattern that - // never matched. - assert_eq!(received.headers.get("x-other").unwrap(), "ok"); + // The SAME `x-*` recovers a stripped header that is not a slot, + // so the assertion above is the rule firing rather than a pattern + // that was never asked. + assert_eq!( + received.headers.get("x-stripped-control").unwrap(), + "recovered" + ); } /// Naming it in full is still consent — the rule narrows how a slot @@ -3298,7 +3303,10 @@ mod tests { let received = &upstream.received_requests().await.unwrap()[0]; assert!(received.headers.get("x-end-user").is_none()); - assert_eq!(received.headers.get("x-other").unwrap(), "ok"); + assert_eq!( + received.headers.get("x-stripped-control").unwrap(), + "recovered" + ); } #[tokio::test] @@ -3348,9 +3356,15 @@ mod tests { assert_eq!(resp.status(), StatusCode::OK); let received = &upstream.received_requests().await.unwrap()[0]; - assert_eq!(received.headers.get("x-other").unwrap(), "ok"); - // No slot was declared, so `x-gw-key` is an ordinary header the - // glob carries — the narrowing is per route, not global. + assert_eq!( + received.headers.get("x-stripped-control").unwrap(), + "recovered" + ); + // `x-gw-key` is in this ProviderKey's strip set, so `forwards()` + // IS asked about it here — and answers yes, because THIS route + // declared no slot. That is what makes the narrowing per route + // rather than a name added to the shared list: widen it to a + // global and this assertion fails. assert_eq!(received.headers.get("x-gw-key").unwrap(), "not-a-slot-here"); // And the shared rule is untouched: `x-*` never reached // `authorization`, so the ProviderKey's credential still rides @@ -3383,21 +3397,33 @@ mod tests { map.insert(k.clone(), v.clone()); } + // `strip_headers` names three `x-` headers, so `x-*` is asked + // about all three and the CONTROL below is a real observation of + // the glob firing. Without one in the strip set, a passthrough + // route forwards it by default whatever the patterns say — an + // assertion that proves nothing about this rule. + let pk_json = r#"{"display_name":"openai-up","secret":"sk-upstream", + "api_base":"http://unused","provider":"openai","adapter":"openai", + "strip_headers":["authorization","x-api-key","x-gw-key","x-end-user", + "x-stripped-control"]}"#; + let pk: ProviderKey = serde_json::from_str(pk_json).unwrap(); + let snap = AisixSnapshot::new(); - snap.provider_keys - .insert(provider_key_entry("http://unused")); + snap.provider_keys.insert(ResourceEntry::new(PK_ID, pk, 1)); snap.apikeys.insert(apikey_entry("sk-caller", Some(&["*"]))); snap.passthrough_routes .insert(route_entry("route-slot", json)); (upstream, snap) } - /// A caller request carrying `headers` plus one ordinary `x-` header. + /// A caller request carrying `headers` plus the control header — an + /// `x-` name the ProviderKey strips, so only a live `x-*` pattern + /// puts it back on the wire. fn slot_request(headers: &[(&str, &str)]) -> Request { let mut b = Request::builder() .method("GET") .uri("/passthrough/openai/v1/models") - .header("x-other", "ok"); + .header("x-stripped-control", "recovered"); for (k, v) in headers { b = b.header(*k, *v); } diff --git a/crates/aisix-proxy/src/realtime.rs b/crates/aisix-proxy/src/realtime.rs index 79367075d..edb14924c 100644 --- a/crates/aisix-proxy/src/realtime.rs +++ b/crates/aisix-proxy/src/realtime.rs @@ -358,11 +358,23 @@ async fn prepare( // where a per-request mechanism goes silently missing — an operator // who declared that this upstream reads the caller's credential got it // on every other `/v1/*` endpoint and not here. - let forwarded = aisix_gateway::ForwardedClientHeaders::resolve( + let mut forwarded = aisix_gateway::ForwardedClientHeaders::resolve( &aisix_gateway::UpstreamHeaderContext::from_overrides(pk_entry.value.request.as_ref()) .with_client_headers(headers) .with_surface_blocked(REALTIME_HANDSHAKE_SLOTS), ); + // The WebSocket client renders the handshake as text and refuses a + // header value it cannot read as a string, failing the whole upstream + // connection. Dropping the entry keeps a caller who sent one obs-text + // byte from being unable to open a session at all — on every other + // face the same header is forwarded byte-for-byte. + let dropped = forwarded.drop_non_ascii_values(); + if dropped > 0 { + tracing::debug!( + dropped, + "forwarded client headers with non-ASCII values are not sent on a realtime handshake" + ); + } let mut upstream_request = match pk_entry.value.adapter { Some(Adapter::Openai) => { @@ -1479,6 +1491,67 @@ mod tests { ); } + /// A header value the WebSocket client cannot render as text fails + /// the whole upstream connection, not just that header — so the + /// session must still open, minus the one entry. The same value is + /// forwarded byte-for-byte on every other face. + /// + /// Driven over a raw socket rather than through `connect_async`: + /// tungstenite renders the handshake as text on the way OUT too, so a + /// tungstenite client cannot send this header at all. A browser can, + /// and hyper accepts it inbound — which is exactly why the gateway + /// has to handle it. + #[tokio::test] + async fn a_non_ascii_forwarded_value_does_not_sink_the_session() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let (up_addr, handshake, _frames) = spawn_upstream().await; + let snap = snapshot_forwarding(&format!("http://{up_addr}/v1"), &["x-*"]); + let (addr, _state, _rx) = serve(snap).await; + + let mut sock = tokio::net::TcpStream::connect(addr).await.unwrap(); + // Latin-1 `José` in `x-user-name`: legal in a header value, and + // `x-*` admits it alongside the readable `x-user-jwt`. + let mut req = Vec::new(); + req.extend_from_slice(b"GET /v1/realtime?model=rt-model HTTP/1.1\r\n"); + req.extend_from_slice(format!("Host: {addr}\r\n").as_bytes()); + req.extend_from_slice(b"Upgrade: websocket\r\nConnection: Upgrade\r\n"); + req.extend_from_slice(b"Sec-WebSocket-Version: 13\r\n"); + req.extend_from_slice(b"Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n"); + req.extend_from_slice(b"Authorization: Bearer sk-caller\r\n"); + req.extend_from_slice(b"x-user-jwt: eyJraw\r\n"); + req.extend_from_slice(b"x-user-name: Jos\xe9\r\n"); + req.extend_from_slice(b"\r\n"); + sock.write_all(&req).await.unwrap(); + + let mut buf = [0u8; 256]; + let n = tokio::time::timeout(Duration::from_secs(5), sock.read(&mut buf)) + .await + .expect("the gateway must answer the upgrade") + .unwrap(); + let status = String::from_utf8_lossy(&buf[..n]).to_string(); + assert!( + status.starts_with("HTTP/1.1 101"), + "one unreadable header must not fail the session, got: {}", + status.lines().next().unwrap_or_default() + ); + + // The upstream handshake happened, carrying the readable header + // and not the other one. + let (_uri, seen) = tokio::time::timeout(Duration::from_secs(5), async { + loop { + if let Some(h) = handshake.lock().unwrap().clone() { + return h; + } + tokio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await + .expect("upstream handshake recorded"); + assert_eq!(header_str(&seen, "x-user-jwt"), "eyJraw"); + assert!(seen.get("x-user-name").is_none()); + } + /// The credential collision, on this face as on every other: the /// operator declared that this upstream reads the caller's own /// credential, so the ProviderKey's stands aside rather than joining diff --git a/schemas/resources/provider_key.schema.json b/schemas/resources/provider_key.schema.json index 8479e9602..30c282f1d 100644 --- a/schemas/resources/provider_key.schema.json +++ b/schemas/resources/provider_key.schema.json @@ -129,7 +129,7 @@ "type": "object" }, "forward_client_headers": { - "description": "Inbound client headers forwarded to the upstream, as single-`*` glob patterns matched case-insensitively against the header name (`\"anthropic-beta\"`, `\"x-trace-*\"`, `\"authorization\"`). Empty — the default — forwards nothing, which is the behavior of every standard-protocol endpoint before AISIX-Cloud#1167.\n\nA header named here reaches the upstream whatever the gateway would otherwise do with it. Naming a credential slot — `authorization`, `proxy-authorization`, `x-api-key`, `api-key`, `x-goog-api-key`, `cookie` — hands the upstream the caller's own credential in place of the one this ProviderKey would inject there, never both. That is what lets an internal service that already authorizes on the end user's `Authorization` keep doing so unchanged. Any OTHER header the gateway had already set is left alone: it selects how the exchange works, not who it is from.\n\nA credential slot, and `traceparent` / `tracestate`, are forwarded only when a pattern names them exactly — a glob such as `\"*\"` or `\"x-*\"` is a statement about the operator's own headers, not consent to hand a third party the caller's credential or to graft the caller's trace onto that party's telemetry.\n\nTwo cases where a named header still does not reach the upstream: a `default_headers` entry of the same name wins it, since both are operator configuration and the static one is the more specific choice; and on an AWS Bedrock provider six names are refused from either source. Five of them (`authorization`, `x-amz-date`, `x-amz-content-sha256`, `x-amz-security-token`, `x-amz-target`) are SigV4 inputs the request signer derives, so a supplied value would break the signature rather than authenticate anyone; the sixth, `x-amzn-bedrock-accept`, selects the response wire shape the gateway then decodes.\n\nNaming a credential slot needs a data plane new enough to honor it; an older one refuses those names outright, so the pattern has no effect there rather than a different one.\n\nHeaders whose forwarding would break the exchange rather than change who it comes from are never forwarded whatever the patterns say: `host`, the hop-by-hop headers that describe the caller's own connection, and the gateway's `x-aisix-*` namespace. The headers describing a body this gateway re-serializes or a response shape it parses (`content-type`, `content-length`, `accept`, `anthropic-version`, `x-stainless-*`) are excluded for the same reason. `traceparent` and `tracestate` are forwarded only when a pattern names them exactly — a glob is not read as consent to graft the caller's trace onto the upstream's telemetry.", + "description": "Inbound client headers forwarded to the upstream, as single-`*` glob patterns matched case-insensitively against the header name (`\"anthropic-beta\"`, `\"x-trace-*\"`, `\"authorization\"`). Empty — the default — forwards nothing, which is the behavior of every standard-protocol endpoint before AISIX-Cloud#1167.\n\nA header named here reaches the upstream whatever the gateway would otherwise do with it. Naming a credential slot — `authorization`, `proxy-authorization`, `x-api-key`, `api-key`, `x-goog-api-key`, `cookie` — hands the upstream the caller's own credential in place of the one this ProviderKey would inject there, never both. That is what lets an internal service that already authorizes on the end user's `Authorization` keep doing so unchanged. Any OTHER header the gateway had already set is left alone: it selects how the exchange works, not who it is from.\n\nA credential slot, and `traceparent` / `tracestate`, are forwarded only when a pattern names them exactly — a glob such as `\"*\"` or `\"x-*\"` is a statement about the operator's own headers, not consent to hand a third party the caller's credential or to graft the caller's trace onto that party's telemetry.\n\nTwo cases where a named header still does not reach the upstream: a `default_headers` entry of the same name wins it, since both are operator configuration and the static one is the more specific choice; and on an AWS Bedrock provider the request signer owns `authorization`, `x-amz-date`, `x-amz-content-sha256`, `x-amz-security-token`, `x-amz-target` and `x-amzn-bedrock-accept`, and drops any supplied value. A value there would not authenticate anyone: it either loses to the signer or breaks the signature.\n\nNaming a credential slot needs a data plane new enough to honor it; an older one refuses those names outright, so the pattern has no effect there rather than a different one.\n\nHeaders whose forwarding would break the exchange rather than change who it comes from are never forwarded whatever the patterns say: `host`, the hop-by-hop headers that describe the caller's own connection, and the gateway's `x-aisix-*` namespace. The headers describing a body this gateway re-serializes or a response shape it parses (`content-type`, `content-length`, `accept`, `anthropic-version`, `x-stainless-*`) are excluded for the same reason. `traceparent` and `tracestate` are forwarded only when a pattern names them exactly — a glob is not read as consent to graft the caller's trace onto the upstream's telemetry.", "items": { "type": "string" }, diff --git a/tests/e2e/src/cases/forward-client-headers-e2e.test.ts b/tests/e2e/src/cases/forward-client-headers-e2e.test.ts index c500a938e..1984e2812 100644 --- a/tests/e2e/src/cases/forward-client-headers-e2e.test.ts +++ b/tests/e2e/src/cases/forward-client-headers-e2e.test.ts @@ -338,6 +338,22 @@ describe("forward_client_headers e2e: one capability across every proxy face", ( // A route that names its OWN gateway-credential slot, and one that // names none. `x-*` is the same pattern on both: what differs is // whether the route declared a slot for it to have to leave alone. + // + // Their ProviderKey strips all three `x-` names below, which is what + // makes the assertions mean anything: a passthrough route relays + // whatever it does not strip, so a header outside the strip set + // arrives upstream whether or not a pattern matched it. + const slotPk = await seed.createProviderKey({ + display_name: "fwd-slot-pk", + api_key: GATEWAY_SECRET, + api_base: upstream.baseUrl, + strip_headers: [ + "authorization", + GATEWAY_KEY_HEADER, + "x-end-user", + CUSTOM_HEADER, + ], + }); await seed.createPassthroughRoute({ name: "fwd-route-headerkey", path_prefix: `${ROUTE_PREFIX}-headerkey`, @@ -345,7 +361,7 @@ describe("forward_client_headers e2e: one capability across every proxy face", ( auth_mode: "header_key", auth_header_name: GATEWAY_KEY_HEADER, credential_mode: "inject", - provider_key_id: routePk.id, + provider_key_id: slotPk.id, identity_header: "x-end-user", forward_client_headers: ["x-*"], }); @@ -357,7 +373,7 @@ describe("forward_client_headers e2e: one capability across every proxy face", ( anonymous_key_id: callerKey.id, source_cidrs: ["127.0.0.0/8", "::1/128"], credential_mode: "inject", - provider_key_id: routePk.id, + provider_key_id: slotPk.id, forward_client_headers: ["x-*"], }); @@ -496,7 +512,7 @@ describe("forward_client_headers e2e: one capability across every proxy face", ( "content-type": "application/json", [GATEWAY_KEY_HEADER]: CALLER_KEY, "x-end-user": "alice@example.com", - "x-allowed": "yes", + [CUSTOM_HEADER]: "recovered", }, body: JSON.stringify({ model: "gpt-4o-mini", messages: [] }), }).then((r) => bodyOf(r, "the header_key route")); @@ -504,9 +520,11 @@ describe("forward_client_headers e2e: one capability across every proxy face", ( const seen = since(mark).at(-1)!; expect(seen.headers[GATEWAY_KEY_HEADER]).toBeUndefined(); expect(seen.headers["x-end-user"]).toBeUndefined(); - // The same `x-*` still carries an ordinary header, so the two - // assertions above are the rule firing rather than a dead pattern. - expect(seen.headers["x-allowed"]).toBe("yes"); + // The SAME `x-*` recovers a stripped header that is not a slot. All + // three are in this ProviderKey's strip set, so the pattern was asked + // about each of them — without that, a header the route never strips + // arrives upstream regardless and proves nothing. + expect(seen.headers[CUSTOM_HEADER]).toBe("recovered"); }); test("a route that names no slot of its own is unchanged", async () => { @@ -521,14 +539,17 @@ describe("forward_client_headers e2e: one capability across every proxy face", ( headers: { "content-type": "application/json", [GATEWAY_KEY_HEADER]: "not-a-slot-on-this-route", - "x-allowed": "yes", + [CUSTOM_HEADER]: "recovered", }, body: JSON.stringify({ model: "gpt-4o-mini", messages: [] }), }).then((r) => bodyOf(r, "the anonymous route")); const seen = since(mark).at(-1)!; + // `x-gw-key` is in this ProviderKey's strip set, so the pattern IS + // asked about it — and answers yes, because THIS route declared no + // slot. Widen the narrowing to a global list and this line fails. expect(seen.headers[GATEWAY_KEY_HEADER]).toBe("not-a-slot-on-this-route"); - expect(seen.headers["x-allowed"]).toBe("yes"); + expect(seen.headers[CUSTOM_HEADER]).toBe("recovered"); // And the shared rule is untouched: the ProviderKey's credential // still rides alone. expect(seen.headers.authorization).toBe(`Bearer ${GATEWAY_SECRET}`); diff --git a/tests/e2e/src/cases/realtime-ws-e2e.test.ts b/tests/e2e/src/cases/realtime-ws-e2e.test.ts index eb5d6c673..7002b67c9 100644 --- a/tests/e2e/src/cases/realtime-ws-e2e.test.ts +++ b/tests/e2e/src/cases/realtime-ws-e2e.test.ts @@ -46,7 +46,13 @@ const CALLER_KEY_HASH = createHash("sha256") interface RealtimeUpstream { port: number; - handshakes: { url: string; authorization: string; openaiBeta?: string }[]; + handshakes: { + url: string; + authorization: string; + openaiBeta?: string; + /** Every header of the upstream handshake, for the forwarding case. */ + headers: Record; + }[]; frames: string[]; close(): Promise; } @@ -62,6 +68,12 @@ async function startRealtimeUpstream(): Promise { url: req.url ?? "", authorization: (req.headers.authorization as string) ?? "", openaiBeta: req.headers["openai-beta"] as string | undefined, + headers: Object.fromEntries( + Object.entries(req.headers).map(([k, v]) => [ + k.toLowerCase(), + Array.isArray(v) ? v.join(",") : String(v ?? ""), + ]), + ), }); socket.on("message", (data) => { frames.push(data.toString()); @@ -130,6 +142,25 @@ describe("realtime e2e: /v1/realtime WebSocket relay (#721)", () => { provider_key_id: pk.id, }); + // A second upstream binding on the SAME mock server whose ProviderKey + // opts into forwarding. `/v1/realtime` builds its handshake by hand + // rather than through the shared pipeline, so this is the only layer + // that proves the setting survives a real binary + etcd propagation. + const fwdPk = await seed.createProviderKey({ + display_name: "realtime-e2e-fwd-pk", + secret: "sk-upstream-realtime", + api_base: `http://127.0.0.1:${upstream.port}/v1`, + request: { + forward_client_headers: ["x-user-jwt", "sec-websocket-protocol"], + }, + }); + await seed.createModel({ + display_name: "realtime-e2e-fwd-model", + provider: "openai", + model_name: "gpt-realtime-mock", + provider_key_id: fwdPk.id, + }); + // JWT identity resolving (via a claim mapping) to a key that may NOT // use the realtime model — drives the post-auth refusal attribution // test (#932). The OTLP receiver is how that test reads the error @@ -277,6 +308,53 @@ describe("realtime e2e: /v1/realtime WebSocket relay (#721)", () => { expect(hs.openaiBeta).toBeUndefined(); }); + test("forward_client_headers reaches the upstream handshake, minus the browser credential", async (ctx) => { + if (!etcdReachable || !app || !upstream) { + ctx.skip(); + return; + } + const mark = upstream.handshakes.length; + + // A server-side client, so headers can be set. The subprotocol list + // still carries the caller's own gateway key — the browser flow's + // credential channel — and the ProviderKey names + // `sec-websocket-protocol` EXACTLY, which on every other face would + // be consent. This face refuses it outright. + const wsUrl = `${app.proxyUrl.replace("http://", "ws://")}/v1/realtime?model=realtime-e2e-fwd-model`; + const ws = new WsClient(wsUrl, [ + "realtime", + `openai-insecure-api-key.${CALLER_PLAINTEXT}`, + ], { + headers: { "x-user-jwt": "caller.jwt.value" }, + }); + await new Promise((resolve, reject) => { + ws.on("open", () => resolve()); + ws.on("error", (e: Error) => reject(e)); + }); + // The gateway accepts the client upgrade before it dials upstream, so + // `open` alone proves nothing about the upstream handshake. Driving + // one frame and awaiting the answer is what forces it. + const answered = new Promise((resolve) => { + ws.on("message", (data: Buffer) => resolve(data.toString())); + }); + ws.send(JSON.stringify({ type: "session.update", session: {} })); + expect(JSON.parse(await answered).type).toBe("response.done"); + + const seen = upstream.handshakes.slice(mark); + expect(seen.length, "the upstream handshake must have happened").toBe(1); + expect(seen[0].headers["x-user-jwt"]).toBe("caller.jwt.value"); + // Named exactly and still refused: relaying it would hand the + // provider the caller's own AISIX key, and would override the + // subprotocol the gateway negotiates. + expect(seen[0].headers["sec-websocket-protocol"] ?? "").not.toContain( + CALLER_PLAINTEXT, + ); + // The gateway's own credential still authenticates the session. + expect(seen[0].authorization).toBe("Bearer sk-upstream-realtime"); + + ws.close(); + }); + test("a plain http GET answers the envelope, not a bare rejection", async (ctx) => { if (!etcdReachable || !app) { ctx.skip();