Open IamLizu opened 3 months ago
Good point @IamLizu! I was having similar issues recently with other actions that are generating PRs, I was thinking that maybe it is a GitHub issue, but maybe we need to do some changes in our side :thinking:
Maybe we need to define a pull-requests
level access (Docs).
Do you want to work on it?
@UlisesGascon sure, I will be happy to sort this one out.
Edit: There's another thing, do you think we should only call the addLabels
when we labelsToAdd
is not-empty?
This workflow is still failing, as can be seen in the following PR (#1575) with the same error.
Strange, could it be related with who is triggering the action run? I remember that @UlisesGascon ran one manually and it worked.
Maybe updating actions/github-script@v5
to the latest version could resolve these errors by using a more up-to-date version of Node.js.
Edit: https://github.com/expressjs/expressjs.com/actions/runs/10464179650/job/28977347683?pr=1554
I've noticed that in some cases, the script tries to add all the labels for the translation when only a single image or a simple page has been modified, that's what I've seen in the logs.
Some actions:
Edit: I tested it on my repository in this PR (https://github.com/bjohansebas/expressjs.com/pull/3), and it does exhibit that behavior. Should this be happening, or is it a bug? Additionally, it's curious how the pipeline works, which makes me think it might be a repository permissions issue or, as @IamLizu mentioned, it could be related to who is triggering the action run.
Hey 👋
While I was working on a personal project yesterday, I have faced this as well. It appears we need to specify GitHub token in the workflow and the GitHub token should have all required permissions.
I will go ahead and create a PR for this one. I hope we can later check our existing token or setup if needed by someone who has access.
I checked the execution of the action
is not running with write permission
Hi @carlosstenzel 👋
is not running with write permission
Yes, I think that even after specifying write
for pull_request the GITHUB_TOKEN
is being generated for a read-only scope for the PRs for those who do not have write access to the repository.
Assuming that, I suggest that we specify a PAT for this. I had actually implemented this PAT approach in one of my repos recently. There might be other ways, I am open to other instructions or thoughts.
In case we move with the PAT approach, I have push a fix for this specifying the PAT token in the workflow like I did in my repository. But I will need to know the name of the PAT token. PAT has to have necessary permissions and should be set in repository secret.
And if someone else wants to do this another way other than PAT, please feel free to open a PR. I will assign it to you.
cc: @jonchurch @crandmck
Hey @carlosstenzel 👋
Sorry I guess I missed this one somehow. Thanks for marking that related comment, and yes, it is what's happening so far. I agree with you as I have already said this in my earlier comments.
I wonder if its possible to solve without using a PAT.
@IamLizu I think there is no other way to solve it unless they fix the repository/organization permissions or use the PAT as you suggested
Agree with the PAT option In terms of security, creating a Personal Access Token (PAT) is better than configuring the default GITHUB_TOKEN, the GITHUB_TOKEN can be used for other (bad)purposes and pass in PR review. By using a PAT specifically for this purpose, it becomes easier to detect any misuse in pull requests and simpler to revoke the token if we encounter any problems or leaks.
cc: @expressjs/express-tc
I have noticed that nodejs has a process for requesting classic access tokens when needed, https://github.com/nodejs/admin/blob/main/request-an-access-token.md
It's interesting that despite having write permissions, it still doesn't work.
The fine-grained token must have at least one of the following permission sets:
"Issues" repository permissions (write) "Pull requests" repository permissions (write)
https://docs.github.com/en/rest/issues/labels?apiVersion=2022-11-28#add-labels-to-an-issue
@bjohansebas
please, add a modified translation page, to test the action in https://github.com/expressjs/expressjs.com/pull/1642
This has to do with permission model for PRs originating from Forks in actions. docs
You can see the Action history that any PR which is a branch in the repo itself has the right perms.
After looking into this, idk that there's a safe way to do this tbh. We can use pull_request_target
to give forks the right permissions, but then they could possibly leak the token w/ those elevated perms. Same w/ the PAT approach, could trigger a workflow run, alter the workflow fil ein the PR, leak the token.
A github bot or github app might be the safest way to do this, can edit the PRs w/o having elevated permissions in a workflow file that can be altered by PRs coming from forks.
I think we should have our own GitHub bot, there are many things that can be automated (like documentation between organizations) and it would avoid security risks like the one with pull_request_target
.
As seen in #1552, the translation.yml gives the following error,
I assume the token used to run this action has permission problem? Perhaps we should give full access to repo?