diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 23961d940c4..a77a28bc581 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -475,7 +475,7 @@ All database access. Uses `sqlx::query()` (runtime, not compile-time macros) — | `error.rs` | Database error types | **Channel types:** `Stream`, `Forum`, `Dm`, `Workflow`, `System` -**Member roles:** `Owner`, `Admin`, `Member`, `Guest`, `Bot` +**Member roles:** `Owner`, `Admin`, `Member`, `Guest`, `Bot` (`Bot` = `Member` permissions; the relay stores `member` and shows `bot` for keys with a recorded agent owner) **Workflow statuses:** `Active`, `Disabled`, `Archived` **Run statuses:** `Pending`, `Running`, `WaitingApproval`, `Completed`, `Failed`, `Cancelled` diff --git a/NOSTR.md b/NOSTR.md index 1450c0cdf33..8eb1c7fbf16 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -101,6 +101,31 @@ relay to specific external Nostr identities without granting full access. SELECT encode(pubkey, 'hex'), added_at, note FROM pubkey_allowlist; ``` +### The bot role + +`bot` is a label, not a permission tier. It has the same permissions as +`member`, and the relay chooses between the two: + +- When an add or role change ends at `member` or `bot`, the relay stores + `member`. It ignores which of the two the client asked for. Asking for the + other one is not a role change. +- The relay shows an ordinary member as `bot` if the key has a recorded + agent owner, and as `member` otherwise, in kind:39002 rosters and every + other place it reports a role. +- An agent owner is recorded once, by the first NIP-OA authentication, and + never changes. When it is recorded, the relay republishes the kind:39002 + rosters of that key's channels so they show `bot`. +- `owner`, `admin` and `guest` are stored and shown as asked. + +Clients follow three rules: + +- **Write:** send `member` for an ordinary member. The relay accepts `bot` + from older clients and stores the same role it would store for `member`. +- **Parse:** read a `bot` role in a kind:39002 roster or kind:9000 as `member` + as soon as it is parsed. Do not keep `bot` as a separate client role. +- **Identity:** never infer that a key is an agent from its channel role. A + key is an agent only when a verified NIP-OA attestation says so. + ### Announce channels Roles have the same permissions in every channel. A channel's settings diff --git a/crates/buzz-core/src/channel.rs b/crates/buzz-core/src/channel.rs index 3e3a7b10904..6d56a61e17a 100644 --- a/crates/buzz-core/src/channel.rs +++ b/crates/buzz-core/src/channel.rs @@ -347,7 +347,13 @@ pub fn decide_member_put(put: MemberPut<'_>) -> Result() + .is_ok_and(|current| current.is_same_role(requested)) + { return Ok(requested); } // A stored role this build does not know is changed only by an elevated @@ -439,7 +445,9 @@ impl FromStr for ChannelType { /// A member's role within a channel. /// /// The hierarchy for permission checks is: Owner > Admin > Member > Guest. -/// Bot is a **separate designation** — it is not part of the linear hierarchy. +/// Bot has the same permissions as Member. The relay stores `member` for +/// both and shows `bot` for a key with a recorded agent owner, so the role +/// only labels an owned agent; it grants and refuses nothing extra. /// Use [`MemberRole::permission_level`] for numeric comparisons in authorization. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum MemberRole { @@ -451,7 +459,8 @@ pub enum MemberRole { Member, /// Read-only external participant. Guest, - /// Automated agent or integration (not in the role hierarchy). + /// A member whose key has a recorded agent owner. Same permissions as + /// [`MemberRole::Member`]; the relay computes it when it reads a role. Bot, } @@ -472,6 +481,19 @@ impl MemberRole { matches!(self, Self::Owner | Self::Admin) } + /// Whether this is the ordinary member role, `member` or `bot`. A client + /// may send either; the relay shows `bot` exactly when the key has a + /// recorded agent owner. + pub fn is_ordinary_member(&self) -> bool { + matches!(self, Self::Member | Self::Bot) + } + + /// Whether `self` and `other` are the same role: equal, or both ordinary. + /// `member` and `bot` are one role shown with two labels. + pub fn is_same_role(self, other: MemberRole) -> bool { + self == other || (self.is_ordinary_member() && other.is_ordinary_member()) + } + /// Whether this role may write in a channel: post, react, edit, change /// topic, add people, push to git. The same in every channel: every role /// except [`MemberRole::Guest`] may, and a bot counts as a member. @@ -504,21 +526,18 @@ impl MemberRole { /// Numeric permission level for authorization comparisons. /// - /// Higher = more privileged. Bot returns 0 (must use explicit grants). + /// Higher = more privileged. Bot has the same level as Member. /// Use `role.permission_level() >= required.permission_level()` for checks. pub fn permission_level(self) -> u8 { match self { Self::Owner => 4, Self::Admin => 3, - Self::Member => 2, + Self::Member | Self::Bot => 2, Self::Guest => 1, - Self::Bot => 0, } } /// Returns true if this role meets or exceeds the required role's permission level. - /// - /// Bot never meets any requirement (returns false for all non-Bot requirements). pub fn has_at_least(self, required: MemberRole) -> bool { self.permission_level() >= required.permission_level() } @@ -759,6 +778,32 @@ mod tests { } } + #[test] + fn bot_has_the_member_permission_level() { + assert_eq!( + MemberRole::Bot.permission_level(), + MemberRole::Member.permission_level() + ); + for required in [ + MemberRole::Owner, + MemberRole::Admin, + MemberRole::Member, + MemberRole::Guest, + MemberRole::Bot, + ] { + assert_eq!( + MemberRole::Bot.has_at_least(required), + MemberRole::Member.has_at_least(required), + "{required}" + ); + } + assert!(MemberRole::Bot.is_ordinary_member()); + assert!(MemberRole::Member.is_ordinary_member()); + for role in [MemberRole::Owner, MemberRole::Admin, MemberRole::Guest] { + assert!(!role.is_ordinary_member(), "{role}"); + } + } + #[test] fn only_guests_cannot_post() { for role in [ diff --git a/crates/buzz-core/src/git_perms.rs b/crates/buzz-core/src/git_perms.rs index 391781163b8..8d440dc7289 100644 --- a/crates/buzz-core/src/git_perms.rs +++ b/crates/buzz-core/src/git_perms.rs @@ -345,8 +345,9 @@ pub fn parse_protection_tag_with_warnings( .parse() .map_err(|_| RuleParseError::InvalidRole(role_str.to_string()))?; // Reject push:bot and push:guest — nonsensical rules. - // Bot is promoted to Member at the policy layer; push:bot is meaningless. - // Guest cannot push regardless; push:guest would be confusing. + // Bot has the member permission level, so push:bot would mean + // push:member. Guest cannot push regardless; push:guest would be + // confusing. if matches!(role, MemberRole::Bot | MemberRole::Guest) { return Err(RuleParseError::InvalidRole(role_str.to_string())); } @@ -931,19 +932,25 @@ mod tests { } #[test] - fn evaluate_bot_cannot_push_without_explicit_grant() { + fn evaluate_bot_pushes_like_a_member() { let rules = vec![]; - let update = RefUpdate { - ref_name: "refs/heads/feature".to_string(), - kind: UpdateKind::FastForward, - old_oid: "a".repeat(40), - new_oid: "b".repeat(40), - }; - // Bot has permission_level 0 at the core evaluator level. - // NOTE: The policy layer (policy.rs) promotes Bot → Member before calling - // evaluate_push, so bots in a channel CAN push in practice. This test - // verifies the raw evaluator behavior; the promotion is tested in policy. - assert!(evaluate_ref_update(&update, MemberRole::Bot, &rules).is_err()); + for (ref_name, kind) in [ + ("refs/heads/feature", UpdateKind::FastForward), + ("refs/heads/feature", UpdateKind::NonFastForward), + ("refs/heads/main", UpdateKind::FastForward), + ] { + let update = RefUpdate { + ref_name: ref_name.to_string(), + kind, + old_oid: "a".repeat(40), + new_oid: "b".repeat(40), + }; + assert_eq!( + evaluate_ref_update(&update, MemberRole::Bot, &rules).is_ok(), + evaluate_ref_update(&update, MemberRole::Member, &rules).is_ok(), + "{ref_name} {kind:?}" + ); + } } #[test] diff --git a/crates/buzz-db/src/store/channel_members.rs b/crates/buzz-db/src/store/channel_members.rs index 4ed7d14dfce..671158c5b51 100644 --- a/crates/buzz-db/src/store/channel_members.rs +++ b/crates/buzz-db/src/store/channel_members.rs @@ -22,7 +22,8 @@ pub struct MemberRecord { pub channel_id: Uuid, /// Compressed public key bytes of the member. pub pubkey: Vec, - /// Role string (e.g. `"owner"`, `"member"`, `"bot"`). + /// Role string (e.g. `"owner"`, `"member"`, `"bot"`), as + /// [`member_role_label`] shows it. pub role: String, /// When the member joined. pub joined_at: DateTime, @@ -32,6 +33,35 @@ pub struct MemberRecord { pub removed_at: Option>, } +/// SQL for the role that a `channel_members` row aliased `cm` shows. +/// +/// `member` and `bot` are one role with the same permissions. The relay +/// stores `member` for both, and an ordinary member shows `bot` exactly when +/// its key has a recorded agent owner in the community. Older relays stored +/// `bot` when a client asked for it, so a stored `bot` follows the same rule. +/// `owner`, `admin` and `guest` show as stored. An agent owner is recorded +/// once and never changes, so the label of a key changes at most once. +/// +/// One exception: the `users` foreign key to the owner is `ON DELETE SET +/// NULL`, so deleting the owner's `users` row clears the agent's owner. Today +/// only community deletion deletes `users` rows, and it removes every row of +/// the community. A user deletion that purges `users` rows must republish the +/// rosters of the deleted owner's agents, or they keep showing `bot`. +/// +/// Every read of a member's role uses this, so rosters, member lists and role +/// checks agree. +macro_rules! member_role_label { + () => { + "(CASE WHEN cm.role IN ('member', 'bot') THEN \ + CASE WHEN EXISTS (SELECT 1 FROM users u_owner \ + WHERE u_owner.community_id = cm.community_id AND u_owner.pubkey = cm.pubkey \ + AND u_owner.agent_owner_pubkey IS NOT NULL) \ + THEN 'bot' ELSE 'member' END \ + ELSE cm.role::text END)" + }; +} +pub(crate) use member_role_label; + /// 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. @@ -85,6 +115,10 @@ pub(crate) async fn acquire_channel_membership_lock( /// /// On failure, DO raises the PostgreSQL error that migration 0032's retired /// trigger raised, so callers see the same SQLSTATE and message. +/// +/// `member` and `bot` count as one role here: the label a roster shows is +/// computed when the roster is captured ([`member_role_label`]), and an agent +/// owner recorded between capture and insert changes only that label. pub(crate) async fn validate_channel_roster_snapshot( tx: &mut AdmittedTx, channel_id: Uuid, @@ -102,14 +136,17 @@ pub(crate) async fn validate_channel_roster_snapshot( NOT IN ('owner', 'admin', 'bot', 'member', 'guest')) ) THEN 1 WHEN - (SELECT COALESCE(array_agg(encode(cm.pubkey, 'hex') || ':' || cm.role::text + (SELECT COALESCE(array_agg(encode(cm.pubkey, 'hex') || ':' || + CASE WHEN cm.role IN ('member', 'bot') THEN 'member' + ELSE cm.role::text END ORDER BY cm.pubkey), ARRAY[]::text[]) FROM channel_members cm WHERE cm.community_id = $1 AND cm.channel_id = $2 AND cm.removed_at IS NULL) IS DISTINCT FROM (SELECT COALESCE(array_agg(lower(roster_tag.tag_json->>1) || ':' || - (roster_tag.tag_json->>3) + CASE WHEN roster_tag.tag_json->>3 IN ('member', 'bot') + THEN 'member' ELSE roster_tag.tag_json->>3 END ORDER BY decode(roster_tag.tag_json->>1, 'hex')), ARRAY[]::text[]) FROM jsonb_array_elements($3::jsonb) AS roster_tag(tag_json) @@ -492,6 +529,31 @@ impl LockedMemberSnapshot { } } +/// Refuse to serve until migration 0081 has retired the 0032 roster trigger. +/// +/// This relay shows the computed `member`/`bot` label in kind:39002 rosters. +/// The 0032 trigger compares roster labels with the stored role exactly, so +/// while it is installed it refuses every roster that shows `bot` for a stored +/// `member`. Membership changes would still commit, but those channels' +/// rosters would stop updating. +pub async fn verify_channel_roster_trigger_retired<'e>( + executor: impl sqlx::PgExecutor<'e>, +) -> Result<()> { + let installed: bool = sqlx::query_scalar( + "SELECT to_regprocedure('public.guard_channel_roster_snapshot()') IS NOT NULL \ + OR EXISTS (SELECT 1 FROM pg_trigger \ + WHERE tgname = 'trg_events_guard_channel_roster_snapshot')", + ) + .fetch_one(executor) + .await?; + if installed { + return Err(DbError::InvalidData( + "the 0032 channel roster trigger is still installed; apply migration 0081".into(), + )); + } + Ok(()) +} + /// Capture all active members while holding the same per-channel lock used by /// membership writers. /// @@ -527,15 +589,17 @@ pub async fn lock_member_snapshot( ) .await?; acquire_channel_membership_lock(tx.conn(), community_id, channel_id).await?; - let rows = sqlx::query( + let rows = sqlx::query(concat!( + "SELECT cm.channel_id, cm.pubkey, ", + member_role_label!(), + " AS role, cm.joined_at, cm.invited_by, cm.removed_at", r#" - SELECT cm.channel_id, cm.pubkey, cm.role::text AS role, cm.joined_at, cm.invited_by, cm.removed_at FROM channel_members cm JOIN channels c ON cm.community_id = c.community_id AND cm.channel_id = c.id AND c.deleted_at IS NULL WHERE cm.community_id = $1 AND cm.channel_id = $2 AND cm.removed_at IS NULL ORDER BY cm.joined_at ASC - "#, - ) + "# + )) .bind(community_id.as_uuid()) .bind(channel_id) .fetch_all(tx.conn()) @@ -857,10 +921,13 @@ async fn put_members_on( if let Some(actor) = invited_by { involved.push(actor.to_vec()); } - let mut roles: std::collections::HashMap, String> = sqlx::query( - "SELECT pubkey, role::text AS role FROM channel_members \ - WHERE community_id = $1 AND channel_id = $2 AND pubkey = ANY($3) AND removed_at IS NULL", - ) + let mut roles: std::collections::HashMap, String> = sqlx::query(concat!( + "SELECT cm.pubkey, ", + member_role_label!(), + " AS role FROM channel_members cm \ + WHERE cm.community_id = $1 AND cm.channel_id = $2 AND cm.pubkey = ANY($3) \ + AND cm.removed_at IS NULL" + )) .bind(community_id.as_uuid()) .bind(channel_id) .bind(&involved) @@ -959,25 +1026,33 @@ async fn put_members_on( .zip(&decided) .filter_map(|(target, decision)| { let (_, role) = decision.as_ref().ok()?; - Some((target.pubkey.to_vec(), role.as_str())) + // Store `member` for both ordinary roles; reads show `bot` for an + // owned agent (see `member_role_label`). + let stored = if role.is_ordinary_member() { + MemberRole::Member + } else { + *role + }; + Some((target.pubkey.to_vec(), stored.as_str())) }) .unzip(); let mut written: std::collections::HashMap, MemberRecord> = std::collections::HashMap::with_capacity(write_keys.len()); if !write_keys.is_empty() { - let rows = sqlx::query( + let rows = sqlx::query(concat!( r#" - INSERT INTO channel_members (community_id, channel_id, pubkey, role, invited_by) + INSERT INTO channel_members AS cm (community_id, channel_id, pubkey, role, invited_by) SELECT $1, $2, t.pubkey, t.role::member_role, $5 FROM UNNEST($3::bytea[], $4::text[]) AS t(pubkey, role) ON CONFLICT (community_id, channel_id, pubkey) DO UPDATE SET - sync_modified_at = CASE WHEN channel_members.removed_at IS NOT NULL THEN clock_timestamp() ELSE channel_members.sync_modified_at END, + sync_modified_at = CASE WHEN cm.removed_at IS NOT NULL THEN clock_timestamp() ELSE cm.sync_modified_at END, removed_at = NULL, removed_by = NULL, role = EXCLUDED.role - RETURNING channel_id, pubkey, role::text AS role, joined_at, invited_by, removed_at - "#, - ) + RETURNING cm.channel_id, cm.pubkey, "#, + member_role_label!(), + " AS role, cm.joined_at, cm.invited_by, cm.removed_at" + )) .bind(community_id.as_uuid()) .bind(channel_id) .bind(&write_keys) @@ -1386,15 +1461,17 @@ async fn get_members_with_operation( operation: crate::observability::WriterOperation, ) -> Result> { let mut connection = crate::observability::acquire_writer(pool, operation).await?; - let rows = sqlx::query( + let rows = sqlx::query(concat!( + "SELECT cm.channel_id, cm.pubkey, ", + member_role_label!(), + " AS role, cm.joined_at, cm.invited_by, cm.removed_at", r#" - SELECT cm.channel_id, cm.pubkey, cm.role::text AS role, cm.joined_at, cm.invited_by, cm.removed_at FROM channel_members cm JOIN channels c ON cm.community_id = c.community_id AND cm.channel_id = c.id AND c.deleted_at IS NULL WHERE cm.community_id = $1 AND cm.channel_id = $2 AND cm.removed_at IS NULL ORDER BY cm.joined_at ASC - "#, - ) + "# + )) .bind(community_id.as_uuid()) .bind(channel_id) .fetch_all(&mut *connection) @@ -1422,15 +1499,17 @@ pub async fn get_members_bulk( crate::observability::WriterOperation::Authorization, ) .await?; - let rows = sqlx::query( + let rows = sqlx::query(concat!( + "SELECT cm.channel_id, cm.pubkey, ", + member_role_label!(), + " AS role, cm.joined_at, cm.invited_by, cm.removed_at", r#" - SELECT cm.channel_id, cm.pubkey, cm.role::text AS role, cm.joined_at, cm.invited_by, cm.removed_at FROM channel_members cm JOIN channels c ON cm.community_id = c.community_id AND cm.channel_id = c.id AND c.deleted_at IS NULL WHERE cm.community_id = $1 AND cm.channel_id = ANY($2) AND cm.removed_at IS NULL ORDER BY cm.joined_at ASC - "#, - ) + "# + )) .bind(community_id.as_uuid()) .bind(channel_ids) .fetch_all(&mut *connection) @@ -1746,6 +1825,33 @@ pub async fn list_large_channel_rosters_needing_reconciliation( .collect() } +/// The channels where `pubkey` is an active ordinary member (`member` or +/// `bot`). Recording an agent owner changes the label these rows show, so +/// their rosters are republished. +pub async fn list_ordinary_member_channels( + pool: &PgPool, + community_id: CommunityId, + pubkey: &[u8], +) -> Result> { + let mut connection = crate::observability::acquire_writer( + pool, + crate::observability::WriterOperation::Authorization, + ) + .await?; + Ok(sqlx::query_scalar( + "SELECT cm.channel_id FROM channel_members cm \ + JOIN channels c ON cm.community_id = c.community_id AND cm.channel_id = c.id \ + AND c.deleted_at IS NULL \ + WHERE cm.community_id = $1 AND cm.pubkey = $2 AND cm.role IN ('member', 'bot') \ + AND cm.removed_at IS NULL \ + ORDER BY cm.channel_id", + ) + .bind(community_id.as_uuid()) + .bind(pubkey) + .fetch_all(&mut *connection) + .await?) +} + /// Transaction-aware variant of [`get_active_role_tx`]. async fn get_active_role_tx( tx: &mut PgConnection, @@ -1753,10 +1859,13 @@ async fn get_active_role_tx( channel_id: Uuid, pubkey: &[u8], ) -> Result> { - let row = sqlx::query( - "SELECT role::text AS role FROM channel_members \ - WHERE community_id = $1 AND channel_id = $2 AND pubkey = $3 AND removed_at IS NULL", - ) + let row = sqlx::query(concat!( + "SELECT ", + member_role_label!(), + " AS role FROM channel_members cm \ + WHERE cm.community_id = $1 AND cm.channel_id = $2 AND cm.pubkey = $3 \ + AND cm.removed_at IS NULL" + )) .bind(community_id.as_uuid()) .bind(channel_id) .bind(pubkey) @@ -1915,7 +2024,8 @@ pub async fn get_accessible_channels( .collect() } -/// Returns all bot-role members with their channel memberships in one community. +/// Returns all members that show the `bot` role (ordinary members with a +/// recorded agent owner) with their channel memberships in one community. /// /// Channels are returned as a JSON array of `{name, id}` objects via `json_agg`, /// preserving the 1:1 name↔UUID pairing. No separate string_agg ordering issues. @@ -1936,7 +2046,8 @@ pub async fn get_bot_members( FROM channel_members cm LEFT JOIN users u ON cm.community_id = u.community_id AND cm.pubkey = u.pubkey JOIN channels c ON cm.community_id = c.community_id AND cm.channel_id = c.id AND c.deleted_at IS NULL - WHERE cm.community_id = $1 AND cm.role = 'bot' AND cm.removed_at IS NULL + WHERE cm.community_id = $1 AND cm.role IN ('member', 'bot') AND cm.removed_at IS NULL + AND u.agent_owner_pubkey IS NOT NULL GROUP BY cm.pubkey, u.display_name, u.agent_type, u.capabilities LIMIT 1000 "#, @@ -2112,11 +2223,13 @@ pub async fn get_member_role( crate::observability::WriterOperation::Authorization, ) .await?; - let row = sqlx::query( - "SELECT cm.role::text AS role FROM channel_members cm \ + let row = sqlx::query(concat!( + "SELECT ", + member_role_label!(), + " AS role FROM channel_members cm \ JOIN channels c ON cm.community_id = c.community_id AND cm.channel_id = c.id AND c.deleted_at IS NULL \ - WHERE cm.community_id = $1 AND cm.channel_id = $2 AND cm.pubkey = $3 AND cm.removed_at IS NULL", - ) + WHERE cm.community_id = $1 AND cm.channel_id = $2 AND cm.pubkey = $3 AND cm.removed_at IS NULL" + )) .bind(community_id.as_uuid()) .bind(channel_id) .bind(pubkey) @@ -2126,6 +2239,18 @@ pub async fn get_member_role( } impl Db { + /// Fail unless migration 0081 has retired the 0032 roster trigger. See + /// [`verify_channel_roster_trigger_retired`]. + #[datastore_span(name = "verify_channel_roster_trigger_retired", system = "postgresql")] + pub async fn verify_channel_roster_trigger_retired(&self) -> Result<()> { + let mut connection = crate::observability::acquire_writer( + &self.pool, + crate::observability::WriterOperation::Bootstrap, + ) + .await?; + verify_channel_roster_trigger_retired(&mut *connection).await + } + /// Capture the active roster while holding the membership-writer lock. #[datastore_span(name = "lock_member_snapshot", system = "postgresql")] pub async fn lock_member_snapshot( @@ -2404,6 +2529,16 @@ impl Db { .await } + /// Returns the channels where `pubkey` is an active ordinary member. + #[datastore_span(name = "list_ordinary_member_channels", system = "postgresql")] + pub async fn list_ordinary_member_channels( + &self, + community_id: CommunityId, + pubkey: &[u8], + ) -> Result> { + list_ordinary_member_channels(&self.pool, community_id, pubkey).await + } + /// Returns full channel records for all channels a user can access. #[datastore_span(name = "get_accessible_channels", system = "postgresql")] pub async fn get_accessible_channels( @@ -2934,7 +3069,7 @@ mod postgres_tests { .expect("insert large roster"); // The roster check requires canonical four-field p tags - // whose roles exactly match channel_members, including the creator's + // whose roles match channel_members, including the creator's // owner row created by create_test_channel. let creator_hex = hex::encode(&creator); let stale_tags: Vec = @@ -3581,7 +3716,8 @@ mod postgres_tests { let role = get_member_role(&pool, community, channel.id, &agent) .await .expect("read role"); - assert_eq!(role.as_deref(), Some("member")); + // The agent has a recorded owner, so its ordinary role shows `bot`. + assert_eq!(role.as_deref(), Some("bot")); } /// `put_members` decides each person as though each were its own write: @@ -4764,6 +4900,34 @@ mod postgres_tests { for role in ["owner", "admin", "bot", "member", "guest"] { cases.push((role, json!([["p", owner_hex, "", role]]), Some(role), None)); } + // `member` and `bot` are one role; which label a roster shows is + // decided at read time, so the check does not compare them. + cases.extend([ + ( + "stored member shown as bot", + json!([["p", owner_hex, "", "bot"]]), + Some("member"), + None, + ), + ( + "stored bot shown as member", + json!([["p", owner_hex, "", "member"]]), + Some("bot"), + None, + ), + ( + "guest shown as member", + json!([["p", owner_hex, "", "member"]]), + Some("guest"), + Some(mismatch), + ), + ( + "member shown as guest", + json!([["p", owner_hex, "", "guest"]]), + Some("member"), + Some(mismatch), + ), + ]); assert_ne!( owner_hex, owner_hex.to_uppercase(), @@ -5465,46 +5629,48 @@ mod postgres_tests { .expect("create channel"); } - /// Migration 0081 retires the 0032 roster trigger. Before it, the trigger - /// and its function are installed; after it, both are gone and the + /// Migration 0081 retires the 0032 roster trigger: after migrating, the /// database accepts a raw kind:39002 insert that disagrees with - /// membership, because only the app-side check guards rosters now. + /// membership, because only the app-side check guards rosters now. The + /// startup check refuses a database where the trigger is still + /// installed and accepts it after 0081. /// - /// Mutation: remove the DROP statements from 0081 → the trigger refuses - /// the insert → RED. + /// Mutations: remove the DROP statements from 0081 → the trigger refuses + /// the insert → RED; make the startup check always pass → the pre-0081 + /// database is accepted → RED. #[tokio::test] #[ignore = "requires Postgres"] - async fn migration_0081_retires_the_roster_trigger() { + async fn migrations_retire_the_roster_trigger() { use nostr::{EventBuilder, Keys, Kind, Tag}; - async fn roster_trigger_installed(pool: &PgPool) -> bool { - sqlx::query_scalar( - "SELECT to_regprocedure('public.guard_channel_roster_snapshot()') IS NOT NULL \ - OR EXISTS (SELECT 1 FROM pg_trigger \ - WHERE tgname = 'trg_events_guard_channel_roster_snapshot')", - ) - .fetch_one(pool) - .await - .expect("look up the roster trigger") - } - let (pool, _scratch) = ScratchDb::migrated_through( &crate::test_support::database_url(), "roster_fence_retired", 80, ) .await; + let db = Db::from_pool(pool.clone()); + let error = db + .verify_channel_roster_trigger_retired() + .await + .expect_err("before 0081 the trigger is installed"); assert!( - roster_trigger_installed(&pool).await, - "before 0081 the trigger is installed" + error.to_string().contains("apply migration 0081"), + "the startup check must name the missing migration: {error}" ); migration::run_migrations(&pool) .await .expect("apply migration 0081"); - assert!( - !roster_trigger_installed(&pool).await, - "0081 must drop the roster trigger and its function" - ); + db.verify_channel_roster_trigger_retired() + .await + .expect("0081 retires the trigger"); + let function: Option = sqlx::query_scalar( + "SELECT to_regprocedure('public.guard_channel_roster_snapshot()')::text", + ) + .fetch_one(&pool) + .await + .expect("look up the roster function"); + assert_eq!(function, None, "0081 must drop the roster function"); let community_uuid = Uuid::new_v4(); let channel = Uuid::new_v4(); @@ -5533,4 +5699,340 @@ mod postgres_tests { .await .expect("without the trigger, a raw roster insert is not checked"); } + + async fn stored_role( + pool: &PgPool, + community_id: Uuid, + channel_id: Uuid, + pubkey: &[u8], + ) -> String { + sqlx::query_scalar( + "SELECT role::text FROM channel_members \ + WHERE community_id = $1 AND channel_id = $2 AND pubkey = $3", + ) + .bind(community_id) + .bind(channel_id) + .bind(pubkey) + .fetch_one(pool) + .await + .expect("read stored role") + } + + async fn open_test_channel(pool: &PgPool, community_id: Uuid, creator: &[u8]) -> Uuid { + create_test_channel( + pool, + community_id, + "ordinary-role", + ChannelType::Stream, + ChannelVisibility::Open, + None, + creator, + None, + ) + .await + .expect("create channel") + .id + } + + /// Every read shows an ordinary member as `bot` exactly when the key has + /// a recorded agent owner, whichever ordinary role a client asked for or + /// an older relay stored. Writes store `member`, so recording the owner + /// changes the label without touching any row. Elevated and guest roles + /// show as stored. + /// + /// Mutations: read the stored role in any of the readers → the agent + /// shows `member` after its owner is recorded → RED; store the requested + /// role → the stored `bot` assertion fails → RED. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn ordinary_member_label_follows_the_recorded_agent_owner() { + let pool = setup_pool().await; + let db = Db::from_pool(pool.clone()); + let community_id = make_test_community(&pool).await; + let community = CommunityId::from_uuid(community_id); + let relay_pubkey = random_pubkey(); + let creator = random_pubkey(); + let owner = random_pubkey(); + let agent = random_pubkey(); + let person = random_pubkey(); + let legacy = random_pubkey(); + for key in [&creator, &owner, &agent, &person, &legacy] { + ensure_user(&pool, community, key) + .await + .expect("ensure user"); + } + let channel = open_test_channel(&pool, community_id, &creator).await; + let guest_channel = open_test_channel(&pool, community_id, &creator).await; + add_member(&pool, community, channel, &agent, MemberRole::Bot, None) + .await + .expect("add unowned agent asking for bot"); + add_member(&pool, community, channel, &person, MemberRole::Member, None) + .await + .expect("add person"); + add_member( + &pool, + community, + guest_channel, + &agent, + MemberRole::Guest, + None, + ) + .await + .expect("add agent as guest"); + // An older relay stored `bot` when a client asked for it. + sqlx::query( + "INSERT INTO channel_members (community_id, channel_id, pubkey, role) \ + VALUES ($1, $2, $3, 'bot')", + ) + .bind(community_id) + .bind(channel) + .bind(&legacy) + .execute(&pool) + .await + .expect("seed legacy bot row"); + + // Every reader of a role, keyed by pubkey. + let labels = |channel: Uuid| { + let (pool, db, relay_pubkey) = (pool.clone(), db.clone(), relay_pubkey.clone()); + async move { + let mut views: Vec, String>> = Vec::new(); + views.push( + get_members(&pool, community, channel) + .await + .expect("get members") + .into_iter() + .map(|m| (m.pubkey, m.role)) + .collect(), + ); + views.push( + get_members_bulk(&pool, community, &[channel]) + .await + .expect("get members bulk") + .into_iter() + .map(|m| (m.pubkey, m.role)) + .collect(), + ); + let snapshot = db + .lock_member_snapshot(community, channel, &relay_pubkey) + .await + .expect("lock member snapshot"); + views.push( + snapshot + .members + .iter() + .map(|m| (m.pubkey.clone(), m.role.clone())) + .collect(), + ); + snapshot.release().await.expect("release snapshot"); + let mut single = std::collections::HashMap::new(); + for key in views[0].keys() { + let role = get_member_role(&pool, community, channel, key) + .await + .expect("get member role") + .expect("active member"); + single.insert(key.clone(), role); + } + views.push(single); + for view in &views[1..] { + assert_eq!(view, &views[0], "every reader shows the same labels"); + } + views.swap_remove(0) + } + }; + let bots = |pool: PgPool| async move { + get_bot_members(&pool, community) + .await + .expect("get bot members") + .into_iter() + .map(|b| b.pubkey) + .collect::>() + }; + + let before = labels(channel).await; + assert_eq!(before[&agent], "member"); + assert_eq!(before[&person], "member"); + assert_eq!( + before[&legacy], "member", + "a stored bot without an owner is a member" + ); + assert_eq!( + stored_role(&pool, community_id, channel, &agent).await, + "member" + ); + assert!(bots(pool.clone()).await.is_empty()); + + set_agent_owner(&pool, community, &agent, &owner) + .await + .expect("record agent owner"); + let after = labels(channel).await; + assert_eq!(after[&agent], "bot"); + assert_eq!(after[&person], "member"); + assert_eq!( + stored_role(&pool, community_id, channel, &agent).await, + "member" + ); + assert_eq!(labels(guest_channel).await[&agent], "guest"); + assert_eq!(bots(pool.clone()).await, vec![agent.clone()]); + + // Asking for the other ordinary role is not a role change: an + // unprivileged self re-add succeeds, changes nothing, and reports the + // computed label before and after. + let put = put_members( + &pool, + community, + channel, + &[MemberPut { + pubkey: &agent, + requested: Some(MemberRole::Member), + }], + Some(&agent), + ) + .await + .expect("re-add owned agent") + .pop() + .expect("one result") + .expect("re-add allowed"); + assert_eq!(put.prior_role.as_deref(), Some("bot")); + assert_eq!(put.member.role, "bot"); + assert_eq!( + stored_role(&pool, community_id, channel, &agent).await, + "member" + ); + } + + /// The roster check treats `member` and `bot` as one role, so an owner + /// recorded between roster capture and insert does not refuse the + /// publication; the background republish then replaces the stale label. + /// + /// Mutation: compare labels exactly in `validate_channel_roster_snapshot` + /// → the captured roster is refused → RED. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn owner_record_during_roster_publication_converges() { + let pool = setup_pool().await; + let db = Db::from_pool(pool.clone()); + let community_id = make_test_community(&pool).await; + let community = CommunityId::from_uuid(community_id); + let relay_keys = Keys::generate(); + let relay_pubkey = relay_keys.public_key().to_bytes(); + let creator = random_pubkey(); + let owner = random_pubkey(); + let agent = random_pubkey(); + for key in [&creator, &owner, &agent] { + ensure_user(&pool, community, key) + .await + .expect("ensure user"); + } + let channel = open_test_channel(&pool, community_id, &creator).await; + add_member(&pool, community, channel, &agent, MemberRole::Member, None) + .await + .expect("add unowned agent"); + let roster = |snapshot: &LockedMemberSnapshot, created_at: u64| { + let tags = + std::iter::once(nostr::Tag::parse(["d", &channel.to_string()]).expect("d tag")) + .chain(snapshot.members.iter().map(|m| { + nostr::Tag::parse(["p", &hex::encode(&m.pubkey), "", &m.role]) + .expect("p tag") + })) + .collect::>(); + nostr::EventBuilder::new(nostr::Kind::Custom(39002), "") + .tags(tags) + .custom_created_at(nostr::Timestamp::from(created_at)) + .sign_with_keys(&relay_keys) + .expect("sign roster") + }; + let agent_label = |tags: &serde_json::Value| { + tags.as_array() + .expect("tags") + .iter() + .find(|tag| tag[0] == "p" && tag[1] == hex::encode(&agent)) + .map(|tag| tag[3].as_str().expect("role").to_owned()) + }; + let live_tags = || async { + sqlx::query_scalar::<_, serde_json::Value>( + "SELECT tags FROM events WHERE community_id = $1 AND channel_id = $2 \ + AND kind = 39002 AND deleted_at IS NULL", + ) + .bind(community_id) + .bind(channel) + .fetch_one(&pool) + .await + .expect("read live roster") + }; + let at = nostr::Timestamp::now().as_secs(); + + let mut snapshot = db + .lock_member_snapshot(community, channel, &relay_pubkey) + .await + .expect("capture roster"); + let captured = roster(&snapshot, at); + // The owner record does not take the membership lock, so it commits + // while the publisher holds it. + tokio::time::timeout( + std::time::Duration::from_secs(5), + set_agent_owner(&pool, community, &agent, &owner), + ) + .await + .expect("owner record must not wait on the membership lock") + .expect("record owner"); + assert!( + snapshot + .replace_member_event(&captured) + .await + .expect("a roster captured before the record is accepted") + .1 + ); + snapshot.release().await.expect("commit captured roster"); + assert_eq!(agent_label(&live_tags().await).as_deref(), Some("member")); + + // The background republish after the owner record. + let mut snapshot = db + .lock_member_snapshot(community, channel, &relay_pubkey) + .await + .expect("capture roster for republish"); + let republished = roster(&snapshot, at + 1); + snapshot + .replace_member_event(&republished) + .await + .expect("republish"); + snapshot.release().await.expect("commit republish"); + assert_eq!(agent_label(&live_tags().await).as_deref(), Some("bot")); + } + + /// The republish after an owner record lists the agent's active ordinary + /// memberships only. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn ordinary_member_channels_list_only_active_ordinary_rows() { + let pool = setup_pool().await; + let community_id = make_test_community(&pool).await; + let community = CommunityId::from_uuid(community_id); + let creator = random_pubkey(); + let agent = random_pubkey(); + for key in [&creator, &agent] { + ensure_user(&pool, community, key) + .await + .expect("ensure user"); + } + let member = open_test_channel(&pool, community_id, &creator).await; + let removed = open_test_channel(&pool, community_id, &creator).await; + let guest = open_test_channel(&pool, community_id, &creator).await; + for channel in [member, removed] { + add_member(&pool, community, channel, &agent, MemberRole::Member, None) + .await + .expect("add agent"); + } + add_member(&pool, community, guest, &agent, MemberRole::Guest, None) + .await + .expect("add agent as guest"); + remove_member(&pool, community, removed, &agent, &agent) + .await + .expect("agent leaves"); + assert_eq!( + list_ordinary_member_channels(&pool, community, &agent) + .await + .expect("list channels"), + vec![member] + ); + } } diff --git a/crates/buzz-db/src/store/dm.rs b/crates/buzz-db/src/store/dm.rs index ebdbc45d271..66b069816eb 100644 --- a/crates/buzz-db/src/store/dm.rs +++ b/crates/buzz-db/src/store/dm.rs @@ -36,7 +36,8 @@ pub struct DmParticipant { pub pubkey: Vec, /// Optional display name from the users table. pub display_name: Option, - /// Member role string (always "member" for DMs). + /// Member role string: `"member"`, or `"bot"` for a key with a recorded + /// agent owner. pub role: String, } @@ -313,9 +314,11 @@ pub async fn list_dms_for_user( let updated_at: DateTime = row.try_get("updated_at")?; // Fetch participants for this DM. - let member_rows = sqlx::query( + let member_rows = sqlx::query(concat!( + "SELECT cm.pubkey, ", + crate::channel_members::member_role_label!(), + " AS role, u.display_name", r#" - SELECT cm.pubkey, cm.role::text AS role, u.display_name FROM channel_members cm LEFT JOIN users u ON u.community_id = cm.community_id @@ -324,8 +327,8 @@ pub async fn list_dms_for_user( AND cm.channel_id = $2 AND cm.removed_at IS NULL ORDER BY cm.joined_at ASC - "#, - ) + "# + )) .bind(community_id.as_uuid()) .bind(channel_id) .fetch_all(pool) diff --git a/crates/buzz-relay/src/api/git/policy.rs b/crates/buzz-relay/src/api/git/policy.rs index 1e376501ab1..0ded3ec7d3d 100644 --- a/crates/buzz-relay/src/api/git/policy.rs +++ b/crates/buzz-relay/src/api/git/policy.rs @@ -7,17 +7,11 @@ //! 2. Resolves kind:30617 → protection rules //! 3. Grants owner authority to the repo key or its verified managed-agent owner //! 4. Otherwise resolves the pusher's channel role via buzz-channel binding -//! 5. Promotes Bot → Member (bots in a channel push as members) -//! 6. Calls `buzz_core::git_perms::evaluate_push()` -//! 7. Returns 200 (allow) or 403 (deny with reasons) +//! 5. Calls `buzz_core::git_perms::evaluate_push()` +//! 6. Returns 200 (allow) or 403 (deny with reasons) //! -//! # Bot Role Model -//! -//! Bots are intentionally added to channels by members/admins. For git push, -//! they're promoted to Member — protection rules still apply. Bot is a -//! designation (what it is), not a permission tier (what it can do). The -//! promotion is scoped to this module; the core `MemberRole::Bot` hierarchy -//! is unchanged. +//! A `bot` channel role pushes as a member: [`MemberRole::Bot`] has the +//! member permission level, and protection rules still apply. //! //! # Security invariants //! @@ -400,15 +394,7 @@ pub async fn hook_policy_check( } }; - // 8. Effective git role: bots intentionally added to a channel push as members. - // Protection rules (push:admin, no-force-push, require-patch, etc.) still apply. - // Bot is a designation (what it is), not a permission tier (what it can do). - let git_role = match role { - MemberRole::Bot => MemberRole::Member, - other => other, - }; - - // 9. Classify ref updates and evaluate policy. + // 8. Classify ref updates and evaluate policy. let updates: Vec = req .ref_updates .iter() @@ -420,7 +406,7 @@ pub async fn hook_policy_check( }) .collect(); - match evaluate_push(&updates, git_role, &rules) { + match evaluate_push(&updates, role, &rules) { Ok(()) => Json(HookCallbackResponse { allowed: true, denials: vec![], diff --git a/crates/buzz-relay/src/api/invites.rs b/crates/buzz-relay/src/api/invites.rs index 3710f5747e8..9c2aaba12ea 100644 --- a/crates/buzz-relay/src/api/invites.rs +++ b/crates/buzz-relay/src/api/invites.rs @@ -415,6 +415,26 @@ async fn mint_invite_inner( }))) } +/// A claim just recorded the claimant's agent owner, after its transaction +/// committed. Do what first AUTH does when it records an owner: update this +/// relay's owner caches, make sockets opened without the owner reconnect with +/// it, and republish the claimant's rosters in the background, because +/// rosters show `bot` for an owned agent. +fn owner_recorded_by_claim( + state: &Arc, + tenant: &buzz_core::TenantContext, + claimant: &nostr::PublicKey, + owner: &nostr::PublicKey, +) { + super::relay_members::remember_agent_owner(state, tenant, claimant, owner); + state.disconnect_unowned_agent_clusterwide(tenant, claimant.as_bytes()); + crate::handlers::side_effects::spawn_agent_roster_republish( + tenant, + Arc::clone(state), + claimant.to_bytes().to_vec(), + ); +} + /// Claim an invite code — `POST /api/invites/claim`, NIP-98 signed by the /// *joining* pubkey. Exempt from the relay-membership gate by design. /// @@ -481,8 +501,8 @@ pub async fn claim_invite( .map_err(map_claim_error)?; metrics::counter!("buzz_users_created_total", "community" => tenant.host().to_owned()) .increment(write.outcome.users_created); - if write.outcome.owner_link_created { - state.disconnect_unowned_agent_clusterwide(&tenant, pubkey.as_bytes()); + if let (true, Some(owner)) = (write.outcome.owner_link_created, verified_owner.as_ref()) { + owner_recorded_by_claim(&state, &tenant, &pubkey, owner); } // The member-added delta and the auto-join only on Joined, never on // other outcomes; the membership-list fan-out on every outcome. @@ -576,8 +596,8 @@ pub async fn claim_invite( .map_err(map_claim_error)?; metrics::counter!("buzz_users_created_total", "community" => tenant.host().to_owned()) .increment(write.outcome.users_created); - if write.outcome.owner_link_created { - state.disconnect_unowned_agent_clusterwide(&tenant, pubkey.as_bytes()); + if let (true, Some(owner)) = (write.outcome.owner_link_created, verified_owner.as_ref()) { + owner_recorded_by_claim(&state, &tenant, &pubkey, owner); } let was_inserted = write.outcome.inserted; finish_committed_claim(&tenant, &state, &claimer_hex, write.snapshot, was_inserted).await; diff --git a/crates/buzz-relay/src/api/invites/postgres_tests/owner_admission_tests.rs b/crates/buzz-relay/src/api/invites/postgres_tests/owner_admission_tests.rs index ee5a490d4f8..38fa98e0376 100644 --- a/crates/buzz-relay/src/api/invites/postgres_tests/owner_admission_tests.rs +++ b/crates/buzz-relay/src/api/invites/postgres_tests/owner_admission_tests.rs @@ -240,4 +240,133 @@ mod postgres_tests { assert_eq!(users(&state, community).await, before); } } + + /// A claim that records the agent's owner makes its ordinary rows show + /// `bot`, so the rosters of channels it is already in must be republished, + /// as on first AUTH. No membership change happens in those channels. The + /// claim also replaces this relay's cached "not owner" answers, which + /// observer frames trust. + /// + /// Mutation: remove the republish or the cache update from + /// `owner_recorded_by_claim`, or call that helper from only one claim + /// format → that format keeps `member` or a cached `false` → RED. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn owner_recorded_by_v1_and_v2_claims_updates_owner_caches_and_republishes_rosters() { + use buzz_core::channel::{ChannelType, ChannelVisibility, MemberRole}; + + let (state, host, community, issuer) = fixture().await; + let v2 = mint_code( + state.clone(), + &host, + &issuer, + serde_json::json!({"max_uses":1}), + ) + .await; + let key = derive_invite_key(&state.relay_keypair); + let (v1, _) = crate::invite_token::mint_invite(&key, community, 3600); + for code in [v1, v2] { + let (owner, agent) = (Keys::generate(), Keys::generate()); + let agent_bytes = agent.public_key().to_bytes(); + state + .db + .ensure_user(community, &agent_bytes) + .await + .expect("seed agent user"); + let mut channels = Vec::new(); + for name in ["claim-roster-a", "claim-roster-b"] { + let channel = state + .db + .create_channel( + community, + name, + ChannelType::Stream, + ChannelVisibility::Open, + None, + &issuer.public_key().to_bytes(), + None, + ) + .await + .expect("create channel") + .id; + state + .db + .add_member( + community, + channel, + &agent_bytes, + MemberRole::Member, + Some(&issuer.public_key().to_bytes()), + ) + .await + .expect("add unowned agent"); + channels.push(channel); + } + + // An owner check before the claim cached "not owner". + let owner_key = ( + community, + agent_bytes.to_vec(), + owner.public_key().to_bytes().to_vec(), + ); + let author_key = (community, agent_bytes.to_vec()); + state.observer_owner_cache.insert(owner_key.clone(), false); + state.author_type_cache.insert(author_key.clone(), false); + + let tag = buzz_sdk::nip_oa::compute_auth_tag(&owner, &agent.public_key(), "").unwrap(); + let response = post_with_owner_tag( + state.clone(), + &host, + "/api/invites/claim", + &agent, + serde_json::json!({"code":code}).to_string(), + &tag, + ) + .await; + assert_eq!(response.status(), StatusCode::OK); + assert!(state + .db + .is_agent_owner( + community, + agent.public_key().as_bytes(), + owner.public_key().as_bytes() + ) + .await + .unwrap()); + assert_eq!(state.observer_owner_cache.get(&owner_key), Some(true)); + assert_eq!(state.author_type_cache.get(&author_key), Some(true)); + + let agent_hex = agent.public_key().to_hex(); + let deadline = std::time::Instant::now() + Duration::from_secs(10); + for channel in channels { + loop { + let live: Option = sqlx::query_scalar( + "SELECT tags FROM events WHERE community_id = $1 AND channel_id = $2 \ + AND kind = 39002 AND deleted_at IS NULL", + ) + .bind(community.as_uuid()) + .bind(channel) + .fetch_optional(state.db.pool()) + .await + .expect("read live roster"); + let shows_bot = + live.as_ref() + .and_then(|tags| tags.as_array()) + .is_some_and(|tags| { + tags.iter().any(|tag| { + tag[0] == "p" && tag[1] == agent_hex.as_str() && tag[3] == "bot" + }) + }); + if shows_bot { + break; + } + assert!( + std::time::Instant::now() < deadline, + "code {code}: the roster must show the claimant as bot: {live:?}" + ); + tokio::time::sleep(Duration::from_millis(50)).await; + } + } + } + } } diff --git a/crates/buzz-relay/src/api/mod.rs b/crates/buzz-relay/src/api/mod.rs index 129d7dd732c..75051a2fdb7 100644 --- a/crates/buzz-relay/src/api/mod.rs +++ b/crates/buzz-relay/src/api/mod.rs @@ -589,14 +589,34 @@ pub mod relay_members { None => write_agent_owner(state, tenant, agent, owner).await, }; if link == OwnerLink::Linked { - state - .author_type_cache - .insert((tenant.community(), agent.to_bytes().to_vec()), true); - state.observer_owner_cache.insert(owner_key, true); + remember_agent_owner(state, tenant, agent, owner); } link } + /// Record a stored agent→owner link in this relay's owner caches. Owner + /// checks cache a miss as `false`, so every path that records an owner + /// calls this; otherwise those checks keep saying "not owner" until the + /// entry expires. + pub fn remember_agent_owner( + state: &AppState, + tenant: &TenantContext, + agent: &nostr::PublicKey, + owner: &nostr::PublicKey, + ) { + state + .author_type_cache + .insert((tenant.community(), agent.to_bytes().to_vec()), true); + state.observer_owner_cache.insert( + ( + tenant.community(), + agent.to_bytes().to_vec(), + owner.to_bytes().to_vec(), + ), + true, + ); + } + /// Record `owner` for an agent that had none when read. The write is /// first-write-wins, so a concurrent writer that got there first is /// judged by the same read. @@ -640,6 +660,15 @@ pub mod relay_members { // without it would only be found by an owner-to-agent lookup // at revoke time; make them reconnect with the owner attached. state.disconnect_unowned_agent_clusterwide(tenant, &agent.to_bytes()); + // Rosters show `bot` for an owned agent, so the agent's + // rosters now show the wrong label. Republish them in the + // background so a contended roster lock cannot hold first + // AUTH past its deadline. + crate::handlers::side_effects::spawn_agent_roster_republish( + tenant, + std::sync::Arc::new(state.clone()), + agent.to_bytes().to_vec(), + ); OwnerLink::Linked } // Another writer stored an owner first. diff --git a/crates/buzz-relay/src/handlers/auth.rs b/crates/buzz-relay/src/handlers/auth.rs index b2bd26efe7a..4536e462a5e 100644 --- a/crates/buzz-relay/src/handlers/auth.rs +++ b/crates/buzz-relay/src/handlers/auth.rs @@ -2637,6 +2637,151 @@ mod tests { assert!(!watcher.cancel.is_cancelled(), "the bystander stays"); } + /// First AUTH with a NIP-OA credential records the owner, which makes + /// the agent's ordinary rows show `bot`. The rosters must follow, but + /// a contended roster lock must not hold AUTH: the pending AUTH + /// expires after 5 s, the same as the default lock timeout. The + /// republish also waits for a relay-wide permit, so a burst of first + /// sign-ins holds only a few writer connections. While every permit + /// is held, the republish waits on the permit before it reaches the + /// roster lock, so AUTH is checked against the whole republish rather + /// than the lock alone. + /// + /// Mutations: await the roster republish inside + /// `materialize_nip_oa_owner` → AUTH waits on the held permits → RED; + /// skip the republish permit → the roster changes while every permit + /// is held → RED. + #[tokio::test] + #[ignore = "requires Postgres — runs in postgres-ci nextest lane"] + async fn first_auth_is_not_held_by_a_contended_roster_lock() { + use buzz_core::channel::{ChannelType, ChannelVisibility, MemberRole}; + use std::sync::Arc; + let state = auth_test_state_real_db_expect().await; + let community = seeded_community(&state).await; + let (creator, owner, agent) = (Keys::generate(), Keys::generate(), Keys::generate()); + for key in [&creator, &agent] { + state + .db + .ensure_user(community, &key.public_key().to_bytes()) + .await + .expect("seed user"); + } + let channel = state + .db + .create_channel( + community, + "roster-lock", + ChannelType::Stream, + ChannelVisibility::Open, + None, + &creator.public_key().to_bytes(), + None, + ) + .await + .expect("create channel") + .id; + state + .db + .add_member( + community, + channel, + &agent.public_key().to_bytes(), + MemberRole::Member, + Some(&creator.public_key().to_bytes()), + ) + .await + .expect("add unowned agent"); + + // Hold every republish permit and the roster lock the republish + // needs. + let permits = Arc::clone(&state.roster_republish_semaphore) + .acquire_many_owned(crate::state::ROSTER_REPUBLISH_CONCURRENCY as u32) + .await + .expect("hold every republish permit"); + let relay_pubkey = state.relay_keypair.public_key().to_bytes(); + let held = state + .db + .lock_member_snapshot(community, channel, &relay_pubkey) + .await + .expect("hold roster lock"); + + let auth_tag = buzz_sdk::nip_oa::compute_auth_tag(&owner, &agent.public_key(), "") + .expect("sign NIP-OA credential"); + let auth_tag: Vec = serde_json::from_str(&auth_tag).expect("tag JSON"); + let (conn, _ctrl) = registered_pending_conn(&state, community, "roster-lock"); + tokio::time::timeout( + std::time::Duration::from_secs(2), + handle_auth( + signed_auth(&agent, "roster-lock", Some(auth_tag)), + Arc::clone(&conn), + Arc::clone(&state), + ), + ) + .await + .expect("AUTH must not wait on the roster lock"); + assert!( + matches!(conn.auth_state_snapshot(), AuthState::Authenticated(_)), + "the agent is admitted while the roster lock is held" + ); + assert!(state + .db + .is_agent_owner( + community, + agent.public_key().as_bytes(), + owner.public_key().as_bytes(), + ) + .await + .expect("read owner")); + + let agent_hex = agent.public_key().to_hex(); + let live_roster = || async { + let live: Option = sqlx::query_scalar( + "SELECT tags FROM events WHERE community_id = $1 AND channel_id = $2 \ + AND kind = 39002 AND deleted_at IS NULL", + ) + .bind(community.as_uuid()) + .bind(channel) + .fetch_optional(state.db.pool()) + .await + .expect("read live roster"); + live + }; + let shows_bot = |live: &Option| { + live.as_ref() + .and_then(|tags| tags.as_array()) + .is_some_and(|tags| { + tags.iter().any(|tag| { + tag[0] == "p" && tag[1] == agent_hex.as_str() && tag[3] == "bot" + }) + }) + }; + + // With the roster lock free but every permit held, the republish + // still waits. + held.release().await.expect("release roster lock"); + let before = live_roster().await; + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + let waiting = live_roster().await; + assert_eq!(waiting, before, "no republish runs without a permit"); + assert!(!shows_bot(&waiting), "{waiting:?}"); + + // Once a permit is free, the background republish names the agent + // `bot` in the live roster. + drop(permits); + let deadline = Instant::now() + std::time::Duration::from_secs(10); + loop { + let live = live_roster().await; + if shows_bot(&live) { + break; + } + assert!( + Instant::now() < deadline, + "the roster must converge to bot after the lock is released: {live:?}" + ); + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + } + /// An untagged AUTH that read a NULL stored owner just before a /// concurrent owner link commits must not end up admitted ownerless: /// either the link's disconnect closes it, or it carries the owner. diff --git a/crates/buzz-relay/src/handlers/channel_authz.rs b/crates/buzz-relay/src/handlers/channel_authz.rs index 1c1c6649924..492217a7e27 100644 --- a/crates/buzz-relay/src/handlers/channel_authz.rs +++ b/crates/buzz-relay/src/handlers/channel_authz.rs @@ -530,6 +530,37 @@ mod tests { Some(MemberRole::Bot), Ok(CheckAddPolicy), ), + // `member` and `bot` are one role: asking for the other one is not + // a role change, so an unprivileged actor's re-add stays allowed. + ( + "open", + &[(1, "owner"), (2, "bot")], + 9, + None, + 2, + Some(Member), + Ok(CheckAddPolicy), + ), + ( + "private", + &[(1, "owner"), (2, "member"), (3, "member")], + 3, + Some(Member), + 2, + Some(Bot), + Ok(CheckAddPolicy), + ), + // A guest is a different role, so promoting one to `bot` is still + // a privileged role change. + ( + "open", + &[(1, "owner"), (2, "guest")], + 9, + None, + 2, + Some(Bot), + Err(E::RoleChangeDenied), + ), // An absent role tag requests no change, so the role-change gate // never fires even for an unprivileged actor. ( diff --git a/crates/buzz-relay/src/handlers/put_user_postgres_tests.rs b/crates/buzz-relay/src/handlers/put_user_postgres_tests.rs index 637f7b42e64..02c06d0d204 100644 --- a/crates/buzz-relay/src/handlers/put_user_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/put_user_postgres_tests.rs @@ -105,14 +105,14 @@ async fn joined_notices( /// One event adds a new person, leaves an existing member alone, takes a /// per-`p` role, and reports a refused agent without refusing the others. /// Mutations: refuse the event on any refused person → RED; read only the -/// first `p` → later people missing → RED; ignore the `p` role → agent is a -/// member, not a bot → RED; notify a no-op re-add → two notices → RED. +/// first `p` → later people missing → RED; ignore the `p` role → the admin +/// is a member → RED; notify a no-op re-add → two notices → RED. #[tokio::test] #[ignore = "requires Postgres"] async fn put_user_adds_each_person_and_reports_each_result() { let state = state().await; let tenant = community(&state, "put-user-batch").await; - let (owner, existing, person, bot, refusing) = ( + let (owner, existing, person, admin, refusing) = ( Keys::generate(), Keys::generate(), Keys::generate(), @@ -131,10 +131,10 @@ async fn put_user_adds_each_person_and_reports_each_result() { .await; refuse_third_party_adds(&state, &tenant, &refusing).await; - let (existing_hex, person_hex, bot_hex, refusing_hex) = ( + let (existing_hex, person_hex, admin_hex, refusing_hex) = ( existing.public_key().to_hex(), person.public_key().to_hex(), - bot.public_key().to_hex(), + admin.public_key().to_hex(), refusing.public_key().to_hex(), ); let result = ingest( @@ -147,7 +147,7 @@ async fn put_user_adds_each_person_and_reports_each_result() { &[&existing_hex], &[&person_hex], &[&refusing_hex], - &[&bot_hex, "bot"], + &[&admin_hex, "admin"], &[&person_hex], ], &[], @@ -160,7 +160,7 @@ async fn put_user_adds_each_person_and_reports_each_result() { assert_eq!( report(&result.message), serde_json::json!({ - "added": [person_hex, bot_hex], + "added": [person_hex, admin_hex], "already_member": [existing_hex], "rejected": [{ "reason": "policy:nobody — this agent has disabled external channel additions", @@ -176,8 +176,10 @@ async fn put_user_adds_each_person_and_reports_each_result() { Some("member") ); assert_eq!( - role_of(&state, &tenant, channel_id, &bot).await.as_deref(), - Some("bot") + role_of(&state, &tenant, channel_id, &admin) + .await + .as_deref(), + Some("admin") ); assert_eq!(role_of(&state, &tenant, channel_id, &refusing).await, None); assert_eq!( @@ -190,7 +192,7 @@ async fn put_user_adds_each_person_and_reports_each_result() { ); let roster = group_state_tags(&state, &tenant, channel_id, 39002).await; - for hex in [&person_hex, &bot_hex] { + for hex in [&person_hex, &admin_hex] { assert!( roster.iter().any(|tag| tag[0] == "p" && &tag[1] == hex), "{roster:?}" @@ -247,6 +249,140 @@ async fn put_user_reports_role_changes_as_updated() { ); } +/// `member` and `bot` are one role. The label is computed from the agent +/// owner record, which does not take the membership lock, so an owner record +/// can commit after `put_members` reads the prior roles and before its write +/// returns the new ones. A re-add of that agent then reads `member` before +/// and `bot` after, and is still reported as `already_member` with no second +/// joined notice. Real role changes in the same event are still `updated`. +/// +/// The test holds a SHARE lock on `channel_members`: the prior-role read +/// passes it and the write waits for it before its statement snapshot. The +/// owner record commits while the write waits. +/// Mutation: compare the labels exactly in `persist_put_user` → the agent is +/// `updated` and gets a second joined notice → RED. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn put_user_re_add_racing_the_first_owner_record_is_already_member() { + let state = state().await; + let tenant = community(&state, "put-user-owner-race").await; + let (owner, agent, agent_owner, admin, guest) = ( + Keys::generate(), + Keys::generate(), + Keys::generate(), + Keys::generate(), + Keys::generate(), + ); + let (agent_hex, admin_hex, guest_hex) = ( + agent.public_key().to_hex(), + admin.public_key().to_hex(), + guest.public_key().to_hex(), + ); + let channel_id = create_channel(&state, &tenant, &owner, &[&["p", &agent_hex]]).await; + let setup = ingest( + &state, + &tenant, + put_user( + &owner, + channel_id, + &[&[&admin_hex, "admin"], &[&guest_hex, "guest"]], + &[], + ), + &owner, + ) + .await + .expect("set up admin and guest"); + assert!(setup.accepted, "{}", setup.message); + for key in [&agent, &agent_owner] { + state + .db + .ensure_user(tenant.community(), &key.public_key().to_bytes()) + .await + .expect("ensure user"); + } + + let mut lock = state.db.pool().begin().await.expect("begin lock"); + sqlx::query("LOCK TABLE channel_members IN SHARE MODE") + .execute(&mut *lock) + .await + .expect("lock channel_members"); + let re_add = tokio::spawn({ + let (state, tenant, owner) = (Arc::clone(&state), tenant.clone(), owner.clone()); + let event = put_user( + &owner, + channel_id, + &[ + &[&agent_hex], + &[&admin_hex, "member"], + &[&guest_hex, "member"], + ], + &[], + ); + async move { ingest(&state, &tenant, event, &owner).await } + }); + + // Wait until the transaction that holds this channel's membership lock + // (`acquire_channel_membership_lock`) is waiting to write + // `channel_members`: its prior-role read has run. + let lock_name = format!( + "buzz_channel_membership:{}:{channel_id}", + tenant.community().as_uuid() + ); + tokio::time::timeout(std::time::Duration::from_secs(10), async { + loop { + let waiting: bool = sqlx::query_scalar( + "SELECT EXISTS ( \ + SELECT 1 FROM pg_locks held \ + JOIN pg_locks waiting ON waiting.pid = held.pid \ + WHERE held.locktype = 'advisory' AND held.granted AND held.objsubid = 1 \ + AND held.classid::bigint = (k.key >> 32) & 4294967295 \ + AND held.objid::bigint = k.key & 4294967295 \ + AND waiting.locktype = 'relation' AND NOT waiting.granted \ + AND waiting.relation = 'channel_members'::regclass \ + ) FROM (SELECT hashtextextended($1, 0) AS key) k", + ) + .bind(&lock_name) + .fetch_one(state.db.pool()) + .await + .expect("inspect the membership write"); + if waiting { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .expect("the membership write reached the table lock"); + + assert!(state + .db + .set_agent_owner( + tenant.community(), + &agent.public_key().to_bytes(), + &agent_owner.public_key().to_bytes(), + ) + .await + .expect("record owner")); + lock.rollback().await.expect("release lock"); + + let result = re_add.await.expect("join re-add").expect("re-add"); + assert!(result.accepted, "{}", result.message); + assert_eq!( + report(&result.message), + serde_json::json!({ + "updated": [admin_hex, guest_hex], + "already_member": [agent_hex], + }) + ); + assert_eq!( + role_of(&state, &tenant, channel_id, &agent) + .await + .as_deref(), + Some("bot") + ); + assert_eq!(joined_notices(&state, &tenant, channel_id, &agent).await, 1); +} + /// When nobody named may be added the event is refused: one person keeps the /// single-add error, several get the grouped report. A malformed `p` refuses /// the whole event. diff --git a/crates/buzz-relay/src/handlers/relay_admin.rs b/crates/buzz-relay/src/handlers/relay_admin.rs index 83c9138e1fd..19868f6da68 100644 --- a/crates/buzz-relay/src/handlers/relay_admin.rs +++ b/crates/buzz-relay/src/handlers/relay_admin.rs @@ -1131,9 +1131,11 @@ mod postgres_tests { ) .await .expect("agent owner"); + // The owned agent joins as a member; its recorded owner makes the + // role read as `bot`. for (agent, expected) in [ (&refusing, [None, None]), - (&owned, [Some("member".to_string()), None]), + (&owned, [Some("bot".to_string()), None]), ] { let add = EventBuilder::new(Kind::Custom(RELAY_ADMIN_ADD_MEMBER as u16), "") .tags(vec![Tag::public_key(agent.public_key())]) diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index a10b74d04e6..d55a02b748b 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -1856,6 +1856,17 @@ impl PutUserReport { } } +/// Whether a re-add left a member's role unchanged. `member` and `bot` are +/// one role: both labels are computed from the agent owner record, and the +/// first owner record can commit between the prior-role read and the write, +/// so the same row can read `member` and then `bot`. +fn member_role_unchanged(prior: &str, current: &str) -> bool { + match (prior.parse::(), current.parse::()) { + (Ok(prior), Ok(current)) => prior.is_same_role(current), + _ => prior == current, + } +} + /// Store a validated kind:9000 and apply it, returning the stored event, /// whether it was new, and its OK message. /// @@ -1956,7 +1967,9 @@ pub(crate) async fn persist_put_user( // A no-op re-add posts no second "joined" notice, but still sends // the member-added notification, which re-signals an agent that // missed the first one. - Some(prior) if prior == put.member.role => report.already_member.push(target_hex), + Some(prior) if member_role_unchanged(&prior, &put.member.role) => { + report.already_member.push(target_hex) + } prior => { if let Err(e) = emit_system_message( tenant, @@ -4069,50 +4082,105 @@ pub async fn reconcile_large_channel_member_snapshots( &relay_pubkey.to_bytes(), ) .await?; + let summary = republish_roster_candidates( + state, + candidates + .into_iter() + .map(|candidate| (candidate.community_id, candidate.host, candidate.channel_id)) + .collect(), + "large channel roster reconciliation failed", + ) + .await; + metrics::counter!("buzz_channel_roster_reconciliations_total") + .increment(summary.repaired as u64); + Ok(summary) +} + +/// Republish each `(community, host, channel)` roster under its membership +/// lock, counting failures instead of stopping. Each roster first takes a +/// relay-wide republish permit, so concurrent callers share a small bound on +/// the writer connections they hold. +async fn republish_roster_candidates( + state: &Arc, + candidates: Vec<(buzz_core::CommunityId, String, uuid::Uuid)>, + failure_message: &'static str, +) -> ReconcileSummary { + let relay_pubkey = state.relay_keypair.public_key(); let relay_pubkey_hex = relay_pubkey.to_hex(); let mut reconciled = 0usize; let mut failed = 0usize; - for candidate in candidates { + for (community_id, host, channel_id) in candidates { let result = async { - let channel_id = candidate.channel_id; + let _permit = Arc::clone(&state.roster_republish_semaphore) + .acquire_owned() + .await?; // Hold the membership-writer lock from roster capture through // replacement. Otherwise a rolling deployment can publish stale // roster A after another relay commits and publishes roster B. let mut member_snapshot = state .db - .lock_member_snapshot(candidate.community_id, channel_id, &relay_pubkey.to_bytes()) + .lock_member_snapshot(community_id, channel_id, &relay_pubkey.to_bytes()) .await?; - let tenant = TenantContext::resolved(candidate.community_id, candidate.host.clone()); + let tenant = TenantContext::resolved(community_id, host.clone()); let stored_members = store_group_members_event(state, &mut member_snapshot).await?; member_snapshot.release().await?; dispatch_group_members_event(&tenant, state, stored_members, &relay_pubkey_hex).await; - Ok::(true) + Ok::<(), anyhow::Error>(()) } .await; match result { - Ok(true) => reconciled += 1, - Ok(false) => {} + Ok(()) => reconciled += 1, Err(error) => { failed += 1; metrics::counter!("buzz_channel_roster_reconciliation_failures_total").increment(1); - warn!( - community_id = %candidate.community_id, - host = %candidate.host, - channel_id = %candidate.channel_id, - %error, - "large channel roster reconciliation failed" - ); + warn!(%community_id, %host, %channel_id, %error, "{failure_message}"); } } } - metrics::counter!("buzz_channel_roster_reconciliations_total").increment(reconciled as u64); - Ok(ReconcileSummary { + ReconcileSummary { repaired: reconciled, failed, - }) + } +} + +/// Republish, in the background, the kind:39002 roster of every channel +/// where `agent` is an active ordinary member. Call it after recording the +/// agent's owner: rosters show `bot` for an owned agent, so each of those +/// rosters now shows the wrong label. +/// +/// The channels are listed inside the task, after the owner record has +/// committed, so a channel the agent joins concurrently is either listed here +/// or published by its own join with the new label. Each roster is captured +/// and replaced under the channel's membership lock, like every other roster +/// writer. That lock can wait up to the lock timeout, so the caller does not +/// wait: first AUTH must finish inside its own deadline. A failure is logged +/// and counted and does not stop the other channels. A roster left stale (for +/// example by a relay that stopped first) shows only the wrong label, which +/// has the same permissions; the next membership change repairs it. +pub fn spawn_agent_roster_republish(tenant: &TenantContext, state: Arc, agent: Vec) { + let (community_id, host) = (tenant.community(), tenant.host().to_owned()); + tokio::spawn(async move { + let channels = match state + .db + .list_ordinary_member_channels(community_id, &agent) + .await + { + Ok(channels) => channels, + Err(error) => { + metrics::counter!("buzz_channel_roster_reconciliation_failures_total").increment(1); + warn!(%community_id, %error, "listing an agent's channels for roster republish failed"); + return; + } + }; + let candidates = channels + .into_iter() + .map(|channel_id| (community_id, host.clone(), channel_id)) + .collect(); + republish_roster_candidates(&state, candidates, "channel roster republish failed").await; + }); } /// Reconcile channels that exist in the DB but don't have kind:39000 events. @@ -4546,6 +4614,27 @@ pub async fn publish_nipia_unarchived( mod tests { use super::*; + /// Mutation: compare the labels exactly → `member` then `bot` is + /// reported as a change → RED. + #[test] + fn re_add_treats_member_and_bot_as_one_role() { + assert!(member_role_unchanged("member", "bot")); + assert!(member_role_unchanged("bot", "member")); + assert!(member_role_unchanged("admin", "admin")); + for (prior, current) in [ + ("guest", "member"), + ("guest", "bot"), + ("member", "admin"), + ("bot", "owner"), + ("admin", "member"), + ] { + assert!( + !member_role_unchanged(prior, current), + "{prior} -> {current}" + ); + } + } + fn create_with(creator: &nostr::Keys, tags: Vec>) -> Event { nostr::EventBuilder::new(nostr::Kind::Custom(9007), "") .tags( diff --git a/crates/buzz-relay/src/main.rs b/crates/buzz-relay/src/main.rs index f48926f72d2..1da3edf14d7 100644 --- a/crates/buzz-relay/src/main.rs +++ b/crates/buzz-relay/src/main.rs @@ -1119,6 +1119,18 @@ async fn run_relay_main(boot: BootTracker) -> anyhow::Result<()> { ); } + // This relay shows computed `member`/`bot` roster labels, which the 0032 + // trigger refuses. Fail closed until migration 0081 has dropped it, so a + // skipped migration stops the rollout instead of freezing rosters. + let step = StepTimer::start(StartupStep::ChannelRosterTriggerRetired); + if let Err(error) = state.db.verify_channel_roster_trigger_retired().await { + error!(%error, "Channel roster trigger check failed"); + return Err(anyhow::anyhow!( + "Channel roster trigger check failed; apply migration 0081 before starting this relay: {error}" + )); + } + step.finish(); + // Repair legacy NIP-29 channel rosters that were persisted while the // canonical member query still truncated at 1,000 rows. let mut step = StepTimer::start(StartupStep::LargeRosterReconcile); diff --git a/crates/buzz-relay/src/startup_steps.rs b/crates/buzz-relay/src/startup_steps.rs index dc6a6a5c28d..65af22ab4fb 100644 --- a/crates/buzz-relay/src/startup_steps.rs +++ b/crates/buzz-relay/src/startup_steps.rs @@ -73,6 +73,8 @@ pub enum StartupStep { MeshInitialReconcile, /// Run the git object-store conformance probe (when enabled). GitConformanceProbe, + /// Verify that migration 0081 retired the 0032 roster trigger. + ChannelRosterTriggerRetired, /// Repair large NIP-29 channel roster snapshots. LargeRosterReconcile, /// Bind the private health listener. @@ -99,6 +101,7 @@ impl StartupStep { Self::MeshPublish, Self::MeshInitialReconcile, Self::GitConformanceProbe, + Self::ChannelRosterTriggerRetired, Self::LargeRosterReconcile, Self::HealthBind, Self::ListenerBind, @@ -122,6 +125,7 @@ impl StartupStep { Self::MeshPublish => "mesh_publish", Self::MeshInitialReconcile => "mesh_initial_reconcile", Self::GitConformanceProbe => "git_conformance_probe", + Self::ChannelRosterTriggerRetired => "channel_roster_trigger_retired", Self::LargeRosterReconcile => "large_roster_reconcile", Self::HealthBind => "health_bind", Self::ListenerBind => "listener_bind", diff --git a/crates/buzz-relay/src/state.rs b/crates/buzz-relay/src/state.rs index c37c9e3d281..e54084d8281 100644 --- a/crates/buzz-relay/src/state.rs +++ b/crates/buzz-relay/src/state.rs @@ -1223,6 +1223,10 @@ pub const THREAD_SUMMARY_REFRESH_INTERVAL: std::time::Duration = std::time::Dura /// local fan-out). A hung refresh is abandoned so its root keeps emitting. pub const THREAD_SUMMARY_REFRESH_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); +/// Background roster republishes that may run at once. See +/// [`AppState::roster_republish_semaphore`]. +pub const ROSTER_REPUBLISH_CONCURRENCY: usize = 2; + /// Shared application state, cloned cheaply via inner `Arc` fields. #[derive(Clone)] pub struct AppState { @@ -1265,6 +1269,11 @@ pub struct AppState { pub git_semaphore: Arc, /// Semaphore limiting concurrent media upload parsing/transcoding work. pub media_upload_semaphore: Arc, + /// Semaphore limiting background roster republishes across the relay. + /// Each one holds a writer connection while it waits for a channel's + /// membership lock, so a burst of first agent sign-ins sharing channels + /// must not take more than a few pool connections. + pub roster_republish_semaphore: Arc, /// Workflow engine for background processing. pub workflow_engine: Arc, @@ -1590,6 +1599,7 @@ impl AppState { handler_semaphore: Arc::new(Semaphore::new(max_concurrent_handlers)), git_semaphore: Arc::new(Semaphore::new(git_max_concurrent_ops)), media_upload_semaphore: Arc::new(Semaphore::new(media_max_concurrent_uploads)), + roster_republish_semaphore: Arc::new(Semaphore::new(ROSTER_REPUBLISH_CONCURRENCY)), workflow_engine, relay_keypair, huddle_liveness_generation: Uuid::new_v4(), diff --git a/crates/buzz-test-client/tests/e2e_relay.rs b/crates/buzz-test-client/tests/e2e_relay.rs index 39d7d74a6db..b8bf65a94f6 100644 --- a/crates/buzz-test-client/tests/e2e_relay.rs +++ b/crates/buzz-test-client/tests/e2e_relay.rs @@ -2824,10 +2824,17 @@ async fn test_private_channel_any_member_can_invite() { accepted, "private-channel {actor_role} should add {target_role}, got: {msg}" ); + // The target has no recorded agent owner, so the relay shows + // `member` when `bot` is asked for. + let stored_role = if target_role == "bot" { + "member" + } else { + target_role + }; assert_eq!( member_role(&url, &owner_keys, &channel_id, &target_pubkey_hex).await, - Some(target_role.to_string()), - "private-channel {actor_role} add must persist the {target_role} role" + Some(stored_role.to_string()), + "private-channel {actor_role} add of {target_role} must show {stored_role}" ); } @@ -4043,11 +4050,12 @@ async fn test_nip29_member_raises_guest_agent() { .await; assert!(accepted, "a member raises a guest agent: {msg}"); member_client.disconnect().await.ok(); + // The agent has a recorded owner, so the relay shows its member role as `bot`. assert_eq!( member_role(&url, &owner, &channel_id, &agent_hex) .await .as_deref(), - Some("member") + Some("bot") ); let (accepted, msg) = post_ws(&url, &channel_id, &agent, "agent post").await; assert!(accepted, "raised agent post: {msg}"); diff --git a/deploy/charts/buzz/README.md b/deploy/charts/buzz/README.md index 4c664dd2db6..1e01aa492de 100644 --- a/deploy/charts/buzz/README.md +++ b/deploy/charts/buzz/README.md @@ -616,7 +616,7 @@ default so long-lived WebSocket connections have time to drain. Schema migrations are embedded in the relay binary via `sqlx::migrate!` and run at startup, gated by `BUZZ_AUTO_MIGRATE` (default `true`). Multiple replicas race-safely behind a Postgres advisory lock. `helm upgrade` is the entire upgrade procedure. -Migration 0032 is a hard compatibility boundary for relay versions that publish repaired channel rosters. Older relays verify its roster-fence trigger before opening listeners and refuse to start if the trigger is missing or inert. Newer relays run the same check in every roster writer and do not require the trigger. A later release drops it, and that release requires every relay to run a version without the startup check first. Apply migrations before rolling the relay; for large installations, prefer a controlled `buzz-admin migrate` job with PostgreSQL lock monitoring before the code rollout. +Migration 0032 is a hard compatibility boundary for relay versions that publish repaired channel rosters. Older relays verify its roster-fence trigger before opening listeners and refuse to start if the trigger is missing or inert. Newer relays run the same check in every roster writer and do not require the trigger. Migration 0081 drops the trigger, so every relay must run a version without the startup check (#8611 or later) before 0081 is applied; an older relay then refuses to start. Relays from the 0081 release on show a computed `member`/`bot` label in rosters that the trigger would refuse, so they refuse to start while the trigger is installed. Apply 0081 before rolling them out. A roster that an older relay published keeps its stored label until the channel's next membership change republishes it. Both labels have the same permissions. Apply migrations before rolling the relay; for large installations, prefer a controlled `buzz-admin migrate` job with PostgreSQL lock monitoring before the code rollout. If you prefer decoupling migrations from serving, set `migrate.autoMigrate=false`. **In that mode the chart does not run migrations for you** — you own running `buzz-admin migrate` (separate Pod / one-shot Job) against the database before every `helm install` / `helm upgrade`. Readiness probes only verify DB connectivity, not schema freshness, so a pod will appear healthy against an unmigrated schema and fail under load. A pre-upgrade Helm Job for this is on the chart roadmap; the values knob `migrate.preUpgradeJob.enabled` is reserved. diff --git a/examples/countdown-bot/README.md b/examples/countdown-bot/README.md index 09b3c135200..52c5bd86976 100644 --- a/examples/countdown-bot/README.md +++ b/examples/countdown-bot/README.md @@ -16,7 +16,9 @@ profile, subscribe to events, and publish kind `9` channel messages can be a bot On startup it publishes a profile named **Countdown Bot** with a small embedded SVG clock icon, then best-effort publishes a NIP-29 `kind:9000` self-add with `role=bot`. That channel membership is what makes the bot show up in the -members list and in Buzz's mention autocomplete. +members list and in Buzz's mention autocomplete. The relay decides the role it +shows: `bot` when the key has a recorded agent owner (the owner-attested path +below), otherwise `member`. Both roles have the same permissions. ## Auth paths @@ -76,7 +78,8 @@ Relay requirements for this path: Relay access and channel access are separate. Owner-attested auth can admit the bot to the relay, but the bot still publishes as its own pubkey. The bot tries to -self-add to open channels as a `bot` member on startup. For private channels, +self-add to open channels on startup; with a recorded owner the relay shows it +as a `bot` member. For private channels, an owner/admin must add the bot pubkey to the channel membership before expecting it to appear in members, resolve in mention autocomplete, or read/write messages.