fix: prevent stale downloads from reviving removed repositories

This commit is contained in:
tdurieux
2026-09-06 09:17:33 +02:00
parent 2af66cd1b6
commit 11eb160d2b
4 changed files with 45 additions and 12 deletions
+1
View File
@@ -1,5 +1,6 @@
{ {
"ERRORS": { "ERRORS": {
"repository_job_cancelled": "The repository changed or was removed while processing.",
"unknown_error": "Unknown error, contact the admin.", "unknown_error": "Unknown error, contact the admin.",
"unreachable": "Anonymous GitHub is unreachable, contact the admin.", "unreachable": "Anonymous GitHub is unreachable, contact the admin.",
"request_error": "Unable to download the file, check your connection or contact the admin.", "request_error": "Unable to download the file, check your connection or contact the admin.",
+20 -12
View File
@@ -470,22 +470,30 @@ export default class Repository {
* @param status the new status * @param status the new status
* @param errorMessage a potential error message to display * @param errorMessage a potential error message to display
*/ */
public protectLifecycle = false;
async updateStatus(status: RepositoryStatus, statusMessage?: string) { async updateStatus(status: RepositoryStatus, statusMessage?: string) {
if (!status) return this.model; if (!status) return this.model;
this._model.status = status; const statusDate = new Date();
this._model.statusDate = new Date(); if (isConnected) {
this._model.statusMessage = statusMessage; const result = await AnonymizedRepositoryModel.updateOne(
if (!isConnected) return this.model; {
await AnonymizedRepositoryModel.updateOne( _id: this._model._id,
{ _id: this._model._id }, ...(this.protectLifecycle ? {
{ status: { $nin: [RepositoryStatus.REMOVING, RepositoryStatus.REMOVED,
$set: { RepositoryStatus.EXPIRING, RepositoryStatus.EXPIRED] },
status, anonymizeDate: this._model.anonymizeDate,
statusDate: this._model.statusDate, } : {}),
statusMessage,
}, },
{ $set: { status, statusDate, statusMessage } }
).exec();
if (this.protectLifecycle && result.matchedCount === 0) {
throw new AnonymousError("repository_job_cancelled", { httpStatus: 410 });
} }
).exec(); }
this._model.status = status;
this._model.statusDate = statusDate;
this._model.statusMessage = statusMessage;
} }
/** /**
@@ -24,6 +24,9 @@ export default async function (job: SandboxedJob<RepoJobData, void>) {
await connect(); await connect();
const repo = await getRepository(job.data.repoId); const repo = await getRepository(job.data.repoId);
if ([RepositoryStatus.REMOVING, RepositoryStatus.REMOVED,
RepositoryStatus.EXPIRING, RepositoryStatus.EXPIRED].some((status) => status === repo.status)) return;
repo.protectLifecycle = true;
const token = await getToken(repo); const token = await getToken(repo);
const tokenKey = token.slice(-8); const tokenKey = token.slice(-8);
@@ -88,6 +91,7 @@ export default async function (job: SandboxedJob<RepoJobData, void>) {
logger.info("downloaded", { repoId: job.data.repoId }); logger.info("downloaded", { repoId: job.data.repoId });
} catch (error) { } catch (error) {
clearInterval(statusInterval); clearInterval(statusInterval);
if (error instanceof Error && error.message === "repository_job_cancelled") return;
if (tickPromise) await tickPromise; if (tickPromise) await tickPromise;
// Rate-limited: delay the job and free the worker slot // Rate-limited: delay the job and free the worker slot
@@ -123,6 +127,7 @@ export default async function (job: SandboxedJob<RepoJobData, void>) {
} }
} catch (error: unknown) { } catch (error: unknown) {
clearInterval(statusInterval); clearInterval(statusInterval);
if (error instanceof Error && error.message === "repository_job_cancelled") return;
if (tickPromise) { if (tickPromise) {
try { try {
await tickPromise; await tickPromise;
+19
View File
@@ -55,6 +55,25 @@ describe("production regressions", function () {
expect(emitted).to.equal(false); expect(emitted).to.equal(false);
}); });
for (const status of ["removing", "removed", "expiring", "expired"]) {
it(`ignores delayed downloads for ${status} repositories`, async function () {
stub(db, "connect", async () => {});
stub(db, "getRepository", async () => ({ status }));
stub(gh, "getToken", async () => { throw new Error("must not download"); });
await require("../src/queue/processes/downloadRepository").default({ data: { repoId: "repo" } });
});
it(`does not let an active worker overwrite concurrent ${status}`, async function () {
stub(db, "isConnected", true);
const model = new RepoModel({ repoId: "repo", status: "download", anonymizeDate: new Date(1000) });
const repo = new Repository(model); repo.protectLifecycle = true;
stub(RepoModel, "updateOne", filter => ({ exec: async () => ({ matchedCount:
require("sift").default(filter)({ _id: model._id, anonymizeDate: model.anonymizeDate, status }) ? 1 : 0 }) }));
try { await repo.updateStatus("ready"); throw new Error("expected rejection"); }
catch (error) { expect(error.message).to.equal("repository_job_cancelled"); }
expect(repo.status).to.equal("download");
});
}
it("denies another GitHub identity access through a reused coauthor username", async function () { it("denies another GitHub identity access through a reused coauthor username", async function () {
const User = require("../src/core/User").default; const User = require("../src/core/User").default;
const UserModel = require("../src/core/model/users/users.model").default; const UserModel = require("../src/core/model/users/users.model").default;