fix(nuget): harden README fetch with body size limit + redirect host-pinning

[SaurabhMaydeo]

Follow-up to #1330, applying the same README-fetch hardening to the NuGet validator. Scoped to NuGet — no behavior change for existing publishers.

NuGet and cargo are the only validators that read a raw README body (npm/pypi decode JSON metadata directly), so this finishes the pair started in #1330.

Changes

• Cap the README body with io.LimitReader (5 MiB, matching maxCargoReadmeBytes), so an oversized response can't exhaust validator memory.
• Pin redirects on the client: each hop must stay on the originating request's host, and on the real NuGet base must keep https + the default port. This covers the service-index, README, and package-index fetches. The decision is a small pure function (nugetRedirectAllowed) with a table test, mirroring cargo's cargoURLAllowed / TestCargoURLAllowed.

Not publisher-exploitable today — the README URL comes from the trusted service index and is served same-host — so this is defense-in-depth, same framing as #1330.

NuGet has no separate README CDN host like cargo's static.crates.io: the README URL is built from the service-index ReadmeUriTemplate and served from the same host. So redirects are pinned to the request's own host rather than a static allowlist. If NuGet ever serves READMEs cross-host, this would need to become an allowlist like cargo's.

Test plan

• go test ./internal/validators/registries/ (incl. existing live-API tests)
• New unit tests: TestNuGetRedirectAllowed (cross-host, http-downgrade, non-default-port, metadata-IP, userinfo) + httptest tests for README truncation at the cap and cross-host redirect refusal
• gofmt, go vet, and golangci-lint v2.11.4 (the version CI pins) all clean — 0 issues

Docker/Postgres integration tests run in CI.

[UgaTheDev]

Nice, tightly-scoped hardening. Two things I want to call out as genuinely well done before the nits:

The redirect pin is anchored correctly. CheckRedirect compares each hop against via[0].URL rather than the previous hop, which is the form that actually holds. I checked that it's enforced on every hop, not just the first, with a two-hop chain where the first redirect stays on-host and the second leaves it:

hops served=2 err=failed to fetch NuGet README: Get "http://evil.example/x":
  refusing redirect to unexpected URL "http://evil.example/x"

Laundering — hop to a forbidden host and then back to the allowed one — is refused at the forbidden hop, so the forbidden host is never connected to:

hops=1 err=failed to fetch NuGet README: Get "http://127.0.0.2:1/relay?back=...":
  refusing redirect to unexpected URL "http://127.0.0.2:1/relay?back=..."

And re-implementing the 10-hop cap is not redundant — defining CheckRedirect replaces net/http's default limit, so dropping it would have introduced an infinite-redirect hang. It works:

hops=10 err=failed to fetch NuGet README: Get "/loop10": stopped after 10 redirects

Legitimate multi-hop same-host chains still resolve (4 hops, ValidReadme), so the pin isn't over-tight for real traffic.

The size cap is enforced before the read, not after. io.ReadAll(io.LimitReader(...)) is the right order — memory is bounded rather than checked once already consumed. Serving 64 MiB against the 5 MiB cap:

served=64 MiB cap=5 MiB state=InvalidReadme totalAllocDelta=16 MiB

A Content-Length that lies in the other direction (claims 10 bytes, sends 32 MiB) doesn't bypass it either — the transport truncates and the validator fails closed:

err=failed to read NuGet README content: unexpected EOF

I also confirmed the behaviour-preservation claim against live nuget.org rather than taking it on faith. The service index resolves both relevant resources to the same host as the base URL:

PackageBaseAddress/3.0.0    https://api.nuget.org/v3-flatcontainer/
ReadmeUriTemplate/6.13.0    https://api.nuget.org/v3-flatcontainer/{lower_id}/{lower_version}/readme

and real README fetches return 200 with no redirect at all (redirect_url= empty for newtonsoft.json/13.0.3 and serilog/4.2.0), so no existing package loses validation to the new policy.

One note on severity, since the PR description undersells it: this is not CLI-only hardening. ValidateNuGet is reached from ValidatePublishRequest in the server's publish path and from ValidateUpdateRequest on edit, gated on EnableRegistryValidation, which defaults to true. So the unbounded io.ReadAll this replaces was a server-side memory amplification reachable by any authenticated publisher: point a package's embedded README at something huge and the registry buffers all of it during publish. Worth saying so in the description — it makes the case for merging stronger, not weaker.

Three non-blocking observations:

