Skip to content

Commit b473a5a

Browse files
feat(http): ignore proxy forwarding headers by default
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>
1 parent 57f4df4 commit b473a5a

5 files changed

Lines changed: 85 additions & 14 deletions

File tree

cmd/github-mcp-server/main.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ var (
153153
ExcludeTools: excludeTools,
154154
EnabledFeatures: enabledFeatures,
155155
InsidersMode: viper.GetBool("insiders"),
156+
TrustProxyHeaders: viper.GetBool("trust-proxy-headers"),
156157
}
157158

158159
return ghhttp.RunHTTPServer(httpConfig)
@@ -186,6 +187,7 @@ func init() {
186187
httpCmd.Flags().String("base-url", "", "Base URL where this server is publicly accessible (for OAuth resource metadata)")
187188
httpCmd.Flags().String("base-path", "", "Externally visible base path for the HTTP server (for OAuth resource metadata)")
188189
httpCmd.Flags().Bool("scope-challenge", false, "Enable OAuth scope challenge responses")
190+
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.")
189191

190192
// Bind flag to viper
191193
_ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets"))
@@ -205,6 +207,7 @@ func init() {
205207
_ = viper.BindPFlag("base-url", httpCmd.Flags().Lookup("base-url"))
206208
_ = viper.BindPFlag("base-path", httpCmd.Flags().Lookup("base-path"))
207209
_ = viper.BindPFlag("scope-challenge", httpCmd.Flags().Lookup("scope-challenge"))
210+
_ = viper.BindPFlag("trust-proxy-headers", httpCmd.Flags().Lookup("trust-proxy-headers"))
208211
// Add subcommands
209212
rootCmd.AddCommand(stdioCmd)
210213
rootCmd.AddCommand(httpCmd)

docs/streamable-http.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,18 @@ The OAuth protected resource metadata's `resource` attribute will be populated w
5959

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

62+
### Behind a Trusted Proxy (advanced)
63+
64+
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.
65+
66+
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:
67+
68+
```bash
69+
github-mcp-server http --trust-proxy-headers
70+
```
71+
72+
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.
73+
6274
## Client Configuration
6375

6476
### Using OAuth Authentication

pkg/http/oauth/oauth.go

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,15 @@ type Config struct {
4949
// This is used to restore the original path when a proxy strips a base path before forwarding.
5050
// If empty, requests are treated as already using the external path.
5151
ResourcePath string
52+
53+
// TrustProxyHeaders indicates whether X-Forwarded-Host and X-Forwarded-Proto
54+
// should be honored when deriving the effective host and scheme for OAuth
55+
// resource URLs. This must only be enabled when the server is deployed
56+
// behind a trusted proxy that sets these headers; otherwise an untrusted
57+
// client can influence the OAuth resource metadata URL advertised to MCP
58+
// clients. When BaseURL is set, it always takes precedence and these
59+
// headers are unused.
60+
TrustProxyHeaders bool
5261
}
5362

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

198207
// GetEffectiveHostAndScheme returns the effective host and scheme for a request.
208+
//
209+
// X-Forwarded-Host and X-Forwarded-Proto are only honored when cfg.TrustProxyHeaders
210+
// is true. Without that opt-in, an untrusted client could otherwise influence the
211+
// OAuth resource metadata URL advertised to MCP clients.
199212
func GetEffectiveHostAndScheme(r *http.Request, cfg *Config) (host, scheme string) { //nolint:revive
200-
if fh := r.Header.Get(headers.ForwardedHostHeader); fh != "" {
201-
host = fh
202-
} else {
213+
trustProxy := cfg != nil && cfg.TrustProxyHeaders
214+
215+
if trustProxy {
216+
if fh := r.Header.Get(headers.ForwardedHostHeader); fh != "" {
217+
host = fh
218+
}
219+
}
220+
if host == "" {
203221
host = r.Host
204222
}
205223
if host == "" {
206224
host = "localhost"
207225
}
208-
if fp := r.Header.Get(headers.ForwardedProtoHeader); fp != "" {
209-
scheme = strings.ToLower(fp)
210-
} else {
226+
227+
if trustProxy {
228+
if fp := r.Header.Get(headers.ForwardedProtoHeader); fp != "" {
229+
scheme = strings.ToLower(fp)
230+
}
231+
}
232+
if scheme == "" {
211233
if r.TLS != nil {
212234
scheme = "https"
213235
} else {

pkg/http/oauth/oauth_test.go

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,19 @@ func TestGetEffectiveHostAndScheme(t *testing.T) {
8484
expectedHost: "example.com",
8585
expectedScheme: "http", // defaults to http
8686
},
87+
{
88+
name: "X-Forwarded-Host ignored by default",
89+
setupRequest: func() *http.Request {
90+
req := httptest.NewRequest(http.MethodGet, "/test", nil)
91+
req.Host = "internal.example.com"
92+
req.Header.Set(headers.ForwardedHostHeader, "attacker.example.com")
93+
req.Header.Set(headers.ForwardedProtoHeader, "https")
94+
return req
95+
},
96+
cfg: &Config{},
97+
expectedHost: "internal.example.com",
98+
expectedScheme: "http",
99+
},
87100
{
88101
name: "request with X-Forwarded-Host header",
89102
setupRequest: func() *http.Request {
@@ -92,7 +105,7 @@ func TestGetEffectiveHostAndScheme(t *testing.T) {
92105
req.Header.Set(headers.ForwardedHostHeader, "public.example.com")
93106
return req
94107
},
95-
cfg: &Config{},
108+
cfg: &Config{TrustProxyHeaders: true},
96109
expectedHost: "public.example.com",
97110
expectedScheme: "http",
98111
},
@@ -104,7 +117,7 @@ func TestGetEffectiveHostAndScheme(t *testing.T) {
104117
req.Header.Set(headers.ForwardedProtoHeader, "http")
105118
return req
106119
},
107-
cfg: &Config{},
120+
cfg: &Config{TrustProxyHeaders: true},
108121
expectedHost: "example.com",
109122
expectedScheme: "http",
110123
},
@@ -117,7 +130,7 @@ func TestGetEffectiveHostAndScheme(t *testing.T) {
117130
req.Header.Set(headers.ForwardedProtoHeader, "https")
118131
return req
119132
},
120-
cfg: &Config{},
133+
cfg: &Config{TrustProxyHeaders: true},
121134
expectedHost: "public.example.com",
122135
expectedScheme: "https",
123136
},
@@ -142,7 +155,7 @@ func TestGetEffectiveHostAndScheme(t *testing.T) {
142155
req.Header.Set(headers.ForwardedProtoHeader, "http")
143156
return req
144157
},
145-
cfg: &Config{},
158+
cfg: &Config{TrustProxyHeaders: true},
146159
expectedHost: "example.com",
147160
expectedScheme: "http",
148161
},
@@ -154,7 +167,7 @@ func TestGetEffectiveHostAndScheme(t *testing.T) {
154167
req.Header.Set(headers.ForwardedProtoHeader, "HTTPS")
155168
return req
156169
},
157-
cfg: &Config{},
170+
cfg: &Config{TrustProxyHeaders: true},
158171
expectedHost: "example.com",
159172
expectedScheme: "https",
160173
},
@@ -301,8 +314,21 @@ func TestBuildResourceMetadataURL(t *testing.T) {
301314
expectedURL: "https://custom.example.com/.well-known/oauth-protected-resource/mcp",
302315
},
303316
{
304-
name: "with forwarded headers",
317+
name: "with forwarded headers ignored by default",
305318
cfg: &Config{},
319+
setupRequest: func() *http.Request {
320+
req := httptest.NewRequest(http.MethodGet, "/mcp", nil)
321+
req.Host = "internal.example.com"
322+
req.Header.Set(headers.ForwardedHostHeader, "attacker.example.com")
323+
req.Header.Set(headers.ForwardedProtoHeader, "https")
324+
return req
325+
},
326+
resourcePath: "/mcp",
327+
expectedURL: "http://internal.example.com/.well-known/oauth-protected-resource/mcp",
328+
},
329+
{
330+
name: "with forwarded headers",
331+
cfg: &Config{TrustProxyHeaders: true},
306332
setupRequest: func() *http.Request {
307333
req := httptest.NewRequest(http.MethodGet, "/mcp", nil)
308334
req.Host = "internal.example.com"

pkg/http/server.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ type ServerConfig struct {
4343
// This is used to restore the original path when a proxy strips a base path before forwarding.
4444
ResourcePath string
4545

46+
// TrustProxyHeaders indicates whether X-Forwarded-Host and X-Forwarded-Proto
47+
// should be honored when constructing OAuth resource metadata URLs. Only
48+
// enable this when the server is deployed behind a trusted proxy that sets
49+
// these headers. When BaseURL is set, it always wins and this setting has
50+
// no effect.
51+
TrustProxyHeaders bool
52+
4653
// ExportTranslations indicates if we should export translations
4754
// See: https://github.com/github/github-mcp-server?tab=readme-ov-file#i18n--overriding-descriptions
4855
ExportTranslations bool
@@ -150,8 +157,9 @@ func RunHTTPServer(cfg ServerConfig) error {
150157

151158
// Register OAuth protected resource metadata endpoints
152159
oauthCfg := &oauth.Config{
153-
BaseURL: cfg.BaseURL,
154-
ResourcePath: cfg.ResourcePath,
160+
BaseURL: cfg.BaseURL,
161+
ResourcePath: cfg.ResourcePath,
162+
TrustProxyHeaders: cfg.TrustProxyHeaders,
155163
}
156164

157165
serverOptions := []HandlerOption{}

0 commit comments

Comments
 (0)