Merge pull request 'fix: check read permissions for code owner review requests' (#5996) from gusted/forgejo-codeowners into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5996
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

services/issue/pull.go

@@ -10,6 +10,8 @@ import (
 
 	issues_model "code.gitea.io/gitea/models/issues"
 	org_model "code.gitea.io/gitea/models/organization"
+	access_model "code.gitea.io/gitea/models/perm/access"
+	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/gitrepo"
@@ -117,7 +119,11 @@ func PullRequestCodeOwnersReview(ctx context.Context, issue *issues_model.Issue,
 	}
 
 	for _, u := range uniqUsers {
-		if u.ID != issue.Poster.ID {
+		permission, err := access_model.GetUserRepoPermission(ctx, issue.Repo, u)
+		if err != nil {
+			return nil, fmt.Errorf("GetUserRepoPermission: %w", err)
+		}
+		if u.ID != issue.Poster.ID && permission.CanRead(unit.TypePullRequests) {
 			comment, err := issues_model.AddReviewRequest(ctx, issue, u, issue.Poster)
 			if err != nil {
 				log.Warn("Failed add assignee user: %s to PR review: %s#%d, error: %s", u.Name, pr.BaseRepo.Name, pr.ID, err)

tests/integration/codeowner_test.go

@@ -14,6 +14,7 @@ import (
 	"testing"
 	"time"
 
+	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
 	repo_model "code.gitea.io/gitea/models/repo"
 	unit_model "code.gitea.io/gitea/models/unit"
@@ -159,5 +160,42 @@ func TestCodeOwner(t *testing.T) {
 			pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "branch"})
 			unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 4})
 		})
+
+		t.Run("Codeowner user with no permission", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			// Make repository private, only user2 (owner of repository) has now access to this repository.
+			repo.IsPrivate = true
+			_, err := db.GetEngine(db.DefaultContext).Cols("is_private").Update(repo)
+			require.NoError(t, err)
+
+			err = os.WriteFile(path.Join(dstPath, "README.md"), []byte("## very senstive info"), 0o666)
+			require.NoError(t, err)
+
+			err = git.AddChanges(dstPath, true)
+			require.NoError(t, err)
+
+			err = git.CommitChanges(dstPath, git.CommitChangesOptions{
+				Committer: &git.Signature{
+					Email: "user2@example.com",
+					Name:  "user2",
+					When:  time.Now(),
+				},
+				Author: &git.Signature{
+					Email: "user2@example.com",
+					Name:  "user2",
+					When:  time.Now(),
+				},
+				Message: "Add secrets to the README.",
+			})
+			require.NoError(t, err)
+
+			err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/main", "-o", "topic=codeowner-private").Run(&git.RunOpts{Dir: dstPath})
+			require.NoError(t, err)
+
+			// In CODEOWNERS file the codeowner for README.md is user5, but does not have access to this private repository.
+			pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "user2/codeowner-private"})
+			unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5})
+		})
 	})
 }