feat(functions): hourly cleanup of expired restore requests (Future.md security #1)

A restore_requests doc left behind — partner wrapped the couple key but the
recipient never completed, or nobody ever answered — keeps its ECIES keybox
forever. It's sealed to the recipient alone, so it's not exploitable, but key
material has no business lying around, and expiry was enforced only client-side
(fulfil-time check, delete-before-re-request, delete-on-complete): a request
whose device disappeared simply lived forever.

cleanupExpiredRestoreRequests runs hourly (requests expire in 30 min, so a
stranded keybox now lives ~1.5 h at most) over a collectionGroup query on
expiresAt — chosen over iterating couples deliberately, because it also reaps
requests orphaned under already-deleted couple docs, which a parent iteration
can never see. Backed by a new COLLECTION_GROUP fieldOverride on
restore_requests.expiresAt (expiresAt is epoch millis, not a Timestamp, which
also rules out native Firestore TTL).

Deletes only on positive evidence: a pure predicate re-verifies every query hit
(a real expiresAt past a 5-min grace so a mid-completion restore is never raced;
a day-old createdAt as the defensive fallback when expiresAt is unusable;
neither → leave it and log). Status is deliberately irrelevant — a
DECLINED-after-READY doc still carries the keybox.

Requests that expired recently while still waiting on someone (REQUESTED/READY)
nudge the requester — "start a new one whenever you're ready" — through the
house pipeline (notification_queue + sendPushToUser, quiet hours respected).
The doc is deleted BEFORE the nudge, so a notify failure costs a nudge, never a
duplicate; a 2-h notify window keeps the first deploy from blasting the ancient
backlog. queueAndPush moves from being file-local in onRestoreRequested.ts to a
shared notifications/queueAndPush.ts — the cleanup needed identical semantics,
and two copies of notification plumbing is how the same bug ends up existing
twice.

Sweep never throws (a scheduled-function throw retries in a storm; the next
hourly run IS the retry): per-doc Promise.allSettled, one summary log line via
the structured logger.

15 new tests; grace-window mutation check kills exactly the guard test.
Functions suite 98/98, tsc clean. Deploy (scoped — the RevenueCat webhook must
stay undeployed): firebase deploy --only firestore:indexes, then
--only functions:cleanupExpiredRestoreRequests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

firestore.indexes.json

@@ -4,32 +4,56 @@
       "collectionGroup": "invites",
       "queryScope": "COLLECTION",
       "fields": [
-        { "fieldPath": "inviterUserId", "order": "ASCENDING" },
+        {
-        { "fieldPath": "createdAt", "order": "DESCENDING" }
+          "fieldPath": "inviterUserId",
+          "order": "ASCENDING"
+        },
+        {
+          "fieldPath": "createdAt",
+          "order": "DESCENDING"
+        }
       ]
     },
     {
       "collectionGroup": "capsules",
       "queryScope": "COLLECTION_GROUP",
       "fields": [
-        { "fieldPath": "status", "order": "ASCENDING" },
+        {
-        { "fieldPath": "unlockAt", "order": "ASCENDING" }
+          "fieldPath": "status",
+          "order": "ASCENDING"
+        },
+        {
+          "fieldPath": "unlockAt",
+          "order": "ASCENDING"
+        }
       ]
     },
     {
       "collectionGroup": "questions",
       "queryScope": "COLLECTION",
       "fields": [
-        { "fieldPath": "active", "order": "ASCENDING" },
+        {
-        { "fieldPath": "isPremium", "order": "ASCENDING" }
+          "fieldPath": "active",
+          "order": "ASCENDING"
+        },
+        {
+          "fieldPath": "isPremium",
+          "order": "ASCENDING"
+        }
       ]
     },
     {
       "collectionGroup": "bucket_list",
       "queryScope": "COLLECTION",
       "fields": [
-        { "fieldPath": "category", "order": "ASCENDING" },
+        {
-        { "fieldPath": "addedAt", "order": "DESCENDING" }
+          "fieldPath": "category",
+          "order": "ASCENDING"
+        },
+        {
+          "fieldPath": "addedAt",
+          "order": "DESCENDING"
+        }
       ]
     }
   ],
