Support for pull_request_target

[PatrickHeneise]

This action fails with

Error: Unsupported event pull_request_target (currently supported events include push, pull_request)

name: Process Images

on:
  workflow_dispatch:
  pull_request_target:
    types:
      - opened
      - reopened
      - synchronize
      - ready_for_review
      - unlocked
    branches:
      - main

jobs:
  compress:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
        with:
          ref: ${{ github.event.pull_request.head.sha }}
      - name: Tinify Image Action
        uses: namoscato/[email protected]
        with:
          api_key: ${{ secrets.TINIFY_API_KEY }}
          github_token: ${{ secrets.GITHUB_TOKEN }}

pull_request_target is a new event type that allows access to the repository secrets (TINIFY_API_KEY) when the pull request is coming from a fork.

Maybe remove the event check all together and allow events like workflow_dispatch as well? GitHub Actions changes frequently.

For more information, see https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/#improvements-for-public-repository-forks

[namoscato]

@PatrickHeneise, good catch – thanks! I'll take care of this over the weekend.

[namoscato]

@PatrickHeneise, #201 adds support for pull_request_target, but I'm running into some issues with actions/checkout and git context when actually pushing up the commit.

The existing push / pull_request events relied on checkout via the source branch:

- uses: actions/checkout@v2
  with:
    ref: ${{ github.head_ref }}

and pushed via:

git push origin

which won't work for forked repositories.

I'm still determining the best way to support all contexts; let me know if you have any thoughts in the meantime.

[namoscato]

Well, I spent some more time with this today with no luck. See actions/checkout#455 (comment) for some context, but this might take some time to sort out – or it might not be possible to support.

[PatrickHeneise]

Thanks a lot for looking into this so quickly!

Have you tried ${{ github.event.pull_request.head.sha }}? This seems to be common practice when using pull_request_target.

[namoscato]

Have you tried ${{ github.event.pull_request.head.sha }}?

Yeah, I did add support for this. It checkouts the pull request HEAD commit (instead of the default merge commit), which is necessary for pull_request_target contexts in which you want to interact with the git history (i.e. push a commit). But the act of pushing fails, which seems to be some sort of permissions issue.

[PatrickHeneise]

Ah, ok. Yeah, the permission stuff is definitely confusing with Actions :( I'll try a few things too, see if we can solve this.

[namoscato]

@PatrickHeneise, I've since stumbled across this blog post which gives a pretty good overview of why some of these constraints are in place to mitigate security issues.

I've tried a few more things, most recently triggering the pull_request_target workflow by labeling the fork PR as a user with write privileges to the target repository, without success. I think even if we could get this approach to work, there would be a minor security vulnerability: Bad actors could theoretically gain access to secrets (i.e. TINIFY_API_KEY), but hopefully this would be caught by manual reviews of the PR before assigning a label. namoscato/action-tinify@pull_request_target re-enables the pull_request_target event if you want to play around with this (presumably broken) state.

The only other idea I have at the moment involves leveraging the workflow_run event as described in that blog post. This seems a little involved, but I think the idea would be:

1. run some pull_request workflow that uploads new images as artifacts
2. subsequently download and compress the images in a separate workflow_run workflow

I would think (1) could happen with core actions, but I could possibly add a mode to namoscato/action-tinify that just outputs uncompressed image paths?

Let me know if you think that latter idea would even be worth pursuing...

[PatrickHeneise]

I've worked with workflow_run a little bit and didn't have a great experience so far. Maybe that'll get better in the future. I see a few potential issues here, if main is protected, it'll be hard to get a commit through. And opening another PR would result in a loop of actions.

Definitely trickier than anticipated, and I feel this is an issue GitHub should address, as this is common practice to open a PR from a fork.

[PatrickHeneise]

Closing this for now. There seems to be some development at GitHub to solve the permission issue.