Use RepoClient.Search API in SAST check (#857)

Co-authored-by: Azeem Shaikh <azeems@google.com>

checks/sast.go

@@ -20,6 +20,7 @@ import (
 	"github.com/google/go-github/v38/github"
 
 	"github.com/ossf/scorecard/v2/checker"
+	"github.com/ossf/scorecard/v2/clients"
 	sce "github.com/ossf/scorecard/v2/errors"
 )
 
@@ -158,20 +159,23 @@ func sastToolInCheckRuns(c *checker.CheckRequest) (int, error) {
 
 // nolint
 func codeQLInCheckDefinitions(c *checker.CheckRequest) (int, error) {
-	searchQuery := ("github/codeql-action path:/.github/workflows repo:" + c.Owner + "/" + c.Repo)
-	results, _, err := c.Client.Search.Code(c.Ctx, searchQuery, &github.SearchOptions{})
+	searchRequest := clients.SearchRequest{
+		Query: "github/codeql-action",
+		Path:  "/.github/workflows",
+	}
+	resp, err := c.RepoClient.Search(searchRequest)
 	if err != nil {
 		return checker.InconclusiveResultScore,
 			sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("Client.Search.Code: %v", err))
 	}
 
-	for _, result := range results.CodeResults {
-		c.Dlogger.Debug("CodeQL detected: %s", result.GetPath())
+	for _, result := range resp.Results {
+		c.Dlogger.Debug("CodeQL detected: %s", result.Path)
 	}
 
 	// TODO: check if it's enabled as cron or presubmit.
 	// TODO: check which branches it is enabled on. We should find main.
-	if *results.Total > 0 {
+	if resp.Hits > 0 {
 		c.Dlogger.Info("SAST tool detected: CodeQL")
 		return checker.MaxResultScore, nil
 	}

clients/githubrepo/client.go

@@ -33,6 +33,7 @@ type Client struct {
 	repoClient   *github.Client
 	graphClient  *graphqlHandler
 	contributors *contributorsHandler
+	search       *searchHandler
 	ctx          context.Context
 	tarball      tarballHandler
 }
@@ -64,6 +65,9 @@ func (client *Client) InitRepo(owner, repoName string) error {
 		return fmt.Errorf("error during contributorsHandler.init: %w", err)
 	}
 
+	// Setup Search.
+	client.search.init(client.ctx, owner, repoName)
+
 	return nil
 }
 
@@ -113,21 +117,8 @@ func (client *Client) GetDefaultBranch() (clients.BranchRef, error) {
 }
 
 // Search implements RepoClient.Search.
-func (client *Client) Search(request clients.SearchRequest) (clients.SearchResult, error) {
-	var query string
-	if request.Filename == "" {
-		query = fmt.Sprintf("%s repo:%s/%s", request.Query, client.owner, client.repoName)
-	} else {
-		query = fmt.Sprintf("%s repo:%s/%s in:file filename:%s",
-			request.Query, client.owner, client.repoName, request.Filename)
-	}
-	res, _, err := client.repoClient.Search.Code(client.ctx, query, &github.SearchOptions{})
-	if err != nil {
-		return clients.SearchResult{}, fmt.Errorf("Search.Code: %w", err)
-	}
-	return clients.SearchResult{
-		Hits: res.GetTotal(),
-	}, nil
+func (client *Client) Search(request clients.SearchRequest) (clients.SearchResponse, error) {
+	return client.search.search(request)
 }
 
 // Close implements RepoClient.Close.

clients/githubrepo/search.go

@@ -0,0 +1,88 @@
+// Copyright 2021 Security 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 githubrepo
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/google/go-github/v38/github"
+
+	"github.com/ossf/scorecard/v2/clients"
+)
+
+var errEmptyQuery = errors.New("search query is empty")
+
+type searchHandler struct {
+	ghClient *github.Client
+	ctx      context.Context
+	owner    string
+	repo     string
+}
+
+func (handler *searchHandler) init(ctx context.Context, owner, repo string) {
+	handler.ctx = ctx
+	handler.owner = owner
+	handler.repo = repo
+}
+
+func (handler *searchHandler) search(request clients.SearchRequest) (clients.SearchResponse, error) {
+	query, err := handler.buildQuery(request)
+	if err != nil {
+		return clients.SearchResponse{}, fmt.Errorf("handler.buildQuery: %w", err)
+	}
+
+	resp, _, err := handler.ghClient.Search.Code(handler.ctx, query, &github.SearchOptions{})
+	if err != nil {
+		return clients.SearchResponse{}, fmt.Errorf("Search.Code: %w", err)
+	}
+	return searchResponseFrom(resp), nil
+}
+
+func (handler *searchHandler) buildQuery(request clients.SearchRequest) (string, error) {
+	if request.Query == "" {
+		return "", fmt.Errorf("%w", errEmptyQuery)
+	}
+	var queryBuilder strings.Builder
+	if _, err := queryBuilder.WriteString(
+		fmt.Sprintf("%s repo:%s/%s", request.Query, handler.owner, handler.repo)); err != nil {
+		return "", fmt.Errorf("WriteString: %w", err)
+	}
+	if request.Filename != "" {
+		if _, err := queryBuilder.WriteString(
+			fmt.Sprintf(" in:file filename:%s", request.Filename)); err != nil {
+			return "", fmt.Errorf("WriteString: %w", err)
+		}
+	}
+	if request.Path != "" {
+		if _, err := queryBuilder.WriteString(fmt.Sprintf(" path:%s", request.Path)); err != nil {
+			return "", fmt.Errorf("WriteString: %w", err)
+		}
+	}
+	return queryBuilder.String(), nil
+}
+
+func searchResponseFrom(resp *github.CodeSearchResult) clients.SearchResponse {
+	var ret clients.SearchResponse
+	ret.Hits = resp.GetTotal()
+	for _, result := range resp.CodeResults {
+		ret.Results = append(ret.Results, clients.SearchResult{
+			Path: result.GetPath(),
+		})
+	}
+	return ret
+}

clients/githubrepo/search_test.go

@@ -0,0 +1,108 @@
+// Copyright 2021 Security 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 githubrepo
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/ossf/scorecard/v2/clients"
+)
+
+func TestBuildQuery(t *testing.T) {
+	t.Parallel()
+	testcases := []struct {
+		searchReq       clients.SearchRequest
+		expectedErrType error
+		name            string
+		owner           string
+		repo            string
+		expectedQuery   string
+		hasError        bool
+	}{
+		{
+			name:  "Basic",
+			owner: "testowner",
+			repo:  "testrepo",
+			searchReq: clients.SearchRequest{
+				Query: "testquery",
+			},
+			expectedQuery: "testquery repo:testowner/testrepo",
+		},
+		{
+			name:            "EmptyQuery",
+			owner:           "testowner",
+			repo:            "testrepo",
+			searchReq:       clients.SearchRequest{},
+			hasError:        true,
+			expectedErrType: errEmptyQuery,
+		},
+		{
+			name:  "WithFilename",
+			owner: "testowner",
+			repo:  "testrepo",
+			searchReq: clients.SearchRequest{
+				Query:    "testquery",
+				Filename: "filename1.txt",
+			},
+			expectedQuery: "testquery repo:testowner/testrepo in:file filename:filename1.txt",
+		},
+		{
+			name:  "WithPath",
+			owner: "testowner",
+			repo:  "testrepo",
+			searchReq: clients.SearchRequest{
+				Query: "testquery",
+				Path:  "dir1/file1.txt",
+			},
+			expectedQuery: "testquery repo:testowner/testrepo path:dir1/file1.txt",
+		},
+		{
+			name:  "WithFilenameAndPath",
+			owner: "testowner",
+			repo:  "testrepo",
+			searchReq: clients.SearchRequest{
+				Query:    "testquery",
+				Filename: "filename1.txt",
+				Path:     "dir1/dir2",
+			},
+			expectedQuery: "testquery repo:testowner/testrepo in:file filename:filename1.txt path:dir1/dir2",
+		},
+	}
+
+	for _, testcase := range testcases {
+		testcase := testcase
+		t.Run(testcase.name, func(t *testing.T) {
+			t.Parallel()
+
+			handler := searchHandler{
+				owner: testcase.owner,
+				repo:  testcase.repo,
+			}
+
+			query, err := handler.buildQuery(testcase.searchReq)
+			if !testcase.hasError && err != nil {
+				t.Fatalf("expected - no error, got: %v", err)
+			}
+			if testcase.hasError && !errors.Is(err, testcase.expectedErrType) {
+				t.Fatalf("expectedErrType - %v, got - %v",
+					testcase.expectedErrType, err)
+			} else if query != testcase.expectedQuery {
+				t.Fatalf("expectedQuery - %s, got - %s",
+					testcase.expectedQuery, query)
+			}
+		})
+	}
+}

clients/repo_client.go

@@ -54,6 +54,6 @@ type RepoClient interface {
 	ListCommits() ([]Commit, error)
 	ListReleases() ([]Release, error)
 	ListContributors() ([]Contributor, error)
-	Search(request SearchRequest) (SearchResult, error)
+	Search(request SearchRequest) (SearchResponse, error)
 	Close() error
 }

clients/search.go

@@ -15,13 +15,21 @@
 package clients
 
 // SearchRequest queries a repo for `Query`.
-// If `Filename` is provided, only files with matching path is queried.
+// If `Filename` is provided, only matching filenames are queried.
+// If `Path` is provided, only files with matching paths are queried.
 type SearchRequest struct {
 	Query    string
 	Filename string
+	Path     string
 }
 
-// SearchResult returns the results from a search request on a repo.
+// SearchResponse returns the results from a search request on a repo.
+type SearchResponse struct {
+	Results []SearchResult
+	Hits    int
+}
+
+// SearchResult represents a matching result from the search query.
 type SearchResult struct {
-	Hits int
+	Path string
 }