Fix flaky test for NuGet lock file parser in Gemnasium
Summary
Unit test TestNuget/Parse/duplicates/dependencies
is flaky, making the pipeline of the gemnasium project fail from time to time.
This is might not be a user-facing bug but contributors to the gemnasium
project have to trigger a new pipeline when the go test
job fail. That's a waste of time, and when deploying a new release of gemnasium
this makes for extra context switching between issue page, MR page, and pipeline view, until the pipeline succeeds.
Further details
The unit test fails at scanner/parser/nuget/nuget_test.go#L80 when comparing the dependencies extracted from ./fixtures/duplicates/packages.lock.json to the ones recorded in ./duplicates/dependencies.json.
Here's an excerpt of the package.lock.json
:
{
"version": 1,
"dependencies": {
".NETCoreApp,Version=v5.0": {
"jive": {
"type": "Direct",
"requested": "[0.2.0, )",
"resolved": "0.1.0"
},
"Microsoft.ChakraCore": {
"type": "Direct",
"requested": "[1.12, )",
"resolved": "1.12"
}
},
".NETCoreApp,Version=v3.0": {
"jive": {
"type": "Direct",
"requested": "[0.1.0, )",
"resolved": "0.1.0",
"contentHash": "oPBGSgY7X2/CMnWGDVdCIDnvaQGXfZz+hnC2PmvjStqDz72ZwNbkPqXYo47QyGOfLYdsp1aieP+Km2KyZMDUSg=="
},
"Microsoft.ChakraCore": {
"type": "Direct",
"requested": "[1.11.18, )",
"resolved": "1.11.18",
"contentHash": "jPtaoN8ZaRcioMvl2aUMiqhmKVMxYYyR0UCCsQ1viKC9qoiqZGWtXufdOAm1U/IzULAtDmVBTdsSdDyt4ND81A=="
}
}
}
(This fixture might be updated in gitlab-org/security-products/analyzers/gemnasium!197 (merged).)
Currently the parser either selects version 1.12
or version 1.11.18
of Microsoft.ChakraCore
as a top-level dependency because it resolves dependency names to package names and versions using a map, and this map is built when iterating the "targets" and their dependencies in a non-deterministic way.
Links
- https://gitlab.com/gitlab-org/security-products/analyzers/gemnasium/-/blob/v2.29.6/scanner/parser/nuget/nuget.go#L75
- https://gitlab.com/gitlab-org/security-products/analyzers/gemnasium/-/blob/v2.29.6/scanner/parser/nuget/nuget.go#L86
- https://gitlab.com/gitlab-org/security-products/analyzers/gemnasium/-/blob/v2.29.6/scanner/parser/nuget/nuget.go#L95
NOTE: This only affects the dependencies
return by the NuGet parser, not the packages
.
Steps to reproduce
Trigger pipelines for the gemnasium project until the go test
job fails.
What is the current bug behavior?
go test
job fails because TestNuget/Parse/duplicates/dependencies
fails because it either returns Microsoft.ChakraCore
1.11.18 or 1.12 as a top-level, when processing the duplicates
fixtures
What is the expected correct behavior?
go test
always succeeds, and the NuGet lock file parser always returns both Microsoft.ChakraCore
1.11.18 and 1.12 as top-level dependencies
Relevant logs and/or screenshots
https://gitlab.com/gitlab-org/security-products/analyzers/gemnasium/-/jobs/1303773084
=== RUN TestNuget/Parse/duplicates/dependencies
nuget_test.go:54:
Error Trace: nuget_test.go:54
Error: elements differ
extra elements in list A:
([]interface {}) (len=2) {
(parser.Dependency) {
From: (*parser.Package)(<nil>),
To: (*parser.Package)({
Name: (string) (len=20) "Microsoft.ChakraCore",
Version: (string) (len=7) "1.11.18"
}),
VersionRange: (string) (len=11) "[1.11.18, )",
Dev: (bool) false
},
(parser.Dependency) {
From: (*parser.Package)(<nil>),
To: (*parser.Package)({
Name: (string) (len=20) "Microsoft.ChakraCore",
Version: (string) (len=7) "1.11.18"
}),
VersionRange: (string) (len=8) "[1.12, )",
Dev: (bool) false
}
}
extra elements in list B:
([]interface {}) (len=2) {
(parser.Dependency) {
From: (*parser.Package)(<nil>),
To: (*parser.Package)({
Name: (string) (len=20) "Microsoft.ChakraCore",
Version: (string) (len=4) "1.12"
}),
VersionRange: (string) (len=11) "[1.11.18, )",
Dev: (bool) false
},
(parser.Dependency) {
From: (*parser.Package)(<nil>),
To: (*parser.Package)({
Name: (string) (len=20) "Microsoft.ChakraCore",
Version: (string) (len=4) "1.12"
}),
VersionRange: (string) (len=8) "[1.12, )",
Dev: (bool) false
}
}
Full log: issue-328451-job.log
Possible fixes
- workaround: change the way the NuGet lock file parser build the
dependencies
, and make it deterministic when processing a lock file that has multiple targets with similar or identical packages - workaround: only keep 1 target defined in the lock file, and ignore others; this ensures data consistent and simplify the implementation, but vulnerabilities might not be reported if the target being scanned isn't the one used in production
- workaround: generate 1 dependency graph per target, and merge them before returning
packages
anddependencies
; merging dependency graphs isn't trivial though because they might end sharing the very same node, because of the way duplicates are removed from the returnedpackages
- true fix: return 1 dependency graph per target; this is a large change impacting everything from the analyzer to the report format to the GitLab backend and frontend