git.iko.soy/crush
on main help sign in
feat(config): resolve MCP url through shell expansion

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.

commitf71645753fb8429356354b1354a58c439580d9bc authorChristian Rocha <christian@rocha.is> dateSat 2 May 2026, 10:22 (6 months ago) parentdbd40d8
4 files changed+218−4
compare with main
unifiedsplit
internal/agent/tools/mcp/init.go+12−4 internal/agent/tools/mcp/init_test.go+96 internal/config/config.go+22 internal/config/mcp_resolved_url_test.go+88
internal/agent/tools/mcp/init.go+12−4
@@ -461,7 +461,11 @@ func createTransport(ctx context.Context, m config.MCPConfig, resolver config.Va
461461 Command: cmd,
462462 }, nil
463463 case config.MCPHttp:
464− if strings.TrimSpace(m.URL) == "" {
464+ url, err := m.ResolvedURL(resolver)
465+ if err != nil {
466+ return nil, err
467+ }
468+ if strings.TrimSpace(url) == "" {
465469 return nil, fmt.Errorf("mcp http config requires a non-empty 'url' field")
466470 }
467471 headers, err := m.ResolvedHeaders(resolver)
@@ -474,11 +478,15 @@ func createTransport(ctx context.Context, m config.MCPConfig, resolver config.Va
474478 },
475479 }
476480 return &mcp.StreamableClientTransport{
477− Endpoint: m.URL,
481+ Endpoint: url,
478482 HTTPClient: client,
479483 }, nil
480484 case config.MCPSSE:
481− if strings.TrimSpace(m.URL) == "" {
485+ url, err := m.ResolvedURL(resolver)
486+ if err != nil {
487+ return nil, err
488+ }
489+ if strings.TrimSpace(url) == "" {
482490 return nil, fmt.Errorf("mcp sse config requires a non-empty 'url' field")
483491 }
484492 headers, err := m.ResolvedHeaders(resolver)
@@ -491,7 +499,7 @@ func createTransport(ctx context.Context, m config.MCPConfig, resolver config.Va
491499 },
492500 }
493501 return &mcp.SSEClientTransport{
494− Endpoint: m.URL,
502+ Endpoint: url,
495503 HTTPClient: client,
496504 }, nil
497505 default:
internal/agent/tools/mcp/init_test.go+96
@@ -4,6 +4,8 @@ import (
44 "context"
55 "testing"
66
7+ "github.com/charmbracelet/crush/internal/config"
8+ "github.com/charmbracelet/crush/internal/env"
79 "github.com/modelcontextprotocol/go-sdk/mcp"
810 "github.com/stretchr/testify/require"
911 "go.uber.org/goleak"
@@ -36,3 +38,97 @@ func TestMCPSession_CancelOnClose(t *testing.T) {
3638 // After Close, the context must be cancelled.
3739 require.ErrorIs(t, ctx.Err(), context.Canceled)
3840}
41+
42+// TestCreateTransport_URLResolution pins that m.URL goes through the
43+// same resolver seam as command, args, env, and headers. Covers both
44+// the HTTP and SSE branches, success and failure, so a regression in
45+// ResolvedURL wiring is caught at the transport layer rather than only
46+// at the config layer.
47+func TestCreateTransport_URLResolution(t *testing.T) {
48+ t.Parallel()
49+
50+ shell := config.NewShellVariableResolver(env.NewFromMap(map[string]string{
51+ "MCP_HOST": "mcp.example.com",
52+ }))
53+
54+ t.Run("http success expands $VAR", func(t *testing.T) {
55+ t.Parallel()
56+ m := config.MCPConfig{
57+ Type: config.MCPHttp,
58+ URL: "https://$MCP_HOST/api",
59+ }
60+ tr, err := createTransport(t.Context(), m, shell)
61+ require.NoError(t, err)
62+ require.NotNil(t, tr)
63+ sct, ok := tr.(*mcp.StreamableClientTransport)
64+ require.True(t, ok, "expected StreamableClientTransport, got %T", tr)
65+ require.Equal(t, "https://mcp.example.com/api", sct.Endpoint)
66+ })
67+
68+ t.Run("sse success expands $(cmd)", func(t *testing.T) {
69+ t.Parallel()
70+ m := config.MCPConfig{
71+ Type: config.MCPSSE,
72+ URL: "https://$(echo mcp.example.com)/events",
73+ }
74+ tr, err := createTransport(t.Context(), m, shell)
75+ require.NoError(t, err)
76+ sse, ok := tr.(*mcp.SSEClientTransport)
77+ require.True(t, ok, "expected SSEClientTransport, got %T", tr)
78+ require.Equal(t, "https://mcp.example.com/events", sse.Endpoint)
79+ })
80+
81+ t.Run("http unset var surfaces error, no transport created", func(t *testing.T) {
82+ t.Parallel()
83+ m := config.MCPConfig{
84+ Type: config.MCPHttp,
85+ URL: "https://$MCP_MISSING_HOST/api",
86+ }
87+ tr, err := createTransport(t.Context(), m, shell)
88+ require.Error(t, err)
89+ require.Nil(t, tr)
90+ require.Contains(t, err.Error(), "url:")
91+ require.Contains(t, err.Error(), "$MCP_MISSING_HOST")
92+ })
93+
94+ t.Run("sse failing $(cmd) surfaces error, no transport created", func(t *testing.T) {
95+ t.Parallel()
96+ m := config.MCPConfig{
97+ Type: config.MCPSSE,
98+ URL: "https://$(false)/events",
99+ }
100+ tr, err := createTransport(t.Context(), m, shell)
101+ require.Error(t, err)
102+ require.Nil(t, tr)
103+ require.Contains(t, err.Error(), "url:")
104+ require.Contains(t, err.Error(), "$(false)")
105+ })
106+
107+ t.Run("http empty-after-resolve still fails the non-empty guard", func(t *testing.T) {
108+ t.Parallel()
109+ // ${MCP_EMPTY:-} resolves to the empty string (no error),
110+ // then the existing TrimSpace guard in createTransport must
111+ // reject it so we never spawn a transport against "".
112+ m := config.MCPConfig{
113+ Type: config.MCPHttp,
114+ URL: "${MCP_EMPTY:-}",
115+ }
116+ tr, err := createTransport(t.Context(), m, shell)
117+ require.Error(t, err)
118+ require.Nil(t, tr)
119+ require.Contains(t, err.Error(), "non-empty 'url'")
120+ })
121+
122+ t.Run("identity resolver round-trips template verbatim", func(t *testing.T) {
123+ t.Parallel()
124+ // Client mode forwards the template to the server; no local
125+ // expansion, no error on unset vars.
126+ tmpl := "https://$MCP_MISSING_HOST/api"
127+ m := config.MCPConfig{Type: config.MCPHttp, URL: tmpl}
128+ tr, err := createTransport(t.Context(), m, config.IdentityResolver())
129+ require.NoError(t, err)
130+ sct, ok := tr.(*mcp.StreamableClientTransport)
131+ require.True(t, ok)
132+ require.Equal(t, tmpl, sct.Endpoint)
133+ })
134+}
internal/config/config.go+22
@@ -339,6 +339,28 @@ func (m MCPConfig) ResolvedArgs(r VariableResolver) ([]string, error) {
339339 return out, nil
340340}
341341
342+// ResolvedURL returns m.URL expanded through the given resolver. The
343+// receiver is not mutated. Errors from the resolver are already
344+// sanitized by ResolveValue and are wrapped with %w for errors.Is/As.
345+//
346+// URLs run through the same shell-expansion pipeline as the other
347+// fields, so a literal '$' (e.g. OData query strings containing
348+// $filter/$select) must be escaped as '\$' or '${DOLLAR:-$}' to avoid
349+// being interpreted as a variable reference. Same constraint already
350+// applies to command, args, env, and headers.
351+//
352+// See ResolvedEnv for guidance on picking a resolver.
353+func (m MCPConfig) ResolvedURL(r VariableResolver) (string, error) {
354+ if m.URL == "" {
355+ return "", nil
356+ }
357+ v, err := r.ResolveValue(m.URL)
358+ if err != nil {
359+ return "", fmt.Errorf("url: %w", err)
360+ }
361+ return v, nil
362+}
363+
342364// ResolvedHeaders returns m.Headers with every value expanded through
343365// the given resolver. A fresh map is allocated; m.Headers is never
344366// mutated. On the first resolution failure it returns nil and an error
internal/config/mcp_resolved_url_test.gonew file+88
@@ -0,0 +1,88 @@
1+package config
2+
3+import (
4+ "errors"
5+ "testing"
6+
7+ "github.com/charmbracelet/crush/internal/env"
8+ "github.com/stretchr/testify/require"
9+)
10+
11+func TestMCPConfig_ResolvedURL(t *testing.T) {
12+ t.Parallel()
13+
14+ t.Run("empty url short-circuits without calling resolver", func(t *testing.T) {
15+ t.Parallel()
16+ m := MCPConfig{Type: MCPHttp}
17+ got, err := m.ResolvedURL(stubResolver{err: errors.New("should not be called")})
18+ require.NoError(t, err)
19+ require.Empty(t, got)
20+ })
21+
22+ t.Run("literal url passes through unchanged", func(t *testing.T) {
23+ t.Parallel()
24+ m := MCPConfig{Type: MCPHttp, URL: "https://mcp.example.com/api"}
25+ got, err := m.ResolvedURL(NewShellVariableResolver(env.NewFromMap(nil)))
26+ require.NoError(t, err)
27+ require.Equal(t, "https://mcp.example.com/api", got)
28+ })
29+
30+ t.Run("expands $VAR with shell resolver", func(t *testing.T) {
31+ t.Parallel()
32+ m := MCPConfig{Type: MCPHttp, URL: "https://$MCP_HOST/api"}
33+ r := NewShellVariableResolver(env.NewFromMap(map[string]string{"MCP_HOST": "mcp.example.com"}))
34+ got, err := m.ResolvedURL(r)
35+ require.NoError(t, err)
36+ require.Equal(t, "https://mcp.example.com/api", got)
37+ })
38+
39+ t.Run("expands $(cmd) with shell resolver", func(t *testing.T) {
40+ t.Parallel()
41+ m := MCPConfig{Type: MCPSSE, URL: "https://$(echo mcp.example.com)/events"}
42+ got, err := m.ResolvedURL(NewShellVariableResolver(env.NewFromMap(nil)))
43+ require.NoError(t, err)
44+ require.Equal(t, "https://mcp.example.com/events", got)
45+ })
46+
47+ t.Run("unset var is an error wrapping the template", func(t *testing.T) {
48+ t.Parallel()
49+ m := MCPConfig{Type: MCPHttp, URL: "https://$MCP_MISSING_HOST/api"}
50+ _, err := m.ResolvedURL(NewShellVariableResolver(env.NewFromMap(nil)))
51+ require.Error(t, err)
52+ require.Contains(t, err.Error(), "url:")
53+ require.Contains(t, err.Error(), "$MCP_MISSING_HOST")
54+ require.Contains(t, err.Error(), "unbound")
55+ })
56+
57+ t.Run("failing command substitution is an error", func(t *testing.T) {
58+ t.Parallel()
59+ m := MCPConfig{Type: MCPHttp, URL: "https://$(false)/api"}
60+ _, err := m.ResolvedURL(NewShellVariableResolver(env.NewFromMap(nil)))
61+ require.Error(t, err)
62+ require.Contains(t, err.Error(), "url:")
63+ require.Contains(t, err.Error(), "$(false)")
64+ })
65+
66+ t.Run("identity resolver round-trips template verbatim", func(t *testing.T) {
67+ t.Parallel()
68+ // In client mode expansion happens server-side; the client must
69+ // forward the template without touching it and without erroring
70+ // on unset vars.
71+ tmpl := "https://$MCP_HOST/$(vault read -f url)"
72+ m := MCPConfig{Type: MCPHttp, URL: tmpl}
73+ got, err := m.ResolvedURL(IdentityResolver())
74+ require.NoError(t, err)
75+ require.Equal(t, tmpl, got)
76+ })
77+}
78+
79+// stubResolver returns ("", err) for every call. Paired with a non-nil
80+// err the empty-URL test asserts ResolvedURL short-circuits before
81+// reaching ResolveValue: if it didn't, the test would fail with err.
82+type stubResolver struct {
83+ err error
84+}
85+
86+func (s stubResolver) ResolveValue(v string) (string, error) {
87+ return "", s.err
88+}