diff --git a/public/i18n/locale-en.json b/public/i18n/locale-en.json index 66285e7..1ec75d1 100644 --- a/public/i18n/locale-en.json +++ b/public/i18n/locale-en.json @@ -1,5 +1,6 @@ { "ERRORS": { + "repository_job_cancelled": "The repository changed or was removed while processing.", "unknown_error": "Unknown error, 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.", diff --git a/src/core/Repository.ts b/src/core/Repository.ts index 53dd195..2cf7d48 100644 --- a/src/core/Repository.ts +++ b/src/core/Repository.ts @@ -470,22 +470,30 @@ export default class Repository { * @param status the new status * @param errorMessage a potential error message to display */ + public protectLifecycle = false; + async updateStatus(status: RepositoryStatus, statusMessage?: string) { if (!status) return this.model; - this._model.status = status; - this._model.statusDate = new Date(); - this._model.statusMessage = statusMessage; - if (!isConnected) return this.model; - await AnonymizedRepositoryModel.updateOne( - { _id: this._model._id }, - { - $set: { - status, - statusDate: this._model.statusDate, - statusMessage, + const statusDate = new Date(); + if (isConnected) { + const result = await AnonymizedRepositoryModel.updateOne( + { + _id: this._model._id, + ...(this.protectLifecycle ? { + status: { $nin: [RepositoryStatus.REMOVING, RepositoryStatus.REMOVED, + RepositoryStatus.EXPIRING, RepositoryStatus.EXPIRED] }, + anonymizeDate: this._model.anonymizeDate, + } : {}), }, + { $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; } /** diff --git a/src/queue/processes/downloadRepository.ts b/src/queue/processes/downloadRepository.ts index 577f868..1d93ac4 100644 --- a/src/queue/processes/downloadRepository.ts +++ b/src/queue/processes/downloadRepository.ts @@ -24,6 +24,9 @@ export default async function (job: SandboxedJob) { await connect(); 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 tokenKey = token.slice(-8); @@ -88,6 +91,7 @@ export default async function (job: SandboxedJob) { logger.info("downloaded", { repoId: job.data.repoId }); } catch (error) { clearInterval(statusInterval); + if (error instanceof Error && error.message === "repository_job_cancelled") return; if (tickPromise) await tickPromise; // Rate-limited: delay the job and free the worker slot @@ -123,6 +127,7 @@ export default async function (job: SandboxedJob) { } } catch (error: unknown) { clearInterval(statusInterval); + if (error instanceof Error && error.message === "repository_job_cancelled") return; if (tickPromise) { try { await tickPromise; diff --git a/test/production-regressions.test.js b/test/production-regressions.test.js index ed172bc..b713825 100644 --- a/test/production-regressions.test.js +++ b/test/production-regressions.test.js @@ -55,6 +55,25 @@ describe("production regressions", function () { 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 () { const User = require("../src/core/User").default; const UserModel = require("../src/core/model/users/users.model").default;