Unxpected behavior with "fail-on-severity" configuration option

[virangdoshi]

Hello,

I would like to flag an unexpected behavior in the way Dependency Review action fails on a severity level. The action does not honor the configuration fail-on-severity if there are other "non-ignored" GHSA's with the severity lower than the one set in fail-on-severity.

For example, with the config option set fail-on-severity: high, the action fails even if the GHSA is added to allow-ghsas configuration. This behavior occurs when there is a moderate severity vulnerability in addition to the high severity vulnerability in the introduced dependency. The config fail-on-severity: high, is supposed to fail if dependencies contain a high severity vulnerability. In practice the action seems to fail even if it contains ignored high GHSA's and in addition, the dependency also contains GHSA's with severity lower than the one configured in fail-on-severity. The summary/debug logs still show the "ignored" GHSA as the cause of failure

Here are PR's in a public repository that reproduces this behavior:
virangdoshi/juice-shop#30 - Configured to fail on high. A high criticality "ignored" vulnerability with multiple "medium" criticality vulnerability

virangdoshi/juice-shop#26 - Configured to fail on critical. A ctitical "ignored" vulnerability with multiple "high" criticality vulnerability

Any help or clarification on this issue would be highly appreciated! Thanks in advance

[virangdoshi]

I did some digging into the filtering flow and have come up with a possible reason for the behavior. I have raised a PR with a fix based on my analysis: #622

Looking at filter here: https://github.com/actions/dependency-review-action/blob/1cbb0489072933d6823ebce2028a73d48261ea0d/src/main.ts#L85C21-L85C21, it first filters by allowed GHSAS and then by severity of vulnerability. This leads to incorrect filtering, possibly due to one of the filters now working as expected.

Switching the order seems to work and give accurate results. The order of filter matters and gives different output based on which filter is applied first. Test cases to reproduce 2 different orders of filtering give separate results: https://github.com/actions/dependency-review-action/pull/622/files#diff-fc6d7537d3a9c088f297d6b58708d0ab512e31c9dadba846573ff8a83fb4c973R128

[febuiles]

@virangdoshi Can you test against the action branch fix-advisory-filters and see if you can still reproduce this behavior?

      uses: actions/dependency-review-action@fix-advisory-filters
        with:
        ... your options

I have a PR up that I think will fix this issue: #623.

I have setup the same test harness you shared before, and it seems to be working now:

• PR with Critical GHSA
• PR with High GHSA

[virangdoshi]

Thanks! @febuiles
Both my test PRs are passing and working as expected with the fix-advisory-filters 🚀
virangdoshi/juice-shop#30
virangdoshi/juice-shop#26

Can this also be applied to an older branch? At my company, we are using 3.0.8 (https://github.com/actions/dependency-review-action/releases/tag/v3.0.8). The reason for using this older tag, is that the the release after 3.0.8 is throwing snapshot warning in the summary, which is undesired. Using 3.0.8 does not throw snapshot errors and we plan to continue using this older release

[febuiles]

@virangdoshi can you share a screenshot or text log of the unexpected error you're seeing in this issue: #566?

Maybe we can get that fixed for a new release too.

[virangdoshi]

That would be awesome! Here is the screenshot of the summary

This was popping up in every summary. Tried enabling that config in the suggestion, but to no effect.
(Scrubbed the head SHA in the screenshot)

[febuiles]

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

[virangdoshi]

@febuiles Any chance this fix can be ported back to the 3.0.8 version? I am sure there might be other folks who might benefit from not having the snapshot errors show up in the summary.

[febuiles]

@virangdoshi we can't modify existing versions. We need to get a reproduction case of #626 in order to release a new fix that can benefit all folks.