Visitar URL original
fix(cargo): harden README fetch, clarify status handling, strengthen … · modelcontextprotocol/registry@7a6d1bc · GitHub
Skip to content

Commit 7a6d1bc

Browse files
rdimitrovclaude
rdimitrov
and
claude
authored
fix(cargo): harden README fetch, clarify status handling, strengthen tests (#1330)
Follow-up to #1207, addressing items from the cargo validator review. **Scoped to cargo + shared helpers — no behavior change for existing npm/pypi/nuget/oci/mcpb publishers.** Safe to merge and promote. ### Changes **SSRF hardening of the two-call README retrieval** - Pin the step-2 README URL to an allowed host (`static.crates.io` in prod; the base host under test) *before* fetching, so a metadata response can't steer the validator at an internal/attacker host. - `CheckRedirect` policy pins every redirect hop to the same allowlist (the initial URL being pinned isn't enough if an upstream 3xx is followed). - Cap the README body with `io.LimitReader` (5 MiB). > Not publisher-exploitable today (the URL comes from crates.io), so this is defense-in-depth — but it makes the "host is pinned" guarantee hold in code rather than rely on crates.io's behavior. **Clearer status handling** (previously any non-5xx/non-200 collapsed to "not found") - **429** → reported as transient/retryable, at both the metadata and README steps. - **403** → disambiguated via the crate-version endpoint: genuinely-missing stays "not found"; *exists-but-no-README* gets an actionable "add a README with `mcp-name` and republish" message (mirrors the NuGet validator). This continues the 5xx-vs-403 diagnostic split @P4ST4S started in the #1207 review — thanks! **Shared `containsMCPNameToken` helper** Boundary-anchored ownership-token match (prevents prefix confusion, e.g. a README declaring `io.github.acme/widget-pro` satisfying a claim for `io.github.acme/widget`), used here **by the cargo validator only**. Adopting it in PyPI/NuGet is a separate follow-up because it's a behavior change for those already-live registries. **Tests & docs** Hermetic positive `ServerNameFormats`; combined fixture gains a version-existence endpoint + cases for 403-missing vs 403-no-README, 429, and prefix-confusion rejection; new foreign-README-host (SSRF) test; internal test for the helper. Docs: list `crates.io` in the registry-requirements list + the `Package` model doc. ### Testing `go build`, `go vet`, full `./internal/validators/...` suite (incl. live cargo positive), `make check-schema`, `validate-examples` all pass. No new `golangci-lint` findings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 227621f commit 7a6d1bc

6 files changed

Lines changed: 370 additions & 52 deletions

File tree

‎docs/reference/server-json/official-registry-requirements.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ Only trusted public registries are supported. Private registries and alternative
3333
- **NPM**: `https://registry.npmjs.org` only
3434
- **PyPI**: `https://pypi.org` only
3535
- **NuGet**: `https://api.nuget.org/v3/index.json` only
36+
- **Cargo**: `https://crates.io` only
3637
- **Docker/OCI**:
3738
- Docker Hub (`docker.io`)
3839
- GitHub Container Registry (`ghcr.io`)

‎internal/validators/registries/cargo.go‎

Lines changed: 157 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"io"
99
"net/http"
1010
"net/url"
11-
"strings"
1211
"time"
1312

1413
"github.com/modelcontextprotocol/registry/pkg/model"
@@ -19,6 +18,19 @@ var (
1918
ErrMissingVersionForCargo = errors.New("package version is required for Cargo packages")
2019
)
2120

21+
// cargoUserAgent identifies the validator to crates.io. crates.io's crawler
22+
// policy expects a non-generic User-Agent with a contact URL; a bare UA may be
23+
// rate-limited or blocked. (Distinct from the package-level userAgent constant
24+
// used by the NuGet validator, which has no contact URL.)
25+
const cargoUserAgent = "MCP-Registry-Validator/1.0 (https://registry.modelcontextprotocol.io)"
26+
27+
// cargoStaticHost is the CDN host crates.io serves rendered READMEs from.
28+
const cargoStaticHost = "static.crates.io"
29+
30+
// maxCargoReadmeBytes caps how much of a rendered README we buffer, so a hostile
31+
// or oversized response cannot exhaust validator memory.
32+
const maxCargoReadmeBytes = 5 << 20 // 5 MiB
33+
2234
// CargoReadmeMetaResponse is the structure returned by the crates.io readme metadata endpoint.
2335
//
2436
// With `Accept: application/json`, crates.io's /api/v1/crates/{name}/{version}/readme
@@ -74,14 +86,134 @@ func ValidateCargo(ctx context.Context, pkg model.Package, serverName string) er
7486
return validateCargoREADME(ctx, pkg, serverName)
7587
}
7688

