From 3a53292993741375352ecd307025dae0672770a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 15:02:43 +0000 Subject: [PATCH] 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)) + } }