-
-
Notifications
You must be signed in to change notification settings - Fork 238
Collect existing fix commits for project-kb #1987
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: ziad hany <[email protected]>
Add a test for the ProjectKB importer and collect fix commits pipeline. Signed-off-by: ziad hany <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ziadhany, see some suggestions.
if not commit_id or not repo_url: | ||
continue | ||
|
||
commit_url = repo_url.replace(".git", "") + "/commit/" + commit_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This path is only valid for GitHub repos, are we sure we only have GitHub repos in project kb advisory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, Project KB Advisory is just one GitHub repository.
advisories = AdvisoryV2.objects.filter(advisory_id__in=vuln_ids).prefetch_related( | ||
"impacted_packages__affecting_packages" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not want to merge the advisory info coming from different source.
for impact in advisory.impacted_packages.all(): | ||
for pkg in impact.affecting_packages.all(): | ||
codefixes.append( | ||
CodeFixV2( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO we should treat this as an advisory and update impact_package model to hold the fixed and affecting commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main issue is how to relate a fix commit to an impacted package.
A large portion of existing fix commit databases only provide the CVE-XXXX, the Git commit, and the repository.
IMHO, we should have an advisory, but the code fix should be considered as a reference URL, with an optional relation to the impacted packages. Since we don't know which version or package (purl) is going to be impacted by this commit.
Uh oh!
There was an error while loading. Please reload this page.