fix(httpapi): percent-decode the {tool} path param before the approval lookup (#1750)

# Pull Request

## Description

chi routes on `RawPath`, so path params arrive percent-encoded whenever
the client's encoding differs from Go's default. MCP-1118 fixed this for
`{id}` with `decodeServerIDParam` on the `/servers/{id}` subtree, but
the nested `{tool}` param is still read raw by its two handlers:

- `GET /api/v1/servers/{id}/tools/{tool}/diff` (`handleGetToolDiff`)
- `POST /api/v1/servers/{id}/tools/{tool}/enabled`
(`handleSetToolEnabled`)

**Trigger.** The Web UI builds both URLs with
`encodeURIComponent(toolName)` (`frontend/src/services/api.ts:577`,
`:618`), which encodes `:` as `%3A` and `/` as `%2F`. Go's default path
encoding leaves `:` alone, so for a tool named `ns:erase` the request
has a non-empty `RawPath`, chi hands the handler `ns%3Aerase`, and the
exact-match approval lookup misses. The Review diff for that tool never
loads and the per-tool enable/disable toggle fails. Tool names carrying
`:` or `/` are upstream-defined and already accounted for elsewhere (see
`canonicalIdentity` in `internal/profile/glob.go`).

`decodeServerIDParam` cannot cover `{tool}`: it runs as subtree
middleware, before the nested route has been matched and the param
populated. So the fix is the same `decodePathParam` call in the two
handlers, as `handleAddFromRegistry` already does for `{serverId}`.

## 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

`TestToolSubresource_EncodedToolNameUnescaped`
(`internal/httpapi/tool_path_param_decode_test.go`) drives both routes
through `Server.ServeHTTP` with `ns%3Aerase` and `ns%2Ferase` and
asserts the decoded name reaches the lookup.

```
CGO_ENABLED=0 go test ./internal/httpapi -count=1                                  # ok
# with server.go reverted:
CGO_ENABLED=0 go test ./internal/httpapi -run TestToolSubresource -count=1         # FAIL, all 4 subtests
gofmt -l <changed files>                                                           # clean
```

(`CGO_ENABLED=0` only because my local macOS SDK cannot link cgo test
binaries; nothing here uses cgo.)


## Note on the CodeQL alert

CodeQL reports `go/reflected-xss` at `internal/httpapi/middleware.go:85`
(`statusRecorder.Write`) because the decoded tool name now reaches the
response. The only sinks are `writeSuccess` / `writeError`, both of
which go through `writeJSON`: `Content-Type: application/json` plus
`json.Encoder` with its default HTML escaping (`<`, `>`, `&` become
`\u003c` and so on). The raw `chi.URLParam` value already reached the
same sinks before this change. The decode only turns `%3A` into `:` so
the controller lookup matches.

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

Author: rudycelekli

internal/httpapi/server.go

@@ -7275,7 +7275,7 @@ func (s *Server) handleSetToolEnabled(w http.ResponseWriter, r *http.Request) {
 	}
 
 	serverID := chi.URLParam(r, "id")
-	toolName := chi.URLParam(r, "tool")
+	toolName := decodePathParam(chi.URLParam(r, "tool"))
 	if serverID == "" || toolName == "" {
 		s.writeError(w, r, http.StatusBadRequest, "Server ID and tool name required")
 		return
@@ -7402,7 +7402,7 @@ func (s *Server) handleSetAllToolsEnabled(enabled bool) http.HandlerFunc {
 
 func (s *Server) handleGetToolDiff(w http.ResponseWriter, r *http.Request) {
 	serverID := chi.URLParam(r, "id")
-	toolName := chi.URLParam(r, "tool")
+	toolName := decodePathParam(chi.URLParam(r, "tool"))
 
 	if serverID == "" || toolName == "" {
 		s.writeError(w, r, http.StatusBadRequest, "Server ID and tool name required")

internal/httpapi/tool_path_param_decode_test.go

@@ -0,0 +1,67 @@
+package httpapi
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/storage"
+)
+
+// TestToolSubresource_EncodedToolNameUnescaped: chi routes on RawPath, so the
+// {tool} param arrives percent-encoded whenever the encoding differs from Go's
+// default (the Web UI sends encodeURIComponent(toolName), which encodes ':' and
+// '/'). Tool names such as "ns:erase" or "ns/erase" must be decoded before the
+// exact-match approval lookup, the same way {id} is (MCP-1118).
+func TestToolSubresource_EncodedToolNameUnescaped(t *testing.T) {
+	for _, tc := range []struct{ encoded, decoded string }{
+		{"ns%3Aerase", "ns:erase"},
+		{"ns%2Ferase", "ns/erase"},
+	} {
+		newCtrl := func() *mockToolQuarantineController {
+			return &mockToolQuarantineController{
+				apiKey: "test-key",
+				approvals: []*storage.ToolApprovalRecord{{
+					ServerName:          "github",
+					ToolName:            tc.decoded,
+					Status:              storage.ToolApprovalStatusChanged,
+					PreviousDescription: "old",
+					CurrentDescription:  "new",
+				}},
+			}
+		}
+
+		t.Run("GET diff "+tc.decoded, func(t *testing.T) {
+			server := NewServer(newCtrl(), zap.NewNop().Sugar(), nil)
+			req := httptest.NewRequest(http.MethodGet, "/api/v1/servers/github/tools/"+tc.encoded+"/diff", nil)
+			req.Header.Set("X-API-Key", "test-key")
+			w := httptest.NewRecorder()
+			server.ServeHTTP(w, req)
+
+			require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+			var resp map[string]interface{}
+			require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp))
+			data := resp["data"].(map[string]interface{})
+			assert.Equal(t, "new", data["current_description"])
+		})
+
+		t.Run("POST enabled "+tc.decoded, func(t *testing.T) {
+			ctrl := newCtrl()
+			server := NewServer(ctrl, zap.NewNop().Sugar(), nil)
+			req := httptest.NewRequest(http.MethodPost, "/api/v1/servers/github/tools/"+tc.encoded+"/enabled",
+				bytes.NewBufferString(`{"enabled":false}`))
+			req.Header.Set("X-API-Key", "test-key")
+			w := httptest.NewRecorder()
+			server.ServeHTTP(w, req)
+
+			require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+			assert.Equal(t, tc.decoded, ctrl.setToolEnabledTool)
+		})
+	}
+}