fix(nvd-cve-osv): handle the edge case of a valid repo reference (#2678)

The repo caching validates repos are usable for tag to commit mapping
(i.e. they have tags) before adding them.

It's possible that a CVE has a commit reference for a repo with no tags
(but it's usable as-is because we assume the commit is a Fixed commit).

These repos were not being added to the repo cache, but the repo cache
was being used as a short cut by subsequent calls to
`ReposFromReferences()`, overwriting the internal state on what repos
were known for a CVE with multiple CPEs.

This meant that for CVE-2024-45313, which has multiple CPE entries:
- the `Fixed` commit was being extracted as a reference (as desired)
- the full set of known repos was not being cached
- the versions also extracted didn't resolve (as can happen)
- the CVE was being rejected as having no Fixed commits (because these
are searched for repo, from the set of *expected* repos, which was being
overwritten by the cache hit on the second iteration through the CPEs)

This meant that CVE-2024-45313 initially successfully converted before
the NVD analyzed it, but started failing to convert after the NVD added
CPEs.

vulnfeeds/cmd/nvd-cve-osv/main.go

@@ -197,12 +197,6 @@ func refAcceptable(ref cves.Reference, tagDenyList []string) bool {
 
 // Examines the CVE references for a CVE and derives repos for it, optionally caching it.
 func ReposFromReferences(CVE string, cache VendorProductToRepoMap, vp *VendorProduct, refs []cves.Reference, tagDenyList []string) (repos []string) {
-	// This currently only gets called for cache misses, but make it not rely on that assumption.
-	if vp != nil {
-		if cachedRepos, ok := cache[*vp]; ok {
-			return cachedRepos
-		}
-	}
 	for _, ref := range refs {
 		// If any of the denylist tags are in the ref's tag set, it's out of consideration.
 		if !refAcceptable(ref, tagDenyList) {
@@ -225,6 +219,7 @@ func ReposFromReferences(CVE string, cache VendorProductToRepoMap, vp *VendorPro
 		repos = append(repos, repo)
 		maybeUpdateVPRepoCache(cache, vp, repo)
 	}
+	Logger.Infof("[%s]: Derived %q for %q %q using references", CVE, repos, vp.Vendor, vp.Product)
 	return repos
 }
 

vulnfeeds/cmd/nvd-cve-osv/main_test.go

@@ -27,7 +27,7 @@ func TestReposFromReferences(t *testing.T) {
 				cache: nil,
 				vp:    &VendorProduct{"theradsystem_project", "theradsystem"},
 				refs: []cves.Reference{
-				{
+					{
 						Source: "cna@vuldb.com",
 						Tags:   []string{"Patch", "Third Party Advisory"},
 						Url:    "https://github.com/saemorris/TheRadSystem/commit/bfba26bd34af31648a11af35a0bb66f1948752a6"},
@@ -45,3 +45,80 @@ func TestReposFromReferences(t *testing.T) {
 		})
 	}
 }
+
+func Test_maybeUpdateVPRepoCache(t *testing.T) {
+	type args struct {
+		cache VendorProductToRepoMap
+		vp    *VendorProduct
+		repos []string
+	}
+	tests := []struct {
+		name      string
+		args      args
+		wantCache VendorProductToRepoMap
+	}{
+		{
+			name: "Test with no cache",
+			args: args{
+				cache: nil,
+				vp:    &VendorProduct{"avendor", "aproduct"},
+				repos: []string{"https://github.com/google/osv.dev"},
+			},
+			wantCache: nil,
+		},
+		{
+			name: "Test with an empty cache",
+			args: args{
+				cache: VendorProductToRepoMap{},
+				vp:    &VendorProduct{"avendor", "aproduct"},
+				repos: []string{"https://github.com/google/osv.dev"},
+			},
+			wantCache: VendorProductToRepoMap{
+				VendorProduct{"avendor", "aproduct"}: []string{"https://github.com/google/osv.dev"},
+			},
+		},
+		{
+			name: "Test with an empty cache and an unusable repo",
+			args: args{
+				cache: VendorProductToRepoMap{},
+				vp:    &VendorProduct{"avendor", "aproduct"},
+				repos: []string{"https://github.com/vendor/repo"},
+			},
+			wantCache: VendorProductToRepoMap{},
+		},
+		{
+			name: "Test with an existing cache",
+			args: args{
+				cache: VendorProductToRepoMap{
+					VendorProduct{"avendor", "aproduct"}: []string{"https://github.com/google/osv.dev"},
+				},
+				vp:    &VendorProduct{"avendor", "aproduct"},
+				repos: []string{"https://github.com/google/osv-scanner"},
+			},
+			wantCache: VendorProductToRepoMap{
+				VendorProduct{"avendor", "aproduct"}: []string{"https://github.com/google/osv.dev", "https://github.com/google/osv-scanner"},
+			},
+		},
+		{
+			name: "Test with an empty cache adding two values",
+			args: args{
+				cache: VendorProductToRepoMap{},
+				vp:    &VendorProduct{"avendor", "aproduct"},
+				repos: []string{"https://github.com/google/osv.dev", "https://github.com/google/osv-scanner"},
+			},
+			wantCache: VendorProductToRepoMap{
+				VendorProduct{"avendor", "aproduct"}: []string{"https://github.com/google/osv.dev", "https://github.com/google/osv-scanner"},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			for _, repo := range tt.args.repos {
+				maybeUpdateVPRepoCache(tt.args.cache, tt.args.vp, repo)
+			}
+			if !reflect.DeepEqual(tt.args.cache, tt.wantCache) {
+				t.Errorf("maybeUpdateVPRepoCache() have %#v, wanted %#v", tt.args.cache, tt.wantCache)
+			}
+		})
+	}
+}