refactor(relay): extract NIP-29 membership authorization (#7285)

## Why

NIP-29 membership authorization for kinds 9000, 9001, and 9022 was
embedded in a large async handler, requiring Postgres and Redis to test
and repeating the last-owner rule across five call sites. This addresses
[TheSentinel454/buzz#24](https://github.com/TheSentinel454/buzz/issues/24)
without adding the broader state/repository abstraction suggested there
because HTTP and WebSocket writes already share ingestion.

## What

- Extract pure, typed membership authorization decisions while leaving
database reads and mutations in `validate_admin_event`.
- Preserve existing client-visible errors and independent database
last-owner safeguards.
- Collapse five last-owner policy restatements into one predicate and
share the identical self-departure policy.
- Include the relay decision modules in `just test-unit`, so these tests
execute in CI.

## Risk Assessment

Medium-low. This touches production relay authorization, but
intentionally preserves wire behavior and database defense in depth;
exhaustive decision tables and relay-backed tests cover the affected
paths.

## Simplification

This removes repeated policy from the orchestration path and makes the
rule set directly testable without introducing a repository trait or
second transport path.

## Verification

Verified at `15255a090797f85874921120003c645962fefaed`:

- `just test-unit` — all 10 package summaries passed. An initial run hit
two unrelated timing-sensitive `buzz-acp` failures; the complete retry
passed 905/905 in that package.
- `cargo fmt --all -- --check`
- `cargo clippy --workspace --all-targets -- -D warnings`
- `just file-size-check` — 10/10 policy tests passed.
- `just security-review-check` — 13/13 tests passed.
- Push preflight — `push-head-scope`, `branch-skew`, `file-size-check`,
`rust-tests`, and `desktop-tauri-checks` passed.
- Blox Postgres relay lane — 84/84 at the byte-identical source patch;
full live `e2e_relay` behavior matched the clean base (45 pass and the
same pre-existing kind:9002 failure on each).

Generated with Claude Code

---------

Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Luke Tornquist
2026-09-04 10:46:55 -04:00
committed by GitHub
parent 01bacb8df3
commit cd02b693aa
8 changed files with 916 additions and 134 deletions

View File

@@ -569,6 +569,7 @@ jobs:
cargo test -p buzz-test-client --test e2e_persona --test e2e_team_catalog --test e2e_nostr_interop --test e2e_project -- --ignored --nocapture
cargo test -p buzz-test-client --test e2e_relay invite -- --ignored --nocapture
cargo test -p buzz-test-client --test e2e_relay nip43_membership_snapshots_are_rejected -- --ignored --nocapture
cargo test -p buzz-test-client --test e2e_relay nip29_departure_wire -- --ignored --nocapture
env:
RELAY_URL: ws://localhost:3000
GIT_CREDENTIAL_NOSTR_BIN: ${{ github.workspace }}/target/ci/git-credential-nostr

View File

@@ -429,8 +429,21 @@ test-unit:
# disabled_mode_regression_pin_unauthenticated_request_is_served on the
# DB-free /probe route, and its Host/Origin gating is covered here by
# disabled_mode_still_requires_the_correct_host / _a_matching_origin.
# The second clause adds the relay's pure authorization-decision tests:
# the NIP-29 channel membership grid (handlers::channel_authz), the
# moderation capability grid (handlers::moderation_authz), and the pure
# helpers in handlers::side_effects. They ran in NO lane before —
# `test(/^api::admin::/)` never matched them, and the PostgreSQL lane
# pairs `--run-ignored ignored-only` with a `postgres_tests::`
# default-filter — so a red one shipped green, exactly the gap the
# api::admin clause above was added to close.
# Deliberately scoped to these three modules instead of all of
# `handlers::`: the wider set is mostly Postgres-backed, and five of its
# non-postgres_tests cases only "pass" without a database by waiting out
# the ~30s sqlx acquire timeout, so they do not belong in the infra-free
# unit job either.
cargo nextest run -p buzz-relay --lib \
-E 'test(/^api::admin::/) - test(=api::admin::tests::disabled_mode_allows_unauthenticated_requests_on_the_admin_host) - test(=api::admin::tests::nip98_mode_unrostered_signer_does_not_consume_a_replay_slot)'
-E '(test(/^api::admin::/) - test(=api::admin::tests::disabled_mode_allows_unauthenticated_requests_on_the_admin_host) - test(=api::admin::tests::nip98_mode_unrostered_signer_does_not_consume_a_replay_slot)) + test(/^handlers::channel_authz::/) + test(/^handlers::moderation_authz::/) + test(/^handlers::side_effects::tests::/)'
# ACP author-gate and queue tests protect the trust boundary between
# relay events and agent prompts. They are infra-free; ignored lifecycle
# tests remain excluded and run in their dedicated integration lanes.

View File

@@ -0,0 +1,694 @@
//! Pure NIP-29 channel membership-authority decisions (kinds 9000/9001/9022).
//!
//! `validate_admin_event` in [`super::side_effects`] keeps every database read;
//! this module holds only the policy those reads feed. Each function takes
//! already-resolved data and returns a typed decision, so the authorization
//! rules are exhaustively unit-testable without Postgres or Redis — the same
//! shell/pure split [`super::moderation_authz`] uses for the moderation
//! capability grid.
//!
//! ## Error strings are the wire contract
//!
//! Every [`ChannelAuthzError`] message is returned to clients verbatim in the
//! NIP-29 `OK` frame. The variants deliberately preserve the two historically
//! distinct last-owner phrasings — [`ChannelAuthzError::LastOwnerRemoval`] for
//! the pre-storage validator and
//! [`ChannelAuthzError::LastOwnerRemovalTransferFirst`] for the side-effect
//! appliers. The *rule* is defined once in [`is_sole_owner`]; only the wording
//! differs per call site.
use buzz_db::channel::{MemberRecord, MemberRole};
/// A membership-authority denial. `Display` is the client-visible reason.
#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
pub enum ChannelAuthzError {
/// Actor holds no authority for this action.
#[error("actor not authorized")]
ActorNotAuthorized,
/// Actor tried to grant `owner`/`admin` without holding it.
#[error("only owners/admins may grant elevated roles")]
ElevatedRoleGrantDenied,
/// Actor tried to change an active member's role without being elevated.
#[error("only owners/admins may change an active member's role")]
RoleChangeDenied,
/// Demoting the channel's only owner would orphan it.
#[error("cannot demote the last owner — transfer ownership first")]
LastOwnerDemotion,
/// Actor is not an active member of the channel.
#[error("actor is not an active member")]
NotActiveMember,
/// Removing the channel's only owner would orphan it (validator wording).
#[error("cannot remove the last owner")]
LastOwnerRemoval,
/// Removing the channel's only owner would orphan it (applier wording).
#[error("cannot remove the last owner — transfer ownership first")]
LastOwnerRemovalTransferFirst,
/// `owner_only` policy on an agent with no owner recorded.
#[error("policy:owner_only — agent has no owner set")]
PolicyOwnerOnlyNoOwner,
/// `owner_only` policy and the actor is not the agent's owner.
#[error("policy:owner_only — only the agent owner can add this agent")]
PolicyOwnerOnlyDenied,
/// `nobody` policy — the agent has opted out of third-party adds.
#[error("policy:nobody — this agent has disabled external channel additions")]
PolicyNobody,
}
/// Whether `pubkey` is the channel's only remaining `owner`.
///
/// This is the single definition of last-owner protection. Every call site
/// that once restated it — kind:9000 demotion, kind:9001 self-removal,
/// kind:9022 leave, and the `handle_remove_user` / `handle_leave_request`
/// appliers — asks this one question and supplies its own wording.
///
/// `members` must already be filtered to *active* membership; both
/// `get_members` and `get_members_for_event_write` are, so a soft-removed
/// owner row never counts toward the roster.
pub fn is_sole_owner(members: &[MemberRecord], pubkey: &[u8]) -> bool {
let mut owners = members.iter().filter(|m| m.role == "owner");
let sole = matches!(owners.next(), Some(first) if first.pubkey == pubkey);
sole && owners.next().is_none()
}
/// Decide whether `actor` may remove themselves from the channel.
///
/// Shared by kind:9001 self-removal and kind:9022 leave — they enforced
/// character-identical rules and messages before this seam existed.
pub fn decide_self_departure(
members: &[MemberRecord],
actor: &[u8],
) -> Result<(), ChannelAuthzError> {
if !members.iter().any(|m| m.pubkey == actor) {
return Err(ChannelAuthzError::NotActiveMember);
}
if is_sole_owner(members, actor) {
return Err(ChannelAuthzError::LastOwnerRemoval);
}
Ok(())
}
/// The outcome of a kind:9000 membership-authority check.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum PutUserDecision {
/// Authorized outright. A self-add bypasses the target's agent
/// `channel_add_policy` — you may always add yourself.
Allow,
/// Authorized so far; the caller must still load and evaluate the target's
/// agent `channel_add_policy` via [`decide_channel_add_policy`].
CheckAddPolicy,
}
/// Decide whether `actor` may add `target` to the channel, or change the role
/// `target` already holds (NIP-29 kind:9000 PUT_USER).
///
/// `actor_role` and `members` must come from an *active* membership read;
/// `requested_role` is `None` when the event carries no `role` tag, which
/// means "no role change requested" rather than "demote to member".
///
/// The database read for the target's agent channel-add policy stays with the
/// caller: this returns [`PutUserDecision::CheckAddPolicy`] when that read is
/// still required.
pub fn decide_put_user(
visibility: &str,
actor_role: Option<MemberRole>,
requested_role: Option<MemberRole>,
members: &[MemberRecord],
target: &[u8],
actor: &[u8],
) -> Result<PutUserDecision, ChannelAuthzError> {
// Open channels allow any authenticated user; private channels require the
// actor to be an existing active member. Any active member may add an
// ordinary member, guest, or bot, but only owners/admins may grant an
// elevated role.
if visibility == "private" {
if actor_role.is_none() {
return Err(ChannelAuthzError::ActorNotAuthorized);
}
if requested_role.is_some_and(|role| role.is_elevated())
&& !actor_role.is_some_and(|role| role.is_elevated())
{
return Err(ChannelAuthzError::ElevatedRoleGrantDenied);
}
}
// Changing an ACTIVE existing member's role is privileged in both
// directions, on every visibility. `members` comes from a `removed_at IS
// NULL` read, 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((existing, role)) = members
.iter()
.find(|m| m.pubkey == target)
.zip(requested_role)
.filter(|(m, role)| m.role != role.as_str())
{
if !actor_role.is_some_and(|r| r.is_elevated()) {
return Err(ChannelAuthzError::RoleChangeDenied);
}
if existing.role == "owner" && role != MemberRole::Owner && is_sole_owner(members, target) {
return Err(ChannelAuthzError::LastOwnerDemotion);
}
}
// Self-add: always allowed regardless of the target's agent policy.
if target == actor {
return Ok(PutUserDecision::Allow);
}
Ok(PutUserDecision::CheckAddPolicy)
}
/// Evaluate a target agent's `channel_add_policy` for a third-party add.
///
/// `policy` and `owner` come from `get_agent_channel_policy`; callers skip
/// this entirely when the target has no policy row, or when the add is a
/// self-add ([`PutUserDecision::Allow`]).
///
/// Unknown policy values allow. The database enum prevents them from being
/// stored, so this is defence in depth rather than reachable behaviour — if a
/// new value is added to the enum, extend this match.
pub fn decide_channel_add_policy(
policy: &str,
owner: Option<&[u8]>,
actor: &[u8],
) -> Result<(), ChannelAuthzError> {
match policy {
"owner_only" => {
let owner = owner.ok_or(ChannelAuthzError::PolicyOwnerOnlyNoOwner)?;
if actor != owner {
return Err(ChannelAuthzError::PolicyOwnerOnlyDenied);
}
Ok(())
}
"nobody" => Err(ChannelAuthzError::PolicyNobody),
_ => Ok(()),
}
}
/// What authority `actor` holds to remove *somebody else* (kind:9001).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum RemoveOtherDecision {
/// Channel owner or admin — authorized for any target.
Allow,
/// An active non-elevated member. Authorized only if they own the target
/// agent, which the caller must confirm with a database read.
CheckAgentOwner,
/// Not an active member. Denied without any further read — you must be in
/// the channel to remove anyone, even your own bot.
Deny,
}
/// Classify `actor`'s authority to remove another member (NIP-29 kind:9001).
pub fn classify_remove_other(members: &[MemberRecord], actor: &[u8]) -> RemoveOtherDecision {
match members.iter().find(|m| m.pubkey == actor) {
Some(m) if m.role == "owner" || m.role == "admin" => RemoveOtherDecision::Allow,
Some(_) => RemoveOtherDecision::CheckAgentOwner,
None => RemoveOtherDecision::Deny,
}
}
#[cfg(test)]
mod tests {
use super::*;
use chrono::Utc;
use uuid::Uuid;
/// Build a member roster from `(pubkey_byte, role)` pairs.
fn roster(entries: &[(u8, &str)]) -> Vec<MemberRecord> {
let channel_id = Uuid::new_v4();
entries
.iter()
.map(|(tag, role)| MemberRecord {
channel_id,
pubkey: vec![*tag; 32],
role: (*role).to_string(),
joined_at: Utc::now(),
invited_by: None,
removed_at: None,
})
.collect()
}
fn pk(tag: u8) -> Vec<u8> {
vec![tag; 32]
}
/// A roster literal: `(pubkey_tag, role)` pairs.
type Roster<'a> = &'a [(u8, &'a str)];
/// `(roster, subject, expected)`.
type SoleOwnerCase<'a> = (Roster<'a>, u8, bool);
/// `(roster, actor, expected_error)`.
type DepartureCase<'a> = (Roster<'a>, u8, Option<ChannelAuthzError>);
/// `(roster, actor, expected_decision)`.
type RemoveOtherCase<'a> = (Roster<'a>, u8, RemoveOtherDecision);
/// `(policy, owner_tag, actor, expected_error)`.
type AddPolicyCase<'a> = (&'a str, Option<u8>, u8, Option<ChannelAuthzError>);
/// The single definition of last-owner protection, over the full shape
/// space the five former call sites covered.
#[test]
fn sole_owner_table() {
let cases: &[SoleOwnerCase] = &[
// Only owner, and it is the subject.
(&[(1, "owner")], 1, true),
(&[(1, "owner"), (2, "member")], 1, true),
(&[(1, "owner"), (2, "admin"), (3, "bot")], 1, true),
// Only owner, but the subject is somebody else.
(&[(1, "owner"), (2, "member")], 2, false),
(&[(1, "owner")], 2, false),
// Two owners: neither is sole.
(&[(1, "owner"), (2, "owner")], 1, false),
(&[(1, "owner"), (2, "owner")], 2, false),
(&[(1, "owner"), (2, "owner"), (3, "member")], 3, false),
// No owners at all.
(&[(1, "member"), (2, "admin")], 1, false),
(&[], 1, false),
// An admin is not an owner.
(&[(1, "admin")], 1, false),
];
for (entries, subject, expected) in cases {
let members = roster(entries);
assert_eq!(
is_sole_owner(&members, &pk(*subject)),
*expected,
"roster {entries:?} subject {subject}"
);
}
}
/// kind:9001 self-removal and kind:9022 leave share one rule: the actor
/// must be an active member, and must not be the channel's only owner.
#[test]
fn self_departure_table() {
let cases: &[DepartureCase] = &[
// Non-member cannot leave.
(&[(1, "owner")], 9, Some(ChannelAuthzError::NotActiveMember)),
(&[], 1, Some(ChannelAuthzError::NotActiveMember)),
// Sole owner is pinned.
(
&[(1, "owner"), (2, "member")],
1,
Some(ChannelAuthzError::LastOwnerRemoval),
),
(
&[(1, "owner")],
1,
Some(ChannelAuthzError::LastOwnerRemoval),
),
// Co-owner may leave.
(&[(1, "owner"), (2, "owner")], 1, None),
// Non-owner roles may always leave.
(&[(1, "owner"), (2, "member")], 2, None),
(&[(1, "owner"), (2, "admin")], 2, None),
(&[(1, "owner"), (2, "bot")], 2, None),
(&[(1, "owner"), (2, "guest")], 2, None),
];
for (entries, actor, expected) in cases {
let members = roster(entries);
assert_eq!(
decide_self_departure(&members, &pk(*actor)).err(),
*expected,
"roster {entries:?} actor {actor}"
);
}
}
/// kind:9000 authorization, over visibility × actor role × requested role
/// × existing-target shape. Ordering matters: the private-channel gates
/// run before the role-change gates, which run before the self-add
/// shortcut.
#[test]
fn put_user_table() {
use ChannelAuthzError as E;
use MemberRole::{Admin, Member, Owner};
use PutUserDecision::{Allow, CheckAddPolicy};
// (visibility, roster, actor, actor_role, target, requested_role, expected)
type Case<'a> = (
&'a str,
&'a [(u8, &'a str)],
u8,
Option<MemberRole>,
u8,
Option<MemberRole>,
Result<PutUserDecision, E>,
);
let cases: &[Case] = &[
// ── Private channels require the actor to be an active member ──
(
"private",
&[(1, "owner")],
9,
None,
5,
Some(Member),
Err(E::ActorNotAuthorized),
),
// Even a self-add cannot bootstrap membership into a private channel.
(
"private",
&[(1, "owner")],
9,
None,
9,
None,
Err(E::ActorNotAuthorized),
),
// ── Private: only elevated actors may grant elevated roles ──
(
"private",
&[(1, "owner"), (2, "member")],
2,
Some(Member),
5,
Some(Admin),
Err(E::ElevatedRoleGrantDenied),
),
(
"private",
&[(1, "owner"), (2, "member")],
2,
Some(Member),
5,
Some(Owner),
Err(E::ElevatedRoleGrantDenied),
),
// An elevated actor may grant an elevated role.
(
"private",
&[(1, "owner")],
1,
Some(Owner),
5,
Some(Admin),
Ok(CheckAddPolicy),
),
(
"private",
&[(1, "owner"), (2, "admin")],
2,
Some(Admin),
5,
Some(Admin),
Ok(CheckAddPolicy),
),
// A plain member may still add an ordinary member to a private channel.
(
"private",
&[(1, "owner"), (2, "member")],
2,
Some(Member),
5,
Some(Member),
Ok(CheckAddPolicy),
),
// ── Open channels skip both private gates entirely ──
(
"open",
&[(1, "owner")],
9,
None,
5,
Some(Member),
Ok(CheckAddPolicy),
),
(
"open",
&[(1, "owner")],
9,
None,
5,
Some(Admin),
Ok(CheckAddPolicy),
),
// ── Changing an ACTIVE member's role is privileged on every visibility ──
(
"open",
&[(1, "owner"), (2, "member")],
9,
None,
2,
Some(Admin),
Err(E::RoleChangeDenied),
),
(
"open",
&[(1, "owner"), (2, "member"), (3, "member")],
3,
Some(Member),
2,
Some(Admin),
Err(E::RoleChangeDenied),
),
// Demotion is privileged in the same way as promotion.
(
"open",
&[(1, "owner"), (2, "admin"), (3, "member")],
3,
Some(Member),
2,
Some(Member),
Err(E::RoleChangeDenied),
),
// An elevated actor may change roles.
(
"open",
&[(1, "owner"), (2, "member")],
1,
Some(Owner),
2,
Some(Admin),
Ok(CheckAddPolicy),
),
// ── Last-owner demotion guard ──
// The sole owner demoting themselves.
(
"open",
&[(1, "owner"), (2, "member")],
1,
Some(Owner),
1,
Some(Member),
Err(E::LastOwnerDemotion),
),
// Another owner demoting the sole owner is impossible (they'd be an
// owner too), but an admin demoting the sole owner is not.
(
"open",
&[(1, "owner"), (2, "admin")],
2,
Some(Admin),
1,
Some(Member),
Err(E::LastOwnerDemotion),
),
// With a co-owner present the demotion is allowed.
(
"open",
&[(1, "owner"), (2, "owner")],
1,
Some(Owner),
2,
Some(Member),
Ok(CheckAddPolicy),
),
// Owner → Owner is not a demotion, so the guard does not fire; it is
// also not a role change, so it short-circuits as idempotent.
(
"open",
&[(1, "owner")],
1,
Some(Owner),
1,
Some(Owner),
Ok(Allow),
),
// ── Re-adding at the same role stays idempotent (huddle bot path) ──
(
"open",
&[(1, "owner"), (2, "bot")],
9,
None,
2,
Some(MemberRole::Bot),
Ok(CheckAddPolicy),
),
// An absent role tag requests no change, so the role-change gate
// never fires even for an unprivileged actor.
(
"open",
&[(1, "owner"), (2, "member")],
9,
None,
2,
None,
Ok(CheckAddPolicy),
),
// ── Self-add short-circuits the agent channel-add policy ──
("open", &[(1, "owner")], 9, None, 9, Some(Member), Ok(Allow)),
(
"open",
&[(1, "owner"), (2, "member")],
2,
Some(Member),
2,
Some(Member),
Ok(Allow),
),
(
"private",
&[(1, "owner"), (2, "member")],
2,
Some(Member),
2,
None,
Ok(Allow),
),
];
for (visibility, entries, actor, actor_role, target, requested_role, expected) in cases {
let members = roster(entries);
assert_eq!(
decide_put_user(
visibility,
*actor_role,
*requested_role,
&members,
&pk(*target),
&pk(*actor),
),
*expected,
"visibility {visibility} roster {entries:?} actor {actor} target {target} requested {requested_role:?}"
);
}
}
/// The target's agent `channel_add_policy`, evaluated for a third-party
/// add. Self-adds never reach here.
#[test]
fn channel_add_policy_table() {
use ChannelAuthzError as E;
// (policy, owner, actor, expected)
let cases: &[AddPolicyCase] = &[
// "anyone" allows any actor.
("anyone", None, 7, None),
("anyone", Some(1), 7, None),
// "nobody" blocks every actor, including the agent's own owner.
("nobody", Some(7), 7, Some(E::PolicyNobody)),
("nobody", None, 7, Some(E::PolicyNobody)),
// "owner_only" admits exactly the configured owner.
("owner_only", Some(7), 7, None),
("owner_only", Some(1), 7, Some(E::PolicyOwnerOnlyDenied)),
// "owner_only" with no owner recorded is a misconfiguration, and
// fails closed with its own distinct message.
("owner_only", None, 7, Some(E::PolicyOwnerOnlyNoOwner)),
// Unknown values fall through to allow; the DB enum prevents them
// from being stored, so this is defence in depth.
("something_new", None, 7, None),
("", None, 7, None),
];
for (policy, owner, actor, expected) in cases {
let owner_bytes = owner.map(pk);
assert_eq!(
decide_channel_add_policy(policy, owner_bytes.as_deref(), &pk(*actor)).err(),
*expected,
"policy {policy} owner {owner:?} actor {actor}"
);
}
}
/// kind:9001 removal of somebody else. A plain member is not denied
/// outright — they may still own the target agent, which requires a
/// database read the caller performs.
#[test]
fn remove_other_table() {
use RemoveOtherDecision::{Allow, CheckAgentOwner, Deny};
let cases: &[RemoveOtherCase] = &[
// Owners and admins may remove anyone.
(&[(1, "owner"), (2, "member")], 1, Allow),
(&[(1, "owner"), (2, "admin")], 2, Allow),
// A plain member, guest, or bot may only remove an agent they own.
(&[(1, "owner"), (2, "member")], 2, CheckAgentOwner),
(&[(1, "owner"), (2, "guest")], 2, CheckAgentOwner),
(&[(1, "owner"), (2, "bot")], 2, CheckAgentOwner),
// Non-members are denied without an agent-owner read: you must be
// in the channel to remove anyone, even your own bot.
(&[(1, "owner")], 9, Deny),
(&[], 9, Deny),
];
for (entries, actor, expected) in cases {
let members = roster(entries);
assert_eq!(
classify_remove_other(&members, &pk(*actor)),
*expected,
"roster {entries:?} actor {actor}"
);
}
}
/// The wire contract: these strings reach NIP-29 clients verbatim, and the
/// two last-owner phrasings are deliberately distinct.
#[test]
fn error_strings_are_the_wire_contract() {
let cases = [
(
ChannelAuthzError::ActorNotAuthorized,
"actor not authorized",
),
(
ChannelAuthzError::ElevatedRoleGrantDenied,
"only owners/admins may grant elevated roles",
),
(
ChannelAuthzError::RoleChangeDenied,
"only owners/admins may change an active member's role",
),
(
ChannelAuthzError::LastOwnerDemotion,
"cannot demote the last owner — transfer ownership first",
),
(
ChannelAuthzError::NotActiveMember,
"actor is not an active member",
),
(
ChannelAuthzError::LastOwnerRemoval,
"cannot remove the last owner",
),
(
ChannelAuthzError::LastOwnerRemovalTransferFirst,
"cannot remove the last owner — transfer ownership first",
),
(
ChannelAuthzError::PolicyOwnerOnlyNoOwner,
"policy:owner_only — agent has no owner set",
),
(
ChannelAuthzError::PolicyOwnerOnlyDenied,
"policy:owner_only — only the agent owner can add this agent",
),
(
ChannelAuthzError::PolicyNobody,
"policy:nobody — this agent has disabled external channel additions",
),
];
for (error, expected) in cases {
assert_eq!(error.to_string(), expected);
}
}
}

