From 32b0f41be9cd9a144ef28bd0f75659667b2a102d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 19:52:30 +0000 Subject: [PATCH] test: move re-add-after-remove regression to Quartz MlsGroup test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bug (re-added user rejoins without a usable own_leaf, so they can neither decrypt new group messages nor send) reproduces entirely with two in-process MlsGroup instances — no relay or second Nostr client needed. A pure Quartz unit test runs in seconds and catches the regression deterministically, where the bash interop version depended on whitenoise-rs, wnd daemons and a local relay. - quartz: add MlsGroupLifecycleTest.testReaddAfterRemove_RejoinerCanEncryptAndDecrypt covering create → add → round-trip → remove → re-add with a fresh KeyPackage → round-trip again, asserting leafIndex and both encrypt/decrypt directions on the rejoined instance. - interop: drop test_17_readd_after_remove from the headless suite and its registration, since the unit test supersedes it. https://claude.ai/code/session_01JaeqZwVNLKvUUvXWRzPmRP --- .../marmot/mls/MlsGroupLifecycleTest.kt | 56 ++++++++++++ tools/marmot-interop/headless/tests-manage.sh | 89 ------------------- .../marmot-interop/marmot-interop-headless.sh | 1 - 3 files changed, 56 insertions(+), 90 deletions(-) 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 bfd27bce6..0240d2cec 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 @@ -27,6 +27,7 @@ import kotlin.test.assertContentEquals import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull +import kotlin.test.assertTrue /** * End-to-end lifecycle tests for MlsGroup covering the full protocol flow: @@ -298,6 +299,61 @@ class MlsGroupLifecycleTest { ) } + /** + * Regression: removing a member and re-adding them must produce a fully- + * functional group instance on the rejoiner's side. In the originally + * reported bug, the re-added user came back with no usable own_leaf — + * the UI showed the group but the user could neither decrypt new + * messages nor send any. This exercises the full loop with a fresh + * KeyPackage on re-add and asserts both directions still work. + */ + @Test + fun testReaddAfterRemove_RejoinerCanEncryptAndDecrypt() { + val alice = MlsGroup.create("alice".encodeToByteArray()) + + // First add: Bob joins. + val firstBundle = createStandaloneKeyPackage("bob") + val firstAdd = alice.addMember(firstBundle.keyPackage.toTlsBytes()) + val bob1 = MlsGroup.processWelcome(firstAdd.welcomeBytes!!, firstBundle) + + // Baseline round-trip works before the remove. + val hello = "hello before remove".encodeToByteArray() + assertContentEquals(hello, bob1.decrypt(alice.encrypt(hello)).content) + val pong = "pong before remove".encodeToByteArray() + assertContentEquals(pong, alice.decrypt(bob1.encrypt(pong)).content) + + // Alice removes Bob. bob1 is now stale (left-behind, no commit to apply). + alice.removeMember(bob1.leafIndex) + assertEquals(1, alice.memberCount) + + // Re-add: Bob publishes a fresh KeyPackage, Alice adds it, Bob processes + // the new Welcome from scratch. This is the exact flow driven by the + // info screen's inline "Add member" after an earlier removal. + val secondBundle = createStandaloneKeyPackage("bob") + val secondAdd = alice.addMember(secondBundle.keyPackage.toTlsBytes()) + val bob2 = MlsGroup.processWelcome(secondAdd.welcomeBytes!!, secondBundle) + + assertEquals(alice.epoch, bob2.epoch, "Rejoiner must be at Alice's epoch") + assertEquals(2, alice.memberCount) + assertEquals(2, bob2.memberCount) + + // Own-leaf sanity: the rejoiner must actually hold a leaf in the tree. + assertTrue(bob2.leafIndex >= 0, "Rejoiner must have a valid leafIndex after re-add") + + // The bug surfaces here — if bob2 has no usable leaf / keying material, + // one of these two round-trips fails. + val afterHello = "hello after re-add".encodeToByteArray() + val decrypted = bob2.decrypt(alice.encrypt(afterHello)) + assertContentEquals(afterHello, decrypted.content, "Rejoiner could not decrypt") + assertEquals(0, decrypted.senderLeafIndex, "Sender should still be Alice at leaf 0") + + val afterPong = "pong after re-add".encodeToByteArray() + val bobEncrypted = bob2.encrypt(afterPong) + val aliceSees = alice.decrypt(bobEncrypted) + assertContentEquals(afterPong, aliceSees.content, "Rejoiner could not send (no usable leaf)") + assertEquals(bob2.leafIndex, aliceSees.senderLeafIndex) + } + // ----------------------------------------------------------------------- // 8. Signing key rotation (Update proposal) // ----------------------------------------------------------------------- diff --git a/tools/marmot-interop/headless/tests-manage.sh b/tools/marmot-interop/headless/tests-manage.sh index b4a293c8e..fd14c49f9 100644 --- a/tools/marmot-interop/headless/tests-manage.sh +++ b/tools/marmot-interop/headless/tests-manage.sh @@ -155,95 +155,6 @@ test_08_admin_promote_demote() { fi } -test_17_readd_after_remove() { - banner "Test 17 — Re-add after remove restores B's leaf" - local id="17 re-add after remove" - - # Scenario reported on the info screen: A creates a group, adds B, - # removes B, then adds B again. Before the fix B rejoined without a - # usable own_leaf, so B could see metadata but could neither decrypt - # new posts nor sign messages. This test catches that regression by - # driving a full round-trip (A→B and B→A) after the re-add. - - local out gid mls_gid - out=$(amy_json marmot group create --name "Interop-17-readd") || { - record_result "$id" fail "amy create failed"; return - } - gid=$(printf '%s' "$out" | jq -r '.group_id') - mls_gid=$(printf '%s' "$out" | jq -r '.mls_group_id') - [[ -n "$gid" && -n "$mls_gid" ]] || { - record_result "$id" fail "missing ids from create"; return - } - - # A adds B. - amy_json marmot group add "$gid" "$B_NPUB" >/dev/null || { - record_result "$id" fail "initial add B failed"; return - } - wait_for_invite B 60 >/dev/null || { - record_result "$id" fail "B never received first invite"; return - } - wn_b groups accept "$mls_gid" >/dev/null 2>&1 || true - - # Confirm the baseline works: A → B and B → A both deliver. - amy_json marmot message send "$gid" "pre-remove hello" >/dev/null || { - record_result "$id" fail "A send before remove failed"; return - } - wait_for_message B "$mls_gid" "pre-remove hello" 90 || { - record_result "$id" fail "B missed pre-remove hello"; return - } - wn_b messages send "$mls_gid" "pre-remove pong" >/dev/null 2>&1 || true - amy_json marmot await message "$gid" --match "pre-remove pong" --timeout 60 >/dev/null || { - record_result "$id" fail "A missed B's pre-remove pong"; return - } - - # A removes B. - amy_json marmot group remove "$gid" "$B_NPUB" >/dev/null || { - record_result "$id" fail "remove B failed"; return - } - # Let the Remove commit propagate to B's wnd. - local deadline=$(( $(date +%s) + 120 )) removed=0 - while [[ $(date +%s) -lt $deadline ]]; do - if ! wn_b --json groups members "$mls_gid" 2>/dev/null \ - | jq -e --arg p "$B_HEX" '(.result // .) | .[]? | select((.pubkey // .public_key) == $p)' \ - >/dev/null 2>&1; then - removed=1; break - fi - sleep 3 - done - [[ "$removed" -eq 1 ]] || warn "B still appears in members after remove — continuing" - - # B republishes its KP so the next add can find a fresh one. - wn_b keys publish >/dev/null 2>&1 || true - sleep 3 - - # A re-adds B. - amy_json marmot group add "$gid" "$B_NPUB" >/dev/null || { - record_result "$id" fail "re-add B failed"; return - } - wait_for_invite B 60 >/dev/null || { - record_result "$id" fail "B never received second invite"; return - } - wn_b groups accept "$mls_gid" >/dev/null 2>&1 || true - - # Regression probes: after re-add B must have a usable leaf again. - # (1) A→B: proves B can decrypt new group messages. - amy_json marmot message send "$gid" "post-readd hello" >/dev/null || { - record_result "$id" fail "A send after re-add failed"; return - } - wait_for_message B "$mls_gid" "post-readd hello" 120 || { - record_result "$id" fail "B didn't decrypt post-readd message (missing own_leaf?)"; return - } - # (2) B→A: proves B can still sign/commit — the exact breakage reported. - wn_b messages send "$mls_gid" "post-readd pong" >/dev/null 2>&1 || { - record_result "$id" fail "B send after re-add failed (likely no leaf)"; return - } - if amy_json marmot await message "$gid" --match "post-readd pong" --timeout 120 >/dev/null; then - record_result "$id" pass - else - record_result "$id" fail "A never saw B's post-readd pong (B couldn't type)" - fi -} - test_11_leave_group() { banner "Test 11 — Leave group" local id="11 leave group" diff --git a/tools/marmot-interop/marmot-interop-headless.sh b/tools/marmot-interop/marmot-interop-headless.sh index 1baa6365f..df2bd8308 100755 --- a/tools/marmot-interop/marmot-interop-headless.sh +++ b/tools/marmot-interop/marmot-interop-headless.sh @@ -123,4 +123,3 @@ test_13_keypackage_rotation test_14_wn_removes_a test_15_wn_member_leaves test_16_wn_keypackage_rotation -test_17_readd_after_remove