mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
docs(concord): Banlist rank gap is a conformance bug, not a spec gap
Correcting the previous commit's reasoning. It concluded from the two implementations that CORD-04 does not rank-gate the Banlist and that enforcing it would be a unilateral divergence. Reading the actual spec (github.com/concord-protocol/concord, not in the Armada repo) shows the opposite: §3 is normative and binds "every action" — "the actor must hold the required bit and strictly outrank its target — equal cannot act on equal (an admin cannot ban a peer admin)" — and §5 step 3 restates it. Banning is the example the rule itself picks. Only §4, the section defining the Banlist, states the bit half alone. Both independent implementations read §4 in isolation and made the same mistake, which is evidence about the section rather than about the readers. So the fold fix is spec-mandated. It remains consensus-affecting (we would ignore bans Armada honors until they ship), so it wants coordination rather than a race, and the fold is still unchanged here. Adds docs/concord-banlist-rank-conformance.md to share upstream: verbatim spec citations, both implementations' gates, a delta-based rule that makes the per-target requirement expressible against a whole-list entity, and two further reproduced findings — a banned BAN-holder can lift their own ban (so bans do not stick against any BAN holder), and a forked ban survives an unban that does not chain onto it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
0ae6bc6698
commit
ea6762b137
@@ -95,12 +95,24 @@ pinned messages were never exercised.
|
||||
moderator's ban of a rank-1 admin is therefore **accepted** by the fold, and the admin then loses
|
||||
every permission (`hasPermission` is `!isBanned && …`). **Armada has the identical gap** — its
|
||||
`banlistGate` calls the rank-blind `isAuthorized(.., Permissions.BAN)` while its role path uses
|
||||
the rank-aware `canActOnPosition` — so enforcing rank on the fold unilaterally would make us
|
||||
ignore bans every other client honors. Needs a spec conversation, like CORD-05.
|
||||
Amethyst now refuses to *author* such a ban (`Account.concordBanTarget` and the Members roster
|
||||
both route through `canActOn`), which restricts only what we write, never what we accept. Three
|
||||
the rank-aware `canActOnPosition`.
|
||||
**This is a conformance bug, NOT a spec gap** — an earlier note here said the opposite and was
|
||||
wrong. CORD-04 §3 is explicit and normative: "One hard rule binds every action: the actor must
|
||||
hold the required bit **and** *strictly* outrank its target — equal cannot act on equal (an admin
|
||||
cannot ban a peer admin)", restated as step 3 of §5. Only §4, the section that defines the
|
||||
Banlist, omits the rank half — and both independent implementations read §4 in isolation and made
|
||||
the same mistake. Spec: <https://github.com/concord-protocol/concord> (`04.md`).
|
||||
So the fold change is spec-*mandated*, not a unilateral divergence. It is still
|
||||
consensus-affecting (we would ignore bans Armada honors until they ship), so it wants
|
||||
coordination rather than a race. Full write-up to share upstream:
|
||||
`docs/concord-banlist-rank-conformance.md`.
|
||||
Amethyst currently refuses only to *author* such a ban (`Account.concordBanTarget` and the Members
|
||||
roster both route through `canActOn`), which restricts what we write, never what we accept. Three
|
||||
`@Ignore`-d tests in `AuthorityResolverTest` record the fold-level invariant; un-ignore them when
|
||||
the spec closes it.
|
||||
we enforce.
|
||||
Also reproduced, and covered in the write-up: a banned member holding BAN can lift their own ban
|
||||
(the gate reads role-derived permissions, so bans do not stick against any BAN holder), and a
|
||||
forked ban survives an unban that does not chain onto it.
|
||||
- Notification cards whose target note isn't in `LocalCache` render "Event is loading or can't be
|
||||
found in your relay list" (seen on old zaps). `tagsAnEventByUser` needs the reacted-to note
|
||||
loaded, so deep history stays partially unresolved. Cosmetic, pre-existing.
|
||||
|
||||
@@ -0,0 +1,223 @@
|
||||
# Concord: the Banlist is not rank-gated in any implementation (CORD-04 conformance)
|
||||
|
||||
**Status:** conformance bug, reproduced in Amethyst, present by inspection in Armada.
|
||||
**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.
|
||||
|
||||
---
|
||||
|
||||
## 1. Summary
|
||||
|
||||
CORD-04 §3 requires that, for **every** action, the actor both hold the required permission bit
|
||||
**and** strictly outrank its target. It names banning as the worked example. But CORD-04 §4 — the
|
||||
section that actually defines the Banlist — states only the bit half of the rule:
|
||||
|
||||
> The Banlist is the one *anti*-roster: a signed list of npubs, honored only if its signer holds `BAN`.
|
||||
|
||||
Both known implementations implement §4 as written and omit §3's rank half at the Banlist write
|
||||
path. The result is that **a low-ranked moderator can ban the admins above them, and the owner.**
|
||||
|
||||
This is not a spec gap. §3 is unambiguous and normative. It is a conformance bug that both clients
|
||||
share, and it is almost certainly caused by §4 being readable in isolation.
|
||||
|
||||
## 2. What the spec requires
|
||||
|
||||
CORD-04 §3 (emphasis in original):
|
||||
|
||||
> `position` orders authority, **lower is higher**: the owner is position 0 (never a Role), a
|
||||
> roleless member is effectively last, and a member's rank is the lowest position among their Roles.
|
||||
> One hard rule binds every action: the actor must hold the required bit **and** *strictly* outrank
|
||||
> its target — equal cannot act on equal (an admin cannot ban a peer admin) — and no edition may
|
||||
> claim a position at or above its own signer, so nobody can promote themselves toward the top.
|
||||
|
||||
Two things stand out: the rule binds **every action**, and the parenthetical example it chooses is
|
||||
*exactly a ban* — "an admin cannot ban a peer admin."
|
||||
|
||||
Restated as step 3 of the normative authorisation algorithm, CORD-04 §5:
|
||||
|
||||
> To honor an action, a reader:
|
||||
> 1. Verifies the seal, learning the actor's real npub.
|
||||
> 2. Folds the Roster and resolves that npub's effective permissions and position.
|
||||
> 3. Confirms the actor holds the action's required bit **and** strictly outranks its target, traced
|
||||
> to the owner.
|
||||
|
||||
And on the owner, CORD-04 §2:
|
||||
|
||||
> The Roster is **owner-rooted**: … the chain terminates at the **owner**, who is proven by the
|
||||
> `community_id` itself (CORD-02), occupies position 0, and is **supreme and unremovable**.
|
||||
|
||||
The Grant path states the same rule inline, which is presumably why it got implemented there:
|
||||
|
||||
> A **Grant** maps a member's npub to their Roles, honored only if its signer outranks every Role it
|
||||
> hands out (§3).
|
||||
|
||||
CORD-02 §5 likewise restates it for Kicks: "a Kick honored only if its signer holds `KICK` and
|
||||
outranks the target, CORD-04".
|
||||
|
||||
**§4 is the only permissioned entity whose section does not restate the rank half.** That asymmetry
|
||||
looks like the proximate cause of the bug in both codebases.
|
||||
|
||||
## 3. What the implementations do
|
||||
|
||||
### Armada (`src/concord-v2/lib/control.ts`)
|
||||
|
||||
```ts
|
||||
const banlistGate = (p: ParsedEdition): boolean => {
|
||||
if (!isAuthorized(roster, p.author, ownerHex, Permissions.BAN)) return false;
|
||||
if (!citationOk(p)) return false;
|
||||
try {
|
||||
return Array.isArray(JSON.parse(p.content));
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
};
|
||||
```
|
||||
|
||||
with (`src/concord-v2/lib/roles.ts`):
|
||||
|
||||
```ts
|
||||
export function isAuthorized(
|
||||
roles: CommunityRoles, actorHex: string, ownerHex: string | undefined, permission: bigint,
|
||||
): boolean {
|
||||
if (ownerHex === actorHex) return true;
|
||||
return hasPermission(roles, actorHex, permission);
|
||||
}
|
||||
```
|
||||
|
||||
`isAuthorized` is a pure bit lookup. Notably Armada **already has** the correct primitive and uses
|
||||
it on the role path:
|
||||
|
||||
```ts
|
||||
export function canActOnPosition(
|
||||
roles: CommunityRoles, actorHex: string, ownerHex: string | undefined,
|
||||
targetPosition: number, permission: bigint,
|
||||
): boolean {
|
||||
if (ownerHex === actorHex) return true;
|
||||
return hasPermission(roles, actorHex, permission) &&
|
||||
outranks(roles, actorHex, ownerHex, targetPosition);
|
||||
}
|
||||
```
|
||||
|
||||
So the fix is largely a matter of routing the Banlist gate through the primitive that already
|
||||
exists — though see §5, because a whole-list entity needs a per-entry rule.
|
||||
|
||||
### Amethyst (`quartz/…/concord/cord04Roles/AuthorityResolver.kt`)
|
||||
|
||||
```kotlin
|
||||
fun banGate(e: ControlEdition): Boolean =
|
||||
e.author.lowercase() == ownerLower ||
|
||||
effectivePermissionsOf(e.author.lowercase()).has(ConcordPermissions.BAN)
|
||||
```
|
||||
|
||||
and the parallel gate in `ConcordCommunityState.authorizedHeads`:
|
||||
|
||||
```kotlin
|
||||
EditionFold.foldGated(list, floors) {
|
||||
authority.isOwner(it.author) || (bit != null && authority.hasPermission(it.author, bit))
|
||||
}
|
||||
```
|
||||
|
||||
Same shape: bit only, no rank. Amethyst also has a correct `canActOn(actor, target, bit)` that
|
||||
implements §3 faithfully, and uses it for Grants — and, since this report, for deciding what the UI
|
||||
will *author*. Neither client rank-checks the *contents* of a Banlist edition.
|
||||
|
||||
## 4. Consequences
|
||||
|
||||
All of the following were reproduced against Amethyst's fold with unit tests. Community: owner,
|
||||
`alice` = Admin at position 1 (holds `BAN`), `bob` = Mod at position 5 (holds `BAN`).
|
||||
|
||||
| # | Scenario | Result | Spec violated |
|
||||
|---|---|---|---|
|
||||
| 1 | `bob` (pos 5) bans `alice` (pos 1) | **ban is honored** | §3 "strictly outrank its target" |
|
||||
| 2 | `bob` bans the **owner** | **ban is honored** | §2 "supreme and unremovable" |
|
||||
| 3 | A banned member holding `BAN` unbans themselves | **honored** | §4 "drops every event from a banned npub … or authority action" |
|
||||
| 4 | A forked ban survives an unban that does not chain onto it | **stays banned** | — (interacts with §4 re-heal; see below) |
|
||||
|
||||
**Why #1 is escalation, not a cosmetic ordering issue.** Once banned, a member loses *all*
|
||||
authority: effective-permission checks are `!isBanned && …` in both clients, and §4 mandates that
|
||||
honest clients drop every event from a banned npub. So a single edition from the most junior `BAN`
|
||||
holder permanently silences every admin above them. In a community whose owner is inactive — the
|
||||
common case CORD-06 Refounding exists to address — there is no one left who can undo it.
|
||||
|
||||
**#2 is the sharpest violation.** The spec goes out of its way to protect position 0, including the
|
||||
clause "no Role may ever claim position 0, or an owner could create a peer nobody outranks." A
|
||||
Banlist entry bypasses all of that, because the Banlist is just a list of keys and nothing checks
|
||||
what is in it.
|
||||
|
||||
**#3 cuts the other way and is worth deciding deliberately.** Because the gate reads role-derived
|
||||
permissions rather than post-ban ones, a banned moderator can lift their own ban. So bans do not
|
||||
stick against anyone holding `BAN`. Note this one is a genuine fixpoint-ordering question, not a
|
||||
plain oversight: you cannot know who is banned until you fold the Banlist, and you cannot decide who
|
||||
may fold into the Banlist without knowing who is banned. The spec should say which order wins.
|
||||
|
||||
**#4 is a pre-existing interaction, not necessarily a bug.** §4's convergence design ("re-heal …
|
||||
guarantees convergence to the union") means a ban that forks off the head is unioned in and is not
|
||||
cleared by an unban that chains elsewhere. That is correct for *legitimate* concurrent bans. It
|
||||
becomes a weapon once #1 holds, because a rogue's ban is both unauthorised and sticky.
|
||||
|
||||
## 5. Proposed fix
|
||||
|
||||
The difficulty is that the Banlist is a single whole-list document ("replaced entire on every
|
||||
edit"), while §3's rule is stated per *target*. A gate can only be applied to a whole edition, so
|
||||
the rule needs a per-entry formulation. We suggest specifying it as a **delta rule**:
|
||||
|
||||
> An edition of the Banlist is evaluated against the state its `prev` denotes. Let *delta* be the
|
||||
> symmetric difference between the edition's list and that state's list — the npubs it adds and the
|
||||
> npubs it removes. For each npub in *delta*, the signer MUST hold `BAN` and strictly outrank that
|
||||
> npub, evaluated against the Roster as of that edition. The owner MUST NOT appear in the Banlist;
|
||||
> an edition adding them is invalid. Entries in *delta* the signer may not act on are **ignored**;
|
||||
> the remainder of the edition applies.
|
||||
|
||||
Rationale for each clause:
|
||||
|
||||
- **Delta, not whole list.** Otherwise every edition would have to re-justify every standing ban,
|
||||
and a moderator could never edit a list containing someone senior — including to add a peon.
|
||||
- **Removals are gated too.** Unbanning is an authority action on the target; without this, the
|
||||
rank rule is trivially bypassed (ban is symmetric with unban here).
|
||||
- **Ignore, don't reject.** Rejecting the whole edition would make one bad entry discard a
|
||||
legitimate bulk-ban — which §4 explicitly recommends as the collision remedy — and would let a
|
||||
rogue grief the list by forcing rejections. Ignoring converges and composes with re-heal.
|
||||
- **"As of that edition."** Ranks change. The spec should state that a Banlist edition's authority
|
||||
is judged against the Roster the fold has settled behind it, which is what §5's "traced to the
|
||||
owner" already implies for the Roster and what both clients already do for Grants.
|
||||
- **Owner excluded explicitly.** Today unbannability is only *derivable* from position 0 plus
|
||||
strict outranking. Since the Banlist bypasses rank entirely, an explicit prohibition is worth one
|
||||
sentence.
|
||||
|
||||
We'd also suggest **§4 restating the rank half inline**, the way §2 does for Grants and CORD-02 §5
|
||||
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.
|
||||
|
||||
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.
|
||||
|
||||
## 6. A note on rollout
|
||||
|
||||
This is a consensus-affecting change: a client that enforces the rank rule will ignore bans that a
|
||||
client which does not will honor, so the Banlist can diverge between implementations until both
|
||||
sides ship. We would rather coordinate that than race it.
|
||||
|
||||
Amethyst has therefore **not** changed its fold. We have only stopped our UI from *authoring* a ban
|
||||
the actor does not outrank — which restricts what we write, never what we accept, and so cannot
|
||||
diverge anyone's view. We are ready to enforce on the fold as soon as there is agreement on the rule
|
||||
above and a rough sense of timing.
|
||||
|
||||
## 7. Evidence
|
||||
|
||||
Reproductions live in Amethyst's `AuthorityResolverTest`
|
||||
(`quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/concord/cord04Roles/`), currently `@Ignore`d
|
||||
with this issue as the reason so they document the invariant without failing the build:
|
||||
|
||||
- `aBanHolderCannotBanAMemberItDoesNotOutrank`
|
||||
- `aBanHolderCannotBanTheOwner`
|
||||
- `anUnrankedBanIsDroppedWithoutOrphaningTheRestOfTheList` (asserts the "ignore, don't reject"
|
||||
semantics proposed above)
|
||||
|
||||
Two companion tests, which pass today, pin the behaviour a fix must preserve:
|
||||
|
||||
- `aBanHolderStillBansThoseItOutranks`
|
||||
- `theOwnerBansAnyone`
|
||||
|
||||
Happy to port these to Armada's suite or restate them as spec examples if that is useful.
|
||||
Reference in New Issue
Block a user