✨ New probes: code-review

probes/codeApproved/def.yml

@@ -0,0 +1,21 @@
+# © 2023 Nokia
+# Licensed under the Apache License 2.0
+# SPDX-License-Identifier: Apache-2.0
+
+
+id: codeApproved
+short: Check that all recent changesets have been approved by someone who is not the author of the changeset.
+motivation: >
+  To ensure that the review process works, the proposed changes
+  should have a minimum number of approvals.
+implementation: >
+  The implementation looks for the reviews of a changeset and that their state is "APPROVED"
+outcome:
+  - If all changesets were approved, the probe returns OutcomePositive (1)
+  - If a changeset was not approved, the prove returns OutcomeNegative (0)
+remediation:
+  effort: Low
+  text:
+    - Follow the instructions https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/approving-a-pull-request-with-required-reviews.
+  markdown:
+    - Follow the instructions https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/approving-a-pull-request-with-required-reviews.

probes/codeApproved/impl.go

@@ -0,0 +1,61 @@
+// © 2023 Nokia
+// Licensed under the Apache License 2.0
+// SPDX-License-Identifier: Apache-2.0
+
+package codeApproved
+
+import (
+	"embed"
+	"fmt"
+
+	"github.com/ossf/scorecard/v4/checker"
+	"github.com/ossf/scorecard/v4/finding"
+)
+
+//go:embed *.yml
+var fs embed.FS
+
+const probe = "codeApproved"
+
+func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
+	rawReviewData := &raw.CodeReviewResults
+	return approvedRun(rawReviewData, fs, probe, finding.OutcomePositive, finding.OutcomeNegative)
+}
+
+/*
+** Looks through the data and validates that each changeset has been approved at least once.
+*/
+
+func approvedRun(reviewData *checker.CodeReviewData, fs embed.FS, probeID string,
+	positiveOutcome, negativeOutcome finding.Outcome,
+) ([]finding.Finding, string, error) {
+	var findings []finding.Finding
+	var approvedReviews = 0
+	changesets := reviewData.DefaultBranchChangesets
+	var numChangesets = len(changesets)
+	for x := range changesets {
+		data := &changesets[x]
+		for y := range data.Reviews {
+			if data.Reviews[y].State == "APPROVED" && data.Reviews[y].Author.Login != data.Author.Login {
+				approvedReviews += 1
+				break
+			}
+		}
+	}
+	if approvedReviews >= numChangesets {
+		f, err := finding.NewWith(fs, probeID, fmt.Sprintf("All changesets approved (%v out of %v).", approvedReviews, numChangesets),
+			nil, positiveOutcome)
+		if err != nil {
+			return nil, probeID, fmt.Errorf("create finding: %w", err)
+		}
+		findings = append(findings, *f)
+	} else {
+		f, err := finding.NewWith(fs, probeID, fmt.Sprintf("Not all changesets approved. Found %v approvals among %v changesets.", approvedReviews, numChangesets),
+			nil, negativeOutcome)
+		if err != nil {
+			return nil, probeID, fmt.Errorf("create finding: %w", err)
+		}
+		findings = append(findings, *f)
+	}
+	return findings, probeID, nil
+}

probes/codeReviewTwoReviewers/def.yml

@@ -0,0 +1,21 @@
+# © 2023 Nokia
+# Licensed under the Apache License 2.0
+# SPDX-License-Identifier: Apache-2.0
+
+id: codeReviewTwoReviewers
+short: Check that at least two reviewers review a change before merging.
+motivation: >
+  To ensure that the review process works, the proposed changes
+  should have a minimum number of approvals.
+implementation: >
+  The implementation looks for the number of reviewers for the last few changesets.
+  Only unique reviewer logins that aren't the same as the changeset author are counted.
+outcome:
+  - If all the changes had at least two reviewers, the probe returns OutcomePositive (1)
+  - If the changes had fewer than two reviewers, the prove returns OutcomeNegative (0)
+remediation:
+  effort: Low
+  text:
+    - Follow the instructions https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/approving-a-pull-request-with-required-reviews.
+  markdown:
+    - Follow the instructions https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/approving-a-pull-request-with-required-reviews.

probes/codeReviewTwoReviewers/impl.go

