nodejs / security-wg

Node.js Ecosystem Security Working Group
MIT License
501 stars 122 forks source link

Automate updates of all dependencies #828

Closed mhdawson closed 1 year ago

mhdawson commented 2 years ago

PR's like this are really hard to validate and should probably be done through automation.

https://github.com/nodejs/node/pull/44283

@RafaelGSS,@facutuesca is that something you could add to your do list?

https://github.com/nodejs/node/blob/main/doc/contributing/maintaining-root-certs.md)

richardlau commented 2 years ago

FYI if you are not aware there is a PR to add a workflow for updating the timezone information: https://github.com/nodejs/node/pull/43988.

Updating ICU itself is sometimes done together with V8 updates if V8 has bumped ICU versions.

mhdawson commented 2 years ago

@richardlau thanks for pointing that out.

@RafaelGSS, @facutuesca maybe what we should focus on is looking at all of the dependencies, if upates are automated and if not identify which ones should be, and prioritize which ones we'd want to automate. (For example I think we had some discusssions around openSSL, but I think we should track/work on them as an overall program to ensure progress).

If that makes sense to you two I might update the title of this issue to be more about doing it in general for all of the dependencies.

RafaelGSS commented 2 years ago

That makes sense to me.

facutuesca commented 2 years ago

@mhdawson I'll start working on this (looking at all dependencies and see which ones we could update with a script). Should we change the title of the issue to match that?

mhdawson commented 2 years ago

@facutuesca thanks, I've updated the title.

mhdawson commented 2 years ago

I updated the first part of the issue to have the list of deps along with checkboxes. We can track progress there in terms of which ones we have automated versus not so far.

mhdawson commented 2 years ago

@BethGriggs if you have any insight/suggestions of what we might need/want to include in the automation based on what you have leared about SALSA, that info would be good to factor into how we do the automation.

facutuesca commented 2 years ago

@mhdawson

The following dependencies are already updated automatically via a Github action:

The following have a script + docs on how to update them (but no GH Action):

The following have only docs on how to update them:

Finally, the following don't have any docs/scripts/etc:

RafaelGSS commented 2 years ago

I'd go with the ones that are often updated, such as OpenSSL / ICU / zlib

richardlau commented 2 years ago

ICU doesn't update very often (about once a year), although we do now have an automated workflow for updating the timezone information (which updates more often) in the ICU data file.

I believe the npm team have their own automation to push npm releases into Node.js core.

mhdawson commented 2 years ago

I'd agree that starting with the ones we update most often would be good, and in particular OpenSSL. I think we have a few starting points. I'd written up https://github.com/nodejs/node/issues/42395 and I think that @RafaelGSS had also done some work on that front as well.

I also think that working on the list for which we have no instructions is also a priority as I see not having that documented as a risk we might get it wrong if we do have to do an update.

mhdawson commented 2 years ago

@facutuesca and thanks for the good categorization, it's good to be able to look at the overall list like that.

richardlau commented 2 years ago

Root certificates is another thing we could add to the list: https://github.com/nodejs/node/issues/45477 Update process: https://github.com/nodejs/node/blob/main/doc/contributing/maintaining-root-certs.md

FWIW Adoptium have some automation for something similar for Termurin Java builds (thanks @sxa for the pointer 🙇): https://github.com/adoptium/temurin-build/blob/master/.github/workflows/ca-cert-updater.yml I believe their version of mk-ca-bundle.pl differs from ours and they also have less information in their commit messages regarding the certificates removed/added or NSS version the update is based on.

mhdawson commented 2 years ago

@richardlau thanks for pointing that out. I think starting with automating the root cert updates would be a good thing to start with.

facutuesca commented 1 year ago

So far we have added:

Currently on review:

cc @RafaelGSS

mhdawson commented 1 year ago

@facutuesca thanks for progressing this and the update.

marco-ippolito commented 1 year ago

we can update the list since nghttp2 is now updated with action with this pr https://github.com/nodejs/node/pull/46700

RafaelGSS commented 1 year ago

@marco-ippolito I've selected llhttp as completed, but IIRC we use a different major of llhttp on v16/v14, so this automation wouldn't solve all the lines.

Maybe bring this discussion up in the next security-wg call? We might find some ideas.

Note, the case of llhttp happens to OpenSSL and I assume to other libraries as well.

marco-ippolito commented 1 year ago

Let's discuss about it in the next security-wg I'm happy to work on it

marco-ippolito commented 1 year ago

feel free to edit if I made a mistake update on dependecy status:

The following dependencies are already updated automatically via a Github action:

The following have only docs on how to update them:

Finally, the following don't have any docs/scripts/etc:

richardlau commented 1 year ago

Root certificates is another thing we could add to the list: nodejs/node#45477 Update process: https://github.com/nodejs/node/blob/main/doc/contributing/maintaining-root-certs.md

Opened a PR for updating the root certificates: https://github.com/nodejs/node/pull/47425

mhdawson commented 1 year ago

@richardlau thanks! Great to see that automated.

Ranieri93 commented 1 year ago

Hi everyone! As @marco-ippolito suggested, I'm currently working into uvwasi automation

RafaelGSS commented 1 year ago

UPDATE from #945

tniessen commented 1 year ago

histogram (version header file missing I've opened a PR to include it)

FWIW, the version header file is not necessary. You can take a look at my approach from https://github.com/nodejs/node/pull/47482, which uses some git magic to figure out if anything changed between the latest upstream version and what's in the node repository. It doesn't actually matter what version is in the node repo. The only difference to https://github.com/nodejs/node/pull/47482 is that for googletest, we follow HEAD, whereas for HdrHistogram_c we probably want to use the latest published release.

marco-ippolito commented 1 year ago

Updated by github action:

Finally, the following don't have any docs/scripts/etc:

PR for dependencies overview: https://github.com/nodejs/node/pull/47589

One left to go 🥳

tniessen commented 1 year ago

There appears to be an issue with nghttp3 updates, see https://github.com/nodejs/node/pull/47576#issuecomment-1537382159.

marco-ippolito commented 1 year ago

Last dependency update automation https://github.com/nodejs/node/pull/48171

marco-ippolito commented 1 year ago

and it's merged 🎉🎉🎉🎉🎉🎉 all the dependencies are automated 🥳

UlisesGascon commented 1 year ago

Bravo!!