From 76a9b0470a7eb2c94a192a1c394d2fade47bbd7d Mon Sep 17 00:00:00 2001 From: Spencer Schrock Date: Thu, 4 Apr 2024 14:43:30 -0700 Subject: [PATCH] :warning: Only include probes which ran for `probe` format (#3991) * add findings to check results struct these dont make it to the JSON output format as theyre not copied to the jsonCheckResultV2 struct in AsJSON2() Signed-off-by: Spencer Schrock * populate CheckResult findings It would be nice if the evaluation functions did this for us, but would require changes to theCreate*ScoreResult functions. It was simpler just to set it in one place at the check level. Signed-off-by: Spencer Schrock --------- Signed-off-by: Spencer Schrock --- checker/check_result.go | 5 +++-- checks/binary_artifact.go | 4 +++- checks/branch_protection.go | 4 +++- checks/ci_tests.go | 4 +++- checks/cii_best_practices.go | 4 +++- checks/code_review.go | 1 + checks/contributors.go | 4 +++- checks/dangerous_workflow.go | 4 +++- checks/dependency_update_tool.go | 4 +++- checks/fuzzing.go | 4 +++- checks/license.go | 4 +++- checks/maintained.go | 4 +++- checks/packaging.go | 4 +++- checks/permissions.go | 4 +++- checks/pinned_dependencies.go | 4 +++- checks/sast.go | 4 +++- checks/security_policy.go | 4 +++- checks/signed_releases.go | 4 +++- checks/vulnerabilities.go | 4 +++- checks/webhook.go | 4 +++- pkg/scorecard.go | 22 ++++------------------ 21 files changed, 62 insertions(+), 38 deletions(-) diff --git a/checker/check_result.go b/checker/check_result.go index b6aa83d7..9efd2310 100644 --- a/checker/check_result.go +++ b/checker/check_result.go @@ -66,8 +66,9 @@ type CheckResult struct { Score int Reason string Details []CheckDetail - // Structured results. - Rules []string // TODO(X): add support. + + // Findings from the check's probes. + Findings []finding.Finding } // CheckDetail contains information for each detail. diff --git a/checks/binary_artifact.go b/checks/binary_artifact.go index f57d1545..820e748c 100644 --- a/checks/binary_artifact.go +++ b/checks/binary_artifact.go @@ -57,5 +57,7 @@ func BinaryArtifacts(c *checker.CheckRequest) checker.CheckResult { return checker.CreateRuntimeErrorResult(CheckBinaryArtifacts, e) } - return evaluation.BinaryArtifacts(CheckBinaryArtifacts, findings, c.Dlogger) + ret := evaluation.BinaryArtifacts(CheckBinaryArtifacts, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/branch_protection.go b/checks/branch_protection.go index 33f6d609..75f872c6 100644 --- a/checks/branch_protection.go +++ b/checks/branch_protection.go @@ -54,5 +54,7 @@ func BranchProtection(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. - return evaluation.BranchProtection(CheckBranchProtection, findings, c.Dlogger) + ret := evaluation.BranchProtection(CheckBranchProtection, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/ci_tests.go b/checks/ci_tests.go index b5d37405..6d595c6a 100644 --- a/checks/ci_tests.go +++ b/checks/ci_tests.go @@ -53,5 +53,7 @@ func CITests(c *checker.CheckRequest) checker.CheckResult { return checker.CreateRuntimeErrorResult(CheckCITests, e) } - return evaluation.CITests(CheckCITests, findings, c.Dlogger) + ret := evaluation.CITests(CheckCITests, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/cii_best_practices.go b/checks/cii_best_practices.go index 4f39c9c7..fb8a9b7a 100644 --- a/checks/cii_best_practices.go +++ b/checks/cii_best_practices.go @@ -54,5 +54,7 @@ func CIIBestPractices(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. - return evaluation.CIIBestPractices(CheckCIIBestPractices, findings, c.Dlogger) + ret := evaluation.CIIBestPractices(CheckCIIBestPractices, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/code_review.go b/checks/code_review.go index 6791ca54..70634e73 100644 --- a/checks/code_review.go +++ b/checks/code_review.go @@ -49,5 +49,6 @@ func CodeReview(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. + // TODO(#3049) return evaluation.CodeReview(CheckCodeReview, c.Dlogger, &rawData) } diff --git a/checks/contributors.go b/checks/contributors.go index 15db76f2..e83e0b5e 100644 --- a/checks/contributors.go +++ b/checks/contributors.go @@ -54,5 +54,7 @@ func Contributors(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. - return evaluation.Contributors(CheckContributors, findings, c.Dlogger) + ret := evaluation.Contributors(CheckContributors, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/dangerous_workflow.go b/checks/dangerous_workflow.go index 71384bfe..9a9e8e1b 100644 --- a/checks/dangerous_workflow.go +++ b/checks/dangerous_workflow.go @@ -57,5 +57,7 @@ func DangerousWorkflow(c *checker.CheckRequest) checker.CheckResult { return checker.CreateRuntimeErrorResult(CheckDangerousWorkflow, e) } - return evaluation.DangerousWorkflow(CheckDangerousWorkflow, findings, c.Dlogger) + ret := evaluation.DangerousWorkflow(CheckDangerousWorkflow, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/dependency_update_tool.go b/checks/dependency_update_tool.go index 486c4a19..d8d667c8 100644 --- a/checks/dependency_update_tool.go +++ b/checks/dependency_update_tool.go @@ -57,5 +57,7 @@ func DependencyUpdateTool(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. - return evaluation.DependencyUpdateTool(CheckDependencyUpdateTool, findings, c.Dlogger) + ret := evaluation.DependencyUpdateTool(CheckDependencyUpdateTool, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/fuzzing.go b/checks/fuzzing.go index 67748301..0efa3ef3 100644 --- a/checks/fuzzing.go +++ b/checks/fuzzing.go @@ -54,5 +54,7 @@ func Fuzzing(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. - return evaluation.Fuzzing(CheckFuzzing, findings, c.Dlogger) + ret := evaluation.Fuzzing(CheckFuzzing, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/license.go b/checks/license.go index f062cf5a..a832a1ae 100644 --- a/checks/license.go +++ b/checks/license.go @@ -56,5 +56,7 @@ func License(c *checker.CheckRequest) checker.CheckResult { return checker.CreateRuntimeErrorResult(CheckLicense, e) } - return evaluation.License(CheckLicense, findings, c.Dlogger) + ret := evaluation.License(CheckLicense, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/maintained.go b/checks/maintained.go index d94e6978..7e64e3d8 100644 --- a/checks/maintained.go +++ b/checks/maintained.go @@ -54,5 +54,7 @@ func Maintained(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. - return evaluation.Maintained(CheckMaintained, findings, c.Dlogger) + ret := evaluation.Maintained(CheckMaintained, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/packaging.go b/checks/packaging.go index cfa78782..34c7af9f 100644 --- a/checks/packaging.go +++ b/checks/packaging.go @@ -65,5 +65,7 @@ func Packaging(c *checker.CheckRequest) checker.CheckResult { return checker.CreateRuntimeErrorResult(CheckPackaging, e) } - return evaluation.Packaging(CheckPackaging, findings, c.Dlogger) + ret := evaluation.Packaging(CheckPackaging, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/permissions.go b/checks/permissions.go index 8d7fbe73..1ace874d 100644 --- a/checks/permissions.go +++ b/checks/permissions.go @@ -58,5 +58,7 @@ func TokenPermissions(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. - return evaluation.TokenPermissions(CheckTokenPermissions, findings, c.Dlogger) + ret := evaluation.TokenPermissions(CheckTokenPermissions, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/pinned_dependencies.go b/checks/pinned_dependencies.go index 04f8af51..a1f04065 100644 --- a/checks/pinned_dependencies.go +++ b/checks/pinned_dependencies.go @@ -58,5 +58,7 @@ func PinningDependencies(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. - return evaluation.PinningDependencies(CheckPinnedDependencies, findings, c.Dlogger) + ret := evaluation.PinningDependencies(CheckPinnedDependencies, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/sast.go b/checks/sast.go index 405e4929..11f497e2 100644 --- a/checks/sast.go +++ b/checks/sast.go @@ -54,5 +54,7 @@ func SAST(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. - return evaluation.SAST(CheckSAST, findings, c.Dlogger) + ret := evaluation.SAST(CheckSAST, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/security_policy.go b/checks/security_policy.go index 30410d02..38fa5088 100644 --- a/checks/security_policy.go +++ b/checks/security_policy.go @@ -57,5 +57,7 @@ func SecurityPolicy(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. - return evaluation.SecurityPolicy(CheckSecurityPolicy, findings, c.Dlogger) + ret := evaluation.SecurityPolicy(CheckSecurityPolicy, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/signed_releases.go b/checks/signed_releases.go index 52267243..a4473aaa 100644 --- a/checks/signed_releases.go +++ b/checks/signed_releases.go @@ -54,5 +54,7 @@ func SignedReleases(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. - return evaluation.SignedReleases(CheckSignedReleases, findings, c.Dlogger) + ret := evaluation.SignedReleases(CheckSignedReleases, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/vulnerabilities.go b/checks/vulnerabilities.go index 34bd302f..a5b8a086 100644 --- a/checks/vulnerabilities.go +++ b/checks/vulnerabilities.go @@ -57,5 +57,7 @@ func Vulnerabilities(c *checker.CheckRequest) checker.CheckResult { return checker.CreateRuntimeErrorResult(CheckVulnerabilities, e) } - return evaluation.Vulnerabilities(CheckVulnerabilities, findings, c.Dlogger) + ret := evaluation.Vulnerabilities(CheckVulnerabilities, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/checks/webhook.go b/checks/webhook.go index cc328456..c84c5987 100644 --- a/checks/webhook.go +++ b/checks/webhook.go @@ -69,5 +69,7 @@ func WebHooks(c *checker.CheckRequest) checker.CheckResult { } // Return the score evaluation. - return evaluation.Webhooks(CheckWebHooks, findings, c.Dlogger) + ret := evaluation.Webhooks(CheckWebHooks, findings, c.Dlogger) + ret.Findings = findings + return ret } diff --git a/pkg/scorecard.go b/pkg/scorecard.go index 6d074049..45012ca7 100644 --- a/pkg/scorecard.go +++ b/pkg/scorecard.go @@ -32,8 +32,6 @@ import ( "github.com/ossf/scorecard/v4/finding" proberegistration "github.com/ossf/scorecard/v4/internal/probes" "github.com/ossf/scorecard/v4/options" - "github.com/ossf/scorecard/v4/probes" - "github.com/ossf/scorecard/v4/probes/zrunner" ) // errEmptyRepository indicates the repository is empty. @@ -164,25 +162,13 @@ func runScorecard(ctx context.Context, // If the user runs checks go runEnabledChecks(ctx, repo, request, checksToRun, resultsCh) + exposeFindings := os.Getenv(options.EnvVarScorecardExperimental) == "1" for result := range resultsCh { ret.Checks = append(ret.Checks, result) - } - if value, _ := os.LookupEnv(options.EnvVarScorecardExperimental); value == "1" { - // Run the probes. - var findings []finding.Finding - // TODO(#3049): only run the probes for checks. - // NOTE: We will need separate functions to support: - // - `--probes X,Y` - // - `--check-definitions-file path/to/config.yml - // NOTE: we discard the returned error because the errors are - // already contained in the findings and we want to return the findings - // to users. - // See https://github.com/ossf/scorecard/blob/main/probes/zrunner/runner.go#L34-L45. - // We also don't want the entire scorecard run to fail if a single error is encountered. - //nolint:errcheck - findings, _ = zrunner.Run(&ret.RawResults, probes.All) - ret.Findings = findings + if exposeFindings { + ret.Findings = append(ret.Findings, result.Findings...) + } } return ret, nil }