From 1e570b0508efe546ab37ed21f28a6adb69687610 Mon Sep 17 00:00:00 2001 From: Blake Pettersson Date: Mon, 26 Jun 2023 16:23:50 +0200 Subject: [PATCH] fix: loosen source not permitted helm errors With #12255, we check if a source is first permitted before running `helm template`. This works a bit too well, since this may break previously working manifests. If an `AppProject` has a set of `sourceRepos` which are more restrictive than `*`, and it also has Helm public dependencies (repos with credentials would not work with 2.7x due to the fact they get filtered out before ending up on the repo server). Whereas before this would work, this currently fails on `HEAD` but not in `2.7x`. What we instead do here is that we only run this check if the chart failed to download - if it does then we run a check to see if the repo is in the allowed repos list. If the repo is not in the allowed repos list, we return the same error as in #12555, otherwise we bubble up the error. Should fix #13833. Signed-off-by: Blake Pettersson --- reposerver/repository/repository.go | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 25a5a0f937e3bf..ed041666cfe106 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -1048,9 +1048,9 @@ func runHelmBuild(appPath string, h helm.Helm) error { manifestGenerateLock.Lock(appPath) defer manifestGenerateLock.Unlock(appPath) - // the `helm dependency build` is potentially time consuming 1~2 seconds - // marker file is used to check if command already run to avoid running it again unnecessary - // file is removed when repository re-initialized (e.g. when another commit is processed) + // the `helm dependency build` is potentially a time-consuming 1~2 seconds, + // a marker file is used to check if command already run to avoid running it again unnecessarily + // the file is removed when repository is re-initialized (e.g. when another commit is processed) markerFile := path.Join(appPath, helmDepUpMarkerFile) _, err := os.Stat(markerFile) if err == nil { @@ -1066,6 +1066,11 @@ func runHelmBuild(appPath string, h helm.Helm) error { return os.WriteFile(markerFile, []byte("marker"), 0644) } +func isSourcePermitted(url string, repos []string) bool { + p := v1alpha1.AppProject{Spec: v1alpha1.AppProjectSpec{SourceRepos: repos}} + return p.IsSourcePermitted(v1alpha1.ApplicationSource{RepoURL: url}) +} + func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclient.ManifestRequest, isLocal bool, gitRepoPaths io.TempPaths) ([]*unstructured.Unstructured, error) { concurrencyAllowed := isConcurrencyAllowed(appPath) if !concurrencyAllowed { @@ -1186,6 +1191,20 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie } if err != nil { + var reposNotPermitted []string + // We do a sanity check here to give a nicer error message in case any of the Helm repositories are not permitted by + // the AppProject which the application is a part of + for _, repo := range helmRepos { + match := regexp.MustCompile(fmt.Sprintf("could not download (oci|https?)://%s", repo.Repo)) + if match.MatchString(err.Error()) && !isSourcePermitted(repo.Repo, q.ProjectSourceRepos) { + reposNotPermitted = append(reposNotPermitted, repo.Repo) + } + } + + if len(reposNotPermitted) > 0 { + return nil, status.Errorf(codes.PermissionDenied, "helm repos %s are not permitted in project '%s'", strings.Join(reposNotPermitted, ", "), q.ProjectName) + } + return nil, err }