diff --git a/pkg/helm/actions/get_chart.go b/pkg/helm/actions/get_chart.go index d82dab25411..30fc47cdf4d 100644 --- a/pkg/helm/actions/get_chart.go +++ b/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) diff --git a/pkg/helm/actions/install_chart.go b/pkg/helm/actions/install_chart.go index 9cb08a05339..bcb9db1f190 100644 --- a/pkg/helm/actions/install_chart.go +++ b/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:/// and http(s):///.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) } diff --git a/pkg/helm/actions/install_chart_test.go b/pkg/helm/actions/install_chart_test.go index 792111ca7be..5c068410b95 100644 --- a/pkg/helm/actions/install_chart_test.go +++ b/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) } }) } diff --git a/pkg/helm/handlers/handlerChartVerifier.go b/pkg/helm/handlers/handlerChartVerifier.go index 44b39293a84..743d3ec2bfa 100644 --- a/pkg/helm/handlers/handlerChartVerifier.go +++ b/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 { diff --git a/pkg/helm/handlers/handler_chartVerifier_test.go b/pkg/helm/handlers/handler_chartVerifier_test.go index 1ef663ab416..6ba92b6ba6e 100644 --- a/pkg/helm/handlers/handler_chartVerifier_test.go +++ b/pkg/helm/handlers/handler_chartVerifier_test.go @@ -25,8 +25,11 @@ 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 @@ -34,12 +37,14 @@ func TestHelmHandlers_HandleChartVerifier(t *testing.T) { }{ { 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) + } + }) + } +}