✨ Move "EnforcesAdmins" to tier 5 Branch-Protection

checks/branch_protection_test.go

@@ -134,7 +134,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) {
 			name: "Only development branch",
 			expected: scut.TestReturn{
 				Error:         nil,
-				Score:         2,
+				Score:         3,
 				NumberOfWarn:  7,
 				NumberOfInfo:  2,
 				NumberOfDebug: 0,
@@ -173,7 +173,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) {
 			name: "Take worst of release and development",
 			expected: scut.TestReturn{
 				Error:         nil,
-				Score:         2,
+				Score:         4,
 				NumberOfWarn:  9,
 				NumberOfInfo:  10,
 				NumberOfDebug: 0,
@@ -285,7 +285,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) {
 			name: "Ignore a non-branch targetcommitish",
 			expected: scut.TestReturn{
 				Error:         nil,
-				Score:         2,
+				Score:         3,
 				NumberOfWarn:  7,
 				NumberOfInfo:  2,
 				NumberOfDebug: 0,

checks/evaluation/branch_protection.go

@@ -25,7 +25,7 @@ import (
 const (
 	minReviews = 2
 	// Points incremented at each level.
-	adminNonAdminBasicLevel     = 3 // Level 1.
+	basicLevel                  = 3 // Level 1.
 	adminNonAdminReviewLevel    = 3 // Level 2.
 	nonAdminContextLevel        = 2 // Level 3.
 	nonAdminThoroughReviewLevel = 1 // Level 4.
@@ -35,7 +35,6 @@ const (
 
 type scoresInfo struct {
 	basic               int
-	adminBasic          int
 	review              int
 	adminReview         int
 	context             int
@@ -71,7 +70,6 @@ func BranchProtection(name string, dl checker.DetailLogger,
 			})
 		}
 		score.scores.basic, score.maxes.basic = basicNonAdminProtection(&b, dl)
-		score.scores.adminBasic, score.maxes.adminBasic = basicAdminProtection(&b, dl)
 		score.scores.review, score.maxes.review = nonAdminReviewProtection(&b)
 		score.scores.adminReview, score.maxes.adminReview = adminReviewProtection(&b, dl)
 		score.scores.context, score.maxes.context = nonAdminContextProtection(&b, dl)
@@ -114,15 +112,6 @@ func computeNonAdminBasicScore(scores []levelScore) int {
 	return score
 }
 
-func computeAdminBasicScore(scores []levelScore) int {
-	score := 0
-	for i := range scores {
-		s := scores[i]
-		score += s.scores.adminBasic
-	}
-	return score
-}
-
 func computeNonAdminReviewScore(scores []levelScore) int {
 	score := 0
 	for i := range scores {
@@ -194,12 +183,9 @@ func computeScore(scores []levelScore) (int, error) {
 
 	// First, check if they all pass the basic (admin and non-admin) checks.
 	maxBasicScore := maxScore.basic * len(scores)
-	maxAdminBasicScore := maxScore.adminBasic * len(scores)
 	basicScore := computeNonAdminBasicScore(scores)
-	adminBasicScore := computeAdminBasicScore(scores)
-	score += noarmalizeScore(basicScore+adminBasicScore, maxBasicScore+maxAdminBasicScore, adminNonAdminBasicLevel)
-	if basicScore != maxBasicScore ||
-		adminBasicScore != maxAdminBasicScore {
+	score += noarmalizeScore(basicScore, maxBasicScore, basicLevel)
+	if basicScore != maxBasicScore {
 		return int(score), nil
 	}
 
@@ -308,30 +294,6 @@ func basicNonAdminProtection(branch *clients.BranchRef, dl checker.DetailLogger)
 	return score, max
 }
 
-func basicAdminProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) {
-	score := 0
-	max := 0
-	// Only log information if the branch is protected.
-	log := branch.Protected != nil && *branch.Protected
-
-	// nil typically means we do not have access to the value.
-	if branch.BranchProtectionRule.EnforceAdmins != nil {
-		// Note: we don't inrecase max possible score for non-admin viewers.
-		max++
-		switch *branch.BranchProtectionRule.EnforceAdmins {
-		case true:
-			info(dl, log, "settings apply to administrators on branch '%s'", *branch.Name)
-			score++
-		case false:
-			warn(dl, log, "settings do not apply to administrators on branch '%s'", *branch.Name)
-		}
-	} else {
-		debug(dl, log, "unable to retrieve whether or not settings apply to administrators on branch '%s'", *branch.Name)
-	}
-
-	return score, max
-}
-
 func nonAdminContextProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) {
 	score := 0
 	max := 0
@@ -422,6 +384,22 @@ func adminThoroughReviewProtection(branch *clients.BranchRef, dl checker.DetailL
 	} else {
 		debug(dl, log, "unable to retrieve review dismissal on branch '%s'", *branch.Name)
 	}
+
+	// nil typically means we do not have access to the value.
+	if branch.BranchProtectionRule.EnforceAdmins != nil {
+		// Note: we don't inrecase max possible score for non-admin viewers.
+		max++
+		switch *branch.BranchProtectionRule.EnforceAdmins {
+		case true:
+			info(dl, log, "settings apply to administrators on branch '%s'", *branch.Name)
+			score++
+		case false:
+			warn(dl, log, "settings do not apply to administrators on branch '%s'", *branch.Name)
+		}
+	} else {
+		debug(dl, log, "unable to retrieve whether or not settings apply to administrators on branch '%s'", *branch.Name)
+	}
+
 	return score, max
 }
 

checks/evaluation/branch_protection_test.go

@@ -25,7 +25,6 @@ import (
 func testScore(branch *clients.BranchRef, codeownersFiles []string, dl checker.DetailLogger) (int, error) {
 	var score levelScore
 	score.scores.basic, score.maxes.basic = basicNonAdminProtection(branch, dl)
-	score.scores.adminBasic, score.maxes.adminBasic = basicAdminProtection(branch, dl)
 	score.scores.review, score.maxes.review = nonAdminReviewProtection(branch)
 	score.scores.adminReview, score.maxes.adminReview = adminReviewProtection(branch, dl)
 	score.scores.context, score.maxes.context = nonAdminContextProtection(branch, dl)
@@ -53,7 +52,7 @@ func TestIsBranchProtected(t *testing.T) {
 			name: "Nothing is enabled",
 			expected: scut.TestReturn{
 				Error:         nil,
-				Score:         2,
+				Score:         3,
 				NumberOfWarn:  7,
 				NumberOfInfo:  2,
 				NumberOfDebug: 0,
@@ -98,7 +97,7 @@ func TestIsBranchProtected(t *testing.T) {
 			name: "Required status check enabled",
 			expected: scut.TestReturn{
 				Error:         nil,
-				Score:         2,
+				Score:         4,
 				NumberOfWarn:  5,
 				NumberOfInfo:  4,
 				NumberOfDebug: 0,
@@ -129,7 +128,7 @@ func TestIsBranchProtected(t *testing.T) {
 			name: "Required status check enabled without checking for status string",
 			expected: scut.TestReturn{
 				Error:         nil,
-				Score:         2,
+				Score:         4,
 				NumberOfWarn:  6,
 				NumberOfInfo:  3,
 				NumberOfDebug: 0,
@@ -160,7 +159,7 @@ func TestIsBranchProtected(t *testing.T) {
 			name: "Required pull request enabled",
 			expected: scut.TestReturn{
 				Error:         nil,
-				Score:         2,
+				Score:         4,
 				NumberOfWarn:  6,
 				NumberOfInfo:  3,
 				NumberOfDebug: 0,
@@ -222,7 +221,7 @@ func TestIsBranchProtected(t *testing.T) {
 			name: "Required linear history enabled",
 			expected: scut.TestReturn{
 				Error:         nil,
-				Score:         2,
+				Score:         3,
 				NumberOfWarn:  6,
 				NumberOfInfo:  3,
 				NumberOfDebug: 0,

docs/checks.md

@@ -109,7 +109,6 @@ Note: If Scorecard is run without an administrative access token, the requiremen
 Tier 1 Requirements (3/10 points):
   - Prevent force push
   - Prevent branch deletion
-  - For administrators: Include administrator for review
 
 Tier 2 Requirements (6/10 points):
   - Require at least 1 reviewer for approval before merging
@@ -125,6 +124,7 @@ Tier 4 Requirements (9/10 points):
 
 Tier 5 Requirements (10/10 points):
   - For administrators: Dismiss stale reviews and approvals when new commits are pushed
+  - For administrators: Include administrator for review
 
 GitLab Integration Status:
   - GitLab associates releases with commits and not with the branch. Releases are ignored in this portion of the scoring.

docs/checks/internal/checks.yaml

@@ -198,7 +198,6 @@ checks:
       Tier 1 Requirements (3/10 points):
         - Prevent force push
         - Prevent branch deletion
-        - For administrators: Include administrator for review
 
       Tier 2 Requirements (6/10 points):
         - Require at least 1 reviewer for approval before merging
@@ -214,6 +213,7 @@ checks:
 
       Tier 5 Requirements (10/10 points):
         - For administrators: Dismiss stale reviews and approvals when new commits are pushed
+        - For administrators: Include administrator for review
 
       GitLab Integration Status:
         - GitLab associates releases with commits and not with the branch. Releases are ignored in this portion of the scoring.