fix(docker): pass env values to docker via environment, not -e KEY=VALUE argv

docs/docker-isolation.md

@@ -321,7 +321,17 @@ Environment variables from server configuration are automatically passed to cont
 }
 ```
 
-These become Docker arguments: `-e API_KEY=your-secret-key -e DEBUG=true`
+These become key-only Docker arguments, `-e API_KEY -e DEBUG`, and the values are set in the
+environment of the `docker` process, which reads them from there. The same applies to a server
+whose `command` is `docker run …` itself. A value never appears in the `docker` command line, so
+`ps aux` and `/proc/<pid>/cmdline`, which every local user can read, do not show it
+([#1809](https://github.com/smart-mcp-proxy/mcpproxy-go/issues/1809)). It is still visible to the
+process owner and root in `/proc/<pid>/environ` of the `docker` client, and inside the container
+(`docker inspect`), as any container environment variable is. `PATH` is the one value still passed
+inline (`-e PATH=…`), so a configured container `PATH` is exact; it is not a secret. Whenever
+`docker` is started through your login shell (it could not be resolved to an absolute path, or on
+Linux with neither `DOCKER_HOST` nor `DOCKER_CONTEXT` exported), a profile that exports or unsets
+one of these names before `docker` runs wins over the configured value.
 
 ## Docker-in-Docker Prevention
 

docs/features/keyring-integration.md

@@ -50,8 +50,12 @@ MCPProxy supports three types of secret references that can be used anywhere in
 
 ### File references
 
-`${file:/absolute/path}` keeps a secret out of the config file and out of the process environment
-(`docker inspect`, `/proc/<pid>/environ`). The same rules apply wherever it is used:
+`${file:/absolute/path}` keeps a secret out of the config file and out of mcpproxy's own process
+environment. A reference used in a server's `env` is resolved when the server connects and the value
+is passed to that server as an environment variable, which for a Docker server also means the
+container's environment (`docker inspect`) and the `docker` client's `/proc/<pid>/environ`; it is
+never put on the `docker` command line ([#1809](https://github.com/smart-mcp-proxy/mcpproxy-go/issues/1809)).
+The same rules apply wherever it is used:
 
 - The path must be absolute. A symlink is followed (Kubernetes secret volumes are symlinks) but must
   point at a regular file. On Windows an absolute path has a drive letter (`C:\ProgramData\mcpproxy\key`

internal/upstream/core/connection_docker.go

@@ -1,10 +1,10 @@
 package core
 
 import (
-	"fmt"
 	"os"
 	"path/filepath"
 	"runtime"
+	"sort"
 	"strings"
 
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/shellwrap"
@@ -324,7 +324,13 @@ func (c *Client) insertCidfileIntoDockerArgs(args []string, cidFile string) []st
 
 // injectEnvVarsIntoDockerArgs injects environment variables as -e flags into Docker run args
 // The flags are inserted after "run" but before the image name
-// Example: ["run", "-i", "--rm", "image"] -> ["run", "-e", "KEY=VAL", "-i", "--rm", "image"]
+// Example: ["run", "-i", "--rm", "image"] -> ["run", "-e", "KEY", "-i", "--rm", "image"]
+//
+// Only the KEY is placed in argv (#1809). Docker's `-e KEY` form reads the
+// value from the docker CLI's own environment, which the spawn paths populate
+// from config.Env (connection_stdio.go / connection_launcher.go env merge).
+// A `-e KEY=VALUE` token would land in the docker process's argv, where
+// `ps aux` and /proc/<pid>/cmdline expose resolved secrets to every local user.
 func (c *Client) injectEnvVarsIntoDockerArgs(args []string, envVars map[string]string) []string {
 	if len(args) < 2 || args[0] != cmdRun {
 		return args
@@ -333,18 +339,47 @@ func (c *Client) injectEnvVarsIntoDockerArgs(args []string, envVars map[string]s
 	// Build new args with env vars injected after "run"
 	newArgs := make([]string, 0, len(args)+len(envVars)*2)
 	newArgs = append(newArgs, args[0]) // "run"
-
-	// Add -e flags for each env var
-	for key, value := range envVars {
-		newArgs = append(newArgs, "-e", fmt.Sprintf("%s=%s", key, value))
-	}
+	newArgs = append(newArgs, dockerEnvKeyFlags(envVars)...)
 
 	// Add remaining args (flags and image name)
 	newArgs = append(newArgs, args[1:]...)
 
 	return newArgs
 }
 
+// dockerEnvKeyFlags renders config.Env as key-only `-e KEY` docker flags, in
+// sorted key order so argv and its log line are deterministic. The values
+// travel in the docker process environment, never in argv (#1809).
+//
+// PATH is the one exception: the spawn paths rewrite the docker client's PATH
+// after the config.Env merge (prependDockerDirToPath, #715), so `-e PATH` would
+// forward that host-side value into the container instead of the configured
+// one. PATH is never a secret, so it is inlined as `-e PATH=<value>`. The match
+// is case-insensitive (Windows treats `Path` and `PATH` as one variable) and
+// keeps the configured spelling.
+//
+// Whenever docker is started through the user's login shell (resolveDockerSpawn:
+// docker not resolvable to a verified absolute path, or a non-Darwin host with
+// neither DOCKER_HOST nor DOCKER_CONTEXT exported) a profile that exports or
+// unsets one of these names before docker runs wins over the configured value;
+// the direct-exec path has no such step.
+func dockerEnvKeyFlags(envVars map[string]string) []string {
+	keys := make([]string, 0, len(envVars))
+	for key := range envVars {
+		keys = append(keys, key)
+	}
+	sort.Strings(keys)
+	flags := make([]string, 0, len(keys)*2)
+	for _, key := range keys {
+		if strings.EqualFold(key, "PATH") {
+			flags = append(flags, "-e", key+"="+envVars[key])
+			continue
+		}
+		flags = append(flags, "-e", key)
+	}
+	return flags
+}
+
 // insertCidfileIntoShellDockerCommand inserts --cidfile into a shell-wrapped Docker command
 func (c *Client) insertCidfileIntoShellDockerCommand(shellArgs []string, cidFile string) []string {
 	// Shell args look like:

internal/upstream/core/connection_docker_test.go

@@ -320,7 +320,7 @@ func TestSetupDockerIsolationDirectExecsWhenDockerHostInEnv(t *testing.T) {
 // isolation-injection path. With docker resolved to a verified absolute
 // executable and the daemon env guaranteed (macOS), the spawn must direct-exec
 // the ABSOLUTE docker path (no `$SHELL -l -c` wrap), carry the injected
-// `-e KEY=VALUE` env flags, and accept an args-based cidfile insertion. Before
+// key-only `-e KEY` env flags (#1809), and accept an args-based cidfile insertion. Before
 // this fix the user path shell-wrapped bare `docker`, producing GH #696's
 // `zsh:1: command not found: docker` on a default Docker Desktop macOS install
 // (docker CLI only inside the app bundle, not on the login-shell PATH).
@@ -346,8 +346,8 @@ func TestResolveDockerSpawnUserSuppliedDockerRunDirectExec(t *testing.T) {
 	assert.Equal(t, cmdRun, args[0],
 		"first arg must be the raw 'run' token (not a shell -c string), got: %v", args)
 	// The injected -e env flag must survive into the docker run argv.
-	assert.Subset(t, args, []string{"-e", "SLACK_TOKEN=xoxb-secret"},
-		"injected -e env flags must be present in the direct-exec argv, got: %v", args)
+	assert.Subset(t, args, []string{"-e", "SLACK_TOKEN"},
+		"injected key-only -e env flags must be present in the direct-exec argv, got: %v", args)
 	for _, a := range args {
 		assert.NotContains(t, a, " run ",
 			"args must be raw argv tokens, not a single shell command string, got: %v", args)
@@ -383,10 +383,10 @@ func TestResolveDockerSpawnUserSuppliedFallsBackToShellWrap(t *testing.T) {
 	cmdStr := shellArgs[len(shellArgs)-1]
 	assert.True(t, strings.HasPrefix(cmdStr, "docker run"),
 		"on resolution failure the spawn must fall back to bare 'docker', got: %s", cmdStr)
-	// The real command string still carries the actual env value (redaction is a
-	// log-only concern, not a spawn concern).
-	assert.Contains(t, cmdStr, "-e SLACK_TOKEN=xoxb-secret",
-		"injected -e env flag must survive into the shell command string, got: %s", cmdStr)
+	// The command string carries the key only; the value reaches docker through
+	// the spawn environment (#1809).
+	assert.Contains(t, cmdStr, "-e SLACK_TOKEN ",
+		"injected key-only -e env flag must survive into the shell command string, got: %s", cmdStr)
 }
 
 // TestDockerDaemonEnvGuaranteed unit-tests the gate directly.

internal/upstream/core/connection_launcher_test.go

@@ -47,8 +47,8 @@ func TestBuildLauncherCmdUserSuppliedDockerRunDirectExec(t *testing.T) {
 		"second argv token must be the raw 'run' token, not a shell -c string, got: %v", cmd.Args)
 
 	// Injected -e env flag must survive into the docker run argv.
-	assert.Subset(t, cmd.Args, []string{"-e", "SLACK_TOKEN=xoxb-secret"},
-		"injected -e env flags must be present in the direct-exec argv, got: %v", cmd.Args)
+	assert.Subset(t, cmd.Args, []string{"-e", "SLACK_TOKEN"},
+		"injected key-only -e env flags must be present in the direct-exec argv, got: %v", cmd.Args)
 
 	// cidfile inserted right after 'run' via the args-based helper.
 	require.NotEmpty(t, cidFile)
@@ -92,8 +92,8 @@ func TestBuildLauncherCmdUserSuppliedDockerRunShellWrapFallback(t *testing.T) {
 	last := cmd.Args[len(cmd.Args)-1]
 	assert.Contains(t, last, "docker run",
 		"fallback must shell-wrap a 'docker run' command string, got: %s", last)
-	assert.Contains(t, last, "-e SLACK_TOKEN=xoxb-secret",
-		"injected -e env flag must survive into the shell command string, got: %s", last)
+	assert.Contains(t, last, "-e SLACK_TOKEN ",
+		"injected key-only -e env flag must survive into the shell command string, got: %s", last)
 }
 
 // TestBuildLauncherCmdPlainCommandStillShellWraps is the no-regression guard: a

internal/upstream/core/docker_env_argv_test.go

@@ -0,0 +1,115 @@
+package core
+
+import (
+	"context"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/secureenv"
+)
+
+// Issue #1809: a server's `env` values (including resolved ${keyring:…},
+// ${env:…} and ${file:…} secrets) must never appear in the docker process's
+// argv, where `ps aux` / `/proc/<pid>/cmdline` expose them to every local
+// user. Docker's `-e KEY` (no `=`) form takes the value from the docker CLI's
+// own environment, which the spawn paths already populate from config.Env.
+
+const argvSecret = "xoxb-secret"
+
+func assertNoSecretInArgv(t *testing.T, args []string) {
+	t.Helper()
+	for _, a := range args {
+		assert.NotContains(t, a, argvSecret,
+			"env value must not appear in docker argv, got: %v", args)
+	}
+}
+
+func TestInjectEnvVarsIntoDockerArgs_KeyOnlyNoValueInArgv(t *testing.T) {
+	c := newUserSuppliedDockerTestClient()
+	c.config.Env = map[string]string{"SLACK_TOKEN": argvSecret, "A_FIRST": "v1"}
+
+	args := c.injectEnvVarsIntoDockerArgs(c.config.Args, c.config.Env)
+
+	assertNoSecretInArgv(t, args)
+	assert.Equal(t, cmdRun, args[0])
+	// Keys are emitted sorted so argv (and the log line) is deterministic.
+	assert.Equal(t, []string{cmdRun, "-e", "A_FIRST", "-e", "SLACK_TOKEN", "-i", "--rm", "mcp/example"}, args)
+}
+
+// PATH is rewritten by prependDockerDirToPath after the env merge, so a
+// configured container PATH must be inlined to stay exact; it is not a secret.
+func TestInjectEnvVarsIntoDockerArgs_PathStaysInline(t *testing.T) {
+	c := newUserSuppliedDockerTestClient()
+	c.config.Env = map[string]string{"PATH": "/opt/tool/bin:/usr/bin", "SLACK_TOKEN": argvSecret}
+
+	args := c.injectEnvVarsIntoDockerArgs(c.config.Args, c.config.Env)
+
+	assertNoSecretInArgv(t, args)
+	assert.Equal(t, []string{cmdRun, "-e", "PATH=/opt/tool/bin:/usr/bin", "-e", "SLACK_TOKEN", "-i", "--rm", "mcp/example"}, args)
+
+	// Windows spells it `Path`; the match is case-insensitive and the configured
+	// spelling is kept.
+	c.config.Env = map[string]string{"Path": `C:\tool;C:\Windows`}
+	args = c.injectEnvVarsIntoDockerArgs(c.config.Args, c.config.Env)
+	assert.Equal(t, []string{cmdRun, "-e", `Path=C:\tool;C:\Windows`, "-i", "--rm", "mcp/example"}, args)
+}
+
+func TestBuildDockerArgs_IsolationKeyOnlyNoValueInArgv(t *testing.T) {
+	im := NewIsolationManager(config.DefaultDockerIsolationConfig())
+	serverConfig := &config.ServerConfig{
+		Name:    "iso-server",
+		Command: "python",
+		Args:    []string{"-m", "mcp_server"},
+		Env:     map[string]string{"SLACK_TOKEN": argvSecret},
+	}
+
+	args, err := im.BuildDockerArgs(serverConfig, "python")
+	require.NoError(t, err)
+
+	assertNoSecretInArgv(t, args)
+	assert.Subset(t, args, []string{"-e", "SLACK_TOKEN"},
+		"isolation argv must carry the key-only -e flag, got: %v", args)
+}
+
+func TestBuildLauncherCmd_UserDockerRun_SecretInEnvNotArgv(t *testing.T) {
+	fakeDocker := writeFakeDockerExecutable(t)
+	forceDockerDaemonEnvGOOS(t, osDarwin)
+	orig := resolveDockerBinary
+	t.Cleanup(func() { resolveDockerBinary = orig })
+	resolveDockerBinary = func(_ *zap.Logger) (string, error) { return fakeDocker, nil }
+
+	c := newUserSuppliedDockerTestClient()
+	c.envManager = secureenv.NewManager(nil)
+
+	cmd, _, _, err := c.buildLauncherCmd(context.Background(), true)
+	require.NoError(t, err)
+
+	assertNoSecretInArgv(t, cmd.Args)
+	assert.Subset(t, cmd.Args, []string{"-e", "SLACK_TOKEN"})
+	assert.Contains(t, cmd.Env, "SLACK_TOKEN="+argvSecret,
+		"the value must reach docker through its environment, got env: %v", cmd.Env)
+}
+
+func TestBuildLauncherCmd_UserDockerRun_ShellWrapSecretInEnvNotArgv(t *testing.T) {
+	orig := resolveDockerBinary
+	t.Cleanup(func() { resolveDockerBinary = orig })
+	resolveDockerBinary = func(_ *zap.Logger) (string, error) { return "", fmt.Errorf("docker not found") }
+
+	c := newUserSuppliedDockerTestClient()
+	c.envManager = secureenv.NewManager(nil)
+
+	cmd, _, _, err := c.buildLauncherCmd(context.Background(), true)
+	require.NoError(t, err)
+
+	assertNoSecretInArgv(t, cmd.Args)
+	last := cmd.Args[len(cmd.Args)-1]
+	assert.True(t, strings.Contains(last, "-e SLACK_TOKEN "),
+		"shell command string must carry the key-only -e flag, got: %s", last)
+	assert.Contains(t, cmd.Env, "SLACK_TOKEN="+argvSecret)
+}

internal/upstream/core/isolation.go

@@ -623,10 +623,10 @@ func (im *IsolationManager) BuildDockerArgs(serverConfig *config.ServerConfig, r
 		args = append(args, "--workdir", serverConfig.Isolation.WorkingDir)
 	}
 
-	// Add environment variables from server config
-	for key, value := range serverConfig.Env {
-		args = append(args, "-e", fmt.Sprintf("%s=%s", key, value))
-	}
+	// Add environment variables from server config as key-only `-e KEY` flags.
+	// The values are in the docker process environment (the connect paths merge
+	// config.Env into the spawn env), never in argv (#1809).
+	args = append(args, dockerEnvKeyFlags(serverConfig.Env)...)
 
 	// Add global extra args
 	args = append(args, im.globalConfig.ExtraArgs...)