From 4fed59ac85a0f17506e76536cd002b4b714e602c Mon Sep 17 00:00:00 2001
From: Rodin <rodin@foredthought.ai>
Date: Sun, 10 May 2026 16:50:07 -0700
Subject: [PATCH] yaml: enable strict field checking to catch typos

Addresses PR #58 MINOR finding: YAML decoder now rejects unknown fields.

- Enable KnownFields(true) on YAML decoder to catch typos like
  'focuss' or 'identiy' in persona files
- Since yaml.Node.Decode() doesn't support KnownFields, we now
  do a two-pass decode: first pass checks depth limits, second
  pass decodes with strict field checking
- Add tests for unknown field rejection at top-level and nested levels
---
 review/persona.go      | 15 +++++++++---
 review/persona_test.go | 53 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/review/persona.go b/review/persona.go
index 2dc7a05..3172735 100644
--- a/review/persona.go
+++ b/review/persona.go
@@ -150,12 +150,15 @@ func parsePersona(data []byte, source string) (*Persona, error) {
 	return &p, nil
 }
 
-// unmarshalYAMLWithDepthLimit unmarshals YAML data with explicit depth limiting.
-// This protects against stack exhaustion from deeply nested structures.
+// unmarshalYAMLWithDepthLimit unmarshals YAML data with explicit depth limiting
+// and strict field checking. This protects against stack exhaustion from deeply
+// nested structures and catches typos in field names.
 // Note: Multi-document YAML files are accepted but only the first document is
 // parsed; additional documents are silently ignored. This is acceptable for
 // persona files where multi-document support is not a use case.
 func unmarshalYAMLWithDepthLimit(data []byte, out any, maxDepth int) error {
+	// First pass: decode into a yaml.Node to check depth limits.
+	// This prevents stack exhaustion before we attempt to decode into structs.
 	var node yaml.Node
 	dec := yaml.NewDecoder(bytes.NewReader(data))
 	if err := dec.Decode(&node); err != nil {
@@ -166,7 +169,13 @@ func unmarshalYAMLWithDepthLimit(data []byte, out any, maxDepth int) error {
 		return err
 	}
 
-	return node.Decode(out)
+	// Second pass: decode with strict field checking enabled.
+	// KnownFields(true) rejects unknown keys, catching typos like "focuss" or "identiy".
+	// We must re-decode from the original data because yaml.Node.Decode() doesn't
+	// support the KnownFields option.
+	strictDec := yaml.NewDecoder(bytes.NewReader(data))
+	strictDec.KnownFields(true)
+	return strictDec.Decode(out)
 }
 
 // checkYAMLDepth recursively checks that YAML nodes don't exceed the depth limit.
diff --git a/review/persona_test.go b/review/persona_test.go
index bb6fa14..cd77434 100644
--- a/review/persona_test.go
+++ b/review/persona_test.go
@@ -512,3 +512,56 @@ func TestListBuiltinPersonasSortedOrder(t *testing.T) {
 		}
 	}
 }
+
+func TestYAMLUnknownFieldsRejected(t *testing.T) {
+	tests := []struct {
+		name    string
+		content string
+		wantErr string
+	}{
+		{
+			name: "unknown top-level field",
+			content: `name: test
+identity: test identity
+unknown_field: should fail
+`,
+			wantErr: "unknown_field",
+		},
+		{
+			name: "typo in field name",
+			content: `name: test
+identiy: typo should fail
+`,
+			wantErr: "identiy",
+		},
+		{
+			name: "unknown field in severity",
+			content: `name: test
+identity: test
+severity:
+  major: Major
+  minro: typo
+`,
+			wantErr: "minro",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			dir := t.TempDir()
+			path := filepath.Join(dir, "unknown.yaml")
+			if err := os.WriteFile(path, []byte(tt.content), 0644); err != nil {
+				t.Fatalf("failed to write test file: %v", err)
+			}
+
+			_, err := LoadPersona(path)
+			if err == nil {
+				t.Errorf("expected error for unknown field %q, got nil", tt.wantErr)
+				return
+			}
+			if !strings.Contains(err.Error(), tt.wantErr) {
+				t.Errorf("error = %q, want containing %q", err.Error(), tt.wantErr)
+			}
+		})
+	}
+}