fix(cli): refuse cross-origin redirects so credentials stay with the configured server

.changesets/1792715944-523a54e4.yaml

@@ -0,0 +1,10 @@
+id: 1792715944-523a54e4
+features:
+    - core
+targets:
+    - cli
+type: fix
+bump: patch
+description: refuse cross-origin redirects so credentials are only sent to the configured server
+author: TristanSpeakEasy
+date: "2026-10-23"

templates/perms.go

@@ -24,6 +24,7 @@ var fileMode = map[string]fs.FileMode{
 	"templates/cli/auxiliary/internal":                                                                                  0o20000000755,
 	"templates/cli/auxiliary/internal/client":                                                                           0o20000000755,
 	"templates/cli/auxiliary/internal/client/client.go.stmpl":                                                           0o644,
+	"templates/cli/auxiliary/internal/client/client_test.go.stmpl":                                                      0o644,
 	"templates/cli/auxiliary/internal/client/diagnostics.go.stmpl":                                                      0o644,
 	"templates/cli/auxiliary/internal/clierrors":                                                                        0o20000000755,
 	"templates/cli/auxiliary/internal/clierrors/clierrors.go.stmpl":                                                     0o644,

templates/templates/cli/auxiliary/internal/client/client.go.stmpl

@@ -6,6 +6,9 @@
 {{- addImport "internal/config" true -}}
 {{- addImport "net" -}}
 {{- addImport "net/http" -}}
+{{- addImport "net/url" -}}
+{{- addImport "errors" -}}
+{{- addImport "strings" -}}
 {{- addImport "sync" -}}
 {{- addImport "time" -}}
 {{- addImport "fmt" -}}
@@ -83,7 +86,7 @@ func NewClient(cmd *cobra.Command, allowedSecurityFields ...string) (*sdk.{{temp
 
     // Diagnostics and test client composition.
     // Order: test client (innermost) → diagnostics wrapper (outermost).
-    var httpClient HTTPClient = &http.Client{Transport: newPhaseBoundedTransport(cmd)}
+    var httpClient HTTPClient = &redirectGuardClient{&http.Client{Transport: newPhaseBoundedTransport(cmd), CheckRedirect: SameOriginRedirect}}
     if testClient := testclient.NewTestHTTPClient(); testClient != nil {
         httpClient = testClient
     }
@@ -97,6 +100,86 @@ func NewClient(cmd *cobra.Command, allowedSecurityFields ...string) (*sdk.{{temp
     {{- end}}
 }
 
+// SameOriginRedirect refuses redirects to another scheme or host, and
+// redirects that change the request method: net/http forwards custom
+// credential headers (API keys) across origins and turns a POST answered with
+// 301/302/303 into a bodiless GET.
+func SameOriginRedirect(req *http.Request, via []*http.Request) error {
+    if len(via) >= 10 {
+        return fmt.Errorf("stopped after 10 redirects")
+    }
+    from := via[0].URL
+    if urlOrigin(req.URL) != urlOrigin(from) {
+        return &RedirectRefusedError{message: fmt.Sprintf("refusing to follow a redirect from %s://%s to %s://%s: credentials are only sent to the configured server", from.Scheme, from.Host, req.URL.Scheme, req.URL.Host)}
+    }
+    if prev := via[len(via)-1]; req.Method != prev.Method {
+        return &RedirectRefusedError{message: fmt.Sprintf("refusing to follow a redirect to %s that would send %s instead of %s: the server tried to convert the request", req.URL.Path, req.Method, prev.Method)}
+    }
+    return nil
+}
+
+// RedirectRefusedError is returned for a redirect the CLI declined to follow.
+type RedirectRefusedError struct {
+    message string
+}
+
+func (e *RedirectRefusedError) Error() string { return e.message }
+
+func (e *RedirectRefusedError) CLIRedirectRefused() bool { return true }
+
+func (e *RedirectRefusedError) CLIHints() []string {
+    return []string{"Point --server-url at the endpoint the server redirects to, so the request needs no redirect"}
+}
+
+func (e *RedirectRefusedError) CLIOmitTypeHints() bool { return true }
+
+// redirectGuardClient strips the query, fragment and userinfo from the
+// Location net/http attaches to a refused redirect, so tokens in it are never
+// printed in error or debug output.
+type redirectGuardClient struct {
+    *http.Client
+}
+
+func (c *redirectGuardClient) Do(req *http.Request) (*http.Response, error) {
+    res, err := c.Client.Do(req)
+    var urlErr *url.Error
+    var refused *RedirectRefusedError
+    if errors.As(err, &urlErr) && errors.As(err, &refused) {
+        if u, parseErr := url.Parse(urlErr.URL); parseErr == nil {
+            urlErr.URL = (&url.URL{Scheme: u.Scheme, Host: u.Host, Path: u.Path}).String()
+        } else {
+            urlErr.URL = "<redirect target>"
+        }
+    }
+    return res, err
+}
+
+func urlOrigin(u *url.URL) string {
+    scheme := strings.ToLower(u.Scheme)
+    port := u.Port()
+    if port == "" {
+        switch scheme {
+        case "https":
+            port = "443"
+        case "http":
+            port = "80"
+        }
+    }
+    return scheme + "://" + net.JoinHostPort(asciiLower(u.Hostname()), port)
+}
+
+// asciiLower folds only A-Z. Unicode case folding would equate hosts the
+// transport dials as different IDNA names (U+0130 "İ" and "i").
+func asciiLower(s string) string {
+    b := []byte(s)
+    for i, c := range b {
+        if 'A' <= c && c <= 'Z' {
+            b[i] = c + ('a' - 'A')
+        }
+    }
+    return string(b)
+}
+
 func newPhaseBoundedTransport(cmd *cobra.Command) http.RoundTripper {
     var phase time.Duration
     if s := resolveStringFlag(cmd, "timeout"); s != "" {

templates/templates/cli/auxiliary/internal/client/client_test.go.stmpl

@@ -0,0 +1,203 @@
+package client
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/spf13/cobra"
+
+	"{{getGolangPackage}}/internal/output"
+)
+
+func TestSameOriginRedirect(t *testing.T) {
+	var leakedKey, landedMethod, landedBody string
+	other := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		leakedKey = r.Header.Get("x-api-key")
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer other.Close()
+
+	origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/cross":
+			http.Redirect(w, r, other.URL+"/landed", http.StatusTemporaryRedirect)
+		case "/same":
+			http.Redirect(w, r, "/landed", http.StatusTemporaryRedirect)
+		case "/convert":
+			http.Redirect(w, r, "/landed", http.StatusSeeOther)
+		default:
+			body, _ := io.ReadAll(r.Body)
+			landedMethod, landedBody = r.Method, string(body)
+			w.WriteHeader(http.StatusOK)
+		}
+	}))
+	defer origin.Close()
+
+	httpClient := &http.Client{CheckRedirect: SameOriginRedirect}
+	do := func(path string) (*http.Response, error) {
+		req, err := http.NewRequest(http.MethodPost, origin.URL+path, strings.NewReader("{}"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		req.Header.Set("x-api-key", "secret")
+		return httpClient.Do(req)
+	}
+
+	res, err := do("/same")
+	if err != nil {
+		t.Fatalf("same-origin redirect: %v", err)
+	}
+	_ = res.Body.Close()
+	if res.StatusCode != http.StatusOK {
+		t.Fatalf("same-origin redirect status = %d", res.StatusCode)
+	}
+	if landedMethod != http.MethodPost || landedBody != "{}" {
+		t.Fatalf("307 redirect landed as %s with body %q", landedMethod, landedBody)
+	}
+
+	landedMethod = ""
+	_, err = do("/convert")
+	if err == nil || !strings.Contains(err.Error(), "the server tried to convert the request") {
+		t.Fatalf("method-changing redirect error = %v", err)
+	}
+	if landedMethod != "" {
+		t.Fatalf("method-changing redirect reached the target as %s", landedMethod)
+	}
+
+	_, err = do("/cross")
+	if err == nil || !strings.Contains(err.Error(), "refusing to follow a redirect") {
+		t.Fatalf("cross-origin redirect error = %v", err)
+	}
+	if leakedKey != "" {
+		t.Fatalf("credential header reached the redirect target: %q", leakedKey)
+	}
+	var refused *RedirectRefusedError
+	if !errors.As(err, &refused) {
+		t.Fatalf("cross-origin redirect error is %T, want *RedirectRefusedError", err)
+	}
+
+	c := output.Classify(&cobra.Command{}, err)
+	if c.Type != output.ErrorTypeProtocol {
+		t.Fatalf("refused redirect classified as %s", c.Type)
+	}
+	for _, hint := range c.Hints {
+		if strings.Contains(hint, "network connectivity") {
+			t.Fatalf("refused redirect carries connection hints: %v", c.Hints)
+		}
+	}
+}
+
+func TestSameOriginRedirectOrigins(t *testing.T) {
+	for _, tc := range []struct {
+		from, to string
+		allowed  bool
+	}{
+		{"https://api.example.com/a", "https://api.example.com/b", true},
+		{"https://api.example.com/a", "https://API.example.com:443/b", true},
+		{"http://api.example.com:80/a", "http://api.example.com/b", true},
+		{"https://api.example.com/a", "http://api.example.com/b", false},
+		{"https://api.example.com/a", "https://api.example.com:8443/b", false},
+		{"https://api.example.com/a", "https://other.example.com/b", false},
+		{"http://i.example/a", "http://\u0130.example/b", false},
+		{"http://\u0130.example/a", "http://\u0130.example/b", true},
+	} {
+		from, err := http.NewRequest(http.MethodGet, tc.from, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		to, err := http.NewRequest(http.MethodGet, tc.to, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		err = SameOriginRedirect(to, []*http.Request{from})
+		if tc.allowed && err != nil {
+			t.Errorf("%s -> %s: unexpected error %v", tc.from, tc.to, err)
+		}
+		if !tc.allowed && err == nil {
+			t.Errorf("%s -> %s: redirect was allowed", tc.from, tc.to)
+		}
+	}
+}
+
+func TestSameOriginRedirectNeverDialsIDNAVariant(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/start" {
+			w.Header()["Location"] = []string{"http://\u0130.example/landed"}
+			w.WriteHeader(http.StatusTemporaryRedirect)
+			return
+		}
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer server.Close()
+
+	var dialed []string
+	dialer := &net.Dialer{}
+	transport := &http.Transport{DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
+		dialed = append(dialed, addr)
+		return dialer.DialContext(ctx, network, server.Listener.Addr().String())
+	}}
+	defer transport.CloseIdleConnections()
+	httpClient := &redirectGuardClient{&http.Client{Transport: transport, CheckRedirect: SameOriginRedirect}}
+
+	req, err := http.NewRequest(http.MethodPost, "http://i.example/start", strings.NewReader("{}"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	req.Header.Set("x-api-key", "secret")
+	_, err = httpClient.Do(req)
+	var refused *RedirectRefusedError
+	if !errors.As(err, &refused) {
+		t.Fatalf("redirect to an IDNA variant host was not refused: %v", err)
+	}
+	if len(dialed) != 1 || dialed[0] != "i.example:80" {
+		t.Fatalf("dialed %v, want only i.example:80", dialed)
+	}
+}
+
+func TestRefusedRedirectRedactsLocation(t *testing.T) {
+	const secret = "synthetic-secret"
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/convert" {
+			http.Redirect(w, r, "https://user:"+secret+"@"+r.Host+"/landed?access_token="+secret+"#"+secret, http.StatusSeeOther)
+			return
+		}
+		http.Redirect(w, r, "/landed?access_token="+secret+"#"+secret, http.StatusSeeOther)
+	}))
+	defer server.Close()
+
+	for _, path := range []string{"/same-origin", "/convert"} {
+		var debug bytes.Buffer
+		httpClient := &DebugClient{
+			Inner:  &redirectGuardClient{&http.Client{CheckRedirect: SameOriginRedirect}},
+			Stderr: &debug,
+		}
+		req, err := http.NewRequest(http.MethodPost, server.URL+path, strings.NewReader("{}"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		_, err = httpClient.Do(req)
+		var refused *RedirectRefusedError
+		if !errors.As(err, &refused) {
+			t.Fatalf("%s: redirect was not refused: %v", path, err)
+		}
+		wrapped := fmt.Errorf("error sending request: %w", err)
+		c := output.Classify(&cobra.Command{}, wrapped)
+		for name, text := range map[string]string{
+			"error":          wrapped.Error(),
+			"classification": fmt.Sprintf("%+v", c),
+			"debug":          debug.String(),
+		} {
+			if strings.Contains(text, secret) {
+				t.Errorf("%s: %s output contains the redirect secret: %s", path, name, text)
+			}
+		}
+	}
+}

templates/templates/cli/auxiliary/internal/output/classify.go.stmpl

@@ -466,6 +466,10 @@ func classifyStatuslessError(err error, origin ErrorOrigin) ErrorType {
 	if errors.As(err, &streamErr) {
 		return ErrorTypeAPI
 	}
+	var refused interface{ CLIRedirectRefused() bool }
+	if errors.As(err, &refused) && refused.CLIRedirectRefused() {
+		return ErrorTypeProtocol
+	}
 	if origin == ErrorOriginCLI {
 		if isConnectionError(err) {
 			return ErrorTypeConnection