ossf/scorecard
1
1
Fork 0
mirror of https://github.com/ossf/scorecard.git synced 2024-09-17 11:57:12 +03:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Replace checker.BP with clients.BP (#1953)

Browse Source 
Co-authored-by: Azeem Shaikh <azeems@google.com>
This commit is contained in:
Azeem Shaikh 2022-05-24 12:34:07 -07:00 committed by GitHub
parent d5e755cb08
commit edd371cf7d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 248 additions and 231 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
checker/raw_result.go
View File

@ -113,27 +113,7 @@ type WebhooksData struct {
// BranchProtectionsData contains the raw results // BranchProtectionsData contains the raw results
// for the Branch-Protection check. // for the Branch-Protection check.
type BranchProtectionsData struct { type BranchProtectionsData struct {
Branches []BranchProtectionData Branches []clients.BranchRef
}
// BranchProtectionData contains the raw results
// for one branch.
//nolint:govet
type BranchProtectionData struct {
Protected *bool
AllowsDeletions *bool
AllowsForcePushes *bool
RequiresCodeOwnerReviews *bool
RequiresLinearHistory *bool
DismissesStaleReviews *bool
EnforcesAdmins *bool
RequiresStatusChecks *bool
RequiresUpToDateBranchBeforeMerging *bool
RequiredApprovingReviewCount *int
// StatusCheckContexts is always available, so
// we don't use a pointer.
StatusCheckContexts []string
Name string
} }
// Tool represents a tool. // Tool represents a tool.

83
checks/evaluation/branch_protection.go
View File

@ -18,6 +18,7 @@ import (
"fmt" "fmt"
"github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/checker"
"github.com/ossf/scorecard/v4/clients"
sce "github.com/ossf/scorecard/v4/errors" sce "github.com/ossf/scorecard/v4/errors"
) )
@ -64,7 +65,7 @@ func BranchProtection(name string, dl checker.DetailLogger,
protected := !(b.Protected != nil && !*b.Protected) protected := !(b.Protected != nil && !*b.Protected)
if !protected { if !protected {
dl.Warn(&checker.LogMessage{ dl.Warn(&checker.LogMessage{
Text: fmt.Sprintf("branch protection not enabled for branch '%s'", b.Name), Text: fmt.Sprintf("branch protection not enabled for branch '%s'", *b.Name),
}) })
} }
score.scores.basic, score.maxes.basic = basicNonAdminProtection(&b, dl) score.scores.basic, score.maxes.basic = basicNonAdminProtection(&b, dl)
@ -253,30 +254,30 @@ func warn(dl checker.DetailLogger, doLogging bool, desc string, args ...interfac
}) })
} }
func basicNonAdminProtection(branch *checker.BranchProtectionData, dl checker.DetailLogger) (int, int) { func basicNonAdminProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) {
score := 0 score := 0
max := 0 max := 0
// Only log information if the branch is protected. // Only log information if the branch is protected.
log := branch.Protected != nil && *branch.Protected log := branch.Protected != nil && *branch.Protected
max++ max++
if branch.AllowsForcePushes != nil { if branch.BranchProtectionRule.AllowForcePushes != nil {
switch *branch.AllowsForcePushes { switch *branch.BranchProtectionRule.AllowForcePushes {
case true: case true:
warn(dl, log, "'force pushes' enabled on branch '%s'", branch.Name) warn(dl, log, "'force pushes' enabled on branch '%s'", *branch.Name)
case false: case false:
info(dl, log, "'force pushes' disabled on branch '%s'", branch.Name) info(dl, log, "'force pushes' disabled on branch '%s'", *branch.Name)
score++ score++
} }
} }
max++ max++
if branch.AllowsDeletions != nil { if branch.BranchProtectionRule.AllowDeletions != nil {
switch *branch.AllowsDeletions { switch *branch.BranchProtectionRule.AllowDeletions {
case true: case true:
warn(dl, log, "'allow deletion' enabled on branch '%s'", branch.Name) warn(dl, log, "'allow deletion' enabled on branch '%s'", *branch.Name)
case false: case false:
info(dl, log, "'allow deletion' disabled on branch '%s'", branch.Name) info(dl, log, "'allow deletion' disabled on branch '%s'", *branch.Name)
score++ score++
} }
} }
@ -284,31 +285,31 @@ func basicNonAdminProtection(branch *checker.BranchProtectionData, dl checker.De
return score, max return score, max
} }
func basicAdminProtection(branch *checker.BranchProtectionData, dl checker.DetailLogger) (int, int) { func basicAdminProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) {
score := 0 score := 0
max := 0 max := 0
// Only log information if the branch is protected. // Only log information if the branch is protected.
log := branch.Protected != nil && *branch.Protected log := branch.Protected != nil && *branch.Protected
// nil typically means we do not have access to the value. // nil typically means we do not have access to the value.
if branch.EnforcesAdmins != nil { if branch.BranchProtectionRule.EnforceAdmins != nil {
// Note: we don't inrecase max possible score for non-admin viewers. // Note: we don't inrecase max possible score for non-admin viewers.
max++ max++
switch *branch.EnforcesAdmins { switch *branch.BranchProtectionRule.EnforceAdmins {
case true: case true:
info(dl, log, "settings apply to administrators on branch '%s'", branch.Name) info(dl, log, "settings apply to administrators on branch '%s'", *branch.Name)
score++ score++
case false: case false:
warn(dl, log, "settings do not apply to administrators on branch '%s'", branch.Name) warn(dl, log, "settings do not apply to administrators on branch '%s'", *branch.Name)
} }
} else { } else {
debug(dl, log, "unable to retrieve whether or not settings apply to administrators on branch '%s'", branch.Name) debug(dl, log, "unable to retrieve whether or not settings apply to administrators on branch '%s'", *branch.Name)
} }
return score, max return score, max
} }
func nonAdminContextProtection(branch *checker.BranchProtectionData, dl checker.DetailLogger) (int, int) { func nonAdminContextProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) {
score := 0 score := 0
max := 0 max := 0
// Only log information if the branch is protected. // Only log information if the branch is protected.
@ -320,75 +321,75 @@ func nonAdminContextProtection(branch *checker.BranchProtectionData, dl checker.
// to having no status check at all. // to having no status check at all.
max++ max++
switch { switch {
case len(branch.StatusCheckContexts) > 0: case len(branch.BranchProtectionRule.CheckRules.Contexts) > 0:
info(dl, log, "status check found to merge onto on branch '%s'", branch.Name) info(dl, log, "status check found to merge onto on branch '%s'", *branch.Name)
score++ score++
default: default:
warn(dl, log, "no status checks found to merge onto branch '%s'", branch.Name) warn(dl, log, "no status checks found to merge onto branch '%s'", *branch.Name)
} }
return score, max return score, max
} }
func nonAdminReviewProtection(branch *checker.BranchProtectionData) (int, int) { func nonAdminReviewProtection(branch *clients.BranchRef) (int, int) {
score := 0 score := 0
max := 0 max := 0
max++ max++
if branch.RequiredApprovingReviewCount != nil && if branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount != nil &&
*branch.RequiredApprovingReviewCount > 0 { *branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount > 0 {
// We do not display anything here, it's done in nonAdminThoroughReviewProtection() // We do not display anything here, it's done in nonAdminThoroughReviewProtection()
score++ score++
} }
return score, max return score, max
} }
func adminReviewProtection(branch *checker.BranchProtectionData, dl checker.DetailLogger) (int, int) { func adminReviewProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) {
score := 0 score := 0
max := 0 max := 0
// Only log information if the branch is protected. // Only log information if the branch is protected.
log := branch.Protected != nil && *branch.Protected log := branch.Protected != nil && *branch.Protected
if branch.RequiresUpToDateBranchBeforeMerging != nil { if branch.BranchProtectionRule.CheckRules.UpToDateBeforeMerge != nil {
// Note: `This setting will not take effect unless at least one status check is enabled`. // Note: `This setting will not take effect unless at least one status check is enabled`.
max++ max++
switch *branch.RequiresUpToDateBranchBeforeMerging { switch *branch.BranchProtectionRule.CheckRules.UpToDateBeforeMerge {
case true: case true:
info(dl, log, "status checks require up-to-date branches for '%s'", branch.Name) info(dl, log, "status checks require up-to-date branches for '%s'", *branch.Name)
score++ score++
default: default:
warn(dl, log, "status checks do not require up-to-date branches for '%s'", branch.Name) warn(dl, log, "status checks do not require up-to-date branches for '%s'", *branch.Name)
} }
} else { } else {
debug(dl, log, "unable to retrieve whether up-to-date branches are needed to merge on branch '%s'", branch.Name) debug(dl, log, "unable to retrieve whether up-to-date branches are needed to merge on branch '%s'", *branch.Name)
} }
return score, max return score, max
} }
func adminThoroughReviewProtection(branch *checker.BranchProtectionData, dl checker.DetailLogger) (int, int) { func adminThoroughReviewProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) {
score := 0 score := 0
max := 0 max := 0
// Only log information if the branch is protected. // Only log information if the branch is protected.
log := branch.Protected != nil && *branch.Protected log := branch.Protected != nil && *branch.Protected
if branch.DismissesStaleReviews != nil { if branch.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews != nil {
// Note: we don't inrecase max possible score for non-admin viewers. // Note: we don't inrecase max possible score for non-admin viewers.
max++ max++
switch *branch.DismissesStaleReviews { switch *branch.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews {
case true: case true:
info(dl, log, "Stale review dismissal enabled on branch '%s'", branch.Name) info(dl, log, "Stale review dismissal enabled on branch '%s'", *branch.Name)
score++ score++
case false: case false:
warn(dl, log, "Stale review dismissal disabled on branch '%s'", branch.Name) warn(dl, log, "Stale review dismissal disabled on branch '%s'", *branch.Name)
} }
} else { } else {
debug(dl, log, "unable to retrieve review dismissal on branch '%s'", branch.Name) debug(dl, log, "unable to retrieve review dismissal on branch '%s'", *branch.Name)
} }
return score, max return score, max
} }
func nonAdminThoroughReviewProtection(branch *checker.BranchProtectionData, dl checker.DetailLogger) (int, int) { func nonAdminThoroughReviewProtection(branch *clients.BranchRef, dl checker.DetailLogger) (int, int) {
score := 0 score := 0
max := 0 max := 0
@ -396,18 +397,18 @@ func nonAdminThoroughReviewProtection(branch *checker.BranchProtectionData, dl c
log := branch.Protected != nil && *branch.Protected log := branch.Protected != nil && *branch.Protected
max++ max++
if branch.RequiredApprovingReviewCount != nil { if branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount != nil {
switch *branch.RequiredApprovingReviewCount >= minReviews { switch *branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount >= minReviews {
case true: case true:
info(dl, log, "number of required reviewers is %d on branch '%s'", info(dl, log, "number of required reviewers is %d on branch '%s'",
*branch.RequiredApprovingReviewCount, branch.Name) *branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount, *branch.Name)
score++ score++
default: default:
warn(dl, log, "number of required reviewers is only %d on branch '%s'", warn(dl, log, "number of required reviewers is only %d on branch '%s'",
*branch.RequiredApprovingReviewCount, branch.Name) *branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount, *branch.Name)
} }
} else { } else {
warn(dl, log, "number of required reviewers is 0 on branch '%s'", branch.Name) warn(dl, log, "number of required reviewers is 0 on branch '%s'", *branch.Name)
} }
return score, max return score, max
} }

304
checks/evaluation/branch_protection_test.go
View File

@ -18,10 +18,11 @@ import (
"testing" "testing"
"github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/checker"
"github.com/ossf/scorecard/v4/clients"
scut "github.com/ossf/scorecard/v4/utests" scut "github.com/ossf/scorecard/v4/utests"
) )
func testScore(branch *checker.BranchProtectionData, dl checker.DetailLogger) (int, error) { func testScore(branch *clients.BranchRef, dl checker.DetailLogger) (int, error) {
var score levelScore var score levelScore
score.scores.basic, score.maxes.basic = basicNonAdminProtection(branch, dl) score.scores.basic, score.maxes.basic = basicNonAdminProtection(branch, dl)
score.scores.adminBasic, score.maxes.adminBasic = basicAdminProtection(branch, dl) score.scores.adminBasic, score.maxes.adminBasic = basicAdminProtection(branch, dl)
@ -38,12 +39,12 @@ func TestIsBranchProtected(t *testing.T) {
t.Parallel() t.Parallel()
trueVal := true trueVal := true
falseVal := false falseVal := false
var zeroVal int var zeroVal int32
oneVal := 1 var oneVal int32 = 1
branchVal := "branch-name"
tests := []struct { tests := []struct {
name string name string
branch *checker.BranchProtectionData branch *clients.BranchRef
expected scut.TestReturn expected scut.TestReturn
}{ }{
{ {
@ -55,19 +56,25 @@ func TestIsBranchProtected(t *testing.T) {
NumberOfInfo: 2, NumberOfInfo: 2,
NumberOfDebug: 0, NumberOfDebug: 0,
}, },
branch: &checker.BranchProtectionData{ branch: &clients.BranchRef{
Name: "branch-name", Name: &branchVal,
Protected: &trueVal, Protected: &trueVal,
RequiresStatusChecks: &trueVal, BranchProtectionRule: clients.BranchProtectionRule{
RequiresUpToDateBranchBeforeMerging: &falseVal, AllowDeletions: &falseVal,
StatusCheckContexts: nil, AllowForcePushes: &falseVal,
DismissesStaleReviews: &falseVal, RequireLinearHistory: &falseVal,
RequiresCodeOwnerReviews: &falseVal, EnforceAdmins: &falseVal,
RequiredApprovingReviewCount: &zeroVal, RequiredPullRequestReviews: clients.PullRequestReviewRule{
EnforcesAdmins: &falseVal, DismissStaleReviews: &falseVal,
RequiresLinearHistory: &falseVal, RequireCodeOwnerReviews: &falseVal,
AllowsForcePushes: &falseVal, RequiredApprovingReviewCount: &zeroVal,
AllowsDeletions: &falseVal, },
CheckRules: clients.StatusChecksRule{
RequiresStatusChecks: &trueVal,
Contexts: nil,
UpToDateBeforeMerge: &falseVal,
},
},
}, },
}, },
{ {
@ -79,8 +86,8 @@ func TestIsBranchProtected(t *testing.T) {
NumberOfInfo: 0, NumberOfInfo: 0,
NumberOfDebug: 3, NumberOfDebug: 3,
}, },
branch: &checker.BranchProtectionData{ branch: &clients.BranchRef{
Name: "branch-name", Name: &branchVal,
Protected: &trueVal, Protected: &trueVal,
}, },
}, },
@ -93,19 +100,25 @@ func TestIsBranchProtected(t *testing.T) {
NumberOfInfo: 4, NumberOfInfo: 4,
NumberOfDebug: 0, NumberOfDebug: 0,
}, },
branch: &checker.BranchProtectionData{ branch: &clients.BranchRef{
Name: "branch-name", Name: &branchVal,
Protected: &trueVal, Protected: &trueVal,
RequiresStatusChecks: &trueVal, BranchProtectionRule: clients.BranchProtectionRule{
RequiresUpToDateBranchBeforeMerging: &trueVal, RequiredPullRequestReviews: clients.PullRequestReviewRule{
StatusCheckContexts: []string{"foo"}, DismissStaleReviews: &falseVal,
DismissesStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal,
RequiresCodeOwnerReviews: &falseVal, RequiredApprovingReviewCount: &zeroVal,
RequiredApprovingReviewCount: &zeroVal, },
EnforcesAdmins: &falseVal, CheckRules: clients.StatusChecksRule{
RequiresLinearHistory: &falseVal, RequiresStatusChecks: &trueVal,
AllowsForcePushes: &falseVal, UpToDateBeforeMerge: &trueVal,
AllowsDeletions: &falseVal, Contexts: []string{"foo"},
},
EnforceAdmins: &falseVal,
RequireLinearHistory: &falseVal,
AllowForcePushes: &falseVal,
AllowDeletions: &falseVal,
},
}, },
}, },
{ {
@ -117,19 +130,25 @@ func TestIsBranchProtected(t *testing.T) {
NumberOfInfo: 3, NumberOfInfo: 3,
NumberOfDebug: 0, NumberOfDebug: 0,
}, },
branch: &checker.BranchProtectionData{ branch: &clients.BranchRef{
Name: "branch-name", Name: &branchVal,
Protected: &trueVal, Protected: &trueVal,
RequiresStatusChecks: &trueVal, BranchProtectionRule: clients.BranchProtectionRule{
RequiresUpToDateBranchBeforeMerging: &trueVal, EnforceAdmins: &falseVal,
StatusCheckContexts: nil, RequireLinearHistory: &falseVal,
DismissesStaleReviews: &falseVal, AllowForcePushes: &falseVal,
RequiresCodeOwnerReviews: &falseVal, AllowDeletions: &falseVal,
RequiredApprovingReviewCount: &zeroVal, RequiredPullRequestReviews: clients.PullRequestReviewRule{
EnforcesAdmins: &falseVal, DismissStaleReviews: &falseVal,
RequiresLinearHistory: &falseVal, RequireCodeOwnerReviews: &falseVal,
AllowsForcePushes: &falseVal, RequiredApprovingReviewCount: &zeroVal,
AllowsDeletions: &falseVal, },
CheckRules: clients.StatusChecksRule{
RequiresStatusChecks: &trueVal,
UpToDateBeforeMerge: &trueVal,
Contexts: nil,
},
},
}, },
}, },
{ {
@ -141,19 +160,25 @@ func TestIsBranchProtected(t *testing.T) {
NumberOfInfo: 3, NumberOfInfo: 3,
NumberOfDebug: 0, NumberOfDebug: 0,
}, },
branch: &checker.BranchProtectionData{ branch: &clients.BranchRef{
Name: "branch-name", Name: &branchVal,
Protected: &trueVal, Protected: &trueVal,
RequiresStatusChecks: &trueVal, BranchProtectionRule: clients.BranchProtectionRule{
RequiresUpToDateBranchBeforeMerging: &falseVal, EnforceAdmins: &falseVal,
StatusCheckContexts: []string{"foo"}, RequireLinearHistory: &trueVal,
DismissesStaleReviews: &falseVal, AllowForcePushes: &falseVal,
RequiresCodeOwnerReviews: &falseVal, AllowDeletions: &falseVal,
RequiredApprovingReviewCount: &oneVal, CheckRules: clients.StatusChecksRule{
EnforcesAdmins: &falseVal, RequiresStatusChecks: &trueVal,
RequiresLinearHistory: &trueVal, UpToDateBeforeMerge: &falseVal,
AllowsForcePushes: &falseVal, Contexts: []string{"foo"},
AllowsDeletions: &falseVal, },
RequiredPullRequestReviews: clients.PullRequestReviewRule{
DismissStaleReviews: &falseVal,
RequireCodeOwnerReviews: &falseVal,
RequiredApprovingReviewCount: &oneVal,
},
},
}, },
}, },
{ {
@ -165,19 +190,25 @@ func TestIsBranchProtected(t *testing.T) {
NumberOfInfo: 4, NumberOfInfo: 4,
NumberOfDebug: 0, NumberOfDebug: 0,
}, },
branch: &checker.BranchProtectionData{ branch: &clients.BranchRef{
Name: "branch-name", Name: &branchVal,
Protected: &trueVal, Protected: &trueVal,
RequiresStatusChecks: &falseVal, BranchProtectionRule: clients.BranchProtectionRule{
RequiresUpToDateBranchBeforeMerging: &falseVal, EnforceAdmins: &trueVal,
StatusCheckContexts: []string{"foo"}, RequireLinearHistory: &trueVal,
DismissesStaleReviews: &falseVal, AllowForcePushes: &falseVal,
RequiresCodeOwnerReviews: &falseVal, AllowDeletions: &falseVal,
RequiredApprovingReviewCount: &zeroVal, CheckRules: clients.StatusChecksRule{
EnforcesAdmins: &trueVal, RequiresStatusChecks: &falseVal,
RequiresLinearHistory: &trueVal, UpToDateBeforeMerge: &falseVal,
AllowsForcePushes: &falseVal, Contexts: []string{"foo"},
AllowsDeletions: &falseVal, },
RequiredPullRequestReviews: clients.PullRequestReviewRule{
DismissStaleReviews: &falseVal,
RequireCodeOwnerReviews: &falseVal,
RequiredApprovingReviewCount: &zeroVal,
},
},
}, },
}, },
{ {
@ -189,19 +220,25 @@ func TestIsBranchProtected(t *testing.T) {
NumberOfInfo: 3, NumberOfInfo: 3,
NumberOfDebug: 0, NumberOfDebug: 0,
}, },
branch: &checker.BranchProtectionData{ branch: &clients.BranchRef{
Name: "branch-name", Name: &branchVal,
Protected: &trueVal, Protected: &trueVal,
RequiresStatusChecks: &falseVal, BranchProtectionRule: clients.BranchProtectionRule{
RequiresUpToDateBranchBeforeMerging: &falseVal, EnforceAdmins: &falseVal,
StatusCheckContexts: []string{"foo"}, RequireLinearHistory: &trueVal,
DismissesStaleReviews: &falseVal, AllowForcePushes: &falseVal,
RequiresCodeOwnerReviews: &falseVal, AllowDeletions: &falseVal,
RequiredApprovingReviewCount: &zeroVal, CheckRules: clients.StatusChecksRule{
EnforcesAdmins: &falseVal, RequiresStatusChecks: &falseVal,
RequiresLinearHistory: &trueVal, UpToDateBeforeMerge: &falseVal,
AllowsForcePushes: &falseVal, Contexts: []string{"foo"},
AllowsDeletions: &falseVal, },
RequiredPullRequestReviews: clients.PullRequestReviewRule{
DismissStaleReviews: &falseVal,
RequireCodeOwnerReviews: &falseVal,
RequiredApprovingReviewCount: &zeroVal,
},
},
}, },
}, },
{ {
@ -213,19 +250,26 @@ func TestIsBranchProtected(t *testing.T) {
NumberOfInfo: 2, NumberOfInfo: 2,
NumberOfDebug: 0, NumberOfDebug: 0,
}, },
branch: &checker.BranchProtectionData{ branch: &clients.BranchRef{
Name: "branch-name", Name: &branchVal,
Protected: &trueVal, Protected: &trueVal,
RequiresStatusChecks: &falseVal, BranchProtectionRule: clients.BranchProtectionRule{
RequiresUpToDateBranchBeforeMerging: &falseVal, EnforceAdmins: &falseVal,
StatusCheckContexts: []string{"foo"}, RequireLinearHistory: &falseVal,
DismissesStaleReviews: &falseVal, AllowForcePushes: &trueVal,
RequiresCodeOwnerReviews: &falseVal, AllowDeletions: &falseVal,
RequiredApprovingReviewCount: &zeroVal,
EnforcesAdmins: &falseVal, CheckRules: clients.StatusChecksRule{
RequiresLinearHistory: &falseVal, RequiresStatusChecks: &falseVal,
AllowsForcePushes: &trueVal, UpToDateBeforeMerge: &falseVal,
AllowsDeletions: &falseVal, Contexts: []string{"foo"},
},
RequiredPullRequestReviews: clients.PullRequestReviewRule{
DismissStaleReviews: &falseVal,
RequireCodeOwnerReviews: &falseVal,
RequiredApprovingReviewCount: &zeroVal,
},
},
}, },
}, },
{ {
@ -237,19 +281,25 @@ func TestIsBranchProtected(t *testing.T) {
NumberOfInfo: 2, NumberOfInfo: 2,
NumberOfDebug: 0, NumberOfDebug: 0,
}, },
branch: &checker.BranchProtectionData{ branch: &clients.BranchRef{
Name: "branch-name", Name: &branchVal,
Protected: &trueVal, Protected: &trueVal,
RequiresStatusChecks: &falseVal, BranchProtectionRule: clients.BranchProtectionRule{
RequiresUpToDateBranchBeforeMerging: &falseVal, EnforceAdmins: &falseVal,
StatusCheckContexts: []string{"foo"}, RequireLinearHistory: &falseVal,
DismissesStaleReviews: &falseVal, AllowForcePushes: &falseVal,
RequiresCodeOwnerReviews: &falseVal, AllowDeletions: &trueVal,
RequiredApprovingReviewCount: &zeroVal, CheckRules: clients.StatusChecksRule{
EnforcesAdmins: &falseVal, RequiresStatusChecks: &falseVal,
RequiresLinearHistory: &falseVal, UpToDateBeforeMerge: &falseVal,
AllowsForcePushes: &falseVal, Contexts: []string{"foo"},
AllowsDeletions: &trueVal, },
RequiredPullRequestReviews: clients.PullRequestReviewRule{
DismissStaleReviews: &falseVal,
RequireCodeOwnerReviews: &falseVal,
RequiredApprovingReviewCount: &zeroVal,
},
},
}, },
}, },
{ {
@ -261,19 +311,25 @@ func TestIsBranchProtected(t *testing.T) {
NumberOfInfo: 6, NumberOfInfo: 6,
NumberOfDebug: 0, NumberOfDebug: 0,
}, },
branch: &checker.BranchProtectionData{ branch: &clients.BranchRef{
Name: "branch-name", Name: &branchVal,
Protected: &trueVal, Protected: &trueVal,
RequiresStatusChecks: &falseVal, BranchProtectionRule: clients.BranchProtectionRule{
RequiresUpToDateBranchBeforeMerging: &trueVal, EnforceAdmins: &trueVal,
StatusCheckContexts: []string{"foo"}, RequireLinearHistory: &trueVal,
DismissesStaleReviews: &trueVal, AllowForcePushes: &falseVal,
RequiresCodeOwnerReviews: &trueVal, AllowDeletions: &falseVal,
RequiredApprovingReviewCount: &oneVal, CheckRules: clients.StatusChecksRule{
EnforcesAdmins: &trueVal, RequiresStatusChecks: &falseVal,
RequiresLinearHistory: &trueVal, UpToDateBeforeMerge: &trueVal,
AllowsForcePushes: &falseVal, Contexts: []string{"foo"},
AllowsDeletions: &falseVal, },
RequiredPullRequestReviews: clients.PullRequestReviewRule{
DismissStaleReviews: &trueVal,
RequireCodeOwnerReviews: &trueVal,
RequiredApprovingReviewCount: &oneVal,
},
},
}, },
}, },
} }

