starter-workflow template gives `Resource not accessible by integration` about labeler HOT 80 CLOSED

What is the end solution?

from labeler.

tl;dr, change

on:
- pull_request

to

on:
- pull_request_target

GitHub has introduced a new event type: pull_request_target, which allows to run workflows from base branch and pass a token with write permission.

In order to solve this, we’ve added a new pull_request_target event, which behaves in an almost identical way to the pull_request event with the same set of filters and payload. However, instead of running against the workflow and code from the merge commit, the event runs against the workflow and code from the base of the pull request. This means the workflow is running from a trusted source and is given access to a read/write token as well as secrets enabling the maintainer to safely comment on or label a pull request. This event can be used in combination with the private repository settings as well.

from labeler.

Since this action doesn't work for a lot of users including me, I created some sort of alternative at https://github.com/marketplace/actions/periodic-labeler. This isn't anything fancy and the workaround is to just run it periodically from master branch. Configuration is the same as in action located here, so migration should be simple. I am already using it in @cloudalchemy org.

from labeler.

Apparently, GitHub is treating dependabot prs like forks. Here is the related article. As @TobKed has suggested, I used the following snippet on the top level of my workflow to grant the needed permissions. So thank you very much.

permissions:
  # All other permissions are set to none
  checks: write
  contents: read
  pull-requests: write

Additional question to @TobKed assuming that the repository will always be private and I will only use dependabot to update private submodules that my repository is dependent to, do you think it is also possible/safe to use pull_request_target as my event trigger without adding any permissions like you have suggested?

from labeler.

Hm, so it looks like this is an issue with forks:
works fine on my branch
fails on fork

I'll reach out internally and figure out if that's expected scoping of permissions for the GITHUB_TOKEN and we can go from there

from labeler.

@damccorm option 2 certainly sounds far more useful for us - our workflow has every developer on our project making PRs from their forks (and presumably that's a fairly common workflow).

from labeler.

Hello @damccorm, Is there any way or workaround to trigger the action on the base repository so that the GitHub Action token will have both read/write access when there is a PR from the forked repository. And there is no need to re-write the whole code base.

from labeler.

My proposal would be to have a flag for the repository admin to enable read/write access only to the trusted actions.

I'm completely okay with forks having the ability to apply labels and no ability to do anything else like create labels or any other write action. If someone abuses applying labels then I'll ban them.

from labeler.

So unless I'm misunderstanding, this action doesn't work for the 90% the most common (fork, branch, push to PR) github workflow 😖

Yeah, believe me you're not the only one having a solid WTF moment.

from labeler.

I'm completely okay with forks having the ability to apply labels and no ability to do anything else like create labels or any other write action. If someone abuses applying labels then I'll ban them.

Indeed. We'll take a closer look at this for sure; I just wanted to explain why it's not trivial and something that we want to make sure that we get right when we do.

from labeler.

I have created a Github app (based on Probot) that takes similar configurations and does the same job and works for Forked PRs too.

More information: https://github.com/kaxil/boring-cyborg
App Page: https://github.com/apps/boring-cyborg

We are already using it on Apache Airflow repo.

from labeler.

Option 1 makes this action useless, as whoever who has write access can put the labels themselves as necessary, though in a repo with 90 percent or more contribution through forks, we really wanted to use this action to reduce manual work.

Why can't you just use the default branches labeler.yml as canonical config?

from labeler.

We've enabled this for our repository too and are facing the same issue. Would be good to at least define some way to handle this in the workflow, so we can customize the logic.
Of course the best option would be to support what was already suggested by @ibakshay:

...so that the GitHub Action token will have both read/write access when there is a PR from the forked repository. And there is no need to re-write the whole code base.

from labeler.

@ethomson Thank you very much for the detailed response. I totally agree that read/write access should not be given for the PR to all the actions for security reasons.

My proposal would be to have a flag for the repository admin to enable read/write access only to the trusted actions.
This issue is raised multiple times in the GitHub Community Forum.

from labeler.

Could someone please reopen this? I know the "bug" doesn't come from actions/labeler itself, but this is such a major limitation that someone at GitHub should find a way to make this work!!

Seriously, having running actions from forks in a read-only environment makes actions pretty much useless! You can't run any kind of CI / lint / action really. Well, you can, but you cannot report it within the PR itself, which makes the entire thing pointless!

from labeler.

I'll have to respectfully disagree. PRs should at the very least be able to act on themselves…

from labeler.

@potiuk I suggest you read the comment I wrote on GitHub Community, it explains it all 😄

from labeler.

I'm experiencing this in an enterprise repo with PRs opened by dependabot. As far as I can tell, dependabot creates a branch, not a fork. Is there a proper/secure way to get this action to work for dependabot PRs?

from labeler.

Thanks for investigating, @damccorm. And the update seems like a good alternative for my use at least. Others can reopen this if necessary, but I'll close it as it's expected behavior.

Just pinging you, @squidsoup to loop you in.

from labeler.

I spoke too soon. Switching this to cron by itself won't work because we assume its going to be run on a PR. Trying to figure out what makes the most sense here. Option 1 is to just not add labels to forks (but we shouldn't throw like we do now regardless). Option 2 is to update it to filter through all pull requests.

