A file that is both unprotected and protected is unprotected

models/fixtures/protected_branch.yml

@@ -1 +1,27 @@
-[] # empty
+-
+  id: 1
+  repo_id: 1
+  branch_name: master
+  can_push: true
+  enable_whitelist: false
+  whitelist_user_i_ds: '[]'
+  whitelist_team_i_ds: '[]'
+  enable_merge_whitelist: false
+  whitelist_deploy_keys: false
+  merge_whitelist_user_i_ds: '[]'
+  merge_whitelist_team_i_ds: '[]'
+  enable_status_check: false
+  status_check_contexts: '[]'
+  enable_approvals_whitelist: false
+  approvals_whitelist_user_i_ds: '[]'
+  approvals_whitelist_team_i_ds: '[]'
+  required_approvals: 0
+  block_on_rejected_reviews: false
+  block_on_official_review_requests: false
+  block_on_outdated_branch: false
+  dismiss_stale_approvals: false
+  require_signed_commits: false
+  protected_file_patterns: 'disallow_push'
+  unprotected_file_patterns: 'allow_push'
+  created_unix: 1688973030
+  updated_unix: 1688973030

services/repository/files/update.go

@@ -459,44 +459,55 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file
 	return nil
 }
 
+func checkTreePathProtected(ctx context.Context, protectedBranch *git_model.ProtectedBranch, doer *user_model.User, treePaths []string) error {
+	globUnprotected := protectedBranch.GetUnprotectedFilePatterns()
+	globProtected := protectedBranch.GetProtectedFilePatterns()
+	canUserPush := protectedBranch.CanUserPush(ctx, doer)
+	for _, treePath := range treePaths {
+		isUnprotectedFile := false
+		if len(globUnprotected) != 0 {
+			isUnprotectedFile = protectedBranch.IsUnprotectedFile(globUnprotected, treePath)
+		}
+		if !canUserPush && !isUnprotectedFile {
+			return models.ErrUserCannotCommit{
+				UserName: doer.LowerName,
+			}
+		}
+		if !isUnprotectedFile && protectedBranch.IsProtectedFile(globProtected, treePath) {
+			return models.ErrFilePathProtected{
+				Path: treePath,
+			}
+		}
+	}
+	return nil
+}
+
 // VerifyBranchProtection verify the branch protection for modifying the given treePath on the given branch
 func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, branchName string, treePaths []string) error {
 	protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branchName)
 	if err != nil {
 		return err
 	}
-	if protectedBranch != nil {
-		protectedBranch.Repo = repo
-		globUnprotected := protectedBranch.GetUnprotectedFilePatterns()
-		globProtected := protectedBranch.GetProtectedFilePatterns()
-		canUserPush := protectedBranch.CanUserPush(ctx, doer)
-		for _, treePath := range treePaths {
-			isUnprotectedFile := false
-			if len(globUnprotected) != 0 {
-				isUnprotectedFile = protectedBranch.IsUnprotectedFile(globUnprotected, treePath)
-			}
-			if !canUserPush && !isUnprotectedFile {
-				return models.ErrUserCannotCommit{
-					UserName: doer.LowerName,
-				}
-			}
-			if protectedBranch.IsProtectedFile(globProtected, treePath) {
-				return models.ErrFilePathProtected{
-					Path: treePath,
-				}
+	if protectedBranch == nil { // no matched rule for branch name
+		return nil
+	}
+
+	protectedBranch.Repo = repo
+	if err := checkTreePathProtected(ctx, protectedBranch, doer, treePaths); err != nil {
+		return err
+	}
+
+	if protectedBranch.RequireSignedCommits {
+		_, _, _, err := asymkey_service.SignCRUDAction(ctx, repo.RepoPath(), doer, repo.RepoPath(), branchName)
+		if err != nil {
+			if !asymkey_service.IsErrWontSign(err) {
+				return err
 			}
-		}
-		if protectedBranch.RequireSignedCommits {
-			_, _, _, err := asymkey_service.SignCRUDAction(ctx, repo.RepoPath(), doer, repo.RepoPath(), branchName)
-			if err != nil {
-				if !asymkey_service.IsErrWontSign(err) {
-					return err
-				}
-				return models.ErrUserCannotCommit{
-					UserName: doer.LowerName,
-				}
+			return models.ErrUserCannotCommit{
+				UserName: doer.LowerName,
 			}
 		}
 	}
+
 	return nil
 }

services/repository/files/update_test.go

@@ -0,0 +1,45 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package files
+
+import (
+	"context"
+	"testing"
+
+	git_model "code.gitea.io/gitea/models/git"
+	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_checkTreePathProtected(t *testing.T) {
+	unittest.PrepareTestEnv(t)
+
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	pb := unittest.AssertExistsAndLoadBean(t, &git_model.ProtectedBranch{ID: 1})
+
+	kases := []struct {
+		TreePath string
+		CanPush  bool
+	}{
+		{
+			TreePath: "allow_push",
+			CanPush:  true,
+		},
+		{
+			TreePath: "disallow_push",
+			CanPush:  false,
+		},
+	}
+
+	for _, kase := range kases {
+		err := checkTreePathProtected(context.Background(), pb, user2, []string{kase.TreePath})
+		if kase.CanPush {
+			assert.Nil(t, err)
+		} else {
+			assert.Error(t, err)
+		}
+	}
+}