doc: call out the PAT having write permissions #1018
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1018 +/- ##
=======================================
Coverage 62.19% 62.19%
=======================================
Files 4 4
Lines 246 246
=======================================
Hits 153 153
Misses 77 77
Partials 16 16 |
README.md
Outdated
| @@ -47,24 +47,24 @@ Running the Scorecard action on a fork repository is not supported. | |||
|
|
|||
| Private repositories need a Personal Access Token (PAT). | |||
|
|
|||
| Public repositories need a PAT to enable the [Branch-Protection](https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection) check. Without a PAT, Scorecards will run all checks except the Branch-Protection check. | |||
| Public repositories need a PAT to enable the [Branch-Protection](https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection) check. Without a PAT, Scorecards will run all checks except the Branch-Protection check. Due to a limitation of the GitHub permission model, the PAT needs [write permisison to the repository](https://docs.github.com/en/developers/apps/building-oauth-apps/scopes-for-oauth-apps#available-scopes) through the `repo` scope. Beware that the PAT will be stored as a [GitHub encrypted secret](https://docs.github.com/en/actions/security-guides/encrypted-secrets) and be accessible by all the workflows and maintainers of a repository. | |||
There was a problem hiding this comment.
Beware that the PAT will be stored as a [GitHub encrypted secret](https://docs.github.com/en/actions/security-guides/encrypted-secrets) and be accessible by all the workflows and maintainers of a repository.
Will the implications of this warning be clear to the reader or can we spell them out? Why should they be concerned about the PAT/secret being accessible to other workflows and maintainers? What could happen if misused?
If it's a strong warning, I'd make it stand out more: Important: the PAT will be stored...
There was a problem hiding this comment.
They should be worried about it because it increases the chance of the secret leaking to someone who is not the owner of the token. Can you suggest a wording? I'll add the Warning: in all occurrences of the text
There was a problem hiding this comment.
I just have one comment on the content:
- the copy in starter-workflows/code-scanning is not updated (the two copies could use merging otherwise too, there seem to be small differences)
This looks like an improvement so I'll "approve" based on that but I have a higher level comment as well, will leave it separately.
|
Following is based on my understanding that for typical projects, a PAT is only required for the Branch-Protection check, other functionality is unaffected. I believe that the risk from putting a personal developer token in GH secrets far, far outweighs the benefit that the Branch-Protection check provides. In my opinion no-one should be adding a PAT to secrets for that and the instructions should not advice to do so. If this means Branch-Protection check is not available, then that's a small price to pay for doing the right thing. I think the most correct call is to not advice users to create PATs in the main README, or at most mention them in a separate section for private repositories (but even in that case I question the value proposition of using PATs -- I would personally not do that with my account even on a private repo). |
Let me remove the starter-workflow. The starter-workflow is this one actions/starter-workflows#1830 |
@olivekl Could you advise on the wording to make it clearer there are risks to using a PAT? And that we don't advise using it? |
|
@naveensrinivasan PTAL |
closes #1003
@jku can you review the changes?