quarantine_security list_quarantined returns env/headers/oauth secrets in plaintext, unauthenticated

Summary

quarantine_security operation=list_quarantined serialises raw config.ServerConfig values, returning every quarantined server's env, headers, and oauth.client_secret in plaintext — over the /mcp endpoint, which is unauthenticated by default (require_mcp_auth: false).

Reproduction

Verified against main (0750fcd) on a scratch instance. Every request below was sent to /mcp with no X-API-Key header and no credential of any kind.

Server configured with a secret env var and a secret header:

env: {'QA_SECRET_TOKEN': 'sk-super-secret-value-12345'}
headers: {'X-AuthToken': 'hdr-secret-98765'}

Then, unauthenticated:

{"method":"tools/call","params":{"name":"quarantine_security",
 "arguments":{"operation":"list_quarantined"}}}

Response:

LEAKS env secret: True
LEAKS header secret: True
...,"env":{"QA_SECRET_TOKEN":"sk-super-secret-value-12345"},
    "headers":{"X-AuthToken":"hdr-secret-98765"},...

Why it is reachable unauthenticated

internal/server/mcp.go:4136 marshals the storage result directly:

servers, err := p.storage.ListQuarantinedUpstreamServers()   // []*config.ServerConfig
...
jsonResult, err := json.Marshal(map[string]interface{}{"servers": servers, ...})

config.ServerConfig carries Env, Headers and OAuth — none are masked on this path.

The agent-token gate is:

if authCtx := auth.AuthContextFromContext(ctx); authCtx != nil && !authCtx.IsAdmin() {
	// reject
}

A nil auth context — an unauthenticated /mcp session — passes straight through. The gate blocks scoped agent tokens but not the absence of a token. /mcp is documented as intentionally unprotected for client compatibility, so on a default install this is exposed to anything that can reach the listener, including a prompt-injected agent already connected to the proxy.

Impact

Quarantine exists to hold untrusted servers pending review. Exactly those servers' credentials are the ones this dumps. An attacker who can reach /mcp — or an LLM agent following injected instructions — can exfiltrate every quarantined server's API keys, auth headers, and OAuth client secrets in one call.

The response is also persisted verbatim into the activity store (internal/server/mcp.go:3500), so the secrets land in BBolt and in mcpproxy activity list output.

Sibling paths (same root cause: raw values on a recorded response)

Found in the same review, not individually reproduced:

upstream_servers list with RevealSecretHeaders=true persists its intentionally-revealed credentials into the activity store.
tail_log (mcp.go:5493) returns raw log lines, then persists them.
Connection errors inserted unscrubbed at mcp.go:4940, 5785, 5790 — errors that echo URLs, headers, or process args reach both the response and the activity store, and mcp.go:4938 logs the original.

Suggested fix

Mask env / headers / oauth.client_secret in handleListQuarantinedUpstreams before marshalling. The inspection use case needs to know which keys are set, not their values.
Fix the auth gate so a nil auth context is treated as unprivileged on secret-revealing operations, rather than as admin.
More generally: these handlers marshal config.ServerConfig directly. A dedicated redacted view type for every MCP-facing surface would stop the next field that gets added from leaking by default.

Found while working on #1146 (which seals the request-argument and config-diff paths; this one is a separate, pre-existing leak on the response path and is not fixed by that PR).

Correction to the mechanism, and the leak is wider than reported

Two corrections after reading the code properly. The vulnerability is real and reproduces exactly as reported — only my explanation of why it is reachable was wrong.

1. It is not a nil auth context — it is a deliberate anonymous→admin upgrade

I wrote that authCtx != nil && !authCtx.IsAdmin() lets a nil context through. That is wrong for the HTTP path. mcpAuthMiddleware (internal/server/server.go:336) explicitly installs auth.AdminContext() for an unauthenticated /mcp request (lines 352-354), and again for an unrecognised token (lines 426-428). All five MCP mounts are wrapped.

So the gate is not bypassed by nil — an anonymous caller is deliberately promoted to admin for back-compat. A nil context can only arise on the stdio transport (server.go:963, no middleware) and from in-process callers.

This changes the fix: it has to distinguish "anonymous admin" from "authenticated admin", not merely be nil-safe.

2. The leak is wider than env + headers

Observed payload from list_quarantined:

{"servers":[{"name":"evil","quarantined":true,
  "url":"https://host/mcp?token=urlsecret999",
  "args":["mcp-foo","--api-key","sk-argv-secret-777"],
  "env":{"QA_SECRET_TOKEN":"sk-super-secret-value-12345"},
  "headers":{"X-AuthToken":"hdr-secret-98765"},
  "oauth":{"client_id":"cid","client_secret":"oauth-secret-4242"}}]}

URL query credentials and argv credentials leak too — not just env/headers/oauth.client_secret.

3. Two sibling paths reassessed

upstream_servers list is not the same severity. handleListUpstreams already masks env/headers/url via oauth.Redact* (upstream_servers list should redact env secrets and sensitive URL query params #872) and scrubs last_error. Its genuine remaining holes are narrower: reveal_secret_headers:true hands raw values to any caller including an unauthenticated one; the raw response text is recorded to activity unredacted; and the projection is a hand-written key list, so a future ServerConfig field added to it leaks by default.
inspect_quarantined does not emit server config at all — only tool analysis. My listing it was wrong.
tail_log does leak, and mcpproxy is the source: internal/upstream/core/connection.go:110 writes the raw URL including ?token= into server-<name>.log on every connection attempt, and connection_launcher.go:79 pipes the child's stdout/stderr into the same file. handleTailLog returns those lines verbatim.
Connection errors at mcp.go:5857/5862/5006 return LastError unscrubbed, while the list path at 3806 scrubs the same error. The inconsistency is the bug.

A fix is in progress on fix/1148-quarantine-secrets.

fix(security): route the remaining server-payload doors through one r…

test(security): follow the upstream_stats producer rename in the door…

fix(security): gate the REST and SSE doors on caller identity and tok…

fix(security): stop credentials reaching logs, stdout and structured …

fix(security): mask secrets on MCP server payloads and stop nil/anony…

fix(security): bind the argv unmask, and close the six sibling gaps the

fix(security): refuse echoed argv masks instead of reverting them, an…

fix(security): close the last raw-URL log and status sites the smart-…

fix(security): bind or refuse every mask on the write path, and close…

fix(security): one redaction rule and one revert rule for every serve…

fix(security): derive the mask net from its renderers, and walk neste…

fix(security): mask shared servers on the server edition's per-user door

fix(telemetry): stop four diagnostics instruments measuring themselves (

Navigation Menu

Repository navigation

quarantine_security list_quarantined returns env/headers/oauth secrets in plaintext, unauthenticated #1148

Summary

Reproduction

Why it is reachable unauthenticated

Impact

Sibling paths (same root cause: raw values on a recorded response)

Suggested fix

Correction to the mechanism, and the leak is wider than reported

1. It is not a nil auth context — it is a deliberate anonymous→admin upgrade

2. The leak is wider than env + headers

3. Two sibling paths reassessed

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

quarantine_security list_quarantined returns env/headers/oauth secrets in plaintext, unauthenticated #1148

Description

Summary

Reproduction

Why it is reachable unauthenticated

Impact

Sibling paths (same root cause: raw values on a recorded response)

Suggested fix

Activity

Dumbris commented on Sep 1, 2026

Correction to the mechanism, and the leak is wider than reported

1. It is not a nil auth context — it is a deliberate anonymous→admin upgrade

2. The leak is wider than env + headers

3. Two sibling paths reassessed

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions