fix(docker): capture DOCKER_* daemon vars from login shell for direct-exec spawn

Codex round-2 (P2) on PR #699: the round-1 fix only added DOCKER_HOST/
DOCKER_CONTEXT/… to the secure-env allowlist, which preserves vars already in
os.Environ(). When mcpproxy is launched from the macOS tray / LaunchAgent those
vars exist ONLY in the user's login shell rc files, so the allowlist was a
no-op and the direct-exec docker spawn still hit the default socket
(Colima/rootless/remote daemons unreachable).

Fix: capture the docker daemon-access vars FROM the login shell, mirroring the
existing LoginShellPATH machinery.

- internal/shellwrap/shellwrap.go — new LoginShellEnvVars(logger, keys): one
  `<shell> -l -c` invocation prints the requested vars bracketed by markers and
  separated by 0x1F (banner-safe), parsed and cached per-key process-wide.
  Returns only non-empty values; no-op on Windows / on capture failure.
- internal/secureenv/manager.go — BuildSecureEnvironment, gated on EnhancePath
  (the upstream-spawn signal), injects any docker daemon var still missing from
  the env via the login shell. Vars already present (allowlist + os.Environ)
  are left untouched, so it only forks when something is actually missing, and
  the underlying capture is cached. Overridable loginShellEnvFn for tests.

The round-1 allowlist append (client.go) is kept: it still covers vars that ARE
in os.Environ() under a custom operator allowlist. The two paths are
complementary.

TDD: TestLoginShellEnvVars_CapturesFromShellOnly (var exported only inside the
fake login shell, absent from os.Environ) + TestBuildSecureEnvironment_Injects
DockerVarFromLoginShell (restrictive allowlist proves the login-shell path, not
the allowlist, supplied DOCKER_HOST) + EnhancePath-gating guard. All offline.

Related #696

Author: Dumbris

internal/secureenv/launchd_path_test.go

@@ -225,6 +225,72 @@ func withFakeLoginShellPath(path string) func() {
 	return func() { loginShellPATHFn = prev }
 }
 
