How to safely run on pull_request_target?

[solvaholic]

When contributors will use forks to propose DNS configuration changes, a GitHub Actions workflow triggered on the pull_request event cannot run octodns-sync because the workflow does not have access to secrets.

Triggering the workflow on pull_request_target makes secrets available - and makes it possible to run untested code in your repository. This has come up, related to this Action, in #41, #67.

How to safely run this Action on the pull_request_target event?

[solvaholic]

Here are links to a couple mentions of ways to more safely run this Action on the pull_request_target event:

• Improve comment formatting #67 (comment)
• add_pr_comment adds new comment for each run #41 (comment)

A workflow run on pull_request_target runs in the context of the pull request base ref, for example main. Any files the workflow needs from the pull request head, then, must be checked out in the workflow. Ensuring the workflow checks out only octodns configuration files should limit risk considerably.

To further reduce the risk of exposing DNS provider privileged API keys, configure separate environments for validating and deploying changes. For example configure one environment with read-only DNS API keys, and a second with read-write DNS API keys. Then, for each workflow job, specify which environment it has access to.

To further reduce the risk of unauthorized users pushing changes to the repository, specify which permissions GitHub.token gets for each job.

What feedback do y'all have about ☝️ these approaches? What additional suggestions would you make?

[solvaholic]

Gah! There's a blog post about this:

https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

[solvaholic]

Ok. I finally feel like I understand why the example configuration @xt0rted provided works. I think:

• The test/validate and deploy workflows are triggered on separate events, so they're separate
• Grouping tasks in jobs and workflows this way enables finer-grained permission control
• Triggering deploy on /deploy comment in an issue prevents some risks and sidesteps some obstacles
• I had no idea, before trying all this out, what a deployment is, or does
• Creating a deployment says, like, Deploy <commit> to <environment>; Another workflow can respond to that by deploying <commit> to <environment>

To minimize the risk of running a workflow on the pull_request_target event, I think:

• Scope permissions of the containing workflow or job to issues: write
• Use environments to limit jobs' and steps' access to secrets
• Be mindful what you expose through env: variables
• Explicitly checkout contents from refs/pull/NNN/merge or head

So, for example, when DNS changes will come through pull requests from forks, one could use these two workflows:

• Validate: Runs on workflow_dispatch (manually) and on pull_request_target
  • permissions grants issues: write and contents: read
  • Run octodns-sync to generate the plan (requires access to API token secrets)
  • If triggered from pull request, add plan to comment
• Deploy: Runs on workflow_dispatch (manually) and on issue_comment
  • If issue comment matches deploy command, run; Else skip
  • Run octodns-sync to generate the plan (requires access to API token secrets)

Of course, there are many other ways to go about it. My suggestion here ☝️ prioritizes a) not requiring a personal access token and b) grouping workflow jobs by trigger.

[solvaholic]

The documentation added for #65 alludes to the problem running on pull_request_target can help solve:

Note: If users will propose DNS changes in pull requests from forks then it may be necessary to work around some GitHub token permission constraints.

So I think, to solve this issue:

• Create docs/pull_request_target.md: Describe the token complications; Link to references and example workflows
• Describe, in ☝️ that doc, an example solution
• Link ☝️ that doc from docs/add_pr_comment.md
• Link in README.md also?