Provider HTTP clients follow redirects to subdomains and other ports with the key attached #1896

What happened

The HTTP clients that carry the configured provider key are built with no CheckRedirect, so they use Go's default redirect policy. That policy forwards Authorization when a redirect stays on the same host, including a different port, or moves to a subdomain of it. Go strips the header only for an unrelated domain.

Observed on 2026-10-22 against dev@d4cef55fa (go1.26.5) with the stub test below. The completion answers 307 to a second stub; every *.test name dials loopback:

=== RUN   TestIssueReproRedirectForwardsKey/evil.api.repro.test
    configured host api.repro.test:41324 redirected to evil.api.repro.test:41618 -> received evil.api.repro.test:41618 Authorization="Bearer sk-or-REPRO-KEY" (err=<nil>)
=== RUN   TestIssueReproRedirectForwardsKey/api.repro.test
    configured host api.repro.test:40818 redirected to api.repro.test:41496 -> received api.repro.test:41496 Authorization="Bearer sk-or-REPRO-KEY" (err=<nil>)
=== RUN   TestIssueReproRedirectForwardsKey/other-vendor.test
    configured host api.repro.test:41420 redirected to other-vendor.test:41452 -> received other-vendor.test:41452 Authorization="" (err=<nil>)

The request also completes at the redirect target, so nothing tells the person the call went elsewhere. Found during the #1894 credential audit and listed there as a follow-up. It has not been seen against a real provider: it needs a service, or something in front of it, that answers an API call with a redirect.

Replication

Deterministic (no model). Save the test below as internal/provider/zz_issue_repro_test.go and run:

go test -count=1 -run TestIssueReproRedirectForwardsKey -v ./internal/provider/

Today the subdomain and different-port cases log Authorization="Bearer sk-or-REPRO-KEY" at the redirect target; only the unrelated domain logs "".

package provider

import (
	"context"
	"fmt"
	"net"
	"net/http"
	"net/http/httptest"
	"strings"
	"testing"
)

func TestIssueReproRedirectForwardsKey(t *testing.T) {
	for _, target := range []string{"evil.api.repro.test", "api.repro.test", "other-vendor.test"} {
		t.Run(target, func(t *testing.T) {
			seen := make(chan string, 4)
			second := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
				seen <- r.Host + " Authorization=" + fmt.Sprintf("%q", r.Header.Get("Authorization"))
				w.Header().Set("Content-Type", "application/json")
				fmt.Fprint(w, `{"id":"gen-1","choices":[{"message":{"role":"assistant","content":"ok"},"finish_reason":"stop"}],"usage":{"prompt_tokens":1,"completion_tokens":1,"total_tokens":2}}`)
			}))
			defer second.Close()
			_, secondPort, _ := net.SplitHostPort(strings.TrimPrefix(second.URL, "http://"))
			first := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
				http.Redirect(w, r, "http://"+target+":"+secondPort+r.URL.Path, http.StatusTemporaryRedirect)
			}))
			defer first.Close()
			_, firstPort, _ := net.SplitHostPort(strings.TrimPrefix(first.URL, "http://"))
			// Every *.test name dials loopback; the client has no CheckRedirect, as in NewClient.
			transport := &http.Transport{DialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
				_, port, _ := net.SplitHostPort(address)
				return (&net.Dialer{}).DialContext(ctx, network, net.JoinHostPort("127.0.0.1", port))
			}}
			client, err := NewClient(Config{APIKey: "sk-or-REPRO-KEY", BaseURL: "http://api.repro.test:" + firstPort + "/api/v1",
				Model: "repro/model", HTTPClient: &http.Client{Transport: transport}})
			if err != nil {
				t.Fatal(err)
			}
			_, err = client.CompleteWithMessages(context.Background(), userMessages("hello"))
			select {
			case got := <-seen:
				t.Logf("configured host api.repro.test:%s redirected to %s:%s -> received %s (err=%v)", firstPort, target, secondPort, got, err)
			default:
				t.Logf("redirect target %s never asked; err=%v", target, err)
			}
		})
	}
}

The test passes its own HTTPClient only to map hostnames to loopback. Like NewClient's default (&http.Client{Transport: SharedTransport(), Timeout: ...}), it sets no CheckRedirect, so the redirect policy under test is the product's.

Field (real models). Not observed. It needs a provider, proxy or misconfigured custom endpoint that answers an API call with a 3xx to a subdomain or another port of its own host.

Where

Clients that send the configured key and keep the default redirect policy:

• NewClient in internal/provider/client.go: search httpClient = &http.Client{Transport: SharedTransport(). This covers both the request client and the stream client.
• NewMediaClient in internal/provider/media.go: search client = &http.Client{Transport: SharedTransport(), Timeout: config.Timeout}.
• The lane sheet fetcher in internal/provider/lanes.go, service probes and model listings in internal/config/sources.go, and the catalog fetch in internal/catalog/catalog.go: search each file for &http.Client{.

Already safe, for reference: the credential-free connectivity probe in internal/provider/connectivity.go (search ErrUseLastResponse), internal/update/client.go and internal/seniordev/tool/web_common.go each set their own CheckRedirect.

The fix

One shared redirect rule for every client that carries a provider key. Follow a redirect only when its scheme, host and port all equal the original request's. Stop at any other redirect and return the 3xx as an error naming the target host, rather than following it with or without the key.

Stopping, rather than following without the key, keeps the outcome visible: a silently stripped header would turn into a confusing 401 at a host the person never configured. Same-origin redirects keep working.

Acceptance

• e2e: through the real binary against a stub whose /chat/completions answers 307 to a second stub on another port. The second stub records no request at all, and the run fails with an error naming the redirect target's host.
• e2e: the control: a same-origin 307 (same scheme, host and port, different path) is still followed, and the run answers normally.
• Unit: the test above, asserted: the subdomain and different-port targets receive no request, and the same-origin case is followed with the bearer.
• Law: a go/ast law, so it rides the laws gate, that every http.Client constructed in internal/provider, internal/config and internal/catalog sets CheckRedirect through the shared rule.
• The change entry's invalidates says provider keys previously followed same-host and subdomain redirects.