Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 SAST: add Snyk probe #3689

Merged
merged 13 commits into from
Dec 19, 2023
2 changes: 2 additions & 0 deletions checker/raw_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
24 changes: 21 additions & 3 deletions checks/evaluation/sast.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"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"
)

Expand All @@ -32,14 +33,15 @@
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]
Expand All @@ -48,6 +50,8 @@
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
Expand All @@ -68,6 +72,9 @@
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 {
Expand Down Expand Up @@ -157,11 +164,22 @@
})
return checker.MaxResultScore
case finding.OutcomeNegative:
dl.Warn(&checker.LogMessage{
return checker.MinResultScore
default:
panic("Should not happen")

Check warning on line 169 in checks/evaluation/sast.go

View check run for this annotation

Codecov / codecov/patch

checks/evaluation/sast.go#L168-L169

Added lines #L168 - L169 were not covered by tests
}
}

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

Check warning on line 183 in checks/evaluation/sast.go

View check run for this annotation

Codecov / codecov/patch

checks/evaluation/sast.go#L183

Added line #L183 was not covered by tests
}
}
51 changes: 49 additions & 2 deletions checks/evaluation/sast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ func TestSAST(t *testing.T) {
Probe: "sastToolCodeQLInstalled",
Outcome: finding.OutcomePositive,
},
{
Probe: "sastToolSnykInstalled",
Outcome: finding.OutcomeNegative,
},
{
Probe: "sastToolRunsOnAllCommits",
Outcome: finding.OutcomePositive,
Expand All @@ -56,6 +60,10 @@ func TestSAST(t *testing.T) {
Probe: "sastToolCodeQLInstalled",
Outcome: finding.OutcomePositive,
},
{
Probe: "sastToolSnykInstalled",
Outcome: finding.OutcomeNegative,
},
{
Probe: "sastToolRunsOnAllCommits",
Outcome: finding.OutcomePositive,
Expand All @@ -79,6 +87,7 @@ func TestSAST(t *testing.T) {
result: scut.TestReturn{
Score: 10,
NumberOfInfo: 3,
NumberOfWarn: 0,
},
},
{
Expand All @@ -90,6 +99,10 @@ func TestSAST(t *testing.T) {
Probe: "sastToolCodeQLInstalled",
Outcome: finding.OutcomeNegative,
},
{
Probe: "sastToolSnykInstalled",
Outcome: finding.OutcomeNegative,
},
{
Probe: "sastToolRunsOnAllCommits",
Outcome: finding.OutcomeNotApplicable,
Expand All @@ -109,7 +122,7 @@ func TestSAST(t *testing.T) {
result: scut.TestReturn{
Score: 10,
NumberOfInfo: 1,
NumberOfWarn: 2,
NumberOfWarn: 1,
},
},
{
Expand All @@ -119,6 +132,10 @@ func TestSAST(t *testing.T) {
Probe: "sastToolCodeQLInstalled",
Outcome: finding.OutcomeNegative,
},
{
Probe: "sastToolSnykInstalled",
Outcome: finding.OutcomeNegative,
},
{
Probe: "sastToolRunsOnAllCommits",
Outcome: finding.OutcomeNegative,
Expand All @@ -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
Expand Down
72 changes: 72 additions & 0 deletions checks/raw/sast.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@
}
data.Workflows = append(data.Workflows, sonarWorkflows...)

snykWorkflows, err := getSnykWorkflows(c)
spencerschrock marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return data, err
}

Check warning on line 73 in checks/raw/sast.go

View check run for this annotation

Codecov / codecov/patch

checks/raw/sast.go#L72-L73

Added lines #L72 - L73 were not covered by tests
data.Workflows = append(data.Workflows, snykWorkflows...)
return data, nil
}

Expand Down Expand Up @@ -192,6 +197,73 @@
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
}

Check warning on line 209 in checks/raw/sast.go

View check run for this annotation

Codecov / codecov/patch

checks/raw/sast.go#L208-L209

Added lines #L208 - L209 were not covered by tests
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)
}

Check warning on line 236 in checks/raw/sast.go

View check run for this annotation

Codecov / codecov/patch

checks/raw/sast.go#L234-L236

Added lines #L234 - L236 were not covered by tests

// 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)
}

Check warning on line 243 in checks/raw/sast.go

View check run for this annotation

Codecov / codecov/patch

checks/raw/sast.go#L241-L243

Added lines #L241 - L243 were not covered by tests

workflow, errs := actionlint.Parse(content)
if len(errs) > 0 && workflow == nil {
return false, fileparser.FormatActionlintError(errs)
}

Check warning on line 248 in checks/raw/sast.go

View check run for this annotation

Codecov / codecov/patch

checks/raw/sast.go#L247-L248

Added lines #L247 - L248 were not covered by tests

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
Expand Down
23 changes: 23 additions & 0 deletions checks/raw/sast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions checks/raw/testdata/.github/workflows/github-workflow-snyk.yaml
Original file line number Diff line number Diff line change
@@ -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 }}
2 changes: 2 additions & 0 deletions probes/entries.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -112,6 +113,7 @@ var (
}
SAST = []ProbeImpl{
sastToolCodeQLInstalled.Run,
sastToolSnykInstalled.Run,
sastToolRunsOnAllCommits.Run,
sastToolSonarInstalled.Run,
}
Expand Down
29 changes: 29 additions & 0 deletions probes/sastToolSnykInstalled/def.yml
Original file line number Diff line number Diff line change
@@ -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).
spencerschrock marked this conversation as resolved.
Show resolved Hide resolved
- 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
Loading
Loading