diff --git a/checker/check_result.go b/checker/check_result.go index 485f3748a42..31e9f15b2bf 100644 --- a/checker/check_result.go +++ b/checker/check_result.go @@ -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) } } diff --git a/checks/evaluation/dependency_update_tool.go b/checks/evaluation/dependency_update_tool.go index 05cb0b72986..a764a1e69fb 100644 --- a/checks/evaluation/dependency_update_tool.go +++ b/checks/evaluation/dependency_update_tool.go @@ -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") } diff --git a/checks/evaluation/finding.go b/checks/evaluation/finding.go deleted file mode 100644 index 97e79bf044a..00000000000 --- a/checks/evaluation/finding.go +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2023 OpenSSF Scorecard Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package evaluation - -import ( - "github.com/ossf/scorecard/v4/finding" -) - -func nonFalseFindings(findings []finding.Finding) []finding.Finding { - var ff []finding.Finding - for i := range findings { - f := &findings[i] - if f.Outcome == finding.OutcomeFalse { - continue - } - ff = append(ff, *f) - } - return ff -} - -func falseFindings(findings []finding.Finding) []finding.Finding { - var ff []finding.Finding - for i := range findings { - f := &findings[i] - if f.Outcome == finding.OutcomeFalse { - ff = append(ff, *f) - } - } - return ff -} diff --git a/checks/evaluation/fuzzing.go b/checks/evaluation/fuzzing.go index 1995c7b3c28..1f48da797d0 100644 --- a/checks/evaluation/fuzzing.go +++ b/checks/evaluation/fuzzing.go @@ -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") } diff --git a/checks/evaluation/license.go b/checks/evaluation/license.go index 2caf3c41070..219b87552d6 100644 --- a/checks/evaluation/license.go +++ b/checks/evaluation/license.go @@ -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, @@ -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) @@ -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 { diff --git a/checks/evaluation/maintained.go b/checks/evaluation/maintained.go index fb974471f16..1ef9daa06dd 100644 --- a/checks/evaluation/maintained.go +++ b/checks/evaluation/maintained.go @@ -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]) @@ -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 -} diff --git a/checks/evaluation/maintained_test.go b/checks/evaluation/maintained_test.go index d2ec527f940..89bf9b16f7d 100644 --- a/checks/evaluation/maintained_test.go +++ b/checks/evaluation/maintained_test.go @@ -121,7 +121,7 @@ func TestMaintained(t *testing.T) { }, result: scut.TestReturn{ Score: 0, - NumberOfWarn: 3, + NumberOfWarn: 1, }, }, } diff --git a/checks/evaluation/packaging.go b/checks/evaluation/packaging.go index df5151afb33..0bb6af54541 100644 --- a/checks/evaluation/packaging.go +++ b/checks/evaluation/packaging.go @@ -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") } diff --git a/checks/evaluation/security_policy.go b/checks/evaluation/security_policy.go index 8ec3356dee4..33abf22d080 100644 --- a/checks/evaluation/security_policy.go +++ b/checks/evaluation/security_policy.go @@ -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) @@ -56,7 +60,12 @@ 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 { @@ -64,18 +73,8 @@ func SecurityPolicy(name string, findings []finding.Finding, dl checker.DetailLo 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) } diff --git a/checks/evaluation/security_policy_test.go b/checks/evaluation/security_policy_test.go index ccaa7512718..88aa90d7184 100644 --- a/checks/evaluation/security_policy_test.go +++ b/checks/evaluation/security_policy_test.go @@ -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, }, }, { diff --git a/checks/evaluation/signed_releases.go b/checks/evaluation/signed_releases.go index 5b7c06ca75a..581794104a8 100644 --- a/checks/evaluation/signed_releases.go +++ b/checks/evaluation/signed_releases.go @@ -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] @@ -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 { @@ -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 { diff --git a/checks/evaluation/vulnerabilities.go b/checks/evaluation/vulnerabilities.go index fc5e1e60283..4c37e23f36f 100644 --- a/checks/evaluation/vulnerabilities.go +++ b/checks/evaluation/vulnerabilities.go @@ -37,9 +37,15 @@ func Vulnerabilities(name string, return checker.CreateRuntimeErrorResult(name, e) } - vulnsFound := falseFindings(findings) - numVulnsFound := len(vulnsFound) - checker.LogFindings(vulnsFound, dl) + var numVulnsFound int + for i := range findings { + f := &findings[i] + // TODO(#3654), this needs to be swapped. But it's a complicated swap so doing it not in here. + if f.Outcome == finding.OutcomeFalse { + numVulnsFound++ + checker.LogFinding(dl, f, checker.DetailWarn) + } + } score := checker.MaxResultScore - numVulnsFound