From bc38cfe556acc73a1000f8d6b37e77dc456529d4 Mon Sep 17 00:00:00 2001 From: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Date: Mon, 17 Nov 2025 22:29:55 +0100 Subject: [PATCH 01/15] cmd/secret: use new store (#217) * cmd/secret: use new store Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> * Add temporary secrets engine client Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> * Lint fixes + remove tests + don't return cred value * update docs --------- Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com> Co-authored-by: Saurabh Davala --- cmd/docker-mcp/backup/dump.go | 5 +- cmd/docker-mcp/commands/root.go | 2 +- cmd/docker-mcp/commands/secret.go | 125 +++++++++----- .../secret-management/secret/credstore.go | 154 ++++++------------ .../secret-management/secret/export.go | 40 ----- .../secret-management/secret/list.go | 39 ----- cmd/docker-mcp/secret-management/secret/rm.go | 36 ---- .../secret-management/secret/secret_test.go | 42 +---- .../secret-management/secret/secretsengine.go | 56 +++++++ .../secret-management/secret/set.go | 36 +--- .../reference/docker_mcp_secret.yaml | 13 +- .../reference/docker_mcp_secret_ls.yaml | 6 +- .../reference/docker_mcp_secret_rm.yaml | 4 +- .../reference/docker_mcp_secret_set.yaml | 8 +- docs/generator/reference/mcp.md | 24 +-- docs/generator/reference/mcp_secret.md | 12 +- docs/generator/reference/mcp_secret_ls.md | 2 +- docs/generator/reference/mcp_secret_rm.md | 2 +- docs/generator/reference/mcp_secret_set.md | 8 +- pkg/desktop/features.go | 15 ++ pkg/desktop/secrets.go | 6 +- 21 files changed, 265 insertions(+), 370 deletions(-) delete mode 100644 cmd/docker-mcp/secret-management/secret/export.go delete mode 100644 cmd/docker-mcp/secret-management/secret/list.go delete mode 100644 cmd/docker-mcp/secret-management/secret/rm.go create mode 100644 cmd/docker-mcp/secret-management/secret/secretsengine.go diff --git a/cmd/docker-mcp/backup/dump.go b/cmd/docker-mcp/backup/dump.go index 94f6b7cd..b22d225e 100644 --- a/cmd/docker-mcp/backup/dump.go +++ b/cmd/docker-mcp/backup/dump.go @@ -63,9 +63,8 @@ func Dump(ctx context.Context, docker docker.Client) ([]byte, error) { var secrets []desktop.Secret for _, secret := range storedSecrets { secrets = append(secrets, desktop.Secret{ - Name: secret.Name, - Provider: secret.Provider, - Value: secretValues[secret.Name], + Name: secret.Name, + Value: secretValues[secret.Name], }) } diff --git a/cmd/docker-mcp/commands/root.go b/cmd/docker-mcp/commands/root.go index 032c59d6..55936427 100644 --- a/cmd/docker-mcp/commands/root.go +++ b/cmd/docker-mcp/commands/root.go @@ -40,7 +40,6 @@ func Root(ctx context.Context, cwd string, dockerCli command.Cli) *cobra.Command HiddenDefaultCmd: true, }, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - cmd.SetContext(ctx) if err := plugin.PersistentPreRunE(cmd, args); err != nil { return err } @@ -60,6 +59,7 @@ func Root(ctx context.Context, cwd string, dockerCli command.Cli) *cobra.Command }, Version: version.Version, } + cmd.SetContext(ctx) cmd.SetVersionTemplate("{{.Version}}\n") cmd.Flags().BoolP("version", "v", false, "Print version information and quit") cmd.SetHelpTemplate(helpTemplate) diff --git a/cmd/docker-mcp/commands/secret.go b/cmd/docker-mcp/commands/secret.go index d5b7dc50..d248b34b 100644 --- a/cmd/docker-mcp/commands/secret.go +++ b/cmd/docker-mcp/commands/secret.go @@ -1,21 +1,30 @@ package commands import ( + "encoding/json" "errors" "fmt" + "slices" "strings" "github.com/spf13/cobra" + "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/formatting" "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/secret" + "github.com/docker/mcp-gateway/pkg/desktop" "github.com/docker/mcp-gateway/pkg/docker" ) const setSecretExample = ` ### Use secrets for postgres password with default policy -> docker mcp secret set POSTGRES_PASSWORD=my-secret-password -> docker run -d -l x-secret:POSTGRES_PASSWORD=/pwd.txt -e POSTGRES_PASSWORD_FILE=/pwd.txt -p 5432 postgres +> docker mcp secret set postgres_password=my-secret-password + +Inject the secret by querying by ID +> docker run -d -e POSTGRES_PASSWORD=se://docker/mcp/generic/postgres_password -p 5432 postgres + +Another way to inject secrets would be to use a pattern. +> docker run -d -e POSTGRES_PASSWORD=se://**/postgres_password -p 5432 postgres ### Pass the secret via STDIN @@ -23,55 +32,108 @@ const setSecretExample = ` > cat pwd.txt | docker mcp secret set POSTGRES_PASSWORD ` -func secretCommand(docker docker.Client) *cobra.Command { +func secretCommand(_ docker.Client) *cobra.Command { cmd := &cobra.Command{ Use: "secret", - Short: "Manage secrets", + Short: "Manage secrets in the local OS Keychain", Example: strings.Trim(setSecretExample, "\n"), + PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { + err := desktop.CheckHasDockerPass(cmd.Context()) + if err != nil { + return err + } + return nil + }, } cmd.AddCommand(rmSecretCommand()) cmd.AddCommand(listSecretCommand()) cmd.AddCommand(setSecretCommand()) - cmd.AddCommand(exportSecretCommand(docker)) return cmd } func rmSecretCommand() *cobra.Command { - var opts secret.RmOpts + var all bool cmd := &cobra.Command{ Use: "rm name1 name2 ...", - Short: "Remove secrets from Docker Desktop's secret store", + Short: "Remove secrets from the local OS Keychain", RunE: func(cmd *cobra.Command, args []string) error { - if err := validateRmArgs(args, opts); err != nil { + if err := validateRmArgs(args, all); err != nil { return err } - return secret.Remove(cmd.Context(), args, opts) + + ids := slices.Clone(args) + if all { + var err error + ids, err = secret.List(cmd.Context()) + if err != nil { + return err + } + } + + var errs []error + for _, s := range ids { + errs = append(errs, secret.DeleteSecret(cmd.Context(), s)) + } + return errors.Join(errs...) }, } flags := cmd.Flags() - flags.BoolVar(&opts.All, "all", false, "Remove all secrets") + flags.BoolVar(&all, "all", false, "Remove all secrets") return cmd } -func validateRmArgs(args []string, opts secret.RmOpts) error { - if len(args) == 0 && !opts.All { +func validateRmArgs(args []string, all bool) error { + if len(args) == 0 && !all { return errors.New("either provide a secret name or use --all to remove all secrets") } return nil } func listSecretCommand() *cobra.Command { - var opts secret.ListOptions + var outJSON bool cmd := &cobra.Command{ Use: "ls", - Short: "List all secret names in Docker Desktop's secret store", + Short: "List all secrets from the local OS Keychain as well as any active Secrets Engine provider", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { - return secret.List(cmd.Context(), opts) + // query the Secrets Engine instead to get all the secrets from + // all active providers. + l, err := secret.GetSecrets(cmd.Context()) + if err != nil { + return err + } + if outJSON { + type secretListItem struct { + ID string `json:"id"` + Provider string `json:"provider"` + } + output := make([]secretListItem, 0, len(l)) + for _, env := range l { + output = append(output, secretListItem{ + ID: env.ID, + Provider: env.Provider, + }) + } + if len(output) == 0 { + output = []secretListItem{} // Guarantee empty list (instead of displaying null) + } + jsonData, err := json.MarshalIndent(output, "", " ") + if err != nil { + return err + } + fmt.Println(string(jsonData)) + return nil + } + var rows [][]string + for _, v := range l { + rows = append(rows, []string{v.ID, v.Provider}) + } + formatting.PrettyPrintTable(rows, []int{40, 120}) + return nil }, } flags := cmd.Flags() - flags.BoolVar(&opts.JSON, "json", false, "Print as JSON.") + flags.BoolVar(&outJSON, "json", false, "Print as JSON.") return cmd } @@ -79,13 +141,10 @@ func setSecretCommand() *cobra.Command { opts := &secret.SetOpts{} cmd := &cobra.Command{ Use: "set key[=value]", - Short: "Set a secret in Docker Desktop's secret store", + Short: "Set a secret in the local OS Keychain", Example: strings.Trim(setSecretExample, "\n"), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if !secret.IsValidProvider(opts.Provider) { - return fmt.Errorf("invalid provider: %s", opts.Provider) - } var s secret.Secret if isNotImplicitReadFromStdinSyntax(args, *opts) { va, err := secret.ParseArg(args[0], *opts) @@ -105,30 +164,10 @@ func setSecretCommand() *cobra.Command { } flags := cmd.Flags() flags.StringVar(&opts.Provider, "provider", "", "Supported: credstore, oauth/") + _ = flags.MarkDeprecated("provider", "option will be ignored") return cmd } -func isNotImplicitReadFromStdinSyntax(args []string, opts secret.SetOpts) bool { - return strings.Contains(args[0], "=") || len(args) > 1 || opts.Provider != "" -} - -func exportSecretCommand(docker docker.Client) *cobra.Command { - return &cobra.Command{ - Use: "export [server1] [server2] ...", - Short: "Export secrets for the specified servers", - Hidden: true, - Args: cobra.MinimumNArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - secrets, err := secret.Export(cmd.Context(), docker, args) - if err != nil { - return err - } - - for name, secret := range secrets { - _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s=%s\n", name, secret) - } - - return nil - }, - } +func isNotImplicitReadFromStdinSyntax(args []string, _ secret.SetOpts) bool { + return strings.Contains(args[0], "=") || len(args) > 1 } diff --git a/cmd/docker-mcp/secret-management/secret/credstore.go b/cmd/docker-mcp/secret-management/secret/credstore.go index e1d3adfe..9b62da0b 100644 --- a/cmd/docker-mcp/secret-management/secret/credstore.go +++ b/cmd/docker-mcp/secret-management/secret/credstore.go @@ -1,125 +1,75 @@ +// This package stores secrets in the local OS Keychain. package secret import ( + "bufio" + "bytes" "context" - "io" + "fmt" "os/exec" + "path" "strings" - - "github.com/docker/docker-credential-helpers/client" - "github.com/docker/docker-credential-helpers/credentials" - - "github.com/docker/mcp-gateway/pkg/desktop" ) -type CredStoreProvider struct { - credentialHelper credentials.Helper -} - -func NewCredStoreProvider() *CredStoreProvider { - return &CredStoreProvider{credentialHelper: GetHelper()} -} - +type CredStoreProvider struct{} + +func cmd(ctx context.Context, args ...string) *exec.Cmd { + return exec.CommandContext(ctx, "docker", append([]string{"pass"}, args...)...) +} + +// getSecretKey prefixes the secrets with the docker/mcp/generic namespace. +// Additional namespaces can be added when defining the secretName. +// +// Example: +// +// secretName = "mysecret/application/id" +// return "docker/mcp/generic/mysecret/application/id" +// +// This can later then be queried by the Secrets Engine using a pattern or direct +// ID match. +// +// Example: +// +// # anything under mcp/generic +// pattern = "docker/mcp/generic/**" +// # specific to mysecret +// pattern = "docker/mcp/generic/mysecret/application/**" func getSecretKey(secretName string) string { - return "sm_" + secretName + return path.Join("docker/mcp/generic/", secretName) } -func (store *CredStoreProvider) GetSecret(id string) (string, error) { - _, val, err := store.credentialHelper.Get(getSecretKey(id)) +func List(ctx context.Context) ([]string, error) { + c := cmd(ctx, "ls") + out, err := c.Output() if err != nil { - return "", err + return nil, fmt.Errorf("could not list secrets: %s\n%s", bytes.TrimSpace(out), err) } - return val, nil -} - -func (store *CredStoreProvider) SetSecret(id string, value string) error { - return store.credentialHelper.Add(&credentials.Credentials{ - ServerURL: getSecretKey(id), - Username: "mcp", - Secret: value, - }) -} - -func (store *CredStoreProvider) DeleteSecret(id string) error { - return store.credentialHelper.Delete(getSecretKey(id)) -} - -func GetHelper() credentials.Helper { - credentialHelperPath := desktop.Paths().CredentialHelperPath() - return Helper{ - program: newShellProgramFunc(credentialHelperPath), - } -} - -// newShellProgramFunc creates programs that are executed in a Shell. -func newShellProgramFunc(name string) client.ProgramFunc { - return func(args ...string) client.Program { - return &shell{cmd: exec.CommandContext(context.Background(), name, args...)} + scanner := bufio.NewScanner(bytes.NewReader(out)) + var secrets []string + for scanner.Scan() { + secret := scanner.Text() + if len(secret) == 0 { + continue + } + secrets = append(secrets, secret) } + return secrets, nil } -// shell invokes shell commands to talk with a remote credentials-helper. -type shell struct { - cmd *exec.Cmd -} - -// Output returns responses from the remote credentials-helper. -func (s *shell) Output() ([]byte, error) { - return s.cmd.Output() -} - -// Input sets the input to send to a remote credentials-helper. -func (s *shell) Input(in io.Reader) { - s.cmd.Stdin = in -} - -// Helper wraps credential helper program. -type Helper struct { - // name string - program client.ProgramFunc -} - -func (h Helper) List() (map[string]string, error) { - return map[string]string{}, nil -} - -// Add stores new credentials. -func (h Helper) Add(creds *credentials.Credentials) error { - username, secret, err := h.Get(creds.ServerURL) - if err != nil && !credentials.IsErrCredentialsNotFound(err) && !isErrDecryption(err) { - return err - } - if username == creds.Username && secret == creds.Secret { - return nil - } - if err := client.Store(h.program, creds); err != nil { - return err +func setSecret(ctx context.Context, id string, value string) error { + c := cmd(ctx, "set", getSecretKey(id)) + c.Stdin = strings.NewReader(value) + out, err := c.CombinedOutput() + if err != nil { + return fmt.Errorf("could not store secret: %s\n%s", bytes.TrimSpace(out), err) } return nil } -// Delete removes credentials. -func (h Helper) Delete(serverURL string) error { - if _, _, err := h.Get(serverURL); err != nil { - if credentials.IsErrCredentialsNotFound(err) { - return nil - } - return err - } - return client.Erase(h.program, serverURL) -} - -// Get returns the username and secret to use for a given registry server URL. -func (h Helper) Get(serverURL string) (string, string, error) { - creds, err := client.Get(h.program, serverURL) +func DeleteSecret(ctx context.Context, id string) error { + out, err := cmd(ctx, "rm", getSecretKey(id)).CombinedOutput() if err != nil { - return "", "", err + return fmt.Errorf("could not delete secret: %s\n%s\n%s", id, bytes.TrimSpace(out), err) } - return creds.Username, creds.Secret, nil -} - -func isErrDecryption(err error) bool { - return err != nil && strings.Contains(err.Error(), "gpg: decryption failed: No secret key") + return nil } - -var _ credentials.Helper = Helper{} diff --git a/cmd/docker-mcp/secret-management/secret/export.go b/cmd/docker-mcp/secret-management/secret/export.go deleted file mode 100644 index 3c7c271c..00000000 --- a/cmd/docker-mcp/secret-management/secret/export.go +++ /dev/null @@ -1,40 +0,0 @@ -package secret - -import ( - "context" - "fmt" - "sort" - "strings" - - "github.com/docker/mcp-gateway/pkg/catalog" - "github.com/docker/mcp-gateway/pkg/docker" -) - -func Export(ctx context.Context, docker docker.Client, serverNames []string) (map[string]string, error) { - catalog, err := catalog.Get(ctx) - if err != nil { - return nil, err - } - - var secretNames []string - for _, serverName := range serverNames { - serverName = strings.TrimSpace(serverName) - - serverSpec, ok := catalog.Servers[serverName] - if !ok { - return nil, fmt.Errorf("server %s not found in catalog", serverName) - } - - for _, s := range serverSpec.Secrets { - secretNames = append(secretNames, s.Name) - } - } - - if len(secretNames) == 0 { - return map[string]string{}, nil - } - - sort.Strings(secretNames) - - return docker.ReadSecrets(ctx, secretNames, false) -} diff --git a/cmd/docker-mcp/secret-management/secret/list.go b/cmd/docker-mcp/secret-management/secret/list.go deleted file mode 100644 index 3893c3a4..00000000 --- a/cmd/docker-mcp/secret-management/secret/list.go +++ /dev/null @@ -1,39 +0,0 @@ -package secret - -import ( - "context" - "encoding/json" - "fmt" - - "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/formatting" - "github.com/docker/mcp-gateway/pkg/desktop" -) - -type ListOptions struct { - JSON bool -} - -func List(ctx context.Context, opts ListOptions) error { - l, err := desktop.NewSecretsClient().ListJfsSecrets(ctx) - if err != nil { - return err - } - - if opts.JSON { - if len(l) == 0 { - l = []desktop.StoredSecret{} // Guarantee empty list (instead of displaying null) - } - jsonData, err := json.MarshalIndent(l, "", " ") - if err != nil { - return err - } - fmt.Println(string(jsonData)) - return nil - } - var rows [][]string - for _, v := range l { - rows = append(rows, []string{v.Name, v.Provider}) - } - formatting.PrettyPrintTable(rows, []int{40, 120}) - return nil -} diff --git a/cmd/docker-mcp/secret-management/secret/rm.go b/cmd/docker-mcp/secret-management/secret/rm.go deleted file mode 100644 index 1e6e78d2..00000000 --- a/cmd/docker-mcp/secret-management/secret/rm.go +++ /dev/null @@ -1,36 +0,0 @@ -package secret - -import ( - "context" - "errors" - "fmt" - - "github.com/docker/mcp-gateway/pkg/desktop" -) - -type RmOpts struct { - All bool -} - -func Remove(ctx context.Context, names []string, opts RmOpts) error { - c := desktop.NewSecretsClient() - if opts.All && len(names) == 0 { - l, err := c.ListJfsSecrets(ctx) - if err != nil { - return err - } - for _, secret := range l { - names = append(names, secret.Name) - } - } - var errs []error - for _, name := range names { - if err := c.DeleteJfsSecret(ctx, name); err != nil { - errs = append(errs, err) - fmt.Printf("failed removing secret %s\n", name) - continue - } - fmt.Printf("removed secret %s\n", name) - } - return errors.Join(errs...) -} diff --git a/cmd/docker-mcp/secret-management/secret/secret_test.go b/cmd/docker-mcp/secret-management/secret/secret_test.go index 24577d8c..6755e931 100644 --- a/cmd/docker-mcp/secret-management/secret/secret_test.go +++ b/cmd/docker-mcp/secret-management/secret/secret_test.go @@ -1,7 +1,6 @@ package secret import ( - "errors" "testing" "github.com/stretchr/testify/assert" @@ -10,25 +9,19 @@ import ( func TestGetSecretKey(t *testing.T) { result := getSecretKey("mykey") - assert.Equal(t, "sm_mykey", result) + assert.Equal(t, "docker/mcp/generic/mykey", result) } func TestParseArg(t *testing.T) { // Test key=value parsing - secret, err := ParseArg("key=value", SetOpts{Provider: Credstore}) + secret, err := ParseArg("key=value", SetOpts{}) require.NoError(t, err) assert.Equal(t, "key", secret.key) assert.Equal(t, "value", secret.val) - // Test key-only for non-direct providers - secret, err = ParseArg("keyname", SetOpts{Provider: "oauth/github"}) - require.NoError(t, err) - assert.Equal(t, "keyname", secret.key) - assert.Empty(t, secret.val) - - // Test error on key=value with non-direct provider - _, err = ParseArg("key=value", SetOpts{Provider: "oauth/github"}) - assert.Error(t, err) + // Test invalid format (no = sign) + _, err = ParseArg("just-a-key", SetOpts{}) + assert.Error(t, err, "should error when no = sign is present") } func TestIsDirectValueProvider(t *testing.T) { @@ -36,28 +29,3 @@ func TestIsDirectValueProvider(t *testing.T) { assert.True(t, isDirectValueProvider(Credstore)) assert.False(t, isDirectValueProvider("oauth/github")) } - -func TestIsValidProvider(t *testing.T) { - // Valid providers - assert.True(t, IsValidProvider("")) - assert.True(t, IsValidProvider(Credstore)) - assert.True(t, IsValidProvider("oauth/github")) - assert.True(t, IsValidProvider("oauth/google")) - - // Invalid providers - assert.False(t, IsValidProvider("invalid")) - assert.False(t, IsValidProvider("oauth")) -} - -func TestIsErrDecryption(t *testing.T) { - // Test decryption error detection - decryptErr := errors.New("gpg: decryption failed: No secret key") - assert.True(t, isErrDecryption(decryptErr)) - - // Test other errors - otherErr := errors.New("some other error") - assert.False(t, isErrDecryption(otherErr)) - - // Test nil - assert.False(t, isErrDecryption(nil)) -} diff --git a/cmd/docker-mcp/secret-management/secret/secretsengine.go b/cmd/docker-mcp/secret-management/secret/secretsengine.go new file mode 100644 index 00000000..e0cfc4f5 --- /dev/null +++ b/cmd/docker-mcp/secret-management/secret/secretsengine.go @@ -0,0 +1,56 @@ +package secret + +import ( + "bytes" + "context" + "encoding/json" + "net" + "net/http" + "os" + "path/filepath" +) + +type Envelope struct { + ID string `json:"id"` + Value []byte `json:"value"` + Provider string `json:"provider"` +} + +func socketPath() string { + if dir, err := os.UserCacheDir(); err == nil { + return filepath.Join(dir, "docker-secrets-engine", "engine.sock") + } + return filepath.Join(os.TempDir(), "docker-secrets-engine", "engine.sock") +} + +// This is a temporary Client and should be removed once we have the secrets engine +// client SDK imported. +var c = http.Client{ + Transport: &http.Transport{ + DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) { + d := &net.Dialer{} + return d.DialContext(ctx, "unix", socketPath()) + }, + }, +} + +func GetSecrets(ctx context.Context) ([]Envelope, error) { + pattern := []byte(`{"pattern": "docker/mcp/**"}`) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, "http://localhost/resolver.v1.ResolverService/GetSecrets", bytes.NewReader(pattern)) + if err != nil { + return nil, err + } + req.Header.Add("Content-Type", "application/json") + + resp, err := c.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + var secrets map[string][]Envelope + if err := json.NewDecoder(resp.Body).Decode(&secrets); err != nil { + return nil, err + } + return secrets["envelopes"], nil +} diff --git a/cmd/docker-mcp/secret-management/secret/set.go b/cmd/docker-mcp/secret-management/secret/set.go index db5847fa..98d0a21f 100644 --- a/cmd/docker-mcp/secret-management/secret/set.go +++ b/cmd/docker-mcp/secret-management/secret/set.go @@ -6,15 +6,19 @@ import ( "os" "strings" - "github.com/docker/mcp-gateway/pkg/desktop" "github.com/docker/mcp-gateway/pkg/tui" ) const ( + // Specify to use the credential store provider. + // + // Deprecated: Not used. Credstore = "credstore" ) type SetOpts struct { + // Provider sets the store provider + // Deprecated: this field will be removed in the next release Provider string } @@ -36,9 +40,6 @@ type Secret struct { } func ParseArg(arg string, opts SetOpts) (*Secret, error) { - if !isDirectValueProvider(opts.Provider) && strings.Contains(arg, "=") { - return nil, fmt.Errorf("provider cannot be used with key=value pairs: %s", arg) - } if !isDirectValueProvider(opts.Provider) { return &Secret{key: arg, val: ""}, nil } @@ -53,29 +54,6 @@ func isDirectValueProvider(provider string) bool { return provider == "" || provider == Credstore } -func Set(ctx context.Context, s Secret, opts SetOpts) error { - if opts.Provider == Credstore { - p := NewCredStoreProvider() - if err := p.SetSecret(s.key, s.val); err != nil { - return err - } - } - return desktop.NewSecretsClient().SetJfsSecret(ctx, desktop.Secret{ - Name: s.key, - Value: s.val, - Provider: opts.Provider, - }) -} - -func IsValidProvider(provider string) bool { - if provider == "" { - return true - } - if strings.HasPrefix(provider, "oauth/") { - return true - } - if provider == Credstore { - return true - } - return false +func Set(ctx context.Context, s Secret, _ SetOpts) error { + return setSecret(ctx, s.key, s.val) } diff --git a/docs/generator/reference/docker_mcp_secret.yaml b/docs/generator/reference/docker_mcp_secret.yaml index c10fdaf6..963f7282 100644 --- a/docs/generator/reference/docker_mcp_secret.yaml +++ b/docs/generator/reference/docker_mcp_secret.yaml @@ -1,6 +1,6 @@ command: docker mcp secret -short: Manage secrets -long: Manage secrets +short: Manage secrets in the local OS Keychain +long: Manage secrets in the local OS Keychain pname: docker mcp plink: docker_mcp.yaml cname: @@ -14,8 +14,13 @@ clink: examples: |- ### Use secrets for postgres password with default policy - > docker mcp secret set POSTGRES_PASSWORD=my-secret-password - > docker run -d -l x-secret:POSTGRES_PASSWORD=/pwd.txt -e POSTGRES_PASSWORD_FILE=/pwd.txt -p 5432 postgres + > docker mcp secret set postgres_password=my-secret-password + + Inject the secret by querying by ID + > docker run -d -e POSTGRES_PASSWORD=se://docker/mcp/generic/postgres_password -p 5432 postgres + + Another way to inject secrets would be to use a pattern. + > docker run -d -e POSTGRES_PASSWORD=se://**/postgres_password -p 5432 postgres ### Pass the secret via STDIN diff --git a/docs/generator/reference/docker_mcp_secret_ls.yaml b/docs/generator/reference/docker_mcp_secret_ls.yaml index b3f8545e..de4eeb5f 100644 --- a/docs/generator/reference/docker_mcp_secret_ls.yaml +++ b/docs/generator/reference/docker_mcp_secret_ls.yaml @@ -1,6 +1,8 @@ command: docker mcp secret ls -short: List all secret names in Docker Desktop's secret store -long: List all secret names in Docker Desktop's secret store +short: | + List all secrets from the local OS Keychain as well as any active Secrets Engine provider +long: | + List all secrets from the local OS Keychain as well as any active Secrets Engine provider usage: docker mcp secret ls pname: docker mcp secret plink: docker_mcp_secret.yaml diff --git a/docs/generator/reference/docker_mcp_secret_rm.yaml b/docs/generator/reference/docker_mcp_secret_rm.yaml index 89541dfa..59728b99 100644 --- a/docs/generator/reference/docker_mcp_secret_rm.yaml +++ b/docs/generator/reference/docker_mcp_secret_rm.yaml @@ -1,6 +1,6 @@ command: docker mcp secret rm -short: Remove secrets from Docker Desktop's secret store -long: Remove secrets from Docker Desktop's secret store +short: Remove secrets from the local OS Keychain +long: Remove secrets from the local OS Keychain usage: docker mcp secret rm name1 name2 ... pname: docker mcp secret plink: docker_mcp_secret.yaml diff --git a/docs/generator/reference/docker_mcp_secret_set.yaml b/docs/generator/reference/docker_mcp_secret_set.yaml index c3e8b4ab..7aede822 100644 --- a/docs/generator/reference/docker_mcp_secret_set.yaml +++ b/docs/generator/reference/docker_mcp_secret_set.yaml @@ -1,6 +1,6 @@ command: docker mcp secret set -short: Set a secret in Docker Desktop's secret store -long: Set a secret in Docker Desktop's secret store +short: Set a secret in the local OS Keychain +long: Set a secret in the local OS Keychain usage: docker mcp secret set key[=value] pname: docker mcp secret plink: docker_mcp_secret.yaml @@ -8,8 +8,8 @@ options: - option: provider value_type: string description: 'Supported: credstore, oauth/' - deprecated: false - hidden: false + deprecated: true + hidden: true experimental: false experimentalcli: false kubernetes: false diff --git a/docs/generator/reference/mcp.md b/docs/generator/reference/mcp.md index 4cdac747..94d00b6f 100644 --- a/docs/generator/reference/mcp.md +++ b/docs/generator/reference/mcp.md @@ -5,18 +5,18 @@ Manage MCP servers and clients ### Subcommands -| Name | Description | -|:----------------------------|:------------------------------| -| [`catalog`](mcp_catalog.md) | Manage MCP server catalogs | -| [`client`](mcp_client.md) | Manage MCP clients | -| [`config`](mcp_config.md) | Manage the configuration | -| [`feature`](mcp_feature.md) | Manage experimental features | -| [`gateway`](mcp_gateway.md) | Manage the MCP Server gateway | -| [`policy`](mcp_policy.md) | Manage secret policies | -| [`secret`](mcp_secret.md) | Manage secrets | -| [`server`](mcp_server.md) | Manage servers | -| [`tools`](mcp_tools.md) | Manage tools | -| [`version`](mcp_version.md) | Show the version information | +| Name | Description | +|:----------------------------|:----------------------------------------| +| [`catalog`](mcp_catalog.md) | Manage MCP server catalogs | +| [`client`](mcp_client.md) | Manage MCP clients | +| [`config`](mcp_config.md) | Manage the configuration | +| [`feature`](mcp_feature.md) | Manage experimental features | +| [`gateway`](mcp_gateway.md) | Manage the MCP Server gateway | +| [`policy`](mcp_policy.md) | Manage secret policies | +| [`secret`](mcp_secret.md) | Manage secrets in the local OS Keychain | +| [`server`](mcp_server.md) | Manage servers | +| [`tools`](mcp_tools.md) | Manage tools | +| [`version`](mcp_version.md) | Show the version information | ### Options diff --git a/docs/generator/reference/mcp_secret.md b/docs/generator/reference/mcp_secret.md index e5e1b99b..2f4f03ba 100644 --- a/docs/generator/reference/mcp_secret.md +++ b/docs/generator/reference/mcp_secret.md @@ -1,15 +1,15 @@ # docker mcp secret -Manage secrets +Manage secrets in the local OS Keychain ### Subcommands -| Name | Description | -|:---------------------------|:-------------------------------------------------------| -| [`ls`](mcp_secret_ls.md) | List all secret names in Docker Desktop's secret store | -| [`rm`](mcp_secret_rm.md) | Remove secrets from Docker Desktop's secret store | -| [`set`](mcp_secret_set.md) | Set a secret in Docker Desktop's secret store | +| Name | Description | +|:---------------------------|:------------------------------------------------------------------------------------------| +| [`ls`](mcp_secret_ls.md) | List all secrets from the local OS Keychain as well as any active Secrets Engine provider | +| [`rm`](mcp_secret_rm.md) | Remove secrets from the local OS Keychain | +| [`set`](mcp_secret_set.md) | Set a secret in the local OS Keychain | diff --git a/docs/generator/reference/mcp_secret_ls.md b/docs/generator/reference/mcp_secret_ls.md index 327bab6c..8b9b1094 100644 --- a/docs/generator/reference/mcp_secret_ls.md +++ b/docs/generator/reference/mcp_secret_ls.md @@ -1,7 +1,7 @@ # docker mcp secret ls -List all secret names in Docker Desktop's secret store +List all secrets from the local OS Keychain as well as any active Secrets Engine provider ### Options diff --git a/docs/generator/reference/mcp_secret_rm.md b/docs/generator/reference/mcp_secret_rm.md index b95de299..37ec70d4 100644 --- a/docs/generator/reference/mcp_secret_rm.md +++ b/docs/generator/reference/mcp_secret_rm.md @@ -1,7 +1,7 @@ # docker mcp secret rm -Remove secrets from Docker Desktop's secret store +Remove secrets from the local OS Keychain ### Options diff --git a/docs/generator/reference/mcp_secret_set.md b/docs/generator/reference/mcp_secret_set.md index 228f2281..aba25e20 100644 --- a/docs/generator/reference/mcp_secret_set.md +++ b/docs/generator/reference/mcp_secret_set.md @@ -1,13 +1,7 @@ # docker mcp secret set -Set a secret in Docker Desktop's secret store - -### Options - -| Name | Type | Default | Description | -|:-------------|:---------|:--------|:---------------------------------------| -| `--provider` | `string` | | Supported: credstore, oauth/ | +Set a secret in the local OS Keychain diff --git a/pkg/desktop/features.go b/pkg/desktop/features.go index f4839892..b0bd56a7 100644 --- a/pkg/desktop/features.go +++ b/pkg/desktop/features.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "os" + "os/exec" "github.com/PaesslerAG/jsonpath" ) @@ -44,6 +45,20 @@ func CheckFeatureIsEnabled(ctx context.Context, settingName string, label string return nil } +var ErrDockerPassUnsupported = errors.New("docker pass has not been installed") + +func CheckHasDockerPass(ctx context.Context) error { + err := exec.CommandContext(ctx, "docker", "pass").Run() + execStatus, ok := err.(*exec.ExitError) + if !ok { + return err + } + if execStatus.ExitCode() > 0 { + return ErrDockerPassUnsupported + } + return nil +} + func getAdminSettings() (map[string]any, error) { buf, err := os.ReadFile(Paths().AdminSettingPath) if err != nil { diff --git a/pkg/desktop/secrets.go b/pkg/desktop/secrets.go index b53befa7..ce57f58d 100644 --- a/pkg/desktop/secrets.go +++ b/pkg/desktop/secrets.go @@ -6,7 +6,11 @@ import ( ) type StoredSecret struct { - Name string `json:"name"` + Name string `json:"name"` + // Provider specifies the provider under which the secret is stored + // + // Deprecated: will be removed in a future release and is not currently + // used by default Provider string `json:"provider,omitempty"` } From 915424255c7b5ceb67cdc8284e3e11e54b26bcab Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Mon, 17 Nov 2025 16:47:33 -0800 Subject: [PATCH 02/15] inject secrets by ID + return empty secrets map --- pkg/gateway/clientpool.go | 10 ++--- pkg/gateway/configuration.go | 60 +++---------------------- pkg/gateway/configuration_workingset.go | 30 +------------ pkg/gateway/mcpadd.go | 22 +-------- 4 files changed, 12 insertions(+), 110 deletions(-) diff --git a/pkg/gateway/clientpool.go b/pkg/gateway/clientpool.go index bac0eda9..57576aeb 100644 --- a/pkg/gateway/clientpool.go +++ b/pkg/gateway/clientpool.go @@ -322,13 +322,9 @@ func (cp *clientPool) argsAndEnv(serverConfig *catalog.ServerConfig, readOnly *b for _, s := range serverConfig.Spec.Secrets { args = append(args, "-e", s.Env) - secretValue, ok := serverConfig.Secrets[s.Name] - if ok { - env = append(env, fmt.Sprintf("%s=%s", s.Env, secretValue)) - } else { - log.Logf("Warning: Secret '%s' not found for server '%s', setting %s=", s.Name, serverConfig.Name, s.Env) - env = append(env, fmt.Sprintf("%s=%s", s.Env, "")) - } + // Build se:// URI with full namespace path + secretURI := fmt.Sprintf("se://docker/mcp/generic/%s", s.Name) + env = append(env, fmt.Sprintf("%s=%s", s.Env, secretURI)) } // Env diff --git a/pkg/gateway/configuration.go b/pkg/gateway/configuration.go index 5d7c57ba..076ac016 100644 --- a/pkg/gateway/configuration.go +++ b/pkg/gateway/configuration.go @@ -372,29 +372,10 @@ func (c *FileBasedConfiguration) readOnce(ctx context.Context) (Configuration, e return Configuration{}, fmt.Errorf("reading tools: %w", err) } + // Secrets no longer read during configuration load + // Instead, se:// URIs are passed to containers and resolved at runtime var secrets map[string]string - if c.SecretsPath == "docker-desktop" { - secrets, err = c.readDockerDesktopSecrets(ctx, servers, serverNames) - if err != nil { - return Configuration{}, fmt.Errorf("reading MCP Toolkit's secrets: %w", err) - } - } else { - // Unless SecretsPath is only `docker-desktop`, we don't fail if secrets can't be read. - // It's ok for the MCP tookit's to not be available (in Cloud Run, for example). - // It's ok for secrets .env file to not exist. - var err error - for secretPath := range strings.SplitSeq(c.SecretsPath, ":") { - if secretPath == "docker-desktop" { - secrets, err = c.readDockerDesktopSecrets(ctx, servers, serverNames) - } else { - secrets, err = c.readSecretsFromFile(ctx, secretPath) - } - - if err == nil { - break - } - } - } + secrets = make(map[string]string) log.Log("- Configuration read in", time.Since(start)) return Configuration{ @@ -521,39 +502,8 @@ func (c *FileBasedConfiguration) readToolsConfig(ctx context.Context) (config.To } func (c *FileBasedConfiguration) readDockerDesktopSecrets(ctx context.Context, servers map[string]catalog.Server, serverNames []string) (map[string]string, error) { - // Use a map to deduplicate secret names - uniqueSecretNames := make(map[string]struct{}) - - for _, serverName := range serverNames { - serverName := strings.TrimSpace(serverName) - - serverSpec, ok := servers[serverName] - if !ok { - continue - } - - for _, s := range serverSpec.Secrets { - uniqueSecretNames[s.Name] = struct{}{} - } - } - - if len(uniqueSecretNames) == 0 { - return map[string]string{}, nil - } - - // Convert map keys to slice - var secretNames []string - for name := range uniqueSecretNames { - secretNames = append(secretNames, name) - } - - log.Log(" - Reading secrets", secretNames) - secretsByName, err := c.docker.ReadSecrets(ctx, secretNames, true) - if err != nil { - return nil, fmt.Errorf("finding secrets %s: %w", secretNames, err) - } - - return secretsByName, nil + // Secrets no longer read - se:// URIs passed to containers and resolved at runtime + return map[string]string{}, nil } func (c *FileBasedConfiguration) readSecretsFromFile(ctx context.Context, path string) (map[string]string, error) { diff --git a/pkg/gateway/configuration_workingset.go b/pkg/gateway/configuration_workingset.go index fdc9baf9..609c5c6c 100644 --- a/pkg/gateway/configuration_workingset.go +++ b/pkg/gateway/configuration_workingset.go @@ -158,34 +158,8 @@ func (c *WorkingSetConfiguration) readSecrets(ctx context.Context, workingSet wo } func (c *WorkingSetConfiguration) readDockerDesktopSecrets(ctx context.Context, servers []workingset.Server) (map[string]string, error) { - // Use a map to deduplicate secret names - uniqueSecretNames := make(map[string]struct{}) - - for _, server := range servers { - serverSpec := server.Snapshot.Server - - for _, s := range serverSpec.Secrets { - uniqueSecretNames[s.Name] = struct{}{} - } - } - - if len(uniqueSecretNames) == 0 { - return map[string]string{}, nil - } - - // Convert map keys to slice - var secretNames []string - for name := range uniqueSecretNames { - secretNames = append(secretNames, name) - } - - log.Log(" - Reading secrets from Docker Desktop", secretNames) - secretsByName, err := c.docker.ReadSecrets(ctx, secretNames, true) - if err != nil { - return nil, fmt.Errorf("finding secrets %s: %w", secretNames, err) - } - - return secretsByName, nil + // Secrets no longer read - se:// URIs passed to containers and resolved at runtime + return map[string]string{}, nil } func getServersUsingProvider(workingSet workingset.WorkingSet, providerRef string) []workingset.Server { diff --git a/pkg/gateway/mcpadd.go b/pkg/gateway/mcpadd.go index b697fb3d..019dc308 100644 --- a/pkg/gateway/mcpadd.go +++ b/pkg/gateway/mcpadd.go @@ -84,27 +84,9 @@ func (g *Gateway) createMcpAddTool(clientConfig *clientConfig) *ToolRegistration g.configuration.serverNames = append(g.configuration.serverNames, serverName) } - // Fetch updated secrets for the new server list - if g.configurator != nil { - if fbc, ok := g.configurator.(*FileBasedConfiguration); ok { - updatedSecrets, err := fbc.readDockerDesktopSecrets(ctx, g.configuration.servers, g.configuration.serverNames) - if err == nil { - g.configuration.secrets = updatedSecrets - } else { - log.Log("Warning: Failed to update secrets:", err) - } - } - } - - // Check if all required secrets are set + // Secrets validation removed - se:// URIs are resolved at runtime by Docker Desktop + // Containers will fail to start if secrets are missing in the secrets engine var missingSecrets []string - if serverConfig != nil { - for _, secret := range serverConfig.Spec.Secrets { - if value, exists := g.configuration.secrets[secret.Name]; !exists || value == "" { - missingSecrets = append(missingSecrets, secret.Name) - } - } - } // Check if all required config values are set and validate against schema var missingConfig []string From 49d560d3f2be9ab380ae2c14ad0059a3d06f00e7 Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Mon, 17 Nov 2025 17:53:49 -0800 Subject: [PATCH 03/15] Remove jfs references and commnds which utilize jfs like: dump, restore --- cmd/docker-mcp/backup/dump.go | 31 ----- cmd/docker-mcp/backup/restore.go | 34 +----- cmd/docker-mcp/backup/types.go | 4 - cmd/docker-mcp/commands/policy.go | 58 --------- cmd/docker-mcp/commands/root.go | 1 - .../secret-management/policy/dump.go | 18 --- .../secret-management/policy/set.go | 11 -- cmd/docker-mcp/server/secret_config.go | 12 +- pkg/desktop/connection_other.go | 4 - pkg/desktop/secrets.go | 65 ---------- pkg/docker/client.go | 1 - pkg/docker/secrets.go | 112 ------------------ pkg/gateway/clientpool_test.go | 6 +- pkg/gateway/configuration.go | 46 +------ pkg/gateway/configuration_workingset.go | 2 +- 15 files changed, 13 insertions(+), 392 deletions(-) delete mode 100644 cmd/docker-mcp/commands/policy.go delete mode 100644 cmd/docker-mcp/secret-management/policy/dump.go delete mode 100644 cmd/docker-mcp/secret-management/policy/set.go delete mode 100644 pkg/desktop/secrets.go delete mode 100644 pkg/docker/secrets.go diff --git a/cmd/docker-mcp/backup/dump.go b/cmd/docker-mcp/backup/dump.go index b22d225e..4f89ded2 100644 --- a/cmd/docker-mcp/backup/dump.go +++ b/cmd/docker-mcp/backup/dump.go @@ -6,7 +6,6 @@ import ( "github.com/docker/mcp-gateway/cmd/docker-mcp/catalog" "github.com/docker/mcp-gateway/pkg/config" - "github.com/docker/mcp-gateway/pkg/desktop" "github.com/docker/mcp-gateway/pkg/docker" ) @@ -45,42 +44,12 @@ func Dump(ctx context.Context, docker docker.Client) ([]byte, error) { catalogFiles[name] = string(catalogFileContent) } - secretsClient := desktop.NewSecretsClient() - storedSecrets, err := secretsClient.ListJfsSecrets(ctx) - if err != nil { - return nil, err - } - - var secretNames []string - for _, secret := range storedSecrets { - secretNames = append(secretNames, secret.Name) - } - secretValues, err := docker.ReadSecrets(ctx, secretNames, false) - if err != nil { - return nil, err - } - - var secrets []desktop.Secret - for _, secret := range storedSecrets { - secrets = append(secrets, desktop.Secret{ - Name: secret.Name, - Value: secretValues[secret.Name], - }) - } - - policy, err := secretsClient.GetJfsPolicy(ctx) - if err != nil { - return nil, err - } - backup := Backup{ Config: string(configContent), Registry: string(registryContent), Catalog: string(catalogContent), CatalogFiles: catalogFiles, Tools: string(toolsConfig), - Secrets: secrets, - Policy: policy, } return json.Marshal(backup) diff --git a/cmd/docker-mcp/backup/restore.go b/cmd/docker-mcp/backup/restore.go index 6d0280a0..12065efc 100644 --- a/cmd/docker-mcp/backup/restore.go +++ b/cmd/docker-mcp/backup/restore.go @@ -6,10 +6,9 @@ import ( "github.com/docker/mcp-gateway/cmd/docker-mcp/catalog" "github.com/docker/mcp-gateway/pkg/config" - "github.com/docker/mcp-gateway/pkg/desktop" ) -func Restore(ctx context.Context, backupData []byte) error { +func Restore(_ context.Context, backupData []byte) error { var backup Backup if err := json.Unmarshal(backupData, &backup); err != nil { return err @@ -50,36 +49,5 @@ func Restore(ctx context.Context, backupData []byte) error { } } - secretsClient := desktop.NewSecretsClient() - - secretsBefore, err := secretsClient.ListJfsSecrets(ctx) - if err != nil { - return err - } - - secretsKeep := map[string]bool{} - for _, secret := range backup.Secrets { - if err := secretsClient.SetJfsSecret(ctx, desktop.Secret{ - Name: secret.Name, - Value: secret.Value, - Provider: secret.Provider, - }); err != nil { - return err - } - secretsKeep[secret.Name] = true - } - - for _, secret := range secretsBefore { - if !secretsKeep[secret.Name] { - if err := secretsClient.DeleteJfsSecret(ctx, secret.Name); err != nil { - return err - } - } - } - - if err := secretsClient.SetJfsPolicy(ctx, backup.Policy); err != nil { - return err - } - return nil } diff --git a/cmd/docker-mcp/backup/types.go b/cmd/docker-mcp/backup/types.go index 79e223e8..c34b9223 100644 --- a/cmd/docker-mcp/backup/types.go +++ b/cmd/docker-mcp/backup/types.go @@ -1,13 +1,9 @@ package backup -import "github.com/docker/mcp-gateway/pkg/desktop" - type Backup struct { Config string `json:"config"` Registry string `json:"registry"` Catalog string `json:"catalog"` CatalogFiles map[string]string `json:"catalogFiles"` Tools string `json:"tools"` - Secrets []desktop.Secret `json:"secrets"` - Policy string `json:"policy"` } diff --git a/cmd/docker-mcp/commands/policy.go b/cmd/docker-mcp/commands/policy.go deleted file mode 100644 index b5713757..00000000 --- a/cmd/docker-mcp/commands/policy.go +++ /dev/null @@ -1,58 +0,0 @@ -package commands - -import ( - "os" - "strings" - - "github.com/spf13/cobra" - - "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/policy" - "github.com/docker/mcp-gateway/pkg/tui" -) - -const setPolicyExample = ` -### Backup the current policy to a file -docker mcp policy dump > policy.conf - -### Set a new policy -docker mcp policy set "my-secret allows postgres" - -### Restore the previous policy -cat policy.conf | docker mcp policy set -` - -func policyCommand() *cobra.Command { - cmd := &cobra.Command{ - Use: "policy", - Aliases: []string{"policies"}, - Short: "Manage secret policies", - } - - cmd.AddCommand(&cobra.Command{ - Use: "set ", - Short: "Set a policy for secret management in Docker Desktop", - Args: cobra.MaximumNArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - bytes, err := tui.ReadAllWithContext(cmd.Context(), os.Stdin) - if err != nil { - return err - } - args = append(args, string(bytes)) - } - return policy.Set(cmd.Context(), args[0]) - }, - Example: strings.Trim(setPolicyExample, "\n"), - }) - - cmd.AddCommand(&cobra.Command{ - Use: "dump", - Short: "Dump the policy content", - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { - return policy.Dump(cmd.Context()) - }, - }) - - return cmd -} diff --git a/cmd/docker-mcp/commands/root.go b/cmd/docker-mcp/commands/root.go index 55936427..de06ec8a 100644 --- a/cmd/docker-mcp/commands/root.go +++ b/cmd/docker-mcp/commands/root.go @@ -80,7 +80,6 @@ func Root(ctx context.Context, cwd string, dockerCli command.Cli) *cobra.Command cmd.AddCommand(featureCommand(dockerCli)) cmd.AddCommand(gatewayCommand(dockerClient, dockerCli)) cmd.AddCommand(oauthCommand()) - cmd.AddCommand(policyCommand()) cmd.AddCommand(registryCommand()) cmd.AddCommand(secretCommand(dockerClient)) cmd.AddCommand(serverCommand(dockerClient, dockerCli)) diff --git a/cmd/docker-mcp/secret-management/policy/dump.go b/cmd/docker-mcp/secret-management/policy/dump.go deleted file mode 100644 index 9d843990..00000000 --- a/cmd/docker-mcp/secret-management/policy/dump.go +++ /dev/null @@ -1,18 +0,0 @@ -package policy - -import ( - "context" - "fmt" - - "github.com/docker/mcp-gateway/pkg/desktop" -) - -func Dump(ctx context.Context) error { - l, err := desktop.NewSecretsClient().GetJfsPolicy(ctx) - if err != nil { - return err - } - - fmt.Println(l) - return nil -} diff --git a/cmd/docker-mcp/secret-management/policy/set.go b/cmd/docker-mcp/secret-management/policy/set.go deleted file mode 100644 index e9a7a2ea..00000000 --- a/cmd/docker-mcp/secret-management/policy/set.go +++ /dev/null @@ -1,11 +0,0 @@ -package policy - -import ( - "context" - - "github.com/docker/mcp-gateway/pkg/desktop" -) - -func Set(ctx context.Context, data string) error { - return desktop.NewSecretsClient().SetJfsPolicy(ctx, data) -} diff --git a/cmd/docker-mcp/server/secret_config.go b/cmd/docker-mcp/server/secret_config.go index 3efcbc3f..de379573 100644 --- a/cmd/docker-mcp/server/secret_config.go +++ b/cmd/docker-mcp/server/secret_config.go @@ -2,22 +2,24 @@ package server import ( "context" + "strings" - "github.com/docker/mcp-gateway/pkg/desktop" + "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/secret" ) // getConfiguredSecretNames returns a map of configured secret names for quick lookup. // This is a shared helper used by both ls.go and enable.go. func getConfiguredSecretNames(ctx context.Context) (map[string]struct{}, error) { - secretsClient := desktop.NewSecretsClient() - configuredSecrets, err := secretsClient.ListJfsSecrets(ctx) + envelopes, err := secret.GetSecrets(ctx) if err != nil { return nil, err } configuredSecretNames := make(map[string]struct{}) - for _, secret := range configuredSecrets { - configuredSecretNames[secret.Name] = struct{}{} + for _, env := range envelopes { + // Extract base name from full ID (e.g., "docker/mcp/generic/brave.api_key" → "brave.api_key") + name := strings.TrimPrefix(env.ID, "docker/mcp/generic/") + configuredSecretNames[name] = struct{}{} } return configuredSecretNames, nil diff --git a/pkg/desktop/connection_other.go b/pkg/desktop/connection_other.go index 9c6533b9..afc9f3ae 100644 --- a/pkg/desktop/connection_other.go +++ b/pkg/desktop/connection_other.go @@ -16,10 +16,6 @@ func dialAuth(ctx context.Context) (net.Conn, error) { return dial(ctx, Paths().ToolsSocket) } -func dialSecrets(ctx context.Context) (net.Conn, error) { - return dial(ctx, Paths().JFSSocket) -} - func dial(ctx context.Context, path string) (net.Conn, error) { dialer := net.Dialer{} return dialer.DialContext(ctx, "unix", path) diff --git a/pkg/desktop/secrets.go b/pkg/desktop/secrets.go deleted file mode 100644 index ce57f58d..00000000 --- a/pkg/desktop/secrets.go +++ /dev/null @@ -1,65 +0,0 @@ -package desktop - -import ( - "context" - "fmt" -) - -type StoredSecret struct { - Name string `json:"name"` - // Provider specifies the provider under which the secret is stored - // - // Deprecated: will be removed in a future release and is not currently - // used by default - Provider string `json:"provider,omitempty"` -} - -type Secret struct { - Name string `json:"name"` - Provider string `json:"provider,omitempty"` - Value string `json:"value"` -} - -func NewSecretsClient() *Secrets { - return &Secrets{ - rawClient: newRawClient(dialSecrets), - } -} - -type Secrets struct { - rawClient *RawClient -} - -func (c *Secrets) DeleteJfsSecret(ctx context.Context, secret string) error { - AvoidResourceSaverMode(ctx) - - return c.rawClient.Delete(ctx, fmt.Sprintf("/secrets/%v", secret)) -} - -func (c *Secrets) GetJfsPolicy(ctx context.Context) (string, error) { - AvoidResourceSaverMode(ctx) - - var result string - err := c.rawClient.Get(ctx, "/policy", &result) - return result, err -} - -func (c *Secrets) ListJfsSecrets(ctx context.Context) ([]StoredSecret, error) { - AvoidResourceSaverMode(ctx) - - var result []StoredSecret - err := c.rawClient.Get(ctx, "/secrets", &result) - return result, err -} - -func (c *Secrets) SetJfsPolicy(ctx context.Context, body string) error { - AvoidResourceSaverMode(ctx) - - return c.rawClient.Post(ctx, "/policy", body, nil) -} - -func (c *Secrets) SetJfsSecret(ctx context.Context, secret Secret) error { - AvoidResourceSaverMode(ctx) - - return c.rawClient.Post(ctx, "/secrets", secret, nil) -} diff --git a/pkg/docker/client.go b/pkg/docker/client.go index dda22fcc..d861d476 100644 --- a/pkg/docker/client.go +++ b/pkg/docker/client.go @@ -34,7 +34,6 @@ type Client interface { RemoveNetwork(ctx context.Context, name string) error ConnectNetwork(ctx context.Context, networkName, container, hostname string) error InspectVolume(ctx context.Context, name string) (volume.Volume, error) - ReadSecrets(ctx context.Context, names []string, lenient bool) (map[string]string, error) } type dockerClient struct { diff --git a/pkg/docker/secrets.go b/pkg/docker/secrets.go deleted file mode 100644 index e52bf49b..00000000 --- a/pkg/docker/secrets.go +++ /dev/null @@ -1,112 +0,0 @@ -package docker - -import ( - "context" - "encoding/json" - "fmt" - "maps" - "os" - "os/exec" - "runtime" - "strings" - - "github.com/docker/mcp-gateway/pkg/desktop" - "github.com/docker/mcp-gateway/pkg/log" -) - -const jcatImage = "docker/jcat@sha256:76719466e8b99a65dd1d37d9ab94108851f009f0f687dce7ff8a6fc90575c4d4" - -func (c *dockerClient) ReadSecrets(ctx context.Context, names []string, lenient bool) (map[string]string, error) { - if len(names) == 0 { - return map[string]string{}, nil // No secrets to read - } - - if err := c.PullImage(ctx, jcatImage); err != nil { - return nil, err - } - - if lenient && len(names) == 1 { - // If there's only one secret, read it directly and fall back to one-by-one reading if needed - return c.readSecretsOneByOneOptional(ctx, names) - } - - secrets, err := c.readSecrets(ctx, names) - if err != nil { - if lenient && strings.Contains(err.Error(), "no such secret") { - return c.readSecretsOneByOneOptional(ctx, names) - } - - return nil, fmt.Errorf("reading secrets %w", err) - } - - return secrets, nil -} - -func (c *dockerClient) readSecrets(ctx context.Context, names []string) (map[string]string, error) { - flags := []string{"--network=none", "--pull=never"} - var command []string - - for i, name := range names { - file := fmt.Sprintf("/.s%d", i) - flags = append(flags, "-l", "x-secret:"+name+"="+file) - command = append(command, file) - } - - var args []string - - // When running in cloud mode but not in a container, we might be able to use Docker Desktop's special socket - // to read the secrets. - if os.Getenv("DOCKER_MCP_IN_CONTAINER") != "1" { - var path string - switch runtime.GOOS { - case "windows": - path = "npipe://" + strings.ReplaceAll(desktop.Paths().RawDockerSocket, `\`, `/`) - default: - // On Darwin/Linux, we do it only if the socket actually exists. - if _, err := os.Stat(desktop.Paths().RawDockerSocket); err == nil { - path = "unix://" + desktop.Paths().RawDockerSocket - } - } - if path != "" { - args = append(args, "-H", path) - } - } - args = append(args, "run", "--rm") - args = append(args, flags...) - args = append(args, jcatImage) - args = append(args, command...) - buf, err := exec.CommandContext(ctx, "docker", args...).CombinedOutput() - if err != nil { - return nil, fmt.Errorf("reading secrets %w: %s", err, string(buf)) - } - - var list []string - if err := json.Unmarshal(buf, &list); err != nil { - return nil, err - } - - values := map[string]string{} - for i := range names { - values[names[i]] = list[i] - } - - return values, nil -} - -// readSecretsOneByOne reads secrets one by one, which is useful for lenient mode. -// It's slower but can handle cases where some secrets might not exist. -func (c *dockerClient) readSecretsOneByOneOptional(ctx context.Context, names []string) (map[string]string, error) { - secrets := map[string]string{} - - for _, name := range names { - values, err := c.readSecrets(ctx, []string{name}) - if err != nil { - log.Logf("couldn't read secret %s: %v", name, err) - continue - } - - maps.Copy(secrets, values) - } - - return secrets, nil -} diff --git a/pkg/gateway/clientpool_test.go b/pkg/gateway/clientpool_test.go index 36a5e577..0f464dff 100644 --- a/pkg/gateway/clientpool_test.go +++ b/pkg/gateway/clientpool_test.go @@ -43,7 +43,7 @@ grafana: "-l", "docker-mcp=true", "-l", "docker-mcp-tool-type=mcp", "-l", "docker-mcp-name=grafana", "-l", "docker-mcp-transport=stdio", "-e", "GRAFANA_API_KEY", "-e", "GRAFANA_URL", }, args) - assert.Equal(t, []string{"GRAFANA_API_KEY=API_KEY", "GRAFANA_URL=TEST"}, env) + assert.Equal(t, []string{"GRAFANA_API_KEY=se://docker/mcp/generic/grafana.api_key", "GRAFANA_URL=TEST"}, env) } func TestApplyConfigMongoDB(t *testing.T) { @@ -63,7 +63,7 @@ secrets: "-l", "docker-mcp=true", "-l", "docker-mcp-tool-type=mcp", "-l", "docker-mcp-name=mongodb", "-l", "docker-mcp-transport=stdio", "-e", "MDB_MCP_CONNECTION_STRING", }, args) - assert.Equal(t, []string{"MDB_MCP_CONNECTION_STRING=HOST:PORT"}, env) + assert.Equal(t, []string{"MDB_MCP_CONNECTION_STRING=se://docker/mcp/generic/mongodb.connection_string"}, env) } func TestApplyConfigNotion(t *testing.T) { @@ -87,7 +87,7 @@ env: "-l", "docker-mcp=true", "-l", "docker-mcp-tool-type=mcp", "-l", "docker-mcp-name=notion", "-l", "docker-mcp-transport=stdio", "-e", "INTERNAL_INTEGRATION_TOKEN", "-e", "OPENAPI_MCP_HEADERS", }, args) - assert.Equal(t, []string{"INTERNAL_INTEGRATION_TOKEN=ntn_DUMMY", `OPENAPI_MCP_HEADERS={"Authorization": "Bearer ntn_DUMMY", "Notion-Version": "2022-06-28"}`}, env) + assert.Equal(t, []string{"INTERNAL_INTEGRATION_TOKEN=se://docker/mcp/generic/notion.internal_integration_token", `OPENAPI_MCP_HEADERS={"Authorization": "Bearer se://docker/mcp/generic/notion.internal_integration_token", "Notion-Version": "2022-06-28"}`}, env) } func TestApplyConfigMountAs(t *testing.T) { diff --git a/pkg/gateway/configuration.go b/pkg/gateway/configuration.go index 076ac016..3cc5858e 100644 --- a/pkg/gateway/configuration.go +++ b/pkg/gateway/configuration.go @@ -1,8 +1,6 @@ package gateway import ( - "bufio" - "bytes" "context" "fmt" "os" @@ -374,8 +372,7 @@ func (c *FileBasedConfiguration) readOnce(ctx context.Context) (Configuration, e // Secrets no longer read during configuration load // Instead, se:// URIs are passed to containers and resolved at runtime - var secrets map[string]string - secrets = make(map[string]string) + secrets := make(map[string]string) log.Log("- Configuration read in", time.Since(start)) return Configuration{ @@ -501,47 +498,6 @@ func (c *FileBasedConfiguration) readToolsConfig(ctx context.Context) (config.To return mergedToolsConfig, nil } -func (c *FileBasedConfiguration) readDockerDesktopSecrets(ctx context.Context, servers map[string]catalog.Server, serverNames []string) (map[string]string, error) { - // Secrets no longer read - se:// URIs passed to containers and resolved at runtime - return map[string]string{}, nil -} - -func (c *FileBasedConfiguration) readSecretsFromFile(ctx context.Context, path string) (map[string]string, error) { - secrets := map[string]string{} - - buf, err := os.ReadFile(path) - if err != nil { - return nil, fmt.Errorf("reading secrets from %s: %w", path, err) - } - - scanner := bufio.NewScanner(bytes.NewReader(buf)) - for scanner.Scan() { - if ctx.Err() != nil { - return nil, ctx.Err() - } - - line := scanner.Text() - if strings.HasPrefix(line, "#") { - continue - } - - line = strings.TrimSpace(line) - if line == "" { - continue - } - - var key, value string - key, value, ok := strings.Cut(line, "=") - if !ok { - return nil, fmt.Errorf("invalid line in secrets file: %s", line) - } - - secrets[key] = value - } - - return secrets, nil -} - // readServersFromOci fetches and parses server definitions from OCI references func (c *FileBasedConfiguration) readServersFromOci(_ context.Context) (map[string]catalog.Server, error) { ociServers := make(map[string]catalog.Server) diff --git a/pkg/gateway/configuration_workingset.go b/pkg/gateway/configuration_workingset.go index 609c5c6c..7fac6bfb 100644 --- a/pkg/gateway/configuration_workingset.go +++ b/pkg/gateway/configuration_workingset.go @@ -157,7 +157,7 @@ func (c *WorkingSetConfiguration) readSecrets(ctx context.Context, workingSet wo return providerSecrets, nil } -func (c *WorkingSetConfiguration) readDockerDesktopSecrets(ctx context.Context, servers []workingset.Server) (map[string]string, error) { +func (c *WorkingSetConfiguration) readDockerDesktopSecrets(_ context.Context, _ []workingset.Server) (map[string]string, error) { // Secrets no longer read - se:// URIs passed to containers and resolved at runtime return map[string]string{}, nil } From 51295fbfd92aeb22da42ee2bfb3b0fafcd32ea63 Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Mon, 17 Nov 2025 18:09:19 -0800 Subject: [PATCH 04/15] Use secrets client for remote case --- pkg/mcp/remote.go | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/pkg/mcp/remote.go b/pkg/mcp/remote.go index c9cd9687..68b433d2 100644 --- a/pkg/mcp/remote.go +++ b/pkg/mcp/remote.go @@ -10,6 +10,7 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/secret" "github.com/docker/mcp-gateway/pkg/catalog" "github.com/docker/mcp-gateway/pkg/oauth" ) @@ -47,10 +48,12 @@ func (c *remoteMCPClient) Initialize(ctx context.Context, _ *mcp.InitializeParam transport = c.config.Spec.Remote.Transport } - // Secrets to env + // Secrets to env - for remote MCPs, we need actual values (not se:// URIs) + // because they're used in HTTP headers env := map[string]string{} - for _, secret := range c.config.Spec.Secrets { - env[secret.Env] = c.config.Secrets[secret.Name] + for _, s := range c.config.Spec.Secrets { + value := getSecretValue(ctx, s.Name) + env[s.Env] = value } // Headers @@ -121,6 +124,21 @@ func (c *remoteMCPClient) AddRoots(roots []*mcp.Root) { c.roots = roots } +func getSecretValue(ctx context.Context, secretName string) string { + envelopes, err := secret.GetSecrets(ctx) + if err != nil { + return "" + } + + fullID := "docker/mcp/generic/" + secretName + for _, env := range envelopes { + if env.ID == fullID { + return string(env.Value) + } + } + return "" +} + func expandEnv(value string, secrets map[string]string) string { return os.Expand(value, func(name string) string { return secrets[name] From 585455609a03a80d6aaac4f7b45cf2adcca159b5 Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Wed, 19 Nov 2025 18:09:17 -0800 Subject: [PATCH 05/15] Support OAuth DCR flows wip --- .../secret-management/secret/secretsengine.go | 49 +++++++--- pkg/mcp/remote.go | 25 ++--- pkg/oauth/credhelper.go | 97 +++++++++++-------- 3 files changed, 99 insertions(+), 72 deletions(-) diff --git a/cmd/docker-mcp/secret-management/secret/secretsengine.go b/cmd/docker-mcp/secret-management/secret/secretsengine.go index e0cfc4f5..2fd37801 100644 --- a/cmd/docker-mcp/secret-management/secret/secretsengine.go +++ b/cmd/docker-mcp/secret-management/secret/secretsengine.go @@ -35,22 +35,45 @@ var c = http.Client{ } func GetSecrets(ctx context.Context) ([]Envelope, error) { - pattern := []byte(`{"pattern": "docker/mcp/**"}`) - req, err := http.NewRequestWithContext(ctx, http.MethodPost, "http://localhost/resolver.v1.ResolverService/GetSecrets", bytes.NewReader(pattern)) - if err != nil { - return nil, err + // Workaround: Query multiple patterns since docker/mcp/** double-wildcard isn't working + // TODO: Remove once Secrets Engine fixes pattern matching bug + patterns := []string{ + `{"pattern": "docker/mcp/generic/*"}`, // Generic secrets (docker pass) + `{"pattern": "docker/mcp/oauth/*"}`, // OAuth tokens } - req.Header.Add("Content-Type", "application/json") - resp, err := c.Do(req) - if err != nil { - return nil, err + allSecrets := make(map[string]Envelope) // Use map to deduplicate by ID + + for _, pattern := range patterns { + req, err := http.NewRequestWithContext(ctx, http.MethodPost, "http://localhost/resolver.v1.ResolverService/GetSecrets", bytes.NewReader([]byte(pattern))) + if err != nil { + return nil, err + } + req.Header.Add("Content-Type", "application/json") + + resp, err := c.Do(req) + if err != nil { + return nil, err + } + + var secrets map[string][]Envelope + if err := json.NewDecoder(resp.Body).Decode(&secrets); err != nil { + resp.Body.Close() + return nil, err + } + resp.Body.Close() + + // Merge results, deduplicating by ID + for _, env := range secrets["envelopes"] { + allSecrets[env.ID] = env + } } - defer resp.Body.Close() - var secrets map[string][]Envelope - if err := json.NewDecoder(resp.Body).Decode(&secrets); err != nil { - return nil, err + // Convert map back to slice + result := make([]Envelope, 0, len(allSecrets)) + for _, env := range allSecrets { + result = append(result, env) } - return secrets["envelopes"], nil + + return result, nil } diff --git a/pkg/mcp/remote.go b/pkg/mcp/remote.go index 68b433d2..36dd7849 100644 --- a/pkg/mcp/remote.go +++ b/pkg/mcp/remote.go @@ -12,6 +12,7 @@ import ( "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/secret" "github.com/docker/mcp-gateway/pkg/catalog" + "github.com/docker/mcp-gateway/pkg/log" "github.com/docker/mcp-gateway/pkg/oauth" ) @@ -64,8 +65,11 @@ func (c *remoteMCPClient) Initialize(ctx context.Context, _ *mcp.InitializeParam // Add OAuth token if remote server has OAuth configuration if c.config.Spec.OAuth != nil && len(c.config.Spec.OAuth.Providers) > 0 { - token := c.getOAuthToken(ctx) - if token != "" { + credHelper := oauth.NewOAuthCredentialHelper() + token, err := credHelper.GetOAuthToken(ctx, c.config.Name) + if err != nil { + log.Logf("Failed to get OAuth token for %s: %v", c.config.Name, err) + } else if token != "" { headers["Authorization"] = "Bearer " + token } } @@ -164,20 +168,3 @@ func (h *headerRoundTripper) RoundTrip(req *http.Request) (*http.Response, error } return h.base.RoundTrip(newReq) } - -func (c *remoteMCPClient) getOAuthToken(ctx context.Context) string { - if c.config.Spec.OAuth == nil || len(c.config.Spec.OAuth.Providers) == 0 { - return "" - } - - // Use secure credential helper to get OAuth token directly from system credential store - // This bypasses the vulnerable IPC endpoint that exposes tokens - credHelper := oauth.NewOAuthCredentialHelper() - token, err := credHelper.GetOAuthToken(ctx, c.config.Name) - if err != nil { - // Token might not exist if user hasn't authorized yet - return "" - } - - return token -} diff --git a/pkg/oauth/credhelper.go b/pkg/oauth/credhelper.go index 9ec74001..088d9a40 100644 --- a/pkg/oauth/credhelper.go +++ b/pkg/oauth/credhelper.go @@ -12,7 +12,7 @@ import ( "github.com/docker/docker-credential-helpers/client" "github.com/docker/docker-credential-helpers/credentials" - "github.com/docker/mcp-gateway/pkg/desktop" + "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/secret" "github.com/docker/mcp-gateway/pkg/log" "github.com/docker/mcp-gateway/pkg/oauth/dcr" ) @@ -42,43 +42,47 @@ type TokenStatus struct { } // GetOAuthToken retrieves an OAuth token for the specified server -// It follows this flow: -// 1. Get DCR client info to retrieve provider name and authorization endpoint -// 2. Construct credential key using: [AuthorizationEndpoint]/[ProviderName] -// 3. Retrieve token from credential helper func (h *CredentialHelper) GetOAuthToken(ctx context.Context, serverName string) (string, error) { - var credentialKey string + var tokenSecret string - // Get DCR client based on mode if IsCEMode() { - // CE mode: Read DCR client from credential helper + // CE mode: Use credential helper directly dcrMgr := dcr.NewManager(h.credentialHelper, "") client, err := dcrMgr.GetDCRClient(serverName) if err != nil { - log.Logf("- Failed to get DCR client for %s: %v", serverName, err) return "", fmt.Errorf("no DCR client found for %s: %w", serverName, err) } - credentialKey = fmt.Sprintf("%s/%s", client.AuthorizationEndpoint, client.ProviderName) + credentialKey := fmt.Sprintf("%s/%s", client.AuthorizationEndpoint, client.ProviderName) + + _, tokenSecret, err = h.credentialHelper.Get(credentialKey) + if err != nil { + if credentials.IsErrCredentialsNotFound(err) { + return "", fmt.Errorf("OAuth token not found for %s. Run 'docker mcp oauth authorize %s' to authenticate", serverName, serverName) + } + return "", fmt.Errorf("failed to retrieve OAuth token for %s: %w", serverName, err) + } } else { - // Desktop mode: Use Desktop API - client := desktop.NewAuthClient() - dcrClient, err := client.GetDCRClient(ctx, serverName) + // Desktop mode: Query Secrets Engine + envelopes, err := secret.GetSecrets(ctx) if err != nil { - log.Logf("- Failed to get DCR client for %s: %v", serverName, err) - return "", fmt.Errorf("no DCR client found for %s: %w", serverName, err) + return "", fmt.Errorf("failed to query Secrets Engine: %w", err) } - credentialKey = fmt.Sprintf("%s/%s", dcrClient.AuthorizationEndpoint, dcrClient.ProviderName) - } - // Retrieve token from credential helper - _, tokenSecret, err := h.credentialHelper.Get(credentialKey) - if err != nil { - if credentials.IsErrCredentialsNotFound(err) { - log.Logf("- OAuth token not found for key: %s", credentialKey) - return "", fmt.Errorf("OAuth token not found for %s (key: %s). Run 'docker mcp oauth authorize %s' to authenticate", serverName, credentialKey, serverName) + // Look for OAuth token (namespace: docker/mcp/oauth/{provider}) + oauthID := "docker/mcp/oauth/" + serverName + + found := false + for _, env := range envelopes { + if env.ID == oauthID { + tokenSecret = string(env.Value) + found = true + break + } + } + + if !found { + return "", fmt.Errorf("OAuth token not found for %s. Run 'docker mcp oauth authorize %s' to authenticate", serverName, serverName) } - log.Logf("- Failed to retrieve token from credential helper: %v", err) - return "", fmt.Errorf("failed to retrieve OAuth token for %s: %w", serverName, err) } if tokenSecret == "" { @@ -109,40 +113,53 @@ func (h *CredentialHelper) GetOAuthToken(ctx context.Context, serverName string) // GetTokenStatus checks if an OAuth token is valid and whether it needs refresh func (h *CredentialHelper) GetTokenStatus(ctx context.Context, serverName string) (TokenStatus, error) { - var credentialKey string + var tokenSecret string - // Get DCR client based on mode if IsCEMode() { - // CE mode: Read DCR client from credential helper + // CE mode: Use credential helper directly dcrMgr := dcr.NewManager(h.credentialHelper, "") client, err := dcrMgr.GetDCRClient(serverName) if err != nil { return TokenStatus{Valid: false}, fmt.Errorf("no DCR client found for %s: %w", serverName, err) } - credentialKey = fmt.Sprintf("%s/%s", client.AuthorizationEndpoint, client.ProviderName) + credentialKey := fmt.Sprintf("%s/%s", client.AuthorizationEndpoint, client.ProviderName) + + _, tokenSecret, err = h.credentialHelper.Get(credentialKey) + if err != nil { + if credentials.IsErrCredentialsNotFound(err) { + return TokenStatus{Valid: false}, fmt.Errorf("OAuth token not found for %s", serverName) + } + return TokenStatus{Valid: false}, fmt.Errorf("failed to retrieve OAuth token for %s: %w", serverName, err) + } } else { - // Desktop mode: Use Desktop API - client := desktop.NewAuthClient() - dcrClient, err := client.GetDCRClient(ctx, serverName) + // Desktop mode: Query Secrets Engine + envelopes, err := secret.GetSecrets(ctx) if err != nil { - return TokenStatus{Valid: false}, fmt.Errorf("no DCR client found for %s: %w", serverName, err) + return TokenStatus{Valid: false}, fmt.Errorf("failed to query Secrets Engine: %w", err) } - credentialKey = fmt.Sprintf("%s/%s", dcrClient.AuthorizationEndpoint, dcrClient.ProviderName) - } - // Retrieve token from credential helper - _, tokenSecret, err := h.credentialHelper.Get(credentialKey) - if err != nil { - if credentials.IsErrCredentialsNotFound(err) { + // Look for OAuth token (namespace: docker/mcp/oauth/{provider}) + oauthID := "docker/mcp/oauth/" + serverName + + found := false + for _, env := range envelopes { + if env.ID == oauthID { + tokenSecret = string(env.Value) + found = true + break + } + } + + if !found { return TokenStatus{Valid: false}, fmt.Errorf("OAuth token not found for %s", serverName) } - return TokenStatus{Valid: false}, fmt.Errorf("failed to retrieve OAuth token for %s: %w", serverName, err) } if tokenSecret == "" { return TokenStatus{Valid: false}, fmt.Errorf("empty OAuth token found for %s", serverName) } + // Base64 decode the token tokenJSON, err := base64.StdEncoding.DecodeString(tokenSecret) if err != nil { return TokenStatus{Valid: false}, fmt.Errorf("failed to decode OAuth token for %s: %w", serverName, err) From 1a546ddf5913ffe98b8c08a47082fa5984a4bd4e Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Thu, 20 Nov 2025 08:07:01 -0800 Subject: [PATCH 06/15] remove JFS windows connection --- pkg/desktop/connection_windows.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/desktop/connection_windows.go b/pkg/desktop/connection_windows.go index fcf2f91d..4518cf8a 100644 --- a/pkg/desktop/connection_windows.go +++ b/pkg/desktop/connection_windows.go @@ -15,10 +15,6 @@ func dialAuth(ctx context.Context) (net.Conn, error) { return dial(ctx, Paths().ToolsSocket) } -func dialSecrets(ctx context.Context) (net.Conn, error) { - return dial(ctx, Paths().JFSSocket) -} - func dial(ctx context.Context, path string) (net.Conn, error) { return winio.DialPipeContext(ctx, path) } From d2602e1cd19a80afdefa142b5b42bc9f56997262 Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Thu, 20 Nov 2025 08:12:59 -0800 Subject: [PATCH 07/15] remake docs for CI --- docs/generator/reference/docker_mcp.yaml | 2 -- docs/generator/reference/mcp.md | 1 - 2 files changed, 3 deletions(-) diff --git a/docs/generator/reference/docker_mcp.yaml b/docs/generator/reference/docker_mcp.yaml index 83b5306c..9b32240e 100644 --- a/docs/generator/reference/docker_mcp.yaml +++ b/docs/generator/reference/docker_mcp.yaml @@ -14,7 +14,6 @@ cname: - docker mcp config - docker mcp feature - docker mcp gateway - - docker mcp policy - docker mcp secret - docker mcp server - docker mcp tools @@ -25,7 +24,6 @@ clink: - docker_mcp_config.yaml - docker_mcp_feature.yaml - docker_mcp_gateway.yaml - - docker_mcp_policy.yaml - docker_mcp_secret.yaml - docker_mcp_server.yaml - docker_mcp_tools.yaml diff --git a/docs/generator/reference/mcp.md b/docs/generator/reference/mcp.md index 94d00b6f..7340f841 100644 --- a/docs/generator/reference/mcp.md +++ b/docs/generator/reference/mcp.md @@ -12,7 +12,6 @@ Manage MCP servers and clients | [`config`](mcp_config.md) | Manage the configuration | | [`feature`](mcp_feature.md) | Manage experimental features | | [`gateway`](mcp_gateway.md) | Manage the MCP Server gateway | -| [`policy`](mcp_policy.md) | Manage secret policies | | [`secret`](mcp_secret.md) | Manage secrets in the local OS Keychain | | [`server`](mcp_server.md) | Manage servers | | [`tools`](mcp_tools.md) | Manage tools | From 527a07f1f4c1d05edecb177208929ccdaa4d970d Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Thu, 20 Nov 2025 12:07:17 -0800 Subject: [PATCH 08/15] remove additional JFS references --- pkg/desktop/paths.go | 1 - pkg/desktop/sockets_darwin.go | 1 - pkg/desktop/sockets_linux.go | 2 -- pkg/desktop/sockets_windows.go | 1 - 4 files changed, 5 deletions(-) diff --git a/pkg/desktop/paths.go b/pkg/desktop/paths.go index 34a5a5b5..00faef47 100644 --- a/pkg/desktop/paths.go +++ b/pkg/desktop/paths.go @@ -6,7 +6,6 @@ type DockerDesktopPaths struct { AdminSettingPath string BackendSocket string RawDockerSocket string - JFSSocket string ToolsSocket string CredentialHelperPath func() string } diff --git a/pkg/desktop/sockets_darwin.go b/pkg/desktop/sockets_darwin.go index 2a9ed278..4ddc6a74 100644 --- a/pkg/desktop/sockets_darwin.go +++ b/pkg/desktop/sockets_darwin.go @@ -20,7 +20,6 @@ func getDockerDesktopPaths() (DockerDesktopPaths, error) { AdminSettingPath: filepath.Join(applicationSupport, "admin-settings.json"), BackendSocket: filepath.Join(data, "backend.sock"), RawDockerSocket: filepath.Join(data, "docker.raw.sock"), - JFSSocket: filepath.Join(data, "jfs.sock"), ToolsSocket: filepath.Join(data, "tools.sock"), CredentialHelperPath: getCredentialHelperPath, }, nil diff --git a/pkg/desktop/sockets_linux.go b/pkg/desktop/sockets_linux.go index 272eeb1f..be6e33b8 100644 --- a/pkg/desktop/sockets_linux.go +++ b/pkg/desktop/sockets_linux.go @@ -26,7 +26,6 @@ func getDockerDesktopPaths() (DockerDesktopPaths, error) { AdminSettingPath: "/usr/share/docker-desktop/admin-settings.json", BackendSocket: filepath.Join(home, ".docker/desktop/backend.sock"), RawDockerSocket: filepath.Join(home, ".docker/desktop/docker.raw.sock"), - JFSSocket: filepath.Join(home, ".docker/desktop/jfs.sock"), ToolsSocket: filepath.Join(home, ".docker/desktop/tools.sock"), CredentialHelperPath: getCredentialHelperPath, }, nil @@ -37,7 +36,6 @@ func getDockerDesktopPaths() (DockerDesktopPaths, error) { AdminSettingPath: "/usr/share/docker-desktop/admin-settings.json", BackendSocket: "/run/host-services/backend.sock", RawDockerSocket: "/var/run/docker.sock.raw", - JFSSocket: "/run/host-services/jfs.sock", ToolsSocket: "/run/host-services/tools.sock", CredentialHelperPath: getCredentialHelperPath, }, nil diff --git a/pkg/desktop/sockets_windows.go b/pkg/desktop/sockets_windows.go index 6971d5ae..1bbea15b 100644 --- a/pkg/desktop/sockets_windows.go +++ b/pkg/desktop/sockets_windows.go @@ -17,7 +17,6 @@ func getDockerDesktopPaths() (DockerDesktopPaths, error) { AdminSettingPath: filepath.Join(appData, `DockerDesktop\admin-settings.json`), BackendSocket: `\\.\pipe\dockerBackendApiServer`, RawDockerSocket: `\\.\pipe\docker_engine_linux`, - JFSSocket: `\\.\pipe\dockerJfs`, ToolsSocket: `\\.\pipe\dockerTools`, CredentialHelperPath: getCredentialHelperPath, }, nil From baae072559f20170deffbad7b36c71c55840b1ba Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Mon, 24 Nov 2025 17:32:24 -0800 Subject: [PATCH 09/15] updates --- cmd/docker-mcp/commands/secret.go | 8 ++--- .../secret-management/secret/credstore.go | 32 ++++++++++++++++++- .../secret-management/secret/secretsengine.go | 6 ++-- cmd/docker-mcp/server/secret_config.go | 5 ++- pkg/gateway/clientpool.go | 3 +- pkg/mcp/remote.go | 5 ++- pkg/oauth/credhelper.go | 8 ++--- 7 files changed, 49 insertions(+), 18 deletions(-) diff --git a/cmd/docker-mcp/commands/secret.go b/cmd/docker-mcp/commands/secret.go index d248b34b..3fd2500d 100644 --- a/cmd/docker-mcp/commands/secret.go +++ b/cmd/docker-mcp/commands/secret.go @@ -104,14 +104,14 @@ func listSecretCommand() *cobra.Command { } if outJSON { type secretListItem struct { - ID string `json:"id"` - Provider string `json:"provider"` + Name string `json:"name"` + Provider string `json:"provider,omitempty"` } output := make([]secretListItem, 0, len(l)) for _, env := range l { output = append(output, secretListItem{ - ID: env.ID, - Provider: env.Provider, + Name: secret.StripNamespace(env.ID), + Provider: string(env.Value), }) } if len(output) == 0 { diff --git a/cmd/docker-mcp/secret-management/secret/credstore.go b/cmd/docker-mcp/secret-management/secret/credstore.go index 9b62da0b..af8a56d6 100644 --- a/cmd/docker-mcp/secret-management/secret/credstore.go +++ b/cmd/docker-mcp/secret-management/secret/credstore.go @@ -11,6 +11,13 @@ import ( "strings" ) +const ( + // Namespace prefixes for different secret types + NamespaceGeneric = "docker/mcp/generic/" + NamespaceOAuth = "docker/mcp/oauth/" + NamespaceOAuthDCR = "docker/mcp/oauth-dcr/" +) + type CredStoreProvider struct{} func cmd(ctx context.Context, args ...string) *exec.Cmd { @@ -35,7 +42,30 @@ func cmd(ctx context.Context, args ...string) *exec.Cmd { // # specific to mysecret // pattern = "docker/mcp/generic/mysecret/application/**" func getSecretKey(secretName string) string { - return path.Join("docker/mcp/generic/", secretName) + return path.Join(NamespaceGeneric, secretName) +} + +// GetSecretKey constructs the full namespaced ID for a generic MCP secret +func GetSecretKey(secretName string) string { + return getSecretKey(secretName) +} + +// GetOAuthKey constructs the full namespaced ID for an OAuth token +func GetOAuthKey(provider string) string { + return path.Join(NamespaceOAuth, provider) +} + +// GetDCRKey constructs the full namespaced ID for a DCR client config +func GetDCRKey(serverName string) string { + return path.Join(NamespaceOAuthDCR, serverName) +} + +// StripNamespace removes the namespace prefix from a secret ID to get the simple name +func StripNamespace(secretID string) string { + name := strings.TrimPrefix(secretID, NamespaceGeneric) + name = strings.TrimPrefix(name, NamespaceOAuth) + name = strings.TrimPrefix(name, NamespaceOAuthDCR) + return name } func List(ctx context.Context) ([]string, error) { diff --git a/cmd/docker-mcp/secret-management/secret/secretsengine.go b/cmd/docker-mcp/secret-management/secret/secretsengine.go index 2fd37801..fcc7714d 100644 --- a/cmd/docker-mcp/secret-management/secret/secretsengine.go +++ b/cmd/docker-mcp/secret-management/secret/secretsengine.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "net" "net/http" "os" @@ -38,8 +39,9 @@ func GetSecrets(ctx context.Context) ([]Envelope, error) { // Workaround: Query multiple patterns since docker/mcp/** double-wildcard isn't working // TODO: Remove once Secrets Engine fixes pattern matching bug patterns := []string{ - `{"pattern": "docker/mcp/generic/*"}`, // Generic secrets (docker pass) - `{"pattern": "docker/mcp/oauth/*"}`, // OAuth tokens + fmt.Sprintf(`{"pattern": "%s*"}`, NamespaceGeneric), // Generic secrets (docker pass) + fmt.Sprintf(`{"pattern": "%s*"}`, NamespaceOAuth), // OAuth tokens + fmt.Sprintf(`{"pattern": "%s*"}`, NamespaceOAuthDCR), // DCR configs } allSecrets := make(map[string]Envelope) // Use map to deduplicate by ID diff --git a/cmd/docker-mcp/server/secret_config.go b/cmd/docker-mcp/server/secret_config.go index de379573..c5db3130 100644 --- a/cmd/docker-mcp/server/secret_config.go +++ b/cmd/docker-mcp/server/secret_config.go @@ -2,7 +2,6 @@ package server import ( "context" - "strings" "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/secret" ) @@ -17,8 +16,8 @@ func getConfiguredSecretNames(ctx context.Context) (map[string]struct{}, error) configuredSecretNames := make(map[string]struct{}) for _, env := range envelopes { - // Extract base name from full ID (e.g., "docker/mcp/generic/brave.api_key" → "brave.api_key") - name := strings.TrimPrefix(env.ID, "docker/mcp/generic/") + // Extract base name from full ID using centralized namespace stripper + name := secret.StripNamespace(env.ID) configuredSecretNames[name] = struct{}{} } diff --git a/pkg/gateway/clientpool.go b/pkg/gateway/clientpool.go index 57576aeb..b34cbfc0 100644 --- a/pkg/gateway/clientpool.go +++ b/pkg/gateway/clientpool.go @@ -10,6 +10,7 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/secret" "github.com/docker/mcp-gateway/pkg/catalog" "github.com/docker/mcp-gateway/pkg/docker" "github.com/docker/mcp-gateway/pkg/eval" @@ -323,7 +324,7 @@ func (cp *clientPool) argsAndEnv(serverConfig *catalog.ServerConfig, readOnly *b args = append(args, "-e", s.Env) // Build se:// URI with full namespace path - secretURI := fmt.Sprintf("se://docker/mcp/generic/%s", s.Name) + secretURI := fmt.Sprintf("se://%s", secret.GetSecretKey(s.Name)) env = append(env, fmt.Sprintf("%s=%s", s.Env, secretURI)) } diff --git a/pkg/mcp/remote.go b/pkg/mcp/remote.go index 36dd7849..50da6524 100644 --- a/pkg/mcp/remote.go +++ b/pkg/mcp/remote.go @@ -49,8 +49,7 @@ func (c *remoteMCPClient) Initialize(ctx context.Context, _ *mcp.InitializeParam transport = c.config.Spec.Remote.Transport } - // Secrets to env - for remote MCPs, we need actual values (not se:// URIs) - // because they're used in HTTP headers + // Secrets to env env := map[string]string{} for _, s := range c.config.Spec.Secrets { value := getSecretValue(ctx, s.Name) @@ -134,7 +133,7 @@ func getSecretValue(ctx context.Context, secretName string) string { return "" } - fullID := "docker/mcp/generic/" + secretName + fullID := secret.GetSecretKey(secretName) for _, env := range envelopes { if env.ID == fullID { return string(env.Value) diff --git a/pkg/oauth/credhelper.go b/pkg/oauth/credhelper.go index 088d9a40..15e56df0 100644 --- a/pkg/oauth/credhelper.go +++ b/pkg/oauth/credhelper.go @@ -68,8 +68,8 @@ func (h *CredentialHelper) GetOAuthToken(ctx context.Context, serverName string) return "", fmt.Errorf("failed to query Secrets Engine: %w", err) } - // Look for OAuth token (namespace: docker/mcp/oauth/{provider}) - oauthID := "docker/mcp/oauth/" + serverName + // Look for OAuth token using centralized namespace helper + oauthID := secret.GetOAuthKey(serverName) found := false for _, env := range envelopes { @@ -138,8 +138,8 @@ func (h *CredentialHelper) GetTokenStatus(ctx context.Context, serverName string return TokenStatus{Valid: false}, fmt.Errorf("failed to query Secrets Engine: %w", err) } - // Look for OAuth token (namespace: docker/mcp/oauth/{provider}) - oauthID := "docker/mcp/oauth/" + serverName + // Look for OAuth token using centralized namespace helper + oauthID := secret.GetOAuthKey(serverName) found := false for _, env := range envelopes { From e890cf7ffcc7009f2d6725c26476347ff72fd689 Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Mon, 24 Nov 2025 17:44:34 -0800 Subject: [PATCH 10/15] fix return type --- cmd/docker-mcp/commands/secret.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/docker-mcp/commands/secret.go b/cmd/docker-mcp/commands/secret.go index 3fd2500d..a6d061c7 100644 --- a/cmd/docker-mcp/commands/secret.go +++ b/cmd/docker-mcp/commands/secret.go @@ -111,7 +111,7 @@ func listSecretCommand() *cobra.Command { for _, env := range l { output = append(output, secretListItem{ Name: secret.StripNamespace(env.ID), - Provider: string(env.Value), + Provider: env.Provider, }) } if len(output) == 0 { From 312e404f1f58df653954dc6d6bde83b41a3c6659 Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Mon, 1 Dec 2025 21:23:06 -0800 Subject: [PATCH 11/15] PR feedback: provider help, uri construction, compose secrets --- cmd/docker-mcp/commands/secret.go | 2 +- pkg/gateway/clientpool.go | 11 +++-- pkg/gateway/configuration.go | 77 +++++++++++++++++++++++++++++-- pkg/mcp/remote.go | 3 +- 4 files changed, 83 insertions(+), 10 deletions(-) diff --git a/cmd/docker-mcp/commands/secret.go b/cmd/docker-mcp/commands/secret.go index a6d061c7..5fc2a47a 100644 --- a/cmd/docker-mcp/commands/secret.go +++ b/cmd/docker-mcp/commands/secret.go @@ -164,7 +164,7 @@ func setSecretCommand() *cobra.Command { } flags := cmd.Flags() flags.StringVar(&opts.Provider, "provider", "", "Supported: credstore, oauth/") - _ = flags.MarkDeprecated("provider", "option will be ignored") + _ = flags.MarkDeprecated("provider", "all secrets now stored via docker pass in OS Keychain") return cmd } diff --git a/pkg/gateway/clientpool.go b/pkg/gateway/clientpool.go index b34cbfc0..bac0eda9 100644 --- a/pkg/gateway/clientpool.go +++ b/pkg/gateway/clientpool.go @@ -10,7 +10,6 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" - "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/secret" "github.com/docker/mcp-gateway/pkg/catalog" "github.com/docker/mcp-gateway/pkg/docker" "github.com/docker/mcp-gateway/pkg/eval" @@ -323,9 +322,13 @@ func (cp *clientPool) argsAndEnv(serverConfig *catalog.ServerConfig, readOnly *b for _, s := range serverConfig.Spec.Secrets { args = append(args, "-e", s.Env) - // Build se:// URI with full namespace path - secretURI := fmt.Sprintf("se://%s", secret.GetSecretKey(s.Name)) - env = append(env, fmt.Sprintf("%s=%s", s.Env, secretURI)) + secretValue, ok := serverConfig.Secrets[s.Name] + if ok { + env = append(env, fmt.Sprintf("%s=%s", s.Env, secretValue)) + } else { + log.Logf("Warning: Secret '%s' not found for server '%s', setting %s=", s.Name, serverConfig.Name, s.Env) + env = append(env, fmt.Sprintf("%s=%s", s.Env, "")) + } } // Env diff --git a/pkg/gateway/configuration.go b/pkg/gateway/configuration.go index 3cc5858e..995ca632 100644 --- a/pkg/gateway/configuration.go +++ b/pkg/gateway/configuration.go @@ -1,6 +1,8 @@ package gateway import ( + "bufio" + "bytes" "context" "fmt" "os" @@ -11,6 +13,7 @@ import ( "github.com/fsnotify/fsnotify" "gopkg.in/yaml.v3" + "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/secret" "github.com/docker/mcp-gateway/pkg/catalog" "github.com/docker/mcp-gateway/pkg/config" "github.com/docker/mcp-gateway/pkg/docker" @@ -370,9 +373,41 @@ func (c *FileBasedConfiguration) readOnce(ctx context.Context) (Configuration, e return Configuration{}, fmt.Errorf("reading tools: %w", err) } - // Secrets no longer read during configuration load - // Instead, se:// URIs are passed to containers and resolved at runtime - secrets := make(map[string]string) + // Helper function to build se:// URIs for all required secrets + buildSecretsURIs := func() map[string]string { + uris := make(map[string]string) + for _, serverName := range serverNames { + server, ok := servers[serverName] + if !ok { + continue + } + for _, s := range server.Secrets { + uris[s.Name] = fmt.Sprintf("se://%s", secret.GetSecretKey(s.Name)) + } + } + return uris + } + + var secrets map[string]string + if c.SecretsPath == "docker-desktop" { + // Pure Docker Desktop mode: use se:// URIs + secrets = buildSecretsURIs() + } else { + // Mixed or file-only mode: iterate through paths + // Unless SecretsPath is only `docker-desktop`, we don't fail if secrets can't be read. + // It's ok for the MCP toolkit's to not be available (in Cloud Run, for example). + // It's ok for secrets .env file to not exist. + for secretPath := range strings.SplitSeq(c.SecretsPath, ":") { + if secretPath == "docker-desktop" { + secrets = buildSecretsURIs() + break + } + secrets, err = c.readSecretsFromFile(ctx, secretPath) + if err == nil { + break + } + } + } log.Log("- Configuration read in", time.Since(start)) return Configuration{ @@ -543,3 +578,39 @@ func (c *FileBasedConfiguration) readServersFromOci(_ context.Context) (map[stri return ociServers, nil } + +func (c *FileBasedConfiguration) readSecretsFromFile(ctx context.Context, path string) (map[string]string, error) { + secrets := map[string]string{} + + buf, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("reading secrets from %s: %w", path, err) + } + + scanner := bufio.NewScanner(bytes.NewReader(buf)) + for scanner.Scan() { + if ctx.Err() != nil { + return nil, ctx.Err() + } + + line := scanner.Text() + if strings.HasPrefix(line, "#") { + continue + } + + line = strings.TrimSpace(line) + if line == "" { + continue + } + + var key, value string + key, value, ok := strings.Cut(line, "=") + if !ok { + return nil, fmt.Errorf("invalid line in secrets file: %s", line) + } + + secrets[key] = value + } + + return secrets, nil +} diff --git a/pkg/mcp/remote.go b/pkg/mcp/remote.go index 50da6524..7eaa9543 100644 --- a/pkg/mcp/remote.go +++ b/pkg/mcp/remote.go @@ -52,8 +52,7 @@ func (c *remoteMCPClient) Initialize(ctx context.Context, _ *mcp.InitializeParam // Secrets to env env := map[string]string{} for _, s := range c.config.Spec.Secrets { - value := getSecretValue(ctx, s.Name) - env[s.Env] = value + env[s.Env] = getSecretValue(ctx, s.Name) } // Headers From c90a2825cb3b9a5df61b6ec7a021c19ea0448ea7 Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Mon, 1 Dec 2025 21:41:44 -0800 Subject: [PATCH 12/15] Address PR feedback --- cmd/docker-mcp/commands/root.go | 4 ++-- cmd/docker-mcp/commands/secret.go | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/docker-mcp/commands/root.go b/cmd/docker-mcp/commands/root.go index 1dbb7ad4..2b19c4c8 100644 --- a/cmd/docker-mcp/commands/root.go +++ b/cmd/docker-mcp/commands/root.go @@ -40,6 +40,7 @@ func Root(ctx context.Context, cwd string, dockerCli command.Cli) *cobra.Command HiddenDefaultCmd: true, }, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + cmd.SetContext(ctx) if err := plugin.PersistentPreRunE(cmd, args); err != nil { return err } @@ -59,7 +60,6 @@ func Root(ctx context.Context, cwd string, dockerCli command.Cli) *cobra.Command }, Version: version.Version, } - cmd.SetContext(ctx) cmd.SetVersionTemplate("{{.Version}}\n") cmd.Flags().BoolP("version", "v", false, "Print version information and quit") cmd.SetHelpTemplate(helpTemplate) @@ -81,7 +81,7 @@ func Root(ctx context.Context, cwd string, dockerCli command.Cli) *cobra.Command cmd.AddCommand(gatewayCommand(dockerClient, dockerCli)) cmd.AddCommand(oauthCommand()) cmd.AddCommand(registryCommand()) - cmd.AddCommand(secretCommand(dockerClient)) + cmd.AddCommand(secretCommand()) cmd.AddCommand(serverCommand(dockerClient, dockerCli)) cmd.AddCommand(toolsCommand(dockerClient, dockerCli)) cmd.AddCommand(versionCommand()) diff --git a/cmd/docker-mcp/commands/secret.go b/cmd/docker-mcp/commands/secret.go index 5fc2a47a..a0a6c819 100644 --- a/cmd/docker-mcp/commands/secret.go +++ b/cmd/docker-mcp/commands/secret.go @@ -12,7 +12,6 @@ import ( "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/formatting" "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/secret" "github.com/docker/mcp-gateway/pkg/desktop" - "github.com/docker/mcp-gateway/pkg/docker" ) const setSecretExample = ` @@ -32,7 +31,7 @@ Another way to inject secrets would be to use a pattern. > cat pwd.txt | docker mcp secret set POSTGRES_PASSWORD ` -func secretCommand(_ docker.Client) *cobra.Command { +func secretCommand() *cobra.Command { cmd := &cobra.Command{ Use: "secret", Short: "Manage secrets in the local OS Keychain", From 724f2592c4ef436e69e959f10fa87f9df75444b8 Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Tue, 2 Dec 2025 15:39:28 -0800 Subject: [PATCH 13/15] subcommand fix, remove dead code, add test for file based env secrets --- cmd/docker-mcp/commands/root.go | 20 +++++++++ cmd/docker-mcp/commands/secret.go | 8 ---- pkg/gateway/clientpool_test.go | 32 ++++++++++++-- pkg/gateway/configuration_workingset.go | 57 ++++++------------------- 4 files changed, 61 insertions(+), 56 deletions(-) diff --git a/cmd/docker-mcp/commands/root.go b/cmd/docker-mcp/commands/root.go index 2b19c4c8..de20b1bf 100644 --- a/cmd/docker-mcp/commands/root.go +++ b/cmd/docker-mcp/commands/root.go @@ -3,6 +3,7 @@ package commands import ( "context" "os" + "slices" "github.com/docker/cli/cli-plugins/plugin" "github.com/docker/cli/cli/command" @@ -45,6 +46,13 @@ func Root(ctx context.Context, cwd string, dockerCli command.Cli) *cobra.Command return err } + // Check for docker-credential-desktop for secret commands + if isSubcommandOf(cmd, []string{"secret"}) { + if err := desktop.CheckHasDockerPass(cmd.Context()); err != nil { + return err + } + } + if os.Getenv("DOCKER_MCP_IN_CONTAINER") != "1" { runningInDockerCE, err := docker.RunningInDockerCE(ctx, dockerCli) if err != nil { @@ -100,3 +108,15 @@ func unhideHiddenCommands(cmd *cobra.Command) { unhideHiddenCommands(c) } } + +func isSubcommandOf(cmd *cobra.Command, names []string) bool { + if cmd == nil { + return false + } + + if slices.Contains(names, cmd.Name()) { + return true + } + + return isSubcommandOf(cmd.Parent(), names) +} diff --git a/cmd/docker-mcp/commands/secret.go b/cmd/docker-mcp/commands/secret.go index a0a6c819..bc5d824b 100644 --- a/cmd/docker-mcp/commands/secret.go +++ b/cmd/docker-mcp/commands/secret.go @@ -11,7 +11,6 @@ import ( "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/formatting" "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/secret" - "github.com/docker/mcp-gateway/pkg/desktop" ) const setSecretExample = ` @@ -36,13 +35,6 @@ func secretCommand() *cobra.Command { Use: "secret", Short: "Manage secrets in the local OS Keychain", Example: strings.Trim(setSecretExample, "\n"), - PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { - err := desktop.CheckHasDockerPass(cmd.Context()) - if err != nil { - return err - } - return nil - }, } cmd.AddCommand(rmSecretCommand()) cmd.AddCommand(listSecretCommand()) diff --git a/pkg/gateway/clientpool_test.go b/pkg/gateway/clientpool_test.go index 0f464dff..a8731272 100644 --- a/pkg/gateway/clientpool_test.go +++ b/pkg/gateway/clientpool_test.go @@ -33,7 +33,7 @@ grafana: url: TEST ` secrets := map[string]string{ - "grafana.api_key": "API_KEY", + "grafana.api_key": "se://docker/mcp/generic/grafana.api_key", } args, env := argsAndEnv(t, "grafana", catalogYAML, configYAML, secrets, nil) @@ -53,7 +53,7 @@ secrets: env: MDB_MCP_CONNECTION_STRING ` secrets := map[string]string{ - "mongodb.connection_string": "HOST:PORT", + "mongodb.connection_string": "se://docker/mcp/generic/mongodb.connection_string", } args, env := argsAndEnv(t, "mongodb", catalogYAML, "", secrets, nil) @@ -77,7 +77,7 @@ env: value: '{"Authorization": "Bearer $INTERNAL_INTEGRATION_TOKEN", "Notion-Version": "2022-06-28"}' ` secrets := map[string]string{ - "notion.internal_integration_token": "ntn_DUMMY", + "notion.internal_integration_token": "se://docker/mcp/generic/notion.internal_integration_token", } args, env := argsAndEnv(t, "notion", catalogYAML, "", secrets, nil) @@ -90,6 +90,32 @@ env: assert.Equal(t, []string{"INTERNAL_INTEGRATION_TOKEN=se://docker/mcp/generic/notion.internal_integration_token", `OPENAPI_MCP_HEADERS={"Authorization": "Bearer se://docker/mcp/generic/notion.internal_integration_token", "Notion-Version": "2022-06-28"}`}, env) } +func TestApplyConfigFileBasedSecrets(t *testing.T) { + // Test that file-based secrets (actual values) pass through correctly + catalogYAML := ` +secrets: + - name: db.password + env: DB_PASSWORD + - name: api.key + env: API_KEY +` + // File-based mode: secrets map contains actual values (not se:// URIs) + secrets := map[string]string{ + "db.password": "my-actual-db-password", + "api.key": "my-actual-api-key", + } + + args, env := argsAndEnv(t, "myserver", catalogYAML, "", secrets, nil) + + assert.Equal(t, []string{ + "run", "--rm", "-i", "--init", "--security-opt", "no-new-privileges", "--cpus", "1", "--memory", "2Gb", "--pull", "never", + "-l", "docker-mcp=true", "-l", "docker-mcp-tool-type=mcp", "-l", "docker-mcp-name=myserver", "-l", "docker-mcp-transport=stdio", + "-e", "DB_PASSWORD", "-e", "API_KEY", + }, args) + // File-based mode: actual values pass through unchanged + assert.Equal(t, []string{"DB_PASSWORD=my-actual-db-password", "API_KEY=my-actual-api-key"}, env) +} + func TestApplyConfigMountAs(t *testing.T) { catalogYAML := ` volumes: diff --git a/pkg/gateway/configuration_workingset.go b/pkg/gateway/configuration_workingset.go index fda76f5a..a9abb055 100644 --- a/pkg/gateway/configuration_workingset.go +++ b/pkg/gateway/configuration_workingset.go @@ -7,6 +7,7 @@ import ( "fmt" "time" + "github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/secret" "github.com/docker/mcp-gateway/pkg/catalog" "github.com/docker/mcp-gateway/pkg/config" "github.com/docker/mcp-gateway/pkg/db" @@ -70,16 +71,17 @@ func (c *WorkingSetConfiguration) readOnce(ctx context.Context, dao db.DAO) (Con } cfg := make(map[string]map[string]any) - flattenedSecrets := make(map[string]string) - providerSecrets, err := c.readSecrets(ctx, workingSet) - if err != nil { - return Configuration{}, fmt.Errorf("failed to read secrets: %w", err) - } - - for provider, s := range providerSecrets { - for name, value := range s { - flattenedSecrets[provider+"_"+name] = value + // Build se:// URIs for secrets (resolved at runtime by secrets engine) + // Keys are prefixed with the secrets provider reference to namespace them + secrets := make(map[string]string) + for _, server := range workingSet.Servers { + providerPrefix := "" + if server.Secrets != "" { + providerPrefix = server.Secrets + "_" + } + for _, s := range server.Snapshot.Server.Secrets { + secrets[providerPrefix+s.Name] = fmt.Sprintf("se://%s", secret.GetSecretKey(s.Name)) } } @@ -120,7 +122,7 @@ func (c *WorkingSetConfiguration) readOnce(ctx context.Context, dao db.DAO) (Con servers: servers, config: cfg, tools: toolsConfig, - secrets: flattenedSecrets, + secrets: secrets, }, nil } @@ -139,38 +141,3 @@ func (c *WorkingSetConfiguration) readTools(workingSet workingset.WorkingSet) co } return toolsConfig } - -func (c *WorkingSetConfiguration) readSecrets(ctx context.Context, workingSet workingset.WorkingSet) (map[string]map[string]string, error) { - providerSecrets := make(map[string]map[string]string) - for providerRef, secretConfig := range workingSet.Secrets { - servers := getServersUsingProvider(workingSet, providerRef) - - switch secretConfig.Provider { - case workingset.SecretProviderDockerDesktop: - secrets, err := c.readDockerDesktopSecrets(ctx, servers) - if err != nil { - return nil, fmt.Errorf("failed to read docker desktop secrets: %w", err) - } - providerSecrets[providerRef] = secrets - default: - return nil, fmt.Errorf("unknown secret provider: %s", secretConfig.Provider) - } - } - - return providerSecrets, nil -} - -func (c *WorkingSetConfiguration) readDockerDesktopSecrets(_ context.Context, _ []workingset.Server) (map[string]string, error) { - // Secrets no longer read - se:// URIs passed to containers and resolved at runtime - return map[string]string{}, nil -} - -func getServersUsingProvider(workingSet workingset.WorkingSet, providerRef string) []workingset.Server { - servers := make([]workingset.Server, 0) - for _, server := range workingSet.Servers { - if server.Secrets == providerRef { - servers = append(servers, server) - } - } - return servers -} From 6cd8aa81feb4c0d4ef6f459693a44a1b1d72130e Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Tue, 2 Dec 2025 16:06:37 -0800 Subject: [PATCH 14/15] Update comment for clarity and secretsengine pattern to include nested --- cmd/docker-mcp/commands/root.go | 2 +- cmd/docker-mcp/secret-management/secret/secretsengine.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/docker-mcp/commands/root.go b/cmd/docker-mcp/commands/root.go index de20b1bf..5f27d037 100644 --- a/cmd/docker-mcp/commands/root.go +++ b/cmd/docker-mcp/commands/root.go @@ -46,7 +46,7 @@ func Root(ctx context.Context, cwd string, dockerCli command.Cli) *cobra.Command return err } - // Check for docker-credential-desktop for secret commands + // Note: Using PersistentPreRunE in secretCommand would override this parent hook if isSubcommandOf(cmd, []string{"secret"}) { if err := desktop.CheckHasDockerPass(cmd.Context()); err != nil { return err diff --git a/cmd/docker-mcp/secret-management/secret/secretsengine.go b/cmd/docker-mcp/secret-management/secret/secretsengine.go index fcc7714d..160b75fb 100644 --- a/cmd/docker-mcp/secret-management/secret/secretsengine.go +++ b/cmd/docker-mcp/secret-management/secret/secretsengine.go @@ -39,9 +39,9 @@ func GetSecrets(ctx context.Context) ([]Envelope, error) { // Workaround: Query multiple patterns since docker/mcp/** double-wildcard isn't working // TODO: Remove once Secrets Engine fixes pattern matching bug patterns := []string{ - fmt.Sprintf(`{"pattern": "%s*"}`, NamespaceGeneric), // Generic secrets (docker pass) - fmt.Sprintf(`{"pattern": "%s*"}`, NamespaceOAuth), // OAuth tokens - fmt.Sprintf(`{"pattern": "%s*"}`, NamespaceOAuthDCR), // DCR configs + fmt.Sprintf(`{"pattern": "%s**"}`, NamespaceGeneric), // Generic secrets (docker pass) + fmt.Sprintf(`{"pattern": "%s**"}`, NamespaceOAuth), // OAuth tokens + fmt.Sprintf(`{"pattern": "%s**"}`, NamespaceOAuthDCR), // DCR configs } allSecrets := make(map[string]Envelope) // Use map to deduplicate by ID From 261cd00a4cc2e1927668bdb74c993bf98f6cee44 Mon Sep 17 00:00:00 2001 From: Saurabh Davala Date: Tue, 2 Dec 2025 18:41:44 -0800 Subject: [PATCH 15/15] log secret path in verbose mode + mask secrets --- pkg/gateway/clientpool.go | 15 +++++++++++++++ pkg/mcp/remote.go | 31 +++++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/pkg/gateway/clientpool.go b/pkg/gateway/clientpool.go index bac0eda9..93d631c0 100644 --- a/pkg/gateway/clientpool.go +++ b/pkg/gateway/clientpool.go @@ -324,6 +324,9 @@ func (cp *clientPool) argsAndEnv(serverConfig *catalog.ServerConfig, readOnly *b secretValue, ok := serverConfig.Secrets[s.Name] if ok { + if cp.Verbose { + log.Logf(" - %s: %s", s.Env, maskSecret(secretValue)) + } env = append(env, fmt.Sprintf("%s=%s", s.Env, secretValue)) } else { log.Logf("Warning: Secret '%s' not found for server '%s', setting %s=", s.Name, serverConfig.Name, s.Env) @@ -392,6 +395,18 @@ func expandEnvList(values []string, env []string) []string { return expanded } +// maskSecret shows the first few characters of a secret followed by asterisks. +// se:// URIs are shown in full since they're just references, not actual secrets. +func maskSecret(value string) string { + if strings.HasPrefix(value, "se://") { + return value + } + if len(value) <= 4 { + return "****" + } + return value[:4] + "****" +} + type clientGetter struct { once sync.Once client mcpclient.Client diff --git a/pkg/mcp/remote.go b/pkg/mcp/remote.go index 7eaa9543..358dda79 100644 --- a/pkg/mcp/remote.go +++ b/pkg/mcp/remote.go @@ -30,7 +30,7 @@ func NewRemoteMCPClient(config *catalog.ServerConfig) Client { } } -func (c *remoteMCPClient) Initialize(ctx context.Context, _ *mcp.InitializeParams, _ bool, _ *mcp.ServerSession, _ *mcp.Server, _ CapabilityRefresher) error { +func (c *remoteMCPClient) Initialize(ctx context.Context, _ *mcp.InitializeParams, verbose bool, _ *mcp.ServerSession, _ *mcp.Server, _ CapabilityRefresher) error { if c.initialized.Load() { return fmt.Errorf("client already initialized") } @@ -52,7 +52,19 @@ func (c *remoteMCPClient) Initialize(ctx context.Context, _ *mcp.InitializeParam // Secrets to env env := map[string]string{} for _, s := range c.config.Spec.Secrets { - env[s.Env] = getSecretValue(ctx, s.Name) + // First check if secret was provided via configuration (file-based or se:// URI) + if value, ok := c.config.Secrets[s.Name]; ok && value != "" { + if verbose { + log.Logf(" - %s: %s", s.Env, maskSecret(value)) + } + env[s.Env] = value + } else { + // Fall back to secrets engine (Docker Desktop direct API) + if verbose { + log.Logf(" - Fetching secret: %s", secret.GetSecretKey(s.Name)) + } + env[s.Env] = getSecretValue(ctx, s.Name) + } } // Headers @@ -63,6 +75,9 @@ func (c *remoteMCPClient) Initialize(ctx context.Context, _ *mcp.InitializeParam // Add OAuth token if remote server has OAuth configuration if c.config.Spec.OAuth != nil && len(c.config.Spec.OAuth.Providers) > 0 { + if verbose { + log.Logf(" - Using OAuth token for: %s", c.config.Name) + } credHelper := oauth.NewOAuthCredentialHelper() token, err := credHelper.GetOAuthToken(ctx, c.config.Name) if err != nil { @@ -147,6 +162,18 @@ func expandEnv(value string, secrets map[string]string) string { }) } +// maskSecret shows the first few characters of a secret followed by asterisks. +// se:// URIs are shown in full since they're just references, not actual secrets. +func maskSecret(value string) string { + if strings.HasPrefix(value, "se://") { + return value + } + if len(value) <= 4 { + return "****" + } + return value[:4] + "****" +} + // headerRoundTripper is an http.RoundTripper that adds custom headers to all requests type headerRoundTripper struct { base http.RoundTripper