Conversation
… resolve
When mcpproxy runs as a macOS App Bundle or LaunchAgent, os.Getenv("PATH")
is often `/usr/bin:/bin`. shellwrap.ResolveDockerPath already works around
that for the docker binary itself by caching an absolute path from the
user's login shell, but `docker pull` still fails for registries that
need credential helpers (e.g. docker-credential-desktop), because the
docker CLI re-execs the helper via its own PATH lookup — and the helper
lives in /usr/local/bin or /opt/homebrew/bin, which are absent from the
ambient launchd PATH.
This adds a new LoginShellPATH() helper that captures the login shell's
PATH once per process via `sh -l -c 'printf %s "$PATH"'` and caches it.
MinimalEnv() now merges that login-shell PATH with the ambient PATH
(login-shell first, deduped) so subprocesses spawned by the scanner can
locate credential helpers and other developer tools. Ambient behavior is
fully preserved on Windows and in the success-path fallback when the
login shell cannot be executed.
Fixes #381
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deploying mcpproxy-docs with
|
| Latest commit: |
d55f630
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6b809b86.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-381-shellwrap-minimalenv.mcpproxy-docs.pages.dev |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 24286156026 --repo smart-mcp-proxy/mcpproxy-go
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
shellwrap.LoginShellPATH()— captures the user's login-shell PATH once per process (viash -l -c 'printf %s "\$PATH"') and caches it.MinimalEnv()now merges that login-shell PATH with the ambient PATH (login-shell entries first, deduped), so scanner subprocesses can resolve docker credential helpers (e.g.docker-credential-desktop) even when mcpproxy was started from a LaunchAgent / macOS App Bundle with a minimal PATH.Fixes #381.
Why
ResolveDockerPath()already works around the LaunchAgent PATH problem for the docker binary itself, butdocker pullwas still failing on registries configured with credential helpers:That helper lives in
/usr/local/binor/opt/homebrew/bin— directories missing from the/usr/bin:/bin-style PATH that mcpproxy inherits when launched from a GUI. The docker CLI re-execs the helper via its own$PATHlookup, so the fix has to flow throughcmd.Env, notexec.LookPath.Test plan
go test -race ./internal/shellwrap/(new tests cover merge-dedup, successful capture via fake\$SHELL, and fallback when the shell is broken)go test -race ./internal/security/scanner/go build ./...+go build -tags server ./...golangci-lint run ./internal/shellwrap/...PATH=/usr/bin:/bin,MinimalEnv()now returns PATH enriched with/opt/homebrew/bin,/usr/local/bin, etc.🤖 Generated with Claude Code