"Error: Resource not accessible by integration" when using Dependabot

[WyriHaximus]

When using this action on a PR opened by Dependabot I still get the error in the title. Which permissions do I have to set on the workflow to make it work?

[WyriHaximus]

Opened a community issue, will make PR's when there comes a working set of permissions from there: https://github.saobby.my.eu.orgmunity/t/permissions-nesecary-to-comment-on-a-pr/179047

GitHub Support Community

Permissions nesecary to comment on a PR

Been working on a workflow that will comment based on changes in the PR. But when the PR comes from the outside or Dependabot it doesn’t have all the write permissions the repo’s owner has. Does anyone know which permissions have to be set to what to make an action be allowed to comment on a PR, when that PR isn’t coming from the repo owner?

[polarathene]

I recently split my pull_request workflow to use an additional workflow_run workflow to avoid the issue with non-collaborator contributions.

This action thankfully has the number option, which allowed me to reference the PR to comment on when pull_request event data wasn't available to the workflow (I had to transfer this via upload/download of artifacts like the blog post by Github demonstrates).

I wasn't fond of the copy/paste of scripts (especially since I have to hand off to other project maintainers), so this action + a popular download artifact action (which supports artifacts from other workflows) helped simplify that 👍

---

Try something like this:

Workflow 1 (pr-comment.yml):

name: 'PR event for comment'

on:
  pull_request:

jobs:
  prep-comment-event:
    name: 'Prep for comment action'
    runs-on: ubuntu-20.04
    steps:
      - name: 'Prepare artifact for transfer'
        run: |
          # Save ENV for transfer
          echo "PR_HEADSHA=${{ github.event.pull_request.head.sha }}" >> pr.env
          echo "PR_NUMBER=${{ github.event.pull_request.number }}"    >> pr.env
          echo "PR_TITLE=${{ github.event.pull_request.title }}"      >> pr.env

      - name: 'Upload artifact for workflow transfer'
        uses: actions/upload-artifact@v2
        with:
          name: pr-metadata
          path: pr.env
          retention-days: 1

Workflow 2 (run-comment.yml):

name: 'Comment on PR'

on:
  workflow_run:
    workflows: ["PR event for comment"]
    types:
      - completed

