fleetdm / fleet

Open-source platform for IT, security, and infrastructure teams. (Linux, macOS, Chrome, Windows, cloud, data center)
https://fleetdm.com
Other
3k stars 416 forks source link

Improve empty states for Software > Vulnerabilities #19857

Closed rachaelshaw closed 3 weeks ago

rachaelshaw commented 3 months ago

Goal

User story
As a user on the vulnerabilities page,
I want to see a clearer explanation of why a CVE search returned no results
so that I can understand whether Fleet has no record of it, or whether I'm not affected.

The customer wants to be able to differentiate between the following scenarios:

Context

Changes

Product

Engineering

ℹ️  Please read this issue carefully and understand it. Pay special attention to UI wireframes, especially "dev notes".

QA

Risk assessment

Manual testing steps

  1. Step 1
  2. Step 2
  3. Step 3

Testing notes

Following this flow chart will help in QA: https://drive.google.com/file/d/1HHhJoiYdNR9xMmznd4GtQ6Sbim8aFvrP/view?usp=sharing

Confirmation

  1. [ ] Engineer (@____): Added comment to user story confirming successful completion of QA.
  2. [ ] QA (@____): Added comment to user story confirming successful completion of QA.
noahtalerman commented 3 months ago

Rachael: was discussed but I didn't file issue:

Adding more filtering/sorting throughout the UI — we have some issues for this in drafting to bring into the next sprint, so probably worth seeing what they think about those, and making specific requests for improvements going forward. (It's the kind of thing I see us iterating on a bit at a time; I agree it'd be nice to have better filters in a lot of places.)

sharon-fdm commented 3 months ago

EST: BE 5 FE 3

zayhanlon commented 3 months ago

@noahtalerman do you want to review the figma with customer-faltona or think we're good to go?

noahtalerman commented 3 months ago

Hey @zayhanlon, good catch. Do you think we can add someone from customer-faltona to a design review next week?

zayhanlon commented 3 months ago

@noahtalerman absolutely - let me see if they can make it

zayhanlon commented 2 months ago

@lukeheath for this first hackathon that's designed, estimated and ready to go - the customer has suggested 8/12-8/13 for the development days (next sprint). letting you know here for assignment of an engineer (let me know if we should follow a different process)

@sharon-fdm @noahtalerman FYI

lukeheath commented 2 months ago

@zayhanlon This will be the virtual hackathon format, correct? I recall discussing the format, but I'm not certain where we landed. Do you have a format in mind?

zayhanlon commented 2 months ago

correct @lukeheath ! so basically the engineers can do the work as they would at home, but the customer is available for an end of day check-in to review progress on the issue, and then @noahtalerman and I discussed if maybe we want to patch the feature out or depending what date the customer selects, we could also just ship in the upcoming release.

lukeheath commented 2 months ago

@zayhanlon Got it. So overall same process, but a check-in call at the beginning with the customer, and then an end of day check-in each of the two days?

@sharon-fdm Please work with your team to select one frontend and one backend engineer that can schedule two days to work on this story and join the virtual hackathon. Just let me know if you have any questions. Thanks!

lukeheath commented 2 months ago

@sharon-fdm To be clear, this will be happening next sprint.

zayhanlon commented 2 months ago

@lukeheath we actually did the beginning check in call already (they joined the final design review and gave a thumbs up) :)

thanks! @sharon-fdm keep me posted so i can get meeting invites out

noahtalerman commented 2 months ago

TODOs:

More info: VINCE has the latest stuff: https://vuls.cert.org/confluence/display/VIN/VINCE+Documentation

noahtalerman commented 2 months ago

https://www.loom.com/share/8d288c9f30f54de9bddafc49586369f1?sid=9ca468bc-96d3-4d88-b46a-e0417f1fe922

getvictor commented 2 months ago

