Measure Android coverage, and round-trip the vault
JaCoCo was not configured on either JVM module, so the Android numbers were unknown rather than good. :engine and :client are plain Kotlin/JVM precisely so they can be measured without an emulator; now they are. Baseline: engine 84.9% lines, client 47.3%. Vault was the outlier in engine at 53%, and its uncovered half was almost entirely save/load pairs — credentials, trust pins, the delivery key, the key-log pins, the roster, avatars, per-line deletion. That is not an arbitrary gap. It is the exact shape of a bug that has already shipped in this app twice. `requireKeyLog` was computed, held in memory and never written down, so a security control the UI offered silently reverted on every relaunch. `Trust.changed` says the same thing in its own doc comment: returned by `record()`, persisted by nothing, so `state()` could never answer CHANGED, the "Accept new keys" button could never render, and a contact who reinstalled became permanently unmessageable while the header insisted nothing had changed. Neither was hard to write. Both were invisible because a save and a load that disagree still compile and only differ after a restart, which no test performed. Every test here writes with one Vault and reads with a second built over the same bytes, because that is the only arrangement in which the two halves are forced to agree. Includes the AAD binding the class doc promises: a trust record copied into another contact's slot must not open, or someone with file access could attribute a stranger's keys to someone you trust. Vault 53.0% -> 84.9% lines, engine 84.9% -> 92.3%. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
0eea69b023
commit
a40cc2d2d4
@@ -6,6 +6,7 @@
|
||||
// OkHttp works identically on both.
|
||||
plugins {
|
||||
kotlin("jvm")
|
||||
jacoco
|
||||
}
|
||||
|
||||
repositories { mavenCentral() }
|
||||
@@ -38,3 +39,12 @@ tasks.test {
|
||||
useJUnitPlatform()
|
||||
testLogging { events("passed", "failed", "skipped") }
|
||||
}
|
||||
|
||||
// Coverage for this module. `:engine` and `:client` are plain JVM libraries
|
||||
// precisely so they can be measured and tested without an emulator, so there is
|
||||
// no reason for their numbers to be a guess.
|
||||
tasks.withType<Test>().configureEach { finalizedBy(tasks.named("jacocoTestReport")) }
|
||||
tasks.named<JacocoReport>("jacocoTestReport") {
|
||||
dependsOn(tasks.named("test"))
|
||||
reports { xml.required.set(true); html.required.set(true) }
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
plugins {
|
||||
kotlin("jvm")
|
||||
jacoco
|
||||
}
|
||||
|
||||
repositories { mavenCentral() }
|
||||
@@ -58,3 +59,12 @@ tasks.test {
|
||||
showStandardStreams = true
|
||||
}
|
||||
}
|
||||
|
||||
// Coverage for this module. `:engine` and `:client` are plain JVM libraries
|
||||
// precisely so they can be measured and tested without an emulator, so there is
|
||||
// no reason for their numbers to be a guess.
|
||||
tasks.withType<Test>().configureEach { finalizedBy(tasks.named("jacocoTestReport")) }
|
||||
tasks.named<JacocoReport>("jacocoTestReport") {
|
||||
dependsOn(tasks.named("test"))
|
||||
reports { xml.required.set(true); html.required.set(true) }
|
||||
}
|
||||
|
||||
@@ -0,0 +1,342 @@
|
||||
package io.flamenet.e2e
|
||||
|
||||
import org.json.JSONArray
|
||||
import org.json.JSONObject
|
||||
import javax.crypto.KeyGenerator
|
||||
import javax.crypto.SecretKey
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertContentEquals
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertFailsWith
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* Round trips for everything the vault remembers.
|
||||
*
|
||||
* 🔴 **This class of bug has already shipped here once.** `requireKeyLog` was
|
||||
* computed, held in memory, and never written down, so a security control the
|
||||
* UI offered silently reverted on every relaunch. `Trust.changed` carries the
|
||||
* same warning in its own doc comment: it was returned by `record()` and
|
||||
* persisted by nothing, so a contact who reinstalled became permanently
|
||||
* unmessageable while the header insisted nothing had changed.
|
||||
*
|
||||
* Neither was a hard bug to write. Both were invisible because a save and a
|
||||
* load that disagree still compile, still run, and only differ after a restart
|
||||
* — which no test performed. Every test below writes with one `Vault` instance
|
||||
* and reads with a second one built over the same bytes, because that is the
|
||||
* only arrangement in which the two halves have to agree.
|
||||
*/
|
||||
class VaultPersistenceTest {
|
||||
|
||||
private class MapStorage : Vault.Storage {
|
||||
val records = LinkedHashMap<String, ByteArray>()
|
||||
override fun read(name: String) = records[name]
|
||||
override fun write(name: String, bytes: ByteArray) { records[name] = bytes }
|
||||
override fun delete(name: String) { records.remove(name) }
|
||||
override fun names() = records.keys.toList()
|
||||
}
|
||||
|
||||
private fun key(): SecretKey =
|
||||
KeyGenerator.getInstance("AES").apply { init(256) }.generateKey()
|
||||
|
||||
/** Write with one instance, read with another — a restart, in effect. */
|
||||
private fun <T> roundTrip(write: (Vault) -> Unit, read: (Vault) -> T): T {
|
||||
val storage = MapStorage(); val secret = key()
|
||||
write(Vault(storage, secret))
|
||||
return read(Vault(storage, secret))
|
||||
}
|
||||
|
||||
// -- Credential ----------------------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `a credential survives a restart`() {
|
||||
val original = Vault.Credential(
|
||||
relayBaseUrl = "https://relay.example", username = "ada",
|
||||
appPassword = "hunter2", userId = 42, fnid = "ada@example")
|
||||
val restored = roundTrip({ it.saveCredential(original) }, { it.loadCredential() })
|
||||
assertEquals(original, restored)
|
||||
}
|
||||
|
||||
/**
|
||||
* A credential written before addresses existed is still a valid
|
||||
* credential and must not fail to load.
|
||||
*/
|
||||
@Test
|
||||
fun `a credential with no address round-trips as null rather than empty`() {
|
||||
val restored = roundTrip({
|
||||
it.saveCredential(Vault.Credential(
|
||||
"https://relay.example", "ada", "pw", 42, fnid = null))
|
||||
}, { it.loadCredential() })
|
||||
assertNull(restored!!.fnid)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `no credential reads back as null, not as a blank one`() {
|
||||
assertNull(Vault(MapStorage(), key()).loadCredential())
|
||||
}
|
||||
|
||||
// -- Trust ---------------------------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `a trust record survives a restart`() {
|
||||
val original = Vault.Trust(
|
||||
identities = listOf("ik-one", "ik-two"), verified = true,
|
||||
fnid = "bob@example", changed = false)
|
||||
val restored = roundTrip({ it.saveTrust(7, original) }, { it.loadTrust(7) })
|
||||
assertEquals(original, restored)
|
||||
}
|
||||
|
||||
/**
|
||||
* 🔴 The flag whose absence made reinstalled contacts unmessageable. If
|
||||
* this does not persist, `state()` can never answer CHANGED, so the
|
||||
* "Accept new keys" button never renders and the only route back is
|
||||
* clearing app data.
|
||||
*/
|
||||
@Test
|
||||
fun `a detected key change is still there after a restart`() {
|
||||
val restored = roundTrip({
|
||||
it.saveTrust(7, Vault.Trust(listOf("ik-old"), verified = false, changed = true))
|
||||
}, { it.loadTrust(7) })
|
||||
assertTrue(restored!!.changed)
|
||||
// and the pinned identities are unchanged: the flag records that
|
||||
// something else was OFFERED, not that we believe it.
|
||||
assertEquals(listOf("ik-old"), restored.identities)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `trust with no address round-trips as null`() {
|
||||
val restored = roundTrip({
|
||||
it.saveTrust(7, Vault.Trust(listOf("ik"), verified = false, fnid = null))
|
||||
}, { it.loadTrust(7) })
|
||||
assertNull(restored!!.fnid)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `trust is per contact`() {
|
||||
val storage = MapStorage(); val secret = key()
|
||||
val v = Vault(storage, secret)
|
||||
v.saveTrust(1, Vault.Trust(listOf("ik-1"), verified = true))
|
||||
v.saveTrust(2, Vault.Trust(listOf("ik-2"), verified = false))
|
||||
val reread = Vault(storage, secret)
|
||||
assertEquals(listOf("ik-1"), reread.loadTrust(1)!!.identities)
|
||||
assertEquals(listOf("ik-2"), reread.loadTrust(2)!!.identities)
|
||||
assertTrue(reread.loadTrust(1)!!.verified)
|
||||
assertTrue(!reread.loadTrust(2)!!.verified)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `forgetting a contact's trust removes it and leaves others alone`() {
|
||||
val storage = MapStorage(); val secret = key()
|
||||
val v = Vault(storage, secret)
|
||||
v.saveTrust(1, Vault.Trust(listOf("ik-1"), verified = true))
|
||||
v.saveTrust(2, Vault.Trust(listOf("ik-2"), verified = true))
|
||||
v.forgetTrust(1)
|
||||
val reread = Vault(storage, secret)
|
||||
assertNull(reread.loadTrust(1))
|
||||
assertEquals(listOf("ik-2"), reread.loadTrust(2)!!.identities)
|
||||
}
|
||||
|
||||
/**
|
||||
* 🔴 **Every record's name is bound into the AAD**, so a blob is not
|
||||
* portable between slots. Without that, someone with file access could
|
||||
* paste one contact's pinned keys into another contact's slot and the app
|
||||
* would accept them — attributing a stranger's identity to someone you
|
||||
* trust.
|
||||
*/
|
||||
@Test
|
||||
fun `a trust record moved into another contact's slot does not open`() {
|
||||
val storage = MapStorage(); val secret = key()
|
||||
val v = Vault(storage, secret)
|
||||
v.saveTrust(1, Vault.Trust(listOf("ik-victim"), verified = true))
|
||||
v.saveTrust(2, Vault.Trust(listOf("ik-other"), verified = false))
|
||||
|
||||
val slotOne = storage.names().first { it.endsWith("/1") || it.endsWith("1") }
|
||||
val slotTwo = storage.names().first { it != slotOne }
|
||||
storage.records[slotTwo] = storage.records[slotOne]!!.copyOf()
|
||||
|
||||
assertFailsWith<Vault.Corrupt> { Vault(storage, secret).loadTrust(2) }
|
||||
}
|
||||
|
||||
// -- Sealed sender delivery key -----------------------------------------
|
||||
|
||||
@Test
|
||||
fun `a delivery key survives a restart with its peers`() {
|
||||
val mine = ByteArray(32) { 9 }
|
||||
val restored = roundTrip({
|
||||
it.saveDelivery(Vault.Delivery(
|
||||
key = mine, published = true,
|
||||
peers = mapOf(5L to ByteArray(32) { 1 }, 6L to ByteArray(32) { 2 }),
|
||||
toldPeers = setOf(5L)))
|
||||
}, { it.loadDelivery() })!!
|
||||
|
||||
assertContentEquals(mine, restored.key)
|
||||
assertTrue(restored.published)
|
||||
assertEquals(setOf(5L, 6L), restored.peers.keys)
|
||||
assertContentEquals(ByteArray(32) { 1 }, restored.peers[5L])
|
||||
assertEquals(setOf(5L), restored.toldPeers)
|
||||
}
|
||||
|
||||
/**
|
||||
* `published` is the relay's knowledge, not ours. Losing it across a
|
||||
* restart means either re-publishing forever or believing a key is
|
||||
* reachable when the relay has never seen its digest.
|
||||
*/
|
||||
@Test
|
||||
fun `an unpublished delivery key does not come back as published`() {
|
||||
val restored = roundTrip({
|
||||
it.saveDelivery(Vault.Delivery(ByteArray(32) { 3 }, published = false))
|
||||
}, { it.loadDelivery() })!!
|
||||
assertTrue(!restored.published)
|
||||
}
|
||||
|
||||
// -- Key transparency pins ----------------------------------------------
|
||||
|
||||
/**
|
||||
* The pin is what makes the log append-only from this device's point of
|
||||
* view: a head smaller than the one we already saw is a rewind. A pin that
|
||||
* does not survive a restart makes every launch trust whatever it is
|
||||
* handed first.
|
||||
*/
|
||||
@Test
|
||||
fun `a key log pin survives a restart`() {
|
||||
val restored = roundTrip({
|
||||
it.saveKeyLogPin(Vault.KeyLogPin(size = 128, root = "cm9vdA=="))
|
||||
}, { it.loadKeyLogPin() })!!
|
||||
assertEquals(128, restored.size)
|
||||
assertEquals("cm9vdA==", restored.root)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the pinned issuer key survives a restart`() {
|
||||
val issuer = ByteArray(32) { it.toByte() }
|
||||
val restored = roundTrip({ it.saveKeyLogIssuerPin(issuer) },
|
||||
{ it.keyLogIssuerPin() })
|
||||
assertContentEquals(issuer, restored)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `no issuer pin reads back as null`() {
|
||||
assertNull(Vault(MapStorage(), key()).keyLogIssuerPin())
|
||||
}
|
||||
|
||||
// -- Roster --------------------------------------------------------------
|
||||
|
||||
@Test
|
||||
fun `a roster survives a restart with its off-server flag`() {
|
||||
val items = JSONArray().put(JSONObject().put("id", 1).put("name", "ada"))
|
||||
val (readItems, offServer) = roundTrip({ it.saveRoster(items, offServer = true) },
|
||||
{ it.loadRoster() })
|
||||
assertEquals(1, readItems.length())
|
||||
assertEquals("ada", readItems.getJSONObject(0).getString("name"))
|
||||
assertTrue(offServer)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an absent roster reads back empty rather than null`() {
|
||||
val (items, offServer) = Vault(MapStorage(), key()).loadRoster()
|
||||
assertEquals(0, items.length())
|
||||
assertTrue(!offServer)
|
||||
}
|
||||
|
||||
// -- Avatars -------------------------------------------------------------
|
||||
|
||||
private fun ref(id: String = "blob-1") = Attachment.Ref(
|
||||
kind = Attachment.IMAGE, id = id, key = ByteArray(32) { 4 },
|
||||
digest = ByteArray(32) { 5 }, bytes = 100, mime = "image/jpeg",
|
||||
width = 64, height = 64)
|
||||
|
||||
@Test
|
||||
fun `a contact's avatar survives a restart`() {
|
||||
val restored = roundTrip({ it.saveAvatar(11, ref()) }, { it.avatarFor(11) })!!
|
||||
assertEquals("blob-1", restored.id)
|
||||
assertContentEquals(ByteArray(32) { 4 }, restored.key)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `clearing a contact's avatar is remembered`() {
|
||||
val storage = MapStorage(); val secret = key()
|
||||
Vault(storage, secret).saveAvatar(11, ref())
|
||||
Vault(storage, secret).saveAvatar(11, null)
|
||||
assertNull(Vault(storage, secret).avatarFor(11))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `my own avatar survives a restart`() {
|
||||
val restored = roundTrip({ it.saveMyAvatar(ref("mine")) }, { it.myAvatar() })!!
|
||||
assertEquals("mine", restored.id)
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a peer already has our avatar is what stops us re-sending it on
|
||||
* every message. Forgetting it across a restart turns a one-time upload
|
||||
* into a per-launch one.
|
||||
*/
|
||||
@Test
|
||||
fun `who we have already shared an avatar with is remembered`() {
|
||||
val restored = roundTrip({ it.markAvatarShared(11, "blob-1") },
|
||||
{ it.avatarSharedWith(11) })
|
||||
assertEquals("blob-1", restored)
|
||||
}
|
||||
|
||||
// -- History deletion ----------------------------------------------------
|
||||
|
||||
private fun line(at: Long, mine: Boolean, text: String) =
|
||||
JSONObject().put("at", at).put("mine", mine).put("text", text)
|
||||
|
||||
@Test
|
||||
fun `deleting one line leaves the rest of the thread intact`() {
|
||||
val storage = MapStorage(); val secret = key()
|
||||
val v = Vault(storage, secret)
|
||||
v.replaceHistory("t", JSONArray()
|
||||
.put(line(1, true, "first"))
|
||||
.put(line(2, false, "second"))
|
||||
.put(line(3, true, "third")))
|
||||
|
||||
Vault(storage, secret).deleteLine("t", at = 2, mine = false, text = "second")
|
||||
|
||||
val kept = Vault(storage, secret).history("t")
|
||||
assertEquals(2, kept.length())
|
||||
assertEquals("first", kept.getJSONObject(0).getString("text"))
|
||||
assertEquals("third", kept.getJSONObject(1).getString("text"))
|
||||
}
|
||||
|
||||
/**
|
||||
* Keyed by timestamp AND side AND text, so two messages in the same
|
||||
* millisecond cannot take each other down.
|
||||
*/
|
||||
@Test
|
||||
fun `deleting is precise about which line it removes`() {
|
||||
val storage = MapStorage(); val secret = key()
|
||||
Vault(storage, secret).replaceHistory("t", JSONArray()
|
||||
.put(line(5, true, "same ms mine"))
|
||||
.put(line(5, false, "same ms theirs"))
|
||||
.put(line(5, true, "same ms mine too")))
|
||||
|
||||
Vault(storage, secret).deleteLine("t", at = 5, mine = true, text = "same ms mine")
|
||||
|
||||
val kept = Vault(storage, secret).history("t")
|
||||
assertEquals(2, kept.length())
|
||||
assertEquals("same ms theirs", kept.getJSONObject(0).getString("text"))
|
||||
assertEquals("same ms mine too", kept.getJSONObject(1).getString("text"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `deleting a line that is not there changes nothing`() {
|
||||
val storage = MapStorage(); val secret = key()
|
||||
Vault(storage, secret).replaceHistory("t", JSONArray().put(line(1, true, "only")))
|
||||
Vault(storage, secret).deleteLine("t", at = 99, mine = true, text = "absent")
|
||||
assertEquals(1, Vault(storage, secret).history("t").length())
|
||||
}
|
||||
|
||||
/** "Deleted means deleted" — the record goes, not a tombstone. */
|
||||
@Test
|
||||
fun `forgetting a thread removes its record entirely`() {
|
||||
val storage = MapStorage(); val secret = key()
|
||||
Vault(storage, secret).replaceHistory("t", JSONArray().put(line(1, true, "hi")))
|
||||
val before = storage.names().size
|
||||
Vault(storage, secret).forgetHistory("t")
|
||||
assertEquals(before - 1, storage.names().size)
|
||||
assertEquals(0, Vault(storage, secret).history("t").length())
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user