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 6b5b91062..04ff884a5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -2167,15 +2167,33 @@ class Account( * Publish or rotate KeyPackage events. */ suspend fun publishMarmotKeyPackages() { - val manager = marmotManager ?: return - if (!isWriteable()) return + val manager = + marmotManager ?: run { + Log.w("MarmotDbg") { "publishMarmotKeyPackages: marmotManager is NULL — no-op" } + return + } + if (!isWriteable()) { + Log.w("MarmotDbg") { "publishMarmotKeyPackages: account is not writeable — no-op" } + return + } val relays = keyPackagePublishRelays() + val needsRotation = manager.needsKeyPackageRotation() + Log.d("MarmotDbg") { + "publishMarmotKeyPackages: needsRotation=$needsRotation relays=${relays.size}" + } - if (manager.needsKeyPackageRotation()) { + if (needsRotation) { val rotatedEvents = manager.rotateConsumedKeyPackages(relays.toList()) + Log.d("MarmotDbg") { + "publishMarmotKeyPackages: rotateConsumedKeyPackages produced ${rotatedEvents.size} event(s)" + } rotatedEvents.forEach { event -> cache.justConsumeMyOwnEvent(event) + Log.d("MarmotDbg") { + "publishMarmotKeyPackages: publishing rotated kind:${event.kind} id=${event.id.take(8)}… " + + "→ ${relays.size} relay(s): ${relays.map { it.url }}" + } client.publish(event, relays) } } @@ -2313,12 +2331,30 @@ class Account( targetLeafIndex: Int, groupRelays: Set, ) { - val manager = marmotManager ?: return - if (!isWriteable()) return + Log.d("MarmotDbg") { + "removeMarmotGroupMember: group=${nostrGroupId.take(8)}… targetLeafIndex=$targetLeafIndex " + + "groupRelays=${groupRelays.size}" + } + val manager = + marmotManager ?: run { + Log.w("MarmotDbg") { "removeMarmotGroupMember: marmotManager is NULL — no-op" } + return + } + if (!isWriteable()) { + Log.w("MarmotDbg") { "removeMarmotGroupMember: account is not writeable — no-op" } + return + } val outbound = manager.removeMember(nostrGroupId, targetLeafIndex) + Log.d("MarmotDbg") { + "removeMarmotGroupMember: built commit kind=${outbound.signedEvent.kind} id=${outbound.signedEvent.id.take(8)}…" + } val chatroom = marmotGroupList.getOrCreateGroup(nostrGroupId) manager.syncMetadataTo(nostrGroupId, chatroom) + Log.d("MarmotDbg") { + "removeMarmotGroupMember: publishing commit id=${outbound.signedEvent.id.take(8)}… " + + "to ${groupRelays.size} relay(s): ${groupRelays.map { it.url }}" + } client.publish(outbound.signedEvent, groupRelays) } @@ -3075,11 +3111,14 @@ class Account( val innerEvent = com.vitorpamplona.quartz.nip01Core.core.Event .fromJson(json) - // wasVerified=true: these events were already verified - // via MLS credential identity when first decrypted (see - // GroupEventHandler). Persisted inner events are - // unsigned per MIP-03, so justVerify would fail and - // reactions/deletions would silently drop here. + // wasVerified=true: MIP-03 inner events are + // unsigned rumors (empty sig), authenticated + // via the MLS credential-identity check in + // GroupEventHandler when first decrypted. + // Running Nostr sig verify here (justVerify + // via wasVerified=false) would silently drop + // kind:7 reactions / kind:5 deletions since + // they never carry a Schnorr signature. val isNew = cache.justConsume(innerEvent, null, true) val innerNote = cache.getOrCreateNote(innerEvent.id) if (isNew) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 371b73ea7..c47d64d38 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -1477,7 +1477,16 @@ class AccountViewModel( alt(caption) } } - val innerEvent = account.signer.sign(template) + // MIP-03: inner events MUST remain unsigned (no `sig`) so a leaked + // plaintext can't be replayed as a valid public kind:9. Authorship + // is authenticated by the MLS sender's LeafNode + the pubkey↔ + // credential-identity equality check on the receive side. + val innerEvent = + com.vitorpamplona.quartz.nip59Giftwrap.rumors.RumorAssembler + .assembleRumor( + account.signer.pubKey, + template, + ) val relays = marmotGroupRelays(nostrGroupId) account.sendMarmotGroupMessage(nostrGroupId, innerEvent, relays) } 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 6226c6fd8..b1ab71975 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 @@ -50,6 +50,8 @@ import com.vitorpamplona.quartz.nipACWebRtcCalls.events.CallRejectEvent import com.vitorpamplona.quartz.nipACWebRtcCalls.events.CallRenegotiateEvent import com.vitorpamplona.quartz.utils.Log import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock class EventProcessor( private val account: Account, @@ -549,6 +551,29 @@ class GroupEventHandler( private val account: Account, private val cache: LocalCache, ) : EventHandler { + /** + * Per-group buffer of kind:445 events whose outer ChaCha20-Poly1305 + * layer couldn't be decrypted yet — typically because they carry an + * epoch the receiver hasn't advanced to. The relay subscription + * doesn't guarantee arrival order, so a run like + * + * 1) B sends msg-1..5 at epoch 1 + * 2) B adds C (commit advances B → epoch 2) + * 3) B sends msg-6..8 at epoch 2 + * 4) B renames (commit advances B → epoch 3) + * + * can reach a receiver in the order {msg-6, msg-7, msg-8, rename, + * add-C, msg-1..5} depending on relay ordering. Everything from step + * 3+ above is `UndecryptableOuterLayer` until the add-C commit lands; + * we re-run those pending events every time an epoch-advancing + * CommitProcessed fires for the same group. + * + * Bounded per-group so a stuck stream of "truly undecryptable" + * pre-join events can't grow unbounded. FIFO eviction on overflow. + */ + private val pendingUndecryptable: MutableMap> = mutableMapOf() + private val pendingMutex = Mutex() + override suspend fun add( event: GroupEvent, eventNote: Note, @@ -592,28 +617,46 @@ class GroupEventHandler( "GroupEventHandler.add: ApplicationMessage decrypted innerKind=${innerEvent.kind} " + "innerId=${innerEvent.id.take(8)}… author=${innerEvent.pubKey.take(8)}…" } - // wasVerified=true: MIP-03 inner events are unsigned (sig is empty - // or placeholder), so justVerify() would fail. Authenticity is - // already guaranteed by the MLS layer — MarmotInboundProcessor - // enforces innerEvent.pubKey == MLS sender credential identity - // before returning ApplicationMessage. Without this, side-channel - // inner events from other Marmot clients (kind:7 reactions, - // kind:5 deletions) are silently dropped by LocalCache, so a - // WhiteNoise reaction never attaches to its target chat bubble. - if (cache.justConsume(innerEvent, null, true)) { - val innerNote = cache.getOrCreateNote(innerEvent.id) + // wasVerified=true: MIP-03 inner events are unsigned + // rumors (empty sig); LocalCache.justVerify would reject + // every one on Schnorr verification. Authenticity is + // already guaranteed by MLS — MarmotInboundProcessor + // enforces innerEvent.pubKey == MLS sender's credential + // identity before returning ApplicationMessage. Without + // this, wn-signed kind:9 chat messages, kind:7 reactions, + // and kind:5 deletions would all silently drop here. + // + // Always call marmotGroupList.addMessage regardless of + // justConsume's return value. `addMessageSync` dedupes + // by Note identity, so a duplicate is a cheap no-op — + // but a previous-session persist that hydrated the + // cache at startup without populating the chatroom + // needs this path to surface the note (otherwise the + // operator saw nothing but a misleading "inner event + // already in cache" log and the message never rendered). + val isNew = cache.justConsume(innerEvent, null, true) + val innerNote = cache.getOrCreateNote(innerEvent.id) + if (isNew) { innerNote.event = innerEvent - - // Track the message in the Marmot group chatroom - account.marmotGroupList.addMessage(result.groupId, innerNote) - - // Persist the decrypted plaintext so the message - // survives an app restart. Marmot/MLS application - // messages cannot be re-decrypted once the ratchet - // has advanced, so we must capture them here. - manager.persistDecryptedMessage(result.groupId, result.innerEventJson) } else { - Log.d("MarmotDbg") { "GroupEventHandler.add: inner event already in cache (duplicate)" } + Log.d("MarmotDbg") { + "GroupEventHandler.add: inner event already in cache — surfacing in chatroom anyway" + } + } + + // Track the message in the Marmot group chatroom + account.marmotGroupList.addMessage(result.groupId, innerNote) + + // Persist the decrypted plaintext so the message + // survives an app restart. Marmot/MLS application + // messages cannot be re-decrypted once the ratchet + // has advanced, so we must capture them here. Skip + // when the cache reported duplicate — the message + // was already persisted on the run that originally + // decrypted it (the message store appends, so a + // re-persist would silently grow the on-disk log). + if (isNew) { + manager.persistDecryptedMessage(result.groupId, result.innerEventJson) } } @@ -624,6 +667,10 @@ 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) + // Epoch just advanced — drain any kind:445 events that + // previously failed as UndecryptableOuterLayer for this + // group. See `pendingUndecryptable` for the scenario. + retryPendingFor(result.groupId, eventNote, publicNote) } is GroupEventResult.CommitPending -> { @@ -637,12 +684,31 @@ class GroupEventHandler( } is GroupEventResult.UndecryptableOuterLayer -> { - // Expected for commits + application messages from epochs - // that predate our join — per MLS forward secrecy we - // never held those keys. Not a bug, not a warning. + // Two common causes for this result: + // (a) event's epoch predates our join — we never held + // those exporter keys, nothing to retry later. + // (b) event's epoch is ahead of our current one — the + // epoch-advancing commit for this group hasn't + // arrived yet but will in a moment (relays don't + // guarantee arrival order across a kind:445 + // subscription). We can't tell (a) from (b) here, + // so buffer the event and retry after any future + // CommitProcessed for this group advances us. + // + // Bounded per-group: keep the most recent + // MAX_PENDING_PER_GROUP events, FIFO-evict on overflow. + // Prevents a flood of genuinely pre-join events from + // pinning unbounded memory. Log.d("MarmotDbg") { "GroupEventHandler.add: undecryptable outer layer for group=${result.groupId.take(8)}… " + - "(current + ${result.retainedEpochCount} retained epoch key(s) tried) — likely from before our join" + "(current + ${result.retainedEpochCount} retained epoch key(s) tried) — buffering for post-commit retry" + } + pendingMutex.withLock { + val queue = pendingUndecryptable.getOrPut(result.groupId) { ArrayDeque() } + if (queue.size >= MAX_PENDING_PER_GROUP) { + queue.removeFirst() + } + queue.addLast(event) } } @@ -655,4 +721,37 @@ class GroupEventHandler( Log.e("MarmotDbg", "GroupEventHandler.add: exception processing kind:445", e) } } + + /** + * Drain the pending-undecryptable queue for [groupId] and re-run each + * event through `add`. A successful retry may itself emit another + * CommitProcessed and trigger recursive draining — that's fine, MLS + * epoch advances are strictly monotonic so recursion depth is bounded + * by the number of queued events. + * + * `eventNote` / `publicNote` are re-used for the retry path; they only + * feed into relay-activity tracking and compose-note lookup, both of + * which are safe to re-fire with the triggering event's notes. + */ + private suspend fun retryPendingFor( + groupId: String, + eventNote: Note, + publicNote: Note, + ) { + val drained = + pendingMutex.withLock { + pendingUndecryptable.remove(groupId)?.toList() ?: return + } + if (drained.isEmpty()) return + Log.d("MarmotDbg") { + "GroupEventHandler.retryPendingFor: replaying ${drained.size} previously-undecryptable kind:445 event(s) for group=${groupId.take(8)}…" + } + for (pending in drained) { + add(pending, eventNote, publicNote) + } + } + + companion object { + private const val MAX_PENDING_PER_GROUP = 64 + } } 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 d8a509ab8..c1fc14d85 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 @@ -155,17 +155,25 @@ class MarmotManager( ): OutboundGroupEvent = outboundProcessor.buildGroupEvent(nostrGroupId, innerEvent) /** - * Build a kind:9 chat-message GroupEvent from plain text. The inner event is - * signed with this manager's signer and optionally persisted to the local - * decrypted-message log so `loadStoredMessages` reflects our own outbound - * immediately (without waiting for relay loopback). + * Build a kind:9 chat-message GroupEvent from plain text. The inner + * event is built as an UNSIGNED rumor per MIP-03 ("Inner events MUST + * remain unsigned — this ensures leaked events cannot be published to + * public relays"): the MLS sender authenticates via the LeafNode + * credential + the `pubkey` ↔ sender-identity equality check, so + * the inner Nostr signature is redundant, and leaving it in would + * let a leaked plaintext be replayed as a valid public kind:9. * - * Platform callers that already maintain their own "own event" cache (i.e. - * Amethyst's `LocalCache.justConsumeMyOwnEvent`) should pass `persistOwn = false`. - * Headless callers (CLI) should leave it at the default. + * Optionally persisted to the local decrypted-message log so + * `loadStoredMessages` reflects our own outbound immediately + * (without waiting for relay loopback). + * + * Platform callers that already maintain their own "own event" cache + * (i.e. Amethyst's `LocalCache.justConsumeMyOwnEvent`) should pass + * `persistOwn = false`. Headless callers (CLI) should leave it at + * the default. * * @return the signed kind:445 outer event together with the inner kind:9 - * event id, so the caller can reference it for replies/reactions. + * rumor id, so the caller can reference it for replies/reactions. */ suspend fun buildTextMessage( nostrGroupId: HexKey, @@ -175,7 +183,9 @@ class MarmotManager( val template = com.vitorpamplona.quartz.nip01Core.signers .eventTemplate(kind = 9, description = text) - val innerEvent = signer.sign(template) + val innerEvent = + com.vitorpamplona.quartz.nip59Giftwrap.rumors.RumorAssembler + .assembleRumor(signer.pubKey, template) val outbound = buildGroupMessage(nostrGroupId, innerEvent) if (persistOwn) persistDecryptedMessage(nostrGroupId, innerEvent.toJson()) return TextMessageBundle(outbound = outbound, innerEvent = innerEvent) @@ -566,9 +576,14 @@ class MarmotManager( chatroom.adminPubkeys.value = metadata.adminPubkeys chatroom.relays.value = metadata.relays } + val previousCount = chatroom.members.value.size val members = memberPubkeys(nostrGroupId) chatroom.members.value = members chatroom.memberCount.value = members.size + Log.d("MarmotDbg") { + "syncMetadataTo: group=${nostrGroupId.take(8)}… members $previousCount→${members.size} " + + "(leafs=${members.map { it.leafIndex }})" + } } } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroup.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroup.kt index 3316be1d2..bdf98aba5 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroup.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroup.kt @@ -488,29 +488,50 @@ class MlsGroup private constructor( // the decryption into `UpdatePathError(UnableToDecrypt)`. val updatePath: UpdatePath? = if (needsPath && pathSecrets.isNotEmpty()) { - val copath = BinaryTree.copath(myLeafIndex, tree.leafCount) + // RFC 9420 §7.9: UpdatePath carries one node per entry in the + // **filtered** direct path — parents whose copath subtree has + // empty resolution are omitted (encrypting to them is + // equivalent to encrypting to their only non-blank child). + // Sender and receiver must agree on filtering or the + // UpdatePath length check fails and openmls/wn reject the + // commit with "UpdatePath node count doesn't match". + val (filteredDp, filteredCp) = tree.filteredDirectPath(myLeafIndex) + val directPath = BinaryTree.directPath(myLeafIndex, tree.leafCount) + + // path_secrets are derived one per UNFILTERED level so the + // KDF chain reaches the root regardless of filtering + // (commit_secret = DeriveSecret(root_path_secret, "path") + // must be computable even when the root is in a filtered-out + // position). Select the subset aligned to the filtered + // direct path for UpdatePath node generation. + val filteredPathSecrets = + directPath.indices.mapNotNull { i -> + val idxInFiltered = filteredDp.indexOf(directPath[i]) + if (idxInFiltered >= 0) pathSecrets[i] else null + } // Capture sibling tree hashes BEFORE applying the UpdatePath — // parent_hash computation (RFC 9420 §7.9.2) uses the - // ORIGINAL sibling-subtree tree hashes. + // ORIGINAL sibling-subtree tree hashes. Only needed at the + // filtered positions (those are the nodes whose parent_hash + // we're going to compute). val preUpdateSiblingHashes = capturePreUpdateSiblingHashes(myLeafIndex) // Stage path-keys into the tree so subsequent parent_hash / // tree_hash computations reflect the new keys. We'll fill in // the HPKE-encrypted secrets once we know the post-commit - // context bytes. + // context bytes. One staged node per FILTERED level. val stagedPathNodes = - pathSecrets.zip(copath).map { (pathKey, _) -> + filteredPathSecrets.map { pathKey -> UpdatePathNode(pathKey.publicKey, emptyList()) } tree.applyUpdatePath(myLeafIndex, stagedPathNodes) - // Compute parent_hash for every direct-path parent node and - // for the committer's leaf (RFC 9420 §7.9.2). + // Compute parent_hash for every FILTERED-direct-path parent + // node and for the committer's leaf (RFC 9420 §7.9.2). val (parentNodeHashes, leafParentHash) = computeSenderParentHashes(myLeafIndex, preUpdateSiblingHashes) - val directPath = BinaryTree.directPath(myLeafIndex, tree.leafCount) - for (nodeIdx in directPath) { + for (nodeIdx in filteredDp) { val existing = tree.getNode(nodeIdx) if (existing is com.vitorpamplona.quartz.marmot.mls.tree.TreeNode.Parent) { tree.setParent( @@ -559,7 +580,7 @@ class MlsGroup private constructor( ).toTlsBytes() val pathNodes = - stagedPathNodes.zip(copath).zip(pathSecrets) { (staged, copathNode), pathKey -> + stagedPathNodes.zip(filteredCp).zip(filteredPathSecrets) { (staged, copathNode), pathKey -> val resolution = tree.resolution(copathNode).filterNot { resNode -> BinaryTree.isLeaf(resNode) && @@ -1291,12 +1312,15 @@ class MlsGroup private constructor( } // After verification, patch the computed parent_hash values into - // the direct-path parent nodes. The sender's tree has these + // the FILTERED-direct-path parent nodes. The sender's tree has these // values filled in; receivers must match for treeHash() to agree - // (and thus for the epoch key schedule to converge). + // (and thus for the epoch key schedule to converge). Unfiltered + // parents have been blanked by the proposal or were never on the + // chain — skip them. val (recvParentHashes, _) = computeSenderParentHashes(senderLeafIndex, preUpdateSiblingHashes) - for (nodeIdx in BinaryTree.directPath(senderLeafIndex, tree.leafCount)) { + val (filteredDp, filteredCp) = tree.filteredDirectPath(senderLeafIndex) + for (nodeIdx in filteredDp) { val existing = tree.getNode(nodeIdx) if (existing is com.vitorpamplona.quartz.marmot.mls.tree.TreeNode.Parent) { tree.setParent( @@ -1308,8 +1332,15 @@ class MlsGroup private constructor( } } - // Decrypt path secret from our copath node - val directPath = BinaryTree.directPath(senderLeafIndex, tree.leafCount) + // Decrypt path secret from our copath node. + // + // RFC 9420 §7.9: UpdatePath nodes align to the sender's FILTERED + // direct path, so `commonAncestorIdx` must be computed against + // that filtered list (not the unfiltered directPath). Using the + // unfiltered index picks the wrong `updatePath.nodes[i]` and + // HPKE-decrypt fails silently — causing `ConfirmationTagMismatch` + // at best, or a completely wrong commit_secret at worst. + val unfilteredDirectPath = BinaryTree.directPath(senderLeafIndex, tree.leafCount) val myPath = BinaryTree.directPath(myLeafIndex, tree.leafCount) // Path-decryption context (RFC 9420 §7.6): matches what the @@ -1322,11 +1353,19 @@ class MlsGroup private constructor( treeHash = tree.treeHash(), ).toTlsBytes() - // Find the common ancestor - val commonAncestorIdx = directPath.indexOfFirst { it in myPath } - if (commonAncestorIdx >= 0 && commonAncestorIdx < updatePath.nodes.size) { - val pathNode = updatePath.nodes[commonAncestorIdx] - val copathNodeIdx = BinaryTree.copath(senderLeafIndex, tree.leafCount)[commonAncestorIdx] + // Find the unfiltered common-ancestor index (we need this for the + // KDF step count: commit_secret = DeriveSecret(path_secret[root]) + // requires walking one KDF step per unfiltered level from the + // common ancestor to the root). + val commonAncestorUnfilteredIdx = unfilteredDirectPath.indexOfFirst { it in myPath } + // Find the filtered common-ancestor index (for picking the right + // UpdatePath node + copath resolution). + val commonAncestorNode = + if (commonAncestorUnfilteredIdx >= 0) unfilteredDirectPath[commonAncestorUnfilteredIdx] else -1 + val commonAncestorFilteredIdx = filteredDp.indexOf(commonAncestorNode) + if (commonAncestorFilteredIdx >= 0 && commonAncestorFilteredIdx < updatePath.nodes.size) { + val pathNode = updatePath.nodes[commonAncestorFilteredIdx] + val copathNodeIdx = filteredCp[commonAncestorFilteredIdx] val resolution = tree.resolution(copathNodeIdx).filterNot { resNode -> BinaryTree.isLeaf(resNode) && @@ -1354,7 +1393,13 @@ class MlsGroup private constructor( // advances one step past the root; quartz was stopping at the root // and diverging — that's what caused `ConfirmationTagMismatch` on // every cross-impl commit. - val stepsToRoot = directPath.size - commonAncestorIdx - 1 + // + // Step count is measured against the UNFILTERED direct + // path — the path_secret chain advances one KDF step per + // level regardless of whether that level emits a + // UpdatePath node, so filtering changes which nodes carry + // ciphertext but not the number of KDF steps. + val stepsToRoot = unfilteredDirectPath.size - commonAncestorUnfilteredIdx - 1 var currentSecret = pathSecret repeat(stepsToRoot) { currentSecret = MlsCryptoProvider.deriveSecret(currentSecret, "path") @@ -1568,30 +1613,40 @@ class MlsGroup private constructor( senderLeafIndex: Int, preUpdateSiblingHashes: Map, ): Pair, ByteArray> { - val directPath = BinaryTree.directPath(senderLeafIndex, tree.leafCount) - if (directPath.isEmpty()) return Pair(emptyMap(), ByteArray(0)) + // RFC 9420 §7.9.2: parent_hash chain walks the FILTERED direct path. + // A filtered-out parent is never on the chain because its copath + // subtree is blank — there's no sibling tree hash to fold into the + // next hop. Using the unfiltered path here produces a chain that + // doesn't match what openmls/wn compute on the other side. + val (filteredDp, _) = tree.filteredDirectPath(senderLeafIndex) + val unfilteredDp = BinaryTree.directPath(senderLeafIndex, tree.leafCount) + if (filteredDp.isEmpty()) return Pair(emptyMap(), ByteArray(0)) + + // preUpdateSiblingHashes is indexed by level in the UNFILTERED path — + // map each filtered node to its unfiltered level so we pick the + // correct sibling tree hash. + val filteredLevels = filteredDp.map { unfilteredDp.indexOf(it) } val hashes = mutableMapOf() // Root's parent_hash is empty by convention (RFC 9420 §7.9.2). - hashes[directPath.last()] = ByteArray(0) + hashes[filteredDp.last()] = ByteArray(0) // Walk top-down (root has no parent → already set). For each node - // X below root, parent_hash(X) = Hash(encode(ParentHashInput{ - // encryption_key = parent(X).encryption_key, - // parent_hash = parent(X).parent_hash, - // original_sibling_tree_hash = tree_hash(sibling(X))_preUpdate, - // })). - for (i in directPath.size - 2 downTo 0) { - val xIdx = directPath[i] - val parentIdx = directPath[i + 1] + // X below root on the filtered path, parent_hash(X) is computed + // with parent(X)'s fields, where "parent" is the NEXT entry on the + // filtered direct path. + for (i in filteredDp.size - 2 downTo 0) { + val xIdx = filteredDp[i] + val parentIdx = filteredDp[i + 1] + val parentLevel = filteredLevels[i + 1] val parentNode = tree.getNode(parentIdx) if (parentNode !is com.vitorpamplona.quartz.marmot.mls.tree.TreeNode.Parent) { hashes[xIdx] = ByteArray(0) continue } val siblingTreeHash = - preUpdateSiblingHashes[i + 1] - ?: error("missing pre-update sibling tree hash at level ${i + 1}") + preUpdateSiblingHashes[parentLevel] + ?: error("missing pre-update sibling tree hash at level $parentLevel") hashes[xIdx] = MlsCryptoProvider.hash( encodeParentHashInput( @@ -1603,14 +1658,15 @@ class MlsGroup private constructor( } // The committer's leaf carries parent_hash(parent(leaf)) = parent_hash - // computed with the immediate parent's (directPath[0]) fields. - val immediateParentIdx = directPath.first() + // computed with the immediate parent's (filteredDp[0]) fields. + val immediateParentIdx = filteredDp.first() + val immediateParentLevel = filteredLevels.first() val immediateParent = tree.getNode(immediateParentIdx) val leafParentHash = if (immediateParent is com.vitorpamplona.quartz.marmot.mls.tree.TreeNode.Parent) { val siblingTreeHash = - preUpdateSiblingHashes[0] - ?: error("missing pre-update sibling tree hash at leaf level") + preUpdateSiblingHashes[immediateParentLevel] + ?: error("missing pre-update sibling tree hash at level $immediateParentLevel") MlsCryptoProvider.hash( encodeParentHashInput( encryptionKey = immediateParent.parentNode.encryptionKey, @@ -1676,8 +1732,12 @@ class MlsGroup private constructor( updatePath: UpdatePath, preUpdateSiblingHashes: Map, ): Boolean { - val directPath = BinaryTree.directPath(senderLeafIndex, tree.leafCount) - if (directPath.isEmpty()) return true + // Filtered direct path drives the parent_hash chain — a sender + // with an empty filtered direct path has no parents to hash so + // the leaf's parent_hash field should be an empty byte string + // and we can short-circuit verification. + val (filteredDp, _) = tree.filteredDirectPath(senderLeafIndex) + if (filteredDp.isEmpty()) return true // RFC 9420 §7.9.2: compute what the sender's parent_hash chain // SHOULD have been given the post-update tree state, then compare diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroupManager.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroupManager.kt index ca5f592c5..cc7ef8e2f 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroupManager.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroupManager.kt @@ -660,8 +660,18 @@ class MlsGroupManager( if (privMsg.epoch != retained.epoch) return null // Derive sender data key/nonce using ciphertext sample (RFC 9420 §6.3.1) + // RFC 9420 §6.3.2: ciphertext_sample is the first KDF.Nh bytes + // (32 for HKDF-SHA256), not AEAD.Nk (16). Using AEAD.Nk here made + // sender-data decryption silently fail for every retained-epoch + // message and turned the fallback path into a no-op — the + // symptom was interop Test 12 (offline catch-up): kind:9 + // messages encrypted under epoch N arriving after a 1→N+1 + // commit got rejected with "Message epoch X doesn't match + // current epoch Y" instead of being pulled through this path. + // Same fix already applied to MlsGroup.decrypt (line ~878); + // this branch was missed when that one was patched. val ciphertextSample = - privMsg.ciphertext.copyOfRange(0, minOf(privMsg.ciphertext.size, MlsCryptoProvider.AEAD_KEY_LENGTH)) + privMsg.ciphertext.copyOfRange(0, minOf(privMsg.ciphertext.size, MlsCryptoProvider.HASH_OUTPUT_LENGTH)) val senderDataKey = MlsCryptoProvider.expandWithLabel( retained.senderDataSecret, @@ -710,13 +720,23 @@ class MlsGroupManager( contentAad.putUint8(privMsg.contentType.value) contentAad.putOpaqueVarInt(privMsg.authenticatedData) - val plaintext = + val pmcBytes = MlsCryptoProvider.aeadDecrypt(kng.key, guardedNonce, contentAad.toByteArray(), privMsg.ciphertext) + // AEAD plaintext is a PrivateMessageContent struct (RFC 9420 + // §6.3.1): `applicationData || signature || padding`. The + // main decrypt path parses this and returns the inner + // applicationData; the retained-epoch branch was returning the + // raw struct, which made callers see a length-prefixed blob + // with signature + zero-padding glued onto the end. Extract the + // applicationData the same way. + val pmcReader = TlsReader(pmcBytes) + val applicationData = pmcReader.readOpaqueVarInt() + DecryptedMessage( senderLeafIndex = senderLeafIndex, contentType = privMsg.contentType, - content = plaintext, + content = applicationData, epoch = privMsg.epoch, ) } catch (_: Exception) { diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/tree/RatchetTree.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/tree/RatchetTree.kt index 39f70a786..77aba3bfe 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/tree/RatchetTree.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/tree/RatchetTree.kt @@ -266,22 +266,51 @@ class RatchetTree( return MlsCryptoProvider.hash(writer.toByteArray()) } + /** + * RFC 9420 §4.1.2 "filtered direct path": + * the direct path of a leaf node L, with any parent node removed whose + * child on the copath of L has an empty resolution (unmerged_leaves + * count toward the resolution). + * + * Returns parallel lists (filteredDirectPath, filteredCopath). This is + * what openmls / whitenoise-rs use for `UpdatePath.nodes` generation + * and application — omitting a parent whose copath subtree is entirely + * blank is spec-required because encrypting to that parent's key pair + * is equivalent to encrypting to its only non-blank child (which is + * already on the direct path). Relevant in, e.g., a 3-member group + * where the committer removes the middle leaf: the sibling subtree at + * the level above the committer becomes fully blank, and the parent at + * that level is dropped from the UpdatePath. + */ + fun filteredDirectPath(leafIndex: Int): Pair, List> { + val directPath = BinaryTree.directPath(leafIndex, _leafCount) + val copath = BinaryTree.copath(leafIndex, _leafCount) + val filteredDp = mutableListOf() + val filteredCp = mutableListOf() + for (i in directPath.indices) { + if (resolution(copath[i]).isNotEmpty()) { + filteredDp.add(directPath[i]) + filteredCp.add(copath[i]) + } + } + return filteredDp to filteredCp + } + /** * Apply an UpdatePath to the tree: update parent nodes along the sender's - * direct path with the provided path nodes. + * **filtered** direct path (RFC 9420 §7.9) with the provided path nodes. */ fun applyUpdatePath( senderLeafIndex: Int, pathNodes: List, ) { - val directPath = BinaryTree.directPath(senderLeafIndex, _leafCount) - - require(pathNodes.size == directPath.size) { - "UpdatePath node count (${pathNodes.size}) doesn't match direct path length (${directPath.size})" + val (filteredDp, _) = filteredDirectPath(senderLeafIndex) + require(pathNodes.size == filteredDp.size) { + "UpdatePath node count (${pathNodes.size}) doesn't match filtered direct path length (${filteredDp.size})" } - for (i in directPath.indices) { - val nodeIdx = directPath[i] + for (i in filteredDp.indices) { + val nodeIdx = filteredDp[i] val pathNode = pathNodes[i] setParent( diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt index ebfa6775d..9727705f2 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt @@ -158,6 +158,84 @@ class MarmotPipelineTest { } } + /** + * Regression: the interop harness's Test 12 (offline catch-up) drove + * kind:9 application messages encrypted under epoch N into an Amethyst + * receiver that had already processed a 1→N+1 commit (e.g. add-member). + * Those epoch-N messages ought to decrypt through the retained-epoch + * fallback in [MlsGroupManager.decrypt], but the fallback was silently + * returning null because `tryDecryptWithRetainedEpoch` derived the + * sender-data sample using `AEAD_KEY_LENGTH` (16) instead of + * `HASH_OUTPUT_LENGTH` (32) — the same bug that was fixed on the + * main decrypt path earlier but never propagated to the retained + * branch. This test reproduces the flow and asserts the retained + * path actually decrypts. + */ + @Test + fun testDecryptRetainedEpoch_ApplicationMessageAfterCommit() { + runBlocking { + val aliceMgr = createGroupManager() + val bobMgr = createGroupManager() + + // 32-byte identities so they fit the MarmotGroupData 32-byte + // admin-pubkey slots. + val aliceId = ByteArray(32) { 0xA1.toByte() } + val bobId = ByteArray(32) { 0xB2.toByte() } + val carolId = ByteArray(32) { 0xC3.toByte() } + + aliceMgr.createGroup(groupId, aliceId) + // Install the MarmotGroupData extension so Welcome messages carry + // the NostrGroupData (MlsGroupManager.processWelcome requires it). + aliceMgr.updateGroupExtensions( + nostrGroupId = groupId, + extensions = + listOf( + com.vitorpamplona.quartz.marmot.mip01Groups + .MarmotGroupData( + nostrGroupId = groupId, + adminPubkeys = listOf(aliceId.toHexKey()), + ).toExtension(), + ), + ) + + // Bob joins at epoch 2 (epoch 0 = create, epoch 1 = GCE above, + // epoch 2 = add Bob). + val aliceGroup = aliceMgr.getGroup(groupId)!! + val bobBundle = aliceGroup.createKeyPackage(bobId, ByteArray(0)) + val addBob = aliceMgr.addMember(groupId, bobBundle.keyPackage.toTlsBytes()) + bobMgr.processWelcome(addBob.welcomeBytes!!, bobBundle) + val bobJoinEpoch = aliceMgr.getGroup(groupId)!!.epoch + assertEquals(bobJoinEpoch, bobMgr.getGroup(groupId)!!.epoch, "bob must be at alice's epoch after Welcome") + + // Bob encrypts a message at his current epoch — this would be + // held by a lossy relay and only reach Alice after she's + // already processed the next commit. + val bobStaleMsg = bobMgr.encrypt(groupId, "bob's offline epoch-N message".encodeToByteArray()) + + // Alice adds Carol, advancing to epoch N+1. Bob's earlier + // message is now out-of-order relative to Alice's tree state. + val carolBundle = aliceGroup.createKeyPackage(carolId, ByteArray(0)) + aliceMgr.addMember(groupId, carolBundle.keyPackage.toTlsBytes()) + assertEquals( + bobJoinEpoch + 1, + aliceMgr.getGroup(groupId)!!.epoch, + "alice must have advanced one epoch past bob's encrypt time", + ) + + // Bob's stale message now arrives at Alice. The primary decrypt + // path throws "Message epoch X doesn't match current epoch X+1"; + // the retained-epoch fallback inside MlsGroupManager.decrypt + // must pick it up. + val decrypted = aliceMgr.decrypt(groupId, bobStaleMsg) + assertEquals( + "bob's offline epoch-N message", + decrypted.content.decodeToString(), + "Alice must decrypt Bob's pre-commit message via retained-epoch fallback", + ) + assertEquals(bobJoinEpoch, decrypted.epoch, "message was encrypted at bob's join epoch") + } + } + @Test fun testInboundRejectsNonMemberGroup() { runBlocking { diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt index 0240d2cec..6609561c9 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt @@ -354,6 +354,66 @@ class MlsGroupLifecycleTest { assertEquals(bob2.leafIndex, aliceSees.senderLeafIndex) } + /** + * Regression: 3-member group, committer removes the MIDDLE leaf while the + * trailing leaf remains occupied. Interop harness Test 06 hit this with + * whitenoise-rs: the interop log showed quartz rejecting wn's commit + * with "Failed to apply commit: UpdatePath node count (1) doesn't match + * direct path length (2)". That error comes from + * `RatchetTree.applyUpdatePath` comparing the received node count against + * the CURRENT tree's direct-path length — the two sides disagree on the + * post-proposal leaf_count, so the path-length invariant breaks. + * + * Scenario Amethyst observed: + * Tree before: [B=leaf 0 (admin/committer), C=leaf 1, A=leaf 2] + * Commit: Remove(leaf 1) + * Tree after: [B=leaf 0, _blank_, A=leaf 2] (leaf 2 still occupied, + * so RFC §7.8 does NOT trim) + * + * Both sender and every receiver should end up at leaf_count=3 post- + * proposal with the sender's direct path at size 2 ([node 1, root 3]). + * If quartz's receiver processes the commit cleanly, the test passes. If + * this reproduction also reports "UpdatePath node count … doesn't match + * direct path length …", we've isolated the bug to the quartz side, not + * a wn-specific oddity. + */ + @Test + fun testRemoveMiddleLeaf_ReceiverAcceptsCommit() { + // Alice creates, adds Bob (→ leaf 1), adds Carol (→ leaf 2). + val alice = MlsGroup.create("alice".encodeToByteArray()) + + val bobBundle = createStandaloneKeyPackage("bob") + val addBobResult = alice.addMember(bobBundle.keyPackage.toTlsBytes()) + val bob = MlsGroup.processWelcome(addBobResult.welcomeBytes!!, bobBundle) + + val carolBundle = createStandaloneKeyPackage("carol") + val addCarolResult = alice.addMember(carolBundle.keyPackage.toTlsBytes()) + bob.processFramedCommit(addCarolResult.framedCommitBytes) + val carol = MlsGroup.processWelcome(addCarolResult.welcomeBytes!!, carolBundle) + + check(alice.memberCount == 3 && bob.memberCount == 3 && carol.memberCount == 3) { + "pre-condition: all three members must be present" + } + + // Alice removes Bob (leaf 1) — the MIDDLE leaf. Carol (leaf 2) stays, + // so trailing-blank trim does NOT fire and leaf_count stays at 3. + val removeResult = alice.removeMember(bob.leafIndex) + + // Carol, the remaining non-committer, must be able to apply the + // commit. This is where the interop regression surfaces. + carol.processFramedCommit(removeResult.framedCommitBytes) + + // After the Remove, Alice and Carol are the only members but the + // tree still has leaf_count=3 (leaf 1 blank, leaf 2 = Carol). + assertEquals(alice.epoch, carol.epoch, "Carol must be at Alice's epoch after remove") + assertEquals(2, alice.memberCount, "Alice: 2 active members after removing Bob") + assertEquals(2, carol.memberCount, "Carol: 2 active members after removing Bob") + + // Post-remove forward-secrecy round-trip: Alice sends, Carol decrypts. + val msg = "after removing bob".encodeToByteArray() + assertContentEquals(msg, carol.decrypt(alice.encrypt(msg)).content) + } + // ----------------------------------------------------------------------- // 8. Signing key rotation (Update proposal) // ----------------------------------------------------------------------- diff --git a/tools/marmot-interop/lib.sh b/tools/marmot-interop/lib.sh index 0196117b1..913c2d8f9 100644 --- a/tools/marmot-interop/lib.sh +++ b/tools/marmot-interop/lib.sh @@ -177,17 +177,50 @@ jq_group_id() { # ------- polling helpers ----------------------------------------------------- -# wait_for_invite -# Echoes the first new group_id that appears in `wn groups invites --json`. +# Snapshot currently-pending invites on as a comma-separated list of +# gids. Use this BEFORE triggering a publish that should produce a new +# welcome, then pass the result to `wait_for_invite` as the ignore list so +# a stale welcome left over from a previous harness run (wnd persists +# pending invites across restarts) doesn't register as the arrival we +# just triggered. +snapshot_invites() { + local who="$1" wnfn + if [[ "$who" == "B" ]]; then wnfn=wn_b + else wnfn=wn_c; fi + local raw + raw=$("$wnfn" --json groups invites 2>/dev/null || true) + # jq_group_id reads one invite at a time, so iterate the array in shell + # rather than trying to handle all shapes in a single jq expression. + local gids=() + while IFS= read -r one; do + [[ -z "$one" || "$one" == "null" ]] && continue + local g + g=$(printf '%s' "$one" | jq_group_id) + [[ -n "$g" ]] && gids+=("$g") + done < <(printf '%s' "$raw" | jq -c '(.result // .) | .[]?' 2>/dev/null) + # bash 3.2 (stock macOS) treats "${gids[*]}" on an empty array as an + # unbound reference under `set -u`, so guard the expansion. + if (( ${#gids[@]} > 0 )); then + local IFS=',' + printf '%s' "${gids[*]}" + fi +} + +# wait_for_invite [ignore-csv] +# Echoes the first pending group_id that isn't listed in +# (comma-separated hex gids). Use the ignore list to skip welcomes left +# pending by previous harness runs — otherwise `wait_for_invite` can +# fire on a stale invite and the caller ends up driving a group the +# publisher isn't a member of, which then looks like a kind:445 drop. # Returns 1 on timeout. # # Also prints a heartbeat every ~10s with: # - elapsed/remaining time -# - current count of pending welcomes (should be 0 until it isn't) +# - current count of pending welcomes (total, pre-filter) # - last relevant line of wnd stderr (grep for welcome/giftwrap/subscribe/error) # so a stalled poll gives the operator something to forward. wait_for_invite() { - local who="$1" timeout="${2:-60}" deadline gid start last_hb + local who="$1" timeout="${2:-60}" ignore_csv="${3:-}" deadline gid start last_hb local wnfn data_dir if [[ "$who" == "B" ]]; then wnfn=wn_b; data_dir="$B_DIR" else wnfn=wn_c; data_dir="$C_DIR"; fi @@ -198,27 +231,32 @@ wait_for_invite() { # Post-v0.2 `wn --json groups invites` returns `{"result": [...]}` # (older builds returned the bare array). Peel the wrapper when # present so a pending invite is actually detected. - gid=$("$wnfn" --json groups invites 2>/dev/null \ - | jq -c '(.result // .) | .[0] // empty' 2>/dev/null | jq_group_id || true) - if [[ -n "${gid:-}" ]]; then - printf '%s\n' "$gid" - return 0 - fi + local raw + raw=$("$wnfn" --json groups invites 2>/dev/null || true) + # Walk every pending invite (not just .[0]) so we skip past stales. + while IFS= read -r one; do + [[ -z "$one" || "$one" == "null" ]] && continue + gid=$(printf '%s' "$one" | jq_group_id) + [[ -z "$gid" ]] && continue + if [[ ",$ignore_csv," != *",$gid,"* ]]; then + printf '%s\n' "$gid" + return 0 + fi + done < <(printf '%s' "$raw" | jq -c '(.result // .) | .[]?' 2>/dev/null) # Heartbeat every ~10s. local now=$(date +%s) if (( now - last_hb >= 10 )); then local elapsed=$(( now - start )) remaining=$(( deadline - now )) local pending - pending=$("$wnfn" --json groups invites 2>/dev/null \ - | jq '(.result // .) | length' 2>/dev/null || echo "?") + pending=$(printf '%s' "$raw" | jq '(.result // .) | length' 2>/dev/null || echo "?") local recent="" if [[ -f "$data_dir/logs/stderr.log" ]]; then recent=$(tail -n 200 "$data_dir/logs/stderr.log" 2>/dev/null \ | grep -iE 'welcome|giftwrap|gift_wrap|mls|subscribe|1059|444|error|warn' \ | tail -n 1 || true) fi - info "$who wait_for_invite +${elapsed}s (remaining ${remaining}s) pending=$pending tail: ${recent:-}" + info "$who wait_for_invite +${elapsed}s (remaining ${remaining}s) pending=$pending ignoring=${ignore_csv:-} tail: ${recent:-}" last_hb=$now fi @@ -293,13 +331,56 @@ extract_pubkey() { if [[ -n "$v" ]]; then printf '%s' "$v"; return; fi } -# Best-effort npub -> hex conversion via `wn users show`. If the lookup fails -# (e.g. --json not supported or user not resolvable), returns the input -# unchanged — downstream comparisons tolerate either form. +# Pure-awk bech32 decoder for npub1... → 32-byte lowercase hex. Needed because +# `wn users show` only resolves identities the local daemon already knows +# about, so A's npub (the Amethyst account) can't be converted via wn — and a +# failed conversion leaks the raw npub into hex-equality checks like the +# member-list assertion in Test 02, which then fails with +# "member list missing npub1…". +bech32_decode_npub() { + local npub="$1" + [[ "$npub" == npub1* ]] || return 1 + local body="${npub#npub1}" + # Need at least 6 chars of checksum + some data. + (( ${#body} > 6 )) || return 1 + local hex + hex=$(awk -v data="$body" 'BEGIN { + charset = "qpzry9x8gf2tvdw0s3jn54khce6mua7l" + for (i = 0; i < 32; i++) map[substr(charset, i + 1, 1)] = i + data = substr(data, 1, length(data) - 6) + acc = 0; bits = 0; out = "" + for (i = 1; i <= length(data); i++) { + c = substr(data, i, 1) + if (!(c in map)) exit 1 + acc = acc * 32 + map[c] + bits += 5 + while (bits >= 8) { + bits -= 8 + pow = 1 + for (j = 0; j < bits; j++) pow *= 2 + byte = int(acc / pow) + acc -= byte * pow + out = out sprintf("%02x", byte) + } + } + print out + }' 2>/dev/null) || return 1 + # 32-byte x-only pubkey = 64 hex chars. + [[ ${#hex} -eq 64 ]] || return 1 + printf '%s' "$hex" +} + +# npub -> hex conversion. Tries `wn users show` first (handy when it works, +# since it also validates the npub against the daemon's view), then falls +# back to a local bech32 decode so unknown npubs still convert. Returns the +# input unchanged only if both paths fail, which should never happen for a +# well-formed npub. npub_to_hex() { local npub="$1" raw hex raw=$(wn_b --json users show "$npub" 2>/dev/null || true) - hex=$(printf '%s' "$raw" | jq -r '.pubkey // .public_key // .hex // empty' 2>/dev/null || true) + hex=$(printf '%s' "$raw" \ + | jq -r '(.result // .) | (.pubkey // .public_key // .hex // .[0]?.pubkey // .[0]?.public_key // empty)' \ + 2>/dev/null || true) if [[ -z "$hex" || "$hex" == "null" ]]; then # fallback to yaml-ish output raw=$(wn_b users show "$npub" 2>/dev/null || true) @@ -307,9 +388,13 @@ npub_to_hex() { fi if [[ -n "$hex" && "$hex" != "null" ]]; then printf '%s' "$hex" - else - printf '%s' "$npub" + return fi + if hex=$(bech32_decode_npub "$npub"); then + printf '%s' "$hex" + return + fi + printf '%s' "$npub" } # ------- state file ---------------------------------------------------------- @@ -424,6 +509,70 @@ Then paste /tmp/amethyst-marmot.log contents here so the failure report has both the Amethyst-side publish log and the wn-side subscription log ($note)." } +# Dump everything we know about an outbound kind:445 from a wn daemon, so +# when the operator reports "Amethyst didn't see it" we can tell them which +# side of the wire to investigate: +# +# - `messages list` output (did the daemon persist the message locally, +# i.e. is this a relay-reach issue, a membership issue, or a pure +# Amethyst-receive issue?) +# - `groups show` metadata (which MLS-extension relays is wn using for +# this group — these are the SAME relays Amethyst subscribes to, so if +# the lists don't match that's the bug) +# - live relay connection status +# +# Output is tee'd to stderr (live) and $LOG_FILE (forward). +dump_outbound_diagnostics() { + local who="$1" gid="$2" needle="${3:-}" tag="${4:-post-send}" + local wnfn + if [[ "$who" == "B" ]]; then wnfn=wn_b + else wnfn=wn_c; fi + + printf '\n%s%s---- [%s] %s outbound diagnostics (group %.8s…) ----%s\n' \ + "$C_BOLD" "$C_CYAN" "$tag" "$who" "$gid" "$C_RESET" >&2 + printf '\n==== [%s] %s outbound diagnostics (group %s) ====\n' "$tag" "$who" "$gid" >>"$LOG_FILE" + + step "$who local view of latest messages in group" + local list_raw + list_raw=$("$wnfn" --json messages list "$gid" --limit 5 2>/dev/null || true) + printf ' %s\n' "${list_raw:-}" >>"$LOG_FILE" + printf '%s' "$list_raw" \ + | jq -r '(.result // .) | .[]? | " \(.id // .event_id // "?") \((.content // .text // "") | tostring | .[0:80])"' 2>/dev/null \ + | tee -a "$LOG_FILE" >&2 || true + if [[ -n "$needle" ]]; then + if printf '%s' "$list_raw" | jq -e --arg n "$needle" \ + '(.result // .) | .[]? | select((.content // .text // "") | contains($n))' \ + >/dev/null 2>&1; then + info "$who local store contains \"$needle\" — send reached wn's own DB (kind:445 went out)" + else + warn "$who local store does NOT contain \"$needle\" — `messages send` never persisted; check stderr.log" + fi + fi + + step "$who MLS group metadata (relays here are what Amethyst subscribes on)" + local meta_raw + meta_raw=$("$wnfn" --json groups show "$gid" 2>/dev/null || true) + printf ' %s\n' "${meta_raw:-}" >>"$LOG_FILE" + local meta_relays + meta_relays=$(printf '%s' "$meta_raw" \ + | jq -r '(.result // .) | (.relays // .group.relays // [])[]? | (. | tostring)' 2>/dev/null \ + | paste -sd',' -) + info " group relays (MLS extension): ${meta_relays:-}" + local meta_name meta_epoch + meta_name=$(printf '%s' "$meta_raw" | jq -r '(.result // .) | (.name // .group.name // "?")' 2>/dev/null) + meta_epoch=$(printf '%s' "$meta_raw" | jq -r '(.result // .) | (.epoch // .group.epoch // "?")' 2>/dev/null) + info " group name: $meta_name epoch: $meta_epoch" + + step "$who relay connection status right now" + "$wnfn" --json relays list 2>/dev/null \ + | jq -r '(.result // .)[]? | " \(.url // .relay.url // "?") [\(.type // "?")] [\(.status // .connection_status // "?")]"' 2>/dev/null \ + | tee -a "$LOG_FILE" >&2 || true + + printf '%s%s---- end outbound diagnostics ----%s\n\n' \ + "$C_BOLD" "$C_CYAN" "$C_RESET" >&2 + printf '==== end [%s] %s outbound diagnostics ====\n\n' "$tag" "$who" >>"$LOG_FILE" +} + # ------- group cleanup ------------------------------------------------------- cleanup_group() { local gid="$1" who="${2:-B}" diff --git a/tools/marmot-interop/marmot-interop.sh b/tools/marmot-interop/marmot-interop.sh index 4674fbe0c..f8f7e4702 100755 --- a/tools/marmot-interop/marmot-interop.sh +++ b/tools/marmot-interop/marmot-interop.sh @@ -34,7 +34,13 @@ C_HEX="" A_NPUB="" A_HEX="" -DEFAULT_RELAYS=( "wss://relay.damus.io" "wss://nos.lol" "wss://relay.primal.net" ) +DEFAULT_RELAYS=( + "wss://relay.damus.io" + "wss://nos.lol" + "wss://relay.primal.net" + "wss://nostr.bitcoiner.social" + "wss://nostr.mom" +) USE_LOCAL_RELAYS=0 ENABLE_TRANSPONDER=0 NO_BUILD=0 @@ -249,6 +255,128 @@ prompt_for_a_npub() { [[ "$A_HEX" != "$A_NPUB" ]] && info "A hex: $A_HEX" } +# Trigger wn's on-demand discovery for A and dump what wn actually +# learned — kinds 10002 / 10050 / 10051 from A's configured relays. +# Called after prompt_for_a_npub so wn's SQLite cache is populated +# BEFORE the test suite starts, and so the operator can spot "wn sees +# nothing for A" failures up front instead of as a cryptic Test 03 +# timeout. +# +# Implementation: +# - `wn keys check ` calls `resolve_user_blocking` which issues +# a targeted fetch for kinds [0, 10002, 10050, 10051] on the +# discovery-plane relays. That's the only published side-effect +# way to populate wn's user_relays table (no public CLI reads +# another user's relay lists directly). +# - After the fetch, `sqlite3` is used to SELECT from `user_relays` +# joined against `users` + `relays`. If sqlite3 isn't installed +# the probe degrades to "trust it, go run the tests" with a warning. +discover_a_relays() { + banner "Discovering A's advertised relays via wn" + step "wn_b keys check \$A_NPUB — populates wn's user_relays cache for A" + local kp_raw kp_event_id + kp_raw=$(wn_b --json keys check "$A_NPUB" 2>>"$LOG_FILE" || true) + printf 'wn_b keys check A raw: %s\n' "$kp_raw" >>"$LOG_FILE" + kp_event_id=$(printf '%s' "$kp_raw" | jq -r '.result.event_id // .event_id // empty' 2>/dev/null || true) + if [[ -n "$kp_event_id" && "$kp_event_id" != "null" ]]; then + info "wn_b found A's KeyPackage (kind:30443) — discovery plane is working" + else + warn "wn_b could NOT find A's KeyPackage. wn is bootstrapped on ${DEFAULT_RELAYS[*]}." + warn "Either Amethyst never published a KeyPackage, or it's only on relays wn can't reach." + warn "All later tests will fail. Fix this before continuing (tap KP publish in Amethyst settings)." + fi + # Also prime wn_c so Tests 04+ (which use C as a third member) can + # target A without another round-trip later. + wn_c --json keys check "$A_NPUB" >>"$LOG_FILE" 2>&1 || true + + # Give wn ~3s to process the kind:10002/10050/10051 events that + # came back in the same fetch (they're not event_id-returned, they + # just land in user_relays via the subscription callback). + sleep 3 + + if ! command -v sqlite3 >/dev/null 2>&1; then + warn "sqlite3 not installed — skipping wn user_relays cache probe." + warn "If Test 03 fails with 'no invite arrived', install sqlite3 or rerun with --local-relays." + return + fi + + # wn stores its database under $data_dir/release/.sqlite. + # Find the DB file by looking for the only *.sqlite under release/. + local db + db=$(find "$B_DIR/release" -maxdepth 1 -name '*.sqlite' -print -quit 2>/dev/null || true) + if [[ -z "$db" || ! -s "$db" ]]; then + warn "wn SQLite DB not found under $B_DIR/release — cannot probe user_relays." + return + fi + + step "wn_b's cached view of A's relay lists (SELECT from user_relays)" + local rows + # wn's SQLite schema stores `users.pubkey` — we don't have a public + # CLI to confirm the column type, and in practice it's been observed + # stored as both BLOB (raw 32 bytes) and TEXT (lowercase hex) across + # versions. Try both forms so the probe doesn't falsely report "NO + # cached relay entries" when the cache is actually populated (symptom: + # this diagnostic warned loudly even though Tests 02/03/04/05 were + # delivering welcomes successfully — the welcomes were flowing, the + # probe was just querying the wrong column encoding). + rows=$(sqlite3 -readonly "$db" \ + "SELECT ur.relay_type, r.url FROM user_relays ur + JOIN users u ON u.id = ur.user_id + JOIN relays r ON r.id = ur.relay_id + WHERE u.pubkey = X'$A_HEX' + OR LOWER(CAST(u.pubkey AS TEXT)) = LOWER('$A_HEX') + ORDER BY ur.relay_type, r.url;" 2>>"$LOG_FILE" || true) + if [[ -z "$rows" ]]; then + warn "wn has NO cached relay entries for A ($A_HEX)." + warn " → Either Amethyst hasn't published kind:10002/10050/10051 to any relay wn_b" + warn " can reach, OR wn's SQLite schema changed pubkey column encoding again." + warn " → If later tests deliver welcomes successfully, the schema is the culprit;" + warn " grep the daemon's migration files for the users.pubkey column type." + return + fi + + # Collate by relay_type for readability. + local nip65 inbox kp + nip65=$(printf '%s\n' "$rows" | awk -F'|' '$1=="nip65"{print " "$2}') + inbox=$(printf '%s\n' "$rows" | awk -F'|' '$1=="inbox"{print " "$2}') + kp=$(printf '%s\n' "$rows" | awk -F'|' '$1=="key_package"{print " "$2}') + info "A's kind:10002 (nip65):" + printf '%s\n' "${nip65:- }" | tee -a "$LOG_FILE" >&2 + info "A's kind:10050 (DM inbox, used for Marmot welcome delivery):" + printf '%s\n' "${inbox:- }" | tee -a "$LOG_FILE" >&2 + info "A's kind:10051 (KeyPackage relays):" + printf '%s\n' "${kp:- }" | tee -a "$LOG_FILE" >&2 + + # Warn loudly if A's DM inbox is non-empty but shares no relay with + # wn_b's own configured set — that's the exact failure mode the + # operator already hit ("my DM inbox has auth.nostr1.com, wn can't + # publish there") and it silently breaks Test 03+. + if [[ -n "$inbox" ]]; then + local wn_relays + wn_relays=$(wn_b --json relays list 2>/dev/null \ + | jq -r '(.result // .)[]? | (.url // .relay.url // empty)' 2>/dev/null) + local overlap="" + while IFS= read -r a_url; do + a_url="${a_url# }" + [[ -z "$a_url" ]] && continue + if printf '%s\n' "$wn_relays" | grep -qxF "$a_url"; then + overlap+="$a_url," + fi + done <<< "$inbox" + overlap="${overlap%,}" + if [[ -z "$overlap" ]]; then + warn "A's DM inbox (kind:10050) shares ZERO relays with wn_b's bootstrap set." + warn " wn may still reach A's 10050 relays if they're public — but if any require" + warn " NIP-42 AUTH (auth.nostr1.com) or a whitelist (relay.0xchat.com), the welcome" + warn " gift wrap silently fails to publish." + warn " Real-world fix: edit Amethyst's DM Inbox list to include at least one of:" + warn " ${DEFAULT_RELAYS[*]}" + else + info "A's DM inbox overlaps wn's bootstrap at: $overlap — welcomes should flow" + fi + fi +} + # --- relays ------------------------------------------------------------------ configure_relays() { banner "Configuring relays" @@ -281,6 +409,36 @@ configure_relays() { add_relay wn_c "$r" done + # Push a kind:0 profile for each wn identity BEFORE the kind:30443 / + # 10050 / 1059 probes. Several public relays (damus.io in particular) + # silently drop non-profile kinds from accounts they've never seen + # before, so a fresh wn identity created inside the harness never gets + # its gift wraps or inbox lists stored. Publishing kind:0 first + # registers the account in the relay's "known users" set, which lets + # every later publish go through. + publish_profile() { + local who="$1" wnfn + if [[ "$who" == "B" ]]; then wnfn=wn_b; else wnfn=wn_c; fi + local name="marmot-interop $who" + local about="Scripted wn identity for Amethyst<->whitenoise-rs interop harness" + local out + if out=$("$wnfn" profile update --name "$name" --about "$about" 2>&1); then + info "$who kind:0 published (name=\"$name\")" + printf '%s profile update: %s\n' "$who" "$out" >>"$LOG_FILE" + else + warn "$who profile update failed: $out" + printf '%s profile update: %s\n' "$who" "$out" >>"$LOG_FILE" + fi + } + step "publishing kind:0 profiles so relays treat B and C as 'known' accounts" + publish_profile B + publish_profile C + # Give the relay a beat to persist + ack the kind:0 before the next + # publish (some relays gate subsequent writes on the profile being + # indexed, and without this pause the first `keys publish` that + # follows sometimes races ahead of the kind:0 commit). + sleep 2 + step "B relay list after configure" local relay_list relay_list=$(wn_b --json relays list 2>/dev/null || true) @@ -333,7 +491,15 @@ configure_relays() { fi step "sanity-check kinds 10050/1059/445: B creates group + invites C + exchanges a message" - local sanity_gid sanity_c_gid sanity_create + local sanity_gid sanity_c_gid sanity_create c_ignore + # Snapshot C's pending invites before we publish, so wait_for_invite + # doesn't fire on a stale welcome left over from a previous run (wnd + # persists pending invites across restarts — we saw this cause the + # kind:445 sanity probe to send to B's new group while C was still + # sitting on an unaccepted welcome from a previous group that B + # wasn't a member of anymore). + c_ignore=$(snapshot_invites C) + [[ -n "$c_ignore" ]] && info "C already has stale invites, ignoring: $c_ignore" sanity_create=$(wn_b --json groups create "sanity-check" "$C_NPUB" 2>>"$LOG_FILE" || true) sanity_gid=$(printf '%s' "$sanity_create" | jq_group_id) printf 'sanity groups create raw: %s\n' "$sanity_create" >>"$LOG_FILE" @@ -341,8 +507,11 @@ configure_relays() { warn "sanity group create failed — see $LOG_FILE (stale account state? rerun with 'rm -rf state/')" else info "sanity group id: $sanity_gid" - if sanity_c_gid=$(wait_for_invite C 30); then + if sanity_c_gid=$(wait_for_invite C 30 "$c_ignore"); then info "kind:10050+1059 ok — C received welcome (gid: $sanity_c_gid)" + if [[ "$sanity_c_gid" != "$sanity_gid" ]]; then + warn "gid mismatch — C saw $sanity_c_gid, B created $sanity_gid (likely a stale welcome slipped past the filter)" + fi wn_c groups accept "$sanity_c_gid" >/dev/null 2>&1 || true wn_b messages send "$sanity_gid" "sanity-ping" >/dev/null 2>&1 || true if wait_for_message C "$sanity_c_gid" "sanity-ping" 30; then @@ -362,19 +531,42 @@ configure_relays() { } instruct_amethyst_setup() { - local list if [[ "$USE_LOCAL_RELAYS" -eq 1 ]]; then - list=" ws://10.0.2.2:8080 (Android emulator) - ws://:8080 (physical device on same Wi-Fi)" - else - list=$(printf ' %s\n' "${DEFAULT_RELAYS[@]}") + # Offline/sandbox path: we own the only relay, so the harness DOES + # need to dictate Amethyst's relay config — nothing is discoverable + # via the public network. + prompt_human "Configure Amethyst to match this --local-relays harness: + 1. Settings -> Relays: add as READ+WRITE + ws://10.0.2.2:8080 (Android emulator) + ws://:8080 (physical device on same Wi-Fi) + 2. Settings -> Key Package Relays: add the SAME URL + 3. Settings -> DM Inbox Relays (NIP-17/kind:10050): add the SAME URL + 4. Trigger key-package publish (toggle KP relay on/off if needed) + 5. Confirm your Amethyst account is logged in with npub: $A_NPUB" + return fi - prompt_human "Configure Amethyst to match this harness: - 1. Settings -> Relays: add the following as READ+WRITE -$list - 2. Settings -> Key Package Relays: add the SAME URLs - 3. Trigger key-package publish (toggle KP relay on/off if needed) - 4. Confirm your Amethyst account is logged in with npub: $A_NPUB" + + # Public-relay path: the harness should behave like any real Nostr + # client — discover A's advertised relays via kind:10002 / 10050 / + # 10051 and publish there, rather than forcing A to adopt the + # harness's own relay set. That lets the tests surface real-world + # interop failures (e.g. A's DM inbox points at auth-required or + # unreachable relays) instead of hiding them behind shared config. + prompt_human "No relay reconfiguration required. Just confirm: + 1. Amethyst is logged in as npub: + $A_NPUB + 2. Amethyst has published (on its own chosen relays): + - kind:30443 KeyPackage + - kind:10051 Key Package Relay List + - kind:10050 DM Inbox Relay List + - kind:10002 NIP-65 Outbox/Inbox Relay List + Any one of Amethyst's public write relays will do — the wn daemons + below are bootstrapped on $(printf '%s, ' "${DEFAULT_RELAYS[@]}" | sed 's/, $//') and will + discover A's advertised relays automatically. + 3. If the wn-side diagnostic after this prompt shows that wn cannot + see any of A's four lists, Amethyst hasn't published them to any + relay wn can reach — republish them (toggle a relay off/on in + Settings, then verify via an external relay inspector)." } # ==== tests ================================================================== @@ -421,6 +613,12 @@ test_02_amethyst_creates_group() { # Amethyst sends it. dump_daemon_diagnostics B "pre-invite baseline" + # Snapshot B's already-pending invites so we can tell the new welcome + # apart from a leftover from a previous run. + local b_ignore + b_ignore=$(snapshot_invites B) + [[ -n "$b_ignore" ]] && info "B already has stale invites, ignoring: $b_ignore" + prompt_human "In Amethyst: 1. Tap '+' -> Create Group 2. Name: Interop-02 @@ -434,7 +632,7 @@ that's the bug." step "polling B's daemon for invite (60s, heartbeat every ~10s)" local gid - if ! gid=$(wait_for_invite B 60); then + if ! gid=$(wait_for_invite B 60 "$b_ignore"); then fail_msg "no invite arrived at B" dump_daemon_diagnostics B "post-timeout (test_02)" prompt_amethyst_logcat "test_02 timeout" @@ -473,11 +671,41 @@ that's the bug." fi step "B sends reply" - wn_b messages send "$gid" "hello from wn" >/dev/null 2>&1 || warn "send returned nonzero" + local send_out + if ! send_out=$(wn_b messages send "$gid" "hello from wn" 2>&1); then + warn "wn messages send returned nonzero: $send_out" + fi + printf 'wn messages send (test_02 reply) raw: %s\n' "$send_out" >>"$LOG_FILE" + # Give the relay a couple seconds to fan the commit/message back out to + # Amethyst's subscription before we ask the operator to eyeball the UI. + sleep 4 + + # Dump the B side up front — if B's own store doesn't contain the reply + # or the MLS extension relays don't match what Amethyst subscribes to, + # the operator can confirm "fail" with a specific reason instead of a + # blind eyeball check. + dump_outbound_diagnostics B "$gid" "hello from wn" "test_02 B->A reply" if confirm "Does Amethyst now show 'hello from wn' in the same group?"; then record_result "02 Amethyst->B create+invite" pass else + # The B-side dump above told us whether the send reached B's own DB and + # which relays wn tagged on the kind:445. Now pull the Amethyst side so + # the failure report has both halves of the pipe: which relays the + # kind:445 subscription actually targets, whether the event arrived, + # and (if it did) whether GroupEventHandler dropped it. + prompt_human "Capture Amethyst's side of the B->A kind:445 path. On the adb host: + + adb logcat -d -v time | grep -E \ + 'MarmotDbg|GroupEventHandler|MarmotGroupEventsEoseManager|kind:445' \\ + | tail -n 200 > /tmp/amethyst-marmot.log + +Then paste /tmp/amethyst-marmot.log here. Key lines to look for: + - 'MarmotGroupEventsEoseManager.updateFilter: group=${gid:0:8}…' + the relay + list — must match the 'group relays (MLS extension)' in the B-side dump. + - 'GroupEventHandler.add: kind:445 id=… groupId=${gid:0:8}…' (event arrived). + - 'GroupEventHandler.add: not a member of group=${gid:0:8}…' (dropped). + - 'GroupEventHandler.add: processGroupEvent returned ApplicationMessage…' (decrypted)." record_result "02 Amethyst->B create+invite" fail "Amethyst did not display reply" fi } @@ -543,13 +771,17 @@ test_04_three_member_group() { dump_daemon_diagnostics C "pre-invite baseline (test_04)" + local c_ignore + c_ignore=$(snapshot_invites C) + [[ -n "$c_ignore" ]] && info "C already has stale invites, ignoring: $c_ignore" + prompt_human "In Amethyst, open the group from Test 02 (or 'Interop-04-bootstrap'). Group Info -> Add Member -> paste: $C_NPUB Confirm the invite is sent." step "polling C for invite (60s, heartbeat every ~10s)" local c_gid - if ! c_gid=$(wait_for_invite C 60); then + if ! c_gid=$(wait_for_invite C 60 "$c_ignore"); then fail_msg "C never received invite" dump_daemon_diagnostics C "post-timeout (test_04)" prompt_amethyst_logcat "test_04 timeout" @@ -582,7 +814,8 @@ test_05_wn_adds_amethyst_existing() { banner "Test 05 — wn adds Amethyst to existing B+C group" step "B creates group with only C, then adds A" - local out gid + local out gid c_ignore + c_ignore=$(snapshot_invites C) out=$(wn_b --json groups create "Interop-05" "$C_NPUB" 2>>"$LOG_FILE") printf 'wn groups create raw output: %s\n' "$out" >>"$LOG_FILE" gid=$(printf '%s' "$out" | jq_group_id) @@ -592,8 +825,9 @@ test_05_wn_adds_amethyst_existing() { return fi save_state GROUP_05 "$gid" - if wait_for_invite C 30 >/dev/null; then - wn_c groups accept "$gid" >/dev/null 2>&1 || true + local c_new_gid + if c_new_gid=$(wait_for_invite C 30 "$c_ignore"); then + wn_c groups accept "$c_new_gid" >/dev/null 2>&1 || true fi step "B adds A to the group" @@ -624,33 +858,61 @@ test_06_member_removal() { skip_msg "Test 06 requires Test 05 state"; record_result "06 member removal" skip "no GROUP_05"; return fi - prompt_human "In Amethyst, open group 'Interop-05' -> Group Info -> Members. - Tap C ($C_NPUB) -> Remove. - Confirm the removal." + # Interop-05 was created by B (wn), so B is the sole admin. Per + # MIP-03 `enforceAuthorizedProposalSet`, non-admin members may only + # commit a self-Update or a SelfRemove-only proposal set — a Remove + # proposal from A (a plain member here) is rejected before publish + # with "non-admin members may only commit …". The previous revision + # of this test asked the operator to remove C from Amethyst's UI, + # which is correct per spec to REJECT — so the test itself was + # misaligned with the admin model. Drive the removal from B (the + # admin) instead, and observe its effect on A + C. + step "B (admin) removes C from Interop-05" + local remove_out + if ! remove_out=$(wn_b groups remove-members "$gid" "$C_NPUB" 2>&1); then + fail_msg "wn_b groups remove-members failed: $remove_out" + printf 'wn_b groups remove-members: %s\n' "$remove_out" >>"$LOG_FILE" + record_result "06 member removal" fail "wn remove returned nonzero" + return + fi + printf 'wn_b groups remove-members: %s\n' "$remove_out" >>"$LOG_FILE" - step "verifying C is removed (60s poll)" - local deadline=$(( $(date +%s) + 60 )) removed=0 + step "verifying C is removed from B's + C's view (60s poll)" + local deadline=$(( $(date +%s) + 60 )) removed_b=0 removed_c=0 while [[ $(date +%s) -lt $deadline ]]; do + if ! wn_b --json groups members "$gid" 2>/dev/null \ + | jq -e --arg p "$C_HEX" '(.result // .) | .[]? | select((.pubkey // .public_key) == $p)' >/dev/null 2>&1; then + removed_b=1 + fi if ! wn_c --json groups members "$gid" 2>/dev/null \ | jq -e --arg p "$C_HEX" '(.result // .) | .[]? | select((.pubkey // .public_key) == $p)' >/dev/null 2>&1; then - removed=1; break + removed_c=1 fi + [[ "$removed_b" -eq 1 ]] && break sleep 3 done + if [[ "$removed_b" -ne 1 ]]; then + warn "B still shows C as a member after remove — commit may not have applied" + fi + info "post-remove: B sees C gone=$removed_b, C sees self gone=$removed_c" - if [[ "$removed" -ne 1 ]]; then - warn "C still appears to be a member; proceeding anyway" + prompt_human "In Amethyst, open 'Interop-05' -> Group Info and confirm C is no longer listed." + if ! confirm "Does Amethyst's member list now show only you and B (not C)?"; then + record_result "06 member removal" fail "Amethyst still shows C as a member" + return fi prompt_human "In Amethyst, send: 'after removing C'" if wait_for_message B "$gid" "after removing C" 30; then - info "B still receives messages (expected)" + info "B still receives messages (expected — B + A are the remaining members)" else fail_msg "B stopped receiving messages (unexpected)" record_result "06 member removal" fail "B lost access"; return fi - # C should NOT be able to decrypt the new message + # C should NOT be able to decrypt the new message — forward secrecy + # at the post-remove epoch means C's retained keys cannot open any + # kind:445 sealed under the new epoch's exporter. sleep 5 if wait_for_message C "$gid" "after removing C" 10; then fail_msg "C still decrypted a post-removal message — forward secrecy broken" @@ -664,37 +926,58 @@ test_06_member_removal() { test_07_metadata_rename() { banner "Test 07 — Group metadata rename round-trip (MIP-01)" - local gid - gid=$(load_state GROUP_02 || true) - if [[ -z "${gid:-}" ]]; then - skip_msg "no GROUP_02"; record_result "07 metadata rename" skip "no group state"; return - fi - - prompt_human "In Amethyst, open group 'Interop-02' -> Group Info -> Edit. + # Forward direction: A (admin of Interop-02) renames → B observes. + local gid_a + gid_a=$(load_state GROUP_02 || true) + if [[ -z "${gid_a:-}" ]]; then + skip_msg "no GROUP_02"; record_result "07a metadata rename A->B" skip "no group state" + else + prompt_human "In Amethyst, open group 'Interop-02' -> Group Info -> Edit. Rename to: Interop-02-renamed Save." - step "polling B for name change (60s)" - local deadline=$(( $(date +%s) + 60 )) name="" - while [[ $(date +%s) -lt $deadline ]]; do - name=$(wn_b --json groups show "$gid" 2>/dev/null | jq -r '(.result // .) | .name // empty') - [[ "$name" == "Interop-02-renamed" ]] && break - sleep 3 - done - if [[ "$name" == "Interop-02-renamed" ]]; then - pass_msg "B sees renamed group" - else - fail_msg "B still sees name: $name" - record_result "07 metadata rename" fail "rename did not propagate to B"; return + step "polling B for name change (60s)" + local deadline_a=$(( $(date +%s) + 60 )) name_a="" + while [[ $(date +%s) -lt $deadline_a ]]; do + name_a=$(wn_b --json groups show "$gid_a" 2>/dev/null | jq -r '(.result // .) | .name // empty') + [[ "$name_a" == "Interop-02-renamed" ]] && break + sleep 3 + done + if [[ "$name_a" == "Interop-02-renamed" ]]; then + pass_msg "B sees renamed Interop-02" + record_result "07a metadata rename A->B" pass + else + fail_msg "B still sees name: $name_a" + record_result "07a metadata rename A->B" fail "rename did not propagate to B" + fi fi - step "B renames back" - wn_b groups rename "$gid" "Interop-02-reverse" >/dev/null 2>&1 || true + # Reverse direction: B (admin of Interop-03) renames → A observes. + # + # The previous revision issued the reverse rename on Interop-02 via + # `wn_b groups rename` — but B is a plain member of Interop-02, so + # MIP-03's `enforceAuthorizedProposalSet` rejects the GCE commit on + # the wn side and the rename never reaches any relay. Use the group + # B actually owns (Interop-03) for the reverse leg instead. + local gid_b + gid_b=$(load_state GROUP_03 || true) + if [[ -z "${gid_b:-}" ]]; then + skip_msg "no GROUP_03 — skipping reverse rename" + record_result "07b metadata rename B->A" skip "no GROUP_03" + return + fi - if confirm "Does Amethyst now show the group name as 'Interop-02-reverse'?"; then - record_result "07 metadata rename" pass + step "B (admin) renames Interop-03 -> Interop-03-renamed" + local rename_out + if ! rename_out=$(wn_b groups rename "$gid_b" "Interop-03-renamed" 2>&1); then + warn "wn_b groups rename returned nonzero: $rename_out" + printf 'wn_b groups rename: %s\n' "$rename_out" >>"$LOG_FILE" + fi + + if confirm "Does Amethyst now show the group previously named 'Interop-03' as 'Interop-03-renamed'?"; then + record_result "07b metadata rename B->A" pass else - record_result "07 metadata rename" fail "Amethyst did not pick up rename" + record_result "07b metadata rename B->A" fail "Amethyst did not pick up rename" fi } @@ -710,9 +993,11 @@ test_08_admin_promote_demote() { step "B (creator+admin) adds C so we have 3 members" wn_c keys publish >/dev/null 2>&1 || true sleep 3 + local c_ignore_08 c_new_gid_08 + c_ignore_08=$(snapshot_invites C) wn_b groups add-members "$gid" "$C_NPUB" >/dev/null 2>&1 || warn "add-members C returned nonzero" - if wait_for_invite C 30 >/dev/null; then - wn_c groups accept "$gid" >/dev/null 2>&1 || true + if c_new_gid_08=$(wait_for_invite C 30 "$c_ignore_08"); then + wn_c groups accept "$c_new_gid_08" >/dev/null 2>&1 || true fi step "B promotes A to admin" @@ -814,7 +1099,10 @@ test_10_concurrent_commits() { step "verifying B's view is consistent (name + epoch)" local b_view - b_view=$(wn_b --json groups show "$gid" 2>/dev/null | jq '{name, epoch}' 2>/dev/null || echo "{}") + # Post-v0.2 wn wraps `groups show` output in {"result": {...}}; peel + # that wrapper before extracting the fields so the info log isn't + # "name: null, epoch: null" on every run. + b_view=$(wn_b --json groups show "$gid" 2>/dev/null | jq '(.result // .) | {name, epoch}' 2>/dev/null || echo "{}") info "B state: $b_view" prompt_human "Read the group name now shown in Amethyst." @@ -890,9 +1178,11 @@ test_12_offline_catchup() { done step "B adds C, then sends 3 more messages" + local c_ignore_12 c_new_gid_12 + c_ignore_12=$(snapshot_invites C) wn_b groups add-members "$gid" "$C_NPUB" >/dev/null 2>&1 || true - if wait_for_invite C 30 >/dev/null; then - wn_c groups accept "$gid" >/dev/null 2>&1 || true + if c_new_gid_12=$(wait_for_invite C 30 "$c_ignore_12"); then + wn_c groups accept "$c_new_gid_12" >/dev/null 2>&1 || true fi for i in 6 7 8; do wn_b messages send "$gid" "offline-msg-$i" >/dev/null 2>&1 || true @@ -1005,6 +1295,15 @@ main() { dump_daemon_diagnostics B "post-configure" dump_daemon_diagnostics C "post-configure" + # Let wn discover A's advertised relays via its normal subscription + # plane, then dump what wn actually sees. This is the "Am I going to + # be able to reach this user?" probe — surfaces up front the kind of + # failure (A's 10050 unreachable from wn, missing KP list, etc.) that + # would otherwise bite as a silent Test 03 timeout. + if [[ "$USE_LOCAL_RELAYS" -ne 1 ]]; then + discover_a_relays + fi + instruct_amethyst_setup test_01_keypackage_discovery