@noahtalerman

  1. The CVE format is CVE-YYYY-<4 or more digits>, so CVE-2023-9 is not a valid format.
  2. Will we always require the user to type in the CVE- prefix? Seems like we can make it optional.
  3. Does known_vulnerability assume an exact match? For example, if the search for CVE-2024-1234 returned no results and is not known to Fleet, but CVE-2024-12345 is known to Fleet.
noahtalerman commented 1 month ago

The CVE format is CVE-YYYY-<4 or more digits>, so CVE-2023-9 is not a valid format.

@getvictor thanks! Makes sense to use that correct format for validation instead. I updated the copy in the Figma here.

Will we always require the user to type in the CVE- prefix? Seems like we can make it optional.

Hmm, good idea to make it optional to give the user more grace. I think let's keep the help text the same though.

Note that we want to do this validation on the frontend.

noahtalerman commented 1 month ago

Does known_vulnerability assume an exact match? For example, if the search for https://github.com/advisories/GHSA-44xr-qjp2-rmf4 (CVE-2024-1234) returned no results and is not known to Fleet, but CVE-2024-12345 is known to Fleet.

@getvictor I think we go with yes. As a user searching CVE-2024-1234 I would expect to see the "not a known CVE" state here.

In the meantime, I'll send a message to the customer in Slack to learn more.

zayhanlon commented 1 month ago

@getvictor @RachelElysia - this was a hackathon ask for customer-faltona. they are OOO this week, but can meet on monday or tuesday next week if you want to share what's been built and get some feedback (as part of the process) before closing it out :)

getvictor commented 1 month ago

@RachelElysia the backend change for this is on main

RachelElysia commented 1 month ago

@getvictor cc: @zayhanlon awesome!! I'll try to get started on the FE Monday! I was under the impression this was going to be a specific day so I hadn't started on this in favor of P2s, but probably can finish by EOD Tuesday

noahtalerman commented 1 month ago

Hey @RachelElysia I think let's plan on demoing this scenario to the customer when we meet w/ them.

It's hard to describe and get feedback over Slack...

cc @zayhanlon

zayhanlon commented 1 month ago

@RachelElysia @getvictor scheduling a call with you both and the customer for this week. stand by!

RachelElysia commented 1 month ago

@zayhanlon cc: @getvictor @mike-j-thomas frontend is merged to main, new graphics and all

noahtalerman commented 1 month ago

Hey @rachaelshaw and @getvictor I updated several dev notes and some copy in Figma following our call w/ the customer.

I recorded this Loom that includes changes we discussed during the call and changes I thought about after the call.

Please take a look at let me know if you have any questions or if it doesn't track with what y'all were thinking.

Thanks!

getvictor commented 1 month ago

@RachelElysia The latest backend change has been merged to main.

lukeheath commented 1 month ago

@sharon-fdm @getvictor @RachelElysia We're bringing this back to expedited drafting to make a small requirement change. Will update you later today.

lukeheath commented 1 month ago

@sharon-fdm I've added a new backend sub task to this story that we need to estimate: https://github.com/fleetdm/fleet/issues/21392. Would you please review with @getvictor, estimate, and bring onto the sprint board?

@RachelElysia has full context from our design review, but we decided not to enforce exact match searching on the frontend and instead do it on the backend so that this endpoint returns consistent results to the user between the UI and API. We're bringing this back through expedited drafting, but note this is not a priority ticket and I expect this change will mean we need to push to the next release.

sharon-fdm commented 1 month ago

Thanks, @lukeheath. @getvictor, do we need to roll back the changes you already merged?

getvictor commented 1 month ago

@lukeheath @sharon-fdm The current backend change that's on main is the following: If query is a legal CVE format (CVE-YYYY-<4+ digits>) then return a known_vulnerability field as part of the response.

I assume we still want the known_vulnerability field as part of https://github.com/fleetdm/fleet/issues/21392 In that case, we don't need to rip out the current code, just modify it.

getvictor commented 1 month ago

@marko-lisica @lukeheath @rachaelshaw

