From 772e40debd045b94051defdf6a9f33cc9f2fe5a6 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Wed, 14 Oct 2026 20:23:12 +0200 Subject: [PATCH] fix: retain recipient suppression after access recovery (#897) * fix: retain recipient suppression after access recovery * fix: migrate cold starts and fence resource deactivation * docs: stop every writer before alert reconciliation --- Dockerfile | 2 +- README.md | 54 +-- scripts/deploy.sh | 8 +- .../anonymizedGists/anonymizedGists.schema.ts | 3 + .../anonymizedPullRequests.schema.ts | 3 + .../anonymizedRepositories.schema.ts | 3 + .../model/owner-alert-lifecycle.schema.ts | 54 +++ src/core/model/users/users.schema.ts | 1 + src/core/owner-notifications.ts | 84 +++-- test/deploy-owner-alert-migration.test.js | 12 +- test/owner-notifications.test.js | 11 +- test/recipient-alerts-mongo.test.js | 316 ++++++++++++++---- 12 files changed, 429 insertions(+), 122 deletions(-) create mode 100644 src/core/model/owner-alert-lifecycle.schema.ts diff --git a/Dockerfile b/Dockerfile index a34640b..6d82a0a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -26,7 +26,7 @@ RUN --mount=type=cache,id=npm-prod,target=/root/.npm,sharing=locked \ # Stage 4: minimal runtime FROM node:22-alpine AS runtime -LABEL science.4open.owner-alert-protocol="recipient-v1" +LABEL science.4open.owner-alert-protocol="recipient-v2" ENV NODE_ENV=production ENV PORT=5000 EXPOSE 5000 diff --git a/README.md b/README.md index 64ad28f..2c8346c 100644 --- a/README.md +++ b/README.md @@ -116,20 +116,15 @@ Alerts cover invalid GitHub credentials and repository access errors encountered while reading or refreshing content. No periodic access scan is performed. Emails contain no repository names, anonymous URLs, or credentials. -An atomic recipient claim allows one email attempt across repositories, pull -requests, gists and accounts sharing the same address. Sign-in does not reopen -alerts. An explicit reconnect or resource change prepares recovery, but the -recipient stays blocked until a fresh GitHub read confirms access to the resource -that triggered the alert. A fresh read during an in-flight send is retained and -applied when that send completes. A newer reset or source change invalidates -that recovery evidence. Cached reads and successful reads of other resources -cannot reopen it. Removed or expired resources and disabled owners no longer -block future alerts after their send has completed. +An atomic recipient claim allows one email attempt per normalized address across +repositories, pull requests, gists and accounts. Suppression has no time expiry. +Sign-in, reconnecting, successful access recovery, resource removal or expiration, +and disabling the original account retain suppression for that address. Inactive +resources cannot trigger new alerts. Changing to an address that has never been +notified permits its first alert. -Dispatched recipient guards have no time expiry, so there are no daily reminders. -Failed deliveries remain blocked until confirmed recovery. An interrupted send -whose outcome is unknown stays blocked to avoid duplicate delivery. Unsent claims -can be cancelled by confirmed recovery before dispatch. Bookkeeping failures +Failed deliveries and interrupted sends whose outcome is unknown also stay +blocked to avoid duplicate delivery. Unsent claims can be cancelled by confirmed recovery before dispatch. Bookkeeping failures release claims when no provider request has started. Pending claims interrupted before dispatch can be reclaimed after a 60-second lease during startup or a new notification attempt. Claim identity and state checks prevent a paused sender @@ -150,22 +145,31 @@ Recovery recorded during an active reset is retried when reset finishes or its lease expires, with the original source and revision checks. Provider idempotency keys and redacted attempt records support auditing without storing recipient addresses -or repository names in the audit log. Attempt records are saved before dispatch, +or repository names in the audit log. Records include owner and resource IDs, +resource kind, generation and a categorized access failure to identify the +trigger without exposing the address. Attempt records are saved before dispatch, with completion recorded separately. Existing resource claims are migrated to recipient guards before new application instances serve requests using an indexed scan with resumable checkpoints. Other instances wait while its lease is healthy, and take over if the lease expires. -Every existing resource claim is retained behind the recipient guard. Recovering -one resource atomically marks its claim recovered; other claims keep the recipient -blocked until they recover or are retired. A newer owner reset makes its matching -claims require another fresh read. Completed provider results are recorded before -guard updates, with bounded retries and reconciliation on startup or a later -notification. Retirement also checks the latest owner reset revision. +Every existing resource claim is retained behind the recipient guard. Recovery +marks claims recovered without authorizing another email. Completed provider +results are recorded before guard updates, with bounded retries and reconciliation +on startup or a later notification. The upgrade restores missing recipient guards +from immutable provider completion records, including guards previously released +by recovery or resource removal. This history scan uses an index and resumable +checkpoints and does not send email. Its `recipient-alerts-v2` database marker is +independent of the v1 marker, so cold starts also restore missing suppression. +Resource lifecycle mutations advance the owner dispatch fence and block new +attempts until the status write completes. Concurrent mutations keep separate +reservations. Failed or ambiguous status writes retain suppression for that +owner; run the offline reconciliation command below after stopping all writers +to clear interrupted reservations safely. -Use `scripts/deploy.sh` for the first upgrade to recipient alerts. The new image -has the `science.4open.owner-alert-protocol=recipient-v1` label. When running -application containers lack that label, the script builds the replacement, stops -and verifies all old API and streamer containers, then prepares a full claim +Use `scripts/deploy.sh` when upgrading recipient alert suppression. The new image +has the `science.4open.owner-alert-protocol=recipient-v2` label. When running +application containers have an older or missing protocol label, the script builds +the replacement, stops and verifies all old API and streamer containers, then prepares a full claim reconciliation and starts the new streamers and API. Stopped legacy containers from a rollback also trigger reconciliation, so claims written after an earlier migration are included. This path also runs when retrying with an already-built @@ -174,7 +178,7 @@ unsent pending claims and clears their matching legacy fields before backfill. This upgrade pauses traffic while claims migrate and allows up to 15 minutes for startup, configurable with `MIGRATION_HEALTH_TIMEOUT`. Subsequent deployments keep the usual rolling streamer replacement. Manual upgrades must -stop all legacy application writers, run +stop all application writers, run `docker compose run --rm --no-deps --entrypoint node anonymous_github ./build/scripts/reconcile-legacy-owner-alerts.js` using the new image, and then start the new application instances. Run this preparation after legacy writers have operated, even if their containers were diff --git a/scripts/deploy.sh b/scripts/deploy.sh index 8c5cc2a..dab0c01 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -37,15 +37,15 @@ list_streamers() { } # Capture application writers, including stopped containers from a rollback. -# Legacy instances do not participate in recipient claims, so they must stop -# before a new instance marks the one-time claim migration complete. +# Older instances can release recipient guards, so they must stop before +# a new instance marks permanent-suppression migration complete. application_ids="$(docker compose ps --all -q anonymous_github streamer)" apps=() if [[ -n $application_ids ]]; then mapfile -t apps <<<"$application_ids"; fi legacy_alerts=0 for app in "${apps[@]}"; do protocol="$(docker inspect -f '{{index .Config.Labels "science.4open.owner-alert-protocol"}}' "$app")" - [[ $protocol == recipient-v1 ]] || legacy_alerts=1 + [[ $protocol == recipient-v2 ]] || legacy_alerts=1 done # 1. Build @@ -56,7 +56,7 @@ after="$(image_id "$IMAGE")" if [[ $legacy_alerts == 1 ]]; then protocol="$(docker image inspect -f '{{index .Config.Labels "science.4open.owner-alert-protocol"}}' "$IMAGE")" - [[ $protocol == recipient-v1 ]] || { warn "replacement image lacks recipient alert migration support"; exit 1; } + [[ $protocol == recipient-v2 ]] || { warn "replacement image lacks recipient alert migration support"; exit 1; } TIMEOUT="${MIGRATION_HEALTH_TIMEOUT:-900}" log "Pausing legacy application writers before recipient alert migration" docker compose stop -t 30 anonymous_github streamer diff --git a/src/core/model/anonymizedGists/anonymizedGists.schema.ts b/src/core/model/anonymizedGists/anonymizedGists.schema.ts index 920f57c..9e9f32d 100644 --- a/src/core/model/anonymizedGists/anonymizedGists.schema.ts +++ b/src/core/model/anonymizedGists/anonymizedGists.schema.ts @@ -1,3 +1,4 @@ +import { ownerAlertLifecycle } from "../owner-alert-lifecycle.schema"; import { Schema } from "mongoose"; const AnonymizedGistSchema = new Schema({ @@ -74,4 +75,6 @@ const AnonymizedGistSchema = new Schema({ AnonymizedGistSchema.index({ status: 1, "options.expirationDate": 1 }); +AnonymizedGistSchema.plugin(ownerAlertLifecycle); + export default AnonymizedGistSchema; diff --git a/src/core/model/anonymizedPullRequests/anonymizedPullRequests.schema.ts b/src/core/model/anonymizedPullRequests/anonymizedPullRequests.schema.ts index ff45881..ac29a23 100644 --- a/src/core/model/anonymizedPullRequests/anonymizedPullRequests.schema.ts +++ b/src/core/model/anonymizedPullRequests/anonymizedPullRequests.schema.ts @@ -1,3 +1,4 @@ +import { ownerAlertLifecycle } from "../owner-alert-lifecycle.schema"; import { repositoryAccessSchema } from "../repository-access.schema"; import { Schema } from "mongoose"; @@ -71,4 +72,6 @@ const AnonymizedPullRequestSchema = new Schema({ AnonymizedPullRequestSchema.index({ status: 1, "options.expirationDate": 1 }); +AnonymizedPullRequestSchema.plugin(ownerAlertLifecycle); + export default AnonymizedPullRequestSchema; diff --git a/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts b/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts index ff8e19b..23f197b 100644 --- a/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts +++ b/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts @@ -1,3 +1,4 @@ +import { ownerAlertLifecycle } from "../owner-alert-lifecycle.schema"; import { repositoryAccessSchema } from "../repository-access.schema"; import { Schema } from "mongoose"; @@ -123,4 +124,6 @@ AnonymizedRepositorySchema.index({ "options.expirationDate": 1, }); +AnonymizedRepositorySchema.plugin(ownerAlertLifecycle); + export default AnonymizedRepositorySchema; diff --git a/src/core/model/owner-alert-lifecycle.schema.ts b/src/core/model/owner-alert-lifecycle.schema.ts new file mode 100644 index 0000000..57a9a42 --- /dev/null +++ b/src/core/model/owner-alert-lifecycle.schema.ts @@ -0,0 +1,54 @@ +import { randomUUID } from "crypto"; +import { Document, Query, Schema, Types, mongo } from "mongoose"; + +const inactive = new Set(["archived", "expired", "expiring", "removed", "removing"]); +type Owner = { _id: Types.ObjectId; accessAlertResetRevision?: number; accessAlertDeactivations?: string[] }; +type Fence = { users: mongo.Collection; owners: Types.ObjectId[]; token: string }; + +/** Lifecycle mutations and email dispatch compete on the owner document. */ +export function ownerAlertLifecycle(schema: Schema): void { + const fences = new WeakMap(); + const begin = async (operation: object, db: { collection(name: string): unknown }, owners: Types.ObjectId[]) => { + if (!owners.length) return; + const users = db.collection("users") as mongo.Collection; + const token = randomUUID(); + // Do not expire this reservation: an interrupted database mutation may still + // commit. Offline reconciliation can clear it after all writers have stopped. + await users.updateMany({ _id: { $in: owners } }, { + $inc: { accessAlertResetRevision: 1 }, $addToSet: { accessAlertDeactivations: token }, + }); + fences.set(operation, { users, owners, token }); + }; + const finish = async (operation: object) => { + const fence = fences.get(operation); + if (!fence) return; + await fence.users.updateMany({ _id: { $in: fence.owners } }, { $pull: { accessAlertDeactivations: fence.token } }); + fences.delete(operation); + }; + schema.pre(["updateOne", "updateMany", "findOneAndUpdate"], async function (this: Query) { + const update = this.getUpdate(); + if (!update) return; + const stages = (Array.isArray(update) ? update : [update]) as { $set?: { status?: unknown }; status?: unknown }[]; + if (!stages.some(stage => { const status = stage.$set?.status ?? stage.status; return typeof status === "string" && inactive.has(status); })) return; + const filter = this.getFilter(); + const lookup = this.model.distinct("owner", filter); + const collation = this.getOptions().collation; + if (collation) lookup.collation(collation); + const owners = (await lookup.exec()).filter(id => id && Types.ObjectId.isValid(id)); + await begin(this, this.model.db, owners); + // Preserve the owner snapshot: reassignment must not move this mutation to + // an account whose dispatch fence was not advanced. + this.setQuery({ $and: [filter, { $or: [{ owner: { $in: owners } }, { owner: null }] }] }); + }); + schema.post(["updateOne", "updateMany", "findOneAndUpdate"], async function () { await finish(this); }); + schema.pre("save", async function (this: Document & { status?: string; owner?: Types.ObjectId; $where?: Record }) { + if (!inactive.has(this.status || "") || !this.isModified("status")) return; + const current = this.isNew ? null : await this.collection.findOne({ _id: this._id }, { projection: { owner: 1 } }); + const owners = [current?.owner, this.owner].filter((id): id is Types.ObjectId => Boolean(id && Types.ObjectId.isValid(id))); + await begin(this, this.db, owners); + if (current) this.$where = { $and: [this.$where || {}, { owner: current.owner ?? null }] }; + }); + schema.post("save", async function () { await finish(this); }); + // Failed or ambiguous mutations retain their reservation. Releasing one here + // could permit dispatch while the resource write subsequently completes. +} diff --git a/src/core/model/users/users.schema.ts b/src/core/model/users/users.schema.ts index 27589a1..348597d 100644 --- a/src/core/model/users/users.schema.ts +++ b/src/core/model/users/users.schema.ts @@ -21,6 +21,7 @@ const UserSchema = new Schema({ accessAlertResetRevision: { type: Number, select: false }, accessAlertResetUntil: { type: Date, select: false }, accessAlertFenceRevision: { type: Number, select: false }, + accessAlertDeactivations: { type: [String], default: undefined, select: false }, accessAlertDispatches: { type: Schema.Types.Mixed, select: false }, emails: [ { diff --git a/src/core/owner-notifications.ts b/src/core/owner-notifications.ts index ed11b09..172dea5 100644 --- a/src/core/owner-notifications.ts +++ b/src/core/owner-notifications.ts @@ -64,9 +64,10 @@ type AlertRecord = { _id: string; state?: string; lease?: string; leaseUntil?: D generation?: string | null; attemptId?: string; claimedAt?: Date; completedAt?: Date; recipientKey?: string; resetRevision?: number; httpStatus?: number; startedAt?: Date; progress?: Record; legacyClaims?: LegacyClaim[]; - resourceRevision?: number; deferredRecoveries?: DeferredRecovery[]; dispatchPrepared?: boolean }; + resourceRevision?: number; deferredRecoveries?: DeferredRecovery[]; dispatchPrepared?: boolean; + deliveryProgress?: { completedAt: Date; id: string }; failureCode?: string }; const alertCollection = (name = RECIPIENT_ALERTS) => UserModel.db.collection(name) as unknown as mongo.Collection; -const MIGRATION = "recipient-alerts-v1"; +const MIGRATION = "recipient-alerts-v2"; const PENDING_LEASE_MS = 60000; const expiredPending = () => ({ state: "pending", claimedAt: { $lte: new Date(Date.now() - PENDING_LEASE_MS) } }); const expiredPreparation = () => ({ state: { $in: ["pending", "sending"] }, claimedAt: { $lte: new Date(Date.now() - PENDING_LEASE_MS) } }); @@ -82,6 +83,7 @@ async function prepareOwnerDispatch(record: AlertRecord, revision: number | null const path = `accessAlertDispatches.${record.attemptId}`; const prepared = await UserModel.db.collection("users").updateOne({ _id: record.owner, accessAlertResetRevision: generationFilter(revision), + "accessAlertDeactivations.0": { $exists: false }, $or: [{ accessAlertResetUntil: { $exists: false } }, { accessAlertResetUntil: { $lte: new Date() } }], }, [{ $set: { accessAlertDispatches: { $arrayToObject: { $filter: { input: { $objectToArray: { $ifNull: ["$accessAlertDispatches", {}] } }, as: "dispatch", @@ -101,6 +103,7 @@ async function commitOwnerRevision(owner: Types.ObjectId, revision: number | nul const path = `accessAlertDispatches.${record?.attemptId}`; const committed = await UserModel.db.collection("users").updateOne({ _id: owner, accessAlertResetRevision: generationFilter(revision), + "accessAlertDeactivations.0": { $exists: false }, ...(record ? { [`${path}.state`]: "prepared" } : {}), $or: [{ accessAlertResetUntil: { $exists: false } }, { accessAlertResetUntil: { $lte: new Date() } }], }, record ? { $set: { [`${path}.state`]: "committed" } } : { $inc: { accessAlertFenceRevision: 1 } }); @@ -152,11 +155,14 @@ async function reconcileUnsentAlerts(writersStopped = false): Promise { } } -/** Call only after all legacy application writers have stopped. */ +/** Call only after all application writers have stopped. */ export async function prepareLegacyOwnerAlertMigration(): Promise { await reconcileUnsentAlerts(true); + // Interrupted lifecycle mutations cannot resume once every writer is stopped. + await UserModel.db.collection("users").updateMany({ accessAlertDeactivations: { $exists: true } }, + { $unset: { accessAlertDeactivations: "" }, $inc: { accessAlertResetRevision: 1 } }); await alertCollection().updateOne({ _id: MIGRATION }, { $set: { state: "new" }, - $unset: { progress: "", lease: "", leaseUntil: "" } }, { upsert: true }); + $unset: { progress: "", deliveryProgress: "", lease: "", leaseUntil: "" } }, { upsert: true }); } async function reconcileCompletedSend(record: AlertRecord): Promise { @@ -219,6 +225,43 @@ export async function initializeOwnerAccessAlerts(): Promise { .catch(error => { heartbeatError = error; }); }, 5000); try { + // Delivery evidence survives v1 guard recovery and resource deletion. Restore + // missing guards once, before traffic, so those addresses cannot be emailed again. + const events = alertCollection("owner_access_alert_events"); + const index = "owner_access_alert_delivery_history"; + await events.createIndex({ completedAt: 1, _id: 1 }, { name: index, + partialFilterExpression: { completedAt: { $exists: true } } }); + const checkpoint = (await alerts.findOne({ _id: MIGRATION, lease }))?.deliveryProgress; + const filter = { state: { $in: ["accepted", "failed"] }, completedAt: { $exists: true }, + ...(checkpoint ? { $or: [{ completedAt: { $gt: checkpoint.completedAt } }, + { completedAt: checkpoint.completedAt, _id: { $gt: checkpoint.id } }] } : {}) }; + let lastDelivery: { completedAt: Date; id: string } | undefined; + let delivered = 0; + const saveDeliveries = async () => { + if (!lastDelivery) return; + const saved = await alerts.updateOne({ _id: MIGRATION, lease }, { $set: { deliveryProgress: lastDelivery } }); + if (!saved.matchedCount) throw new Error("owner_alert_migration_lease_lost"); + }; + for await (const event of events.find(filter).sort({ completedAt: 1, _id: 1 }).hint(index).batchSize(100)) { + if (heartbeatError) throw heartbeatError; + if (event.recipientKey && /^[0-9a-f]{64}$/.test(event.recipientKey)) { + const previous = await alerts.findOne({ _id: event.recipientKey }); + if (previous && (["pending", "cancelled"].includes(previous.state || "") + || (previous.state === "sending" && previous.dispatchPrepared))) { + // A v1 repeat may still be preparing after its earlier guard was + // released. Cancel known-unsent work even while its lease is fresh; + // committed or ambiguous dispatches remain blocked instead. + await releaseUnsentAlert(previous, { state: previous.state }); + } + await alerts.updateOne({ _id: event.recipientKey }, { $setOnInsert: { + state: "recipient_notified", attemptId: event.attemptId ?? randomUUID(), + claimedAt: event.completedAt, completedAt: event.completedAt, httpStatus: event.httpStatus, + } }, { upsert: true }); + } + lastDelivery = { completedAt: event.completedAt!, id: event._id }; + if (++delivered % 100 === 0) await saveDeliveries(); + } + await saveDeliveries(); for (const [kind, name] of Object.entries(resourceCollections)) { const resources = UserModel.db.collection(name); const index = "owner_access_alert_backfill"; @@ -293,7 +336,8 @@ async function completeRecovery(candidate: AlertRecord, recovery: DeferredRecove } else if (candidate.state === "pending") { await releaseUnsentAlert(candidate, { ...fence, state: "pending" }); } else { - await alertCollection().deleteOne(fence); + // Recovery does not authorize another generic email to the same address. + await alertCollection().updateOne(fence, { $set: { state: "recovered" } }); } } @@ -337,7 +381,7 @@ async function applyDeferredRecoveries(key: string, attemptId?: string): Promise } } -/** Only a fresh upstream read after owner action may reopen a recipient alert. */ +/** Confirm recovery after owner action; only known-unsent claims may be released. */ export async function withOwnerAccessRecovery(ownerId: string, resource: AlertResource, work: () => Promise, expectedSource: Record = {}, confirmed: (result: T) => boolean = () => true): Promise { let snapshot: { accessAlertGeneration?: string } | null = null; let candidates: AlertRecord[] = []; @@ -436,10 +480,11 @@ export async function notifyOwnerAccessProblem(ownerId: string, error: unknown, try { if (!resource || !Types.ObjectId.isValid(resource.id) || !Types.ObjectId.isValid(ownerId)) return; const collection = UserModel.db.collection(resourceCollections[resource.kind]); - const identity = { _id: new Types.ObjectId(resource.id), owner: new Types.ObjectId(ownerId) }; + const identity = { _id: new Types.ObjectId(resource.id), owner: new Types.ObjectId(ownerId), + status: { $nin: ["archived", "expired", "expiring", "removed", "removing"] } }; const ownerState = await UserModel.db.collection("users").findOne({ _id: identity.owner }, - { projection: { accessAlertResetRevision: 1, accessAlertResetUntil: 1 } }); - if (!ownerState || ownerState.accessAlertResetUntil?.getTime() > Date.now()) return; + { projection: { accessAlertResetRevision: 1, accessAlertResetUntil: 1, accessAlertDeactivations: 1 } }); + if (!ownerState || ownerState.accessAlertDeactivations?.length || ownerState.accessAlertResetUntil?.getTime() > Date.now()) return; const ownerRevision = ownerState.accessAlertResetRevision ?? null; const snapshot = await collection.findOne(identity, { projection: { accessAlertGeneration: 1, accessAlertClaimedAt: 1, accessAlertClaimRevision: 1 } }); if (!snapshot) return; @@ -486,16 +531,9 @@ export async function notifyOwnerAccessProblem(ownerId: string, error: unknown, } retired = await releaseUnsentAlert(previous, previous.state === "cancelled" ? retirement : { ...retirement, ...expiredPending() }); } else { - const claims = previous.state === "recovered" ? [] : previous.legacyClaims?.filter(claim => !claim.recovered) ?? [previous as LegacyClaim]; - for (const claim of claims) { - const previousResource = await UserModel.db.collection(resourceCollections[claim.resourceKind]).findOne( - { _id: claim.resourceId, owner: claim.owner }, { projection: { status: 1 } }); - const previousOwner = await UserModel.findById(claim.owner).select("status").lean(); - if (previousOwner && !["banned", "removed"].includes(previousOwner.status || "") - && previousResource && !["removed", "expired"].includes(previousResource.status)) return; - } - if (previous.legacyClaims) retirement.legacyClaims = previous.legacyClaims; - retired = Boolean((await alerts.deleteOne(retirement)).deletedCount); + // Delivered, legacy, recovered and ambiguous sends stay blocked permanently, + // even if their original resource or account no longer exists. + return; } if (!retired) return; } @@ -516,7 +554,10 @@ export async function notifyOwnerAccessProblem(ownerId: string, error: unknown, const base = /^https?:\/\//.test(config.APP_HOSTNAME) ? config.APP_HOSTNAME : `https://${config.APP_HOSTNAME}`; const url = new URL("/connections", base).href; await alertCollection("owner_access_alert_events").insertOne({ _id: attemptId, attemptId, - recipientKey: key, state: "prepared", startedAt: new Date() }); + recipientKey: key, state: "prepared", startedAt: new Date(), owner: identity.owner, + resourceKind: resource.kind, resourceId: identity._id, generation: record.generation, + failureCode: accessErrors.has(typeof error === "string" ? error : error instanceof Error ? error.message : "") + ? typeof error === "string" ? error : (error as Error).message : "upstream_access_failure" }); const claim = await collection.updateOne({ ...identity, accessAlertGeneration: generationFilter(record.generation), accessAlertClaimRevision: revisionFilter(record.resourceRevision ?? 0), @@ -557,7 +598,8 @@ export async function notifyOwnerAccessProblem(ownerId: string, error: unknown, try { await retryAlertWrite(async () => { try { await alertCollection("owner_access_alert_events").insertOne({ _id: `${attemptId}/completion`, attemptId, - recipientKey: key, state, httpStatus, completedAt }); } + recipientKey: key, state, httpStatus, completedAt, owner: identity.owner, + resourceKind: resource.kind, resourceId: identity._id, generation: record.generation }); } catch (error) { if ((error as { code?: number }).code !== 11000) throw error; } }); } catch { logger.warn("owner access email audit unavailable", { attemptId }); } diff --git a/test/deploy-owner-alert-migration.test.js b/test/deploy-owner-alert-migration.test.js index 9a737f5..9274526 100644 --- a/test/deploy-owner-alert-migration.test.js +++ b/test/deploy-owner-alert-migration.test.js @@ -18,11 +18,11 @@ const args = process.argv.slice(2), state = JSON.parse(fs.readFileSync(process.e fs.appendFileSync(process.env.TASK_DOCKER_LOG, JSON.stringify(args) + "\\n"); const finish = (output = "", code = 0) => { fs.writeFileSync(process.env.TASK_DOCKER_STATE, JSON.stringify(state)); process.stdout.write(output); process.exit(code); }; if (args[0] === "image" && args[1] === "inspect") { - if (args.some(a => a.includes("Labels"))) finish(state.badProtocol ? "" : "recipient-v1"); + if (args.some(a => a.includes("Labels"))) finish(state.badProtocol ? "" : "recipient-v2"); finish(state.built ? "sha:new" : "sha:old"); } if (args[0] === "inspect") { - if (args.some(a => a.includes("Labels"))) finish(""); + if (args.some(a => a.includes("Labels"))) finish(state.oldProtocol || ""); if (args.includes("{{.State.Running}}")) finish(state.stopped && !state.refuseStop ? "false" : "true"); finish("healthy"); } @@ -68,6 +68,14 @@ finish("unexpected docker call", 2); expect(result.calls.slice(up + 1, api).some(args => args[0] === "inspect" && args.at(-1) === name)).to.equal(true); } }); + it("stops recipient-v1 writers before restoring permanent recipient suppression", () => { + const result = deploy({ oldProtocol: "recipient-v1" }); + expect(result.status, result.stderr).to.equal(0); + const stop = result.calls.findIndex(args => args[0] === "compose" && args[1] === "stop"); + const reconcile = result.calls.findIndex(args => args[0] === "compose" && args[1] === "run"); + const up = result.calls.findIndex(args => args[0] === "compose" && args[1] === "up"); + expect(stop).to.be.greaterThan(-1); expect(reconcile).to.be.greaterThan(stop); expect(up).to.be.greaterThan(reconcile); + }); it("aborts before migration starts if a legacy writer remains running", () => { const result = deploy({ refuseStop: true }); expect(result.status).not.to.equal(0); diff --git a/test/owner-notifications.test.js b/test/owner-notifications.test.js index 2c42bc4..0343bee 100644 --- a/test/owner-notifications.test.js +++ b/test/owner-notifications.test.js @@ -154,7 +154,7 @@ describe("owner access notifications", () => { expect(events[1].state).to.equal("accepted"); }); - it("reopens an alert only after owner action and successful access to its triggering resource", async () => { + it("keeps the recipient suppressed after owner action and successful access", async () => { await notifyOwnerAccessProblem(ownerId, "token_expired", resource); await notifyOwnerAccessProblem(ownerId, "token_expired", other); expect(sent).to.have.length(1); @@ -165,8 +165,9 @@ describe("owner access notifications", () => { expect(recipients.size).to.equal(1); await notifications.withOwnerAccessRecovery(ownerId, resource, async () => {}); await notifyOwnerAccessProblem(ownerId, "token_expired", resource); - expect(sent).to.have.length(2); - expect(events).to.have.length(4); + expect(sent).to.have.length(1); + expect([...recipients.values()][0].state).to.equal("recovered"); + expect(events).to.have.length(2); }); it("cannot clear another owner's recipient guard", async () => { @@ -217,7 +218,7 @@ describe("owner access notifications", () => { expect(sent).to.have.length(1); }); - for (const failure of ["rejected", "timeout"]) it(`retains the claim after ${failure} until confirmed recovery`, async () => { + for (const failure of ["rejected", "timeout"]) it(`retains the claim after ${failure} including confirmed recovery`, async () => { let attempts = 0; stub(global, "fetch", async () => { attempts++; if (failure === "timeout") throw new Error("timeout"); return { ok: false }; }); await notifyOwnerAccessProblem(ownerId, "token_expired", resource); @@ -228,7 +229,7 @@ describe("owner access notifications", () => { expect(attempts).to.equal(1); await notifications.withOwnerAccessRecovery(ownerId, resource, async () => {}); await notifyOwnerAccessProblem(ownerId, "token_expired", resource); - expect(attempts).to.equal(2); + expect(attempts).to.equal(1); }); it("does not claim an alert without configuration, an active owner, and an address", async () => { diff --git a/test/recipient-alerts-mongo.test.js b/test/recipient-alerts-mongo.test.js index 242579e..0e9056b 100644 --- a/test/recipient-alerts-mongo.test.js +++ b/test/recipient-alerts-mongo.test.js @@ -14,7 +14,7 @@ describeMongo("recipient access alerts in MongoDB", function () { const guards = () => mongoose.connection.collection(alerts.RECIPIENT_ALERTS); const recipient = () => alerts.recipientAlertKey("owner@example.com"); async function create(ownerId, kind = "repository") { - const id = new mongoose.Types.ObjectId(); await coll(kind).insertOne({ _id: id, owner: ownerId, repoId: String(id), status: "ready" }); return { kind, id: String(id) }; + const id = new mongoose.Types.ObjectId(); await coll(kind).insertOne({ _id: id, owner: ownerId, [{ repository: "repoId", gist: "gistId", "pull-request": "pullRequestId" }[kind]]: String(id), status: "ready" }); return { kind, id: String(id) }; } before(async () => { nativeFetch = global.fetch; originalConfig = { ...config }; @@ -35,6 +35,189 @@ describeMongo("recipient access alerts in MongoDB", function () { expect(sent).to.have.length(1); expect(await guards().countDocuments({ _id: recipient() })).to.equal(1); expect(await mongoose.connection.collection("owner_access_alert_events").countDocuments({ state: "accepted" })).to.equal(1); }); + for (const kind of ["repository", "gist", "pull-request"]) { + for (const status of ["archived", "expired", "expiring", "removed", "removing"]) it(`does not notify for ${status} ${kind}`, async () => { + const inactive = await create(owner._id, kind); + await coll(kind).updateOne({ _id: new mongoose.Types.ObjectId(inactive.id) }, { $set: { status } }); + await notify(owner.id, "token_expired", inactive); + expect(sent).to.have.length(0); expect(await guards().findOne({ _id: recipient() })).to.equal(null); + expect(await mongoose.connection.collection("owner_access_alert_events").countDocuments()).to.equal(0); + }); + } + it("cancels a send when the source is removed during audit preparation", async () => { + const events = mongoose.connection.collection("owner_access_alert_events"), insert = events.insertOne; + let entered, release; const gate = new Promise(resolve => { entered = resolve; }); + events.insertOne = async function (...args) { entered(); await new Promise(resolve => { release = resolve; }); return insert.apply(this, args); }; + const pending = notify(owner.id, "token_expired", resource); + try { + await gate; + await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { status: "removed" } }); + } finally { events.insertOne = insert; release(); await pending; } + expect(sent).to.have.length(0); expect(await guards().findOne({ _id: recipient() })).to.equal(null); + expect((await coll(resource.kind).findOne({ _id: new mongoose.Types.ObjectId(resource.id) })).accessAlertClaimedAt).to.equal(undefined); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); + }); + it("records the triggering resource and categorized failure without its address or source name", async () => { + await notify(owner.id, "token_expired", resource); + const records = await mongoose.connection.collection("owner_access_alert_events").find().toArray(); + expect(records).to.have.length(2); + for (const record of records) { + expect(String(record.owner)).to.equal(owner.id); expect(String(record.resourceId)).to.equal(resource.id); + expect(record.resourceKind).to.equal(resource.kind); expect(record.generation).to.equal(null); + expect(JSON.stringify(record)).not.to.include("owner@example.com"); + } + expect(records.find(record => record.state === "prepared").failureCode).to.equal("token_expired"); + }); + for (const state of ["accepted", "failed"]) it(`restores a missing guard from ${state} provider history before serving traffic`, async () => { + await mongoose.connection.collection("owner_access_alert_events").insertOne({ _id: "old/completion", attemptId: "old", + recipientKey: recipient(), state, completedAt: new Date(1) }); + await Promise.all(Array.from({ length: 5 }, () => alerts.initializeOwnerAccessAlerts())); + expect((await guards().findOne({ _id: recipient() })).state).to.equal("recipient_notified"); + await notify(owner.id, "token_expired", resource); await notify(otherOwner.id, "token_expired", other); + await reset(owner.id, resource); await recover(owner.id, resource, async () => {}); + await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(0); + await alerts.initializeOwnerAccessAlerts(); expect(sent).to.have.length(0); + }); + it("does not restore a delivery guard from an uncompleted preparation event", async () => { + await mongoose.connection.collection("owner_access_alert_events").insertOne({ _id: "prepared", recipientKey: recipient(), state: "prepared", startedAt: new Date(1) }); + await alerts.initializeOwnerAccessAlerts(); expect(await guards().findOne({ _id: recipient() })).to.equal(null); + await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(1); + }); + it("resumes provider history restoration after a checkpointed startup failure", async () => { + const events = mongoose.connection.collection("owner_access_alert_events"); + const keys = Array.from({ length: 105 }, (_, i) => alerts.recipientAlertKey(`old${i}@example.com`)); + await events.insertMany(keys.map((recipientKey, i) => ({ _id: String(i).padStart(3, "0") + "/completion", + recipientKey, attemptId: String(i), state: "accepted", completedAt: new Date(1) }))); + const collection = guards(), update = collection.updateOne; let restored = 0; + collection.updateOne = async function (filter, operation, ...args) { + if (operation.$setOnInsert?.state === "recipient_notified" && ++restored === 103) throw new Error("interrupted history backfill"); + return update.call(this, filter, operation, ...args); + }; + try { await alerts.initializeOwnerAccessAlerts(); throw new Error("Expected migration failure"); } + catch (error) { expect(error.message).to.equal("interrupted history backfill"); } + finally { collection.updateOne = update; } + expect((await guards().findOne({ _id: "recipient-alerts-v2" })).deliveryProgress.id).to.equal("099/completion"); + await guards().updateOne({ _id: "recipient-alerts-v2" }, { $set: { leaseUntil: new Date(0) } }); + await alerts.initializeOwnerAccessAlerts(); + expect(await guards().countDocuments({ state: "recipient_notified" })).to.equal(105); + expect((await guards().findOne({ _id: "recipient-alerts-v2" })).deliveryProgress.id).to.equal("104/completion"); + expect((await events.indexes()).some(index => index.name === "owner_access_alert_delivery_history" && index.partialFilterExpression)).to.equal(true); + expect(sent).to.have.length(0); + }); + it("runs the v2 history migration on a cold start with only a ready v1 marker", async () => { + await guards().insertOne({ _id: "recipient-alerts-v1", state: "ready" }); + await mongoose.connection.collection("owner_access_alert_events").insertOne({ _id: "v1/completion", attemptId: "v1", + recipientKey: recipient(), state: "accepted", completedAt: new Date(1) }); + await alerts.initializeOwnerAccessAlerts(); + expect((await guards().findOne({ _id: "recipient-alerts-v2" })).state).to.equal("ready"); + expect((await guards().findOne({ _id: recipient() })).state).to.equal("recipient_notified"); + await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(0); + }); + it("cancels a fresh unsent v1 repeat before restoring its delivery suppression", async () => { + const attemptId = "v1-unsent-repeat"; + await guards().insertMany([{ _id: "recipient-alerts-v1", state: "ready" }, { + _id: recipient(), owner: owner._id, resourceKind: resource.kind, resourceId: new mongoose.Types.ObjectId(resource.id), + attemptId, state: "pending", dispatchPrepared: true, resourceRevision: 0, claimedAt: new Date(), + }]); + await mongoose.connection.collection("users").updateOne({ _id: owner._id }, { $set: { + [`accessAlertDispatches.${attemptId}`]: { state: "prepared", claimedAt: new Date() }, + } }); + await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { + accessAlertClaimedAt: new Date(), accessAlertAttemptId: attemptId, accessAlertClaimRevision: 1, + } }); + await mongoose.connection.collection("owner_access_alert_events").insertOne({ _id: "v1-delivered/completion", attemptId: "v1-delivered", + recipientKey: recipient(), state: "accepted", completedAt: new Date(1) }); + await alerts.initializeOwnerAccessAlerts(); + expect((await guards().findOne({ _id: recipient() })).state).to.equal("recipient_notified"); + expect((await coll(resource.kind).findOne({ _id: new mongoose.Types.ObjectId(resource.id) })).accessAlertClaimedAt).to.equal(undefined); + expect((await mongoose.connection.collection("users").findOne({ _id: owner._id })).accessAlertDispatches[attemptId]).to.equal(undefined); + await alerts.initializeOwnerAccessAlerts(); await notify(owner.id, "token_expired", resource); + expect(sent).to.have.length(0); + }); + const models = () => ({ + repository: require("../src/core/model/anonymizedRepositories/anonymizedRepositories.model").default, + gist: require("../src/core/model/anonymizedGists/anonymizedGists.model").default, + "pull-request": require("../src/core/model/anonymizedPullRequests/anonymizedPullRequests.model").default, + }); + for (const kind of ["repository", "gist", "pull-request"]) { + it(`fences ${kind} deletion after the sender's last resource check`, async () => { + const source = await create(owner._id, kind); + const collection = guards(), update = collection.updateOne; + let entered, release; const gate = new Promise(resolve => { entered = resolve; }); + collection.updateOne = async function (filter, operation, ...args) { + if (operation.$set?.state === "sending") { entered(); await new Promise(resolve => { release = resolve; }); } + return update.call(this, filter, operation, ...args); + }; + const pending = notify(owner.id, "token_expired", source); + try { + await gate; + await models()[kind].updateOne({ _id: source.id }, { $set: { status: "removing" } }).exec(); + } finally { collection.updateOne = update; release(); await pending; } + expect(sent).to.have.length(0); expect(await guards().findOne({ _id: recipient() })).to.equal(null); + expect((await mongoose.connection.collection("users").findOne({ _id: owner._id })).accessAlertDeactivations).to.deep.equal([]); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); + }); + } + it("does not deactivate a resource reassigned after the lifecycle owner snapshot", async () => { + const Model = models().repository, users = mongoose.connection.collection("users"), update = users.updateMany; + let entered, release; const gate = new Promise(resolve => { entered = resolve; }); + users.updateMany = async function (filter, operation, ...args) { + if (operation.$addToSet?.accessAlertDeactivations) { entered(); await new Promise(resolve => { release = resolve; }); } + return update.call(this, filter, operation, ...args); + }; + const pending = Model.updateOne({ _id: resource.id }, { $set: { status: "removing" } }).exec(); + let result; + try { + await gate; + await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { owner: otherOwner._id } }); + } finally { users.updateMany = update; release(); result = await pending; } + expect(result.matchedCount).to.equal(0); + expect((await coll(resource.kind).findOne({ _id: new mongoose.Types.ObjectId(resource.id) })).status).to.equal("ready"); + await notify(otherOwner.id, "token_expired", resource); expect(sent).to.have.length(1); + }); + it("blocks new senders until all concurrent lifecycle mutations finish", async () => { + const second = await create(owner._id), active = await create(owner._id); + const Model = models().repository, collection = Model.collection, update = collection.updateOne; + const releases = new Map(); let entered; const gate = new Promise(resolve => { entered = resolve; }); + collection.updateOne = async function (filter, operation, ...args) { + if (operation.$set?.status === "removing") { + await new Promise(resolve => { releases.set(String(filter.$and[0]._id), resolve); if (releases.size === 2) entered(); }); + } + return update.call(this, filter, operation, ...args); + }; + const first = Model.updateOne({ _id: resource.id }, { $set: { status: "removing" } }).exec(); + const last = Model.updateOne({ _id: second.id }, { $set: { status: "removing" } }).exec(); + try { + await gate; + expect((await mongoose.connection.collection("users").findOne({ _id: owner._id })).accessAlertDeactivations).to.have.length(2); + await notify(owner.id, "token_expired", active); expect(sent).to.have.length(0); + releases.get(second.id)(); await last; + await notify(owner.id, "token_expired", active); expect(sent).to.have.length(0); + } finally { collection.updateOne = update; for (const release of releases.values()) release(); await Promise.all([first, last]); } + await notify(owner.id, "token_expired", active); expect(sent).to.have.length(1); + }); + it("retains an ambiguous lifecycle reservation until offline reconciliation", async () => { + const Model = models().repository, collection = Model.collection, update = collection.updateOne; + collection.updateOne = async () => { throw new Error("lost database acknowledgement"); }; + try { await Model.updateOne({ _id: resource.id }, { $set: { status: "removing" } }).exec(); throw new Error("Expected mutation failure"); } + catch (error) { expect(error.message).to.equal("lost database acknowledgement"); } + finally { collection.updateOne = update; } + await alerts.initializeOwnerAccessAlerts(); + await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(0); + await alerts.prepareLegacyOwnerAlertMigration(); await alerts.initializeOwnerAccessAlerts(); + await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(1); + }); + for (const operation of ["updateMany", "findOneAndUpdate", "save"]) it(`fences inactive lifecycle status via ${operation}`, async () => { + const Model = models().repository; + const before = await mongoose.connection.collection("users").findOne({ _id: owner._id }); + if (operation === "save") { + const doc = await Model.findById(resource.id); doc.status = "expired"; await doc.save(); + } else await Model[operation]({ _id: resource.id }, { $set: { status: "expired" } }).exec(); + const after = await mongoose.connection.collection("users").findOne({ _id: owner._id }); + expect(after.accessAlertResetRevision).to.equal((before.accessAlertResetRevision || 0) + 1); + expect(after.accessAlertDeactivations).to.deep.equal([]); + await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(0); + }); it("does not reopen after reconnect alone, unrelated resource success, or failed recovery", async () => { await notify(owner.id, "token_expired", resource); await reset(owner.id); await recover(otherOwner.id, other, async () => {}); @@ -42,14 +225,17 @@ describeMongo("recipient access alerts in MongoDB", function () { await notify(owner.id, "token_expired", resource); await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); }); - it("requires owner action before a successful upstream read can reopen the recipient", async () => { + it("keeps suppression after recovery but allows a first alert to a new address", async () => { await notify(owner.id, "token_expired", resource); - await recover(owner.id, resource, async () => {}); expect(await guards().findOne({ _id: recipient() })).not.to.equal(null); await reset(owner.id, resource); await recover(owner.id, resource, async () => {}); - expect(await guards().findOne({ _id: recipient() })).to.equal(null); + expect((await guards().findOne({ _id: recipient() })).state).to.equal("recovered"); + await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(1); + await Users.updateOne({ _id: owner._id }, { $set: { notificationEmail: "new@example.com" } }); await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(2); expect(sent[0].headers["Idempotency-Key"]).not.to.equal(sent[1].headers["Idempotency-Key"]); expect(await mongoose.connection.collection("owner_access_alert_events").countDocuments()).to.equal(4); + await Users.updateOne({ _id: owner._id }, { $set: { notificationEmail: "OWNER@example.com" } }); + await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(2); }); it("does not release a pending send when recovery races with provider delivery", async () => { let release, entered; const gate = new Promise(resolve => { entered = resolve; }); @@ -77,14 +263,17 @@ describeMongo("recipient access alerts in MongoDB", function () { }); it("fences a second owner action that races after recovery's final resource check", async () => { await notify(owner.id, "token_expired", resource); await reset(owner.id, resource); - const collection = guards(), remove = collection.deleteOne; + const collection = guards(), update = collection.updateOne; let entered, release; const gate = new Promise(resolve => { entered = resolve; }); - collection.deleteOne = async function (...args) { entered(); await new Promise(resolve => { release = resolve; }); return remove.apply(this, args); }; + collection.updateOne = async function (filter, operation, ...args) { + if (operation.$set?.state === "recovered") { entered(); await new Promise(resolve => { release = resolve; }); } + return update.call(this, filter, operation, ...args); + }; try { const pending = recover(owner.id, resource, async () => {}); await gate; await reset(owner.id, resource); release(); await pending; - expect(await guards().findOne({ _id: recipient() })).not.to.equal(null); - } finally { collection.deleteOne = remove; } + expect((await guards().findOne({ _id: recipient() })).state).to.equal("accepted"); + } finally { collection.updateOne = update; } }); it("does not accept a successful read of a stale source binding", async () => { await notify(owner.id, "token_expired", resource); await reset(owner.id, resource); @@ -98,11 +287,11 @@ describeMongo("recipient access alerts in MongoDB", function () { await notify(otherOwner.id, "repo_not_found", other); expect(sent).to.have.length(1); expect((await guards().findOne({ _id: recipient() })).state).to.equal("failed"); }); - it("releases a guard for a removed resource without erasing delivery history", async () => { + it("keeps a removed resource's recipient guard and delivery history", async () => { await notify(owner.id, "token_expired", resource); await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { status: "removed" } }); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(2); - expect(await mongoose.connection.collection("owner_access_alert_events").countDocuments()).to.equal(4); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); + expect(await mongoose.connection.collection("owner_access_alert_events").countDocuments()).to.equal(2); }); for (const step of ["resource update", "resource verification", "audit insertion", "dispatch transition"]) it(`releases an unsent guard after ${step} fails`, async () => { const collection = step.startsWith("resource") ? coll(resource.kind) : step === "audit insertion" ? mongoose.connection.collection("owner_access_alert_events") : guards(); @@ -165,7 +354,7 @@ describeMongo("recipient access alerts in MongoDB", function () { expect((await guards().findOne({ _id: recipient() })).state).to.equal("sending"); }); it("reaps only expired unsent claims during startup and never reseeds them", async () => { - await guards().insertOne({ _id: "recipient-alerts-v1", state: "ready" }); + await guards().insertOne({ _id: "recipient-alerts-v2", state: "ready" }); await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { accessAlertClaimedAt: new Date(0), accessAlertAttemptId: recipient() } }); for (const [key, state, date] of [[recipient(), "pending", new Date(0)], ["recent", "pending", new Date()], ["sending", "sending", new Date(0)], ["accepted", "accepted", new Date(0)], ["failed", "failed", new Date(0)]]) { @@ -226,7 +415,7 @@ describeMongo("recipient access alerts in MongoDB", function () { await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(1); }); for (const changed of ["generation", "attempt"]) it(`preserves newer legacy fields when the ${changed} no longer matches the cancelled attempt`, async () => { - await guards().insertOne({ _id: "recipient-alerts-v1", state: "ready" }); + await guards().insertOne({ _id: "recipient-alerts-v2", state: "ready" }); await guards().insertOne({ _id: recipient(), owner: owner._id, resourceKind: resource.kind, resourceId: new mongoose.Types.ObjectId(resource.id), generation: "old-generation", attemptId: "old-attempt", state: "pending", claimedAt: new Date(0) }); const generation = changed === "generation" ? "new-generation" : "old-generation"; @@ -256,7 +445,7 @@ describeMongo("recipient access alerts in MongoDB", function () { await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(1); }); it("cleans recent pending legacy fields once deployment has stopped every writer", async () => { - await guards().insertOne({ _id: "recipient-alerts-v1", state: "ready" }); + await guards().insertOne({ _id: "recipient-alerts-v2", state: "ready" }); await guards().insertOne({ _id: recipient(), owner: owner._id, resourceKind: resource.kind, resourceId: new mongoose.Types.ObjectId(resource.id), attemptId: "recent-unsent", state: "pending", claimedAt: new Date() }); await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { @@ -267,7 +456,7 @@ describeMongo("recipient access alerts in MongoDB", function () { expect((await coll(resource.kind).findOne({ _id: new mongoose.Types.ObjectId(resource.id) })).accessAlertClaimedAt).to.equal(undefined); }); it("preserves an untagged legacy delivery written during rollback while cancelling a native pending attempt", async () => { - await guards().insertOne({ _id: "recipient-alerts-v1", state: "ready" }); + await guards().insertOne({ _id: "recipient-alerts-v2", state: "ready" }); await guards().insertOne({ _id: recipient(), owner: owner._id, resourceKind: resource.kind, resourceId: new mongoose.Types.ObjectId(resource.id), attemptId: "native-unsent", state: "pending", claimedAt: new Date() }); await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { accessAlertClaimedAt: new Date() } }); @@ -275,7 +464,7 @@ describeMongo("recipient access alerts in MongoDB", function () { expect((await guards().findOne({ _id: recipient() })).state).to.equal("legacy"); await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(0); await reset(owner.id, resource); await recover(owner.id, resource, async () => {}); - await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(1); + await notify(owner.id, "token_expired", resource); expect(sent).to.have.length(0); }); it("blocks dispatch while an owner reset has advanced the guard revision but not the resource generation", async () => { const events = mongoose.connection.collection("owner_access_alert_events"), insert = events.insertOne; @@ -363,8 +552,8 @@ describeMongo("recipient access alerts in MongoDB", function () { expect(record.deferredRecoveries).to.have.length(1); await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); } finally { release(); await pending; } - expect(await guards().findOne({ _id: recipient() })).to.equal(null); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(2); + expect((await guards().findOne({ _id: recipient() })).state).to.equal("recovered"); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); }); it("invalidates in-flight recovery evidence after a newer reset", async () => { let entered, release; const gate = new Promise(resolve => { entered = resolve; }); @@ -379,7 +568,7 @@ describeMongo("recipient access alerts in MongoDB", function () { expect((await guards().findOne({ _id: recipient() })).state).to.equal("accepted"); await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); await recover(owner.id, resource, async () => {}); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(2); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); }); it("invalidates deferred recovery if its source binding changes before delivery completes", async () => { await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { source: { repositoryName: "owner/repo" } } }); @@ -396,7 +585,7 @@ describeMongo("recipient access alerts in MongoDB", function () { } finally { release(); await pending; } expect((await guards().findOne({ _id: recipient() })).state).to.equal("accepted"); await recover(owner.id, resource, async () => {}, { "source.repositoryName": "new/repo" }); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(2); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); }); it("rejects dispatch when reset starts after the guard reservation but before the owner commitment", async () => { const collection = guards(), update = collection.updateOne, updateMany = collection.updateMany; @@ -433,21 +622,21 @@ describeMongo("recipient access alerts in MongoDB", function () { await gate; await recover(owner.id, resource, async () => {}); expect((await guards().findOne({ _id: recipient() })).deferredRecoveries).to.have.length(1); } finally { users.updateOne = update; release(); await action; } - expect(await guards().findOne({ _id: recipient() })).to.equal(null); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(2); + expect((await guards().findOne({ _id: recipient() })).state).to.equal("recovered"); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); }); it("retries saved recovery after a crashed reset lease expires", async () => { await notify(owner.id, "token_expired", resource); await reset(owner.id, resource); await mongoose.connection.collection("users").updateOne({ _id: owner._id }, { $set: { accessAlertResetUntil: new Date(Date.now() + 150) } }); await recover(owner.id, resource, async () => {}); expect(await guards().findOne({ _id: recipient() })).not.to.equal(null); - for (let i = 0; i < 100 && await guards().findOne({ _id: recipient() }); i++) await new Promise(resolve => global.setTimeout(resolve, 20)); - expect(await guards().findOne({ _id: recipient() })).to.equal(null); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(2); + for (let i = 0; i < 100 && (await guards().findOne({ _id: recipient() }))?.state !== "recovered"; i++) await new Promise(resolve => global.setTimeout(resolve, 20)); + expect((await guards().findOne({ _id: recipient() })).state).to.equal("recovered"); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); }); for (const state of ["prepared", "cancelled", "committed"]) it(`reconciles interrupted guard reservation with a ${state} owner dispatch record`, async () => { const attemptId = `interrupted-${state}`; - await guards().insertMany([{ _id: "recipient-alerts-v1", state: "ready" }, { + await guards().insertMany([{ _id: "recipient-alerts-v2", state: "ready" }, { _id: recipient(), owner: owner._id, resourceKind: resource.kind, resourceId: new mongoose.Types.ObjectId(resource.id), attemptId, state: "sending", dispatchPrepared: true, resourceRevision: 0, claimedAt: new Date(0), }]); @@ -477,22 +666,22 @@ describeMongo("recipient access alerts in MongoDB", function () { const account = await mongoose.connection.collection("users").findOne({ _id: owner._id }); expect(Object.keys(account.accessAlertDispatches)).to.deep.equal(["unknown"]); }); - it("allows active accounts to receive alerts when a shared-address guard belongs to a banned owner", async () => { + it("keeps shared-address suppression when the original owner is banned", async () => { await notify(owner.id, "token_expired", resource); await Users.updateOne({ _id: owner._id }, { $set: { status: "banned" } }); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(2); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); }); it("resumes an indexed migration from its last completed checkpoint", async () => { await Users.updateOne({ _id: otherOwner._id }, { $set: { notificationEmail: "second@example.com" } }); const second = await create(otherOwner._id); await coll("repository").updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { accessAlertClaimedAt: new Date(1) } }); await coll("repository").updateOne({ _id: new mongoose.Types.ObjectId(second.id) }, { $set: { accessAlertClaimedAt: new Date(2) } }); - await guards().insertOne({ _id: "recipient-alerts-v1", state: "new", leaseUntil: new Date(0), + await guards().insertOne({ _id: "recipient-alerts-v2", state: "new", leaseUntil: new Date(0), progress: { repository: { claimedAt: new Date(1), id: new mongoose.Types.ObjectId(resource.id) } } }); await alerts.initializeOwnerAccessAlerts(); expect(await guards().findOne({ _id: recipient() })).to.equal(null); expect(await guards().findOne({ _id: alerts.recipientAlertKey("second@example.com") })).not.to.equal(null); - expect((await guards().findOne({ _id: "recipient-alerts-v1" })).progress.repository.claimedAt.getTime()).to.equal(2); + expect((await guards().findOne({ _id: "recipient-alerts-v2" })).progress.repository.claimedAt.getTime()).to.equal(2); expect((await coll("repository").indexes()).some(i => i.name === "owner_access_alert_backfill" && i.partialFilterExpression)).to.equal(true); }); it("confirms recovery on a fresh uncached file read but never on a cache hit", async () => { @@ -512,9 +701,11 @@ describeMongo("recipient access alerts in MongoDB", function () { const source = new GitHubStream({ repoId: resource.id, organization: "owner", repoName: "repo", commit: "pinned", getToken: async () => "mock-token" }); source.downloadWithFallback = async () => Readable.from(["fresh-bytes"]); const read = () => source.getFileContentCache("a.txt", resource.id, () => ({ sha: "a".repeat(40), size: 11 })); - (await read()).destroy(); expect(await guards().findOne({ _id: recipient() })).to.equal(null); + (await read()).destroy(); expect((await guards().findOne({ _id: recipient() })).state).to.equal("recovered"); + const secondKey = alerts.recipientAlertKey("second@example.com"); + await Users.updateOne({ _id: owner._id }, { $set: { notificationEmail: "second@example.com" } }); await notify(owner.id, "token_expired", resource); await reset(owner.id, resource); - (await read()).destroy(); expect(await guards().findOne({ _id: recipient() })).not.to.equal(null); + (await read()).destroy(); expect((await guards().findOne({ _id: secondKey })).state).to.equal("accepted"); } finally { database.isConnected = originals.isConnected; storage.fileInfo = originals.fileInfo; storage.write = originals.write; storage.read = originals.read; } }); it("confirms recovery when a successful scheduled refresh finds the same commit", async () => { @@ -532,7 +723,7 @@ describeMongo("recipient access alerts in MongoDB", function () { try { await notify(owner.id, "token_expired", resource); await reset(owner.id, resource); await repo.updateIfNeeded({ force: false }); - expect(await guards().findOne({ _id: recipient() })).to.equal(null); + expect((await guards().findOne({ _id: recipient() })).state).to.equal("recovered"); } finally { github.getRepositoryFromGitHub = original; } }); it("seeds old claims once while five instances start concurrently", async () => { @@ -553,7 +744,7 @@ describeMongo("recipient access alerts in MongoDB", function () { await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(0); await reset(otherOwner.id, other); await recover(otherOwner.id, other, async () => {}); expect((await guards().findOne({ _id: recipient() })).state).to.equal("recovered"); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(0); }); it("does not retire a legacy group when only its first triggering resource is removed", async () => { await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { accessAlertClaimedAt: new Date(1) } }); @@ -562,9 +753,9 @@ describeMongo("recipient access alerts in MongoDB", function () { await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { status: "removed" } }); await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(0); await reset(otherOwner.id, other); await recover(otherOwner.id, other, async () => {}); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(0); }); - it("atomically releases the final legacy guard during concurrent recoveries", async () => { + it("atomically marks concurrent legacy recoveries without releasing suppression", async () => { await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { accessAlertClaimedAt: new Date(1) } }); await coll(other.kind).updateOne({ _id: new mongoose.Types.ObjectId(other.id) }, { $set: { accessAlertClaimedAt: new Date(2) } }); await alerts.initializeOwnerAccessAlerts(); await reset(owner.id, resource); await reset(otherOwner.id, other); @@ -580,7 +771,7 @@ describeMongo("recipient access alerts in MongoDB", function () { await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(0); expect((await guards().findOne({ _id: recipient() })).state).to.equal("legacy"); await recover(owner.id, resource, async () => {}); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(0); }); it("fences legacy claim mutation when reset races after the final resource check", async () => { await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { accessAlertClaimedAt: new Date(1) } }); @@ -603,11 +794,11 @@ describeMongo("recipient access alerts in MongoDB", function () { await alerts.initializeOwnerAccessAlerts(); expect((await guards().findOne({ _id: recipient() })).state).to.equal("recovered"); await coll(other.kind).updateOne({ _id: new mongoose.Types.ObjectId(other.id) }, { $set: { accessAlertClaimedAt: new Date() } }); await alerts.prepareLegacyOwnerAlertMigration(); - const marker = await guards().findOne({ _id: "recipient-alerts-v1" }); expect(marker.state).to.equal("new"); expect(marker.progress).to.equal(undefined); + const marker = await guards().findOne({ _id: "recipient-alerts-v2" }); expect(marker.state).to.equal("new"); expect(marker.progress).to.equal(undefined); await alerts.initializeOwnerAccessAlerts(); await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(0); expect((await guards().findOne({ _id: recipient() })).legacyClaims.filter(claim => !claim.recovered)).to.have.length(1); await reset(otherOwner.id, other); await recover(otherOwner.id, other, async () => {}); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(0); }); it("preserves a native guard's origin when rollback reconciliation adds another legacy claim", async () => { await alerts.initializeOwnerAccessAlerts(); await notify(owner.id, "token_expired", resource); await reset(owner.id, resource); @@ -617,22 +808,17 @@ describeMongo("recipient access alerts in MongoDB", function () { await reset(otherOwner.id, other); await recover(otherOwner.id, other, async () => {}); await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); await recover(owner.id, resource, async () => {}); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(2); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); }); - it("fences completed-guard retirement against a concurrent reactivation and reset", async () => { + it("keeps suppression through removal, reactivation, reset and recovery", async () => { await notify(owner.id, "token_expired", resource); + const original = await guards().findOne({ _id: recipient() }); await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { status: "removed" } }); - const collection = guards(), remove = collection.deleteOne; - let entered, release; const gate = new Promise(resolve => { entered = resolve; }); - collection.deleteOne = async function (...args) { entered(); await new Promise(resolve => { release = resolve; }); return remove.apply(this, args); }; - const pending = notify(otherOwner.id, "token_expired", other); - try { - await gate; - await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { status: "ready" } }); - await reset(owner.id, resource); - } finally { collection.deleteOne = remove; release(); await pending; } - expect(sent).to.have.length(1); expect(await guards().findOne({ _id: recipient() })).not.to.equal(null); - await recover(owner.id, resource, async () => {}); await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(2); + await notify(otherOwner.id, "token_expired", other); + await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { status: "ready" } }); + await reset(owner.id, resource); await recover(owner.id, resource, async () => {}); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); + expect((await guards().findOne({ _id: recipient() })).attemptId).to.equal(original.attemptId); }); for (const accepted of [true, false]) it(`retries the guard update after a definite ${accepted ? "accepted" : "rejected"} provider response`, async () => { const collection = guards(), update = collection.updateOne; let writes = 0; @@ -662,7 +848,7 @@ describeMongo("recipient access alerts in MongoDB", function () { if (startup) await alerts.initializeOwnerAccessAlerts(); else await notify(otherOwner.id, "token_expired", other); expect((await guards().findOne({ _id: recipient() })).state).to.equal("accepted"); expect(sent).to.have.length(1); await reset(owner.id, resource); await recover(owner.id, resource, async () => {}); - await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(2); + await notify(otherOwner.id, "token_expired", other); expect(sent).to.have.length(1); }); it("records a provider completion once after its insertion acknowledgement is lost", async () => { const collection = mongoose.connection.collection("owner_access_alert_events"), insert = collection.insertOne; @@ -690,7 +876,7 @@ describeMongo("recipient access alerts in MongoDB", function () { await gate; await notify(otherOwner.id, "token_expired", other); await reset(owner.id, resource); await recover(owner.id, resource, async () => {}); } finally { collection.updateOne = update; release(); await pending; } - expect(sent).to.have.length(1); expect(await guards().findOne({ _id: recipient() })).to.equal(null); + expect(sent).to.have.length(1); expect((await guards().findOne({ _id: recipient() })).state).to.equal("recovered"); }); it("confirms truncated-file metadata recovery only on a successful fresh matching-source probe", async () => { const database = require("../src/server/database"); @@ -705,31 +891,33 @@ describeMongo("recipient access alerts in MongoDB", function () { try { await notify(owner.id, "token_expired", resource); await reset(owner.id, resource); expect((await source.fetchFileInfoFromPath("a.txt")).sha).to.equal("a".repeat(40)); - expect(await guards().findOne({ _id: recipient() })).to.equal(null); + expect((await guards().findOne({ _id: recipient() })).state).to.equal("recovered"); + const secondKey = alerts.recipientAlertKey("second@example.com"); + await Users.updateOne({ _id: owner._id }, { $set: { notificationEmail: "second@example.com" } }); await notify(owner.id, "token_expired", resource); await reset(owner.id, resource); response = { type: "dir" }; expect(await source.fetchFileInfoFromPath("a.txt")).to.equal(null); - expect(await guards().findOne({ _id: recipient() })).not.to.equal(null); + expect((await guards().findOne({ _id: secondKey })).state).to.equal("accepted"); response = Object.assign(new Error("Forbidden"), { status: 403 }); try { await source.fetchFileInfoFromPath("a.txt"); throw new Error("Expected a failed probe"); } catch (error) { expect(error.message).to.equal("Forbidden"); } - expect(await guards().findOne({ _id: recipient() })).not.to.equal(null); + expect((await guards().findOne({ _id: secondKey })).state).to.equal("accepted"); response = { type: "file", sha: "a".repeat(40), size: 11 }; await coll(resource.kind).updateOne({ _id: new mongoose.Types.ObjectId(resource.id) }, { $set: { "source.repositoryName": "different/source" } }); await source.fetchFileInfoFromPath("a.txt"); - expect(await guards().findOne({ _id: recipient() })).not.to.equal(null); + expect((await guards().findOne({ _id: secondKey })).state).to.equal("accepted"); } finally { database.isConnected = originals.isConnected; utils.octokit = originals.octokit; } }); it("resumes a migration after a startup crashed and its lease expired", async () => { - await guards().insertOne({ _id: "recipient-alerts-v1", state: "new", lease: "crashed", leaseUntil: new Date(0) }); - await alerts.initializeOwnerAccessAlerts(); expect((await guards().findOne({ _id: "recipient-alerts-v1" })).state).to.equal("ready"); + await guards().insertOne({ _id: "recipient-alerts-v2", state: "new", lease: "crashed", leaseUntil: new Date(0) }); + await alerts.initializeOwnerAccessAlerts(); expect((await guards().findOne({ _id: "recipient-alerts-v2" })).state).to.equal("ready"); }); it("continues waiting beyond 300 polls while another migration holds a healthy lease", async () => { - await guards().insertOne({ _id: "recipient-alerts-v1", state: "new", lease: "active-migration", leaseUntil: new Date(Date.now() + 600000) }); + await guards().insertOne({ _id: "recipient-alerts-v2", state: "new", lease: "active-migration", leaseUntil: new Date(Date.now() + 600000) }); const collection = guards(), update = collection.updateOne, timeout = global.setTimeout; let polls = 0; collection.updateOne = async function (filter, ...args) { - if (filter._id === "recipient-alerts-v1" && filter.$or && ++polls === 305) { - await update.call(this, { _id: "recipient-alerts-v1" }, { $set: { state: "ready" } }); + if (filter._id === "recipient-alerts-v2" && filter.$or && ++polls === 305) { + await update.call(this, { _id: "recipient-alerts-v2" }, { $set: { state: "ready" } }); } return update.call(this, filter, ...args); }; @@ -737,6 +925,6 @@ describeMongo("recipient access alerts in MongoDB", function () { try { await alerts.initializeOwnerAccessAlerts(); } finally { collection.updateOne = update; global.setTimeout = timeout; } expect(polls).to.equal(305); - expect((await guards().findOne({ _id: "recipient-alerts-v1" })).state).to.equal("ready"); + expect((await guards().findOne({ _id: "recipient-alerts-v2" })).state).to.equal("ready"); }); });