feature: branch protection without admin token (#823)

* branch protection without admin permission

Signed-off-by: Asra Ali <asraa@google.com>

* handle other errors

Signed-off-by: Asra Ali <asraa@google.com>

* fix lint

Signed-off-by: Asra Ali <asraa@google.com>

Co-authored-by: Naveen <172697+naveensrinivasan@users.noreply.github.com>
This commit is contained in:
asraa 2021-08-12 11:54:28 -04:00 committed by GitHub
parent a10baab917
commit cc312f2d1d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 68 additions and 12 deletions

View File

@ -16,6 +16,8 @@ package checks
import (
"context"
"errors"
"net/http"
"regexp"
"github.com/google/go-github/v32/github"
@ -100,12 +102,14 @@ func checkReleaseAndDevBranchProtection(ctx context.Context, r repositories, dl
checkBranches[*repo.DefaultBranch] = true
protected := true
unknown := false
// Check protections on all the branches.
for b := range checkBranches {
p, err := isBranchProtected(branches, b)
if err != nil {
return checker.CreateRuntimeErrorResult(CheckBranchProtection, err)
}
// nolint
if !p {
protected = false
dl.Warn("branch protection not enabled for branch '%s'", b)
@ -113,7 +117,17 @@ func checkReleaseAndDevBranchProtection(ctx context.Context, r repositories, dl
// The branch is protected. Check the protection.
score, err := getProtectionAndCheck(ctx, r, dl, ownerStr, repoStr, b)
if err != nil {
return checker.CreateRuntimeErrorResult(CheckBranchProtection, err)
if errors.Is(err, errInternalBranchNotFound) {
// Without an admin token, you only get information on the protection boolean.
// Add a score of 1 (minimal branch protection) for this protected branch.
unknown = true
scores = append(scores, 1)
dl.Warn("no detailed settings available for branch protection '%s'", b)
continue
} else {
// Github timeout or other error
return checker.CreateRuntimeErrorResult(CheckBranchProtection, err)
}
}
scores = append(scores, score)
}
@ -135,6 +149,11 @@ func checkReleaseAndDevBranchProtection(ctx context.Context, r repositories, dl
"branch protection is fully enabled on development and all release branches")
}
if unknown {
return checker.CreateResultWithScore(CheckBranchProtection,
"branch protection is enabled on development and all release branches but settings are unknown", score)
}
return checker.CreateResultWithScore(CheckBranchProtection,
"branch protection is not maximal on development and all release branches", score)
}
@ -170,9 +189,14 @@ func isBranchProtected(branches []*github.Branch, name string) (bool, error) {
func getProtectionAndCheck(ctx context.Context, r repositories, dl checker.DetailLogger, ownerStr, repoStr,
branch string) (int, error) {
// We only call this if the branch is protected. An error indicates not found.
protection, _, err := r.GetBranchProtection(ctx, ownerStr, repoStr, branch)
// We only call this if the branch is protected.
protection, resp, err := r.GetBranchProtection(ctx, ownerStr, repoStr, branch)
if err != nil {
// Check the type of error. A not found error indicates that permissions are denied.
if resp.StatusCode == http.StatusNotFound {
//nolint
return 1, sce.Create(errInternalBranchNotFound, errInternalBranchNotFound.Error())
}
//nolint
return checker.InconclusiveResultScore, sce.Create(sce.ErrScorecardInternal, err.Error())
}

View File

@ -31,6 +31,7 @@ type mockRepos struct {
protections map[string]*github.Protection
defaultBranch *string
releases []*string
nonadmin bool
}
func (m mockRepos) Get(ctx context.Context, o, r string) (
@ -51,11 +52,13 @@ func (m mockRepos) ListReleases(ctx context.Context, owner string,
func (m mockRepos) GetBranchProtection(ctx context.Context, o string, r string,
b string) (*github.Protection, *github.Response, error) {
p, ok := m.protections[b]
if ok {
return p, &github.Response{
Response: &http.Response{StatusCode: http.StatusAccepted},
}, nil
if !m.nonadmin {
p, ok := m.protections[b]
if ok {
return p, &github.Response{
Response: &http.Response{StatusCode: http.StatusAccepted},
}, nil
}
}
return nil, &github.Response{
Response: &http.Response{StatusCode: http.StatusNotFound},
@ -88,6 +91,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) {
defaultBranch *string
releases []*string
protections map[string]*github.Protection
nonadmin bool
}{
{
name: "Only development branch",
@ -395,6 +399,34 @@ func TestReleaseAndDevBranchProtected(t *testing.T) {
},
},
},
{
name: "Non-admin check with protected release and development",
expected: scut.TestReturn{
Errors: nil,
Score: 1,
NumberOfWarn: 2,
NumberOfInfo: 0,
NumberOfDebug: 0,
},
nonadmin: true,
defaultBranch: &main,
branches: []*string{&rel1, &main},
releases: []*string{&rel1},
protections: map[string]*github.Protection{
"main": {
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: true,
Contexts: []string{"foo"},
},
},
"release/v.1": {
RequiredStatusChecks: &github.RequiredStatusChecks{
Strict: true,
Contexts: []string{"foo"},
},
},
},
},
}
for _, tt := range tests {
@ -406,6 +438,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) {
branches: tt.branches,
releases: tt.releases,
protections: tt.protections,
nonadmin: tt.nonadmin,
}
dl := scut.TestDetailLogger{}
r := checkReleaseAndDevBranchProtection(context.Background(), m,

View File

@ -22,7 +22,6 @@ import (
"github.com/ossf/scorecard/v2/checker"
"github.com/ossf/scorecard/v2/checks"
sce "github.com/ossf/scorecard/v2/errors"
scut "github.com/ossf/scorecard/v2/utests"
)
@ -41,9 +40,9 @@ var _ = Describe("E2E TEST:"+checks.CheckBranchProtection, func() {
Dlogger: &dl,
}
expected := scut.TestReturn{
Errors: []error{sce.ErrScorecardInternal},
Score: checker.InconclusiveResultScore,
NumberOfWarn: 0,
Errors: nil,
Score: 1,
NumberOfWarn: 3,
NumberOfInfo: 0,
NumberOfDebug: 0,
}