From 6b3778929f100cbae441bfdda5ac934ec5e62959 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Fri, 30 Oct 2026 13:21:05 -0400 Subject: [PATCH 01/13] fix(relay): refuse a second listed open channel with the same name Two open channels in a community could share a name, so the channel directory showed two "square-ios" entries with nothing to tell them apart. The relay now refuses a create, rename, private-to-open switch or unlisted-to-listed switch that would give a listed channel (an open, not unlisted stream or forum that is not deleted) a name another listed channel in the community already has, ignoring case. An archived channel keeps its name. Private channels, DMs and other types may still repeat names: nobody browses them side by side, and refusing a name a private channel holds would reveal that the channel exists. The check runs in the same transaction as the write, after it, under a per-name advisory lock, so two writers claiming one name take turns. Kind 9007 and 9002 report it as "invalid: an open channel named ... already exists". The store also reports a write that the follow-up unique index idx_channels_listed_name refuses as the same error, so this release stays correct beside that index during its rollout and after a rollback. scripts/maintenance/rename_duplicate_listed_channel_names.sql renames the duplicates that already exist. It locks a batch's rows before any name lock, checks each locked row again, and gives it a name that no listed channel has. `buzz-admin reconcile-channels --channel --metadata` republishes a renamed channel's kind:39000 so clients see the new name. A live edit can race it, as it can race the relay's own publisher, so it runs with no relay serving. It signs later than the stored event, as the relay does, and fails unless the stored event matches the channel row after its write. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- crates/buzz-admin/src/main.rs | 98 ++++++- .../tests/postgres_reconcile_metadata.rs | 111 +++++++ crates/buzz-db/src/error.rs | 5 + crates/buzz-db/src/runtime/tests.rs | 3 + .../listed_channel_name_postgres_tests.rs | 226 +++++++++++++++ crates/buzz-db/src/store/channel.rs | 182 +++++++++++- .../src/store/channel_name_postgres_tests.rs | 270 ++++++++++++++++++ crates/buzz-db/src/store/mod.rs | 3 + .../handlers/group_state_postgres_tests.rs | 94 ++++++ crates/buzz-relay/src/handlers/ingest.rs | 11 +- .../rename_duplicate_listed_channel_names.sql | 161 +++++++++++ 11 files changed, 1148 insertions(+), 16 deletions(-) create mode 100644 crates/buzz-admin/tests/postgres_reconcile_metadata.rs create mode 100644 crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs create mode 100644 crates/buzz-db/src/store/channel_name_postgres_tests.rs create mode 100644 scripts/maintenance/rename_duplicate_listed_channel_names.sql diff --git a/crates/buzz-admin/src/main.rs b/crates/buzz-admin/src/main.rs index b286037ac1d..47820818217 100644 --- a/crates/buzz-admin/src/main.rs +++ b/crates/buzz-admin/src/main.rs @@ -139,12 +139,18 @@ enum Command { /// /// Without `--channel`, only channels missing discovery metadata are /// reconciled. With `--channel`, only that channel's member snapshot is - /// replaced; canonical metadata and admin events remain untouched. + /// replaced; canonical metadata and admin events remain untouched unless + /// `--metadata` is given. ReconcileChannels { /// Optional channel UUID to force-republish. #[arg(long)] channel: Option, + /// With `--channel`, also republish its kind:39000 metadata from the + /// channel row, for example after an operator rename. + #[arg(long, requires = "channel")] + metadata: bool, + /// Relay private key (hex) for signing events. Falls back to /// BUZZ_RELAY_PRIVATE_KEY env var. If neither is set, generates /// an ephemeral key (events will be unverifiable after restart). @@ -213,8 +219,12 @@ async fn run(cli: Cli) -> Result { Command::Deletions { command } => deletions::run(command).await, Command::UserDeletions { command } => user_deletions::run(command).await, Command::NipFi { command } => nip_fi::run(command).await, - Command::ReconcileChannels { channel, relay_key } => { - reconcile_channels(channel, relay_key).await?; + Command::ReconcileChannels { + channel, + metadata, + relay_key, + } => { + reconcile_channels(channel, metadata, relay_key).await?; Ok(0) } } @@ -723,6 +733,7 @@ async fn resolve_admin_tenant(db: &Db) -> Result { async fn reconcile_channels( channel_arg: Option, + republish_metadata: bool, relay_key_arg: Option, ) -> Result<()> { use buzz_core::channel::group_state_identity_tags; @@ -802,11 +813,16 @@ async fn reconcile_channels( let members = db.get_members(tenant.community(), channel.id).await?; - // A targeted repair is deliberately roster-only. kind:39000 metadata - // is richer than this legacy backfill builder, and kind:39001 is not - // part of the stale-roster incident; replacing either can destroy - // canonical state. Full backfill still creates all three event kinds + // A targeted repair is roster-only unless `--metadata` asks for + // kind:39000 too; kind:39001 is never replaced by a targeted repair. + // kind:39000 is built from the channel row by the same function the + // relay uses, so an operator rename (for example + // scripts/maintenance/rename_duplicate_listed_channel_names.sql) + // reaches clients. Full backfill still creates all three event kinds // for channels with no discovery metadata. + if republish_metadata { + republish_channel_metadata(&db, &tenant, channel.id, &relay_keys).await?; + } if target_channel.is_none() { // kind:39000 — channel metadata { @@ -876,6 +892,74 @@ async fn reconcile_channels( Ok(()) } +/// Makes the newest relay-signed kind:39000 of a channel match its row, for +/// example after an operator rename. +/// +/// Run it with no relay serving: the relay builds kind:39000 from a channel +/// read outside the replacement (`emit_group_discovery_events`), so a live +/// edit between this command's read and write could leave the older +/// snapshot. The command signs later than the stored event, as the relay +/// does, so it also replaces an event from the same second. It fails unless +/// a read after its write finds the stored event matching the row. +async fn republish_channel_metadata( + db: &Db, + tenant: &TenantContext, + channel_id: uuid::Uuid, + relay_keys: &Keys, +) -> Result<()> { + let latest = || async { + let stored = db + .query_events(&buzz_db::event::EventQuery { + kinds: Some(vec![39000]), + pubkey: Some(relay_keys.public_key().to_bytes().to_vec()), + d_tag: Some(channel_id.to_string()), + limit: Some(1), + ..buzz_db::event::EventQuery::for_community(tenant.community()) + }) + .await?; + Ok::<_, anyhow::Error>(stored.into_iter().next().map(|stored| stored.event)) + }; + let row_tags = || async { + let channel = db.get_channel(tenant.community(), channel_id).await?; + let members = db.get_members(tenant.community(), channel_id).await?; + Ok::<_, anyhow::Error>(buzz_db::channel::group_metadata_tags(&channel, &members)) + }; + let matches = |event: &nostr::Event, tags: &[Vec]| { + event + .tags + .iter() + .map(|tag| tag.as_slice()) + .eq(tags.iter().map(Vec::as_slice)) + }; + + let tags = row_tags().await?; + let stored = latest().await?; + if stored.as_ref().is_some_and(|event| matches(event, &tags)) { + return Ok(()); + } + let created_at = stored + .map_or(0, |event| event.created_at.as_secs() + 1) + .max(nostr::Timestamp::now().as_secs()); + let event = EventBuilder::new(Kind::Custom(39000), "") + .tags( + tags.into_iter() + .map(Tag::parse) + .collect::, _>>()?, + ) + .custom_created_at(nostr::Timestamp::from(created_at)) + .sign_with_keys(relay_keys) + .map_err(|e| anyhow::anyhow!("sign kind:39000: {e}"))?; + db.replace_addressable_event(tenant.community(), &event, Some(channel_id)) + .await?; + let tags = row_tags().await?; + if !latest().await?.is_some_and(|event| matches(&event, &tags)) { + return Err(anyhow::anyhow!( + "channel {channel_id} changed while its kind:39000 was republished; stop every relay and run the command again" + )); + } + Ok(()) +} + #[cfg(test)] mod storage_snapshot_tests { use std::sync::atomic::{AtomicUsize, Ordering}; diff --git a/crates/buzz-admin/tests/postgres_reconcile_metadata.rs b/crates/buzz-admin/tests/postgres_reconcile_metadata.rs new file mode 100644 index 00000000000..2a05101c8db --- /dev/null +++ b/crates/buzz-admin/tests/postgres_reconcile_metadata.rs @@ -0,0 +1,111 @@ +//! `buzz-admin reconcile-channels --channel --metadata` leaves the newest +//! relay-signed kind:39000 matching the channel row, also when an older +//! snapshot of the row holds the replacement with a newer timestamp. + +use buzz_core::CommunityId; +use buzz_db::event::EventQuery; +use buzz_db::{Db, DbConfig}; +use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; +use tokio::process::Command; +use uuid::Uuid; + +fn admin_binary() -> std::ffi::OsString { + // Nextest relocates binaries when extracting an archive. Cargo's build-time + // path remains the fallback for local cargo test runs. + std::env::var_os("NEXTEST_BIN_EXE_buzz_admin") + .unwrap_or_else(|| env!("CARGO_BIN_EXE_buzz-admin").into()) +} + +fn name_tag(event: &nostr::Event) -> Option<&str> { + event.tags.iter().find_map(|tag| match tag.as_slice() { + [key, value, ..] if key == "name" => Some(value.as_str()), + _ => None, + }) +} + +#[tokio::test] +#[ignore = "requires Postgres"] +async fn metadata_republish_replaces_a_stale_snapshot_that_holds_the_replacement() { + let url = std::env::var("BUZZ_TEST_DATABASE_URL") + .expect("run through scripts/postgres-test-run.sh for an isolated database"); + let db = Db::new(&DbConfig { + database_url: url.clone(), + ..DbConfig::default() + }) + .await + .expect("connect to test DB"); + let community = Uuid::new_v4(); + let host = format!("reconcile-{}.example", community.simple()); + sqlx::query("INSERT INTO communities (id, host) VALUES ($1, $2)") + .bind(community) + .bind(&host) + .execute(db.pool()) + .await + .expect("insert community"); + let community = CommunityId::from_uuid(community); + let channel = Uuid::new_v4(); + sqlx::query( + "INSERT INTO channels (id, community_id, name, channel_type, visibility, created_by) \ + VALUES ($1, $2, 'square-ios-79f8e9d4', 'stream', 'open', $3)", + ) + .bind(channel) + .bind(community.as_uuid()) + .bind(Keys::generate().public_key().to_bytes().to_vec()) + .execute(db.pool()) + .await + .expect("insert renamed channel"); + + // The row before the repair renamed it, dated ahead of this command's + // clock: a relay that read the old row published it, signing later than + // the event it replaced. The command must still replace it. + let relay = Keys::generate(); + let mut before_rename = db.get_channel(community, channel).await.unwrap(); + before_rename.name = "square-ios".to_owned(); + let stale = EventBuilder::new(Kind::Custom(39000), "") + .tags( + buzz_db::channel::group_metadata_tags(&before_rename, &[]) + .into_iter() + .map(Tag::parse) + .collect::, _>>() + .unwrap(), + ) + .custom_created_at(Timestamp::from(Timestamp::now().as_secs() + 60)) + .sign_with_keys(&relay) + .unwrap(); + let (_, inserted) = db + .replace_addressable_event(community, &stale, Some(channel)) + .await + .unwrap(); + assert!(inserted); + + let output = Command::new(admin_binary()) + .args(["reconcile-channels", "--channel", &channel.to_string()]) + .args([ + "--metadata", + "--relay-key", + &relay.secret_key().to_secret_hex(), + ]) + .env("DATABASE_URL", &url) + .env("RELAY_URL", format!("wss://{host}")) + .output() + .await + .expect("run buzz-admin"); + assert!( + output.status.success(), + "reconcile-channels failed:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + + let newest = db + .query_events(&EventQuery { + kinds: Some(vec![39000]), + d_tag: Some(channel.to_string()), + limit: Some(1), + ..EventQuery::for_community(community) + }) + .await + .unwrap(); + let newest = &newest.first().expect("kind:39000 stored").event; + assert_eq!(name_tag(newest), Some("square-ios-79f8e9d4")); + assert!(newest.created_at > stale.created_at); +} diff --git a/crates/buzz-db/src/error.rs b/crates/buzz-db/src/error.rs index 5b43d443642..39e03f5392e 100644 --- a/crates/buzz-db/src/error.rs +++ b/crates/buzz-db/src/error.rs @@ -25,6 +25,11 @@ pub enum DbError { #[error("channel not found: {0}")] ChannelNotFound(uuid::Uuid), + /// Another listed open channel in the community already has this name + /// (see `channel::ensure_listed_channel_name_unique`). + #[error("an open channel named \"{0}\" already exists")] + ChannelNameTaken(String), + /// The requested member is not in the channel. #[error("member not found in channel {0}")] MemberNotFound(uuid::Uuid), diff --git a/crates/buzz-db/src/runtime/tests.rs b/crates/buzz-db/src/runtime/tests.rs index 71d77c6f8f3..b7c9d5fe2ac 100644 --- a/crates/buzz-db/src/runtime/tests.rs +++ b/crates/buzz-db/src/runtime/tests.rs @@ -4069,6 +4069,9 @@ async fn e_tag_any_runs_with_real_binds_on_p_join_and_count() { #[path = "tests/thread_window_postgres_tests.rs"] mod thread_window_postgres_tests; +#[path = "tests/listed_channel_name_postgres_tests.rs"] +mod listed_channel_name_postgres_tests; + #[path = "tests/reader_pool_postgres_tests.rs"] mod reader_pool_postgres_tests; diff --git a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs new file mode 100644 index 00000000000..b223312ddea --- /dev/null +++ b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs @@ -0,0 +1,226 @@ +//! scripts/maintenance/rename_duplicate_listed_channel_names.sql leaves no two +//! listed channels in a community with one name, ignoring case. It keeps the +//! name on the oldest unarchived channel, picks a new name that is free, and +//! leaves alone a row that a relay changed while the script waited for it. + +use super::*; + +const REPAIR_SCRIPT: &str = + include_str!("../../../../../scripts/maintenance/rename_duplicate_listed_channel_names.sql"); + +struct Seed { + community: Uuid, + created_by: Vec, +} + +impl Seed { + async fn new(pool: &PgPool) -> Self { + let community = Uuid::new_v4(); + sqlx::query("INSERT INTO communities (id, host) VALUES ($1, $2)") + .bind(community) + .bind(format!("listed-name-{}.example", community.simple())) + .execute(pool) + .await + .expect("insert community"); + Self { + community, + created_by: nostr::Keys::generate().public_key().to_bytes().to_vec(), + } + } + + /// Inserts an open stream `age` seconds old with plain SQL, as a relay + /// without the name rule did. + async fn open(&self, pool: &PgPool, id: Uuid, name: &str, age: i64) { + sqlx::query( + "INSERT INTO channels (id, community_id, name, channel_type, visibility, created_by, \ + created_at) \ + VALUES ($1, $2, $3, 'stream', 'open', $4, now() - make_interval(secs => $5))", + ) + .bind(id) + .bind(self.community) + .bind(name) + .bind(&self.created_by) + .bind(age as f64) + .execute(pool) + .await + .expect("insert channel"); + } + + async fn names(&self, pool: &PgPool) -> Vec<(Uuid, String)> { + sqlx::query_as("SELECT id, name FROM channels WHERE community_id = $1 ORDER BY id") + .bind(self.community) + .fetch_all(pool) + .await + .expect("read channel names") + } +} + +fn renamed(id: Uuid, name: &str) -> (Uuid, String) { + (id, format!("{name}-{}", &id.to_string()[..8])) +} + +/// Runs the repair the way `psql -f` does: the `DO` block as its own +/// statement, so that its per-batch COMMITs run outside a transaction block. +/// A repair that stops making progress fails at the statement timeout. +/// Returns the number of rows the script's validation query reports. +pub(super) async fn run_repair(pool: &PgPool) -> i64 { + let (repair, validation) = REPAIR_SCRIPT + .split_once("\n-- Validation.") + .expect("script has a validation query"); + let mut conn = pool.acquire().await.expect("acquire repair connection"); + sqlx::query("SET statement_timeout = '30s'") + .execute(&mut *conn) + .await + .expect("bound the repair"); + sqlx::raw_sql(sqlx::AssertSqlSafe(repair)) + .execute(&mut *conn) + .await + .expect("run repair"); + drop(conn); + let (_, query) = validation.split_once("\nSELECT").expect("validation query"); + sqlx::query_scalar::<_, i64>(sqlx::AssertSqlSafe(format!( + "SELECT count(*) FROM (SELECT{}) AS remaining", + query.trim_end().trim_end_matches(';') + ))) + .fetch_one(pool) + .await + .expect("run validation") +} + +#[tokio::test] +#[ignore = "requires Postgres"] +async fn repair_keeps_each_listed_name_on_its_oldest_unarchived_channel() { + let admin = PgPool::connect(&admin_url().await).await.unwrap(); + let (pool, name) = create_scratch_db(&admin, "listed_name_repair").await; + let seed = Seed::new(&pool).await; + let [kept, archived, newer, private, deleted] = [(); 5].map(|_| Uuid::new_v4()); + seed.open(&pool, kept, "square-ios", 30).await; + seed.open(&pool, archived, "Square-iOS", 60).await; + seed.open(&pool, newer, "SQUARE-IOS", 10).await; + seed.open(&pool, private, "square-ios", 5).await; + seed.open(&pool, deleted, "square-ios", 90).await; + sqlx::query( + "UPDATE channels SET \ + archived_at = CASE WHEN id = $2 THEN now() END, \ + visibility = CASE WHEN id = $3 THEN 'private'::channel_visibility ELSE visibility END, \ + deleted_at = CASE WHEN id = $4 THEN now() END \ + WHERE community_id = $1", + ) + .bind(seed.community) + .bind(archived) + .bind(private) + .bind(deleted) + .execute(&pool) + .await + .unwrap(); + // A community with no duplicates is not touched. + let other = Seed::new(&pool).await; + other.open(&pool, Uuid::new_v4(), "square-ios", 0).await; + let other_before = other.names(&pool).await; + + assert_eq!(run_repair(&pool).await, 0, "no duplicates remain"); + let mut expected = vec![ + (kept, "square-ios".to_owned()), + renamed(archived, "Square-iOS"), + renamed(newer, "SQUARE-IOS"), + (private, "square-ios".to_owned()), + (deleted, "square-ios".to_owned()), + ]; + expected.sort(); + assert_eq!(seed.names(&pool).await, expected); + assert_eq!(other.names(&pool).await, other_before); + + assert_eq!(run_repair(&pool).await, 0); + assert_eq!( + seed.names(&pool).await, + expected, + "a second run changes nothing" + ); + drop_scratch_db(&admin, pool, &name).await; +} + +#[tokio::test] +#[ignore = "requires Postgres"] +async fn repair_picks_a_free_name_when_the_short_one_is_taken() { + let admin = PgPool::connect(&admin_url().await).await.unwrap(); + let (pool, name) = create_scratch_db(&admin, "listed_name_repair_taken").await; + let seed = Seed::new(&pool).await; + // The longest name the column holds, so the new name must truncate it. + let long = "a".repeat(255); + let duplicate = Uuid::parse_str("20000000-0000-4000-8000-000000000000").unwrap(); + let (kept, squatter) = (Uuid::new_v4(), Uuid::new_v4()); + seed.open(&pool, kept, &long, 30).await; + seed.open(&pool, duplicate, &long, 10).await; + // Another listed channel already has the short new name, in another case. + let short = format!("{}-20000000", "A".repeat(246)); + seed.open(&pool, squatter, &short, 60).await; + + assert_eq!(run_repair(&pool).await, 0, "no duplicates remain"); + let mut expected = vec![ + (kept, long.clone()), + (duplicate, format!("{}-{duplicate}", "a".repeat(218))), + (squatter, short), + ]; + expected.sort(); + assert_eq!(seed.names(&pool).await, expected); + assert!(expected.iter().all(|(_, name)| name.chars().count() <= 255)); + drop_scratch_db(&admin, pool, &name).await; +} + +#[tokio::test] +#[ignore = "requires Postgres"] +async fn repair_leaves_a_row_that_stopped_being_a_duplicate_while_it_waited() { + let admin = PgPool::connect(&admin_url().await).await.unwrap(); + let (pool, name) = create_scratch_db(&admin, "listed_name_repair_wait").await; + let seed = Seed::new(&pool).await; + let (kept, duplicate) = (Uuid::new_v4(), Uuid::new_v4()); + seed.open(&pool, kept, "payments", 30).await; + seed.open(&pool, duplicate, "payments", 10).await; + + // A relay write holds the duplicate's row while the script starts. + let mut relay = pool.begin().await.unwrap(); + sqlx::query("SELECT 1 FROM channels WHERE community_id = $1 AND id = $2 FOR NO KEY UPDATE") + .bind(seed.community) + .bind(duplicate) + .execute(&mut *relay) + .await + .unwrap(); + let repair_pool = PgPool::connect_with( + (*pool.connect_options()) + .clone() + .application_name("listed-name-repair"), + ) + .await + .unwrap(); + let repair = tokio::spawn({ + let repair_pool = repair_pool.clone(); + async move { run_repair(&repair_pool).await } + }); + crate::test_support::wait_for_lock_wait(&pool, "listed-name-repair").await; + + // The relay makes the channel private and renames it, then commits. + sqlx::query( + "UPDATE channels SET name = 'payments-private', visibility = 'private' \ + WHERE community_id = $1 AND id = $2", + ) + .bind(seed.community) + .bind(duplicate) + .execute(&mut *relay) + .await + .unwrap(); + relay.commit().await.unwrap(); + + assert_eq!(repair.await.unwrap(), 0); + let mut expected = vec![ + (kept, "payments".to_owned()), + (duplicate, "payments-private".to_owned()), + ]; + expected.sort(); + assert_eq!( + seed.names(&pool).await, + expected, + "the private row is not renamed" + ); + repair_pool.close().await; + drop_scratch_db(&admin, pool, &name).await; +} diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index 36ae4a1dc6b..f376419a1ff 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -159,7 +159,9 @@ pub async fn create_channel( .bind(labels) .bind(unlisted) .execute(tx.conn()) - .await?; + .await + .map_err(|e| listed_name_taken(e, Some(name)))?; + ensure_listed_channel_name_unique(&mut tx, id).await?; sqlx::query( r#" @@ -396,12 +398,14 @@ async fn insert_channel_rows( .bind(labels) .bind(unlisted) .execute(tx.conn()) - .await? + .await + .map_err(|e| listed_name_taken(e, Some(name)))? .rows_affected(); let was_created = rows_affected > 0; if was_created { + ensure_listed_channel_name_unique(tx, channel_id).await?; // Bootstrap the creator as owner. sqlx::query( r#" @@ -450,6 +454,81 @@ async fn insert_channel_rows( Ok(was_created) } +/// SQL that is true for a channel the channel directory lists: an open, +/// listed stream or forum that is not deleted. An archived channel is still +/// listed, so it keeps its name. Within a community, no two of these share a +/// name, ignoring case ([`ensure_listed_channel_name_unique`]). +pub(crate) const LISTED_CHANNEL_SQL: &str = + "visibility = 'open' AND NOT unlisted AND channel_type IN ('stream', 'forum') \ + AND deleted_at IS NULL"; + +/// Refuses, with [`DbError::ChannelNameTaken`], a write on `tx` that left +/// `channel_id` listed ([`LISTED_CHANNEL_SQL`]) under a name another listed +/// channel in the community already has, ignoring case. Private channels, +/// DMs and the other types may repeat names: nobody browses them side by +/// side, and refusing a name that a private channel holds would tell anyone +/// that the channel exists. +/// +/// Call it after the write that creates the channel or changes its name, +/// visibility or unlisted flag. It first takes a transaction advisory lock +/// on the name, so two writers that claim one name take turns and the second +/// sees the first's committed row. A writer takes this lock last, after the +/// locks of [`lock_channel_for_settings_write`], and holds at most one. +async fn ensure_listed_channel_name_unique( + tx: &mut crate::AdmittedTx, + channel_id: Uuid, +) -> Result<()> { + let community_id = tx.community(); + let name: Option = sqlx::query_scalar(sqlx::AssertSqlSafe(format!( + "SELECT name FROM channels WHERE community_id = $1 AND id = $2 AND {LISTED_CHANNEL_SQL}" + ))) + .bind(community_id.as_uuid()) + .bind(channel_id) + .fetch_optional(tx.conn()) + .await?; + let Some(name) = name else { + return Ok(()); + }; + sqlx::query("SELECT pg_advisory_xact_lock(hashtextextended($1 || lower($2), 0))") + .bind(format!("buzz_channel_name:{}:", community_id.as_uuid())) + .bind(&name) + .execute(tx.conn()) + .await?; + let taken: bool = sqlx::query_scalar(sqlx::AssertSqlSafe(format!( + "SELECT EXISTS (SELECT 1 FROM channels WHERE community_id = $1 AND id <> $2 \ + AND lower(name) = lower($3) AND {LISTED_CHANNEL_SQL})" + ))) + .bind(community_id.as_uuid()) + .bind(channel_id) + .bind(&name) + .fetch_one(tx.conn()) + .await?; + if taken { + return Err(DbError::ChannelNameTaken(name)); + } + Ok(()) +} + +/// The unique index on `(community_id, lower(name))` over +/// [`LISTED_CHANNEL_SQL`] channels that migration 0079 adds. It refuses a +/// duplicate at the write itself, before +/// [`ensure_listed_channel_name_unique`] runs, so this release maps the +/// refusal too: it can run beside that index during the rollout of 0079 and +/// after a rollback. +const LISTED_NAME_INDEX: &str = "idx_channels_listed_name"; + +/// Reports a channel write that [`LISTED_NAME_INDEX`] refused as +/// [`DbError::ChannelNameTaken`] for `name`, the name the channel would have +/// had. +fn listed_name_taken(error: sqlx::Error, name: Option<&str>) -> DbError { + match (&error, name) { + (sqlx::Error::Database(db), Some(name)) if db.constraint() == Some(LISTED_NAME_INDEX) => { + DbError::ChannelNameTaken(name.to_owned()) + } + _ => error.into(), + } +} + /// SQL that is true when `column` holds a legacy channel type /// ([`buzz_core::channel::LEGACY_CHANNEL_TYPES`]): ` IN ('stream', …)`. /// @@ -1193,21 +1272,30 @@ async fn update_channel_on( q = q.bind(community_id.as_uuid()); q = q.bind(channel_id); - let policy_changed = if updates.visibility.is_some() + // The current name is what a visibility or unlisted change that + // [`LISTED_NAME_INDEX`] refuses would have listed. + let (policy_changed, current_name) = if updates.visibility.is_some() || updates.ttl_seconds.is_some() || updates.unlisted.is_some() { - sqlx::query_scalar::<_,bool>("SELECT (($3::text IS NOT NULL AND visibility::text IS DISTINCT FROM $3) OR ($4 AND ttl_seconds IS DISTINCT FROM $5::int) OR ($6::bool IS NOT NULL AND unlisted IS DISTINCT FROM $6)) FROM channels WHERE community_id=$1 AND id=$2") + sqlx::query_as::<_,(bool, String)>("SELECT (($3::text IS NOT NULL AND visibility::text IS DISTINCT FROM $3) OR ($4 AND ttl_seconds IS DISTINCT FROM $5::int) OR ($6::bool IS NOT NULL AND unlisted IS DISTINCT FROM $6)), name FROM channels WHERE community_id=$1 AND id=$2") .bind(community_id.as_uuid()).bind(channel_id).bind(updates.visibility.as_deref()) .bind(updates.ttl_seconds.is_some()).bind(updates.ttl_seconds.flatten()).bind(updates.unlisted) - .fetch_one(tx.conn()).await? + .fetch_one(tx.conn()).await + .map(|(changed, name)| (changed, Some(name)))? } else { - false + (false, None) }; - let result = q.execute(tx.conn()).await?; + let result = q + .execute(tx.conn()) + .await + .map_err(|e| listed_name_taken(e, updates.name.as_deref().or(current_name.as_deref())))?; if result.rows_affected() == 0 { return Err(DbError::ChannelNotFound(channel_id)); } + if updates.name.is_some() || updates.visibility.is_some() || updates.unlisted.is_some() { + ensure_listed_channel_name_unique(tx, channel_id).await?; + } if policy_changed { sqlx::query( "UPDATE communities SET artifact_policy_modified_at=clock_timestamp() WHERE id=$1", @@ -2570,6 +2658,86 @@ mod postgres_tests { assert!(matches!(outcome, ChannelCreate::Created(_)), "{outcome:?}"); } + /// Holds an open create of the listed channel `race` on its own + /// transaction while a second create claims the same name beside it. + async fn second_listed_create_beside_open_create(commit_first: bool) -> Result { + let pool = setup_pool().await; + let community = CommunityId::from_uuid(make_test_community(&pool).await); + let creator = Keys::generate(); + let creator_bytes = creator.public_key().to_bytes().to_vec(); + let listed = |name: &'static str| NewChannel { + name, + visibility: ChannelVisibility::Open, + ..new_channel(&creator_bytes, &[]) + }; + + let mut first = crate::begin_community_write_transaction( + &pool, + community, + crate::observability::WriterOperation::EventWrite, + ) + .await + .expect("begin first create"); + assert!( + insert_channel_rows(&mut first, Uuid::new_v4(), listed("race")) + .await + .expect("first create's rows") + ); + + let second_id = Uuid::new_v4(); + let second = tokio::spawn({ + let (creator, creator_bytes) = (creator.clone(), creator_bytes.clone()); + async move { + let pool = crate::test_support::named_pool("listed-name-race").await; + create_channel_with_event( + &pool, + None, + community, + second_id, + NewChannel { + name: "RACE", + visibility: ChannelVisibility::Open, + ..new_channel(&creator_bytes, &[]) + }, + &create_event(&creator, second_id), + ) + .await + } + }); + crate::test_support::wait_for_lock_wait(&pool, "listed-name-race").await; + assert!( + !second.is_finished(), + "the second create waits for the open create's name" + ); + + if commit_first { + first.commit().await.expect("commit first create"); + } else { + drop(first); + } + second.await.expect("second create task") + } + + #[tokio::test] + #[ignore = "requires Postgres"] + async fn a_listed_create_beside_an_open_create_of_its_name_is_refused_once_committed() { + let outcome = second_listed_create_beside_open_create(true).await; + assert!( + matches!(&outcome, Err(DbError::ChannelNameTaken(name)) if name == "RACE"), + "{outcome:?}" + ); + } + + #[tokio::test] + #[ignore = "requires Postgres"] + async fn a_listed_create_beside_a_failed_create_of_its_name_succeeds() { + let outcome = second_listed_create_beside_open_create(false).await; + assert!( + matches!(outcome, Ok(ChannelCreate::Created(_))), + "{outcome:?}" + ); + } + #[tokio::test] #[ignore = "requires Postgres"] async fn a_create_whose_event_fails_leaves_no_channel() { diff --git a/crates/buzz-db/src/store/channel_name_postgres_tests.rs b/crates/buzz-db/src/store/channel_name_postgres_tests.rs new file mode 100644 index 00000000000..bc387dc2a3c --- /dev/null +++ b/crates/buzz-db/src/store/channel_name_postgres_tests.rs @@ -0,0 +1,270 @@ +//! Two listed open channels in a community never share a name, ignoring +//! case. Every write that can make a channel claim a listed name goes through +//! the real store functions here: create, rename, private to open, and +//! unlisted to listed. An archived channel keeps its name; a deleted one +//! frees it. + +use crate::channel::ChannelUpdate; +use crate::error::DbError; +use crate::Db; +use buzz_core::channel::{ChannelType, ChannelVisibility}; +use buzz_core::CommunityId; +use nostr::Keys; +use sqlx::PgPool; +use uuid::Uuid; + +struct Fixture { + db: Db, + community: CommunityId, + owner: Keys, +} + +impl Fixture { + async fn new() -> Self { + let pool = PgPool::connect(&crate::test_support::database_url()) + .await + .expect("connect to test DB"); + if std::env::var("BUZZ_TEST_SCHEMA_MODE").as_deref() != Ok("desired") { + crate::migration::run_migrations(&pool) + .await + .expect("migrate test DB"); + } + Self { + community: insert_community(&pool).await, + db: Db::from_pool(pool), + owner: Keys::generate(), + } + } + + async fn create( + &self, + name: &str, + channel_type: ChannelType, + visibility: ChannelVisibility, + ) -> crate::error::Result { + self.create_in(self.community, name, channel_type, visibility) + .await + } + + async fn create_in( + &self, + community: CommunityId, + name: &str, + channel_type: ChannelType, + visibility: ChannelVisibility, + ) -> crate::error::Result { + self.db + .create_channel( + community, + name, + channel_type, + visibility, + None, + &self.owner.public_key().to_bytes(), + None, + ) + .await + .map(|channel| channel.id) + } + + async fn open(&self, name: &str) -> Uuid { + self.create(name, ChannelType::Stream, ChannelVisibility::Open) + .await + .expect("create open channel") + } + + async fn update(&self, channel: Uuid, update: ChannelUpdate) -> crate::error::Result<()> { + self.db + .update_channel(self.community, channel, update) + .await + .map(|_| ()) + } + + async fn listing_state(&self, channel: Uuid) -> (String, String, bool) { + let channel = self + .db + .get_channel(self.community, channel) + .await + .expect("reload channel"); + (channel.name, channel.visibility, channel.unlisted) + } +} + +async fn insert_community(pool: &PgPool) -> CommunityId { + let community = CommunityId::from_uuid(Uuid::new_v4()); + sqlx::query("INSERT INTO communities(id,host) VALUES($1,$2)") + .bind(community.as_uuid()) + .bind(format!("channel-name-{}.test", community.as_uuid())) + .execute(pool) + .await + .expect("insert community"); + community +} + +fn assert_taken(result: crate::error::Result, name: &str) { + match result { + Err(DbError::ChannelNameTaken(taken)) => assert_eq!(taken, name), + other => panic!("expected {name:?} to be taken, got {other:?}"), + } +} + +#[tokio::test] +#[ignore = "requires Postgres"] +async fn a_listed_name_is_taken_only_for_listed_channels() { + let f = Fixture::new().await; + f.open("Square-iOS").await; + + // Another listed channel cannot take it, whatever its case or type, and + // the display prefix is not part of the name. + for (name, channel_type) in [ + ("square-ios", ChannelType::Stream), + ("SQUARE-IOS", ChannelType::Forum), + ("#square-ios ", ChannelType::Stream), + ] { + assert_taken( + f.create(name, channel_type, ChannelVisibility::Open).await, + buzz_core::channel::canonical_channel_name(name), + ); + } + + // Channels the directory does not list may repeat it. + for (channel_type, visibility) in [ + (ChannelType::Stream, ChannelVisibility::Private), + (ChannelType::Forum, ChannelVisibility::Private), + (ChannelType::Dm, ChannelVisibility::Private), + (ChannelType::Workflow, ChannelVisibility::Open), + (ChannelType::System, ChannelVisibility::Open), + ] { + f.create("square-ios", channel_type, visibility) + .await + .unwrap_or_else(|e| panic!("{channel_type:?}/{visibility:?}: {e}")); + } + + // Names are per community. + let other = insert_community(f.db.pool()).await; + f.create_in( + other, + "square-ios", + ChannelType::Stream, + ChannelVisibility::Open, + ) + .await + .expect("another community may use the name"); +} + +#[tokio::test] +#[ignore = "requires Postgres"] +async fn a_change_that_would_list_a_taken_name_changes_nothing() { + let f = Fixture::new().await; + f.open("payments").await; + + let private = f + .create("payments", ChannelType::Stream, ChannelVisibility::Private) + .await + .expect("private channel"); + let unlisted = f.open("payments-unlisted").await; + f.update( + unlisted, + ChannelUpdate { + name: Some("payments".into()), + unlisted: Some(true), + ..Default::default() + }, + ) + .await + .expect("an unlisted channel may share the name"); + let other = f.open("payments-team").await; + + let cases = [ + ( + private, + ChannelUpdate { + visibility: Some("open".into()), + ..Default::default() + }, + ), + ( + unlisted, + ChannelUpdate { + unlisted: Some(false), + ..Default::default() + }, + ), + ( + other, + ChannelUpdate { + name: Some("PAYMENTS".into()), + description: Some("rolled back with the name".into()), + ..Default::default() + }, + ), + ]; + for (channel, update) in cases { + let before = f.listing_state(channel).await; + let description = + f.db.get_channel(f.community, channel) + .await + .expect("reload") + .description; + let attempted = update.name.clone().unwrap_or_else(|| before.0.clone()); + assert_taken(f.update(channel, update).await, &attempted); + assert_eq!(f.listing_state(channel).await, before); + assert_eq!( + f.db.get_channel(f.community, channel) + .await + .expect("reload") + .description, + description, + "the whole update rolls back" + ); + } + + // A rename to a free name, and the same name in another case, still work. + f.update( + other, + ChannelUpdate { + name: Some("payments-core".into()), + ..Default::default() + }, + ) + .await + .expect("rename to a free name"); + f.update( + other, + ChannelUpdate { + name: Some("Payments-Core".into()), + ..Default::default() + }, + ) + .await + .expect("a channel may change the case of its own name"); +} + +#[tokio::test] +#[ignore = "requires Postgres"] +async fn an_archived_channel_keeps_its_name_until_it_is_deleted() { + let f = Fixture::new().await; + let archived = f.open("square-ios").await; + f.db.archive_channel(f.community, archived) + .await + .expect("archive"); + + assert_taken( + f.create("square-ios", ChannelType::Stream, ChannelVisibility::Open) + .await, + "square-ios", + ); + f.db.unarchive_channel(f.community, archived) + .await + .expect("unarchive never collides"); + f.db.archive_channel(f.community, archived) + .await + .expect("archive again"); + + assert!(f + .db + .soft_delete_channel(f.community, archived) + .await + .expect("delete")); + f.open("square-ios").await; +} diff --git a/crates/buzz-db/src/store/mod.rs b/crates/buzz-db/src/store/mod.rs index e71f292ef27..bd2d709151b 100644 --- a/crates/buzz-db/src/store/mod.rs +++ b/crates/buzz-db/src/store/mod.rs @@ -96,6 +96,9 @@ mod legacy_channel_types_postgres_tests; #[cfg(test)] mod unlisted_channels_postgres_tests; +#[cfg(test)] +mod channel_name_postgres_tests; + #[cfg(test)] mod admission_postgres_tests; diff --git a/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs b/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs index 0e3859c05b9..fd1996340e2 100644 --- a/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs @@ -1074,3 +1074,97 @@ async fn removed_creator_join_racing_a_private_switch_is_refused() { "a removed creator must not rejoin a channel that became private" ); } + +/// A create or rename that would list a second open channel under a taken +/// name is refused with its reason as a client error, and stores nothing. +/// Mutation: let `ChannelNameTaken` fall through to the internal-error arm +/// of either kind → RED. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn a_taken_open_name_is_refused_as_a_client_error() { + use crate::handlers::ingest::IngestError; + + let state = state().await; + let tenant = community(&state, "group-state-name-taken").await; + let owner = Keys::generate(); + let create = |name: &str| { + let channel_id = Uuid::new_v4(); + let event = EventBuilder::new(Kind::Custom(9007), "") + .tags([ + Tag::parse(["h", &channel_id.to_string()]).unwrap(), + Tag::parse(["name", name]).unwrap(), + ]) + .sign_with_keys(&owner) + .expect("sign 9007"); + (channel_id, event) + }; + + let (_, first) = create("launch"); + assert!( + ingest_event(&state, &tenant, first, http_auth(&owner)) + .await + .expect("first create") + .accepted + ); + + let (second, event) = create("Launch"); + match ingest_event(&state, &tenant, event, http_auth(&owner)).await { + Err(IngestError::Rejected(message)) => assert_eq!( + message, + "invalid: an open channel named \"Launch\" already exists" + ), + Err(other) => panic!("expected a refusal, got {other:?}"), + Ok(result) => panic!("expected a refusal, got {:?}", result.message), + } + assert!( + state + .db + .get_channel(tenant.community(), second) + .await + .is_err(), + "the refused create stores no channel" + ); + + let other = create_channel(&state, &tenant, &owner, &[]).await; + let before = state + .db + .get_channel(tenant.community(), other) + .await + .expect("channel") + .name; + let rename = EventBuilder::new(Kind::Custom(9002), "") + .tags([ + Tag::parse(["h", &other.to_string()]).unwrap(), + Tag::parse(["name", "LAUNCH"]).unwrap(), + ]) + .sign_with_keys(&owner) + .expect("sign 9002"); + let rename_id = rename.id; + match ingest_event(&state, &tenant, rename, http_auth(&owner)).await { + Err(IngestError::Rejected(message)) => assert_eq!( + message, + "invalid: an open channel named \"LAUNCH\" already exists" + ), + Err(other) => panic!("expected a refusal, got {other:?}"), + Ok(result) => panic!("expected a refusal, got {:?}", result.message), + } + assert_eq!( + state + .db + .get_channel(tenant.community(), other) + .await + .expect("channel") + .name, + before, + "the refused rename keeps the name" + ); + let stored: bool = sqlx::query_scalar( + "SELECT EXISTS (SELECT 1 FROM events WHERE community_id = $1 AND id = $2)", + ) + .bind(tenant.community().as_uuid()) + .bind(rename_id.as_bytes().as_slice()) + .fetch_one(state.db.pool()) + .await + .expect("look up the 9002"); + assert!(!stored, "the refused 9002 is not stored"); +} diff --git a/crates/buzz-relay/src/handlers/ingest.rs b/crates/buzz-relay/src/handlers/ingest.rs index 70ebc75a9d8..d394558253f 100644 --- a/crates/buzz-relay/src/handlers/ingest.rs +++ b/crates/buzz-relay/src/handlers/ingest.rs @@ -3559,8 +3559,12 @@ async fn ingest_event_inner( .db .create_channel_with_event(tenant.community(), create.channel_id, channel, &event) .await - .map_err(|e| IngestError::Internal(format!("error: {e}")))? - { + .map_err(|e| match e { + buzz_db::DbError::ChannelNameTaken(_) => { + IngestError::Rejected(format!("invalid: {e}")) + } + _ => IngestError::Internal(format!("error: {e}")), + })? { buzz_db::channel::ChannelCreate::Created(stored) => { metrics::counter!( "buzz_channels_created_total", @@ -3625,6 +3629,9 @@ async fn ingest_event_inner( Some(buzz_db::DbError::AccessDenied(reason)) => { IngestError::Rejected(format!("invalid: {reason}")) } + Some(taken @ buzz_db::DbError::ChannelNameTaken(_)) => { + IngestError::Rejected(format!("invalid: {taken}")) + } _ => IngestError::Internal(format!("error: channel update failed: {e}")), })? } else if buzz_core::kind::is_replaceable(kind_u32) { diff --git a/scripts/maintenance/rename_duplicate_listed_channel_names.sql b/scripts/maintenance/rename_duplicate_listed_channel_names.sql new file mode 100644 index 00000000000..0264031bb07 --- /dev/null +++ b/scripts/maintenance/rename_duplicate_listed_channel_names.sql @@ -0,0 +1,161 @@ +-- OUT-OF-BAND MAINTENANCE: do not run from relay startup migrations. +-- +-- Gives every listed open channel a name that no other listed open channel in +-- its community has, ignoring case. A listed channel is an open, not unlisted +-- stream or forum that is not deleted; an archived one still counts. This is +-- the rule the relay enforces on create and rename (`LISTED_CHANNEL_SQL` in +-- crates/buzz-db/src/store/channel.rs). It repairs the duplicates created +-- before that rule, so the contract migration's unique index can build. +-- +-- Run it only after every serving relay enforces the rule. An older relay can +-- create a new duplicate while the script runs. +-- +-- In each group of channels that share a name, one channel keeps it: the +-- oldest unarchived one, or the oldest archived one when all are archived. +-- Every other channel in the group becomes "-", or "-" when another listed channel already has that name. +-- The script prints a NOTICE for each rename. When both names are taken, it +-- leaves the channel unchanged with a NOTICE, and the validation below +-- reports it. +-- +-- The rename writes only the channel row. Clients read the name from the +-- relay-signed kind:39000, so republish it for each renamed channel: +-- +-- buzz-admin reconcile-channels --channel --metadata +-- +-- Each batch renames up to 100 channels in one community and commits. It +-- takes the community deletion lock first and skips a community that is not +-- active. It locks every row of the batch before it takes the relay's name +-- lock for any new name, in the order the relay takes them. After it locks a +-- row, it checks again that the row is still a listed duplicate, because a +-- relay may have changed it meanwhile. The script is idempotent: a second run +-- finds nothing to rename. +-- +-- Run it outside a transaction, so that each batch can commit: +-- +-- psql -v ON_ERROR_STOP=1 -f scripts/maintenance/rename_duplicate_listed_channel_names.sql + +DO $$ +DECLARE + target UUID; + skipped UUID[] := '{}'; + batch UUID[]; + duplicate UUID; + live RECORD; + new_name TEXT; + renamed INT; +BEGIN + LOOP + SELECT ch.community_id INTO target + FROM channels ch + WHERE ch.visibility = 'open' AND NOT ch.unlisted + AND ch.channel_type IN ('stream', 'forum') AND ch.deleted_at IS NULL + AND ch.community_id <> ALL (skipped) + GROUP BY ch.community_id, lower(ch.name) + HAVING count(*) > 1 + LIMIT 1; + EXIT WHEN target IS NULL; + + PERFORM set_config('lock_timeout', '5s', true); + PERFORM pg_advisory_xact_lock_shared(community_deletion_lock_key(target)); + IF NOT EXISTS ( + SELECT 1 FROM communities + WHERE id = target AND deletion_state = 'active' AND deleted_at IS NULL + ) THEN + RAISE NOTICE 'skipped community %: it is not active', target; + skipped := skipped || target; + COMMIT; + CONTINUE; + END IF; + + -- Lock every row of the batch before any name lock, in the order + -- the relay takes them. + SELECT array_agg(locked.id ORDER BY locked.id) INTO batch + FROM ( + WITH ranked AS ( + SELECT id, row_number() OVER ( + PARTITION BY lower(name) + ORDER BY archived_at IS NOT NULL, created_at, id + ) AS place + FROM channels + WHERE community_id = target + AND visibility = 'open' AND NOT unlisted + AND channel_type IN ('stream', 'forum') AND deleted_at IS NULL + ) + SELECT ch.id + FROM channels ch + JOIN ranked ON ranked.id = ch.id + WHERE ch.community_id = target AND ranked.place > 1 + ORDER BY ch.id + LIMIT 100 + FOR NO KEY UPDATE OF ch + ) locked; + + renamed := 0; + FOREACH duplicate IN ARRAY coalesce(batch, '{}') + LOOP + -- The ranking used the statement's snapshot. Now that the row is + -- locked, read it again: a relay may have renamed it, made it + -- private, or renamed the channel that kept the name. + SELECT ch.name, ch.archived_at, ch.created_at INTO live + FROM channels ch + WHERE ch.community_id = target AND ch.id = duplicate + AND ch.visibility = 'open' AND NOT ch.unlisted + AND ch.channel_type IN ('stream', 'forum') AND ch.deleted_at IS NULL; + CONTINUE WHEN NOT FOUND OR NOT EXISTS ( + SELECT 1 FROM channels other + WHERE other.community_id = target AND other.id <> duplicate + AND lower(other.name) = lower(live.name) + AND other.visibility = 'open' AND NOT other.unlisted + AND other.channel_type IN ('stream', 'forum') AND other.deleted_at IS NULL + AND (other.archived_at IS NOT NULL, other.created_at, other.id) + < (live.archived_at IS NOT NULL, live.created_at, duplicate) + ); + + new_name := NULL; + FOREACH new_name IN ARRAY ARRAY[ + left(live.name, 246) || '-' || left(duplicate::text, 8), + left(live.name, 218) || '-' || duplicate::text + ] LOOP + PERFORM pg_advisory_xact_lock(hashtextextended( + 'buzz_channel_name:' || target::text || ':' || lower(new_name), 0)); + EXIT WHEN NOT EXISTS ( + SELECT 1 FROM channels other + WHERE other.community_id = target AND other.id <> duplicate + AND lower(other.name) = lower(new_name) + AND other.visibility = 'open' AND NOT other.unlisted + AND other.channel_type IN ('stream', 'forum') AND other.deleted_at IS NULL + ); + new_name := NULL; + END LOOP; + IF new_name IS NULL THEN + RAISE NOTICE 'left channel % in community % as "%": both new names are taken', + duplicate, target, live.name; + CONTINUE; + END IF; + + UPDATE channels SET name = new_name, updated_at = NOW() + WHERE community_id = target AND id = duplicate; + RAISE NOTICE 'renamed channel % in community % from "%" to "%"', + duplicate, target, live.name, new_name; + renamed := renamed + 1; + END LOOP; + + -- A batch that found nothing to rename would find the same community + -- again; skip it so the loop ends. The validation below reports it. + IF renamed = 0 THEN + skipped := skipped || target; + END IF; + COMMIT; + END LOOP; +END +$$; + +-- Validation. Expected result: zero rows. A row names a community the script +-- skipped (not active) or a duplicate created while it ran; rerun the script. +SELECT community_id, lower(name) AS name, count(*) AS channels +FROM channels +WHERE visibility = 'open' AND NOT unlisted + AND channel_type IN ('stream', 'forum') AND deleted_at IS NULL +GROUP BY community_id, lower(name) +HAVING count(*) > 1; From 5b5efd7572a7c74a6e3d64261ac15065b51fdf35 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Fri, 30 Oct 2026 15:43:59 -0400 Subject: [PATCH 02/13] fix(relay): rename duplicate channel names through the relay Replace the direct-SQL repair, which needed every relay pod stopped, with renames that go through the relay's normal kind:9002 path: - A relay operator may send a name-only kind:9002 for any channel. Every other privileged change still needs a channel owner or admin. A moderator gets no new right. - scripts/maintenance/plan_duplicate_listed_channel_renames.sql lists the renames (read-only). rename_duplicate_listed_channel_names.sh sends each one with `buzz channels update`, signed by an operator key, then reads the plan again and fails if any rename remains. The relay enforces the name rule and publishes kind:39000 as for any rename, so relays keep serving while it runs. - Remove `buzz-admin reconcile-channel-groups --metadata` and the SQL repair script; nothing needs them now. - Refuse a kind:9007 with no h tag at ingest, before it is stored. Such a create used to run in side effects, so a refusal there (a taken name) still answered accepted. Every first-party client sends h. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- crates/buzz-admin/src/main.rs | 98 +------ .../tests/postgres_reconcile_metadata.rs | 111 -------- .../listed_channel_name_postgres_tests.rs | 251 +++++++++++------- crates/buzz-relay/src/api/admin/auth.rs | 12 + crates/buzz-relay/src/api/admin/mod.rs | 1 + .../handlers/group_state_postgres_tests.rs | 181 ++++++++----- crates/buzz-relay/src/handlers/ingest.rs | 11 +- .../buzz-relay/src/handlers/side_effects.rs | 83 +++--- .../plan_duplicate_listed_channel_renames.sql | 46 ++++ .../rename_duplicate_listed_channel_names.sh | 87 ++++++ .../rename_duplicate_listed_channel_names.sql | 161 ----------- 11 files changed, 478 insertions(+), 564 deletions(-) delete mode 100644 crates/buzz-admin/tests/postgres_reconcile_metadata.rs create mode 100644 scripts/maintenance/plan_duplicate_listed_channel_renames.sql create mode 100755 scripts/maintenance/rename_duplicate_listed_channel_names.sh delete mode 100644 scripts/maintenance/rename_duplicate_listed_channel_names.sql diff --git a/crates/buzz-admin/src/main.rs b/crates/buzz-admin/src/main.rs index 47820818217..b286037ac1d 100644 --- a/crates/buzz-admin/src/main.rs +++ b/crates/buzz-admin/src/main.rs @@ -139,18 +139,12 @@ enum Command { /// /// Without `--channel`, only channels missing discovery metadata are /// reconciled. With `--channel`, only that channel's member snapshot is - /// replaced; canonical metadata and admin events remain untouched unless - /// `--metadata` is given. + /// replaced; canonical metadata and admin events remain untouched. ReconcileChannels { /// Optional channel UUID to force-republish. #[arg(long)] channel: Option, - /// With `--channel`, also republish its kind:39000 metadata from the - /// channel row, for example after an operator rename. - #[arg(long, requires = "channel")] - metadata: bool, - /// Relay private key (hex) for signing events. Falls back to /// BUZZ_RELAY_PRIVATE_KEY env var. If neither is set, generates /// an ephemeral key (events will be unverifiable after restart). @@ -219,12 +213,8 @@ async fn run(cli: Cli) -> Result { Command::Deletions { command } => deletions::run(command).await, Command::UserDeletions { command } => user_deletions::run(command).await, Command::NipFi { command } => nip_fi::run(command).await, - Command::ReconcileChannels { - channel, - metadata, - relay_key, - } => { - reconcile_channels(channel, metadata, relay_key).await?; + Command::ReconcileChannels { channel, relay_key } => { + reconcile_channels(channel, relay_key).await?; Ok(0) } } @@ -733,7 +723,6 @@ async fn resolve_admin_tenant(db: &Db) -> Result { async fn reconcile_channels( channel_arg: Option, - republish_metadata: bool, relay_key_arg: Option, ) -> Result<()> { use buzz_core::channel::group_state_identity_tags; @@ -813,16 +802,11 @@ async fn reconcile_channels( let members = db.get_members(tenant.community(), channel.id).await?; - // A targeted repair is roster-only unless `--metadata` asks for - // kind:39000 too; kind:39001 is never replaced by a targeted repair. - // kind:39000 is built from the channel row by the same function the - // relay uses, so an operator rename (for example - // scripts/maintenance/rename_duplicate_listed_channel_names.sql) - // reaches clients. Full backfill still creates all three event kinds + // A targeted repair is deliberately roster-only. kind:39000 metadata + // is richer than this legacy backfill builder, and kind:39001 is not + // part of the stale-roster incident; replacing either can destroy + // canonical state. Full backfill still creates all three event kinds // for channels with no discovery metadata. - if republish_metadata { - republish_channel_metadata(&db, &tenant, channel.id, &relay_keys).await?; - } if target_channel.is_none() { // kind:39000 — channel metadata { @@ -892,74 +876,6 @@ async fn reconcile_channels( Ok(()) } -/// Makes the newest relay-signed kind:39000 of a channel match its row, for -/// example after an operator rename. -/// -/// Run it with no relay serving: the relay builds kind:39000 from a channel -/// read outside the replacement (`emit_group_discovery_events`), so a live -/// edit between this command's read and write could leave the older -/// snapshot. The command signs later than the stored event, as the relay -/// does, so it also replaces an event from the same second. It fails unless -/// a read after its write finds the stored event matching the row. -async fn republish_channel_metadata( - db: &Db, - tenant: &TenantContext, - channel_id: uuid::Uuid, - relay_keys: &Keys, -) -> Result<()> { - let latest = || async { - let stored = db - .query_events(&buzz_db::event::EventQuery { - kinds: Some(vec![39000]), - pubkey: Some(relay_keys.public_key().to_bytes().to_vec()), - d_tag: Some(channel_id.to_string()), - limit: Some(1), - ..buzz_db::event::EventQuery::for_community(tenant.community()) - }) - .await?; - Ok::<_, anyhow::Error>(stored.into_iter().next().map(|stored| stored.event)) - }; - let row_tags = || async { - let channel = db.get_channel(tenant.community(), channel_id).await?; - let members = db.get_members(tenant.community(), channel_id).await?; - Ok::<_, anyhow::Error>(buzz_db::channel::group_metadata_tags(&channel, &members)) - }; - let matches = |event: &nostr::Event, tags: &[Vec]| { - event - .tags - .iter() - .map(|tag| tag.as_slice()) - .eq(tags.iter().map(Vec::as_slice)) - }; - - let tags = row_tags().await?; - let stored = latest().await?; - if stored.as_ref().is_some_and(|event| matches(event, &tags)) { - return Ok(()); - } - let created_at = stored - .map_or(0, |event| event.created_at.as_secs() + 1) - .max(nostr::Timestamp::now().as_secs()); - let event = EventBuilder::new(Kind::Custom(39000), "") - .tags( - tags.into_iter() - .map(Tag::parse) - .collect::, _>>()?, - ) - .custom_created_at(nostr::Timestamp::from(created_at)) - .sign_with_keys(relay_keys) - .map_err(|e| anyhow::anyhow!("sign kind:39000: {e}"))?; - db.replace_addressable_event(tenant.community(), &event, Some(channel_id)) - .await?; - let tags = row_tags().await?; - if !latest().await?.is_some_and(|event| matches(&event, &tags)) { - return Err(anyhow::anyhow!( - "channel {channel_id} changed while its kind:39000 was republished; stop every relay and run the command again" - )); - } - Ok(()) -} - #[cfg(test)] mod storage_snapshot_tests { use std::sync::atomic::{AtomicUsize, Ordering}; diff --git a/crates/buzz-admin/tests/postgres_reconcile_metadata.rs b/crates/buzz-admin/tests/postgres_reconcile_metadata.rs deleted file mode 100644 index 2a05101c8db..00000000000 --- a/crates/buzz-admin/tests/postgres_reconcile_metadata.rs +++ /dev/null @@ -1,111 +0,0 @@ -//! `buzz-admin reconcile-channels --channel --metadata` leaves the newest -//! relay-signed kind:39000 matching the channel row, also when an older -//! snapshot of the row holds the replacement with a newer timestamp. - -use buzz_core::CommunityId; -use buzz_db::event::EventQuery; -use buzz_db::{Db, DbConfig}; -use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; -use tokio::process::Command; -use uuid::Uuid; - -fn admin_binary() -> std::ffi::OsString { - // Nextest relocates binaries when extracting an archive. Cargo's build-time - // path remains the fallback for local cargo test runs. - std::env::var_os("NEXTEST_BIN_EXE_buzz_admin") - .unwrap_or_else(|| env!("CARGO_BIN_EXE_buzz-admin").into()) -} - -fn name_tag(event: &nostr::Event) -> Option<&str> { - event.tags.iter().find_map(|tag| match tag.as_slice() { - [key, value, ..] if key == "name" => Some(value.as_str()), - _ => None, - }) -} - -#[tokio::test] -#[ignore = "requires Postgres"] -async fn metadata_republish_replaces_a_stale_snapshot_that_holds_the_replacement() { - let url = std::env::var("BUZZ_TEST_DATABASE_URL") - .expect("run through scripts/postgres-test-run.sh for an isolated database"); - let db = Db::new(&DbConfig { - database_url: url.clone(), - ..DbConfig::default() - }) - .await - .expect("connect to test DB"); - let community = Uuid::new_v4(); - let host = format!("reconcile-{}.example", community.simple()); - sqlx::query("INSERT INTO communities (id, host) VALUES ($1, $2)") - .bind(community) - .bind(&host) - .execute(db.pool()) - .await - .expect("insert community"); - let community = CommunityId::from_uuid(community); - let channel = Uuid::new_v4(); - sqlx::query( - "INSERT INTO channels (id, community_id, name, channel_type, visibility, created_by) \ - VALUES ($1, $2, 'square-ios-79f8e9d4', 'stream', 'open', $3)", - ) - .bind(channel) - .bind(community.as_uuid()) - .bind(Keys::generate().public_key().to_bytes().to_vec()) - .execute(db.pool()) - .await - .expect("insert renamed channel"); - - // The row before the repair renamed it, dated ahead of this command's - // clock: a relay that read the old row published it, signing later than - // the event it replaced. The command must still replace it. - let relay = Keys::generate(); - let mut before_rename = db.get_channel(community, channel).await.unwrap(); - before_rename.name = "square-ios".to_owned(); - let stale = EventBuilder::new(Kind::Custom(39000), "") - .tags( - buzz_db::channel::group_metadata_tags(&before_rename, &[]) - .into_iter() - .map(Tag::parse) - .collect::, _>>() - .unwrap(), - ) - .custom_created_at(Timestamp::from(Timestamp::now().as_secs() + 60)) - .sign_with_keys(&relay) - .unwrap(); - let (_, inserted) = db - .replace_addressable_event(community, &stale, Some(channel)) - .await - .unwrap(); - assert!(inserted); - - let output = Command::new(admin_binary()) - .args(["reconcile-channels", "--channel", &channel.to_string()]) - .args([ - "--metadata", - "--relay-key", - &relay.secret_key().to_secret_hex(), - ]) - .env("DATABASE_URL", &url) - .env("RELAY_URL", format!("wss://{host}")) - .output() - .await - .expect("run buzz-admin"); - assert!( - output.status.success(), - "reconcile-channels failed:\n{}", - String::from_utf8_lossy(&output.stderr) - ); - - let newest = db - .query_events(&EventQuery { - kinds: Some(vec![39000]), - d_tag: Some(channel.to_string()), - limit: Some(1), - ..EventQuery::for_community(community) - }) - .await - .unwrap(); - let newest = &newest.first().expect("kind:39000 stored").event; - assert_eq!(name_tag(newest), Some("square-ios-79f8e9d4")); - assert!(newest.created_at > stale.created_at); -} diff --git a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs index b223312ddea..291382cca13 100644 --- a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs +++ b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs @@ -1,29 +1,32 @@ -//! scripts/maintenance/rename_duplicate_listed_channel_names.sql leaves no two -//! listed channels in a community with one name, ignoring case. It keeps the -//! name on the oldest unarchived channel, picks a new name that is free, and -//! leaves alone a row that a relay changed while the script waited for it. +//! scripts/maintenance/plan_duplicate_listed_channel_renames.sql lists the +//! renames that leave no two listed channels in a community with one name, +//! ignoring case. It keeps the name on the oldest unarchived channel and picks +//! a new name that no other listed channel has. use super::*; -const REPAIR_SCRIPT: &str = - include_str!("../../../../../scripts/maintenance/rename_duplicate_listed_channel_names.sql"); +const PLAN_SQL: &str = + include_str!("../../../../../scripts/maintenance/plan_duplicate_listed_channel_renames.sql"); struct Seed { community: Uuid, + host: String, created_by: Vec, } impl Seed { async fn new(pool: &PgPool) -> Self { let community = Uuid::new_v4(); + let host = format!("listed-name-{}.example", community.simple()); sqlx::query("INSERT INTO communities (id, host) VALUES ($1, $2)") .bind(community) - .bind(format!("listed-name-{}.example", community.simple())) + .bind(&host) .execute(pool) .await .expect("insert community"); Self { community, + host, created_by: nostr::Keys::generate().public_key().to_bytes().to_vec(), } } @@ -59,39 +62,43 @@ fn renamed(id: Uuid, name: &str) -> (Uuid, String) { (id, format!("{name}-{}", &id.to_string()[..8])) } -/// Runs the repair the way `psql -f` does: the `DO` block as its own -/// statement, so that its per-batch COMMITs run outside a transaction block. -/// A repair that stops making progress fails at the statement timeout. -/// Returns the number of rows the script's validation query reports. -pub(super) async fn run_repair(pool: &PgPool) -> i64 { - let (repair, validation) = REPAIR_SCRIPT - .split_once("\n-- Validation.") - .expect("script has a validation query"); - let mut conn = pool.acquire().await.expect("acquire repair connection"); - sqlx::query("SET statement_timeout = '30s'") - .execute(&mut *conn) - .await - .expect("bound the repair"); - sqlx::raw_sql(sqlx::AssertSqlSafe(repair)) - .execute(&mut *conn) - .await - .expect("run repair"); - drop(conn); - let (_, query) = validation.split_once("\nSELECT").expect("validation query"); - sqlx::query_scalar::<_, i64>(sqlx::AssertSqlSafe(format!( - "SELECT count(*) FROM (SELECT{}) AS remaining", - query.trim_end().trim_end_matches(';') +/// The plan for `host`: (channel, current name, new name), as the rename +/// script reads it. +pub(super) async fn rename_plan(pool: &PgPool, host: &str) -> Vec<(Uuid, String, String)> { + let (_, query) = PLAN_SQL.split_once("\nWITH").expect("plan query"); + sqlx::query_as(sqlx::AssertSqlSafe(format!( + "WITH{}", + query + .trim_end() + .trim_end_matches(';') + .replace(":'host'", "$1") ))) - .fetch_one(pool) + .bind(host) + .fetch_all(pool) .await - .expect("run validation") + .expect("run the rename plan") +} + +/// Applies the plan the way the relay applies each rename the script sends, +/// and returns how many channels it renamed. +pub(super) async fn rename_duplicates(pool: &PgPool, host: &str) -> usize { + let plan = rename_plan(pool, host).await; + for (channel, _, new_name) in &plan { + sqlx::query("UPDATE channels SET name = $2 WHERE id = $1") + .bind(channel) + .bind(new_name) + .execute(pool) + .await + .expect("rename channel"); + } + plan.len() } #[tokio::test] #[ignore = "requires Postgres"] -async fn repair_keeps_each_listed_name_on_its_oldest_unarchived_channel() { +async fn plan_keeps_each_listed_name_on_its_oldest_unarchived_channel() { let admin = PgPool::connect(&admin_url().await).await.unwrap(); - let (pool, name) = create_scratch_db(&admin, "listed_name_repair").await; + let (pool, name) = create_scratch_db(&admin, "listed_name_plan").await; let seed = Seed::new(&pool).await; let [kept, archived, newer, private, deleted] = [(); 5].map(|_| Uuid::new_v4()); seed.open(&pool, kept, "square-ios", 30).await; @@ -113,37 +120,58 @@ async fn repair_keeps_each_listed_name_on_its_oldest_unarchived_channel() { .execute(&pool) .await .unwrap(); - // A community with no duplicates is not touched. + // Another community's channel with the same name is not a duplicate. let other = Seed::new(&pool).await; other.open(&pool, Uuid::new_v4(), "square-ios", 0).await; - let other_before = other.names(&pool).await; + // A community that is not active is left alone. + let inactive = Seed::new(&pool).await; + inactive.open(&pool, Uuid::new_v4(), "payments", 20).await; + inactive.open(&pool, Uuid::new_v4(), "payments", 10).await; + crate::test_support::quiesce_community_for_tests( + &pool, + buzz_core::CommunityId::from_uuid(inactive.community), + ) + .await; - assert_eq!(run_repair(&pool).await, 0, "no duplicates remain"); let mut expected = vec![ + ( + archived, + "Square-iOS".to_owned(), + renamed(archived, "Square-iOS").1, + ), + ( + newer, + "SQUARE-IOS".to_owned(), + renamed(newer, "SQUARE-IOS").1, + ), + ]; + expected.sort(); + assert_eq!(rename_plan(&pool, &seed.host).await, expected); + assert!(rename_plan(&pool, &other.host).await.is_empty()); + assert!(rename_plan(&pool, &inactive.host).await.is_empty()); + + assert_eq!(rename_duplicates(&pool, &seed.host).await, 2); + assert!( + rename_plan(&pool, &seed.host).await.is_empty(), + "nothing is left to rename" + ); + let mut names = vec![ (kept, "square-ios".to_owned()), renamed(archived, "Square-iOS"), renamed(newer, "SQUARE-IOS"), (private, "square-ios".to_owned()), (deleted, "square-ios".to_owned()), ]; - expected.sort(); - assert_eq!(seed.names(&pool).await, expected); - assert_eq!(other.names(&pool).await, other_before); - - assert_eq!(run_repair(&pool).await, 0); - assert_eq!( - seed.names(&pool).await, - expected, - "a second run changes nothing" - ); + names.sort(); + assert_eq!(seed.names(&pool).await, names); drop_scratch_db(&admin, pool, &name).await; } #[tokio::test] #[ignore = "requires Postgres"] -async fn repair_picks_a_free_name_when_the_short_one_is_taken() { +async fn plan_picks_a_free_name_when_the_short_one_is_taken() { let admin = PgPool::connect(&admin_url().await).await.unwrap(); - let (pool, name) = create_scratch_db(&admin, "listed_name_repair_taken").await; + let (pool, name) = create_scratch_db(&admin, "listed_name_plan_taken").await; let seed = Seed::new(&pool).await; // The longest name the column holds, so the new name must truncate it. let long = "a".repeat(255); @@ -155,72 +183,89 @@ async fn repair_picks_a_free_name_when_the_short_one_is_taken() { let short = format!("{}-20000000", "A".repeat(246)); seed.open(&pool, squatter, &short, 60).await; - assert_eq!(run_repair(&pool).await, 0, "no duplicates remain"); - let mut expected = vec![ - (kept, long.clone()), - (duplicate, format!("{}-{duplicate}", "a".repeat(218))), - (squatter, short), - ]; - expected.sort(); - assert_eq!(seed.names(&pool).await, expected); - assert!(expected.iter().all(|(_, name)| name.chars().count() <= 255)); + let new_name = format!("{}-{duplicate}", "a".repeat(218)); + assert_eq!( + rename_plan(&pool, &seed.host).await, + vec![(duplicate, long.clone(), new_name.clone())] + ); + assert!(new_name.chars().count() <= 255); drop_scratch_db(&admin, pool, &name).await; } +/// The real script, with a stand-in CLI that logs each call and applies the +/// rename. A name may hold any character but NUL, so a name holding control +/// characters must still yield exactly one rename, of its own channel. +#[cfg(unix)] #[tokio::test] #[ignore = "requires Postgres"] -async fn repair_leaves_a_row_that_stopped_being_a_duplicate_while_it_waited() { +async fn script_renames_each_planned_channel_once_whatever_its_name() { let admin = PgPool::connect(&admin_url().await).await.unwrap(); - let (pool, name) = create_scratch_db(&admin, "listed_name_repair_wait").await; + let (pool, name) = create_scratch_db(&admin, "listed_name_script").await; let seed = Seed::new(&pool).await; - let (kept, duplicate) = (Uuid::new_v4(), Uuid::new_v4()); - seed.open(&pool, kept, "payments", 30).await; - seed.open(&pool, duplicate, "payments", 10).await; - - // A relay write holds the duplicate's row while the script starts. - let mut relay = pool.begin().await.unwrap(); - sqlx::query("SELECT 1 FROM channels WHERE community_id = $1 AND id = $2 FOR NO KEY UPDATE") - .bind(seed.community) - .bind(duplicate) - .execute(&mut *relay) - .await - .unwrap(); - let repair_pool = PgPool::connect_with( - (*pool.connect_options()) - .clone() - .application_name("listed-name-repair"), - ) - .await - .unwrap(); - let repair = tokio::spawn({ - let repair_pool = repair_pool.clone(); - async move { run_repair(&repair_pool).await } - }); - crate::test_support::wait_for_lock_wait(&pool, "listed-name-repair").await; + let [kept, duplicate, other] = [(); 3].map(|_| Uuid::new_v4()); + let tricky = format!("ops\x1e{other}\x1fowned\x1f\n\t\"$(x)"); + seed.open(&pool, kept, &tricky, 30).await; + seed.open(&pool, duplicate, &tricky, 10).await; + seed.open(&pool, other, "other", 20).await; - // The relay makes the channel private and renames it, then commits. - sqlx::query( - "UPDATE channels SET name = 'payments-private', visibility = 'private' \ - WHERE community_id = $1 AND id = $2", + let dir = std::env::temp_dir().join(format!("listed-name-script-{}", Uuid::new_v4().simple())); + std::fs::create_dir(&dir).unwrap(); + let (cli, log) = (dir.join("buzz"), dir.join("calls")); + std::fs::write( + &cli, + "#!/usr/bin/env bash\nset -euo pipefail\nprintf '%s\\0' \"$@\" >>\"$STUB_LOG\"\n\ + psql \"$DATABASE_URL\" -X -q -v ON_ERROR_STOP=1 -v id=\"$4\" -v name=\"$6\" <<'SQL'\n\ + UPDATE channels SET name = :'name' WHERE id = :'id';\nSQL\n", ) - .bind(seed.community) - .bind(duplicate) - .execute(&mut *relay) - .await .unwrap(); - relay.commit().await.unwrap(); + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&cli, std::fs::Permissions::from_mode(0o755)).unwrap(); - assert_eq!(repair.await.unwrap(), 0); - let mut expected = vec![ - (kept, "payments".to_owned()), - (duplicate, "payments-private".to_owned()), - ]; - expected.sort(); - assert_eq!( - seed.names(&pool).await, - expected, - "the private row is not renamed" + let base = admin_url().await; + let database_url = format!("{}/{}", &base[..base.rfind('/').unwrap()], name); + let mut path = std::env::var("PATH").unwrap_or_default(); + if let Ok(bin) = std::env::var("PG_BIN_DIR") { + path = format!("{bin}:{path}"); + } + let output = std::process::Command::new(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../scripts/maintenance/rename_duplicate_listed_channel_names.sh" + )) + .arg(&seed.host) + .env("PATH", path) + .env("DATABASE_URL", &database_url) + .env("BUZZ_PRIVATE_KEY", "unused") + .env("BUZZ_CLI", &cli) + .env("STUB_LOG", &log) + .output() + .expect("run the rename script"); + assert!( + output.status.success(), + "script failed: {}", + String::from_utf8_lossy(&output.stderr) ); - repair_pool.close().await; + + let new_name = renamed(duplicate, &tricky).1; + let calls = std::fs::read(&log).unwrap(); + let args: Vec<&[u8]> = calls.split(|b| *b == 0).collect(); + let channel = duplicate.to_string(); + let expected: Vec<&[u8]> = vec![ + b"channels", + b"update", + b"--channel", + channel.as_bytes(), + b"--name", + new_name.as_bytes(), + b"", + ]; + assert_eq!(args, expected, "one call, for the duplicate only"); + let mut names = vec![ + (kept, tricky.clone()), + (duplicate, new_name.clone()), + (other, "other".to_owned()), + ]; + names.sort(); + assert_eq!(seed.names(&pool).await, names); + std::fs::remove_dir_all(&dir).unwrap(); drop_scratch_db(&admin, pool, &name).await; } diff --git a/crates/buzz-relay/src/api/admin/auth.rs b/crates/buzz-relay/src/api/admin/auth.rs index 89961eb1a95..b0ae99fc7bb 100644 --- a/crates/buzz-relay/src/api/admin/auth.rs +++ b/crates/buzz-relay/src/api/admin/auth.rs @@ -311,6 +311,18 @@ pub async fn resolve_admin_principal( .ok_or_else(ApiError::forbidden) } +/// True when `pubkey` holds the operator role (not moderator), with the same +/// precedence as [`lookup_admin_principal`]. A roster lookup failure is an +/// `Err`, never "not an operator". +pub(crate) async fn is_relay_operator( + state: &AppState, + pubkey: [u8; 32], +) -> Result { + Ok(lookup_admin_principal(state, pubkey) + .await? + .is_some_and(|p| p.role == AdminRole::Operator)) +} + /// Effective staff grant for `pubkey`, same precedence as /// [`resolve_admin_principal`]. `Ok(None)` means "not staff"; a roster lookup /// failure is an `Err`, so callers fail closed instead of treating it as diff --git a/crates/buzz-relay/src/api/admin/mod.rs b/crates/buzz-relay/src/api/admin/mod.rs index 15ad690bc77..84a59e917b6 100644 --- a/crates/buzz-relay/src/api/admin/mod.rs +++ b/crates/buzz-relay/src/api/admin/mod.rs @@ -40,6 +40,7 @@ pub(crate) fn is_admin_host(state: &crate::state::AppState, headers: &HeaderMap) /// [`auth::admin_api_origin`]). Re-exported so the NIP-11 builder can derive /// the advertised origin without reaching into the private `auth` module. pub(crate) use auth::admin_api_origin; +pub(crate) use auth::is_relay_operator; /// Build the deployment-admin routes. /// diff --git a/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs b/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs index fd1996340e2..587979c3e38 100644 --- a/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs @@ -8,7 +8,7 @@ use buzz_db::event::EventQuery; use nostr::{EventBuilder, Keys, Kind, Tag}; use uuid::Uuid; -use crate::handlers::ingest::{ingest_event, HttpAuthMethod, IngestAuth}; +use crate::handlers::ingest::{ingest_event, HttpAuthMethod, IngestAuth, IngestError}; use crate::state::AppState; use buzz_auth::Scope; @@ -885,79 +885,49 @@ async fn system_channel_defaults_private_and_hidden() { assert!(!has_marker(&tags, "hidden"), "{tags:?}"); } -/// A kind:9007 without an `h` tag is created by the side-effect handler, not -/// by the ingest pre-create. That path must give a system channel the same -/// private default, so strangers can neither read nor write it. -/// Mutation: default visibility to open in `handle_create_group` → RED. +/// A kind:9007 without an `h` tag is refused before it is stored. Its channel +/// would be created only after the event committed, so a refused create (a +/// taken name, for example) would still answer accepted. +/// Mutation: drop the `h`-tag refusal in `ingest_event` → RED. #[tokio::test] #[ignore = "requires Postgres"] -async fn system_channel_without_h_tag_defaults_private() { +async fn create_without_h_tag_is_refused_before_it_is_stored() { let state = state().await; - let tenant = community(&state, "group-state-system-no-h").await; + let tenant = community(&state, "group-state-create-no-h").await; let creator = Keys::generate(); - let stranger = Keys::generate(); - let name = format!("system-no-h-{}", Uuid::new_v4().simple()); + let name = format!("no-h-{}", Uuid::new_v4().simple()); let create = EventBuilder::new(Kind::Custom(9007), "") - .tags([ - Tag::parse(["name", &name]).unwrap(), - Tag::parse(["channel_type", "system"]).unwrap(), - ]) + .tags([Tag::parse(["name", &name]).unwrap()]) .sign_with_keys(&creator) .expect("sign 9007"); - let result = ingest_event(&state, &tenant, create, http_auth(&creator)) - .await - .unwrap_or_else(|error| panic!("create channel: {error:?}")); - assert!(result.accepted, "{}", result.message); - - let (channel_id, visibility): (Uuid, String) = sqlx::query_as( - "SELECT id, visibility::text FROM channels WHERE community_id = $1 AND name = $2", - ) - .bind(*tenant.community().as_uuid()) - .bind(&name) - .fetch_one(state.db.pool()) - .await - .expect("created channel"); - assert_eq!(visibility, "private"); - - let tags = group_state_tags(&state, &tenant, channel_id, 39000).await; - assert!(has_marker(&tags, "private"), "{tags:?}"); - assert!(has_marker(&tags, "hidden"), "{tags:?}"); - assert!(!has_marker(&tags, "public"), "{tags:?}"); + let event_id = create.id; + match ingest_event(&state, &tenant, create, http_auth(&creator)).await { + Err(IngestError::Rejected(message)) => assert_eq!( + message, + "invalid: create-group must include an h tag with the new channel id" + ), + Err(other) => panic!("expected a refusal, got {other:?}"), + Ok(result) => panic!("expected a refusal, got {:?}", result.message), + } - let stranger_bytes = stranger.public_key().to_bytes().to_vec(); + let channels: i64 = + sqlx::query_scalar("SELECT count(*) FROM channels WHERE community_id = $1 AND name = $2") + .bind(*tenant.community().as_uuid()) + .bind(&name) + .fetch_one(state.db.pool()) + .await + .unwrap(); + assert_eq!(channels, 0, "no channel is created"); assert!( - crate::handlers::ingest::check_channel_membership( - tenant.community(), - &state, - channel_id, - &stranger_bytes, - None, - crate::handlers::ingest::ChannelWrite::Membership, - ) - .await - .is_err(), - "a stranger must not pass the channel access check" + state + .db + .get_event_by_id(tenant.community(), event_id.as_bytes()) + .await + .unwrap() + .is_none(), + "the refused create is not stored" ); - let readable = state - .db - .get_accessible_channel_ids(tenant.community(), &stranger_bytes) - .await - .expect("accessible channels"); - assert!(!readable.contains(&channel_id), "stranger can read it"); - - let message = EventBuilder::new(Kind::Custom(9), "hello") - .tags([Tag::parse(["h", &channel_id.to_string()]).unwrap()]) - .sign_with_keys(&stranger) - .unwrap(); - match ingest_event(&state, &tenant, message, http_auth(&stranger)).await { - Ok(result) => assert!( - !result.accepted, - "stranger wrote to a private system channel: {}", - result.message - ), - Err(error) => assert!(format!("{error:?}").contains("restricted"), "{error:?}"), - } } /// A removed creator's kind:9021 that passes the open-channel check while the @@ -1082,8 +1052,6 @@ async fn removed_creator_join_racing_a_private_switch_is_refused() { #[tokio::test] #[ignore = "requires Postgres"] async fn a_taken_open_name_is_refused_as_a_client_error() { - use crate::handlers::ingest::IngestError; - let state = state().await; let tenant = community(&state, "group-state-name-taken").await; let owner = Keys::generate(); @@ -1168,3 +1136,86 @@ async fn a_taken_open_name_is_refused_as_a_client_error() { .expect("look up the 9002"); assert!(!stored, "the refused 9002 is not stored"); } + +/// A relay operator who is not a channel member may rename the channel, and +/// the relay publishes the new name in kind:39000. The operator may change +/// nothing else, and a relay moderator may not rename it. +/// Mutation: drop the operator branch in `validate_admin_event`, or drop its +/// `is_rename_only` or operator-role condition → RED. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn a_relay_operator_may_only_rename_a_channel() { + let state = state().await; + let tenant = community(&state, "group-state-operator-rename").await; + let owner = Keys::generate(); + let channel = create_channel(&state, &tenant, &owner, &[]).await; + let operator = Keys::generate(); + let moderator = Keys::generate(); + for (keys, role) in [(&operator, "operator"), (&moderator, "moderator")] { + state + .db + .upsert_relay_operator( + &keys.public_key().to_bytes(), + role, + &owner.public_key().to_bytes(), + true, + ) + .await + .expect("grant relay role"); + } + let edit = |keys: &Keys, tags: &[&[&str]]| { + let mut all = vec![Tag::parse(["h", &channel.to_string()]).unwrap()]; + all.extend( + tags.iter() + .map(|tag| Tag::parse(tag.iter().copied()).unwrap()), + ); + EventBuilder::new(Kind::Custom(9002), "") + .tags(all) + .sign_with_keys(keys) + .expect("sign 9002") + }; + let community_id = tenant.community(); + let name = |state: Arc| async move { + state + .db + .get_channel(community_id, channel) + .await + .expect("channel") + .name + }; + let before = name(state.clone()).await; + + for (keys, tags) in [ + (&moderator, &[&["name", "by-moderator"][..]][..]), + ( + &operator, + &[&["name", "by-operator"][..], &["about", "changed"][..]][..], + ), + ] { + match ingest_event(&state, &tenant, edit(keys, tags), http_auth(keys)).await { + Err(IngestError::Rejected(message)) => { + assert!(message.contains("not authorized"), "{message}") + } + Err(other) => panic!("expected a refusal, got {other:?}"), + Ok(result) => panic!("expected a refusal, got {:?}", result.message), + } + assert_eq!(name(state.clone()).await, before); + } + + let renamed = ingest_event( + &state, + &tenant, + edit(&operator, &[&["name", "renamed-by-operator"]]), + http_auth(&operator), + ) + .await + .unwrap_or_else(|error| panic!("operator rename: {error:?}")); + assert!(renamed.accepted, "{}", renamed.message); + assert_eq!(name(state.clone()).await, "renamed-by-operator"); + let tags = group_state_tags(&state, &tenant, channel, 39000).await; + assert!( + tags.iter() + .any(|tag| tag.len() >= 2 && tag[0] == "name" && tag[1] == "renamed-by-operator"), + "{tags:?}" + ); +} diff --git a/crates/buzz-relay/src/handlers/ingest.rs b/crates/buzz-relay/src/handlers/ingest.rs index d394558253f..c02d6627737 100644 --- a/crates/buzz-relay/src/handlers/ingest.rs +++ b/crates/buzz-relay/src/handlers/ingest.rs @@ -3213,8 +3213,9 @@ async fn ingest_event_inner( .map_err(|e| IngestError::Rejected(format!("invalid: {e}")))?; } - // A kind:9007 with an `h` tag creates its channel when it is stored, in the - // same transaction (see the storage dispatch below). + // A kind:9007 must carry the new channel's id in its `h` tag. It creates + // its channel when it is stored, in the same transaction (see the storage + // dispatch below). let mut channel_create: Option = None; if kind_u32 == KIND_NIP29_CREATE_GROUP { @@ -3291,6 +3292,12 @@ async fn ingest_event_inner( unlisted, members, }); + } else { + // The channel is created in the same commit as its event, so a + // refused create (a taken name, for example) reaches the client. + return Err(IngestError::Rejected( + "invalid: create-group must include an h tag with the new channel id".into(), + )); } } diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index 8f2977bb04e..a0094b147c9 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -858,6 +858,16 @@ pub async fn validate_admin_event( { return Ok(()); } + // A relay operator may rename any channel, so that + // duplicate names can be repaired through this path + // (scripts/maintenance/rename_duplicate_listed_channel_names.sh). + // Every other privileged change still needs a channel + // owner or admin. + if is_rename_only(event) + && actor_is_relay_operator(state, event.pubkey.to_bytes()).await? + { + return Ok(()); + } Err(anyhow::anyhow!( "actor not authorized for name/about/archived/visibility/ttl/label/join_role/unlisted changes" )) @@ -1297,6 +1307,39 @@ async fn emit_typed_membership_notification( Ok(()) } +/// True when the only metadata a kind:9002 changes is the channel name. +fn is_rename_only(event: &Event) -> bool { + const METADATA_TAGS: &[&str] = &[ + "name", + "about", + "archived", + "topic", + "purpose", + "visibility", + "ttl", + "t", + "join_role", + "unlisted", + ]; + let mut changes = event + .tags + .iter() + .map(|t| t.kind().to_string()) + .filter(|kind| METADATA_TAGS.contains(&kind.as_str())); + changes.all(|kind| kind == "name") && event.tags.iter().any(|t| t.kind().to_string() == "name") +} + +/// True when `pubkey` holds the relay operator role. A roster lookup failure +/// is an error, never "not an operator". +async fn actor_is_relay_operator( + state: &crate::state::AppState, + pubkey: [u8; 32], +) -> anyhow::Result { + crate::api::admin::is_relay_operator(state, pubkey) + .await + .map_err(|e| anyhow::anyhow!("relay staff lookup failed: {}", e.message)) +} + /// Sign, store (replacing previous), and fan-out a single addressable discovery event. async fn emit_addressable_discovery_event( tenant: &TenantContext, @@ -2508,17 +2551,14 @@ async fn handle_create_group( .unwrap_or_default(); let unlisted = create_unlisted_from_tags(event)?; - // If the event has an h-tag UUID, ingest_event() already created the channel - // via create_channel_with_id(). Fetch it rather than creating a duplicate. - // If no h-tag, fall back to the original auto-UUID creation path. + // Ingest already created the channel named by the h tag, in the same + // commit as this event. Fetch it rather than creating a duplicate. // - // Double-count analysis (C5): the counter increments below do NOT - // double-count vs. ingest.rs. For the h-tag path, ingest increments on - // was_created=true and this handler only reaches create_channel() on a DB - // lookup Err — an error recovery path where ingest's channel is - // inaccessible, so the counter correctly records a new creation. For the - // no-h-tag path, ingest never creates the channel, so this is the sole - // increment. + // Double-count analysis (C5): the counter increment below does NOT + // double-count vs. ingest.rs. Ingest increments on was_created=true and + // this handler only reaches create_channel() on a DB lookup Err — an + // error recovery path where ingest's channel is inaccessible, so the + // counter correctly records a new creation. // `members_added`: ingest's create also added the event's `p` tags. let (channel, members_added) = if let Some(client_uuid) = extract_h_tag_channel(event) { match state @@ -2554,27 +2594,8 @@ async fn handle_create_group( } } } else { - let ch = state - .db - .create_labeled_channel( - tenant.community(), - &name, - channel_type, - visibility, - description.as_deref(), - &actor_bytes, - ttl_seconds, - &labels, - unlisted, - ) - .await?; - metrics::counter!( - "buzz_channels_created_total", - "community" => tenant.host().to_owned(), - "type" => channel_type.to_string() - ) - .increment(1); - (ch, false) + // Ingest refuses a kind:9007 without an `h` tag before storing it. + anyhow::bail!("missing or invalid h tag"); }; // Creator becomes owner — evict any stale negative membership lookup. diff --git a/scripts/maintenance/plan_duplicate_listed_channel_renames.sql b/scripts/maintenance/plan_duplicate_listed_channel_renames.sql new file mode 100644 index 00000000000..f0d681f1a0f --- /dev/null +++ b/scripts/maintenance/plan_duplicate_listed_channel_renames.sql @@ -0,0 +1,46 @@ +-- OUT-OF-BAND MAINTENANCE, read-only. rename_duplicate_listed_channel_names.sh +-- runs this query and sends each rename it lists to the relay. +-- +-- Lists the renames that give every listed channel in one community a name +-- that no other listed channel there has, ignoring case. A listed channel is +-- an open, not unlisted stream or forum that is not deleted; an archived one +-- still counts. This is the rule the relay enforces on create and rename +-- (`LISTED_CHANNEL_SQL` in crates/buzz-db/src/store/channel.rs). +-- +-- In each group of channels that share a name, one channel keeps it: the +-- oldest unarchived one, or the oldest archived one when all are archived. +-- Every other channel in the group gets "-", or "-" when another listed channel already has that name. +-- It lists nothing for a community that is not active. +-- +-- psql -X -v ON_ERROR_STOP=1 -v host= -f scripts/maintenance/plan_duplicate_listed_channel_renames.sql + +WITH listed AS ( + SELECT ch.id, ch.name, ch.archived_at, ch.created_at + FROM channels ch + JOIN communities co ON co.id = ch.community_id + WHERE co.host = :'host' + AND co.deletion_state = 'active' AND co.deleted_at IS NULL + AND ch.visibility = 'open' AND NOT ch.unlisted + AND ch.channel_type IN ('stream', 'forum') AND ch.deleted_at IS NULL +), +duplicates AS ( + SELECT id, name, + left(name, 246) || '-' || left(id::text, 8) AS short_name, + left(name, 218) || '-' || id::text AS long_name + FROM ( + SELECT id, name, row_number() OVER ( + PARTITION BY lower(name) + ORDER BY archived_at IS NOT NULL, created_at, id + ) AS place + FROM listed + ) ranked + WHERE place > 1 +) +SELECT d.id AS channel_id, d.name, + CASE WHEN EXISTS ( + SELECT 1 FROM listed other + WHERE other.id <> d.id AND lower(other.name) = lower(d.short_name) + ) THEN d.long_name ELSE d.short_name END AS new_name +FROM duplicates d +ORDER BY d.id; diff --git a/scripts/maintenance/rename_duplicate_listed_channel_names.sh b/scripts/maintenance/rename_duplicate_listed_channel_names.sh new file mode 100755 index 00000000000..bc56cd8c205 --- /dev/null +++ b/scripts/maintenance/rename_duplicate_listed_channel_names.sh @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# OUT-OF-BAND MAINTENANCE: renames listed channels that share a name in one +# community, so the contract migration's unique index can build. +# +# It reads the renames from plan_duplicate_listed_channel_renames.sql and sends +# each one to the relay as a kind:9002 rename, signed by a relay operator. The +# relay applies a rename the way it applies any other: it refuses a name that +# another listed channel has, and publishes the new kind:39000, so clients +# show the new name. Relays keep serving while it runs. +# +# Run it only after every serving relay enforces the name rule and accepts an +# operator rename. An older relay refuses the rename and can create new +# duplicates. +# +# A rename that fails is reported and skipped: the relay refused it (for +# example, because a channel took the new name meanwhile), or the CLI could +# not reach the relay. At the end the script reads the plan again and fails +# if any rename remains; run it again. +# +# Usage: +# DATABASE_URL=postgres://... BUZZ_PRIVATE_KEY= \ +# scripts/maintenance/rename_duplicate_listed_channel_names.sh [--dry-run] +# +# BUZZ_PRIVATE_KEY must belong to a relay operator (RELAY_OPERATOR_PUBKEYS, or +# an `operator` row in relay_operators), not a moderator. BUZZ_RELAY_URL +# defaults to wss://. BUZZ_CLI defaults to `buzz`. +set -euo pipefail + +usage() { + echo "usage: $0 [--dry-run] " >&2 + exit 2 +} + +dry_run=false +if [[ ${1:-} == --dry-run ]]; then + dry_run=true + shift +fi +[[ $# -eq 1 ]] || usage +host=$1 +: "${DATABASE_URL:?set DATABASE_URL}" +if ! $dry_run; then + : "${BUZZ_PRIVATE_KEY:?set BUZZ_PRIVATE_KEY to a relay operator key}" +fi +relay_url=${BUZZ_RELAY_URL:-wss://$host} +buzz=${BUZZ_CLI:-buzz} +plan_sql="$(cd "$(dirname "$0")" && pwd)/plan_duplicate_listed_channel_renames.sql" + +plan=$(mktemp) +trap 'rm -f "$plan"' EXIT + +# Every field ends with a NUL byte. Postgres text cannot hold one, so any +# name, with any control character in it, stays one field. +read_plan() { + psql "$DATABASE_URL" -X -q -v ON_ERROR_STOP=1 -v host="$host" \ + -A -t -z -0 -f "$plan_sql" >"$plan" +} + +read_plan +# The plan is the loop's stdin, so the CLI reads /dev/null instead. +renamed=0 +failed=0 +while IFS= read -r -d '' channel && IFS= read -r -d '' name && + IFS= read -r -d '' new_name; do + if $dry_run; then + printf 'would rename %s from %q to %q\n' "$channel" "$name" "$new_name" + elif BUZZ_RELAY_URL=$relay_url "$buzz" channels update \ + --channel "$channel" --name "$new_name" /dev/null; then + printf 'renamed %s from %q to %q\n' "$channel" "$name" "$new_name" + renamed=$((renamed + 1)) + else + printf 'could not rename %s from %q to %q\n' \ + "$channel" "$name" "$new_name" >&2 + failed=$((failed + 1)) + fi +done <"$plan" + +$dry_run && exit 0 + +read_plan +# Three fields, so three NUL bytes, for each rename. +remaining=$(($(tr -cd '\000' <"$plan" | wc -c) / 3)) +echo "renamed $renamed, failed $failed, remaining $remaining" +if [[ $remaining -ne 0 ]]; then + echo "duplicate listed channel names remain in $host; run the script again" >&2 + exit 1 +fi diff --git a/scripts/maintenance/rename_duplicate_listed_channel_names.sql b/scripts/maintenance/rename_duplicate_listed_channel_names.sql deleted file mode 100644 index 0264031bb07..00000000000 --- a/scripts/maintenance/rename_duplicate_listed_channel_names.sql +++ /dev/null @@ -1,161 +0,0 @@ --- OUT-OF-BAND MAINTENANCE: do not run from relay startup migrations. --- --- Gives every listed open channel a name that no other listed open channel in --- its community has, ignoring case. A listed channel is an open, not unlisted --- stream or forum that is not deleted; an archived one still counts. This is --- the rule the relay enforces on create and rename (`LISTED_CHANNEL_SQL` in --- crates/buzz-db/src/store/channel.rs). It repairs the duplicates created --- before that rule, so the contract migration's unique index can build. --- --- Run it only after every serving relay enforces the rule. An older relay can --- create a new duplicate while the script runs. --- --- In each group of channels that share a name, one channel keeps it: the --- oldest unarchived one, or the oldest archived one when all are archived. --- Every other channel in the group becomes "-", or "-" when another listed channel already has that name. --- The script prints a NOTICE for each rename. When both names are taken, it --- leaves the channel unchanged with a NOTICE, and the validation below --- reports it. --- --- The rename writes only the channel row. Clients read the name from the --- relay-signed kind:39000, so republish it for each renamed channel: --- --- buzz-admin reconcile-channels --channel --metadata --- --- Each batch renames up to 100 channels in one community and commits. It --- takes the community deletion lock first and skips a community that is not --- active. It locks every row of the batch before it takes the relay's name --- lock for any new name, in the order the relay takes them. After it locks a --- row, it checks again that the row is still a listed duplicate, because a --- relay may have changed it meanwhile. The script is idempotent: a second run --- finds nothing to rename. --- --- Run it outside a transaction, so that each batch can commit: --- --- psql -v ON_ERROR_STOP=1 -f scripts/maintenance/rename_duplicate_listed_channel_names.sql - -DO $$ -DECLARE - target UUID; - skipped UUID[] := '{}'; - batch UUID[]; - duplicate UUID; - live RECORD; - new_name TEXT; - renamed INT; -BEGIN - LOOP - SELECT ch.community_id INTO target - FROM channels ch - WHERE ch.visibility = 'open' AND NOT ch.unlisted - AND ch.channel_type IN ('stream', 'forum') AND ch.deleted_at IS NULL - AND ch.community_id <> ALL (skipped) - GROUP BY ch.community_id, lower(ch.name) - HAVING count(*) > 1 - LIMIT 1; - EXIT WHEN target IS NULL; - - PERFORM set_config('lock_timeout', '5s', true); - PERFORM pg_advisory_xact_lock_shared(community_deletion_lock_key(target)); - IF NOT EXISTS ( - SELECT 1 FROM communities - WHERE id = target AND deletion_state = 'active' AND deleted_at IS NULL - ) THEN - RAISE NOTICE 'skipped community %: it is not active', target; - skipped := skipped || target; - COMMIT; - CONTINUE; - END IF; - - -- Lock every row of the batch before any name lock, in the order - -- the relay takes them. - SELECT array_agg(locked.id ORDER BY locked.id) INTO batch - FROM ( - WITH ranked AS ( - SELECT id, row_number() OVER ( - PARTITION BY lower(name) - ORDER BY archived_at IS NOT NULL, created_at, id - ) AS place - FROM channels - WHERE community_id = target - AND visibility = 'open' AND NOT unlisted - AND channel_type IN ('stream', 'forum') AND deleted_at IS NULL - ) - SELECT ch.id - FROM channels ch - JOIN ranked ON ranked.id = ch.id - WHERE ch.community_id = target AND ranked.place > 1 - ORDER BY ch.id - LIMIT 100 - FOR NO KEY UPDATE OF ch - ) locked; - - renamed := 0; - FOREACH duplicate IN ARRAY coalesce(batch, '{}') - LOOP - -- The ranking used the statement's snapshot. Now that the row is - -- locked, read it again: a relay may have renamed it, made it - -- private, or renamed the channel that kept the name. - SELECT ch.name, ch.archived_at, ch.created_at INTO live - FROM channels ch - WHERE ch.community_id = target AND ch.id = duplicate - AND ch.visibility = 'open' AND NOT ch.unlisted - AND ch.channel_type IN ('stream', 'forum') AND ch.deleted_at IS NULL; - CONTINUE WHEN NOT FOUND OR NOT EXISTS ( - SELECT 1 FROM channels other - WHERE other.community_id = target AND other.id <> duplicate - AND lower(other.name) = lower(live.name) - AND other.visibility = 'open' AND NOT other.unlisted - AND other.channel_type IN ('stream', 'forum') AND other.deleted_at IS NULL - AND (other.archived_at IS NOT NULL, other.created_at, other.id) - < (live.archived_at IS NOT NULL, live.created_at, duplicate) - ); - - new_name := NULL; - FOREACH new_name IN ARRAY ARRAY[ - left(live.name, 246) || '-' || left(duplicate::text, 8), - left(live.name, 218) || '-' || duplicate::text - ] LOOP - PERFORM pg_advisory_xact_lock(hashtextextended( - 'buzz_channel_name:' || target::text || ':' || lower(new_name), 0)); - EXIT WHEN NOT EXISTS ( - SELECT 1 FROM channels other - WHERE other.community_id = target AND other.id <> duplicate - AND lower(other.name) = lower(new_name) - AND other.visibility = 'open' AND NOT other.unlisted - AND other.channel_type IN ('stream', 'forum') AND other.deleted_at IS NULL - ); - new_name := NULL; - END LOOP; - IF new_name IS NULL THEN - RAISE NOTICE 'left channel % in community % as "%": both new names are taken', - duplicate, target, live.name; - CONTINUE; - END IF; - - UPDATE channels SET name = new_name, updated_at = NOW() - WHERE community_id = target AND id = duplicate; - RAISE NOTICE 'renamed channel % in community % from "%" to "%"', - duplicate, target, live.name, new_name; - renamed := renamed + 1; - END LOOP; - - -- A batch that found nothing to rename would find the same community - -- again; skip it so the loop ends. The validation below reports it. - IF renamed = 0 THEN - skipped := skipped || target; - END IF; - COMMIT; - END LOOP; -END -$$; - --- Validation. Expected result: zero rows. A row names a community the script --- skipped (not active) or a duplicate created while it ran; rerun the script. -SELECT community_id, lower(name) AS name, count(*) AS channels -FROM channels -WHERE visibility = 'open' AND NOT unlisted - AND channel_type IN ('stream', 'forum') AND deleted_at IS NULL -GROUP BY community_id, lower(name) -HAVING count(*) > 1; From f3cd64179cd0ab09eaa3195a6d9ea847a4e22036 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Fri, 30 Oct 2026 17:06:39 -0400 Subject: [PATCH 03/13] fix(scripts): pass rename values as --name=value clap reads a separate value that starts with "-" as a flag, so a duplicate named "-ops" failed to rename on every run. Attach each value with "=" and make the test's stand-in CLI accept only that form, with a name that starts with "-". Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- .../tests/listed_channel_name_postgres_tests.rs | 15 ++++++++------- .../rename_duplicate_listed_channel_names.sh | 3 ++- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs index 291382cca13..60d96a505d6 100644 --- a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs +++ b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs @@ -194,7 +194,8 @@ async fn plan_picks_a_free_name_when_the_short_one_is_taken() { /// The real script, with a stand-in CLI that logs each call and applies the /// rename. A name may hold any character but NUL, so a name holding control -/// characters must still yield exactly one rename, of its own channel. +/// characters must still yield exactly one rename, of its own channel. Like +/// clap, the stand-in takes a value that starts with "-" only as `--name=`. #[cfg(unix)] #[tokio::test] #[ignore = "requires Postgres"] @@ -203,7 +204,7 @@ async fn script_renames_each_planned_channel_once_whatever_its_name() { let (pool, name) = create_scratch_db(&admin, "listed_name_script").await; let seed = Seed::new(&pool).await; let [kept, duplicate, other] = [(); 3].map(|_| Uuid::new_v4()); - let tricky = format!("ops\x1e{other}\x1fowned\x1f\n\t\"$(x)"); + let tricky = format!("-ops\x1e{other}\x1fowned\x1f\n\t\"$(x)"); seed.open(&pool, kept, &tricky, 30).await; seed.open(&pool, duplicate, &tricky, 10).await; seed.open(&pool, other, "other", 20).await; @@ -214,7 +215,8 @@ async fn script_renames_each_planned_channel_once_whatever_its_name() { std::fs::write( &cli, "#!/usr/bin/env bash\nset -euo pipefail\nprintf '%s\\0' \"$@\" >>\"$STUB_LOG\"\n\ - psql \"$DATABASE_URL\" -X -q -v ON_ERROR_STOP=1 -v id=\"$4\" -v name=\"$6\" <<'SQL'\n\ + [[ $3 == --channel=* && $4 == --name=* ]] || exit 2\n\ + psql \"$DATABASE_URL\" -X -q -v ON_ERROR_STOP=1 -v id=\"${3#*=}\" -v name=\"${4#*=}\" <<'SQL'\n\ UPDATE channels SET name = :'name' WHERE id = :'id';\nSQL\n", ) .unwrap(); @@ -248,14 +250,13 @@ async fn script_renames_each_planned_channel_once_whatever_its_name() { let new_name = renamed(duplicate, &tricky).1; let calls = std::fs::read(&log).unwrap(); let args: Vec<&[u8]> = calls.split(|b| *b == 0).collect(); - let channel = duplicate.to_string(); + let channel = format!("--channel={duplicate}"); + let name_arg = format!("--name={new_name}"); let expected: Vec<&[u8]> = vec![ b"channels", b"update", - b"--channel", channel.as_bytes(), - b"--name", - new_name.as_bytes(), + name_arg.as_bytes(), b"", ]; assert_eq!(args, expected, "one call, for the duplicate only"); diff --git a/scripts/maintenance/rename_duplicate_listed_channel_names.sh b/scripts/maintenance/rename_duplicate_listed_channel_names.sh index bc56cd8c205..314e3ff3c03 100755 --- a/scripts/maintenance/rename_duplicate_listed_channel_names.sh +++ b/scripts/maintenance/rename_duplicate_listed_channel_names.sh @@ -62,10 +62,11 @@ renamed=0 failed=0 while IFS= read -r -d '' channel && IFS= read -r -d '' name && IFS= read -r -d '' new_name; do + # --name=value, so a name that starts with "-" is not read as a flag. if $dry_run; then printf 'would rename %s from %q to %q\n' "$channel" "$name" "$new_name" elif BUZZ_RELAY_URL=$relay_url "$buzz" channels update \ - --channel "$channel" --name "$new_name" /dev/null; then + --channel="$channel" --name="$new_name" /dev/null; then printf 'renamed %s from %q to %q\n' "$channel" "$name" "$new_name" renamed=$((renamed + 1)) else From 6da124914440de32efa2cea78aac30b998100d72 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Fri, 30 Oct 2026 17:16:08 -0400 Subject: [PATCH 04/13] fix(relay): let an operator rename an archived channel The relay refused every edit of an archived channel unless it also unarchived it, so the rename script could not repair an archived duplicate such as the old square-ios copy, and the unique index could not be built. A relay operator's name-only rename now applies to an archived channel and leaves it archived. Other edits of an archived channel are still refused. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- crates/buzz-db/src/store/channel.rs | 5 +++ .../handlers/group_state_postgres_tests.rs | 31 +++++++++++++++++++ .../buzz-relay/src/handlers/side_effects.rs | 4 +++ 3 files changed, 40 insertions(+) diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index f376419a1ff..06122786bd2 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -1496,6 +1496,9 @@ pub struct ChannelMetadataEdit { pub purpose: Option, /// `Some(true)` archives the channel; `Some(false)` restores it. pub archived: Option, + /// Lets this edit change an archived channel, which stays archived. Only + /// a relay operator's name-only rename sets it. + pub keep_archived: bool, } /// The result of [`Db::apply_channel_metadata_event`]. @@ -1553,6 +1556,7 @@ pub(crate) async fn apply_channel_metadata_event( // channel already in the requested state. match (locked.archived(), edit.archived) { (true, Some(false)) | (false, None | Some(true)) => {} + (true, None) if edit.keep_archived => {} (true, _) => return Err(DbError::AccessDenied("channel is archived".to_string())), (false, Some(false)) => { return Err(DbError::AccessDenied("channel is not archived".to_string())); @@ -1588,6 +1592,7 @@ pub(crate) async fn apply_channel_metadata_event( topic, purpose, archived, + keep_archived: _, } = edit; let actor = event.pubkey.to_bytes(); if !columns.is_empty() { diff --git a/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs b/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs index 587979c3e38..aa9d35487bf 100644 --- a/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs @@ -1202,6 +1202,27 @@ async fn a_relay_operator_may_only_rename_a_channel() { assert_eq!(name(state.clone()).await, before); } + // The rename repairs archived duplicates too, so archive the channel. An + // owner still may not edit it while it is archived. + state + .db + .archive_channel(community_id, channel) + .await + .expect("archive"); + match ingest_event( + &state, + &tenant, + edit(&owner, &[&["name", "by-owner"]]), + http_auth(&owner), + ) + .await + { + Err(IngestError::Rejected(message)) => assert!(message.contains("archived"), "{message}"), + Err(other) => panic!("expected a refusal, got {other:?}"), + Ok(result) => panic!("expected a refusal, got {:?}", result.message), + } + assert_eq!(name(state.clone()).await, before); + let renamed = ingest_event( &state, &tenant, @@ -1212,6 +1233,16 @@ async fn a_relay_operator_may_only_rename_a_channel() { .unwrap_or_else(|error| panic!("operator rename: {error:?}")); assert!(renamed.accepted, "{}", renamed.message); assert_eq!(name(state.clone()).await, "renamed-by-operator"); + assert!( + state + .db + .get_channel(community_id, channel) + .await + .expect("channel") + .archived_at + .is_some(), + "the rename keeps the channel archived" + ); let tags = group_state_tags(&state, &tenant, channel, 39000).await; assert!( tags.iter() diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index a0094b147c9..107af84f6a7 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -2217,6 +2217,10 @@ pub(crate) async fn persist_edit_metadata( if let Some(labels) = channel_labels_from_tags(event) { edit.columns.labels = Some(labels.map_err(|e| anyhow::anyhow!("{e}"))?); } + // An operator's rename may repair an archived duplicate name, so it + // applies to an archived channel and leaves it archived. + edit.keep_archived = + is_rename_only(event) && actor_is_relay_operator(state, event.pubkey.to_bytes()).await?; let now = chrono::Utc::now(); let mut notices = Vec::new(); From 2a9d4578d291befafabbb9facd8a7ce2e4e80948 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Fri, 30 Oct 2026 17:59:50 -0400 Subject: [PATCH 05/13] fix(relay): limit the operator rename to listed channels A relay operator's name-only kind:9002 now applies only to a listed channel (open, not unlisted, stream or forum, not deleted), the only channels whose names must be unique. A private channel, an unlisted channel and a DM need a channel owner or admin, as before. The relay looks up the operator only for a rename of an archived channel, and the kind:9002 tag lists now have one definition. The rename script checks the community first: it stops for an archived community, one being deleted, or an unknown host, and says what to do. It stops at the first auth refusal (CLI exit code 3), and its header says the operator key must be a member of the community, because POST /events checks membership first. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- .../listed_channel_name_postgres_tests.rs | 158 ++++++++++++++---- .../handlers/group_state_postgres_tests.rs | 67 ++++++++ .../buzz-relay/src/handlers/side_effects.rs | 117 +++++++------ .../rename_duplicate_listed_channel_names.sh | 61 ++++++- 4 files changed, 313 insertions(+), 90 deletions(-) diff --git a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs index 60d96a505d6..5bfcc5539ea 100644 --- a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs +++ b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs @@ -192,39 +192,30 @@ async fn plan_picks_a_free_name_when_the_short_one_is_taken() { drop_scratch_db(&admin, pool, &name).await; } -/// The real script, with a stand-in CLI that logs each call and applies the -/// rename. A name may hold any character but NUL, so a name holding control -/// characters must still yield exactly one rename, of its own channel. Like -/// clap, the stand-in takes a value that starts with "-" only as `--name=`. +/// Runs the real script for `host` against database `db`, with `cli_body` as +/// the stand-in CLI. Returns the script's output and each CLI call's +/// arguments. #[cfg(unix)] -#[tokio::test] -#[ignore = "requires Postgres"] -async fn script_renames_each_planned_channel_once_whatever_its_name() { - let admin = PgPool::connect(&admin_url().await).await.unwrap(); - let (pool, name) = create_scratch_db(&admin, "listed_name_script").await; - let seed = Seed::new(&pool).await; - let [kept, duplicate, other] = [(); 3].map(|_| Uuid::new_v4()); - let tricky = format!("-ops\x1e{other}\x1fowned\x1f\n\t\"$(x)"); - seed.open(&pool, kept, &tricky, 30).await; - seed.open(&pool, duplicate, &tricky, 10).await; - seed.open(&pool, other, "other", 20).await; - +async fn run_rename_script( + host: &str, + db: &str, + cli_body: &str, +) -> (std::process::Output, Vec>>) { let dir = std::env::temp_dir().join(format!("listed-name-script-{}", Uuid::new_v4().simple())); std::fs::create_dir(&dir).unwrap(); let (cli, log) = (dir.join("buzz"), dir.join("calls")); std::fs::write( &cli, - "#!/usr/bin/env bash\nset -euo pipefail\nprintf '%s\\0' \"$@\" >>\"$STUB_LOG\"\n\ - [[ $3 == --channel=* && $4 == --name=* ]] || exit 2\n\ - psql \"$DATABASE_URL\" -X -q -v ON_ERROR_STOP=1 -v id=\"${3#*=}\" -v name=\"${4#*=}\" <<'SQL'\n\ - UPDATE channels SET name = :'name' WHERE id = :'id';\nSQL\n", + format!( + "#!/usr/bin/env bash\nset -euo pipefail\nprintf '%s\\0' \"$@\" '' >>\"$STUB_LOG\"\n{cli_body}" + ), ) .unwrap(); use std::os::unix::fs::PermissionsExt; std::fs::set_permissions(&cli, std::fs::Permissions::from_mode(0o755)).unwrap(); let base = admin_url().await; - let database_url = format!("{}/{}", &base[..base.rfind('/').unwrap()], name); + let database_url = format!("{}/{}", &base[..base.rfind('/').unwrap()], db); let mut path = std::env::var("PATH").unwrap_or_default(); if let Ok(bin) = std::env::var("PG_BIN_DIR") { path = format!("{bin}:{path}"); @@ -233,7 +224,7 @@ async fn script_renames_each_planned_channel_once_whatever_its_name() { env!("CARGO_MANIFEST_DIR"), "/../../scripts/maintenance/rename_duplicate_listed_channel_names.sh" )) - .arg(&seed.host) + .arg(host) .env("PATH", path) .env("DATABASE_URL", &database_url) .env("BUZZ_PRIVATE_KEY", "unused") @@ -241,6 +232,45 @@ async fn script_renames_each_planned_channel_once_whatever_its_name() { .env("STUB_LOG", &log) .output() .expect("run the rename script"); + + // Each call ends with an empty field, so "\0\0" splits the calls. + let calls = std::fs::read(&log).unwrap_or_default(); + let calls = calls + .split(|b| *b == 0) + .collect::>() + .split(|field| field.is_empty()) + .filter(|call| !call.is_empty()) + .map(|call| call.iter().map(|field| field.to_vec()).collect()) + .collect(); + std::fs::remove_dir_all(&dir).unwrap(); + (output, calls) +} + +/// The real script, with a stand-in CLI that logs each call and applies the +/// rename. A name may hold any character but NUL, so a name holding control +/// characters must still yield exactly one rename, of its own channel. Like +/// clap, the stand-in takes a value that starts with "-" only as `--name=`. +#[cfg(unix)] +#[tokio::test] +#[ignore = "requires Postgres"] +async fn script_renames_each_planned_channel_once_whatever_its_name() { + let admin = PgPool::connect(&admin_url().await).await.unwrap(); + let (pool, name) = create_scratch_db(&admin, "listed_name_script").await; + let seed = Seed::new(&pool).await; + let [kept, duplicate, other] = [(); 3].map(|_| Uuid::new_v4()); + let tricky = format!("-ops\x1e{other}\x1fowned\x1f\n\t\"$(x)"); + seed.open(&pool, kept, &tricky, 30).await; + seed.open(&pool, duplicate, &tricky, 10).await; + seed.open(&pool, other, "other", 20).await; + + let (output, calls) = run_rename_script( + &seed.host, + &name, + "[[ $3 == --channel=* && $4 == --name=* ]] || exit 2\n\ + psql \"$DATABASE_URL\" -X -q -v ON_ERROR_STOP=1 -v id=\"${3#*=}\" -v name=\"${4#*=}\" <<'SQL'\n\ + UPDATE channels SET name = :'name' WHERE id = :'id';\nSQL\n", + ) + .await; assert!( output.status.success(), "script failed: {}", @@ -248,18 +278,13 @@ async fn script_renames_each_planned_channel_once_whatever_its_name() { ); let new_name = renamed(duplicate, &tricky).1; - let calls = std::fs::read(&log).unwrap(); - let args: Vec<&[u8]> = calls.split(|b| *b == 0).collect(); - let channel = format!("--channel={duplicate}"); - let name_arg = format!("--name={new_name}"); - let expected: Vec<&[u8]> = vec![ - b"channels", - b"update", - channel.as_bytes(), - name_arg.as_bytes(), - b"", - ]; - assert_eq!(args, expected, "one call, for the duplicate only"); + let expected: Vec>> = vec![vec![ + b"channels".to_vec(), + b"update".to_vec(), + format!("--channel={duplicate}").into_bytes(), + format!("--name={new_name}").into_bytes(), + ]]; + assert_eq!(calls, expected, "one call, for the duplicate only"); let mut names = vec![ (kept, tricky.clone()), (duplicate, new_name.clone()), @@ -267,6 +292,69 @@ async fn script_renames_each_planned_channel_once_whatever_its_name() { ]; names.sort(); assert_eq!(seed.names(&pool).await, names); - std::fs::remove_dir_all(&dir).unwrap(); + drop_scratch_db(&admin, pool, &name).await; +} + +/// An auth refusal (CLI exit code 3, for example 403 relay_membership_required) +/// stops the script at the first rename, with a message that names the +/// membership precondition. +/// Mutation: drop the exit-code-3 branch in the script → RED. +#[cfg(unix)] +#[tokio::test] +#[ignore = "requires Postgres"] +async fn script_stops_at_the_first_auth_refusal() { + let admin = PgPool::connect(&admin_url().await).await.unwrap(); + let (pool, name) = create_scratch_db(&admin, "listed_name_script_auth").await; + let seed = Seed::new(&pool).await; + for (group, age) in [("first", 30), ("first", 10), ("second", 30), ("second", 10)] { + seed.open(&pool, Uuid::new_v4(), group, age).await; + } + + let (output, calls) = run_rename_script(&seed.host, &name, "exit 3\n").await; + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!(output.status.code(), Some(1), "{stderr}"); + assert_eq!(calls.len(), 1, "it stops after the first refusal"); + assert!(stderr.contains("a member of"), "{stderr}"); + drop_scratch_db(&admin, pool, &name).await; +} + +/// The relay serves only an active, unarchived community, so the script stops +/// before any rename for an archived community, one being deleted, or a host +/// no community has, and says why. +/// Mutation: drop the community check in the script → RED. +#[cfg(unix)] +#[tokio::test] +#[ignore = "requires Postgres"] +async fn script_stops_for_a_community_the_relay_does_not_serve() { + let admin = PgPool::connect(&admin_url().await).await.unwrap(); + let (pool, name) = create_scratch_db(&admin, "listed_name_script_state").await; + let archived = Seed::new(&pool).await; + let deleting = Seed::new(&pool).await; + for seed in [&archived, &deleting] { + seed.open(&pool, Uuid::new_v4(), "ops", 30).await; + seed.open(&pool, Uuid::new_v4(), "ops", 10).await; + } + sqlx::query("UPDATE communities SET archived_at = now() WHERE id = $1") + .bind(archived.community) + .execute(&pool) + .await + .unwrap(); + crate::test_support::quiesce_community_for_tests( + &pool, + buzz_core::CommunityId::from_uuid(deleting.community), + ) + .await; + + for (host, message) in [ + (archived.host.as_str(), "Unarchive it"), + (deleting.host.as_str(), "being deleted"), + ("missing.example", "no community has host"), + ] { + let (output, calls) = run_rename_script(host, &name, "exit 0\n").await; + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!(output.status.code(), Some(1), "{host}: {stderr}"); + assert!(calls.is_empty(), "{host}: no rename"); + assert!(stderr.contains(message), "{host}: {stderr}"); + } drop_scratch_db(&admin, pool, &name).await; } diff --git a/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs b/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs index aa9d35487bf..42c445ccd78 100644 --- a/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs @@ -1250,3 +1250,70 @@ async fn a_relay_operator_may_only_rename_a_channel() { "{tags:?}" ); } + +/// The operator rename covers only listed channels: open, not unlisted, a +/// stream or forum. A relay operator may not rename a private channel, an +/// unlisted channel or a DM. +/// Mutation: drop the listed check in `operator_may_rename` → RED. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn a_relay_operator_may_not_rename_an_unlisted_channel() { + let state = state().await; + let tenant = community(&state, "group-state-operator-unlisted").await; + let owner = Keys::generate(); + let other = Keys::generate(); + let operator = Keys::generate(); + state + .db + .upsert_relay_operator( + &operator.public_key().to_bytes(), + "operator", + &owner.public_key().to_bytes(), + true, + ) + .await + .expect("grant relay role"); + let private = create_channel(&state, &tenant, &owner, &[&["visibility", "private"]]).await; + let unlisted = create_channel(&state, &tenant, &owner, &[&["unlisted", "true"]]).await; + let (dm, _) = state + .db + .open_dm( + tenant.community(), + &[ + &owner.public_key().to_bytes(), + &other.public_key().to_bytes(), + ], + &owner.public_key().to_bytes(), + ) + .await + .expect("open dm"); + + for (label, channel) in [("private", private), ("unlisted", unlisted), ("dm", dm.id)] { + let before = state + .db + .get_channel(tenant.community(), channel) + .await + .expect("channel"); + if label == "unlisted" { + assert!(before.unlisted, "the channel is unlisted"); + } + let event = EventBuilder::new(Kind::Custom(9002), "") + .tags([ + Tag::parse(["h", &channel.to_string()]).unwrap(), + Tag::parse(["name", "by-operator"]).unwrap(), + ]) + .sign_with_keys(&operator) + .expect("sign 9002"); + match ingest_event(&state, &tenant, event, http_auth(&operator)).await { + Err(IngestError::Rejected(_)) => {} + Err(other) => panic!("{label}: expected a refusal, got {other:?}"), + Ok(result) => panic!("{label}: expected a refusal, got {:?}", result.message), + } + let after = state + .db + .get_channel(tenant.community(), channel) + .await + .expect("channel"); + assert_eq!(after.name, before.name, "{label}"); + } +} diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index 107af84f6a7..eb4895baae9 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -654,23 +654,10 @@ pub async fn validate_admin_event( } 9002 => { // EDIT_METADATA: require at least one recognized metadata tag. - const RECOGNIZED_TAGS: &[&str] = &[ - "name", - "about", - "archived", - "topic", - "purpose", - "visibility", - "ttl", - "t", - "join_role", - "unlisted", - "auto_join", - ]; let has_recognized = event .tags .iter() - .any(|t| RECOGNIZED_TAGS.contains(&t.kind().to_string().as_str())); + .any(|t| is_metadata_tag(&t.kind().to_string())); if !has_recognized { return Err(anyhow::anyhow!( "kind:9002 must include at least one metadata tag (name, about, archived, topic, purpose, visibility, ttl, t, join_role, unlisted, auto_join)" @@ -824,21 +811,14 @@ pub async fn validate_admin_event( // require owner/admin; topic/purpose allow any member who may // post; auto_join was decided above by relay role, so an event // carrying only that tag needs neither. - let has_member_tag = event.tags.iter().any(|t| { - let k = t.kind().to_string(); - k == "topic" || k == "purpose" - }); - let has_privileged_tag = event.tags.iter().any(|t| { - let k = t.kind().to_string(); - k == "name" - || k == "about" - || k == "archived" - || k == "visibility" - || k == "ttl" - || k == "t" - || k == "join_role" - || k == "unlisted" - }); + let has_member_tag = event + .tags + .iter() + .any(|t| MEMBER_METADATA_TAGS.contains(&t.kind().to_string().as_str())); + let has_privileged_tag = event + .tags + .iter() + .any(|t| PRIVILEGED_METADATA_TAGS.contains(&t.kind().to_string().as_str())); if has_privileged_tag { let members = state.db.get_members(tenant.community(), channel_id).await?; let actor_member = members.iter().find(|m| m.pubkey == actor_bytes); @@ -858,14 +838,12 @@ pub async fn validate_admin_event( { return Ok(()); } - // A relay operator may rename any channel, so that + // A relay operator may rename a listed channel, so that // duplicate names can be repaired through this path // (scripts/maintenance/rename_duplicate_listed_channel_names.sh). // Every other privileged change still needs a channel // owner or admin. - if is_rename_only(event) - && actor_is_relay_operator(state, event.pubkey.to_bytes()).await? - { + if operator_may_rename(state, event, &channel).await? { return Ok(()); } Err(anyhow::anyhow!( @@ -904,7 +882,7 @@ pub async fn validate_admin_event( Ok(()) } else { // Every recognized tag must be authorized by one of the arms - // above; a tag added to `RECOGNIZED_TAGS` but to neither list + // above; a tag added to `is_metadata_tag` but to no rule // is refused rather than accepted with no authorization. Err(anyhow::anyhow!( "kind:9002 metadata tag has no authorization rule" @@ -1307,25 +1285,57 @@ async fn emit_typed_membership_notification( Ok(()) } +/// kind:9002 tags that only a channel owner or admin may change. +const PRIVILEGED_METADATA_TAGS: &[&str] = &[ + "name", + "about", + "archived", + "visibility", + "ttl", + "t", + "join_role", + "unlisted", +]; + +/// kind:9002 tags that any member who may post may change. +const MEMBER_METADATA_TAGS: &[&str] = &["topic", "purpose"]; + +/// kind:9002 tags that a relay owner or admin may change, whatever their +/// channel role. +const RELAY_ROLE_METADATA_TAGS: &[&str] = &["auto_join"]; + +/// True when `kind` is a kind:9002 tag that changes channel metadata. +fn is_metadata_tag(kind: &str) -> bool { + PRIVILEGED_METADATA_TAGS.contains(&kind) + || MEMBER_METADATA_TAGS.contains(&kind) + || RELAY_ROLE_METADATA_TAGS.contains(&kind) +} + +/// True when a relay operator sends `event` and it only renames `channel`, a +/// listed channel: open, not unlisted, a stream or forum, and not deleted. +/// Those are the channels whose names must be unique, so this is the repair +/// path for a duplicate name and grants nothing else. +async fn operator_may_rename( + state: &crate::state::AppState, + event: &Event, + channel: &buzz_db::channel::ChannelRecord, +) -> anyhow::Result { + let listed = channel.visibility == "open" + && !channel.unlisted + && matches!(channel.channel_type.as_str(), "stream" | "forum") + && channel.deleted_at.is_none(); + Ok(listed + && is_rename_only(event) + && actor_is_relay_operator(state, event.pubkey.to_bytes()).await?) +} + /// True when the only metadata a kind:9002 changes is the channel name. fn is_rename_only(event: &Event) -> bool { - const METADATA_TAGS: &[&str] = &[ - "name", - "about", - "archived", - "topic", - "purpose", - "visibility", - "ttl", - "t", - "join_role", - "unlisted", - ]; let mut changes = event .tags .iter() .map(|t| t.kind().to_string()) - .filter(|kind| METADATA_TAGS.contains(&kind.as_str())); + .filter(|kind| is_metadata_tag(kind)); changes.all(|kind| kind == "name") && event.tags.iter().any(|t| t.kind().to_string() == "name") } @@ -2218,9 +2228,16 @@ pub(crate) async fn persist_edit_metadata( edit.columns.labels = Some(labels.map_err(|e| anyhow::anyhow!("{e}"))?); } // An operator's rename may repair an archived duplicate name, so it - // applies to an archived channel and leaves it archived. - edit.keep_archived = - is_rename_only(event) && actor_is_relay_operator(state, event.pubkey.to_bytes()).await?; + // applies to an archived channel and leaves it archived. Look up the + // operator only for a rename of an archived channel. + if is_rename_only(event) { + let channel = state + .db + .get_channel_for_event_write(tenant.community(), channel_id) + .await?; + edit.keep_archived = + channel.archived_at.is_some() && operator_may_rename(state, event, &channel).await?; + } let now = chrono::Utc::now(); let mut notices = Vec::new(); diff --git a/scripts/maintenance/rename_duplicate_listed_channel_names.sh b/scripts/maintenance/rename_duplicate_listed_channel_names.sh index 314e3ff3c03..521c6893c8e 100755 --- a/scripts/maintenance/rename_duplicate_listed_channel_names.sh +++ b/scripts/maintenance/rename_duplicate_listed_channel_names.sh @@ -15,15 +15,29 @@ # A rename that fails is reported and skipped: the relay refused it (for # example, because a channel took the new name meanwhile), or the CLI could # not reach the relay. At the end the script reads the plan again and fails -# if any rename remains; run it again. +# if any rename remains; run it again. If one rename fails on every run +# because another channel holds its new name, rename that channel by hand with +# `buzz channels update --channel= --name=`. An auth refusal +# (CLI exit code 3) stops the script at once, because every other rename would +# fail the same way. # # Usage: # DATABASE_URL=postgres://... BUZZ_PRIVATE_KEY= \ # scripts/maintenance/rename_duplicate_listed_channel_names.sh [--dry-run] # # BUZZ_PRIVATE_KEY must belong to a relay operator (RELAY_OPERATOR_PUBKEYS, or -# an `operator` row in relay_operators), not a moderator. BUZZ_RELAY_URL -# defaults to wss://. BUZZ_CLI defaults to `buzz`. +# an `operator` row in relay_operators), not a moderator. The key must also be +# a member of the community: POST /events checks community membership before +# it checks the operator role, so a key that is not a member gets 403 +# relay_membership_required. Add it first with +# `RELAY_URL=wss:// buzz-admin add-member --pubkey `. +# The relay serves only an active, unarchived community, so the script checks +# the community first. For an archived one, it stops (or, with --dry-run, warns): +# unarchive it with `buzz-admin communities unarchive`, run this script, then +# archive it again. For one being deleted, it stops: no rename can land there, +# and its duplicates go when its deletion completes. +# BUZZ_RELAY_URL defaults to wss://. BUZZ_CLI defaults to +# `buzz`. set -euo pipefail usage() { @@ -56,6 +70,35 @@ read_plan() { -A -t -z -0 -f "$plan_sql" >"$plan" } +community_state=$(psql "$DATABASE_URL" -X -q -v ON_ERROR_STOP=1 -v host="$host" \ + -A -t <<'SQL' +SELECT CASE + WHEN deleted_at IS NOT NULL OR deletion_state <> 'active' THEN 'deleting' + WHEN archived_at IS NOT NULL THEN 'archived' + ELSE 'active' +END +FROM communities WHERE host = :'host'; +SQL +) +case $community_state in +active) ;; +archived) + echo "$host is archived, and the relay applies no rename there. Unarchive it" \ + "with \`buzz-admin communities unarchive\`, run this script, then archive" \ + "it again." >&2 + $dry_run || exit 1 + ;; +deleting) + echo "$host is being deleted, and the relay applies no rename there. Its" \ + "duplicates go when its deletion completes." >&2 + exit 1 + ;; +*) + echo "no community has host $host" >&2 + exit 1 + ;; +esac + read_plan # The plan is the loop's stdin, so the CLI reads /dev/null instead. renamed=0 @@ -65,10 +108,18 @@ while IFS= read -r -d '' channel && IFS= read -r -d '' name && # --name=value, so a name that starts with "-" is not read as a flag. if $dry_run; then printf 'would rename %s from %q to %q\n' "$channel" "$name" "$new_name" - elif BUZZ_RELAY_URL=$relay_url "$buzz" channels update \ - --channel="$channel" --name="$new_name" /dev/null; then + continue + fi + status=0 + BUZZ_RELAY_URL=$relay_url "$buzz" channels update \ + --channel="$channel" --name="$new_name" /dev/null || status=$? + if [[ $status -eq 0 ]]; then printf 'renamed %s from %q to %q\n' "$channel" "$name" "$new_name" renamed=$((renamed + 1)) + elif [[ $status -eq 3 ]]; then + echo "the relay refused the key for $host: it must be a relay operator" \ + "and a member of $host (renamed $renamed before this)" >&2 + exit 1 else printf 'could not rename %s from %q to %q\n' \ "$channel" "$name" "$new_name" >&2 From 1d463dedb4d0bfc15ea053dd305717e5d0d07f70 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 2 Nov 2026 09:21:33 -0500 Subject: [PATCH 06/13] test(db): use ScratchDb in the listed-name tests Main replaced the per-file scratch database helpers with the ScratchDb guard (#8747), so these tests now use it and drop their database even when they fail. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- .../listed_channel_name_postgres_tests.rs | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs index 5bfcc5539ea..fd48050a22d 100644 --- a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs +++ b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs @@ -97,8 +97,7 @@ pub(super) async fn rename_duplicates(pool: &PgPool, host: &str) -> usize { #[tokio::test] #[ignore = "requires Postgres"] async fn plan_keeps_each_listed_name_on_its_oldest_unarchived_channel() { - let admin = PgPool::connect(&admin_url().await).await.unwrap(); - let (pool, name) = create_scratch_db(&admin, "listed_name_plan").await; + let (pool, _scratch) = ScratchDb::migrated(&admin_url().await, "listed_name_plan").await; let seed = Seed::new(&pool).await; let [kept, archived, newer, private, deleted] = [(); 5].map(|_| Uuid::new_v4()); seed.open(&pool, kept, "square-ios", 30).await; @@ -164,14 +163,12 @@ async fn plan_keeps_each_listed_name_on_its_oldest_unarchived_channel() { ]; names.sort(); assert_eq!(seed.names(&pool).await, names); - drop_scratch_db(&admin, pool, &name).await; } #[tokio::test] #[ignore = "requires Postgres"] async fn plan_picks_a_free_name_when_the_short_one_is_taken() { - let admin = PgPool::connect(&admin_url().await).await.unwrap(); - let (pool, name) = create_scratch_db(&admin, "listed_name_plan_taken").await; + let (pool, _scratch) = ScratchDb::migrated(&admin_url().await, "listed_name_plan_taken").await; let seed = Seed::new(&pool).await; // The longest name the column holds, so the new name must truncate it. let long = "a".repeat(255); @@ -189,7 +186,6 @@ async fn plan_picks_a_free_name_when_the_short_one_is_taken() { vec![(duplicate, long.clone(), new_name.clone())] ); assert!(new_name.chars().count() <= 255); - drop_scratch_db(&admin, pool, &name).await; } /// Runs the real script for `host` against database `db`, with `cli_body` as @@ -254,8 +250,7 @@ async fn run_rename_script( #[tokio::test] #[ignore = "requires Postgres"] async fn script_renames_each_planned_channel_once_whatever_its_name() { - let admin = PgPool::connect(&admin_url().await).await.unwrap(); - let (pool, name) = create_scratch_db(&admin, "listed_name_script").await; + let (pool, scratch) = ScratchDb::migrated(&admin_url().await, "listed_name_script").await; let seed = Seed::new(&pool).await; let [kept, duplicate, other] = [(); 3].map(|_| Uuid::new_v4()); let tricky = format!("-ops\x1e{other}\x1fowned\x1f\n\t\"$(x)"); @@ -265,7 +260,7 @@ async fn script_renames_each_planned_channel_once_whatever_its_name() { let (output, calls) = run_rename_script( &seed.host, - &name, + scratch.name(), "[[ $3 == --channel=* && $4 == --name=* ]] || exit 2\n\ psql \"$DATABASE_URL\" -X -q -v ON_ERROR_STOP=1 -v id=\"${3#*=}\" -v name=\"${4#*=}\" <<'SQL'\n\ UPDATE channels SET name = :'name' WHERE id = :'id';\nSQL\n", @@ -292,7 +287,6 @@ async fn script_renames_each_planned_channel_once_whatever_its_name() { ]; names.sort(); assert_eq!(seed.names(&pool).await, names); - drop_scratch_db(&admin, pool, &name).await; } /// An auth refusal (CLI exit code 3, for example 403 relay_membership_required) @@ -303,19 +297,17 @@ async fn script_renames_each_planned_channel_once_whatever_its_name() { #[tokio::test] #[ignore = "requires Postgres"] async fn script_stops_at_the_first_auth_refusal() { - let admin = PgPool::connect(&admin_url().await).await.unwrap(); - let (pool, name) = create_scratch_db(&admin, "listed_name_script_auth").await; + let (pool, scratch) = ScratchDb::migrated(&admin_url().await, "listed_name_script_auth").await; let seed = Seed::new(&pool).await; for (group, age) in [("first", 30), ("first", 10), ("second", 30), ("second", 10)] { seed.open(&pool, Uuid::new_v4(), group, age).await; } - let (output, calls) = run_rename_script(&seed.host, &name, "exit 3\n").await; + let (output, calls) = run_rename_script(&seed.host, scratch.name(), "exit 3\n").await; let stderr = String::from_utf8_lossy(&output.stderr); assert_eq!(output.status.code(), Some(1), "{stderr}"); assert_eq!(calls.len(), 1, "it stops after the first refusal"); assert!(stderr.contains("a member of"), "{stderr}"); - drop_scratch_db(&admin, pool, &name).await; } /// The relay serves only an active, unarchived community, so the script stops @@ -326,8 +318,7 @@ async fn script_stops_at_the_first_auth_refusal() { #[tokio::test] #[ignore = "requires Postgres"] async fn script_stops_for_a_community_the_relay_does_not_serve() { - let admin = PgPool::connect(&admin_url().await).await.unwrap(); - let (pool, name) = create_scratch_db(&admin, "listed_name_script_state").await; + let (pool, scratch) = ScratchDb::migrated(&admin_url().await, "listed_name_script_state").await; let archived = Seed::new(&pool).await; let deleting = Seed::new(&pool).await; for seed in [&archived, &deleting] { @@ -350,11 +341,10 @@ async fn script_stops_for_a_community_the_relay_does_not_serve() { (deleting.host.as_str(), "being deleted"), ("missing.example", "no community has host"), ] { - let (output, calls) = run_rename_script(host, &name, "exit 0\n").await; + let (output, calls) = run_rename_script(host, scratch.name(), "exit 0\n").await; let stderr = String::from_utf8_lossy(&output.stderr); assert_eq!(output.status.code(), Some(1), "{host}: {stderr}"); assert!(calls.is_empty(), "{host}: no rename"); assert!(stderr.contains(message), "{host}: {stderr}"); } - drop_scratch_db(&admin, pool, &name).await; } From e6da0c089637bf4997e464241671eb6b2807f8fc Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 2 Nov 2026 09:21:37 -0500 Subject: [PATCH 07/13] fix(relay): decide an operator rename under the row lock The relay checked that a channel is listed before an operator's rename, without the channel row lock, and the store did not check it again. An owner who made the channel private or unlisted meanwhile still got the rename. ChannelMetadataEdit::operator_rename replaces keep_archived. The relay sets it for a relay operator's name-only rename made without channel authority. The store then refuses the rename unless the locked row is listed, and lets it rename an archived channel, which stays archived. Validation keeps its early check for a clear refusal. A name-only rename no longer reads the channel. It looks up the operator role, and channel authority only for an operator. auto_join, added on main, joins the kind:9002 tag lists as a relay-role tag, so a rename that also sets it is not name-only. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- crates/buzz-db/src/store/channel.rs | 54 ++++++++---- .../handlers/group_state_postgres_tests.rs | 75 +++++++++++++++++ .../buzz-relay/src/handlers/side_effects.rs | 84 ++++++++++--------- 3 files changed, 155 insertions(+), 58 deletions(-) diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index 06122786bd2..ebb170ca8df 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -1056,6 +1056,7 @@ pub(crate) struct ChannelSettingsWriter<'tx> { ttl: TtlLock, previous_visibility: String, archived: bool, + listed: bool, } impl ChannelSettingsWriter<'_> { @@ -1070,6 +1071,12 @@ impl ChannelSettingsWriter<'_> { self.archived } + /// Whether the channel was listed ([`LISTED_CHANNEL_SQL`]) when the lock + /// was taken. The row lock keeps this current until the transaction ends. + pub(crate) fn listed(&self) -> bool { + self.listed + } + /// The locked transaction, for writes that are not channel settings /// (for example, storing the event that requested the change). pub(crate) fn tx(&mut self) -> &mut crate::AdmittedTx { @@ -1158,15 +1165,16 @@ async fn lock_channel_row( // `FOR NO KEY UPDATE`, like a huddle join: it orders this write against // every other settings write and a huddle join, but not against the // `KEY SHARE` lock that a membership INSERT takes for its foreign key. - let (previous_visibility, archived): (String, bool) = sqlx::query_as( - "SELECT visibility::text, archived_at IS NOT NULL FROM channels \ - WHERE community_id = $1 AND id = $2 AND deleted_at IS NULL FOR NO KEY UPDATE", - ) - .bind(community_id.as_uuid()) - .bind(channel_id) - .fetch_optional(tx.conn()) - .await? - .ok_or(DbError::ChannelNotFound(channel_id))?; + let (previous_visibility, archived, listed): (String, bool, bool) = + sqlx::query_as(sqlx::AssertSqlSafe(format!( + "SELECT visibility::text, archived_at IS NOT NULL, {LISTED_CHANNEL_SQL} FROM channels \ + WHERE community_id = $1 AND id = $2 AND deleted_at IS NULL FOR NO KEY UPDATE" + ))) + .bind(community_id.as_uuid()) + .bind(channel_id) + .fetch_optional(tx.conn()) + .await? + .ok_or(DbError::ChannelNotFound(channel_id))?; Ok(ChannelSettingsWriter { tx, channel_id, @@ -1174,6 +1182,7 @@ async fn lock_channel_row( ttl, previous_visibility, archived, + listed, }) } @@ -1496,9 +1505,11 @@ pub struct ChannelMetadataEdit { pub purpose: Option, /// `Some(true)` archives the channel; `Some(false)` restores it. pub archived: Option, - /// Lets this edit change an archived channel, which stays archived. Only - /// a relay operator's name-only rename sets it. - pub keep_archived: bool, + /// Set for a relay operator's name-only rename, made without channel + /// authority. The edit then applies only to a listed channel + /// ([`LISTED_CHANNEL_SQL`]), checked under the row lock, and may rename + /// an archived channel, which stays archived. + pub operator_rename: bool, } /// The result of [`Db::apply_channel_metadata_event`]. @@ -1550,13 +1561,20 @@ pub(crate) async fn apply_channel_metadata_event( previous_visibility, }); } - // Decide the archive state here, after the duplicate check and under - // the row lock, not during validation. A resend that races the first - // commit then sees that commit as a duplicate (above), never as a - // channel already in the requested state. + // Decide the archive state and an operator's right to rename here, + // after the duplicate check and under the row lock, not during + // validation. A resend that races the first commit then sees that commit + // as a duplicate (above), never as a channel already in the requested + // state, and an owner who makes the channel private or unlisted + // meanwhile is seen. + if edit.operator_rename && !locked.listed() { + return Err(DbError::AccessDenied( + "a relay operator may rename only a listed channel".to_string(), + )); + } match (locked.archived(), edit.archived) { (true, Some(false)) | (false, None | Some(true)) => {} - (true, None) if edit.keep_archived => {} + (true, None) if edit.operator_rename => {} (true, _) => return Err(DbError::AccessDenied("channel is archived".to_string())), (false, Some(false)) => { return Err(DbError::AccessDenied("channel is not archived".to_string())); @@ -1592,7 +1610,7 @@ pub(crate) async fn apply_channel_metadata_event( topic, purpose, archived, - keep_archived: _, + operator_rename: _, } = edit; let actor = event.pubkey.to_bytes(); if !columns.is_empty() { diff --git a/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs b/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs index 42c445ccd78..6f325d3b442 100644 --- a/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs @@ -1251,6 +1251,81 @@ async fn a_relay_operator_may_only_rename_a_channel() { ); } +/// An owner may make a channel private or unlisted after the relay has +/// validated an operator's rename and before the rename is stored. The store +/// decides under the channel row lock, so the rename is refused then and the +/// name is unchanged. +/// Mutation: drop the `operator_rename && !locked.listed()` check in +/// `apply_channel_metadata_event`, or never set `operator_rename` in +/// `persist_edit_metadata` → RED. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn an_operator_rename_is_refused_when_the_channel_stops_being_listed() { + let state = state().await; + let tenant = community(&state, "group-state-operator-race").await; + let owner = Keys::generate(); + let operator = Keys::generate(); + state + .db + .upsert_relay_operator( + &operator.public_key().to_bytes(), + "operator", + &owner.public_key().to_bytes(), + true, + ) + .await + .expect("grant relay role"); + let edit = |keys: &Keys, channel: Uuid, tag: [&str; 2]| { + EventBuilder::new(Kind::Custom(9002), "") + .tags([ + Tag::parse(["h", &channel.to_string()]).unwrap(), + Tag::parse(tag).unwrap(), + ]) + .sign_with_keys(keys) + .expect("sign 9002") + }; + + for change in [["visibility", "private"], ["unlisted", "true"]] { + let channel = create_channel(&state, &tenant, &owner, &[]).await; + let before = state + .db + .get_channel(tenant.community(), channel) + .await + .expect("channel") + .name; + let rename = edit(&operator, channel, ["name", "by-operator"]); + super::side_effects::validate_admin_event(&tenant, 9002, &rename, &state) + .await + .unwrap_or_else(|error| panic!("{change:?}: the listed channel validates: {error}")); + + let hidden = ingest_event( + &state, + &tenant, + edit(&owner, channel, change), + http_auth(&owner), + ) + .await + .unwrap_or_else(|error| panic!("{change:?}: owner change: {error:?}")); + assert!(hidden.accepted, "{change:?}: {}", hidden.message); + + let error = super::side_effects::persist_edit_metadata(&tenant, &rename, &state) + .await + .err() + .unwrap_or_else(|| panic!("{change:?}: the rename must be refused")); + assert!( + format!("{error:#}").contains("only a listed channel"), + "{change:?}: {error:#}" + ); + let after = state + .db + .get_channel(tenant.community(), channel) + .await + .expect("channel") + .name; + assert_eq!(after, before, "{change:?}"); + } +} + /// The operator rename covers only listed channels: open, not unlisted, a /// stream or forum. A relay operator may not rename a private channel, an /// unlisted channel or a DM. diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index eb4895baae9..4a20a072169 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -545,6 +545,26 @@ fn check_deletion_privacy_k_tag(event: &Event, target_kind: u32) -> anyhow::Resu /// /// Checking all active owners (not just the first) is correct under co-ownership: /// an agent may be promoted to owner later, or multiple agents may co-own a channel. +/// True when `actor` may change a channel's privileged metadata: a channel +/// owner or admin, or the owning human of an active owner-role agent in the +/// channel, even when that human is not a member (this diverges from +/// kind:9001 intentionally). +async fn actor_has_channel_authority( + state: &Arc, + community_id: buzz_core::CommunityId, + channel_id: Uuid, + actor_bytes: &[u8], +) -> anyhow::Result { + let members = state.db.get_members(community_id, channel_id).await?; + if members + .iter() + .any(|m| m.pubkey == actor_bytes && (m.role == "owner" || m.role == "admin")) + { + return Ok(true); + } + actor_owns_any_owner_agent(state, community_id, &members, actor_bytes).await +} + async fn actor_owns_any_owner_agent( state: &Arc, community_id: buzz_core::CommunityId, @@ -820,37 +840,23 @@ pub async fn validate_admin_event( .iter() .any(|t| PRIVILEGED_METADATA_TAGS.contains(&t.kind().to_string().as_str())); if has_privileged_tag { - 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(()), - _ => { - // Allow the owning human of any active owner-role agent in the - // channel, even when the human is not a channel member — - // diverges from kind:9001 intentionally. - if actor_owns_any_owner_agent( - state, - tenant.community(), - &members, - &actor_bytes, - ) - .await? - { - return Ok(()); - } - // A relay operator may rename a listed channel, so that - // duplicate names can be repaired through this path - // (scripts/maintenance/rename_duplicate_listed_channel_names.sh). - // Every other privileged change still needs a channel - // owner or admin. - if operator_may_rename(state, event, &channel).await? { - return Ok(()); - } - Err(anyhow::anyhow!( - "actor not authorized for name/about/archived/visibility/ttl/label/join_role/unlisted changes" - )) - } + if actor_has_channel_authority(state, tenant.community(), channel_id, &actor_bytes) + .await? + { + return Ok(()); } + // A relay operator may rename a listed channel, so that + // duplicate names can be repaired through this path + // (scripts/maintenance/rename_duplicate_listed_channel_names.sh). + // Every other privileged change still needs a channel owner + // or admin. The store checks "listed" again under the row + // lock (`ChannelMetadataEdit::operator_rename`). + if operator_may_rename(state, event, &channel).await? { + return Ok(()); + } + Err(anyhow::anyhow!( + "actor not authorized for name/about/archived/visibility/ttl/label/join_role/unlisted changes" + )) } else if has_member_tag { // topic/purpose: any member whose role may post (never a // guest). @@ -2227,17 +2233,15 @@ pub(crate) async fn persist_edit_metadata( if let Some(labels) = channel_labels_from_tags(event) { edit.columns.labels = Some(labels.map_err(|e| anyhow::anyhow!("{e}"))?); } - // An operator's rename may repair an archived duplicate name, so it - // applies to an archived channel and leaves it archived. Look up the - // operator only for a rename of an archived channel. - if is_rename_only(event) { - let channel = state - .db - .get_channel_for_event_write(tenant.community(), channel_id) + // A relay operator without channel authority may rename only a listed + // channel, archived ones included, so that duplicate names can be + // repaired. The store decides that under the channel row lock, so an + // owner who makes the channel private or unlisted meanwhile is seen. + // Channel authority is looked up only for an operator's rename. + edit.operator_rename = is_rename_only(event) + && actor_is_relay_operator(state, event.pubkey.to_bytes()).await? + && !actor_has_channel_authority(state, tenant.community(), channel_id, &actor_bytes) .await?; - edit.keep_archived = - channel.archived_at.is_some() && operator_may_rename(state, event, &channel).await?; - } let now = chrono::Utc::now(); let mut notices = Vec::new(); From a4d51f5d94be87ed1e71d6a84e05929303da969b Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 2 Nov 2026 09:21:40 -0500 Subject: [PATCH 08/13] docs: describe the listed-name rule in NOSTR.md Rows for kind:9007 and kind:9002 now say that a create needs an h tag, that a taken listed name is refused, and that a relay operator may rename a listed channel. The rename script says to unarchive a community before adding the operator key as a member, because add-member finds only an unarchived community. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- NOSTR.md | 4 ++-- .../maintenance/rename_duplicate_listed_channel_names.sh | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/NOSTR.md b/NOSTR.md index 1450c0cdf33..c393a7d2f2d 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -50,10 +50,10 @@ PGPASSWORD=buzz_dev psql -h localhost -U buzz -d buzz -c \ | **Reactions (kind:7)** | ✅ | Standard NIP-25; channel derived from target event's `#e` tag (client `#h` ignored) | | **Deletions (kind:5)** | ✅ | Standard NIP-09; self-authored only. `#h` optional, `#e` required | | **User profiles (kind:0)** | ✅ | NIP-01 metadata; synced to users table (display_name, avatar, about, NIP-05). NIP-05 handles must canonicalize to this relay's domain — off-domain or invalid handles are silently cleared. If a NIP-05 handle collides with another user's (UNIQUE constraint), the handle is skipped but other profile fields (display_name, avatar, about) are still synced. | -| **Group creation (kind:9007)** | ✅ | NIP-29; include `name` tag, optional `visibility`, `channel_type`, label `t` and `unlisted` tags | +| **Group creation (kind:9007)** | ✅ | NIP-29; include an `h` tag with the new channel id and a `name` tag, optional `visibility`, `channel_type`, label `t` and `unlisted` tags. The relay refuses a 9007 without `h`. It refuses a listed channel (open, not unlisted, stream or forum) whose name another listed channel already has, ignoring case and including archived ones: `invalid: an open channel named "" already exists`. | | **Add user (kind:9000)** | ✅ | Open: any user, subject to target's `channel_add_policy` (`owner_only`/`nobody` can block). Private: owner/admin only. Self-add bypasses agent policy but not private-channel auth. A guest cannot add anyone. A new member with no role tag gets the channel's `join_role`; a non-member adding themselves may ask for `join_role` or a lower role. One event may name up to 100 people: one `p` tag each, with an optional role as the `p` tag's third element (NIP-29), falling back to the event's `role` tag. Each person is checked and added on their own. When the event names several people, the OK message is `members: ` followed by JSON listing the non-empty `added`, `updated` (role changed), `already_member` and `rejected` (`[{"reason", "pubkeys"}]`) results. The event is refused only when it is malformed or nobody it names may be added; then one person keeps the single-add error, and several get `invalid: members: {…}`. The event and its membership changes are stored together, so an `error:` rejection stores nothing and the same event can be resent; a resent stored event returns `duplicate:` without results. Re-adding someone at their current role posts no second `member_joined` notice. | | **Remove user (kind:9001)** | ✅ | Self-remove allowed (with last-owner guard). Removing others: owner/admin only. | -| **Edit group metadata (kind:9002)** | ✅ | `name`/`about`/label `t`/`visibility`/`ttl`/`archived`/`join_role`/`unlisted` tags: owner/admin. `topic`/`purpose` tags: any member except a guest. `auto_join` tag: relay owner/admin (see [Default channels](#default-channels)). Buzz applies 9002 as a partial update: an absent tag leaves that setting unchanged. | +| **Edit group metadata (kind:9002)** | ✅ | `name`/`about`/label `t`/`visibility`/`ttl`/`archived`/`join_role`/`unlisted` tags: owner/admin. `topic`/`purpose` tags: any member except a guest. `auto_join` tag: relay owner/admin (see [Default channels](#default-channels)). A relay operator may also send a 9002 that changes only `name` on a listed channel, archived ones included, so that duplicate names can be repaired; the channel stays archived. The relay refuses a `name`, `visibility` or `unlisted` change that leaves a listed channel with a name another listed channel already has: `invalid: an open channel named "" already exists`. Buzz applies 9002 as a partial update: an absent tag leaves that setting unchanged. | | **Admin delete event (kind:9005)** | ✅ | Event author can always delete own. Otherwise owner/admin required. Target must be in same channel. | | **Group deletion (kind:9008)** | ✅ | Owner only. | | **Leave group (kind:9022)** | ✅ | Any member. Last-owner guard prevents orphaned groups. | diff --git a/scripts/maintenance/rename_duplicate_listed_channel_names.sh b/scripts/maintenance/rename_duplicate_listed_channel_names.sh index 521c6893c8e..64d7bc422d3 100755 --- a/scripts/maintenance/rename_duplicate_listed_channel_names.sh +++ b/scripts/maintenance/rename_duplicate_listed_channel_names.sh @@ -33,7 +33,8 @@ # `RELAY_URL=wss:// buzz-admin add-member --pubkey `. # The relay serves only an active, unarchived community, so the script checks # the community first. For an archived one, it stops (or, with --dry-run, warns): -# unarchive it with `buzz-admin communities unarchive`, run this script, then +# unarchive it with `buzz-admin communities unarchive`, then add the member +# (add-member also finds only an unarchived community), run this script, and # archive it again. For one being deleted, it stops: no rename can land there, # and its duplicates go when its deletion completes. # BUZZ_RELAY_URL defaults to wss://. BUZZ_CLI defaults to @@ -84,8 +85,8 @@ case $community_state in active) ;; archived) echo "$host is archived, and the relay applies no rename there. Unarchive it" \ - "with \`buzz-admin communities unarchive\`, run this script, then archive" \ - "it again." >&2 + "with \`buzz-admin communities unarchive\`, add this key as a member," \ + "run this script, then archive it again." >&2 $dry_run || exit 1 ;; deleting) From 90289577212a65adda7e2c64501986d0359c960b Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 2 Nov 2026 09:42:26 -0500 Subject: [PATCH 09/13] fix(scripts): free a starter name that only archived channels hold Released desktop and mobile apps create `general` and `welcome-everyone` when no open, unarchived stream has the name. With the listed-name rule, the relay refuses that create while an archived channel holds the name, so their starter setup fails. The rename plan now also renames every holder of a starter name when all of them are archived. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- .../listed_channel_name_postgres_tests.rs | 72 +++++++++++++++++++ .../plan_duplicate_listed_channel_renames.sql | 20 ++++-- 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs index fd48050a22d..29a8bcdacd8 100644 --- a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs +++ b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs @@ -165,6 +165,78 @@ async fn plan_keeps_each_listed_name_on_its_oldest_unarchived_channel() { assert_eq!(seed.names(&pool).await, names); } +/// Released desktop and mobile apps create `general` and `welcome-everyone` +/// when no open, unarchived stream has the name, and the relay refuses that +/// create while an archived channel holds it. The plan renames every holder of +/// a starter name when all of them are archived, and no other archived name. +/// Mutation: drop the `all_archived` condition in the plan → RED. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn plan_frees_a_starter_name_that_only_archived_channels_hold() { + let (pool, _scratch) = + ScratchDb::migrated(&admin_url().await, "listed_name_plan_starter").await; + let seed = Seed::new(&pool).await; + let [general, welcome_old, welcome_new, random] = [(); 4].map(|_| Uuid::new_v4()); + seed.open(&pool, general, "General", 30).await; + seed.open(&pool, welcome_old, "welcome-everyone", 60).await; + seed.open(&pool, welcome_new, "welcome-everyone", 10).await; + seed.open(&pool, random, "random", 20).await; + // A live `general` keeps its name; its archived namesake is a plain duplicate. + let live = Seed::new(&pool).await; + let [live_general, archived_general] = [(); 2].map(|_| Uuid::new_v4()); + live.open(&pool, live_general, "general", 10).await; + live.open(&pool, archived_general, "general", 60).await; + sqlx::query("UPDATE channels SET archived_at = now() WHERE id = ANY($1)") + .bind(vec![ + general, + welcome_old, + welcome_new, + random, + archived_general, + ]) + .execute(&pool) + .await + .unwrap(); + + let mut expected = vec![ + (general, "General".to_owned(), renamed(general, "General").1), + ( + welcome_old, + "welcome-everyone".to_owned(), + renamed(welcome_old, "welcome-everyone").1, + ), + ( + welcome_new, + "welcome-everyone".to_owned(), + renamed(welcome_new, "welcome-everyone").1, + ), + ]; + expected.sort(); + assert_eq!(rename_plan(&pool, &seed.host).await, expected); + assert_eq!( + rename_plan(&pool, &live.host).await, + vec![( + archived_general, + "general".to_owned(), + renamed(archived_general, "general").1 + )] + ); + + assert_eq!(rename_duplicates(&pool, &seed.host).await, 3); + assert!( + rename_plan(&pool, &seed.host).await.is_empty(), + "nothing is left to rename" + ); + let mut names = vec![ + renamed(general, "General"), + renamed(welcome_old, "welcome-everyone"), + renamed(welcome_new, "welcome-everyone"), + (random, "random".to_owned()), + ]; + names.sort(); + assert_eq!(seed.names(&pool).await, names); +} + #[tokio::test] #[ignore = "requires Postgres"] async fn plan_picks_a_free_name_when_the_short_one_is_taken() { diff --git a/scripts/maintenance/plan_duplicate_listed_channel_renames.sql b/scripts/maintenance/plan_duplicate_listed_channel_renames.sql index f0d681f1a0f..bbb64d69a73 100644 --- a/scripts/maintenance/plan_duplicate_listed_channel_renames.sql +++ b/scripts/maintenance/plan_duplicate_listed_channel_renames.sql @@ -8,11 +8,18 @@ -- (`LISTED_CHANNEL_SQL` in crates/buzz-db/src/store/channel.rs). -- -- In each group of channels that share a name, one channel keeps it: the --- oldest unarchived one, or the oldest archived one when all are archived. +-- oldest unarchived one, or the oldest archived one when all are archived +-- (except a starter name, below). -- Every other channel in the group gets "-", or "-" when another listed channel already has that name. -- It lists nothing for a community that is not active. -- +-- It also renames every channel named "general" or "welcome-everyone" when +-- all the listed channels with that name are archived. Released desktop and +-- mobile apps create those starter channels when no open, unarchived stream +-- has the name, and the relay refuses that create while an archived channel +-- holds the name. The rename frees the name for them. +-- -- psql -X -v ON_ERROR_STOP=1 -v host= -f scripts/maintenance/plan_duplicate_listed_channel_renames.sql WITH listed AS ( @@ -29,13 +36,16 @@ duplicates AS ( left(name, 246) || '-' || left(id::text, 8) AS short_name, left(name, 218) || '-' || id::text AS long_name FROM ( - SELECT id, name, row_number() OVER ( - PARTITION BY lower(name) - ORDER BY archived_at IS NOT NULL, created_at, id - ) AS place + SELECT id, name, + row_number() OVER ( + PARTITION BY lower(name) + ORDER BY archived_at IS NOT NULL, created_at, id + ) AS place, + bool_and(archived_at IS NOT NULL) OVER (PARTITION BY lower(name)) AS all_archived FROM listed ) ranked WHERE place > 1 + OR (all_archived AND lower(name) IN ('general', 'welcome-everyone')) ) SELECT d.id AS channel_id, d.name, CASE WHEN EXISTS ( From 06d5e0039bea6f05b971d1442b13a55aa6b3ad6e Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 2 Nov 2026 09:58:01 -0500 Subject: [PATCH 10/13] fix(relay): let an operator who owns a channel rename it while archived The operator rename set its flag only for an operator without channel authority, so an operator who owns or administers an archived listed duplicate fell to the owner path, which refuses an archived channel. The rename script then failed on that channel on every run. Keep two facts apart: the rename is an operator's name-only rename, and the actor has channel authority. The archived exemption applies to any operator rename of a listed channel; the listed check applies only without channel authority. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- NOSTR.md | 2 +- crates/buzz-db/src/store/channel.rs | 16 +++-- .../handlers/group_state_postgres_tests.rs | 63 ++++++++++++++++++- .../buzz-relay/src/handlers/side_effects.rs | 17 ++--- 4 files changed, 82 insertions(+), 16 deletions(-) diff --git a/NOSTR.md b/NOSTR.md index c393a7d2f2d..b3d628786f4 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -53,7 +53,7 @@ PGPASSWORD=buzz_dev psql -h localhost -U buzz -d buzz -c \ | **Group creation (kind:9007)** | ✅ | NIP-29; include an `h` tag with the new channel id and a `name` tag, optional `visibility`, `channel_type`, label `t` and `unlisted` tags. The relay refuses a 9007 without `h`. It refuses a listed channel (open, not unlisted, stream or forum) whose name another listed channel already has, ignoring case and including archived ones: `invalid: an open channel named "" already exists`. | | **Add user (kind:9000)** | ✅ | Open: any user, subject to target's `channel_add_policy` (`owner_only`/`nobody` can block). Private: owner/admin only. Self-add bypasses agent policy but not private-channel auth. A guest cannot add anyone. A new member with no role tag gets the channel's `join_role`; a non-member adding themselves may ask for `join_role` or a lower role. One event may name up to 100 people: one `p` tag each, with an optional role as the `p` tag's third element (NIP-29), falling back to the event's `role` tag. Each person is checked and added on their own. When the event names several people, the OK message is `members: ` followed by JSON listing the non-empty `added`, `updated` (role changed), `already_member` and `rejected` (`[{"reason", "pubkeys"}]`) results. The event is refused only when it is malformed or nobody it names may be added; then one person keeps the single-add error, and several get `invalid: members: {…}`. The event and its membership changes are stored together, so an `error:` rejection stores nothing and the same event can be resent; a resent stored event returns `duplicate:` without results. Re-adding someone at their current role posts no second `member_joined` notice. | | **Remove user (kind:9001)** | ✅ | Self-remove allowed (with last-owner guard). Removing others: owner/admin only. | -| **Edit group metadata (kind:9002)** | ✅ | `name`/`about`/label `t`/`visibility`/`ttl`/`archived`/`join_role`/`unlisted` tags: owner/admin. `topic`/`purpose` tags: any member except a guest. `auto_join` tag: relay owner/admin (see [Default channels](#default-channels)). A relay operator may also send a 9002 that changes only `name` on a listed channel, archived ones included, so that duplicate names can be repaired; the channel stays archived. The relay refuses a `name`, `visibility` or `unlisted` change that leaves a listed channel with a name another listed channel already has: `invalid: an open channel named "" already exists`. Buzz applies 9002 as a partial update: an absent tag leaves that setting unchanged. | +| **Edit group metadata (kind:9002)** | ✅ | `name`/`about`/label `t`/`visibility`/`ttl`/`archived`/`join_role`/`unlisted` tags: owner/admin. `topic`/`purpose` tags: any member except a guest. `auto_join` tag: relay owner/admin (see [Default channels](#default-channels)). A relay operator may also send a 9002 that changes only `name` on a listed channel, archived ones included, whatever its channel role, so that duplicate names can be repaired; the channel stays archived. The relay refuses a `name`, `visibility` or `unlisted` change that leaves a listed channel with a name another listed channel already has: `invalid: an open channel named "" already exists`. Buzz applies 9002 as a partial update: an absent tag leaves that setting unchanged. | | **Admin delete event (kind:9005)** | ✅ | Event author can always delete own. Otherwise owner/admin required. Target must be in same channel. | | **Group deletion (kind:9008)** | ✅ | Owner only. | | **Leave group (kind:9022)** | ✅ | Any member. Last-owner guard prevents orphaned groups. | diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index ebb170ca8df..f3d8c9ca0a1 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -1505,11 +1505,14 @@ pub struct ChannelMetadataEdit { pub purpose: Option, /// `Some(true)` archives the channel; `Some(false)` restores it. pub archived: Option, - /// Set for a relay operator's name-only rename, made without channel - /// authority. The edit then applies only to a listed channel - /// ([`LISTED_CHANNEL_SQL`]), checked under the row lock, and may rename - /// an archived channel, which stays archived. + /// Set for a relay operator's name-only rename, whatever the operator's + /// channel role. It may rename an archived listed channel + /// ([`LISTED_CHANNEL_SQL`]), which stays archived. Checked under the row + /// lock. pub operator_rename: bool, + /// Set when the actor is the channel's owner or admin. An operator rename + /// without it applies only to a listed channel. + pub channel_authority: bool, } /// The result of [`Db::apply_channel_metadata_event`]. @@ -1567,14 +1570,14 @@ pub(crate) async fn apply_channel_metadata_event( // as a duplicate (above), never as a channel already in the requested // state, and an owner who makes the channel private or unlisted // meanwhile is seen. - if edit.operator_rename && !locked.listed() { + if edit.operator_rename && !edit.channel_authority && !locked.listed() { return Err(DbError::AccessDenied( "a relay operator may rename only a listed channel".to_string(), )); } match (locked.archived(), edit.archived) { (true, Some(false)) | (false, None | Some(true)) => {} - (true, None) if edit.operator_rename => {} + (true, None) if edit.operator_rename && locked.listed() => {} (true, _) => return Err(DbError::AccessDenied("channel is archived".to_string())), (false, Some(false)) => { return Err(DbError::AccessDenied("channel is not archived".to_string())); @@ -1611,6 +1614,7 @@ pub(crate) async fn apply_channel_metadata_event( purpose, archived, operator_rename: _, + channel_authority: _, } = edit; let actor = event.pubkey.to_bytes(); if !columns.is_empty() { diff --git a/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs b/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs index 6f325d3b442..b09c390fe56 100644 --- a/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/group_state_postgres_tests.rs @@ -1140,8 +1140,12 @@ async fn a_taken_open_name_is_refused_as_a_client_error() { /// A relay operator who is not a channel member may rename the channel, and /// the relay publishes the new name in kind:39000. The operator may change /// nothing else, and a relay moderator may not rename it. +/// An operator who owns an archived listed channel may rename it as well, but +/// not an archived unlisted one. /// Mutation: drop the operator branch in `validate_admin_event`, or drop its -/// `is_rename_only` or operator-role condition → RED. +/// `is_rename_only` or operator-role condition, or set `operator_rename` only +/// without channel authority, or drop `locked.listed()` from the archived +/// exemption → RED. #[tokio::test] #[ignore = "requires Postgres"] async fn a_relay_operator_may_only_rename_a_channel() { @@ -1249,6 +1253,63 @@ async fn a_relay_operator_may_only_rename_a_channel() { .any(|tag| tag.len() >= 2 && tag[0] == "name" && tag[1] == "renamed-by-operator"), "{tags:?}" ); + + // An operator who owns an archived listed channel may rename it too, and + // it stays archived: the operator rule holds whatever the channel role. + let owned = create_channel(&state, &tenant, &operator, &[]).await; + state + .db + .archive_channel(community_id, owned) + .await + .expect("archive"); + let rename = EventBuilder::new(Kind::Custom(9002), "") + .tags([ + Tag::parse(["h", &owned.to_string()]).unwrap(), + Tag::parse(["name", "renamed-by-owning-operator"]).unwrap(), + ]) + .sign_with_keys(&operator) + .expect("sign 9002"); + let renamed = ingest_event(&state, &tenant, rename, http_auth(&operator)) + .await + .unwrap_or_else(|error| panic!("owning operator rename: {error:?}")); + assert!(renamed.accepted, "{}", renamed.message); + let owned = state + .db + .get_channel(community_id, owned) + .await + .expect("channel"); + assert_eq!(owned.name, "renamed-by-owning-operator"); + assert!(owned.archived_at.is_some(), "the rename keeps it archived"); + + // The archived exemption covers only a listed channel: the same operator + // may not rename an archived unlisted channel it owns. + let hidden = create_channel(&state, &tenant, &operator, &[&["unlisted", "true"]]).await; + state + .db + .archive_channel(community_id, hidden) + .await + .expect("archive"); + let rename = EventBuilder::new(Kind::Custom(9002), "") + .tags([ + Tag::parse(["h", &hidden.to_string()]).unwrap(), + Tag::parse(["name", "hidden-renamed"]).unwrap(), + ]) + .sign_with_keys(&operator) + .expect("sign 9002"); + match ingest_event(&state, &tenant, rename, http_auth(&operator)).await { + Err(IngestError::Rejected(message)) => assert!(message.contains("archived"), "{message}"), + Err(other) => panic!("expected a refusal, got {other:?}"), + Ok(result) => panic!("expected a refusal, got {:?}", result.message), + } + assert_ne!( + state + .db + .get_channel(community_id, hidden) + .await + .expect("channel") + .name, + "hidden-renamed" + ); } /// An owner may make a channel private or unlisted after the relay has diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index 4a20a072169..ae306eb842f 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -2233,15 +2233,16 @@ pub(crate) async fn persist_edit_metadata( if let Some(labels) = channel_labels_from_tags(event) { edit.columns.labels = Some(labels.map_err(|e| anyhow::anyhow!("{e}"))?); } - // A relay operator without channel authority may rename only a listed - // channel, archived ones included, so that duplicate names can be - // repaired. The store decides that under the channel row lock, so an - // owner who makes the channel private or unlisted meanwhile is seen. + // A relay operator may rename a listed channel, archived ones included, + // whatever the operator's channel role, so that duplicate names can be + // repaired. Without channel authority the rename applies only to a + // listed channel. The store decides both under the channel row lock, so + // an owner who makes the channel private or unlisted meanwhile is seen. // Channel authority is looked up only for an operator's rename. - edit.operator_rename = is_rename_only(event) - && actor_is_relay_operator(state, event.pubkey.to_bytes()).await? - && !actor_has_channel_authority(state, tenant.community(), channel_id, &actor_bytes) - .await?; + edit.operator_rename = + is_rename_only(event) && actor_is_relay_operator(state, event.pubkey.to_bytes()).await?; + edit.channel_authority = edit.operator_rename + && actor_has_channel_authority(state, tenant.community(), channel_id, &actor_bytes).await?; let now = chrono::Utc::now(); let mut notices = Vec::new(); From bb2a68d728603b7b55fa74348700f5437d3a211c Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 2 Nov 2026 10:00:20 -0500 Subject: [PATCH 11/13] fix(scripts): keep a starter name on the clients' fixed starter id Released desktop and mobile apps create general and welcome-everyone at a fixed UUIDv5 id per relay. When an archived channel holds that id, the relay refuses the create there whatever the name, so renaming that channel frees nothing and only moves the clients' own starter off its name. The rename script now computes those ids from the relay URL and passes them to the plan. In an all-archived group, the channel at a starter id keeps the name, and the starter-name rule renames nothing for that name. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- .../listed_channel_name_postgres_tests.rs | 34 +++++++++++++++++-- .../plan_duplicate_listed_channel_renames.sql | 28 ++++++++++----- .../rename_duplicate_listed_channel_names.sh | 25 ++++++++++++-- 3 files changed, 74 insertions(+), 13 deletions(-) diff --git a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs index 29a8bcdacd8..3cae04af688 100644 --- a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs +++ b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs @@ -65,6 +65,15 @@ fn renamed(id: Uuid, name: &str) -> (Uuid, String) { /// The plan for `host`: (channel, current name, new name), as the rename /// script reads it. pub(super) async fn rename_plan(pool: &PgPool, host: &str) -> Vec<(Uuid, String, String)> { + rename_plan_with_starters(pool, host, &[]).await +} + +/// The plan for `host` when the clients' starter channels have `starter_ids`. +async fn rename_plan_with_starters( + pool: &PgPool, + host: &str, + starter_ids: &[Uuid], +) -> Vec<(Uuid, String, String)> { let (_, query) = PLAN_SQL.split_once("\nWITH").expect("plan query"); sqlx::query_as(sqlx::AssertSqlSafe(format!( "WITH{}", @@ -72,8 +81,10 @@ pub(super) async fn rename_plan(pool: &PgPool, host: &str) -> Vec<(Uuid, String, .trim_end() .trim_end_matches(';') .replace(":'host'", "$1") + .replace(":'starter_ids'::uuid[]", "$2::uuid[]") ))) .bind(host) + .bind(starter_ids) .fetch_all(pool) .await .expect("run the rename plan") @@ -168,8 +179,10 @@ async fn plan_keeps_each_listed_name_on_its_oldest_unarchived_channel() { /// Released desktop and mobile apps create `general` and `welcome-everyone` /// when no open, unarchived stream has the name, and the relay refuses that /// create while an archived channel holds it. The plan renames every holder of -/// a starter name when all of them are archived, and no other archived name. -/// Mutation: drop the `all_archived` condition in the plan → RED. +/// a starter name when all of them are archived, and no other archived name, +/// unless one of them is at the clients' starter id. +/// Mutation: drop the `all_archived` or `NOT starter_held` condition in the +/// plan, or the `at_starter_id` sort key → RED. #[tokio::test] #[ignore = "requires Postgres"] async fn plan_frees_a_starter_name_that_only_archived_channels_hold() { @@ -235,6 +248,23 @@ async fn plan_frees_a_starter_name_that_only_archived_channels_hold() { ]; names.sort(); assert_eq!(seed.names(&pool).await, names); + + // An archived `general` at the clients' starter id keeps the name, even + // when an older archived namesake exists: the relay refuses a create at + // that id whatever the name. Only the namesake is renamed. + let fixed = Seed::new(&pool).await; + let [starter, older] = [(); 2].map(|_| Uuid::new_v4()); + fixed.open(&pool, starter, "general", 10).await; + fixed.open(&pool, older, "general", 60).await; + sqlx::query("UPDATE channels SET archived_at = now() WHERE id = ANY($1)") + .bind(vec![starter, older]) + .execute(&pool) + .await + .unwrap(); + assert_eq!( + rename_plan_with_starters(&pool, &fixed.host, &[starter, Uuid::new_v4()]).await, + vec![(older, "general".to_owned(), renamed(older, "general").1)] + ); } #[tokio::test] diff --git a/scripts/maintenance/plan_duplicate_listed_channel_renames.sql b/scripts/maintenance/plan_duplicate_listed_channel_renames.sql index bbb64d69a73..db71686505b 100644 --- a/scripts/maintenance/plan_duplicate_listed_channel_renames.sql +++ b/scripts/maintenance/plan_duplicate_listed_channel_renames.sql @@ -8,8 +8,9 @@ -- (`LISTED_CHANNEL_SQL` in crates/buzz-db/src/store/channel.rs). -- -- In each group of channels that share a name, one channel keeps it: the --- oldest unarchived one, or the oldest archived one when all are archived --- (except a starter name, below). +-- oldest unarchived one, or when all are archived, the clients' starter +-- channel if it is one of them, else the oldest (except a starter name, +-- below). -- Every other channel in the group gets "-", or "-" when another listed channel already has that name. -- It lists nothing for a community that is not active. @@ -18,12 +19,19 @@ -- all the listed channels with that name are archived. Released desktop and -- mobile apps create those starter channels when no open, unarchived stream -- has the name, and the relay refuses that create while an archived channel --- holds the name. The rename frees the name for them. +-- holds the name. The rename frees the name for them. The apps create each +-- starter at a fixed id (`starter_ids`, from the rename script). When an +-- archived channel holds that id, the relay refuses the create at the id +-- whatever the name, so no rename helps: the archived starter then keeps +-- the name, and only its duplicates are renamed. -- --- psql -X -v ON_ERROR_STOP=1 -v host= -f scripts/maintenance/plan_duplicate_listed_channel_renames.sql +-- psql -X -v ON_ERROR_STOP=1 -v host= \ +-- -v starter_ids='{,}' \ +-- -f scripts/maintenance/plan_duplicate_listed_channel_renames.sql WITH listed AS ( - SELECT ch.id, ch.name, ch.archived_at, ch.created_at + SELECT ch.id, ch.name, ch.archived_at, ch.created_at, + ch.id = ANY(:'starter_ids'::uuid[]) AS at_starter_id FROM channels ch JOIN communities co ON co.id = ch.community_id WHERE co.host = :'host' @@ -39,13 +47,17 @@ duplicates AS ( SELECT id, name, row_number() OVER ( PARTITION BY lower(name) - ORDER BY archived_at IS NOT NULL, created_at, id + ORDER BY archived_at IS NOT NULL, + archived_at IS NOT NULL AND NOT at_starter_id, + created_at, id ) AS place, - bool_and(archived_at IS NOT NULL) OVER (PARTITION BY lower(name)) AS all_archived + bool_and(archived_at IS NOT NULL) OVER (PARTITION BY lower(name)) AS all_archived, + bool_or(at_starter_id) OVER (PARTITION BY lower(name)) AS starter_held FROM listed ) ranked WHERE place > 1 - OR (all_archived AND lower(name) IN ('general', 'welcome-everyone')) + OR (all_archived AND NOT starter_held + AND lower(name) IN ('general', 'welcome-everyone')) ) SELECT d.id AS channel_id, d.name, CASE WHEN EXISTS ( diff --git a/scripts/maintenance/rename_duplicate_listed_channel_names.sh b/scripts/maintenance/rename_duplicate_listed_channel_names.sh index 64d7bc422d3..2806740a68f 100755 --- a/scripts/maintenance/rename_duplicate_listed_channel_names.sh +++ b/scripts/maintenance/rename_duplicate_listed_channel_names.sh @@ -37,8 +37,9 @@ # (add-member also finds only an unarchived community), run this script, and # archive it again. For one being deleted, it stops: no rename can land there, # and its duplicates go when its deletion completes. -# BUZZ_RELAY_URL defaults to wss://. BUZZ_CLI defaults to -# `buzz`. +# BUZZ_RELAY_URL defaults to wss://; it must be the URL +# clients use, because it also gives the clients' starter channel ids. +# BUZZ_CLI defaults to `buzz`. The script needs psql and python3. set -euo pipefail usage() { @@ -61,6 +62,24 @@ relay_url=${BUZZ_RELAY_URL:-wss://$host} buzz=${BUZZ_CLI:-buzz} plan_sql="$(cd "$(dirname "$0")" && pwd)/plan_duplicate_listed_channel_renames.sql" +# The ids released desktop and mobile apps create their starter channels at: +# UUIDv5 of "starter-channel:v1::", where the origin +# is the relay URL with wss:// as https:// and ws:// as http:// +# (starter_channel_uuid in desktop/src-tauri/src/commands/channels.rs). The +# plan keeps the name on an archived starter at one of these ids. +starter_scope=${relay_url%/} +case $starter_scope in +wss://*) starter_scope=https://${starter_scope#wss://} ;; +ws://*) starter_scope=http://${starter_scope#ws://} ;; +esac +starter_ids=$(python3 -I -c ' +import sys, uuid +ns = uuid.UUID("3ce33bea-8f09-5f1b-9c85-8a7d2659e6b0") +ids = (uuid.uuid5(ns, "starter-channel:v1:%s:%s" % (sys.argv[1], slug)) + for slug in ("general", "welcome-everyone")) +print("{" + ",".join(map(str, ids)) + "}") +' "$starter_scope") + plan=$(mktemp) trap 'rm -f "$plan"' EXIT @@ -68,7 +87,7 @@ trap 'rm -f "$plan"' EXIT # name, with any control character in it, stays one field. read_plan() { psql "$DATABASE_URL" -X -q -v ON_ERROR_STOP=1 -v host="$host" \ - -A -t -z -0 -f "$plan_sql" >"$plan" + -v starter_ids="$starter_ids" -A -t -z -0 -f "$plan_sql" >"$plan" } community_state=$(psql "$DATABASE_URL" -X -q -v ON_ERROR_STOP=1 -v host="$host" \ From 65d0c0ca9cbdf6e8182524ebcd584d99c3d2a7d7 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 2 Nov 2026 10:34:55 -0500 Subject: [PATCH 12/13] fix(scripts): bind each starter id to its name across every channel row The starter rule looked for a starter id only among the listed channels with that name. A row at the id that was renamed, made private or unlisted, or soft-deleted still makes the relay refuse the create there, so the archived namesakes were renamed for nothing. The plan now checks the id against every channel row in the community, per starter name. The script trims whitespace and every trailing "/" from the origin, as desktop does, accepts BUZZ_STARTER_ORIGIN for a desktop build with its own relay HTTP origin, and prints the ids it computed so they can be checked against a client. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- .../listed_channel_name_postgres_tests.rs | 35 ++++++++++++++-- .../plan_duplicate_listed_channel_renames.sql | 30 ++++++++----- .../rename_duplicate_listed_channel_names.sh | 42 +++++++++++-------- 3 files changed, 77 insertions(+), 30 deletions(-) diff --git a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs index 3cae04af688..cfc3a7a0b63 100644 --- a/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs +++ b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs @@ -180,9 +180,9 @@ async fn plan_keeps_each_listed_name_on_its_oldest_unarchived_channel() { /// when no open, unarchived stream has the name, and the relay refuses that /// create while an archived channel holds it. The plan renames every holder of /// a starter name when all of them are archived, and no other archived name, -/// unless one of them is at the clients' starter id. -/// Mutation: drop the `all_archived` or `NOT starter_held` condition in the -/// plan, or the `at_starter_id` sort key → RED. +/// unless a channel row has that name's starter id. +/// Mutation: drop the `all_archived` or `NOT IN starters` condition in the +/// plan, the `at_starter_id` sort key, or the slug binding in `starters` → RED. #[tokio::test] #[ignore = "requires Postgres"] async fn plan_frees_a_starter_name_that_only_archived_channels_hold() { @@ -265,6 +265,35 @@ async fn plan_frees_a_starter_name_that_only_archived_channels_hold() { rename_plan_with_starters(&pool, &fixed.host, &[starter, Uuid::new_v4()]).await, vec![(older, "general".to_owned(), renamed(older, "general").1)] ); + + // Any row at a starter's id blocks the create there, whatever its name or + // visibility, so the starter rule renames nothing for that name. Each id + // binds only its own name: `welcome-everyone` is still freed. + let moved = Seed::new(&pool).await; + let [general_id, archived_general, archived_welcome] = [(); 3].map(|_| Uuid::new_v4()); + moved.open(&pool, general_id, "general", 90).await; + moved.open(&pool, archived_general, "general", 30).await; + moved + .open(&pool, archived_welcome, "welcome-everyone", 30) + .await; + sqlx::query("UPDATE channels SET name = 'general-old', visibility = 'private' WHERE id = $1") + .bind(general_id) + .execute(&pool) + .await + .unwrap(); + sqlx::query("UPDATE channels SET archived_at = now() WHERE id = ANY($1)") + .bind(vec![archived_general, archived_welcome]) + .execute(&pool) + .await + .unwrap(); + assert_eq!( + rename_plan_with_starters(&pool, &moved.host, &[general_id, Uuid::new_v4()]).await, + vec![( + archived_welcome, + "welcome-everyone".to_owned(), + renamed(archived_welcome, "welcome-everyone").1 + )] + ); } #[tokio::test] diff --git a/scripts/maintenance/plan_duplicate_listed_channel_renames.sql b/scripts/maintenance/plan_duplicate_listed_channel_renames.sql index db71686505b..4ef4a567dc2 100644 --- a/scripts/maintenance/plan_duplicate_listed_channel_renames.sql +++ b/scripts/maintenance/plan_duplicate_listed_channel_renames.sql @@ -20,18 +20,28 @@ -- mobile apps create those starter channels when no open, unarchived stream -- has the name, and the relay refuses that create while an archived channel -- holds the name. The rename frees the name for them. The apps create each --- starter at a fixed id (`starter_ids`, from the rename script). When an --- archived channel holds that id, the relay refuses the create at the id --- whatever the name, so no rename helps: the archived starter then keeps --- the name, and only its duplicates are renamed. +-- starter at a fixed id (`starter_ids`, from the rename script, in the order +-- general, welcome-everyone). When any channel row has that id, whatever +-- its name, visibility or state, the relay refuses the create at the id, so +-- no rename helps: the starter rule then renames nothing for that name. An +-- archived starter at its id also keeps the name over its namesakes. -- -- psql -X -v ON_ERROR_STOP=1 -v host= \ -- -v starter_ids='{,}' \ -- -f scripts/maintenance/plan_duplicate_listed_channel_renames.sql -WITH listed AS ( +WITH starters AS ( + -- The starter names whose fixed id some channel row already has. + SELECT s.slug, s.id + FROM unnest(ARRAY['general', 'welcome-everyone'], :'starter_ids'::uuid[]) AS s(slug, id) + JOIN communities co ON co.host = :'host' + WHERE EXISTS ( + SELECT 1 FROM channels ch WHERE ch.community_id = co.id AND ch.id = s.id + ) +), +listed AS ( SELECT ch.id, ch.name, ch.archived_at, ch.created_at, - ch.id = ANY(:'starter_ids'::uuid[]) AS at_starter_id + ch.id IN (SELECT id FROM starters) AS at_starter_id FROM channels ch JOIN communities co ON co.id = ch.community_id WHERE co.host = :'host' @@ -51,13 +61,13 @@ duplicates AS ( archived_at IS NOT NULL AND NOT at_starter_id, created_at, id ) AS place, - bool_and(archived_at IS NOT NULL) OVER (PARTITION BY lower(name)) AS all_archived, - bool_or(at_starter_id) OVER (PARTITION BY lower(name)) AS starter_held + bool_and(archived_at IS NOT NULL) OVER (PARTITION BY lower(name)) AS all_archived FROM listed ) ranked WHERE place > 1 - OR (all_archived AND NOT starter_held - AND lower(name) IN ('general', 'welcome-everyone')) + OR (all_archived + AND lower(name) IN ('general', 'welcome-everyone') + AND lower(name) NOT IN (SELECT slug FROM starters)) ) SELECT d.id AS channel_id, d.name, CASE WHEN EXISTS ( diff --git a/scripts/maintenance/rename_duplicate_listed_channel_names.sh b/scripts/maintenance/rename_duplicate_listed_channel_names.sh index 2806740a68f..e20c8438b96 100755 --- a/scripts/maintenance/rename_duplicate_listed_channel_names.sh +++ b/scripts/maintenance/rename_duplicate_listed_channel_names.sh @@ -37,9 +37,10 @@ # (add-member also finds only an unarchived community), run this script, and # archive it again. For one being deleted, it stops: no rename can land there, # and its duplicates go when its deletion completes. -# BUZZ_RELAY_URL defaults to wss://; it must be the URL -# clients use, because it also gives the clients' starter channel ids. -# BUZZ_CLI defaults to `buzz`. The script needs psql and python3. +# BUZZ_RELAY_URL defaults to wss://. It also gives the +# clients' starter channel ids unless BUZZ_STARTER_ORIGIN is set (below), so +# it must be the URL clients use. BUZZ_CLI defaults to `buzz`. The script +# needs psql and python3. set -euo pipefail usage() { @@ -63,22 +64,29 @@ buzz=${BUZZ_CLI:-buzz} plan_sql="$(cd "$(dirname "$0")" && pwd)/plan_duplicate_listed_channel_renames.sql" # The ids released desktop and mobile apps create their starter channels at: -# UUIDv5 of "starter-channel:v1::", where the origin -# is the relay URL with wss:// as https:// and ws:// as http:// -# (starter_channel_uuid in desktop/src-tauri/src/commands/channels.rs). The -# plan keeps the name on an archived starter at one of these ids. -starter_scope=${relay_url%/} -case $starter_scope in -wss://*) starter_scope=https://${starter_scope#wss://} ;; -ws://*) starter_scope=http://${starter_scope#ws://} ;; -esac -starter_ids=$(python3 -I -c ' +# UUIDv5 of "starter-channel:v1::" (starter_channel_uuid in +# desktop/src-tauri/src/commands/channels.rs). The origin is the clients' +# relay HTTP origin: BUZZ_STARTER_ORIGIN when set (for a desktop build with +# its own BUZZ_RELAY_HTTP), else the relay URL with wss:// as https:// and +# ws:// as http://, trimmed of whitespace and trailing "/" as desktop does. +# The script prints the ids so they can be checked against a client. +starter=$(python3 -I -c ' import sys, uuid +origin = sys.argv[1].strip().rstrip("/") +if not sys.argv[2]: + for ws, http in (("wss://", "https://"), ("ws://", "http://")): + if origin.startswith(ws): + origin = http + origin[len(ws):] + break ns = uuid.UUID("3ce33bea-8f09-5f1b-9c85-8a7d2659e6b0") -ids = (uuid.uuid5(ns, "starter-channel:v1:%s:%s" % (sys.argv[1], slug)) - for slug in ("general", "welcome-everyone")) -print("{" + ",".join(map(str, ids)) + "}") -' "$starter_scope") +ids = [str(uuid.uuid5(ns, "starter-channel:v1:%s:%s" % (origin.strip(), slug))) + for slug in ("general", "welcome-everyone")] +print(origin) +print("{" + ",".join(ids) + "}") +' "${BUZZ_STARTER_ORIGIN:-$relay_url}" "${BUZZ_STARTER_ORIGIN:-}") +starter_origin=${starter%%$'\n'*} +starter_ids=${starter#*$'\n'} +echo "starter channel ids for $starter_origin (general, welcome-everyone): $starter_ids" >&2 plan=$(mktemp) trap 'rm -f "$plan"' EXIT From ba4039eab826e315aecab615b4f3663e34257b3f Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Mon, 2 Nov 2026 12:03:07 -0500 Subject: [PATCH 13/13] docs(relay): fix stale comments around the listed-name rule Put the NIP-OA owner doc back on actor_owns_any_owner_agent, name migration 0081 in the index comment, and stop describing kind:9007's h tag as optional now that the create path refuses a create without one. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Logan Johnson --- crates/buzz-db/src/store/channel.rs | 4 ++-- crates/buzz-relay/src/handlers/ingest.rs | 5 +++-- crates/buzz-relay/src/handlers/side_effects.rs | 12 ++++++------ 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index f3d8c9ca0a1..97f82d40c53 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -510,10 +510,10 @@ async fn ensure_listed_channel_name_unique( } /// The unique index on `(community_id, lower(name))` over -/// [`LISTED_CHANNEL_SQL`] channels that migration 0079 adds. It refuses a +/// [`LISTED_CHANNEL_SQL`] channels that migration 0081 adds. It refuses a /// duplicate at the write itself, before /// [`ensure_listed_channel_name_unique`] runs, so this release maps the -/// refusal too: it can run beside that index during the rollout of 0079 and +/// refusal too: it can run beside that index during the rollout of 0081 and /// after a rollback. const LISTED_NAME_INDEX: &str = "idx_channels_listed_name"; diff --git a/crates/buzz-relay/src/handlers/ingest.rs b/crates/buzz-relay/src/handlers/ingest.rs index c02d6627737..9cae758646e 100644 --- a/crates/buzz-relay/src/handlers/ingest.rs +++ b/crates/buzz-relay/src/handlers/ingest.rs @@ -2848,7 +2848,7 @@ async fn ingest_event_inner( // Channel-scoped tokens cannot publish global events — that would bypass // the token's channel restriction. This covers kind:1 (global text notes), // kind:3 (contact lists), kind:0 (profiles), and kind:9007 (create-group - // without an h-tag, which would auto-assign a server UUID). + // without an h-tag, which the create path also refuses). return Err(IngestError::AuthFailed( "restricted: channel-scoped tokens cannot publish global events".into(), )); @@ -4282,7 +4282,8 @@ mod postgres_tests { #[test] fn create_group_does_not_require_h_tag() { - // kind:9007 creates the channel — h-tag is optional (client-chosen UUID) + // kind:9007 names its new channel in h, but the create path checks that + // itself (it refuses a create without one), not requires_h_channel_scope assert!(!requires_h_channel_scope(KIND_NIP29_CREATE_GROUP)); } diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index ae306eb842f..61228f4e136 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -539,12 +539,6 @@ fn check_deletion_privacy_k_tag(event: &Event, target_kind: u32) -> anyhow::Resu Ok(()) } -/// Returns `true` if `actor_bytes` is the NIP-OA owner of **any** active owner-role -/// member in `members`. Used by kind:9002 and kind:9008 to authorize the owning -/// human of a channel's agent-owner(s) even when the human is not a channel member. -/// -/// Checking all active owners (not just the first) is correct under co-ownership: -/// an agent may be promoted to owner later, or multiple agents may co-own a channel. /// True when `actor` may change a channel's privileged metadata: a channel /// owner or admin, or the owning human of an active owner-role agent in the /// channel, even when that human is not a member (this diverges from @@ -565,6 +559,12 @@ async fn actor_has_channel_authority( actor_owns_any_owner_agent(state, community_id, &members, actor_bytes).await } +/// Returns `true` if `actor_bytes` is the NIP-OA owner of **any** active owner-role +/// member in `members`. Used by kind:9002 and kind:9008 to authorize the owning +/// human of a channel's agent-owner(s) even when the human is not a channel member. +/// +/// Checking all active owners (not just the first) is correct under co-ownership: +/// an agent may be promoted to owner later, or multiple agents may co-own a channel. async fn actor_owns_any_owner_agent( state: &Arc, community_id: buzz_core::CommunityId,