Add the OpenSSF Scorecard Github Action

[wwuck]

What's the problem this feature will solve?

https://github.com/ossf/scorecard is a useful tool for analysing the project's security best-practices. It would be nice to see the pip project add the github action to enable this.

I saw an existing MR using some of this tool's output at #11226, so adding the github action would enable better visibility on any future issues.

Describe the solution you'd like

I can submit a PR for this if you think it would be a good addition to the pip CI workflow. I would probably copy an existing PR like docker/compose#9846 to ensure best practice. The associated issue docker/compose#9845 also has some screenshots of the output.

Alternative Solutions

N/A

Additional context

N/A

Code of Conduct

• I agree to follow the PSF Code of Conduct.

[pfmoore]

I’m concerned that this would set an expectation that we would fix any issues flagged by the scan, and I don’t think we currently have the maintainer resources to make such a commitment. To an extent this depends on what the scan flags - if it tells us that everything is currently in order, then that’s a different matter. But typically these sorts of metrics involve an initial (often non-trivial) exercise to “put your house in order”, and IMO we shouldn’t make a blind commitment to prioritising such work over our existing backlog of maintenance work.

Maybe individual pull requests submitted by individuals with an interest in the specific change, like #11226, are a better approach than expecting auto-generated output from a job to motivate someone to do the work?

Given that this sort of metric and dashboard is often of primary interest to corporate users, though, maybe there is scope for an interested sponsor to fund work to add this? Either by providing developer resource directly, or by providing funds to hire someone. See https://github.com/psf/fundable-packaging-improvements for more details.

[wwuck]

It's designed to improve the supply-chain security of essential open-source projects, and I think pip definitely qualifies under that banner. Taking a look at the various checks that are run, you can see it is more on the github project configuration side of things and not really a code quality scanner.

I looked at the project website https://securityscorecards.dev/ and followed the CLI instructions to run a manual scan on pypa/pip repository and it looks like most of the checks are returning high score so there shouldn't be much needed to fix up. The benefit of running as a workflow action is that you more visibility on when any project configuration change/redesign by github might need a tweak to the settings for optimal project security.

You can run it with --show-details to see the full details of each check result.

docker run -e GITHUB_AUTH_TOKEN=<public_repo_token> gcr.io/openssf/scorecard:stable --repo=github.com/pypa/pip
Unable to find image 'gcr.io/openssf/scorecard:stable' locally
stable: Pulling from openssf/scorecard
a7ca0d9ba68f: Pull complete 
fe5ca62666f0: Pull complete 
b02a7525f878: Pull complete 
fcb6f6d2c998: Pull complete 
e8c73c638ae9: Pull complete 
1e3d9b7d1452: Pull complete 
4aa0ea1413d3: Pull complete 
7c881f9ab25e: Pull complete 
5627a970d25e: Pull complete 
19cf2287de7f: Pull complete 
2758d0c31c8c: Pull complete 
08553ba93cfe: Pull complete 
f04e6a5f0ce3: Pull complete 
Digest: sha256:5daab46bcce9ffdcbf829d9bada623e810a9ffd974e5deba5c8534a69ddc9c33
Status: Downloaded newer image for gcr.io/openssf/scorecard:stable
Starting [Token-Permissions]
Starting [CI-Tests]
Starting [Packaging]
Starting [Maintained]
Starting [Dangerous-Workflow]
Starting [Fuzzing]
Starting [Security-Policy]
Starting [Pinned-Dependencies]
Starting [SAST]
Starting [License]
Starting [Binary-Artifacts]
Starting [Vulnerabilities]
Starting [Dependency-Update-Tool]
Starting [CII-Best-Practices]
Starting [Code-Review]
Starting [Contributors]
Starting [Branch-Protection]
Starting [Signed-Releases]
Aggregate score: 6.5 / 10

Check scores:
Finished [Dependency-Update-Tool]
Finished [CII-Best-Practices]
Finished [Code-Review]
Finished [Contributors]
Finished [Branch-Protection]
Finished [Signed-Releases]
Finished [Token-Permissions]
Finished [CI-Tests]
Finished [Packaging]
Finished [Maintained]
Finished [Dangerous-Workflow]
Finished [Fuzzing]
Finished [Security-Policy]
Finished [Pinned-Dependencies]
Finished [SAST]
Finished [License]
Finished [Binary-Artifacts]
Finished [Vulnerabilities]

