From f2160f626425d5edaf6edb2a62add8644826ef45 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 21:02:37 +0000 Subject: [PATCH 01/13] test(quartz): pin what a soft-banned Concord staffer can still do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CORD-04 §4 row 3 of docs/concord-banlist-rank-conformance.md was left open as "a genuine fixpoint-ordering question". This reproduces what that gap costs. ConcordCommunityState.fold gates METADATA/CHANNEL/INVITE through authority.hasPermission (`!isBanned && ..`), but ROLE, GRANT and BANLIST are gated inside AuthorityResolver.resolve by holdsManageRoles / bitsOf / effectivePermissionsOf, none of which consult the banlist — and none of which can, as written, since the roles/grants fixpoint settles before `banned` is computed. So half the Control Plane honors a ban and half is blind to it. A banned member who still holds control_root therefore keeps the roster: they revoke the surviving moderators, retire the roles beneath them, ban everyone they outrank, and — since a role edition they author is honored — mint a fresh, unbanned npub at the next position down. That npub passes every ban-aware gate, so it tombstones the channels (terminal ids), rewrites the metadata, and, being a non-banned BAN holder, is accepted as a rotator by drainConcordRekeys. The tests assert the CURRENT, VULNERABLE behaviour so it cannot regress silently; each ESCALATION assertion is to be inverted, not deleted, when the ordering rule lands. Two companions pin what the fix must preserve: self-unban and puppet-unban both stay refused, closed already by the delta rank rule. Also records why a chain-local fix is insufficient — forking the banlist at genesis dodges any "was the author banned by this edition's parent" rule, and §4's re-heal union carries the rogue bans in anyway. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- .../cord04Roles/BannedStaffEscalationTest.kt | 276 ++++++++++++++++++ 1 file changed, 276 insertions(+) create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt new file mode 100644 index 0000000000..8ad22aba6b --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt @@ -0,0 +1,276 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.concord.cord04Roles + +import com.vitorpamplona.quartz.concord.cord02Community.ConcordCommunityState +import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * What a **soft-banned staffer** can still do to a community — the reproduction behind + * `docs/concord-banlist-rank-conformance.md` §4 row 3, which the report left open as "a genuine + * fixpoint-ordering question, not a plain oversight". + * + * The asymmetry these tests pin: [ConcordCommunityState.fold] gates METADATA / CHANNEL / INVITE + * through `authority.hasPermission`, which is `!isBanned && …`, but ROLE, GRANT and BANLIST are + * gated *inside* [AuthorityResolver.resolve] by `holdsManageRoles` / `bitsOf` / + * `effectivePermissionsOf` — none of which consult the banlist. Nor could they as written: the + * roles/grants fixpoint runs before `banned` is computed at all. So half the Control Plane honors + * a ban and half is structurally blind to it, and a banned member who still holds `control_root` + * keeps full authority over the roster. + * + * **These tests assert the CURRENT, VULNERABLE behaviour**, so the escalation cannot regress + * silently or be "fixed" by accident without someone noticing. Every `ESCALATION:` assertion here + * must be INVERTED — not deleted — when the ordering rule lands. [selfUnbanIsStillRefused] and + * [aJuniorPuppetCannotLiftASeniorsBan] are the opposite: they pin behaviour the fix must preserve. + * + * Note for whoever writes that fix: a chain-local rule ("the author must not be banned by the state + * their edition chains from") is NOT sufficient — see + * [aBannedAdminForksTheBanlistAtGenesisRatherThanChainingOntoTheirOwnBan]. The rule has to bind + * CORD-04 §4's re-heal union too. + */ +class BannedStaffEscalationTest { + private val owner = "0f".repeat(32) + private val alice = "a1".repeat(32) // Admin, position 1 — the member who gets banned + private val bob = "b2".repeat(32) // Mod, position 5 + private val carol = "c3".repeat(32) // plain member, no role + private val puppet = "e5".repeat(32) // a fresh npub alice controls + + private val adminRole = "11".repeat(32) + private val modRole = "22".repeat(32) + private val puppetRole = "33".repeat(32) + + private val banlistEntity = "44".repeat(32) + private val channelEntity = "55".repeat(32) + private val metadataEntity = "66".repeat(32) + private val bobGrantEntity = "32".repeat(32) + private val puppetGrantEntity = "35".repeat(32) + + // MANAGE_ROLES|MANAGE_CHANNELS|MANAGE_METADATA|KICK|BAN|CREATE_INVITE = 1+2+4+8+16+64 + private val adminJson = """{"name":"Admin","position":1,"permissions":"95"}""" + private val modJson = """{"name":"Mod","position":5,"permissions":"24"}""" // KICK|BAN + + private fun edition( + kind: ControlEntityKind, + entity: String, + version: Long, + prev: ByteArray?, + content: String, + author: String, + rumorId: String, + ) = ControlEdition(kind, entity.hexToByteArray(), version, prev, null, content, author, rumorId, 0) + + private fun role( + id: String, + json: String, + author: String = owner, + version: Long = 0, + prev: ByteArray? = null, + ) = edition(ControlEntityKind.ROLE, id, version, prev, json, author, "role-$id-$version-$author") + + private fun grant( + coordinate: String, + member: String, + roleIds: List, + author: String, + version: Long = 0, + prev: ByteArray? = null, + ) = edition( + ControlEntityKind.GRANT, + coordinate, + version, + prev, + """{"member":"$member","role_ids":[${roleIds.joinToString(",") { "\"$it\"" }}]}""", + author, + "grant-$coordinate-$version-$author", + ) + + private fun banlist( + author: String, + version: Long, + prev: ByteArray?, + vararg banned: String, + ) = edition( + ControlEntityKind.BANLIST, + banlistEntity, + version, + prev, + "[${banned.joinToString(",") { "\"$it\"" }}]", + author, + "ban-$version-$author", + ) + + private fun channel( + json: String, + author: String, + version: Long, + prev: ByteArray?, + ) = edition(ControlEntityKind.CHANNEL, channelEntity, version, prev, json, author, "chan-$version-$author") + + private fun metadata( + json: String, + author: String, + version: Long, + prev: ByteArray?, + ) = edition(ControlEntityKind.METADATA, metadataEntity, version, prev, json, author, "meta-$version-$author") + + private val channelV0 = channel("""{"name":"general"}""", owner, 0, null) + private val metadataV0 = metadata("""{"name":"My Community"}""", owner, 0, null) + private val bobGrantV0 = grant(bobGrantEntity, bob, listOf(modRole), owner) + private val modRoleV0 = role(modRole, modJson) + + /** The owner-authored community every test starts from: two roles, two grants, a channel, metadata. */ + private fun community() = + mutableListOf( + role(adminRole, adminJson), + modRoleV0, + grant("31".repeat(32), alice, listOf(adminRole), owner), + bobGrantV0, + channelV0, + metadataV0, + ) + + /** The owner bans alice. Genesis of the banlist, so every test can fork or chain off it. */ + private val ownerBansAlice = banlist(owner, 0, null, alice) + + /** Alice, already banned, mints a role just below herself and hands it to a fresh npub. */ + private fun aliceMintsAPuppet() = + listOf( + role(puppetRole, """{"name":"Puppet","position":2,"permissions":"95"}""", author = alice), + grant(puppetGrantEntity, puppet, listOf(puppetRole), author = alice), + ) + + @Test + fun aBanStripsTheAuthorityCheckedByFoldButNotTheOneCheckedByTheResolver() { + val r = AuthorityResolver.resolve(community() + ownerBansAlice, owner) + + assertTrue(r.isBanned(alice), "the owner's ban lands") + assertFalse(r.hasPermission(alice, ConcordPermissions.MANAGE_ROLES), "the ban-aware check refuses her") + // ...but this is the one every ROLE/GRANT/BANLIST gate inside resolve() actually consults. + assertTrue( + r.effectivePermissions(alice).has(ConcordPermissions.MANAGE_ROLES), + "ESCALATION: a banned staffer keeps the permissions the resolver's own gates read", + ) + } + + @Test + fun aBannedAdminPromotesAFreshSockpuppetToAdmin() { + val r = AuthorityResolver.resolve(community() + ownerBansAlice + aliceMintsAPuppet(), owner) + + assertEquals(2, r.rank(puppet), "ESCALATION: the banned admin's role edition is honored") + assertFalse(r.isBanned(puppet), "the puppet is a clean npub — nothing to filter it on") + assertTrue( + r.hasPermission(puppet, ConcordPermissions.MANAGE_CHANNELS), + "ESCALATION: a banned member minted a live admin with the ban-aware check passing", + ) + } + + @Test + fun theSockpuppetDeletesEveryChannelAndRewritesTheMetadata() { + val editions = + community() + ownerBansAlice + aliceMintsAPuppet() + + // A channel tombstone is terminal — CORD-03: the id is never reused. + channel("""{"name":"general","deleted":true}""", puppet, 1, channelV0.hash) + + metadata("""{"name":"Owned by the guy you banned"}""", puppet, 1, metadataV0.hash) + + val state = ConcordCommunityState.fold(editions, owner) + + assertEquals(0, state.channels.size, "ESCALATION: the community's channels are irrecoverably tombstoned") + assertEquals("Owned by the guy you banned", state.metadata?.name, "ESCALATION: and its identity rewritten") + } + + @Test + fun theSockpuppetBansEveryMemberBeneathIt() { + val editions = community() + ownerBansAlice + aliceMintsAPuppet() + banlist(puppet, 1, ownerBansAlice.hash, alice, bob, carol) + + val r = AuthorityResolver.resolve(editions, owner) + + assertTrue(r.isBanned(bob), "ESCALATION: the surviving moderator is silenced, losing all authority with it") + assertTrue(r.isBanned(carol), "ESCALATION: and the plain members with them") + } + + @Test + fun aBannedAdminBansEveryoneBeneathThemWithoutNeedingAPuppetAtAll() { + val editions = community() + ownerBansAlice + banlist(alice, 1, ownerBansAlice.hash, alice, bob, carol) + + val r = AuthorityResolver.resolve(editions, owner) + + assertTrue(r.isBanned(bob), "ESCALATION: banGate reads effectivePermissionsOf, which ignores her own ban") + assertTrue(r.isBanned(carol), "ESCALATION: same") + } + + @Test + fun aBannedAdminForksTheBanlistAtGenesisRatherThanChainingOntoTheirOwnBan() { + // The same attack as above, except her edition does NOT chain onto the edition that banned + // her — it forks at genesis. So a rule that only asks "was the author banned by this + // edition's parent?" never sees her ban, and CORD-04 §4's re-heal union carries her bans in + // regardless. Any fix has to bind the union, not just the chain. + val editions = community() + ownerBansAlice + banlist(alice, 0, null, bob, carol) + + val r = AuthorityResolver.resolve(editions, owner) + + assertTrue(r.isBanned(alice), "the owner's ban survives the fork — the union is down-only") + assertTrue(r.isBanned(bob), "ESCALATION: and so does the banned admin's, healed in as a concurrent ban") + assertTrue(r.isBanned(carol), "ESCALATION: same") + } + + @Test + fun aBannedAdminRevokesTheSurvivingModerators() { + val editions = community() + ownerBansAlice + grant(bobGrantEntity, bob, emptyList(), author = alice, version = 1, prev = bobGrantV0.hash) + + val r = AuthorityResolver.resolve(editions, owner) + + assertEquals(null, r.rank(bob), "ESCALATION: a banned admin stripped a live moderator's roles") + assertFalse(r.hasPermission(bob, ConcordPermissions.BAN), "ESCALATION: leaving nobody but the owner able to act") + } + + @Test + fun aBannedAdminDeletesEveryRoleBeneathThem() { + val tombstone = role(modRole, """{"name":"Mod","position":5,"permissions":"24","deleted":true}""", author = alice, version = 1, prev = modRoleV0.hash) + + val r = AuthorityResolver.resolve(community() + ownerBansAlice + tombstone, owner) + + assertEquals(null, r.roles()[modRole], "ESCALATION: a banned admin retired a role beneath them") + assertEquals(null, r.rank(bob), "ESCALATION: every holder of it silently loses their standing") + } + + @Test + fun selfUnbanIsStillRefused() { + // docs/concord-banlist-rank-conformance.md §4 row 3, the half that IS closed: the delta rule + // gates removals too, and strict outranking means nobody outranks themselves. + val editions = community() + ownerBansAlice + banlist(alice, 1, ownerBansAlice.hash) + + assertTrue(AuthorityResolver.resolve(editions, owner).isBanned(alice), "a banned member may not lift their own ban") + } + + @Test + fun aJuniorPuppetCannotLiftASeniorsBan() { + // The puppet sits at position 2 and alice at 1, and no edition may claim a position at or + // above its own signer — so her delegation chain can only ever descend. Nothing she mints + // can outrank her, and so nothing she mints can unban her. + val editions = community() + ownerBansAlice + aliceMintsAPuppet() + banlist(puppet, 1, ownerBansAlice.hash) + + assertTrue(AuthorityResolver.resolve(editions, owner).isBanned(alice), "the puppet does not outrank its creator") + } +} From fb7c710a88a6940ff783dbbd92025e4d326e312b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 21:11:54 +0000 Subject: [PATCH 02/13] test(geode): pin that a Concord plane key cannot delete the channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A soft ban leaves the community_root in the ex-member's hands, so they keep deriving the channel's stream key. CORD-01 signs every wrap with that shared key rather than with the author, so on the wire a Concord channel looks like a single author publishing everything — and NIP-09/NIP-62 authorize on the outer pubkey. Read naively that hands any ex-member a one-event wipe of the whole community's history, and geode's own Nip09DeletionTest guarantee ("a kind-5 from pubkey X cannot delete pubkey Y's events") would be vacuous inside a plane. It is refused, but only because of a rule written for something else: Event.owner() gives a kind-1059 to its p-tag RECIPIENT rather than its signer, and ConcordStreamEnvelope stamps a freshly random p-tag on every wrap. Each wrap is therefore owned by a one-time key nobody holds, attacker included. Neither half was written with this attack in mind and either one silently re-opens it, so both are pinned: two tests fail if ownership ever moves back to the signer, and a counterfactual (a wrap addressed to a real key IS deletable by its holder) fails the moment that p-tag becomes anything a member holds. Scope: this is our relay's rule, not the protocol's. A third-party relay that authorizes deletion by matching pubkey still hands every ex-member a wipe button, and a Refounding only protects the future. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- .../geode/ConcordPlaneKeyDeletionTest.kt | 242 ++++++++++++++++++ 1 file changed, 242 insertions(+) create mode 100644 geode/src/test/kotlin/com/vitorpamplona/geode/ConcordPlaneKeyDeletionTest.kt diff --git a/geode/src/test/kotlin/com/vitorpamplona/geode/ConcordPlaneKeyDeletionTest.kt b/geode/src/test/kotlin/com/vitorpamplona/geode/ConcordPlaneKeyDeletionTest.kt new file mode 100644 index 0000000000..15c5fcd7a1 --- /dev/null +++ b/geode/src/test/kotlin/com/vitorpamplona/geode/ConcordPlaneKeyDeletionTest.kt @@ -0,0 +1,242 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.geode + +import com.vitorpamplona.quartz.concord.cord03Channels.ChannelChat +import com.vitorpamplona.quartz.concord.cord03Channels.ConcordChannelKeys +import com.vitorpamplona.quartz.concord.envelope.ConcordStreamEnvelope +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import com.vitorpamplona.quartz.nip01Core.relay.client.NostrClient +import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.publishAndConfirm +import com.vitorpamplona.quartz.nip01Core.relay.client.reqs.SubscriptionListener +import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer +import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal +import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerSync +import com.vitorpamplona.quartz.nip09Deletions.DeletionEvent +import com.vitorpamplona.quartz.nip62RequestToVanish.RequestToVanishEvent +import com.vitorpamplona.quartz.utils.RandomInstance +import com.vitorpamplona.quartz.utils.TimeUtils +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * Why a soft-banned member cannot delete a Concord community's history from the relay — and what + * keeps it that way. + * + * The worry is real. CORD-01 inverts NIP-59: every wrap on a plane is signed by the *shared stream + * key*, not by its author, and the true author only exists inside the encrypted seal. A member + * banned yesterday still derives `group_key("concord/channel", community_root, channel_id, epoch)` + * from the root they kept, so they can still sign events *as the channel itself*. If NIP-09 and + * NIP-62 authorized on the outer `pubkey`, [Nip09DeletionTest]'s "a kind-5 from pubkey X cannot + * delete pubkey Y's events" would be vacuous inside a plane: one event from any ex-member would + * erase the whole community's history. + * + * What stops it is [com.vitorpamplona.quartz.nip01Core.store.owner]: a kind-1059 gift wrap is + * controlled by its **p-tag recipient**, not its signer. Concord stamps a *freshly random* p-tag on + * every wrap ([ConcordStreamEnvelope.wrapSeal]), so each wrap is owned by a one-time key that + * nobody — attacker, author, or owner — ever holds. The channel is undeletable by construction. + * + * Both halves of that are load-bearing and neither was written for this reason, so both are pinned + * here: [theEphemeralPTagIsWhatMakesTheChannelUndeletable] fails the moment the p-tag becomes a + * real key, and the first two tests fail the moment ownership goes back to the signer. + * + * **Scope.** This is our relay's rule, not the protocol's. A community publishes wherever its + * metadata points, and a third-party relay that reads NIP-09 the naive way — deletion authorized by + * matching `pubkey` — hands every ex-member a wipe button for the whole channel. The protocol-level + * fix is the same one that already exists for everything else: a CORD-06 Refounding rotates the + * plane address, which protects the future but cannot restore what a relay already dropped. + */ +class ConcordPlaneKeyDeletionTest { + private lateinit var hub: InProcessRelays + private lateinit var scope: CoroutineScope + private lateinit var client: NostrClient + private val relayUrl: NormalizedRelayUrl = RelayUrlNormalizer.normalize("ws://127.0.0.1:7770/") + + @BeforeTest + fun setup() { + hub = InProcessRelays() + scope = CoroutineScope(Dispatchers.Default + SupervisorJob()) + client = NostrClient(hub, scope) + } + + @AfterTest + fun teardown() { + client.disconnect() + scope.cancel() + hub.close() + } + + private suspend fun query(filter: Filter): List { + val ch = Channel(Channel.UNLIMITED) + val subId = "sub-${System.nanoTime()}" + client.subscribe( + subId, + mapOf(relayUrl to listOf(filter)), + object : SubscriptionListener { + override suspend fun onEvent( + event: Event, + isLive: Boolean, + relay: NormalizedRelayUrl, + forFilters: List?, + ) { + ch.trySend(Msg.Ev(event)) + } + + override fun onEose( + relay: NormalizedRelayUrl, + forFilters: List?, + ) { + ch.trySend(Msg.Eose) + } + }, + ) + val events = mutableListOf() + withTimeout(5000) { + while (true) { + when (val msg = ch.receive()) { + is Msg.Ev -> events += msg.event + Msg.Eose -> return@withTimeout + } + } + } + client.unsubscribe(subId) + return events + } + + private sealed interface Msg { + data class Ev( + val event: Event, + ) : Msg + + object Eose : Msg + } + + /** A public channel plane: derived from the community root, so every member holds its secret. */ + private val communityRoot = RandomInstance.bytes(32) + private val channelId = RandomInstance.bytes(32) + private val plane = ConcordChannelKeys.publicChannel(communityRoot, channelId, rootEpoch = 0) + + /** The plane's own signer — what the banned member reconstructs from the root they kept. */ + private fun planeSigner() = NostrSignerSync(KeyPair(privKey = plane.secretKey)) + + private suspend fun postAs( + author: NostrSignerInternal, + text: String, + createdAt: Long, + ): Event { + val rumor = ChannelChat.message(author.pubKey, channelId.toHexKey(), epoch = 0, text = text, createdAt = createdAt) + return ConcordStreamEnvelope.wrap(rumor, plane, author, encrypted = true, createdAt = createdAt) + } + + @Test + fun aPlaneKeyHolderCannotDeleteTheChannelsHistory() = + runBlocking { + val now = TimeUtils.now() + val bob = NostrSignerInternal(KeyPair()) + val carol = NostrSignerInternal(KeyPair()) + + val history = + listOf( + postAs(bob, "hello", now), + postAs(carol, "hi bob", now + 1), + postAs(bob, "how's the project going?", now + 2), + ) + history.forEach { assertEquals(true, client.publishAndConfirm(it, setOf(relayUrl)), "seed the channel history") } + assertEquals(3, query(Filter(authors = listOf(plane.publicKeyHex))).size, "three messages on the plane") + + // The banned member still derives `plane`, and every wrap above IS authored by it — so + // this kind-5 satisfies a same-author check. It must still be refused. + val deletion = planeSigner().sign(DeletionEvent.build(history, createdAt = now + 10)) + assertEquals(true, client.publishAndConfirm(deletion, setOf(relayUrl)), "the relay accepts the event itself") + + assertEquals( + 3, + query(Filter(authors = listOf(plane.publicKeyHex), kinds = listOf(ConcordStreamEnvelope.KIND_WRAP))).size, + "signing as the plane must NOT delete the community's messages", + ) + } + + @Test + fun aPlaneKeyHolderCannotVanishTheChannelPlane() = + runBlocking { + val now = TimeUtils.now() + val bob = NostrSignerInternal(KeyPair()) + + val history = listOf(postAs(bob, "one", now), postAs(bob, "two", now + 1)) + history.forEach { client.publishAndConfirm(it, setOf(relayUrl)) } + assertEquals(2, query(Filter(authors = listOf(plane.publicKeyHex))).size) + + // NIP-62 needs no per-event targeting: one event, and everything that pubkey published + // is gone. The sharpest version of the attack, and the same rule has to stop it. + val vanish = planeSigner().sign(RequestToVanishEvent.build(relayUrl, "", createdAt = now + 10)) + assertEquals(true, client.publishAndConfirm(vanish, setOf(relayUrl))) + + assertEquals( + 2, + query(Filter(authors = listOf(plane.publicKeyHex), kinds = listOf(ConcordStreamEnvelope.KIND_WRAP))).size, + "a kind-62 signed as the plane must not wipe the channel", + ) + } + + @Test + fun theEphemeralPTagIsWhatMakesTheChannelUndeletableSoDoNotMakeItMeaningful() = + runBlocking { + // The counterfactual, so the invariant is visible rather than incidental: ownership of a + // 1059 follows the p-tag, so a wrap addressed to a REAL key is deletable by whoever holds + // that key. Concord is safe only because `wrapSeal` stamps a fresh throwaway pubkey there. + // If that p-tag ever becomes something a member holds — a recipient, a channel id, a + // community id — every ex-holder of it can delete the plane's history. + val now = TimeUtils.now() + val mallory = NostrSignerInternal(KeyPair()) + + val addressedWrap = + planeSigner().signNormal( + now, + ConcordStreamEnvelope.KIND_WRAP, + arrayOf(arrayOf("p", mallory.pubKey)), + "not-a-real-seal", + ) + assertEquals(true, client.publishAndConfirm(addressedWrap, setOf(relayUrl))) + assertEquals(1, query(Filter(ids = listOf(addressedWrap.id))).size) + + val deletion = mallory.sign(DeletionEvent.build(listOf(addressedWrap), createdAt = now + 1)) + assertEquals(true, client.publishAndConfirm(deletion, setOf(relayUrl))) + + assertEquals( + 0, + query(Filter(ids = listOf(addressedWrap.id))).size, + "the p-tag recipient owns a 1059 — which is exactly why Concord's p-tag must stay random", + ) + } +} From 41a035034ba514f9964b8e1e1d1f6731715c789e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 21:30:14 +0000 Subject: [PATCH 03/13] test(quartz): pin the hand-crafted routes out of a Concord ban MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ban/unban verb is not the threat model — a malicious client writes editions directly, so what matters is which routes the FOLD refuses. Three more, all of them ones the UI would never author. Two are refused, and it is worth pinning why, because neither is refused by the rule you would expect. Removing yourself from the banlist is caught by the delta rule's strict outranking (nobody outranks themselves), so the sharper attempt does not remove anything: it forks the banlist at genesis, or builds a private chain, that simply never mentions him, at a version high enough to win the head fold. There is then nothing to remove and the rank rule never fires. What catches it is CORD-04 §4's re-heal — the owner's edition is not on the forged head's back-chain, so it is unioned back in as a concurrent ban. The union is load-bearing security here, not just convergence. The third works. A §3 compaction re-wraps one edition per entity and the ROTATOR picks it, so a rotator can decline to carry the banlist forward; every edition it serves is genuine and no signature check can see the omission. A banned member cannot rotate — drainConcordRekeys gates the rotator on the ban-aware hasPermission — but the puppet from the previous commit is not banned and can. EntityFloor is the entire defense, so the community splits: clients that already folded the ban refuse the rollback, fresh joiners have no floor and see no ban. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- .../cord04Roles/BannedStaffEscalationTest.kt | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt index 8ad22aba6b..aedadd885a 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt @@ -273,4 +273,54 @@ class BannedStaffEscalationTest { assertTrue(AuthorityResolver.resolve(editions, owner).isBanned(alice), "the puppet does not outrank its creator") } + + @Test + fun aForkedBanlistThatOmitsHimCannotLaunderTheBanAway() { + // A malicious client is not limited to what the ban/unban verb will author. The sharpest + // hand-crafted route does not try to REMOVE his ban — removal is what the strict-outrank-self + // rule guards — it forks at genesis and simply never mentions him, at a version high enough + // to win the head fold. The head's own effective list then never carried his ban, so there is + // nothing to remove and the rank rule never fires. + // + // §4's re-heal is what closes it: the owner's edition is authorized and is NOT on the forked + // head's back-chain, so it is unioned back in as a concurrent ban. + val editions = community() + ownerBansAlice + banlist(alice, 99, null, carol) + + assertTrue(AuthorityResolver.resolve(editions, owner).isBanned(alice), "the re-heal union must put the owner's ban back") + } + + @Test + fun aPrivateBanlistChainOfHisOwnCannotLaunderTheBanAway() { + // The same idea two editions deep, so the winning head has a clean ancestry entirely of his + // own making. Ancestry is walked over the full pool, so the owner's ban is still recognised + // as a concurrent fork rather than a superseded ancestor. + val mine = banlist(alice, 50, null) + val editions = community() + ownerBansAlice + mine + banlist(alice, 51, mine.hash) + + assertTrue(AuthorityResolver.resolve(editions, owner).isBanned(alice), "a self-authored chain must not launder the ban away") + } + + @Test + fun aRogueRotatorCompactsTheBanAwayForEveryClientWithoutAFloor() { + // The route that does work, and the one no signature check can catch. A CORD-06 §3 compaction + // re-wraps ONE edition per entity and the ROTATOR picks it, so a rotator can simply not carry + // the banlist forward. Every edition it serves is genuine; the ban is erased by omission. + // + // A banned member cannot rotate (drainConcordRekeys gates the rotator on hasPermission, which + // is ban-aware) — but the puppet minted above is not banned, and it can. EntityFloor is the + // whole defense, so this splits the community in two: clients that already folded the ban + // refuse the rollback, while fresh joiners have no floor to refuse with and see no ban at all. + val editions = community() + ownerBansAlice + val floors = ConcordCommunityState.authorizedHeads(editions, owner) + val compacted = editions.filter { it.entityKind != ControlEntityKind.BANLIST } + + assertFalse( + ConcordCommunityState.fold(compacted, owner).authority.isBanned(alice), + "ESCALATION: a fresh joiner holds no floor, so the omitted ban simply never existed", + ) + assertTrue( + ConcordCommunityState.fold(compacted, owner, floors).authority.isBanned(alice), + "a client that already folded the ban must refuse the rollback", + ) + } } From 1e6cda712dc965575b39ea0553dac7887f9af3da Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 23:06:54 +0000 Subject: [PATCH 04/13] docs(concord): audit the soft-ban and Control Plane attack surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collects the findings from this branch into docs/concord-soft-ban-audit.md, each marked Verified (a test reproduces it, named) or Read (follows from the code, untested), with a suggested order of attack. Adds the reproduction for the one finding that was still unverified, and it did not hold up the way it was first described. Version inflation does not poison the anti-rollback floor through the chain walk — that walk advances only to head.version + 1 citing the head's hash, so a fresh joiner is untouched. It goes through the COMPACTION ARM: once a client holds a floor and the entity is in the epoch snapshot, the head comes from bootstrapHead, which is highest-version at or above the floor with no prev, no hash and no contiguity. Version is then the whole contest and Long.MAX_VALUE wins it permanently — the floor rises to MAX_VALUE, no honest edition can exceed it, and a Refounding that drops the poison falls back to EntityFloor.known, which is the poison. That makes it the worst item on the list: unrecoverable, and authored in the tests by a current, legitimately granted moderator — no ban, no sockpuppet, one ordinary permission bit. compactControlPlane picks per entity by raw max version too, so honest rotators carry it into every future epoch. The banlist escapes only because AuthorityResolver folds it on a floor-less chain walk and re-heals the union, so an honest ban still lands. That accident is all that separates this from a permanently unmoderatable community, so it is pinned by its own test. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- docs/concord-soft-ban-audit.md | 230 ++++++++++++++++++ .../ControlPlaneVersionExhaustionTest.kt | 167 +++++++++++++ 2 files changed, 397 insertions(+) create mode 100644 docs/concord-soft-ban-audit.md create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/ControlPlaneVersionExhaustionTest.kt diff --git a/docs/concord-soft-ban-audit.md b/docs/concord-soft-ban-audit.md new file mode 100644 index 0000000000..239258ff40 --- /dev/null +++ b/docs/concord-soft-ban-audit.md @@ -0,0 +1,230 @@ +# Concord: soft-ban and Control Plane audit + +**Scope:** what a removed member — or a moderator who turns — can still do to a Concord community, +assuming a **malicious client** (no client-side rule binds them; only cryptography, the fold, and +the relay do). +**Date:** 2026-08-08. **Status:** findings only, nothing fixed yet. +**Companion:** `docs/concord-banlist-rank-conformance.md` (the rank half of CORD-04 §4, already +reported to Armada and fixed here). + +Each finding says how it was established. **Verified** means a test in this repo reproduces it; +**Read** means it follows from the code but no test was written. Every "Verified" line names the +test. + +--- + +## Summary + +| # | Finding | Severity | Needs a ban? | Recoverable? | +|---|---------|----------|--------------|--------------| +| [V1](#v1) | One edition at `version = Long.MAX_VALUE` pins an entity forever | **Critical** | No — any bit-holder | **No** | +| [V2](#v2) | A banned staffer keeps Role/Grant/Banlist authority | **Critical** | Yes | Yes (Refounding) | +| [V3](#v3) | A rogue rotator compacts the banlist away | High | Via V2 | Partly | +| [V4](#v4) | The Refounding recipient set is attacker-inflatable | High | No | Yes | +| [V5](#v5) | The ban is a per-pubkey display rule; the channel key is not revoked | High | Yes | Yes (Refounding) | +| [V6](#v6) | Channel history is deletable on a naive third-party relay | High | Yes | **No** (history) | +| [V7](#v7) | Banlist rank rule diverges from Armada | Medium | — | — | +| [V8](#v8) | A soft ban revokes no read access and no live invite | Medium | Yes | Yes (Refounding) | +| [V9](#v9) | The base-rekey plane is writable by every member | Low | Yes | Yes | + +The two structural causes worth naming up front, because most of the list collapses into them: + +- **Authority is checked in two places that disagree.** `ConcordCommunityState.fold` gates + METADATA/CHANNEL/INVITE through `authority.hasPermission` (`!isBanned && …`), while ROLE, GRANT + and BANLIST are gated *inside* `AuthorityResolver.resolve` by `holdsManageRoles` / `bitsOf` / + `effectivePermissionsOf`, none of which consult the banlist. That is V2, and V3 follows from it. +- **A ban removes standing, never keys.** Everything a member holds — `community_root`, channel + keys, `control_root` if staff, live invite links — survives it. Only a CORD-06 Refounding rotates + those, which is why V4 (making Refounding expensive) is worth more to an attacker than it looks. + +--- + +## V1 — One edition at `Long.MAX_VALUE` pins an entity forever + +**Critical. Does not require a banned user, a sockpuppet, or the owner's absence. Unrecoverable.** + +*Verified:* `quartz/…/cord04Roles/ControlPlaneVersionExhaustionTest.kt` (3 tests). + +Any current holder of an entity's permission bit publishes one edition at `version = +Long.MAX_VALUE`. For every client that holds an `EntityFloor` for that entity, that edition becomes +the permanent head: + +1. it wins, so the entity shows the attacker's content; +2. `authorizedHeads` raises the entity's floor to `Long.MAX_VALUE`; +3. no honest edition can ever exceed that floor, so the entity can never be repaired; +4. a Refounding that drops the poison does not help — nothing is offered at or above the floor, the + fold reports a gap, and falls back to `EntityFloor.known`, which *is* the poison. + +The chain walk is not the weakness (it advances only to `head.version + 1` citing the head's hash, +so a fresh joiner is unaffected). The weakness is the **compaction arm** of `EditionFold.foldEntity`: +once a floor exists and the entity is in the epoch snapshot — which `fold` always builds from the +editions handed to it — the head comes from `bootstrapHead`, i.e. *highest version at or above the +floor*, with no `prev`, no hash, no contiguity. Version becomes the whole contest. + +Concretely: a moderator with `MANAGE_CHANNELS` deletes `#general` permanently for everyone; one +with `MANAGE_METADATA` renames the community permanently. Demoting or banning them afterwards +changes nothing — the damage is in every client's floor. `ConcordRefounding.compactControlPlane` +also selects the head per entity by raw highest version, ungated, so an honest rotator carries the +poison into every future epoch, where fresh joiners then anchor on it as their baseline. + +The banlist survives, by accident: `AuthorityResolver` folds it on its own floor-less chain walk and +re-heals the union across authorized editions, so an honest ban still lands. That accident is the +only thing separating this from a permanently unmoderatable community, and it is now pinned by +`aPoisonedBanlistStillAcceptsTheOwnersBan`. + +**Fix direction.** The compaction arm needs a bound, since it is the arm that trades contiguity for +cross-epoch tolerance. Options, roughly in order of preference: + +- Cap the version delta the arm will accept in one step (a compacted head is legitimately ahead of + the floor, but by a chain's worth, not by 2^63). Anything above the cap is a gap, not a head. +- Make `bootstrapHead` prefer the highest version *reachable by a chain* among the offered editions, + falling back to raw version only when no chain connects. +- Have `compactControlPlane` select the authority-gated fold head rather than raw max version, so a + poison is at least not propagated by honest rotators. + +The first is the smallest change and closes the unrecoverability; the third should happen regardless. + +## V2 — A banned staffer keeps Role, Grant and Banlist authority + +**Critical.** *Verified:* `quartz/…/cord04Roles/BannedStaffEscalationTest.kt` (13 tests). + +`hasPermission` is ban-aware; the resolver's internal gates are not, and structurally cannot be as +written — the roles/grants fixpoint settles before `banned` is computed. So a banned member who +still holds `control_root` keeps the roster. In the reproduction they: + +- ban every member they outrank, directly, with no puppet; +- revoke the surviving moderators' grants and retire the roles beneath them; +- **mint a fresh, unbanned npub** at the next position down, which then passes every ban-aware gate: + deletes every channel, rewrites the metadata, bans the rest of the community, creates invites; +- and, because `drainConcordRekeys` authorizes a rotator by `hasPermission(rotator, BAN)`, that + puppet can publish a Refounding omitting the owner — every honest client follows it and the owner + is stranded on a dead root. + +Self-unban is *not* reachable and neither is a puppet-unban: the delta rule gates removals and +strict outranking means nobody outranks themselves, while no edition may claim a position at or +above its signer, so the delegation chain only descends. Two hand-crafted attempts that avoid +removal entirely — forking the banlist at genesis, and building a private chain — are also refused, +by CORD-04 §4's re-heal union rather than by the rank rule. **The union is load-bearing security +here, not just convergence.** + +**Fix direction.** Make the resolver's gates ban-aware. The ordering problem is real (you cannot +know who is banned before folding the banlist, nor who may write it before knowing who is banned), +so resolve it as a bounded two-pass where authority only ever *shrinks*: pass A settles the roster +as today and computes the banlist; pass B re-resolves roles/grants dropping editions whose author is +banned in pass A; then recompute the banlist under pass B's roster, keeping only bans still +authorized. Deterministic, terminates, no oscillation on mutual bans. **Consensus-affecting**: until +Armada ships the same rule, we will drop editions they honor. + +## V3 — A rogue rotator compacts the banlist away + +**High.** *Verified:* `aRogueRotatorCompactsTheBanAwayForEveryClientWithoutAFloor`. + +A CORD-06 §3 compaction re-wraps one edition per entity and the *rotator* picks it, so a rotator can +decline to carry the banlist forward. Every edition it serves is genuine, so no signature check sees +the omission — `EntityFloor`'s own KDoc names this case ("clearing a banlist"). A banned member +cannot rotate, but the V2 puppet can. + +The result is not a clean unban but a **split community**: clients that already folded the ban +refuse the rollback and still see it, fresh joiners have no floor and see no ban at all. Two +populations permanently disagreeing about who is a member, with no event either side can call +forged. Closing V2 removes the puppet and takes this with it; floors alone do not, since they only +protect people who were already there. + +## V4 — The Refounding recipient set is attacker-inflatable + +**High.** *Read:* `ConcordCommunitySession.allMembers()` / `emitChannelRumors`; +`AccountConcordActions.refoundConcordCommunity` step 2; `ConcordRefounding.buildBaseRekeyWraps`. + +`allMembers()` = Guestbook joins ∪ `observedAuthors` ∪ roster ∪ owner, and it *is* the Refounding +recipient set. Both contributing sets are unbounded and both are attacker-writable: Guestbook joins +are self-signed (any key, no authority), and every author we decrypt is folded into +`observedAuthors` by design (CORD-02 §5, "observably present"). + +So each throwaway npub an attacker posts from, or announces, is one more mandatory NIP-44 blob in +the next Refounding, chunked 120 per event. 100k identities ⇒ ~100k encryptions and ~830 published +events — while they keep posting. **The attack inflates the cost of its own remedy**, and the remedy +is the only hard removal Concord has. + +This is the cheapest thing on the list to fix and the only one that is not consensus-affecting: cap +the recipient set, prefer recent/attested members when over the cap, and surface what was dropped +(a silent truncation strands real members). Worth doing first. + +## V5 — The ban is a per-pubkey display rule and the channel key is not revoked + +**High.** *Read:* `Account.consumeConcordRumorGated` (`isBanned(rumor.pubKey)`), `Account.isAcceptable`. + +Writing to a channel needs the channel key, which the ban does not take away; the seal author is +whatever key the client feels like using. A malicious client therefore posts every message from a +fresh npub and `isBanned` never matches — moderation is whack-a-mole against an infinite identity +supply. Each message also costs every member two NIP-44 decrypts and two signature verifications +*before* the banlist check runs, and each fresh author inflates V4. + +There is no client-side answer; only a Refounding rotates the key out from under them. That is the +correct design, which is why V4 matters so much. + +## V6 — Channel history is deletable on a naive third-party relay + +**High, external.** *Verified (that we are safe):* +`geode/…/ConcordPlaneKeyDeletionTest.kt` (3 tests). + +CORD-01 signs every wrap with the shared stream key, so on the wire a Concord channel is one author +publishing everything — and every member holds that author's secret. NIP-09 and NIP-62 authorize on +the outer `pubkey`. Read the obvious way, that hands any ex-member a one-event wipe of the whole +community's history, and geode's own guarantee ("a kind-5 from pubkey X cannot delete pubkey Y's +events") is vacuous inside a plane. + +**On our relay it is refused, but only because of a rule written for something else:** +`Event.owner()` gives a kind-1059 to its *p-tag recipient* rather than its signer, and +`ConcordStreamEnvelope` stamps a freshly random p-tag on every wrap, so each wrap is owned by a +one-time key nobody holds. Both halves are load-bearing, neither was written for this, and either +one silently re-opens the hole — all three are now pinned, including a counterfactual showing a wrap +addressed to a *real* key is deletable by its holder. + +A community publishes wherever its metadata points. Any relay that authorizes deletion by matching +`pubkey` still hands every ex-member the wipe button, and a Refounding protects only the future. +Worth a note in the CORD-01 spec and a line in the relay-selection guidance. + +## V7 — Banlist rank rule diverges from Armada + +**Medium, known, deliberate.** See `docs/concord-banlist-rank-conformance.md`, already reported. + +We enforce §3's rank half on the Banlist and Armada does not, so the two clients can show different +banlists. Shipped knowingly. Row 3 of that report ("a banned `BAN` holder unbans themselves") was +left open as a fixpoint-ordering question — V2 is the general form of it, and the fix proposed there +resolves both. + +## V8 — A soft ban revokes no read access and no live invite + +**Medium, inherent.** *Read:* CORD-02/05. + +Until a Refounding, a banned member decrypts everything published — the ban only stops honest +clients from *showing* their posts, not from delivering the group's posts to them. They also keep +any invite links they created while privileged; those still resolve to bundles carrying the current +root. Publishing the root, or one live link, invites an unbanned crowd that each has to be banned +individually (and see V5). + +Not a bug so much as the definition of a soft ban, but it belongs on the list because the UI should +say so: "Ban" and "Remove from community" are very different promises and users will read the first +as the second. + +## V9 — The base-rekey plane is writable by every member + +**Low.** *Read:* `ConcordKeyDerivation.baseRekeyAddress`, `AccountConcordActions.drainConcordRekeys`. + +The base-rekey address derives from `community_root`, so any member — banned included — can mint +valid wraps there. Authorization happens after the blobs are scanned, so a flood costs every member +a locator scan per blob on every revision tick. Bounded work per wrap and no correctness impact; +listed for completeness. + +--- + +## Suggested order + +1. **V4** — cheapest, not consensus-affecting, and it protects the remedy every other fix depends on. +2. **V1** — worst blast radius and the only unrecoverable one; does not need an attacker to be + banned or privileged beyond a single ordinary bit. +3. **V2** (+V3, +V7 row 3) — one two-pass change closes all three. Coordinate with Armada first; + this one splits consensus. +4. **V6** — spec note + relay guidance; our own behaviour is already correct and now pinned. +5. **V5 / V8** — UI honesty about what a ban does, and a "Remove from community" affordance that + Refounds rather than bans. diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/ControlPlaneVersionExhaustionTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/ControlPlaneVersionExhaustionTest.kt new file mode 100644 index 0000000000..bbba1dc8d3 --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/ControlPlaneVersionExhaustionTest.kt @@ -0,0 +1,167 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.concord.cord04Roles + +import com.vitorpamplona.quartz.concord.cord02Community.ConcordCommunityState +import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * **V1 in `docs/concord-soft-ban-audit.md` — reproduction.** A single Control Plane edition at + * `version = Long.MAX_VALUE` pins its entity to the author's content permanently, for every client + * that holds a floor for it. + * + * The chain walk is not the weakness — it advances only to `head.version + 1` citing the head's + * hash, so an inflated version is unreachable and a fresh joiner is unaffected. The weakness is the + * **compaction arm** of [EditionFold.foldEntity]: once a client holds a floor for an entity and that + * entity appears in the epoch snapshot (which `ConcordCommunityState.fold` always builds from the + * editions handed to it), the head is chosen by [EditionFold.bootstrapHead] — *highest version at or + * above the floor*, with no `prev`, no hash, and no contiguity. Version is then the whole contest, + * and `Long.MAX_VALUE` wins it forever: + * + * 1. the poison becomes the head, so the entity shows the attacker's content; + * 2. `authorizedHeads` raises the entity's floor to `Long.MAX_VALUE`; + * 3. no honest edition can ever exceed that floor, so the entity can never be repaired; + * 4. a Refounding that drops the poison does not help either — nothing is offered at or above the + * floor, so the fold reports a gap and falls back to [EntityFloor.known], which *is* the poison. + * + * Note who the attacker is. Every test here is authored by **bob, a current and legitimately granted + * moderator** — not a banned member, not a sockpuppet. Any holder of the entity's permission bit can + * do this at any time, and demoting or banning them afterwards changes nothing, because the damage + * is already in every client's floor. It is also carried into every future epoch by + * `ConcordRefounding.compactControlPlane`, which selects the head per entity by raw highest version. + * + * The banlist is the one entity that survives, and by accident: `AuthorityResolver` folds it with + * its own floor-less chain walk and then re-heals the union across authorized editions, so an + * honest ban lands even when the head is poisoned. [aPoisonedBanlistStillAcceptsTheOwnersBan] pins + * that, because it is the only thing standing between this bug and a permanently unmoderatable + * community. + */ +class ControlPlaneVersionExhaustionTest { + private val owner = "0f".repeat(32) + private val bob = "b2".repeat(32) + + private val modRole = "22".repeat(32) + private val metadataEntity = "66".repeat(32) + private val channelEntity = "55".repeat(32) + private val banlistEntity = "44".repeat(32) + + private fun edition( + kind: ControlEntityKind, + entity: String, + version: Long, + prev: ByteArray?, + content: String, + author: String, + rumorId: String, + ) = ControlEdition(kind, entity.hexToByteArray(), version, prev, null, content, author, rumorId, 0) + + /** bob holds exactly one bit, granted by the owner, entirely legitimately. */ + private fun communityWhereBobHolds( + permissions: String, + vararg rest: ControlEdition, + ) = listOf( + edition(ControlEntityKind.ROLE, modRole, 0, null, """{"name":"Mod","position":5,"permissions":"$permissions"}""", owner, "role-mod"), + edition(ControlEntityKind.GRANT, "32".repeat(32), 0, null, """{"member":"$bob","role_ids":["$modRole"]}""", owner, "grant-bob"), + ) + rest + + @Test + fun oneEditionAtMaxVersionPinsTheMetadataForever() { + val metadataV0 = edition(ControlEntityKind.METADATA, metadataEntity, 0, null, """{"name":"My Community"}""", owner, "meta-0") + val community = communityWhereBobHolds(ConcordPermissions.of(ConcordPermissions.MANAGE_METADATA).toWire(), metadataV0) + + // A client that has folded this community once holds a floor for the metadata entity. + val floorsBefore = ConcordCommunityState.authorizedHeads(community, owner) + assertEquals(0, floorsBefore[metadataEntity]?.version, "an ordinary floor at the genesis edition") + + val poison = edition(ControlEntityKind.METADATA, metadataEntity, Long.MAX_VALUE, metadataV0.hash, """{"name":"PWNED"}""", bob, "meta-poison") + val floorsAfter = ConcordCommunityState.authorizedHeads(community + poison, owner, floorsBefore) + assertEquals(Long.MAX_VALUE, floorsAfter[metadataEntity]?.version, "VULNERABLE: the floor is now at the top of the version space") + + // The owner tries to repair it, chaining honestly onto their own genesis. + val repair = edition(ControlEntityKind.METADATA, metadataEntity, 1, metadataV0.hash, """{"name":"My Community"}""", owner, "meta-1") + val pool = community + poison + repair + + assertEquals( + "My Community", + ConcordCommunityState.fold(pool, owner).metadata?.name, + "a fresh joiner walks the chain and is unaffected", + ) + assertEquals( + "PWNED", + ConcordCommunityState.fold(pool, owner, floorsAfter).metadata?.name, + "VULNERABLE: every client holding a floor is pinned to the attacker's content", + ) + assertEquals( + "PWNED", + ConcordCommunityState.fold(community + repair, owner, floorsAfter).metadata?.name, + "VULNERABLE: even a Refounding that drops the poison falls back to it as EntityFloor.known", + ) + } + + @Test + fun oneEditionAtMaxVersionDeletesAChannelForever() { + val channelV0 = edition(ControlEntityKind.CHANNEL, channelEntity, 0, null, """{"name":"general"}""", owner, "chan-0") + val community = communityWhereBobHolds(ConcordPermissions.of(ConcordPermissions.MANAGE_CHANNELS).toWire(), channelV0) + + val floorsBefore = ConcordCommunityState.authorizedHeads(community, owner) + val poison = edition(ControlEntityKind.CHANNEL, channelEntity, Long.MAX_VALUE, channelV0.hash, """{"name":"general","deleted":true}""", bob, "chan-poison") + val floorsAfter = ConcordCommunityState.authorizedHeads(community + poison, owner, floorsBefore) + + val repair = edition(ControlEntityKind.CHANNEL, channelEntity, 1, channelV0.hash, """{"name":"general"}""", owner, "chan-1") + val pool = community + poison + repair + + assertEquals(1, ConcordCommunityState.fold(pool, owner).channels.size, "a fresh joiner still sees the channel") + assertEquals(0, ConcordCommunityState.fold(pool, owner, floorsAfter).channels.size, "VULNERABLE: the channel is gone and cannot be restored") + assertEquals( + 0, + ConcordCommunityState.fold(community + repair, owner, floorsAfter).channels.size, + "VULNERABLE: dropping the poison does not bring the channel back", + ) + } + + @Test + fun aPoisonedBanlistStillAcceptsTheOwnersBan() { + // The saving grace, and the reason this bug is "unmoderatable community" rather than + // "community with a broken name". AuthorityResolver folds the banlist on its own floor-less + // chain walk and re-heals the union across every authorized edition, so the owner's ban lands + // even while the banlist's own floor sits at Long.MAX_VALUE. Do not "unify" the banlist onto + // the floored fold without replacing this protection. + val banlistV0 = edition(ControlEntityKind.BANLIST, banlistEntity, 0, null, "[]", owner, "ban-0") + val community = communityWhereBobHolds(ConcordPermissions.of(ConcordPermissions.BAN).toWire(), banlistV0) + + val floorsBefore = ConcordCommunityState.authorizedHeads(community, owner) + val poison = edition(ControlEntityKind.BANLIST, banlistEntity, Long.MAX_VALUE, banlistV0.hash, "[]", bob, "ban-poison") + val floorsAfter = ConcordCommunityState.authorizedHeads(community + poison, owner, floorsBefore) + assertEquals(Long.MAX_VALUE, floorsAfter[banlistEntity]?.version, "the banlist floor is poisoned like any other") + + val ownerBansBob = edition(ControlEntityKind.BANLIST, banlistEntity, 1, banlistV0.hash, """["$bob"]""", owner, "ban-1") + val pool = community + poison + ownerBansBob + + assertTrue(ConcordCommunityState.fold(pool, owner).authority.isBanned(bob), "a fresh joiner honors the ban") + assertTrue( + ConcordCommunityState.fold(pool, owner, floorsAfter).authority.isBanned(bob), + "the re-heal union must keep the banlist working even with a poisoned floor", + ) + } +} From d37e183a575ed9266f58a1404c953b4e89b3c068 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 01:28:22 +0000 Subject: [PATCH 05/13] docs(concord): audit the surfaces the first pass never opened MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first pass was bounded by the Control Plane, the fold and the relay. Three more findings from the surfaces it skipped, plus an explicit list of what is still unexamined so the next reader knows where the edges are. V10 is the serious one, and it forks. ConcordStrandedRecovery.isStranded takes only (entry, bundle): no banlist check, no check that we were legitimately re-keyed. The whole test is "the bundle at my stored invite_ref sits at a higher epoch than I do", and the unlock token lives in the link fragment an ex-member keeps forever. So whether a removed member walks back in depends only on whether anything re-mints at that coordinate. Amethyst mints a fresh link signer per invite and the Refounding neither re-mints nor revokes, so today nothing does — which means stranded recovery never fires for anyone, and the cure that drainConcordRekeys' KDoc points to for "a BAN-holder can evict anyone, the owner included, by omission" does not actually exist. If any client does re-mint at a stable coordinate, as CORD-05's design describes, then every removed member auto-recovers the new root on the 15-minute sweep and re-announces a Guestbook join. Either the safety net is missing or the only hard removal is undone; which one it is needs a spec answer, not a patch. V11: voice rooms authenticate with the channel's derived voice signer key against a stateless SFU that holds no community secret and cannot know a banlist exists, so a banned member keeps talking until a Refounding. V12: ingestTyping filters on binding and self only, so they keep showing as "typing". Checked and sound, recorded so they are not re-audited: the envelope pins rumor.pubKey == seal.pubKey (no author impersonation), and Note.latestConcordEdit is author-gated, so a member cannot rewrite someone else's message. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- docs/concord-soft-ban-audit.md | 81 ++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/docs/concord-soft-ban-audit.md b/docs/concord-soft-ban-audit.md index 239258ff40..8f284085ff 100644 --- a/docs/concord-soft-ban-audit.md +++ b/docs/concord-soft-ban-audit.md @@ -26,6 +26,9 @@ test. | [V7](#v7) | Banlist rank rule diverges from Armada | Medium | — | — | | [V8](#v8) | A soft ban revokes no read access and no live invite | Medium | Yes | Yes (Refounding) | | [V9](#v9) | The base-rekey plane is writable by every member | Low | Yes | Yes | +| [V10](#v10) | Stranded recovery: either broken, or a removal bypass | **Critical** | Yes | — | +| [V11](#v11) | Voice rooms are key-gated, not roster-gated | High | Yes | Yes (Refounding) | +| [V12](#v12) | Typing indicators are not ban-filtered | Low | Yes | Yes | The two structural causes worth naming up front, because most of the list collapses into them: @@ -216,8 +219,86 @@ valid wraps there. Authorization happens after the blobs are scanned, so a flood a locator scan per blob on every revision tick. Bounded work per wrap and no correctness impact; listed for completeness. +## V10 — Stranded recovery: either broken, or a removal bypass + +**Critical, and it forks — one of the two halves is true and both are bad.** +*Read:* `ConcordStrandedRecovery`, `AccountConcordActions.recoverStrandedConcordCommunities`, +`AccountConcordActions.mintConcordInvite`. + +`ConcordStrandedRecovery.isStranded` / `mergeForward` take only `(entry, bundle)`. There is **no +banlist check and no check that we were legitimately re-keyed** — the entire test is "the bundle at +my stored `inviteRef` sits at a higher epoch than I do". The unlock token lives in the link +fragment, which an ex-member keeps forever. So whether a removed member walks back in with the new +root depends *only* on whether the bundle at that coordinate ever advances an epoch. + +In Amethyst it never does: `mintConcordInvite` mints a **fresh link signer per mint**, so nothing +re-publishes at an existing coordinate, and `refoundConcordCommunity` does not re-mint or revoke +anything. Two consequences, and they are the fork: + +- **If nothing re-mints** — today's behaviour — then stranded recovery never fires *for anyone*. + That makes it dead code, and the cure `drainConcordRekeys`' own KDoc points to for "a BAN-holder + can evict anyone (the owner included) by omission" does not exist. An owner evicted by a rogue + admin has no way back. +- **If anything re-mints at a stable coordinate** — which is what CORD-05's design describes ("the + community keeps publishing its bundle at that same addressable coordinate, re-minted at the + current epoch"), so plausibly Armada in a cross-client community — then every removed member who + joined through a still-live link auto-recovers the new root on the 15-minute sweep, and + re-announces a Guestbook join so they look current again. **Refounding, the only hard removal, + is silently undone.** + +Note also that `refoundConcordCommunity` never revokes the invite links the removed member created +or joined through, even though `ControlEntityKind.INVITE_REVOKED` exists and `classifyInvite` +already honors it. + +**Fix direction.** Decide the intended semantics first — this needs a spec answer, not a patch. +Then: gate `mergeForward` on not being banned in the epoch we are merging *from*, have the +Refounding revoke the removed members' links, and either implement re-minting (so legitimate +recovery works) or drop the mechanism and give evicted owners a different route. + +## V11 — Voice rooms are key-gated, not roster-gated + +**High.** *Read:* `ConcordBrokerToken`, CORD-07 §2. + +A member proves voice-room membership by signing a NIP-98 kind-27235 request with the channel's +**derived voice signer key**, whose pubkey is the SFU room name. The broker is stateless and holds +no community secret, so it cannot consult the Control Plane and has no idea a banlist exists. A +banned member keeps that key until a Refounding, so they can join the voice room and stay in it. +Nothing on the client side can evict them — kicking them from the UI does not kick them from the SFU. + +This is the one place where a ban fails *audibly*, in real time, in front of everyone. Worth ranking +above its technical severity for that reason alone. + +## V12 — Typing indicators are not ban-filtered + +**Low.** *Read:* `ConcordCommunitySession.ingestTyping`. + +`ingestTyping` checks the rumor is a typing heartbeat, is bound to the channel/epoch, and is not our +own — and nothing else. A banned member (or any fresh npub holding the channel key, see V5) shows +in the "… is typing" row indefinitely. Cheap to fix and user-visible: the promise a ban makes is +that the member disappears, and here they do not. + --- +## What was NOT examined + +This audit is bounded by what was opened. Checked and found sound: the wrap/seal envelope (no author +impersonation — `rumor.pubKey == seal.pubKey` and `rumor.verifyId()`), Concord chat edits +(`Note.latestConcordEdit` is author-gated, so a member cannot rewrite someone else's message), and +self-unban (V2). + +Not looked at at all: + +- **Private channels** (CORD-03 derived keys) — key delivery on grant, and channel-scoped rekey. + Note that no channel-scoped rekey *receive* path appears to exist: `drainConcordRekeys` handles + `ROOT_SCOPE` only, and `entry.privateChannels` is carried forward but never populated by a + delivery path. If that is right, the only removal Amethyst can perform is a full-community + Refounding — which is exactly what V4 makes expensive. +- **In-plane reactions and deletes** — the edit path is author-gated; the delete path was not read. +- **Guestbook kicks** (kind 3309) — the builder documents a KICK-bit + rank rule; the receive side + was not verified against it. +- Unread counts and notification triggers, media/upload references from messages, the NIP-53 nests + overlap, and the desktop client's Concord paths. + ## Suggested order 1. **V4** — cheapest, not consensus-affecting, and it protects the remedy every other fix depends on. From f52a8b0432d3c4abd2ca10fa5b2b9fdfc51b5929 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 01:41:52 +0000 Subject: [PATCH 06/13] docs(concord): split the audit by what the attacker needs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-reviewed every finding against the shipping app rather than against the protocol, and split the list in two: what a banned user can do with stock Amethyst (our bugs) versus what needs a hand-written client (fix in the fold, or defend against). Several items moved, and the review turned up a new one that belongs at the top. A1 is new and is the realistic attack. mintConcordInvite checks only that the account is writeable and that we hold the community — no CREATE_INVITE, no banlist — and unlike the Edit and channel buttons next to it, the invite IconButton carries no guard at all. A banned user stays in the app, taps person-add, and shares a working link to the community. The mint publishes a fresh link signer, so revoking the links they were given does not touch the ones they make; and because the bundle is a standalone kind-33301 outside the Control Plane, the CREATE_INVITE bit the fold enforces on INVITE_* entities never applies to the actual invite mechanism. A3 is the general form: every moderation verb checks isWriteable() and the Control write key and nothing else, so authority lives in the composable that draws the button — and those gates use effectivePermissions, which is ban-blind. Ban and Remove survive only because a second, unrelated condition routes through the ban-aware canActOn. refoundConcordCommunity guards itself with effectivePermissions outright, so a banned BAN-holder can launch a Refounding from the shipping app; honest receivers refuse it, but that is a race against banlist propagation, not a check. A2 moves to Part A because our own client is what performs it: the recovery sweep runs every 15 minutes with no banlist check. C2 (voice) is downgraded from High — ConcordBrokerToken and VoicePresence are referenced nowhere outside quartz, so there is no shipping path to attack. It is a note for whoever wires one up. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- docs/concord-soft-ban-audit.md | 347 ++++++++++++++++++++++----------- 1 file changed, 233 insertions(+), 114 deletions(-) diff --git a/docs/concord-soft-ban-audit.md b/docs/concord-soft-ban-audit.md index 8f284085ff..58429394ca 100644 --- a/docs/concord-soft-ban-audit.md +++ b/docs/concord-soft-ban-audit.md @@ -1,48 +1,205 @@ # Concord: soft-ban and Control Plane audit -**Scope:** what a removed member — or a moderator who turns — can still do to a Concord community, -assuming a **malicious client** (no client-side rule binds them; only cryptography, the fold, and -the relay do). -**Date:** 2026-08-08. **Status:** findings only, nothing fixed yet. +**Scope:** what a removed member — or a moderator who turns — can still do to a Concord community. +**Date:** 2026-08-09. **Status:** findings only, nothing fixed yet. **Companion:** `docs/concord-banlist-rank-conformance.md` (the rank half of CORD-04 §4, already reported to Armada and fixed here). Each finding says how it was established. **Verified** means a test in this repo reproduces it; -**Read** means it follows from the code but no test was written. Every "Verified" line names the -test. +**Read** means it follows from the code but no test was written. Every "Verified" line names the test. + +--- + +## How to read this list + +Findings are split by **what the attacker needs**, because that decides who owns the fix and how +urgent it is: + +- **[Part A — reachable from stock Amethyst](#part-a).** A banned user opens the shipping app and + taps a button, or our own client does it for them on a timer. These are straightforwardly *our + bugs*, they need no attacker sophistication at all, and every one of them is fixable in this repo + without touching the protocol or coordinating with anyone. +- **[Part B — requires a malicious client](#part-b).** The attacker writes their own events, so no + client-side rule binds them. We cannot stop them from *authoring* anything; we can only refuse to + *honor* it. Fixes live in the fold, the store, or the spec. +- **[Part C — interop and not-yet-shipped surfaces](#part-c).** + +The distinction is not academic. Part A is where the realistic attacker is: an irritated user who +just got banned has the app already installed and is not going to write a Nostr client. Part B is +where the *damage ceiling* is. Fix Part A first because it is cheap and it is what will actually +happen; fix Part B because it is what ends communities. + +Two structural causes account for most of both halves: + +- **Authority is checked in several places that disagree.** `ConcordCommunityState.fold` gates + METADATA/CHANNEL/INVITE through the ban-aware `authority.hasPermission`. `AuthorityResolver` + gates ROLE/GRANT/BANLIST internally through `holdsManageRoles` / `bitsOf` / + `effectivePermissionsOf`, which are ban-blind. The **UI** gates through `effectivePermissions`, + also ban-blind. The **action layer** mostly does not gate at all. Same question, four answers. +- **A ban removes standing, never keys.** `community_root`, channel keys, `control_root` if staff, + and live invite links all survive it. Only a CORD-06 Refounding rotates those — which is why + anything that makes Refounding expensive (B4) or reversible (A2) is worth more to an attacker + than it first looks. --- ## Summary -| # | Finding | Severity | Needs a ban? | Recoverable? | -|---|---------|----------|--------------|--------------| -| [V1](#v1) | One edition at `version = Long.MAX_VALUE` pins an entity forever | **Critical** | No — any bit-holder | **No** | -| [V2](#v2) | A banned staffer keeps Role/Grant/Banlist authority | **Critical** | Yes | Yes (Refounding) | -| [V3](#v3) | A rogue rotator compacts the banlist away | High | Via V2 | Partly | -| [V4](#v4) | The Refounding recipient set is attacker-inflatable | High | No | Yes | -| [V5](#v5) | The ban is a per-pubkey display rule; the channel key is not revoked | High | Yes | Yes (Refounding) | -| [V6](#v6) | Channel history is deletable on a naive third-party relay | High | Yes | **No** (history) | -| [V7](#v7) | Banlist rank rule diverges from Armada | Medium | — | — | -| [V8](#v8) | A soft ban revokes no read access and no live invite | Medium | Yes | Yes (Refounding) | -| [V9](#v9) | The base-rekey plane is writable by every member | Low | Yes | Yes | -| [V10](#v10) | Stranded recovery: either broken, or a removal bypass | **Critical** | Yes | — | -| [V11](#v11) | Voice rooms are key-gated, not roster-gated | High | Yes | Yes (Refounding) | -| [V12](#v12) | Typing indicators are not ban-filtered | Low | Yes | Yes | +### Part A — reachable from stock Amethyst (our bugs) -The two structural causes worth naming up front, because most of the list collapses into them: +| # | Finding | Severity | Was | +|---|---------|----------|-----| +| [A1](#a1) | Any member — banned included — mints a working invite in one tap | **Critical** | new | +| [A2](#a2) | Stranded recovery runs on a timer and never checks the banlist | **Critical** | V10 | +| [A3](#a3) | The action layer has no permission checks; the UI's are ban-blind | High | new | +| [A4](#a4) | A banned member keeps broadcasting "typing", and we keep showing it | Low | V12 | +| [A5](#a5) | A banned member's own client keeps reading and rendering everything | Medium | V8 | -- **Authority is checked in two places that disagree.** `ConcordCommunityState.fold` gates - METADATA/CHANNEL/INVITE through `authority.hasPermission` (`!isBanned && …`), while ROLE, GRANT - and BANLIST are gated *inside* `AuthorityResolver.resolve` by `holdsManageRoles` / `bitsOf` / - `effectivePermissionsOf`, none of which consult the banlist. That is V2, and V3 follows from it. -- **A ban removes standing, never keys.** Everything a member holds — `community_root`, channel - keys, `control_root` if staff, live invite links — survives it. Only a CORD-06 Refounding rotates - those, which is why V4 (making Refounding expensive) is worth more to an attacker than it looks. +### Part B — requires a malicious client + +| # | Finding | Severity | Needs a ban? | Recoverable? | Was | +|---|---------|----------|--------------|--------------|-----| +| [B1](#b1) | One edition at `version = Long.MAX_VALUE` pins an entity forever | **Critical** | No — any bit-holder | **No** | V1 | +| [B2](#b2) | A banned staffer keeps Role/Grant/Banlist authority | **Critical** | Yes | Yes (Refounding) | V2 | +| [B3](#b3) | A rogue rotator compacts the banlist away | High | Via B2 | Partly | V3 | +| [B4](#b4) | The Refounding recipient set is attacker-inflatable | High | No | Yes | V4 | +| [B5](#b5) | The ban is per-pubkey; the channel key is not revoked | High | Yes | Yes (Refounding) | V5 | +| [B6](#b6) | Channel history is deletable on a naive third-party relay | High | Yes | **No** (history) | V6 | +| [B7](#b7) | The base-rekey plane is writable by every member | Low | Yes | Yes | V9 | + +### Part C — interop and not-yet-shipped + +| # | Finding | Severity | Was | +|---|---------|----------|-----| +| [C1](#c1) | Banlist rank rule diverges from Armada | Medium | V7 | +| [C2](#c2) | CORD-07 voice rooms are key-gated, not roster-gated | Design | V11 | --- -## V1 — One edition at `Long.MAX_VALUE` pins an entity forever +# Part A — reachable from stock Amethyst + +No custom tooling. A banned user with the shipping app, or our own background sweep. + +## A1 — Any member, banned included, mints a working invite in one tap + +**Critical. The single most likely thing an irritated banned user actually does.** +*Read:* `AccountConcordActions.mintConcordInvite`, `ConcordChannelListScreen` (the `PersonAdd` +`IconButton`). + +`mintConcordInvite` checks exactly two things: that the account is writeable, and that we have the +community in our joined list. **No `CREATE_INVITE` check. No banlist check.** And unlike the Edit +and channel-management buttons beside it, the invite `IconButton` is rendered with no `canEdit` +guard at all — it is always there, for everyone. + +So the flow is: get banned, stay in the app, tap the person-add icon, share the link. The minted +bundle carries the community root we still hold, so anyone who opens it joins for real. Every +invited account is a fresh unbanned npub that moderators then have to ban one at a time. + +Two aggravating details. The mint publishes a **fresh link signer per invite**, so it is a brand-new +coordinate — revoking the links the banned member was given does not touch the ones they mint. +And `CREATE_INVITE` is a real permission bit that the fold enforces on `INVITE_*` Control entities, +but the actual invite mechanism is a standalone kind-33301 addressable event published *outside* the +Control Plane, so that gate never applies to it. The permission is, in practice, unenforced. + +**Fix.** Gate `mintConcordInvite` on `hasPermission(me, CREATE_INVITE) || isOwner(me)`, and gate the +button on the same. This is contained, uncontroversial, and closes the realistic attack. Do it first. + +## A2 — Stranded recovery runs on a timer and never checks the banlist + +**Critical, and it forks.** *Read:* `ConcordStrandedRecovery`, +`AccountConcordActions.recoverStrandedConcordCommunities`, `AccountConcordActions.mintConcordInvite`. + +This is in Part A because **our own client performs it, unprompted**: the recovery sweep runs on the +revision tick for every joined community holding an `inviteRef`, every 15 minutes. The banned user +does nothing but leave the app installed. + +`ConcordStrandedRecovery.isStranded` / `mergeForward` take only `(entry, bundle)` — no banlist +check, no check that we were legitimately re-keyed. The whole test is "the bundle at my stored +`inviteRef` sits at a higher epoch than I do", and the unlock token lives in the link fragment an +ex-member keeps forever. So whether a removed member walks back in depends *only* on whether +anything re-mints at that coordinate: + +- **If nothing re-mints** — today, since Amethyst mints a fresh link signer per invite and the + Refounding neither re-mints nor revokes — stranded recovery never fires for anyone. It is dead + code, and the cure `drainConcordRekeys`' own KDoc points to for "a BAN-holder can evict anyone + (the owner included) by omission" does not exist. An owner evicted by a rogue admin has no way back. +- **If anything re-mints at a stable coordinate** — which is what CORD-05's design describes, so + plausibly Armada in a cross-client community — every removed member auto-recovers the new root and + re-announces a Guestbook join, looking current again. **The only hard removal is silently undone.** + +Note also that `refoundConcordCommunity` never revokes the links the removed member created or +joined through, though `ControlEntityKind.INVITE_REVOKED` exists and `classifyInvite` honors it. + +**Fix.** Decide the intended semantics first — this needs a spec answer. Then gate `mergeForward` on +not being banned in the epoch we merge *from*, have the Refounding revoke the removed members' +links, and either implement re-minting so legitimate recovery works, or drop the mechanism and give +evicted owners another route. + +## A3 — The action layer has no permission checks; the UI's are ban-blind + +**High (defense in depth).** *Read:* `AccountConcordActions` (`banConcordMember`, +`unbanConcordMember`, `editConcordMetadata`, `deleteConcordChannel`, `refoundConcordCommunity`), +`ConcordMembersScreen`, `ConcordChannelListScreen`. + +Every moderation verb checks `isWriteable()` and the Control write key, and **nothing else** — no +permission bit, no banlist. Authority lives entirely in the composable that draws the button. Two +consequences: + +1. **The UI's own gates are ban-blind.** `iCanBan`, `canEdit` (metadata) and `canManageChannels` all + use `effectivePermissions`, which ignores the banlist. A banned admin still sees the Edit and + channel-management controls. Those particular editions are dropped by every client's fold + (METADATA/CHANNEL are `hasPermission`-gated), so the result is a **silently no-op control** — + which this codebase elsewhere explicitly calls out as worse than no control at all. +2. **Ban/Remove survive only because of a second, unrelated gate.** `canBan` is + `viewerCanBan && canBanTarget`, and `canBanTarget` routes through `canActOn`, which *is* + ban-aware. Remove the second condition and a banned admin gets a working Ban button. That is a + thin margin for a Critical-severity outcome (B2). + +`refoundConcordCommunity` is the sharpest instance: its own guard is +`isOwner || effectivePermissions(me).has(BAN)` — deliberately ban-blind — so a banned BAN-holder can +launch a full community Refounding from the shipping app. Honest receivers refuse it +(`drainConcordRekeys` checks the ban-aware `hasPermission`), so the blast radius today is noise plus +self-stranding — but it is a race against banlist propagation, and a fresh joiner who has not folded +the ban yet has no reason to refuse. + +**Fix.** Move the authority check into the action layer where it cannot be bypassed by a new caller +(desktop, CLI, a future screen), and switch every `effectivePermissions` used as an authorization +test to `hasPermission`. Keep `effectivePermissions` only where the question really is "what do +their roles say", independent of standing. + +## A4 — A banned member keeps broadcasting "typing", and we keep showing it + +**Low, both halves ours.** *Read:* `AccountConcordActions.sendConcordTyping`, +`ConcordCommunitySession.ingestTyping`. + +The send side checks `isWriteable()` and nothing else, so a banned member's stock app keeps emitting +kind-23311 heartbeats. The receive side checks that the rumor is a typing heartbeat, is bound to the +channel/epoch, and is not our own — and nothing else. So a banned member sits in the "… is typing" +row indefinitely, in a channel where every message they send is hidden. Cheap to fix on both ends, +and it directly contradicts what a ban promises the user. + +## A5 — A banned member's own client keeps reading and rendering everything + +**Medium, partly inherent.** *Read:* CORD-02/05, `ConcordCommunitySession`. + +Until a Refounding, a ban stops honest clients from *showing* the banned member's posts; it does not +stop delivering the community's posts *to* them. Their stock app keeps subscribing, decrypting and +rendering the whole community in real time. They also keep any invite links they hold (and can mint +more — A1). + +The cryptography here is inherent to a soft ban, but the **product** side is ours: "Ban" and "Remove +from community" are very different promises and the UI presents them as neighbours in one menu. +Worth making the difference explicit at the point of choice, and worth defaulting destructive +moderation to the Refounding path. + +--- + +# Part B — requires a malicious client + +The attacker writes their own events, so nothing client-side binds them. We can only refuse to honor +what they publish. + +## B1 — One edition at `Long.MAX_VALUE` pins an entity forever **Critical. Does not require a banned user, a sockpuppet, or the owner's absence. Unrecoverable.** @@ -87,7 +244,7 @@ cross-epoch tolerance. Options, roughly in order of preference: The first is the smallest change and closes the unrecoverability; the third should happen regardless. -## V2 — A banned staffer keeps Role, Grant and Banlist authority +## B2 — A banned staffer keeps Role, Grant and Banlist authority **Critical.** *Verified:* `quartz/…/cord04Roles/BannedStaffEscalationTest.kt` (13 tests). @@ -118,22 +275,22 @@ banned in pass A; then recompute the banlist under pass B's roster, keeping only authorized. Deterministic, terminates, no oscillation on mutual bans. **Consensus-affecting**: until Armada ships the same rule, we will drop editions they honor. -## V3 — A rogue rotator compacts the banlist away +## B3 — A rogue rotator compacts the banlist away **High.** *Verified:* `aRogueRotatorCompactsTheBanAwayForEveryClientWithoutAFloor`. A CORD-06 §3 compaction re-wraps one edition per entity and the *rotator* picks it, so a rotator can decline to carry the banlist forward. Every edition it serves is genuine, so no signature check sees the omission — `EntityFloor`'s own KDoc names this case ("clearing a banlist"). A banned member -cannot rotate, but the V2 puppet can. +cannot rotate, but the B2 puppet can. The result is not a clean unban but a **split community**: clients that already folded the ban refuse the rollback and still see it, fresh joiners have no floor and see no ban at all. Two populations permanently disagreeing about who is a member, with no event either side can call -forged. Closing V2 removes the puppet and takes this with it; floors alone do not, since they only +forged. Closing B2 removes the puppet and takes this with it; floors alone do not, since they only protect people who were already there. -## V4 — The Refounding recipient set is attacker-inflatable +## B4 — The Refounding recipient set is attacker-inflatable **High.** *Read:* `ConcordCommunitySession.allMembers()` / `emitChannelRumors`; `AccountConcordActions.refoundConcordCommunity` step 2; `ConcordRefounding.buildBaseRekeyWraps`. @@ -152,7 +309,7 @@ This is the cheapest thing on the list to fix and the only one that is not conse the recipient set, prefer recent/attested members when over the cap, and surface what was dropped (a silent truncation strands real members). Worth doing first. -## V5 — The ban is a per-pubkey display rule and the channel key is not revoked +## B5 — The ban is a per-pubkey display rule and the channel key is not revoked **High.** *Read:* `Account.consumeConcordRumorGated` (`isBanned(rumor.pubKey)`), `Account.isAcceptable`. @@ -160,12 +317,12 @@ Writing to a channel needs the channel key, which the ban does not take away; th whatever key the client feels like using. A malicious client therefore posts every message from a fresh npub and `isBanned` never matches — moderation is whack-a-mole against an infinite identity supply. Each message also costs every member two NIP-44 decrypts and two signature verifications -*before* the banlist check runs, and each fresh author inflates V4. +*before* the banlist check runs, and each fresh author inflates B4. There is no client-side answer; only a Refounding rotates the key out from under them. That is the -correct design, which is why V4 matters so much. +correct design, which is why B4 matters so much. -## V6 — Channel history is deletable on a naive third-party relay +## B6 — Channel history is deletable on a naive third-party relay **High, external.** *Verified (that we are safe):* `geode/…/ConcordPlaneKeyDeletionTest.kt` (3 tests). @@ -187,30 +344,7 @@ A community publishes wherever its metadata points. Any relay that authorizes de `pubkey` still hands every ex-member the wipe button, and a Refounding protects only the future. Worth a note in the CORD-01 spec and a line in the relay-selection guidance. -## V7 — Banlist rank rule diverges from Armada - -**Medium, known, deliberate.** See `docs/concord-banlist-rank-conformance.md`, already reported. - -We enforce §3's rank half on the Banlist and Armada does not, so the two clients can show different -banlists. Shipped knowingly. Row 3 of that report ("a banned `BAN` holder unbans themselves") was -left open as a fixpoint-ordering question — V2 is the general form of it, and the fix proposed there -resolves both. - -## V8 — A soft ban revokes no read access and no live invite - -**Medium, inherent.** *Read:* CORD-02/05. - -Until a Refounding, a banned member decrypts everything published — the ban only stops honest -clients from *showing* their posts, not from delivering the group's posts to them. They also keep -any invite links they created while privileged; those still resolve to bundles carrying the current -root. Publishing the root, or one live link, invites an unbanned crowd that each has to be banned -individually (and see V5). - -Not a bug so much as the definition of a soft ban, but it belongs on the list because the UI should -say so: "Ban" and "Remove from community" are very different promises and users will read the first -as the second. - -## V9 — The base-rekey plane is writable by every member +## B7 — The base-rekey plane is writable by every member **Low.** *Read:* `ConcordKeyDerivation.baseRekeyAddress`, `AccountConcordActions.drainConcordRekeys`. @@ -219,63 +353,36 @@ valid wraps there. Authorization happens after the blobs are scanned, so a flood a locator scan per blob on every revision tick. Bounded work per wrap and no correctness impact; listed for completeness. -## V10 — Stranded recovery: either broken, or a removal bypass -**Critical, and it forks — one of the two halves is true and both are bad.** -*Read:* `ConcordStrandedRecovery`, `AccountConcordActions.recoverStrandedConcordCommunities`, -`AccountConcordActions.mintConcordInvite`. +--- -`ConcordStrandedRecovery.isStranded` / `mergeForward` take only `(entry, bundle)`. There is **no -banlist check and no check that we were legitimately re-keyed** — the entire test is "the bundle at -my stored `inviteRef` sits at a higher epoch than I do". The unlock token lives in the link -fragment, which an ex-member keeps forever. So whether a removed member walks back in with the new -root depends *only* on whether the bundle at that coordinate ever advances an epoch. +# Part C — interop and not-yet-shipped -In Amethyst it never does: `mintConcordInvite` mints a **fresh link signer per mint**, so nothing -re-publishes at an existing coordinate, and `refoundConcordCommunity` does not re-mint or revoke -anything. Two consequences, and they are the fork: +## C1 — Banlist rank rule diverges from Armada -- **If nothing re-mints** — today's behaviour — then stranded recovery never fires *for anyone*. - That makes it dead code, and the cure `drainConcordRekeys`' own KDoc points to for "a BAN-holder - can evict anyone (the owner included) by omission" does not exist. An owner evicted by a rogue - admin has no way back. -- **If anything re-mints at a stable coordinate** — which is what CORD-05's design describes ("the - community keeps publishing its bundle at that same addressable coordinate, re-minted at the - current epoch"), so plausibly Armada in a cross-client community — then every removed member who - joined through a still-live link auto-recovers the new root on the 15-minute sweep, and - re-announces a Guestbook join so they look current again. **Refounding, the only hard removal, - is silently undone.** +**Medium, known, deliberate.** See `docs/concord-banlist-rank-conformance.md`, already reported. -Note also that `refoundConcordCommunity` never revokes the invite links the removed member created -or joined through, even though `ControlEntityKind.INVITE_REVOKED` exists and `classifyInvite` -already honors it. +We enforce §3's rank half on the Banlist and Armada does not, so the two clients can show different +banlists. Shipped knowingly. Row 3 of that report ("a banned `BAN` holder unbans themselves") was +left open as a fixpoint-ordering question — B2 is the general form of it, and the fix proposed there +resolves both. -**Fix direction.** Decide the intended semantics first — this needs a spec answer, not a patch. -Then: gate `mergeForward` on not being banned in the epoch we are merging *from*, have the -Refounding revoke the removed members' links, and either implement re-minting (so legitimate -recovery works) or drop the mechanism and give evicted owners a different route. +## C2 — Voice rooms are key-gated, not roster-gated -## V11 — Voice rooms are key-gated, not roster-gated +**Design-level; not currently reachable.** *Read:* `ConcordBrokerToken`, CORD-07 §2. -**High.** *Read:* `ConcordBrokerToken`, CORD-07 §2. +Downgraded from High on review: `ConcordBrokerToken` and `VoicePresence` are referenced nowhere +outside `quartz`, so Amethyst ships no Concord voice path yet. This is a note for whoever wires +one up, not a live hole. A member proves voice-room membership by signing a NIP-98 kind-27235 request with the channel's **derived voice signer key**, whose pubkey is the SFU room name. The broker is stateless and holds no community secret, so it cannot consult the Control Plane and has no idea a banlist exists. A banned member keeps that key until a Refounding, so they can join the voice room and stay in it. Nothing on the client side can evict them — kicking them from the UI does not kick them from the SFU. +It would be the one place where a ban fails *audibly*, in real time, in front of everyone, so it is +worth designing the roster check in before shipping rather than after. -This is the one place where a ban fails *audibly*, in real time, in front of everyone. Worth ranking -above its technical severity for that reason alone. - -## V12 — Typing indicators are not ban-filtered - -**Low.** *Read:* `ConcordCommunitySession.ingestTyping`. - -`ingestTyping` checks the rumor is a typing heartbeat, is bound to the channel/epoch, and is not our -own — and nothing else. A banned member (or any fresh npub holding the channel key, see V5) shows -in the "… is typing" row indefinitely. Cheap to fix and user-visible: the promise a ban makes is -that the member disappears, and here they do not. --- @@ -284,7 +391,7 @@ that the member disappears, and here they do not. This audit is bounded by what was opened. Checked and found sound: the wrap/seal envelope (no author impersonation — `rumor.pubKey == seal.pubKey` and `rumor.verifyId()`), Concord chat edits (`Note.latestConcordEdit` is author-gated, so a member cannot rewrite someone else's message), and -self-unban (V2). +self-unban (B2). Not looked at at all: @@ -292,7 +399,7 @@ Not looked at at all: Note that no channel-scoped rekey *receive* path appears to exist: `drainConcordRekeys` handles `ROOT_SCOPE` only, and `entry.privateChannels` is carried forward but never populated by a delivery path. If that is right, the only removal Amethyst can perform is a full-community - Refounding — which is exactly what V4 makes expensive. + Refounding — which is exactly what B4 makes expensive. - **In-plane reactions and deletes** — the edit path is author-gated; the delete path was not read. - **Guestbook kicks** (kind 3309) — the builder documents a KICK-bit + rank rule; the receive side was not verified against it. @@ -301,11 +408,23 @@ Not looked at at all: ## Suggested order -1. **V4** — cheapest, not consensus-affecting, and it protects the remedy every other fix depends on. -2. **V1** — worst blast radius and the only unrecoverable one; does not need an attacker to be - banned or privileged beyond a single ordinary bit. -3. **V2** (+V3, +V7 row 3) — one two-pass change closes all three. Coordinate with Armada first; - this one splits consensus. -4. **V6** — spec note + relay guidance; our own behaviour is already correct and now pinned. -5. **V5 / V8** — UI honesty about what a ban does, and a "Remove from community" affordance that - Refounds rather than bans. +**Part A first.** It is the whole of the realistic threat — a banned user with the app already +installed — and none of it needs coordination with anyone. + +1. **A1** — one guard on `mintConcordInvite` plus one on its button. Smallest fix on the list and it + closes the attack a banned user will actually reach for. +2. **A3** — move authority into the action layer and replace `effectivePermissions` with + `hasPermission` everywhere it is used as an authorization test. This is also the cheapest partial + mitigation for B2: it shrinks what a banned staffer can do *without* writing their own client. +3. **A2** — needs the semantics decided before any code. Raise it with the spec. +4. **A4 / A5** — small, user-visible, and they make the product honest about what a ban is. + +**Then Part B**, hardest first because the ceiling is highest: + +5. **B4** — cheap, not consensus-affecting, and it protects the remedy every other fix depends on. +6. **B1** — worst blast radius, the only unrecoverable one, and the bar is a single ordinary + permission bit. +7. **B2 (+B3, +C1's open row)** — one two-pass change closes all three. Coordinate with Armada + first; this one splits consensus. +8. **B6** — spec note plus relay-selection guidance; our own behaviour is already correct and pinned. +9. **B5 / B7** — accept, or bound. From 3a53292993741375352ecd307025dae0672770a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 15:02:43 +0000 Subject: [PATCH 07/13] fix(concord): close the soft-ban holes reachable from the shipping app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part A of docs/concord-soft-ban-audit.md — the ones a banned user reaches by tapping a button, no custom tooling involved. A1. mintConcordInvite checked that the account was writeable and that we held the community, and nothing else, while its button was the one control on the screen with no gate at all. A member banned a minute ago could hand out a working link to the community they were removed from, and every account they invited arrived as a fresh un-banned npub. Now gated on CREATE_INVITE, both in the verb and on the button. Worth noting the bit was not enforced anywhere else: the fold gates the INVITE_* Control entities on it, but a link's bundle is a standalone kind-33301 published outside the Control Plane, so this check is the only one that exists. A3. Every moderation verb checked isWriteable() plus the Control write key — which is a spam gate, never authority (CORD-02 §5) — and left the real decision to whichever composable drew the button. Those gates then tested effectivePermissions, which ignores the banlist, so a banned staffer kept seeing the controls; the editions were dropped by everyone's fold, making them silently no-op, which this codebase elsewhere calls out as worse than absent. Ban and Remove survived only because a second, unrelated condition happened to route through the ban-aware canActOn. Authority now lives in the action layer behind isAuthorizedFor(), so a caller from desktop, amy or a future screen inherits it, and every authorization test uses hasPermission. refoundConcordCommunity's own guard was ban-blind outright and now rank-checks each removed member too. A2. The recovery sweep merges us onto any higher-epoch bundle found at our stored invite_ref, and an ex-member keeps that link's unlock token forever — so our own background timer walked a removed member back into the epoch a Refounding had rotated them out of. isStranded/mergeForward now take bannedAtCurrentEpoch as a required argument rather than leaving it to callers, because a caller that forgets it inverts the mechanism. The liveness half of that finding (nothing re-mints at a stable coordinate, so legitimate recovery never fires either) needs a spec answer and is untouched here. A4. Typing heartbeats are filtered on both ends, so a banned member stops announcing that they are typing messages nobody will see. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- .../amethyst/model/AccountConcordActions.kt | 110 ++++++++++++++++-- .../concord/ConcordChannelListScreen.kt | 49 +++++--- .../concord/ConcordMembersScreen.kt | 5 +- .../commons/actions/ConcordActions.kt | 3 +- .../model/concord/ConcordCommunitySession.kt | 3 + .../cord05Invites/ConcordStrandedRecovery.kt | 17 ++- .../ConcordStrandedRecoveryTest.kt | 26 +++-- 7 files changed, 173 insertions(+), 40 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt index fba41f5e01..c403e08986 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt @@ -171,6 +171,16 @@ class AccountConcordActions( val entry = account.concordChannelList.liveCommunities.value .firstOrNull { it.id == communityId } ?: return null + // CREATE_INVITE, and not while banned. This used to check only that we held the community, + // which made minting the one moderation-free action in the app: a member the owner had just + // banned could tap the invite button and hand out a working link to the community they were + // removed from, and every account they invited arrived as a fresh un-banned npub. + // + // Note the bit is not otherwise enforced anywhere. The fold gates the INVITE_* Control + // entities on CREATE_INVITE, but a link's bundle is a standalone kind-33301 published + // OUTSIDE the Control Plane, so no fold ever sees it. This check is the only one there is. + val session = account.concordSessions.sessionFor(communityId) ?: return null + if (!isAuthorizedFor(session, ConcordPermissions.CREATE_INVITE)) return null val invite = ConcordActions.inviteFor( communityIdHex = entry.id, @@ -430,7 +440,17 @@ class AccountConcordActions( channelIdHex: String, ) { if (!account.isWriteable()) return - val entry = account.concordSessions.sessionFor(communityId)?.entry ?: return + val session = account.concordSessions.sessionFor(communityId) ?: return + // A ban hides every message we send, so continuing to announce that we are typing them is + // both noise and a contradiction of what the ban told the room. Filtered on the receive side + // too (ConcordCommunitySession.ingestTyping) — a malicious client would keep sending. + if (session.state.value + ?.authority + ?.isBanned(account.signer.pubKey) == true + ) { + return + } + val entry = session.entry val channelKey = ConcordActions.publicChannel(entry.root.hexToByteArray(), channelIdHex.hexToByteArray(), entry.rootEpoch) val wrap = ConcordActions.buildChannelTyping(account.signer, channelKey, channelIdHex, entry.rootEpoch, TimeUtils.now()) val relays = entry.relays.mapNotNullTo(mutableSetOf()) { RelayUrlNormalizer.normalizeOrNull(it) } @@ -487,6 +507,49 @@ class AccountConcordActions( return cp } + /** + * Whether this account may take the action guarded by [bit] in [session] — and, when [target] is + * given, take it *against that member* (CORD-04 §3's rank rule, "equal cannot act on equal"). + * + * Every moderation verb below funnels through this. It used to live only in the composables that + * drew the buttons, which failed three ways: the screens tested `effectivePermissions`, which + * ignores the banlist, so a banned staffer still saw the controls; a verb reached from anywhere + * else (desktop, `amy`, a new screen) inherited no check at all; and holding `control_root` — + * a spam gate, never authority (CORD-02 §5) — was the only thing actually being enforced. + * + * Fails **closed**, with one deliberate exception: the owner is read from [ConcordCommunityListEntry] + * rather than from the fold, because the community id proves them (CORD-02) and they must stay able + * to moderate before their Control Plane has finished folding — or through a fold a rogue has + * damaged. Everyone else needs a resolved roster, so an unfolded community grants nobody else + * anything. + */ + private fun isAuthorizedFor( + session: ConcordCommunitySession, + bit: Int, + target: HexKey? = null, + ): Boolean { + val me = account.signer.pubKey + if (session.entry.owner.equals(me, ignoreCase = true)) return true + val authority = session.state.value?.authority ?: return false + // hasPermission, never effectivePermissions: the latter reads the roles alone and would let a + // banned staffer keep acting for as long as they hold the key. + val allowed = if (target == null) authority.hasPermission(me, bit) else authority.canActOn(me, target, bit) + if (!allowed) { + Log.w("Concord") { "Refusing a Concord action in ${session.entry.id}: not authorized for bit $bit${target?.let { " on $it" } ?: ""} (CORD-04 §3)" } + } + return allowed + } + + /** [controlKeysForWrite] gated by [isAuthorizedFor] — the standing check and the key check together. */ + private fun controlKeysForAction( + session: ConcordCommunitySession, + bit: Int, + target: HexKey? = null, + ): ControlPlaneKeys? { + if (!isAuthorizedFor(session, bit, target)) return null + return controlKeysForWrite(session) + } + /** Grant [member] exactly [roleIds] (empty list revokes their roles). */ suspend fun grantConcordRole( communityId: String, @@ -495,7 +558,7 @@ class AccountConcordActions( ): Boolean { val session = account.concordSessions.sessionFor(communityId) ?: return false if (!account.isWriteable()) return false - val cp = controlKeysForWrite(session) ?: return false + val cp = controlKeysForAction(session, ConcordPermissions.MANAGE_ROLES, member) ?: return false // A Grant that first makes its member staff must deliver the control_root in the same // edition (CORD-04 §3) — grantWithStaffDelivery attaches the pairwise wrap when the // roles carry a Control-writing bit and we hold the secret to hand over. @@ -567,7 +630,7 @@ class AccountConcordActions( ): Boolean { val session = account.concordSessions.sessionFor(communityId) ?: return false if (!account.isWriteable()) return false - val cp = controlKeysForWrite(session) ?: return false + val cp = controlKeysForAction(session, ConcordPermissions.MANAGE_ROLES, member) ?: return false val existing = session.state.value @@ -609,7 +672,7 @@ class AccountConcordActions( ): Boolean { val session = account.concordSessions.sessionFor(communityId) ?: return false if (!account.isWriteable()) return false - val cp = controlKeysForWrite(session) ?: return false + val cp = controlKeysForAction(session, ConcordPermissions.MANAGE_ROLES, member) ?: return false val grantWrap = ConcordModeration.grant(account.signer, cp, communityId.hexToByteArray(), member, emptyList(), session.controlEditions(), TimeUtils.now(), owner = session.entry.owner) publishConcordWrap(session.entry, grantWrap) return true @@ -657,7 +720,7 @@ class AccountConcordActions( ): Boolean { val session = account.concordSessions.sessionFor(communityId) ?: return false if (!account.isWriteable()) return false - val cp = controlKeysForWrite(session) ?: return false + val cp = controlKeysForAction(session, ConcordPermissions.BAN, member) ?: return false val wrap = ConcordModeration.ban(account.signer, cp, communityId.hexToByteArray(), member, session.controlEditions(), TimeUtils.now(), owner = session.entry.owner) publishConcordWrap(session.entry, wrap) return true @@ -670,7 +733,7 @@ class AccountConcordActions( ): Boolean { val session = account.concordSessions.sessionFor(communityId) ?: return false if (!account.isWriteable()) return false - val cp = controlKeysForWrite(session) ?: return false + val cp = controlKeysForAction(session, ConcordPermissions.BAN, member) ?: return false val wrap = ConcordModeration.unban(account.signer, cp, communityId.hexToByteArray(), member, session.controlEditions(), TimeUtils.now(), owner = session.entry.owner) publishConcordWrap(session.entry, wrap) return true @@ -701,10 +764,22 @@ class AccountConcordActions( val session = account.concordSessions.sessionFor(communityId) ?: return false val state = session.state.value ?: return false val authority = state.authority - val iCanBan = authority.isOwner(account.signer.pubKey) || authority.effectivePermissions(account.signer.pubKey).has(ConcordPermissions.BAN) + // hasPermission, not effectivePermissions: a Refounding is the hardest action in the protocol + // and this guard used to ignore the banlist, so a banned BAN-holder could launch one from the + // shipping app. Honest receivers refuse such a rotation (drainConcordRekeys checks the same + // ban-aware predicate), but that is a race against banlist propagation, not a check. + val iCanBan = authority.isOwner(account.signer.pubKey) || authority.hasPermission(account.signer.pubKey, ConcordPermissions.BAN) if (!iCanBan) return false val removedLower = removed.mapTo(HashSet()) { it.lowercase() } if (removedLower.isEmpty() || removedLower.any { authority.isOwner(it) }) return false + // Removal is the hardest form of a ban, so it takes the same rank rule (CORD-04 §3): an admin + // cannot Refound a peer admin out of the community any more than they could ban one. The owner + // short-circuits, as everywhere else, because canActOn starts at hasPermission. + if (!authority.isOwner(account.signer.pubKey) && + removedLower.any { !authority.canActOn(account.signer.pubKey, it, ConcordPermissions.BAN) } + ) { + return false + } // A Refounding writes the current plane (the pre-rotation bans) and the new one (the // compaction), so on a split epoch it takes the current control_root (CORD-02 §2). A // rank-qualified refounder whose secret hasn't arrived yet must wait for re-delivery. @@ -986,7 +1061,18 @@ class AccountConcordActions( // Only a live bundle recovers: an expired/revoked link is not a rotation we missed. val bundle = (ConcordActions.classifyInvite(wraps, parsed.fragment.token) as? InviteBundleStatus.Live)?.invite ?: continue - val merged = ConcordActions.recoverStranded(entry, bundle) ?: continue + // A removed member holds the link's unlock token forever, so without this the sweep + // walks them straight back into the epoch they were rotated out of — see A2 in + // docs/concord-soft-ban-audit.md. Read off the epoch we are LEAVING, which is the last + // one whose Control Plane we can still fold. + val bannedHere = + account.concordSessions + .sessionFor(entry.id) + ?.state + ?.value + ?.authority + ?.isBanned(account.signer.pubKey) == true + val merged = ConcordActions.recoverStranded(entry, bundle, bannedHere) ?: continue if (!adoptedConcordRotations.add("${entry.id}:${merged.rootEpoch}")) continue Log.i("Concord", "Stranded recovery: ${entry.id} ${entry.rootEpoch} -> ${merged.rootEpoch}") account.sendMyPublicAndPrivateOutbox(account.concordChannelList.follow(merged)) @@ -1009,7 +1095,7 @@ class AccountConcordActions( ): Boolean { val session = account.concordSessions.sessionFor(communityId) ?: return false if (!account.isWriteable()) return false - val cp = controlKeysForWrite(session) ?: return false + val cp = controlKeysForAction(session, ConcordPermissions.MANAGE_METADATA) ?: return false val metadata = MetadataEntity(name = name, icon = icon, banner = banner, description = description, relays = relays) val wrap = ConcordModeration.editMetadata(account.signer, cp, communityId.hexToByteArray(), metadata, session.controlEditions(), TimeUtils.now(), owner = session.entry.owner) publishConcordWrap(session.entry, wrap) @@ -1027,7 +1113,7 @@ class AccountConcordActions( ): Boolean { val session = account.concordSessions.sessionFor(communityId) ?: return false if (!account.isWriteable()) return false - val cp = controlKeysForWrite(session) ?: return false + val cp = controlKeysForAction(session, ConcordPermissions.MANAGE_CHANNELS) ?: return false val channelId = RandomInstance.bytes(32) val channel = ChannelEntity(name = name.trim()) val wrap = ConcordModeration.defineChannel(account.signer, cp, channelId, channel, session.controlEditions(), TimeUtils.now(), owner = session.entry.owner) @@ -1043,7 +1129,7 @@ class AccountConcordActions( ): Boolean { val session = account.concordSessions.sessionFor(communityId) ?: return false if (!account.isWriteable()) return false - val cp = controlKeysForWrite(session) ?: return false + val cp = controlKeysForAction(session, ConcordPermissions.MANAGE_CHANNELS) ?: return false // Carry the standing definition forward and change only the name. A ChannelEntity built from // scratch defaults `private` and `voice` to false, so renaming a private channel used to // publish an edition declaring it PUBLIC — and a voice channel became a text channel. @@ -1066,7 +1152,7 @@ class AccountConcordActions( ): Boolean { val session = account.concordSessions.sessionFor(communityId) ?: return false if (!account.isWriteable()) return false - val cp = controlKeysForWrite(session) ?: return false + val cp = controlKeysForAction(session, ConcordPermissions.MANAGE_CHANNELS) ?: return false // Same as rename: preserve the standing flags so a tombstone does not also silently // reclassify the channel it retires. val standing = diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordChannelListScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordChannelListScreen.kt index d8c94f5ace..d33dfee5e2 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordChannelListScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordChannelListScreen.kt @@ -170,10 +170,14 @@ fun ConcordChannelListScreen( // Rank alone isn't enough on a split epoch: publishing any Control edition also takes the // control_root (CORD-02 §2), which a freshly promoted staffer may not hold yet (CORD-04 §3), // so the affordance waits for the key too. + // hasPermission, never effectivePermissions: the latter reads the roles alone, so a banned + // moderator kept seeing every control here. The editions they authored were dropped by everyone's + // fold, which made these buttons silently no-op — worse than absent, and the same trap this file + // already avoids for the Roles… menu. val canManageChannels = state?.authority?.let { it.isOwner(account.signer.pubKey) || - it.effectivePermissions(account.signer.pubKey).has(ConcordPermissions.MANAGE_CHANNELS) + it.hasPermission(account.signer.pubKey, ConcordPermissions.MANAGE_CHANNELS) } == true && session?.controlPlaneKeys()?.canWrite == true @@ -242,10 +246,19 @@ fun ConcordChannelListScreen( val canEdit = state?.authority?.let { it.isOwner(account.signer.pubKey) || - it.effectivePermissions(account.signer.pubKey).has(ConcordPermissions.MANAGE_METADATA) + it.hasPermission(account.signer.pubKey, ConcordPermissions.MANAGE_METADATA) } == true && session?.controlPlaneKeys()?.canWrite == true + // Minting an invite hands out a working key to the community, so it takes + // CREATE_INVITE like any other privileged action. This button used to be the one + // control on the screen with no gate at all. + val canInvite = + state?.authority?.let { + it.isOwner(account.signer.pubKey) || + it.hasPermission(account.signer.pubKey, ConcordPermissions.CREATE_INVITE) + } == true + IconButton(onClick = { nav.nav(Route.ConcordMembers(communityId)) }) { SymbolIcon(symbol = MaterialSymbols.Group, contentDescription = stringRes(com.vitorpamplona.amethyst.R.string.concord_members_title)) } @@ -254,22 +267,24 @@ fun ConcordChannelListScreen( SymbolIcon(symbol = MaterialSymbols.Edit, contentDescription = stringRes(com.vitorpamplona.amethyst.R.string.concord_edit_title)) } } - IconButton( - enabled = !minting, - onClick = { - minting = true - scope.launch { - try { - inviteLink = account.concord.mintConcordInvite(communityId) - } finally { - // Always clear the flag — a thrown mint would otherwise leave the - // button disabled until the screen is recreated. - minting = false + if (canInvite) { + IconButton( + enabled = !minting, + onClick = { + minting = true + scope.launch { + try { + inviteLink = account.concord.mintConcordInvite(communityId) + } finally { + // Always clear the flag — a thrown mint would otherwise leave the + // button disabled until the screen is recreated. + minting = false + } } - } - }, - ) { - SymbolIcon(symbol = MaterialSymbols.PersonAdd, contentDescription = stringRes(com.vitorpamplona.amethyst.R.string.concord_invite_action)) + }, + ) { + SymbolIcon(symbol = MaterialSymbols.PersonAdd, contentDescription = stringRes(com.vitorpamplona.amethyst.R.string.concord_invite_action)) + } } // Overflow, mirroring the NIP-29 relay-group top bar: destructive membership diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordMembersScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordMembersScreen.kt index 57b146653b..a9e166c46c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordMembersScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/concord/ConcordMembersScreen.kt @@ -133,7 +133,10 @@ fun ConcordMembersScreen( } val iAmOwner = state?.authority?.isOwner(myPubKey) == true - val iCanBan = state?.let { it.authority.isOwner(myPubKey) || it.authority.effectivePermissions(myPubKey).has(ConcordPermissions.BAN) } == true + // hasPermission, never effectivePermissions: a banned BAN-holder used to keep the whole Ban / + // Remove menu. It only stayed harmless because `canBanTarget` below routes through canActOn, + // which IS ban-aware — a thin margin for the escalation in docs/concord-soft-ban-audit.md. + val iCanBan = state?.let { it.authority.isOwner(myPubKey) || it.authority.hasPermission(myPubKey, ConcordPermissions.BAN) } == true val iCanManageRoles = state?.authority?.hasPermission(myPubKey, ConcordPermissions.MANAGE_ROLES) == true // The roles this viewer may actually hand out. The fold drops a grant whose granter does diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActions.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActions.kt index d99679263f..dff1143c45 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActions.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActions.kt @@ -454,7 +454,8 @@ object ConcordActions { fun recoverStranded( entry: ConcordCommunityListEntry, bundle: CommunityInvite, - ): ConcordCommunityListEntry? = ConcordStrandedRecovery.mergeForward(entry, bundle) + bannedAtCurrentEpoch: Boolean, + ): ConcordCommunityListEntry? = ConcordStrandedRecovery.mergeForward(entry, bundle, bannedAtCurrentEpoch) /** Decrypts + validates a fetched bundle event with the link token; null if invalid. */ fun openBundle( diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/concord/ConcordCommunitySession.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/concord/ConcordCommunitySession.kt index 7670ff69c6..58f6156b89 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/concord/ConcordCommunitySession.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/concord/ConcordCommunitySession.kt @@ -486,6 +486,9 @@ class ConcordCommunitySession( if (!ChannelChat.isTyping(rumor) || !ChannelChat.isBoundTo(rumor, channelIdHex, epoch)) return val who = rumor.pubKey.lowercase() if (who == myPubKey.lowercase()) return // never show my own typing back to me + // A banned member's messages are dropped everywhere, so their typing heartbeat must be too — + // otherwise they sit in the "… is typing" row forever in a channel they cannot be heard in. + if (_state.value?.authority?.isBanned(who) == true) return val now = TimeUtils.now() // Update the map and publish inside the lock so a concurrent heartbeat on another // channel can't publish an older snapshot last and drop this channel's typers. diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordStrandedRecovery.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordStrandedRecovery.kt index 4f6e02d447..9869cadb91 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordStrandedRecovery.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordStrandedRecovery.kt @@ -46,12 +46,24 @@ object ConcordStrandedRecovery { * True when [bundle], resolved at [entry]'s stored invite link, proves we were * left behind: it must describe the same community and sit at a strictly higher * epoch. Same or lower is a no-op (we are current, or the bundle is stale). + * + * [bannedAtCurrentEpoch] is the caller's answer to "does the community, as I fold + * it right now, have me on its banlist?" — and a `true` refuses the recovery + * outright. It is a required argument rather than a caller-side `if` because + * getting it wrong turns this mechanism inside out: recovery exists so a member + * *wrongly* omitted from a rotation can catch up, but the test it performs (a + * higher epoch at a link whose unlock token an ex-member keeps forever) cannot + * tell that member apart from one the community deliberately removed. Without + * this, a Refounding — the only hard removal Concord has — is undone by our own + * background sweep a few minutes later. */ fun isStranded( entry: ConcordCommunityListEntry, bundle: CommunityInvite, + bannedAtCurrentEpoch: Boolean, ): Boolean = - entry.inviteRef != null && + !bannedAtCurrentEpoch && + entry.inviteRef != null && bundle.communityId.equals(entry.id, ignoreCase = true) && bundle.rootEpoch > entry.rootEpoch @@ -73,8 +85,9 @@ object ConcordStrandedRecovery { fun mergeForward( entry: ConcordCommunityListEntry, bundle: CommunityInvite, + bannedAtCurrentEpoch: Boolean, ): ConcordCommunityListEntry? { - if (!isStranded(entry, bundle)) return null + if (!isStranded(entry, bundle, bannedAtCurrentEpoch)) return null // Bank the epoch we are leaving with its control_pk, so its Control Plane // stays re-subscribable for the anti-rollback floor (a split epoch's address diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordStrandedRecoveryTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordStrandedRecoveryTest.kt index 6c0f9aa59c..96c1124e57 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordStrandedRecoveryTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord05Invites/ConcordStrandedRecoveryTest.kt @@ -82,7 +82,7 @@ class ConcordStrandedRecoveryTest { val prior = HeldRoot(0L, "aa".repeat(32)) val stranded = entry(epoch = 1, heldRoots = listOf(prior)) - val merged = ConcordStrandedRecovery.mergeForward(stranded, bundle(epoch = 5)) + val merged = ConcordStrandedRecovery.mergeForward(stranded, bundle(epoch = 5), bannedAtCurrentEpoch = false) assertNotNull(merged, "a higher-epoch bundle at our own invite link means we were left behind") // adopted the new epoch's access root @@ -106,27 +106,27 @@ class ConcordStrandedRecoveryTest { @Test fun sameEpochBundleIsANoOp() { - assertNull(ConcordStrandedRecovery.mergeForward(entry(epoch = 5), bundle(epoch = 5))) - assertFalse(ConcordStrandedRecovery.isStranded(entry(epoch = 5), bundle(epoch = 5))) + assertNull(ConcordStrandedRecovery.mergeForward(entry(epoch = 5), bundle(epoch = 5), bannedAtCurrentEpoch = false)) + assertFalse(ConcordStrandedRecovery.isStranded(entry(epoch = 5), bundle(epoch = 5), bannedAtCurrentEpoch = false)) } @Test fun lowerEpochBundleIsANoOp() { // Epoch-monotonic: a stale bundle must never walk the membership backwards. - assertNull(ConcordStrandedRecovery.mergeForward(entry(epoch = 7), bundle(epoch = 3))) + assertNull(ConcordStrandedRecovery.mergeForward(entry(epoch = 7), bundle(epoch = 3), bannedAtCurrentEpoch = false)) } @Test fun entryWithoutInviteRefIsInert() { // Direct invites and legacy entries have no anchor — expected, not an error. val noAnchor = entry(epoch = 1, ref = null) - assertFalse(ConcordStrandedRecovery.isStranded(noAnchor, bundle(epoch = 9))) - assertNull(ConcordStrandedRecovery.mergeForward(noAnchor, bundle(epoch = 9))) + assertFalse(ConcordStrandedRecovery.isStranded(noAnchor, bundle(epoch = 9), bannedAtCurrentEpoch = false)) + assertNull(ConcordStrandedRecovery.mergeForward(noAnchor, bundle(epoch = 9), bannedAtCurrentEpoch = false)) } @Test fun bundleForAnotherCommunityIsIgnored() { - assertNull(ConcordStrandedRecovery.mergeForward(entry(epoch = 1), bundle(epoch = 9, id = "99".repeat(32)))) + assertNull(ConcordStrandedRecovery.mergeForward(entry(epoch = 1), bundle(epoch = 9, id = "99".repeat(32)), bannedAtCurrentEpoch = false)) } // ---- the bare `#` anchor form ---------------------------- @@ -252,4 +252,16 @@ class ConcordStrandedRecoveryTest { assertEquals(4L, other.rootEpoch) assertEquals(inviteRef, other.inviteRef) } + + @Test + fun aBannedMemberDoesNotRecoverIntoTheEpochTheyWereRemovedFrom() { + // The removal case the higher-epoch test cannot tell apart on its own: an ex-member keeps the + // link's unlock token forever, so without the ban gate the recovery sweep merges them into the + // very epoch a Refounding rotated them out of. See A2 in docs/concord-soft-ban-audit.md. + val stranded = entry(epoch = 1) + assertFalse(ConcordStrandedRecovery.isStranded(stranded, bundle(epoch = 5), bannedAtCurrentEpoch = true)) + assertNull(ConcordStrandedRecovery.mergeForward(stranded, bundle(epoch = 5), bannedAtCurrentEpoch = true)) + // ...and the legitimate case still works, so the gate is not just "recovery off". + assertNotNull(ConcordStrandedRecovery.mergeForward(stranded, bundle(epoch = 5), bannedAtCurrentEpoch = false)) + } } From 54c412da7aace3e93e6c82cffb85e634b5fd7638 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 15:12:41 +0000 Subject: [PATCH 08/13] fix(quartz): stop a stray edition from pinning a Control entity forever MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit B1 in docs/concord-soft-ban-audit.md, the worst item on the list: one edition at version = Long.MAX_VALUE permanently pinned its entity to the author's content, for every client holding a floor for it, with no way back. The floor rose to MAX_VALUE, no honest edition could exceed it, and a Refounding that dropped the poison fell back to EntityFloor.known — the poison. Authored in the tests by a current, legitimately granted moderator: no ban, no sockpuppet, one ordinary permission bit. The chain walk was never the weakness; it advances only to head.version + 1 citing the head's hash, so a fresh joiner was untouched. The compaction arm was: it trades contiguity for cross-epoch tolerance, which left VERSION as the only contest an edition had to win. Two changes. The arm now tries the floor-anchored chain first and falls back to the raw-version bootstrap only when nothing connects, so a stray never wins a fold where the honest chain is present. And the bootstrap will not follow a jump of more than MAX_COMPACTION_VERSION_JUMP above the floor — a compacted head is legitimately ahead by a chain's worth, not by 2^63 — so the version space cannot be exhausted in a step. A new test pins the tolerance the arm exists for, so the bound cannot later be tightened into breaking CORD-06 §3. compactControlPlane picked its per-entity head by raw highest version too, which made an honest rotator the delivery mechanism: a disconnected stray never joins the chain but won that comparison, and was re-wrapped into the new epoch as the entity's whole history, where fresh joiners anchor on it. It now picks the chain head, keeping foldEntity's fresh-joiner fallback for the dangling `prev` a prior compaction leaves behind. The three reproductions now assert the fixed behaviour. The banlist's escape hatch (a floor-less chain walk plus the re-heal union) is kept and still pinned. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- .../quartz/concord/cord04Roles/EditionFold.kt | 66 ++++++++- .../concord/cord06Rekey/ConcordRefounding.kt | 26 +++- .../ControlPlaneVersionExhaustionTest.kt | 132 +++++++++++++----- 3 files changed, 176 insertions(+), 48 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/EditionFold.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/EditionFold.kt index 4e5b8fa72c..956a0ec461 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/EditionFold.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/EditionFold.kt @@ -150,9 +150,63 @@ object EditionFold { floorVersion: Long, ): ControlEdition? = editions - .filter { it.version >= floorVersion } + .filter { it.version >= floorVersion && it.version - floorVersion <= MAX_COMPACTION_VERSION_JUMP } .minWithOrNull(compareByDescending { it.version }.thenBy { it.rumorId }) + /** + * How far above the floor the compaction arm will follow an edition in one step. + * + * The arm trades contiguity for cross-epoch tolerance, which made VERSION the only contest an + * edition had to win — so a single authorized edition at `version = Long.MAX_VALUE` used to + * become an entity's permanent head: it won the arm, `authorizedHeads` raised the floor to + * `Long.MAX_VALUE`, and from there no honest edition could ever exceed the floor again. Even a + * Refounding that dropped the poison did not help, because nothing was then offered at or above + * the floor and the fold fell back to [EntityFloor.known] — the poison itself. See B1 in + * `docs/concord-soft-ban-audit.md`. + * + * A compacted head is legitimately ahead of the floor by however many editions the entity gained + * while we were away — a chain's worth, not 2^63. This bound is deliberately far above any real + * community (a channel renamed a thousand times a day for three years stays under it) and far + * below the point where the version space can be exhausted. Anything beyond it is not a + * compaction we missed; it is someone reaching for the ceiling, and it is treated as a gap. + */ + const val MAX_COMPACTION_VERSION_JUMP = 1_000_000L + + /** + * The floor-anchored chain head among [editions], or null when nothing connects to [floor]. + * + * The same anchor-then-walk the main path uses, factored out so the compaction arm can try it + * first: the anchor is the floor's own edition (same version AND hash) or its immediate + * successor citing that hash, then the walk climbs while each `version + 1` cites the current + * head. Reports no gap — a null here means "fall back", not "refuse". + */ + private fun chainHead( + editions: List, + floor: EntityFloor, + ): ControlEdition? { + val byVersion = HashMap>() + for (e in editions) byVersion.getOrPut(e.version) { ArrayList() }.add(e) + + val lowest = byVersion.keys.filter { it >= floor.version }.minOrNull() ?: return null + val winner = byVersion[lowest]?.minByOrNull { it.rumorId } ?: return null + var head = + when (lowest) { + floor.version -> winner.takeIf { it.hashHex == floor.hashHex } + floor.version + 1 -> winner.takeIf { it.prevHash != null && it.prevHash.toHexKey() == floor.hashHex } + else -> null + } ?: return null + + while (true) { + val next = + byVersion[head.version + 1] + ?.filter { it.prevHash != null && it.prevHash.toHexKey() == head.hashHex } + ?.minByOrNull { it.rumorId } + ?: break + head = next + } + return head + } + /** * Groups mixed [editions] by entity id and folds each to its head, honoring the * per-entity anti-rollback [floors] (keyed by [ControlEdition.entityIdHex]). @@ -210,10 +264,16 @@ object EditionFold { // to the compacted head. Presence of the entity in the snapshot selects the ARM; // version selects the HEAD, over every edition we hold and not just the subset. if (floor != null && snapshot != null && editions.any { it.rumorId in snapshot }) { + // Chain first, bootstrap only as the fallback. The arm exists for the case where the + // offered head genuinely cannot be connected — but when it CAN be, the connected head is + // strictly better evidence than "highest number wins", and preferring it denies a stray + // high-version edition its free win in every ordinary fold. The bootstrap keeps the + // cross-epoch case working, now bounded by MAX_COMPACTION_VERSION_JUMP. + chainHead(editions, floor)?.let { return it } return bootstrapHead(editions, floor.version) ?: run { - // Nothing at or above the floor was served: the head we already accepted - // vanished from the offered set — withheld, so fail closed. + // Nothing admissible at or above the floor was served: the head we already + // accepted vanished from the offered set — withheld, so fail closed. onGap(editions[0].entityIdHex, floor.version, editions.maxOf { it.version }) floor.known } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefounding.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefounding.kt index 18df7e3c46..7dc80de9cb 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefounding.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefounding.kt @@ -21,6 +21,7 @@ package com.vitorpamplona.quartz.concord.cord06Rekey import com.vitorpamplona.quartz.concord.cord04Roles.ControlEdition +import com.vitorpamplona.quartz.concord.cord04Roles.EditionFold import com.vitorpamplona.quartz.concord.crypto.ConcordKeyDerivation import com.vitorpamplona.quartz.concord.crypto.ControlPlaneKeys import com.vitorpamplona.quartz.concord.crypto.GroupKey @@ -167,18 +168,29 @@ object ConcordRefounding { priorControlKeys: ControlPlaneKeys, newControlKeys: ControlPlaneKeys, ): List { - // entity coordinate -> (head edition, its verified seal) - val heads = HashMap>() + // entity coordinate -> every edition we can open, paired with its verified seal. + val byCoordinate = HashMap>>() for (wrap in priorWraps) { val opened = ConcordStreamEnvelope.openOrNull(wrap, priorControlKeys) ?: continue val edition = ControlEdition.fromRumor(opened.rumor) ?: continue val coord = edition.entityKind.wire + ":" + edition.entityIdHex - val current = heads[coord] - if (current == null || edition.version > current.first.version) { - heads[coord] = edition to opened.seal - } + byCoordinate.getOrPut(coord) { ArrayList() }.add(edition to opened.seal) } - return heads.values.map { (_, seal) -> ConcordStreamEnvelope.wrapSeal(seal, newControlKeys, createdAt = seal.createdAt) } + + // The head is the CHAIN head, not the highest version. Picking by raw version made an honest + // rotator the delivery mechanism for a disconnected stray: an edition minted at an arbitrary + // version never joins the chain, but it won this comparison and was then re-wrapped into the + // new epoch as that entity's whole history — where a fresh joiner, holding no floor, anchors + // on it as their baseline. See B1 in `docs/concord-soft-ban-audit.md`. foldEntity walks from + // genesis and keeps the fresh-joiner fallback for a head whose own `prev` dangles into an + // epoch this rotator no longer holds, which is the ordinary shape after a prior compaction. + val out = ArrayList(byCoordinate.size) + for ((_, entries) in byCoordinate) { + val head = EditionFold.foldEntity(entries.map { it.first }) ?: continue + val seal = entries.firstOrNull { it.first.rumorId == head.rumorId }?.second ?: continue + out.add(ConcordStreamEnvelope.wrapSeal(seal, newControlKeys, createdAt = seal.createdAt)) + } + return out } /** diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/ControlPlaneVersionExhaustionTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/ControlPlaneVersionExhaustionTest.kt index bbba1dc8d3..c97ed0dba9 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/ControlPlaneVersionExhaustionTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/ControlPlaneVersionExhaustionTest.kt @@ -27,35 +27,43 @@ import kotlin.test.assertEquals import kotlin.test.assertTrue /** - * **V1 in `docs/concord-soft-ban-audit.md` — reproduction.** A single Control Plane edition at - * `version = Long.MAX_VALUE` pins its entity to the author's content permanently, for every client - * that holds a floor for it. + * **B1 in `docs/concord-soft-ban-audit.md` — regression guard.** A single Control Plane edition at + * `version = Long.MAX_VALUE` used to pin its entity to the author's content permanently, for every + * client that held a floor for it. These tests failed before the fix and pass after it. * - * The chain walk is not the weakness — it advances only to `head.version + 1` citing the head's - * hash, so an inflated version is unreachable and a fresh joiner is unaffected. The weakness is the - * **compaction arm** of [EditionFold.foldEntity]: once a client holds a floor for an entity and that - * entity appears in the epoch snapshot (which `ConcordCommunityState.fold` always builds from the - * editions handed to it), the head is chosen by [EditionFold.bootstrapHead] — *highest version at or - * above the floor*, with no `prev`, no hash, and no contiguity. Version is then the whole contest, - * and `Long.MAX_VALUE` wins it forever: + * The chain walk was never the weakness — it advances only to `head.version + 1` citing the head's + * hash, so an inflated version is unreachable and a fresh joiner was unaffected. The weakness was the + * **compaction arm** of `EditionFold.foldEntity`: once a client held a floor for an entity and that + * entity appeared in the epoch snapshot (which `ConcordCommunityState.fold` always builds from the + * editions handed to it), the head came from the raw-version bootstrap — *highest version at or above + * the floor*, with no `prev`, no hash, and no contiguity. Version was then the whole contest, and + * `Long.MAX_VALUE` won it forever: * - * 1. the poison becomes the head, so the entity shows the attacker's content; - * 2. `authorizedHeads` raises the entity's floor to `Long.MAX_VALUE`; - * 3. no honest edition can ever exceed that floor, so the entity can never be repaired; - * 4. a Refounding that drops the poison does not help either — nothing is offered at or above the - * floor, so the fold reports a gap and falls back to [EntityFloor.known], which *is* the poison. + * 1. the poison became the head, so the entity showed the attacker's content; + * 2. `authorizedHeads` raised the entity's floor to `Long.MAX_VALUE`; + * 3. no honest edition could ever exceed that floor, so the entity could never be repaired; + * 4. a Refounding that dropped the poison did not help either — nothing was then offered at or above + * the floor, so the fold reported a gap and fell back to `EntityFloor.known`, which *was* the poison. + * + * Two changes close it, and both are pinned below. The arm now tries the floor-anchored **chain** + * first and only falls back to the raw-version bootstrap when nothing connects, so a stray never wins + * a fold where the honest chain is present; and the bootstrap will not follow a jump larger than + * [EditionFold.MAX_COMPACTION_VERSION_JUMP], so the version space cannot be exhausted in one step. + * [aGenuineCompactionJumpIsStillFollowed] pins the tolerance the arm exists for, so the bound cannot + * be tightened into breaking CORD-06 §3. * * Note who the attacker is. Every test here is authored by **bob, a current and legitimately granted * moderator** — not a banned member, not a sockpuppet. Any holder of the entity's permission bit can - * do this at any time, and demoting or banning them afterwards changes nothing, because the damage - * is already in every client's floor. It is also carried into every future epoch by - * `ConcordRefounding.compactControlPlane`, which selects the head per entity by raw highest version. + * could do this at any time, and demoting or banning them afterwards changed nothing, because the + * damage was already in every client's floor. `ConcordRefounding.compactControlPlane` also selected + * the head per entity by raw highest version, which made an honest rotator the delivery mechanism — + * it now picks the chain head instead. * - * The banlist is the one entity that survives, and by accident: `AuthorityResolver` folds it with + * The banlist was the one entity that survived, and by accident: `AuthorityResolver` folds it with * its own floor-less chain walk and then re-heals the union across authorized editions, so an - * honest ban lands even when the head is poisoned. [aPoisonedBanlistStillAcceptsTheOwnersBan] pins - * that, because it is the only thing standing between this bug and a permanently unmoderatable - * community. + * honest ban landed even when the head was poisoned. [aPoisonedBanlistStillAcceptsTheOwnersBan] + * keeps pinning that, because it was the only thing standing between this bug and a permanently + * unmoderatable community. */ class ControlPlaneVersionExhaustionTest { private val owner = "0f".repeat(32) @@ -86,7 +94,7 @@ class ControlPlaneVersionExhaustionTest { ) + rest @Test - fun oneEditionAtMaxVersionPinsTheMetadataForever() { + fun oneEditionAtMaxVersionNoLongerPinsTheMetadata() { val metadataV0 = edition(ControlEntityKind.METADATA, metadataEntity, 0, null, """{"name":"My Community"}""", owner, "meta-0") val community = communityWhereBobHolds(ConcordPermissions.of(ConcordPermissions.MANAGE_METADATA).toWire(), metadataV0) @@ -96,7 +104,7 @@ class ControlPlaneVersionExhaustionTest { val poison = edition(ControlEntityKind.METADATA, metadataEntity, Long.MAX_VALUE, metadataV0.hash, """{"name":"PWNED"}""", bob, "meta-poison") val floorsAfter = ConcordCommunityState.authorizedHeads(community + poison, owner, floorsBefore) - assertEquals(Long.MAX_VALUE, floorsAfter[metadataEntity]?.version, "VULNERABLE: the floor is now at the top of the version space") + assertEquals(0, floorsAfter[metadataEntity]?.version, "the floor must not follow a stray to the top of the version space") // The owner tries to repair it, chaining honestly onto their own genesis. val repair = edition(ControlEntityKind.METADATA, metadataEntity, 1, metadataV0.hash, """{"name":"My Community"}""", owner, "meta-1") @@ -108,19 +116,19 @@ class ControlPlaneVersionExhaustionTest { "a fresh joiner walks the chain and is unaffected", ) assertEquals( - "PWNED", + "My Community", ConcordCommunityState.fold(pool, owner, floorsAfter).metadata?.name, - "VULNERABLE: every client holding a floor is pinned to the attacker's content", + "a client holding a floor follows the honest chain, not the stray", ) assertEquals( - "PWNED", + "My Community", ConcordCommunityState.fold(community + repair, owner, floorsAfter).metadata?.name, - "VULNERABLE: even a Refounding that drops the poison falls back to it as EntityFloor.known", + "and a Refounding that drops the poison stays repaired", ) } @Test - fun oneEditionAtMaxVersionDeletesAChannelForever() { + fun oneEditionAtMaxVersionNoLongerDeletesAChannel() { val channelV0 = edition(ControlEntityKind.CHANNEL, channelEntity, 0, null, """{"name":"general"}""", owner, "chan-0") val community = communityWhereBobHolds(ConcordPermissions.of(ConcordPermissions.MANAGE_CHANNELS).toWire(), channelV0) @@ -132,28 +140,29 @@ class ControlPlaneVersionExhaustionTest { val pool = community + poison + repair assertEquals(1, ConcordCommunityState.fold(pool, owner).channels.size, "a fresh joiner still sees the channel") - assertEquals(0, ConcordCommunityState.fold(pool, owner, floorsAfter).channels.size, "VULNERABLE: the channel is gone and cannot be restored") + assertEquals(1, ConcordCommunityState.fold(pool, owner, floorsAfter).channels.size, "and so does a client holding a floor") assertEquals( - 0, + 1, ConcordCommunityState.fold(community + repair, owner, floorsAfter).channels.size, - "VULNERABLE: dropping the poison does not bring the channel back", + "the channel survives a Refounding too", ) } @Test fun aPoisonedBanlistStillAcceptsTheOwnersBan() { - // The saving grace, and the reason this bug is "unmoderatable community" rather than - // "community with a broken name". AuthorityResolver folds the banlist on its own floor-less - // chain walk and re-heals the union across every authorized edition, so the owner's ban lands - // even while the banlist's own floor sits at Long.MAX_VALUE. Do not "unify" the banlist onto - // the floored fold without replacing this protection. + // This was the saving grace before the fix — the reason the bug was "community with a broken + // name" rather than "community nobody can moderate". AuthorityResolver folds the banlist on + // its own floor-less chain walk and re-heals the union across every authorized edition, so + // the owner's ban landed even while the banlist's floor sat at Long.MAX_VALUE. The floor can + // no longer be poisoned, but keep this: do not "unify" the banlist onto the floored fold + // without replacing the protection. val banlistV0 = edition(ControlEntityKind.BANLIST, banlistEntity, 0, null, "[]", owner, "ban-0") val community = communityWhereBobHolds(ConcordPermissions.of(ConcordPermissions.BAN).toWire(), banlistV0) val floorsBefore = ConcordCommunityState.authorizedHeads(community, owner) val poison = edition(ControlEntityKind.BANLIST, banlistEntity, Long.MAX_VALUE, banlistV0.hash, "[]", bob, "ban-poison") val floorsAfter = ConcordCommunityState.authorizedHeads(community + poison, owner, floorsBefore) - assertEquals(Long.MAX_VALUE, floorsAfter[banlistEntity]?.version, "the banlist floor is poisoned like any other") + assertEquals(0, floorsAfter[banlistEntity]?.version, "the banlist floor is no longer poisonable either") val ownerBansBob = edition(ControlEntityKind.BANLIST, banlistEntity, 1, banlistV0.hash, """["$bob"]""", owner, "ban-1") val pool = community + poison + ownerBansBob @@ -164,4 +173,51 @@ class ControlPlaneVersionExhaustionTest { "the re-heal union must keep the banlist working even with a poisoned floor", ) } + + @Test + fun aGenuineCompactionJumpIsStillFollowed() { + // The tolerance the compaction arm exists for, pinned so the bound above cannot be tightened + // into breaking CORD-06 §3. After a Refounding the compacted head carries the `prev` it had + // before compaction, citing an edition in the PRIOR epoch that this client no longer holds — + // so it connects to nothing, and its version is legitimately several ahead of our floor. + val metadataV0 = edition(ControlEntityKind.METADATA, metadataEntity, 0, null, """{"name":"My Community"}""", owner, "meta-0") + val community = communityWhereBobHolds(ConcordPermissions.of(ConcordPermissions.MANAGE_METADATA).toWire(), metadataV0) + val floors = ConcordCommunityState.authorizedHeads(community, owner) + + val danglingPrev = ByteArray(32) { 0x7f } + val compacted = edition(ControlEntityKind.METADATA, metadataEntity, 4, danglingPrev, """{"name":"Renamed While We Were Away"}""", owner, "meta-compacted") + + assertEquals( + "Renamed While We Were Away", + ConcordCommunityState.fold(community.filter { it.entityKind != ControlEntityKind.METADATA } + compacted, owner, floors).metadata?.name, + "a compacted head whose prev dangles by design must still be adopted", + ) + } + + @Test + fun aJumpBeyondTheCapIsRefusedAsAGap() { + // Same shape as the genuine compaction above, one version past the bound: not a compaction we + // missed, so the fold reports a gap and keeps what it already had rather than following it. + val metadataV0 = edition(ControlEntityKind.METADATA, metadataEntity, 0, null, """{"name":"My Community"}""", owner, "meta-0") + val community = communityWhereBobHolds(ConcordPermissions.of(ConcordPermissions.MANAGE_METADATA).toWire(), metadataV0) + val floors = ConcordCommunityState.authorizedHeads(community, owner) + + val danglingPrev = ByteArray(32) { 0x7f } + val tooFar = + edition( + ControlEntityKind.METADATA, + metadataEntity, + EditionFold.MAX_COMPACTION_VERSION_JUMP + 1, + danglingPrev, + """{"name":"PWNED"}""", + bob, + "meta-far", + ) + + assertEquals( + "My Community", + ConcordCommunityState.fold(community.filter { it.entityKind != ControlEntityKind.METADATA } + tooFar, owner, floors).metadata?.name, + "a jump past the bound is a gap, not a head", + ) + } } From 4e99aafb59f5f081f5fe6f13924abcc15ee38f8e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 15:47:41 +0000 Subject: [PATCH 09/13] fix(quartz): honor the banlist against the Control Plane itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit B2 in docs/concord-soft-ban-audit.md, plus B4's bound and the audit's status pass. hasPermission was ban-aware; the resolver's own ROLE/GRANT/BANLIST gates were not, and could not be as written — the roles/grants fixpoint settled before `banned` was computed at all. So half the Control Plane honored a ban and half was blind to it, and a banned staffer still holding control_root kept the whole roster: banning everyone beneath them, revoking the surviving moderators, retiring the roles under them, and minting a fresh un-banned npub that passed every ban-aware gate and finished the job. resolve() is now a bounded two-pass where authority only ever shrinks. Pass A resolves as before and yields a candidate banlist; pass B re-resolves with every author on it treated as holding no authority. Two passes always, so it terminates by construction, and mutual bans cannot oscillate because the rank rule makes them unreachable — only someone who strictly outranks you may ban you, and you cannot outrank them back. A chain-local rule would not have worked: forking the banlist at genesis means no parent ever mentions the ban and §4's re-heal union carries it in regardless, so the rule is a whole-pass mask rather than a per-edition check. This cascades, deliberately: every edition a banned member ever authored is dropped, grants included, so banning an admin also demotes everyone that admin promoted. That is the literal reading of CORD-04 §4 and it is what kills the sockpuppet, but a legitimate promotion by a later-banned admin vanishes with it and has to be re-issued. Both the cascade and its blast radius are pinned, and the trade-off is written up in the Armada report as the answer to its own open row 3 — which also widens the divergence recorded there: we now drop editions they honor wherever a privileged member was banned. B4: the Refounding recipient set is capped. allMembers() is the Guestbook ∪ observedAuthors ∪ the roster, and the first two are unbounded and attacker-writable, so each throwaway npub someone posts from became one more mandatory blob in the next Refounding — the attack inflating the cost of its own remedy. The owner-rooted roster is kept first and anything dropped is logged, never silently truncated, because a dropped member is stranded. The nine escalation reproductions now assert the fixed behaviour. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- .../amethyst/model/AccountConcordActions.kt | 48 ++++++- docs/concord-banlist-rank-conformance.md | 35 ++++- docs/concord-soft-ban-audit.md | 134 ++++++++++++------ .../concord/cord04Roles/AuthorityResolver.kt | 52 ++++++- .../cord04Roles/BannedStaffEscalationTest.kt | 111 ++++++++++----- 5 files changed, 299 insertions(+), 81 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt index c403e08986..5b5c8fe0db 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt @@ -33,6 +33,7 @@ import com.vitorpamplona.quartz.concord.cord02Community.ConcordCommunityListEven import com.vitorpamplona.quartz.concord.cord02Community.HeldRoot import com.vitorpamplona.quartz.concord.cord02Community.ImagePointer import com.vitorpamplona.quartz.concord.cord03Channels.ChannelChat +import com.vitorpamplona.quartz.concord.cord04Roles.AuthorityResolver import com.vitorpamplona.quartz.concord.cord04Roles.ChannelEntity import com.vitorpamplona.quartz.concord.cord04Roles.ConcordJson import com.vitorpamplona.quartz.concord.cord04Roles.ConcordPermissions @@ -76,6 +77,15 @@ private const val CONCORD_ADMIN_ROLE = "Admin" */ private const val RECOVERY_CHECK_INTERVAL_MS = 15 * 60 * 1000L +/** + * How many recipients one Refounding will re-key. See `AccountConcordActions.boundRecipients`. + * + * 120 blobs ride in each kind-3303 chunk, so this is ~42 published events and ~5k NIP-44 + * encryptions at the ceiling — heavy but survivable on a phone, and far above any real community. + * Raising it raises the cost of the attack it exists to bound, not the safety. + */ +private const val MAX_REFOUNDING_RECIPIENTS = 5_000 + /** * Concord (encrypted communities) orchestration for an [Account]: join/create/ * invite flows, channel messages/reactions/edits/typing, roles and moderation, @@ -810,7 +820,7 @@ class AccountConcordActions( .apply { removeAll(removedLower) removeAll(authority.bannedMembers()) - }.toList() + }.let { candidates -> boundRecipients(candidates, authority) } // 3. Build the refounding: new root, compacted Control Plane, per-recipient rekey blobs. val entry = session.entry @@ -853,6 +863,42 @@ class AccountConcordActions( return true } + /** + * Caps the Refounding recipient set, keeping the members whose standing we can actually vouch + * for when there are too many. + * + * `allMembers()` is the Guestbook ∪ `observedAuthors` ∪ the roster, and the first two are + * unbounded and attacker-writable: a Guestbook Join is self-signed by any key at all, and every + * author we decrypt is folded in by design (CORD-02 §5, "observably present"). So each throwaway + * npub someone posts from, or simply announces, becomes one more mandatory blob in the next + * Refounding — meaning the attack inflates the cost of its own remedy, and the remedy is the only + * hard removal Concord has. See B4 in `docs/concord-soft-ban-audit.md`. + * + * The roster and the owner are kept unconditionally: they are owner-rooted, so they cannot be + * padded from outside. The remainder fills the budget, and anything dropped is **logged rather + * than silently truncated** — a dropped member is stranded on the dead epoch and their only way + * back is a recovery path that needs to know it happened. + */ + private fun boundRecipients( + candidates: Set, + authority: AuthorityResolver, + ): List { + if (candidates.size <= MAX_REFOUNDING_RECIPIENTS) return candidates.toList() + + val vouched = authority.roleHolders() + authority.staffMembers() + val kept = LinkedHashSet(MAX_REFOUNDING_RECIPIENTS) + candidates.filterTo(kept) { it in vouched } + for (candidate in candidates) { + if (kept.size >= MAX_REFOUNDING_RECIPIENTS) break + kept.add(candidate) + } + Log.w("Concord") { + "Refounding recipient set capped at $MAX_REFOUNDING_RECIPIENTS of ${candidates.size}: " + + "${candidates.size - kept.size} member(s) will be stranded on the prior epoch" + } + return kept.toList() + } + // Rotations we've already adopted ("communityId:epoch"), so a base-rekey wrap still buffered // in the pre-rebuild window (the session rebuild off `liveCommunities` is async) is not // adopted — and re-published — twice on successive revision ticks. diff --git a/docs/concord-banlist-rank-conformance.md b/docs/concord-banlist-rank-conformance.md index afbe98fdbd..04e92c9de7 100644 --- a/docs/concord-banlist-rank-conformance.md +++ b/docs/concord-banlist-rank-conformance.md @@ -1,7 +1,8 @@ # Concord: the Banlist is not rank-gated in any implementation (CORD-04 conformance) **Status:** conformance bug. Reproduced in Amethyst and **fixed there** (see §6); present by -inspection in Armada. +inspection in Armada. Finding #3, left open in §4 as a fixpoint-ordering question, is now also +implemented in Amethyst — see the 2026-08-09 update before §Rollout status. **Severity:** privilege escalation. Any `BAN` holder can neutralise every authority above them, including the owner. **Reported by:** Amethyst (MIT), 2026-07-20. Findings verified by unit test; see "Evidence" below. @@ -190,6 +191,38 @@ We'd also suggest **§4 restating the rank half inline**, the way §2 does for G does for Kicks. Both independent implementations read §4 in isolation and both got it wrong the same way; that is strong evidence the section is the problem, not the readers. +### Update, 2026-08-09: we have now implemented #3 + +Amethyst now answers the ordering question rather than leaving it open, because #3 turned out to be +the doorway to a full community takeover and not merely an inconsistency — a banned staffer who kept +`control_root` kept the entire roster, and could mint a fresh un-banned npub that passed every +ban-aware gate. The write-up is `docs/concord-soft-ban-audit.md` (B2). + +The rule we shipped: **authority only ever shrinks, over two passes.** Pass A resolves exactly as +before and yields a candidate banlist; pass B re-resolves with every author on that list treated as +holding no authority at all, for roles, grants and the Banlist alike. Two passes, always, so it +terminates by construction. It cannot oscillate on mutual bans either, because the rank rule makes +them unreachable: only a member who strictly outranks you may ban you, and you cannot outrank them +back. + +Note what it costs, because it is not obvious and you would hit it too: this **cascades**. Every +edition a banned member ever authored is dropped, grants included, so banning an admin also demotes +everyone that admin promoted. We think that is the literal reading of §4 and it is what kills the +sockpuppet — but a legitimate promotion by a later-banned admin vanishes with it, and the owner has +to re-issue it. If you read §4 as scoping only to editions authored *after* the ban, say so; that is +implementable too, but it needs the spec to define an ordering between an edition and a Banlist +entry, which today it does not. + +A chain-local rule is not enough, and this is the trap worth flagging: "the author must not be banned +by the state their edition chains from" is bypassed by forking the Banlist at genesis, where no +parent ever mentions the ban and §4's re-heal union carries it in anyway. The rule has to bind the +union, which is why ours is a whole-pass mask rather than a per-edition check. + +This widens the divergence in §6: we now drop editions you honor in any community where a privileged +member was banned, not only where a signer failed to outrank their target. + +--- + Separately, please rule on **#3**: whether a banned npub's Banlist edition is honored. Our reading of §4 ("drops every event from a banned npub — message, reaction, edit, or authority action") is that it must not be, but the fixpoint ordering needs to be stated for that to be implementable consistently. diff --git a/docs/concord-soft-ban-audit.md b/docs/concord-soft-ban-audit.md index 58429394ca..1da43ed4d0 100644 --- a/docs/concord-soft-ban-audit.md +++ b/docs/concord-soft-ban-audit.md @@ -1,7 +1,9 @@ # Concord: soft-ban and Control Plane audit **Scope:** what a removed member — or a moderator who turns — can still do to a Concord community. -**Date:** 2026-08-09. **Status:** findings only, nothing fixed yet. +**Date:** 2026-08-09. **Status:** A1–A4, B1, B2 and B4 are **fixed** on this branch; the rest are +accepted, deferred to the spec, or belong to other people's relays. Each section carries its own +status line. **Companion:** `docs/concord-banlist-rank-conformance.md` (the rank half of CORD-04 §4, already reported to Armada and fixed here). @@ -47,32 +49,32 @@ Two structural causes account for most of both halves: ### Part A — reachable from stock Amethyst (our bugs) -| # | Finding | Severity | Was | -|---|---------|----------|-----| -| [A1](#a1) | Any member — banned included — mints a working invite in one tap | **Critical** | new | -| [A2](#a2) | Stranded recovery runs on a timer and never checks the banlist | **Critical** | V10 | -| [A3](#a3) | The action layer has no permission checks; the UI's are ban-blind | High | new | -| [A4](#a4) | A banned member keeps broadcasting "typing", and we keep showing it | Low | V12 | -| [A5](#a5) | A banned member's own client keeps reading and rendering everything | Medium | V8 | +| # | Finding | Severity | Status | +|---|---------|----------|--------| +| [A1](#a1) | Any member — banned included — mints a working invite in one tap | **Critical** | **Fixed** | +| [A2](#a2) | Stranded recovery runs on a timer and never checks the banlist | **Critical** | **Fixed** (security half; liveness half open) | +| [A3](#a3) | The action layer has no permission checks; the UI's are ban-blind | High | **Fixed** | +| [A4](#a4) | A banned member keeps broadcasting "typing", and we keep showing it | Low | **Fixed** | +| [A5](#a5) | A banned member's own client keeps reading and rendering everything | Medium | Inherent — product decision | ### Part B — requires a malicious client -| # | Finding | Severity | Needs a ban? | Recoverable? | Was | -|---|---------|----------|--------------|--------------|-----| -| [B1](#b1) | One edition at `version = Long.MAX_VALUE` pins an entity forever | **Critical** | No — any bit-holder | **No** | V1 | -| [B2](#b2) | A banned staffer keeps Role/Grant/Banlist authority | **Critical** | Yes | Yes (Refounding) | V2 | -| [B3](#b3) | A rogue rotator compacts the banlist away | High | Via B2 | Partly | V3 | -| [B4](#b4) | The Refounding recipient set is attacker-inflatable | High | No | Yes | V4 | -| [B5](#b5) | The ban is per-pubkey; the channel key is not revoked | High | Yes | Yes (Refounding) | V5 | -| [B6](#b6) | Channel history is deletable on a naive third-party relay | High | Yes | **No** (history) | V6 | -| [B7](#b7) | The base-rekey plane is writable by every member | Low | Yes | Yes | V9 | +| # | Finding | Severity | Needs a ban? | Status | +|---|---------|----------|--------------|--------| +| [B1](#b1) | One edition at `version = Long.MAX_VALUE` pins an entity forever | **Critical** | No — any bit-holder | **Fixed** | +| [B2](#b2) | A banned staffer keeps Role/Grant/Banlist authority | **Critical** | Yes | **Fixed** (consensus-affecting) | +| [B3](#b3) | A rogue rotator compacts the banlist away | High | Via B2 | **Mitigated** by B2 | +| [B4](#b4) | The Refounding recipient set is attacker-inflatable | High | No | **Fixed** (bounded) | +| [B5](#b5) | The ban is per-pubkey; the channel key is not revoked | High | Yes | Inherent — Refounding is the answer | +| [B6](#b6) | Channel history is deletable on a naive third-party relay | High | Yes | Correct here; external relays at risk | +| [B7](#b7) | The base-rekey plane is writable by every member | Low | Yes | Accepted | ### Part C — interop and not-yet-shipped -| # | Finding | Severity | Was | -|---|---------|----------|-----| -| [C1](#c1) | Banlist rank rule diverges from Armada | Medium | V7 | -| [C2](#c2) | CORD-07 voice rooms are key-gated, not roster-gated | Design | V11 | +| # | Finding | Severity | Status | +|---|---------|----------|--------| +| [C1](#c1) | Banlist rank rule diverges from Armada | Medium | Reported; B2 widens the divergence | +| [C2](#c2) | CORD-07 voice rooms are key-gated, not roster-gated | Design | Note for whoever ships voice | --- @@ -82,6 +84,9 @@ No custom tooling. A banned user with the shipping app, or our own background sw ## A1 — Any member, banned included, mints a working invite in one tap +**Status: fixed.** `mintConcordInvite` and its button now require `CREATE_INVITE` (or ownership). + + **Critical. The single most likely thing an irritated banned user actually does.** *Read:* `AccountConcordActions.mintConcordInvite`, `ConcordChannelListScreen` (the `PersonAdd` `IconButton`). @@ -106,6 +111,12 @@ button on the same. This is contained, uncontroversial, and closes the realistic ## A2 — Stranded recovery runs on a timer and never checks the banlist +**Status: security half fixed; liveness half open.** `isStranded` / `mergeForward` now take +`bannedAtCurrentEpoch` as a *required* argument, so a removed member is no longer walked back in. +Whether anything should re-mint at a stable coordinate — without which legitimate recovery never +fires for anyone — still needs a spec answer and is untouched. + + **Critical, and it forks.** *Read:* `ConcordStrandedRecovery`, `AccountConcordActions.recoverStrandedConcordCommunities`, `AccountConcordActions.mintConcordInvite`. @@ -137,6 +148,10 @@ evicted owners another route. ## A3 — The action layer has no permission checks; the UI's are ban-blind +**Status: fixed.** Authority now lives in `AccountConcordActions.isAuthorizedFor`, which every +moderation verb funnels through, and every authorization test uses the ban-aware `hasPermission`. + + **High (defense in depth).** *Read:* `AccountConcordActions` (`banConcordMember`, `unbanConcordMember`, `editConcordMetadata`, `deleteConcordChannel`, `refoundConcordCommunity`), `ConcordMembersScreen`, `ConcordChannelListScreen`. @@ -169,6 +184,9 @@ their roles say", independent of standing. ## A4 — A banned member keeps broadcasting "typing", and we keep showing it +**Status: fixed on both ends.** + + **Low, both halves ours.** *Read:* `AccountConcordActions.sendConcordTyping`, `ConcordCommunitySession.ingestTyping`. @@ -180,6 +198,11 @@ and it directly contradicts what a ban promises the user. ## A5 — A banned member's own client keeps reading and rendering everything +**Status: inherent; no code change.** The cryptography cannot be fixed without a Refounding, so what +is left is a product decision about how "Ban" and "Remove from community" are presented. Left for a +design pass rather than guessed at here. + + **Medium, partly inherent.** *Read:* CORD-02/05, `ConcordCommunitySession`. Until a Refounding, a ban stops honest clients from *showing* the banned member's posts; it does not @@ -201,6 +224,12 @@ what they publish. ## B1 — One edition at `Long.MAX_VALUE` pins an entity forever +**Status: fixed.** The compaction arm tries the floor-anchored chain first and bounds the bootstrap +jump at `EditionFold.MAX_COMPACTION_VERSION_JUMP`; `compactControlPlane` picks the chain head rather +than raw max version. The three reproductions now assert the fixed behaviour, and +`aGenuineCompactionJumpIsStillFollowed` pins the CORD-06 §3 tolerance the bound must not break. + + **Critical. Does not require a banned user, a sockpuppet, or the owner's absence. Unrecoverable.** *Verified:* `quartz/…/cord04Roles/ControlPlaneVersionExhaustionTest.kt` (3 tests). @@ -246,6 +275,14 @@ The first is the smallest change and closes the unrecoverability; the third shou ## B2 — A banned staffer keeps Role, Grant and Banlist authority +**Status: fixed — and consensus-affecting.** `AuthorityResolver.resolve` is now a bounded two-pass +where authority only shrinks. Note the deliberate cascade it brings: every edition a banned member +ever authored is dropped, so banning an admin also demotes everyone that admin promoted. That is the +literal reading of CORD-04 §4 and it is what kills the sockpuppet, but a legitimate promotion by a +later-banned admin vanishes with it and has to be re-issued. Until Armada ships the same rule the two +clients can disagree about any community where a privileged member was banned. + + **Critical.** *Verified:* `quartz/…/cord04Roles/BannedStaffEscalationTest.kt` (13 tests). `hasPermission` is ban-aware; the resolver's internal gates are not, and structurally cannot be as @@ -277,6 +314,11 @@ Armada ships the same rule, we will drop editions they honor. ## B3 — A rogue rotator compacts the banlist away +**Status: mitigated by B2.** The rotator this needed was the sockpuppet, which can no longer be +minted. A *legitimately* privileged rotator can still omit the banlist, and `EntityFloor` remains the +only defense for clients that already folded it — unchanged, and still worth a spec fix. + + **High.** *Verified:* `aRogueRotatorCompactsTheBanAwayForEveryClientWithoutAFloor`. A CORD-06 §3 compaction re-wraps one edition per entity and the *rotator* picks it, so a rotator can @@ -292,6 +334,10 @@ protect people who were already there. ## B4 — The Refounding recipient set is attacker-inflatable +**Status: fixed (bounded).** The recipient set is capped, the owner-rooted roster is kept first, and +anything dropped is logged rather than silently truncated. + + **High.** *Read:* `ConcordCommunitySession.allMembers()` / `emitChannelRumors`; `AccountConcordActions.refoundConcordCommunity` step 2; `ConcordRefounding.buildBaseRekeyWraps`. @@ -311,6 +357,9 @@ the recipient set, prefer recent/attested members when over the cap, and surface ## B5 — The ban is a per-pubkey display rule and the channel key is not revoked +**Status: inherent.** No client-side fix exists; a Refounding is the answer, which is why B4 mattered. + + **High.** *Read:* `Account.consumeConcordRumorGated` (`isBanned(rumor.pubKey)`), `Account.isAcceptable`. Writing to a channel needs the channel key, which the ban does not take away; the seal author is @@ -324,6 +373,10 @@ correct design, which is why B4 matters so much. ## B6 — Channel history is deletable on a naive third-party relay +**Status: correct on our relay and pinned; external relays remain exposed.** Needs a CORD-01 spec note +and relay-selection guidance, not code. + + **High, external.** *Verified (that we are safe):* `geode/…/ConcordPlaneKeyDeletionTest.kt` (3 tests). @@ -346,6 +399,9 @@ Worth a note in the CORD-01 spec and a line in the relay-selection guidance. ## B7 — The base-rekey plane is writable by every member +**Status: accepted.** Bounded work per wrap, no correctness impact. + + **Low.** *Read:* `ConcordKeyDerivation.baseRekeyAddress`, `AccountConcordActions.drainConcordRekeys`. The base-rekey address derives from `community_root`, so any member — banned included — can mint @@ -406,25 +462,19 @@ Not looked at at all: - Unread counts and notification triggers, media/upload references from messages, the NIP-53 nests overlap, and the desktop client's Concord paths. -## Suggested order +## What is left -**Part A first.** It is the whole of the realistic threat — a banned user with the app already -installed — and none of it needs coordination with anyone. - -1. **A1** — one guard on `mintConcordInvite` plus one on its button. Smallest fix on the list and it - closes the attack a banned user will actually reach for. -2. **A3** — move authority into the action layer and replace `effectivePermissions` with - `hasPermission` everywhere it is used as an authorization test. This is also the cheapest partial - mitigation for B2: it shrinks what a banned staffer can do *without* writing their own client. -3. **A2** — needs the semantics decided before any code. Raise it with the spec. -4. **A4 / A5** — small, user-visible, and they make the product honest about what a ban is. - -**Then Part B**, hardest first because the ceiling is highest: - -5. **B4** — cheap, not consensus-affecting, and it protects the remedy every other fix depends on. -6. **B1** — worst blast radius, the only unrecoverable one, and the bar is a single ordinary - permission bit. -7. **B2 (+B3, +C1's open row)** — one two-pass change closes all three. Coordinate with Armada - first; this one splits consensus. -8. **B6** — spec note plus relay-selection guidance; our own behaviour is already correct and pinned. -9. **B5 / B7** — accept, or bound. +1. **A2's liveness half** — decide whether a community re-mints its invite bundle at a stable + coordinate. Today nothing does, so stranded recovery never fires for anyone, and an owner evicted + by a rogue admin has no route back. Needs a spec answer before code. +2. **C1 / B2 interop** — tell Armada about the two-pass rule, as with the rank rule before it. The + divergence is now wider: we drop editions they honor whenever a privileged member is banned. +3. **B6** — a CORD-01 note that a plane's wraps must stay owned by a key nobody holds, plus guidance + that a relay authorizing NIP-09/62 by `pubkey` hands every ex-member a wipe button. +4. **B3's residue** — a legitimately privileged rotator can still omit an entity during compaction. + `EntityFloor` catches it for clients that were present; fresh joiners have nothing. +5. **A5** — a design pass on how "Ban" and "Remove from community" are presented, since they promise + very different things. +6. **The unexamined surfaces below**, particularly private channels — there appears to be no + channel-scoped rekey receive path at all, which would mean the full-community Refounding is the + only removal Amethyst can perform. diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt index 039a63e7ca..ff85147ea6 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt @@ -135,9 +135,54 @@ data class AuthorityResolver private constructor( /** The owner's rank — supreme and unremovable. No Role may claim it. */ const val OWNER_RANK = 0L + /** + * The owner-rooted authority state of a community, with the banlist honored **against the + * Control Plane itself** (CORD-04 §4: a reader "drops every event from a banned npub — + * message, reaction, edit, or authority action"). + * + * This is a bounded two-pass, because the rule is circular as stated: you cannot know who is + * banned until you fold the Banlist, and you cannot decide who may write the Banlist without + * knowing who is banned. `docs/concord-banlist-rank-conformance.md` §4 row 3 flagged that to + * the spec authors and left it open. We resolve it by making authority only ever **shrink**: + * + * - **Pass A** resolves exactly as before, ban-blind, and yields a candidate banlist. + * - **Pass B** re-resolves with every author in that banlist treated as unauthorized, for + * roles, grants and the banlist alike. + * + * Two passes, always, so it terminates by construction — pass B never feeds back. It cannot + * oscillate on mutual bans either, because the rank rule makes them unreachable: only a + * member who strictly outranks you may ban you, and you cannot outrank them back. + * + * **This cascades, deliberately.** Every edition a banned member ever authored is dropped, + * including grants they made while in good standing — so banning an admin also demotes + * everyone that admin promoted. That is the literal reading of §4, and it is the point: the + * escalation in `docs/concord-soft-ban-audit.md` B2 was a banned staffer minting a fresh, + * un-banned npub and acting through it, and dropping the grant is what kills the puppet. The + * cost is that a legitimate promotion by a later-banned admin vanishes too, and the owner has + * to re-issue it. + * + * **Consensus-affecting.** Armada gates the Control Plane on role-derived permissions alone, + * so until it ships the same rule the two clients can disagree about any community where a + * privileged member was banned. + */ fun resolve( editions: Collection, ownerPubKey: String, + ): AuthorityResolver { + val passA = resolveOnce(editions, ownerPubKey, bannedAuthors = emptySet()) + if (passA.banned.isEmpty()) return passA + return resolveOnce(editions, ownerPubKey, bannedAuthors = passA.banned) + } + + /** + * One resolution pass. [bannedAuthors] are treated as holding no authority at all — their + * role, grant and banlist editions are dropped rather than merely being unable to act on + * others. Empty on pass A; pass A's banlist on pass B. See [resolve]. + */ + private fun resolveOnce( + editions: Collection, + ownerPubKey: String, + bannedAuthors: Set, ): AuthorityResolver { val ownerLower = ownerPubKey.lowercase() @@ -191,6 +236,7 @@ data class AuthorityResolver private constructor( ): Boolean { val author = e.author.lowercase() if (author == ownerLower) return true + if (author in bannedAuthors) return false if (!holdsManageRoles(author)) return false val authorRank = rankOf(author) ?: return false val r = ConcordJson.decodeOrNull(e.content) ?: return false @@ -223,6 +269,7 @@ data class AuthorityResolver private constructor( fun grantGate(e: ControlEdition): Boolean { val granter = e.author.lowercase() if (granter == ownerLower) return true + if (granter in bannedAuthors) return false if (!holdsManageRoles(granter)) return false val granterRank = rankOf(granter) ?: return false val g = ConcordJson.decodeOrNull(e.content) ?: return false @@ -269,7 +316,9 @@ data class AuthorityResolver private constructor( // a concurrent ban is never lost, while an on-chain unban still takes effect. val allBanlist = editions.filter { it.entityKind == ControlEntityKind.BANLIST } - fun banGate(e: ControlEdition): Boolean = e.author.lowercase() == ownerLower || effectivePermissionsOf(e.author.lowercase()).has(ConcordPermissions.BAN) + fun banGate(e: ControlEdition): Boolean = + e.author.lowercase() == ownerLower || + (e.author.lowercase() !in bannedAuthors && effectivePermissionsOf(e.author.lowercase()).has(ConcordPermissions.BAN)) val authorizedBanlist = allBanlist.filter(::banGate) // CORD-04 §3's rank rule binds "every action", and it names banning as its example ("an @@ -292,6 +341,7 @@ data class AuthorityResolver private constructor( // owner is never a valid target — not even for themselves. if (target == ownerLower) return false if (author == ownerLower) return true + if (author in bannedAuthors) return false if (!effectivePermissionsOf(author).has(ConcordPermissions.BAN)) return false val authorRank = rankOf(author) ?: return false val targetRank = rankOf(target) ?: Long.MAX_VALUE // no roles ⇒ lowest authority diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt index aedadd885a..5b6c0fc64d 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt @@ -28,27 +28,31 @@ import kotlin.test.assertFalse import kotlin.test.assertTrue /** - * What a **soft-banned staffer** can still do to a community — the reproduction behind - * `docs/concord-banlist-rank-conformance.md` §4 row 3, which the report left open as "a genuine - * fixpoint-ordering question, not a plain oversight". + * **B2 in `docs/concord-soft-ban-audit.md` — regression guard.** What a soft-banned staffer used to + * be able to do to a community, and can no longer. Every test here failed before the two-pass rule + * in [AuthorityResolver.resolve] and passes after it. * - * The asymmetry these tests pin: [ConcordCommunityState.fold] gates METADATA / CHANNEL / INVITE - * through `authority.hasPermission`, which is `!isBanned && …`, but ROLE, GRANT and BANLIST are - * gated *inside* [AuthorityResolver.resolve] by `holdsManageRoles` / `bitsOf` / - * `effectivePermissionsOf` — none of which consult the banlist. Nor could they as written: the - * roles/grants fixpoint runs before `banned` is computed at all. So half the Control Plane honors - * a ban and half is structurally blind to it, and a banned member who still holds `control_root` - * keeps full authority over the roster. + * The asymmetry they were written to pin: [ConcordCommunityState.fold] gates METADATA / CHANNEL / + * INVITE through `authority.hasPermission`, which is `!isBanned && …`, but ROLE, GRANT and BANLIST + * were gated *inside* [AuthorityResolver.resolve] by `holdsManageRoles` / `bitsOf` / + * `effectivePermissionsOf` — none of which consulted the banlist, nor could they as written, since + * the roles/grants fixpoint ran before `banned` was computed at all. Half the Control Plane honored + * a ban and half was structurally blind to it, so a banned member still holding `control_root` kept + * full authority over the roster: they banned everyone beneath them, revoked the surviving + * moderators, retired the roles under them, and minted a fresh un-banned npub that passed every + * ban-aware gate and finished the job. * - * **These tests assert the CURRENT, VULNERABLE behaviour**, so the escalation cannot regress - * silently or be "fixed" by accident without someone noticing. Every `ESCALATION:` assertion here - * must be INVERTED — not deleted — when the ordering rule lands. [selfUnbanIsStillRefused] and - * [aJuniorPuppetCannotLiftASeniorsBan] are the opposite: they pin behaviour the fix must preserve. + * The fix resolves the ordering by making authority only ever shrink across two passes — see + * [AuthorityResolver.resolve]. Note that a chain-local rule ("the author must not be banned by the + * state their edition chains from") would NOT have been enough: + * [aBannedAdminForksTheBanlistAtGenesisRatherThanChainingOntoTheirOwnBan] forks at genesis so no + * parent ever mentions the ban, and CORD-04 §4's re-heal union would carry it in anyway. The rule + * had to bind the union too, which is why it is expressed as a whole-pass mask. * - * Note for whoever writes that fix: a chain-local rule ("the author must not be banned by the state - * their edition chains from") is NOT sufficient — see - * [aBannedAdminForksTheBanlistAtGenesisRatherThanChainingOntoTheirOwnBan]. The rule has to bind - * CORD-04 §4's re-heal union too. + * Three tests pin behaviour the fix had to *preserve* rather than change: + * [selfUnbanIsStillRefused], [aJuniorPuppetCannotLiftASeniorsBan], and + * [aBanByOneAdminDoesNotDropTheGrantsOfAnother]. One pins the cost it deliberately accepts: + * [banningAnAdminAlsoDemotesEveryoneThatAdminPromoted]. */ class BannedStaffEscalationTest { private val owner = "0f".repeat(32) @@ -167,10 +171,12 @@ class BannedStaffEscalationTest { assertTrue(r.isBanned(alice), "the owner's ban lands") assertFalse(r.hasPermission(alice, ConcordPermissions.MANAGE_ROLES), "the ban-aware check refuses her") - // ...but this is the one every ROLE/GRANT/BANLIST gate inside resolve() actually consults. + // effectivePermissions still reports what her ROLES say — that is its job, and the members + // screen reads it to label her. What changed is that the resolver's own ROLE/GRANT/BANLIST + // gates no longer consult it for a banned author; they drop the edition outright. assertTrue( r.effectivePermissions(alice).has(ConcordPermissions.MANAGE_ROLES), - "ESCALATION: a banned staffer keeps the permissions the resolver's own gates read", + "the role-derived view is unchanged — only what it authorizes is", ) } @@ -178,11 +184,11 @@ class BannedStaffEscalationTest { fun aBannedAdminPromotesAFreshSockpuppetToAdmin() { val r = AuthorityResolver.resolve(community() + ownerBansAlice + aliceMintsAPuppet(), owner) - assertEquals(2, r.rank(puppet), "ESCALATION: the banned admin's role edition is honored") - assertFalse(r.isBanned(puppet), "the puppet is a clean npub — nothing to filter it on") - assertTrue( + assertEquals(null, r.rank(puppet), "the banned admin's role and grant editions are both dropped") + assertFalse(r.isBanned(puppet), "the puppet itself is a clean npub — it is never banned, just powerless") + assertFalse( r.hasPermission(puppet, ConcordPermissions.MANAGE_CHANNELS), - "ESCALATION: a banned member minted a live admin with the ban-aware check passing", + "a banned member cannot mint authority it no longer has to give", ) } @@ -196,8 +202,8 @@ class BannedStaffEscalationTest { val state = ConcordCommunityState.fold(editions, owner) - assertEquals(0, state.channels.size, "ESCALATION: the community's channels are irrecoverably tombstoned") - assertEquals("Owned by the guy you banned", state.metadata?.name, "ESCALATION: and its identity rewritten") + assertEquals(1, state.channels.size, "the puppet holds nothing, so its tombstone is inert") + assertEquals("My Community", state.metadata?.name, "and the community keeps its identity") } @Test @@ -206,8 +212,8 @@ class BannedStaffEscalationTest { val r = AuthorityResolver.resolve(editions, owner) - assertTrue(r.isBanned(bob), "ESCALATION: the surviving moderator is silenced, losing all authority with it") - assertTrue(r.isBanned(carol), "ESCALATION: and the plain members with them") + assertFalse(r.isBanned(bob), "the puppet's banlist edition is unauthorized, so the moderator stands") + assertFalse(r.isBanned(carol), "and so do the plain members") } @Test @@ -216,8 +222,9 @@ class BannedStaffEscalationTest { val r = AuthorityResolver.resolve(editions, owner) - assertTrue(r.isBanned(bob), "ESCALATION: banGate reads effectivePermissionsOf, which ignores her own ban") - assertTrue(r.isBanned(carol), "ESCALATION: same") + assertTrue(r.isBanned(alice), "her own ban stands — it was the owner's") + assertFalse(r.isBanned(bob), "banGate now drops a banned author's edition outright") + assertFalse(r.isBanned(carol), "same") } @Test @@ -231,8 +238,8 @@ class BannedStaffEscalationTest { val r = AuthorityResolver.resolve(editions, owner) assertTrue(r.isBanned(alice), "the owner's ban survives the fork — the union is down-only") - assertTrue(r.isBanned(bob), "ESCALATION: and so does the banned admin's, healed in as a concurrent ban") - assertTrue(r.isBanned(carol), "ESCALATION: same") + assertFalse(r.isBanned(bob), "the fix binds the UNION too: her fork is dropped before it can be healed in") + assertFalse(r.isBanned(carol), "same") } @Test @@ -241,8 +248,8 @@ class BannedStaffEscalationTest { val r = AuthorityResolver.resolve(editions, owner) - assertEquals(null, r.rank(bob), "ESCALATION: a banned admin stripped a live moderator's roles") - assertFalse(r.hasPermission(bob, ConcordPermissions.BAN), "ESCALATION: leaving nobody but the owner able to act") + assertEquals(5, r.rank(bob), "a banned admin's revoke is dropped, so the moderator keeps their role") + assertTrue(r.hasPermission(bob, ConcordPermissions.BAN), "and keeps the authority that comes with it") } @Test @@ -251,8 +258,8 @@ class BannedStaffEscalationTest { val r = AuthorityResolver.resolve(community() + ownerBansAlice + tombstone, owner) - assertEquals(null, r.roles()[modRole], "ESCALATION: a banned admin retired a role beneath them") - assertEquals(null, r.rank(bob), "ESCALATION: every holder of it silently loses their standing") + assertEquals(5, r.roles()[modRole]?.position, "a banned admin's tombstone is dropped, so the role survives") + assertEquals(5, r.rank(bob), "and its holders keep their standing") } @Test @@ -323,4 +330,36 @@ class BannedStaffEscalationTest { "a client that already folded the ban must refuse the rollback", ) } + + @Test + fun banningAnAdminAlsoDemotesEveryoneThatAdminPromoted() { + // The deliberate cascade, pinned because it is surprising and because it is the whole point. + // CORD-04 §4 drops every event from a banned npub, authority actions included, so a grant + // they made while in good standing goes too. That is what kills a sockpuppet minted moments + // before the ban — and the same rule costs the owner a legitimate promotion, which they have + // to re-issue. See B2 in docs/concord-soft-ban-audit.md. + val promoted = grant("36".repeat(32), carol, listOf(modRole), author = alice) + + val before = AuthorityResolver.resolve(community() + promoted, owner) + assertEquals(5, before.rank(carol), "while alice is in good standing, her grant stands") + + val after = AuthorityResolver.resolve(community() + promoted + ownerBansAlice, owner) + assertEquals(null, after.rank(carol), "banning alice retroactively drops the grant she authored") + } + + @Test + fun aBanByOneAdminDoesNotDropTheGrantsOfAnother() { + // The cascade must follow the banned author, not spread. Bob is untouched by alice's ban, so + // everything he authored keeps standing. + val carolByBob = grant("37".repeat(32), carol, listOf(modRole), author = bob) + // bob is a Mod at position 5 and the role he hands out is that same position, so the grant is + // only honored when authored by someone who outranks it — the owner does, bob does not. + val carolByOwner = grant("38".repeat(32), carol, listOf(modRole), author = owner) + + val r = AuthorityResolver.resolve(community() + ownerBansAlice + carolByBob + carolByOwner, owner) + + assertTrue(r.isBanned(alice), "alice is the only one banned") + assertEquals(5, r.rank(bob), "bob is untouched") + assertEquals(5, r.rank(carol), "and the owner's grant of carol stands") + } } From 6147f72c8112e8a0e9652064a970695634b09743 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 15:54:55 +0000 Subject: [PATCH 10/13] docs(concord): check the audit against Armada, and correct two conclusions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read gitlab.com/soapbox-pub/armada src/concord-v2/ against every finding. Two conclusions change. B2 is NOT consensus-affecting, and the warning in the last commit was wrong. Armada's foldControlState already runs the same bounded two-pass — fold once, take the banlist, re-fold with banned authors' editions excluded — arrived at independently, same shape, same CORD-04 §4 justification in the comment. This change brings us into line rather than out of it. One narrower divergence remains: they keep pass 1's banlist as final, we recompute it in pass 2, so a banned admin's mass-ban still stands for them and is dropped by us. Both defensible; ours closes an attack theirs leaves open, and the self-erasure they guard against is unreachable under the rank rule. A2's fork is resolved, in favour of the fix having been necessary. useLinkRefreshWatch2 re-posts every invite bundle on each epoch change, so the "if anything re-mints at a stable coordinate" branch is what actually happens — in any cross-client community a removed member's Amethyst client would have pulled the new root within fifteen minutes. Their catch-up is push instead: a privileged member sends a direct invite carrying the fresher root, so a human authorizes each re-admission, and useBanSelfRemove2 has a banned member's own client silently drop the community. The liveness half stands and now has two concrete options rather than an open question. Also recorded: B1 is present in Armada unfixed, in exactly the same place (bootstrapHead is unbounded, headCandidates uses it, pickHead raises the floor) — the second bug both clients share by reading one section the same way, so it goes to them in writing like the rank rule did. A1 was ours alone; they gate invite creation on CREATE_INVITE in both the hook and the page. C1 is unchanged on their side. A4 is a shared gap. And a divergence in the other direction: their banlist takes only the head's content, with no §4 re-heal union, so we honor concurrent bans they drop. B4 is marked unchecked rather than guessed at — I could not locate their recipient-set construction with confidence. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- docs/concord-banlist-rank-conformance.md | 23 +++++++ docs/concord-soft-ban-audit.md | 80 +++++++++++++++++++++--- 2 files changed, 94 insertions(+), 9 deletions(-) diff --git a/docs/concord-banlist-rank-conformance.md b/docs/concord-banlist-rank-conformance.md index 04e92c9de7..d48623c113 100644 --- a/docs/concord-banlist-rank-conformance.md +++ b/docs/concord-banlist-rank-conformance.md @@ -213,6 +213,29 @@ to re-issue it. If you read §4 as scoping only to editions authored *after* the implementable too, but it needs the spec to define an ordering between an edition and a Banlist entry, which today it does not. +We checked your implementation before writing this, and you got there first: `foldControlState` +already runs the same two-pass, with the same §4 justification in the comment. So this is us catching +up, not diverging — with one narrower difference. You keep **pass 1's** Banlist as the final word; +we recompute it in pass 2. So a banned admin's mass-ban of everyone beneath them still stands for you +and is dropped by us. Your stated reason is to stop the anti-roster erasing itself; ours is that an +edition should not outlive its author's removal, and the self-erasure case is unreachable under the +rank rule anyway, since only a member who strictly outranks you can ban you. We would rather converge +than be right — tell us which way and we will move. + +Two more things that fell out of reading `src/concord-v2/` side by side, both worth their own look: + +- **`bootstrapHead` has no bound** (`lib/version.ts`), and `headCandidates` uses it for the + compaction arm while `pickHead` then raises the stored floor to whatever won. One authorized + edition at `version = 2^63 - 1` therefore becomes an entity's permanent head: the floor rises to + match, nothing honest can exceed it, and even a Refounding that drops the edition falls back to the + remembered head — which is that edition. It needs no ban and no sockpuppet, just one ordinary + permission bit. This is the second bug both implementations share by reading the same section the + same way; ours is described in `docs/concord-soft-ban-audit.md` (B1), and we bounded the jump the + arm will follow. +- **Your Banlist takes only the gated head's content**, with no §4 re-heal union. We union in every + authorized non-ancestor edition, which is what defeats an attempt to launder a ban away by forking + the list at genesis. So we honor concurrent bans you drop. Which is normative? + A chain-local rule is not enough, and this is the trap worth flagging: "the author must not be banned by the state their edition chains from" is bypassed by forking the Banlist at genesis, where no parent ever mentions the ban and §4's re-heal union carries it in anyway. The rule has to bind the diff --git a/docs/concord-soft-ban-audit.md b/docs/concord-soft-ban-audit.md index 1da43ed4d0..6040d8ee7f 100644 --- a/docs/concord-soft-ban-audit.md +++ b/docs/concord-soft-ban-audit.md @@ -62,7 +62,7 @@ Two structural causes account for most of both halves: | # | Finding | Severity | Needs a ban? | Status | |---|---------|----------|--------------|--------| | [B1](#b1) | One edition at `version = Long.MAX_VALUE` pins an entity forever | **Critical** | No — any bit-holder | **Fixed** | -| [B2](#b2) | A banned staffer keeps Role/Grant/Banlist authority | **Critical** | Yes | **Fixed** (consensus-affecting) | +| [B2](#b2) | A banned staffer keeps Role/Grant/Banlist authority | **Critical** | Yes | **Fixed** (matches Armada) | | [B3](#b3) | A rogue rotator compacts the banlist away | High | Via B2 | **Mitigated** by B2 | | [B4](#b4) | The Refounding recipient set is attacker-inflatable | High | No | **Fixed** (bounded) | | [B5](#b5) | The ban is per-pubkey; the channel key is not revoked | High | Yes | Inherent — Refounding is the answer | @@ -111,10 +111,13 @@ button on the same. This is contained, uncontroversial, and closes the realistic ## A2 — Stranded recovery runs on a timer and never checks the banlist -**Status: security half fixed; liveness half open.** `isStranded` / `mergeForward` now take -`bannedAtCurrentEpoch` as a *required* argument, so a removed member is no longer walked back in. -Whether anything should re-mint at a stable coordinate — without which legitimate recovery never -fires for anyone — still needs a spec answer and is untouched. +**Status: security half fixed; liveness half open — and the fork is now resolved.** `isStranded` / +`mergeForward` take `bannedAtCurrentEpoch` as a *required* argument, so a removed member is no longer +walked back in. The open question was whether anything re-mints at a stable coordinate. **Armada +does** — `useLinkRefreshWatch2` re-posts every bundle on each epoch change — so in any cross-client +community this was a *live* removal bypass, not a hypothetical, and the fix was load-bearing. The +liveness half stands: Amethyst re-mints nothing, so legitimate recovery never fires for an +Amethyst-only community. See [the Armada comparison](#armada) for the two ways out. **Critical, and it forks.** *Read:* `ConcordStrandedRecovery`, @@ -275,13 +278,13 @@ The first is the smallest change and closes the unrecoverability; the third shou ## B2 — A banned staffer keeps Role, Grant and Banlist authority -**Status: fixed — and consensus-affecting.** `AuthorityResolver.resolve` is now a bounded two-pass +**Status: fixed. Not consensus-affecting after all** — see [Armada comparison](#armada). Armada +already implements the same two-pass, so this brings us *into* line rather than out of it. One +narrower divergence remains, described there. `AuthorityResolver.resolve` is now a bounded two-pass where authority only shrinks. Note the deliberate cascade it brings: every edition a banned member ever authored is dropped, so banning an admin also demotes everyone that admin promoted. That is the literal reading of CORD-04 §4 and it is what kills the sockpuppet, but a legitimate promotion by a -later-banned admin vanishes with it and has to be re-issued. Until Armada ships the same rule the two -clients can disagree about any community where a privileged member was banned. - +later-banned admin vanishes with it and has to be re-issued. **Critical.** *Verified:* `quartz/…/cord04Roles/BannedStaffEscalationTest.kt` (13 tests). @@ -440,6 +443,65 @@ It would be the one place where a ban fails *audibly*, in real time, in front of worth designing the roster check in before shipping rather than after. + +--- + +## Armada comparison (checked 2026-08-09) + +Read against `gitlab.com/soapbox-pub/armada` at `src/concord-v2/`. Worth doing before shipping any of +this, and it changed two conclusions. + +**B2 — they already do it, and we had it backwards.** `foldControlState` (`lib/control.ts`) runs the +same bounded two-pass: fold once, take the banlist, and if any edition was authored by someone on it, +re-fold with those editions excluded. Independently arrived at, same shape, same CORD-04 §4 +justification in the comment. So this change brings us *into* line with Armada rather than out of it, +and the consensus warning in the earlier revision of this doc was wrong. + +One real divergence remains, and it is ours to defend: Armada keeps **pass 1's** banlist as the final +word ("the first pass's Banlist stays the final word"), while we recompute the banlist in pass 2. So +a banned admin's mass-ban of everyone beneath them still stands in Armada and is dropped by us — the +`aBannedAdminBansEveryoneBeneathThemWithoutNeedingAPuppetAtAll` case. Their stated reason is to stop +the anti-roster erasing itself; ours is that an edition from a banned author should not survive its +own author's removal. Both are defensible; ours closes an attack theirs leaves open, and the +self-erasure they worry about is unreachable for us because the rank rule makes mutual bans +impossible (only someone who strictly outranks you can ban you). Worth raising with them. + +**B1 — the same bug, unfixed, in exactly the same place.** `bootstrapHead` (`lib/version.ts:155`) +takes the highest version at or above the floor with no bound; `headCandidates` uses it for the +compaction arm; `pickHead` then raises the stored floor to whatever won. That is the whole +version-exhaustion chain. This is now the second bug both implementations share because both read +the same section the same way, and it deserves the same treatment as the rank rule: a written report. + +**A1 — ours alone.** Armada gates invite creation on `CREATE_INVITE` in both the hook +(`useInvites2.ts`) and the page (`canCreateInvite`). We were the only client handing a banned member +a working invite button. + +**A2 — different architecture, and it is better.** Armada's catch-up is **push**, not pull: a +privileged member sends a stranded member a direct invite carrying the fresher root +(`useDirectInvites2`, `catchUp`), so a human authorizes each re-admission. `useRekeyWatch2` merely +reports `{ stranded: boolean }` for the UI. They also ship `useBanSelfRemove2`: a banned member's own +client silently drops the community from their private list — network-silent, deliberately narrower +than rekey-exclusion, because "a rotation can be a mistake; a ban is a judgment". Our pull-from-my-own- +old-link design is what made the bypass possible, and their per-epoch bundle refresh is what would +have supplied the higher epoch to pull. Two ways forward: adopt a refresh of our own (restores +liveness, keeps the pull design and its risk), or move to their push model (safer, and it is what the +one existing implementation does). + +**C1 — still open on their side.** `banlistGate` remains a bare `isAuthorized(roster, author, owner, +BAN)`: no rank check, no delta rule. The divergence from +`docs/concord-banlist-rank-conformance.md` is unchanged. + +**A divergence in the other direction.** Armada's banlist takes only the gated head's content — +there is no §4 re-heal union. Ours unions in every authorized non-ancestor edition, which is what +defeats the genesis-fork laundering attempt in `BannedStaffEscalationTest`. So we honor concurrent +bans they drop. Worth a spec question about which is normative. + +**A4 — shared gap.** No ban filter on typing there either. + +**B4 — not established.** I could not locate a recipient-set bound in their rekey path, but I also +could not locate the recipient-set construction itself with confidence, so treat this as unchecked +rather than as a finding either way. + --- ## What was NOT examined From 54d3bfc84a6f78d5adb3f108c8f3c23ba3813ee5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 16:09:53 +0000 Subject: [PATCH 11/13] perf(quartz): skip the resolver's second pass when it cannot change anything MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two-pass ban-aware fold doubles resolve(), which runs once per held epoch in controlFloorsLocked plus once in fold, on a client that re-folds the whole buffer from scratch on every Control Plane change. So the second pass is now skipped unless a banned member actually authored a Control edition — not merely when the banlist is empty. Bans overwhelmingly land on plain members who hold no role and write nothing, and for those pass B is provably identical to pass A. Armada's fold checks the same condition. Measured over ConcordCommunityState.fold (throwaway benchmark, not committed; 226 and 2059 editions, 200 reps after warmup). Pass A is byte-for-byte the old algorithm, so the single-pass rows are the before-numbers: 226 eds, no bans 1457 us 226 eds, 20 bans, none authored 994 us 226 eds, 20 bans, one authored -> pass B 1881 us 2059 eds, no bans 2194 us 2059 eds, 50 bans, none authored 2001 us 2059 eds, 50 bans, one authored -> pass B 5697 us 2059 eds, with floors (B1's arm) 2015 us So the common case is free, and B1's chain-first compaction arm is not measurable — the floored fold matches the unfloored one. A banned staffer costs ~2-3x, which is the price of the fix and is paid only under the attack. The audit records this, plus the standing opportunity it surfaced: we have no fold memoization where Armada does, which predates this work and would absorb the pass-B cost too. Not done here — that is a change to make on its own merits. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- docs/concord-soft-ban-audit.md | 40 +++++++++++++++++++ .../concord/cord04Roles/AuthorityResolver.kt | 5 +++ 2 files changed, 45 insertions(+) diff --git a/docs/concord-soft-ban-audit.md b/docs/concord-soft-ban-audit.md index 6040d8ee7f..8e766c8b46 100644 --- a/docs/concord-soft-ban-audit.md +++ b/docs/concord-soft-ban-audit.md @@ -502,6 +502,46 @@ bans they drop. Worth a spec question about which is normative. could not locate the recipient-set construction itself with confidence, so treat this as unchecked rather than as a finding either way. + +--- + +## Performance of the fixes + +Measured on the JVM with a synthetic Control Plane (throwaway benchmark, not committed — +`ConcordCommunityState.fold` over 226 and 2059 editions, 200 reps after warmup). Pass A of the +two-pass resolver is byte-for-byte the old algorithm, so the single-pass rows below *are* the +before-numbers. + +| Case | µs / fold | +|---|---| +| 226 editions, no bans | 1457 | +| 226 editions, 20 bans, none of them authors | 994 | +| 226 editions, 20 bans, one an author → pass B runs | 1881 | +| 2059 editions, no bans | 2194 | +| 2059 editions, 50 bans, none of them authors | 2001 | +| 2059 editions, 50 bans, one an author → pass B runs | 5697 | +| 2059 editions, with floors (B1's compaction arm) | 2015 | + +Two things to take from it. + +**B1 costs nothing measurable.** Trying the floor-anchored chain before the raw-version bootstrap +adds a per-entity version index on the compaction arm, but an entity carries a handful of editions, +and the floored fold measures the same as the unfloored one. + +**B2 costs a second fold, but only when it can change the answer.** `resolve` skips pass B when +nobody is banned *or* when nobody banned ever authored a Control edition — the overwhelmingly common +shape, since bans land on plain members who hold no role and write nothing. Those rows show no +regression. When a banned member *did* author editions — a banned staffer, exactly the case B2 exists +for — the fold costs ~2–3× more. That is the price of the fix and it is paid only by communities +under the attack. + +**Worth knowing, unrelated to this work:** Amethyst re-folds the whole buffer from scratch on every +Control Plane change, and `resolve` runs once per held epoch inside `controlFloorsLocked` plus once +in `fold`, so a refresh is already several folds. Armada memoizes the fold by +`(community, owner, floors, snapshot, edition ids)`; we do not. That is the real optimization here, +it predates these fixes, and it would also absorb the pass-B cost. Left alone deliberately — it is a +change to make on its own merits, with its own measurements. + --- ## What was NOT examined diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt index ff85147ea6..ffc637bd2c 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt @@ -170,7 +170,12 @@ data class AuthorityResolver private constructor( ownerPubKey: String, ): AuthorityResolver { val passA = resolveOnce(editions, ownerPubKey, bannedAuthors = emptySet()) + // Pass B costs a whole second fold, so skip it unless it could change something. Nobody + // banned, or nobody banned who ever wrote to the Control Plane — the overwhelmingly common + // shape, since most bans land on plain members who hold no role and author no editions — + // and pass B is provably identical to pass A. This is also what Armada's fold checks. if (passA.banned.isEmpty()) return passA + if (editions.none { it.author.lowercase() in passA.banned }) return passA return resolveOnce(editions, ownerPubKey, bannedAuthors = passA.banned) } From 9cc19c60ca6f9d83b548b741488a95b7003e6904 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 16:38:35 +0000 Subject: [PATCH 12/13] fix(concord): three defects found auditing this branch's own changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review of the diff before merge. One of these is a real correctness bug in the B2 fix as shipped. The resolver stopped after two passes, which left the mask a pass resolved UNDER disagreeing with the banlist that pass produced — and the disagreement is not cosmetic. A moderator whose only ban came from an admin the owner banned concurrently is released by pass 2, correctly; but pass 2 had already dropped her editions, because she was on pass 1's list. The fold then reported her as a moderator in good standing whose promotions had silently vanished, and did so deterministically, so she never got them back. resolve() now iterates until the mask and the resulting banlist agree. The mask cannot simply be assumed to shrink, which is why this is bounded rather than proven monotone: masking an author can strip a THIRD member's role, which drops their rank to roleless, which lets a junior BAN holder who previously could not reach them ban them after all. The loop keeps its last pass if it does not settle within the cap — still better than the two-pass answer, and it always terminates. Real communities settle on the first or second pass, and the skip-if-no-banned-author guard means most never enter the loop at all. boundRecipients could exceed its own budget while reporting that it had capped at it, because the roster was added with filterTo before the budget loop ran. The roster now goes in whole deliberately — it is owner-rooted and cannot be padded from outside, and dropping an admin to make room for a stranger inverts the point — and the log reports what was actually kept and dropped. mintConcordInvite started requiring a session, which the owner's own invite button would not have on a cold start, since sessions are built asynchronously off the joined list. The owner is proven by the community id, so they are read off the entry; everyone else still needs the folded roster. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- .../amethyst/model/AccountConcordActions.kt | 24 +++++++++---- docs/concord-soft-ban-audit.md | 7 ++-- .../concord/cord04Roles/AuthorityResolver.kt | 35 +++++++++++++++++-- .../cord04Roles/BannedStaffEscalationTest.kt | 32 +++++++++++++++++ 4 files changed, 86 insertions(+), 12 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt index 5b5c8fe0db..fc812f1bb8 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt @@ -189,8 +189,13 @@ class AccountConcordActions( // Note the bit is not otherwise enforced anywhere. The fold gates the INVITE_* Control // entities on CREATE_INVITE, but a link's bundle is a standalone kind-33301 published // OUTSIDE the Control Plane, so no fold ever sees it. This check is the only one there is. - val session = account.concordSessions.sessionFor(communityId) ?: return null - if (!isAuthorizedFor(session, ConcordPermissions.CREATE_INVITE)) return null + // The owner is proven by the community id (CORD-02), so they are read off the entry and can + // mint before the session exists — the session is built asynchronously off the joined list, + // and requiring it here would have made the owner's own invite button fail on a cold start. + // Everyone else needs the folded roster, so no session means no invite. + val session = account.concordSessions.sessionFor(communityId) + val amOwner = entry.owner.equals(account.signer.pubKey, ignoreCase = true) + if (!amOwner && (session == null || !isAuthorizedFor(session, ConcordPermissions.CREATE_INVITE))) return null val invite = ConcordActions.inviteFor( communityIdHex = entry.id, @@ -885,16 +890,23 @@ class AccountConcordActions( ): List { if (candidates.size <= MAX_REFOUNDING_RECIPIENTS) return candidates.toList() + // The roster goes in whole even if it alone exceeds the budget: it is owner-rooted, so it + // cannot be padded from outside, and dropping an admin to make room for a stranger inverts + // the point of the cap. val vouched = authority.roleHolders() + authority.staffMembers() - val kept = LinkedHashSet(MAX_REFOUNDING_RECIPIENTS) + val kept = LinkedHashSet() candidates.filterTo(kept) { it in vouched } for (candidate in candidates) { if (kept.size >= MAX_REFOUNDING_RECIPIENTS) break kept.add(candidate) } - Log.w("Concord") { - "Refounding recipient set capped at $MAX_REFOUNDING_RECIPIENTS of ${candidates.size}: " + - "${candidates.size - kept.size} member(s) will be stranded on the prior epoch" + val dropped = candidates.size - kept.size + if (dropped > 0) { + Log.w("Concord") { + "Refounding recipient set trimmed to ${kept.size} of ${candidates.size} " + + "(budget $MAX_REFOUNDING_RECIPIENTS, roster kept whole): $dropped member(s) will be " + + "stranded on the prior epoch" + } } return kept.toList() } diff --git a/docs/concord-soft-ban-audit.md b/docs/concord-soft-ban-audit.md index 8e766c8b46..43848a1302 100644 --- a/docs/concord-soft-ban-audit.md +++ b/docs/concord-soft-ban-audit.md @@ -528,12 +528,13 @@ Two things to take from it. adds a per-entity version index on the compaction arm, but an entity carries a handful of editions, and the floored fold measures the same as the unfloored one. -**B2 costs a second fold, but only when it can change the answer.** `resolve` skips pass B when +**B2 costs a further fold, but only when it can change the answer.** `resolve` skips pass B when nobody is banned *or* when nobody banned ever authored a Control edition — the overwhelmingly common shape, since bans land on plain members who hold no role and write nothing. Those rows show no regression. When a banned member *did* author editions — a banned staffer, exactly the case B2 exists -for — the fold costs ~2–3× more. That is the price of the fix and it is paid only by communities -under the attack. +for — the fold costs ~2–3× more, and one more pass again in the rare case where a banned member had +themselves authored a ban. That is the price of the fix and it is paid only by communities under the +attack. **Worth knowing, unrelated to this work:** Amethyst re-folds the whole buffer from scratch on every Control Plane change, and `resolve` runs once per held epoch inside `controlFloorsLocked` plus once diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt index ffc637bd2c..126eaade9b 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt @@ -135,6 +135,14 @@ data class AuthorityResolver private constructor( /** The owner's rank — supreme and unremovable. No Role may claim it. */ const val OWNER_RANK = 0L + /** + * How many times [resolve] will re-fold chasing a stable banlist. Real communities settle on + * the first or second — the mask only moves when a banned member authored a *ban*, and it + * stops moving as soon as those are gone. The cap is a termination backstop for an + * adversarial edition set, not a tuning knob. + */ + private const val MAX_BAN_RESOLUTION_PASSES = 4 + /** * The owner-rooted authority state of a community, with the banlist honored **against the * Control Plane itself** (CORD-04 §4: a reader "drops every event from a banned npub — @@ -170,13 +178,34 @@ data class AuthorityResolver private constructor( ownerPubKey: String, ): AuthorityResolver { val passA = resolveOnce(editions, ownerPubKey, bannedAuthors = emptySet()) - // Pass B costs a whole second fold, so skip it unless it could change something. Nobody + // A further pass costs a whole fold, so skip it unless it could change something. Nobody // banned, or nobody banned who ever wrote to the Control Plane — the overwhelmingly common // shape, since most bans land on plain members who hold no role and author no editions — - // and pass B is provably identical to pass A. This is also what Armada's fold checks. + // and the next pass is provably identical to this one. Armada's fold checks the same. if (passA.banned.isEmpty()) return passA if (editions.none { it.author.lowercase() in passA.banned }) return passA - return resolveOnce(editions, ownerPubKey, bannedAuthors = passA.banned) + + // Iterate to a fixpoint where the mask a pass was resolved UNDER equals the banlist that + // pass produced. Stopping at two passes leaves those two disagreeing, and the disagreement + // is not cosmetic: a moderator whose only ban came from an admin the owner banned + // concurrently is released by pass 2 — correctly — but pass 2 dropped her editions too, + // because she was on pass 1's list. The fold then reports her as a moderator in good + // standing whose promotions have silently vanished, and it does so deterministically, so + // she never gets them back. + // + // The mask cannot simply be assumed to shrink: masking an author can strip a THIRD + // member's role, dropping their rank to "roleless", which lets a junior BAN holder who + // could not previously reach them ban them after all. So this is bounded rather than + // proven monotone, and it keeps the last pass it computed if it somehow does not settle — + // still strictly better than the two-pass answer, and it always terminates. + var mask = passA.banned + var result = passA + repeat(MAX_BAN_RESOLUTION_PASSES) { + result = resolveOnce(editions, ownerPubKey, bannedAuthors = mask) + if (result.banned == mask) return result + mask = result.banned + } + return result } /** diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt index 5b6c0fc64d..3bed1c8575 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/BannedStaffEscalationTest.kt @@ -362,4 +362,36 @@ class BannedStaffEscalationTest { assertEquals(5, r.rank(bob), "bob is untouched") assertEquals(5, r.rank(carol), "and the owner's grant of carol stands") } + + @Test + fun aMemberReleasedByTheSecondPassKeepsTheEditionsTheyAuthored() { + // The mask a pass resolves UNDER has to equal the banlist that pass produces, or the fold + // reports a state that contradicts itself. Concretely: the rogue admin bans a moderator while + // the owner concurrently bans the rogue. The moderator is correctly released — the only ban on + // her came from someone who turned out to be banned — but a fold that stops after two passes + // has already dropped her editions, because she was on the FIRST pass's list. She then reads + // as a moderator in good standing whose promotions silently vanished, deterministically and + // forever. resolve() iterates until the two agree. + val juniorRole = "23".repeat(32) + val seniorRole = "24".repeat(32) + val editions = + community() + + // the baseline Mod role carries no MANAGE_ROLES, so give bob one that can grant + role(seniorRole, """{"name":"Senior","position":5,"permissions":"95"}""") + + grant(bobGrantEntity, bob, listOf(seniorRole), author = owner, version = 1, prev = bobGrantV0.hash) + + role(juniorRole, """{"name":"Junior","position":9,"permissions":"8"}""") + + // bob promotes carol himself, while in good standing + grant("39".repeat(32), carol, listOf(juniorRole), author = bob) + + // the rogue admin bans bob... + banlist(alice, 0, null, bob) + + // ...while the owner concurrently bans the rogue, never naming bob + ownerBansAlice + + val r = AuthorityResolver.resolve(editions, owner) + + assertTrue(r.isBanned(alice), "the owner's ban of the rogue stands") + assertFalse(r.isBanned(bob), "and the rogue's ban of the moderator falls with them") + assertEquals(5, r.rank(bob), "the released moderator keeps their own role") + assertEquals(9, r.rank(carol), "and the promotion they authored survives with them") + } } From f1241e891bb46ae9bbcdbfb48a3a1a23f653f894 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 17:11:45 +0000 Subject: [PATCH 13/13] fix(quartz): compaction must carry the authority-gated head, not the chain head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Findings from an independent review of this branch (a different model, per CONTRIBUTING-WITH-AI.md). Two were real, and the first is a regression this branch introduced. compactControlPlane picked its per-entity head with a bare structural chain walk, which is worse than the raw max-version it replaced. With no floor, foldEntity anchors at the lowest-version edition carrying no `prev` — and after a PRIOR compaction the genuine head's `prev` dangles into a trimmed epoch by design. So a forged `version = 1, prev = null` decoy outranks a real v50→v52 chain, and because nothing in this path checks a signature it became the entity's entire carried-forward state. A forged empty banlist would have erased every ban at the next Refounding. Reproduced, then fixed by selecting the owner-rooted authority-gated head — the same edition ConcordCommunityState.fold would seat, so the new epoch starts where the old one left off, and an unprivileged author cannot influence the choice at all. recoverStrandedConcordCommunities derived its new ban gate with `?.isBanned(..) == true`, which reads "not banned" when the session does not exist yet or its first fold has not landed. The sweep runs on the revision tick, so a banned member's own client would have hit that window on cold start and recovered itself — the exact bypass the gate exists to stop. It now fails closed and retries on the next sweep. Also from the review: resolve() now warns when the ban fixpoint exhausts its pass cap without settling, instead of silently returning a roster folded under a mask that no longer matches its banlist; and banGate stops lowercasing the same author three times. Two review findings are accepted rather than fixed, and recorded on the PR: the anchor tie-break picks the lowest rumor id before testing whether that candidate connects (pre-existing, and changing it is consensus-affecting), and non-owner moderators now need a resolved roster before a verb succeeds, which is the intended fail-closed trade. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj --- .../amethyst/model/AccountConcordActions.kt | 16 +++- .../commons/actions/ConcordActions.kt | 2 + .../commons/actions/ConcordActionsTest.kt | 1 + .../model/concord/ConcordRollbackFloorTest.kt | 6 +- .../concord/cord04Roles/AuthorityResolver.kt | 18 ++++- .../concord/cord06Rekey/ConcordRefounding.kt | 38 ++++++---- .../cord06Rekey/ConcordRefoundingTest.kt | 76 +++++++++++++++++++ .../cord06Rekey/ControlRootRotationTest.kt | 2 + 8 files changed, 138 insertions(+), 21 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt index fc812f1bb8..f659e38564 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt @@ -852,6 +852,7 @@ class AccountConcordActions( recipientsXOnly = recipients, staffXOnly = staff, createdAt = TimeUtils.now(), + ownerPubKey = entry.owner, ) // 4. Publish the compacted Control Plane (the new epoch's state) then the rekey blobs @@ -1123,13 +1124,24 @@ class AccountConcordActions( // walks them straight back into the epoch they were rotated out of — see A2 in // docs/concord-soft-ban-audit.md. Read off the epoch we are LEAVING, which is the last // one whose Control Plane we can still fold. - val bannedHere = + // + // Fails CLOSED. `?.isBanned(..) == true` reads "not banned" for a session that does not + // exist yet or whose first fold has not landed, and this sweep runs on the revision tick + // — so a banned member's own client would have hit that window on cold start and + // recovered itself, which is precisely the bypass this gate exists to stop. No verdict + // means no recovery; the next sweep retries once the roster is known. + val authority = account.concordSessions .sessionFor(entry.id) ?.state ?.value ?.authority - ?.isBanned(account.signer.pubKey) == true + if (authority == null) { + Log.i("Concord") { "Stranded-recovery check deferred for ${entry.id}: control plane not folded yet" } + lastConcordRecoveryCheck.remove(entry.id) + continue + } + val bannedHere = authority.isBanned(account.signer.pubKey) val merged = ConcordActions.recoverStranded(entry, bundle, bannedHere) ?: continue if (!adoptedConcordRotations.add("${entry.id}:${merged.rootEpoch}")) continue Log.i("Concord", "Stranded recovery: ${entry.id} ${entry.rootEpoch} -> ${merged.rootEpoch}") diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActions.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActions.kt index dff1143c45..e70fe18364 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActions.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActions.kt @@ -545,6 +545,7 @@ object ConcordActions { recipientsXOnly: List, staffXOnly: Set, createdAt: Long, + ownerPubKey: HexKey, ): RefoundingBuild = ConcordRefounding.build( rotatorSigner = rotatorSigner, @@ -558,6 +559,7 @@ object ConcordActions { recipientsXOnly = recipientsXOnly, staffXOnly = staffXOnly, createdAt = createdAt, + ownerPubKey = ownerPubKey, ) /** diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActionsTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActionsTest.kt index a5d8ff772d..82169a7708 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActionsTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActionsTest.kt @@ -123,6 +123,7 @@ class ConcordActionsTest { // Only the owner is staff, so only the owner's blob carries the secret. staffXOnly = setOf(owner.pubKey), createdAt = 5L, + ownerPubKey = owner.pubKey, ) val baseRekey = ConcordActions.nextBaseRekeyPlane(community.communityRoot, community.communityId, community.rootEpoch) diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/concord/ConcordRollbackFloorTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/concord/ConcordRollbackFloorTest.kt index d3c1a50a75..ae06a84891 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/concord/ConcordRollbackFloorTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/concord/ConcordRollbackFloorTest.kt @@ -78,7 +78,7 @@ class ConcordRollbackFloorTest { // new epoch's plane is split and addressed by the derived signer, not the root. val newControlRoot = ByteArray(32) { 0x44 } val newControl = ControlPlaneKeys.forStaff(newRoot, community.communityId, newEpoch, newControlRoot) - val rolledBack = ConcordRefounding.compactControlPlane(community.genesisWraps, community.controlPlane, newControl) + val rolledBack = ConcordRefounding.compactControlPlane(community.genesisWraps, community.controlPlane, newControl, community.ownerPubKey) val entry = ConcordCommunityListEntry( @@ -145,7 +145,7 @@ class ConcordRollbackFloorTest { val newControlRoot = ByteArray(32) { 0x44 } val newControl = ControlPlaneKeys.forStaff(newRoot, community.communityId, newEpoch, newControlRoot) // Honest: compacted from the FULL prior plane, so each entity's head (metadata v1) survives. - val honest = ConcordRefounding.compactControlPlane(epoch0Wraps, community.controlPlane, newControl) + val honest = ConcordRefounding.compactControlPlane(epoch0Wraps, community.controlPlane, newControl, community.ownerPubKey) val entry = ConcordCommunityListEntry( @@ -186,7 +186,7 @@ class ConcordRollbackFloorTest { // new epoch's plane is split and addressed by the derived signer, not the root. val newControlRoot = ByteArray(32) { 0x44 } val newControl = ControlPlaneKeys.forStaff(newRoot, community.communityId, newEpoch, newControlRoot) - val compacted = ConcordRefounding.compactControlPlane(community.genesisWraps, community.controlPlane, newControl) + val compacted = ConcordRefounding.compactControlPlane(community.genesisWraps, community.controlPlane, newControl, community.ownerPubKey) val entry = ConcordCommunityListEntry( diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt index 126eaade9b..5677e56244 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt @@ -21,6 +21,7 @@ package com.vitorpamplona.quartz.concord.cord04Roles import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import com.vitorpamplona.quartz.utils.Log /** * Resolves the owner-rooted authority state of a Concord community from its @@ -132,6 +133,8 @@ data class AuthorityResolver private constructor( } companion object { + private const val TAG = "ConcordAuthorityResolver" + /** The owner's rank — supreme and unremovable. No Role may claim it. */ const val OWNER_RANK = 0L @@ -205,6 +208,14 @@ data class AuthorityResolver private constructor( if (result.banned == mask) return result mask = result.banned } + // Exhausted the cap without settling. The returned roster was folded under a mask that is + // no longer the banlist beside it, so this is reported rather than swallowed — the same + // reasoning as EditionFold.LOG_GAP: an unsettled fold is either an adversarial edition set + // or a rule of ours that does not converge, and both are things a reader wants to know. + Log.w(TAG) { + "Banlist resolution did not settle in $MAX_BAN_RESOLUTION_PASSES passes for owner $ownerPubKey " + + "(${editions.size} editions, ${result.banned.size} banned): keeping the last pass" + } return result } @@ -350,9 +361,10 @@ data class AuthorityResolver private constructor( // a concurrent ban is never lost, while an on-chain unban still takes effect. val allBanlist = editions.filter { it.entityKind == ControlEntityKind.BANLIST } - fun banGate(e: ControlEdition): Boolean = - e.author.lowercase() == ownerLower || - (e.author.lowercase() !in bannedAuthors && effectivePermissionsOf(e.author.lowercase()).has(ConcordPermissions.BAN)) + fun banGate(e: ControlEdition): Boolean { + val author = e.author.lowercase() + return author == ownerLower || (author !in bannedAuthors && effectivePermissionsOf(author).has(ConcordPermissions.BAN)) + } val authorizedBanlist = allBanlist.filter(::banGate) // CORD-04 §3's rank rule binds "every action", and it names banning as its example ("an diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefounding.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefounding.kt index 7dc80de9cb..74fb73b594 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefounding.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefounding.kt @@ -20,8 +20,8 @@ */ package com.vitorpamplona.quartz.concord.cord06Rekey +import com.vitorpamplona.quartz.concord.cord02Community.ConcordCommunityState import com.vitorpamplona.quartz.concord.cord04Roles.ControlEdition -import com.vitorpamplona.quartz.concord.cord04Roles.EditionFold import com.vitorpamplona.quartz.concord.crypto.ConcordKeyDerivation import com.vitorpamplona.quartz.concord.crypto.ControlPlaneKeys import com.vitorpamplona.quartz.concord.crypto.GroupKey @@ -124,11 +124,12 @@ object ConcordRefounding { recipientsXOnly: List, staffXOnly: Set, createdAt: Long, + ownerPubKey: HexKey, ): RefoundingBuild { val newEpoch = rootEpoch + 1 val newControlKeys = ControlPlaneKeys.forStaff(newRoot, communityId, newEpoch, newControlRoot) - val controlWraps = compactControlPlane(priorControlWraps, priorControlKeys, newControlKeys) + val controlWraps = compactControlPlane(priorControlWraps, priorControlKeys, newControlKeys, ownerPubKey) val baseRekeyKey = ConcordKeyDerivation.baseRekeyAddress(priorRoot, communityId, newEpoch) val prevCommit = ConcordKeyDerivation.epochKeyCommitment(rootEpoch, priorRoot).toHexKey() @@ -167,6 +168,7 @@ object ConcordRefounding { priorWraps: List, priorControlKeys: ControlPlaneKeys, newControlKeys: ControlPlaneKeys, + ownerPubKey: HexKey, ): List { // entity coordinate -> every edition we can open, paired with its verified seal. val byCoordinate = HashMap>>() @@ -177,17 +179,27 @@ object ConcordRefounding { byCoordinate.getOrPut(coord) { ArrayList() }.add(edition to opened.seal) } - // The head is the CHAIN head, not the highest version. Picking by raw version made an honest - // rotator the delivery mechanism for a disconnected stray: an edition minted at an arbitrary - // version never joins the chain, but it won this comparison and was then re-wrapped into the - // new epoch as that entity's whole history — where a fresh joiner, holding no floor, anchors - // on it as their baseline. See B1 in `docs/concord-soft-ban-audit.md`. foldEntity walks from - // genesis and keeps the fresh-joiner fallback for a head whose own `prev` dangles into an - // epoch this rotator no longer holds, which is the ordinary shape after a prior compaction. - val out = ArrayList(byCoordinate.size) - for ((_, entries) in byCoordinate) { - val head = EditionFold.foldEntity(entries.map { it.first }) ?: continue - val seal = entries.firstOrNull { it.first.rumorId == head.rumorId }?.second ?: continue + // The head to carry forward is the one every READER honors — the authority-gated head — not + // the highest version and not the bare structural chain head. + // + // Raw highest version made an honest rotator the delivery mechanism for a disconnected stray: + // an edition minted at an arbitrary version never joins the chain, but it won that comparison + // and was re-wrapped into the new epoch as the entity's whole history (B1 in + // `docs/concord-soft-ban-audit.md`). The bare chain walk is *worse*, and this is the trap: + // with no floor it anchors at the lowest-version edition carrying no `prev`, and after a prior + // compaction the real head's `prev` dangles by design — so a forged `version = 1, prev = null` + // decoy outranks a genuine v50→v52 chain and, because nothing here checks signatures, becomes + // the entity's entire carried-forward state. A forged empty banlist would erase every ban. + // + // Gating on the owner-rooted roster is the only selection that cannot be gamed by an + // unprivileged author, and it is exactly what ConcordCommunityState.fold would seat, so the + // compacted epoch starts where the previous one left off. + val editions = byCoordinate.values.flatten() + val honored = ConcordCommunityState.authorizedHeads(editions.map { it.first }, ownerPubKey) + val out = ArrayList(honored.size) + for ((_, floor) in honored) { + val head = floor.known ?: continue + val seal = editions.firstOrNull { it.first.rumorId == head.rumorId }?.second ?: continue out.add(ConcordStreamEnvelope.wrapSeal(seal, newControlKeys, createdAt = seal.createdAt)) } return out diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefoundingTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefoundingTest.kt index 06778c4c71..dbc6e9c81d 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefoundingTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefoundingTest.kt @@ -74,6 +74,7 @@ class ConcordRefoundingTest { recipientsXOnly = listOf(alice.pubKey, bob.pubKey), staffXOnly = setOf(owner.pubKey), createdAt = now, + ownerPubKey = owner.pubKey, ) assertEquals(community.rootEpoch + 1, build.newEpoch) @@ -113,6 +114,7 @@ class ConcordRefoundingTest { recipientsXOnly = listOf(alice.pubKey), staffXOnly = setOf(owner.pubKey), createdAt = now, + ownerPubKey = owner.pubKey, ) val newControl = build.newControlKeys @@ -184,6 +186,7 @@ class ConcordRefoundingTest { recipientsXOnly = listOf(alice.pubKey), staffXOnly = setOf(owner.pubKey), createdAt = now, + ownerPubKey = owner.pubKey, ) val newControl = build.newControlKeys @@ -223,6 +226,7 @@ class ConcordRefoundingTest { recipientsXOnly = listOf(alice.pubKey), staffXOnly = setOf(owner.pubKey), createdAt = now, + ownerPubKey = owner.pubKey, ) val baseRekeyKey = ConcordKeyDerivation.baseRekeyAddress(community.communityRoot, community.communityId, build.newEpoch) @@ -230,4 +234,76 @@ class ConcordRefoundingTest { val wrongRoot = ByteArray(32) { 0x11 } assertNull(ConcordRefounding.findNewRoot(build.rekeyWraps, baseRekeyKey, alice, community.communityId, wrongRoot, community.rootEpoch)) } + + @Test + fun compactionRefusesAForgedGenesisAndCarriesTheAuthorizedHead() = + runTest { + // A compaction re-wraps ONE edition per entity and nothing downstream re-checks the + // choice, so how that edition is picked is a security decision, not a detail. + // + // Raw highest-version lets a stray at an arbitrary version through. But the bare + // structural chain walk is worse: with no floor it anchors at the lowest-version edition + // carrying no `prev`, and after a PRIOR compaction the real head's `prev` dangles into a + // trimmed epoch by design — so a forged `version = 1, prev = null` decoy outranks a + // genuine v50→v52 chain, and becomes the entity's entire carried-forward state. A forged + // empty banlist would erase every ban that way. Only the owner-rooted gate is safe. + val community = ConcordCommunityFactory.create(owner, "Test", now) + val communityId = community.communityId + val control = community.controlPlane + + // The metadata entity, already compacted once: its head chains from an epoch we no longer hold. + val danglingPrev = ByteArray(32) { 0x7F } + val realHead = + ConcordStreamEnvelope.wrap( + ControlEditionBuilder.rumor( + owner.pubKey, + ControlEntityKind.METADATA, + communityId, + 50, + danglingPrev, + ConcordJson.instance.encodeToString(MetadataEntity.serializer(), MetadataEntity(name = "Real")), + now, + null, + ), + control, + owner, + encrypted = false, + createdAt = now, + ) + + // carol holds nothing at all and mints a genesis-shaped decoy at version 1. + val forged = + ConcordStreamEnvelope.wrap( + ControlEditionBuilder.rumor( + carol.pubKey, + ControlEntityKind.METADATA, + communityId, + 1, + null, + ConcordJson.instance.encodeToString(MetadataEntity.serializer(), MetadataEntity(name = "PWNED")), + now, + null, + ), + control, + carol, + encrypted = false, + createdAt = now, + ) + + val newEpoch = community.rootEpoch + 1 + val newControl = + com.vitorpamplona.quartz.concord.crypto.ControlPlaneKeys + .forStaff(newRoot, communityId, newEpoch, newControlRoot) + val compacted = ConcordRefounding.compactControlPlane(listOf(realHead, forged), control, newControl, owner.pubKey) + + val carried = + compacted + .mapNotNull { ConcordStreamEnvelope.openOrNull(it, newControl) } + .mapNotNull { ControlEdition.fromRumor(it.rumor) } + .filter { it.entityKind == ControlEntityKind.METADATA } + + assertEquals(1, carried.size, "one metadata edition carried forward") + assertEquals(50, carried.single().version, "the owner's real head, not the forged genesis") + assertEquals("Real", ConcordJson.decodeOrNull(carried.single().content)?.name) + } } diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ControlRootRotationTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ControlRootRotationTest.kt index bcd5eec1e5..7a4440e1a0 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ControlRootRotationTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ControlRootRotationTest.kt @@ -109,6 +109,7 @@ class ControlRootRotationTest { recipientsXOnly = listOf(owner.pubKey, moderator.pubKey, member.pubKey), staffXOnly = setOf(owner.pubKey, moderator.pubKey), createdAt = now, + ownerPubKey = owner.pubKey, ) val baseRekey = ConcordKeyDerivation.baseRekeyAddress(community.communityRoot, community.communityId, build.newEpoch) @@ -149,6 +150,7 @@ class ControlRootRotationTest { recipientsXOnly = listOf(owner.pubKey, member.pubKey), staffXOnly = setOf(owner.pubKey), createdAt = now, + ownerPubKey = owner.pubKey, ) // The rotator's own view writes; a member's view of the same epoch only reads.