search

dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.65k stars 1k forks source link

Configuration of a minimum package age required before a PR is created #3651

Open ericmatthys opened 3 years ago

ericmatthys commented 3 years ago

A "cooldown" feature would delay opening a PR for a newly updated dependency until a certain amount of time has passed. This feature could help teams with mature / stable projects that would like to be more conservative with dependency updates, but still stay on top of regular updates. A cooldown period would allow for patch updates after a release without impacting the project or creating excess noise. Also, for packages that update very frequently, a cooldown period could help cut down on noise from that project without changing the entire schedule to be slower. It would also be useful to allow the cooldown time to be configurable for different semver versions. For example, I might set semver-major to 1 month, semver-minor to 1 week, and semver-patch to 3 days.

Here's how a config snippet might look:

updates:
  - package-ecosystem: "npm"
    directory: "/"
    schedule:
      interval: "daily"
      cooldown:
      - semver-major: 30
      - semver-minor: 7
      - semver-patch: 3

Here's a scenario using those values in mind:

  1. Package updates from 1.1.0 to 1.1.1
  2. 3 days pass
  3. PR for 1.1.1 is created
  4. Package updates from 1.1.1 to 1.2.0
  5. 2 days pass
  6. Package updates from 1.2.0 to 1.2.1 with a bug fix
  7. 1 day passes
  8. Package updates from 1.2.1 to 1.2.2 with a bug fix
  9. 4 days pass
  10. PR for 1.2.2 is created
  11. Package updates from 1.2.2 to 1.3.0
  12. 3 days pass
  13. Package updates from 1.3.0 to 1.4.0, resetting the cooldown because it's another minor update
  14. 7 days pass
  15. PR for 1.4.0 is created
asciimike commented 3 years ago

Hah, this is actually an option we looked at when thinking about https://github.com/dependabot/dependabot-core/issues/2219. It's not on the near term roadmap, but definitely makes sense and something we'd be interested in adding in the future.

hiqua commented 3 years ago

This feature would be very welcome.

Beyond having less PRs from frequently updated projects, there's also the problem of how many of them are even relevant when several libraries used in the same project are made incompatible with each other in a version bump.

If A v1 and B v1 are currently used:

  1. A goes from v1 to v2
  2. B v1 is compatible with A v1 but not with A v2
  3. B has not anticipated A v2, but will introduce fixes later in B v2

An open PR is basically TODO item, but in this case the best course of action is often to wait right after A v2 release, in case 3) happens and the version bump is less painful that it seemed at first glance. And if 3) doesn't happen, then a manual fix can be done. But opening the PR right after A v2 release is premature.

In short, I'd much rather have PRs open when I'm reasonably certain I have to do something if they don't pass the CI (rather than just leaving them open and wait for $n days to give a chance for the issue to fix itsel).

ZackKanter commented 2 years ago

We need this as well. FWIW, Snyk's default/non-configurable behavior is a 21-day delay:

Snyk does not recommend upgrades to versions that are less than 21 days old. This is to avoid versions that introduce functional bugs and subsequently get unpublished, or versions that are released from a compromised account (where the account owner has lost control to someone with a malicious intent).

This setting cannot be overridden. It would be very nice to have this a configurable option in dependabot.

ZackKanter commented 2 years ago

Renovate calls this stabilityDays.

limonte commented 1 year ago

Yes please!

When a vulnerability is introduced to an npm package, it takes at least several days to discover the vulnerability and to report the vulnerable release to security databases (npm, Snyk, dependabot, etc.)

Because of that reason, it would make sense for developers to use 3rd party dependencies with the version that matches these conditions:

L1ghtman2k commented 1 year ago

We have multiple production environments of different maturity, and we use gitops model to deploy our changes to the clusters(So, Dependabot is a big part of it).

It would be nice to have Dependabot update production clusters of the highest maturity only after the first two environments have been updated(one way I could see this achieved is via this feature request)

JacobEvelyn commented 12 months ago

Just sharing that given the increasing frequency of sophisticated supply-chain attacks, over the past few years this feature has gone from nice-to-have to a necessity for us.

We're planning on hacking together this functionality on top of Dependabot PRs using the GitHub API, but it will be a pain because there's so much more state to manage (e.g. PRs open but not yet at the cooldown period, GitHub labels, branch rules, etc.) than there would be if this was a first-class Dependabot feature. Even just adding it ecosystem-by-ecosystem would be very welcome.

@jeffwidman would Dependabot be open to PRs that support this feature for a given ecosystem? For example, the RubyGems API call Dependabot is makes returns published timestamps (example) and it looks straightforward to change the Gem::Version objects we create from that API response to a duck-type that can also provide timestamps, and use those to determine whether to create a PR. Even if the feature doesn't yet support private registries/etc. or every ecosystem, this would still be very valuable to us. We could even beta-test it as an undocumented config setting. We might be able to make a PR for this but won't bother if it has no hope of getting accepted.

JacobEvelyn commented 11 months ago