In theory I like option 2, but we need to be careful or we'll get rate limited - along those lines, we need a way of skipping PRs that we've already processed - maybe we could add a "triaged" label or something, but that's kind of ugly.

Thoughts? My instinct is to start with option 1 - better error handling on forks - and then move on to option 2 as appropriate

from labeler.

Yeah, agreed option 2 makes a lot more sense.

Why can't you just use the default branches labeler.yml as canonical config?

The issue isn't getting the labeler.yml, the issue here is that workflows that run on forks don't have write permissions (so they can't create labels)

from labeler.

Hello @damccorm, Is there any way or workaround to trigger the action on the base repository so that the GitHub Action token will have both read/write access when there is a PR from the forked repository. And there is no need to re-write the whole code base.

Workflows are run in the context "of the repository". But when any workflow is run because a pull request triggered it, then the token is downgraded to a read-only token.

It's not "the fork's token", it's still the repository's token, it's just read-only.

This is intentional. It's so that somebody can't open a pull request that gives them a token that they could use to write to your repository.

We'll continue to investigate workarounds, but the security model here is important to keep in mind.

from labeler.

@ethomson wouldn't it be possible for the token to have the same permissions as the person who initiates the action? That would prevent labels to be created anyway, but comments, for instance, shouldn't be a problem, since any one has got permission to create them.

We'll make some changes to this to make it easier to use, but given the security impact, we're analyzing this carefully.

@smay1613 surprisingly, you can grab your GitHub token and create an external request using curl, for instance, like this answer points out.

The token is still read-only, no matter how you use it. Its ability to be exfiltrated is why we've made it read-only for forks.

from labeler.

Thanks everybody for the commentary, I appreciate that this isn't helpful for many of your workflows. We're investigating some changes to token permissions to help enable fork-based workflows, but I don't have an ETA on that.

from labeler.

@Toflar The problem is this is a SUPER common use case - I assumed the whole reason this workflow was listed so prominently was FOR this exact use case - and the docs say nothing to the contrary. Can we please merge #50? That would have saved me like an hour of time.

@ethomson

from labeler.

GitHub actions if they are not incredibly broken (seems perhaps they are working as designed) then they are at the very least marketed and misrepresented horribly by GitHub. I wish I had back the hour or two I wasted trying to do anything useful with Actions on a OSS project. I'm sure they are working for someone, but I wish GitHub would write better docs and market them in a more responsible fashion.

It seems GitHub knows enough about a project (based on all the other info it provides the admin) that it could make better choices about suggesting someone use these tools if there was zero chance they were going to fit the workflow in question. @Ecco I feel your pain.

from labeler.

I'm not sure we're saying exactly the same thing. I don't want to be too explicit about what the problem is here exactly - or how it needs to be fixed, but there is a big problem with Actions (for many people).

from labeler.

So it turns out that this is working as intended after all. We can't give write permissions to forks for security reasons (e.g. the forked user changes your yaml file to write bad things to your repo), so this should fail on forks.

With that said, the docs are wrong here and need to be updated. Already added actions/starter-workflows#78 to update the template, will also follow up to update docs here.

I'm having this issue after transferring a repository to an organisation, no forks

from labeler.

@OndroMih

It looks like a solution is to trigger the action on the pull_request_target event instead of the pull-request event.

This may be unsecure, PTAL at Keeping your GitHub Actions and workflows secure Part 1: Preventing pwn requests

TL;DR: Combining pull_request_target workflow trigger with an explicit checkout of an untrusted PR is a dangerous practice that may lead to repository compromise.

It was solved in another, more secure way (different action used, bur rule is the same) in https://github.com/apache/incubator-doris/ and https://github.com/apache/airflow. PTAL on usage:

• https://github.com/apache/incubator-doris/blob/c2520c878c3d827e27c32f1dab5e5953a25b52e7/.github/workflows/approve-label-trigger.yml
• https://github.com/apache/incubator-doris/blob/c2520c878c3d827e27c32f1dab5e5953a25b52e7/.github/workflows/approve-label.yml

from labeler.

I am also having the same issue on the PRs opened by dependabot. The problematic action is supposed to generate a test report and update the check statuses. Any ideas for a workaround?

Also @igor-petrik-invitae were you able to figure out a way to make the actions work with dependabot?

from labeler.

@dgokcin it is possible to use pull_request_target (actually I started to use it like that in my private repositories recently). Is it safe? I think yes, but I am not 100% sure about that. If it is related to some critical code I would suggest consultations with security specialists :)

