From 9ad80e1ebaf3b5598797f103a7b204c71dbbf456 Mon Sep 17 00:00:00 2001 From: Luc Date: Fri, 24 Jul 2026 17:40:39 +0000 Subject: [PATCH] fix: delete repo-scoped rows of seven more tables when deleting a repository (#38534) Fixes #38494 `DeleteRepositoryDirectly` left rows behind in seven registered tables carrying a repo-scoped key: `action_variable`, `action_run_attempt`, `action_tasks_version`, `renamed_branch`, `commit_status_summary`, `commit_status_index` and `repo_transfer`. Repository IDs are `pk autoincr` and never reissued, so the orphaned rows were unreachable, but they accumulated forever (unbounded table growth, referential inconsistency; not a security issue, see the issue discussion). This adds all seven to the `deleteBeans` cascade, each placed next to its sibling bean (`CommitStatus`/`CommitStatusIndex`/`CommitStatusSummary`, `Branch`/`RenamedBranch`, `Secret`/`ActionVariable`, `ActionRun`/`ActionRunAttempt`). `repo_transfer` goes through the same cascade rather than `DeleteRepositoryTransfer` so teardown stays one mechanism; the existing reaper remains for the transfer flows. The test inserts one row per previously-orphaned table (the fixture files ask test cases to prepare their own data), deletes the repository, and asserts each table is purged. Without the fix it fails on all seven tables. --------- Signed-off-by: wxiaoguang Signed-off-by: Luc Co-authored-by: wxiaoguang --- services/repository/delete.go | 7 +++++++ services/repository/delete_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/services/repository/delete.go b/services/repository/delete.go index 5e5877c4ef..38368fa2f4 100644 --- a/services/repository/delete.go +++ b/services/repository/delete.go @@ -150,7 +150,10 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams &repo_model.Collaboration{RepoID: repoID}, &issues_model.Comment{RefRepoID: repoID}, &git_model.CommitStatus{RepoID: repoID}, + &git_model.CommitStatusIndex{RepoID: repoID}, + &git_model.CommitStatusSummary{RepoID: repoID}, &git_model.Branch{RepoID: repoID}, + &git_model.RenamedBranch{RepoID: repoID}, &git_model.LFSLock{RepoID: repoID}, &repo_model.LanguageStat{RepoID: repoID}, &repo_model.RepoLicense{RepoID: repoID}, @@ -163,22 +166,26 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams &repo_model.Release{RepoID: repoID}, &repo_model.RepoIndexerStatus{RepoID: repoID}, &repo_model.Redirect{RedirectRepoID: repoID}, + &repo_model.RepoTransfer{RepoID: repoID}, // this column doesn't have index, maybe it's fine since the table shouldn't be too large. &repo_model.RepoUnit{RepoID: repoID}, &repo_model.Star{RepoID: repoID}, &admin_model.Task{RepoID: repoID}, &repo_model.Watch{RepoID: repoID}, &webhook.Webhook{RepoID: repoID}, &secret_model.Secret{RepoID: repoID}, + &actions_model.ActionVariable{RepoID: repoID}, &actions_model.ActionTaskStep{RepoID: repoID}, &actions_model.ActionTask{RepoID: repoID}, &actions_model.ActionRunJob{RepoID: repoID}, &actions_model.ActionRun{RepoID: repoID}, + &actions_model.ActionRunAttempt{RepoID: repoID}, &actions_model.ActionRunner{RepoID: repoID}, &actions_model.ActionScheduleSpec{RepoID: repoID}, &actions_model.ActionSchedule{RepoID: repoID}, &actions_model.ActionArtifact{RepoID: repoID}, &actions_model.ActionRunJobSummary{RepoID: repoID}, &actions_model.ActionRunnerToken{RepoID: repoID}, + &actions_model.ActionTasksVersion{RepoID: repoID}, &actions_model.ActionScopedWorkflowSource{SourceRepoID: repoID}, &issues_model.IssuePin{RepoID: repoID}, ); err != nil { diff --git a/services/repository/delete_test.go b/services/repository/delete_test.go index b85ad10736..c747d0087e 100644 --- a/services/repository/delete_test.go +++ b/services/repository/delete_test.go @@ -6,6 +6,9 @@ package repository_test import ( "testing" + actions_model "gitea.dev/models/actions" + "gitea.dev/models/db" + git_model "gitea.dev/models/git" "gitea.dev/models/organization" repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" @@ -52,3 +55,28 @@ func TestDeleteOwnerRepositoriesDirectly(t *testing.T) { assert.NoError(t, repo_service.DeleteOwnerRepositoriesDirectly(t.Context(), user)) } + +func TestDeleteRepositoryDirectlyPurgesRepoScopedRows(t *testing.T) { + unittest.PrepareTestEnv(t) + + // One row per table that repository deletion used to leave behind (#38494). + assert.NoError(t, db.Insert(t.Context(), + &actions_model.ActionVariable{RepoID: 1, Name: "to_purge", Data: "value"}, + &actions_model.ActionRunAttempt{RepoID: 1, RunID: unittest.NonexistentID, Attempt: 1}, + &actions_model.ActionTasksVersion{RepoID: 1, Version: 1}, + &git_model.RenamedBranch{RepoID: 1, From: "old-name", To: "new-name"}, + &git_model.CommitStatusSummary{RepoID: 1, SHA: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", State: "success"}, + &repo_model.RepoTransfer{RepoID: 1, DoerID: 2, RecipientID: 3}, + )) + unittest.AssertExistsAndLoadBean(t, &git_model.CommitStatusIndex{RepoID: 1}) + + assert.NoError(t, repo_service.DeleteRepositoryDirectly(t.Context(), 1)) + + unittest.AssertNotExistsBean(t, &actions_model.ActionVariable{RepoID: 1}) + unittest.AssertNotExistsBean(t, &actions_model.ActionRunAttempt{RepoID: 1}) + unittest.AssertNotExistsBean(t, &actions_model.ActionTasksVersion{RepoID: 1}) + unittest.AssertNotExistsBean(t, &git_model.RenamedBranch{RepoID: 1}) + unittest.AssertNotExistsBean(t, &git_model.CommitStatusSummary{RepoID: 1}) + unittest.AssertNotExistsBean(t, &git_model.CommitStatusIndex{RepoID: 1}) + unittest.AssertNotExistsBean(t, &repo_model.RepoTransfer{RepoID: 1}) +}