@jeffwidman just pinging you again on this question ☝️ (or let me know if there's someone else I should ask).

IvanRibakov commented 8 months ago

Agree with @limonte and @JacobEvelyn that while initially this feature was presented to be about stability and noise, it has far more importance from the security point of view. You might think that if Dependabot PR with compromised dependency is created, the impact is small until the PR is merged + there may be additional delay until that merge commit is released. However the damage from the pwn requests coming from compromised dependencies is going to be instantaneous as soon as the Dependabot PR is created and the CI workflow is kicked off. So for those use cases when other recommendations for mitigating pwn requests are not feasible, the last thing that is left is to try to delay the introduction of new dependency versions.

Please consider security implications of lacking this feature and consider increasing it's priority in the backlog.

fallemand commented 5 months ago

Any update over this feature request? 🙏

hballangan-mdsol commented 4 months ago

+1 on this feature request

gagagast commented 3 months ago

+1

kilianpaquier commented 3 months ago

+1

stefan-spiess commented 3 months ago

This feature would play an essential role in supply chain attack prevention, which is very important on nowadays. Renovate has this feature implemented for years now with https://docs.renovatebot.com/configuration-options/#minimumreleaseage.

erheron commented 3 months ago

Hi folks, I'm curious if anybody has other alternatives to this? We're currently thinking about a (quite simple) in-house script, but would be nice to hear from the community ! 👀

IvanRibakov commented 3 months ago

@erheron are you talking about script to

  1. completely bypass Dependabot because of the limitation discussed in this issue, or
  2. introduce delay for new releases ?

If it's the 2nd, can you please share which entry point are you considering to use to prevent Dependabot from creating new PRs?

antonmos commented 3 months ago

We use Artifactory for pulling external pacakges and it is configured to prevent use of dependency versions that are less then 14 days old. Since dependabot is not aware of this limitation, it creates PRs that immediately fail (they can only succeed after 14 days). However, if a new version of the dependency is released after 13 days, that PR is closed and a new one is opened, that again will fail for 14 days.

matheusml commented 6 days ago

+1

This would be very beneficial for us

IvanRibakov commented 6 days ago

@jonjanego I see you were the last person to update the labels of this feature request. I'm assuming you have some relationship to the Dependabot team. I apologise if my assumption is wrong (if so, perhaps you could help tag appropriate person to draw attention to this issue?).

Can you please consider reviewing the current F:noise label and re-classifying this issue taking into account latest feedback that points out this feature's security implications:

jonjanego commented 6 days ago

@jonjanego I see you were the last person to update the labels of this feature request. I'm assuming you have some relationship to the Dependabot team. I apologise if my assumption is wrong (if so, perhaps you could help tag appropriate person to draw attention to this issue?).

Can you please consider reviewing the current F:noise label and re-classifying this issue taking into account latest feedback that points out this feature's security implications:

Hi @IvanRibakov - yes, I'm one of the PMs at GitHub supporting Dependabot. Could you clarify what you are asking about for the label? These are things we use to help us organize themes of user feedback in our internal project boards. I don't know the history of that label's creation, but they're used for that purpose, not as a statement of the value / priority of the feedback. The L:Noise label is described as "related to Dependabot being noisy, or initiatives to make Dependabot quieter", which this seems somewhat related to, but again, it's for helping us to group the hundreds of pieces of user feedback internally.

Regarding this feature request as a whole - agree that it is a valuable one that we could look at within GitHub in a few different contexts. There aren't any specific features planned around this at this time, but it's something that we're thinking about!

In the meantime you may want to take a look at the dependency review action, which, while not directly addressing this, can be used to help you understand more about the dependencies introduced in a PR and automatically fail the PR if they meet certain criteria.

IvanRibakov commented 6 days ago

HI @jonjanego , thanks for reaching out!

I don't know the history of that label's creation, but they're used for that purpose, not as a statement of the value / priority of the feedback.

Ok, I get that labels do not directly reflect the value/priority of the feature request, but whoever uses these categories on your end must be factoring in the semantics of the label category when prioritizing them. Otherwise, what is the real value of categorizing issues if it's not used to help navigate and prioritize the work backlog?

The L:Noise label is described as "related to Dependabot being noisy, or initiatives to make Dependabot quieter", which this seems somewhat related to

I don't have a problem with the current label, I can see how it makes sense. Not sure if adding multiple F: labels is an option for issues that lie at the cross-section of varios problem domains?

In the meantime you may want to take a look at the dependency review action, which, while not directly addressing this, can be used to help you understand more about the dependencies introduced in a PR and automatically fail the PR if they meet certain criteria.

Unfortunately dependency review action can not help prevent running Dependabot-initiated CI workflows for latest dependency versions that might have been intentionally compromised for the attacks on the supply chain. The reason is simple - there is an inevitable time delay between the publishing of the latest dependency version and it being analyzed by security community and a vulnerability/attack vector being reported. The only sa(f/n)e thing to do in this case is to let latest dependency versions "cool down" for some time before being applied. During this "cool down" period the vulnerabilities may be detected and patches issued.

ZackKanter commented 5 days ago

The L:Noise label is described as "related to Dependabot being noisy, or initiatives to make Dependabot quieter", which this seems somewhat related to, but again, it's for helping us to group the hundreds of pieces of user feedback internally.

In the meantime you may want to take a look at the dependency review action, which, while not directly addressing this, can be used to help you understand more about the dependencies introduced in a PR and automatically fail the PR if they meet certain criteria.

@jonjanego appreciate you chiming in. I think this misses the core of the issue, which is that the lack of the ability to configure minimum package age is a massive security hole in Dependabot. Right now, anyone who uses Dependabot will get package updates without delay – this dramatically expands the speed and blast radius of any supply chain attack.

While it may technically be possible to avoid this by using the dependency review action, only a tiny percentage of users will i) realize that they need to do this in order to insulate themselves from supply chain attacks, and ii) go through the trouble of configuring a separate action to mitigate the attack vector. If this were a simple config within Dependabot (for example, Renovate has stabilityDays) it would be easily discoverable and configurable by everyday users. Without this config, it requires diligence and effort beyond what's realistic to expect from an everyday user – so by adopting Dependabot, users are opening themselves to a well-known attack vector to which they otherwise would not have been vulnerable.

The fact that this would be a fairly trivial non-breaking change to Dependabot makes it even more difficult to understand.