[rudycelekli]

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

• I have tested these changes locally
• I have added/updated tests that prove my fix is effective or my feature works
• 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.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Dumbris]

Verified: the PR test fails on main for the reason described and passes on this branch merged with main; race tests for the touched packages and CI lint (bare and server tags) are clean. Also built and tested together with the rest of this batch, independently reviewed, and cross-reviewed with a second model.

[Dumbris]

Merged. Thank you, @rudycelekli! The Review diff and the per-tool enable toggle now work for tool names the Web UI percent-encodes (ns:erase). (The CodeQL reflected-XSS note on this PR is a false positive: the value only reaches application/json responses written by json.Encoder, which HTML-escapes.)