From a7e98b980ee19a7ddd79547e2abcbe1453e09668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 11 Nov 2026 00:45:22 +0100 Subject: [PATCH] fix(cycle): audit the feeds a revoke actually turned off DELETE audited the feed it read before its two revoke passes. When a rotation committed in between, that feed was already revoked by the rotation and the second pass revoked its successor, so the trail showed the old feed revoked twice and the new one never. Read back the rows carrying this request's revokedAt stamp and audit each. --- .../__tests__/delete-audit.test.ts | 105 ++++++++++++++++++ src/app/api/cycle/feed-subscription/route.ts | 33 ++++-- 2 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 src/app/api/cycle/feed-subscription/__tests__/delete-audit.test.ts diff --git a/src/app/api/cycle/feed-subscription/__tests__/delete-audit.test.ts b/src/app/api/cycle/feed-subscription/__tests__/delete-audit.test.ts new file mode 100644 index 000000000..f5e0c264e --- /dev/null +++ b/src/app/api/cycle/feed-subscription/__tests__/delete-audit.test.ts @@ -0,0 +1,105 @@ +/** + * Turning the subscription off audits the feeds this request revoked, not + * the one it read first. A rotation that commits while the DELETE waits has + * already revoked that one and made a successor; the second pass revokes the + * successor, and the trail must name it. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const m = vi.hoisted(() => ({ + requireGuardianAuth: vi.fn(), + findLiveFeed: vi.fn(), + updateMany: vi.fn(), + findMany: vi.fn(), + audit: vi.fn(async () => undefined), +})); + +vi.mock("@/lib/api-handler", () => ({ + apiHandler: unknown>(fn: T) => fn, + requireGuardianAuth: m.requireGuardianAuth, + assertRecentCookieProof: vi.fn(), +})); +vi.mock("@/lib/cycle/feed/status", () => ({ + FEED_SUMMARY_SELECT: {}, + cycleFeedStatus: vi.fn(), + findLiveFeed: m.findLiveFeed, + toFeedSummary: vi.fn(), +})); +vi.mock("@/lib/db", () => ({ + prisma: { + cycleCalendarFeed: { updateMany: m.updateMany, findMany: m.findMany }, + }, +})); +vi.mock("@/lib/auth/audit", () => ({ auditLog: m.audit })); +vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); + +import { DELETE } from "../route"; + +const OLD = { + id: "feed-old", + tokenPrefix: "hlf_old", + createdAt: new Date("2026-11-01T00:00:00Z"), +}; +const SUCCESSOR = { + id: "feed-new", + tokenPrefix: "hlf_new", + createdAt: new Date("2026-11-10T00:00:00Z"), +}; + +beforeEach(() => { + vi.clearAllMocks(); + m.requireGuardianAuth.mockResolvedValue({ + authMethod: "cookie", + user: { id: "u1" }, + }); + m.findLiveFeed.mockResolvedValue(OLD); +}); + +describe("DELETE /api/cycle/feed-subscription", () => { + it("audits the successor a racing rotation made, not the feed read first", async () => { + // The rotation revoked OLD before the first pass, so the first pass + // revokes nothing and the second revokes the successor. + m.updateMany + .mockResolvedValueOnce({ count: 0 }) + .mockResolvedValueOnce({ count: 1 }); + m.findMany.mockResolvedValue([SUCCESSOR]); + + const res = await (DELETE as unknown as () => Promise)(); + expect(res.status).toBe(200); + + const stamp = m.updateMany.mock.calls[0]![0].data.revokedAt as Date; + expect(m.findMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: { userId: "u1", revokedAt: stamp }, + }), + ); + expect(m.audit).toHaveBeenCalledTimes(1); + expect(m.audit).toHaveBeenCalledWith( + "cycle.feed.revoke", + expect.objectContaining({ + userId: "u1", + details: expect.objectContaining({ feedId: SUCCESSOR.id }), + }), + ); + }); + + it("audits every feed the request revoked", async () => { + m.updateMany.mockResolvedValue({ count: 1 }); + m.findMany.mockResolvedValue([OLD, SUCCESSOR]); + + await (DELETE as unknown as () => Promise)(); + expect(m.audit.mock.calls.map((c) => c[1].details.feedId)).toEqual([ + OLD.id, + SUCCESSOR.id, + ]); + }); + + it("writes no revoke row when another request already turned it off", async () => { + m.updateMany.mockResolvedValue({ count: 0 }); + m.findMany.mockResolvedValue([]); + + const res = await (DELETE as unknown as () => Promise)(); + expect(res.status).toBe(200); + expect(m.audit).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/cycle/feed-subscription/route.ts b/src/app/api/cycle/feed-subscription/route.ts index a91f2d627..0acb4c30d 100644 --- a/src/app/api/cycle/feed-subscription/route.ts +++ b/src/app/api/cycle/feed-subscription/route.ts @@ -259,18 +259,29 @@ export const DELETE = apiHandler(async () => { // The link stops working on the provider's next poll. What the provider // already copied is beyond reach, which the settings card says before the // link is ever made. - await auditLog("cycle.feed.revoke", { - userId: user.id, - details: { - feedId: live.id, - ...destroyedDetails({ - model: "CycleCalendarFeed", - id: live.id, - label: live.tokenPrefix, - effectiveAt: live.createdAt, - }), - }, + // + // The trail names the feeds this request revoked, read back by the one + // stamp both passes wrote: a racing rotation may have revoked the feed + // read above itself, leaving its successor as the one turned off here. + const revoked = await prisma.cycleCalendarFeed.findMany({ + where: { userId: user.id, revokedAt }, + select: { id: true, tokenPrefix: true, createdAt: true }, + orderBy: { createdAt: "asc" }, }); + for (const feed of revoked) { + await auditLog("cycle.feed.revoke", { + userId: user.id, + details: { + feedId: feed.id, + ...destroyedDetails({ + model: "CycleCalendarFeed", + id: feed.id, + label: feed.tokenPrefix, + effectiveAt: feed.createdAt, + }), + }, + }); + } return apiSuccess({ revoked: true }); });