Github checkout action doesn't see PR modifications

Hello everyone

I have an issue I’ve been tracking for a long time and I was never able to quite pinpoint it, until today.
It appears that for pull requests, modifications are not always grabbed by the checkout action.

I’ll use the example I stumbled upon today to clarify myself.

If you take a look at the following PR, created by dependabot:
build(deps): bump aiohttp from 3.7.4.post0 to 3.8.1 by dependabot[bot] · Pull Request #407 · TomerFi/switcher_webapi · GitHub.

You will note in the files tab that the only modification introduced by this PR is modifying the requirements.txt, bumping the aiohttp requirement from 3.7.4.post0 to 3.8.1.

Now if you take a look at the following workflow run, triggered by the above PR:
build(deps): bump aiohttp from 3.7.4.post0 to 3.8.1 · TomerFi/switcher_webapi@858d303 · GitHub.

You’ll see I got 4 jobs in this workflow, it doesn’t matter what they do for this particular issue of mine,
just note the first two steps of each, they were put there to showcase my point.

The Lint documentation files job

The first two steps are:

    steps:
      - name: Checkout sources
        uses: actions/checkout@v2.4.0

      - name: temp pring requirements.txt
        run: cat requirements.txt

The contents of the requirements.txt:

aioswitcher==2.0.7
aiohttp==3.7.4.post0

The Build project job

The first two steps are:

    steps:
      - name: Checkout sources
        uses: actions/checkout@v2.4.0
        with:
          ref: ${{ github.head_ref }}

      - name: temp pring requirements.txt
        run: cat requirements.txt

The contents of the requirements.txt:

aioswitcher==2.0.7
aiohttp==3.8.1

The Build docker job

The first two steps are:

    steps:
      - name: Checkout sources
        uses: actions/checkout@v2.4.0
        with:
          ref: ${{ github.sha }}

      - name: temp pring requirements.txt
        run: cat requirements.txt

The contents of the requirements.txt:

aioswitcher==2.0.7
aiohttp==3.7.4.post0

The Push coverage report to CodeCov job

The first two steps are:

    steps:
      - name: Source checkout
        uses: actions/checkout@v2.4.0
        with:
          fetch_depth: 0

      - name: temp pring requirements.txt
        run: cat requirements.txt

The contents of the requirements.txt:

aioswitcher==2.0.7
aiohttp==3.7.4.post0

It would appear that if I need the PR to see the correct modifications,
I need to manually configure the checkout action to checkout the branch from which the PR was created.
All other checkouts, actually checkout the merge commit id, and for some weird reason, don’t see the modifications from the PR.

This caused me, on a couple of occasions, to merge a PR just to eventually see it fail other workflows triggered by the merge, and eventually, I had to manually do a git restore and force push it for each occurrence.

Am I missing something here?

Figured it out!

The checkout action uses the sha value for the checkout process by default.
And I was using pull_request_target instead of pull_request as the trigger for the workflow.

Based on the documentation:
When using pull_request as the trigger:

  • GITHUB_SHA = Last merge commit on the GITHUB_REF branch
  • GITHUB_REF = PR merge branch refs/pull/:prNumber/merge

When using pull_request_target as the trigger:

  • GITHUB_SHA = Last commit on the PR base branch
  • GITHUB_REF = PR base branch

So I was actually checking out the base branch because I was using pull_request_target.

As a workaround, if you want to use pull_request_target as the trigger, you can configure the checkout action to use the head_ref in order to grab the PR base branch:

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

Here there be :dragon:s

Be very careful about using the head ref from a pull_request_target.

Races :racehorse: and Conflicts :crossed_swords:

Fwiw, I’ve just finished debugging the fact that refs/pull/:prNumber/merge is unreliable – it’s apparently generated by an asynchronous task that afaict doesn’t properly handle merge conflicts after an update. In my experience that meant that if the latest pushed head conflicted with the base branch, the refs/pull/:prNumber/merge commit would still exist, but just point to the previous merge result even though the PR itself would correctly say that the PR couldn’t be merged because there’s a conflict.

1 Like

This is an excellent post, thank you very much for sharing,
I’m definitely considering modifications to my workflows after reading it.

I should point out that I do use a couple of other safety precautions:

  • I use permissions for the GITHUB_TOKEN to limit the access for workflows triggered by pull_request_target.

  • I configure my repositories to always require my approval for all outside collaborators and not just first-time ones:
    image

This is interesting, I’d love to hear more about this.

I might write something up more formally (possibly on dev.to?), but for the time being, here’s what I have:

I’ve basically replaced all instances where I ask for the magic commit with this action that generates it on the fly. In terms of cost, it’s a bit of extra cpu time (and unfortunately a lot of git history fetching since I don’t know offhand of an easy way to find the greatest common ancestor using GitHub APIs and didn’t want to spend the brain power & cpu power to find it for something that isn’t particularly important on average).

I’m not done deploying this change internally but I expect to have it fully deployed internally by the end of the month and then as I do a refresh of my check-spelling workflow, I’ll include this as well.