2ba9a732c7e9

fix: sanitize sync entries and re-cap password history on merge (v2.10.19)

Audit finding MN-3, slice 4 (final) of the verified-fix queue. Sync is live, so unsanitized peer entries were a Major-class gap. - New vault/SyncEntrySanitizer (pure, mirrors the browser's sanitizeRestoredEntry): blank titles dropped; husk passkeys (missing credentialId/publicKeyCose/rpId/userHandle/userName) drop the whole entry; credential-less logins dropped unless a passkey is present — nested or the browser's flat shape, which consolidatePasskeyEntries merges on unlock, so browser->Android passkey sync keeps working - passwordHistory re-capped to 50 on merge (a peer could otherwise plant unbounded history) - Wired into VaultRepository.applyMergedRemoteEntries, the single funnel for inbound sync entries (id-binding + HV-1 clamping already existed) Tests: SyncEntrySanitizerTest (7). Full suite 924, only the 5 known pre-existing failures.

time
2026-08-03 23:51 UTC · 2 months ago
machine
koh2
parent
96e60825c044
4 files VERSION+1 −1app/src/main/java/com/peachpasswords/vault/SyncEntrySanitizer.kt+42 −0app/src/main/java/com/peachpasswords/vault/VaultRepository.kt+5 −2app/src/test/java/com/peachpasswords/vault/SyncEntrySanitizerTest.kt+84 −0
VERSION+1 −1
@@ -1 +1 @@1-2.10.181+2.10.19

view file

app/src/main/java/com/peachpasswords/vault/SyncEntrySanitizer.kt+42 −0
@@ -0,0 +1,42 @@1+package com.peachpasswords.vault2+3+/**4+ * MN-3 (2026-06-09 audit, reopened 2026-08-03): content sanitization for5+ * entries arriving from a sync peer. The typed Kotlin model already rejects6+ * missing REQUIRED fields at decode time; this covers what decode cannot:7+ * blank-but-present values, credential-less login husks, and unbounded8+ * passwordHistory. Mirrors the browser's `sanitizeRestoredEntry`9+ * (vault.ts) — with one Android-specific allowance: passkey fields may10+ * arrive FLAT on the entry (the browser's legacy shape, see VaultEntry's11+ * MN-5 note), which `consolidatePasskeyEntries` merges on unlock.12+ *13+ * Returns null when the entry must be dropped entirely.14+ */15+object SyncEntrySanitizer {16+17+    fun sanitize(entry: VaultEntry, historyMax: Int): VaultEntry? {18+        val title = entry.title.trim()19+        if (title.isEmpty()) return null20+21+        // A nested passkey missing its identifying fields is a credential-less22+        // husk — refuse the whole entry rather than import it (browser parity:23+        // "entry claimed a passkey but the shape didn't validate").24+        entry.passkey?.let { pk ->25+            if (pk.credentialId.isBlank() || pk.publicKeyCose.isBlank() ||26+                pk.rpId.isBlank() || pk.userHandle.isBlank() || pk.userName.isBlank()27+            ) {28+                return null29+            }30+        }31+32+        // Login entries must carry a password OR a passkey (nested or the33+        // browser's flat shape, identified by rpId + credentialId).34+        val hasFlatPasskey = !entry.rpId.isNullOrBlank() && !entry.credentialId.isNullOrBlank()35+        if (entry.isLogin() && entry.passkey == null && entry.password == null && !hasFlatPasskey) {36+            return null37+        }38+39+        val history = entry.passwordHistory?.takeIf { it.isNotEmpty() }?.take(historyMax)40+        return entry.copy(title = title, passwordHistory = history)41+    }42+}

view file

app/src/main/java/com/peachpasswords/vault/VaultRepository.kt+5 −2
@@ -2012,8 +2012,11 @@20122012         // entry against every honest local edit. The browser clamps in the20132013         // matching place (vault.ts applyMergedRemoteEntries).20142014         val mergeNow = System.currentTimeMillis()2015-        val clamped = remoteEntries.map { entry ->2016-            entry.copy(2015+        val clamped = remoteEntries.mapNotNull { entry ->2016+            // MN-3: drop peer entries that decode cleanly but are semantically2017+            // hollow (blank title, credential-less login, husk passkey) and2018+            // re-cap passwordHistory before anything merges into the vault.2019+            SyncEntrySanitizer.sanitize(entry, PASSWORD_HISTORY_MAX)?.copy(20172020                 modifiedAt = SyncLimits.clampSyncTimestamp(entry.modifiedAt, mergeNow),20182021                 deletedAt = entry.deletedAt?.let {20192022                     SyncLimits.clampSyncTimestamp(it, mergeNow)

view file

app/src/test/java/com/peachpasswords/vault/SyncEntrySanitizerTest.kt+84 −0
@@ -0,0 +1,84 @@1+package com.peachpasswords.vault2+3+import org.junit.Assert.assertEquals4+import org.junit.Assert.assertNotNull5+import org.junit.Assert.assertNull6+import org.junit.Test7+8+/**9+ * MN-3: sync-peer entries are sanitized before merge — mirrors the browser's10+ * sanitizeRestoredEntry, with the Android allowance for flat passkey fields.11+ */12+class SyncEntrySanitizerTest {13+    private val historyMax = 5014+15+    @Test fun blank_titles_are_dropped() {16+        assertNull(SyncEntrySanitizer.sanitize(entry(title = ""), historyMax))17+        assertNull(SyncEntrySanitizer.sanitize(entry(title = "   "), historyMax))18+    }19+20+    @Test fun titles_are_trimmed_and_valid_entries_pass() {21+        val sanitized = SyncEntrySanitizer.sanitize(entry(title = "  GitHub  "), historyMax)22+        assertEquals("GitHub", sanitized?.title)23+    }24+25+    @Test fun credential_less_logins_are_dropped() {26+        assertNull(SyncEntrySanitizer.sanitize(entry(password = null), historyMax))27+        assertNotNull(SyncEntrySanitizer.sanitize(entry(password = "hunter2"), historyMax))28+    }29+30+    @Test fun passkey_only_logins_survive_nested_or_flat() {31+        // Nested passkey, no password — the modern shape.32+        val nested = entry(password = null).copy(passkey = passkey())33+        assertNotNull(SyncEntrySanitizer.sanitize(nested, historyMax))34+        // Flat passkey fields (browser legacy shape) — consolidated on unlock.35+        val flat = entry(password = null).copy(rpId = "example.com", credentialId = "cred-1")36+        assertNotNull(SyncEntrySanitizer.sanitize(flat, historyMax))37+    }38+39+    @Test fun husk_passkeys_drop_the_whole_entry() {40+        val husk = entry(password = null).copy(passkey = passkey(credentialId = ""))41+        assertNull(SyncEntrySanitizer.sanitize(husk, historyMax))42+        val noKey = entry(password = null).copy(passkey = passkey(publicKeyCose = " "))43+        assertNull(SyncEntrySanitizer.sanitize(noKey, historyMax))44+    }45+46+    @Test fun non_login_types_need_no_password() {47+        val card = entry(type = "card", password = null).copy(cardNumber = "4111111111111111")48+        assertNotNull(SyncEntrySanitizer.sanitize(card, historyMax))49+    }50+51+    @Test fun password_history_is_re_capped_on_merge() {52+        val bloated = (1..(historyMax + 25)).map { PasswordHistoryItem(password = "pw-$it", changedAt = it.toLong()) }53+        val sanitized = SyncEntrySanitizer.sanitize(entry(passwordHistory = bloated), historyMax)54+        assertEquals(historyMax, sanitized?.passwordHistory?.size)55+        assertEquals("pw-1", sanitized?.passwordHistory?.first()?.password)56+57+        val small = listOf(PasswordHistoryItem(password = "old", changedAt = 1))58+        assertEquals(small, SyncEntrySanitizer.sanitize(entry(passwordHistory = small), historyMax)?.passwordHistory)59+        assertNull(SyncEntrySanitizer.sanitize(entry(passwordHistory = null), historyMax)?.passwordHistory)60+    }61+62+    private fun entry(63+        title: String = "Example",64+        type: String? = "login",65+        password: String? = "secret",66+        passwordHistory: List<PasswordHistoryItem>? = null,67+    ) = VaultEntry(68+        id = "entry-1",69+        type = type,70+        title = title,71+        password = password,72+        passwordHistory = passwordHistory,73+        createdAt = 1,74+        modifiedAt = 2,75+    )76+77+    private fun passkey(credentialId: String = "cred-1", publicKeyCose: String = "cose") = PasskeyData(78+        credentialId = credentialId,79+        publicKeyCose = publicKeyCose,80+        rpId = "example.com",81+        userHandle = "handle",82+        userName = "june",83+    )84+}

view file