[dont merge me!] fix(gateway): stop following upstream redirects on dispatched requests

crates/aisix-gateway/src/lib.rs

@@ -51,6 +51,7 @@ pub use upstream_headers::{
     RESERVED_UPSTREAM_HEADERS,
 };
 pub use upstream_http::{
-    client_builder, error_with_causes, transport_error_message, UpstreamHttpConfig,
+    client_builder, dispatch_client_builder, dispatch_client_fallback, error_with_causes,
+    transport_error_message, UpstreamHttpConfig,
 };
 pub use upstream_tls::TlsSettings;

crates/aisix-gateway/src/upstream_http.rs

@@ -153,6 +153,53 @@ pub fn client_builder() -> reqwest::ClientBuilder {
     apply_tls(b, &cfg.tls)
 }
 
+/// [`client_builder`] for the clients that carry a caller's request to an
+/// AI provider, which additionally refuse to follow redirects.
+///
+/// reqwest follows up to 10 redirects by default, and the gateway had
+/// never opted out. A provider answering a dispatched POST with a `301`
+/// or `302` therefore made reqwest re-issue it as a `GET` against the
+/// `Location` host and hand back that response as the completion; a `307`
+/// or `308` replayed the prompt body there verbatim. Only
+/// `authorization`, `cookie`, and the proxy-auth headers are dropped when
+/// the hop crosses hosts, so the vendor credential schemes that do not
+/// use `authorization` — Azure's `api-key`, Anthropic's `x-api-key` —
+/// were carried to whatever host the `Location` named.
+///
+/// Nothing in the gateway was written for that: a 3xx from an upstream
+/// collapses to a 502 in `BridgeError`, the access log records the
+/// configured endpoint rather than the one that answered, and an operator
+/// who never configured the redirect target has no way to see it. Refusing
+/// the redirect turns the upstream's 3xx into the 502 the error path
+/// already describes.
+///
+/// The same reasoning covers the guardrail vendors: an inspection call
+/// POSTs the caller's prompt to an operator-configured endpoint under a
+/// vendor credential header (`Ocp-Apim-Subscription-Key`, and the rest),
+/// none of which reqwest strips on a cross-host hop either.
+///
+/// Not applied to every outbound client. The remaining ones either
+/// already refuse redirects at their own construction site (JWKS/OIDC
+/// discovery, MCP OAuth token, MCP OpenAPI tool calls, A2A) or talk to
+/// an operator's own collector, where an endpoint behind a rewrite is
+/// ordinary (telemetry, heartbeat, OTLP export).
+pub fn dispatch_client_builder() -> reqwest::ClientBuilder {
+    client_builder().redirect(reqwest::redirect::Policy::none())
+}
+
+/// The client to fall back to when [`dispatch_client_builder`] fails to
+/// build — a malformed deployment CA, say.
+///
+/// The connection settings are lost either way; what must not be lost is
+/// the redirect refusal, which `reqwest::Client::new()` would silently
+/// restore. Builds from a policy alone, which cannot fail.
+pub fn dispatch_client_fallback() -> reqwest::Client {
+    reqwest::Client::builder()
+        .redirect(reqwest::redirect::Policy::none())
+        .build()
+        .expect("a client with only a redirect policy always builds")
+}
+
 /// Layer the outbound trust decision onto a builder. Split out so the
 /// per-ProviderKey clients get byte-for-byte the same treatment as the
 /// shared one.
@@ -271,6 +318,50 @@ mod tests {
         assert!(client.is_ok(), "{:?}", client.err());
     }
 
+    /// Both dispatch clients hand a 3xx back to the caller instead of
+    /// following it — including the fallback, which is reached when the
+    /// deployment's TLS material fails to apply and which
+    /// `reqwest::Client::new()` would have quietly restored to
+    /// following-by-default.
+    #[tokio::test]
+    async fn dispatch_clients_hand_back_a_redirect_instead_of_following_it() {
+        use std::io::{Read, Write};
+
+        let listener = std::net::TcpListener::bind(("127.0.0.1", 0)).expect("bind");
+        let addr = listener.local_addr().expect("addr");
+        let server = std::thread::spawn(move || {
+            for _ in 0..2 {
+                let (mut socket, _) = listener.accept().expect("accept");
+                let mut buf = [0u8; 1024];
+                let _ = socket.read(&mut buf);
+                let _ = socket.write_all(
+                    b"HTTP/1.1 301 Moved Permanently\r\n\
+                      Location: http://127.0.0.1:1/elsewhere\r\n\
+                      Content-Length: 0\r\nConnection: close\r\n\r\n",
+                );
+            }
+        });
+
+        for (label, client) in [
+            (
+                "dispatch_client_builder",
+                dispatch_client_builder().build().expect("builds"),
+            ),
+            ("dispatch_client_fallback", dispatch_client_fallback()),
+        ] {
+            let res = client
+                .post(format!("http://{addr}/v1/chat/completions"))
+                .body("{}")
+                .send()
+                .await
+                .unwrap_or_else(|e| panic!("{label}: {e}"));
+            // Following it would have dialed port 1, where nothing
+            // listens, and surfaced as a transport error instead.
+            assert_eq!(res.status(), 301, "{label} followed the redirect");
+        }
+        server.join().expect("server thread");
+    }
+
     /// Every outbound HTTP client in the workspace must be built from
     /// [`client_builder`], or it silently keeps reqwest's defaults — no
     /// connect timeout, TCP keepalive off, and a 90s pooled-connection
