greenkeeperio / greenkeeper

:robot: :palm_tree: Real-time automated dependency updates for npm and GitHub
https://greenkeeper.io/
Apache License 2.0
1.6k stars 95 forks source link

Weekly Dependency Updates #808

Open simlu opened 6 years ago

simlu commented 6 years ago

This has been requested before, but the ticket was closed with the reason "Single-PR defies the purpose of knowing which dependency update broke the build." - That might be true, but I don't care about that. I care about ease of updating dependencies.

The problem is that sometimes multiple dependencies update at the same time and greenkeeper creates multiple prs. The pr merged in first creates a conflict for the other prs since the lock file is also updated.

I just want an easy way to manage the dependencies of my projects, without having to resolve conflicts and it seems I'm not the only person with this problem.

Pyup does weekly prs with a settings file and my python projects much nicer to manage than my node ones. Worth taking a look for inspiration.

Looking forward to a response here! Thank you

janl commented 6 years ago

Thanks for the heads-up and the pointer to existing solutions that work better for you, we’ll start looking into how these things fit into Greenkeeper.

simlu commented 6 years ago

Considering https://github.com/eslint/eslint-scope/issues/39

We will cancel and disable greenkeeper for now since it is too spammy to be used to update every dependency individually without auto merging. But auto merging is pretty dangerous as this shows.

Would love to re-subscribe once this is implemented as I still love automatic dependency prs.

janl commented 6 years ago

Since that version wasn’t published as latest, Greenkeeper didn’t send any updates for this.

@simlu are you using pinned dependencies or open ranges? The latter one should greatly reduce chattiness.

simlu commented 6 years ago

Agreed, that in this particular case we got lucky.

@janl We use pinned dependencies since we use a lock file and npm ci to install. If we were using version ranges we wouldn't get greekeeper updates and the lock file wouldn't get updated. So unfortunately your suggestion doesn't work for us - unless I'm misunderstanding?

janl commented 6 years ago

@simlu I think there is a way to set up https://github.com/greenkeeperio/greenkeeper-lockfile in a way to get in-range updates for lockfiles, but I’ll have to defamiliarise myself with the details. This is a very valid use-case tho, so we totally understand your situation. It’s the end of day for me here, and I’ll look into this tomorrow and report back. Thanks a lot for the feedback!

simlu commented 6 years ago

@janl Even if that was possible, you would still run into the same issues with conflicts as mentioned initially?

No worries. Those timezones :) Here it's just the start of the day. Thank you very much for being responsive!

travi commented 6 years ago

for a few more things to consider, the compromised package was looking for tokens. in many projects, a token exists even in scope of the job running on ci. this particular attack only looked for .npmrc in the user's home directory (assuming either $HOME or $USERPROFILE. had it looked for $NPM_TOKEN or the project's local .npmrc, it would have been able to find tokens in more ci setups, even if the in-range version was never turned into a PR and merged into the project.

this attack appears to be almost directly based on this article. i don't think there is much that can be done on the greenkeeper side to prevent a situation like this, though, without more updates to how token scope is handled on the npm side. while the article mentions greenkeeper directly, even the suggested integrations wouldnt be helpful in a situation like this since the vulnerability hadn't been discovered/reported yet.

not suggesting changes, but situations do make you stop and consider risks again, so hopefully these thoughts are helpful to rounding out the consideration at least.

simlu commented 6 years ago

@travi

(1) Very much agreed. This was a very amateurishly executed exploit. Which was probably also why it was discovered this quickly.

(2) Agreed. This is not a problem with greenkeeper itself. However greenkeeper helps in very fast distribution and cascading, which leaves short time to react. Having daily, weekly, bi-weekly or monthly dependency updates would certainly help slow this down and give time to discover the vulnerability before it spreads.

(3) Agreed. We used to use greenkeeper + auto merges of prs, since it allowed very little maintenance to keep a project up to date. However the mentioned issue definitely made us reconsider. We would be ok with manual merges if they were weekly, but since that is not possible we decided to abandon greenkeeper for now - don't get me wrong I love greenkeeper and automatic dependency updates. Our risk evaluation has just changed and functionality available doesn't meet our requirements at this point.

travi commented 6 years ago

yep, i hear the concern and can't disagree. i do have a hard time thinking that slowing down is completely the right answer either, though. even waiting for daily, weekly, bi-annually, or monthly updates will miss vulnerabilities, especially without help of a service that automates it.

honestly, i think the ideal solution isn't to take on the burden of manually updating again or delaying updates, but instead having some way to shorten the window as much as possible. this might be an opportunity for greenkeeper, or another service if beyond the scope of greenkeeper, but sending PRs to install safe versions into projects that happened to get a compromised version as soon as the vulnerability is discovered would at least make the window of time that the version exists in a project as small as possible. admittedly, that could still be too late for attacks like this that only need initial access, but even those attacks may exist for longer than an artificial waiting period if less amateurishly executed.

no easy answer in cases like this

simlu commented 6 years ago

It's certainly not the full answer, but part of it.

My reasoning goes as following: (a) We don't have a standardized, sophisticated system that would analyze packages and report on all known exploits (e.g. how anti virus programs work). There are different systems that catch different things. (b) Until that exists we need to rely on different sources helping. One of which is manual auditing. I'm sure there are also other tools/websites that monitor new releases and analyze them. (c) All mentioned in (b) will need time since they are async. So spreading out releases gives them time to do "their thing" before an exploit is "everywhere". That's where delaying releases and dependency propagation will help.

Yes, this will not prevent all exploits and some might get discovered later (same as some viruses also bypass all anti virus programs). But it will certainly make it a lot harder and less likely to create something that auto propagates everywhere within days.

I'd personally love to see an option "only update deps that have been live for at least X days".

janl commented 6 years ago

Thanks all for your feedback on this!

I'd personally love to see an option "only update deps that have been live for at least X days".

Final (somewhat tongue in cheek) thought for the day: nobody wants to be first in a wave of attack, but if nobody goes first, we wouldn't find the issues 😂

simlu commented 6 years ago

@janl Wrt your previous post: Different people have different security requirements and for some it might be acceptable to update dependencies immediately, while others will wait a year to make sure things are stable (e.g. If you're in block chain).

Wrt lock file update idea: I'm assuming you were referring to https://github.com/greenkeeperio/greenkeeper/issues/434

However even if this worked it wouldn't solve any of the merge conflict issues. We would now be doing lock file PRs instead of package PRs.

I'm not sure what is required under the hood. What would it take to implement the proposed feature?

janl commented 6 years ago

while others will wait a year to make sure things are stable (e.g. If you're in block chain).

GK issues & PRs are advisory only, nobody needs to merge them :)

What would it take to implement the proposed feature?

As a rough sketch: the registry-change or create[-group]-version-branch jobs would have to check if a repo is configured with a time spec for when to run updates, and then instead of executing the jobs like normal, store them in a database, so that a periodic job (see index.js for a bunch of them) can look through that database to see if any queued up jobs should be run then.

simlu commented 6 years ago

@janl Thanks for the quick reply! Looking at my suggestions, all the problems are coming from merge conflicts. Is there an easy way to recreate greenkeeper PRs if they have a merge conflict? Or is there a legitimate reason (other than overhead and work) not to do that?

janl commented 6 years ago

That’s what we’re looking to fix in #777