From ad153506575fdb192c2c8750c773dba11804db26 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Mon, 20 Nov 2023 23:29:29 +0000 Subject: [PATCH 01/12] SAST: add Snyk probe Adds Snyk's GitHub action (https://github.com/snyk/actions) as a probe. Signed-off-by: David Korczynski --- checker/raw_result.go | 2 + checks/evaluation/sast.go | 30 +++++- checks/evaluation/sast_test.go | 21 ++++- checks/raw/sast.go | 72 +++++++++++++++ probes/entries.go | 2 + probes/sastToolSnykInstalled/def.yml | 29 ++++++ probes/sastToolSnykInstalled/impl.go | 57 ++++++++++++ probes/sastToolSnykInstalled/impl_test.go | 106 ++++++++++++++++++++++ 8 files changed, 316 insertions(+), 3 deletions(-) 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 abac71eecf2..676bd9b1437 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..be0ba810a79 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 @@ -103,6 +107,13 @@ func SAST(name string, } // Sast inconclusive. + if snykScore != checker.InconclusiveResultScore { + if snykScore == checker.MaxResultScore { + return checker.CreateMaxScoreResult(name, "SAST tool detected: Snyk") + } + return checker.CreateMinScoreResult(name, "no SAST tool detected") + } + if codeQlScore != checker.InconclusiveResultScore { if codeQlScore == checker.MaxResultScore { return checker.CreateMaxScoreResult(name, "SAST tool detected: CodeQL") @@ -165,3 +176,20 @@ func getCodeQLScore(f *finding.Finding, dl checker.DetailLogger) int { 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: + dl.Warn(&checker.LogMessage{ + Text: f.Message, + }) + return checker.MinResultScore + default: + panic("Should not happen") + } +} diff --git a/checks/evaluation/sast_test.go b/checks/evaluation/sast_test.go index a8b2f8f5728..1570dfaeb8a 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: 1, }, }, { @@ -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: 3, }, }, { @@ -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,7 +151,7 @@ func TestSAST(t *testing.T) { }, result: scut.TestReturn{ Score: 3, - NumberOfWarn: 2, + NumberOfWarn: 3, NumberOfInfo: 0, }, }, diff --git a/checks/raw/sast.go b/checks/raw/sast.go index 9765ecfed81..fd362c354ea 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 action == "snyk/actions/setup" { + *paths = append(*paths, path) + } + } + } + return true, nil +} + type sonarConfig struct { url string file checker.File diff --git a/probes/entries.go b/probes/entries.go index c83e7338365..28b4d8a7959 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -40,6 +40,7 @@ import ( "github.com/ossf/scorecard/v4/probes/packagedWithAutomatedWorkflow" "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" @@ -102,6 +103,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..ac509806af1 --- /dev/null +++ b/probes/sastToolSnykInstalled/impl.go @@ -0,0 +1,57 @@ +// 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) + } + 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..5aa1df0111f --- /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) + } + } + }) + } +} From ff866c3e11fe22188b00ed412a6a1a1a66acab11 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Tue, 21 Nov 2023 10:56:53 +0000 Subject: [PATCH 02/12] nit Signed-off-by: David Korczynski --- probes/sastToolSnykInstalled/impl.go | 2 +- probes/sastToolSnykInstalled/impl_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/probes/sastToolSnykInstalled/impl.go b/probes/sastToolSnykInstalled/impl.go index ac509806af1..9ca39717981 100644 --- a/probes/sastToolSnykInstalled/impl.go +++ b/probes/sastToolSnykInstalled/impl.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// nolint:stylecheck +//nolint:stylecheck package sastToolSnykInstalled import ( diff --git a/probes/sastToolSnykInstalled/impl_test.go b/probes/sastToolSnykInstalled/impl_test.go index 5aa1df0111f..5ed948246d5 100644 --- a/probes/sastToolSnykInstalled/impl_test.go +++ b/probes/sastToolSnykInstalled/impl_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// nolint:stylecheck +//nolint:stylecheck package sastToolSnykInstalled import ( From 9f8d4c1a1e9fe6154120c0a3dba18998f961d35b Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Tue, 21 Nov 2023 11:05:21 +0000 Subject: [PATCH 03/12] e2e: adjust sast test to additional probe Signed-off-by: David Korczynski --- e2e/sast_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/sast_test.go b/e2e/sast_test.go index 484547f81b1..9c4442436fc 100644 --- a/e2e/sast_test.go +++ b/e2e/sast_test.go @@ -45,7 +45,7 @@ var _ = Describe("E2E TEST:"+checks.CheckSAST, func() { expected := scut.TestReturn{ Error: nil, Score: 10, - NumberOfWarn: 1, + NumberOfWarn: 2, NumberOfInfo: 1, NumberOfDebug: 0, } From 08012f4adf7496bb8cd657a40b72c532cd716de1 Mon Sep 17 00:00:00 2001 From: DavidKorczynski Date: Tue, 12 Dec 2023 11:59:06 +0000 Subject: [PATCH 04/12] checks: sast: nit, fix e2e test Signed-off-by: DavidKorczynski --- checks/evaluation/sast.go | 1 - 1 file changed, 1 deletion(-) diff --git a/checks/evaluation/sast.go b/checks/evaluation/sast.go index be0ba810a79..60d128f5f2e 100644 --- a/checks/evaluation/sast.go +++ b/checks/evaluation/sast.go @@ -111,7 +111,6 @@ func SAST(name string, if snykScore == checker.MaxResultScore { return checker.CreateMaxScoreResult(name, "SAST tool detected: Snyk") } - return checker.CreateMinScoreResult(name, "no SAST tool detected") } if codeQlScore != checker.InconclusiveResultScore { From e870f9fd289768351453e2da38e4f51cfd0b0d46 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Tue, 12 Dec 2023 14:39:25 +0000 Subject: [PATCH 05/12] Add test with positive outcome Signed-off-by: David Korczynski --- checks/evaluation/sast.go | 13 ++++++------- checks/evaluation/sast_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/checks/evaluation/sast.go b/checks/evaluation/sast.go index 60d128f5f2e..255b1c4be42 100644 --- a/checks/evaluation/sast.go +++ b/checks/evaluation/sast.go @@ -79,6 +79,12 @@ func SAST(name string, // retun checker.InconclusiveResultScore. return checker.CreateRuntimeErrorResult(name, sce.ErrScorecardInternal) } + // Sast inconclusive. + if snykScore != checker.InconclusiveResultScore { + if snykScore == checker.MaxResultScore { + return checker.CreateMaxScoreResult(name, "SAST tool detected: Snyk") + } + } // Both scores are conclusive. // We assume the CodeQl config uses a cron and is not enabled as pre-submit. @@ -106,13 +112,6 @@ func SAST(name string, } } - // Sast inconclusive. - if snykScore != checker.InconclusiveResultScore { - if snykScore == checker.MaxResultScore { - return checker.CreateMaxScoreResult(name, "SAST tool detected: Snyk") - } - } - if codeQlScore != checker.InconclusiveResultScore { if codeQlScore == checker.MaxResultScore { return checker.CreateMaxScoreResult(name, "SAST tool detected: CodeQL") diff --git a/checks/evaluation/sast_test.go b/checks/evaluation/sast_test.go index 1570dfaeb8a..16dc25bbee2 100644 --- a/checks/evaluation/sast_test.go +++ b/checks/evaluation/sast_test.go @@ -155,6 +155,36 @@ func TestSAST(t *testing.T) { 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: 1, + NumberOfInfo: 2, + }, + }, } for _, tt := range tests { tt := tt From 7d9efd68337e779a1c2ce36618629a063d769741 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Tue, 12 Dec 2023 14:41:24 +0000 Subject: [PATCH 06/12] fix comment Signed-off-by: David Korczynski --- checks/evaluation/sast.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checks/evaluation/sast.go b/checks/evaluation/sast.go index 255b1c4be42..e03bb81f481 100644 --- a/checks/evaluation/sast.go +++ b/checks/evaluation/sast.go @@ -79,7 +79,6 @@ func SAST(name string, // retun checker.InconclusiveResultScore. return checker.CreateRuntimeErrorResult(name, sce.ErrScorecardInternal) } - // Sast inconclusive. if snykScore != checker.InconclusiveResultScore { if snykScore == checker.MaxResultScore { return checker.CreateMaxScoreResult(name, "SAST tool detected: Snyk") @@ -112,6 +111,7 @@ func SAST(name string, } } + // Sast inconclusive. if codeQlScore != checker.InconclusiveResultScore { if codeQlScore == checker.MaxResultScore { return checker.CreateMaxScoreResult(name, "SAST tool detected: CodeQL") From 8387b487c7f8efc3f541a8f6049e208a9192d9d5 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Tue, 12 Dec 2023 16:53:41 +0000 Subject: [PATCH 07/12] sast: snyk: add workflow test Signed-off-by: David Korczynski --- checks/raw/sast_test.go | 23 +++++++++++++++++++ .../workflows/github-workflow-snyk.yaml | 19 +++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 checks/raw/testdata/.github/workflows/github-workflow-snyk.yaml 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..e3063b46309 --- /dev/null +++ b/checks/raw/testdata/.github/workflows/github-workflow-snyk.yaml @@ -0,0 +1,19 @@ +name: Snyk Scan + +on: pull +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 }} From 3f061768080f088b75e9220b25eef485468d69c6 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Wed, 13 Dec 2023 21:30:17 +0000 Subject: [PATCH 08/12] address review Signed-off-by: David Korczynski --- checks/evaluation/sast.go | 8 +------- checks/evaluation/sast_test.go | 8 ++++---- checks/raw/sast.go | 2 +- .../testdata/.github/workflows/github-workflow-snyk.yaml | 2 +- e2e/sast_test.go | 2 +- 5 files changed, 8 insertions(+), 14 deletions(-) diff --git a/checks/evaluation/sast.go b/checks/evaluation/sast.go index e03bb81f481..ff4ac30c698 100644 --- a/checks/evaluation/sast.go +++ b/checks/evaluation/sast.go @@ -166,9 +166,6 @@ func getCodeQLScore(f *finding.Finding, dl checker.DetailLogger) int { }) return checker.MaxResultScore case finding.OutcomeNegative: - dl.Warn(&checker.LogMessage{ - Text: f.Message, - }) return checker.MinResultScore default: panic("Should not happen") @@ -183,11 +180,8 @@ func getSnykScore(f *finding.Finding, dl checker.DetailLogger) int { }) return checker.MaxResultScore case finding.OutcomeNegative: - dl.Warn(&checker.LogMessage{ - Text: f.Message, - }) 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 16dc25bbee2..5bd8722c453 100644 --- a/checks/evaluation/sast_test.go +++ b/checks/evaluation/sast_test.go @@ -87,7 +87,7 @@ func TestSAST(t *testing.T) { result: scut.TestReturn{ Score: 10, NumberOfInfo: 3, - NumberOfWarn: 1, + NumberOfWarn: 0, }, }, { @@ -122,7 +122,7 @@ func TestSAST(t *testing.T) { result: scut.TestReturn{ Score: 10, NumberOfInfo: 1, - NumberOfWarn: 3, + NumberOfWarn: 1, }, }, { @@ -151,7 +151,7 @@ func TestSAST(t *testing.T) { }, result: scut.TestReturn{ Score: 3, - NumberOfWarn: 3, + NumberOfWarn: 1, NumberOfInfo: 0, }, }, @@ -181,7 +181,7 @@ func TestSAST(t *testing.T) { }, result: scut.TestReturn{ Score: 10, - NumberOfWarn: 1, + NumberOfWarn: 0, NumberOfInfo: 2, }, }, diff --git a/checks/raw/sast.go b/checks/raw/sast.go index fd362c354ea..c31af4046c6 100644 --- a/checks/raw/sast.go +++ b/checks/raw/sast.go @@ -256,7 +256,7 @@ var searchGitHubActionWorkflowSnyk fileparser.DoWhileTrueOnFileContent = func(pa // Parse out repo / SHA. uses := strings.TrimPrefix(e.Uses.Value, "actions://") action, _, _ := strings.Cut(uses, "@") - if action == "snyk/actions/setup" { + if strings.Contains(action, "snyk/actions/") { *paths = append(*paths, path) } } diff --git a/checks/raw/testdata/.github/workflows/github-workflow-snyk.yaml b/checks/raw/testdata/.github/workflows/github-workflow-snyk.yaml index e3063b46309..306486284a6 100644 --- a/checks/raw/testdata/.github/workflows/github-workflow-snyk.yaml +++ b/checks/raw/testdata/.github/workflows/github-workflow-snyk.yaml @@ -1,6 +1,6 @@ name: Snyk Scan -on: pull +on: pull_request permissions: contents: read diff --git a/e2e/sast_test.go b/e2e/sast_test.go index 9c4442436fc..484547f81b1 100644 --- a/e2e/sast_test.go +++ b/e2e/sast_test.go @@ -45,7 +45,7 @@ var _ = Describe("E2E TEST:"+checks.CheckSAST, func() { expected := scut.TestReturn{ Error: nil, Score: 10, - NumberOfWarn: 2, + NumberOfWarn: 1, NumberOfInfo: 1, NumberOfDebug: 0, } From 5789512bed530f85723499b669493cea1722a784 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Wed, 13 Dec 2023 21:38:10 +0000 Subject: [PATCH 09/12] sast: adjust snyk to be the same with sonar Signed-off-by: David Korczynski --- checks/evaluation/sast.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/checks/evaluation/sast.go b/checks/evaluation/sast.go index ff4ac30c698..a5650a72e52 100644 --- a/checks/evaluation/sast.go +++ b/checks/evaluation/sast.go @@ -72,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 { @@ -79,11 +82,6 @@ func SAST(name string, // retun checker.InconclusiveResultScore. return checker.CreateRuntimeErrorResult(name, sce.ErrScorecardInternal) } - if snykScore != checker.InconclusiveResultScore { - if snykScore == checker.MaxResultScore { - return checker.CreateMaxScoreResult(name, "SAST tool detected: Snyk") - } - } // Both scores are conclusive. // We assume the CodeQl config uses a cron and is not enabled as pre-submit. From 15f2dcc72aede91cb9f10633ad54f938fb3a4c11 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Wed, 13 Dec 2023 21:55:37 +0000 Subject: [PATCH 10/12] provide path to WF file Signed-off-by: David Korczynski --- probes/sastToolSnykInstalled/impl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/probes/sastToolSnykInstalled/impl.go b/probes/sastToolSnykInstalled/impl.go index 9ca39717981..cc1af9abd44 100644 --- a/probes/sastToolSnykInstalled/impl.go +++ b/probes/sastToolSnykInstalled/impl.go @@ -39,7 +39,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for _, wf := range r.Workflows { if wf.Type == checker.SnykWorkflow { f, err := finding.NewWith(fs, Probe, - "SAST tool installed: Snyk", nil, + fmt.Sprintf("SAST tool installed: Snyk at location '%v'", wf.File.Path), nil, finding.OutcomePositive) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) From 418779df4b30e9f647ce6385790bfcbd53147342 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Wed, 13 Dec 2023 22:04:33 +0000 Subject: [PATCH 11/12] adjust path for finding Signed-off-by: David Korczynski --- probes/sastToolSnykInstalled/impl.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/probes/sastToolSnykInstalled/impl.go b/probes/sastToolSnykInstalled/impl.go index cc1af9abd44..dba73332c18 100644 --- a/probes/sastToolSnykInstalled/impl.go +++ b/probes/sastToolSnykInstalled/impl.go @@ -39,11 +39,14 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for _, wf := range r.Workflows { if wf.Type == checker.SnykWorkflow { f, err := finding.NewWith(fs, Probe, - fmt.Sprintf("SAST tool installed: Snyk at location '%v'", wf.File.Path), nil, + "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 } } From 026fe3f07f74dfd9efd7c14cc0e2e0f1e9130783 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Mon, 18 Dec 2023 12:45:22 -0800 Subject: [PATCH 12/12] use prefix rather than contains Signed-off-by: David Korczynski --- checks/raw/sast.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checks/raw/sast.go b/checks/raw/sast.go index c31af4046c6..08384cd35b1 100644 --- a/checks/raw/sast.go +++ b/checks/raw/sast.go @@ -256,7 +256,7 @@ var searchGitHubActionWorkflowSnyk fileparser.DoWhileTrueOnFileContent = func(pa // Parse out repo / SHA. uses := strings.TrimPrefix(e.Uses.Value, "actions://") action, _, _ := strings.Cut(uses, "@") - if strings.Contains(action, "snyk/actions/") { + if strings.HasPrefix(action, "snyk/actions/") { *paths = append(*paths, path) } }