mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
docs(concord): audit the surfaces the first pass never opened
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj
This commit is contained in:
@@ -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.
|
||||
|
||||
## <a name="v10"></a>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.
|
||||
|
||||
## <a name="v11"></a>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.
|
||||
|
||||
## <a name="v12"></a>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.
|
||||
|
||||
Reference in New Issue
Block a user