Skip to content

Commit ee01860

Browse files
banjohbanjoh
authored
fix: handle OCI digest algorithm prefix in chart downloader (#31601)
* 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 <mbuevans@gmail.com> * Add cmd test for OCI references with tag+digest Signed-off-by: Evans Mungai <mbuevans@gmail.com> * Move oci registry push result to a struct Signed-off-by: Evans Mungai <mbuevans@gmail.com> * Review comments from PR review Signed-off-by: Evans Mungai <mbuevans@gmail.com> * Fix failing test Signed-off-by: Evans Mungai <mbuevans@gmail.com> --------- Signed-off-by: Evans Mungai <mbuevans@gmail.com>
1 parent b26ec6b commit ee01860

5 files changed

Lines changed: 123 additions & 8 deletions

File tree

‎pkg/chart/common/util/jsonschema.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func ValidateAgainstSchema(ch chart.Charter, values map[string]interface{}) erro
8383
slog.Debug("chart name", "chart-name", chrt.Name())
8484
err := ValidateAgainstSingleSchema(values, chrt.Schema())
8585
if err != nil {
86-
sb.WriteString(fmt.Sprintf("%s:\n", chrt.Name()))
86+
fmt.Fprintf(&sb, "%s:\n", chrt.Name())
8787
sb.WriteString(err.Error())
8888
}
8989
}
@@ -103,10 +103,8 @@ func ValidateAgainstSchema(ch chart.Charter, values map[string]interface{}) erro
103103

104104
subchartValues, ok := raw.(map[string]any)
105105
if !ok {
106-
sb.WriteString(fmt.Sprintf(
107-
"%s:\ninvalid type for values: expected object (map), got %T\n",
108-
sub.Name(), raw,
109-
))
106+
fmt.Fprintf(&sb, "%s:\ninvalid type for values: expected object (map), got %T\n",
107+
sub.Name(), raw)
110108
continue
111109
}
112110

‎pkg/cmd/pull_test.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"net/http/httptest"
2323
"os"
2424
"path/filepath"
25+
"strings"
2526
"testing"
2627

2728
"helm.sh/helm/v4/pkg/repo/v1/repotest"
@@ -506,3 +507,54 @@ func TestPullFileCompletion(t *testing.T) {
506507
checkFileCompletion(t, "pull", false)
507508
checkFileCompletion(t, "pull repo/chart", false)
508509
}
510+
511+
// TestPullOCIWithTagAndDigest tests pulling an OCI chart with both tag and digest specified.
512+
// This is a regression test for https://github.com/helm/helm/issues/31600
513+
func TestPullOCIWithTagAndDigest(t *testing.T) {
514+
srv := repotest.NewTempServer(
515+
t,
516+
repotest.WithChartSourceGlob("testdata/testcharts/*.tgz*"),
517+
)
518+
defer srv.Stop()
519+
520+
ociSrv, err := repotest.NewOCIServer(t, srv.Root())
521+
if err != nil {
522+
t.Fatal(err)
523+
}
524+
result := ociSrv.RunWithReturn(t)
525+
526+
contentCache := t.TempDir()
527+
outdir := t.TempDir()
528+
529+
// Test: pull with tag and digest (the fixed bug from issue #31600)
530+
// Previously this failed with "encoding/hex: invalid byte: U+0073 's'"
531+
ref := fmt.Sprintf("oci://%s/u/ocitestuser/oci-dependent-chart:0.1.0@%s",
532+
ociSrv.RegistryURL, result.PushedChart.Manifest.Digest)
533+
534+
cmd := fmt.Sprintf("pull %s -d '%s' --registry-config %s --content-cache %s --plain-http",
535+
ref,
536+
outdir,
537+
filepath.Join(srv.Root(), "config.json"),
538+
contentCache,
539+
)
540+
541+
_, _, err = executeActionCommand(cmd)
542+
if err != nil {
543+
t.Fatalf("pull with tag+digest failed: %v", err)
544+
}
545+
546+
// Verify the file was downloaded
547+
// When digest is present, the filename uses the digest format (e.g. chart@sha256-hex.tgz)
548+
expectedFile := filepath.Join(outdir, "oci-dependent-chart-0.1.0.tgz")
549+
if _, err := os.Stat(expectedFile); err != nil {
550+
// Try the digest-based filename; parse algorithm:hex to avoid fixed-offset assumptions
551+
algorithm, digestPart, ok := strings.Cut(result.PushedChart.Manifest.Digest, ":")
552+
if !ok {
553+
t.Fatalf("digest must be in algorithm:hex format, got %q", result.PushedChart.Manifest.Digest)
554+
}
555+
expectedFile = filepath.Join(outdir, fmt.Sprintf("oci-dependent-chart@%s-%s.tgz", algorithm, digestPart))
556+
if _, err := os.Stat(expectedFile); err != nil {
557+
t.Errorf("expected chart file not found: %v", err)
558+
}
559+
}
560+
}

