config: strip credentials on cross-host redirects

config/headers_test.go

@@ -17,8 +17,15 @@
 package config
 
 import (
+	"fmt"
+	"io"
+	"net"
 	"net/http"
+	"net/http/httptest"
+	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestReservedHeaders(t *testing.T) {
@@ -29,3 +36,111 @@ func TestReservedHeaders(t *testing.T) {
 		}
 	}
 }
+
+func TestHeadersRoundTripperSameHost(t *testing.T) {
+	// All headers, including sensitive ones, must be forwarded on same-host requests.
+	for _, header := range []string{"Cookie", "X-Custom-Header"} {
+		t.Run(header, func(t *testing.T) {
+			received := ""
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				received = r.Header.Get(header)
+				fmt.Fprint(w, "ok")
+			}))
+			t.Cleanup(server.Close)
+
+			headers := &Headers{
+				Headers: map[string]Header{
+					header: {Values: []string{"testvalue"}},
+				},
+			}
+			rt := NewHeadersRoundTripper(headers, http.DefaultTransport)
+
+			req, err := http.NewRequest(http.MethodGet, server.URL, nil)
+			require.NoError(t, err)
+
+			resp, err := rt.RoundTrip(req)
+			require.NoError(t, err)
+			defer resp.Body.Close()
+			body, err := io.ReadAll(resp.Body)
+			require.NoError(t, err)
+			require.Equal(t, "ok", strings.TrimSpace(string(body)))
+			require.Equalf(t, "testvalue", received, "header %q must be forwarded on same-host request", header)
+		})
+	}
+}
+
+func TestHeadersRoundTripperCrossHostRedirect(t *testing.T) {
+	// Cookie must be set on the initial request but stripped on cross-host redirects.
+	cookieOnRedirect := ""
+	target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		cookieOnRedirect = r.Header.Get("Cookie")
+		fmt.Fprint(w, "ok")
+	}))
+	t.Cleanup(target.Close)
+
+	// Use "localhost" as the redirect target hostname so that it differs from
+	// "127.0.0.1" used by the origin server, making it a cross-host redirect.
+	targetPort := target.Listener.Addr().(*net.TCPAddr).Port
+	targetURL := fmt.Sprintf("http://localhost:%d", targetPort)
+
+	cookieOnOrigin := ""
+	origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		cookieOnOrigin = r.Header.Get("Cookie")
+		http.Redirect(w, r, targetURL, http.StatusFound)
+	}))
+	t.Cleanup(origin.Close)
+
+	cfg := HTTPClientConfig{
+		FollowRedirects: true,
+		HTTPHeaders: &Headers{
+			Headers: map[string]Header{
+				"Cookie": {Values: []string{"session=abc"}},
+			},
+		},
+	}
+	client, err := NewClientFromConfig(cfg, "test")
+	require.NoError(t, err)
+
+	resp, err := client.Get(origin.URL)
+	require.NoError(t, err)
+	defer resp.Body.Close()
+	_, err = io.ReadAll(resp.Body)
+	require.NoError(t, err)
+
+	require.Equalf(t, "session=abc", cookieOnOrigin, "Cookie must be set on the initial request.")
+	require.Emptyf(t, cookieOnRedirect, "Cookie must not be forwarded on a cross-host redirect.")
+}
+
+func TestHeadersRoundTripperSameHostRedirect(t *testing.T) {
+	// Cookie must be forwarded on same-host redirects.
+	mux := http.NewServeMux()
+	cookieOnRedirect := ""
+	mux.HandleFunc("/start", func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, "/end", http.StatusFound)
+	})
+	mux.HandleFunc("/end", func(w http.ResponseWriter, r *http.Request) {
+		cookieOnRedirect = r.Header.Get("Cookie")
+		fmt.Fprint(w, "ok")
+	})
+	server := httptest.NewServer(mux)
+	t.Cleanup(server.Close)
+
+	cfg := HTTPClientConfig{
+		FollowRedirects: true,
+		HTTPHeaders: &Headers{
+			Headers: map[string]Header{
+				"Cookie": {Values: []string{"session=abc"}},
+			},
+		},
+	}
+	client, err := NewClientFromConfig(cfg, "test")
+	require.NoError(t, err)
+
+	resp, err := client.Get(server.URL + "/start")
+	require.NoError(t, err)
+	defer resp.Body.Close()
+	_, err = io.ReadAll(resp.Body)
+	require.NoError(t, err)
+
+	require.Equalf(t, "session=abc", cookieOnRedirect, "Cookie must be forwarded on a same-host redirect.")
+}

config/http_config.go

@@ -721,6 +721,14 @@ func NewRoundTripperFromConfigWithContext(ctx context.Context, cfg HTTPClientCon
 		}
 
 		if cfg.HTTPHeaders != nil {
+			// Strip sensitive headers added by headersRoundTripper on cross-host
+			// redirects before they reach the transport. Only needed when
+			// redirects are actually followed; when FollowRedirects is false
+			// CheckRedirect returns ErrUseLastResponse immediately so there are
+			// no subsequent requests.
+			if cfg.FollowRedirects {
+				rt = &sensitiveHeadersStripRT{next: rt}
+			}
 			rt = NewHeadersRoundTripper(cfg.HTTPHeaders, rt)
 		}
 
