✨ [experimental] Probe support for security policy check (ossf#3241)

* update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * update Signed-off-by: laurentsimon <[email protected]> * fix unit tests Signed-off-by: laurentsimon <[email protected]> * comments Signed-off-by: laurentsimon <[email protected]> * compilation fix Signed-off-by: laurentsimon <[email protected]> * missing file Signed-off-by: laurentsimon <[email protected]> * missing file Signed-off-by: laurentsimon <[email protected]> * update reason string Signed-off-by: laurentsimon <[email protected]> * typo Signed-off-by: laurentsimon <[email protected]> * fix unit tests Signed-off-by: laurentsimon <[email protected]> * typo Signed-off-by: laurentsimon <[email protected]> * unit tests and linnter Signed-off-by: laurentsimon <[email protected]> * comments Signed-off-by: laurentsimon <[email protected]> * comments Signed-off-by: laurentsimon <[email protected]> * missing file Signed-off-by: laurentsimon <[email protected]> * unit tests for probes Signed-off-by: laurentsimon <[email protected]> * linter Signed-off-by: laurentsimon <[email protected]> * revert FileSize change Signed-off-by: laurentsimon <[email protected]> --------- Signed-off-by: laurentsimon <[email protected]>
laurentsimon · Aug 4, 2023 · a8b255a · a8b255a
1 parent f30ff23
commit a8b255a
Show file tree

Hide file tree

Showing 39 changed files with 2,466 additions and 318 deletions.
diff --git a/checks/dependency_update_tool.go b/checks/dependency_update_tool.go
@@ -44,13 +44,12 @@ func DependencyUpdateTool(c *checker.CheckRequest) checker.CheckResult {
 		return checker.CreateRuntimeErrorResult(CheckDependencyUpdateTool, e)
 	}
 
-	// Return raw results.
-	if c.RawResults != nil {
-		c.RawResults.DependencyUpdateToolResults = rawData
-	}
+	// Set the raw results.
+	pRawResults := getRawResults(c)
+	pRawResults.DependencyUpdateToolResults = rawData
 
 	// Evaluate the probes.
-	findings, err := evaluateProbes(c, CheckDependencyUpdateTool, probes.DependencyToolUpdates)
+	findings, err := evaluateProbes(c, pRawResults, probes.DependencyToolUpdates)
 	if err != nil {
 		e := sce.WithMessage(sce.ErrScorecardInternal, err.Error())
 		return checker.CreateRuntimeErrorResult(CheckDependencyUpdateTool, e)

diff --git a/checks/dependency_update_tool_test.go b/checks/dependency_update_tool_test.go
@@ -150,11 +150,9 @@ func TestDependencyUpdateTool(t *testing.T) {
 			mockRepo.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil)
 			mockRepo.EXPECT().SearchCommits(gomock.Any()).Return(tt.SearchCommits, nil).Times(tt.CallSearchCommits)
 			dl := scut.TestDetailLogger{}
-			raw := checker.RawResults{}
 			c := &checker.CheckRequest{
 				RepoClient: mockRepo,
 				Dlogger:    &dl,
-				RawResults: &raw,
 			}
 			res := DependencyUpdateTool(c)
 

diff --git a/checks/evaluation/dependency_update_tool.go b/checks/evaluation/dependency_update_tool.go
@@ -16,13 +16,29 @@ package evaluation
 
 import (
 	"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/toolDependabotInstalled"
+	"github.com/ossf/scorecard/v4/probes/toolPyUpInstalled"
+	"github.com/ossf/scorecard/v4/probes/toolRenovateInstalled"
+	"github.com/ossf/scorecard/v4/probes/toolSonatypeLiftInstalled"
 )
 
 // DependencyUpdateTool applies the score policy for the Dependency-Update-Tool check.
 func DependencyUpdateTool(name string,
 	findings []finding.Finding,
 ) checker.CheckResult {
+	expectedProbes := []string{
+		toolDependabotInstalled.Probe,
+		toolPyUpInstalled.Probe,
+		toolRenovateInstalled.Probe,
+		toolSonatypeLiftInstalled.Probe,
+	}
+	if !finding.UniqueProbesEqual(findings, expectedProbes) {
+		e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results")
+		return checker.CreateRuntimeErrorResult(name, e)
+	}
+
 	for i := range findings {
 		f := &findings[i]
 		if f.Outcome == finding.OutcomePositive {

diff --git a/checks/evaluation/dependency_update_tool_test.go b/checks/evaluation/dependency_update_tool_test.go
@@ -39,6 +39,18 @@ func TestDependencyUpdateTool(t *testing.T) {
 					Probe:   "toolDependabotInstalled",
 					Outcome: finding.OutcomePositive,
 				},
+				{
+					Probe:   "toolPyUpInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "toolRenovateInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "toolSonatypeLiftInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
 			},
 			want: checker.CheckResult{
 				Score: 10,
@@ -47,10 +59,22 @@ func TestDependencyUpdateTool(t *testing.T) {
 		{
 			name: "renovate",
 			findings: []finding.Finding{
+				{
+					Probe:   "toolDependabotInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "toolPyUpInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
 				{
 					Probe:   "toolRenovateInstalled",
 					Outcome: finding.OutcomePositive,
 				},
+				{
+					Probe:   "toolSonatypeLiftInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
 			},
 			want: checker.CheckResult{
 				Score: 10,
@@ -59,10 +83,22 @@ func TestDependencyUpdateTool(t *testing.T) {
 		{
 			name: "pyup",
 			findings: []finding.Finding{
+				{
+					Probe:   "toolDependabotInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
 				{
 					Probe:   "toolPyUpInstalled",
 					Outcome: finding.OutcomePositive,
 				},
+				{
+					Probe:   "toolRenovateInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "toolSonatypeLiftInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
 			},
 			want: checker.CheckResult{
 				Score: 10,
@@ -72,7 +108,19 @@ func TestDependencyUpdateTool(t *testing.T) {
 			name: "sonatype",
 			findings: []finding.Finding{
 				{
-					Probe:   "toolSonatypeInstalled",
+					Probe:   "toolDependabotInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "toolPyUpInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "toolRenovateInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "toolSonatypeLiftInstalled",
 					Outcome: finding.OutcomePositive,
 				},
 			},
@@ -96,7 +144,7 @@ func TestDependencyUpdateTool(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 				{
-					Probe:   "toolSonatypeInstalled",
+					Probe:   "toolSonatypeLiftInstalled",
 					Outcome: finding.OutcomeNegative,
 				},
 			},
@@ -107,8 +155,7 @@ func TestDependencyUpdateTool(t *testing.T) {
 		{
 			name: "empty tool list",
 			want: checker.CheckResult{
-				Score: 0,
-				Error: nil,
+				Score: -1,
 			},
 		},
 	}

diff --git a/checks/evaluation/security_policy.go b/checks/evaluation/security_policy.go
@@ -18,139 +18,62 @@ import (
 	"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/securityPolicyContainsLinks"
+	"github.com/ossf/scorecard/v4/probes/securityPolicyContainsText"
+	"github.com/ossf/scorecard/v4/probes/securityPolicyContainsVulnerabilityDisclosure"
+	"github.com/ossf/scorecard/v4/probes/securityPolicyPresent"
 )
 
-func scoreSecurityCriteria(f checker.File,
-	info []checker.SecurityPolicyInformation,
-	dl checker.DetailLogger,
-) int {
-	var urls, emails, discvuls, linkedContentLen, score int
-
-	emails = countSecInfo(info, checker.SecurityPolicyInformationTypeEmail, true)
-	urls = countSecInfo(info, checker.SecurityPolicyInformationTypeLink, true)
-	discvuls = countSecInfo(info, checker.SecurityPolicyInformationTypeText, false)
-
-	for _, i := range findSecInfo(info, checker.SecurityPolicyInformationTypeEmail, true) {
-		linkedContentLen += len(i.InformationValue.Match)
-	}
-	for _, i := range findSecInfo(info, checker.SecurityPolicyInformationTypeLink, true) {
-		linkedContentLen += len(i.InformationValue.Match)
-	}
-
-	msg := checker.LogMessage{
-		Path: f.Path,
-		Type: f.Type,
-		Text: "",
-	}
-
-	// #1: linked content found (email/http): score += 6
-	if (urls + emails) > 0 {
-		score += 6
-		msg.Text = "Found linked content in security policy"
-		dl.Info(&msg)
-	} else {
-		msg.Text = "no email or URL found in security policy"
-		dl.Warn(&msg)
-	}
-
-	// #2: more bytes than the sum of the length of all the linked content found: score += 3
-	//     rationale: there appears to be information and context around those links
-	//     no credit if there is just a link to a site or an email address (those given above)
-	//     the test here is that each piece of linked content will likely contain a space
-	//     before and after the content (hence the two multiplier)
-	if f.FileSize > 1 && (f.FileSize > uint(linkedContentLen+((urls+emails)*2))) {
-		score += 3
-		msg.Text = "Found text in security policy"
-		dl.Info(&msg)
-	} else {
-		msg.Text = "No text (beyond any linked content) found in security policy"
-		dl.Warn(&msg)
+// SecurityPolicy applies the score policy for the Security-Policy check.
+func SecurityPolicy(name string, findings []finding.Finding) checker.CheckResult {
+	// We have 4 unique probes, each should have a finding.
+	expectedProbes := []string{
+		securityPolicyContainsVulnerabilityDisclosure.Probe,
+		securityPolicyContainsLinks.Probe,
+		securityPolicyContainsText.Probe,
+		securityPolicyPresent.Probe,
 	}
-
-	// #3: found whole number(s) and or match(es) to "Disclos" and or "Vuln": score += 1
-	//     rationale: works towards the intent of the security policy file
-	//     regarding whom to contact about vuls and disclosures and timing
-	//     e.g., we'll disclose, report a vulnerability, 30 days, etc.
-	//     looking for at least 2 hits
-	if discvuls > 1 {
-		score += 1
-		msg.Text = "Found disclosure, vulnerability, and/or timelines in security policy"
-		dl.Info(&msg)
-	} else {
-		msg.Text = "One or no descriptive hints of disclosure, vulnerability, and/or timelines in security policy"
-		dl.Warn(&msg)
+	if !finding.UniqueProbesEqual(findings, expectedProbes) {
+		e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results")
+		return checker.CreateRuntimeErrorResult(name, e)
 	}
 
-	return score
-}
-
-func countSecInfo(secInfo []checker.SecurityPolicyInformation,
-	infoType checker.SecurityPolicyInformationType,
-	unique bool,
-) int {
-	keys := make(map[string]bool)
-	count := 0
-	for _, entry := range secInfo {
-		if _, present := keys[entry.InformationValue.Match]; !present && entry.InformationType == infoType {
-			keys[entry.InformationValue.Match] = true
-			count += 1
-		} else if !unique && entry.InformationType == infoType {
-			count += 1
+	score := 0
+	m := make(map[string]bool)
+	for i := range findings {
+		f := &findings[i]
+		if f.Outcome == finding.OutcomePositive {
+			switch f.Probe {
+			case securityPolicyContainsVulnerabilityDisclosure.Probe:
+				score += scoreProbeOnce(f.Probe, m, 1)
+			case securityPolicyContainsLinks.Probe:
+				score += scoreProbeOnce(f.Probe, m, 6)
+			case securityPolicyContainsText.Probe:
+				score += scoreProbeOnce(f.Probe, m, 3)
+			case securityPolicyPresent.Probe:
+				m[f.Probe] = true
+			default:
+				e := sce.WithMessage(sce.ErrScorecardInternal, "unknown probe results")
+				return checker.CreateRuntimeErrorResult(name, e)
+			}
 		}
 	}
-	return count
-}
-
-func findSecInfo(secInfo []checker.SecurityPolicyInformation,
-	infoType checker.SecurityPolicyInformationType,
-	unique bool,
-) []checker.SecurityPolicyInformation {
-	keys := make(map[string]bool)
-	var secList []checker.SecurityPolicyInformation
-	for _, entry := range secInfo {
-		if _, present := keys[entry.InformationValue.Match]; !present && entry.InformationType == infoType {
-			keys[entry.InformationValue.Match] = true
-			secList = append(secList, entry)
-		} else if !unique && entry.InformationType == infoType {
-			secList = append(secList, entry)
+	_, defined := m[securityPolicyPresent.Probe]
+	if !defined {
+		if score > 0 {
+			e := sce.WithMessage(sce.ErrScorecardInternal, "score calculation problem")
+			return checker.CreateRuntimeErrorResult(name, e)
 		}
-	}
-	return secList
-}
-
-// SecurityPolicy applies the score policy for the Security-Policy check.
-func SecurityPolicy(name string, dl checker.DetailLogger, r *checker.SecurityPolicyData) checker.CheckResult {
-	if r == nil {
-		e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data")
-		return checker.CreateRuntimeErrorResult(name, e)
-	}
-
-	// Apply the policy evaluation.
-	if len(r.PolicyFiles) == 0 {
-		// If the file is unset, directly return as not detected.
 		return checker.CreateMinScoreResult(name, "security policy file not detected")
 	}
 
-	// TODO: although this a loop, the raw checks will only return one security policy
-	// when more than one security policy file can be aggregated into a composite
-	// score, that logic can be comprehended here.
-	score := 0
-	for _, spd := range r.PolicyFiles {
-		score = scoreSecurityCriteria(spd.File,
-			spd.Information, dl)
-
-		msg := checker.LogMessage{
-			Path: spd.File.Path,
-			Type: spd.File.Type,
-		}
-		if msg.Type == finding.FileTypeURL {
-			msg.Text = "security policy detected in org repo"
-		} else {
-			msg.Text = "security policy detected in current repo"
-		}
+	return checker.CreateResultWithScore(name, "security policy file detected", score)
+}
 
-		dl.Info(&msg)
+func scoreProbeOnce(probeID string, m map[string]bool, bump int) int {
+	if _, exists := m[probeID]; !exists {
+		m[probeID] = true
+		return bump
 	}
-
-	return checker.CreateResultWithScore(name, "security policy file detected", score)
+	return 0
 }