search

ossf / scorecard

OpenSSF Scorecard - Security health metrics for Open Source
https://scorecard.dev
Apache License 2.0
4.63k stars 503 forks source link

Improve scorecard score for scorecard repo #1618

Closed inferno-chromium closed 1 year ago

inferno-chromium commented 2 years ago

Describe the bug A clear and concise description of what the bug is.

Reproduction steps https://deps.dev/go/github.com%2Fossf%2Fscorecard Are these false positives? If not, please fix. Also, for false positives, is there any easy way to add exceptions/ignore-lists [this is important as these workflows issues probably happen on other criticial repos as well]

Expected behavior 10/10 score for scorecard repo.

Additional context Add any other context about the problem here.

naveensrinivasan commented 2 years ago

Here is the latest release https://deps.dev/go/github.com%2Fossf%2Fscorecard%2Fv4

justaugustus commented 2 years ago

Sounds fun! /assign

justaugustus commented 2 years ago

Local run against https://github.com/ossf/scorecard/pull/1629:

./scorecard --repo=https://github.com/ossf/scorecard --show-details
Starting [Maintained]
Starting [Dependency-Update-Tool]
Starting [Fuzzing]
Starting [Contributors]
Starting [CI-Tests]
Starting [Signed-Releases]
Starting [SAST]
Starting [License]
Starting [Token-Permissions]
Starting [Vulnerabilities]
Starting [CII-Best-Practices]
Starting [Code-Review]
Starting [Pinned-Dependencies]
Starting [Binary-Artifacts]
Starting [Branch-Protection]
Starting [Security-Policy]
Starting [Dangerous-Workflow]
Starting [Packaging]
Finished [Maintained]
Finished [Fuzzing]
Finished [Contributors]
Finished [CI-Tests]
Finished [Signed-Releases]
Finished [SAST]
Finished [Dependency-Update-Tool]
Finished [Token-Permissions]
Finished [Vulnerabilities]
Finished [CII-Best-Practices]
Finished [Code-Review]
Finished [Pinned-Dependencies]
Finished [License]
Finished [Branch-Protection]
Finished [Security-Policy]
Finished [Dangerous-Workflow]
Finished [Packaging]
Finished [Binary-Artifacts]

RESULTS

Aggregate score: 8.0 / 10

Check scores:

SCORE NAME REASON DETAILS DOCUMENTATION/REMEDIATION
10 / 10 Binary-Artifacts no binaries found in the repo https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#binary-artifacts
8 / 10 Branch-Protection branch protection is not Info: 'force pushes' disabled https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#branch-protection
maximal on development and all on branch 'main' Info:
release branches 'allow deletion' disabled on
branch 'main' Info: status
check found to merge onto on
branch 'main' Warn: number of
required reviewers is only 1
on branch 'main'
10 / 10 CI-Tests 30 out of 30 merged PRs https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#ci-tests
checked by a CI test -- score
normalized to 10
2 / 10 CII-Best-Practices badge detected: in_progress https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#cii-best-practices
10 / 10 Code-Review all last 30 commits are https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#code-review
reviewed through GitHub
10 / 10 Contributors 24 different companies found Info: contributors work for: https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#contributors
-- score normalized to 10 kubernetes-csi,inclusivenaming,apiclarity,todogroup,chainguard-dev,maintainers,util-linux,kubernetes,kubernetes-client,kubernetes-sigs,dexidp,google,media-streaming-mesh,cisco-open,bit-broker,sigstore,linux
foundation,datto,ossf,multi-factor-auth-users,slsa-framework,Conservatory,systemd,kubeflow
0 / 10 Dangerous-Workflow dangerous workflow patterns Warn: untrusted code checkout '${{ https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#dangerous-workflow
detected github.event.pull_request.head.sha
}}':
.github/workflows/integration.yml:35
10 / 10 Dependency-Update-Tool update tool detected Info: Dependabot detected: https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#dependency-update-tool
.github/dependabot.yml:1
0 / 10 Fuzzing project is not fuzzed https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#fuzzing
10 / 10 License license file detected Info: : LICENSE:1 https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#license
10 / 10 Maintained 30 commit(s) out of 30 and 29 https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#maintained
issue activity out of 30 found
in the last 90 days -- score
normalized to 10
10 / 10 Packaging publishing workflow detected Info: candidate golang publishing workflow: https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#packaging
.github/workflows/goreleaser.yaml:23 Info:
GitHub publishing workflow used in run
https://api.github.com/repos/ossf/scorecard/actions/runs/1699660366:
.github/workflows/goreleaser.yaml:1
9 / 10 Pinned-Dependencies dependency not pinned by hash Warn: GitHub-owned action https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#pinned-dependencies
detected -- score normalized not pinned by hash:
to 9 .github/workflows/scorecard-analysis.yml:42
Warn: GitHub-owned action
not pinned by hash:
.github/workflows/scorecard-analysis.yml:49
Info: Third-party actions are pinned Info:
Dockerfile dependencies are pinned Info:
no insecure (not pinned by hash) dependency
downloads found in Dockerfiles Info: no
insecure (not pinned by hash) dependency
downloads found in shell scripts Info: no
insecure (not pinned by hash) dependency
downloads found in GitHub workflows
10 / 10 SAST SAST tool is run on all Info: all commits (30) are https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#sast
commits checked with a SAST tool Info:
SAST tool detected: CodeQL
10 / 10 Security-Policy security policy file detected Info: security policy https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#security-policy
detected: SECURITY.md:1
10 / 10 Signed-Releases 5 out of 5 artifacts are Info: signed release artifact: scorecard_checksums.txt.sig: https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#signed-releases
signed -- score normalized to https://api.github.com/repos/ossf/scorecard/releases/assets/54007192
10 Info: signed release artifact: scorecard_checksums.txt.sig:
https://api.github.com/repos/ossf/scorecard/releases/assets/53909195
Info: signed release artifact: scorecard_checksums.txt.sig:
https://api.github.com/repos/ossf/scorecard/releases/assets/50228425
Info: signed release artifact: scorecard_checksums.txt.sig:
https://api.github.com/repos/ossf/scorecard/releases/assets/48176202
Info: signed release artifact: scorecard_3.0.1_checksums.txt.sig:
https://api.github.com/repos/ossf/scorecard/releases/assets/46535592
7 / 10 Token-Permissions non read-only tokens detected Info: top level 'contents' https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#token-permissions
in GitHub workflows permission set to 'read':
.github/workflows/codeql-analysis.yml:37
Warn: top level 'statuses'
permission set to 'write':
.github/workflows/codeql-analysis.yml:38
Warn: top level 'security-events'
permission set to 'write':
.github/workflows/codeql-analysis.yml:39
Warn: no top level permission defined:
.github/workflows/goreleaser.yaml:1 Info:
candidate golang publishing workflow:
.github/workflows/goreleaser.yaml:23 Info:
candidate golang publishing workflow:
.github/workflows/goreleaser.yaml:23
Info: top level 'contents'
permission set to 'read':
.github/workflows/integration.yml:20
Warn: no top level permission defined:
.github/workflows/main.yml:1 Info: job
level 'contents' permission set to 'read':
.github/workflows/main.yml:22 Info: job
level 'contents' permission set to 'read':
.github/workflows/main.yml:68 Info: top
level permissions set to 'read-all':
.github/workflows/scorecard-analysis.yml:13
Info: top level permissions set to
'read-all': .github/workflows/stale.yml:20
Info: top level permissions set to
'read-all': .github/workflows/verify.yml:19
Warn: job level 'checks' permission set to
'write': .github/workflows/verify.yml:24
10 / 10 Vulnerabilities no vulnerabilities detected https://github.com/ossf/scorecard/blob/b9a0fdc2778c71a942f073443adba0cf3a3aa11e/docs/checks.md#vulnerabilities
laurentsimon commented 2 years ago

