Dependabot triggered Actions cant access secrets or use a writable token #3253
Comments
|
I can confirm that. Now Dependabot can't read secrets anymore at all. |
|
Any secrets are not available for Dependabot. |
|
Can't share links to private repos, but here is a couple of public ones: mdreizin/chrome-bookmarks-alfred-workflow#96 |
|
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? |
|
the team support answer me with this: 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: and now my workflow is totally broken. Please add automerge to dependabot. Thanks. |
|
I am trying to fix the problem using a workaround: 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. |
|
I have the same issue see https://github.com/synyx/urlaubsverwaltung/pull/1882/checks?check_run_id=2075418072 |
|
yeah, I can confirm that moving from
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 :( |
|
Recommended solution by using https://github.com/mdreizin/gatsby-plugin-robots-txt/blob/master/.github/workflows/dev.yml#L6 |
|
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 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
|
|
people from this issue might want to subscribe to this issue: we all are facing the same issue. BTW, I tested the alternative and again it work for github_token but not for secrets (at least for me) |
|
I will try to move from dependabot to renovate to asset the feasibility of the migration. renovate: https://www.whitesourcesoftware.com/free-developer-tools/renovate |
|
@mercuriete The biggest issue with For instance, you have |
|
@mercuriete I am also considering migrating to The funny thing: all PRs created by The support team still has not provided any ways how to get rid of that issue. |
|
Maybe anybody from Dependabot team could provide a solution to fix missing secrets on PR created by |
|
hahaha 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. |
|
@mercuriete I am sorry, but they failed now by other reasons. I would happy to see how they would fix that: https://github.com/dependabot/dependabot-core/blob/main/.github/workflows/ci.yml#L46 before: https://github.com/dependabot/dependabot-core/pull/3160/checks?check_run_id=1924805335 after: https://github.com/dependabot/dependabot-core/pull/3249/checks?check_run_id=2063635316 result: secrets are empty |
|
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
Additionally, a bug was introduced where 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 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/checkoutEither use ### .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 🙇🏻 |
|
Re: @mercuriete's
Private registry support is coming out shortly, so stay tuned and follow this issue and the changelog. |
Hey @asciimike, I already tried that but without success. The workflows that that get triggered via 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/[email protected]
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
branch: ${{ github.ref }}but now the github token does not have the permission to push to this branch. |
|
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: everything that involves a personal access token. Please give us a working example of private registries making use a PAT. thanks |
|
@derTobsch: I just set up a It seems like the answer is " @mercuriete: 1 and 2 will be solved more elegantly via github/roadmap#67, which is imminent. 3 will have to use |
|
I have confirmed that |
|
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 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? |
|
RE using |
|
Are you sure that workflow_run wouldn't work? Did you try? That gets
totally disassociated from the branch event as far as I understand and
should always have secrets. You just have to move anything away from your
push event workflow to the workflow_run workflow which uses secrets and the
push event on main branch won't fail, though I do disagree personally with
the push event on main not being trusted
…On Fri, 28 May 2021 at 14:35, Ahmad Nassri ***@***.***> wrote:
RE using workflow_run as some folks pointed out in the thread to address
the issue after @dependabot merge it wouldn't address the issue, as the
subsequent triggered push event on main branch will still fail.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3253 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEKO66UYNXE27SYJIZ4MTXTTP6EYLANCNFSM4Y5PUFIQ>
.
|
I don't need to try it to know it wont work ... since it's a different context altogether. the comment / command even if I type it up manually by hand in the comments, the result will be the same. look at the commit itself: ahmadnassri/node-oas-request@1917083 when it's coming from dependabot as the author, that's when it loses access to secrets.
|
|
Yes I know. But then that merge triggers a push event that can trigger a
workflow run event through the workflow that has secrets access, and can
work the same way as for a pull request event. You should really give it a
go if you’re interested in getting Dependabot fully working again
…On Fri, 28 May 2021 at 17.18, Ahmad Nassri ***@***.***> wrote:
Are you sure that workflow_run wouldn't work? Did you try?
I don't need to try it to know it wont work ..
the comment / command @dependabot merge is what triggers a merge ... has
nothing to do with the pull request, or the workflow_run approach ..
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3253 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEKO66RCAWQBXDX446JTPJ3TP6X6DANCNFSM4Y5PUFIQ>
.
|
|
well, I'll give it a try, but if that DOES work, it seems the dependabot team has some fixing to do cuz it doesn't make sense to block |
The reason |
yeah, but we don't care at this point. If malicious code reach master, accessing github action secret it's not the worst case scenario, we should worry about the production code then. But if we trigger manually it works so in this case the malicious code would still have access to it. |
|
Also the next time we merge anything to master the code will also have
access to the secrets so seems truly unnecessary
…On Mon, 31 May 2021 at 08.32, Moroine Bentefrit ***@***.***> wrote:
it doesn't make sense to block push on master from getting secrets, just
because the author is dependabot
The reason push is blocked is the same reason pull_request is blocked:
malicious package code could be executed during this event.
yeah, but we don't care at this point. If malicious code reach master,
accessing github action secret it's not the worst case scenario, we should
worry about the production code then. But if we trigger manually it works
so in this case the malicious code would still have access to it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3253 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEKO66QRM2H2DZEXBKJKLHTTQMUQLANCNFSM4Y5PUFIQ>
.
|
|
@moroine @emilgoldsmith That was also my thought. Seems more like security theatre to me, then actually thread prevention. Making it really hard for people to use a tool, isn't the same as preventing a foot gun. And while there is a way you could argue why these changes are helping, there are also a lot of arguments for why they barely do, while they force everyone — no matter if they are in the line of danger or not — to run circles around a very specific edge case, that could still occur. The real problem here are projects, that leak unnecessary secrets to their merge steps, which could better be stopped by educating people and forcing them to actively approve those decisions, than by just making the workflow more complicated. And es stated above: making merging harder is next to pointless, because the malicious code could execute 2 minutes afterwards when the next pull request ist merged. On the one hand you treat devs like children, who need their toy taken away. On the other hand you force them to implement a very complicated, brittle and most importantly: hard to to understand* workflow, that basically makes this tool close to unusable, while introducing a lot of noise into a process that is supposed to keep your head clear. Im pretty sure this change will do more damage, by making people stop regular updates, than it will prevent by stopping any real world attack scenarios. Also if these changes make people stop using Dependabot and update their dependencies by hand, no one will have gained anything. *tests are run from the branch that is merged into, not the commit that is merged. Which is super counter-intuitive, and now every user of dependabot needs to understand this. |
|
As a company with private repositories and disallowed forking I just want to have an option to let dependabot read all secrets in all workflows. I don't want any workflow or process change. Seems to be as well that this particular case wouldn't be affected by potential security vulnerabilities (as described above) |
This would completely fix the issue if they just allowed us to run it as normal with a checkbox "allow potential sec issues". The most annoying part of this issue is the fact that this change does nothing to fix it, it just makes it more difficult for maintainers. The security threat is still the same |
This is exactly what has happened in our case. We just stopped updating dependancies because of this which introduces more potential for issues than there was in the first place |
|
@mattvb91 @BorntraegerMarc @essenmitsosse I just drop dependabot on all organizations managed by me and I started to use renovatebot. you can try renovatebot by dropping this file on the root of the project for example: renovate.json {
"extends": [
"config:base"
],
"schedule": ["after 7pm and before 7am"],
"prConcurrentLimit": 2,
"packageRules": [
{
"matchUpdateTypes": ["minor", "patch", "pin", "digest"],
"automerge": true
},
{
"matchDepTypes": ["devDependencies"],
"automerge": true
},
{
"matchPackageNames": ["aws-sdk"],
"extends": ["schedule:monthly"]
}
]
}This example is for a Nodejs project with automerge of patch version bumps on dependencies you need to install the application using the marketplace first: Remember this approach will have the security concern that dependabot has but It can be useful for some people with some use cases. I am not recommending using this alternative, I am just telling what I use meanwhile waiting for dependabot implementing a user-friendly solution. Thanks and I hope this helps someone. |
This comment has been minimized.
This comment has been minimized.
|
I agree that the design of this is broken, especially in the way that understanding what is happening here, and why, is very difficult for people who have not been following this thread closely (witness the number of misunderstandings that still keep coming up around this issue). If you have a workflow that needs access to secrets, and it runs but without having access to those secrets, it can lead to errors that are extremely hard to understand. I think a much better solution to this whole issue would be the following:
This has the following benefits:
|
|
What would be the „right“ way to create a merge request yourself in a |
@asciimike sorry for mentioning you again... is there a feature request somewhere about that so that I can give it a thumbs up? We havent made any changes yet (as I havent digged up the best approach yet, we simply rerun all dependabot prs atm manually). Since we have only one secret (the read only artifactory token), which is needed to install dependencies. PS: Dependabot private registry updates works great, however |
This comment has been minimized.
This comment has been minimized.
|
bumping because this thread is so hard to follow - anyone who ran into issues switching to GitHub native dependabot because your workflows need to access to GitHub secrets or auto merging, @Kocal's post is super helpful+clear, and did the trick for me |
|
Here is what I hope is a definitive FAQ related to this change. Docs have been published and will be updated, but I'll leave this thread up for the foreseeable future in case it's still helpful. TL;DR: See "GitHub Actions: Workflows triggered by Dependabot PRs will run with read-only permissions" and "Keeping your GitHub Actions and workflows secure: Preventing pwn requests". What and whyWhat changed?For GitHub Actions workflows initiated by Dependabot (
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. Is this actually a problem?Yes. Attacks like eslint-scope, which included a malicious Doesn't Dependabot not run
|
|
Above is a FAQ compiled from this thread, and hopefully answers all outstanding questions, or directs folks to the right channels. I recognize that people are upset about this change, and there are certainly plenty of things that we (GitHub) need to do in the future to both clean this up further as well as prevent similar situations from occurring again. That said, I believe this issue has outlived its usefulness, and I'm going to close and lock this issue now, as:
On 1: While everyone is welcome to question product decisions and engage in a productive conversation on how to resolve issues, when comments become pointlessly negative or turn personal, the community is actively harmed. Several recent comments fall into the latter camp, and (in addition to violating our code of conduct) it just sucks for everyone to have to deal with that level of negativity. On 2: the majority of the comments are feature requests from the Actions team (e.g. "how can I opt-in to the old behavior") that nobody in this repo can act on. These comments should be directed to the Actions team, who is in the best position to engage with you and get the right product built. Some chunk of the remaining comments are related to Dependabot features, which are already captured in this repo, or should be captured in separate issues. I am active and will engage appropriately. Again, I agree that this isn't an ideal situation for anyone, and I want to thank those of you who have rallied together to come up with as elegant a solution as we can at the moment. For those of you interested in continuing to carry that torch, the Actions team is willing to engage on the issues related to them, and I speak for the entire Dependabot team when I say we are also happy to engage on specific issues we have control of. |
asciimike
changed the title
|
There are two changes coming down the pipeline that will affect folks here:
With the two of these changes, folks should be able to regain the original functionality. |
and others
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
The text was updated successfully, but these errors were encountered: