fix(api): decode %2F in /servers/{id}/* path param via middleware (MCP-1118) (#624)

All /api/v1/servers/{id}/* sub-resource handlers were receiving the raw
percent-encoded {id} param from chi (e.g. io.github.owner%2Frepo for a
slash-name official-registry server). This caused 404s for the Tools and
Logs tabs in the Web UI and for CLI sub-resource commands on slash-name
servers, since exact-match server lookups never found the encoded literal.

Fix: add decodeServerIDParam middleware to the /servers/{id} route group.
It decodes {id} in the chi route context once, so every handler in the
subtree (tools, logs, restart, approve, scan, …) receives the real name
without each one needing an explicit decodePathParam call.

Also remove the now-redundant per-handler decodePathParam in
handleGetServerLogs (was the only handler previously fixed individually).

Same root cause as MCP-1056 (registry add-server slash ids, PR #591).

Tests: TestServerSubresource_SlashServerIDUnescaped (tools/restart/tool-calls)
       TestGetServerLogs_SlashServerIDUnescaped (existing, still passes)

Author: Dumbris

internal/httpapi/server.go

@@ -602,6 +602,15 @@ func (s *Server) setupRoutes() {
 		r.Post("/servers/enable_all", s.handleEnableAll)
 		r.Post("/servers/disable_all", s.handleDisableAll)
 		r.Route("/servers/{id}", func(r chi.Router) {
+			// chi routes on RawPath, so the {id} param arrives percent-encoded.
+			// Official modelcontextprotocol/registry v0.1 ids are namespace/name,
+			// so the slash reaches handlers as %2F. Decode it once here so every
+			// /servers/{id}/* sub-resource handler (tools, logs, restart, approve,
+			// scan, …) does its exact-match server lookup against the real name
+			// rather than 404ing on the encoded literal (MCP-1118, same class as
+			// MCP-1056). Centralising the decode also prevents new sub-resource
+			// routes from silently reintroducing the gap.
+			r.Use(decodeServerIDParam)
 			r.Patch("/", s.handlePatchServer)                          // Partial update server config
 			r.Delete("/", s.handleRemoveServer)                        // T002: Remove server
 			r.Post("/config-to-secret", s.handleConvertConfigToSecret) // Move a header / env value into OS keyring
@@ -2678,10 +2687,10 @@ func (s *Server) handleGetGlobalTools(w http.ResponseWriter, r *http.Request) {
 // @Failure 500 {object} contracts.ErrorResponse "Internal server error"
 // @Router /api/v1/servers/{id}/logs [get]
 func (s *Server) handleGetServerLogs(w http.ResponseWriter, r *http.Request) {
-	// chi routes on RawPath, so a namespace/name server id (e.g.
-	// io.github.evidai/polymarket-guard) arrives percent-encoded. Decode it
-	// before lookup, matching handleAddFromRegistry (MCP-1111 / #598).
-	serverID := decodePathParam(chi.URLParam(r, "id"))
+	// The {id} param is percent-decoded by decodeServerIDParam middleware on the
+	// /servers/{id} subtree (MCP-1118), so a namespace/name server id such as
+	// io.github.evidai/polymarket-guard reaches us already unescaped.
+	serverID := chi.URLParam(r, "id")
 	if serverID == "" {
 		s.writeError(w, r, http.StatusBadRequest, "Server ID required")
 		return
@@ -4168,6 +4177,26 @@ func decodePathParam(raw string) string {
 	return raw
 }
 
+// decodeServerIDParam percent-decodes the {id} path param of the /servers/{id}
+// route subtree in place. chi matches the parent /servers/{id} segment (and thus
+// populates "id") before mounting this subrouter, so the value is available to
+// this middleware. Slash-name servers (io.github.owner/repo) arrive as
+// io.github.owner%2Frepo; decoding here makes every sub-resource handler's
+// exact-match server lookup work without each one having to call
+// decodePathParam (MCP-1118).
+func decodeServerIDParam(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if rctx := chi.RouteContext(r.Context()); rctx != nil {
+			for i, k := range rctx.URLParams.Keys {
+				if k == "id" {
+					rctx.URLParams.Values[i] = decodePathParam(rctx.URLParams.Values[i])
+				}
+			}
+		}
+		next.ServeHTTP(w, r)
+	})
+}
+
 func (s *Server) handleAddFromRegistry(w http.ResponseWriter, r *http.Request) {
 	// chi routes on RawPath, so path params arrive percent-encoded. Official
 	// modelcontextprotocol/registry v0.1 ids are namespace/name, so the slash

internal/httpapi/server_path_param_decode_test.go

@@ -0,0 +1,99 @@
+package httpapi
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap/zaptest"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts"
+)
+
+// capturingToolCallsController records the serverID forwarded to the controller
+// so the test can assert the {id} path param was percent-decoded before lookup.
+type capturingToolCallsController struct {
+	*MockServerController
+	gotServerID string
+}
+
+func (c *capturingToolCallsController) GetServerToolCalls(serverID string, _ int) ([]*contracts.ToolCallRecord, error) {
+	c.gotServerID = serverID
+	return nil, nil
+}
+
+// TestServerSubresource_SlashServerIDUnescaped reproduces MCP-1118: official
+// modelcontextprotocol/registry v0.1 server ids are namespace/name (e.g.
+// io.github.owner/repo) and chi routes on RawPath, so the {id} param of every
+// /api/v1/servers/{id}/* sub-resource handler arrives percent-encoded
+// (io.github.owner%2Frepo). Without decoding, the exact-match server lookup
+// targets the literal encoded name and 404s, so the Web UI server-detail Tools
+// and Logs tabs (and CLI sub-resources) never load for slash-name servers.
+//
+// The fix decodes {id} once at the /servers/{id} route subtree, so this asserts
+// the decoded name reaches both the management-service path (tools, restart) and
+// the controller path (tool-calls).
+func TestServerSubresource_SlashServerIDUnescaped(t *testing.T) {
+	const (
+		encoded = "io.github.owner%2Frepo"
+		decoded = "io.github.owner/repo"
+	)
+
+	// dataField unwraps the APIResponse envelope and returns the requested
+	// string field from the data object.
+	dataString := func(t *testing.T, body []byte, key string) string {
+		t.Helper()
+		var resp struct {
+			Success bool                   `json:"success"`
+			Data    map[string]interface{} `json:"data"`
+		}
+		require.NoError(t, json.Unmarshal(body, &resp), "body=%s", string(body))
+		require.True(t, resp.Success, "request should succeed; body=%s", string(body))
+		v, _ := resp.Data[key].(string)
+		return v
+	}
+
+	t.Run("GET /tools echoes decoded server name", func(t *testing.T) {
+		logger := zaptest.NewLogger(t).Sugar()
+		server := NewServer(&MockServerController{}, logger, nil)
+
+		req := httptest.NewRequest(http.MethodGet, "/api/v1/servers/"+encoded+"/tools", http.NoBody)
+		w := httptest.NewRecorder()
+		server.ServeHTTP(w, req)
+
+		require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+		assert.Equal(t, decoded, dataString(t, w.Body.Bytes(), "server_name"),
+			"server id must be percent-decoded before lookup")
+	})
+
+	t.Run("POST /restart echoes decoded server name", func(t *testing.T) {
+		logger := zaptest.NewLogger(t).Sugar()
+		server := NewServer(&MockServerController{}, logger, nil)
+
+		req := httptest.NewRequest(http.MethodPost, "/api/v1/servers/"+encoded+"/restart", http.NoBody)
+		w := httptest.NewRecorder()
+		server.ServeHTTP(w, req)
+
+		require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+		assert.Equal(t, decoded, dataString(t, w.Body.Bytes(), "server"),
+			"server id must be percent-decoded before lookup")
+	})
+
+	t.Run("GET /tool-calls forwards decoded server name to controller", func(t *testing.T) {
+		logger := zaptest.NewLogger(t).Sugar()
+		controller := &capturingToolCallsController{MockServerController: &MockServerController{}}
+		server := NewServer(controller, logger, nil)
+
+		req := httptest.NewRequest(http.MethodGet, "/api/v1/servers/"+encoded+"/tool-calls", http.NoBody)
+		w := httptest.NewRecorder()
+		server.ServeHTTP(w, req)
+
+		require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+		assert.Equal(t, decoded, controller.gotServerID,
+			"server id must be percent-decoded before reaching the controller")
+		assert.Equal(t, decoded, dataString(t, w.Body.Bytes(), "server_name"))
+	})
+}