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 <wxiaoguang@gmail.com>

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)
 }
 

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)
 

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))

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())

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())

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()))

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, ",")
 		}
 

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

templates/repo/settings/tags.tmpl

@@ -25,12 +25,12 @@
 								</div>
 								<div class="whitelist field">
 									<label>{{ctx.Locale.Tr "repo.settings.tags.protection.allowed.users"}}</label>
-									{{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)}}
 								</div>
 								{{if .Owner.IsOrganization}}
 									<div class="whitelist field">
 										<label>{{ctx.Locale.Tr "repo.settings.tags.protection.allowed.teams"}}</label>
-										{{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)}}
 									</div>
 								{{end}}
 								<div class="field">
@@ -65,16 +65,16 @@
 										<td>
 											{{if or .AllowlistUserIDs (and $.Owner.IsOrganization .AllowlistTeamIDs)}}
 												{{$userIDs := .AllowlistUserIDs}}
-												{{range $.Users}}
-													{{if SliceUtils.Contains $userIDs .ID}}
-														<a class="ui basic label" href="{{.HomeLink}}">{{ctx.AvatarUtils.Avatar . 26}} {{.GetDisplayName}}</a>
+												{{range $user := $.PushUsers}}
+													{{if SliceUtils.Contains $userIDs $user.ID}}
+														<a class="ui basic label" {{if gt $user.ID 0}}href="{{$user.HomeLink}}"{{end}}>{{ctx.AvatarUtils.Avatar $user 20}} {{$user.GetDisplayName}}</a>
 													{{end}}
 												{{end}}
 												{{if $.Owner.IsOrganization}}
 													{{$teamIDs := .AllowlistTeamIDs}}
-													{{range $.Teams}}
-														{{if SliceUtils.Contains $teamIDs .ID}}
-															<a class="ui basic label" href="{{$.Owner.OrganisationLink}}/teams/{{PathEscape .LowerName}}">{{.Name}}</a>
+													{{range $team := $.PushTeams}}
+														{{if SliceUtils.Contains $teamIDs $team.ID}}
+															<a class="ui basic label" href="{{$.Owner.OrganisationLink}}/teams/{{PathEscape $team.LowerName}}">{{$team.Name}}</a>
 														{{end}}
 													{{end}}
 												{{end}}

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())
+	})
+}