@@ -39,6 +63,16 @@
       "fieldPath": "expiresAt",
       "ttl": true,
       "indexes": []
+    },
+    {
+      "collectionGroup": "restore_requests",
+      "fieldPath": "expiresAt",
+      "indexes": [
+        {
+          "queryScope": "COLLECTION_GROUP",
+          "order": "ASCENDING"
+        }
+      ]
     }
   ]
 }

functions/src/backup/cleanupRestoreRequests.test.ts

@@ -0,0 +1,164 @@
+import {
+  GRACE_MS,
+  FALLBACK_TTL_MS,
+  NOTIFY_WINDOW_MS,
+  shouldReapRestoreRequest,
+  shouldNotifyExpiry,
+  sweepExpiredRestoreRequests,
+} from './cleanupRestoreRequests'
+import { queueAndPush } from '../notifications/queueAndPush'
+
+jest.mock('../notifications/queueAndPush', () => ({ queueAndPush: jest.fn().mockResolvedValue(undefined) }))
+
+const NOW = 1_800_000_000_000
+
+describe('shouldReapRestoreRequest', () => {
+  it('keeps a fresh request', () => {
+    expect(shouldReapRestoreRequest({ status: 'REQUESTED', expiresAt: NOW + 60_000 }, NOW)).toBe(false)
+  })
+
+  it('keeps an expired request still inside the grace window', () => {
+    // The client already treats it as expired; we wait out the grace so we can never race a
+    // mid-completion restore.
+    expect(shouldReapRestoreRequest({ status: 'READY', expiresAt: NOW - GRACE_MS + 1_000 }, NOW)).toBe(false)
+  })
+
+  it('reaps expired requests regardless of status — DECLINED-after-READY still carries the keybox', () => {
+    const expiresAt = NOW - GRACE_MS - 1_000
+    for (const status of ['REQUESTED', 'READY', 'DECLINED', 'RESTORED', 'EXPIRED']) {
+      expect(shouldReapRestoreRequest({ status, expiresAt }, NOW)).toBe(true)
+    }
+  })
+
+  it('falls back to createdAt when expiresAt is unusable', () => {
+    expect(shouldReapRestoreRequest({ createdAt: NOW - FALLBACK_TTL_MS - 1 }, NOW)).toBe(true)
+    expect(shouldReapRestoreRequest({ expiresAt: 0, createdAt: NOW - FALLBACK_TTL_MS - 1 }, NOW)).toBe(true)
+    expect(shouldReapRestoreRequest({ createdAt: NOW - 60_000 }, NOW)).toBe(false)
+  })
+
+  it('leaves a doc with neither field — delete only on positive evidence', () => {
+    expect(shouldReapRestoreRequest({}, NOW)).toBe(false)
+    expect(shouldReapRestoreRequest({ expiresAt: 'soon', createdAt: null }, NOW)).toBe(false)
+  })
+})
+
+describe('shouldNotifyExpiry', () => {
+  const justExpired = NOW - GRACE_MS - 60_000
+
+  it('notifies for a recently expired request still waiting on someone', () => {
+    expect(shouldNotifyExpiry({ status: 'REQUESTED', expiresAt: justExpired }, NOW)).toBe(true)
+    expect(shouldNotifyExpiry({ status: 'READY', expiresAt: justExpired }, NOW)).toBe(true)
+  })
+
+  it('stays silent for terminal states — RESTORED succeeded, DECLINED was answered', () => {
+    expect(shouldNotifyExpiry({ status: 'RESTORED', expiresAt: justExpired }, NOW)).toBe(false)
+    expect(shouldNotifyExpiry({ status: 'DECLINED', expiresAt: justExpired }, NOW)).toBe(false)
+  })
+
+  it('stays silent for backlog debris — first deploy must not blast ancient expiries', () => {
+    expect(shouldNotifyExpiry({ status: 'REQUESTED', expiresAt: NOW - NOTIFY_WINDOW_MS - GRACE_MS - 1_000 }, NOW)).toBe(false)
+  })
+
+  it('stays silent for the createdAt-fallback branch (no real expiry to report)', () => {
+    expect(shouldNotifyExpiry({ status: 'REQUESTED', createdAt: NOW - FALLBACK_TTL_MS - 1 }, NOW)).toBe(false)
+  })
+
+  it('never notifies for something it would not reap', () => {
+    expect(shouldNotifyExpiry({ status: 'REQUESTED', expiresAt: NOW + 60_000 }, NOW)).toBe(false)
+    expect(shouldNotifyExpiry({ status: 'REQUESTED', expiresAt: NOW - GRACE_MS + 1_000 }, NOW)).toBe(false)
+  })
+})
+
+describe('sweepExpiredRestoreRequests', () => {
+  const mockQueueAndPush = queueAndPush as jest.Mock
+
+  function fakeDoc(opts: {
+    id: string
+    data: Record<string, unknown>
+    coupleExists?: boolean
+    deleteError?: Error
+  }) {
+    const coupleRef = {
+      id: `couple_${opts.id}`,
+      get: jest.fn().mockResolvedValue({ exists: opts.coupleExists ?? true }),
+    }
+    return {
+      id: opts.id,
+      data: () => opts.data,
+      ref: {
+        path: `couples/couple_${opts.id}/restore_requests/${opts.id}`,
+        parent: { parent: coupleRef },
+        delete: opts.deleteError
+          ? jest.fn().mockRejectedValue(opts.deleteError)
+          : jest.fn().mockResolvedValue(undefined),
+      },
+    }
+  }
+
+  function fakeDb(docs: ReturnType<typeof fakeDoc>[]) {
+    return {
+      collectionGroup: jest.fn().mockReturnValue({
+        where: jest.fn().mockReturnValue({
+          limit: jest.fn().mockReturnValue({
+            get: jest.fn().mockResolvedValue({ size: docs.length, docs }),
+          }),
+        }),
+      }),
+    } as unknown as FirebaseFirestore.Firestore
+  }
+
+  beforeEach(() => mockQueueAndPush.mockClear())
+
+  it('deletes the expired doc and nudges the requester', async () => {
+    const doc = fakeDoc({ id: 'uidA', data: { status: 'READY', recipientUid: 'uidA', expiresAt: NOW - GRACE_MS - 60_000 } })
+    const counts = await sweepExpiredRestoreRequests(fakeDb([doc]), NOW)
+
+    expect(doc.ref.delete).toHaveBeenCalledTimes(1)
+    expect(mockQueueAndPush).toHaveBeenCalledWith(expect.anything(), 'uidA', expect.objectContaining({ type: 'restore_request_expired' }))
+    expect(counts).toMatchObject({ scanned: 1, reaped: 1, notified: 1, failed: 0 })
+  })
+
+  it('leaves a query hit that fails re-verify', async () => {
+    // The index says expired; the doc says otherwise (clock skew, weird data). The predicate wins.
+    const doc = fakeDoc({ id: 'uidB', data: { status: 'REQUESTED', expiresAt: NOW + 60_000 } })
+    const counts = await sweepExpiredRestoreRequests(fakeDb([doc]), NOW)
+
+    expect(doc.ref.delete).not.toHaveBeenCalled()
+    expect(counts).toMatchObject({ reaped: 0, skipped: 1 })
+  })
+
+  it('skips the nudge for an orphan reap (couple gone) but still deletes', async () => {
+    const doc = fakeDoc({
+      id: 'uidC',
+      data: { status: 'READY', expiresAt: NOW - GRACE_MS - 60_000 },
+      coupleExists: false,
+    })
+    const counts = await sweepExpiredRestoreRequests(fakeDb([doc]), NOW)
+
+    expect(doc.ref.delete).toHaveBeenCalledTimes(1)
+    expect(mockQueueAndPush).not.toHaveBeenCalled()
+    expect(counts).toMatchObject({ reaped: 1, notified: 0 })
+  })
+
+  it('one failing doc never stops the sweep', async () => {
+    const bad = fakeDoc({
+      id: 'uidD',
+      data: { status: 'READY', expiresAt: NOW - GRACE_MS - 60_000 },
+      deleteError: new Error('firestore unavailable'),
+    })
+    const good = fakeDoc({ id: 'uidE', data: { status: 'REQUESTED', recipientUid: 'uidE', expiresAt: NOW - GRACE_MS - 60_000 } })
+    const counts = await sweepExpiredRestoreRequests(fakeDb([bad, good]), NOW)
+
+    expect(good.ref.delete).toHaveBeenCalledTimes(1)
+    expect(counts).toMatchObject({ scanned: 2, reaped: 1, failed: 1 })
+  })
+
+  it('a notify failure costs the nudge, never the reap or the pass', async () => {
+    mockQueueAndPush.mockRejectedValueOnce(new Error('fcm down'))
+    const doc = fakeDoc({ id: 'uidF', data: { status: 'READY', recipientUid: 'uidF', expiresAt: NOW - GRACE_MS - 60_000 } })
+    const counts = await sweepExpiredRestoreRequests(fakeDb([doc]), NOW)
+
+    expect(doc.ref.delete).toHaveBeenCalledTimes(1)
+    expect(counts).toMatchObject({ reaped: 1, notified: 0, failed: 0 })
+  })
+})

