🌱 Use const keys for SAST and Pinned-Dependencies probe Values map (#…

…3767) * use const key for pinned-dependencies value map * use const key for sast value map --------- Signed-off-by: Spencer Schrock <sschrock@google.com>
ossf · Jan 4, 2024 · 55b6b76 · 55b6b76
1 parent 141ac4d
commit 55b6b76
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 31 deletions.
diff --git a/checks/evaluation/pinned_dependencies.go b/checks/evaluation/pinned_dependencies.go
@@ -49,6 +49,9 @@ const (
 	gitHubOwnedActionWeight int = 2
 	thirdPartyActionWeight  int = 8
 	normalWeight            int = gitHubOwnedActionWeight + thirdPartyActionWeight
+
+	// depTypeKey is the Values map key used to fetch the dependency type.
+	depTypeKey = "dependencyType"
 )
 
 var (
@@ -172,7 +175,7 @@ func dependenciesToFindings(r *checker.PinningDependenciesData) ([]finding.Findi
 				f.Remediation = ruleRemToProbeRem(rr.Remediation)
 			}
 			f = f.WithValues(map[string]int{
-				"dependencyType": dependencyTypes[rr.Type],
+				depTypeKey: dependencyTypes[rr.Type],
 			})
 			findings = append(findings, *f)
 		} else {
@@ -189,7 +192,7 @@ func dependenciesToFindings(r *checker.PinningDependenciesData) ([]finding.Findi
 				Location: loc,
 			}
 			f = f.WithValues(map[string]int{
-				"dependencyType": dependencyTypes[rr.Type],
+				depTypeKey: dependencyTypes[rr.Type],
 			})
 			findings = append(findings, *f)
 		}
@@ -256,7 +259,7 @@ func PinningDependencies(name string, c *checker.CheckRequest,
 		default:
 			// ignore
 		}
-		updatePinningResults(intToDepType[f.Values["dependencyType"]],
+		updatePinningResults(intToDepType[f.Values[depTypeKey]],
 			f.Outcome, f.Location.Snippet,
 			&wp, pr)
 	}

diff --git a/checks/evaluation/sast.go b/checks/evaluation/sast.go
@@ -163,9 +163,10 @@ func getSASTScore(f *finding.Finding, dl checker.DetailLogger) int {
 			Text: f.Message,
 		})
 	default:
-		checker.CreateProportionalScore(f.Values["totalPullRequestsAnalyzed"], f.Values["totalPullRequestsMerged"])
 	}
-	return checker.CreateProportionalScore(f.Values["totalPullRequestsAnalyzed"], f.Values["totalPullRequestsMerged"])
+	analyzed := f.Values[sastToolRunsOnAllCommits.AnalyzedPRsKey]
+	total := f.Values[sastToolRunsOnAllCommits.TotalPRsKey]
+	return checker.CreateProportionalScore(analyzed, total)
 }
 
 // getSastToolScore returns positive if the project runs the Sast tool

diff --git a/checks/evaluation/sast_test.go b/checks/evaluation/sast_test.go
@@ -19,6 +19,7 @@ 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/sastToolRunsOnAllCommits"
 	scut "github.com/ossf/scorecard/v4/utests"
 )
 
@@ -44,7 +45,7 @@ func TestSAST(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 				{
-					Probe:   "sastToolRunsOnAllCommits",
+					Probe:   sastToolRunsOnAllCommits.Probe,
 					Outcome: finding.OutcomePositive,
 				},
 			},
