Skip to content

Commit 5e9c9cd

Browse files
feat(security): mcp_reject_query_apikey refuses ?apikey= credentials on MCP endpoints (#1696)
## Description Fixes #1695. Adds the opt-in config key `mcp_reject_query_apikey` (default `false`, so behaviour is unchanged unless set). When `true`, every `/mcp*` endpoint (`/mcp`, `/mcp/all`, `/mcp/call`, `/mcp/code`, `/mcp/p/...`) answers a request that carries an `apikey` query parameter with `401` and a message that names the header forms, before any credential lookup. A credential in a URL ends up in reverse-proxy logs, browser history and `Referer` headers; this lets a deployment whose agents can all send a header make that impossible. The refusal is explicit rather than "treat as no token": with `require_mcp_auth` off an unrecognised credential is served as the anonymous administrator, so silently ignoring the parameter would downgrade a caller that believes it is authenticated. Scope: MCP endpoints only. The REST API, Web UI and `/events` keep accepting `?apikey=` (the UI builds such URLs for SSE and exports). The key is live (listed in `undiffedHotConfigFields`), no restart. Changes: `internal/config/config.go`, `internal/server/server.go` (`mcpAuthMiddleware`), `internal/runtime/config_hotreload.go`, docs (`agent-tokens.md`, `config-file.md`), CHANGELOG, regenerated `oas/` (via `make swagger`, which is clean on `main` without this change). Checked with a built binary: with the key on, `/mcp`, `/mcp/all` and `/mcp/p/x` with `?apikey=` return 401 with the message, a header credential is not refused, and `GET /api/v1/status?apikey=` still returns 200. `go build -tags server ./...` builds. ## Testing - [x] I have tested these changes locally - [x] I have added/updated tests that prove my fix is effective or my feature works (`internal/server/mcp_reject_query_apikey_test.go`: refused on `/mcp`, `/mcp/all`, `/mcp/code` for populated, empty and agent-token values with the handler never reached; header credentials and unrelated query parameters unaffected; default keeps the query form working and authenticated, not anonymous) - [x] All existing tests pass (`go test ./internal/config ./internal/runtime ./cmd/release-gate`, and the `TestMCPAuthMiddleware*` tests in `internal/server`; I did not run the full `internal/server` e2e tests that need a built binary or `npx`) --------- Co-authored-by: Algis Dumbris <a.dumbris@gmail.com>
1 parent 559fba0 commit 5e9c9cd

9 files changed

Lines changed: 111 additions & 2 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ Releases follow [Semantic Versioning](https://semver.org/).
7979

8080
### Features
8181

82+
- **`mcp_reject_query_apikey` refuses query-parameter credentials on MCP endpoints.** Set it to `true` and every MCP endpoint (`/mcp`, `/mcp/p/<slug>`, `/mcp/all`, `/mcp/code`, `/mcp/call` and the legacy `/v1/tool_code` aliases) answers a request carrying `?apikey=` with a 401 instead of honouring it, which forces every client onto `Authorization: Bearer` or `X-API-Key`. It cannot stop a URL that was already sent from being logged by whatever sits in front of the proxy; it makes sure such a client stops working until it is migrated, so the token stops appearing in access logs, proxy logs and browser history from then on. The refusal is explicit rather than "no credential" because, with `require_mcp_auth` off, an unrecognised credential would otherwise be served as the anonymous administrator. Default `false`, so nothing changes unless you opt in; the REST API, the Web UI and its `/events` stream are not affected. Live, no restart. **Caveat:** the Connect wizard writes Codex (and the generic fallback) entries with `?apikey=<credential>` in the URL, so a client connected that way gets a 401 once the option is on: remove the query parameter and carry the credential in a header instead (`env_http_headers` or `http_headers`, see the Connect clients guide); Connect itself is not yet aware of the option.
8283
- **config: `MCPPROXY_API_KEY_FILE` reads the API key from a file (#1706).** The `*_FILE` form of Docker Compose/Swarm secrets, Kubernetes secret volumes and systemd `LoadCredential=`: the key is not in the process environment (`docker inspect`, `/proc/<pid>/environ`) and is never written to `mcp_config.json`. The path must be absolute; one trailing LF or CRLF is removed; at most 64 KiB is read. Setting both `MCPPROXY_API_KEY` and `MCPPROXY_API_KEY_FILE` (both non-empty) is an error: mcpproxy refuses to start with a message naming both variables (never a key or file contents), and the CLI commands and the tray stop the same way. An empty value is treated as unset. A file that is named but cannot be used stops startup with an error naming the variable and path: mcpproxy does not fall back to the config file or a generated key. `mcpproxy status`, the other CLI commands and the tray read it too, and a tray-launched core reads the file itself instead of receiving the key in its environment. `Config.EnsureAPIKey` now also returns an error. (refs #1706, spec 111 FR-014/FR-030/US7)
8384

8485
- **catalog:** a catalog source whose live search times out or fails is now answered from the listing

‎docs/configuration/config-file.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ MCPProxy uses a JSON configuration file located at `~/.mcpproxy/mcp_config.json`
6262
| `trusted_hosts` | string[] | `[]` | Non-loopback `Host` header values accepted on a loopback listener. Needed when running behind a reverse proxy — see [Reverse Proxy Deployment](/operations/reverse-proxy). This is DNS-rebinding protection only; it is **not** the control for forwarded headers (see `trusted_proxies`) |
6363
| `trusted_proxies` | string[] | `[]` (trust nobody) | CIDRs or IP addresses whose `X-Forwarded-For`, `X-Real-IP`, `X-Forwarded-Proto` and `X-Forwarded-Host` headers are believed. Headers from any other peer are ignored and the direct `RemoteAddr` is used. Env `MCPPROXY_TRUSTED_PROXIES` (comma list). Live (hot-reload, no restart). Validation: `trusted_proxies[N] "value" is not a valid CIDR or IP address` — refused identically at boot, `PATCH /api/v1/config` and `/config/apply`. See [Reverse Proxy Deployment](/operations/reverse-proxy#trusted_proxies-forwarded-headers) |
6464
| `require_mcp_auth` | boolean | `false` | Require an API key on the `/mcp` endpoint (off by default for client compatibility). Enable when exposing MCPProxy beyond localhost. **Server edition:** forced to `true` whenever `server_edition.enabled` is `true` — an explicit `false` is not an error, but boot logs `require_mcp_auth: false is overridden to true because server_edition.enabled is true` and `mcpproxy doctor` reports the same finding |
65+
| `mcp_reject_query_apikey` | boolean | `false` | Refuse a `?apikey=` query parameter on every MCP endpoint (`/mcp*` and the legacy `/v1/tool_code` aliases) with a 401, forcing clients onto `Authorization: Bearer <token>` or `X-API-Key` so tokens stop appearing in access logs, proxy logs and browser history (a URL already sent is still logged in transit; the option makes the client fail until it is migrated). MCP endpoints only: the REST API, the Web UI and `/events` keep accepting the query form. Clients connected by the Connect wizard with `?apikey=` in the URL (Codex, the generic fallback) must be switched to a header carrier first. Live, no restart |
6566
| `enable_socket` | boolean | `true` | Enable Unix socket/named pipe for local communication |
6667
| `forward_client_headers` | boolean | `true` | Master switch for [client header forwarding](/configuration/upstream-servers#client-header-forwarding). Nothing is forwarded until a server lists names in `forward_headers`. Env `MCPPROXY_FORWARD_CLIENT_HEADERS=false` disables it for the process (never persisted). |
6768

‎docs/features/agent-tokens.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,7 @@ curl -X POST http://localhost:8080/api/v1/tokens \
745745
| Field | Type | Default | Description |
746746
|-------|------|---------|-------------|
747747
| `require_mcp_auth` | bool | `false` | Require authentication on `/mcp` endpoint |
748+
| `mcp_reject_query_apikey` | bool | `false` | Refuse a `?apikey=` query parameter on every MCP endpoint (`/mcp*` and the legacy `/v1/tool_code` aliases) with a 401, forcing clients onto `Authorization: Bearer <token>` or `X-API-Key`. It does not stop a URL already sent from being logged in transit; it makes a query-form client fail until it is migrated, so tokens stop appearing in access logs, proxy logs and browser history. Applies to MCP endpoints only; the REST API, the Web UI and its `/events` stream keep accepting the query form. Clients the Connect wizard configured with `?apikey=` in the URL (Codex, the generic fallback) need a header carrier first (see [Connect clients](./connect-clients.md)). Takes effect without a restart |
748749
| `api_key` | string | auto-generated | Admin API key for full access |
749750

750751
### CLI Flags

‎internal/config/config.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,14 @@ type Config struct {
378378
// Security settings
379379
APIKey string `json:"api_key,omitempty" mapstructure:"api-key"` // API key for REST API authentication
380380
RequireMCPAuth bool `json:"require_mcp_auth" mapstructure:"require-mcp-auth"` // Require authentication on /mcp endpoint (default: false)
381+
// MCPRejectQueryAPIKey makes every MCP endpoint (/mcp*) refuse a request that
382+
// carries a ?apikey= query parameter, with a 401, instead of honouring it. A credential in a URL ends up in access logs,
383+
// proxy logs and browser history; MCP clients can send Authorization:
384+
// Bearer or X-API-Key instead. Default false (the query form keeps
385+
// working, for clients that can only configure a URL). It covers only the
386+
// MCP endpoints: the Web UI and its SSE stream (/events) still use the
387+
// query form. Live, no restart needed.
388+
MCPRejectQueryAPIKey bool `json:"mcp_reject_query_apikey,omitempty" mapstructure:"mcp-reject-query-apikey"`
381389
// TrustedHosts lists non-loopback Host header values accepted on loopback
382390
// listeners (GH #898). DNS-rebinding protection rejects requests whose Host
383391
// header is not a loopback address when mcpproxy listens on loopback; a

‎internal/runtime/config_hotreload.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ func (r *ConfigApplyResult) FormatChangedFields() string {
599599
// adopted into the live config, and reported by its JSON key.
600600
var undiffedHotConfigFields = []string{
601601
"EnableTray", "TopK", "MaxResultSizeChars", "InitTimeout", "ForwardProxyEnv",
602-
"RequireMCPAuth", "CheckServerRepo", "DockerRecovery",
602+
"RequireMCPAuth", "MCPRejectQueryAPIKey", "CheckServerRepo", "DockerRecovery",
603603
"RegistriesLocked", "AllowPrivateRegistryFetch", "Features",
604604
"ToolResponseSessionRiskWarning", "OAuthExpiryWarningHours",
605605
"ActivityRetentionDays", "ActivityMaxRecords", "ActivityMaxSizeMB",
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package server
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
"go.uber.org/zap"
11+
12+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
13+
)
14+
15+
func newRejectQueryTestServer(t *testing.T, reject bool) *Server {
16+
t.Helper()
17+
cfg := config.DefaultConfig()
18+
cfg.DataDir = t.TempDir()
19+
cfg.Listen = "127.0.0.1:0"
20+
cfg.APIKey = "test-api-key-reject-query"
21+
cfg.MCPRejectQueryAPIKey = reject
22+
srv, err := NewServer(cfg, zap.NewNop())
23+
require.NoError(t, err)
24+
t.Cleanup(func() { _ = srv.Shutdown() })
25+
return srv
26+
}
27+
28+
// With mcp_reject_query_apikey a URL-borne credential is a 401 on every MCP
29+
// path, and the downstream handler never runs. It must not degrade to the
30+
// anonymous administrator that an unrecognised credential gets when
31+
// require_mcp_auth is off.
32+
func TestMCPAuthMiddleware_RejectQueryAPIKey(t *testing.T) {
33+
srv := newRejectQueryTestServer(t, true)
34+
35+
for _, path := range []string{"/mcp", "/mcp/all", "/mcp/code"} {
36+
t.Run("query credential refused on "+path, func(t *testing.T) {
37+
for _, q := range []string{"?apikey=test-api-key-reject-query", "?apikey=", "?apikey=mcp_agt_whatever"} {
38+
seen, code := captureAuthContext(t, srv, httptest.NewRequest(http.MethodPost, path+q, http.NoBody))
39+
assert.Equal(t, http.StatusUnauthorized, code, q)
40+
assert.Nil(t, seen, "handler must not run for %s", q)
41+
}
42+
})
43+
}
44+
45+
t.Run("header credentials still work", func(t *testing.T) {
46+
req := httptest.NewRequest(http.MethodPost, "/mcp", http.NoBody)
47+
req.Header.Set("X-API-Key", "test-api-key-reject-query")
48+
seen, code := captureAuthContext(t, srv, req)
49+
require.Equal(t, http.StatusOK, code)
50+
require.NotNil(t, seen)
51+
assert.True(t, seen.IsAdmin())
52+
assert.False(t, seen.Anonymous)
53+
54+
req = httptest.NewRequest(http.MethodPost, "/mcp", http.NoBody)
55+
req.Header.Set("Authorization", "Bearer test-api-key-reject-query")
56+
seen, code = captureAuthContext(t, srv, req)
57+
require.Equal(t, http.StatusOK, code)
58+
require.NotNil(t, seen)
59+
assert.False(t, seen.Anonymous)
60+
})
61+
62+
t.Run("other query parameters are untouched", func(t *testing.T) {
63+
seen, code := captureAuthContext(t, srv, httptest.NewRequest(http.MethodPost, "/mcp?profile=work", http.NoBody))
64+
require.Equal(t, http.StatusOK, code)
65+
require.NotNil(t, seen)
66+
})
67+
}
68+
69+
// The default keeps the query form working, so existing clients that can only
70+
// configure a URL are unaffected.
71+
func TestMCPAuthMiddleware_QueryAPIKeyAcceptedByDefault(t *testing.T) {
72+
srv := newRejectQueryTestServer(t, false)
73+
74+
seen, code := captureAuthContext(t, srv, httptest.NewRequest(http.MethodPost, "/mcp?apikey=test-api-key-reject-query", http.NoBody))
75+
require.Equal(t, http.StatusOK, code)
76+
require.NotNil(t, seen)
77+
assert.True(t, seen.IsAdmin())
78+
assert.False(t, seen.Anonymous, "the query credential must still authenticate by default")
79+
}

‎internal/server/server.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,15 @@ func (s *Server) mcpAuthMiddleware(next http.Handler) http.Handler {
574574
// consumers.
575575
next = directRequestKindMiddleware(next)
576576
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
577+
// mcp_reject_query_apikey: refuse a URL-borne credential outright. It
578+
// must be a 401 and not "no token": with require_mcp_auth off a request
579+
// without a recognised credential is served as the anonymous
580+
// administrator, so silently ignoring the parameter would downgrade a
581+
// caller that thinks it is authenticated.
582+
if cfg := s.runtime.Config(); cfg != nil && cfg.MCPRejectQueryAPIKey && r.URL.Query().Has("apikey") {
583+
http.Error(w, `{"error":"A credential in the ?apikey= query parameter is not accepted on MCP endpoints. Send it as Authorization: Bearer <token> or X-API-Key."}`, http.StatusUnauthorized)
584+
return
585+
}
577586
token := httpapi.ExtractToken(r)
578587
if token == "" {
579588
// Check if MCP auth is required

‎oas/docs.go‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎oas/swagger.yaml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,16 @@ components:
266266
inline-response ceiling from 50k to up to 500k chars. Omit the key for
267267
the 500000 default; set it to 0 to disable the annotation.
268268
type: integer
269+
mcp_reject_query_apikey:
270+
description: |-
271+
MCPRejectQueryAPIKey makes every MCP endpoint (/mcp*) refuse a request that
272+
carries a ?apikey= query parameter, with a 401, instead of honouring it. A credential in a URL ends up in access logs,
273+
proxy logs and browser history; MCP clients can send Authorization:
274+
Bearer or X-API-Key instead. Default false (the query form keeps
275+
working, for clients that can only configure a URL). It covers only the
276+
MCP endpoints: the Web UI and its SSE stream (/events) still use the
277+
query form. Live, no restart needed.
278+
type: boolean
269279
mcpServers:
270280
items:
271281
$ref: '#/components/schemas/config.ServerConfig'

0 commit comments

Comments
 (0)