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 60d96a505d..5bfcc5539e 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,6 +192,60 @@ async fn plan_picks_a_free_name_when_the_short_one_is_taken() { drop_scratch_db(&admin, pool, &name).await; } +/// 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 @@ -209,38 +263,14 @@ async fn script_renames_each_planned_channel_once_whatever_its_name() { seed.open(&pool, duplicate, &tricky, 10).await; seed.open(&pool, other, "other", 20).await; - 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\ + 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", ) - .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 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"); + .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 aa9d35487b..42c445ccd7 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 e052eddd69..0cf4e89033 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -647,22 +647,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", - ]; 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)" @@ -773,17 +761,10 @@ pub async fn validate_admin_event( // name/about/archived/visibility/ttl/labels/join_role/unlisted // require owner/admin; // topic/purpose allow any member who may post. - 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_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); @@ -803,14 +784,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!( @@ -1243,25 +1222,51 @@ 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"]; + +/// 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) +} + +/// 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") } @@ -2145,9 +2150,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 314e3ff3c0..521c6893c8 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