Defer NuGet service index discovery until requested

[jeffwidman]

Why

NuGet feed configuration currently performs a synchronous service-index request for every configured feed before the proxy starts listening. Each request can wait up to 10 seconds, so readiness depends on external registries even when a feed is never used.

Local measurements:

Configuration	Time to Listening
One unreachable NuGet feed	10.39 s
Two unreachable NuGet feeds	20.48 s
Response-driven discovery	357 ms

This change removes all constructor-time HTTP requests. Unused feeds add no startup work, and registry outages no longer delay the proxy from accepting connections.

Impact and behavioral changes

• NuGet resource discovery now uses the service-index response that NuGet already requests through the proxy. Discovery completes before that response reaches NuGet, including when the response comes from the proxy cache, so subsequent resource requests can be authenticated normally.
• Scheme-less service-index URLs now support resource discovery when NuGet requests them over HTTP or HTTPS. Previously they could participate in request matching, but constructor discovery could not fetch them as absolute URLs.
• An explicitly configured service-index scheme is respected during discovery, and HTTP and HTTPS versions of the same index can coexist. Static request authentication remains scheme-agnostic for compatibility; OIDC remains HTTPS-only.
• Static credentials now prefer the matching URL with the longest path, with host-only credentials as fallback. A narrowly scoped feed credential can no longer be shadowed by a broader credential merely because it appeared first.
• Same-origin service-index redirects register an additional credential route. Cross-origin redirects continue discovery without registering one; independently matching credentials continue to apply under the existing NuGet matching behavior.
• Duplicate static resource URLs remain first-registration-wins, which supports legitimate feed aliases that publish the same resource endpoint. Later claims are ignored and logged without credential material instead of causing requests to fail.
• Existing trust behavior for cross-origin resource URLs declared by a successfully retrieved service index is unchanged.
• Live response bodies are replayed unchanged. Bodyless 204/205 responses remain bodyless, partial read failures preserve consumed and unread bytes, and unsuccessful or malformed responses do not register routes.

Validation

Coverage includes constructor startup behavior, static and OIDC discovery, cached responses, explicit and scheme-less URLs, HTTP/HTTPS coexistence, credential precedence, duplicate routes, redirects, unsuccessful responses, body replay, and concurrent registration.

• Both Docker targets built successfully.
• go test -race -shuffle=on -count=2 -v ./... passed in the test image.
• Follow-up changes pass go test -race ./internal/handlers -count=1.

Checklist

• Complete tests and linters pass.
• New behavior and regressions have focused coverage.
• The change and its impact are documented.

[Copilot]

Pull request overview

Defers NuGet service-index discovery from proxy startup to proxied index responses.

Changes:

• Adds response-driven, cache-compatible NuGet route discovery.
• Synchronizes and deduplicates discovered credentials.
• Expands static/OIDC discovery and redirect tests.

Show a summary per file

File	Description
proxy.go	Registers NuGet preparation before caching and response discovery afterward.
internal/handlers/nuget_feed.go	Implements deferred discovery, redirects, replay, and synchronized routes.
internal/handlers/nuget_feed_test.go	Tests discovery, redirects, concurrency, and body replay.
internal/handlers/oidc_handling_test.go	Adapts OIDC fixtures to response-driven discovery.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (3)

internal/handlers/nuget_feed.go:284

• If io.ReadAll returns some bytes together with an error, this replay body exposes only the consumed prefix and drops anything still readable from the original body. That changes/truncates the package manager's live response on the error path. Replay the consumed bytes followed by the original reader, as readPythonIndexDiscoveryBody does.

		Reader: bytes.NewReader(body),

internal/handlers/nuget_feed.go:291

