diff --git a/app/services/protection/bypass.go b/app/services/protection/bypass.go index a842c9acd..cf30ed441 100644 --- a/app/services/protection/bypass.go +++ b/app/services/protection/bypass.go @@ -29,8 +29,7 @@ type DefBypass struct { func (v DefBypass) matches(actor *types.Principal, isRepoOwner bool) bool { return actor != nil && - (actor.Admin || - v.RepoOwners && isRepoOwner || + (v.RepoOwners && isRepoOwner || slices.Contains(v.UserIDs, actor.ID)) } diff --git a/app/services/protection/bypass_test.go b/app/services/protection/bypass_test.go index ed3497ef6..630573de8 100644 --- a/app/services/protection/bypass_test.go +++ b/app/services/protection/bypass_test.go @@ -38,10 +38,11 @@ func TestBranch_matches(t *testing.T) { exp: false, }, { - name: "admin", - bypass: DefBypass{UserIDs: nil, RepoOwners: false}, + name: "admin-no-owner", + bypass: DefBypass{UserIDs: nil, RepoOwners: true}, actor: admin, - exp: true, + owner: false, + exp: false, }, { name: "repo-owners-false", diff --git a/app/services/protection/rule_branch_test.go b/app/services/protection/rule_branch_test.go index 4285132c6..4bd1a2285 100644 --- a/app/services/protection/rule_branch_test.go +++ b/app/services/protection/rule_branch_test.go @@ -47,7 +47,7 @@ func TestBranch_MergeVerify(t *testing.T) { expVs: []types.RuleViolations{}, }, { - name: "admin-no-bypass", + name: "admin-no-owner", branch: Branch{ Bypass: DefBypass{}, PullReq: DefPullReq{ @@ -58,7 +58,8 @@ func TestBranch_MergeVerify(t *testing.T) { }, in: MergeVerifyInput{ Actor: admin, - AllowBypass: false, + IsRepoOwner: false, + AllowBypass: true, PullReq: &types.PullReq{UnresolvedCount: 1}, }, expOut: MergeVerifyOutput{ @@ -68,7 +69,7 @@ func TestBranch_MergeVerify(t *testing.T) { }, expVs: []types.RuleViolations{ { - Bypassable: true, + Bypassable: false, Bypassed: false, Violations: []types.Violation{ {Code: codePullReqCommentsReqResolveAll}, @@ -314,7 +315,7 @@ func TestBranch_RequiredChecks(t *testing.T) { }, }, { - name: "admin-bypassable", + name: "admin-no-owner", branch: Branch{ Bypass: DefBypass{}, PullReq: DefPullReq{ @@ -322,11 +323,12 @@ func TestBranch_RequiredChecks(t *testing.T) { }, }, in: RequiredChecksInput{ - Actor: admin, + Actor: admin, + IsRepoOwner: false, }, expOut: RequiredChecksOutput{ - RequiredIdentifiers: nil, - BypassableIdentifiers: map[string]struct{}{"abc": {}}, + RequiredIdentifiers: map[string]struct{}{"abc": {}}, + BypassableIdentifiers: nil, }, }, { @@ -407,21 +409,22 @@ func TestBranch_RefChangeVerify(t *testing.T) { expVs: []types.RuleViolations{}, }, { - name: "admin-no-bypass", + name: "admin-no-owner", branch: Branch{ Bypass: DefBypass{}, Lifecycle: DefLifecycle{DeleteForbidden: true}, }, in: RefChangeVerifyInput{ Actor: admin, - AllowBypass: false, + IsRepoOwner: false, + AllowBypass: true, RefAction: RefActionDelete, RefType: RefTypeBranch, RefNames: []string{"abc"}, }, expVs: []types.RuleViolations{ { - Bypassable: true, + Bypassable: false, Bypassed: false, Violations: []types.Violation{ {Code: codeLifecycleDelete},