silverstripe / module-standardiser

Tool that standardises some files in Silverstripe modules
BSD 3-Clause "New" or "Revised" License
1 stars 1 forks source link

FIX Use pull_request_target so we have access to secrets #65

Closed GuySartorelli closed 2 months ago

GuySartorelli commented 2 months ago

As per https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflows-in-forked-repositories

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

This explains why I saw it working in my own account (PR from my account to my account) but it failed in the org (where the PR came from a fork).

Swapping to pull_request_target does elevate the default permissions for the built-in token as per https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target

For workflows that are triggered by the pull_request_target event, the GITHUB_TOKEN is granted read/write repository permission unless the permissions key is specified and the workflow can access secrets, even when it is triggered from a fork.

We do explicitly specify the permissions key and intentionally keep it empty, so we should be able to access secrets while having a read-only github token.

Other things to note:

you should make sure that you do not check out, build, or run untrusted code from the pull request with this event

We're not doing any of those

This event runs in the context of the base of the pull request, rather than in the context of the merge commit, as the pull_request event does. This prevents execution of unsafe code from the head of the pull request that could alter your repository or steal any secrets you use in your workflow.

So this should be robust against changes to the action within the PR that triggers it, unlike when using the pull_request event. It's actually keeping our secret safer than if the pull_request event allowed access to secrets.

Note

Two commits here - one to do what I mentioned above and another to update the name of the action and job per comments on the issue.

Issue