Skip to content

Commit 2a99e93

Browse files
authored
fix(httpapi): percent-decode the registry {id} on search, refresh, edit and remove (#1774)
## Problem Custom registry ids are user-chosen (`registry add-source --id ...`) and aren't restricted to a slug, so they can contain characters such as `:` or `/`. The CLI builds these URLs with `url.PathEscape` and the Web UI with `encodeURIComponent`, so those characters reach the server percent-encoded. chi routes on `RawPath`, which means `chi.URLParam(r, "id")` returns the encoded form (`acme%3Ainternal`) and the exact-match registry lookup misses. As a result, a registry added as `acme:internal` can't be searched, refreshed, edited or removed. `handleAddFromRegistry` already decodes the same param (MCP-1056); the other four handlers don't. ## Fix Wrap `{id}` in the existing `decodePathParam` in `handleSearchRegistryServers`, `handleRefreshRegistryCache`, `handleEditRegistrySource` and `handleRemoveRegistrySource`. ## Test `TestRegistryRoutes_DecodeIDParam` sends `acme%3Ainternal` to all four routes and checks the id passed to the controller. On main all four subtests receive `acme%3Ainternal`; with this change they receive `acme:internal`. `go test -count=1 ./internal/httpapi/` passes. Made with [Cursor](https://cursor.com)
1 parent 599a2ee commit 2a99e93

2 files changed

Lines changed: 71 additions & 4 deletions

File tree

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package httpapi
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"strings"
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"go.uber.org/zap/zaptest"
11+
12+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
13+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts"
14+
)
15+
16+
// registryIDRecorder records the registry id each registry handler forwards.
17+
type registryIDRecorder struct {
18+
*MockServerController
19+
got string
20+
}
21+
22+
func (c *registryIDRecorder) SearchRegistryServers(id, _, _ string, _ int) ([]interface{}, *contracts.RegistryCacheInfo, error) {
23+
c.got = id
24+
return []interface{}{}, nil, nil
25+
}
26+
27+
func (c *registryIDRecorder) RefreshRegistryCache(id string) (int, error) {
28+
c.got = id
29+
return 0, nil
30+
}
31+
32+
func (c *registryIDRecorder) RemoveRegistrySourceRef(id string) (*config.RegistryEntry, *contracts.RegistryAddError, error) {
33+
c.got = id
34+
return &config.RegistryEntry{ID: id}, nil, nil
35+
}
36+
37+
func (c *registryIDRecorder) EditRegistrySourceRef(id, _, _, _ string) (*config.RegistryEntry, *contracts.RegistryAddError, error) {
38+
c.got = id
39+
return &config.RegistryEntry{ID: id}, nil, nil
40+
}
41+
42+
// A custom registry id is user-chosen (add-source --id) and may contain
43+
// characters the CLI and Web UI percent-encode. chi routes on RawPath, so the
44+
// {id} param arrives encoded and must be decoded before the exact-match lookup,
45+
// as handleAddFromRegistry already does.
46+
func TestRegistryRoutes_DecodeIDParam(t *testing.T) {
47+
routes := []struct{ method, path, body string }{
48+
{http.MethodGet, "/api/v1/registries/acme%3Ainternal/servers", ""},
49+
{http.MethodPost, "/api/v1/registries/acme%3Ainternal/refresh", ""},
50+
{http.MethodDelete, "/api/v1/registries/acme%3Ainternal", ""},
51+
{http.MethodPut, "/api/v1/registries/acme%3Ainternal", `{"name":"Acme"}`},
52+
}
53+
for _, rt := range routes {
54+
t.Run(rt.method+" "+rt.path, func(t *testing.T) {
55+
ctrl := &registryIDRecorder{MockServerController: &MockServerController{}}
56+
srv := NewServer(ctrl, zaptest.NewLogger(t).Sugar(), nil)
57+
58+
req := httptest.NewRequest(rt.method, rt.path, strings.NewReader(rt.body))
59+
req.Header.Set("X-API-Key", mockControllerAPIKey)
60+
w := httptest.NewRecorder()
61+
srv.ServeHTTP(w, req)
62+
63+
assert.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
64+
assert.Equal(t, "acme:internal", ctrl.got)
65+
})
66+
}
67+
}

‎internal/httpapi/server.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6419,7 +6419,7 @@ func (s *Server) handleListRegistries(w http.ResponseWriter, r *http.Request) {
64196419
// @Security ApiKeyQuery
64206420
// @Router /api/v1/registries/{id}/servers [get]
64216421
func (s *Server) handleSearchRegistryServers(w http.ResponseWriter, r *http.Request) {
6422-
registryID := chi.URLParam(r, "id")
6422+
registryID := decodePathParam(chi.URLParam(r, "id"))
64236423
if registryID == "" {
64246424
s.writeError(w, r, http.StatusBadRequest, "Registry ID is required")
64256425
return
@@ -6675,7 +6675,7 @@ func (s *Server) handleAddRegistrySource(w http.ResponseWriter, r *http.Request)
66756675
// @Security ApiKeyQuery
66766676
// @Router /api/v1/registries/{id} [delete]
66776677
func (s *Server) handleRemoveRegistrySource(w http.ResponseWriter, r *http.Request) {
6678-
registryID := chi.URLParam(r, "id")
6678+
registryID := decodePathParam(chi.URLParam(r, "id"))
66796679
if registryID == "" {
66806680
s.writeError(w, r, http.StatusBadRequest, "Registry ID is required")
66816681
return
@@ -6714,7 +6714,7 @@ func (s *Server) handleRemoveRegistrySource(w http.ResponseWriter, r *http.Reque
67146714
// @Security ApiKeyQuery
67156715
// @Router /api/v1/registries/{id} [put]
67166716
func (s *Server) handleEditRegistrySource(w http.ResponseWriter, r *http.Request) {
6717-
registryID := chi.URLParam(r, "id")
6717+
registryID := decodePathParam(chi.URLParam(r, "id"))
67186718
if registryID == "" {
67196719
s.writeError(w, r, http.StatusBadRequest, "Registry ID is required")
67206720
return
@@ -6753,7 +6753,7 @@ func (s *Server) handleEditRegistrySource(w http.ResponseWriter, r *http.Request
67536753
// @Failure 500 {object} contracts.ErrorResponse "Failed to refresh registry cache"
67546754
// @Router /api/v1/registries/{id}/refresh [post]
67556755
func (s *Server) handleRefreshRegistryCache(w http.ResponseWriter, r *http.Request) {
6756-
registryID := chi.URLParam(r, "id")
6756+
registryID := decodePathParam(chi.URLParam(r, "id"))
67576757
if registryID == "" {
67586758
s.writeError(w, r, http.StatusBadRequest, "Registry ID is required")
67596759
return

0 commit comments

Comments
 (0)