From 0a5fa4b2ae30d9ede9ede8b9ce757546e8319f14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 25 Sep 2026 09:17:25 +0200 Subject: [PATCH] fix(crypto): page the key-rotation walks by id, not by cursor The corpus walk and the rotation script paged with Prisma's cursor and skip. Prisma resolves that cursor by reading the page's last row again, so when that row was hard-deleted before the next page (a backup replaced, a cached response expired), the next page came back empty and the walk ended early. The scan then counted fewer rows than the database holds, which could read as nothing left on the old key while rows it never reached still were. Both walks now ask for the rows whose id is greater than the last one read, which needs no row to still exist. --- scripts/rotate-encryption-key.ts | 9 ++- .../rotation-walk-keyset-guard.test.ts | 34 +++++++++ .../__tests__/encryption-corpus-blob.test.ts | 17 ++--- .../__tests__/encryption-corpus.test.ts | 17 +++-- src/lib/crypto/encryption-corpus.ts | 7 +- ...cryption-corpus-cursor-row-deleted.test.ts | 74 +++++++++++++++++++ 6 files changed, 133 insertions(+), 25 deletions(-) create mode 100644 src/__tests__/rotation-walk-keyset-guard.test.ts create mode 100644 tests/integration/encryption-corpus-cursor-row-deleted.test.ts diff --git a/scripts/rotate-encryption-key.ts b/scripts/rotate-encryption-key.ts index 58a23e4f4..744436a92 100644 --- a/scripts/rotate-encryption-key.ts +++ b/scripts/rotate-encryption-key.ts @@ -114,8 +114,7 @@ interface PagedDelegate { select: Record; orderBy: { id: "asc" }; take: number; - cursor?: { id: string }; - skip?: number; + where?: { id: { gt: string } }; }) => Promise>>; update: (args: { where: { id: string }; @@ -142,7 +141,9 @@ async function* pagedRows( select: { id: true, [field]: true }, orderBy: { id: "asc" }, take: PAGE_SIZE, - ...(cursor ? { cursor: { id: cursor }, skip: 1 } : {}), + // `id > last`, not Prisma's `cursor` + `skip`: the cursor form looks + // the last row up again, and a row deleted between pages ends the walk. + ...(cursor ? { where: { id: { gt: cursor } } } : {}), }); if (rows.length === 0) return; yield* rows; @@ -786,7 +787,7 @@ async function main() { select: { id: true, textEncrypted: true, verbatimTextEncrypted: true }, orderBy: { id: "asc" }, take: 100, - ...(cursor ? { cursor: { id: cursor }, skip: 1 } : {}), + ...(cursor ? { where: { id: { gt: cursor } } } : {}), }); if (rows.length === 0) break; for (const row of rows) { diff --git a/src/__tests__/rotation-walk-keyset-guard.test.ts b/src/__tests__/rotation-walk-keyset-guard.test.ts new file mode 100644 index 000000000..ee5b53a5c --- /dev/null +++ b/src/__tests__/rotation-walk-keyset-guard.test.ts @@ -0,0 +1,34 @@ +/** + * The key-rotation walks page by `id > last`, never by Prisma's cursor. + * + * `cursor: { id }, skip: 1` looks the last row of a page up again. When that + * row was hard-deleted between pages, Prisma answers the next page with + * nothing, the walk stops early, and the scan reports a corpus smaller than + * the one on disk, which can read as "nothing left on the old key". The + * library walk is proven against Postgres in + * `tests/integration/encryption-corpus-cursor-row-deleted.test.ts`; the + * rotation script calls `process.exit` on import and cannot be driven from a + * test, so its walks are held to the same shape here. + * + * Mutation check: put `cursor: { id: cursor }, skip: 1` back in either file + * and this goes red. + */ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +const WALKS = [ + "scripts/rotate-encryption-key.ts", + "src/lib/crypto/encryption-corpus.ts", +] as const; + +describe("rotation walks page by key", () => { + it.each(WALKS)("%s pages with `id > last`, not cursor + skip", (path) => { + const code = readFileSync(path, "utf8") + .split("\n") + .filter((line) => !line.trim().startsWith("//")) + .join("\n"); + expect(code).not.toMatch(/cursor\s*:\s*\{[^}]*\}\s*,\s*skip\s*:/); + // Non-zero proof: the walk is still there and still pages by key. + expect(code).toMatch(/\{\s*gt\s*:\s*cursor\s*\}/); + }); +}); diff --git a/src/lib/crypto/__tests__/encryption-corpus-blob.test.ts b/src/lib/crypto/__tests__/encryption-corpus-blob.test.ts index 57cb3820e..c436f83b8 100644 --- a/src/lib/crypto/__tests__/encryption-corpus-blob.test.ts +++ b/src/lib/crypto/__tests__/encryption-corpus-blob.test.ts @@ -45,13 +45,12 @@ interface FindManyArgs { select: Record; orderBy?: Record; take?: number; - cursor?: Record; - skip?: number; + where?: Record; } /** - * Pagination-honouring fake delegate: applies orderBy(id) + cursor + skip + - * take exactly like Prisma keyset pagination, and records every findMany + * Pagination-honouring fake delegate: applies orderBy(id) + `id > last` + + * take exactly like the keyset pagination the walk sends, and records every findMany * call so the test can assert the batch boundedness. */ function makeDocClient(rows: DocRow[]): { @@ -68,10 +67,8 @@ function makeDocClient(rows: DocRow[]): { findMany: async (args: FindManyArgs) => { findManyCalls.push(args); let list = [...store].sort((a, b) => a.id.localeCompare(b.id)); - if (args.cursor) { - const idx = list.findIndex((r) => r.id === args.cursor!.id); - list = list.slice(idx + (args.skip ?? 0)); - } + const after = args.where?.id?.gt; + if (after !== undefined) list = list.filter((r) => r.id > after); if (args.take !== undefined) list = list.slice(0, args.take); return list.map((row) => { const out: Record = {}; @@ -181,8 +178,8 @@ describe("codec-dispatched blob rotation (bounded batches)", () => { expect(call.orderBy).toEqual({ id: "asc" }); } // Cursor advanced between pages. - expect(findManyCalls[0].cursor).toBeUndefined(); - expect(findManyCalls[1].cursor).toEqual({ id: pad(24) }); + expect(findManyCalls[0].where).toBeUndefined(); + expect(findManyCalls[1].where).toEqual({ id: { gt: pad(24) } }); // Every row now reads under the active key, each in its OWN codec. for (const row of store) { diff --git a/src/lib/crypto/__tests__/encryption-corpus.test.ts b/src/lib/crypto/__tests__/encryption-corpus.test.ts index db764a92e..4698853da 100644 --- a/src/lib/crypto/__tests__/encryption-corpus.test.ts +++ b/src/lib/crypto/__tests__/encryption-corpus.test.ts @@ -45,15 +45,16 @@ function makeClient(seed: Record): { if (client[key]) continue; const model = col.model; client[key] = { - // `take` / `cursor` are honoured so the batched-column walk terminates - // here the way it does against Postgres. - findMany: async ({ select, take, cursor, skip }) => { + // `take` / `id > last` are honoured so the batched-column walk + // terminates here the way it does against Postgres. + findMany: async ({ select, take, where }) => { const fields = Object.keys(select); - let rows = store[model]; - if (cursor) { - const at = rows.findIndex((r) => r.id === cursor.id); - rows = at < 0 ? [] : rows.slice(at + (skip ?? 0)); - } + let rows = [...store[model]].sort((a, b) => + String(a.id).localeCompare(String(b.id)), + ); + const after = where?.id?.gt; + if (after !== undefined) + rows = rows.filter((r) => String(r.id) > after); if (take !== undefined) rows = rows.slice(0, take); return rows.map((row) => { const out: Record = {}; diff --git a/src/lib/crypto/encryption-corpus.ts b/src/lib/crypto/encryption-corpus.ts index 4efc0d46d..82ebf6bfc 100644 --- a/src/lib/crypto/encryption-corpus.ts +++ b/src/lib/crypto/encryption-corpus.ts @@ -69,8 +69,7 @@ interface ColumnDelegate { select: Record; orderBy?: Record; take?: number; - cursor?: Record; - skip?: number; + where?: Record; }) => Promise>>; update: (args: { where: Record; @@ -190,7 +189,9 @@ async function walkColumn( }, orderBy: { [pk]: "asc" }, take: batchSize, - ...(cursor ? { cursor: { [pk]: cursor }, skip: 1 } : {}), + // `id > last`, not Prisma's `cursor` + `skip`: the cursor form looks + // the last row up again, and a row deleted between pages ends the walk. + ...(cursor ? { where: { [pk]: { gt: cursor } } } : {}), }); if (rows.length === 0) break; for (const row of rows) { diff --git a/tests/integration/encryption-corpus-cursor-row-deleted.test.ts b/tests/integration/encryption-corpus-cursor-row-deleted.test.ts new file mode 100644 index 000000000..0c1db9744 --- /dev/null +++ b/tests/integration/encryption-corpus-cursor-row-deleted.test.ts @@ -0,0 +1,74 @@ +/** + * A key-rotation walk that loses the row it paged from still reads the rest. + * + * The corpus walk and the rotation script paged with Prisma's + * `cursor: { id }, skip: 1`. Prisma resolves that cursor by looking the row up + * again, so when the row a page ended on was hard-deleted before the next + * page (a backup replaced, an idempotency entry expired), the next page came + * back empty. The walk ended early and the scan reported fewer rows, possibly + * "nothing left on the old key", while rows it never read were still under + * it. The walks page by `id > last id` now, which needs no row to exist. + * + * Mutation check: put the `cursor`/`skip` paging back in `walkColumn` and the + * scan counts one page of rows instead of all of them. + */ +import { beforeEach, describe, expect, it } from "vitest"; + +import { encrypt } from "@/lib/crypto"; +import { ENCRYPTED_COLUMNS } from "@/lib/crypto/encrypted-columns"; +import { + BLOB_ROTATION_BATCH_SIZE, + scanColumn, + type CorpusClient, +} from "@/lib/crypto/encryption-corpus"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); +}); + +describe("encryption corpus walk", () => { + it("reads every row even when the row a page ended on is deleted", async () => { + const prisma = getPrismaClient(); + const user = await prisma.user.create({ + data: { username: "cursor-walk", email: "cursor-walk@example.test" }, + }); + const count = BLOB_ROTATION_BATCH_SIZE + 5; + await prisma.dataBackup.createMany({ + data: Array.from({ length: count }, (_, i) => ({ + id: `backup-${String(i).padStart(3, "0")}`, + userId: user.id, + type: `MANUAL_UPLOAD_${i}`, + data: encrypt(`{"n":${i}}`), + })), + }); + + let pages = 0; + const client = { + dataBackup: { + findMany: async (args: never) => { + const rows = (await prisma.dataBackup.findMany(args)) as Array<{ + id: string; + }>; + pages += 1; + // The first page's last row disappears before the second is read. + if (pages === 1 && rows.length > 0) { + await prisma.dataBackup.delete({ where: { id: rows.at(-1)!.id } }); + } + return rows; + }, + update: (args: never) => prisma.dataBackup.update(args), + }, + } as unknown as CorpusClient; + + const col = ENCRYPTED_COLUMNS.find( + (c) => c.model === "DataBackup" && c.field === "data", + )!; + const scan = await scanColumn(client, col); + + // Every row was read once: the deleted one on the first page, the rest + // on the pages after it. + expect(scan.total).toBe(count); + expect(pages).toBeGreaterThan(1); + }); +});