dependabot/dependabot-core

Dependabot triggered Actions cant access secrets or use a writable token

WtfJoke opened this issue · 167 comments

WtfJoke commented

Moderator note: If you're here because your Dependabot triggered actions are broken, read our updated docs or jump to #3253 (comment) for a FAQ

Package manager/ecosystem
npm

Manifest contents prior to update

Updated dependency

What you expected to see, versus what you actually saw
Since ~08.03.21 our dependabot pull requests fail, because they cant access the npm private registry anymore.
We figured out, the reason is because dependabot cant read secrets anymore (see https://github.com/github/docs/pull/4397/files).

When we rerun the pull requests they succeed as the used GITHUB_TOKEN has permission to read the secret.

Is there a solution or workaround in place?

Images of the diff or a link to the PR, issue or logs

https://github.com/github/docs/pull/4397/files

mdreizin commented

I can confirm that. Now Dependabot can't read secrets anymore at all.

mdreizin commented

Any secrets are not available for Dependabot.

mdreizin commented

Can't share links to private repos, but here is a couple of public ones:

mdreizin/chrome-bookmarks-alfred-workflow#96
mdreizin/gatsby-plugin-robots-txt#418

mdreizin commented

I contacted yesterday support team and they denied that issue and refused to fix.

Could you please fix it as soon as possible, because many users are affected including paid ones?

  • 👍1
  • 😄1
mercuriete commented

the team support answer me with this:

We recently made changes to dependabot which means they will receive a read-only GITHUB_TOKEN and will not have access to any secrets available in the repository.

https://github.blog/changelog/2021-02-19-github-actions-workflows-triggered-by-dependabot-prs-will-run-with-read-only-permissions/

So it is not a bug its a feature.

My use case is I am auto merging branches after a CI passes using this action:
https://github.com/ahmadnassri/action-dependabot-auto-merge

and now my workflow is totally broken.

Please add automerge to dependabot.

Thanks.

  • 👍7
  • 😕4
mercuriete commented

I am trying to fix the problem using a workaround:
https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull_request_target

it is mention in that post but I think with pull_request_target the tokens are insecure to forks. So I am only using that workaround to private repositories.

mercuriete commented

yeah, I can confirm that moving from

on: pull_request
to
on: pull_request_target

fix the problem

but you need to read the docs first -> https://securitylab.github.com/research/github-actions-preventing-pwn-requests

the main problem as a CI developer is that if you make a change on the pipeline you can't see the change until is in the base branch which is difficult for CI developing.

I hope it helps someone.

EDIT: It only work with GITHUB_TOKEN if you need personal access token it won't work :(

  • 👍2
jasperroel commented

I'm running into the same issue. Steps taken so far (since https://github.blog/changelog/2021-02-19-github-actions-workflows-triggered-by-dependabot-prs-will-run-with-read-only-permissions/ mentions that switching to pull_request_target should grant access to repository secrets again).

Unfortunately, even these steps seem to only switch the GITHUB_TOKEN to a write, but I still do not see any of the other expected secrets.

1 Update pull_request to pull_request_target

Change

on:
  pull_request:
    branches:
      - develop

to

on:
  pull_request_target:
    branches:
      - develop

2 Limit to dependabot

Added if: github.actor == 'dependabot[bot]' so that the jobs with secrets only run for Dependabot

3 Modify the checkout to still grab the head checkout

Change:

      - name: Checkout
        uses: actions/checkout@v2.3.4

to

      - name: Checkout
        uses: actions/checkout@v2.3.4
        with:
          ref: ${{ github.event.pull_request.head.sha }}

As other mentioned that seems to switch the ${{ secrets.GITHUB_TOKEN }} from a read-only to a write token, but other secrets remain unaccessible.

For example:

      - name: Automerge Dependabot
        uses: actions/github-script@v3
        with:
          github-token: ${{ secrets.PAT_PUSH_TOKEN }}
          script: |
            const output = `@dependabot squash and merge`;

            github.issues.createComment({
              issue_number: context.issue.number,
              owner: context.repo.owner,
              repo: context.repo.repo,
              body: output
            })

fails with a

Error: Input required and not supplied: github-token
    at getInput (/home/runner/work/_actions/actions/github-script/v3/dist/index.js:1452:15)
Error: Unhandled error: Error: Input required and not supplied: github-token
mercuriete commented

people from this issue might want to subscribe to this issue:
#2268

we all are facing the same issue.

BTW, I tested the alternative
on: workflow_run:
suggested here: https://securitylab.github.com/research/github-actions-preventing-pwn-requests

and again it work for github_token but not for secrets (at least for me)
with that alternative the secrets are all empty.

mercuriete commented

I will try to move from dependabot to renovate to asset the feasibility of the migration.
right now dependabot is broken for enterprise users that make use of GitHub packages to upload/download to/from npm private registries and a lot of enterprise use cases.
(AWS deployments are broken as well when the job is triggered by dependabot)

renovate: https://www.whitesourcesoftware.com/free-developer-tools/renovate

mdreizin commented

@mercuriete The biggest issue with on: workflow_run is missing ability to fail the whole pipeline, because they run in different context.

For instance, you have ci workflow and cd one which depends on ci via on: workflow_run (which needs access to your secrets) you won't see if cd fails or not without visiting ie https://github.com/dependabot/dependabot-core/actions to see actual status.

mdreizin commented

@mercuriete I am also considering migrating to renovate bot.

The funny thing: all PRs created by dependabot on that repo also fail https://github.com/dependabot/dependabot-core/pulls?q=is%3Aopen+is%3Apr+label%3Adependencies :)

The support team still has not provided any ways how to get rid of that issue.

mdreizin commented

Maybe anybody from Dependabot team could provide a solution to fix missing secrets on PR created by dependabot?

  • 👍1
mercuriete commented

hahaha
it seems dependabot is eating its own dog food https://es.wikipedia.org/wiki/Dogfooding

https://github.com/dependabot/dependabot-core/pull/3258/checks?check_run_id=2083846045

I'm glad they are facing the same issue because they have to fix it by themselves.

  • 👀3
asciimike commented

Hey folks, Dependabot PM here.

First off, apologies for the quick change and continued brokenness 😞 .

What changed?

The specific change is twofold, in that during pull_request triggered workflows:

  • your GITHUB_TOKEN is read-only
  • secrets can't be accessed

Additionally, a bug was introduced where pull_request_target also had these properties, which made the migration process worse. This was fixed at ~2300 UTC on March 11th, so pull_request_target should be working for those of you trying to use it.

Why was this change made?

This change was made in response to reported vulnerabilities in these types of workflows, and we felt that it critical to ensure our developers were protected before they were publicly disclosed.

Specifically, a malicious dependency could execute code to exfiltrate secrets or perform something other malicious action in the repo (deleting files, etc.).

By making secrets unavailable and tokens read only, this prevents a compromised dependency from being able to exfiltrate those secrets or perform any malicious write actions.

How do I fix a broken workflow?

As per the advice in https://securitylab.github.com/research/github-actions-preventing-pwn-requests, you can either use pull_request_target or create two workflows, one untrusted that generates whatever artifacts, and one trusted that does the push of the changes.

Taking a simple example:

### .github/workflows/dependabot_automerge.yml
### This workflow now has no secrets and a read-only token
name: Dependabot Workflow
on:
  pull_request

jobs:
  do-stuff:
    runs-on: ubuntu-latest
    if: ${{ github.actor == 'dependabot[bot]' }}
    steps:
      - uses: actions/checkout

Either use pull_request_target:

### .github/workflows/dependabot_automerge.yml
### As of 2300 UTC on 11 March, this workflow has secrets and a read-write token
name: Dependabot Workflow
on:
  pull_request_target

jobs:
  do-stuff:
    runs-on: ubuntu-latest
    if: ${{ github.actor == 'dependabot[bot]' }}
    steps:
      - uses: actions/checkout
        with:
          ref: ${{ github.event.pull_request.head.sha }}
          github-token: ${{ secrets.GITHUB_TOKEN }}

Or make it two workflows:

### .github/workflows/dependabot_pr.yml
### This workflow doesn't have access to secrets and has a read-only token
name: Dependabot PR Check
on:
  pull_request

jobs:
  check-dependabot:
    runs-on: ubuntu-latest
    if: ${{ github.actor == 'dependabot[bot]' }}
    steps:
      - run: echo "PR created by Dependabot"

Which triggers:

### .github/workflows/dependabot_automerge.yml
### This workflow has access to secrets and a read-write token
name: Dependabot Automerge
on:
  workflow_run:
    workflows: ["Dependabot PR Check"]
    types: 
      - completed

jobs:
  do-stuff:
    runs-on: ubuntu-latest
    if: ${{ github.event.workflow_run.conclusion == 'success' }}
    steps:
      - uses: actions/checkout
        with:
          ref: ${{ github.event.pull_request.head.sha }}
          github-token: ${{ secrets.GITHUB_TOKEN }}

Again, I'm really sorry for the churn and brokenness here, but hopefully this should get folks moving in the right direction 🙇🏻

  • 👍5
  • ❤️16
asciimike commented

Re: @mercuriete's

I will try to move from dependabot to renovate to asset the feasibility of the migration.
right now dependabot is broken for enterprise users that make use of GitHub packages to upload/download to/from npm private registries and a lot of enterprise use cases.
(AWS deployments are broken as well when the job is triggered by dependabot)

renovate: https://www.whitesourcesoftware.com/free-developer-tools/renovate

Private registry support is coming out shortly, so stay tuned and follow this issue and the changelog.

  • 👍1
derTobsch commented

Hey folks, Dependabot PM here.

First off, apologies for the quick change and continued brokenness disappointed .

What changed?

The specific change is twofold, in that during pull_request triggered workflows:

* your `GITHUB_TOKEN` is read-only

* `secrets` can't be accessed

Additionally, a bug was introduced where pull_request_target also had these properties, which made the migration process worse. This was fixed at ~2300 UTC on March 11th, so pull_request_target should be working for those of you trying to use it.

Why was this change made?

This change was made in response to reported vulnerabilities in these types of workflows, and we felt that it critical to ensure our developers were protected before they were publicly disclosed.

Specifically, a malicious dependency could execute code to exfiltrate secrets or perform something other malicious action in the repo (deleting files, etc.).

By making secrets unavailable and tokens read only, this prevents a compromised dependency from being able to exfiltrate those secrets or perform any malicious write actions.

How do I fix a broken workflow?

As per the advice in https://securitylab.github.com/research/github-actions-preventing-pwn-requests, you can either use pull_request_target or create two workflows, one untrusted that generates whatever artifacts, and one trusted that does the push of the changes.

Taking a simple example:

### .github/workflows/dependabot_automerge.yml
### This workflow now has no secrets and a read-only token
name: Dependabot Automerge
on:
  pull_request

jobs:
  automerge:
    runs-on: ubuntu-latest
    if: ${{ github.actor == 'dependabot[bot]' }}
    steps:
      - uses: ...

Either use pull_request_target:

### .github/workflows/dependabot_automerge.yml
### As of 2300 UTC on 11 March, this workflow has secrets and a read-write token
name: Dependabot Automerge
on:
  pull_request_target

jobs:
  automerge:
    runs-on: ubuntu-latest
    if: ${{ github.actor == 'dependabot[bot]' }}
    steps:
      - uses: ...
        with:
          github-token: ${{ secrets.GITHUB_TOKEN }}

Or make it two workflows:

### .github/workflows/dependabot_pr.yml
### This workflow doesn't have access to secrets and has a read-only token
name: Dependabot PR Check
on:
  pull_request

jobs:
  check_dependabot:
    runs-on: ubuntu-latest
    if: ${{ github.actor == 'dependabot[bot]' }}
    steps:
      - run: echo "PR created by Dependabot"

Which triggers:

### .github/workflows/dependabot_automerge.yml
### This workflow has access to secrets and a read-write token
name: Dependabot Automerge
on:
  workflow_run:
    workflows: ["Dependabot PR Check"]
    types: 
      - completed

jobs:
  automerge:
    runs-on: ubuntu-latest
    if: ${{ github.event.workflow_run.conclusion == 'success' }}
    steps:
      - uses: ...

Again, I'm really sorry for the churn and brokenness here, but hopefully this should get folks moving in the right direction 🙇🏻

Hey @asciimike,

I already tried that but without success. The workflows that that get triggered via workflow_run has no access to the secrets see https://github.com/synyx/urlaubsverwaltung/runs/2093235834?check_suite_focus=true - are my workflows and trigger correct?

What I did with a little more details:

name: Urlaubsverwaltung CI

on:
  schedule:
    - cron: "2 4 * * *"
  push:
    branches:
      - master
      - v3.x
  pull_request:
  workflow_dispatch:

jobs:
  build:
    name: build and analyse
    runs-on: ubuntu-20.04
    steps:
      - name: Checkout repository
        uses: actions/checkout@v2
        ...
      - name: Build
        run: ./mvnw --batch-mode -Pcoverage clean verify
...

and this triggeres the workflow https://github.com/synyx/urlaubsverwaltung/blob/master/.github/workflows/update-assets-manifest.yml e.g. with

name: Update assets-manifest

on:
  workflow_run:
    workflows: [ "Urlaubsverwaltung CI" ]
    branches: [ "dependabot/npm_and_yarn/**" ]
    types:
      - completed
  workflow_dispatch:

jobs:
  build:
    runs-on: ubuntu-20.04
    if: ${{ github.event.workflow_run.conclusion == 'success' }}
    steps:
      - name: Checkout repository
        uses: actions/checkout@v2
        with:
          persist-credentials: false
     ...
      - name: Assets-Manifest push
        uses: ad-m/github-push-action@v0.6.0
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}
          branch: ${{ github.ref }}

