fix(downloader): check --repo and dependency downloads against the index digest

helm pull --repo, chart lookups with --repo and dependency build/update
look the chart up in the index themselves and pass only its absolute URL
to the downloader, which resolves no digest for a bare URL, so the index
digest check never ran on those paths.

Add repo.FindChartInRepoURLWithDigest and a ChartDownloader.IndexDigest
field so those callers can hand over the digest of the entry they picked,
and have the dependency manager pass the digest of the entry it selected.

Signed-off-by: ashvinctrl <sharmaashvin27@gmail.com>

pkg/action/install.go

@@ -937,7 +937,7 @@ func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) (
 		dl.Verify = downloader.VerifyAlways
 	}
 	if c.RepoURL != "" {
-		chartURL, err := repo.FindChartInRepoURL(
+		chartURL, digest, err := repo.FindChartInRepoURLWithDigest(
 			c.RepoURL,
 			name,
 			getter.All(settings),
@@ -951,6 +951,7 @@ func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) (
 			return "", err
 		}
 		name = chartURL
+		dl.IndexDigest = digest
 
 		// Only pass the user/pass on when the user has said to or when the
 		// location of the chart repo and the chart are the same domain.

pkg/action/install_test.go

@@ -47,8 +47,10 @@ import (
 	ci "helm.sh/helm/v4/pkg/chart"
 
 	"helm.sh/helm/v4/internal/test"
+	"helm.sh/helm/v4/internal/test/ensure"
 	"helm.sh/helm/v4/pkg/chart/common"
 	chart "helm.sh/helm/v4/pkg/chart/v2"
+	"helm.sh/helm/v4/pkg/cli"
 	"helm.sh/helm/v4/pkg/kube"
 	kubefake "helm.sh/helm/v4/pkg/kube/fake"
 	"helm.sh/helm/v4/pkg/registry"
@@ -1253,3 +1255,19 @@ func TestInstallRelease_WaitOptionsPassedDownstream(t *testing.T) {
 	// Verify that WaitOptions were passed to GetWaiter
 	is.NotEmpty(failer.RecordedWaitOptions, "WaitOptions should be passed to GetWaiter")
 }
+
+func TestLocateChart_RepoURLRejectsChartNotMatchingIndexDigest(t *testing.T) {
+	ensure.HelmHome(t)
+	srv := tamperedRepoServer(t)
+	settings := cli.New()
+
+	c := &ChartPathOptions{RepoURL: srv.URL(), Version: "0.1.0"}
+	_, err := c.LocateChart("signtest", settings)
+	require.ErrorContains(t, err, "does not match the digest recorded for it in the repository index")
+
+	entries, err := os.ReadDir(settings.ContentCache)
+	if !errors.Is(err, fs.ErrNotExist) {
+		require.NoError(t, err)
+		assert.Empty(t, entries, "rejected chart must not be written to the content cache")
+	}
+}

pkg/action/pull.go

@@ -117,7 +117,7 @@ func (p *Pull) Run(chartRef string) (string, error) {
 
 	downloadSourceRef := chartRef
 	if p.RepoURL != "" {
-		chartURL, err := repo.FindChartInRepoURL(
+		chartURL, digest, err := repo.FindChartInRepoURLWithDigest(
 			p.RepoURL,
 			chartRef,
 			getter.All(p.Settings),
@@ -131,6 +131,7 @@ func (p *Pull) Run(chartRef string) (string, error) {
 			return out.String(), err
 		}
 		downloadSourceRef = chartURL
+		c.IndexDigest = digest
 	}
 
 	saved, v, err := c.DownloadTo(downloadSourceRef, p.Version, dest)

pkg/action/pull_test.go

@@ -17,16 +17,21 @@ limitations under the License.
 package action
 
 import (
+	"bytes"
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"helm.sh/helm/v4/internal/test/ensure"
 	"helm.sh/helm/v4/pkg/cli"
 	"helm.sh/helm/v4/pkg/registry"
+	"helm.sh/helm/v4/pkg/repo/v1/repotest"
 )
 
 func TestNewPull(t *testing.T) {
@@ -47,7 +52,16 @@ func TestPullSetRegistryClient(t *testing.T) {
 }
 
 func TestPullRun_ChartNotFound(t *testing.T) {
-	srv, err := startLocalServerForTests(t, nil)
+	fileBytes, err := os.ReadFile("../repo/v1/testdata/local-index.yaml")
+	require.NoError(t, err)
+	// The fixture's placeholder digest is not a valid sha256, and --repo pulls
+	// check the index digest, so give it a well-formed one to let the pull get
+	// as far as the missing archive.
+	fileBytes = bytes.ReplaceAll(fileBytes, []byte("sha256:1234567890abcdef"), []byte("sha256:"+strings.Repeat("0", 64)))
+	srv, err := startLocalServerForTests(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, err := w.Write(fileBytes)
+		assert.NoError(t, err)
+	}))
 	require.NoError(t, err)
 	defer srv.Close()
 
@@ -76,3 +90,36 @@ func startLocalServerForTests(t *testing.T, handler http.Handler) (*httptest.Ser
 
 	return httptest.NewServer(handler), nil
 }
+
+// tamperedRepoServer serves a chart repository whose index records the digest
+// of signtest-0.1.0.tgz while the archive it serves has changed since.
+func tamperedRepoServer(t *testing.T) *repotest.Server {
+	t.Helper()
+	srv := repotest.NewTempServer(t, repotest.WithChartSourceGlob("../downloader/testdata/signtest-0.1.0.tgz"))
+	t.Cleanup(srv.Stop)
+
+	served := filepath.Join(srv.Root(), "signtest-0.1.0.tgz")
+	original, err := os.ReadFile(served)
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(served, append(original, "appended by a rewritten mirror"...), 0o644))
+	return srv
+}
+
+func TestPullRun_RepoURLRejectsChartNotMatchingIndexDigest(t *testing.T) {
+	ensure.HelmHome(t)
+	srv := tamperedRepoServer(t)
+
+	config := actionConfigFixture(t)
+	client := NewPull(WithConfig(config))
+	client.Settings = cli.New()
+	client.RepoURL = srv.URL()
+	client.Version = "0.1.0"
+	client.DestDir = t.TempDir()
+
+	_, err := client.Run("signtest")
+	require.ErrorContains(t, err, "does not match the digest recorded for it in the repository index")
+
+	entries, err := os.ReadDir(client.DestDir)
+	require.NoError(t, err)
+	assert.Empty(t, entries, "rejected chart must not be written to the destination")
+}

pkg/downloader/chart_downloader.go

@@ -85,6 +85,13 @@ type ChartDownloader struct {
 
 	// Cache specifies the cache implementation to use.
 	Cache Cache
+
+	// IndexDigest is the digest a repository index records for the chart when
+	// the caller has already resolved the index entry to an absolute URL and
+	// passes that URL as the ref, as `--repo` and dependency downloads do. The
+	// downloaded archive is checked against it. It is ignored for any other
+	// kind of ref.
+	IndexDigest string
 }
 
 // DownloadTo retrieves a chart. Depending on the settings, it may also download a provenance file.
@@ -424,7 +431,7 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (string, *url
 			if errors.Is(err, ErrNoOwnerRepo) {
 				// Make sure to add the ref URL as the URL for the getter
 				c.Options = append(c.Options, getter.WithURL(ref))
-				return "", u, nil
+				return c.IndexDigest, u, nil
 			}
 			return "", u, err
 		}
@@ -445,7 +452,7 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (string, *url
 				getter.WithPassCredentialsAll(rc.PassCredentialsAll),
 			)
 		}
-		return "", u, nil
+		return c.IndexDigest, u, nil
 	}
 
 	// See if it's of the form: repo/path_to_chart
@@ -629,8 +636,9 @@ func loadRepoConfig(file string) (*repo.File, error) {
 // only as a cache key, so a repository that served an archive not matching its
 // own index was accepted without complaint.
 //
-// It is a no-op when the index carries no digest, which is the case for a chart
+// It is a no-op when there is no index digest to check against, which is the
-// referenced by a bare URL, so those keep working as before. OCI references are
+// case for a chart referenced by a bare URL with no IndexDigest set, so those
+// keep working as before. OCI references are
 // excluded on purpose: the digest resolved for them identifies a manifest
 // rather than the archive bytes, and the registry client already checks it.
 func verifyIndexDigest(ref string, u *url.URL, digestString string, want [sha256.Size]byte, data []byte) error {

pkg/downloader/chart_downloader_test.go

@@ -664,3 +664,44 @@ func TestIndexDigestVerificationScope(t *testing.T) {
 		assert.NoError(t, err)
 	})
 }
+
+func TestDownload_ResolvedURLCheckedAgainstIndexDigest(t *testing.T) {
+	// Callers such as `helm pull --repo` and dependency downloads look the
+	// chart up in the index themselves and pass on only its absolute URL, so
+	// the index digest has to come in through IndexDigest.
+	setup := func(t *testing.T) (*ChartDownloader, string) {
+		t.Helper()
+		srv, c, _ := tamperedChartServer(t, t.TempDir())
+		idx, err := repo.LoadIndexFile(filepath.Join(srv.Root(), "index.yaml"))
+		require.NoError(t, err)
+		cv, err := idx.Get("signtest", "0.1.0")
+		require.NoError(t, err)
+		require.NotEmpty(t, cv.Digest)
+		c.IndexDigest = cv.Digest
+		return c, srv.URL() + "/signtest-0.1.0.tgz"
+	}
+
+	t.Run("DownloadTo", func(t *testing.T) {
+		c, chartURL := setup(t)
+		dest := t.TempDir()
+		_, _, err := c.DownloadTo(chartURL, "", dest)
+		require.ErrorContains(t, err, "does not match the digest recorded for it in the repository index")
+
+		entries, err := os.ReadDir(dest)
+		require.NoError(t, err)
+		assert.Empty(t, entries, "rejected chart must not be written to the destination")
+	})
+
+	t.Run("DownloadToCache", func(t *testing.T) {
+		c, chartURL := setup(t)
+		_, _, err := c.DownloadToCache(chartURL, "")
+		require.ErrorContains(t, err, "does not match the digest recorded for it in the repository index")
+
+		digestBytes, err := hex.DecodeString(stripDigestAlgorithm(c.IndexDigest))
+		require.NoError(t, err)
+		var want [sha256.Size]byte
+		copy(want[:], digestBytes)
+		_, err = c.Cache.Get(want, CacheChart)
+		assert.Error(t, err, "rejected chart must not be written to the content cache")
+	})
+}

pkg/downloader/manager.go

@@ -318,7 +318,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 
 		// Any failure to resolve/download a chart should fail:
 		// https://github.com/helm/helm/issues/1439
-		churl, username, password, insecureSkipTLSVerify, passCredentialsAll, caFile, certFile, keyFile, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repos)
+		churl, digest, username, password, insecureSkipTLSVerify, passCredentialsAll, caFile, certFile, keyFile, err := m.findChartURL(dep.Name, dep.Version, dep.Repository, repos)
 		if err != nil {
 			saveError = fmt.Errorf("could not find %s: %w", churl, err)
 			break
@@ -340,6 +340,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error {
 			ContentCache:     m.ContentCache,
 			RegistryClient:   m.RegistryClient,
 			Getters:          m.Getters,
+			IndexDigest:      digest,
 			Options: []getter.Option{
 				getter.WithBasicAuth(username, password),
 				getter.WithPassCredentialsAll(passCredentialsAll),
@@ -722,9 +723,9 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error {
 // repoURL is the repository to search
 //
 // If it finds a URL that is "relative", it will prepend the repoURL.
-func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*repo.ChartRepository) (url, username, password string, insecureSkipTLSVerify, passCredentialsAll bool, caFile, certFile, keyFile string, err error) {
+func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*repo.ChartRepository) (url, digest, username, password string, insecureSkipTLSVerify, passCredentialsAll bool, caFile, certFile, keyFile string, err error) {
 	if registry.IsOCI(repoURL) {
-		return fmt.Sprintf("%s/%s:%s", repoURL, name, version), "", "", false, false, "", "", "", nil
+		return fmt.Sprintf("%s/%s:%s", repoURL, name, version), "", "", "", false, false, "", "", "", nil
 	}
 
 	for _, cr := range repos {
@@ -735,17 +736,18 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
 		entry, err = findEntryByName(name, cr)
 		if err != nil {
 			// TODO: Consider refactoring this function to reduce the number of returned values while preserving behavior.
-			return url, username, password, insecureSkipTLSVerify, passCredentialsAll, caFile, certFile, keyFile, err
+			return url, digest, username, password, insecureSkipTLSVerify, passCredentialsAll, caFile, certFile, keyFile, err
 		}
 		var ve *repo.ChartVersion
 		ve, err = findVersionedEntry(version, entry)
 		if err != nil {
-			return url, username, password, insecureSkipTLSVerify, passCredentialsAll, caFile, certFile, keyFile, err
+			return url, digest, username, password, insecureSkipTLSVerify, passCredentialsAll, caFile, certFile, keyFile, err
 		}
 		url, err = repo.ResolveReferenceURL(repoURL, ve.URLs[0])
 		if err != nil {
-			return url, username, password, insecureSkipTLSVerify, passCredentialsAll, caFile, certFile, keyFile, err
+			return url, digest, username, password, insecureSkipTLSVerify, passCredentialsAll, caFile, certFile, keyFile, err
 		}
+		digest = ve.Digest
 		username = cr.Config.Username
 		password = cr.Config.Password
 		passCredentialsAll = cr.Config.PassCredentialsAll
@@ -754,14 +756,14 @@ func (m *Manager) findChartURL(name, version, repoURL string, repos map[string]*
 		certFile = cr.Config.CertFile
 		keyFile = cr.Config.KeyFile
 
-		return url, username, password, insecureSkipTLSVerify, passCredentialsAll, caFile, certFile, keyFile, err
+		return url, digest, username, password, insecureSkipTLSVerify, passCredentialsAll, caFile, certFile, keyFile, err
 	}
-	url, err = repo.FindChartInRepoURL(repoURL, name, m.Getters, repo.WithChartVersion(version), repo.WithClientTLS(certFile, keyFile, caFile))
+	url, digest, err = repo.FindChartInRepoURLWithDigest(repoURL, name, m.Getters, repo.WithChartVersion(version), repo.WithClientTLS(certFile, keyFile, caFile))
 	if err == nil {
-		return url, username, password, false, false, "", "", "", nil
+		return url, digest, username, password, false, false, "", "", "", nil
 	}
 	err = fmt.Errorf("chart %s not found in %s: %w", name, repoURL, err)
-	return url, username, password, false, false, "", "", "", err
+	return url, digest, username, password, false, false, "", "", "", err
 }
 
 // findEntryByName finds an entry in the chart repository whose name matches the given name.

pkg/downloader/manager_test.go

@@ -27,9 +27,11 @@ import (
 	"github.com/stretchr/testify/require"
 	"sigs.k8s.io/yaml"
 
+	"helm.sh/helm/v4/internal/test/ensure"
 	chart "helm.sh/helm/v4/pkg/chart/v2"
 	"helm.sh/helm/v4/pkg/chart/v2/loader"
 	chartutil "helm.sh/helm/v4/pkg/chart/v2/util"
+	"helm.sh/helm/v4/pkg/cli"
 	"helm.sh/helm/v4/pkg/getter"
 	"helm.sh/helm/v4/pkg/repo/v1"
 	"helm.sh/helm/v4/pkg/repo/v1/repotest"
@@ -66,7 +68,7 @@ func TestFindChartURL(t *testing.T) {
 	version := "0.1.0"
 	repoURL := "http://example.com/charts"
 
-	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(name, version, repoURL, repos)
+	churl, _, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(name, version, repoURL, repos)
 	require.NoError(t, err)
 
 	assert.Equal(t, "https://charts.helm.sh/stable/alpine-0.1.0.tgz", churl, "Unexpected URL %q", churl)
@@ -79,7 +81,7 @@ func TestFindChartURL(t *testing.T) {
 	version = "1.2.3"
 	repoURL = "https://example-https-insecureskiptlsverify.com"
 
-	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos)
+	churl, _, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos)
 	require.NoError(t, err)
 
 	assert.True(t, insecureSkipTLSVerify, "Unexpected insecureSkipTLSVerify %t", insecureSkipTLSVerify)
@@ -92,7 +94,7 @@ func TestFindChartURL(t *testing.T) {
 	version = "1.2.3"
 	repoURL = "http://example.com/helm"
 
-	churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos)
+	churl, _, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos)
 	require.NoError(t, err)
 
 	assert.Equal(t, "http://example.com/helm/charts/foo-1.2.3.tgz", churl, "Unexpected URL %q", churl)
@@ -655,3 +657,45 @@ func TestWriteLock(t *testing.T) {
 		assert.Error(t, writeLock(filePath, lock, false))
 	})
 }
+
+func TestDownloadAll_RejectsDependencyNotMatchingIndexDigest(t *testing.T) {
+	tests := []struct {
+		name       string
+		configured bool
+	}{
+		{name: "repository in repositories.yaml", configured: true},
+		{name: "repository not in repositories.yaml", configured: false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ensure.HelmHome(t)
+			contentCache := t.TempDir()
+			srv, _, _ := tamperedChartServer(t, contentCache)
+
+			repoConfig := filepath.Join(srv.Root(), "repositories.yaml")
+			if !tt.configured {
+				repoConfig = filepath.Join(t.TempDir(), "repositories.yaml")
+			}
+			chartPath := t.TempDir()
+			m := &Manager{
+				Out:              new(bytes.Buffer),
+				ChartPath:        chartPath,
+				RepositoryConfig: repoConfig,
+				RepositoryCache:  srv.Root(),
+				ContentCache:     contentCache,
+				Getters:          getter.All(&cli.EnvSettings{}),
+			}
+			dep := &chart.Dependency{
+				Name:       "signtest",
+				Repository: srv.URL(),
+				Version:    "0.1.0",
+			}
+
+			err := m.downloadAll([]*chart.Dependency{dep})
+			require.ErrorContains(t, err, "does not match the digest recorded for it in the repository index")
+
+			_, err = os.Stat(filepath.Join(chartPath, "charts", "signtest-0.1.0.tgz"))
+			assert.ErrorIs(t, err, fs.ErrNotExist, "rejected dependency must not be saved to charts/")
+		})
+	}
+}

pkg/repo/v1/chartrepo.go

@@ -174,6 +174,15 @@ func WithInsecureSkipTLSVerify(insecureSkipTLSVerify bool) FindChartInRepoURLOpt
 // FindChartInRepoURL finds chart in chart repository pointed by repoURL
 // without adding repo to repositories
 func FindChartInRepoURL(repoURL, chartName string, getters getter.Providers, options ...FindChartInRepoURLOption) (string, error) {
+	chartURL, _, err := FindChartInRepoURLWithDigest(repoURL, chartName, getters, options...)
+	return chartURL, err
+}
+
+// FindChartInRepoURLWithDigest is FindChartInRepoURL that also returns the
+// digest the repository index records for the chart archive, so the caller can
+// check the archive it downloads from the returned URL against it. The digest
+// is empty when the index entry does not carry one.
+func FindChartInRepoURLWithDigest(repoURL, chartName string, getters getter.Providers, options ...FindChartInRepoURLOption) (string, string, error) {
 	opts := findChartInRepoURLOptions{}
 	for _, option := range options {
 		option(&opts)
@@ -197,11 +206,11 @@ func FindChartInRepoURL(repoURL, chartName string, getters getter.Providers, opt
 	}
 	r, err := NewChartRepository(&c, getters)
 	if err != nil {
-		return "", err
+		return "", "", err
 	}
 	idx, err := r.DownloadIndexFile()
 	if err != nil {
-		return "", fmt.Errorf("looks like %q is not a valid chart repository or cannot be reached: %w", repoURL, err)
+		return "", "", fmt.Errorf("looks like %q is not a valid chart repository or cannot be reached: %w", repoURL, err)
 	}
 	defer func() {
 		os.RemoveAll(filepath.Join(r.CachePath, helmpath.CacheChartsFile(r.Config.Name)))
@@ -211,7 +220,7 @@ func FindChartInRepoURL(repoURL, chartName string, getters getter.Providers, opt
 	// Read the index file for the repository to get chart information and return chart URL
 	repoIndex, err := LoadIndexFile(idx)
 	if err != nil {
-		return "", err
+		return "", "", err
 	}
 
 	errMsg := fmt.Sprintf("chart %q", chartName)
@@ -220,24 +229,24 @@ func FindChartInRepoURL(repoURL, chartName string, getters getter.Providers, opt
 	}
 	cv, err := repoIndex.Get(chartName, opts.ChartVersion)
 	if err != nil {
-		return "", ChartNotFoundError{
+		return "", "", ChartNotFoundError{
 			Chart:   errMsg,
 			RepoURL: repoURL,
 		}
 	}
 
 	if len(cv.URLs) == 0 {
-		return "", fmt.Errorf("%s has no downloadable URLs", errMsg)
+		return "", "", fmt.Errorf("%s has no downloadable URLs", errMsg)
 	}
 
 	chartURL := cv.URLs[0]
 
 	absoluteChartURL, err := ResolveReferenceURL(repoURL, chartURL)
 	if err != nil {
-		return "", fmt.Errorf("failed to make chart URL absolute: %w", err)
+		return "", "", fmt.Errorf("failed to make chart URL absolute: %w", err)
 	}
 
-	return absoluteChartURL, nil
+	return absoluteChartURL, cv.Digest, nil
 }
 
 // ResolveReferenceURL resolves refURL relative to baseURL.

pkg/repo/v1/chartrepo_test.go

@@ -196,6 +196,17 @@ func TestFindChartInRepoURL(t *testing.T) {
 	assert.Equalf(t, "https://charts.helm.sh/stable/nginx-0.1.0.tgz", chartURL, "%s is not the valid URL", chartURL)
 }
 
+func TestFindChartInRepoURLWithDigest(t *testing.T) {
+	srv, err := startLocalServerForTests(nil)
+	require.NoError(t, err)
+	defer srv.Close()
+
+	chartURL, digest, err := FindChartInRepoURLWithDigest(srv.URL, "nginx", getter.All(&cli.EnvSettings{}), WithChartVersion("0.2.0"))
+	require.NoError(t, err)
+	assert.Equal(t, "https://charts.helm.sh/stable/nginx-0.2.0.tgz", chartURL)
+	assert.Equal(t, "sha256:1234567890abcdef", digest)
+}
+
 func TestErrorFindChartInRepoURL(t *testing.T) {
 	g := getter.All(&cli.EnvSettings{
 		RepositoryCache: t.TempDir(),