jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-10-27 04:30:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): authorize kind:9000 role changes in both directions (#3017)

Browse Source 
## Summary

NIP-29 `kind:9000` (PUT_USER) role changes were only authorized when the
**new** role was elevated. Demotions were unauthorized, so any
authenticated user could strip a channel owner to `member` with a single
event — and the demotion was unrecoverable, since the ex-owner then
lacked the privilege to restore themselves.

Reported by @Tyler in `#buzz-security`. Verified true, plus two adjacent
defects the report flagged and one it did not.

## The defects

1. **Demotion unauthorized.** The actor check only fired when the
*requested* role was elevated. Lowering someone's role skipped it
entirely.
2. **Open channels skipped the actor check.** It was nested under
`visibility == "private"`.
3. **`add_member` had no last-owner guard** while `remove_member` did —
so a channel could be left with zero owners.
4. **(Not in the report.)** An absent `role` tag defaulted to `Member`,
so a bare self-targeted PUT_USER silently demoted the sender. No
attacker required.

## The fix

**`crates/buzz-db/src/channel.rs`** — the authority, because it also
covers the desktop/admin callers that bypass the relay validator:

- Changing an **active** member's role requires an elevated actor **in
both directions**. Re-adding at the same role stays unguarded and
idempotent (the huddle bot-add and `kind:9021` join paths depend on
this).
- Last-owner guard in `add_member`, mirroring `remove_member`.
- Keyed on the **active** role (`removed_at IS NULL`). A soft-removed
row's role is history, not live authority — otherwise soft-deleted
ownership becomes a resurrection token: a kicked owner self-rejoins via
`9021` and silently regains ownership.
- New `pg_advisory_xact_lock` on a channel-membership namespace, taken
as the first statement in both `add_member` and `remove_member`. Both
read an owner `COUNT` and then write a *different* row, so READ
COMMITTED alone lets two concurrent demotions each observe 2 owners and
together leave 0.
- `remove_member`'s `is_agent_owner` lookup moved before the transaction
opens — it borrows a second pool connection, and issuing it while
holding the lock could self-deadlock on a small pool. Safe because
`agent_owner_pubkey` is immutable (first-mint-wins).

**`crates/buzz-relay/src/handlers/side_effects.rs`**:

- Role tag is now `Option` — absent means "no role change requested"
rather than defaulting to `Member`.
- Actor-role lookup hoisted out of the `visibility == "private"` block,
so open channels are covered.
- Role-change and last-owner guards on every visibility. Rejecting here
*as well as* in the DB means clients get a real error instead of an `OK`
whose side effect then fails silently.

## Verification

**Mutation tested — every guard stubbed individually to confirm a test
actually dies.** Three of eight guards were originally uncovered and
survived being disabled with the suite fully green:

| Guard | Dying test |
|---|---|
| DB actor-auth | *survived* → **new**
`unprivileged_member_cannot_demote_a_co_owner` |
| DB last-owner | `owner_can_still_manage_roles_after_demotion_guard` |
| DB active-role (soft-remove) |
`kicked_owner_rejoins_as_member_not_owner` + 3 |
| `add_member` advisory lock |
`membership_writes_serialize_on_the_shared_channel_lock` |
| `remove_member` advisory lock | +
`remove_member_rejects_an_actor_demoted_while_it_waited` |
| relay no-role-tag preservation |
`test_nip29_put_user_without_role_tag_preserves_role` |
| relay actor-auth | *survived* → **new**
`test_nip29_relay_rejects_role_change_by_unprivileged_actor` |
| relay last-owner | *survived* → **new**
`test_nip29_relay_rejects_last_owner_self_demotion` |

The three gaps shared one cause: every existing test asserts resulting
**state** ("the role did not change"), and the DB guards enforce that
state, masking every layer above them. With a relay guard stubbed the
relay answers `accepted:true` and logs `Side effect failed: access
denied: ...` while the state assertion still passes — the entire relay
validator could be deleted unnoticed. The new relay tests assert
`accepted == false` instead, the one observable only the validator
controls. Each new test is verified in both directions: green against
the real fix, failing with its intended message when its guard alone is
stubbed.

**Test runs** (at `9461eedb`):

- `buzz-db`, serial: **210 passed / 3 failed** — the same 3 failures as
clean `main` (202/3), which are pre-existing and unrelated
(`concurrent_same_owner_create…`,
`create_community_with_owner_is_atomic…`,
`test_usage_metrics_lock_has_single_owner…`). +8 = the new tests.
- `e2e_relay --ignored`: **40 passed / 3 failed**. Clean `main` on the
same relay is 35/6 — the same 3 infra failures
(`test_invite_mint_and_claim…`, `test_subscription_limit_enforced`,
`test_unarchive_emits_member_added_notification`) plus the 3 security
tests that fail unpatched and pass here.
- `cargo fmt`, `clippy`, `git diff --check` all clean.

**Live manual drive** against a locally running relay, using raw
`nak`-signed events (the `buzz` CLI refuses malformed `kind:9000`, so
the guards have to be exercised directly):

- *Rejected:* member demotes owner; member demotes admin; self-promote
to owner; self-promote to admin; admin demotes the last owner; sole
owner self-demote; demoted ex-owner demotes last owner; private-channel
member demotes owner; non-member demotes owner in private.
- *Allowed:* bare PUT_USER with no role tag (owner keeps role);
idempotent re-add at same role; owner promotes admin→owner, then owner2
legitimately demotes owner1.
- *Resurrection defeated:* owner promotes attacker to admin → kicks them
(`9001`) → attacker self-rejoins (`9021`) → returns as **member**, not
admin, and cannot demote the owner.
- Normal ops unaffected throughout: channel creation, messaging, member
listing, and legitimate governance all work.

## Behavior change to be aware of

Huddle bot-add sends `role="bot"`. If the target is **already an active
member at a different role**, that is now a role change and requires an
elevated actor. Previously it silently re-roled them — the same privesc
primitive through a different door, so narrowing it is intended.

This does not break the huddle flow in the path that matters: the
ephemeral channel add (the one that fails hard) is performed by the
host, who *created* that channel and is therefore its owner — verified
live. The parent-channel add is already explicitly best-effort,
capturing the error into `parent_error` with a comment anticipating "may
already be member"; adding a non-member agent there still works.
Flagging it rather than burying it.

## Notes

- Commit is **signoff-only, not cryptographically signed** — `-S` fails
in this environment (git tries to load the agent npub as an SSH key
file). DCO trailers are present and correct.
- Branch was merged with `origin/main` via `--no-ff` (not rebased).
Upstream had 17 commits, none touching these files, no migration
changes.

---------

Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
Co-authored-by: Dawn (sprout agent) <c6237ef84fa537c78dcee78efd2d4e59f728859c7f194da42ac51ededfa0be05@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
Tyler
2026-07-27 10:24:19 -04:00
committed by GitHub
parent c5c4f390b6
commit 00ecf2cac7
3 changed files with 1315 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

812
crates/buzz-db/src/channel.rs
View File

@@ -332,6 +332,40 @@ pub async fn set_canvas(
Ok(())
}
/// Namespace for the per-channel membership advisory lock. Serializes the
/// role-authorization + last-owner-count + write sequences in [`add_member`]
/// and [`remove_member`] against each other.
///
/// Both functions read an owner COUNT and then write a *different* row than the
/// one they counted, so `READ COMMITTED` snapshot isolation alone permits two
/// concurrent demotions (or a demotion racing a removal) to each observe two
/// owners, each pass, and together leave zero — the exact governance loss the
/// guards exist to prevent. An advisory key rather than `SELECT ... FOR UPDATE`
/// on the channel row: membership is its own contention domain and must not
/// serialize against unrelated channel metadata writers (`update_channel`,
/// `set_topic`, the TTL transition). Distinct key domain from
/// `buzz_channel_ttl:`.
const CHANNEL_MEMBERSHIP_LOCK_NAMESPACE: &str = "buzz_channel_membership:";
/// Take the per-channel membership lock. MUST be the first statement in the
/// transaction that then reads roles/owner counts and writes membership, so the
/// whole check-then-write sequence is atomic against a concurrent one.
async fn acquire_channel_membership_lock(
tx: &mut Transaction<'_, Postgres>,
community_id: CommunityId,
channel_id: Uuid,
) -> Result<()> {
sqlx::query("SELECT pg_advisory_xact_lock(hashtextextended($1, 0))")
.bind(format!(
"{CHANNEL_MEMBERSHIP_LOCK_NAMESPACE}{}:{}",
community_id.as_uuid(),
channel_id
))
.execute(&mut **tx)
.await?;
Ok(())
}
/// Add a member to a channel.
///
/// Role enforcement:
@@ -360,6 +394,10 @@ pub async fn add_member(
let mut tx = pool.begin().await?;
// First statement: serialize the whole role-check / owner-count / upsert
// sequence against concurrent membership writes on this channel.
acquire_channel_membership_lock(&mut tx, community_id, channel_id).await?;
let channel = get_channel_tx(&mut tx, community_id, channel_id).await?;
let effective_role = if channel.visibility == "private" {
@@ -411,6 +449,56 @@ pub async fn add_member(
}
};
// Changing an *active* member's role is privileged in BOTH directions.
// Demotion is as consequential as promotion: only owners/admins may grant
// elevated roles, so a demoted owner cannot restore themselves. Guarding
// only `role.is_elevated()` above therefore left owner→member demotion
// unauthorized-by-anyone. Re-adding an active member with the role they
// already hold stays idempotent and unguarded — the huddle bot-add and
// kind:9021 join paths rely on that.
//
// Deliberately keyed on the *active* role. A soft-removed row's stored role
// is history, not live authority: `removed_at` says it is no longer in
// force. Reactivation therefore lands at whatever `effective_role` the
// checks above already authorized — `Member` for any unprivileged caller,
// elevated only when a currently-elevated granter asked for it. Inferring
// current authority from a removed row would make soft-deleted ownership a
// resurrection token: an owner removed by another owner could self-rejoin
// via kind:9021 (`Member, None`) and silently regain ownership.
let current_role = get_active_role_tx(&mut tx, community_id, channel_id, pubkey).await?;
if let Some(current_role) = current_role.filter(|r| r != effective_role.as_str()) {
let actor_role = match invited_by {
Some(inviter) => get_active_role_tx(&mut tx, community_id, channel_id, inviter).await?,
None => None,
};
let actor_role: Option<MemberRole> = actor_role.and_then(|r| r.parse().ok());
if !actor_role.is_some_and(|r| r.is_elevated()) {
return Err(DbError::AccessDenied(
"only owners/admins may change an active member's role".to_string(),
));
}
// Defense-in-depth, mirroring `remove_member`: a demotion must not
// strip the channel of its last owner, which would leave nobody able
// to moderate, edit metadata, or re-grant ownership.
if current_role == "owner" && effective_role != MemberRole::Owner {
let row = sqlx::query(
"SELECT COUNT(*) as cnt FROM channel_members \
WHERE community_id = $1 AND channel_id = $2 AND role = 'owner' AND removed_at IS NULL",
)
.bind(community_id.as_uuid())
.bind(channel_id)
.fetch_one(&mut *tx)
.await?;
let owner_count: i64 = row.try_get("cnt")?;
if owner_count <= 1 {
return Err(DbError::AccessDenied(
"cannot demote the last owner — transfer ownership first".to_string(),
));
}
}
}
sqlx::query(
r#"
INSERT INTO channel_members (community_id, channel_id, pubkey, role, invited_by)
@@ -452,10 +540,19 @@ pub async fn add_member(
/// removing themselves.
///
/// Returns `Err(DbError::MemberNotFound)` if the target is not an active member.
/// The actor's role check and the UPDATE run inside a transaction to prevent a
/// TOCTOU race where the actor's role changes between the check and the update.
/// The `is_agent_owner` check runs outside the transaction against the main pool
/// because `agent_owner_pubkey` is immutable (set once at token mint).
///
/// The per-channel membership lock is the transaction's first statement, so the
/// actor's role check, the last-owner count, and the UPDATE are all serialized
/// against concurrent membership writes — otherwise a concurrent demotion of the
/// actor could commit after their role was read and this removal would proceed on
/// a stale elevated role.
///
/// The `is_agent_owner` lookup deliberately runs *before* the transaction opens:
/// it borrows a second connection from `pool`, and issuing it while holding the
/// lock could deadlock against ourselves on a small pool. That is safe because
/// `agent_owner_pubkey` is immutable — [`crate::user::set_agent_owner`] only
/// updates it when it `IS NULL` (first-mint-wins), so its value cannot change
/// under us and needs no serialization.
pub async fn remove_member(
pool: &PgPool,
community_id: CommunityId,
@@ -463,9 +560,24 @@ pub async fn remove_member(
pubkey: &[u8],
actor_pubkey: &[u8],
) -> Result<()> {
let is_self_remove = pubkey == actor_pubkey;
// Immutable, and must not be queried while holding the lock (second pool
// connection). Resolved up front so every *mutable* authorization read can
// sit behind the serialization point below.
let actor_is_agent_owner = if is_self_remove {
false
} else {
crate::user::is_agent_owner(pool, community_id, pubkey, actor_pubkey).await?
};
let mut tx = pool.begin().await?;
let is_self_remove = pubkey == actor_pubkey;
// First statement: serialize the actor-role check, the last-owner count and
// the UPDATE against concurrent membership writes on this channel (same key
// as `add_member`).
acquire_channel_membership_lock(&mut tx, community_id, channel_id).await?;
if !is_self_remove {
let actor_role_str = get_active_role_tx(&mut tx, community_id, channel_id, actor_pubkey)
.await?
@@ -473,11 +585,7 @@ pub async fn remove_member(
let actor_role: MemberRole = actor_role_str.parse().map_err(|_| {
DbError::InvalidData(format!("invalid role in database: {actor_role_str}"))
})?;
// Safe to query outside the transaction: agent_owner_pubkey is immutable
// (set once at token mint, first-mint-wins).
if !actor_role.is_elevated()
&& !crate::user::is_agent_owner(pool, community_id, pubkey, actor_pubkey).await?
{
if !actor_role.is_elevated() && !actor_is_agent_owner {
return Err(DbError::AccessDenied(
"only owners/admins or the agent's owner may remove other members".to_string(),
));
@@ -1892,4 +2000,688 @@ mod tests {
"random user should not be able to remove someone else's bot"
);
}
/// SECURITY REPRO (Dawn, kind:9000 demotion report): an unprivileged plain
/// member calls add_member with role=Member against the channel OWNER.
/// If this succeeds, add_member has no demotion authorization and no
/// last-owner guard.
#[tokio::test]
#[ignore = "requires Postgres"]
async fn repro_unprivileged_member_can_demote_owner() {
let pool = setup_pool().await;
let community_id = make_test_community(&pool).await;
let community = CommunityId::from_uuid(community_id);
let victim_owner = random_pubkey();
let attacker = random_pubkey();
for pk in [&victim_owner, &attacker] {
ensure_user(&pool, community, pk)
.await
.expect("ensure user");
}
let channel = create_test_channel(
&pool,
community_id,
"repro-demote-owner",
ChannelType::Stream,
ChannelVisibility::Open,
None,
&victim_owner,
None,
)
.await
.expect("create channel");
// create_test_channel already seeds the creator as 'owner', mirroring
// create_channel's own INSERT (channel.rs:131-145).
let role_of = |members: Vec<MemberRecord>, pk: Vec<u8>| -> Option<String> {
members.into_iter().find(|m| m.pubkey == pk).map(|m| m.role)
};
let before = role_of(
get_members(&pool, community, channel.id)
.await
.expect("members"),
victim_owner.clone(),
);
assert_eq!(
before.as_deref(),
Some("owner"),
"victim must start as owner"
);
// Attacker: plain member, not owner/admin.
add_member(
&pool,
community,
channel.id,
&attacker,
MemberRole::Member,
None,
)
.await
.expect("attacker self-joins open channel");
// The attack: attacker is `invited_by` and demotes the owner.
let res = add_member(
&pool,
community,
channel.id,
&victim_owner,
MemberRole::Member,
Some(&attacker),
)
.await;
let after = role_of(
get_members(&pool, community, channel.id)
.await
.expect("members"),
victim_owner.clone(),
);
let owners = get_members(&pool, community, channel.id)
.await
.expect("members")
.into_iter()
.filter(|m| m.role == "owner")
.count();
assert!(
res.is_err(),
"unprivileged member must not be able to demote the owner"
);
assert_eq!(after.as_deref(), Some("owner"), "owner role must survive");
assert_eq!(owners, 1, "channel must still have its owner");
}
/// SECURITY REPRO (Dawn): same demotion on a PRIVATE channel, where the
/// attacker is a plain member. The report claims any member suffices here.
#[tokio::test]
#[ignore = "requires Postgres"]
async fn repro_private_channel_member_can_demote_owner() {
let pool = setup_pool().await;
let community_id = make_test_community(&pool).await;
let community = CommunityId::from_uuid(community_id);
let victim_owner = random_pubkey();
let attacker = random_pubkey();
for pk in [&victim_owner, &attacker] {
ensure_user(&pool, community, pk)
.await
.expect("ensure user");
}
let channel = create_test_channel(
&pool,
community_id,
"repro-demote-owner-private",
ChannelType::Stream,
ChannelVisibility::Private,
None,
&victim_owner,
None,
)
.await
.expect("create private channel");
// Owner invites the attacker as a plain member (legitimate).
add_member(
&pool,
community,
channel.id,
&attacker,
MemberRole::Member,
Some(&victim_owner),
)
.await
.expect("owner invites attacker");
// Attack: plain member demotes the owner.
let res = add_member(
&pool,
community,
channel.id,
&victim_owner,
MemberRole::Member,
Some(&attacker),
)
.await;
let members = get_members(&pool, community, channel.id)
.await
.expect("members");
let victim_role = members
.iter()
.find(|m| m.pubkey == victim_owner)
.map(|m| m.role.clone());
let owners = members.iter().filter(|m| m.role == "owner").count();
assert!(
res.is_err(),
"plain member must not be able to demote the owner on a private channel"
);
assert_eq!(
victim_role.as_deref(),
Some("owner"),
"owner role must survive"
);
assert_eq!(owners, 1, "channel must still have its owner");
}
/// The fix must not break legitimate role management: an owner demoting a
/// co-owner (while another owner remains) must still succeed, and promotion
/// by an owner must still succeed.
#[tokio::test]
#[ignore = "requires Postgres"]
async fn owner_can_still_manage_roles_after_demotion_guard() {
let pool = setup_pool().await;
let community_id = make_test_community(&pool).await;
let community = CommunityId::from_uuid(community_id);
let owner = random_pubkey();
let other = random_pubkey();
for pk in [&owner, &other] {
ensure_user(&pool, community, pk)
.await
.expect("ensure user");
}
let channel = create_test_channel(
&pool,
community_id,
"roles-still-manageable",
ChannelType::Stream,
ChannelVisibility::Open,
None,
&owner,
None,
)
.await
.expect("create channel");
// Owner promotes `other` to owner — allowed (actor is elevated).
add_member(
&pool,
community,
channel.id,
&other,
MemberRole::Owner,
Some(&owner),
)
.await
.expect("owner may promote to owner");
// Owner demotes the co-owner back to member — allowed: actor is elevated
// and another owner remains, so the last-owner guard does not trip.
add_member(
&pool,
community,
channel.id,
&other,
MemberRole::Member,
Some(&owner),
)
.await
.expect("owner may demote a co-owner while another owner remains");
let members = get_members(&pool, community, channel.id)
.await
.expect("members");
let role_of = |pk: &Vec<u8>| {
members
.iter()
.find(|m| &m.pubkey == pk)
.map(|m| m.role.clone())
};
assert_eq!(role_of(&other).as_deref(), Some("member"));
assert_eq!(role_of(&owner).as_deref(), Some("owner"));
// Idempotent re-add at the SAME role must stay unguarded even from a
// non-elevated actor — the huddle bot-add path depends on this.
let bot = random_pubkey();
ensure_user(&pool, community, &bot)
.await
.expect("ensure bot");
add_member(
&pool,
community,
channel.id,
&bot,
MemberRole::Bot,
Some(&owner),
)
.await
.expect("add bot");
add_member(
&pool,
community,
channel.id,
&bot,
MemberRole::Bot,
Some(&other),
)
.await
.expect("re-adding at the same role must remain idempotent");
// But the last owner cannot be demoted, even by themselves.
let err = add_member(
&pool,
community,
channel.id,
&owner,
MemberRole::Member,
Some(&owner),
)
.await
.expect_err("last owner must not be demotable");
println!("last-owner demotion rejected: {err}");
}
/// Isolates the actor-authorization guard from the last-owner guard.
///
/// `repro_unprivileged_member_can_demote_owner` demotes the *sole* owner, so
/// the last-owner guard alone is enough to reject it: stubbing out the actor
/// check leaves that test green and the authorization hole invisible. Here a
/// second owner remains, so the last-owner guard cannot fire and only the
/// actor check stands between an unprivileged member and a co-owner's role.
#[tokio::test]
#[ignore = "requires Postgres"]
async fn unprivileged_member_cannot_demote_a_co_owner() {
let pool = setup_pool().await;
let community_id = make_test_community(&pool).await;
let community = CommunityId::from_uuid(community_id);
let owner = random_pubkey();
let co_owner = random_pubkey();
let attacker = random_pubkey();
for pk in [&owner, &co_owner, &attacker] {
ensure_user(&pool, community, pk)
.await
.expect("ensure user");
}
let channel = create_test_channel(
&pool,
community_id,
"co-owner-demotion-authz",
ChannelType::Stream,
ChannelVisibility::Open,
None,
&owner,
None,
)
.await
.expect("create channel");
add_member(
&pool,
community,
channel.id,
&co_owner,
MemberRole::Owner,
Some(&owner),
)
.await
.expect("owner may promote a co-owner");
add_member(
&pool,
community,
channel.id,
&attacker,
MemberRole::Member,
None,
)
.await
.expect("attacker self-joins the open channel");
// Two owners remain, so the last-owner guard cannot reject this. Only
// the actor-authorization check can.
let err = add_member(
&pool,
community,
channel.id,
&co_owner,
MemberRole::Member,
Some(&attacker),
)
.await
.expect_err("an unprivileged member must not demote a co-owner");
println!("co-owner demotion by unprivileged actor rejected: {err}");
let members = get_members(&pool, community, channel.id)
.await
.expect("members");
let role_of = |pk: &Vec<u8>| {
members
.iter()
.find(|m| &m.pubkey == pk)
.map(|m| m.role.clone())
};
assert_eq!(
role_of(&co_owner).as_deref(),
Some("owner"),
"co-owner must keep their role"
);
assert_eq!(
members.iter().filter(|m| m.role == "owner").count(),
2,
"both owners must survive"
);
}
/// Sets up an open channel with exactly two owners, returning
/// `(community, channel_id, owner_a, owner_b)`.
async fn channel_with_two_owners(
pool: &PgPool,
name: &str,
) -> (CommunityId, Uuid, Vec<u8>, Vec<u8>) {
let community_id = make_test_community(pool).await;
let community = CommunityId::from_uuid(community_id);
let owner_a = random_pubkey();
let owner_b = random_pubkey();
for pk in [&owner_a, &owner_b] {
ensure_user(pool, community, pk).await.expect("ensure user");
}
let channel = create_test_channel(
pool,
community_id,
name,
ChannelType::Stream,
ChannelVisibility::Open,
None,
&owner_a,
None,
)
.await
.expect("create channel");
add_member(
pool,
community,
channel.id,
&owner_b,
MemberRole::Owner,
Some(&owner_a),
)
.await
.expect("promote second owner");
(community, channel.id, owner_a, owner_b)
}
/// The lock must be shared with `remove_member`: a demotion racing an owner
/// removal goes through a separate count/update path, so both must serialize
/// on the same key or they can jointly empty the owner set.
///
/// Deterministic rather than timing-based: an outer transaction takes the
/// per-channel membership key first, then each membership writer must block
/// until it is released. Verified by mutation — dropping the lock from either
/// function makes that call return immediately and fails this test.
#[tokio::test]
#[ignore]
async fn membership_writes_serialize_on_the_shared_channel_lock() {
let pool = setup_pool().await;
let (community, channel_id, owner_a, owner_b) =
channel_with_two_owners(&pool, "membership-lock-shared").await;
for label in ["add_member", "remove_member"] {
// Hold the same advisory key an in-tree membership write would take.
let mut holder = pool.begin().await.expect("begin lock holder");
acquire_channel_membership_lock(&mut holder, community, channel_id)
.await
.expect("holder acquires membership key");
let pool2 = pool.clone();
let (target, actor) = (owner_a.clone(), owner_b.clone());
let mut writer = tokio::spawn(async move {
match label {
"add_member" => add_member(
&pool2,
community,
channel_id,
&target,
MemberRole::Member,
Some(&actor),
)
.await
.map(|_| ()),
_ => remove_member(&pool2, community, channel_id, &target, &actor).await,
}
});
// While the key is held, the writer must make no progress.
let blocked =
tokio::time::timeout(std::time::Duration::from_millis(750), &mut writer).await;
assert!(
blocked.is_err(),
"{label} completed while the channel membership key was held — \
it is not serializing on the shared lock"
);
println!("{label} blocked on the held membership key, as required");
// Releasing the key lets it proceed.
holder.rollback().await.expect("release membership key");
tokio::time::timeout(std::time::Duration::from_secs(10), writer)
.await
.expect("writer must proceed once the key is released")
.expect("writer task panicked")
.expect("writer must succeed after the key is released");
// Restore two owners for the next iteration.
add_member(
&pool,
community,
channel_id,
&owner_a,
MemberRole::Owner,
Some(&owner_b),
)
.await
.expect("restore second owner");
}
}
/// Every *mutable* authorization read must sit behind the membership lock.
/// A remover that reads its elevated role before acquiring the lock can be
/// demoted by a concurrent writer and still proceed on the stale role.
///
/// Deterministic: the holder takes the key, `remove_member` blocks on it, the
/// holder then demotes the remover and commits. Once the key is released the
/// remover must re-read its (now unprivileged) role and be rejected.
#[tokio::test]
#[ignore]
async fn remove_member_rejects_an_actor_demoted_while_it_waited() {
let pool = setup_pool().await;
let (community, channel_id, owner_a, owner_b) =
channel_with_two_owners(&pool, "stale-actor-role").await;
// owner_b removes a plain member, so the last-owner guard is not what
// rejects this — only the actor's own role can.
let victim = random_pubkey();
ensure_user(&pool, community, &victim)
.await
.expect("ensure victim");
add_member(
&pool,
community,
channel_id,
&victim,
MemberRole::Member,
Some(&owner_a),
)
.await
.expect("add victim");
let mut holder = pool.begin().await.expect("begin lock holder");
acquire_channel_membership_lock(&mut holder, community, channel_id)
.await
.expect("holder acquires membership key");
let pool2 = pool.clone();
let (actor, target) = (owner_b.clone(), victim.clone());
let mut remover = tokio::spawn(async move {
remove_member(&pool2, community, channel_id, &target, &actor).await
});
// Must be waiting on the key, not already authorized past it.
assert!(
tokio::time::timeout(std::time::Duration::from_millis(750), &mut remover)
.await
.is_err(),
"remove_member must block on the membership key before authorizing"
);
// Demote the waiting actor to a plain member and release the key.
sqlx::query(
"UPDATE channel_members SET role = 'member' \
WHERE community_id = $1 AND channel_id = $2 AND pubkey = $3",
)
.bind(community.as_uuid())
.bind(channel_id)
.bind(&owner_b)
.execute(&mut *holder)
.await
.expect("demote the waiting actor");
holder.commit().await.expect("commit demotion");
let result = tokio::time::timeout(std::time::Duration::from_secs(10), remover)
.await
.expect("remover must proceed once the key is released")
.expect("remover task panicked");
let err = result.expect_err("a demoted actor must not remove another member");
println!("stale-role removal rejected: {err}");
// The victim must still be an active member.
let members = get_members(&pool, community, channel_id)
.await
.expect("members");
assert!(
members.iter().any(|m| m.pubkey == victim),
"victim must not have been removed by a demoted actor"
);
}
/// A soft-removed row keeps its stored `role`, but that role is history,
/// not live authority — `removed_at` says it is no longer in force. So
/// reactivation must land at the baseline the caller was authorized for,
/// never at the role the row happens to remember.
///
/// Regression for the sharper vulnerability the alternative would create:
/// an owner kicked by another owner self-rejoins through the kind:9021
/// path (`Member`, no inviter) and must come back as a plain member. If
/// `add_member` inferred authority from the removed row, soft-deleted
/// ownership would be a resurrection token.
///
/// Two owners on purpose, so the last-owner guard can never be what
/// decides the outcome — only role resolution can.
#[tokio::test]
#[ignore]
async fn kicked_owner_rejoins_as_member_not_owner() {
let pool = setup_pool().await;
let (community, channel_id, owner_a, owner_b) =
channel_with_two_owners(&pool, "kicked-owner-rejoin").await;
// owner_a kicks owner_b (allowed: owner_a remains as the last owner).
remove_member(&pool, community, channel_id, &owner_b, &owner_a)
.await
.expect("an owner may remove another owner");
let stored: String = sqlx::query_scalar(
"SELECT role::text FROM channel_members \
WHERE community_id = $1 AND channel_id = $2 AND pubkey = $3",
)
.bind(community.as_uuid())
.bind(channel_id)
.bind(&owner_b)
.fetch_one(&pool)
.await
.expect("stored role survives soft removal");
assert_eq!(
stored, "owner",
"the removed row still remembers `owner` — which is exactly why \
authorization must not read it"
);
// The kind:9021 self-rejoin path: `Member`, no inviter.
add_member(
&pool,
community,
channel_id,
&owner_b,
MemberRole::Member,
None,
)
.await
.expect("a removed member may rejoin an open channel");
let rejoined = get_member_role(&pool, community, channel_id, &owner_b)
.await
.expect("read role after rejoin");
assert_eq!(
rejoined.as_deref(),
Some("member"),
"a kicked owner must rejoin at baseline privilege, not regain ownership"
);
}
/// The other side of the same boundary: reactivation may reach an elevated
/// role, but only because a *currently* elevated granter asked for it.
#[tokio::test]
#[ignore]
async fn removed_owner_is_restored_only_by_a_current_owner() {
let pool = setup_pool().await;
let (community, channel_id, owner_a, owner_b) =
channel_with_two_owners(&pool, "removed-owner-restore").await;
remove_member(&pool, community, channel_id, &owner_b, &owner_a)
.await
.expect("an owner may remove another owner");
// An unprivileged member cannot re-add them at `owner`.
let rando = random_pubkey();
ensure_user(&pool, community, &rando)
.await
.expect("ensure rando");
add_member(
&pool,
community,
channel_id,
&rando,
MemberRole::Member,
None,
)
.await
.expect("rando self-joins open channel");
let denied = add_member(
&pool,
community,
channel_id,
&owner_b,
MemberRole::Owner,
Some(&rando),
)
.await;
assert!(
matches!(denied, Err(DbError::AccessDenied(_))),
"an unprivileged actor must not re-add anyone at `owner`, got {denied:?}"
);
// The remaining owner can.
add_member(
&pool,
community,
channel_id,
&owner_b,
MemberRole::Owner,
Some(&owner_a),
)
.await
.expect("a current owner may restore ownership");
let restored = get_member_role(&pool, community, channel_id, &owner_b)
.await
.expect("read role after restore");
assert_eq!(restored.as_deref(), Some("owner"));
}
}

101
crates/buzz-relay/src/handlers/side_effects.rs
View File

@@ -291,35 +291,39 @@ pub async fn validate_admin_event(
match kind {
9000 => {
// Validate role tag if present
let role_str = extract_tag_value(event, "role").unwrap_or_else(|| "member".to_string());
if role_str.parse::<buzz_db::channel::MemberRole>().is_err() {
return Err(anyhow::anyhow!("invalid role: {role_str}"));
}
// An absent role tag means "no role change requested": for an existing
// member that preserves the role they already hold, and only defaults
// to Member for a genuinely new member. Defaulting unconditionally to
// Member made a bare self-targeted PUT_USER silently demote an owner.
let role_str = extract_tag_value(event, "role");
let requested_role = match role_str {
Some(ref s) => match s.parse::<buzz_db::channel::MemberRole>() {
Ok(r) => Some(r),
Err(_) => return Err(anyhow::anyhow!("invalid role: {s}")),
},
None => None,
};
let members = state.db.get_members(tenant.community(), channel_id).await?;
let actor_role: Option<buzz_db::channel::MemberRole> = members
.iter()
.find(|m| m.pubkey == actor_bytes)
.and_then(|m| m.role.parse().ok());
// PUT_USER: open channels allow any authenticated user; private channels
// require the actor to be an existing member (any role can invite).
if channel.visibility == "private" {
let members = state.db.get_members(tenant.community(), channel_id).await?;
let actor_member = members.iter().find(|m| m.pubkey == actor_bytes);
match actor_member {
Some(_) => {}
None => return Err(anyhow::anyhow!("actor not authorized")),
if actor_role.is_none() {
return Err(anyhow::anyhow!("actor not authorized"));
}
// Only owners/admins may grant elevated roles.
let role: buzz_db::channel::MemberRole = role_str.parse().unwrap();
if role.is_elevated() {
let actor_role: buzz_db::channel::MemberRole = actor_member
.unwrap()
.role
.parse()
.unwrap_or(buzz_db::channel::MemberRole::Member);
if !actor_role.is_elevated() {
return Err(anyhow::anyhow!(
"only owners/admins may grant elevated roles"
));
}
if requested_role.is_some_and(|r| r.is_elevated())
&& !actor_role.is_some_and(|r| r.is_elevated())
{
return Err(anyhow::anyhow!(
"only owners/admins may grant elevated roles"
));
}
}
@@ -327,6 +331,39 @@ pub async fn validate_admin_event(
let target_pubkey =
extract_p_tag(event).ok_or_else(|| anyhow::anyhow!("missing p tag"))?;
// Changing an ACTIVE existing member's role is privileged in both
// directions, on every visibility. `get_members` filters
// `removed_at IS NULL`, so a soft-removed row is deliberately not an
// "existing member" here: its stored role is history, not live
// authority, and reactivation is governed by the elevated-granter
// check above rather than by the role the row remembers.
//
// `add_member` is the authority (it also covers the desktop/admin
// callers that skip this validator); rejecting here too means the
// client gets a real error instead of an OK for an event whose side
// effect then fails. Re-adding at the same role stays idempotent —
// the huddle bot-add path relies on that.
if let Some((target, role)) = members
.iter()
.find(|m| m.pubkey == target_pubkey)
.zip(requested_role)
.filter(|(m, role)| m.role != role.as_str())
{
if !actor_role.is_some_and(|r| r.is_elevated()) {
return Err(anyhow::anyhow!(
"only owners/admins may change an active member's role"
));
}
if target.role == "owner"
&& role != buzz_db::channel::MemberRole::Owner
&& members.iter().filter(|m| m.role == "owner").count() <= 1
{
return Err(anyhow::anyhow!(
"cannot demote the last owner — transfer ownership first"
));
}
}
// Self-add: always allowed regardless of policy.
if target_pubkey == actor_bytes {
return Ok(());
@@ -1208,10 +1245,22 @@ async fn handle_put_user(
let channel_id =
extract_h_tag_channel(event).ok_or_else(|| anyhow::anyhow!("missing h tag"))?;
let target_pubkey = extract_p_tag(event).ok_or_else(|| anyhow::anyhow!("missing p tag"))?;
let role_str = extract_tag_value(event, "role").unwrap_or_else(|| "member".to_string());
let role: MemberRole = role_str
.parse()
.map_err(|_| anyhow::anyhow!("invalid role: {role_str}"))?;
// No role tag = no role change: preserve an existing member's current role and
// fall back to Member only for a new member. Unconditionally defaulting to
// Member let a bare PUT_USER silently demote an existing owner/admin.
let role: MemberRole = match extract_tag_value(event, "role") {
Some(role_str) => role_str
.parse()
.map_err(|_| anyhow::anyhow!("invalid role: {role_str}"))?,
None => state
.db
.get_members(tenant.community(), channel_id)
.await?
.iter()
.find(|m| m.pubkey == target_pubkey)
.and_then(|m| m.role.parse().ok())
.unwrap_or(MemberRole::Member),
};
let actor_bytes = event.pubkey.to_bytes().to_vec();

438
crates/buzz-test-client/tests/e2e_relay.rs
View File

@@ -2475,3 +2475,441 @@ async fn test_reply_ingest_pushes_live_thread_summary() {
client.disconnect().await.expect("disconnect");
}
/// Read a member's authoritative role from the relay-signed kind:39002 member
/// list. The relay's own view of membership, not the client's — a kind:9000 can
/// be `accepted` (stored) while its membership side effect fails, so asserting
/// on the OK alone cannot see a broken write.
async fn member_role(url: &str, keys: &Keys, channel_id: &str, pubkey_hex: &str) -> Option<String> {
let mut ws = BuzzTestClient::connect(url, keys).await.expect("connect");
let sid = sub_id("members");
let filter = Filter::new()
.kind(Kind::Custom(39002))
.custom_tags(SingleLetterTag::lowercase(Alphabet::D), [channel_id]);
ws.subscribe(&sid, vec![filter])
.await
.expect("subscribe 39002");
let events = ws
.collect_until_eose(&sid, Duration::from_secs(5))
.await
.expect("39002 EOSE");
ws.disconnect().await.ok();
// Latest 39002 wins; find this pubkey's tag. Shape is
// ["p", pubkey, relay_url, role] (side_effects.rs:1058-1062) — role is
// index 3, not 2.
events.iter().max_by_key(|e| e.created_at).and_then(|e| {
e.tags.iter().find_map(|t| {
let p = t.as_slice();
(p.len() >= 4 && p[0] == "p" && p[1] == pubkey_hex).then(|| p[3].clone())
})
})
}
/// SECURITY REPRO (Dawn): can an unprivileged NON-MEMBER demote the owner of an
/// OPEN channel to `member` with a single kind:9000? Asserts the reported
/// vulnerability is FIXED; it fails on vulnerable code.
#[tokio::test]
#[ignore]
async fn test_nip29_put_user_cannot_demote_owner() {
let url = relay_url();
let victim_keys = Keys::generate();
let victim_hex = victim_keys.public_key().to_hex();
let attacker_keys = Keys::generate();
// Victim creates an open channel -> victim is seeded as its owner.
let channel_id = create_test_channel(&victim_keys).await;
let before = member_role(&url, &victim_keys, &channel_id, &victim_hex).await;
assert_eq!(
before.as_deref(),
Some("owner"),
"victim must start as owner (got {before:?})"
);
// The attack: attacker (not a member, not the creator) publishes
// kind:9000 { h=channel, p=victim, role=member }.
let mut ws = BuzzTestClient::connect(&url, &attacker_keys)
.await
.expect("connect as attacker");
let event = EventBuilder::new(Kind::Custom(9000), "")
.tags([
Tag::parse(["h", &channel_id]).unwrap(),
Tag::parse(["p", &victim_hex]).unwrap(),
Tag::parse(["role", "member"]).unwrap(),
])
.sign_with_keys(&attacker_keys)
.expect("sign kind 9000");
let ok = ws.send_event(event).await.expect("send kind 9000");
ws.disconnect().await.ok();
let after = member_role(&url, &victim_keys, &channel_id, &victim_hex).await;
println!(
"attacker kind:9000 accepted = {} ({})",
ok.accepted, ok.message
);
println!("victim role before = {before:?}, after = {after:?}");
assert_eq!(
after.as_deref(),
Some("owner"),
"PRIVESC: unprivileged non-member demoted the channel owner to {after:?}"
);
assert!(
!ok.accepted,
"unprivileged non-member's role-demotion kind:9000 must be rejected"
);
}
/// SECURITY REPRO (Dawn), follow-on questions the report asserts but does not test:
/// after the owner is demoted, (a) can the attacker promote itself to owner, and
/// (b) can the ex-owner restore its own role? Both must be rejected — which is
/// exactly what makes the demotion unrecoverable over the relay.
#[tokio::test]
#[ignore]
async fn test_nip29_owner_demotion_recovery_paths() {
let url = relay_url();
let victim_keys = Keys::generate();
let victim_hex = victim_keys.public_key().to_hex();
let attacker_keys = Keys::generate();
let attacker_hex = attacker_keys.public_key().to_hex();
let channel_id = create_test_channel(&victim_keys).await;
let put_user = |signer: Keys, target_hex: String, role: &'static str| {
let channel_id = channel_id.clone();
let url = url.clone();
async move {
let mut ws = BuzzTestClient::connect(&url, &signer)
.await
.expect("connect");
// `allow_self_tagging` is REQUIRED: EventBuilder otherwise silently
// drops any `p` tag matching the signer (nostr-0.44.3
// builder.rs:435-449), which would make self-targeted PUT_USER
// events fail as "missing p tag" and mask the real verdict.
let event = EventBuilder::new(Kind::Custom(9000), "")
.allow_self_tagging()
.tags([
Tag::parse(["h", &channel_id]).unwrap(),
Tag::parse(["p", &target_hex]).unwrap(),
Tag::parse(["role", role]).unwrap(),
])
.sign_with_keys(&signer)
.expect("sign kind 9000");
let ok = ws.send_event(event).await.expect("send kind 9000");
ws.disconnect().await.ok();
ok
}
};
// Step 1: strip the owner.
let demote = put_user(attacker_keys.clone(), victim_hex.clone(), "member").await;
println!(
"1. attacker demotes owner -> accepted={} {}",
demote.accepted, demote.message
);
// Step 2: attacker tries to make itself owner.
let self_promote = put_user(attacker_keys.clone(), attacker_hex.clone(), "owner").await;
println!(
"2. attacker self->owner -> accepted={} {}",
self_promote.accepted, self_promote.message
);
// Step 3: ex-owner tries to restore itself.
let restore = put_user(victim_keys.clone(), victim_hex.clone(), "owner").await;
println!(
"3. ex-owner restores self -> accepted={} {}",
restore.accepted, restore.message
);
// `accepted` only means the event was stored — the membership side effect can
// still fail. Read the authoritative roles back from the relay-signed 39002.
let mut ws = BuzzTestClient::connect(&url, &victim_keys)
.await
.expect("connect");
let sid = sub_id("members-final");
let filter = Filter::new().kind(Kind::Custom(39002)).custom_tags(
SingleLetterTag::lowercase(Alphabet::D),
[channel_id.as_str()],
);
ws.subscribe(&sid, vec![filter])
.await
.expect("subscribe 39002");
let events = ws
.collect_until_eose(&sid, Duration::from_secs(5))
.await
.expect("39002 EOSE");
ws.disconnect().await.ok();
let latest = events.iter().max_by_key(|e| e.created_at).expect("a 39002");
let roles: Vec<(String, String)> = latest
.tags
.iter()
.filter_map(|t| {
let p = t.as_slice();
(p.len() >= 4 && p[0] == "p").then(|| (p[1].clone(), p[3].clone()))
})
.collect();
let role_of = |hex: &str| {
roles
.iter()
.find(|(pk, _)| pk == hex)
.map(|(_, r)| r.clone())
};
println!("FINAL victim role = {:?}", role_of(&victim_hex));
println!("FINAL attacker role = {:?}", role_of(&attacker_hex));
assert_eq!(
role_of(&victim_hex).as_deref(),
Some("owner"),
"owner must retain its role through all three attempts"
);
assert_eq!(
role_of(&attacker_hex),
None,
"attacker must never gain a role"
);
}
/// SECURITY REPRO (Dawn), finding #2: a kind:9000 PUT_USER with NO `role` tag.
///
/// `handle_put_user` used to default an absent role tag to `member`, so a bare
/// self-targeted PUT_USER silently demoted the sender — no attacker required.
/// `test_nip29_put_user_self_add_bypasses_policy` sends exactly this event but
/// asserts only `ok.accepted`, and side-effect failures are logged rather than
/// surfaced (ingest.rs:2460-2467), so an `accepted` assertion structurally
/// cannot observe the demotion. This asserts the resulting STATE instead.
///
/// TWO owners on purpose. With a sole owner the last-owner guard rejects the
/// demotion anyway, so the test would pass even with the role-preservation
/// removed — it would be measuring a different defect's fix. A second owner
/// disarms that guard, leaving the absent-role-tag handling as the only thing
/// that can decide the outcome.
#[tokio::test]
#[ignore]
async fn test_nip29_put_user_without_role_tag_preserves_role() {
let url = relay_url();
let owner_a = Keys::generate();
let owner_b = Keys::generate();
let b_hex = owner_b.public_key().to_hex();
let channel_id = create_test_channel(&owner_a).await;
// owner_a promotes owner_b, so the channel has two owners.
let mut ws = BuzzTestClient::connect(&url, &owner_a)
.await
.expect("connect as owner_a");
let promote = EventBuilder::new(Kind::Custom(9000), "")
.tags([
Tag::parse(["h", &channel_id]).unwrap(),
Tag::parse(["p", &b_hex]).unwrap(),
Tag::parse(["role", "owner"]).unwrap(),
])
.sign_with_keys(&owner_a)
.expect("sign promote");
let ok = ws.send_event(promote).await.expect("send promote");
ws.disconnect().await.ok();
assert!(ok.accepted, "promote rejected: {}", ok.message);
assert_eq!(
member_role(&url, &owner_a, &channel_id, &b_hex)
.await
.as_deref(),
Some("owner"),
"owner_b must be a second owner before the probe"
);
// The probe: owner_b sends a bare self-targeted PUT_USER — h + p, no `role`.
// `allow_self_tagging` is required or EventBuilder drops the self `p` tag
// (nostr-0.44.3 builder.rs:435-449) and the event fails as "missing p tag".
let mut ws = BuzzTestClient::connect(&url, &owner_b)
.await
.expect("connect as owner_b");
let bare = EventBuilder::new(Kind::Custom(9000), "")
.allow_self_tagging()
.tags([
Tag::parse(["h", &channel_id]).unwrap(),
Tag::parse(["p", &b_hex]).unwrap(),
])
.sign_with_keys(&owner_b)
.expect("sign bare put_user");
let ok = ws.send_event(bare).await.expect("send bare put_user");
ws.disconnect().await.ok();
assert!(
ok.accepted,
"self-targeted PUT_USER must stay accepted: {}",
ok.message
);
assert_eq!(
member_role(&url, &owner_a, &channel_id, &b_hex)
.await
.as_deref(),
Some("owner"),
"an absent role tag means no role change — it must not demote an owner"
);
}
/// SECURITY (Dawn), relay-layer guard isolation — the *validator*, not the DB.
///
/// The DB guards in `add_member` are what actually stop the privesc, and every
/// other test here asserts the resulting STATE ("the role didn't change").
/// That makes them structurally blind to `validate_admin_event`: stubbing out
/// either relay-side check leaves the whole nip29 suite green, because the DB
/// still refuses the write and the role is still correct. Verified by mutation
/// — the relay returns `accepted:true` and logs `Side effect failed: access
/// denied: ...` while the state assertion happily passes.
///
/// The relay guards earn their keep by giving the client an honest
/// `accepted:false` instead of an OK for an event that silently fails after the
/// fact. So these two tests assert `accepted == false` — the one observable
/// only the validator controls — and each is shaped so exactly one guard can
/// fire.
///
/// Guard under test: "only owners/admins may change an active member's role".
/// TWO owners on purpose, so the last-owner guard cannot fire and take the
/// credit; a plain member targeting a co-owner leaves the actor check as the
/// only thing that can reject.
#[tokio::test]
#[ignore]
async fn test_nip29_relay_rejects_role_change_by_unprivileged_actor() {
let url = relay_url();
let owner_a = Keys::generate();
let owner_b = Keys::generate();
let b_hex = owner_b.public_key().to_hex();
let attacker = Keys::generate();
let attacker_hex = attacker.public_key().to_hex();
let channel_id = create_test_channel(&owner_a).await;
// owner_a promotes owner_b -> the channel has two owners.
let mut ws = BuzzTestClient::connect(&url, &owner_a)
.await
.expect("connect as owner_a");
let promote = EventBuilder::new(Kind::Custom(9000), "")
.tags([
Tag::parse(["h", &channel_id]).unwrap(),
Tag::parse(["p", &b_hex]).unwrap(),
Tag::parse(["role", "owner"]).unwrap(),
])
.sign_with_keys(&owner_a)
.expect("sign promote");
let ok = ws.send_event(promote).await.expect("send promote");
ws.disconnect().await.ok();
assert!(ok.accepted, "promote rejected: {}", ok.message);
// The attacker joins the open channel as a plain member.
let mut ws = BuzzTestClient::connect(&url, &attacker)
.await
.expect("connect as attacker");
let join = EventBuilder::new(Kind::Custom(9000), "")
.allow_self_tagging()
.tags([
Tag::parse(["h", &channel_id]).unwrap(),
Tag::parse(["p", &attacker_hex]).unwrap(),
Tag::parse(["role", "member"]).unwrap(),
])
.sign_with_keys(&attacker)
.expect("sign self-join");
let ok = ws.send_event(join).await.expect("send self-join");
ws.disconnect().await.ok();
assert!(ok.accepted, "self-join rejected: {}", ok.message);
assert_eq!(
member_role(&url, &owner_a, &channel_id, &attacker_hex)
.await
.as_deref(),
Some("member"),
"attacker must be an active plain member before the probe"
);
// The probe: a plain member demotes a co-owner. Two owners remain, so only
// the actor-authorization guard can reject this.
let mut ws = BuzzTestClient::connect(&url, &attacker)
.await
.expect("connect as attacker");
let attack = EventBuilder::new(Kind::Custom(9000), "")
.tags([
Tag::parse(["h", &channel_id]).unwrap(),
Tag::parse(["p", &b_hex]).unwrap(),
Tag::parse(["role", "member"]).unwrap(),
])
.sign_with_keys(&attacker)
.expect("sign attack");
let ok = ws.send_event(attack).await.expect("send attack");
ws.disconnect().await.ok();
println!(
"unprivileged co-owner demotion -> accepted={} {}",
ok.accepted, ok.message
);
assert!(
!ok.accepted,
"the relay validator must reject an unprivileged actor's role change, \
not accept it and let the side effect fail silently"
);
assert_eq!(
member_role(&url, &owner_a, &channel_id, &b_hex)
.await
.as_deref(),
Some("owner"),
"co-owner must keep their role"
);
}
/// SECURITY (Dawn), relay-layer guard isolation — see the test above for why
/// `accepted` rather than state is the assertion that matters here.
///
/// Guard under test: the relay-side last-owner check. The actor is the SOLE
/// owner demoting themselves, so the actor-authorization guard is satisfied
/// (an owner is elevated) and cannot mask the result — only the last-owner
/// check can reject.
#[tokio::test]
#[ignore]
async fn test_nip29_relay_rejects_last_owner_self_demotion() {
let url = relay_url();
let owner = Keys::generate();
let owner_hex = owner.public_key().to_hex();
let channel_id = create_test_channel(&owner).await;
assert_eq!(
member_role(&url, &owner, &channel_id, &owner_hex)
.await
.as_deref(),
Some("owner"),
"creator must be the sole owner before the probe"
);
// The probe: the sole owner demotes themselves. Elevated actor, so the
// actor check passes; the last-owner guard is the only thing left.
let mut ws = BuzzTestClient::connect(&url, &owner)
.await
.expect("connect as owner");
let demote = EventBuilder::new(Kind::Custom(9000), "")
.allow_self_tagging()
.tags([
Tag::parse(["h", &channel_id]).unwrap(),
Tag::parse(["p", &owner_hex]).unwrap(),
Tag::parse(["role", "member"]).unwrap(),
])
.sign_with_keys(&owner)
.expect("sign self-demote");
let ok = ws.send_event(demote).await.expect("send self-demote");
ws.disconnect().await.ok();
println!(
"sole-owner self-demotion -> accepted={} {}",
ok.accepted, ok.message
);
assert!(
!ok.accepted,
"the relay validator must reject demoting the last owner, not accept \
it and let the side effect fail silently"
);
assert_eq!(
member_role(&url, &owner, &channel_id, &owner_hex)
.await
.as_deref(),
Some("owner"),
"the last owner must keep their role"
);
}