Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further validate repos by only accepting ones with tags #2233

Merged
merged 3 commits into from
May 24, 2024

Conversation

andrewpollock
Copy link
Contributor

@andrewpollock andrewpollock commented May 23, 2024

Tags are necessary for version resolution, and a repo without them is useless to us, and many of the repos in the current denylist do not have any tags.

This enables a radical simplification of the repo denylist and largely removes ongoing maintenance burden.

Latest run in Production:

nvdcve-2.0-2024.json Metrics: {TotalCVEs:11389 CVEsForApplications:1581 CVEsForKnownRepos:2364 OSVRecordsGenerated:1093 Outcomes:map[]}

Local test run:

nvdcve-2.0-2024.json Metrics: {TotalCVEs:11511 CVEsForApplications:1581 CVEsForKnownRepos:1651 OSVRecordsGenerated:1047 Outcomes:map[]}

A fabulous improvement in CVEsForKnownRepos, a much (durably) firmer looking denominator for conversion metrics.

Tags are necessary for version resolution, and a repo without them is
useless to us, and many of the repos in the current denylist do not have
any tags.
The new and improved git.ValidRepo() reduces the maintenance burden of
the denylist, as most of the repositories on it now fail validation.
Copy link
Collaborator

@oliverchang oliverchang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@andrewpollock andrewpollock merged commit 28d1e63 into google:master May 24, 2024
11 checks passed
andrewpollock added a commit to andrewpollock/osv.dev that referenced this pull request May 27, 2024
present in the VendorProductToRepoMap (intentionally).

This also means that CVEs for repos that lack tags but otherwise have a
commit hash in the CVE's references now get excluded from
consideration from conversion.

CVE-2023-0327 before google#2233 had an entry in the VendorProductToRepoMap
for `theradsystem_project:theradsystem`,
even though the repo wasn't usable. Afterwards, it does not:

```
[CVE-2023-0327]: Failed to derive any repos for "theradsystem_project" "theradsystem"
[CVE-2023-0327]: Summary: [CPEs=1 AppCPEs=1 DerivedRepos=0]
[CVE-2023-0327]: Passing due to lack of viable repository
```

Add test coverage for `ReposFromReferences()` and fix a couple of bugs
that it flushed out for when there is no VendorProductToRepoMap cache in
use.

Remove some unnecessary code stubs that aren't going to get filled out
(the approach to determining CVE scope changed to trying them all,
regardless of language)
andrewpollock added a commit that referenced this pull request May 28, 2024
#2233 introduced a small regression, in that repositories for CPEs that
have no usable tags are no longer present in the VendorProductToRepoMap
(intentionally).

This also means that CVEs for these repos that lack tags but otherwise
have a commit hash in the CVE's references now get excluded from
consideration from conversion.

CVE-2023-0327 before #2233 had an entry in the VendorProductToRepoMap
for `theradsystem_project:theradsystem`,
even though the repo wasn't usable. Afterwards, it does not, resulting
in:

```
[CVE-2023-0327]: Failed to derive any repos for "theradsystem_project" "theradsystem"
[CVE-2023-0327]: Summary: [CPEs=1 AppCPEs=1 DerivedRepos=0]
[CVE-2023-0327]: Passing due to lack of viable repository
```

because of the added rigour of `cves.ValidRepo()`


https://github.com/google/osv.dev/blob/7ab7d213d4daba3f0c65954b06c035aad5feb498/vulnfeeds/cmd/nvd-cve-osv/main.go#L248-L250

Add a less rigourous variation of repo validation that merely validates
repo _existence_ for this last resort conversion attempt.

Add test coverage for `ReposFromReferences()` and fix a couple of bugs
that it flushed out for when there is no VendorProductToRepoMap cache in
use.

Remove some unnecessary code stubs that aren't going to get filled out
(the approach to determining CVE scope changed to trying them all,
regardless of language)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants