🌱 convert packaging check to probe

Signed-off-by: AdamKorcz <[email protected]>
ossf · Sep 18, 2023 · d1be750 · d1be750
1 parent 4a0e3ff
commit d1be750
Show file tree

Hide file tree

Showing 7 changed files with 266 additions and 185 deletions.
diff --git a/checks/evaluation/packaging.go b/checks/evaluation/packaging.go
@@ -15,75 +15,49 @@
 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/packagedWithGithubActions"
 )
 
 // Packaging applies the score policy for the Packaging check.
-func Packaging(name string, dl checker.DetailLogger, r *checker.PackagingData) checker.CheckResult {
-	if r == nil {
-		e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data")
-		return checker.CreateRuntimeErrorResult(name, e)
+func Packaging(name string,
+	findings []finding.Finding,
+	dl checker.DetailLogger,
+) checker.CheckResult {
+	expectedProbes := []string{
+		packagedWithGithubActions.Probe,
 	}
 
-	pass := false
-	for _, p := range r.Packages {
-		if p.Msg != nil {
-			// This is a debug message. Let's just replay the message.
-			dl.Debug(&checker.LogMessage{
-				Text: *p.Msg,
-			})
-			continue
-		}
-
-		// Presence of a single non-debug message means the
-		// check passes.
-		pass = true
-
-		msg, err := createLogMessage(p)
-		if err != nil {
-			return checker.CreateRuntimeErrorResult(name, err)
-		}
-		dl.Info(&msg)
+	err := validateFindings(findings, expectedProbes)
+	if err != nil {
+		return checker.CreateRuntimeErrorResult(name, err)
 	}
 
-	if pass {
-		return checker.CreateMaxScoreResult(name,
-			"publishing workflow detected")
+	// Currently there is only a single packaging probe that returns
+	// a single positive or negative outcome. As such, in this evaluation,
+	// we return max score if the outcome is positive and lowest score if
+	// the outcome is negative.
+	for _, f := range findings {
+		if f.Outcome == finding.OutcomePositive {
+			// Log all findings except the negative ones.
+			checker.LogFindings(nonNegativeFindings(findings), dl)
+			return checker.CreateMaxScoreResult(name, "project is published as package")
+		}
 	}
 
-	dl.Warn(&checker.LogMessage{
-		Text: "no GitHub/GitLab publishing workflow detected",
-	})
-
-	return checker.CreateInconclusiveResult(name,
-		"no published package detected")
+	checker.LogFindings(nonNegativeFindings(findings), dl)
+	return checker.CreateMinScoreResult(name, "project is not published as package")
 }
 
-func createLogMessage(p checker.Package) (checker.LogMessage, error) {
-	var msg checker.LogMessage
-
-	if p.Msg != nil {
-		return msg, sce.WithMessage(sce.ErrScorecardInternal, "Msg should be nil")
-	}
-
-	if p.File == nil {
-		return msg, sce.WithMessage(sce.ErrScorecardInternal, "File field is nil")
+func validateFindings(findings []finding.Finding, expectedProbes []string) error {
+	if !finding.UniqueProbesEqual(findings, expectedProbes) {
+		return sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results")
 	}
 
-	if p.File != nil {
-		msg.Path = p.File.Path
-		msg.Type = p.File.Type
-		msg.Offset = p.File.Offset
+	if len(findings) == 0 {
+		return sce.WithMessage(sce.ErrScorecardInternal, "found 0 findings. Should not happen")
 	}
-
-	if len(p.Runs) == 0 {
-		return msg, sce.WithMessage(sce.ErrScorecardInternal, "no run data")
-	}
-
-	msg.Text = fmt.Sprintf("GitHub/GitLab publishing workflow used in run %s", p.Runs[0].URL)
-
-	return msg, nil
+	return nil
 }
diff --git a/checks/evaluation/packaging_test.go b/checks/evaluation/packaging_test.go
@@ -16,162 +16,79 @@ package evaluation
 import (
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
-
 	"github.com/ossf/scorecard/v4/checker"
+	sce "github.com/ossf/scorecard/v4/errors"
+	"github.com/ossf/scorecard/v4/finding"
 	scut "github.com/ossf/scorecard/v4/utests"
 )
 
-func Test_createLogMessage(t *testing.T) {
-	msg := "msg"
+func TestPackaging(t *testing.T) {
 	t.Parallel()
-	tests := []struct { //nolint:govet
-		name    string
-		args    checker.Package
-		want    checker.LogMessage
-		wantErr bool
+	tests := []struct {
+		name     string
+		findings []finding.Finding
+		result   scut.TestReturn
 	}{
 		{
-			name:    "nil package",
-			args:    checker.Package{},
-			want:    checker.LogMessage{},
-			wantErr: true,
-		},
-		{
-			name: "nil file",
-			args: checker.Package{
-				File: nil,
-			},
-			want:    checker.LogMessage{},
-			wantErr: true,
-		},
-		{
-			name: "msg is not nil",
-			args: checker.Package{
-				File: &checker.File{},
-				Msg:  &msg,
-			},
-			want: checker.LogMessage{
-				Text: "",
-			},
-			wantErr: true,
-		},
-		{
-			name: "file is not nil",
-			args: checker.Package{
-				File: &checker.File{
-					Path: "path",
+			name: "test positive outcome",
+			findings: []finding.Finding{
+				{
+					Probe:   "packagedWithGithubActions",
+					Outcome: finding.OutcomePositive,
 				},
 			},
-			want: checker.LogMessage{
-				Path: "path",
+			result: scut.TestReturn{
+				Score:        checker.MaxResultScore,
+				NumberOfInfo: 1,
 			},
-			wantErr: true,
 		},
 		{
-			name: "runs are not zero",
-			args: checker.Package{
-				File: &checker.File{
-					Path: "path",
-				},
-				Runs: []checker.Run{
-					{},
+			name: "test positive outcome with wrong probes",
+			findings: []finding.Finding{
+				{
+					Probe:   "wrongProbe",
+					Outcome: finding.OutcomePositive,
 				},
 			},
-			want: checker.LogMessage{
-				Text: "GitHub/GitLab publishing workflow used in run ",
-				Path: "path",
-			},
-		},
-	}
-	for _, tt := range tests {
-		tt := tt // Parallel testing
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-			got, err := createLogMessage(tt.args)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("createLogMessage() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if !cmp.Equal(got, tt.want) {
-				t.Errorf("createLogMessage() got = %v, want %v", got, cmp.Diff(got, tt.want))
-			}
-		})
-	}
-}
-
-func TestPackaging(t *testing.T) {
-	t.Parallel()
-	type args struct { //nolint:govet
-		name string
-		dl   checker.DetailLogger
-		r    *checker.PackagingData
-	}
-	tests := []struct {
-		name string
-		args args
-		want checker.CheckResult
-	}{
-		{
-			name: "nil packaging data",
-			args: args{
-				name: "name",
-				dl:   nil,
-				r:    nil,
-			},
-			want: checker.CheckResult{
-				Name:    "name",
-				Version: 2,
-				Score:   -1,
-				Reason:  "internal error: empty raw data",
+			result: scut.TestReturn{
+				Score: -1,
+				Error: sce.ErrScorecardInternal,
 			},
 		},
 		{
-			name: "empty packaging data",
-			args: args{
-				name: "name",
-				dl:   &scut.TestDetailLogger{},
-				r:    &checker.PackagingData{},
+			name: "test negative outcome",
+			findings: []finding.Finding{
+				{
+					Probe:   "packagedWithGithubActions",
+					Outcome: finding.OutcomeNegative,
+				},
 			},
-			want: checker.CheckResult{
-				Name:    "name",
-				Version: 2,
-				Score:   -1,
-				Reason:  "no published package detected",
+			result: scut.TestReturn{
+				Score: checker.MinResultScore,
 			},
 		},
 		{
-			name: "runs are not zero",
-			args: args{
-				dl: &scut.TestDetailLogger{},
-				r: &checker.PackagingData{
-					Packages: []checker.Package{
-						{
-							File: &checker.File{
-								Path: "path",
-							},
-							Runs: []checker.Run{
-								{},
-							},
-						},
-					},
+			name: "test negative outcome with wrong probes",
+			findings: []finding.Finding{
+				{
+					Probe:   "wrongProbe",
+					Outcome: finding.OutcomeNegative,
 				},
 			},
-			want: checker.CheckResult{
-				Name:    "",
-				Version: 2,
-				Score:   10,
-				Reason:  "publishing workflow detected",
+			result: scut.TestReturn{
+				Score: -1,
+				Error: sce.ErrScorecardInternal,
 			},
 		},
 	}
 	for _, tt := range tests {
 		tt := tt // Parallel testing
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			if got := Packaging(tt.args.name, tt.args.dl, tt.args.r); !cmp.Equal(got, tt.want, cmpopts.IgnoreFields(checker.CheckResult{}, "Error")) { //nolint:lll
-				t.Errorf("Packaging() = %v, want %v", got, cmp.Diff(got, tt.want, cmpopts.IgnoreFields(checker.CheckResult{}, "Error"))) //nolint:lll
+			dl := scut.TestDetailLogger{}
+			got := Packaging(tt.name, tt.findings, &dl)
+			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
+				t.Errorf("got %v, expected %v", got, tt.result)
 			}
 		})
 	}

diff --git a/checks/packaging.go b/checks/packaging.go
@@ -22,6 +22,8 @@ import (
 	"github.com/ossf/scorecard/v4/clients/githubrepo"
 	"github.com/ossf/scorecard/v4/clients/gitlabrepo"
 	sce "github.com/ossf/scorecard/v4/errors"
+	"github.com/ossf/scorecard/v4/probes"
+	"github.com/ossf/scorecard/v4/probes/zrunner"
 )
 
 // CheckPackaging is the registered name for Packaging.
@@ -54,10 +56,14 @@ func Packaging(c *checker.CheckRequest) checker.CheckResult {
 		return checker.CreateRuntimeErrorResult(CheckPackaging, e)
 	}
 
-	// Set the raw results.
-	if c.RawResults != nil {
-		c.RawResults.PackagingResults = rawData
+	pRawResults := getRawResults(c)
+	pRawResults.PackagingResults = rawData
+
+	findings, err := zrunner.Run(pRawResults, probes.Packaging)
+	if err != nil {
+		e := sce.WithMessage(sce.ErrScorecardInternal, err.Error())
+		return checker.CreateRuntimeErrorResult(CheckPackaging, e)
 	}
 
-	return evaluation.Packaging(CheckPackaging, c.Dlogger, &rawData)
+	return evaluation.Packaging(CheckPackaging, findings, c.Dlogger)
 }
diff --git a/probes/entries.go b/probes/entries.go
@@ -24,6 +24,7 @@ import (
 	"github.com/ossf/scorecard/v4/probes/fuzzedWithPropertyBasedHaskell"
 	"github.com/ossf/scorecard/v4/probes/fuzzedWithPropertyBasedJavascript"
 	"github.com/ossf/scorecard/v4/probes/fuzzedWithPropertyBasedTypescript"
+	"github.com/ossf/scorecard/v4/probes/packagedWithGithubActions"
 	"github.com/ossf/scorecard/v4/probes/securityPolicyContainsLinks"
 	"github.com/ossf/scorecard/v4/probes/securityPolicyContainsText"
 	"github.com/ossf/scorecard/v4/probes/securityPolicyContainsVulnerabilityDisclosure"
@@ -65,6 +66,9 @@ var (
 		fuzzedWithPropertyBasedTypescript.Run,
 		fuzzedWithPropertyBasedJavascript.Run,
 	}
+	Packaging = []ProbeImpl{
+		packagedWithGithubActions.Run,
+	}
 )
 
 //nolint:gochecknoinits

diff --git a/probes/packagedWithGithubActions/def.yml b/probes/packagedWithGithubActions/def.yml
@@ -0,0 +1,27 @@
+# Copyright 2023 OpenSSF Scorecard Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+id: packagedWithGithubActions
+short: Check that the project is fuzzed using OSS-Fuzz
+motivation: >
+  Packages give users of a project an easy way to download, install, update, and uninstall the software by a package manager. In particular, they make it easy for users to receive security patches as updates.
+implementation: >
+  The implementation checks all the packages of the project. If one of these packages do not have a debug message, then it is considered a release package and the probe returns a positive outcome.
+outcome:
+  - If the project has a package without a debug message, the outcome is positive.
+  - If the project has a package with a debug message, the outcome is negative.
+remediation:
+  effort: Low
+  text:
+    - Use a GitHub action to release your package to language-specific hubs.