Remove JFS references + secrets engine injection #247

saucow · 2025-11-18T04:35:08Z

What I did

Secrets Engine migration by removing all JFS (Docker Desktop socket API) dependencies and implementing se:// URI-based secret injection.
Builds upon cmd/secret: use new store #217 and Remove jfs #220

Changes

Secret Injection

Container MCPs

Secrets injected as se:// URIs, resolved by Docker Desktop at container runtime. Gateway never holds secret values in memory.

Remote MCPs

Secrets queried from Secrets Engine and expanded into HTTP headers. Actual values required for Bearer tokens and header interpolation.

OAuth Tokens

OAuth tokens retrieved from Secrets Engine in Desktop mode. Falls back to credential helpers in CE mode. Token refresh monitoring also uses Secrets Engine.

Commands Removed

docker mcp policy set/dump - Policy management incompatible with Secrets Engine access control
docker mcp secret export - Replaced by Secrets Engine queries

Commands Changed

`docker mcp secret set`

Stores secrets via docker pass to OS Keychain under docker/mcp/generic/ namespace.

`docker mcp secret ls`

Queries Secrets Engine HTTP API instead of JFS socket. JSON output format compatible with Docker Desktop UI.

`docker mcp secret rm`

Deletes secrets via docker pass rm. Only removes docker-pass provider secrets. Legacy secrets must be removed via OS credential tools.

* 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 <saurabh.davala@docker.com>

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

slimslenderslacks

I think we can merge this to a release/4.54 branch. But we can't merge this to main yet. No one outside docker can run this version of the gateway.

I'm also concerned that we're breaking compose file secrets.

slimslenderslacks · 2025-12-01T20:44:56Z

cmd/docker-mcp/backup/dump.go

Can we get rid of the entire backup package. It's superseded by profiles and secrets and now that the policy dump and set commands are gone, can we verify whether this entire package can be removed?

Agree, I think we are safe to remove it. Could be another PR though.

sounds good, will address in a follow-up PR

slimslenderslacks · 2025-12-01T21:02:48Z

pkg/mcp/remote.go

-	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)


no err here. Don't need the value

Updated to directly set

slimslenderslacks · 2025-12-01T21:05:58Z

cmd/docker-mcp/commands/secret.go

 	}
 	flags := cmd.Flags()
 	flags.StringVar(&opts.Provider, "provider", "", "Supported: credstore, oauth/<provider>")
+	_ = flags.MarkDeprecated("provider", "option will be ignored")


is there something we can say about how providers are now supported with "docker pass"

Good point - updated to say: all secrets now stored via docker pass in OS Keychain"

slimslenderslacks · 2025-12-01T21:31:23Z

pkg/gateway/configuration.go

-	}
+	// Secrets no longer read during configuration load
+	// Instead, se:// URIs are passed to containers and resolved at runtime
+	secrets := make(map[string]string)


Could this be where we construct the se:// uris so that we do it in only one place?

Yes added a helper method: buildSecretsURIs here

slimslenderslacks · 2025-12-01T21:34:22Z

pkg/gateway/configuration.go

-		if err != nil {
-			return Configuration{}, fmt.Errorf("reading MCP Toolkit's secrets: %w", err)
-		}
-	} else {


I think this breaks some compose examples because we're not changing the --secrets options to the gateway yet.

Ah yes, restored the: readSecretsFromFile flow. To verify I did the following locally:

Created secrets.env file:

brave.api_key=your-brave-api-key apify.api_key=your-apify-api-key

Ran following command:

docker-mcp gateway run \ --secrets=secrets.env \ --servers=brave,apify \ --verbose

cmrigney

Nice! The only blocking thing is the chore (because it can cause bugs). I haven't been able to test it locally yet, but everything else in the code lgtm.

cmrigney · 2025-12-01T21:39:12Z

cmd/docker-mcp/backup/dump.go

Agree, I think we are safe to remove it. Could be another PR though.

cmrigney · 2025-12-01T21:42:39Z

cmd/docker-mcp/commands/root.go

 		},
 		Version: version.Version,
 	}
+	cmd.SetContext(ctx)


question: What was the reason for this change? Curious only because I'm pretty sure this cmd is different than the cmd inside of PersistentPreRunE. e.g. the one in that function is the actual subcommand that got run.

Yeah good point, reverted this

cmrigney · 2025-12-01T21:43:52Z

