Detailed changes
@@ -461,7 +461,11 @@ func createTransport(ctx context.Context, m config.MCPConfig, resolver config.Va
Command: cmd,
}, nil
case config.MCPHttp:
- if strings.TrimSpace(m.URL) == "" {
+ url, err := m.ResolvedURL(resolver)
+ if err != nil {
+ return nil, err
+ }
+ if strings.TrimSpace(url) == "" {
return nil, fmt.Errorf("mcp http config requires a non-empty 'url' field")
}
headers, err := m.ResolvedHeaders(resolver)
@@ -474,11 +478,15 @@ func createTransport(ctx context.Context, m config.MCPConfig, resolver config.Va
},
}
return &mcp.StreamableClientTransport{
- Endpoint: m.URL,
+ Endpoint: url,
HTTPClient: client,
}, nil
case config.MCPSSE:
- if strings.TrimSpace(m.URL) == "" {
+ url, err := m.ResolvedURL(resolver)
+ if err != nil {
+ return nil, err
+ }
+ if strings.TrimSpace(url) == "" {
return nil, fmt.Errorf("mcp sse config requires a non-empty 'url' field")
}
headers, err := m.ResolvedHeaders(resolver)
@@ -491,7 +499,7 @@ func createTransport(ctx context.Context, m config.MCPConfig, resolver config.Va
},
}
return &mcp.SSEClientTransport{
- Endpoint: m.URL,
+ Endpoint: url,
HTTPClient: client,
}, nil
default:
@@ -4,6 +4,8 @@ import (
"context"
"testing"
+ "github.com/charmbracelet/crush/internal/config"
+ "github.com/charmbracelet/crush/internal/env"
"github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
@@ -36,3 +38,97 @@ func TestMCPSession_CancelOnClose(t *testing.T) {
// After Close, the context must be cancelled.
require.ErrorIs(t, ctx.Err(), context.Canceled)
}
+
+// TestCreateTransport_URLResolution pins that m.URL goes through the
+// same resolver seam as command, args, env, and headers. Covers both
+// the HTTP and SSE branches, success and failure, so a regression in
+// ResolvedURL wiring is caught at the transport layer rather than only
+// at the config layer.
+func TestCreateTransport_URLResolution(t *testing.T) {
+ t.Parallel()
+
+ shell := config.NewShellVariableResolver(env.NewFromMap(map[string]string{
+ "MCP_HOST": "mcp.example.com",
+ }))
+
+ t.Run("http success expands $VAR", func(t *testing.T) {
+ t.Parallel()
+ m := config.MCPConfig{
+ Type: config.MCPHttp,
+ URL: "https://$MCP_HOST/api",
+ }
+ tr, err := createTransport(t.Context(), m, shell)
+ require.NoError(t, err)
+ require.NotNil(t, tr)
+ sct, ok := tr.(*mcp.StreamableClientTransport)
+ require.True(t, ok, "expected StreamableClientTransport, got %T", tr)
+ require.Equal(t, "https://mcp.example.com/api", sct.Endpoint)
+ })
+
+ t.Run("sse success expands $(cmd)", func(t *testing.T) {
+ t.Parallel()
+ m := config.MCPConfig{
+ Type: config.MCPSSE,
+ URL: "https://$(echo mcp.example.com)/events",
+ }
+ tr, err := createTransport(t.Context(), m, shell)
+ require.NoError(t, err)
+ sse, ok := tr.(*mcp.SSEClientTransport)
+ require.True(t, ok, "expected SSEClientTransport, got %T", tr)
+ require.Equal(t, "https://mcp.example.com/events", sse.Endpoint)
+ })
+
+ t.Run("http unset var surfaces error, no transport created", func(t *testing.T) {
+ t.Parallel()
+ m := config.MCPConfig{
+ Type: config.MCPHttp,
+ URL: "https://$MCP_MISSING_HOST/api",
+ }
+ tr, err := createTransport(t.Context(), m, shell)
+ require.Error(t, err)
+ require.Nil(t, tr)
+ require.Contains(t, err.Error(), "url:")
+ require.Contains(t, err.Error(), "$MCP_MISSING_HOST")
+ })
+
+ t.Run("sse failing $(cmd) surfaces error, no transport created", func(t *testing.T) {
+ t.Parallel()
+ m := config.MCPConfig{
+ Type: config.MCPSSE,
+ URL: "https://$(false)/events",
+ }
+ tr, err := createTransport(t.Context(), m, shell)
+ require.Error(t, err)
+ require.Nil(t, tr)
+ require.Contains(t, err.Error(), "url:")
+ require.Contains(t, err.Error(), "$(false)")
+ })
+
+ t.Run("http empty-after-resolve still fails the non-empty guard", func(t *testing.T) {
+ t.Parallel()
+ // ${MCP_EMPTY:-} resolves to the empty string (no error),
+ // then the existing TrimSpace guard in createTransport must
+ // reject it so we never spawn a transport against "".
+ m := config.MCPConfig{
+ Type: config.MCPHttp,
+ URL: "${MCP_EMPTY:-}",
+ }
+ tr, err := createTransport(t.Context(), m, shell)
+ require.Error(t, err)
+ require.Nil(t, tr)
+ require.Contains(t, err.Error(), "non-empty 'url'")
+ })
+
+ t.Run("identity resolver round-trips template verbatim", func(t *testing.T) {
+ t.Parallel()
+ // Client mode forwards the template to the server; no local
+ // expansion, no error on unset vars.
+ tmpl := "https://$MCP_MISSING_HOST/api"
+ m := config.MCPConfig{Type: config.MCPHttp, URL: tmpl}
+ tr, err := createTransport(t.Context(), m, config.IdentityResolver())
+ require.NoError(t, err)
+ sct, ok := tr.(*mcp.StreamableClientTransport)
+ require.True(t, ok)
+ require.Equal(t, tmpl, sct.Endpoint)
+ })
+}
@@ -339,6 +339,28 @@ func (m MCPConfig) ResolvedArgs(r VariableResolver) ([]string, error) {
return out, nil
}
+// ResolvedURL returns m.URL expanded through the given resolver. The
+// receiver is not mutated. Errors from the resolver are already
+// sanitized by ResolveValue and are wrapped with %w for errors.Is/As.
+//
+// URLs run through the same shell-expansion pipeline as the other
+// fields, so a literal '$' (e.g. OData query strings containing
+// $filter/$select) must be escaped as '\$' or '${DOLLAR:-$}' to avoid
+// being interpreted as a variable reference. Same constraint already
+// applies to command, args, env, and headers.
+//
+// See ResolvedEnv for guidance on picking a resolver.
+func (m MCPConfig) ResolvedURL(r VariableResolver) (string, error) {
+ if m.URL == "" {
+ return "", nil
+ }
+ v, err := r.ResolveValue(m.URL)
+ if err != nil {
+ return "", fmt.Errorf("url: %w", err)
+ }
+ return v, nil
+}
+
// ResolvedHeaders returns m.Headers with every value expanded through
// the given resolver. A fresh map is allocated; m.Headers is never
// mutated. On the first resolution failure it returns nil and an error
@@ -0,0 +1,88 @@
+package config
+
+import (
+ "errors"
+ "testing"
+
+ "github.com/charmbracelet/crush/internal/env"
+ "github.com/stretchr/testify/require"
+)
+
+func TestMCPConfig_ResolvedURL(t *testing.T) {
+ t.Parallel()
+
+ t.Run("empty url short-circuits without calling resolver", func(t *testing.T) {
+ t.Parallel()
+ m := MCPConfig{Type: MCPHttp}
+ got, err := m.ResolvedURL(stubResolver{err: errors.New("should not be called")})
+ require.NoError(t, err)
+ require.Empty(t, got)
+ })
+
+ t.Run("literal url passes through unchanged", func(t *testing.T) {
+ t.Parallel()
+ m := MCPConfig{Type: MCPHttp, URL: "https://mcp.example.com/api"}
+ got, err := m.ResolvedURL(NewShellVariableResolver(env.NewFromMap(nil)))
+ require.NoError(t, err)
+ require.Equal(t, "https://mcp.example.com/api", got)
+ })
+
+ t.Run("expands $VAR with shell resolver", func(t *testing.T) {
+ t.Parallel()
+ m := MCPConfig{Type: MCPHttp, URL: "https://$MCP_HOST/api"}
+ r := NewShellVariableResolver(env.NewFromMap(map[string]string{"MCP_HOST": "mcp.example.com"}))
+ got, err := m.ResolvedURL(r)
+ require.NoError(t, err)
+ require.Equal(t, "https://mcp.example.com/api", got)
+ })
+
+ t.Run("expands $(cmd) with shell resolver", func(t *testing.T) {
+ t.Parallel()
+ m := MCPConfig{Type: MCPSSE, URL: "https://$(echo mcp.example.com)/events"}
+ got, err := m.ResolvedURL(NewShellVariableResolver(env.NewFromMap(nil)))
+ require.NoError(t, err)
+ require.Equal(t, "https://mcp.example.com/events", got)
+ })
+
+ t.Run("unset var is an error wrapping the template", func(t *testing.T) {
+ t.Parallel()
+ m := MCPConfig{Type: MCPHttp, URL: "https://$MCP_MISSING_HOST/api"}
+ _, err := m.ResolvedURL(NewShellVariableResolver(env.NewFromMap(nil)))
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "url:")
+ require.Contains(t, err.Error(), "$MCP_MISSING_HOST")
+ require.Contains(t, err.Error(), "unbound")
+ })
+
+ t.Run("failing command substitution is an error", func(t *testing.T) {
+ t.Parallel()
+ m := MCPConfig{Type: MCPHttp, URL: "https://$(false)/api"}
+ _, err := m.ResolvedURL(NewShellVariableResolver(env.NewFromMap(nil)))
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "url:")
+ require.Contains(t, err.Error(), "$(false)")
+ })
+
+ t.Run("identity resolver round-trips template verbatim", func(t *testing.T) {
+ t.Parallel()
+ // In client mode expansion happens server-side; the client must
+ // forward the template without touching it and without erroring
+ // on unset vars.
+ tmpl := "https://$MCP_HOST/$(vault read -f url)"
+ m := MCPConfig{Type: MCPHttp, URL: tmpl}
+ got, err := m.ResolvedURL(IdentityResolver())
+ require.NoError(t, err)
+ require.Equal(t, tmpl, got)
+ })
+}
+
+// stubResolver returns ("", err) for every call. Paired with a non-nil
+// err the empty-URL test asserts ResolvedURL short-circuits before
+// reaching ResolveValue: if it didn't, the test would fail with err.
+type stubResolver struct {
+ err error
+}
+
+func (s stubResolver) ResolveValue(v string) (string, error) {
+ return "", s.err
+}