Recommend ${{ github.token }} instead of PAT

[imjasonh]

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.

[imjasonh]

Concretely, I'd like to propose changing this line:

scorecard-action/starter-workflows/code-scanning/scorecards.yml

Line 49 in 9523c76

repo_token: ${{ secrets.SCORECARD_TOKEN }}

to:

repo_token: ${{ secrets.SCORECARD_TOKEN || github.token }}

...then have that change applied to the upstream starter-workflows repo.

This shouldn't break existing users that do specify SCORECARD_TOKEN, and doesn't preclude future users from specifying a secret PAT if they want to, but lets users who don't specify it to still have a functioning workflow.

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?

[azeemshaikh38]

Hi @imjasonh, please see discussions here and here. TLDR is that some features of Scorecard action do not work with the regular GITHUB_TOKEN and require a PAT. So I'm surprised that you were able to get your workflow running successfully with just GITHUB_TOKEN.

Could you point us to your successful run with GITHUB_TOKEN?

[imjasonh]

Config: https://github.com/imjasonh/kaniko/blob/59e03dcf6fe45d26567a55b27e4ad9ab7feb2972/.github/workflows/scorecards-analysis.yml#L33

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, because I don't have the secret: https://github.com/imjasonh/kaniko/runs/4999240550?check_suite_focus=true

[laurentsimon]

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.

[azeemshaikh38]

This is what I propose as next steps here:

1. Update our docs to say that we recommend GITHUB_TOKEN unless folks want to run Branch-Protection check in which case they need to provide a PAT.
2. Update our starter workflow to use GITHUB_TOKEN by default but provide instructions in comment to upgrade it to PAT if users want to enable Branch Protection.

@imjasonh does that sound good? @laurentsimon @olivekl fyi.

[imjasonh]

@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. 😄

[vsoch]

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 ${{ secrets.GITHUB_TOKEN }} makes most sense. And then if someone wants the advanced features (branch protection checks) they can get special instructions for how to do that.

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.

[laurentsimon]

Thanks for the feedback, much appreciated. We'll update this thread once we have this fixed.

[laurentsimon]

@naveensrinivasan @justaugustus any thoughts on the above? This seems to align with our discussion on webhook check that requires more dangerous permissions.

[georgettica]

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

[laurentsimon]

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.

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 automationDetails.id, but there are some checks that are expected to always be run "together", i.e. the SARIF results should list them all.

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

they are working on it /cc @josepalafox @jhutchings1 @davidstaheli

[laurentsimon]

fyi, #185 is merged. We're going to cut a release in the next couple weeks. Progress tracked in #97