fix(api): decode %2F in /servers/{id}/* path param via middleware (MCP-1118)#624
Merged
Conversation
…P-1118)
All /api/v1/servers/{id}/* sub-resource handlers were receiving the raw
percent-encoded {id} param from chi (e.g. io.github.owner%2Frepo for a
slash-name official-registry server). This caused 404s for the Tools and
Logs tabs in the Web UI and for CLI sub-resource commands on slash-name
servers, since exact-match server lookups never found the encoded literal.
Fix: add decodeServerIDParam middleware to the /servers/{id} route group.
It decodes {id} in the chi route context once, so every handler in the
subtree (tools, logs, restart, approve, scan, …) receives the real name
without each one needing an explicit decodePathParam call.
Also remove the now-redundant per-handler decodePathParam in
handleGetServerLogs (was the only handler previously fixed individually).
Same root cause as MCP-1056 (registry add-server slash ids, PR #591).
Tests: TestServerSubresource_SlashServerIDUnescaped (tools/restart/tool-calls)
TestGetServerLogs_SlashServerIDUnescaped (existing, still passes)
Deploying mcpproxy-docs with
|
| Latest commit: |
6acf036
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d985fc92.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://mcp-1118-decode-server-path.mcpproxy-docs.pages.dev |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 27156372175 --repo smart-mcp-proxy/mcpproxy-go
|
There was a problem hiding this comment.
✅ Gatekeeper approval — Codex review verdict: ACCEPT.
This approval is posted automatically by the MCPProxy Gatekeeper App on behalf of the Codex reviewer (verdict of record lives in the Paperclip review thread). Author≠approver satisfied; QA + CI gates enforced separately.
Auto-approved per Model B (MCP-1249).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Official modelcontextprotocol/registry v0.1 server IDs are
namespace/name(e.g.io.github.owner/repo). chi routes on RawPath, so the{id}path parameter of every/api/v1/servers/{id}/*sub-resource handler arrived percent-encoded (io.github.owner%2Frepo). This caused 404s for:mcpproxy upstream logs,upstream restart, and other sub-resource commandsSame root cause as MCP-1056 (registry add-server slash ids, PR #591).
Fix
Add a
decodeServerIDParammiddleware to the/servers/{id}route group. It decodes the{id}param in the chi route context once, so every handler in the subtree (tools, logs, restart, approve, scan, tool-calls, …) receives the real name without each one needing an explicitdecodePathParamcall.Also removes the now-redundant per-handler
decodePathParamcall fromhandleGetServerLogs(the only handler previously fixed individually).Tests
TestServerSubresource_SlashServerIDUnescaped— new; covers tools, restart, and tool-calls endpoints with slash-encoded server IDTestGetServerLogs_SlashServerIDUnescaped— existing; still passesCloses #598