From c35ab932750f042f3cb9f0aaddd91e5e229802fc Mon Sep 17 00:00:00 2001 From: kerobbi Date: Wed, 21 Jan 2026 15:27:23 +0000 Subject: [PATCH 01/13] add readonly and toolset support --- pkg/context/request.go | 35 +++++++++++++ pkg/http/handler.go | 66 ++++++++++++++++++++---- pkg/http/handler_test.go | 107 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 198 insertions(+), 10 deletions(-) create mode 100644 pkg/context/request.go create mode 100644 pkg/http/handler_test.go diff --git a/pkg/context/request.go b/pkg/context/request.go new file mode 100644 index 000000000..1ca48f32b --- /dev/null +++ b/pkg/context/request.go @@ -0,0 +1,35 @@ +package context + +import "context" + +// readonlyCtxKey is a context key for read-only mode +type readonlyCtxKey struct{} + +// WithReadonly adds read-only mode state to the context +func WithReadonly(ctx context.Context, enabled bool) context.Context { + return context.WithValue(ctx, readonlyCtxKey{}, enabled) +} + +// IsReadonly retrieves the read-only mode state from the context +func IsReadonly(ctx context.Context) bool { + if enabled, ok := ctx.Value(readonlyCtxKey{}).(bool); ok { + return enabled + } + return false +} + +// toolsetCtxKey is a context key for the active toolset +type toolsetCtxKey struct{} + +// WithToolset adds the active toolset to the context +func WithToolset(ctx context.Context, toolset string) context.Context { + return context.WithValue(ctx, toolsetCtxKey{}, toolset) +} + +// GetToolset retrieves the active toolset from the context +func GetToolset(ctx context.Context) string { + if toolset, ok := ctx.Value(toolsetCtxKey{}).(string); ok { + return toolset + } + return "" +} diff --git a/pkg/http/handler.go b/pkg/http/handler.go index f2fcb531f..6bb3c9b61 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -4,8 +4,10 @@ import ( "context" "log/slog" "net/http" + "slices" "strings" + 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" @@ -78,6 +80,28 @@ func NewHTTPMcpHandler(cfg *HTTPServerConfig, func (h *HTTPMcpHandler) RegisterRoutes(r chi.Router) { 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) +} + +// withReadonly is middleware that sets readonly mode in the request context +func withReadonly(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := ghcontext.WithReadonly(r.Context(), true) + next.ServeHTTP(w, r.WithContext(ctx)) + }) +} + +// withToolset is middleware that extracts the toolset from the URL and sets it in the request context +func withToolset(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + toolset := chi.URLParam(r, "toolset") + ctx := ghcontext.WithToolset(r.Context(), toolset) + next.ServeHTTP(w, r.WithContext(ctx)) + }) } func (h *HTTPMcpHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { @@ -124,25 +148,38 @@ func DefaultInventoryFactory(cfg *HTTPServerConfig, t translations.TranslationHe b = b.WithFeatureChecker(checker) } - b = InventoryFiltersForRequestHeaders(r, b) + b = InventoryFiltersForRequest(r, b) return b.Build() } } -// InventoryFiltersForRequestHeaders applies inventory filters based on HTTP request headers. -// Whitespace is trimmed from comma-separated values; empty values are ignored. -func InventoryFiltersForRequestHeaders(r *http.Request, builder *inventory.Builder) *inventory.Builder { - if r.Header.Get(headers.MCPReadOnlyHeader) != "" { +// InventoryFiltersForRequest applies inventory filters from request context and headers +// Whitespace is trimmed from comma-separated values; empty values are ignored +// Route configuration (context) takes precedence over headers for toolsets +func InventoryFiltersForRequest(r *http.Request, builder *inventory.Builder) *inventory.Builder { + ctx := r.Context() + + // Enable readonly mode if set in context or via header + if ghcontext.IsReadonly(ctx) || relaxedParseBool(r.Header.Get(headers.MCPReadOnlyHeader)) { builder = builder.WithReadOnly(true) } - if toolsetsStr := r.Header.Get(headers.MCPToolsetsHeader); toolsetsStr != "" { - toolsets := parseCommaSeparatedHeader(toolsetsStr) - builder = builder.WithToolsets(toolsets) + // Parse request configuration + contextToolset := ghcontext.GetToolset(ctx) + headerToolsets := parseCommaSeparatedHeader(r.Header.Get(headers.MCPToolsetsHeader)) + tools := parseCommaSeparatedHeader(r.Header.Get(headers.MCPToolsHeader)) + + // Apply toolset filtering (route wins, then header, then tools-only mode, else defaults) + switch { + case contextToolset != "": + builder = builder.WithToolsets([]string{contextToolset}) + case len(headerToolsets) > 0: + builder = builder.WithToolsets(headerToolsets) + case len(tools) > 0: + builder = builder.WithToolsets([]string{}) } - if toolsStr := r.Header.Get(headers.MCPToolsHeader); toolsStr != "" { - tools := parseCommaSeparatedHeader(toolsStr) + if len(tools) > 0 { builder = builder.WithTools(github.CleanTools(tools)) } @@ -166,3 +203,12 @@ func parseCommaSeparatedHeader(value string) []string { } return result } + +// relaxedParseBool parses a string into a boolean value, treating various +// common false values or empty strings as false, and everything else as true. +// It is case-insensitive and trims whitespace. +func relaxedParseBool(s string) bool { + s = strings.TrimSpace(strings.ToLower(s)) + falseValues := []string{"", "false", "0", "no", "off", "n", "f"} + return !slices.Contains(falseValues, s) +} diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go new file mode 100644 index 000000000..982a16c4c --- /dev/null +++ b/pkg/http/handler_test.go @@ -0,0 +1,107 @@ +package http + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + ghcontext "github.com/github/github-mcp-server/pkg/context" + "github.com/github/github-mcp-server/pkg/http/headers" + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" +) + +func mockTool(name, toolsetID string, readOnly bool) inventory.ServerTool { + return inventory.ServerTool{ + Tool: mcp.Tool{ + Name: name, + Annotations: &mcp.ToolAnnotations{ReadOnlyHint: readOnly}, + }, + Toolset: inventory.ToolsetMetadata{ + ID: inventory.ToolsetID(toolsetID), + Description: "Test: " + toolsetID, + }, + } +} + +func TestInventoryFiltersForRequest(t *testing.T) { + tools := []inventory.ServerTool{ + mockTool("get_file_contents", "repos", true), + mockTool("create_repository", "repos", false), + mockTool("list_issues", "issues", true), + mockTool("issue_write", "issues", false), + } + + tests := []struct { + name string + contextSetup func(context.Context) context.Context + headers map[string]string + expectedTools []string + }{ + { + name: "no filters applies defaults", + contextSetup: func(ctx context.Context) context.Context { return ctx }, + expectedTools: []string{"get_file_contents", "create_repository", "list_issues", "issue_write"}, + }, + { + name: "readonly from context filters write tools", + contextSetup: func(ctx context.Context) context.Context { + return ghcontext.WithReadonly(ctx, true) + }, + expectedTools: []string{"get_file_contents", "list_issues"}, + }, + { + name: "toolset from context filters to toolset", + contextSetup: func(ctx context.Context) context.Context { + return ghcontext.WithToolset(ctx, "repos") + }, + expectedTools: []string{"get_file_contents", "create_repository"}, + }, + { + name: "context toolset takes precedence over header", + contextSetup: func(ctx context.Context) context.Context { + return ghcontext.WithToolset(ctx, "repos") + }, + headers: map[string]string{ + headers.MCPToolsetsHeader: "issues", + }, + expectedTools: []string{"get_file_contents", "create_repository"}, + }, + { + name: "tools are additive with toolsets", + contextSetup: func(ctx context.Context) context.Context { return ctx }, + headers: map[string]string{ + headers.MCPToolsetsHeader: "repos", + headers.MCPToolsHeader: "list_issues", + }, + expectedTools: []string{"get_file_contents", "create_repository", "list_issues"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + for k, v := range tt.headers { + req.Header.Set(k, v) + } + req = req.WithContext(tt.contextSetup(req.Context())) + + builder := inventory.NewBuilder(). + SetTools(tools). + WithToolsets([]string{"all"}) + + builder = InventoryFiltersForRequest(req, builder) + inv := builder.Build() + + available := inv.AvailableTools(context.Background()) + toolNames := make([]string, len(available)) + for i, tool := range available { + toolNames[i] = tool.Tool.Name + } + + assert.ElementsMatch(t, tt.expectedTools, toolNames) + }) + } +} From e91a8758100c4c3c0a661ad5ae55eb47efe3a144 Mon Sep 17 00:00:00 2001 From: kerobbi Date: Thu, 22 Jan 2026 15:26:46 +0000 Subject: [PATCH 02/13] address feedback --- pkg/context/request.go | 20 ++++++------- pkg/http/handler.go | 61 ++++++++-------------------------------- pkg/http/handler_test.go | 13 +++++---- pkg/http/server.go | 2 ++ 4 files changed, 30 insertions(+), 66 deletions(-) diff --git a/pkg/context/request.go b/pkg/context/request.go index 1ca48f32b..42fb65dfa 100644 --- a/pkg/context/request.go +++ b/pkg/context/request.go @@ -18,18 +18,18 @@ func IsReadonly(ctx context.Context) bool { return false } -// toolsetCtxKey is a context key for the active toolset -type toolsetCtxKey struct{} +// toolsetsCtxKey is a context key for the active toolsets +type toolsetsCtxKey struct{} -// WithToolset adds the active toolset to the context -func WithToolset(ctx context.Context, toolset string) context.Context { - return context.WithValue(ctx, toolsetCtxKey{}, toolset) +// WithToolsets adds the active toolsets to the context +func WithToolsets(ctx context.Context, toolsets []string) context.Context { + return context.WithValue(ctx, toolsetsCtxKey{}, toolsets) } -// GetToolset retrieves the active toolset from the context -func GetToolset(ctx context.Context) string { - if toolset, ok := ctx.Value(toolsetCtxKey{}).(string); ok { - return toolset +// GetToolsets retrieves the active toolsets from the context +func GetToolsets(ctx context.Context) []string { + if toolsets, ok := ctx.Value(toolsetsCtxKey{}).([]string); ok { + return toolsets } - return "" + return nil } diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 6bb3c9b61..10ad6ec3f 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -4,8 +4,6 @@ import ( "context" "log/slog" "net/http" - "slices" - "strings" ghcontext "github.com/github/github-mcp-server/pkg/context" "github.com/github/github-mcp-server/pkg/github" @@ -99,7 +97,7 @@ func withReadonly(next http.Handler) http.Handler { func withToolset(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { toolset := chi.URLParam(r, "toolset") - ctx := ghcontext.WithToolset(r.Context(), toolset) + ctx := ghcontext.WithToolsets(r.Context(), []string{toolset}) next.ServeHTTP(w, r.WithContext(ctx)) }) } @@ -143,7 +141,7 @@ func DefaultInventoryFactory(cfg *HTTPServerConfig, t translations.TranslationHe b := github.NewInventory(t).WithDeprecatedAliases(github.DeprecatedToolAliases) // Feature checker composition - headerFeatures := parseCommaSeparatedHeader(r.Header.Get(headers.MCPFeaturesHeader)) + headerFeatures := headers.ParseCommaSeparated(r.Header.Get(headers.MCPFeaturesHeader)) if checker := ComposeFeatureChecker(headerFeatures, staticChecker); checker != nil { b = b.WithFeatureChecker(checker) } @@ -153,62 +151,25 @@ func DefaultInventoryFactory(cfg *HTTPServerConfig, t translations.TranslationHe } } -// InventoryFiltersForRequest applies inventory filters from request context and headers -// Whitespace is trimmed from comma-separated values; empty values are ignored -// Route configuration (context) takes precedence over headers for toolsets +// InventoryFiltersForRequest applies filters to the inventory builder +// based on the request context and headers func InventoryFiltersForRequest(r *http.Request, builder *inventory.Builder) *inventory.Builder { ctx := r.Context() - // Enable readonly mode if set in context or via header - if ghcontext.IsReadonly(ctx) || relaxedParseBool(r.Header.Get(headers.MCPReadOnlyHeader)) { + if ghcontext.IsReadonly(ctx) { builder = builder.WithReadOnly(true) } - // Parse request configuration - contextToolset := ghcontext.GetToolset(ctx) - headerToolsets := parseCommaSeparatedHeader(r.Header.Get(headers.MCPToolsetsHeader)) - tools := parseCommaSeparatedHeader(r.Header.Get(headers.MCPToolsHeader)) - - // Apply toolset filtering (route wins, then header, then tools-only mode, else defaults) - switch { - case contextToolset != "": - builder = builder.WithToolsets([]string{contextToolset}) - case len(headerToolsets) > 0: - builder = builder.WithToolsets(headerToolsets) - case len(tools) > 0: - builder = builder.WithToolsets([]string{}) + if toolsets := ghcontext.GetToolsets(ctx); len(toolsets) > 0 { + builder = builder.WithToolsets(toolsets) } - if len(tools) > 0 { + if tools := headers.ParseCommaSeparated(r.Header.Get(headers.MCPToolsHeader)); len(tools) > 0 { + if len(ghcontext.GetToolsets(ctx)) == 0 { + builder = builder.WithToolsets([]string{}) + } builder = builder.WithTools(github.CleanTools(tools)) } return builder } - -// parseCommaSeparatedHeader splits a header value by comma, trims whitespace, -// and filters out empty values. -func parseCommaSeparatedHeader(value string) []string { - if value == "" { - return []string{} - } - - parts := strings.Split(value, ",") - result := make([]string, 0, len(parts)) - for _, p := range parts { - trimmed := strings.TrimSpace(p) - if trimmed != "" { - result = append(result, trimmed) - } - } - return result -} - -// relaxedParseBool parses a string into a boolean value, treating various -// common false values or empty strings as false, and everything else as true. -// It is case-insensitive and trims whitespace. -func relaxedParseBool(s string) bool { - s = strings.TrimSpace(strings.ToLower(s)) - falseValues := []string{"", "false", "0", "no", "off", "n", "f"} - return !slices.Contains(falseValues, s) -} diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index 982a16c4c..da30f5a03 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -55,14 +55,14 @@ func TestInventoryFiltersForRequest(t *testing.T) { { name: "toolset from context filters to toolset", contextSetup: func(ctx context.Context) context.Context { - return ghcontext.WithToolset(ctx, "repos") + return ghcontext.WithToolsets(ctx, []string{"repos"}) }, expectedTools: []string{"get_file_contents", "create_repository"}, }, { name: "context toolset takes precedence over header", contextSetup: func(ctx context.Context) context.Context { - return ghcontext.WithToolset(ctx, "repos") + return ghcontext.WithToolsets(ctx, []string{"repos"}) }, headers: map[string]string{ headers.MCPToolsetsHeader: "issues", @@ -70,11 +70,12 @@ func TestInventoryFiltersForRequest(t *testing.T) { expectedTools: []string{"get_file_contents", "create_repository"}, }, { - name: "tools are additive with toolsets", - contextSetup: func(ctx context.Context) context.Context { return ctx }, + name: "tools are additive with toolsets", + contextSetup: func(ctx context.Context) context.Context { + return ghcontext.WithToolsets(ctx, []string{"repos"}) + }, headers: map[string]string{ - headers.MCPToolsetsHeader: "repos", - headers.MCPToolsHeader: "list_issues", + headers.MCPToolsHeader: "list_issues", }, expectedTools: []string{"get_file_contents", "create_repository", "list_issues"}, }, diff --git a/pkg/http/server.go b/pkg/http/server.go index ac9a35c08..0fd1ad581 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -12,6 +12,7 @@ import ( "time" "github.com/github/github-mcp-server/pkg/github" + "github.com/github/github-mcp-server/pkg/http/middleware" "github.com/github/github-mcp-server/pkg/lockdown" "github.com/github/github-mcp-server/pkg/translations" "github.com/github/github-mcp-server/pkg/utils" @@ -98,6 +99,7 @@ func RunHTTPServer(cfg HTTPServerConfig) error { r := chi.NewRouter() handler := NewHTTPMcpHandler(&cfg, deps, t, logger) + r.Use(middleware.WithRequestConfig) handler.RegisterRoutes(r) addr := fmt.Sprintf(":%d", cfg.Port) From d9e9bae79620c94c5be88c2918fd29b720ea54f8 Mon Sep 17 00:00:00 2001 From: kerobbi Date: Thu, 22 Jan 2026 15:27:10 +0000 Subject: [PATCH 03/13] forgotten files --- pkg/http/headers/parse.go | 21 ++++++++++++++ pkg/http/middleware/request_config.go | 41 +++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 pkg/http/headers/parse.go create mode 100644 pkg/http/middleware/request_config.go diff --git a/pkg/http/headers/parse.go b/pkg/http/headers/parse.go new file mode 100644 index 000000000..2b5eddacd --- /dev/null +++ b/pkg/http/headers/parse.go @@ -0,0 +1,21 @@ +package headers + +import "strings" + +// ParseCommaSeparated splits a header value by comma, trims whitespace, +// and filters out empty values +func ParseCommaSeparated(value string) []string { + if value == "" { + return []string{} + } + + parts := strings.Split(value, ",") + result := make([]string, 0, len(parts)) + for _, p := range parts { + trimmed := strings.TrimSpace(p) + if trimmed != "" { + result = append(result, trimmed) + } + } + return result +} diff --git a/pkg/http/middleware/request_config.go b/pkg/http/middleware/request_config.go new file mode 100644 index 000000000..b55cabb0c --- /dev/null +++ b/pkg/http/middleware/request_config.go @@ -0,0 +1,41 @@ +package middleware + +import ( + "net/http" + "slices" + "strings" + + ghcontext "github.com/github/github-mcp-server/pkg/context" + "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 +func WithRequestConfig(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + // Only set from headers if not already set in ctx + if !ghcontext.IsReadonly(ctx) { + if relaxedParseBool(r.Header.Get(headers.MCPReadOnlyHeader)) { + ctx = ghcontext.WithReadonly(ctx, true) + } + } + + if len(ghcontext.GetToolsets(ctx)) == 0 { + if toolsets := headers.ParseCommaSeparated(r.Header.Get(headers.MCPToolsetsHeader)); len(toolsets) > 0 { + ctx = ghcontext.WithToolsets(ctx, toolsets) + } + } + + next.ServeHTTP(w, r.WithContext(ctx)) + }) +} + +// relaxedParseBool parses a string into a boolean value, treating various +// common false values or empty strings as false, and everything else as true. +// It is case-insensitive and trims whitespace. +func relaxedParseBool(s string) bool { + s = strings.TrimSpace(strings.ToLower(s)) + falseValues := []string{"", "false", "0", "no", "off", "n", "f"} + return !slices.Contains(falseValues, s) +} From 9320ca00af6bb8ca2af80b154c6134c023d89710 Mon Sep 17 00:00:00 2001 From: kerobbi Date: Thu, 22 Jan 2026 16:22:03 +0000 Subject: [PATCH 04/13] remove redundant checks in WithRequestConfig --- pkg/http/middleware/request_config.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/pkg/http/middleware/request_config.go b/pkg/http/middleware/request_config.go index b55cabb0c..3ba23fd8b 100644 --- a/pkg/http/middleware/request_config.go +++ b/pkg/http/middleware/request_config.go @@ -14,17 +14,12 @@ func WithRequestConfig(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - // Only set from headers if not already set in ctx - if !ghcontext.IsReadonly(ctx) { - if relaxedParseBool(r.Header.Get(headers.MCPReadOnlyHeader)) { - ctx = ghcontext.WithReadonly(ctx, true) - } + if relaxedParseBool(r.Header.Get(headers.MCPReadOnlyHeader)) { + ctx = ghcontext.WithReadonly(ctx, true) } - if len(ghcontext.GetToolsets(ctx)) == 0 { - if toolsets := headers.ParseCommaSeparated(r.Header.Get(headers.MCPToolsetsHeader)); len(toolsets) > 0 { - ctx = ghcontext.WithToolsets(ctx, toolsets) - } + if toolsets := headers.ParseCommaSeparated(r.Header.Get(headers.MCPToolsetsHeader)); len(toolsets) > 0 { + ctx = ghcontext.WithToolsets(ctx, toolsets) } next.ServeHTTP(w, r.WithContext(ctx)) From d45c41484132763219bd4f6734d4b9d5e814bc2a Mon Sep 17 00:00:00 2001 From: kerobbi Date: Fri, 23 Jan 2026 10:53:40 +0000 Subject: [PATCH 05/13] move middleware in RegisterRoutes --- pkg/http/handler.go | 4 +++- pkg/http/server.go | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 10ad6ec3f..0009f45ae 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -76,9 +76,11 @@ func NewHTTPMcpHandler(cfg *HTTPServerConfig, } } +// RegisterRoutes registers the routes for the MCP server func (h *HTTPMcpHandler) RegisterRoutes(r chi.Router) { - r.Mount("/", h) + r.Use(middleware.WithRequestConfig) + r.Mount("/", h) // Mount readonly and toolset routes r.With(withToolset).Mount("/x/{toolset}", h) r.With(withReadonly, withToolset).Mount("/x/{toolset}/readonly", h) diff --git a/pkg/http/server.go b/pkg/http/server.go index 0fd1ad581..ac9a35c08 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -12,7 +12,6 @@ import ( "time" "github.com/github/github-mcp-server/pkg/github" - "github.com/github/github-mcp-server/pkg/http/middleware" "github.com/github/github-mcp-server/pkg/lockdown" "github.com/github/github-mcp-server/pkg/translations" "github.com/github/github-mcp-server/pkg/utils" @@ -99,7 +98,6 @@ func RunHTTPServer(cfg HTTPServerConfig) error { r := chi.NewRouter() handler := NewHTTPMcpHandler(&cfg, deps, t, logger) - r.Use(middleware.WithRequestConfig) handler.RegisterRoutes(r) addr := fmt.Sprintf(":%d", cfg.Port) From 854bbd2c9c726c583ab7586967b3aa15e2ba287b Mon Sep 17 00:00:00 2001 From: kerobbi Date: Fri, 23 Jan 2026 12:10:29 +0000 Subject: [PATCH 06/13] improve comment and add TestParseCommaSeparated --- pkg/http/handler.go | 1 + pkg/http/headers/parse_test.go | 58 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 pkg/http/headers/parse_test.go diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 0009f45ae..e01924fde 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -77,6 +77,7 @@ func NewHTTPMcpHandler(cfg *HTTPServerConfig, } // RegisterRoutes registers the routes for the MCP server +// URL-based values take precedence over header-based values func (h *HTTPMcpHandler) RegisterRoutes(r chi.Router) { r.Use(middleware.WithRequestConfig) diff --git a/pkg/http/headers/parse_test.go b/pkg/http/headers/parse_test.go new file mode 100644 index 000000000..d8b55a696 --- /dev/null +++ b/pkg/http/headers/parse_test.go @@ -0,0 +1,58 @@ +package headers + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseCommaSeparated(t *testing.T) { + tests := []struct { + name string + input string + expected []string + }{ + { + name: "empty string", + input: "", + expected: []string{}, + }, + { + name: "single value", + input: "foo", + expected: []string{"foo"}, + }, + { + name: "multiple values", + input: "foo,bar,baz", + expected: []string{"foo", "bar", "baz"}, + }, + { + name: "whitespace trimmed", + input: " foo , bar , baz ", + expected: []string{"foo", "bar", "baz"}, + }, + { + name: "empty values filtered", + input: "foo,,bar,", + expected: []string{"foo", "bar"}, + }, + { + name: "only commas", + input: ",,,", + expected: []string{}, + }, + { + name: "whitespace only values filtered", + input: "foo, ,bar", + expected: []string{"foo", "bar"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ParseCommaSeparated(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} From 283b9ba59b9de2156017e21d71035cd179e7d3bf Mon Sep 17 00:00:00 2001 From: kerobbi Date: Fri, 23 Jan 2026 12:16:09 +0000 Subject: [PATCH 07/13] fix broken TestInventoryFiltersForRequest --- pkg/http/handler_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index da30f5a03..07df5b35a 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -11,6 +11,7 @@ import ( "github.com/github/github-mcp-server/pkg/inventory" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func mockTool(name, toolsetID string, readOnly bool) inventory.ServerTool { @@ -94,7 +95,8 @@ func TestInventoryFiltersForRequest(t *testing.T) { WithToolsets([]string{"all"}) builder = InventoryFiltersForRequest(req, builder) - inv := builder.Build() + inv, err := builder.Build() + require.NoError(t, err) available := inv.AvailableTools(context.Background()) toolNames := make([]string, len(available)) From 53dfd4c48bf599146685d046a5ae957ef6672ea3 Mon Sep 17 00:00:00 2001 From: kerobbi Date: Fri, 23 Jan 2026 14:22:11 +0000 Subject: [PATCH 08/13] parse X-MCP-Tools into ctx --- pkg/context/request.go | 16 ++++++++++++++++ pkg/http/handler.go | 2 +- pkg/http/handler_test.go | 7 +++---- pkg/http/middleware/request_config.go | 4 ++++ 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/pkg/context/request.go b/pkg/context/request.go index 42fb65dfa..8b9169955 100644 --- a/pkg/context/request.go +++ b/pkg/context/request.go @@ -33,3 +33,19 @@ func GetToolsets(ctx context.Context) []string { } return nil } + +// toolsCtxKey is a context key for tools +type toolsCtxKey struct{} + +// WithTools adds the tools to the context +func WithTools(ctx context.Context, tools []string) context.Context { + return context.WithValue(ctx, toolsCtxKey{}, tools) +} + +// GetTools retrieves the tools from the context +func GetTools(ctx context.Context) []string { + if tools, ok := ctx.Value(toolsCtxKey{}).([]string); ok { + return tools + } + return nil +} diff --git a/pkg/http/handler.go b/pkg/http/handler.go index f7c8ae1f4..9f7fd71fa 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -173,7 +173,7 @@ func InventoryFiltersForRequest(r *http.Request, builder *inventory.Builder) *in builder = builder.WithToolsets(toolsets) } - if tools := headers.ParseCommaSeparated(r.Header.Get(headers.MCPToolsHeader)); len(tools) > 0 { + if tools := ghcontext.GetTools(ctx); len(tools) > 0 { if len(ghcontext.GetToolsets(ctx)) == 0 { builder = builder.WithToolsets([]string{}) } diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index 07df5b35a..ac7dc95f4 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -73,10 +73,9 @@ func TestInventoryFiltersForRequest(t *testing.T) { { name: "tools are additive with toolsets", contextSetup: func(ctx context.Context) context.Context { - return ghcontext.WithToolsets(ctx, []string{"repos"}) - }, - headers: map[string]string{ - headers.MCPToolsHeader: "list_issues", + ctx = ghcontext.WithToolsets(ctx, []string{"repos"}) + ctx = ghcontext.WithTools(ctx, []string{"list_issues"}) + return ctx }, expectedTools: []string{"get_file_contents", "create_repository", "list_issues"}, }, diff --git a/pkg/http/middleware/request_config.go b/pkg/http/middleware/request_config.go index 3ba23fd8b..0f7d3b2c7 100644 --- a/pkg/http/middleware/request_config.go +++ b/pkg/http/middleware/request_config.go @@ -22,6 +22,10 @@ func WithRequestConfig(next http.Handler) http.Handler { ctx = ghcontext.WithToolsets(ctx, toolsets) } + if tools := headers.ParseCommaSeparated(r.Header.Get(headers.MCPToolsHeader)); len(tools) > 0 { + ctx = ghcontext.WithTools(ctx, tools) + } + next.ServeHTTP(w, r.WithContext(ctx)) }) } From a92b77b687f15a826247c2580847f0527454b0bc Mon Sep 17 00:00:00 2001 From: kerobbi Date: Fri, 23 Jan 2026 14:29:48 +0000 Subject: [PATCH 09/13] clean up TestInventoryFiltersForRequest --- pkg/http/handler_test.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index ac7dc95f4..83a2438d7 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -7,7 +7,6 @@ import ( "testing" ghcontext "github.com/github/github-mcp-server/pkg/context" - "github.com/github/github-mcp-server/pkg/http/headers" "github.com/github/github-mcp-server/pkg/inventory" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" @@ -38,7 +37,6 @@ func TestInventoryFiltersForRequest(t *testing.T) { tests := []struct { name string contextSetup func(context.Context) context.Context - headers map[string]string expectedTools []string }{ { @@ -61,14 +59,11 @@ func TestInventoryFiltersForRequest(t *testing.T) { expectedTools: []string{"get_file_contents", "create_repository"}, }, { - name: "context toolset takes precedence over header", + name: "tools alone clears default toolsets", contextSetup: func(ctx context.Context) context.Context { - return ghcontext.WithToolsets(ctx, []string{"repos"}) - }, - headers: map[string]string{ - headers.MCPToolsetsHeader: "issues", + return ghcontext.WithTools(ctx, []string{"list_issues"}) }, - expectedTools: []string{"get_file_contents", "create_repository"}, + expectedTools: []string{"list_issues"}, }, { name: "tools are additive with toolsets", @@ -84,9 +79,6 @@ func TestInventoryFiltersForRequest(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", nil) - for k, v := range tt.headers { - req.Header.Set(k, v) - } req = req.WithContext(tt.contextSetup(req.Context())) builder := inventory.NewBuilder(). From bbb1d10bba454c232b737ff14c733623ad16ed7f Mon Sep 17 00:00:00 2001 From: kerobbi Date: Thu, 22 Jan 2026 11:43:56 +0000 Subject: [PATCH 10/13] review http args and add lockdown ctx helpers --- cmd/github-mcp-server/main.go | 3 +++ pkg/context/request.go | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index b9d8af64f..ed5ea961a 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -96,6 +96,7 @@ var ( Short: "Start HTTP server", Long: `Start an HTTP server that listens for MCP requests over HTTP.`, RunE: func(_ *cobra.Command, _ []string) error { + ttl := viper.GetDuration("repo-access-cache-ttl") httpConfig := ghhttp.HTTPServerConfig{ Version: version, Host: viper.GetString("host"), @@ -104,6 +105,8 @@ var ( EnableCommandLogging: viper.GetBool("enable-command-logging"), LogFilePath: viper.GetString("log-file"), ContentWindowSize: viper.GetInt("content-window-size"), + LockdownMode: viper.GetBool("lockdown-mode"), + RepoAccessCacheTTL: &ttl, } return ghhttp.RunHTTPServer(httpConfig) diff --git a/pkg/context/request.go b/pkg/context/request.go index 8b9169955..94882a3ce 100644 --- a/pkg/context/request.go +++ b/pkg/context/request.go @@ -49,3 +49,19 @@ func GetTools(ctx context.Context) []string { } return nil } + +// lockdownCtxKey is a context key for lockdown mode +type lockdownCtxKey struct{} + +// WithLockdownMode adds lockdown mode state to the context +func WithLockdownMode(ctx context.Context, enabled bool) context.Context { + return context.WithValue(ctx, lockdownCtxKey{}, enabled) +} + +// IsLockdownMode retrieves the lockdown mode state from the context +func IsLockdownMode(ctx context.Context) bool { + if enabled, ok := ctx.Value(lockdownCtxKey{}).(bool); ok { + return enabled + } + return false +} From 07e4b96968f755d424e54214ee3367750b3b46bb Mon Sep 17 00:00:00 2001 From: kerobbi Date: Thu, 22 Jan 2026 11:47:05 +0000 Subject: [PATCH 11/13] parse lockdown header and update GetFlags to retrieve ff from ctx --- pkg/github/dependencies.go | 10 +++++++--- pkg/github/issues.go | 6 +++--- pkg/github/pullrequests.go | 6 +++--- pkg/http/handler.go | 4 ++++ pkg/http/headers/headers.go | 2 ++ 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go index d9f9b602d..34c56a1d1 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -87,7 +87,7 @@ type ToolDependencies interface { GetT() translations.TranslationHelperFunc // GetFlags returns feature flags - GetFlags() FeatureFlags + GetFlags(ctx context.Context) FeatureFlags // GetContentWindowSize returns the content window size for log truncation GetContentWindowSize() int @@ -165,7 +165,7 @@ func (d BaseDeps) GetRepoAccessCache(_ context.Context) (*lockdown.RepoAccessCac func (d BaseDeps) GetT() translations.TranslationHelperFunc { return d.T } // GetFlags implements ToolDependencies. -func (d BaseDeps) GetFlags() FeatureFlags { return d.Flags } +func (d BaseDeps) GetFlags(_ context.Context) FeatureFlags { return d.Flags } // GetContentWindowSize implements ToolDependencies. func (d BaseDeps) GetContentWindowSize() int { return d.ContentWindowSize } @@ -379,7 +379,11 @@ func (d *RequestDeps) GetRepoAccessCache(ctx context.Context) (*lockdown.RepoAcc func (d *RequestDeps) GetT() translations.TranslationHelperFunc { return d.T } // GetFlags implements ToolDependencies. -func (d *RequestDeps) GetFlags() FeatureFlags { return d.Flags } +func (d *RequestDeps) GetFlags(ctx context.Context) FeatureFlags { + return FeatureFlags{ + LockdownMode: ghcontext.IsLockdownMode(ctx), + } +} // GetContentWindowSize implements ToolDependencies. func (d *RequestDeps) GetContentWindowSize() int { return d.ContentWindowSize } diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 309e03ff3..c4cc54175 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -334,7 +334,7 @@ func GetIssue(ctx context.Context, client *github.Client, deps ToolDependencies, if err != nil { return nil, fmt.Errorf("failed to get repo access cache: %w", err) } - flags := deps.GetFlags() + flags := deps.GetFlags(ctx) issue, resp, err := client.Issues.Get(ctx, owner, repo, issueNumber) if err != nil { @@ -389,7 +389,7 @@ func GetIssueComments(ctx context.Context, client *github.Client, deps ToolDepen if err != nil { return nil, fmt.Errorf("failed to get repo access cache: %w", err) } - flags := deps.GetFlags() + flags := deps.GetFlags(ctx) opts := &github.IssueListCommentsOptions{ ListOptions: github.ListOptions{ @@ -449,7 +449,7 @@ func GetSubIssues(ctx context.Context, client *github.Client, deps ToolDependenc if err != nil { return nil, fmt.Errorf("failed to get repo access cache: %w", err) } - featureFlags := deps.GetFlags() + featureFlags := deps.GetFlags(ctx) opts := &github.IssueListOptions{ ListOptions: github.ListOptions{ diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 614e4496f..5d9be21f1 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -139,7 +139,7 @@ func GetPullRequest(ctx context.Context, client *github.Client, deps ToolDepende if err != nil { return nil, fmt.Errorf("failed to get repo access cache: %w", err) } - ff := deps.GetFlags() + ff := deps.GetFlags(ctx) pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) if err != nil { @@ -350,7 +350,7 @@ func GetPullRequestReviewComments(ctx context.Context, gqlClient *githubv4.Clien if err != nil { return nil, fmt.Errorf("failed to get repo access cache: %w", err) } - ff := deps.GetFlags() + ff := deps.GetFlags(ctx) // Convert pagination parameters to GraphQL format gqlParams, err := pagination.ToGraphQLParams() @@ -437,7 +437,7 @@ func GetPullRequestReviews(ctx context.Context, client *github.Client, deps Tool if err != nil { return nil, fmt.Errorf("failed to get repo access cache: %w", err) } - ff := deps.GetFlags() + ff := deps.GetFlags(ctx) reviews, resp, err := client.PullRequests.ListReviews(ctx, owner, repo, pullNumber, nil) if err != nil { diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 9f7fd71fa..2d210591a 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -106,6 +106,10 @@ func withToolset(next http.Handler) http.Handler { } func (h *HTTPMcpHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if relaxedParseBool(r.Header.Get(headers.MCPLockdownHeader)) { + r = r.WithContext(ghcontext.WithLockdownMode(r.Context(), true)) + } + inventory, err := h.inventoryFactoryFunc(r) if err != nil { w.WriteHeader(http.StatusInternalServerError) diff --git a/pkg/http/headers/headers.go b/pkg/http/headers/headers.go index b73104c34..a1580cf96 100644 --- a/pkg/http/headers/headers.go +++ b/pkg/http/headers/headers.go @@ -32,6 +32,8 @@ const ( MCPToolsetsHeader = "X-MCP-Toolsets" // MCPToolsHeader is a comma-separated list of MCP tools that the request is for. MCPToolsHeader = "X-MCP-Tools" + // MCPLockdownHeader indicates whether lockdown mode is enabled. + MCPLockdownHeader = "X-MCP-Lockdown" // MCPFeaturesHeader is a comma-separated list of feature flags to enable. MCPFeaturesHeader = "X-MCP-Features" ) From cc173c3684e5b544a6f011845c3c444f9fdcb5ad Mon Sep 17 00:00:00 2001 From: kerobbi Date: Thu, 22 Jan 2026 11:47:25 +0000 Subject: [PATCH 12/13] clean up and fix tests --- pkg/github/dependencies.go | 2 -- pkg/github/server_test.go | 2 +- pkg/http/server.go | 3 --- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go index 34c56a1d1..dc67a4da5 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -262,7 +262,6 @@ func NewRequestDeps( lockdownMode bool, repoAccessOpts []lockdown.RepoAccessOption, t translations.TranslationHelperFunc, - flags FeatureFlags, contentWindowSize int, featureChecker inventory.FeatureFlagChecker, ) *RequestDeps { @@ -272,7 +271,6 @@ func NewRequestDeps( lockdownMode: lockdownMode, RepoAccessOpts: repoAccessOpts, T: t, - Flags: flags, ContentWindowSize: contentWindowSize, featureChecker: featureChecker, } diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index d8ce27ed9..807195c9b 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -56,7 +56,7 @@ func (s stubDeps) GetRepoAccessCache(ctx context.Context) (*lockdown.RepoAccessC return s.repoAccessCache, nil } func (s stubDeps) GetT() translations.TranslationHelperFunc { return s.t } -func (s stubDeps) GetFlags() FeatureFlags { return s.flags } +func (s stubDeps) GetFlags(ctx context.Context) FeatureFlags { return s.flags } func (s stubDeps) GetContentWindowSize() int { return s.contentWindowSize } func (s stubDeps) IsFeatureEnabled(_ context.Context, _ string) bool { return false } diff --git a/pkg/http/server.go b/pkg/http/server.go index c6054f727..5bc43723c 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -89,9 +89,6 @@ func RunHTTPServer(cfg HTTPServerConfig) error { cfg.LockdownMode, repoAccessOpts, t, - github.FeatureFlags{ - LockdownMode: cfg.LockdownMode, - }, cfg.ContentWindowSize, nil, ) From 4e54bf033aed5a49a5e888d314bb2f58f836072b Mon Sep 17 00:00:00 2001 From: kerobbi Date: Fri, 23 Jan 2026 13:23:04 +0000 Subject: [PATCH 13/13] fix GetFlags check, move lockdown parsing in WithRequestConfig and fix broken tests --- pkg/github/dependencies.go | 2 +- pkg/github/feature_flags_test.go | 2 +- pkg/http/handler.go | 4 ---- pkg/http/middleware/request_config.go | 4 ++++ 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/github/dependencies.go b/pkg/github/dependencies.go index dc67a4da5..bdcafe933 100644 --- a/pkg/github/dependencies.go +++ b/pkg/github/dependencies.go @@ -379,7 +379,7 @@ func (d *RequestDeps) GetT() translations.TranslationHelperFunc { return d.T } // GetFlags implements ToolDependencies. func (d *RequestDeps) GetFlags(ctx context.Context) FeatureFlags { return FeatureFlags{ - LockdownMode: ghcontext.IsLockdownMode(ctx), + LockdownMode: d.lockdownMode && ghcontext.IsLockdownMode(ctx), } } diff --git a/pkg/github/feature_flags_test.go b/pkg/github/feature_flags_test.go index fb50448af..f9cfe4acb 100644 --- a/pkg/github/feature_flags_test.go +++ b/pkg/github/feature_flags_test.go @@ -45,7 +45,7 @@ func HelloWorldTool(t translations.TranslationHelperFunc) inventory.ServerTool { if deps.IsFeatureEnabled(ctx, RemoteMCPEnthusiasticGreeting) { greeting += " Welcome to the future of MCP! 🎉" } - if deps.GetFlags().InsiderMode { + if deps.GetFlags(ctx).InsiderMode { greeting += " Experimental features are enabled! 🚀" } diff --git a/pkg/http/handler.go b/pkg/http/handler.go index 2d210591a..9f7fd71fa 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -106,10 +106,6 @@ func withToolset(next http.Handler) http.Handler { } func (h *HTTPMcpHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - if relaxedParseBool(r.Header.Get(headers.MCPLockdownHeader)) { - r = r.WithContext(ghcontext.WithLockdownMode(r.Context(), true)) - } - inventory, err := h.inventoryFactoryFunc(r) if err != nil { w.WriteHeader(http.StatusInternalServerError) diff --git a/pkg/http/middleware/request_config.go b/pkg/http/middleware/request_config.go index 0f7d3b2c7..da9127bf1 100644 --- a/pkg/http/middleware/request_config.go +++ b/pkg/http/middleware/request_config.go @@ -26,6 +26,10 @@ func WithRequestConfig(next http.Handler) http.Handler { ctx = ghcontext.WithTools(ctx, tools) } + if relaxedParseBool(r.Header.Get(headers.MCPLockdownHeader)) { + ctx = ghcontext.WithLockdownMode(ctx, true) + } + next.ServeHTTP(w, r.WithContext(ctx)) }) }