Go-Gitea
/
gitea
mirror of https://github.com/go-gitea/gitea.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

pull: Fix CODEOWNERS absolute path matching. (#37244) 

Patterns starting with "/" (e.g. /docs/.*\.md) never matched because git
returns relative paths without a leading slash. Strip the leading "/"
before compiling the regex since the ^...$ anchoring already provides
root-relative semantics.

Fixes: #28107
Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: Giteabot <teabot@gitea.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
pull/37260/head^2
JoeGruffins 2026-04-18 06:54:49 +09:00 committed by GitHub
parent e43422b042
commit 0161f3019b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 89 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
models/issues/pull.go
View File

@ -877,7 +877,12 @@ func ParseCodeOwnersLine(ctx context.Context, tokens []string) (*CodeOwnerRule,
warnings := make([]string, 0)
expr := fmt.Sprintf("^%s$", strings.TrimPrefix(tokens[0], "!"))
// Strip leading "!" for negative rules, then strip leading "/" since
// git returns relative paths (e.g. "docs/foo.md" not "/docs/foo.md")
// and the regex is already anchored with ^...$, so the "/" is redundant.
pattern := strings.TrimPrefix(tokens[0], "!")
pattern = strings.TrimPrefix(pattern, "/")
expr := fmt.Sprintf("^%s$", pattern)
rule.Rule, err = regexp2.Compile(expr, regexp2.None)
if err != nil {
warnings = append(warnings, fmt.Sprintf("incorrect codeowner regexp: %s", err))

132
models/issues/pull_test.go
View File

@ -17,16 +17,43 @@ import (
"github.com/stretchr/testify/require"
)
func TestPullRequest_LoadAttributes(t *testing.T) {
func TestPullRequest(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
t.Run("LoadAttributes", testPullRequestLoadAttributes)
t.Run("LoadIssue", testPullRequestLoadIssue)
t.Run("LoadBaseRepo", testPullRequestLoadBaseRepo)
t.Run("LoadHeadRepo", testPullRequestLoadHeadRepo)
t.Run("PullRequestsNewest", testPullRequestsNewest)
t.Run("PullRequestsOldest", testPullRequestsOldest)
t.Run("GetUnmergedPullRequest", testGetUnmergedPullRequest)
t.Run("HasUnmergedPullRequestsByHeadInfo", testHasUnmergedPullRequestsByHeadInfo)
t.Run("GetUnmergedPullRequestsByHeadInfo", testGetUnmergedPullRequestsByHeadInfo)
t.Run("GetUnmergedPullRequestsByBaseInfo", testGetUnmergedPullRequestsByBaseInfo)
t.Run("GetPullRequestByIndex", testGetPullRequestByIndex)
t.Run("GetPullRequestByID", testGetPullRequestByID)
t.Run("GetPullRequestByIssueID", testGetPullRequestByIssueID)
t.Run("PullRequest_UpdateCols", testPullRequestUpdateCols)
t.Run("PullRequest_IsWorkInProgress", testPullRequestIsWorkInProgress)
t.Run("PullRequest_GetWorkInProgressPrefixWorkInProgress", testPullRequestGetWorkInProgressPrefixWorkInProgress)
t.Run("DeleteOrphanedObjects", testDeleteOrphanedObjects)
t.Run("ParseCodeOwnersLine", testParseCodeOwnersLine)
t.Run("CodeOwnerAbsolutePathPatterns", testCodeOwnerAbsolutePathPatterns)
t.Run("GetApprovers", testGetApprovers)
t.Run("GetPullRequestByMergedCommit", testGetPullRequestByMergedCommit)
t.Run("Migrate_InsertPullRequests", testMigrateInsertPullRequests)
t.Run("PullRequestsClosedRecentSortType", testPullRequestsClosedRecentSortType)
t.Run("LoadRequestedReviewers", testLoadRequestedReviewers)
}
func testPullRequestLoadAttributes(t *testing.T) {
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1})
assert.NoError(t, pr.LoadAttributes(t.Context()))
assert.NotNil(t, pr.Merger)
assert.Equal(t, pr.MergerID, pr.Merger.ID)
}
func TestPullRequest_LoadIssue(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testPullRequestLoadIssue(t *testing.T) {
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1})
assert.NoError(t, pr.LoadIssue(t.Context()))
assert.NotNil(t, pr.Issue)
@ -36,8 +63,7 @@ func TestPullRequest_LoadIssue(t *testing.T) {
assert.Equal(t, int64(2), pr.Issue.ID)
}
func TestPullRequest_LoadBaseRepo(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testPullRequestLoadBaseRepo(t *testing.T) {
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1})
assert.NoError(t, pr.LoadBaseRepo(t.Context()))
assert.NotNil(t, pr.BaseRepo)
@ -47,8 +73,7 @@ func TestPullRequest_LoadBaseRepo(t *testing.T) {
assert.Equal(t, pr.BaseRepoID, pr.BaseRepo.ID)
}
func TestPullRequest_LoadHeadRepo(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testPullRequestLoadHeadRepo(t *testing.T) {
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1})
assert.NoError(t, pr.LoadHeadRepo(t.Context()))
assert.NotNil(t, pr.HeadRepo)
@ -59,8 +84,7 @@ func TestPullRequest_LoadHeadRepo(t *testing.T) {
// TODO TestNewPullRequest
func TestPullRequestsNewest(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testPullRequestsNewest(t *testing.T) {
prs, count, err := issues_model.PullRequests(t.Context(), 1, &issues_model.PullRequestsOptions{
ListOptions: db.ListOptions{
Page: 1,
@ -77,7 +101,7 @@ func TestPullRequestsNewest(t *testing.T) {
}
}
func TestPullRequests_Closed_RecentSortType(t *testing.T) {
func testPullRequestsClosedRecentSortType(t *testing.T) {
// Issue ID | Closed At. | Updated At
// 2 | 1707270001 | 1707270001
// 3 | 1707271000 | 1707279999
@ -90,7 +114,6 @@ func TestPullRequests_Closed_RecentSortType(t *testing.T) {
{"recentclose", []int64{11, 3, 2}},
}
assert.NoError(t, unittest.PrepareTestDatabase())
_, err := db.Exec(t.Context(), "UPDATE issue SET closed_unix = 1707270001, updated_unix = 1707270001, is_closed = true WHERE id = 2")
require.NoError(t, err)
_, err = db.Exec(t.Context(), "UPDATE issue SET closed_unix = 1707271000, updated_unix = 1707279999, is_closed = true WHERE id = 3")
@ -118,9 +141,7 @@ func TestPullRequests_Closed_RecentSortType(t *testing.T) {
}
}
func TestLoadRequestedReviewers(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testLoadRequestedReviewers(t *testing.T) {
pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
assert.NoError(t, pull.LoadIssue(t.Context()))
issue := pull.Issue
@ -146,8 +167,7 @@ func TestLoadRequestedReviewers(t *testing.T) {
assert.Empty(t, pull.RequestedReviewers)
}
func TestPullRequestsOldest(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testPullRequestsOldest(t *testing.T) {
prs, count, err := issues_model.PullRequests(t.Context(), 1, &issues_model.PullRequestsOptions{
ListOptions: db.ListOptions{
Page: 1,
@ -164,8 +184,7 @@ func TestPullRequestsOldest(t *testing.T) {
}
}
func TestGetUnmergedPullRequest(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testGetUnmergedPullRequest(t *testing.T) {
pr, err := issues_model.GetUnmergedPullRequest(t.Context(), 1, 1, "branch2", "master", issues_model.PullRequestFlowGithub)
assert.NoError(t, err)
assert.Equal(t, int64(2), pr.ID)
@ -175,9 +194,7 @@ func TestGetUnmergedPullRequest(t *testing.T) {
assert.True(t, issues_model.IsErrPullRequestNotExist(err))
}
func TestHasUnmergedPullRequestsByHeadInfo(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testHasUnmergedPullRequestsByHeadInfo(t *testing.T) {
exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(t.Context(), 1, "branch2")
assert.NoError(t, err)
assert.True(t, exist)
@ -187,8 +204,7 @@ func TestHasUnmergedPullRequestsByHeadInfo(t *testing.T) {
assert.False(t, exist)
}
func TestGetUnmergedPullRequestsByHeadInfo(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testGetUnmergedPullRequestsByHeadInfo(t *testing.T) {
prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(t.Context(), 1, "branch2")
assert.NoError(t, err)
assert.Len(t, prs, 1)
@ -198,8 +214,7 @@ func TestGetUnmergedPullRequestsByHeadInfo(t *testing.T) {
}
}
func TestGetUnmergedPullRequestsByBaseInfo(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testGetUnmergedPullRequestsByBaseInfo(t *testing.T) {
prs, err := issues_model.GetUnmergedPullRequestsByBaseInfo(t.Context(), 1, "master")
assert.NoError(t, err)
assert.Len(t, prs, 1)
@ -209,8 +224,7 @@ func TestGetUnmergedPullRequestsByBaseInfo(t *testing.T) {
assert.Equal(t, "master", pr.BaseBranch)
}
func TestGetPullRequestByIndex(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testGetPullRequestByIndex(t *testing.T) {
pr, err := issues_model.GetPullRequestByIndex(t.Context(), 1, 2)
assert.NoError(t, err)
assert.Equal(t, int64(1), pr.BaseRepoID)
@ -225,8 +239,7 @@ func TestGetPullRequestByIndex(t *testing.T) {
assert.True(t, issues_model.IsErrPullRequestNotExist(err))
}
func TestGetPullRequestByID(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testGetPullRequestByID(t *testing.T) {
pr, err := issues_model.GetPullRequestByID(t.Context(), 1)
assert.NoError(t, err)
assert.Equal(t, int64(1), pr.ID)
@ -237,8 +250,7 @@ func TestGetPullRequestByID(t *testing.T) {
assert.True(t, issues_model.IsErrPullRequestNotExist(err))
}
func TestGetPullRequestByIssueID(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testGetPullRequestByIssueID(t *testing.T) {
pr, err := issues_model.GetPullRequestByIssueID(t.Context(), 2)
assert.NoError(t, err)
assert.Equal(t, int64(2), pr.IssueID)
@ -248,8 +260,7 @@ func TestGetPullRequestByIssueID(t *testing.T) {
assert.True(t, issues_model.IsErrPullRequestNotExist(err))
}
func TestPullRequest_UpdateCols(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testPullRequestUpdateCols(t *testing.T) {
pr := &issues_model.PullRequest{
ID: 1,
BaseBranch: "baseBranch",
@ -265,9 +276,7 @@ func TestPullRequest_UpdateCols(t *testing.T) {
// TODO TestAddTestPullRequestTask
func TestPullRequest_IsWorkInProgress(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testPullRequestIsWorkInProgress(t *testing.T) {
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
pr.LoadIssue(t.Context())
@ -280,9 +289,7 @@ func TestPullRequest_IsWorkInProgress(t *testing.T) {
assert.True(t, pr.IsWorkInProgress(t.Context()))
}
func TestPullRequest_GetWorkInProgressPrefixWorkInProgress(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testPullRequestGetWorkInProgressPrefixWorkInProgress(t *testing.T) {
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
pr.LoadIssue(t.Context())
@ -296,9 +303,7 @@ func TestPullRequest_GetWorkInProgressPrefixWorkInProgress(t *testing.T) {
assert.Equal(t, "[wip]", pr.GetWorkInProgressPrefix(t.Context()))
}
func TestDeleteOrphanedObjects(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testDeleteOrphanedObjects(t *testing.T) {
countBefore, err := db.GetEngine(t.Context()).Count(&issues_model.PullRequest{})
assert.NoError(t, err)
@ -317,7 +322,7 @@ func TestDeleteOrphanedObjects(t *testing.T) {
assert.Equal(t, countBefore, countAfter)
}
func TestParseCodeOwnersLine(t *testing.T) {
func testParseCodeOwnersLine(t *testing.T) {
type CodeOwnerTest struct {
Line string
Tokens []string
@ -331,6 +336,8 @@ func TestParseCodeOwnersLine(t *testing.T) {
{Line: `docs/(aws|google|azure)/[^/]*\\.(md|txt) @org3 @org2/team2`, Tokens: []string{`docs/(aws|google|azure)/[^/]*\.(md|txt)`, "@org3", "@org2/team2"}},
{Line: `\#path @org3`, Tokens: []string{`#path`, "@org3"}},
{Line: `path\ with\ spaces/ @org3`, Tokens: []string{`path with spaces/`, "@org3"}},
{Line: `/docs/.*\\.md @user1`, Tokens: []string{`/docs/.*\.md`, "@user1"}},
{Line: `!/assets/.*\\.(bin|exe|msi) @user1`, Tokens: []string{`!/assets/.*\.(bin|exe|msi)`, "@user1"}},
}
for _, g := range given {
@ -339,8 +346,37 @@ func TestParseCodeOwnersLine(t *testing.T) {
}
}
func TestGetApprovers(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testCodeOwnerAbsolutePathPatterns(t *testing.T) {
type testCase struct {
content string
file string
expected bool
}
cases := []testCase{
// Absolute path pattern should match (leading "/" stripped)
{content: "/README.md @user5\n", file: "README.md", expected: true},
// Absolute path pattern in subdirectory
{content: "/docs/.* @user5\n", file: "docs/foo.md", expected: true},
// Absolute path should not match nested paths it shouldn't
{content: "/docs/.* @user5\n", file: "other/docs/foo.md", expected: false},
// Relative path still works
{content: "README.md @user5\n", file: "README.md", expected: true},
// Negated absolute path pattern
{content: "!/.* @user5\n", file: "README.md", expected: false},
}
for _, c := range cases {
rules, _ := issues_model.GetCodeOwnersFromContent(t.Context(), c.content)
require.NotEmpty(t, rules)
rule := rules[0]
regexpMatched, _ := rule.Rule.MatchString(c.file)
ruleMatched := regexpMatched == !rule.Negative
assert.Equal(t, c.expected, ruleMatched, "pattern %q against file %q", c.content, c.file)
}
}
func testGetApprovers(t *testing.T) {
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 5})
// Official reviews are already deduplicated. Allow unofficial reviews
// to assert that there are no duplicated approvers.
@ -350,8 +386,7 @@ func TestGetApprovers(t *testing.T) {
assert.Equal(t, expected, approvers)
}
func TestGetPullRequestByMergedCommit(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testGetPullRequestByMergedCommit(t *testing.T) {
pr, err := issues_model.GetPullRequestByMergedCommit(t.Context(), 1, "1a8823cd1a9549fde083f992f6b9b87a7ab74fb3")
assert.NoError(t, err)
assert.EqualValues(t, 1, pr.ID)
@ -362,8 +397,7 @@ func TestGetPullRequestByMergedCommit(t *testing.T) {
assert.ErrorAs(t, err, &issues_model.ErrPullRequestNotExist{})
}
func TestMigrate_InsertPullRequests(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testMigrateInsertPullRequests(t *testing.T) {
reponame := "repo1"
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{Name: reponame})
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})