but now the github token does not have the permission to push to this branch.

mercuriete commented

same here, I tried yesterday both cases and even though github_token was present, secrets are not available making all normal use cases unusable.

use cases broken:
1- yarn install where you are pulling packages from private registry (npm)
2- docker login/docker pull from images from private registry
3- send reports to sonarqube.io

everything that involves a personal access token.

Please give us a working example of private registries making use a PAT.

thanks

asciimike commented

@derTobsch: I just set up a workflow run triggered by a Dependabot PR and I'm seeing the same behavior 😞

It seems like the answer is "workflow_run inherits the secrets and token from the previous PR" (the docs seem to indicate this, though the actions team indicated that it should have the secrets and a write token). The answer is to use pull_request_target (which I have confirmed has a read-write token), though at that point, there's no reason to use the second workflow (and indeed, my testing shows that the workflow_run runs as read-only without secrets, so you can't use it!).

@mercuriete: 1 and 2 will be solved more elegantly via github/roadmap#67, which is imminent. 3 will have to use pull_request_target or wait for the change as above.

  • 👍2
asciimike commented

I have confirmed that workflow_run now works as expected, with the workflow now able to access secrets and a read-write GITHUB_TOKEN. I believe this is now back to a place where folks should be able to fix themselves in a fairly straightforward way.

  • 👍2
  • 🎉2
  • ❤️2
