-
Notifications
You must be signed in to change notification settings - Fork 98
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
Unxpected behavior with "fail-on-severity" configuration option #618
Comments
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 |
@virangdoshi Can you test against the action branch
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: |
Thanks! @febuiles Can this also be applied to an older branch? At my company, we are using |
@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. |
Closing this issue (fixed in https://github.com/actions/dependency-review-action/releases/tag/v3.1.4) |
@febuiles Any chance this fix can be ported back to the |
@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. |
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 infail-on-severity
.For example, with the config option set
fail-on-severity: high
, the action fails even if the GHSA is added toallow-ghsas
configuration. This behavior occurs when there is amoderate
severity vulnerability in addition to thehigh
severity vulnerability in the introduced dependency. The configfail-on-severity: high
, is supposed to fail if dependencies contain ahigh
severity vulnerability. In practice the action seems to fail even if it contains ignoredhigh
GHSA's and in addition, the dependency also contains GHSA's with severity lower than the one configured infail-on-severity
. The summary/debug logs still show the "ignored" GHSA as the cause of failureHere are PR's in a public repository that reproduces this behavior:
virangdoshi/juice-shop#30 - Configured to fail on
high
. Ahigh
criticality "ignored" vulnerability with multiple "medium" criticality vulnerabilityvirangdoshi/juice-shop#26 - Configured to fail on
critical
. Actitical
"ignored" vulnerability with multiple "high" criticality vulnerabilityAny help or clarification on this issue would be highly appreciated! Thanks in advance
The text was updated successfully, but these errors were encountered: