Skip to content

fix: prevent glob injection and cross-server secret leakage in MCP gateway#467

Merged
saucow merged 1 commit intodocker:mainfrom
morrisd-dev:fix/secret-name-glob-injection
Apr 6, 2026
Merged

fix: prevent glob injection and cross-server secret leakage in MCP gateway#467
saucow merged 1 commit intodocker:mainfrom
morrisd-dev:fix/secret-name-glob-injection

Conversation

@morrisd-dev
Copy link
Copy Markdown
Contributor

@morrisd-dev morrisd-dev commented Apr 3, 2026

Summary

Fixes two secret over-disclosure vulnerabilities in the MCP gateway reported via security@docker.com (related to SSH-10):

  • Primary (glob injection): A remote MCP server catalog entry with name: ** caused GetDefaultSecretKey to produce docker/mcp/**, which was embedded verbatim into the JSON pattern sent to the Desktop secrets resolver. The resolver treats this as a glob and returns all vault secrets, which then get injected into outbound HTTP headers to the attacker's server.
  • Secondary (cross-server leakage): configuration.Find() passed the full c.secrets map (all servers' secrets) to every ServerConfig. An attacker who guesses a secret name from the public catalog could retrieve another server's secret without touching the secrets engine.

Changes

  • credstore.go — Add ValidateSecretName() as a single enforcement point; rejects names containing glob metacharacters (*?[]{)
  • remote.go — Call ValidateSecretName() in getSecretValue() before constructing the secrets engine query, blocking HTTP header exfiltration
  • secrets_uri.go — Call ValidateSecretName() in both buildFallbackURIs and buildVerifiedURIs to block glob injection into se:// URIs for container-based servers
  • configuration.go — Scope Secrets in Find() to only the keys declared by that specific server (resolves the pre-existing TODO comment)

Test plan

  • TestValidateSecretName covers valid and invalid name cases
  • make lint-darwin passes
  • End-to-end: a catalog entry with name: ** no longer leaks secrets into remote server headers
  • Legitimate secrets with normal names continue to resolve correctly

🤖 Generated with Claude Code

@morrisd-dev morrisd-dev requested a review from a team as a code owner April 3, 2026 15:18
…teway

Validates secret names against glob metacharacters before they reach the
Desktop secrets resolver, which treats the pattern field as a glob. A
catalog entry with name "**" would otherwise cause GetSecret to send
{"pattern": "docker/mcp/**"}, returning all vault secrets and injecting
them into outbound HTTP headers to the remote server (SSH-10 / external
report).

Also scopes the secrets map in configuration.Find() to only the keys
declared by each server, preventing cross-server secret access by name
guessing.

- Add ValidateSecretName() to the secret package as the enforcement point
- Call it in remote.go:getSecretValue() to block HTTP header exfiltration
- Call it in secrets_uri.go buildFallbackURIs/buildVerifiedURIs to block
  glob injection into se:// URIs for container-based servers
- Scope Secrets in configuration.Find() to per-server declared secrets

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@morrisd-dev morrisd-dev force-pushed the fix/secret-name-glob-injection branch from 87c9b07 to cf4d4bc Compare April 3, 2026 15:22
@xenoscopic
Copy link
Copy Markdown
Contributor

@slimslenderslacks Can you take a look?

Copy link
Copy Markdown
Contributor

@saucow saucow left a comment

Choose a reason for hiding this comment

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

Reviewed and tested locally.

Glob injection fix (ValidateSecretName): Confirmed the issue — a wildcard in the secret name gets passed through to the secrets resolver pattern query. Blocking * is the correct fix; it's the only character the resolver interprets as a wildcard.

Secret scoping in Find(): Good cleanup. Resolves the pre-existing TODO and applies least-privilege to the secrets map. Both code paths that consume ServerConfig.Secrets (remote.go and clientpool.go) already iterate only the server's declared secrets and look up by declared name, so the full map wasn't directly exploitable, but scoping it down is the right thing to do.

On the ?[]{ blocking: The resolver already rejects these characters before they reach the matching logic, so blocking them in the gateway is harmless defense-in-depth rather than a required fix. No issue including them.

On path traversal: The ValidateSecretName doc comment mentions "path traversal sequences" but the function doesn't block .. or /. This is fine for the scope of this PR — the wildcard fix is what matters here. Noting it for accuracy since the doc comment implies broader coverage than the implementation provides.

Testing:

  • Happy path: container servers with API key secrets, container servers with OAuth secrets, and remote servers with OAuth all work correctly. No validation warnings for legitimate secret names. Tool calls succeed across all server types.
  • Negative path: a test catalog entry with a wildcard secret name pointing at a local server. Gateway correctly rejects the name with a warning and the remote server receives empty headers. No secret leaked.

@saucow saucow merged commit cd0f7c0 into docker:main Apr 6, 2026
5 checks passed
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.

4 participants