OCPBUGS-90080: Validate chart URL in /api/helm/verify to prevent SSRF

The `/api/helm/verify` endpoint accepted arbitrary chart_url values without validation, allowing authenticated users to make the backend issue requests to internal network addresses. I reused the existing IsValidChartURL check (already enforced on install and get-chart paths) to reject URLs that are not oci:// or http(s)://*.tgz before the backend attempts any connection.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: fsgreco

pkg/helm/actions/get_chart.go

@@ -67,7 +67,7 @@ func GetChart(url string, conf *action.Configuration, repositoryNamespace string
 // Secret in namespace with username and password keys when the registry requires authentication.
 func GetChartFromURL(url string, conf *action.Configuration, namespace string, client dynamic.Interface, coreClient corev1client.CoreV1Interface, filesCleanup bool, basicAuthSecretName string) (*chart.Chart, error) {
 
-	if !isValidChartURL(url) {
+	if !IsValidChartURL(url) {
 		return nil, fmt.Errorf("invalid chart URL: %s, must be oci:// URL or http(s)://*.tgz", url)
 	}
 	cmd := action.NewInstall(conf)

pkg/helm/actions/install_chart.go

@@ -58,9 +58,9 @@ type RegistryClientSetter interface {
 	SetRegistryClient(rc *registry.Client)
 }
 
-// isValidChartURL validates chart URLs using RFC-compliant hostname labels.
+// IsValidChartURL validates chart URLs using RFC-compliant hostname labels.
 // Accepts oci://<registry>/<path> and http(s)://<host>/<path>.tgz|tar.gz URLs.
-func isValidChartURL(raw string) bool {
+func IsValidChartURL(raw string) bool {
 	return ociURLRe.MatchString(raw) || httpURLRe.MatchString(raw)
 }
 
@@ -324,7 +324,7 @@ func addAuthSecretAnnotation(ch *chart.Chart, secretName string) {
 // basicAuthSecretName names a Secret in ns containing username and password keys for registry auth.
 func InstallChartFromURL(ns, name, url string, vals map[string]interface{}, conf *action.Configuration, coreClient corev1client.CoreV1Interface, version, basicAuthSecretName string) (*kv1.Secret, error) {
 
-	if !isValidChartURL(url) {
+	if !IsValidChartURL(url) {
 		return nil, fmt.Errorf("invalid chart URL: %s, must be oci:// URL or http(s)://*.tgz", url)
 	}
 

pkg/helm/actions/install_chart_test.go

@@ -663,9 +663,9 @@ func TestIsValidChartURL(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := isValidChartURL(tt.url)
+			got := IsValidChartURL(tt.url)
 			if got != tt.valid {
-				t.Errorf("isValidChartURL(%q) = %v, want %v", tt.url, got, tt.valid)
+				t.Errorf("IsValidChartURL(%q) = %v, want %v", tt.url, got, tt.valid)
 			}
 		})
 	}

pkg/helm/handlers/handlerChartVerifier.go

@@ -3,7 +3,9 @@ package handlers
 import (
 	"encoding/json"
 	"fmt"
+	"net"
 	"net/http"
+	"net/url"
 
 	"github.com/openshift/console/pkg/auth"
 	"github.com/openshift/console/pkg/helm/actions"
@@ -47,6 +49,28 @@ func (h *verifierHandlers) restConfig(bearerToken string) *rest.Config {
 		Transport:   h.Transport,
 	}
 }
+func isPrivateURL(raw string) bool {
+	u, err := url.Parse(raw)
+	if err != nil {
+		return true
+	}
+	host := u.Hostname()
+	ip := net.ParseIP(host)
+	if ip == nil {
+		ips, err := net.LookupIP(host)
+		if err != nil || len(ips) == 0 {
+			return true
+		}
+		for _, ip := range ips {
+			if ip.IsLoopback() || ip.IsPrivate() || ip.IsUnspecified() || ip.IsLinkLocalUnicast() {
+				return true
+			}
+		}
+		return false
+	}
+	return ip.IsLoopback() || ip.IsPrivate() || ip.IsUnspecified() || ip.IsLinkLocalUnicast()
+}
+
 func (h *verifierHandlers) HandleChartVerifier(user *auth.User, w http.ResponseWriter, r *http.Request) {
 	var req HelmVerifierRequest
 
@@ -55,6 +79,15 @@ func (h *verifierHandlers) HandleChartVerifier(user *auth.User, w http.ResponseW
 		serverutils.SendResponse(w, http.StatusBadRequest, serverutils.ApiError{Err: fmt.Sprintf("Failed to parse request: %v", err)})
 		return
 	}
+	if !actions.IsValidChartURL(req.ChartUrl) {
+		serverutils.SendResponse(w, http.StatusBadRequest, serverutils.ApiError{Err: "invalid chart URL: must be oci:// or http(s)://*.tgz"})
+		return
+	}
+	if isPrivateURL(req.ChartUrl) {
+		serverutils.SendResponse(w, http.StatusBadRequest, serverutils.ApiError{Err: "chart URL must not point to a private or loopback address"})
+		return
+	}
+
 	conf := h.getActionConfigurations(h.ApiServerHost, "default", user.Token, &h.Transport)
 	resp, err := h.chartVerifier(req.ChartUrl, req.Values, conf)
 	if err != nil {

pkg/helm/handlers/handler_chartVerifier_test.go

@@ -25,21 +25,26 @@ func fakeChartVerification(reportSummary string, err error) func(chartUrl string
 	}
 }
 func TestHelmHandlers_HandleChartVerifier(t *testing.T) {
+	validBody := `{"chart_url":"https://example.com/charts/mychart-1.0.0.tgz"}`
+
 	tests := []struct {
 		name             string
+		body             string
 		expectedResponse string
 		ReportSummary    string
 		error
 		httpStatusCode int
 	}{
 		{
 			name:             "Error occurred",
+			body:             validBody,
 			expectedResponse: `{"error":"Failed to verify chart: Chart path is invalid"}`,
 			error:            errors.New("Chart path is invalid"),
 			httpStatusCode:   http.StatusBadGateway,
 		},
 		{
 			name:             "Successful chart verification",
+			body:             validBody,
 			ReportSummary:    fakeReportSummary,
 			httpStatusCode:   http.StatusOK,
 			expectedResponse: ``,
@@ -50,7 +55,7 @@ func TestHelmHandlers_HandleChartVerifier(t *testing.T) {
 			handlers := fakeVerifierHandler()
 			handlers.chartVerifier = fakeChartVerification(tt.ReportSummary, tt.error)
 
-			request := httptest.NewRequest("", "/foo", strings.NewReader("{}"))
+			request := httptest.NewRequest("", "/foo", strings.NewReader(tt.body))
 			response := httptest.NewRecorder()
 
 			handlers.HandleChartVerifier(&auth.User{}, response, request)
@@ -66,3 +71,33 @@ func TestHelmHandlers_HandleChartVerifier(t *testing.T) {
 		})
 	}
 }
+
+func TestHelmHandlers_HandleChartVerifier_RejectsInvalidURLs(t *testing.T) {
+	tests := []struct {
+		name string
+		body string
+	}{
+		{"rejects non-tgz HTTP URL", `{"chart_url":"http://example.com/charts/mychart"}`},
+		{"rejects empty chart_url", `{"chart_url":""}`},
+		{"rejects ftp scheme", `{"chart_url":"ftp://example.com/chart.tgz"}`},
+		{"rejects file scheme", `{"chart_url":"file:///etc/passwd"}`},
+		{"rejects private IP with tgz", `{"chart_url":"http://172.28.1.76:8849/evil.tgz"}`},
+		{"rejects loopback", `{"chart_url":"http://127.0.0.1/chart.tgz"}`},
+		{"rejects localhost", `{"chart_url":"http://localhost/chart.tgz"}`},
+		{"rejects IPv6 loopback", `{"chart_url":"http://[::1]/chart.tgz"}`},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			handlers := fakeVerifierHandler()
+			handlers.chartVerifier = fakeChartVerification("", nil)
+
+			request := httptest.NewRequest("POST", "/api/helm/verify", strings.NewReader(tt.body))
+			response := httptest.NewRecorder()
+
+			handlers.HandleChartVerifier(&auth.User{}, response, request)
+			if response.Code != http.StatusBadRequest {
+				t.Errorf("expected status 400 but got %v", response.Code)
+			}
+		})
+	}
+}