View File

@@ -2,6 +2,8 @@
pub mod admin_action_worker;
pub mod admin_outbox_worker;
pub mod auth;
/// Pure NIP-29 channel membership-authority decisions (kinds 9000/9001/9022).
pub mod channel_authz;
/// Subscription close (CLOSE) handler.
pub mod close;
/// Command executor — transactional processing for command kinds.

View File

@@ -16,6 +16,7 @@ use buzz_core::kind::{
use buzz_core::StoredEvent;
use buzz_db::channel::{MemberRecord, MemberRole};
use super::channel_authz::{self, ChannelAuthzError, PutUserDecision, RemoveOtherDecision};
use super::event::dispatch_persistent_event;
use crate::protocol::RelayMessage;
use crate::state::AppState;
@@ -365,120 +366,51 @@ pub async fn validate_admin_event(
let target_pubkey =
extract_p_tag(event).ok_or_else(|| anyhow::anyhow!("missing p tag"))?;
// PUT_USER: open channels allow any authenticated user; private channels
// require the actor to be an existing active member. Any active member may
// add an ordinary member, guest, or bot, but only owners/admins may grant
// an elevated role.
if channel.visibility == "private" {
if actor_role.is_none() {
return Err(anyhow::anyhow!("actor not authorized"));
}
if requested_role.is_some_and(|role| role.is_elevated())
&& !actor_role.is_some_and(|role| role.is_elevated())
{
return Err(anyhow::anyhow!(
"only owners/admins may grant elevated roles"
));
}
}
// 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(());
}
// Third-party add: check channel_add_policy on the target.
if let Some((policy, owner)) = state
.db
.get_agent_channel_policy(tenant.community(), &target_pubkey)
.await?
{
match policy.as_str() {
"owner_only" => {
let owner_bytes = owner.ok_or_else(|| {
anyhow::anyhow!("policy:owner_only — agent has no owner set")
})?;
if actor_bytes != owner_bytes {
return Err(anyhow::anyhow!(
"policy:owner_only — only the agent owner can add this agent"
));
}
// Authorization policy — visibility gate, elevated-grant gate,
// active-member role-change gate, and last-owner demotion — lives in
// `channel_authz`, which is pure and table-tested. The database reads
// it depends on stay here.
match channel_authz::decide_put_user(
&channel.visibility,
actor_role,
requested_role,
&members,
&target_pubkey,
&actor_bytes,
)? {
// Self-add: always allowed regardless of policy.
PutUserDecision::Allow => Ok(()),
// Third-party add: check channel_add_policy on the target.
PutUserDecision::CheckAddPolicy => {
if let Some((policy, owner)) = state
.db
.get_agent_channel_policy(tenant.community(), &target_pubkey)
.await?
{
channel_authz::decide_channel_add_policy(
&policy,
owner.as_deref(),
&actor_bytes,
)?;
}
"nobody" => {
return Err(anyhow::anyhow!(
"policy:nobody — this agent has disabled external channel additions"
));
}
// "anyone" or any unknown value → allow.
// NOTE: DB ENUM constraint prevents unknown values from being stored.
// If a new policy value is added to the ENUM, update this match.
_ => {}
Ok(())
}
}
Ok(())
}
9001 => {
// REMOVE_USER: self-remove allowed unless actor is the last owner; removing others requires owner/admin
let target_pubkey =
extract_p_tag(event).ok_or_else(|| anyhow::anyhow!("missing p tag"))?;
let members = state.db.get_members(tenant.community(), channel_id).await?;
if target_pubkey == actor_bytes {
// Self-removal: must be an active member, and cannot be the last owner.
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 {
None => {
return Err(anyhow::anyhow!("actor is not an active member"));
}
Some(m) if m.role == "owner" => {
let owner_count = members.iter().filter(|m| m.role == "owner").count();
if owner_count <= 1 {
return Err(anyhow::anyhow!("cannot remove the last owner"));
}
}
_ => {}
}
channel_authz::decide_self_departure(&members, &actor_bytes)?;
Ok(())
} else {
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(m) if m.role == "owner" || m.role == "admin" => Ok(()),
Some(_) => {
match channel_authz::classify_remove_other(&members, &actor_bytes) {
RemoveOtherDecision::Allow => Ok(()),
RemoveOtherDecision::CheckAgentOwner => {
if state
.db
.is_agent_owner(tenant.community(), &target_pubkey, &actor_bytes)
@@ -486,13 +418,13 @@ pub async fn validate_admin_event(
{
Ok(())
} else {
Err(anyhow::anyhow!("actor not authorized"))
Err(ChannelAuthzError::ActorNotAuthorized.into())
}
}
// Non-members fall here. We intentionally do NOT check
// is_agent_owner for non-members — you must be in the channel
// to remove anyone, even your own bot.
_ => Err(anyhow::anyhow!("actor not authorized")),
RemoveOtherDecision::Deny => Err(ChannelAuthzError::ActorNotAuthorized.into()),
}
}
}
@@ -742,20 +674,9 @@ pub async fn validate_admin_event(
}
9022 => {
// LEAVE_REQUEST: must be an active member, and cannot be the last owner.
// Identical rule to kind:9001 self-removal, including its wording.
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 {
None => {
return Err(anyhow::anyhow!("actor is not an active member"));
}
Some(m) if m.role == "owner" => {
let owner_count = members.iter().filter(|m| m.role == "owner").count();
if owner_count <= 1 {
return Err(anyhow::anyhow!("cannot remove the last owner"));
}
}
_ => {}
}
channel_authz::decide_self_departure(&members, &actor_bytes)?;
Ok(())
}
_ => Ok(()),
@@ -1459,14 +1380,8 @@ async fn handle_remove_user(
.db
.get_members_for_event_write(tenant.community(), channel_id)
.await?;
let owner_count = members.iter().filter(|m| m.role == "owner").count();
let actor_is_owner = members
.iter()
.any(|m| m.pubkey == actor_bytes && m.role == "owner");
if actor_is_owner && owner_count <= 1 {
return Err(anyhow::anyhow!(
"cannot remove the last owner — transfer ownership first"
));
if channel_authz::is_sole_owner(&members, &actor_bytes) {
return Err(ChannelAuthzError::LastOwnerRemovalTransferFirst.into());
}
}
@@ -2126,14 +2041,8 @@ async fn handle_leave_request(
.db
.get_members_for_event_write(tenant.community(), channel_id)
.await?;
let owner_count = members.iter().filter(|m| m.role == "owner").count();
let actor_is_owner = members
.iter()
.any(|m| m.pubkey == actor_bytes && m.role == "owner");
if actor_is_owner && owner_count <= 1 {
return Err(anyhow::anyhow!(
"cannot remove the last owner — transfer ownership first"
));
if channel_authz::is_sole_owner(&members, &actor_bytes) {
return Err(ChannelAuthzError::LastOwnerRemovalTransferFirst.into());
}
state

View File

@@ -2345,6 +2345,154 @@ async fn add_member_with_role_ws(
(ok.accepted, ok.message)
}
/// Submit a self-targeted NIP-29 departure and return the relay's exact OK
/// frame payload. kind:9001 carries a self `p` tag; kind:9022 does not.
async fn self_departure_ws(url: &str, channel_id: &str, actor: &Keys, kind: u16) -> (bool, String) {
let h_tag = Tag::parse(["h", channel_id]).unwrap();
let event = match kind {
9001 => EventBuilder::new(Kind::Custom(kind), "")
.allow_self_tagging()
.tags([
h_tag,
Tag::parse(["p", &actor.public_key().to_hex()]).unwrap(),
])
.sign_with_keys(actor)
.expect("sign kind:9001 self-removal"),
9022 => EventBuilder::new(Kind::Custom(kind), "")
.tags([h_tag])
.sign_with_keys(actor)
.expect("sign kind:9022 leave request"),
_ => panic!("unsupported self-departure kind: {kind}"),
};
let mut client = BuzzTestClient::connect(url, actor)
.await
.expect("connect departure actor");
let ok = client.send_event(event).await.expect("send self-departure");
client.disconnect().await.ok();
(ok.accepted, ok.message)
}
async fn promote_co_owner(url: &str, channel_id: &str, owner: &Keys, co_owner: &Keys) {
let mut client = BuzzTestClient::connect(url, owner)
.await
.expect("connect channel owner");
let result = add_member_with_role_ws(
&mut client,
channel_id,
&co_owner.public_key().to_hex(),
"owner",
owner,
)
.await;
client.disconnect().await.ok();
assert_eq!(result, (true, String::new()), "promote co-owner OK frame");
assert_eq!(
member_role(url, owner, channel_id, &co_owner.public_key().to_hex())
.await
.as_deref(),
Some("owner"),
"the setup must leave a second active owner"
);
}
/// Binds kind:9001's production `validate_admin_event` call to the WebSocket
/// OK frame. The DB applier has a different rejection message, so this exact
/// historical result can only come from the pre-storage relay validator.
#[tokio::test]
#[ignore]
async fn test_nip29_departure_wire_kind_9001_sole_owner_rejected() {
let url = relay_url();
let owner = Keys::generate();
let channel_id = create_test_channel(&owner).await;
let result = self_departure_ws(&url, &channel_id, &owner, 9001).await;
assert_eq!(
result,
(false, "invalid: cannot remove the last owner".to_string())
);
}
/// An open channel lets the nonmember event reach the per-kind validator; a
/// private channel would be rejected earlier by the generic membership gate.
#[tokio::test]
#[ignore]
async fn test_nip29_departure_wire_kind_9001_nonmember_rejected() {
let url = relay_url();
let owner = Keys::generate();
let nonmember = Keys::generate();
let channel_id = create_test_channel(&owner).await;
let result = self_departure_ws(&url, &channel_id, &nonmember, 9001).await;
assert_eq!(
result,
(false, "invalid: actor is not an active member".to_string())
);
}
#[tokio::test]
#[ignore]
async fn test_nip29_departure_wire_kind_9001_co_owner_allowed() {
let url = relay_url();
let owner = Keys::generate();
let co_owner = Keys::generate();
let channel_id = create_test_channel(&owner).await;
promote_co_owner(&url, &channel_id, &owner, &co_owner).await;
let result = self_departure_ws(&url, &channel_id, &co_owner, 9001).await;
assert_eq!(result, (true, String::new()));
}
/// Binds kind:9022's distinct production `validate_admin_event` call to the
/// same historical WebSocket rejection contract as self-removal.
#[tokio::test]
#[ignore]
async fn test_nip29_departure_wire_kind_9022_sole_owner_rejected() {
let url = relay_url();
let owner = Keys::generate();
let channel_id = create_test_channel(&owner).await;
let result = self_departure_ws(&url, &channel_id, &owner, 9022).await;
assert_eq!(
result,
(false, "invalid: cannot remove the last owner".to_string())
);
}
#[tokio::test]
#[ignore]
async fn test_nip29_departure_wire_kind_9022_nonmember_rejected() {
let url = relay_url();
let owner = Keys::generate();
let nonmember = Keys::generate();
let channel_id = create_test_channel(&owner).await;
let result = self_departure_ws(&url, &channel_id, &nonmember, 9022).await;
assert_eq!(
result,
(false, "invalid: actor is not an active member".to_string())
);
}
#[tokio::test]
#[ignore]
async fn test_nip29_departure_wire_kind_9022_co_owner_allowed() {
let url = relay_url();
let owner = Keys::generate();
let co_owner = Keys::generate();
let channel_id = create_test_channel(&owner).await;
promote_co_owner(&url, &channel_id, &owner, &co_owner).await;
let result = self_departure_ws(&url, &channel_id, &co_owner, 9022).await;
assert_eq!(result, (true, String::new()));
}
/// Any active member can add any ordinary role to a private channel.
#[tokio::test]
#[ignore]

View File

@@ -125,6 +125,18 @@ run_unit_tests() {
# step with `just test-unit`; ignored lifecycle tests run elsewhere.
run_test_step "buzz-acp unit tests" \
cargo test -p buzz-acp --lib -- --nocapture
# Mirror the three infra-free relay handler modules in `just test-unit`'s
# nextest expression. Keep the side-effects filter pinned to `::tests::` so
# it does not select the sibling Postgres-backed test module.
run_test_step "buzz-relay channel authorization tests" \
cargo test -p buzz-relay --lib handlers::channel_authz:: -- --nocapture
run_test_step "buzz-relay moderation authorization tests" \
cargo test -p buzz-relay --lib handlers::moderation_authz:: -- --nocapture
run_test_step "buzz-relay side-effects helper tests" \
cargo test -p buzz-relay --lib handlers::side_effects::tests:: -- --nocapture
}
# ---- DB / integration tests (infra required) --------------------------------

View File

@@ -37,6 +37,7 @@ clients_call=$(extract_job clients "$orchestrator")
mobile_swift_call=$(extract_job mobile-swift-domain "$orchestrator")
desktop_gate=$(extract_job desktop "$orchestrator")
macos_gate=$(extract_job desktop-build-macos "$orchestrator")
relay_e2e_job=$(extract_job relay-e2e "$relay_workflow")
[[ "$desktop_call" == *'uses: ./.github/workflows/_ci-desktop.yml'* ]] ||
fail "Desktop Domain must call _ci-desktop.yml"
@@ -68,6 +69,8 @@ grep -Fq "inputs.lane == 'postgres'" "$relay_workflow" ||
fail "PostgreSQL tests must be selected by their isolated lane"
grep -Fq "inputs.lane == 'artifacts'" "$relay_workflow" ||
fail "relay artifacts must be selected by their isolated lane"
[[ "$relay_e2e_job" == *'cargo test -p buzz-test-client --test e2e_relay nip29_departure_wire -- --ignored --nocapture'* ]] ||
fail "Relay E2E must select the NIP-29 departure wire tests"
grep -Fq "inputs.lane == 'mobile-swift'" "$clients_workflow" ||
fail "Mobile Swift must be selected by its isolated lane"