jobs:
  comment:
    name: 'Comment on PR'
    runs-on: ubuntu-20.04
    if: ${{ github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success' }}
    steps:
      - name: 'Download build artifact'
        uses: dawidd6/action-download-artifact@v2
        with:
          github_token: "${{ secrets.GITHUB_TOKEN }}"
          run_id: ${{ github.event.workflow_run.id }}
          workflow: pr-comment.yml
          name: pr-metadata

      - name: 'Restore preserved ENV'
        run: cat pr.env >> "${GITHUB_ENV}"

      - uses: marocchino/sticky-pull-request-comment@v2
        with:
          number: ${{ env.PR_NUMBER }}
          message: |
            Comment on PR #${{ env.PR_NUMBER }} is ready! :tada:

            Triggered via PR commit: ${{ env.PR_HEADSHA }}

You can do a build or anything else in the first workflow that doesn't require secrets, that is for running untrusted third-party code (the reason Github prevents non-collaborators PRs from having secrets available).

Then you can use upload to volley the data you need over to the 2nd workflow (this one runs from master/main branch), I show how to store some values into ENV and read in that file contents as ENV to use for the actions needing a secure context.

It's definitely not as elegant or simple as I'd want it. In my case I'm doing a deploy that needs secrets, so I doubt I can keep it as a single workflow.

[marocchino]

I made a wrap so that I can simply put it in the output from the artifact, but I'd be happy if this helps.

https://github.com/marocchino/sticky-pull-request-comment/blob/main/.github/workflows/comment_on_workflow_run.yml

GitHub

marocchino/sticky-pull-request-comment

create comment on pull request, if exists update that comment. - marocchino/sticky-pull-request-comment

[WyriHaximus]

Having a look at both, thanks! This turned out to be a lot more complicated then I thought :(

[marocchino]

I am sorry for that. 😢 This error did not exist before and is due to a policy change in GitHub. There seems to be no way unless GitHub changes thier minds.

[WyriHaximus]

Hey if anything I'm annoyed at GitHub, not at you @marocchino 😄 . Will have a few tries at it this week to come up with the most concise solution.

[polarathene]

There are some actions that help simplify the workflow example I gave further, but I try to avoid relying on third-party too much. If you do find a better solution do share here :)

[WyriHaximus]

Will do, been tweeting with someone from Dependabot yesterday about this which gave some interesting insights to make it simpler. But need to confirm a few things before testing that possibility out.

[WyriHaximus]

Came to this solution in my case, not using this action since I wanted to provide the simplest set up possible for it's users, but it should also work for this action:

name: Composer Diff
on:
  pull_request_target:
    types:
      - opened
      - synchronize
      - reopened
permissions:
  pull-requests: write
jobs:
  comment-composer-lock-diff:
    name: Comment composer.lock diff
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v1
      - name: Comment composer.lock diff
        uses: WyriHaximus/github-action-composer.lock-diff@main
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

[WyriHaximus]

Ow for reference here is the Twitter thread, thought I posted this last week already: https://twitter.com/WyriHaximus/status/1393679576828686340

Twitter

Cees-Jan Kiewiet on Twitter

“@asciimike @dependabot @github It required some fiddling, and also learned that it will always take the version from the default branch, but it works https://t.co/swjnj6GEKn 🎉🎉🎉🎉! Thanks for the pointers”

[polarathene]

Came to this solution in my case, not using this action since I wanted to provide the simplest set up possible for it's users, but it should also work for this action:

I haven't looked into what your action is doing, but it looks like there might be two concerns?

The official Github blog post that I linked earlier in this discussion explicitly discourages what you're doing with pull_request_target AFAIK?

EDIT: Can probably ignore the collapsed content below. Just make sure your users are cautioned not to recklessly modify / adopt the workflow example to their own needs without being aware of the risks that pull_request_target can pose. (I see that you already have 👍 )

Note you can use v2 checkout and get the same intent IIRC, it's just an explicit option to not get a shallow checkout. Should be documented on the action README.

---

Original comment with quotes and questions

I quote from that blog post some snippets related to two concerns, plus some extra related quotes that might be helpful to anyone not wanting to read the whole article:

Concern 1

pull_request_target should be against the target branch, thus there shouldn't be an actual diff for composer lock? I could be mistaken as I haven't verified myself, it might be the first commit is diffed.

Since you're sharing the workflow I assume it does provide some meaningful diff as intended, but did you test updating the pull request which would change the diff and find that change is correctly picked up by your workflow, or does it remain the same?

These blog post snippets seem relevant?:

We have also noticed a pattern that has a vulnerable intent of use, however due to misunderstanding of pull_request_target ends up being a broken, but not vulnerable, workflow. In these cases, repositories switched to using the pull_request_target trigger, but use the default values for the actions/checkout action.

This doesn’t do what the authors most likely intended. In case of a PR, it builds the latest changeset from the target repository. The workflow is broken in the sense that it does not actually build the PR, but luckily is not vulnerable, since there is no explicit checkout of the actual PR contents or unsafe handling of those contents.

My understanding of this is that if you're creating a PR for master branch, then when using pull_request_target event for a workflow, you would have that branches latest commit, not the PR contents? That's not the case for your composer diff?

Concern 2

Your checking out code step seems to be advised against?:

If your workflow scenario simply requires commenting on the PR, but does not require a check out of the modified code, using pull_request_target is a logical shortcut.

**EDIT: It seems I might have misunderstood as this line references an example that uses the checkout action with a ref to the pull request head commit SHA:

Unfortunately some repository workflows take this a step further and use pull_request_target with an explicit PR checkout

Thus they do explicitly checkout the PR contents instead of the target branch commit contents (which would have none of the PR afaik)

Still, you probably want to be careful when advising users with that snippet elsewhere. Be sure to make them aware that there are security risks if they modify it to include steps that are vulnerable as described in the blog post.

And I assume your custom action is just doing a diff on the composer lock file, no downloading/running of dependencies that the contributor could add to a PR, just diffing dependencies?

Other useful highlights from that linked article:

Workflows triggered via pull_request_target have write permission to the target repository. They also have access to target repository secrets.
This means the standard checkout action uses the target repository to prevent accidental usage of the user supplied code.
The intent is to use the trigger for PRs that do not require dangerous processing, say building or running the content of the PR.

That last line is specifically stating that building/running content of PR is dangerous when it's from an untrusted third-party, thus users must be careful not to do that with pull_request_target which can provide access to secrets.

Together with the pull_request_target, a new trigger workflow_run was introduced to enable scenarios that require building the untrusted code and also need write permissions to update the PR with e.g. code coverage results or other test results.

To do this in a secure manner, the untrusted code must be handled via the pull_request trigger so that it is isolated in an unprivileged environment. The workflow processing the PR should then store any results like code coverage or failed/passed tests in artifacts and exit.

If the workflow uses actions/checkout and does not pass the optional parameter persist-credentials as false, it makes it even worse. The default for the parameter is true. It means that in any subsequent steps any running code can simply read the stored repository token from the disk.

[WyriHaximus]

@polarathene Thanks for your long write up, will read it entirely later because my eye caught some interesting notes in there. About the announce the action on Twitter, but before that, I wanted to be sure of absolutely one thing: Changing the workflow in the PR doesn't run the changes made to the workflow with elevated permissions.

So I asked a friend to make a PR altering the workflow and observe the output of the workflow. The PR https://github.com/WyriHaximus/github-action-composer.lock-diff/pull/35/files makes an innocent change, but the workflow from the default branch is executed: https://github.com/WyriHaximus/github-action-composer.lock-diff/runs/2603184264?check_suite_focus=true

And also funny enough GitHub seems to suggest you use this to create comments and add labels:

This event allows you to do things like create workflows that label and comment on pull requests based on the contents of the event payload.

I Will probably add an extra note about not adjusting without reading those pages thoroughly is not a bad idea.

[polarathene]

And also funny enough GitHub seems to suggest you use this to create comments and add labels

Yeah it's fine for that afaik 👍

Just as long as you're not relying on running untrusted code (technically includes any packages that are updated and looks harmless like version bumps from dependabot),

Your diff works because your action is indirectly accessing the PR branch, that would be a no-no if a third-party contribution had any ability to manipulate that to run other code, which in this case I don't think it does (I'm not too familiar with PHP so I only glanced over it briefly).

Workflows are never run from PR changes afaik, even on pull_request event from yourself without any fork. They trigger/run from the target branch or primary/default branch iirc. workflow_run is always from primary branch.

[WyriHaximus]

And also funny enough GitHub seems to suggest you use this to create comments and add labels

Yeah it's fine for that afaik 👍

Yup, but still not happy with it, it's IMHO still to much permissions for what it really needs. Heck I wouldn't mind having to grant permissions per HTP method + path combination.

Just as long as you're not relying on running untrusted code (technically includes any packages that are updated and looks harmless like version bumps from dependabot),

This is what worries me the most, getting a supply chain attack like that is a P.I.T.A., so going to have to inspect every PR dependabot makes. Which is also where that action comes in handy :D.

Your diff works because your action is indirectly accessing the PR branch, that would be a no-no if a third-party contribution had any ability to manipulate that to run other code, which in this case I don't think it does (I'm not too familiar with PHP so I only glanced over it briefly).

