Security updates must specify the dependency to be updated#51
Conversation
jakecoffman
left a comment
There was a problem hiding this comment.
Interesting since security-updates-only: true is set. Maybe we should have an explicit check for these things in the updater and reject invalid jobs? I can't imagine there's many other cases like this though.
We have at least one other 'working by accident' scenario I'm aware of - we only specify a single dependency for new security update jobs, but we will attempt to handle multiple - I've left that one in place as unpicking it is non-trivial. This is a gap between old standalone Dependabot and GitHub-integrated, my plain is to strictly match payloads to Operations ( i.e. so we either support it or don't ) and then make sure none are still going down the legacy route, after which we can start throwing an exception if you try a config that isn't valid. |
This smoke test was failing for the new, isolated security update operation class in dependabot/dependabot-core#6961
It was odd as local testing with the CLI was showing the right outcome, but I realised that one consequence of this isolated updater is that it enforced the de-facto contract that a security update job must specify the
dependenciesit is attempting to update.Currently, if we do not set the dependencies we effectively perform an "All Versions Update" by suppress any updates we could make other than those we have advisory data for.
This 'working by accident' state is something the isolated Operations are trying to remediate, so let's update the test to use the real-world job definition.
Notes
In the future, if we wanted to have some kind of 'All Security Updates possible' workflow, this is something we should treat as a new Operation class to be clear about the input payloads, etc, so we head into the right code line. There's still more work to be done in the Updater in order to make it more flexible/fully featured but for now I think closing the door on these accidental outcomes is in the spirit of the refactor.