From 5da1d8dcd7aa59bd18c1ef9e1dfede093ab233db Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Thu, 30 Oct 2025 15:45:55 +0100 Subject: [PATCH] feat: add model pull request IsForkPullRequest helper So the logic by which a pull request is considered to be a fork from a security standpoint is in one place. --- models/issues/pull.go | 17 +++++++++++++++++ models/issues/pull_test.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/models/issues/pull.go b/models/issues/pull.go index 44ead3b9f1..6f65ef3af9 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -470,6 +470,23 @@ func (pr *PullRequest) GetReviewCommentsCount(ctx context.Context) int { return int(count) } +func (pr *PullRequest) IsForkPullRequest() bool { + var isForkPullRequest bool + + switch pr.Flow { + case PullRequestFlowGithub: + isForkPullRequest = pr.IsFromFork() + case PullRequestFlowAGit: + // there is no fork concept in AGit flow, anyone with read permission can push refs/for// to the repo. + // So we must treat it as a fork pull request because it may be from an untrusted user + isForkPullRequest = true + default: + // unknown flow, treat it as it's a fork pull request + isForkPullRequest = true + } + return isForkPullRequest +} + // IsChecking returns true if this pull request is still checking conflict. func (pr *PullRequest) IsChecking() bool { return pr.Status == PullRequestStatusChecking diff --git a/models/issues/pull_test.go b/models/issues/pull_test.go index db10386b5a..dd97ed34b2 100644 --- a/models/issues/pull_test.go +++ b/models/issues/pull_test.go @@ -466,6 +466,43 @@ func TestGetPullRequestByMergedCommit(t *testing.T) { require.ErrorAs(t, err, &issues_model.ErrPullRequestNotExist{}) } +func TestPullRequest_IsForkPullRequest(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("FlowGithub from a fork", func(t *testing.T) { + pr := &issues_model.PullRequest{ + Flow: issues_model.PullRequestFlowGithub, + HeadRepoID: 111, + BaseRepoID: 222, + } + assert.True(t, pr.IsForkPullRequest()) + }) + + t.Run("FlowGithub from the same repository", func(t *testing.T) { + pr := &issues_model.PullRequest{ + Flow: issues_model.PullRequestFlowGithub, + HeadRepoID: 111, + BaseRepoID: 111, + } + assert.False(t, pr.IsForkPullRequest()) + }) + + t.Run("PullRequestFlowAGit", func(t *testing.T) { + pr := &issues_model.PullRequest{ + Flow: issues_model.PullRequestFlowAGit, + } + assert.True(t, pr.IsForkPullRequest()) + }) + + t.Run("Other", func(t *testing.T) { + unknown := issues_model.PullRequestFlow(4854) + pr := &issues_model.PullRequest{ + Flow: unknown, + } + assert.True(t, pr.IsForkPullRequest()) + }) +} + func TestMigrate_InsertPullRequests(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) reponame := "repo1"