Feature: allow job-level write-permissions only when job steps are properly hash-pinned

[diogoteles08]

Is your feature request related to a problem? Please describe

Context

The issue #2338 discussed the problem of getting a straight 0/10 on the Token-Permissions check if you have a single job with permissions contents: write or packages: write. It ended up with the implementation of the PR #2355, and now if a job explicitly defines a job-level write permission, it will get a Warning on the logs but won't have a decrease on Token-Permission score.

Problem

Although I agree with the motivations of the discussed problem, I think that having a job-level contents: write or packages: write still brings relevant risks, which should be reflected on the Token-Permission score. I understand that having a job-level write permissions probably means that the maintainer knows the action and explicitly decided to trust in it -- and I also agree this decision should be considered, so I'm not saying we should get to 0/10 --, but the risks still exists and should be considered for whoever wants to evaluate the security of the repository.

E.g., if you use dangerous permissions when calling a GitHub Action without hash-pinning, your repository would be vulnerable on the scenario of a malicious or buggy release -- which could be actually a new released or a change of an existing tag, so even actions pinned as @X.Y.Z aren't immune to attacks.

Describe the solution you'd like

I suggest to keep rewarding repositories that declare write permissions as job-level, but only grant the maximum score if the write permission is declared job-level and all job steps are properly hash-pinned. In this way, we would also ensure the dangerous permissions could not be abused on a malicious release.

Considering the case of write permissions on workflows, I purpose that the score for the Token-Permission check could be decreased according to the following table:	Dangerous permission pattern	Score decreased by occurrence	Motivation
Workflow without any permissions definition or with top-level write permissions	10	Permissions are imposed according to the repository's default permission. If the repository doesn't manually change it, the default set by GitHub will be write-all permissions.
Write permissions for contents, packages or actions given job-level, but without hash-pinning	3*	Permissions may be abused by malicious releases.
Write permissions for contents, packages or actions given job-level, with proper hash-pinning	0

* The value 3 hasn't been discussed and should be reviewed.

[evverx]

Write permissions for contents, packages or actions given job-level, but without hash-pinning

Dunno. Hashes just mitigate some issues but if those hashes point to forks thanks to imposter commits they can actually make things worse. If actions rely on, say, a bunch of unpinned JS modules glued together with duct tape and popsicle sticks repositories using them can still be pwned regardless of whether those actions are pinned or not.

I think that scorecard should decide what it flags. If it should flag potentially dangerous stuff it should flag all the actions with the write permissions (regardless of whether they are pinned or not). It could help to figure out what should be reviewed/audited/scanned and that's https://github.com/ossf/scorecard/issues/2991.

If the idea is to show whether projects are aware of permissions in general and follow the best practices I think https://github.com/ossf/scorecard/issues/2338 should suffice.

Then again it depends on what scorecard is trying to accomplish.

[spencerschrock]

Dunno. Hashes just mitigate some issues but if those hashes point to forks thanks to imposter commits they can actually make things worse.

I view this as a GitHub implementation problem, not as a pinning problem. But yes, the result is it can negatively impact projects. When I'm reviewing PRs, I check that the SHA belongs to the intended repo, but it's a burden that not every maintainer will do.

I think that scorecard should decide what it flags. If the idea is to show whether projects are aware of permissions in general and follow the best practices I think #2338 should suffice.

My view is the check highlights the default github token permissions, and tries to enforce least privilege. At some point the check added logic for "some permissions are more dangerous than others", but we don't have the resources to maintain an allowlist/audit.

I'm curious about @laurentsimon's opinion.

[evverx]

I view this as a GitHub implementation problem, not as a pinning problem

Agreed. Though actions should be pinned in the first place because there is no way to ship actions where consumers could refer to them using meaningful labels that would point to immutable things. Hashes just get around that GitHub shortcoming.

I'm reviewing PRs, I check that the SHA belongs to the intended repo, but it's a burden that not every maintainer will do

Agreed. That's why I think before asking maintainers to pin their actions that part should be automated too (and as far as I understand https://github.com/ossf/scorecard-webapp/pull/389 should kind of address it but Ideally imposter commits should be caught when PRs are opened. Malicious hashes if they are actually malicious should never end up in repositories especially if they use the scorecard action).

[github-actions[bot]]

Stale issue message

[github-actions[bot]]

This issue is stale because it has been open for 60 days with no activity.