From 9af22f2ab21243daa9d261d08d18766775fdeba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 05:38:44 +0200 Subject: [PATCH] fix(account): give account deletion the wipe's transaction budget The cascade behind user.delete removes every row the account owns, so on a large record it outlived Prisma's 5 s interactive default and rolled back with a 500 on every attempt. The deletion stays atomic with the Guardian check and now carries the same budget as the data wipe. --- .../lifecycle-transaction-budget.test.ts | 81 +++++++++++++++++++ src/lib/managed-profiles/lifecycle.ts | 30 ++++++- 2 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 src/lib/managed-profiles/__tests__/lifecycle-transaction-budget.test.ts diff --git a/src/lib/managed-profiles/__tests__/lifecycle-transaction-budget.test.ts b/src/lib/managed-profiles/__tests__/lifecycle-transaction-budget.test.ts new file mode 100644 index 000000000..ded662439 --- /dev/null +++ b/src/lib/managed-profiles/__tests__/lifecycle-transaction-budget.test.ts @@ -0,0 +1,81 @@ +/** + * Both transactions that end in `user.delete` carry the deletion budget. + * + * The cascade behind deleting an account row removes everything the account + * owns, so on a large record it outlives Prisma's 5 s interactive default. The + * transaction then rolled back and the person got a 500 with nothing deleted, + * on every retry. Pinned at the call: a budget declared and not passed is the + * failure this replaces. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { transaction } = vi.hoisted(() => ({ transaction: vi.fn() })); + +vi.mock("@/lib/db", () => ({ prisma: { $transaction: transaction } })); +vi.mock("@/lib/jobs/offhost-purge", () => ({ + kickOffhostPurge: vi.fn(async () => undefined), + requestOffhostPurge: vi.fn(async () => undefined), +})); +vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn(async () => undefined) })); + +import { + ACCOUNT_DELETION_TRANSACTION_OPTIONS, + deleteGuardianAccountWithLifecycle, + deleteManagedProfile, +} from "../lifecycle"; +import { WIPE_TRANSACTION_TIMEOUT_MS } from "@/lib/data-wipe/wipe-plan"; + +/** A transaction client that holds no grants and one managed profile. */ +function fakeTx() { + return { + $queryRaw: vi.fn(async () => [{ locked: 1 }]), + user: { + findUnique: vi.fn(async () => ({ + id: "profile-1", + managedProfileAt: new Date(), + })), + delete: vi.fn(async () => ({})), + }, + accountGrant: { + findMany: vi.fn(async () => []), + findFirst: vi.fn(async () => ({ id: "grant-1" })), + count: vi.fn(async () => 2), + }, + }; +} + +beforeEach(() => { + transaction.mockReset(); + transaction.mockImplementation( + async (run: (tx: ReturnType) => Promise) => + run(fakeTx()), + ); +}); + +describe("account deletion transaction budget", () => { + it("is well past Prisma's 5 s default and matches the wipe's", () => { + expect(ACCOUNT_DELETION_TRANSACTION_OPTIONS.timeout).toBe( + WIPE_TRANSACTION_TIMEOUT_MS, + ); + expect(ACCOUNT_DELETION_TRANSACTION_OPTIONS.timeout).toBeGreaterThan(5_000); + }); + + it("is passed by the guardian account deletion", async () => { + await deleteGuardianAccountWithLifecycle("guardian-1", async () => "ok"); + expect(transaction).toHaveBeenCalledTimes(1); + expect(transaction.mock.calls[0][1]).toEqual( + ACCOUNT_DELETION_TRANSACTION_OPTIONS, + ); + }); + + it("is passed by the managed profile deletion", async () => { + await deleteManagedProfile({ + profileId: "profile-1", + guardianId: "guardian-1", + }); + expect(transaction).toHaveBeenCalledTimes(1); + expect(transaction.mock.calls[0][1]).toEqual( + ACCOUNT_DELETION_TRANSACTION_OPTIONS, + ); + }); +}); diff --git a/src/lib/managed-profiles/lifecycle.ts b/src/lib/managed-profiles/lifecycle.ts index 8ea7ad142..1b581906d 100644 --- a/src/lib/managed-profiles/lifecycle.ts +++ b/src/lib/managed-profiles/lifecycle.ts @@ -6,6 +6,10 @@ import { prisma } from "@/lib/db"; import { auditLog } from "@/lib/auth/audit"; import type { Prisma } from "@/generated/prisma/client"; import { dayKeyAsUtcMidnight, dateOnlyKey } from "@/lib/tz/date-only"; +import { + WIPE_TRANSACTION_MAX_WAIT_MS, + WIPE_TRANSACTION_TIMEOUT_MS, +} from "@/lib/data-wipe/wipe-plan"; type Transaction = Prisma.TransactionClient; @@ -79,6 +83,28 @@ export async function reduceManagedProfileGuardian( }); } +/** + * Budget for a transaction that ends in `user.delete`. + * + * Deleting an account row is one statement, but the cascade behind it removes + * every row the account owns across the whole schema — the same set the data + * wipe deletes model by model — so its duration grows with the record. Prisma's + * default interactive-transaction timeout is 5 s; an account with a few years + * of imported samples blew through it, the transaction rolled back, and the + * person was answered 500 with nothing deleted, every time they tried. + * + * Splitting the delete into batches outside the transaction is not an option: + * the Guardian invariant is decided under the record lock inside this + * transaction, and a refusal there must leave the account whole. Batches + * committed before the refusal would be a partial erasure the person was told + * did not happen. So the deletion stays atomic and gets the wipe's budget, + * which was sized for the same work. + */ +export const ACCOUNT_DELETION_TRANSACTION_OPTIONS = { + timeout: WIPE_TRANSACTION_TIMEOUT_MS, + maxWait: WIPE_TRANSACTION_MAX_WAIT_MS, +} as const; + /** * Keep account deletion and its Guardian checks inside the same transaction. * Profiles are locked in id order so an account protecting more than one @@ -123,7 +149,7 @@ export async function deleteGuardianAccountWithLifecycle( } return deleteAccount(tx); - }); + }, ACCOUNT_DELETION_TRANSACTION_OPTIONS); } /** @@ -217,7 +243,7 @@ export async function deleteManagedProfile(input: { await requestOffhostPurge(tx, profile.id, "managed_profile_deleted"); await tx.user.delete({ where: { id: profile.id } }); }); - }); + }, ACCOUNT_DELETION_TRANSACTION_OPTIONS); await kickOffhostPurge(); }