‎pkg/downloader/chart_downloader.go‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,15 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven
125125
var digest32 [32]byte
126126
if hash != "" {
127127
// if there is a hash, populate the other formats
128-
digest, err = hex.DecodeString(hash)
128+
// Strip the algorithm prefix (e.g., "sha256:") if present
129+
digest, err = hex.DecodeString(stripDigestAlgorithm(hash))
129130
if err != nil {
130131
return "", nil, err
131132
}
133+
if len(digest) != 32 {
134+
return "", nil, fmt.Errorf("invalid digest length: %d", len(digest))
135+
}
136+
132137
copy(digest32[:], digest)
133138
if pth, err := c.Cache.Get(digest32, CacheChart); err == nil {
134139
fdata, err := os.ReadFile(pth)
@@ -231,10 +236,14 @@ func (c *ChartDownloader) DownloadToCache(ref, version string) (string, *provena
231236
c.Options = append(c.Options, getter.WithAcceptHeader("application/gzip,application/octet-stream"))
232237

233238
// Check the cache for the file
234-
digest, err := hex.DecodeString(digestString)
239+
// Strip the algorithm prefix (e.g., "sha256:") if present
240+
digest, err := hex.DecodeString(stripDigestAlgorithm(digestString))
235241
if err != nil {
236242
return "", nil, fmt.Errorf("unable to decode digest: %w", err)
237243
}
244+
if digestString != "" && len(digest) != 32 {
245+
return "", nil, fmt.Errorf("invalid digest length: %d", len(digest))
246+
}
238247
var digest32 [32]byte
239248
copy(digest32[:], digest)
240249

@@ -584,3 +593,12 @@ func loadRepoConfig(file string) (*repo.File, error) {
584593
}
585594
return r, nil
586595
}
596+
597+
// stripDigestAlgorithm removes the algorithm prefix (e.g., "sha256:") from a digest string.
598+
// If no prefix is present, the original string is returned unchanged.
599+
func stripDigestAlgorithm(digest string) string {
600+
if idx := strings.Index(digest, ":"); idx >= 0 {
601+
return digest[idx+1:]
602+
}
603+
return digest
604+
}

‎pkg/downloader/chart_downloader_test.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"path/filepath"
2424
"testing"
2525

26+
"github.com/stretchr/testify/assert"
2627
"github.com/stretchr/testify/require"
2728

2829
"helm.sh/helm/v4/internal/test/ensure"
@@ -486,3 +487,34 @@ func TestDownloadToCache(t *testing.T) {
486487
c.Keyring = ""
487488
})
488489
}
490+
491+
func TestStripDigestAlgorithm(t *testing.T) {
492+
tests := map[string]struct {
493+
input string
494+
expected string
495+
}{
496+
"sha256 prefixed digest": {
497+
input: "sha256:aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d",
498+
expected: "aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d",
499+
},
500+
"sha512 prefixed digest": {
501+
input: "sha512:abcdef1234567890",
502+
expected: "abcdef1234567890",
503+
},
504+
"plain hex digest without prefix": {
505+
input: "aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d",
506+
expected: "aef46c66a7f2d5a12a7e3f54a64790daf5c9a9e66af3f46955efdaa6c900341d",
507+
},
508+
"empty string": {
509+
input: "",
510+
expected: "",
511+
},
512+
}
513+
514+
for name, tt := range tests {
515+
t.Run(name, func(t *testing.T) {
516+
result := stripDigestAlgorithm(tt.input)
517+
assert.Equalf(t, tt.expected, result, "stripDigestAlgorithm(%q) = %q, want %q", tt.input, result, tt.expected)
518+
})
519+
}
520+
}

‎pkg/repo/v1/repotest/server.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,10 @@ type OCIServerRunConfig struct {
153153

154154
type OCIServerOpt func(config *OCIServerRunConfig)
155155

156+
type OCIServerRunResult struct {
157+
PushedChart *ociRegistry.PushResult
158+
}
159+
156160
func WithDependingChart(c *chart.Chart) OCIServerOpt {
157161
return func(config *OCIServerRunConfig) {
158162
config.DependingChart = c
@@ -210,6 +214,11 @@ func NewOCIServer(t *testing.T, dir string) (*OCIServer, error) {
210214
}
211215

212216
func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) {
217+
t.Helper()
218+
_ = srv.RunWithReturn(t, opts...)
219+
}
220+
221+
func (srv *OCIServer) RunWithReturn(t *testing.T, opts ...OCIServerOpt) *OCIServerRunResult {
213222
t.Helper()
214223
cfg := &OCIServerRunConfig{}
215224
for _, fn := range opts {
@@ -284,7 +293,9 @@ func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) {
284293
srv.Client = registryClient
285294
c := cfg.DependingChart
286295
if c == nil {
287-
return
296+
return &OCIServerRunResult{
297+
PushedChart: result,
298+
}
288299
}
289300

290301
dependingRef := fmt.Sprintf("%s/u/ocitestuser/%s:%s",
@@ -308,6 +319,10 @@ func (srv *OCIServer) Run(t *testing.T, opts ...OCIServerOpt) {
308319
result.Manifest.Digest, result.Manifest.Size,
309320
result.Config.Digest, result.Config.Size,
310321
result.Chart.Digest, result.Chart.Size)
322+
323+
return &OCIServerRunResult{
324+
PushedChart: result,
325+
}
311326
}
312327

313328
// Root gets the docroot for the server.

0 commit comments

Comments
 (0)