refactor: use AddNotes and AddSource fields in CVEList conversion (#4080)

Tidy up the code a bit to use an AddNotes method to append notes and
sources.

Author: jess-lowe

vulnfeeds/cmd/cvelist2osv/converter.go

@@ -23,6 +23,7 @@ const (
 
 // ConversionMetrics holds the collected data about the conversion process for a single CVE.
 type ConversionMetrics struct {
+	CVEID          cves.CVEID                      `json:"id"`              // The CVE ID
 	CNA            string                          `json:"cna"`             // The CNA that assigned the CVE.
 	Outcome        string                          `json:"outcome"`         // The final outcome of the conversion (e.g., "Successful", "Failed").
 	Repos          []string                        `json:"repos"`           // A list of repositories extracted from the CVE's references.
@@ -31,6 +32,17 @@ type ConversionMetrics struct {
 	Notes          []string                        `json:"notes"`           // A collection of notes and warnings generated during conversion.
 }
 
+// AddNote adds a formatted note to the ConversionMetrics.
+func (m *ConversionMetrics) AddNote(format string, a ...any) {
+	m.Notes = append(m.Notes, fmt.Sprintf(format, a...))
+	logger.Debug(fmt.Sprintf(format, a...), slog.String("cna", m.CNA), slog.String("cve", string(m.CVEID)))
+}
+
+// AddSource appends a source to the ConversionMetrics
+func (m *ConversionMetrics) AddSource(source VersionSource) {
+	m.VersionSources = append(m.VersionSources, source)
+}
+
 // RefTagDenyList contains reference tags that are often associated with unreliable or
 // irrelevant repository URLs. References with these tags are currently ignored
 // to avoid incorrect repository associations.
@@ -55,7 +67,7 @@ func extractConversionMetrics(cve cves.CVE5, refs []osvschema.Reference, metrics
 	}
 	metrics.RefTypesCount = refTypeCounts
 	for refType, count := range refTypeCounts {
-		metrics.Notes = append(metrics.Notes, fmt.Sprintf("[%s]: Reference Type %s: %d", cve.Metadata.CVEID, refType, count))
+		metrics.AddNote("[%s]: Reference Type %s: %d", cve.Metadata.CVEID, refType, count)
 	}
 
 	// TODO(jesslowe): Add more analysis based on ADP containers, CVSS, KEV, CWE, etc.
@@ -79,14 +91,14 @@ func FromCVE5(cve cves.CVE5, refs []cves.Reference, metrics *ConversionMetrics)
 
 	published, err := cves.ParseCVE5Timestamp(cve.Metadata.DatePublished)
 	if err != nil {
-		metrics.Notes = append(metrics.Notes, "Published date failed to parse, setting time to now")
+		metrics.AddNote("[%s]: Published date failed to parse, setting time to now", cve.Metadata.CVEID)
 		published = time.Now()
 	}
 	v.Published = published
 
 	modified, err := cves.ParseCVE5Timestamp(cve.Metadata.DateUpdated)
 	if err != nil {
-		metrics.Notes = append(metrics.Notes, "Modified date failed to parse, setting time to now")
+		metrics.AddNote("[%s]: Modified date failed to parse, setting time to now", cve.Metadata.CVEID)
 		modified = time.Now()
 	}
 	v.Modified = modified
@@ -166,7 +178,7 @@ func ConvertAndExportCVEToOSV(cve cves.CVE5, directory string) error {
 	cveID := cve.Metadata.CVEID
 	cnaAssigner := cve.Metadata.AssignerShortName
 	references := identifyPossibleURLs(cve)
-	metrics := &ConversionMetrics{}
+	metrics := &ConversionMetrics{CVEID: cveID, CNA: cnaAssigner}
 	// Create a base OSV record from the CVE.
 	v := FromCVE5(cve, references, metrics)
 
@@ -175,7 +187,9 @@ func ConvertAndExportCVEToOSV(cve cves.CVE5, directory string) error {
 
 	// Try to extract repository URLs from references.
 	repos, repoNotes := cves.ReposFromReferencesCVEList(string(cveID), references, RefTagDenyList)
-	metrics.Notes = append(metrics.Notes, repoNotes...)
+	for _, note := range repoNotes {
+		metrics.AddNote("%s", note)
+	}
 	metrics.Repos = repos
 
 	vulnDir := filepath.Join(directory, cnaAssigner)

vulnfeeds/cmd/cvelist2osv/version_extraction.go

@@ -125,19 +125,19 @@ func AddVersionInfo(cve cves.CVE5, v *vulns.Vulnerability, metrics *ConversionMe
 
 		v.Affected = append(v.Affected, aff)
 		if hasGit {
-			metrics.VersionSources = append(metrics.VersionSources, VersionSourceGit)
+			metrics.AddSource(VersionSourceGit)
 		} else {
-			metrics.VersionSources = append(metrics.VersionSources, VersionSourceAffected)
+			metrics.AddSource(VersionSourceAffected)
 		}
 	}
 
 	// If no versions were found so far, fall back to CPEs.
 	if !gotVersions {
-		metrics.Notes = append(metrics.Notes, "No versions in affected, attempting to extract from CPE")
+		metrics.AddNote("No versions in affected, attempting to extract from CPE")
 		cpeRanges, cpeStrings, err := findCPEVersionRanges(cve)
 		if err == nil && len(cpeRanges) > 0 {
 			gotVersions = true
-			metrics.VersionSources = append(metrics.VersionSources, VersionSourceCPE)
+			metrics.AddSource(VersionSourceCPE)
 			aff := osvschema.Affected{}
 			for _, vr := range cpeRanges {
 				vr.Type = osvschema.RangeEcosystem
@@ -147,19 +147,21 @@ func AddVersionInfo(cve cves.CVE5, v *vulns.Vulnerability, metrics *ConversionMe
 			aff.DatabaseSpecific["CPEs"] = vulns.Unique(cpeStrings)
 			v.Affected = append(v.Affected, aff)
 		} else if err != nil {
-			metrics.Notes = append(metrics.Notes, err.Error())
+			metrics.AddNote("%s", err.Error())
 		}
 	}
 
 	// As a last resort, try extracting versions from the description text.
 	if !gotVersions {
-		metrics.Notes = append(metrics.Notes, "No versions in CPEs so attempting extraction from description")
+		metrics.AddNote("No versions in CPEs so attempting extraction from description")
 		versions, extractNotes := cves.ExtractVersionsFromText(nil, cves.EnglishDescription(cve.Containers.CNA.Descriptions))
-		metrics.Notes = append(metrics.Notes, extractNotes...)
+		for _, note := range extractNotes {
+			metrics.AddNote("%s", note)
+		}
 		if len(versions) > 0 {
 			// NOTE: These versions are not currently saved due to the need for better validation.
-			metrics.VersionSources = append(metrics.VersionSources, VersionSourceDescription)
-			metrics.Notes = append(metrics.Notes, fmt.Sprintf("Extracted versions from description but did not save them: %+v", versions))
+			metrics.AddSource(VersionSourceDescription)
+			metrics.AddNote("Extracted versions from description but did not save them: %+v", versions)
 		}
 	}
 }
@@ -225,7 +227,7 @@ func extractVersionsFromAffectedField(affected cves.Affected, cnaAssigner string
 // It sorts the introduced and fixed versions to create chronological ranges.
 func findInverseAffectedRanges(cveAff cves.Affected, cnaAssigner string, metrics *ConversionMetrics) (ranges []osvschema.Range, versType VersionRangeType) {
 	if cnaAssigner != "Linux" {
-		metrics.Notes = append(metrics.Notes, "Currently only supporting Linux inverse logic")
+		metrics.AddNote("Currently only supporting Linux inverse logic")
 		return nil, VersionRangeTypeUnknown
 	}
 	var introduced []string
@@ -240,7 +242,7 @@ func findInverseAffectedRanges(cveAff cves.Affected, cnaAssigner string, metrics
 			case 3:
 				introduced = append(introduced, versionValue)
 			default:
-				metrics.Notes = append(metrics.Notes, "Bad non-semver version given: "+versionValue)
+				metrics.AddNote("Bad non-semver version given: %s", versionValue)
 				continue
 			}
 		}
@@ -275,15 +277,15 @@ func findInverseAffectedRanges(cveAff cves.Affected, cnaAssigner string, metrics
 	for index, f := range fixed {
 		if index < len(introduced) {
 			ranges = append(ranges, buildVersionRange(introduced[index], "", f))
-			metrics.Notes = append(metrics.Notes, "Introduced from version value - "+introduced[index])
-			metrics.Notes = append(metrics.Notes, "Fixed from version value - "+f)
+			metrics.AddNote("Introduced from version value - %s", introduced[index])
+			metrics.AddNote("Fixed from version value - %s", f)
 		}
 	}
 
 	if len(ranges) != 0 {
 		return ranges, VersionRangeTypeSemver
 	}
-	metrics.Notes = append(metrics.Notes, "no ranges found")
+	metrics.AddNote("no ranges found")
 
 	return nil, VersionRangeTypeUnknown
 }
@@ -304,30 +306,30 @@ func findNormalAffectedRanges(affected cves.Affected, metrics *ConversionMetrics
 		// Quality check the version strings to avoid using filler content.
 		vQuality := vulns.CheckQuality(vers.Version)
 		if !vQuality.AtLeast(acceptableQuality) {
-			metrics.Notes = append(metrics.Notes, fmt.Sprintf("Version value for %s %s is filler or empty", affected.Vendor, affected.Product))
+			metrics.AddNote("Version value for %s %s is filler or empty", affected.Vendor, affected.Product)
 		}
 		vLessThanQual := vulns.CheckQuality(vers.LessThan)
 		vLTOEQual := vulns.CheckQuality(vers.LessThanOrEqual)
 
 		hasRange := vLessThanQual.AtLeast(acceptableQuality) || vLTOEQual.AtLeast(acceptableQuality)
-		metrics.Notes = append(metrics.Notes, fmt.Sprintf("Range detected: %v", hasRange))
+		metrics.AddNote("Range detected: %v", hasRange)
 		// Handle cases where 'lessThan' is mistakenly the same as 'version'.
 		if vers.LessThan != "" && vers.LessThan == vers.Version {
-			metrics.Notes = append(metrics.Notes, fmt.Sprintf("Warning: lessThan (%s) is the same as introduced (%s)\n", vers.LessThan, vers.Version))
+			metrics.AddNote("Warning: lessThan (%s) is the same as introduced (%s)\n", vers.LessThan, vers.Version)
 			hasRange = false
 		}
 
 		if hasRange {
 			if vQuality.AtLeast(acceptableQuality) {
 				introduced = vers.Version
-				metrics.Notes = append(metrics.Notes, fmt.Sprintf("%s - Introduced from version value - %s", vQuality.String(), vers.Version))
+				metrics.AddNote("%s - Introduced from version value - %s", vQuality.String(), vers.Version)
 			}
 			if vLessThanQual.AtLeast(acceptableQuality) {
 				fixed = vers.LessThan
-				metrics.Notes = append(metrics.Notes, fmt.Sprintf("%s - Fixed from LessThan value - %s", vLessThanQual.String(), vers.LessThan))
+				metrics.AddNote("%s - Fixed from LessThan value - %s", vLessThanQual.String(), vers.LessThan)
 			} else if vLTOEQual.AtLeast(acceptableQuality) {
 				lastaffected = vers.LessThanOrEqual
-				metrics.Notes = append(metrics.Notes, fmt.Sprintf("%s - LastAffected from LessThanOrEqual value- %s", vLTOEQual.String(), vers.LessThanOrEqual))
+				metrics.AddNote("%s - LastAffected from LessThanOrEqual value- %s", vLTOEQual.String(), vers.LessThanOrEqual)
 			}
 
 			if introduced != "" && fixed != "" {
@@ -342,7 +344,7 @@ func findNormalAffectedRanges(affected cves.Affected, metrics *ConversionMetrics
 		// In this case only vers.Version exists which either means that it is _only_ that version that is
 		// affected, but more likely, it affects up to that version. It could also mean that the range is given
 		// in one line instead - like "< 1.5.3" or "< 2.45.4, >= 2.0 " or just "before 1.4.7", so check for that.
-		metrics.Notes = append(metrics.Notes, "Only version exists")
+		metrics.AddNote("Only version exists")
 		// GitHub often encodes the range directly in the version string.
 
 		av, err := git.ParseVersionRange(vers.Version)
@@ -365,12 +367,14 @@ func findNormalAffectedRanges(affected cves.Affected, metrics *ConversionMetrics
 		}
 
 		// Try to extract versions from text like "before 1.4.7".
-		possibleVersions, note := cves.ExtractVersionsFromText(nil, vers.Version)
-		if note != nil {
-			metrics.Notes = append(metrics.Notes, note...)
+		possibleVersions, notes := cves.ExtractVersionsFromText(nil, vers.Version)
+
+		for _, note := range notes {
+			metrics.AddNote("%s", note)
 		}
+
 		if possibleVersions != nil {
-			metrics.Notes = append(metrics.Notes, "Versions retrieved from text but not used CURRENTLY")
+			metrics.AddNote("Versions retrieved from text but not used CURRENTLY")
 			continue
 		}