diff --git a/internal/ghmcp/oauth_test.go b/internal/ghmcp/oauth_test.go index b358876232..62cdf92166 100644 --- a/internal/ghmcp/oauth_test.go +++ b/internal/ghmcp/oauth_test.go @@ -576,8 +576,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() @@ -600,6 +602,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 12306e6a23..dce3b5e54a 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -67,7 +67,7 @@ func createGitHubClients(cfg github.MCPServerConfig, apiHost utils.APIHostResolv // the latter installs its own round tripper that would pin the static token // and shadow the dynamic one. restUATransport := &transport.UserAgentTransport{ - Transport: http.DefaultTransport, + Transport: &transport.APIVersionTransport{Transport: http.DefaultTransport}, Agent: fmt.Sprintf("github-mcp-server/%s", cfg.Version), } var restClient *gogithub.Client 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.github.com", "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 49b6f6315a..cd0ee6edb1 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -324,6 +324,9 @@ func (d *RequestDeps) GetClient(ctx context.Context) (*gogithub.Client, error) { // Construct REST client restClient, err := gogithub.NewClient( + gogithub.WithHTTPClient(&http.Client{ + Transport: &transport.APIVersionTransport{Transport: http.DefaultTransport}, + }), gogithub.WithAuthToken(token), gogithub.WithUserAgent(fmt.Sprintf("github-mcp-server/%s", d.version)), gogithub.WithEnterpriseURLs(baseRestURL.String(), uploadURL.String()), diff --git a/pkg/github/dependencies_test.go b/pkg/github/dependencies_test.go index 1d747cae47..6c24254654 100644 --- a/pkg/github/dependencies_test.go +++ b/pkg/github/dependencies_test.go @@ -4,20 +4,65 @@ import ( "context" "errors" "log/slog" + "net/http" + "net/http/httptest" + "net/url" "testing" + ghcontext "github.com/github/github-mcp-server/pkg/context" "github.com/github/github-mcp-server/pkg/github" + "github.com/github/github-mcp-server/pkg/http/headers" "github.com/github/github-mcp-server/pkg/observability" "github.com/github/github-mcp-server/pkg/observability/metrics" "github.com/github/github-mcp-server/pkg/translations" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +type requestDepsAPIHost struct { + url *url.URL +} + +func (h requestDepsAPIHost) BaseRESTURL(context.Context) (*url.URL, error) { return h.url, nil } +func (h requestDepsAPIHost) GraphqlURL(context.Context) (*url.URL, error) { return h.url, nil } +func (h requestDepsAPIHost) UploadURL(context.Context) (*url.URL, error) { return h.url, nil } +func (h requestDepsAPIHost) RawURL(context.Context) (*url.URL, error) { return h.url, nil } +func (h requestDepsAPIHost) AuthorizationServerURL(context.Context) (*url.URL, error) { + return h.url, nil +} + func testExporters() observability.Exporters { obs, _ := observability.NewExporters(slog.New(slog.DiscardHandler), metrics.NewNoopMetrics()) return obs } +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() + + serverURL, err := url.Parse(server.URL) + require.NoError(t, err) + apiHost := requestDepsAPIHost{url: serverURL} + deps := github.NewRequestDeps(apiHost, "test", false, nil, nil, 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) +} + func TestIsFeatureEnabled_WithEnabledFlag(t *testing.T) { t.Parallel() diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index e2bf8b684b..5b14dc926b 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -75,7 +75,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", } @@ -87,7 +87,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 e032a0ce93..f914ae72c4 100644 --- a/pkg/http/headers/headers.go +++ b/pkg/http/headers/headers.go @@ -53,4 +53,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..292e69fa85 --- /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 GitHub REST API version on requests to +// GitHub.com and GitHub Enterprise Cloud. +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.github.com/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.github.com", "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.github.com"} fetcher := NewFetcher(apiHost, FetcherOptions{}) diff --git a/pkg/utils/api.go b/pkg/utils/api.go index 95dfbd1d5b..4a9c81bad9 100644 --- a/pkg/utils/api.go +++ b/pkg/utils/api.go @@ -256,10 +256,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 40fcb8f26a..4b73624dad 100644 --- a/pkg/utils/api_test.go +++ b/pkg/utils/api_test.go @@ -73,3 +73,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) + }) + } +}