1. When we met with the customer, we agreed to only support the exact search for CVEs in the UI. Adding the double quotes requirements feels like a step in the wrong direction. For example, if the user searches for CVE-2023-1234 and they get back CVE-2023-12345, how do we indicate to them whether Fleet knows about CVE-2023-1234 or that the need to put double quotes around the query?

2. What if the customer misses a double quote, like "CVE-2023-1234 -- should frontend validate that?

3. We already have an endpoint for exact match which fronend can use:

GET /api/v1/fleet/vulnerabilities/{cve}

I propose that if Fleet knows about the vulnerability (but no hosts match it), that endpoint returns a 204 instead of 404

  1. Since we are doing an exact match, why not display the vulnerability details on the Software->Vulnerabilities tab, instead of having the user click and go to another page to get the same info?

marko-lisica commented 1 month ago

When we met with the customer, we agreed to only support the exact search for CVEs in the UI. Adding the double quotes requirements feels like a step in the wrong direction. For example, if the user searches for CVE-2023-1234 and they get back CVE-2023-12345, how do we indicate to them whether Fleet knows about CVE-2023-1234 or that the need to put double quotes around the query?

@getvictor The idea is to wrap a string in double quotes to indicate it should return for exact match. So if user search for "CVE-2023-1234" and there's no results, Fleet will return one of the new empty states ("This is not a known CVE" or "This is a known vulnerability (CVE), but it wasn't detected on any hosts."). If they search without quotes it will be same fuzzy search we have today with same empty states. We could add a tooltip to indicate that users can search for exact match if they wrap CVE in quotes.

We already have an endpoint for exact match which fronend can use:

GET /api/v1/fleet/vulnerabilities/{cve}

Since we already have an endpoint for this, I think it makes sense to have logic on the frontend that checks if CVE is wrapped with quotes and based on that call this endpoint instead of GET /api/latest/fleet/vulnerabilities?query={search_string}

I propose that if Fleet knows about the vulnerability (but no hosts match it), that endpoint returns a 204 instead of 404

I agree, we can return 204 and probably 404 in case that CVE is not known?

lukeheath commented 1 month ago

Since we already have an endpoint for this, I think it makes sense to have logic on the frontend that checks if CVE is wrapped with quotes and based on that call this endpoint instead of GET /api/latest/fleet/vulnerabilities?query={search_string}

This makes sense to me. If the user indicates they want exact match (by wrapping the string in quotes), the frontend will route the request to the exact match endpoint. Otherwise, it will use the fuzzy search endpoint. Thoughts? @RachelElysia @marko-lisica

marko-lisica commented 1 month ago

This makes sense to me. If the user indicates they want exact match (by wrapping the string in quotes), the frontend will route the request to the exact match endpoint. Otherwise, it will use the fuzzy search endpoint. Thoughts?

@lukeheath @RachelElysia That's what I specified here in Figma. I'll bring this to design review so we take one more look at it before re-estimation.

georgekarrv commented 1 month ago

I would strongly urge to use a toggle (like show versions) and do this without quotes

Why?

getvictor commented 1 month ago

Remaining estimates:

FE:5 BE:3

@getvictor to add subtask if needed

noahtalerman commented 3 weeks ago

Hey @zayhanlon, heads up that this customer request (hackathon) was shipped! 🎉

fleet-release commented 3 weeks ago

Clearer explanations, CVE search results bloom, Understanding grows.

Patagonia121 commented 5 days ago

@noahtalerman I received some interesting/actionable feedback from the customer about this feature that they have to put the CVE in quotes when searching for it in Fleet. General takeaway is that this feature is usable and solves the zero-day problem, but that it's a tad-bit annoying to have to put quotes around the CVE everytime they perform a search since they never want to do fuzzy searching for CVEs.

noahtalerman commented 4 days ago

Hey @Patagonia121 thanks!

Can you please open a fresh feature request w/ Gong snippet attached? This way, that request goes into the product "Inbox" where we intake new requests. With the gong recording we have the full context on the ask.