-
Notifications
You must be signed in to change notification settings - Fork 70
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
Recommend ${{ github.token }}
instead of PAT
#78
Comments
${{ github.token }}
instead of PAT
Concretely, I'd like to propose changing this line:
to:
...then have that change applied to the upstream This shouldn't break existing users that do specify Then we could rephrase the documentation to make the PAT setup optional, if you prefer the long-lived auth token for some reason. @laurentsimon I didn't see this discussed at all in #21, was this idea discussed and rejected for some reason? |
Hi @imjasonh, please see discussions here and here. TLDR is that some features of Scorecard action do not work with the regular Could you point us to your successful run with |
Passing run: https://github.com/imjasonh/kaniko/runs/5007039311?check_suite_focus=true The repo has no configured secrets: Here's a failing run from before I added the |
GITHUB_TOKEN works but cannot access data for Branch Protection, so you'll never receive branch protection alerts in your dashboard. This is why we asked to use a PAT. I'll let @azeemshaikh38 follow-up with you. We are going to move to GITHUB_TOKEN and work on a better way to access branch protection. |
This is what I propose as next steps here:
@imjasonh does that sound good? @laurentsimon @olivekl fyi. |
That sounds great to me. The branch protection check is nice, but not if it means managing and possibly leaking a long-lived auth token. 😄 |
I saw this, came to check out the project (and read the README), and came here to say this! So needing a PAT was (or would be) a stopping point for me, and I think the default without branch protection (and suggesting to the user) to use The current issue is that a lot of users are going to stop reading when they see PAT (I certainly almost did!) If you want to ping me when the changes are done I can test it out on one of my repos for the first time and give feedback about the docs, etc. |
Thanks for the feedback, much appreciated. We'll update this thread once we have this fixed. |
@naveensrinivasan @justaugustus any thoughts on the above? This seems to align with our discussion on webhook check that requires more dangerous permissions. |
I would like to have 2 jobs in the action, that I can give the branch protection one the PAT with even more minimal permissions. additionally pinging the GH folks about this as currently for branch protection it is required to have a PAT, and I would like for that not to be the case |
that would require the ability to pass scorecard the list of checks you want to enable. This is technically do-able, but I need to double check nothing breaks if you were to upload multiple SARIF results instead of a single file. For branch protection I think it would work because we use a different
they are working on it /cc @josepalafox @jhutchings1 @davidstaheli |
The documentation recommends creating a read-only personal access token to run the action, when the action's own token (available as
${{ github.token }}
) seems to work just fine, and requires no setup.edit: AFAIK, the GitHub Actions token also cannot be used after the workflow is complete, unlike a PAT which lasts forever, and could grant an attacker access indefinitely -- though read-only, if configured as recommended.
Was there a reason the PAT is preferred over the GHA token?
If this change sounds fine, I'm happy to send a PR to update the docs.
The text was updated successfully, but these errors were encountered: