From 02c03d69d35963e40f9662d4a793a55d78649444 Mon Sep 17 00:00:00 2001 From: Jorge <97680247+JorgeDLMV@users.noreply.github.com> Date: Wed, 28 Oct 2026 10:19:35 +0100 Subject: [PATCH] enhance(repo): allow system users in protected tag allowlist (#40038) Allowlisting gitea-actions lets any workflow of the repository with write access push matching tags, the same trade-off branches already have. Allowing gitea:deploy-key is new for tags. Co-authored-by: wxiaoguang --- models/user/user.go | 11 +++-- models/user/user_test.go | 2 +- routers/api/v1/repo/issue.go | 2 +- routers/api/v1/repo/issue_assignee.go | 2 +- routers/api/v1/repo/tag.go | 4 +- routers/private/hook_pre_receive_test.go | 59 +++++++++++++++++++++-- routers/web/repo/issue_new.go | 2 +- routers/web/repo/setting/protected_tag.go | 8 +-- templates/repo/settings/tags.tmpl | 16 +++--- tests/integration/repo_tag_test.go | 36 ++++++++++++++ 10 files changed, 116 insertions(+), 26 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 6da7cb68c53..66eff59ef42 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1056,8 +1056,9 @@ func GetUserByIDs(ctx context.Context, ids []int64) ([]*User, error) { // Glossaries for XxxUser: // * Possible: The user may or may not exist in the database, but we can still return a valid user object (e.g. Ghost user) for the given ID. // * Individual: The user is a real person, not an organization or a bot. -// * Doer: The user is the one performing an action -// * Valid: The user is an active user and not prohibited from signing in +// * Doer: The user is the one performing an action (individual, bot or system) +// * Valid: The user is an active and existing user and not prohibited from signing in +// * Real: The user is an existing one and not a system user // * System: Negative ID (e.g. Ghost, Actions, DeployKey, etc.) and not a real person, Ghost is not included if there is no "Possible" keyword // GetPossibleUserByID returns the possible user and its ID. If the user doesn't exist, it returns Ghost user @@ -1224,7 +1225,7 @@ func getDoerUsersByNames(ctx context.Context, names []string, allowSystemDoer bo return users, errFinal } -func GetDoerUsersByNames(ctx context.Context, names []string) ([]*User, error) { +func GetRealDoerUsersByNames(ctx context.Context, names []string) ([]*User, error) { return getDoerUsersByNames(ctx, names, false) } @@ -1232,9 +1233,9 @@ func GetDoerUserIDsByNamesWithSystem(ctx context.Context, names []string) ([]int return getDoerUserIDsByNames(ctx, names, true) } -// GetDoerUserIDsByNames returns a slice of ids corresponds to names. +// GetRealDoerUserIDsByNames returns a slice of ids corresponds to names. // It always returns found user IDs even if some names are not found, but returns an error if any name is not found. -func GetDoerUserIDsByNames(ctx context.Context, names []string) ([]int64, error) { +func GetRealDoerUserIDsByNames(ctx context.Context, names []string) ([]int64, error) { return getDoerUserIDsByNames(ctx, names, false) } diff --git a/models/user/user_test.go b/models/user/user_test.go index a720d9425df..c70879a0679 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -358,7 +358,7 @@ func TestCreateUserWithoutCustomTimestamps(t *testing.T) { func TestGetDoerUserIDsByNames(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - ids, err := user_model.GetDoerUserIDsByNames(t.Context(), []string{"user1", "user2", "none_existing_user"}) + ids, err := user_model.GetRealDoerUserIDsByNames(t.Context(), []string{"user1", "user2", "none_existing_user"}) assert.ErrorIs(t, err, util.ErrNotExist) assert.Equal(t, []int64{1, 2}, ids) diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 705bc1b751d..1e787244727 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -526,7 +526,7 @@ func loadAssigneeUserIDs(ctx *context.APIContext, oneAssignee string, multipleAs assigneeNames.Add(strings.ToLower(assignee)) } assigneeNames.Remove("") - assignees, err := user_model.GetDoerUsersByNames(ctx, assigneeNames.Values()) + assignees, err := user_model.GetRealDoerUsersByNames(ctx, assigneeNames.Values()) if err != nil { if errors.Is(err, util.ErrNotExist) { ctx.APIError(http.StatusUnprocessableEntity, fmt.Sprintf("Assignee does not exist: %s", err)) diff --git a/routers/api/v1/repo/issue_assignee.go b/routers/api/v1/repo/issue_assignee.go index 2fe242fbbdc..1aa8c07655e 100644 --- a/routers/api/v1/repo/issue_assignee.go +++ b/routers/api/v1/repo/issue_assignee.go @@ -186,7 +186,7 @@ func updateIssueAssignees(ctx *context.APIContext, opts api.IssueAssigneesOption return } - assigneeIDs, err := user_model.GetDoerUserIDsByNames(ctx, opts.Assignees) + assigneeIDs, err := user_model.GetRealDoerUserIDsByNames(ctx, opts.Assignees) if err != nil { if errors.Is(err, util.ErrNotExist) { ctx.APIError(http.StatusUnprocessableEntity, err.Error()) diff --git a/routers/api/v1/repo/tag.go b/routers/api/v1/repo/tag.go index cd76416055b..ccb1be22b9e 100644 --- a/routers/api/v1/repo/tag.go +++ b/routers/api/v1/repo/tag.go @@ -412,7 +412,7 @@ func CreateTagProtection(ctx *context.APIContext) { } var whitelistUsers, whitelistTeams []int64 - whitelistUsers, err = user_model.GetDoerUserIDsByNames(ctx, form.WhitelistUsernames) + whitelistUsers, err = user_model.GetDoerUserIDsByNamesWithSystem(ctx, form.WhitelistUsernames) if err != nil { if errors.Is(err, util.ErrNotExist) { ctx.APIError(http.StatusUnprocessableEntity, err.Error()) @@ -530,7 +530,7 @@ func EditTagProtection(ctx *context.APIContext) { } if form.WhitelistUsernames != nil { - whitelistUsers, err = user_model.GetDoerUserIDsByNames(ctx, form.WhitelistUsernames) + whitelistUsers, err = user_model.GetDoerUserIDsByNamesWithSystem(ctx, form.WhitelistUsernames) if err != nil { if errors.Is(err, util.ErrNotExist) { ctx.APIError(http.StatusUnprocessableEntity, err.Error()) diff --git a/routers/private/hook_pre_receive_test.go b/routers/private/hook_pre_receive_test.go index cb9deb8c435..1ee2d57fdd2 100644 --- a/routers/private/hook_pre_receive_test.go +++ b/routers/private/hook_pre_receive_test.go @@ -8,8 +8,10 @@ import ( "testing" "gitea.dev/models/db" + deploykey_model "gitea.dev/models/deploykey" git_model "gitea.dev/models/git" issues_model "gitea.dev/models/issues" + "gitea.dev/models/perm" repo_model "gitea.dev/models/repo" "gitea.dev/models/unit" "gitea.dev/models/unittest" @@ -22,8 +24,14 @@ import ( "github.com/stretchr/testify/require" ) -func TestPreReceiveActionsBranch(t *testing.T) { +func TestPreReceive(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) + t.Run("Branch", testPreReceiveActionsBranch) + t.Run("Tag", testPreReceiveSystemUserTag) + t.Run("CanWrite", testPreReceiveCanWriteCodePerBranch) +} + +func testPreReceiveActionsBranch(t *testing.T) { for _, tc := range []struct { name string protection *git_model.ProtectedBranchConfig @@ -91,13 +99,56 @@ func TestPreReceiveActionsBranch(t *testing.T) { } } +func testPreReceiveSystemUserTag(t *testing.T) { + deployKey, err := deploykey_model.AddDeployKeyToken(t.Context(), 2, "tag-pusher", perm.AccessModeWrite) + require.NoError(t, err) + actionsDoer := user_model.NewActionsUserWithTaskID(53) + deployKeyDoer := user_model.NewDeployKeyUserWithKeyID(deployKey.ID) + + for _, tc := range []struct { + name string + doer *user_model.User + allowlist []int64 + allowed bool + }{ + {name: "actions allowlisted", doer: actionsDoer, allowlist: []int64{user_model.ActionsUserID}, allowed: true}, + {name: "actions not allowlisted", doer: actionsDoer, allowlist: []int64{2, user_model.DeployKeyUserID}}, + {name: "deploy key allowlisted", doer: deployKeyDoer, allowlist: []int64{user_model.DeployKeyUserID}, allowed: true}, + {name: "deploy key not allowlisted", doer: deployKeyDoer, allowlist: []int64{2, user_model.ActionsUserID}}, + } { + t.Run(tc.name, func(t *testing.T) { + mockCtx, resp := contexttest.MockPrivateContext(t, "/") + ctx := &preReceiveContext{PrivateContext: mockCtx, opts: &private.HookOptions{UserID: tc.doer.ID}} + ctx.SetPathParam("owner", "user2") + ctx.SetPathParam("repo", "repo2") + RepoAssignment(ctx.PrivateContext) + require.False(t, ctx.Written()) + defer ctx.Repo.GitRepo.Close() + + require.True(t, loadContextDoerPermission(ctx.PrivateContext, tc.doer.ID, tc.doer.ExtDoerData.EncodeToString())) + + protectedTag := &git_model.ProtectedTag{RepoID: ctx.Repo.Repository.ID, NamePattern: "v*", AllowlistUserIDs: tc.allowlist} + require.NoError(t, git_model.InsertProtectedTag(t.Context(), protectedTag)) + defer func() { + require.NoError(t, git_model.DeleteProtectedTag(t.Context(), protectedTag)) + }() + + preReceiveTag(ctx, git.RefNameFromTag("v1.0.0")) + if tc.allowed { + assert.False(t, ctx.Written(), resp.Body.String()) + } else { + assert.Equal(t, http.StatusForbidden, resp.Code) + assert.Contains(t, resp.Body.String(), "Tag v1.0.0 is protected") + } + }) + } +} + // TestPreReceiveCanWriteCodePerBranch ensures the maintainer-edit write grant is evaluated against // the exact ref being pushed on every call, derived from that ref rather than shared mutable state. // Otherwise, a per-branch grant (an open PR with "allow edits from maintainers") could be batched // together with a protected branch or a tag to escalate into full repository write. -func TestPreReceiveCanWriteCodePerBranch(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - +func testPreReceiveCanWriteCodePerBranch(t *testing.T) { baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) headRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 11}) require.NoError(t, baseRepo.LoadOwner(t.Context())) diff --git a/routers/web/repo/issue_new.go b/routers/web/repo/issue_new.go index 6d82bda3f2e..e9bb3cdecd6 100644 --- a/routers/web/repo/issue_new.go +++ b/routers/web/repo/issue_new.go @@ -80,7 +80,7 @@ func setTemplateIfExists(ctx *context.Context, ctxDataKey string, possibleFiles metaData.LabelsData.SetSelectedLabelNames(template.Labels) } if metaData.AssigneesData.SelectedAssigneeIDs == "" { - selectedAssigneeIDs, _ := user_model.GetDoerUserIDsByNames(ctx, template.Assignees) + selectedAssigneeIDs, _ := user_model.GetRealDoerUserIDsByNames(ctx, template.Assignees) metaData.AssigneesData.SelectedAssigneeIDs = base.JoinInt64s(selectedAssigneeIDs, ",") } diff --git a/routers/web/repo/setting/protected_tag.go b/routers/web/repo/setting/protected_tag.go index e3efb39848c..c4312eb96a6 100644 --- a/routers/web/repo/setting/protected_tag.go +++ b/routers/web/repo/setting/protected_tag.go @@ -14,6 +14,7 @@ import ( "gitea.dev/models/perm" access_model "gitea.dev/models/perm/access" "gitea.dev/models/unit" + user_model "gitea.dev/models/user" "gitea.dev/modules/base" "gitea.dev/modules/setting" "gitea.dev/modules/templates" @@ -157,12 +158,13 @@ func setTagsContext(ctx *context.Context) error { } ctx.Data["ProtectedTags"] = protectedTags - users, err := access_model.GetUsersWithAnyUnitAccess(ctx, ctx.Repo.Repository, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) + repoUsers, err := access_model.GetUsersWithAnyUnitAccess(ctx, ctx.Repo.Repository, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) if err != nil { ctx.ServerError("GetUsersWithUnitAccess", err) return err } - ctx.Data["Users"] = users + users := []*user_model.User{user_model.NewActionsUser(), user_model.NewDeployKeyUser()} + ctx.Data["PushUsers"] = append(users, repoUsers...) if ctx.Repo.Owner.IsOrganization() { teams, err := organization.GetTeamsWithAccessToAnyRepoUnit(ctx, ctx.Repo.Owner.ID, ctx.Repo.Repository.ID, perm.AccessModeRead, unit.TypeCode, unit.TypePullRequests) @@ -170,7 +172,7 @@ func setTagsContext(ctx *context.Context) error { ctx.ServerError("Repo.Owner.TeamsWithAccessToRepo", err) return err } - ctx.Data["Teams"] = teams + ctx.Data["PushTeams"] = teams } return nil diff --git a/templates/repo/settings/tags.tmpl b/templates/repo/settings/tags.tmpl index d0ad2881f2d..60dde61c139 100644 --- a/templates/repo/settings/tags.tmpl +++ b/templates/repo/settings/tags.tmpl @@ -25,12 +25,12 @@
- {{template "repo/settings/user_dropdown" (dict "Name" "allowlist_users" "Value" .allowlist_users "Users" .Users)}} + {{template "repo/settings/user_dropdown" (dict "Name" "allowlist_users" "Value" .allowlist_users "Users" .PushUsers)}}
{{if .Owner.IsOrganization}}
- {{template "repo/settings/team_dropdown" (dict "Name" "allowlist_teams" "Value" .allowlist_teams "Teams" .Teams)}} + {{template "repo/settings/team_dropdown" (dict "Name" "allowlist_teams" "Value" .allowlist_teams "Teams" .PushTeams)}}
{{end}}
@@ -65,16 +65,16 @@ {{if or .AllowlistUserIDs (and $.Owner.IsOrganization .AllowlistTeamIDs)}} {{$userIDs := .AllowlistUserIDs}} - {{range $.Users}} - {{if SliceUtils.Contains $userIDs .ID}} - {{ctx.AvatarUtils.Avatar . 26}} {{.GetDisplayName}} + {{range $user := $.PushUsers}} + {{if SliceUtils.Contains $userIDs $user.ID}} + {{ctx.AvatarUtils.Avatar $user 20}} {{$user.GetDisplayName}} {{end}} {{end}} {{if $.Owner.IsOrganization}} {{$teamIDs := .AllowlistTeamIDs}} - {{range $.Teams}} - {{if SliceUtils.Contains $teamIDs .ID}} - {{.Name}} + {{range $team := $.PushTeams}} + {{if SliceUtils.Contains $teamIDs $team.ID}} + {{$team.Name}} {{end}} {{end}} {{end}} diff --git a/tests/integration/repo_tag_test.go b/tests/integration/repo_tag_test.go index a045b174b3a..b489d5bb68d 100644 --- a/tests/integration/repo_tag_test.go +++ b/tests/integration/repo_tag_test.go @@ -170,3 +170,39 @@ func TestRepushTag(t *testing.T) { assert.False(t, respRelease.IsDraft) }) } + +func TestProtectedTagSystemUsers(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + token := getUserToken(t, "user2", auth_model.AccessTokenScopeWriteRepository) + path := "/api/v1/repos/user2/repo1/tag_protections" + + t.Run("API", func(t *testing.T) { + resp := MakeRequest(t, NewRequestWithJSON(t, "POST", path, &api.CreateTagProtectionOption{ + NamePattern: "sys-*", WhitelistUsernames: []string{"user2", "gitea-actions", "gitea:deploy-key"}, + }).AddTokenAuth(token), http.StatusCreated) + pt := DecodeJSON(t, resp, &api.TagProtection{}) + assert.Equal(t, []string{"user2", "gitea-actions", "gitea:deploy-key"}, pt.WhitelistUsernames) + + resp = MakeRequest(t, NewRequestWithJSON(t, "PATCH", fmt.Sprintf("%s/%d", path, pt.ID), &api.EditTagProtectionOption{ + WhitelistUsernames: []string{"gitea-actions"}, + }).AddTokenAuth(token), http.StatusOK) + pt = DecodeJSON(t, resp, &api.TagProtection{}) + assert.Equal(t, []string{"gitea-actions"}, pt.WhitelistUsernames) + + MakeRequest(t, NewRequestWithJSON(t, "POST", path, &api.CreateTagProtectionOption{ + NamePattern: "missing-*", WhitelistUsernames: []string{"no-such-user"}, + }).AddTokenAuth(token), http.StatusUnprocessableEntity) + }) + + t.Run("Web", func(t *testing.T) { + session := loginUser(t, "user2") + resp := session.MakeRequest(t, NewRequest(t, "GET", "/user2/repo1/settings/tags"), http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + menu := htmlDoc.Find(`input[name="allowlist_users"] ~ .menu`) + assert.Equal(t, 1, menu.Find(fmt.Sprintf(`.item[data-value="%d"]`, user_model.ActionsUserID)).Length()) + assert.Equal(t, 1, menu.Find(fmt.Sprintf(`.item[data-value="%d"]`, user_model.DeployKeyUserID)).Length()) + assert.Contains(t, htmlDoc.Find("table").Text(), "gitea-actions") + assert.Zero(t, htmlDoc.Find(`table a[href$="/gitea-actions"]`).Length()) + }) +}