From c35ab932750f042f3cb9f0aaddd91e5e229802fc Mon Sep 17 00:00:00 2001 From: kerobbi Date: Wed, 21 Jan 2026 15:27:23 +0000 Subject: [PATCH] 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) + }) + } +}