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
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1619,6 +1619,8 @@ docker run -i --rm \
ghcr.io/github/github-mcp-server
```

In HTTP mode, this flag (or `GITHUB_LOCKDOWN_MODE`) is an upper bound: the `X-MCP-Lockdown` request header can enable lockdown mode when the operator has not, but it cannot disable lockdown mode the operator has already enabled. See the [Server Configuration Guide](docs/server-configuration.md#lockdown-mode) for details.

The behavior of lockdown mode depends on the tool invoked.

Following tools will return an error when the author lacks the push access:
Expand Down
3 changes: 2 additions & 1 deletion docs/remote-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,10 @@ The Remote GitHub MCP server has optional headers equivalent to the Local server
- `X-MCP-Readonly`: Enables only "read" tools.
- Equivalent to `GITHUB_READ_ONLY` env var for Local server.
- If this header is empty, "false", "f", "no", "n", "0", or "off" (ignoring whitespace and case), it will be interpreted as false. All other values are interpreted as true.
- `X-MCP-Lockdown`: Enables lockdown mode, hiding public issue details created by users without push access.
- `X-MCP-Lockdown`: Enables lockdown mode, hiding public issue details created by users without push access. Lockdown mode is a best-effort content filter, not a security boundary.
- Equivalent to `GITHUB_LOCKDOWN_MODE` env var for Local server.
- If this header is empty, "false", "f", "no", "n", "0", or "off" (ignoring whitespace and case), it will be interpreted as false. All other values are interpreted as true.
- Server-side lockdown configuration is an upper bound: if the operator has already enabled lockdown mode, this header cannot disable it for a request. The header can only enable (or redundantly re-enable) lockdown mode; it cannot relax lockdown mode below the operator's configuration.
- `X-MCP-Insiders`: Enables insiders mode for early access to new features.
- Equivalent to `GITHUB_INSIDERS` env var or `--insiders` flag for Local server.
- If this header is empty, "false", "f", "no", "n", "0", or "off" (ignoring whitespace and case), it will be interpreted as false. All other values are interpreted as true.
Expand Down
4 changes: 4 additions & 0 deletions docs/server-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ Note: **read-only** mode acts as a strict security filter that takes precedence

Note: **excluded tools** takes precedence over toolsets and individual tools — listed tools are always excluded, even if their toolset is enabled or they are explicitly added via `--tools` / `X-MCP-Tools`.

Note: server-side **lockdown mode** (`--lockdown-mode` / `GITHUB_LOCKDOWN_MODE`) is an upper bound in HTTP mode — once an operator enables it, the `X-MCP-Lockdown` header can no longer disable it for a given request. A request may still use the header to enable lockdown mode for itself when the operator has not already enabled it server-wide, but it can never relax lockdown mode below what the operator configured. Lockdown mode remains a best-effort content filter, not a security boundary.

---

## Configuration Examples
Expand Down Expand Up @@ -292,6 +294,8 @@ When active, this mode will disable all tools that are not read-only even if the

Lockdown mode ensures the server only surfaces content in public repositories from users with push access to that repository. Private repositories are unaffected, and collaborators retain full access to their own content.

> In HTTP mode, server-side lockdown mode (`--lockdown-mode` / `GITHUB_LOCKDOWN_MODE`) is an upper bound: the `X-MCP-Lockdown` header can enable lockdown mode for a request when the operator has not enabled it server-wide, but it cannot disable lockdown mode the operator has already enabled.

Lockdown mode is a best-effort content filter meant to reduce prompt-injection risk from untrusted repository content; it is not an authorization boundary. It does not restrict what the underlying credential can otherwise read or write, and content withheld from a filtered tool response may still be reachable through other tools or direct GitHub API access with the same credential.

As an intentional exception, content authored by trusted bot accounts (currently `github-actions[bot]` and `copilot`) is always treated as safe, regardless of push access, so routine automation output isn't filtered.
Expand Down
12 changes: 10 additions & 2 deletions pkg/github/dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,9 +439,17 @@ func (d *RequestDeps) GetRawClient(ctx context.Context) (*raw.Client, error) {
return rawClient, nil
}

// effectiveLockdownMode reports whether lockdown mode is active for the
// request. d.lockdownMode is an operator-set upper bound: the per-request
// X-MCP-Lockdown header (ghcontext.IsLockdownMode) can only enable lockdown,
// never disable one the operator already turned on.
func (d *RequestDeps) effectiveLockdownMode(ctx context.Context) bool {
return d.lockdownMode || ghcontext.IsLockdownMode(ctx)
}

// GetRepoAccessCache implements ToolDependencies.
func (d *RequestDeps) GetRepoAccessCache(ctx context.Context) (*lockdown.RepoAccessCache, error) {
if !d.lockdownMode {
if !d.effectiveLockdownMode(ctx) {
return nil, nil
}

Expand All @@ -466,7 +474,7 @@ func (d *RequestDeps) GetT() translations.TranslationHelperFunc { return d.T }
// GetFlags implements ToolDependencies.
func (d *RequestDeps) GetFlags(ctx context.Context) FeatureFlags {
return FeatureFlags{
LockdownMode: d.lockdownMode && ghcontext.IsLockdownMode(ctx),
LockdownMode: d.effectiveLockdownMode(ctx),
}
}

Expand Down
110 changes: 110 additions & 0 deletions pkg/github/dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,116 @@ func TestIsFeatureEnabled_EmptyFlagName(t *testing.T) {
assert.False(t, result, "Expected false for empty flag name")
}

// TestRequestDepsLockdownModeIsUpperBound verifies the X-MCP-Lockdown header
// can only enable lockdown, never disable the operator's server-side setting.
func TestRequestDepsLockdownModeIsUpperBound(t *testing.T) {
t.Parallel()

resolver := newRequestDepsAPIHostResolver(t, "https://example.com")

newDeps := func(serverLockdown bool) *github.RequestDeps {
return github.NewRequestDeps(
resolver,
"test",
serverLockdown,
nil,
translations.NullTranslationHelper,
0,
nil,
testExporters(),
)
}

tokenCtx := func(requestLockdown bool) context.Context {
ctx := ghcontext.WithTokenInfo(context.Background(), &ghcontext.TokenInfo{Token: "request-token"})
if requestLockdown {
ctx = ghcontext.WithLockdownMode(ctx, true)
}
return ctx
}

tests := []struct {
name string
serverLockdown bool
requestLockdown bool
wantLockdownMode bool
}{
{
name: "neither server nor request enable lockdown",
serverLockdown: false,
requestLockdown: false,
wantLockdownMode: false,
},
{
name: "server-only lockdown is enforced without a request header",
serverLockdown: true,
requestLockdown: false,
wantLockdownMode: true,
},
{
name: "request-only lockdown can enable it when the server has not",
serverLockdown: false,
requestLockdown: true,
wantLockdownMode: true,
},
{
name: "server and request both enabling lockdown stays enabled",
serverLockdown: true,
requestLockdown: true,
wantLockdownMode: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

deps := newDeps(tt.serverLockdown)
ctx := tokenCtx(tt.requestLockdown)

flags := deps.GetFlags(ctx)
assert.Equal(t, tt.wantLockdownMode, flags.LockdownMode, "GetFlags().LockdownMode")

cache, err := deps.GetRepoAccessCache(ctx)
require.NoError(t, err)
if tt.wantLockdownMode {
assert.NotNil(t, cache, "expected a repo access cache to be built when lockdown mode is effectively enabled")
} else {
assert.Nil(t, cache, "expected no repo access cache when lockdown mode is effectively disabled")
}
})
}
}

// TestRequestDepsLockdownModeCannotBeDisabledByOmittingHeader is a regression
// test for #3104: omitting the X-MCP-Lockdown header must not disable
// server-enabled lockdown mode.
func TestRequestDepsLockdownModeCannotBeDisabledByOmittingHeader(t *testing.T) {
t.Parallel()

resolver := newRequestDepsAPIHostResolver(t, "https://example.com")
deps := github.NewRequestDeps(
resolver,
"test",
true, // server-enabled lockdown
nil,
translations.NullTranslationHelper,
0,
nil,
testExporters(),
)

// No X-MCP-Lockdown header sent.
ctx := ghcontext.WithTokenInfo(context.Background(), &ghcontext.TokenInfo{Token: "request-token"})

flags := deps.GetFlags(ctx)
assert.True(t, flags.LockdownMode, "server-enabled lockdown mode must remain enabled when a request omits the lockdown header")

cache, err := deps.GetRepoAccessCache(ctx)
require.NoError(t, err)
assert.NotNil(t, cache, "repo access cache must still be built so server-enabled lockdown mode can be enforced")
}

func TestIsFeatureEnabled_CheckerError(t *testing.T) {
t.Parallel()

Expand Down
10 changes: 5 additions & 5 deletions pkg/http/transport/bearer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestBearerAuthTransport(t *testing.T) {
defer server.Close()

rt := &BearerAuthTransport{
Transport: http.DefaultTransport,
Transport: newIsolatedTransport(t),
Token: tc.token,
TokenProvider: tc.tokenProvider,
}
Expand Down Expand Up @@ -91,7 +91,7 @@ func TestBearerAuthTransport_TokenProviderResolvedPerRequest(t *testing.T) {

current := ""
rt := &BearerAuthTransport{
Transport: http.DefaultTransport,
Transport: newIsolatedTransport(t),
TokenProvider: func() string { return current },
}

Expand Down Expand Up @@ -126,7 +126,7 @@ func TestBearerAuthTransport_PassesGraphQLFeaturesHeader(t *testing.T) {
defer server.Close()

rt := &BearerAuthTransport{
Transport: http.DefaultTransport,
Transport: newIsolatedTransport(t),
Token: "token",
}

Expand All @@ -150,7 +150,7 @@ func TestBearerAuthTransport_DoesNotMutateOriginalRequest(t *testing.T) {
defer server.Close()

rt := &BearerAuthTransport{
Transport: http.DefaultTransport,
Transport: newIsolatedTransport(t),
Token: "token",
}

Expand Down Expand Up @@ -356,7 +356,7 @@ func TestBearerAuthTransport_RedirectHostScoping(t *testing.T) {
require.NoError(t, err)

client := &http.Client{Transport: &BearerAuthTransport{
Transport: http.DefaultTransport,
Transport: newIsolatedTransport(t),
Token: "secret-token",
AllowedHosts: []string{sourceURL.Host, allowedTargetURL.Host},
}}
Expand Down
9 changes: 5 additions & 4 deletions pkg/http/transport/graphql_features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestGraphQLFeaturesTransport(t *testing.T) {

// Create the transport
transport := &GraphQLFeaturesTransport{
Transport: http.DefaultTransport,
Transport: newIsolatedTransport(t),
}

// Create a request
Expand All @@ -91,9 +91,10 @@ func TestGraphQLFeaturesTransport(t *testing.T) {
}
}

// TestGraphQLFeaturesTransport_NilTransport exercises the real
// http.DefaultTransport fallback, so it can't run in parallel with tests that
// close their own servers (that closes DefaultTransport's idle conns too).
func TestGraphQLFeaturesTransport_NilTransport(t *testing.T) {
t.Parallel()

var capturedHeader string

// Create a test server
Expand Down Expand Up @@ -133,7 +134,7 @@ func TestGraphQLFeaturesTransport_DoesNotMutateOriginalRequest(t *testing.T) {

// Create the transport
transport := &GraphQLFeaturesTransport{
Transport: http.DefaultTransport,
Transport: newIsolatedTransport(t),
}

// Create a request with features
Expand Down
20 changes: 20 additions & 0 deletions pkg/http/transport/helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package transport

import (
"net/http"
"testing"
)

// newIsolatedTransport returns an http.Transport owned by a single test.
//
// Sharing http.DefaultTransport across parallel tests is unsafe: closing one
// test's httptest.Server also closes DefaultTransport's idle connections,
// breaking other tests still using it. Tests asserting DefaultTransport
// fallback behavior specifically must use it directly and not run in parallel.
func newIsolatedTransport(t *testing.T) *http.Transport {
t.Helper()

transport := &http.Transport{}
t.Cleanup(transport.CloseIdleConnections)
return transport
}
Loading