From 6ec626cde289283f902a1e6ac00e9896486739f6 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Fri, 4 Sep 2026 15:34:29 -0400 Subject: [PATCH 1/3] =?UTF-8?q?Share=20a=20file=20into=20a=20chat,=20and?= =?UTF-8?q?=20save=20one=20back=20=E2=80=94=20with=20the=20label=20it=20ca?= =?UTF-8?q?rries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- PARITY.md | 20 +++- .../io/flamenet/messenger/app/ChatScreens.kt | 83 +++++++++++-- .../io/flamenet/messenger/app/FilesScreen.kt | 112 ++++++++++++++++++ .../messenger/app/MessengerViewModel.kt | 106 +++++++++++++++++ .../io/flamenet/messenger/app/Session.kt | 55 +++++++++ .../messenger/app/FileShareWireTest.kt | 109 +++++++++++++++++ .../io/flamenet/client/Conversations.kt | 23 +++- .../kotlin/io/flamenet/client/LocalVault.kt | 22 ++++ .../io/flamenet/client/LocalVaultTest.kt | 46 +++++++ 9 files changed, 562 insertions(+), 14 deletions(-) create mode 100644 app/src/test/kotlin/io/flamenet/messenger/app/FileShareWireTest.kt diff --git a/PARITY.md b/PARITY.md index 8737a4c..b6edd1a 100644 --- a/PARITY.md +++ b/PARITY.md @@ -340,8 +340,8 @@ the messenger rather than staying separate apps. | Files engine: KDF and identity record | βœ… an identity written by the browser unlocks here | | Local vault: add, organise, delete | βœ… index, folders, both storage modes, bulk delete with its report | | Files screen: set up, unlock, browse, add, folders, delete, promote | βœ… ⚠️ **not yet run on a device** β€” see below | -| Bell–LaPadula labels and clearance | 🟑 lattice, encoding and clearance in Kotlin, pinned to independently-computed vectors; the β˜…-property is unenforced on **both** clients β€” android#15 | -| Share a file into a chat or group | βšͺ | +| The security lattice (labels, clearance) | βœ… encoding and clearance in Kotlin, pinned to independently-computed vectors. ⚠️ **Not Bell–LaPadula** β€” the β˜…-property is deliberately unenforced on both clients; see android#15 | +| Share a file into a chat or group | βœ… both directions β€” send from the vault, save a received one back at the label it arrived with | | Wallet engine: BIP-39, Base58, SLIP-0010 | βœ… pinned to the published vectors | | Wallet engine: address derivation and signing | βœ… BIP-44 path, Base58 address, and the engine's own RFC 8032 signs β€” no API-33 floor | | Wallet: BIP-39 seed, create and restore | βœ… create, import, restore, sealed at rest, with the phrase shown once and explained | @@ -365,6 +365,22 @@ inner one for convenience would be a real reduction and not a refactor β€” a vau that opens because the phone is open is a folder with a lock drawn on it. +πŸ”΄ **Files is NOT a mandatory access control system, and nothing should say it +is.** The sharing path forced the question android#15 asked, and the answer was +already written on the wire format: *the label is a MARKING, not a permission.* +The content key travels in the same envelope as the file β€” by design, because +that is what makes sending work β€” so no client-side refusal is a control. There +is no trusted computing base: the person holds every key, on a device they +control, running a client they could modify. **A suggestion dressed as a control +is worse than no control**, because it is the one people plan around. + +What Files does provide, and what may be claimed: cryptographic read separation +(no label key, no plaintext β€” enforced by mathematics, not by a check), and a +classification that travels with a file and is preserved on the recipient's +phone. Bell–LaPadula's β˜…-property is deliberately absent on both clients, and +the test in `LabelTest` asserts that absence so it fails loudly if one platform +implements it alone. + πŸ”΄ **The Files crypto is held to `vectors/files-v1.json`, which the JavaScript engine in the browser produced β€” a third implementation, not the Swift one.** Checking Kotlin against Swift would only establish that the two agree, and two diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt b/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt index 49275b0..81bcd05 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/ChatScreens.kt @@ -1974,6 +1974,9 @@ fun ColumnScope.ChatScreen( // this screen that did not belong to that row. var showStickers by remember { mutableStateOf(false) } var showEmoji by remember { mutableStateOf(false) } + var showAttach by remember { mutableStateOf(false) } + /** The Files picker, opened from the attach panel. */ + var pickingFile by remember { mutableStateOf(false) } var micGranted by remember { mutableStateOf( @@ -2074,8 +2077,19 @@ fun ColumnScope.ChatScreen( verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(Space.sm), ) { - IconAction(icon = KitIcon.Attach, label = "Attach a photo", tag = "attach", - onClick = { pickImage.launch("image/*") }) + // πŸ”΄ **A choice, not a shortcut to the photo picker.** This was labelled + // "Attach a photo" and launched the gallery β€” which is also exactly what + // the camera button beside it does, so the row had two controls doing + // one thing and no way at all to send a document. Opening a small panel + // keeps the kit's row at its four controls while giving the second kind + // of attachment somewhere to live. + IconAction(icon = KitIcon.Attach, + label = if (showAttach) "Close attachments" else "Attach", + tag = "attach", + onClick = { + showAttach = !showAttach + if (showAttach) { showEmoji = false; showStickers = false } + }) // ⚠️ `BasicTextField` inside our own pill rather than a Material text // field with its shape overridden: `OutlinedTextField` reserves room @@ -2186,6 +2200,25 @@ fun ColumnScope.ChatScreen( } } + if (showAttach) { + AttachPanel( + model = model, + onPhoto = { showAttach = false; pickImage.launch("image/*") }, + onFiles = { showAttach = false; pickingFile = true }, + ) + } + + if (pickingFile) { + PickFileToSendSheet( + model = model, + onCancel = { pickingFile = false }, + onPick = { entry -> + pickingFile = false + model.sendFileFromVault(entry, chat) + }, + ) + } + if (showStickers) { StickerPicker { sticker -> // A sticker IS the message. There is nothing to add it to and @@ -2408,7 +2441,10 @@ private fun Bubble( io.flamenet.e2e.Attachment.AUDIO -> VoiceBubble(ref, model) // A document, not a picture: AttachedImage would download // a PDF and draw nothing. - io.flamenet.e2e.Attachment.FILE -> FileBubble(ref, line.mine) + io.flamenet.e2e.Attachment.FILE -> FileBubble( + ref, line.mine, + onSave = if (line.mine) null else { { model.saveAttachmentToFiles(ref) } }, + ) else -> AttachedImage(ref, model) } } @@ -3783,7 +3819,11 @@ fun ColumnScope.MailboxesSection( * other half is built. */ @Composable -private fun FileBubble(ref: io.flamenet.e2e.Attachment.Ref, mine: Boolean) { +private fun FileBubble( + ref: io.flamenet.e2e.Attachment.Ref, + mine: Boolean, + onSave: (() -> Unit)? = null, +) { val onColour = if (mine) MaterialTheme.colorScheme.onPrimary else MaterialTheme.colorScheme.onSurface Row( @@ -3835,6 +3875,22 @@ private fun FileBubble(ref: io.flamenet.e2e.Attachment.Ref, mine: Boolean) { ) } } + // πŸ”΄ **Offered on a received file only.** Saving something already + // in the vault back into it would make a second, unrelated copy at + // whatever label this bubble happens to carry β€” which is how one + // document quietly becomes two at different classifications. + if (onSave != null) { + Text( + "Save to Files", + fontSize = 12.sp, + fontWeight = FontWeight.Medium, + color = onColour, + modifier = Modifier + .clickable(onClick = onSave) + .padding(top = 2.dp) + .testTag("chat.file.save"), + ) + } } } } @@ -3844,14 +3900,19 @@ private fun FileBubble(ref: io.flamenet.e2e.Attachment.Ref, mine: Boolean) { * ask on either side any more. An ordinal outside the set is shown as itself * rather than guessed at β€” inventing "Secret" for a level this deployment calls * something else would be worse than a number. + * + * πŸ”΄ **One vocabulary, not two.** This was a second `when` listing the same + * four names β€” agreeing with [FileLevels] by coincidence, with nothing making + * them agree. Two places to change is how one of them gets changed: rename a + * level for the Files screen and the chat bubble keeps showing the old word for + * the same number, on the same phone, in the same conversation. + * + * ⚠️ It matters more than a rename. The ORDINAL is what travels on the wire, + * so the name is the only thing telling somebody what a number means β€” and two + * answers to that is worse than none. */ -private fun classificationName(ordinal: Int): String = when (ordinal) { - 0 -> "Unclassified" - 1 -> "Internal" - 2 -> "Confidential" - 3 -> "Restricted" - else -> "Level $ordinal" -} +private fun classificationName(ordinal: Int): String = + FileLevels.levelName(ordinal.toShort()) /** * What is being typed after an unfinished `@`, or null when there isn't one. diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/FilesScreen.kt b/app/src/main/kotlin/io/flamenet/messenger/app/FilesScreen.kt index 1625f37..321833f 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/FilesScreen.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/FilesScreen.kt @@ -539,3 +539,115 @@ private fun ColumnScope.LevelPicker(selected: Short, onPick: (Short) -> Unit) { } } } + +// ---- Into and out of a conversation -------------------------------------- + +/** + * What can be attached to a message. + * + * ⚠️ Two kinds, and they are not variations of one thing: a photo comes from + * the gallery and is re-encoded for sending; a document comes from the vault + * and carries a classification with it. + */ +@Composable +fun ColumnScope.AttachPanel( + model: MessengerViewModel, + onPhoto: () -> Unit, + onFiles: () -> Unit, +) { + Column( + modifier = Modifier + .fillMaxWidth() + .clip(RoundedCornerShape(Radius.small)) + .background(Palette.SurfaceAlt) + .padding(Space.md) + .testTag("attach-panel"), + verticalArrangement = Arrangement.spacedBy(Space.sm), + ) { + KitSecondaryButton(text = "Photo", onClick = onPhoto, + modifier = Modifier.fillMaxWidth(), tag = "attach-photo") + KitSecondaryButton( + text = if (model.filesUnlocked()) "From Files" else "From Files (locked)", + onClick = onFiles, + modifier = Modifier.fillMaxWidth(), tag = "attach-files", + ) + } +} + +/** + * Choose a document from the vault, from inside a conversation. + * + * πŸ”΄ **Sending is a downgrade, and the sheet says so before the tap.** The + * content key travels in the same envelope as the message, so a classification + * does not stop the recipient opening it β€” it goes so their phone files it the + * same way. Every word here has to carry that, because the alternative is a + * person believing "Restricted" did something on the way. + * + * ⚠️ **Never asks for the Files passphrase.** A conversation that prompted for + * one would be a conversation that has started collecting one, and Files goes + * out of its way not to hold it. Locked says so and points at the Files screen. + */ +@Composable +fun ColumnScope.PickFileToSendSheet( + model: MessengerViewModel, + onCancel: () -> Unit, + onPick: (LocalVault.Entry) -> Unit, +) { + val unlocked = model.filesUnlocked() + // Read once when the sheet opens: the vault does not change underneath it, + // and re-reading on every recomposition means decrypting every filename + // again for each frame. + val entries = remember(unlocked) { if (unlocked) model.filesForSending() else emptyList() } + + Column( + modifier = Modifier + .fillMaxWidth() + .clip(RoundedCornerShape(Radius.small)) + .background(Palette.SurfaceAlt) + .padding(Space.md) + .testTag("pick-file-sheet"), + ) { + Text("Send a file", fontWeight = FontWeight.Medium, fontSize = 16.sp) + Spacer(Modifier.height(Space.sm)) + + if (!unlocked) { + Text( + "Files is locked. Open Files from your account and unlock it, " + + "then come back β€” your passphrase isn't asked for here.", + fontSize = 13.sp, color = Palette.SecondaryText, + ) + } else if (entries.isEmpty()) { + Text("Nothing in your files yet.", fontSize = 13.sp, color = Palette.SecondaryText) + } else { + Text( + "Whoever you send this to will be able to open it. The " + + "classification travels with the file so it's filed the " + + "same way on their phone β€” it doesn't stop them reading it.", + fontSize = 12.sp, color = Palette.SecondaryText, + ) + Spacer(Modifier.height(Space.sm)) + for (entry in entries) { + Row( + modifier = Modifier + .fillMaxWidth() + .clickable { onPick(entry) } + .padding(vertical = Space.sm) + .testTag("pick-file-row"), + verticalAlignment = Alignment.CenterVertically, + ) { + Column(Modifier.weight(1f)) { + Text(entry.name ?: "Unnamed", fontSize = 15.sp, + maxLines = 1, overflow = TextOverflow.Ellipsis) + Text( + FileLevels.levelName(entry.label.classification), + fontSize = 11.sp, color = Palette.SecondaryText, + ) + } + } + } + } + Spacer(Modifier.height(Space.sm)) + KitSecondaryButton(text = "Cancel", onClick = onCancel, + modifier = Modifier.fillMaxWidth(), tag = "pick-file-cancel") + } +} diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt index 2ccd0ed..396ffc1 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -1649,6 +1649,112 @@ class MessengerViewModel(app: Application) : AndroidViewModel(app) { ) } + // -- Files, into and out of a conversation -------------------------------- + + /** + * Whether a conversation can offer to send from Files right now. + * + * ⚠️ **A chat never asks for the Files passphrase.** A conversation that + * prompted for one would be a conversation that has started collecting one, + * and Files goes out of its way not to hold it. Locked means the picker says + * so and points at the Files screen. + */ + fun filesUnlocked(): Boolean = filesVault != null + + /** Everything sendable, anywhere in the tree. */ + fun filesForSending(): List = + filesVault?.let { runCatching { it.allFiles() }.getOrDefault(emptyList()) } ?: emptyList() + + /** + * Send a document from the vault into this conversation. + * + * πŸ”΄ **This is a downgrade, and the screen says so before the tap rather + * than after.** The content key goes in the same envelope, so the + * classification does not stop the recipient reading it β€” it travels so + * their phone files it the same way. Nothing here refuses on the label, + * because nothing here could: refusing in a client whose user holds the + * keys is a suggestion, and one dressed as a control is worse than none. + */ + fun sendFileFromVault(entry: LocalVault.Entry, chat: Phase.Chat) = + viewModelScope.launch { + val open = filesVault ?: return@launch + val live = session ?: return@launch + filesUpdate { it.copy(busy = "Sending…", problem = null, notice = null) } + val done = withContext(Dispatchers.IO) { + runCatching { + val bytes = open.contents(entry.node) + val name = entry.name ?: "file" + val label = entry.label + val group = chat.group + if (group != null) { + live.sendGroupFile(group, bytes, name, + label.classification.toInt(), label.compartments.sorted()) + } else { + live.sendFile(chat.peerId ?: return@runCatching, + bytes, name, + label.classification.toInt(), label.compartments.sorted()) + } + } + } + done.fold( + onSuccess = { + filesUpdate { it.copy(busy = null) } + update { + it.copy(openChat = it.openChat?.copy(lines = live.history(chat.threadId))) + } + }, + onFailure = { e -> + filesUpdate { + it.copy(busy = null, + problem = "Couldn't send that file. ${e.message ?: ""}".trim()) + } + }, + ) + } + + /** + * Save a received document into the vault. + * + * πŸ”΄ **Always an encrypted copy, never a link.** There is no file on this + * phone to point at β€” the bytes arrived over the wire β€” so the two storage + * modes are not a choice here. + * + * ⚠️ **Filed under the label it arrived with, not under one this device + * chose.** Re-labelling somebody else's document on receipt would quietly + * launder its classification, and the number is the only thing that survived + * the trip. + */ + fun saveAttachmentToFiles(ref: io.flamenet.e2e.Attachment.Ref) = viewModelScope.launch { + val open = filesVault + if (open == null) { + filesUpdate { + it.copy(problem = "Files is locked. Open Files and unlock it, then " + + "come back β€” your passphrase isn't asked for here.") + } + return@launch + } + val live = session ?: return@launch + filesUpdate { it.copy(busy = "Saving…", problem = null, notice = null) } + val done = withContext(Dispatchers.IO) { + runCatching { + val bytes = live.attachmentBytes(ref, getApplication().cacheDir) + val name = ref.filename.ifEmpty { "Received file" } + open.add(bytes, name, + Label(ref.classification.toShort(), ref.compartments.toSet())) + name + } + } + done.fold( + onSuccess = { name -> filesUpdate { it.copy(busy = null, notice = "Saved $name to Files.") } }, + onFailure = { e -> + filesUpdate { + it.copy(busy = null, + problem = "Couldn't save that file. ${e.message ?: ""}".trim()) + } + }, + ) + } + fun filesClearMessages() = filesUpdate { it.copy(problem = null, notice = null) } private fun finishFilesWork(result: Result<*>, notice: String?) { diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt index 04b964f..e16351e 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/Session.kt @@ -360,6 +360,61 @@ class Session( return sent } + /** + * Send a document from the Files vault. + * + * πŸ”΄ **The label travels; it does not gate.** The content key is in the same + * envelope as the message, so the recipient can open this whatever their + * clearance β€” the classification goes so their phone files it the same way, + * and every sentence on the way to this call has to say that rather than + * implying a control. See `Attachment.FILE`. + * + * ⚠️ `application/octet-stream` and not a guess from the extension. A + * document announced as `image/png` renders as a broken image rather than a + * file somebody can save, and the vault stores no MIME type to be right + * with. + */ + fun sendFile( + peerUserId: Long, + bytes: ByteArray, + name: String, + classification: Int, + compartments: List, + ): io.flamenet.client.Conversations.Sent { + val sent = conversations.sendAttachment( + peerUserId, bytes, "application/octet-stream", Attachment.FILE, + filename = name, classification = classification, compartments = compartments) + record(Vault.threadForUser(peerUserId), "", mine = true, + attachment = sent.attachment, + delivery = if (sent.reachedSomeone) Delivery.DELIVERED else Delivery.PENDING) + return sent + } + + /** + * Send a document to a group. + * + * ⚠️ **Sealed and uploaded once, then fanned out** β€” one blob, one key, one + * upload, referenced by every member's envelope. And the count is worth + * saying out loud before tapping: a Restricted file into a group of eleven + * hands the key to eleven people. + */ + fun sendGroupFile( + group: RelayClient.Group, + bytes: ByteArray, + name: String, + classification: Int, + compartments: List, + ): Conversations.Sent { + val sent = conversations.sendGroupAttachment( + group, userId, bytes, "application/octet-stream", width = 0, height = 0, + kind = Attachment.FILE, filename = name, + classification = classification, compartments = compartments) + record(Vault.threadForGroup(group.id), "", mine = true, + attachment = sent.attachment, author = userId, + delivery = if (sent.reachedSomeone) Delivery.DELIVERED else Delivery.PENDING) + return sent + } + /** Bytes for an attachment, cached on disk after the first fetch. */ fun attachmentBytes(ref: Attachment.Ref, cacheDir: java.io.File): ByteArray { val cached = java.io.File(cacheDir, "att-" + ref.id.replace('/', '_')) diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/FileShareWireTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/FileShareWireTest.kt new file mode 100644 index 0000000..2654bfd --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/FileShareWireTest.kt @@ -0,0 +1,109 @@ +package io.flamenet.messenger.app + +import io.flamenet.e2e.Attachment +import io.flamenet.e2e.Content +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * What a document sent from Files actually puts on the wire. + * + * πŸ”΄ **The classification is the only thing that survives the trip.** The + * recipient's phone files the document at whatever number arrives β€” there is + * nothing else to consult and no server to ask β€” so a label dropped in transit + * does not error anywhere: the file simply arrives Unclassified and gets filed + * that way, on the most permissive setting it could possibly have. + * + * ⚠️ These check the ENVELOPE. `Session.sendFile` needs a relay and a vault, so + * the send itself is covered on device; the shape that crosses to the other app + * is what an interop bug would live in, and it is checkable here. + */ +class FileShareWireTest { + + private fun ref(classification: Int, compartments: List, name: String = "Q3.txt") = + Attachment.Ref( + kind = Attachment.FILE, id = "blob-1", + key = ByteArray(32) { 1 }, digest = ByteArray(32) { 2 }, + bytes = 4096, mime = "application/octet-stream", + filename = name, classification = classification, compartments = compartments, + ) + + private fun roundTrip(r: Attachment.Ref): Attachment.Ref { + val parsed = assertNotNull(Content.parse(Content.attachment(r))) + return assertNotNull(parsed.attachment, "the attachment did not survive the envelope") + } + + @Test + fun `a file keeps its classification and its compartments`() { + val back = roundTrip(ref(3, listOf(1, 2))) + assertEquals(3, back.classification, "the classification was lost in the envelope") + assertEquals(listOf(1, 2), back.compartments) + assertEquals("Q3.txt", back.filename) + } + + /** + * πŸ”΄ **Unclassified must round-trip as a real value, not as an absence.** + * Zero is the default for a missing field, so a sender that dropped the key + * and a sender that meant zero are indistinguishable at the other end. This + * pins that the zero case still parses as a file rather than as something + * the recipient has to guess about. + */ + @Test + fun `an unclassified file is still a file, not a missing label`() { + val back = roundTrip(ref(0, emptyList())) + assertEquals(0, back.classification) + assertTrue(back.compartments.isEmpty()) + assertEquals(Attachment.FILE, back.kind) + } + + /** + * ⚠️ A file envelope always carries a caption, even when nobody wrote one. + * Builds predating this attachment kind show the caption and drop the rest, + * so an empty one reaches them as "[unsupported attachment]" with no clue + * what was sent. + */ + @Test + fun `a file with no caption still says what it is to an older build`() { + val parsed = assertNotNull(Content.parse(Content.attachment(ref(1, emptyList())))) + assertTrue( + parsed.text.contains("Q3.txt"), + "an older build would render this as an unsupported attachment: '${parsed.text}'", + ) + } + + @Test + fun `a caption somebody wrote is not replaced by the filename`() { + val parsed = assertNotNull( + Content.parse(Content.attachment(ref(1, emptyList()), caption = "the numbers"))) + assertEquals("the numbers", parsed.text) + } + + /** + * ⚠️ Compartment order is normalised on the way out, so two senders who + * chose the same areas produce the same bytes. Unsorted, the label would + * still be *correct* and would still encode differently β€” which matters + * because the canonical encoding is authenticated data elsewhere. + */ + @Test + fun `compartments arrive in a stable order`() { + assertEquals(listOf(1, 2, 5), roundTrip(ref(2, listOf(5, 1, 2))).compartments.sorted()) + } + + /** + * πŸ”΄ An image must NOT gain a label. `label` is written only for the file + * kind β€” a photo carrying `cls: 0` would tell the other end there is a + * classification here to respect, and there is not. + */ + @Test + fun `an image carries no label at all`() { + val image = Attachment.Ref( + kind = Attachment.IMAGE, id = "b", key = ByteArray(32), digest = ByteArray(32), + bytes = 10, mime = "image/jpeg", width = 4, height = 4, + ) + val json = org.json.JSONObject(Content.attachment(image)) + assertTrue(!json.has("cls"), "an image envelope is claiming a classification") + assertTrue(!json.has("cmp"), "an image envelope is claiming compartments") + } +} diff --git a/client/src/main/kotlin/io/flamenet/client/Conversations.kt b/client/src/main/kotlin/io/flamenet/client/Conversations.kt index 87bf21b..feaf925 100644 --- a/client/src/main/kotlin/io/flamenet/client/Conversations.kt +++ b/client/src/main/kotlin/io/flamenet/client/Conversations.kt @@ -270,6 +270,18 @@ class Conversations( height: Int = 0, duration: Double = 0.0, groupId: String? = null, + /** File only: the document's own name. */ + filename: String = "", + /** + * File only: the label it was filed under. + * + * πŸ”΄ **A MARKING, not a permission.** The content key rides in the same + * envelope, so whoever can read the message can read the file. This + * travels so the recipient can file it the same way β€” it must never be + * described, here or on screen, as something that stops them opening it. + */ + classification: Int = 0, + compartments: List = emptyList(), ): Sent { val sealed = Attachment.seal(data) val id = relay.uploadAttachment(sealed.blob) @@ -277,6 +289,8 @@ class Conversations( kind = kind, id = id, key = sealed.key, digest = sealed.digest, bytes = sealed.blob.size, mime = mime, width = width, height = height, duration = duration, + filename = filename, classification = classification, + compartments = compartments.sorted(), ) return sendPlaintext( toUserId, @@ -301,12 +315,19 @@ class Conversations( width: Int, height: Int, caption: String? = null, + kind: String = Attachment.IMAGE, + filename: String = "", + /** See the note on [sendAttachment]: a marking, not a permission. */ + classification: Int = 0, + compartments: List = emptyList(), ): Sent { val sealed = Attachment.seal(data) val id = relay.uploadAttachment(sealed.blob) val ref = Attachment.Ref( - kind = Attachment.IMAGE, id = id, key = sealed.key, digest = sealed.digest, + kind = kind, id = id, key = sealed.key, digest = sealed.digest, bytes = sealed.blob.size, mime = mime, width = width, height = height, + filename = filename, classification = classification, + compartments = compartments.sorted(), ) val body = Content.attachment(ref, caption, group.id, groupName = vault.groupName(group.id)) diff --git a/client/src/main/kotlin/io/flamenet/client/LocalVault.kt b/client/src/main/kotlin/io/flamenet/client/LocalVault.kt index 09fb810..c9094e4 100644 --- a/client/src/main/kotlin/io/flamenet/client/LocalVault.kt +++ b/client/src/main/kotlin/io/flamenet/client/LocalVault.kt @@ -344,6 +344,28 @@ class LocalVault( found } + /** + * Every file in the whole tree, for a picker that has no business showing + * folders. + * + * πŸ”΄ **The whole tree, not the top of it.** iOS's picker lists `list(nil)`, + * so a document filed into a folder cannot be sent from a conversation at + * all β€” and nothing says so; the file is simply absent from the list, which + * reads as "it isn't there". Filed as ios#68. + * + * ⚠️ Unreadable and missing rows are dropped here rather than shown + * disabled. Everywhere else this class insists on showing a row you cannot + * open β€” because hiding it would leave somebody unable to see that anything + * is there. A picker is the exception: its only purpose is choosing + * something to send, and a row that can never be chosen is furniture. + */ + fun allFiles(): List = lock.withLock { + nodes.filter { !it.isFolder } + .map { Entry(it, nameOf(it), isMissing = isMissing(it)) } + .filter { it.name != null && !it.isMissing } + .sortedWith(compareBy(String.CASE_INSENSITIVE_ORDER) { it.name ?: "" }) + } + fun contents(of: Node): ByteArray = lock.withLock { readContents(of) } private fun readContents(node: Node): ByteArray { diff --git a/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt b/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt index c6f151b..7f5a9f2 100644 --- a/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt @@ -468,6 +468,52 @@ class LocalVaultTest { assertEquals(listOf("b.txt"), reopen().list(null).map { it.name }) } + // ---- The picker ------------------------------------------------------ + + /** + * πŸ”΄ **The whole tree, not the top of it.** iOS's picker lists the root + * only, so a document filed into a folder cannot be sent from a + * conversation at all β€” and nothing says so: the file is simply absent, + * which reads as "it isn't there". Filed as ios#68. + */ + @Test + fun `everything sendable is offered, however deep it is filed`() { + val folder = vault.createFolder("Reports", open) + val inner = vault.createFolder("2026", open, parentId = folder.id) + vault.add("a".toByteArray(), "top.txt", open) + vault.add("b".toByteArray(), "buried.txt", secret, parentId = inner.id) + + assertEquals(listOf("buried.txt", "top.txt"), vault.allFiles().map { it.name }) + } + + /** Folders are places, not files. Sending one means nothing. */ + @Test + fun `folders are not offered`() { + vault.createFolder("Reports", open) + assertTrue(vault.allFiles().isEmpty()) + } + + /** + * ⚠️ **The one place this class hides a row it cannot read.** Everywhere + * else an unreadable row is shown, because hiding it would leave somebody + * unable to see that anything is there. A picker's only purpose is choosing + * something to send, and a row that can never be chosen is furniture. + */ + @Test + fun `rows that could never be sent are left out of the picker`() { + refs.files["doc://gone"] = "x".toByteArray() + vault.reference("doc://gone", "vanished.pdf", open) + vault.add("readable".toByteArray(), "fine.txt", open) + refs.files.remove("doc://gone") + + assertEquals(listOf("fine.txt"), vault.allFiles().map { it.name }, + "a file that has moved was offered as sendable") + + val stranger = LocalVault(root, ByteArray(32) { 99 }, refs).also { it.load() } + assertTrue(stranger.allFiles().isEmpty(), + "a row this device cannot decrypt was offered as sendable") + } + // ---- The tree -------------------------------------------------------- @Test -- 2.54.0 From 436ce240c74fe0ce23f8542a0cfacf26b7bad894 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Fri, 4 Sep 2026 16:08:19 -0400 Subject: [PATCH 2/3] Fix: the vault index failed to load on a real phone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit πŸ”΄ **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 --- PARITY.md | 43 +++++++++++--- .../kotlin/io/flamenet/client/LocalVault.kt | 41 +++++++++++-- .../io/flamenet/client/LocalVaultTest.kt | 57 +++++++++++++++++++ 3 files changed, 128 insertions(+), 13 deletions(-) diff --git a/PARITY.md b/PARITY.md index b6edd1a..17f455f 100644 --- a/PARITY.md +++ b/PARITY.md @@ -339,7 +339,7 @@ the messenger rather than staying separate apps. | Files engine: names, key wrapping, chunked content | βœ… pinned to the **browser's** published vectors, not to the Swift engine | | Files engine: KDF and identity record | βœ… an identity written by the browser unlocks here | | Local vault: add, organise, delete | βœ… index, folders, both storage modes, bulk delete with its report | -| Files screen: set up, unlock, browse, add, folders, delete, promote | βœ… ⚠️ **not yet run on a device** β€” see below | +| Files screen: set up, unlock, browse, add, folders, delete, promote | βœ… **verified on a device** β€” set up, encrypt, lock, cold-start reload, link via SAF | | The security lattice (labels, clearance) | βœ… encoding and clearance in Kotlin, pinned to independently-computed vectors. ⚠️ **Not Bell–LaPadula** β€” the β˜…-property is deliberately unenforced on both clients; see android#15 | | Share a file into a chat or group | βœ… both directions β€” send from the vault, save a received one back at the label it arrived with | | Wallet engine: BIP-39, Base58, SLIP-0010 | βœ… pinned to the published vectors | @@ -348,13 +348,40 @@ the messenger rather than staying separate apps. | Wallet: fresh receive address per receive | βœ… counter in the vault; the screen says why the address changes | | Wallet: balance and transactions from a node | βœ… both, with **unread drawn as an em dash and never as zero** | -⚠️ **The Files screen compiles and every layer under it is tested, but it has -not been exercised on a device or emulator.** The machine was carrying an iOS -simulator at 99% CPU when it landed, and booting an emulator on top of that -produces a flaky run rather than evidence. Two things specifically need a real -device before this can be called done: the Storage Access Framework picker and -its persistable read grant, and whether a reopened app can still read a linked -file after a reboot. Neither can be faked on the JVM. +πŸ”΄ **The device run found a bug no JVM test could have.** 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 phone: + +- `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. +- `bookmark` produced a reference pointing at `"null"`, permanently missing. + +Absence is now decided by `JSONObject.isNull`, never by comparing a decoded +string to a sentinel, and the literal `"null"` is rejected too. Two regression +tests reproduce the device's behaviour on the JVM by writing the literal string +the platform hands over β€” feeding a real JSON `null` would pass with the broken +code and prove nothing. + +⚠️ **Worth a sweep.** `optString` on a nullable field is wrong everywhere in +this app, not only here, and it fails only on the platform it ships to. + +**What the device run covered** (emulator, API 35): set-up with the passphrase, +a wrong passphrase refused, the SAF `OpenDocument` picker with the display name +pre-filled, encrypting into the vault at a chosen classification, lock and +unlock, and a cold-start reload of the vault. On disk: the blob begins `ENCL\x01`, +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. πŸ”΄ **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 c9094e4..41044b8 100644 --- a/client/src/main/kotlin/io/flamenet/client/LocalVault.kt +++ b/client/src/main/kotlin/io/flamenet/client/LocalVault.kt @@ -276,23 +276,54 @@ class LocalVault( .put("storage", node.storage.wire) .put("bookmark", node.bookmark ?: JSONObject.NULL) + /** + * πŸ”΄ **`optString` is not safe on a nullable field, and the JVM will not + * tell you.** Android's bundled `org.json` coerces a JSON `null` to the + * four-character string `"null"`; the Maven `org.json` these tests run + * against returns `""`. Every nullable field below therefore passed on a + * laptop and broke on a phone: + * + * - `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 already 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. + * - `bookmark` produced a reference pointing at `"null"`, permanently + * missing. + * + * ⚠️ So absence is decided by [JSONObject.isNull], 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. + * + * Found by running the app on a device (android!52). No JVM test could have + * caught it, which is the argument for the device run, not against the + * tests. + */ + private fun optional(o: JSONObject, key: String): String? { + if (o.isNull(key)) return null + val value = o.optString(key, "") + return value.takeIf { it.isNotEmpty() && it != "null" } + } + private fun decode(o: JSONObject): Node { val bits = o.optJSONArray("compartments") ?: JSONArray() return Node( id = UUID.fromString(o.getString("id")), - parentId = o.optString("parentID", "").takeIf { it.isNotEmpty() }?.let(UUID::fromString), + parentId = optional(o, "parentID")?.let(UUID::fromString), isFolder = o.optBoolean("isFolder", false), classification = o.getInt("classification").toShort(), compartments = (0 until bits.length()).map { bits.getInt(it) }, wrappedNodeKey = unb64(o.getString("wrappedNodeKey")), nameDigest = unb64(o.getString("nameDigest")), wrappedName = unb64(o.getString("wrappedName")), - wrappedContentKey = o.optString("wrappedContentKey", "") - .takeIf { it.isNotEmpty() }?.let(::unb64), + wrappedContentKey = optional(o, "wrappedContentKey")?.let(::unb64), byteCount = o.optInt("byteCount", 0), addedAtMillis = o.optLong("addedAt", 0L), - storage = Storage.parse(o.optString("storage", "").takeIf { it.isNotEmpty() }), - bookmark = o.optString("bookmark", "").takeIf { it.isNotEmpty() }, + storage = Storage.parse(optional(o, "storage")), + bookmark = optional(o, "bookmark"), ) } diff --git a/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt b/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt index 7f5a9f2..c7d54e3 100644 --- a/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt +++ b/client/src/test/kotlin/io/flamenet/client/LocalVaultTest.kt @@ -160,6 +160,63 @@ class LocalVaultTest { assertEquals(LocalVault.Storage.VAULT, reopen().list(null).single().storage) } + /** + * πŸ”΄ **The bug the JVM could not see.** Android's bundled `org.json` + * coerces a JSON `null` to the four-character string `"null"`; the Maven + * `org.json` this suite runs against returns `""`. Every nullable field in + * the index therefore passed here and broke on a phone β€” a vault with any + * top-level file threw `Invalid UUID string: null` on load, so adding the + * first file worked and reopening the app lost everything. + * + * ⚠️ **This test writes the literal string `"null"` on purpose.** That is + * exactly what the decoder is handed on a device, so putting it in the JSON + * here is what makes the JVM run reproduce the device's behaviour. Feeding + * a real JSON `null` β€” which is what the vault actually writes β€” would pass + * with the broken code and prove nothing. + */ + @Test + fun `a nullable field decodes as absent however the platform hands it over`() { + val node = vault.add("x".toByteArray(), "top.txt", secret) + val reference = File(root, "index.json").readText() + + for (shape in listOf( + reference.replace("\"parentID\":null", "\"parentID\":\"null\""), + reference.replace("\"parentID\":null", ""). // key missing entirely + replace(",,", ","), + reference, + )) { + File(root, "index.json").writeText(shape) + val reopened = reopen() + assertEquals(1, reopened.count, "the index failed to load: $shape") + assertNull(assertNotNull(reopened.node(node.id)).parentId) + assertEquals("top.txt", reopened.list(null).single().name) + } + } + + /** + * πŸ”΄ The one that would NOT have thrown. `"null"` is valid base64, so a + * content key decoded from it is a plausible three bytes β€” and the file + * simply refuses to decrypt, for ever, with nothing to explain why. + */ + @Test + fun `a bookmark or content key of the literal string null is treated as absent`() { + val node = vault.add("x".toByteArray(), "f.txt", secret) + val index = File(root, "index.json") + index.writeText( + index.readText() + .replace("\"bookmark\":null", "\"bookmark\":\"null\"") + .replace(Regex("\"wrappedContentKey\":\"[^\"]*\""), + "\"wrappedContentKey\":\"null\"") + ) + val reopened = reopen() + val back = assertNotNull(reopened.node(node.id)) + assertNull(back.bookmark) + assertNull(back.wrappedContentKey, + "a three-byte content key decoded from the string \"null\" was accepted") + // And the failure is one somebody can act on, not a decryption error. + assertFailsWith { reopened.contents(back) } + } + @Test fun `an empty directory loads as an empty vault rather than failing`() { val fresh = File(root, "elsewhere").also { it.mkdirs() } -- 2.54.0 From 2d05bbe7c43552756726f73123121711e0a9f8ab Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Fri, 4 Sep 2026 16:10:47 -0400 Subject: [PATCH 3/3] Fix: a contact with no address was pinned as the string "null" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../src/main/kotlin/io/flamenet/e2e/Vault.kt | 31 +++++++++- .../test/kotlin/io/flamenet/e2e/VaultTest.kt | 62 +++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt index aa8f5ae..393688d 100644 --- a/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt +++ b/engine/src/main/kotlin/io/flamenet/e2e/Vault.kt @@ -461,15 +461,42 @@ class Vault(private val storage: Storage, private val key: SecretKey) { Trust( identities = (0 until a.length()).map { a.getString(it) }, verified = o.optBoolean("verified", false), - fnid = o.optString("fnid").ifEmpty { null }, + // πŸ”΄ **`"null"` is rejected, and that is not paranoia β€” records + // written that way are already on phones.** `saveTrust` used to + // write an explicit JSON null here, and Android's `org.json` + // coerces that to the four-character string on the way back out. + // See the note on [saveTrust]; this is the half that repairs what + // has already been stored. + fnid = o.optString("fnid").takeIf { it.isNotEmpty() && it != "null" }, changed = o.optBoolean("changed", false), ) } + /** + * πŸ”΄ **The key is OMITTED when there is no address, never written as an + * explicit JSON null.** Android's bundled `org.json` reads a JSON null back + * as the four-character string `"null"`, so a contact pinned before their + * fnid was known reloaded with `fnid = "null"` β€” a non-null value that is + * not an address. What that caused: + * + * - `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, fired on an ordinary first sighting. + * - `rememberIdentity` returns early once `fnid != null`, so the real + * address was never resolved and the record stayed wrong for good. + * - `pinnedFnid` fed `"null"` into key-log verification, which recomputes a + * leaf against the address a contact was accepted under. Every check + * would have been against the wrong subject. + * + * An absent key reads as `""` on both platforms, which is what the reader + * has always expected. + */ fun saveTrust(userId: Long, trust: Trust) = writeJson(trustName(userId), JSONObject() .put("identities", org.json.JSONArray(trust.identities)) .put("verified", trust.verified) - .put("fnid", trust.fnid ?: JSONObject.NULL) + .also { if (trust.fnid != null) it.put("fnid", trust.fnid) } .put("changed", trust.changed)) fun forgetTrust(userId: Long) = storage.delete(trustName(userId)) diff --git a/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt b/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt index 003acc3..da915ad 100644 --- a/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt +++ b/engine/src/test/kotlin/io/flamenet/e2e/VaultTest.kt @@ -315,4 +315,66 @@ class VaultTest { Vault(storage, secret).saveIdentity(identity) assertNotEquals(Bytes.b64(first), Bytes.b64(storage.records.getValue("identity"))) } + + // ---- Trust records and the platform's JSON --------------------------- + + /** + * πŸ”΄ **A contact with no known address must come back with NO address.** + * + * `saveTrust` used to write `"fnid": null` explicitly, and 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. `Trust.pin` compares a + * newly offered fnid against the pinned one and treats a difference as a + * different person, so the first time a real address arrived the contact was + * marked **CHANGED**: the "security keys changed" warning, which is meant to + * mean an attack, fired on an ordinary first sighting. + * + * ⚠️ The JVM's `org.json` returns `""` there, so this passed on a laptop and + * failed on a phone. The two tests below cover both halves β€” what is + * written now, and what has to be tolerated because it is already on + * devices in the field. + */ + @Test + fun `a trust record with no address writes no address at all`() { + val storage = MapStorage() + val secret = key() + Vault(storage, secret).saveTrust(7, Vault.Trust(listOf("aaa"), verified = false, fnid = null)) + + assertNull(Vault(storage, secret).loadTrust(7)?.fnid, + "an absent address came back as a value") + } + + /** + * ⚠️ **This is the test that reproduces the device**, and the one above is + * not. On the JVM a real JSON null reads back as `""` whether the writer + * omits the key or not, so the round trip above passes with the bug intact. + * Handing the reader the literal string is what Android actually does, so + * writing it here is what makes a laptop run tell the truth. + */ + + @Test + fun `a record already holding the literal string null reads as no address`() { + // πŸ”΄ Records written the old way exist on phones already. The reader has + // to repair them, or those contacts stay permanently mis-addressed β€” + // `rememberIdentity` returns early once `fnid != null`, so nothing would + // ever ask again. + val storage = MapStorage() + val secret = key() + val vault = Vault(storage, secret) + vault.saveTrust(7, Vault.Trust(listOf("aaa"), verified = false, fnid = "null")) + + assertNull(Vault(storage, secret).loadTrust(7)?.fnid, + "the literal string \"null\" was accepted as somebody's global address") + } + + @Test + fun `a real address still round-trips`() { + val storage = MapStorage() + val secret = key() + Vault(storage, secret).saveTrust( + 7, Vault.Trust(listOf("aaa"), verified = true, fnid = "fnid:abc123")) + val back = Vault(storage, secret).loadTrust(7) + assertEquals("fnid:abc123", back?.fnid) + assertTrue(back!!.verified) + } } -- 2.54.0