From 8b096ad4c01ebaeb05aa8ec38505d119fa8bb2f7 Mon Sep 17 00:00:00 2001 From: laurentsimon <64505099+laurentsimon@users.noreply.github.com> Date: Tue, 12 Sep 2023 08:28:06 -0700 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20checks/evaluation=20logs=20findings?= =?UTF-8?q?=20(#3409)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * checks/validation logs findings Signed-off-by: laurentsimon * gofmt file Signed-off-by: laurentsimon * linter Signed-off-by: laurentsimon * revert go.sum Signed-off-by: laurentsimon * typo Signed-off-by: laurentsimon * add unit tests and address comments Signed-off-by: laurentsimon * update comment Signed-off-by: laurentsimon * missing file Signed-off-by: laurentsimon * use option 1 Signed-off-by: laurentsimon * use got / want in test Signed-off-by: laurentsimon * missing tests updates Signed-off-by: laurentsimon --------- Signed-off-by: laurentsimon --- checker/check_result.go | 4 +- checks/dependency_update_tool.go | 5 +- checks/dependency_update_tool_test.go | 10 +- checks/evaluation/dependency_update_tool.go | 9 +- .../evaluation/dependency_update_tool_test.go | 52 ++-- checks/evaluation/finding.go | 31 ++ checks/evaluation/fuzzing.go | 6 +- checks/evaluation/fuzzing_test.go | 288 ++++++++---------- checks/evaluation/security_policy.go | 11 +- checks/evaluation/security_policy_test.go | 50 +-- checks/fuzzing.go | 5 +- checks/fuzzing_test.go | 2 +- checks/{run_probes.go => probes.go} | 22 -- checks/security_policy.go | 5 +- e2e/dependency_update_tool_test.go | 4 +- e2e/fuzzing_test.go | 6 +- 16 files changed, 261 insertions(+), 249 deletions(-) create mode 100644 checks/evaluation/finding.go rename checks/{run_probes.go => probes.go} (59%) diff --git a/checker/check_result.go b/checker/check_result.go index 730db8ac..edb98d0f 100644 --- a/checker/check_result.go +++ b/checker/check_result.go @@ -195,7 +195,7 @@ func CreateRuntimeErrorResult(name string, e error) CheckResult { } // LogFindings logs the list of findings. -func LogFindings(findings []finding.Finding, dl DetailLogger) error { +func LogFindings(findings []finding.Finding, dl DetailLogger) { for i := range findings { f := &findings[i] switch f.Outcome { @@ -213,6 +213,4 @@ func LogFindings(findings []finding.Finding, dl DetailLogger) error { }) } } - - return nil } diff --git a/checks/dependency_update_tool.go b/checks/dependency_update_tool.go index 1222c6e0..b010511a 100644 --- a/checks/dependency_update_tool.go +++ b/checks/dependency_update_tool.go @@ -20,6 +20,7 @@ import ( "github.com/ossf/scorecard/v4/checks/raw" sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/probes" + "github.com/ossf/scorecard/v4/probes/zrunner" ) // CheckDependencyUpdateTool is the exported name for Automatic-Depdendency-Update. @@ -49,12 +50,12 @@ func DependencyUpdateTool(c *checker.CheckRequest) checker.CheckResult { pRawResults.DependencyUpdateToolResults = rawData // Evaluate the probes. - findings, err := evaluateProbes(c, pRawResults, probes.DependencyToolUpdates) + findings, err := zrunner.Run(pRawResults, probes.DependencyToolUpdates) if err != nil { e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) return checker.CreateRuntimeErrorResult(CheckDependencyUpdateTool, e) } // Return the score evaluation. - return evaluation.DependencyUpdateTool(CheckDependencyUpdateTool, findings) + return evaluation.DependencyUpdateTool(CheckDependencyUpdateTool, findings, c.Dlogger) } diff --git a/checks/dependency_update_tool_test.go b/checks/dependency_update_tool_test.go index 7ec37966..6598499c 100644 --- a/checks/dependency_update_tool_test.go +++ b/checks/dependency_update_tool_test.go @@ -51,7 +51,7 @@ func TestDependencyUpdateTool(t *testing.T) { CallSearchCommits: 0, expected: scut.TestReturn{ NumberOfInfo: 1, - NumberOfWarn: 3, + NumberOfWarn: 0, Score: 10, }, }, @@ -64,7 +64,7 @@ func TestDependencyUpdateTool(t *testing.T) { CallSearchCommits: 0, expected: scut.TestReturn{ NumberOfInfo: 1, - NumberOfWarn: 3, + NumberOfWarn: 0, Score: 10, }, }, @@ -103,7 +103,7 @@ func TestDependencyUpdateTool(t *testing.T) { CallSearchCommits: 1, expected: scut.TestReturn{ NumberOfInfo: 1, - NumberOfWarn: 3, + NumberOfWarn: 0, Score: 10, }, }, @@ -118,7 +118,7 @@ func TestDependencyUpdateTool(t *testing.T) { CallSearchCommits: 1, expected: scut.TestReturn{ NumberOfInfo: 1, - NumberOfWarn: 3, + NumberOfWarn: 0, Score: 10, }, }, @@ -136,7 +136,7 @@ func TestDependencyUpdateTool(t *testing.T) { CallSearchCommits: 1, expected: scut.TestReturn{ NumberOfInfo: 1, - NumberOfWarn: 3, + NumberOfWarn: 0, Score: 10, }, }, diff --git a/checks/evaluation/dependency_update_tool.go b/checks/evaluation/dependency_update_tool.go index a60ab7f2..6a45afc5 100644 --- a/checks/evaluation/dependency_update_tool.go +++ b/checks/evaluation/dependency_update_tool.go @@ -24,9 +24,10 @@ import ( "github.com/ossf/scorecard/v4/probes/toolSonatypeLiftInstalled" ) -// DependencyUpdateTool applies the score policy for the Dependency-Update-Tool check. +// DependencyUpdateTool applies the score policy and logs the details +// for the Dependency-Update-Tool check. func DependencyUpdateTool(name string, - findings []finding.Finding, + findings []finding.Finding, dl checker.DetailLogger, ) checker.CheckResult { expectedProbes := []string{ toolDependabotInstalled.Probe, @@ -42,9 +43,13 @@ func DependencyUpdateTool(name string, for i := range findings { f := &findings[i] if f.Outcome == finding.OutcomePositive { + // Log all findings except the negative ones. + checker.LogFindings(nonNegativeFindings(findings), dl) return checker.CreateMaxScoreResult(name, "update tool detected") } } + // Log all findings. + checker.LogFindings(findings, dl) return checker.CreateMinScoreResult(name, "no update tool detected") } diff --git a/checks/evaluation/dependency_update_tool_test.go b/checks/evaluation/dependency_update_tool_test.go index 98be6cee..bbe0a0e3 100644 --- a/checks/evaluation/dependency_update_tool_test.go +++ b/checks/evaluation/dependency_update_tool_test.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/ossf/scorecard/v4/checker" + sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/finding" scut "github.com/ossf/scorecard/v4/utests" ) @@ -28,9 +29,7 @@ func TestDependencyUpdateTool(t *testing.T) { tests := []struct { name string findings []finding.Finding - err bool - want checker.CheckResult - expected scut.TestReturn + result scut.TestReturn }{ { name: "dependabot", @@ -52,8 +51,9 @@ func TestDependencyUpdateTool(t *testing.T) { Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: 10, + result: scut.TestReturn{ + Score: checker.MaxResultScore, + NumberOfInfo: 1, }, }, { @@ -76,8 +76,9 @@ func TestDependencyUpdateTool(t *testing.T) { Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: 10, + result: scut.TestReturn{ + Score: checker.MaxResultScore, + NumberOfInfo: 1, }, }, { @@ -100,8 +101,9 @@ func TestDependencyUpdateTool(t *testing.T) { Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: 10, + result: scut.TestReturn{ + Score: checker.MaxResultScore, + NumberOfInfo: 1, }, }, { @@ -128,8 +130,9 @@ func TestDependencyUpdateTool(t *testing.T) { Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: 10, + result: scut.TestReturn{ + Score: checker.MaxResultScore, + NumberOfInfo: 1, }, }, { @@ -152,8 +155,9 @@ func TestDependencyUpdateTool(t *testing.T) { Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: 0, + result: scut.TestReturn{ + Score: checker.MinResultScore, + NumberOfWarn: 4, }, }, { @@ -172,9 +176,9 @@ func TestDependencyUpdateTool(t *testing.T) { Outcome: finding.OutcomeNegative, }, }, - err: true, - want: checker.CheckResult{ - Score: -1, + result: scut.TestReturn{ + Score: checker.InconclusiveResultScore, + Error: sce.ErrScorecardInternal, }, }, { @@ -201,8 +205,9 @@ func TestDependencyUpdateTool(t *testing.T) { Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: -1, + result: scut.TestReturn{ + Score: checker.InconclusiveResultScore, + Error: sce.ErrScorecardInternal, }, }, } @@ -211,13 +216,10 @@ func TestDependencyUpdateTool(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - got := DependencyUpdateTool(tt.name, tt.findings) - if tt.want.Score != got.Score { - t.Errorf("DependencyUpdateTool() got Score = %v, want %v for %v", got.Score, tt.want.Score, tt.name) - } - if tt.err && got.Error == nil { - t.Errorf("DependencyUpdateTool() error = %v, want %v for %v", got.Error, tt.want.Error, tt.name) - return + dl := scut.TestDetailLogger{} + got := DependencyUpdateTool(tt.name, tt.findings, &dl) + if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) { + t.Errorf("got %v, expected %v", got, tt.result) } }) } diff --git a/checks/evaluation/finding.go b/checks/evaluation/finding.go new file mode 100644 index 00000000..f11c8fad --- /dev/null +++ b/checks/evaluation/finding.go @@ -0,0 +1,31 @@ +// 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 nonNegativeFindings(findings []finding.Finding) []finding.Finding { + var ff []finding.Finding + for i := range findings { + f := &findings[i] + if f.Outcome == finding.OutcomeNegative { + continue + } + ff = append(ff, *f) + } + return ff +} diff --git a/checks/evaluation/fuzzing.go b/checks/evaluation/fuzzing.go index 8fe621ed..f058c2e6 100644 --- a/checks/evaluation/fuzzing.go +++ b/checks/evaluation/fuzzing.go @@ -29,7 +29,7 @@ import ( // Fuzzing applies the score policy for the Fuzzing check. func Fuzzing(name string, - findings []finding.Finding, + findings []finding.Finding, dl checker.DetailLogger, ) checker.CheckResult { // We have 7 unique probes, each should have a finding. expectedProbes := []string{ @@ -51,8 +51,12 @@ func Fuzzing(name string, for i := range findings { f := &findings[i] if f.Outcome == finding.OutcomePositive { + // Log all findings except the negative ones. + checker.LogFindings(nonNegativeFindings(findings), dl) 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/fuzzing_test.go b/checks/evaluation/fuzzing_test.go index 1300f1e2..a1b54847 100644 --- a/checks/evaluation/fuzzing_test.go +++ b/checks/evaluation/fuzzing_test.go @@ -16,190 +16,166 @@ package evaluation import ( "testing" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/ossf/scorecard/v4/checker" + sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/finding" + scut "github.com/ossf/scorecard/v4/utests" ) func TestFuzzing(t *testing.T) { t.Parallel() - type args struct { //nolint + tests := []struct { name string findings []finding.Finding - } - tests := []struct { - name string - args args - want checker.CheckResult + result scut.TestReturn }{ { name: "Fuzzing - no fuzzing", - args: args{ - name: "Fuzzing", - findings: []finding.Finding{ - { - Probe: "fuzzedWithClusterFuzzLite", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithGoNative", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithOneFuzz", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithOSSFuzz", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithPropertyBasedHaskell", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithPropertyBasedJavascript", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithPropertyBasedTypescript", - Outcome: finding.OutcomeNegative, - }, + findings: []finding.Finding{ + { + Probe: "fuzzedWithClusterFuzzLite", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithGoNative", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithOneFuzz", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithOSSFuzz", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithPropertyBasedHaskell", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithPropertyBasedJavascript", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithPropertyBasedTypescript", + Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: 0, - Name: "Fuzzing", - Version: 2, - Reason: "project is not fuzzed", + result: scut.TestReturn{ + Score: checker.MinResultScore, + NumberOfWarn: 7, }, }, { name: "Fuzzing - fuzzing GoNative", - args: args{ - name: "Fuzzing", - findings: []finding.Finding{ - { - Probe: "fuzzedWithClusterFuzzLite", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithGoNative", - Outcome: finding.OutcomePositive, - }, - { - Probe: "fuzzedWithOneFuzz", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithOSSFuzz", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithPropertyBasedHaskell", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithPropertyBasedJavascript", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithPropertyBasedTypescript", - Outcome: finding.OutcomeNegative, - }, + findings: []finding.Finding{ + { + Probe: "fuzzedWithClusterFuzzLite", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithGoNative", + Outcome: finding.OutcomePositive, + }, + { + Probe: "fuzzedWithOneFuzz", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithOSSFuzz", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithPropertyBasedHaskell", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithPropertyBasedJavascript", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithPropertyBasedTypescript", + Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: 10, - Name: "Fuzzing", - Version: 2, - Reason: "project is fuzzed", + result: scut.TestReturn{ + Score: checker.MaxResultScore, + NumberOfInfo: 1, }, }, + { name: "Fuzzing - fuzzing missing GoNative finding", - args: args{ - name: "Fuzzing", - findings: []finding.Finding{ - { - Probe: "fuzzedWithClusterFuzzLite", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithOneFuzz", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithOSSFuzz", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithPropertyBasedHaskell", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithPropertyBasedJavascript", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithPropertyBasedTypescript", - Outcome: finding.OutcomeNegative, - }, + findings: []finding.Finding{ + { + Probe: "fuzzedWithClusterFuzzLite", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithOneFuzz", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithOSSFuzz", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithPropertyBasedHaskell", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithPropertyBasedJavascript", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithPropertyBasedTypescript", + Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: -1, - Name: "Fuzzing", - Version: 2, - Reason: "internal error: invalid probe results", + result: scut.TestReturn{ + Score: checker.InconclusiveResultScore, + Error: sce.ErrScorecardInternal, }, }, { name: "Fuzzing - fuzzing invalid probe name", - args: args{ - name: "Fuzzing", - findings: []finding.Finding{ - { - Probe: "fuzzedWithClusterFuzzLite", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithGoNative", - Outcome: finding.OutcomePositive, - }, - { - Probe: "fuzzedWithOneFuzz", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithOSSFuzz", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithPropertyBasedHaskell", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithPropertyBasedJavascript", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithPropertyBasedTypescript", - Outcome: finding.OutcomeNegative, - }, - { - Probe: "fuzzedWithInvalidProbeName", - Outcome: finding.OutcomePositive, - }, + findings: []finding.Finding{ + { + Probe: "fuzzedWithClusterFuzzLite", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithGoNative", + Outcome: finding.OutcomePositive, + }, + { + Probe: "fuzzedWithOneFuzz", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithOSSFuzz", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithPropertyBasedHaskell", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithPropertyBasedJavascript", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithPropertyBasedTypescript", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "fuzzedWithInvalidProbeName", + Outcome: finding.OutcomePositive, }, }, - want: checker.CheckResult{ - Score: -1, - Name: "Fuzzing", - Version: 2, - Reason: "internal error: invalid probe results", + result: scut.TestReturn{ + Score: checker.InconclusiveResultScore, + Error: sce.ErrScorecardInternal, }, }, } @@ -207,8 +183,10 @@ func TestFuzzing(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := Fuzzing(tt.args.name, tt.args.findings); !cmp.Equal(got, tt.want, cmpopts.IgnoreFields(checker.CheckResult{}, "Error")) { //nolint:lll - t.Errorf("Fuzzing() = %v, want %v", got, cmp.Diff(got, tt.want, cmpopts.IgnoreFields(checker.CheckResult{}, "Error"))) //nolint:lll + dl := scut.TestDetailLogger{} + got := Fuzzing(tt.name, tt.findings, &dl) + if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) { + t.Errorf("got %v, expected %v", got, tt.result) } }) } diff --git a/checks/evaluation/security_policy.go b/checks/evaluation/security_policy.go index 736fdf3a..10125c6a 100644 --- a/checks/evaluation/security_policy.go +++ b/checks/evaluation/security_policy.go @@ -25,7 +25,7 @@ import ( ) // SecurityPolicy applies the score policy for the Security-Policy check. -func SecurityPolicy(name string, findings []finding.Finding) checker.CheckResult { +func SecurityPolicy(name string, findings []finding.Finding, dl checker.DetailLogger) checker.CheckResult { // We have 4 unique probes, each should have a finding. expectedProbes := []string{ securityPolicyContainsVulnerabilityDisclosure.Probe, @@ -64,9 +64,18 @@ func SecurityPolicy(name string, findings []finding.Finding) checker.CheckResult 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 positive. + // If the score is less than checker.MaxResultScore, some findings are negative, + // so we log both positive and negative 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 35a8563f..2e3f9395 100644 --- a/checks/evaluation/security_policy_test.go +++ b/checks/evaluation/security_policy_test.go @@ -18,7 +18,9 @@ import ( "testing" "github.com/ossf/scorecard/v4/checker" + sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/finding" + scut "github.com/ossf/scorecard/v4/utests" ) func TestSecurityPolicy(t *testing.T) { @@ -27,8 +29,7 @@ func TestSecurityPolicy(t *testing.T) { tests := []struct { name string findings []finding.Finding - err bool - want checker.CheckResult + result scut.TestReturn }{ { name: "missing findings links", @@ -46,8 +47,9 @@ func TestSecurityPolicy(t *testing.T) { Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: -1, + result: scut.TestReturn{ + Score: checker.InconclusiveResultScore, + Error: sce.ErrScorecardInternal, }, }, { @@ -74,8 +76,9 @@ func TestSecurityPolicy(t *testing.T) { Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: -1, + result: scut.TestReturn{ + Score: checker.InconclusiveResultScore, + Error: sce.ErrScorecardInternal, }, }, { @@ -98,8 +101,10 @@ func TestSecurityPolicy(t *testing.T) { Outcome: finding.OutcomePositive, }, }, - want: checker.CheckResult{ - Score: 0, + result: scut.TestReturn{ + Score: checker.MinResultScore, + NumberOfInfo: 1, + NumberOfWarn: 3, }, }, { @@ -122,8 +127,9 @@ func TestSecurityPolicy(t *testing.T) { Outcome: finding.OutcomeNegative, }, }, - want: checker.CheckResult{ - Score: -1, + result: scut.TestReturn{ + Score: checker.InconclusiveResultScore, + Error: sce.ErrScorecardInternal, }, }, { @@ -146,8 +152,10 @@ func TestSecurityPolicy(t *testing.T) { Outcome: finding.OutcomePositive, }, }, - want: checker.CheckResult{ - Score: 6, + result: scut.TestReturn{ + Score: 6, + NumberOfInfo: 2, + NumberOfWarn: 2, }, }, { @@ -170,8 +178,9 @@ func TestSecurityPolicy(t *testing.T) { Outcome: finding.OutcomePositive, }, }, - want: checker.CheckResult{ - Score: 10, + result: scut.TestReturn{ + Score: checker.MaxResultScore, + NumberOfInfo: 4, }, }, } @@ -180,15 +189,10 @@ func TestSecurityPolicy(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - - got := SecurityPolicy("SecurityPolicy", tt.findings) - if tt.err { - if got.Score != -1 { - t.Errorf("SecurityPolicy() = %v, want %v", got, tt.want) - } - } - if got.Score != tt.want.Score { - t.Errorf("SecurityPolicy() = %v, want %v for %v", got.Score, tt.want.Score, tt.name) + dl := scut.TestDetailLogger{} + got := SecurityPolicy(tt.name, tt.findings, &dl) + if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) { + t.Errorf("got %v, expected %v", got, tt.result) } }) } diff --git a/checks/fuzzing.go b/checks/fuzzing.go index 6d26df57..67748301 100644 --- a/checks/fuzzing.go +++ b/checks/fuzzing.go @@ -20,6 +20,7 @@ import ( "github.com/ossf/scorecard/v4/checks/raw" sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/probes" + "github.com/ossf/scorecard/v4/probes/zrunner" ) // CheckFuzzing is the registered name for Fuzzing. @@ -46,12 +47,12 @@ func Fuzzing(c *checker.CheckRequest) checker.CheckResult { pRawResults.FuzzingResults = rawData // Evaluate the probes. - findings, err := evaluateProbes(c, pRawResults, probes.Fuzzing) + findings, err := zrunner.Run(pRawResults, probes.Fuzzing) if err != nil { e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) return checker.CreateRuntimeErrorResult(CheckFuzzing, e) } // Return the score evaluation. - return evaluation.Fuzzing(CheckFuzzing, findings) + return evaluation.Fuzzing(CheckFuzzing, findings, c.Dlogger) } diff --git a/checks/fuzzing_test.go b/checks/fuzzing_test.go index 5fd34bbd..191eedfd 100644 --- a/checks/fuzzing_test.go +++ b/checks/fuzzing_test.go @@ -76,7 +76,7 @@ func TestFuzzing(t *testing.T) { }, wantErr: false, expected: scut.TestReturn{ - NumberOfWarn: 6, + NumberOfWarn: 0, NumberOfDebug: 0, NumberOfInfo: 1, Score: 10, diff --git a/checks/run_probes.go b/checks/probes.go similarity index 59% rename from checks/run_probes.go rename to checks/probes.go index 22ff0e58..357da984 100644 --- a/checks/run_probes.go +++ b/checks/probes.go @@ -15,31 +15,9 @@ package checks import ( - "fmt" - "github.com/ossf/scorecard/v4/checker" - "github.com/ossf/scorecard/v4/finding" - "github.com/ossf/scorecard/v4/probes" - "github.com/ossf/scorecard/v4/probes/zrunner" ) -// evaluateProbes runs the probes in probesToRun and logs its findings. -func evaluateProbes(c *checker.CheckRequest, rawResults *checker.RawResults, - probesToRun []probes.ProbeImpl, -) ([]finding.Finding, error) { - // Run the probes. - findings, err := zrunner.Run(rawResults, probesToRun) - if err != nil { - return nil, fmt.Errorf("zrunner.Run: %w", err) - } - - // Log the findings. - if err := checker.LogFindings(findings, c.Dlogger); err != nil { - return nil, fmt.Errorf("LogFindings: %w", err) - } - return findings, nil -} - // getRawResults returns a pointer to the raw results in the CheckRequest // if the pointer is not nil. Else, it creates a new raw result. func getRawResults(c *checker.CheckRequest) *checker.RawResults { diff --git a/checks/security_policy.go b/checks/security_policy.go index 902c7032..1046c566 100644 --- a/checks/security_policy.go +++ b/checks/security_policy.go @@ -20,6 +20,7 @@ import ( "github.com/ossf/scorecard/v4/checks/raw" sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/probes" + "github.com/ossf/scorecard/v4/probes/zrunner" ) // CheckSecurityPolicy is the registred name for SecurityPolicy. @@ -49,12 +50,12 @@ func SecurityPolicy(c *checker.CheckRequest) checker.CheckResult { pRawResults.SecurityPolicyResults = rawData // Evaluate the probes. - findings, err := evaluateProbes(c, pRawResults, probes.SecurityPolicy) + findings, err := zrunner.Run(pRawResults, probes.SecurityPolicy) if err != nil { e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) return checker.CreateRuntimeErrorResult(CheckSecurityPolicy, e) } // Return the score evaluation. - return evaluation.SecurityPolicy(CheckSecurityPolicy, findings) + return evaluation.SecurityPolicy(CheckSecurityPolicy, findings, c.Dlogger) } diff --git a/e2e/dependency_update_tool_test.go b/e2e/dependency_update_tool_test.go index 836f5c96..a580a6bb 100644 --- a/e2e/dependency_update_tool_test.go +++ b/e2e/dependency_update_tool_test.go @@ -48,7 +48,7 @@ var _ = Describe("E2E TEST:"+checks.CheckDependencyUpdateTool, func() { expected := scut.TestReturn{ Error: nil, Score: checker.MaxResultScore, - NumberOfWarn: 3, + NumberOfWarn: 0, NumberOfInfo: 1, NumberOfDebug: 0, } @@ -75,7 +75,7 @@ var _ = Describe("E2E TEST:"+checks.CheckDependencyUpdateTool, func() { expected := scut.TestReturn{ Error: nil, Score: checker.MaxResultScore, - NumberOfWarn: 3, + NumberOfWarn: 0, NumberOfInfo: 1, NumberOfDebug: 0, } diff --git a/e2e/fuzzing_test.go b/e2e/fuzzing_test.go index 6d33e84c..7344e3d7 100644 --- a/e2e/fuzzing_test.go +++ b/e2e/fuzzing_test.go @@ -51,7 +51,7 @@ var _ = Describe("E2E TEST:"+checks.CheckFuzzing, func() { expected := scut.TestReturn{ Error: nil, Score: checker.MaxResultScore, - NumberOfWarn: 6, + NumberOfWarn: 0, NumberOfInfo: 1, NumberOfDebug: 0, } @@ -80,7 +80,7 @@ var _ = Describe("E2E TEST:"+checks.CheckFuzzing, func() { expected := scut.TestReturn{ Error: nil, Score: checker.MaxResultScore, - NumberOfWarn: 6, + NumberOfWarn: 0, NumberOfInfo: 1, NumberOfDebug: 0, } @@ -109,7 +109,7 @@ var _ = Describe("E2E TEST:"+checks.CheckFuzzing, func() { expected := scut.TestReturn{ Error: nil, Score: checker.MaxResultScore, - NumberOfWarn: 6, + NumberOfWarn: 0, NumberOfInfo: 2, NumberOfDebug: 0, }