Skip to content

Commit 87c9b07

Browse files
morrisd-devclaude
andcommitted
fix: prevent glob injection and cross-server secret leakage in MCP gateway
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>
1 parent 88f82d1 commit 87c9b07

File tree

5 files changed

+43
-1
lines changed

5 files changed

+43
-1
lines changed

cmd/docker-mcp/secret-management/secret/credstore.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,16 @@ func cmd(ctx context.Context, args ...string) *exec.Cmd {
2424
return exec.CommandContext(ctx, "docker", append([]string{"pass"}, args...)...)
2525
}
2626

27+
// ValidateSecretName returns an error if the name contains glob metacharacters
28+
// or path traversal sequences that could be injected into the pattern sent to
29+
// the Desktop secrets resolver (which treats the pattern field as a glob).
30+
func ValidateSecretName(name string) error {
31+
if strings.ContainsAny(name, "*?[]{") {
32+
return fmt.Errorf("secret name %q contains illegal glob metacharacter", name)
33+
}
34+
return nil
35+
}
36+
2737
// GetDefaultSecretKey constructs the full namespaced ID for an MCP secret
2838
// using the default namespace (docker/mcp/).
2939
//

cmd/docker-mcp/secret-management/secret/secret_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,18 @@ func TestGetDefaultSecretKey(t *testing.T) {
1212
assert.Equal(t, "docker/mcp/mykey", result)
1313
}
1414

15+
func TestValidateSecretName(t *testing.T) {
16+
valid := []string{"mykey", "postgres_password", "GITHUB_TOKEN", "some-key", "key.with.dots"}
17+
for _, name := range valid {
18+
assert.NoError(t, ValidateSecretName(name), "expected %q to be valid", name)
19+
}
20+
21+
invalid := []string{"**", "*", "docker/mcp/**", "key*", "key?", "key[0]", "key{a}"}
22+
for _, name := range invalid {
23+
assert.Error(t, ValidateSecretName(name), "expected %q to be invalid", name)
24+
}
25+
}
26+
1527
func TestParseArg(t *testing.T) {
1628
// Test key=value parsing
1729
secret, err := ParseArg("key=value", SetOpts{})

pkg/gateway/configuration.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,21 @@ func (c *Configuration) Find(serverName string) (*catalog.ServerConfig, *map[str
9292

9393
// Is it an MCP Server?
9494
if server.Image != "" || server.SSEEndpoint != "" || server.Remote.URL != "" {
95+
// Scope secrets to only the keys declared by this server so that a
96+
// compromised or malicious server cannot access another server's secrets.
97+
scopedSecrets := make(map[string]string, len(server.Secrets))
98+
for _, s := range server.Secrets {
99+
if v, ok := c.secrets[s.Name]; ok {
100+
scopedSecrets[s.Name] = v
101+
}
102+
}
95103
return &catalog.ServerConfig{
96104
Name: serverName,
97105
Spec: server,
98106
Config: map[string]any{
99107
oci.CanonicalizeServerName(serverName): c.config[oci.CanonicalizeServerName(serverName)],
100108
},
101-
Secrets: c.secrets, // TODO: we could keep just the secrets for this server
109+
Secrets: scopedSecrets,
102110
}, nil, true
103111
}
104112

pkg/gateway/secrets_uri.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ func buildFallbackURIs(configs []ServerSecretConfig) map[string]string {
4848
secretToOAuthID := oauthMapping(cfg)
4949

5050
for _, s := range cfg.Secrets {
51+
if err := secret.ValidateSecretName(s.Name); err != nil {
52+
log.Logf("Warning: skipping secret with invalid name %q: %v", s.Name, err)
53+
continue
54+
}
5155
secretName := cfg.Namespace + s.Name
5256
if oauthSecretID, ok := secretToOAuthID[s.Name]; ok {
5357
secretNameToURI[secretName] = "se://" + oauthSecretID
@@ -68,6 +72,10 @@ func buildVerifiedURIs(configs []ServerSecretConfig, availableSecrets map[string
6872
secretToOAuthID := oauthMapping(cfg)
6973

7074
for _, s := range cfg.Secrets {
75+
if err := secret.ValidateSecretName(s.Name); err != nil {
76+
log.Logf("Warning: skipping secret with invalid name %q: %v", s.Name, err)
77+
continue
78+
}
7179
secretName := cfg.Namespace + s.Name
7280

7381
// Try OAuth token first

pkg/mcp/remote.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,10 @@ func (c *remoteMCPClient) AddRoots(roots []*mcp.Root) {
169169
}
170170

171171
func getSecretValue(ctx context.Context, secretName string) string {
172+
if err := secret.ValidateSecretName(secretName); err != nil {
173+
log.Logf("Warning: skipping secret with invalid name %q: %v", secretName, err)
174+
return ""
175+
}
172176
fullID := secret.GetDefaultSecretKey(secretName)
173177
env, err := secret.GetSecret(ctx, fullID)
174178
if err != nil {

0 commit comments

Comments
 (0)