Skip to content

Commit e0db9b3

Browse files
committed
Merge branch 'http-stack-2' into scope-challenge-http
2 parents ab6a5ca + 206f7f3 commit e0db9b3

File tree

11 files changed

+103
-24
lines changed

11 files changed

+103
-24
lines changed

.github/workflows/docker-publish.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ jobs:
6060
# https://github.com/docker/login-action
6161
- name: Log into registry ${{ env.REGISTRY }}
6262
if: github.event_name != 'pull_request'
63-
uses: docker/login-action@5e57cd118135c172c3672efd75eb46360885c0ef # v3.6.0
63+
uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3.7.0
6464
with:
6565
registry: ${{ env.REGISTRY }}
6666
username: ${{ github.actor }}

internal/ghmcp/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
133133
inventoryBuilder := github.NewInventory(cfg.Translator).
134134
WithDeprecatedAliases(github.DeprecatedToolAliases).
135135
WithReadOnly(cfg.ReadOnly).
136-
WithToolsets(cfg.EnabledToolsets).
136+
WithToolsets(github.ResolvedEnabledToolsets(cfg.DynamicToolsets, cfg.EnabledToolsets, cfg.EnabledTools)).
137137
WithTools(github.CleanTools(cfg.EnabledTools)).
138138
WithServerInstructions().
139139
WithFeatureChecker(featureChecker)

