Attach GitHub token only to configured GitHub hosts

internal/ghmcp/server.go

@@ -62,6 +62,16 @@ func createGitHubClients(cfg github.MCPServerConfig, apiHost utils.APIHostResolv
 		return nil, fmt.Errorf("failed to get Raw URL: %w", err)
 	}
 
+	// allowedHosts scopes the bearer token to the configured GitHub hosts, so a
+	// response that redirects off them does not carry the token to the redirect
+	// target. See transport.BearerAuthTransport.
+	allowedHosts := []string{
+		restURL.Hostname(),
+		uploadURL.Hostname(),
+		graphQLURL.Hostname(),
+		rawURL.Hostname(),
+	}
+
 	// Construct REST client. When a TokenProvider is configured, we
 	// authenticate via BearerAuthTransport and skip go-github's WithAuthToken:
 	// the latter installs its own round tripper that would pin the static token
@@ -76,6 +86,7 @@ func createGitHubClients(cfg github.MCPServerConfig, apiHost utils.APIHostResolv
 			gogithub.WithHTTPClient(&http.Client{Transport: &transport.BearerAuthTransport{
 				Transport:     restUATransport,
 				TokenProvider: cfg.TokenProvider,
+				AllowedHosts:  allowedHosts,
 			}}),
 			gogithub.WithEnterpriseURLs(restURL.String(), uploadURL.String()),
 		)
@@ -99,6 +110,7 @@ func createGitHubClients(cfg github.MCPServerConfig, apiHost utils.APIHostResolv
 			},
 			Token:         cfg.Token,
 			TokenProvider: cfg.TokenProvider,
+			AllowedHosts:  allowedHosts,
 		},
 	}
 

pkg/github/dependencies.go

@@ -343,6 +343,33 @@ func (d *RequestDeps) GetGQLClient(ctx context.Context) (*githubv4.Client, error
 	}
 	token := tokenInfo.Token
 
+	baseRestURL, err := d.apiHosts.BaseRESTURL(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get base REST URL: %w", err)
+	}
+	uploadURL, err := d.apiHosts.UploadURL(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get upload URL: %w", err)
+	}
+	graphqlURL, err := d.apiHosts.GraphqlURL(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get GraphQL URL: %w", err)
+	}
+	rawURL, err := d.apiHosts.RawURL(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get Raw URL: %w", err)
+	}
+
+	// allowedHosts scopes the bearer token to the configured GitHub hosts, so a
+	// response that redirects off them does not carry the token to the redirect
+	// target. See transport.BearerAuthTransport.
+	allowedHosts := []string{
+		baseRestURL.Hostname(),
+		uploadURL.Hostname(),
+		graphqlURL.Hostname(),
+		rawURL.Hostname(),
+	}
+
 	// Construct GraphQL client
 	// We use NewEnterpriseClient unconditionally since we already parsed the API host
 	// Wrap transport with GraphQLFeaturesTransport to inject feature flags from context,
@@ -352,15 +379,11 @@ func (d *RequestDeps) GetGQLClient(ctx context.Context) (*githubv4.Client, error
 			Transport: &transport.GraphQLFeaturesTransport{
 				Transport: http.DefaultTransport,
 			},
-			Token: token,
+			Token:        token,
+			AllowedHosts: allowedHosts,
 		},
 	}
 
-	graphqlURL, err := d.apiHosts.GraphqlURL(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get GraphQL URL: %w", err)
-	}
-
 	gqlClient := githubv4.NewEnterpriseClient(graphqlURL.String(), gqlHTTPClient)
 	return gqlClient, nil
 }

pkg/http/transport/bearer.go

@@ -15,6 +15,22 @@ type BearerAuthTransport struct {
 	// TokenProvider, when non-nil, supplies the bearer token for each request
 	// and takes precedence over Token.
 	TokenProvider func() string
+
+	// AllowedHosts, when non-empty, restricts the hosts the Authorization
+	// header is attached to. The token is set only when the request host
+	// matches one of these entries (case-insensitive, host only, port
+	// ignored). This scopes the credential to the configured GitHub hosts, so
+	// that if a response redirects off them the token is not carried to the
+	// redirect target.
+	//
+	// net/http strips a cross-host Authorization header when it follows a
+	// redirect, but only for headers set on the initial request. This
+	// transport re-adds the header on every hop, so that protection does not
+	// otherwise apply here.
+	//
+	// When empty, the token is attached to every request, preserving the
+	// prior behavior.
+	AllowedHosts []string
 }
 
 func (t *BearerAuthTransport) RoundTrip(req *http.Request) (*http.Response, error) {
@@ -23,7 +39,7 @@ func (t *BearerAuthTransport) RoundTrip(req *http.Request) (*http.Response, erro
 	if t.TokenProvider != nil {
 		token = t.TokenProvider()
 	}
-	if token != "" {
+	if token != "" && t.hostAllowed(req.URL.Hostname()) {
 		req.Header.Set(headers.AuthorizationHeader, "Bearer "+token)
 	}
 
@@ -34,3 +50,17 @@ func (t *BearerAuthTransport) RoundTrip(req *http.Request) (*http.Response, erro
 
 	return t.Transport.RoundTrip(req)
 }
+
+// hostAllowed reports whether the token may be attached to a request bound for
+// host. An empty AllowedHosts allows all hosts, preserving prior behavior.
+func (t *BearerAuthTransport) hostAllowed(host string) bool {
+	if len(t.AllowedHosts) == 0 {
+		return true
+	}
+	for _, h := range t.AllowedHosts {
+		if strings.EqualFold(h, host) {
+			return true
+		}
+	}
+	return false
+}

pkg/http/transport/bearer_test.go

@@ -162,3 +162,82 @@ func TestBearerAuthTransport_DoesNotMutateOriginalRequest(t *testing.T) {
 
 	assert.Empty(t, req.Header.Get(headers.AuthorizationHeader), "original request must not be mutated")
 }
+
+// hostRecordingTransport records the Authorization header seen for each request
+// host, so a test can assert what the token would be attached to without a live
+// network. It stands in for the real transport at the bottom of the chain.
+type hostRecordingTransport struct {
+	authByHost map[string]string
+}
+
+func (h *hostRecordingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	h.authByHost[req.URL.Hostname()] = req.Header.Get(headers.AuthorizationHeader)
+	return &http.Response{
+		StatusCode: http.StatusOK,
+		Body:       http.NoBody,
+		Header:     make(http.Header),
+		Request:    req,
+	}, nil
+}
+
+// TestBearerAuthTransport_HostScoping verifies that when AllowedHosts is set,
+// the token is attached to a request on an allowed host but withheld from a
+// request to any other host. A redirect off the configured GitHub hosts arrives
+// here as a RoundTrip to a different host, so this is the property that keeps
+// the token from following such a redirect. net/http's own cross-host stripping
+// does not cover it, because this transport re-adds the header on every hop.
+//
+// The hosts are distinct hostnames (matching the real case: api.github.com
+// versus objects.githubusercontent.com) rather than two loopback servers on
+// different ports, because AllowedHosts matches on hostname and ignores port.
+func TestBearerAuthTransport_HostScoping(t *testing.T) {
+	t.Parallel()
+
+	rec := &hostRecordingTransport{authByHost: map[string]string{}}
+	rt := &BearerAuthTransport{
+		Transport:    rec,
+		Token:        "secret-token",
+		AllowedHosts: []string{"api.github.com", "raw.githubusercontent.com"},
+	}
+
+	for _, target := range []string{
+		"https://api.github.com/repos/o/r",
+		"https://raw.githubusercontent.com/o/r/main/f", // allowed, different host
+		"https://objects.githubusercontent.com/evil",   // redirect target, not allowed
+		"https://attacker.example.com/steal",           // arbitrary host, not allowed
+	} {
+		req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, target, nil)
+		require.NoError(t, err)
+		resp, err := rt.RoundTrip(req)
+		require.NoError(t, err)
+		resp.Body.Close()
+	}
+
+	assert.Equal(t, "Bearer secret-token", rec.authByHost["api.github.com"],
+		"token must be sent to an allowed host")
+	assert.Equal(t, "Bearer secret-token", rec.authByHost["raw.githubusercontent.com"],
+		"token must be sent to every allowed host")
+	assert.Empty(t, rec.authByHost["objects.githubusercontent.com"],
+		"token must not be sent to a non-allowed host (a redirect target)")
+	assert.Empty(t, rec.authByHost["attacker.example.com"],
+		"token must not be sent to an arbitrary non-allowed host")
+}
+
+// TestBearerAuthTransport_EmptyAllowedHostsPreservesBehavior verifies the
+// backward-compatible default: with no AllowedHosts, the token is attached to
+// every host, exactly as before this change.
+func TestBearerAuthTransport_EmptyAllowedHostsPreservesBehavior(t *testing.T) {
+	t.Parallel()
+
+	rec := &hostRecordingTransport{authByHost: map[string]string{}}
+	rt := &BearerAuthTransport{Transport: rec, Token: "secret-token"}
+
+	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "https://anywhere.example.com/x", nil)
+	require.NoError(t, err)
+	resp, err := rt.RoundTrip(req)
+	require.NoError(t, err)
+	resp.Body.Close()
+
+	assert.Equal(t, "Bearer secret-token", rec.authByHost["anywhere.example.com"],
+		"with no AllowedHosts, token attaches to every host as before")
+}