from labeler.

I just submitted a support ticket about this, but probably should have checked here first! thanks @damccorm

from labeler.

Same problem when you try to create a comment to an issue. Which shouldn't be a problem, since anyone can do that, right?

from labeler.

So, to be clear, if I open a PR on a repo where I am not an admin, then the Labeler action should fail?

No, it's not about your permissions. The GITHUB_TOKEN has a writable token for the repository. It's about whether you're opening a PR from a fork, or whether you're opening a PR directly in the repository.

The PR from a fork does not have a writable token and will fail. The PR from a repository will succeed since the GITHUB_TOKEN has write permissions.

from labeler.

I don't understand this... the labels exist in the main (non forked) repo, which is exactly where I want the labels. Am I missing something? I'm not seeing the issue.

When a workflow runs on a pull request event, the workflow run gets a read only token to the main repository. This is so that somebody can’t fork your repository and maliciously write to your code, create harmful issues or exfiltrate tokens. Or - I’m afraid - add labels to issues. The token is explicitly read only for security.

We’re considering some other alternatives here, but at the moment, this will not work with PRs from forks.

from labeler.

I don't understand why there's a WTF moment? This action was created trying to implement exactly this functionality but it's currently not possible because of how tokens work in GitHub Actions. There's absolutely nothing hard about this to understand^^

from labeler.

@ethomson what you said is all correct. But let's try a possible solution

A common way to update a workflow is through the pull request process. You'd want to see how your workflow works in the PR process, not have to wait until it was merged to master. You want pull request validation on the workflow itself, in this case.

One who wants updates workflow normally has write access to the repository, so he can make a pull request from the same repo.

But also, like you said, it's also not sufficient. It wouldn't catch all security issues. It would be easy enough for me to add a new test that is print "$env['SOME_SECRET']" and that would be no good.

If workflow would uses a version from the target branch, then as I said it eliminates most security concerns. But in some cases, a malicious person can hack the system by changing other files in repo (e.g. when workflow calls other scripts from the repo). A solution for this scenario could be an option in settings to restrict access in a forked pull request (i.e. as it works now) or adding new setting to specify which files have to be secured and modified version of those files must not be used in workflows of forked pull requests

from labeler.

You can see from this issue only on how many users are affected because of this limitation from GitHub Actions 😞.

from labeler.

@yyyc514 Would you mind upvoting my comment on GitHub community to try and raise awareness about this? Thanks 😄

from labeler.

This issue appears as closed but the error still happens, any hint?

from labeler.

It looks like a solution is to trigger the action on the pull_request_target event instead of the pull-request event.

I see that the README is now updated to use the pull_request_target event. I just wanted to comment about it also here because Github's template still uses pull_request and it might be useful to people that search for a solution and they come across this page.

from labeler.

Hm, haven't seen that before. Are you able to share your repo by chance (or just the relevant pieces)?

from labeler.

Hey @damccorm, thanks for the response.
Yes, sure. The repo is over at github.com/binary-com/deriv-app. Although due to the failing action, we have currently disabled it, but you can find the PR that enabled Labeler here, and an example of a failed run here.

from labeler.

So it turns out that this is working as intended after all. We can't give write permissions to forks for security reasons (e.g. the forked user changes your yaml file to write bad things to your repo), so this should fail on forks.

With that said, the docs are wrong here and need to be updated. Already added actions/starter-workflows#78 to update the template, will also follow up to update docs here.

