Fix: promoting a link to the vault appeared to do nothing

🔴 `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 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-09-04 16:19:18 -04:00
co-authored by Claude Opus 5
parent 2d05bbe7c4
commit 0a4709dd0e
3 changed files with 135 additions and 7 deletions
+20 -3
View File
@@ -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
@@ -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. */
@@ -404,6 +404,72 @@ class LocalVaultTest {
assertFailsWith<LocalVault.VaultException> { 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