Reusable workflows for npm publish #443
Conversation
sheplu
force-pushed
the
rfc-reusable-workflow
branch
from
9ce2f94 to
e2e6827
Compare
sheplu
changed the title
|
@expressjs/express-tc to be reviewed for last comments or changes - I already have the PR ready to be opened on most of the repo (but we can merge and test that by waves) |
|
PR containing the reusable workflow, readme and documentation https://github.com/expressjs/ci-workflows/pull/1/changes |
sheplu
force-pushed
the
rfc-reusable-workflow
branch
from
e2e6827 to
c714894
Compare
sheplu
force-pushed
the
rfc-reusable-workflow
branch
from
c714894 to
59697fa
Compare
|
|
||
| ### Proposal | ||
|
|
||
| - Create and maintain centralized reusable workflows under a repository such as: |
There was a problem hiding this comment.
Why one repo for everything instead of as an action that does one thing?
| - name: publish | ||
| run: | | ||
| export NODE_AUTH_TOKEN=${{ secrets.NPM_PUBLISH }} | ||
| npm publish --otp ${{ steps.wait-for-secrets.outputs.OTP }} --access public |
There was a problem hiding this comment.
Why 2FA instead of trusted publishing? Should we wait for secrets for both the token and 2FA, to promote people creating one off tokens as needed instead of having to keep dozens of granular tokens up to date and rotated?
| types: [released] | ||
|
|
||
| permissions: | ||
| id-token: write |
There was a problem hiding this comment.
Do we need this if we aren't using trusted publishing?
|
|
||
| jobs: | ||
| publish: | ||
| uses: expressjs/ci-workflows/.github/workflows/release.yaml@v1 |
There was a problem hiding this comment.
If it's a repo of different workflows, should versioning by prefixed by the "workflow"?
| name: Publish package | ||
| on: | ||
| release: | ||
| types: [released] |
There was a problem hiding this comment.
Should we use tags instead of releases? It might be confusing to create a release and have people try and install it before it's available.
| - Implement and document reusable workflows. | ||
| - Configure organization-wide secrets (npm token, GitHub token, etc.) if required. | ||
| - Roll out reusable workflows in prioritized repositories. | ||
| - Establish contribution guidelines and versioning strategy for workflow updates. |
There was a problem hiding this comment.
Would/could this repo that's created operate under the existing "captain" model?
| ### Technical Considerations | ||
|
|
||
| - Use `workflow_call` and workflow permissions properly. | ||
| - Use only organization-level secrets, not per-repo secrets for shared steps. |
There was a problem hiding this comment.
How would this work for the NPM token? Which user is publishing? Do we need to create a new shared user if we're using tokens? That feels like it opens up a new vulnerable space.
|
|
||
| ## Prior Art | ||
|
|
||
| - GitHub officially recommends this pattern for large organizations. |
There was a problem hiding this comment.
When you say "this pattern" do you mean a shared workflow, or using NPM tokens and publishing from CI? Some of the doc seems to refer to creating a shared workflow repo which I think is not contentious, but other parts are specifically referring to the publish script, which is a bit more contentious.
| - name: publish | ||
| run: | | ||
| export NODE_AUTH_TOKEN=${{ secrets.NPM_PUBLISH }} | ||
| npm publish --otp ${{ steps.wait-for-secrets.outputs.OTP }} --access public |
There was a problem hiding this comment.
Who can fill in the information for the wait-for-secrets step? Does this create an exploitable vector for someone to inject a custom script or commands here? It's unlikely to be a real attack vector since they'd need the valid 2FA to still publish, but with incorrectly configured upstream permissions it seems they might be able to do something.
| - `release.yml` — publish to npm and create GitHub releases (optional approval gates) | ||
|
|
||
| ```yaml | ||
| - uses: step-security/wait-for-secrets@v1 |
There was a problem hiding this comment.
Are we comfortable adding a third-party dependency here? It would require expending the "trusted actions" of the organization too, which isn't in this proposal. It also appears that it might not be maintained with 19 open PRs and no work in almost a year.
2 participants
RFC about reusable workflows for publishing packages, defining which issues they are solving
follow-up PR for the naming of the central repository #455