Filter private users from reviewer list

routers/api/v1/repo/collaborators.go

@@ -330,7 +330,7 @@ func GetReviewers(ctx *context.APIContext) {
 		return
 	}
 
-	reviewers, err := pull_service.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, 0)
+	reviewers, err := pull_service.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer, 0)
 	if err != nil {
 		ctx.APIErrorInternal(err)
 		return

routers/web/repo/issue_page_meta.go

@@ -278,7 +278,7 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) {
 
 	if data.CanChooseReviewer {
 		var err error
-		reviewers, err = pull_service.GetReviewers(ctx, repo, ctx.Doer.ID, posterID)
+		reviewers, err = pull_service.GetReviewers(ctx, repo, ctx.Doer, posterID)
 		if err != nil {
 			ctx.ServerError("GetReviewers", err)
 			return

services/pull/reviewer.go

@@ -22,7 +22,8 @@ import (
 // - For collaborator, all users that have read access or higher to the repository.
 // - For repository under organization, users under the teams which have read permission or higher of pull request unit
 // - Owner will be listed if it's not an organization, not the poster and not in the list of reviewers
-func GetReviewers(ctx context.Context, repo *repo_model.Repository, doerID, posterID int64) ([]*user_model.User, error) {
+// - Users with visibility the doer cannot see are filtered out via BuildCanSeeUserCondition
+func GetReviewers(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, posterID int64) ([]*user_model.User, error) {
 	if err := repo.LoadOwner(ctx); err != nil {
 		return nil, err
 	}
@@ -53,8 +54,17 @@ func GetReviewers(ctx context.Context, repo *repo_model.Repository, doerID, post
 	// and just waste 1 unit is cheaper than re-allocate memory once.
 	users := make([]*user_model.User, 0, len(uniqueUserIDs)+1)
 	if len(uniqueUserIDs) > 0 {
-		if err := e.In("id", uniqueUserIDs.Values()).
-			Where(builder.Eq{"`user`.is_active": true}).
+		cond := builder.And(
+			builder.In("`user`.id", uniqueUserIDs.Values()),
+			builder.Eq{"`user`.is_active": true},
+		)
+		// Hide users the doer cannot see (visibility=private when not in same org/self).
+		// BuildCanSeeUserCondition returns nil for admins (no extra filter).
+		// Regression guard for issue #37371.
+		if visCond := user_model.BuildCanSeeUserCondition(doer); visCond != nil {
+			cond = cond.And(visCond)
+		}
+		if err := e.Where(cond).
 			OrderBy(user_model.GetOrderByName()).
 			Find(&users); err != nil {
 			return nil, err

services/pull/reviewer_test.go

@@ -8,6 +8,7 @@ import (
 
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
 	pull_service "code.gitea.io/gitea/services/pull"
 
 	"github.com/stretchr/testify/assert"
@@ -16,46 +17,84 @@ import (
 func TestRepoGetReviewers(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	user11 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 11})
+
 	// test public repo
 	repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
 
 	ctx := t.Context()
-	reviewers, err := pull_service.GetReviewers(ctx, repo1, 2, 0)
+	reviewers, err := pull_service.GetReviewers(ctx, repo1, user2, 0)
 	assert.NoError(t, err)
 	if assert.Len(t, reviewers, 1) {
 		assert.ElementsMatch(t, []int64{2}, []int64{reviewers[0].ID})
 	}
 
 	// should not include doer and remove the poster
-	reviewers, err = pull_service.GetReviewers(ctx, repo1, 11, 2)
+	reviewers, err = pull_service.GetReviewers(ctx, repo1, user11, 2)
 	assert.NoError(t, err)
 	assert.Empty(t, reviewers)
 
 	// should not include PR poster, if PR poster would be otherwise eligible
-	reviewers, err = pull_service.GetReviewers(ctx, repo1, 11, 4)
+	reviewers, err = pull_service.GetReviewers(ctx, repo1, user11, 4)
 	assert.NoError(t, err)
 	assert.Len(t, reviewers, 1)
 
 	// test private user repo
 	repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
 
-	reviewers, err = pull_service.GetReviewers(ctx, repo2, 2, 4)
+	reviewers, err = pull_service.GetReviewers(ctx, repo2, user2, 4)
 	assert.NoError(t, err)
 	assert.Len(t, reviewers, 1)
 	assert.EqualValues(t, 2, reviewers[0].ID)
 
 	// test private org repo
 	repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
 
-	reviewers, err = pull_service.GetReviewers(ctx, repo3, 2, 1)
+	reviewers, err = pull_service.GetReviewers(ctx, repo3, user2, 1)
 	assert.NoError(t, err)
 	assert.Len(t, reviewers, 2)
 
-	reviewers, err = pull_service.GetReviewers(ctx, repo3, 2, 2)
+	reviewers, err = pull_service.GetReviewers(ctx, repo3, user2, 2)
 	assert.NoError(t, err)
 	assert.Len(t, reviewers, 1)
 }
 
+// TestRepoGetReviewersAppliesVisibility is a regression guard for issue
+// #37371: GetReviewers must apply user visibility rules to non-admin doers
+// (BuildCanSeeUserCondition). Restricted users that are explicit
+// collaborators must remain selectable as reviewers.
+func TestRepoGetReviewersAppliesVisibility(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	repo4 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4})
+
+	// user29 is a restricted collaborator with public visibility on repo4.
+	// As a collaborator they have access to the repo and must appear in the
+	// reviewer list for both admin and non-admin doers.
+	user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
+	assert.False(t, user5.IsAdmin)
+
+	reviewers, err := pull_service.GetReviewers(t.Context(), repo4, user5, 0)
+	assert.NoError(t, err)
+	seen := make(map[int64]bool, len(reviewers))
+	for _, u := range reviewers {
+		seen[u.ID] = true
+	}
+	assert.True(t, seen[29], "restricted collaborator with public visibility must be selectable")
+
+	admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+	assert.True(t, admin.IsAdmin)
+
+	reviewersAdmin, err := pull_service.GetReviewers(t.Context(), repo4, admin, 0)
+	assert.NoError(t, err)
+	seenAdmin := make(map[int64]bool, len(reviewersAdmin))
+	for _, u := range reviewersAdmin {
+		seenAdmin[u.ID] = true
+	}
+	assert.True(t, seenAdmin[29], "admin must see restricted collaborator")
+}
+
 func TestRepoGetReviewerTeams(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())