🌱 convert vulnerabilities check to probe (#3487)

* 🌱 convert vulnerabilities check to probe Signed-off-by: AdamKorcz <adam@adalogics.com> * rename probe + nits Signed-off-by: AdamKorcz <adam@adalogics.com> * edit def.yml Signed-off-by: AdamKorcz <adam@adalogics.com> * Add vuln ID dynamically to def.yml Signed-off-by: AdamKorcz <adam@adalogics.com> * Elaborate the purpose of test data in unit test Signed-off-by: AdamKorcz <adam@adalogics.com> * Move logging out of loop and change logic of negativeFindings() Signed-off-by: AdamKorcz <adam@adalogics.com> * preserve number of vulns found in output Signed-off-by: AdamKorcz <adam@adalogics.com> * Preserve grouping of vulns Signed-off-by: AdamKorcz <adam@adalogics.com> * fix linter issues Signed-off-by: AdamKorcz <adam@adalogics.com> * Add remediation data Signed-off-by: AdamKorcz <adam@adalogics.com> * use checker.LogFindings() Signed-off-by: AdamKorcz <adam@adalogics.com> --------- Signed-off-by: AdamKorcz <adam@adalogics.com>
2024-09-11 08:55:27 +03:00 · 2023-10-25 18:02:24 +01:00 · 2023-10-25 18:02:24 +01:00 · de022dacc4
commit de022dacc4
parent f2bbd0af62
8 changed files with 368 additions and 62 deletions
--- a/checks/evaluation/finding.go
+++ b/checks/evaluation/finding.go
@ -34,10 +34,9 @@ func negativeFindings(findings []finding.Finding) []finding.Finding {
 	var ff []finding.Finding
 	for i := range findings {
 		f := &findings[i]
-		if f.Outcome == finding.OutcomePositive {
-			continue
+		if f.Outcome == finding.OutcomeNegative {
+			ff = append(ff, *f)
 		}
-		ff = append(ff, *f)
 	}
 	return ff
 }
--- a/checks/evaluation/vulnerabilities.go
+++ b/checks/evaluation/vulnerabilities.go
@ -16,45 +16,37 @@ package evaluation

 import (
 	"fmt"
-	"strings"
-
-	"github.com/google/osv-scanner/pkg/grouper"

 	"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/hasOSVVulnerabilities"
 )

 // Vulnerabilities applies the score policy for the Vulnerabilities check.
-func Vulnerabilities(name string, dl checker.DetailLogger,
-	r *checker.VulnerabilitiesData,
+func Vulnerabilities(name string,
+	findings []finding.Finding,
+	dl checker.DetailLogger,
 ) checker.CheckResult {
-	if r == nil {
-		e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data")
+	expectedProbes := []string{
+		hasOSVVulnerabilities.Probe,
+	}
+
+	if !finding.UniqueProbesEqual(findings, expectedProbes) {
+		e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results")
 		return checker.CreateRuntimeErrorResult(name, e)
 	}

-	aliasVulnerabilities := []grouper.IDAliases{}
-	for _, vuln := range r.Vulnerabilities {
-		aliasVulnerabilities = append(aliasVulnerabilities, grouper.IDAliases(vuln))
-	}
+	vulnsFound := negativeFindings(findings)
+	numVulnsFound := len(vulnsFound)
+	checker.LogFindings(vulnsFound, dl)

-	IDs := grouper.Group(aliasVulnerabilities)
-	score := checker.MaxResultScore - len(IDs)
+	score := checker.MaxResultScore - numVulnsFound

 	if score < checker.MinResultScore {
 		score = checker.MinResultScore
 	}

-	if len(IDs) > 0 {
-		for _, v := range IDs {
-			dl.Warn(&checker.LogMessage{
-				Text: fmt.Sprintf("Project is vulnerable to: %s", strings.Join(v.IDs, " / ")),
-			})
-		}
-
-		return checker.CreateResultWithScore(name,
-			fmt.Sprintf("%v existing vulnerabilities detected", len(IDs)), score)
-	}
-
-	return checker.CreateMaxScoreResult(name, "no vulnerabilities detected")
+	return checker.CreateResultWithScore(name,
+		fmt.Sprintf("%v existing vulnerabilities detected", numVulnsFound), score)
 }
--- a/checks/evaluation/vulnerabilities_test.go
+++ b/checks/evaluation/vulnerabilities_test.go
@ -17,8 +17,8 @@ package evaluation
 import (
 	"testing"

-	"github.com/ossf/scorecard/v4/checker"
-	"github.com/ossf/scorecard/v4/clients"
+	sce "github.com/ossf/scorecard/v4/errors"
+	"github.com/ossf/scorecard/v4/finding"
 	scut "github.com/ossf/scorecard/v4/utests"
 )

@ -26,53 +26,110 @@ import (
 func TestVulnerabilities(t *testing.T) {
 	t.Parallel()
 	//nolint
-	type args struct {
-		name string
-		r    *checker.VulnerabilitiesData
-	}
 	tests := []struct {
 		name     string
-		args     args
-		want     checker.CheckResult
+		findings     []finding.Finding
+		result     scut.TestReturn
 		expected []struct {
 			lineNumber uint
 		}
 	}{
 		{
 			name: "no vulnerabilities",
-			args: args{
-				name: "vulnerabilities_test.go",
-				r: &checker.VulnerabilitiesData{
-					Vulnerabilities: []clients.Vulnerability{},
+			findings: []finding.Finding {
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomePositive,
 				},
 			},
-			want: checker.CheckResult{
+			result: scut.TestReturn{
 				Score: 10,
 			},
 		},
 		{
-			name: "one vulnerability",
-			args: args{
-				name: "vulnerabilities_test.go",
-				r: &checker.VulnerabilitiesData{
-					Vulnerabilities: []clients.Vulnerability{
-						{
-							ID: "CVE-2019-1234",
-						},
-					},
+			name: "three vulnerabilities",
+			findings: []finding.Finding {
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
 				},
 			},
-			want: checker.CheckResult{
-				Score: 9,
+			result: scut.TestReturn{
+				Score: 7,
+				NumberOfWarn: 3,
 			},
 		},
 		{
-			name: "one vulnerability",
-			args: args{
-				name: "vulnerabilities_test.go",
+			name: "twelve vulnerabilities to check that score is not less than 0",
+			findings: []finding.Finding {
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "hasOSVVulnerabilities",
+					Outcome: finding.OutcomeNegative,
+				},
 			},
-			want: checker.CheckResult{
+			result: scut.TestReturn{
+				Score: 0,
+				NumberOfWarn: 12,
+			},
+		},
+		{
+			name: "invalid findings",
+			findings: []finding.Finding {},
+			result: scut.TestReturn{
 				Score: -1,
+				Error: sce.ErrScorecardInternal,
 			},
 		},
 	}
@ -81,9 +138,9 @@ func TestVulnerabilities(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 			dl := scut.TestDetailLogger{}
-			res := Vulnerabilities(tt.args.name, &dl, tt.args.r)
-			if res.Score != tt.want.Score {
-				t.Errorf("Vulnerabilities() = %v, want %v", res.Score, tt.want.Score)
+			got := Vulnerabilities(tt.name, tt.findings, &dl)
+			if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) {
+				t.Errorf("got %v, expected %v", got, tt.result)
 			}
 		})
 	}
--- a/checks/vulnerabilities.go
+++ b/checks/vulnerabilities.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"
 )

 // CheckVulnerabilities is the registered name for the OSV check.
@ -45,9 +47,15 @@ func Vulnerabilities(c *checker.CheckRequest) checker.CheckResult {
 	}

 	// Set the raw results.
-	if c.RawResults != nil {
-		c.RawResults.VulnerabilitiesResults = rawData
+	pRawResults := getRawResults(c)
+	pRawResults.VulnerabilitiesResults = rawData
+
+	// Evaluate the probes.
+	findings, err := zrunner.Run(pRawResults, probes.Vulnerabilities)
+	if err != nil {
+		e := sce.WithMessage(sce.ErrScorecardInternal, err.Error())
+		return checker.CreateRuntimeErrorResult(CheckVulnerabilities, e)
 	}

-	return evaluation.Vulnerabilities(CheckVulnerabilities, c.Dlogger, &rawData)
+	return evaluation.Vulnerabilities(CheckVulnerabilities, findings, c.Dlogger)
 }
--- a/probes/entries.go
+++ b/probes/entries.go
@ -34,6 +34,7 @@ import (
 	"github.com/ossf/scorecard/v4/probes/hasFSFOrOSIApprovedLicense"
 	"github.com/ossf/scorecard/v4/probes/hasLicenseFile"
 	"github.com/ossf/scorecard/v4/probes/hasLicenseFileAtTopDir"
+	"github.com/ossf/scorecard/v4/probes/hasOSVVulnerabilities"
 	"github.com/ossf/scorecard/v4/probes/packagedWithAutomatedWorkflow"
 	"github.com/ossf/scorecard/v4/probes/securityPolicyContainsLinks"
 	"github.com/ossf/scorecard/v4/probes/securityPolicyContainsText"
@ -91,6 +92,9 @@ var (
 	Contributors = []ProbeImpl{
 		contributorsFromOrgOrCompany.Run,
 	}
+	Vulnerabilities = []ProbeImpl{
+		hasOSVVulnerabilities.Run,
+	}
 )

 //nolint:gochecknoinits
--- a/probes/hasOSVVulnerabilities/def.yml
+++ b/probes/hasOSVVulnerabilities/def.yml
@ -0,0 +1,33 @@
+# 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: hasOSVVulnerabilities
+short: Check whether the project has known vulnerabilities
+motivation: >
+  This check determines whether the project has open, unfixed vulnerabilities in its own codebase or its dependencies using the OSV (Open Source Vulnerabilities) service. An open vulnerability may be exploited by attackers and should be fixed as soon as possible.
+implementation: >
+ The implementation fetches data from OSV.dev about the project which shows whether a given project has known, unfixed vulnerabilities. The implementation uses the number of known, unfixed vulnerabilities to score.
+outcome:
+  - The probe returns one negative outcome for each vulnerability found in OSV.
+  - If there are no known vulnerabilities from the raw results, the probe returns one positive outcome.
+remediation:
+  effort: High
+  text:
+    - Fix the ${{ metadata.osvid }} by following information from https://osv.dev/${{ metadata.osvid }}.
+    - If the vulnerability is in a dependency, update the dependency to a non-vulnerable version. If no update is available, consider whether to remove the dependency.
+    - If you believe the vulnerability does not affect your project, the vulnerability can be ignored. To ignore, create an osv-scanner.toml file next to the dependency manifest (e.g. package-lock.json) and specify the ID to ignore and reason. Details on the structure of osv-scanner.toml can be found on OSV-Scanner repository.
+  markdown:
+    - Fix the ${{ metadata.osvid }} by following information from [OSV](https://osv.dev/${{ metadata.osvid }}).
+    - If the vulnerability is in a dependency, update the dependency to a non-vulnerable version. If no update is available, consider whether to remove the dependency.
+    - If you believe the vulnerability does not affect your project, the vulnerability can be ignored. To ignore, create an osv-scanner.toml ([example](https://github.com/google/osv.dev/blob/eb99b02ec8895fe5b87d1e76675ddad79a15f817/vulnfeeds/osv-scanner.toml)) file next to the dependency manifest (e.g. package-lock.json) and specify the ID to ignore and reason. Details on the structure of osv-scanner.toml can be found on [OSV-Scanner repository](https://github.com/google/osv-scanner#ignore-vulnerabilities-by-id).
--- a/probes/hasOSVVulnerabilities/impl.go
+++ b/probes/hasOSVVulnerabilities/impl.go
@ -0,0 +1,76 @@
+// 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.
+
+// nolint:stylecheck
+package hasOSVVulnerabilities
+
+import (
+	"embed"
+	"fmt"
+	"strings"
+
+	"github.com/google/osv-scanner/pkg/grouper"
+
+	"github.com/ossf/scorecard/v4/checker"
+	"github.com/ossf/scorecard/v4/finding"
+	"github.com/ossf/scorecard/v4/probes/internal/utils/uerror"
+)
+
+//go:embed *.yml
+var fs embed.FS
+
+const Probe = "hasOSVVulnerabilities"
+
+func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
+	if raw == nil {
+		return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil)
+	}
+
+	var findings []finding.Finding
+
+	// if no vulns were found
+	if len(raw.VulnerabilitiesResults.Vulnerabilities) == 0 {
+		f, err := finding.NewWith(fs, Probe,
+			"Project does not contain OSV vulnerabilities", nil,
+			finding.OutcomePositive)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
+		}
+		findings = append(findings, *f)
+		return findings, Probe, nil
+	}
+
+	aliasVulnerabilities := []grouper.IDAliases{}
+	for _, vuln := range raw.VulnerabilitiesResults.Vulnerabilities {
+		aliasVulnerabilities = append(aliasVulnerabilities, grouper.IDAliases(vuln))
+	}
+
+	IDs := grouper.Group(aliasVulnerabilities)
+
+	for _, vuln := range IDs {
+		f, err := finding.NewWith(fs, Probe,
+			"Project contains OSV vulnerabilities", nil,
+			finding.OutcomeNegative)
+		if err != nil {
+			return nil, Probe, fmt.Errorf("create finding: %w", err)
+		}
+		f = f.WithMessage(fmt.Sprintf("Project is vulnerable to: %s",
+			strings.Join(vuln.IDs, " / ")))
+		f = f.WithRemediationMetadata(map[string]string{
+			"osvid": strings.Join(vuln.IDs[:], ","),
+		})
+		findings = append(findings, *f)
+	}
+	return findings, Probe, nil
+}
--- a/probes/hasOSVVulnerabilities/impl_test.go
+++ b/probes/hasOSVVulnerabilities/impl_test.go
@ -0,0 +1,137 @@
+// 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.
+
+// nolint:stylecheck
+package hasOSVVulnerabilities
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+
+	"github.com/ossf/scorecard/v4/checker"
+	"github.com/ossf/scorecard/v4/clients"
+	"github.com/ossf/scorecard/v4/finding"
+	"github.com/ossf/scorecard/v4/finding/probe"
+)
+
+func Test_Run(t *testing.T) {
+	t.Parallel()
+	// nolint:govet
+	tests := []struct {
+		name            string
+		raw             *checker.RawResults
+		outcomes        []finding.Outcome
+		expectedFinding *finding.Finding
+		err             error
+	}{
+		{
+			name: "vulnerabilities present",
+			raw: &checker.RawResults{
+				VulnerabilitiesResults: checker.VulnerabilitiesData{
+					Vulnerabilities: []clients.Vulnerability{
+						{ID: "foo"},
+					},
+				},
+			},
+			outcomes: []finding.Outcome{
+				finding.OutcomeNegative,
+			},
+		},
+		{
+			name: "vulnerabilities not present",
+			raw: &checker.RawResults{
+				VulnerabilitiesResults: checker.VulnerabilitiesData{
+					Vulnerabilities: []clients.Vulnerability{},
+				},
+			},
+			outcomes: []finding.Outcome{
+				finding.OutcomePositive,
+			},
+		},
+		{
+			name: "vulnerabilities not present",
+			raw: &checker.RawResults{
+				VulnerabilitiesResults: checker.VulnerabilitiesData{
+					Vulnerabilities: []clients.Vulnerability{},
+				},
+			},
+			outcomes: []finding.Outcome{
+				finding.OutcomePositive,
+			},
+		},
+		{
+			name: "vulnerabilities and metadata present. 'foo' must appear in the findings remediation text.",
+			raw: &checker.RawResults{
+				VulnerabilitiesResults: checker.VulnerabilitiesData{
+					Vulnerabilities: []clients.Vulnerability{
+						{ID: "foo"},
+					},
+				},
+			},
+			outcomes: []finding.Outcome{
+				finding.OutcomeNegative,
+			},
+			expectedFinding: &finding.Finding{
+				Probe:   "hasOSVVulnerabilities",
+				Message: "Project is vulnerable to: foo",
+				Remediation: &probe.Remediation{
+					//nolint
+					Text: `Fix the foo by following information from https://osv.dev/foo.
+If the vulnerability is in a dependency, update the dependency to a non-vulnerable version. If no update is available, consider whether to remove the dependency.
+If you believe the vulnerability does not affect your project, the vulnerability can be ignored. To ignore, create an osv-scanner.toml file next to the dependency manifest (e.g. package-lock.json) and specify the ID to ignore and reason. Details on the structure of osv-scanner.toml can be found on OSV-Scanner repository.`,
+					//nolint
+					Markdown: `Fix the foo by following information from [OSV](https://osv.dev/foo).
+If the vulnerability is in a dependency, update the dependency to a non-vulnerable version. If no update is available, consider whether to remove the dependency.
+If you believe the vulnerability does not affect your project, the vulnerability can be ignored. To ignore, create an osv-scanner.toml ([example](https://github.com/google/osv.dev/blob/eb99b02ec8895fe5b87d1e76675ddad79a15f817/vulnfeeds/osv-scanner.toml)) file next to the dependency manifest (e.g. package-lock.json) and specify the ID to ignore and reason. Details on the structure of osv-scanner.toml can be found on [OSV-Scanner repository](https://github.com/google/osv-scanner#ignore-vulnerabilities-by-id).`,
+					Effort: 3,
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		tt := tt // Re-initializing variable so it is not changed while executing the closure below
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			findings, s, err := Run(tt.raw)
+			if !cmp.Equal(tt.err, err, cmpopts.EquateErrors()) {
+				t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(tt.err, err, cmpopts.EquateErrors()))
+			}
+			if err != nil {
+				return
+			}
+			if diff := cmp.Diff(Probe, s); diff != "" {
+				t.Errorf("mismatch (-want +got):\n%s", diff)
+			}
+			if diff := cmp.Diff(len(tt.outcomes), len(findings)); diff != "" {
+				t.Errorf("mismatch (-want +got):\n%s", diff)
+			}
+			for i := range tt.outcomes {
+				outcome := &tt.outcomes[i]
+				f := &findings[i]
+				if diff := cmp.Diff(*outcome, f.Outcome); diff != "" {
+					t.Errorf("mismatch (-want +got):\n%s", diff)
+				}
+				if tt.expectedFinding != nil {
+					f := &findings[i]
+					if diff := cmp.Diff(tt.expectedFinding, f); diff != "" {
+						t.Errorf("mismatch (-want +got):\n%s", diff)
+					}
+				}
+			}
+		})
+	}
+}