from labeler.

Could we change the config to use the repo's token, rather than the fork's token? If this works conceptually, we (exercism pps) could experiment more.

from labeler.

I'm completely okay with forks having the ability to apply labels and no ability to do anything else like create labels or any other write action. If someone abuses applying labels then I'll ban them.

And also I don't mind if the forks have the ability to create a comment on PR or issue with trusted actions

from labeler.

So, to be clear, if I open a PR on a repo where I am not an admin, then the Labeler action should fail?

from labeler.

@logankilpatrick totally correct.

from labeler.

Any workarounds here?

from labeler.

@ethomson wouldn't it be possible for the token to have the same permissions as the person who initiates the action? That would prevent labels to be created anyway, but comments, for instance, shouldn't be a problem, since any one has got permission to create them.

from labeler.

@smay1613 surprisingly, you can grab your GitHub token and create an external request using curl, for instance, like this answer points out.

from labeler.

from labeler.

@ethomson is this token not persistent? So, hardcoding the read/write token would not work too?

from labeler.

The current workaround would be to use GitHub Apps or Probot.

from labeler.

Since this action doesn't work for a lot of users including me, I created some sort of alternative at https://github.com/marketplace/actions/periodic-labeler. This isn't anything fancy and the workaround is to just run it periodically from master branch. Configuration is the same as in action located here, so migration should be simple. I am already using it in @cloudalchemy org.

We tried to use the periodic-labeler at https://github.com/apache/airflow and it did not work. After some investigation we found that the pagination is missing in the implementation and that labeler will run only for the oldest 30 open PRs. This might get very confusing because it seems it works, but there are no visible changes in labels (old PRs are on further pages if you have a lot of them). Fix to it waits for merging - it also contains all the explanation: paulfantom/periodic-labeler#4

from labeler.

My issue is that the starter workflow defined works perfectly for pull requests. However, if one pushes a minor change directly to master - the labeler runs and fails with the exact same error. Why would on: [pull_request] fire for a push to master?

https://github.com/IMULMUL/DependencyCheck/commit/e6cd21685bd41264b6990b35f60fe2fc52690e21/checks?check_suite_id=384901058

from labeler.

The issue isn't getting the labeler.yml, the issue here is that workflows that run on forks don't have write permissions (so they can't create labels)

I don't understand this... the labels exist in the main (non forked) repo, which is exactly where I want the labels. Am I missing something? I'm not seeing the issue.

from labeler.

When a workflow runs on a pull request event, the workflow run gets a read only token to the main repository.

So under what circumstances does this actually work? If the token is always read-only how does it EVER do what its' supposed to do - add labels?

from labeler.

So under what circumstances does this actually work? If the token is always read-only how does it EVER do what its' supposed to do - add labels?

When a pull request comes from a non-fork, as may happen within organizations.

from labeler.

With #49 this action should work when scheduling, it must be committed to the master at the destination already.

from labeler.

harmful issues or exfiltrate tokens. Or - I’m afraid - add labels to issues. The token is explicitly read only for security.

So unless I'm misunderstanding, this action doesn't work for the 90% the most common (fork, branch, push to PR) github workflow 😖

from labeler.

What about using Pull Request Workflow from the target branch and not from the fork? It'd fix most security issues

from labeler.

I mean ignore version of the workflow file from the fork and use one from target branch instead

from labeler.

A common way to update a workflow is through the pull request process. You'd want to see how your workflow works in the PR process, not have to wait until it was merged to master. You want pull request validation on the workflow itself, in this case.

But also, like you said, it's also not sufficient. It wouldn't catch all security issues. It would be easy enough for me to add a new test that is print "$env['SOME_SECRET']" and that would be no good.

from labeler.

I'd be fine with a labeler action that ran on the branch periodically for issues and/or PRs. It'd be nice to be able to test the workflow from a PR, but I could live without that for now (until it's sorted out). I'm sure other users would also really (REALLY) appreciate updates to the docs, indicating that commonly used PR-based actions cannot affect labels.

Suggestion for the future:

What about when running on a PR, have the action operate in a kind of "test mode". In this mode, instead of updating labels directly, it could simply posts comments to the triggering PR indicating the labels it would have updated. Then once merged, a periodic branch-based issue/PR labeling operations can happen as is currently supported.

from labeler.

@ethomson, I have a possible solution proposal. How about having trusted Actions ?. Where the project maintainer will have the possibility to give write access to only trusted actions and can turn it off anytime If he/she feels the action is suspicious.

from labeler.

I am afraid, this is as it should be. I hit the problem myself recently - but I had to find another solution to caching problems in this case. I did not realise at first but when I imagined what "roque users" could do with write access - without code review from committers to our code - it was scary! Replacing pushed images with some "roque ones", pushing code and tags .. you name it.

I think it's a good rule and one that makes you think and try other solutions as you might simply not realise how dangerous it is.

from labeler.

I went on a (long) rant on GitHub Community about this too

from labeler.

@Ecco > what do you mean by "Act on themselves" ? As described here:
https://help.github.com/en/actions/configuring-and-managing-workflows/authenticating-with-the-github_token#permissions-for-the-github_token - the actions are per service not per PR. How would you act "on themselves" for:

1. actions, 2) checks, 3) contents 4) deployments 5) issues 6) metadata, 7) packages 9) repository projects, 10) statuses. ....

There is one case 8) pull request - that falls into "act on themselves" case but what exactly do you want for the token to allow on the pull request?

from labeler.

@Ecco I did. Comment and label. It is what I saw there.

Suggestion: Maybe you can you simply open an issue when you explicitly say "I want my Github Action can comment on the PR from the forked repo" and another " I want my Github Action can label the PR from a forked repo"?

I think such requests should be treated individually. Each of them has a different security considerations and they can have some profound impact. And maybe there is a reason for doing it this way. For example comment on a PR might in-turn trigger another action.

I've been following (actually I neede it for Apache Airfow that I am PMC member of) a similar discussion in GitLab: https://gitlab.com/gitlab-org/gitlab/issues/5667#note_223311926 and I followed all the discussions, related issues and there are many, many security considerations that you have to take into account. Switching to read-only is really reasonable before you understand and discuss all the consequences of "write" access for forked repos for any of the "actions" you can have "write" access to.

Long story short - implementing fork support for GitLab has been continuously postponed for last 6-7 months or so. And I start to believe it makes sense as those potential security problems can be really bad. I suggest @Ecco -> read discussions there. They are very insightful.

from labeler.

Is there any workaround to this in the meantime? I tried to implement a solution like ethereum/ethereum-org-website#345 but it still seems to be failing on forked repos.

dynamoose/dynamoose@5cf55f2

from labeler.

I still haven't found a solution here.

I tried adding this as a periodic cron job but hit this error:

As @damccorm mentioned, the action assumes it's going to be run on a PR & fails when it's unable to access github.context.payload.pull_request within getPrNumber().

from labeler.

I've created a github actions workflow that will automatically label PRs from the default branch of a repo based on a cron schedule. This way, PRs from forks will be labeled appropriately - https://github.com/marketplace/actions/prow-github-actions

Here's a working example: jpmcb/prow-github-actions#12

prow-github-actions has alot of other features too, so feel free to raise an issue with questions, bugs, or feature requests!

from labeler.

So there is no solution? this issue made this action totally unusable. :(

from labeler.

from labeler.

Adding the - pull_request_target is not enough. You need to go to the org/enterprise settings and do the following

from labeler.

it failed on me too...
Do you know how to fix it? frankenstein91/Arch-Ansible-Install#6

from labeler.

@igor-petrik-invitae @dgokcin is triggering labeler workflow on workflow_run by workflow pull_request_review with specified permissions like described in my comment #12 (comment) (PTAL on the airflow example) will not solve problem of permissions?

from labeler.

@TobKed I have not since I am having this issue on a branch not on a fork. The PR is created by the dependabot user. Do you think that the problem is happening because of the permisisons of the dependabot user?

from labeler.

Apparently, GitHub is treating dependabot prs like forks. Here is the related article. As @TobKed has suggested, I used the following snippet on the top level of my workflow to grant the needed permissions. So thank you very much.

permissions:
  # All other permissions are set to none
  checks: write
  contents: read
  pull-requests: write

Additional question to @TobKed assuming that the repository will always be private and I will only use dependabot to update private submodules that my repository is dependent to, do you think it is also possible/safe to use pull_request_target as my event trigger without adding any permissions like you have suggested?

pull_request_target is not safe and not recommended at all as it compares all the changes against master, which makes the tests unreliable.

from labeler.