fix(api): honor branch protection force push and allowlist options on create and edit (#39990)

The branch protection API handled push and force push options
inconsistently. Create dropped force push unless push was enabled, and
edit ignored allowlist fields unless their parent was set to true in the
same request.

- Create treats force push independently of push, like edit and the web
UI. Enforcement is unchanged.
- Edit applies `enable_push_whitelist`, `enable_force_push_allowlist`
and both deploy key flags against the rule's resulting state, like the
deletion fields. `{"enable_push_whitelist": false}` now disables the
whitelist instead of being ignored.
- Documents that `enable_force_push_allowlist` requires
`enable_force_push` on create.

Fixes https://github.com/go-gitea/gitea/issues/35121

---------

Co-authored-by: silverwind <me@silverwind.io>

modules/indexer/code/indexer_test.go

@@ -61,17 +61,10 @@ func testIndexer(name string, t *testing.T, indexer internal.Indexer) {
 				{Path: "unicode.txt", Content: unicodeSearchContent},
 			},
 		}}))
-		assert.NoError(t, setupRepositoryIndexes(t.Context(), indexer))
-		// Wait for the index to catch up: ES/OpenSearch make writes visible
-		// only after a refresh (default interval: 1s). Bleve is synchronous
-		// and passes on the first iteration.
-		require.Eventually(t, func() bool {
-			total, _, _, err := indexer.Search(t.Context(), &internal.SearchOptions{
-				Keyword:   "Description",
-				Paginator: &db.ListOptions{Page: 1, PageSize: 1},
-			})
-			return err == nil && total > 0
-		}, 10*time.Second, 100*time.Millisecond, "index did not become searchable")
+		require.NoError(t, setupRepositoryIndexes(t.Context(), indexer))
+		if refresher, ok := indexer.(interface{ Refresh(context.Context) error }); ok {
+			require.NoError(t, refresher.Refresh(t.Context()))
+		}
 
 		keywords := []struct {
 			RepoIDs    []int64

modules/structs/repo_branch.go

@@ -82,15 +82,16 @@ type BranchProtection struct {
 // CreateBranchProtectionOption options for creating a branch protection
 type CreateBranchProtectionOption struct {
 	// Deprecated: true
-	BranchName                    string   `json:"branch_name"`
-	RuleName                      string   `json:"rule_name"`
-	Priority                      int64    `json:"priority"`
-	EnablePush                    bool     `json:"enable_push"`
-	EnablePushWhitelist           bool     `json:"enable_push_whitelist"`
-	PushWhitelistUsernames        []string `json:"push_whitelist_usernames"`
-	PushWhitelistTeams            []string `json:"push_whitelist_teams"`
-	PushWhitelistDeployKeys       bool     `json:"push_whitelist_deploy_keys"`
-	EnableForcePush               bool     `json:"enable_force_push"`
+	BranchName              string   `json:"branch_name"`
+	RuleName                string   `json:"rule_name"`
+	Priority                int64    `json:"priority"`
+	EnablePush              bool     `json:"enable_push"`
+	EnablePushWhitelist     bool     `json:"enable_push_whitelist"`
+	PushWhitelistUsernames  []string `json:"push_whitelist_usernames"`
+	PushWhitelistTeams      []string `json:"push_whitelist_teams"`
+	PushWhitelistDeployKeys bool     `json:"push_whitelist_deploy_keys"`
+	EnableForcePush         bool     `json:"enable_force_push"`
+	// EnableForcePushAllowlist only takes effect when enable_force_push is true
 	EnableForcePushAllowlist      bool     `json:"enable_force_push_allowlist"`
 	ForcePushAllowlistUsernames   []string `json:"force_push_allowlist_usernames"`
 	ForcePushAllowlistTeams       []string `json:"force_push_allowlist_teams"`

routers/api/v1/repo/branch.go

@@ -758,7 +758,7 @@ func CreateBranchProtection(ctx *context.APIContext) {
 	if form.EnablePush && form.EnablePushWhitelist && form.PushWhitelistDeployKeys {
 		whitelistUsers = legacySetDeployKeyAllowlist(whitelistUsers, true)
 	}
-	if form.EnablePush && form.EnableForcePush && form.EnableForcePushAllowlist && form.ForcePushAllowlistDeployKeys {
+	if form.EnableForcePush && form.EnableForcePushAllowlist && form.ForcePushAllowlistDeployKeys {
 		forcePushAllowlistUsers = legacySetDeployKeyAllowlist(forcePushAllowlistUsers, true)
 	}
 
@@ -766,8 +766,8 @@ func CreateBranchProtection(ctx *context.APIContext) {
 		ProtectedBranchConfig: git_model.ProtectedBranchConfig{
 			CanPush:                       form.EnablePush,
 			EnableWhitelist:               form.EnablePush && form.EnablePushWhitelist,
-			CanForcePush:                  form.EnablePush && form.EnableForcePush,
-			EnableForcePushAllowlist:      form.EnablePush && form.EnableForcePush && form.EnableForcePushAllowlist,
+			CanForcePush:                  form.EnableForcePush,
+			EnableForcePushAllowlist:      form.EnableForcePush && form.EnableForcePushAllowlist,
 			CanDelete:                     form.EnablePush && form.EnableDeletion,
 			EnableDeletionAllowlist:       form.EnablePush && form.EnableDeletion && form.EnableDeletionAllowlist,
 			EnableMergeWhitelist:          form.EnableMergeWhitelist,
@@ -874,46 +874,26 @@ func EditBranchProtection(ctx *context.APIContext) {
 	pushDeployKeys := protectBranch.HasDeployKeyPush()
 	forcePushDeployKeys := protectBranch.HasDeployKeyForcePush()
 	if form.EnablePush != nil {
-		if !*form.EnablePush {
-			protectBranch.CanPush = false
-			protectBranch.EnableWhitelist = false
-			pushDeployKeys = false
-		} else {
-			protectBranch.CanPush = true
-			if form.EnablePushWhitelist != nil {
-				if !*form.EnablePushWhitelist {
-					protectBranch.EnableWhitelist = false
-					pushDeployKeys = false
-				} else {
-					protectBranch.EnableWhitelist = true
-					if form.PushWhitelistDeployKeys != nil {
-						pushDeployKeys = *form.PushWhitelistDeployKeys
-					}
-				}
-			}
-		}
+		protectBranch.CanPush = *form.EnablePush
 	}
+	if form.EnablePushWhitelist != nil {
+		protectBranch.EnableWhitelist = *form.EnablePushWhitelist
+	}
+	if form.PushWhitelistDeployKeys != nil {
+		pushDeployKeys = *form.PushWhitelistDeployKeys
+	}
+	protectBranch.EnableWhitelist = protectBranch.CanPush && protectBranch.EnableWhitelist
 
 	if form.EnableForcePush != nil {
-		if !*form.EnableForcePush {
-			protectBranch.CanForcePush = false
-			protectBranch.EnableForcePushAllowlist = false
-			forcePushDeployKeys = false
-		} else {
-			protectBranch.CanForcePush = true
-			if form.EnableForcePushAllowlist != nil {
-				if !*form.EnableForcePushAllowlist {
-					protectBranch.EnableForcePushAllowlist = false
-					forcePushDeployKeys = false
-				} else {
-					protectBranch.EnableForcePushAllowlist = true
-					if form.ForcePushAllowlistDeployKeys != nil {
-						forcePushDeployKeys = *form.ForcePushAllowlistDeployKeys
-					}
-				}
-			}
-		}
+		protectBranch.CanForcePush = *form.EnableForcePush
 	}
+	if form.EnableForcePushAllowlist != nil {
+		protectBranch.EnableForcePushAllowlist = *form.EnableForcePushAllowlist
+	}
+	if form.ForcePushAllowlistDeployKeys != nil {
+		forcePushDeployKeys = *form.ForcePushAllowlistDeployKeys
+	}
+	protectBranch.EnableForcePushAllowlist = protectBranch.CanForcePush && protectBranch.EnableForcePushAllowlist
 
 	if form.EnableDeletion != nil {
 		protectBranch.CanDelete = *form.EnableDeletion
@@ -1008,8 +988,8 @@ func EditBranchProtection(ctx *context.APIContext) {
 	if form.ForcePushAllowlistUsernames != nil && form.ForcePushAllowlistDeployKeys == nil && slices.Contains(forcePushAllowlistUsers, user_model.DeployKeyUserID) {
 		forcePushDeployKeys = true
 	}
-	whitelistUsers = legacySetDeployKeyAllowlist(whitelistUsers, pushDeployKeys)
-	forcePushAllowlistUsers = legacySetDeployKeyAllowlist(forcePushAllowlistUsers, forcePushDeployKeys)
+	whitelistUsers = legacySetDeployKeyAllowlist(whitelistUsers, protectBranch.EnableWhitelist && pushDeployKeys)
+	forcePushAllowlistUsers = legacySetDeployKeyAllowlist(forcePushAllowlistUsers, protectBranch.EnableForcePushAllowlist && forcePushDeployKeys)
 
 	var whitelistTeams, forcePushAllowlistTeams, deletionAllowlistTeams, mergeWhitelistTeams, approvalsWhitelistTeams, bypassAllowlistTeams []int64
 	if repo.Owner.IsOrganization() {

templates/swagger/v1-openapi3.generated.json

@@ -3802,6 +3802,7 @@
             "x-go-name": "EnableForcePush"
           },
           "enable_force_push_allowlist": {
+            "description": "EnableForcePushAllowlist only takes effect when enable_force_push is true",
             "type": "boolean",
             "x-go-name": "EnableForcePushAllowlist"
           },

templates/swagger/v1-swagger.generated.json

@@ -26255,6 +26255,7 @@
           "x-go-name": "EnableForcePush"
         },
         "enable_force_push_allowlist": {
+          "description": "EnableForcePushAllowlist only takes effect when enable_force_push is true",
           "type": "boolean",
           "x-go-name": "EnableForcePushAllowlist"
         },

tests/integration/api_branch_test.go

@@ -502,8 +502,7 @@ func TestAPIBranchProtectionDeployKeyAllowlists(t *testing.T) {
 	t.Run("LegacyFlagsDisableDeployKeys", func(t *testing.T) {
 		createProtection(t, "disable-deploy-keys", true)
 		resp := MakeRequest(t, NewRequestWithJSON(t, "PATCH", path+"/disable-deploy-keys", map[string]any{
-			"enable_push": true, "enable_push_whitelist": true, "push_whitelist_deploy_keys": false,
-			"enable_force_push": true, "enable_force_push_allowlist": true, "force_push_allowlist_deploy_keys": false,
+			"push_whitelist_deploy_keys": false, "force_push_allowlist_deploy_keys": false,
 			"push_whitelist_usernames": []string{"user2", "gitea:deploy-key"}, "force_push_allowlist_usernames": []string{"gitea:deploy-key"},
 		}).AddTokenAuth(token), http.StatusOK)
 		bp := DecodeJSON(t, resp, &api.BranchProtection{})
@@ -529,6 +528,41 @@ func TestAPIBranchProtectionDeployKeyAllowlists(t *testing.T) {
 		assert.Equal(t, []string{"gitea:deploy-key"}, bp.ForcePushAllowlistUsernames)
 	})
 
+	t.Run("ChildFieldsFollowEffectiveParents", func(t *testing.T) {
+		protectionFlags := func(resp *httptest.ResponseRecorder) api.BranchProtection {
+			bp := DecodeJSON(t, resp, &api.BranchProtection{})
+			return api.BranchProtection{
+				EnablePush: bp.EnablePush, EnablePushWhitelist: bp.EnablePushWhitelist, PushWhitelistDeployKeys: bp.PushWhitelistDeployKeys,
+				EnableForcePush: bp.EnableForcePush, EnableForcePushAllowlist: bp.EnableForcePushAllowlist, ForcePushAllowlistDeployKeys: bp.ForcePushAllowlistDeployKeys,
+			}
+		}
+		resp := MakeRequest(t, NewRequestWithJSON(t, "POST", path, &api.CreateBranchProtectionOption{
+			RuleName: "child-fields", EnablePushWhitelist: true, PushWhitelistDeployKeys: true,
+			EnableForcePush: true, EnableForcePushAllowlist: true, ForcePushAllowlistDeployKeys: true,
+		}).AddTokenAuth(token), http.StatusCreated)
+		assert.Equal(t, api.BranchProtection{EnableForcePush: true, EnableForcePushAllowlist: true, ForcePushAllowlistDeployKeys: true}, protectionFlags(resp))
+
+		children := map[string]any{
+			"enable_push_whitelist": true, "push_whitelist_deploy_keys": true,
+			"enable_force_push_allowlist": true, "force_push_allowlist_deploy_keys": true,
+		}
+		for _, testCase := range []struct {
+			body     map[string]any
+			expected api.BranchProtection
+		}{
+			{map[string]any{"enable_push": true, "enable_force_push_allowlist": false}, api.BranchProtection{EnablePush: true, EnableForcePush: true}},
+			{children, api.BranchProtection{
+				EnablePush: true, EnablePushWhitelist: true, PushWhitelistDeployKeys: true,
+				EnableForcePush: true, EnableForcePushAllowlist: true, ForcePushAllowlistDeployKeys: true,
+			}},
+			{map[string]any{"enable_push": false, "enable_force_push": false}, api.BranchProtection{}},
+			{children, api.BranchProtection{}},
+		} {
+			resp := MakeRequest(t, NewRequestWithJSON(t, "PATCH", path+"/child-fields", testCase.body).AddTokenAuth(token), http.StatusOK)
+			assert.Equal(t, testCase.expected, protectionFlags(resp))
+		}
+	})
+
 	t.Run("CreateWithDeployKeyUsernames", func(t *testing.T) {
 		resp := MakeRequest(t, NewRequestWithJSON(t, "POST", path, &api.CreateBranchProtectionOption{
 			RuleName: "deploy-key-usernames", EnablePush: true, EnablePushWhitelist: true,