diff --git a/pkg/policies/branch/branch.go b/pkg/policies/branch/branch.go index 8897124b..8ec44f5f 100644 --- a/pkg/policies/branch/branch.go +++ b/pkg/policies/branch/branch.go @@ -494,6 +494,15 @@ func fix(ctx context.Context, rep repositories, c *github.Client, // no sense to continue, just return return nil } + if rsp != nil && rsp.StatusCode == http.StatusNotFound { + log.Warn(). + Str("org", owner). + Str("repo", repo). + Str("area", polName). + Msg("Fix action selected, but branch protection has been disabled on this repository.") + // no sense to continue, just return + return nil + } return err } continue @@ -636,6 +645,14 @@ func fix(ctx context.Context, rep repositories, c *github.Client, Msg("Action set to fix, but did not accept admin:write permissions update.") return nil } + if rsp != nil && rsp.StatusCode == http.StatusNotFound { + log.Warn(). + Str("org", owner). + Str("repo", repo). + Str("area", polName). + Msg("Fix action selected, but branch protection has been disabled on this repository.") + return nil + } return err } log.Info(). diff --git a/pkg/policies/branch/branch_test.go b/pkg/policies/branch/branch_test.go index a39a755f..9d1504ff 100644 --- a/pkg/policies/branch/branch_test.go +++ b/pkg/policies/branch/branch_test.go @@ -1371,6 +1371,7 @@ func TestFix(t *testing.T) { Prot map[string]github.Protection SignatureProt map[string]github.SignaturesProtectedBranch cofigEnabled bool + FailUpdateWith int Exp map[string]github.ProtectionRequest ExpSignatureRequests map[string]bool }{ @@ -1490,6 +1491,60 @@ func TestFix(t *testing.T) { }, ExpSignatureRequests: map[string]bool{}, }, + { + // Regression test for https://github.com/ossf/allstar/issues/562: + // GitHub returns 404 when creating branch protection on a repo + // that has protection disabled via rulesets migration. Fix + // should warn and return nil, not error out. + Name: "AddProtectionFromScratchDisabled", + Org: OrgConfig{ + EnforceDefault: true, + RequireApproval: true, + ApprovalCount: 2, + DismissStale: true, + BlockForce: true, + EnforceOnAdmins: true, + }, + Repo: RepoConfig{}, + Prot: map[string]github.Protection{}, + cofigEnabled: true, + FailUpdateWith: http.StatusNotFound, + Exp: map[string]github.ProtectionRequest{}, + SignatureProt: map[string]github.SignaturesProtectedBranch{}, + ExpSignatureRequests: map[string]bool{}, + }, + { + // Same regression, but on the path that updates existing + // protection rather than creating it from scratch. + Name: "EnforceAdminsDisabled", + Org: OrgConfig{ + EnforceDefault: true, + EnforceOnAdmins: true, + }, + Repo: RepoConfig{}, + Prot: map[string]github.Protection{ + "main": { + AllowForcePushes: &github.AllowForcePushes{ + Enabled: false, + }, + EnforceAdmins: &github.AdminEnforcement{ + Enabled: false, + }, + RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{ + RequiredApprovingReviewCount: 0, + }, + }, + }, + cofigEnabled: true, + FailUpdateWith: http.StatusNotFound, + Exp: map[string]github.ProtectionRequest{}, + SignatureProt: map[string]github.SignaturesProtectedBranch{ + "main": { + Enabled: github.Ptr(false), + }, + }, + ExpSignatureRequests: map[string]bool{}, + }, { Name: "NotEnabled", Org: OrgConfig{ @@ -2065,6 +2120,13 @@ func TestFix(t *testing.T) { branch string, preq *github.ProtectionRequest) (*github.Protection, *github.Response, error, ) { + if test.FailUpdateWith != 0 { + return nil, &github.Response{ + Response: &http.Response{ + StatusCode: test.FailUpdateWith, + }, + }, errors.New("update failed") + } got[branch] = *preq return nil, nil, nil }