From d519a0a9bba219a4b1301969caeac6bb8cf707c6 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Fri, 9 Oct 2026 07:38:52 +0000 Subject: [PATCH 1/2] fix: send one access alert per repository until owner action --- README.md | 11 +- src/core/AnonymizedFile.ts | 10 +- src/core/Gist.ts | 2 +- src/core/GitHubUtils.ts | 8 +- src/core/PullRequest.ts | 2 +- src/core/Repository.ts | 4 +- src/core/github-app.ts | 9 - .../anonymizedGists/anonymizedGists.schema.ts | 1 + .../anonymizedPullRequests.schema.ts | 1 + .../anonymizedRepositories.schema.ts | 1 + src/core/owner-notifications.ts | 49 ++++-- src/queue/processes/downloadRepository.ts | 2 +- src/server/routes/connection.ts | 2 + src/server/routes/gist-private.ts | 4 + src/server/routes/github-app.ts | 4 +- src/server/routes/pullRequest-private.ts | 4 + src/server/routes/repository-private.ts | 3 + test/owner-notifications.test.js | 155 ++++++++++-------- test/repository-refresh.test.js | 16 ++ 19 files changed, 172 insertions(+), 116 deletions(-) diff --git a/README.md b/README.md index 03c1e75..1132049 100644 --- a/README.md +++ b/README.md @@ -110,11 +110,12 @@ 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 database claim limits alerts to one per owner every 24 hours across -workers and repositories. Sending is best effort: delivery failures do not block -repository requests. A definite provider rejection allows a later access error -to retry after ten minutes; ambiguous timeouts retain the daily limit. -A process interrupted after claiming an alert may skip that day's email. +An atomic database claim allows one email attempt per repository, pull request, +or gist until its owner explicitly refreshes it, saves its settings or connection, +or reconnects GitHub. Reconnecting GitHub resets alerts for that owner's resources. +Viewing content, scheduled updates, and automatic token renewal never reset alerts. +There is no daily reminder. Claims also remain after delivery failures or an +interrupted process, avoiding duplicate messages until the owner acts. Leaving either Resend setting empty disables delivery and the email prompt. ## Scope of anonymization diff --git a/src/core/AnonymizedFile.ts b/src/core/AnonymizedFile.ts index 6bdd354..858a4bb 100644 --- a/src/core/AnonymizedFile.ts +++ b/src/core/AnonymizedFile.ts @@ -271,7 +271,7 @@ export default class AnonymizedFile { }; if (typeof source.fetchFileInfoFromPath !== "function") return null; const recovered = await source.fetchFileInfoFromPath(this.anonymizedPath).catch(error => { - void notifyOwnerAccessProblem(this.repository.owner.id, error); + void notifyOwnerAccessProblem(this.repository.owner.id, error, { kind: "repository", id: String(this.repository.model._id) }); throw error; }); if (!recovered) return null; @@ -360,10 +360,10 @@ export default class AnonymizedFile { }); } const content = await this.repository.source?.getFileContent(this).catch(error => { - void notifyOwnerAccessProblem(this.repository.owner.id, error); + void notifyOwnerAccessProblem(this.repository.owner.id, error, { kind: "repository", id: String(this.repository.model._id) }); throw error; }); - content.on("error", error => { void notifyOwnerAccessProblem(this.repository.owner.id, error); }); + content.on("error", error => { void notifyOwnerAccessProblem(this.repository.owner.id, error, { kind: "repository", id: String(this.repository.model._id) }); }); const cacheWasReset = this.repository.model.isReseted; if (cacheWasReset) { await this.repository.markCachePresent(); @@ -405,7 +405,7 @@ export default class AnonymizedFile { }); content.on("error", err => { const { error } = streamerErrorToAnonymous(err, { repoId: this.repository.repoId, filePath: this.anonymizedPath }); - void notifyOwnerAccessProblem(this.repository.owner.id, error); + void notifyOwnerAccessProblem(this.repository.owner.id, error, { kind: "repository", id: String(this.repository.model._id) }); }); return content; } @@ -485,7 +485,7 @@ export default class AnonymizedFile { filePath: this.anonymizedPath, } ); - void notifyOwnerAccessProblem(this.repository.owner.id, error); + void notifyOwnerAccessProblem(this.repository.owner.id, error, { kind: "repository", id: String(this.repository.model._id) }); error.value = this; handleError(error, res); if (res.headersSent && !res.writableEnded) res.destroy(); diff --git a/src/core/Gist.ts b/src/core/Gist.ts index 10e1e6b..0f9dd34 100644 --- a/src/core/Gist.ts +++ b/src/core/Gist.ts @@ -70,7 +70,7 @@ export default class Gist { async download() { try { await this.downloadWithAccess(); } catch (error) { - void notifyOwnerAccessProblem(this.owner.id, error); + void notifyOwnerAccessProblem(this.owner.id, error, { kind: "gist", id: String(this.model._id) }); throw error; } } diff --git a/src/core/GitHubUtils.ts b/src/core/GitHubUtils.ts index 040b3cd..97e5868 100644 --- a/src/core/GitHubUtils.ts +++ b/src/core/GitHubUtils.ts @@ -263,7 +263,7 @@ export function octokit(token: string) { export { waitForTokenGate }; -export async function checkToken(token: string, ownerId?: string) { +export async function checkToken(token: string) { const oct = octokit(token); try { const context = githubTokenContext(token); @@ -275,7 +275,6 @@ export async function checkToken(token: string, ownerId?: string) { // Only a confirmed invalid credential permits fallback. Network failures, // permission failures and rate limits must retain their original meaning. if ((err as { status?: number })?.status !== 401) throw err; - if (ownerId) void notifyOwnerAccessProblem(ownerId, "token_expired"); return false; } } @@ -286,7 +285,7 @@ export async function getToken(repository: Repository) { try { return await measureStage("authorization", () => resolveRepositoryToken(repository)); } catch (error) { - void notifyOwnerAccessProblem(repository.owner.id, error); + void notifyOwnerAccessProblem(repository.owner.id, error, { kind: "repository", id: String(repository.model._id) }); throw error; } } @@ -321,11 +320,12 @@ async function resolveRepositoryToken(repository: Repository) { return refreshed; } } - const check = await checkToken(ownerAccessToken, repository.owner.id); + const check = await checkToken(ownerAccessToken); if (check) { checkedRepositoryTokens.set(repository, ownerAccessToken); return ownerAccessToken; } + void notifyOwnerAccessProblem(repository.owner.id, "token_expired", { kind: "repository", id: String(repository.model._id) }); return config.GITHUB_TOKEN; } return (await getCredentialToken(repository.owner.id, "github", { diff --git a/src/core/PullRequest.ts b/src/core/PullRequest.ts index 3b63401..35b4198 100644 --- a/src/core/PullRequest.ts +++ b/src/core/PullRequest.ts @@ -36,7 +36,7 @@ export default class PullRequest { async download() { try { await this.downloadContent(); } catch (error) { - void notifyOwnerAccessProblem(this.owner.id, error); + void notifyOwnerAccessProblem(this.owner.id, error, { kind: "pull-request", id: String(this.model._id) }); throw error; } } diff --git a/src/core/Repository.ts b/src/core/Repository.ts index 0e1ce4a..e99dc58 100644 --- a/src/core/Repository.ts +++ b/src/core/Repository.ts @@ -225,7 +225,7 @@ export default class Repository { const streamed = this.model.source.type !== "Zip"; const source = this.source; const files = await source.getFiles(opt.progress).catch(error => { - void notifyOwnerAccessProblem(this.owner.id, error); + void notifyOwnerAccessProblem(this.owner.id, error, { kind: "repository", id: String(this.model._id) }); throw error; }); const sourceWithTruncation = source as unknown as { truncatedFolderList?: string[] }; @@ -630,7 +630,7 @@ export default class Repository { async updateIfNeeded(opt?: { force: boolean }): Promise { try { await this.refreshIfNeeded(opt); } catch (error) { - void notifyOwnerAccessProblem(this.owner.id, error); + void notifyOwnerAccessProblem(this.owner.id, error, { kind: "repository", id: String(this.model._id) }); throw error; } } diff --git a/src/core/github-app.ts b/src/core/github-app.ts index 1fb72eb..83d0bea 100644 --- a/src/core/github-app.ts +++ b/src/core/github-app.ts @@ -1,4 +1,3 @@ -import { notifyOwnerAccessProblem } from "./owner-notifications"; import { registerGitHubToken } from "./github-token-context"; import { createSign, randomUUID } from "crypto"; import { readFileSync } from "fs"; @@ -88,14 +87,6 @@ export async function saveAppGrant(ownerId: string, data: AppTokenResponse) { } export async function appUserToken(ownerId: string): Promise { - try { return await resolveAppUserToken(ownerId); } - catch (error) { - void notifyOwnerAccessProblem(ownerId, error); - throw error; - } -} - -async function resolveAppUserToken(ownerId: string): Promise { if (!config.GITHUB_APP_ENABLED) throw appError("github_app_disabled", 503); const user = await UserModel.findById(ownerId).select("status externalIDs").lean(); if (!user || user.status === "removed" || user.status === "banned") throw appError(); diff --git a/src/core/model/anonymizedGists/anonymizedGists.schema.ts b/src/core/model/anonymizedGists/anonymizedGists.schema.ts index f825010..ca2fa66 100644 --- a/src/core/model/anonymizedGists/anonymizedGists.schema.ts +++ b/src/core/model/anonymizedGists/anonymizedGists.schema.ts @@ -1,6 +1,7 @@ import { Schema } from "mongoose"; const AnonymizedGistSchema = new Schema({ + accessAlertClaimedAt: { type: Date, select: false }, gistId: { type: String, index: { unique: true }, diff --git a/src/core/model/anonymizedPullRequests/anonymizedPullRequests.schema.ts b/src/core/model/anonymizedPullRequests/anonymizedPullRequests.schema.ts index 9fab883..570343b 100644 --- a/src/core/model/anonymizedPullRequests/anonymizedPullRequests.schema.ts +++ b/src/core/model/anonymizedPullRequests/anonymizedPullRequests.schema.ts @@ -2,6 +2,7 @@ import { repositoryAccessSchema } from "../repository-access.schema"; import { Schema } from "mongoose"; const AnonymizedPullRequestSchema = new Schema({ + accessAlertClaimedAt: { type: Date, select: false }, pullRequestId: { type: String, index: { unique: true }, diff --git a/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts b/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts index c09db82..4d77d40 100644 --- a/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts +++ b/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts @@ -2,6 +2,7 @@ import { repositoryAccessSchema } from "../repository-access.schema"; import { Schema } from "mongoose"; const AnonymizedRepositorySchema = new Schema({ + accessAlertClaimedAt: { type: Date, select: false }, reviewConsentRevision: { type: Number, select: false }, repoId: { type: String, diff --git a/src/core/owner-notifications.ts b/src/core/owner-notifications.ts index 5f1b315..e3e4882 100644 --- a/src/core/owner-notifications.ts +++ b/src/core/owner-notifications.ts @@ -1,10 +1,28 @@ +import { Types } from "mongoose"; import { isGitHubRateLimitError } from "./github-rate-limit"; import config from "../config"; import UserModel from "./model/users/users.model"; import { createLogger } from "./logger"; const logger = createLogger("owner-notifications"); -const DAY = 24 * 60 * 60 * 1000; +const resourceCollections = { + repository: "anonymizedrepositories", + "pull-request": "anonymizedpullrequests", + gist: "anonymizedgists", +} as const; +export interface AlertResource { kind: keyof typeof resourceCollections; id: string; } + +// Only explicit owner actions call this; background fetches never reset alerts. +export async function resetOwnerAccessAlerts(ownerId: string, resource?: AlertResource): Promise { + if (UserModel.db.readyState !== 1 || !Types.ObjectId.isValid(ownerId)) return; + try { + const filter = { owner: new Types.ObjectId(ownerId), + ...(resource ? { _id: new Types.ObjectId(resource.id) } : {}) }; + for (const collection of resource ? [resourceCollections[resource.kind]] : Object.values(resourceCollections)) { + await UserModel.db.collection(collection).updateMany(filter, { $unset: { accessAlertClaimedAt: "" } }); + } + } catch { logger.warn("owner access alert reset unavailable"); } +} const accessErrors = new Set([ "token_expired", "github_oauth_required", "github_app_reconnect_required", "github_app_access_required", "repo_not_found", "repository_not_found", @@ -44,23 +62,23 @@ export function isAccessFailure(error: unknown): boolean { } /** Best effort; mail and database failures must not change repository behavior. */ -export async function notifyOwnerAccessProblem(ownerId: string, error: unknown): Promise { +export async function notifyOwnerAccessProblem(ownerId: string, error: unknown, resource: AlertResource): Promise { if (!config.RESEND_API_KEY || !config.EMAIL_FROM || !isAccessFailure(error)) return; try { - const now = new Date(); - // The atomic claim limits all repositories and workers to one email per owner - // per day. Claim before sending so concurrent requests cannot flood an inbox. - const owner = await UserModel.findOneAndUpdate({ - _id: ownerId, status: { $nin: ["removed", "banned"] }, - $and: [{ $or: [{ "emails.0": { $exists: true } }, { notificationEmail: { $exists: true, $ne: "" } }] }], - $or: [{ accessAlertAfter: { $exists: false } }, { accessAlertAfter: { $lte: now } }], - }, { $set: { accessAlertAfter: new Date(now.getTime() + DAY) } }, { new: true }).lean(); + if (!resource || !Types.ObjectId.isValid(resource.id) || !Types.ObjectId.isValid(ownerId)) return; + const owner = await UserModel.findOne({ _id: ownerId, status: { $nin: ["removed", "banned"] } }).lean(); if (!owner) return; const to = notificationEmail(owner.emails, owner.notificationEmail); if (!to) return; + // Claim the resource atomically before delivery. This has no expiry: neither + // repeated readers nor workers can send again until the owner acts. + const claim = await UserModel.db.collection(resourceCollections[resource.kind]).updateOne({ + _id: new Types.ObjectId(resource.id), owner: new Types.ObjectId(ownerId), + accessAlertClaimedAt: { $exists: false }, + }, { $set: { accessAlertClaimedAt: new Date() } }); + if (!claim.modifiedCount) return; const base = /^https?:\/\//.test(config.APP_HOSTNAME) ? config.APP_HOSTNAME : `https://${config.APP_HOSTNAME}`; const url = new URL("/connections", base).href; - let rejected = false; try { const response = await fetch("https://api.resend.com/emails", { method: "POST", @@ -72,13 +90,10 @@ export async function notifyOwnerAccessProblem(ownerId: string, error: unknown): text: `Anonymous GitHub could not access GitHub using your connection, or could not read a repository.\n\nOpen your connections page to reconnect your account or restore repository access:\n${url}\n\nYou are receiving this email because you own content on Anonymous GitHub. You can change your alert address on the connections page.\n`, }), }); - rejected = !response.ok; - if (rejected) throw new Error("email_delivery_failed"); + if (!response.ok) throw new Error("email_delivery_failed"); } catch { - // Only a definite rejection may retry early. A timeout can occur after - // Resend accepted the message, so retain the daily claim in that case. - if (rejected) await UserModel.updateOne({ _id: ownerId, accessAlertAfter: new Date(now.getTime() + DAY) }, - { $set: { accessAlertAfter: new Date(now.getTime() + 10 * 60000) } }); + // Keep the claim even on failure: delivery can be ambiguous and no + // automatic retry may send another email before owner action. logger.warn("owner access email delivery failed"); } } catch { diff --git a/src/queue/processes/downloadRepository.ts b/src/queue/processes/downloadRepository.ts index ba44600..2b9a8e1 100644 --- a/src/queue/processes/downloadRepository.ts +++ b/src/queue/processes/downloadRepository.ts @@ -143,7 +143,7 @@ export default async function (job: SandboxedJob) { if (error instanceof DelayedError || (error instanceof Error && error.name === "DelayedError")) { throw error; } - await notifyOwnerAccessProblem(repo.owner.id, error); + await notifyOwnerAccessProblem(repo.owner.id, error, { kind: "repository", id: String(repo.model._id) }); logger.error("finished with error", { ...serializeError(error), repoId: job.data.repoId, diff --git a/src/server/routes/connection.ts b/src/server/routes/connection.ts index 66e24eb..1e1fe00 100644 --- a/src/server/routes/connection.ts +++ b/src/server/routes/connection.ts @@ -1,3 +1,4 @@ +import { resetOwnerAccessAlerts } from "../../core/owner-notifications"; import { createClient } from "redis"; import * as passport from "passport"; import * as session from "express-session"; @@ -120,6 +121,7 @@ export const verify = async ( return; } await setCredential(String(user!._id), accessToken); + await resetOwnerAccessAlerts(String(user!._id)); done(null, { username: user!.username, user }); } catch (error) { logger.error("verify failed", serializeError(error)); diff --git a/src/server/routes/gist-private.ts b/src/server/routes/gist-private.ts index 59882a1..548ad3f 100644 --- a/src/server/routes/gist-private.ts +++ b/src/server/routes/gist-private.ts @@ -1,3 +1,4 @@ +import { resetOwnerAccessAlerts } from "../../core/owner-notifications"; import * as express from "express"; import { ensureAuthenticated } from "./connection"; @@ -29,6 +30,7 @@ router.post( const user = await getUser(req); isOwnerOrAdmin([gist.owner.id], user); + if (user.id === gist.owner.id) await resetOwnerAccessAlerts(user.id, { kind: "gist", id: String(gist.model._id) }); await gist.updateIfNeeded({ force: true }); res.json({ status: gist.status }); } catch (error) { @@ -68,6 +70,7 @@ router.post( { $set: { "options.expirationDate": newExpiration } } ).exec(); + if (user.id === gist.owner.id) await resetOwnerAccessAlerts(user.id, { kind: "gist", id: String(gist.model._id) }); await gist.updateIfNeeded({ force: true }); res.json({ status: gist.status, expirationDate: newExpiration }); } catch (error) { @@ -222,6 +225,7 @@ router.post( } ).exec(); await gist.updateStatus(RepositoryStatus.PREPARING); + if (user.id === gist.owner.id) await resetOwnerAccessAlerts(user.id, { kind: "gist", id: String(gist.model._id) }); await gist.updateIfNeeded({ force: true }); res.json(gist.toJSON()); } catch (error) { diff --git a/src/server/routes/github-app.ts b/src/server/routes/github-app.ts index efb3900..c818767 100644 --- a/src/server/routes/github-app.ts +++ b/src/server/routes/github-app.ts @@ -1,4 +1,4 @@ -import { notificationEmail, normalizeEmail } from "../../core/owner-notifications"; +import { notificationEmail, normalizeEmail, resetOwnerAccessAlerts } from "../../core/owner-notifications"; import * as express from "express"; import { createHmac, randomBytes, randomUUID, timingSafeEqual } from "crypto"; import config from "../../config"; @@ -94,6 +94,7 @@ router.get("/app/callback", enabled, async (req, res) => { } if (isDisabledAccount(user.status)) throw appError("not_connected", 403); await saveAppGrant(user.id, tokens); + await resetOwnerAccessAlerts(user.id); await new Promise((resolve, reject) => req.login({ username: user!.username, user }, err => err ? reject(err) : resolve())); res.redirect(flow.install ? `/github/app/install?returnTo=${encodeURIComponent(flow.returnTo)}&repository=${encodeURIComponent(flow.repository || "")}` : flow.returnTo); } catch (error) { handleError(error, res, req); } @@ -251,6 +252,7 @@ router.post("/connections/migrate", async (req, res) => { } }; const result = isRepo ? await RepositoryModel.updateOne(filter, change) : await PullRequestModel.updateOne(filter, change); if (!result.modifiedCount) throw appError("connection_changed", 409); + await resetOwnerAccessAlerts(user.id, { kind: isRepo ? "repository" : "pull-request", id: String(model._id) }); res.json({ connection }); } catch (error) { handleError(error, res, req); } }); diff --git a/src/server/routes/pullRequest-private.ts b/src/server/routes/pullRequest-private.ts index e620604..5c16444 100644 --- a/src/server/routes/pullRequest-private.ts +++ b/src/server/routes/pullRequest-private.ts @@ -1,3 +1,4 @@ +import { resetOwnerAccessAlerts } from "../../core/owner-notifications"; import { selectRepositoryAccess } from "../../core/github-app"; import * as express from "express"; import { ensureAuthenticated } from "./connection"; @@ -30,6 +31,7 @@ router.post( const user = await getUser(req); isOwnerOrAdmin([pullRequest.owner.id], user); + if (user.id === pullRequest.owner.id) await resetOwnerAccessAlerts(user.id, { kind: "pull-request", id: String(pullRequest.model._id) }); await pullRequest.updateIfNeeded({ force: true }); res.json({ status: pullRequest.status }); } catch (error) { @@ -71,6 +73,7 @@ router.post( { $set: { "options.expirationDate": newExpiration } } ).exec(); + if (user.id === pullRequest.owner.id) await resetOwnerAccessAlerts(user.id, { kind: "pull-request", id: String(pullRequest.model._id) }); await pullRequest.updateIfNeeded({ force: true }); res.json({ status: pullRequest.status, expirationDate: newExpiration }); } catch (error) { @@ -242,6 +245,7 @@ router.post( } ).exec(); await pullRequest.updateStatus(RepositoryStatus.PREPARING); + if (user.id === pullRequest.owner.id) await resetOwnerAccessAlerts(user.id, { kind: "pull-request", id: String(pullRequest.model._id) }); await pullRequest.updateIfNeeded({ force: true }); res.json({ ...pullRequest.toJSON(), connection: pullRequest.model.githubAccess?.kind || "oauth" }); } catch (error) { diff --git a/src/server/routes/repository-private.ts b/src/server/routes/repository-private.ts index f4983ad..1f19bc3 100644 --- a/src/server/routes/repository-private.ts +++ b/src/server/routes/repository-private.ts @@ -1,3 +1,4 @@ +import { resetOwnerAccessAlerts } from "../../core/owner-notifications"; import { randomUUID } from "crypto"; import { githubQuotaKey } from "../../core/github-token-context"; import { selectRepositoryAccess, boundAppToken, appError } from "../../core/github-app"; @@ -146,6 +147,7 @@ router.post( throw new AnonymousError("invalid_status", { httpStatus: 409 }); } + if (user.id === repo.owner.id) await resetOwnerAccessAlerts(user.id, { kind: "repository", id: String(repo.model._id) }); await repo.refresh(); res.json({ status: repo.status }); } catch (error) { @@ -621,6 +623,7 @@ router.post( } ).exec(); if (!saved.matchedCount) throw appError("connection_changed", 409); + if (user.id === repo.owner.id) await resetOwnerAccessAlerts(user.id, { kind: "repository", id: String(repo.model._id) }); if (!sourceChanged && !reactivating) { return res.json({ status: repo.status }); } diff --git a/test/owner-notifications.test.js b/test/owner-notifications.test.js index 54c94fa..aa4f59c 100644 --- a/test/owner-notifications.test.js +++ b/test/owner-notifications.test.js @@ -8,20 +8,31 @@ const { normalizeEmail, notificationEmail, notifyOwnerAccessProblem } = notifica describe("owner access notifications", () => { const restores = []; const stub = (object, key, value) => { const old = object[key]; restores.push(() => object[key] = old); object[key] = value; }; - let sent, claims, retryWrites, owner, after; + const ownerId = "507f1f77bcf86cd799439011"; + const resource = { kind: "repository", id: "507f1f77bcf86cd799439012" }; + const other = { kind: "repository", id: "507f1f77bcf86cd799439013" }; + let owner, sent, claims, rows; beforeEach(() => { - sent = []; claims = []; retryWrites = []; after = null; owner = { status: "active", emails: [{ email: "owner@example.com", default: true }] }; + sent = []; claims = []; rows = new Map(); stub(config, "RESEND_API_KEY", "test-resend-key"); stub(config, "EMAIL_FROM", "alerts@example.com"); stub(config, "APP_HOSTNAME", "anonymous.example.com"); - stub(User, "findOneAndUpdate", (filter, update) => ({ lean: async () => { - claims.push(filter); - if (!owner || ["removed", "banned"].includes(owner.status) || (!owner.emails.length && !owner.notificationEmail) || after > new Date()) return null; - after = update.$set.accessAlertAfter; - return owner; - } })); - stub(User, "updateOne", async (filter, update) => { retryWrites.push({ filter, update }); after = update.$set.accessAlertAfter; }); + stub(User, "findOne", () => ({ lean: async () => ["removed", "banned"].includes(owner?.status) ? null : owner })); + Object.defineProperty(User.db, "readyState", { configurable: true, value: 1 }); + restores.push(() => { delete User.db.readyState; }); + stub(User.db, "collection", name => ({ + updateOne: async (filter, update) => { + claims.push(filter); + const key = `${name}:${filter.owner}:${filter._id}`; + if (rows.has(key)) return { modifiedCount: 0 }; + rows.set(key, update.$set.accessAlertClaimedAt); + return { modifiedCount: 1 }; + }, + updateMany: async filter => { + for (const key of rows.keys()) if (key.startsWith(`${name}:${filter.owner}:`) && (!filter._id || key.endsWith(`:${filter._id}`))) rows.delete(key); + }, + })); stub(global, "fetch", async (url, options) => { sent.push({ url, options, body: JSON.parse(options.body) }); return { ok: true }; }); }); afterEach(() => { while (restores.length) restores.pop()(); }); @@ -34,63 +45,73 @@ describe("owner access notifications", () => { expect(notificationEmail([{ email: "first@example.com", default: false }, { email: "primary@example.com", default: true }])).to.equal("primary@example.com"); }); - it("sends one private Resend message across concurrent failures", async () => { - await Promise.all(Array.from({ length: 10 }, () => notifyOwnerAccessProblem("owner-id", "token_expired"))); + it("sends once per resource across concurrent readers and repeated errors, with no time expiry", async () => { + await Promise.all(Array.from({ length: 10 }, () => notifyOwnerAccessProblem(ownerId, "token_expired", resource))); expect(sent).to.have.length(1); + for (const key of rows.keys()) rows.set(key, new Date("2000-01-01")); + await notifyOwnerAccessProblem(ownerId, "repo_not_found", resource); + expect(sent).to.have.length(1); + await notifyOwnerAccessProblem(ownerId, "repo_not_found", other); + expect(sent).to.have.length(2); expect(sent[0].url).to.equal("https://api.resend.com/emails"); - expect(sent[0].options.headers.Authorization).to.equal("Bearer test-resend-key"); expect(sent[0].body.to).to.deep.equal(["owner@example.com"]); expect(sent[0].body.text).to.include("https://anonymous.example.com/connections"); - expect(JSON.stringify(sent[0].body)).not.to.include("test-resend-key"); - expect(claims[0].status).to.deep.equal({ $nin: ["removed", "banned"] }); - expect(claims[0].$or).to.have.length(2); + expect(claims[0].accessAlertClaimedAt).to.deep.equal({ $exists: false }); }); - it("ignores disabled delivery, missing recipients and transient failures", async () => { - for (const code of ["github_rate_limit_exceeded", "github_unavailable", "ETIMEDOUT", "quota_exceeded"]) { - await notifyOwnerAccessProblem("owner-id", new Error(code)); - } - await notifyOwnerAccessProblem("owner-id", Object.assign(new Error("repo_not_found"), { cause: { status: 503 } })); - await notifyOwnerAccessProblem("owner-id", Object.assign(new Error("token_expired"), { cause: new Error("network timeout") })); - expect(claims).to.have.length(0); - owner.emails = []; - await notifyOwnerAccessProblem("owner-id", "repo_not_found"); - owner.emails = [{ email: "owner@example.com", default: true }]; - owner.status = "banned"; - await notifyOwnerAccessProblem("owner-id", "repo_not_found"); - config.RESEND_API_KEY = ""; - await notifyOwnerAccessProblem("owner-id", "token_expired"); - expect(sent).to.have.length(0); - }); - - it("backs off after delivery failure without throwing", async () => { - stub(global, "fetch", async () => ({ ok: false })); - await notifyOwnerAccessProblem("owner-id", "repo_not_accessible"); - expect(retryWrites).to.have.length(1); - expect(after.getTime() - Date.now()).to.be.within(590000, 600000); - await notifyOwnerAccessProblem("owner-id", "repo_not_accessible"); - expect(retryWrites).to.have.length(1); - }); - - it("retains the daily claim when delivery may already have happened", async () => { - let attempts = 0; - stub(global, "fetch", async () => { attempts++; throw new Error("response timed out after acceptance"); }); - await notifyOwnerAccessProblem("owner-id", "token_expired"); - expect(retryWrites).to.have.length(0); - expect(after.getTime() - Date.now()).to.be.within(86390000, 86400000); - await notifyOwnerAccessProblem("owner-id", "token_expired"); - expect(attempts).to.equal(1); - }); - - it("uses a dedicated alert address without requiring or replacing profile emails", async () => { - owner.notificationEmail = "alerts@example.com"; - const original = JSON.stringify(owner.emails); - await notifyOwnerAccessProblem("owner-id", "token_expired"); - expect(sent[0].body.to).to.deep.equal(["alerts@example.com"]); - expect(JSON.stringify(owner.emails)).to.equal(original); - after = null; owner.emails = []; - await notifyOwnerAccessProblem("owner-id", "token_expired"); + it("owner action resets only the selected resource and cannot reset another owner's alerts", async () => { + await notifyOwnerAccessProblem(ownerId, "token_expired", resource); + await notifyOwnerAccessProblem(ownerId, "token_expired", other); + await notifications.resetOwnerAccessAlerts("507f1f77bcf86cd799439099", resource); + await notifyOwnerAccessProblem(ownerId, "token_expired", resource); expect(sent).to.have.length(2); + await notifications.resetOwnerAccessAlerts(ownerId, resource); + await notifyOwnerAccessProblem(ownerId, "token_expired", resource); + await notifyOwnerAccessProblem(ownerId, "token_expired", other); + expect(sent).to.have.length(3); + }); + + it("reconnecting resets all of the owner's resource types", async () => { + const resources = [resource, { ...resource, kind: "pull-request" }, { ...resource, kind: "gist" }]; + for (const item of resources) await notifyOwnerAccessProblem(ownerId, "token_expired", item); + expect(sent).to.have.length(3); + await notifications.resetOwnerAccessAlerts(ownerId); + for (const item of resources) await notifyOwnerAccessProblem(ownerId, "token_expired", item); + expect(sent).to.have.length(6); + }); + + for (const failure of ["rejected", "timeout"]) it(`retains the claim after ${failure} until owner action`, 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); + await notifyOwnerAccessProblem(ownerId, "token_expired", resource); + expect(attempts).to.equal(1); + await notifications.resetOwnerAccessAlerts(ownerId, resource); + await notifyOwnerAccessProblem(ownerId, "token_expired", resource); + expect(attempts).to.equal(2); + }); + + it("does not claim an alert without configuration, an active owner, and an address", async () => { + config.RESEND_API_KEY = ""; + await notifyOwnerAccessProblem(ownerId, "token_expired", resource); + config.RESEND_API_KEY = "test-key"; + owner.status = "banned"; + await notifyOwnerAccessProblem(ownerId, "token_expired", resource); + owner.status = "active"; owner.emails = []; + await notifyOwnerAccessProblem(ownerId, "token_expired", resource); + expect(claims).to.have.length(0); + owner.notificationEmail = "alerts@example.com"; + await notifyOwnerAccessProblem(ownerId, "token_expired", resource); + expect(sent[0].body.to).to.deep.equal(["alerts@example.com"]); + }); + + it("ignores transient failures and contains database failures", async () => { + await notifyOwnerAccessProblem(ownerId, new Error("github_rate_limit_exceeded"), resource); + await notifyOwnerAccessProblem(ownerId, Object.assign(new Error("repo_not_found"), { cause: { status: 503 } }), resource); + expect(claims).to.have.length(0); + stub(User, "findOne", () => { throw new Error("database down"); }); + await notifyOwnerAccessProblem(ownerId, "token_expired", resource); + expect(sent).to.have.length(0); }); it("recognizes raw authentication and missing-resource errors without alerting for outages", () => { @@ -113,14 +134,8 @@ describe("owner access notifications", () => { expect(notifications.isAccessFailure(new AnonymousError("file_not_accessible", { httpStatus: 403, cause: forbidden }))).to.equal(true); }); - it("does not let database failures escape", async () => { - stub(User, "findOneAndUpdate", () => { throw new Error("database down"); }); - await notifyOwnerAccessProblem("owner-id", "token_expired"); - expect(sent).to.have.length(0); - }); }); - describe("owner email settings", () => { const utils = require("../src/server/routes/route-utils"); const router = require("../src/server/routes/github-app").githubAppRouter; @@ -238,14 +253,14 @@ describe("repository notification hooks", () => { const error = new Error("github_app_access_required"); stub(app, "boundAppToken", async () => { throw error; }); await failsWith(() => tokens.getToken(repo), error); - expect(sent).to.deep.equal([[repo.owner.id, error]]); + expect(sent).to.deep.equal([[repo.owner.id, error, { kind: "repository", id: String(repo.model._id) }]]); }); it("notifies the saved owner when reading the repository tree fails", async () => { const error = new Error("repo_not_found"); stub(Files, "exists", () => ({ exec: async () => null })); Object.defineProperty(repo, "source", { value: { getFiles: async () => { throw error; } } }); await failsWith(() => repo.files(), error); - expect(sent).to.deep.equal([[repo.owner.id, error]]); + expect(sent).to.deep.equal([[repo.owner.id, error, { kind: "repository", id: String(repo.model._id) }]]); }); for (const step of ["branches", "commit"]) it(`notifies on ${step} access errors during refresh`, async () => { const error = Object.assign(new Error("Forbidden"), { status: 403 }); @@ -256,14 +271,14 @@ describe("repository notification hooks", () => { getCommitInfo: async () => { throw error; }, })); await failsWith(() => repo.updateIfNeeded({ force: true }), error); - expect(sent).to.deep.equal([[repo.owner.id, error]]); + expect(sent).to.deep.equal([[repo.owner.id, error, { kind: "repository", id: String(repo.model._id) }]]); }); it("notifies the saved owner when a repository refresh cannot read GitHub", async () => { const error = new Error("repo_not_found"); repo.getToken = async () => "token"; stub(github, "getRepositoryFromGitHub", async () => { throw error; }); await failsWith(() => repo.updateIfNeeded({ force: true }), error); - expect(sent).to.deep.equal([[repo.owner.id, error]]); + expect(sent).to.deep.equal([[repo.owner.id, error, { kind: "repository", id: String(repo.model._id) }]]); }); }); @@ -297,7 +312,7 @@ describe("gist and pull-request notification hooks", () => { try { await resource.download(); } catch (err) { caught = err; } expect(caught).to.be.instanceOf(Error); expect(caught.message).to.equal(type === "gist" && failure === "oauth" ? "github_oauth_required" : error.message); - expect(sent).to.deep.equal([[resource.owner.id, caught]]); + expect(sent).to.deep.equal([[resource.owner.id, caught, { kind: type, id: String(resource.model._id) }]]); }); } }); diff --git a/test/repository-refresh.test.js b/test/repository-refresh.test.js index 7014877..fd1dd25 100644 --- a/test/repository-refresh.test.js +++ b/test/repository-refresh.test.js @@ -50,6 +50,22 @@ describe("repository refresh and restoration", () => { expect(failure.httpStatus).to.equal(403); }); + for (const actingOwner of [true, false]) { + it(`${actingOwner ? "rearms" : "does not rearm"} alerts when ${actingOwner ? "the owner" : "an admin"} refreshes`, async () => { + const repo = repository("ready"); + const notifications = require("../src/core/owner-notifications"); + const resets = []; + stub(notifications, "resetOwnerAccessAlerts", async (...args) => { resets.push(args); }); + const id = actingOwner ? repo.owner.id : "admin"; + stub(utils, "getUser", async () => ({ id, model: { id }, isAdmin: !actingOwner })); + repo.refresh = async () => { + expect(resets).to.have.length(actingOwner ? 1 : 0); + }; + await refresh({}, { json: () => {} }); + expect(resets).to.deep.equal(actingOwner ? [[repo.owner.id, { kind: "repository", id: String(repo.model._id) }]] : []); + }); + } + for (const commit of ["saved-sha", "new-sha"]) { it(`rebuilds a removed repository at ${commit} while preserving its ID`, async () => { const repo = repository("removed"); From 633e5ddcae09d8fb4252c0fd887895742b2ae2e1 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Fri, 9 Oct 2026 07:47:51 +0000 Subject: [PATCH 2/2] fix: rearm owner alerts across recovery actions safely --- README.md | 4 +- .../anonymizedGists/anonymizedGists.schema.ts | 1 + .../anonymizedPullRequests.schema.ts | 1 + .../anonymizedRepositories.schema.ts | 1 + src/core/owner-notifications.ts | 13 +++- src/server/routes/github-app.ts | 1 + src/server/routes/repository-private.ts | 4 +- test/owner-alert-actions.test.js | 65 +++++++++++++++++++ test/owner-notifications.test.js | 37 +++++++++-- 9 files changed, 117 insertions(+), 10 deletions(-) create mode 100644 test/owner-alert-actions.test.js diff --git a/README.md b/README.md index 1132049..4484c70 100644 --- a/README.md +++ b/README.md @@ -112,7 +112,9 @@ Emails contain no repository names, anonymous URLs, or credentials. An atomic database claim allows one email attempt per repository, pull request, or gist until its owner explicitly refreshes it, saves its settings or connection, -or reconnects GitHub. Reconnecting GitHub resets alerts for that owner's resources. +or reconnects GitHub. Claiming ownership also clears the previous owner's alert. +Reconnecting GitHub or completing a verified App installation update resets alerts +for that owner's resources. Resets invalidate pending claims from older failures. Viewing content, scheduled updates, and automatic token renewal never reset alerts. There is no daily reminder. Claims also remain after delivery failures or an interrupted process, avoiding duplicate messages until the owner acts. diff --git a/src/core/model/anonymizedGists/anonymizedGists.schema.ts b/src/core/model/anonymizedGists/anonymizedGists.schema.ts index ca2fa66..65f06f1 100644 --- a/src/core/model/anonymizedGists/anonymizedGists.schema.ts +++ b/src/core/model/anonymizedGists/anonymizedGists.schema.ts @@ -1,6 +1,7 @@ import { Schema } from "mongoose"; const AnonymizedGistSchema = new Schema({ + accessAlertGeneration: { type: String, select: false }, accessAlertClaimedAt: { type: Date, select: false }, gistId: { type: String, diff --git a/src/core/model/anonymizedPullRequests/anonymizedPullRequests.schema.ts b/src/core/model/anonymizedPullRequests/anonymizedPullRequests.schema.ts index 570343b..8ae76ed 100644 --- a/src/core/model/anonymizedPullRequests/anonymizedPullRequests.schema.ts +++ b/src/core/model/anonymizedPullRequests/anonymizedPullRequests.schema.ts @@ -2,6 +2,7 @@ import { repositoryAccessSchema } from "../repository-access.schema"; import { Schema } from "mongoose"; const AnonymizedPullRequestSchema = new Schema({ + accessAlertGeneration: { type: String, select: false }, accessAlertClaimedAt: { type: Date, select: false }, pullRequestId: { type: String, diff --git a/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts b/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts index 4d77d40..cd5acf9 100644 --- a/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts +++ b/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts @@ -2,6 +2,7 @@ import { repositoryAccessSchema } from "../repository-access.schema"; import { Schema } from "mongoose"; const AnonymizedRepositorySchema = new Schema({ + accessAlertGeneration: { type: String, select: false }, accessAlertClaimedAt: { type: Date, select: false }, reviewConsentRevision: { type: Number, select: false }, repoId: { diff --git a/src/core/owner-notifications.ts b/src/core/owner-notifications.ts index e3e4882..1122277 100644 --- a/src/core/owner-notifications.ts +++ b/src/core/owner-notifications.ts @@ -1,3 +1,4 @@ +import { randomUUID } from "crypto"; import { Types } from "mongoose"; import { isGitHubRateLimitError } from "./github-rate-limit"; import config from "../config"; @@ -19,7 +20,7 @@ export async function resetOwnerAccessAlerts(ownerId: string, resource?: AlertRe const filter = { owner: new Types.ObjectId(ownerId), ...(resource ? { _id: new Types.ObjectId(resource.id) } : {}) }; for (const collection of resource ? [resourceCollections[resource.kind]] : Object.values(resourceCollections)) { - await UserModel.db.collection(collection).updateMany(filter, { $unset: { accessAlertClaimedAt: "" } }); + await UserModel.db.collection(collection).updateMany(filter, { $unset: { accessAlertClaimedAt: "" }, $set: { accessAlertGeneration: randomUUID() } }); } } catch { logger.warn("owner access alert reset unavailable"); } } @@ -66,14 +67,20 @@ export async function notifyOwnerAccessProblem(ownerId: string, error: unknown, if (!config.RESEND_API_KEY || !config.EMAIL_FROM || !isAccessFailure(error)) return; 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 snapshot = await collection.findOne(identity, { projection: { accessAlertGeneration: 1 } }); + if (!snapshot) return; const owner = await UserModel.findOne({ _id: ownerId, status: { $nin: ["removed", "banned"] } }).lean(); if (!owner) return; const to = notificationEmail(owner.emails, owner.notificationEmail); if (!to) return; // Claim the resource atomically before delivery. This has no expiry: neither // repeated readers nor workers can send again until the owner acts. - const claim = await UserModel.db.collection(resourceCollections[resource.kind]).updateOne({ - _id: new Types.ObjectId(resource.id), owner: new Types.ObjectId(ownerId), + const claim = await collection.updateOne({ + ...identity, + // A reset during the owner lookup invalidates this older failure. + accessAlertGeneration: snapshot.accessAlertGeneration ?? { $exists: false }, accessAlertClaimedAt: { $exists: false }, }, { $set: { accessAlertClaimedAt: new Date() } }); if (!claim.modifiedCount) return; diff --git a/src/server/routes/github-app.ts b/src/server/routes/github-app.ts index c818767..4cd003d 100644 --- a/src/server/routes/github-app.ts +++ b/src/server/routes/github-app.ts @@ -149,6 +149,7 @@ router.get("/app/setup", enabled, async (req, res) => { if (req.query.setup_action !== "request") { const installations = await userInstallations(user.id); if (!installations.some(i => String(i.id) === req.query.installation_id)) throw appError("github_app_access_required"); + await resetOwnerAccessAlerts(user.id); } await UserModel.updateOne({ _id: user.id }, { $set: { repositories: [] } }); res.redirect(flow.returnTo); diff --git a/src/server/routes/repository-private.ts b/src/server/routes/repository-private.ts index 1f19bc3..49b9d40 100644 --- a/src/server/routes/repository-private.ts +++ b/src/server/routes/repository-private.ts @@ -115,7 +115,8 @@ router.post("/claim", async (req, res) => { await AnonymizedRepositoryModel.updateOne( { repoId: repoConfig.repoId }, - { $set: { owner: user.model.id, githubAccess: selectedAccess.binding } } + { $set: { owner: user.model.id, githubAccess: selectedAccess.binding, accessAlertGeneration: randomUUID() }, + $unset: { accessAlertClaimedAt: "" } } ).collation({ locale: "en", strength: 2 }); return res.send("Ok"); } catch (error) { @@ -199,6 +200,7 @@ router.post( { $set: updates } ).exec(); if (saved && saved.matchedCount === 0) throw new AnonymousError("invalid_status", { httpStatus: 409 }); + if (user.id === repo.owner.id) await resetOwnerAccessAlerts(user.id, { kind: "repository", id: String(repo.model._id) }); if (reactivating) { // Expiration removes the cached files. Rebuild the saved commit diff --git a/test/owner-alert-actions.test.js b/test/owner-alert-actions.test.js new file mode 100644 index 0000000..59e0fae --- /dev/null +++ b/test/owner-alert-actions.test.js @@ -0,0 +1,65 @@ +const { expect } = require("chai"); +require("ts-node/register/transpile-only"); +const notifications = require("../src/core/owner-notifications"); +const utils = require("../src/server/routes/route-utils"); +const app = require("../src/core/github-app"); +const db = require("../src/server/database"); +const github = require("../src/core/source/GitHubRepository"); +const Resources = require("../src/core/model/anonymizedRepositories/anonymizedRepositories.model").default; +const Sources = require("../src/core/model/repositories/repositories.model").default; +const Users = require("../src/core/model/users/users.model").default; +const repositoryRouter = require("../src/server/routes/repository-private").default; +const appRouter = require("../src/server/routes/github-app").githubAppRouter; +const handler = (router, path) => router.stack.find(layer => layer.route?.path === path).route.stack.at(-1).handle; + +describe("owner actions rearm access alerts", () => { + let restores, resets; + const owner = "507f1f77bcf86cd799439011"; + function stub(object, key, value) { + const old = object[key]; restores.push(() => { object[key] = old; }); object[key] = value; + } + beforeEach(() => { + restores = []; resets = []; + stub(utils, "getUser", async () => ({ id: owner, model: { id: owner } })); + stub(utils, "handleError", error => { throw error; }); + stub(notifications, "resetOwnerAccessAlerts", async (...args) => { resets.push(args); }); + }); + afterEach(() => { while (restores.length) restores.pop()(); }); + + for (const isOwner of [true, false]) it(`extension ${isOwner ? "rearms for its owner" : "does not rearm for an admin"}`, async () => { + const id = isOwner ? owner : "admin"; + stub(utils, "getUser", async () => ({ id, model: { id }, isAdmin: !isOwner })); + const repo = { owner: { id: owner }, status: "ready", model: { _id: "resource", options: {} } }; + stub(utils, "getRepo", async () => repo); + stub(Resources, "updateOne", () => ({ exec: async () => ({ matchedCount: 1 }) })); + await handler(repositoryRouter, "/:repoId/extend")({}, { json() {} }); + expect(resets).to.deep.equal(isOwner ? [[owner, { kind: "repository", id: "resource" }]] : []); + }); + + it("clears the previous owner's claim atomically when a repository is claimed", async () => { + stub(db, "getRepository", async () => ({ repoId: "resource", model: { source: { repositoryId: "source" } } })); + stub(app, "selectRepositoryAccess", async () => ({ token: "token", binding: { kind: "oauth" } })); + stub(github, "getRepositoryFromGitHub", async () => ({ id: 42 })); + stub(Sources, "findById", async () => ({ externalId: 42 })); + let write; + stub(Resources, "updateOne", (_filter, update) => ({ collation: async () => { write = update; } })); + await handler(repositoryRouter, "/claim")({ body: { repoId: "resource", repoUrl: "https://github.com/owner/repo" }, query: {} }, { send() {} }); + expect(write.$set.owner).to.equal(owner); + expect(write.$set.accessAlertGeneration).to.be.a("string"); + expect(write.$unset).to.have.property("accessAlertClaimedAt"); + }); + + for (const action of ["install", "request", "unverified"]) it(`handles ${action} installation completion without premature resets`, async () => { + stub(app, "userInstallations", async () => [{ id: 42 }]); + stub(Users, "updateOne", async () => ({})); + const req = { session: { + githubInstallFlow: { ownerId: owner, state: "state", expires: Date.now() + 60000, returnTo: "/connections" }, + save: callback => callback(), + }, isAuthenticated: () => true, query: { state: "state", setup_action: action, installation_id: action === "unverified" ? "99" : "42" } }; + let failure; + try { await handler(appRouter, "/app/setup")(req, { redirect() {} }); } catch (error) { failure = error; } + expect(resets).to.deep.equal(action === "install" ? [[owner]] : []); + if (action === "unverified") expect(failure?.message).to.equal("github_app_access_required"); + else expect(failure).to.equal(undefined); + }); +}); diff --git a/test/owner-notifications.test.js b/test/owner-notifications.test.js index aa4f59c..74c7b35 100644 --- a/test/owner-notifications.test.js +++ b/test/owner-notifications.test.js @@ -11,10 +11,10 @@ describe("owner access notifications", () => { const ownerId = "507f1f77bcf86cd799439011"; const resource = { kind: "repository", id: "507f1f77bcf86cd799439012" }; const other = { kind: "repository", id: "507f1f77bcf86cd799439013" }; - let owner, sent, claims, rows; + let owner, sent, claims, rows, generations; beforeEach(() => { owner = { status: "active", emails: [{ email: "owner@example.com", default: true }] }; - sent = []; claims = []; rows = new Map(); + sent = []; claims = []; rows = new Map(); generations = new Map(); stub(config, "RESEND_API_KEY", "test-resend-key"); stub(config, "EMAIL_FROM", "alerts@example.com"); stub(config, "APP_HOSTNAME", "anonymous.example.com"); @@ -22,15 +22,23 @@ describe("owner access notifications", () => { Object.defineProperty(User.db, "readyState", { configurable: true, value: 1 }); restores.push(() => { delete User.db.readyState; }); stub(User.db, "collection", name => ({ + findOne: async filter => { + const key = `${name}:${filter.owner}:${filter._id}`; + if (!generations.has(key)) generations.set(key, undefined); + return { accessAlertGeneration: generations.get(key) }; + }, updateOne: async (filter, update) => { claims.push(filter); const key = `${name}:${filter.owner}:${filter._id}`; - if (rows.has(key)) return { modifiedCount: 0 }; + if (rows.has(key) || generations.get(key) !== (typeof filter.accessAlertGeneration === "string" ? filter.accessAlertGeneration : undefined)) return { modifiedCount: 0 }; rows.set(key, update.$set.accessAlertClaimedAt); return { modifiedCount: 1 }; }, - updateMany: async filter => { - for (const key of rows.keys()) if (key.startsWith(`${name}:${filter.owner}:`) && (!filter._id || key.endsWith(`:${filter._id}`))) rows.delete(key); + updateMany: async (filter, update) => { + for (const key of generations.keys()) if (key.startsWith(`${name}:${filter.owner}:`) && (!filter._id || key.endsWith(`:${filter._id}`))) { + rows.delete(key); + generations.set(key, update.$set.accessAlertGeneration); + } }, })); stub(global, "fetch", async (url, options) => { sent.push({ url, options, body: JSON.parse(options.body) }); return { ok: true }; }); @@ -71,6 +79,25 @@ describe("owner access notifications", () => { expect(sent).to.have.length(3); }); + it("rejects a pre-action failure paused during owner lookup", async () => { + let resume, entered; + const waiting = new Promise(resolve => { entered = resolve; }); + stub(User, "findOne", () => ({ lean: async () => { + entered(); + await new Promise(resolve => { resume = resolve; }); + return owner; + } })); + const oldFailure = notifyOwnerAccessProblem(ownerId, "token_expired", resource); + await waiting; + await notifications.resetOwnerAccessAlerts(ownerId, resource); + resume(); + await oldFailure; + expect(sent).to.have.length(0); + stub(User, "findOne", () => ({ lean: async () => owner })); + await notifyOwnerAccessProblem(ownerId, "token_expired", resource); + expect(sent).to.have.length(1); + }); + it("reconnecting resets all of the owner's resource types", async () => { const resources = [resource, { ...resource, kind: "pull-request" }, { ...resource, kind: "gist" }]; for (const item of resources) await notifyOwnerAccessProblem(ownerId, "token_expired", item);