diff --git a/NOSTR.md b/NOSTR.md index 1450c0cdf33..b3d628786f4 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, 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/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..cfc3a7a0b63 --- /dev/null +++ b/crates/buzz-db/src/runtime/tests/listed_channel_name_postgres_tests.rs @@ -0,0 +1,481 @@ +//! 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 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(&host) + .execute(pool) + .await + .expect("insert community"); + Self { + community, + host, + 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])) +} + +/// 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{}", + query + .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") +} + +/// 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 plan_keeps_each_listed_name_on_its_oldest_unarchived_channel() { + 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; + 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(); + // 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; + // 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; + + 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()), + ]; + names.sort(); + 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, +/// 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() { + 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); + + // 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)] + ); + + // 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] +#[ignore = "requires Postgres"] +async fn plan_picks_a_free_name_when_the_short_one_is_taken() { + 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); + 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; + + 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); +} + +/// 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)] +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, + 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()], 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}"); + } + let output = std::process::Command::new(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../scripts/maintenance/rename_duplicate_listed_channel_names.sh" + )) + .arg(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"); + + // 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 (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)"); + 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, + 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", + ) + .await; + assert!( + output.status.success(), + "script failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let new_name = renamed(duplicate, &tricky).1; + 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()), + (other, "other".to_owned()), + ]; + names.sort(); + assert_eq!(seed.names(&pool).await, names); +} + +/// 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 (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, 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}"); +} + +/// 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 (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] { + 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, 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}"); + } +} diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index 36ae4a1dc6b..97f82d40c53 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 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 0081 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', …)`. /// @@ -977,6 +1056,7 @@ pub(crate) struct ChannelSettingsWriter<'tx> { ttl: TtlLock, previous_visibility: String, archived: bool, + listed: bool, } impl ChannelSettingsWriter<'_> { @@ -991,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 { @@ -1079,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, @@ -1095,6 +1182,7 @@ async fn lock_channel_row( ttl, previous_visibility, archived, + listed, }) } @@ -1193,21 +1281,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", @@ -1408,6 +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, 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`]. @@ -1459,12 +1564,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 && !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 && 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())); @@ -1500,6 +1613,8 @@ pub(crate) async fn apply_channel_metadata_event( topic, purpose, archived, + operator_rename: _, + channel_authority: _, } = edit; let actor = event.pubkey.to_bytes(); if !columns.is_empty() { @@ -2570,6 +2685,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/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 0e3859c05b9..b09c390fe56 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 @@ -1074,3 +1044,412 @@ 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() { + 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"); +} + +/// 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, 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() { + 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); + } + + // 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, + 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"); + 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() + .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 +/// 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. +/// 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/ingest.rs b/crates/buzz-relay/src/handlers/ingest.rs index 70ebc75a9d8..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(), )); @@ -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(), + )); } } @@ -3559,8 +3566,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 +3636,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) { @@ -4268,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 8f2977bb04e..61228f4e136 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -539,6 +539,26 @@ fn check_deletion_privacy_k_tag(event: &Event, target_kind: u32) -> anyhow::Resu Ok(()) } +/// 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 +} + /// 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. @@ -654,23 +674,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,45 +831,32 @@ 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); - 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(()); - } - 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). @@ -894,7 +888,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" @@ -1297,6 +1291,71 @@ 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 { + let mut changes = event + .tags + .iter() + .map(|t| t.kind().to_string()) + .filter(|kind| is_metadata_tag(kind)); + 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, @@ -2174,6 +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 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?; + 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(); @@ -2508,17 +2577,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 +2620,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..4ef4a567dc2 --- /dev/null +++ b/scripts/maintenance/plan_duplicate_listed_channel_renames.sql @@ -0,0 +1,78 @@ +-- 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 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. +-- +-- 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. The apps create each +-- 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 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 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' + 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, + 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 + FROM listed + ) ranked + WHERE place > 1 + 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 ( + 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..e20c8438b96 --- /dev/null +++ b/scripts/maintenance/rename_duplicate_listed_channel_names.sh @@ -0,0 +1,167 @@ +#!/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. 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. 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`, 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://. 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() { + 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" + +# The ids released desktop and mobile apps create their starter channels at: +# 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 = [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 + +# 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" \ + -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" \ + -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\`, add this key as a member," \ + "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 +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" + 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 + 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