Make sure vulnerability id is_cve or is_vulcoid #389
Conversation
This makes sure that vulnerability id supplied in alpine_linux importer is either a cve, vulcoid or empty so as to stand on the definition of vulnerability id. It could be possible to introduce a validator at the model level for the same as well using these functions Signed-off-by: Hritik Vijay <hritikxx8@gmail.com>
|
@Hritik14 detecting these when we are at last step, ie during model creation won't be as effective as detection on creation of I would rather add the validation to the |
vulnerabilities/data_source.py
Outdated
| @staticmethod | ||
| def is_cve(id: str): | ||
| c = id.split("-") | ||
| if len(c) == 3 and c[0].lower() == "cve" and c[1].isdigit() and c[2].isdigit(): | ||
| return True | ||
| return False | ||
|
|
||
| @staticmethod | ||
| def is_vulcoid(id: str): | ||
| c = id.split("-") | ||
| if ( | ||
| len(c) == 4 | ||
| and c[0].lower() == "vulcoid" | ||
| and c[1].isdigit() | ||
| and c[2].isdigit() | ||
| and c[3].isdigit() | ||
| ): | ||
| return True | ||
| return False | ||
|
|
There was a problem hiding this comment.
These are not required. is-cve is duplicated we already a method for doing that validation at https://github.com/nexB/vulnerablecode/blob/3d66b4e82ee31422ab907d3388f739b768ffd2ac/vulnerabilities/helpers.py#L81
is_vulcoid is not needed since vulcoids are not assigned manually. The advisories without vulnerability id are assigned vulcoid, see https://github.com/nexB/vulnerablecode/blob/3d66b4e82ee31422ab907d3388f739b768ffd2ac/vulnerabilities/models.py#L65
| references=references, | ||
| vulnerability_id=vuln_ids[0] if vuln_ids[0] != "CVE-????-?????" else "", | ||
| vulnerability_id=vuln_ids[0] | ||
| if self.is_cve(vuln_ids[0]) or self.is_vulcoid(vuln_ids[0]) |
There was a problem hiding this comment.
As said above, validate for only cve using the helper
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com>
|
I've applied the requested changes. Please review. Also, I think it would be better to squash and merge the PR to avoid intermediate commits. |
|
LGTM ! could you squash and force push the commits from your side ? |
|
I would do it but it would just mess up the git history (I'm not really in favor of --force). If you could squash like it's shown here I guess it would be lot easier https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/merging-a-pull-request |
2 participants
This makes sure that vulnerability id supplied in alpine_linux importer
is either a cve, vulcoid or empty so as to stand on the definition of
vulnerability id.
It could be possible to introduce a validator at the model level for the
same as well using these functions.
I would propose that we create a validators class where we would keep some custom validators which could be used inside the models to keep the database protected against invalid input. One such validator for
vulnerability idwould look like the followingSigned-off-by: Hritik Vijay hritikxx8@gmail.com