Conversation
… tests A failed `mcp-publisher login http` gives no way to tell whether the fault is the key, the file content, or the Registry's ability to reach the endpoint, and the Registry's egress addresses are not a fixed set that can be allowlisted. Document what the Registry actually requests (URL, headers, 200 only, no redirects, 4 KiB body limit, 10 second budget, Google Cloud origin), why a local curl proves nothing about server-side verification, and how to reproduce the fetch from outside the network that hosts the domain. Pin the same contract with tests. The existing fetcher tests inject a custom http.Client, so the production client's redirect policy was never exercised and nothing asserted the request headers or body trimming. Splitting the client construction into a helper that accepts a transport lets those tests drive the production timeout and redirect policy against a local TLS server; behavior is unchanged. Related to modelcontextprotocol#1625
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.
Summary
Answers the documentation ask in #1625: when HTTP verification fails there is no way to tell whether the fault is the key, the file, or the Registry's ability to reach the endpoint, and the Registry's egress addresses are not a fixed set that can be allowlisted.
Documented fetch contract (
docs/modelcontextprotocol-io/authentication.mdx)New "Troubleshooting failed HTTP verification" subsection describing what the Registry actually requests when
mcp-publisher login httpruns, why a successful localcurlproves nothing, and how to reproduce the fetch from outside your own network:GET https://<domain>/.well-known/mcp-registry-authwithAccept: text/plainandUser-Agent: mcp-registry/1.0200 OKonly — redirects are not followed, so a3xxfails verificationIt then lists the edge rules that break verification in practice (WAF/bot-management challenges, geo-blocking or allowlists that exclude cloud ranges, an unreachable
AAAArecord, redirects, an HTML error page served with200 OK), how to read the Registry's own fetch error out of the login failure, and where to fix it.The same contract is pinned by tests (
internal/api/handlers/v0/auth/http_internal_test.go)These properties were previously untested: the existing fetcher tests inject a custom
http.Client, so the production client's redirect policy was never exercised, and nothing asserted the request headers or whitespace trimming.TestHTTPKeyFetcherClient_DoesNotFollowRedirects— a301must fail verification and the redirect target must never be requested.TestHTTPKeyFetcherClient_TimeoutAndRequestHeaders— pins the 10 second timeout, the two documented request headers, andTrimSpaceon the proof record.TestHTTPKeyFetcher_RejectsResponseAboveDocumentedLimit— a200 OKbody above 4096 bytes fails.To make the production client testable, client construction moved into
newHTTPKeyFetcherClient(transport http.RoundTripper);NewDefaultHTTPKeyFetcherpasses the same cloned transport withsafeDialContext, so production behavior is unchanged. This PR changes no production behavior.Verification
go build ./...— clean.go vet ./...— clean.gofmt -l internal/— clean.go test ./internal/api/handlers/v0/auth/ -count=1—ok(whole package passes).CheckRedirectpolicy fromnewHTTPKeyFetcherClientmakes it fail withstopped after 10 redirects, which is exactly the regression it guards.go test ./internal/api/handlers/v0/ -count=1fails on this machine only because that suite needs PostgreSQL fromdocker-compose up -d postgres; unrelated to this change.golangci-lintis not installed locally, somake lintwas not run.Related to #1625.