Skip to content

Commit 1a24cbb

Browse files
authored
fix(registry): unescape slash serverId before add lookup (#591)
Official modelcontextprotocol/registry v0.1 ids are namespace/name. chi routes on RawPath, so {serverId} arrives percent-encoded (microsoft%2Fmarkitdown) and the exact-match registry lookup never matched, making every namespaced server un-addable on CLI, REST, and Web UI (discovery was unaffected). handleAddFromRegistry now url.PathUnescape-s both the registry id and server id (via a decodePathParam helper that falls back to the raw value on a malformed escape) before AddServerFromRegistryRef. Adds an end-to-end regression test driving a %2F-encoded serverId through the chi router. Related MCP-1056
1 parent 3d3c06d commit 1a24cbb

2 files changed

Lines changed: 75 additions & 2 deletions

File tree

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package httpapi
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"net/http"
7+
"net/http/httptest"
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
"go.uber.org/zap/zaptest"
13+
14+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
15+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts"
16+
)
17+
18+
// capturingRegistryController records the exact registryID/serverID the HTTP
19+
// handler forwards to the controller, so the test can assert they were
20+
// percent-decoded before lookup (MCP-1056).
21+
type capturingRegistryController struct {
22+
*MockServerController
23+
gotRegistryID string
24+
gotServerID string
25+
}
26+
27+
func (c *capturingRegistryController) AddServerFromRegistryRef(_ context.Context, registryID, serverID, _ string, _ map[string]string, _ *bool) (*config.ServerConfig, *contracts.RegistryAddError, error) {
28+
c.gotRegistryID = registryID
29+
c.gotServerID = serverID
30+
return &config.ServerConfig{Name: "markitdown", Protocol: "stdio", Enabled: true}, nil, nil
31+
}
32+
33+
// TestAddFromRegistry_SlashServerIDUnescaped reproduces MCP-1056: official v0.1
34+
// registry ids are namespace/name. chi routes on RawPath, so the {serverId}
35+
// path param arrives percent-encoded (microsoft%2Fmarkitdown). The handler must
36+
// url.PathUnescape it before lookup, otherwise the exact-match lookup fails with
37+
// server_not_found and the server is un-addable on CLI/REST/Web UI.
38+
func TestAddFromRegistry_SlashServerIDUnescaped(t *testing.T) {
39+
logger := zaptest.NewLogger(t).Sugar()
40+
controller := &capturingRegistryController{MockServerController: &MockServerController{}}
41+
server := NewServer(controller, logger, nil)
42+
43+
// microsoft/markitdown, percent-encoded as a single path segment.
44+
req := httptest.NewRequest(http.MethodPost, "/api/v1/registries/github-mcp/servers/microsoft%2Fmarkitdown/add", http.NoBody)
45+
w := httptest.NewRecorder()
46+
server.ServeHTTP(w, req)
47+
48+
require.Equal(t, http.StatusOK, w.Code, "add should succeed; body=%s", w.Body.String())
49+
50+
var resp contracts.APIResponse
51+
require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp))
52+
assert.True(t, resp.Success, "response should indicate success")
53+
54+
assert.Equal(t, "microsoft/markitdown", controller.gotServerID, "serverId must be percent-decoded before registry lookup")
55+
assert.Equal(t, "github-mcp", controller.gotRegistryID, "registry id must be percent-decoded before lookup")
56+
}

internal/httpapi/server.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"io"
99
"math"
1010
"net/http"
11+
"net/url"
1112
"sort"
1213
"strconv"
1314
"strings"
@@ -4136,9 +4137,25 @@ func (s *Server) handleSearchRegistryServers(w http.ResponseWriter, r *http.Requ
41364137
// @Security ApiKeyAuth
41374138
// @Security ApiKeyQuery
41384139
// @Router /api/v1/registries/{id}/servers/{serverId}/add [post]
4140+
// decodePathParam percent-decodes a chi path parameter. chi matches routes on
4141+
// the raw (encoded) path, so parameters that legitimately contain reserved
4142+
// characters such as "/" (encoded as %2F) arrive encoded. On a malformed escape
4143+
// sequence it returns the original value unchanged so the downstream lookup can
4144+
// surface a normal not-found rather than a decode panic.
4145+
func decodePathParam(raw string) string {
4146+
if decoded, err := url.PathUnescape(raw); err == nil {
4147+
return decoded
4148+
}
4149+
return raw
4150+
}
4151+
41394152
func (s *Server) handleAddFromRegistry(w http.ResponseWriter, r *http.Request) {
4140-
registryID := chi.URLParam(r, "id")
4141-
serverID := chi.URLParam(r, "serverId")
4153+
// chi routes on RawPath, so path params arrive percent-encoded. Official
4154+
// modelcontextprotocol/registry v0.1 ids are namespace/name, so the slash
4155+
// reaches us as %2F and must be decoded before the exact-match registry
4156+
// lookup, otherwise every namespaced server is un-addable (MCP-1056).
4157+
registryID := decodePathParam(chi.URLParam(r, "id"))
4158+
serverID := decodePathParam(chi.URLParam(r, "serverId"))
41424159
if registryID == "" || serverID == "" {
41434160
s.writeError(w, r, http.StatusBadRequest, "registry id and server id are required")
41444161
return

0 commit comments

Comments
 (0)