fix(security): 4 CRITICAL authorization bugs from 2026-07-21 audit rerun

- health.ts: deleteWeightLog/deleteVetVisit/deleteVaccine checked pet
  ownership but deleted the target row by id alone, never verifying it
  belonged to that pet — any owner could delete another pet's health
  records. Fixed with fetch-then-check (same pattern as comments.ts).
- notifications.ts: list/unreadCount/markAllRead/markRead had zero
  assertPetOwnership calls — any owner could read or clear another
  pet's notification inbox by supplying its (publicly-visible) petId.
- ads.ts: toggleReaction/addComment/toggleRepost never verified petId
  ownership, unlike the near-identical post reactions/comments/reposts
  routers — allowed attributing ad interactions to arbitrary pets.
- FollowButton.tsx: early return before 3 hooks violated Rules of
  Hooks, crashing React when the active pet switched to one already
  rendered in the same list/grid (explore, followers list).

Added regression tests for all 4 (health.ts and notifications.ts had
zero test coverage before this — not a coincidence, per the audit).
This commit is contained in:
admin committed 2026-07-22 18:21:39 +02:00
1 parent b3fa42b14a
commit 03abe472da
8 files changed
+377 -5

No files matched your search

+19
View File
@@ -72,10 +72,25 @@ describe("ads.getFeedData", () => {
});
});
describe("ads.toggleReaction — ownership", () => {
it("throws FORBIDDEN when petId does not belong to the caller (2026-07-21 audit fix)", async () => {
const { caller, mockPrisma } = await createAdsCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "victim-pet", ownerId: "owner-2" });
await expect(
caller.ads.toggleReaction({ adId: "ad-1", petId: "victim-pet" })
).rejects.toMatchObject({ code: "FORBIDDEN" });
expect(mockPrisma.adReaction.create).not.toHaveBeenCalled();
});
});
describe("ads.toggleReaction", () => {
it("creates a reaction and increments adReactionCount", async () => {
const { caller, mockPrisma } = await createAdsCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "pet-1", ownerId: "owner-1" });
mockPrisma.adReaction.findUnique.mockResolvedValue(null);
mockPrisma.adReaction.create.mockResolvedValue({ id: "adrxn-1" });
mockPrisma.advertisement.update.mockResolvedValue({ adReactionCount: 5 });
@@ -97,6 +112,7 @@ describe("ads.toggleReaction", () => {
it("removes an existing reaction and decrements adReactionCount", async () => {
const { caller, mockPrisma } = await createAdsCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "pet-1", ownerId: "owner-1" });
mockPrisma.adReaction.findUnique.mockResolvedValue({ id: "adrxn-1" });
mockPrisma.adReaction.delete.mockResolvedValue({ id: "adrxn-1" });
mockPrisma.advertisement.update.mockResolvedValue({ adReactionCount: 4 });
@@ -118,6 +134,7 @@ describe("ads.addComment", () => {
it("creates a comment and increments adCommentCount", async () => {
const { caller, mockPrisma } = await createAdsCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "pet-1", ownerId: "owner-1" });
mockPrisma.adComment.create.mockResolvedValue({
id: "adcmt-1",
adId: "ad-1",
@@ -143,6 +160,7 @@ describe("ads.toggleRepost", () => {
it("creates a repost and increments adRepostCount", async () => {
const { caller, mockPrisma } = await createAdsCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "pet-1", ownerId: "owner-1" });
mockPrisma.adRepost.findUnique.mockResolvedValue(null);
mockPrisma.adRepost.create.mockResolvedValue({ id: "adrep-1" });
mockPrisma.advertisement.update.mockResolvedValue({ adRepostCount: 2 });
@@ -164,6 +182,7 @@ describe("ads.toggleRepost", () => {
it("removes an existing repost and decrements adRepostCount", async () => {
const { caller, mockPrisma } = await createAdsCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "pet-1", ownerId: "owner-1" });
mockPrisma.adRepost.findUnique.mockResolvedValue({ id: "adrep-1" });
mockPrisma.adRepost.delete.mockResolvedValue({ id: "adrep-1" });
mockPrisma.advertisement.update.mockResolvedValue({ adRepostCount: 1 });
+125
View File
@@ -0,0 +1,125 @@
/**
* Health Router Unit Tests — regression coverage for the 2026-07-21 audit IDOR fix.
*
* deleteWeightLog/deleteVetVisit/deleteVaccine previously verified that the
* caller owns `input.petId`, but deleted the target row by `id` alone —
* never checking that the row actually belongs to that pet. Any owner could
* delete another pet's health record by pairing its `id` with one of their
* own pets' `petId`. Fixed by fetching the record first and checking
* `record.petId !== input.petId` before deleting (same pattern as
* comments.ts's delete).
*/
import { describe, it, expect, vi } from "vitest";
vi.mock("server-only", () => ({}));
vi.mock("@clerk/nextjs/server", () => ({
auth: vi.fn().mockResolvedValue({ userId: "owner-1" }),
}));
vi.mock("@/lib/prisma", () => ({ prisma: {} }));
import { createMockPrisma } from "@/__tests__/helpers/prisma-mock";
async function createHealthCaller(overrides?: {
userId?: string | null;
prisma?: ReturnType<typeof createMockPrisma>;
}) {
const { healthRouter } = await import("@/trpc/routers/health");
const { initTRPC } = await import("@trpc/server");
const mockPrisma = overrides?.prisma ?? createMockPrisma();
const userId = overrides?.userId !== undefined ? overrides.userId : "owner-1";
const t = initTRPC
.context<{ userId: string | null; prisma: typeof mockPrisma }>()
.create();
const testRouter = t.router({ health: healthRouter });
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const caller = testRouter.createCaller({ userId, prisma: mockPrisma as any });
return { caller, mockPrisma };
}
describe("health.deleteWeightLog", () => {
it("throws FORBIDDEN when the record belongs to a different pet than input.petId", async () => {
const { caller, mockPrisma } = await createHealthCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "my-pet", ownerId: "owner-1" });
mockPrisma.weightLog.findUnique.mockResolvedValue({ id: "log-1", petId: "victim-pet" });
await expect(
caller.health.deleteWeightLog({ id: "log-1", petId: "my-pet" })
).rejects.toMatchObject({ code: "FORBIDDEN" });
expect(mockPrisma.weightLog.delete).not.toHaveBeenCalled();
});
it("deletes the record when it belongs to the owned pet", async () => {
const { caller, mockPrisma } = await createHealthCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "my-pet", ownerId: "owner-1" });
mockPrisma.weightLog.findUnique.mockResolvedValue({ id: "log-1", petId: "my-pet" });
mockPrisma.weightLog.delete.mockResolvedValue({ id: "log-1" });
const result = await caller.health.deleteWeightLog({ id: "log-1", petId: "my-pet" });
expect(mockPrisma.weightLog.delete).toHaveBeenCalledWith({ where: { id: "log-1" } });
expect(result).toEqual({ success: true });
});
});
describe("health.deleteVetVisit", () => {
it("throws FORBIDDEN when the record belongs to a different pet than input.petId", async () => {
const { caller, mockPrisma } = await createHealthCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "my-pet", ownerId: "owner-1" });
mockPrisma.vetVisit.findUnique.mockResolvedValue({ id: "visit-1", petId: "victim-pet" });
await expect(
caller.health.deleteVetVisit({ id: "visit-1", petId: "my-pet" })
).rejects.toMatchObject({ code: "FORBIDDEN" });
expect(mockPrisma.vetVisit.delete).not.toHaveBeenCalled();
});
it("deletes the record when it belongs to the owned pet", async () => {
const { caller, mockPrisma } = await createHealthCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "my-pet", ownerId: "owner-1" });
mockPrisma.vetVisit.findUnique.mockResolvedValue({ id: "visit-1", petId: "my-pet" });
mockPrisma.vetVisit.delete.mockResolvedValue({ id: "visit-1" });
const result = await caller.health.deleteVetVisit({ id: "visit-1", petId: "my-pet" });
expect(mockPrisma.vetVisit.delete).toHaveBeenCalledWith({ where: { id: "visit-1" } });
expect(result).toEqual({ success: true });
});
});
describe("health.deleteVaccine", () => {
it("throws FORBIDDEN when the record belongs to a different pet than input.petId", async () => {
const { caller, mockPrisma } = await createHealthCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "my-pet", ownerId: "owner-1" });
mockPrisma.vaccine.findUnique.mockResolvedValue({ id: "vax-1", petId: "victim-pet" });
await expect(
caller.health.deleteVaccine({ id: "vax-1", petId: "my-pet" })
).rejects.toMatchObject({ code: "FORBIDDEN" });
expect(mockPrisma.vaccine.delete).not.toHaveBeenCalled();
});
it("deletes the record when it belongs to the owned pet", async () => {
const { caller, mockPrisma } = await createHealthCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "my-pet", ownerId: "owner-1" });
mockPrisma.vaccine.findUnique.mockResolvedValue({ id: "vax-1", petId: "my-pet" });
mockPrisma.vaccine.delete.mockResolvedValue({ id: "vax-1" });
const result = await caller.health.deleteVaccine({ id: "vax-1", petId: "my-pet" });
expect(mockPrisma.vaccine.delete).toHaveBeenCalledWith({ where: { id: "vax-1" } });
expect(result).toEqual({ success: true });
});
});
+50
View File
@@ -203,6 +203,31 @@ interface MockAdminRoleClient {
interface MockNotificationClient {
create: MockFn;
findMany: MockFn;
findUnique: MockFn;
count: MockFn;
updateMany: MockFn;
}
interface MockWeightLogClient {
findMany: MockFn;
findUnique: MockFn;
create: MockFn;
delete: MockFn;
}
interface MockVetVisitClient {
findMany: MockFn;
findUnique: MockFn;
create: MockFn;
delete: MockFn;
}
interface MockVaccineClient {
findMany: MockFn;
findUnique: MockFn;
create: MockFn;
delete: MockFn;
}
interface MockPrismaClient {
@@ -235,6 +260,9 @@ interface MockPrismaClient {
adminRole: MockAdminRoleClient;
notification: MockNotificationClient;
legalDocumentVersion: MockLegalDocumentVersionClient;
weightLog: MockWeightLogClient;
vetVisit: MockVetVisitClient;
vaccine: MockVaccineClient;
$transaction: MockFn;
$disconnect: MockFn;
}
@@ -405,6 +433,28 @@ export function createMockPrisma(): MockPrismaClient {
// need to configure this every time.
notification: {
create: vi.fn().mockResolvedValue({}),
findMany: vi.fn(),
findUnique: vi.fn(),
count: vi.fn(),
updateMany: vi.fn(),
},
weightLog: {
findMany: vi.fn(),
findUnique: vi.fn(),
create: vi.fn(),
delete: vi.fn(),
},
vetVisit: {
findMany: vi.fn(),
findUnique: vi.fn(),
create: vi.fn(),
delete: vi.fn(),
},
vaccine: {
findMany: vi.fn(),
findUnique: vi.fn(),
create: vi.fn(),
delete: vi.fn(),
},
$transaction: vi.fn(),
$disconnect: vi.fn(),
+138
View File
@@ -0,0 +1,138 @@
/**
* Notifications Router Unit Tests — regression coverage for the 2026-07-21
* audit finding: list/unreadCount/markAllRead/markRead accepted a
* client-supplied petId (or, for markRead, a notificationId) with zero
* ownership verification. Any authenticated owner could read or clear
* another pet's notification inbox. Fixed by adding assertPetOwnership
* (and, for markRead, loading the notification first to check its
* recipientPetId belongs to the caller).
*/
import { describe, it, expect, vi } from "vitest";
vi.mock("server-only", () => ({}));
vi.mock("@clerk/nextjs/server", () => ({
auth: vi.fn().mockResolvedValue({ userId: "owner-1" }),
}));
vi.mock("@/lib/prisma", () => ({ prisma: {} }));
import { createMockPrisma } from "@/__tests__/helpers/prisma-mock";
async function createNotificationsCaller(overrides?: {
userId?: string | null;
prisma?: ReturnType<typeof createMockPrisma>;
}) {
const { notificationsRouter } = await import("@/trpc/routers/notifications");
const { initTRPC } = await import("@trpc/server");
const mockPrisma = overrides?.prisma ?? createMockPrisma();
const userId = overrides?.userId !== undefined ? overrides.userId : "owner-1";
const t = initTRPC
.context<{ userId: string | null; prisma: typeof mockPrisma }>()
.create();
const testRouter = t.router({ notifications: notificationsRouter });
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const caller = testRouter.createCaller({ userId, prisma: mockPrisma as any });
return { caller, mockPrisma };
}
describe("notifications.list", () => {
it("throws FORBIDDEN when petId does not belong to the caller", async () => {
const { caller, mockPrisma } = await createNotificationsCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "victim-pet", ownerId: "owner-2" });
await expect(
caller.notifications.list({ petId: "victim-pet" })
).rejects.toMatchObject({ code: "FORBIDDEN" });
expect(mockPrisma.notification.findMany).not.toHaveBeenCalled();
});
it("returns notifications when petId belongs to the caller", async () => {
const { caller, mockPrisma } = await createNotificationsCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "my-pet", ownerId: "owner-1" });
mockPrisma.notification.findMany.mockResolvedValue([]);
const result = await caller.notifications.list({ petId: "my-pet" });
expect(result).toEqual({ notifications: [], nextCursor: null });
});
});
describe("notifications.unreadCount", () => {
it("throws FORBIDDEN when petId does not belong to the caller", async () => {
const { caller, mockPrisma } = await createNotificationsCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "victim-pet", ownerId: "owner-2" });
await expect(
caller.notifications.unreadCount({ petId: "victim-pet" })
).rejects.toMatchObject({ code: "FORBIDDEN" });
expect(mockPrisma.notification.count).not.toHaveBeenCalled();
});
});
describe("notifications.markAllRead", () => {
it("throws FORBIDDEN when petId does not belong to the caller", async () => {
const { caller, mockPrisma } = await createNotificationsCaller();
mockPrisma.pet.findUnique.mockResolvedValue({ id: "victim-pet", ownerId: "owner-2" });
await expect(
caller.notifications.markAllRead({ petId: "victim-pet" })
).rejects.toMatchObject({ code: "FORBIDDEN" });
expect(mockPrisma.notification.updateMany).not.toHaveBeenCalled();
});
});
describe("notifications.markRead", () => {
it("throws FORBIDDEN when the notification's recipientPetId does not belong to the caller", async () => {
const { caller, mockPrisma } = await createNotificationsCaller();
mockPrisma.notification.findUnique.mockResolvedValue({
id: "notif-1",
recipientPetId: "victim-pet",
});
mockPrisma.pet.findUnique.mockResolvedValue({ id: "victim-pet", ownerId: "owner-2" });
await expect(
caller.notifications.markRead({ notificationId: "notif-1" })
).rejects.toMatchObject({ code: "FORBIDDEN" });
expect(mockPrisma.notification.updateMany).not.toHaveBeenCalled();
});
it("throws NOT_FOUND when the notification does not exist", async () => {
const { caller, mockPrisma } = await createNotificationsCaller();
mockPrisma.notification.findUnique.mockResolvedValue(null);
await expect(
caller.notifications.markRead({ notificationId: "missing" })
).rejects.toMatchObject({ code: "NOT_FOUND" });
});
it("marks the notification read when recipientPetId belongs to the caller", async () => {
const { caller, mockPrisma } = await createNotificationsCaller();
mockPrisma.notification.findUnique.mockResolvedValue({
id: "notif-1",
recipientPetId: "my-pet",
});
mockPrisma.pet.findUnique.mockResolvedValue({ id: "my-pet", ownerId: "owner-1" });
mockPrisma.notification.updateMany.mockResolvedValue({ count: 1 });
await caller.notifications.markRead({ notificationId: "notif-1" });
expect(mockPrisma.notification.updateMany).toHaveBeenCalledWith({
where: { id: "notif-1" },
data: { read: true },
});
});
});
+8 -5
View File
@@ -57,17 +57,17 @@ export function FollowButton({
const queryClient = useQueryClient();
const [showUnfollowDialog, setShowUnfollowDialog] = useState(false);
// Hidden on own pet profile (T-02-11 guard)
if (followerPetId === followeePetId) return null;
const isOwnPet = followerPetId === followeePetId;
const isFollowingQueryOptions = trpc.follows.isFollowing.queryOptions({
followerPetId,
followeePetId,
});
const { data: isFollowing, isPending: isLoadingStatus } = useQuery(
isFollowingQueryOptions
);
const { data: isFollowing, isPending: isLoadingStatus } = useQuery({
...isFollowingQueryOptions,
enabled: !isOwnPet,
});
const followMutation = useMutation(
trpc.follows.create.mutationOptions({
@@ -123,6 +123,9 @@ export function FollowButton({
const isPending = followMutation.isPending || unfollowMutation.isPending;
// Hidden on own pet profile (T-02-11 guard) — after all hooks, per Rules of Hooks
if (isOwnPet) return null;
function handleClick() {
if (isPending || isLoadingStatus) return;
+7
View File
@@ -1,6 +1,7 @@
import "server-only";
import { z } from "zod";
import { router, protectedProcedure } from "@/trpc/init";
import { assertPetOwnership } from "@/lib/assert-pet-ownership";
export const adsRouter = router({
// Returns reaction/comment/repost counts + whether the given pet has interacted
@@ -33,6 +34,8 @@ export const adsRouter = router({
toggleReaction: protectedProcedure
.input(z.object({ adId: z.string().min(1), petId: z.string().min(1) }))
.mutation(async ({ ctx, input }) => {
await assertPetOwnership(ctx.prisma, input.petId, ctx.userId);
const existing = await ctx.prisma.adReaction.findUnique({
where: { adId_petId: { adId: input.adId, petId: input.petId } },
});
@@ -81,6 +84,8 @@ export const adsRouter = router({
})
)
.mutation(async ({ ctx, input }) => {
await assertPetOwnership(ctx.prisma, input.petId, ctx.userId);
const [comment] = await ctx.prisma.$transaction([
ctx.prisma.adComment.create({
data: { adId: input.adId, petId: input.petId, body: input.body },
@@ -99,6 +104,8 @@ export const adsRouter = router({
toggleRepost: protectedProcedure
.input(z.object({ adId: z.string().min(1), petId: z.string().min(1) }))
.mutation(async ({ ctx, input }) => {
await assertPetOwnership(ctx.prisma, input.petId, ctx.userId);
const existing = await ctx.prisma.adRepost.findUnique({
where: { adId_petId: { adId: input.adId, petId: input.petId } },
});
+16
View File
@@ -1,4 +1,5 @@
import { z } from "zod";
import { TRPCError } from "@trpc/server";
import { router, protectedProcedure } from "@/trpc/init";
import { assertPetOwnership as assertOwner } from "@/lib/assert-pet-ownership";
@@ -39,6 +40,11 @@ export const healthRouter = router({
.input(z.object({ id: z.string().min(1), petId: z.string().min(1) }))
.mutation(async ({ ctx, input }) => {
await assertOwner(ctx.prisma, input.petId, ctx.userId);
const log = await ctx.prisma.weightLog.findUnique({ where: { id: input.id } });
if (!log) throw new TRPCError({ code: "NOT_FOUND" });
if (log.petId !== input.petId) {
throw new TRPCError({ code: "FORBIDDEN", message: "This record does not belong to that pet." });
}
await ctx.prisma.weightLog.delete({ where: { id: input.id } });
return { success: true };
}),
@@ -81,6 +87,11 @@ export const healthRouter = router({
.input(z.object({ id: z.string().min(1), petId: z.string().min(1) }))
.mutation(async ({ ctx, input }) => {
await assertOwner(ctx.prisma, input.petId, ctx.userId);
const visit = await ctx.prisma.vetVisit.findUnique({ where: { id: input.id } });
if (!visit) throw new TRPCError({ code: "NOT_FOUND" });
if (visit.petId !== input.petId) {
throw new TRPCError({ code: "FORBIDDEN", message: "This record does not belong to that pet." });
}
await ctx.prisma.vetVisit.delete({ where: { id: input.id } });
return { success: true };
}),
@@ -123,6 +134,11 @@ export const healthRouter = router({
.input(z.object({ id: z.string().min(1), petId: z.string().min(1) }))
.mutation(async ({ ctx, input }) => {
await assertOwner(ctx.prisma, input.petId, ctx.userId);
const vaccine = await ctx.prisma.vaccine.findUnique({ where: { id: input.id } });
if (!vaccine) throw new TRPCError({ code: "NOT_FOUND" });
if (vaccine.petId !== input.petId) {
throw new TRPCError({ code: "FORBIDDEN", message: "This record does not belong to that pet." });
}
await ctx.prisma.vaccine.delete({ where: { id: input.id } });
return { success: true };
}),
+14
View File
@@ -1,6 +1,8 @@
import "server-only";
import { z } from "zod";
import { TRPCError } from "@trpc/server";
import { router, protectedProcedure } from "@/trpc/init";
import { assertPetOwnership } from "@/lib/assert-pet-ownership";
const PAGE_SIZE = 30;
@@ -14,6 +16,8 @@ export const notificationsRouter = router({
})
)
.query(async ({ ctx, input }) => {
await assertPetOwnership(ctx.prisma, input.petId, ctx.userId);
const notifications = await ctx.prisma.notification.findMany({
where: {
recipientPetId: input.petId,
@@ -38,6 +42,8 @@ export const notificationsRouter = router({
unreadCount: protectedProcedure
.input(z.object({ petId: z.string().min(1) }))
.query(async ({ ctx, input }) => {
await assertPetOwnership(ctx.prisma, input.petId, ctx.userId);
const count = await ctx.prisma.notification.count({
where: { recipientPetId: input.petId, read: false },
});
@@ -47,6 +53,12 @@ export const notificationsRouter = router({
markRead: protectedProcedure
.input(z.object({ notificationId: z.string().min(1) }))
.mutation(async ({ ctx, input }) => {
const notification = await ctx.prisma.notification.findUnique({
where: { id: input.notificationId },
});
if (!notification) throw new TRPCError({ code: "NOT_FOUND" });
await assertPetOwnership(ctx.prisma, notification.recipientPetId, ctx.userId);
await ctx.prisma.notification.updateMany({
where: { id: input.notificationId },
data: { read: true },
@@ -56,6 +68,8 @@ export const notificationsRouter = router({
markAllRead: protectedProcedure
.input(z.object({ petId: z.string().min(1) }))
.mutation(async ({ ctx, input }) => {
await assertPetOwnership(ctx.prisma, input.petId, ctx.userId);
await ctx.prisma.notification.updateMany({
where: { recipientPetId: input.petId, read: false },
data: { read: true },