Merge PR #225: Centralize YAML handling with safe unmarshal wrappers

Author: merge-queue-bot

PLAN.md

@@ -33,7 +33,7 @@ footer: |
 | 114 | ✅     | sonnet | [MDS034 message clarity and flavor-vs-rule docs](plan/114_mds034-message-and-flavor-vs-rule-docs.md) |
 | 120 | 🔲     | sonnet | [Unify glob matcher and field naming across mdsmith](plan/120_glob-unification.md)                   |
 | 121 | 🔲     | opus   | [Expose mdsmith to VS Code via Language Server Protocol](plan/121_vscode-integration.md)             |
-| 121 | 🔲     | sonnet | [Review and centralize YAML handling](plan/121_yaml-handling-review.md)                              |
+| 121 | ✅     | sonnet | [Review and centralize YAML handling](plan/121_yaml-handling-review.md)                              |
 | 122 | 🔲     | sonnet | [VS Code hover help and palette commands](plan/122_vscode-hover-and-palette.md)                      |
 | 124 | 🔲     | sonnet | [No space inside code spans rule](plan/124_no-space-in-code-spans.md)                                |
 | 125 | 🔲     | sonnet | [No space inside link text rule](plan/125_no-space-in-link-text.md)                                  |

cmd/mdsmith/main.go

@@ -9,7 +9,6 @@ import (
 	"runtime/debug"
 
 	flag "github.com/spf13/pflag"
-	"gopkg.in/yaml.v3"
 
 	"github.com/jeduden/mdsmith/internal/concepts"
 	"github.com/jeduden/mdsmith/internal/config"
@@ -22,6 +21,7 @@ import (
 	"github.com/jeduden/mdsmith/internal/query"
 	"github.com/jeduden/mdsmith/internal/rule"
 	ruledocs "github.com/jeduden/mdsmith/internal/rules"
+	"github.com/jeduden/mdsmith/internal/yamlutil"
 
 	// Import all rule packages so their init() functions register rules.
 	_ "github.com/jeduden/mdsmith/internal/rules/ambiguousemphasis"
@@ -415,11 +415,8 @@ func readFrontMatterRaw(path string, maxBytes int64) (map[string]any, error) {
 	delim := []byte("---\n")
 	yamlBytes := prefix[len(delim) : len(prefix)-len(delim)]
 
-	if err := lint.RejectYAMLAliases(yamlBytes); err != nil {
-		return nil, fmt.Errorf("parsing front matter: %w", err)
-	}
 	var raw map[string]any
-	if err := yaml.Unmarshal(yamlBytes, &raw); err != nil {
+	if err := yamlutil.UnmarshalSafe(yamlBytes, &raw); err != nil {
 		return nil, fmt.Errorf("parsing front matter: %w", err)
 	}
 	// Distinguish empty front matter (---\n---\n) from absent front matter.
@@ -463,7 +460,7 @@ func runInit(args []string) int {
 	fm := true
 	cfg.FrontMatter = &fm
 
-	data, err := yaml.Marshal(cfg)
+	data, err := yamlutil.Marshal(cfg)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "mdsmith: marshalling config: %v\n", err)
 		return 2

internal/archetype/gensection/parse.go

@@ -7,7 +7,7 @@ import (
 
 	"github.com/jeduden/mdsmith/internal/fieldinterp"
 	"github.com/jeduden/mdsmith/internal/lint"
-	"gopkg.in/yaml.v3"
+	"github.com/jeduden/mdsmith/internal/yamlutil"
 )
 
 // MarkerPair holds the line numbers and parsed content of a start/end marker pair.
@@ -219,13 +219,7 @@ func ParseYAMLBody(
 ) (map[string]any, []lint.Diagnostic) {
 	var rawMap map[string]any
 	if mp.YAMLBody != "" {
-		if err := lint.RejectYAMLAliases([]byte(mp.YAMLBody)); err != nil {
-			return nil, []lint.Diagnostic{
-				MakeDiag(ruleID, ruleName, filePath, mp.StartLine,
-					fmt.Sprintf("generated section YAML: %v", err)),
-			}
-		}
-		if err := yaml.Unmarshal([]byte(mp.YAMLBody), &rawMap); err != nil {
+		if err := yamlutil.UnmarshalSafe([]byte(mp.YAMLBody), &rawMap); err != nil {
 			return nil, []lint.Diagnostic{
 				MakeDiag(ruleID, ruleName, filePath, mp.StartLine,
 					fmt.Sprintf("generated section has invalid YAML: %v", err)),

internal/config/convention.go

@@ -97,10 +97,11 @@ func copyConventionPreset(p map[string]RuleCfg) map[string]RuleCfg {
 // clean type error. Inspecting the raw node tag is the only way to
 // catch the type mismatch before that coercion happens.
 func validateConventionScalar(data []byte) error {
+	// yaml.Unmarshal into yaml.Node does not expand aliases, so this
+	// is safe without an alias-rejection pre-check. Errors are swallowed
+	// because Load's subsequent UnmarshalSafe call will surface them.
 	var node yaml.Node
 	if err := yaml.Unmarshal(data, &node); err != nil {
-		// A YAML parse error is reported by Load's yaml.Unmarshal
-		// call already; do not double-report.
 		return nil
 	}
 	if node.Kind != yaml.DocumentNode || len(node.Content) == 0 {

internal/config/load.go

@@ -6,8 +6,8 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/jeduden/mdsmith/internal/lint"
 	"github.com/jeduden/mdsmith/internal/rule"
+	"github.com/jeduden/mdsmith/internal/yamlutil"
 	"gopkg.in/yaml.v3"
 )
 
@@ -25,18 +25,14 @@ func Load(path string) (*Config, error) {
 		return nil, fmt.Errorf("reading config file: %w", err)
 	}
 
-	if err := lint.RejectYAMLAliases(data); err != nil {
-		return nil, fmt.Errorf("parsing config file: %w", err)
-	}
-
-	// Catch non-string `convention:` values before yaml.Unmarshal
+	// Catch non-string `convention:` values before UnmarshalSafe
 	// silently coerces them into the string field.
 	if err := validateConventionScalar(data); err != nil {
 		return nil, fmt.Errorf("parsing config file: %w", err)
 	}
 
 	var cfg Config
-	if err := yaml.Unmarshal(data, &cfg); err != nil {
+	if err := yamlutil.UnmarshalSafe(data, &cfg); err != nil {
 		return nil, fmt.Errorf("parsing config file: %w", err)
 	}
 
@@ -81,11 +77,8 @@ func Load(path string) (*Config, error) {
 // present in data, or an empty set on parse error. It rejects
 // anchor/alias usage for the same reason yamlHasKey does.
 func topLevelKeySet(data []byte) map[string]bool {
-	if err := lint.RejectYAMLAliases(data); err != nil {
-		return map[string]bool{}
-	}
-	var node yaml.Node
-	if err := yaml.Unmarshal(data, &node); err != nil {
+	node, err := yamlutil.UnmarshalNodeSafe(data)
+	if err != nil {
 		return map[string]bool{}
 	}
 	if node.Kind != yaml.DocumentNode || len(node.Content) == 0 {

internal/corpus/config.go

@@ -7,8 +7,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/jeduden/mdsmith/internal/lint"
-	"gopkg.in/yaml.v3"
+	"github.com/jeduden/mdsmith/internal/yamlutil"
 )
 
 const (
@@ -32,11 +31,8 @@ func LoadConfig(path string) (*Config, error) {
 		return nil, fmt.Errorf("read config: %w", err)
 	}
 
-	if err := lint.RejectYAMLAliases(content); err != nil {
-		return nil, fmt.Errorf("parse config yaml: %w", err)
-	}
 	var cfg Config
-	if err := yaml.Unmarshal(content, &cfg); err != nil {
+	if err := yamlutil.UnmarshalSafe(content, &cfg); err != nil {
 		return nil, fmt.Errorf("parse config yaml: %w", err)
 	}
 	applyConfigDefaults(&cfg)
@@ -82,11 +78,8 @@ func mergeLocalOverrides(configPath string, cfg *Config) error {
 		return fmt.Errorf("read local override config: %w", err)
 	}
 
-	if err := lint.RejectYAMLAliases(content); err != nil {
-		return fmt.Errorf("parse local override config: %w", err)
-	}
 	var local localOverrideConfig
-	if err := yaml.Unmarshal(content, &local); err != nil {
+	if err := yamlutil.UnmarshalSafe(content, &local); err != nil {
 		return fmt.Errorf("parse local override config: %w", err)
 	}
 

internal/kindsout/kindsout.go

@@ -12,7 +12,7 @@ import (
 	"strings"
 
 	"github.com/jeduden/mdsmith/internal/config"
-	"gopkg.in/yaml.v3"
+	"github.com/jeduden/mdsmith/internal/yamlutil"
 )
 
 // --- JSON shapes ---
@@ -202,7 +202,7 @@ func WriteBodyText(w io.Writer, name string, body config.KindBody) error {
 		Rules:      body.Rules,
 		Categories: body.Categories,
 	}
-	data, err := yaml.Marshal(wrap)
+	data, err := yamlutil.Marshal(wrap)
 	if err != nil {
 		return err
 	}

internal/lint/frontmatter.go

@@ -3,7 +3,7 @@ package lint
 import (
 	"bytes"
 
-	"gopkg.in/yaml.v3"
+	"github.com/jeduden/mdsmith/internal/yamlutil"
 )
 
 // StripFrontMatter removes YAML front matter delimited by "---\n"
@@ -47,14 +47,10 @@ func ParseFrontMatterKinds(fm []byte) ([]string, error) {
 		return nil, nil
 	}
 
-	if err := RejectYAMLAliases(body); err != nil {
-		return nil, err
-	}
-
 	var parsed struct {
 		Kinds []string `yaml:"kinds"`
 	}
-	if err := yaml.Unmarshal(body, &parsed); err != nil {
+	if err := yamlutil.UnmarshalSafe(body, &parsed); err != nil {
 		return nil, err
 	}
 	return parsed.Kinds, nil

internal/lint/yamlsafe.go

@@ -7,71 +7,27 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var yamlAliasTests = []struct {
-	name    string
-	input   string
-	wantErr bool
-}{
-	{"clean YAML", "title: Hello\nauthor: World\n", false},
-	{"anchor definition", "base: &base\n  name: foo\n", true},
-	{"alias reference", "child:\n  <<: *base\n", true},
-	{"ampersand in quoted string", "title: \"Q&A Session\"\n", false},
-	{"ampersand in single quoted string", "title: 'Q&A'\n", false},
-	{"asterisk in quoted string", "note: \"use *bold* text\"\n", false},
-	{"ampersand in unquoted value", "title: Q&A\n", false},
-	{"billion laughs chain", "a: &a [\"lol\"]\nb: &b [*a,*a]\nc: &c [*b,*b]\n", true},
-	{"empty input", "", false},
-	{"asterisk not followed by identifier", "note: 5 * 3 = 15\n", false},
-	{"anchor at start of line", "&anchor value\n", true},
-	{"alias at start of value", "key: *alias\n", true},
-	{"block scalar with ampersand", "key: |\n  &name in block\n", false},
-	{"block scalar with asterisk", "key: >\n  *name in folded\n", false},
-	{"comment with ampersand", "key: val # &anchor\n", false},
-	{"comment with asterisk", "key: val # *alias\n", false},
-	{"escaped quote in double string", "key: \"she said \\\"&hello\\\"\"\n", false},
-	{"doubled single quote", "key: 'it''s &here'\n", false},
-	{"syntax error ignored", "key: [unclosed\n", false},
-	{"multi-document clean", "title: a\n---\ntitle: b\n", false},
-}
-
-func TestRejectYAMLAliases(t *testing.T) {
-	for _, tt := range yamlAliasTests {
-		t.Run(tt.name, func(t *testing.T) {
-			err := RejectYAMLAliases([]byte(tt.input))
-			if tt.wantErr {
-				require.Error(t, err)
-				assert.Contains(t, err.Error(), "anchors/aliases are not permitted")
-			} else {
-				assert.NoError(t, err)
-			}
-		})
-	}
-}
-
+// TestRejectYAMLAliases_FrontMatter verifies that ParseFrontMatterKinds
+// rejects YAML anchors/aliases via the yamlutil safe-unmarshal path.
 func TestRejectYAMLAliases_FrontMatter(t *testing.T) {
 	t.Run("anchor in front matter is rejected", func(t *testing.T) {
-		doc := []byte("---\na: &a [\"lol\"]\nb: &b [*a,*a]\nc: &c [*b,*b]\n---\n# Title\n")
+		doc := []byte("---\na: &a [\"lol\"]\nb: &b [*a,*a]\nc: &c [*b,*b]\nkinds: [doc]\n---\n# Title\n")
 		prefix, content := StripFrontMatter(doc)
 		require.NotNil(t, prefix)
 		assert.Contains(t, string(content), "# Title")
 
-		delim := []byte("---\n")
-		yamlBytes := prefix[len(delim) : len(prefix)-len(delim)]
-
-		err := RejectYAMLAliases(yamlBytes)
+		_, err := ParseFrontMatterKinds(prefix)
 		require.Error(t, err)
 		assert.Contains(t, err.Error(), "anchors/aliases are not permitted")
 	})
 
 	t.Run("clean front matter is accepted", func(t *testing.T) {
-		doc := []byte("---\ntitle: \"Q&A Guide\"\nstatus: draft\n---\n# Title\n")
+		doc := []byte("---\ntitle: \"Q&A Guide\"\nkinds: [doc]\n---\n# Title\n")
 		prefix, _ := StripFrontMatter(doc)
 		require.NotNil(t, prefix)
 
-		delim := []byte("---\n")
-		yamlBytes := prefix[len(delim) : len(prefix)-len(delim)]
-
-		err := RejectYAMLAliases(yamlBytes)
-		assert.NoError(t, err)
+		kinds, err := ParseFrontMatterKinds(prefix)
+		require.NoError(t, err)
+		assert.Equal(t, []string{"doc"}, kinds)
 	})
 }