Skip to content

fix(mcp): support X-Forwarded-Proto in SSE endpoint scheme#113

Open
sjhddh wants to merge 1 commit into
Ed1s0nZ:mainfrom
sjhddh:fix/sse-scheme-proxy
Open

fix(mcp): support X-Forwarded-Proto in SSE endpoint scheme#113
sjhddh wants to merge 1 commit into
Ed1s0nZ:mainfrom
sjhddh:fix/sse-scheme-proxy

Conversation

@sjhddh

@sjhddh sjhddh commented May 5, 2026

Copy link
Copy Markdown

When the MCP SSE server runs behind a TLS-terminating reverse proxy (nginx, Caddy, ALB), r.TLS is nil so the advertised endpoint URL ends up as http://..., breaking clients that connected over https://.

This adds an X-Forwarded-Proto fallback before defaulting to http, matching standard reverse-proxy conventions. r.URL.Scheme (when explicitly set) still wins.

scheme := "http"
if r.TLS != nil {
    scheme = "https"
} else if proto := r.Header.Get("X-Forwarded-Proto"); proto != "" {
    scheme = proto
}
if r.URL.Scheme != "" {
    scheme = r.URL.Scheme
}

Replaces #65 (closed; that PR could not be reopened because the branch was force-pushed to fix the commit author identity).

@mxnstrexgl mxnstrexgl left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Hermes Agent Auto-Review — PR #113

SSE endpoint X-Forwarded-Proto fix

⚠️ Warning

  • X-Forwarded-Proto is trust-dependent: This header can be spoofed by clients if the server is directly exposed to the internet without a trusted reverse proxy stripping/re-setting the header. An attacker can set X-Forwarded-Proto: https to trick the SSE endpoint into generating https:// scheme URLs even when behind HTTP. Ensure this code path is only reachable behind a trusted proxy that sets this header.

✅ Looks Good

  • Minimal, targeted fix — correctly falls through to the existing r.URL.Scheme check.
  • TLS check takes priority — r.TLS != nil still wins, which is correct.
  • Fixes real issue — SSE URLs would be wrong behind a TLS-terminating proxy without this.

Reviewed by Hermes Agent

@mxnstrexgl mxnstrexgl left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Hermes Agent Security Review

Verdict: Changes Requested — 1 warning-level issue.

⚠️ Warning

X-Forwarded-Proto header trusted without validation — The value is used directly as scheme without checking it equals "http" or "https". Any client (not just a trusted reverse proxy) can set this to an arbitrary string like javascript:// (potential reflected XSS in URL rendering) or //evil.com (potential redirect hijacking).

Fix (2 lines):

} else if proto := r.Header.Get("X-Forwarded-Proto"); proto == "http" || proto == "https" {
    scheme = proto
}

✅ Clean

  • No SSRF risk (scheme used only for response URLs, not outbound requests)

Reviewed by Hermes Agent (cron auto-review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants