diff --git a/go/pkg/lockfile/compat.go b/go/pkg/lockfile/compat.go index 9deb9c9..78bda77 100644 --- a/go/pkg/lockfile/compat.go +++ b/go/pkg/lockfile/compat.go @@ -53,6 +53,12 @@ func parseInternal(contents []byte, policy *VersionPolicy, paths []string) (File if err := yaml.Unmarshal(contents, &root); err != nil { return File{}, newYAMLParseError(err) } + // yaml.v3's Decode rejects duplicate mapping keys, but with a generic + // message. Detect duplicate dependency keys first so we can return a + // domain-specific, positioned error before the generic decode error fires. + if pe := rejectDuplicateDependencyKeys(&root); pe != nil { + return File{}, pe + } var f File if err := root.Decode(&f); err != nil { return File{}, newYAMLParseError(err) @@ -477,3 +483,38 @@ func validateKnownFieldsVersioned(f *File, paths []string, version string) *Pars } return nil } + +// rejectDuplicateDependencyKeys walks the top-level `dependencies` mapping and +// returns a positioned ParseError on the first duplicate key. yaml.v3's Decode +// would reject duplicates too, but with a generic message; this yields a +// domain-specific one. Defensive: any unexpected node shape returns nil and +// lets the normal decode path handle it. +func rejectDuplicateDependencyKeys(root *yaml.Node) *ParseError { + m := docMapping(root) + if m == nil { + return nil + } + _, deps := mappingEntry(m, "dependencies") + if deps == nil || deps.Kind != yaml.MappingNode { + return nil + } + seen := make(map[string]int, len(deps.Content)/2) + for i := 0; i+1 < len(deps.Content); i += 2 { + k := deps.Content[i] + // Only compare real string scalar keys. Aliases or non-scalar keys can + // share a Value (e.g. empty), so skip them and let Decode surface the + // normal shape error rather than a misleading duplicate. + if k.Kind != yaml.ScalarNode || k.Tag != "!!str" { + continue + } + if first, dup := seen[k.Value]; dup { + return &ParseError{ + Line: k.Line, + Column: k.Column, + Msg: fmt.Sprintf("duplicate dependency key %q in lockfile dependencies (first defined at line %d)", k.Value, first), + } + } + seen[k.Value] = k.Line + } + return nil +} diff --git a/go/pkg/lockfile/compat_test.go b/go/pkg/lockfile/compat_test.go index 04cc805..2391a4c 100644 --- a/go/pkg/lockfile/compat_test.go +++ b/go/pkg/lockfile/compat_test.go @@ -237,3 +237,45 @@ func TestDefaultPolicy(t *testing.T) { assert.Equal(t, "v0.0.1", p.Min) assert.Equal(t, Version, p.Max) } + +func TestParse_DuplicateDependencyKey_ConflictingSHAs(t *testing.T) { + // Two identical dependency keys with conflicting commit SHAs must be + // rejected with a domain-specific, positioned parse error rather than + // yaml.v3's generic "mapping key already defined" message. + input := `version: 'v0.0.2' +workflows: + '.github/workflows/scenario-duplicate-deps-conflicting-failure.yml': + - 'nodeselector/actions-test-fixtures@dup-test' +dependencies: + 'nodeselector/actions-test-fixtures@dup-test': + ref: 'dup-test' + commit: 'sha1-33a384c001ed694ba938667a1d5ace65d6c49de3' + owner_id: 29457092 + repo_id: 1203329948 + 'nodeselector/actions-test-fixtures@dup-test': + ref: 'dup-test' + commit: 'sha1-0000000000000000000000000000000000000000' + owner_id: 29457092 + repo_id: 1203329948 +` + for _, tc := range []struct { + name string + parse func() (File, error) + }{ + {"Parse", func() (File, error) { return Parse([]byte(input)) }}, + {"ParseWithPolicy", func() (File, error) { return ParseWithPolicy([]byte(input), DefaultPolicy()) }}, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := tc.parse() + require.Error(t, err) + assert.Contains(t, err.Error(), "duplicate dependency key") + assert.Contains(t, err.Error(), "nodeselector/actions-test-fixtures@dup-test") + + // Must be the domain path (positioned *ParseError), not a bare + // yaml.v3 error — this is what surfaces as a dispatch-time 422. + var pe *ParseError + require.ErrorAs(t, err, &pe) + assert.NotZero(t, pe.Line, "duplicate key error must carry a line position") + }) + } +}