diff --git a/internal/validators/registries/nuget.go b/internal/validators/registries/nuget.go index c229ba02c..9ffaccba6 100644 --- a/internal/validators/registries/nuget.go +++ b/internal/validators/registries/nuget.go @@ -22,6 +22,11 @@ var ( const userAgent = "MCP-Registry-Validator/1.0" +// maxNuGetReadmeBytes caps how much of a package README we buffer, so a hostile +// or oversized response cannot exhaust validator memory. Mirrors the cargo +// validator's maxCargoReadmeBytes. +const maxNuGetReadmeBytes = 5 << 20 // 5 MiB + type cachedServiceIndex struct { index *serviceIndex expiresAt time.Time @@ -76,7 +81,7 @@ func ValidateNuGet(ctx context.Context, pkg model.Package, serverName string) er return ErrMissingVersionForNuget } - client := &http.Client{Timeout: 10 * time.Second} + client := newNuGetHTTPClient(pkg.RegistryBaseURL) // Fetch the service serviceIndex serviceIndex, err := fetchAndCacheServiceIndex(ctx, client, pkg.RegistryBaseURL) @@ -150,6 +155,56 @@ func validateAndNormalizeBaseURL(pkg *model.Package) error { return nil } +// nugetRedirectAllowed reports whether a redirect to target is permitted, given +// the originating request. The target host must match the origin's host; +// additionally, for the real NuGet base, the scheme must not change from the +// origin's (which is https) and the port must stay default — so a redirect +// cannot downgrade the scheme or steer the validator at a non-standard port on +// an otherwise-trusted host. For test (httptest) bases only the host is checked, +// so mocks on http/loopback keep working. +// +// This is the NuGet analogue of cargoURLAllowed. NuGet has no separate README +// CDN host (the README URL comes from the trusted service-index template and is +// served from the same host), so the allowed host is the origin's own host +// rather than a static allow-list. +func nugetRedirectAllowed(target, origin *url.URL, baseURL string) bool { + if target.Hostname() != origin.Hostname() { + return false + } + if baseURL == model.RegistryURLNuGet { + if target.Scheme != origin.Scheme { + return false + } + if p := target.Port(); p != "" && p != "443" { + return false + } + } + return true +} + +// newNuGetHTTPClient builds the client used for all NuGet calls (service index, +// README, and package index). Its CheckRedirect pins every redirect hop to the +// originating request's host (see nugetRedirectAllowed), so an upstream 3xx +// cannot steer the validator at an internal or attacker-chosen host (SSRF). +// NuGet serves these resources directly without cross-host redirects, so this is +// behaviour-preserving for real packages. +func newNuGetHTTPClient(baseURL string) *http.Client { + return &http.Client{ + Timeout: 10 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return errors.New("stopped after 10 redirects") + } + // via is non-empty whenever net/http invokes CheckRedirect, so + // via[0] is the originating request. + if !nugetRedirectAllowed(req.URL, via[0].URL, baseURL) { + return fmt.Errorf("refusing redirect to unexpected URL %q", req.URL.Redacted()) + } + return nil + }, + } +} + func fetchAndCacheServiceIndex(ctx context.Context, client *http.Client, baseURL string) (*serviceIndex, error) { cacheMu.RLock() if cached, exists := serviceIndexCache[baseURL]; exists { @@ -234,8 +289,9 @@ func validateReadme(ctx context.Context, serverName, lowerID, lowerVersion strin defer resp.Body.Close() if resp.StatusCode == http.StatusOK { - // Check README content - readmeBytes, err := io.ReadAll(resp.Body) + // Check README content. Bound the read so a hostile or oversized + // response cannot exhaust validator memory. + readmeBytes, err := io.ReadAll(io.LimitReader(resp.Body, maxNuGetReadmeBytes)) if err != nil { return NoReadme, fmt.Errorf("failed to read NuGet README content: %w", err) } diff --git a/internal/validators/registries/nuget_internal_test.go b/internal/validators/registries/nuget_internal_test.go new file mode 100644 index 000000000..7c5a90221 --- /dev/null +++ b/internal/validators/registries/nuget_internal_test.go @@ -0,0 +1,129 @@ +package registries + +import ( + "bytes" + "context" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/modelcontextprotocol/registry/pkg/model" +) + +// TestNuGetRedirectAllowed covers the redirect SSRF policy directly, including +// the real-base scheme/port branch that the httptest-driven tests below cannot +// exercise (they use http/loopback bases). Mirrors cargo's TestCargoURLAllowed. +func TestNuGetRedirectAllowed(t *testing.T) { + const prod = model.RegistryURLNuGet // https://api.nuget.org/v3/index.json + const prodOrigin = "https://api.nuget.org/v3-flatcontainer/x/1.0.0/readme" + const mockBase = "http://127.0.0.1:54321" + const mockOrigin = "http://127.0.0.1:54321/x/1.0.0/readme" + + cases := []struct { + desc string + target string + origin string + baseURL string + want bool + }{ + {"prod: same-host https default", prodOrigin, prodOrigin, prod, true}, + {"prod: same-host explicit :443", "https://api.nuget.org:443/x", prodOrigin, prod, true}, + {"prod: http downgrade refused", "http://api.nuget.org/x", prodOrigin, prod, false}, + {"prod: non-default port refused", "https://api.nuget.org:8080/internal", prodOrigin, prod, false}, + {"prod: cross-host refused", "https://evil.example/x", prodOrigin, prod, false}, + {"prod: metadata IP refused", "https://169.254.169.254/latest/meta-data/", prodOrigin, prod, false}, + {"prod: userinfo trick refused", "https://api.nuget.org@evil.example/x", prodOrigin, prod, false}, + {"test base: same host any scheme/port ok", "http://127.0.0.1:54321/readme", mockOrigin, mockBase, true}, + {"test base: cross host refused", "http://127.0.0.2:54321/x", mockOrigin, mockBase, false}, + } + + for _, tc := range cases { + t.Run(tc.desc, func(t *testing.T) { + target, err := url.Parse(tc.target) + if err != nil { + t.Fatalf("parse target %q: %v", tc.target, err) + } + origin, err := url.Parse(tc.origin) + if err != nil { + t.Fatalf("parse origin %q: %v", tc.origin, err) + } + if got := nugetRedirectAllowed(target, origin, tc.baseURL); got != tc.want { + t.Fatalf("nugetRedirectAllowed(%q, origin=%q, base=%q) = %v, want %v", tc.target, tc.origin, tc.baseURL, got, tc.want) + } + }) + } +} + +// nugetTestIndex builds a minimal service index whose ReadmeUriTemplate points +// at the given base, so validateReadme fetches the README from a mock server. +func nugetTestIndex(base string) *serviceIndex { + return &serviceIndex{Resources: []serviceIndexResource{ + {Type: "ReadmeUriTemplate/6.13.0", ID: base + "/{lower_id}/{lower_version}/readme"}, + }} +} + +// TestValidateReadme_TruncatesOversizedBody verifies the README read is bounded: +// a token placed past maxNuGetReadmeBytes is truncated away, so it is not found. +func TestValidateReadme_TruncatesOversizedBody(t *testing.T) { + const serverName = "io.github.test/pkg" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + // maxNuGetReadmeBytes of filler first, THEN the token — a correct + // io.LimitReader stops before the token is reached. + _, _ = w.Write(bytes.Repeat([]byte("a"), maxNuGetReadmeBytes)) + _, _ = fmt.Fprintf(w, "\nmcp-name: %s\n", serverName) + })) + defer srv.Close() + + client := newNuGetHTTPClient(srv.URL) + state, err := validateReadme(context.Background(), serverName, "pkg", "1.0.0", client, nugetTestIndex(srv.URL)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if state != InvalidReadme { + t.Fatalf("token past the %d-byte cap should be truncated -> InvalidReadme, got %v", maxNuGetReadmeBytes, state) + } +} + +// TestValidateReadme_TokenWithinCap is the positive control: a token within the +// cap is found normally. +func TestValidateReadme_TokenWithinCap(t *testing.T) { + const serverName = "io.github.test/pkg" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprintf(w, "intro text\nmcp-name: %s\n", serverName) + })) + defer srv.Close() + + client := newNuGetHTTPClient(srv.URL) + state, err := validateReadme(context.Background(), serverName, "pkg", "1.0.0", client, nugetTestIndex(srv.URL)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if state != ValidReadme { + t.Fatalf("expected ValidReadme, got %v", state) + } +} + +// TestValidateReadme_RefusesCrossHostRedirect verifies the client refuses a +// redirect that leaves the README host (SSRF guard). The redirect target is +// never connected to, because CheckRedirect rejects it first. +func TestValidateReadme_RefusesCrossHostRedirect(t *testing.T) { + const serverName = "io.github.test/pkg" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // 302 to a different host; must NOT be followed. + http.Redirect(w, r, "http://evil.example/readme", http.StatusFound) + })) + defer srv.Close() + + client := newNuGetHTTPClient(srv.URL) + _, err := validateReadme(context.Background(), serverName, "pkg", "1.0.0", client, nugetTestIndex(srv.URL)) + if err == nil { + t.Fatal("expected an error: a cross-host redirect should be refused") + } + if !strings.Contains(err.Error(), "redirect") { + t.Fatalf("expected a redirect-refusal error, got: %v", err) + } +}