RESULTS
-------
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
|  SCORE  |          NAME          |             REASON             |                                               DOCUMENTATION/REMEDIATION                                               |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | Binary-Artifacts       | binaries present in source     | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#binary-artifacts       |
|         |                        | code                           |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 3 / 10  | Branch-Protection      | branch protection is not       | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#branch-protection      |
|         |                        | maximal on development and all |                                                                                                                       |
|         |                        | release branches               |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | CI-Tests               | 6 out of 6 merged PRs          | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#ci-tests               |
|         |                        | checked by a CI test -- score  |                                                                                                                       |
|         |                        | normalized to 10               |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 2 / 10  | CII-Best-Practices     | badge detected: InProgress     | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#cii-best-practices     |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Code-Review            | all changesets reviewed        | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#code-review            |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Contributors           | project has 142 contributing   | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#contributors           |
|         |                        | companies or organizations     |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Dangerous-Workflow     | no dangerous workflow patterns | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#dangerous-workflow     |
|         |                        | detected                       |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Dependency-Update-Tool | update tool detected           | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#dependency-update-tool |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Fuzzing                | project is fuzzed              | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#fuzzing                |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | License                | license file detected          | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#license                |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Maintained             | 30 commit(s) and 18 issue      | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#maintained             |
|         |                        | activity found in the last 90  |                                                                                                                       |
|         |                        | days -- score normalized to 10 |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| ?       | Packaging              | packaging workflow not         | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#packaging              |
|         |                        | detected                       |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | Pinned-Dependencies    | dependency not pinned by hash  | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#pinned-dependencies    |
|         |                        | detected -- score normalized   |                                                                                                                       |
|         |                        | to 0                           |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | SAST                   | SAST tool is not run on all    | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#sast                   |
|         |                        | commits -- score normalized to |                                                                                                                       |
|         |                        | 0                              |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 9 / 10  | Security-Policy        | security policy file detected  | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#security-policy        |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| ?       | Signed-Releases        | no releases found              | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#signed-releases        |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 0 / 10  | Token-Permissions      | detected GitHub workflow       | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#token-permissions      |
|         |                        | tokens with excessive          |                                                                                                                       |
|         |                        | permissions                    |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Vulnerabilities        | 0 existing vulnerabilities     | https://github.com/ossf/scorecard/blob/f1e703f5006c2cd8d27c86368f0aed0fd286a976/docs/checks.md#vulnerabilities        |
|         |                        | detected                       |                                                                                                                       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------|

[notatallshaw]

Looking at the "0/10" scores:

• Binary-Artifacts - These are mostly test packages which are fine, one is an image for the docs which is presumably fine, and some vendored executables, specifically from distlib, e.g. src/pip/_vendor/distlib/t64.exe, I don't know the history of these, but they are presumably required for entrypoint creation
• Pinned-Dependencies "dependency not pinned by hash detected" - Pip doesn't have any external dependencies so this check seems broken, pip vendors it's third party code so no hashes are required
• SAST "SAST tool is not run on all commits" - This check is tautological
• Token-Permissions "detected GitHub workflow tokens with excessive permissions" - I guess there was a PR and a discussion here and it wasn't clearly answered why the fix was worth it: chore: Set permissions for GitHub actions #11226

[wwuck]

@notatallshaw

• Pinned-Dependencies: I think the dependency pinning is looking at the github workflow files. I can't see any of those with hashes. See https://github.com/docker/compose/pull/9846/files for an example of this. The version number in a comment will also get updated by dependabot when the action is pinned by hash. For actions that are pinned-by-hash, bump the human readable version number in the code comment dependabot/dependabot-core#4691

• SAST: This is checking for any use of static analysis tools such as CodeQL or SonarCloud in the github workflows. I can't see any in there. https://github.com/ossf/scorecard/blob/main/docs/checks.md#sast

• Token-Permissions: I'm not an expert on this area so I'm hoping that someone from GOSST may be better placed to answer why the changes are necessary. @joycebrum @diogoteles08?

Looking at https://github.com/ossf/wg-securing-critical-projects and https://docs.google.com/spreadsheets/d/1ONZ4qeMq8xmeCHX03lIgIYE4MEXVfVL6oj05lbuXTDM/edit#gid=577559548, I can see that pip is already identified by OSSF as a critical project so they might already have an idea on what should be fixed for permissions.

[wwuck]

There is also an online scorecard viewable at https://securityscorecards.dev/viewer/?uri=github.com/pypa/pip which shows the full scan output.

[pfmoore]

There is also an online scorecard viewable at https://securityscorecards.dev/viewer/?uri=github.com/pypa/pip which shows the full scan output.

So if that’s already available to anyone interested, there doesn’t seem to be any point in us duplicating that work.

[wwuck]

So if that’s already available to anyone interested, there doesn’t seem to be any point in us duplicating that work.

The problem with that is that people will just ignore it and never check it. Having it integrated into the CI pipeline will more likely ensure that accidental changes in workflow yaml are picked up during code reviews.

[wwuck]

I used the tool at https://app.stepsecurity.io/secureworkflow/ linked from the scorecards docs to generate a sample PR for some of the fixes. wwuck#1

If any of that looks useful, I can submit one or more PRs to the pip repository? I'm aware that not all of those changes may be a good fit for the pip project workflows.

[notatallshaw]

I'm not a pip maintainer, but if I was in your position and thought it worthwhile, I think I would have better luck raising a PR to fix the issues you think are worth fixing, and PR to rimplement the OpenSSF Scorecard seperaretly. Then the actions and the scorecard can be reviewed on their merits separately.

[pfmoore]

Pinned-Dependencies: I think the dependency pinning is looking at the github workflow files. I can't see any of those with hashes. See https://github.com/docker/compose/pull/9846/files for an example of this. The version number in a comment will also get updated by dependabot when the action is pinned by hash.

I'm actually a fairly strong -1 on this. Reviewing version numbers is easy, but reviewing hashes is frankly impractical. If the delivery mechanism for github actions doesn't mean that using version X.Y of an action is secure, then I'm inclined to say that's an issue with the delivery mechanism, not with pip's use of actions.

Taking an example from the linked PR:

 ossf/scorecard-action@937ffa90d79c7d720498178154ad4c7ba1e4ad8c # tag=v2.0.6

Can you tell what's wrong with this? Would you have spotted it if I hadn't mentioned there was a problem? Spoiler: I changed the hash so it doesn't match the tag quoted in the comment. And because the tag is a comment, nothing will flag the discrepancy (except maybe some external checker like this one, but that feels like adding infrastructure to check for problems caused by the infrastructure itself...) So in my view, using hashes is less auditable (and hence secure) than using version numbers.

So I think another question that needs to be addressed here is whether it's possible to select which checks we want to opt into? If not, then I'm even less enthusiastic about adding this.

I'm not a pip maintainer, but if I was in your position and thought it worthwhile, I think I would have better luck raising a PR to fix the issues you think are worth fixing, and PR to rimplement the OpenSSF Scorecard seperaretly. Then the actions and the scorecard can be reviewed on their merits separately.

I am a pip maintainer, and I agree with this. You are still quite likely to get pushback on the idea, but it'll be more specific and therefore more useful. You'll also have to present arguments for any of the changes you're proposing, which is a good thing - we're not likely to accept any change simply because some metric (no matter how respected it is) says we should.

[pfmoore]

The problem with that is that people will just ignore it and never check it. Having it integrated into the CI pipeline will more likely ensure that accidental changes in workflow yaml are picked up during code reviews.

So you're saying that we should reject contributions if they don't pass these checks? I disagree - we have enough trouble getting contributors already without adding extra barriers.

IMO, if someone isn't willing to review the 3rd party report, they have no right to demand that the pip maintainers do that for them. And if they do review the report, then they can submit PRs to fix the issue. I repeat, I don't think this is something we should be spending volunteer maintainer time on - the people who need this should be willing to do the work.

[diogoteles08]

• Token-Permissions: I'm not an expert on this area so I'm hoping that someone from GOSST may be better placed to answer why the changes are necessary. @joycebrum @diogoteles08?

Hi! Thanks for the contact ^^ Sure I can help

The Token-Permissions check is important to make sure you have minimal permissions defined on your workflows. It secures you against erroneous or malicious behaviours from the external jobs you call -- it's specially important in case they get compromised, for example.

