Issue #618 Change order of filtering to get correct list of vulnerabilities

[virangdoshi]

This PR attempts to resolve Issue 618, where filtering by severity and allowed ghsas does not seem to work as expected.

Currently filtering by allowed advisory first then by severity, yields different (incorrect) output. Changing the order of filtering --> filter by severity first then by allowed advisory seems to resolve the issue. Added 2 test cases to reproduce the scenario.

[Needs review] "Deny packages" check and "Invalid License" check to run against the unfiltered changes. This is to ensure that it does not have incorrect output from the trimmed/filtered list. Also, these 2 checks should run independently of vulnerability/severity checks and not on a subset of the changes, since they are kind of unrelated. For example, a filtered change might have an incompatible/restrictive license issue

[febuiles]

@virangdoshi thanks for this PR! After looking closer at this issue, I have noticed that the current behavior of the Action when dealing with GHSAs is to entirely drop the dependency change, instead of dropping the specific vulnerability.

Given this example from your test repo:

{
    "change_type": "added",
    "manifest": "package.json",
    "ecosystem": "npm",
    "name": "lodash",
    "version": "4.17.0",
    "package_url": "pkg:npm/lodash@4.17.0",
    "license": "MIT",
    "source_repository_url": "https://github.com/lodash/lodash",
    "scope": "runtime",
    "vulnerabilities": [
      {
        "severity": "critical",
        "advisory_ghsa_id": "GHSA-jf85-cpcp-j695",
        "advisory_summary": "Prototype Pollution in lodash",
        "advisory_url": "https://github.com/advisories/GHSA-jf85-cpcp-j695"
      },
      {
        "severity": "high",
        "advisory_ghsa_id": "GHSA-4xc9-xhrj-v574",
        "advisory_summary": "Prototype Pollution in lodash",
        "advisory_url": "https://github.com/advisories/GHSA-4xc9-xhrj-v574"
      },
      {
        "severity": "high",
        "advisory_ghsa_id": "GHSA-35jh-r3h4-6jhm",
        "advisory_summary": "Command Injection in lodash",
        "advisory_url": "https://github.com/advisories/GHSA-35jh-r3h4-6jhm"
      },
      {
        "severity": "high",
        "advisory_ghsa_id": "GHSA-p6mc-m468-83gw",
        "advisory_summary": "Prototype Pollution in lodash",
        "advisory_url": "https://github.com/advisories/GHSA-p6mc-m468-83gw"
      },
      {
        "severity": "moderate",
        "advisory_ghsa_id": "GHSA-x5rq-j2xg-h7qm",
        "advisory_summary": "Regular Expression Denial of Service (ReDoS) in lodash",
        "advisory_url": "https://github.com/advisories/GHSA-x5rq-j2xg-h7qm"
      },
      {
        "severity": "moderate",
        "advisory_ghsa_id": "GHSA-29mw-wpgm-hmr9",
        "advisory_summary": "Regular Expression Denial of Service (ReDoS) in lodash",
        "advisory_url": "https://github.com/advisories/GHSA-29mw-wpgm-hmr9"
      },
      {
        "severity": "low",
        "advisory_ghsa_id": "GHSA-fvqr-27wr-82fm",
        "advisory_summary": "Prototype Pollution in lodash",
        "advisory_url": "https://github.com/advisories/GHSA-fvqr-27wr-82fm"
      }

If we add GHSA-fvqr-27wr-82fm to allow_ghsas (a low severity vulnerability), the entire lodash dependency will be dropped from the list of changes. This behavior should be fixed regardless of the order of the operations (and doing that fixes the bug without needing to alter the order I think).

I've opened a PR in #623 and would appreciate your feedback.

[febuiles]

Re:

"Deny packages" check and "Invalid License" check to run against the unfiltered changes. This is to ensure that it does not have incorrect output from the trimmed/filtered list. Also, these 2 checks should run independently of vulnerability/severity checks and not on a subset of the changes, since they are kind of unrelated.

Agreed! Same with license issues (GHSAs have nothing to do with those). Feel free to create an issue, or better yet, submit a PR :)

[febuiles]

Closing (fixed in main: https://github.com/actions/dependency-review-action/releases/tag/v3.1.4)