diff --git a/checks/active.go b/checks/active.go index f54d0439..01acfa76 100644 --- a/checks/active.go +++ b/checks/active.go @@ -18,8 +18,6 @@ import ( "fmt" "time" - "github.com/google/go-github/v32/github" - "github.com/ossf/scorecard/v2/checker" sce "github.com/ossf/scorecard/v2/errors" ) @@ -37,7 +35,15 @@ func init() { } func IsActive(c *checker.CheckRequest) checker.CheckResult { - commits, _, err := c.Client.Repositories.ListCommits(c.Ctx, c.Owner, c.Repo, &github.CommitsListOptions{}) + archived, err := c.RepoClient.IsArchived() + if err != nil { + return checker.CreateRuntimeErrorResult(CheckActive, err) + } + if archived { + return checker.CreateMinScoreResult(CheckActive, "repo is marked as archived") + } + + commits, err := c.RepoClient.ListCommits() if err != nil { return checker.CreateRuntimeErrorResult(CheckActive, err) } @@ -50,15 +56,10 @@ func IsActive(c *checker.CheckRequest) checker.CheckResult { threshold := time.Now().In(tz).AddDate(0, 0, -1*lookBackDays) totalCommits := 0 for _, commit := range commits { - commitFull, _, err := c.Client.Git.GetCommit(c.Ctx, c.Owner, c.Repo, commit.GetSHA()) - if err != nil { - return checker.CreateRuntimeErrorResult(CheckActive, err) - } - if commitFull.GetAuthor().GetDate().After(threshold) { + if commit.CommittedDate.After(threshold) { totalCommits++ } } - return checker.CreateProportionalScoreResult(CheckActive, fmt.Sprintf("%d commit(s) found in the last %d days", totalCommits, lookBackDays), totalCommits, commitsPerWeek*lookBackDays/daysInOneWeek) diff --git a/checks/code_review.go b/checks/code_review.go index cc861cc4..4a4ffb34 100644 --- a/checks/code_review.go +++ b/checks/code_review.go @@ -18,8 +18,6 @@ import ( "fmt" "strings" - "github.com/google/go-github/v32/github" - "github.com/ossf/scorecard/v2/checker" sce "github.com/ossf/scorecard/v2/errors" ) @@ -167,9 +165,9 @@ func prowCodeReview(c *checker.CheckRequest) (int, string, error) { //nolint func commitMessageHints(c *checker.CheckRequest) (int, string, error) { - commits, _, err := c.Client.Repositories.ListCommits(c.Ctx, c.Owner, c.Repo, &github.CommitsListOptions{}) + commits, err := c.RepoClient.ListCommits() if err != nil { - //nolint + // nolint: wraperror return checker.InconclusiveResultScore, "", sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("Client.Repositories.ListCommits: %v", err)) } @@ -178,7 +176,7 @@ func commitMessageHints(c *checker.CheckRequest) (int, string, error) { totalReviewed := 0 for _, commit := range commits { isBot := false - committer := commit.GetCommitter().GetLogin() + committer := commit.Committer.Login for _, substring := range []string{"bot", "gardener"} { if strings.Contains(committer, substring) { isBot = true @@ -193,10 +191,10 @@ func commitMessageHints(c *checker.CheckRequest) (int, string, error) { total++ // check for gerrit use via Reviewed-on and Reviewed-by - commitMessage := commit.GetCommit().GetMessage() + commitMessage := commit.Message if strings.Contains(commitMessage, "\nReviewed-on: ") && strings.Contains(commitMessage, "\nReviewed-by: ") { - c.Dlogger.Debug("Gerrit review found for commit '%s'", commit.GetCommit().GetSHA()) + c.Dlogger.Debug("Gerrit review found for commit '%s'", commit.SHA) totalReviewed++ continue } diff --git a/checks/vulnerabilities.go b/checks/vulnerabilities.go index bf5db8e3..cc5d6c0f 100644 --- a/checks/vulnerabilities.go +++ b/checks/vulnerabilities.go @@ -21,8 +21,6 @@ import ( "net/http" "strings" - "github.com/google/go-github/v32/github" - "github.com/ossf/scorecard/v2/checker" sce "github.com/ossf/scorecard/v2/errors" ) @@ -57,22 +55,18 @@ func (resp *osvResponse) getVulnerabilities() []string { } func HasUnfixedVulnerabilities(c *checker.CheckRequest) checker.CheckResult { - commits, _, err := c.Client.Repositories.ListCommits(c.Ctx, c.Owner, c.Repo, &github.CommitsListOptions{ - ListOptions: github.ListOptions{ - PerPage: 1, - }, - }) + commits, err := c.RepoClient.ListCommits() if err != nil { e := sce.Create(sce.ErrScorecardInternal, "Client.Repositories.ListCommits") return checker.CreateRuntimeErrorResult(CheckVulnerabilities, e) } - if len(commits) != 1 || commits[0].SHA == nil { + if len(commits) < 1 || commits[0].SHA == "" { return checker.CreateInconclusiveResult(CheckVulnerabilities, "no commits found") } query, err := json.Marshal(&osvQuery{ - Commit: *commits[0].SHA, + Commit: commits[0].SHA, }) if err != nil { e := sce.Create(sce.ErrScorecardInternal, "json.Marshal") diff --git a/clients/branch_ref.go b/clients/branch_ref.go new file mode 100644 index 00000000..bae5ca7c --- /dev/null +++ b/clients/branch_ref.go @@ -0,0 +1,24 @@ +// 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 clients + +type BranchRef struct { + Name string + BranchProtectionRule BranchProtectionRule +} + +type BranchProtectionRule struct { + RequiredApprovingReviewCount int +} diff --git a/clients/commit.go b/clients/commit.go new file mode 100644 index 00000000..6afea3ee --- /dev/null +++ b/clients/commit.go @@ -0,0 +1,28 @@ +// 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 clients + +import "time" + +type Commit struct { + CommittedDate time.Time + Message string + SHA string + Committer User +} + +type User struct { + Login string +} diff --git a/clients/githubrepo/client.go b/clients/githubrepo/client.go index bbbae59d..2e8e8ef0 100644 --- a/clients/githubrepo/client.go +++ b/clients/githubrepo/client.go @@ -66,6 +66,14 @@ func (client *Client) ListMergedPRs() ([]clients.PullRequest, error) { return client.graphClient.getMergedPRs() } +func (client *Client) ListCommits() ([]clients.Commit, error) { + return client.graphClient.getCommits() +} + +func (client *Client) IsArchived() (bool, error) { + return client.graphClient.isArchived() +} + func (client *Client) GetDefaultBranch() (clients.BranchRef, error) { return client.graphClient.getDefaultBranch() } diff --git a/clients/githubrepo/graphql.go b/clients/githubrepo/graphql.go index 9279ee4e..7fbb670c 100644 --- a/clients/githubrepo/graphql.go +++ b/clients/githubrepo/graphql.go @@ -28,16 +28,34 @@ const ( pullRequestsToAnalyze = 30 reviewsToAnalyze = 30 labelsToAnalyze = 30 + commitsToAnalyze = 30 ) // nolint: govet type graphqlData struct { Repository struct { + IsArchived githubv4.Boolean DefaultBranchRef struct { Name githubv4.String BranchProtectionRule struct { RequiredApprovingReviewCount githubv4.Int } + Target struct { + Commit struct { + History struct { + Nodes []struct { + CommittedDate githubv4.DateTime + Message githubv4.String + Oid githubv4.GitObjectID + Committer struct { + User struct { + Login githubv4.String + } + } + } + } `graphql:"history(first: $commitsToAnalyze)"` + } `graphql:"... on Commit"` + } } PullRequests struct { Nodes []struct { @@ -65,7 +83,9 @@ type graphqlHandler struct { client *githubv4.Client data *graphqlData prs []clients.PullRequest + commits []clients.Commit defaultBranchRef clients.BranchRef + archived bool } func (handler *graphqlHandler) init(ctx context.Context, owner, repo string) error { @@ -75,14 +95,17 @@ func (handler *graphqlHandler) init(ctx context.Context, owner, repo string) err "pullRequestsToAnalyze": githubv4.Int(pullRequestsToAnalyze), "reviewsToAnalyze": githubv4.Int(reviewsToAnalyze), "labelsToAnalyze": githubv4.Int(labelsToAnalyze), + "commitsToAnalyze": githubv4.Int(commitsToAnalyze), } handler.data = new(graphqlData) if err := handler.client.Query(ctx, handler.data, vars); err != nil { // nolint: wrapcheck return sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("githubv4.Query: %v", err)) } - handler.prs = pullRequestFrom(*handler.data) - handler.defaultBranchRef = defaultBranchRefFrom(*handler.data) + handler.archived = bool(handler.data.Repository.IsArchived) + handler.prs = pullRequestFrom(handler.data) + handler.defaultBranchRef = defaultBranchRefFrom(handler.data) + handler.commits = commitsFrom(handler.data) return nil } @@ -94,7 +117,15 @@ func (handler *graphqlHandler) getDefaultBranch() (clients.BranchRef, error) { return handler.defaultBranchRef, nil } -func pullRequestFrom(data graphqlData) []clients.PullRequest { +func (handler *graphqlHandler) getCommits() ([]clients.Commit, error) { + return handler.commits, nil +} + +func (handler *graphqlHandler) isArchived() (bool, error) { + return handler.archived, nil +} + +func pullRequestFrom(data *graphqlData) []clients.PullRequest { ret := make([]clients.PullRequest, len(data.Repository.PullRequests.Nodes)) for i, pr := range data.Repository.PullRequests.Nodes { toAppend := clients.PullRequest{ @@ -119,7 +150,7 @@ func pullRequestFrom(data graphqlData) []clients.PullRequest { return ret } -func defaultBranchRefFrom(data graphqlData) clients.BranchRef { +func defaultBranchRefFrom(data *graphqlData) clients.BranchRef { return clients.BranchRef{ Name: string(data.Repository.DefaultBranchRef.Name), BranchProtectionRule: clients.BranchProtectionRule{ @@ -128,3 +159,18 @@ func defaultBranchRefFrom(data graphqlData) clients.BranchRef { }, } } + +func commitsFrom(data *graphqlData) []clients.Commit { + ret := make([]clients.Commit, 0) + for _, commit := range data.Repository.DefaultBranchRef.Target.Commit.History.Nodes { + ret = append(ret, clients.Commit{ + CommittedDate: commit.CommittedDate.Time, + Message: string(commit.Message), + SHA: string(commit.Oid), + Committer: clients.User{ + Login: string(commit.Committer.User.Login), + }, + }) + } + return ret +} diff --git a/clients/pull_request.go b/clients/pull_request.go new file mode 100644 index 00000000..bb4fb771 --- /dev/null +++ b/clients/pull_request.go @@ -0,0 +1,40 @@ +// 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 clients + +import ( + "time" +) + +// nolint: govet +type PullRequest struct { + MergedAt time.Time + MergeCommit MergeCommit + Number int + Labels []Label + Reviews []Review +} + +type MergeCommit struct { + AuthoredByCommitter bool +} + +type Label struct { + Name string +} + +type Review struct { + State string +} diff --git a/clients/repo_client.go b/clients/repo_client.go index 73fa133d..c42ed858 100644 --- a/clients/repo_client.go +++ b/clients/repo_client.go @@ -16,7 +16,6 @@ package clients import ( "fmt" - "time" ) // UPGRADEv2: use ErrRepoUnreachable instead. @@ -40,39 +39,11 @@ func NewRepoUnavailableError(err error) error { type RepoClient interface { InitRepo(owner, repo string) error + IsArchived() (bool, error) ListFiles(predicate func(string) (bool, error)) ([]string, error) GetFileContent(filename string) ([]byte, error) ListMergedPRs() ([]PullRequest, error) GetDefaultBranch() (BranchRef, error) + ListCommits() ([]Commit, error) Close() error } - -type BranchRef struct { - Name string - BranchProtectionRule BranchProtectionRule -} - -type BranchProtectionRule struct { - RequiredApprovingReviewCount int -} - -// nolint: govet -type PullRequest struct { - MergedAt time.Time - MergeCommit MergeCommit - Number int - Labels []Label - Reviews []Review -} - -type MergeCommit struct { - AuthoredByCommitter bool -} - -type Label struct { - Name string -} - -type Review struct { - State string -} diff --git a/e2e/active_test.go b/e2e/active_test.go index c0e8dbea..2f07e35b 100644 --- a/e2e/active_test.go +++ b/e2e/active_test.go @@ -23,6 +23,7 @@ import ( "github.com/ossf/scorecard/v2/checker" "github.com/ossf/scorecard/v2/checks" + "github.com/ossf/scorecard/v2/clients/githubrepo" scut "github.com/ossf/scorecard/v2/utests" ) @@ -30,11 +31,14 @@ var _ = Describe("E2E TEST:Active", func() { Context("E2E TEST:Validating active status", func() { It("Should return valid active status", func() { dl := scut.TestDetailLogger{} + repoClient := githubrepo.CreateGithubRepoClient(context.Background(), ghClient, graphClient) + err := repoClient.InitRepo("apache", "airflow") + Expect(err).Should(BeNil()) req := checker.CheckRequest{ Ctx: context.Background(), Client: ghClient, HTTPClient: httpClient, - RepoClient: nil, + RepoClient: repoClient, Owner: "apache", Repo: "airflow", GraphClient: graphClient,