@@ -302,9 +393,16 @@ mod tests {
                 if sanctioned_rmcp_site && line.contains("rmcp_reqwest::Client::") {
                     continue;
                 }
-                if line.contains("reqwest::Client::builder()")
-                    || line.contains("reqwest::Client::new()")
-                {
+                // Prose about a constructor is not a call to one.
+                if line.trim_start().starts_with("//") {
+                    continue;
+                }
+                // `Client::new()` unqualified, too: every one of these
+                // files imports the type, and the fallback arm of a
+                // failed build is where a bare client hides
+                // (`.unwrap_or_else(|_| Client::new())` gives back
+                // reqwest's defaults, redirect following included).
+                if line.contains("reqwest::Client::builder()") || line.contains("Client::new()") {
                     offenders.push(format!("{}:{}", file.display(), n + 1));
                 }
             }
@@ -460,6 +558,117 @@ mod tests {
         );
     }
 
+    /// A client that carries a caller's request to a provider must be
+    /// built from [`dispatch_client_builder`], so an upstream 3xx becomes
+    /// the 502 the error path describes instead of a silent hop to
+    /// whatever host the `Location` named.
+    ///
+    /// Stated as a whitelist rather than a pattern: **every**
+    /// `client_builder()` site in the workspace either builds a dispatch
+    /// client or is named below. A rule shaped the other way — "files
+    /// that look like a bridge must use the dispatch builder" — passes
+    /// silently for a client put in `src/client.rs`, or in
+    /// `src/bridge/mod.rs`, or in a surface nobody thought of, which is
+    /// how the guardrail clients were missed the first time.
+    ///
+    /// Adding an outbound client therefore forces a decision here, and
+    /// the decision it forces is the safe-by-default one.
+    #[test]
+    fn every_outbound_client_is_classified_for_redirects() {
+        /// Sites that build a client from [`client_builder`] and are
+        /// *not* dispatch, with why a redirect there is not the same
+        /// question. Everything else must use
+        /// [`dispatch_client_builder`].
+        const NON_DISPATCH: &[(&str, &str)] = &[
+            (
+                "aisix-mcp/src/oauth.rs",
+                "sets `Policy::none()` itself; an OAuth token endpoint never \
+                 legitimately redirects",
+            ),
+            (
+                "aisix-mcp/src/openapi.rs",
+                "sets `Policy::none()` itself, for the generated tool calls",
+            ),
+            (
+                "aisix-proxy/src/jwt.rs",
+                "sets `Policy::none()` itself; a JWKS endpoint never \
+                 legitimately redirects",
+            ),
+            (
+                "aisix-a2a/src/bridge.rs",
+                "sets `Policy::none()` itself; an A2A agent does not redirect \
+                 its JSON-RPC endpoint",
+            ),
+            (
+                "aisix-obs/src/otlp_http_sink.rs",
+                "the operator's own collector; an endpoint behind a rewrite is \
+                 ordinary and carries no vendor credential of ours",
+            ),
+            (
+                "aisix-server/src/heartbeat.rs",
+                "the control plane the deployment is registered with",
+            ),
+            (
+                "aisix-server/src/telemetry.rs",
+                "the control plane the deployment is registered with",
+            ),
+        ];
+
+        let crates_dir = std::path::Path::new(concat!(env!("CARGO_MANIFEST_DIR"), "/.."));
+        let mut dispatch_sites = 0;
+        let mut classified = std::collections::HashSet::new();
+        let mut offenders = Vec::new();
+        for file in rust_sources(crates_dir) {
+            // This module defines both builders.
+            if file.ends_with("upstream_http.rs") {
+                continue;
+            }
+            let path = file.to_string_lossy().replace('\\', "/");
+            let src = std::fs::read_to_string(&file).expect("read source");
+            for (n, line) in production_half(&src).lines().enumerate() {
+                // Prose about the builders is not a call to one.
+                if !line.contains("client_builder()") || line.trim_start().starts_with("//") {
+                    continue;
+                }
+                if line.contains("dispatch_client_builder()") {
+                    dispatch_sites += 1;
+                } else if let Some((named, _why)) =
+                    NON_DISPATCH.iter().find(|(f, _)| path.ends_with(f))
+                {
+                    classified.insert(*named);
+                } else {
+                    offenders.push(format!("{}:{}", file.display(), n + 1));
+                }
+            }
+        }
+
+        assert!(
+            offenders.is_empty(),
+            "these build an outbound client that follows redirects. If it \
+             carries a caller's payload or a gateway-held credential, build \
+             it from `dispatch_client_builder()`; if a redirect there is \
+             genuinely ordinary, add it to NON_DISPATCH with the reason:\n{}",
+            offenders.join("\n"),
+        );
+        assert!(
+            dispatch_sites >= 13,
+            "found {dispatch_sites} dispatch client construction sites, \
+             expected at least 13 — the probe no longer matches the code and \
+             this test proves nothing",
+        );
+        let stale: Vec<_> = NON_DISPATCH
+            .iter()
+            .map(|(f, _)| *f)
+            .filter(|f| !classified.contains(f))
+            .collect();
+        assert!(
+            stale.is_empty(),
+            "these NON_DISPATCH entries no longer match a client_builder() \
+             site; drop them so the list keeps meaning something:\n{}",
+            stale.join("\n"),
+        );
+    }
+
     fn rust_sources(dir: &std::path::Path) -> Vec<std::path::PathBuf> {
         let mut out = Vec::new();
         let Ok(entries) = std::fs::read_dir(dir) else {

crates/aisix-gateway/src/upstream_tls.rs

@@ -257,7 +257,7 @@ fn worker_client() -> Option<reqwest::Client> {
     }
     WORKER_CLIENT.with(|cell| {
         cell.get_or_init(|| {
-            match crate::upstream_http::client_builder()
+            match crate::upstream_http::dispatch_client_builder()
                 .user_agent(DISPATCH_USER_AGENT)
                 .build()
             {
@@ -321,7 +321,8 @@ fn build_provider_key_client(tls: &ProviderKeyTls) -> Result<reqwest::Client, St
     // than replacing them: a deployment CA and a per-key CA are both
     // trust roots, and a client presenting the deployment's mTLS
     // identity must keep presenting it.
-    let mut builder = crate::upstream_http::client_builder().user_agent(PROVIDER_KEY_USER_AGENT);
+    let mut builder =
+        crate::upstream_http::dispatch_client_builder().user_agent(PROVIDER_KEY_USER_AGENT);
     if let Some(pem) = tls.ca_cert.as_ref().filter(|p| !p.trim().is_empty()) {
         let roots = reqwest::Certificate::from_pem_bundle(pem.as_bytes())
             .map_err(|e| format!("provider_key.tls.ca_cert: {e}"))?;

crates/aisix-guardrails/src/aliyun.rs

@@ -106,7 +106,7 @@ impl AliyunTextModerationGuardrail {
         // Same connection-layer settings as every provider call: a bound
         // connect phase, TCP keepalive on, and pooled connections expired
         // before a hop in front of the guardrail service reaps them.
-        let client = aisix_gateway::client_builder()
+        let client = aisix_gateway::dispatch_client_builder()
             .build()
             .expect("guardrail http client builds");
         let endpoint = cfg

crates/aisix-guardrails/src/aliyun_ai_guardrail.rs

@@ -128,7 +128,7 @@ impl AliyunAiGuardrail {
         // Same connection-layer settings as every provider call: a bound
         // connect phase, TCP keepalive on, and pooled connections expired
         // before a hop in front of the guardrail service reaps them.
-        let client = aisix_gateway::client_builder()
+        let client = aisix_gateway::dispatch_client_builder()
             .build()
             .expect("guardrail http client builds");
         let endpoint = cfg

crates/aisix-guardrails/src/lakera.rs

@@ -101,7 +101,7 @@ impl LakeraGuardrail {
         // Same connection-layer settings as every provider call: a bound
         // connect phase, TCP keepalive on, and pooled connections expired
         // before a hop in front of the guardrail service reaps them.
-        let client = aisix_gateway::client_builder()
+        let client = aisix_gateway::dispatch_client_builder()
             .build()
             .expect("guardrail http client builds");
         Self {

crates/aisix-guardrails/src/openai_moderation.rs

@@ -97,7 +97,7 @@ impl OpenaiModerationGuardrail {
         // Same connection-layer settings as every provider call: a bound
         // connect phase, TCP keepalive on, and pooled connections expired
         // before a hop in front of the guardrail service reaps them.
-        let client = aisix_gateway::client_builder()
+        let client = aisix_gateway::dispatch_client_builder()
             .build()
             .expect("guardrail http client builds");
         Self {

crates/aisix-guardrails/src/presidio.rs

@@ -126,7 +126,7 @@ impl PresidioGuardrail {
         // Same connection-layer settings as every provider call: a bound
         // connect phase, TCP keepalive on, and pooled connections expired
         // before a hop in front of the guardrail service reaps them.
-        let client = aisix_gateway::client_builder()
+        let client = aisix_gateway::dispatch_client_builder()
             .build()
             .expect("guardrail http client builds");
         Self {

crates/aisix-guardrails/src/prompt_shield.rs

@@ -101,7 +101,7 @@ impl PromptShieldGuardrail {
         // Per-call timeout is enforced via tokio::time::timeout in
         // call_api(); the connection layer is the shared one, so a pooled
         // connection expires before a hop in front of the service reaps it.
-        let client = aisix_gateway::client_builder()
+        let client = aisix_gateway::dispatch_client_builder()
             .build()
             .expect("guardrail http client builds");
         Self {

crates/aisix-guardrails/src/text_moderation.rs

@@ -100,7 +100,7 @@ impl TextModerationGuardrail {
         // Same connection-layer settings as every provider call: a bound
         // connect phase, TCP keepalive on, and pooled connections expired
         // before a hop in front of the guardrail service reaps them.
-        let client = aisix_gateway::client_builder()
+        let client = aisix_gateway::dispatch_client_builder()
             .build()
             .expect("guardrail http client builds");
         Self {

crates/aisix-mcp/src/bridge.rs

@@ -58,7 +58,12 @@ pub const DEFAULT_UPSTREAM_TIMEOUT: Duration = Duration::from_secs(30);
 ///   MCP server behind an enterprise CA is exactly as common as a model
 ///   endpoint behind one, and the PEM has to be re-parsed rather than
 ///   reused because rmcp's `Certificate` is a different crate version's
-///   type than the one `upstream_tls` caches for the workspace line.
+///   type than the one `upstream_tls` caches for the workspace line;
+/// - redirects are refused, as they are on every other client that
+///   carries a caller's payload under a gateway-held credential. A
+///   `tools/call` POSTs the caller's arguments under the MCP server's
+///   `x-api-key` or bearer, and reqwest does not strip either on a hop
+///   to whatever host a `Location` names.
 ///
 /// One client for all MCP upstreams = one shared pool, matching how the
 /// provider bridges share theirs (auth is injected per-request by the
@@ -69,6 +74,7 @@ fn shared_http_client() -> rmcp_reqwest::Client {
         .get_or_init(|| {
             let cfg = aisix_gateway::upstream_http::config();
             let mut b = rmcp_reqwest::Client::builder()
+                .redirect(rmcp_reqwest::redirect::Policy::none())
                 .pool_idle_timeout(cfg.pool_idle_timeout)
                 .tcp_keepalive(cfg.tcp_keepalive);
             if let Some(d) = cfg.connect_timeout {
@@ -112,7 +118,15 @@ fn shared_http_client() -> rmcp_reqwest::Client {
             if !cfg.tls.verify {
                 b = b.danger_accept_invalid_certs(true);
             }
-            b.build().unwrap_or_else(|_| rmcp_reqwest::Client::new())
+            // The connection settings are lost if the build fails; the
+            // redirect refusal must not be, which `Client::new()` would
+            // silently give back.
+            b.build().unwrap_or_else(|_| {
+                rmcp_reqwest::Client::builder()
+                    .redirect(rmcp_reqwest::redirect::Policy::none())
+                    .build()
+                    .expect("a client with only a redirect policy always builds")
+            })
         })
         .clone()
 }

crates/aisix-provider-anthropic/src/bridge.rs

@@ -78,10 +78,10 @@ impl Default for AnthropicBridge {
 }
 
 fn default_client() -> Client {
-    aisix_gateway::client_builder()
+    aisix_gateway::dispatch_client_builder()
         .user_agent("aisix/0.1")
         .build()
-        .unwrap_or_else(|_| Client::new())
+        .unwrap_or_else(|_| aisix_gateway::dispatch_client_fallback())
 }
 
 /// Path suffixes the Anthropic bridge appends. If an operator

crates/aisix-provider-azure-openai/src/bridge.rs

@@ -186,10 +186,10 @@ impl Default for AzureOpenAiBridge {
 }
 
 fn default_client() -> Client {
-    aisix_gateway::client_builder()
+    aisix_gateway::dispatch_client_builder()
         .user_agent("aisix/0.1")
         .build()
-        .unwrap_or_else(|_| Client::new())
+        .unwrap_or_else(|_| aisix_gateway::dispatch_client_fallback())
 }
 
 /// Parsed Azure upstream reference resolved from a provider_key's