extant2000/flamenet-messenger-android
1
0
Fork 0
Code Issues 7 Pull Requests Actions Packages Projects Releases Wiki Activity

Apply inbound edits and pins instead of filing them as messages (§6.15/#6.16/#6.19)

Browse Source 
🔴 This was an interop bug, not a missing feature. The engine has parsed
`editOf`, `pins` and `fwd` for as long as they have existed, and it exposes
`isRenderable` — "does this belong in a thread" — as the single answer.
`Session`'s inbound loop never consulted it.

So three things arrived from the iPhone app and were filed as ordinary bubbles.
An edit carries its new wording in `text`, so it drew as a DUPLICATE beside the
typo it was meant to replace — a correction that made the thread worse than the
mistake did. A pin carries only an id, so it drew as a blank bubble. And a
forward was drawn as the sender's own words.

Worst of the three is the one nobody had hit yet: a control envelope from a
NEWER peer carries `ctl` and a kind this build has never heard of, and would
have drawn a bubble full of JSON in somebody's conversation. That is precisely
the failure `isControl` was written to prevent, defeated by nobody calling it —
and it would have shipped to every older Android install the first time a
control kind was added on iOS. The guard is `isRenderable` rather than a list of
kinds this build knows, because an allowlist is only correct until the other
platform adds one.

What is applied now:

- an edit rewrites the message it names and stamps `edited`, so a bubble can
  say so — silently replacing the text is worse than the typo, because the
  reader cannot tell a correction from their own misremembering;
- a pin is stored per thread, sealed in the vault like the transcript, as an ID
  and never a copy: a copy goes stale against an edit and outlives a deletion;
- `fwd` reaches the transcript so second-hand words are not drawn as the
  sender's own — THAT it was forwarded, never from whom.

🔴 An inbound edit is applied only when it comes from the person whose message
it is. Without that check any contact sharing a session could rewrite anybody's
words in the reader's transcript — a better attack than sending a message of
their own, because the words would carry somebody else's name.

⚠️ Clearing a thread clears its pin. A banner pointing at a message that was
just cleared reads as the clear not having worked.

⚠️ `fwd` is read as both a boolean and a number, for the same reason `isControl`
accepts both: `org.json`'s `optBoolean` does not coerce a number, and that is
how this class of bug shipped once already.

9 unit tests. App builds, every JVM suite green. The three PARITY rows stay 🟡:
the inbound half is done, originating each still needs the UI.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-09-04 13:53:13 -04:00
co-authored by Claude Opus 5
parent ab8ff658b2
commit e0420a00c6
4 changed files with 344 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
PARITY.md
+29 -3
View File
@@ -94,15 +94,15 @@ From `E2E/E2EInbox.swift`, which is the real feature list.
| History at rest | ✅ encrypted, keyed by person not device |
| Session restore across restarts | ✅ credential and device both survive |
| Reactions | ✅ transcript carries a `mid`; long-press gives a five-emoji row, one emoji per person |
| Edit a sent message (§6.15) | 🟡 codec back, and the `mid` it was blocked on now exists — applying an inbound edit and offering the control are UI work |
| Edit a sent message (§6.15) | 🟡 inbound edits now APPLY to the message they name; offering the control is UI work |
| Drafts that survive a relaunch | ✅ sealed in the vault, not SharedPreferences |
| Forwarding (§6.16) | 🟡 codec back; originating needs the UI |
| Forwarding (§6.16) | 🟡 `fwd` now reaches the transcript so second-hand words are not drawn as the sender's own; originating needs the UI |
| Group message ids and quotes (§6.9) | ✅ transcript stores both; sent and queued copies share one id |
| Mentions (§6.18) | ✅ codec, an `@` picker in the group composer, and ids recorded at pick time rather than re-derived from the text |
| Per-message timers (§6.17) | ✅ combining rule and per-message pruning; composer control in progress |
| Search inside messages | ✅ scanned, never indexed |
| Search inside messages | ✅ scanned, never indexed — no plaintext index at rest |
| Pinned message (§6.19) | 🟡 codec back; the banner and the control are UI work |
| Pinned message (§6.19) | 🟡 inbound pins now stored per thread, sealed in the vault; the banner and the control are UI work |
| Photos | ✅ send via the system picker, render inline |
| Voice notes (record + play) | ✅ AAC/MP4, tap to record, tap to send |
| Stories, and seen-state | ✅ sent per-contact, expired bytes deleted |
@@ -222,6 +222,32 @@ somebody is grinding attempts. The steps are clamped before the shift, and
---
## Inbound control envelopes
🔴 **The app never asked the engine whether an envelope belonged in a thread.**
`Content.isRenderable` has existed since §6.15 and `Session`'s inbound loop did
not consult it, so three things arrived from the iPhone app and were filed as
ordinary bubbles:
- an **edit** carries its new wording in `text`, so it drew as a duplicate
beside the typo it was meant to replace — a correction that made the thread
worse;
- a **pin** carries only an id, so it drew as a blank bubble;
- and any **control envelope from a newer peer** would have drawn a bubble full
of JSON — the failure `isControl` was written to prevent, defeated by nobody
calling it.
All three now branch before filing, and the last is guarded by `isRenderable`
itself rather than by a list of kinds this build happens to know: an allowlist is
only correct until the other platform adds one.
⚠️ An inbound edit is applied only when it comes from the person whose message
it is. Without that check any contact sharing a session could rewrite anybody's
words in the reader's transcript — a better attack than sending a message of
their own, because the words would carry somebody else's name.
---
## Test safety
| Feature | iOS | Android | Note |
app/src/main/kotlin/io/flamenet/messenger/app/Session.kt
+147 -5
View File
@@ -116,6 +116,21 @@ class Session(
* to know anything had been dropped.
*/
val about: Content.Reference? = null,
/**
* Second-hand words (§6.16).
*
* ⚠️ THAT it was forwarded, never from whom.
*/
val forwarded: Boolean = false,
/**
* When this message was last edited (§6.15), or null if never.
*
* 🔴 **Kept so the bubble can say "edited".** A correction that silently
* replaces the text is worse than the typo: the reader has no way to
* know the words changed after they read them, and no way to tell a
* correction from their own misremembering.
*/
val editedAt: Long? = null,
/**
* A timer this one message carries, in seconds (§6.17); 0 for none.
*
@@ -163,6 +178,12 @@ class Session(
link = m.optJSONObject("lnk")?.let { Content.LinkPreview.fromJson(it) },
messageId = m.optString("mid").takeIf { it.isNotEmpty() },
about = m.optJSONObject("re")?.let { Content.Reference.fromJson(it) },
// ⚠️ Both shapes, for the same reason `Content.isForwarded`
// accepts both: `org.json`'s `optBoolean` does not coerce a
// number, so reading only that would miss a `"fwd":1` written
// by any other engine.
forwarded = m.optBoolean("fwd", false) || m.optInt("fwd", 0) != 0,
editedAt = m.optLong("edited", 0L).takeIf { it > 0L },
lifetimeSeconds = m.optInt("secs", 0),
)
}
@@ -184,6 +205,15 @@ class Session(
about: Content.Reference? = null,
/** A timer this one message carries, in seconds (§6.17); 0 for none. */
lifetimeSeconds: Int = 0,
/**
* Whether the sender is passing on somebody else's words (§6.16).
*
* ⚠️ Says THAT it was forwarded and never from whom — the point of a
* forward is that the original author does not travel with it. This
* exists so second-hand words are not shown as the sender's own, which
* is a different thing from attribution.
*/
forwarded: Boolean = false,
) {
vault.append(threadId, JSONObject()
.put("author", author)
@@ -201,10 +231,10 @@ class Session(
}
.also { if (attachment != null) it.put("kind", attachment.kind).put("att", attachment.toJson()) }
.also { if (link != null) it.put("lnk", link.toJson()) }
// ⚠️ Written only when present. A null on every message would grow
// the transcript for nothing and make "no id" indistinguishable
// from "an id of nothing".
.also { if (messageId != null) it.put("mid", messageId) }
// ⚠️ Written only when true, for the same reason `secs` is. A false
// on every message grows the transcript for a flag that is almost
// always absent.
.also { if (forwarded) it.put("fwd", true) }
.also { if (about != null) it.put("re", about.toJson()) })
// 🔴 A reply arriving cancels the reminder, and it is done HERE because
@@ -627,6 +657,70 @@ class Session(
return hit
}
/**
* Apply an inbound edit to the message it names (§6.15).
*
* 🔴 **An edit is never a bubble of its own.** It changes one that is
* already there, or it does nothing. Android had the codec and no
* application, so an edit from the iPhone app arrived carrying the new
* wording in `text` and was filed as an ORDINARY MESSAGE — a correction
* showing up as a duplicate beside the typo it was meant to replace, which
* is worse than the typo.
*
* 🔴 **Only the author may edit their own message.** Without this check any
* contact sharing a session could rewrite anybody's words in the reader's
* transcript, which is a far better attack than sending a message of their
* own: the words would carry somebody else's name.
*
* Returns whether a message was found and changed.
*/
fun applyEdit(
threadId: String,
messageId: String,
author: Long,
newText: String,
at: Long = System.currentTimeMillis(),
): Boolean {
val stored = vault.history(threadId)
var hit = false
for (i in 0 until stored.length()) {
val row = stored.getJSONObject(i)
if (row.optString("mid") != messageId) continue
// 🔴 The author check. `mine` is this device's own message; an
// inbound edit must come from the person whose message it is.
val wroteIt = if (row.optBoolean("mine")) author == userId
else row.optLong("author") == author
if (!wroteIt) break
row.put("text", newText).put("edited", at)
hit = true
break
}
if (hit) vault.replaceHistory(threadId, stored)
return hit
}
/**
* The message pinned in a thread, or null (§6.19).
*
* ⚠️ An id, never a copy. A stored copy would go stale the moment the
* message was edited, and would survive the message being deleted — a
* banner quoting words that are no longer in the conversation.
*/
fun pinnedMessage(threadId: String): String? =
vault.pinnedMessage(threadId)
/**
* Pin a message, or take the pin down with an empty [messageId].
*
* ⚠️ **The last pin wins and there is only ever one.** A thread with a list
* of pins is a second inbox; §6.19 is one banner, and two people pinning at
* once resolve by whoever's envelope arrived last — the same rule the
* transcript already uses for everything else.
*/
fun applyPin(threadId: String, messageId: String) {
vault.savePinnedMessage(threadId, messageId.takeIf { it.isNotBlank() })
}
/**
* React to somebody's message, or take it back with an empty [emoji].
*
@@ -924,6 +1018,37 @@ class Session(
if (story.isActive(now)) incomingStories += story
continue
}
// 🔴 **An EDIT (§6.15) changes a message; it is never one.**
// Android had the codec and no application, so an edit from the
// iPhone app fell straight through this loop carrying its new
// wording in `text` and was filed as an ordinary bubble — a
// correction arriving as a duplicate beside the typo it was meant
// to replace, which is worse than the typo.
message.content.editOf?.let { editing ->
val gid = if (message.fromAddress != null) null
else message.content.groupId
val thread = gid?.let(Vault::threadForGroup)
?: message.fromAddress?.let(Vault::threadForAddress)
?: Vault.threadForUser(message.fromUser)
if (applyEdit(thread, editing, message.fromUser,
message.content.text.orEmpty())) {
touched += thread
}
continue
}
// 🔴 **A PIN (§6.19) is an id, not a bubble.** Same story: without
// this it was filed as a message whose text was empty, so a pin
// from the other app drew a blank bubble in the conversation.
message.content.pins?.let { pinned ->
val gid = if (message.fromAddress != null) null
else message.content.groupId
val thread = gid?.let(Vault::threadForGroup)
?: message.fromAddress?.let(Vault::threadForAddress)
?: Vault.threadForUser(message.fromUser)
applyPin(thread, pinned)
touched += thread
continue
}
// A reaction to a MESSAGE. Folded onto the message it is about,
// never filed as one of its own — a thread where every thumbs-up
// is a bubble is a thread nobody can read.
@@ -997,6 +1122,19 @@ class Session(
// worse, match by coincidence. Groups do not federate yet, so a
// remote sender's group claim is ignored and the message files in
// their own thread.
// 🔴 **The guard that stops this list needing to be complete.**
// Every branch above names one kind of non-message. `isRenderable`
// is the engine's own answer to "does this belong in a thread", and
// it is false for reactions, call signals, edits, pins and — the
// case that matters most — ANY control envelope from a newer peer,
// which carries `ctl` and a kind this build has never heard of.
//
// Without it, a control kind added on iOS ships a bubble full of
// JSON into somebody's conversation on every Android install that
// predates it. The engine has had `isRenderable` since §6.15; the
// app simply never consulted it.
if (!message.content.isRenderable) continue
val gid = if (message.fromAddress != null) null else message.content.groupId
if (gid != null) {
val known = groupRoster(gid)
@@ -1046,7 +1184,11 @@ class Session(
// side.
messageId = message.content.messageId,
about = message.content.about,
lifetimeSeconds = message.content.lifetimeSeconds ?: 0)
lifetimeSeconds = message.content.lifetimeSeconds ?: 0,
// §6.16 — so second-hand words are not drawn as the sender's
// own. The engine has read `fwd` off the wire since forwarding
// shipped; the transcript was where it stopped.
forwarded = message.content.isForwarded)
touched += thread
}
app/src/test/kotlin/io/flamenet/messenger/app/InboundControlTest.kt
+138
View File
@@ -0,0 +1,138 @@
package io.flamenet.messenger.app
import io.flamenet.e2e.Content
import kotlin.test.Test
import kotlin.test.assertFalse
import kotlin.test.assertTrue
/**
* Edits, pins and unknown control envelopes must never become bubbles
* (ios#6.15/#6.16/#6.19, Android half).
*
* 🔴 **The bug this closes was an interop bug, not a missing feature.** The
* engine has parsed `editOf`, `pins` and `fwd` for as long as they have
* existed, and it exposes `isRenderable` — "does this belong in a thread" — as
* the single answer. `Session`'s inbound loop never consulted it.
*
* So an edit from the iPhone app arrived carrying its new wording in `text` and
* was filed as an ORDINARY MESSAGE: a correction drawn as a duplicate beside the
* typo it was meant to replace. A pin was filed as a bubble whose text was
* empty. And any control envelope from a newer peer — which carries `ctl` and a
* kind this build has never heard of — would have drawn a bubble full of JSON
* in somebody's conversation.
*
* ⚠️ These test the ENGINE's classification, which is what the app now branches
* on. The app-side application (`applyEdit`, `applyPin`) needs a `Vault` and a
* keystore, so it is covered on device in `FeatureUseTest`; what is checkable
* here is the decision that used to be missing.
*/
class InboundControlTest {
private fun parse(json: String): Content = Content.parse(json)!!
// ---- What must not be drawn -----------------------------------------
/**
* 🔴 The case that shipped a duplicate bubble. An edit carries the new
* wording, so it looks exactly like a message to anything that does not ask.
*/
@Test
fun `an edit is not renderable, even though it carries text`() {
val edit = parse(Content.edit("the corrected wording", of = "mid-1"))
assertTrue(edit.editOf == "mid-1", "the codec no longer reads editOf")
assertTrue(edit.text?.isNotEmpty() == true, "an edit carries its new text")
assertFalse(edit.isRenderable, "an edit would be filed as a message")
}
/** A pin names an id and has nothing to show. */
@Test
fun `a pin is not renderable`() {
val pin = parse(Content.pin("mid-1"))
assertTrue(pin.pins == "mid-1")
assertFalse(pin.isRenderable, "a pin would be filed as an empty bubble")
}
/** Taking a pin down is the same envelope with an empty id. */
@Test
fun `taking a pin down is also not renderable`() {
val unpin = parse(Content.pin(""))
assertTrue(unpin.pins == "", "an empty id is how a pin is removed")
assertFalse(unpin.isRenderable)
}
/**
* 🔴 **The case an allowlist cannot cover, and the reason the app branches
* on `isRenderable` rather than on a list of kinds it knows.** A control
* envelope from a NEWER peer has a kind nothing here has heard of. Keying on
* known kinds ships a bubble full of JSON to every older install the first
* time a new control message is added on the other platform.
*/
@Test
fun `a control envelope from a newer peer is not renderable`() {
val fromTheFuture = parse(
"""{"fnc":1,"kind":"something-invented-later","ctl":1,"text":"{}"}"""
)
assertTrue(fromTheFuture.isControl, "the ctl flag was not honoured")
assertFalse(fromTheFuture.isRenderable,
"an unknown control kind would be drawn as a message")
}
/**
* ⚠️ Both shapes of the flag. `org.json`'s `optBoolean` does not coerce a
* number, so an engine sending `"ctl":1` rather than `"ctl":true` would slip
* past a reader that checked only one — which is how this class of bug got
* shipped once already.
*/
@Test
fun `the control flag is honoured as a number and as a boolean`() {
assertFalse(parse("""{"fnc":1,"kind":"x","ctl":1,"text":"{}"}""").isRenderable)
assertFalse(parse("""{"fnc":1,"kind":"x","ctl":true,"text":"{}"}""").isRenderable)
}
// ---- What must still be drawn ---------------------------------------
/**
* 🔴 The guard must not swallow ordinary messages. A `continue` in the wrong
* place is a conversation that silently stops receiving.
*/
@Test
fun `an ordinary message is still renderable`() {
val text = parse(Content.text("hello", messageId = "mid-1"))
assertTrue(text.isRenderable)
}
@Test
fun `a reply is still renderable`() {
val reply = parse(
Content.text("answering", messageId = "mid-2",
about = Content.Reference("mid-1", 7L, "the question")),
)
assertTrue(reply.isRenderable)
assertTrue(reply.about?.messageId == "mid-1")
}
/**
* 🔴 **A forward IS a message.** It is second-hand words, not a control
* envelope — the flag changes how it is drawn, not whether it is.
*/
@Test
fun `a forwarded message is renderable, and says so`() {
val forwarded = parse(
Content.text("passed along", messageId = "mid-3", forwarded = true),
)
assertTrue(forwarded.isRenderable, "a forward was treated as control")
assertTrue(forwarded.isForwarded, "the fwd flag was lost")
}
/**
* ⚠️ And the same both-shapes rule for `fwd`, for the same reason. An engine
* writing `1` where this one writes `true` would have its forwards drawn as
* the sender's own words.
*/
@Test
fun `the forwarded flag is honoured as a number and as a boolean`() {
assertTrue(parse("""{"fnc":1,"kind":"text","fwd":1,"text":"x"}""").isForwarded)
assertTrue(parse("""{"fnc":1,"kind":"text","fwd":true,"text":"x"}""").isForwarded)
assertFalse(parse("""{"fnc":1,"kind":"text","text":"x"}""").isForwarded)
}
}
engine/src/main/kotlin/io/flamenet/e2e/Vault.kt
+30 -1
View File
@@ -1658,7 +1658,36 @@ class Vault(private val storage: Storage, private val key: SecretKey) {
* make the next message from that person undecryptable — a "clear chat"
* that quietly breaks the chat.
*/
fun forgetHistory(threadId: String) = storage.delete(historyName(threadId))
fun forgetHistory(threadId: String) {
storage.delete(historyName(threadId))
// ⚠️ The pin goes with the transcript. A banner pointing at a message
// that was just cleared is a reference to nothing, and it would survive
// "clear this conversation" — which reads as the clear not having
// worked.
storage.delete(pinnedName(threadId))
}
// -- Pinned message (§6.19) ----------------------------------------------
private fun pinnedName(threadId: String) = "pinned-$threadId"
/**
* The message pinned in a thread, or null.
*
* 🔴 **Encrypted at rest like the transcript, not a preference.** Which
* message somebody pinned in which conversation is exactly the shape of
* metadata the vault exists to keep off a stolen phone — and it is stored
* as an ID, never a copy, so it cannot go stale against an edit or outlive
* a deletion.
*/
fun pinnedMessage(threadId: String): String? =
readJson(pinnedName(threadId))?.optString("mid")?.takeIf { it.isNotEmpty() }
/** Pin a message, or clear the pin with null. */
fun savePinnedMessage(threadId: String, messageId: String?) {
if (messageId.isNullOrEmpty()) storage.delete(pinnedName(threadId))
else writeJson(pinnedName(threadId), JSONObject().put("mid", messageId))
}
/**
* Delete one line from a thread's history (delete for me).