fix(marmot): never render a system row a peer asserted

Rendering kind:1210 rows, added earlier today, trusted the wrong thing. MLS
authenticates that a member SENT an inner payload; it says nothing about
whether the payload is true. The renderer read `actor` and `subject` straight
out of that payload, so any member could send a well-formed 1210 saying "X
removed Y" or "X renamed the group" and Amethyst would draw it as a system
caption — attributed, styled as history, indistinguishable from a real one,
in the part of a conversation a reader trusts most.

`syncGroupSystemRows` already documented the rule ("one that arrives over the
wire is an assertion by its sender, not a derived fact"); the render path
simply did not honour it. The reference client draws the same line from the
other side — its raw 1210 parser nulls attribution outright and marks every
result unauthenticated, with a fuzz target asserting exactly that.

The rule now lives at the one choke point every row passes through:
`MarmotGroupList` shows a 1210 only when this client authored it. That is the
right test because a derived row is diffed from MLS-authenticated state and is
always authored by the account itself. It has to be there rather than at
ingest, because rows arrive by two routes — live decryption and the restart
re-read of the local log — and the log holds received payloads too, so an
ingest-only guard would have let a forgery back in on the next launch.

Dropping the sender's version costs nothing: every client that applied the
same commits derives the same rows.

The same feature had a second defect, which the first one was hiding. Derived
rows were persisted but never surfaced, so they appeared only after a restart,
and Android derived them solely for its own commits. In practice the only 1210s
reaching the feed live were the untrusted ones. Rows now surface as they are
derived, and every accepted commit derives them, not just ours.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016kCuA6tc4JQzHPCDd39GHq

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()
 

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.

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

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 —

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)
+    }
+}

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<Event>()
+            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)
+        }
 }