@@ -73,11 +74,11 @@ func TestSAST(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 				{
-					Probe:   "sastToolRunsOnAllCommits",
+					Probe:   sastToolRunsOnAllCommits.Probe,
 					Outcome: finding.OutcomePositive,
 					Values: map[string]int{
-						"totalPullRequestsAnalyzed": 1,
-						"totalPullRequestsMerged":   2,
+						sastToolRunsOnAllCommits.AnalyzedPRsKey: 1,
+						sastToolRunsOnAllCommits.TotalPRsKey:    2,
 					},
 				},
 				{
@@ -118,11 +119,11 @@ func TestSAST(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 				{
-					Probe:   "sastToolRunsOnAllCommits",
+					Probe:   sastToolRunsOnAllCommits.Probe,
 					Outcome: finding.OutcomePositive,
 					Values: map[string]int{
-						"totalPullRequestsAnalyzed": 1,
-						"totalPullRequestsMerged":   2,
+						sastToolRunsOnAllCommits.AnalyzedPRsKey: 1,
+						sastToolRunsOnAllCommits.TotalPRsKey:    2,
 					},
 				},
 				{
@@ -158,7 +159,7 @@ func TestSAST(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 				{
-					Probe:   "sastToolRunsOnAllCommits",
+					Probe:   sastToolRunsOnAllCommits.Probe,
 					Outcome: finding.OutcomeNotApplicable,
 				},
 				{
@@ -199,11 +200,11 @@ func TestSAST(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 				{
-					Probe:   "sastToolRunsOnAllCommits",
+					Probe:   sastToolRunsOnAllCommits.Probe,
 					Outcome: finding.OutcomeNegative,
 					Values: map[string]int{
-						"totalPullRequestsAnalyzed": 1,
-						"totalPullRequestsMerged":   3,
+						sastToolRunsOnAllCommits.AnalyzedPRsKey: 1,
+						sastToolRunsOnAllCommits.TotalPRsKey:    3,
 					},
 				},
 				{
@@ -229,11 +230,11 @@ func TestSAST(t *testing.T) {
 					Outcome: finding.OutcomePositive,
 				},
 				{
-					Probe:   "sastToolRunsOnAllCommits",
+					Probe:   sastToolRunsOnAllCommits.Probe,
 					Outcome: finding.OutcomePositive,
 					Values: map[string]int{
-						"totalPullRequestsAnalyzed": 1,
-						"totalPullRequestsMerged":   3,
+						sastToolRunsOnAllCommits.AnalyzedPRsKey: 1,
+						sastToolRunsOnAllCommits.TotalPRsKey:    3,
 					},
 				},
 				{
@@ -267,11 +268,11 @@ func TestSAST(t *testing.T) {
 					Outcome: finding.OutcomeNegative,
 				},
 				{
-					Probe:   "sastToolRunsOnAllCommits",
+					Probe:   sastToolRunsOnAllCommits.Probe,
 					Outcome: finding.OutcomePositive,
 					Values: map[string]int{
-						"totalPullRequestsAnalyzed": 1,
-						"totalPullRequestsMerged":   3,
+						sastToolRunsOnAllCommits.AnalyzedPRsKey: 1,
+						sastToolRunsOnAllCommits.TotalPRsKey:    3,
 					},
 				},
 				{

diff --git a/probes/sastToolRunsOnAllCommits/impl.go b/probes/sastToolRunsOnAllCommits/impl.go
@@ -27,7 +27,13 @@ import (
 //go:embed *.yml
 var fs embed.FS
 
-const Probe = "sastToolRunsOnAllCommits" //#nosec
+const (
+	Probe = "sastToolRunsOnAllCommits"
+	// TotalPRsKey is the Values map key which specifies the total number of PRs being evaluated.
+	TotalPRsKey = "totalPullRequestsMerged"
+	// AnalyzedPRsKey is the Values map key which specifies the number of PRs analyzed by a SAST.
+	AnalyzedPRsKey = "totalPullRequestsAnalyzed"
+)
 
 func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 	if raw == nil {
@@ -59,8 +65,8 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 		return []finding.Finding{*f}, Probe, nil
 	}
 
-	f = f.WithValue("totalPullRequestsAnalyzed", totalPullRequestsAnalyzed)
-	f = f.WithValue("totalPullRequestsMerged", totalPullRequestsMerged)
+	f = f.WithValue(AnalyzedPRsKey, totalPullRequestsAnalyzed)
+	f = f.WithValue(TotalPRsKey, totalPullRequestsMerged)
 
 	if totalPullRequestsAnalyzed == totalPullRequestsMerged {
 		msg := fmt.Sprintf("all commits (%v) are checked with a SAST tool", totalPullRequestsMerged)

diff --git a/probes/sastToolRunsOnAllCommits/impl_test.go b/probes/sastToolRunsOnAllCommits/impl_test.go
@@ -54,11 +54,11 @@ func Test_Run(t *testing.T) {
 			},
 			expectedFindings: []finding.Finding{
 				{
-					Probe:   "sastToolRunsOnAllCommits",
+					Probe:   Probe,
 					Message: "1 commits out of 2 are checked with a SAST tool",
 					Values: map[string]int{
-						"totalPullRequestsAnalyzed": 1,
-						"totalPullRequestsMerged":   2,
+						AnalyzedPRsKey: 1,
+						TotalPRsKey:    2,
 					},
 				},
 			},
@@ -83,12 +83,12 @@ func Test_Run(t *testing.T) {
 			},
 			expectedFindings: []finding.Finding{
 				{
-					Probe:   "sastToolRunsOnAllCommits",
+					Probe:   Probe,
 					Message: "all commits (2) are checked with a SAST tool",
 					Outcome: finding.OutcomePositive,
 					Values: map[string]int{
-						"totalPullRequestsAnalyzed": 2,
-						"totalPullRequestsMerged":   2,
+						AnalyzedPRsKey: 2,
+						TotalPRsKey:    2,
 					},
 				},
 			},