Fix: promoting a link to the vault appeared to do nothing #83
Merged
paulhitt
merged 4 commits from 2026-09-04 20:50:12 +00:00
fix/node-equality-conflated-updates into main
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Paul HittandClaude Opus 5
|
0a4709dd0e |
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> |
||
|
Paul HittandClaude Opus 5
|
2d05bbe7c4 |
Fix: a contact with no address was pinned as the string "null"
The same platform difference as the vault index, in shipped code, with a worse consequence. 🔴 `saveTrust` wrote `"fnid": null` explicitly when a contact's global address was not yet known. Android's bundled `org.json` reads a JSON null back as the four-character string `"null"` — a non-null value that is not an address. What that caused on a phone: - `Trust.pin` compares a newly offered fnid against the pinned one and treats a difference as a different person. `"null" != "fnid:…"`, so the FIRST TIME a real address arrived the contact was marked **CHANGED** — the "security keys changed" warning, which is supposed to mean an attack, firing on an ordinary first sighting. The vault's own comment about minting a fresh device warns about exactly this shape of false alarm. - `rememberIdentity` returns early once `fnid != null`, so the real address was never resolved and the record stayed wrong permanently. - `pinnedFnid` fed `"null"` into key-log verification, which recomputes a leaf against the address a contact was accepted under — every check against the wrong subject. Two halves, because records written the old way are already on phones: the writer now omits the key entirely (an absent key reads as `""` on both platforms, which is what the reader always expected), and the reader rejects the literal `"null"` so existing records repair themselves. ⚠️ The test that matters is the one feeding the reader the literal string. On the JVM a real JSON null reads back as `""` whether the key is omitted or not, so the plain round-trip passes with the bug intact — which is why this survived every suite. Confirmed by restoring the old reader and watching it go red. Found by the sweep flagged in android!54. `JSONObject.NULL` is written in exactly two places in this app, and both were wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul HittandClaude Opus 5
|
436ce240c7 |
Fix: the vault index failed to load on a real phone
🔴 **Android's bundled `org.json` coerces a JSON `null` to the four-character string `"null"`. The Maven `org.json` the tests run against returns `""`.** Every nullable field in the vault index therefore passed on a laptop and broke on a device: - `parentID` threw `Invalid UUID string: null`, so a vault holding any top-level file failed to load AT ALL. Adding the first file worked, because it was still in memory; reopening the app lost everything, with an error nobody could act on. - `wrappedContentKey` was worse, because it did NOT throw. `"null"` is valid base64, so the node came back with a plausible three-byte content key and simply refused to decrypt, for ever, with nothing to explain it. - `bookmark` produced a reference pointing at `"null"`, permanently missing. Absence is now decided by `JSONObject.isNull` and never by comparing a decoded string to a sentinel. The literal `"null"` is rejected as well: it is not a valid value for any of these fields, and tolerating it costs nothing next to reintroducing this. ⚠️ **The two regression tests write the literal string `"null"` into the index on purpose.** That is exactly what the decoder is handed on a device, so it is what makes a JVM run reproduce the device's behaviour. Feeding a real JSON `null` — which is what the vault actually writes — passes with the broken code and proves nothing. Both were confirmed by restoring the old decoding and watching them go red. Found by running the app (android!52). No JVM test could have caught it, which is an argument for the device run rather than against the tests. ⚠️ `optString` on a nullable field is wrong everywhere in this app, not only here, and it fails only on the platform it ships to. Worth a sweep. Verified on the emulator afterwards: the vault survives a cold start, and on disk the blob begins `ENCL\x01`, both wrapped keys carry version 0x11, the name 0x01, the filename is not in the clear, and the plaintext is absent from the whole app container. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul HittandClaude Opus 5
|
6ec626cde2 |
Share a file into a chat, and save one back — with the label it carries
Both directions: pick a document from the vault inside a conversation, and save a received one into the vault at the classification it arrived with. 🔴 **This is the answer to android#15, and the codebase had already given it.** `Attachment.FILE` has carried this since before the lattice existed: *the label is a MARKING, not a permission — the content key is in the same envelope, so whoever can read the message can read the file.* That settles the ★-property question. The key travels with the file by design, because that is what makes sending work at all; there is no trusted computing base to enforce against, since the person holds every key on a device they control. A client-side refusal would be a suggestion, and **a suggestion dressed as a control is worse than none** — it is the one people plan around. So neither client implements the ★-property, `LabelTest` keeps asserting that absence, and PARITY.md now says plainly that Files is not mandatory access control. What it IS: cryptographic read separation, plus a classification that travels. The picker says so before the tap, in those words. Three things found on the way: 🔴 **A second copy of the vocabulary.** `classificationName` in ChatScreens listed the same four names as `FileLevels`, agreeing by coincidence with nothing making them agree. Rename a level for the Files screen and the chat bubble keeps showing the old word for the same number, in the same conversation, on the same phone. Collapsed to one. 🔴 **Attach and Camera did the same thing.** Both launched the gallery, and there was no way to send a document at all. Attach now opens a small panel — photo, or from Files — which keeps the kit's composer row at its four controls. 🔴 **iOS's picker lists the root only**, so a file inside a folder cannot be sent from a conversation, and nothing says so — it is simply absent, which reads as "it isn't there". `LocalVault.allFiles()` walks the whole tree. Filed as ios#68. ⚠️ The picker is the ONE place this vault hides a row it cannot read. Everywhere else it insists on showing them, because hiding one leaves somebody unable to see anything is there; a picker exists only for choosing, and a row that can never be chosen is furniture. ⚠️ A received file is always saved as an encrypted copy, never a link — there is no file on this phone to point at — and always at the label it arrived with. Re-labelling somebody else's document on receipt would launder its classification, and the number is the only thing that survived the trip. 9 new tests. Each guard proved by breaking the code: writing the label for every attachment kind fails the image test; dropping compartments fails the two that name them; removing the fallback caption fails the older-build test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |