fix(security): harden access checks and migration validation (#38324)

Harden access checks for issue dependencies, team repository membership,
notifications, stars, tracked times and repository migrations.
This commit is contained in:
bircni
2026-07-10 19:30:43 +02:00
committed by GitHub
parent f452c369ac
commit aab3242f7b
15 changed files with 304 additions and 15 deletions
+22
View File
@@ -682,6 +682,22 @@ func getRepositoryByParams(ctx *context.APIContext) *repo_model.Repository {
return repo
}
func canChangeTeamRepository(ctx *context.APIContext) bool {
if ctx.Org.Organization.RepoAdminChangeTeamAccess {
return true
}
isOwner, err := ctx.Org.Organization.IsOwnedBy(ctx, ctx.Doer.ID)
if err != nil {
ctx.APIErrorInternal(err)
return false
}
if !isOwner {
ctx.APIError(http.StatusForbidden, "user is nor repo admin nor owner")
return false
}
return true
}
// AddTeamRepository api for adding a repository to a team
func AddTeamRepository(ctx *context.APIContext) {
// swagger:operation PUT /teams/{id}/repos/{org}/{repo} organization orgAddTeamRepository
@@ -718,6 +734,9 @@ func AddTeamRepository(ctx *context.APIContext) {
if ctx.Written() {
return
}
if !canChangeTeamRepository(ctx) {
return
}
if access, err := access_model.AccessLevel(ctx, ctx.Doer, repo); err != nil {
ctx.APIErrorInternal(err)
return
@@ -770,6 +789,9 @@ func RemoveTeamRepository(ctx *context.APIContext) {
if ctx.Written() {
return
}
if !canChangeTeamRepository(ctx) {
return
}
if access, err := access_model.AccessLevel(ctx, ctx.Doer, repo); err != nil {
ctx.APIErrorInternal(err)
return
+33
View File
@@ -9,6 +9,7 @@ import (
"gitea.dev/models/db"
issues_model "gitea.dev/models/issues"
access_model "gitea.dev/models/perm/access"
"gitea.dev/models/unit"
user_model "gitea.dev/models/user"
api "gitea.dev/modules/structs"
@@ -132,11 +133,33 @@ func ListTrackedTimes(ctx *context.APIContext) {
ctx.APIErrorInternal(err)
return
}
trackedTimes, err = filterTrackedTimesByAccess(ctx, trackedTimes)
if err != nil {
ctx.APIErrorInternal(err)
return
}
ctx.SetTotalCountHeader(count)
ctx.JSON(http.StatusOK, convert.ToTrackedTimeList(ctx, ctx.Doer, trackedTimes))
}
func filterTrackedTimesByAccess(ctx *context.APIContext, trackedTimes issues_model.TrackedTimeList) (issues_model.TrackedTimeList, error) {
filtered := make(issues_model.TrackedTimeList, 0, len(trackedTimes))
for _, trackedTime := range trackedTimes {
if trackedTime.Issue == nil || trackedTime.Issue.Repo == nil {
continue
}
permission, err := access_model.GetIndividualUserRepoPermission(ctx, trackedTime.Issue.Repo, ctx.Doer)
if err != nil {
return nil, err
}
if permission.HasAnyUnitAccessOrPublicAccess() {
filtered = append(filtered, trackedTime)
}
}
return filtered, nil
}
// AddTime add time manual to the given issue
func AddTime(ctx *context.APIContext) {
// swagger:operation Post /repos/{owner}/{repo}/issues/{index}/times issue issueAddTime
@@ -542,6 +565,11 @@ func ListTrackedTimesByRepository(ctx *context.APIContext) {
ctx.APIErrorInternal(err)
return
}
trackedTimes, err = filterTrackedTimesByAccess(ctx, trackedTimes)
if err != nil {
ctx.APIErrorInternal(err)
return
}
ctx.SetTotalCountHeader(count)
ctx.JSON(http.StatusOK, convert.ToTrackedTimeList(ctx, ctx.Doer, trackedTimes))
@@ -604,6 +632,11 @@ func ListMyTrackedTimes(ctx *context.APIContext) {
ctx.APIErrorInternal(err)
return
}
trackedTimes, err = filterTrackedTimesByAccess(ctx, trackedTimes)
if err != nil {
ctx.APIErrorInternal(err)
return
}
ctx.SetTotalCountHeader(count)
ctx.JSON(http.StatusOK, convert.ToTrackedTimeList(ctx, ctx.Doer, trackedTimes))
+6 -3
View File
@@ -33,13 +33,16 @@ func getStarredRepos(ctx *context.APIContext, user *user_model.User, private boo
return nil, err
}
repos := make([]*api.Repository, len(starredRepos))
for i, starred := range starredRepos {
repos := make([]*api.Repository, 0, len(starredRepos))
for _, starred := range starredRepos {
permission, err := access_model.GetIndividualUserRepoPermission(ctx, starred, user)
if err != nil {
return nil, err
}
repos[i] = convert.ToRepo(ctx, starred, permission)
if !permission.HasAnyUnitAccessOrPublicAccess() {
continue
}
repos = append(repos, convert.ToRepo(ctx, starred, permission))
}
return repos, nil
}
+19
View File
@@ -130,6 +130,25 @@ func RemoveDependency(ctx *context.Context) {
return
}
// Existing cross-repo dependencies must remain removable even when
// AllowCrossRepositoryDependencies is disabled, so only enforce that the
// doer can read the dependency's repository.
if issue.RepoID != dep.RepoID {
if err := dep.LoadRepo(ctx); err != nil {
ctx.ServerError("loadRepo", err)
return
}
depRepoPerm, err := access_model.GetDoerRepoPermission(ctx, dep.Repo, ctx.Doer)
if err != nil {
ctx.ServerError("GetDoerRepoPermission", err)
return
}
if !depRepoPerm.CanReadIssuesOrPulls(dep.IsPull) {
ctx.Redirect(issue.Link())
return
}
}
if err = issues_model.RemoveIssueDependency(ctx, ctx.Doer, issue, dep, depType); err != nil {
if issues_model.IsErrDependencyNotExists(err) {
ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_not_exist"))
+26
View File
@@ -4,6 +4,7 @@
package user
import (
stdCtx "context"
"fmt"
"net/http"
"strings"
@@ -12,8 +13,10 @@ import (
"gitea.dev/models/db"
git_model "gitea.dev/models/git"
issues_model "gitea.dev/models/issues"
access_model "gitea.dev/models/perm/access"
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unit"
user_model "gitea.dev/models/user"
"gitea.dev/modules/base"
"gitea.dev/modules/container"
"gitea.dev/modules/log"
@@ -97,6 +100,12 @@ func prepareUserNotificationsData(ctx *context.Context) {
return
}
failCount += len(failures)
notifications, failures, err = filterNotificationsByRepoAccess(ctx, ctx.Doer, notifications)
if err != nil {
ctx.ServerError("filterNotificationsByRepoAccess", err)
return
}
failCount += len(failures)
failures, err = notifications.LoadIssues(ctx)
if err != nil {
@@ -135,6 +144,23 @@ func prepareUserNotificationsData(ctx *context.Context) {
ctx.Data["Page"] = pager
}
func filterNotificationsByRepoAccess(ctx stdCtx.Context, doer *user_model.User, notifications activities_model.NotificationList) (activities_model.NotificationList, []int, error) {
failures := make([]int, 0)
for i, notification := range notifications {
if notification.Repository == nil {
continue
}
perm, err := access_model.GetIndividualUserRepoPermission(ctx, notification.Repository, doer)
if err != nil {
return nil, nil, err
}
if !perm.HasAnyUnitAccessOrPublicAccess() {
failures = append(failures, i)
}
}
return notifications.Without(failures), failures, nil
}
// NotificationStatusPost is a route for changing the status of a notification
func NotificationStatusPost(ctx *context.Context) {
notificationID := ctx.FormInt64("notification_id")
+37
View File
@@ -0,0 +1,37 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package user
import (
"testing"
activities_model "gitea.dev/models/activities"
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unittest"
user_model "gitea.dev/models/user"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestFilterNotificationsByRepoAccess(t *testing.T) {
require.NoError(t, unittest.LoadFixtures())
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 40})
inaccessibleRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
require.True(t, inaccessibleRepo.IsPrivate)
accessibleRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
notifications := activities_model.NotificationList{
{ID: 1, Repository: inaccessibleRepo},
{ID: 2, Repository: accessibleRepo},
}
filtered, failures, err := filterNotificationsByRepoAccess(t.Context(), doer, notifications)
require.NoError(t, err)
assert.Equal(t, []int{0}, failures)
require.Len(t, filtered, 1)
assert.EqualValues(t, 2, filtered[0].ID)
}