diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index f46871d4c1..889917c7bb 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -3789,6 +3789,17 @@ class Account( // Restore Marmot MLS group state on startup if (marmotManager != null) { + // Derived kind:1210 rows go straight into the conversation. Only + // DERIVED rows arrive here — one received over the wire is an + // assertion by its sender and is dropped at ingest — so these are + // safe to render with attribution. + marmotManager.onSystemRowDerived = { groupId, row -> + cache.justConsume(row, null, true) + val note = cache.getOrCreateNote(row.id) + note.event = row + marmotGroupList.addMessage(groupId, note) + } + scope.launch(Dispatchers.IO) { marmotManager.restoreAll() diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt index cb5e51da5c..15bc34c0c7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt @@ -726,7 +726,9 @@ class GroupEventHandler( } } - // Track the message in the Marmot group chatroom + // Track the message in the Marmot group chatroom. A + // peer-sent kind:1210 is dropped inside addMessage — see + // `MarmotGroupList.isDisplayableFeedMessage`. account.marmotGroupList.addMessage(result.groupId, innerNote) // Persist the decrypted plaintext so the message @@ -764,6 +766,12 @@ class GroupEventHandler( // Sync MIP-01 metadata after epoch advance (extensions may have changed) val chatroom = account.marmotGroupList.getOrCreateGroup(result.groupId) manager.syncMetadataTo(result.groupId, chatroom) + // The epoch just advanced, so whatever this commit changed + // is now canonical state — which is exactly what a kind:1210 + // row is derived from. Deriving here covers OTHER members' + // commits; our own are derived by `commitAndPublish`. Both + // reach the feed through `onSystemRowDerived`. + manager.syncGroupSystemRows(result.groupId) // Epoch just advanced — drain any kind:445 events that // previously failed as UndecryptableOuterLayer for this // group. See `pendingUndecryptable` for the scenario. diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index 79ec80fd96..17de8a23ac 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -142,6 +142,20 @@ class MarmotManager( val inboundProcessor = MarmotInboundProcessor(groupManager, keyPackageRotationManager) val outboundProcessor = MarmotOutboundProcessor(groupManager) val welcomeSender = MarmotWelcomeSender(signer) + + /** + * Called for every kind:1210 row this client DERIVES, so a front end can + * surface it in the conversation as it happens. + * + * Only derived rows come through here, and that is the point. A 1210 that + * arrives over the wire is an assertion by its sender — see + * [syncGroupSystemRows] — so a renderer that took its `actor`/`subject` + * from the payload would let any member forge an attributed history row. + * These rows are diffed from MLS-authenticated state instead, which is why + * they are safe to attribute. + */ + var onSystemRowDerived: ((nostrGroupId: HexKey, row: Event) -> Unit)? = null + val publishGate = publishObligationStore?.let { MarmotPublishGate(groupManager, it) } ?: MarmotPublishGate(groupManager) @@ -1194,7 +1208,12 @@ class MarmotManager( // app event has a pubkey — this client, whose local derivation // it is. val appEvent = row.toAppEvent(actor ?: signer.pubKey, now) - persistDecryptedMessage(nostrGroupId, appEvent.toJson().dropLast(1) + ",\"sig\":\"\"}") + val json = appEvent.toJson().dropLast(1) + ",\"sig\":\"\"}" + persistDecryptedMessage(nostrGroupId, json) + // Surface it now as well as persisting it. Without this the + // row appears only after a restart re-reads the log, which is + // the wrong moment to learn that someone was removed. + Event.fromJsonOrNull(json)?.let { onSystemRowDerived?.invoke(nostrGroupId, it) } } store.recordGroupSnapshot(nostrGroupId, current.encode()) rows diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupList.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupList.kt index 939ac252d7..63be18ff2b 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupList.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupList.kt @@ -142,18 +142,43 @@ class MarmotGroupList( * 1210 system rows are NOT in this list. They are group-state captions * rather than messages, but they belong in the conversation in * chronological order, so the feed carries them and the renderer gives - * them their own style instead of a chat bubble. + * them their own style instead of a chat bubble — subject to the + * authorship rule below. */ private fun isDisplayableFeedMessage(msg: Note): Boolean { val kind = msg.event?.kind ?: return true + if (kind == MARMOT_INNER_KIND_SYSTEM_ROW) return isOwnDerivedSystemRow(msg) return kind !in NON_CHAT_INNER_KINDS } + /** + * A kind:1210 row is shown only when THIS client derived it. + * + * MLS authenticates that a member sent an inner payload; it says nothing + * about whether the payload is true. A received 1210 is therefore an + * assertion by its sender, with an `actor` and `subject` of the sender's + * choosing — so rendering one would let any member forge an attributed + * history row ("X removed Y") indistinguishable from a real one, in the + * part of the conversation a reader trusts most. + * + * Rows this client derives are diffed from MLS-authenticated group state + * (`MarmotManager.syncGroupSystemRows`) and are always authored by the + * account itself, so authorship is exactly the test. Nothing is lost by + * dropping the sender's version: every client that applied the same + * commits derives the same rows. + * + * The check has to live here rather than at ingest because rows reach the + * feed by two routes — live decryption and the restart re-read of the + * local log — and the log holds received payloads too. + */ + private fun isOwnDerivedSystemRow(msg: Note): Boolean = msg.event?.pubKey == ownerPubKey + companion object { private const val MARMOT_INNER_KIND_DELETION = 5 private const val MARMOT_INNER_KIND_REACTION = 7 private const val MARMOT_INNER_KIND_EDIT = 1009 private const val MARMOT_INNER_KIND_STREAM_START = 1200 + private const val MARMOT_INNER_KIND_SYSTEM_ROW = 1210 // Push token gossip. Routing data for a notification server, addressed // to the other members' clients rather than to the people in the room — diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupFeedVisibilityTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupFeedVisibilityTest.kt new file mode 100644 index 0000000000..1caf3dacd4 --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupFeedVisibilityTest.kt @@ -0,0 +1,108 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.model.marmotGroups + +import com.vitorpamplona.amethyst.commons.model.AddressableNote +import com.vitorpamplona.amethyst.commons.model.Note +import com.vitorpamplona.amethyst.commons.model.User +import com.vitorpamplona.amethyst.commons.model.UserContext +import com.vitorpamplona.quartz.nip01Core.core.Event +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * Which inner app events become rows in a Marmot group's conversation. + * + * The kind:1210 rule is a security boundary, not a display preference. MLS + * authenticates that a member SENT a payload; it says nothing about whether the + * payload is TRUE. The reference client draws the same line — its own fuzz + * target asserts that raw 1210 JSON "must not authenticate a payload actor" and + * that a parsed payload stays an unauthenticated projection. + */ +class MarmotGroupFeedVisibilityTest { + private val owner = "a".repeat(64) + private val peer = "b".repeat(64) + private val groupId = "c".repeat(64) + + private val context = UserContext { addr -> AddressableNote(addr) } + + private fun note( + kind: Int, + pubKey: String, + id: String = "${kind}0".padEnd(64, 'f'), + content: String = "", + ): Note { + val event = Event(id, pubKey, 1_800_000_000L, kind, emptyArray(), content, "") + return Note(event.id).also { it.loadEvent(event, User(pubKey, context), emptyList()) } + } + + private fun list() = MarmotGroupList(owner) + + private fun visibleCount( + list: MarmotGroupList, + note: Note, + ): Int { + list.addMessage(groupId, note) + return list.getOrCreateGroup(groupId).messages.size + } + + @Test + fun `a chat message is shown`() { + assertEquals(1, visibleCount(list(), note(9, peer, content = "hello"))) + } + + @Test + fun `a system row this client derived is shown`() { + // Derived rows are diffed from MLS-authenticated state and are always + // authored by the account itself, so authorship is what marks them. + assertEquals(1, visibleCount(list(), note(1210, owner))) + } + + @Test + fun `a system row sent by another member is refused`() { + // The forgery this blocks: any member can send a well-formed 1210 + // naming someone else as the actor of a removal or a rename, and it + // would render exactly like a real one in the part of the conversation + // a reader trusts most. + assertEquals(0, visibleCount(list(), note(1210, peer))) + } + + @Test + fun `the side-channel kinds never become rows`() { + // Reactions, deletions, edits, stream anchors and push token gossip all + // reach LocalCache — they drive other UI — but none is a message. + listOf(5, 7, 1009, 1200, 447, 448, 449).forEach { kind -> + assertEquals(0, visibleCount(list(), note(kind, peer)), "kind $kind must not render as a row") + } + } + + @Test + fun `authorship is the only thing that admits a system row`() { + // Not the group, not the arrival path, not the payload's own claims. + val list = list() + val forged = note(1210, peer, id = "1".repeat(64), content = """{"v":1,"system_type":"member_removed","data":{"actor":"$owner"}}""") + list.addMessage(groupId, forged) + assertTrue(list.getOrCreateGroup(groupId).messages.size == 0, "a payload cannot vouch for itself") + assertFalse(list.groupIdForNote(forged.idHex) == groupId) + } +} diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotEditsAndSystemRowsTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotEditsAndSystemRowsTest.kt index 7a85111cec..d40d9216a4 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotEditsAndSystemRowsTest.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotEditsAndSystemRowsTest.kt @@ -183,4 +183,25 @@ class MarmotEditsAndSystemRowsTest { val rows = restarted.loadStoredMessages(nostrGroupId).mapNotNull { Event.fromJsonOrNull(it) }.filter { it.kind == MarmotAppEvent.KIND_SYSTEM } assertEquals(1, rows.size) } + + @Test + fun `a derived row is surfaced as it happens, not only after a restart`() = + runBlocking { + // Rows used to reach the conversation only when a restart re-read + // the local log, which is the wrong moment to learn that someone + // was removed from the group. + val f = Fixture() + f.createGroup(name = "before") + f.manager.syncGroupSystemRows(nostrGroupId) + + val surfaced = mutableListOf() + f.manager.onSystemRowDerived = { _, row -> surfaced.add(row) } + f.manager.setGroupProfile(nostrGroupId, "after", "") + + assertEquals(1, surfaced.size, "a rename must surface exactly one row") + assertEquals(MarmotAppEvent.KIND_SYSTEM, surfaced.single().kind) + // Authored by this client: a derived row is OUR reading of + // authenticated state, and the feed admits a 1210 on exactly that. + assertEquals(f.signer.pubKey, surfaced.single().pubKey) + } }