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
3 changes: 3 additions & 0 deletions cmd/github-mcp-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ var (
ExcludeTools: excludeTools,
EnabledFeatures: enabledFeatures,
InsidersMode: viper.GetBool("insiders"),
TrustProxyHeaders: viper.GetBool("trust-proxy-headers"),
}

return ghhttp.RunHTTPServer(httpConfig)
Expand Down Expand Up @@ -186,6 +187,7 @@ func init() {
httpCmd.Flags().String("base-url", "", "Base URL where this server is publicly accessible (for OAuth resource metadata)")
httpCmd.Flags().String("base-path", "", "Externally visible base path for the HTTP server (for OAuth resource metadata)")
httpCmd.Flags().Bool("scope-challenge", false, "Enable OAuth scope challenge responses")
httpCmd.Flags().Bool("trust-proxy-headers", false, "Honor X-Forwarded-Host and X-Forwarded-Proto when constructing OAuth resource metadata URLs. Only enable when the server is deployed behind a trusted proxy that sets these headers. Ignored when --base-url is set.")

// Bind flag to viper
_ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets"))
Expand All @@ -205,6 +207,7 @@ func init() {
_ = viper.BindPFlag("base-url", httpCmd.Flags().Lookup("base-url"))
_ = viper.BindPFlag("base-path", httpCmd.Flags().Lookup("base-path"))
_ = viper.BindPFlag("scope-challenge", httpCmd.Flags().Lookup("scope-challenge"))
_ = viper.BindPFlag("trust-proxy-headers", httpCmd.Flags().Lookup("trust-proxy-headers"))
// Add subcommands
rootCmd.AddCommand(stdioCmd)
rootCmd.AddCommand(httpCmd)
Expand Down
12 changes: 12 additions & 0 deletions docs/streamable-http.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,18 @@ The OAuth protected resource metadata's `resource` attribute will be populated w

This allows OAuth clients to discover authentication requirements and endpoint information automatically.

### Behind a Trusted Proxy (advanced)

By default, the server ignores the `X-Forwarded-Host` and `X-Forwarded-Proto` headers when constructing OAuth resource metadata URLs, so an untrusted client cannot influence the URL advertised to MCP clients. For most deployments, setting `--base-url` to the externally visible URL is the right approach.

If the server sits behind an internal forwarder that you fully control (for example, an in-cluster gateway that needs to preserve the originating hostname per request), you can opt into honoring those headers:

```bash
github-mcp-server http --trust-proxy-headers
```

Equivalent environment variable: `GITHUB_TRUST_PROXY_HEADERS=1`. Only enable this when the upstream proxy is trusted to set or strip these headers; otherwise prefer `--base-url`. When `--base-url` is set, it always takes precedence and `--trust-proxy-headers` has no effect.

## Client Configuration

### Using OAuth Authentication
Expand Down
34 changes: 28 additions & 6 deletions pkg/http/oauth/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,15 @@ type Config struct {
// This is used to restore the original path when a proxy strips a base path before forwarding.
// If empty, requests are treated as already using the external path.
ResourcePath string

// TrustProxyHeaders indicates whether X-Forwarded-Host and X-Forwarded-Proto
// should be honored when deriving the effective host and scheme for OAuth
// resource URLs. This must only be enabled when the server is deployed
// behind a trusted proxy that sets these headers; otherwise an untrusted
// client can influence the OAuth resource metadata URL advertised to MCP
// clients. When BaseURL is set, it always takes precedence and these
// headers are unused.
TrustProxyHeaders bool
}

// AuthHandler handles OAuth-related HTTP endpoints.
Expand Down Expand Up @@ -196,18 +205,31 @@ func (h *AuthHandler) buildResourceURL(r *http.Request, resourcePath string) str
}

// GetEffectiveHostAndScheme returns the effective host and scheme for a request.
//
// X-Forwarded-Host and X-Forwarded-Proto are only honored when cfg.TrustProxyHeaders
// is true. Without that opt-in, an untrusted client could otherwise influence the
// OAuth resource metadata URL advertised to MCP clients.
func GetEffectiveHostAndScheme(r *http.Request, cfg *Config) (host, scheme string) { //nolint:revive
if fh := r.Header.Get(headers.ForwardedHostHeader); fh != "" {
host = fh
} else {
trustProxy := cfg != nil && cfg.TrustProxyHeaders

if trustProxy {
if fh := r.Header.Get(headers.ForwardedHostHeader); fh != "" {
host = fh
}
}
if host == "" {
host = r.Host
}
if host == "" {
host = "localhost"
}
if fp := r.Header.Get(headers.ForwardedProtoHeader); fp != "" {
scheme = strings.ToLower(fp)
} else {

if trustProxy {
if fp := r.Header.Get(headers.ForwardedProtoHeader); fp != "" {
scheme = strings.ToLower(fp)
}
}
Comment thread
SamMorrowDrums marked this conversation as resolved.
if scheme == "" {
if r.TLS != nil {
scheme = "https"
} else {
Expand Down
38 changes: 32 additions & 6 deletions pkg/http/oauth/oauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,19 @@ func TestGetEffectiveHostAndScheme(t *testing.T) {
expectedHost: "example.com",
expectedScheme: "http", // defaults to http
},
{
name: "X-Forwarded-Host ignored by default",
setupRequest: func() *http.Request {
req := httptest.NewRequest(http.MethodGet, "/test", nil)
req.Host = "internal.example.com"
req.Header.Set(headers.ForwardedHostHeader, "attacker.example.com")
req.Header.Set(headers.ForwardedProtoHeader, "https")
return req
},
cfg: &Config{},
expectedHost: "internal.example.com",
expectedScheme: "http",
},
{
name: "request with X-Forwarded-Host header",
setupRequest: func() *http.Request {
Expand All @@ -92,7 +105,7 @@ func TestGetEffectiveHostAndScheme(t *testing.T) {
req.Header.Set(headers.ForwardedHostHeader, "public.example.com")
return req
},
cfg: &Config{},
cfg: &Config{TrustProxyHeaders: true},
expectedHost: "public.example.com",
expectedScheme: "http",
},
Expand All @@ -104,7 +117,7 @@ func TestGetEffectiveHostAndScheme(t *testing.T) {
req.Header.Set(headers.ForwardedProtoHeader, "http")
return req
},
cfg: &Config{},
cfg: &Config{TrustProxyHeaders: true},
expectedHost: "example.com",
expectedScheme: "http",
},
Expand All @@ -117,7 +130,7 @@ func TestGetEffectiveHostAndScheme(t *testing.T) {
req.Header.Set(headers.ForwardedProtoHeader, "https")
return req
},
cfg: &Config{},
cfg: &Config{TrustProxyHeaders: true},
expectedHost: "public.example.com",
expectedScheme: "https",
},
Expand All @@ -142,7 +155,7 @@ func TestGetEffectiveHostAndScheme(t *testing.T) {
req.Header.Set(headers.ForwardedProtoHeader, "http")
return req
},
cfg: &Config{},
cfg: &Config{TrustProxyHeaders: true},
expectedHost: "example.com",
expectedScheme: "http",
},
Expand All @@ -154,7 +167,7 @@ func TestGetEffectiveHostAndScheme(t *testing.T) {
req.Header.Set(headers.ForwardedProtoHeader, "HTTPS")
return req
},
cfg: &Config{},
cfg: &Config{TrustProxyHeaders: true},
expectedHost: "example.com",
expectedScheme: "https",
},
Expand Down Expand Up @@ -301,8 +314,21 @@ func TestBuildResourceMetadataURL(t *testing.T) {
expectedURL: "https://custom.example.com/.well-known/oauth-protected-resource/mcp",
},
{
name: "with forwarded headers",
name: "with forwarded headers ignored by default",
cfg: &Config{},
setupRequest: func() *http.Request {
req := httptest.NewRequest(http.MethodGet, "/mcp", nil)
req.Host = "internal.example.com"
req.Header.Set(headers.ForwardedHostHeader, "attacker.example.com")
req.Header.Set(headers.ForwardedProtoHeader, "https")
return req
},
resourcePath: "/mcp",
expectedURL: "http://internal.example.com/.well-known/oauth-protected-resource/mcp",
},
{
name: "with forwarded headers",
cfg: &Config{TrustProxyHeaders: true},
setupRequest: func() *http.Request {
req := httptest.NewRequest(http.MethodGet, "/mcp", nil)
req.Host = "internal.example.com"
Expand Down
12 changes: 10 additions & 2 deletions pkg/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ type ServerConfig struct {
// This is used to restore the original path when a proxy strips a base path before forwarding.
ResourcePath string

// TrustProxyHeaders indicates whether X-Forwarded-Host and X-Forwarded-Proto
// should be honored when constructing OAuth resource metadata URLs. Only
// enable this when the server is deployed behind a trusted proxy that sets
// these headers. When BaseURL is set, it always wins and this setting has
// no effect.
TrustProxyHeaders bool

// ExportTranslations indicates if we should export translations
// See: https://github.com/github/github-mcp-server?tab=readme-ov-file#i18n--overriding-descriptions
ExportTranslations bool
Expand Down Expand Up @@ -150,8 +157,9 @@ func RunHTTPServer(cfg ServerConfig) error {

// Register OAuth protected resource metadata endpoints
oauthCfg := &oauth.Config{
BaseURL: cfg.BaseURL,
ResourcePath: cfg.ResourcePath,
BaseURL: cfg.BaseURL,
ResourcePath: cfg.ResourcePath,
TrustProxyHeaders: cfg.TrustProxyHeaders,
}

serverOptions := []HandlerOption{}
Expand Down
Loading