From 20597e1eddfe0a3a293b74747f7425c3da54c193 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Fri, 17 Oct 2025 15:44:34 +0200 Subject: [PATCH 01/22] Refactor Actions Token Access * allow public repositories --- models/perm/access/repo_permission.go | 29 +++++++++++++++++++++++++++ routers/api/v1/api.go | 19 +----------------- routers/web/repo/githttp.go | 25 ++++++----------------- services/lfs/server.go | 25 +++++++++-------------- 4 files changed, 45 insertions(+), 53 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 678b18442ee57..603a0c9c58161 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -8,6 +8,7 @@ import ( "fmt" "slices" + 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" @@ -253,6 +254,34 @@ func finalProcessRepoUnitPermission(user *user_model.User, perm *Permission) { } } +// GetActionsUserRepoPermission returns the actions user permissions to the repository +func GetActionsUserRepoPermission(ctx context.Context, repo *repo_model.Repository, actionsUser *user_model.User, taskID int64) (perm Permission, err error) { + if actionsUser.ID != user_model.ActionsUserID { + setting.PanicInDevOrTesting("GetActionsUserRepoPermission can only be called by the actions user") + } + task, err := actions_model.GetTaskByID(ctx, taskID) + if err != nil { + return perm, err + } + if task.RepoID != repo.ID { + // Allow public repo read access + return GetUserRepoPermission(ctx, repo, actionsUser) + } + + var accessMode perm_model.AccessMode + if task.IsForkPullRequest { + accessMode = perm_model.AccessModeRead + } else { + accessMode = perm_model.AccessModeWrite + } + + if err := repo.LoadUnits(ctx); err != nil { + return perm, err + } + perm.SetUnitsWithDefaultAccessMode(repo.Units, accessMode) + 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) { defer func() { diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 5f52ee8a4323d..e6238acce04f2 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -70,7 +70,6 @@ import ( "net/http" "strings" - actions_model "code.gitea.io/gitea/models/actions" auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/perm" @@ -190,27 +189,11 @@ func repoAssignment() func(ctx *context.APIContext) { if ctx.Doer != nil && ctx.Doer.ID == user_model.ActionsUserID { taskID := ctx.Data["ActionsTaskID"].(int64) - task, err := actions_model.GetTaskByID(ctx, taskID) + ctx.Repo.Permission, err = access_model.GetActionsUserRepoPermission(ctx, repo, ctx.Doer, taskID) if err != nil { ctx.APIErrorInternal(err) return } - if task.RepoID != repo.ID { - ctx.APIErrorNotFound() - return - } - - if task.IsForkPullRequest { - ctx.Repo.Permission.AccessMode = perm.AccessModeRead - } else { - ctx.Repo.Permission.AccessMode = perm.AccessModeWrite - } - - if err := ctx.Repo.Repository.LoadUnits(ctx); err != nil { - ctx.APIErrorInternal(err) - return - } - ctx.Repo.Permission.SetUnitsWithDefaultAccessMode(ctx.Repo.Repository.Units, ctx.Repo.Permission.AccessMode) } else { needTwoFactor, err := doerNeedTwoFactorAuth(ctx, ctx.Doer) if err != nil { diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index 1b7e75f84e0b0..6aa28ef1cf9e5 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -19,7 +19,6 @@ import ( "sync" "time" - actions_model "code.gitea.io/gitea/models/actions" auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" @@ -190,29 +189,17 @@ func httpBase(ctx *context.Context) *serviceHandler { if ctx.Data["IsActionsToken"] == true { taskID := ctx.Data["ActionsTaskID"].(int64) - task, err := actions_model.GetTaskByID(ctx, taskID) + p, err := access_model.GetActionsUserRepoPermission(ctx, repo, ctx.Doer, taskID) if err != nil { - ctx.ServerError("GetTaskByID", err) - return nil - } - if task.RepoID != repo.ID { - ctx.PlainText(http.StatusForbidden, "User permission denied") + ctx.ServerError("GetUserRepoPermission", err) return nil } - if task.IsForkPullRequest { - if accessMode > perm.AccessModeRead { - ctx.PlainText(http.StatusForbidden, "User permission denied") - return nil - } - environ = append(environ, fmt.Sprintf("%s=%d", repo_module.EnvActionPerm, perm.AccessModeRead)) - } else { - if accessMode > perm.AccessModeWrite { - ctx.PlainText(http.StatusForbidden, "User permission denied") - return nil - } - environ = append(environ, fmt.Sprintf("%s=%d", repo_module.EnvActionPerm, perm.AccessModeWrite)) + if !p.CanAccess(accessMode, unitType) { + ctx.PlainText(http.StatusNotFound, "Repository not found") + return nil } + environ = append(environ, fmt.Sprintf("%s=%d", repo_module.EnvActionPerm, p.UnitAccessMode(unitType))) } else { p, err := access_model.GetUserRepoPermission(ctx, repo, ctx.Doer) if err != nil { diff --git a/services/lfs/server.go b/services/lfs/server.go index 9f2e532f23ae8..b6dfe6edbc53e 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -19,7 +19,6 @@ import ( "strconv" "strings" - actions_model "code.gitea.io/gitea/models/actions" auth_model "code.gitea.io/gitea/models/auth" git_model "code.gitea.io/gitea/models/git" perm_model "code.gitea.io/gitea/models/perm" @@ -519,28 +518,22 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho accessMode = perm_model.AccessModeWrite } + var perm access_model.Permission + var err error if ctx.Data["IsActionsToken"] == true { taskID := ctx.Data["ActionsTaskID"].(int64) - task, err := actions_model.GetTaskByID(ctx, taskID) + perm, err = access_model.GetActionsUserRepoPermission(ctx, repository, ctx.Doer, taskID) if err != nil { - log.Error("Unable to GetTaskByID for task[%d] Error: %v", taskID, err) + log.Error("Unable to GetActionsUserRepoPermission for task[%d] Error: %v", taskID, err) return false } - if task.RepoID != repository.ID { + } else { + // ctx.IsSigned is unnecessary here, this will be checked in perm.CanAccess + perm, err = access_model.GetUserRepoPermission(ctx, repository, ctx.Doer) + if err != nil { + log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", ctx.Doer, repository, err) return false } - - if task.IsForkPullRequest { - return accessMode <= perm_model.AccessModeRead - } - return accessMode <= perm_model.AccessModeWrite - } - - // ctx.IsSigned is unnecessary here, this will be checked in perm.CanAccess - perm, err := access_model.GetUserRepoPermission(ctx, repository, ctx.Doer) - if err != nil { - log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", ctx.Doer, repository, err) - return false } canRead := perm.CanAccess(accessMode, unit.TypeCode) From 3fc4f64f5103980de37d745c87e3ab2bfe249ae4 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 19 Oct 2025 13:45:39 +0200 Subject: [PATCH 02/22] block public read access --- models/perm/access/repo_permission.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 603a0c9c58161..92bec14523d08 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -264,8 +264,8 @@ func GetActionsUserRepoPermission(ctx context.Context, repo *repo_model.Reposito return perm, err } if task.RepoID != repo.ID { - // Allow public repo read access - return GetUserRepoPermission(ctx, repo, actionsUser) + // FIXME allow public repo read access if tokenless pull is enabled + return perm, nil } var accessMode perm_model.AccessMode From c9f6e9c23d020eae4bd19d1c1f6d71813cf06543 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 19 Oct 2025 13:48:07 +0200 Subject: [PATCH 03/22] Prevent create orgs --- models/user/user_system.go | 21 +++---- tests/integration/actions_job_token_test.go | 70 +++++++++++++++++++++ 2 files changed, 80 insertions(+), 11 deletions(-) create mode 100644 tests/integration/actions_job_token_test.go diff --git a/models/user/user_system.go b/models/user/user_system.go index e07274d291eb5..11008c77d4544 100644 --- a/models/user/user_system.go +++ b/models/user/user_system.go @@ -48,17 +48,16 @@ func IsGiteaActionsUserName(name string) bool { // NewActionsUser creates and returns a fake user for running the actions. func NewActionsUser() *User { return &User{ - ID: ActionsUserID, - Name: ActionsUserName, - LowerName: ActionsUserName, - IsActive: true, - FullName: "Gitea Actions", - Email: ActionsUserEmail, - KeepEmailPrivate: true, - LoginName: ActionsUserName, - Type: UserTypeBot, - AllowCreateOrganization: true, - Visibility: structs.VisibleTypePublic, + ID: ActionsUserID, + Name: ActionsUserName, + LowerName: ActionsUserName, + IsActive: true, + FullName: "Gitea Actions", + Email: ActionsUserEmail, + KeepEmailPrivate: true, + LoginName: ActionsUserName, + Type: UserTypeBot, + Visibility: structs.VisibleTypePublic, } } diff --git a/tests/integration/actions_job_token_test.go b/tests/integration/actions_job_token_test.go new file mode 100644 index 0000000000000..5df33a01e0840 --- /dev/null +++ b/tests/integration/actions_job_token_test.go @@ -0,0 +1,70 @@ +package integration + +import ( + "encoding/base64" + "net/url" + "testing" + + actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/structs" + "github.com/stretchr/testify/require" +) + +func TestActionsJobTokenAccess(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + t.Run("Write Access", testActionsJobTokenAccess(u, false)) + t.Run("Read Access", testActionsJobTokenAccess(u, true)) + }) +} + +func testActionsJobTokenAccess(u *url.URL, isFork bool) func(t *testing.T) { + return func(t *testing.T) { + task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 47}) + require.NoError(t, task.GenerateToken()) + task.Status = actions_model.StatusRunning + task.IsForkPullRequest = isFork + err := actions_model.UpdateTask(t.Context(), task, "token_hash", "token_salt", "token_last_eight", "status", "is_fork_pull_request") + require.NoError(t, err) + session := emptyTestSession(t) + context := APITestContext{ + Session: session, + Token: task.Token, + Username: "user5", + Reponame: "repo4", + } + dstPath := t.TempDir() + + u.Path = context.GitPath() + u.User = url.UserPassword("gitea-actions", task.Token) + + t.Run("Git Clone", doGitClone(dstPath, u)) + + t.Run("API Get Repository", doAPIGetRepository(context, func(t *testing.T, r structs.Repository) { + require.EqualValues(t, "repo4", r.Name) + require.EqualValues(t, "user5", r.Owner.UserName) + })) + + if isFork { + context.ExpectedCode = 403 + } + t.Run("API Create File", doAPICreateFile(context, "test.txt", &structs.CreateFileOptions{ + FileOptions: structs.FileOptions{ + NewBranchName: "new-branch", + Message: "Create File", + }, + ContentBase64: base64.StdEncoding.EncodeToString([]byte(`This is a test file created using job token.`)), + })) + + context.ExpectedCode = 500 + t.Run("Fail to Create Repository", doAPICreateRepository(context, true)) + + context.ExpectedCode = 403 + t.Run("Fail to Delete Repository", doAPIDeleteRepository(context)) + + t.Run("Fail to Create Organization", doAPICreateOrganization(context, &structs.CreateOrgOption{ + UserName: "actions", + FullName: "Gitea Actions", + })) + } +} From 3c03b2b5523fcfbf45af7a0c7a77743a8785427f Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 19 Oct 2025 13:54:33 +0200 Subject: [PATCH 04/22] fix format --- tests/integration/actions_job_token_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/actions_job_token_test.go b/tests/integration/actions_job_token_test.go index 5df33a01e0840..ff43394aec5d0 100644 --- a/tests/integration/actions_job_token_test.go +++ b/tests/integration/actions_job_token_test.go @@ -8,6 +8,7 @@ import ( actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/structs" + "github.com/stretchr/testify/require" ) From 2ce0dbf8db9f1cd950a9b86a51ea1f446832736f Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 19 Oct 2025 14:04:07 +0200 Subject: [PATCH 05/22] fix tests --- tests/integration/actions_job_token_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/actions_job_token_test.go b/tests/integration/actions_job_token_test.go index ff43394aec5d0..ee604eb90bf05 100644 --- a/tests/integration/actions_job_token_test.go +++ b/tests/integration/actions_job_token_test.go @@ -42,8 +42,8 @@ func testActionsJobTokenAccess(u *url.URL, isFork bool) func(t *testing.T) { t.Run("Git Clone", doGitClone(dstPath, u)) t.Run("API Get Repository", doAPIGetRepository(context, func(t *testing.T, r structs.Repository) { - require.EqualValues(t, "repo4", r.Name) - require.EqualValues(t, "user5", r.Owner.UserName) + require.Equal(t, "repo4", r.Name) + require.Equal(t, "user5", r.Owner.UserName) })) if isFork { From 0b541baa5c3c4711494d414d4c772c2051ae396f Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 19 Oct 2025 14:24:31 +0200 Subject: [PATCH 06/22] . --- tests/integration/actions_job_token_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/integration/actions_job_token_test.go b/tests/integration/actions_job_token_test.go index ee604eb90bf05..a29a5a69510ae 100644 --- a/tests/integration/actions_job_token_test.go +++ b/tests/integration/actions_job_token_test.go @@ -1,3 +1,6 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + package integration import ( From ba3032bdb5ac9f42e85cd00e52d7e0d370ff093d Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Tue, 21 Oct 2025 17:40:41 +0200 Subject: [PATCH 07/22] Update models/perm/access/repo_permission.go Signed-off-by: ChristopherHX --- models/perm/access/repo_permission.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 92bec14523d08..6460db86e0148 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -257,7 +257,7 @@ func finalProcessRepoUnitPermission(user *user_model.User, perm *Permission) { // GetActionsUserRepoPermission returns the actions user permissions to the repository func GetActionsUserRepoPermission(ctx context.Context, repo *repo_model.Repository, actionsUser *user_model.User, taskID int64) (perm Permission, err error) { if actionsUser.ID != user_model.ActionsUserID { - setting.PanicInDevOrTesting("GetActionsUserRepoPermission can only be called by the actions user") + return perm, fmt.Errorf("api GetActionsUserRepoPermission can only be called by the actions user") } task, err := actions_model.GetTaskByID(ctx, taskID) if err != nil { From 88caa6515a3b7903c10c172bade055007ef7487a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 21 Oct 2025 23:51:55 +0800 Subject: [PATCH 08/22] fix lint --- models/perm/access/repo_permission.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 6460db86e0148..df96db8d5a48f 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -5,6 +5,7 @@ package access import ( "context" + "errors" "fmt" "slices" @@ -257,7 +258,7 @@ func finalProcessRepoUnitPermission(user *user_model.User, perm *Permission) { // GetActionsUserRepoPermission returns the actions user permissions to the repository func GetActionsUserRepoPermission(ctx context.Context, repo *repo_model.Repository, actionsUser *user_model.User, taskID int64) (perm Permission, err error) { if actionsUser.ID != user_model.ActionsUserID { - return perm, fmt.Errorf("api GetActionsUserRepoPermission can only be called by the actions user") + return perm, errors.New("api GetActionsUserRepoPermission can only be called by the actions user") } task, err := actions_model.GetTaskByID(ctx, taskID) if err != nil { From 38fd35cce5e20dd97159cc0d5b5278227c00c847 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 21 Oct 2025 19:50:35 +0200 Subject: [PATCH 09/22] apply feedback lfs auth * todo tests --- services/lfs/server.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/services/lfs/server.go b/services/lfs/server.go index faa29605674bb..2e18f7b772d3a 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -546,22 +546,21 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho accessMode = perm_model.AccessModeWrite } - var perm access_model.Permission - var err error if ctx.Data["IsActionsToken"] == true { taskID := ctx.Data["ActionsTaskID"].(int64) - perm, err = access_model.GetActionsUserRepoPermission(ctx, repository, ctx.Doer, taskID) + perm, err := access_model.GetActionsUserRepoPermission(ctx, repository, ctx.Doer, taskID) if err != nil { log.Error("Unable to GetActionsUserRepoPermission for task[%d] Error: %v", taskID, err) return false } - } else { - // ctx.IsSigned is unnecessary here, this will be checked in perm.CanAccess - perm, err = access_model.GetUserRepoPermission(ctx, repository, ctx.Doer) - if err != nil { - log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", ctx.Doer, repository, err) - return false - } + return perm.CanAccess(accessMode, unit.TypeCode) + } + + // ctx.IsSigned is unnecessary here, this will be checked in perm.CanAccess + perm, err := access_model.GetUserRepoPermission(ctx, repository, ctx.Doer) + if err != nil { + log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", ctx.Doer, repository, err) + return false } canRead := perm.CanAccess(accessMode, unit.TypeCode) From e1e14e7cdf7ca131a665b3c6fc7e7fca9caef281 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 21 Oct 2025 22:58:39 +0200 Subject: [PATCH 10/22] fix lfs locks for actions user --- models/git/lfs_lock.go | 14 +++++++ services/convert/convert.go | 8 +++- services/lfs/locks.go | 20 ++++++++- tests/integration/actions_job_token_test.go | 46 +++++++++++++++++++++ tests/integration/api_repo_file_get_test.go | 8 ++-- 5 files changed, 89 insertions(+), 7 deletions(-) diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go index c5f9a4e6dec0a..9df09e781ebb8 100644 --- a/models/git/lfs_lock.go +++ b/models/git/lfs_lock.go @@ -186,6 +186,20 @@ func CheckLFSAccessForRepo(ctx context.Context, ownerID int64, repo *repo_model. if ownerID == 0 { return ErrLFSUnauthorizedAction{repo.ID, "undefined", mode} } + if ownerID == user_model.ActionsUserID { + taskId, ok := ctx.Value(user_model.ActionsUserName).(int64) + if !ok || taskId == 0 { + return ErrLFSUnauthorizedAction{repo.ID, user_model.ActionsUserName, mode} + } + perm, err := access_model.GetActionsUserRepoPermission(ctx, repo, user_model.NewActionsUser(), taskId) + if err != nil { + return err + } + if !perm.CanAccess(mode, unit.TypeCode) { + return ErrLFSUnauthorizedAction{repo.ID, user_model.ActionsUserName, mode} + } + return nil + } u, err := user_model.GetUserByID(ctx, ownerID) if err != nil { return err diff --git a/services/convert/convert.go b/services/convert/convert.go index 0de38221409bb..208afa0202ca6 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -771,7 +771,13 @@ func ToOAuth2Application(app *auth.OAuth2Application) *api.OAuth2Application { // ToLFSLock convert a LFSLock to api.LFSLock func ToLFSLock(ctx context.Context, l *git_model.LFSLock) *api.LFSLock { - u, err := user_model.GetUserByID(ctx, l.OwnerID) + var u *user_model.User + var err error + if l.OwnerID == user_model.ActionsUserID { + u = user_model.NewActionsUser() + } else { + u, err = user_model.GetUserByID(ctx, l.OwnerID) + } if err != nil { return nil } diff --git a/services/lfs/locks.go b/services/lfs/locks.go index 264001f0f984f..417c3877d5d52 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -4,6 +4,7 @@ package lfs import ( + go_context "context" "net/http" "strconv" "strings" @@ -11,6 +12,7 @@ import ( auth_model "code.gitea.io/gitea/models/auth" git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/json" lfs_module "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" @@ -175,7 +177,14 @@ func PostLockHandler(ctx *context.Context) { return } - lock, err := git_model.CreateLFSLock(ctx, repository, &git_model.LFSLock{ + var lockCtx go_context.Context = ctx + // Pass Actions Task ID in context if creating lock using Actions Job Token + if ctx.Doer != nil && ctx.Doer.ID == user_model.ActionsUserID { + taskID := ctx.Data["ActionsTaskID"].(int64) + lockCtx = go_context.WithValue(lockCtx, user_model.ActionsUserName, taskID) + } + + lock, err := git_model.CreateLFSLock(lockCtx, repository, &git_model.LFSLock{ Path: req.Path, OwnerID: ctx.Doer.ID, }) @@ -315,7 +324,14 @@ func UnLockHandler(ctx *context.Context) { return } - lock, err := git_model.DeleteLFSLockByID(ctx, ctx.PathParamInt64("lid"), repository, ctx.Doer, req.Force) + var lockCtx go_context.Context = ctx + // Pass Actions Task ID in context if deleting lock using Actions Job Token + if ctx.Doer != nil && ctx.Doer.ID == user_model.ActionsUserID { + taskID := ctx.Data["ActionsTaskID"].(int64) + lockCtx = go_context.WithValue(lockCtx, user_model.ActionsUserName, taskID) + } + + lock, err := git_model.DeleteLFSLockByID(lockCtx, ctx.PathParamInt64("lid"), repository, ctx.Doer, req.Force) if err != nil { if git_model.IsErrLFSUnauthorizedAction(err) { ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) diff --git a/tests/integration/actions_job_token_test.go b/tests/integration/actions_job_token_test.go index a29a5a69510ae..3f7c69fb2336a 100644 --- a/tests/integration/actions_job_token_test.go +++ b/tests/integration/actions_job_token_test.go @@ -5,13 +5,18 @@ package integration import ( "encoding/base64" + "net/http" "net/url" "testing" actions_model "code.gitea.io/gitea/models/actions" + auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/structs" + api "code.gitea.io/gitea/modules/structs" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -72,3 +77,44 @@ func testActionsJobTokenAccess(u *url.URL, isFork bool) func(t *testing.T) { })) } } + +func TestActionsJobTokenAccessLFS(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + httpContext := NewAPITestContext(t, "user2", "repo-lfs-test", auth_model.AccessTokenScopeWriteUser, auth_model.AccessTokenScopeWriteRepository) + t.Run("Create Repository", doAPICreateRepository(httpContext, false, func(t *testing.T, repository api.Repository) { + + task := &actions_model.ActionTask{} + require.NoError(t, task.GenerateToken()) + task.Status = actions_model.StatusRunning + task.IsForkPullRequest = false + task.RepoID = repository.ID + err := db.Insert(t.Context(), task) + require.NoError(t, err) + session := emptyTestSession(t) + httpContext := APITestContext{ + Session: session, + Token: task.Token, + Username: "user2", + Reponame: "repo-lfs-test", + } + + u.Path = httpContext.GitPath() + dstPath := t.TempDir() + + u.Path = httpContext.GitPath() + u.User = url.UserPassword("gitea-actions", task.Token) + + t.Run("Clone", doGitClone(dstPath, u)) + + dstPath2 := t.TempDir() + + t.Run("Partial Clone", doPartialGitClone(dstPath2, u)) + + lfs := lfsCommitAndPushTest(t, dstPath, testFileSizeSmall)[0] + + reqLFS := NewRequest(t, "GET", "/api/v1/repos/user2/repo-lfs-test/media/"+lfs).AddTokenAuth(task.Token) + respLFS := MakeRequestNilResponseRecorder(t, reqLFS, http.StatusOK) + assert.Equal(t, testFileSizeSmall, respLFS.Length) + })) + }) +} diff --git a/tests/integration/api_repo_file_get_test.go b/tests/integration/api_repo_file_get_test.go index 379851b689ca5..d7bafe66be703 100644 --- a/tests/integration/api_repo_file_get_test.go +++ b/tests/integration/api_repo_file_get_test.go @@ -25,8 +25,8 @@ func TestAPIGetRawFileOrLFS(t *testing.T) { // Test with LFS onGiteaRun(t, func(t *testing.T, u *url.URL) { - httpContext := NewAPITestContext(t, "user2", "repo-lfs-test", auth_model.AccessTokenScopeWriteRepository) - doAPICreateRepository(httpContext, false, func(t *testing.T, repository api.Repository) { + httpContext := NewAPITestContext(t, "user2", "repo-lfs-test", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + t.Run("repo-lfs-test", doAPICreateRepository(httpContext, false, func(t *testing.T, repository api.Repository) { u.Path = httpContext.GitPath() dstPath := t.TempDir() @@ -41,9 +41,9 @@ func TestAPIGetRawFileOrLFS(t *testing.T) { lfs := lfsCommitAndPushTest(t, dstPath, testFileSizeSmall)[0] - reqLFS := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/media/"+lfs) + reqLFS := NewRequest(t, "GET", "/api/v1/repos/user2/repo-lfs-test/media/"+lfs).AddTokenAuth(httpContext.Token) respLFS := MakeRequestNilResponseRecorder(t, reqLFS, http.StatusOK) assert.Equal(t, testFileSizeSmall, respLFS.Length) - }) + })) }) } From cd7c5e3cd63cca6ed5847b916e2671319db5054b Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 21 Oct 2025 23:12:15 +0200 Subject: [PATCH 11/22] remove space --- tests/integration/actions_job_token_test.go | 45 ++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/tests/integration/actions_job_token_test.go b/tests/integration/actions_job_token_test.go index 3f7c69fb2336a..3bee64f292537 100644 --- a/tests/integration/actions_job_token_test.go +++ b/tests/integration/actions_job_token_test.go @@ -82,7 +82,6 @@ func TestActionsJobTokenAccessLFS(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { httpContext := NewAPITestContext(t, "user2", "repo-lfs-test", auth_model.AccessTokenScopeWriteUser, auth_model.AccessTokenScopeWriteRepository) t.Run("Create Repository", doAPICreateRepository(httpContext, false, func(t *testing.T, repository api.Repository) { - task := &actions_model.ActionTask{} require.NoError(t, task.GenerateToken()) task.Status = actions_model.StatusRunning @@ -118,3 +117,47 @@ func TestActionsJobTokenAccessLFS(t *testing.T) { })) }) } + +// func TestActionsJobTokenAccessLFS(t *testing.T) { +// onGiteaRun(t, func(t *testing.T, u *url.URL) { +// task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 47}) +// require.NoError(t, task.GenerateToken()) +// task.Status = actions_model.StatusRunning +// task.IsForkPullRequest = false +// err := actions_model.UpdateTask(t.Context(), task, "token_hash", "token_salt", "token_last_eight", "status", "is_fork_pull_request") +// require.NoError(t, err) +// session := emptyTestSession(t) +// httpContext := APITestContext{ +// Session: session, +// Token: task.Token, +// Username: "user5", +// Reponame: "repo4", +// } + +// u.Path = httpContext.GitPath() +// dstPath := t.TempDir() + +// u.Path = httpContext.GitPath() +// u.User = url.UserPassword("gitea-actions", task.Token) + +// t.Run("Clone", doGitClone(dstPath, u)) + +// t.Run("lfs-branch", doGitCheckoutBranch(dstPath, "--orphan", "lfs-branch")) + +// doGitPull(dstPath, "-f") + +// t.Run("delete master branch", doBranchDelete(httpContext, httpContext.Username, httpContext.Reponame, "master")) + +// t.Run("new-branch", doGitPushTestRepository(dstPath)) + +// dstPath2 := t.TempDir() + +// t.Run("Partial Clone", doPartialGitClone(dstPath2, u)) + +// lfs := lfsCommitAndPushTest(t, dstPath, testFileSizeSmall)[0] + +// reqLFS := NewRequest(t, "GET", "/api/v1/repos/user5/repo4/media/"+lfs) +// respLFS := MakeRequestNilResponseRecorder(t, reqLFS, http.StatusOK) +// assert.Equal(t, testFileSizeSmall, respLFS.Length) +// }) +// } From 51cf3daa86fd5ca9588dd7cab0cde1b4f6dd484a Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 21 Oct 2025 23:13:39 +0200 Subject: [PATCH 12/22] remove old tests --- tests/integration/actions_job_token_test.go | 44 --------------------- 1 file changed, 44 deletions(-) diff --git a/tests/integration/actions_job_token_test.go b/tests/integration/actions_job_token_test.go index 3bee64f292537..36ac5f4074a4b 100644 --- a/tests/integration/actions_job_token_test.go +++ b/tests/integration/actions_job_token_test.go @@ -117,47 +117,3 @@ func TestActionsJobTokenAccessLFS(t *testing.T) { })) }) } - -// func TestActionsJobTokenAccessLFS(t *testing.T) { -// onGiteaRun(t, func(t *testing.T, u *url.URL) { -// task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 47}) -// require.NoError(t, task.GenerateToken()) -// task.Status = actions_model.StatusRunning -// task.IsForkPullRequest = false -// err := actions_model.UpdateTask(t.Context(), task, "token_hash", "token_salt", "token_last_eight", "status", "is_fork_pull_request") -// require.NoError(t, err) -// session := emptyTestSession(t) -// httpContext := APITestContext{ -// Session: session, -// Token: task.Token, -// Username: "user5", -// Reponame: "repo4", -// } - -// u.Path = httpContext.GitPath() -// dstPath := t.TempDir() - -// u.Path = httpContext.GitPath() -// u.User = url.UserPassword("gitea-actions", task.Token) - -// t.Run("Clone", doGitClone(dstPath, u)) - -// t.Run("lfs-branch", doGitCheckoutBranch(dstPath, "--orphan", "lfs-branch")) - -// doGitPull(dstPath, "-f") - -// t.Run("delete master branch", doBranchDelete(httpContext, httpContext.Username, httpContext.Reponame, "master")) - -// t.Run("new-branch", doGitPushTestRepository(dstPath)) - -// dstPath2 := t.TempDir() - -// t.Run("Partial Clone", doPartialGitClone(dstPath2, u)) - -// lfs := lfsCommitAndPushTest(t, dstPath, testFileSizeSmall)[0] - -// reqLFS := NewRequest(t, "GET", "/api/v1/repos/user5/repo4/media/"+lfs) -// respLFS := MakeRequestNilResponseRecorder(t, reqLFS, http.StatusOK) -// assert.Equal(t, testFileSizeSmall, respLFS.Length) -// }) -// } From 7cb49399ba908921bac1433af771476b1d9a0cc0 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 21 Oct 2025 23:25:02 +0200 Subject: [PATCH 13/22] add access_model.ActionsTaskIDKey --- models/git/lfs_lock.go | 2 +- models/perm/access/repo_permission.go | 5 +++++ services/lfs/locks.go | 10 +++++----- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go index 9df09e781ebb8..12da780d018fd 100644 --- a/models/git/lfs_lock.go +++ b/models/git/lfs_lock.go @@ -187,7 +187,7 @@ func CheckLFSAccessForRepo(ctx context.Context, ownerID int64, repo *repo_model. return ErrLFSUnauthorizedAction{repo.ID, "undefined", mode} } if ownerID == user_model.ActionsUserID { - taskId, ok := ctx.Value(user_model.ActionsUserName).(int64) + taskId, ok := ctx.Value(access_model.ActionsTaskIDKey).(int64) if !ok || taskId == 0 { return ErrLFSUnauthorizedAction{repo.ID, user_model.ActionsUserName, mode} } diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index df96db8d5a48f..73813efe83e6b 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -255,6 +255,11 @@ func finalProcessRepoUnitPermission(user *user_model.User, perm *Permission) { } } +type ActionsTaskIDKeyType struct{} + +// ActionsTaskIDKey is the context key for actions task ID in modules without context service like lfs locks +var ActionsTaskIDKey ActionsTaskIDKeyType + // GetActionsUserRepoPermission returns the actions user permissions to the repository func GetActionsUserRepoPermission(ctx context.Context, repo *repo_model.Repository, actionsUser *user_model.User, taskID int64) (perm Permission, err error) { if actionsUser.ID != user_model.ActionsUserID { diff --git a/services/lfs/locks.go b/services/lfs/locks.go index 417c3877d5d52..9329e0ec778f5 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -11,8 +11,8 @@ import ( auth_model "code.gitea.io/gitea/models/auth" git_model "code.gitea.io/gitea/models/git" + access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" - user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/json" lfs_module "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" @@ -179,9 +179,9 @@ func PostLockHandler(ctx *context.Context) { var lockCtx go_context.Context = ctx // Pass Actions Task ID in context if creating lock using Actions Job Token - if ctx.Doer != nil && ctx.Doer.ID == user_model.ActionsUserID { + if ctx.Data["IsActionsToken"] == true { taskID := ctx.Data["ActionsTaskID"].(int64) - lockCtx = go_context.WithValue(lockCtx, user_model.ActionsUserName, taskID) + lockCtx = go_context.WithValue(lockCtx, access_model.ActionsTaskIDKey, taskID) } lock, err := git_model.CreateLFSLock(lockCtx, repository, &git_model.LFSLock{ @@ -326,9 +326,9 @@ func UnLockHandler(ctx *context.Context) { var lockCtx go_context.Context = ctx // Pass Actions Task ID in context if deleting lock using Actions Job Token - if ctx.Doer != nil && ctx.Doer.ID == user_model.ActionsUserID { + if ctx.Data["IsActionsToken"] == true { taskID := ctx.Data["ActionsTaskID"].(int64) - lockCtx = go_context.WithValue(lockCtx, user_model.ActionsUserName, taskID) + lockCtx = go_context.WithValue(lockCtx, access_model.ActionsTaskIDKey, taskID) } lock, err := git_model.DeleteLFSLockByID(lockCtx, ctx.PathParamInt64("lid"), repository, ctx.Doer, req.Force) From df0f3f3634424147456138f9f97e93629994b0bd Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 21 Oct 2025 23:37:45 +0200 Subject: [PATCH 14/22] fix name --- models/git/lfs_lock.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go index 12da780d018fd..ad265c4095991 100644 --- a/models/git/lfs_lock.go +++ b/models/git/lfs_lock.go @@ -187,11 +187,11 @@ func CheckLFSAccessForRepo(ctx context.Context, ownerID int64, repo *repo_model. return ErrLFSUnauthorizedAction{repo.ID, "undefined", mode} } if ownerID == user_model.ActionsUserID { - taskId, ok := ctx.Value(access_model.ActionsTaskIDKey).(int64) - if !ok || taskId == 0 { + taskID, ok := ctx.Value(access_model.ActionsTaskIDKey).(int64) + if !ok || taskID == 0 { return ErrLFSUnauthorizedAction{repo.ID, user_model.ActionsUserName, mode} } - perm, err := access_model.GetActionsUserRepoPermission(ctx, repo, user_model.NewActionsUser(), taskId) + perm, err := access_model.GetActionsUserRepoPermission(ctx, repo, user_model.NewActionsUser(), taskID) if err != nil { return err } From 04b2c098b24fd0d3e916cb03e8621de6677732f3 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 21 Oct 2025 23:38:32 +0200 Subject: [PATCH 15/22] fix double import --- tests/integration/actions_job_token_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integration/actions_job_token_test.go b/tests/integration/actions_job_token_test.go index 36ac5f4074a4b..e587f6cfb2176 100644 --- a/tests/integration/actions_job_token_test.go +++ b/tests/integration/actions_job_token_test.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/structs" - api "code.gitea.io/gitea/modules/structs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -81,7 +80,7 @@ func testActionsJobTokenAccess(u *url.URL, isFork bool) func(t *testing.T) { func TestActionsJobTokenAccessLFS(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { httpContext := NewAPITestContext(t, "user2", "repo-lfs-test", auth_model.AccessTokenScopeWriteUser, auth_model.AccessTokenScopeWriteRepository) - t.Run("Create Repository", doAPICreateRepository(httpContext, false, func(t *testing.T, repository api.Repository) { + t.Run("Create Repository", doAPICreateRepository(httpContext, false, func(t *testing.T, repository structs.Repository) { task := &actions_model.ActionTask{} require.NoError(t, task.GenerateToken()) task.Status = actions_model.StatusRunning From 0c13471bc649fc90013007e2902fdd32cb37bd9e Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 22 Oct 2025 00:18:14 +0200 Subject: [PATCH 16/22] pass taskID as parameter --- models/git/lfs_lock.go | 13 ++++++------- models/perm/access/repo_permission.go | 5 ----- routers/web/repo/setting/lfs.go | 4 ++-- services/lfs/locks.go | 18 +++++++----------- 4 files changed, 15 insertions(+), 25 deletions(-) diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go index ad265c4095991..238b1e87b27f8 100644 --- a/models/git/lfs_lock.go +++ b/models/git/lfs_lock.go @@ -69,9 +69,9 @@ func (l *LFSLock) LoadOwner(ctx context.Context) error { } // CreateLFSLock creates a new lock. -func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLock) (*LFSLock, error) { +func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLock, taskID int64) (*LFSLock, error) { return db.WithTx2(ctx, func(ctx context.Context) (*LFSLock, error) { - if err := CheckLFSAccessForRepo(ctx, lock.OwnerID, repo, perm.AccessModeWrite); err != nil { + if err := CheckLFSAccessForRepo(ctx, lock.OwnerID, repo, perm.AccessModeWrite, taskID); err != nil { return nil, err } @@ -158,14 +158,14 @@ func CountLFSLockByRepoID(ctx context.Context, repoID int64) (int64, error) { } // DeleteLFSLockByID deletes a lock by given ID. -func DeleteLFSLockByID(ctx context.Context, id int64, repo *repo_model.Repository, u *user_model.User, force bool) (*LFSLock, error) { +func DeleteLFSLockByID(ctx context.Context, id int64, repo *repo_model.Repository, u *user_model.User, force bool, taskID int64) (*LFSLock, error) { return db.WithTx2(ctx, func(ctx context.Context) (*LFSLock, error) { lock, err := GetLFSLockByID(ctx, id) if err != nil { return nil, err } - if err := CheckLFSAccessForRepo(ctx, u.ID, repo, perm.AccessModeWrite); err != nil { + if err := CheckLFSAccessForRepo(ctx, u.ID, repo, perm.AccessModeWrite, taskID); err != nil { return nil, err } @@ -182,13 +182,12 @@ func DeleteLFSLockByID(ctx context.Context, id int64, repo *repo_model.Repositor } // CheckLFSAccessForRepo check needed access mode base on action -func CheckLFSAccessForRepo(ctx context.Context, ownerID int64, repo *repo_model.Repository, mode perm.AccessMode) error { +func CheckLFSAccessForRepo(ctx context.Context, ownerID int64, repo *repo_model.Repository, mode perm.AccessMode, taskID int64) error { if ownerID == 0 { return ErrLFSUnauthorizedAction{repo.ID, "undefined", mode} } if ownerID == user_model.ActionsUserID { - taskID, ok := ctx.Value(access_model.ActionsTaskIDKey).(int64) - if !ok || taskID == 0 { + if taskID == 0 { return ErrLFSUnauthorizedAction{repo.ID, user_model.ActionsUserName, mode} } perm, err := access_model.GetActionsUserRepoPermission(ctx, repo, user_model.NewActionsUser(), taskID) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 73813efe83e6b..df96db8d5a48f 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -255,11 +255,6 @@ func finalProcessRepoUnitPermission(user *user_model.User, perm *Permission) { } } -type ActionsTaskIDKeyType struct{} - -// ActionsTaskIDKey is the context key for actions task ID in modules without context service like lfs locks -var ActionsTaskIDKey ActionsTaskIDKeyType - // GetActionsUserRepoPermission returns the actions user permissions to the repository func GetActionsUserRepoPermission(ctx context.Context, repo *repo_model.Repository, actionsUser *user_model.User, taskID int64) (perm Permission, err error) { if actionsUser.ID != user_model.ActionsUserID { diff --git a/routers/web/repo/setting/lfs.go b/routers/web/repo/setting/lfs.go index a558231df1224..14c9c92c36a58 100644 --- a/routers/web/repo/setting/lfs.go +++ b/routers/web/repo/setting/lfs.go @@ -198,7 +198,7 @@ func LFSLockFile(ctx *context.Context) { _, err := git_model.CreateLFSLock(ctx, ctx.Repo.Repository, &git_model.LFSLock{ Path: lockPath, OwnerID: ctx.Doer.ID, - }) + }, 0) if err != nil { if git_model.IsErrLFSLockAlreadyExist(err) { ctx.Flash.Error(ctx.Tr("repo.settings.lfs_lock_already_exists", originalPath)) @@ -217,7 +217,7 @@ func LFSUnlock(ctx *context.Context) { ctx.NotFound(nil) return } - _, err := git_model.DeleteLFSLockByID(ctx, ctx.PathParamInt64("lid"), ctx.Repo.Repository, ctx.Doer, true) + _, err := git_model.DeleteLFSLockByID(ctx, ctx.PathParamInt64("lid"), ctx.Repo.Repository, ctx.Doer, true, 0) if err != nil { ctx.ServerError("LFSUnlock", err) return diff --git a/services/lfs/locks.go b/services/lfs/locks.go index 9329e0ec778f5..ad9f576f89514 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -4,14 +4,12 @@ package lfs import ( - go_context "context" "net/http" "strconv" "strings" auth_model "code.gitea.io/gitea/models/auth" git_model "code.gitea.io/gitea/models/git" - access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/modules/json" lfs_module "code.gitea.io/gitea/modules/lfs" @@ -177,17 +175,16 @@ func PostLockHandler(ctx *context.Context) { return } - var lockCtx go_context.Context = ctx + var taskID int64 // Pass Actions Task ID in context if creating lock using Actions Job Token if ctx.Data["IsActionsToken"] == true { - taskID := ctx.Data["ActionsTaskID"].(int64) - lockCtx = go_context.WithValue(lockCtx, access_model.ActionsTaskIDKey, taskID) + taskID = ctx.Data["ActionsTaskID"].(int64) } - lock, err := git_model.CreateLFSLock(lockCtx, repository, &git_model.LFSLock{ + lock, err := git_model.CreateLFSLock(ctx, repository, &git_model.LFSLock{ Path: req.Path, OwnerID: ctx.Doer.ID, - }) + }, taskID) if err != nil { if git_model.IsErrLFSLockAlreadyExist(err) { ctx.JSON(http.StatusConflict, api.LFSLockError{ @@ -324,14 +321,13 @@ func UnLockHandler(ctx *context.Context) { return } - var lockCtx go_context.Context = ctx + var taskID int64 // Pass Actions Task ID in context if deleting lock using Actions Job Token if ctx.Data["IsActionsToken"] == true { - taskID := ctx.Data["ActionsTaskID"].(int64) - lockCtx = go_context.WithValue(lockCtx, access_model.ActionsTaskIDKey, taskID) + taskID = ctx.Data["ActionsTaskID"].(int64) } - lock, err := git_model.DeleteLFSLockByID(lockCtx, ctx.PathParamInt64("lid"), repository, ctx.Doer, req.Force) + lock, err := git_model.DeleteLFSLockByID(ctx, ctx.PathParamInt64("lid"), repository, ctx.Doer, req.Force, taskID) if err != nil { if git_model.IsErrLFSUnauthorizedAction(err) { ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) From 5551b2f32e2dd737457c0050811e2b8920b15268 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 22 Oct 2025 00:25:01 +0200 Subject: [PATCH 17/22] fix comment --- services/lfs/locks.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/lfs/locks.go b/services/lfs/locks.go index ad9f576f89514..1d7f43bce3e72 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -176,7 +176,7 @@ func PostLockHandler(ctx *context.Context) { } var taskID int64 - // Pass Actions Task ID in context if creating lock using Actions Job Token + // Passing a non zero Actions Task ID as parameter if creating lock using Actions Job Token if ctx.Data["IsActionsToken"] == true { taskID = ctx.Data["ActionsTaskID"].(int64) } @@ -322,7 +322,7 @@ func UnLockHandler(ctx *context.Context) { } var taskID int64 - // Pass Actions Task ID in context if deleting lock using Actions Job Token + // Passing a non zero Actions Task ID as parameter if deleting lock using Actions Job Token if ctx.Data["IsActionsToken"] == true { taskID = ctx.Data["ActionsTaskID"].(int64) } From beb05819efeff9acd0ee645610733723c9779aa7 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 22 Oct 2025 01:18:09 +0200 Subject: [PATCH 18/22] apply feedback --- services/convert/convert.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/services/convert/convert.go b/services/convert/convert.go index 208afa0202ca6..9f8fff970ccc0 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -771,13 +771,7 @@ func ToOAuth2Application(app *auth.OAuth2Application) *api.OAuth2Application { // ToLFSLock convert a LFSLock to api.LFSLock func ToLFSLock(ctx context.Context, l *git_model.LFSLock) *api.LFSLock { - var u *user_model.User - var err error - if l.OwnerID == user_model.ActionsUserID { - u = user_model.NewActionsUser() - } else { - u, err = user_model.GetUserByID(ctx, l.OwnerID) - } + u, err := user_model.GetPossibleUserByID(ctx, l.OwnerID) if err != nil { return nil } From 01e683169280dc78fa33ba649e78f066f43a2ade Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 22 Oct 2025 01:25:09 +0200 Subject: [PATCH 19/22] strip duplicated auth in lfs model --- models/git/lfs_lock.go | 47 ++------------------------------- routers/web/repo/setting/lfs.go | 4 +-- services/lfs/locks.go | 16 ++--------- 3 files changed, 6 insertions(+), 61 deletions(-) diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go index 238b1e87b27f8..184e616915d07 100644 --- a/models/git/lfs_lock.go +++ b/models/git/lfs_lock.go @@ -11,10 +11,7 @@ import ( "time" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/models/perm" - access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -69,12 +66,8 @@ func (l *LFSLock) LoadOwner(ctx context.Context) error { } // CreateLFSLock creates a new lock. -func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLock, taskID int64) (*LFSLock, error) { +func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLock) (*LFSLock, error) { return db.WithTx2(ctx, func(ctx context.Context) (*LFSLock, error) { - if err := CheckLFSAccessForRepo(ctx, lock.OwnerID, repo, perm.AccessModeWrite, taskID); err != nil { - return nil, err - } - lock.Path = util.PathJoinRel(lock.Path) lock.RepoID = repo.ID @@ -158,17 +151,13 @@ func CountLFSLockByRepoID(ctx context.Context, repoID int64) (int64, error) { } // DeleteLFSLockByID deletes a lock by given ID. -func DeleteLFSLockByID(ctx context.Context, id int64, repo *repo_model.Repository, u *user_model.User, force bool, taskID int64) (*LFSLock, error) { +func DeleteLFSLockByID(ctx context.Context, id int64, repo *repo_model.Repository, u *user_model.User, force bool) (*LFSLock, error) { return db.WithTx2(ctx, func(ctx context.Context) (*LFSLock, error) { lock, err := GetLFSLockByID(ctx, id) if err != nil { return nil, err } - if err := CheckLFSAccessForRepo(ctx, u.ID, repo, perm.AccessModeWrite, taskID); err != nil { - return nil, err - } - if !force && u.ID != lock.OwnerID { return nil, errors.New("user doesn't own lock and force flag is not set") } @@ -180,35 +169,3 @@ func DeleteLFSLockByID(ctx context.Context, id int64, repo *repo_model.Repositor return lock, nil }) } - -// CheckLFSAccessForRepo check needed access mode base on action -func CheckLFSAccessForRepo(ctx context.Context, ownerID int64, repo *repo_model.Repository, mode perm.AccessMode, taskID int64) error { - if ownerID == 0 { - return ErrLFSUnauthorizedAction{repo.ID, "undefined", mode} - } - if ownerID == user_model.ActionsUserID { - if taskID == 0 { - return ErrLFSUnauthorizedAction{repo.ID, user_model.ActionsUserName, mode} - } - perm, err := access_model.GetActionsUserRepoPermission(ctx, repo, user_model.NewActionsUser(), taskID) - if err != nil { - return err - } - if !perm.CanAccess(mode, unit.TypeCode) { - return ErrLFSUnauthorizedAction{repo.ID, user_model.ActionsUserName, mode} - } - return nil - } - u, err := user_model.GetUserByID(ctx, ownerID) - if err != nil { - return err - } - perm, err := access_model.GetUserRepoPermission(ctx, repo, u) - if err != nil { - return err - } - if !perm.CanAccess(mode, unit.TypeCode) { - return ErrLFSUnauthorizedAction{repo.ID, u.DisplayName(), mode} - } - return nil -} diff --git a/routers/web/repo/setting/lfs.go b/routers/web/repo/setting/lfs.go index 14c9c92c36a58..a558231df1224 100644 --- a/routers/web/repo/setting/lfs.go +++ b/routers/web/repo/setting/lfs.go @@ -198,7 +198,7 @@ func LFSLockFile(ctx *context.Context) { _, err := git_model.CreateLFSLock(ctx, ctx.Repo.Repository, &git_model.LFSLock{ Path: lockPath, OwnerID: ctx.Doer.ID, - }, 0) + }) if err != nil { if git_model.IsErrLFSLockAlreadyExist(err) { ctx.Flash.Error(ctx.Tr("repo.settings.lfs_lock_already_exists", originalPath)) @@ -217,7 +217,7 @@ func LFSUnlock(ctx *context.Context) { ctx.NotFound(nil) return } - _, err := git_model.DeleteLFSLockByID(ctx, ctx.PathParamInt64("lid"), ctx.Repo.Repository, ctx.Doer, true, 0) + _, err := git_model.DeleteLFSLockByID(ctx, ctx.PathParamInt64("lid"), ctx.Repo.Repository, ctx.Doer, true) if err != nil { ctx.ServerError("LFSUnlock", err) return diff --git a/services/lfs/locks.go b/services/lfs/locks.go index 1d7f43bce3e72..264001f0f984f 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -175,16 +175,10 @@ func PostLockHandler(ctx *context.Context) { return } - var taskID int64 - // Passing a non zero Actions Task ID as parameter if creating lock using Actions Job Token - if ctx.Data["IsActionsToken"] == true { - taskID = ctx.Data["ActionsTaskID"].(int64) - } - lock, err := git_model.CreateLFSLock(ctx, repository, &git_model.LFSLock{ Path: req.Path, OwnerID: ctx.Doer.ID, - }, taskID) + }) if err != nil { if git_model.IsErrLFSLockAlreadyExist(err) { ctx.JSON(http.StatusConflict, api.LFSLockError{ @@ -321,13 +315,7 @@ func UnLockHandler(ctx *context.Context) { return } - var taskID int64 - // Passing a non zero Actions Task ID as parameter if deleting lock using Actions Job Token - if ctx.Data["IsActionsToken"] == true { - taskID = ctx.Data["ActionsTaskID"].(int64) - } - - lock, err := git_model.DeleteLFSLockByID(ctx, ctx.PathParamInt64("lid"), repository, ctx.Doer, req.Force, taskID) + lock, err := git_model.DeleteLFSLockByID(ctx, ctx.PathParamInt64("lid"), repository, ctx.Doer, req.Force) if err != nil { if git_model.IsErrLFSUnauthorizedAction(err) { ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) From 236e6609507aa174b685f5613c00ac21e63db68c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 22 Oct 2025 08:39:34 +0800 Subject: [PATCH 20/22] remove ErrLFSUnauthorizedAction, add comment, fix test --- models/git/lfs.go | 31 --------------------- services/lfs/locks.go | 14 ---------- services/lfs/server.go | 7 +++-- tests/integration/api_repo_file_get_test.go | 8 +++--- tests/integration/api_repo_lfs_test.go | 17 +++++------ 5 files changed, 18 insertions(+), 59 deletions(-) diff --git a/models/git/lfs.go b/models/git/lfs.go index 8bba060ff975f..a4ae3e7beebe8 100644 --- a/models/git/lfs.go +++ b/models/git/lfs.go @@ -8,7 +8,6 @@ import ( "fmt" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" @@ -42,30 +41,6 @@ func (err ErrLFSLockNotExist) Unwrap() error { return util.ErrNotExist } -// ErrLFSUnauthorizedAction represents a "LFSUnauthorizedAction" kind of error. -type ErrLFSUnauthorizedAction struct { - RepoID int64 - UserName string - Mode perm.AccessMode -} - -// IsErrLFSUnauthorizedAction checks if an error is a ErrLFSUnauthorizedAction. -func IsErrLFSUnauthorizedAction(err error) bool { - _, ok := err.(ErrLFSUnauthorizedAction) - return ok -} - -func (err ErrLFSUnauthorizedAction) Error() string { - if err.Mode == perm.AccessModeWrite { - return fmt.Sprintf("User %s doesn't have write access for lfs lock [rid: %d]", err.UserName, err.RepoID) - } - return fmt.Sprintf("User %s doesn't have read access for lfs lock [rid: %d]", err.UserName, err.RepoID) -} - -func (err ErrLFSUnauthorizedAction) Unwrap() error { - return util.ErrPermissionDenied -} - // ErrLFSLockAlreadyExist represents a "LFSLockAlreadyExist" kind of error. type ErrLFSLockAlreadyExist struct { RepoID int64 @@ -93,12 +68,6 @@ type ErrLFSFileLocked struct { UserName string } -// IsErrLFSFileLocked checks if an error is a ErrLFSFileLocked. -func IsErrLFSFileLocked(err error) bool { - _, ok := err.(ErrLFSFileLocked) - return ok -} - func (err ErrLFSFileLocked) Error() string { return fmt.Sprintf("File is lfs locked [repo: %d, locked by: %s, path: %s]", err.RepoID, err.UserName, err.Path) } diff --git a/services/lfs/locks.go b/services/lfs/locks.go index 264001f0f984f..5bc3f6b95a4e5 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -187,13 +187,6 @@ func PostLockHandler(ctx *context.Context) { }) return } - if git_model.IsErrLFSUnauthorizedAction(err) { - ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) - ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ - Message: "You must have push access to create locks : " + err.Error(), - }) - return - } log.Error("Unable to CreateLFSLock in repository %-v at %s for user %-v: Error: %v", repository, req.Path, ctx.Doer, err) ctx.JSON(http.StatusInternalServerError, api.LFSLockError{ Message: "internal server error : Internal Server Error", @@ -317,13 +310,6 @@ func UnLockHandler(ctx *context.Context) { lock, err := git_model.DeleteLFSLockByID(ctx, ctx.PathParamInt64("lid"), repository, ctx.Doer, req.Force) if err != nil { - if git_model.IsErrLFSUnauthorizedAction(err) { - ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="gitea-lfs"`) - ctx.JSON(http.StatusUnauthorized, api.LFSLockError{ - Message: "You must have push access to delete locks : " + err.Error(), - }) - return - } log.Error("Unable to DeleteLFSLockByID[%d] by user %-v with force %t: Error: %v", ctx.PathParamInt64("lid"), ctx.Doer, req.Force, err) ctx.JSON(http.StatusInternalServerError, api.LFSLockError{ Message: "unable to delete lock : Internal Server Error", diff --git a/services/lfs/server.go b/services/lfs/server.go index 2e18f7b772d3a..faf31fa4992df 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -563,11 +563,14 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho return false } - canRead := perm.CanAccess(accessMode, unit.TypeCode) - if canRead && (!requireSigned || ctx.IsSigned) { + canAccess := perm.CanAccess(accessMode, unit.TypeCode) + // if it doesn't require sign-in and anonymous user has access, return true + // if the user is already signed in (for example: by session auth method), and the doer can access, return true + if canAccess && (!requireSigned || ctx.IsSigned) { return true } + // now, either sign-in is required or the ctx.Doer cannot access, check the LFS token user, err := parseToken(ctx, authorization, repository, accessMode) if err != nil { // Most of these are Warn level - the true internal server errors are logged in parseToken already diff --git a/tests/integration/api_repo_file_get_test.go b/tests/integration/api_repo_file_get_test.go index d7bafe66be703..ec50cf52f497a 100644 --- a/tests/integration/api_repo_file_get_test.go +++ b/tests/integration/api_repo_file_get_test.go @@ -9,7 +9,6 @@ import ( "testing" auth_model "code.gitea.io/gitea/models/auth" - api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -25,8 +24,9 @@ func TestAPIGetRawFileOrLFS(t *testing.T) { // Test with LFS onGiteaRun(t, func(t *testing.T, u *url.URL) { - httpContext := NewAPITestContext(t, "user2", "repo-lfs-test", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) - t.Run("repo-lfs-test", doAPICreateRepository(httpContext, false, func(t *testing.T, repository api.Repository) { + createLFSTestRepository(t, "repo-lfs-test") + httpContext := NewAPITestContext(t, "user2", "repo-lfs-test", auth_model.AccessTokenScopeWriteRepository) + t.Run("repo-lfs-test", func(t *testing.T) { u.Path = httpContext.GitPath() dstPath := t.TempDir() @@ -44,6 +44,6 @@ func TestAPIGetRawFileOrLFS(t *testing.T) { reqLFS := NewRequest(t, "GET", "/api/v1/repos/user2/repo-lfs-test/media/"+lfs).AddTokenAuth(httpContext.Token) respLFS := MakeRequestNilResponseRecorder(t, reqLFS, http.StatusOK) assert.Equal(t, testFileSizeSmall, respLFS.Length) - })) + }) }) } diff --git a/tests/integration/api_repo_lfs_test.go b/tests/integration/api_repo_lfs_test.go index da4c89a1b7303..fb55d311ccf63 100644 --- a/tests/integration/api_repo_lfs_test.go +++ b/tests/integration/api_repo_lfs_test.go @@ -23,6 +23,7 @@ import ( "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestAPILFSNotStarted(t *testing.T) { @@ -59,12 +60,12 @@ func TestAPILFSMediaType(t *testing.T) { MakeRequest(t, req, http.StatusUnsupportedMediaType) } -func createLFSTestRepository(t *testing.T, name string) *repo_model.Repository { - ctx := NewAPITestContext(t, "user2", "lfs-"+name+"-repo", auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) +func createLFSTestRepository(t *testing.T, repoName string) *repo_model.Repository { + ctx := NewAPITestContext(t, "user2", repoName, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) t.Run("CreateRepo", doAPICreateRepository(ctx, false)) - repo, err := repo_model.GetRepositoryByOwnerAndName(t.Context(), "user2", "lfs-"+name+"-repo") - assert.NoError(t, err) + repo, err := repo_model.GetRepositoryByOwnerAndName(t.Context(), "user2", repoName) + require.NoError(t, err) return repo } @@ -74,7 +75,7 @@ func TestAPILFSBatch(t *testing.T) { setting.LFS.StartServer = true - repo := createLFSTestRepository(t, "batch") + repo := createLFSTestRepository(t, "lfs-batch-repo") content := []byte("dummy1") oid := storeObjectInRepo(t, repo.ID, &content) @@ -253,7 +254,7 @@ func TestAPILFSBatch(t *testing.T) { assert.NoError(t, err) assert.True(t, exist) - repo2 := createLFSTestRepository(t, "batch2") + repo2 := createLFSTestRepository(t, "lfs-batch2-repo") content := []byte("dummy0") storeObjectInRepo(t, repo2.ID, &content) @@ -329,7 +330,7 @@ func TestAPILFSUpload(t *testing.T) { setting.LFS.StartServer = true - repo := createLFSTestRepository(t, "upload") + repo := createLFSTestRepository(t, "lfs-upload-repo") content := []byte("dummy3") oid := storeObjectInRepo(t, repo.ID, &content) @@ -433,7 +434,7 @@ func TestAPILFSVerify(t *testing.T) { setting.LFS.StartServer = true - repo := createLFSTestRepository(t, "verify") + repo := createLFSTestRepository(t, "lfs-verify-repo") content := []byte("dummy3") oid := storeObjectInRepo(t, repo.ID, &content) From 08a0caaacd740b5bd1e4061614a0639ae6678a58 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 22 Oct 2025 09:11:17 +0800 Subject: [PATCH 21/22] fix test --- models/user/user.go | 7 ++++++- routers/api/v1/repo/repo.go | 3 +++ services/lfs/server.go | 4 +++- tests/integration/actions_job_token_test.go | 9 ++++----- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 80d5eb5ec4e40..a2ec835d56904 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -249,8 +249,13 @@ func (u *User) MaxCreationLimit() int { } // CanCreateRepoIn checks whether the doer(u) can create a repository in the owner -// NOTE: functions calling this assume a failure due to repository count limit; it ONLY checks the repo number LIMIT, if new checks are added, those functions should be revised +// NOTE: functions calling this assume a failure due to repository count limit, or the owner is not a real user. +// It ONLY checks the repo number LIMIT or whether owner user is real. If new checks are added, those functions should be revised. +// TODO: the callers can only return ErrReachLimitOfRepo, need to fine tune to support other error types in the future. func (u *User) CanCreateRepoIn(owner *User) bool { + if u.ID <= 0 { + return false // fake user like Ghost or Actions user + } if u.IsAdmin { return true } diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 2f5a969b6b720..bb6bda587db6c 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -28,6 +28,7 @@ import ( repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/validation" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" @@ -270,6 +271,8 @@ func CreateUserRepo(ctx *context.APIContext, owner *user_model.User, opt api.Cre db.IsErrNamePatternNotAllowed(err) || label.IsErrTemplateLoad(err) { ctx.APIError(http.StatusUnprocessableEntity, err) + } else if errors.Is(err, util.ErrPermissionDenied) { + ctx.APIError(http.StatusForbidden, err) } else { ctx.APIErrorInternal(err) } diff --git a/services/lfs/server.go b/services/lfs/server.go index faf31fa4992df..81991de434486 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -556,7 +556,7 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho return perm.CanAccess(accessMode, unit.TypeCode) } - // ctx.IsSigned is unnecessary here, this will be checked in perm.CanAccess + // it works for both anonymous request and signed-in user, then perm.CanAccess will do the permission check perm, err := access_model.GetUserRepoPermission(ctx, repository, ctx.Doer) if err != nil { log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", ctx.Doer, repository, err) @@ -571,6 +571,8 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho } // now, either sign-in is required or the ctx.Doer cannot access, check the LFS token + // however, "ctx.Doer exists but cannot access then check LFS token" should not really happen: + // * why a request can be sent with both valid user session and valid LFS token then use LFS token to access? user, err := parseToken(ctx, authorization, repository, accessMode) if err != nil { // Most of these are Warn level - the true internal server errors are logged in parseToken already diff --git a/tests/integration/actions_job_token_test.go b/tests/integration/actions_job_token_test.go index e587f6cfb2176..c4e8e880eb824 100644 --- a/tests/integration/actions_job_token_test.go +++ b/tests/integration/actions_job_token_test.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -53,9 +54,7 @@ func testActionsJobTokenAccess(u *url.URL, isFork bool) func(t *testing.T) { require.Equal(t, "user5", r.Owner.UserName) })) - if isFork { - context.ExpectedCode = 403 - } + context.ExpectedCode = util.Iif(isFork, http.StatusForbidden, http.StatusCreated) t.Run("API Create File", doAPICreateFile(context, "test.txt", &structs.CreateFileOptions{ FileOptions: structs.FileOptions{ NewBranchName: "new-branch", @@ -64,10 +63,10 @@ func testActionsJobTokenAccess(u *url.URL, isFork bool) func(t *testing.T) { ContentBase64: base64.StdEncoding.EncodeToString([]byte(`This is a test file created using job token.`)), })) - context.ExpectedCode = 500 + context.ExpectedCode = http.StatusForbidden t.Run("Fail to Create Repository", doAPICreateRepository(context, true)) - context.ExpectedCode = 403 + context.ExpectedCode = http.StatusForbidden t.Run("Fail to Delete Repository", doAPIDeleteRepository(context)) t.Run("Fail to Create Organization", doAPICreateOrganization(context, &structs.CreateOrgOption{ From 2b51f0ad8062e8ae92c39af0914c89cd794f9cd0 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 22 Oct 2025 09:37:28 +0800 Subject: [PATCH 22/22] fix test --- models/user/user.go | 2 +- models/user/user_test.go | 41 +++++++++++++++++++++------------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index a2ec835d56904..3583694cf9f6e 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -253,7 +253,7 @@ func (u *User) MaxCreationLimit() int { // It ONLY checks the repo number LIMIT or whether owner user is real. If new checks are added, those functions should be revised. // TODO: the callers can only return ErrReachLimitOfRepo, need to fine tune to support other error types in the future. func (u *User) CanCreateRepoIn(owner *User) bool { - if u.ID <= 0 { + if u.ID <= 0 || owner.ID <= 0 { return false // fake user like Ghost or Actions user } if u.IsAdmin { diff --git a/models/user/user_test.go b/models/user/user_test.go index 4201ec4816c86..6a530553d7ea0 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -648,33 +648,36 @@ func TestGetInactiveUsers(t *testing.T) { func TestCanCreateRepo(t *testing.T) { defer test.MockVariableValue(&setting.Repository.MaxCreationLimit)() const noLimit = -1 - doerNormal := &user_model.User{} - doerAdmin := &user_model.User{IsAdmin: true} + doerActions := user_model.NewActionsUser() + doerNormal := &user_model.User{ID: 2} + doerAdmin := &user_model.User{ID: 1, IsAdmin: true} t.Run("NoGlobalLimit", func(t *testing.T) { setting.Repository.MaxCreationLimit = noLimit - assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: noLimit})) - assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 0})) - assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 100})) + assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 0})) + assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 100})) + assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: noLimit})) - assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: noLimit})) - assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 0})) - assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 100})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 0})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 100})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: noLimit})) + assert.False(t, doerActions.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: noLimit})) + assert.False(t, doerAdmin.CanCreateRepoIn(doerActions)) }) t.Run("GlobalLimit50", func(t *testing.T) { setting.Repository.MaxCreationLimit = 50 - assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: noLimit})) - assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 60, MaxRepoCreation: noLimit})) // limited by global limit - assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 0})) - assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 100})) - assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{NumRepos: 60, MaxRepoCreation: 100})) - - assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: noLimit})) - assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 60, MaxRepoCreation: noLimit})) - assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 0})) - assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 10, MaxRepoCreation: 100})) - assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{NumRepos: 60, MaxRepoCreation: 100})) + assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: noLimit})) + assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 60, MaxRepoCreation: noLimit})) // limited by global limit + assert.False(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 0})) + assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 100})) + assert.True(t, doerNormal.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 60, MaxRepoCreation: 100})) + + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: noLimit})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 60, MaxRepoCreation: noLimit})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 0})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 10, MaxRepoCreation: 100})) + assert.True(t, doerAdmin.CanCreateRepoIn(&user_model.User{ID: 2, NumRepos: 60, MaxRepoCreation: 100})) }) }