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

Are read-all permissions necessary? #1461

Open
funnelfiasco opened this issue Nov 7, 2024 · 5 comments
Open

Are read-all permissions necessary? #1461

funnelfiasco opened this issue Nov 7, 2024 · 5 comments

Comments

@funnelfiasco
Copy link

I'm using zizmor to audit GUAC's GitHub workflows and the scorecard workflow reports excessive permissions:

🌈 completed scorecard.yml
warning[excessive-permissions]: overly broad workflow or job-level permissions
  --> .github/workflows/scorecard.yml:18:1
   |
18 | permissions: read-all
   | --------------------- uses read-all permissions
   |
   = note: audit confidence → High

1 findings (0 ignored): 0 unknown, 0 informational, 0 low, 1 medium, 0 high

I don't see anything in this action's docs that explain why read-all is necessary. Does the action require read access to all possible permissions or is that a convenience instead of enumerating the specific permissions required?

If read-all is necessary, I'd be happy to submit a PR to add a mention in the docs.

@spencerschrock
Copy link
Member

It is likely a matter of convenience, with a dash of future proofing further updates. The answer is likely different for public vs. private repos as well, and it's something we haven't done a good job of documenting. GitHub has a handy auditing tool for determining least privilege for their REST API, but Scorecard uses the graphQL API which isn't supported by the monitor.

At the very least, this past issue implies contents and actions are useful. Skimming the permission list, I could see a few more being needed depending on the check:

  • checks
  • issues
  • pull-requests
  • statuses
    being required in some scenarios.

But certain feature requests means new permissions may be used in the future, such as attestations.

@funnelfiasco
Copy link
Author

Thanks for the info. I'll see if the GUAC maintainers are interested in experimenting with me to see if we can identify a minimum level of permissions. If so, I'll contribute that knowledge upstream.

@mattip
Copy link

mattip commented Dec 8, 2024

It would be nice to either work with zizmor to tag this warning as inconsequential or fix this in the workflow.

@mattip
Copy link

mattip commented Dec 8, 2024

Strangely, I set the permissions to empty {} and the analysis ran https://github.com/numpy/numpy/actions/workflows/scorecards.yml. I am not sure what that means...

@funnelfiasco
Copy link
Author

We've been running it on GUAC with a workflow-level contents: read and security-events: write & id-token: write on the analysis job without issue.

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

No branches or pull requests

3 participants