From 0a4709dd0ea7c4da97796257dafba5bbf8ca23ab Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Fri, 4 Sep 2026 16:19:18 -0400 Subject: [PATCH] Fix: promoting a link to the vault appeared to do nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🔴 `Node.equals` compared only the id. It was written that way to dodge a real trap — a data class compares `ByteArray` by identity, so the generated equals calls two identical rows different — and dodging it broke a whole feature. Promoting a link keeps the node's id and changes its `storage`. So the new row compared EQUAL to the one it replaced, the state object holding it compared equal too, `StateFlow` conflated the update away, and Compose never saw it. On screen the file vanished from Linked and did not appear in the vault until the app was restarted. That reads as losing somebody's file. Every field is compared now, with `contentEquals` for the four byte arrays — the identity trap handled rather than avoided. ⚠️ **This suite could not have caught it, and the new tests say why.** Everything here reads the vault, and the vault was right the whole time: the index on disk had both rows, and a fresh load rendered both. The failure was in what the vault handed a state holder that conflates on equality. So the tests assert that property directly — a promoted row must not equal its old self, an unchanged row read twice must, and every mutable field must be visible in equality. Found by watching it on an emulator. Confirmed by restoring the old equals and watching both go red, then re-verified on device: the promoted row now appears immediately with no restart, and its size corrects from the provider's reported 4 kB to the real 14 kB plaintext length — which is itself proof the row genuinely updated. Also verified on device while here: a persistable read grant survives a reboot, a full cold boot and an app update (`persistable=0x1 persisted=0x1`), promoting drops the bookmark from the index, and the person's own file is left untouched where they put it. Co-Authored-By: Claude Opus 5 --- PARITY.md | 23 ++++++- .../kotlin/io/flamenet/client/LocalVault.kt | 53 +++++++++++++-- .../io/flamenet/client/LocalVaultTest.kt | 66 +++++++++++++++++++ 3 files changed, 135 insertions(+), 7 deletions(-) diff --git a/PARITY.md b/PARITY.md index 17f455f..24a54f5 100644 --- a/PARITY.md +++ b/PARITY.md @@ -379,9 +379,26 @@ both wrapped keys carry version `0x11` and the name `0x01`, the filename is not in the clear, and the plaintext is absent from the entire app container. Linking a file works and reports "Nothing was copied". -⚠️ **Still owed: whether a persistable read grant survives a reboot.** The -emulator reboot ran at a machine load of 38 and the launcher itself ANR'd, which -is not a condition that produces evidence about anything. +✅ **The persistable read grant survives.** Checked after a device reboot, a +full emulator cold boot AND an app update: `dumpsys activity permissions` shows +`persistable=0x1 persisted=0x1` for the document, and the row reads through to +the file rather than reporting it moved. Promoting a link to the vault also +works on device — the bookmark is dropped from the index, the original file is +left untouched where its owner put it, and the byte count corrects from the +provider's reported size to the real plaintext length. + +🔴 **The device run found a second bug: promoting a link appeared to do +nothing.** `Node.equals` compared only the id — written to dodge `ByteArray` +identity comparison — so a promoted row compared EQUAL to the reference it +replaced. The state object holding it compared equal too, `StateFlow` conflated +the update away, and the screen never changed: the file vanished from Linked and +did not appear in the vault until the app was restarted, which reads as losing +somebody's file. Every field is now compared, with `contentEquals` for the byte +arrays. + +⚠️ Neither of the two device bugs was catchable by this suite as it stood. The +tests read the vault; the failures were in what the vault handed a `StateFlow` +and in what the platform's JSON handed the decoder. 🔴 **Android asks for a Files passphrase, exactly as iOS does, and this was a decision rather than a port.** The Android wallet sits behind the Keystore-sealed diff --git a/client/src/main/kotlin/io/flamenet/client/LocalVault.kt b/client/src/main/kotlin/io/flamenet/client/LocalVault.kt index 41044b8..39fe68c 100644 --- a/client/src/main/kotlin/io/flamenet/client/LocalVault.kt +++ b/client/src/main/kotlin/io/flamenet/client/LocalVault.kt @@ -146,10 +146,55 @@ class LocalVault( ) { val label: Label get() = Label(classification, compartments.toSet()) - // Hand-written because a data class compares ByteArray by identity, so - // the generated equals would call two identical rows different. - override fun equals(other: Any?): Boolean = other is Node && other.id == id - override fun hashCode(): Int = id.hashCode() + /** + * 🔴 **Every field, with the byte arrays compared by CONTENT.** + * + * This was `other.id == id` — written to dodge the fact that a data + * class compares `ByteArray` by identity, and wrong in a way that broke + * a whole feature. Promoting a link to the vault keeps the node's id and + * changes its `storage`, so the new row compared EQUAL to the old one: + * the state object it was in compared equal too, `StateFlow` conflated + * the update away, and Compose never saw it. On screen the file vanished + * from Linked and did not appear in the vault until the app was + * restarted — which reads as losing somebody's file. + * + * ⚠️ The identity-vs-content trap is real, so it is handled rather than + * avoided: `contentEquals` for the four byte arrays, ordinary equality + * for the rest. + */ + override fun equals(other: Any?): Boolean = + other is Node && + other.id == id && + other.parentId == parentId && + other.isFolder == isFolder && + other.classification == classification && + other.compartments == compartments && + other.wrappedNodeKey.contentEquals(wrappedNodeKey) && + other.nameDigest.contentEquals(nameDigest) && + other.wrappedName.contentEquals(wrappedName) && + (other.wrappedContentKey?.contentEquals(wrappedContentKey ?: ByteArray(0)) + ?: (wrappedContentKey == null)) && + other.byteCount == byteCount && + other.addedAtMillis == addedAtMillis && + other.storage == storage && + other.bookmark == bookmark + + override fun hashCode(): Int { + var result = id.hashCode() + result = 31 * result + (parentId?.hashCode() ?: 0) + result = 31 * result + isFolder.hashCode() + result = 31 * result + classification + result = 31 * result + compartments.hashCode() + result = 31 * result + wrappedNodeKey.contentHashCode() + result = 31 * result + nameDigest.contentHashCode() + result = 31 * result + wrappedName.contentHashCode() + result = 31 * result + (wrappedContentKey?.contentHashCode() ?: 0) + result = 31 * result + byteCount + result = 31 * result + addedAtMillis.hashCode() + result = 31 * result + storage.hashCode() + result = 31 * result + (bookmark?.hashCode() ?: 0) + return result + } } /** What a screen shows: the row, plus the name if this device can read it. */ diff --git a/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt b/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt index c7d54e3..a0d80e4 100644 --- a/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt @@ -404,6 +404,72 @@ class LocalVaultTest { assertFailsWith { vault.encrypt(node.id) } } + // ---- Change detection ------------------------------------------------ + + /** + * 🔴 **A changed row must not compare equal to what it replaced.** + * + * `Node.equals` was `other.id == id`, written to dodge `ByteArray` + * identity comparison. Promoting a link keeps the node's id and changes its + * `storage`, so the new row compared equal to the old one — the state object + * holding it compared equal too, `StateFlow` conflated the update away, and + * the screen never changed. On a device the file vanished from Linked and + * did not reappear in the vault until the app was restarted, which reads as + * losing somebody's file. + * + * ⚠️ Found by watching it on an emulator, not by any test. The suite passed + * throughout, because everything here reads the vault rather than a + * `StateFlow` — so this asserts the property the UI actually depends on. + */ + @Test + fun `a promoted row does not compare equal to the reference it replaced`() { + refs.files["doc://1"] = "promote me".toByteArray() + val before = vault.reference("doc://1", "doc.pdf", secret) + vault.encrypt(before.id) + val after = assertNotNull(vault.node(before.id)) + + assertEquals(before.id, after.id, "the promote should keep the same row") + assertTrue(before != after, + "a row whose storage changed compares equal to its old self — any " + + "state holder that conflates on equality will drop the update") + assertTrue(listOf(before) != listOf(after), "the LIST compares equal too") + } + + /** ...while two reads of an unchanged row still are equal, which is the point. */ + @Test + fun `an unchanged row read twice compares equal`() { + val node = vault.add("x".toByteArray(), "f.txt", secret) + val a = assertNotNull(reopen().node(node.id)) + val b = assertNotNull(reopen().node(node.id)) + assertEquals(a, b, "two reads of one unchanged row differ — every refresh " + + "would redraw and byte arrays are being compared by identity again") + assertEquals(a.hashCode(), b.hashCode()) + } + + /** + * ⚠️ Every mutable field, not just `storage`. A rename, a move, a + * re-classification and a re-wrap all have to be visible to a UI that + * conflates on equality. + */ + @Test + fun `each field that can change is visible in equality`() { + val node = vault.add("x".toByteArray(), "f.txt", secret) + val base = assertNotNull(vault.node(node.id)) + val changed = listOf( + "parentId" to base.copy(parentId = UUID.randomUUID()), + "classification" to base.copy(classification = 9), + "compartments" to base.copy(compartments = listOf(4)), + "byteCount" to base.copy(byteCount = base.byteCount + 1), + "storage" to base.copy(storage = LocalVault.Storage.REFERENCE), + "bookmark" to base.copy(bookmark = "doc://x"), + "wrappedContentKey" to base.copy(wrappedContentKey = null), + "wrappedName" to base.copy(wrappedName = ByteArray(9)), + ) + for ((field, other) in changed) { + assertTrue(base != other, "a change to $field is invisible to equality") + } + } + // ---- Deleting -------------------------------------------------------- @Test