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

CVE overhaul #13412

Merged
merged 116 commits into from
Sep 13, 2024
Merged

CVE overhaul #13412

merged 116 commits into from
Sep 13, 2024

Conversation

akbarkz
Copy link
Contributor

@akbarkz akbarkz commented Dec 15, 2023

Done

  • This is a feature branch for CVE Overhaul story

QA

  • Check out this feature branch
  • Run the site using the command ./run serve or dotrun
  • View the site locally in your web browser at: http://0.0.0.0:8001/
    • Be sure to test on mobile, tablet and desktop screen sizes

Issue / Card

Fixes https://warthogs.atlassian.net/browse/WD-162, #11216, #11591, #12056, #12055, #12054, #13024, #13386

Help

QA steps - Commit guidelines

@webteam-app
Copy link

webteam-app commented Dec 15, 2023

Demo starting at https://ubuntu-com-13412.demos.haus/security/cves

Copy link

codecov bot commented Dec 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.49%. Comparing base (f29bbc1) to head (76a55d9).
Report is 160 commits behind head on main.

Current head 76a55d9 differs from pull request most recent head 2756c5a

Please upload reports for the commit 2756c5a to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13412      +/-   ##
==========================================
+ Coverage   74.29%   74.49%   +0.19%     
==========================================
  Files         107      107              
  Lines        2871     2854      -17     
  Branches      964      954      -10     
==========================================
- Hits         2133     2126       -7     
+ Misses        712      704       -8     
+ Partials       26       24       -2     

see 5 files with indirect coverage changes

@mtruj013 mtruj013 force-pushed the cve-overhaul branch 2 times, most recently from 5ffe32a to e60496f Compare June 4, 2024 07:23
@mtruj013 mtruj013 force-pushed the cve-overhaul branch 2 times, most recently from cd8874d to 409418f Compare August 5, 2024 16:26
@mtruj013 mtruj013 force-pushed the cve-overhaul branch 2 times, most recently from 83bf2d3 to c1a59df Compare August 14, 2024 13:35
@juanruitina
Copy link
Contributor

juanruitina commented Aug 30, 2024

@mtruj013 Some final comments after having a comprehensive look at it all.

  • Please change the icons for the Vulnerable statuses from p-icon--warning to p-icon--error in home, search results and about page (it's correct already on the individual CVE page). I agreed with @lyubomir-popov on this in the interim until we come up with custom icons.

In the home:

  • Can you please add the u-responsive-realign styles to the "Learn more about CVEs..." and the "By Ubuntu release" link lists? So spacing is a bit better on mobile.
  • Under "By Ubuntu release", can we generate the links dynamically? I see 24.04 is not there, and 23.10 is even when it's already out of support.
  • "Other releases" should just point to https://ubuntu-com-13412.demos.haus/security/cves?q=

In the search results:

In the individual CVE page:

  • Please add "Learn more about Ubuntu priority" link at the bottom of the "Why is this CVE high priority?" section when section exists. Should point to https://ubuntu.com/security/cves/about#priority
  • I accidentally stumbled upon this CVE, which doesn't seem to affect any release. Should we even list these? I feel tempted to change the notification to "No releases are affected by this CVE." and hide the table, but I worry it will be misleading.

Screenshot 2024-08-30 at 14 01 57

  • Spacing seems a bit off above the "Why is this CVE high priority?" heading. Maybe you can apply the ID and the scroll style to the H3 instead and get rid of the div container?
  • When comparing this CVE with the live version I see the notes are missing. Is this because of the demo? (If there are no notes, the "Read the notes from the security team" link should be hidden).
  • Please add a "What do statuses mean?" link after the "How can I get the fixes?" one.

In the About page:

  • Add paper background

Very proud of our work here, I really think it's a great improvement.

@mtruj013
Copy link
Contributor

mtruj013 commented Sep 5, 2024

Thanks @juanruitina! To answer some of your questions:

"Other releases" should just point to https://ubuntu-com-13412.demos.haus/security/cves?q=

This was already the case so that's why I've left it unchecked

I accidentally stumbled upon this CVE, which doesn't seem to affect any release. Should we even list these?

This is because we're filtering out packages which only have upstream statuses, prod version here. I do still think we should list these but hide the table as you suggested as it still seems like it has info people might want.

When comparing this CVE with the live version I see the notes are missing. Is this because of the demo?

No, it's because we were hiding notes if the priority reason existed. IIRC one of the cves we were testing against in a past pr had the same exact info for both sections so I was given the feedback to hide the notes of the reason was included in the payload. I think this is a better solution though (to not hide notes based on the existence of the priority reason I mean)

@akbarkz akbarkz merged commit 05bc47a into main Sep 13, 2024
25 of 26 checks passed
@akbarkz akbarkz deleted the cve-overhaul branch September 13, 2024 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants