laantungir/amethyst
1
0
Fork 0
mirror of https://github.com/vitorpamplona/amethyst.git synced 2026-08-10 08:27:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(quartz): compaction must carry the authority-gated head, not the chain head

Browse Source 
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj
This commit is contained in:
Claude
2026-08-09 17:11:45 +00:00
parent 9cc19c60ca
commit f1241e891b
8 changed files with 138 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountConcordActions.kt
+14 -2
View File
@@ -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}")
commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActions.kt
+2
View File
@@ -545,6 +545,7 @@ object ConcordActions {
recipientsXOnly: List<HexKey>,
staffXOnly: Set<HexKey>,
createdAt: Long,
ownerPubKey: HexKey,
): RefoundingBuild =
ConcordRefounding.build(
rotatorSigner = rotatorSigner,
@@ -558,6 +559,7 @@ object ConcordActions {
recipientsXOnly = recipientsXOnly,
staffXOnly = staffXOnly,
createdAt = createdAt,
ownerPubKey = ownerPubKey,
)
/**
commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/ConcordActionsTest.kt
+1
View File
@@ -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)
commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/concord/ConcordRollbackFloorTest.kt
+3 -3
View File
@@ -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(
quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/AuthorityResolver.kt
+15 -3
View File
@@ -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
quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefounding.kt
+25 -13
View File
@@ -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<HexKey>,
staffXOnly: Set<HexKey>,
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<Event>,
priorControlKeys: ControlPlaneKeys,
newControlKeys: ControlPlaneKeys,
ownerPubKey: HexKey,
): List<Event> {
// entity coordinate -> every edition we can open, paired with its verified seal.
val byCoordinate = HashMap<String, MutableList<Pair<ControlEdition, Event>>>()
@@ -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<Event>(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<Event>(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
quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ConcordRefoundingTest.kt
+76
View File
@@ -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<MetadataEntity>(carried.single().content)?.name)
}
}
quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord06Rekey/ControlRootRotationTest.kt
+2
View File
@@ -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.