🌱 SAST: add Snyk probe (#3689)

* SAST: add Snyk probe Adds Snyk's GitHub action (https://github.com/snyk/actions) as a probe. Signed-off-by: David Korczynski <[email protected]> * nit Signed-off-by: David Korczynski <[email protected]> * e2e: adjust sast test to additional probe Signed-off-by: David Korczynski <[email protected]> * checks: sast: nit, fix e2e test Signed-off-by: DavidKorczynski <[email protected]> * Add test with positive outcome Signed-off-by: David Korczynski <[email protected]> * fix comment Signed-off-by: David Korczynski <[email protected]> * sast: snyk: add workflow test Signed-off-by: David Korczynski <[email protected]> * address review Signed-off-by: David Korczynski <[email protected]> * sast: adjust snyk to be the same with sonar Signed-off-by: David Korczynski <[email protected]> * provide path to WF file Signed-off-by: David Korczynski <[email protected]> * adjust path for finding Signed-off-by: David Korczynski <[email protected]> * use prefix rather than contains Signed-off-by: David Korczynski <[email protected]> --------- Signed-off-by: David Korczynski <[email protected]> Signed-off-by: DavidKorczynski <[email protected]>
ossf · Dec 19, 2023 · 2ef20f1 · 2ef20f1
1 parent 21bbe80
commit 2ef20f1
Show file tree

Hide file tree

Showing 10 changed files with 383 additions and 5 deletions.
diff --git a/checker/raw_result.go b/checker/raw_result.go
@@ -257,6 +257,8 @@ const (
 	CodeQLWorkflow SASTWorkflowType = "CodeQL"
 	// SonarWorkflow represents a workflow that runs Sonar.
 	SonarWorkflow SASTWorkflowType = "Sonar"
+	// SnykWorkflow represents a workflow that runs Snyk.
+	SnykWorkflow SASTWorkflowType = "Snyk"
 )
 
 // SASTWorkflow represents a SAST workflow.

diff --git a/checks/evaluation/sast.go b/checks/evaluation/sast.go
@@ -20,6 +20,7 @@ import (
 	"github.com/ossf/scorecard/v4/finding"
 	"github.com/ossf/scorecard/v4/probes/sastToolCodeQLInstalled"
 	"github.com/ossf/scorecard/v4/probes/sastToolRunsOnAllCommits"
+	"github.com/ossf/scorecard/v4/probes/sastToolSnykInstalled"
 	"github.com/ossf/scorecard/v4/probes/sastToolSonarInstalled"
 )
 
@@ -32,14 +33,15 @@ func SAST(name string,
 		sastToolCodeQLInstalled.Probe,
 		sastToolRunsOnAllCommits.Probe,
 		sastToolSonarInstalled.Probe,
+		sastToolSnykInstalled.Probe,
 	}
 
 	if !finding.UniqueProbesEqual(findings, expectedProbes) {
 		e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results")
 		return checker.CreateRuntimeErrorResult(name, e)
 	}
 
-	var sastScore, codeQlScore, sonarScore int
+	var sastScore, codeQlScore, snykScore, sonarScore int
 	// Assign sastScore, codeQlScore and sonarScore
 	for i := range findings {
 		f := &findings[i]
@@ -48,6 +50,8 @@ func SAST(name string,
 			sastScore = getSASTScore(f, dl)
 		case sastToolCodeQLInstalled.Probe:
 			codeQlScore = getCodeQLScore(f, dl)
+		case sastToolSnykInstalled.Probe:
+			snykScore = getSnykScore(f, dl)
 		case sastToolSonarInstalled.Probe:
 			if f.Outcome == finding.OutcomePositive {
 				sonarScore = checker.MaxResultScore
@@ -68,6 +72,9 @@ func SAST(name string,
 	if sonarScore == checker.MaxResultScore {
 		return checker.CreateMaxScoreResult(name, "SAST tool detected")
 	}
+	if snykScore == checker.MaxResultScore {
+		return checker.CreateMaxScoreResult(name, "SAST tool detected: Snyk")
+	}
 
 	if sastScore == checker.InconclusiveResultScore &&
 		codeQlScore == checker.InconclusiveResultScore {
@@ -157,11 +164,22 @@ func getCodeQLScore(f *finding.Finding, dl checker.DetailLogger) int {
 		})
 		return checker.MaxResultScore
 	case finding.OutcomeNegative:
-		dl.Warn(&checker.LogMessage{
+		return checker.MinResultScore
+	default:
+		panic("Should not happen")
+	}
+}
+
+func getSnykScore(f *finding.Finding, dl checker.DetailLogger) int {
+	switch f.Outcome {
+	case finding.OutcomePositive:
+		dl.Info(&checker.LogMessage{
 			Text: f.Message,
 		})
+		return checker.MaxResultScore
+	case finding.OutcomeNegative:
 		return checker.MinResultScore
 	default:
-		panic("Should not happen")
+		return checker.InconclusiveResultScore
 	}
 }
diff --git a/checks/evaluation/sast_test.go b/checks/evaluation/sast_test.go
@@ -39,6 +39,10 @@ func TestSAST(t *testing.T) {
 					Probe:   "sastToolCodeQLInstalled",
 					Outcome: finding.OutcomePositive,
 				},
+				{
+					Probe:   "sastToolSnykInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
 				{
 					Probe:   "sastToolRunsOnAllCommits",
 					Outcome: finding.OutcomePositive,
@@ -56,6 +60,10 @@ func TestSAST(t *testing.T) {
 					Probe:   "sastToolCodeQLInstalled",
 					Outcome: finding.OutcomePositive,
 				},
+				{
+					Probe:   "sastToolSnykInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
 				{
 					Probe:   "sastToolRunsOnAllCommits",
 					Outcome: finding.OutcomePositive,
@@ -79,6 +87,7 @@ func TestSAST(t *testing.T) {
 			result: scut.TestReturn{
 				Score:        10,
 				NumberOfInfo: 3,
+				NumberOfWarn: 0,
 			},
 		},
 		{
@@ -90,6 +99,10 @@ func TestSAST(t *testing.T) {
 					Probe:   "sastToolCodeQLInstalled",
 					Outcome: finding.OutcomeNegative,
 				},
+				{
+					Probe:   "sastToolSnykInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
 				{
 					Probe:   "sastToolRunsOnAllCommits",
 					Outcome: finding.OutcomeNotApplicable,
@@ -109,7 +122,7 @@ func TestSAST(t *testing.T) {
 			result: scut.TestReturn{
 				Score:        10,
 				NumberOfInfo: 1,
-				NumberOfWarn: 2,
+				NumberOfWarn: 1,
 			},
 		},
 		{
@@ -119,6 +132,10 @@ func TestSAST(t *testing.T) {
 					Probe:   "sastToolCodeQLInstalled",
 					Outcome: finding.OutcomeNegative,
 				},
+				{
+					Probe:   "sastToolSnykInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
 				{
 					Probe:   "sastToolRunsOnAllCommits",
 					Outcome: finding.OutcomeNegative,
@@ -134,10 +151,40 @@ func TestSAST(t *testing.T) {
 			},
 			result: scut.TestReturn{
 				Score:        3,
-				NumberOfWarn: 2,
+				NumberOfWarn: 1,
 				NumberOfInfo: 0,
 			},
 		},
+		{
+			name: "Snyk is installed, Sonar and CodeQL are not installed",
+			findings: []finding.Finding{
+				{
+					Probe:   "sastToolCodeQLInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
+				{
+					Probe:   "sastToolSnykInstalled",
+					Outcome: finding.OutcomePositive,
+				},
+				{
+					Probe:   "sastToolRunsOnAllCommits",
+					Outcome: finding.OutcomePositive,
+					Values: map[string]int{
+						"totalPullRequestsAnalyzed": 1,
+						"totalPullRequestsMerged":   3,
+					},
+				},
+				{
+					Probe:   "sastToolSonarInstalled",
+					Outcome: finding.OutcomeNegative,
+				},
+			},
+			result: scut.TestReturn{
+				Score:        10,
+				NumberOfWarn: 0,
+				NumberOfInfo: 2,
+			},
+		},
 	}
 	for _, tt := range tests {
 		tt := tt

diff --git a/checks/raw/sast.go b/checks/raw/sast.go
@@ -67,6 +67,11 @@ func SAST(c *checker.CheckRequest) (checker.SASTData, error) {
 	}
 	data.Workflows = append(data.Workflows, sonarWorkflows...)
 
+	snykWorkflows, err := getSnykWorkflows(c)
+	if err != nil {
+		return data, err
+	}
+	data.Workflows = append(data.Workflows, snykWorkflows...)
 	return data, nil
 }
 
@@ -192,6 +197,73 @@ var searchGitHubActionWorkflowCodeQL fileparser.DoWhileTrueOnFileContent = func(
 	return true, nil
 }
 
+func getSnykWorkflows(c *checker.CheckRequest) ([]checker.SASTWorkflow, error) {
+	var workflowPaths []string
+	var sastWorkflows []checker.SASTWorkflow
+	err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{
+		Pattern:       ".github/workflows/*",
+		CaseSensitive: false,
+	}, searchGitHubActionWorkflowSnyk, &workflowPaths)
+	if err != nil {
+		return sastWorkflows, err
+	}
+	for _, path := range workflowPaths {
+		sastWorkflow := checker.SASTWorkflow{
+			File: checker.File{
+				Path:   path,
+				Offset: checker.OffsetDefault,
+				Type:   finding.FileTypeSource,
+			},
+			Type: checker.SnykWorkflow,
+		}
+
+		sastWorkflows = append(sastWorkflows, sastWorkflow)
+	}
+	return sastWorkflows, nil
+}
+
+var searchGitHubActionWorkflowSnyk fileparser.DoWhileTrueOnFileContent = func(path string,
+	content []byte,
+	args ...interface{},
+) (bool, error) {
+	if !fileparser.IsWorkflowFile(path) {
+		return true, nil
+	}
+
+	if len(args) != 1 {
+		return false, fmt.Errorf(
+			"searchSnyk requires exactly 1 arguments: %w", errInvalid)
+	}
+
+	// Verify the type of the data.
+	paths, ok := args[0].(*[]string)
+	if !ok {
+		return false, fmt.Errorf(
+			"searchSnyk expects arg[0] of type *[]string: %w", errInvalid)
+	}
+
+	workflow, errs := actionlint.Parse(content)
+	if len(errs) > 0 && workflow == nil {
+		return false, fileparser.FormatActionlintError(errs)
+	}
+
+	for _, job := range workflow.Jobs {
+		for _, step := range job.Steps {
+			e, ok := step.Exec.(*actionlint.ExecAction)
+			if !ok || e == nil || e.Uses == nil {
+				continue
+			}
+			// Parse out repo / SHA.
+			uses := strings.TrimPrefix(e.Uses.Value, "actions://")
+			action, _, _ := strings.Cut(uses, "@")
+			if strings.HasPrefix(action, "snyk/actions/") {
+				*paths = append(*paths, path)
+			}
+		}
+	}
+	return true, nil
+}
+
 type sonarConfig struct {
 	url  string
 	file checker.File

diff --git a/checks/raw/sast_test.go b/checks/raw/sast_test.go
@@ -126,6 +126,29 @@ func TestSAST(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:  "Has Snyk",
+			files: []string{".github/workflows/github-workflow-snyk.yaml"},
+			commits: []clients.Commit{
+				{
+					AssociatedMergeRequest: clients.PullRequest{
+						Number: 1,
+					},
+				},
+			},
+			expected: checker.SASTData{
+				Workflows: []checker.SASTWorkflow{
+					{
+						Type: checker.SnykWorkflow,
+						File: checker.File{
+							Path:   ".github/workflows/github-workflow-snyk.yaml",
+							Offset: checker.OffsetDefault,
+							Type:   finding.FileTypeSource,
+						},
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		tt := tt // Re-initializing variable so it is not changed while executing the closure below

diff --git a/checks/raw/testdata/.github/workflows/github-workflow-snyk.yaml b/checks/raw/testdata/.github/workflows/github-workflow-snyk.yaml
@@ -0,0 +1,19 @@
+name: Snyk Scan
+
+on: pull_request
+permissions:
+  contents: read
+
+jobs:
+  scan-snyk:
+    runs-on: ubuntu-latest
+    permissions:
+      security-events: write
+    steps:
+      - uses: actions/checkout@master
+      - uses: snyk/actions/setup@master
+      - name:  Run Snyk Scanning
+        run: |
+          snyk test
+        env:
+          SNYK_TOKEN: ${{ secrets.SNYK_TOKEN }}
diff --git a/probes/entries.go b/probes/entries.go
@@ -49,6 +49,7 @@ import (
 	"github.com/ossf/scorecard/v4/probes/releasesHaveProvenance"
 	"github.com/ossf/scorecard/v4/probes/sastToolCodeQLInstalled"
 	"github.com/ossf/scorecard/v4/probes/sastToolRunsOnAllCommits"
+	"github.com/ossf/scorecard/v4/probes/sastToolSnykInstalled"
 	"github.com/ossf/scorecard/v4/probes/sastToolSonarInstalled"
 	"github.com/ossf/scorecard/v4/probes/securityPolicyContainsLinks"
 	"github.com/ossf/scorecard/v4/probes/securityPolicyContainsText"
@@ -112,6 +113,7 @@ var (
 	}
 	SAST = []ProbeImpl{
 		sastToolCodeQLInstalled.Run,
+		sastToolSnykInstalled.Run,
 		sastToolRunsOnAllCommits.Run,
 		sastToolSonarInstalled.Run,
 	}

diff --git a/probes/sastToolSnykInstalled/def.yml b/probes/sastToolSnykInstalled/def.yml
@@ -0,0 +1,29 @@
+# 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: sastToolSnykInstalled
+short: Check that the project uses the Snyk github action
+motivation: >
+  SAST is testing run on source code before the application is run. Using SAST tools can prevent known classes of bugs from being inadvertently introduced in the codebase.
+implementation: >
+  The implementation checks whether the project invokes the snyk/actions action.
+outcome:
+  - If the project uses the snyk/actions/* action, the probe returns one finding with OutcomePositive (1).
+  - If the project does not use the snyk/actions/* action, the probe returns one finding with OutcomeNegative (0).
+remediation:
+  effort: Medium
+  text:
+    - Follow the steps in https://github.com/snyk/actions
+  markdown:
+    - Follow the steps in https://github.com/snyk/actions