Reject ambiguous NuGet credential routes

internal/handlers/nuget_feed.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"slices"
 	"strings"
 	"sync"
 
@@ -37,13 +38,15 @@ type nugetV3IndexResponse struct {
 
 // NugetFeedHandler handles requests to nuget feeds, adding auth.
 type NugetFeedHandler struct {
-	credentials         []nugetFeedCredentials
-	credentialURLs      map[string]struct{}
-	credentialsMutex    sync.RWMutex
-	discoverySources    []nugetDiscoveryAuth
-	discoverySourceURLs map[string]struct{}
-	discoveryMutex      sync.RWMutex
-	oidcRegistry        *oidc.OIDCRegistry
+	credentials           []nugetFeedCredentials
+	credentialURLs        map[string]struct{}
+	credentialsMutex      sync.RWMutex
+	credentialClaims      map[string]*nugetCredentialClaim
+	credentialClaimsMutex sync.RWMutex
+	discoverySources      []nugetDiscoveryAuth
+	discoverySourceURLs   map[string]struct{}
+	discoveryMutex        sync.RWMutex
+	oidcRegistry          *oidc.OIDCRegistry
 }
 
 type nugetFeedCredentials struct {
@@ -62,11 +65,19 @@ type nugetDiscoveryAuth struct {
 	authenticateURL bool
 }
 
+type nugetCredentialClaim struct {
+	url         string
+	auth        nugetDiscoveryAuth
+	sourceURLs  []string
+	conflicting bool
+}
+
 // NewNugetFeedHandler returns a new NugetFeedHandler.
 func NewNugetFeedHandler(creds config.Credentials) *NugetFeedHandler {
 	handler := NugetFeedHandler{
 		credentials:         []nugetFeedCredentials{},
 		credentialURLs:      make(map[string]struct{}),
+		credentialClaims:    make(map[string]*nugetCredentialClaim),
 		discoverySourceURLs: make(map[string]struct{}),
 		oidcRegistry:        oidc.NewOIDCRegistry(),
 	}
@@ -86,11 +97,13 @@ func NewNugetFeedHandler(creds config.Credentials) *NugetFeedHandler {
 		oidcCredential, _, ok := handler.oidcRegistry.Register(cred, []string{"url"}, "nuget feed")
 		if ok {
 			if url != "" {
-				handler.addDiscoverySource(nugetDiscoveryAuth{
+				source := nugetDiscoveryAuth{
 					serviceIndexURL: url,
 					oidc:            oidcCredential,
 					authenticateURL: true,
-				})
+				}
+				handler.addDiscoverySource(source)
+				handler.claimCredentialRoute(url, source, nil)
 			}
 			continue
 		}
@@ -108,12 +121,14 @@ func NewNugetFeedHandler(creds config.Credentials) *NugetFeedHandler {
 		}
 		handler.addStaticCredential(feedCred)
 		if url != "" && (token != "" || password != "") {
-			handler.addDiscoverySource(nugetDiscoveryAuth{
+			source := nugetDiscoveryAuth{
 				serviceIndexURL: url,
 				static:          feedCred,
 				hasStatic:       true,
 				authenticateURL: true,
-			})
+			}
+			handler.addDiscoverySource(source)
+			handler.claimCredentialRoute(url, source, nil)
 		}
 	}
 
@@ -221,6 +236,13 @@ func (h *NugetFeedHandler) HandleRequest(req *http.Request, proxyCtx *goproxy.Pr
 	if (req.URL.Scheme != "http" && req.URL.Scheme != "https") || !helpers.MethodPermitted(req, "GET", "HEAD") {
 		return req, nil
 	}
+	// A first claim may remain in the static or OIDC registry after a later
+	// claim makes the route ambiguous. Check conflicts before every credential
+	// lookup so no registered NuGet credential can be used for that route.
+	if h.requestMatchesConflictingRoute(req) {
+		logging.RequestLogf(proxyCtx, "* refusing to inject credentials for request matching an ambiguous NuGet route: %s", req.URL.String())
+		return req, nil
+	}
 
 	// Try OIDC credentials first (HTTPS only to avoid leaking tokens over plaintext)
 	if req.URL.Scheme == "https" {
@@ -294,6 +316,9 @@ func (h *NugetFeedHandler) HandleResponse(resp *http.Response, proxyCtx *goproxy
 	}
 
 	for _, discoveredURL := range extraUrlsFromSourceResponse(body, discoveryAuth.serviceIndexURL) {
+		if !h.claimCredentialRoute(discoveredURL, discoveryAuth, proxyCtx) {
+			continue
+		}
 		if discoveryAuth.oidc != nil {
 			h.oidcRegistry.RegisterURL(discoveredURL, discoveryAuth.oidc, "nuget resource")
 			continue
@@ -342,6 +367,10 @@ func (h *NugetFeedHandler) registerServiceIndexRedirect(resp *http.Response, sou
 	}
 
 	if redirectedSource.authenticateURL {
+		if !h.claimCredentialRoute(redirectURL.String(), redirectedSource, proxyCtx) {
+			logging.RequestLogf(proxyCtx, "  registered nuget service-index redirect: %s", redirectURL.String())
+			return
+		}
 		if source.oidc != nil {
 			h.oidcRegistry.RegisterURL(redirectURL.String(), source.oidc, "nuget service-index redirect")
 		}
@@ -384,6 +413,63 @@ func (h *NugetFeedHandler) addDiscoverySource(source nugetDiscoveryAuth) bool {
 	return true
 }
 
+func (h *NugetFeedHandler) claimCredentialRoute(rawURL string, auth nugetDiscoveryAuth, proxyCtx *goproxy.ProxyCtx) bool {
+	key := nugetCredentialURLKey(rawURL)
+	h.credentialClaimsMutex.Lock()
+	defer h.credentialClaimsMutex.Unlock()
+
+	claim, ok := h.credentialClaims[key]
+	if !ok {
+		h.credentialClaims[key] = &nugetCredentialClaim{
+			url:        rawURL,
+			auth:       auth,
+			sourceURLs: []string{auth.serviceIndexURL},
+		}
+		return true
+	}
+
+	if !slices.Contains(claim.sourceURLs, auth.serviceIndexURL) {
+		claim.sourceURLs = append(claim.sourceURLs, auth.serviceIndexURL)
+	}
+	if equivalentNugetDiscoveryAuth(claim.auth, auth) {
+		return false
+	}
+
+	claim.conflicting = true
+	logging.RequestLogf(proxyCtx,
+		"conflicting NuGet credentials for route %s claimed by service indexes %s; NuGet credentials will not be injected for matching requests",
+		claim.url, strings.Join(claim.sourceURLs, ", "))
+	return false
+}
+
+func (h *NugetFeedHandler) requestMatchesConflictingRoute(req *http.Request) bool {
+	h.credentialClaimsMutex.RLock()
+	defer h.credentialClaimsMutex.RUnlock()
+	for _, claim := range h.credentialClaims {
+		if claim.conflicting && helpers.UrlMatchesRequest(req, claim.url, true) {
+			return true
+		}
+	}
+	return false
+}
+
+func equivalentNugetDiscoveryAuth(first, second nugetDiscoveryAuth) bool {
+	if first.hasStatic || second.hasStatic {
+		return first.hasStatic && second.hasStatic && staticNugetCredentialValue(first.static) == staticNugetCredentialValue(second.static)
+	}
+	return first.oidc.Equivalent(second.oidc)
+}
+
+func staticNugetCredentialValue(credential nugetFeedCredentials) string {
+	if credential.token != "" {
+		return credential.token
+	}
+	if credential.password != "" {
+		return credential.username + ":" + credential.password
+	}
+	return ""
+}
+
 func markNugetDiscovery(proxyCtx *goproxy.ProxyCtx, auth nugetDiscoveryAuth) {
 	if proxyCtx != nil {
 		proxyctx.SetValue(proxyCtx, nugetDiscoveryCtxKey, auth)

internal/handlers/nuget_feed_test.go

@@ -492,6 +492,123 @@ func TestNugetFeedHandlerConcurrentDiscoveryIsDeduplicated(t *testing.T) {
 	assert.Len(t, handler.credentials, 2)
 }
 
+func TestNugetFeedHandlerAllowsSharedRouteWithSameCredential(t *testing.T) {
+	handler := NewNugetFeedHandler(config.Credentials{
+		config.Credential{"type": "nuget_feed", "url": "https://first.example/index.json", "token": "user:password"},
+		config.Credential{"type": "nuget_feed", "url": "https://second.example/index.json", "username": "user", "password": "password"},
+	})
+	response := `{"version":"3.0.0","resources":[{"@id":"https://cdn.example.com/packages","@type":"PackageBaseAddress/3.0.0"}]}`
+	discoverNugetFeed(t, handler, "https://first.example/index.json", http.StatusOK, response)
+	discoverNugetFeed(t, handler, "https://second.example/index.json", http.StatusOK, response)
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "https://cdn.example.com/packages/example/index.json", nil)
+	req = handleRequestAndClose(handler, req, &goproxy.ProxyCtx{})
+	assertHasBasicAuth(t, req, "user", "password", "shared route with same credential")
+}
+
+func TestNugetFeedHandlerRejectsSharedRouteWithDifferentCredentials(t *testing.T) {
+	for _, order := range [][]string{
+		{"https://first.example/index.json", "https://second.example/index.json"},
+		{"https://second.example/index.json", "https://first.example/index.json"},
+	} {
+		handler := NewNugetFeedHandler(config.Credentials{
+			config.Credential{"type": "nuget_feed", "url": "https://first.example/index.json", "token": "first-token"},
+			config.Credential{"type": "nuget_feed", "url": "https://second.example/index.json", "token": "second-token"},
+		})
+		response := `{"version":"3.0.0","resources":[{"@id":"https://cdn.example.com/packages","@type":"PackageBaseAddress/3.0.0"}]}`
+		var buf bytes.Buffer
+		log.SetOutput(&buf)
+		for _, sourceURL := range order {
+			discoverNugetFeed(t, handler, sourceURL, http.StatusOK, response)
+		}
+
+		req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "https://cdn.example.com/packages/example/index.json", nil)
+		req = handleRequestAndClose(handler, req, &goproxy.ProxyCtx{})
+		assertUnauthenticated(t, req, "shared route with different credentials")
+		assert.Contains(t, buf.String(), "conflicting NuGet credentials for route https://cdn.example.com/packages")
+		assert.Contains(t, buf.String(), "https://first.example/index.json")
+		assert.Contains(t, buf.String(), "https://second.example/index.json")
+	}
+}
+
+func TestNugetFeedHandlerRejectsOIDCAndMixedCredentialConflicts(t *testing.T) {
+	t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", "https://token.actions.example.com")
+	t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "test-token")
+	oidcCredential := config.Credential{
+		"type": "nuget_feed", "url": "https://first.example/index.json",
+		"tenant-id": "first-tenant", "client-id": "first-client",
+	}
+	testCases := []struct {
+		name        string
+		credential  config.Credential
+		conflicting bool
+	}{
+		{
+			name: "equivalent OIDC credential",
+			credential: config.Credential{
+				"type": "nuget_feed", "url": "https://second.example/index.json",
+				"tenant-id": "first-tenant", "client-id": "first-client",
+			},
+			conflicting: false,
+		},
+		{
+			name: "different OIDC credential",
+			credential: config.Credential{
+				"type": "nuget_feed", "url": "https://second.example/index.json",
+				"tenant-id": "second-tenant", "client-id": "second-client",
+			},
+			conflicting: true,
+		},
+		{
+			name:        "static credential",
+			credential:  config.Credential{"type": "nuget_feed", "url": "https://second.example/index.json", "token": "static-token"},
+			conflicting: true,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			handler := NewNugetFeedHandler(config.Credentials{oidcCredential, testCase.credential})
+			responseBody := `{"version":"3.0.0","resources":[{"@id":"https://cdn.example.com/packages","@type":"PackageBaseAddress/3.0.0"}]}`
+			for _, sourceURL := range []string{"https://first.example/index.json", "https://second.example/index.json"} {
+				proxyCtx := &goproxy.ProxyCtx{}
+				req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, sourceURL, nil)
+				handler.PrepareRequest(req, proxyCtx)
+				handler.HandleResponse(&http.Response{
+					StatusCode: http.StatusOK,
+					Body:       io.NopCloser(strings.NewReader(responseBody)),
+				}, proxyCtx)
+			}
+
+			req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "https://cdn.example.com/packages/example/index.json", nil)
+			assert.Equal(t, testCase.conflicting, handler.requestMatchesConflictingRoute(req))
+			if testCase.conflicting {
+				req = handleRequestAndClose(handler, req, &goproxy.ProxyCtx{})
+				assertUnauthenticated(t, req, "ambiguous OIDC route")
+			}
+		})
+	}
+}
+
+func TestNugetFeedHandlerKeepsDistinctMirrorRoutesIndependent(t *testing.T) {
+	handler := NewNugetFeedHandler(config.Credentials{
+		config.Credential{"type": "nuget_feed", "url": "https://first.example/index.json", "token": "first-token"},
+		config.Credential{"type": "nuget_feed", "url": "https://mirror.example/index.json", "token": "mirror-token"},
+	})
+	discoverNugetFeed(t, handler, "https://first.example/index.json", http.StatusOK,
+		`{"version":"3.0.0","resources":[{"@id":"https://first.example/packages","@type":"PackageBaseAddress/3.0.0"}]}`)
+	discoverNugetFeed(t, handler, "https://mirror.example/index.json", http.StatusOK,
+		`{"version":"3.0.0","resources":[{"@id":"https://mirror.example/packages","@type":"PackageBaseAddress/3.0.0"}]}`)
+
+	firstReq := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "https://first.example/packages/example/index.json", nil)
+	firstReq = handleRequestAndClose(handler, firstReq, &goproxy.ProxyCtx{})
+	assertHasTokenAuth(t, firstReq, "Bearer", "first-token", "first mirror route")
+
+	mirrorReq := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "https://mirror.example/packages/example/index.json", nil)
+	mirrorReq = handleRequestAndClose(handler, mirrorReq, &goproxy.ProxyCtx{})
+	assertHasTokenAuth(t, mirrorReq, "Bearer", "mirror-token", "second mirror route")
+}
+
 func TestNugetFeedHandlerPrefersMostSpecificURLCredential(t *testing.T) {
 	handler := NewNugetFeedHandler(config.Credentials{
 		config.Credential{

internal/oidc/oidc_credential.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/url"
+	"reflect"
 	"sync"
 	"time"
 
@@ -80,6 +81,12 @@ func (c *OIDCCredential) Provider() string {
 	return c.parameters.Name()
 }
 
+// Equivalent reports whether two credentials request tokens with the same
+// provider parameters. Cached token state does not affect equivalence.
+func (c *OIDCCredential) Equivalent(other *OIDCCredential) bool {
+	return c != nil && other != nil && reflect.DeepEqual(c.parameters, other.parameters)
+}
+
 func CreateOIDCCredential(cred config.Credential) (*OIDCCredential, error) {
 	if !IsOIDCConfigured() {
 		return nil, fmt.Errorf("OIDC is not configured")

internal/oidc/oidc_credential_test.go

@@ -18,6 +18,16 @@ import (
 	"github.com/dependabot/proxy/internal/config"
 )
 
+func TestOIDCCredentialEquivalent(t *testing.T) {
+	first := &OIDCCredential{parameters: &AzureOIDCParameters{TenantID: "tenant", ClientID: "client"}}
+	same := &OIDCCredential{parameters: &AzureOIDCParameters{TenantID: "tenant", ClientID: "client"}}
+	different := &OIDCCredential{parameters: &AzureOIDCParameters{TenantID: "tenant", ClientID: "other"}}
+
+	assert.True(t, first.Equivalent(same))
+	assert.False(t, first.Equivalent(different))
+	assert.False(t, first.Equivalent(nil))
+}
+
 func TestSuccessfulAuthenticationDoesNotMakeARepeatedRequest(t *testing.T) {
 	// these variables are necessary
 	os.Setenv(envActionsIDTokenRequestURL, "https://example.com/token")