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

Improve scorecard score for scorecard repo #1618

Closed
5 tasks
inferno-chromium opened this issue Feb 9, 2022 · 12 comments
Closed
5 tasks

Improve scorecard score for scorecard repo #1618

inferno-chromium opened this issue Feb 9, 2022 · 12 comments
Assignees
Labels
kind/bug Something isn't working priority/must-do Upcoming release

Comments

@inferno-chromium
Copy link
Contributor

inferno-chromium commented Feb 9, 2022

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.


@inferno-chromium inferno-chromium added the kind/bug Something isn't working label Feb 9, 2022
@naveensrinivasan
Copy link
Member

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

@naveensrinivasan naveensrinivasan added the help wanted Community contributions welcome, maintainers supportive of idea but not a high priority label Feb 9, 2022
@justaugustus
Copy link
Member

Sounds fun!
/assign

@justaugustus
Copy link
Member

Local run against #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

@justaugustus justaugustus added priority/must-do Upcoming release and removed help wanted Community contributions welcome, maintainers supportive of idea but not a high priority labels Feb 12, 2022
@laurentsimon
Copy link
Contributor

laurentsimon commented Feb 14, 2022

re: dangerous workflow. There's a false positive tracked in #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
Copy link
Contributor

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

@varunsh-coder
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

@varunsh-coder
Copy link
Contributor

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
Copy link

GuillaumeRoss commented Mar 28, 2022

For the tokens, I submitted #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
Copy link
Member

Closing as we're up to a 9.7 currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working priority/must-do Upcoming release
Projects
Status: Done
Development

No branches or pull requests

7 participants