From 2ef20f17fb2e64147c83440cd2c769653454015a Mon Sep 17 00:00:00 2001 From: DavidKorczynski Date: Tue, 19 Dec 2023 03:20:47 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=8C=B1=20SAST:=20add=20Snyk=20probe=20(#3?= =?UTF-8?q?689)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * SAST: add Snyk probe Adds Snyk's GitHub action (https://github.com/snyk/actions) as a probe. Signed-off-by: David Korczynski * nit Signed-off-by: David Korczynski * e2e: adjust sast test to additional probe Signed-off-by: David Korczynski * checks: sast: nit, fix e2e test Signed-off-by: DavidKorczynski * Add test with positive outcome Signed-off-by: David Korczynski * fix comment Signed-off-by: David Korczynski * sast: snyk: add workflow test Signed-off-by: David Korczynski * address review Signed-off-by: David Korczynski * sast: adjust snyk to be the same with sonar Signed-off-by: David Korczynski * provide path to WF file Signed-off-by: David Korczynski * adjust path for finding Signed-off-by: David Korczynski * use prefix rather than contains Signed-off-by: David Korczynski --------- Signed-off-by: David Korczynski Signed-off-by: DavidKorczynski --- checker/raw_result.go | 2 + checks/evaluation/sast.go | 24 +++- checks/evaluation/sast_test.go | 51 ++++++++- checks/raw/sast.go | 72 ++++++++++++ checks/raw/sast_test.go | 23 ++++ .../workflows/github-workflow-snyk.yaml | 19 ++++ probes/entries.go | 2 + probes/sastToolSnykInstalled/def.yml | 29 +++++ probes/sastToolSnykInstalled/impl.go | 60 ++++++++++ probes/sastToolSnykInstalled/impl_test.go | 106 ++++++++++++++++++ 10 files changed, 383 insertions(+), 5 deletions(-) create mode 100644 checks/raw/testdata/.github/workflows/github-workflow-snyk.yaml create mode 100644 probes/sastToolSnykInstalled/def.yml create mode 100644 probes/sastToolSnykInstalled/impl.go create mode 100644 probes/sastToolSnykInstalled/impl_test.go diff --git a/checker/raw_result.go b/checker/raw_result.go index 878e00af362..8da8ef07494 100644 --- 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 index 1fbe6007d4b..a5650a72e52 100644 --- 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,6 +33,7 @@ func SAST(name string, sastToolCodeQLInstalled.Probe, sastToolRunsOnAllCommits.Probe, sastToolSonarInstalled.Probe, + sastToolSnykInstalled.Probe, } if !finding.UniqueProbesEqual(findings, expectedProbes) { @@ -39,7 +41,7 @@ func SAST(name string, 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 index a8b2f8f5728..5bd8722c453 100644 --- 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 index 9765ecfed81..08384cd35b1 100644 --- 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 index a1ace653a82..939c84bf85e 100644 --- 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 new file mode 100644 index 00000000000..306486284a6 --- /dev/null +++ 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 index 3ae6dcae2ad..77247eb6c61 100644 --- 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 new file mode 100644 index 00000000000..d56d5503e80 --- /dev/null +++ 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 diff --git a/probes/sastToolSnykInstalled/impl.go b/probes/sastToolSnykInstalled/impl.go new file mode 100644 index 00000000000..dba73332c18 --- /dev/null +++ b/probes/sastToolSnykInstalled/impl.go @@ -0,0 +1,60 @@ +// 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 sastToolSnykInstalled + +import ( + "embed" + "fmt" + + "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 = "sastToolSnykInstalled" + +func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { + if raw == nil { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } + + r := raw.SASTResults + + for _, wf := range r.Workflows { + if wf.Type == checker.SnykWorkflow { + f, err := finding.NewWith(fs, Probe, + "SAST tool installed: Snyk", nil, + finding.OutcomePositive) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + f = f.WithLocation(&finding.Location{ + Path: wf.File.Path, + }) + return []finding.Finding{*f}, Probe, nil + } + } + f, err := finding.NewWith(fs, Probe, + "Snyk tool not installed", nil, + finding.OutcomeNegative) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + return []finding.Finding{*f}, Probe, nil +} diff --git a/probes/sastToolSnykInstalled/impl_test.go b/probes/sastToolSnykInstalled/impl_test.go new file mode 100644 index 00000000000..5ed948246d5 --- /dev/null +++ b/probes/sastToolSnykInstalled/impl_test.go @@ -0,0 +1,106 @@ +// 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 sastToolSnykInstalled + +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/finding" +) + +func Test_Run(t *testing.T) { + t.Parallel() + //nolint:govet + tests := []struct { + name string + raw *checker.RawResults + outcomes []finding.Outcome + err error + }{ + { + name: "snyk present", + err: nil, + raw: &checker.RawResults{ + SASTResults: checker.SASTData{ + Workflows: []checker.SASTWorkflow{ + { + Type: checker.CodeQLWorkflow, + }, + { + Type: checker.SnykWorkflow, + }, + { + Type: checker.SonarWorkflow, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + }, + }, + { + name: "snyk not present", + err: nil, + raw: &checker.RawResults{ + SASTResults: checker.SASTData{ + Workflows: []checker.SASTWorkflow{ + { + Type: checker.SonarWorkflow, + }, + { + Type: checker.CodeQLWorkflow, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + }, + } + 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) + } + } + }) + } +}