1. The service-index and package-index reads are still unbounded. The new client is used for all three NuGet calls and the redirect pin covers all three, but only the README path got a size cap. json.NewDecoder(resp.Body).Decode(&index) streams without limit, and the result is then retained in serviceIndexCache for cacheDuration (1 hour). Streaming 48 MiB of syntactically valid service-index JSON:

streamed=48 MiB resources parsed=883012 err=<nil>
  totalAllocDelta=326 MiB heapInUseDelta=209 MiB

That 209 MiB stays resident for the cache duration. The versions array in validatePackageExists has the same shape. In production this is much less alarming than it looks — validateAndNormalizeBaseURL rejects any base URL that isn't the canonical NuGet one, so there's exactly one cache key and its content comes from api.nuget.org, the same host the README comes from. But that's precisely the point: if the README response is untrusted enough to bound, so is the index response from the same host. Not something I'd block on, and arguably its own PR, but the comment on maxNuGetReadmeBytes claims a hostile response "cannot exhaust validator memory" and that's only true of one of the three paths.

2. Truncation is silent and indistinguishable from a genuinely missing token. If a README exceeds the cap and the mcp-name: token sits past it, the publisher gets "must appear as 'mcp-name: ...' in the package README. Add it to your README" — which is misleading, since they did. Practically this is remote: the caps are ~600x the largest README I sampled (1.9 KB and 8.0 KB). But the fix is cheap and the idiom already exists in this repo — internal/api/handlers/v0/auth/http.go reads MaxKeyResponseSize+1 precisely so it can tell the two cases apart. Reading maxNuGetReadmeBytes+1 and branching on len(b) > maxNuGetReadmeBytes would let you say "README exceeds 5 MiB; the ownership token must appear within the first 5 MiB" instead. Same for cargo, which has the same silent truncation.

3. Host comparison is exact-string, so it over-blocks equivalent hostnames. target.Hostname() != origin.Hostname() is a byte comparison, and net/url does not normalize case or the trailing root dot:

target=https://API.NUGET.ORG/...        Hostname()="API.NUGET.ORG"   allowed=false
target=https://api.nuget.org./...       Hostname()="api.nuget.org."  allowed=false
target=https://api.nuget.org:443/x      Port()="443"                 allowed=true
target=https://api.nuget.org:0443/x     Port()="0443"                allowed=false

All four fail closed, so none of these is a hole — but the first two are the same host being refused, which would surface as a confusing publish failure if an upstream ever emitted a differently-cased Location. strings.EqualFold plus trimming a trailing dot would make the comparison equivalence-correct rather than string-correct. Low priority given the verified absence of redirects on the real endpoints. Cargo's allow-list lookup has the same case sensitivity, if you want to fix both together.

Worth noting for anyone reading the policy later: the pin anchors to the host of the originating request, and for the README that host comes from the service index's ReadmeUriTemplate. So the guarantee is "a 3xx cannot move the fetch off the host the index pointed us at" — the index itself remains the trust root. The doc comment already says as much, which I appreciate; it's the honest framing.

Tests pass, including the pre-existing live-network ones:

ok  github.com/modelcontextprotocol/registry/internal/validators              13.220s
ok  github.com/modelcontextprotocol/registry/internal/validators/registries   48.923s

The new table test covering the scheme/port branch that httptest bases can't reach is a good call — that branch is unreachable from ValidateNuGet in production (the base URL check forces the strict path), so a direct unit test is the only way it gets exercised at all.

None of the above blocks this. The mechanism does what it says and I couldn't get past either guard.

[JosephDoUrden]

Pulled the branch and ran it locally. go build and go test ./internal/validators/... pass, including the live nuget.org tests, and a local merge with current main is clean and still green. I also tried weakening the guards one at a time (removed the LimitReader, made the host check accept everything, dropped the scheme/port branch), every one got caught by the new tests, so the behaviour is properly pinned. The policy matches the merged cargo one from #1330, same 5 MiB cap and same scheme/port rules, just anchored to the request's own host instead of a static allowlist, the description explains why. One small gap vs cargo: the URL built from ReadmeUriTemplate is fetched with no pre-check, only redirect hops are covered. The live index serves the template from api.nuget.org anyway, so pinning it would change nothing for real packages, fine as a follow-up. The earlier comment already covers the service-index read and truncation UX points, so not repeating those. Looks merge-ready to me.

[UgaTheDev]

Re-checked the current head (d6151d9) and I have nothing further — CI is green and my earlier points are addressed. Nothing blocking from my side; ready for a maintainer.