From 362539b78e50dfcc4bd1b9368fdd2a1b8cab0129 Mon Sep 17 00:00:00 2001 From: bircni Date: Fri, 10 Jul 2026 00:21:44 +0200 Subject: [PATCH] fix(api): stop leaking private repo metadata after access revocation (#38321) The `/user/starred` and `/user/subscriptions` endpoints returned private repositories a user had starred/watched even after their access to those repositories was revoked, still exposing the repository name, description and visibility (including later metadata changes). Private repositories in the starred/watched queries are now gated on the actor's current access via `AccessibleRepositoryCondition`, so users who no longer have access no longer receive the metadata. Public repositories and public-only tokens are unaffected. --- models/repo/user_repo.go | 16 +++++++- routers/api/v1/user/star.go | 1 + routers/api/v1/user/watch.go | 1 + tests/integration/api_user_star_test.go | 49 +++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) diff --git a/models/repo/user_repo.go b/models/repo/user_repo.go index 94425c0a1d8..2a861a17bb5 100644 --- a/models/repo/user_repo.go +++ b/models/repo/user_repo.go @@ -22,6 +22,9 @@ type StarredReposOptions struct { StarrerID int64 RepoOwnerID int64 IncludePrivate bool + // Actor is the user the private repositories are gated on: a private repo is only + // returned when Actor still has access to it, even if it was starred while access was granted. + Actor *user_model.User } func (opts *StarredReposOptions) ApplyPublicOnly(publicOnly bool) { @@ -39,7 +42,10 @@ func (opts *StarredReposOptions) ToConds() builder.Cond { "repository.owner_id": opts.RepoOwnerID, }) } - if !opts.IncludePrivate { + if opts.IncludePrivate { + // only include private repos the actor can still access, so metadata does not leak after access revocation + cond = cond.And(AccessibleRepositoryCondition(opts.Actor, unit.TypeInvalid)) + } else { cond = cond.And(builder.Eq{ "repository.is_private": false, }) @@ -66,6 +72,9 @@ type WatchedReposOptions struct { WatcherID int64 RepoOwnerID int64 IncludePrivate bool + // Actor is the user the private repositories are gated on: a private repo is only + // returned when Actor still has access to it, even if it was watched while access was granted. + Actor *user_model.User } func (opts *WatchedReposOptions) ApplyPublicOnly(publicOnly bool) { @@ -83,7 +92,10 @@ func (opts *WatchedReposOptions) ToConds() builder.Cond { "repository.owner_id": opts.RepoOwnerID, }) } - if !opts.IncludePrivate { + if opts.IncludePrivate { + // only include private repos the actor can still access, so metadata does not leak after access revocation + cond = cond.And(AccessibleRepositoryCondition(opts.Actor, unit.TypeInvalid)) + } else { cond = cond.And(builder.Eq{ "repository.is_private": false, }) diff --git a/routers/api/v1/user/star.go b/routers/api/v1/user/star.go index c78b6872c5a..c097117d1b2 100644 --- a/routers/api/v1/user/star.go +++ b/routers/api/v1/user/star.go @@ -24,6 +24,7 @@ func getStarredRepos(ctx *context.APIContext, user *user_model.User, private boo ListOptions: utils.GetListOptions(ctx), StarrerID: user.ID, IncludePrivate: private, + Actor: user, } opts.ApplyPublicOnly(ctx.PublicOnly) diff --git a/routers/api/v1/user/watch.go b/routers/api/v1/user/watch.go index 2d1f55d8709..e0396efea02 100644 --- a/routers/api/v1/user/watch.go +++ b/routers/api/v1/user/watch.go @@ -22,6 +22,7 @@ func getWatchedRepos(ctx *context.APIContext, user *user_model.User, private boo ListOptions: utils.GetListOptions(ctx), WatcherID: user.ID, IncludePrivate: private, + Actor: user, } opts.ApplyPublicOnly(ctx.PublicOnly) diff --git a/tests/integration/api_user_star_test.go b/tests/integration/api_user_star_test.go index 056d4cd70fc..8976a7669aa 100644 --- a/tests/integration/api_user_star_test.go +++ b/tests/integration/api_user_star_test.go @@ -9,11 +9,14 @@ import ( "testing" auth_model "gitea.dev/models/auth" + "gitea.dev/models/perm" + repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" "gitea.dev/modules/setting" api "gitea.dev/modules/structs" "gitea.dev/modules/test" + repo_service "gitea.dev/services/repository" "gitea.dev/tests" "github.com/stretchr/testify/assert" @@ -93,6 +96,52 @@ func TestAPIStar(t *testing.T) { }) } +// TestAPIStarredReposAccessRevoked ensures a private repository disappears from a user's +// starred list once their access to it has been revoked, so no metadata leaks afterwards. +func TestAPIStarredReposAccessRevoked(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // user2/repo2 is a private repository owned by user2 + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + collaborator := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + + // grant the collaborator read access and star the private repo as them + require.NoError(t, repo_service.AddOrUpdateCollaborator(t.Context(), repo, collaborator, perm.AccessModeRead)) + require.NoError(t, repo_model.StarRepo(t.Context(), collaborator, repo, true)) + + token := getUserToken(t, collaborator.Name, auth_model.AccessTokenScopeReadUser, auth_model.AccessTokenScopeReadRepository) + + // while access is granted, the private repo is visible in the starred list + req := NewRequest(t, "GET", "/api/v1/user/starred").AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + repos := DecodeJSON(t, resp, []api.Repository{}) + require.Len(t, repos, 1) + assert.Equal(t, repo.FullName(), repos[0].FullName) + + // revoke access + require.NoError(t, repo_service.DeleteCollaboration(t.Context(), repo, collaborator)) + + // the star record still exists, but the repo (and its metadata) must no longer be returned + assert.True(t, repo_model.IsStaring(t.Context(), collaborator.ID, repo.ID)) + req = NewRequest(t, "GET", "/api/v1/user/starred").AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusOK) + repos = DecodeJSON(t, resp, []api.Repository{}) + assert.Empty(t, repos) + + // sanity: the owner still sees their own private repo + ownerToken := getUserToken(t, owner.Name, auth_model.AccessTokenScopeReadUser, auth_model.AccessTokenScopeReadRepository) + require.NoError(t, repo_model.StarRepo(t.Context(), owner, repo, true)) + req = NewRequest(t, "GET", "/api/v1/user/starred").AddTokenAuth(ownerToken) + resp = MakeRequest(t, req, http.StatusOK) + repos = DecodeJSON(t, resp, []api.Repository{}) + fullNames := make([]string, len(repos)) + for i, r := range repos { + fullNames[i] = r.FullName + } + assert.Contains(t, fullNames, repo.FullName()) +} + func TestAPIStarDisabled(t *testing.T) { defer tests.PrepareTestEnv(t)()