@@ -0,0 +1,110 @@
+// © 2023 Nokia
+// Licensed under the Apache License 2.0
+// SPDX-License-Identifier: Apache-2.0
+
+package codeReviewTwoReviewers
+
+import (
+	"embed"
+	"fmt"
+	"github.com/ossf/scorecard/v4/checker"
+	"github.com/ossf/scorecard/v4/finding"
+)
+
+//go:embed *.yml
+var fs embed.FS
+
+const probe = "codeReviewTwoReviewers"
+const minimumReviewers = 2
+const noReviewerFound = -1
+
+func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
+	rawReviewData := &raw.CodeReviewResults
+	return codeReviewRun(rawReviewData, fs, probe, finding.OutcomePositive, finding.OutcomeNegative)
+}
+
+/*
+** Looks through the data and validates author and reviewers of a changeset
+** Scorecard currently only supports GitHub revisions and generates a positive
+** score in the case of other platforms. This probe is created to ensure that
+** there are a number of unique reviewers for each changeset.
+ */
+
+func codeReviewRun(reviewData *checker.CodeReviewData, fs embed.FS, probeID string,
+	positiveOutcome, negativeOutcome finding.Outcome,
+) ([]finding.Finding, string, error) {
+	var findings []finding.Finding
+	leastFoundReviewers := 0
+	changesets := reviewData.DefaultBranchChangesets
+	for i := range changesets {
+		data := &changesets[i]
+		if data.Author.Login == "" {
+			return authorNotFound(findings, probeID, fs)
+		}
+		reviewersList := make([]string, len(data.Reviews))
+		for i := range data.Reviews {
+			reviewersList[i] = data.Reviews[i].Author.Login
+		}
+		numReviewers := uniqueReviewers(data.Author.Login, reviewersList)
+		if numReviewers == noReviewerFound {
+			return reviewerNotFound(findings, probeID, fs)
+		} else if i == 0 || numReviewers < leastFoundReviewers {
+			leastFoundReviewers = numReviewers
+		}
+	}
+	if leastFoundReviewers >= minimumReviewers {
+		f, err := finding.NewWith(fs, probeID, fmt.Sprintf("%v unique reviewers found for at least one changeset, %v wanted.", leastFoundReviewers, minimumReviewers),
+			nil, positiveOutcome)
+		if err != nil {
+			return nil, probeID, fmt.Errorf("create finding: %w", err)
+		}
+		findings = append(findings, *f)
+	} else {
+		f, err := finding.NewWith(fs, probeID, fmt.Sprintf("%v unique reviewer(s) found for at least one changeset, %v wanted.", leastFoundReviewers, minimumReviewers),
+			nil, negativeOutcome)
+		if err != nil {
+			return nil, probeID, fmt.Errorf("create finding: %w", err)
+		}
+		findings = append(findings, *f)
+	}
+	return findings, probeID, nil
+}
+
+func uniqueReviewers(authorLogin string, reviewers []string) int {
+	uniqueReviewers := 0
+	for i := range reviewers {
+		duplicateCount := 0
+		if (reviewers[i] == "") {
+			return -1
+		}
+		for j := range reviewers {
+			if reviewers[j] == reviewers[i] && j > i {
+				duplicateCount++
+			}
+		}
+		if reviewers[i] != authorLogin && duplicateCount == 0 {
+			uniqueReviewers++
+		}
+	}
+	return uniqueReviewers
+}
+
+func authorNotFound(findings []finding.Finding, probeID string,
+	fs embed.FS) ([]finding.Finding, string, error) {
+	f, err := finding.NewNotAvailable(fs, probeID, fmt.Sprintf("Could not retrieve the author of a changeset."), nil)
+	if err != nil {
+		return nil, probeID, fmt.Errorf("create finding: %w", err)
+	}
+	findings = append(findings, *f)
+	return findings, probeID, nil
+}
+
+func reviewerNotFound(findings []finding.Finding, probeID string,
+	fs embed.FS) ([]finding.Finding, string, error) {
+		f, err := finding.NewNegative(fs, probeID, fmt.Sprintf("Could not retrieve reviewers of a changeset."), nil)
+		if err != nil {
+			return nil, probeID, fmt.Errorf("create finding: %w", err)
+		}
+		findings = append(findings, *f)
+		return findings, probeID, nil
+}

probes/codeReviewed/def.yml

