From c5112b91df3e48120a369b8c89924421e13349b9 Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Wed, 3 Dec 2025 13:48:49 +0000 Subject: [PATCH 1/3] fix: strip digest algorithm prefix before hex decoding OCI references with tag+digest (e.g., chart:1.0@sha256:abc...) failed with "invalid byte" error because the sha256: prefix was passed to hex.DecodeString(). Signed-off-by: Evans Mungai --- pkg/downloader/chart_downloader.go | 15 ++++++++-- pkg/downloader/chart_downloader_test.go | 38 +++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index ee4f8abe3..95505a40b 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -125,7 +125,8 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven var digest32 [32]byte if hash != "" { // if there is a hash, populate the other formats - digest, err = hex.DecodeString(hash) + // Strip the algorithm prefix (e.g., "sha256:") if present + digest, err = hex.DecodeString(stripDigestAlgorithm(hash)) if err != nil { return "", nil, err } @@ -225,7 +226,8 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena c.Options = append(c.Options, getter.WithAcceptHeader("application/gzip,application/octet-stream")) // Check the cache for the file - digest, err := hex.DecodeString(digestString) + // Strip the algorithm prefix (e.g., "sha256:") if present + digest, err := hex.DecodeString(stripDigestAlgorithm(digestString)) if err != nil { return "", nil, fmt.Errorf("unable to decode digest: %w", err) } @@ -578,3 +580,12 @@ func loadRepoConfig(file string) (*repo.File, error) { } return r, nil } + +// stripDigestAlgorithm removes the algorithm prefix (e.g., "sha256:") from a digest string. +// If no prefix is present, the original string is returned unchanged. +func stripDigestAlgorithm(digest string) string { + if idx := strings.Index(digest, ":"); idx >= 0 { + return digest[idx+1:] + } + return digest +} diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index 4349ecef9..05cf64e12 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -485,3 +485,41 @@ func TestDownloadToCache(t *testing.T) { c.Keyring = "" }) } + +func TestStripDigestAlgorithm(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "sha256 prefixed digest", + input: "sha256:aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d", + expected: "aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d", + }, + { + name: "sha512 prefixed digest", + input: "sha512:abcdef1234567890", + expected: "abcdef1234567890", + }, + { + name: "plain hex digest without prefix", + input: "aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d", + expected: "aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d", + }, + { + name: "empty string", + input: "", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := stripDigestAlgorithm(tt.input) + if result != tt.expected { + t.Errorf("stripDigestAlgorithm(%q) = %q, want %q", tt.input, result, tt.expected) + } + }) + } +} From 9b34535a84a51e8b3660f613ace57d94538fe21c Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Wed, 3 Dec 2025 14:02:09 +0000 Subject: [PATCH 2/3] Add cmd test for OCI references with tag+digest Signed-off-by: Evans Mungai --- pkg/cmd/pull_test.go | 48 ++++++++++++++++++++++++++++++++++ pkg/repo/v1/repotest/server.go | 12 +++++---- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/pull_test.go b/pkg/cmd/pull_test.go index 96631fe05..36efe7b7f 100644 --- a/pkg/cmd/pull_test.go +++ b/pkg/cmd/pull_test.go @@ -506,3 +506,51 @@ func TestPullFileCompletion(t *testing.T) { checkFileCompletion(t, "pull", false) checkFileCompletion(t, "pull repo/chart", false) } + +// TestPullOCIWithTagAndDigest tests pulling an OCI chart with both tag and digest specified. +// This is a regression test for https://github.com/helm/helm/issues/31600 +func TestPullOCIWithTagAndDigest(t *testing.T) { + srv := repotest.NewTempServer( + t, + repotest.WithChartSourceGlob("testdata/testcharts/*.tgz*"), + ) + defer srv.Stop() + + ociSrv, err := repotest.NewOCIServer(t, srv.Root()) + if err != nil { + t.Fatal(err) + } + ociSrv.Run(t) + + contentCache := t.TempDir() + outdir := t.TempDir() + + // Test: pull with tag and digest (the fixed bug from issue #31600) + // Previously this failed with "encoding/hex: invalid byte: U+0073 's'" + ref := fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart:0.1.0@%s", + ociSrv.RegistryURL, ociSrv.ManifestDigest) + + cmd := fmt.Sprintf("pull %s -d '%s' --registry-config %s --content-cache %s --plain-http", + ref, + outdir, + filepath.Join(srv.Root(), "config.json"), + contentCache, + ) + + _, _, err = executeActionCommand(cmd) + if err != nil { + t.Fatalf("pull with tag+digest failed: %v", err) + } + + // Verify the file was downloaded + // When digest is present, the filename uses the digest format + expectedFile := filepath.Join(outdir, "oci-dependent-chart-0.1.0.tgz") + if _, err := os.Stat(expectedFile); err != nil { + // Try the digest-based filename + digestPart := ociSrv.ManifestDigest[7:] // strip "sha256:" + expectedFile = filepath.Join(outdir, fmt.Sprintf("oci-dependent-chart@sha256-%s.tgz", digestPart)) + if _, err := os.Stat(expectedFile); err != nil { + t.Errorf("expected chart file not found: %v", err) + } + } +} diff --git a/pkg/repo/v1/repotest/server.go b/pkg/repo/v1/repotest/server.go index 12b96de5a..2e27e1ed9 100644 --- a/pkg/repo/v1/repotest/server.go +++ b/pkg/repo/v1/repotest/server.go @@ -140,11 +140,12 @@ func newServer(t *testing.T, docroot string, options ...ServerOption) *Server { type OCIServer struct { *registry.Registry - RegistryURL string - Dir string - TestUsername string - TestPassword string - Client *ociRegistry.Client + RegistryURL string + Dir string + TestUsername string + TestPassword string + Client *ociRegistry.Client + ManifestDigest string // Digest of the pushed oci-dependent-chart manifest } type OCIServerRunConfig struct { @@ -282,6 +283,7 @@ func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) { result.Chart.Digest, result.Chart.Size) srv.Client = registryClient + srv.ManifestDigest = result.Manifest.Digest c := cfg.DependingChart if c == nil { return From 10c4c253a2d59d47d110f7c6920832295b0b41e9 Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Thu, 4 Dec 2025 16:48:36 +0000 Subject: [PATCH 3/3] Move oci registry push result to a struct Signed-off-by: Evans Mungai --- pkg/cmd/pull_test.go | 6 +++--- pkg/repo/v1/repotest/server.go | 26 +++++++++++++++++--------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/pull_test.go b/pkg/cmd/pull_test.go index 36efe7b7f..1c5af3146 100644 --- a/pkg/cmd/pull_test.go +++ b/pkg/cmd/pull_test.go @@ -520,7 +520,7 @@ func TestPullOCIWithTagAndDigest(t *testing.T) { if err != nil { t.Fatal(err) } - ociSrv.Run(t) + result := ociSrv.Run(t) contentCache := t.TempDir() outdir := t.TempDir() @@ -528,7 +528,7 @@ func TestPullOCIWithTagAndDigest(t *testing.T) { // Test: pull with tag and digest (the fixed bug from issue #31600) // Previously this failed with "encoding/hex: invalid byte: U+0073 's'" ref := fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart:0.1.0@%s", - ociSrv.RegistryURL, ociSrv.ManifestDigest) + ociSrv.RegistryURL, result.PushedChart.Manifest.Digest) cmd := fmt.Sprintf("pull %s -d '%s' --registry-config %s --content-cache %s --plain-http", ref, @@ -547,7 +547,7 @@ func TestPullOCIWithTagAndDigest(t *testing.T) { expectedFile := filepath.Join(outdir, "oci-dependent-chart-0.1.0.tgz") if _, err := os.Stat(expectedFile); err != nil { // Try the digest-based filename - digestPart := ociSrv.ManifestDigest[7:] // strip "sha256:" + digestPart := result.PushedChart.Manifest.Digest[7:] // strip "sha256:" expectedFile = filepath.Join(outdir, fmt.Sprintf("oci-dependent-chart@sha256-%s.tgz", digestPart)) if _, err := os.Stat(expectedFile); err != nil { t.Errorf("expected chart file not found: %v", err) diff --git a/pkg/repo/v1/repotest/server.go b/pkg/repo/v1/repotest/server.go index 2e27e1ed9..6380e1a86 100644 --- a/pkg/repo/v1/repotest/server.go +++ b/pkg/repo/v1/repotest/server.go @@ -140,12 +140,11 @@ func newServer(t *testing.T, docroot string, options ...ServerOption) *Server { type OCIServer struct { *registry.Registry - RegistryURL string - Dir string - TestUsername string - TestPassword string - Client *ociRegistry.Client - ManifestDigest string // Digest of the pushed oci-dependent-chart manifest + RegistryURL string + Dir string + TestUsername string + TestPassword string + Client *ociRegistry.Client } type OCIServerRunConfig struct { @@ -154,6 +153,10 @@ type OCIServerRunConfig struct { type OCIServerOpt func(config *OCIServerRunConfig) +type OCIServerRunResult struct { + PushedChart *ociRegistry.PushResult +} + func WithDependingChart(c *chart.Chart) OCIServerOpt { return func(config *OCIServerRunConfig) { config.DependingChart = c @@ -210,7 +213,7 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) { }, nil } -func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) { +func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) *OCIServerRunResult { t.Helper() cfg := &OCIServerRunConfig{} for _, fn := range opts { @@ -283,10 +286,11 @@ func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) { result.Chart.Digest, result.Chart.Size) srv.Client = registryClient - srv.ManifestDigest = result.Manifest.Digest c := cfg.DependingChart if c == nil { - return + return &OCIServerRunResult{ + PushedChart: result, + } } dependingRef := fmt.Sprintf("%s/u/ocitestuser/%s:%s", @@ -310,6 +314,10 @@ func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) { result.Manifest.Digest, result.Manifest.Size, result.Config.Digest, result.Config.Size, result.Chart.Digest, result.Chart.Size) + + return &OCIServerRunResult{ + PushedChart: result, + } } // Root gets the docroot for the server.