From f95b7aec084646693d68f86c3360fce85dc74083 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 2 Jul 2026 19:54:01 -0400 Subject: [PATCH] fix(relay): allow owner-of-agent to delete agent messages, surface delete errors in desktop The desktop grants the delete button to self-author OR owner-of-agent via `canManageMessage`, but the relay's kind:5 validator only honored self-author. An owner deleting their agent's message received a silent relay rejection because `useDeleteMessageMutation` had no `onError` handler. Relay (validate_standard_deletion_event, side_effects.rs): - e-tag branch: when target_author != actor, fall through to is_agent_owner before rejecting. If owner, re-gate on channel membership so a removed private-channel member cannot delete their agent's historic messages after access is revoked. Open channels pass without membership. Mirrors the membership re-gate from validate_edit_ownership. - a-tag branch: same is_agent_owner fallback. Membership re-gate applies when the deletion event carries a resolvable h-tag channel context; global addressable events (no channel context) pass on owner-check alone. - Self-author fast-path unchanged: no membership re-gate on delete. Desktop (hooks.ts): - Add onError to useDeleteMessageMutation that surfaces a toast so relay rejections are visible rather than silently swallowed. Tests (side_effects.rs): five DB-backed integration tests with graceful-skip when DATABASE_URL is unavailable: - delete_own_message_succeeds - delete_owner_agent_message_as_member_succeeds - delete_non_owner_agent_message_is_rejected - delete_owner_agent_message_removed_from_private_channel_is_rejected - delete_owner_agent_message_in_open_channel_without_membership_succeeds Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../buzz-relay/src/handlers/side_effects.rs | 436 +++++++++++++++++- desktop/src/features/messages/hooks.ts | 4 + 2 files changed, 436 insertions(+), 4 deletions(-) diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index c5485721b0..a7ce2e904c 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -180,7 +180,7 @@ pub async fn validate_standard_deletion_event( let target_ids = extract_target_event_ids(event); if !has_e_tag(event) { - // a-tag deletion: verify author owns the addressable event + // a-tag deletion: verify author owns the addressable event. let a_tag = event .tags .iter() @@ -194,7 +194,35 @@ pub async fn validate_standard_deletion_event( let target_pubkey_bytes = hex::decode(parts[1]).map_err(|_| anyhow::anyhow!("invalid pubkey in a-tag"))?; if target_pubkey_bytes != actor_bytes { - return Err(anyhow::anyhow!("must be event author")); + // Not self-author: check whether actor owns the target-pubkey agent. + let is_owner = state + .db + .is_agent_owner(tenant.community(), &target_pubkey_bytes, &actor_bytes) + .await + .map_err(|e| anyhow::anyhow!("db error checking agent ownership: {e}"))?; + if !is_owner { + return Err(anyhow::anyhow!("must be event author")); + } + // Owner confirmed. Re-gate on membership when a channel context is resolvable + // (parity with the e-tag owner path below; if no channel context, owner-check alone + // is sufficient — the addressable event may be global). + if let Some(ch_id) = extract_h_tag_channel(event) { + let is_member = state + .is_member_cached(tenant.community(), ch_id, &actor_bytes) + .await + .map_err(|e| anyhow::anyhow!("db error checking membership: {e}"))?; + if !is_member { + let is_open = state + .db + .get_channel(tenant.community(), ch_id) + .await + .map(|ch| ch.visibility == "open") + .unwrap_or(false); + if !is_open { + return Err(anyhow::anyhow!("restricted: not a channel member")); + } + } + } } return Ok(()); } @@ -208,8 +236,39 @@ pub async fn validate_standard_deletion_event( let target_author = effective_message_author(&target_event.event, &state.relay_keypair.public_key()); - if target_author != actor_bytes { - return Err(anyhow::anyhow!("must be event author")); + if target_author == actor_bytes { + // Self-author fast-path: accepted without membership re-gate, matching current + // behavior (a removal from a private channel revokes future send access; it does + // not retroactively block deleting their own historic messages). + } else { + // Not self-author: check whether actor owns the agent that authored the target. + let is_owner = state + .db + .is_agent_owner(tenant.community(), &target_author, &actor_bytes) + .await + .map_err(|e| anyhow::anyhow!("db error checking agent ownership: {e}"))?; + if !is_owner { + return Err(anyhow::anyhow!("must be event author")); + } + // Owner confirmed. Re-gate on channel membership so that a removed private-channel + // member cannot delete their agent's messages after access is revoked. + if let Some(ch_id) = target_event.channel_id { + let is_member = state + .is_member_cached(tenant.community(), ch_id, &actor_bytes) + .await + .map_err(|e| anyhow::anyhow!("db error checking membership: {e}"))?; + if !is_member { + let is_open = state + .db + .get_channel(tenant.community(), ch_id) + .await + .map(|ch| ch.visibility == "open") + .unwrap_or(false); + if !is_open { + return Err(anyhow::anyhow!("restricted: not a channel member")); + } + } + } } } @@ -2962,3 +3021,372 @@ fn topic_for_subscription(channel_id: Option) -> EventTopic { None => EventTopic::Global, } } + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use nostr::{EventBuilder, Keys, Kind, Tag}; + use uuid::Uuid; + + use super::validate_standard_deletion_event; + use crate::config::Config; + use buzz_core::tenant::TenantContext; + + // ── helpers ────────────────────────────────────────────────────────────── + + async fn test_pool() -> Option { + let url = std::env::var("DATABASE_URL") + .unwrap_or_else(|_| "postgres://buzz:buzz_dev@localhost:5432/buzz".into()); + sqlx::PgPool::connect(&url).await.ok() + } + + async fn test_state(pool: sqlx::PgPool) -> Option> { + let db = buzz_db::Db::from_pool(pool.clone()); + let config = Config::from_env().ok()?; + let redis_pool = deadpool_redis::Config::from_url(&config.redis_url) + .create_pool(Some(deadpool_redis::Runtime::Tokio1)) + .ok()?; + let pubsub = Arc::new( + buzz_pubsub::PubSubManager::new(&config.redis_url, redis_pool.clone()) + .await + .ok()?, + ); + let audit = buzz_audit::AuditService::new(pool.clone()); + let auth = buzz_auth::AuthService::new(config.auth.clone()); + let search = buzz_search::SearchService::new(pool.clone()); + let workflow_engine = Arc::new(buzz_workflow::WorkflowEngine::new( + db.clone(), + buzz_workflow::WorkflowConfig::default(), + )); + let media_storage = buzz_media::MediaStorage::new(&config.media).ok()?; + let (state, _) = crate::state::AppState::new( + config, + db, + redis_pool, + audit, + pubsub, + auth, + search, + workflow_engine, + Keys::generate(), + media_storage, + ); + Some(Arc::new(state)) + } + + async fn seed_community(pool: &sqlx::PgPool) -> TenantContext { + let id = Uuid::new_v4(); + let host = format!("del-test-{}.example", id.simple()); + sqlx::query("INSERT INTO communities (id, host) VALUES ($1, $2)") + .bind(id) + .bind(&host) + .execute(pool) + .await + .expect("insert community"); + TenantContext::resolved(buzz_core::tenant::CommunityId::from_uuid(id), host) + } + + /// Insert a user row (no agent_owner_pubkey). + async fn insert_user(pool: &sqlx::PgPool, community_id: Uuid, pubkey: &[u8]) { + sqlx::query( + "INSERT INTO users (community_id, pubkey) VALUES ($1, $2) ON CONFLICT DO NOTHING", + ) + .bind(community_id) + .bind(pubkey) + .execute(pool) + .await + .expect("insert user"); + } + + /// Insert a user row with agent_owner_pubkey set (marks `pubkey` as an agent owned by `owner`). + async fn insert_agent_with_owner( + pool: &sqlx::PgPool, + community_id: Uuid, + pubkey: &[u8], + owner_pubkey: &[u8], + ) { + sqlx::query( + "INSERT INTO users (community_id, pubkey, agent_owner_pubkey) \ + VALUES ($1, $2, $3) ON CONFLICT (community_id, pubkey) DO \ + UPDATE SET agent_owner_pubkey = EXCLUDED.agent_owner_pubkey", + ) + .bind(community_id) + .bind(pubkey) + .bind(owner_pubkey) + .execute(pool) + .await + .expect("insert agent with owner"); + } + + /// Insert a private channel; returns its UUID. + async fn insert_private_channel(pool: &sqlx::PgPool, community_id: Uuid) -> Uuid { + let ch_id = Uuid::new_v4(); + sqlx::query( + "INSERT INTO channels (id, community_id, name, visibility, created_by) \ + VALUES ($1, $2, $3, 'private', $4)", + ) + .bind(ch_id) + .bind(community_id) + .bind(format!("test-{}", ch_id.simple())) + .bind(vec![0u8; 32]) + .execute(pool) + .await + .expect("insert channel"); + ch_id + } + + /// Insert an open channel; returns its UUID. + async fn insert_open_channel(pool: &sqlx::PgPool, community_id: Uuid) -> Uuid { + let ch_id = Uuid::new_v4(); + sqlx::query( + "INSERT INTO channels (id, community_id, name, visibility, created_by) \ + VALUES ($1, $2, $3, 'open', $4)", + ) + .bind(ch_id) + .bind(community_id) + .bind(format!("test-{}", ch_id.simple())) + .bind(vec![0u8; 32]) + .execute(pool) + .await + .expect("insert open channel"); + ch_id + } + + /// Add a member to a channel. + async fn add_member(pool: &sqlx::PgPool, community_id: Uuid, channel_id: Uuid, pubkey: &[u8]) { + sqlx::query( + "INSERT INTO channel_members (community_id, channel_id, pubkey, role, invited_by) \ + VALUES ($1, $2, $3, 'member', $4) ON CONFLICT DO NOTHING", + ) + .bind(community_id) + .bind(channel_id) + .bind(pubkey) + .bind(pubkey) + .execute(pool) + .await + .expect("add member"); + } + + /// Build a signed kind:5 delete event with one e-tag targeting `target_id_hex`. + fn delete_event_e_tag(actor_keys: &Keys, target_id_hex: &str) -> nostr::Event { + let tags = vec![Tag::parse(["e", target_id_hex]).unwrap()]; + EventBuilder::new(Kind::Custom(5), "") + .tags(tags) + .sign_with_keys(actor_keys) + .unwrap() + } + + // ── tests ───────────────────────────────────────────────────────────────── + + /// Self-author can always delete their own message (no membership re-gate on delete). + #[tokio::test] + async fn delete_own_message_succeeds() { + let Some(pool) = test_pool().await else { + return; + }; + let Some(state) = test_state(pool.clone()).await else { + return; + }; + let tenant = seed_community(&pool).await; + let community_uuid = *tenant.community().as_uuid(); + + let actor_keys = Keys::generate(); + let actor_bytes = actor_keys.public_key().to_bytes().to_vec(); + insert_user(&pool, community_uuid, &actor_bytes).await; + + let ch_id = insert_private_channel(&pool, community_uuid).await; + add_member(&pool, community_uuid, ch_id, &actor_bytes).await; + + // Insert the target event authored by actor. + let target_event = EventBuilder::new(Kind::Custom(9), "hello") + .tags([Tag::parse(["h", &ch_id.to_string()]).unwrap()]) + .sign_with_keys(&actor_keys) + .unwrap(); + let target_id_hex = target_event.id.to_hex(); + state + .db + .insert_event(tenant.community(), &target_event, Some(ch_id)) + .await + .expect("insert target"); + + let delete = delete_event_e_tag(&actor_keys, &target_id_hex); + validate_standard_deletion_event(&tenant, &delete, &state) + .await + .expect("self-author delete must succeed"); + } + + /// Owning human can delete their agent's message when they are a channel member. + #[tokio::test] + async fn delete_owner_agent_message_as_member_succeeds() { + let Some(pool) = test_pool().await else { + return; + }; + let Some(state) = test_state(pool.clone()).await else { + return; + }; + let tenant = seed_community(&pool).await; + let community_uuid = *tenant.community().as_uuid(); + + let owner_keys = Keys::generate(); + let agent_keys = Keys::generate(); + let owner_bytes = owner_keys.public_key().to_bytes().to_vec(); + let agent_bytes = agent_keys.public_key().to_bytes().to_vec(); + + insert_user(&pool, community_uuid, &owner_bytes).await; + insert_agent_with_owner(&pool, community_uuid, &agent_bytes, &owner_bytes).await; + + let ch_id = insert_private_channel(&pool, community_uuid).await; + add_member(&pool, community_uuid, ch_id, &owner_bytes).await; + + // Target event is authored by the agent. + let target_event = EventBuilder::new(Kind::Custom(9), "agent says hi") + .tags([Tag::parse(["h", &ch_id.to_string()]).unwrap()]) + .sign_with_keys(&agent_keys) + .unwrap(); + let target_id_hex = target_event.id.to_hex(); + state + .db + .insert_event(tenant.community(), &target_event, Some(ch_id)) + .await + .expect("insert agent target"); + + // Delete is signed by the owner, not the agent. + let delete = delete_event_e_tag(&owner_keys, &target_id_hex); + validate_standard_deletion_event(&tenant, &delete, &state) + .await + .expect("owner delete of agent message must succeed"); + } + + /// Non-owner, non-author cannot delete another agent's message. + #[tokio::test] + async fn delete_non_owner_agent_message_is_rejected() { + let Some(pool) = test_pool().await else { + return; + }; + let Some(state) = test_state(pool.clone()).await else { + return; + }; + let tenant = seed_community(&pool).await; + let community_uuid = *tenant.community().as_uuid(); + + let owner_keys = Keys::generate(); + let agent_keys = Keys::generate(); + let attacker_keys = Keys::generate(); + let owner_bytes = owner_keys.public_key().to_bytes().to_vec(); + let agent_bytes = agent_keys.public_key().to_bytes().to_vec(); + let attacker_bytes = attacker_keys.public_key().to_bytes().to_vec(); + + insert_user(&pool, community_uuid, &owner_bytes).await; + insert_agent_with_owner(&pool, community_uuid, &agent_bytes, &owner_bytes).await; + insert_user(&pool, community_uuid, &attacker_bytes).await; + + let ch_id = insert_private_channel(&pool, community_uuid).await; + add_member(&pool, community_uuid, ch_id, &attacker_bytes).await; + + let target_event = EventBuilder::new(Kind::Custom(9), "agent says hi") + .tags([Tag::parse(["h", &ch_id.to_string()]).unwrap()]) + .sign_with_keys(&agent_keys) + .unwrap(); + let target_id_hex = target_event.id.to_hex(); + state + .db + .insert_event(tenant.community(), &target_event, Some(ch_id)) + .await + .expect("insert agent target"); + + // Attacker (not the owner) tries to delete the agent's message. + let delete = delete_event_e_tag(&attacker_keys, &target_id_hex); + let err = validate_standard_deletion_event(&tenant, &delete, &state) + .await + .expect_err("non-owner must be rejected"); + assert!( + err.to_string().contains("must be event author"), + "expected 'must be event author', got: {err}" + ); + } + + /// Owner who has been removed from a private channel cannot delete their agent's messages. + #[tokio::test] + async fn delete_owner_agent_message_removed_from_private_channel_is_rejected() { + let Some(pool) = test_pool().await else { + return; + }; + let Some(state) = test_state(pool.clone()).await else { + return; + }; + let tenant = seed_community(&pool).await; + let community_uuid = *tenant.community().as_uuid(); + + let owner_keys = Keys::generate(); + let agent_keys = Keys::generate(); + let owner_bytes = owner_keys.public_key().to_bytes().to_vec(); + let agent_bytes = agent_keys.public_key().to_bytes().to_vec(); + + insert_user(&pool, community_uuid, &owner_bytes).await; + insert_agent_with_owner(&pool, community_uuid, &agent_bytes, &owner_bytes).await; + // Owner is intentionally NOT added as a channel member. + + let ch_id = insert_private_channel(&pool, community_uuid).await; + + let target_event = EventBuilder::new(Kind::Custom(9), "agent says hi") + .tags([Tag::parse(["h", &ch_id.to_string()]).unwrap()]) + .sign_with_keys(&agent_keys) + .unwrap(); + let target_id_hex = target_event.id.to_hex(); + state + .db + .insert_event(tenant.community(), &target_event, Some(ch_id)) + .await + .expect("insert agent target"); + + let delete = delete_event_e_tag(&owner_keys, &target_id_hex); + let err = validate_standard_deletion_event(&tenant, &delete, &state) + .await + .expect_err("removed owner must be rejected"); + assert!( + err.to_string().contains("restricted: not a channel member"), + "expected 'restricted: not a channel member', got: {err}" + ); + } + + /// Owner CAN delete their agent's message in an open channel even without being a member. + #[tokio::test] + async fn delete_owner_agent_message_in_open_channel_without_membership_succeeds() { + let Some(pool) = test_pool().await else { + return; + }; + let Some(state) = test_state(pool.clone()).await else { + return; + }; + let tenant = seed_community(&pool).await; + let community_uuid = *tenant.community().as_uuid(); + + let owner_keys = Keys::generate(); + let agent_keys = Keys::generate(); + let owner_bytes = owner_keys.public_key().to_bytes().to_vec(); + let agent_bytes = agent_keys.public_key().to_bytes().to_vec(); + + insert_user(&pool, community_uuid, &owner_bytes).await; + insert_agent_with_owner(&pool, community_uuid, &agent_bytes, &owner_bytes).await; + // Owner has no membership in the open channel. + + let ch_id = insert_open_channel(&pool, community_uuid).await; + + let target_event = EventBuilder::new(Kind::Custom(9), "agent says hi") + .tags([Tag::parse(["h", &ch_id.to_string()]).unwrap()]) + .sign_with_keys(&agent_keys) + .unwrap(); + let target_id_hex = target_event.id.to_hex(); + state + .db + .insert_event(tenant.community(), &target_event, Some(ch_id)) + .await + .expect("insert agent target"); + + let delete = delete_event_e_tag(&owner_keys, &target_id_hex); + validate_standard_deletion_event(&tenant, &delete, &state) + .await + .expect("owner delete in open channel without membership must succeed"); + } +} diff --git a/desktop/src/features/messages/hooks.ts b/desktop/src/features/messages/hooks.ts index 2209f24fab..ef51272381 100644 --- a/desktop/src/features/messages/hooks.ts +++ b/desktop/src/features/messages/hooks.ts @@ -1,5 +1,6 @@ import { useEffect, useEffectEvent } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { toast } from "sonner"; import { channelMessagesKey, @@ -686,6 +687,9 @@ export function useDeleteMessageMutation(channel: Channel | null) { (current = []) => current.filter((message) => message.id !== eventId), ); }, + onError: (error) => { + toast.error(error.message || "Failed to delete message."); + }, }); }