Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions crates/buzz-core/src/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
}
Expand Down Expand Up @@ -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!(
Expand Down
143 changes: 141 additions & 2 deletions crates/buzz-db/src/store/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ pub async fn create_channel(
labels: &[String],
unlisted: bool,
) -> Result<ChannelRecord> {
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 {}",
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1168,6 +1178,7 @@ pub(crate) struct ChannelSettingsWriter<'tx> {
admission: bool,
ttl: TtlLock,
previous_visibility: String,
channel_type: String,
archived: bool,
listed: bool,
}
Expand Down Expand Up @@ -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())
Expand All @@ -1294,6 +1305,7 @@ async fn lock_channel_row(
admission,
ttl,
previous_visibility,
channel_type,
archived,
listed,
})
Expand All @@ -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();
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down
Loading
Loading