essenmitsosse commented

Secrets in Dependabot PRs are considered a security risk, since an updated dependency could introduce malicious code, that could steal secrets — as far as I understand the basis for this issue.

Switching to pull_request_target or workflow_run allows the PR to read those secrets, but doesn't this just open up the security issue again?

As far as I understand, all this would mean, that there is no secure way to use Dependabot on a repository that for example needs an NPM token to install a private dependency. But technical that would also mean that on such a repo, I would have to manually check every dependency update for malicious code — which seems completely unfeasable.

Am I missing something here or is this the situation we're in?

  • 👍6
  • 👀1
ruudk commented

For anyone else that is having issues with this mess: I forgot to add ref: ${{ github.event.pull_request.head.sha }} on actions/checkout when I replaced on: pull_request with on: pull_request_target. If you don't add this you are basically checking out the main branch on your PR builds 🤯

  • 👍5
  • ❤️2
  • 👀2
derTobsch commented

If I use workflow_run now for my sonarcloud or other workflows that are considered insecure now. How can I achieve that these workflows are able to add comments to the original PR? Or how can they be published in the 'All checks have passed' section in a PR?

sonarsource community issue: https://community.sonarsource.com/t/github-action-how-does-the-sonar-maven-plugin-know-on-which/40101

ejntaylor commented

@derTobsch Try: http://github.com/haya14busa/action-workflow_run-status

I was unable to get this to work with matrix but otherwise think it will help you.

derTobsch commented

@derTobsch Try: http://github.com/haya14busa/action-workflow_run-status

I was unable to get this to work with matrix but otherwise think it will help you.

k, this seems the way to go. I already added this, but hoped there is a default github action way to do this.

  • 👀1
boredland commented

This is a total nightmare. I hate it.

  • 👍16
derTobsch commented

Is there a way to prevent a step to run if it was triggered by a pull_request event? Then for now I would just deactivate the steps with secrets.

asciimike commented

@mercuriete (and anyone else using secrets to access registries) we just launched support for private registries in beta, so you should be unblocked for doing that in more well supported way.

@essenmitsosse re:

Secrets in Dependabot PRs are considered a security risk, since an updated dependency could introduce malicious code, that could steal secrets — as far as I understand the basis for this issue.

Switching to pull_request_target or workflow_run allows the PR to read those secrets, but doesn't this just open up the security issue again?

As far as I understand, all this would mean, that there is no secure way to use Dependabot on a repository that for example needs an NPM token to install a private dependency.

For this use case in particular, see the above: we just added support for private registries, so the majority of folks shouldn't need to do this in Actions anymore.

Short answer to other cases: because Dependabot effectively runs untrusted code during the update process, yes.

I think the first thing we can do to make this better is fetch secrets from the newly released Dependabot secret store (currently used for private registries, above), which would limit the blast radius to Dependabot's secrets (likely just the npm token, as opposed to your AWS credentials or whatever deployment secrets used).

There are probably some other things we could do to Actions (e.g. disallow external networking, or at least when the action runs with a new network connection, you'd have to whitelist it) that would prevent a certain class of exfiltration/code execution risks, but again, it's not guaranteed.

But technical that would also mean that on such a repo, I would have to manually check every dependency update for malicious code — which seems completely unfeasable.

Most larger/more security conscious organizations mirror packages from public registries into internal registries after they undergo a security review, if they're allowed at all, specifically to solve for this.

Dependabot can't guarantee that the new versions are free of bugs or malware. There are related tools (e.g. GitHub's code scanning, JFrog Xray, etc.) that provide insight into what's actually running, and over time I'm sure that we can get better about providing signals of correctness or trustworthyness, but we're not there yet :/

@derTobsch, re:

Is there a way to prevent a step to run if it was triggered by a pull_request event? Then for now I would just deactivate the steps with secrets.

EDIT: It's not documented but: if: ${{ github.event.workflow_run.event == 'pull_request' }} should work.

  • 👍3
  • ❤️2
WtfJoke commented

I think the first thing we can do to make this better is fetch secrets from the newly released Dependabot secret store (currently used for private registries, above), which would limit the blast radius to Dependabot's secrets (likely just the npm token, as opposed to your AWS credentials or whatever deployment secrets used).

That would be very helpful for us. 👍

We use it for read access to a private npm registry (artifactory).

  • 👍1
asciimike commented

@WtfJoke I'd strongly recommend using https://docs.github.com/en/github/administering-a-repository/configuration-options-for-dependency-updates#npm-registry in your flow, if possible. Should be a streamlined experience for npm from a private registry like artifactory.

  • 👍2
  • ❤️2
WtfJoke commented

@asciimike That looks promising. Thanks for the hint!

Just skimmed through, it looks like its possible to provide multiple urls, is that correct? EDIT: Looks like :)
Does it also support scopes, because we use multiple scoped registries

essenmitsosse commented

@asciimike Thanks for the reply and the clearification. I see that the whole security issue in general is a huge topic., that can't be easily fixed. Just adding the secret to the dependabot secrets doesn't result in the old workflow working again — so there seems to be more overhead to configure. So right now, I'm just switchting to pull_request_target to save myself the headache.

But this brings up the question of what making dependabot harder to use really achives here. Basically I have a readonly NPM Token stored in my secrets and all I want to be able to say is "That is save for dependabot to use and I'm aware of the implications".

Also in most cases, a dependabot PR will merged if it passes, thus ending up one way or the other in the build pipeline that has all the secrets. For some repos that build pipeline will kick in immediately after merging. Plus: you are already controlling which secrets to pass to which build step, so if your deploy credentials are passed to your install step, that's a configuration issue.

So this introduces a lot of complexity to a build pipeline, while not really adressing the underlying issue. It merely makes people aware of the situation, that they shouldn't leak their deploy credentials to an untrusted process.

So the question for me is: does this really solve any problem or does it just make it a bit harder to foot gun yourself — without any really protection — while complicating things for the majority of users?

Or is there something I'm missing here?

  • 👍12
mercuriete commented

IMHO the security issue should be addressed by the owner of the repository.
yarn install is insecure by design even on your local workstation.
to mitigate that, it is recommended to not to do post-install scripts like this:

      - name: Install dependencies skip post-install
        # Skip post-install scripts here, as a malicious
        # script could steal NODE_AUTH_TOKEN.
        run: yarn install --ignore-scripts --frozen-lockfile
        env:
          NODE_AUTH_TOKEN: ${{ secrets.GPR_READ_TOKEN }}
      - name: Install dependencies with post-install
        # `yarn install --force` will run all those post-install scripts for us.
        run: yarn install --force --frozen-lockfile
        env:
          NODE_AUTH_TOKEN: "FAKE_TOKEN"

That pipeline is broken right now because secrets are not accessible when dependabot triggers a pipeline.

Can somebody confirm that pulll_request_target fix the problem?
because I tried a few days ago and I wasn't working.

For me this change is disruptive and the security concern should be addressed by the security team inside our side.

Thanks for your support.

PS: I am talking on private repositories where nobody can make PR outside our control.
PS2: IMHO the previous behaviour should be restored on private repos with a configuration checkbox.
PS3: Funny comic about this issue: https://xkcd.com/1172/

  • 😄2
kevinlul commented

I'm still confused, are we meant to have main workflows trigger off of workflow_run instead? This week our team has had a lot of Dependabot stuff break. The first was the change from pull_request to pull_request_target for our own labelling workflow, since Dependabot PRs are treated as external. The second I'm trying to figure out what to do here, but the behaviour I'm observing is that workflows on the main branch triggered due to a commit via @dependabot squash and merge do not have access to secrets.

asciimike commented

@WtfJoke re:

Does it also support scopes, because we use multiple scoped registries

Yes, if you've got scopes set up on the registry, it should support scopes. Depending on how you've got registries set up you may have to have multiple updaters segmented using allow or ignore, but it should work.

@kevinlul re:

The second I'm trying to figure out what to do here, but the behaviour I'm observing is that workflows on the main branch triggered due to a commit via @dependabot squash and merge do not have access to secrets.

Is this a push event? If so, I believe that has the same restrictions as pull_request (and pull_request_review, and pull_request_review_comment). I think the answer is that you'll have to do the trampolining through workflow_run like I did above :/

@mercuriete re:

Can somebody confirm that pulll_request_target fix the problem? because I tried a few days ago and I wasn't working.

Yes. It was broken initially, but has since been fixed. I've confirmed that it gets secrets and a read-write token as the blog post specifies.

  • 👍1
  • 😕1
  • ❤️1
dentarg commented

So the question for me is: does this really solve any problem or does it just make it a bit harder to foot gun yourself — without any really protection — while complicating things for the majority of users?

@essenmitsosse I think it solves the problem that GitHub had: their Dependabot pull requests ran with access to your secrets. You enabled @dependabot and got owned by an unmerged PR, not a great experience for you and not a great story for GitHub.

If you merge a PR with a compromised dependancy, that's on you, not GitHub.

asciimike commented

@mercuriete re:

IMHO the security issue should be addressed by the owner of the repository. yarn install is insecure by design even on your local workstation. to mitigate that, it is recommended to not to do post-install scripts like this:

Dependabot runs with the equivalent of --ignore-scripts on by default (and indeed there's no way to turn it off to allow scripts to run).

Unfortunately this behavior is common for many package managers (bundler,pip, mix...). Most of the time it's because the package manager DSL is just another file in the programming language, so people can do weird and wonderful things (the XKCD applies equally to this). For these three languages, we provide an insecure-external-code-execution to allow folks to break out and do the potentially unsafe thing if required.

Similar to the --ignore-scripts flag, pip has provided a nice workaround for this in the form of the --only-binary :all: flag, which disallows source packages. We're obviously in favor of more package managers doing this.

@essenmitsosse re:

So this introduces a lot of complexity to a build pipeline, while not really adressing the underlying issue. It merely makes people aware of the situation, that they shouldn't leak their deploy credentials to an untrusted process.

So the question for me is: does this really solve any problem or does it just make it a bit harder to foot gun yourself — without any really protection — while complicating things for the majority of users?

In the views of the security researcher(s) who reported the vulnerability, the team at GitHub who reviewed the report and agreed with its assessment, and the Actions team who made the change, the belief is that this change makes developers as a whole safer.

When building developer tools I believe in two principles: 1) the defaults should be safe, 2) there should be escape hatches that let developers who assume the risks perform unsafe actions. In general, unsafe tools fail as developers get burnt by them and stop using them (or build safer default on top of them), while tools that are too safe never gain widespread adoption because they are too limited (see the referenced XKCD--developers like to do weird things).

Finding that balance is difficult and ever changing. This change lands on the side of "making the defaults safer" at the expense of "requiring more developers to consciously assume risk and use the provided escape hatch." Previously, everyone was unconsciously assuming that same risk!

We strive to update our products with safer defaults and without loss of functionality or decreased usability, even in the face of a changing vulnerability landscape. Switching where secrets are fetched from or allowing developers to scope their token permissions on a per-workflow basis will give developers more freedom to do things that were previously potentially unsafe, and make them safer, potentially eventually allowing us to relax restrictions.

I'm working on getting the docs (and that changelog post) updated to contain all of this information so we have a single source. In the future, we clearly have work to do to communicate these changes better, and I've already brought up better ways of notifying developers of impending breakage, especially on the short timelines often required by a disclosure schedule.

  • 👍1
  • 👎2
  • ❤️1
asciimike commented

@mercuriete re:

IMHO the previous behaviour should be restored on private repos with a configuration checkbox.

You specifically mean "if dependabot is pulling from a private repo/registry, we should allow pull_request etc. runs to get secrets and have a read-write token"? The thought here being that "if I own the registry, I trust that no malicious code has gotten in"?

mercuriete commented

@mercuriete re:

IMHO the previous behaviour should be restored on private repos with a configuration checkbox.

You specifically mean "if dependabot is pulling from a private repo/registry, we should allow pull_request etc. runs to get secrets and have a read-write token"? The thought here being that "if I own the registry, I trust that no malicious code has gotten in"?

Yes, I meant that.
Maybe I am missing something. What I meant was... I am the administrator of the organization so nobody can create a malicious PR. So for private repositories where all users are members and have user access control managed by the administrator I can control who is able to create a PR/fork.

Thanks for your clarifications!
I will try with pull_request_target today. thanks!

derTobsch commented

@asciimike maybe you can help me out. Are there different event_types or information in the github context for 'forked pull requests 'and 'normal pull requests'? I cannot find anything about that just the 'pull_request' event_name.

cahaseler commented

I'm still having trouble figuring out what the solution here is. Our use case is the following:

On Pull Request in private controlled repo that only trusted people can push to, trigger a GA workflow to run a series of cypress tests to confirm that the updated package doesn't break core functionality. This requires installing the app and running a node server using a bunch of secrets stored in Github Secrets. Without access to all those secrets, the app doesn't start.

Now that dependabot can't read those secrets, it's not clear what we're supposed to do. Using Pull_Request_Target doesn't help because that runs on the base commit that's already been tested, not the new code.

From what I'm reading here, you've essentially removed any useful functionality dependabot might have for my team. Is there another product that does something similar and still works? As a paying Github enterprise customer, is there a way to get back the feature I paid for and worked fine last week?

  • 👍2
covertbert commented

Just to clarify @asciimike, is the plan to also make the new "Dependabot secrets" available to Dependabot-created, pull_request-triggered workflow runs? Or is the long term solution to implement one of the workarounds mentioned above?

covertbert commented

@cahaseler I think you want to read this comment for the problem you're having.

  • 👍1
  • ❤️1
khitrenovich commented

@cahaseler I think you want to read this comment for the problem you're having.

I don't think the combination of pull_request_target and ref: ${{ github.event.pull_request.head.sha }} fully solves the issue - not for us, at least. We do have several checks, some are pretty complicated. I want those checks to run on all the PRs (whether they come from dependabot or not), some of them will run on master too and some may be triggered manually. Having to replicate those checks specifically for dependabot PRs does not seem manageable...

  • 👍3
