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 suggested fixed version #2271

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

tomersein
Copy link
Contributor

@tomersein tomersein commented Nov 20, 2024

closes - #2264

this PR aims to add a new field to match details which specifies the suggested fixed version

@willmurphyscode
Copy link
Contributor

The discussion at #2264 (comment) should be wrapped up before we merge this.

Signed-off-by: tomersein <[email protected]>
@tomersein tomersein changed the title Filter unrelated fixed version Sort fixed version according to the package version Dec 13, 2024
@tomersein
Copy link
Contributor Author

hi @willmurphyscode ,
fixed it from filtering to sorting (1st one is most relevant)

Signed-off-by: tomersein <[email protected]>
Signed-off-by: tomersein <[email protected]>
Signed-off-by: tomersein <[email protected]>
Signed-off-by: tomersein <[email protected]>
Signed-off-by: tomersein <[email protected]>
Signed-off-by: tomersein <[email protected]>
Signed-off-by: tomersein <[email protected]>
@tomersein tomersein changed the title Sort fixed version according to the package version add suggested fixed version Jan 4, 2025
Signed-off-by: tomersein <[email protected]>
@tomersein
Copy link
Contributor Author

hi @willmurphyscode
added a new field inside match details as you suggested

@willmurphyscode willmurphyscode linked an issue Jan 23, 2025 that may be closed by this pull request
@willmurphyscode
Copy link
Contributor

Taking a look today. I am using the following as a manual test case:

grype eclipse-temurin@sha256:7b8b90b35214757840f79fc78794bf4b06cb6ac92a8853429de16366e837c302

This image includes [email protected]+11, which is vulnerable to CVE-2025-21502, and for which fixes have been back-ported to many JDKs, giving us a chance to test whether grype recommends weird old JDK versions.

go run ./cmd/grype eclipse-temurin@sha256:7b8b90b35214757840f79fc78794bf4b06cb6ac92a8853429de16366e837c302 -o json | jq '.matches[] | select(.artifact.name | test("openjdk")) |select(.vulnerability.id | test("CVE-2025-21502")) | .matchDetails[0]'

produces the expected output:

{
  "type": "cpe-match",
  "matcher": "stock-matcher",
  "searchedBy": {
    "namespace": "nvd:cpe",
    "cpes": [
      "cpe:2.3:a:oracle:openjdk:17.0.13:*:*:*:*:*:*:*"
    ],
    "package": {
      "name": "openjdk",
      "version": "17.0.13+11"
    }
  },
  "found": {
    "vulnerabilityID": "CVE-2025-21502",
    "versionConstraint": "< 1.8.0_441 || >= 1.9-ea, < 8.0.441 || >= 9-ea, < 11.0.26 || >= 12-ea, < 17.0.14 || >= 18-ea, < 21.0.6 || >= 22-ea, < 23.0.2 (jvm)",
    "cpes": [
      "cpe:2.3:a:oracle:openjdk:*:*:*:*:*:*:*:*"
    ]
  },
  "suggestedFixedVersion": "17.0.14"
}

I think there are two changes left to make:

  1. Adding some unit tests to cover the behavior on the new change.
  2. (Optional) Adding some element in the UI so that the suggestedFixedVersion is highlighted in some way. This is optional because it's easy to add later.

@anchore/tools what do we think of the name suggestedFixedVersion?

@wagoodman
Copy link
Contributor

wagoodman commented Jan 23, 2025

I'm generally ok with this field name, but if there could be more fix information in the future then saving a object for that could be nice:

{
  "type": "cpe-match",
  "matcher": "stock-matcher",
  ...
  "fix": {
    "suggestedVersion": "17.0.14"
  }
}

We do have some spots in the v6 schema where other fix information could be plumbed through one day (such as date released, git commit, etc).

Or we don't even need to say that it's "suggested":

{
  "type": "cpe-match",
  "matcher": "stock-matcher",
  ...
  "fix": {
    "version": "17.0.14"
  }
}

@willmurphyscode
Copy link
Contributor

Hi @tomersein! We talked about the naming a bit, and we'd like 2 changes to this PR before we can merge it:

  1. Can you add some unit tests to cover the new behavior? Especially covering the logic of how we pick the suggested version.
  2. Instead of putting suggestedFixedVersion directly on match details, can you take @wagoodman 's suggestion and make a field on match details called Fix which has a field called suggestedVersion? Then the JSON would look like this:
{
  "type": "cpe-match",
  "matcher": "stock-matcher",
  ...
  "fix": {
    "suggestedVersion": "17.0.14"
  }
}

That gives us a place to add more deta about the fix later, and keeps us from having a single field with a long name.

Thanks!

Matcher: string(d.Matcher),
SearchedBy: d.SearchedBy,
Found: d.Found,
SuggestedFixedVersion: suggestedFixedVersion,
Copy link
Contributor

@kzantow kzantow Jan 24, 2025

Choose a reason for hiding this comment

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

I suspect this was discussed before, so apologies for suggesting it again... but what about having this be a slice, populated with all known upgrades, sorted from "smallest" to "largest". I might even call this field "Upgrades": iterate over all known fix versions, omit any fix versions less than the current software version. So fix[0] would be "the easiest" fix, but there would be potentially other upgrades you could do, like upgrading to a OpenJDK 21 from OpenJDK 11, for example. If you were on OpenJDK 11, suggesting OpenJDK 11.x rather than 21 seems less than ideal for something that seems to be a recommendation, even if it's probably the easiest fix. Using this same logic for the table view "fixed-in" would seem to be good to do, also

@tomersein
Copy link
Contributor Author

hi @kzantow @willmurphyscode @wagoodman
fixed, let me know how to proceed further :)

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.

list of unrelated versions in the remediation
4 participants