Skip to content

Commit

Permalink
🐛 Address friction logs' comments (#899)
Browse files Browse the repository at this point in the history
* fixes

* fix

* fix

* fixes

* doc

* missing file

* fixes

* comments

* typo
  • Loading branch information
laurentsimon committed Aug 25, 2021
1 parent 1c7c1e3 commit 9eb7929
Show file tree
Hide file tree
Showing 10 changed files with 167 additions and 117 deletions.
186 changes: 103 additions & 83 deletions README.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ const CheckDependencyUpdateTool = "Dependency-Update-Tool"

//nolint
func init() {
registerCheck(CheckDependencyUpdateTool, AutomaticDependencyUpdate)
registerCheck(CheckDependencyUpdateTool, UsesDependencyUpdateTool)
}

// AutomaticDependencyUpdate will check the repository if it contains Automatic dependency update.
func AutomaticDependencyUpdate(c *checker.CheckRequest) checker.CheckResult {
// UsesDependencyUpdateTool will check the repository uses a dependency update tool.
func UsesDependencyUpdateTool(c *checker.CheckRequest) checker.CheckResult {
var r bool
err := CheckIfFileExists(CheckDependencyUpdateTool, c, fileExists, &r)
if err != nil {
Expand Down
26 changes: 13 additions & 13 deletions checks/active.go → checks/maintained.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,37 +23,37 @@ import (
)

const (
// CheckActive is the exported check name for Active.
CheckActive = "Active"
lookBackDays = 90
commitsPerWeek = 1
daysInOneWeek = 7
// CheckMaintained is the exported check name for Maintained.
CheckMaintained = "Maintained"
lookBackDays = 90
commitsPerWeek = 1
daysInOneWeek = 7
)

//nolint:gochecknoinits
func init() {
registerCheck(CheckActive, IsActive)
registerCheck(CheckMaintained, IsMaintained)
}

// IsActive runs Active check.
func IsActive(c *checker.CheckRequest) checker.CheckResult {
// IsMaintained runs Maintained check.
func IsMaintained(c *checker.CheckRequest) checker.CheckResult {
archived, err := c.RepoClient.IsArchived()
if err != nil {
return checker.CreateRuntimeErrorResult(CheckActive, err)
return checker.CreateRuntimeErrorResult(CheckMaintained, err)
}
if archived {
return checker.CreateMinScoreResult(CheckActive, "repo is marked as archived")
return checker.CreateMinScoreResult(CheckMaintained, "repo is marked as archived")
}

commits, err := c.RepoClient.ListCommits()
if err != nil {
return checker.CreateRuntimeErrorResult(CheckActive, err)
return checker.CreateRuntimeErrorResult(CheckMaintained, err)
}

tz, err := time.LoadLocation("UTC")
if err != nil {
e := sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("time.LoadLocation: %v", err))
return checker.CreateRuntimeErrorResult(CheckActive, e)
return checker.CreateRuntimeErrorResult(CheckMaintained, e)
}
threshold := time.Now().In(tz).AddDate(0, 0, -1*lookBackDays)
totalCommits := 0
Expand All @@ -62,7 +62,7 @@ func IsActive(c *checker.CheckRequest) checker.CheckResult {
totalCommits++
}
}
return checker.CreateProportionalScoreResult(CheckActive,
return checker.CreateProportionalScoreResult(CheckMaintained,
fmt.Sprintf("%d commit(s) found in the last %d days", totalCommits, lookBackDays),
totalCommits, commitsPerWeek*lookBackDays/daysInOneWeek)
}
26 changes: 26 additions & 0 deletions checks/write.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,27 @@
# Requirements for a check

If you'd like to add a check, make sure it is something that meets the following
criteria and then create a new GitHub Issue to discuss with the team:

- The scorecard must only be composed of automate-able, objective data. For
example, a project having 10 contributors doesn’t necessarily mean it’s more
secure than a project with say 50 contributors. But, having two maintainers
might be preferable to only having one - the larger bus factor and ability
to provide code reviews is objectively better.
- The scorecard criteria can be as specific as possible and not limited
general recommendations. For example, for Go, we can recommend/require
specific linters and analyzers to be run on the codebase.
- The scorecard can be populated for any open source project without any work
or interaction from maintainers.
- Maintainers must be provided with a mechanism to correct any automated
scorecard findings they feel were made in error, provide "hints" for
anything we can't detect automatically, and even dispute the applicability
of a given scorecard finding for that repository.
- Any criteria in the scorecard must be actionable. It should be possible,
with help, for any project to "check all the boxes".
- Any solution to compile a scorecard should be usable by the greater open
source community to monitor upstream security.

# How to write a check

The steps to writting a check are as follow:
Expand Down Expand Up @@ -25,6 +49,8 @@ The steps to writting a check are as follow:
* Use `checker.DetailLogger.Debug()` to provide detail in verbose mode:
this is showed only when the user supplies the `--verbosity Debug`
option.
* If your message relates to a file, try to provide information such as
the `Path`, line number `Offset` and `Snippet`.

4. If the checks fails in a way that is irrecoverable, return a result with
`checker.CreateRuntimeErrorResult()` function: For example, if an error is
Expand Down
2 changes: 1 addition & 1 deletion docs/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ please contribute!

This check tries to determine if the project is "actively maintained".
A project which is not active may not be patched, may not have its dependencies patched, or may not be actively tested and used. A low score is therefore considered `High` risk.
The check currently works by looking for commits within the last 90 days, and outputs the highest score if there are at least 1 commit/week during this period.
The check currently works by looking whether the repo is archived or not. If it is archived, it returns the minimum score. If it is not, the check looks for commits within the last 90 days, and outputs the highest score if there are at least 1 commit/week during this period.

**Remediation steps**
- There is *NO* remediation work needed here. This is just to indicate your project activity and maintenance commitment.
Expand Down
6 changes: 4 additions & 2 deletions docs/checks/checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# This is the source of truth for all check descriptions and remediation steps.
# Run `cd checks/main && go run /main` to generate `checks.json` and `checks.md`.
checks:
Active:
Maintained:
risk: High
tags: supply-chain, security
short: Determines if the project is "actively maintained".
Expand All @@ -26,7 +26,9 @@ checks:
dependencies patched, or may not be actively tested and used.
A low score is therefore considered `High` risk.
The check currently works by looking for commits within the last 90 days, and
The check currently works by looking whether the repo is archived or not.
If it is archived, it returns the minimum score. If it is not,
the check looks for commits within the last 90 days, and
outputs the highest score if there are at least 1 commit/week during this period.
remediation:
- >-
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ import (

// TODO: use dedicated repo that don't change.
// TODO: need negative results.
var _ = Describe("E2E TEST:Automatic-Dependency-Update", func() {
Context("E2E TEST:Validating dependencies are automatically updated", func() {
It("Should return deps are automatically updated for dependabot", func() {
var _ = Describe("E2E TEST:"+checks.CheckDependencyUpdateTool, func() {
Context("E2E TEST:Validating dependencies are updated with a tool", func() {
It("Should return repo uses dependabot", func() {
dl := scut.TestDetailLogger{}
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), ghClient, graphClient)
err := repoClient.InitRepo("ossf", "scorecard")
Expand All @@ -54,15 +54,15 @@ var _ = Describe("E2E TEST:Automatic-Dependency-Update", func() {
NumberOfDebug: 0,
}

result := checks.AutomaticDependencyUpdate(&req)
result := checks.UsesDependencyUpdateTool(&req)
// UPGRADEv2: to remove.
// Old version.
Expect(result.Error).Should(BeNil())
Expect(result.Pass).Should(BeTrue())
// New version.
Expect(scut.ValidateTestReturn(nil, "dependabot", &expected, &result, &dl)).Should(BeTrue())
})
It("Should return deps are automatically updated for renovatebot", func() {
It("Should return repo uses renovatebot", func() {
dl := scut.TestDetailLogger{}
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), ghClient, graphClient)
err := repoClient.InitRepo("netlify", "netlify-cms")
Expand All @@ -84,7 +84,7 @@ var _ = Describe("E2E TEST:Automatic-Dependency-Update", func() {
NumberOfInfo: 1,
NumberOfDebug: 0,
}
result := checks.AutomaticDependencyUpdate(&req)
result := checks.UsesDependencyUpdateTool(&req)
// UPGRADEv2: to remove.
// Old version.
Expect(result.Error).Should(BeNil())
Expand Down
4 changes: 3 additions & 1 deletion e2e/executable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ var _ = Describe("E2E TEST:executable", func() {

for _, c := range data.Checks {
switch c.CheckName {
case checks.CheckActive:
case checks.CheckMaintained:
Expect(c.Pass).Should(BeTrue(), c.CheckName)
case checks.CheckDependencyUpdateTool:
Expect(c.Pass).Should(BeTrue(), c.CheckName)
case checks.CheckBranchProtection:
Expect(c.Pass).Should(BeTrue(), c.CheckName)
Expand Down
8 changes: 4 additions & 4 deletions e2e/active_test.go → e2e/maintained_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ import (
scut "github.com/ossf/scorecard/v2/utests"
)

var _ = Describe("E2E TEST:Active", func() {
Context("E2E TEST:Validating active status", func() {
It("Should return valid active status", func() {
var _ = Describe("E2E TEST:"+checks.CheckMaintained, func() {
Context("E2E TEST:Validating maintained status", func() {
It("Should return valid maintained status", func() {
dl := scut.TestDetailLogger{}
repoClient := githubrepo.CreateGithubRepoClient(context.Background(), ghClient, graphClient)
err := repoClient.InitRepo("apache", "airflow")
Expand All @@ -51,7 +51,7 @@ var _ = Describe("E2E TEST:Active", func() {
NumberOfInfo: 0,
NumberOfDebug: 0,
}
result := checks.IsActive(&req)
result := checks.IsMaintained(&req)
// UPGRADEv2: to remove.
// Old version.
Expect(result.Error).Should(BeNil())
Expand Down
8 changes: 4 additions & 4 deletions pkg/scorecard_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ func (r *ScorecardResult) AsString(showDetails bool, logLevel zapcore.Level, wri
if row.Score == checker.InconclusiveResultScore {
x[0] = "?"
} else {
x[0] = fmt.Sprintf("%d", row.Score)
x[0] = fmt.Sprintf("%d / %d", row.Score, checker.MaxResultScore)
}

doc := fmt.Sprintf("github.com/ossf/scorecard/blob/main/docs/checks.md#%s", strings.ToLower(row.Name))
x[1] = row.Reason
x[2] = row.Name
x[1] = row.Name
x[2] = row.Reason
if showDetails {
details, show := detailsToString(row.Details2, logLevel)
if show {
Expand All @@ -107,7 +107,7 @@ func (r *ScorecardResult) AsString(showDetails bool, logLevel zapcore.Level, wri
}

table := tablewriter.NewWriter(os.Stdout)
header := []string{"Score", "Reason", "Name"}
header := []string{"Score", "Name", "Reason"}
if showDetails {
header = append(header, "Details")
}
Expand Down

0 comments on commit 9eb7929

Please sign in to comment.