fix(mcp): rate-limit follow-ups from #831 review

cmd/gosqlx-mcp/main.go

@@ -25,6 +25,9 @@
 //	GOSQLX_MCP_OPENAI_APPS_CHALLENGE  ChatGPT app domain-verification token
 //	GOSQLX_MCP_TRUSTED_PROXY_HEADER   trusted client-IP header (e.g. CF-Connecting-IP)
 //	GOSQLX_MCP_TRUSTED_PROXY_HOPS     trusted proxies appending to X-Forwarded-For
+//	GOSQLX_MCP_TRUSTED_PROXY_SECRET_HEADER  header carrying the proxy shared secret
+//	GOSQLX_MCP_TRUSTED_PROXY_SECRET   shared secret; forwarded IPs are trusted only
+//	                                  on requests that carry it
 //
 // Pass --stdio to serve MCP over stdin/stdout instead of HTTP.
 //
@@ -117,6 +120,9 @@ func runHTTP() error {
 	}()
 
 	log.Printf("gosqlx-mcp: listening on %s (auth=%v, client-ip=%s)\n", cfg.Addr(), cfg.AuthEnabled(), clientIPSource(cfg))
+	if msg := sharedClientIPWarning(cfg); msg != "" {
+		log.Print(msg)
+	}
 	if err := httpSrv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
 		return fmt.Errorf("server error: %w", err)
 	}
@@ -152,16 +158,35 @@ func withBodyDeadline(next http.Handler, timeout time.Duration) http.Handler {
 	})
 }
 
-// clientIPSource describes where rate limiting takes the client IP from.
+// clientIPSource describes where rate limiting takes the client IP from. It
+// names the secret header when one gates the forwarded address, but never
+// the secret itself.
 func clientIPSource(cfg *gosqlxmcp.Config) string {
+	var src string
 	switch {
 	case cfg.TrustedProxyHeader != "":
-		return cfg.TrustedProxyHeader
+		src = cfg.TrustedProxyHeader
 	case cfg.TrustedProxyHops > 0:
-		return fmt.Sprintf("X-Forwarded-For[-%d]", cfg.TrustedProxyHops)
+		src = fmt.Sprintf("X-Forwarded-For[-%d]", cfg.TrustedProxyHops)
 	default:
 		return "RemoteAddr"
 	}
+	if cfg.TrustedProxySecretHeader != "" {
+		src += " when " + cfg.TrustedProxySecretHeader + " matches, else RemoteAddr"
+	}
+	return src
+}
+
+// sharedClientIPWarning returns the startup warning logged when rate limiting
+// is keyed on RemoteAddr only, or "" when a trusted proxy is configured.
+func sharedClientIPWarning(cfg *gosqlxmcp.Config) string {
+	if cfg.TrustedProxyHeader != "" || cfg.TrustedProxyHops > 0 {
+		return ""
+	}
+	return "gosqlx-mcp: warning: no trusted proxy configured, so rate limits are keyed on the TCP peer address. " +
+		"If this server runs behind a reverse proxy or load balancer, every client shares the proxy's " +
+		"rate-limit bucket and per-client GET (SSE) stream cap. Set GOSQLX_MCP_TRUSTED_PROXY_HEADER " +
+		"or GOSQLX_MCP_TRUSTED_PROXY_HOPS (see docs/MCP_GUIDE.md, \"Client IP and reverse proxies\")."
 }
 
 // newMux builds the HTTP routes:

cmd/gosqlx-mcp/main_test.go

