fix(cluster-h): wrap admin delete+moderation-log writes in a transaction
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
45af6b2a93
commit
215fba80d4
3 files changed
+159
-63
No files matched your search
@@ -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 });
|
||||
});
|
||||
});
|
||||
@@ -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(),
|
||||
|
||||
+73
-63
@@ -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 };
|
||||
}),
|
||||
|
||||
|
||||
Reference in new issue
Block a user