From 21c63a8cc972feb3fa82040931b49a40889b728e Mon Sep 17 00:00:00 2001
From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Date: Thu, 17 Sep 2026 13:55:59 -0400
Subject: [PATCH 1/7] fix!: Send credentials only to configured origins

Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
---
 github/copilot.go             |   5 +
 github/copilot_test.go        |  46 +++++
 github/github.go              | 219 ++++++++++++++++++----
 github/github_test.go         | 340 +++++++++++++++++++++++++++++++++-
 github/repos_releases.go      |  36 ++--
 github/repos_releases_test.go |  46 +++--
 6 files changed, 608 insertions(+), 84 deletions(-)

diff --git a/github/copilot.go b/github/copilot.go
index e53a80e38eb..2992c131684 100644
--- a/github/copilot.go
+++ b/github/copilot.go
@@ -1617,6 +1617,11 @@ type CopilotUserPeriodicMetrics struct {
 
 // fetchMetricsReport performs a GET against the provided download URL and returns the raw
 // http.Response. The caller is responsible for closing the body.
+//
+// The download URL is a value the caller reads out of a report response, which
+// may name any host. No host check belongs here: the client attaches its
+// credentials only to its configured API and upload origins, so a link that
+// points elsewhere is fetched without them, as is any redirect target.
 func (s *CopilotService) fetchMetricsReport(ctx context.Context, url string) (*http.Response, *Response, error) {
 	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
 	if err != nil {
diff --git a/github/copilot_test.go b/github/copilot_test.go
index 3eb26dda7ff..6d61a3900d5 100644
--- a/github/copilot_test.go
+++ b/github/copilot_test.go
@@ -10,6 +10,8 @@ import (
 	"fmt"
 	"log"
 	"net/http"
+	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -3905,6 +3907,50 @@ func TestCopilotService_DownloadCopilotMetrics(t *testing.T) {
 	}
 }
 
+// TestCopilotService_DownloadMetrics_ForeignHostGetsNoCredentials covers the
+// download helpers whose URL comes straight out of a report response, and which
+// therefore may name any host: DownloadCopilotMetrics, and the fetchMetricsReport
+// backed Download*Metrics methods. The client attaches its token only to its own
+// configured origins, so a download link naming some other host is fetched
+// unauthenticated. That is a property of the client's credential wrapper rather
+// than of any one of these methods, which is why none of them needs a host check
+// of its own.
+func TestCopilotService_DownloadMetrics_ForeignHostGetsNoCredentials(t *testing.T) {
+	t.Parallel()
+	client, _, _ := setup(t)
+
+	auth := make(chan string, 1)
+	foreign := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		select {
+		case auth <- r.Header.Get("Authorization"):
+		default:
+		}
+		if strings.HasSuffix(r.URL.Path, "/metrics") {
+			// DownloadCopilotMetrics decodes an array...
+			fmt.Fprint(w, `[]`)
+			return
+		}
+		// ...and DownloadDailyMetrics decodes an object.
+		fmt.Fprint(w, `{}`)
+	}))
+	t.Cleanup(foreign.Close)
+
+	authedClient, err := client.Clone(WithAuthToken("secret-token"))
+	if err != nil {
+		t.Fatalf("Client.Clone returned error: %v", err)
+	}
+
+	if _, _, err := authedClient.Copilot.DownloadDailyMetrics(t.Context(), foreign.URL+"/daily"); err != nil {
+		t.Fatalf("DownloadDailyMetrics returned error: %v", err)
+	}
+	assertRecordedAuthHeader(t, auth, "")
+
+	if _, _, err := authedClient.Copilot.DownloadCopilotMetrics(t.Context(), foreign.URL+"/metrics"); err != nil {
+		t.Fatalf("DownloadCopilotMetrics returned error: %v", err)
+	}
+	assertRecordedAuthHeader(t, auth, "")
+}
+
 func TestCopilotService_DownloadDailyMetrics(t *testing.T) {
 	t.Parallel()
 	client, mux, _ := setup(t)
diff --git a/github/github.go b/github/github.go
index 463ff2f204e..7264d6bd461 100644
--- a/github/github.go
+++ b/github/github.go
@@ -168,6 +168,15 @@ type Client struct {
 	client                *http.Client // HTTP client used to communicate with the API.
 	clientIgnoreRedirects *http.Client // HTTP client used to communicate with the API on endpoints where we don't want to follow redirects.
 
+	// authToken is the token configured by [WithAuthToken], retained so that
+	// [Client.Clone] can install it against the clone's own origins.
+	authToken *string
+
+	// baseTransport is the transport the credential wrapper was installed on
+	// top of, so that [Client.Clone] can rebase onto it rather than layering a
+	// second wrapper over the first.
+	baseTransport http.RoundTripper
+
 	// Base URL for API requests. Defaults to the public GitHub API, but can be
 	// set to a domain endpoint to use with GitHub Enterprise. baseURL should
 	// always be specified with a trailing slash.
@@ -257,8 +266,9 @@ type service struct {
 }
 
 // Client returns the http.Client used by this GitHub client.
-// This should only be used for requests to the GitHub API because
-// request headers will contain an authorization token.
+// The token configured by [WithAuthToken] is attached only to requests to this
+// client's configured API and upload origins, so the returned client may also
+// be used for requests to other hosts without leaking the token.
 func (c *Client) Client() *http.Client {
 	clientCopy := *c.client
 	return &clientCopy
@@ -436,7 +446,9 @@ func WithEnvProxy() ClientOptionsFunc {
 }
 
 // WithAuthToken returns a ClientOptionsFunc that sets the authentication token
-// for a Client. If not set, the client will make unauthenticated requests.
+// for a Client. The token is attached only to requests to the Client's
+// configured API and upload origins; a request to any other origin is sent
+// without it. If not set, the client will make unauthenticated requests.
 func WithAuthToken(token string) ClientOptionsFunc {
 	return func(o *clientOptions) error {
 		if token == "" {
@@ -604,25 +616,6 @@ func newClient(opts clientOptions) (*Client, error) {
 		c.client.Transport = t2
 	}
 
-	if opts.token != nil {
-		transport := c.client.Transport
-		if transport == nil {
-			transport = http.DefaultTransport
-		}
-		c.client.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) {
-			req = req.Clone(req.Context())
-			req.Header.Set("Authorization", fmt.Sprintf("Bearer %v", *opts.token))
-			return transport.RoundTrip(req)
-		})
-	}
-
-	c.clientIgnoreRedirects = &http.Client{
-		Transport:     c.client.Transport,
-		Timeout:       c.client.Timeout,
-		Jar:           c.client.Jar,
-		CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse },
-	}
-
 	if opts.apiVersionMin != nil {
 		c.apiVersionMin = *opts.apiVersionMin
 	}
@@ -649,6 +642,36 @@ func newClient(opts clientOptions) (*Client, error) {
 		c.uploadURL, _ = url.Parse(uploadBaseURL)
 	}
 
+	// Install the credential wrapper only after the API and upload origins are
+	// known: the wrapper consults them for every request, so that a request to
+	// any other origin goes out unauthenticated instead of carrying the token.
+	c.baseTransport = c.client.Transport
+	if opts.token != nil {
+		token := "Bearer " + *opts.token
+		c.authToken = opts.token
+
+		transport := c.baseTransport
+		if transport == nil {
+			transport = http.DefaultTransport
+		}
+
+		c.client.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) {
+			if c.shouldAuthorizeRequest(req.URL) {
+				req = req.Clone(req.Context())
+				req.Header.Set("Authorization", token)
+			}
+
+			return transport.RoundTrip(req)
+		})
+	}
+
+	c.clientIgnoreRedirects = &http.Client{
+		Transport:     c.client.Transport,
+		Timeout:       c.client.Timeout,
+		Jar:           c.client.Jar,
+		CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse },
+	}
+
 	c.disableRateLimitCheck = opts.disableRateLimitCheck
 
 	if !c.disableRateLimitCheck {
@@ -709,6 +732,83 @@ func newClient(opts clientOptions) (*Client, error) {
 	return c, nil
 }
 
+// Credentials — the token configured by [WithAuthToken], and the credentials on
+// [BasicAuthTransport] and [UnauthenticatedRateLimitedTransport] — are sent
+// only to origins the caller has configured. Any other destination, including
+// a redirect hop, goes out with no credentials attached. Such a request is
+// never rejected and the credentials are never forwarded: it is simply sent
+// unauthenticated, so that a caller cannot leak a token by handing the client a
+// URL whose host it does not control.
+//
+// sameOrigin is the one predicate behind that rule. Every place that decides
+// whether a destination may receive credentials — the WithAuthToken wrapper,
+// both exported auth transports, and the redirect guards — must use it, so the
+// answer cannot differ depending on which code path a request happens to take.
+
+// defaultAuthOrigins are the origins credentials may be sent to when no
+// allowlist is configured: GitHub.com's API and upload hosts. An empty
+// allowlist must never be read as "any origin".
+var defaultAuthOrigins = []*url.URL{
+	{Scheme: "https", Host: "api.github.com"},
+	{Scheme: "https", Host: "uploads.github.com"},
+}
+
+// sameOrigin reports whether u and base share an origin: the same scheme, the
+// same hostname, and the same port once the scheme's default port is implied. A
+// nil argument never matches.
+func sameOrigin(u, base *url.URL) bool {
+	if u == nil || base == nil {
+		return false
+	}
+
+	return strings.EqualFold(u.Scheme, base.Scheme) &&
+		strings.EqualFold(u.Hostname(), base.Hostname()) &&
+		normalizedPort(u) == normalizedPort(base)
+}
+
+// normalizedPort returns u's explicit port, or the default port for u's scheme
+// so that "https://ghe.example.com" and "https://ghe.example.com:443" compare
+// equal. It returns "" for a scheme with no default port.
+func normalizedPort(u *url.URL) string {
+	if port := u.Port(); port != "" {
+		return port
+	}
+
+	switch strings.ToLower(u.Scheme) {
+	case "http":
+		return "80"
+	case "https":
+		return "443"
+	default:
+		return ""
+	}
+}
+
+// isAllowedOrigin reports whether u matches one of origins. An empty origins
+// list means [defaultAuthOrigins], not "allow every origin".
+func isAllowedOrigin(u *url.URL, origins []*url.URL) bool {
+	if len(origins) == 0 {
+		origins = defaultAuthOrigins
+	}
+
+	for _, origin := range origins {
+		if sameOrigin(u, origin) {
+			return true
+		}
+	}
+
+	return false
+}
+
+// shouldAuthorizeRequest reports whether the credentials configured on c may be
+// sent to u. The client's own origins are read here, at request time, rather
+// than captured when the wrapper was installed: GitHub Enterprise installs and
+// test setups assign baseURL/uploadURL directly, and the rest of the request
+// plumbing (NewRequest, BaseURL) reads them the same way.
+func (c *Client) shouldAuthorizeRequest(u *url.URL) bool {
+	return sameOrigin(u, c.baseURL) || sameOrigin(u, c.uploadURL)
+}
+
 // UserAgent returns the User-Agent header value for the client.
 func (c *Client) UserAgent() string {
 	return c.userAgent
@@ -736,7 +836,9 @@ func (c *Client) UploadURL() string {
 // the same rate limit information as the original client, but it is not
 // updated when the original client's rate limit information is updated.
 // The returned client is independent of the original client and can be
-// modified without affecting the original client.
+// modified without affecting the original client. Any token configured by
+// [WithAuthToken] carries over to the clone, and is re-scoped to the clone's
+// own API and upload origins.
 func (c *Client) Clone(opts ...ClientOptionsFunc) (*Client, error) {
 	if c.client == nil {
 		return nil, errUninitialized
@@ -748,6 +850,7 @@ func (c *Client) Clone(opts ...ClientOptionsFunc) (*Client, error) {
 		userAgent:                               &c.userAgent,
 		baseURL:                                 new(*c.baseURL),
 		uploadURL:                               new(*c.uploadURL),
+		token:                                   c.authToken,
 		disableRateLimitCheck:                   c.disableRateLimitCheck,
 		rateLimitRedirectionalEndpoints:         c.rateLimitRedirectionalEndpoints,
 		maxSecondaryRateLimitRetryAfterDuration: &c.maxSecondaryRateLimitRetryAfterDuration,
@@ -764,8 +867,17 @@ func (c *Client) Clone(opts ...ClientOptionsFunc) (*Client, error) {
 	}
 
 	if o.httpClient == nil {
+		// A clone that carries a token installs the credential against its own
+		// origins, so it starts from the unwrapped transport: reusing the
+		// wrapped one would stack a second check that still enforces the
+		// original client's origins.
+		transport := c.client.Transport
+		if c.authToken != nil {
+			transport = c.baseTransport
+		}
+
 		o.httpClient = &http.Client{
-			Transport:     c.client.Transport,
+			Transport:     transport,
 			CheckRedirect: c.client.CheckRedirect,
 			Jar:           c.client.Jar,
 			Timeout:       c.client.Timeout,
@@ -1395,10 +1507,12 @@ func (c *Client) bareDoUntilFound(req *http.Request, maxRedirects int) (*url.URL
 					return nil, nil, errInvalidLocation
 				}
 				newURL := c.baseURL.ResolveReference(rerr.Location)
-				// Refuse to follow a permanent redirect to a different host:
-				// req.Clone preserves Authorization headers added by the auth
-				// transport, so a cross-host target would leak credentials.
-				if newURL.Host != c.baseURL.Host {
+				// Refuse to follow a permanent redirect outside the origins
+				// this client may send credentials to: the auth transport
+				// attaches them on every hop, so a cross-host target would
+				// leak them. This uses the same predicate as the transport so
+				// that the two cannot disagree about which origin is allowed.
+				if !c.shouldAuthorizeRequest(newURL) {
 					return nil, response, fmt.Errorf("refusing to follow cross-host redirect from %q to %q", c.baseURL.Host, newURL.Host)
 				}
 				newRequest := req.Clone(req.Context())
@@ -2052,6 +2166,16 @@ type UnauthenticatedRateLimitedTransport struct {
 	// application.
 	ClientSecret string
 
+	// AllowedOrigins limits the origins ClientID and ClientSecret are sent to.
+	// The credentials are attached only to requests whose origin matches one of
+	// these; every other request, including a redirect hop, is sent without
+	// them.
+	//
+	// If empty, the GitHub.com API and upload origins are used. An empty
+	// AllowedOrigins does not mean "any origin": set it explicitly when talking
+	// to GitHub Enterprise.
+	AllowedOrigins []*url.URL
+
 	// Transport is the underlying HTTP transport to use when making requests.
 	// It will default to http.DefaultTransport if nil.
 	Transport http.RoundTripper
@@ -2066,9 +2190,12 @@ func (t *UnauthenticatedRateLimitedTransport) RoundTrip(req *http.Request) (*htt
 		return nil, errors.New("t.ClientSecret is empty")
 	}
 
-	req2 := setCredentialsAsHeaders(req, t.ClientID, t.ClientSecret)
+	if isAllowedOrigin(req.URL, t.AllowedOrigins) {
+		req = setCredentialsAsHeaders(req, t.ClientID, t.ClientSecret)
+	}
+
 	// Make the HTTP request.
-	return t.transport().RoundTrip(req2)
+	return t.transport().RoundTrip(req)
 }
 
 // Client returns an *http.Client that makes requests which are subject to the
@@ -2093,6 +2220,15 @@ type BasicAuthTransport struct {
 	Password string // GitHub password
 	OTP      string // one-time password for users with two-factor auth enabled
 
+	// AllowedOrigins limits the origins the credentials above are sent to. They
+	// are attached only to requests whose origin matches one of these; every
+	// other request, including a redirect hop, is sent without them.
+	//
+	// If empty, the GitHub.com API and upload origins are used. An empty
+	// AllowedOrigins does not mean "any origin": set it explicitly when talking
+	// to GitHub Enterprise.
+	AllowedOrigins []*url.URL
+
 	// Transport is the underlying HTTP transport to use when making requests.
 	// It will default to http.DefaultTransport if nil.
 	Transport http.RoundTripper
@@ -2100,11 +2236,14 @@ type BasicAuthTransport struct {
 
 // RoundTrip implements the RoundTripper interface.
 func (t *BasicAuthTransport) RoundTrip(req *http.Request) (*http.Response, error) {
-	req2 := setCredentialsAsHeaders(req, t.Username, t.Password)
-	if t.OTP != "" {
-		req2.Header.Set(headerOTP, t.OTP)
+	if isAllowedOrigin(req.URL, t.AllowedOrigins) {
+		req = setCredentialsAsHeaders(req, t.Username, t.Password)
+		if t.OTP != "" {
+			req.Header.Set(headerOTP, t.OTP)
+		}
 	}
-	return t.transport().RoundTrip(req2)
+
+	return t.transport().RoundTrip(req)
 }
 
 // Client returns an *http.Client that makes requests that are authenticated
@@ -2190,11 +2329,11 @@ func (c *Client) roundTripWithOptionalFollowRedirect(ctx context.Context, u stri
 	return resp, err
 }
 
-// checkRedirectHost returns an error if the redirect target is on a different
-// host than the client's configured BaseURL. This prevents credentials attached
-// by the auth transport from being sent to an attacker-controlled host when a
-// compromised or malicious API response returns a cross-origin Location header.
-// An empty Location is also rejected.
+// checkRedirectHost returns an error if the redirect target is outside the
+// origins this client may send credentials to. The auth transport attaches
+// credentials on every hop, so a cross-origin Location header would otherwise
+// carry them to a host the caller never configured, when a compromised or
+// malicious API response supplies one. An empty Location is also rejected.
 func (c *Client) checkRedirectHost(location string) error {
 	if location == "" {
 		return errInvalidLocation
@@ -2205,7 +2344,7 @@ func (c *Client) checkRedirectHost(location string) error {
 	}
 	// Resolve relative locations against BaseURL so relative paths are allowed.
 	target = c.baseURL.ResolveReference(target)
-	if target.Host != c.baseURL.Host {
+	if !c.shouldAuthorizeRequest(target) {
 		return fmt.Errorf("refusing to follow cross-host redirect from %q to %q", c.baseURL.Host, target.Host)
 	}
 	return nil
diff --git a/github/github_test.go b/github/github_test.go
index 69c05ae12e4..133c4c481c0 100644
--- a/github/github_test.go
+++ b/github/github_test.go
@@ -2441,12 +2441,17 @@ func TestDo_AcceptedError_LargeBodyTruncated(t *testing.T) {
 // does not leak the client secret.
 func TestDo_sanitizeURL(t *testing.T) {
 	t.Parallel()
+	baseURL := &url.URL{Scheme: "http", Host: "127.0.0.1:0", Path: "/"} // Use port 0 on purpose to trigger a dial TCP error, expect to get "dial tcp 127.0.0.1:0: connect: can't assign requested address".
 	tp := &UnauthenticatedRateLimitedTransport{
 		ClientID:     "id",
 		ClientSecret: "secret",
+		// Scope the transport to the origin below so that the credentials really are
+		// attached to this request; otherwise the test would pass vacuously, with the
+		// secret never sent in the first place.
+		AllowedOrigins: []*url.URL{baseURL},
 	}
 	unauthedClient := mustNewClient(t, WithHTTPClient(tp.Client()))
-	unauthedClient.baseURL = &url.URL{Scheme: "http", Host: "127.0.0.1:0", Path: "/"} // Use port 0 on purpose to trigger a dial TCP error, expect to get "dial tcp 127.0.0.1:0: connect: can't assign requested address".
+	unauthedClient.baseURL = baseURL
 	req, err := unauthedClient.NewRequest(t.Context(), "GET", ".", nil)
 	if err != nil {
 		t.Fatalf("NewRequest returned unexpected error: %v", err)
@@ -4509,8 +4514,9 @@ func TestUnauthenticatedRateLimitedTransport(t *testing.T) {
 	})
 
 	tp := &UnauthenticatedRateLimitedTransport{
-		ClientID:     clientID,
-		ClientSecret: clientSecret,
+		ClientID:       clientID,
+		ClientSecret:   clientSecret,
+		AllowedOrigins: []*url.URL{client.baseURL},
 	}
 	unauthedClient := mustNewClient(t, WithHTTPClient(tp.Client()))
 	unauthedClient.baseURL = client.baseURL
@@ -4519,6 +4525,50 @@ func TestUnauthenticatedRateLimitedTransport(t *testing.T) {
 	assertNilError(t, err)
 }
 
+func TestUnauthenticatedRateLimitedTransport_originScope(t *testing.T) {
+	t.Parallel()
+	clientID, clientSecret := "id", "secret"
+
+	// The origin the transport is scoped to receives the credentials...
+	allowed, allowedMux, _ := setup(t)
+	allowedMux.HandleFunc("/", func(_ http.ResponseWriter, r *http.Request) {
+		if _, _, ok := r.BasicAuth(); !ok {
+			t.Error("request to an allowed origin does not contain basic auth credentials")
+		}
+	})
+
+	// ...and any other origin does not, even when the same transport is used.
+	foreign, foreignMux, _ := setup(t)
+	foreignMux.HandleFunc("/", func(_ http.ResponseWriter, r *http.Request) {
+		if id, secret, ok := r.BasicAuth(); ok {
+			t.Errorf("request to an unrelated origin contained basic auth credentials %q/%q", id, secret)
+		}
+	})
+
+	tp := &UnauthenticatedRateLimitedTransport{
+		ClientID:       clientID,
+		ClientSecret:   clientSecret,
+		AllowedOrigins: []*url.URL{allowed.baseURL},
+	}
+
+	for _, test := range []struct {
+		name string
+		base *url.URL
+	}{
+		{name: "allowed origin", base: allowed.baseURL},
+		{name: "foreign origin", base: foreign.baseURL},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			c := mustNewClient(t, WithHTTPClient(tp.Client()))
+			c.baseURL = test.base
+			req, _ := c.NewRequest(t.Context(), "GET", ".", nil)
+			_, err := c.Do(req, nil)
+			assertNilError(t, err)
+		})
+	}
+}
+
 func TestUnauthenticatedRateLimitedTransport_missingFields(t *testing.T) {
 	t.Parallel()
 	// missing ClientID
@@ -4585,9 +4635,10 @@ func TestBasicAuthTransport(t *testing.T) {
 	})
 
 	tp := &BasicAuthTransport{
-		Username: username,
-		Password: password,
-		OTP:      otp,
+		Username:       username,
+		Password:       password,
+		OTP:            otp,
+		AllowedOrigins: []*url.URL{client.baseURL},
 	}
 	basicAuthClient := mustNewClient(t, WithHTTPClient(tp.Client()))
 	basicAuthClient.baseURL = client.baseURL
@@ -4596,6 +4647,283 @@ func TestBasicAuthTransport(t *testing.T) {
 	assertNilError(t, err)
 }
 
+func TestBasicAuthTransport_originScope(t *testing.T) {
+	t.Parallel()
+	username, password, otp := "u", "p", "123456"
+
+	// The origin the transport is scoped to receives both credentials and the OTP...
+	allowed, allowedMux, _ := setup(t)
+	allowedMux.HandleFunc("/", func(_ http.ResponseWriter, r *http.Request) {
+		if u, p, ok := r.BasicAuth(); !ok || u != username || p != password {
+			t.Error("request to an allowed origin does not contain the expected basic auth credentials")
+		}
+		if got := r.Header.Get(headerOTP); got != otp {
+			t.Errorf("request to an allowed origin contained OTP %q, want %q", got, otp)
+		}
+	})
+
+	// ...and any other origin receives neither. The OTP is a second factor, so a
+	// foreign origin must not see it even though it is not a "credential" per se.
+	foreign, foreignMux, _ := setup(t)
+	foreignMux.HandleFunc("/", func(_ http.ResponseWriter, r *http.Request) {
+		if u, p, ok := r.BasicAuth(); ok {
+			t.Errorf("request to an unrelated origin contained basic auth credentials %q/%q", u, p)
+		}
+		if got := r.Header.Get(headerOTP); got != "" {
+			t.Errorf("request to an unrelated origin contained OTP %q, want none", got)
+		}
+	})
+
+	tp := &BasicAuthTransport{
+		Username:       username,
+		Password:       password,
+		OTP:            otp,
+		AllowedOrigins: []*url.URL{allowed.baseURL},
+	}
+
+	for _, test := range []struct {
+		name string
+		base *url.URL
+	}{
+		{name: "allowed origin", base: allowed.baseURL},
+		{name: "foreign origin", base: foreign.baseURL},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			c := mustNewClient(t, WithHTTPClient(tp.Client()))
+			c.baseURL = test.base
+			req, _ := c.NewRequest(t.Context(), "GET", ".", nil)
+			_, err := c.Do(req, nil)
+			assertNilError(t, err)
+		})
+	}
+}
+
+// authRecorderServer returns a server that reports the Authorization header of
+// each request it serves on the returned channel. The channel is buffered and
+// keeps only the first value, so a test cannot block the server, and a test that
+// expects exactly one request does not have to drain it.
+func authRecorderServer(t *testing.T) (*httptest.Server, <-chan string) {
+	t.Helper()
+	auth := make(chan string, 1)
+	srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		select {
+		case auth <- r.Header.Get("Authorization"):
+		default:
+		}
+	}))
+	t.Cleanup(srv.Close)
+	return srv, auth
+}
+
+// assertRecordedAuthHeader asserts the Authorization header the recorded server
+// received. It fails if the server was never reached, so a test cannot pass
+// merely because the request never left the client.
+func assertRecordedAuthHeader(t *testing.T, auth <-chan string, want string) {
+	t.Helper()
+	select {
+	case got := <-auth:
+		if got != want {
+			t.Errorf("Authorization header sent to the server = %q, want %q", got, want)
+		}
+	default:
+		t.Fatal("the server was never reached")
+	}
+}
+
+func TestSameOrigin(t *testing.T) {
+	t.Parallel()
+	for _, tt := range []struct {
+		name string
+		a    string
+		b    string
+		want bool
+	}{
+		{name: "identical", a: "https://api.github.com", b: "https://api.github.com", want: true},
+		{name: "path and trailing slash are irrelevant", a: "https://api.github.com/", b: "https://api.github.com/repos/o/r", want: true},
+		{name: "default https port is implied", a: "https://api.github.com", b: "https://api.github.com:443", want: true},
+		{name: "default http port is implied", a: "http://ghe.example.com:80", b: "http://ghe.example.com", want: true},
+		{name: "hostname case is ignored", a: "https://API.GitHub.com", b: "https://api.github.com", want: true},
+		{name: "scheme case is ignored", a: "HTTPS://api.github.com", b: "https://api.github.com", want: true},
+		{name: "userinfo is not part of the origin", a: "https://user:pass@api.github.com", b: "https://api.github.com", want: true},
+		{name: "scheme is compared", a: "http://api.github.com", b: "https://api.github.com", want: false},
+		{name: "explicit non-default port differs", a: "https://ghe.example.com", b: "https://ghe.example.com:8443", want: false},
+		{name: "two ports differ", a: "http://127.0.0.1:8080", b: "http://127.0.0.1:9090", want: false},
+		{name: "subdomain is a different origin", a: "https://evil.api.github.com", b: "https://api.github.com", want: false},
+		{name: "suffix is a different origin", a: "https://api.github.com.evil.example", b: "https://api.github.com", want: false},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			a, b := mustParseURL(t, tt.a), mustParseURL(t, tt.b)
+			if got := sameOrigin(a, b); got != tt.want {
+				t.Errorf("sameOrigin(%q, %q) = %v, want %v", tt.a, tt.b, got, tt.want)
+			}
+			// Neither argument is privileged, so the answer must not depend on
+			// which one is the destination.
+			if got := sameOrigin(b, a); got != tt.want {
+				t.Errorf("sameOrigin(%q, %q) = %v, want %v", tt.b, tt.a, got, tt.want)
+			}
+		})
+	}
+
+	t.Run("nil never matches", func(t *testing.T) {
+		t.Parallel()
+		base := mustParseURL(t, "https://api.github.com")
+		if sameOrigin(nil, nil) {
+			t.Error("sameOrigin(nil, nil) = true, want false")
+		}
+		if sameOrigin(nil, base) {
+			t.Error("sameOrigin(nil, base) = true, want false")
+		}
+		if sameOrigin(base, nil) {
+			t.Error("sameOrigin(base, nil) = true, want false")
+		}
+	})
+}
+
+func TestIsAllowedOrigin(t *testing.T) {
+	t.Parallel()
+	ghe := mustParseURL(t, "https://ghe.example.com")
+	uploads := mustParseURL(t, "https://uploads.github.com")
+
+	for _, tt := range []struct {
+		name    string
+		u       string
+		origins []*url.URL
+		want    bool
+	}{
+		{name: "api.github.com is allowed by default", u: "https://api.github.com", want: true},
+		{name: "uploads.github.com is allowed by default", u: "https://uploads.github.com", want: true},
+		{name: "the default port may be spelled out", u: "https://api.github.com:443", want: true},
+		{name: "an unrelated host is not allowed by default", u: "https://evil.example.com", want: false},
+		{name: "http is not allowed by default", u: "http://api.github.com", want: false},
+		{name: "a configured origin is allowed", u: "https://ghe.example.com", origins: []*url.URL{ghe}, want: true},
+		{name: "an origin absent from the allowlist is not", u: "https://api.github.com", origins: []*url.URL{ghe}, want: false},
+		{name: "any one match is enough", u: "https://uploads.github.com", origins: []*url.URL{ghe, uploads}, want: true},
+		{name: "a nil entry is skipped", u: "https://ghe.example.com", origins: []*url.URL{nil, ghe}, want: true},
+		{name: "a nil-only allowlist allows nothing", u: "https://api.github.com", origins: []*url.URL{nil}, want: false},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			if got := isAllowedOrigin(mustParseURL(t, tt.u), tt.origins); got != tt.want {
+				t.Errorf("isAllowedOrigin(%q, %v) = %v, want %v", tt.u, tt.origins, got, tt.want)
+			}
+		})
+	}
+}
+
+// TestClient_tokenOriginScope is the policy's end-to-end guarantee on the
+// default client: a [WithAuthToken] token reaches the client's configured API
+// and upload origins, and no other destination.
+func TestClient_tokenOriginScope(t *testing.T) {
+	t.Parallel()
+	api, apiAuth := authRecorderServer(t)
+	upload, uploadAuth := authRecorderServer(t)
+	foreign, foreignAuth := authRecorderServer(t)
+
+	const token = "secret-token"
+	client := mustNewClient(t, WithAuthToken(token), WithURLs(&api.URL, &upload.URL))
+
+	for _, tt := range []struct {
+		name string
+		url  string
+		auth <-chan string
+		want string
+	}{
+		{name: "API origin", url: api.URL, auth: apiAuth, want: "Bearer " + token},
+		{name: "upload origin", url: upload.URL, auth: uploadAuth, want: "Bearer " + token},
+		{name: "foreign origin", url: foreign.URL, auth: foreignAuth, want: ""},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			req, err := client.NewRequest(t.Context(), "GET", tt.url, nil)
+			if err != nil {
+				t.Fatalf("NewRequest returned error: %v", err)
+			}
+			if _, err := client.Do(req, nil); err != nil {
+				t.Fatalf("Do returned error: %v", err)
+			}
+			assertRecordedAuthHeader(t, tt.auth, tt.want)
+		})
+	}
+}
+
+// TestClient_tokenNotForwardedOnCrossOriginRedirect is the guarantee the policy
+// exists for. The credential wrapper runs on every hop, including redirect hops,
+// and decides per destination, so a redirect cannot carry the token off the
+// configured origins.
+//
+// net/http's own redirect handling cannot be relied on here: it decides whether
+// to copy sensitive headers by comparing hostnames and ignoring the port, so two
+// servers on the same host with different ports count as a single destination.
+// This test uses exactly that pair, so the outcome depends on the wrapper alone:
+// it is the wrapper's per-hop decision that must keep the token off the redirect
+// target.
+func TestClient_tokenNotForwardedOnCrossOriginRedirect(t *testing.T) {
+	t.Parallel()
+	foreign, foreignAuth := authRecorderServer(t)
+
+	redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, foreign.URL+"/steal", http.StatusFound)
+	}))
+	t.Cleanup(redirector.Close)
+
+	if got := mustParseURL(t, redirector.URL).Hostname(); got != mustParseURL(t, foreign.URL).Hostname() {
+		t.Fatalf("this test requires both servers to share a hostname, got %q and %q", redirector.URL, foreign.URL)
+	}
+
+	client := mustNewClient(t, WithAuthToken("secret-token"), WithURLs(&redirector.URL, nil))
+
+	req, err := client.NewRequest(t.Context(), "GET", ".", nil)
+	if err != nil {
+		t.Fatalf("NewRequest returned error: %v", err)
+	}
+	// Do follows the redirect, so the foreign server really is contacted; the
+	// only question is what it is sent.
+	if _, err := client.Do(req, nil); err != nil {
+		t.Fatalf("Do returned error: %v", err)
+	}
+
+	assertRecordedAuthHeader(t, foreignAuth, "")
+}
+
+// TestClient_CloneReScopesToken verifies that a clone carries the token over but
+// installs it against the clone's own origins, rather than the origins the
+// original client was built with.
+func TestClient_CloneReScopesToken(t *testing.T) {
+	t.Parallel()
+	first, firstAuth := authRecorderServer(t)
+	second, secondAuth := authRecorderServer(t)
+
+	const token = "secret-token"
+	original := mustNewClient(t, WithAuthToken(token), WithURLs(&first.URL, nil))
+
+	clone, err := original.Clone(WithURLs(&second.URL, nil))
+	if err != nil {
+		t.Fatalf("Clone returned error: %v", err)
+	}
+
+	// The clone's own origin receives the token...
+	req, err := clone.NewRequest(t.Context(), "GET", ".", nil)
+	if err != nil {
+		t.Fatalf("NewRequest returned error: %v", err)
+	}
+	if _, err := clone.Do(req, nil); err != nil {
+		t.Fatalf("Do returned error: %v", err)
+	}
+	assertRecordedAuthHeader(t, secondAuth, "Bearer "+token)
+
+	// ...and the original client's origin does not, since it is foreign to the clone.
+	req, err = clone.NewRequest(t.Context(), "GET", first.URL, nil)
+	if err != nil {
+		t.Fatalf("NewRequest returned error: %v", err)
+	}
+	if _, err := clone.Do(req, nil); err != nil {
+		t.Fatalf("Do returned error: %v", err)
+	}
+	assertRecordedAuthHeader(t, firstAuth, "")
+}
+
 func TestBasicAuthTransport_transport(t *testing.T) {
 	t.Parallel()
 	// default transport
diff --git a/github/repos_releases.go b/github/repos_releases.go
index 8fb4ebea611..3c676609216 100644
--- a/github/repos_releases.go
+++ b/github/repos_releases.go
@@ -12,7 +12,6 @@ import (
 	"io"
 	"mime"
 	"net/http"
-	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -326,9 +325,11 @@ func (s *RepositoriesService) GetReleaseAsset(ctx context.Context, owner, repo s
 // of the io.ReadCloser. Exactly one of rc and redirectURL will be zero.
 //
 // followRedirectsClient can be passed to download the asset from a redirected
-// location. Specifying any http.Client is possible, but passing http.DefaultClient
-// is recommended, except when the specified repository is private, in which case
-// it's necessary to pass an http.Client that performs authenticated requests.
+// location. The redirect target is typically a pre-signed third-party URL (for
+// example S3), so http.DefaultClient is recommended. The client's own
+// credentials reach only its configured API and upload origins, so they are not
+// attached to the redirected request in any case; supply a client that adds its
+// own credentials only if the redirect target requires them.
 // If nil is passed the redirectURL will be returned instead.
 //
 // GitHub API docs: https://docs.github.com/rest/releases/assets?apiVersion=2022-11-28#get-a-release-asset
@@ -503,27 +504,16 @@ func (s *RepositoriesService) UploadReleaseAssetFromRelease(
 
 	// If this is a *relative* URL (no scheme), normalize it by trimming a leading "/"
 	// so it works with Client.BaseURL path prefixes (e.g. "/api-v3/").
+	//
+	// An absolute URL replaces the client's configured upload host entirely.
+	// That is deliberately left to the client's transport, which attaches the
+	// caller's Authorization header only to the client's configured API and
+	// upload origins: a response naming a foreign host therefore cannot take
+	// the token with it, and does not need a host check here. In the default
+	// configuration the upload host is uploads.github.com rather than
+	// api.github.com, so both are configured origins the token may reach.
 	if !strings.HasPrefix(uploadURL, "http://") && !strings.HasPrefix(uploadURL, "https://") {
 		uploadURL = strings.TrimPrefix(uploadURL, "/")
-	} else {
-		// This helper is the one upload entry point whose URL comes from a server
-		// response rather than from the caller, and an absolute URL replaces the
-		// client's configured upload host entirely. Left unchecked, a response could
-		// name any host and receive the artifact together with the caller's
-		// Authorization header. Keep the upload on the host the client was
-		// configured with; that is uploads.github.com rather than api.github.com in
-		// the default configuration, which is why the comparison is against
-		// uploadURL and not baseURL.
-		u, err := url.Parse(uploadURL)
-		if err != nil {
-			return nil, nil, err
-		}
-		if !strings.EqualFold(u.Host, s.client.uploadURL.Host) {
-			return nil, nil, fmt.Errorf(
-				"upload URL host %v does not match the client's configured upload host %v",
-				u.Host, s.client.uploadURL.Host,
-			)
-		}
 	}
 
 	// addOptions will append name/label query params (same behavior as UploadReleaseAsset).
diff --git a/github/repos_releases_test.go b/github/repos_releases_test.go
index 67012cf3245..69e57d0d395 100644
--- a/github/repos_releases_test.go
+++ b/github/repos_releases_test.go
@@ -851,34 +851,50 @@ func TestRepositoriesService_UploadReleaseAssetFromRelease_AbsoluteTemplate(t *t
 	}
 }
 
-func TestRepositoriesService_UploadReleaseAssetFromRelease_ForeignHostIsRejected(t *testing.T) {
+func TestRepositoriesService_UploadReleaseAssetFromRelease_ForeignHostGetsNoCredentials(t *testing.T) {
 	t.Parallel()
 	client, _, _ := setup(t)
 
 	// A server that hands out an absolute upload URL naming a different host must not
-	// be able to redirect the upload - and therefore the caller's Authorization header
-	// and the artifact body - away from the host the client was configured with.
-	var leaked int
-	evil := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
-		leaked++
+	// be able to take the caller's credentials with it: the client attaches its token
+	// only to its own configured origins. The upload itself is still attempted - the
+	// policy is to send such a request unauthenticated rather than to reject it - so
+	// the body does reach the foreign host, and the request must arrive with no
+	// Authorization header.
+	authHeaders := make(chan string, 1)
+	foreign := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		select {
+		case authHeaders <- r.Header.Get("Authorization"):
+		default:
+		}
 		fmt.Fprint(w, `{"id":1}`)
 	}))
-	t.Cleanup(evil.Close)
+	t.Cleanup(foreign.Close)
 
-	body := []byte("private artifact\n")
+	authedClient, err := client.Clone(WithAuthToken("secret-token"))
+	if err != nil {
+		t.Fatalf("Client.Clone returned error: %v", err)
+	}
+
+	body := []byte("artifact\n")
 	reader := bytes.NewReader(body)
 	size := int64(len(body))
 
-	release := &RepositoryRelease{UploadURL: evil.URL + "/upload{?name,label}"}
+	release := &RepositoryRelease{UploadURL: foreign.URL + "/upload{?name,label}"}
 	ctx := t.Context()
-	_, _, err := client.Repositories.UploadReleaseAssetFromRelease(
+	if _, _, err := authedClient.Repositories.UploadReleaseAssetFromRelease(
 		ctx, release, &UploadOptions{Name: "n.txt"}, reader, size,
-	)
-	if err == nil {
-		t.Fatal("expected an error for an upload URL naming a foreign host, got nil")
+	); err != nil {
+		t.Fatalf("UploadReleaseAssetFromRelease returned error: %v", err)
 	}
-	if leaked != 0 {
-		t.Fatalf("upload reached the foreign host %v time(s); the token and body must never be sent there", leaked)
+
+	select {
+	case got := <-authHeaders:
+		if got != "" {
+			t.Fatalf("upload to a foreign host carried Authorization %q; the token must never be sent there", got)
+		}
+	default:
+		t.Fatal("upload never reached the foreign host")
 	}
 }
 

From 582c158d8a956553c68e7f5d56659ae21a156d79 Mon Sep 17 00:00:00 2001
From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Date: Thu, 17 Sep 2026 14:23:15 -0400
Subject: [PATCH 2/7] Improve code coverage

Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
---
 github/github_test.go | 66 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/github/github_test.go b/github/github_test.go
index 133c4c481c0..f233ad11c8f 100644
--- a/github/github_test.go
+++ b/github/github_test.go
@@ -3644,6 +3644,28 @@ func TestBareDoUntilFound_RejectsCrossHostRedirect(t *testing.T) {
 	}
 }
 
+// TestBareDoUntilFound_MissingRedirectLocation covers a 301 that carries no
+// Location header at all. There is no target to resolve, so the redirect cannot
+// be followed and the caller gets an error rather than a request built from an
+// empty Location.
+func TestBareDoUntilFound_MissingRedirectLocation(t *testing.T) {
+	t.Parallel()
+	client, mux, _ := setup(t)
+
+	mux.HandleFunc("/", func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusMovedPermanently)
+	})
+
+	req, _ := client.NewRequest(t.Context(), "GET", ".", nil)
+	_, _, err := client.bareDoUntilFound(req, 1)
+	if err == nil {
+		t.Fatal("Expected a 301 with no Location header to be rejected, got nil error.")
+	}
+	if !errors.Is(err, errInvalidLocation) {
+		t.Errorf("Expected errInvalidLocation, got: %v", err)
+	}
+}
+
 // TestRoundTripWithOptionalFollowRedirect_RejectsCrossHostRedirect verifies
 // that roundTripWithOptionalFollowRedirect refuses to follow a 301 redirect to
 // a different host, preventing Authorization-header leakage to attacker-