cmd/docker-mcp/commands/secret.go

 `

-func secretCommand(docker docker.Client) *cobra.Command {
+func secretCommand(_ docker.Client) *cobra.Command {


nitpick (non-blocking): Could we just remove this param?

Yeah no longer needed, removed

cmrigney · 2025-12-01T21:47:35Z

cmd/docker-mcp/commands/secret.go

-		Short:   "Manage secrets",
+		Short:   "Manage secrets in the local OS Keychain",
 		Example: strings.Trim(setSecretExample, "\n"),
+		PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {


chore: I unfortunately just learned the hard way that this can break things. See Slack thread.
I used an isSubcommandOf approach to work around this: https://github.com/docker/mcp-gateway/pull/266/files

Ah I see, thanks for calling that out. Updated to have:

// 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 } }

in: cmd/docker-mcp/commands/root.go

cmrigney · 2025-12-01T22:06:42Z

pkg/gateway/configuration_workingset.go

-	}
-
-	return secretsByName, nil
+func (c *WorkingSetConfiguration) readDockerDesktopSecrets(_ context.Context, _ []workingset.Server) (map[string]string, error) {


question: Does this mean we'll have some more cleanup after we merge this PR?

Good point, since with secrets engine we don't need to directly read secrets anymore, this is all dead code. I've removed this method all-together and instead we are returning se:// URI's for secrets as these are "injected" when starting up the container. Ex:
docker run -d -e POSTGRES_PASSWORD=se://docker/mcp/generic/postgres_password -p 5432 postgres

Benehiko · 2025-12-02T13:20:11Z

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

+	// 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
+	}


I haven't been able to reproduce this. This won't match secrets that have nested namespaces, e.g. docker/mcp/generic/myapp/a-secret-key.

Got it, updated to have ** pattern for each type. Once we start getting all secrets when requesting docker/mcp/** will udpate

Benehiko and others added 4 commits November 17, 2025 13:29

inject secrets by ID + return empty secrets map

9154242

Remove jfs references and commnds which utilize jfs like: dump, restore

49d560d

Use secrets client for remote case

51295fb

saucow requested a review from Benehiko November 18, 2025 04:35

saucow changed the title ~~Secrets engine injection~~ Remove JFS references + secrets engine injection Nov 18, 2025

saucow added 4 commits November 19, 2025 18:09

Support OAuth DCR flows wip

5854556

remove JFS windows connection

1a546dd

remake docs for CI

d2602e1

remove additional JFS references

527a07f

saucow commented Nov 21, 2025

View reviewed changes

cmd/docker-mcp/secret-management/secret/secretsengine.go Outdated Show resolved Hide resolved

Merge branch 'main' into secrets-engine-injection

d888b86

saucow changed the base branch from secrets-engine to main November 24, 2025 22:54

saucow added 2 commits November 24, 2025 17:32

updates

baae072

fix return type

e890cf7

saucow requested review from cmrigney and slimslenderslacks November 25, 2025 01:46

saucow marked this pull request as ready for review November 25, 2025 01:46

saucow requested a review from a team as a code owner November 25, 2025 01:46

saucow requested a review from bobbyhouse December 1, 2025 16:46

slimslenderslacks requested changes Dec 1, 2025

View reviewed changes

saucow changed the base branch from main to release/4.54 December 1, 2025 21:36

cmrigney requested changes Dec 1, 2025

View reviewed changes

saucow added 2 commits December 1, 2025 21:23

PR feedback: provider help, uri construction, compose secrets

312e404

Address PR feedback

c90a282

Benehiko reviewed Dec 2, 2025

View reviewed changes

saucow added 4 commits December 2, 2025 15:39

subcommand fix, remove dead code, add test for file based env secrets

724f259

Update comment for clarity and secretsengine pattern to include nested

6cd8aa8

Merge branch 'main' into secrets-engine-injection

5939775

log secret path in verbose mode + mask secrets

261cd00

saucow requested review from Benehiko, cmrigney and slimslenderslacks December 3, 2025 05:13

Remove JFS references + secrets engine injection #247

Are you sure you want to change the base?

Remove JFS references + secrets engine injection #247

Uh oh!

Conversation

saucow commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

Changes

Secret Injection

Container MCPs

Remote MCPs

OAuth Tokens

Commands Removed

Commands Changed

docker mcp secret set

docker mcp secret ls

docker mcp secret rm

Uh oh!

Uh oh!

slimslenderslacks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmrigney left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

saucow commented Nov 18, 2025 •

edited

Loading

`docker mcp secret set`

`docker mcp secret ls`

`docker mcp secret rm`

cmrigney left a comment •

edited

Loading