From 4952b129c5b4be291f5dd41f19b8da281ea8e5fe Mon Sep 17 00:00:00 2001 From: null Date: Wed, 15 Jul 2026 22:33:31 -0500 Subject: [PATCH] fix(crypto): stop telling users "waiting for partner" when their key is gone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit decryptPartnerAnswer returned a nullable payload, and null meant two opposite things: the partner hasn't released their key yet (wait — all is well), or the keybox is sitting right there and this device cannot open it (your key is gone). The reveal UI rendered WAITING_FOR_PARTNER for both. Someone whose key was lost was told to keep waiting for something that had already happened — and could never report the bug accurately, because the app described it wrongly. A LOST_LOCAL_KEY phase with honest copy has existed the whole time; this path just never reached it. Now a typed PartnerAnswerResult: WaitingForPartner (no keybox), Decrypted, or KeyUnavailable (keybox present, no local key and no usable escrow — or a payload we hold the key for and still can't open, which is broken, not pending). Mutation-checked: collapsing KeyUnavailable back into WaitingForPartner fails exactly the three tests that assert the distinction. Repaired collateral from my own edit: the replacement over-cut and deleted releaseOwnKeyForThread / decryptOwnThreadAnswer / decryptPartnerThreadAnswer. Compile caught it; restored verbatim from git, then diffed the function list before/after to prove nothing else went missing. Co-Authored-By: Claude Opus 4.8 --- .../app/closer/crypto/SealedRevealManager.kt | 30 +++++-- .../ui/answers/AnswerRevealViewModel.kt | 20 +++-- .../closer/crypto/PartnerAnswerResultTest.kt | 86 +++++++++++++++++++ 3 files changed, 122 insertions(+), 14 deletions(-) create mode 100644 app/src/test/java/app/closer/crypto/PartnerAnswerResultTest.kt diff --git a/app/src/main/java/app/closer/crypto/SealedRevealManager.kt b/app/src/main/java/app/closer/crypto/SealedRevealManager.kt index 7eaf7b6e..b4f2fec7 100644 --- a/app/src/main/java/app/closer/crypto/SealedRevealManager.kt +++ b/app/src/main/java/app/closer/crypto/SealedRevealManager.kt @@ -79,6 +79,20 @@ class SealedRevealManager @Inject constructor( * @return The decrypted [SealedAnswerEncryptor.AnswerPayload], or null if the * partner has not yet released their key. */ + /** + * Why this is a typed result and not a nullable payload: null conflated two opposite situations — + * "the partner hasn't released their key yet" (wait, all is well) and "the keybox is right there + * but we can't open it" (this device's key is gone). The UI showed WAITING_FOR_PARTNER for both, + * so a user whose key was lost was told to keep waiting for something that had already happened. + */ + sealed interface PartnerAnswerResult { + /** No keybox yet — the partner genuinely hasn't released their key. */ + data object WaitingForPartner : PartnerAnswerResult + data class Decrypted(val payload: SealedAnswerEncryptor.AnswerPayload) : PartnerAnswerResult + /** The keybox exists but this device can't open it: no local key and no usable escrow. */ + data object KeyUnavailable : PartnerAnswerResult + } + suspend fun decryptPartnerAnswer( coupleId: String, date: String, @@ -86,13 +100,13 @@ class SealedRevealManager @Inject constructor( partnerId: String, userId: String, encryptedPayload: String - ): SealedAnswerEncryptor.AnswerPayload? { + ): PartnerAnswerResult { val keybox = releaseKeyDataSource.readReleaseKey( coupleId = coupleId, date = date, senderUserId = partnerId, recipientUserId = userId - ) ?: return null + ) ?: return PartnerAnswerResult.WaitingForPartner // loadPrivateKey — NOT getOrCreatePrivateKey. Generating a fresh keypair here would mismatch // the published public key the partner sealed to, turning a recoverable state into garbage. @@ -100,7 +114,8 @@ class SealedRevealManager @Inject constructor( // first: that is exactly the case this used to fail silently on. val myPrivateKey = userKeyManager.loadPrivateKey() ?: restorePrivateKeyFromEscrow(userId, coupleId) - ?: return null + ?: return PartnerAnswerResult.KeyUnavailable + val oneTimeKey = releaseKeyEncryptor.unwrapFromSender( keyboxB64 = keybox, recipientPrivateKey = myPrivateKey, @@ -110,20 +125,17 @@ class SealedRevealManager @Inject constructor( recipientUserId = userId ) - return sealedAnswerEncryptor.open( + val payload = sealedAnswerEncryptor.open( encryptedPayload = encryptedPayload, keyHandle = oneTimeKey, coupleId = coupleId, questionId = questionId, userId = partnerId ) + // A keybox we hold the key for but still can't open is a broken/foreign payload, not a wait. + return payload?.let { PartnerAnswerResult.Decrypted(it) } ?: PartnerAnswerResult.KeyUnavailable } - // ── Thread reveal ───────────────────────────────────────────────────────────── - // Thread answers use the threadId (not a date) as the key-store identifier and - // as the AAD "questionId", so daily-question and thread keys for the same - // question ID are always cryptographically distinct. - suspend fun releaseOwnKeyForThread( coupleId: String, threadId: String, diff --git a/app/src/main/java/app/closer/ui/answers/AnswerRevealViewModel.kt b/app/src/main/java/app/closer/ui/answers/AnswerRevealViewModel.kt index 1f4fa2d3..5a4ce3f8 100644 --- a/app/src/main/java/app/closer/ui/answers/AnswerRevealViewModel.kt +++ b/app/src/main/java/app/closer/ui/answers/AnswerRevealViewModel.kt @@ -386,7 +386,7 @@ class AnswerRevealViewModel @Inject constructor( return } - val payload = runCatching { + val result = runCatching { sealedRevealManager.decryptPartnerAnswer( coupleId = coupleId, date = effectiveDate(state.partnerAnswer), @@ -395,11 +395,21 @@ class AnswerRevealViewModel @Inject constructor( userId = userId, encryptedPayload = encryptedPayload ) - }.onFailure { crashReporter.recordException(it) }.getOrNull() + }.onFailure { crashReporter.recordException(it) } + .getOrDefault(SealedRevealManager.PartnerAnswerResult.KeyUnavailable) - if (payload == null) { - _uiState.update { it.copy(sealedRevealPhase = SealedRevealPhase.WAITING_FOR_PARTNER) } - return + val payload = when (result) { + is SealedRevealManager.PartnerAnswerResult.Decrypted -> result.payload + // Say the true thing. "Waiting for partner" here used to mean "your key is gone" as often + // as it meant waiting, and the user had no way to tell — or to report it. + SealedRevealManager.PartnerAnswerResult.KeyUnavailable -> { + _uiState.update { it.copy(sealedRevealPhase = SealedRevealPhase.LOST_LOCAL_KEY) } + return + } + SealedRevealManager.PartnerAnswerResult.WaitingForPartner -> { + _uiState.update { it.copy(sealedRevealPhase = SealedRevealPhase.WAITING_FOR_PARTNER) } + return + } } // The sealed payload carries only option IDs; map them to labels via the question so the diff --git a/app/src/test/java/app/closer/crypto/PartnerAnswerResultTest.kt b/app/src/test/java/app/closer/crypto/PartnerAnswerResultTest.kt new file mode 100644 index 00000000..140b5ff7 --- /dev/null +++ b/app/src/test/java/app/closer/crypto/PartnerAnswerResultTest.kt @@ -0,0 +1,86 @@ +package app.closer.crypto + +import app.closer.core.crash.CrashReporter +import app.closer.data.remote.FirestoreDeviceKeyDataSource +import app.closer.data.remote.FirestoreReleaseKeyDataSource +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * "Waiting for your partner" must mean the partner. Nothing else. + * + * This used to be a nullable payload, and null meant two opposite things: the partner hasn't released + * their key yet (wait — all is well), or the keybox is right there and this device can't open it + * (your key is gone). The UI said WAITING_FOR_PARTNER for both, so someone whose key was lost was + * told to keep waiting for something that had already happened — a bug they could never report + * accurately, because the app described it wrongly. + */ +class PartnerAnswerResultTest { + + private val userKeyManager: UserKeyManager = mockk() + private val pendingAnswerKeyStore: PendingAnswerKeyStore = mockk() + private val releaseKeyEncryptor: ReleaseKeyEncryptor = mockk() + private val sealedAnswerEncryptor: SealedAnswerEncryptor = mockk() + private val deviceKeyDataSource: FirestoreDeviceKeyDataSource = mockk() + private val releaseKeyDataSource: FirestoreReleaseKeyDataSource = mockk() + private val encryptionManager: CoupleEncryptionManager = mockk() + private val fieldEncryptor: FieldEncryptor = mockk() + private val crashReporter: CrashReporter = mockk(relaxUnitFun = true) + + private val manager = SealedRevealManager( + userKeyManager, pendingAnswerKeyStore, releaseKeyEncryptor, sealedAnswerEncryptor, + deviceKeyDataSource, releaseKeyDataSource, encryptionManager, fieldEncryptor, crashReporter + ) + + private suspend fun decrypt() = manager.decryptPartnerAnswer( + coupleId = "c1", + date = "2026-07-16", + questionId = "q1", + partnerId = "partner", + userId = "me", + encryptedPayload = "sealed:v1:whatever" + ) + + @Test + fun `no keybox yet means we really are waiting for the partner`() = runTest { + coEvery { releaseKeyDataSource.readReleaseKey(any(), any(), any(), any()) } returns null + + assertEquals(SealedRevealManager.PartnerAnswerResult.WaitingForPartner, decrypt()) + } + + @Test + fun `keybox present but no local key and no escrow is LOST, not waiting`() = runTest { + // The exact second-device case. Saying "waiting for partner" here is the lie. + coEvery { releaseKeyDataSource.readReleaseKey(any(), any(), any(), any()) } returns "keybox:v1:x" + every { userKeyManager.loadPrivateKey() } returns null + coEvery { deviceKeyDataSource.getPrivateKeyEscrow("me") } returns null + + assertEquals(SealedRevealManager.PartnerAnswerResult.KeyUnavailable, decrypt()) + } + + @Test + fun `keybox present and escrow unreadable is LOST, not waiting`() = runTest { + // Escrow exists but the couple key isn't here (or the blob is corrupt) — still lost, still + // must not claim we're waiting. + coEvery { releaseKeyDataSource.readReleaseKey(any(), any(), any(), any()) } returns "keybox:v1:x" + every { userKeyManager.loadPrivateKey() } returns null + coEvery { deviceKeyDataSource.getPrivateKeyEscrow("me") } returns "enc:v1:blob" + every { encryptionManager.aeadFor("c1") } returns null + + assertEquals(SealedRevealManager.PartnerAnswerResult.KeyUnavailable, decrypt()) + } + + @Test + fun `escrow failure is recorded, never thrown into the reveal flow`() = runTest { + coEvery { releaseKeyDataSource.readReleaseKey(any(), any(), any(), any()) } returns "keybox:v1:x" + every { userKeyManager.loadPrivateKey() } returns null + coEvery { deviceKeyDataSource.getPrivateKeyEscrow("me") } throws RuntimeException("offline") + + assertEquals(SealedRevealManager.PartnerAnswerResult.KeyUnavailable, decrypt()) + io.mockk.coVerify { crashReporter.recordException(any()) } + } +}