[Experiment] Store and evaluate package version ranges - #270
Conversation
- Add a table VulnerablePackageVersionRange - This table stores version less purl string and a FK of vulnerability - Added manager method which on given a purl string which does not exist in the db tries to find whether it satisfies a version range, if yes then that vulnerability is returned Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>
pombredanne
left a comment
pombredanne
left a comment
There was a problem hiding this comment.
Thanks! I reckon this still an experiment and a WIP, but getting some more tests here would be essential. I think we will need this for sure.
I added some other comments for your consideration.
| @dataclasses.dataclass | ||
| class VulnPkgVersionRange: | ||
|
|
||
| # This dataclass is nested inside the `Advisory` class below. It is used |
There was a problem hiding this comment.
Should this be a docstring instead of a comment?
| # dataclass. | ||
|
|
||
| purl: str | ||
| version_ranges: str |
There was a problem hiding this comment.
I called this a requirement in scancode https://github.com/nexB/scancode-toolkit/blob/ba4bbf216c6f44572662d16c76214a08b0a69e7e/src/packagedcode/models.py#L331
I wonder if we should rename it in scancode or here.... but in anycase I would like to keep in a single common name for the same concept.
| raise TypeError | ||
|
|
||
| @dataclasses.dataclass | ||
| class VulnPkgVersionRange: |
There was a problem hiding this comment.
Could we find a better name?
PackageSpecification may be? what else could be a good one?
| impacted_package_urls: Iterable[PackageURL] | ||
| resolved_package_urls: Iterable[PackageURL] = dataclasses.field(default_factory=list) | ||
| resolved_package_urls: Iterable[PackageURL] | ||
| vuln_pkg_version_ranges : Iterable[VulnPkgVersionRange] |
There was a problem hiding this comment.
I reckon you would want that vuln_pkg_version_ranges be optional? and replace the the impacted_package_urls ?
Or would we keep both?
Could you have both at the same time?
And if we have bot would these be rather names something like impacted_package_urls_specification , or ``impacted_package_urls_requirementsorimpacted_package_urls_version ranges` or ...?`
| 'data_source_cfg': { | ||
| 'endpoint': 'https://api.github.com/graphql', | ||
| 'ecosystems': ['MAVEN', 'NUGET', 'COMPOSER'] | ||
| 'ecosystems': ['MAVEN', 'NUGET'] |
There was a problem hiding this comment.
Why dropping composer from here?
There was a problem hiding this comment.
And actually why even filtering based on "ecosystems" ? Why not get them all?
|
@pombredanne Thanks for the review. What kind of cases should be covered by these tests ? Re. naming no strong opinions here but IMHO |
I am fine with |
2 participants
tries to find whether it satisfies a version range, if yes then that vulnerability is returned
Signed-off-by: Shivam Sandbhor shivam.sandbhor@gmail.com