From 75925a90f751e48b01d610753c35355212c35285 Mon Sep 17 00:00:00 2001 From: Azeem Shaikh Date: Mon, 15 Nov 2021 09:30:18 -0800 Subject: [PATCH] Fix nil-ptr dereference --- checks/branch_protection.go | 21 ++++++++++-- checks/branch_protection_test.go | 56 +++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/checks/branch_protection.go b/checks/branch_protection.go index f4473d4767b..cbc1f6c26fb 100644 --- a/checks/branch_protection.go +++ b/checks/branch_protection.go @@ -82,13 +82,21 @@ func (b branchMap) getBranchByName(name string) (*clients.BranchRef, error) { func getBranchMapFrom(branches []*clients.BranchRef) branchMap { ret := make(branchMap) for _, branch := range branches { - if branch.Name != nil && *branch.Name != "" { - ret[*branch.Name] = branch + branchName := getBranchName(branch) + if branchName != "" { + ret[branchName] = branch } } return ret } +func getBranchName(branch *clients.BranchRef) string { + if branch == nil || branch.Name == nil { + return "" + } + return *branch.Name +} + // BranchProtection runs Branch-Protection check. func BranchProtection(c *checker.CheckRequest) checker.CheckResult { // Checks branch protection on both release and development branch. @@ -142,7 +150,10 @@ func checkReleaseAndDevBranchProtection( if err != nil { return checker.CreateRuntimeErrorResult(CheckBranchProtection, err) } - checkBranches[*defaultBranch.Name] = true + defaultBranchName := getBranchName(defaultBranch) + if defaultBranchName != "" { + checkBranches[defaultBranchName] = true + } var scores []int // Check protections on all the branches. @@ -164,6 +175,10 @@ func checkReleaseAndDevBranchProtection( scores = append(scores, score) } + if len(scores) == 0 { + return checker.CreateInconclusiveResult(CheckBranchProtection, "unable to detect any development/release branches") + } + score := checker.AggregateScores(scores...) switch score { case checker.MinResultScore: diff --git a/checks/branch_protection_test.go b/checks/branch_protection_test.go index e776d1b1cab..c98e24fffbd 100644 --- a/checks/branch_protection_test.go +++ b/checks/branch_protection_test.go @@ -28,7 +28,8 @@ import ( func getBranch(branches []*clients.BranchRef, name string) *clients.BranchRef { for _, branch := range branches { - if *branch.Name == name { + branchName := getBranchName(branch) + if branchName == name { return branch } } @@ -68,6 +69,59 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { releases []string nonadmin bool }{ + { + name: "Nil release and main branch names", + expected: scut.TestReturn{ + Error: nil, + Score: -1, + NumberOfWarn: 0, + NumberOfInfo: 0, + NumberOfDebug: 0, + }, + defaultBranch: main, + branches: []*clients.BranchRef{ + { + Name: nil, + Protected: &trueVal, + BranchProtectionRule: clients.BranchProtectionRule{ + RequiredStatusChecks: clients.StatusChecksRule{ + Strict: &trueVal, + Contexts: []string{"foo"}, + }, + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + DismissStaleReviews: &trueVal, + RequireCodeOwnerReviews: &trueVal, + RequiredApprovingReviewCount: &oneVal, + }, + EnforceAdmins: &trueVal, + RequireLinearHistory: &trueVal, + AllowForcePushes: &falseVal, + AllowDeletions: &falseVal, + }, + }, + { + Name: nil, + Protected: &trueVal, + BranchProtectionRule: clients.BranchProtectionRule{ + RequiredStatusChecks: clients.StatusChecksRule{ + Strict: &falseVal, + Contexts: nil, + }, + RequiredPullRequestReviews: clients.PullRequestReviewRule{ + DismissStaleReviews: &falseVal, + RequireCodeOwnerReviews: &falseVal, + RequiredApprovingReviewCount: &zeroVal, + }, + EnforceAdmins: &falseVal, + RequireLinearHistory: &falseVal, + AllowForcePushes: &falseVal, + AllowDeletions: &falseVal, + }, + }, + nil, + }, + releases: []string{}, + }, { name: "Only development branch", expected: scut.TestReturn{