04623ed925Registry-wide hardening of the `mcp-name:` ownership-token match, plus the cargo follow-up fixes from the #1330 review — consolidated here per maintainer request. Rebased onto current `main` (the merged #1330 cargo commit drops out), so the diff is just the net-new work. ### Changes - **PyPI/NuGet: boundary-anchored token match.** Replace `strings.Contains(readme, "mcp-name: "+name)` with the shared `containsMCPNameToken`, so a README declaring a longer name (`…/widget-pro`) no longer satisfies an ownership claim for a shorter prefix (`…/widget`). (Cargo already got this in #1330; NPM is unaffected — it compares an exact metadata field.) - **Matcher: treat the HTML comment close as a boundary.** `<!-- mcp-name: NAME-->` / `<!--mcp-name: NAME-->` (any spacing) validate again — the documented hidden-comment form for PyPI/NuGet — while a genuine longer name (`…/widget--pro`) still does not. - **Cargo hardening (review of #1330):** pin scheme+port (not just host) on the README fetch and redirects; a rate-limited/failed crate-version existence probe now reports *transient* instead of "not found"; a 403 with the crate present no longer flatly asserts "no README". - **Docs:** note the token must be followed by a boundary. ### ⚠️ Behavior change for PyPI/NuGet — read before merging The match is **strictly stricter** (verified by fuzz, 2.3M execs: it can only flip pass→fail, never fail→pass). After the comment-close fix, the **only** forms that newly fail are unusual *inline* ones: the token ending a sentence (`…/my-mcp.`) or glued to a trailing `/`. The documented forms (own line, in `<!-- … -->`) are unaffected. **Correcting an earlier claim:** this re-validates on **edits too**, not just new versions — `edit.go → UpdateServer → ValidateUpdateRequest → validateRegistryOwnership` runs the token check on any edit of a live server (only delete-transitions skip it). So an existing PyPI/NuGet server whose README uses one of the breaking inline forms would fail on its next publish *or* edit. Given the v0.1 API-freeze posture, this should land as a deliberate, noted change. ### Testing `go build`, `vet`, `gofmt`, `golangci-lint` (prod files), `check-schema`, `validate-examples` all clean. Hermetic + **live** PyPI/NuGet/cargo suite passes. New: `TestCargoURLAllowed`, a combined-fixture probe-429→transient case, comment-form matcher cases, and `FuzzContainsMCPNameToken` (the strictly-stricter property, 2.3M execs). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
47 lines
2 KiB
Go
47 lines
2 KiB
Go
package registries
|
|
|
|
import (
|
|
"net/url"
|
|
"testing"
|
|
|
|
"github.com/modelcontextprotocol/registry/pkg/model"
|
|
)
|
|
|
|
// TestCargoURLAllowed covers the SSRF allow-check: for the real crates.io base,
|
|
// the host must be allow-listed AND the scheme/port must be https/default; for a
|
|
// test (httptest) base only the host is checked so mocks keep working.
|
|
func TestCargoURLAllowed(t *testing.T) {
|
|
prodHosts := cargoAllowedHosts(model.RegistryURLCrates) // {crates.io, static.crates.io}
|
|
mockBase := "http://127.0.0.1:54321"
|
|
mockHosts := cargoAllowedHosts(mockBase) // {127.0.0.1}
|
|
|
|
cases := []struct {
|
|
desc string
|
|
raw string
|
|
baseURL string
|
|
hosts map[string]struct{}
|
|
want bool
|
|
}{
|
|
{"prod: https static.crates.io", "https://static.crates.io/readmes/x/x.html", model.RegistryURLCrates, prodHosts, true},
|
|
{"prod: https crates.io", "https://crates.io/api/v1/crates/x/1.0.0", model.RegistryURLCrates, prodHosts, true},
|
|
{"prod: http downgrade rejected", "http://static.crates.io/x", model.RegistryURLCrates, prodHosts, false},
|
|
{"prod: non-default port rejected", "https://static.crates.io:8443/x", model.RegistryURLCrates, prodHosts, false},
|
|
{"prod: explicit 443 ok", "https://static.crates.io:443/x", model.RegistryURLCrates, prodHosts, true},
|
|
{"prod: foreign host rejected", "https://evil.example/x", model.RegistryURLCrates, prodHosts, false},
|
|
{"prod: userinfo host is evil rejected", "https://static.crates.io@evil.example/x", model.RegistryURLCrates, prodHosts, false},
|
|
{"test base: mock host any scheme/port ok", "http://127.0.0.1:54321/readme-static/x", mockBase, mockHosts, true},
|
|
{"test base: foreign host rejected", "http://127.0.0.2:54321/x", mockBase, mockHosts, false},
|
|
}
|
|
|
|
for _, tc := range cases {
|
|
t.Run(tc.desc, func(t *testing.T) {
|
|
u, err := url.Parse(tc.raw)
|
|
if err != nil {
|
|
t.Fatalf("parse %q: %v", tc.raw, err)
|
|
}
|
|
if got := cargoURLAllowed(u, tc.baseURL, tc.hosts); got != tc.want {
|
|
t.Fatalf("cargoURLAllowed(%q, base=%q) = %v, want %v", tc.raw, tc.baseURL, got, tc.want)
|
|
}
|
|
})
|
|
}
|
|
}
|