From 215fba80d42b4542fddd74eaa7057856620f97b6 Mon Sep 17 00:00:00 2001 From: neodk2004 Date: Thu, 16 Jul 2026 20:39:20 +0200 Subject: [PATCH] fix(cluster-h): wrap admin delete+moderation-log writes in a transaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deletePost, banUser, unbanUser, dismissReport, and deleteReportedPost each ran the mutation and the moderationLog.create as two separate sequential awaits. The audit flagged this as "should be Promise.all", but that would have introduced a real bug: if the delete/ban call failed, a concurrently fired log write could still succeed, recording an action that never happened. Wrapped both in $transaction instead (matching the pattern deleteComment already used) — atomic, and the log can only be written if the mutation actually succeeded. Reviewed the audit's other Cluster H finding (a few include-vs-select over-fetches) and found nothing actionable — the flagged spots already narrow relations with select where it matters; the unnarrowed ones are small lookup/relation tables used in full by their call sites. Co-Authored-By: Claude Sonnet 5 --- src/__tests__/admin.test.ts | 72 ++++++++++++++ src/__tests__/helpers/prisma-mock.ts | 14 +++ src/trpc/routers/admin.ts | 136 ++++++++++++++------------- 3 files changed, 159 insertions(+), 63 deletions(-) diff --git a/src/__tests__/admin.test.ts b/src/__tests__/admin.test.ts index f261bb4..b49298d 100644 --- a/src/__tests__/admin.test.ts +++ b/src/__tests__/admin.test.ts @@ -95,3 +95,75 @@ describe("admin.deleteComment", () => { expect(mockPrisma.comment.delete).not.toHaveBeenCalled(); }); }); + +// Cluster H: deletePost/banUser/unbanUser/dismissReport/deleteReportedPost +// now wrap the mutation + moderation-log write in $transaction so a failed +// mutation can never leave behind a log entry describing an action that +// didn't actually happen. +describe("admin transactional moderation actions", () => { + it("deletePost wraps delete + log in a transaction", async () => { + vi.stubEnv("ADMIN_OWNER_ID", "admin-1"); + const { caller, mockPrisma } = await createAdminCaller(); + mockPrisma.post.delete.mockResolvedValue({ id: "post-1" }); + mockPrisma.moderationLog.create.mockResolvedValue({ id: "log-1" }); + + const result = await caller.admin.deletePost({ postId: "post-1", reason: "spam" }); + + expect(mockPrisma.$transaction).toHaveBeenCalledWith(expect.any(Array)); + expect(mockPrisma.post.delete).toHaveBeenCalledWith({ where: { id: "post-1" } }); + expect(result).toEqual({ success: true }); + }); + + it("banUser wraps upsert + log in a transaction", async () => { + vi.stubEnv("ADMIN_OWNER_ID", "admin-1"); + const { caller, mockPrisma } = await createAdminCaller(); + mockPrisma.userBan.upsert.mockResolvedValue({ ownerId: "owner-2" }); + mockPrisma.moderationLog.create.mockResolvedValue({ id: "log-1" }); + + const result = await caller.admin.banUser({ ownerId: "owner-2", reason: "abuse" }); + + expect(mockPrisma.$transaction).toHaveBeenCalledWith(expect.any(Array)); + expect(mockPrisma.userBan.upsert).toHaveBeenCalled(); + expect(result).toEqual({ success: true }); + }); + + it("unbanUser wraps delete + log in a transaction", async () => { + vi.stubEnv("ADMIN_OWNER_ID", "admin-1"); + const { caller, mockPrisma } = await createAdminCaller(); + mockPrisma.userBan.delete.mockResolvedValue({ ownerId: "owner-2" }); + mockPrisma.moderationLog.create.mockResolvedValue({ id: "log-1" }); + + const result = await caller.admin.unbanUser({ ownerId: "owner-2" }); + + expect(mockPrisma.$transaction).toHaveBeenCalledWith(expect.any(Array)); + expect(mockPrisma.userBan.delete).toHaveBeenCalledWith({ where: { ownerId: "owner-2" } }); + expect(result).toEqual({ success: true }); + }); + + it("dismissReport wraps delete + log in a transaction", async () => { + vi.stubEnv("ADMIN_OWNER_ID", "admin-1"); + const { caller, mockPrisma } = await createAdminCaller(); + mockPrisma.report.findUnique.mockResolvedValue({ id: "report-1" }); + mockPrisma.report.delete.mockResolvedValue({ id: "report-1" }); + mockPrisma.moderationLog.create.mockResolvedValue({ id: "log-1" }); + + const result = await caller.admin.dismissReport({ reportId: "report-1" }); + + expect(mockPrisma.$transaction).toHaveBeenCalledWith(expect.any(Array)); + expect(mockPrisma.report.delete).toHaveBeenCalledWith({ where: { id: "report-1" } }); + expect(result).toEqual({ success: true }); + }); + + it("deleteReportedPost wraps delete + log in a transaction", async () => { + vi.stubEnv("ADMIN_OWNER_ID", "admin-1"); + const { caller, mockPrisma } = await createAdminCaller(); + mockPrisma.post.delete.mockResolvedValue({ id: "post-1" }); + mockPrisma.moderationLog.create.mockResolvedValue({ id: "log-1" }); + + const result = await caller.admin.deleteReportedPost({ postId: "post-1" }); + + expect(mockPrisma.$transaction).toHaveBeenCalledWith(expect.any(Array)); + expect(mockPrisma.post.delete).toHaveBeenCalledWith({ where: { id: "post-1" } }); + expect(result).toEqual({ success: true }); + }); +}); diff --git a/src/__tests__/helpers/prisma-mock.ts b/src/__tests__/helpers/prisma-mock.ts index 32dc70a..d9b1ace 100644 --- a/src/__tests__/helpers/prisma-mock.ts +++ b/src/__tests__/helpers/prisma-mock.ts @@ -116,9 +116,16 @@ interface MockReportClient { create: MockFn; findMany: MockFn; findUnique: MockFn; + delete: MockFn; count: MockFn; } +interface MockUserBanClient { + upsert: MockFn; + delete: MockFn; + findUnique: MockFn; +} + // ─── Phase 3 mock interfaces ────────────────────────────────────────────────── interface MockReactionClient { @@ -207,6 +214,7 @@ interface MockPrismaClient { follow: MockFollowClient; block: MockBlockClient; report: MockReportClient; + userBan: MockUserBanClient; // Phase 3 reaction: MockReactionClient; comment: MockCommentClient; @@ -316,8 +324,14 @@ export function createMockPrisma(): MockPrismaClient { create: vi.fn(), findMany: vi.fn(), findUnique: vi.fn(), + delete: vi.fn(), count: vi.fn(), }, + userBan: { + upsert: vi.fn(), + delete: vi.fn(), + findUnique: vi.fn(), + }, // Phase 3 mocks reaction: { findUnique: vi.fn(), diff --git a/src/trpc/routers/admin.ts b/src/trpc/routers/admin.ts index 7587369..ced0dd0 100644 --- a/src/trpc/routers/admin.ts +++ b/src/trpc/routers/admin.ts @@ -115,16 +115,18 @@ export const adminRouter = router({ .input(z.object({ postId: z.string().min(1), reason: z.string().max(500).optional() })) .mutation(async ({ ctx, input }) => { await assertAdmin(ctx.prisma, ctx.userId); - await ctx.prisma.post.delete({ where: { id: input.postId } }); - await ctx.prisma.moderationLog.create({ - data: { - action: "DELETE_POST", - targetType: "POST", - targetId: input.postId, - reason: input.reason ?? null, - moderatorId: ctx.userId, - }, - }); + await ctx.prisma.$transaction([ + ctx.prisma.post.delete({ where: { id: input.postId } }), + ctx.prisma.moderationLog.create({ + data: { + action: "DELETE_POST", + targetType: "POST", + targetId: input.postId, + reason: input.reason ?? null, + moderatorId: ctx.userId, + }, + }), + ]); return { success: true }; }), @@ -195,30 +197,32 @@ export const adminRouter = router({ await assertAdmin(ctx.prisma, ctx.userId); if (input.ownerId === ctx.userId) throw new TRPCError({ code: "BAD_REQUEST", message: "Cannot ban yourself." }); - await ctx.prisma.userBan.upsert({ - where: { ownerId: input.ownerId }, - create: { - ownerId: input.ownerId, - reason: input.reason ?? null, - bannedBy: ctx.userId, - expiresAt: input.expiresAt ? new Date(input.expiresAt) : null, - }, - update: { - reason: input.reason ?? null, - bannedBy: ctx.userId, - bannedAt: new Date(), - expiresAt: input.expiresAt ? new Date(input.expiresAt) : null, - }, - }); - await ctx.prisma.moderationLog.create({ - data: { - action: "BAN_USER", - targetType: "USER", - targetId: input.ownerId, - reason: input.reason ?? null, - moderatorId: ctx.userId, - }, - }); + await ctx.prisma.$transaction([ + ctx.prisma.userBan.upsert({ + where: { ownerId: input.ownerId }, + create: { + ownerId: input.ownerId, + reason: input.reason ?? null, + bannedBy: ctx.userId, + expiresAt: input.expiresAt ? new Date(input.expiresAt) : null, + }, + update: { + reason: input.reason ?? null, + bannedBy: ctx.userId, + bannedAt: new Date(), + expiresAt: input.expiresAt ? new Date(input.expiresAt) : null, + }, + }), + ctx.prisma.moderationLog.create({ + data: { + action: "BAN_USER", + targetType: "USER", + targetId: input.ownerId, + reason: input.reason ?? null, + moderatorId: ctx.userId, + }, + }), + ]); return { success: true }; }), @@ -226,16 +230,18 @@ export const adminRouter = router({ .input(z.object({ ownerId: z.string().min(1) })) .mutation(async ({ ctx, input }) => { await assertAdmin(ctx.prisma, ctx.userId); - await ctx.prisma.userBan.delete({ where: { ownerId: input.ownerId } }); - await ctx.prisma.moderationLog.create({ - data: { - action: "UNBAN_USER", - targetType: "USER", - targetId: input.ownerId, - reason: null, - moderatorId: ctx.userId, - }, - }); + await ctx.prisma.$transaction([ + ctx.prisma.userBan.delete({ where: { ownerId: input.ownerId } }), + ctx.prisma.moderationLog.create({ + data: { + action: "UNBAN_USER", + targetType: "USER", + targetId: input.ownerId, + reason: null, + moderatorId: ctx.userId, + }, + }), + ]); return { success: true }; }), @@ -290,15 +296,17 @@ export const adminRouter = router({ await assertAdmin(ctx.prisma, ctx.userId); const report = await ctx.prisma.report.findUnique({ where: { id: input.reportId } }); if (!report) throw new TRPCError({ code: "NOT_FOUND" }); - await ctx.prisma.report.delete({ where: { id: input.reportId } }); - await ctx.prisma.moderationLog.create({ - data: { - action: "DISMISS_REPORT", - targetType: "REPORT", - targetId: input.reportId, - moderatorId: ctx.userId, - }, - }); + await ctx.prisma.$transaction([ + ctx.prisma.report.delete({ where: { id: input.reportId } }), + ctx.prisma.moderationLog.create({ + data: { + action: "DISMISS_REPORT", + targetType: "REPORT", + targetId: input.reportId, + moderatorId: ctx.userId, + }, + }), + ]); return { success: true }; }), @@ -307,16 +315,18 @@ export const adminRouter = router({ .mutation(async ({ ctx, input }) => { await assertAdmin(ctx.prisma, ctx.userId); // Delete the post (cascades to reports, comments, reactions via Prisma schema) - await ctx.prisma.post.delete({ where: { id: input.postId } }); - await ctx.prisma.moderationLog.create({ - data: { - action: "DELETE_POST", - targetType: "POST", - targetId: input.postId, - reason: input.reason ?? null, - moderatorId: ctx.userId, - }, - }); + await ctx.prisma.$transaction([ + ctx.prisma.post.delete({ where: { id: input.postId } }), + ctx.prisma.moderationLog.create({ + data: { + action: "DELETE_POST", + targetType: "POST", + targetId: input.postId, + reason: input.reason ?? null, + moderatorId: ctx.userId, + }, + }), + ]); return { success: true }; }),