feelepxyz commented

👋 We've set up a sample workflow that runs on workflow_run and has write access to dependabot branches here: https://github.com/dependabot/dependabot-actions-workflow/tree/main/.github/workflows

asciimike commented

@mercuriete and @cahaseler:

I am the administrator of the organization so nobody can create a malicious PR. So for private repositories where all users are members and have user access control managed by the administrator I can control who is able to create a PR/fork.

On Pull Request in private controlled repo that only trusted people can push to, trigger a GA workflow to run a series of cypress tests to confirm that the updated package doesn't break core functionality. This requires installing the app and running a node server using a bunch of secrets stored in Github Secrets. Without access to all those secrets, the app doesn't start.

The issue isn't that "a malicious PR is created by someone in your org", the issue is that Dependabot creates a PR that pulls in a new version of a dependency, and that dependency may contain malicious code. If everyone on this thread is operating in an environment where they either use no external dependencies, or they have a world class security organization that vets every single update prior to it being available in a private registry, it would be "safe" to operate without this check, but practically, the majority of software organizations don't operate this way.

To make it even more concrete, something like the eslint credential theft could run in this environment and steal your credentials. We want to ensure that developers are safe from this by default, and made aware of it for those who choose to do it, hence the big scary-looking red box in the docs and the link to the blog post on how to protect yourself.

@derTobsch:

maybe you can help me out. Are there different event_types or information in the github context for 'forked pull requests 'and 'normal pull requests'? I cannot find anything about that just the 'pull_request' event_name.

You're looking for these docs: https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull-request-events-for-forked-repositories

@covertbert:

Just to clarify @asciimike, is the plan to also make the new "Dependabot secrets" available to Dependabot-created, pull_request-triggered workflow runs? Or is the long term solution to implement one of the workarounds mentioned above?

We just launched them publicly on Monday, so we don't have immediate plans to fork over (especially since we've got some learning to do on how this change was made), but it's something we've discussed internally.

@khitrenovich:

I don't think the combination of pull_request_target and ref: ${{ github.event.pull_request.head.sha }} fully solves the issue - not for us, at least.

What are you observing as the difference in behavior between:

on:
  pull_request

jobs:
  do-some-work:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2

which is presumably what existed before, and

on:
  pull_request_target

jobs:
  do-some-work:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
        with:
          ref: ${{ github.event.pull_request.head.sha }}
  • 👍1
  • ❤️2
khitrenovich commented

@asciimike -
I have workflows that are configured as

on:
  push:
    branches:
      - master
  pull_request:
    branches:
      - master

or even

on:
  workflow_dispatch:
  schedule:
    - cron:  '13 * * * *'
  pull_request:
    branches:
      - master

For runs triggered by master merges, on schedule etc there will be no pull_request data on the event. How it is supposed to work then? Besides that, sometimes we modify worrkflows themselves, and I do want those changes to go through our regular set of checks - which seems impossible with pull_request_target setup.

asciimike commented

@khitrenovich I admit I don't know if I can fully understand what you're trying to do without diving deeply into your setup, but it sounds like you're already living in a world where you've got different event data coming in, and are presumably branching based on those events?

Something like if: ${{ github.actor == 'dependabot[bot]' }} might be helpful for identifying Dependabot specific behavior.

push is the only one where pull_request_target doesn't exist, so you'd have to break that out into a separate step with a workflow_run doing any of the secret/token management.

agrobbin commented

@asciimike is there an appetite for allowing Dependabot PRs to potentially access secrets without being given write-access to the repo? Or are those intrinsically linked to one another?

The reason I ask is that we are users of Knapsack Pro and Percy, both of which require a token to be present in our CI. Since we want Dependabot to run CI against new versions of dependencies, they need those secrets, which currently are stored in the GitHub Actions Secrets system. While neither of those tokens really have to be secret, in that they don't give access to a production-like system (deployment or anything), the ideal situation would definitely be to have those remain secret, while letting Dependabot access them.

