Skip to content

docs(auth): document HTTP verification requirements and pin them with tests - #1640

Open
ump45nose wants to merge 1 commit into
modelcontextprotocol:mainfrom
ump45nose:contrib/3205bf050377
Open

ump45nose wants to merge 1 commit into
modelcontextprotocol:mainfrom
ump45nose:contrib/3205bf050377

Conversation

@ump45nose

Copy link
Copy Markdown

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 http runs, why a successful local curl proves nothing, and how to reproduce the fetch from outside your own network:

  • GET https://<domain>/.well-known/mcp-registry-auth with Accept: text/plain and User-Agent: mcp-registry/1.0
  • 200 OK only — redirects are not followed, so a 3xx fails verification
  • proof record of at most 4096 bytes, with surrounding whitespace ignored
  • 10 second budget for the whole request
  • the request originates from Google Cloud, where the hosted Registry runs, so per-request IP allowlisting is not a workable fix

It then lists the edge rules that break verification in practice (WAF/bot-management challenges, geo-blocking or allowlists that exclude cloud ranges, an unreachable AAAA record, redirects, an HTML error page served with 200 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 — a 301 must fail verification and the redirect target must never be requested.
  • TestHTTPKeyFetcherClient_TimeoutAndRequestHeaders — pins the 10 second timeout, the two documented request headers, and TrimSpace on the proof record.
  • TestHTTPKeyFetcher_RejectsResponseAboveDocumentedLimit — a 200 OK body above 4096 bytes fails.

To make the production client testable, client construction moved into newHTTPKeyFetcherClient(transport http.RoundTripper); NewDefaultHTTPKeyFetcher passes the same cloned transport with safeDialContext, 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=1ok (whole package passes).
  • The redirect test is load-bearing: deleting the CheckRedirect policy from newHTTPKeyFetcherClient makes it fail with stopped after 10 redirects, which is exactly the regression it guards.
  • go test ./internal/api/handlers/v0/ -count=1 fails on this machine only because that suite needs PostgreSQL from docker-compose up -d postgres; unrelated to this change. golangci-lint is not installed locally, so make lint was not run.

Related to #1625.

AI assistance disclosure: AI was used to discover this opportunity and draft the change or text. The submission was checked against the prepared artifact and recorded verification evidence.

… 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant