yaml: enable strict field checking to catch typos
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 9m33s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 9m54s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 10m51s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 11m30s
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 9m33s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 9m54s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 10m51s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 11m30s
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
This commit is contained in:
+12
-3
@@ -150,12 +150,15 @@ func parsePersona(data []byte, source string) (*Persona, error) {
|
|||||||
return &p, nil
|
return &p, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// unmarshalYAMLWithDepthLimit unmarshals YAML data with explicit depth limiting.
|
// unmarshalYAMLWithDepthLimit unmarshals YAML data with explicit depth limiting
|
||||||
// This protects against stack exhaustion from deeply nested structures.
|
// 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
|
// Note: Multi-document YAML files are accepted but only the first document is
|
||||||
// parsed; additional documents are silently ignored. This is acceptable for
|
// parsed; additional documents are silently ignored. This is acceptable for
|
||||||
// persona files where multi-document support is not a use case.
|
// persona files where multi-document support is not a use case.
|
||||||
func unmarshalYAMLWithDepthLimit(data []byte, out any, maxDepth int) error {
|
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
|
var node yaml.Node
|
||||||
dec := yaml.NewDecoder(bytes.NewReader(data))
|
dec := yaml.NewDecoder(bytes.NewReader(data))
|
||||||
if err := dec.Decode(&node); err != nil {
|
if err := dec.Decode(&node); err != nil {
|
||||||
@@ -166,7 +169,13 @@ func unmarshalYAMLWithDepthLimit(data []byte, out any, maxDepth int) error {
|
|||||||
return err
|
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.
|
// checkYAMLDepth recursively checks that YAML nodes don't exceed the depth limit.
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user