From 93f6ce703d5fd172a9de4e028c172970b0e8d1a0 Mon Sep 17 00:00:00 2001 From: Jess Lowe Date: Mon, 29 Sep 2025 00:28:20 +0000 Subject: [PATCH 1/5] Refactor commit extraction for better reusability --- vulnfeeds/cves/versions.go | 43 ++++++++++++++++++--------------- vulnfeeds/cves/versions_test.go | 2 +- vulnfeeds/git/versions.go | 16 +----------- vulnfeeds/models/types.go | 14 +++++++++++ 4 files changed, 39 insertions(+), 36 deletions(-) diff --git a/vulnfeeds/cves/versions.go b/vulnfeeds/cves/versions.go index a69733e7547..5b38eef6bcb 100644 --- a/vulnfeeds/cves/versions.go +++ b/vulnfeeds/cves/versions.go @@ -515,44 +515,47 @@ func ValidateAndCanonicalizeLink(link string, httpClient *http.Client) (canonica } // For URLs referencing commits in supported Git repository hosts, return a cloneable AffectedCommit. -func ExtractGitCommit(link string, commitType models.CommitType, httpClient *http.Client) (ac models.AffectedCommit, err error) { - r, err := Repo(link) +func ExtractGitAffectedCommit(link string, commitType models.CommitType, httpClient *http.Client) (ac models.AffectedCommit, err error) { + c, r, err := ExtractGitCommit(link, httpClient) if err != nil { return ac, err } + ac.SetRepo(r) + + models.SetCommitByType(&ac, commitType, c) + + return ac, nil +} + +func ExtractGitCommit(link string, httpClient *http.Client) (string, string, error) { + var commit string + r, err := Repo(link) + if err != nil { + return "", "", err + } + c, err := Commit(link) if err != nil { - return ac, err + return "", "", err } + commit = c + // If URL doesn't validate, treat it as linkrot. possiblyDifferentLink, err := ValidateAndCanonicalizeLink(link, httpClient) if err != nil { - return ac, err + return "", "", err } // restart the entire extraction process when the URL changes (i.e. handle a // redirect to a completely different host, instead of a redirect within // GitHub) if possiblyDifferentLink != link { - return ExtractGitCommit(possiblyDifferentLink, commitType, httpClient) + return ExtractGitCommit(possiblyDifferentLink, httpClient) } - ac.SetRepo(r) - - switch commitType { - case models.Introduced: - ac.SetIntroduced(c) - case models.LastAffected: - ac.SetLastAffected(c) - case models.Limit: - ac.SetLimit(c) - case models.Fixed: - ac.SetFixed(c) - } - - return ac, nil + return commit, r, nil } func HasVersion(validVersions []string, version string) bool { @@ -676,7 +679,7 @@ func deduplicateAffectedCommits(commits []models.AffectedCommit) []models.Affect func ExtractVersionInfo(cve CVE, validVersions []string, httpClient *http.Client) (v models.VersionInfo, notes []string) { for _, reference := range cve.References { // (Potentially faulty) Assumption: All viable Git commit reference links are fix commits. - if commit, err := ExtractGitCommit(reference.URL, models.Fixed, httpClient); err == nil { + if commit, err := ExtractGitAffectedCommit(reference.URL, models.Fixed, httpClient); err == nil { v.AffectedCommits = append(v.AffectedCommits, commit) } } diff --git a/vulnfeeds/cves/versions_test.go b/vulnfeeds/cves/versions_test.go index 0890ba81149..bf52530f7e2 100644 --- a/vulnfeeds/cves/versions_test.go +++ b/vulnfeeds/cves/versions_test.go @@ -691,7 +691,7 @@ func TestExtractGitCommit(t *testing.T) { if !tc.disableExpiryDate.IsZero() && time.Now().After(tc.disableExpiryDate) { t.Logf("test %q: extractGitCommit(%q, %q) has been enabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) } - got, err := ExtractGitCommit(tc.inputLink, tc.inputCommitType, client) + got, err := ExtractGitAffectedCommit(tc.inputLink, tc.inputCommitType, client) if err != nil && !tc.expectFailure { t.Errorf("test %q: extractGitCommit for %q (%q) errored unexpectedly: %#v", tc.description, tc.inputLink, tc.inputCommitType, err) } diff --git a/vulnfeeds/git/versions.go b/vulnfeeds/git/versions.go index ccee3c8e8f3..bcc3404dc0d 100644 --- a/vulnfeeds/git/versions.go +++ b/vulnfeeds/git/versions.go @@ -25,20 +25,6 @@ import ( var versionRangeRegex = regexp.MustCompile(`^(>=|<=|~|\^|>|<|=)\s*([0-9a-zA-Z\.\-]+)(?:,\s*(>=|<=|~|\^|>|<|=)\s*([0-9a-zA-Z\.\-]+))?$`) // Used to parse version strings from the GitHub CNA. -// setCommitByType sets the appropriate commit field on an AffectedCommit based on the CommitType. -func setCommitByType(ac *models.AffectedCommit, commitType models.CommitType, commitHash string) { - switch commitType { - case models.Introduced: - ac.SetIntroduced(commitHash) - case models.LastAffected: - ac.SetLastAffected(commitHash) - case models.Limit: - ac.SetLimit(commitHash) - case models.Fixed: - ac.SetFixed(commitHash) - } -} - // findFuzzyCommit takes an already normalized version and the mapping of repo tags to // normalized tags and commits, and performs fuzzy matching to find a commit hash. func findFuzzyCommit(normalizedVersion string, normalizedTags map[string]NormalizedTag) (string, bool) { @@ -87,7 +73,7 @@ func VersionToAffectedCommit(version string, repo string, commitType models.Comm return ac, err } ac.SetRepo(repo) - setCommitByType(&ac, commitType, commitHash) + models.SetCommitByType(&ac, commitType, commitHash) return ac, nil } diff --git a/vulnfeeds/models/types.go b/vulnfeeds/models/types.go index 26e8880450a..b48c3eefab8 100644 --- a/vulnfeeds/models/types.go +++ b/vulnfeeds/models/types.go @@ -15,6 +15,20 @@ type AffectedCommit struct { LastAffected string `json:"last_affected,omitempty" yaml:"last_affected,omitempty"` } +// SetCommitByType sets the appropriate commit field on an AffectedCommit based on the CommitType. +func SetCommitByType(ac *AffectedCommit, commitType CommitType, commitHash string) { + switch commitType { + case Introduced: + ac.SetIntroduced(commitHash) + case LastAffected: + ac.SetLastAffected(commitHash) + case Limit: + ac.SetLimit(commitHash) + case Fixed: + ac.SetFixed(commitHash) + } +} + func (ac *AffectedCommit) SetRepo(repo string) { // GitHub.com repos are demonstrably case-insensitive, and frequently // expressed in URLs with varying cases, so normalize them to lowercase. From b8fca94394af56144c8df3208c6c6fcfd8993c93 Mon Sep 17 00:00:00 2001 From: Jess Lowe Date: Mon, 29 Sep 2025 02:08:01 +0000 Subject: [PATCH 2/5] Address comments - avoid infinite loop --- vulnfeeds/cves/versions.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/vulnfeeds/cves/versions.go b/vulnfeeds/cves/versions.go index 5b38eef6bcb..69841865f70 100644 --- a/vulnfeeds/cves/versions.go +++ b/vulnfeeds/cves/versions.go @@ -515,8 +515,9 @@ func ValidateAndCanonicalizeLink(link string, httpClient *http.Client) (canonica } // For URLs referencing commits in supported Git repository hosts, return a cloneable AffectedCommit. -func ExtractGitAffectedCommit(link string, commitType models.CommitType, httpClient *http.Client) (ac models.AffectedCommit, err error) { - c, r, err := ExtractGitCommit(link, httpClient) +func ExtractGitAffectedCommit(link string, commitType models.CommitType, httpClient *http.Client) (models.AffectedCommit, error) { + var ac models.AffectedCommit + c, r, err := ExtractGitCommit(link, httpClient, 0) if err != nil { return ac, err } @@ -528,7 +529,10 @@ func ExtractGitAffectedCommit(link string, commitType models.CommitType, httpCli return ac, nil } -func ExtractGitCommit(link string, httpClient *http.Client) (string, string, error) { +func ExtractGitCommit(link string, httpClient *http.Client, depth int) (string, string, error) { + if depth > 10 { + return "", "", fmt.Errorf("max recursion depth exceeded for %s", link) + } var commit string r, err := Repo(link) if err != nil { @@ -552,7 +556,7 @@ func ExtractGitCommit(link string, httpClient *http.Client) (string, string, err // redirect to a completely different host, instead of a redirect within // GitHub) if possiblyDifferentLink != link { - return ExtractGitCommit(possiblyDifferentLink, httpClient) + return ExtractGitCommit(possiblyDifferentLink, httpClient, depth+1) } return commit, r, nil From a1669164c79b3eff25095112d8e2db9162473303 Mon Sep 17 00:00:00 2001 From: Jess Lowe Date: Mon, 29 Sep 2025 02:08:45 +0000 Subject: [PATCH 3/5] fix test name --- vulnfeeds/cves/versions_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vulnfeeds/cves/versions_test.go b/vulnfeeds/cves/versions_test.go index bf52530f7e2..c0c2c7fba62 100644 --- a/vulnfeeds/cves/versions_test.go +++ b/vulnfeeds/cves/versions_test.go @@ -686,10 +686,10 @@ func TestExtractGitCommit(t *testing.T) { t.Skipf("test %q: running on Cloud Build", tc.description) } if time.Now().Before(tc.disableExpiryDate) { - t.Skipf("test %q: extractGitCommit for %q (%q) has been skipped due to known outage and will be reenabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) + t.Skipf("test %q: ExtractGitAffectedCommit for %q (%q) has been skipped due to known outage and will be reenabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) } if !tc.disableExpiryDate.IsZero() && time.Now().After(tc.disableExpiryDate) { - t.Logf("test %q: extractGitCommit(%q, %q) has been enabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) + t.Logf("test %q: ExtractGitAffectedCommit(%q, %q) has been enabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) } got, err := ExtractGitAffectedCommit(tc.inputLink, tc.inputCommitType, client) if err != nil && !tc.expectFailure { From 13c6bdacb8ce77d537dc0420d1946567fa193bdb Mon Sep 17 00:00:00 2001 From: Jess Lowe Date: Mon, 29 Sep 2025 02:09:59 +0000 Subject: [PATCH 4/5] make extract private --- vulnfeeds/cves/versions.go | 4 ++-- vulnfeeds/cves/versions_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vulnfeeds/cves/versions.go b/vulnfeeds/cves/versions.go index 69841865f70..911bc366308 100644 --- a/vulnfeeds/cves/versions.go +++ b/vulnfeeds/cves/versions.go @@ -515,7 +515,7 @@ func ValidateAndCanonicalizeLink(link string, httpClient *http.Client) (canonica } // For URLs referencing commits in supported Git repository hosts, return a cloneable AffectedCommit. -func ExtractGitAffectedCommit(link string, commitType models.CommitType, httpClient *http.Client) (models.AffectedCommit, error) { +func extractGitAffectedCommit(link string, commitType models.CommitType, httpClient *http.Client) (models.AffectedCommit, error) { var ac models.AffectedCommit c, r, err := ExtractGitCommit(link, httpClient, 0) if err != nil { @@ -683,7 +683,7 @@ func deduplicateAffectedCommits(commits []models.AffectedCommit) []models.Affect func ExtractVersionInfo(cve CVE, validVersions []string, httpClient *http.Client) (v models.VersionInfo, notes []string) { for _, reference := range cve.References { // (Potentially faulty) Assumption: All viable Git commit reference links are fix commits. - if commit, err := ExtractGitAffectedCommit(reference.URL, models.Fixed, httpClient); err == nil { + if commit, err := extractGitAffectedCommit(reference.URL, models.Fixed, httpClient); err == nil { v.AffectedCommits = append(v.AffectedCommits, commit) } } diff --git a/vulnfeeds/cves/versions_test.go b/vulnfeeds/cves/versions_test.go index c0c2c7fba62..f5def7c576d 100644 --- a/vulnfeeds/cves/versions_test.go +++ b/vulnfeeds/cves/versions_test.go @@ -691,7 +691,7 @@ func TestExtractGitCommit(t *testing.T) { if !tc.disableExpiryDate.IsZero() && time.Now().After(tc.disableExpiryDate) { t.Logf("test %q: ExtractGitAffectedCommit(%q, %q) has been enabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) } - got, err := ExtractGitAffectedCommit(tc.inputLink, tc.inputCommitType, client) + got, err := extractGitAffectedCommit(tc.inputLink, tc.inputCommitType, client) if err != nil && !tc.expectFailure { t.Errorf("test %q: extractGitCommit for %q (%q) errored unexpectedly: %#v", tc.description, tc.inputLink, tc.inputCommitType, err) } From 8e627312c947abd6174a58fcd126fb0886e05f44 Mon Sep 17 00:00:00 2001 From: Jess Lowe Date: Mon, 29 Sep 2025 03:00:37 +0000 Subject: [PATCH 5/5] fix names of tests --- vulnfeeds/cves/versions_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/vulnfeeds/cves/versions_test.go b/vulnfeeds/cves/versions_test.go index f5def7c576d..ec7d3f87ef4 100644 --- a/vulnfeeds/cves/versions_test.go +++ b/vulnfeeds/cves/versions_test.go @@ -686,20 +686,20 @@ func TestExtractGitCommit(t *testing.T) { t.Skipf("test %q: running on Cloud Build", tc.description) } if time.Now().Before(tc.disableExpiryDate) { - t.Skipf("test %q: ExtractGitAffectedCommit for %q (%q) has been skipped due to known outage and will be reenabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) + t.Skipf("test %q: extractGitAffectedCommit for %q (%q) has been skipped due to known outage and will be reenabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) } if !tc.disableExpiryDate.IsZero() && time.Now().After(tc.disableExpiryDate) { - t.Logf("test %q: ExtractGitAffectedCommit(%q, %q) has been enabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) + t.Logf("test %q: extractGitAffectedCommit(%q, %q) has been enabled on %s.", tc.description, tc.inputLink, tc.inputCommitType, tc.disableExpiryDate) } got, err := extractGitAffectedCommit(tc.inputLink, tc.inputCommitType, client) if err != nil && !tc.expectFailure { - t.Errorf("test %q: extractGitCommit for %q (%q) errored unexpectedly: %#v", tc.description, tc.inputLink, tc.inputCommitType, err) + t.Errorf("test %q: extractGitAffectedCommit for %q (%q) errored unexpectedly: %#v", tc.description, tc.inputLink, tc.inputCommitType, err) } if err == nil && tc.expectFailure { - t.Errorf("test %q: extractGitCommit for %q (%q) did not error as unexpected!", tc.description, tc.inputLink, tc.inputCommitType) + t.Errorf("test %q: extractGitAffectedCommit for %q (%q) did not error as unexpected!", tc.description, tc.inputLink, tc.inputCommitType) } if !reflect.DeepEqual(got, tc.expectedAffectedCommit) { - t.Errorf("test %q: extractGitCommit for %q was incorrect, got: %#v, expected: %#v", tc.description, tc.inputLink, got, tc.expectedAffectedCommit) + t.Errorf("test %q: extractGitAffectedCommit for %q was incorrect, got: %#v, expected: %#v", tc.description, tc.inputLink, got, tc.expectedAffectedCommit) } }) }