Add validation for commit-based APIs (#1635)

Co-authored-by: Azeem Shaikh <[email protected]>

checks/binary_artifact_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/golang/mock/gomock"
 
 	"github.com/ossf/scorecard/v4/checker"
+	"github.com/ossf/scorecard/v4/clients"
 	"github.com/ossf/scorecard/v4/clients/localdir"
 	"github.com/ossf/scorecard/v4/log"
 	scut "github.com/ossf/scorecard/v4/utests"
@@ -72,7 +73,7 @@ func TestBinaryArtifacts(t *testing.T) {
 			ctx := context.Background()
 
 			client := localdir.CreateLocalDirClient(ctx, logger)
-			if err := client.InitRepo(repo, "HEAD"); err != nil {
+			if err := client.InitRepo(repo, clients.HeadSHA); err != nil {
 				t.Errorf("InitRepo: %v", err)
 			}
 

checks/license_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/golang/mock/gomock"
 
 	"github.com/ossf/scorecard/v4/checker"
+	"github.com/ossf/scorecard/v4/clients"
 	"github.com/ossf/scorecard/v4/clients/localdir"
 	"github.com/ossf/scorecard/v4/log"
 	scut "github.com/ossf/scorecard/v4/utests"
@@ -154,7 +155,7 @@ func TestLicenseFileSubdirectory(t *testing.T) {
 			ctx := context.Background()
 
 			client := localdir.CreateLocalDirClient(ctx, logger)
-			if err := client.InitRepo(repo, "HEAD"); err != nil {
+			if err := client.InitRepo(repo, clients.HeadSHA); err != nil {
 				t.Errorf("InitRepo: %v", err)
 			}
 

checks/raw/security_policy.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/ossf/scorecard/v4/checker"
 	"github.com/ossf/scorecard/v4/checks/fileparser"
+	"github.com/ossf/scorecard/v4/clients"
 	"github.com/ossf/scorecard/v4/clients/githubrepo"
 	sce "github.com/ossf/scorecard/v4/errors"
 	"github.com/ossf/scorecard/v4/log"
@@ -80,7 +81,7 @@ func SecurityPolicy(c *checker.CheckRequest) (checker.SecurityPolicyData, error)
 		Repo:       c.Repo.Org(),
 	}
 
-	err = dotGitHub.RepoClient.InitRepo(dotGitHub.Repo, "HEAD")
+	err = dotGitHub.RepoClient.InitRepo(dotGitHub.Repo, clients.HeadSHA)
 	switch {
 	case err == nil:
 		defer dotGitHub.RepoClient.Close()

checks/sast.go

@@ -31,7 +31,6 @@ var allowedConclusions = map[string]bool{"success": true, "neutral": true}
 
 //nolint:gochecknoinits
 func init() {
-	// TODO(#575): Check if we can support commit-based requests here.
 	if err := registerCheck(CheckSAST, SAST, nil); err != nil {
 		// This should never happen.
 		panic(err)

clients/githubrepo/branches.go

@@ -17,6 +17,7 @@ package githubrepo
 import (
 	"context"
 	"fmt"
+	"strings"
 	"sync"
 
 	"github.com/google/go-github/v38/github"
@@ -113,31 +114,34 @@ type branchesHandler struct {
 	once             *sync.Once
 	ctx              context.Context
 	errSetup         error
-	owner            string
-	repo             string
+	repourl          *repoURL
 	defaultBranchRef *clients.BranchRef
 	branches         []*clients.BranchRef
 }
 
-func (handler *branchesHandler) init(ctx context.Context, owner, repo string) {
+func (handler *branchesHandler) init(ctx context.Context, repourl *repoURL) {
 	handler.ctx = ctx
-	handler.owner = owner
-	handler.repo = repo
+	handler.repourl = repourl
 	handler.errSetup = nil
 	handler.once = new(sync.Once)
 }
 
 func (handler *branchesHandler) setup() error {
 	handler.once.Do(func() {
+		if !strings.EqualFold(handler.repourl.commitSHA, clients.HeadSHA) {
+			handler.errSetup = fmt.Errorf("%w: branches only supported for HEAD queries", clients.ErrUnsupportedFeature)
+			return
+		}
 		vars := map[string]interface{}{
-			"owner":         githubv4.String(handler.owner),
-			"name":          githubv4.String(handler.repo),
+			"owner":         githubv4.String(handler.repourl.owner),
+			"name":          githubv4.String(handler.repourl.repo),
 			"refsToAnalyze": githubv4.Int(refsToAnalyze),
 			"refPrefix":     githubv4.String(refPrefix),
 		}
 		handler.data = new(branchesData)
 		if err := handler.graphClient.Query(handler.ctx, handler.data, vars); err != nil {
 			handler.errSetup = sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("githubv4.Query: %v", err))
+			return
 		}
 		handler.defaultBranchRef = getBranchRefFrom(handler.data.Repository.DefaultBranchRef)
 		handler.branches = getBranchRefsFrom(handler.data.Repository.Refs.Nodes, handler.defaultBranchRef)

clients/githubrepo/checkruns.go

@@ -17,6 +17,7 @@ package githubrepo
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/google/go-github/v38/github"
 
@@ -25,21 +26,22 @@ import (
 )
 
 type checkrunsHandler struct {
-	client *github.Client
-	ctx    context.Context
-	owner  string
-	repo   string
+	client  *github.Client
+	ctx     context.Context
+	repourl *repoURL
 }
 
-func (handler *checkrunsHandler) init(ctx context.Context, owner, repo string) {
+func (handler *checkrunsHandler) init(ctx context.Context, repourl *repoURL) {
 	handler.ctx = ctx
-	handler.owner = owner
-	handler.repo = repo
+	handler.repourl = repourl
 }
 
 func (handler *checkrunsHandler) listCheckRunsForRef(ref string) ([]clients.CheckRun, error) {
-	checkRuns, _, err := handler.client.Checks.ListCheckRunsForRef(handler.ctx, handler.owner, handler.repo, ref,
-		&github.ListCheckRunsOptions{})
+	if !strings.EqualFold(handler.repourl.commitSHA, clients.HeadSHA) {
+		return nil, fmt.Errorf("%w: ListCheckRuns only supported for HEAD queries", clients.ErrUnsupportedFeature)
+	}
+	checkRuns, _, err := handler.client.Checks.ListCheckRunsForRef(
+		handler.ctx, handler.repourl.owner, handler.repourl.repo, ref, &github.ListCheckRunsOptions{})
 	if err != nil {
 		return nil, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("ListCheckRunsForRef: %v", err))
 	}

clients/githubrepo/client.go

@@ -34,8 +34,7 @@ var errInputRepoType = errors.New("input repo should be of type repoURL")
 
 // Client is GitHub-specific implementation of RepoClient.
 type Client struct {
-	owner        string
-	repoName     string
+	repourl      *repoURL
 	repo         *github.Repository
 	repoClient   *github.Client
 	graphClient  *graphqlHandler
@@ -62,46 +61,50 @@ func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string) error {
 	if err != nil {
 		return sce.WithMessage(sce.ErrRepoUnreachable, err.Error())
 	}
+
 	client.repo = repo
-	client.owner = repo.Owner.GetLogin()
-	client.repoName = repo.GetName()
+	client.repourl = &repoURL{
+		owner:         repo.Owner.GetLogin(),
+		repo:          repo.GetName(),
+		defaultBranch: repo.GetDefaultBranch(),
+		commitSHA:     commitSHA,
+	}
 
 	// Init tarballHandler.
 	if err := client.tarball.init(client.ctx, client.repo, commitSHA); err != nil {
 		return fmt.Errorf("error during tarballHandler.init: %w", err)
 	}
 
 	// Setup GraphQL.
-	client.graphClient.init(client.ctx, client.owner, client.repoName,
-		client.repo.GetDefaultBranch(), commitSHA)
+	client.graphClient.init(client.ctx, client.repourl)
 
 	// Setup contributorsHandler.
-	client.contributors.init(client.ctx, client.owner, client.repoName)
+	client.contributors.init(client.ctx, client.repourl)
 
 	// Setup branchesHandler.
-	client.branches.init(client.ctx, client.owner, client.repoName)
+	client.branches.init(client.ctx, client.repourl)
 
 	// Setup releasesHandler.
-	client.releases.init(client.ctx, client.owner, client.repoName)
+	client.releases.init(client.ctx, client.repourl)
 
 	// Setup workflowsHandler.
-	client.workflows.init(client.ctx, client.owner, client.repoName)
+	client.workflows.init(client.ctx, client.repourl)
 
 	// Setup checkrunsHandler.
-	client.checkruns.init(client.ctx, client.owner, client.repoName)
+	client.checkruns.init(client.ctx, client.repourl)
 
 	// Setup statusesHandler.
-	client.statuses.init(client.ctx, client.owner, client.repoName)
+	client.statuses.init(client.ctx, client.repourl)
 
 	// Setup searchHandler.
-	client.search.init(client.ctx, client.owner, client.repoName)
+	client.search.init(client.ctx, client.repourl)
 
 	return nil
 }
 
 // URI implements RepoClient.URI.
 func (client *Client) URI() string {
-	return fmt.Sprintf("github.com/%s/%s", client.owner, client.repoName)
+	return fmt.Sprintf("github.com/%s/%s", client.repourl.owner, client.repourl.repo)
 }
 
 // ListFiles implements RepoClient.ListFiles.
@@ -224,7 +227,7 @@ func CreateOssFuzzRepoClient(ctx context.Context, logger *log.Logger) (clients.R
 	}
 
 	ossFuzzRepoClient := CreateGithubRepoClient(ctx, logger)
-	if err := ossFuzzRepoClient.InitRepo(ossFuzzRepo, "HEAD"); err != nil {
+	if err := ossFuzzRepoClient.InitRepo(ossFuzzRepo, clients.HeadSHA); err != nil {
 		return nil, fmt.Errorf("error during InitRepo: %w", err)
 	}
 	return ossFuzzRepoClient, nil

clients/githubrepo/contributors.go

@@ -17,6 +17,7 @@ package githubrepo
 import (
 	"context"
 	"fmt"
+	"strings"
 	"sync"
 
 	"github.com/google/go-github/v38/github"
@@ -29,25 +30,28 @@ type contributorsHandler struct {
 	once         *sync.Once
 	ctx          context.Context
 	errSetup     error
-	owner        string
-	repo         string
+	repourl      *repoURL
 	contributors []clients.Contributor
 }
 
-func (handler *contributorsHandler) init(ctx context.Context, owner, repo string) {
+func (handler *contributorsHandler) init(ctx context.Context, repourl *repoURL) {
 	handler.ctx = ctx
-	handler.owner = owner
-	handler.repo = repo
+	handler.repourl = repourl
 	handler.errSetup = nil
 	handler.once = new(sync.Once)
 }
 
 func (handler *contributorsHandler) setup() error {
 	handler.once.Do(func() {
+		if !strings.EqualFold(handler.repourl.commitSHA, clients.HeadSHA) {
+			handler.errSetup = fmt.Errorf("%w: ListContributors only supported for HEAD queries", clients.ErrUnsupportedFeature)
+			return
+		}
 		contribs, _, err := handler.ghClient.Repositories.ListContributors(
-			handler.ctx, handler.owner, handler.repo, &github.ListContributorsOptions{})
+			handler.ctx, handler.repourl.owner, handler.repourl.repo, &github.ListContributorsOptions{})
 		if err != nil {
 			handler.errSetup = fmt.Errorf("error during ListContributors: %w", err)
+			return
 		}
 
 		for _, contrib := range contribs {

clients/githubrepo/graphql.go

@@ -114,42 +114,36 @@ type graphqlData struct {
 }
 
 type graphqlHandler struct {
-	client        *githubv4.Client
-	data          *graphqlData
-	once          *sync.Once
-	ctx           context.Context
-	errSetup      error
-	owner         string
-	repo          string
-	defaultBranch string
-	commitSHA     string
-	commits       []clients.Commit
-	issues        []clients.Issue
-	archived      bool
+	client   *githubv4.Client
+	data     *graphqlData
+	once     *sync.Once
+	ctx      context.Context
+	errSetup error
+	repourl  *repoURL
+	commits  []clients.Commit
+	issues   []clients.Issue
+	archived bool
 }
 
-func (handler *graphqlHandler) init(ctx context.Context, owner, repo, defaultBranch, commitSHA string) {
+func (handler *graphqlHandler) init(ctx context.Context, repourl *repoURL) {
 	handler.ctx = ctx
-	handler.owner = owner
-	handler.repo = repo
-	handler.defaultBranch = defaultBranch
-	handler.commitSHA = commitSHA
+	handler.repourl = repourl
 	handler.data = new(graphqlData)
 	handler.errSetup = nil
 	handler.once = new(sync.Once)
 }
 
 func (handler *graphqlHandler) setup() error {
 	handler.once.Do(func() {
-		commitExpression := handler.commitSHA
-		if strings.EqualFold(handler.commitSHA, "HEAD") {
+		commitExpression := handler.repourl.commitSHA
+		if strings.EqualFold(handler.repourl.commitSHA, clients.HeadSHA) {
 			// TODO(#575): Confirm that this works as expected.
-			commitExpression = fmt.Sprintf("heads/%s", handler.defaultBranch)
+			commitExpression = fmt.Sprintf("heads/%s", handler.repourl.defaultBranch)
 		}
 
 		vars := map[string]interface{}{
-			"owner":                  githubv4.String(handler.owner),
-			"name":                   githubv4.String(handler.repo),
+			"owner":                  githubv4.String(handler.repourl.owner),
+			"name":                   githubv4.String(handler.repourl.repo),
 			"pullRequestsToAnalyze":  githubv4.Int(pullRequestsToAnalyze),
 			"issuesToAnalyze":        githubv4.Int(issuesToAnalyze),
 			"issueCommentsToAnalyze": githubv4.Int(issueCommentsToAnalyze),
@@ -163,7 +157,7 @@ func (handler *graphqlHandler) setup() error {
 			return
 		}
 		handler.archived = bool(handler.data.Repository.IsArchived)
-		handler.commits, handler.errSetup = commitsFrom(handler.data, handler.owner, handler.repo)
+		handler.commits, handler.errSetup = commitsFrom(handler.data, handler.repourl.owner, handler.repourl.repo)
 		if handler.errSetup != nil {
 			return
 		}
@@ -180,13 +174,19 @@ func (handler *graphqlHandler) getCommits() ([]clients.Commit, error) {
 }
 
 func (handler *graphqlHandler) getIssues() ([]clients.Issue, error) {
+	if !strings.EqualFold(handler.repourl.commitSHA, clients.HeadSHA) {
+		return nil, fmt.Errorf("%w: ListIssues only supported for HEAD queries", clients.ErrUnsupportedFeature)
+	}
 	if err := handler.setup(); err != nil {
 		return nil, fmt.Errorf("error during graphqlHandler.setup: %w", err)
 	}
 	return handler.issues, nil
 }
 
 func (handler *graphqlHandler) isArchived() (bool, error) {
+	if !strings.EqualFold(handler.repourl.commitSHA, clients.HeadSHA) {
+		return false, fmt.Errorf("%w: IsArchived only supported for HEAD queries", clients.ErrUnsupportedFeature)
+	}
 	if err := handler.setup(); err != nil {
 		return false, fmt.Errorf("error during graphqlHandler.setup: %w", err)
 	}