Well, this is where I also had lots of fun figuring some things out as it needs both the current and the PR branch HEAD locally. (Just realised the action is missing support for fork PR's to do that properly.) So the only way to get it to work easily was to use checkout v1. Making that there is no need to have write access to contents. (The same should be possible for any other package ecosystem out there.)

Workflows are never run from PR changes afaik, even on pull_request event from yourself without any fork. They trigger/run from the target branch or primary/default branch iirc. workflow_run is always from primary branch.

Funny enough: I know this, and I still check to be absolutely sure because I don't want to be wrong with these kinds of things.

[polarathene]

Well, this is where I also had lots of fun figuring some things out as it needs both the current and the PR branch HEAD locally. (Just realised the action is missing support for fork PR's to do that properly.)
So the only way to get it to work easily was to use checkout v1.

Have I misunderstood what you've said, or is the actions/checkout README clear about getting the desired outcome with v2?:

Only a single commit is fetched by default, for the ref/SHA that triggered the workflow. Set fetch-depth: 0 to fetch all history for all branches and tags.

    - uses: actions/checkout@v2
      with:
        ref: ${{ github.event.pull_request.base.sha }}
        path: pr_target

    - uses: actions/checkout@v2
      with:
        ref: ${{ github.event.pull_request.head.sha }}
        path: pr

Something like that should checkout the target branch for a PR and the PR branch itself into subdirectories to get the composer file to diff against. Note that the PR base ref/SHA may not always be the latest commit for the target branch.

You can also as the actions/checkout README suggests, just fetch: 0 to get all history + branches like v1 instead of a shallow clone (which the example snippet above is doing for the two target refs). If you'd rather not use the PR base ref/SHA, you can add a specific branch name too. IIRC, by default if no value specified it uses the GITHUB_REF env var which value depends on context of the workflow trigger event.

---

Making that there is no need to have write access to contents. (The same should be possible for any other package ecosystem out there.)

If there isn't a need for the git credentials, your actions/checkout may want to avoid persisting those during the job run (so they're removed after the checkout steps), just add persist-credentials: false in with options.

---

I don't know if this applies to composer or other package managers, and when it just comes to diffing it probably doesn't matter at all, but I'd just generally be more concerned about copy/paste users that take your example and modify it with their own additions unaware of the security risks they add to compromising their repo secrets:

Essentially, if you check out the forked repo’s PR, the forked repo PR could’ve added a pre/post hook script and that could easily be executed if the workflow has a step that executes ‘npm run some-script’.

You should be able to checkout the PR and generate a diff with the pull_request event, then just volley the data over to a workflow_run event if you're concerned about doing things more securely? Might be worth adding as an alternative example, I refer to it as multi-staged workflow for lack of a better name.

More verbose/hassle but at least there is less risk since when a user needs access to secrets it has to be done in workflow_run and it's made clear not to run any untrusted third-party code on that workflow.

[marocchino]

Close because there is no activity.
I recently updated the README and added an explanation of write permissions, which you may want to read.