Fix track time list permission check (#36662) (#36744)

Backport #36662 by @lunny

Signed-off-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

Author: 4 people

services/convert/issue.go

@@ -13,6 +13,7 @@ import (
 	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/cache"
 	"code.gitea.io/gitea/modules/label"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
@@ -226,7 +227,21 @@ func ToStopWatches(ctx context.Context, doer *user_model.User, sws []*issues_mod
 // ToTrackedTimeList converts TrackedTimeList to API format
 func ToTrackedTimeList(ctx context.Context, doer *user_model.User, tl issues_model.TrackedTimeList) api.TrackedTimeList {
 	result := make([]*api.TrackedTime, 0, len(tl))
+	permCache := cache.NewEphemeralCache()
 	for _, t := range tl {
+		// If the issue is not loaded, conservatively skip this entry to avoid bypassing permission checks.
+		if t.Issue == nil || t.Issue.Repo == nil {
+			continue
+		}
+		perm, err := cache.GetWithEphemeralCache(ctx, permCache, "repo-perm", t.Issue.RepoID, func(ctx context.Context, repoID int64) (access_model.Permission, error) {
+			return access_model.GetUserRepoPermission(ctx, t.Issue.Repo, doer)
+		})
+		if err != nil {
+			continue
+		}
+		if !perm.CanReadIssuesOrPulls(t.Issue.IsPull) {
+			continue
+		}
 		result = append(result, ToTrackedTime(ctx, doer, t))
 	}
 	return result

services/convert/issue_test.go

@@ -18,6 +18,7 @@ import (
 	"code.gitea.io/gitea/modules/timeutil"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestLabel_ToLabel(t *testing.T) {
@@ -83,3 +84,43 @@ func TestToStopWatchesRespectsPermissions(t *testing.T) {
 	assert.Len(t, visibleAdmin, 2)
 	assert.ElementsMatch(t, []string{"repo1", "repo3"}, []string{visibleAdmin[0].RepoName, visibleAdmin[1].RepoName})
 }
+
+func TestToTrackedTime(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	ctx := t.Context()
+	publicIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: 1})
+	privateIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: 3})
+	regularUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
+	adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+
+	publicTrackedTime := &issues_model.TrackedTime{IssueID: publicIssue.ID, UserID: regularUser.ID, Time: 3600}
+	privateTrackedTime := &issues_model.TrackedTime{IssueID: privateIssue.ID, UserID: regularUser.ID, Time: 1800}
+	require.NoError(t, db.Insert(ctx, publicTrackedTime))
+	require.NoError(t, db.Insert(ctx, privateTrackedTime))
+
+	t.Run("NilIssues", func(t *testing.T) {
+		list := ToTrackedTimeList(ctx, regularUser, issues_model.TrackedTimeList{publicTrackedTime, privateTrackedTime})
+		assert.Empty(t, list)
+	})
+
+	t.Run("NilRepo", func(t *testing.T) {
+		badTrackedTime := &issues_model.TrackedTime{Issue: &issues_model.Issue{RepoID: 999999}}
+		visible := ToTrackedTimeList(ctx, regularUser, issues_model.TrackedTimeList{badTrackedTime})
+		assert.Empty(t, visible)
+	})
+
+	trackedTimes := issues_model.TrackedTimeList{publicTrackedTime, privateTrackedTime}
+	require.NoError(t, trackedTimes.LoadAttributes(ctx))
+
+	t.Run("ToRegularUser", func(t *testing.T) {
+		list := ToTrackedTimeList(ctx, regularUser, trackedTimes)
+		require.Len(t, list, 1)
+		assert.Equal(t, "repo1", list[0].Issue.Repo.Name)
+	})
+	t.Run("ToAdminUser", func(t *testing.T) {
+		list := ToTrackedTimeList(ctx, adminUser, trackedTimes)
+		require.Len(t, list, 2)
+		assert.ElementsMatch(t, []string{"repo1", "repo3"}, []string{list[0].Issue.Repo.Name, list[1].Issue.Repo.Name})
+	})
+}