From 8f9b265f5a176c086d706696ea30b2bfefe60828 Mon Sep 17 00:00:00 2001
From: Matt Hook <hookenz@gmail.com>
Date: Thu, 22 Feb 2024 11:35:01 +1300
Subject: [PATCH] fix(helm) tighten up helm requests [EE-6722] (#11233)

---
 api/http/handler/helm/handler.go  | 15 ++++++++------
 pkg/libhelm/binary/search_repo.go | 34 ++++++++++++++++++++++++++++---
 pkg/libhelm/binary/show.go        |  2 +-
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/api/http/handler/helm/handler.go b/api/http/handler/helm/handler.go
index 584ebfa8a..0108f7ef1 100644
--- a/api/http/handler/helm/handler.go
+++ b/api/http/handler/helm/handler.go
@@ -38,19 +38,20 @@ func NewHandler(bouncer security.BouncerService, dataStore dataservices.DataStor
 		kubeClusterAccessService: kubeClusterAccessService,
 	}
 
-	h.Use(middlewares.WithEndpoint(dataStore.Endpoint(), "id"))
+	h.Use(middlewares.WithEndpoint(dataStore.Endpoint(), "id"),
+		bouncer.AuthenticatedAccess)
 
 	// `helm list -o json`
 	h.Handle("/{id}/kubernetes/helm",
-		bouncer.AuthenticatedAccess(httperror.LoggerHandler(h.helmList))).Methods(http.MethodGet)
+		httperror.LoggerHandler(h.helmList)).Methods(http.MethodGet)
 
 	// `helm delete RELEASE_NAME`
 	h.Handle("/{id}/kubernetes/helm/{release}",
-		bouncer.AuthenticatedAccess(httperror.LoggerHandler(h.helmDelete))).Methods(http.MethodDelete)
+		httperror.LoggerHandler(h.helmDelete)).Methods(http.MethodDelete)
 
 	// `helm install [NAME] [CHART] flags`
 	h.Handle("/{id}/kubernetes/helm",
-		bouncer.AuthenticatedAccess(httperror.LoggerHandler(h.helmInstall))).Methods(http.MethodPost)
+		httperror.LoggerHandler(h.helmInstall)).Methods(http.MethodPost)
 
 	// Deprecated
 	h.Handle("/{id}/kubernetes/helm/repositories",
@@ -69,12 +70,14 @@ func NewTemplateHandler(bouncer security.BouncerService, helmPackageManager libh
 		requestBouncer:     bouncer,
 	}
 
+	h.Use(bouncer.AuthenticatedAccess)
+
 	h.Handle("/templates/helm",
-		bouncer.AuthenticatedAccess(httperror.LoggerHandler(h.helmRepoSearch))).Methods(http.MethodGet)
+		httperror.LoggerHandler(h.helmRepoSearch)).Methods(http.MethodGet)
 
 	// helm show [COMMAND] [CHART] [REPO] flags
 	h.Handle("/templates/helm/{command:chart|values|readme}",
-		bouncer.AuthenticatedAccess(httperror.LoggerHandler(h.helmShow))).Methods(http.MethodGet)
+		httperror.LoggerHandler(h.helmShow)).Methods(http.MethodGet)
 
 	return h
 }
diff --git a/pkg/libhelm/binary/search_repo.go b/pkg/libhelm/binary/search_repo.go
index de9c04507..fc3d8aef8 100644
--- a/pkg/libhelm/binary/search_repo.go
+++ b/pkg/libhelm/binary/search_repo.go
@@ -18,6 +18,7 @@ import (
 )
 
 var errRequiredSearchOptions = errors.New("repo is required")
+var errInvalidRepoURL = errors.New("the request failed since either the Helm repository was not found or the index.yaml is not valid")
 
 type File struct {
 	APIVersion string             `yaml:"apiVersion" json:"apiVersion"`
@@ -64,6 +65,11 @@ func (hbpm *helmBinaryPackageManager) SearchRepo(searchRepoOpts options.SearchRe
 		}
 	}
 
+	// Allow redirect behavior to be overriden if specified.
+	if client.CheckRedirect == nil {
+		client.CheckRedirect = defaultCheckRedirect
+	}
+
 	url, err := url.ParseRequestURI(searchRepoOpts.Repo)
 	if err != nil {
 		return nil, errors.Wrap(err, fmt.Sprintf("invalid helm chart URL: %s", searchRepoOpts.Repo))
@@ -72,20 +78,42 @@ func (hbpm *helmBinaryPackageManager) SearchRepo(searchRepoOpts options.SearchRe
 	url.Path = path.Join(url.Path, "index.yaml")
 	resp, err := client.Get(url.String())
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to get index file")
+		return nil, errInvalidRepoURL
 	}
 	defer resp.Body.Close()
 
 	var file File
 	err = yaml.NewDecoder(resp.Body).Decode(&file)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to decode index file")
+		return nil, errInvalidRepoURL
+	}
+
+	// Validate index.yaml
+	if file.APIVersion == "" || file.Entries == nil {
+		return nil, errInvalidRepoURL
 	}
 
 	result, err := json.Marshal(file)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to marshal index file")
+		return nil, errInvalidRepoURL
 	}
 
 	return result, nil
 }
+
+// defaultCheckRedirect is a default CheckRedirect for helm
+// We don't allow redirects to URLs not ending in index.yaml
+// After that we follow the go http client behavior which is to stop
+// after a maximum of 10 redirects
+func defaultCheckRedirect(req *http.Request, via []*http.Request) error {
+	// The request url must end in index.yaml
+	if path.Base(req.URL.Path) != "index.yaml" {
+		return errors.New("the request URL must end in index.yaml")
+	}
+
+	// default behavior below
+	if len(via) >= 10 {
+		return errors.New("stopped after 10 redirects")
+	}
+	return nil
+}
diff --git a/pkg/libhelm/binary/show.go b/pkg/libhelm/binary/show.go
index 4fe49aeef..848e8ab26 100644
--- a/pkg/libhelm/binary/show.go
+++ b/pkg/libhelm/binary/show.go
@@ -22,7 +22,7 @@ func (hbpm *helmBinaryPackageManager) Show(showOpts options.ShowOptions) ([]byte
 
 	result, err := hbpm.run("show", args, showOpts.Env)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to run helm show on specified args")
+		return nil, errors.New("the request failed since either the Helm repository was not found or the chart does not exist")
 	}
 
 	return result, nil