efforting.tech/gitea
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

Fix issue label deletion with Actions tokens (#37013)

Browse Source 
Use shared repo permission resolution for Actions task users in issue
label remove and clear paths, and add a regression test for deleting
issue labels with a Gitea Actions token.

This fixes issue label deletion when the request is authenticated with a
Gitea Actions token.
Fixes #37011 

The bug was that the delete path re-resolved repository permissions
using the normal user permission helper, which does not handle Actions
task users. As a result, `DELETE
/api/v1/repos/{owner}/{repo}/issues/{index}/labels/{id}` could return
`500` for Actions tokens even though label listing and label addition
worked.

---------

Co-authored-by: Codex <codex@openai.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Giteabot <teabot@gitea.io>
This commit is contained in:
Nicolas
2026-03-29 11:21:14 +02:00
committed by GitHub
parent a1b0bffd0c
commit db7eb4d51b
61 changed files with 268 additions and 181 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
models/git/protected_branch.go
View File

@@ -495,9 +495,9 @@ func updateUserWhitelist(ctx context.Context, repo *repo_model.Repository, curre
if err != nil {
return nil, fmt.Errorf("GetUserByID [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err)
}
perm, err := access_model.GetUserRepoPermission(ctx, repo, user)
perm, err := access_model.GetIndividualUserRepoPermission(ctx, repo, user)
if err != nil {
return nil, fmt.Errorf("GetUserRepoPermission [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err)
return nil, fmt.Errorf("GetIndividualUserRepoPermission [user_id: %d, repo_id: %d]: %v", userID, repo.ID, err)
}
if !perm.CanWrite(unit.TypeCode) {

2
models/issues/issue_label.go
View File

@@ -356,7 +356,7 @@ func ClearIssueLabels(ctx context.Context, issue *Issue, doer *user_model.User)
return err
}
perm, err := access_model.GetUserRepoPermission(ctx, issue.Repo, doer)
perm, err := access_model.GetDoerRepoPermission(ctx, issue.Repo, doer)
if err != nil {
return err
}

4
models/issues/issue_update.go
View File

@@ -665,9 +665,9 @@ func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *u
continue
}
// Normal users must have read access to the referencing issue
perm, err := access_model.GetUserRepoPermission(ctx, issue.Repo, user)
perm, err := access_model.GetIndividualUserRepoPermission(ctx, issue.Repo, user)
if err != nil {
return nil, fmt.Errorf("GetUserRepoPermission [%d]: %w", user.ID, err)
return nil, fmt.Errorf("GetIndividualUserRepoPermission [%d]: %w", user.ID, err)
}
if !perm.CanReadIssuesOrPulls(issue.IsPull) {
continue

2
models/issues/issue_xref.go
View File

@@ -207,7 +207,7 @@ func (issue *Issue) verifyReferencedIssue(stdCtx context.Context, ctx *crossRefe
// Check doer permissions; set action to None if the doer can't change the destination
if refIssue.RepoID != ctx.OrigIssue.RepoID || ref.Action != references.XRefActionNone {
perm, err := access_model.GetUserRepoPermission(stdCtx, refIssue.Repo, ctx.Doer)
perm, err := access_model.GetDoerRepoPermission(stdCtx, refIssue.Repo, ctx.Doer)
if err != nil {
return nil, references.XRefActionNone, err
}

2
models/issues/pull_list.go
View File

@@ -93,7 +93,7 @@ func CanMaintainerWriteToBranch(ctx context.Context, p access_model.Permission,
if err != nil {
continue
}
prPerm, err := access_model.GetUserRepoPermission(ctx, pr.BaseRepo, user)
prPerm, err := access_model.GetIndividualUserRepoPermission(ctx, pr.BaseRepo, user)
if err != nil {
continue
}

2
models/issues/review.go
View File

@@ -919,7 +919,7 @@ func CanMarkConversation(ctx context.Context, issue *Issue, doer *user_model.Use
return false, nil
}
if doer.ID != issue.PosterID {
p, err := access_model.GetUserRepoPermission(ctx, issue.Repo, doer)
p, err := access_model.GetDoerRepoPermission(ctx, issue.Repo, doer)
if err != nil {
return false, err
}

30
models/perm/access/repo_permission.go
View File

@@ -331,7 +331,7 @@ func GetActionsUserRepoPermission(ctx context.Context, repo *repo_model.Reposito
// Check permission like simple user but limit to read-only (PR #36095)
// Enhanced to also grant read-only access if isSameRepo is true and target repository is public
botPerm, err := GetUserRepoPermission(ctx, repo, user_model.NewActionsUser())
botPerm, err := GetIndividualUserRepoPermission(ctx, repo, user_model.NewActionsUser())
if err != nil {
return perm, err
}
@@ -379,8 +379,19 @@ func GetActionsUserRepoPermission(ctx context.Context, repo *repo_model.Reposito
return perm, nil
}
// GetUserRepoPermission returns the user permissions to the repository
func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (perm Permission, err error) {
// GetDoerRepoPermission returns the repository permission for the current actor,
// dispatching to GetActionsUserRepoPermission when the actor is an Actions token user.
func GetDoerRepoPermission(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (Permission, error) {
if taskID, ok := user_model.GetActionsUserTaskID(user); ok {
return GetActionsUserRepoPermission(ctx, repo, user, taskID)
}
return GetIndividualUserRepoPermission(ctx, repo, user)
}
// GetIndividualUserRepoPermission returns the permissions for an explicit user identity.
// In most request paths, callers should use GetDoerRepoPermission instead.
// Unlike GetDoerRepoPermission, this helper does not resolve Actions task users.
func GetIndividualUserRepoPermission(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (perm Permission, err error) {
defer func() {
if err == nil {
finalProcessRepoUnitPermission(user, &perm)
@@ -539,8 +550,9 @@ func AccessLevel(ctx context.Context, user *user_model.User, repo *repo_model.Re
// AccessLevelUnit returns the Access a user has to a repository's. Will return NoneAccess if the
// user does not have access.
// This helper only supports explicit user identities and does not resolve Actions task users.
func AccessLevelUnit(ctx context.Context, user *user_model.User, repo *repo_model.Repository, unitType unit.Type) (perm_model.AccessMode, error) { //nolint:revive // export stutter
perm, err := GetUserRepoPermission(ctx, repo, user)
perm, err := GetIndividualUserRepoPermission(ctx, repo, user)
if err != nil {
return perm_model.AccessModeNone, err
}
@@ -559,7 +571,7 @@ func CanBeAssigned(ctx context.Context, user *user_model.User, repo *repo_model.
if user.IsOrganization() {
return false, fmt.Errorf("organization can't be added as assignee [user_id: %d, repo_id: %d]", user.ID, repo.ID)
}
perm, err := GetUserRepoPermission(ctx, repo, user)
perm, err := GetIndividualUserRepoPermission(ctx, repo, user)
if err != nil {
return false, err
}
@@ -568,6 +580,7 @@ func CanBeAssigned(ctx context.Context, user *user_model.User, repo *repo_model.
}
// HasAnyUnitAccess see the comment of "perm.HasAnyUnitAccess"
// This helper only supports explicit user identities and does not resolve Actions task users.
func HasAnyUnitAccess(ctx context.Context, userID int64, repo *repo_model.Repository) (bool, error) {
var user *user_model.User
var err error
@@ -577,7 +590,7 @@ func HasAnyUnitAccess(ctx context.Context, userID int64, repo *repo_model.Reposi
return false, err
}
}
perm, err := GetUserRepoPermission(ctx, repo, user)
perm, err := GetIndividualUserRepoPermission(ctx, repo, user)
if err != nil {
return false, err
}
@@ -625,13 +638,14 @@ func GetUserIDsWithUnitAccess(ctx context.Context, repo *repo_model.Repository,
}
// CheckRepoUnitUser check whether user could visit the unit of this repository
// This helper only supports explicit user identities and does not resolve Actions task users.
func CheckRepoUnitUser(ctx context.Context, repo *repo_model.Repository, user *user_model.User, unitType unit.Type) bool {
if user != nil && user.IsAdmin {
return true
}
perm, err := GetUserRepoPermission(ctx, repo, user)
perm, err := GetIndividualUserRepoPermission(ctx, repo, user)
if err != nil {
log.Error("GetUserRepoPermission: %w", err)
log.Error("GetIndividualUserRepoPermission: %w", err)
return false
}

44
models/perm/access/repo_permission_test.go
View File

@@ -6,6 +6,7 @@ package access
import (
"testing"
actions_model "code.gitea.io/gitea/models/actions"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
perm_model "code.gitea.io/gitea/models/perm"
@@ -157,8 +158,13 @@ func TestUnitAccessMode(t *testing.T) {
assert.Equal(t, perm_model.AccessModeRead, perm.UnitAccessMode(unit.TypeWiki), "has unit, and map, use map")
}
func TestGetUserRepoPermission(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func TestGetRepoPermission(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
t.Run("GetIndividualUserRepoPermission", testGetIndividualUserRepoPermission)
t.Run("GetDoerRepoPermission", testGetDoerRepoPermission)
}
func testGetIndividualUserRepoPermission(t *testing.T) {
ctx := t.Context()
repo32 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 32}) // org public repo
require.NoError(t, repo32.LoadOwner(ctx))
@@ -172,7 +178,7 @@ func TestGetUserRepoPermission(t *testing.T) {
require.NoError(t, db.Insert(ctx, &organization.TeamUser{OrgID: org.ID, TeamID: team.ID, UID: user.ID}))
t.Run("DoerInTeamWithNoRepo", func(t *testing.T) {
perm, err := GetUserRepoPermission(ctx, repo32, user)
perm, err := GetIndividualUserRepoPermission(ctx, repo32, user)
require.NoError(t, err)
assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode)
assert.Nil(t, perm.unitsMode) // doer in the team, but has no access to the repo
@@ -181,7 +187,7 @@ func TestGetUserRepoPermission(t *testing.T) {
require.NoError(t, db.Insert(ctx, &organization.TeamRepo{OrgID: org.ID, TeamID: team.ID, RepoID: repo32.ID}))
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeCode, AccessMode: perm_model.AccessModeNone}))
t.Run("DoerWithTeamUnitAccessNone", func(t *testing.T) {
perm, err := GetUserRepoPermission(ctx, repo32, user)
perm, err := GetIndividualUserRepoPermission(ctx, repo32, user)
require.NoError(t, err)
assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode)
assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeCode])
@@ -191,7 +197,7 @@ func TestGetUserRepoPermission(t *testing.T) {
require.NoError(t, db.TruncateBeans(ctx, &organization.TeamUnit{}))
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeCode, AccessMode: perm_model.AccessModeWrite}))
t.Run("DoerWithTeamUnitAccessWrite", func(t *testing.T) {
perm, err := GetUserRepoPermission(ctx, repo32, user)
perm, err := GetIndividualUserRepoPermission(ctx, repo32, user)
require.NoError(t, err)
assert.Equal(t, perm_model.AccessModeRead, perm.AccessMode)
assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeCode])
@@ -204,7 +210,7 @@ func TestGetUserRepoPermission(t *testing.T) {
require.NoError(t, db.TruncateBeans(ctx, &organization.TeamUnit{}, &Access{})) // The user has access set of that repo, remove it, it is useless for our test
require.NoError(t, db.Insert(ctx, &organization.TeamRepo{OrgID: org.ID, TeamID: team.ID, RepoID: repo3.ID}))
t.Run("DoerWithNoopTeamOnPrivateRepo", func(t *testing.T) {
perm, err := GetUserRepoPermission(ctx, repo3, user)
perm, err := GetIndividualUserRepoPermission(ctx, repo3, user)
require.NoError(t, err)
assert.Equal(t, perm_model.AccessModeNone, perm.AccessMode)
assert.Equal(t, perm_model.AccessModeNone, perm.unitsMode[unit.TypeCode])
@@ -214,7 +220,7 @@ func TestGetUserRepoPermission(t *testing.T) {
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeCode, AccessMode: perm_model.AccessModeNone}))
require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeIssues, AccessMode: perm_model.AccessModeRead}))
t.Run("DoerWithReadIssueTeamOnPrivateRepo", func(t *testing.T) {
perm, err := GetUserRepoPermission(ctx, repo3, user)
perm, err := GetIndividualUserRepoPermission(ctx, repo3, user)
require.NoError(t, err)
assert.Equal(t, perm_model.AccessModeNone, perm.AccessMode)
assert.Equal(t, perm_model.AccessModeNone, perm.unitsMode[unit.TypeCode])
@@ -233,7 +239,7 @@ func TestGetUserRepoPermission(t *testing.T) {
require.NoError(t, db.Insert(ctx, repo_model.Collaboration{RepoID: repo3.ID, UserID: user.ID, Mode: perm_model.AccessModeWrite}))
require.NoError(t, db.Insert(ctx, Access{RepoID: repo3.ID, UserID: user.ID, Mode: perm_model.AccessModeWrite}))
t.Run("DoerWithReadIssueTeamAndWriteCollaboratorOnPrivateRepo", func(t *testing.T) {
perm, err := GetUserRepoPermission(ctx, repo3, user)
perm, err := GetIndividualUserRepoPermission(ctx, repo3, user)
require.NoError(t, err)
assert.Equal(t, perm_model.AccessModeWrite, perm.AccessMode)
assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeCode])
@@ -245,3 +251,25 @@ func TestGetUserRepoPermission(t *testing.T) {
assert.Equal(t, user.ID, users[0].ID)
})
}
func testGetDoerRepoPermission(t *testing.T) {
ctx := t.Context()
repo4 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4})
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
task47 := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 47})
actionsDoer := user_model.NewActionsUserWithTaskID(task47.ID)
regularUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
actionsPerm, err := GetDoerRepoPermission(ctx, repo4, actionsDoer)
require.NoError(t, err)
directPerm, err := GetActionsUserRepoPermission(ctx, repo4, actionsDoer, task47.ID)
require.NoError(t, err)
assert.Equal(t, directPerm, actionsPerm)
doerPerm, err := GetDoerRepoPermission(ctx, repo1, regularUser)
require.NoError(t, err)
individualPerm, err := GetIndividualUserRepoPermission(ctx, repo1, regularUser)
require.NoError(t, err)
assert.Equal(t, individualPerm, doerPerm)
}