functions/src/backup/cleanupRestoreRequests.ts

@@ -0,0 +1,137 @@
+import * as admin from 'firebase-admin'
+import { onSchedule } from 'firebase-functions/v2/scheduler'
+import { queueAndPush } from '../notifications/queueAndPush'
+import { logger } from '../log'
+
+/**
+ * Cloud Function: cleanupExpiredRestoreRequests
+ *
+ * A `restore_requests` doc left behind (partner wrapped the couple key but the recipient never
+ * completed, or nobody ever answered) keeps its ECIES `keybox` — ciphertext sealed only to the
+ * recipient, useless to anyone else, but key material has no business lying around. Expiry is
+ * otherwise enforced only client-side (fulfil-time check, delete-before-re-request, delete-on-
+ * complete), so a request whose device disappeared lives forever.
+ *
+ * Hourly sweep: requests expire 30 minutes after creation, so this bounds a stranded keybox's life
+ * to ~1.5 h. Uses a collectionGroup query on purpose — it also reaps requests orphaned under
+ * already-deleted couple docs, which an iterate-the-couples sweep can never see (deleted parents
+ * don't list). Needs the COLLECTION_GROUP fieldOverride on `restore_requests.expiresAt` in
+ * `firestore.indexes.json`.
+ *
+ * Admin SDK bypasses the recipient-only delete rule, which is the point: nobody else CAN clean these.
+ */
+
+/** Never race a restore that is mid-completion: the client treats these as expired well before us. */
+export const GRACE_MS = 5 * 60 * 1000
+/** Reap docs with no usable `expiresAt` once they are unambiguously ancient. */
+export const FALLBACK_TTL_MS = 24 * 60 * 60 * 1000
+/**
+ * Only tell the requester about expiries that just happened (~2 sweep cycles). Without this bound,
+ * the first deploy would blast a notification for every ancient doc in the backlog at once.
+ */
+export const NOTIFY_WINDOW_MS = 2 * 60 * 60 * 1000
+
+interface RestoreRequestFields {
+  status?: unknown
+  expiresAt?: unknown
+  createdAt?: unknown
+  recipientUid?: unknown
+}
+
+const asMillis = (v: unknown): number => (typeof v === 'number' && v > 0 ? v : 0)
+
+/**
+ * Pure reap decision, re-verified per doc even though the query already filtered — the query is an
+ * index scan, this is the contract. Deletes only on positive evidence of staleness:
+ * a real `expiresAt` past the grace window, or (defensively — the create rule doesn't validate field
+ * values) no usable `expiresAt` but a `createdAt` at least a day old. A doc with neither is left
+ * alone for a human to wonder about.
+ */
+export function shouldReapRestoreRequest(data: RestoreRequestFields, nowMs: number): boolean {
+  const expiresAt = asMillis(data.expiresAt)
+  if (expiresAt > 0) return nowMs > expiresAt + GRACE_MS
+  const createdAt = asMillis(data.createdAt)
+  return createdAt > 0 && nowMs > createdAt + FALLBACK_TTL_MS
+}
+
+/**
+ * Pure notify decision: only for a request that expired *recently* while still waiting on someone
+ * (REQUESTED: partner never approved; READY: recipient never completed). Terminal states are debris —
+ * RESTORED already succeeded and DECLINED was answered; telling anyone again is noise.
+ */
+export function shouldNotifyExpiry(data: RestoreRequestFields, nowMs: number): boolean {
+  const expiresAt = asMillis(data.expiresAt)
+  if (expiresAt <= 0 || nowMs <= expiresAt + GRACE_MS) return false
+  if (nowMs - expiresAt > NOTIFY_WINDOW_MS) return false
+  return data.status === 'REQUESTED' || data.status === 'READY'
+}
+
+/**
+ * The sweep, with `db` injectable for tests. Per-doc failures are isolated (one bad doc can't stop
+ * the pass); the doc is deleted BEFORE the requester is notified so a notify failure costs a nudge,
+ * never a duplicate (the doc it would re-trigger on is already gone).
+ */
+export async function sweepExpiredRestoreRequests(
+  db: admin.firestore.Firestore,
+  nowMs: number
+): Promise<{ scanned: number; reaped: number; notified: number; skipped: number; failed: number }> {
+  const snap = await db
+    .collectionGroup('restore_requests')
+    .where('expiresAt', '<=', nowMs - GRACE_MS)
+    .limit(200)
+    .get()
+
+  const counts = { scanned: snap.size, reaped: 0, notified: 0, skipped: 0, failed: 0 }
+
+  const results = await Promise.allSettled(
+    snap.docs.map(async (doc) => {
+      const data = (doc.data() ?? {}) as RestoreRequestFields
+      if (!shouldReapRestoreRequest(data, nowMs)) {
+        counts.skipped++
+        logger.warn('[cleanupRestoreRequests] query hit failed re-verify; leaving doc', { path: doc.ref.path })
+        return
+      }
+      const notify = shouldNotifyExpiry(data, nowMs)
+      await doc.ref.delete()
+      counts.reaped++
+
+      if (!notify) return
+      // Best-effort: a missed nudge is fine, a failed delete is not — hence delete-first above.
+      // Skip when the couple itself is gone (an orphan reap has nobody meaningful to notify).
+      try {
+        const coupleRef = doc.ref.parent.parent
+        const recipientUid = typeof data.recipientUid === 'string' && data.recipientUid ? data.recipientUid : doc.id
+        if (!coupleRef) return
+        const coupleDoc = await coupleRef.get()
+        if (!coupleDoc.exists) return
+        await queueAndPush(db, recipientUid, {
+          type: 'restore_request_expired',
+          title: 'Your restore request expired',
+          body: 'No worries — start a new one whenever you’re ready.',
+          coupleId: coupleRef.id,
+        })
+        counts.notified++
+      } catch (e) {
+        logger.warn('[cleanupRestoreRequests] expiry notice failed', { path: doc.ref.path, error: String(e) })
+      }
+    })
+  )
+  counts.failed = results.filter((r) => r.status === 'rejected').length
+  return counts
+}
+
+export const cleanupExpiredRestoreRequests = onSchedule(
+  { schedule: 'every 1 hours', timeoutSeconds: 180 },
+  async () => {
+    try {
+      const counts = await sweepExpiredRestoreRequests(admin.firestore(), Date.now())
+      logger.log(
+        `[cleanupRestoreRequests] scanned ${counts.scanned}; reaped ${counts.reaped}; ` +
+          `notified ${counts.notified}; skipped ${counts.skipped}; failed ${counts.failed}`
+      )
+    } catch (e) {
+      // Never rethrow: a scheduled-function failure retries in a storm, and the next hourly run IS the retry.
+      logger.error('[cleanupRestoreRequests] sweep failed', { error: String(e) })
+    }
+  }
+)

functions/src/backup/onRestoreRequested.ts

@@ -1,10 +1,8 @@
 import * as admin from 'firebase-admin'
 import { onDocumentCreated, onDocumentUpdated } from 'firebase-functions/v2/firestore'
-import { recipientInQuietHours } from '../notifications/quietHours'
+import { queueAndPush } from '../notifications/queueAndPush'
-import { sendPushToUser } from '../notifications/push'
 import { logger } from '../log'
 
-const CHANNEL = 'partner_activity'
 // Suppress duplicate pushes when a request doc is rapidly deleted+recreated (a compromised account
 // could loop that to spam both partners) and when an event is redelivered. The in-app queue entry is
 // still written every time. A genuine re-request after the window still notifies — so this is a time
@@ -22,38 +20,6 @@ export function isRestoreReadyTransition(beforeStatus: unknown, afterStatus: unk
   return afterStatus === 'READY' && beforeStatus !== 'READY'
 }
 
-/**
- * Write the durable in-app queue entry (always) and — unless quiet hours suppress it — push to every device.
- * `bypassQuietHours` is for security signals (the "was this you?" self-alert) that must not be silenced.
- */
-async function queueAndPush(
-  db: admin.firestore.Firestore,
-  uid: string,
-  opts: { type: string; title: string; body: string; coupleId: string; bypassQuietHours?: boolean }
-): Promise<void> {
-  const { type, title, body, coupleId, bypassQuietHours } = opts
-  await db.collection('users').doc(uid).collection('notification_queue').add({
-    type, title, body, read: false, createdAt: admin.firestore.FieldValue.serverTimestamp(),
-  })
-
-  const userDoc = await db.collection('users').doc(uid).get()
-  if (!userDoc.exists) return
-  const userData = userDoc.data()
-  if (!bypassQuietHours && recipientInQuietHours(userData)) return
-
-  await sendPushToUser(
-    db,
-    admin.messaging(),
-    uid,
-    {
-      notification: { title, body },
-      data: { type, couple_id: coupleId },
-      android: { notification: { channelId: CHANNEL } },
-    },
-    userData,
-  )
-}
-
 /**
  * Fires when a member starts a partner-assisted restore
  * (`couples/{coupleId}/restore_requests/{recipientUid}` created on a new/wiped device). Sends TWO

functions/src/index.ts

@@ -33,6 +33,7 @@ export { onDateReflectionWritten } from './dates/onDateReflectionWritten'
 export { onDateReflectionRevealed } from './dates/onDateReflectionRevealed'
 export { onDateHistoryCreated } from './dates/onDateHistoryCreated'
 export { onRestoreRequested, onRestoreFulfilled } from './backup/onRestoreRequested'
+export { cleanupExpiredRestoreRequests } from './backup/cleanupRestoreRequests'
 export {
   assignDailyQuestion,
   assignDailyQuestionCallable,

functions/src/notifications/queueAndPush.ts

@@ -0,0 +1,46 @@
+import * as admin from 'firebase-admin'
+import { recipientInQuietHours } from './quietHours'
+import { sendPushToUser } from './push'
+
+/**
+ * Write the durable in-app queue entry (always) and — unless quiet hours suppress it — push to every
+ * device. `bypassQuietHours` is for security signals (the "was this you?" self-alert class) that must
+ * not be silenced.
+ *
+ * Extracted from `backup/onRestoreRequested.ts` when the restore-request cleanup sweep needed the same
+ * behavior — one implementation, so queue-entry/quiet-hours/push semantics can't drift between callers.
+ */
+export async function queueAndPush(
+  db: admin.firestore.Firestore,
+  uid: string,
+  opts: {
+    type: string
+    title: string
+    body: string
+    coupleId: string
+    bypassQuietHours?: boolean
+    channelId?: string
+  }
+): Promise<void> {
+  const { type, title, body, coupleId, bypassQuietHours, channelId = 'partner_activity' } = opts
+  await db.collection('users').doc(uid).collection('notification_queue').add({
+    type, title, body, read: false, createdAt: admin.firestore.FieldValue.serverTimestamp(),
+  })
+
+  const userDoc = await db.collection('users').doc(uid).get()
+  if (!userDoc.exists) return
+  const userData = userDoc.data()
+  if (!bypassQuietHours && recipientInQuietHours(userData)) return
+
+  await sendPushToUser(
+    db,
+    admin.messaging(),
+    uid,
+    {
+      notification: { title, body },
+      data: { type, couple_id: coupleId },
+      android: { notification: { channelId } },
+    },
+    userData,
+  )
+}