Skip to content

Commit 4d7add9

Browse files
committed
including section on interpolation and pull_request_target
1 parent 0916f36 commit 4d7add9

1 file changed

Lines changed: 47 additions & 3 deletions

File tree

practices/actions-best-practices.md

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,12 +181,12 @@ jobs:
181181
### Secure Pull Request Workflows
182182

183183
- Don't expose secrets to pull request workflows from forks
184-
- Use `pull_request_target` carefully with read-only permissions
184+
- Do not use `pull_request_target` in workflows. It runs in the context of the base repository with access to secrets and write permissions, even for PRs from untrusted forks. This makes it a common vector for supply chain attacks ("pwn requests"). Use `pull_request` instead
185185

186186
```yaml
187-
# Safer approach for PR workflows
187+
# Required approach for PR workflows
188188
on:
189-
pull_request:
189+
pull_request:
190190
jobs:
191191
test:
192192
runs-on: ubuntu-latest
@@ -198,6 +198,50 @@ jobs:
198198
run: npm test
199199
```
200200

201+
The one widely-used exception is Dependabot auto-merge workflows. GitHub's own [auto-merge guidance](https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions) relies on `pull_request_target` because Dependabot PRs need write access to trigger merges. If you use this pattern, restrict permissions to the minimum required (e.g. `pull-requests: write`, `contents: write`), and ensure the workflow never checks out or executes code from the PR head. Any such workflow must be reviewed and approved before merging.
202+
203+
To ensure the workflow never checks out or executes PR head code:
204+
205+
- **Do not include an `actions/checkout` step.** Auto-merge workflows only need to call the GitHub API (e.g. to approve or merge); they do not need the repository contents.
206+
- If a checkout is unavoidable, **never use the PR head ref or SHA** — do not pass `ref: ${{ github.event.pull_request.head.sha }}` or `ref: ${{ github.head_ref }}` to `actions/checkout`. Omitting `ref` checks out the base branch.
207+
- **Do not interpolate any PR-supplied values directly into `run:` steps.** This applies to all workflows, not just `pull_request_target` — see [Avoid Script Injection](#avoid-script-injection) below.
208+
- **Limit the workflow trigger.** Scope `pull_request_target` to specific activity types (e.g. `opened`, `synchronize`) and, where possible, add a condition to restrict execution to Dependabot only:
209+
210+
```yaml
211+
on:
212+
pull_request_target:
213+
types: [opened, synchronize]
214+
215+
jobs:
216+
auto-merge:
217+
if: github.actor == 'dependabot[bot]'
218+
```
219+
220+
### Avoid Script Injection
221+
222+
Script injection can occur in any workflow — not just `pull_request_target` — whenever untrusted values are interpolated directly into a `run:` step. GitHub Actions expressions are evaluated before the shell executes the command, so an attacker who controls an input value (e.g. a PR title, branch name, or issue body) can inject arbitrary shell commands.
223+
224+
Do not use `${{ }}` expressions directly inside `run:` blocks. Instead, assign them to environment variables and reference those variables in the shell:
225+
226+
```yaml
227+
# Unsafe - expression evaluated before shell runs; attacker controls PR title
228+
- run: echo "${{ github.event.pull_request.title }}"
229+
230+
# Safe - value passed via environment variable; shell treats it as data, not code
231+
- run: echo "$PR_TITLE"
232+
env:
233+
PR_TITLE: ${{ github.event.pull_request.title }}
234+
```
235+
236+
This applies to all context values that may contain user-controlled input, including but not limited to:
237+
238+
- `github.event.pull_request.title` / `.body` / `.head.ref`
239+
- `github.event.issue.title` / `.body`
240+
- `github.event.comment.body`
241+
- `github.head_ref`
242+
243+
Values sourced entirely from within your own organisation (e.g. `github.repository`, `github.sha`) carry lower risk, but the environment variable pattern is still the preferred style for consistency and reviewability.
244+
201245
### Implement Required Reviews
202246

203247
- Enforce branch protection rules

0 commit comments

Comments
 (0)