search

nodejs / node-core-utils

CLI tools for Node.js Core collaborators
https://nodejs.github.io/node-core-utils/
MIT License
234 stars 106 forks source link

chore: auto-publish package to npm after release #511

Closed targos closed 2 years ago

targos commented 3 years ago

Labeled "do not land" because we need to setup the npm token if this is accepted.

codecov[bot] commented 3 years ago

Codecov Report

Merging #511 (1bd61fe) into main (5e85166) will increase coverage by 0.06%. The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #511      +/-   ##
==========================================
+ Coverage   84.10%   84.16%   +0.06%     
==========================================
  Files          37       37              
  Lines        4051     4067      +16     
==========================================
+ Hits         3407     3423      +16     
  Misses        644      644
Impacted Files Coverage Δ
lib/auth.js 87.60% <0.00%> (+1.88%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5e85166...1bd61fe. Read the comment docs.

mmarchini commented 3 years ago

This would remove 2FA when we publish. Are we ok with that?

targos commented 3 years ago

npm now has automation tokens now, so we don't have to disable 2FA requirements on the package. AFAIU the only risk we would have is that someone with write access to this repo pushes something to steal the token?

mmarchini commented 3 years ago

We still drop 2FA for our "main" publish workflow (assuming the automation becomes the main workflow). It's not ideal IMO but probably low risk enough that we can try it?

targos commented 3 years ago

GitHub Actions now have environment protection rules and environment secrets: https://github.blog/changelog/2020-12-15-github-actions-environments-environment-protection-rules-and-environment-secrets-beta/ Maybe that can be used to protect an npm token and/or only allow a subset of collaborators to publish the package?

targos commented 2 years ago

I will reimplement the change differently if it's likely to land.

targos commented 2 years ago

Updated. We still need to setup an npm automation token before merging this. Who would be able to do that?

targos commented 2 years ago

The NPM_TOKEN secret is installed. Would you like to review again?