fix: address issue #31866

pull/31916/head
gabrnavarro 2 weeks ago
parent e31a078e6e
commit 429dfdd10a

@ -38,7 +38,7 @@ require (
golang.org/x/crypto v0.48.0
golang.org/x/term v0.40.0
golang.org/x/text v0.34.0
gopkg.in/yaml.v3 v3.0.1 // indirect
gopkg.in/yaml.v3 v3.0.1
k8s.io/api v0.35.1
k8s.io/apiextensions-apiserver v0.35.1
k8s.io/apimachinery v0.35.1

@ -26,7 +26,6 @@ import (
"github.com/Masterminds/semver/v3"
"golang.org/x/term"
"sigs.k8s.io/yaml"
ci "helm.sh/helm/v4/pkg/chart"
"helm.sh/helm/v4/pkg/chart/loader"
@ -176,8 +175,9 @@ func (p *Package) Clearsign(filename string) error {
return errors.New("invalid chart apiVersion")
}
// Marshal chart metadata to YAML bytes
metadataBytes, err := yaml.Marshal(ch.Metadata)
// Marshal chart metadata to YAML bytes suitable for provenance files.
// Uses flow style for sequences to avoid PGP dash-escaping of YAML list items.
metadataBytes, err := provenance.MarshalMetadata(ch.Metadata)
if err != nil {
return fmt.Errorf("failed to marshal chart metadata: %w", err)
}

@ -28,6 +28,7 @@ import (
"github.com/ProtonMail/go-crypto/openpgp"
"github.com/ProtonMail/go-crypto/openpgp/clearsign"
"github.com/ProtonMail/go-crypto/openpgp/packet"
yaml3 "gopkg.in/yaml.v3"
"sigs.k8s.io/yaml"
)
@ -35,6 +36,38 @@ var defaultPGPConfig = packet.Config{
DefaultHash: crypto.SHA512,
}
// MarshalMetadata serializes metadata to YAML suitable for use in a provenance file.
//
// Unlike standard YAML marshaling, this function uses flow style (inline) for
// sequence (array) fields. This avoids PGP clear-text signature "dash escaping",
// where lines beginning with '-' are prefixed with '- ', which would cause YAML
// list items to be double-nested (e.g., "- - foo" instead of "- foo") in the
// provenance file.
func MarshalMetadata(meta any) ([]byte, error) {
// Marshal to block-style YAML first
yamlBytes, err := yaml.Marshal(meta)
if err != nil {
return nil, err
}
// Parse into a yaml.v3 Node tree so we can set flow style on sequences
var node yaml3.Node
if err := yaml3.Unmarshal(yamlBytes, &node); err != nil {
return nil, err
}
setFlowStyleForSequences(&node)
return yaml3.Marshal(&node)
}
// setFlowStyleForSequences recursively sets flow style on all YAML sequence nodes.
func setFlowStyleForSequences(node *yaml3.Node) {
if node.Kind == yaml3.SequenceNode {
node.Style = yaml3.FlowStyle
}
for _, n := range node.Content {
setFlowStyleForSequences(n)
}
}
// SumCollection represents a collection of file and image checksums.
//
// Files are of the form:

@ -28,7 +28,6 @@ import (
"github.com/ProtonMail/go-crypto/openpgp/packet"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sigs.k8s.io/yaml"
"helm.sh/helm/v4/pkg/chart/v2/loader"
)
@ -71,6 +70,10 @@ const (
// with shasum.
// shasum -a 256 hashtest-1.2.3.tgz > testdata/hashtest.sha256
testSumfile = "testdata/hashtest.sha256"
// testChartfileWithKeywords is a chart archive that includes keywords and sources
// fields to test that they are not double-nested in the provenance file.
testChartfileWithKeywords = "testdata/hashtest-keywords-1.2.3.tgz"
)
// testMessageBlock represents the expected message block for the testdata/hashtest chart.
@ -93,7 +96,7 @@ func loadChartMetadataForSigning(t *testing.T, chartPath string) []byte {
t.Fatal(err)
}
metadataBytes, err := yaml.Marshal(chart.Metadata)
metadataBytes, err := MarshalMetadata(chart.Metadata)
if err != nil {
t.Fatal(err)
}
@ -121,6 +124,51 @@ func TestMessageBlock(t *testing.T) {
}
}
// TestMessageBlockKeywordsAndSources verifies that keywords and sources
// are not double-nested in the provenance file metadata.
// Regression test for https://github.com/helm/helm/issues/31866
func TestMessageBlockKeywordsAndSources(t *testing.T) {
metadataBytes := loadChartMetadataForSigning(t, testChartfileWithKeywords)
// Verify the metadata YAML does not contain double-nested arrays
metadataStr := string(metadataBytes)
if strings.Contains(metadataStr, "- - ") {
t.Errorf("metadata contains double-nested array entries (- - ), indicating keywords/sources are incorrectly wrapped:\n%s", metadataStr)
}
// Verify the metadata contains correctly formatted keywords and sources using flow style.
// Flow style avoids PGP dash-escaping, which would otherwise double-nest list items.
assert.Contains(t, metadataStr, "keywords: [foo, bar]", "keywords should use flow style to avoid PGP dash-escaping")
assert.Contains(t, metadataStr, "sources:", "sources field should be present")
// Verify the metadata can be round-tripped: unmarshal back into Metadata and check fields
chart, err := loader.LoadFile(testChartfileWithKeywords)
if err != nil {
t.Fatal(err)
}
assert.Equal(t, []string{"foo", "bar"}, chart.Metadata.Keywords, "keywords should be a flat string slice")
assert.Equal(t, []string{"https://example.com", "https://example.org"}, chart.Metadata.Sources, "sources should be a flat string slice")
// Verify the full message block is well-formed
archiveData, err := os.ReadFile(testChartfileWithKeywords)
if err != nil {
t.Fatal(err)
}
out, err := messageBlock(archiveData, filepath.Base(testChartfileWithKeywords), metadataBytes)
if err != nil {
t.Fatal(err)
}
// Parse the message block back and verify checksums section is valid
sc, err := parseMessageBlock(out.Bytes())
if err != nil {
t.Fatal(err)
}
if _, ok := sc.Files["hashtest-keywords-1.2.3.tgz"]; !ok {
t.Error("chart file not found in message block checksums")
}
}
func TestParseMessageBlock(t *testing.T) {
sc, err := parseMessageBlock([]byte(testMessageBlock))
if err != nil {

@ -0,0 +1 @@
c6a54456551f83940a228d454d207f65b426dee2163d578410900fb9fcc46f7e hashtest-keywords-1.2.3.tgz

@ -0,0 +1,12 @@
apiVersion: v2
name: hashtest-keywords
description: Test chart with keywords and sources
keywords:
- foo
- bar
sources:
- https://example.com
- https://example.org
type: application
version: 1.2.3
appVersion: "1.0.0"
Loading…
Cancel
Save