22
checks/raw/branch_protection.go
View File

@ -89,27 +89,7 @@ func BranchProtection(c clients.RepoClient) (checker.BranchProtectionsData, erro
return checker.BranchProtectionsData{}, err return checker.BranchProtectionsData{}, err
} }
// Protected field only indates that the branch matches rawData.Branches = append(rawData.Branches, *branch)
// one `Branch protection rules`. All settings may be disabled,
// so it does not provide any guarantees.
protected := !(branch.Protected != nil && !*branch.Protected)
bpData := checker.BranchProtectionData{Name: b}
bp := branch.BranchProtectionRule
bpData.Protected = &protected
bpData.RequiresLinearHistory = bp.RequireLinearHistory
bpData.AllowsForcePushes = bp.AllowForcePushes
bpData.AllowsDeletions = bp.AllowDeletions
bpData.EnforcesAdmins = bp.EnforceAdmins
bpData.RequiresCodeOwnerReviews = bp.RequiredPullRequestReviews.RequireCodeOwnerReviews
bpData.DismissesStaleReviews = bp.RequiredPullRequestReviews.DismissStaleReviews
bpData.RequiresUpToDateBranchBeforeMerging = bp.CheckRules.UpToDateBeforeMerge
if bp.RequiredPullRequestReviews.RequiredApprovingReviewCount != nil {
v := int(*bp.RequiredPullRequestReviews.RequiredApprovingReviewCount)
bpData.RequiredApprovingReviewCount = &v
}
bpData.StatusCheckContexts = bp.CheckRules.Contexts
rawData.Branches = append(rawData.Branches, bpData)
} }
// No error, return the data. // No error, return the data.

