[release-4.22] OCPBUGS-115003: Validate chart URL in /api/helm/verify to prevent SSRF

pkg/helm/actions/get_chart.go

@@ -65,7 +65,7 @@ func GetChart(url string, conf *action.Configuration, repositoryNamespace string
 
 func GetChartFromURL(url string, conf *action.Configuration, namespace string, client dynamic.Interface, coreClient corev1client.CoreV1Interface, filesCleanup bool) (*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

@@ -48,9 +48,9 @@ var (
 	httpURLRe = regexp.MustCompile(`(?i)^https?://` + hostPort + `/.+\.(?:tar\.gz|tgz)$`)
 )
 
-// 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)
 }
 
@@ -263,7 +263,7 @@ func InstallChartAsync(ns, name, url string, vals map[string]interface{}, conf *
 // If not provided, version is extracted from the OCI URL tag when applicable.
 func InstallChartFromURL(ns, name, url string, vals map[string]interface{}, conf *action.Configuration, coreClient corev1client.CoreV1Interface, version 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

@@ -530,9 +530,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

@@ -55,6 +55,10 @@ 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
+	}
 	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,30 @@ func TestHelmHandlers_HandleChartVerifier(t *testing.T) {
 		})
 	}
 }
+
+func TestHelmHandlers_HandleChartVerifier_RejectsInvalidURLs(t *testing.T) {
+	tests := []struct {
+		name string
+		body string
+	}{
+		{"rejects internal IP without tgz", `{"chart_url":"http://172.28.1.76:8849/nacos"}`},
+		{"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"}`},
+	}
+	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)
+			}
+		})
+	}
+}