Add workflow scope to github-bot token used on nodejs/docker-node

[nschonni]

Why

https://github.com/docker-library/official-images started to use GitHub Actions, so the automated PRs have started failing as the current PAT can no longer push to it's own fork that it creates PRs for.

Add workflow scope to the Personal Access Token we use on nodejs/docker-node. I'm not sure if the reverse lookup from the existing token is easy. I believe it can be found #556 (comment)

Potential risk

The risks outlined in #572 probably also apply here

[mmarchini]

cc @nodejs/tsc @nodejs/community-committee

[mmarchini]

docker-library/official-images started to use GitHub Actions, so the automated PRs have started failing as the current PAT can no longer push to it's own fork that it creates PRs for.

care to elaborate? I'm not sure I understand why it fails without workflow scope. A link to a failure (and to a discussion on that issue, if it exists) would be helpful.

[nschonni]

Sure! Here is the failure we get now. EX: https://github.com/nodejs/docker-node/runs/1449886770?check_suite_focus=true#step:5:496

/usr/bin/git push --force-with-lease fork HEAD:refs/heads/node
To https://github.com/nodejs-github-bot/official-images
! [remote rejected] HEAD -> node (refusing to allow a Personal Access Token to create or update workflow .github/workflows/munge-pr.yml without workflow scope)

The munge-pr.yml file is actually something the Docker folks added in official images https://github.com/docker-library/official-images/blob/master/.github/workflows/munge-pr.yml. Since they added it, our PAT now requires the workflow scope to push to the github-bot fork when creating the automated PR to their repo.

It's one of those oddities I haven't seen in awhile, but was common when actions started to roll out. Now a day, things like GitHub Desktop fix the tokens it generates, but pushing a commit that includes a workflow file (even just a fast-forward merge) requires the scope.

[Trott]

I'm not sure how to proceed, but I don't want to see this stall either. @mmarchini or anyone else have anything to add at this point?

[mmarchini]

@Trott this needs the usual four approvals (two from TSC, two from CommComm). I didn't approve yet because I want to understand why the workflow needs this permission (didn't have time to look into it yet, probably won't have time until late next week). If others feel comfortable approving it I would mind though.

[mmarchini]

To be clear: I'm not blocking this, just withholding my approval until I understand how the permission fit in the workflow.

[nschonni]

@mmarchini I'm not really sure what else I can add to my previous repsonse

[mmarchini]

@nschonni I think your response is enough, I just didn't have time to explore it.

[mhdawson]

I think I'm +1 on this. It seems that it increases our risk because being able to push changes to workflow files could also somebody to export GITHUB_TOKEN which in terms could provide some additional privileges.

I'm not sure we have much choice so we need to depend on the protection from where the Personal Access Token. @nschonni is that Jenkins?

[nschonni]

Not Jenkins, it's used by this GitHub Action https://github.com/nodejs/docker-node/blob/8ca0250e3aad08328fd8719bbe2f6a61ba004a2f/.github/workflows/official-pr.yml#L45 and is stored as a Repo level secret. Here is what it prints currently https://github.com/nodejs/docker-node/runs/1449886770?check_suite_focus=true#step:5:3
That Action only runs on pull_request_target, so a drive-by PR can't alter anything unless the PR changing it lands on the default branch.

I remember there have been issues in the past with the masking breaking on Travis, but I'm not sure if GitHub Actions have had one of those events (yet).

[mhdawson]

@nschonni thanks, I figured it was either Jenkins or GitHub actions.. I'll reaffirm my +1

[targos]

+1. This token is only used to update a fork and to open a pull request. If a workflow file was changed by a malicious actor, that would not pass the PR code review.

[mmarchini]

(still needs two more +1s @nodejs/tsc @nodejs/community-committee)

[mmarchini]

oh I see, since GitHub doesn't have an option to sync forks automatically, if there are changes on docker-library/official-images workflows, it will fail. I really wish GH had an auto-sync option by default :(

Ok, so this is more risky than #572 because it actually pulls code from a repository outside of the org. @nschonni do we need to keep Actions enabled on the fork (nodejs-github-bot/official-images)? If we can disable Actions I'm fine adding the scope to the PAT (it might be disabled already, I don't see any runs there).

[mmarchini]

Since they added it, our PAT now requires the workflow scope to push to the github-bot fork when creating the automated PR to their repo.

the scope is only needed when a workflow file is changed upstream. it doesn't matter which file it is.

[ljharb]

@mmarchini i use this action: https://github.com/wei/pull to keep forks up to date automatically.

[mmarchini]

@ljharb does it need workflow scope for when a workflow file is updated? Or do you just use github-provided token?

[mmarchini]

It looks like it have the same issue: wei/pull#249 (comment)

[nschonni]

in our case, we checkout the upstream repo for creating the patch anyway, so syncing forks isn't an issue in this case.

Ok, so this is more risky than #572 because it actually pulls code from a repository outside of the org. @nschonni do we need to keep Actions enabled on the fork (nodejs-github-bot/official-images)? If we can disable Actions I'm fine adding the scope to the PAT (it might be disabled already, I don't see any runs there).

Turning off the actions on the fork repo is fine, because it's the Action on nodejs/docker-node that actually does the work

[joyeecheung]

+1 from me

[mmarchini]

@nschonni disabled Actions on nodejs/official-images and created a new PAT with workflow scope for nodejs/docker-node (updated the existing secret with that token, so you shouldn't need to make any changes to the Action for it to work). I'll close this issue, please let me know if it doesn't work.