diff --git a/checker/raw_result.go b/checker/raw_result.go index 3ffa5503548..fd58dd7223e 100644 --- a/checker/raw_result.go +++ b/checker/raw_result.go @@ -22,6 +22,13 @@ type RawResults struct { SecurityPolicyResults SecurityPolicyData DependencyUpdateToolResults DependencyUpdateToolData BranchProtectionResults BranchProtectionsData + CodeReviewResults CodeReviewData +} + +// CodeReviewData contains the raw results +// for the Code-Review check. +type CodeReviewData struct { + DefaultBranchCommits []DefaultBranchCommit } // VulnerabilitiesData contains the raw results @@ -84,7 +91,7 @@ type Tool struct { Runs []Run // Issues created by the tool. Issues []Issue - // Merges requests created by the tool. + // Merge requests created by the tool. MergeRequests []MergeRequest Name string URL string @@ -104,10 +111,36 @@ type Issue struct { // TODO: add fields, e.g., state=[opened|closed] } +// DefaultBranchCommit represents a commit +// to the default branch. +type DefaultBranchCommit struct { + // Fields below are taken directly from cloud + // version control systems, e.g. GitHub. + SHA string + CommitMessage string + MergeRequest *MergeRequest + Committer User +} + // MergeRequest represents a merge request. +//nolint:govet type MergeRequest struct { - URL string - // TODO: add fields, e.g., State=["merged"|"closed"] + Number int + Labels []string + Reviews []Review + Author User +} + +// Review represent a review using the built-in review system. +type Review struct { + Reviewer User + State string + // TODO(Review): add fields here if needed. +} + +// User represent a user. +type User struct { + Login string } // File represents a file. diff --git a/checks/code_review.go b/checks/code_review.go index 089fe7d6f2d..c4491b272b1 100644 --- a/checks/code_review.go +++ b/checks/code_review.go @@ -15,10 +15,9 @@ package checks import ( - "fmt" - "strings" - "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/checks/evaluation" + "github.com/ossf/scorecard/v4/checks/raw" sce "github.com/ossf/scorecard/v4/errors" ) @@ -27,196 +26,26 @@ const CheckCodeReview = "Code-Review" //nolint:gochecknoinits func init() { - if err := registerCheck(CheckCodeReview, DoesCodeReview); err != nil { + if err := registerCheck(CheckCodeReview, CodeReview); err != nil { // this should never happen panic(err) } } -// DoesCodeReview attempts to determine whether a project requires review before code gets merged. -// It uses a set of heuristics: -// - Looking at the repo configuration to see if reviews are required. -// - Checking if most of the recent merged PRs were "Approved". -// - Looking for other well-known review labels. -func DoesCodeReview(c *checker.CheckRequest) checker.CheckResult { - // GitHub reviews. - ghScore, ghReason, err := githubCodeReview(c) - if err != nil { - return checker.CreateRuntimeErrorResult(CheckCodeReview, err) - } - - // Review messages. - hintScore, hintReason, err := commitMessageHints(c) - if err != nil { - return checker.CreateRuntimeErrorResult(CheckCodeReview, err) - } - - score, reason := selectBestScoreAndReason(hintScore, ghScore, hintReason, ghReason, c.Dlogger) - - // Prow CI/CD. - prowScore, prowReason, err := prowCodeReview(c) - if err != nil { - return checker.CreateRuntimeErrorResult(CheckCodeReview, err) - } - - score, reason = selectBestScoreAndReason(prowScore, score, prowReason, reason, c.Dlogger) - if score == checker.MinResultScore { - c.Dlogger.Info3(&checker.LogMessage{ - Text: reason, - }) - return checker.CreateResultWithScore(CheckCodeReview, "no reviews detected", score) - } - - if score == checker.InconclusiveResultScore { - return checker.CreateInconclusiveResult(CheckCodeReview, "no reviews detected") - } - - return checker.CreateResultWithScore(CheckCodeReview, checker.NormalizeReason(reason, score), score) -} - -//nolint -func selectBestScoreAndReason(s1, s2 int, r1, r2 string, - dl checker.DetailLogger) (int, string) { - if s1 > s2 { - dl.Info3(&checker.LogMessage{ - Text: r2, - }) - return s1, r1 - } - - dl.Info3(&checker.LogMessage{ - Text: r1, - }) - return s2, r2 -} - -//nolint -func githubCodeReview(c *checker.CheckRequest) (int, string, error) { - // Look at some merged PRs to see if they were reviewed. - totalMerged := 0 - totalReviewed := 0 - prs, err := c.RepoClient.ListMergedPRs() +// CodeReview will check if the maintainers perform code review. +func CodeReview(c *checker.CheckRequest) checker.CheckResult { + rawData, err := raw.CodeReview(c.RepoClient) if err != nil { - return 0, "", sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("RepoClient.ListMergedPRs: %v", err)) + e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + return checker.CreateRuntimeErrorResult(CheckCodeReview, e) } - for _, pr := range prs { - if pr.MergedAt.IsZero() { - continue - } - totalMerged++ - - // Check if the PR is approved by a reviewer. - foundApprovedReview := false - for _, r := range pr.Reviews { - if r.State == "APPROVED" { - c.Dlogger.Debug3(&checker.LogMessage{ - Text: fmt.Sprintf("found review approved pr: %d", pr.Number), - }) - totalReviewed++ - foundApprovedReview = true - break - } - } - - // Check if the PR is committed by someone other than author. this is kind - // of equivalent to a review and is done several times on small prs to save - // time on clicking the approve button. - if !foundApprovedReview && - pr.MergeCommit.Committer.Login != "" && - pr.MergeCommit.Committer.Login != pr.Author.Login { - c.Dlogger.Debug3(&checker.LogMessage{ - Text: fmt.Sprintf("found PR#%d with committer (%s) different from author (%s)", - pr.Number, pr.Author.Login, pr.MergeCommit.Committer.Login), - }) - totalReviewed++ - foundApprovedReview = true - } - - if !foundApprovedReview { - c.Dlogger.Debug3(&checker.LogMessage{ - Text: fmt.Sprintf("merged PR without code review: %d", pr.Number), - }) - } - - } - - return createReturn("GitHub", totalReviewed, totalMerged) -} - -//nolint -func prowCodeReview(c *checker.CheckRequest) (int, string, error) { - // Look at some merged PRs to see if they were reviewed - totalMerged := 0 - totalReviewed := 0 - prs, err := c.RepoClient.ListMergedPRs() - if err != nil { - sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("RepoClient.ListMergedPRs: %v", err)) - } - for _, pr := range prs { - if pr.MergedAt.IsZero() { - continue - } - totalMerged++ - for _, l := range pr.Labels { - if l.Name == "lgtm" || l.Name == "approved" { - totalReviewed++ - break - } - } - } - - return createReturn("Prow", totalReviewed, totalMerged) -} - -//nolint -func commitMessageHints(c *checker.CheckRequest) (int, string, error) { - commits, err := c.RepoClient.ListCommits() - if err != nil { - return checker.InconclusiveResultScore, "", - sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("Client.Repositories.ListCommits: %v", err)) - } - - total := 0 - totalReviewed := 0 - for _, commit := range commits { - isBot := false - committer := commit.Committer.Login - for _, substring := range []string{"bot", "gardener"} { - if strings.Contains(committer, substring) { - isBot = true - break - } - } - if isBot { - c.Dlogger.Debug3(&checker.LogMessage{ - Text: fmt.Sprintf("skip commit from bot account: %s", committer), - }) - continue - } - - total++ - - // check for gerrit use via Reviewed-on and Reviewed-by - commitMessage := commit.Message - if strings.Contains(commitMessage, "\nReviewed-on: ") && - strings.Contains(commitMessage, "\nReviewed-by: ") { - c.Dlogger.Debug3(&checker.LogMessage{ - Text: fmt.Sprintf("Gerrit review found for commit '%s'", commit.SHA), - }) - totalReviewed++ - continue - } - } - - return createReturn("Gerrit", totalReviewed, total) -} -//nolint -func createReturn(reviewName string, reviewed, total int) (int, string, error) { - if total > 0 { - reason := fmt.Sprintf("%s code reviews found for %v commits out of the last %v", reviewName, reviewed, total) - return checker.CreateProportionalScore(reviewed, total), reason, nil + // Return raw results. + if c.RawResults != nil { + c.RawResults.CodeReviewResults = rawData + return checker.CheckResult{} } - return checker.InconclusiveResultScore, fmt.Sprintf("no %v commits found", reviewName), nil + // Return the score evaluation. + return evaluation.CodeReview(CheckCodeReview, c.Dlogger, &rawData) } diff --git a/checks/code_review_test.go b/checks/code_review_test.go index 9ddc251b752..b00bb8d1d53 100644 --- a/checks/code_review_test.go +++ b/checks/code_review_test.go @@ -69,7 +69,7 @@ func TestCodereview(t *testing.T) { }, }, { - name: "Valid PR's and commits as not a bot", + name: "Valid GitHub PR", prs: []clients.PullRequest{ { Number: 1, @@ -79,11 +79,39 @@ func TestCodereview(t *testing.T) { State: "APPROVED", }, }, + MergeCommit: clients.Commit{ + SHA: "sha", + }, + }, + }, + commits: []clients.Commit{ + { + SHA: "sha", + Committer: clients.User{ + Login: "user", + }, + }, + }, + + expected: checker.CheckResult{ + Score: 10, + Pass: true, + }, + }, + { + name: "Valid Prow PR as not a bot", + prs: []clients.PullRequest{ + { + Number: 1, + MergedAt: time.Now(), Labels: []clients.Label{ { Name: "lgtm", }, }, + MergeCommit: clients.Commit{ + SHA: "sha", + }, }, }, commits: []clients.Commit{ @@ -100,23 +128,20 @@ func TestCodereview(t *testing.T) { Pass: true, }, }, - { - name: "Valid PR's and commits as bot", + name: "Valid Prow PR and commits as bot", prs: []clients.PullRequest{ { Number: 1, MergedAt: time.Now(), - Reviews: []clients.Review{ - { - State: "APPROVED", - }, - }, Labels: []clients.Label{ { Name: "lgtm", }, }, + MergeCommit: clients.Commit{ + SHA: "sha", + }, }, }, commits: []clients.Commit{ @@ -133,56 +158,52 @@ func TestCodereview(t *testing.T) { Pass: true, }, }, - { - name: "Valid PR's and commits not yet merged", + name: "Valid PR's and commits with merged by someone else", prs: []clients.PullRequest{ { Number: 1, - Reviews: []clients.Review{ - { - State: "APPROVED", - }, - }, Labels: []clients.Label{ { Name: "lgtm", }, }, + MergeCommit: clients.Commit{ + SHA: "sha", + Committer: clients.User{ + Login: "bot", + }, + }, }, }, commits: []clients.Commit{ { SHA: "sha", Committer: clients.User{ - Login: "bot", + Login: "bob", }, }, }, expected: checker.CheckResult{ - Score: -1, + Score: 0, }, }, { - name: "Valid PR's and commits with merged by someone else", + name: "2 PRs 2 review on GitHub", prs: []clients.PullRequest{ { - Number: 1, + Number: 1, + MergedAt: time.Now(), Reviews: []clients.Review{ { State: "APPROVED", }, }, - Labels: []clients.Label{ - { - Name: "lgtm", - }, - }, MergeCommit: clients.Commit{ SHA: "sha", Committer: clients.User{ - Login: "bob", + Login: "bot", }, }, }, @@ -191,13 +212,19 @@ func TestCodereview(t *testing.T) { { SHA: "sha", Committer: clients.User{ - Login: "bot", + Login: "bob", + }, + }, + { + SHA: "sha2", + Committer: clients.User{ + Login: "bob", }, }, }, expected: checker.CheckResult{ - Score: -1, + Score: 5, }, }, } @@ -225,7 +252,7 @@ func TestCodereview(t *testing.T) { RepoClient: mockRepo, } req.Dlogger = &scut.TestDetailLogger{} - res := DoesCodeReview(&req) + res := CodeReview(&req) if tt.err != nil { if res.Error2 == nil { diff --git a/checks/evaluation/code_review.go b/checks/evaluation/code_review.go new file mode 100644 index 00000000000..7514eb43e1e --- /dev/null +++ b/checks/evaluation/code_review.go @@ -0,0 +1,197 @@ +// 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 evaluation + +import ( + "fmt" + "strings" + + "github.com/ossf/scorecard/v4/checker" + sce "github.com/ossf/scorecard/v4/errors" +) + +var ( + reviewPlatformGitHub = "GitHub" + reviewPlatformProw = "Prow" + reviewPlatformGerrit = "Gerrit" +) + +// CodeReview applies the score policy for the Code-Review check. +func CodeReview(name string, dl checker.DetailLogger, + r *checker.CodeReviewData) checker.CheckResult { + if r == nil { + e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data") + return checker.CreateRuntimeErrorResult(name, e) + } + + totalCommits := 0 + totalReviewed := map[string]int{ + // The 3 platforms we support. + reviewPlatformGitHub: 0, + reviewPlatformProw: 0, + reviewPlatformGerrit: 0, + } + + for i := range r.DefaultBranchCommits { + commit := r.DefaultBranchCommits[i] + + // New commit to consider. + totalCommits++ + + rs := getApprovedReviewSystem(&commit, dl) + // No commits. + if rs == "" { + continue + } + + totalReviewed[rs]++ + } + + if totalCommits == 0 { + return checker.CreateInconclusiveResult(name, "no commits found") + } + + if totalReviewed[reviewPlatformGitHub] == 0 && + totalReviewed[reviewPlatformGerrit] == 0 && + totalReviewed[reviewPlatformProw] == 0 { + // Only show all warnings if all fail. + // We should not show warning if at least one succeeds, as this is confusing. + for k := range totalReviewed { + dl.Warn3(&checker.LogMessage{ + Text: fmt.Sprintf("no %s reviews found", k), + }) + } + + return checker.CreateMinScoreResult(name, "no reviews found") + } + + // Consider a single review system. + nbReviews, reviewSystem := computeReviews(totalReviewed) + if nbReviews == totalCommits { + return checker.CreateMaxScoreResult(name, + fmt.Sprintf("all last %v commits are reviewed through %s", totalCommits, reviewSystem)) + } + + reason := fmt.Sprintf("%s code reviews found for %v commits out of the last %v", reviewSystem, nbReviews, totalCommits) + return checker.CreateProportionalScoreResult(name, reason, nbReviews, totalCommits) +} + +func computeReviews(m map[string]int) (int, string) { + n := 0 + s := "" + for k, v := range m { + if v > n { + n = v + s = k + } + } + return n, s +} + +func isBot(name string) bool { + for _, substring := range []string{"bot", "gardener"} { + if strings.Contains(name, substring) { + return true + } + } + return false +} + +func getApprovedReviewSystem(c *checker.DefaultBranchCommit, dl checker.DetailLogger) string { + switch { + case isReviewedOnGitHub(c, dl): + return reviewPlatformGitHub + + case isReviewedOnProw(c, dl): + return reviewPlatformProw + + case isReviewedOnGerrit(c, dl): + return reviewPlatformGerrit + } + + return "" +} + +func isReviewedOnGitHub(c *checker.DefaultBranchCommit, dl checker.DetailLogger) bool { + mr := c.MergeRequest + if mr == nil { + return false + } + + for _, r := range mr.Reviews { + if r.State == "APPROVED" { + dl.Debug3(&checker.LogMessage{ + Text: fmt.Sprintf("%s #%d merge request approved", + reviewPlatformGitHub, mr.Number), + }) + return true + } + } + + // Check if the merge request is committed by someone other than author. This is kind + // of equivalent to a review and is done several times on small prs to save + // time on clicking the approve button. + if c.Committer.Login != "" && + c.Committer.Login != mr.Author.Login { + dl.Debug3(&checker.LogMessage{ + Text: fmt.Sprintf("%s #%d merge request approved", + reviewPlatformGitHub, mr.Number), + }) + return true + } + + return false +} + +func isReviewedOnProw(c *checker.DefaultBranchCommit, dl checker.DetailLogger) bool { + if isBot(c.Committer.Login) { + dl.Debug3(&checker.LogMessage{ + Text: fmt.Sprintf("skip commit from bot account: %s", c.Committer.Login), + }) + return true + } + + if c.MergeRequest != nil { + for _, l := range c.MergeRequest.Labels { + if l == "lgtm" || l == "approved" { + dl.Debug3(&checker.LogMessage{ + Text: fmt.Sprintf("%s #%d merge request approved", + reviewPlatformProw, c.MergeRequest.Number), + }) + return true + } + } + } + return false +} + +func isReviewedOnGerrit(c *checker.DefaultBranchCommit, dl checker.DetailLogger) bool { + if isBot(c.Committer.Login) { + dl.Debug3(&checker.LogMessage{ + Text: fmt.Sprintf("skip commit from bot account: %s", c.Committer.Login), + }) + return true + } + + m := c.CommitMessage + if strings.Contains(m, "\nReviewed-on: ") && + strings.Contains(m, "\nReviewed-by: ") { + dl.Debug3(&checker.LogMessage{ + Text: fmt.Sprintf("%s commit approved", reviewPlatformGerrit), + }) + return true + } + return false +} diff --git a/checks/raw/code_review.go b/checks/raw/code_review.go new file mode 100644 index 00000000000..07f88e3d984 --- /dev/null +++ b/checks/raw/code_review.go @@ -0,0 +1,133 @@ +// Copyright 2020 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 raw + +import ( + "fmt" + + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/clients" +) + +// CodeReview retrieves the raw data for the Code-Review check. +func CodeReview(c clients.RepoClient) (checker.CodeReviewData, error) { + results := []checker.DefaultBranchCommit{} + + // Look at the latest commits. + commits, err := c.ListCommits() + if err != nil { + return checker.CodeReviewData{}, fmt.Errorf("%w", err) + } + + oc := make(map[string]checker.DefaultBranchCommit) + for _, commit := range commits { + com := commitRequest(commit) + // Keep an index of commits by SHA. + oc[commit.SHA] = com + } + + // Look at merge requests. + mrs, err := c.ListMergedPRs() + if err != nil { + return checker.CodeReviewData{}, fmt.Errorf("%w", err) + } + + for i := range mrs { + mr := mrs[i] + + if mr.MergedAt.IsZero() { + continue + } + + // If the merge request is not a recent commit, skip. + com, exists := oc[mr.MergeCommit.SHA] + + if exists { + // Sanity checks the logins are the same. + // TODO(#1543): re-enable this code. + //nolint:gocritic + /*if com.Committer.Login != mr.MergeCommit.Committer.Login { + return checker.CodeReviewData{}, sce.WithMessage(sce.ErrScorecardInternal, + fmt.Sprintf("commit login (%s) different from merge request commit login (%s)", + com.Committer.Login, mr.MergeCommit.Committer.Login)) + }*/ + + // We have a recent merge request, update it. + com.MergeRequest = mergeRequest(&mr) + oc[com.SHA] = com + } + } + + for _, v := range oc { + results = append(results, v) + } + + if len(results) > 0 { + return checker.CodeReviewData{DefaultBranchCommits: results}, nil + } + + return checker.CodeReviewData{DefaultBranchCommits: results}, nil +} + +func commitRequest(c clients.Commit) checker.DefaultBranchCommit { + r := checker.DefaultBranchCommit{ + Committer: checker.User{ + Login: c.Committer.Login, + }, + SHA: c.SHA, + CommitMessage: c.Message, + } + + return r +} + +func mergeRequest(mr *clients.PullRequest) *checker.MergeRequest { + r := checker.MergeRequest{ + Number: mr.Number, + Author: checker.User{ + Login: mr.Author.Login, + }, + + Labels: labels(mr), + Reviews: reviews(mr), + } + return &r +} + +func labels(mr *clients.PullRequest) []string { + labels := []string{} + for _, l := range mr.Labels { + labels = append(labels, l.Name) + } + return labels +} + +func reviews(mr *clients.PullRequest) []checker.Review { + reviews := []checker.Review{} + for _, m := range mr.Reviews { + r := checker.Review{ + State: m.State, + } + + if m.Author != nil && + m.Author.Login != "" { + r.Reviewer = checker.User{ + Login: m.Author.Login, + } + } + reviews = append(reviews, r) + } + return reviews +} diff --git a/clients/githubrepo/graphql.go b/clients/githubrepo/graphql.go index 63c7fce52f3..15b7de0ea31 100644 --- a/clients/githubrepo/graphql.go +++ b/clients/githubrepo/graphql.go @@ -84,7 +84,10 @@ type graphqlData struct { } `graphql:"labels(last: $labelsToAnalyze)"` Reviews struct { Nodes []struct { - State githubv4.String + State githubv4.String + Author struct { + Login githubv4.String + } } } `graphql:"reviews(last: $reviewsToAnalyze)"` } @@ -211,18 +214,29 @@ func pullRequestsFrom(data *graphqlData, repoOwner, repoName string) []clients.P Committer: clients.User{ Login: string(commit.Author.User.Login), }, + SHA: string(pr.MergeCommit.Oid), }, } + for _, label := range pr.Labels.Nodes { toAppend.Labels = append(toAppend.Labels, clients.Label{ Name: string(label.Name), }) } for _, review := range pr.Reviews.Nodes { - toAppend.Reviews = append(toAppend.Reviews, clients.Review{ + r := clients.Review{ State: string(review.State), - }) + } + + a := clients.User{ + Login: string(review.Author.Login), + } + if a.Login != "" { + r.Author = &a + } + toAppend.Reviews = append(toAppend.Reviews, r) } + ret = append(ret, toAppend) break } diff --git a/clients/pull_request.go b/clients/pull_request.go index a7ef3141c08..e4700f74098 100644 --- a/clients/pull_request.go +++ b/clients/pull_request.go @@ -37,5 +37,6 @@ type Label struct { // Review represents a PR review. type Review struct { - State string + Author *User + State string } diff --git a/e2e/code_review_test.go b/e2e/code_review_test.go index 8da5de5b006..d1b654f40fd 100644 --- a/e2e/code_review_test.go +++ b/e2e/code_review_test.go @@ -47,11 +47,11 @@ var _ = Describe("E2E TEST:CodeReview", func() { expected := scut.TestReturn{ Error: nil, Score: checker.MinResultScore, - NumberOfWarn: 0, - NumberOfInfo: 3, + NumberOfWarn: 3, + NumberOfInfo: 0, NumberOfDebug: 0, } - result := checks.DoesCodeReview(&req) + result := checks.CodeReview(&req) Expect(scut.ValidateTestReturn(nil, "use code reviews", &expected, &result, &dl)).Should(BeTrue()) Expect(repoClient.Close()).Should(BeNil()) }) diff --git a/pkg/json_raw_results.go b/pkg/json_raw_results.go index 649266e67e9..6744f896a26 100644 --- a/pkg/json_raw_results.go +++ b/pkg/json_raw_results.go @@ -64,6 +64,33 @@ type jsonBranchProtection struct { Name string `json:"name"` } +type jsonReview struct { + Reviewer jsonUser `json:"reviewer"` + State string `json:"state"` +} + +type jsonUser struct { + Login string `json:"login"` +} + +//nolint:govet +type jsonMergeRequest struct { + Number int `json:"number"` + Labels []string `json:"labels"` + Reviews []jsonReview `json:"reviews"` + Author jsonUser `json:"author"` +} + +type jsonDefaultBranchCommit struct { + // ApprovedReviews *jsonApprovedReviews `json:"approved-reviews"` + Committer jsonUser `json:"committer"` + MergeRequest *jsonMergeRequest `json:"merge-request"` + CommitMessage string `json:"commit-message"` + SHA string `json:"sha"` + + // TODO: check runs, etc. +} + type jsonRawResults struct { DatabaseVulnerabilities []jsonDatabaseVulnerability `json:"database-vulnerabilities"` // List of binaries found in the repo. @@ -76,6 +103,52 @@ type jsonRawResults struct { DependencyUpdateTools []jsonTool `json:"dependency-update-tools"` // Branch protection settings for development and release branches. BranchProtections []jsonBranchProtection `json:"branch-protections"` + // Commits. + DefaultBranchCommits []jsonDefaultBranchCommit `json:"default-branch-commits"` +} + +//nolint:unparam +func (r *jsonScorecardRawResult) addCodeReviewRawResults(cr *checker.CodeReviewData) error { + r.Results.DefaultBranchCommits = []jsonDefaultBranchCommit{} + for _, commit := range cr.DefaultBranchCommits { + com := jsonDefaultBranchCommit{ + Committer: jsonUser{ + Login: commit.Committer.Login, + }, + CommitMessage: commit.CommitMessage, + SHA: commit.SHA, + } + + // Merge request field. + if commit.MergeRequest != nil { + mr := jsonMergeRequest{ + Number: commit.MergeRequest.Number, + Author: jsonUser{ + Login: commit.MergeRequest.Author.Login, + }, + } + + if len(commit.MergeRequest.Labels) > 0 { + mr.Labels = commit.MergeRequest.Labels + } + + for _, r := range commit.MergeRequest.Reviews { + mr.Reviews = append(mr.Reviews, jsonReview{ + State: r.State, + Reviewer: jsonUser{ + Login: r.Reviewer.Login, + }, + }) + } + + com.MergeRequest = &mr + } + + com.CommitMessage = commit.CommitMessage + + r.Results.DefaultBranchCommits = append(r.Results.DefaultBranchCommits, com) + } + return nil } type jsonDatabaseVulnerability struct { @@ -195,6 +268,11 @@ func (r *jsonScorecardRawResult) fillJSONRawResults(raw *checker.RawResults) err return sce.WithMessage(sce.ErrScorecardInternal, err.Error()) } + // Code-Review. + if err := r.addCodeReviewRawResults(&raw.CodeReviewResults); err != nil { + return sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + } + return nil }