24
cron/format/json_raw_results.go
View File

@ -48,7 +48,7 @@ type jsonTool struct {
} }
type jsonBranchProtectionSettings struct { type jsonBranchProtectionSettings struct {
RequiredApprovingReviewCount *int `json:"requiredReviewerCount"` RequiredApprovingReviewCount *int32 `json:"requiredReviewerCount"`
AllowsDeletions *bool `json:"allowsDeletions"` AllowsDeletions *bool `json:"allowsDeletions"`
AllowsForcePushes *bool `json:"allowsForcePushes"` AllowsForcePushes *bool `json:"allowsForcePushes"`
RequiresCodeOwnerReviews *bool `json:"requiresCodeOwnerReview"` RequiresCodeOwnerReviews *bool `json:"requiresCodeOwnerReview"`
@ -223,20 +223,20 @@ func addBranchProtectionRawResults(r *jsonScorecardRawResult, bp *checker.Branch
var bp *jsonBranchProtectionSettings var bp *jsonBranchProtectionSettings
if v.Protected != nil && *v.Protected { if v.Protected != nil && *v.Protected {
bp = &jsonBranchProtectionSettings{ bp = &jsonBranchProtectionSettings{
AllowsDeletions: v.AllowsDeletions, AllowsDeletions: v.BranchProtectionRule.AllowDeletions,
AllowsForcePushes: v.AllowsForcePushes, AllowsForcePushes: v.BranchProtectionRule.AllowForcePushes,
RequiresCodeOwnerReviews: v.RequiresCodeOwnerReviews, RequiresCodeOwnerReviews: v.BranchProtectionRule.RequiredPullRequestReviews.RequireCodeOwnerReviews,
RequiresLinearHistory: v.RequiresLinearHistory, RequiresLinearHistory: v.BranchProtectionRule.RequireLinearHistory,
DismissesStaleReviews: v.DismissesStaleReviews, DismissesStaleReviews: v.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews,
EnforcesAdmins: v.EnforcesAdmins, EnforcesAdmins: v.BranchProtectionRule.EnforceAdmins,
RequiresStatusChecks: v.RequiresStatusChecks, RequiresStatusChecks: v.BranchProtectionRule.CheckRules.RequiresStatusChecks,
RequiresUpToDateBranchBeforeMerging: v.RequiresUpToDateBranchBeforeMerging, RequiresUpToDateBranchBeforeMerging: v.BranchProtectionRule.CheckRules.UpToDateBeforeMerge,
RequiredApprovingReviewCount: v.RequiredApprovingReviewCount, RequiredApprovingReviewCount: v.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount,
StatusCheckContexts: v.StatusCheckContexts, StatusCheckContexts: v.BranchProtectionRule.CheckRules.Contexts,
} }
} }
r.Results.BranchProtections = append(r.Results.BranchProtections, jsonBranchProtection{ r.Results.BranchProtections = append(r.Results.BranchProtections, jsonBranchProtection{
Name: v.Name, Name: *v.Name,
Protection: bp, Protection: bp,
}) })
} }

24
pkg/json_raw_results.go
View File

@ -53,7 +53,7 @@ type jsonTool struct {
} }
type jsonBranchProtectionSettings struct { type jsonBranchProtectionSettings struct {
RequiredApprovingReviewCount *int `json:"required-reviewer-count"` RequiredApprovingReviewCount *int32 `json:"required-reviewer-count"`
AllowsDeletions *bool `json:"allows-deletions"` AllowsDeletions *bool `json:"allows-deletions"`
AllowsForcePushes *bool `json:"allows-force-pushes"` AllowsForcePushes *bool `json:"allows-force-pushes"`
RequiresCodeOwnerReviews *bool `json:"requires-code-owner-review"` RequiresCodeOwnerReviews *bool `json:"requires-code-owner-review"`
@ -495,20 +495,20 @@ func (r *jsonScorecardRawResult) addBranchProtectionRawResults(bp *checker.Branc
var bp *jsonBranchProtectionSettings var bp *jsonBranchProtectionSettings
if v.Protected != nil && *v.Protected { if v.Protected != nil && *v.Protected {
bp = &jsonBranchProtectionSettings{ bp = &jsonBranchProtectionSettings{
AllowsDeletions: v.AllowsDeletions, AllowsDeletions: v.BranchProtectionRule.AllowDeletions,
AllowsForcePushes: v.AllowsForcePushes, AllowsForcePushes: v.BranchProtectionRule.AllowForcePushes,
RequiresCodeOwnerReviews: v.RequiresCodeOwnerReviews, RequiresCodeOwnerReviews: v.BranchProtectionRule.RequiredPullRequestReviews.RequireCodeOwnerReviews,
RequiresLinearHistory: v.RequiresLinearHistory, RequiresLinearHistory: v.BranchProtectionRule.RequireLinearHistory,
DismissesStaleReviews: v.DismissesStaleReviews, DismissesStaleReviews: v.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews,
EnforcesAdmins: v.EnforcesAdmins, EnforcesAdmins: v.BranchProtectionRule.EnforceAdmins,
RequiresStatusChecks: v.RequiresStatusChecks, RequiresStatusChecks: v.BranchProtectionRule.CheckRules.RequiresStatusChecks,
RequiresUpToDateBranchBeforeMerging: v.RequiresUpToDateBranchBeforeMerging, RequiresUpToDateBranchBeforeMerging: v.BranchProtectionRule.CheckRules.UpToDateBeforeMerge,
RequiredApprovingReviewCount: v.RequiredApprovingReviewCount, RequiredApprovingReviewCount: v.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount,
StatusCheckContexts: v.StatusCheckContexts, StatusCheckContexts: v.BranchProtectionRule.CheckRules.Contexts,
} }
} }
r.Results.BranchProtections = append(r.Results.BranchProtections, jsonBranchProtection{ r.Results.BranchProtections = append(r.Results.BranchProtections, jsonBranchProtection{
Name: v.Name, Name: *v.Name,
Protection: bp, Protection: bp,
}) })
} }