From a044f4c38bdf4eef3eb03049a65aec9d6c5e0b46 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 00:27:59 +0000 Subject: [PATCH] fix(marmot): create current-profile groups from the CLI, and fix the relay sets that broke them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `amy marmot group create` now builds a current-profile group; `--legacy` keeps the MIP-era path for reproducing groups already on disk. The difference is what a group REQUIRES of a joining leaf — the account identity proof, which every conformant peer's KeyPackage carries, versus `0xF2EE`, which none of them do. With this, `group add` accepts an MDK KeyPackage where it previously failed the capability gate outright. Making that work surfaced two bugs that would each have been fatal on their own. `MarmotManager.groupRelays` read only the legacy `0xF2EE` extension, but a current-profile group routes through `NostrRoutingV1` (`0x8004`). Every current-profile group therefore had an EMPTY recipient scope — so under publish-before-apply no commit could ever be acknowledged, and no such group could ever advance past epoch 0. It now reads both. And the local-network relay filter turned up a third time, in NIP-65: `parseReadNorm`/`parseWriteNorm` dropped loopback entries, so our own outbox and inbox read as empty while `nip65` showed the relay. Everything then published to the default relay set — which is why commits and Welcomes were going to public relays instead of the harness's loopback. Same split as before: filtered for someone else's list (it is attacker-supplied input, and it is what exempts a relay from Tor), unfiltered for reading back our own. Two conformance fixes came with it. The Welcome rumor carried an `encoding` tag, which the binding forbids outright for every event shape it defines — a receiver that switched decoders on one could be steered into a different parse of the same bytes. And we implemented encrypted-media v2 last commit but never advertised `0x800b` in the leaf, so a group requiring it would refuse us; the advertised list now carries it, with a note that an id belongs there only when the component is actually implemented. `MarmotMipBehaviorTest` asserted the MIP-era rule that a rumor MUST carry an encoding tag. The adopted binding reverses it, so the test now asserts the current rule. Interop test 01 still passes. Test 02 (invite MDK into our group) reaches MDK but is not yet ingested; a control run confirms MDK->MDK invites work in this harness, so the remaining defect is ours, in the Welcome. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016kCuA6tc4JQzHPCDd39GHq --- .../com/vitorpamplona/amethyst/cli/Context.kt | 23 +++++----- .../amethyst/cli/commands/GroupCommands.kt | 4 +- .../cli/commands/GroupCreateCommand.kt | 46 +++++++++++++------ .../amethyst/cli/commands/RelayCommands.kt | 13 ++++-- .../amethyst/commons/marmot/MarmotManager.kt | 23 +++++++--- .../CurrentProfileGroupFactory.kt | 8 ++++ .../marmot/mip02Welcome/WelcomeEvent.kt | 6 ++- .../AdvertisedRelayListEvent.kt | 6 +++ .../tags/AdvertisedRelayInfoTag.kt | 39 +++++++++++----- .../quartz/marmot/MarmotMipBehaviorTest.kt | 12 +++-- 10 files changed, 128 insertions(+), 52 deletions(-) diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/Context.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/Context.kt index 15b1205945..a25ee4d8e1 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/Context.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/Context.kt @@ -457,7 +457,7 @@ class Context( * Android app. */ suspend fun outboxRelays(): Set = - relaysOf(identity.pubKeyHex)?.writeRelaysNorm()?.takeIf { it.isNotEmpty() }?.toSet() + relaysOf(identity.pubKeyHex)?.allWriteRelaysNorm()?.takeIf { it.isNotEmpty() }?.toSet() ?: DefaultNIP65RelaySet /** @@ -468,7 +468,7 @@ class Context( * marked. */ suspend fun nip65ReadRelays(): Set = - relaysOf(identity.pubKeyHex)?.readRelaysNorm()?.takeIf { it.isNotEmpty() }?.toSet() + relaysOf(identity.pubKeyHex)?.allReadRelaysNorm()?.takeIf { it.isNotEmpty() }?.toSet() ?: outboxRelays() /** @@ -476,7 +476,7 @@ class Context( * to [DefaultDMRelayList] when no kind:10050 has been seen. */ suspend fun inboxRelays(): Set = - dmInboxOf(identity.pubKeyHex)?.relays()?.takeIf { it.isNotEmpty() }?.toSet() + dmInboxOf(identity.pubKeyHex)?.allRelays()?.takeIf { it.isNotEmpty() }?.toSet() ?: DefaultDMRelayList.toSet() /** @@ -885,14 +885,15 @@ class Context( } ?: input } - fun marmotGroupRelays(nostrGroupId: HexKey): Set { - val m = marmot.groupMetadata(nostrGroupId) ?: return emptySet() - return m.relays - .mapNotNull { - com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer - .normalizeOrNull(it) - }.toSet() - } + /** + * The group's own relay set, from whichever routing component it carries. + * + * Delegates rather than reading `MarmotGroupData` directly: a + * current-profile group has no `0xF2EE` extension at all, and reading only + * that one silently returned an empty set for every group the current + * profile creates. + */ + fun marmotGroupRelays(nostrGroupId: HexKey): Set = marmot.groupRelays(nostrGroupId).toSet() override fun close() { // Nothing to persist for an anonymous run (no account dir to write into). diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/GroupCommands.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/GroupCommands.kt index a729eb6cbb..eff9dd0b90 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/GroupCommands.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/GroupCommands.kt @@ -27,7 +27,9 @@ object GroupCommands { """ |amy marmot group — MLS group management | - | marmot group create [--name NAME] create an empty group (self-only) + | marmot group create [--name NAME] create an empty group (self-only); + | [--legacy] --legacy builds a MIP-era group that + | only 0xF2EE-capable leaves can join | marmot group list list joined groups | marmot group show GID print full group details | marmot group members GID print members diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/GroupCreateCommand.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/GroupCreateCommand.kt index 3f72301a57..7adb4d60b0 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/GroupCreateCommand.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/GroupCreateCommand.kt @@ -24,6 +24,7 @@ import com.vitorpamplona.amethyst.cli.Args import com.vitorpamplona.amethyst.cli.Context import com.vitorpamplona.amethyst.cli.DataDir import com.vitorpamplona.amethyst.cli.Output +import com.vitorpamplona.quartz.marmot.appComponents.GroupProfileV1 import com.vitorpamplona.quartz.marmot.mip01Groups.MarmotGroupData import com.vitorpamplona.quartz.nip01Core.core.toHexKey import com.vitorpamplona.quartz.utils.RandomInstance @@ -35,32 +36,51 @@ object GroupCreateCommand { ): Int { val args = Args(rest) val name = args.flag("name", "")!! + val legacy = args.bool("legacy") args.rejectUnknown() Context.open(dataDir).use { ctx -> ctx.prepare() val gid = RandomInstance.bytes(32).toHexKey() - - // Stamp initial metadata via the shared factory so UI + CLI stay - // byte-identical. Bake the MarmotGroupData extension into the - // epoch-0 GroupContext directly (see `MarmotManager.createGroup`) - // so later invitees receive a pre-populated group from the - // welcome and never have to chase an undecryptable bootstrap - // commit that predates their membership. val outboxUrls = ctx.outboxRelays().map { it.url } - val metadata = - MarmotGroupData.bootstrap( + + if (legacy) { + // MIP-era group: its GroupContext requires the `0xF2EE` + // group-data extension, so only members whose leaves advertise + // that capability can be added. Kept for reproducing the + // behaviour of groups already on disk. + // + // Bake MarmotGroupData into the epoch-0 GroupContext directly + // so later invitees receive a pre-populated group from the + // welcome and never have to chase an undecryptable bootstrap + // commit that predates their membership. + val metadata = + MarmotGroupData.bootstrap( + nostrGroupId = gid, + creatorPubKey = ctx.identity.pubKeyHex, + outboxRelays = outboxUrls, + name = name, + ) + ctx.marmot.createGroup(gid, initialMetadata = metadata) + } else { + // Current profile by default. The difference is what the group + // REQUIRES of a joining leaf: a current-profile group asks for + // the account identity proof, which every conformant peer's + // KeyPackage carries, while a legacy group asks for `0xF2EE`, + // which none of them do. Defaulting to legacy made every + // outside member un-addable. + ctx.marmot.createCurrentProfileGroup( nostrGroupId = gid, - creatorPubKey = ctx.identity.pubKeyHex, - outboxRelays = outboxUrls, - name = name, + relays = outboxUrls, + profile = if (name.isEmpty()) null else GroupProfileV1(name, ""), ) - ctx.marmot.createGroup(gid, initialMetadata = metadata) + } Output.emit( mapOf( "group_id" to gid, "mls_group_id" to ctx.marmot.mlsGroupIdHex(gid), "name" to name, + "profile" to if (legacy) "legacy" else "current", "epoch" to ctx.marmot.groupEpoch(gid), ), ) diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/RelayCommands.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/RelayCommands.kt index cb2fabe839..09f31e2e29 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/RelayCommands.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/RelayCommands.kt @@ -557,8 +557,11 @@ object RelayCommands { mapOf( "noun" to "nip65", "kind" to AdvertisedRelayListEvent.KIND, - "read" to (nip65?.readRelaysNorm()?.map { it.url } ?: emptyList()), - "write" to (nip65?.writeRelaysNorm()?.map { it.url } ?: emptyList()), + // Unfiltered: this is OUR list, and reporting it + // through the attacker-input filter would hide a + // local relay the operator deliberately configured. + "read" to (nip65?.allReadRelaysNorm()?.map { it.url } ?: emptyList()), + "write" to (nip65?.allWriteRelaysNorm()?.map { it.url } ?: emptyList()), "relays" to (nip65?.relaysNorm()?.map { it.url } ?: emptyList()), ), ) @@ -641,8 +644,8 @@ object RelayCommands { val self = ctx.identity.pubKeyHex val nip65 = ctx.relaysOf(self) val out = linkedMapOf() - out["outbox"] = nip65?.writeRelaysNorm()?.map { it.url } ?: emptyList() - out["inbox"] = nip65?.readRelaysNorm()?.map { it.url } ?: emptyList() + out["outbox"] = nip65?.allWriteRelaysNorm()?.map { it.url } ?: emptyList() + out["inbox"] = nip65?.allReadRelaysNorm()?.map { it.url } ?: emptyList() out["nip65"] = nip65?.relaysNorm()?.map { it.url } ?: emptyList() for (flat in FLATS) { out[flat.jsonKey] = flat.read(ctx, self).map { it.url } @@ -732,7 +735,7 @@ object RelayCommands { facet: Facet, ): List { val nip65 = ctx.relaysOf(self) - val urls = if (facet == Facet.OUTBOX) nip65?.writeRelaysNorm() else nip65?.readRelaysNorm() + val urls = if (facet == Facet.OUTBOX) nip65?.allWriteRelaysNorm() else nip65?.allReadRelaysNorm() return urls?.map { it.url } ?: emptyList() } 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 9b16da3617..69e86bd978 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 @@ -668,13 +668,22 @@ class MarmotManager( * list, so a commit's acknowledgement has to come from an endpoint the * GROUP names — the same set every other member is listening on. */ - fun groupRelays(nostrGroupId: HexKey): List = - groupManager - .getGroup(nostrGroupId) - ?.currentMarmotData() - ?.relays - .orEmpty() - .mapNotNull { RelayUrlNormalizer.normalizeOrNull(it) } + fun groupRelays(nostrGroupId: HexKey): List { + val group = groupManager.getGroup(nostrGroupId) ?: return emptyList() + // A current-profile group routes through `marmot.transport.nostr.routing.v1` + // (`0x8004`); only a legacy group carries its relays in the `0xF2EE` + // group-data extension. Reading just the legacy one left every + // current-profile group with an empty recipient scope, so its commits + // had nowhere to be acknowledged and never became canonical. + val routed = + group + .currentGroupState() + .routing + ?.relays + .orEmpty() + val legacy = group.currentMarmotData()?.relays.orEmpty() + return (routed + legacy).distinct().mapNotNull { RelayUrlNormalizer.normalizeOrNull(it) } + } /** * Nuke all local Marmot state — every MLS group, every retained epoch diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/appComponents/CurrentProfileGroupFactory.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/appComponents/CurrentProfileGroupFactory.kt index 08a287c417..f70baa4648 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/appComponents/CurrentProfileGroupFactory.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/appComponents/CurrentProfileGroupFactory.kt @@ -55,6 +55,13 @@ object CurrentProfileGroupFactory { * * `0x0001` is in the list because a client advertising `app_data_dictionary` * must understand and advertise `app_components` itself. + * + * This list is what decides which groups will accept us. A group states the + * components it REQUIRES, and a leaf that does not advertise every one of + * them is refused — so a component we implement but forget to list here is + * invisible, and one we list but do not implement is a lie that surfaces + * later as a group we cannot actually participate in. Add an id here only + * when the component is implemented. */ val SUPPORTED_COMPONENTS: List = listOf( @@ -65,6 +72,7 @@ object CurrentProfileGroupFactory { AppComponentIds.NOSTR_ROUTING_V1, AppComponentIds.MESSAGE_RETENTION_V1, AppComponentIds.ACCOUNT_IDENTITY_PROOF_V2, + AppComponentIds.GROUP_ENCRYPTED_MEDIA_V2, AppComponentIds.GROUP_LIFECYCLE_V1, ) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip02Welcome/WelcomeEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip02Welcome/WelcomeEvent.kt index 552f9f07bf..38d70653d6 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip02Welcome/WelcomeEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip02Welcome/WelcomeEvent.kt @@ -89,7 +89,11 @@ class WelcomeEvent( ) = eventTemplate(KIND, welcomeBase64, createdAt) { keyPackageEventId(keyPackageEventId) welcomeRelays(relays) - encoding() + // No `encoding` tag. `transports/nostr.md`: "A sender MUST NOT add + // an `encoding` tag for any event shape in this document" — a + // receiver that switched decoders on one could be steered into a + // different parse of the same bytes, so the binding removes the + // negotiation entirely rather than defining it. nostrGroupId?.let { addUnique(arrayOf("h", it)) } initializer() } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip65RelayList/AdvertisedRelayListEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip65RelayList/AdvertisedRelayListEvent.kt index f17c9b57d1..8a619a021f 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip65RelayList/AdvertisedRelayListEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip65RelayList/AdvertisedRelayListEvent.kt @@ -47,10 +47,16 @@ class AdvertisedRelayListEvent( fun readRelaysNorm() = tags.mapNotNull(AdvertisedRelayInfo::parseReadNorm).ifEmpty { null } + /** Read-marked relays including local ones. For reading back OUR OWN list. */ + fun allReadRelaysNorm() = tags.mapNotNull(AdvertisedRelayInfo::parseReadNormUnfiltered).ifEmpty { null } + fun writeRelays() = tags.mapNotNull(AdvertisedRelayInfo::parseWrite).ifEmpty { null } fun writeRelaysNorm() = tags.mapNotNull(AdvertisedRelayInfo::parseWriteNorm).ifEmpty { null } + /** Write-marked relays including local ones. For reading back OUR OWN list. */ + fun allWriteRelaysNorm() = tags.mapNotNull(AdvertisedRelayInfo::parseWriteNormUnfiltered).ifEmpty { null } + companion object { const val KIND = 10002 const val ALT = "Relay list to discover the user's content" diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip65RelayList/tags/AdvertisedRelayInfoTag.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip65RelayList/tags/AdvertisedRelayInfoTag.kt index 0338fb9e71..ca6c9fe8e1 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip65RelayList/tags/AdvertisedRelayInfoTag.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip65RelayList/tags/AdvertisedRelayInfoTag.kt @@ -77,32 +77,49 @@ class AdvertisedRelayInfo( return tag[1] } - fun parseReadNorm(tag: Array): NormalizedRelayUrl? { + /** + * Read-marked relays, with local-network entries dropped. + * + * The drop is for lists that came from SOMEONE ELSE: a NIP-65 list is + * attacker-supplied input, an entry naming `127.0.0.1` or an RFC 1918 + * address would aim our connection at our own machine or LAN, and + * `isLocalHost` is also what exempts a relay from Tor. Use + * [parseReadNormUnfiltered] to read back a list this account published + * itself. + */ + fun parseReadNorm(tag: Array): NormalizedRelayUrl? = parseReadNormUnfiltered(tag)?.takeUnless { it.isLocalHost() } + + /** + * Read-marked relays including local ones, for reading back OUR OWN + * list. + * + * A user who configured a local relay meant it, and the filtered form + * reports their write set as empty — which every publisher then treats + * as "unconfigured" and answers with a default relay set the user never + * chose. + */ + fun parseReadNormUnfiltered(tag: Array): NormalizedRelayUrl? { ensure(tag.has(1) && tag[0] == TAG_NAME && tag[1].isNotEmpty()) { return null } if (tag.has(2)) { ensure(AdvertisedRelayType.isRead(tag[2])) { return null } } - val relay = RelayUrlNormalizer.normalizeOrNull(tag[1]) - - ensure(relay != null && !relay.isLocalHost()) { return null } - return RelayUrlNormalizer.normalizeOrNull(tag[1]) } - fun parseWriteNorm(tag: Array): NormalizedRelayUrl? { + /** Write-marked relays, local ones dropped. See [parseReadNorm]. */ + fun parseWriteNorm(tag: Array): NormalizedRelayUrl? = parseWriteNormUnfiltered(tag)?.takeUnless { it.isLocalHost() } + + /** Write-marked relays including local ones, for OUR OWN list. See [parseReadNormUnfiltered]. */ + fun parseWriteNormUnfiltered(tag: Array): NormalizedRelayUrl? { ensure(tag.has(1) && tag[0] == TAG_NAME && tag[1].isNotEmpty()) { return null } if (tag.has(2)) { ensure(AdvertisedRelayType.isWrite(tag[2])) { return null } } - val relay = RelayUrlNormalizer.normalizeOrNull(tag[1]) - - ensure(relay != null && !relay.isLocalHost()) { return null } - - return relay + return RelayUrlNormalizer.normalizeOrNull(tag[1]) } fun assemble( diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotMipBehaviorTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotMipBehaviorTest.kt index 2dcb186e0e..3d56e8839d 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotMipBehaviorTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotMipBehaviorTest.kt @@ -355,9 +355,15 @@ class MarmotMipBehaviorTest { assertEquals(WelcomeEvent.KIND, rumor.kind, "Innermost rumor MUST be kind:444") assertEquals("", rumor.sig, "MIP-02: kind:444 rumor MUST NOT carry a signature") - val encodingTag = rumor.tags.find { it.isNotEmpty() && it[0] == "encoding" } - assertNotNull(encodingTag, "MIP-02: rumor MUST carry [encoding, base64]") - assertEquals("base64", encodingTag[1]) + // The MIP-era rule required an `encoding` tag here. The adopted + // binding REVERSES it: "A sender MUST NOT add an `encoding` tag for + // any event shape in this document." Byte encoding is fixed per + // field, so a negotiated one only gives a receiver a way to be + // steered into a different parse of the same bytes. + assertNull( + rumor.tags.find { it.isNotEmpty() && it[0] == "encoding" }, + "transports/nostr.md: a kind:444 rumor MUST NOT carry an encoding tag", + ) val eTag = rumor.tags.find { it.isNotEmpty() && it[0] == "e" } assertNotNull(eTag, "MIP-02: rumor MUST carry [e, ]")