dependency-review-action
dependency-review-action copied to clipboard
Add Vulnerabilities and license checks
New Feature
Creates two checks:
- A check with vulnerabilities added. If the check fails, it adds to the check the list of vulnerabilities found grouped by manifest. The check fails if vulnerable package(s) are added.
- License check. A license check is created and it lists on the detail incompatible licenses (if any) and the unknown licenses. The check fails if there are incompatible licenses and succeeds otherwise
By separating the failure in two, users can now control if license or vulnerabilities individually prevent the merge from happening.
Checks names are configurable.
Breaking Changes
A breaking change was introduced, the action no longer fails if there are vulnerabilities or license violations. fail-on-violation
parameter can be set to true (default value is false) to maintain backward compatibility.
There is another potential breaking change, since the workflow now requires checks: write
permission, might need to add the new permission explicitly. The dependency review starter workflow will also have to be updated to add this permission
@tspascoal I love the idea of having one check per feature, but the main Action one should never be green if one of the children checks fails. Can we overcome this? If not I don't think we should be spending extra cycles here (it's backwards incompatible and adds extra write security permissions).
@tspascoal I love the idea of having one check per feature, but the main Action one should never be green if one of the children checks fails. Can we overcome this? If not I don't think we should be spending extra cycles here (it's backwards incompatible and adds extra write security permissions).
@febuiles Not failing the action, allows extra flexibility since users can fine grain when the PR can't be merged by requiring the checks them want (one of them or both).
Backward compatibility (failing the action if one of the checks fails) can be achieved by explicitly setting the parameter fail-on-violation
to true (default value is false).
This allows both the extra flexibility or getting the previous behavior with this parameter.
So getting the old behavior by default, is a matter of changing the default value for fail-on-violation
or let the user control by enabling if they want it.
Backward compatibility (failing the action if one of the checks fails) can be achieved by explicitly setting the parameter fail-on-violation to true (default value is false). This allows both the extra flexibility or getting the previous behavior with this parameter.
In order for this to be backwards compatible I'd expect it to behave the same way that it has without the user having to modify their YAML. Remember that we pin all release to v2
atm, so we can't release as part of this version because we'll change the behavior for the current users. We can release a v3
, but we release major versions only for breaking backwards compatibility!
I don't know how the child checks work, could we default fail-on-violation
to true? In that case only users who want this new behavior would be modifying their workflows. We'd be able to switch this behavior once v3
is released if that's the way we want to move forward (still not sure what the right decision is here, would like to hear back from users in case you have feedback!).
Backward compatibility (failing the action if one of the checks fails) can be achieved by explicitly setting the parameter fail-on-violation to true (default value is false). This allows both the extra flexibility or getting the previous behavior with this parameter.
In order for this to be backwards compatible I'd expect it to behave the same way that it has without the user having to modify their YAML. Remember that we pin all release to
v2
atm, so we can't release as part of this version because we'll change the behavior for the current users. We can release av3
, but we release major versions only for breaking backwards compatibility!
Yeah, my intention was to make it a V3, I've added a Upgrading from V2: Breaking changes
section to the readme see Since the new permission is potentially a breaking change, might as well introduce a nicer semantic since users (may) have to change the workflow anyway.
I don't know how the child checks work, could we default
fail-on-violation
to true? In that case only users who want this new behavior would be modifying their workflows. We'd be able to switch this behavior oncev3
is released if that's the way we want to move forward (still not sure what the right decision is here, would like to hear back from users in case you have feedback!).
If you feel that this should be a V2 release, we can certainly make fail-on-violation
true by default (but users might still have to change the workflow due to permissions), and will need to update the readme to not mention the upgrade from V2.
Let me know which scenario you want to achieve:
- V2 with no breaking change on the action failure
- (
fail-on-violation
will default to true) - No mention on readme to upgrade from V2, but we will still need to document the change of permission
- V3 with no breaking change on the action failure (
fail-on-violation
will default to true)
- Upgrade from V2 will be tweaked
- V3 with breaking change on the action failure (
fail-on-violation
will default to false)
I keep forgetting about auth, thanks for bringing it up! We can wait until we have the new config file format before revisiting this.
The latest release of the action (v3
) allows users to specify if they want to allow/disallow a specific check (vulns, licenses) in their runs. I know this is not exactly the same functionality this PR provided, but I feel it's close enough to close this for now.