@@ -0,0 +1,20 @@
+# © 2023 Nokia
+# Licensed under the Apache License 2.0
+# SPDX-License-Identifier: Apache-2.0
+
+id: codeReviewed
+short: Check that there is some review activity for each changeset.
+motivation: >
+  To ensure that the review process works, the proposed changes
+  should have a minimum number of reviews.
+implementation: >
+  The implementation looks for the reviews of a changeset.
+outcome:
+  - If all changesets have review activity, the probe returns OutcomePositive (1)
+  - If a changeset has no review activity, the prove returns OutcomeNegative (0)
+remediation:
+  effort: Low
+  text:
+    - Follow the instructions https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/approving-a-pull-request-with-required-reviews.
+  markdown:
+    - Follow the instructions https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/approving-a-pull-request-with-required-reviews.

probes/codeReviewed/impl.go

@@ -0,0 +1,58 @@
+// © 2023 Nokia
+// Licensed under the Apache License 2.0
+// SPDX-License-Identifier: Apache-2.0
+
+package codeReviewed
+
+import (
+	"embed"
+	"fmt"
+
+	"github.com/ossf/scorecard/v4/checker"
+	"github.com/ossf/scorecard/v4/finding"
+)
+
+//go:embed *.yml
+var fs embed.FS
+
+const probe = "codeReviewed"
+
+func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
+	rawReviewData := &raw.CodeReviewResults
+	return reviewedRun(rawReviewData, fs, probe, finding.OutcomePositive, finding.OutcomeNegative)
+}
+
+/*
+** Looks through the data and validates that each changeset has been approved at least once.
+*/
+
+func reviewedRun(reviewData *checker.CodeReviewData, fs embed.FS, probeID string,
+	positiveOutcome, negativeOutcome finding.Outcome,
+) ([]finding.Finding, string, error) {
+	var findings []finding.Finding
+	var numReviews = 0
+	changesets := reviewData.DefaultBranchChangesets
+	var numChangesets = len(changesets)
+	for x := range changesets {
+		data := &changesets[x]
+		if len(data.Reviews) > 0 {
+			numReviews += 1
+		}
+	}
+	if numReviews >= numChangesets {
+		f, err := finding.NewWith(fs, probeID, fmt.Sprintf("All changesets have review activity (%v out of %v).", numReviews, numChangesets),
+			nil, positiveOutcome)
+		if err != nil {
+			return nil, probeID, fmt.Errorf("create finding: %w", err)
+		}
+		findings = append(findings, *f)
+	} else {
+		f, err := finding.NewWith(fs, probeID, fmt.Sprintf("Not all changesets have review activity. Found %v reviews among %v changesets.", numReviews, numChangesets),
+			nil, negativeOutcome)
+		if err != nil {
+			return nil, probeID, fmt.Errorf("create finding: %w", err)
+		}
+		findings = append(findings, *f)
+	}
+	return findings, probeID, nil
+}

probes/entries.go

@@ -17,6 +17,9 @@ package probes
 import (
 	"github.com/ossf/scorecard/v4/checker"
 	"github.com/ossf/scorecard/v4/finding"
+	"github.com/ossf/scorecard/v4/probes/codeApproved"
+	"github.com/ossf/scorecard/v4/probes/codeReviewed"
+	"github.com/ossf/scorecard/v4/probes/codeReviewTwoReviewers"
 	"github.com/ossf/scorecard/v4/probes/toolDependabotInstalled"
 	"github.com/ossf/scorecard/v4/probes/toolPyUpInstalled"
 	"github.com/ossf/scorecard/v4/probes/toolRenovateInstalled"
@@ -30,19 +33,25 @@ var (
 	// All represents all the probes.
 	All []ProbeImpl
 	// DependencyToolUpdates is all the probes for the
-	// DpendencyUpdateTool check.
+	// DependencyUpdateTool check.
 	DependencyToolUpdates = []ProbeImpl{
 		toolRenovateInstalled.Run,
 		toolDependabotInstalled.Run,
 		toolPyUpInstalled.Run,
 		toolSonatypeLiftInstalled.Run,
 	}
+	CodeReview = []ProbeImpl {
+		codeApproved.Run,
+		codeReviewed.Run,
+		codeReviewTwoReviewers.Run,
+	}
 )
 
 //nolint:gochecknoinits
 func init() {
 	All = concatMultipleProbes([][]ProbeImpl{
 		DependencyToolUpdates,
+		CodeReview,
 	})
 }