pkg/github/dependencies.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,10 +320,14 @@ func (d *RequestDeps) GetGQLClient(ctx context.Context) (*githubv4.Client, error
320320

321321
// Construct GraphQL client
322322
// We use NewEnterpriseClient unconditionally since we already parsed the API host
323+
// Wrap transport with GraphQLFeaturesTransport to inject feature flags from context,
324+
// matching the transport chain used by the remote server.
323325
gqlHTTPClient := &http.Client{
324326
Transport: &transport.BearerAuthTransport{
325-
Transport: http.DefaultTransport,
326-
Token: token,
327+
Transport: &transport.GraphQLFeaturesTransport{
328+
Transport: http.DefaultTransport,
329+
},
330+
Token: token,
327331
},
328332
}
329333

pkg/github/pullrequests.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,7 +1519,7 @@ func SubmitPendingPullRequestReview(ctx context.Context, client *githubv4.Client
15191519
"prNum": githubv4.Int(params.PullNumber),
15201520
}
15211521

1522-
if err := client.Query(context.Background(), &getLatestReviewForViewerQuery, vars); err != nil {
1522+
if err := client.Query(ctx, &getLatestReviewForViewerQuery, vars); err != nil {
15231523
return ghErrors.NewGitHubGraphQLErrorResponse(ctx,
15241524
"failed to get latest review for current user",
15251525
err,
@@ -1604,7 +1604,7 @@ func DeletePendingPullRequestReview(ctx context.Context, client *githubv4.Client
16041604
"prNum": githubv4.Int(params.PullNumber),
16051605
}
16061606

1607-
if err := client.Query(context.Background(), &getLatestReviewForViewerQuery, vars); err != nil {
1607+
if err := client.Query(ctx, &getLatestReviewForViewerQuery, vars); err != nil {
16081608
return ghErrors.NewGitHubGraphQLErrorResponse(ctx,
16091609
"failed to get latest review for current user",
16101610
err,
@@ -1778,7 +1778,7 @@ func AddCommentToPendingReview(t translations.TranslationHelperFunc) inventory.S
17781778
"prNum": githubv4.Int(params.PullNumber),
17791779
}
17801780

1781-
if err := client.Query(context.Background(), &getLatestReviewForViewerQuery, vars); err != nil {
1781+
if err := client.Query(ctx, &getLatestReviewForViewerQuery, vars); err != nil {
17821782
return ghErrors.NewGitHubGraphQLErrorResponse(ctx,
17831783
"failed to get latest review for current user",
17841784
err,

pkg/github/server.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -140,25 +140,23 @@ func registerDynamicTools(server *mcp.Server, inventory *inventory.Inventory, de
140140
}
141141
}
142142

143-
// resolveEnabledToolsets determines which toolsets should be enabled based on config.
143+
// ResolvedEnabledToolsets determines which toolsets should be enabled based on config.
144144
// Returns nil for "use defaults", empty slice for "none", or explicit list.
145-
func resolveEnabledToolsets(cfg *MCPServerConfig) []string {
146-
enabledToolsets := cfg.EnabledToolsets
147-
145+
func ResolvedEnabledToolsets(dynamicToolsets bool, enabledToolsets []string, enabledTools []string) []string {
148146
// In dynamic mode, remove "all" and "default" since users enable toolsets on demand
149-
if cfg.DynamicToolsets && enabledToolsets != nil {
147+
if dynamicToolsets && enabledToolsets != nil {
150148
enabledToolsets = RemoveToolset(enabledToolsets, string(ToolsetMetadataAll.ID))
151149
enabledToolsets = RemoveToolset(enabledToolsets, string(ToolsetMetadataDefault.ID))
152150
}
153151

154152
if enabledToolsets != nil {
155153
return enabledToolsets
156154
}
157-
if cfg.DynamicToolsets {
155+
if dynamicToolsets {
158156
// Dynamic mode with no toolsets specified: start empty so users enable on demand
159157
return []string{}
160158
}
161-
if len(cfg.EnabledTools) > 0 {
159+
if len(enabledTools) > 0 {
162160
// When specific tools are requested but no toolsets, don't use default toolsets
163161
// This matches the original behavior: --tools=X alone registers only X
164162
return []string{}

pkg/github/server_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ func TestResolveEnabledToolsets(t *testing.T) {
214214

215215
for _, tc := range tests {
216216
t.Run(tc.name, func(t *testing.T) {
217-
result := resolveEnabledToolsets(&tc.cfg)
217+
result := ResolvedEnabledToolsets(tc.cfg.DynamicToolsets, tc.cfg.EnabledToolsets, tc.cfg.EnabledTools)
218218
assert.Equal(t, tc.expectedResult, result)
219219
})
220220
}

pkg/http/handler.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,15 @@ func InventoryFiltersForRequest(r *http.Request, builder *inventory.Builder) *in
225225
builder = builder.WithReadOnly(true)
226226
}
227227

228-
if toolsets := ghcontext.GetToolsets(ctx); len(toolsets) > 0 {
229-
builder = builder.WithToolsets(toolsets)
228+
toolsets := ghcontext.GetToolsets(ctx)
229+
tools := ghcontext.GetTools(ctx)
230+
231+
if len(toolsets) > 0 {
232+
builder = builder.WithToolsets(github.ResolvedEnabledToolsets(false, toolsets, tools)) // No dynamic toolsets in HTTP mode
230233
}
231234

232-
if tools := ghcontext.GetTools(ctx); len(tools) > 0 {
233-
if len(ghcontext.GetToolsets(ctx)) == 0 {
235+
if len(tools) > 0 {
236+
if len(toolsets) == 0 {
234237
builder = builder.WithToolsets([]string{})
235238
}
236239
builder = builder.WithTools(github.CleanTools(tools))

pkg/http/middleware/token.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,36 @@ import (
1010
"github.com/github/github-mcp-server/pkg/utils"
1111
)
1212

13+
<<<<<<< HEAD
14+
=======
15+
type authType int
16+
17+
const (
18+
authTypeUnknown authType = iota
19+
authTypeIDE
20+
authTypeGhToken
21+
)
22+
23+
var (
24+
errMissingAuthorizationHeader = fmt.Errorf("%w: missing required Authorization header", mark.ErrBadRequest)
25+
errBadAuthorizationHeader = fmt.Errorf("%w: Authorization header is badly formatted", mark.ErrBadRequest)
26+
errUnsupportedAuthorizationHeader = fmt.Errorf("%w: unsupported Authorization header", mark.ErrBadRequest)
27+
)
28+
29+
var supportedGitHubPrefixes = []string{
30+
"ghp_", // Personal access token (classic)
31+
"github_pat_", // Fine-grained personal access token
32+
"gho_", // OAuth access token
33+
"ghu_", // User access token for a GitHub App
34+
"ghs_", // Installation access token for a GitHub App (a.k.a. server-to-server token)
35+
}
36+
37+
// oldPatternRegexp is the regular expression for the old pattern of the token.
38+
// Until 2021, GitHub API tokens did not have an identifiable prefix. They
39+
// were 40 characters long and only contained the characters a-f and 0-9.
40+
var oldPatternRegexp = regexp.MustCompile(`\A[a-f0-9]{40}\z`)
41+
42+
>>>>>>> http-stack-2
1343
func ExtractUserToken(oauthCfg *oauth.Config) func(next http.Handler) http.Handler {
1444
return func(next http.Handler) http.Handler {
1545
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -45,3 +75,45 @@ func sendAuthChallenge(w http.ResponseWriter, r *http.Request, oauthCfg *oauth.C
4575
w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer resource_metadata=%q`, resourceMetadataURL))
4676
http.Error(w, "Unauthorized", http.StatusUnauthorized)
4777
}
78+
<<<<<<< HEAD
79+
=======
80+
81+
func parseAuthorizationHeader(req *http.Request) (authType authType, token string, _ error) {
82+
authHeader := req.Header.Get(httpheaders.AuthorizationHeader)
83+
if authHeader == "" {
84+
return 0, "", errMissingAuthorizationHeader
85+
}
86+
87+
switch {
88+
// decrypt dotcom token and set it as token
89+
case strings.HasPrefix(authHeader, "GitHub-Bearer "):
90+
return 0, "", errUnsupportedAuthorizationHeader
91+
default:
92+
// support both "Bearer" and "bearer" to conform to api.github.com
93+
if len(authHeader) > 7 && strings.EqualFold(authHeader[:7], "Bearer ") {
94+
token = authHeader[7:]
95+
} else {
96+
token = authHeader
97+
}
98+
}
99+
100+
// Do a naïve check for a colon in the token - currently, only the IDE token has a colon in it.
101+
// ex: tid=1;exp=25145314523;chat=1:<hmac>
102+
if strings.Contains(token, ":") {
103+
return authTypeIDE, token, nil
104+
}
105+
106+
for _, prefix := range supportedGitHubPrefixes {
107+
if strings.HasPrefix(token, prefix) {
108+
return authTypeGhToken, token, nil
109+
}
110+
}
111+
112+
matchesOldTokenPattern := oldPatternRegexp.MatchString(token)
113+
if matchesOldTokenPattern {
114+
return authTypeGhToken, token, nil
115+
}
116+
117+
return 0, "", errBadAuthorizationHeader
118+
}
119+
>>>>>>> http-stack-2

pkg/http/transport/graphql_features.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,16 @@ import (
1717
//
1818
// Usage:
1919
//
20+
// import "github.com/github/github-mcp-server/pkg/http/transport"
21+
//
2022
// httpClient := &http.Client{
21-
// Transport: &github.GraphQLFeaturesTransport{
23+
// Transport: &transport.GraphQLFeaturesTransport{
2224
// Transport: http.DefaultTransport,
2325
// },
2426
// }
2527
// gqlClient := githubv4.NewClient(httpClient)
2628
//
27-
// Then use withGraphQLFeatures(ctx, "feature_name") when calling GraphQL operations.
29+
// Then use ghcontext.WithGraphQLFeatures(ctx, "feature_name") when calling GraphQL operations.
2830
type GraphQLFeaturesTransport struct {
2931
// Transport is the underlying HTTP transport. If nil, http.DefaultTransport is used.
3032
Transport http.RoundTripper

pkg/utils/api.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func newGHECHost(hostname string) (APIHost, error) {
102102
return APIHost{}, fmt.Errorf("failed to parse GHEC GraphQL URL: %w", err)
103103
}
104104

105-
uploadURL, err := url.Parse(fmt.Sprintf("https://uploads.%s", u.Hostname()))
105+
uploadURL, err := url.Parse(fmt.Sprintf("https://uploads.%s/", u.Hostname()))
106106
if err != nil {
107107
return APIHost{}, fmt.Errorf("failed to parse GHEC Upload URL: %w", err)
108108
}

0 commit comments

Comments
 (0)