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

fix(vulnfeeds): validate the entire URL, not the repo part #2836

Merged

Conversation

andrewpollock
Copy link
Contributor

@andrewpollock andrewpollock commented Nov 6, 2024

This commit adjusts link validation, so that it validates the entire link before separating it into a repo and commit. Previously, just the repo URL was validated. This was allowing GitHub commit URLs that were 404'ing to escape detection, which was not the intention.

This was discovered while investigating analysis failures for records that had valid-looking GitHub commit ranges.

This commit adjusts link validation, so that it validates the *entire*
link *before* separating it into a repo and commit. Previously, the repo
just URL was validated. This was allowing GitHub commit URLs that were
404'ing to escape detection, which was not the intention.

This was discovered while investigating analysis failures for records
that had valid-looking GitHub commit ranges.
andrewpollock and others added 2 commits November 7, 2024 05:24
This commit fixes `cves.Commit()` so it still correctly supports
canoncalized git.kernel.org URLs. The canoncalization removes the
`/cgit` hint from the path.

Also add the start of some test coverage for `cves.Commit()`
@andrewpollock andrewpollock enabled auto-merge (squash) November 7, 2024 05:30
@andrewpollock andrewpollock merged commit 971b0f7 into google:master Nov 7, 2024
11 checks passed
andrewpollock added a commit to andrewpollock/osv.dev that referenced this pull request Nov 8, 2024
This commit addresses the performance regression introduced in google#2836 by
only making a remote network request for URLs that have passed initial
muster as being appropriately shaped.
andrewpollock added a commit that referenced this pull request Nov 8, 2024
This commit addresses the performance regression introduced in #2836 by
only making a remote network request for URLs that have passed initial
muster as being appropriately shaped.

(#2836 causes a significant blowout in runtime of a conversion run due
to the aggregate latency of all the remote link checking)
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