fix(marmot): a Commit must not rebuild our own leaf from defaults

A Commit replaces the committer's leaf through the UpdatePath. It is the same member with new key material, so everything the leaf says about the member has to survive — but `buildLeafNode` was called without either `capabilities` or `leafExtensions`, so it fell through to the legacy defaults every time. That cost us every invitation we have ever sent to MDK. A current-profile leaf carries `marmot.member.account-identity-proof.v2` inside an `app_data_dictionary` LEAF extension, and no proposal can put a leaf extension back, so our very first Commit silently demoted the group creator out of the current profile. The rebuilt leaf also stopped advertising the `app_data_dictionary` extension (0x0006) and the `app_data_update` proposal (0x0008) that the group's own `required_capabilities` demands, which makes the resulting tree fail RFC 9420 §7.3 leaf validation for every receiver. MDK reported `PublicGroupError(LeafNodeValidation(UnsupportedExtensions))` and dropped the Welcome minted by that same commit — the invitee simply never saw an invite, with nothing logged on either side. The same omission was in `proposeSigningKeyRotation`, where an Update proposal replaces our leaf for forward secrecy, and in `externalJoin`, which had no way to express a current-profile joiner leaf at all. Verified against MDK 0.9.20 on the interop harness: before, the invitee's pending-invite list stayed empty; after, our group arrives with its routing, profile and admin policy intact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016kCuA6tc4JQzHPCDd39GHq
2026-10-05 19:28:25 +00:00 · 2026-09-09 03:01:17 +00:00
parent a044f4c38b
commit 36c73fd654
4 changed files with 234 additions and 1 deletions
@@ -10,3 +10,4 @@ git/state-git-nip34/
 buzz/state-job-loop/
 buzz/state-workflow-loop/
 buzz/state-agent-exec/
+marmot/state-repro/
@@ -446,6 +446,9 @@ class MlsGroup private constructor(
            (currentLeaf?.credential as? Credential.Basic)?.identity
                ?: ByteArray(0)

+        // An Update replaces our leaf with fresh key material and nothing
+        // else. Capabilities and leaf extensions (the account identity proof
+        // among them) describe the member, not the keys, so they carry over.
        val newLeafNode =
            buildLeafNode(
                encryptionKey = newEncKp.publicKey,
@@ -455,6 +458,8 @@ class MlsGroup private constructor(
                signingKey = newSigKp.privateKey,
                groupId = groupId,
                leafIndex = myLeafIndex,
+                capabilities = currentLeaf?.capabilities ?: marmotLeafCapabilities(),
+                leafExtensions = currentLeaf?.extensions ?: emptyList(),
            )

        val proposal = Proposal.Update(newLeafNode)
@@ -694,18 +699,35 @@ class MlsGroup private constructor(
                // signature we mint fails to verify.
                val effectiveSigningKey = pendingSigningKey ?: signingPrivateKey
                val newEncKp = X25519.generateKeyPair()
+                // RFC 9420 §7.1: an UpdatePath leaf REPLACES our leaf. It is
+                // the same member, so everything about that member that is not
+                // key material carries over — capabilities and the leaf
+                // extensions. Rebuilding from defaults instead is not a
+                // cosmetic loss: a current-profile leaf keeps its
+                // `account-identity-proof` in an `app_data_dictionary` LEAF
+                // extension, and that extension can never be re-added by a
+                // proposal, so dropping it here silently demotes us out of the
+                // current profile at our very first commit. It also drops the
+                // `app_data_dictionary` capability the group's own
+                // `required_capabilities` demands, which makes the resulting
+                // tree fail RFC 9420 §7.3 leaf validation for every receiver —
+                // openmls reports `LeafNodeValidation(UnsupportedExtensions)`
+                // and the Welcome we just minted is unjoinable.
+                val previousLeaf = tree.getLeaf(myLeafIndex)
                val newLeafNode =
                    buildLeafNode(
                        encryptionKey = newEncKp.publicKey,
                        signatureKey = Ed25519.publicFromPrivate(effectiveSigningKey),
                        identity =
-                            (tree.getLeaf(myLeafIndex)?.credential as? Credential.Basic)?.identity
+                            (previousLeaf?.credential as? Credential.Basic)?.identity
                                ?: ByteArray(0),
                        source = LeafNodeSource.COMMIT,
                        signingKey = effectiveSigningKey,
                        groupId = groupId,
                        leafIndex = myLeafIndex,
                        parentHash = leafParentHash,
+                        capabilities = previousLeaf?.capabilities ?: marmotLeafCapabilities(),
+                        leafExtensions = previousLeaf?.extensions ?: emptyList(),
                    )
                encryptionPrivateKey = newEncKp.privateKey
                tree.setLeaf(myLeafIndex, newLeafNode)
@@ -3583,6 +3605,12 @@ class MlsGroup private constructor(
         * @param groupInfoBytes TLS-serialized GroupInfo
         * @param identity the joiner's identity
         * @param signingKey optional Ed25519 signing key (generated if null)
+         * @param capabilities the joiner leaf's capabilities. A current-profile
+         *   join MUST pass [currentProfileLeafCapabilities]; the default only
+         *   satisfies a legacy group's `required_capabilities`.
+         * @param leafExtensions the joiner leaf's extensions. A current-profile
+         *   join MUST pass the `app_data_dictionary` carrying its account
+         *   identity proof — leaf extensions cannot be added after the fact.
         * @return the new MlsGroup along with the raw inner commit bytes and a
         *   wire-ready PublicMessage envelope. Existing group members consume
         *   the framed bytes via [MlsGroup.processFramedCommit].
@@ -3591,6 +3619,8 @@ class MlsGroup private constructor(
            groupInfoBytes: ByteArray,
            identity: ByteArray,
            signingKey: ByteArray? = null,
+            capabilities: Capabilities = marmotLeafCapabilities(),
+            leafExtensions: List<Extension> = emptyList(),
        ): ExternalJoinResult {
            val groupInfo = GroupInfo.decodeTls(TlsReader(groupInfoBytes))
            val groupContext = groupInfo.groupContext
@@ -3655,6 +3685,8 @@ class MlsGroup private constructor(
                    signingKey = sigKp.privateKey,
                    groupId = groupContext.groupId,
                    leafIndex = tree.leafCount,
+                    capabilities = capabilities,
+                    leafExtensions = leafExtensions,
                )
            val myLeafIndex = tree.addLeaf(placeholderLeaf)

@@ -3717,6 +3749,8 @@ class MlsGroup private constructor(
                    groupId = groupContext.groupId,
                    leafIndex = myLeafIndex,
                    parentHash = extLeafParentHash,
+                    capabilities = capabilities,
+                    leafExtensions = leafExtensions,
                )
            tree.setLeaf(myLeafIndex, leafNode)

@@ -0,0 +1,80 @@
+/*
+ * 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.quartz.marmot.mip00KeyPackages
+
+import com.vitorpamplona.quartz.nip01Core.core.toHexKey
+import kotlin.io.encoding.Base64
+import kotlin.io.encoding.ExperimentalEncodingApi
+import kotlin.test.Test
+import kotlin.test.assertContentEquals
+import kotlin.test.assertEquals
+
+/**
+ * A KeyPackage we did not author has to survive decode -> re-encode byte for
+ * byte, because the KeyPackageRef that names the joining member in a Welcome is
+ * `RefHash("MLS 1.0 KeyPackage Reference", serialize(KeyPackage))` over exactly
+ * those bytes. The invitee looks its private bundle up by that hash. One byte
+ * of drift anywhere in the struct -- a dropped extension, a reordered
+ * dictionary entry, a differently-sized length prefix -- and the Welcome names
+ * a KeyPackage nobody has, so the invitee silently has nothing to join with.
+ *
+ * The fixture is a real KeyPackage published by MDK 0.9.20 (`wn`), captured off
+ * the interop harness relay.
+ */
+@OptIn(ExperimentalEncodingApi::class)
+class MdkKeyPackageRoundTripTest {
+    private val mdkFramedKeyPackage = "AAEABQABAAEgGhmBTAXpFdSxyMphrHiFKeFO4cmy5AvBVb1NSULTC18gWC4G7Zsb/wLG17JOfUoWuNLG7MpbwUNz/pgGxFy9EUsgiXlykzgtwdj1WFEW8/pwDNMzuah4ukUFMiySi4Au1loAASCNeT1vOihOZ/jZIF1Lltoa/AY1+fTYX3P/uxDoiykUeQIAAQIAAQgABvLR8tLy1AQACgAIAgABAQAAAABqoLL8AAAAAGsPfwxAkgAGQI5AjAABGRgAAYABgAKAA4AEgAWABoAHgAiACYALgAwAAgEAgAlAaI15PW86KE5n+NkgXUuW2hr8BjX59Nhfc/+7EOiLKRR5AAAAAGqgwQxu5AAeyJtPRpsTzslSt1o2N+kwYHQG5wUPsGgedSFMdRqP4mZeNEn8hi5zImQORBhf7tf4hISvja/gd/prx4ecQEC1dvSgT+hvumSJOgjdmLt9Yq9YCUY1Ih45QZXAy3EzPWTRfr5URwifcj4sscM7k9g7MPoAJQ1P4apLTlSgt7ILBwAGBAMABABAQJUP1427jN2rhINxSQuF2pKugrrvb6Qzo7kUYCGL8cHNcf2QArnyBdPxgNX/IRYk/d0bdQcByU81gSGPeA42PA8="
+
+    @Test
+    fun reEncodesAnMdkKeyPackageByteForByte() {
+        val framed = Base64.decode(mdkFramedKeyPackage)
+        val keyPackage = KeyPackageUtils.decodeKeyPackage(framed)
+
+        // The framed publication is 4 bytes of MLSMessage header plus the
+        // KeyPackage struct; compare against the payload, not the envelope.
+        val payload = framed.copyOfRange(4, framed.size)
+        assertContentEquals(payload, keyPackage.toTlsBytes())
+    }
+
+    @Test
+    fun reFramesToTheExactBytesMdkPublished() {
+        val framed = Base64.decode(mdkFramedKeyPackage)
+        val keyPackage = KeyPackageUtils.decodeKeyPackage(framed)
+        assertContentEquals(framed, KeyPackageUtils.frameKeyPackage(keyPackage))
+    }
+
+    /**
+     * The `i` tag MDK put on the publication is the KeyPackageRef MDK filed its
+     * own private bundle under. Recomputing it from the bytes we decoded is the
+     * end-to-end check: if these agree, a Welcome we address to this member
+     * names a KeyPackage the member can actually find.
+     */
+    @Test
+    fun computesTheSameReferenceMdkAdvertised() {
+        val framed = Base64.decode(mdkFramedKeyPackage)
+        val keyPackage = KeyPackageUtils.decodeKeyPackage(framed)
+        assertEquals(MDK_ADVERTISED_REF, keyPackage.reference().toHexKey())
+    }
+
+    companion object {
+        private const val MDK_ADVERTISED_REF = "1e6606f8e154a0c148587c4334f801fbb7aaf5a806d8971a87961a58366844ef"
+    }
+}
@@ -0,0 +1,118 @@
+/*
+ * 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.quartz.marmot.mls.group
+
+import com.vitorpamplona.quartz.marmot.appComponents.AppComponentIds
+import com.vitorpamplona.quartz.marmot.appComponents.CurrentProfileGroupFactory
+import com.vitorpamplona.quartz.marmot.appComponents.GroupProfileV1
+import com.vitorpamplona.quartz.marmot.mls.components.AppDataDictionary
+import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair
+import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal
+import kotlinx.coroutines.runBlocking
+import kotlin.test.Test
+import kotlin.test.assertContains
+import kotlin.test.assertNotNull
+import kotlin.test.assertTrue
+
+/**
+ * A Commit replaces the committer's own leaf through the UpdatePath. That leaf
+ * is the SAME member with new key material, so everything the leaf says about
+ * the member has to survive: its `capabilities`, and its extensions.
+ *
+ * Rebuilding from defaults instead cost us every invitation. A current-profile
+ * leaf carries `marmot.member.account-identity-proof.v2` inside an
+ * `app_data_dictionary` LEAF extension, and no proposal can put a leaf
+ * extension back, so the first Commit silently demoted the creator out of the
+ * current profile. Worse, the rebuilt leaf stopped advertising the
+ * `app_data_dictionary` extension and `app_data_update` proposal that the
+ * group's own `required_capabilities` demands, so the resulting tree failed
+ * RFC 9420 leaf validation for every receiver: MDK's openmls reported
+ * `LeafNodeValidation(UnsupportedExtensions)` and dropped the Welcome we had
+ * just minted from that same commit.
+ */
+class CommitPreservesLeafIdentityTest {
+    /** `app_data_update`, the proposal a current-profile group requires. */
+    private val appDataUpdateProposalType = 0x0008
+
+    private fun signer(seed: Byte) = NostrSignerInternal(KeyPair(ByteArray(32) { seed }))
+
+    private suspend fun aCurrentProfileGroup(): MlsGroup =
+        CurrentProfileGroupFactory.createGroup(
+            signer = signer(0x11),
+            nostrGroupId = ByteArray(32) { 0x22 },
+            relays = listOf("wss://relay.example.com"),
+            profile = GroupProfileV1("Interop", ""),
+        )
+
+    private fun MlsGroup.myLeaf() = members().firstOrNull { it.first == leafIndex }?.second
+
+    @Test
+    fun theCommitterLeafKeepsItsCurrentProfileCapabilities() =
+        runBlocking<Unit> {
+            val group = aCurrentProfileGroup()
+            val before = assertNotNull(group.myLeaf()).capabilities
+
+            val invitee = CurrentProfileGroupFactory.createKeyPackage(signer(0x33))
+            group.proposeAdd(invitee.keyPackage.toTlsBytes())
+            group.commit()
+
+            val after = assertNotNull(group.myLeaf()).capabilities
+            assertContains(after.extensions, AppDataDictionary.EXTENSION_TYPE)
+            assertContains(after.proposals, appDataUpdateProposalType)
+            assertTrue(before.extensions.all { it in after.extensions })
+            assertTrue(before.proposals.all { it in after.proposals })
+        }
+
+    @Test
+    fun theCommitterLeafKeepsItsAccountIdentityProof() =
+        runBlocking<Unit> {
+            val group = aCurrentProfileGroup()
+
+            val invitee = CurrentProfileGroupFactory.createKeyPackage(signer(0x33))
+            group.proposeAdd(invitee.keyPackage.toTlsBytes())
+            group.commit()
+
+            val leaf = assertNotNull(group.myLeaf())
+            val dictionary =
+                assertNotNull(
+                    leaf.extensions.firstOrNull { it.extensionType == AppDataDictionary.EXTENSION_TYPE },
+                )
+            val decoded = AppDataDictionary.decode(dictionary.extensionData)
+            assertNotNull(decoded[AppComponentIds.ACCOUNT_IDENTITY_PROOF_V2])
+        }
+
+    @Test
+    fun aSigningKeyRotationAlsoKeepsTheLeafIdentity() =
+        runBlocking<Unit> {
+            val group = aCurrentProfileGroup()
+            val before = assertNotNull(group.myLeaf())
+
+            group.proposeSigningKeyRotation()
+            group.commit()
+
+            val after = assertNotNull(group.myLeaf())
+            assertContains(after.capabilities.extensions, AppDataDictionary.EXTENSION_TYPE)
+            assertTrue(
+                after.extensions.any { it.extensionType == AppDataDictionary.EXTENSION_TYPE },
+            )
+            assertTrue(before.extensions.size <= after.extensions.size)
+        }
+}