Scope the checkins collection group, and serve pet history from a callable

firestore.rules

@@ -570,8 +570,36 @@ service cloud.firestore {
     }
 
     // ========= Collection Group queries =========
+    // Scoped to the requesting user's own check-ins.
+    //
+    // Open, this collection group answered "every check-in matching a filter,
+    // across all locations", and the (userId, createdAt) and (petId, createdAt)
+    // collection-group indexes both exist, so it was servable. That turned any
+    // uid into a movement timeline — every place a person has physically been,
+    // with timestamps — for an unauthenticated caller, with no rate limit.
+    //
+    // Scoping by userId closes BOTH harvesting paths, not just the obvious one.
+    // A petId-filtered collection-group query can return documents belonging to
+    // other people, so Firestore refuses it under this rule — which matters,
+    // because pets/{petId} is world-readable and carries ownerId, putting the
+    // same timeline one hop away. The pet profile now reads its history through
+    // getPetCheckinsCallable instead, which stays public but is capped and rate
+    // limited.
+    //
+    // Reading your OWN history stays a direct query: Profile's check-ins tab
+    // does exactly that (src/services/checkins.ts getUserCheckins, called with
+    // the signed-in uid), and there is no reason to route a user's own data
+    // through a callable.
+    //
+    // A location's check-ins are still world-readable through
+    // locations/{locationId}/checkins above. That is the per-place view, not a
+    // per-person one, and it is unchanged.
+    //
+    // The exposure was never that check-ins are visible — the user published
+    // them and the pet page has always shown them. It is that they could be
+    // harvested in bulk, by anyone, with no login and no ceiling.
     match /{path=**}/checkins/{checkinId} {
-      allow read: if true;
+      allow read: if isAuthenticated() && resource.data.userId == request.auth.uid;
     }
 
     // Scoped to the requesting user, unlike the per-post rule above.

functions/src/__tests__/pet-checkins.test.ts

@@ -0,0 +1,156 @@
+import "./setup";
+import { afterAll, beforeEach, describe, expect, it } from "vitest";
+import { admin, db } from "../platform";
+import { getPetCheckinsCallable } from "../places";
+import { callAs, clearRateLimits, errorCodeOf } from "./helpers";
+
+// The checkins collection group is now scoped to the requesting user, which
+// closed the petId route along with the userId one — a petId-filtered
+// collection-group query can return other people's documents, so Firestore
+// refuses it. The pet profile reads its history through this callable instead.
+//
+// It stays PUBLIC: check-ins are content the user chose to publish and the pet
+// page has always shown them. What changed is that harvesting now costs a
+// capped, rate-limited call per pet rather than one unbounded query, and the
+// response carries no user identity at all.
+
+const OWNER = "checkin-owner";
+const PET = "checkin-pet";
+const OTHER_PET = "other-pet";
+const PLACE = "checkin-place";
+
+async function wipe() {
+  for (const c of ["users", "pets", "locations", "callableRateLimits"]) {
+    const snap = await db.collection(c).get();
+    for (const d of snap.docs) await db.recursiveDelete(d.ref).catch(() => undefined);
+  }
+  const users = await admin.auth().listUsers(1000);
+  await Promise.all(
+    users.users.map((u) => admin.auth().deleteUser(u.uid).catch(() => undefined))
+  );
+}
+
+/** Seeds `count` check-ins for a pet, oldest first so ordering is testable. */
+async function seedCheckins(petId: string, count: number) {
+  for (let i = 0; i < count; i += 1) {
+    await db.doc(`locations/${PLACE}/checkins/${petId}_${i}`).set({
+      counted: true,
+      userId: OWNER,
+      userName: "Owner Name",
+      userAvatar: "https://api.dicebear.com/7.x/thumbs/svg?seed=owner",
+      petId,
+      petName: "Rex",
+      photoUrl: `https://res.cloudinary.com/c/image/upload/petnote/${petId}-${i}.jpg`,
+      caption: `visit ${i}`,
+      locationId: PLACE,
+      createdAt: admin.firestore.Timestamp.fromMillis(
+        Date.UTC(2026, 0, 1) + i * 86_400_000
+      ),
+    });
+  }
+}
+
+beforeEach(async () => {
+  await wipe();
+  await clearRateLimits();
+  await db.doc(`users/${OWNER}`).set({ displayName: OWNER });
+  await db.doc(`pets/${PET}`).set({ name: "Rex", ownerId: OWNER });
+  await db.doc(`locations/${PLACE}`).set({ name: "Dog Park" });
+});
+afterAll(wipe);
+
+type Res = {
+  checkins: Array<Record<string, unknown>>;
+};
+
+describe("getPetCheckinsCallable", () => {
+  it("returns a pet's check-ins without being logged in", async () => {
+    // The whole point of the callable: the pet page is public and stays public.
+    await seedCheckins(PET, 3);
+    const res = await callAs<Res>(getPetCheckinsCallable, null, { petId: PET });
+    expect(res.checkins).toHaveLength(3);
+  });
+
+  it("returns them newest first", async () => {
+    await seedCheckins(PET, 3);
+    const res = await callAs<Res>(getPetCheckinsCallable, null, { petId: PET });
+    const times = res.checkins.map((c) => c.createdAtMillis as number);
+    expect(times).toEqual([...times].sort((a, b) => b - a));
+  });
+
+  it("carries no user identity at all", async () => {
+    // The pet page never rendered userId / userName / userAvatar, so a public
+    // endpoint has no reason to hand them out. Omitting them means this route
+    // cannot be turned back into a per-person lookup even in aggregate.
+    await seedCheckins(PET, 1);
+    const res = await callAs<Res>(getPetCheckinsCallable, null, { petId: PET });
+    const [row] = res.checkins;
+    expect(row).not.toHaveProperty("userId");
+    expect(row).not.toHaveProperty("userName");
+    expect(row).not.toHaveProperty("userAvatar");
+    expect(JSON.stringify(res)).not.toContain(OWNER);
+    expect(JSON.stringify(res)).not.toContain("Owner Name");
+    // And it still carries what the page does render.
+    expect(row.photoUrl).toContain("res.cloudinary.com");
+    expect(row.locationId).toBe(PLACE);
+    expect(row.caption).toBe("visit 0");
+  });
+
+  it("caps the row count however many are asked for", async () => {
+    // Without a ceiling, one rate-limited call becomes an unbounded dump.
+    await seedCheckins(PET, 12);
+    const res = await callAs<Res>(getPetCheckinsCallable, null, {
+      petId: PET,
+      limitCount: 100000,
+    });
+    expect(res.checkins.length).toBeLessThanOrEqual(100);
+    expect(res.checkins).toHaveLength(12);
+  });
+
+  it("honours a smaller limit, and refuses a nonsense one gracefully", async () => {
+    await seedCheckins(PET, 5);
+    const few = await callAs<Res>(getPetCheckinsCallable, null, {
+      petId: PET,
+      limitCount: 2,
+    });
+    expect(few.checkins).toHaveLength(2);
+
+    const zero = await callAs<Res>(getPetCheckinsCallable, null, {
+      petId: PET,
+      limitCount: 0,
+    });
+    expect(zero.checkins).toHaveLength(1);
+
+    const nan = await callAs<Res>(getPetCheckinsCallable, null, {
+      petId: PET,
+      limitCount: Number.NaN,
+    });
+    expect(nan.checkins.length).toBeGreaterThan(0);
+  });
+
+  it("does not leak another pet's check-ins", async () => {
+    await seedCheckins(PET, 2);
+    await seedCheckins(OTHER_PET, 2);
+    const res = await callAs<Res>(getPetCheckinsCallable, null, { petId: PET });
+    expect(res.checkins).toHaveLength(2);
+    for (const row of res.checkins) expect(row.petId).toBe(PET);
+  });
+
+  it("rejects a petId that is not a document id", async () => {
+    // requiredDocId: a value with slashes would otherwise be interpolated
+    // into a path and reach a different collection.
+    expect(
+      await errorCodeOf(() =>
+        callAs(getPetCheckinsCallable, null, { petId: "pets/x/family/y" })
+      )
+    ).toBe("invalid-argument");
+    expect(
+      await errorCodeOf(() => callAs(getPetCheckinsCallable, null, {}))
+    ).toBe("invalid-argument");
+  });
+
+  it("returns an empty list for a pet with no check-ins", async () => {
+    const res = await callAs<Res>(getPetCheckinsCallable, null, { petId: PET });
+    expect(res.checkins).toEqual([]);
+  });
+});

functions/src/index.ts

@@ -37,6 +37,7 @@ export {
   addLocationPhotosCallable,
   addPlaceCallable,
   checkInCallable,
+  getPetCheckinsCallable,
   onCheckinCreated,
   onCheckinDeleted,
   onLocationDeleted,

functions/src/places.ts

@@ -39,6 +39,11 @@ const allowedPlaceCategories = new Set([
   "other",
 ]);
 
+// Pet-profile check-in history. The page asks for 100; the ceiling is what
+// stops a caller turning one rate-limited call into an unbounded dump.
+const PET_CHECKIN_PAGE_SIZE = 50;
+const PET_CHECKIN_MAX_PAGE_SIZE = 100;
+
 const allowedPlaceFeatures = new Set([
   "off_leash",
   "fenced",
@@ -844,6 +849,83 @@ export const checkInCallable = onCall(
   return { id: checkinId };
 });
 
+// Public read of one pet's check-in history, for the pet profile.
+//
+// This exists because the checkins collection group had to be closed. Left
+// open it answered "every check-in matching a filter, across all locations",
+// which turned any uid into a movement timeline — every place a person has
+// physically been, with timestamps — to an unauthenticated caller with no
+// rate limit. The (userId, createdAt) and (petId, createdAt) collection-group
+// indexes both exist, so the query was servable.
+//
+// Closing only the userId path would have been theatre: pets/{petId} is
+// world-readable and carries ownerId, so the same timeline was one hop away
+// through petId. Both are closed; this callable is the sanctioned way back in.
+//
+// Still public — no login. Check-ins are content the user chose to publish,
+// and the pet profile has always shown them. What changes is that harvesting
+// now costs a rate-limited callable per pet instead of one unbounded query,
+// and the response carries no user identity at all: the pet page never
+// rendered userId / userName / userAvatar, so they are not returned.
+export const getPetCheckinsCallable = onCall(async (request) => {
+  const data = requestData(request.data) as {
+    petId?: string;
+    limitCount?: number;
+  };
+  const petId = requiredDocId(data.petId, "petId");
+
+  // Hard cap regardless of what the client asks for.
+  const requested =
+    typeof data.limitCount === "number" && Number.isFinite(data.limitCount)
+      ? Math.floor(data.limitCount)
+      : PET_CHECKIN_PAGE_SIZE;
+  const limitCount = Math.min(
+    PET_CHECKIN_MAX_PAGE_SIZE,
+    Math.max(1, requested)
+  );
+
+  // Signed-in callers are bucketed by uid; anonymous ones by client IP, so an
+  // unauthenticated scraper cannot get an unlimited budget just by not
+  // logging in. assertRateLimit only needs a stable string for the bucket.
+  const callerUid = request.auth?.uid;
+  const forwardedFor = request.rawRequest?.headers?.["x-forwarded-for"];
+  const rawIp =
+    (Array.isArray(forwardedFor) ? forwardedFor[0] : forwardedFor)
+      ?.split(",")[0]
+      ?.trim() ||
+    request.rawRequest?.ip ||
+    "unknown";
+  const bucket = callerUid ?? `anon_${rawIp.replace(/[^A-Za-z0-9_.:-]/g, "_")}`;
+  await assertRateLimit(bucket, "getPetCheckins", RATE_LIMITS.read);
+
+  const snap = await db
+    .collectionGroup("checkins")
+    .where("petId", "==", petId)
+    .orderBy("createdAt", "desc")
+    .limit(limitCount)
+    .get();
+
+  return {
+    checkins: snap.docs.map((docSnap) => {
+      const checkin = docSnap.data() ?? {};
+      const createdAt = checkin.createdAt;
+      return {
+        id: docSnap.id,
+        locationId: docSnap.ref.parent.parent?.id ?? "",
+        petId: typeof checkin.petId === "string" ? checkin.petId : "",
+        petName: typeof checkin.petName === "string" ? checkin.petName : "",
+        photoUrl: typeof checkin.photoUrl === "string" ? checkin.photoUrl : "",
+        caption: typeof checkin.caption === "string" ? checkin.caption : "",
+        // Timestamps do not survive the callable boundary as Timestamps.
+        createdAtMillis:
+          createdAt && typeof createdAt.toMillis === "function"
+            ? createdAt.toMillis()
+            : null,
+      };
+    }),
+  };
+});
+
 // Admin-only backfill: rebuilds petFriendlySum / petFriendlyAvg /
 // tagCounts / topTags from the location's review subcollection. Used to
 // initialise these aggregates on locations whose reviews predate the

src/pages/PetProfile.tsx

@@ -8,7 +8,7 @@ import { useAuth } from "../hooks/useAuth";
 import { useToast } from "../contexts/ToastContext";
 import { useFollowPet } from "../hooks/useFollow";
 import { getPetFollowers, type PetFollower } from "../services/follow";
-import { getCheckinsByPet, type Checkin } from "../services/checkins";
+import { getCheckinsByPet, type PetCheckin } from "../services/checkins";
 import { batchGetLocations, type Location } from "../services/locations";
 import {
   deletePet,
@@ -42,7 +42,7 @@ export function PetProfile() {
   const [familyMembers, setFamilyMembers] = useState<FamilyMember[]>([]);
   const [viewerIsFamilyMember, setViewerIsFamilyMember] = useState(false);
 
-  const [checkins, setCheckins] = useState<Checkin[]>([]);
+  const [checkins, setCheckins] = useState<PetCheckin[]>([]);
   const [checkinLocations, setCheckinLocations] = useState<Record<string, Location | null>>(
     {}
   );