Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove persist-credentials or change the default to false #485

Open
briansmith opened this issue Apr 26, 2021 · 10 comments · May be fixed by #1687
Open

Remove persist-credentials or change the default to false #485

briansmith opened this issue Apr 26, 2021 · 10 comments · May be fixed by #1687

Comments

@briansmith
Copy link

Currently one has to resort to explicitly specifying persist-credentials: false to avoid the credentials being persistent. My understanding is that persisting the credentials gives every step in the job that occurs after actions/checkout@v2 implicit access to the token. This is not what people expect and this leads people to write jobs that expose their repo to more risk than they otherwise would.

I propose the persist-credentials feature be removed completely and then v3 be released. Otherwise, if that's not practical, then at least the default should be changed to false.

@haampie
Copy link

haampie commented Oct 5, 2021

I can't believe the default is to persist credentials and expose them to other jobs :( this is a major security issue.

Just as a heads up for anyone stumbling upon this issue:

  1. persist-credentials: false is only relevant when you use ssh authentication, because
  2. GITHUB_TOKEN is always exposed to all jobs, and by default has write access to your repo.

So if you want to harden security, apart from setting persist-credentials: false for ssh auth, make sure that GITHUB_TOKEN auth has no write permission to your repo.

See https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/ for reference.

@fmg-dave
Copy link

+1

@eregon
Copy link

eregon commented Jul 27, 2022

Agreed this seems a severe security issue, because it means any workflow using actions/checkout basically leaks the token to any process/action in that workflow which can just read it from .git/config.

@haampie IIUC it is a problem also with no ssh authentication (the default). The GitHub token is given only to this action and maybe a few other actions/* actions (default: ${{ github.token }} only work for those AFAIK), but is otherwise given to no other action unless done explicitly (like with: token: ${{ github.token }}/${{ secrets.GITHUB_TOKEN }}).
The token is not in the environment.

In other words, actions/checkout leaks the token to .git/config, making it very easy to read for anything running inside the workflow.
If the token was not written to .git/config, then I think stealing the GitHub token would require (one of):

  • passing the token explicitly to some action in the workflow and that action gets compromised
  • actions/checkout gets compromised
  • compromise the workflow definition to e.g. print the token

So, depending on whether the token is explicitly passed to some action:

  • If yes, then it seems the only safety net is to set token permissions
  • If no, it would be safe by default with this change or with persist-credentials: false, regardless of the token permissions. A workaround is to set token permissions

I guess GitHub sees setting token permissions as the more general solution.
If so, fine, but then the default should be secure and so the default workflow permissions should be just contents: read.

https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ also has a mention related to this, search for persist-credentials:

If the workflow uses actions/checkout and does not pass the optional parameter persist-credentials as false, it makes it even worse. The default for the parameter is true. It means that in any subsequent steps any running code can simply read the stored repository token from the disk.

@mgoltzsche
Copy link

+1

cpu added a commit to cpu/rcgen that referenced this issue Aug 24, 2023
cpu added a commit to cpu/rcgen that referenced this issue Aug 25, 2023
cpu added a commit to rustls/rcgen that referenced this issue Aug 28, 2023
@briansmith briansmith changed the title Release a new version v3 with persist-credentials removed or false by default Remove persist-credentials or change the default to false Sep 5, 2023
@briansmith
Copy link
Author

I updated the issue title since v3 and v4 have already shipped, so asking for a new v3 doesn't make sense.

commonquail added a commit to commonquail/commitmsgfmt that referenced this issue Sep 23, 2023
commonquail added a commit to commonquail/commitmsgfmt that referenced this issue Sep 23, 2023
@haampie
Copy link

haampie commented Jan 15, 2024

Fast forward to 2024, and we have https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

PyTorch had several workflows that used the actions/checkout step with a GITHUB_TOKEN that had write permissions. For example, by searching through workflow logs, we can see the periodic.yml workflow also ran on the jenkins-worker-rocm-amd-34 self-hosted runner. The logs confirmed that this workflow used a GITHUB_TOKEN with extensive write permissions.

@ericsciple could you please take this issue seriously and disable persisting credentials to disk?

flwyd added a commit to flwyd/adif-multitool that referenced this issue Feb 4, 2024
persist-credentials defaults to true (see
actions/checkout#485).  It looks like
pull_request workflows run without token access, but it's not clear from
https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
if that means persist-credentials doesn't leave a secret in the .git
directory where a malicious PR could access it.
alxndrsn pushed a commit to alxndrsn/pouchdb that referenced this issue Mar 24, 2024
SourceR85 added a commit to pouchdb/pouchdb that referenced this issue Mar 24, 2024
See: actions/checkout#485

Co-authored-by: alxndrsn <alxndrsn>
Co-authored-by: Steven <[email protected]>
@swebb2066
Copy link

swebb2066 commented Apr 11, 2024

Not sure this is actually necessary. There seems to be a mismatch between the documentated default and the code

As I read the code, the 'persist-credentials' value will be false if the input does not contain a 'persist-credentials' entry.

The issue should be changed to a 'fix the documentation' if my understanding is correct.

michi-covalent added a commit to michi-covalent/checkout that referenced this issue Apr 20, 2024
Change the default value of persist-credentials setting from true to
false to reduce the risk of unintentionally exposing the GITHUB_TOKEN
secret.

Fixes: actions#485

Signed-off-by: Michi Mutsuzaki <[email protected]>
joeyparrish added a commit to joeyparrish/karma-local-wd-launcher that referenced this issue Dec 18, 2024
See actions/checkout#485 and https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials after checkout. There's no call for that, so we will now set `persist-credentials: false` on all checkout actions.
joeyparrish added a commit to joeyparrish/shaka-lab that referenced this issue Dec 18, 2024
See actions/checkout#485 and https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials after checkout. There's no call for that, so we will now set `persist-credentials: false` on all checkout actions.
joeyparrish added a commit to joeyparrish/shaka-packager that referenced this issue Dec 18, 2024
See actions/checkout#485 and https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials after checkout. There's no call for that, so we will now set `persist-credentials: false` on all checkout actions.
joeyparrish added a commit to joeyparrish/shaka-streamer that referenced this issue Dec 18, 2024
See actions/checkout#485 and https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials after checkout. There's no call for that, so we will now set `persist-credentials: false` on all checkout actions.
joeyparrish added a commit to joeyparrish/trace-anything that referenced this issue Dec 18, 2024
See actions/checkout#485 and https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials after checkout. There's no call for that, so we will now set `persist-credentials: false` on all checkout actions.
joeyparrish added a commit to joeyparrish/webdriver-installer that referenced this issue Dec 18, 2024
See actions/checkout#485 and https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials after checkout. There's no call for that, so we will now set `persist-credentials: false` on all checkout actions.
joeyparrish added a commit to joeyparrish/static-ffmpeg-binaries that referenced this issue Dec 18, 2024
See actions/checkout#485 and https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials after checkout. There's no call for that, so we will now set `persist-credentials: false` on all checkout actions.
joeyparrish added a commit to shaka-project/karma-local-wd-launcher that referenced this issue Dec 18, 2024
See actions/checkout#485 and
https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials
after checkout. There's no call for that, so we will now set
`persist-credentials: false` on all checkout actions.
tykus160 pushed a commit to shaka-project/shaka-github-tools that referenced this issue Dec 18, 2024
See actions/checkout#485 and
https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials
after checkout. There's no call for that, so we will now set
`persist-credentials: false` on all checkout actions.
tykus160 pushed a commit to shaka-project/shaka-player that referenced this issue Dec 18, 2024
See actions/checkout#485 and
https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials
after checkout. There's no call for that, so we will now set
`persist-credentials: false` on all checkout actions.

The only exceptions are for the release job, which wants to push a tag
and create a branch, each of which explicitly persist credentials now
and explain why in a comment.
tykus160 pushed a commit to shaka-project/eme-encryption-scheme-polyfill that referenced this issue Dec 18, 2024
See actions/checkout#485 and
https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials
after checkout. There's no call for that, so we will now set
`persist-credentials: false` on all checkout actions.
tykus160 pushed a commit to shaka-project/generic-webdriver-server that referenced this issue Dec 18, 2024
See actions/checkout#485 and
https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials
after checkout. There's no call for that, so we will now set
`persist-credentials: false` on all checkout actions.
tykus160 pushed a commit to shaka-project/webdriver-installer that referenced this issue Dec 18, 2024
See actions/checkout#485 and
https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials
after checkout. There's no call for that, so we will now set
`persist-credentials: false` on all checkout actions.
joeyparrish added a commit to joeyparrish/static-ffmpeg-binaries that referenced this issue Dec 18, 2024
See actions/checkout#485 and https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials after checkout. There's no call for that, so we will now set `persist-credentials: false` on all checkout actions.
joeyparrish added a commit to shaka-project/eme_logger that referenced this issue Dec 18, 2024
See actions/checkout#485 and
https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials
after checkout. There's no call for that, so we will now set
`persist-credentials: false` on all checkout actions.
joeyparrish added a commit to shaka-project/shaka-lab that referenced this issue Dec 18, 2024
See actions/checkout#485 and
https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials
after checkout. There's no call for that, so we will now set
`persist-credentials: false` on all checkout actions.
joeyparrish added a commit to shaka-project/shaka-packager that referenced this issue Dec 18, 2024
See actions/checkout#485 and
https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials
after checkout. There's no call for that, so we will now set
`persist-credentials: false` on all checkout actions.
joeyparrish added a commit to shaka-project/shaka-streamer that referenced this issue Dec 18, 2024
See actions/checkout#485 and
https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials
after checkout. There's no call for that, so we will now set
`persist-credentials: false` on all checkout actions.
joeyparrish added a commit to shaka-project/trace-anything that referenced this issue Dec 18, 2024
See actions/checkout#485 and
https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials
after checkout. There's no call for that, so we will now set
`persist-credentials: false` on all checkout actions.
joeyparrish added a commit to shaka-project/static-ffmpeg-binaries that referenced this issue Dec 19, 2024
See actions/checkout#485 and
https://johnstawinski.com/2024/01/11/playing-with-fire-how-we-executed-a-critical-supply-chain-attack-on-pytorch/

In short, it is a terrible idea to persist even our default credentials
after checkout. There's no call for that, so we will now set
`persist-credentials: false` on all checkout actions.
@qoomon
Copy link

qoomon commented Dec 27, 2024

@swebb2066 the action.yaml is not just documentation it really sets the default value to true

checkout/action.yml

Lines 52 to 54 in cbb7224

persist-credentials:
description: 'Whether to configure the token or SSH key with the local git config'
default: true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants