fix(config): restore full shell expansion in MCP config values#2782
Merged
Conversation
Bojun-Vvibe
added a commit
to Bojun-Vvibe/oss-contributions
that referenced
this pull request
May 2, 2026
ff2799a to
36d2fa6
Compare
Groundwork for replacing the hand-rolled parser in internal/config/resolve.go. Introduces shell.ExpandValue, a single-value expansion entry point built on mvdan.cc/sh/v3's syntax + expand packages. Runs with nounset on and globbing off, preserves internal whitespace, strips only trailing newlines from command substitution output, and bounds/scrubs inner stderr surfaced in errors. Shares the builtin/block/coreutils handler chain with NewShell but uses the caller-provided env verbatim.
Rewires shellVariableResolver onto the embedded shell interpreter used by the bash tool and hooks, replacing the hand-rolled parser. Adds a bounded, scrubbed sanitizeResolveError so resolution failures surface safely without leaking oversized or non-printable inner stderr.
…s/args Adds MCPConfig.ResolvedArgs and changes ResolvedEnv/ResolvedHeaders to take a VariableResolver, so createTransport threads one resolver through command, args, env, and headers. Client mode (IdentityResolver) now keeps all four fields literal for server-side expansion; server mode expands uniformly via the shell resolver.
m.URL now runs through the same resolver as command, args, env, and
headers so $VAR / $(cmd) work in http and sse endpoints. The empty-url
guard runs after resolution so ${X:-} still fails cleanly, and failing
expansions surface via the existing StateError path.
Windows t.TempDir() returns C:\... paths whose backslashes get
consumed as escapes when injected into $(cat ...). Convert to
forward slashes so the embedded shell resolves the path on every OS.
Also accept the Windows coreutils error shape ("cannot find") in
addition to POSIX "exit status" when asserting that inner diagnostic
detail is surfaced — mvdan/sh runs coreutils in-process on Windows
so there is no subprocess exit status to report.
36d2fa6 to
4068422
Compare
andreynering
approved these changes
May 4, 2026
meowgorithm
added a commit
that referenced
this pull request
May 13, 2026
fix(config): restore full shell expansion in MCP config values
2 tasks
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
3 participants
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.
This fixes shell expansion for MCPs, and drastically improves upon the former method.
Basically, shell expansion in config for MCPs broke. When it was working it was home-rolled. It didn't support
${VAR:-default}, mishandled quoted parentheses in$(...), and rejected other constructs the rest of Crush (the bash tool, hooks) already accepts.Now, MCP config uses the same built-in shell interpreter that Crush uses for the
bashtool and hooks, so anything that works there now works in MCP config.This means you can do things like "$(cat /path/to/secret/token)" and "$(vault kv get my/secret/token)".
Also, now when MCP config is not working due to a missing value, now you'll see it.
Fixes #2334.