From 12c07641e0e00d27dd5fb29212dceddffaed09e4 Mon Sep 17 00:00:00 2001 From: tdurieux Date: Sun, 6 Sep 2026 09:16:20 +0200 Subject: [PATCH] fix: persist and recover repository removals for banned owners --- src/queue/index.ts | 10 ++++++++++ src/server/routes/admin.ts | 8 ++++++-- test/backend-reliability.test.js | 18 ++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/queue/index.ts b/src/queue/index.ts index 474c3ee..76e7bbb 100644 --- a/src/queue/index.ts +++ b/src/queue/index.ts @@ -1,3 +1,4 @@ +import UserModel from "../core/model/users/users.model"; import { JobsOptions, Queue, Worker } from "bullmq"; import config from "../config"; import AnonymizedRepositoryModel from "../core/model/anonymizedRepositories/anonymizedRepositories.model"; @@ -152,6 +153,11 @@ export async function recoverStuckRemoving() { // Claim the pending removal before queueing it. A restored repository, or // an error from a later operation, must not revive an old deletion request. const failedJobs = await removeQueue.getJobs(["failed"]); + // Older ban jobs could fail before persisting REMOVING. A banned owner + // still has a current removal request even if that old job left READY. + const bannedOwners = failedJobs.length + ? await UserModel.distinct("_id", { status: "banned" }).exec() + : []; for (const job of failedJobs) { const repoId = job.data?.repoId; if (!repoId) continue; @@ -161,6 +167,10 @@ export async function recoverStuckRemoving() { repoId, $or: [ { status: RepositoryStatus.REMOVING }, + ...(bannedOwners.length ? [{ + owner: { $in: bannedOwners }, + status: { $ne: RepositoryStatus.REMOVED }, + }] : []), ...(job.timestamp ? [{ status: RepositoryStatus.ERROR, diff --git a/src/server/routes/admin.ts b/src/server/routes/admin.ts index eede8be..0de025e 100644 --- a/src/server/routes/admin.ts +++ b/src/server/routes/admin.ts @@ -6,7 +6,7 @@ import AnonymousError from "../../core/AnonymousError"; import AnonymizedRepositoryModel from "../../core/model/anonymizedRepositories/anonymizedRepositories.model"; import ConferenceModel from "../../core/model/conference/conferences.model"; import UserModel from "../../core/model/users/users.model"; -import { cacheQueue, downloadQueue, removeQueue } from "../../queue"; +import { addRemovalJob, cacheQueue, downloadQueue, removeQueue } from "../../queue"; import { queryMetrics } from "../../queue/queueMetrics"; import { computeStats, @@ -1186,7 +1186,11 @@ router.post( let queued = 0; for (const repo of repos) { try { - await removeQueue.add(repo.repoId, { repoId: repo.repoId }, { jobId: `repo-${repo.repoId}` }); + await AnonymizedRepositoryModel.updateOne( + { repoId: repo.repoId }, + { $set: { status: "removing", statusDate: new Date() } } + ).exec(); + await addRemovalJob(repo.repoId); queued++; } catch { // job may already exist in the queue diff --git a/test/backend-reliability.test.js b/test/backend-reliability.test.js index 2538f08..b943bb4 100644 --- a/test/backend-reliability.test.js +++ b/test/backend-reliability.test.js @@ -53,6 +53,24 @@ describe("removal recovery", function () { routeUtils.handleError = originals.handleError; }); + it("retries an older ban job that failed before marking the repository removing", async function () { + let queued = false; + UserModel.distinct = () => ({ exec: async () => ["banned-owner"] }); + queueModule.removeQueue = { + getJobs: async () => [{ data: { repoId: "repo" }, timestamp: 1000, + remove: async () => { throw new Error("must not discard ban"); } }], + getJob: async () => undefined, + add: async () => { queued = true; }, + }; + AnonymizedRepositoryModel.findOneAndUpdate = filter => ({ + collation() { return this; }, + exec: async () => require("sift").default(filter)({ repoId: "repo", owner: "banned-owner", status: "ready" }) ? { repoId: "repo" } : null, + }); + AnonymizedRepositoryModel.find = () => ({ lean: async () => [] }); + await queueModule.recoverStuckRemoving(); + expect(queued).to.equal(true); + }); + for (const status of ["ready", "preparing", "removed", "error"]) { it(`discards an old failed removal after restoration to ${status}`, async function () { let discarded = false;