fix(cliclient): path-escape server and tool names in per-server URLs (#1751)

# Pull Request

## Description

Official-registry server ids are `namespace/name`
(`io.github.owner/repo`, `com.pulsemcp/google-flights`). The daemon
handles them on `/api/v1/servers/{id}/*` by decoding a `%2F`-encoded
`{id}` (`decodeServerIDParam`, MCP-1118), and `GetServerLogs` already
sends `url.PathEscape(serverName)`. Every other per-server method in
`internal/cliclient` splices the raw name into the URL with
`fmt.Sprintf`, so the slash splits the name across path segments and the
request lands on a different route (usually a 404):

```
mcpproxy upstream enable io.github.owner/repo
  -> POST /api/v1/servers/io.github.owner/repo/enable    (route: {id}=io.github.owner, then /repo/enable)
```

Affected: `ServerAction` (enable/disable/restart), `GetServerTools`,
`SetToolEnabled`, `SetAllToolsEnabled`, `TriggerOAuthLoginWithResult`,
`TriggerOAuthLogout`, `PatchServer`, `RemoveServer`, `GetToolApprovals`,
`GetToolDiff`, `ApproveTools`, `BlockTools`. Most of these declare a
local `url` variable that shadows `net/url`, which is probably how the
escape was missed. `ServerAction`'s `var url string` is renamed to
`reqURL`; the `url := fmt.Sprintf(..., url.PathEscape(...))` sites are
fine as written because the new variable's scope starts after the
statement.

Tool names in `SetToolEnabled` and `GetToolDiff` are escaped too. For a
`/` in a tool name, the daemon also has to decode `{tool}`; that is
#1750.

## Testing

- [x] I have tested these changes locally
- [x] I have added/updated tests that prove my fix is effective or my
feature works
- [x] All existing tests pass

`TestPerServerRequests_EscapeServerAndToolNames`
(`internal/cliclient/server_path_escape_test.go`) calls 12 methods
against an `httptest` server and checks `r.URL.EscapedPath()`.

```
CGO_ENABLED=0 go vet ./internal/cliclient                                       # clean
CGO_ENABLED=0 go test ./internal/cliclient -count=1                             # ok
# with client.go reverted:
CGO_ENABLED=0 go test ./internal/cliclient -run TestPerServerRequests -count=1  # FAIL, 12 of 12 cases
gofmt -l internal/cliclient                                                     # clean
```

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: rudycelekli

internal/cliclient/client.go

@@ -607,21 +607,17 @@ func (c *Client) GetServerLogs(ctx context.Context, serverName string, tail int)
 
 // ServerAction performs an action on a server (enable, disable, restart).
 func (c *Client) ServerAction(ctx context.Context, serverName, action string) error {
-	var url string
+	var reqURL string
 	method := http.MethodPost
 
 	switch action {
-	case "enable":
-		url = fmt.Sprintf("%s/api/v1/servers/%s/enable", c.baseURL, serverName)
-	case "disable":
-		url = fmt.Sprintf("%s/api/v1/servers/%s/disable", c.baseURL, serverName)
-	case "restart":
-		url = fmt.Sprintf("%s/api/v1/servers/%s/restart", c.baseURL, serverName)
+	case "enable", "disable", "restart":
+		reqURL = fmt.Sprintf("%s/api/v1/servers/%s/%s", c.baseURL, url.PathEscape(serverName), action)
 	default:
 		return fmt.Errorf("unknown action: %s", action)
 	}
 
-	req, err := http.NewRequestWithContext(ctx, method, url, nil)
+	req, err := http.NewRequestWithContext(ctx, method, reqURL, nil)
 	if err != nil {
 		return fmt.Errorf("failed to create request: %w", err)
 	}
@@ -1209,7 +1205,7 @@ func (c *Client) GetGlobalToolsView(ctx context.Context, query url.Values) ([]ma
 
 // GetServerTools retrieves tools for a specific server from daemon.
 func (c *Client) GetServerTools(ctx context.Context, serverName string) ([]map[string]interface{}, error) {
-	url := fmt.Sprintf("%s/api/v1/servers/%s/tools", c.baseURL, serverName)
+	url := fmt.Sprintf("%s/api/v1/servers/%s/tools", c.baseURL, url.PathEscape(serverName))
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create request: %w", err)
@@ -1256,7 +1252,7 @@ func (c *Client) GetServerTools(ctx context.Context, serverName string) ([]map[s
 // quarantined or toggled before — see Runtime.SetToolEnabled.
 func (c *Client) SetToolEnabled(ctx context.Context, serverName, toolName string, enabled bool) error {
 	url := fmt.Sprintf("%s/api/v1/servers/%s/tools/%s/enabled",
-		c.baseURL, serverName, toolName)
+		c.baseURL, url.PathEscape(serverName), url.PathEscape(toolName))
 	body, err := json.Marshal(map[string]bool{"enabled": enabled})
 	if err != nil {
 		return fmt.Errorf("failed to marshal request: %w", err)
@@ -1304,7 +1300,7 @@ func (c *Client) SetAllToolsEnabled(ctx context.Context, serverName string, enab
 	if enabled {
 		path = "enable_all"
 	}
-	url := fmt.Sprintf("%s/api/v1/servers/%s/tools/%s", c.baseURL, serverName, path)
+	url := fmt.Sprintf("%s/api/v1/servers/%s/tools/%s", c.baseURL, url.PathEscape(serverName), path)
 	req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, nil)
 	if err != nil {
 		return 0, fmt.Errorf("failed to create request: %w", err)
@@ -1367,7 +1363,7 @@ func (c *Client) TriggerOAuthLogin(ctx context.Context, serverName string) error
 // the browser could not be opened (headless hosts, SSH sessions).
 // Returns *contracts.OAuthFlowError for structured OAuth errors (Spec 020).
 func (c *Client) TriggerOAuthLoginWithResult(ctx context.Context, serverName string) (*OAuthLoginResult, error) {
-	url := fmt.Sprintf("%s/api/v1/servers/%s/login", c.baseURL, serverName)
+	url := fmt.Sprintf("%s/api/v1/servers/%s/login", c.baseURL, url.PathEscape(serverName))
 	req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, nil)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create request: %w", err)
@@ -1454,7 +1450,7 @@ func (c *Client) TriggerOAuthLoginWithResult(ctx context.Context, serverName str
 
 // TriggerOAuthLogout clears OAuth token and disconnects a server.
 func (c *Client) TriggerOAuthLogout(ctx context.Context, serverName string) error {
-	url := fmt.Sprintf("%s/api/v1/servers/%s/logout", c.baseURL, serverName)
+	url := fmt.Sprintf("%s/api/v1/servers/%s/logout", c.baseURL, url.PathEscape(serverName))
 	req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, nil)
 	if err != nil {
 		return fmt.Errorf("failed to create request: %w", err)
@@ -1597,7 +1593,7 @@ func (c *Client) AddServer(ctx context.Context, req *AddServerRequest) (*AddServ
 //	{"headers": {"X-Set": "v", "X-Old": null}}       -- combined
 //	{"env": {"API_KEY": null}, "enabled": true}      -- env delete + enable
 func (c *Client) PatchServer(ctx context.Context, serverName string, body []byte) error {
-	url := fmt.Sprintf("%s/api/v1/servers/%s", c.baseURL, serverName)
+	url := fmt.Sprintf("%s/api/v1/servers/%s", c.baseURL, url.PathEscape(serverName))
 	req, err := http.NewRequestWithContext(ctx, http.MethodPatch, url, bytes.NewReader(body))
 	if err != nil {
 		return fmt.Errorf("failed to create request: %w", err)
@@ -1639,7 +1635,7 @@ func (c *Client) PatchServer(ctx context.Context, serverName string, body []byte
 
 // RemoveServer removes an upstream server via the daemon API.
 func (c *Client) RemoveServer(ctx context.Context, serverName string) error {
-	url := fmt.Sprintf("%s/api/v1/servers/%s", c.baseURL, serverName)
+	url := fmt.Sprintf("%s/api/v1/servers/%s", c.baseURL, url.PathEscape(serverName))
 	req, err := http.NewRequestWithContext(ctx, http.MethodDelete, url, nil)
 	if err != nil {
 		return fmt.Errorf("failed to create request: %w", err)
@@ -1865,7 +1861,7 @@ type ToolApprovalRecord struct {
 
 // GetToolApprovals fetches tool approval records for a server (Spec 032).
 func (c *Client) GetToolApprovals(ctx context.Context, serverName string) ([]ToolApprovalRecord, error) {
-	url := fmt.Sprintf("%s/api/v1/servers/%s/tools/export", c.baseURL, serverName)
+	url := fmt.Sprintf("%s/api/v1/servers/%s/tools/export", c.baseURL, url.PathEscape(serverName))
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create request: %w", err)
@@ -1909,7 +1905,7 @@ func (c *Client) GetToolApprovals(ctx context.Context, serverName string) ([]Too
 
 // GetToolDiff fetches the diff for a changed tool (Spec 032).
 func (c *Client) GetToolDiff(ctx context.Context, serverName, toolName string) (*ToolApprovalRecord, error) {
-	url := fmt.Sprintf("%s/api/v1/servers/%s/tools/%s/diff", c.baseURL, serverName, toolName)
+	url := fmt.Sprintf("%s/api/v1/servers/%s/tools/%s/diff", c.baseURL, url.PathEscape(serverName), url.PathEscape(toolName))
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create request: %w", err)
@@ -1951,7 +1947,7 @@ func (c *Client) GetToolDiff(ctx context.Context, serverName, toolName string) (
 
 // ApproveTools approves specific tools or all tools for a server (Spec 032).
 func (c *Client) ApproveTools(ctx context.Context, serverName string, toolNames []string, approveAll bool) (int, error) {
-	url := fmt.Sprintf("%s/api/v1/servers/%s/tools/approve", c.baseURL, serverName)
+	url := fmt.Sprintf("%s/api/v1/servers/%s/tools/approve", c.baseURL, url.PathEscape(serverName))
 
 	reqBody := struct {
 		Tools      []string `json:"tools,omitempty"`
@@ -2012,7 +2008,7 @@ func (c *Client) ApproveTools(ctx context.Context, serverName string, toolNames
 // (Spec 032). Block atomically approves AND disables the tool so it is never
 // left in the approved+enabled state — mirroring the Web UI "Block" action.
 func (c *Client) BlockTools(ctx context.Context, serverName string, toolNames []string, blockAll bool) (int, error) {
-	url := fmt.Sprintf("%s/api/v1/servers/%s/tools/block", c.baseURL, serverName)
+	url := fmt.Sprintf("%s/api/v1/servers/%s/tools/block", c.baseURL, url.PathEscape(serverName))
 
 	reqBody := struct {
 		Tools    []string `json:"tools,omitempty"`

internal/cliclient/server_path_escape_test.go

@@ -0,0 +1,59 @@
+package cliclient
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"go.uber.org/zap"
+)
+
+// Official-registry server ids are namespace/name (io.github.owner/repo). The
+// daemon decodes a %2F-encoded {id} (MCP-1118), but only if the client sends
+// one: an unescaped slash splits the name across path segments and the request
+// lands on a different route.
+func TestPerServerRequests_EscapeServerAndToolNames(t *testing.T) {
+	const (
+		server = "io.github.owner/repo"
+		srvEnc = "io.github.owner%2Frepo"
+		tool   = "ns/erase"
+		tlEnc  = "ns%2Ferase"
+	)
+
+	var got string
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		got = r.URL.EscapedPath()
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"success":true,"data":{}}`))
+	}))
+	defer ts.Close()
+
+	c := NewClient(ts.URL, zap.NewNop().Sugar())
+	ctx := context.Background()
+
+	for _, tc := range []struct {
+		name string
+		call func()
+		want string
+	}{
+		{"enable", func() { _ = c.ServerAction(ctx, server, "enable") }, "/api/v1/servers/" + srvEnc + "/enable"},
+		{"restart", func() { _ = c.ServerAction(ctx, server, "restart") }, "/api/v1/servers/" + srvEnc + "/restart"},
+		{"tools", func() { _, _ = c.GetServerTools(ctx, server) }, "/api/v1/servers/" + srvEnc + "/tools"},
+		{"tool enabled", func() { _ = c.SetToolEnabled(ctx, server, tool, false) }, "/api/v1/servers/" + srvEnc + "/tools/" + tlEnc + "/enabled"},
+		{"all tools", func() { _, _ = c.SetAllToolsEnabled(ctx, server, true) }, "/api/v1/servers/" + srvEnc + "/tools/enable_all"},
+		{"logout", func() { _ = c.TriggerOAuthLogout(ctx, server) }, "/api/v1/servers/" + srvEnc + "/logout"},
+		{"patch", func() { _ = c.PatchServer(ctx, server, []byte(`{}`)) }, "/api/v1/servers/" + srvEnc},
+		{"remove", func() { _ = c.RemoveServer(ctx, server) }, "/api/v1/servers/" + srvEnc},
+		{"approvals", func() { _, _ = c.GetToolApprovals(ctx, server) }, "/api/v1/servers/" + srvEnc + "/tools/export"},
+		{"diff", func() { _, _ = c.GetToolDiff(ctx, server, tool) }, "/api/v1/servers/" + srvEnc + "/tools/" + tlEnc + "/diff"},
+		{"approve", func() { _, _ = c.ApproveTools(ctx, server, nil, true) }, "/api/v1/servers/" + srvEnc + "/tools/approve"},
+		{"block", func() { _, _ = c.BlockTools(ctx, server, nil, true) }, "/api/v1/servers/" + srvEnc + "/tools/block"},
+	} {
+		got = ""
+		tc.call()
+		if got != tc.want {
+			t.Errorf("%s: request path = %q, want %q", tc.name, got, tc.want)
+		}
+	}
+}