The issue is that the only way to do that is to either give Dependabot read/write access to the repo and secrets access via the pull_request_target event (which I tried, and for whatever reason, it didn't work), or hard-code the secrets into our ci.yml workflow file. Of course that is not ideal either, but the repo is private, so that actually seems considerably less risky from a security perspective then giving Dependabot write access to our repo just so it can also access those secrets.

This is all a long way of asking if I am either doing something terribly terribly wrong, or if there is a potential middle ground available here for situations like this.

khitrenovich commented

@asciimike I have a workflow, which is complicated enough, so I don't want to replicate all its steps. There is a set of secrets that this workflow needs to run - read-only token to our private NPM repo to install the dependency and Percy token are just some of the examples. I want to run this workflow on all incoming PRs (with target/master context for dependabot and branch context for all the rest), on master merges (to make sure new changes did not break stuff), on schedule (to make sure our external/integrated systems did not break stuff on us in periods we have no merges), and also manually (to occasionally retrigger runs that had failed). How do I configure such a build system with the new restrictions in place?

  • 👍1
asciimike commented

@agrobbin: some options the Actions team is looking at, from above:

  • allowing use of Dependabot's secrets (instead of Actions)
  • letting developers customize the token permissions on a workflow run

No timeline on either, but it's definitely something we're talking about offering.

I think there's probably also a world where offering an "env vars" product (like secrets, but designed to, well, not be secret) that allows injection into the env without having to put them in code like the current environment variables are.

  • 👍3
agrobbin commented

Interesting, all of those sound like great potential solutions @asciimike!

In the meantime, unfortunately it doesn't sound like there's currently an option for Dependabot to access secrets without also getting a repo write token. While I understand the security posture of Actions itself, the "workarounds" suggested above for Dependabot actually seem like larger security risks to me than simply hard-coding Knapsack/Percy tokens into our workflow file.

While that may not be the case for some who have things like deployment tokens and other production-related secrets available to Actions, if it's simply test-related concerns, hard-coding the secrets so that Dependabot can access them (while not giving Dependabot a write token), seems like a much smaller security risk.

  • 👍1
mercuriete commented

One idea that I have is to create a bot/action that every X minutes try to retry CI Y times with a PAT using GitHub API.
(that is what I am doing manually)

With that idea, all my workflows will work but I don't know how difficult to implement it would be.

mattvb91 commented

I've added the secrets on the dependabot tab on organization level as described here but they are still not being received in the runs: https://docs.github.com/en/code-security/supply-chain-security/managing-encrypted-secrets-for-dependabot

mercuriete commented

@mattvb91 I am not 100% sure but that tutorial is for dependabot to discover new updates on private registries not for actions to receive the secrets.

  • 👍2
mattvb91 commented

@mattvb91 I am not 100% sure but that tutorial is for dependabot to discover new updates on private registries not for actions to receive the secrets.

puuh thanks ok I may have misinterpreted that. This is such a huge mess :(

  • 👍2
mattvb91 commented

@asciimike could you clarify this as im a bit confused:

    if: ${{ github.actor == 'dependabot[bot]' }} <------
    steps:
      - uses: ...
        with:
          github-token: ${{ secrets.GITHUB_TOKEN }}

couldn't someone just make a PR with the if line removed and now we are back to the same problem?

billinghamj commented

@mattvb91 No. Because it's running on pull_request_target, it uses the workflow defined in the target of the PR (e.g. master), so any changes to the workflow in the PR won't be used

  • 👍4
mattvb91 commented

Ok now its clicked, that makes sense. Thank you!

  • 👍1
mercuriete commented

I can confirm that using pull_request_target can access to personal access token but:

if you or a bot write this comment:
@dependabot merge
The author and the committer is dependabot. The action in main branch will fail due to same bug.

For me it doesn't make sense because you are already in your main branch and there are no security issue.

Summarizing:
@dependabot merge command is totally broken.

@asciimike do you recommend me to implement on: workflow_run after push on main branch?
The tutorial on the blog only talks about workflow_run after a pull_request not a push action on main branch.

  • 👍1
asciimike commented

Your action does an automerge by responding @dependabot merge? I might try merging the PR differently to get it out of Dependabot's flow altogether.

I'm talking to the docs folks about getting better documentation on all of this up, so hopefully we can have a better documented path shortly.

  • 👍1
missedone commented

-- Edited with @khitrenovich 's comment below #3253 (comment)

here is my workaround after reading through this thread, it reuses the same workflow yaml for both push/pull_request/pull_request_target in a private github repo.

name: CI

on:
  push:
    branches:
      - main
  pull_request:
    branches:
      - main
  pull_request_target:
    branches:
      - main

jobs:
  build:
    name: Build
    runs-on: [ubuntu-latest]
    if: github.event_name == 'push' || (github.event_name == 'pull_request' && startsWith(github.head_ref, 'dependabot/') == false) || (github.event_name == 'pull_request_target' && startsWith(github.head_ref, 'dependabot/')) 
    steps:
      - name: Dump GitHub context
        env:
          GITHUB_CONTEXT: ${{ toJson(github) }}
        run: echo "$GITHUB_CONTEXT"

      - name: Get PR git sha
        id: gitRef
        run: |
          if [ "${{ github.event_name }}" == 'pull_request_target' ]; then
            echo "::set-output name=sha::${{ github.event.pull_request.head.sha }}"
          else
            echo "::set-output name=sha::${{ github.sha }}"
          fi
      - name: Checkout code
        uses: actions/checkout@v2
        with:
          ref: ${{ steps.gitRef.outputs.sha }}

      - name: Test
        run: |
          curl -u github:${{ secrets.GITHUB_PAT }} https://api.github.com/user
          curl -u github:${{ secrets.GITHUB_PAT }} https://api.github.com/rate_limit
  • 👍2
mercuriete commented

@asciimike I have different kind of actions that started to fail but the one I am talking is the following one:

name: auto-merge

on:
  pull_request_target:

jobs:
  auto-merge:
    runs-on: ubuntu-latest
    if: github.actor == 'dependabot[bot]'
    steps:
      - uses: actions/checkout@v2
      - uses: ahmadnassri/action-dependabot-auto-merge@v2.3
        with:
          github-token: ${{ secrets.token }}
          target: minor

That action is working and comment to the PR "@dependabot merge". After the PR is merged... the action in main branch fails because it cant read secrets.

I will try @missedone solution.

khitrenovich commented

@mercuriete @missedone

This is what I ended up with - note the "duplicate" checkout:

name: build
on:
  pull_request:
    branches:
      - main
  pull_request_target:
    branches:
      - main

jobs:
  build:
    runs-on: ubuntu-latest
    if: |
      (github.event_name == 'pull_request_target' && startsWith(github.head_ref, 'dependabot/npm_and_yarn/')) ||
      (github.event_name != 'pull_request_target' && !startsWith(github.head_ref, 'dependabot/npm_and_yarn/'))
    steps:
    - name: 'Checkout'
      if: ${{ github.event_name != 'pull_request_target' }}
      uses: actions/checkout@v2
    - name: 'Checkout PR'
      if: ${{ github.event_name == 'pull_request_target' }}
      uses: actions/checkout@v2
      with:
        ref: ${{ github.event.pull_request.head.sha }}

The issue I'm dealing with now is Percy - with this setup, they detect Dependabot PR builds as "master"/baseline 🤕

  • 👍1
  • ❤️1
khitrenovich commented

@agrobbin I see you mentioned Percy. Are you running into the same issue with current branch detection?

mattvb91 commented

So ive added pull _request_target:
https://github.com/corejam/corejam/blob/next/.github/workflows/node.js.yml#L10

and last night some dependabot actions went through, however they are back to failing again. I have no idea how to debug this. Secrets are still empty on dependabot runs: https://github.com/corejam/corejam/runs/2234950319?check_suite_focus=true#step:9:8

However some went through last night where it has it. I have no idea what the difference is: https://github.com/corejam/corejam/runs/2230540664?check_suite_focus=true#step:9:8

khitrenovich commented

@mattvb91 Note that actions/checkout@v2 will pull target branch when running from pull_request_target trigger, so you are effectively building your next branch instead of the PR branch. And to your question - those builds that "went through" could have been re-triggered manually (and thus got access to secrets).

mattvb91 commented

Does anyone have a working example where they didn't have to split everything into 2 workflows? Im afraid if I start changing things now im going to break everything for pushes

edit: ah there was more hidden replies I didnt see

Also I still dont understand how this fixes any sort of security issue. Ok you now use pull_request_target so it only triggers the upstream workflow. But I can still just dump out process.env in my code? How does that change anything?

mercuriete commented

I am disabling dependabot entirely and evaluating other competing solutions.

I will back to dependabot when this simple workflow works:

  1. a dependency tool do a pull request
  2. a developer tell the tool to merge to integration branch
  3. Integration branch is deployed to staging environment in cloud infrastructure.

Thanks for your work.
I will keep subscribed until I can implement something like the described workflow in an automated way.

  • 👍1
cahaseler commented

@mercuriete I am doing the same thing - I recommend https://github.com/renovatebot/renovate, does near enough the same thing and is actually more extensible. Shame it's not native GitHub, but it actually works which is far more important.

  • 👍2
  • ❤️2
mattvb91 commented

@khitrenovich thanks I have taken a sample from above to get the correct ref. Still not receiving secrets currently tho any other ideas what I could be missing? https://github.com/corejam/corejam/runs/2236981643?check_suite_focus=true#step:10:8

https://github.com/corejam/corejam/blob/641af7614fdab9bcafe0e90d3ad85d32a394d696/.github/workflows/node.js.yml

khitrenovich commented

@khitrenovich thanks I have taken a sample from above to get the correct ref. Still not receiving secrets currently tho any other ideas what I could be missing? https://github.com/corejam/corejam/runs/2236981643?check_suite_focus=true#step:10:8

https://github.com/corejam/corejam/blob/641af7614fdab9bcafe0e90d3ad85d32a394d696/.github/workflows/node.js.yml

@mattvb91 -
Your workflow is actually now running on push to dependabot branch and not on pull_request_tareget, hence no secrets.

image

mattvb91 commented

@khitrenovich thanks I never noticed that. So is that to do with the order of the on options?

khitrenovich commented

@khitrenovich thanks I never noticed that. So is that to do with the order of the on options?

@mattvb91 You'll need to adjust the if conditions on your jobs, see my sample above.

mercuriete commented

@mattvb91 you have to drop all pipelines on: push and restrict only for integration branch, and leave only on:pull_request_target on the remaining branches

after that you need to understand that you are not able to do automerge or use the command "@dependabot merge" because it will trigger on: push on integration branch without secrets and it will fail on integration branch.

I think @khitrenovich example is very good. I did the same using two files instead if

main.yaml

on:
  push:
    branches:
      - main

pull_request.yaml

on: pull_request_target

using two files is the same as using if (or very similar)

mattvb91 commented

thanks @mercuriete but I wasnt using automerge or the @dependabot merge command as I was always merging them manually.

I think the only solution is to move to another service because I cant invest time into maintaining multiple actions .yamls with duplicate content just for dependabot

  • 👍2
khitrenovich commented

I admit I'm pretty close to dumping dependabot too. While I managed to keep it a single file, but still cannot make Percy work with this setup - and this whole saga took way too much of my time.

  • 👍5
mattvb91 commented

I'm just surprised this hasnt affected way more repos across github. I dont think our use cases are that much of an edge case...

  • 👍4
agrobbin commented

@agrobbin I see you mentioned Percy. Are you running into the same issue with current branch detection?

@khitrenovich what we've resorted to, unfortunately, is just hard-coding the PERCY_TOKEN into our .github/workflows/ci.yml. While it's absolutely not ideal, it's "exposure" is acceptable, compared to having to go down the road of separating the workflow, then figuring out how to tell Percy the current branch.

@asciimike I'm sure you have a lot on your plate, but offering a way to let Dependabot access certain secrets, to keep them out of code, but accessible for situations like Percy, Knapsack Pro, and others, would be a great option!

khitrenovich commented

@agrobbin Unfortunately, I need much more than Percy token, so I had to go down the pull_request_target path. I have a support ticket open with Percy/BrowserStack - given how much time it takes them "to do the research", they don't have clear answer themselves...

mattvb91 commented

@asciimike can you please clarify my current understanding of this. Dependabot had a security issue so you decided to close it by forcing users to use pull_request_target which needed manual action by the user (this is the important part)

I can go to any of the users repositories above and dump out their secrets in a pull request that have specified they are using pull_request_target, therefore making ourselves / themselves a target again for getting secrets stolen? Im not sure I see how this change solves anything other than make it more complicated for maintainers (they are still open to having secrets stolen)

Why not instead just have an interface on a PR to allow repo owners to "allow secrets on this PR run"? It would solve the whole issue

  • 👍4
asciimike commented

#3253 (comment) has the why and the options (pull_request_target is the easy/insecure "old" way, and the two workflows is the more secure "new" way).

Why not instead just have an interface on a PR to allow repo owners to "allow secrets on this PR run"? It would solve the whole issue

Is that not effectively what pull_request_target is (it allows secrets and a read-write token on a run)?

I've outlined the changes I've proposed to the Actions team several times in this thread (e.g. using Dependabot secrets, allowing token scope changes). If/when those changes occur, I'm happy to update this thread.

tsimbalar commented

Thanks a lot @asciimike and the rest of people for the explanations and clarifications 👍

My understanding from what I am reading is therefore that :

  1. "Dependabot commands" such as @dependabot squash and merge or @dependabot merge are and will remain broken right ? They would cause a merge and then a run of push workflows, running without access to Secrets , and therefore very likely to fail. I guess those features should just be removed then ? (in favor of, for instance "auto-merge" enabled on the repo ? )
  2. Dependabot "updates for GitHub Actions" : this is possibly broken too. If we use pull_request_target , then the actions run in the context of the base branch ... does it mean we wouldn't see the "just updated github actions" in those workflow runs ? this means we would then be running PR checks that are actually ... not checking the PR 🤔

Thanks a lot, we are also trying to figure out a way to fix this. To be honest, we were in the phase of migrating from "Dependabot-preview" to the "real GitHub Dependabot ™️ " , but we are definitely having second thoughts.

I've seen from https://github.community/t/dependabot-doesnt-see-github-actions-secrets/167104/22 that at the organization level, there is also a checkbox to "allow Send secrets to workflows from pull requests" in private repos . Would this change anything for dependabot ?

essenmitsosse commented

I also just ran into the second point @tsimbalar mentioned. Dependabot created a PR to update Cypress to a new major release and everything went fine — until the next PR was opened, which was the wirst one to actually run the major Cypress version and it broke.

So basically the "hotfix" pull_request_target to get Dependabot working again actually breaks the whole PR process for. things, that touch what's actually running during the PR process.

At first I thought this was a reasonable compromise, but now I'm having second thoughts (after already spending quite some time getting stuff back running again). Dependabot forces a lot of people to do unnecessary complicated things in an attempt to protect you from some very specific security issues.

Having the ability the handle which secrets can be used by pull request would be a much more simple and straight forward solution in my opinion.

Also like I mentioned before: I already have to declare the secrets a certain process can use by hand. So as long as I only have processes running during a PR that use secrets, that are non-critical, the cure seems worse than the disease here.

As much as I'd like to use Dependabot, actually migrating away from it seems like a less time consuming and error prone solution at the current stage. Also if that is how security issues need to be handled, I am worried about the next security issue that will be discovered and how much time will be necessary to rewrite and hotfix PR logic again.

  • 👍1
mattvb91 commented

#3253 (comment)

Is that not effectively what pull_request_target is (it allows secrets and a read-write token on a run)?

Nope not at all. I can go into any repository here that have identified themselves and I can dump out their environment variables here. What we need is a button to only run our CI with secrets once we have reviewed the PR

essenmitsosse commented

Ok sorry, I see now where I had an error in my logic about the enabling secrets per process and why this doesn't solve the problem.

It still stands that it would be a more viable solution to greenlight certain secrets for PRs or PRs that are opened by Dependabot, so that it runs with the actual process described in the PR, not the one in the main branch. This makes it impossible to test changes to the PR process in a PR.

  • 👍3
asciimike commented

Quick update: if you follow the GitHub Changelog closely, you may have noticed: https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/

If you read more closely, you'll notice:

Pull requests from public forks and Dependabot are still considered a special case and will receive a read token regardless of these settings.

Do not be dismayed, this is still good news! pull_request_target (which previously had a write token) can be downscoped to only contain the permissions you need in the run, e.g.:

name: Dependabot PR Check
on:
  pull_request_target

permissions:
  packages: read

jobs:
  check_dependabot:
    runs-on: ubuntu-latest
    if: ${{ github.actor == 'dependabot[bot]' }}
    steps:
      - run: echo "Hello from Dependabot"

Now has just the packages: read scope (well, and the metadata: read scope, but it needs that).

image

If you pull in the minimal secrets required for the workflow, you should be Reasonably Secure™ (note: consult your local security team for approval).

  • 🎉7
emilgoldsmith commented

I actually think it's a good design decision to not create APIs that allow developers to shoot themselves in the foot security wise, and I learned quite a lot about the vulnerabilities of CI by having to do this work.

For me the most frustrating part is that if I do @dependabot merge the push event on my main branch also doesn't get any secrets. I feel like any push event on the main branch should be trusted always right? One usually has required reviewers, status checks etc. set up so that things won't end up on the main branch if they aren't secure, now I feel I need to put a lot of if checks in all my workflows to skip them if it's a dependabot merge, as I for example push to docker hub to allow my other workflow runs to use images from there in the next workflows, and also couldn't auto deploy in that case.

  • 👍11
  • 👀1
Kocal commented

Having the same issue than #3253 (comment), I've was able to make Dependabot pull requests auto-approvable and auto-mergeable again (changing on.pull_request to on.pull_request_target), but how should we deal with a on.push on the default branch?

Each time there is a push on master (after merging a pull request), we trigger a deploy through Ansible AWX's API but it requires two secrets that cannot be read since Dependabot that merged the pull request.

What should we do? At the moment I don't see any solutions except inlining secrets... 😕

  • 👍2
feelepxyz commented

@Kocal it's a bit of a pain to set up but you can create two workflows, one that triggers on push without write and another that triggers when the first one completes with secrets.

We've set up an example project here that builds PR changes on push without secrets and then pushes the changes to the branch in a separate workflow.

Another tip for setting up auto-merging is to use the gh cli (which will respect branch protections/requirements):

steps:
  - name: Enable auto-merge for Dependabot PRs
    if: ${{ github.actor == 'dependabot[bot]' }}
    run: gh pr merge --auto --merge "${{github.event.pull_request.html_url}}"
    env:
      GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
  • 👍1
  • 👀1
Kocal commented

Thanks @feelepxyz! I was able to successfully make our deploy when pushing on master working for Dependabot pull requests too! 😄

For people in the same situation than me, this is what I've done:

  1. Given our initial auto-deploy workflow:
# .github/workflows/ci.yml
name: Deploy

on:
  push:
    branches:
      - master

jobs:
    deploy:
        runs-on: ubuntu-latest
        steps:
            - run: |
                curl -X POST ${{ secrets.TOWER_URL }}/api/v2/job_templates/YY/launch/ --header 'Authorization: Bearer ${{ secrets.TOWER_OAUTH2_TOKEN }}'

            - run: |
                curl -X POST ${{ secrets.TOWER_URL }}/api/v2/job_templates/YY/launch/ --header 'Authorization: Bearer ${{ secrets.TOWER_OAUTH2_TOKEN }}'
  1. I've created a new workflow that will be completed or failed depending if it's ran by Dependabot or not:
# This workflow runs with a READ-ONLY GITHUB_TOKEN
# https://github.blog/changelog/2021-02-19-github-actions-workflows-triggered-by-dependabot-prs-will-run-with-read-only-permissions/
#
# This workflow exists in order to re-enable auto-deployment after a pull-request from Dependabot has been merged.
#
# The completion of this workflow triggers the `Deploy` workflow which has a read-write GITHUB_TOKEN,
# deploying the project through Tower.
#
# More info: https://github.com/dependabot/dependabot-core/issues/3253#issuecomment-833485162

name: Deploy (check for Dependabot)

on:
  push:
    branches:
      - master

jobs:
  deploy_check_for_dependabot:
    runs-on: ubuntu-latest
    steps:
      - run: true
        if: github.actor == 'dependabot[bot]'

      - run: false
        if: github.actor != 'dependabot[bot]'
  1. Then, I can update our deploy workflow like this:
# This workflow runs:
#  - after someone who is not Dependabot has pushed to master.
#  - or after workflow "Deploy (Check for Dependabot)" completes
#
# It runs with a READ-WRITE GITHUB_TOKEN from the default branch.
# https://github.blog/changelog/2021-02-19-github-actions-workflows-triggered-by-dependabot-prs-will-run-with-read-only-permissions/
#
# Running this workflow after workflow "Deploy (Check for Dependabot)" completes allows us to re-enable auto-deployment after
# a pull-request from Dependabot has been merged.
#
# More info: https://github.com/dependabot/dependabot-core/issues/3253#issuecomment-833485162

name: Deploy

on:
  push:
    branches:
      - master
  workflow_run:
    workflows: ["Deploy (check for Dependabot)"]
    types:
      - completed

env:
    TZ: UTC

jobs:
    deploy:
        if: |
             (github.event_name == 'push' && github.actor != 'dependabot[bot]')
             || (github.event_name == 'workflow_run' && github.event.workflow_run.conclusion == 'success')
        runs-on: ubuntu-latest
        steps:
            - run: |
                curl -X POST ${{ secrets.TOWER_URL }}/api/v2/job_templates/XX/launch/ --header 'Authorization: Bearer ${{ secrets.TOWER_OAUTH2_TOKEN }}'

            - run: |
                curl -X POST ${{ secrets.TOWER_URL }}/api/v2/job_templates/YY/launch/ --header 'Authorization: Bearer ${{ secrets.TOWER_OAUTH2_TOKEN }}'

This way, PR merged to master by our teams are still deployed automatically, and same for Dependabot pull requests! 😄

  • 🎉3
mpdude commented

When I want to test my changed/adapted workflows, what is the easiest way to trigger a run that behaves like a "real" Dependabot PR?

feelepxyz commented

When I want to test my changed/adapted workflows, what is the easiest way to trigger a run that behaves like a "real" Dependabot PR?

PRs from forks should behave the same, or alternatively run @dependabo recreate on an existing open dependabot PR to have it force push changes to the branch: dependabot/dependabot-actions-workflow#2 (comment)

  • 👍1
mpdude commented

I cannot really wrap my head around this...

In order to run tests on the Dependabot PR, I need to pull a private Docker image from the AWS ECR. This is probably the same as in this comment where Alex said he needs a token for Percy to do visual regression testing for the changes proposed.

No matter how I try to approach it, at the end of the day I need to be running untrusted code (the updated dependency; Dependabot might even suggest to update to a malicious version of my Unit Test framework, just to mentally cut things short).

If I have a GITHUB_TOKEN with write access, the code might abuse it while the action runs. If I have secrets in the workflow, they might be exfiltrated. But even if it's only about other dependencies from private repos, this code might be copied anywhere over the network, what might open other loopholes... so, where do I stop?

I don't get how a pull_request_target-based workflow or copying things across "secure" and "insecure" workflows via artifacts could solve this. Apart from that such workflows are really complex and hard to maintain, all the suggestions of "treating PRs as data only" etc. simply don't work for updating dependencies & running tests to see if that breaks things, or do they?

So, will I have to switch to pull_request_target to make clear that I understand and accept the risks? Or is there another way out that I don't see?

  • 👍1
asciimike commented

This is what pull_request_target is for: you can scope the token down to exactly what you need, and be sure to only pull in the secrets required to do the fetch (and in turn, make sure those are scoped to only fetch the required artifacts).

There are definitely cases where folks have to run untrusted code, and this should allow for as minimal a blast radius as possible.

mpdude commented

When I use pull_request_target, it uses the “known safe” workflow definition from the PR base branch.

But, to my knowledge, action code is downloaded to the worker node at the beginning of the workflow run. Right?

So, once I have executed potentially malicious code (tests on updated dependencies), can I still trust actions being run later on, even if the workflow definition is known to be safe?

@asciimike Could you please help me to understand this:

and be sure to only pull in the tokens required to do the fetch (and in turn, make sure those are scoped to only fetch the required artifacts).

I was only aware of a single token, namely GITHUB_TOKEN. Is that what you are referring to?

How could I scope or limit access to selected GA secrets? Or use different secret values (e.g. accounts with less privilege) when running as Dependabot?

Last, does anyone have an example of an “integrated” (single file) workflow that supports both regular pull_request events for regular users (organisation members) and pull_request_target for Dependabot only, that in both cases checks out the merge commit, works for @dependabot merge commands and has access to secrets?