@@ -862,7 +870,7 @@ func NewAuthorizationCredentialsRoundTripper(authType string, authCredentials Se
 }
 
 func (rt *authorizationCredentialsRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
-	if len(req.Header.Get("Authorization")) != 0 {
+	if len(req.Header.Get("Authorization")) != 0 || isCrossHostRedirect(req) {
 		return rt.rt.RoundTrip(req)
 	}
 
@@ -900,7 +908,7 @@ func NewBasicAuthRoundTripper(username, password SecretReader, rt http.RoundTrip
 }
 
 func (rt *basicAuthRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
-	if len(req.Header.Get("Authorization")) != 0 {
+	if len(req.Header.Get("Authorization")) != 0 || isCrossHostRedirect(req) {
 		return rt.rt.RoundTrip(req)
 	}
 	var username string
@@ -1085,6 +1093,9 @@ func (rt *oauth2RoundTripper) RoundTrip(req *http.Request) (*http.Response, erro
 	rt.mtx.RLock()
 	currentRT := rt.lastRT
 	rt.mtx.RUnlock()
+	if isCrossHostRedirect(req) {
+		return currentRT.Base.RoundTrip(req)
+	}
 	return currentRT.RoundTrip(req)
 }
 
@@ -1106,6 +1117,85 @@ func mapToValues(m map[string]string) url.Values {
 	return v
 }
 
+// isCrossHostRedirect reports whether req is a redirect to a different host
+// than the original request. It detects this by walking the req.Response chain
+// (which Go's HTTP client populates on every redirect hop) to find the original
+// request's hostname, then comparing it to the current destination.
+// This works regardless of whether the caller uses NewClientFromConfig or a
+// custom http.Client built from NewRoundTripperFromConfigWithContext directly.
+func isCrossHostRedirect(req *http.Request) bool {
+	if req.Response == nil {
+		return false
+	}
+	originalHost := strings.ToLower(originalRequestHost(req))
+	return !isDomainOrSubdomain(strings.ToLower(req.URL.Hostname()), originalHost)
+}
+
+func originalRequestHost(req *http.Request) string {
+	r := req
+	for r.Response != nil && r.Response.Request != nil {
+		r = r.Response.Request
+	}
+	return r.URL.Hostname()
+}
+
+// sensitiveHeadersOnRedirect lists the headers that must not be forwarded when
+// following a redirect to a different host, mirroring the list in
+// makeHeadersCopier in net/http/client.go.
+var sensitiveHeadersOnRedirect = map[string]struct{}{
+	"Authorization": {},
+	// "Www-Authenticate" is the canonical form produced by
+	// textproto.CanonicalMIMEHeaderKey; it is not a typo of "WWW-Authenticate".
+	"Www-Authenticate":    {},
+	"Cookie":              {},
+	"Cookie2":             {},
+	"Proxy-Authorization": {},
+	"Proxy-Authenticate":  {},
+}
+
+// sensitiveHeadersStripRT strips sensitive headers from requests marked as
+// cross-host redirects before passing them to the underlying transport.
+type sensitiveHeadersStripRT struct {
+	next http.RoundTripper
+}
+
+func (rt *sensitiveHeadersStripRT) RoundTrip(req *http.Request) (*http.Response, error) {
+	if isCrossHostRedirect(req) {
+		req = cloneRequest(req)
+		for h := range sensitiveHeadersOnRedirect {
+			req.Header.Del(h)
+		}
+	}
+	return rt.next.RoundTrip(req)
+}
+
+func (rt *sensitiveHeadersStripRT) CloseIdleConnections() {
+	if ci, ok := rt.next.(closeIdler); ok {
+		ci.CloseIdleConnections()
+	}
+}
+
+// isDomainOrSubdomain reports whether sub is a subdomain (or exact match) of
+// parent. It mirrors isDomainOrSubdomain from net/http/client.go.
+func isDomainOrSubdomain(sub, parent string) bool {
+	if parent == "" {
+		return false
+	}
+	if sub == parent {
+		return true
+	}
+	// A colon means sub is an IPv6 address; a percent sign introduces an IPv6
+	// zone ID. Neither can be a hostname, and both could otherwise pass the
+	// suffix check below (e.g. "::1%.www.example.com" ends with "example.com").
+	if strings.ContainsAny(sub, ":%") {
+		return false
+	}
+	if !strings.HasSuffix(sub, parent) {
+		return false
+	}
+	return sub[len(sub)-len(parent)-1] == '.'
+}
+
 // cloneRequest returns a clone of the provided *http.Request.
 // The clone is a shallow copy of the struct and its Header map.
 func cloneRequest(r *http.Request) *http.Request {