feat(branch-protection): consider if project requires PRs prior to ma…

…ke changes

As discussed at the issue ossf#2727, we're adding the "require PRs prior
to make changes" as another requirement to tier 2. In addition to that,
we're changing the weight of the tier 2 requirements so that
"requiring 1 reviewer" has weight 2, while the other tier 2 requirements
have weight 1

Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com>

checks/evaluation/branch_protection.go

@@ -50,6 +50,14 @@ type levelScore struct {
 	maxes  scoresInfo // Maximum possible score for a branch.
 }
 
+// Evaluates if Scorecard is being run with an administrator token.
+func isUserAdmin(branchProtectionData *clients.BranchRef) bool {
+	// When Scorecard is run without the admin token, Github retrieves both of the following fields as nil,
+	// so we're using them to evaluate if Scorecard is run using admin token or not.
+	return branchProtectionData.BranchProtectionRule.CheckRules.UpToDateBeforeMerge != nil ||
+		branchProtectionData.BranchProtectionRule.RequireLastPushApproval != nil
+}
+
 // BranchProtection runs Branch-Protection check.
 func BranchProtection(name string, dl checker.DetailLogger,
 	r *checker.BranchProtectionsData,
@@ -357,11 +365,11 @@ func nonAdminReviewProtection(branch *clients.BranchRef) (int, int) {
 	score := 0
 	max := 0
 
-	max++
+	max += 2
 	if branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount != nil &&
 		*branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount > 0 {
 		// We do not display anything here, it's done in nonAdminThoroughReviewProtection()
-		score++
+		score += 2
 	}
 	return score, max
 }
@@ -400,6 +408,18 @@ func adminReviewProtection(branch *clients.BranchRef, dl checker.DetailLogger) (
 		}
 	}
 
+	if isUserAdmin(branch) {
+		// If Scorecard is run with admin token, we can interprete GitHub's response to say
+		// if the branch requires PRs prior to code changes.
+		max++
+		if branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount != nil {
+			score++
+			info(dl, log, "PRs are required in order to make changes on branch '%s'", *branch.Name)
+		} else {
+			warn(dl, log, "PRs are not required in order to make changes on branch '%s'", *branch.Name)
+		}
+	}
+
 	return score, max
 }
 
@@ -433,19 +453,27 @@ func nonAdminThoroughReviewProtection(branch *clients.BranchRef, dl checker.Deta
 	log := branch.Protected != nil && *branch.Protected
 
 	max++
-	if branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount != nil {
-		switch *branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount >= minReviews {
-		case true:
+
+	// On this first check we exclude the case of PRs don't being required, covered on adminReviewProtection function
+	if !(isUserAdmin(branch) &&
+		branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount == nil) {
+		switch {
+		// If not running as admin, the nil value can both mean that no reviews are required or no PR are required,
+		// so here we assume no reviews are required.
+		case (!isUserAdmin(branch) &&
+			branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount == nil) ||
+			*branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount == 0:
+			warn(dl, log, "number of required reviewers is 0 on branch '%s'", *branch.Name)
+		case *branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount >= minReviews:
 			info(dl, log, "number of required reviewers is %d on branch '%s'",
 				*branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount, *branch.Name)
 			score++
 		default:
 			warn(dl, log, "number of required reviewers is only %d on branch '%s'",
 				*branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount, *branch.Name)
 		}
-	} else {
-		warn(dl, log, "number of required reviewers is 0 on branch '%s'", *branch.Name)
-	}
+	} 
+
 	return score, max
 }