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

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"))
+	})
+}