Access to secrets in PR from fork using environments and pull_request_target

Hello,

I’m trying to allow access to a secret variable in one job of a workflow, but I would like to be available also for PR from forks. For this reason I’m considering to use the pull_request_target with an explicit checkout. I understand that this is unsafe, and therefore I’m trying to use a Protected Environment that maintainers will have to deploy manually before the job is executed.
At the same time the workflow contains some jobs that do not require access to this variable.

Assuming that the maintainer will check the code before deploying it and stop any malicious attempt, is something like this safe? Is there any way for a malicious PR to access to any secret (i.e repository or organization secrets) before being deployed?

name: The Workflow

on:
  push:
  pull_request:
    types: [opened, synchronize, reopened]
  pull_request_target:
    types: [opened, synchronize, reopened]
  release:
    types: [published]

jobs:
  without_secrets:
    name: 'Job Without Secrets Access'
    if: github.event_name != 'pull_request_target'
    runs-on: ubuntu-20.04
    steps:

    - name: Clone repository
      uses: actions/checkout@v2

    - name: Do something
      run: |
        echo "Do something without secrets access"

  with_secrets:
    name: 'Job With Secrets Access'
    if: github.event_name == 'pull_request_target'
    environment: foo
    runs-on: ubuntu-20.04
    steps:

    - name: Clone repository
      uses: actions/checkout@v2
      with:
        repository: ${{ github.event.pull_request.head.repo.full_name }}
        ref: ${{ github.event.pull_request.head.ref }}

    - name: Do something
      env:
        FOO_TOKEN: ${{ secrets.FOO_TOKEN }}
      run: |
        echo "FOO_TOKEN = ${FOO_TOKEN}"

Now, assuming this is safe (which I’m not sure), I have a second problem…

The with_secrets job is supposed to run for all events, not just for PR, but I don’t want it to stop for an explicit deployment for push and release events. Moreover for these events it is supposed use the checkout action without parameters. Is there a way to do this without duplicating all the code for the job? I’m thinking that perhaps I can have two different environments (foo and foo_unsafe) that contain the same secrets, and require the maintainer intervention only for the unsafe version.
This means that the job would become something like this:

  select_environment:
    runs-on: ubuntu-20.04
    outputs:
      environment: ${{ steps.select.outputs.environment }}
    steps:
    - id: select
      run: |
        if [[ "$GITHUB_EVENT_NAME" = "pull_request_target" ]]; then
          echo "::set-output name=environment::foo_unsafe"
        else
          echo "::set-output name=environment::foo"
        fi

  with_secrets:
    name: 'Job With Secrets Access'
    if: github.event_name != 'pull_request'
    needs: select_environment
    environment: ${{needs.select_environment.outputs.environment}}
    runs-on: ubuntu-20.04
    steps:
      ### Perhaps these 2 "Clone repository" steps can be merged somehow?
    - name: Clone repository [safe]
      if: github.event_name != 'pull_request_target'
      uses: actions/checkout@v2
    - name: Clone repository [unsafe]
      if: github.event_name == 'pull_request_target'
      uses: actions/checkout@v2
      with:
        repository: ${{ github.event.pull_request.head.repo.full_name }}
        ref: ${{ github.event.pull_request.head.ref }}

    - name: Do something
      env:
        FOO_TOKEN: ${{ secrets.FOO_TOKEN }}
      run: |
        echo "FOO_TOKEN = ${FOO_TOKEN}"

but according to the workflow-syntax it is not possible to use an expression to select the environment. Any suggestions on how to do this? I could make it 2 different jobs, but then I would have to duplicate all the code of the steps, remember to modify it twice whenever I need to do some changes, etc.

Thanks in advance for any suggestion.

P.S. This scenario seems pretty common (and safe) to me, I just want to run the workflows on push, pull_request and release events, but I want to stop on PR until the maintainer explicitly consents the access to the secrets… Why is it so hard to implement?

1 Like

I would not consider it to be safe. Here are my thoughts:

With pull_request_target, forks gain access to your secrets, and with an explicit checkout of the fork’s code, it becomes possible for everyone to modify the workflow code so that it leaks the secret. The modified workflow needs to be run for that, however. If a workflow is only run after manual approval by using a protected environment, then it won’t be leaked unless the reviewer overlooks something malicious and approves the run anyway.

Here’s the thing: It doesn’t have to be shell code in a run step. It could also be an Action that steals your secret. It might not be obvious that someone wants to sneak in malicious code. The reviewer will have to thoroughly check each external workflow run request, or you will be at risk.

This is true, but I, as a maintainer, would be very suspect if anyone that I don’t know, and have never contributed to the project, opens a pull request that changes the workflows, and I would check this pull request very thoroughly.

Another thing that could happen is that, at some point, one of the actions that are already in the workflow, are somehow replaced with “malicious” one, force-pushing the tag, or similar. I don’t think there anything we can do in this case…

The PR might not need to change the workflow. In most cases at least the GITHUB_TOKEN will be available in the file system. Other risks will depend on the exact workflow, but if you need to use secrets I assume there are some. Changing code that runs wherever those secrets are used (including changing config files, hooks, etc. for benign tools!) might be enough to exfiltrate or use them. Of course it all depends on the details, I’m just saying there might be non-obvious ways.

1 Like

That raises the question of whether to allow external contributions at all. You could keep it to contributors only. If you trust someone, then you can grant push access and those people will be able to create branches in the repository itself, removing the need to hand out secrets to forks.

Actually, you can protect yourself from that. Inspect the source code of the action, then pin it to the exact commit hash in your workflow. That way, no one can switch out the code, since any modification results in a different hash. Also see Using SHAs. The downside is that you will be stuck with that version of the action, i.e. no automatic updates.

1 Like