envoyproxy / envoyproxy/envoy

Allow use of `pull_request_target` to run `pull_request` code

Open
#26,170 19 comments 0 reactions 0 assignees View on GitHub
area/ci area/security no stalebot
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 20h
Merged PRs (30d)
437

Description

There is currently a PR that uses `pull_request_target` and would allow PRs to run with repo secrets/permissions

This pattern has been discussed quite a bit previously in the context of shifting off of AZP for CI

Essentially `pull_request_target` runs whatever code in the "context" of the target branch - ie `main` and has access to repo secrets and permissions.

**Github absolutely advises against this**, but you can kinda hack it, so that it runs PRs (ie untrusted code) with that context and permissions/secrets

The problem is that there really is no other way to run PRs with secrets on github (its mostly what kept us on AZP until now)

You can do various things to mitigate the risk, but it opens up attack vectors that are incredibly hard to prevent either programatically or by visual inspection so the advice is just dont do it. Security tools that look for vulnerable repos will flag the repo just for using the pattern of pull_request_target + PR git checkout - irrespective of what happens after

There are several possible vectors, including cache poisoning - the defaults are incredibly insecure and the github token is available on disk (by default) and in memory regardless of what you do in a job

The biggest risk is probably to secrets/permissions

### Example attack vector

This is the example given by Github

In the following code, altho the `npm` job (ie untrusted code) is not directly given access to secrets, it can inject code to change how any following jobs work, and thereby snaffle secrets from other steps, or change how they run using the repo authority

```yaml
# INSECURE. Provided as an example only.
on:
pull_request_target

jobs:
build:
name: Build and test
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
with:
ref: ${{ github.event.pull_request.head.sha }}

- uses: actions/setup-node@v1
- run: |
npm install
npm build

- uses: completely/fakeaction@v2
with:
arg1: ${{ secrets.supersecret }}

- uses: fakerepo/comment-on-pr@v1
with:
message: |
Thank you!
```

### Mitigations

Addressing the insecure defaults and preventing the checkout action writing the token to disk can help to some extent, but ultimately are a maginot line if you can still steal the creds from mem, or inject unwanted code.

One possible mitigation is only running untrusted code in containers - and doing so in such a way that the code can neither access secrets (unless explicitly allowed) nor influence future action. Unfortunately this wont help with the mac (mobile) jobs

The key point about migitation i think is that while you _can_ setup such a system securely, keeping it so, is an art in itself

### Refs

- #25770
- #26109
- https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target
- https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
- https://nathandavison.com/blog/github-actions-and-the-threat-of-malicious-pull-requests
- https://gist.github.com/ndavison/d14dbbd9d015eeeef19b923ab80b1f1a
- #26147

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.