89+
// cargoAllowedHosts returns the set of hosts the validator is permitted to talk
90+
// to for a given base URL. For the real crates.io base this is crates.io (the API)
91+
// plus static.crates.io (the rendered-README CDN). For any other base — only the
92+
// httptest-driven tests, since the public ValidateCargo pins the base to
93+
// crates.io — it is the base host itself, so mock servers keep working.
94+
//
95+
// This is the allowlist enforced both on the README pointer (step 2 URL) and on
96+
// every redirect hop, so a metadata response or redirect cannot steer the
97+
// validator at an internal or attacker-chosen host (SSRF).
98+
func cargoAllowedHosts(baseURL string) map[string]struct{} {
99+
hosts := map[string]struct{}{}
100+
if u, err := url.Parse(baseURL); err == nil && u.Hostname() != "" {
101+
hosts[u.Hostname()] = struct{}{}
102+
}
103+
if baseURL == model.RegistryURLCrates {
104+
hosts[cargoStaticHost] = struct{}{}
105+
}
106+
return hosts
107+
}
108+
109+
// newCargoHTTPClient builds the client used for all crates.io calls. The
110+
// CheckRedirect policy pins every redirect hop to allowedHosts, so even though
111+
// the initial URL is host-pinned, an upstream 3xx cannot redirect the validator
112+
// to an unexpected host.
113+
func newCargoHTTPClient(allowedHosts map[string]struct{}) *http.Client {
114+
return &http.Client{
115+
Timeout: 10 * time.Second,
116+
CheckRedirect: func(req *http.Request, via []*http.Request) error {
117+
if _, ok := allowedHosts[req.URL.Hostname()]; !ok {
118+
return fmt.Errorf("refusing redirect to unexpected host %q", req.URL.Hostname())
119+
}
120+
if len(via) >= 10 {
121+
return errors.New("stopped after 10 redirects")
122+
}
123+
return nil
124+
},
125+
}
126+
}
127+
128+
// cargoVersionExists checks whether a specific crate version exists on crates.io,
129+
// used to disambiguate a 403 from the README CDN. static.crates.io (S3) returns
130+
// 403 both for a genuinely-missing crate/version AND for a crate that exists but
131+
// has no rendered README, so a 403 alone cannot tell a publisher which it is.
132+
//
133+
// Returns (exists, determined): determined is false if the existence endpoint
134+
// itself was unreachable or returned an unexpected status, in which case the
135+
// caller should fall back to a generic message rather than assert existence.
136+
func cargoVersionExists(ctx context.Context, client *http.Client, baseURL, identifier, version string) (exists, determined bool) {
137+
versionURL := fmt.Sprintf("%s/api/v1/crates/%s/%s",
138+
baseURL, url.PathEscape(identifier), url.PathEscape(version))
139+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, versionURL, nil)
140+
if err != nil {
141+
return false, false
142+
}
143+
req.Header.Set("User-Agent", cargoUserAgent)
144+
req.Header.Set("Accept", "application/json")
145+
146+
resp, err := client.Do(req)
147+
if err != nil {
148+
return false, false
149+
}
150+
defer resp.Body.Close()
151+
152+
switch resp.StatusCode {
153+
case http.StatusOK:
154+
return true, true
155+
case http.StatusNotFound:
156+
return false, true
157+
default:
158+
return false, false
159+
}
160+
}
161+
162+
// cargoMetadataStatusError maps a non-200 status from the metadata endpoint to a
163+
// caller-actionable error, distinguishing transient upstream conditions (429,
164+
// 5xx) from a genuine fetch failure.
165+
func cargoMetadataStatusError(identifier string, status int) error {
166+
switch {
167+
case status == http.StatusTooManyRequests:
168+
return fmt.Errorf("crates.io rate-limited the metadata request for cargo package '%s' (status: 429) — likely transient, retry later", identifier)
169+
case status >= 500 && status < 600:
170+
return fmt.Errorf("crates.io upstream error fetching metadata for cargo package '%s' (status: %d) — likely transient, retry later", identifier, status)
171+
default:
172+
return fmt.Errorf("cargo package '%s' metadata fetch failed (status: %d)", identifier, status)
173+
}
174+
}
175+
176+
// cargoReadmeStatusError maps a non-200 status from the README CDN to a
177+
// caller-actionable error. 429/5xx are transient; 403 is disambiguated (see
178+
// cargoReadme403Error) because static.crates.io returns it both for a missing
179+
// crate/version and for a crate that has no rendered README.
180+
func cargoReadmeStatusError(ctx context.Context, client *http.Client, pkg model.Package, serverName string, status int) error {
181+
switch {
182+
case status == http.StatusTooManyRequests:
183+
return fmt.Errorf("crates.io rate-limited the README fetch for cargo package '%s' version '%s' (status: 429) — likely transient, retry later", pkg.Identifier, pkg.Version)
184+
case status >= 500 && status < 600:
185+
return fmt.Errorf("crates.io upstream error fetching README for cargo package '%s' version '%s' (status: %d) — likely transient, retry later", pkg.Identifier, pkg.Version, status)
186+
case status == http.StatusForbidden:
187+
return cargoReadme403Error(ctx, client, pkg, serverName)
188+
default:
189+
return fmt.Errorf("cargo package '%s' version '%s' README fetch failed (status: %d)", pkg.Identifier, pkg.Version, status)
190+
}
191+
}
192+
193+
// cargoReadme403Error disambiguates a 403 from static.crates.io (S3's default
194+
// for a missing key): a genuinely-missing crate/version versus a crate that
195+
// exists but has no rendered README. It probes the crate-version metadata
196+
// endpoint so the publisher gets an actionable message rather than a blanket
197+
// "not found".
198+
func cargoReadme403Error(ctx context.Context, client *http.Client, pkg model.Package, serverName string) error {
199+
exists, determined := cargoVersionExists(ctx, client, pkg.RegistryBaseURL, pkg.Identifier, pkg.Version)
200+
switch {
201+
case determined && exists:
202+
return fmt.Errorf("cargo package '%s' version '%s' exists on crates.io but has no rendered README. Add a README containing 'mcp-name: %s' and publish a new version", pkg.Identifier, pkg.Version, serverName)
203+
case determined && !exists:
204+
return fmt.Errorf("cargo package '%s' version '%s' not found on crates.io", pkg.Identifier, pkg.Version)
205+
default:
206+
return fmt.Errorf("cargo package '%s' version '%s' not found on crates.io (status: 403)", pkg.Identifier, pkg.Version)
207+
}
208+
}
209+
77210
// validateCargoREADME performs the two-call README fetch and the mcp-name token
78211
// check. It is split out from ValidateCargo so that httptest-based tests can
79212
// drive the HTTP pipeline against a mock server (exposed via export_test.go),
80213
// bypassing the exact-baseURL guard that ValidateCargo enforces for callers.
81214
func validateCargoREADME(ctx context.Context, pkg model.Package, serverName string) error {
82-
client := &http.Client{Timeout: 10 * time.Second}
83-
// crates.io's crawler policy expects a non-generic User-Agent identifying the source.
84-
userAgent := "MCP-Registry-Validator/1.0 (https://registry.modelcontextprotocol.io)"
215+
allowedHosts := cargoAllowedHosts(pkg.RegistryBaseURL)
216+
client := newCargoHTTPClient(allowedHosts)
85217

86218
// Step 1: fetch the README pointer from the documented API endpoint.
87219
metaURL := fmt.Sprintf("%s/api/v1/crates/%s/%s/readme",
@@ -93,7 +225,7 @@ func validateCargoREADME(ctx context.Context, pkg model.Package, serverName stri
93225
if err != nil {
94226
return fmt.Errorf("failed to create crates.io metadata request: %w", err)
95227
}
96-
metaReq.Header.Set("User-Agent", userAgent)
228+
metaReq.Header.Set("User-Agent", cargoUserAgent)
97229
metaReq.Header.Set("Accept", "application/json")
98230

99231
metaResp, err := client.Do(metaReq)
@@ -103,11 +235,7 @@ func validateCargoREADME(ctx context.Context, pkg model.Package, serverName stri
103235
defer metaResp.Body.Close()
104236

105237
if metaResp.StatusCode != http.StatusOK {
106-
// 5xx from the metadata endpoint is upstream availability, not a missing crate.
107-
if metaResp.StatusCode >= 500 && metaResp.StatusCode < 600 {
108-
return fmt.Errorf("crates.io upstream error fetching metadata for cargo package '%s' (status: %d) — likely transient, retry later", pkg.Identifier, metaResp.StatusCode)
109-
}
110-
return fmt.Errorf("cargo package '%s' metadata fetch failed (status: %d)", pkg.Identifier, metaResp.StatusCode)
238+
return cargoMetadataStatusError(pkg.Identifier, metaResp.StatusCode)
111239
}
112240

113241
var meta CargoReadmeMetaResponse
@@ -118,12 +246,22 @@ func validateCargoREADME(ctx context.Context, pkg model.Package, serverName stri
118246
return fmt.Errorf("cargo package '%s' metadata response missing 'url' field", pkg.Identifier)
119247
}
120248

121-
// Step 2: fetch the rendered README from the URL the API gave us.
249+
// Pin the README pointer to an allowed host before fetching it, so a metadata
250+
// response cannot steer the validator at an internal or attacker-chosen host.
251+
readmeParsed, err := url.Parse(meta.URL)
252+
if err != nil || readmeParsed.Hostname() == "" {
253+
return fmt.Errorf("cargo package '%s': crates.io returned an unparseable README URL", pkg.Identifier)
254+
}
255+
if _, ok := allowedHosts[readmeParsed.Hostname()]; !ok {
256+
return fmt.Errorf("cargo package '%s': crates.io returned a README URL on unexpected host %q — refusing to fetch", pkg.Identifier, readmeParsed.Hostname())
257+
}
258+
259+
// Step 2: fetch the rendered README from the (now host-validated) URL.
122260
readmeReq, err := http.NewRequestWithContext(ctx, http.MethodGet, meta.URL, nil)
123261
if err != nil {
124262
return fmt.Errorf("failed to create crates.io readme request: %w", err)
125263
}
126-
readmeReq.Header.Set("User-Agent", userAgent)
264+
readmeReq.Header.Set("User-Agent", cargoUserAgent)
127265
readmeReq.Header.Set("Accept", "text/html")
128266

129267
readmeResp, err := client.Do(readmeReq)
@@ -132,27 +270,20 @@ func validateCargoREADME(ctx context.Context, pkg model.Package, serverName stri
132270
}
133271
defer readmeResp.Body.Close()
134272

135-
// Missing crates and missing versions surface as 403 from static.crates.io
136-
// (S3's default for missing keys), not 404. 5xx from the CDN is upstream
137-
// availability — surface it as transient so callers can distinguish retryable
138-
// failures from genuinely missing crates.
139273
if readmeResp.StatusCode != http.StatusOK {
140-
if readmeResp.StatusCode >= 500 && readmeResp.StatusCode < 600 {
141-
return fmt.Errorf("crates.io upstream error fetching README for cargo package '%s' version '%s' (status: %d) — likely transient, retry later", pkg.Identifier, pkg.Version, readmeResp.StatusCode)
142-
}
143-
return fmt.Errorf("cargo package '%s' version '%s' not found on crates.io (status: %d)", pkg.Identifier, pkg.Version, readmeResp.StatusCode)
274+
return cargoReadmeStatusError(ctx, client, pkg, serverName, readmeResp.StatusCode)
144275
}
145276

146-
body, err := io.ReadAll(readmeResp.Body)
277+
body, err := io.ReadAll(io.LimitReader(readmeResp.Body, maxCargoReadmeBytes))
147278
if err != nil {
148279
return fmt.Errorf("failed to read rendered README: %w", err)
149280
}
150281

151-
// Search for the mcp-name: <server-name> token. The token contains no characters
152-
// that get HTML-escaped during README rendering (no <, >, &, ", '), so a direct
153-
// substring match against the rendered HTML is reliable.
154-
mcpNamePattern := "mcp-name: " + serverName
155-
if strings.Contains(string(body), mcpNamePattern) {
282+
// Search for the mcp-name: <server-name> ownership token. The token contains no
283+
// characters that get HTML-escaped during README rendering (no <, >, &, ", '),
284+
// so matching against the rendered HTML is reliable; containsMCPNameToken
285+
// additionally requires a trailing boundary to avoid prefix confusion.
286+
if containsMCPNameToken(string(body), serverName) {
156287
return nil
157288
}
158289

0 commit comments

Comments
 (0)