fix: more nvd optimisations and badrepo cache (#4723)

This change fixes some nits, introduces a bad repository cache and stops
the conversion from discarding versions just because a commit was
provided in the references.

- Removed CVEID from error formatting as at time of logging, CVEID is
already provided via slog.
- Removed warnings from some logs in favour of adding to metrics.Notes
file for better investigation later. There may be value in keeping these
at warning levels only if we also are prepared to handle investigating
spikes, in which case logs should appear closer to what is causing the
errors (network requests generally).
- made errors lowercase as linter demands

Author: jess-lowe

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

@@ -89,12 +89,12 @@ func main() {
 	logger.Info("NVD Conversion run complete")
 }
 
-func processCVE(cve models.NVDCVE, vpRepoCache *cves.VPRepoCache, repoTagsCache *git.RepoTagsCache) error {
+func processCVE(cve models.NVDCVE, vpRepoCache *cves.VPRepoCache, repoTagsCache *git.RepoTagsCache) models.ConversionOutcome {
 	metrics := &models.ConversionMetrics{
 		CVEID: cve.ID,
 		CNA:   "nvd",
 	}
-	repos := nvd.FindRepos(cve, vpRepoCache, metrics)
+	repos := nvd.FindRepos(cve, vpRepoCache, repoTagsCache, metrics)
 	metrics.Repos = repos
 
 	var err error
@@ -106,29 +106,28 @@ func processCVE(cve models.NVDCVE, vpRepoCache *cves.VPRepoCache, repoTagsCache
 	}
 	// Parse this error to determine which failure mode it was
 	if err != nil {
-		logger.Warn("Failed to generate an OSV record", slog.String("cve", string(cve.ID)), slog.Any("err", err))
 		if errors.Is(err, nvd.ErrNoRanges) {
 			metrics.Outcome = models.NoRanges
-			return err
+			return models.NoRanges
 		}
 		if errors.Is(err, nvd.ErrUnresolvedFix) {
 			metrics.Outcome = models.FixUnresolvable
-			return err
+			return models.FixUnresolvable
 		}
 		metrics.Outcome = models.ConversionUnknown
 
-		return err
+		return models.ConversionUnknown
 	}
 	metrics.Outcome = models.Successful
 
-	return nil
+	return models.Successful
 }
 
 func worker(wg *sync.WaitGroup, jobs <-chan models.NVDCVE, _ string, vpRepoCache *cves.VPRepoCache, repoTagsCache *git.RepoTagsCache) {
 	defer wg.Done()
 	for cve := range jobs {
-		if err := processCVE(cve, vpRepoCache, repoTagsCache); err != nil {
-			logger.Warn("Failed to generate an OSV record", slog.String("cve", string(cve.ID)), slog.Any("err", err))
+		if processCVE(cve, vpRepoCache, repoTagsCache) != models.Successful {
+			logger.Info("Failed to generate an OSV record", slog.String("cve", string(cve.ID)))
 		} else {
 			logger.Info("Generated OSV record for "+string(cve.ID), slog.String("cve", string(cve.ID)))
 		}

vulnfeeds/conversion/nvd/converter.go

@@ -36,7 +36,7 @@ func CVEToOSV(cve models.NVDCVE, repos []string, cache *git.RepoTagsCache, direc
 		maybeVendorName = CPE.Vendor
 		maybeProductName = CPE.Product
 		if err != nil {
-			return fmt.Errorf("[%s]: Can't generate an OSV record without valid CPE data", cve.ID)
+			return errors.New("can't generate an OSV record without valid CPE data")
 		}
 	}
 
@@ -48,13 +48,13 @@ func CVEToOSV(cve models.NVDCVE, repos []string, cache *git.RepoTagsCache, direc
 		// There are some AffectedVersions to try and resolve to AffectedCommits.
 		if len(repos) == 0 {
 			metrics.AddNote("No affected ranges for %q, and no repos to try and convert %+v to tags with", maybeProductName, versions.AffectedVersions)
-			return fmt.Errorf("[%s]: No affected ranges for %q, and no repos to try and convert %+v to tags with", cve.ID, maybeProductName, versions.AffectedVersions)
+			return fmt.Errorf("no affected ranges for %q, and no repos to try and convert %+v to tags with", maybeProductName, versions.AffectedVersions)
 		}
 		metrics.AddNote("Trying to convert version tags to commits: %v with repos: %v", versions, repos)
-		versions, err = cves.GitVersionsToCommits(cve.ID, versions, repos, cache, metrics)
+		versions, err = cves.GitVersionsToCommits(versions, repos, cache, metrics)
 		if err != nil {
-			metrics.AddNote("Failed to convert version tags to commits: %#v", err)
-			return fmt.Errorf("[%s]: Failed to convert version tags to commits: %#w", cve.ID, err)
+			metrics.AddNote("Failed to convert version tags to commits: %+v", err)
+			return fmt.Errorf("failed to convert version tags to commits: %+v %w", versions, err)
 		}
 		hasAnyFixedCommits := false
 		for _, repo := range repos {
@@ -65,8 +65,8 @@ func CVEToOSV(cve models.NVDCVE, repos []string, cache *git.RepoTagsCache, direc
 		}
 
 		if versions.HasFixedVersions() && !hasAnyFixedCommits {
-			metrics.AddNote("Failed to convert fixed version tags to commits: %#v", versions)
-			return fmt.Errorf("[%s]: Failed to convert fixed version tags to commits: %#v %w", cve.ID, versions, ErrUnresolvedFix)
+			metrics.AddNote("Failed to convert fixed version tags to commits: %+v", versions)
+			return fmt.Errorf("failed to convert fixed version tags to commits: %+v %w", versions, ErrUnresolvedFix)
 		}
 
 		hasAnyLastAffectedCommits := false
@@ -78,8 +78,8 @@ func CVEToOSV(cve models.NVDCVE, repos []string, cache *git.RepoTagsCache, direc
 		}
 
 		if versions.HasLastAffectedVersions() && !hasAnyLastAffectedCommits && !hasAnyFixedCommits {
-			metrics.AddNote("Failed to convert last_affected version tags to commits: %#v", versions)
-			return fmt.Errorf("[%s]: Failed to convert last_affected version tags to commits: %#v %w", cve.ID, versions, ErrUnresolvedFix)
+			metrics.AddNote("Failed to convert last_affected version tags to commits: %+v", versions)
+			return fmt.Errorf("failed to convert last_affected version tags to commits: %+v %w", versions, ErrUnresolvedFix)
 		}
 	}
 
@@ -89,7 +89,7 @@ func CVEToOSV(cve models.NVDCVE, repos []string, cache *git.RepoTagsCache, direc
 
 	if len(v.Affected) == 0 {
 		metrics.AddNote("No affected ranges detected for %q", maybeProductName)
-		return fmt.Errorf("[%s]: No affected ranges detected for %q %w", cve.ID, maybeProductName, ErrNoRanges)
+		return fmt.Errorf("no affected ranges detected for %q %w", maybeProductName, ErrNoRanges)
 	}
 
 	vulnDir := filepath.Join(directory, maybeVendorName, maybeProductName)
@@ -131,7 +131,7 @@ func CVEToPackageInfo(cve models.NVDCVE, repos []string, cache *git.RepoTagsCach
 		maybeVendorName = CPE.Vendor
 		maybeProductName = CPE.Product
 		if err != nil {
-			return fmt.Errorf("[%s]: Can't generate an OSV record without valid CPE data", cve.ID)
+			return errors.New("can't generate an OSV record without valid CPE data")
 		}
 	}
 
@@ -143,13 +143,13 @@ func CVEToPackageInfo(cve models.NVDCVE, repos []string, cache *git.RepoTagsCach
 		// There are some AffectedVersions to try and resolve to AffectedCommits.
 		if len(repos) == 0 {
 			metrics.AddNote("No affected ranges for %q, and no repos to try and convert %+v to tags with", maybeProductName, versions.AffectedVersions)
-			return fmt.Errorf("[%s]: No affected ranges for %q, and no repos to try and convert %+v to tags with", cve.ID, maybeProductName, versions.AffectedVersions)
+			return fmt.Errorf("no affected ranges for %q, and no repos to try and convert %+v to tags with", maybeProductName, versions.AffectedVersions)
 		}
 		logger.Info("Trying to convert version tags to commits", slog.String("cve", string(cve.ID)), slog.Any("versions", versions), slog.Any("repos", repos))
-		versions, err = cves.GitVersionsToCommits(cve.ID, versions, repos, cache, metrics)
+		versions, err = cves.GitVersionsToCommits(versions, repos, cache, metrics)
 		if err != nil {
-			metrics.AddNote("Failed to convert version tags to commits: %#v", err)
-			return fmt.Errorf("[%s]: Failed to convert version tags to commits: %#w", cve.ID, err)
+			metrics.AddNote("Failed to convert version tags to commits: %+v", err)
+			return fmt.Errorf("failed to convert version tags to commits: %+v %w", versions, err)
 		}
 	}
 
@@ -161,8 +161,8 @@ func CVEToPackageInfo(cve models.NVDCVE, repos []string, cache *git.RepoTagsCach
 	}
 
 	if versions.HasFixedVersions() && !hasAnyFixedCommits {
-		metrics.AddNote("Failed to convert fixed version tags to commits: %#v", versions)
-		return fmt.Errorf("[%s]: Failed to convert fixed version tags to commits: %#v %w", cve.ID, versions, ErrUnresolvedFix)
+		metrics.AddNote("Failed to convert fixed version tags to commits: %+v", versions)
+		return fmt.Errorf("failed to convert fixed version tags to commits: %+v %w", versions, ErrUnresolvedFix)
 	}
 
 	hasAnyLastAffectedCommits := false
@@ -173,13 +173,13 @@ func CVEToPackageInfo(cve models.NVDCVE, repos []string, cache *git.RepoTagsCach
 	}
 
 	if versions.HasLastAffectedVersions() && !hasAnyLastAffectedCommits && !hasAnyFixedCommits {
-		metrics.AddNote("Failed to convert last_affected version tags to commits: %#v", versions)
-		return fmt.Errorf("[%s]: Failed to convert last_affected version tags to commits: %#v %w", cve.ID, versions, ErrUnresolvedFix)
+		metrics.AddNote("Failed to convert last_affected version tags to commits: %+v", versions)
+		return fmt.Errorf("failed to convert last_affected version tags to commits: %+v %w", versions, ErrUnresolvedFix)
 	}
 
 	if len(versions.AffectedCommits) == 0 {
 		metrics.AddNote("No affected commit ranges determined for %q", maybeProductName)
-		return fmt.Errorf("[%s]: No affected commit ranges determined for %q %w", cve.ID, maybeProductName, ErrNoRanges)
+		return fmt.Errorf("no affected commit ranges determined for %q %w", maybeProductName, ErrNoRanges)
 	}
 
 	versions.AffectedVersions = nil // these have served their purpose and are not required in the resulting output.
@@ -229,7 +229,7 @@ func CVEToPackageInfo(cve models.NVDCVE, repos []string, cache *git.RepoTagsCach
 }
 
 // FindRepos attempts to find the source code repositories for a given CVE.
-func FindRepos(cve models.NVDCVE, vpRepoCache *cves.VPRepoCache, metrics *models.ConversionMetrics) []string {
+func FindRepos(cve models.NVDCVE, vpRepoCache *cves.VPRepoCache, repoTagsCache *git.RepoTagsCache, metrics *models.ConversionMetrics) []string {
 	// Find repos
 	refs := cve.References
 	CPEs := cves.CPEs(cve)
@@ -246,29 +246,38 @@ func FindRepos(cve models.NVDCVE, vpRepoCache *cves.VPRepoCache, metrics *models
 
 	// Edge case: No CPEs, but perhaps usable references.
 	if len(refs) > 0 && len(CPEs) == 0 {
-		repos := cves.ReposFromReferences(nil, nil, refs, cves.RefTagDenyList, metrics)
+		repos := cves.ReposFromReferences(nil, nil, refs, cves.RefTagDenyList, repoTagsCache, metrics)
 		if len(repos) == 0 {
 			metrics.AddNote("Failed to derive any repos and there were no CPEs")
 			return nil
 		}
 		metrics.AddNote("Derived repos for CVE with no CPEs: %v", repos)
 		reposForCVE = repos
 	}
-
-	// Does it have any application CPEs? Look for pre-computed repos based on VendorProduct.
 	appCPECount := 0
+	vendorProductCombinations := make(map[cves.VendorProduct]bool)
 	for _, CPEstr := range CPEs {
 		CPE, err := cves.ParseCPE(CPEstr)
 		if err != nil {
 			metrics.AddNote("Failed to parse CPE: %v", CPEstr)
-			metrics.Outcome = models.ConversionUnknown
-
 			continue
 		}
-		if CPE.Part == "a" {
-			appCPECount += 1
+		if CPE.Part != "a" {
+			continue
 		}
-		vendorProductKey := cves.VendorProduct{Vendor: CPE.Vendor, Product: CPE.Product}
+		appCPECount += 1
+		vendorProductCombinations[cves.VendorProduct{Vendor: CPE.Vendor, Product: CPE.Product}] = true
+	}
+
+	if len(CPEs) > 0 && appCPECount == 0 {
+		// This CVE is not for software (based on there being CPEs but not any application ones), skip.
+		metrics.Outcome = models.NoSoftware
+		return nil
+	}
+
+	// If there wasn't a repo from the CPE Dictionary, try and derive one from the CVE references.
+	for vendorProductKey := range vendorProductCombinations {
+		// Does it have any application CPEs? Look for pre-computed repos based on VendorProduct.
 		if repos, ok := vpRepoCache.Get(vendorProductKey); ok {
 			metrics.AddNote("Pre-references, derived repos using cache: %v", repos)
 			if len(reposForCVE) == 0 {
@@ -282,35 +291,11 @@ func FindRepos(cve models.NVDCVE, vpRepoCache *cves.VPRepoCache, metrics *models
 				}
 			}
 		}
-	}
-
-	if len(CPEs) > 0 && appCPECount == 0 {
-		// This CVE is not for software (based on there being CPEs but not any application ones), skip.
-		metrics.Outcome = models.NoSoftware
-		return nil
-	}
-
-	vendorProductCombinations := make(map[cves.VendorProduct]bool)
-	for _, CPEstr := range CPEs {
-		CPE, err := cves.ParseCPE(CPEstr)
-		if err != nil {
-			metrics.AddNote("Failed to parse CPE: %v", CPEstr)
-			continue
-		}
-		if CPE.Part != "a" {
-			continue
-		}
-		vendorProductCombinations[cves.VendorProduct{Vendor: CPE.Vendor, Product: CPE.Product}] = true
-	}
-
-	// If there wasn't a repo from the CPE Dictionary, try and derive one from the CVE references.
-	for vendorProductKey := range vendorProductCombinations {
 		if len(reposForCVE) == 0 && len(refs) > 0 {
-			// Continue to only focus on application CPEs.
 			if slices.Contains(cves.VendorProductDenyList, vendorProductKey) {
 				continue
 			}
-			repos := cves.ReposFromReferences(vpRepoCache, &vendorProductKey, refs, cves.RefTagDenyList, metrics)
+			repos := cves.ReposFromReferences(vpRepoCache, &vendorProductKey, refs, cves.RefTagDenyList, repoTagsCache, metrics)
 			if len(repos) == 0 {
 				metrics.AddNote("Failed to derive any repos for %s/%s", vendorProductKey.Vendor, vendorProductKey.Product)
 				continue