action-dependabot-auto-merge icon indicating copy to clipboard operation
action-dependabot-auto-merge copied to clipboard

Dependabot latest change renders this action unusable for public repos

Open ahmadnassri opened this issue 3 years ago • 55 comments

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

Starting March 1st, 2021 workflow runs that are triggered by a pull request from Dependabot will be treated as if they were opened from a repository fork. This means they will receive a read-only GITHUB_TOKEN and will not have access to any secrets available in the repository. This will cause any workflows that attempt to write to the repository to fail.

If your workflow needs to have a write token, you can use the pull_request_target event; however, this is not viable for public repositories due to security risks

I have not seen any success with pull_request_target simply because no dependabot PRs has landed on my private repos since I changed to using pull_request_target but will update this issue and the README if I can validate them working...

pull_request_target might be acceptable for private repos... but I don't believe that will be good enough for public ones.

ahmadnassri avatar Mar 11 '21 14:03 ahmadnassri

I tried the following methods:

  1. pull_request_target on private repos and it doesn't work for me
  2. on: workflow_run: and it doesn't work for me with the same behavior

I hope someone can double-check proving I did something wrong.

upstream bugs related: https://github.com/dependabot/dependabot-core/issues/3253 https://github.com/dependabot/dependabot-core/issues/2268

mercuriete avatar Mar 11 '21 14:03 mercuriete

for (1), I'm seeing the same issue; I haven't checked (2) yet, but if I find time later, I'll check it out and respond here

aaronArinder avatar Mar 11 '21 14:03 aaronArinder

for method 1: are you using a PAT?

ahmadnassri avatar Mar 11 '21 15:03 ahmadnassri

@ahmadnassri: I am, with repo scope (it's a private repo) from a user with push perms to the repo

(also, sweet action! I started trying to integrate it yesterday, which seems like bad timing 😆 )

aaronArinder avatar Mar 11 '21 15:03 aaronArinder

This article linked from the announcement suggests a 2-step strategy where the first workflow runs unprivileged and exposes the checked PR as a build artifact, and the second one fetches that artifact and merges the referenced PR.

akheron avatar Mar 11 '21 15:03 akheron

after further inspection of recent private repo PRs, it seems the "March 1st" date from the Github article is false ...

up-until March 8 things were working fine, only on March 9 did the issues start ... (thanks Github for the misleading info)!

this is why I was confused, becuase I saw the article indicating March 1st, and kept an eye on things for that date and ongoing, and everything seems to work fine ... until the 9th!

ahmadnassri avatar Mar 11 '21 15:03 ahmadnassri

@akheron the 2-step strategy is what I called on: workflow_run and for me behaves similar... github_token it is fine but secrets are empty or not injected. I hope someone can double-check that approach.

mercuriete avatar Mar 11 '21 15:03 mercuriete

@mercuriete: have sample code for that process?

aaronArinder avatar Mar 11 '21 15:03 aaronArinder

this code tries to comment a PR inconditionally

https://github.com/kairos-wallet/web/commit/1f221af6098f6d8f0df7c5b0649377f9f901a2d5

for me it failed like this (reverse patch): https://github.com/kairos-wallet/web/runs/2085091889?check_suite_focus=true when using github_token

and like this when using secrets: https://github.com/kairos-wallet/web/runs/2085126326?check_suite_focus=true

so given that I couldn't make a comment I stop trying using another action.

Another problem is:

Even if you are successful commenting a PR with the following text "@dependabot merge" the merge will be triggered with dependabot permissions.... and after that merge... the main branch will be triggered without full permissions failing Continuous Deployment to AWS or another steps you have on main branch.

TLDR; So even if we could make a comment on a PR the action will fail on main branch.

mercuriete avatar Mar 11 '21 15:03 mercuriete

alas, there's no sad emoji to respond with

aaronArinder avatar Mar 11 '21 15:03 aaronArinder

the merge will be triggered with dependabot permissions.... and after that merge... the main branch will be triggered without full permissions failing Continuous Deployment to AWS or another steps you have on main branch.

is this a new behaviour?

ahmadnassri avatar Mar 11 '21 15:03 ahmadnassri

note: it's REALLY hard to test further variations on this .. since @dependabot recreate doesn't really mimic an actual brand new pull request ... and dependabot is mostly triggered by a schedule or somebody publishing a change to a public package ...

anybody got thoughts on testing?

ahmadnassri avatar Mar 11 '21 15:03 ahmadnassri

I've been closing the PRs and using @dependabot close and then @dependabot reopen, but I'm not sure that actually mimics a brand new PR--it seems to get around the 5-recreate-cap, though

aaronArinder avatar Mar 11 '21 16:03 aaronArinder

I could mimic the behaviour with @dependabot rebase after a new commit on main branch.

@ahmadnassri no, It is not a new behaviour. Remember two days ago when we thought was random?

It turns out It wasnt It was a manual trigger on PR followed by a failure on main branch. And I think It can be reproduced just commenting @dependabot merge on a PR. (Because the merge is triggered by dependabot instead of a PAT)

mercuriete avatar Mar 11 '21 16:03 mercuriete

note: it's REALLY hard to test further variations on this .. since @dependabot recreate doesn't really mimic an actual brand new pull request ... and dependabot is mostly triggered by a schedule or somebody publishing a change to a public package ...

anybody got thoughts on testing?

If you change your main branch to "main-fake" you can downgrade packages and do whatever you want.... after your testing you can delete "main-fake" and then you can change the main branch to "main"

mercuriete avatar Mar 11 '21 16:03 mercuriete

If you change your main branch to "main-fake" you can downgrade packages and do whatever you want....

ah, good trick

ahmadnassri avatar Mar 11 '21 16:03 ahmadnassri

PSA: I have to start my main work day, which will keep me occupied and away from further trying to debug and address this issue, I appreciate the community's feedback and if ya'll keep testing / trying things, would apprciate if you log them in this issue, so that I can circle back to it.

ahmadnassri avatar Mar 11 '21 16:03 ahmadnassri

Would it be possible to have a scheduled workflow go through all open dependabot PRs and merge the ones that have passed all checks? It would probably be enough to run it e.g. once a day.

akheron avatar Mar 11 '21 16:03 akheron

Would it be possible to have a scheduled workflow go through all open dependabot PRs and merge the ones that have passed all checks? It would probably be enough to run it e.g. once a day.

I don't see why not. They're run on the origin repo, so you'll have the ability to post comments. A caveat that I would fear with something like this is that two (Dependabot) PRs might have succeeded individually, and one is not merge conflicting with the other, but when "combined" they'd potentially break CI. A solution to that would be to write something like this:

# Runs on a 1h cron job

first = true
second = true
for dependabot_pr in all_passing_dependabot_prs_sorted_by_oldest_to_newest:
  if first:
    approve_pr(dependabot_pr)
    post_comment("@dependabot merge", dependabot_pr)
    first = false
    continue
  if second:
    post_comment("@dependabot rebase", dependabot_pr)
    break

That way, it carefully merges one at a time and forces a rebase on the "next one". And repeat.

(sorry if that comes across as obvious but it's still worth pointing out)

peterbe avatar Mar 11 '21 19:03 peterbe

One thing I don't understand is; why would it be dangerous/insecure for public repos?. You can definitely "tighten" it by checking the PR author and check that it's not a PR coming from a fork, because dependabot makes its PRs on the origin repo.

I appreciate that you don't want to combine npm install (or make build or whatever) in the same context as you have write-access secrets. If you break it up so that there's a workflow_run workflow, then that workflow will not run npm install but only the steps that auto-merge does today (create an approved review request, post a @dependabot merge comment).

This is what @akheron suggested in this comment: https://github.com/ahmadnassri/action-dependabot-auto-merge/issues/60#issuecomment-796803903 ...if I understood that correctly.

This way you do any npm install in a in-secure context, and the PR comments in a secure context.

Not sure how this relates to the auto-merge action.

peterbe avatar Mar 11 '21 20:03 peterbe

PSA: confirmed that pull_request_target + PAT works fine for private repos. I got a wave of dependabot PRs come in over night and they all worked after switching to pull_request_target yesterday

ahmadnassri avatar Mar 12 '21 13:03 ahmadnassri

a side effect: even though switching to pull_request_target for this action works, since dependabot now makes its PRs are treated as a fork, you might have other workflows that end up failing (e.g. one you trigger on pull_request and do other activities like npm install with a private token)

that workflow no longer works (because of the secrets thing) and will fail, which makes @dependabot merge command not want to merge cuz of the failure...

so now you are faced with the prospect of having to change ALL the other workflows to also run on pull_request_target just so the auto merge works

ahmadnassri avatar Mar 12 '21 13:03 ahmadnassri

since dependabot now makes its PRs from a fork

Uh? I don't see that. https://github.com/mdn/yari/pull/3212 for example was made by Dependabot. It's on the original repo. Not a fork of the repo.

Either way, I'm going to test simply changing my auto-merge.yml from pull_request: to pull_request_target: and see if it solves things.

peterbe avatar Mar 12 '21 14:03 peterbe

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

"a pull request from Dependabot will be treated as if they were opened from a repository fork."

whatever internal magic (if statements) are doing in github's side to treat it as a fork /shrug

ahmadnassri avatar Mar 12 '21 14:03 ahmadnassri

@peterbe in your case, all workflows seem like they will work if you switch them to pull_request_target, except for lighthouse / performance workflow since it needs a secret .. (secrets.LHCI_GITHUB_APP_TOKEN) if you switch it, you might risk leaking that secret to any random PRs

ahmadnassri avatar Mar 12 '21 14:03 ahmadnassri

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

"a pull request from Dependabot will be treated as if they were opened from a repository fork."

whatever internal magic (if statements) are doing in github's side to treat it as a fork /shrug

Sorry. I misinterpreted your sentence. It says they'll be "treated" as if from a fork.

peterbe avatar Mar 12 '21 15:03 peterbe

Sorry. I misinterpreted your sentence. It says they'll be "treated" as if from a fork.

you did not misinterpret :smile: , I originally typed "from a fork" then I corrected what I typed after your response :angel:

ahmadnassri avatar Mar 12 '21 18:03 ahmadnassri

@ahmadnassri can you share with us what scope is needed for that PAT? Im will try again on monday.

mercuriete avatar Mar 13 '21 00:03 mercuriete

@mercuriete same as before repo:push

ahmadnassri avatar Mar 13 '21 01:03 ahmadnassri

Linking another auto-merge GitHub Action which suffers from this issue https://github.com/ridedott/merge-me-action/issues/814

char0n avatar Mar 15 '21 06:03 char0n

some person upstream comented this https://github.com/dependabot/dependabot-core/issues/3253#issuecomment-800503370

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.

I will try to use pull_request_target today I hope it helps somebody reading this issue.

mercuriete avatar Mar 17 '21 10:03 mercuriete

Just for people looking for solution to this:

At the end, this article contains new updated section (search for: Update: 16.3.2021) with possible solution to the problem.

https://www.linkedin.com/pulse/how-keep-your-npm-dependencies-up-to-date-without-wasting-gorej/

char0n avatar Mar 17 '21 10:03 char0n

So to summarize this discussion and the article @char0n mentioned, the best solution appears to be this:

name: auto-merge

on:
  pull_request_target:

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

This has the side effect as @ahmadnassri mentioned that workflow runs which run after a merge by this action don't have access to any secrets and may fail.

We should probably update the README for this action and explain the changes made by GitHub so other user who haven't found this issue can use this great action again 😄

BetaHuhn avatar Mar 17 '21 19:03 BetaHuhn

@BetaHuhn is the checkout necessary in your example? When you leave it out the behavior of the workflow is equivalent with the workflow without it.

char0n avatar Mar 17 '21 20:03 char0n

@char0n I don't think it is.

I just modified the usage example from the top of the README where it is included.

BetaHuhn avatar Mar 17 '21 20:03 BetaHuhn

@BetaHuhn I've run tests and got the exact same results with or without it.

Some examples in README contains checkout, some don't. Maybe it would warrant some explanation, so that it's clear.

char0n avatar Mar 17 '21 20:03 char0n

Maybe it would warrant some explanation, so that it's clear.

Definitely.

BetaHuhn avatar Mar 17 '21 20:03 BetaHuhn

Just to mention it here, I personally decided to switch to @koj-co's dependabot-pr-action as it merges the PR itself and other workflows running after it don't have any limitations like missing secrets.

One disadvantage of their action is that, it needs to run on a schedule instead of directly after the dependabot PR is created and it manually checks if all other CI jobs have passed when it runs, so if a job fails it won't merge it until it can check again on the next cycle.

BetaHuhn avatar Mar 17 '21 20:03 BetaHuhn

Just tried with pull_request_target on one of our private repos:

name: Dependabot PR

on: pull_request_target

jobs:
  auto-approve-and-merge:
    name: Auto Approve & Merge
    runs-on: ubuntu-latest
    if: github.actor == 'dependabot[bot]' || github.actor == 'dependabot-preview[bot]'
    steps:
      - uses: actions/checkout@v2
      - uses: ahmadnassri/action-dependabot-auto-merge@v2
        with:
          target: minor
          github-token: ${{ secrets.GH_ACCESS_TOKEN }}

The action completed successfully and even though the svc pipeline in the image below has write privileges on the repo, it refused to merge.

Screen Shot 2021-03-23 at 6 13 17 PM

EricHaggar avatar Mar 23 '21 22:03 EricHaggar

@EricHaggar are there any other limits on this repo? perhaps branch protections (as the error seems to indicate)?

these are all helpful information, I'm compiling a list of "gotchas" to add to the readme ... please keep them coming.

ahmadnassri avatar Mar 24 '21 02:03 ahmadnassri

@ahmadnassri The only restrictions are on the main branch. However, dependabot-preview is given permission to merge to main 🤔

Screen Shot 2021-03-24 at 11 14 12 AM

EricHaggar avatar Mar 24 '21 15:03 EricHaggar

@EricHaggar you need the personal access token (which belongs to a GitHub user) to have the permission, not dependabot itself

ahmadnassri avatar Mar 24 '21 15:03 ahmadnassri

@ahmadnassri The token that we're using is a global secret shared across our organization's repos.

I finally got it to work by the way 🎉

I had push restrictions on the main branch encircled in the screenshot below, so I unchecked it. I don't know if this is the best approach but since we still require a pull request + approval before merging into main, and we limit who has write access to our repo, it should be safe enough.

Summary: It works with the following configuration in a private repo:

  • on: pull_request_target
  • Requires token with write access (as described here)
  • Remove restrictions on who can push to your main/master branch. Example branch protection configuration below ⬇️

Screen Shot 2021-03-24 at 1 02 03 PM

EricHaggar avatar Mar 24 '21 17:03 EricHaggar

I can confirm that this actions works:

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/[email protected]
        with:
          github-token: ${{ secrets.token }}
          target: minor

But after merging with main branch. The pipeline on main fails due to push action being in fork context and can't access secrets.

mercuriete avatar Mar 24 '21 20:03 mercuriete

But after merging with main branch. The pipeline on main fails due to push action being in fork context and can't access secrets.

Any ideas on getting around this for main/default branch not accessing secrets?

ejntaylor avatar Apr 15 '21 13:04 ejntaylor

@raisonon I just give up with dependabot.

you can try renovatebot with automerge functionality. https://github.com/marketplace/renovate

sorry for the spam but dependabot is not usable for my use case anymore.

mercuriete avatar Apr 15 '21 13:04 mercuriete

Any ideas on getting around this for main/default branch not accessing secrets?

@raisonon As I already mentioned above, one way to keep using Dependabot instead of switching to renovate as @mercuriete mentioned, is to merge the PR with @koj-co's dependabot-pr-action. It merges the PR itself (instead of telling Dependabot to) and other workflows running after on main/default don't have any limitations like missing secrets.

BetaHuhn avatar Apr 15 '21 14:04 BetaHuhn

Thanks @BetaHuhn - would like to avoid a scheduled process but appreciate it's likely the best solution at present.

ejntaylor avatar Apr 15 '21 14:04 ejntaylor

I wonder if we can get around this by triggering a repository_dispatch workflow from the dependabot actor - i suspect then we wont have this secrets issue... https://github.com/peter-evans/repository-dispatch

ejntaylor avatar Apr 19 '21 10:04 ejntaylor

I added pull_request_target to my auto merge config and it works in most of the cases. Like here: https://github.com/bennycode/coinbase-pro-node/pull/494

But when Dependabot needs to rebase a branch, then it doesn't work anymore (Example: https://github.com/bennycode/coinbase-pro-node/pull/490). Is there a workaround for this problem?

bennycode avatar May 16 '21 05:05 bennycode

If there is a problem with rebasing, we could try using synchronize as target like so:

on:
  pull_request:
    types: [synchronize]

ffflorian avatar May 17 '21 14:05 ffflorian

I tried synchronize in another project of mine:

on:
  pull_request_target:
  pull_request:
    types: [synchronize]

But it looks like it runs in a different context with limited permissions because I am now seeing the following error:

/action/node_modules/@actions/core/lib/core.js:94 throw new Error(Input required and not supplied: ${name}); Error: Input required and not supplied: github-token

Build: https://github.com/bennycode/ig-trading-api/pull/176/checks?check_run_id=2645794063

EDIT: I found the solution to my problem. I forgot to run actions/checkout before ahmadnassri/action-dependabot-auto-merge, so it could not consider the semver version and failed to merge some of my dependencies. I don't need the synchronize target any longer and was able to solve my problem with the following code:

merge-dependencies.yml

name: 'Merge Dependencies'

on: [pull_request_target]

jobs:
  auto-merge:
    runs-on: ubuntu-latest
    # Guarantee that commit comes from Dependabot (don't blindly trust external GitHub Actions)
    if: github.actor == 'dependabot[bot]'
    steps:
      - name: 'Checkout repository'
        uses: actions/[email protected]
      - name: 'Automerge dependency updates from Dependabot'
        uses: ahmadnassri/[email protected]
        with:
          github-token: ${{ secrets.WEBTEAM_AUTOMERGE_TOKEN }}

bennycode avatar May 22 '21 11:05 bennycode

It seems Fastify has a similar Action which solves the permission access problem via a Github App https://github.com/fastify/github-action-merge-dependabot

evantahler avatar Jun 07 '21 17:06 evantahler

Also getting this for unknown reasons in private repositories.

EDIT: ah, seems like pull_request_target works. Would be nice to highlight this in the readme

steebchen avatar Jun 11 '21 20:06 steebchen

Been struggling with github-token credentials stuff for a while - there's a few open tickets about this. I am running this in a private repo. I have push permissions. I tried the following:

name: auto-merge

on:
 pull_request_target:

jobs:
 auto-merge:
   runs-on: ubuntu-latest
   steps:
     - uses: actions/checkout@v2
     - uses: ahmadnassri/action-dependabot-auto-merge@v2
       with:
         target: minor
         github-token: ${{ secrets.GITHUB_TOKEN }}

I get this: image

james-s-w-clark avatar May 23 '22 19:05 james-s-w-clark