From 7723430ee3bc47dc42ad2b62448296e55d79e50f Mon Sep 17 00:00:00 2001 From: Mike Ma Date: Fri, 8 May 2026 07:51:01 -0500 Subject: [PATCH] Fix v3 gzip extra field to use a valid RFC 1952 subfield The chart v3 save path was writing raw marker bytes into the gzip `Extra` field, which produced malformed subfield metadata in readers that validate RFC 1952 framing. Wrap Helm's existing marker payload in a proper gzip subfield header (`SI1 SI2 LEN DATA`) and keep the payload unchanged. Reject oversized subfield payloads before encoding the two-byte gzip length. Add regression coverage for the extra field framing, oversized payload guard, and repeatable save hashes. Tested: go test ./internal/chart/v3/util -run TestSavedGzipExtraFieldIsValid -count=1 (Go 1.26 container) Tested: go test ./internal/chart/v3/util -run TestGzipExtraFieldWithSubfieldRejectsOversizedPayload -count=1 (Go 1.26 container) Tested: go test ./internal/chart/v3/util -run TestRepeatableSave -count=1 (Go 1.26 container) Tested: go test ./internal/chart/v3/util -run 'TestSavedGzipExtraFieldIsValid|TestGzipExtraFieldWithSubfieldRejectsOversizedPayload|TestSave|TestRepeatableSave' -count=1 (Go 1.26 container) Signed-off-by: Mike Ma --- internal/chart/v3/util/save.go | 24 ++++++++++++- internal/chart/v3/util/save_test.go | 56 +++++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/internal/chart/v3/util/save.go b/internal/chart/v3/util/save.go index f886c6175..edcf7128e 100644 --- a/internal/chart/v3/util/save.go +++ b/internal/chart/v3/util/save.go @@ -19,6 +19,7 @@ package util import ( "archive/tar" "compress/gzip" + "encoding/binary" "encoding/json" "errors" "fmt" @@ -34,6 +35,22 @@ import ( ) var headerBytes = []byte("+aHR0cHM6Ly95b3V0dS5iZS96OVV6MWljandyTQo=") +var gzipExtraSubfieldID = [2]byte{'r', 'r'} + +const maxGzipExtraSubfieldPayloadLen = 1<<16 - 1 + +func gzipExtraFieldWithSubfield(payload []byte) ([]byte, error) { + if len(payload) > maxGzipExtraSubfieldPayloadLen { + return nil, fmt.Errorf("gzip extra subfield payload too large: %d bytes", len(payload)) + } + + extra := make([]byte, 4+len(payload)) + extra[0] = gzipExtraSubfieldID[0] + extra[1] = gzipExtraSubfieldID[1] + binary.LittleEndian.PutUint16(extra[2:4], uint16(len(payload))) + copy(extra[4:], payload) + return extra, nil +} // SaveDir saves a chart as files in a directory. // @@ -125,6 +142,11 @@ func Save(c *chart.Chart, outDir string) (string, error) { return "", fmt.Errorf("is not a directory: %s", dir) } + gzipExtra, err := gzipExtraFieldWithSubfield(headerBytes) + if err != nil { + return "", err + } + f, err := os.Create(filename) if err != nil { return "", err @@ -132,7 +154,7 @@ func Save(c *chart.Chart, outDir string) (string, error) { // Wrap in gzip writer zipper := gzip.NewWriter(f) - zipper.Extra = headerBytes + zipper.Extra = gzipExtra zipper.Comment = "Helm" // Wrap in tar writer diff --git a/internal/chart/v3/util/save_test.go b/internal/chart/v3/util/save_test.go index 34e7d898e..197bdcad5 100644 --- a/internal/chart/v3/util/save_test.go +++ b/internal/chart/v3/util/save_test.go @@ -127,6 +127,58 @@ func TestSave(t *testing.T) { } } +func TestSavedGzipExtraFieldIsValid(t *testing.T) { + tmp := t.TempDir() + c := &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV3, + Name: "ahab", + Version: "1.2.3", + }, + } + + where, err := Save(c, tmp) + if err != nil { + t.Fatalf("Failed to save: %s", err) + } + + f, err := os.Open(where) + if err != nil { + t.Fatalf("Failed to open saved file: %s", err) + } + defer f.Close() + + r, err := gzip.NewReader(f) + if err != nil { + t.Fatalf("Failed to create gzip reader: %s", err) + } + defer r.Close() + + if len(r.Extra) < 4 { + t.Fatalf("gzip extra field too short to contain a valid subfield: %d byte(s)", len(r.Extra)) + } + + dataLen := int(r.Extra[2]) | int(r.Extra[3])<<8 + if len(r.Extra) != 4+dataLen { + t.Fatalf("gzip extra field has malformed subfield: LEN=%d but %d data byte(s) follow the subfield header", dataLen, len(r.Extra)-4) + } + + if r.Extra[0] != gzipExtraSubfieldID[0] || r.Extra[1] != gzipExtraSubfieldID[1] { + t.Fatalf("gzip extra field has unexpected subfield ID: %q%q", r.Extra[0], r.Extra[1]) + } + + if !bytes.Equal(r.Extra[4:], headerBytes) { + t.Fatalf("gzip extra field payload mismatch") + } +} + +func TestGzipExtraFieldWithSubfieldRejectsOversizedPayload(t *testing.T) { + _, err := gzipExtraFieldWithSubfield(make([]byte, maxGzipExtraSubfieldPayloadLen+1)) + if err == nil { + t.Fatal("expected oversized gzip extra subfield payload to fail") + } +} + // Creates a copy with a different schema; does not modify anything. func withSchema(chart chart.Chart, schema []byte) chart.Chart { chart.Schema = schema @@ -295,7 +347,7 @@ func TestRepeatableSave(t *testing.T) { Schema: []byte("{\n \"title\": \"Values\"\n}"), SchemaModTime: modTime, }, - want: "5bfea18cc3c8cbc265744bc32bffa9489a4dbe87d6b51b90f4255e4839d35e03", + want: "47f62cfbd69445112e92170ce8fc607c868e221045a1ce0696ac0df68841615f", }, { name: "Package 2 files", @@ -317,7 +369,7 @@ func TestRepeatableSave(t *testing.T) { Schema: []byte("{\n \"title\": \"Values\"\n}"), SchemaModTime: modTime, }, - want: "a240365c21e0a2f4a57873132a9b686566a612d08bcb3f20c9446bfff005ccce", + want: "58692b29601b75e1a0cc16d39d48ac2b71f45e05812f8d421a2cede39291fe93", }, } for _, test := range tests {