fix(crypto): rotation must advance keyGeneration — a cached read silently stranded the partner (C-ROTATE-001)

Caught live on the fixture couple, second rotation. The first rotation worked;
the second looked identical from the rotating device — dialog confirmed, no
error, keyset swapped locally — and permanently locked the partner out of every
message written afterwards: 🔒 "Couldn't unlock on this device", with no way to
recover on his own, because the one signal that tells a partner to adopt a new
key never fired.

The chain: rotateCoupleKey read the couple through the cache-first
getCoupleById, got the stale keyGeneration=0 (the doc was already at 1), and
computed next = 1 — the value already on the document. Firestore doesn't count
an unchanged value as a change: affectedKeys() omitted keyGeneration, so the
monotonic rule never engaged, onCoupleKeyRotated never fired, and the partner's
device kept seeing "up to date". Meanwhile the write DID replace
wrappedCoupleKey with a wrap of the newer keyset and the rotating device DID
commit it locally — new content under a key only one phone has.

Same disease as C-AUTH-001, new organ: a cache-first read driving a one-shot
decision. Fixed at both layers:

- client: getCoupleByIdFromServer (Source.SERVER) backs the rotation decision;
  offline throws, nothing is written, the user is told nothing changed —
  rotation must fail loudly rather than half-happen.
- rules: a write that CHANGES wrappedCoupleKey must now carry a strictly
  increased keyGeneration. The stranding write is unrepresentable server-side,
  whatever any future client computes. (The old rule only checked monotonicity
  when keyGeneration was itself in affectedKeys — which is exactly the case
  that never arises when the bug happens.)

CoupleKeyRotationRepositoryTest pins it: generation comes from the server never
the cache, the published generation strictly advances, server write precedes the
local commit, offline fails without guessing. Mutation-checked by reinstating
the cached read — the two key tests fail, restored, suite green.

The fixture partner is un-stranded by rotating once more with this build: the
server read yields the true generation, the write is a real change, the trigger
fires, and adoption delivers the latest keyset (which retains every older key,
so the stranded-era messages decrypt too).

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

app/src/main/java/app/closer/data/remote/FirestoreCoupleDataSource.kt

@@ -98,6 +98,20 @@ class FirestoreCoupleDataSource @Inject constructor(
         return if (snap.exists()) snap.toCouple() else null
     }
 
+    /**
+     * The couple, straight from the server — never the local cache.
+     *
+     * [getCoupleById] is `Source.DEFAULT`, which can answer from cache. That is fine for rendering and
+     * wrong for deciding the next `keyGeneration`: a stale generation makes the rotation write the
+     * value the document ALREADY has, which is not a change, so no partner ever adopts the new key
+     * (C-ROTATE-001 — it stranded a real fixture partner on 🔒 before this existed). Throws when the
+     * server can't be reached, which is the point: rotation must fail loudly rather than half-happen.
+     */
+    suspend fun getCoupleByIdFromServer(coupleId: String): Couple? {
+        val snap = coupleRef(coupleId).get(com.google.firebase.firestore.Source.SERVER).await()
+        return if (snap.exists()) snap.toCouple() else null
+    }
+
     suspend fun updateStreak(coupleId: String) {
         val snap = coupleRef(coupleId).get().await()
         val lastAnsweredAt = snap.millisOrNull("lastAnsweredAt") ?: 0L

app/src/main/java/app/closer/data/repository/CoupleRepositoryImpl.kt

@@ -64,9 +64,12 @@ class CoupleRepositoryImpl @Inject constructor(
     }
 
     override suspend fun rotateCoupleKey(coupleId: String): Result<Unit> = runCatching {
-        // Generation truth lives on the couple doc — read it fresh so concurrent rotations collide
+        // Generation truth lives on the couple doc, and it MUST come from the server: a cached
-        // at the rules' strictly-increasing check instead of silently overwriting each other.
+        // generation makes us write back the value already there, which Firestore doesn't count as a
-        val couple = coupleDataSource.getCoupleById(coupleId) ?: error("couple not found")
+        // change — the rule passes, no trigger fires, and the partner never learns to adopt. That
+        // stranded a real partner on 🔒 (C-ROTATE-001). Offline throws here, which is correct:
+        // nothing is written and the user is told nothing changed.
+        val couple = coupleDataSource.getCoupleByIdFromServer(coupleId) ?: error("couple not found")
         val prepared = encryptionManager.prepareRotation(coupleId, couple.keyGeneration).getOrElse { throw it }
         // Server first: if this write fails, this device stays coherent on the old key and nothing
         // anywhere has changed. If the process dies right after it, this device self-heals through

app/src/test/java/app/closer/data/repository/CoupleKeyRotationRepositoryTest.kt

@@ -0,0 +1,106 @@
+package app.closer.data.repository
+
+import app.closer.core.crash.CrashReporter
+import app.closer.crypto.CoupleEncryptionManager
+import app.closer.crypto.RecoveryKeyManager
+import app.closer.data.remote.FirestoreCoupleDataSource
+import app.closer.data.remote.FirestoreUserDataSource
+import app.closer.domain.model.Couple
+import com.google.crypto.tink.KeysetHandle
+import io.mockk.Runs
+import io.mockk.coEvery
+import io.mockk.coVerify
+import io.mockk.just
+import io.mockk.mockk
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.runTest
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * C-ROTATE-001, caught live: a rotation that doesn't ADVANCE `keyGeneration` strands the partner.
+ *
+ * The generation is the only signal the partner's device has to adopt the new key. Reading it from
+ * cache produced a stale value, so the rotation wrote back the number already on the document —
+ * Firestore doesn't count an unchanged value as a change, so the rules passed, no trigger fired, and
+ * the partner sat on "🔒 Couldn't unlock on this device" for every new message, permanently. It looked
+ * exactly like success from the rotating device.
+ *
+ * These pin the two properties that make that impossible: the generation comes from the SERVER, and
+ * the published generation is strictly greater than the one we read.
+ */
+@OptIn(ExperimentalCoroutinesApi::class)
+class CoupleKeyRotationRepositoryTest {
+
+    private val coupleDataSource: FirestoreCoupleDataSource = mockk()
+    private val userDataSource: FirestoreUserDataSource = mockk()
+    private val encryptionManager: CoupleEncryptionManager = mockk()
+    private val crashReporter: CrashReporter = mockk(relaxUnitFun = true)
+
+    private val repo = CoupleRepositoryImpl(coupleDataSource, userDataSource, encryptionManager, crashReporter)
+
+    private val coupleId = "c1"
+    private val wrapped = RecoveryKeyManager.WrappedKey(cipherB64 = "cipher", saltB64 = "salt", params = "p")
+
+    private fun prepared(newGeneration: Long) = CoupleEncryptionManager.PreparedRotation(
+        rotatedHandle = mockk<KeysetHandle>(),
+        wrapped = wrapped,
+        newGeneration = newGeneration
+    )
+
+    @Test
+    fun `the generation comes from the server, never the cache`() = runTest {
+        // The cache is what lied. If rotation ever reads getCoupleById again, this fails.
+        coEvery { coupleDataSource.getCoupleByIdFromServer(coupleId) } returns Couple(id = coupleId, keyGeneration = 1L)
+        coEvery { encryptionManager.prepareRotation(coupleId, 1L) } returns Result.success(prepared(2L))
+        coEvery { coupleDataSource.rotateWrappedKey(any(), any(), any()) } just Runs
+        every2()
+
+        assertTrue(repo.rotateCoupleKey(coupleId).isSuccess)
+
+        coVerify(exactly = 0) { coupleDataSource.getCoupleById(any()) }
+        coVerify { coupleDataSource.getCoupleByIdFromServer(coupleId) }
+    }
+
+    @Test
+    fun `the published generation strictly advances past the server's`() = runTest {
+        coEvery { coupleDataSource.getCoupleByIdFromServer(coupleId) } returns Couple(id = coupleId, keyGeneration = 7L)
+        coEvery { encryptionManager.prepareRotation(coupleId, 7L) } returns Result.success(prepared(8L))
+        coEvery { coupleDataSource.rotateWrappedKey(any(), any(), any()) } just Runs
+        every2()
+
+        assertTrue(repo.rotateCoupleKey(coupleId).isSuccess)
+
+        // 8 > 7: a real change, so the trigger fires and the partner adopts. Writing 7 here is the bug.
+        coVerify { coupleDataSource.rotateWrappedKey(coupleId, wrapped, 8L) }
+    }
+
+    @Test
+    fun `server write happens before the local commit`() = runTest {
+        // Ordering IS the error handling: a failed write must leave this device on the old key.
+        coEvery { coupleDataSource.getCoupleByIdFromServer(coupleId) } returns Couple(id = coupleId, keyGeneration = 0L)
+        coEvery { encryptionManager.prepareRotation(coupleId, 0L) } returns Result.success(prepared(1L))
+        coEvery { coupleDataSource.rotateWrappedKey(any(), any(), any()) } throws RuntimeException("rules rejected")
+
+        val result = repo.rotateCoupleKey(coupleId)
+
+        assertTrue(result.isFailure)
+        coVerify(exactly = 0) { encryptionManager.commitRotation(any(), any()) }
+    }
+
+    @Test
+    fun `an offline server read fails the rotation instead of guessing`() = runTest {
+        coEvery { coupleDataSource.getCoupleByIdFromServer(coupleId) } throws RuntimeException("unavailable")
+
+        val result = repo.rotateCoupleKey(coupleId)
+
+        assertTrue(result.isFailure)
+        coVerify(exactly = 0) { coupleDataSource.rotateWrappedKey(any(), any(), any()) }
+        coVerify(exactly = 0) { encryptionManager.commitRotation(any(), any()) }
+    }
+
+    /** commitRotation is a plain (non-suspend) store; relax it for the success paths. */
+    private fun every2() {
+        io.mockk.every { encryptionManager.commitRotation(any(), any()) } just Runs
+    }
+}

firestore.rules

@@ -182,9 +182,19 @@ service cloud.firestore {
         && request.resource.data.diff(resource.data).affectedKeys().hasOnly([
              'wrappedCoupleKey', 'kdfSalt', 'kdfParams', 'keyGeneration'
            ])
-        // Key rotation bumps keyGeneration strictly upward alongside the new wrap; a plain
+        // A CHANGED wrap is a key rotation, and a rotation MUST advance keyGeneration: that bump is
-        // phrase re-wrap doesn't touch it. Monotonic like encryptionVersion: never downgradeable,
+        // the only signal the partner's device has to adopt the new key. Publishing a new wrap while
-        // so a replayed/stale rotation write can't roll the couple back to an older wrap.
+        // leaving the generation alone silently strands the partner on 🔒 forever — so the rules make
+        // it impossible rather than trusting every client to compute the next generation correctly
+        // (C-ROTATE-001: a cached read did exactly that). Note an unchanged value is absent from
+        // affectedKeys(), which is what let the stale write through before.
+        && (
+             !request.resource.data.diff(resource.data).affectedKeys().hasAny(['wrappedCoupleKey'])
+             || (request.resource.data.keyGeneration is int
+                  && request.resource.data.keyGeneration > resource.data.get('keyGeneration', 0))
+           )
+        // Monotonic like encryptionVersion: never downgradeable, so a replayed or stale rotation
+        // write can't roll the couple back to an older wrap.
         && (
              !request.resource.data.diff(resource.data).affectedKeys().hasAny(['keyGeneration'])
              || (request.resource.data.keyGeneration is int