From fe4bfbc19f50df93748dd92c5e7f9b3932c19529 Mon Sep 17 00:00:00 2001 From: Brahm Lower Date: Wed, 26 Aug 2026 05:11:03 +0000 Subject: [PATCH 1/2] fix: rescue colon descriptions with a two-word phrase before the colon commentAsDescriptionNodes required 3+ words before a colon to treat a comment as a description rather than a key/value schema field, so a two-word phrase like "Contact us: support@example.com" fell through to being parsed as an unrecognized schema field and silently dropped. No real schema keyword contains a space, so any space in the parsed key is sufficient to know it's a description. Also adds a word-count boundary test, a test covering the same fix applied after a "---" doc separator, and a fuzz test over Parse to catch this class of "weird comment text" bug going forward. Co-Authored-By: Claude Sonnet 5 --- pkg/schema/comments/comment_test.go | 106 ++++++++++++++++++++++++++++ pkg/schema/comments/fuzz_test.go | 49 +++++++++++++ pkg/schema/comments/nodes.go | 10 +-- 3 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 pkg/schema/comments/fuzz_test.go diff --git a/pkg/schema/comments/comment_test.go b/pkg/schema/comments/comment_test.go index 6f77fc2..d6491aa 100644 --- a/pkg/schema/comments/comment_test.go +++ b/pkg/schema/comments/comment_test.go @@ -39,6 +39,11 @@ const CommentWithColonAndMultiWordKey = ` foo: bar ` +const CommentWithColonAndTwoWordKey = ` +# Contact us: support@example.com +foo: bar +` + const SetsSchemaDefault = ` # default: baz foo: bar @@ -116,6 +121,15 @@ func TestBasicCommentParsing(t *testing.T) { assert.Equal(t, "See docs for details: https://example.com", s.Description) }, }, + { + name: "comment with a colon after a two-word phrase is treated as description", + document: CommentWithColonAndTwoWordKey, + validate: func(t *testing.T, s *pkg.JsonSchema, err error) { + t.Helper() + require.NoError(t, err) + assert.Equal(t, "Contact us: support@example.com", s.Description) + }, + }, { name: "comment has no jsonschema properties", document: DoesntSetSchemaProperties, @@ -190,6 +204,98 @@ func TestParseOverridesExtraNodeWithCommentField(t *testing.T) { assert.Equal(t, "string", s.Type) } +// TestColonDescriptionWordCountBoundary sweeps the word count of the phrase +// before a colon to pin the exact boundary of the heuristic in +// commentAsDescriptionNodes: any phrase with 2+ words before the colon is +// rescued as a description, but a single word is indistinguishable from an +// unrecognized schema keyword (like "key: value" in DoesntSetSchemaProperties +// above) and is silently dropped. That single-word case is a known, +// intentional-but-surprising limitation, not something this test expects to +// change without a deliberate decision. +func TestColonDescriptionWordCountBoundary(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + phraseBeforeColon string + value string + expectDescription bool + }{ + { + name: "single word before colon is dropped (known limitation)", + phraseBeforeColon: "Note", + value: "this is important", + expectDescription: false, + }, + { + name: "two words before colon is a description", + phraseBeforeColon: "Contact us", + value: "support@example.com", + expectDescription: true, + }, + { + name: "three words before colon is a description", + phraseBeforeColon: "See docs for", + value: "https://example.com", + expectDescription: true, + }, + { + name: "five words before colon is a description", + phraseBeforeColon: "See docs for full details", + value: "https://example.com", + expectDescription: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + expected := tc.phraseBeforeColon + ": " + tc.value + document := "# " + expected + "\nfoo: bar\n" + + yamlNode := &yaml.Node{} + err := yaml.Unmarshal([]byte(document), yamlNode) + require.NoError(t, err) + + s, err := comments.Parse(getCommentNode(yamlNode), nil) + require.NoError(t, err) + + if tc.expectDescription { + assert.Equal(t, expected, s.Description) + } else { + assert.Empty(t, *s) + } + }) + } +} + +// TestColonDescriptionAfterDelimiter confirms that a colon-containing +// description is rescued the same way whether it appears before or after +// the "---" doc separator: parseNodeComment splits the head comment on +// "---" into independent segments before commentAsDescriptionNodes runs on +// each one, so the fix for the boundary above applies regardless of +// position. +func TestColonDescriptionAfterDelimiter(t *testing.T) { + t.Parallel() + + document := ` +# default: baz +# --- +# Contact us: support@example.com +foo: bar +` + + yamlNode := &yaml.Node{} + err := yaml.Unmarshal([]byte(document), yamlNode) + require.NoError(t, err) + + s, err := comments.Parse(getCommentNode(yamlNode), nil) + require.NoError(t, err) + assert.Equal(t, "baz", s.Default) + assert.Equal(t, "Contact us: support@example.com", s.Description) +} + func TestCommentFieldsSingleLine(t *testing.T) { t.Parallel() diff --git a/pkg/schema/comments/fuzz_test.go b/pkg/schema/comments/fuzz_test.go new file mode 100644 index 0000000..a76c111 --- /dev/null +++ b/pkg/schema/comments/fuzz_test.go @@ -0,0 +1,49 @@ +package comments_test + +import ( + "strings" + "testing" + + "helmvalues/pkg/schema/comments" + + "go.yaml.in/yaml/v4" +) + +// FuzzParseHeadComment guards against the class of bug this package has +// twice shipped: comment text that panics or otherwise crashes Parse +// instead of either succeeding or returning a proper error (a +// duplicate-mapping-key nil pointer panic, and a colon-in-description +// heuristic that silently dropped data). Fuzzing only proves crash-safety — +// it has no oracle for "this description should have survived", so it +// complements rather than replaces the hand-written boundary tests above. +func FuzzParseHeadComment(f *testing.F) { + seeds := []string{ + "comment is just a string", + "Contact us: support@example.com", + "See docs for details: https://example.com", + "Note: this is important", + "Time format: HH:MM:SS", + "default: baz", + "type: string", + "", + ":", + "::::", + "a: b\n---\nc: d", + } + for _, s := range seeds { + f.Add(s) + } + + f.Fuzz(func(t *testing.T, raw string) { + lines := strings.Split(raw, "\n") + for i, line := range lines { + lines[i] = "# " + line + } + + node := &yaml.Node{HeadComment: strings.Join(lines, "\n")} + + // Parse must never panic on arbitrary comment text, regardless of + // whether it returns a valid schema or an error. + _, _ = comments.Parse(node, nil) + }) +} diff --git a/pkg/schema/comments/nodes.go b/pkg/schema/comments/nodes.go index 2804ef0..44864a8 100644 --- a/pkg/schema/comments/nodes.go +++ b/pkg/schema/comments/nodes.go @@ -19,12 +19,14 @@ func commentAsDescriptionNodes(comment string) ([]*yaml.Node, bool) { return KeyValueNodes("description", strings.TrimSpace(comment)), true } - // If the doc is just a string but has a colon in it, which results - // in it being yaml parsed as a doc with a single key/value whose - // key likely has some spaces in it + // If the doc is just a string but has a colon in it, it gets yaml + // parsed as a doc with a single key/value pair instead of a scalar. + // Every real schema keyword (type, default, examples, ...) is a + // single word, so any space in the parsed key means this was never + // meant as a keyword and the whole comment is actually a description. if node.Content[0].Kind == yaml.MappingNode && len(node.Content[0].Content) == 2 && - strings.Count(node.Content[0].Content[0].Value, " ") > 1 { + strings.Contains(node.Content[0].Content[0].Value, " ") { return KeyValueNodes("description", strings.TrimSpace(comment)), true } From 6ebdacc707cf108fecd444d97569f6c6ba89e07b Mon Sep 17 00:00:00 2001 From: Brahm Lower Date: Wed, 26 Aug 2026 05:13:36 +0000 Subject: [PATCH 2/2] lint: fix unused-parameter violation in fuzz test Co-Authored-By: Claude Sonnet 5 --- pkg/schema/comments/fuzz_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/schema/comments/fuzz_test.go b/pkg/schema/comments/fuzz_test.go index a76c111..86c1d12 100644 --- a/pkg/schema/comments/fuzz_test.go +++ b/pkg/schema/comments/fuzz_test.go @@ -34,7 +34,7 @@ func FuzzParseHeadComment(f *testing.F) { f.Add(s) } - f.Fuzz(func(t *testing.T, raw string) { + f.Fuzz(func(_ *testing.T, raw string) { lines := strings.Split(raw, "\n") for i, line := range lines { lines[i] = "# " + line