Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion docs/feature-flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,24 @@ section in the Insiders docs](./insiders-features.md#how-feature-flags-are-resol
| Method | Remote Server | Local Server |
|--------|---------------|--------------|
| Header | `X-MCP-Features: <flag>,<flag>` | N/A |
| URL query parameter | `?features=<flag>,<flag>` on the server URL | N/A |
| CLI flag | N/A | `--features=<flag>,<flag>` |
| Environment variable | N/A | `GITHUB_FEATURES=<flag>,<flag>` |

The URL query parameter exists for clients that compose the server URL on the
user's behalf (hosted IDEs, agent platforms) and cannot set custom headers.
When both the query parameter and the header are present, the header wins —
even when its value is empty, whitespace-only, or contains only unknown flags.
The two channels are never combined.

The complete query string is preserved during OAuth protected-resource metadata
discovery because it is part of the canonical resource identifier. Query
parameters also participate in HTTP cache keys, while MCP responses vary on
`X-MCP-Features` so a header override cannot reuse a response selected for a
different feature set. Feature names are configuration identifiers, not
secrets; as with any URL query value, they may appear in client history, proxy
logs, and server access logs.

Only flags listed in
[`AllowedFeatureFlags`](../pkg/github/feature_flags.go) can be enabled by
end users. Insiders-only flags are not user-toggleable.
Expand Down Expand Up @@ -357,7 +372,7 @@ runtime behavior (such as output formatting) won't appear here.
### `thread_resolution_reason`

- **pull_request_review_write** - Write operations (create, submit, delete) on pull request reviews
- **Required OAuth Scopes**: `repo`
- **OAuth Challenge Scopes**: `repo`
- `body`: Review comment text (string, optional)
- `commitID`: SHA of commit to review (string, optional)
- `event`: Review action to perform. (string, optional)
Expand Down
7 changes: 5 additions & 2 deletions docs/insiders-features.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,9 @@ Insiders is a **meta feature flag** — the same shape as `default` or `all` for

1. **User input.** Users may opt into specific features:
- Local server: `--features=<flag>,<flag>` CLI flag (or `GITHUB_FEATURES` env var).
- Self-hosted HTTP server: `X-MCP-Features: <flag>,<flag>` request header.
- HTTP server: `X-MCP-Features: <flag>,<flag>` request header or a
`?features=<flag>,<flag>` server URL. Header presence takes precedence,
and the two request channels are never combined.
2. **Allowlist filter.** User-supplied flags are filtered against [`AllowedFeatureFlags`](../pkg/github/feature_flags.go). Anything not on the allowlist is silently dropped — flags missing from the allowlist can only be turned on by remote-server feature management, not by end users.
3. **Insiders expansion.** If insiders mode is on (`--insiders`, `/insiders` route, or `X-MCP-Insiders: true`), every flag in [`InsidersFeatureFlags`](../pkg/github/feature_flags.go) is unioned in. The insiders expansion is **not** re-validated against the allowlist — insiders is a server-controlled switch that can reach internal-only flags.
4. **Server-side fallback (remote server only).** Any flag not yet decided falls back to the remote server's feature manager, which can roll a feature out independently of user input or insiders membership.
Expand All @@ -214,7 +216,8 @@ Insiders is a **meta feature flag** — the same shape as `default` or `all` for
### Adding a new feature flag

1. Add a constant in `pkg/github/feature_flags.go`.
2. Add it to `AllowedFeatureFlags` if end users should be able to opt in via `--features` / `X-MCP-Features`.
2. Add it to `AllowedFeatureFlags` if end users should be able to opt in via
`--features`, `X-MCP-Features`, or the `features` URL query parameter.
3. Add it to `InsidersFeatureFlags` if insiders mode should turn it on automatically.
4. Gate the behavior on the concrete flag (`deps.IsFeatureEnabled(ctx, FeatureFlagX)`), never on `cfg.InsidersMode`. There is a `TestGitHubPackageDoesNotReadInsidersMode` guard test that fails if `pkg/github` reads `InsidersMode` directly.
5. The MCP-diff CI workflow picks up new entries in `AllowedFeatureFlags` automatically — see `.github/workflows/mcp-diff.yml`.
2 changes: 1 addition & 1 deletion docs/server-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ We currently support the following ways in which the GitHub MCP Server can be co
| Read-Only Mode | `X-MCP-Readonly` header or `/readonly` URL | `--read-only` flag or `GITHUB_READ_ONLY` env var |
| Lockdown Mode | `X-MCP-Lockdown` header | `--lockdown-mode` flag or `GITHUB_LOCKDOWN_MODE` env var |
| Insiders Mode | `X-MCP-Insiders` header or `/insiders` URL | `--insiders` flag or `GITHUB_INSIDERS` env var |
| Feature Flags | `X-MCP-Features` header | `--features` flag |
| Feature Flags | `X-MCP-Features` header or `?features=` URL query parameter | `--features` flag |
| Scope Filtering | Always enabled | Always enabled |
| Server Name/Title | Not available | `GITHUB_MCP_SERVER_NAME` / `GITHUB_MCP_SERVER_TITLE` env vars or `github-mcp-server-config.json` |

Expand Down
6 changes: 3 additions & 3 deletions pkg/context/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,15 @@ func GetExcludeTools(ctx context.Context) []string {
return nil
}

// headerFeaturesCtxKey is a context key for raw header feature flags
// headerFeaturesCtxKey is a context key for raw HTTP request feature flags.
type headerFeaturesCtxKey struct{}

// WithHeaderFeatures stores the raw feature flags from the X-MCP-Features header into context
// WithHeaderFeatures stores raw HTTP request feature flags in context.
func WithHeaderFeatures(ctx context.Context, features []string) context.Context {
return context.WithValue(ctx, headerFeaturesCtxKey{}, features)
}

// GetHeaderFeatures retrieves the raw feature flags from context
// GetHeaderFeatures retrieves raw HTTP request feature flags from context.
func GetHeaderFeatures(ctx context.Context) []string {
if features, ok := ctx.Value(headerFeaturesCtxKey{}).([]string); ok {
return features
Expand Down
6 changes: 4 additions & 2 deletions pkg/github/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ const FeatureFlagDuplicateDetection = "duplicate_detection"
const FeatureFlagThreadResolutionReason = "thread_resolution_reason"

// AllowedFeatureFlags is the allowlist of feature flags that can be enabled
// by users via --features CLI flag or X-MCP-Features HTTP header.
// by users via --features CLI flag, X-MCP-Features HTTP header, or the
// features URL query parameter.
// Only flags in this list are accepted; unknown flags are silently ignored.
// This is the single source of truth for which flags are user-controllable.
var AllowedFeatureFlags = []string{
Expand Down Expand Up @@ -71,7 +72,8 @@ type FeatureFlags struct {
}

// ResolveFeatureFlags computes the effective set of enabled feature flags by:
// 1. Taking the user-supplied flags (from --features or X-MCP-Features) and
// 1. Taking the user-supplied flags (from --features or HTTP request
// configuration) and
// keeping only those present in AllowedFeatureFlags. Unknown or unsafe
// flags from request input are silently dropped here.
// 2. If insiders mode is on, unioning in every flag from InsidersFeatureFlags.
Expand Down
6 changes: 3 additions & 3 deletions pkg/github/tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ var (
FeatureFlagPullRequestsGranular = "pull_requests_granular"
)

// HeaderAllowedFeatureFlags returns the feature flags that clients may enable via
// the X-MCP-Features header. It delegates to AllowedFeatureFlags as the single
// source of truth.
// HeaderAllowedFeatureFlags returns the feature flags that clients may enable
// through the X-MCP-Features header or features URL query parameter. It
// delegates to AllowedFeatureFlags as the single source of truth.
func HeaderAllowedFeatureFlags() []string {
return slices.Clone(AllowedFeatureFlags)
}
Expand Down
40 changes: 33 additions & 7 deletions pkg/http/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,9 @@ 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"),
// Feature-flagged tools for testing per-request feature selection.
mockToolWithFeatureFlag("needs_holdback", "repos", true, github.FeatureFlagIssueDependencies, ""),
mockToolWithFeatureFlag("hidden_by_holdback", "repos", true, "", github.FeatureFlagIssueDependencies),
}
}

Expand Down Expand Up @@ -293,7 +293,7 @@ func TestHTTPHandlerRoutes(t *testing.T) {
name: "X-MCP-Features header enables flagged tool",
path: "/",
headers: map[string]string{
headers.MCPFeaturesHeader: "mcp_holdback_consolidated_projects",
headers.MCPFeaturesHeader: github.FeatureFlagIssueDependencies,
},
expectedTools: []string{"get_file_contents", "create_repository", "list_issues", "create_issue", "list_pull_requests", "create_pull_request", "needs_holdback"},
},
Expand All @@ -305,6 +305,29 @@ func TestHTTPHandlerRoutes(t *testing.T) {
},
expectedTools: []string{"get_file_contents", "create_repository", "list_issues", "create_issue", "list_pull_requests", "create_pull_request", "hidden_by_holdback"},
},
{
name: "features query parameter enables allowlisted feature",
path: "/?features=" + github.FeatureFlagIssueDependencies,
expectedTools: []string{"get_file_contents", "create_repository", "list_issues", "create_issue", "list_pull_requests", "create_pull_request", "needs_holdback"},
},
{
name: "features query parameter works with toolset and readonly routes",
path: "/x/repos/readonly?features=" + github.FeatureFlagIssueDependencies,
expectedTools: []string{"get_file_contents", "needs_holdback"},
},
{
name: "unknown feature in query parameter is ignored",
path: "/?features=unknown_flag",
expectedTools: []string{"get_file_contents", "create_repository", "list_issues", "create_issue", "list_pull_requests", "create_pull_request", "hidden_by_holdback"},
},
{
name: "unknown header suppresses allowlisted query feature",
path: "/?features=" + github.FeatureFlagIssueDependencies,
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"},
},
{
name: "X-MCP-Exclude-Tools header removes specific tools",
path: "/",
Expand Down Expand Up @@ -346,10 +369,13 @@ func TestHTTPHandlerRoutes(t *testing.T) {
var capturedInventory *inventory.Inventory
var capturedCtx context.Context

// Create feature checker that reads from context without whitelist validation
// (the whitelist is tested separately; here we test the filtering logic)
// Match the production allowlist and insiders expansion behavior.
featureChecker := func(ctx context.Context, flag string) (bool, error) {
return slices.Contains(ghcontext.GetHeaderFeatures(ctx), flag), nil
effective := github.ResolveFeatureFlags(
ghcontext.GetHeaderFeatures(ctx),
ghcontext.IsInsidersMode(ctx),
)
return effective[flag], nil
}

apiHost, err := utils.NewAPIHost("https://api.github.com")
Expand Down
15 changes: 12 additions & 3 deletions pkg/http/middleware/request_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@ import (
"github.com/github/github-mcp-server/pkg/http/headers"
)

const queryParamFeatures = "features"

// 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) {
// Header-selected features can change the response for the same URL.
w.Header().Add(headers.VaryHeader, headers.MCPFeaturesHeader)

ctx := r.Context()

// Readonly mode
Expand Down Expand Up @@ -45,9 +50,13 @@ func WithRequestConfig(next http.Handler) http.Handler {
ctx = ghcontext.WithInsidersMode(ctx, true)
}

// Feature flags
if features := headers.ParseCommaSeparated(r.Header.Get(headers.MCPFeaturesHeader)); len(features) > 0 {
ctx = ghcontext.WithHeaderFeatures(ctx, features)
query := r.URL.Query()
_, hasHeaderFeatures := r.Header[http.CanonicalHeaderKey(headers.MCPFeaturesHeader)]
_, hasQueryFeatures := query[queryParamFeatures]
if hasHeaderFeatures {
ctx = ghcontext.WithHeaderFeatures(ctx, headers.ParseCommaSeparated(r.Header.Get(headers.MCPFeaturesHeader)))
} else if hasQueryFeatures {
ctx = ghcontext.WithHeaderFeatures(ctx, headers.ParseCommaSeparated(query.Get(queryParamFeatures)))
}

next.ServeHTTP(w, r.WithContext(ctx))
Expand Down
112 changes: 112 additions & 0 deletions pkg/http/middleware/request_config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package middleware

import (
"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/stretchr/testify/assert"
)

func TestWithRequestConfigFeatureSelection(t *testing.T) {
tests := []struct {
name string
url string
headerSet bool
headerValue string
wantFeatures []string
wantPresent bool
}{
{
name: "query parameter only",
url: "/?features=mcp_holdback_consolidated_projects",
wantFeatures: []string{"mcp_holdback_consolidated_projects"},
wantPresent: true,
},
{
name: "header only",
url: "/",
headerSet: true,
headerValue: "mcp_holdback_consolidated_projects",
wantFeatures: []string{"mcp_holdback_consolidated_projects"},
wantPresent: true,
},
{
name: "header wins over query parameter, never combined",
url: "/?features=flag_from_query",
headerSet: true,
headerValue: "flag_from_header",
wantFeatures: []string{"flag_from_header"},
wantPresent: true,
},
{
name: "empty header suppresses query parameter",
url: "/?features=flag_from_query",
headerSet: true,
wantFeatures: []string{},
wantPresent: true,
},
{
name: "whitespace-only header suppresses query parameter",
url: "/?features=flag_from_query",
headerSet: true,
headerValue: " , \t ",
wantFeatures: []string{},
wantPresent: true,
},
{
name: "unknown header suppresses query parameter",
url: "/?features=flag_from_query",
headerSet: true,
headerValue: "unknown_from_header",
wantFeatures: []string{"unknown_from_header"},
wantPresent: true,
},
{
name: "empty query value with header",
url: "/?features=",
headerSet: true,
headerValue: "flag_from_header",
wantFeatures: []string{"flag_from_header"},
wantPresent: true,
},
{
name: "empty query value stores an explicit empty selection",
url: "/?features=",
wantFeatures: []string{},
wantPresent: true,
},
{
name: "no channel present stores nothing",
url: "/",
wantFeatures: nil,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
var got []string
handler := WithRequestConfig(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
got = ghcontext.GetHeaderFeatures(r.Context())
w.WriteHeader(http.StatusNoContent)
}))

req := httptest.NewRequest(http.MethodPost, tc.url, nil)
if tc.headerSet {
req.Header.Set(headers.MCPFeaturesHeader, tc.headerValue)
}
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)

assert.Equal(t, tc.wantFeatures, got)
if tc.wantPresent {
assert.NotNil(t, got)
} else {
assert.Nil(t, got)
}
assert.Contains(t, rec.Header().Values(headers.VaryHeader), headers.MCPFeaturesHeader)
})
}
}
18 changes: 15 additions & 3 deletions pkg/http/oauth/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,16 @@ func (h *AuthHandler) buildResourceURL(r *http.Request, resourcePath string) str
if !strings.HasPrefix(resourcePath, "/") {
resourcePath = "/" + resourcePath
}
return baseURL + resourcePath
return appendRawQuery(baseURL+resourcePath, r.URL.RawQuery)
}

// appendRawQuery avoids re-encoding the resource identifier that RFC 9728
// clients compare as an exact string.
func appendRawQuery(target, rawQuery string) string {
if rawQuery == "" {
return target
}
return target + "?" + rawQuery
}

// GetEffectiveHostAndScheme returns the effective host and scheme for a request.
Expand Down Expand Up @@ -248,10 +257,13 @@ func BuildResourceMetadataURL(r *http.Request, cfg *Config, resourcePath string)
suffix = resourcePath
}
}
metadataURL := ""
if cfg != nil && cfg.BaseURL != "" {
return strings.TrimSuffix(cfg.BaseURL, "/") + OAuthProtectedResourcePrefix + suffix
metadataURL = strings.TrimSuffix(cfg.BaseURL, "/") + OAuthProtectedResourcePrefix + suffix
} else {
metadataURL = fmt.Sprintf("%s://%s%s%s", scheme, host, OAuthProtectedResourcePrefix, suffix)
}
return fmt.Sprintf("%s://%s%s%s", scheme, host, OAuthProtectedResourcePrefix, suffix)
return appendRawQuery(metadataURL, r.URL.RawQuery)
}

func normalizeBasePath(path string) string {
Expand Down
Loading
Loading