Add aggregated score (#1046)

* ag scores

* fix

* CSV and string

* comments

* updates

* changes

* fixes
This commit is contained in:
laurentsimon 2021-09-21 15:30:25 -07:00 committed by GitHub
parent fd6e58d615
commit 39bd00c359
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 201 additions and 100 deletions

View File

@ -155,6 +155,7 @@ or ./scorecard --{npm,pypi,rubgems}=<package_name> [--checks=check1,...] [--show
if e != nil {
log.Fatalf("cannot read yaml file: %v", err)
}
switch format {
case formatDefault:
err = repoResult.AsString(showDetails, *logLevel, checkDocs, os.Stdout)

View File

@ -23,7 +23,7 @@ A low score is therefore considered `High` risk.
## Branch-Protection
[Branch protection](https://docs.github.com/en/github/administering-a-repository/defining-the-mergeability-of-pull-requests/about-protected-branches) can enable various rules to enforce certain workflows for branches, such as preventing rewriting of public history (e.g., a *force push*), requiring review before acceptance into a main branch, or passing certain status checks before acceptance into a main branch..
[Branch protection](https://docs.github.com/en/github/administering-a-repository/defining-the-mergeability-of-pull-requests/about-protected-branches) can enable various rules to enforce certain workflows for branches, such as preventing rewriting of public history (e.g., a *force push*), requiring review before acceptance into a main branch, or passing certain status checks before acceptance into a main branch.
Branch protection can reduce the risk of unintentional or malicious code from entering the "main" branch. Branch protection rules that prevent rewriting of public history (e.g., preventing *force push* of public branches) prevent history from changing without external notice. Branch protection rules that require status checks ensure that at least those checks are met before a change is accepted. Branch protection rules that require at least one other reviewer reviews greatly reduces the risk that a compromise of a contributor's account will lead to injection of malicious code. Review also increases the likelihood that an unintentional vulnerability in a contribution will be detected and fixed before the change is accepted. A low score is therefore considered `High` risk.
Note, however, that requiring reviews for every proposed change is impractical for many projects, since many projects simply don't have that many active participants. For more discussion, see [Code Reviews](#code-reviews).
In some cases these rules will need to be suspended. For example, if a past commit includes illegal content (such as child pornography), it may be impractical to hide the commit and in such cases the history may need to be rewritten.
@ -37,7 +37,7 @@ This check determines if the default and release branches are protected with Git
This check tries to determine if the project runs tests before pull requests are merged.
Running tests helps developers catch mistakes early on. A low score is considered 'Low' risk.
The check works by looking for a set of well-known CI-system names in GitHub `CheckRuns` and `Statuses` among the recent commits (~30). A CI-system is considered well-known if its name contains any of the following: appveyor, buildkite, circleci, e2e, github-actions, jenkins, mergeable, test, travis-ci. The check succeeds if at least 75% of successful pull requests have at least one successful check associated with them. A project may meet this criterion yet have a failing scorecard report; there are many ways to implement this criterion and it's especially difficult for an automated tool (like scorecard) to detect them all.
The check works by looking for a set of CI-system names in GitHub `CheckRuns` and `Statuses` among the recent commits (~30). A CI-system is considered well-known if its name contains any of the following: appveyor, buildkite, circleci, e2e, github-actions, jenkins, mergeable, test, travis-ci. A project may meet this criterion yet have a failing scorecard report; there are many ways to implement this criterion, and it's challenging for an automated tool (like scorecard) to detect them all. If a project's system was not detected and you think it should be, please [open an issue in the scorecard project](https://github.com/ossf/scorecard/issues/new/choose).
**Remediation steps**
- Check-in scripts that run all the tests in your repository.
@ -63,7 +63,7 @@ In short, requiring others' review before accepting a change reduces risks to us
The check first tries to detect if Branch-Protection is enabled on the default branch ,and the number of reviewers is at least 1. If this fails, it checks if the recent (~30) commits have a Github-approved review or if the merger is different from the committer (implicit review). It also performs similar check for reviews using [Prow](https://github.com/kubernetes/test-infra/tree/master/prow#readme) (labels "lgtm" or "approved") and Gerrit ("Reviewed-on" and "Reviewed-by").
**Remediation steps**
- If the project has only contributor, or does not have enough reviewers to practically require that all contributions be reviewed, try to recruit more maintainers to the project who will be willing to review others' work. Ideally at least some of these people will be from different organizations. If the project has very limited utility, consider expanding its intended utility so more people will be interested in improving it, and make that larger scope clear to potential contributors.
- If the project has only one contributor, or does not have enough reviewers to practically require that all contributions be reviewed, try to recruit more maintainers to the project who will be willing to review others' work. Ideally at least some of these people will be from different organizations. If the project has very limited utility, consider expanding its intended utility so more people will be interested in improving it, and make that larger scope clear to potential contributors.
- Follow security best practices by performing strict code reviews for every new pull request / merge request.
- Make "code reviews" mandatory in your repository configuration. E.g. [GitHub](https://docs.github.com/en/github/administering-a-repository/about-protected-branches#require-pull-request-reviews-before-merging).
- Enforce the rule for administrators / code owners as well. E.g. [GitHub](https://docs.github.com/en/github/administering-a-repository/about-protected-branches#include-administrators)
@ -73,7 +73,7 @@ The check first tries to detect if Branch-Protection is enabled on the default b
This check tries to determine if the project has a set of contributors from multiple organizations (e.g., companies).
Low score has 'Low' risk.
Some projects cannot meet this requirement. Obviously projects with only one active participant cannot meet it. Many projects have a narrow scope and may not be able to attract the interest of multiple organizations. See [Code Reviews](#code-reviews) for added notes about projects with a small number of participants.
The check works by looking at the authors of recent commits and checking the `Company` field on the GitHub user profile. A contributor must have at least 5 commits in the last 30 commits. The check succeeds if all contributors span at least 2 different organizations (companies).
The check works by looking at the authors of recent commits and checking the `Company` field on the GitHub user profile. A contributor must have at least 5 commits in the last 30 commits. The highest score is achieved when there are contributors from at least 3 different companies in the last 30 commits.
**Remediation steps**
- There is *NO* remediation work needed here. This is to provide some insights on which organization(s) have contributed to the project and making trust decisions based on that. But you can ask your contributors to join their respective organizations.
@ -82,7 +82,7 @@ The check works by looking at the authors of recent commits and checking the `Co
This check tries to determine if the project uses a dependency update tool.
Not updating dependencies makes a project vulnerable to known flaws and prone to attacks. A low score is therefore considered `High` risk.
The checks looks for [dependabot](https://dependabot.com/docs/config-file/) or [renovatebot](https://docs.renovatebot.com/configuration-options/). This check only looks if it is enabled and does not ensure that it is run and pull requests are merged. A project may meet this criterion yet have a failing scorecard report; there are many ways to implement this criterion and it's especially difficult for an automated tool (like scorecard) to detect them all.
The checks looks for [dependabot](https://dependabot.com/docs/config-file/) or [renovatebot](https://docs.renovatebot.com/configuration-options/). This check only looks if it is enabled and does not ensure that it is run and pull requests are merged. A project may meet this criterion yet have a failing scorecard report; there are many ways to implement this criterion, and it's challenging for an automated tool (like scorecard) to detect them all.
**Remediation steps**
- Signup for automatic dependency updates with dependabot or renovatebot and place the config file in the locations that are recommended by these tools.
@ -91,7 +91,7 @@ The checks looks for [dependabot](https://dependabot.com/docs/config-file/) or [
This check tries to determine if the project uses fuzzing.
Fuzzing is important to reduce the number of vulnerabilities in code. A low score is considered 'Medium' risk.
The check currently works by checking if the repo name is in the [OSS-Fuzz](https://github.com/google/oss-fuzz) project list. A project may meet this criterion yet have a failing scorecard report; there are many ways to implement this criterion and it's especially difficult for an automated tool (like scorecard) to detect them all.
The check currently works by checking if the repo name is in the [OSS-Fuzz](https://github.com/google/oss-fuzz) project list. A project may meet this criterion yet have a failing scorecard report; there are many ways to implement this criterion, and it's challenging for an automated tool (like scorecard) to detect them all.
**Remediation steps**
- Integrate the project with OSS-Fuzz by following the instructions [here](https://google.github.io/oss-fuzz/).
@ -108,9 +108,11 @@ The check currently works by looking whether the repo is archived or not. If it
## Packaging
This check tries to determine if the project is published as a package that other developers can install/download.
Packaging your project is essential for users to receive updates and security patches automatically. A low score is considered `Medium` risk.
The check currently looks for [GitHub packaging workflows]( https://docs.github.com/en/packages/learn-github-packages/publishing-a-package) and language-specific GitHub Actions that upload the package to a corresponding hub, e.g., [Npm](https://www.npmjs.com/). There is a plan to add better support to query package manager hubs directly in the future, e.g., for [Npm](https://www.npmjs.com/), [PyPi](https://pypi.org/).
This check tries to determine if the project is published as a package that others can easily download, install, easily update, and uninstall.
It's important that the project provide an easy way to download, install, update, and uninstall the software. It's particularly important to make it easy for users to receive security patches as updates.
This is often done by creating a "package" that is easy to install and uninstall by a package manager. Many program language ecosystems have a generally-used packaging format supported by a language-level package manager tool and public package repository. Many operating system platforms also have at least one package format, tool, and public repository (in some cases the source repository generates system-independent source packages, which are then used by others to generate system executable packages). Container images are yet another way to package software. In some situations packaging is not sensible, but it's wise to package software in so many circumstances that it's worth checking for it.
A low score is considered `Medium` risk.
The check currently looks for [GitHub packaging workflows]( https://docs.github.com/en/packages/learn-github-packages/publishing-a-package) and language-specific GitHub Actions that upload the package to a corresponding hub, e.g., [Npm](https://www.npmjs.com/). There is a plan to add better support to query package manager hubs directly in the future, e.g., for [Npm](https://www.npmjs.com/), [PyPi](https://pypi.org/). A project may meet this criterion yet have a failing scorecard report; some widely-used tools are relatively easy to detect, but it's challenging for an automated tool (like scorecard) to detect them all. If scorecard fails to detect the way you publish a package and you think scorecard should support your use case, please [open an issue in the scorecard project](https://github.com/ossf/scorecard/issues/new/choose).
**Remediation steps**
- Publish your project as a [downloadable package](https://docs.github.com/en/packages/learn-github-packages/publishing-a-package).

View File

@ -503,6 +503,7 @@ checks:
Run CodeQL checks in your CI/CD by following the instructions
[here](https://github.com/github/codeql-action#usage).
Security-Policy:
risk: Medium
short: Determines if the project has published a security policy.
tags: supply-chain, security, policy
description: >-

View File

@ -30,7 +30,7 @@ var checksYAML []byte
// Check stores a check's information.
// nolint:govet
type Check struct {
Risk string `yaml:"-"`
Risk string `yaml:"risk"`
Short string `yaml:"short"`
Description string `yaml:"description"`
Tags string `yaml:"tags"`

View File

@ -21,6 +21,8 @@ import (
docs "github.com/ossf/scorecard/v2/docs/checks"
)
var allowedRisks = map[string]bool{"Critical": true, "High": true, "Medium": true, "Low": true}
func main() {
m, err := docs.Read()
if err != nil {
@ -50,6 +52,11 @@ func main() {
// nolint: goerr113
panic(fmt.Errorf("tags for checkName: %s is empty", check))
}
r := c.GetRisk()
if _, exists := allowedRisks[r]; !exists {
// nolint: goerr113
panic(fmt.Errorf("risk for checkName: %s is invalid: '%s'", check, r))
}
}
for _, check := range m.GetChecks() {
if _, exists := allChecks[check.GetName()]; !exists {

View File

@ -65,10 +65,21 @@ type jsonScorecardV2 struct {
Commit string `json:"commit"`
}
type jsonFloatScore float64
func (s jsonFloatScore) MarshalJSON() ([]byte, error) {
if float64(s) == float64(int(s)) {
return []byte(fmt.Sprintf("%.0f", s)), nil
}
return []byte(fmt.Sprintf("%.1f", s)), nil
}
//nolint:govet
type jsonScorecardResultV2 struct {
Date string `json:"date"`
Repo jsonRepoV2 `json:"repo"`
Scorecard jsonScorecardV2 `json:"scorecard"`
AggScore jsonFloatScore `json:"score"`
Checks []jsonCheckResultV2 `json:"checks"`
Metadata []string `json:"metadata"`
}
@ -111,8 +122,12 @@ func (r *ScorecardResult) AsJSON(showDetails bool, logLevel zapcore.Level, write
// AsJSON2 exports results as JSON for new detail format.
func (r *ScorecardResult) AsJSON2(showDetails bool,
logLevel zapcore.Level, checkDocs docs.Doc, writer io.Writer) error {
encoder := json.NewEncoder(writer)
score, err := r.aggregateScore(checkDocs)
if err != nil {
return err
}
encoder := json.NewEncoder(writer)
out := jsonScorecardResultV2{
Repo: jsonRepoV2{
Name: r.Repo.Name,
@ -124,6 +139,7 @@ func (r *ScorecardResult) AsJSON2(showDetails bool,
},
Date: r.Date.Format("2006-01-02"),
Metadata: r.Metadata,
AggScore: jsonFloatScore(score),
}
//nolint

View File

@ -1,97 +1,101 @@
{
"$schema": "http://json-schema.org/schema#",
"type": "object",
"properties": {
"checks": {
"type": "array",
"items": {
"type": "object",
"properties": {
"details": {
"$schema": "http://json-schema.org/schema#",
"type": "object",
"properties": {
"checks": {
"type": "array",
"items": {
"type": "string"
"type": "object",
"properties": {
"details": {
"type": "array",
"items": {
"type": "string"
}
},
"documentation": {
"type": "object",
"properties": {
"short": {
"type": "string"
},
"url": {
"type": "string"
}
},
"required": [
"url",
"short"
]
},
"name": {
"type": "string"
},
"reason": {
"type": "string"
},
"score": {
"type": "integer"
}
},
"required": [
"details",
"score",
"reason",
"name",
"documentation"
]
}
},
"documentation": {
},
"date": {
"type": "string"
},
"metadata": {
"type": "array",
"items": {
"type": "string"
}
},
"repo": {
"type": "object",
"properties": {
"short": {
"type": "string"
},
"url": {
"type": "string"
}
"commit": {
"type": "string"
},
"name": {
"type": "string"
}
},
"required": [
"url",
"short"
"name",
"commit"
]
},
"name": {
"type": "string"
},
"reason": {
"type": "string"
},
"score": {
"type": "integer"
}
},
"required": [
"details",
"score",
"reason",
"name",
"documentation"
]
}
},
"date": {
"type": "string"
},
"metadata": {
"type": "array",
"items": {
"type": "string"
}
},
"repo": {
"type": "object",
"properties": {
"commit": {
"type": "string"
"score": {
"type": "number"
},
"name": {
"type": "string"
"scorecard": {
"type": "object",
"properties": {
"commit": {
"type": "string"
},
"version": {
"type": "string"
}
},
"required": [
"version",
"commit"
]
}
},
"required": [
"name",
"commit"
]
},
"scorecard": {
"type": "object",
"properties": {
"commit": {
"type": "string"
},
"version": {
"type": "string"
}
},
"required": [
"version",
"commit"
]
}
},
"required": [
"date",
"repo",
"scorecard",
"checks",
"metadata"
]
}
"required": [
"date",
"repo",
"scorecard",
"score",
"checks",
"metadata"
]
}

View File

@ -34,7 +34,7 @@ func jsonMockDocRead() *mockDoc {
d := map[string]mockCheck{
"Check-Name": {
name: "Check-Name",
risk: "not used",
risk: "High",
short: "short description for Check-Name",
description: "not used",
url: "https://github.com/ossf/scorecard/blob/main/docs/checks.md#check-name",
@ -43,7 +43,7 @@ func jsonMockDocRead() *mockDoc {
},
"Check-Name2": {
name: "Check-Name2",
risk: "not used",
risk: "Medium",
short: "short description for Check-Name2",
description: "not used",
url: "https://github.com/ossf/scorecard/blob/main/docs/checks.md#check-name2",
@ -52,7 +52,7 @@ func jsonMockDocRead() *mockDoc {
},
"Check-Name3": {
name: "Check-Name3",
risk: "not used",
risk: "Low",
short: "short description for Check-Name3",
description: "not used",
url: "https://github.com/ossf/scorecard/blob/main/docs/checks.md#check-name3",
@ -494,6 +494,8 @@ func TestJSONOutput(t *testing.T) {
t.Fatalf("%s: Encode: %s", tt.name, err)
}
fmt.Println(string(result.Bytes()))
fmt.Println(string(es.Bytes()))
// Compare outputs.
r := bytes.Compare(result.Bytes(), es.Bytes())
if r != 0 {

View File

@ -52,12 +52,24 @@ type ScorecardResult struct {
Metadata []string
}
func scoreToString(s float64) string {
if s == checker.InconclusiveResultScore {
return "?"
}
return fmt.Sprintf("%.1f", s)
}
// AsCSV outputs ScorecardResult in CSV format.
func (r *ScorecardResult) AsCSV(showDetails bool, logLevel zapcore.Level,
checkDocs docs.Doc, writer io.Writer) error {
score, err := r.aggregateScore(checkDocs)
if err != nil {
return err
}
w := csv.NewWriter(writer)
record := []string{r.Repo.Name}
columns := []string{"Repository"}
record := []string{r.Repo.Name, scoreToString(score)}
columns := []string{"Repository", "AggScore"}
// UPGRADEv2: remove nolint after ugrade.
//nolint
for _, checkResult := range r.Checks {
@ -80,6 +92,45 @@ func (r *ScorecardResult) AsCSV(showDetails bool, logLevel zapcore.Level,
return nil
}
func (r *ScorecardResult) aggregateScore(checkDocs docs.Doc) (float64, error) {
// TODO: calculate the score and make it a field
// of ScorecardResult
weights := map[string]float64{"Critical": 10, "High": 7.5, "Medium": 5, "Low": 2.5}
// Note: aggregate score changes depending on which checks are run.
total := float64(0)
score := float64(0)
for i := range r.Checks {
check := r.Checks[i]
doc, e := checkDocs.GetCheck(check.Name)
if e != nil {
return checker.InconclusiveResultScore,
sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("GetCheck: %s: %v", check.Name, e))
}
risk := doc.GetRisk()
rs, exists := weights[risk]
if !exists {
return checker.InconclusiveResultScore,
sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("Invalid risk for %s: '%s'", check.Name, risk))
}
// This indicates an inconclusive score.
if check.Score < checker.MinResultScore {
continue
}
total += rs
score += rs * float64(check.Score)
}
// Inconclusive result.
if total == 0 {
return checker.InconclusiveResultScore, nil
}
return score / total, nil
}
// AsString returns ScorecardResult in string format.
func (r *ScorecardResult) AsString(showDetails bool, logLevel zapcore.Level,
checkDocs docs.Doc, writer io.Writer) error {
@ -124,6 +175,17 @@ func (r *ScorecardResult) AsString(showDetails bool, logLevel zapcore.Level,
data[i] = x
}
score, err := r.aggregateScore(checkDocs)
if err != nil {
return err
}
s := fmt.Sprintf("Aggregate score: %s / %d\n\n", scoreToString(score), checker.MaxResultScore)
if score == checker.InconclusiveResultScore {
s = "Aggregate score: ?\n\n"
}
fmt.Fprint(os.Stdout, s)
fmt.Fprintln(os.Stdout, "Check scores:")
table := tablewriter.NewWriter(os.Stdout)
header := []string{"Score", "Name", "Reason"}
if showDetails {

View File

@ -8,6 +8,7 @@
"version": "1.2.3",
"commit": "ccbc59901773ab4c051dfcea0cc4201a1567abdd"
},
"score": 5,
"checks": [
{
"details": [

View File

@ -8,6 +8,7 @@
"version": "1.2.3",
"commit": "ccbc59901773ab4c051dfcea0cc4201a1567abdd"
},
"score": 0,
"checks": [
{
"details": [

View File

@ -8,6 +8,7 @@
"version": "1.2.3",
"commit": "ccbc59901773ab4c051dfcea0cc4201a1567abdd"
},
"score": 0,
"checks": [
{
"details": [

View File

@ -8,6 +8,7 @@
"version": "1.2.3",
"commit": "ccbc59901773ab4c051dfcea0cc4201a1567abdd"
},
"score":0,
"checks": [
{
"details": [

View File

@ -8,6 +8,7 @@
"version": "1.2.3",
"commit": "ccbc59901773ab4c051dfcea0cc4201a1567abdd"
},
"score":6,
"checks": [
{
"details": [

View File

@ -8,6 +8,7 @@
"version": "1.2.3",
"commit": "ccbc59901773ab4c051dfcea0cc4201a1567abdd"
},
"score":6,
"checks": [
{
"details": [