diff --git a/crates/buzz-core/src/channel.rs b/crates/buzz-core/src/channel.rs index 3e3a7b10904..35cb6d07dd3 100644 --- a/crates/buzz-core/src/channel.rs +++ b/crates/buzz-core/src/channel.rs @@ -405,12 +405,12 @@ impl ChannelType { } /// Visibility of a new channel of this type when its create event has no - /// `visibility` tag. A system channel holds access-control data, so it is - /// private unless its creator asks for open. + /// `visibility` tag. DMs are private. A system channel holds access-control + /// data, so it is private unless its creator asks for open. pub fn default_visibility(&self) -> ChannelVisibility { match self { - Self::System => ChannelVisibility::Private, - Self::Stream | Self::Forum | Self::Dm | Self::Workflow => ChannelVisibility::Open, + Self::Dm | Self::System => ChannelVisibility::Private, + Self::Stream | Self::Forum | Self::Workflow => ChannelVisibility::Open, } } } @@ -550,9 +550,27 @@ mod tests { use super::{ canonical_channel_name, channel_name_error, channel_type_from_group_state_tags, group_state_identity_tags, parse_channel_labels, ChannelJoinRole, ChannelLabelError, - ChannelType, MemberRole, LEGACY_CHANNEL_TYPES, MAX_CHANNEL_LABELS, RESERVED_CHANNEL_LABELS, + ChannelType, ChannelVisibility, MemberRole, LEGACY_CHANNEL_TYPES, MAX_CHANNEL_LABELS, + RESERVED_CHANNEL_LABELS, }; + #[test] + fn dm_visibility_defaults_private_without_changing_other_channel_types() { + for (channel_type, visibility) in [ + (ChannelType::Dm, ChannelVisibility::Private), + (ChannelType::System, ChannelVisibility::Private), + (ChannelType::Stream, ChannelVisibility::Open), + (ChannelType::Forum, ChannelVisibility::Open), + (ChannelType::Workflow, ChannelVisibility::Open), + ] { + assert_eq!( + channel_type.default_visibility(), + visibility, + "{channel_type}" + ); + } + } + #[test] fn channel_name_error_matches_the_column_limit() { assert_eq!( diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index b868ed8b612..21a7206d38b 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -120,6 +120,7 @@ pub async fn create_channel( labels: &[String], unlisted: bool, ) -> Result { + validate_channel_visibility(channel_type.as_str(), visibility.as_str())?; if created_by.len() != 32 { return Err(DbError::InvalidData(format!( "pubkey must be 32 bytes, got {}", @@ -367,6 +368,7 @@ async fn insert_channel_rows( unlisted, members, } = channel; + validate_channel_visibility(channel_type.as_str(), visibility.as_str())?; let community_id = tx.community(); if created_by.len() != 32 { return Err(DbError::InvalidData(format!( @@ -460,6 +462,14 @@ async fn insert_channel_rows( Ok(was_created) } +fn validate_channel_visibility(channel_type: &str, visibility: &str) -> Result<()> { + if channel_type == ChannelType::Dm.as_str() && visibility != ChannelVisibility::Private.as_str() + { + return Err(DbError::AccessDenied("DMs must be private".into())); + } + Ok(()) +} + /// The unique index (migration 0083) that keeps two channels the channel /// directory lists from sharing a name in a community, ignoring case. It /// covers open, not unlisted streams and forums that are not deleted; an @@ -1168,6 +1178,7 @@ pub(crate) struct ChannelSettingsWriter<'tx> { admission: bool, ttl: TtlLock, previous_visibility: String, + channel_type: String, archived: bool, listed: bool, } @@ -1278,9 +1289,9 @@ 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, listed): (String, bool, bool) = + let (previous_visibility, channel_type, archived, listed): (String, String, bool, bool) = sqlx::query_as(sqlx::AssertSqlSafe(format!( - "SELECT visibility::text, archived_at IS NOT NULL, {LISTED_CHANNEL_SQL} FROM channels \ + "SELECT visibility::text, channel_type::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()) @@ -1294,6 +1305,7 @@ async fn lock_channel_row( admission, ttl, previous_visibility, + channel_type, archived, listed, }) @@ -1306,6 +1318,9 @@ async fn update_channel_on( mut updates: ChannelUpdate, ) -> Result<()> { w.check_covers(&updates)?; + if let Some(visibility) = updates.visibility.as_deref() { + validate_channel_visibility(&w.channel_type, visibility)?; + } let channel_id = w.channel_id; let tx = &mut *w.tx; let community_id = tx.community(); @@ -2762,6 +2777,130 @@ mod postgres_tests { } } + /// Every generic creator and the shared settings writer enforce DM + /// privacy. Other channel types retain both visibility settings. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn dm_visibility_is_enforced_by_channel_writers() { + 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(); + let counts = || async { + sqlx::query_as::<_, (i64, i64, i64)>( + "SELECT (SELECT count(*) FROM channels WHERE community_id=$1), \ + (SELECT count(*) FROM channel_members WHERE community_id=$1), \ + (SELECT count(*) FROM events WHERE community_id=$1)", + ) + .bind(community.as_uuid()) + .fetch_one(&pool) + .await + .expect("count durable create effects") + }; + + for channel_type in [ + ChannelType::Dm, + ChannelType::Stream, + ChannelType::Forum, + ChannelType::Workflow, + ChannelType::System, + ] { + for visibility in [ChannelVisibility::Open, ChannelVisibility::Private] { + for writer in ["generated", "supplied", "event"] { + let id = Uuid::new_v4(); + let name = format!("visibility-{id}"); + let before = counts().await; + let result = match writer { + "generated" => create_channel( + &pool, + community, + &name, + channel_type, + visibility, + None, + &creator_bytes, + None, + &[], + false, + ) + .await + .map(|channel| channel.id), + "supplied" => create_channel_with_id( + &pool, + community, + id, + &name, + channel_type, + visibility, + None, + &creator_bytes, + None, + &[], + ) + .await + .map(|(channel, created)| { + assert!(created); + channel.id + }), + _ => create_channel_with_event( + &pool, + None, + community, + id, + NewChannel { + name: &name, + channel_type, + visibility, + ..new_channel(&creator_bytes, &[]) + }, + &create_event(&creator, id), + ) + .await + .map(|created| { + assert!(matches!(created, ChannelCreate::Created(_))); + id + }), + }; + if channel_type == ChannelType::Dm && visibility == ChannelVisibility::Open { + assert!( + matches!(result, Err(DbError::AccessDenied(ref reason)) + if reason == "DMs must be private"), + "{writer}: {result:?}" + ); + assert_eq!(counts().await, before, "{writer}: refused create is atomic"); + continue; + } + let id = result.expect("supported channel visibility"); + for visibility in [ChannelVisibility::Open, ChannelVisibility::Private] { + let before = get_channel(&pool, community, id).await.expect("channel"); + let result = update_channel( + &pool, + community, + id, + ChannelUpdate { + description: Some("updated together".into()), + visibility: Some(visibility.as_str().into()), + ..Default::default() + }, + ) + .await; + if channel_type == ChannelType::Dm && visibility == ChannelVisibility::Open + { + assert!(matches!(result, Err(DbError::AccessDenied(_)))); + let after = get_channel(&pool, community, id).await.expect("channel"); + assert_eq!(after.visibility, before.visibility); + assert_eq!(after.description, before.description); + } else { + let after = result.expect("supported metadata update"); + assert_eq!(after.visibility, visibility.as_str()); + assert_eq!(after.description.as_deref(), Some("updated together")); + } + } + } + } + } + } + /// Holds a create open on its own transaction (channel row and event both /// written, not committed) while the same create runs again beside it. async fn resend_beside_open_create(commit_first: bool) -> ChannelCreate { diff --git a/crates/buzz-relay/src/handlers/create_with_members_postgres_tests.rs b/crates/buzz-relay/src/handlers/create_with_members_postgres_tests.rs index de8327ddb4e..e1754dd0bab 100644 --- a/crates/buzz-relay/src/handlers/create_with_members_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/create_with_members_postgres_tests.rs @@ -8,7 +8,9 @@ use buzz_db::event::EventQuery; use nostr::{Event, EventBuilder, Keys, Kind, Tag}; use uuid::Uuid; -use crate::handlers::group_state_postgres_tests::{community, group_state_tags, http_auth, state}; +use crate::handlers::group_state_postgres_tests::{ + community, group_state_tags, http_auth, req_event_ids, state, +}; use crate::handlers::ingest::{ingest_event, IngestError, IngestResult}; use crate::state::AppState; @@ -60,6 +62,109 @@ async fn roles( roles } +/// Generic private DMs remain supported, with or without an explicit +/// visibility. An explicit open DM is refused without durable side effects. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn dm_visibility_create_requests_are_atomic() { + let state = state().await; + let tenant = community(&state, "dm-visibility-create").await; + let (creator, participant, outsider) = (Keys::generate(), Keys::generate(), Keys::generate()); + for visibility in [Some("open"), None, Some("private")] { + let channel_id = Uuid::new_v4(); + let mut tags = vec![ + Tag::parse(["h", &channel_id.to_string()]).unwrap(), + Tag::parse(["name", "Direct message"]).unwrap(), + Tag::parse(["channel_type", "dm"]).unwrap(), + Tag::parse(["p", &participant.public_key().to_hex()]).unwrap(), + ]; + if let Some(visibility) = visibility { + tags.push(Tag::parse(["visibility", visibility]).unwrap()); + } + let event = EventBuilder::new(Kind::Custom(9007), "") + .tags(tags) + .sign_with_keys(&creator) + .expect("sign DM creation"); + let result = ingest(&state, &tenant, event, &creator).await; + if visibility == Some("open") { + let error = result.err().expect("open DM must be refused"); + assert!( + matches!(error, IngestError::Rejected(ref reason) + if reason == "invalid: DMs must be private"), + "{error:?}" + ); + let counts: (i64, i64, i64) = sqlx::query_as( + "SELECT (SELECT count(*) FROM channels WHERE community_id=$1), \ + (SELECT count(*) FROM channel_members WHERE community_id=$1), \ + (SELECT count(*) FROM events WHERE community_id=$1)", + ) + .bind(tenant.community().as_uuid()) + .fetch_one(state.db.pool()) + .await + .expect("count refused create effects"); + assert_eq!( + counts, + (0, 0, 0), + "no channel, roster, event or notice survives" + ); + continue; + } + assert!(result.expect("private DM creation").accepted); + let channel = state + .db + .get_channel(tenant.community(), channel_id) + .await + .expect("DM"); + assert_eq!(channel.channel_type, "dm"); + assert_eq!(channel.visibility, "private"); + assert_eq!(roles(&state, &tenant, channel_id).await.len(), 2); + let metadata = group_state_tags(&state, &tenant, channel_id, 39000).await; + assert!(metadata.iter().any(|tag| tag[0] == "private")); + assert!(!metadata.iter().any(|tag| tag[0] == "public")); + + let message = message_event(&participant, channel_id); + let id = message.id.to_hex(); + assert!( + ingest(&state, &tenant, message, &participant) + .await + .expect("participant posts") + .accepted + ); + for reader in [&creator, &participant] { + let ids = req_event_ids( + &state, + &tenant, + reader, + serde_json::json!({"kinds": [9], "#h": [channel_id.to_string()]}), + ) + .await; + assert!(ids.contains(&id), "participants can read their DM"); + } + let ids = req_event_ids( + &state, + &tenant, + &outsider, + serde_json::json!({"kinds": [9, 39000, 39001, 39002]}), + ) + .await; + assert!( + ids.is_empty(), + "global history does not expose DMs or their rosters" + ); + assert!( + ingest( + &state, + &tenant, + message_event(&outsider, channel_id), + &outsider + ) + .await + .is_err(), + "an outsider cannot post to a DM" + ); + } +} + /// The `p` tags join in the create's transaction, the roster lists them, and /// each gets the "joined" notice a kind:9000 would post. /// Mutation: drop `members` from the create → no agent membership → RED. diff --git a/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs b/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs index 54283d5f86c..5edca0bf01b 100644 --- a/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs @@ -8,7 +8,7 @@ use uuid::Uuid; use crate::handlers::group_state_postgres_tests::{ community, create_channel, edit_channel, group_state_tags, http_auth, state, }; -use crate::handlers::ingest::ingest_event; +use crate::handlers::ingest::{ingest_event, IngestError}; /// Count stored events of `kind` in `channel_id`. async fn count_kind( @@ -29,6 +29,85 @@ async fn count_kind( .len() } +/// The database refuses the final visibility under the settings lock. The +/// event, other metadata changes, notices and discovery updates roll back. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn dm_visibility_edits_are_atomic() { + let state = state().await; + let tenant = community(&state, "dm-visibility-edit").await; + let owner = Keys::generate(); + let channel_id = create_channel( + &state, + &tenant, + &owner, + &[&["channel_type", "dm"], &["visibility", "private"]], + ) + .await; + let before = state + .db + .get_channel(tenant.community(), channel_id) + .await + .expect("DM"); + let metadata_before = group_state_tags(&state, &tenant, channel_id, 39000).await; + let notices_before = count_kind(&state, &tenant, channel_id, 40099).await; + let event = EventBuilder::new(Kind::Custom(9002), "") + .tags([ + Tag::parse(["h", &channel_id.to_string()]).unwrap(), + Tag::parse(["about", "must roll back"]).unwrap(), + Tag::parse(["visibility", "private"]).unwrap(), + Tag::parse(["visibility", "open"]).unwrap(), + ]) + .sign_with_keys(&owner) + .expect("sign metadata edit"); + for _ in 0..2 { + let result = ingest_event(&state, &tenant, event.clone(), http_auth(&owner)).await; + let error = result.err().expect("DM cannot become open"); + assert!( + matches!(error, IngestError::Rejected(ref reason) + if reason == "invalid: DMs must be private"), + "{error:?}" + ); + let after = state + .db + .get_channel(tenant.community(), channel_id) + .await + .expect("DM"); + assert_eq!(after.visibility, "private"); + assert_eq!(after.description, before.description); + assert_eq!(count_kind(&state, &tenant, channel_id, 9002).await, 0); + assert_eq!( + count_kind(&state, &tenant, channel_id, 40099).await, + notices_before + ); + assert_eq!( + group_state_tags(&state, &tenant, channel_id, 39000).await, + metadata_before + ); + } + // Repeated settings retain their established last-value-wins behavior. + edit_channel( + &state, + &tenant, + &owner, + channel_id, + &[ + &["visibility", "open"], + &["visibility", "private"], + &["about", "updated privately"], + ], + ) + .await + .expect("private metadata edit"); + let after = state + .db + .get_channel(tenant.community(), channel_id) + .await + .expect("DM"); + assert_eq!(after.visibility, "private"); + assert_eq!(after.description.as_deref(), Some("updated privately")); +} + /// A change that fails inside the write stores nothing and changes nothing, /// and the same event succeeds once the failure is gone. A test trigger makes /// the topic UPDATE fail after the event row and the `about` change were diff --git a/crates/buzz-relay/src/handlers/ingest.rs b/crates/buzz-relay/src/handlers/ingest.rs index 46a1c81f282..34093961b3a 100644 --- a/crates/buzz-relay/src/handlers/ingest.rs +++ b/crates/buzz-relay/src/handlers/ingest.rs @@ -3602,6 +3602,9 @@ async fn ingest_event_inner( .create_channel_with_event(tenant.community(), create.channel_id, channel, &event) .await .map_err(|e| match e { + buzz_db::DbError::AccessDenied(reason) => { + IngestError::Rejected(format!("invalid: {reason}")) + } buzz_db::DbError::ChannelNameTaken(_) => { IngestError::Rejected(format!("invalid: {e}")) }