Skip to content

Commit

Permalink
✨ Use local files instead of search for SAST CodeQL check (ossf#2839)
Browse files Browse the repository at this point in the history
* Look for codeQL action use with local files instead of search.

Signed-off-by: Spencer Schrock <[email protected]>

* Switch SAST mocks to using local file contents.

Signed-off-by: Spencer Schrock <[email protected]>

* Update e2e test

Signed-off-by: Spencer Schrock <[email protected]>

* Remove unneeded code.

The tests deleted here were merged with another test in an earlier commit.

Signed-off-by: Spencer Schrock <[email protected]>

* update

Signed-off-by: Spencer Schrock <[email protected]>

* Add tests to get code coverage up.

Signed-off-by: Spencer Schrock <[email protected]>

---------

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Avishay <[email protected]>
  • Loading branch information
spencerschrock authored and balteravishay committed Apr 13, 2023
1 parent 123010f commit fe0d01d
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 23 deletions.
65 changes: 54 additions & 11 deletions checks/sast.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ import (
"regexp"
"strings"

"github.com/rhysd/actionlint"

"github.com/ossf/scorecard/v4/checker"
"github.com/ossf/scorecard/v4/checks/fileparser"
"github.com/ossf/scorecard/v4/clients"
sce "github.com/ossf/scorecard/v4/errors"
"github.com/ossf/scorecard/v4/finding"
)
Expand Down Expand Up @@ -187,19 +188,18 @@ func sastToolInCheckRuns(c *checker.CheckRequest) (int, error) {
}

func codeQLInCheckDefinitions(c *checker.CheckRequest) (int, error) {
searchRequest := clients.SearchRequest{
Query: "github/codeql-action/analyze",
Path: "/.github/workflows",
}
resp, err := c.RepoClient.Search(searchRequest)
var workflowPaths []string
err := fileparser.OnMatchingFileContentDo(c.RepoClient, fileparser.PathMatcher{
Pattern: ".github/workflows/*",
CaseSensitive: false,
}, searchGitHubActionWorkflowCodeQL, &workflowPaths)
if err != nil {
return checker.InconclusiveResultScore,
sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("Client.Search.Code: %v", err))
return checker.InconclusiveResultScore, err
}

for _, result := range resp.Results {
for _, path := range workflowPaths {
c.Dlogger.Debug(&checker.LogMessage{
Path: result.Path,
Path: path,
Type: finding.FileTypeSource,
Offset: checker.OffsetDefault,
Text: "CodeQL detected",
Expand All @@ -208,7 +208,7 @@ func codeQLInCheckDefinitions(c *checker.CheckRequest) (int, error) {

// TODO: check if it's enabled as cron or presubmit.
// TODO: check which branches it is enabled on. We should find main.
if resp.Hits > 0 {
if len(workflowPaths) > 0 {
c.Dlogger.Info(&checker.LogMessage{
Text: "SAST tool detected: CodeQL",
})
Expand All @@ -221,6 +221,49 @@ func codeQLInCheckDefinitions(c *checker.CheckRequest) (int, error) {
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 {
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
Expand Down
61 changes: 53 additions & 8 deletions checks/sast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"errors"
"fmt"
"os"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -105,9 +106,7 @@ func Test_SAST(t *testing.T) {
},
},
},
searchresult: clients.SearchResponse{Hits: 1, Results: []clients.SearchResult{{
Path: "test.go",
}}},
path: ".github/workflows/github-workflow-sast-codeql.yaml",
checkRuns: []clients.CheckRun{
{
Status: "completed",
Expand Down Expand Up @@ -144,7 +143,7 @@ func Test_SAST(t *testing.T) {
},
},
},
searchresult: clients.SearchResponse{},
path: ".github/workflows/github-workflow-sast-no-codeql.yaml",
checkRuns: []clients.CheckRun{
{
App: clients.CheckRunApp{
Expand Down Expand Up @@ -201,14 +200,14 @@ func Test_SAST(t *testing.T) {
},
{
name: "sonartype config 1 line",
path: "./testdata/pom-1line.xml",
path: "pom-1line.xml",
expected: checker.CheckResult{
Score: 10,
},
},
{
name: "sonartype config 2 lines",
path: "./testdata/pom-2lines.xml",
path: "pom-2lines.xml",
expected: checker.CheckResult{
Score: 10,
},
Expand All @@ -234,13 +233,16 @@ func Test_SAST(t *testing.T) {
mockRepoClient.EXPECT().Search(searchRequest).Return(tt.searchresult, nil).AnyTimes()
mockRepoClient.EXPECT().ListFiles(gomock.Any()).DoAndReturn(
func(predicate func(string) (bool, error)) ([]string, error) {
return []string{"pom.xml"}, nil
if strings.Contains(tt.path, "pom") {
return []string{"pom.xml"}, nil
}
return []string{tt.path}, nil
}).AnyTimes()
mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) {
if tt.path == "" {
return nil, nil
}
content, err := os.ReadFile(tt.path)
content, err := os.ReadFile("./testdata/" + tt.path)
if err != nil {
return content, fmt.Errorf("%w", err)
}
Expand Down Expand Up @@ -342,3 +344,46 @@ func Test_validateSonarConfig(t *testing.T) {
})
}
}

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")
}
})
}
}
24 changes: 24 additions & 0 deletions checks/testdata/.github/workflows/github-workflow-sast-codeql.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# 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.
name: absent workflow
on: [push]

jobs:
Explore-GitHub-Actions:
runs-on: ubuntu-latest
steps:
- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v1
- name: some name
uses: docker/[email protected]
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# 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.
on:
push:
paths:
- 'source/common/**'
pull_request:

jobs:
Some-Build:

strategy:
fail-fast: false

# CodeQL runs on ubuntu-latest and windows-latest
runs-on: ubuntu-latest

steps:
- name: Checkout repository
uses: actions/checkout@v2
with:
fetch-depth: 2

- name: Acme CodeQL
uses: acme/codeql-action/init@v2 # some comment
with:
languages: cpp
8 changes: 4 additions & 4 deletions e2e/sast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ var _ = Describe("E2E TEST:"+checks.CheckSAST, func() {
}
expected := scut.TestReturn{
Error: nil,
Score: 0,
NumberOfWarn: 2,
NumberOfInfo: 0,
NumberOfDebug: 0,
Score: 10,
NumberOfWarn: 1,
NumberOfInfo: 1,
NumberOfDebug: 1,
}
result := checks.SAST(&req)
// New version.
Expand Down

0 comments on commit fe0d01d

Please sign in to comment.