@@ -298,6 +298,8 @@ func TestClientIPSource(t *testing.T) {
 		{"default", gosqlxmcp.Config{}, "RemoteAddr"},
 		{"header", gosqlxmcp.Config{TrustedProxyHeader: "Cf-Connecting-Ip"}, "Cf-Connecting-Ip"},
 		{"hops", gosqlxmcp.Config{TrustedProxyHops: 2}, "X-Forwarded-For[-2]"},
+		{"secret-gated header", gosqlxmcp.Config{TrustedProxyHeader: "Cf-Connecting-Ip", TrustedProxySecretHeader: "X-Origin-Secret", TrustedProxySecret: "s3cret-0123456789abcdef"},
+			"Cf-Connecting-Ip when X-Origin-Secret matches, else RemoteAddr"},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -358,3 +360,43 @@ func TestWithBodyDeadline_SlowBodyOutsideMCP(t *testing.T) {
 		}
 	}
 }
+
+func TestClientIPSource_NeverLogsSecret(t *testing.T) {
+	cfg := gosqlxmcp.Config{TrustedProxyHops: 1, TrustedProxySecretHeader: "X-Origin-Secret", TrustedProxySecret: "s3cret-0123456789abcdef"}
+	if got := clientIPSource(&cfg); strings.Contains(got, cfg.TrustedProxySecret) {
+		t.Fatalf("clientIPSource leaks the secret: %q", got)
+	}
+}
+
+func TestSharedClientIPWarning(t *testing.T) {
+	tests := []struct {
+		name     string
+		cfg      gosqlxmcp.Config
+		wantWarn bool
+	}{
+		{"no trusted proxy warns", gosqlxmcp.Config{Host: "0.0.0.0", Port: 8080}, true},
+		{"loopback still warns (local reverse proxy)", gosqlxmcp.Config{Host: "127.0.0.1", Port: 8080}, true},
+		{"trusted header silences", gosqlxmcp.Config{TrustedProxyHeader: "Cf-Connecting-Ip"}, false},
+		{"trusted hops silences", gosqlxmcp.Config{TrustedProxyHops: 1}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := sharedClientIPWarning(&tt.cfg)
+			if (got != "") != tt.wantWarn {
+				t.Fatalf("sharedClientIPWarning() = %q, want warning=%v", got, tt.wantWarn)
+			}
+			if !tt.wantWarn {
+				return
+			}
+			for _, want := range []string{"warning", "share", "rate-limit bucket", "stream cap",
+				"GOSQLX_MCP_TRUSTED_PROXY_HEADER", "GOSQLX_MCP_TRUSTED_PROXY_HOPS"} {
+				if !strings.Contains(got, want) {
+					t.Errorf("warning %q does not mention %q", got, want)
+				}
+			}
+			if strings.Contains(got, "\n") {
+				t.Errorf("warning should be a single log line: %q", got)
+			}
+		})
+	}
+}

docs/CHATGPT_APP.md

@@ -185,6 +185,19 @@ HTML gains remote `src`/`href` URLs, `@import`, `fetch(`, `XMLHttpRequest` or
 | `/health` | `{"status":"ok","version":<gosqlx.Version>,"tools":<registered tool count>}` |
 | `/.well-known/openai-apps-challenge` | Plain-text domain-verification token. Served only when `GOSQLX_MCP_OPENAI_APPS_CHALLENGE` is set; returns 404 otherwise |
 
+### Rate limits are shared
+
+Rate limits are per client IP (see [MCP_GUIDE.md, Rate Limits](MCP_GUIDE.md#rate-limits)).
+ChatGPT calls the MCP server from OpenAI's servers, not from each user's browser, so
+the "client" the limiter sees is an OpenAI egress address. Many ChatGPT users can
+therefore share one budget of 10 requests/second and 120 weighted requests/minute,
+and heavy use by some can make tool calls fail for others with a rate-limit error
+(HTTP 429, or HTTP 200 with a JSON-RPC error for `initialize`) until the window
+resets. The same applies to any proxy in front of a self-hosted server that is not
+configured as trusted: without `GOSQLX_MCP_TRUSTED_PROXY_HEADER` or
+`GOSQLX_MCP_TRUSTED_PROXY_HOPS`, every client behind it shares one bucket and one
+4-stream `GET` cap, and `gosqlx-mcp` logs a warning at startup.
+
 ### Sessions
 
 The streamable HTTP transport runs in mcp-go's default stateless mode. Session IDs
@@ -197,9 +210,10 @@ invalidates an ID: a later request with the same ID simply starts new state.
 mcp-go's informational transport logs, which include session IDs, are discarded;
 only transport errors are logged, so nothing identifies a session in the logs.
 
-The HTTP server sets `ReadHeaderTimeout`. `Server.Start` (library use) also sets
-`IdleTimeout` but applies only bearer auth, not the rate limiter or the 64 KB body
-cap; `cmd/gosqlx-mcp` wraps the handler in `RateLimitMiddleware`. Call
+The HTTP server sets `ReadHeaderTimeout`. `Server.Start` and `Server.Handler`
+(library use) apply only bearer auth, not `RateLimitMiddleware`, so they have no rate
+limit, stream cap, body read deadline or 64 KB body cap; `cmd/gosqlx-mcp` wraps the
+handler in `NewRateLimitMiddleware`. Call
 `Server.Shutdown` when you discard a `Server` whose `Handler` you used, to stop the
 session sweeper goroutine.
 

docs/MCP_GUIDE.md

@@ -84,10 +84,11 @@ The public server has smart rate limiting to ensure fair usage:
 - **Sustained**: 120 weighted-requests/minute per IP
 - Heavier tools (`analyze_sql`) consume more budget than simple ones (`validate_sql`)
 - Limits tighten automatically under high server load
-- **Streams**: at most 4 concurrent `GET /mcp` (SSE) streams per IP; each costs 1 unit of sustained budget
+- **Streams**: at most 4 concurrent `GET /mcp` (SSE) streams per IP; each opened stream costs 1 unit of sustained budget. A `GET` refused because 4 streams are already open costs nothing
+- **Malformed bodies**: a non-empty POST body that is not a JSON-RPC message or a non-empty batch costs 1 unit, so malformed requests are not free
 - **Request body**: at most 64 KB, and it must arrive within 30 seconds
 
-A denied request gets HTTP `429 Too Many Requests` with a `Retry-After` header (seconds) and a JSON-RPC error (code `-32000`) carrying the request's `id`. A denied `initialize` is the exception: it returns HTTP 200 with the same JSON-RPC error, because mcp-go clients treat any 4xx on `initialize` as a legacy-SSE server. Every response carries `X-RateLimit-Limit`, `X-RateLimit-Remaining` (sustained budget left in the current minute) and `X-RateLimit-Reset` (Unix time the minute ends). An oversized body gets HTTP `413` with a JSON-RPC error that names the limit; a body that stalls gets HTTP `408`.
+A denied request gets HTTP `429 Too Many Requests` with a `Retry-After` header (seconds) and a JSON-RPC error (code `-32000`) carrying the request's `id`. A denied `initialize`, alone or inside a JSON-RPC batch, is the exception: it returns HTTP 200 with the same JSON-RPC error, because mcp-go clients treat any 4xx on `initialize` as a legacy-SSE server. A `GET` refused by the stream cap gets HTTP 429 with `Retry-After: 60`. Every response to a request the limiter evaluates carries `X-RateLimit-Limit`, `X-RateLimit-Remaining` (sustained budget left in the current minute) and `X-RateLimit-Reset` (Unix time the minute ends); stream-cap, body-size and body-timeout rejections, which happen before that check, do not. An oversized body gets HTTP `413` with a JSON-RPC error that names the limit; a body that stalls gets HTTP `408`.
 
 The 64 KB limit is deliberate: `analyze_sql` allocates roughly 500× the SQL size (about 33 MB for 64 KB of SQL, 530 MB for 1 MB), so larger bodies would let a handful of requests exhaust a small instance.
 
@@ -219,6 +220,8 @@ The `gosqlx-mcp` server is configured exclusively via environment variables. No
 | `GOSQLX_MCP_OPENAI_APPS_CHALLENGE` | *(empty)* | string | ChatGPT app domain-verification token served at `/.well-known/openai-apps-challenge` |
 | `GOSQLX_MCP_TRUSTED_PROXY_HEADER` | *(empty)* | string | Header a trusted proxy sets to the client IP (e.g. `CF-Connecting-IP`); must be a valid header name and not `X-Forwarded-For` |
 | `GOSQLX_MCP_TRUSTED_PROXY_HOPS` | `0` | integer | 0–16; number of trusted proxies that append to `X-Forwarded-For` |
+| `GOSQLX_MCP_TRUSTED_PROXY_SECRET_HEADER` | *(empty)* | string | Optional. Header the proxy sets to the shared secret; set together with `GOSQLX_MCP_TRUSTED_PROXY_SECRET`. Must be a valid header name and not `X-Forwarded-For`, `Authorization` or the client-IP header. Requires the trusted proxy header or hops |
+| `GOSQLX_MCP_TRUSTED_PROXY_SECRET` | *(empty)* | string | Optional. Shared secret, at least 16 characters; whitespace-trimmed. Never logged |
 
 **Notes:**
 
@@ -237,6 +240,30 @@ Behind a reverse proxy every request comes from the proxy, so all users would sh
 
 If the header is set, it takes precedence. A missing or unparseable value falls back to `RemoteAddr`. Only enable either option when every request reaches the server through that proxy; otherwise clients can set the header directly.
 
+**Startup warning.** When neither option is set, `gosqlx-mcp` logs one warning at startup: behind a reverse proxy or load balancer every client then shares the proxy's address, so all of them share one rate-limit bucket and one 4-stream `GET` cap. The warning is expected for a server that clients reach directly; behind a proxy, set one of the options above.
+
+#### Requiring a proxy secret (optional)
+
+A trusted client-IP header is only as trustworthy as the path to the server. If the origin can also be reached without the proxy (for example through Render's `*.onrender.com` hostname rather than the Cloudflare zone), a client can send its own `CF-Connecting-IP` and pick its rate-limit key. To close that gap, have the proxy add a secret header and tell the server to trust forwarded addresses only on requests that carry it:
+
+```bash
+GOSQLX_MCP_TRUSTED_PROXY_HEADER=CF-Connecting-IP
+GOSQLX_MCP_TRUSTED_PROXY_SECRET_HEADER=X-Origin-Secret
+GOSQLX_MCP_TRUSTED_PROXY_SECRET=<random value, at least 16 characters>
+```
+
+With both secret variables set, `GOSQLX_MCP_TRUSTED_PROXY_HEADER` and `GOSQLX_MCP_TRUSTED_PROXY_HOPS` apply only when the last value of the secret header equals the secret (compared in constant time). Requests with a missing or wrong secret are not rejected; they are keyed by `RemoteAddr`, so traffic that bypasses the proxy shares one bucket instead of choosing its own. The secret header is removed before the request reaches the MCP handler, and the secret never appears in logs or configuration errors. With neither variable set, behaviour is unchanged.
+
+To set the header with a Cloudflare Transform Rule:
+
+1. Generate a secret, for example `openssl rand -hex 32`.
+2. In the Cloudflare dashboard for the zone that serves the MCP hostname, open **Rules → Transform Rules → Modify Request Header** and create a rule.
+3. Match the MCP hostname, for example *Hostname equals `mcp.example.com`*.
+4. Add the action **Set static**, header name `X-Origin-Secret`, value the secret. *Set* overwrites any value a client sent, so clients cannot supply their own.
+5. Deploy the rule, then set `GOSQLX_MCP_TRUSTED_PROXY_SECRET_HEADER` and `GOSQLX_MCP_TRUSTED_PROXY_SECRET` on the server as secret environment variables (on Render, in the dashboard rather than in `render.yaml`) and restart it.
+
+Deploy the rule before the server variables: until both sides agree, requests fall back to the shared `RemoteAddr` bucket rather than failing. Rotate the secret the same way. The rule must cover every hostname that should get per-client limits.
+
 ### HTTP server limits
 
 `gosqlx-mcp` sets `ReadHeaderTimeout` 10 s, `IdleTimeout` 120 s and `MaxHeaderBytes` 32 KB. It sets no global `ReadTimeout`/`WriteTimeout`, because those would cut long-lived `GET /mcp` SSE streams. POST bodies instead get a 30 s read deadline in the rate-limit middleware, so slow-body (slowloris) connections are closed with HTTP 408.
@@ -726,7 +753,8 @@ func main() {
 | `LoadConfig` | `() (*Config, error)` | Load from env vars |
 | `DefaultConfig` | `() *Config` | Defaults: `127.0.0.1:8080`, auth disabled |
 | `New` | `(cfg *Config) *Server` | Create server with all 7 tools registered |
-| `(*Server).Start` | `(ctx context.Context) error` | Bind, serve, block until ctx cancelled |
+| `(*Server).Start` | `(ctx context.Context) error` | Bind, serve, block until ctx cancelled. Applies bearer auth only, not `RateLimitMiddleware` |
+| `(*Server).Handler` | `() http.Handler` | MCP transport behind bearer auth, without `RateLimitMiddleware`; wrap it with `NewRateLimitMiddleware` before exposing it to untrusted clients |
 | `BearerAuthMiddleware` | `(cfg *Config, next http.Handler) http.Handler` | Auth wrapper; no-op when auth is disabled |
 | `NewRateLimitMiddleware` | `(cfg *Config, next http.Handler) http.Handler` | Rate limiter keyed by client IP, honouring `cfg`'s trusted-proxy settings |
 | `RateLimitMiddleware` | `(next http.Handler) http.Handler` | `NewRateLimitMiddleware` keyed on `RemoteAddr` only |
@@ -744,6 +772,10 @@ type Config struct {
     OpenAIAppsChallenge string // GOSQLX_MCP_OPENAI_APPS_CHALLENGE (default "" = not served)
     TrustedProxyHeader  string // GOSQLX_MCP_TRUSTED_PROXY_HEADER (default "" = not used)
     TrustedProxyHops    int    // GOSQLX_MCP_TRUSTED_PROXY_HOPS (default 0 = X-Forwarded-For ignored)
+
+    // Optional: trust the two fields above only when this header carries the secret.
+    TrustedProxySecretHeader string // GOSQLX_MCP_TRUSTED_PROXY_SECRET_HEADER (default "" = not required)
+    TrustedProxySecret       string // GOSQLX_MCP_TRUSTED_PROXY_SECRET
 }
 ```
 

pkg/mcp/config.go

@@ -36,6 +36,12 @@
 //	                       the client IP (e.g. CF-Connecting-IP); empty = unused
 //	GOSQLX_MCP_TRUSTED_PROXY_HOPS    number of trusted proxies appending to
 //	                       X-Forwarded-For (0-16, default 0 = ignore XFF)
+//	GOSQLX_MCP_TRUSTED_PROXY_SECRET_HEADER  optional header the trusted proxy
+//	                       sets to GOSQLX_MCP_TRUSTED_PROXY_SECRET; when both are
+//	                       set, forwarded client IPs are trusted only on
+//	                       requests that carry the secret
+//	GOSQLX_MCP_TRUSTED_PROXY_SECRET  shared secret for the header above
+//	                       (at least 16 characters)
 package mcp
 
 import (
@@ -82,11 +88,29 @@ type Config struct {
 	// client-supplied and ignored.
 	// Source: GOSQLX_MCP_TRUSTED_PROXY_HOPS (default: 0 - X-Forwarded-For ignored)
 	TrustedProxyHops int
+
+	// TrustedProxySecretHeader and TrustedProxySecret, when both set, make
+	// TrustedProxyHeader and TrustedProxyHops apply only to requests whose
+	// TrustedProxySecretHeader value equals TrustedProxySecret (compared in
+	// constant time). Other requests are keyed by RemoteAddr. This stops a
+	// client that reaches the origin directly, bypassing the proxy, from
+	// choosing its own rate-limit key. The proxy must overwrite the header on
+	// every request (for example with a Cloudflare Transform Rule). The
+	// header is removed before the request reaches the MCP handler. If only
+	// one of the two is set, forwarded addresses are never trusted.
+	// Source: GOSQLX_MCP_TRUSTED_PROXY_SECRET_HEADER and
+	// GOSQLX_MCP_TRUSTED_PROXY_SECRET (default: "" - not required)
+	TrustedProxySecretHeader string
+	TrustedProxySecret       string
 }
 
 // maxTrustedProxyHops bounds GOSQLX_MCP_TRUSTED_PROXY_HOPS.
 const maxTrustedProxyHops = 16
 
+// minTrustedProxySecretLen is the shortest accepted
+// GOSQLX_MCP_TRUSTED_PROXY_SECRET.
+const minTrustedProxySecretLen = 16
+
 // LoadConfig reads configuration from environment variables, applying defaults
 // for any variables that are unset or empty.
 func LoadConfig() (*Config, error) {
@@ -137,9 +161,44 @@ func LoadConfig() (*Config, error) {
 		cfg.TrustedProxyHops = hops
 	}
 
+	if err := loadTrustedProxySecret(cfg); err != nil {
+		return nil, err
+	}
+
 	return cfg, nil
 }
 
+// loadTrustedProxySecret reads GOSQLX_MCP_TRUSTED_PROXY_SECRET_HEADER and
+// GOSQLX_MCP_TRUSTED_PROXY_SECRET into cfg. It must run after the trusted
+// proxy header and hops are loaded. Errors never include the secret.
+func loadTrustedProxySecret(cfg *Config) error {
+	header := strings.TrimSpace(os.Getenv("GOSQLX_MCP_TRUSTED_PROXY_SECRET_HEADER"))
+	secret := strings.TrimSpace(os.Getenv("GOSQLX_MCP_TRUSTED_PROXY_SECRET"))
+	if header == "" && secret == "" {
+		return nil
+	}
+	if header == "" || secret == "" {
+		return fmt.Errorf("GOSQLX_MCP_TRUSTED_PROXY_SECRET_HEADER and GOSQLX_MCP_TRUSTED_PROXY_SECRET must be set together")
+	}
+	if !validHeaderName(header) {
+		return fmt.Errorf("GOSQLX_MCP_TRUSTED_PROXY_SECRET_HEADER: %q is not a valid HTTP header name", header)
+	}
+	canonical := http.CanonicalHeaderKey(header)
+	switch canonical {
+	case "X-Forwarded-For", "Authorization", cfg.TrustedProxyHeader:
+		return fmt.Errorf("GOSQLX_MCP_TRUSTED_PROXY_SECRET_HEADER: %q cannot be used as the secret header", canonical)
+	}
+	if len(secret) < minTrustedProxySecretLen {
+		return fmt.Errorf("GOSQLX_MCP_TRUSTED_PROXY_SECRET: must be at least %d characters", minTrustedProxySecretLen)
+	}
+	if cfg.TrustedProxyHeader == "" && cfg.TrustedProxyHops == 0 {
+		return fmt.Errorf("GOSQLX_MCP_TRUSTED_PROXY_SECRET requires GOSQLX_MCP_TRUSTED_PROXY_HEADER or GOSQLX_MCP_TRUSTED_PROXY_HOPS")
+	}
+	cfg.TrustedProxySecretHeader = canonical
+	cfg.TrustedProxySecret = secret
+	return nil
+}
+
 // validHeaderName reports whether s is a valid HTTP header field name
 // (an RFC 9110 token).
 func validHeaderName(s string) bool {