🌱 Fix pinsDependencies outcomes (#3961)

* switch no dependencies to OutcomeNotApplicable OutcomeNotApplicable is what we've been using for cases where there are no occurences of X. Previously this outcome was used for this probe to handle some error cases, but OutcomeError is currently being used. Existing callers were moved to OutcomeNotSupported. Signed-off-by: Spencer Schrock <sschrock@google.com> * deduplicate location setting checker.File.Location() is nil safe, so this should work when we have a location or not Signed-off-by: Spencer Schrock <sschrock@google.com> * update outcome descriptions Signed-off-by: Spencer Schrock <sschrock@google.com> * simplify OutcomeNotSupported logging path Signed-off-by: Spencer Schrock <sschrock@google.com> * add tests for no deps and processing errors Signed-off-by: Spencer Schrock <sschrock@google.com> --------- Signed-off-by: Spencer Schrock <sschrock@google.com>
ossf · Mar 29, 2024 · 46eea0e · 46eea0e
1 parent 2a45ba6
commit 46eea0e
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 62 deletions.
diff --git a/checks/evaluation/pinned_dependencies.go b/checks/evaluation/pinned_dependencies.go
@@ -81,25 +81,16 @@ func PinningDependencies(name string,
 	for i := range findings {
 		f := findings[i]
 		switch f.Outcome {
-		case finding.OutcomeNotAvailable:
-			return checker.CreateInconclusiveResult(name, "no dependencies found")
 		case finding.OutcomeNotApplicable:
-			if f.Location != nil {
-				dl.Debug(&checker.LogMessage{
-					Path:      f.Location.Path,
-					Type:      f.Location.Type,
-					Offset:    *f.Location.LineStart,
-					EndOffset: *f.Location.LineEnd,
-					Text:      f.Message,
-					Snippet:   *f.Location.Snippet,
-				})
-			} else {
-				dl.Debug(&checker.LogMessage{
-					Text: f.Message,
-				})
-			}
+			return checker.CreateInconclusiveResult(name, "no dependencies found")
+		case finding.OutcomeNotSupported:
+			dl.Debug(&checker.LogMessage{
+				Finding: &f,
+			})
 			continue
 		case finding.OutcomeNegative:
+			// we cant use the finding if we want the remediation to show
+			// finding.Remediation are currently suppressed (#3349)
 			lm := &checker.LogMessage{
 				Path:      f.Location.Path,
 				Type:      f.Location.Type,

diff --git a/checks/evaluation/pinned_dependencies_test.go b/checks/evaluation/pinned_dependencies_test.go
@@ -21,6 +21,7 @@ import (
 
 	"github.com/ossf/scorecard/v4/checker"
 	"github.com/ossf/scorecard/v4/finding"
+	"github.com/ossf/scorecard/v4/probes/pinsDependencies"
 	scut "github.com/ossf/scorecard/v4/utests"
 )
 
@@ -288,7 +289,7 @@ func Test_PinningDependencies(t *testing.T) {
 			findings: []finding.Finding{
 				{
 					Probe:   "pinsDependencies",
-					Outcome: finding.OutcomeNotApplicable,
+					Outcome: finding.OutcomeNotSupported,
 					Location: &finding.Location{
 						Type:      finding.FileTypeText,
 						Path:      "test-file",
@@ -405,6 +406,57 @@ func Test_PinningDependencies(t *testing.T) {
 				NumberOfInfo: 1,
 			},
 		},
+		{
+			name: "no dependencies leads to an inconclusive score",
+			findings: []finding.Finding{
+				{
+					Probe:   pinsDependencies.Probe,
+					Outcome: finding.OutcomeNotApplicable,
+				},
+			},
+			result: scut.TestReturn{
+				Score: checker.InconclusiveResultScore,
+			},
+		},
+		{
+			name: "processing errors are logged as info",
+			findings: []finding.Finding{
+				{
+					Probe:   pinsDependencies.Probe,
+					Outcome: finding.OutcomeError,
+				},
+			},
+			result: scut.TestReturn{
+				Score:        checker.InconclusiveResultScore,
+				NumberOfInfo: 1,
+			},
+		},
+		{
+			name: "processing errors dont affect other dependencies",
+			findings: []finding.Finding{
+				{
+					Probe:   pinsDependencies.Probe,
+					Outcome: finding.OutcomeError,
+				},
+				{
+					Probe:   pinsDependencies.Probe,
+					Outcome: finding.OutcomePositive,
+					Location: &finding.Location{
+						Type:      finding.FileTypeText,
+						Path:      "test-file",
+						LineStart: &testLineStart,
+						Snippet:   &testSnippet,
+					},
+					Values: map[string]string{
+						"dependencyType": string(checker.DependencyUseTypePipCommand),
+					},
+				},
+			},
+			result: scut.TestReturn{
+				Score:        checker.MaxResultScore,
+				NumberOfInfo: 2, // 1 for processing error, 1 for pinned pip ecosystem
+			},
+		},
 	}
 
 	for _, tt := range tests {

diff --git a/probes/pinsDependencies/def.yml b/probes/pinsDependencies/def.yml
@@ -19,9 +19,9 @@ motivation: >
 implementation: >
   The probe works by looking for unpinned dependencies in Dockerfiles, shell scripts, and GitHub workflows which are used during the build and release process of a project. Special considerations for Go modules treat full semantic versions as pinned due to how the Go tool verifies downloaded content against the hashes when anyone first downloaded the module.
 outcome:
-  - For each of the last 5 releases, the probe returns OutcomePositive, if the release has a signature file in the release assets.
-  - For each of the last 5 releases, the probe returns OutcomeNegative, if the release does not have a signature file in the release assets.
-  - If the project has no releases, the probe returns OutcomeNotApplicable.
+  - For supported ecosystem, the probe returns OutcomePositive per pinned dependency.
+  - For supported ecosystem, the probe returns OutcomeNegative per unpinned dependency.
+  - If the project has no supported dependencies, the probe returns OutcomeNotApplicable.
 remediation:
   effort: Medium
   text:

diff --git a/probes/pinsDependencies/impl.go b/probes/pinsDependencies/impl.go
@@ -62,7 +62,8 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 
 	for i := range r.Dependencies {
 		rr := r.Dependencies[i]
-		f, err := finding.NewWith(fs, Probe, "", nil, finding.OutcomeNotApplicable)
+		loc := rr.Location.Location()
+		f, err := finding.NewWith(fs, Probe, "", loc, finding.OutcomeNotSupported)
 		if err != nil {
 			return nil, Probe, fmt.Errorf("create finding: %w", err)
 		}
@@ -71,46 +72,23 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 				e := sce.WithMessage(sce.ErrScorecardInternal, "empty File field")
 				return findings, Probe, e
 			}
-			f = f.WithMessage(*rr.Msg).WithOutcome(finding.OutcomeNotApplicable)
+			f = f.WithMessage(*rr.Msg).WithOutcome(finding.OutcomeNotSupported)
 			findings = append(findings, *f)
 			continue
 		}
 		if rr.Msg != nil {
-			loc := &finding.Location{
-				Type:      rr.Location.Type,
-				Path:      rr.Location.Path,
-				LineStart: &rr.Location.Offset,
-				LineEnd:   &rr.Location.EndOffset,
-				Snippet:   &rr.Location.Snippet,
-			}
-			f = f.WithMessage(*rr.Msg).WithLocation(loc).WithOutcome(finding.OutcomeNotApplicable)
+			f = f.WithMessage(*rr.Msg).WithOutcome(finding.OutcomeNotSupported)
 			findings = append(findings, *f)
 			continue
 		}
 		if rr.Pinned == nil {
-			loc := &finding.Location{
-				Type:      rr.Location.Type,
-				Path:      rr.Location.Path,
-				LineStart: &rr.Location.Offset,
-				LineEnd:   &rr.Location.EndOffset,
-				Snippet:   &rr.Location.Snippet,
-			}
 			f = f.WithMessage(fmt.Sprintf("%s has empty Pinned field", rr.Type)).
-				WithLocation(loc).
-				WithOutcome(finding.OutcomeNotApplicable)
+				WithOutcome(finding.OutcomeNotSupported)
 			findings = append(findings, *f)
 			continue
 		}
 		if !*rr.Pinned {
-			loc := &finding.Location{
-				Type:      rr.Location.Type,
-				Path:      rr.Location.Path,
-				LineStart: &rr.Location.Offset,
-				LineEnd:   &rr.Location.EndOffset,
-				Snippet:   &rr.Location.Snippet,
-			}
 			f = f.WithMessage(generateTextUnpinned(&rr)).
-				WithLocation(loc).
 				WithOutcome(finding.OutcomeNegative)
 			if rr.Remediation != nil {
 				f.Remediation = ruleRemToProbeRem(rr.Remediation)
@@ -120,14 +98,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 			})
 			findings = append(findings, *f)
 		} else {
-			loc := &finding.Location{
-				Type:      rr.Location.Type,
-				Path:      rr.Location.Path,
-				LineStart: &rr.Location.Offset,
-				LineEnd:   &rr.Location.EndOffset,
-				Snippet:   &rr.Location.Snippet,
-			}
-			f = f.WithMessage("").WithLocation(loc).WithOutcome(finding.OutcomePositive)
+			f = f.WithMessage("").WithOutcome(finding.OutcomePositive)
 			f = f.WithValues(map[string]string{
 				DepTypeKey: string(rr.Type),
 			})
@@ -136,9 +107,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 	}
 
 	if len(findings) == 0 {
-		f, err := finding.NewWith(fs, Probe,
-			"no dependencies found", nil,
-			finding.OutcomeNotAvailable)
+		f, err := finding.NewWith(fs, Probe, "no dependencies found", nil, finding.OutcomeNotApplicable)
 		if err != nil {
 			return nil, Probe, fmt.Errorf("create finding: %w", err)
 		}

diff --git a/probes/pinsDependencies/impl_test.go b/probes/pinsDependencies/impl_test.go
@@ -206,7 +206,7 @@ func Test_Run(t *testing.T) {
 				},
 			},
 			outcomes: []finding.Outcome{
-				finding.OutcomeNotAvailable,
+				finding.OutcomeNotApplicable,
 			},
 		},
 		{
@@ -509,7 +509,7 @@ func Test_Run(t *testing.T) {
 				},
 			},
 			outcomes: []finding.Outcome{
-				finding.OutcomeNotApplicable,
+				finding.OutcomeNotSupported,
 			},
 		},
 		{
@@ -527,7 +527,7 @@ func Test_Run(t *testing.T) {
 				},
 			},
 			outcomes: []finding.Outcome{
-				finding.OutcomeNotApplicable,
+				finding.OutcomeNotSupported,
 			},
 		},
 		{
@@ -545,7 +545,7 @@ func Test_Run(t *testing.T) {
 				},
 			},
 			outcomes: []finding.Outcome{
-				finding.OutcomeNotApplicable,
+				finding.OutcomeNotSupported,
 			},
 		},
 		{