mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
Allow bot owners to remove their agents from any channel (#284)
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -405,11 +405,14 @@ pub async fn add_member(
|
||||
|
||||
/// Remove a member from a channel (soft delete).
|
||||
///
|
||||
/// `actor_pubkey` must be an active owner/admin, or the member removing themselves.
|
||||
/// `actor_pubkey` must be an active owner/admin, the agent's owner, or the member
|
||||
/// removing themselves.
|
||||
///
|
||||
/// Returns `Err(DbError::MemberNotFound)` if the target is not an active member.
|
||||
/// The authorization check and the UPDATE run inside a transaction to prevent a
|
||||
/// The actor's role check and the UPDATE run inside a transaction to prevent a
|
||||
/// TOCTOU race where the actor's role changes between the check and the update.
|
||||
/// The `is_agent_owner` check runs outside the transaction against the main pool
|
||||
/// because `agent_owner_pubkey` is immutable (set once at token mint).
|
||||
pub async fn remove_member(
|
||||
pool: &PgPool,
|
||||
channel_id: Uuid,
|
||||
@@ -426,9 +429,13 @@ pub async fn remove_member(
|
||||
let actor_role: MemberRole = actor_role_str.parse().map_err(|_| {
|
||||
DbError::InvalidData(format!("invalid role in database: {actor_role_str}"))
|
||||
})?;
|
||||
if !actor_role.is_elevated() {
|
||||
// Safe to query outside the transaction: agent_owner_pubkey is immutable
|
||||
// (set once at token mint, first-mint-wins).
|
||||
if !actor_role.is_elevated()
|
||||
&& !crate::user::is_agent_owner(pool, pubkey, actor_pubkey).await?
|
||||
{
|
||||
return Err(DbError::AccessDenied(
|
||||
"only owners/admins may remove other members".to_string(),
|
||||
"only owners/admins or the agent's owner may remove other members".to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
@@ -1147,3 +1154,126 @@ pub async fn reap_expired_ephemeral_channels(pool: &PgPool) -> Result<Vec<Uuid>>
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::user::{ensure_user, set_agent_owner};
|
||||
use nostr::Keys;
|
||||
|
||||
const TEST_DB_URL: &str = "postgres://sprout:sprout_dev@localhost:5432/sprout";
|
||||
|
||||
async fn setup_pool() -> PgPool {
|
||||
PgPool::connect(TEST_DB_URL)
|
||||
.await
|
||||
.expect("connect to test DB")
|
||||
}
|
||||
|
||||
fn random_pubkey() -> Vec<u8> {
|
||||
Keys::generate().public_key().serialize().to_vec()
|
||||
}
|
||||
|
||||
/// Agent owner (non-admin) can remove their own bot from a channel.
|
||||
#[tokio::test]
|
||||
#[ignore = "requires Postgres"]
|
||||
async fn test_agent_owner_can_remove_bot() {
|
||||
let pool = setup_pool().await;
|
||||
let owner_pk = random_pubkey();
|
||||
let agent_pk = random_pubkey();
|
||||
|
||||
// Create users and set agent ownership
|
||||
ensure_user(&pool, &owner_pk).await.expect("ensure owner");
|
||||
ensure_user(&pool, &agent_pk).await.expect("ensure agent");
|
||||
set_agent_owner(&pool, &agent_pk, &owner_pk)
|
||||
.await
|
||||
.expect("set agent owner");
|
||||
|
||||
// Create a channel owned by someone else entirely
|
||||
let channel_owner_pk = random_pubkey();
|
||||
ensure_user(&pool, &channel_owner_pk)
|
||||
.await
|
||||
.expect("ensure channel owner");
|
||||
let channel = create_channel(
|
||||
&pool,
|
||||
"test-bot-remove",
|
||||
ChannelType::Stream,
|
||||
ChannelVisibility::Open,
|
||||
None,
|
||||
&channel_owner_pk,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("create channel");
|
||||
|
||||
// Add owner and agent as regular members
|
||||
add_member(&pool, channel.id, &owner_pk, MemberRole::Member, None)
|
||||
.await
|
||||
.expect("add owner as member");
|
||||
add_member(&pool, channel.id, &agent_pk, MemberRole::Member, None)
|
||||
.await
|
||||
.expect("add agent as member");
|
||||
|
||||
// Owner should be able to remove their agent
|
||||
remove_member(&pool, channel.id, &agent_pk, &owner_pk)
|
||||
.await
|
||||
.expect("agent owner should be able to remove their bot");
|
||||
|
||||
// Verify the agent is no longer a member
|
||||
assert!(
|
||||
!is_member(&pool, channel.id, &agent_pk)
|
||||
.await
|
||||
.expect("is_member check"),
|
||||
"agent should no longer be a member"
|
||||
);
|
||||
}
|
||||
|
||||
/// A random non-admin, non-owner user cannot remove someone else's bot.
|
||||
#[tokio::test]
|
||||
#[ignore = "requires Postgres"]
|
||||
async fn test_random_user_cannot_remove_bot() {
|
||||
let pool = setup_pool().await;
|
||||
let owner_pk = random_pubkey();
|
||||
let agent_pk = random_pubkey();
|
||||
let random_pk = random_pubkey();
|
||||
|
||||
// Create users and set agent ownership
|
||||
ensure_user(&pool, &owner_pk).await.expect("ensure owner");
|
||||
ensure_user(&pool, &agent_pk).await.expect("ensure agent");
|
||||
ensure_user(&pool, &random_pk).await.expect("ensure random");
|
||||
set_agent_owner(&pool, &agent_pk, &owner_pk)
|
||||
.await
|
||||
.expect("set agent owner");
|
||||
|
||||
// Create a channel
|
||||
let channel_owner_pk = random_pubkey();
|
||||
ensure_user(&pool, &channel_owner_pk)
|
||||
.await
|
||||
.expect("ensure channel owner");
|
||||
let channel = create_channel(
|
||||
&pool,
|
||||
"test-bot-no-remove",
|
||||
ChannelType::Stream,
|
||||
ChannelVisibility::Open,
|
||||
None,
|
||||
&channel_owner_pk,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("create channel");
|
||||
|
||||
// Add random user and agent as regular members
|
||||
add_member(&pool, channel.id, &random_pk, MemberRole::Member, None)
|
||||
.await
|
||||
.expect("add random as member");
|
||||
add_member(&pool, channel.id, &agent_pk, MemberRole::Member, None)
|
||||
.await
|
||||
.expect("add agent as member");
|
||||
|
||||
// Random user should NOT be able to remove the agent
|
||||
let result = remove_member(&pool, channel.id, &agent_pk, &random_pk).await;
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"random user should not be able to remove someone else's bot"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -557,6 +557,11 @@ impl Db {
|
||||
user::get_agent_channel_policy(&self.pool, pubkey).await
|
||||
}
|
||||
|
||||
/// Check whether `actor_pubkey` is the agent owner of `target_pubkey`.
|
||||
pub async fn is_agent_owner(&self, target_pubkey: &[u8], actor_pubkey: &[u8]) -> Result<bool> {
|
||||
user::is_agent_owner(&self.pool, target_pubkey, actor_pubkey).await
|
||||
}
|
||||
|
||||
/// Set the channel_add_policy for a user.
|
||||
pub async fn set_channel_add_policy(&self, pubkey: &[u8], policy: &str) -> Result<()> {
|
||||
user::set_channel_add_policy(&self.pool, pubkey, policy).await
|
||||
|
||||
@@ -324,6 +324,24 @@ pub async fn get_agent_channel_policy(
|
||||
.transpose()
|
||||
}
|
||||
|
||||
/// Check whether `actor_pubkey` is the `agent_owner_pubkey` of `target_pubkey`.
|
||||
/// Queries `agent_owner_pubkey` directly rather than going through
|
||||
/// `get_agent_channel_policy`, which would fetch unrelated fields.
|
||||
pub async fn is_agent_owner(
|
||||
pool: &PgPool,
|
||||
target_pubkey: &[u8],
|
||||
actor_pubkey: &[u8],
|
||||
) -> Result<bool> {
|
||||
let row = sqlx::query_scalar::<_, bool>(
|
||||
"SELECT agent_owner_pubkey = $2 FROM users WHERE pubkey = $1 AND agent_owner_pubkey IS NOT NULL",
|
||||
)
|
||||
.bind(target_pubkey)
|
||||
.bind(actor_pubkey)
|
||||
.fetch_optional(pool)
|
||||
.await?;
|
||||
Ok(row.unwrap_or(false))
|
||||
}
|
||||
|
||||
/// Set the channel_add_policy for a user.
|
||||
/// Returns an error if the pubkey is not found (rows_affected == 0).
|
||||
/// Returns an error if `policy` is not one of the valid ENUM values.
|
||||
|
||||
@@ -205,6 +205,20 @@ pub async fn validate_admin_event(
|
||||
let actor_member = members.iter().find(|m| m.pubkey == actor_bytes);
|
||||
match actor_member {
|
||||
Some(m) if m.role == "owner" || m.role == "admin" => Ok(()),
|
||||
Some(_) => {
|
||||
if state
|
||||
.db
|
||||
.is_agent_owner(&target_pubkey, &actor_bytes)
|
||||
.await?
|
||||
{
|
||||
Ok(())
|
||||
} else {
|
||||
Err(anyhow::anyhow!("actor not authorized"))
|
||||
}
|
||||
}
|
||||
// Non-members fall here. We intentionally do NOT check
|
||||
// is_agent_owner for non-members — you must be in the channel
|
||||
// to remove anyone, even your own bot.
|
||||
_ => Err(anyhow::anyhow!("actor not authorized")),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -40,6 +40,13 @@ export function useClassifiedMembers(
|
||||
[managedAgentPubkeys, relayAgentPubkeys],
|
||||
);
|
||||
|
||||
const isMyBot = React.useCallback(
|
||||
(member: ChannelMember) => {
|
||||
return managedAgentPubkeys.has(normalizePubkey(member.pubkey));
|
||||
},
|
||||
[managedAgentPubkeys],
|
||||
);
|
||||
|
||||
const { people, bots } = React.useMemo(() => {
|
||||
const peopleList: ChannelMember[] = [];
|
||||
const botList: ChannelMember[] = [];
|
||||
@@ -66,6 +73,7 @@ export function useClassifiedMembers(
|
||||
peopleCount: people.length,
|
||||
botCount: bots.length,
|
||||
isBot,
|
||||
isMyBot,
|
||||
managedAgentsQuery,
|
||||
relayAgentsQuery,
|
||||
};
|
||||
|
||||
@@ -50,7 +50,7 @@ export function MembersSidebar({
|
||||
const removeMemberMutation = useRemoveChannelMemberMutation(channelId);
|
||||
|
||||
const rawMembers = membersQuery.data ?? [];
|
||||
const { people, bots, isBot } = useClassifiedMembers(
|
||||
const { people, bots, isBot, isMyBot } = useClassifiedMembers(
|
||||
rawMembers,
|
||||
currentPubkey,
|
||||
);
|
||||
@@ -78,9 +78,11 @@ export function MembersSidebar({
|
||||
}
|
||||
|
||||
function renderMemberCard(member: ChannelMember, memberIsBot: boolean) {
|
||||
// Any channel member can remove bots they own, regardless of role.
|
||||
const canRemoveMember =
|
||||
(selfMember?.role === "admin" && member.pubkey !== currentPubkey) ||
|
||||
(selfMember?.role === "owner" && isBot(member)) ||
|
||||
(selfMember && isMyBot(member)) ||
|
||||
(currentPubkey && member.pubkey === currentPubkey);
|
||||
const memberLabel = formatMemberName(member, currentPubkey);
|
||||
const profile =
|
||||
|
||||
Reference in New Issue
Block a user