From 03bf95088902c28d335a14d949bd9458ab9dbd0e Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 26 May 2026 14:07:45 +0200 Subject: [PATCH] feat(http): ignore proxy forwarding headers by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Forwarded-Host and X-Forwarded-Proto were unconditionally honored when constructing OAuth resource metadata URLs. In HTTP-mode deployments that do not set --base-url and are not fronted by a proxy that strips these headers, this lets an on-path client influence the URL advertised in WWW-Authenticate and the /.well-known/oauth-protected-resource body. This is a hardening change rather than a true vulnerability — exploiting it requires HTTP without --base-url plus an attacker already positioned to inject the header — but the unsafe default is worth closing. Default behavior now derives host/scheme from r.Host and the TLS state. Setups that rely on a trusted internal forwarder (e.g. an in-cluster gateway that needs to preserve the originating hostname per request) can opt back in with --trust-proxy-headers / GITHUB_TRUST_PROXY_HEADERS=1. --base-url continues to take precedence in all cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cmd/github-mcp-server/main.go | 3 +++ docs/streamable-http.md | 12 +++++++++++ pkg/http/oauth/oauth.go | 34 +++++++++++++++++++++++++------ pkg/http/oauth/oauth_test.go | 38 +++++++++++++++++++++++++++++------ pkg/http/server.go | 12 +++++++++-- 5 files changed, 85 insertions(+), 14 deletions(-) diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index ab8b27bb3c..558fdb9980 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -153,6 +153,7 @@ var ( ExcludeTools: excludeTools, EnabledFeatures: enabledFeatures, InsidersMode: viper.GetBool("insiders"), + TrustProxyHeaders: viper.GetBool("trust-proxy-headers"), } return ghhttp.RunHTTPServer(httpConfig) @@ -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")) @@ -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) diff --git a/docs/streamable-http.md b/docs/streamable-http.md index 0a11c5ea76..8f4a2bff84 100644 --- a/docs/streamable-http.md +++ b/docs/streamable-http.md @@ -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 diff --git a/pkg/http/oauth/oauth.go b/pkg/http/oauth/oauth.go index 3b4d41959f..ffa7669a9d 100644 --- a/pkg/http/oauth/oauth.go +++ b/pkg/http/oauth/oauth.go @@ -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. @@ -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) + } + } + if scheme == "" { if r.TLS != nil { scheme = "https" } else { diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index 6d76b579f3..f39ef39b87 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -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 { @@ -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", }, @@ -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", }, @@ -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", }, @@ -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", }, @@ -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", }, @@ -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" diff --git a/pkg/http/server.go b/pkg/http/server.go index 6fd19a8b9b..3c9d7679e4 100644 --- a/pkg/http/server.go +++ b/pkg/http/server.go @@ -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 @@ -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{}