fix: Reject a metrics download URL whose host differs from the client's

github/copilot.go

@@ -12,6 +12,8 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
+	"strings"
 	"time"
 )
 
@@ -1230,8 +1232,25 @@ func (s *CopilotService) GetOrganizationUserTeamsDailyMetricsReport(ctx context.
 // (see https://github.com/google/go-github/issues/4136).
 // This method is retained
 // for GitHub Enterprise Server installations that may still serve the legacy shape.
-func (s *CopilotService) DownloadCopilotMetrics(ctx context.Context, url string) ([]*CopilotMetrics, *Response, error) {
-	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
+//
+// downloadURL is a value the caller reads out of a prior Get*MetricsReport
+// response, not one it constructs itself. BareDo's auth transport attaches
+// the caller's Authorization header to every request it sends, regardless
+// of host, so a report response naming a foreign host must be rejected
+// before the request goes out. See fetchMetricsReport for the same guard.
+func (s *CopilotService) DownloadCopilotMetrics(ctx context.Context, downloadURL string) ([]*CopilotMetrics, *Response, error) {
+	parsed, err := url.Parse(downloadURL)
+	if err != nil {
+		return nil, nil, err
+	}
+	if !strings.EqualFold(parsed.Host, s.client.baseURL.Host) {
+		return nil, nil, fmt.Errorf(
+			"download URL host %v does not match the client's configured host %v",
+			parsed.Host, s.client.baseURL.Host,
+		)
+	}
+
+	req, err := http.NewRequestWithContext(ctx, "GET", downloadURL, nil)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -1617,8 +1636,28 @@ 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.
-func (s *CopilotService) fetchMetricsReport(ctx context.Context, url string) (*http.Response, *Response, error) {
-	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
+//
+// downloadURL is documented as a value the caller reads out of a prior
+// Get*MetricsReport response's DownloadLinks, not one it constructs itself.
+// The request below goes out through s.client.client, whose auth transport
+// attaches the caller's Authorization header to every request it sends,
+// regardless of host. Left unchecked, a report response naming a foreign
+// host would receive that header. Compare against the client's configured
+// host before the request is built, the same guard applied to UploadURL in
+// UploadReleaseAssetFromRelease and to redirects in bareDoUntilFound.
+func (s *CopilotService) fetchMetricsReport(ctx context.Context, downloadURL string) (*http.Response, *Response, error) {
+	parsed, err := url.Parse(downloadURL)
+	if err != nil {
+		return nil, nil, err
+	}
+	if !strings.EqualFold(parsed.Host, s.client.baseURL.Host) {
+		return nil, nil, fmt.Errorf(
+			"download URL host %v does not match the client's configured host %v",
+			parsed.Host, s.client.baseURL.Host,
+		)
+	}
+
+	req, err := http.NewRequestWithContext(ctx, "GET", downloadURL, nil)
 	if err != nil {
 		return nil, nil, err
 	}

github/copilot_test.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"log"
 	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -4159,6 +4160,86 @@ func TestCopilotService_fetchMetricsReport_closesOriginalBodyOnErrorResponse(t *
 	}
 }
 
+func TestCopilotService_fetchMetricsReport_ForeignHostIsRejected(t *testing.T) {
+	t.Parallel()
+	client, _, _ := setup(t)
+
+	// Simulate the auth transport that attaches the caller's credentials to
+	// every outgoing request, regardless of host: DownloadDailyMetrics takes
+	// a download link read out of a prior report response, not one the
+	// caller constructs, so a report response naming a foreign host must
+	// not be able to redirect that request - and the Authorization header
+	// riding on it - away from the client's configured host.
+	base := client.client.Transport
+	if base == nil {
+		base = http.DefaultTransport
+	}
+	client.client.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) {
+		req.Header.Set("Authorization", "Bearer super-secret-token")
+		return base.RoundTrip(req)
+	})
+
+	var leakedAuth string
+	evil := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		leakedAuth = r.Header.Get("Authorization")
+		fmt.Fprint(w, `[]`)
+	}))
+	t.Cleanup(evil.Close)
+
+	ctx := t.Context()
+	_, _, err := client.Copilot.DownloadDailyMetrics(ctx, evil.URL+"/path/to/daily")
+	if err == nil {
+		t.Fatal("Copilot.DownloadDailyMetrics expected an error for a foreign-host download URL, got nil")
+	}
+	if leakedAuth != "" {
+		t.Fatalf("Authorization header %q reached the foreign host; it must never be sent there", leakedAuth)
+	}
+}
+
+func TestCopilotService_DownloadCopilotMetrics_ForeignHostIsRejected(t *testing.T) {
+	t.Parallel()
+	client, _, _ := setup(t)
+
+	base := client.client.Transport
+	if base == nil {
+		base = http.DefaultTransport
+	}
+	client.client.Transport = roundTripperFunc(func(req *http.Request) (*http.Response, error) {
+		req.Header.Set("Authorization", "Bearer super-secret-token")
+		return base.RoundTrip(req)
+	})
+
+	var leakedAuth string
+	evil := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		leakedAuth = r.Header.Get("Authorization")
+		fmt.Fprint(w, `[]`)
+	}))
+	t.Cleanup(evil.Close)
+
+	ctx := t.Context()
+	_, _, err := client.Copilot.DownloadCopilotMetrics(ctx, evil.URL+"/path/to/download")
+	if err == nil {
+		t.Fatal("Copilot.DownloadCopilotMetrics expected an error for a foreign-host download URL, got nil")
+	}
+	if leakedAuth != "" {
+		t.Fatalf("Authorization header %q reached the foreign host; it must never be sent there", leakedAuth)
+	}
+}
+
+func TestCopilotService_fetchMetricsReport_MalformedDownloadURL(t *testing.T) {
+	t.Parallel()
+	client, _, _ := setup(t)
+
+	// net/url rejects ASCII control characters, so a report response naming
+	// such a URL must surface as an error rather than a panic or a request
+	// to an unchecked host.
+	ctx := t.Context()
+	_, _, err := client.Copilot.DownloadDailyMetrics(ctx, "https://example.com/\x7f/report")
+	if err == nil {
+		t.Fatal("Copilot.DownloadDailyMetrics expected an error for an unparsable download URL, got nil")
+	}
+}
+
 func TestCopilotService_DownloadPeriodicMetrics(t *testing.T) {
 	t.Parallel()
 	client, mux, _ := setup(t)