Skip to content

fix users being able bypass limits with repo transfers #34031

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Mar 31, 2025
8 changes: 8 additions & 0 deletions models/fixtures/repo_transfer.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,11 @@
repo_id: 32
created_unix: 1553610671
updated_unix: 1553610671

-
id: 4
doer_id: 3
recipient_id: 1
repo_id: 5
created_unix: 1553610671
updated_unix: 1553610671
7 changes: 7 additions & 0 deletions routers/api/v1/repo/transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ func Transfer(ctx *context.APIContext) {
return
}

if repo_service.IsRepositoryLimitReached(err) {
ctx.APIError(http.StatusForbidden, err)
return
}

if errors.Is(err, user_model.ErrBlockedUser) {
ctx.APIError(http.StatusForbidden, err)
} else {
Expand Down Expand Up @@ -169,6 +174,8 @@ func AcceptTransfer(ctx *context.APIContext) {
ctx.APIError(http.StatusNotFound, err)
case errors.Is(err, util.ErrPermissionDenied):
ctx.APIError(http.StatusForbidden, err)
case repo_service.IsRepositoryLimitReached(err):
ctx.APIError(http.StatusForbidden, err)
default:
ctx.APIErrorInternal(err)
}
Expand Down
10 changes: 7 additions & 3 deletions routers/web/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,11 +305,15 @@ func CreatePost(ctx *context.Context) {
}

func handleActionError(ctx *context.Context, err error) {
if errors.Is(err, user_model.ErrBlockedUser) {
switch {
case errors.Is(err, user_model.ErrBlockedUser):
ctx.Flash.Error(ctx.Tr("repo.action.blocked_user"))
} else if errors.Is(err, util.ErrPermissionDenied) {
case repo_service.IsRepositoryLimitReached(err):
limit := err.(repo_service.LimitReachedError).Limit
ctx.Flash.Error(ctx.TrN(limit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", limit))
case errors.Is(err, util.ErrPermissionDenied):
ctx.HTTPError(http.StatusNotFound)
} else {
default:
ctx.ServerError(fmt.Sprintf("Action (%s)", ctx.PathParam("action")), err)
}
}
Expand Down
3 changes: 3 additions & 0 deletions routers/web/repo/setting/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,9 @@ func handleSettingsPostTransfer(ctx *context.Context) {
ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplSettingsOptions, nil)
} else if repo_model.IsErrRepoTransferInProgress(err) {
ctx.RenderWithErr(ctx.Tr("repo.settings.transfer_in_progress"), tplSettingsOptions, nil)
} else if repo_service.IsRepositoryLimitReached(err) {
limit := err.(repo_service.LimitReachedError).Limit
ctx.RenderWithErr(ctx.TrN(limit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", limit), tplSettingsOptions, nil)
} else if errors.Is(err, user_model.ErrBlockedUser) {
ctx.RenderWithErr(ctx.Tr("repo.settings.transfer.blocked_user"), tplSettingsOptions, nil)
} else {
Expand Down
22 changes: 22 additions & 0 deletions services/repository/transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,22 @@ import (
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/globallock"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
notify_service "code.gitea.io/gitea/services/notify"
)

type LimitReachedError struct{ Limit int }

func (LimitReachedError) Error() string {
return "Repository limit has been reached"
}

func IsRepositoryLimitReached(err error) bool {
_, ok := err.(LimitReachedError)
return ok
}

func getRepoWorkingLockKey(repoID int64) string {
return fmt.Sprintf("repo_working_%d", repoID)
}
Expand All @@ -49,6 +61,11 @@ func AcceptTransferOwnership(ctx context.Context, repo *repo_model.Repository, d
return err
}

if !doer.IsAdmin && !repoTransfer.Recipient.CanCreateRepo() {
limit := util.Iif(repoTransfer.Recipient.MaxRepoCreation >= 0, repoTransfer.Recipient.MaxRepoCreation, setting.Repository.MaxCreationLimit)
return LimitReachedError{Limit: limit}
}

if !repoTransfer.CanUserAcceptOrRejectTransfer(ctx, doer) {
return util.ErrPermissionDenied
}
Expand Down Expand Up @@ -399,6 +416,11 @@ func StartRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.Use
return err
}

if !doer.IsAdmin && !newOwner.CanCreateRepo() {
limit := util.Iif(newOwner.MaxRepoCreation >= 0, newOwner.MaxRepoCreation, setting.Repository.MaxCreationLimit)
return LimitReachedError{Limit: limit}
}

var isDirectTransfer bool
oldOwnerName := repo.OwnerName

Expand Down
42 changes: 41 additions & 1 deletion services/repository/transfer_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2019 The Gitea Authors. All rights reserved.
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package repository
Expand All @@ -14,11 +14,14 @@ import (
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/feed"
notify_service "code.gitea.io/gitea/services/notify"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

var notifySync sync.Once
Expand Down Expand Up @@ -125,3 +128,40 @@ func TestRepositoryTransfer(t *testing.T) {
err = RejectRepositoryTransfer(db.DefaultContext, repo2, doer)
assert.True(t, repo_model.IsErrNoPendingTransfer(err))
}

// Test transfer rejections
func TestRepositoryTransferRejection(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
// Set limit to 0 repositories so no repositories can be transferred
defer test.MockVariableValue(&setting.Repository.MaxCreationLimit, 0)()

// Admin case
doerAdmin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 5})

transfer, err := repo_model.GetPendingRepositoryTransfer(db.DefaultContext, repo)
require.NoError(t, err)
require.NotNil(t, transfer)
require.NoError(t, transfer.LoadRecipient(db.DefaultContext))

require.True(t, transfer.Recipient.CanCreateRepo()) // admin is not subject to limits

// Administrator should not be affected by the limits so transfer should be successful
assert.NoError(t, AcceptTransferOwnership(db.DefaultContext, repo, doerAdmin))

// Non admin user case
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 10})
repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 21})

transfer, err = repo_model.GetPendingRepositoryTransfer(db.DefaultContext, repo)
require.NoError(t, err)
require.NotNil(t, transfer)
require.NoError(t, transfer.LoadRecipient(db.DefaultContext))

require.False(t, transfer.Recipient.CanCreateRepo()) // regular user is subject to limits

// Cannot accept because of the limit
err = AcceptTransferOwnership(db.DefaultContext, repo, doer)
assert.Error(t, err)
assert.True(t, IsRepositoryLimitReached(err))
}