Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions go/pkg/lockfile/compat.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
42 changes: 42 additions & 0 deletions go/pkg/lockfile/compat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}
}
Loading