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

⚠️ Delegate logging decisions to the checks instead of a helper #4019

Merged
merged 11 commits into from
Apr 9, 2024
28 changes: 10 additions & 18 deletions checker/check_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,23 +243,15 @@ func CreateRuntimeErrorResult(name string, e error) CheckResult {
}
}

// LogFindings logs the list of findings.
func LogFindings(findings []finding.Finding, dl DetailLogger) {
for i := range findings {
f := &findings[i]
switch f.Outcome {
case finding.OutcomeFalse:
dl.Warn(&LogMessage{
Finding: f,
})
case finding.OutcomeTrue:
dl.Info(&LogMessage{
Finding: f,
})
default:
dl.Debug(&LogMessage{
Finding: f,
})
}
// LogFinding logs the given finding at the given level.
func LogFinding(dl DetailLogger, f *finding.Finding, level DetailType) {
lm := LogMessage{Finding: f}
switch level {
case DetailDebug:
dl.Debug(&lm)
case DetailInfo:
dl.Info(&lm)
case DetailWarn:
dl.Warn(&lm)
}
}
20 changes: 14 additions & 6 deletions checks/evaluation/dependency_update_tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,24 @@ func DependencyUpdateTool(name string,
return checker.CreateRuntimeErrorResult(name, e)
}

var usesTool bool
for i := range findings {
f := &findings[i]
if f.Outcome == finding.OutcomeTrue {
// Log all findings except the false ones.
checker.LogFindings(nonFalseFindings(findings), dl)
return checker.CreateMaxScoreResult(name, "update tool detected")
var logLevel checker.DetailType
switch f.Outcome {
case finding.OutcomeFalse:
logLevel = checker.DetailWarn
case finding.OutcomeTrue:
usesTool = true
logLevel = checker.DetailInfo
default:
logLevel = checker.DetailDebug
}
checker.LogFinding(dl, f, logLevel)
}

// Log all findings.
checker.LogFindings(findings, dl)
if usesTool {
return checker.CreateMaxScoreResult(name, "update tool detected")
}
return checker.CreateMinScoreResult(name, "no update tool detected")
}
42 changes: 0 additions & 42 deletions checks/evaluation/finding.go

This file was deleted.

21 changes: 15 additions & 6 deletions checks/evaluation/fuzzing.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,25 @@ func Fuzzing(name string,
return checker.CreateRuntimeErrorResult(name, e)
}

var fuzzerDetected bool
// Compute the score.
for i := range findings {
f := &findings[i]
if f.Outcome == finding.OutcomeTrue {
// Log all findings except the false ones.
checker.LogFindings(nonFalseFindings(findings), dl)
return checker.CreateMaxScoreResult(name, "project is fuzzed")
var logLevel checker.DetailType
switch f.Outcome {
case finding.OutcomeFalse:
logLevel = checker.DetailWarn
case finding.OutcomeTrue:
fuzzerDetected = true
logLevel = checker.DetailInfo
default:
logLevel = checker.DetailDebug
}
checker.LogFinding(dl, f, logLevel)
}

if fuzzerDetected {
return checker.CreateMaxScoreResult(name, "project is fuzzed")
}
// Log all findings.
checker.LogFindings(findings, dl)
return checker.CreateMinScoreResult(name, "project is not fuzzed")
}
13 changes: 9 additions & 4 deletions checks/evaluation/license.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ func License(name string,
findings []finding.Finding,
dl checker.DetailLogger,
) checker.CheckResult {
// We have 3 unique probes, each should have a finding.
expectedProbes := []string{
hasLicenseFile.Probe,
hasFSFOrOSIApprovedLicense.Probe,
Expand All @@ -41,9 +40,12 @@ func License(name string,
// Compute the score.
score := 0
m := make(map[string]bool)
var logLevel checker.DetailType
for i := range findings {
f := &findings[i]
if f.Outcome == finding.OutcomeTrue {
switch f.Outcome {
case finding.OutcomeTrue:
logLevel = checker.DetailInfo
switch f.Probe {
case hasFSFOrOSIApprovedLicense.Probe:
score += scoreProbeOnce(f.Probe, m, 1)
Expand All @@ -53,10 +55,13 @@ func License(name string,
e := sce.WithMessage(sce.ErrScorecardInternal, "unknown probe results")
return checker.CreateRuntimeErrorResult(name, e)
}
case finding.OutcomeFalse:
logLevel = checker.DetailWarn
default:
logLevel = checker.DetailDebug
}
checker.LogFinding(dl, f, logLevel)
}
checker.LogFindings(findings, dl)

_, defined := m[hasLicenseFile.Probe]
if !defined {
if score > 0 {
Expand Down
55 changes: 25 additions & 30 deletions checks/evaluation/maintained.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,15 @@ func Maintained(name string,
return checker.CreateRuntimeErrorResult(name, e)
}

if projectIsArchived(findings) {
checker.LogFindings(falseFindings(findings), dl)
return checker.CreateMinScoreResult(name, "project is archived")
}

if projectWasCreatedInLast90Days(findings) {
checker.LogFindings(falseFindings(findings), dl)
return checker.CreateMinScoreResult(name, "project was created in last 90 days. please review its contents carefully")
}
var archived, recentlyCreated bool

var commitsWithinThreshold, numberOfIssuesUpdatedWithinThreshold int
var err error
for i := range findings {
f := &findings[i]
if f.Outcome == finding.OutcomeTrue {
// currently this works but when we switch notArchived for #3654 this will need to also check the probe
switch f.Outcome {
case finding.OutcomeTrue:
switch f.Probe {
case issueActivityByProjectMember.Probe:
numberOfIssuesUpdatedWithinThreshold, err = strconv.Atoi(f.Values[issueActivityByProjectMember.NumIssuesKey])
Expand All @@ -77,31 +71,32 @@ func Maintained(name string,
return checker.CreateRuntimeErrorResult(name, sce.WithMessage(sce.ErrScorecardInternal, err.Error()))
}
}
case finding.OutcomeFalse:
switch f.Probe {
case notArchived.Probe:
archived = true
case notCreatedRecently.Probe:
recentlyCreated = true
// informational probes dont need logged, but they do factor into the score
case hasRecentCommits.Probe, issueActivityByProjectMember.Probe:
continue
}
checker.LogFinding(dl, f, checker.DetailWarn)
default:
checker.LogFinding(dl, f, checker.DetailDebug)
}
}

if archived {
return checker.CreateMinScoreResult(name, "project is archived")
}

if recentlyCreated {
return checker.CreateMinScoreResult(name, "project was created in last 90 days. please review its contents carefully")
}

return checker.CreateProportionalScoreResult(name, fmt.Sprintf(
"%d commit(s) and %d issue activity found in the last %d days",
commitsWithinThreshold, numberOfIssuesUpdatedWithinThreshold, lookBackDays),
commitsWithinThreshold+numberOfIssuesUpdatedWithinThreshold, activityPerWeek*lookBackDays/daysInOneWeek)
}

func projectIsArchived(findings []finding.Finding) bool {
for i := range findings {
f := &findings[i]
if f.Outcome == finding.OutcomeFalse && f.Probe == notArchived.Probe {
return true
}
}
return false
}

func projectWasCreatedInLast90Days(findings []finding.Finding) bool {
for i := range findings {
f := &findings[i]
if f.Outcome == finding.OutcomeFalse && f.Probe == notCreatedRecently.Probe {
return true
}
}
return false
}
2 changes: 1 addition & 1 deletion checks/evaluation/maintained_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func TestMaintained(t *testing.T) {
},
result: scut.TestReturn{
Score: 0,
NumberOfWarn: 3,
NumberOfWarn: 1,
},
},
}
Expand Down
23 changes: 12 additions & 11 deletions checks/evaluation/packaging.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,22 @@ func Packaging(name string,
// we return max score if the outcome is true and lowest score if
// the outcome is false.
maxScore := false
for _, f := range findings {
f := f
if f.Outcome == finding.OutcomeTrue {
for i := range findings {
f := &findings[i]
var logLevel checker.DetailType
switch f.Outcome {
case finding.OutcomeFalse:
logLevel = checker.DetailWarn
case finding.OutcomeTrue:
maxScore = true
// Log all findings except the false ones.
dl.Info(&checker.LogMessage{
Finding: &f,
})
logLevel = checker.DetailInfo
default:
logLevel = checker.DetailDebug
}
checker.LogFinding(dl, f, logLevel)
}
if maxScore {
return checker.CreateMaxScoreResult(name, "packaging workflow detected")
}

checker.LogFindings(falseFindings(findings), dl)
return checker.CreateInconclusiveResult(name,
"packaging workflow not detected")
return checker.CreateInconclusiveResult(name, "packaging workflow not detected")
}
21 changes: 10 additions & 11 deletions checks/evaluation/security_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,13 @@ func SecurityPolicy(name string, findings []finding.Finding, dl checker.DetailLo

score := 0
m := make(map[string]bool)
var logLevel checker.DetailType
for i := range findings {
f := &findings[i]
if f.Outcome == finding.OutcomeTrue {
// all of the security policy probes are good things if true and bad if false
switch f.Outcome {
case finding.OutcomeTrue:
logLevel = checker.DetailInfo
switch f.Probe {
case securityPolicyContainsVulnerabilityDisclosure.Probe:
score += scoreProbeOnce(f.Probe, m, 1)
Expand All @@ -56,26 +60,21 @@ func SecurityPolicy(name string, findings []finding.Finding, dl checker.DetailLo
e := sce.WithMessage(sce.ErrScorecardInternal, "unknown probe results")
return checker.CreateRuntimeErrorResult(name, e)
}
case finding.OutcomeFalse:
logLevel = checker.DetailWarn
default:
logLevel = checker.DetailDebug
}
checker.LogFinding(dl, f, logLevel)
}
_, defined := m[securityPolicyPresent.Probe]
if !defined {
if score > 0 {
e := sce.WithMessage(sce.ErrScorecardInternal, "score calculation problem")
return checker.CreateRuntimeErrorResult(name, e)
}

// Log all findings.
checker.LogFindings(findings, dl)
return checker.CreateMinScoreResult(name, "security policy file not detected")
}

// Log all findings.
// NOTE: if the score is checker.MaxResultScore, then all findings are true.
// If the score is less than checker.MaxResultScore, some findings are false,
// so we log both true and false findings.
checker.LogFindings(findings, dl)

return checker.CreateResultWithScore(name, "security policy file detected", score)
}

Expand Down
6 changes: 4 additions & 2 deletions checks/evaluation/security_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,10 @@ func TestSecurityPolicy(t *testing.T) {
},
},
result: scut.TestReturn{
Score: checker.InconclusiveResultScore,
Error: sce.ErrScorecardInternal,
Score: checker.InconclusiveResultScore,
Error: sce.ErrScorecardInternal,
NumberOfWarn: 1,
NumberOfInfo: 3,
},
},
{
Expand Down
12 changes: 9 additions & 3 deletions checks/evaluation/signed_releases.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ func SignedReleases(name string,
totalTrue := 0
releaseMap := make(map[string]int)
uniqueReleaseTags := make([]string, 0)
checker.LogFindings(findings, dl)

var logLevel checker.DetailType
for i := range findings {
f := &findings[i]

Expand All @@ -86,9 +86,10 @@ func SignedReleases(name string,
uniqueReleaseTags = append(uniqueReleaseTags, releaseName)
}

if f.Outcome == finding.OutcomeTrue {
switch f.Outcome {
case finding.OutcomeTrue:
logLevel = checker.DetailInfo
totalTrue++

switch f.Probe {
case releasesAreSigned.Probe:
if _, ok := releaseMap[releaseName]; !ok {
Expand All @@ -97,7 +98,12 @@ func SignedReleases(name string,
case releasesHaveProvenance.Probe:
releaseMap[releaseName] = 10
}
case finding.OutcomeFalse:
logLevel = checker.DetailWarn
default:
logLevel = checker.DetailDebug
}
checker.LogFinding(dl, f, logLevel)
}

if totalTrue == 0 {
Expand Down
Loading
Loading