@@ -3666,6 +3688,26 @@ func TestRoundTripWithOptionalFollowRedirect_RejectsCrossHostRedirect(t *testing
 	}
 }
 
+// TestRoundTripWithOptionalFollowRedirect_MissingRedirectLocation covers a 301
+// that carries no Location header at all. There is no target to check or follow,
+// so the caller gets an error rather than a request built from an empty Location.
+func TestRoundTripWithOptionalFollowRedirect_MissingRedirectLocation(t *testing.T) {
+	t.Parallel()
+	client, mux, _ := setup(t)
+
+	mux.HandleFunc("/", func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusMovedPermanently)
+	})
+
+	_, err := client.roundTripWithOptionalFollowRedirect(t.Context(), ".", 1)
+	if err == nil {
+		t.Fatal("Expected a 301 with no Location header to be rejected, got nil error.")
+	}
+	if !errors.Is(err, errInvalidLocation) {
+		t.Errorf("Expected errInvalidLocation, got: %v", err)
+	}
+}
+
 // TestRoundTripWithOptionalFollowRedirect_AllowsSameHostRedirect ensures the
 // cross-host check does not break legitimate same-host 301 follow behavior
 // (the path that rate-limit redirection relies on).
@@ -4747,6 +4789,7 @@ func TestSameOrigin(t *testing.T) {
 		{name: "scheme case is ignored", a: "HTTPS://api.github.com", b: "https://api.github.com", want: true},
 		{name: "userinfo is not part of the origin", a: "https://user:pass@api.github.com", b: "https://api.github.com", want: true},
 		{name: "scheme is compared", a: "http://api.github.com", b: "https://api.github.com", want: false},
+		{name: "a scheme with no default port implies none", a: "ftp://ghe.example.com", b: "ftp://ghe.example.com:21", want: false},
 		{name: "explicit non-default port differs", a: "https://ghe.example.com", b: "https://ghe.example.com:8443", want: false},
 		{name: "two ports differ", a: "http://127.0.0.1:8080", b: "http://127.0.0.1:9090", want: false},
 		{name: "subdomain is a different origin", a: "https://evil.api.github.com", b: "https://api.github.com", want: false},
@@ -4781,6 +4824,29 @@ func TestSameOrigin(t *testing.T) {
 	})
 }
 
+func TestNormalizedPort(t *testing.T) {
+	t.Parallel()
+	for _, tt := range []struct {
+		name string
+		url  string
+		want string
+	}{
+		{name: "http implies its default port", url: "http://ghe.example.com", want: "80"},
+		{name: "https implies its default port", url: "https://ghe.example.com", want: "443"},
+		{name: "an explicit port wins", url: "https://ghe.example.com:8443", want: "8443"},
+		{name: "an explicit default port stays explicit", url: "https://ghe.example.com:443", want: "443"},
+		{name: "a scheme with no default port has none to imply", url: "ftp://ghe.example.com", want: ""},
+		{name: "an explicit port is kept whatever the scheme", url: "ftp://ghe.example.com:21", want: "21"},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			if got := normalizedPort(mustParseURL(t, tt.url)); got != tt.want {
+				t.Errorf("normalizedPort(%q) = %q, want %q", tt.url, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestIsAllowedOrigin(t *testing.T) {
 	t.Parallel()
 	ghe := mustParseURL(t, "https://ghe.example.com")

From 1ce9c8ca54e07278df1c1d7cce3e83b3ea08e2bb Mon Sep 17 00:00:00 2001
From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Date: Thu, 17 Sep 2026 16:48:23 -0400
Subject: [PATCH 3/7] Address feedback from sushant-me

Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
---
 github/copilot.go             |  7 +++
 github/github.go              | 72 ++++++++++++++++++++++++++++---
 github/github_test.go         | 81 ++++++++++++++++++++++++++++++++---
 github/repos_contents.go      | 12 ++++++
 github/repos_releases.go      | 22 +++++++---
 github/repos_releases_test.go | 71 ++++++++++++++++++++++--------
 6 files changed, 230 insertions(+), 35 deletions(-)

diff --git a/github/copilot.go b/github/copilot.go
index 2992c131684..914c732420b 100644
--- a/github/copilot.go
+++ b/github/copilot.go
@@ -1622,6 +1622,13 @@ type CopilotUserPeriodicMetrics struct {
 // may name any host. No host check belongs here: the client attaches its
 // credentials only to its configured API and upload origins, so a link that
 // points elsewhere is fetched without them, as is any redirect target.
+//
+// This is a GET with no body of the caller's, so only the credential half of the
+// origin rules applies to it; refusing a foreign host would break the ordinary
+// path, since GitHub hands back report links on a host of its own choosing. The
+// residual is that the body returned here is whatever that host served, and the
+// Download*Metrics methods decode it as GitHub's report. A caller that must be
+// sure the report is GitHub's should treat it as untrusted input.
 func (s *CopilotService) fetchMetricsReport(ctx context.Context, url string) (*http.Response, *Response, error) {
 	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
 	if err != nil {
diff --git a/github/github.go b/github/github.go
index 7264d6bd461..01b41180c24 100644
--- a/github/github.go
+++ b/github/github.go
@@ -732,18 +732,50 @@ func newClient(opts clientOptions) (*Client, error) {
 	return c, nil
 }
 
+// A request's destination is governed by two rules, and they deliberately differ
+// because a credential and a payload fail in opposite ways.
+//
 // Credentials — the token configured by [WithAuthToken], and the credentials on
 // [BasicAuthTransport] and [UnauthenticatedRateLimitedTransport] — are sent
 // only to origins the caller has configured. Any other destination, including
 // a redirect hop, goes out with no credentials attached. Such a request is
 // never rejected and the credentials are never forwarded: it is simply sent
 // unauthenticated, so that a caller cannot leak a token by handing the client a
-// URL whose host it does not control.
+// URL whose host it does not control. Withholding a credential is safe to do
+// silently because the request can still succeed without it — a pre-signed
+// download URL handed back by the API is the ordinary case.
+//
+// A payload cannot be withheld that way, because a request whose body is dropped
+// cannot succeed at all, and the caller would be told an upload succeeded when
+// nothing was stored. The rule for a body is therefore absolute:
+// [Client.NewUploadRequest] refuses to build an upload aimed at an origin this
+// client was not configured for, so the caller's bytes are never sent to a host
+// the caller did not choose. See [ErrUntrustedUploadDestination].
+//
+// Within a Client both rules read the same two configured origins, BaseURL and
+// UploadURL, which is where [WithEnterpriseURLs] and [WithURLs] put them; a
+// GitHub Enterprise or proxy deployment whose upload host differs from its API
+// host must therefore configure both. The exported transports are not attached
+// to a Client and take their own [BasicAuthTransport.AllowedOrigins] list
+// instead. sameOrigin is the one predicate behind both rules. Every place that
+// decides whether a destination may receive credentials — the WithAuthToken
+// wrapper, both exported auth transports, and the redirect guards — must use it,
+// so the answer cannot differ depending on which code path a request happens to
+// take.
+
+// ErrUntrustedUploadDestination is returned by [Client.NewUploadRequest] when an
+// upload would send its body to an origin this client was not configured for.
+//
+// Upload URLs are routinely read out of an API response — a release's UploadURL
+// is the usual case — so the host in one is chosen by whoever answered the
+// request rather than by the caller. A response naming a foreign host must not
+// be able to take the caller's bytes, and an error is the only safe answer:
+// sending the payload unauthenticated, the way a credential is withheld, would
+// report success for an upload that never reached GitHub.
 //
-// sameOrigin is the one predicate behind that rule. Every place that decides
-// whether a destination may receive credentials — the WithAuthToken wrapper,
-// both exported auth transports, and the redirect guards — must use it, so the
-// answer cannot differ depending on which code path a request happens to take.
+// Configure the destination with [WithURLs] or [WithEnterpriseURLs] if an
+// upload legitimately belongs on a host other than BaseURL or UploadURL.
+var ErrUntrustedUploadDestination = errors.New("refusing to upload to a destination the client is not configured for")
 
 // defaultAuthOrigins are the origins credentials may be sent to when no
 // allowlist is configured: GitHub.com's API and upload hosts. An empty
@@ -809,6 +841,22 @@ func (c *Client) shouldAuthorizeRequest(u *url.URL) bool {
 	return sameOrigin(u, c.baseURL) || sameOrigin(u, c.uploadURL)
 }
 
+// checkUploadDestination returns [ErrUntrustedUploadDestination] when u is not
+// an origin this client may upload to.
+//
+// An upload carries the caller's payload to a URL that a response usually chose,
+// and unlike a credential a body cannot be withheld and then have the request
+// still mean anything. So the destination is refused outright rather than
+// quietly sent unauthenticated, and the refusal happens where the upload request
+// is built, before any byte of the body is written.
+func (c *Client) checkUploadDestination(u *url.URL) error {
+	if c.shouldAuthorizeRequest(u) {
+		return nil
+	}
+
+	return fmt.Errorf("%w: %v", ErrUntrustedUploadDestination, u.Redacted())
+}
+
 // UserAgent returns the User-Agent header value for the client.
 func (c *Client) UserAgent() string {
 	return c.userAgent
@@ -1043,6 +1091,13 @@ func checkURLPathTraversal(urlStr string) error {
 // NewUploadRequest creates an upload request. A relative URL can be provided in
 // urlStr, in which case it is resolved relative to the UploadURL of the Client.
 // Relative URLs should always be specified without a preceding slash.
+//
+// An absolute urlStr replaces the Client's UploadURL, and the host in one of
+// those is usually chosen by an API response rather than by the caller — a
+// release's UploadURL is the usual case. The request is therefore refused with
+// [ErrUntrustedUploadDestination] unless it targets an origin this Client was
+// configured for, so that a response cannot redirect the caller's bytes to a
+// host the caller did not choose.
 func (c *Client) NewUploadRequest(ctx context.Context, urlStr string, reader io.Reader, size int64, mediaType string, opts ...RequestOption) (*http.Request, error) {
 	if !strings.HasSuffix(c.uploadURL.Path, "/") {
 		return nil, fmt.Errorf("uploadURL must have a trailing slash, but %q does not", c.uploadURL)
@@ -1057,6 +1112,13 @@ func (c *Client) NewUploadRequest(ctx context.Context, urlStr string, reader io.
 		return nil, err
 	}
 
+	// Checked here, at the one place an upload request is built, rather than at
+	// each call site: a new upload helper inherits the rule without having to
+	// remember it, and the refusal lands before any of the body is written.
+	if err := c.checkUploadDestination(u); err != nil {
+		return nil, err
+	}
+
 	requestBody := reader
 	if reader != nil {
 		// Wrap the provided reader so transport code does not observe concrete body types
diff --git a/github/github_test.go b/github/github_test.go
index f233ad11c8f..6e413901b68 100644
--- a/github/github_test.go
+++ b/github/github_test.go
@@ -1746,7 +1746,7 @@ func TestNewUploadRequest_setsGetBodyForSeekableReader(t *testing.T) {
 	const content = "upload content"
 	file := openTestFile(t, "upload.txt", content)
 
-	req, err := c.NewUploadRequest(t.Context(), "https://example.com/", file, int64(len(content)), "text/plain")
+	req, err := c.NewUploadRequest(t.Context(), "repos/o/r/releases/1/assets", file, int64(len(content)), "text/plain")
 	if err != nil {
 		t.Fatalf("NewUploadRequest returned unexpected error: %v", err)
 	}
@@ -1815,7 +1815,7 @@ func TestNewUploadRequest_noGetBodyWithoutReaderAt(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			req, err := c.NewUploadRequest(t.Context(), "https://example.com/", tt.reader, 14, "text/plain")
+			req, err := c.NewUploadRequest(t.Context(), "repos/o/r/releases/1/assets", tt.reader, 14, "text/plain")
 			if err != nil {
 				t.Fatalf("NewUploadRequest returned unexpected error: %v", err)
 			}
@@ -1838,7 +1838,7 @@ func TestNewUploadRequest_returnsErrorWhenSeekFails(t *testing.T) {
 		t.Fatalf("closing test file: %v", err)
 	}
 
-	_, err := c.NewUploadRequest(t.Context(), "https://example.com/", file, int64(len(content)), "text/plain")
+	_, err := c.NewUploadRequest(t.Context(), "repos/o/r/releases/1/assets", file, int64(len(content)), "text/plain")
 	if err == nil {
 		t.Error("NewUploadRequest returned nil error when Seek failed, want error")
 	}
@@ -1950,14 +1950,14 @@ func TestNewFormRequest_errorForNoTrailingSlash(t *testing.T) {
 func TestNewUploadRequest_WithVersion(t *testing.T) {
 	t.Parallel()
 	c := mustNewClient(t)
-	req, _ := c.NewUploadRequest(t.Context(), "https://example.com/", nil, 0, "")
+	req, _ := c.NewUploadRequest(t.Context(), "repos/o/r/releases/1/assets", nil, 0, "")
 
 	apiVersion := req.Header.Get(headerAPIVersion)
 	if got, want := apiVersion, api20221128; got != want {
 		t.Errorf("NewRequest() %v header is %v, want %v", headerAPIVersion, got, want)
 	}
 
-	req, _ = c.NewUploadRequest(t.Context(), "https://example.com/", nil, 0, "", WithVersion("2022-11-29"))
+	req, _ = c.NewUploadRequest(t.Context(), "repos/o/r/releases/1/assets", nil, 0, "", WithVersion("2022-11-29"))
 	apiVersion = req.Header.Get(headerAPIVersion)
 	if got, want := apiVersion, "2022-11-29"; got != want {
 		t.Errorf("NewRequest() %v header is %v, want %v", headerAPIVersion, got, want)
@@ -2001,6 +2001,77 @@ func TestNewUploadRequest_errorForNoTrailingSlash(t *testing.T) {
 	}
 }
 
+// TestNewUploadRequest_rejectsUnconfiguredDestination covers the rule that an
+// upload carrying the caller's bytes is refused unless it targets an origin the
+// client was configured for. An upload URL routinely comes out of a response --
+// a release's UploadURL is the usual case -- so the host in one is chosen by
+// whoever answered the request rather than by the caller.
+func TestNewUploadRequest_rejectsUnconfiguredDestination(t *testing.T) {
+	t.Parallel()
+	c := mustNewClient(t)
+
+	tests := []struct {
+		name   string
+		rawurl string
+	}{
+		{"foreign https host", "https://evil.example.com/upload"},
+		{"foreign http host", "http://evil.example.com/upload"},
+		{"scheme downgrade of a configured host", "http://uploads.github.com/upload"},
+		{"configured host as a prefix of the real host", "https://uploads.github.com.evil.example.com/upload"},
+		{"host that merely ends with the configured name", "https://evil-uploads.github.com/upload"},
+		{"subdomain of a configured host", "https://cdn.uploads.github.com/upload"},
+		{"non-default port on a configured host", "https://uploads.github.com:8443/upload"},
+		{"userinfo naming a configured host", "https://uploads.github.com@evil.example.com/upload"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			_, err := c.NewUploadRequest(t.Context(), tt.rawurl, strings.NewReader("x"), 1, "text/plain")
+			if !errors.Is(err, ErrUntrustedUploadDestination) {
+				t.Fatalf("NewUploadRequest(%q): want ErrUntrustedUploadDestination, got %v", tt.rawurl, err)
+			}
+		})
+	}
+}
+
+// TestNewUploadRequest_allowsConfiguredDestination covers the uploads that must
+// keep working: the origins the client was configured for, whether that is the
+// GitHub.com defaults or a GitHub Enterprise or proxy deployment.
+func TestNewUploadRequest_allowsConfiguredDestination(t *testing.T) {
+	t.Parallel()
+
+	ghe := mustNewClient(t, WithEnterpriseURLs("https://ghe.example.com/", "https://uploads.ghe.example.com/"))
+
+	tests := []struct {
+		name   string
+		client *Client
+		rawurl string
+	}{
+		{"relative path resolves against the configured upload origin", mustNewClient(t), "repos/o/r/releases/1/assets"},
+		{"absolute upload origin", mustNewClient(t), "https://uploads.github.com/repos/o/r/releases/1/assets"},
+		{"absolute API origin", mustNewClient(t), "https://api.github.com/repos/o/r/releases/1/assets"},
+		{"userinfo that does not change the real destination", mustNewClient(t), "https://evil.example.com@uploads.github.com/repos/o/r/releases/1/assets"},
+		{"enterprise upload origin", ghe, "https://uploads.ghe.example.com/api/uploads/repos/o/r/releases/1/assets"},
+		{"enterprise API origin", ghe, "https://ghe.example.com/api/v3/repos/o/r/releases/1/assets"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			if _, err := tt.client.NewUploadRequest(t.Context(), tt.rawurl, strings.NewReader("x"), 1, "text/plain"); err != nil {
+				t.Fatalf("NewUploadRequest(%q) returned unexpected error: %v", tt.rawurl, err)
+			}
+		})
+	}
+
+	// An enterprise client is scoped to its own origins: carrying the same rule
+	// over from the default configuration would widen it, not narrow it.
+	if _, err := ghe.NewUploadRequest(t.Context(), "https://uploads.github.com/repos/o/r/releases/1/assets", strings.NewReader("x"), 1, "text/plain"); !errors.Is(err, ErrUntrustedUploadDestination) {
+		t.Fatalf("enterprise client accepted the GitHub.com upload origin: %v", err)
+	}
+}
+
 func TestResponse_populatePageValues(t *testing.T) {
 	t.Parallel()
 	r := http.Response{
diff --git a/github/repos_contents.go b/github/repos_contents.go
index 4c9cb27f5b7..dc6b4a03c1e 100644
--- a/github/repos_contents.go
+++ b/github/repos_contents.go
@@ -192,6 +192,18 @@ func (s *RepositoriesService) DownloadContentsWithMeta(ctx context.Context, owne
 		return nil, fileContent, resp, ErrContentsNoDownloadURL
 	}
 
+	// download_url is a value the API chose, not the caller, so it is in the same
+	// family as a release's UploadURL. The two are not handled the same way, and
+	// the difference is what travels: this request carries no body of the
+	// caller's, and a download link is cross-origin by design — GitHub serves
+	// file content from raw.githubusercontent.com, and asset downloads from a
+	// pre-signed CDN host — so refusing a foreign origin here would break the
+	// ordinary path rather than a dangerous one. What the rules do cover is the
+	// credential: s.client's token is attached only to this client's configured
+	// origins, so a link naming another host is fetched without it. The residual
+	// is that the bytes come from whatever host the response named; a caller that
+	// needs the content to be provably GitHub's should read it from the API
+	// response itself.
 	dlReq, err := http.NewRequestWithContext(ctx, "GET", downloadURL, nil)
 	if err != nil {
 		return nil, fileContent, resp, err
diff --git a/github/repos_releases.go b/github/repos_releases.go
index 3c676609216..1faaafcbb2e 100644
--- a/github/repos_releases.go
+++ b/github/repos_releases.go
@@ -476,6 +476,13 @@ func (s *RepositoriesService) UploadReleaseAsset(ctx context.Context, owner, rep
 // templated like "https://uploads.github.com/.../assets{?name,label}") and uploads
 // the provided data (reader + size) using the existing upload helpers.
 //
+// Because release is normally the object an API call returned, its UploadURL is
+// a value the server chose rather than one the caller did. A release whose
+// UploadURL names an origin the client was not configured for is refused with
+// [ErrUntrustedUploadDestination] rather than uploaded to, so that a response
+// cannot take the artifact to a host of its own choosing. Configure a
+// legitimate alternate upload host with [WithURLs] or [WithEnterpriseURLs].
+//
 // GitHub API docs: https://docs.github.com/rest/releases/assets?apiVersion=2022-11-28#upload-a-release-asset
 //
 //meta:operation POST /repos/{owner}/{repo}/releases/{release_id}/assets
@@ -505,13 +512,14 @@ func (s *RepositoriesService) UploadReleaseAssetFromRelease(
 	// If this is a *relative* URL (no scheme), normalize it by trimming a leading "/"
 	// so it works with Client.BaseURL path prefixes (e.g. "/api-v3/").
 	//
-	// An absolute URL replaces the client's configured upload host entirely.
-	// That is deliberately left to the client's transport, which attaches the
-	// caller's Authorization header only to the client's configured API and
-	// upload origins: a response naming a foreign host therefore cannot take
-	// the token with it, and does not need a host check here. In the default
-	// configuration the upload host is uploads.github.com rather than
-	// api.github.com, so both are configured origins the token may reach.
+	// An absolute URL replaces the client's configured upload host entirely, and
+	// release.UploadURL is normally whatever the API last answered with, so the
+	// host in it is not the caller's choice. NewUploadRequest refuses that case
+	// with ErrUntrustedUploadDestination unless it names a configured origin, so
+	// there is no host check here: every upload this helper builds goes through
+	// that one gate. In the default configuration the upload host is
+	// uploads.github.com rather than api.github.com, and both are configured
+	// origins, so the URL the API hands back is accepted as-is.
 	if !strings.HasPrefix(uploadURL, "http://") && !strings.HasPrefix(uploadURL, "https://") {
 		uploadURL = strings.TrimPrefix(uploadURL, "/")
 	}
diff --git a/github/repos_releases_test.go b/github/repos_releases_test.go
index 69e57d0d395..e9a18ce79a4 100644
--- a/github/repos_releases_test.go
+++ b/github/repos_releases_test.go
@@ -7,6 +7,7 @@ package github
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -851,20 +852,20 @@ func TestRepositoriesService_UploadReleaseAssetFromRelease_AbsoluteTemplate(t *t
 	}
 }
 
-func TestRepositoriesService_UploadReleaseAssetFromRelease_ForeignHostGetsNoCredentials(t *testing.T) {
+func TestRepositoriesService_UploadReleaseAssetFromRelease_ForeignHostIsRejected(t *testing.T) {
 	t.Parallel()
 	client, _, _ := setup(t)
 
-	// A server that hands out an absolute upload URL naming a different host must not
-	// be able to take the caller's credentials with it: the client attaches its token
-	// only to its own configured origins. The upload itself is still attempted - the
-	// policy is to send such a request unauthenticated rather than to reject it - so
-	// the body does reach the foreign host, and the request must arrive with no
-	// Authorization header.
-	authHeaders := make(chan string, 1)
-	foreign := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	// A response that hands out an absolute upload URL naming a different host must
+	// not be able to take the caller's artifact with it. Withholding only the
+	// credential would not be enough here: the body would still arrive, and the call
+	// would return a nil error and a *ReleaseAsset, telling the caller the upload
+	// succeeded when the bytes went somewhere the caller never configured. So the
+	// destination is refused instead, and the foreign host is never contacted at all.
+	reached := make(chan struct{}, 1)
+	foreign := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		select {
-		case authHeaders <- r.Header.Get("Authorization"):
+		case reached <- struct{}{}:
 		default:
 		}
 		fmt.Fprint(w, `{"id":1}`)
@@ -882,19 +883,53 @@ func TestRepositoriesService_UploadReleaseAssetFromRelease_ForeignHostGetsNoCred
 
 	release := &RepositoryRelease{UploadURL: foreign.URL + "/upload{?name,label}"}
 	ctx := t.Context()
-	if _, _, err := authedClient.Repositories.UploadReleaseAssetFromRelease(
+	asset, _, err := authedClient.Repositories.UploadReleaseAssetFromRelease(
 		ctx, release, &UploadOptions{Name: "n.txt"}, reader, size,
-	); err != nil {
-		t.Fatalf("UploadReleaseAssetFromRelease returned error: %v", err)
+	)
+	if !errors.Is(err, ErrUntrustedUploadDestination) {
+		t.Fatalf("UploadReleaseAssetFromRelease to a foreign host: want ErrUntrustedUploadDestination, got err=%v", err)
+	}
+	if asset != nil {
+		t.Errorf("UploadReleaseAssetFromRelease returned asset %+v for a refused upload, want nil", asset)
 	}
 
 	select {
-	case got := <-authHeaders:
-		if got != "" {
-			t.Fatalf("upload to a foreign host carried Authorization %q; the token must never be sent there", got)
-		}
+	case <-reached:
+		t.Fatal("the artifact was sent to a foreign host; the upload must be refused before any byte is written")
 	default:
-		t.Fatal("upload never reached the foreign host")
+	}
+}
+
+func TestRepositoriesService_UploadReleaseAssetFromRelease_ConfiguredUploadHost(t *testing.T) {
+	t.Parallel()
+	client, mux, _ := setup(t)
+
+	// The counterpart to the rejection above: an absolute URL naming the client's own
+	// configured upload origin is the ordinary case and must still be uploaded to.
+	mux.HandleFunc("/repos/o/r/releases/1/assets", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "POST")
+		testFormValues(t, r, values{"name": "cfg.txt"})
+		testPlainBody(t, r, "Upload me !\n")
+		fmt.Fprint(w, `{"id":1}`)
+	})
+
+	body := []byte("Upload me !\n")
+	reader := bytes.NewReader(body)
+	size := int64(len(body))
+
+	uploadURL := client.uploadURL.String() + "repos/o/r/releases/1/assets{?name,label}"
+	release := &RepositoryRelease{UploadURL: uploadURL}
+
+	ctx := t.Context()
+	asset, _, err := client.Repositories.UploadReleaseAssetFromRelease(
+		ctx, release, &UploadOptions{Name: "cfg.txt"}, reader, size,
+	)
+	if err != nil {
+		t.Fatalf("UploadReleaseAssetFromRelease returned error: %v", err)
+	}
+	want := &ReleaseAsset{ID: new(int64(1))}
+	if !cmp.Equal(asset, want) {
+		t.Fatalf("UploadReleaseAssetFromRelease returned %+v, want %+v", asset, want)
 	}
 }
 

From c32fb0b66595bf88658ab3c68ef440218fe35347 Mon Sep 17 00:00:00 2001
From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Date: Thu, 17 Sep 2026 17:19:34 -0400
Subject: [PATCH 4/7] Add missing check; rename error

Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
---
 github/github.go              | 73 +++++++++++++++++++++--------------
 github/github_test.go         | 28 ++++++++++++--
 github/repos_releases.go      | 10 +++--
 github/repos_releases_test.go |  4 +-
 4 files changed, 77 insertions(+), 38 deletions(-)

diff --git a/github/github.go b/github/github.go
index 01b41180c24..5b1fa27049a 100644
--- a/github/github.go
+++ b/github/github.go
@@ -746,11 +746,12 @@ func newClient(opts clientOptions) (*Client, error) {
 // download URL handed back by the API is the ordinary case.
 //
 // A payload cannot be withheld that way, because a request whose body is dropped
-// cannot succeed at all, and the caller would be told an upload succeeded when
-// nothing was stored. The rule for a body is therefore absolute:
-// [Client.NewUploadRequest] refuses to build an upload aimed at an origin this
-// client was not configured for, so the caller's bytes are never sent to a host
-// the caller did not choose. See [ErrUntrustedUploadDestination].
+// cannot succeed at all, and the caller would be told a request succeeded when
+// nothing was stored. The rule for a body is therefore absolute: the two
+// constructors that build one — [Client.NewUploadRequest] and
+// [Client.NewFormRequest] — refuse to aim it at an origin this client was not
+// configured for, so the caller's bytes are never sent to a host the caller did
+// not choose. See [ErrUntrustedDestination].
 //
 // Within a Client both rules read the same two configured origins, BaseURL and
 // UploadURL, which is where [WithEnterpriseURLs] and [WithURLs] put them; a
@@ -763,19 +764,20 @@ func newClient(opts clientOptions) (*Client, error) {
 // so the answer cannot differ depending on which code path a request happens to
 // take.
 
-// ErrUntrustedUploadDestination is returned by [Client.NewUploadRequest] when an
-// upload would send its body to an origin this client was not configured for.
+// ErrUntrustedDestination is returned by [Client.NewUploadRequest] and
+// [Client.NewFormRequest] when a request they build would send its body to an
+// origin this client was not configured for.
 //
-// Upload URLs are routinely read out of an API response — a release's UploadURL
-// is the usual case — so the host in one is chosen by whoever answered the
-// request rather than by the caller. A response naming a foreign host must not
-// be able to take the caller's bytes, and an error is the only safe answer:
-// sending the payload unauthenticated, the way a credential is withheld, would
-// report success for an upload that never reached GitHub.
+// The URL for such a request is routinely read out of an API response — a
+// release's UploadURL is the usual case — so the host in one is chosen by
+// whoever answered the request rather than by the caller. A response naming a
+// foreign host must not be able to take the caller's bytes, and an error is the
+// only safe answer: sending the payload unauthenticated, the way a credential is
+// withheld, would report success for a request that never reached GitHub.
 //
-// Configure the destination with [WithURLs] or [WithEnterpriseURLs] if an
-// upload legitimately belongs on a host other than BaseURL or UploadURL.
-var ErrUntrustedUploadDestination = errors.New("refusing to upload to a destination the client is not configured for")
+// Configure the destination with [WithURLs] or [WithEnterpriseURLs] if a request
+// legitimately belongs on a host other than BaseURL or UploadURL.
+var ErrUntrustedDestination = errors.New("refusing to send a request body to a destination the client is not configured for")
 
 // defaultAuthOrigins are the origins credentials may be sent to when no
 // allowlist is configured: GitHub.com's API and upload hosts. An empty
@@ -841,20 +843,20 @@ func (c *Client) shouldAuthorizeRequest(u *url.URL) bool {
 	return sameOrigin(u, c.baseURL) || sameOrigin(u, c.uploadURL)
 }
 
-// checkUploadDestination returns [ErrUntrustedUploadDestination] when u is not
-// an origin this client may upload to.
+// checkBodyDestination returns [ErrUntrustedDestination] when u is not an origin
+// this client may send a request body to.
 //
-// An upload carries the caller's payload to a URL that a response usually chose,
-// and unlike a credential a body cannot be withheld and then have the request
-// still mean anything. So the destination is refused outright rather than
-// quietly sent unauthenticated, and the refusal happens where the upload request
+// Such a request carries the caller's payload to a URL that a response usually
+// chose, and unlike a credential a body cannot be withheld and then have the
+// request still mean anything. So the destination is refused outright rather
+// than quietly sent unauthenticated, and the refusal happens where the request
 // is built, before any byte of the body is written.
-func (c *Client) checkUploadDestination(u *url.URL) error {
+func (c *Client) checkBodyDestination(u *url.URL) error {
 	if c.shouldAuthorizeRequest(u) {
 		return nil
 	}
 
-	return fmt.Errorf("%w: %v", ErrUntrustedUploadDestination, u.Redacted())
+	return fmt.Errorf("%w: %v", ErrUntrustedDestination, u.Redacted())
 }
 
 // UserAgent returns the User-Agent header value for the client.
@@ -1038,6 +1040,11 @@ func (c *Client) NewRequest(ctx context.Context, method, urlStr string, body any
 // in which case it is resolved relative to the BaseURL of the Client.
 // Relative URLs should always be specified without a preceding slash.
 // Body is sent with Content-Type: application/x-www-form-urlencoded.
+//
+// An absolute urlStr replaces the BaseURL, so the request is refused with
+// [ErrUntrustedDestination] unless it targets an origin this Client was
+// configured for. This constructor carries a body, and a body cannot be withheld
+// the way a credential can: see the destination rules on [ErrUntrustedDestination].
 func (c *Client) NewFormRequest(ctx context.Context, urlStr string, body io.Reader, opts ...RequestOption) (*http.Request, error) {
 	if !strings.HasSuffix(c.baseURL.Path, "/") {
 		return nil, fmt.Errorf("baseURL must have a trailing slash, but %q does not", c.baseURL)
@@ -1052,6 +1059,14 @@ func (c *Client) NewFormRequest(ctx context.Context, urlStr string, body io.Read
 		return nil, err
 	}
 
+	// The same gate as NewUploadRequest, for the same reason: this builds a
+	// request that carries the caller's bytes, and an absolute urlStr is a
+	// destination a response could have supplied. Today's only caller passes a
+	// relative path, so this is a guard against the next one rather than a fix.
+	if err := c.checkBodyDestination(u); err != nil {
+		return nil, err
+	}
+
 	req, err := http.NewRequestWithContext(ctx, "POST", u.String(), body)
 	if err != nil {
 		return nil, err
@@ -1095,7 +1110,7 @@ func checkURLPathTraversal(urlStr string) error {
 // An absolute urlStr replaces the Client's UploadURL, and the host in one of
 // those is usually chosen by an API response rather than by the caller — a
 // release's UploadURL is the usual case. The request is therefore refused with
-// [ErrUntrustedUploadDestination] unless it targets an origin this Client was
+// [ErrUntrustedDestination] unless it targets an origin this Client was
 // configured for, so that a response cannot redirect the caller's bytes to a
 // host the caller did not choose.
 func (c *Client) NewUploadRequest(ctx context.Context, urlStr string, reader io.Reader, size int64, mediaType string, opts ...RequestOption) (*http.Request, error) {
@@ -1112,10 +1127,10 @@ func (c *Client) NewUploadRequest(ctx context.Context, urlStr string, reader io.
 		return nil, err
 	}
 
-	// Checked here, at the one place an upload request is built, rather than at
-	// each call site: a new upload helper inherits the rule without having to
-	// remember it, and the refusal lands before any of the body is written.
-	if err := c.checkUploadDestination(u); err != nil {
+	// The same gate as NewFormRequest, for the same reason: this builds a
+	// request that carries the caller's bytes, and an absolute urlStr is a
+	// destination a response could have supplied.
+	if err := c.checkBodyDestination(u); err != nil {
 		return nil, err
 	}
 
diff --git a/github/github_test.go b/github/github_test.go
index 6e413901b68..d597ccd094d 100644
--- a/github/github_test.go
+++ b/github/github_test.go
@@ -1729,6 +1729,28 @@ func TestNewFormRequest_pathTraversal(t *testing.T) {
 	}
 }
 
+// TestNewFormRequest_rejectsUnconfiguredDestination covers the same rule as
+// TestNewUploadRequest_rejectsUnconfiguredDestination, for the other constructor
+// that builds a body-carrying request. Its only call site passes a relative path
+// today, so this guards the next caller rather than a present exposure: an
+// absolute urlStr is a destination a response could have supplied, and the body
+// cannot be withheld the way a credential can.
+func TestNewFormRequest_rejectsUnconfiguredDestination(t *testing.T) {
+	t.Parallel()
+	c := mustNewClient(t)
+
+	_, err := c.NewFormRequest(t.Context(), "https://evil.example.com/hub", strings.NewReader("a=b"))
+	if !errors.Is(err, ErrUntrustedDestination) {
+		t.Fatalf("NewFormRequest to a foreign host: want ErrUntrustedDestination, got %v", err)
+	}
+
+	// A relative path resolves against BaseURL, which is always configured, so
+	// the ordinary call keeps working.
+	if _, err := c.NewFormRequest(t.Context(), "hub", strings.NewReader("a=b")); err != nil {
+		t.Fatalf("NewFormRequest with a relative path returned unexpected error: %v", err)
+	}
+}
+
 func TestNewUploadRequest_pathTraversal(t *testing.T) {
 	t.Parallel()
 	c := mustNewClient(t)
@@ -2028,8 +2050,8 @@ func TestNewUploadRequest_rejectsUnconfiguredDestination(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 			_, err := c.NewUploadRequest(t.Context(), tt.rawurl, strings.NewReader("x"), 1, "text/plain")
-			if !errors.Is(err, ErrUntrustedUploadDestination) {
-				t.Fatalf("NewUploadRequest(%q): want ErrUntrustedUploadDestination, got %v", tt.rawurl, err)
+			if !errors.Is(err, ErrUntrustedDestination) {
+				t.Fatalf("NewUploadRequest(%q): want ErrUntrustedDestination, got %v", tt.rawurl, err)
 			}
 		})
 	}
@@ -2067,7 +2089,7 @@ func TestNewUploadRequest_allowsConfiguredDestination(t *testing.T) {
 
 	// An enterprise client is scoped to its own origins: carrying the same rule
 	// over from the default configuration would widen it, not narrow it.
-	if _, err := ghe.NewUploadRequest(t.Context(), "https://uploads.github.com/repos/o/r/releases/1/assets", strings.NewReader("x"), 1, "text/plain"); !errors.Is(err, ErrUntrustedUploadDestination) {
+	if _, err := ghe.NewUploadRequest(t.Context(), "https://uploads.github.com/repos/o/r/releases/1/assets", strings.NewReader("x"), 1, "text/plain"); !errors.Is(err, ErrUntrustedDestination) {
 		t.Fatalf("enterprise client accepted the GitHub.com upload origin: %v", err)
 	}
 }
diff --git a/github/repos_releases.go b/github/repos_releases.go
index 1faaafcbb2e..44c021ad14b 100644
--- a/github/repos_releases.go
+++ b/github/repos_releases.go
@@ -479,9 +479,11 @@ func (s *RepositoriesService) UploadReleaseAsset(ctx context.Context, owner, rep
 // Because release is normally the object an API call returned, its UploadURL is
 // a value the server chose rather than one the caller did. A release whose
 // UploadURL names an origin the client was not configured for is refused with
-// [ErrUntrustedUploadDestination] rather than uploaded to, so that a response
-// cannot take the artifact to a host of its own choosing. Configure a
-// legitimate alternate upload host with [WithURLs] or [WithEnterpriseURLs].
+// [ErrUntrustedDestination] rather than uploaded to, so that a response cannot
+// take the artifact to a host of its own choosing. This function performs no
+// host check of its own: the refusal comes from [Client.NewUploadRequest], which
+// every upload here is built through. Configure a legitimate alternate upload
+// host with [WithURLs] or [WithEnterpriseURLs].
 //
 // GitHub API docs: https://docs.github.com/rest/releases/assets?apiVersion=2022-11-28#upload-a-release-asset
 //
@@ -515,7 +517,7 @@ func (s *RepositoriesService) UploadReleaseAssetFromRelease(
 	// An absolute URL replaces the client's configured upload host entirely, and
 	// release.UploadURL is normally whatever the API last answered with, so the
 	// host in it is not the caller's choice. NewUploadRequest refuses that case
-	// with ErrUntrustedUploadDestination unless it names a configured origin, so
+	// with ErrUntrustedDestination unless it names a configured origin, so
 	// there is no host check here: every upload this helper builds goes through
 	// that one gate. In the default configuration the upload host is
 	// uploads.github.com rather than api.github.com, and both are configured
diff --git a/github/repos_releases_test.go b/github/repos_releases_test.go
index e9a18ce79a4..ea02d688112 100644
--- a/github/repos_releases_test.go
+++ b/github/repos_releases_test.go
@@ -886,8 +886,8 @@ func TestRepositoriesService_UploadReleaseAssetFromRelease_ForeignHostIsRejected
 	asset, _, err := authedClient.Repositories.UploadReleaseAssetFromRelease(
 		ctx, release, &UploadOptions{Name: "n.txt"}, reader, size,
 	)
-	if !errors.Is(err, ErrUntrustedUploadDestination) {
-		t.Fatalf("UploadReleaseAssetFromRelease to a foreign host: want ErrUntrustedUploadDestination, got err=%v", err)
+	if !errors.Is(err, ErrUntrustedDestination) {
+		t.Fatalf("UploadReleaseAssetFromRelease to a foreign host: want ErrUntrustedDestination, got err=%v", err)
 	}
 	if asset != nil {
 		t.Errorf("UploadReleaseAssetFromRelease returned asset %+v for a refused upload, want nil", asset)

From 534d41ee576bbb074f143042c880dac020da8d43 Mon Sep 17 00:00:00 2001
From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Date: Fri, 18 Sep 2026 08:28:06 -0400
Subject: [PATCH 5/7] Apply suggestion from @Not-Dhananjay-Mishra

Co-authored-by: Dhananjay Mishra <technicaldmcontact@gmail.com>
---
 github/github_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/github/github_test.go b/github/github_test.go
index d597ccd094d..c967504df93 100644
--- a/github/github_test.go
+++ b/github/github_test.go
@@ -2073,7 +2073,7 @@ func TestNewUploadRequest_allowsConfiguredDestination(t *testing.T) {
 		{"relative path resolves against the configured upload origin", mustNewClient(t), "repos/o/r/releases/1/assets"},
 		{"absolute upload origin", mustNewClient(t), "https://uploads.github.com/repos/o/r/releases/1/assets"},
 		{"absolute API origin", mustNewClient(t), "https://api.github.com/repos/o/r/releases/1/assets"},
-		{"userinfo that does not change the real destination", mustNewClient(t), "https://evil.example.com@uploads.github.com/repos/o/r/releases/1/assets"},
+		{"userinfo that does not change the real destination", mustNewClient(t), "https://example.com@uploads.github.com/repos/o/r/releases/1/assets"},
 		{"enterprise upload origin", ghe, "https://uploads.ghe.example.com/api/uploads/repos/o/r/releases/1/assets"},
 		{"enterprise API origin", ghe, "https://ghe.example.com/api/v3/repos/o/r/releases/1/assets"},
 	}

From 7fb8b8734bc63809f6c8638b8fa9896119962ab1 Mon Sep 17 00:00:00 2001
From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Date: Fri, 18 Sep 2026 08:29:10 -0400
Subject: [PATCH 6/7] Apply suggestion from @Not-Dhananjay-Mishra

Co-authored-by: Dhananjay Mishra <technicaldmcontact@gmail.com>
---
 github/github_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/github/github_test.go b/github/github_test.go
index c967504df93..21d1cc8e776 100644
--- a/github/github_test.go
+++ b/github/github_test.go
@@ -4798,7 +4798,7 @@ func TestBasicAuthTransport_originScope(t *testing.T) {
 	})
 
 	// ...and any other origin receives neither. The OTP is a second factor, so a
-	// foreign origin must not see it even though it is not a "credential" per se.
+	// foreign origin must not see it even though it is not itself a "credential".
 	foreign, foreignMux, _ := setup(t)
 	foreignMux.HandleFunc("/", func(_ http.ResponseWriter, r *http.Request) {
 		if u, p, ok := r.BasicAuth(); ok {

From 4452477d1b6c4e6e125d7ec5426e64ef2de5f337 Mon Sep 17 00:00:00 2001
From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Date: Fri, 18 Sep 2026 08:43:44 -0400
Subject: [PATCH 7/7] Address review feedback

Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
---
 github/copilot_test.go | 67 +++++++++++++++++++++++++++---------------
 github/github_test.go  | 31 +++++++++++++++++--
 2 files changed, 72 insertions(+), 26 deletions(-)

diff --git a/github/copilot_test.go b/github/copilot_test.go
index 6d61a3900d5..996ed014a4d 100644
--- a/github/copilot_test.go
+++ b/github/copilot_test.go
@@ -6,12 +6,12 @@
 package github
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"log"
 	"net/http"
 	"net/http/httptest"
-	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -3907,6 +3907,16 @@ func TestCopilotService_DownloadCopilotMetrics(t *testing.T) {
 	}
 }
 
+// downloadFunc adapts a Download*Metrics method whose decoded payload the table
+// below does not inspect into the error-only shape it runs. Each method returns a
+// different type, so the shared signature is what lets one case list them all.
+func downloadFunc[V any](f func(context.Context, string) (V, *Response, error)) func(context.Context, string) error {
+	return func(ctx context.Context, url string) error {
+		_, _, err := f(ctx, url)
+		return err
+	}
+}
+
 // TestCopilotService_DownloadMetrics_ForeignHostGetsNoCredentials covers the
 // download helpers whose URL comes straight out of a report response, and which
 // therefore may name any host: DownloadCopilotMetrics, and the fetchMetricsReport
@@ -3919,36 +3929,47 @@ func TestCopilotService_DownloadMetrics_ForeignHostGetsNoCredentials(t *testing.
 	t.Parallel()
 	client, _, _ := setup(t)
 
-	auth := make(chan string, 1)
-	foreign := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		select {
-		case auth <- r.Header.Get("Authorization"):
-		default:
-		}
-		if strings.HasSuffix(r.URL.Path, "/metrics") {
-			// DownloadCopilotMetrics decodes an array...
-			fmt.Fprint(w, `[]`)
-			return
-		}
-		// ...and DownloadDailyMetrics decodes an object.
-		fmt.Fprint(w, `{}`)
-	}))
-	t.Cleanup(foreign.Close)
-
 	authedClient, err := client.Clone(WithAuthToken("secret-token"))
 	if err != nil {
 		t.Fatalf("Client.Clone returned error: %v", err)
 	}
 
-	if _, _, err := authedClient.Copilot.DownloadDailyMetrics(t.Context(), foreign.URL+"/daily"); err != nil {
-		t.Fatalf("DownloadDailyMetrics returned error: %v", err)
+	tests := []struct {
+		name string
+		// payload is what the foreign host serves. The methods do not all decode
+		// the same shape, so each case carries one its method can parse: a case
+		// then fails on the header and not on a decode error.
+		payload  string
+		download func(ctx context.Context, url string) error
+	}{
+		{"DownloadCopilotMetrics decodes a JSON array", `[]`, downloadFunc(authedClient.Copilot.DownloadCopilotMetrics)},
+		{"DownloadDailyMetrics decodes a JSON object", `{}`, downloadFunc(authedClient.Copilot.DownloadDailyMetrics)},
+		{"DownloadPeriodicMetrics decodes a JSON object", `{}`, downloadFunc(authedClient.Copilot.DownloadPeriodicMetrics)},
+		{"DownloadUserDailyMetrics decodes NDJSON", `{}`, downloadFunc(authedClient.Copilot.DownloadUserDailyMetrics)},
+		{"DownloadUserPeriodicMetrics decodes NDJSON", `{}`, downloadFunc(authedClient.Copilot.DownloadUserPeriodicMetrics)},
+		{"DownloadRepositoryDailyMetrics decodes NDJSON", `{}`, downloadFunc(authedClient.Copilot.DownloadRepositoryDailyMetrics)},
+		{"DownloadUserTeamsDailyMetrics decodes NDJSON", `{}`, downloadFunc(authedClient.Copilot.DownloadUserTeamsDailyMetrics)},
 	}
-	assertRecordedAuthHeader(t, auth, "")
 
-	if _, _, err := authedClient.Copilot.DownloadCopilotMetrics(t.Context(), foreign.URL+"/metrics"); err != nil {
-		t.Fatalf("DownloadCopilotMetrics returned error: %v", err)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			auth := make(chan string, 1)
+			foreign := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				select {
+				case auth <- r.Header.Get("Authorization"):
+				default:
+				}
+				fmt.Fprint(w, tt.payload)
+			}))
+			t.Cleanup(foreign.Close)
+
+			if err := tt.download(t.Context(), foreign.URL+"/path/to/report"); err != nil {
+				t.Fatalf("download returned error: %v", err)
+			}
+			assertRecordedAuthHeader(t, auth, "")
+		})
 	}
-	assertRecordedAuthHeader(t, auth, "")
 }
 
 func TestCopilotService_DownloadDailyMetrics(t *testing.T) {
diff --git a/github/github_test.go b/github/github_test.go
index 21d1cc8e776..165a2d90841 100644
--- a/github/github_test.go
+++ b/github/github_test.go
@@ -1739,9 +1739,31 @@ func TestNewFormRequest_rejectsUnconfiguredDestination(t *testing.T) {
 	t.Parallel()
 	c := mustNewClient(t)
 
-	_, err := c.NewFormRequest(t.Context(), "https://evil.example.com/hub", strings.NewReader("a=b"))
-	if !errors.Is(err, ErrUntrustedDestination) {
-		t.Fatalf("NewFormRequest to a foreign host: want ErrUntrustedDestination, got %v", err)
+	tests := []struct {
+		name   string
+		rawurl string
+	}{
+		{"foreign https host", "https://evil.example.com/hub"},
+		{"foreign http host", "http://evil.example.com/hub"},
+		{"scheme downgrade of a configured host", "http://api.github.com/hub"},
+		{"configured host as a prefix of the real host", "https://api.github.com.evil.example.com/hub"},
+		{"host that merely ends with the configured name", "https://evil-api.github.com/hub"},
+		{"subdomain of a configured host", "https://cdn.api.github.com/hub"},
+		{"non-default port on a configured host", "https://api.github.com:8443/hub"},
+		// The userinfo is a decoy: the host the check reads is the one after the
+		// @, evil.example.com -- foreign, so the body is refused. The allow table
+		// has the mirror case, where the userinfo names a foreign host instead.
+		{"userinfo naming a configured host", "https://api.github.com@evil.example.com/hub"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			_, err := c.NewFormRequest(t.Context(), tt.rawurl, strings.NewReader("a=b"))
+			if !errors.Is(err, ErrUntrustedDestination) {
+				t.Fatalf("NewFormRequest(%q): want ErrUntrustedDestination, got %v", tt.rawurl, err)
+			}
+		})
 	}
 
 	// A relative path resolves against BaseURL, which is always configured, so
@@ -2043,6 +2065,9 @@ func TestNewUploadRequest_rejectsUnconfiguredDestination(t *testing.T) {
 		{"host that merely ends with the configured name", "https://evil-uploads.github.com/upload"},
 		{"subdomain of a configured host", "https://cdn.uploads.github.com/upload"},
 		{"non-default port on a configured host", "https://uploads.github.com:8443/upload"},
+		// The userinfo is a decoy: the host the check reads is the one after the
+		// @, evil.example.com -- foreign, so the body is refused. The allow table
+		// has the mirror case, where the userinfo names a foreign host instead.
 		{"userinfo naming a configured host", "https://uploads.github.com@evil.example.com/upload"},
 	}