action-phpcs-code-review icon indicating copy to clipboard operation
action-phpcs-code-review copied to clipboard

Need to use GITHUB_TOKEN for new public actions beta

Open lucyllewy opened this issue 4 years ago • 16 comments

Related to #10, the new yml-based actions only expose GITHUB_TOKEN to forked repositories (https://help.github.com/en/articles/virtual-environments-for-github-actions), and therefore pull-requests. The old non-yml actions will be disabled after September 30, 2019 making this a required change.

The problem: https://github.com/automattic/vip-go-ci does not work with GITHUB_TOKEN, even when we assign it to the GH_BOT_TOKEN environment variable, because it tries to access the /user API endpoint with the token and fails because it does not have permission for that request.

lucyllewy avatar Aug 27 '19 23:08 lucyllewy

+1 having trouble here too

sc0ttkclark avatar Oct 07 '19 15:10 sc0ttkclark

Hi,

I maintain vip-go-ci. What can we do to fix this issue? Would support for reading configuration from environmental variables help?

gudmdharalds avatar Jan 06 '20 16:01 gudmdharalds

Hi @gudmdharalds ,

The issue is that Github Actions only allows access to the GITHUB_TOKEN secret (i.e. NO other secrets are permitted) when an action is triggered by a pull-request. The problem is triggered when vip-go-ci tries to access the /user API endpoint which is not permitted by the token stored in GITHUB_TOKEN and so the whole thing blows-up with an access-denied.

lucyllewy avatar Jan 10 '20 14:01 lucyllewy

The problem is triggered when vip-go-ci tries to access the /user API endpoint which is not permitted by the token stored in GITHUB_TOKEN and so the whole thing blows-up with an access-denied.

I understand. So skipping this request would solve it? Are there any other issues you think?

gudmdharalds avatar Jan 13 '20 17:01 gudmdharalds

I'm unsure whether other accesses of the API will be similarly restricted. I believe the GITHUB_TOKEN pretty much only gives read and write to the current repo (the fork if it's a PR, not the forked-from repo) and not to any users.

lucyllewy avatar Jan 14 '20 17:01 lucyllewy

All GitHub secret keys (including custom ones) are withheld from forked repos by design for security. But yeah it’s a bummer in these cases.

sc0ttkclark avatar Jan 15 '20 15:01 sc0ttkclark

Hi @diddledan @sc0ttkclark thanks for reporting this issue and discussing it. @gudmdharalds thank you very much for jumping in to help. It is very generous of you :bowing_man:

There have been multiple GitHub support tickets as well as lots of proposals to get a secure way to share trusted secrets or some similar solutions to get linting, code coverage and other basic GitHub actions to work with forked repos. Unfortunately for us there is no concrete solution yet.

Please refer: https://github.community/t5/GitHub-Actions/Token-permissions-for-forks-once-again/m-p/34160 https://github.community/t5/GitHub-Actions/Allow-secrets-to-be-shared-with-trusted-Actions/m-p/34278#M1862 And many more if you just google search about it.

Testing available options

Taking reference of GitHub actions PR event for forked repos and the fact that GITHUB_TOKEN is available to forked repos. I initially tried using GITHUB_TOKEN itself on a base repo for PHPCS action. It failed because fetching the user info of GITHUB_TOKEN fails.

I then made a fork of vip-go-ci, removed this code and other places where that code's data was being used for testing. Plugged in the forked code in phpcs-code-review-action and it worked for the base repo. Could make comments with GITHUB_TOKEN, no need of GH_BOT_TOKEN. But, unfortunately again, as mentioned previously by @diddledan GITHUB_TOKEN only has read access for PRs coming for forked repos, this edit could not help.

Outcome from testing

  1. From the above test, it was clear that the use of GITHUB_TOKEN does not solve the issue of forked repos directly yet.

  2. An update in vip-go-ci code, an option to skip fetching user info from token can help in removing the creation of bot user for the use of GH_BOT_TOKEN (at least in public repo, have not tested the private repo scenario yet). Natively the github-actions bot can be used to make the review comments.

Exploring possibilities

  1. Add an option in vip-go-ci code to skip sending comments to GitHub PR and print those comments in log itself. This option can be used when PR from forked repo is detected in the action run.

  2. Waiting for some response from the side of GitHub. Actions is still very young, there is a good chance that something might come up soon that can help solve this problem because a lot of people are facing this issue in multiple GitHub actions.

@gudmdharalds what are your thoughts on option 1? Is it a viable option? If you see any possibility in that, let me know. I would love to contribute to vip-go-ci as well work on this patch.

mrrobot47 avatar Jan 18 '20 07:01 mrrobot47

Hi,

Sorry for the delay, I have been busy with other things. I will look into this a bit more and reply in a few days.

Thanks and sorry for the inconvenience.

gudmdharalds avatar Jan 23 '20 16:01 gudmdharalds

Any update on this? Can't use GH_BOT_TOKEN for forked pull request @gudmdharalds @mrrobot47

