Merge commit from fork

* Strip custom-named credential headers on cross-host redirect

The proxy channel for the messages-format provider sets a custom-named
x-api-key header carrying the operator's real upstream key. The manager's
http.Client followed redirects with no CheckRedirect; net/http strips the
standard Authorization header on a cross-host redirect but not custom-named
ones, so a cross-host 3xx from an operator-configured upstream leaked the
upstream key to the redirect target. Add a CheckRedirect policy that strips
credential headers on host change and a regression test.

Signed-off-by: tonghuaroot <tonghuaroot@gmail.com>

* Add regression test for cross-host proxy header leak

Signed-off-by: tonghuaroot <tonghuaroot@gmail.com>

---------

Signed-off-by: tonghuaroot <tonghuaroot@gmail.com>

Author: tonghuaroot

internal/httpclient/manager.go

@@ -99,14 +99,47 @@ func (m *HTTPClientManager) GetClient(config *Config) *http.Client {
 	}
 
 	newClient := &http.Client{
-		Transport: transport,
-		Timeout:   config.RequestTimeout,
+		Transport:     transport,
+		Timeout:       config.RequestTimeout,
+		CheckRedirect: stripSensitiveOnCrossHostRedirect,
 	}
 
 	m.clients[fingerprint] = newClient
 	return newClient
 }
 
+// sensitiveProxyHeaders are custom-named credential headers that proxy channels
+// attach to upstream requests (e.g. x-api-key set by the messages-format
+// channel's ModifyRequest). Unlike the standard Authorization header, net/http
+// does NOT strip these on a cross-host redirect, so without an explicit policy a
+// redirect from the operator-configured upstream to another host would leak the
+// operator's upstream key to that host (CWE-200 / CWE-522).
+var sensitiveProxyHeaders = []string{
+	"Authorization",
+	"x-api-key",
+	"api-key",
+	"X-Goog-Api-Key",
+	"X-Auth-Token",
+}
+
+// stripSensitiveOnCrossHostRedirect removes credential headers when a redirect
+// crosses to a different host, mirroring the protection net/http already gives
+// the standard Authorization header. It preserves the default redirect cap.
+func stripSensitiveOnCrossHostRedirect(req *http.Request, via []*http.Request) error {
+	if len(via) >= 10 {
+		return fmt.Errorf("stopped after 10 redirects")
+	}
+	if len(via) == 0 {
+		return nil
+	}
+	if req.URL.Hostname() != via[0].URL.Hostname() {
+		for _, h := range sensitiveProxyHeaders {
+			req.Header.Del(h)
+		}
+	}
+	return nil
+}
+
 // getFingerprint generates a unique string representation of the client configuration.
 func (c *Config) getFingerprint() string {
 	return fmt.Sprintf(

internal/httpclient/manager_test.go

@@ -0,0 +1,102 @@
+package httpclient
+
+import (
+	"context"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+// TestStripSensitiveOnCrossHostRedirect asserts that the custom-named x-api-key
+// credential header set by a proxy channel's ModifyRequest is NOT replayed to a
+// different host when the operator-configured upstream issues a cross-host
+// redirect. Regression test for the upstream-key leak (CWE-200 / CWE-522).
+func TestStripSensitiveOnCrossHostRedirect(t *testing.T) {
+	var gotAPIKey, gotAuthorization string
+
+	attacker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		gotAPIKey = r.Header.Get("x-api-key")
+		gotAuthorization = r.Header.Get("Authorization")
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer attacker.Close()
+
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, "http://attacker.local/v1/messages", http.StatusFound)
+	}))
+	defer upstream.Close()
+
+	attackerAddr := strings.TrimPrefix(attacker.URL, "http://")
+	upstreamAddr := strings.TrimPrefix(upstream.URL, "http://")
+
+	transport := &http.Transport{
+		DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
+			switch addr {
+			case "victim.local:80":
+				addr = upstreamAddr
+			case "attacker.local:80":
+				addr = attackerAddr
+			}
+			return (&net.Dialer{}).DialContext(ctx, network, addr)
+		},
+	}
+
+	client := &http.Client{
+		Transport:     transport,
+		CheckRedirect: stripSensitiveOnCrossHostRedirect,
+	}
+
+	req, _ := http.NewRequest(http.MethodPost, "http://victim.local/v1/messages", nil)
+	req.Header.Set("x-api-key", "sk-secret-upstream-key")
+	req.Header.Set("Authorization", "Bearer secret-bearer")
+
+	resp, err := client.Do(req)
+	if err != nil {
+		t.Fatalf("request failed: %v", err)
+	}
+	resp.Body.Close()
+
+	if gotAPIKey != "" {
+		t.Errorf("x-api-key leaked to cross-host redirect target: %q", gotAPIKey)
+	}
+	if gotAuthorization != "" {
+		t.Errorf("Authorization leaked to cross-host redirect target: %q", gotAuthorization)
+	}
+}
+
+// TestSensitiveHeadersPreservedSameHost asserts the policy does NOT strip the
+// credential header on a same-host redirect (legitimate behavior must survive).
+func TestSensitiveHeadersPreservedSameHost(t *testing.T) {
+	var gotAPIKey string
+	var hops int
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/redirect" {
+			hops++
+			http.Redirect(w, r, "/final", http.StatusFound)
+			return
+		}
+		gotAPIKey = r.Header.Get("x-api-key")
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer srv.Close()
+
+	client := &http.Client{CheckRedirect: stripSensitiveOnCrossHostRedirect}
+	req, _ := http.NewRequest(http.MethodGet, srv.URL+"/redirect", nil)
+	req.Header.Set("x-api-key", "sk-secret-upstream-key")
+
+	resp, err := client.Do(req)
+	if err != nil {
+		t.Fatalf("request failed: %v", err)
+	}
+	resp.Body.Close()
+
+	if hops == 0 {
+		t.Fatal("expected a same-host redirect hop")
+	}
+	if gotAPIKey != "sk-secret-upstream-key" {
+		t.Errorf("x-api-key was incorrectly stripped on same-host redirect: %q", gotAPIKey)
+	}
+}