From f51216315faea5b1783d6cd493f8ac57734a2aae Mon Sep 17 00:00:00 2001 From: Karim Farid Date: Thu, 2 Apr 2026 23:44:43 +0200 Subject: [PATCH] fix(provenance): use flow style for arrays to avoid PGP dash-escaping Fixes #31866 This fixes an issue where array fields like keywords and sources were double-nested ('- - foo') when serializing Chart metadata into provenance files. By forcing flow-style YAML arrays for these embedded blocks we successfully bypass openpgp cleartext dash-escaping. Signed-off-by: Karim Farid --- pkg/action/package.go | 6 +-- pkg/provenance/sign.go | 33 ++++++++++++++ pkg/provenance/sign_test.go | 91 +++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 3 deletions(-) diff --git a/pkg/action/package.go b/pkg/action/package.go index 86426b412..4a503ae24 100644 --- a/pkg/action/package.go +++ b/pkg/action/package.go @@ -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) } diff --git a/pkg/provenance/sign.go b/pkg/provenance/sign.go index 45d4fe1a5..d3b6919fd 100644 --- a/pkg/provenance/sign.go +++ b/pkg/provenance/sign.go @@ -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 "go.yaml.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: diff --git a/pkg/provenance/sign_test.go b/pkg/provenance/sign_test.go index 8784e6c12..fc39b4d55 100644 --- a/pkg/provenance/sign_test.go +++ b/pkg/provenance/sign_test.go @@ -406,6 +406,97 @@ func TestVerify(t *testing.T) { } } +func TestMarshalMetadataFlowStyle(t *testing.T) { + meta := map[string]any{ + "name": "test-chart", + "version": "1.0.0", + "keywords": []string{"foo", "bar"}, + "sources": []string{"https://example.com"}, + } + + out, err := MarshalMetadata(meta) + if err != nil { + t.Fatal(err) + } + + result := string(out) + + // Flow style means arrays are inline + if !strings.Contains(result, "[foo, bar]") { + t.Errorf("expected flow-style keywords, got:\n%s", result) + } + + // No line should start with "-" which is what triggers PGP dash-escaping + for line := range strings.SplitSeq(result, "\n") { + if strings.HasPrefix(line, "-") { + t.Errorf("found line starting with '-' which will cause PGP dash-escaping: %q", line) + } + } +} + +func TestMarshalMetadataRoundTrip(t *testing.T) { + type Meta struct { + Name string `json:"name"` + Keywords []string `json:"keywords"` + Sources []string `json:"sources"` + } + + original := Meta{ + Name: "test-chart", + Keywords: []string{"foo", "bar"}, + Sources: []string{"https://example.com", "https://example.org"}, + } + + out, err := MarshalMetadata(original) + if err != nil { + t.Fatal(err) + } + + // Unmarshal metadataBytes back directly — not from disk + var restored Meta + if err := yaml.Unmarshal(out, &restored); err != nil { + t.Fatalf("failed to unmarshal metadataBytes back: %v", err) + } + + assert.Equal(t, original.Keywords, restored.Keywords, "keywords should survive round-trip") + assert.Equal(t, original.Sources, restored.Sources, "sources should survive round-trip") +} + +func TestClearSignNoDashEscaping(t *testing.T) { + signer, err := NewFromFiles(testKeyfile, testPubfile) + if err != nil { + t.Fatal(err) + } + + // Build metadata with keywords and sources — the fields that trigger the bug + meta := map[string]any{ + "name": "hashtest", + "version": "1.2.3", + "keywords": []string{"foo", "bar"}, + "sources": []string{"https://example.com"}, + } + + metadataBytes, err := MarshalMetadata(meta) + if err != nil { + t.Fatal(err) + } + + archiveData, err := os.ReadFile(testChartfile) + if err != nil { + t.Fatal(err) + } + + // Run through the FULL signing pipeline + sig, err := signer.ClearSign(archiveData, filepath.Base(testChartfile), metadataBytes) + if err != nil { + t.Fatal(err) + } + + if strings.Contains(sig, "- - ") { + t.Errorf("prov file contains PGP dash-escaped list items '- - ', meaning flow style fix isn't working:\n%s", sig) + } +} + // readSumFile reads a file containing a sum generated by the UNIX shasum tool. func readSumFile(sumfile string) (string, error) { data, err := os.ReadFile(sumfile)