+// withFakeLoginShellEnv swaps loginShellEnvFn for a stub returning `vals`.
+func withFakeLoginShellEnv(vals map[string]string) func() {
+	prev := loginShellEnvFn
+	loginShellEnvFn = func(keys []string) map[string]string {
+		out := make(map[string]string, len(keys))
+		for _, k := range keys {
+			if v, ok := vals[k]; ok {
+				out[k] = v
+			}
+		}
+		return out
+	}
+	return func() { loginShellEnvFn = prev }
+}
+
+// TestBuildSecureEnvironment_InjectsDockerVarFromLoginShell is the MCP-2744
+// round-2 regression: a docker daemon-access var (DOCKER_HOST) present ONLY in
+// the login shell — absent from os.Environ() and not allowlisted — must still
+// be injected into the direct-exec docker-isolation spawn env. The restrictive
+// allowlist guarantees os.Environ() cannot supply it, so a pass proves the
+// login-shell capture path (not the allowlist) did the work.
+func TestBuildSecureEnvironment_InjectsDockerVarFromLoginShell(t *testing.T) {
+	defer withFakeLoginShellPath("/usr/bin:/bin")()
+	defer withFakeLoginShellEnv(map[string]string{"DOCKER_HOST": "tcp://login-shell-only:2375"})()
+
+	m := NewManager(&EnvConfig{
+		InheritSystemSafe: true,
+		EnhancePath:       true,
+		AllowedSystemVars: []string{"PATH", "HOME"}, // DOCKER_* cannot come from os.Environ
+	})
+
+	var got string
+	for _, kv := range m.BuildSecureEnvironment() {
+		if strings.HasPrefix(kv, "DOCKER_HOST=") {
+			got = kv
+		}
+	}
+	assert.Equal(t, "DOCKER_HOST=tcp://login-shell-only:2375", got,
+		"DOCKER_HOST present only in the login shell must be injected into the spawn env")
+}
+
+// TestBuildSecureEnvironment_NoDockerLoginShellWhenEnhanceDisabled verifies the
+// login-shell docker capture is gated on EnhancePath, so non-spawn callers do
+// not pay the login-shell fork.
+func TestBuildSecureEnvironment_NoDockerLoginShellWhenEnhanceDisabled(t *testing.T) {
+	called := false
+	prev := loginShellEnvFn
+	loginShellEnvFn = func(keys []string) map[string]string {
+		called = true
+		return map[string]string{"DOCKER_HOST": "tcp://should-not-be-used:2375"}
+	}
+	t.Cleanup(func() { loginShellEnvFn = prev })
+
+	m := NewManager(&EnvConfig{
+		InheritSystemSafe: true,
+		EnhancePath:       false,
+		AllowedSystemVars: []string{"PATH", "HOME"},
+	})
+
+	for _, kv := range m.BuildSecureEnvironment() {
+		require.False(t, strings.HasPrefix(kv, "DOCKER_HOST=tcp://should-not-be-used"),
+			"login-shell docker var must not be injected when EnhancePath is false")
+	}
+	assert.False(t, called, "login-shell env capture must not run when EnhancePath is false")
+}
+
 func getPATH(envVars []string) string {
 	for _, kv := range envVars {
 		if strings.HasPrefix(kv, "PATH=") {

internal/secureenv/manager.go

@@ -13,6 +13,11 @@ import (
 // caches the result for the process lifetime via sync.Once.
 var loginShellPATHFn = func() string { return shellwrap.LoginShellPATH(nil) }
 
+// loginShellEnvFn captures named env vars from the user's interactive login
+// shell. Overridable in tests. Default delegates to shellwrap.LoginShellEnvVars
+// which caches per-key for the process lifetime.
+var loginShellEnvFn = func(keys []string) map[string]string { return shellwrap.LoginShellEnvVars(nil, keys) }
+
 const (
 	osWindows = "windows"
 	osDarwin  = "darwin"
@@ -287,6 +292,52 @@ func (m *Manager) BuildSecureEnvironment() []string {
 		envVars = m.ensureComprehensivePath(envVars)
 	}
 
+	// MCP-2744: EnhancePath marks an upstream spawn env. The docker-isolation
+	// spawn now direct-execs the resolved docker binary (no login-shell wrap),
+	// so docker daemon-access vars the user exports ONLY in their rc files
+	// (absent from os.Environ() — the macOS tray/LaunchAgent case) would be lost
+	// and `docker run` would silently fall back to the default socket. Capture
+	// any still-missing ones from the login shell. Vars already present (via the
+	// allowlist when they ARE in os.Environ()) are left untouched.
+	if m.config.EnhancePath {
+		envVars = m.ensureDockerDaemonEnv(envVars)
+	}
+
+	return envVars
+}
+
+// ensureDockerDaemonEnv injects docker daemon-access vars (DOCKER_HOST etc.)
+// captured from the user's login shell for any that are not already present in
+// envVars. It only forks a login shell when at least one such var is missing,
+// and the underlying capture is cached process-wide.
+func (m *Manager) ensureDockerDaemonEnv(envVars []string) []string {
+	dockerVars := DockerDaemonEnvVars()
+
+	present := make(map[string]struct{}, len(dockerVars))
+	for _, ev := range envVars {
+		for _, dv := range dockerVars {
+			if strings.HasPrefix(ev, dv+"=") {
+				present[dv] = struct{}{}
+			}
+		}
+	}
+
+	missing := make([]string, 0, len(dockerVars))
+	for _, dv := range dockerVars {
+		if _, ok := present[dv]; !ok {
+			missing = append(missing, dv)
+		}
+	}
+	if len(missing) == 0 {
+		return envVars
+	}
+
+	fromShell := loginShellEnvFn(missing)
+	for _, dv := range missing {
+		if v, ok := fromShell[dv]; ok && v != "" {
+			envVars = append(envVars, dv+"="+v)
+		}
+	}
 	return envVars
 }
 

internal/shellwrap/shellwrap.go

@@ -437,6 +437,132 @@ func resetLoginShellPathCacheForTest() {
 	loginShellPathVal = ""
 }
 
+// --- Login-shell env-var capture -----------------------------------------
+
+// loginShellEnv* markers bracket the captured values inside the login shell's
+// stdout so banner/motd/rc output (oh-my-zsh, direnv, etc.) does not leak into
+// the parsed result. Values are separated by ASCII Unit Separator (0x1F), which
+// cannot appear in a docker connection pointer (host URL, context, path).
+const (
+	loginShellEnvMarkerBegin = "__MCPPROXY_LOGIN_ENV_BEGIN__"
+	loginShellEnvMarkerEnd   = "__MCPPROXY_LOGIN_ENV_END__"
+	loginShellEnvFieldSep    = "\x1f"
+)
+
+var (
+	loginShellEnvMu    sync.Mutex
+	loginShellEnvVals  = map[string]string{} // captured non-empty values
+	loginShellEnvTried = map[string]bool{}   // keys already attempted (incl. empty/unset)
+)
+
+// LoginShellEnvVars returns the values of the named environment variables as
+// seen by the user's interactive login shell (`<shell> -l -c …`). Each key is
+// captured at most once per process and cached; only variables with a non-empty
+// value are present in the returned map.
+//
+// Why this exists (mirrors LoginShellPATH): a macOS App Bundle / LaunchAgent
+// inherits a minimal os.Environ() that omits variables the user exports in
+// their rc files. For docker isolation that means DOCKER_HOST / DOCKER_CONTEXT
+// (Colima, rootless, remote daemons) are invisible to a direct-exec spawn even
+// though `docker` works fine in the user's terminal. Capturing them from the
+// login shell restores parity without re-introducing the brittle login-shell
+// command wrap. Returns an empty map on Windows or when capture fails.
+func LoginShellEnvVars(logger *zap.Logger, keys []string) map[string]string {
+	out := make(map[string]string, len(keys))
+	if runtime.GOOS == osWindows || len(keys) == 0 {
+		return out
+	}
+
+	loginShellEnvMu.Lock()
+	defer loginShellEnvMu.Unlock()
+
+	var missing []string
+	for _, k := range keys {
+		if !loginShellEnvTried[k] {
+			missing = append(missing, k)
+		}
+	}
+	if len(missing) > 0 {
+		captured := captureLoginShellEnv(logger, missing)
+		for _, k := range missing {
+			loginShellEnvTried[k] = true
+			if v, ok := captured[k]; ok && v != "" {
+				loginShellEnvVals[k] = v
+			}
+		}
+	}
+
+	for _, k := range keys {
+		if v, ok := loginShellEnvVals[k]; ok && v != "" {
+			out[k] = v
+		}
+	}
+	return out
+}
+
+// captureLoginShellEnv runs one `<shell> -l -c` invocation that prints the
+// requested variables (bracketed by markers, separated by 0x1F) and parses the
+// result. keys are fixed internal constants (never user input).
+func captureLoginShellEnv(logger *zap.Logger, keys []string) map[string]string {
+	result := make(map[string]string, len(keys))
+	shell := resolveLoginShell()
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	var b strings.Builder
+	b.WriteString("printf %s '" + loginShellEnvMarkerBegin + "'; ")
+	for _, k := range keys {
+		// \037 (octal 0x1F) is the field separator; printf interprets it.
+		b.WriteString("printf '" + k + "=%s\\037' \"$" + k + "\"; ")
+	}
+	b.WriteString("printf %s '" + loginShellEnvMarkerEnd + "'")
+
+	cmd := exec.CommandContext(ctx, shell, "-l", "-c", b.String())
+	out, err := cmd.Output()
+	if err != nil {
+		if logger != nil {
+			logger.Debug("shellwrap: login-shell env capture failed",
+				zap.String("shell", shell), zap.Error(err))
+		}
+		return result
+	}
+
+	body := extractBetweenMarkers(string(out), loginShellEnvMarkerBegin, loginShellEnvMarkerEnd)
+	for _, field := range strings.Split(body, loginShellEnvFieldSep) {
+		eq := strings.IndexByte(field, '=')
+		if eq <= 0 {
+			continue
+		}
+		if val := field[eq+1:]; val != "" {
+			result[field[:eq]] = val
+		}
+	}
+	return result
+}
+
+// extractBetweenMarkers returns the substring of s between begin and end, or ""
+// if either marker is absent.
+func extractBetweenMarkers(s, begin, end string) string {
+	i := strings.Index(s, begin)
+	if i < 0 {
+		return ""
+	}
+	rest := s[i+len(begin):]
+	j := strings.Index(rest, end)
+	if j < 0 {
+		return ""
+	}
+	return rest[:j]
+}
+
+// resetLoginShellEnvCacheForTest is only referenced from shellwrap_test.go.
+func resetLoginShellEnvCacheForTest() {
+	loginShellEnvMu.Lock()
+	defer loginShellEnvMu.Unlock()
+	loginShellEnvVals = map[string]string{}
+	loginShellEnvTried = map[string]bool{}
+}
+
 // --- Minimal environment for scanner subprocesses ------------------------
 
 // MinimalEnv returns a minimal, allow-listed environment suitable for

internal/shellwrap/shellwrap_test.go

@@ -349,6 +349,43 @@ func TestMergePathUnique(t *testing.T) {
 	}
 }
 
+// TestLoginShellEnvVars_CapturesFromShellOnly is the MCP-2744 round-2
+// regression guard: docker daemon-access vars that the user exports ONLY in
+// their login shell (absent from os.Environ — the macOS tray/LaunchAgent case)
+// must be captured from the login shell. The fake shell exports the vars
+// internally so they are NOT inherited from the parent process environment.
+func TestLoginShellEnvVars_CapturesFromShellOnly(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("unix-only: login-shell env capture is a no-op on Windows")
+	}
+	resetLoginShellEnvCacheForTest()
+	t.Cleanup(resetLoginShellEnvCacheForTest)
+
+	dir := t.TempDir()
+	// Fake login shell that exports docker vars itself (so they exist only in
+	// the shell, not in the parent os.Environ) and then evaluates the -c script.
+	script := `#!/bin/sh
+export DOCKER_HOST=tcp://shell-only:2375
+export DOCKER_CONTEXT=colima
+while [ $# -gt 0 ]; do
+  case "$1" in
+    -l|--login) shift ;;
+    -c) shift; eval "$1"; shift ;;
+    *) shift ;;
+  esac
+done
+`
+	p := filepath.Join(dir, "fake-login-shell")
+	require.NoError(t, os.WriteFile(p, []byte(script), 0o755))
+	t.Setenv("SHELL", p)
+
+	got := LoginShellEnvVars(nil, []string{"DOCKER_HOST", "DOCKER_CONTEXT", "DOCKER_CONFIG"})
+	assert.Equal(t, "tcp://shell-only:2375", got["DOCKER_HOST"])
+	assert.Equal(t, "colima", got["DOCKER_CONTEXT"])
+	_, hasEmpty := got["DOCKER_CONFIG"]
+	assert.False(t, hasEmpty, "an unset var must be omitted from the result map, got: %v", got)
+}
+
 // writeFakeShell writes a POSIX shell script at <dir>/fake-shell that
 // ignores its arguments and echoes `wantPath` on stdout. It returns the
 // absolute path to the script so tests can point $SHELL at it.