• Runtime registration makes credential ownership depend on response timing. If two configured indexes concurrently advertise the same resource URL, both the static map and the OIDC registry are first-wins, so whichever index responds first supplies the credential; this was deterministic config order when discovery ran synchronously. Preserve an explicit config priority when deduplicating routes so concurrent responses cannot select a different feed's credential.

	for _, discoveredURL := range extraUrlsFromSourceResponse(body, discoveryAuth.serviceIndexURL) {

internal/handlers/nuget_feed.go:291

• The bytes read here can still be gzip/deflate/Brotli encoded when the NuGet client supplied Accept-Encoding; Go's transport does not transparently decompress responses when that header was already present. extraUrlsFromSourceResponse then sees compressed bytes and silently registers no resource routes, so subsequent cross-origin package requests are unauthenticated. Decode a copy according to Content-Encoding for discovery while replaying the original encoded bytes and headers to the client.

	for _, discoveredURL := range extraUrlsFromSourceResponse(body, discoveryAuth.serviceIndexURL) {

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[brettfo]

Is there a specific reason this (along with the call to PrepareRequest) was moved up here and not kept below?

[jeffwidman]

Yes. PrepareRequest has to run before cacher.OnRequest: a cache hit returns a response immediately and skips later request handlers. Marking the exact service-index request first puts the discovery metadata in the proxy context, so HandleResponse can learn routes from cached and live index responses identically. The actual NuGet authentication handler remains in its previous lower position.

[jeffwidman]

One intentional matching behavior change is worth calling out explicitly: static NuGet URL credentials now use the longest matching URL path, with host-only credentials as fallback.

For example, a credential scoped to /feed/specific wins over one scoped to /feed, regardless of insertion order. This follows the principle of least surprise: the most specifically scoped credential should authenticate the request rather than being shadowed by a broader URL or host credential. I have added this to the PR description and the code comment.

Equal discovered URLs claimed by different credentials are a different ambiguity; I will link a stacked security draft that proposes failing closed for those conflicts.

[jeffwidman]

Stacked security-policy draft: #206

It proposes failing closed when different credentials claim the same normalized NuGet route, while allowing the same effective credential to be deduplicated and leaving ordinary mirrors with distinct resource URLs unaffected. The draft calls out the remaining policy questions for discussion.

[jeffwidman]

Follow-up test-hygiene draft: #211 restores process-wide standard-log and Logrus state after tests that capture or reconfigure logging. It is stacked separately because the issue predates the NuGet discovery change.

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

internal/handlers/nuget_feed.go:362

• A URL-only entry with neither a token nor password still reserves its deduplication key, even though HandleRequest always skips that credential. A later usable credential (including one discovered from another feed) for the same URL is then rejected, so configurations that previously skipped the empty entry and used the later credential now send no authentication. Avoid storing unusable static credentials.

	if credential.url != "" {
		key := nugetCredentialURLKey(credential.url)
		if _, ok := h.credentialURLs[key]; ok {
			return false
		}

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

internal/handlers/nuget_feed.go:435

• This key drops the URL scheme even though service-index matching now requires an explicit scheme to match. If both http://host/index.json and https://host/index.json are configured, the second discovery source is deduplicated; requests using its scheme skip the retained source, so its response never registers resource routes. Use a separate discovery-source key containing the normalized scheme and scheme-appropriate default port; the shared static-credential key also needs care because changing it affects credential precedence.

	return strings.ToLower(parsedURL.Hostname()) + ":" + port + strings.TrimRight(parsedURL.Path, "/") + "?" + parsedURL.RawQuery

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (1)

internal/handlers/nuget_feed.go:289

• io.ReadAll now buffers an unbounded upstream body before NuGet receives any bytes. A slow or oversized response from a configured registry can therefore indefinitely stall this request or exhaust proxy memory. Bound discovery reads (as readPythonIndexDiscoveryBody does), skip registration when the cap is exceeded, and replay the consumed prefix plus the remaining body.

	body, err := io.ReadAll(originalBody)

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

internal/handlers/nuget_feed.go:213

• A conditional service-index GET can return 304 Not Modified on the first request this handler sees (for example, when NuGet already has the index in its HTTP cache). HandleResponse then returns without a body to inspect, while NuGet proceeds using its local index, so none of those resource routes are registered and subsequent private resource requests lack authentication. Remove the conditional validators from marked discovery requests so the proxy always receives (or cache-hits) a complete representation.

		if isNugetServiceIndexRequest(req, source.serviceIndexURL) {
			matchedSource := source
			matchedSource.serviceIndexURL = req.URL.String()
			markNugetDiscovery(proxyCtx, matchedSource)
			return req, nil

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Balanced

[jeffwidman]

Pre-existing on main, out of scope for this PR. This is something @brettfo or @JamieMagee may wish to tackle though, up to them.

[brettfo]

This might need to change in the future. Let's leave it for now to see if it causes errors but I think I remember seeing some repos with internal http feeds. Since those feeds are internal to an organization dependabot doesn't have access to them anyway (they're values like http://package-feed.local) but I wanted to pin this just in case it crops up.