re: dangerous workflow. There's a false positive tracked in https://github.com/ossf/scorecard/issues/1311 that @asraa is working on re: token permissions: we currently remove 0.5-1 points for certain low-risk permissions https://github.com/ossf/scorecard/blob/main/checks/permissions.go#L311-L336 (this is why scorecard has score of 8). This is something I've been thinking of removing but have not had the chance to ask folks' opinion at the meeting yet

laurentsimon commented 2 years ago

why do we need contents:write in goreleaser https://github.com/ossf/scorecard/blob/main/.github/workflows/goreleaser.yaml#L26? Why is this not packages:write Fyi, current packaging check does not distinguish between contents/packages https://github.com/ossf/scorecard/issues/1254

varunsh-coder commented 2 years ago

why do we need contents:write in goreleaser https://github.com/ossf/scorecard/blob/main/.github/workflows/goreleaser.yaml#L26? Why is this not packages:write Fyi, current packaging check does not distinguish between contents/packages #1254

goreleaser writes to releases (example) which needs contents:write. In fact if a publishing workflow did not write a .sig file to releases, the signed release check might fail. May be the signed release check should check for signature info in the package repository (e.g. docker registry).

On another note, I think that regardless of where the artifact is published, it would be good to standardize creating a tag for the release version. So if one is trying to build from source, one can use the tag to get the right commit to build the right version.

Just thinking aloud...

laurentsimon commented 2 years ago

Thanks @varunsh-coder I realized after posting the question that to push a release, we need contents:write. Thanks for confirming!

I think there are APIs to check which tag corresponds to which commit/branch. Do you think this is insufficient?

varunsh-coder commented 2 years ago

Thanks @varunsh-coder I realized after posting the question that to push a release, we need contents:write. Thanks for confirming!

I think there are APIs to check which tag corresponds to which commit/branch. Do you think this is insufficient?

What I was thinking about might be out of scope for scorecards. Sometimes maintainers publish a release to artifact repository, e.g. https://registry.npmjs.org, but forget to create a tag. I noticed https://www.npmjs.com/package/plaid has version 9.9.0 but no tag for that in the repo: https://github.com/plaid/plaid-node/tags. I don't think scorecards can detect this though...may be some related project?

laurentsimon commented 2 years ago

do you think SLSA provenance is something that could address this problem?

varunsh-coder commented 2 years ago

do you think SLSA provenance is something that could address this problem?

I think so. In this case, even if the registry owner could verify if the tag exists in the associated repo, and reject the publish event if it doesn't, it will solve the problem. May be I can submit a request to the https://registry.npmjs.org/ owners...

GuillaumeRoss commented 2 years ago

For the tokens, I submitted https://github.com/ossf/scorecard/pull/1787.

I did notice though that the logic seems to check the yml files, but does not check the default permissions granted to Github actions, so it raises issues when the top level action does not have read on contents explicitly, though if nothing is specified that is what it would get.

This is my first PR to the project so I'll go read the guidelines and be sure my submission is correct - I just happened to be fixing the same warnings in another repo.

spencerschrock commented 1 year ago

Closing as we're up to a 9.7 currently.