diff --git a/docs/remote-server.md b/docs/remote-server.md index 149667393..cad9ed604 100644 --- a/docs/remote-server.md +++ b/docs/remote-server.md @@ -121,13 +121,15 @@ The Remote GitHub MCP server supports the following URL path patterns: - `/` - Default toolset (see ["default" toolset](../README.md#default-toolset)) - `/readonly` - Default toolset in read-only mode - `/insiders` - Default toolset with insiders mode enabled -- `/insiders/readonly` - Default toolset with insiders mode in read-only mode +- `/readonly/insiders` - Default toolset in read-only mode with insiders mode enabled - `/x/all` - All available toolsets - `/x/all/readonly` - All available toolsets in read-only mode - `/x/all/insiders` - All available toolsets with insiders mode enabled +- `/x/all/readonly/insiders` - All available toolsets in read-only mode with insiders mode enabled - `/x/{toolset}` - Single specific toolset - `/x/{toolset}/readonly` - Single specific toolset in read-only mode - `/x/{toolset}/insiders` - Single specific toolset with insiders mode enabled +- `/x/{toolset}/readonly/insiders` - Single specific toolset in read-only mode with insiders mode enabled Note: `{toolset}` can only be a single toolset, not a comma-separated list. To combine multiple toolsets, use the `X-MCP-Toolsets` header instead. Path modifiers like `/readonly` and `/insiders` can be combined with the `X-MCP-Insiders` or `X-MCP-Readonly` headers. diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index a4316e6c9..bb0cc277b 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -135,8 +135,8 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se WithReadOnly(cfg.ReadOnly). WithToolsets(cfg.EnabledToolsets). WithTools(github.CleanTools(cfg.EnabledTools)). - WithServerInstructions() - // WithFeatureChecker(createFeatureChecker(cfg.EnabledFeatures)) + WithServerInstructions(). + WithFeatureChecker(featureChecker) // Apply token scope filtering if scopes are known (for PAT filtering) if cfg.TokenScopes != nil { diff --git a/pkg/context/request.go b/pkg/context/request.go index 94882a3ce..70867f32e 100644 --- a/pkg/context/request.go +++ b/pkg/context/request.go @@ -65,3 +65,35 @@ func IsLockdownMode(ctx context.Context) bool { } return false } + +// insidersCtxKey is a context key for insiders mode +type insidersCtxKey struct{} + +// WithInsidersMode adds insiders mode state to the context +func WithInsidersMode(ctx context.Context, enabled bool) context.Context { + return context.WithValue(ctx, insidersCtxKey{}, enabled) +} + +// IsInsidersMode retrieves the insiders mode state from the context +func IsInsidersMode(ctx context.Context) bool { + if enabled, ok := ctx.Value(insidersCtxKey{}).(bool); ok { + return enabled + } + return false +} + +// headerFeaturesCtxKey is a context key for raw header feature flags +type headerFeaturesCtxKey struct{} + +// WithHeaderFeatures stores the raw feature flags from the X-MCP-Features header into context +func WithHeaderFeatures(ctx context.Context, features []string) context.Context { + return context.WithValue(ctx, headerFeaturesCtxKey{}, features) +} + +// GetHeaderFeatures retrieves the raw feature flags from context +func GetHeaderFeatures(ctx context.Context) []string { + if features, ok := ctx.Value(headerFeaturesCtxKey{}).([]string); ok { + return features + } + return nil +} diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go index bdcafe933..75804ad1f 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -248,7 +248,6 @@ type RequestDeps struct { lockdownMode bool RepoAccessOpts []lockdown.RepoAccessOption T translations.TranslationHelperFunc - Flags FeatureFlags ContentWindowSize int // Feature flag checker for runtime checks @@ -380,6 +379,7 @@ func (d *RequestDeps) GetT() translations.TranslationHelperFunc { return d.T } func (d *RequestDeps) GetFlags(ctx context.Context) FeatureFlags { return FeatureFlags{ LockdownMode: d.lockdownMode && ghcontext.IsLockdownMode(ctx), + InsidersMode: ghcontext.IsInsidersMode(ctx), } } diff --git a/pkg/http/features.go b/pkg/http/features.go deleted file mode 100644 index f8cb41729..000000000 --- a/pkg/http/features.go +++ /dev/null @@ -1,42 +0,0 @@ -package http - -import ( - "context" - "slices" - - "github.com/github/github-mcp-server/pkg/github" - "github.com/github/github-mcp-server/pkg/inventory" -) - -// KnownFeatureFlags are the feature flags that can be enabled via X-MCP-Features header. -var KnownFeatureFlags = []string{ - github.FeatureFlagHoldbackConsolidatedProjects, - github.FeatureFlagHoldbackConsolidatedActions, -} - -// ComposeFeatureChecker combines header-based feature flags with a static checker. -func ComposeFeatureChecker(headerFeatures []string, staticChecker inventory.FeatureFlagChecker) inventory.FeatureFlagChecker { - if len(headerFeatures) == 0 && staticChecker == nil { - return nil - } - - // Only accept header features that are in the known list - headerSet := make(map[string]bool, len(headerFeatures)) - for _, f := range headerFeatures { - if slices.Contains(KnownFeatureFlags, f) { - headerSet[f] = true - } - } - - return func(ctx context.Context, flag string) (bool, error) { - // Header-based: static string matching - if headerSet[flag] { - return true, nil - } - // Static checker - if staticChecker != nil { - return staticChecker(ctx, flag) - } - return false, nil - } -} diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 5b889b36b..7fa38a73d 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -7,7 +7,6 @@ import ( 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/http/middleware" "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/translations" @@ -31,6 +30,7 @@ type Handler struct { type HandlerOptions struct { GitHubMcpServerFactory GitHubMCPServerFactoryFunc InventoryFactory InventoryFactoryFunc + FeatureChecker inventory.FeatureFlagChecker } type HandlerOption func(*HandlerOptions) @@ -47,6 +47,12 @@ func WithInventoryFactory(f InventoryFactoryFunc) HandlerOption { } } +func WithFeatureChecker(checker inventory.FeatureFlagChecker) HandlerOption { + return func(o *HandlerOptions) { + o.FeatureChecker = checker + } +} + func NewHTTPMcpHandler( ctx context.Context, cfg *ServerConfig, @@ -66,7 +72,7 @@ func NewHTTPMcpHandler( inventoryFactory := opts.InventoryFactory if inventoryFactory == nil { - inventoryFactory = DefaultInventoryFactory(cfg, t, nil) + inventoryFactory = DefaultInventoryFactory(cfg, t, opts.FeatureChecker) } return &Handler{ @@ -85,11 +91,17 @@ func NewHTTPMcpHandler( func (h *Handler) RegisterRoutes(r chi.Router) { r.Use(middleware.WithRequestConfig) + // Base routes r.Mount("/", h) - // Mount readonly and toolset routes - r.With(withToolset).Mount("/x/{toolset}", h) - r.With(withReadonly, withToolset).Mount("/x/{toolset}/readonly", h) r.With(withReadonly).Mount("/readonly", h) + r.With(withInsiders).Mount("/insiders", h) + r.With(withReadonly, withInsiders).Mount("/readonly/insiders", h) + + // Toolset routes + r.With(withToolset).Mount("/x/{toolset}", h) + r.With(withToolset, withReadonly).Mount("/x/{toolset}/readonly", h) + r.With(withToolset, withInsiders).Mount("/x/{toolset}/insiders", h) + r.With(withToolset, withReadonly, withInsiders).Mount("/x/{toolset}/readonly/insiders", h) } // withReadonly is middleware that sets readonly mode in the request context @@ -109,6 +121,14 @@ func withToolset(next http.Handler) http.Handler { }) } +// withInsiders is middleware that sets insiders mode in the request context +func withInsiders(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := ghcontext.WithInsidersMode(r.Context(), true) + next.ServeHTTP(w, r.WithContext(ctx)) + }) +} + func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { inventory, err := h.inventoryFactoryFunc(r) if err != nil { @@ -141,15 +161,12 @@ func DefaultGitHubMCPServerFactory(r *http.Request, deps github.ToolDependencies return github.NewMCPServer(r.Context(), cfg, deps, inventory) } -func DefaultInventoryFactory(_ *ServerConfig, t translations.TranslationHelperFunc, staticChecker inventory.FeatureFlagChecker) InventoryFactoryFunc { +// DefaultInventoryFactory creates the default inventory factory for HTTP mode +func DefaultInventoryFactory(_ *ServerConfig, t translations.TranslationHelperFunc, featureChecker inventory.FeatureFlagChecker) InventoryFactoryFunc { return func(r *http.Request) (*inventory.Inventory, error) { - b := github.NewInventory(t).WithDeprecatedAliases(github.DeprecatedToolAliases) - - // Feature checker composition - headerFeatures := headers.ParseCommaSeparated(r.Header.Get(headers.MCPFeaturesHeader)) - if checker := ComposeFeatureChecker(headerFeatures, staticChecker); checker != nil { - b = b.WithFeatureChecker(checker) - } + b := github.NewInventory(t). + WithDeprecatedAliases(github.DeprecatedToolAliases). + WithFeatureChecker(featureChecker) b = InventoryFiltersForRequest(r, b) b.WithServerInstructions() diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index 3db764a85..d02797330 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -32,6 +32,13 @@ func mockTool(name, toolsetID string, readOnly bool) inventory.ServerTool { } } +func mockToolWithFeatureFlag(name, toolsetID string, readOnly bool, enableFlag, disableFlag string) inventory.ServerTool { + tool := mockTool(name, toolsetID, readOnly) + tool.FeatureFlagEnable = enableFlag + tool.FeatureFlagDisable = disableFlag + return tool +} + func TestInventoryFiltersForRequest(t *testing.T) { tools := []inventory.ServerTool{ mockTool("get_file_contents", "repos", true), @@ -115,12 +122,15 @@ func testTools() []inventory.ServerTool { mockTool("create_issue", "issues", false), mockTool("list_pull_requests", "pull_requests", true), mockTool("create_pull_request", "pull_requests", false), + // Feature-flagged tools for testing X-MCP-Features header + mockToolWithFeatureFlag("needs_holdback", "repos", true, "mcp_holdback_consolidated_projects", ""), + mockToolWithFeatureFlag("hidden_by_holdback", "repos", true, "", "mcp_holdback_consolidated_projects"), } } // extractToolNames extracts tool names from an inventory -func extractToolNames(inv *inventory.Inventory) []string { - available := inv.AvailableTools(context.Background()) +func extractToolNames(ctx context.Context, inv *inventory.Inventory) []string { + available := inv.AvailableTools(ctx) names := make([]string, len(available)) for i, tool := range available { names[i] = tool.Tool.Name @@ -141,17 +151,17 @@ func TestHTTPHandlerRoutes(t *testing.T) { { name: "root path returns all tools", path: "/", - expectedTools: []string{"get_file_contents", "create_repository", "list_issues", "create_issue", "list_pull_requests", "create_pull_request"}, + expectedTools: []string{"get_file_contents", "create_repository", "list_issues", "create_issue", "list_pull_requests", "create_pull_request", "hidden_by_holdback"}, }, { name: "readonly path filters write tools", path: "/readonly", - expectedTools: []string{"get_file_contents", "list_issues", "list_pull_requests"}, + expectedTools: []string{"get_file_contents", "list_issues", "list_pull_requests", "hidden_by_holdback"}, }, { name: "toolset path filters to toolset", path: "/x/repos", - expectedTools: []string{"get_file_contents", "create_repository"}, + expectedTools: []string{"get_file_contents", "create_repository", "hidden_by_holdback"}, }, { name: "toolset path with issues", @@ -161,7 +171,7 @@ func TestHTTPHandlerRoutes(t *testing.T) { { name: "toolset readonly path filters to readonly tools in toolset", path: "/x/repos/readonly", - expectedTools: []string{"get_file_contents"}, + expectedTools: []string{"get_file_contents", "hidden_by_holdback"}, }, { name: "toolset readonly path with issues", @@ -198,7 +208,7 @@ func TestHTTPHandlerRoutes(t *testing.T) { headers: map[string]string{ headers.MCPReadOnlyHeader: "true", }, - expectedTools: []string{"get_file_contents", "list_issues", "list_pull_requests"}, + expectedTools: []string{"get_file_contents", "list_issues", "list_pull_requests", "hidden_by_holdback"}, }, { name: "X-MCP-Toolsets header filters to toolset", @@ -206,7 +216,7 @@ func TestHTTPHandlerRoutes(t *testing.T) { headers: map[string]string{ headers.MCPToolsetsHeader: "repos", }, - expectedTools: []string{"get_file_contents", "create_repository"}, + expectedTools: []string{"get_file_contents", "create_repository", "hidden_by_holdback"}, }, { name: "URL toolset takes precedence over header toolset", @@ -222,19 +232,41 @@ func TestHTTPHandlerRoutes(t *testing.T) { headers: map[string]string{ headers.MCPReadOnlyHeader: "false", }, - expectedTools: []string{"get_file_contents", "list_issues", "list_pull_requests"}, + expectedTools: []string{"get_file_contents", "list_issues", "list_pull_requests", "hidden_by_holdback"}, + }, + { + name: "X-MCP-Features header enables flagged tool", + path: "/", + headers: map[string]string{ + headers.MCPFeaturesHeader: "mcp_holdback_consolidated_projects", + }, + expectedTools: []string{"get_file_contents", "create_repository", "list_issues", "create_issue", "list_pull_requests", "create_pull_request", "needs_holdback"}, + }, + { + name: "X-MCP-Features header with unknown flag is ignored", + path: "/", + headers: map[string]string{ + headers.MCPFeaturesHeader: "unknown_flag", + }, + expectedTools: []string{"get_file_contents", "create_repository", "list_issues", "create_issue", "list_pull_requests", "create_pull_request", "hidden_by_holdback"}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var capturedInventory *inventory.Inventory + var capturedCtx context.Context + + // Create feature checker that reads from context (same as production) + featureChecker := createHTTPFeatureChecker() // Create inventory factory that captures the built inventory inventoryFactory := func(r *http.Request) (*inventory.Inventory, error) { + capturedCtx = r.Context() builder := inventory.NewBuilder(). SetTools(tools). - WithToolsets([]string{"all"}) + WithToolsets([]string{"all"}). + WithFeatureChecker(featureChecker) builder = InventoryFiltersForRequest(r, builder) inv, err := builder.Build() if err != nil { @@ -277,7 +309,7 @@ func TestHTTPHandlerRoutes(t *testing.T) { // Verify the inventory was captured and has the expected tools require.NotNil(t, capturedInventory, "inventory should have been created") - toolNames := extractToolNames(capturedInventory) + toolNames := extractToolNames(capturedCtx, capturedInventory) expectedSorted := make([]string, len(tt.expectedTools)) copy(expectedSorted, tt.expectedTools) sort.Strings(expectedSorted) diff --git a/pkg/http/headers/headers.go b/pkg/http/headers/headers.go index a1580cf96..20d436c7c 100644 --- a/pkg/http/headers/headers.go +++ b/pkg/http/headers/headers.go @@ -34,6 +34,8 @@ const ( MCPToolsHeader = "X-MCP-Tools" // MCPLockdownHeader indicates whether lockdown mode is enabled. MCPLockdownHeader = "X-MCP-Lockdown" + // MCPInsidersHeader indicates whether insiders mode is enabled for early access features. + MCPInsidersHeader = "X-MCP-Insiders" // MCPFeaturesHeader is a comma-separated list of feature flags to enable. MCPFeaturesHeader = "X-MCP-Features" ) diff --git a/pkg/http/middleware/request_config.go b/pkg/http/middleware/request_config.go index da9127bf1..5cabe16eb 100644 --- a/pkg/http/middleware/request_config.go +++ b/pkg/http/middleware/request_config.go @@ -9,27 +9,42 @@ import ( "github.com/github/github-mcp-server/pkg/http/headers" ) -// WithRequestConfig is a middleware that extracts MCP-related headers and sets them in the request context +// WithRequestConfig is a middleware that extracts MCP-related headers and sets them in the request context. +// This includes readonly mode, toolsets, tools, lockdown mode, insiders mode, and feature flags. func WithRequestConfig(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() + // Readonly mode if relaxedParseBool(r.Header.Get(headers.MCPReadOnlyHeader)) { ctx = ghcontext.WithReadonly(ctx, true) } + // Toolsets if toolsets := headers.ParseCommaSeparated(r.Header.Get(headers.MCPToolsetsHeader)); len(toolsets) > 0 { ctx = ghcontext.WithToolsets(ctx, toolsets) } + // Tools if tools := headers.ParseCommaSeparated(r.Header.Get(headers.MCPToolsHeader)); len(tools) > 0 { ctx = ghcontext.WithTools(ctx, tools) } + // Lockdown mode if relaxedParseBool(r.Header.Get(headers.MCPLockdownHeader)) { ctx = ghcontext.WithLockdownMode(ctx, true) } + // Insiders mode + if relaxedParseBool(r.Header.Get(headers.MCPInsidersHeader)) { + ctx = ghcontext.WithInsidersMode(ctx, true) + } + + // Feature flags + if features := headers.ParseCommaSeparated(r.Header.Get(headers.MCPFeaturesHeader)); len(features) > 0 { + ctx = ghcontext.WithHeaderFeatures(ctx, features) + } + next.ServeHTTP(w, r.WithContext(ctx)) }) } diff --git a/pkg/http/server.go b/pkg/http/server.go index 33fe23d14..8ea8c641c 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -8,16 +8,26 @@ import ( "net/http" "os" "os/signal" + "slices" "syscall" "time" + ghcontext "github.com/github/github-mcp-server/pkg/context" "github.com/github/github-mcp-server/pkg/github" + "github.com/github/github-mcp-server/pkg/inventory" "github.com/github/github-mcp-server/pkg/lockdown" "github.com/github/github-mcp-server/pkg/translations" "github.com/github/github-mcp-server/pkg/utils" "github.com/go-chi/chi/v5" ) +// knownFeatureFlags are the feature flags that can be enabled via X-MCP-Features header. +// Only these flags are accepted from headers. +var knownFeatureFlags = []string{ + github.FeatureFlagHoldbackConsolidatedProjects, + github.FeatureFlagHoldbackConsolidatedActions, +} + type ServerConfig struct { // Version of the server Version string @@ -83,6 +93,8 @@ func RunHTTPServer(cfg ServerConfig) error { repoAccessOpts = append(repoAccessOpts, lockdown.WithTTL(*cfg.RepoAccessCacheTTL)) } + featureChecker := createHTTPFeatureChecker() + deps := github.NewRequestDeps( apiHost, cfg.Version, @@ -90,12 +102,12 @@ func RunHTTPServer(cfg ServerConfig) error { repoAccessOpts, t, cfg.ContentWindowSize, - nil, + featureChecker, ) r := chi.NewRouter() - handler := NewHTTPMcpHandler(ctx, &cfg, deps, t, logger) + handler := NewHTTPMcpHandler(ctx, &cfg, deps, t, logger, WithFeatureChecker(featureChecker)) handler.RegisterRoutes(r) addr := fmt.Sprintf(":%d", cfg.Port) @@ -128,3 +140,20 @@ func RunHTTPServer(cfg ServerConfig) error { logger.Info("server stopped gracefully") return nil } + +// createHTTPFeatureChecker creates a feature checker that reads header features from context +// and validates them against the knownFeatureFlags whitelist +func createHTTPFeatureChecker() inventory.FeatureFlagChecker { + // Pre-compute whitelist as set for O(1) lookup + knownSet := make(map[string]bool, len(knownFeatureFlags)) + for _, f := range knownFeatureFlags { + knownSet[f] = true + } + + return func(ctx context.Context, flag string) (bool, error) { + if knownSet[flag] && slices.Contains(ghcontext.GetHeaderFeatures(ctx), flag) { + return true, nil + } + return false, nil + } +}