kowsar89 avatar Mar 21 '21 13:03 kowsar89

Hi all,

My apologies for the very late follow up here.

I've been looking into this today, and I think we should be able to make adjustments.

First, regarding:

An update in vip-go-ci code, an option to skip fetching user info from token can help in removing the creation of bot user for the use of GH_BOT_TOKEN

This should be possible, maybe this would be triggered via --github-actions=true option on the command line. Essentially, with this option, vip-go-ci would not make any /user API calls.

Second, regarding:

Add an option in vip-go-ci code to skip sending comments to GitHub PR and print those comments in log itself. This option can be used when PR from forked repo is detected in the action run.

This should be possible, maybe using the same option as above.

Would this have to be universal, or just with particular Pull-Requests? If it is particular Pull-Requests, how can we detect this?

gudmdharalds avatar Mar 23 '21 19:03 gudmdharalds

I am not sure I understand this fully but I am running into the following:

  • I maintain a repo where many users commit their code (bummer: that is what GitHub basically is all about)
  • I use the rtCamp workflow action for WP Coding standards
  • It requests to have a GH_BOT_TOKEN set. We have that all set up (env: GH_BOT_TOKEN: ${{ secrets.GH_BOT_TOKEN }}) and we have our token set in a bot and added it to the repo
  • Whenever a commit comes from a user (thus, from a fork, this is just how GitHub works!)... that action fails with [ERROR] Secret GH_BOT_TOKEN or VAULT_TOKEN is missing. Please add it to this action for proper executio. Yet the token is there.

Am I understanding it correctly that this entire setup can only be used for commits that are not from forks? Therefore the entire workflow is useless. Of course a GitHub repo will never only get commits from within. 99% of all commits are probably from outside (forks) I have to deny the direct commit to each contributor, create one new commit on my own with their code and then commit it from within? Because that works just fine.

I am a bit surprised about this. Can't be that a workflow basically steps the most powerful and core feature of GitHub from working, which is actual forking > committing back to upstream

Is there some solution to this? Am I misunderstanding the issue?

Thank you!

smileBeda avatar Aug 14 '22 04:08 smileBeda

@smileBeda

Note: With the exception of GITHUB_TOKEN, secrets are not passed to the runner when a workflow is triggered from a forked repository.

https://docs.github.com/en/actions/security-guides/encrypted-secrets#using-encrypted-secrets-in-a-workflow

For security reasons, secrets like GH_BOT_TOKEN are not passed to runner when forked repo triggers the action. Hence the missing token error. As for GITHUB_TOKEN, it is passed only with read permissions, so it will not be able to generate comments.

The only option for forked repos is option 1 under Exploring possibilities in this comment. which is yet to be implemented.

mrrobot47 avatar Aug 15 '22 14:08 mrrobot47

@gudmdharalds thanks a lot for your reply and apologies from my end as well for the late reply. For the past year, I have not been very active on open source due to personal reasons.

This should be possible, maybe this would be triggered via --github-actions=true option on the command line. Essentially, with this option, vip-go-ci would not make any /user API calls.

Perfect, this will work. If possible, I will work on this and create a PR when time permits.

Add an option in vip-go-ci code to skip sending comments to GitHub PR and print those comments in log itself. This option can be used when PR from forked repo is detected in the action run.

This should be possible, maybe using the same option as above.

Agree.

Would this have to be universal, or just with particular Pull-Requests? If it is particular Pull-Requests, how can we detect this?

This will be just in case of Pull-Requests which come from forked repositories. We can detect if a PR is from fork or not via GITHUB_EVENT_PATH env variable, which contains json with details of the action event.

mrrobot47 avatar Aug 15 '22 14:08 mrrobot47

Thanks @mrrobot47 for your reply.

I just cannot believe that GitHub Actions in general cannot be used - it is the core of GitHub to allow PR from forked repos. No, more correct: It is required to fork to make a PR unless you have direct access to a project!

So we are all doing something wrong here. It cannot be that entire scans (like this one) are plain simple unusable.

I am thinking - should we allow PR from Forks into a specific branch (say develop) and not scan at all there (skip scan), and then scan when we merge develop into master (which will be done by someone from within and from a internal branch, not a fork.

Would this work?

I truly cannot believe that possibly thousands of actions simply fail because of a comment not being "postable"?

Thank you for your input.

smileBeda avatar Aug 16 '22 01:08 smileBeda

Maybe if the action itself throws a failure with all the lines/messages at the end, if no token available. That way the action still runs on external PRs and we can depend on the pass/fail metric of that as a PR merge requirement still.

sc0ttkclark avatar Aug 16 '22 13:08 sc0ttkclark

@sc0ttkclark agree with you, that is exactly what is proposed under option 1 of Exploring possibilities in this comment.. So, that instead of trying to send comments to PR, it will log them in GH action run logs and pass/fail accordingly.

It is yet to be implemented. Please refer this comment

mrrobot47 avatar Aug 16 '22 13:08 mrrobot47