From 33a153e8cac757011b81985361f00b6b8c8e9639 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Tue, 10 Oct 2023 11:54:20 +0100 Subject: [PATCH 01/15] Convert SAST checks to probes Signed-off-by: AdamKorcz --- checker/raw_result.go | 35 ++ checks/evaluation/sast.go | 170 +++++++++ checks/evaluation/sast_test.go | 136 +++++++ checks/raw/sast.go | 290 ++++++++++++++ checks/raw/sast_test.go | 161 ++++++++ .../.github/workflows/airflows-codeql.yaml | 109 ++++++ checks/raw/testdata/.github/workflows/pom.xml | 4 + checks/sast.go | 358 +----------------- checks/sast_test.go | 241 +++--------- .../workflows/airflow-codeql-workflow.yaml | 109 ++++++ e2e/sast_test.go | 4 +- finding/finding.go | 10 + probes/entries.go | 10 + probes/sastToolCodeQLInstalled/def.yml | 29 ++ probes/sastToolCodeQLInstalled/impl.go | 57 +++ probes/sastToolCodeQLInstalled/impl_test.go | 100 +++++ probes/sastToolRunsOnAllCommits/def.yml | 30 ++ probes/sastToolRunsOnAllCommits/impl.go | 73 ++++ probes/sastToolRunsOnAllCommits/impl_test.go | 128 +++++++ probes/sastToolSonarInstalled/def.yml | 29 ++ probes/sastToolSonarInstalled/impl.go | 57 +++ probes/sastToolSonarInstalled/impl_test.go | 100 +++++ 22 files changed, 1709 insertions(+), 531 deletions(-) create mode 100644 checks/evaluation/sast.go create mode 100644 checks/evaluation/sast_test.go create mode 100644 checks/raw/sast.go create mode 100644 checks/raw/sast_test.go create mode 100644 checks/raw/testdata/.github/workflows/airflows-codeql.yaml create mode 100644 checks/raw/testdata/.github/workflows/pom.xml create mode 100644 checks/testdata/.github/workflows/airflow-codeql-workflow.yaml create mode 100644 probes/sastToolCodeQLInstalled/def.yml create mode 100644 probes/sastToolCodeQLInstalled/impl.go create mode 100644 probes/sastToolCodeQLInstalled/impl_test.go create mode 100644 probes/sastToolRunsOnAllCommits/def.yml create mode 100644 probes/sastToolRunsOnAllCommits/impl.go create mode 100644 probes/sastToolRunsOnAllCommits/impl_test.go create mode 100644 probes/sastToolSonarInstalled/def.yml create mode 100644 probes/sastToolSonarInstalled/impl.go create mode 100644 probes/sastToolSonarInstalled/impl_test.go diff --git a/checker/raw_result.go b/checker/raw_result.go index 1b5b980f37a..89dc0598a18 100644 --- a/checker/raw_result.go +++ b/checker/raw_result.go @@ -39,6 +39,7 @@ type RawResults struct { Metadata MetadataData PackagingResults PackagingData PinningDependenciesResults PinningDependenciesData + SASTResults SASTData SecurityPolicyResults SecurityPolicyData SignedReleasesResults SignedReleasesData TokenPermissionsResults TokenPermissionsData @@ -226,6 +227,40 @@ type SecurityPolicyFile struct { File File } +// SASTData contains the raw results +// for the SAST check. +type SASTData struct { + Workflows []SASTWorkflow + Commits []SASTCommit + NumWorkflows int +} + +type SASTCommit struct { + CommittedDate time.Time + Message string + SHA string + CheckRuns []clients.CheckRun + AssociatedMergeRequest clients.PullRequest + Committer clients.User + Compliant bool +} + +// SASTWorkflowType represents a type of SAST workflow. +type SASTWorkflowType string + +const ( + // CodeQLWorkflow represents a workflow that runs CodeQL. + CodeQLWorkflow SASTWorkflowType = "CodeQL" + // SonarWorkflow represents a workflow that runs Sonar. + SonarWorkflow SASTWorkflowType = "Sonar" +) + +// SASTWorkflow represents a SAST workflow. +type SASTWorkflow struct { + Type SASTWorkflowType + File File +} + // SecurityPolicyData contains the raw results // for the Security-Policy check. type SecurityPolicyData struct { diff --git a/checks/evaluation/sast.go b/checks/evaluation/sast.go new file mode 100644 index 00000000000..4509325a9ca --- /dev/null +++ b/checks/evaluation/sast.go @@ -0,0 +1,170 @@ +// 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. + +package evaluation + +import ( + "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/sastToolCodeQLInstalled" + "github.com/ossf/scorecard/v4/probes/sastToolRunsOnAllCommits" + "github.com/ossf/scorecard/v4/probes/sastToolSonarInstalled" +) + +// SAST applies the score policy for the SAST check. +func SAST(name string, + findings []finding.Finding, dl checker.DetailLogger, +) checker.CheckResult { + // We have 3 unique probes, each should have a finding. + expectedProbes := []string{ + sastToolCodeQLInstalled.Probe, + sastToolRunsOnAllCommits.Probe, + sastToolSonarInstalled.Probe, + } + + if !finding.UniqueProbesEqual(findings, expectedProbes) { + e := sce.WithMessage(sce.ErrScorecardInternal, "invalid probe results") + return checker.CreateRuntimeErrorResult(name, e) + } + + var sastScore, codeQlScore, sonarScore int + // Assign sastScore, codeQlScore and sonarScore + for i := range findings { + f := &findings[i] + switch f.Probe { + case sastToolRunsOnAllCommits.Probe: + sastScore = getSASTScore(f, dl) + case sastToolCodeQLInstalled.Probe: + codeQlScore = getCodeQLScore(f, dl) + case sastToolSonarInstalled.Probe: + if f.Outcome == finding.OutcomePositive { + sonarScore = checker.MaxResultScore + dl.Info(&checker.LogMessage{ + Text: f.Message, + }) + } else if f.Outcome == finding.OutcomeNegative { + sonarScore = checker.MinResultScore + dl.Warn(&checker.LogMessage{ + Text: f.Message, + }) + } + } + } + + if sonarScore == checker.MaxResultScore { + return checker.CreateMaxScoreResult(name, "SAST tool detected") + } + + if sastScore == checker.InconclusiveResultScore && + codeQlScore == checker.InconclusiveResultScore { + // That can never happen since sastToolInCheckRuns can never + // retun checker.InconclusiveResultScore. + return checker.CreateRuntimeErrorResult(name, sce.ErrScorecardInternal) + } + + // Both scores are conclusive. + // We assume the CodeQl config uses a cron and is not enabled as pre-submit. + // TODO: verify the above comment in code. + // We encourage developers to have sast check run on every pre-submit rather + // than as cron jobs through the score computation below. + // Warning: there is a hidden assumption that *any* sast tool is equally good. + if sastScore != checker.InconclusiveResultScore && + codeQlScore != checker.InconclusiveResultScore { + switch { + case sastScore == checker.MaxResultScore: + return checker.CreateMaxScoreResult(name, "SAST tool is run on all commits") + case codeQlScore == checker.MinResultScore: + return checker.CreateResultWithScore(name, + checker.NormalizeReason("SAST tool is not run on all commits", sastScore), sastScore) + + // codeQl is enabled and sast has 0+ (but not all) PRs checks. + case codeQlScore == checker.MaxResultScore: + const sastWeight = 3 + const codeQlWeight = 7 + score := checker.AggregateScoresWithWeight(map[int]int{sastScore: sastWeight, codeQlScore: codeQlWeight}) + return checker.CreateResultWithScore(name, "SAST tool detected but not run on all commits", score) + default: + return checker.CreateRuntimeErrorResult(name, sce.WithMessage(sce.ErrScorecardInternal, "contact team")) + } + } + + // Sast inconclusive. + if codeQlScore != checker.InconclusiveResultScore { + if codeQlScore == checker.MaxResultScore { + return checker.CreateMaxScoreResult(name, "SAST tool detected") + } + return checker.CreateMinScoreResult(name, "no SAST tool detected") + } + + // CodeQl inconclusive. + if sastScore != checker.InconclusiveResultScore { + if sastScore == checker.MaxResultScore { + return checker.CreateMaxScoreResult(name, "SAST tool is run on all commits") + } + + return checker.CreateResultWithScore(name, + checker.NormalizeReason("SAST tool is not run on all commits", sastScore), sastScore) + } + + // Should never happen. + return checker.CreateRuntimeErrorResult(name, sce.WithMessage(sce.ErrScorecardInternal, "contact team")) +} + +// getSASTScore returns the proportional score of how many commits +// run SAST tools. +func getSASTScore(f *finding.Finding, dl checker.DetailLogger) int { + switch f.Outcome { + case finding.OutcomeNotApplicable: + dl.Warn(&checker.LogMessage{ + Text: "no pull requests merged into dev branch", + }) + return checker.InconclusiveResultScore + case finding.OutcomePositive: + dl.Info(&checker.LogMessage{ + Text: f.Message, + }) + case finding.OutcomeNegative: + dl.Warn(&checker.LogMessage{ + Text: f.Message, + }) + default: + checker.CreateProportionalScore(f.Values["totalTested"], f.Values["totalMerged"]) + } + return checker.CreateProportionalScore(f.Values["totalTested"], f.Values["totalMerged"]) +} + +// getCodeQLScore returns positive the project runs CodeQL and negative +// if it doesn't. +func getCodeQLScore(f *finding.Finding, dl checker.DetailLogger) int { + switch f.Outcome { + case finding.OutcomeNotApplicable: + dl.Warn(&checker.LogMessage{ + Text: "no pull requests merged into dev branch", + }) + return checker.InconclusiveResultScore + 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 new file mode 100644 index 00000000000..4db17a17d9b --- /dev/null +++ b/checks/evaluation/sast_test.go @@ -0,0 +1,136 @@ +// 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. +package evaluation + +import ( + "testing" + + "github.com/ossf/scorecard/v4/checker" + sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/finding" + scut "github.com/ossf/scorecard/v4/utests" +) + +func TestSAST(t *testing.T) { + t.Parallel() + tests := []struct { + name string + findings []finding.Finding + result scut.TestReturn + }{ + { + name: "SAST - Missing a probe", + findings: []finding.Finding{ + { + Probe: "sastToolCodeQLInstalled", + Outcome: finding.OutcomePositive, + }, + { + Probe: "sastToolRunsOnAllCommits", + Outcome: finding.OutcomePositive, + }, + }, + result: scut.TestReturn{ + Score: checker.InconclusiveResultScore, + Error: sce.ErrScorecardInternal, + }, + }, + { + name: "Sonar and CodeCQ is installed", + findings: []finding.Finding{ + { + Probe: "sastToolCodeQLInstalled", + Outcome: finding.OutcomePositive, + }, + { + Probe: "sastToolRunsOnAllCommits", + Outcome: finding.OutcomePositive, + Values: map[string]int{ + "totalTested": 1, + "totalMerged": 2, + }, + }, + { + Probe: "sastToolSonarInstalled", + Outcome: finding.OutcomePositive, + }, + }, + result: scut.TestReturn{ + Score: 10, + NumberOfInfo: 3, + }, + }, + { + name: `Sonar is installed. CodeQL is not installed. + Does not have info about whether SAST runs + on every commit.`, + findings: []finding.Finding{ + { + Probe: "sastToolCodeQLInstalled", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "sastToolRunsOnAllCommits", + Outcome: finding.OutcomeNotApplicable, + }, + { + Probe: "sastToolSonarInstalled", + Outcome: finding.OutcomePositive, + }, + }, + result: scut.TestReturn{ + Score: 10, + NumberOfInfo: 1, + NumberOfWarn: 2, + }, + }, + { + name: "Sonar and CodeQL are not installed", + findings: []finding.Finding{ + { + Probe: "sastToolCodeQLInstalled", + Outcome: finding.OutcomeNegative, + }, + { + Probe: "sastToolRunsOnAllCommits", + Outcome: finding.OutcomeNegative, + Values: map[string]int{ + "totalTested": 1, + "totalMerged": 3, + }, + }, + { + Probe: "sastToolSonarInstalled", + Outcome: finding.OutcomeNegative, + }, + }, + result: scut.TestReturn{ + Score: 3, + NumberOfWarn: 3, + NumberOfInfo: 0, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + dl := scut.TestDetailLogger{} + got := SAST(tt.name, tt.findings, &dl) + if !scut.ValidateTestReturn(t, tt.name, &tt.result, &got, &dl) { + t.Errorf("got %v, expected %v", got, tt.result) + } + }) + } +} diff --git a/checks/raw/sast.go b/checks/raw/sast.go new file mode 100644 index 00000000000..ad1bcd08e4a --- /dev/null +++ b/checks/raw/sast.go @@ -0,0 +1,290 @@ +// 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. + +package raw + +import ( + "bufio" + "bytes" + "errors" + "fmt" + "path" + "regexp" + "strings" + + "github.com/rhysd/actionlint" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/checks/fileparser" + sce "github.com/ossf/scorecard/v4/errors" + "github.com/ossf/scorecard/v4/finding" +) + +const CheckSAST = "SAST" + +var errInvalid = errors.New("invalid") + +var sastTools = map[string]bool{"github-code-scanning": true, "lgtm-com": true, "sonarcloud": true} + +var allowedConclusions = map[string]bool{"success": true, "neutral": true} + +// SAST checks for presence of static analysis tools. +func SAST(c *checker.CheckRequest) (checker.SASTData, error) { + var data checker.SASTData + + commits, err := sastToolInCheckRuns(c) + if err != nil { + return data, err + } + data.Commits = commits + + codeQLWorkflows, err := codeQLInCheckDefinitions(c) + if err != nil { + return data, err + } + + data.Workflows = append(data.Workflows, codeQLWorkflows...) + + sonarWorkflows, err := getSonarWorkflows(c) + if err != nil { + return data, err + } + data.Workflows = append(data.Workflows, sonarWorkflows...) + + return data, nil +} + +func sastToolInCheckRuns(c *checker.CheckRequest) ([]checker.SASTCommit, error) { + var sastCommits []checker.SASTCommit + commits, err := c.RepoClient.ListCommits() + if err != nil { + return sastCommits, + sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("RepoClient.ListCommits: %v", err)) + } + + for i := range commits { + pr := commits[i].AssociatedMergeRequest + // TODO(#575): We ignore associated PRs if Scorecard is being run on a fork + // but the PR was created in the original repo. + if pr.MergedAt.IsZero() { + continue + } + + checked := false + crs, err := c.RepoClient.ListCheckRunsForRef(pr.HeadSHA) + if err != nil { + return sastCommits, + sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("Client.Checks.ListCheckRunsForRef: %v", err)) + } + // Note: crs may be `nil`: in this case + // the loop below will be skipped. + for _, cr := range crs { + if cr.Status != "completed" { + continue + } + if !allowedConclusions[cr.Conclusion] { + continue + } + if sastTools[cr.App.Slug] { + c.Dlogger.Debug(&checker.LogMessage{ + Path: cr.URL, + Type: finding.FileTypeURL, + Text: fmt.Sprintf("tool detected: %v", cr.App.Slug), + }) + checked = true + break + } + } + sastCommit := checker.SASTCommit{ + CommittedDate: commits[i].CommittedDate, + Message: commits[i].Message, + SHA: commits[i].SHA, + AssociatedMergeRequest: commits[i].AssociatedMergeRequest, + Committer: commits[i].Committer, + Compliant: checked, + } + sastCommits = append(sastCommits, sastCommit) + } + return sastCommits, nil +} + +func codeQLInCheckDefinitions(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, + }, searchGitHubActionWorkflowCodeQL, &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.CodeQLWorkflow, + } + + sastWorkflows = append(sastWorkflows, sastWorkflow) + } + return sastWorkflows, nil +} + +// Check file content. +var searchGitHubActionWorkflowCodeQL 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( + "searchGitHubActionWorkflowCodeQL requires exactly 1 arguments: %w", errInvalid) + } + + // Verify the type of the data. + paths, ok := args[0].(*[]string) + if !ok { + return false, fmt.Errorf( + "searchGitHubActionWorkflowCodeQL 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 == "github/codeql-action/analyze" { + *paths = append(*paths, path) + } + } + } + return true, nil +} + +type sonarConfig struct { + url string + file checker.File +} + +func getSonarWorkflows(c *checker.CheckRequest) ([]checker.SASTWorkflow, error) { + var config []sonarConfig + var sastWorkflows []checker.SASTWorkflow + err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ + Pattern: "*", + CaseSensitive: false, + }, validateSonarConfig, &config) + if err != nil { + return sastWorkflows, err + } + for _, result := range config { + sastWorkflow := checker.SASTWorkflow{ + File: checker.File{ + Path: result.file.Path, + Offset: result.file.Offset, + EndOffset: result.file.EndOffset, + Type: result.file.Type, + Snippet: result.url, + }, + Type: checker.SonarWorkflow, + } + + sastWorkflows = append(sastWorkflows, sastWorkflow) + } + return sastWorkflows, nil +} + +// Check file content. +var validateSonarConfig fileparser.DoWhileTrueOnFileContent = func(pathfn string, + content []byte, + args ...interface{}, +) (bool, error) { + if !strings.EqualFold(path.Base(pathfn), "pom.xml") { + return true, nil + } + + if len(args) != 1 { + return false, fmt.Errorf( + "validateSonarConfig requires exactly 1 argument: %w", errInvalid) + } + + // Verify the type of the data. + pdata, ok := args[0].(*[]sonarConfig) + if !ok { + return false, fmt.Errorf( + "validateSonarConfig expects arg[0] of type *[]sonarConfig]: %w", errInvalid) + } + + regex := regexp.MustCompile(`\s*(\S+)\s*<\/sonar\.host\.url>`) + match := regex.FindSubmatch(content) + + if len(match) < 2 { + return true, nil + } + offset, err := findLine(content, []byte("")) + if err != nil { + return false, err + } + + endOffset, err := findLine(content, []byte("")) + if err != nil { + return false, err + } + + *pdata = append(*pdata, sonarConfig{ + url: string(match[1]), + file: checker.File{ + Path: pathfn, + Type: finding.FileTypeSource, + Offset: offset, + EndOffset: endOffset, + }, + }) + + return true, nil +} + +func findLine(content, data []byte) (uint, error) { + r := bytes.NewReader(content) + scanner := bufio.NewScanner(r) + + line := 0 + // https://golang.org/pkg/bufio/#Scanner.Scan + for scanner.Scan() { + line++ + if strings.Contains(scanner.Text(), string(data)) { + return uint(line), nil + } + } + + if err := scanner.Err(); err != nil { + return 0, fmt.Errorf("scanner.Err(): %w", err) + } + + return 0, nil +} diff --git a/checks/raw/sast_test.go b/checks/raw/sast_test.go new file mode 100644 index 00000000000..a1ace653a82 --- /dev/null +++ b/checks/raw/sast_test.go @@ -0,0 +1,161 @@ +// Copyright 2021 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. + +package raw + +import ( + "fmt" + "os" + "testing" + + "github.com/golang/mock/gomock" + "github.com/google/go-cmp/cmp" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/clients" + mockrepo "github.com/ossf/scorecard/v4/clients/mockclients" + "github.com/ossf/scorecard/v4/finding" +) + +func TestSAST(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + files []string + commits []clients.Commit + expected checker.SASTData + }{ + { + name: "has codeql 1", + files: []string{ + ".github/workflows/workflow-not-pinned.yaml", + ".github/workflows/pom.xml", + }, + commits: []clients.Commit{ + { + AssociatedMergeRequest: clients.PullRequest{ + Number: 1, + }, + }, + }, + expected: checker.SASTData{ + Workflows: []checker.SASTWorkflow{ + { + Type: checker.CodeQLWorkflow, + File: checker.File{ + Path: ".github/workflows/workflow-not-pinned.yaml", + Offset: checker.OffsetDefault, + Type: finding.FileTypeSource, + }, + }, + { + Type: checker.SonarWorkflow, + File: checker.File{ + Path: ".github/workflows/pom.xml", + Type: finding.FileTypeSource, + Snippet: "https://sonarqube.private.domain", + Offset: 2, + EndOffset: 2, + }, + }, + }, + }, + }, + { + name: "has codeql 2", + files: []string{".github/workflows/github-workflow-multiple-unpinned-uses.yaml"}, + commits: []clients.Commit{ + { + AssociatedMergeRequest: clients.PullRequest{ + Number: 1, + }, + }, + }, + expected: checker.SASTData{ + Workflows: []checker.SASTWorkflow{ + { + Type: checker.CodeQLWorkflow, + File: checker.File{ + Path: ".github/workflows/github-workflow-multiple-unpinned-uses.yaml", + Offset: checker.OffsetDefault, + Type: finding.FileTypeSource, + }, + }, + }, + }, + }, + { + name: "Does not use CodeQL", + files: []string{".github/workflows/github-workflow-download-lines.yaml"}, + expected: checker.SASTData{ + Workflows: nil, + }, + }, + { + name: "Airflows CodeQL workflow - Has CodeQL", + files: []string{".github/workflows/airflows-codeql.yaml"}, + commits: []clients.Commit{ + { + AssociatedMergeRequest: clients.PullRequest{ + Number: 1, + }, + }, + }, + expected: checker.SASTData{ + Workflows: []checker.SASTWorkflow{ + { + Type: checker.CodeQLWorkflow, + File: checker.File{ + Path: ".github/workflows/airflows-codeql.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 + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockRepoClient := mockrepo.NewMockRepoClient(ctrl) + mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil).AnyTimes() + mockRepoClient.EXPECT().ListCommits().DoAndReturn(func() ([]clients.Commit, error) { + return tt.commits, nil + }) + mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { + // This will read the file and return the content + content, err := os.ReadFile("./testdata/" + file) + if err != nil { + return content, fmt.Errorf("%w", err) + } + return content, nil + }).AnyTimes() + req := checker.CheckRequest{ + RepoClient: mockRepoClient, + } + sastWorkflowsGot, err := SAST(&req) + if err != nil { + t.Error(err) + } + if diff := cmp.Diff(tt.expected, sastWorkflowsGot); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/checks/raw/testdata/.github/workflows/airflows-codeql.yaml b/checks/raw/testdata/.github/workflows/airflows-codeql.yaml new file mode 100644 index 00000000000..e07d4a50e54 --- /dev/null +++ b/checks/raw/testdata/.github/workflows/airflows-codeql.yaml @@ -0,0 +1,109 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. +# +--- +name: "CodeQL" + +on: # yamllint disable-line rule:truthy + push: + branches: [main] + schedule: + - cron: '0 2 * * *' + +permissions: + contents: read +concurrency: + group: codeql-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + selective-checks: + name: Selective checks + runs-on: ubuntu-20.04 + outputs: + needs-python-scans: ${{ steps.selective-checks.outputs.needs-python-scans }} + needs-javascript-scans: ${{ steps.selective-checks.outputs.needs-javascript-scans }} + steps: + - name: Checkout repository + uses: actions/checkout@v2 + with: + fetch-depth: 2 + persist-credentials: false + - name: Selective checks + id: selective-checks + env: + EVENT_NAME: ${{ github.event_name }} + TARGET_COMMIT_SHA: ${{ github.sha }} + run: | + if [[ ${EVENT_NAME} == "pull_request" ]]; then + # Run selective checks + ./scripts/ci/selective_ci_checks.sh "${TARGET_COMMIT_SHA}" + else + # Run all checks + ./scripts/ci/selective_ci_checks.sh + fi + + analyze: + name: Analyze + runs-on: ubuntu-20.04 + needs: [selective-checks] + strategy: + fail-fast: false + matrix: + # Override automatic language detection by changing the below list + # Supported options are ['csharp', 'cpp', 'go', 'java', 'javascript', 'python'] + language: ['python', 'javascript'] + permissions: + actions: read + contents: read + pull-requests: read + security-events: write + steps: + - name: Checkout repository + uses: actions/checkout@v2 + with: + persist-credentials: false + if: | + matrix.language == 'python' && needs.selective-checks.outputs.needs-python-scans == 'true' || + matrix.language == 'javascript' && needs.selective-checks.outputs.needs-javascript-scans == 'true' + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v1 + with: + languages: ${{ matrix.language }} + # If you wish to specify custom queries, you can do so here or in a config file. + # By default, queries listed here will override any specified in a config file. + # Prefix the list here with "+" to use these queries and those in the config file. + # queries: ./path/to/local/query, your-org/your-repo/queries@main + if: | + matrix.language == 'python' && needs.selective-checks.outputs.needs-python-scans == 'true' || + matrix.language == 'javascript' && needs.selective-checks.outputs.needs-javascript-scans == 'true' + + # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). + # If this step fails, then you should remove it and run the build manually (see below) + - name: Autobuild + uses: github/codeql-action/autobuild@v1 + if: | + matrix.language == 'python' && needs.selective-checks.outputs.needs-python-scans == 'true' || + matrix.language == 'javascript' && needs.selective-checks.outputs.needs-javascript-scans == 'true' + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 + if: | + matrix.language == 'python' && needs.selective-checks.outputs.needs-python-scans == 'true' || + matrix.language == 'javascript' && needs.selective-checks.outputs.needs-javascript-scans == 'true' \ No newline at end of file diff --git a/checks/raw/testdata/.github/workflows/pom.xml b/checks/raw/testdata/.github/workflows/pom.xml new file mode 100644 index 00000000000..29a367ec0af --- /dev/null +++ b/checks/raw/testdata/.github/workflows/pom.xml @@ -0,0 +1,4 @@ + target/jacoco-report/jacoco.xml +https://sonarqube.private.domain +${projectKey} +${project.artifactId} \ No newline at end of file diff --git a/checks/sast.go b/checks/sast.go index 57cdd003fe8..2e8ef26614d 100644 --- a/checks/sast.go +++ b/checks/sast.go @@ -15,20 +15,13 @@ package checks import ( - "bufio" - "bytes" "errors" - "fmt" - "path" - "regexp" - "strings" - - "github.com/rhysd/actionlint" - "github.com/ossf/scorecard/v4/checker" - "github.com/ossf/scorecard/v4/checks/fileparser" + "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/finding" + "github.com/ossf/scorecard/v4/probes" + "github.com/ossf/scorecard/v4/probes/zrunner" ) // CheckSAST is the registered name for SAST. @@ -55,342 +48,23 @@ func init() { // SAST runs SAST check. func SAST(c *checker.CheckRequest) checker.CheckResult { - sastScore, nonCompliantPRs, sastErr := sastToolInCheckRuns(c) - if sastErr != nil { - return checker.CreateRuntimeErrorResult(CheckSAST, sastErr) - } - - codeQlScore, codeQlErr := codeQLInCheckDefinitions(c) - if codeQlErr != nil { - return checker.CreateRuntimeErrorResult(CheckSAST, codeQlErr) - } - sonarScore, sonarErr := sonarEnabled(c) - if sonarErr != nil { - return checker.CreateRuntimeErrorResult(CheckSAST, sonarErr) - } - - if sonarScore == checker.MaxResultScore { - return checker.CreateMaxScoreResult(CheckSAST, "SAST tool detected") - } - - // Both results are inconclusive. - // Can never happen. - if sastScore == checker.InconclusiveResultScore && - codeQlScore == checker.InconclusiveResultScore { - // That can never happen since sastToolInCheckRuns can never - // retun checker.InconclusiveResultScore. - return checker.CreateRuntimeErrorResult(CheckSAST, sce.ErrScorecardInternal) - } - - // Both scores are conclusive. - // We assume the CodeQl config uses a cron and is not enabled as pre-submit. - // TODO: verify the above comment in code. - // We encourage developers to have sast check run on every pre-submit rather - // than as cron jobs through the score computation below. - // Warning: there is a hidden assumption that *any* sast tool is equally good. - if sastScore != checker.InconclusiveResultScore && - codeQlScore != checker.InconclusiveResultScore { - switch { - case sastScore == checker.MaxResultScore: - return checker.CreateMaxScoreResult(CheckSAST, "SAST tool is run on all commits") - case codeQlScore == checker.MinResultScore: - return checker.CreateResultWithScore(CheckSAST, - checker.NormalizeReason("SAST tool is not run on all commits", sastScore), sastScore) - - // codeQl is enabled and sast has 0+ (but not all) PRs checks. - case codeQlScore == checker.MaxResultScore: - const sastWeight = 3 - const codeQlWeight = 7 - c.Dlogger.Debug(&checker.LogMessage{ - Text: getNonCompliantPRMessage(nonCompliantPRs), - }) - score := checker.AggregateScoresWithWeight(map[int]int{sastScore: sastWeight, codeQlScore: codeQlWeight}) - return checker.CreateResultWithScore(CheckSAST, "SAST tool detected but not run on all commits", score) - default: - return checker.CreateRuntimeErrorResult(CheckSAST, sce.WithMessage(sce.ErrScorecardInternal, "contact team")) - } - } - - // Sast inconclusive. - if codeQlScore != checker.InconclusiveResultScore { - if codeQlScore == checker.MaxResultScore { - return checker.CreateMaxScoreResult(CheckSAST, "SAST tool detected") - } - return checker.CreateMinScoreResult(CheckSAST, "no SAST tool detected") - } - - // CodeQl inconclusive. - if sastScore != checker.InconclusiveResultScore { - if sastScore == checker.MaxResultScore { - return checker.CreateMaxScoreResult(CheckSAST, "SAST tool is run on all commits") - } - - c.Dlogger.Debug(&checker.LogMessage{ - Text: getNonCompliantPRMessage(nonCompliantPRs), - }) - return checker.CreateResultWithScore(CheckSAST, - checker.NormalizeReason("SAST tool is not run on all commits", sastScore), sastScore) - } - - // Should never happen. - return checker.CreateRuntimeErrorResult(CheckSAST, sce.WithMessage(sce.ErrScorecardInternal, "contact team")) -} - -func sastToolInCheckRuns(c *checker.CheckRequest) (int, map[int]int, error) { - commits, err := c.RepoClient.ListCommits() - if err != nil { - return checker.InconclusiveResultScore, nil, - sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("RepoClient.ListCommits: %v", err)) - } - - totalMerged := 0 - totalTested := 0 - nonCompliantPRs := make(map[int]int) - for i := range commits { - pr := commits[i].AssociatedMergeRequest - // TODO(#575): We ignore associated PRs if Scorecard is being run on a fork - // but the PR was created in the original repo. - if pr.MergedAt.IsZero() { - continue - } - totalMerged++ - checked := false - crs, err := c.RepoClient.ListCheckRunsForRef(pr.HeadSHA) - if err != nil { - return checker.InconclusiveResultScore, nil, - sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("Client.Checks.ListCheckRunsForRef: %v", err)) - } - // Note: crs may be `nil`: in this case - // the loop below will be skipped. - for _, cr := range crs { - if cr.Status != "completed" { - continue - } - if !allowedConclusions[cr.Conclusion] { - continue - } - if sastTools[cr.App.Slug] { - c.Dlogger.Debug(&checker.LogMessage{ - Path: cr.URL, - Type: finding.FileTypeURL, - Text: fmt.Sprintf("tool detected: %v", cr.App.Slug), - }) - totalTested++ - checked = true - break - } - } - if !checked { - nonCompliantPRs[pr.Number] = pr.Number - } - } - if totalMerged == 0 { - c.Dlogger.Warn(&checker.LogMessage{ - Text: "no pull requests merged into dev branch", - }) - return checker.InconclusiveResultScore, nil, nil - } - - if totalTested == totalMerged { - c.Dlogger.Info(&checker.LogMessage{ - Text: fmt.Sprintf("all commits (%v) are checked with a SAST tool", totalMerged), - }) - } else { - c.Dlogger.Warn(&checker.LogMessage{ - Text: fmt.Sprintf("%v commits out of %v are checked with a SAST tool", totalTested, totalMerged), - }) - } - - return checker.CreateProportionalScore(totalTested, totalMerged), nonCompliantPRs, nil -} - -func codeQLInCheckDefinitions(c *checker.CheckRequest) (int, error) { - var workflowPaths []string - err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ - Pattern: ".github/workflows/*", - CaseSensitive: false, - }, searchGitHubActionWorkflowCodeQL, &workflowPaths) - if err != nil { - return checker.InconclusiveResultScore, err - } - - for _, path := range workflowPaths { - c.Dlogger.Debug(&checker.LogMessage{ - Path: path, - Type: finding.FileTypeSource, - Offset: checker.OffsetDefault, - Text: "CodeQL detected", - }) - } - - // TODO: check if it's enabled as cron or presubmit. - // TODO: check which branches it is enabled on. We should find main. - if len(workflowPaths) > 0 { - c.Dlogger.Info(&checker.LogMessage{ - Text: "SAST tool detected: CodeQL", - }) - return checker.MaxResultScore, nil - } - - c.Dlogger.Warn(&checker.LogMessage{ - Text: "CodeQL tool not detected", - }) - return checker.MinResultScore, nil -} - -// Check file content. -var searchGitHubActionWorkflowCodeQL 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( - "searchGitHubActionWorkflowCodeQL requires exactly 1 arguments: %w", errInvalid) - } - - // Verify the type of the data. - paths, ok := args[0].(*[]string) - if !ok { - return false, fmt.Errorf( - "searchGitHubActionWorkflowCodeQL 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 == "github/codeql-action/analyze" { - *paths = append(*paths, path) - } - } - } - return true, nil -} - -type sonarConfig struct { - url string - file checker.File -} - -func sonarEnabled(c *checker.CheckRequest) (int, error) { - var config []sonarConfig - err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{ - Pattern: "*", - CaseSensitive: false, - }, validateSonarConfig, &config) + rawData, err := raw.SAST(c) if err != nil { - return checker.InconclusiveResultScore, err - } - for _, result := range config { - c.Dlogger.Info(&checker.LogMessage{ - Path: result.file.Path, - Type: result.file.Type, - Offset: result.file.Offset, - EndOffset: result.file.EndOffset, - Text: "Sonar configuration detected", - Snippet: result.url, - }) - } - - if len(config) > 0 { - return checker.MaxResultScore, nil - } - - return checker.MinResultScore, nil -} - -// Check file content. -var validateSonarConfig fileparser.DoWhileTrueOnFileContent = func(pathfn string, - content []byte, - args ...interface{}, -) (bool, error) { - if !strings.EqualFold(path.Base(pathfn), "pom.xml") { - return true, nil + e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + return checker.CreateRuntimeErrorResult(CheckSAST, e) } - if len(args) != 1 { - return false, fmt.Errorf( - "validateSonarConfig requires exactly 1 argument: %w", errInvalid) - } - - // Verify the type of the data. - pdata, ok := args[0].(*[]sonarConfig) - if !ok { - return false, fmt.Errorf( - "validateSonarConfig expects arg[0] of type *[]sonarConfig]: %w", errInvalid) - } - - regex := regexp.MustCompile(`\s*(\S+)\s*<\/sonar\.host\.url>`) - match := regex.FindSubmatch(content) - - if len(match) < 2 { - return true, nil - } + // Set the raw results. + pRawResults := getRawResults(c) + pRawResults.SASTResults = rawData - offset, err := findLine(content, []byte("")) + // Evaluate the probes. + findings, err := zrunner.Run(pRawResults, probes.SAST) if err != nil { - return false, err - } - - endOffset, err := findLine(content, []byte("")) - if err != nil { - return false, err - } - - *pdata = append(*pdata, sonarConfig{ - url: string(match[1]), - file: checker.File{ - Path: pathfn, - Type: finding.FileTypeSource, - Offset: offset, - EndOffset: endOffset, - }, - }) - - return true, nil -} - -func findLine(content, data []byte) (uint, error) { - r := bytes.NewReader(content) - scanner := bufio.NewScanner(r) - - line := 0 - // https://golang.org/pkg/bufio/#Scanner.Scan - for scanner.Scan() { - line++ - if strings.Contains(scanner.Text(), string(data)) { - return uint(line), nil - } + e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + return checker.CreateRuntimeErrorResult(CheckSAST, e) } - if err := scanner.Err(); err != nil { - return 0, fmt.Errorf("scanner.Err(): %w", err) - } - - return 0, nil -} - -func getNonCompliantPRMessage(intMap map[int]int) string { - var sb strings.Builder - for _, value := range intMap { - if len(sb.String()) != 0 { - sb.WriteString(", ") - } - sb.WriteString(fmt.Sprintf("%d", value)) - } - return fmt.Sprintf("List of pull requests without CI test: %s", sb.String()) + // Return the score evaluation. + return evaluation.SAST(CheckSAST, findings, c.Dlogger) } diff --git a/checks/sast_test.go b/checks/sast_test.go index c4d13d12596..7bbb4345f75 100644 --- a/checks/sast_test.go +++ b/checks/sast_test.go @@ -37,8 +37,8 @@ func Test_SAST(t *testing.T) { //nolint: govet, goerr113 tests := []struct { name string - commits []clients.Commit err error + commits []clients.Commit searchresult clients.SearchResponse checkRuns []clients.CheckRun searchRequest clients.SearchRequest @@ -124,6 +124,7 @@ func Test_SAST(t *testing.T) { }, }, }, + path: "", expected: checker.CheckResult{ Score: 10, }, @@ -152,133 +153,122 @@ func Test_SAST(t *testing.T) { }, }, { - name: "Failed SAST checker should return success status", + name: "Airflow Workflow has CodeQL but has no check runs.", + err: nil, commits: []clients.Commit{ { AssociatedMergeRequest: clients.PullRequest{ MergedAt: time.Now().Add(time.Hour - 1), }, }, - { - AssociatedMergeRequest: clients.PullRequest{ - MergedAt: time.Now().Add(time.Hour - 10), - }, - }, - { - AssociatedMergeRequest: clients.PullRequest{ - MergedAt: time.Now().Add(time.Hour - 20), - }, - }, - { - AssociatedMergeRequest: clients.PullRequest{ - MergedAt: time.Now().Add(time.Hour - 30), - }, - }, - }, - path: ".github/workflows/github-workflow-sast-codeql.yaml", - checkRuns: []clients.CheckRun{ - { - Status: "completed", - App: clients.CheckRunApp{ - Slug: "lgtm-com", - }, - }, }, + searchresult: clients.SearchResponse{}, + path: ".github/workflows/airflow-codeql-workflow.yaml", expected: checker.CheckResult{ Score: 7, }, }, { - name: "Failed SAST checker with checkRuns not completed", + name: "Airflow Workflow has CodeQL and two check runs.", + err: nil, commits: []clients.Commit{ { AssociatedMergeRequest: clients.PullRequest{ MergedAt: time.Now().Add(time.Hour - 1), }, }, + }, + searchresult: clients.SearchResponse{}, + checkRuns: []clients.CheckRun{ { - AssociatedMergeRequest: clients.PullRequest{ - MergedAt: time.Now().Add(time.Hour - 10), - }, - }, - { - AssociatedMergeRequest: clients.PullRequest{ - MergedAt: time.Now().Add(time.Hour - 20), - }, - }, - { - AssociatedMergeRequest: clients.PullRequest{ - MergedAt: time.Now().Add(time.Hour - 30), + Status: "completed", + Conclusion: "success", + App: clients.CheckRunApp{ + Slug: "lgtm-com", }, }, - }, - path: ".github/workflows/github-workflow-sast-no-codeql.yaml", - checkRuns: []clients.CheckRun{ { + Status: "completed", + Conclusion: "success", App: clients.CheckRunApp{ Slug: "lgtm-com", }, }, }, + path: ".github/workflows/airflow-codeql-workflow.yaml", expected: checker.CheckResult{ - Score: 0, + Score: 10, }, }, { - name: "Failed SAST with PullRequest not merged", + name: `Airflow Workflow has CodeQL and two check runs one of + which has wrong type of conlusion. The other is 'success'`, + err: nil, commits: []clients.Commit{ { AssociatedMergeRequest: clients.PullRequest{ - Number: 1, + MergedAt: time.Now().Add(time.Hour - 1), }, }, }, searchresult: clients.SearchResponse{}, checkRuns: []clients.CheckRun{ { + Status: "completed", + Conclusion: "wrongConclusionValue", + App: clients.CheckRunApp{ + Slug: "lgtm-com", + }, + }, + { + Status: "completed", + Conclusion: "success", App: clients.CheckRunApp{ Slug: "lgtm-com", }, }, }, + path: ".github/workflows/airflow-codeql-workflow.yaml", expected: checker.CheckResult{ - Score: 0, + Score: 10, }, }, { - name: "Merged PullRequest in a different repo", + name: `Airflow Workflow has CodeQL and two commits none of which + ran the workflow.`, + err: nil, commits: []clients.Commit{ { AssociatedMergeRequest: clients.PullRequest{ - MergedAt: time.Now(), - Number: 1, + MergedAt: time.Now().Add(time.Hour - 1), + }, + }, + { + AssociatedMergeRequest: clients.PullRequest{ + MergedAt: time.Now().Add(time.Hour - 2), }, }, }, searchresult: clients.SearchResponse{}, checkRuns: []clients.CheckRun{ { + Status: "notCompletedForTestingOnly", + Conclusion: "notSuccessForTestingOnly", + App: clients.CheckRunApp{ + Slug: "lgtm-com", + }, + }, + { + Status: "notCompletedForTestingOnly", + Conclusion: "notSuccessForTestingOnly", App: clients.CheckRunApp{ Slug: "lgtm-com", }, }, }, + path: ".github/workflows/airflow-codeql-workflow.yaml", expected: checker.CheckResult{ - Score: 0, - }, - }, - { - name: "sonartype config 1 line", - path: "pom-1line.xml", - expected: checker.CheckResult{ - Score: 10, - }, - }, - { - name: "sonartype config 2 lines", - path: "pom-2lines.xml", - expected: checker.CheckResult{ - Score: 10, + Score: 7, }, }, } @@ -333,126 +323,3 @@ func Test_SAST(t *testing.T) { }) } } - -func Test_validateSonarConfig(t *testing.T) { - t.Parallel() - - //nolint: govet - tests := []struct { - name string - path string - offset uint - endOffset uint - url string - score int - }{ - { - name: "sonartype config 1 line", - path: "./testdata/pom-1line.xml", - offset: 2, - endOffset: 2, - url: "https://sonarqube.private.domain", - }, - { - name: "sonartype config 2 lines", - path: "./testdata/pom-2lines.xml", - offset: 2, - endOffset: 4, - url: "https://sonarqube.private.domain", - }, - { - name: "wrong filename", - }, - } - for _, tt := range tests { - tt := tt - - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - var config []sonarConfig - var content []byte - var err error - var path string - if tt.path != "" { - content, err = os.ReadFile(tt.path) - if err != nil { - t.Errorf("ReadFile: %v", err) - } - path = "pom.xml" - } - _, err = validateSonarConfig(path, content, &config) - if err != nil { - t.Errorf("Caught error: %v", err) - } - - if path == "" { - if len(config) != 0 { - t.Errorf("Expected no result, got %d for %v", len(config), tt.name) - } - return - } - if len(config) != 1 { - t.Errorf("Expected 1 result, got %d for %v", len(config), tt.name) - } - - if config[0].file.Offset != tt.offset { - t.Errorf("Expected offset %d, got %d for %v", tt.offset, - config[0].file.Offset, tt.name) - } - - if config[0].file.EndOffset != tt.endOffset { - t.Errorf("Expected offset %d, got %d for %v", tt.endOffset, - config[0].file.EndOffset, tt.name) - } - - if config[0].url != tt.url { - t.Errorf("Expected offset %v, got %v for %v", tt.url, - config[0].url, tt.name) - } - }) - } -} - -func Test_searchGitHubActionWorkflowCodeQL_invalid(t *testing.T) { - t.Parallel() - - //nolint: govet - tests := []struct { - name string - path string - args []any - }{ - { - name: "too few arguments", - path: ".github/workflows/github-workflow-sast-codeql.yaml", - args: []any{}, - }, - { - name: "wrong arguments", - path: ".github/workflows/github-workflow-sast-codeql.yaml", - args: []any{ - &[]int{}, - }, - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - var content []byte - var err error - if tt.path != "" { - content, err = os.ReadFile("./testdata/" + tt.path) - if err != nil { - t.Errorf("ReadFile: %v", err) - } - } - _, err = searchGitHubActionWorkflowCodeQL(tt.path, content, tt.args...) - if err == nil { - t.Errorf("Expected error but err was nil") - } - }) - } -} diff --git a/checks/testdata/.github/workflows/airflow-codeql-workflow.yaml b/checks/testdata/.github/workflows/airflow-codeql-workflow.yaml new file mode 100644 index 00000000000..e07d4a50e54 --- /dev/null +++ b/checks/testdata/.github/workflows/airflow-codeql-workflow.yaml @@ -0,0 +1,109 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. +# +--- +name: "CodeQL" + +on: # yamllint disable-line rule:truthy + push: + branches: [main] + schedule: + - cron: '0 2 * * *' + +permissions: + contents: read +concurrency: + group: codeql-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + selective-checks: + name: Selective checks + runs-on: ubuntu-20.04 + outputs: + needs-python-scans: ${{ steps.selective-checks.outputs.needs-python-scans }} + needs-javascript-scans: ${{ steps.selective-checks.outputs.needs-javascript-scans }} + steps: + - name: Checkout repository + uses: actions/checkout@v2 + with: + fetch-depth: 2 + persist-credentials: false + - name: Selective checks + id: selective-checks + env: + EVENT_NAME: ${{ github.event_name }} + TARGET_COMMIT_SHA: ${{ github.sha }} + run: | + if [[ ${EVENT_NAME} == "pull_request" ]]; then + # Run selective checks + ./scripts/ci/selective_ci_checks.sh "${TARGET_COMMIT_SHA}" + else + # Run all checks + ./scripts/ci/selective_ci_checks.sh + fi + + analyze: + name: Analyze + runs-on: ubuntu-20.04 + needs: [selective-checks] + strategy: + fail-fast: false + matrix: + # Override automatic language detection by changing the below list + # Supported options are ['csharp', 'cpp', 'go', 'java', 'javascript', 'python'] + language: ['python', 'javascript'] + permissions: + actions: read + contents: read + pull-requests: read + security-events: write + steps: + - name: Checkout repository + uses: actions/checkout@v2 + with: + persist-credentials: false + if: | + matrix.language == 'python' && needs.selective-checks.outputs.needs-python-scans == 'true' || + matrix.language == 'javascript' && needs.selective-checks.outputs.needs-javascript-scans == 'true' + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v1 + with: + languages: ${{ matrix.language }} + # If you wish to specify custom queries, you can do so here or in a config file. + # By default, queries listed here will override any specified in a config file. + # Prefix the list here with "+" to use these queries and those in the config file. + # queries: ./path/to/local/query, your-org/your-repo/queries@main + if: | + matrix.language == 'python' && needs.selective-checks.outputs.needs-python-scans == 'true' || + matrix.language == 'javascript' && needs.selective-checks.outputs.needs-javascript-scans == 'true' + + # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). + # If this step fails, then you should remove it and run the build manually (see below) + - name: Autobuild + uses: github/codeql-action/autobuild@v1 + if: | + matrix.language == 'python' && needs.selective-checks.outputs.needs-python-scans == 'true' || + matrix.language == 'javascript' && needs.selective-checks.outputs.needs-javascript-scans == 'true' + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 + if: | + matrix.language == 'python' && needs.selective-checks.outputs.needs-python-scans == 'true' || + matrix.language == 'javascript' && needs.selective-checks.outputs.needs-javascript-scans == 'true' \ No newline at end of file diff --git a/e2e/sast_test.go b/e2e/sast_test.go index 2cd7743b7ee..9c4442436fc 100644 --- a/e2e/sast_test.go +++ b/e2e/sast_test.go @@ -45,9 +45,9 @@ var _ = Describe("E2E TEST:"+checks.CheckSAST, func() { expected := scut.TestReturn{ Error: nil, Score: 10, - NumberOfWarn: 1, + NumberOfWarn: 2, NumberOfInfo: 1, - NumberOfDebug: 1, + NumberOfDebug: 0, } result := checks.SAST(&req) // New version. diff --git a/finding/finding.go b/finding/finding.go index da1e5d0e9be..a296efc1ed8 100644 --- a/finding/finding.go +++ b/finding/finding.go @@ -266,6 +266,16 @@ func (f *Finding) WithRemediationMetadata(values map[string]string) *Finding { return f } +// WithValue adds a value to f.Values. +// No copy is made. +func (f *Finding) WithValue(k string, v int) *Finding { + if f.Values == nil { + f.Values = make(map[string]int) + } + f.Values[k] = v + return f +} + // UnmarshalYAML is a custom unmarshalling function // to transform the string into an enum. func (o *Outcome) UnmarshalYAML(n *yaml.Node) error { diff --git a/probes/entries.go b/probes/entries.go index 671877d018e..f23ee815c75 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -38,6 +38,9 @@ import ( "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/sastToolCodeQLInstalled" + "github.com/ossf/scorecard/v4/probes/sastToolRunsOnAllCommits" + "github.com/ossf/scorecard/v4/probes/sastToolSonarInstalled" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsLinks" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsText" "github.com/ossf/scorecard/v4/probes/securityPolicyContainsVulnerabilityDisclosure" @@ -83,6 +86,7 @@ var ( fuzzedWithPropertyBasedTypescript.Run, fuzzedWithPropertyBasedJavascript.Run, } +<<<<<<< HEAD Packaging = []ProbeImpl{ packagedWithAutomatedWorkflow.Run, } @@ -96,6 +100,12 @@ var ( } Vulnerabilities = []ProbeImpl{ hasOSVVulnerabilities.Run, +======= + SAST = []ProbeImpl{ + sastToolCodeQLInstalled.Run, + sastToolRunsOnAllCommits.Run, + sastToolSonarInstalled.Run, +>>>>>>> Convert SAST checks to probes } DangerousWorkflows = []ProbeImpl{ hasDangerousWorkflowScriptInjection.Run, diff --git a/probes/sastToolCodeQLInstalled/def.yml b/probes/sastToolCodeQLInstalled/def.yml new file mode 100644 index 00000000000..68503317a09 --- /dev/null +++ b/probes/sastToolCodeQLInstalled/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: sastToolCodeQLInstalled +short: Check that the project uses the CodeQL github actions +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 github/codeql-action/analyze workflow. +outcome: + - If the project uses the github/codeql-action/analyze workflow, the probe returns one finding with OutcomePositive (1). + - If the project does not use the github/codeql-action/analyze workflow, the probe returns one finding with OutcomeNegative (0). +remediation: + effort: Medium + text: + - Follow the steps in https://docs.github.com/en/code-security/codeql-cli/getting-started-with-the-codeql-cli/preparing-your-code-for-codeql-analysis to integrate CodeQL for your project. + markdown: + - Follow the steps in https://docs.github.com/en/code-security/codeql-cli/getting-started-with-the-codeql-cli/preparing-your-code-for-codeql-analysis to integrate CodeQL for your project. \ No newline at end of file diff --git a/probes/sastToolCodeQLInstalled/impl.go b/probes/sastToolCodeQLInstalled/impl.go new file mode 100644 index 00000000000..ff80f4f12ce --- /dev/null +++ b/probes/sastToolCodeQLInstalled/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 sastToolCodeQLInstalled + +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 = "sastToolCodeQLInstalled" + +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.CodeQLWorkflow { + f, err := finding.NewWith(fs, Probe, + "SAST tool detected: CodeQL", 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, + "CodeQL tool not detected", 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/sastToolCodeQLInstalled/impl_test.go b/probes/sastToolCodeQLInstalled/impl_test.go new file mode 100644 index 00000000000..9f47126b076 --- /dev/null +++ b/probes/sastToolCodeQLInstalled/impl_test.go @@ -0,0 +1,100 @@ +// 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 sastToolCodeQLInstalled + +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: "codeql present", + err: nil, + raw: &checker.RawResults{ + SASTResults: checker.SASTData{ + Workflows: []checker.SASTWorkflow{ + { + Type: checker.CodeQLWorkflow, + }, + { + Type: checker.SonarWorkflow, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + }, + }, + { + name: "codeql not present", + err: nil, + raw: &checker.RawResults{ + SASTResults: checker.SASTData{ + Workflows: []checker.SASTWorkflow{ + { + Type: checker.SonarWorkflow, + }, + }, + }, + }, + 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) + } + } + }) + } +} diff --git a/probes/sastToolRunsOnAllCommits/def.yml b/probes/sastToolRunsOnAllCommits/def.yml new file mode 100644 index 00000000000..4d492e5c257 --- /dev/null +++ b/probes/sastToolRunsOnAllCommits/def.yml @@ -0,0 +1,30 @@ +# 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: sastToolRunsOnAllCommits +short: Checks that a SAST tool runs on all commits in the projects CI. +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 iterates through the projects commits and checks whether any of the check runs for the commits associated merge request was any of the SAST tools that Scorecard supports. +outcome: + - If the project had no commits merged, the probe returns a finding with OutcomeNotApplicable. + - If any of the check runs of an associated merge request of a commit was for a SAST tool, the probe returns one finding with OutcomePositive (1). In addition, the finding will include to values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. + - If none of the check runs of an associated merge request of a commit was for a SAST tool, the probe returns one finding with OutcomeNegative (0). In addition, the finding will include to values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. +remediation: + effort: Low + text: + - Ensure that your SAST tools run on every commit. + markdown: + - Ensure that your SAST tools run on every commit. \ No newline at end of file diff --git a/probes/sastToolRunsOnAllCommits/impl.go b/probes/sastToolRunsOnAllCommits/impl.go new file mode 100644 index 00000000000..c7e285cc19c --- /dev/null +++ b/probes/sastToolRunsOnAllCommits/impl.go @@ -0,0 +1,73 @@ +// 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 sastToolRunsOnAllCommits + +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 = "sastToolRunsOnAllCommits" //#nosec + +func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { + if raw == nil { + return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) + } + + r := raw.SASTResults + + f, err := finding.NewWith(fs, Probe, + "", nil, + finding.OutcomePositive) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) + } + + totalMerged := len(r.Commits) + totalTested := 0 + + for i := range r.Commits { + wf := &r.Commits[i] + if wf.Compliant { + totalTested++ + } + } + + if totalMerged == 0 { + f = f.WithOutcome(finding.OutcomeNotApplicable) + f = f.WithMessage("no pull requests merged into dev branch") + return []finding.Finding{*f}, Probe, nil + } + + f = f.WithValue("totalTested", totalTested) + f = f.WithValue("totalMerged", totalMerged) + + if totalTested == totalMerged { + f = f.WithOutcome(finding.OutcomePositive) + f = f.WithMessage(fmt.Sprintf("all commits (%v) are checked with a SAST tool", totalMerged)) + } else { + f = f.WithOutcome(finding.OutcomeNegative) + f = f.WithMessage(fmt.Sprintf("%v commits out of %v are checked with a SAST tool", totalTested, totalMerged)) + } + return []finding.Finding{*f}, Probe, nil +} diff --git a/probes/sastToolRunsOnAllCommits/impl_test.go b/probes/sastToolRunsOnAllCommits/impl_test.go new file mode 100644 index 00000000000..f1892d4b672 --- /dev/null +++ b/probes/sastToolRunsOnAllCommits/impl_test.go @@ -0,0 +1,128 @@ +// 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 sastToolRunsOnAllCommits + +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 + expectedFindings []finding.Finding + }{ + { + name: "sonar present", + err: nil, + raw: &checker.RawResults{ + SASTResults: checker.SASTData{ + Commits: []checker.SASTCommit{ + { + Compliant: false, + }, + { + Compliant: true, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomeNegative, + }, + expectedFindings: []finding.Finding{ + { + Probe: "sastToolRunsOnAllCommits", + Message: "1 commits out of 2 are checked with a SAST tool", + Values: map[string]int{ + "totalTested": 1, + "totalMerged": 2, + }, + }, + }, + }, + { + name: "sonar present", + err: nil, + raw: &checker.RawResults{ + SASTResults: checker.SASTData{ + Commits: []checker.SASTCommit{ + { + Compliant: true, + }, + { + Compliant: true, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + }, + expectedFindings: []finding.Finding{ + { + Probe: "sastToolRunsOnAllCommits", + Message: "all commits (2) are checked with a SAST tool", + Outcome: finding.OutcomePositive, + Values: map[string]int{ + "totalTested": 2, + "totalMerged": 2, + }, + }, + }, + }, + } + 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 !cmp.Equal(tt.expectedFindings, findings, cmpopts.EquateErrors()) { + t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(tt.expectedFindings, findings, cmpopts.EquateErrors())) + } + }) + } +} diff --git a/probes/sastToolSonarInstalled/def.yml b/probes/sastToolSonarInstalled/def.yml new file mode 100644 index 00000000000..7bbc5e0c464 --- /dev/null +++ b/probes/sastToolSonarInstalled/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: sastToolSonarInstalled +short: Check that the project uses Sonar. +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 the projects pom.xml for use of Sonar. +outcome: + - If the project uses Sonar, the probe returns one finding with OutcomePositive (1). + - If the project does not use the Sonar, the probe returns one finding with OutcomeNegative (0). +remediation: + effort: Medium + text: + - Follow the steps in https://docs.sonarsource.com/sonarqube/latest/setup-and-upgrade/overview/ to integrate Sonar into your project. + markdown: + - Follow the steps in https://docs.sonarsource.com/sonarqube/latest/setup-and-upgrade/overview/ to integrate CodeQL into your project. \ No newline at end of file diff --git a/probes/sastToolSonarInstalled/impl.go b/probes/sastToolSonarInstalled/impl.go new file mode 100644 index 00000000000..822311265b9 --- /dev/null +++ b/probes/sastToolSonarInstalled/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 sastToolSonarInstalled + +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 = "sastToolSonarInstalled" + +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.SonarWorkflow { + f, err := finding.NewWith(fs, Probe, + "SAST tool detected: Sonar", 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, + "Sonar tool not detected", 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/sastToolSonarInstalled/impl_test.go b/probes/sastToolSonarInstalled/impl_test.go new file mode 100644 index 00000000000..eeb150f6a1b --- /dev/null +++ b/probes/sastToolSonarInstalled/impl_test.go @@ -0,0 +1,100 @@ +// 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 sastToolSonarInstalled + +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: "sonar present", + err: nil, + raw: &checker.RawResults{ + SASTResults: checker.SASTData{ + Workflows: []checker.SASTWorkflow{ + { + Type: checker.CodeQLWorkflow, + }, + { + Type: checker.SonarWorkflow, + }, + }, + }, + }, + outcomes: []finding.Outcome{ + finding.OutcomePositive, + }, + }, + { + name: "sonar not present", + err: nil, + raw: &checker.RawResults{ + SASTResults: checker.SASTData{ + Workflows: []checker.SASTWorkflow{ + { + 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 cc0c475185e607af09a7b59293d2ef4970a206b6 Mon Sep 17 00:00:00 2001 From: AdamKorcz <44787359+AdamKorcz@users.noreply.github.com> Date: Mon, 23 Oct 2023 21:37:32 +0100 Subject: [PATCH 02/15] Update checks/evaluation/sast.go Co-authored-by: Raghav Kaul <8695110+raghavkaul@users.noreply.github.com> Signed-off-by: AdamKorcz <44787359+AdamKorcz@users.noreply.github.com> --- 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 4509325a9ca..794c372e092 100644 --- a/checks/evaluation/sast.go +++ b/checks/evaluation/sast.go @@ -103,7 +103,7 @@ func SAST(name string, // Sast inconclusive. if codeQlScore != checker.InconclusiveResultScore { if codeQlScore == checker.MaxResultScore { - return checker.CreateMaxScoreResult(name, "SAST tool detected") + return checker.CreateMaxScoreResult(name, "SAST tool detected: CodeQL") } return checker.CreateMinScoreResult(name, "no SAST tool detected") } From 7921bf967b9ce09e6147c04ad35716f09baef526 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Tue, 24 Oct 2023 10:46:05 +0100 Subject: [PATCH 03/15] preserve file info when logging positive Sonar findings Signed-off-by: AdamKorcz --- checks/evaluation/sast.go | 7 ++++++- checks/evaluation/sast_test.go | 17 +++++++++++++++++ probes/sastToolSonarInstalled/impl.go | 25 +++++++++++++++++-------- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/checks/evaluation/sast.go b/checks/evaluation/sast.go index 794c372e092..27d196a15bf 100644 --- a/checks/evaluation/sast.go +++ b/checks/evaluation/sast.go @@ -52,7 +52,12 @@ func SAST(name string, if f.Outcome == finding.OutcomePositive { sonarScore = checker.MaxResultScore dl.Info(&checker.LogMessage{ - Text: f.Message, + Text: f.Message, + Type: f.Location.Type, + Path: f.Location.Path, + Offset: *f.Location.LineStart, + EndOffset: *f.Location.LineEnd, + Snippet: *f.Location.Snippet, }) } else if f.Outcome == finding.OutcomeNegative { sonarScore = checker.MinResultScore diff --git a/checks/evaluation/sast_test.go b/checks/evaluation/sast_test.go index 4db17a17d9b..8473958845c 100644 --- a/checks/evaluation/sast_test.go +++ b/checks/evaluation/sast_test.go @@ -23,6 +23,9 @@ import ( ) func TestSAST(t *testing.T) { + snippet := "some code snippet" + sline := uint(10) + eline := uint(46) t.Parallel() tests := []struct { name string @@ -64,6 +67,13 @@ func TestSAST(t *testing.T) { { Probe: "sastToolSonarInstalled", Outcome: finding.OutcomePositive, + Location: &finding.Location{ + Type: finding.FileTypeSource, + Path: "path/to/file.txt", + LineStart: &sline, + LineEnd: &eline, + Snippet: &snippet, + }, }, }, result: scut.TestReturn{ @@ -87,6 +97,13 @@ func TestSAST(t *testing.T) { { Probe: "sastToolSonarInstalled", Outcome: finding.OutcomePositive, + Location: &finding.Location{ + Type: finding.FileTypeSource, + Path: "path/to/file.txt", + LineStart: &sline, + LineEnd: &eline, + Snippet: &snippet, + }, }, }, result: scut.TestReturn{ diff --git a/probes/sastToolSonarInstalled/impl.go b/probes/sastToolSonarInstalled/impl.go index 822311265b9..5a364512fc5 100644 --- a/probes/sastToolSonarInstalled/impl.go +++ b/probes/sastToolSonarInstalled/impl.go @@ -37,15 +37,24 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { r := raw.SASTResults for _, wf := range r.Workflows { - if wf.Type == checker.SonarWorkflow { - f, err := finding.NewWith(fs, Probe, - "SAST tool detected: Sonar", nil, - finding.OutcomePositive) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - return []finding.Finding{*f}, Probe, nil + if wf.Type != checker.SonarWorkflow { + continue } + wf := wf + loc := &finding.Location{ + Type: wf.File.Type, + Path: wf.File.Path, + LineStart: &wf.File.Offset, + LineEnd: &wf.File.EndOffset, + Snippet: &wf.File.Snippet, + } + f, err := finding.NewWith(fs, Probe, + "SAST tool detected: Sonar", loc, + 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, "Sonar tool not detected", nil, From eedd304cbb0ba310be9fc6d1fa35b57e29b7197f Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Tue, 24 Oct 2023 11:14:55 +0100 Subject: [PATCH 04/15] rebase Signed-off-by: AdamKorcz --- checks/raw/sast.go | 7 ++++++- checks/sast.go | 12 ------------ probes/entries.go | 4 +--- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/checks/raw/sast.go b/checks/raw/sast.go index ad1bcd08e4a..9765ecfed81 100644 --- a/checks/raw/sast.go +++ b/checks/raw/sast.go @@ -35,7 +35,12 @@ const CheckSAST = "SAST" var errInvalid = errors.New("invalid") -var sastTools = map[string]bool{"github-code-scanning": true, "lgtm-com": true, "sonarcloud": true} +var sastTools = map[string]bool{ + "github-advanced-security": true, + "github-code-scanning": true, + "lgtm-com": true, + "sonarcloud": true, +} var allowedConclusions = map[string]bool{"success": true, "neutral": true} diff --git a/checks/sast.go b/checks/sast.go index 2e8ef26614d..405e492942e 100644 --- a/checks/sast.go +++ b/checks/sast.go @@ -15,7 +15,6 @@ package checks import ( - "errors" "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/checks/evaluation" "github.com/ossf/scorecard/v4/checks/raw" @@ -27,17 +26,6 @@ import ( // CheckSAST is the registered name for SAST. const CheckSAST = "SAST" -var errInvalid = errors.New("invalid") - -var sastTools = map[string]bool{ - "github-advanced-security": true, - "github-code-scanning": true, - "lgtm-com": true, - "sonarcloud": true, -} - -var allowedConclusions = map[string]bool{"success": true, "neutral": true} - //nolint:gochecknoinits func init() { if err := registerCheck(CheckSAST, SAST, nil); err != nil { diff --git a/probes/entries.go b/probes/entries.go index f23ee815c75..c83e7338365 100644 --- a/probes/entries.go +++ b/probes/entries.go @@ -86,7 +86,6 @@ var ( fuzzedWithPropertyBasedTypescript.Run, fuzzedWithPropertyBasedJavascript.Run, } -<<<<<<< HEAD Packaging = []ProbeImpl{ packagedWithAutomatedWorkflow.Run, } @@ -100,12 +99,11 @@ var ( } Vulnerabilities = []ProbeImpl{ hasOSVVulnerabilities.Run, -======= + } SAST = []ProbeImpl{ sastToolCodeQLInstalled.Run, sastToolRunsOnAllCommits.Run, sastToolSonarInstalled.Run, ->>>>>>> Convert SAST checks to probes } DangerousWorkflows = []ProbeImpl{ hasDangerousWorkflowScriptInjection.Run, From 9bd144143b5eda055d1323ba1167c62953dce199 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Thu, 2 Nov 2023 19:56:54 +0000 Subject: [PATCH 05/15] Remove warning logging Signed-off-by: AdamKorcz --- checks/evaluation/sast.go | 10 +--------- checks/evaluation/sast_test.go | 2 +- e2e/sast_test.go | 2 +- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/checks/evaluation/sast.go b/checks/evaluation/sast.go index 27d196a15bf..2943956ccfb 100644 --- a/checks/evaluation/sast.go +++ b/checks/evaluation/sast.go @@ -61,9 +61,6 @@ func SAST(name string, }) } else if f.Outcome == finding.OutcomeNegative { sonarScore = checker.MinResultScore - dl.Warn(&checker.LogMessage{ - Text: f.Message, - }) } } } @@ -133,7 +130,7 @@ func getSASTScore(f *finding.Finding, dl checker.DetailLogger) int { switch f.Outcome { case finding.OutcomeNotApplicable: dl.Warn(&checker.LogMessage{ - Text: "no pull requests merged into dev branch", + Text: f.Message, }) return checker.InconclusiveResultScore case finding.OutcomePositive: @@ -154,11 +151,6 @@ func getSASTScore(f *finding.Finding, dl checker.DetailLogger) int { // if it doesn't. func getCodeQLScore(f *finding.Finding, dl checker.DetailLogger) int { switch f.Outcome { - case finding.OutcomeNotApplicable: - dl.Warn(&checker.LogMessage{ - Text: "no pull requests merged into dev branch", - }) - return checker.InconclusiveResultScore case finding.OutcomePositive: dl.Info(&checker.LogMessage{ Text: f.Message, diff --git a/checks/evaluation/sast_test.go b/checks/evaluation/sast_test.go index 8473958845c..61d6ae56f62 100644 --- a/checks/evaluation/sast_test.go +++ b/checks/evaluation/sast_test.go @@ -134,7 +134,7 @@ func TestSAST(t *testing.T) { }, result: scut.TestReturn{ Score: 3, - NumberOfWarn: 3, + NumberOfWarn: 2, NumberOfInfo: 0, }, }, 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 4c228ed4241726372aa8cb99d3122ab3f82034a5 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 3 Nov 2023 10:05:35 +0000 Subject: [PATCH 06/15] add outcome and message to finding on the same line Signed-off-by: AdamKorcz --- probes/sastToolRunsOnAllCommits/impl.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/probes/sastToolRunsOnAllCommits/impl.go b/probes/sastToolRunsOnAllCommits/impl.go index c7e285cc19c..163bcec2fc3 100644 --- a/probes/sastToolRunsOnAllCommits/impl.go +++ b/probes/sastToolRunsOnAllCommits/impl.go @@ -63,11 +63,11 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { f = f.WithValue("totalMerged", totalMerged) if totalTested == totalMerged { - f = f.WithOutcome(finding.OutcomePositive) - f = f.WithMessage(fmt.Sprintf("all commits (%v) are checked with a SAST tool", totalMerged)) + msg := fmt.Sprintf("all commits (%v) are checked with a SAST tool", totalMerged) + f = f.WithOutcome(finding.OutcomePositive).WithMessage(msg) } else { - f = f.WithOutcome(finding.OutcomeNegative) - f = f.WithMessage(fmt.Sprintf("%v commits out of %v are checked with a SAST tool", totalTested, totalMerged)) + msg := fmt.Sprintf("%v commits out of %v are checked with a SAST tool", totalTested, totalMerged) + f = f.WithOutcome(finding.OutcomeNegative).WithMessage(msg) } return []finding.Finding{*f}, Probe, nil } From 950d78b63837e8f972d8d988b62614f3a9c5426d Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 3 Nov 2023 10:06:37 +0000 Subject: [PATCH 07/15] codeql workflow -> codeql action Signed-off-by: AdamKorcz --- probes/sastToolCodeQLInstalled/def.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/probes/sastToolCodeQLInstalled/def.yml b/probes/sastToolCodeQLInstalled/def.yml index 68503317a09..67de87eab9d 100644 --- a/probes/sastToolCodeQLInstalled/def.yml +++ b/probes/sastToolCodeQLInstalled/def.yml @@ -17,10 +17,10 @@ short: Check that the project uses the CodeQL github actions 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 github/codeql-action/analyze workflow. + The implementation checks whether the project invokes the github/codeql-action/analyze action. outcome: - - If the project uses the github/codeql-action/analyze workflow, the probe returns one finding with OutcomePositive (1). - - If the project does not use the github/codeql-action/analyze workflow, the probe returns one finding with OutcomeNegative (0). + - If the project uses the github/codeql-action/analyze action, the probe returns one finding with OutcomePositive (1). + - If the project does not use the github/codeql-action/analyze action, the probe returns one finding with OutcomeNegative (0). remediation: effort: Medium text: From da733db7b7d94e67afacfe6e018e85f05e6f9333 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 3 Nov 2023 10:11:12 +0000 Subject: [PATCH 08/15] 'the Sonar' -> 'Sonar' in probe def.yml Signed-off-by: AdamKorcz --- probes/sastToolSonarInstalled/def.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/probes/sastToolSonarInstalled/def.yml b/probes/sastToolSonarInstalled/def.yml index 7bbc5e0c464..53d0d5f49b6 100644 --- a/probes/sastToolSonarInstalled/def.yml +++ b/probes/sastToolSonarInstalled/def.yml @@ -20,7 +20,7 @@ implementation: > The implementation checks the projects pom.xml for use of Sonar. outcome: - If the project uses Sonar, the probe returns one finding with OutcomePositive (1). - - If the project does not use the Sonar, the probe returns one finding with OutcomeNegative (0). + - If the project does not the Sonar, the probe returns one finding with OutcomeNegative (0). remediation: effort: Medium text: From 86c324907468141e2bf7ab29457d1fb219c18c06 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 3 Nov 2023 10:13:56 +0000 Subject: [PATCH 09/15] fix typo Signed-off-by: AdamKorcz --- checks/evaluation/sast_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checks/evaluation/sast_test.go b/checks/evaluation/sast_test.go index 61d6ae56f62..c13344a24b5 100644 --- a/checks/evaluation/sast_test.go +++ b/checks/evaluation/sast_test.go @@ -50,7 +50,7 @@ func TestSAST(t *testing.T) { }, }, { - name: "Sonar and CodeCQ is installed", + name: "Sonar and codeQL is installed", findings: []finding.Finding{ { Probe: "sastToolCodeQLInstalled", From f0edb86324ec05ebfb9ecc0a69d0c9b2d7bfe6c6 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 3 Nov 2023 10:16:31 +0000 Subject: [PATCH 10/15] Change how probe creates location Signed-off-by: AdamKorcz --- probes/sastToolSonarInstalled/impl.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/probes/sastToolSonarInstalled/impl.go b/probes/sastToolSonarInstalled/impl.go index 5a364512fc5..edab64768e0 100644 --- a/probes/sastToolSonarInstalled/impl.go +++ b/probes/sastToolSonarInstalled/impl.go @@ -37,17 +37,11 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { r := raw.SASTResults for _, wf := range r.Workflows { + wf := wf if wf.Type != checker.SonarWorkflow { continue } - wf := wf - loc := &finding.Location{ - Type: wf.File.Type, - Path: wf.File.Path, - LineStart: &wf.File.Offset, - LineEnd: &wf.File.EndOffset, - Snippet: &wf.File.Snippet, - } + loc := wf.File.Location() f, err := finding.NewWith(fs, Probe, "SAST tool detected: Sonar", loc, finding.OutcomePositive) From a4e62d659c5a51fb784d5aaf35e392ec1f7eae6f Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 3 Nov 2023 10:39:32 +0000 Subject: [PATCH 11/15] Change names of values Signed-off-by: AdamKorcz --- checks/evaluation/sast.go | 4 ++-- checks/evaluation/sast_test.go | 8 ++++---- probes/sastToolRunsOnAllCommits/impl.go | 19 ++++++++++--------- probes/sastToolRunsOnAllCommits/impl_test.go | 8 ++++---- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/checks/evaluation/sast.go b/checks/evaluation/sast.go index 2943956ccfb..1fbe6007d4b 100644 --- a/checks/evaluation/sast.go +++ b/checks/evaluation/sast.go @@ -142,9 +142,9 @@ func getSASTScore(f *finding.Finding, dl checker.DetailLogger) int { Text: f.Message, }) default: - checker.CreateProportionalScore(f.Values["totalTested"], f.Values["totalMerged"]) + checker.CreateProportionalScore(f.Values["totalPullRequestsAnalyzed"], f.Values["totalPullRequestsMerged"]) } - return checker.CreateProportionalScore(f.Values["totalTested"], f.Values["totalMerged"]) + return checker.CreateProportionalScore(f.Values["totalPullRequestsAnalyzed"], f.Values["totalPullRequestsMerged"]) } // getCodeQLScore returns positive the project runs CodeQL and negative diff --git a/checks/evaluation/sast_test.go b/checks/evaluation/sast_test.go index c13344a24b5..a8b2f8f5728 100644 --- a/checks/evaluation/sast_test.go +++ b/checks/evaluation/sast_test.go @@ -60,8 +60,8 @@ func TestSAST(t *testing.T) { Probe: "sastToolRunsOnAllCommits", Outcome: finding.OutcomePositive, Values: map[string]int{ - "totalTested": 1, - "totalMerged": 2, + "totalPullRequestsAnalyzed": 1, + "totalPullRequestsMerged": 2, }, }, { @@ -123,8 +123,8 @@ func TestSAST(t *testing.T) { Probe: "sastToolRunsOnAllCommits", Outcome: finding.OutcomeNegative, Values: map[string]int{ - "totalTested": 1, - "totalMerged": 3, + "totalPullRequestsAnalyzed": 1, + "totalPullRequestsMerged": 3, }, }, { diff --git a/probes/sastToolRunsOnAllCommits/impl.go b/probes/sastToolRunsOnAllCommits/impl.go index 163bcec2fc3..142cae9ea87 100644 --- a/probes/sastToolRunsOnAllCommits/impl.go +++ b/probes/sastToolRunsOnAllCommits/impl.go @@ -43,30 +43,31 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { return nil, Probe, fmt.Errorf("create finding: %w", err) } - totalMerged := len(r.Commits) - totalTested := 0 + totalPullRequestsMerged := len(r.Commits) + totalPullRequestsAnalyzed := 0 for i := range r.Commits { wf := &r.Commits[i] if wf.Compliant { - totalTested++ + totalPullRequestsAnalyzed++ } } - if totalMerged == 0 { + if totalPullRequestsMerged == 0 { f = f.WithOutcome(finding.OutcomeNotApplicable) f = f.WithMessage("no pull requests merged into dev branch") return []finding.Finding{*f}, Probe, nil } - f = f.WithValue("totalTested", totalTested) - f = f.WithValue("totalMerged", totalMerged) + f = f.WithValue("totalPullRequestsAnalyzed", totalPullRequestsAnalyzed) + f = f.WithValue("totalPullRequestsMerged", totalPullRequestsMerged) - if totalTested == totalMerged { - msg := fmt.Sprintf("all commits (%v) are checked with a SAST tool", totalMerged) + if totalPullRequestsAnalyzed == totalPullRequestsMerged { + msg := fmt.Sprintf("all commits (%v) are checked with a SAST tool", totalPullRequestsMerged) f = f.WithOutcome(finding.OutcomePositive).WithMessage(msg) } else { - msg := fmt.Sprintf("%v commits out of %v are checked with a SAST tool", totalTested, totalMerged) + msg := fmt.Sprintf("%v commits out of %v are checked with a SAST tool", + totalPullRequestsAnalyzed, totalPullRequestsMerged) f = f.WithOutcome(finding.OutcomeNegative).WithMessage(msg) } return []finding.Finding{*f}, Probe, nil diff --git a/probes/sastToolRunsOnAllCommits/impl_test.go b/probes/sastToolRunsOnAllCommits/impl_test.go index f1892d4b672..09c6e988f48 100644 --- a/probes/sastToolRunsOnAllCommits/impl_test.go +++ b/probes/sastToolRunsOnAllCommits/impl_test.go @@ -58,8 +58,8 @@ func Test_Run(t *testing.T) { Probe: "sastToolRunsOnAllCommits", Message: "1 commits out of 2 are checked with a SAST tool", Values: map[string]int{ - "totalTested": 1, - "totalMerged": 2, + "totalPullRequestsAnalyzed": 1, + "totalPullRequestsMerged": 2, }, }, }, @@ -88,8 +88,8 @@ func Test_Run(t *testing.T) { Message: "all commits (2) are checked with a SAST tool", Outcome: finding.OutcomePositive, Values: map[string]int{ - "totalTested": 2, - "totalMerged": 2, + "totalPullRequestsAnalyzed": 2, + "totalPullRequestsMerged": 2, }, }, }, From f9c198cd4cf2f4687a021689bf2507b321ae8bba Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 3 Nov 2023 10:44:33 +0000 Subject: [PATCH 12/15] change 'SAST tool detected: xx' to 'SAST tool installed: xx' Signed-off-by: AdamKorcz --- probes/sastToolCodeQLInstalled/impl.go | 4 ++-- probes/sastToolSonarInstalled/impl.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/probes/sastToolCodeQLInstalled/impl.go b/probes/sastToolCodeQLInstalled/impl.go index ff80f4f12ce..d3f3b656627 100644 --- a/probes/sastToolCodeQLInstalled/impl.go +++ b/probes/sastToolCodeQLInstalled/impl.go @@ -39,7 +39,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for _, wf := range r.Workflows { if wf.Type == checker.CodeQLWorkflow { f, err := finding.NewWith(fs, Probe, - "SAST tool detected: CodeQL", nil, + "SAST tool installed: CodeQL", nil, finding.OutcomePositive) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) @@ -48,7 +48,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } } f, err := finding.NewWith(fs, Probe, - "CodeQL tool not detected", nil, + "CodeQL tool not installed", nil, finding.OutcomeNegative) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) diff --git a/probes/sastToolSonarInstalled/impl.go b/probes/sastToolSonarInstalled/impl.go index edab64768e0..4c68afbeba1 100644 --- a/probes/sastToolSonarInstalled/impl.go +++ b/probes/sastToolSonarInstalled/impl.go @@ -43,7 +43,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } loc := wf.File.Location() f, err := finding.NewWith(fs, Probe, - "SAST tool detected: Sonar", loc, + "SAST tool installed: Sonar", loc, finding.OutcomePositive) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) @@ -51,7 +51,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { return []finding.Finding{*f}, Probe, nil } f, err := finding.NewWith(fs, Probe, - "Sonar tool not detected", nil, + "Sonar tool not installed", nil, finding.OutcomeNegative) if err != nil { return nil, Probe, fmt.Errorf("create finding: %w", err) From 082223074e629cd71b9dbf0ba49373608e19483b Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 3 Nov 2023 11:03:41 +0000 Subject: [PATCH 13/15] make text in probe def.yml easier to read Signed-off-by: AdamKorcz --- probes/sastToolRunsOnAllCommits/def.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/probes/sastToolRunsOnAllCommits/def.yml b/probes/sastToolRunsOnAllCommits/def.yml index 4d492e5c257..9941fac3241 100644 --- a/probes/sastToolRunsOnAllCommits/def.yml +++ b/probes/sastToolRunsOnAllCommits/def.yml @@ -20,8 +20,8 @@ implementation: > The implementation iterates through the projects commits and checks whether any of the check runs for the commits associated merge request was any of the SAST tools that Scorecard supports. outcome: - If the project had no commits merged, the probe returns a finding with OutcomeNotApplicable. - - If any of the check runs of an associated merge request of a commit was for a SAST tool, the probe returns one finding with OutcomePositive (1). In addition, the finding will include to values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. - - If none of the check runs of an associated merge request of a commit was for a SAST tool, the probe returns one finding with OutcomeNegative (0). In addition, the finding will include to values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. + - If the project runs SAST tools successfuly on every pull request before merging, the probe returns one finding with OutcomePositive (1). In addition, the finding will include to values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. + - If the project does not run SAST tools successfuly on every pull request before merging, the probe returns one finding with OutcomeNegative (0). In addition, the finding will include to values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. remediation: effort: Low text: From 0ca8028f73b8d67e13cc006820ae034b1c01076e Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 3 Nov 2023 11:05:27 +0000 Subject: [PATCH 14/15] Change 'to' to 'two' Signed-off-by: AdamKorcz --- probes/sastToolRunsOnAllCommits/def.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/probes/sastToolRunsOnAllCommits/def.yml b/probes/sastToolRunsOnAllCommits/def.yml index 9941fac3241..1656bda7b91 100644 --- a/probes/sastToolRunsOnAllCommits/def.yml +++ b/probes/sastToolRunsOnAllCommits/def.yml @@ -20,8 +20,8 @@ implementation: > The implementation iterates through the projects commits and checks whether any of the check runs for the commits associated merge request was any of the SAST tools that Scorecard supports. outcome: - If the project had no commits merged, the probe returns a finding with OutcomeNotApplicable. - - If the project runs SAST tools successfuly on every pull request before merging, the probe returns one finding with OutcomePositive (1). In addition, the finding will include to values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. - - If the project does not run SAST tools successfuly on every pull request before merging, the probe returns one finding with OutcomeNegative (0). In addition, the finding will include to values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. + - If the project runs SAST tools successfuly on every pull request before merging, the probe returns one finding with OutcomePositive (1). In addition, the finding will include two values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. + - If the project does not run SAST tools successfuly on every pull request before merging, the probe returns one finding with OutcomeNegative (0). In addition, the finding will include two values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. remediation: effort: Low text: From 9b5df4736df192143283f2a1492cdf57ff19af64 Mon Sep 17 00:00:00 2001 From: AdamKorcz Date: Fri, 3 Nov 2023 11:34:53 +0000 Subject: [PATCH 15/15] Minor change Signed-off-by: AdamKorcz --- probes/sastToolRunsOnAllCommits/def.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/probes/sastToolRunsOnAllCommits/def.yml b/probes/sastToolRunsOnAllCommits/def.yml index 1656bda7b91..d11b30857d1 100644 --- a/probes/sastToolRunsOnAllCommits/def.yml +++ b/probes/sastToolRunsOnAllCommits/def.yml @@ -21,7 +21,7 @@ implementation: > outcome: - If the project had no commits merged, the probe returns a finding with OutcomeNotApplicable. - If the project runs SAST tools successfuly on every pull request before merging, the probe returns one finding with OutcomePositive (1). In addition, the finding will include two values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. - - If the project does not run SAST tools successfuly on every pull request before merging, the probe returns one finding with OutcomeNegative (0). In addition, the finding will include two values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. + - If the project does not run any SAST tools successfuly on every pull request before merging, the probe returns one finding with OutcomeNegative (0). In addition, the finding will include two values. 1) How many commits were tested by a SAST tool, and 2) How many commits in total were merged. remediation: effort: Low text: