Add the OpenSSF Scorecard Github Action #12564
Comments
wwuck
added
S: needs triage
|
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. |
|
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 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 | |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------------------------------------------| |
|
Looking at the "0/10" scores:
|
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. |
|
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. |
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. |
|
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. |
|
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'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: 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 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. |
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. |
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. |
|
@pfmoore Thanks for taking the time to read through all of this discussion.
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 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.
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.
In this case I will create a new issue related to CodeQL/SAST for further discussion on that. |
I created PR #12572 to balance and minimize the work of the maintainers. |
We're meaningfully protected from a decent portion of this via a configuration setting: 
|
|
@pradyunsg Thanks for confirming. That’s good to know. |
ichard26
added
C: automation
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
The text was updated successfully, but these errors were encountered: