Skip to content

Fix GraderConfig.UnmarshalYAML to reject unknown fields at grader entry level#150

Closed
Copilot wants to merge 2 commits intolarryo/validate_yamlfrom
copilot/sub-pr-133-again
Closed

Fix GraderConfig.UnmarshalYAML to reject unknown fields at grader entry level#150
Copilot wants to merge 2 commits intolarryo/validate_yamlfrom
copilot/sub-pr-133-again

Conversation

Copy link
Contributor

Copilot AI commented Mar 19, 2026

LoadBenchmarkSpec sets KnownFields(true) on the top-level decoder, but GraderConfig.UnmarshalYAML was calling node.Decode(&raw) which doesn't inherit that setting — unknown keys alongside type, name, weight, etc. were silently accepted.

Changes

  • internal/models/spec.go: Changed GraderConfig.UnmarshalYAML to marshal the node back to bytes and re-decode with an explicit yaml.NewDecoder + KnownFields(true), matching the pattern already used by decodeYAMLNode for the config: sub-object.
  • internal/models/spec_test.go: Added two new cases to TestLoadBenchmarkSpec_StrictYAML — one for an unknown field at the grader entry level and one for an unknown field inside config:.
# Previously silently accepted — now rejected
graders:
  - name: my-grader
    type: text
    unknown_grader_field: oops   # ← was ignored, now errors
    config:
      contains: ["hello"]
      unknown_config_field: oops  # ← already rejected via decodeYAMLNode, now explicitly tested

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: LarryOsterman <8220172+LarryOsterman@users.noreply.github.com>
Copilot AI changed the title [WIP] [WIP] Address feedback on YAML input validation and tests in original PR Fix GraderConfig.UnmarshalYAML to reject unknown fields at grader entry level Mar 19, 2026
Copilot AI requested a review from LarryOsterman March 19, 2026 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants