Address NuGet discovery follow-up feedback

internal/handlers/nuget_feed.go

@@ -171,6 +171,13 @@ func addNugetDiscoveryJob(
 	sourceURLs map[string]struct{},
 	job nugetDiscoveryJob,
 ) {
+	serviceIndexURL, err := normalizeNugetServiceIndexURL(job.serviceIndexURL)
+	if err != nil {
+		logging.RequestLogf(nil, "skipping invalid NuGet service index URL %s: %v", job.serviceIndexURL, err)
+		return
+	}
+	job.serviceIndexURL = serviceIndexURL
+
 	key := nugetDiscoverySourceKey(job.serviceIndexURL)
 	if _, ok := sourceURLs[key]; ok {
 		logging.RequestLogf(nil, "skipping duplicate NuGet service index because it is already registered: %s", job.serviceIndexURL)
@@ -180,6 +187,20 @@ func addNugetDiscoveryJob(
 	*jobs = append(*jobs, job)
 }
 
+func normalizeNugetServiceIndexURL(rawURL string) (string, error) {
+	parsedURL, err := helpers.ParseURLLax(rawURL)
+	if err != nil {
+		return "", err
+	}
+	if parsedURL.Hostname() == "" {
+		return "", fmt.Errorf("missing host")
+	}
+	if parsedURL.Scheme == "" {
+		parsedURL.Scheme = "https"
+	}
+	return parsedURL.String(), nil
+}
+
 func discoverNugetFeedURLs(
 	jobs []nugetDiscoveryJob,
 	client *http.Client,
@@ -240,6 +261,9 @@ func discoverNugetFeedURLsForJob(
 	discoveryClient := *client
 	originalCheckRedirect := client.CheckRedirect
 	discoveryClient.CheckRedirect = func(redirectReq *http.Request, via []*http.Request) error {
+		if job.oidcCredential != nil && !strings.EqualFold(redirectReq.URL.Scheme, "https") {
+			return fmt.Errorf("refusing to redirect OIDC-authenticated NuGet discovery to non-HTTPS URL %s", redirectReq.URL)
+		}
 		if originalCheckRedirect != nil {
 			if err := originalCheckRedirect(redirectReq, via); err != nil {
 				return err
@@ -489,8 +513,9 @@ func authenticateNugetRequest(req *http.Request, cred nugetFeedCredentials, prox
 }
 
 func shouldTreatTokenAsPassword(url *url.URL) bool {
-	if url.Hostname() == "pkgs.dev.azure.com" {
+	hostname := strings.ToLower(url.Hostname())
+	if hostname == "pkgs.dev.azure.com" {
 		return true
 	}
-	return strings.HasSuffix(url.Hostname(), ".pkgs.visualstudio.com") && strings.Contains(url.Path, "/_packaging/")
+	return strings.HasSuffix(hostname, ".pkgs.visualstudio.com") && strings.Contains(url.Path, "/_packaging/")
 }

internal/handlers/nuget_feed_test.go

@@ -240,21 +240,24 @@ func TestNugetFeedHandler(t *testing.T) {
 }
 
 func TestShouldTreatTokenAsPassword(t *testing.T) {
-	// Test case 1: URL with hostname "pkgs.dev.azure.com"
-	url1, _ := url.Parse("https://pkgs.dev.azure.com/example")
-	assert.True(t, shouldTreatTokenAsPassword(url1))
-
-	// Test case 2: URL with visualsutudio hostname suffix
-	url2, _ := url.Parse("https://example.pkgs.visualstudio.com/_packaging/")
-	assert.True(t, shouldTreatTokenAsPassword(url2))
-
-	// Test case 3: Similar but not exactly the same as test case 2; should fail
-	url3, _ := url.Parse("sneaky.example.com/nuget.visualstudio.com/_packaging")
-	assert.False(t, shouldTreatTokenAsPassword(url3))
-
-	// Test case 3: URL with hostname not equal to "pkgs.dev.azure.com" and not matching the pattern
-	url4, _ := url.Parse("https://example.com")
-	assert.False(t, shouldTreatTokenAsPassword(url4))
+	for _, testCase := range []struct {
+		name     string
+		rawURL   string
+		expected bool
+	}{
+		{name: "Azure DevOps", rawURL: "https://pkgs.dev.azure.com/example", expected: true},
+		{name: "uppercase Azure DevOps", rawURL: "https://PKGS.DEV.AZURE.COM/example", expected: true},
+		{name: "Visual Studio", rawURL: "https://example.pkgs.visualstudio.com/_packaging/", expected: true},
+		{name: "uppercase Visual Studio", rawURL: "https://EXAMPLE.PKGS.VISUALSTUDIO.COM/_packaging/", expected: true},
+		{name: "similar Visual Studio path", rawURL: "sneaky.example.com/nuget.visualstudio.com/_packaging"},
+		{name: "unrelated host", rawURL: "https://example.com"},
+	} {
+		t.Run(testCase.name, func(t *testing.T) {
+			parsedURL, err := url.Parse(testCase.rawURL)
+			require.NoError(t, err)
+			assert.Equal(t, testCase.expected, shouldTreatTokenAsPassword(parsedURL))
+		})
+	}
 }
 
 func TestUrlsCanBeDeterminedFromNuGetFeeds(t *testing.T) {
@@ -394,6 +397,26 @@ func TestNewNugetFeedHandlerDiscoversResources(t *testing.T) {
 	assertHasTokenAuth(t, req, "Bearer", "some-token", "resource discovered during construction")
 }
 
+func TestNewNugetFeedHandlerNormalizesSchemeLessDiscoveryURL(t *testing.T) {
+	const resourceURL = "https://cdn.example.com/packages"
+	var discoveryURL string
+	client := &http.Client{
+		Timeout: 5 * time.Second,
+		Transport: nugetRoundTripperFunc(func(req *http.Request) (*http.Response, error) {
+			discoveryURL = req.URL.String()
+			return nugetDiscoveryResponse(resourceURL), nil
+		}),
+	}
+	handler := NewNugetFeedHandler(config.Credentials{
+		{"type": "nuget_feed", "url": "nuget.example.com/index.json", "token": "some-token"},
+	}, client)
+
+	assert.Equal(t, "https://nuget.example.com/index.json", discoveryURL)
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, resourceURL+"/example/index.json", nil)
+	req = handleRequestAndClose(handler, req, &goproxy.ProxyCtx{})
+	assertHasTokenAuth(t, req, "Bearer", "some-token", "resource discovered from scheme-less URL")
+}
+
 func TestNugetFeedHandlerProxyOnlyCredentials(t *testing.T) {
 	handler := NewNugetFeedHandler(config.Credentials{
 		{
@@ -779,6 +802,95 @@ func TestNugetFeedHandlerDiscoversThroughCrossOriginRedirectWithoutLeakingCreden
 	assertHasTokenAuth(t, req, "Bearer", "some-token", "resource discovered after cross-origin redirect")
 }
 
+func TestNugetFeedHandlerStripsAuthenticationAfterHTTPSDowngrade(t *testing.T) {
+	const resourceURL = "https://cdn.example.com/packages"
+	var firstRedirectAuth string
+	var finalRedirectAuth string
+	client := &http.Client{
+		Timeout: 5 * time.Second,
+		Transport: nugetRoundTripperFunc(func(req *http.Request) (*http.Response, error) {
+			switch {
+			case req.URL.Scheme == "https":
+				return nugetRedirectResponse("http://nuget.example.com/index.json"), nil
+			case req.URL.Path == "/index.json":
+				firstRedirectAuth = req.Header.Get("Authorization")
+				return nugetRedirectResponse("/v3/index.json"), nil
+			case req.URL.Path == "/v3/index.json":
+				finalRedirectAuth = req.Header.Get("Authorization")
+				return nugetDiscoveryResponse(resourceURL), nil
+			default:
+				return nil, fmt.Errorf("unexpected discovery URL %s", req.URL)
+			}
+		}),
+	}
+	handler := NewNugetFeedHandler(config.Credentials{
+		{"type": "nuget_feed", "url": "https://nuget.example.com/source/index.json", "token": "some-token"},
+	}, client)
+
+	assert.Empty(t, firstRedirectAuth)
+	assert.Empty(t, finalRedirectAuth)
+	for _, redirectURL := range []string{
+		"http://nuget.example.com/index.json",
+		"http://nuget.example.com/v3/index.json",
+	} {
+		req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, redirectURL, nil)
+		req = handleRequestAndClose(handler, req, &goproxy.ProxyCtx{})
+		assertUnauthenticated(t, req, "HTTP service-index redirect")
+	}
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, resourceURL+"/example/index.json", nil)
+	req = handleRequestAndClose(handler, req, &goproxy.ProxyCtx{})
+	assertHasTokenAuth(t, req, "Bearer", "some-token", "resource discovered after HTTPS downgrade")
+}
+
+func TestNugetFeedHandlerRejectsNonHTTPSOIDCRedirect(t *testing.T) {
+	const tokenURL = "https://actions.example.test/token" //nolint:gosec // test URL
+	t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", tokenURL)
+	t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "request-token")
+
+	var insecureDiscoveryRequested bool
+	client := &http.Client{
+		Timeout: 5 * time.Second,
+		Transport: nugetRoundTripperFunc(func(req *http.Request) (*http.Response, error) {
+			var body string
+			switch req.URL.Hostname() {
+			case "actions.example.test":
+				body = `{"count":1,"value":"github-token"}`
+			case "login.microsoftonline.com":
+				body = `{"access_token":"oidc-token","expires_in":3600,"token_type":"Bearer"}`
+			case "nuget.example.com":
+				if req.URL.Scheme == "http" {
+					insecureDiscoveryRequested = true
+					return nugetDiscoveryResponse("https://cdn.example.com/packages"), nil
+				}
+				return nugetRedirectResponse("http://nuget.example.com/insecure/index.json"), nil
+			default:
+				return nil, fmt.Errorf("unexpected request URL %s", req.URL)
+			}
+			return &http.Response{
+				StatusCode: http.StatusOK,
+				Body:       io.NopCloser(strings.NewReader(body)),
+				Header:     make(http.Header),
+				Request:    req,
+			}, nil
+		}),
+	}
+
+	handler := NewNugetFeedHandler(config.Credentials{
+		{
+			"type":      "nuget_feed",
+			"url":       "https://nuget.example.com/index.json",
+			"tenant-id": "tenant",
+			"client-id": "client",
+		},
+	}, client)
+
+	assert.False(t, insecureDiscoveryRequested)
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "http://nuget.example.com/insecure/index.json", nil)
+	req = handleRequestAndClose(handler, req, &goproxy.ProxyCtx{})
+	assertUnauthenticated(t, req, "rejected non-HTTPS OIDC redirect")
+}
+
 func TestNugetFeedHandlerAuthenticatesSameOriginServiceIndexRedirect(t *testing.T) {
 	testCases := []struct {
 		name          string