Generally we suggest that you explicitly set the workflow and/or job permissions on your workflow files (similarly to suggested on https://github.com/pypa/pip/pull/11226/files), because it makes the permissions clear to anyone looking at the jobs. But another good way to secure them is to make sure to have your repository default permissions set to read-only (read more in GitHub docs). In this way, every time you want to give a write permission to a external job you'd need to explicitly define it on the workflow.

[wwuck]

@pfmoore Thanks for taking the time to read through all of this discussion.

Pinned-Dependencies: I think the dependency pinning is looking at the github workflow files. I can't see any of those with hashes. See https://github.com/docker/compose/pull/9846/files for an example of this. The version number in a comment will also get updated by dependabot when the action is pinned by hash.

I'm actually a fairly strong -1 on this. Reviewing version numbers is easy, but reviewing hashes is frankly impractical. If the delivery mechanism for github actions doesn't mean that using version X.Y of an action is secure, then I'm inclined to say that's an issue with the delivery mechanism, not with pip's use of actions.

Taking an example from the linked PR:

 ossf/scorecard-action@937ffa90d79c7d720498178154ad4c7ba1e4ad8c # tag=v2.0.6

Can you tell what's wrong with this? Would you have spotted it if I hadn't mentioned there was a problem? Spoiler: I changed the hash so it doesn't match the tag quoted in the comment. And because the tag is a comment, nothing will flag the discrepancy (except maybe some external checker like this one, but that feels like adding infrastructure to check for problems caused by the infrastructure itself...) So in my view, using hashes is less auditable (and hence secure) than using version numbers.

The problem with pip's current state of using tag versioning on third-party action repositories is that there is no indication of what version you are actually using.

Eg. https://github.com/pypa/pip/blob/main/.github/workflows/ci.yml#L262

In this case the workflow referencing v1 is going to use the latest v1.2.2 but you receive no warning/notification through dependabot if a new version v1.2.3 is released. The same does apply to github/ and actions/ actions (eg. actions/checkout@v4 in the workflow using v4.1.1) but I think one could argue the case there that they are potentially more trusted. At least Apache seems to think so. In any case, pinning to the full version tag or hash and checking all updates from dependabot via CI workflows would prevent issues like updating to a broken version such as https://github.com/actions/checkout/releases/tag/v4.1.2.

I guess the issue here is the balance of convenience of receiving updates without too much maintenance burden vs having the stability of knowing when a change in actions is potentially affecting the workflow. I can't really answer that myself so if the pip maintainers decide to go with less maintenance burden then I'm not going to raise any arguments against it.

For the hash pinning, in my case perhaps for a PR coming from dependabot I would be more likely to "trust" it to be accurate, but for a PR from any person updating hash pins I would definitely be going to the upstream action repository to verify the commit hash is matching the tag. If I'm using an automated dependency update service then any manual dependency updates are likely to be a red flag and raise questions of why it's appearing in an unrelated PR. This is a downside of actions being git repositories that they don't have the ability to provide a hash of a single release artifact to match to a version, like we can with python wheels/node packages/etc.

GitHub itself does recommend pinning hashes for third-party actions.

github/roadmap#592 would be an added benefit here but it still wouldn't solve the case of a compromised repository pushing a new minor version tag update that goes unnoticed.

ossf/scorecard#2018 is another interesting point of view on this, and I think ensuring proper workflow permissions would go a long way to aid in preventing a compromised action from affecting pip's CI workflows. As there is an existing PR for this I don't see any need to create a new one.

So I think another question that needs to be addressed here is whether it's possible to select which checks we want to opt into? If not, then I'm even less enthusiastic about adding this.

Not with the action, but with the CLI tool it is possible. In any case, it seems like that would be more effort than benefit gained so I won't take that any further on adding the scorecards action at this point. Perhaps it could be revisited when the action gains support for running a subset of checks.

I'm not a pip maintainer, but if I was in your position and thought it worthwhile, I think I would have better luck raising a PR to fix the issues you think are worth fixing, and PR to rimplement the OpenSSF Scorecard seperaretly. Then the actions and the scorecard can be reviewed on their merits separately.

I am a pip maintainer, and I agree with this. You are still quite likely to get pushback on the idea, but it'll be more specific and therefore more useful. You'll also have to present arguments for any of the changes you're proposing, which is a good thing - we're not likely to accept any change simply because some metric (no matter how respected it is) says we should.

In this case I will create a new issue related to CodeQL/SAST for further discussion on that.