From c8da0c2ebbfa6d61ef3a0cf66d73f2590acdee45 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Wed, 1 Oct 2025 17:23:47 +0200 Subject: [PATCH] No longer accept to download dependencies from unknown https repo BREAKING CHANGE: as a security measure charts that declare http(s) dependencies using full URLs (without adding the repo via `helm repo add`) will now fail during dependency update/build. Closes: https://github.com/helm/helm/issues/13461 Signed-off-by: Benoit Tigeot --- pkg/downloader/manager.go | 61 ++++++---------------------------- pkg/downloader/manager_test.go | 42 +++++++++++++++++++++++ 2 files changed, 52 insertions(+), 51 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index d41b8fdb4..981052e59 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -172,14 +172,9 @@ func (m *Manager) Update() error { return err } - // For the repositories Helm is not configured to know about, ensure Helm - // has some information about them and, when possible, the index files - // locally. - // TODO(mattfarina): Repositories should be explicitly added by end users - // rather than automatic. In Helm v4 require users to add repositories. They - // should have to add them in order to make sure they are aware of the - // repositories and opt-in to any locations, for security. - repoNames, err = m.ensureMissingRepos(repoNames, req) + // Ensure every dependency's repository is already configured in Helm + // (no automatic repo add in v4). + repoNames, err = m.validateConfiguredRepos(repoNames, req) if err != nil { return err } @@ -496,14 +491,9 @@ Loop: return nil } -// ensureMissingRepos attempts to ensure the repository information for repos -// not managed by Helm is present. This takes in the repoNames Helm is configured -// to work with along with the chart dependencies. It will find the deps not -// in a known repo and attempt to ensure the data is present for steps like -// version resolution. -func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart.Dependency) (map[string]string, error) { - - var ru []*repo.Entry +// validateConfiguredRepos checks that every dependency uses a repository +// already configured for security reasons. +func (m *Manager) validateConfiguredRepos(repoNames map[string]string, deps []*chart.Dependency) (map[string]string, error) { for _, dd := range deps { @@ -518,41 +508,10 @@ func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart. continue } - // The generated repository name, which will result in an index being - // locally cached, has a name pattern of "helm-manager-" followed by a - // sha256 of the repo name. This assumes end users will never create - // repositories with these names pointing to other repositories. Using - // this method of naming allows the existing repository pulling and - // resolution code to do most of the work. - rn, err := key(dd.Repository) - if err != nil { - return repoNames, err - } - rn = managerKeyPrefix + rn - - repoNames[dd.Name] = rn - - // Assuming the repository is generally available. For Helm managed - // access controls the repository needs to be added through the user - // managed system. This path will work for public charts, like those - // supplied by Bitnami, but not for protected charts, like corp ones - // behind a username and pass. - ri := &repo.Entry{ - Name: rn, - URL: dd.Repository, - } - ru = append(ru, ri) - } - - // Calls to UpdateRepositories (a public function) will only update - // repositories configured by the user. Here we update repos found in - // the dependencies that are not known to the user if update skipping - // is not configured. - if !m.SkipUpdate && len(ru) > 0 { - fmt.Fprintln(m.Out, "Getting updates for unmanaged Helm repositories...") - if err := m.parallelRepoUpdate(ru); err != nil { - return repoNames, err - } + return nil, fmt.Errorf( + "repository %q is not configured.\nAdd it and retry:\n helm repo add %s", + dd.Repository, dd.Repository, + ) } return repoNames, nil diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index 9e27f183f..b71dc93b7 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -22,6 +22,7 @@ import ( "os" "path/filepath" "reflect" + "strings" "testing" "time" @@ -767,3 +768,44 @@ func TestWriteLock(t *testing.T) { assert.Error(t, err) }) } + +func TestValidateConfiguredRepos_UnconfiguredRepo(t *testing.T) { + m := &Manager{} + repoNames := map[string]string{} + deps := []*chart.Dependency{ + { + Name: "redis", + Version: ">= 1.0.0", + Repository: "https://charts.example.com/", + }, + } + + _, err := m.validateConfiguredRepos(repoNames, deps) + if err == nil { + t.Fatalf("expected error for unconfigured repo, got nil") + } + // Error should include copy-pasteable guidance + if !strings.Contains(err.Error(), "helm repo add") { + t.Fatalf("expected actionable error with commands, got: %v", err) + } +} + +func TestValidateConfiguredRepos_ConfiguredRepo(t *testing.T) { + m := &Manager{} + repoNames := map[string]string{"redis": "example"} + deps := []*chart.Dependency{ + { + Name: "redis", + Version: ">= 1.0.0", + Repository: "https://charts.example.com/", + }, + } + + out, err := m.validateConfiguredRepos(repoNames, deps) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out == nil { + t.Fatalf("expected repoNames to be returned, got nil") + } +}