From 667ddab36af2e77a3449e4eff3fc975160a0733e Mon Sep 17 00:00:00 2001 From: Ross Golder Date: Sat, 14 Mar 2026 14:01:05 +0700 Subject: [PATCH 1/2] fix: RecalculateUserAccess sets incorrect minMode for public repos Public repositories were granted AccessModeWrite as the minimum access mode, which incorrectly elevated access for all users on public repos. The minimum should be AccessModeNone, with access granted explicitly through collaborator and team memberships. --- models/perm/access/access.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/models/perm/access/access.go b/models/perm/access/access.go index acc34c434e..e9e5a0c026 100644 --- a/models/perm/access/access.go +++ b/models/perm/access/access.go @@ -231,10 +231,7 @@ func RecalculateTeamAccesses(ctx context.Context, repo *repo_model.Repository, i // RecalculateUserAccess recalculates new access for a single user // Usable if we know access only affected one user func RecalculateUserAccess(ctx context.Context, repo *repo_model.Repository, uid int64) (err error) { - minMode := perm.AccessModeRead - if !repo.IsPrivate { - minMode = perm.AccessModeWrite - } + minMode := perm.AccessModeNone accessMode := perm.AccessModeNone e := db.GetEngine(ctx) From 8f7c155a98a71c43ec2a15504380acb47b3a5fc7 Mon Sep 17 00:00:00 2001 From: Ross Golder Date: Sun, 17 May 2026 13:23:57 +0700 Subject: [PATCH 2/2] fix(perm): fix RecalculateUserAccess to not insert empty access records Add test for RecalculateUserAccess to verify no access record is created for users without collaboration/team membership, and collaborator gets correct access mode. Co-Authored-By: Ross Golder --- models/perm/access/access.go | 2 +- models/perm/access/access_test.go | 33 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/models/perm/access/access.go b/models/perm/access/access.go index e9e5a0c026..eb0bd57633 100644 --- a/models/perm/access/access.go +++ b/models/perm/access/access.go @@ -267,7 +267,7 @@ func RecalculateUserAccess(ctx context.Context, repo *repo_model.Repository, uid // Delete old user accesses and insert new one for repository. if _, err = e.Delete(&Access{RepoID: repo.ID, UserID: uid}); err != nil { return fmt.Errorf("delete old user accesses: %w", err) - } else if accessMode >= minMode { + } else if accessMode > minMode { if err = db.Insert(ctx, &Access{RepoID: repo.ID, UserID: uid, Mode: accessMode}); err != nil { return fmt.Errorf("insert new user accesses: %w", err) } diff --git a/models/perm/access/access_test.go b/models/perm/access/access_test.go index 148a02efa3..b75506cf4c 100644 --- a/models/perm/access/access_test.go +++ b/models/perm/access/access_test.go @@ -26,6 +26,7 @@ func TestAccess(t *testing.T) { t.Run("RecalculateAccesses2", testRecalculateAccesses2) t.Run("RecalculateAccessesUpdateMode", testRecalculateAccessesUpdateMode) t.Run("RecalculateAccessesRemoveAccess", testRecalculateAccessesRemoveAccess) + t.Run("RecalculateUserAccess", testRecalculateUserAccess) } func testAccessLevel(t *testing.T) { @@ -201,3 +202,35 @@ func testRecalculateAccessesRemoveAccess(t *testing.T) { assert.NoError(t, err) assert.False(t, has, "Access should be deleted after removing collaboration") } + +func testRecalculateUserAccess(t *testing.T) { + t.Run("NoAccessForUserWithoutCollaboration", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + assert.NoError(t, repo.LoadOwner(t.Context())) + + userID := int64(8) + _, _ = db.GetEngine(t.Context()).Where("user_id = ? AND repo_id = ?", userID, repo.ID).Delete(&access_model.Access{}) + assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), repo, userID)) + + access := &access_model.Access{UserID: userID, RepoID: repo.ID} + has, err := db.GetEngine(t.Context()).Get(access) + assert.NoError(t, err) + assert.False(t, has, "User without collaboration/team membership should have no access record") + }) + + t.Run("CollaboratorGetsAccess", func(t *testing.T) { + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + assert.NoError(t, repo.LoadOwner(t.Context())) + + userID := int64(4) + _ = db.Insert(t.Context(), &repo_model.Collaboration{UserID: userID, RepoID: repo.ID, Mode: perm_model.AccessModeWrite}) + + assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), repo, userID)) + + access := &access_model.Access{UserID: userID, RepoID: repo.ID} + has, err := db.GetEngine(t.Context()).Get(access) + assert.NoError(t, err) + assert.True(t, has) + assert.Equal(t, perm_model.AccessModeWrite, access.Mode) + }) +}