Skip to content
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

Add missing OSV schema fields to vulnerability page #2132

Conversation

zahraaalizadeh
Copy link
Collaborator

@zahraaalizadeh zahraaalizadeh commented Apr 23, 2024

This change adds the following info to the vulnerability page:

resolves #2081

Regarding the second items on the issue: per-range ecosystem/database_specific fields I left a question on the issue.

@zahraaalizadeh zahraaalizadeh force-pushed the add-missing-osv-schema-fields-to-vulnerability-page branch 2 times, most recently from 7eeb0bd to 22f43d3 Compare May 9, 2024 10:25
@@ -163,57 +200,57 @@ <h3 class="mdc-layout-grid__cell--span-3">
</dd>
</dl>
{% endfor -%}
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also have per-range ecosystem/database_specific: https://ossf.github.io/osv-schema/#affectedecosystem_specific-field

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, in total there's 3 variations here:

  • affected[].ranges[].database_specific
  • affected[].database_specific field
  • affected[].ecosystem_specific field

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a commit to expose it: 47f2185

Also looking at the data models, I reckon currently this data is not recorded on the Bug model:

osv.dev/osv/models.py

Lines 185 to 194 in 47f2185

class AffectedRange2(ndb.Model):
"""Affected range."""
# Type of range.
type = ndb.StringProperty(validator=_check_valid_range_type)
# Repo URL.
repo_url = ndb.StringProperty()
# Events.
events = ndb.LocalStructuredProperty(AffectedEvent, repeated=True)

(Please correct me if I'm missing something.)

It might be worth mentioning, I couldn't find any bug that include database_specific on their ranges though.

sample output:

Screenshot 2024-05-15 at 12 54 52 PM

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oliverchang - FYI added ecae833 to render the on the fly database_specific data too.

{%- if 'purl' in affected.package -%}
<dt>Purl</dt>
<dd>{{ affected.package.purl }}</dd>
{%- endif -%}
{%- endif -%}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also a per-affected severity field: https://ossf.github.io/osv-schema/#affectedseverity-field

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a commit for it c645c99

You might verify it via: BIT-mediawiki-2020-27621

sample output:

Screenshot 2024-05-15 at 9 18 49 AM

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rendering of this is different from the rendering for the top level severity (e.g. on GHSA-76v2-48w6-crxr). Can we make this the same as the top level rendering?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update in 212c8f1

sample output:
Screenshot 2024-05-17 at 4 02 35 PM

@zahraaalizadeh zahraaalizadeh force-pushed the add-missing-osv-schema-fields-to-vulnerability-page branch from 22f43d3 to 47f2185 Compare May 15, 2024 03:33
{% for credit in vulnerability.credits -%}
<li class="credit">
<ul>
<li>{{ credit.name }}{%- if 'type' in credit -%} - {{ credit.type }}{%- endif -%}</li>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a space after the name and before the first "-" ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated: 3c62c56

pre {
white-space: pre-wrap;
overflow: auto;
}


.purl{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a space here after "purl" ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated 511d314

{%- if 'purl' in affected.package -%}
<dt>Purl</dt>
<dd>{{ affected.package.purl }}</dd>
{%- endif -%}
{%- endif -%}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rendering of this is different from the rendering for the top level severity (e.g. on GHSA-76v2-48w6-crxr). Can we make this the same as the top level rendering?

@oliverchang
Copy link
Collaborator

/gcbrun

Copy link
Collaborator

@oliverchang oliverchang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! just one last comment.

@@ -240,7 +240,7 @@ def vulnerability_redirector(potential_vuln_id):
def bug_to_response(bug, detailed=True):
"""Convert a Bug entity to a response object."""
response = osv.vulnerability_to_dict(
bug.to_vulnerability(include_alias=detailed))
bug.to_vulnerability(include_source=detailed, include_alias=detailed))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to show this actually, as it replicates the existing "Source" field on the vulnerabilities field. Can you please revert?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, Thanks for the heads up! I didn't noticed the duplication. I dropped the commit.

Earlier in the html, there is a duplicated `if` statement
to display the pacjange if it existed in the affected
payload.

This change removes the duplicated `if` statement in favour
of the parent `if` statement.
Priorly, affected database specific was linked to
affected range database specific.

This change updates the link to point to the correct address.
@zahraaalizadeh zahraaalizadeh force-pushed the add-missing-osv-schema-fields-to-vulnerability-page branch from 212c8f1 to db8349b Compare May 20, 2024 23:54
Copy link
Collaborator

@oliverchang oliverchang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but holding off on merging this until we've had this week's release, so we can evaluate this in our test instance before it goes to prod.

@oliverchang oliverchang merged commit 8aa87e6 into google:master May 23, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Surface all OSV schema fields on vulnerability details.
2 participants