-
Notifications
You must be signed in to change notification settings - Fork 203
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
Refactor severity score model and fix incorrect suse scores #1636
Conversation
193628e
to
84a3a93
Compare
- Add 'severities' field to AffectedByPackageRelatedVulnerability. - Add 'severities' field to Vulnerability. - Add 'url' field to VulnerabilitySeverity. - Data migration to remove corrupted SUSE scores. - Data migration to populate new VulnerabilitySeverity url field using reference. - Data migration to populate Vulnerability 'severities' M2M relationship. - Delete VulnerabilitySeverity reference field. Signed-off-by: Keshav Priyadarshi <git@keshav.space>
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
- Prefetch related vulnerability, severities, references, and exploits for better performance Signed-off-by: Keshav Priyadarshi <git@keshav.space>
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
84a3a93
to
4107451
Compare
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
model = VulnerabilitySeverity | ||
fields = ["url", "value", "scoring_system", "scoring_elements", "published_at"] | ||
|
||
def to_representation(self, instance): |
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 we need to_representation
method here? if published_at is None, let it be sent as None
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.
Added this just to preserve the response structure we're already using in APIv2 https://github.com/aboutcode-org/vulnerablecode/blob/8a68c97dfa369ad048de3ece14cc1b3cf40591cc/vulnerabilities/api_v2.py#L16C33-L16C64.
IMO yes we should simpy send None
when we don't have published date.
vulnerabilities/migrations/0078_alter_vulnerabilityseverity_options_and_more.py
Show resolved
Hide resolved
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.
LGTM!
@@ -214,10 +220,21 @@ def to_representation(self, instance): | |||
|
|||
return data | |||
|
|||
def get_references(self, vulnerability): |
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.
Minor nit: Why we are having a method here?
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 our old API, references
used to contain the nested severity
, as severity and reference were related through a foreignkey relationship. In our new model we have dissociated reference and severity. To ensure we maintain compatibility for existing users of old API we're manually crafting the references to include the relevant severity.
Signed-off-by: Keshav Priyadarshi <git@keshav.space>
severities
field to both AffectedByPackageRelatedVulnerability and Vulnerability.url
field to VulnerabilitySeverity and removereference
relationship.url
field in VulnerabilitySeverity usingreferences
, set the severity-vulnerablity relationship from the old model.Resolves #1597