mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-09-14 00:55:08 +00:00
fix(concord): three defects found auditing this branch's own changes
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj
This commit is contained in:
@@ -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<HexKey> {
|
||||
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<HexKey>(MAX_REFOUNDING_RECIPIENTS)
|
||||
val kept = LinkedHashSet<HexKey>()
|
||||
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()
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
+32
-3
@@ -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
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+32
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user