142e04d50602
M-04: hidden in-memory session state guarded; sync accessors null when locked; failed create/adopt discards candidate
- time
- 2026-07-29 13:10 UTC · 2 months ago
- machine
- koh2
- agent
- claude-opus-4
- session
- audit-remediation-2026-07-29
- parent
- 9e26e136f055
↓ peach-android-142e04d5.tar.gzatlas at this save
files changed3 files
app/src/main/java/com/peachpasswords/vault/VaultRepository.kt+43 −6app/src/test/java/com/peachpasswords/vault/HiddenSessionStateTest.kt+117 −0app/src/test/java/com/peachpasswords/vault/RecoveryEscrowTest.kt+4 −2app/src/main/java/com/peachpasswords/vault/VaultRepository.kt+43 −6
@@ -286,8 +286,15 @@286286 decoyMode = false287287 rootKey = generateRecoverableRootKey()288288 }289+ // M-04: the fields above have to be live before sealSlotA, since it290+ // seals currentPayload(). That stages a root key the session would291+ // otherwise keep after a failed create — [lock] discards the whole292+ // staged session on every non-success exit below.289293 val slotA = sealSlotA(key, salt)290- if (!store.writeSlotA(slotA)) return CreateVaultResult.Error("storage_write_failed")294+ if (!store.writeSlotA(slotA)) {295+ lock()296+ return CreateVaultResult.Error("storage_write_failed")297+ }291298 // P1-3 (codex): every later step must roll the slot pair back if it292299 // fails — leaving a half-built vault is worse than refusing to make it.293300 val created = try {@@ -301,7 +308,10 @@301308 store.clearVault()302309 false303310 }304- if (!created) return CreateVaultResult.Error("storage_write_failed")311+ if (!created) {312+ lock()313+ return CreateVaultResult.Error("storage_write_failed")314+ }305315 mutateState {306316 derivedKey = key307317 publish()@@ -309,6 +319,7 @@309319 }310320 CreateVaultResult.Success311321 } catch (e: Exception) {322+ lock()312323 CreateVaultResult.Error(e.message ?: "create_failed")313324 }314325 }@@ -463,6 +474,11 @@463474 /** Adopt a derived slot-A [key]/[payload] as the live (real-vault) session. */464475 private fun adoptSlotA(key: ByteArray, payload: VaultPayload): UnlockResult {465476 return try {477+ // M-04: ensureRootKey and autoPurgeExpiredTrash both persist, so they478+ // can throw after the fields above are live. There is no prior session479+ // worth restoring — adoption has already replaced it — so a failed480+ // adoption discards everything rather than leaving entries and keys481+ // resident behind _isUnlocked=false.466482 mutateState {467483 derivedKey = key468484 decoyMode = false@@ -478,6 +494,7 @@478494 }479495 UnlockResult.Success480496 } catch (e: Exception) {497+ lock()481498 UnlockResult.Error(e.message ?: "unlock_failed")482499 }483500 }@@ -712,7 +729,13 @@712729 * fail on their other devices — a duress indicator, which is the one thing713730 * a duress vault must never be.714731 */715- fun syncMasterKey(): ByteArray? = if (decoyMode) null else rootKey?.copyOf()732+ // M-04: the "null when locked" half of the contract was documented but never733+ // enforced — a rootKey outlives _isUnlocked in the recovery half-unlock (see734+ // [unlockWithRecoveryWords], which installs a root key with no password key735+ // yet) and in a create that failed after staging. Sync must fail closed736+ // there, so the authoritative unlocked flag gates it, not just decoyMode.737+ fun syncMasterKey(): ByteArray? =738+ if (decoyMode || !_isUnlocked.value) null else rootKey?.copyOf()716739 717740 /**718741 * The current session's root key, decoy included. For local-only uses that@@ -728,7 +751,8 @@728751 /** Stable per-vault id two paired devices must share to sync. Null when729752 * locked, and never present for the decoy vault (see [syncMasterKey]). */730753 fun syncVaultId(): String? =731- if (decoyMode) null else rootKey?.let { com.peachpasswords.sync.computeVaultId(it) }754+ if (decoyMode || !_isUnlocked.value) null755+ else rootKey?.let { com.peachpasswords.sync.computeVaultId(it) }732756 733757 /**734758 * Browser parity (MV-8): rotate the sync identity by replacing the stable@@ -1347,17 +1371,30 @@13471371 requireUnlocked()13481372 // H-03: hash + escrow-related state fields set under stateLock so a13491373 // concurrent lock() cannot zero them in the middle of buildRecoveryEscrow.1350- mutateState {1374+ // M-04: the flags have to be live before the escrows are built — both seal1375+ // currentPayload() — so capture the prior pair and put it back if the write1376+ // never lands. Otherwise the session reports words set against a slot A1377+ // that still holds no escrow, and the recovery screen never offers a retry.1378+ val prior = mutateState {1379+ val priorPair = recoveryWordHash to recoveryWordsSet13511380 recoveryWordHash = recoveryWordsHash(words)13521381 recoveryWordsSet = true1382+ priorPair13531383 }13541384 // Seal TWO escrows (MF-1): the browser-byte-compatible root-key escrow13551385 // (cross-platform recovery of the sync root key) and Android's whole-payload13561386 // escrow (offline forgot-password restore of entries). Both derive their key13571387 // from the same `wordsToRootKeyMaterial` IKM but use distinct AADs.1358- runCatching {1388+ val persisted = runCatching {13591389 persistSlotAWithEscrows(buildRecoveryEscrow(words), buildRootKeyEscrow(words))13601390 }.getOrDefault(false)1391+ if (!persisted) {1392+ mutateState {1393+ recoveryWordHash = prior.first1394+ recoveryWordsSet = prior.second1395+ }1396+ }1397+ persisted13611398 }13621399 13631400 /** Verify entered words against the stored recovery hash. */app/src/test/java/com/peachpasswords/vault/HiddenSessionStateTest.kt+117 −0
@@ -0,0 +1,117 @@1+package com.peachpasswords.vault2+3+import kotlinx.coroutines.runBlocking4+import org.junit.Assert.assertFalse5+import org.junit.Assert.assertNotNull6+import org.junit.Assert.assertNull7+import org.junit.Assert.assertTrue8+import org.junit.Test9+10+/**11+ * M-04: no operation may leave in-memory session state that disagrees with what12+ * is on disk, and the sync-key accessors must fail closed whenever the session13+ * is not a fully-unlocked real vault.14+ */15+class HiddenSessionStateTest {16+17+ private class FaultStorage : VaultStorage {18+ var slotA: StoredVault? = null19+ var slotB: String? = null20+ var failWriteSlotA = false21+ private var ver: Long = 022+ private var settings = VaultSettings()23+ override fun loadSlotA() = slotA24+ override fun hasVault() = slotA != null25+ override fun writeSlotA(slot: StoredVault): Boolean {26+ if (failWriteSlotA) return false27+ slotA = slot28+ return true29+ }30+ override fun loadSlotB() = slotB31+ override fun writeSlotB(blobBase64: String): Boolean { slotB = blobBase64; return true }32+ var decoy: StoredVault? = null33+ override fun loadDecoySlot() = decoy34+ override fun writeDecoySlot(slot: StoredVault): Boolean { decoy = slot; return true }35+ override fun hasDecoySlot() = decoy != null36+ override fun clearDecoySlot() { decoy = null }37+ override fun version() = ver38+ override fun stampLocalMutation(): Long { ver = maxOf(System.currentTimeMillis(), ver + 1); return ver }39+ override fun loadSettings() = settings40+ override fun writeSettings(s: VaultSettings): Boolean { settings = s; return true }41+ override fun clearVault() { slotA = null; slotB = null; ver = 0; settings = VaultSettings() }42+ }43+44+ private fun recoveryWordsOf(repo: VaultRepository): List<String> {45+ val key = repo.sessionRootKey()!!46+ return try { rootKeyToWords(key) } finally { VaultCrypto.zero(key) }47+ }48+49+ // A failed escrow write must not leave the session claiming words are set:50+ // the recovery screen would stop offering the confirm step against a slot A51+ // that holds no escrow at all.52+ @Test fun failed_escrow_write_restores_prior_recovery_flags() = runBlocking {53+ val storage = FaultStorage()54+ val repo = VaultRepository(storage)55+ repo.createVault("pw")56+ val words = recoveryWordsOf(repo)57+ assertFalse(repo.isRecoveryWordsSet())58+59+ storage.failWriteSlotA = true60+ assertFalse(repo.setRecoveryWords(words))61+ assertFalse("flags must roll back when the escrow never landed", repo.isRecoveryWordsSet())62+ assertFalse(repo.verifyRecoveryWords(words))63+ assertFalse(repo.hasRecoveryEscrow())64+65+ // And the retry still works once storage recovers.66+ storage.failWriteSlotA = false67+ assertTrue(repo.setRecoveryWords(words))68+ assertTrue(repo.isRecoveryWordsSet())69+ assertTrue(repo.verifyRecoveryWords(words))70+ assertTrue(repo.hasRecoveryEscrow())71+ }72+73+ @Test fun sync_accessors_are_null_when_locked() = runBlocking {74+ val repo = VaultRepository(FaultStorage())75+ repo.createVault("pw")76+ assertNotNull(repo.syncMasterKey())77+ assertNotNull(repo.syncVaultId())78+ repo.lock()79+ assertNull(repo.syncMasterKey())80+ assertNull(repo.syncVaultId())81+ }82+83+ // A recovery unlock installs a root key but no password key — the session is84+ // not unlocked yet, so nothing may leave the device under that identity.85+ @Test fun sync_accessors_are_null_during_recovery_half_unlock() = runBlocking {86+ val storage = FaultStorage()87+ val repo = VaultRepository(storage)88+ repo.createVault("pw")89+ val words = recoveryWordsOf(repo)90+ assertTrue(repo.setRecoveryWords(words))91+ repo.lock()92+93+ assertTrue(repo.unlockWithRecoveryWords(words))94+ assertFalse(repo.isUnlocked.value)95+ assertNull(repo.syncMasterKey())96+ assertNull(repo.syncVaultId())97+ assertNotNull("the local-only accessor still sees the recovered key", repo.sessionRootKey())98+99+ // Finishing the reset unlocks the session and the sync identity returns.100+ assertTrue(repo.resetPasswordWithRecoveryWords(words, "new-pw"))101+ assertTrue(repo.isUnlocked.value)102+ assertNotNull(repo.syncMasterKey())103+ assertNotNull(repo.syncVaultId())104+ }105+106+ @Test fun failed_create_retains_no_session_state() = runBlocking {107+ val storage = FaultStorage()108+ storage.failWriteSlotA = true109+ val repo = VaultRepository(storage)110+111+ assertTrue(repo.createVault("pw") is CreateVaultResult.Error)112+ assertFalse(repo.isUnlocked.value)113+ assertNull("a failed create must not retain a root key", repo.sessionRootKey())114+ assertNull(repo.syncMasterKey())115+ assertNull(repo.currentVaultKey())116+ }117+}app/src/test/java/com/peachpasswords/vault/RecoveryEscrowTest.kt+4 −2
@@ -182,14 +182,16 @@182182 repo.createVault("pw")183183 repo.addEntry(login("Local"))184184 val words = recoveryWordsOf(repo)185- val rootKeyBefore = repo.syncMasterKey()!!185+ val rootKeyBefore = repo.sessionRootKey()!!186186 repo.setRecoveryWords(words)187187 // Simulate a browser-made vault: drop the Android-only whole-payload escrow.188188 storage.slotA = storage.slotA!!.copy(recoveryEscrow = null)189189 repo.lock()190190 191191 assertTrue(repo.unlockWithRecoveryWords(words))192- assertArrayEquals(rootKeyBefore, repo.syncMasterKey()) // same root key recovered192+ // sessionRootKey, not syncMasterKey: a recovery unlock has no password key193+ // yet, so the sync accessors fail closed until the reset completes (M-04).194+ assertArrayEquals(rootKeyBefore, repo.sessionRootKey()) // same root key recovered193195 assertTrue(repo.getEntries().isEmpty()) // entries return via sync194196 }195197