From 8287fb1eb52231b92b52fdde529af7c2a89cae57 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 17 Nov 2024 02:29:10 +0100 Subject: [PATCH] fix: check read permissions for code owner review requests - Only send a review request based on the code owner file if the code owner user has read permissions to the pull requests of that repository. - This avoids leaking title of PRs from private repository when a CODEOWNER file is present which contains users that do not have access to the private repository. - Found by @oliverpool. - Integration test added. --- services/issue/pull.go | 8 +++++- tests/integration/codeowner_test.go | 38 +++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/services/issue/pull.go b/services/issue/pull.go index 896802108d..3b61c00afa 100644 --- a/services/issue/pull.go +++ b/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) diff --git a/tests/integration/codeowner_test.go b/tests/integration/codeowner_test.go index b324711cb5..d49a13d5c4 100644 --- a/tests/integration/codeowner_test.go +++ b/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.ms 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}) + }) }) }