diff --git a/cmd/docker-mcp/secret-management/secret/credstore.go b/cmd/docker-mcp/secret-management/secret/credstore.go index d35e38ea0..54855706d 100644 --- a/cmd/docker-mcp/secret-management/secret/credstore.go +++ b/cmd/docker-mcp/secret-management/secret/credstore.go @@ -24,6 +24,16 @@ func cmd(ctx context.Context, args ...string) *exec.Cmd { return exec.CommandContext(ctx, "docker", append([]string{"pass"}, args...)...) } +// ValidateSecretName returns an error if the name contains glob metacharacters +// or path traversal sequences that could be injected into the pattern sent to +// the Desktop secrets resolver (which treats the pattern field as a glob). +func ValidateSecretName(name string) error { + if strings.ContainsAny(name, "*?[]{") { + return fmt.Errorf("secret name %q contains illegal glob metacharacter", name) + } + return nil +} + // GetDefaultSecretKey constructs the full namespaced ID for an MCP secret // using the default namespace (docker/mcp/). // diff --git a/cmd/docker-mcp/secret-management/secret/secret_test.go b/cmd/docker-mcp/secret-management/secret/secret_test.go index b6c8148c7..141cf79dc 100644 --- a/cmd/docker-mcp/secret-management/secret/secret_test.go +++ b/cmd/docker-mcp/secret-management/secret/secret_test.go @@ -12,6 +12,18 @@ func TestGetDefaultSecretKey(t *testing.T) { assert.Equal(t, "docker/mcp/mykey", result) } +func TestValidateSecretName(t *testing.T) { + valid := []string{"mykey", "postgres_password", "GITHUB_TOKEN", "some-key", "key.with.dots"} + for _, name := range valid { + require.NoError(t, ValidateSecretName(name), "expected %q to be valid", name) + } + + invalid := []string{"**", "*", "docker/mcp/**", "key*", "key?", "key[0]", "key{a}"} + for _, name := range invalid { + assert.Error(t, ValidateSecretName(name), "expected %q to be invalid", name) + } +} + func TestParseArg(t *testing.T) { // Test key=value parsing secret, err := ParseArg("key=value", SetOpts{}) diff --git a/pkg/gateway/configuration.go b/pkg/gateway/configuration.go index dd60f3b7f..73f9d2861 100644 --- a/pkg/gateway/configuration.go +++ b/pkg/gateway/configuration.go @@ -92,13 +92,21 @@ func (c *Configuration) Find(serverName string) (*catalog.ServerConfig, *map[str // Is it an MCP Server? if server.Image != "" || server.SSEEndpoint != "" || server.Remote.URL != "" { + // Scope secrets to only the keys declared by this server so that a + // compromised or malicious server cannot access another server's secrets. + scopedSecrets := make(map[string]string, len(server.Secrets)) + for _, s := range server.Secrets { + if v, ok := c.secrets[s.Name]; ok { + scopedSecrets[s.Name] = v + } + } return &catalog.ServerConfig{ Name: serverName, Spec: server, Config: map[string]any{ oci.CanonicalizeServerName(serverName): c.config[oci.CanonicalizeServerName(serverName)], }, - Secrets: c.secrets, // TODO: we could keep just the secrets for this server + Secrets: scopedSecrets, }, nil, true } diff --git a/pkg/gateway/secrets_uri.go b/pkg/gateway/secrets_uri.go index b4140348c..12f76944e 100644 --- a/pkg/gateway/secrets_uri.go +++ b/pkg/gateway/secrets_uri.go @@ -48,6 +48,10 @@ func buildFallbackURIs(configs []ServerSecretConfig) map[string]string { secretToOAuthID := oauthMapping(cfg) for _, s := range cfg.Secrets { + if err := secret.ValidateSecretName(s.Name); err != nil { + log.Logf("Warning: skipping secret with invalid name %q: %v", s.Name, err) + continue + } secretName := cfg.Namespace + s.Name if oauthSecretID, ok := secretToOAuthID[s.Name]; ok { secretNameToURI[secretName] = "se://" + oauthSecretID @@ -68,6 +72,10 @@ func buildVerifiedURIs(configs []ServerSecretConfig, availableSecrets map[string secretToOAuthID := oauthMapping(cfg) for _, s := range cfg.Secrets { + if err := secret.ValidateSecretName(s.Name); err != nil { + log.Logf("Warning: skipping secret with invalid name %q: %v", s.Name, err) + continue + } secretName := cfg.Namespace + s.Name // Try OAuth token first diff --git a/pkg/mcp/remote.go b/pkg/mcp/remote.go index 2bf706187..bc93b0e19 100644 --- a/pkg/mcp/remote.go +++ b/pkg/mcp/remote.go @@ -169,6 +169,10 @@ func (c *remoteMCPClient) AddRoots(roots []*mcp.Root) { } func getSecretValue(ctx context.Context, secretName string) string { + if err := secret.ValidateSecretName(secretName); err != nil { + log.Logf("Warning: skipping secret with invalid name %q: %v", secretName, err) + return "" + } fullID := secret.GetDefaultSecretKey(secretName) env, err := secret.GetSecret(ctx, fullID) if err != nil {