From 5ba5d7e73abe05a46bdbccf5ff6d9fe5790333c8 Mon Sep 17 00:00:00 2001 From: Sai Asish Y Date: Tue, 19 May 2026 15:04:34 -0700 Subject: [PATCH] fix(http): skip AuthTransport bearer on redirected hops AuthTransport.RoundTrip unconditionally re-set the Authorization header, which clobbered stdlib http.Client's cross-host strip on artifact-download redirects to presigned buildkiteartifacts.com / S3 URLs. The presigned URL already carries its own auth via X-Amz-Signature, so the extra header either leaks the bearer or breaks the download with S3's 'Only one auth mechanism allowed' 400. Closes #830 Signed-off-by: Sai Asish Y --- internal/http/refresh_transport.go | 9 ++++-- internal/http/refresh_transport_test.go | 38 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/internal/http/refresh_transport.go b/internal/http/refresh_transport.go index 34f67ca8..ef6bf1e2 100644 --- a/internal/http/refresh_transport.go +++ b/internal/http/refresh_transport.go @@ -52,9 +52,12 @@ type AuthTransport struct { } func (t *AuthTransport) RoundTrip(req *http.Request) (*http.Response, error) { - token := t.TokenSource.Token() - if token != "" { - req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + // Skip re-injecting on redirected hops so stdlib's cross-host strip stays in effect. + if req.Response == nil { + token := t.TokenSource.Token() + if token != "" { + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + } } if t.UserAgent != "" { req.Header.Set("User-Agent", t.UserAgent) diff --git a/internal/http/refresh_transport_test.go b/internal/http/refresh_transport_test.go index 9721505e..19770cd5 100644 --- a/internal/http/refresh_transport_test.go +++ b/internal/http/refresh_transport_test.go @@ -353,3 +353,41 @@ func TestIsTerminalRefreshError(t *testing.T) { } } } + +func TestAuthTransport_DoesNotInjectOnRedirectedHop(t *testing.T) { + t.Parallel() + + var presignedAuth atomic.Value + presignedAuth.Store("") + + presigned := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + presignedAuth.Store(r.Header.Get("Authorization")) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("payload")) + })) + defer presigned.Close() + + api := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, presigned.URL+"/blob?X-Amz-Signature=stub", http.StatusFound) + })) + defer api.Close() + + ts := NewTokenSource("bk-token") + transport := &AuthTransport{Base: http.DefaultTransport, TokenSource: ts} + + client := &http.Client{Transport: transport} + req, err := http.NewRequest(http.MethodGet, api.URL+"/artifact", nil) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + resp, err := client.Do(req) + if err != nil { + t.Fatalf("Do: %v", err) + } + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + + if got := presignedAuth.Load().(string); got != "" { + t.Fatalf("Authorization leaked to presigned host: %q", got) + } +}