-
-
Notifications
You must be signed in to change notification settings - Fork 244
Add support for affected_by_commits, fixed_by_commits, and OSV code fix commits #2017
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
… in Advisory Signed-off-by: ziad hany <ziadhany2016@gmail.com>
2af10cf to
a8ec9f1
Compare
Signed-off-by: ziad hany <ziadhany2016@gmail.com>
Signed-off-by: ziad hany <ziadhany2016@gmail.com>
|
@ziadhany add description in the PR please! |
vulnerabilities/importers/osv.py
Outdated
| except InvalidVersion: | ||
| logger.error(f"Invalid SemverVersion: {version!r} for OSV id: {raw_id!r}") | ||
|
|
||
| if fixed_range_type == "GIT": |
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.
Why this? And what's the get_code_commit function ?
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.
In the OSV schema, a commit be treated as the package version.
https://ossf.github.io/osv-schema/#python-vulnerability
"ranges": [ {
"type": "GIT",
"repo": "https://github.com/owner/repo",
"events": [
{ "introduced": "X" },
{ "fixed": "Y" },
]
} ]
When importing this data into the new model, we currently log it as Unsupported fixed version type.
Instead, we should skip these entries silently to avoid unnecessary log noise, since they are already handled in the get_code_commit function.
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.
Why to skip and not create code commits out of these ?
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.
Because the get_fixed_versions function returns only versions, I thought the name might be misleading.
No problem I’ll update it and rename get_fixed_versions to get_fixed_versions_and_commits
Add all the fields in keys for comparison CodeCommitData Signed-off-by: ziad hany <ziadhany2016@gmail.com>
|
@ziadhany mostly looks good! Please run the importer once and paste the logs here. Thanks! I want to see if we are missing on any data in OSV format. And how does the AdvisoryData and ImpactedPackages looks with the new CommitData. Thanks! |
Signed-off-by: ziad hany <ziadhany2016@gmail.com>
|
@TG1999 This is the log output for the following importers:
the database query result : |
affected_by_commitsandfixed_by_commitsfields in our advisoryfrom_dictandto_dictmethodscompute_checksummethodCodeCommitDataimporter class