🌱 Convert Dangerous Workflow check to probes (ossf#3521)

* 🌱 Convert Dangerous Workflow check to probes Signed-off-by: AdamKorcz <[email protected]> * remove hasAnyWorkflows probe Signed-off-by: AdamKorcz <[email protected]> * combine two conditionals into one Signed-off-by: AdamKorcz <[email protected]> * preserve logging from original evaluation Signed-off-by: AdamKorcz <[email protected]> * rebase Signed-off-by: AdamKorcz <[email protected]> --------- Signed-off-by: AdamKorcz <[email protected]> Signed-off-by: Allen Shearin <[email protected]>
kgangerlm · Nov 13, 2023 · 230f169 · 230f169
1 parent 5fbb614
commit 230f169
Show file tree

Hide file tree

Showing 12 changed files with 712 additions and 151 deletions.
diff --git a/checks/dangerous_workflow.go b/checks/dangerous_workflow.go
@@ -19,6 +19,8 @@ import (
 	"github.com/ossf/scorecard/v4/checks/evaluation"
 	"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"
 )
 
 // CheckDangerousWorkflow is the exported name for Dangerous-Workflow check.
@@ -38,17 +40,22 @@ func init() {
 
 // DangerousWorkflow  will check the repository contains Dangerous-Workflow.
 func DangerousWorkflow(c *checker.CheckRequest) checker.CheckResult {
-	rawData, err := raw.DangerousWorkflow(c.RepoClient)
+	rawData, err := raw.DangerousWorkflow(c)
 	if err != nil {
 		e := sce.WithMessage(sce.ErrScorecardInternal, err.Error())
 		return checker.CreateRuntimeErrorResult(CheckDangerousWorkflow, e)
 	}
 
-	// Return raw results.
-	if c.RawResults != nil {
-		c.RawResults.DangerousWorkflowResults = rawData
+	// Set the raw results.
+	pRawResults := getRawResults(c)
+	pRawResults.DangerousWorkflowResults = rawData
+
+	// Evaluate the probes.
+	findings, err := zrunner.Run(pRawResults, probes.DangerousWorkflows)
+	if err != nil {
+		e := sce.WithMessage(sce.ErrScorecardInternal, err.Error())
+		return checker.CreateRuntimeErrorResult(CheckDangerousWorkflow, e)
 	}
 
-	// Return the score evaluation.
-	return evaluation.DangerousWorkflow(CheckDangerousWorkflow, c.Dlogger, &rawData)
+	return evaluation.DangerousWorkflow(CheckDangerousWorkflow, findings, c.Dlogger)
 }
diff --git a/checks/evaluation/dangerous_workflow.go b/checks/evaluation/dangerous_workflow.go
@@ -15,59 +15,89 @@
 package evaluation
 
 import (
-	"fmt"
-
 	"github.com/ossf/scorecard/v4/checker"
 	sce "github.com/ossf/scorecard/v4/errors"
+	"github.com/ossf/scorecard/v4/finding"
+	"github.com/ossf/scorecard/v4/probes/hasDangerousWorkflowScriptInjection"
+	"github.com/ossf/scorecard/v4/probes/hasDangerousWorkflowUntrustedCheckout"
 )
 
 // DangerousWorkflow applies the score policy for the DangerousWorkflow check.
-func DangerousWorkflow(name string, dl checker.DetailLogger,
-	r *checker.DangerousWorkflowData,
+func DangerousWorkflow(name string,
+	findings []finding.Finding, dl checker.DetailLogger,
 ) checker.CheckResult {
-	if r == nil {
-		e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data")
+	expectedProbes := []string{
+		hasDangerousWorkflowScriptInjection.Probe,
+		hasDangerousWorkflowUntrustedCheckout.Probe,
+	}
+
+	if !finding.UniqueProbesEqual(findings, expectedProbes) {
+		e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results")
 		return checker.CreateRuntimeErrorResult(name, e)
 	}
 
-	if r.NumWorkflows == 0 {
+	if !hasWorkflows(findings) {
 		return checker.CreateInconclusiveResult(name, "no workflows found")
 	}
 
-	for _, e := range r.Workflows {
-		var text string
-		switch e.Type {
-		case checker.DangerousWorkflowUntrustedCheckout:
-			text = fmt.Sprintf("untrusted code checkout '%v'", e.File.Snippet)
-		case checker.DangerousWorkflowScriptInjection:
-			text = fmt.Sprintf("script injection with untrusted input '%v'", e.File.Snippet)
-		default:
-			err := sce.WithMessage(sce.ErrScorecardInternal, "invalid type")
-			return checker.CreateRuntimeErrorResult(name, err)
+	// Log all detected dangerous workflows
+	for i := range findings {
+		f := &findings[i]
+		if f.Outcome == finding.OutcomeNegative {
+			if f.Location == nil {
+				e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results")
+				return checker.CreateRuntimeErrorResult(name, e)
+			}
+			dl.Warn(&checker.LogMessage{
+				Path:    f.Location.Path,
+				Type:    f.Location.Type,
+				Offset:  *f.Location.LineStart,
+				Text:    f.Message,
+				Snippet: *f.Location.Snippet,
+			})
 		}
+	}
 
-		dl.Warn(&checker.LogMessage{
-			Path:    e.File.Path,
-			Type:    e.File.Type,
-			Offset:  e.File.Offset,
-			Text:    text,
-			Snippet: e.File.Snippet,
-		})
+	if hasDWWithUntrustedCheckout(findings) || hasDWWithScriptInjection(findings) {
+		return checker.CreateMinScoreResult(name,
+			"dangerous workflow patterns detected")
 	}
 
-	if len(r.Workflows) > 0 {
-		return createResult(name, checker.MinResultScore)
+	return checker.CreateMaxScoreResult(name,
+		"no dangerous workflow patterns detected")
+}
+
+// Both probes return OutcomeNotApplicable, if there project has no workflows.
+func hasWorkflows(findings []finding.Finding) bool {
+	for i := range findings {
+		f := &findings[i]
+		if f.Outcome == finding.OutcomeNotApplicable {
+			return false
+		}
 	}
-	return createResult(name, checker.MaxResultScore)
+	return true
 }
 
-// Create the result.
-func createResult(name string, score int) checker.CheckResult {
-	if score != checker.MaxResultScore {
-		return checker.CreateResultWithScore(name,
-			"dangerous workflow patterns detected", score)
+func hasDWWithUntrustedCheckout(findings []finding.Finding) bool {
+	for i := range findings {
+		f := &findings[i]
+		if f.Probe == hasDangerousWorkflowUntrustedCheckout.Probe {
+			if f.Outcome == finding.OutcomeNegative {
+				return true
+			}
+		}
 	}
+	return false
+}
 
-	return checker.CreateMaxScoreResult(name,
-		"no dangerous workflow patterns detected")
+func hasDWWithScriptInjection(findings []finding.Finding) bool {
+	for i := range findings {
+		f := &findings[i]
+		if f.Probe == hasDangerousWorkflowScriptInjection.Probe {
+			if f.Outcome == finding.OutcomeNegative {
+				return true
+			}
+		}
+	}
+	return false
 }