From 2f5fb5f91b6494e28503a488bd0739ceaa3b1812 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Fri, 23 Oct 2026 22:23:35 -0400 Subject: [PATCH] fix(cliclient): path-escape server and tool names in per-server URLs Co-authored-by: Cursor --- internal/cliclient/client.go | 34 +++++------ internal/cliclient/server_path_escape_test.go | 59 +++++++++++++++++++ 2 files changed, 74 insertions(+), 19 deletions(-) create mode 100644 internal/cliclient/server_path_escape_test.go diff --git a/internal/cliclient/client.go b/internal/cliclient/client.go index 4479941ea..943419467 100644 --- a/internal/cliclient/client.go +++ b/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"` diff --git a/internal/cliclient/server_path_escape_test.go b/internal/cliclient/server_path_escape_test.go new file mode 100644 index 000000000..b2a7282fe --- /dev/null +++ b/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) + } + } +}