mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
Merge remote-tracking branch 'origin/main' into kennylopez-agent-harness-toggles
Signed-off-by: kenny lopez <klopez4212@gmail.com>
This commit is contained in:
+802
-10
@@ -332,6 +332,40 @@ pub async fn set_canvas(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Namespace for the per-channel membership advisory lock. Serializes the
|
||||
/// role-authorization + last-owner-count + write sequences in [`add_member`]
|
||||
/// and [`remove_member`] against each other.
|
||||
///
|
||||
/// Both functions read an owner COUNT and then write a *different* row than the
|
||||
/// one they counted, so `READ COMMITTED` snapshot isolation alone permits two
|
||||
/// concurrent demotions (or a demotion racing a removal) to each observe two
|
||||
/// owners, each pass, and together leave zero — the exact governance loss the
|
||||
/// guards exist to prevent. An advisory key rather than `SELECT ... FOR UPDATE`
|
||||
/// on the channel row: membership is its own contention domain and must not
|
||||
/// serialize against unrelated channel metadata writers (`update_channel`,
|
||||
/// `set_topic`, the TTL transition). Distinct key domain from
|
||||
/// `buzz_channel_ttl:`.
|
||||
const CHANNEL_MEMBERSHIP_LOCK_NAMESPACE: &str = "buzz_channel_membership:";
|
||||
|
||||
/// Take the per-channel membership lock. MUST be the first statement in the
|
||||
/// transaction that then reads roles/owner counts and writes membership, so the
|
||||
/// whole check-then-write sequence is atomic against a concurrent one.
|
||||
async fn acquire_channel_membership_lock(
|
||||
tx: &mut Transaction<'_, Postgres>,
|
||||
community_id: CommunityId,
|
||||
channel_id: Uuid,
|
||||
) -> Result<()> {
|
||||
sqlx::query("SELECT pg_advisory_xact_lock(hashtextextended($1, 0))")
|
||||
.bind(format!(
|
||||
"{CHANNEL_MEMBERSHIP_LOCK_NAMESPACE}{}:{}",
|
||||
community_id.as_uuid(),
|
||||
channel_id
|
||||
))
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Add a member to a channel.
|
||||
///
|
||||
/// Role enforcement:
|
||||
@@ -360,6 +394,10 @@ pub async fn add_member(
|
||||
|
||||
let mut tx = pool.begin().await?;
|
||||
|
||||
// First statement: serialize the whole role-check / owner-count / upsert
|
||||
// sequence against concurrent membership writes on this channel.
|
||||
acquire_channel_membership_lock(&mut tx, community_id, channel_id).await?;
|
||||
|
||||
let channel = get_channel_tx(&mut tx, community_id, channel_id).await?;
|
||||
|
||||
let effective_role = if channel.visibility == "private" {
|
||||
@@ -411,6 +449,56 @@ pub async fn add_member(
|
||||
}
|
||||
};
|
||||
|
||||
// Changing an *active* member's role is privileged in BOTH directions.
|
||||
// Demotion is as consequential as promotion: only owners/admins may grant
|
||||
// elevated roles, so a demoted owner cannot restore themselves. Guarding
|
||||
// only `role.is_elevated()` above therefore left owner→member demotion
|
||||
// unauthorized-by-anyone. Re-adding an active member with the role they
|
||||
// already hold stays idempotent and unguarded — the huddle bot-add and
|
||||
// kind:9021 join paths rely on that.
|
||||
//
|
||||
// Deliberately keyed on the *active* role. A soft-removed row's stored role
|
||||
// is history, not live authority: `removed_at` says it is no longer in
|
||||
// force. Reactivation therefore lands at whatever `effective_role` the
|
||||
// checks above already authorized — `Member` for any unprivileged caller,
|
||||
// elevated only when a currently-elevated granter asked for it. Inferring
|
||||
// current authority from a removed row would make soft-deleted ownership a
|
||||
// resurrection token: an owner removed by another owner could self-rejoin
|
||||
// via kind:9021 (`Member, None`) and silently regain ownership.
|
||||
let current_role = get_active_role_tx(&mut tx, community_id, channel_id, pubkey).await?;
|
||||
if let Some(current_role) = current_role.filter(|r| r != effective_role.as_str()) {
|
||||
let actor_role = match invited_by {
|
||||
Some(inviter) => get_active_role_tx(&mut tx, community_id, channel_id, inviter).await?,
|
||||
None => None,
|
||||
};
|
||||
let actor_role: Option<MemberRole> = actor_role.and_then(|r| r.parse().ok());
|
||||
if !actor_role.is_some_and(|r| r.is_elevated()) {
|
||||
return Err(DbError::AccessDenied(
|
||||
"only owners/admins may change an active member's role".to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
// Defense-in-depth, mirroring `remove_member`: a demotion must not
|
||||
// strip the channel of its last owner, which would leave nobody able
|
||||
// to moderate, edit metadata, or re-grant ownership.
|
||||
if current_role == "owner" && effective_role != MemberRole::Owner {
|
||||
let row = sqlx::query(
|
||||
"SELECT COUNT(*) as cnt FROM channel_members \
|
||||
WHERE community_id = $1 AND channel_id = $2 AND role = 'owner' AND removed_at IS NULL",
|
||||
)
|
||||
.bind(community_id.as_uuid())
|
||||
.bind(channel_id)
|
||||
.fetch_one(&mut *tx)
|
||||
.await?;
|
||||
let owner_count: i64 = row.try_get("cnt")?;
|
||||
if owner_count <= 1 {
|
||||
return Err(DbError::AccessDenied(
|
||||
"cannot demote the last owner — transfer ownership first".to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
sqlx::query(
|
||||
r#"
|
||||
INSERT INTO channel_members (community_id, channel_id, pubkey, role, invited_by)
|
||||
@@ -452,10 +540,19 @@ pub async fn add_member(
|
||||
/// removing themselves.
|
||||
///
|
||||
/// Returns `Err(DbError::MemberNotFound)` if the target is not an active member.
|
||||
/// 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).
|
||||
///
|
||||
/// The per-channel membership lock is the transaction's first statement, so the
|
||||
/// actor's role check, the last-owner count, and the UPDATE are all serialized
|
||||
/// against concurrent membership writes — otherwise a concurrent demotion of the
|
||||
/// actor could commit after their role was read and this removal would proceed on
|
||||
/// a stale elevated role.
|
||||
///
|
||||
/// The `is_agent_owner` lookup deliberately runs *before* the transaction opens:
|
||||
/// it borrows a second connection from `pool`, and issuing it while holding the
|
||||
/// lock could deadlock against ourselves on a small pool. That is safe because
|
||||
/// `agent_owner_pubkey` is immutable — [`crate::user::set_agent_owner`] only
|
||||
/// updates it when it `IS NULL` (first-mint-wins), so its value cannot change
|
||||
/// under us and needs no serialization.
|
||||
pub async fn remove_member(
|
||||
pool: &PgPool,
|
||||
community_id: CommunityId,
|
||||
@@ -463,9 +560,24 @@ pub async fn remove_member(
|
||||
pubkey: &[u8],
|
||||
actor_pubkey: &[u8],
|
||||
) -> Result<()> {
|
||||
let is_self_remove = pubkey == actor_pubkey;
|
||||
|
||||
// Immutable, and must not be queried while holding the lock (second pool
|
||||
// connection). Resolved up front so every *mutable* authorization read can
|
||||
// sit behind the serialization point below.
|
||||
let actor_is_agent_owner = if is_self_remove {
|
||||
false
|
||||
} else {
|
||||
crate::user::is_agent_owner(pool, community_id, pubkey, actor_pubkey).await?
|
||||
};
|
||||
|
||||
let mut tx = pool.begin().await?;
|
||||
|
||||
let is_self_remove = pubkey == actor_pubkey;
|
||||
// First statement: serialize the actor-role check, the last-owner count and
|
||||
// the UPDATE against concurrent membership writes on this channel (same key
|
||||
// as `add_member`).
|
||||
acquire_channel_membership_lock(&mut tx, community_id, channel_id).await?;
|
||||
|
||||
if !is_self_remove {
|
||||
let actor_role_str = get_active_role_tx(&mut tx, community_id, channel_id, actor_pubkey)
|
||||
.await?
|
||||
@@ -473,11 +585,7 @@ 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}"))
|
||||
})?;
|
||||
// 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, community_id, pubkey, actor_pubkey).await?
|
||||
{
|
||||
if !actor_role.is_elevated() && !actor_is_agent_owner {
|
||||
return Err(DbError::AccessDenied(
|
||||
"only owners/admins or the agent's owner may remove other members".to_string(),
|
||||
));
|
||||
@@ -1892,4 +2000,688 @@ mod tests {
|
||||
"random user should not be able to remove someone else's bot"
|
||||
);
|
||||
}
|
||||
|
||||
/// SECURITY REPRO (Dawn, kind:9000 demotion report): an unprivileged plain
|
||||
/// member calls add_member with role=Member against the channel OWNER.
|
||||
/// If this succeeds, add_member has no demotion authorization and no
|
||||
/// last-owner guard.
|
||||
#[tokio::test]
|
||||
#[ignore = "requires Postgres"]
|
||||
async fn repro_unprivileged_member_can_demote_owner() {
|
||||
let pool = setup_pool().await;
|
||||
let community_id = make_test_community(&pool).await;
|
||||
let community = CommunityId::from_uuid(community_id);
|
||||
let victim_owner = random_pubkey();
|
||||
let attacker = random_pubkey();
|
||||
|
||||
for pk in [&victim_owner, &attacker] {
|
||||
ensure_user(&pool, community, pk)
|
||||
.await
|
||||
.expect("ensure user");
|
||||
}
|
||||
|
||||
let channel = create_test_channel(
|
||||
&pool,
|
||||
community_id,
|
||||
"repro-demote-owner",
|
||||
ChannelType::Stream,
|
||||
ChannelVisibility::Open,
|
||||
None,
|
||||
&victim_owner,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("create channel");
|
||||
|
||||
// create_test_channel already seeds the creator as 'owner', mirroring
|
||||
// create_channel's own INSERT (channel.rs:131-145).
|
||||
let role_of = |members: Vec<MemberRecord>, pk: Vec<u8>| -> Option<String> {
|
||||
members.into_iter().find(|m| m.pubkey == pk).map(|m| m.role)
|
||||
};
|
||||
let before = role_of(
|
||||
get_members(&pool, community, channel.id)
|
||||
.await
|
||||
.expect("members"),
|
||||
victim_owner.clone(),
|
||||
);
|
||||
assert_eq!(
|
||||
before.as_deref(),
|
||||
Some("owner"),
|
||||
"victim must start as owner"
|
||||
);
|
||||
|
||||
// Attacker: plain member, not owner/admin.
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel.id,
|
||||
&attacker,
|
||||
MemberRole::Member,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("attacker self-joins open channel");
|
||||
|
||||
// The attack: attacker is `invited_by` and demotes the owner.
|
||||
let res = add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel.id,
|
||||
&victim_owner,
|
||||
MemberRole::Member,
|
||||
Some(&attacker),
|
||||
)
|
||||
.await;
|
||||
|
||||
let after = role_of(
|
||||
get_members(&pool, community, channel.id)
|
||||
.await
|
||||
.expect("members"),
|
||||
victim_owner.clone(),
|
||||
);
|
||||
let owners = get_members(&pool, community, channel.id)
|
||||
.await
|
||||
.expect("members")
|
||||
.into_iter()
|
||||
.filter(|m| m.role == "owner")
|
||||
.count();
|
||||
|
||||
assert!(
|
||||
res.is_err(),
|
||||
"unprivileged member must not be able to demote the owner"
|
||||
);
|
||||
assert_eq!(after.as_deref(), Some("owner"), "owner role must survive");
|
||||
assert_eq!(owners, 1, "channel must still have its owner");
|
||||
}
|
||||
|
||||
/// SECURITY REPRO (Dawn): same demotion on a PRIVATE channel, where the
|
||||
/// attacker is a plain member. The report claims any member suffices here.
|
||||
#[tokio::test]
|
||||
#[ignore = "requires Postgres"]
|
||||
async fn repro_private_channel_member_can_demote_owner() {
|
||||
let pool = setup_pool().await;
|
||||
let community_id = make_test_community(&pool).await;
|
||||
let community = CommunityId::from_uuid(community_id);
|
||||
let victim_owner = random_pubkey();
|
||||
let attacker = random_pubkey();
|
||||
|
||||
for pk in [&victim_owner, &attacker] {
|
||||
ensure_user(&pool, community, pk)
|
||||
.await
|
||||
.expect("ensure user");
|
||||
}
|
||||
|
||||
let channel = create_test_channel(
|
||||
&pool,
|
||||
community_id,
|
||||
"repro-demote-owner-private",
|
||||
ChannelType::Stream,
|
||||
ChannelVisibility::Private,
|
||||
None,
|
||||
&victim_owner,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("create private channel");
|
||||
|
||||
// Owner invites the attacker as a plain member (legitimate).
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel.id,
|
||||
&attacker,
|
||||
MemberRole::Member,
|
||||
Some(&victim_owner),
|
||||
)
|
||||
.await
|
||||
.expect("owner invites attacker");
|
||||
|
||||
// Attack: plain member demotes the owner.
|
||||
let res = add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel.id,
|
||||
&victim_owner,
|
||||
MemberRole::Member,
|
||||
Some(&attacker),
|
||||
)
|
||||
.await;
|
||||
|
||||
let members = get_members(&pool, community, channel.id)
|
||||
.await
|
||||
.expect("members");
|
||||
let victim_role = members
|
||||
.iter()
|
||||
.find(|m| m.pubkey == victim_owner)
|
||||
.map(|m| m.role.clone());
|
||||
let owners = members.iter().filter(|m| m.role == "owner").count();
|
||||
|
||||
assert!(
|
||||
res.is_err(),
|
||||
"plain member must not be able to demote the owner on a private channel"
|
||||
);
|
||||
assert_eq!(
|
||||
victim_role.as_deref(),
|
||||
Some("owner"),
|
||||
"owner role must survive"
|
||||
);
|
||||
assert_eq!(owners, 1, "channel must still have its owner");
|
||||
}
|
||||
|
||||
/// The fix must not break legitimate role management: an owner demoting a
|
||||
/// co-owner (while another owner remains) must still succeed, and promotion
|
||||
/// by an owner must still succeed.
|
||||
#[tokio::test]
|
||||
#[ignore = "requires Postgres"]
|
||||
async fn owner_can_still_manage_roles_after_demotion_guard() {
|
||||
let pool = setup_pool().await;
|
||||
let community_id = make_test_community(&pool).await;
|
||||
let community = CommunityId::from_uuid(community_id);
|
||||
let owner = random_pubkey();
|
||||
let other = random_pubkey();
|
||||
|
||||
for pk in [&owner, &other] {
|
||||
ensure_user(&pool, community, pk)
|
||||
.await
|
||||
.expect("ensure user");
|
||||
}
|
||||
|
||||
let channel = create_test_channel(
|
||||
&pool,
|
||||
community_id,
|
||||
"roles-still-manageable",
|
||||
ChannelType::Stream,
|
||||
ChannelVisibility::Open,
|
||||
None,
|
||||
&owner,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("create channel");
|
||||
|
||||
// Owner promotes `other` to owner — allowed (actor is elevated).
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel.id,
|
||||
&other,
|
||||
MemberRole::Owner,
|
||||
Some(&owner),
|
||||
)
|
||||
.await
|
||||
.expect("owner may promote to owner");
|
||||
|
||||
// Owner demotes the co-owner back to member — allowed: actor is elevated
|
||||
// and another owner remains, so the last-owner guard does not trip.
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel.id,
|
||||
&other,
|
||||
MemberRole::Member,
|
||||
Some(&owner),
|
||||
)
|
||||
.await
|
||||
.expect("owner may demote a co-owner while another owner remains");
|
||||
|
||||
let members = get_members(&pool, community, channel.id)
|
||||
.await
|
||||
.expect("members");
|
||||
let role_of = |pk: &Vec<u8>| {
|
||||
members
|
||||
.iter()
|
||||
.find(|m| &m.pubkey == pk)
|
||||
.map(|m| m.role.clone())
|
||||
};
|
||||
assert_eq!(role_of(&other).as_deref(), Some("member"));
|
||||
assert_eq!(role_of(&owner).as_deref(), Some("owner"));
|
||||
|
||||
// Idempotent re-add at the SAME role must stay unguarded even from a
|
||||
// non-elevated actor — the huddle bot-add path depends on this.
|
||||
let bot = random_pubkey();
|
||||
ensure_user(&pool, community, &bot)
|
||||
.await
|
||||
.expect("ensure bot");
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel.id,
|
||||
&bot,
|
||||
MemberRole::Bot,
|
||||
Some(&owner),
|
||||
)
|
||||
.await
|
||||
.expect("add bot");
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel.id,
|
||||
&bot,
|
||||
MemberRole::Bot,
|
||||
Some(&other),
|
||||
)
|
||||
.await
|
||||
.expect("re-adding at the same role must remain idempotent");
|
||||
|
||||
// But the last owner cannot be demoted, even by themselves.
|
||||
let err = add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel.id,
|
||||
&owner,
|
||||
MemberRole::Member,
|
||||
Some(&owner),
|
||||
)
|
||||
.await
|
||||
.expect_err("last owner must not be demotable");
|
||||
println!("last-owner demotion rejected: {err}");
|
||||
}
|
||||
|
||||
/// Isolates the actor-authorization guard from the last-owner guard.
|
||||
///
|
||||
/// `repro_unprivileged_member_can_demote_owner` demotes the *sole* owner, so
|
||||
/// the last-owner guard alone is enough to reject it: stubbing out the actor
|
||||
/// check leaves that test green and the authorization hole invisible. Here a
|
||||
/// second owner remains, so the last-owner guard cannot fire and only the
|
||||
/// actor check stands between an unprivileged member and a co-owner's role.
|
||||
#[tokio::test]
|
||||
#[ignore = "requires Postgres"]
|
||||
async fn unprivileged_member_cannot_demote_a_co_owner() {
|
||||
let pool = setup_pool().await;
|
||||
let community_id = make_test_community(&pool).await;
|
||||
let community = CommunityId::from_uuid(community_id);
|
||||
let owner = random_pubkey();
|
||||
let co_owner = random_pubkey();
|
||||
let attacker = random_pubkey();
|
||||
|
||||
for pk in [&owner, &co_owner, &attacker] {
|
||||
ensure_user(&pool, community, pk)
|
||||
.await
|
||||
.expect("ensure user");
|
||||
}
|
||||
|
||||
let channel = create_test_channel(
|
||||
&pool,
|
||||
community_id,
|
||||
"co-owner-demotion-authz",
|
||||
ChannelType::Stream,
|
||||
ChannelVisibility::Open,
|
||||
None,
|
||||
&owner,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("create channel");
|
||||
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel.id,
|
||||
&co_owner,
|
||||
MemberRole::Owner,
|
||||
Some(&owner),
|
||||
)
|
||||
.await
|
||||
.expect("owner may promote a co-owner");
|
||||
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel.id,
|
||||
&attacker,
|
||||
MemberRole::Member,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("attacker self-joins the open channel");
|
||||
|
||||
// Two owners remain, so the last-owner guard cannot reject this. Only
|
||||
// the actor-authorization check can.
|
||||
let err = add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel.id,
|
||||
&co_owner,
|
||||
MemberRole::Member,
|
||||
Some(&attacker),
|
||||
)
|
||||
.await
|
||||
.expect_err("an unprivileged member must not demote a co-owner");
|
||||
println!("co-owner demotion by unprivileged actor rejected: {err}");
|
||||
|
||||
let members = get_members(&pool, community, channel.id)
|
||||
.await
|
||||
.expect("members");
|
||||
let role_of = |pk: &Vec<u8>| {
|
||||
members
|
||||
.iter()
|
||||
.find(|m| &m.pubkey == pk)
|
||||
.map(|m| m.role.clone())
|
||||
};
|
||||
assert_eq!(
|
||||
role_of(&co_owner).as_deref(),
|
||||
Some("owner"),
|
||||
"co-owner must keep their role"
|
||||
);
|
||||
assert_eq!(
|
||||
members.iter().filter(|m| m.role == "owner").count(),
|
||||
2,
|
||||
"both owners must survive"
|
||||
);
|
||||
}
|
||||
|
||||
/// Sets up an open channel with exactly two owners, returning
|
||||
/// `(community, channel_id, owner_a, owner_b)`.
|
||||
async fn channel_with_two_owners(
|
||||
pool: &PgPool,
|
||||
name: &str,
|
||||
) -> (CommunityId, Uuid, Vec<u8>, Vec<u8>) {
|
||||
let community_id = make_test_community(pool).await;
|
||||
let community = CommunityId::from_uuid(community_id);
|
||||
let owner_a = random_pubkey();
|
||||
let owner_b = random_pubkey();
|
||||
for pk in [&owner_a, &owner_b] {
|
||||
ensure_user(pool, community, pk).await.expect("ensure user");
|
||||
}
|
||||
|
||||
let channel = create_test_channel(
|
||||
pool,
|
||||
community_id,
|
||||
name,
|
||||
ChannelType::Stream,
|
||||
ChannelVisibility::Open,
|
||||
None,
|
||||
&owner_a,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("create channel");
|
||||
|
||||
add_member(
|
||||
pool,
|
||||
community,
|
||||
channel.id,
|
||||
&owner_b,
|
||||
MemberRole::Owner,
|
||||
Some(&owner_a),
|
||||
)
|
||||
.await
|
||||
.expect("promote second owner");
|
||||
|
||||
(community, channel.id, owner_a, owner_b)
|
||||
}
|
||||
|
||||
/// The lock must be shared with `remove_member`: a demotion racing an owner
|
||||
/// removal goes through a separate count/update path, so both must serialize
|
||||
/// on the same key or they can jointly empty the owner set.
|
||||
///
|
||||
/// Deterministic rather than timing-based: an outer transaction takes the
|
||||
/// per-channel membership key first, then each membership writer must block
|
||||
/// until it is released. Verified by mutation — dropping the lock from either
|
||||
/// function makes that call return immediately and fails this test.
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn membership_writes_serialize_on_the_shared_channel_lock() {
|
||||
let pool = setup_pool().await;
|
||||
let (community, channel_id, owner_a, owner_b) =
|
||||
channel_with_two_owners(&pool, "membership-lock-shared").await;
|
||||
|
||||
for label in ["add_member", "remove_member"] {
|
||||
// Hold the same advisory key an in-tree membership write would take.
|
||||
let mut holder = pool.begin().await.expect("begin lock holder");
|
||||
acquire_channel_membership_lock(&mut holder, community, channel_id)
|
||||
.await
|
||||
.expect("holder acquires membership key");
|
||||
|
||||
let pool2 = pool.clone();
|
||||
let (target, actor) = (owner_a.clone(), owner_b.clone());
|
||||
let mut writer = tokio::spawn(async move {
|
||||
match label {
|
||||
"add_member" => add_member(
|
||||
&pool2,
|
||||
community,
|
||||
channel_id,
|
||||
&target,
|
||||
MemberRole::Member,
|
||||
Some(&actor),
|
||||
)
|
||||
.await
|
||||
.map(|_| ()),
|
||||
_ => remove_member(&pool2, community, channel_id, &target, &actor).await,
|
||||
}
|
||||
});
|
||||
|
||||
// While the key is held, the writer must make no progress.
|
||||
let blocked =
|
||||
tokio::time::timeout(std::time::Duration::from_millis(750), &mut writer).await;
|
||||
assert!(
|
||||
blocked.is_err(),
|
||||
"{label} completed while the channel membership key was held — \
|
||||
it is not serializing on the shared lock"
|
||||
);
|
||||
println!("{label} blocked on the held membership key, as required");
|
||||
|
||||
// Releasing the key lets it proceed.
|
||||
holder.rollback().await.expect("release membership key");
|
||||
tokio::time::timeout(std::time::Duration::from_secs(10), writer)
|
||||
.await
|
||||
.expect("writer must proceed once the key is released")
|
||||
.expect("writer task panicked")
|
||||
.expect("writer must succeed after the key is released");
|
||||
|
||||
// Restore two owners for the next iteration.
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel_id,
|
||||
&owner_a,
|
||||
MemberRole::Owner,
|
||||
Some(&owner_b),
|
||||
)
|
||||
.await
|
||||
.expect("restore second owner");
|
||||
}
|
||||
}
|
||||
|
||||
/// Every *mutable* authorization read must sit behind the membership lock.
|
||||
/// A remover that reads its elevated role before acquiring the lock can be
|
||||
/// demoted by a concurrent writer and still proceed on the stale role.
|
||||
///
|
||||
/// Deterministic: the holder takes the key, `remove_member` blocks on it, the
|
||||
/// holder then demotes the remover and commits. Once the key is released the
|
||||
/// remover must re-read its (now unprivileged) role and be rejected.
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn remove_member_rejects_an_actor_demoted_while_it_waited() {
|
||||
let pool = setup_pool().await;
|
||||
let (community, channel_id, owner_a, owner_b) =
|
||||
channel_with_two_owners(&pool, "stale-actor-role").await;
|
||||
// owner_b removes a plain member, so the last-owner guard is not what
|
||||
// rejects this — only the actor's own role can.
|
||||
let victim = random_pubkey();
|
||||
ensure_user(&pool, community, &victim)
|
||||
.await
|
||||
.expect("ensure victim");
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel_id,
|
||||
&victim,
|
||||
MemberRole::Member,
|
||||
Some(&owner_a),
|
||||
)
|
||||
.await
|
||||
.expect("add victim");
|
||||
|
||||
let mut holder = pool.begin().await.expect("begin lock holder");
|
||||
acquire_channel_membership_lock(&mut holder, community, channel_id)
|
||||
.await
|
||||
.expect("holder acquires membership key");
|
||||
|
||||
let pool2 = pool.clone();
|
||||
let (actor, target) = (owner_b.clone(), victim.clone());
|
||||
let mut remover = tokio::spawn(async move {
|
||||
remove_member(&pool2, community, channel_id, &target, &actor).await
|
||||
});
|
||||
|
||||
// Must be waiting on the key, not already authorized past it.
|
||||
assert!(
|
||||
tokio::time::timeout(std::time::Duration::from_millis(750), &mut remover)
|
||||
.await
|
||||
.is_err(),
|
||||
"remove_member must block on the membership key before authorizing"
|
||||
);
|
||||
|
||||
// Demote the waiting actor to a plain member and release the key.
|
||||
sqlx::query(
|
||||
"UPDATE channel_members SET role = 'member' \
|
||||
WHERE community_id = $1 AND channel_id = $2 AND pubkey = $3",
|
||||
)
|
||||
.bind(community.as_uuid())
|
||||
.bind(channel_id)
|
||||
.bind(&owner_b)
|
||||
.execute(&mut *holder)
|
||||
.await
|
||||
.expect("demote the waiting actor");
|
||||
holder.commit().await.expect("commit demotion");
|
||||
|
||||
let result = tokio::time::timeout(std::time::Duration::from_secs(10), remover)
|
||||
.await
|
||||
.expect("remover must proceed once the key is released")
|
||||
.expect("remover task panicked");
|
||||
|
||||
let err = result.expect_err("a demoted actor must not remove another member");
|
||||
println!("stale-role removal rejected: {err}");
|
||||
|
||||
// The victim must still be an active member.
|
||||
let members = get_members(&pool, community, channel_id)
|
||||
.await
|
||||
.expect("members");
|
||||
assert!(
|
||||
members.iter().any(|m| m.pubkey == victim),
|
||||
"victim must not have been removed by a demoted actor"
|
||||
);
|
||||
}
|
||||
|
||||
/// A soft-removed row keeps its stored `role`, but that role is history,
|
||||
/// not live authority — `removed_at` says it is no longer in force. So
|
||||
/// reactivation must land at the baseline the caller was authorized for,
|
||||
/// never at the role the row happens to remember.
|
||||
///
|
||||
/// Regression for the sharper vulnerability the alternative would create:
|
||||
/// an owner kicked by another owner self-rejoins through the kind:9021
|
||||
/// path (`Member`, no inviter) and must come back as a plain member. If
|
||||
/// `add_member` inferred authority from the removed row, soft-deleted
|
||||
/// ownership would be a resurrection token.
|
||||
///
|
||||
/// Two owners on purpose, so the last-owner guard can never be what
|
||||
/// decides the outcome — only role resolution can.
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn kicked_owner_rejoins_as_member_not_owner() {
|
||||
let pool = setup_pool().await;
|
||||
let (community, channel_id, owner_a, owner_b) =
|
||||
channel_with_two_owners(&pool, "kicked-owner-rejoin").await;
|
||||
|
||||
// owner_a kicks owner_b (allowed: owner_a remains as the last owner).
|
||||
remove_member(&pool, community, channel_id, &owner_b, &owner_a)
|
||||
.await
|
||||
.expect("an owner may remove another owner");
|
||||
|
||||
let stored: String = sqlx::query_scalar(
|
||||
"SELECT role::text FROM channel_members \
|
||||
WHERE community_id = $1 AND channel_id = $2 AND pubkey = $3",
|
||||
)
|
||||
.bind(community.as_uuid())
|
||||
.bind(channel_id)
|
||||
.bind(&owner_b)
|
||||
.fetch_one(&pool)
|
||||
.await
|
||||
.expect("stored role survives soft removal");
|
||||
assert_eq!(
|
||||
stored, "owner",
|
||||
"the removed row still remembers `owner` — which is exactly why \
|
||||
authorization must not read it"
|
||||
);
|
||||
|
||||
// The kind:9021 self-rejoin path: `Member`, no inviter.
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel_id,
|
||||
&owner_b,
|
||||
MemberRole::Member,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("a removed member may rejoin an open channel");
|
||||
|
||||
let rejoined = get_member_role(&pool, community, channel_id, &owner_b)
|
||||
.await
|
||||
.expect("read role after rejoin");
|
||||
assert_eq!(
|
||||
rejoined.as_deref(),
|
||||
Some("member"),
|
||||
"a kicked owner must rejoin at baseline privilege, not regain ownership"
|
||||
);
|
||||
}
|
||||
|
||||
/// The other side of the same boundary: reactivation may reach an elevated
|
||||
/// role, but only because a *currently* elevated granter asked for it.
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn removed_owner_is_restored_only_by_a_current_owner() {
|
||||
let pool = setup_pool().await;
|
||||
let (community, channel_id, owner_a, owner_b) =
|
||||
channel_with_two_owners(&pool, "removed-owner-restore").await;
|
||||
|
||||
remove_member(&pool, community, channel_id, &owner_b, &owner_a)
|
||||
.await
|
||||
.expect("an owner may remove another owner");
|
||||
|
||||
// An unprivileged member cannot re-add them at `owner`.
|
||||
let rando = random_pubkey();
|
||||
ensure_user(&pool, community, &rando)
|
||||
.await
|
||||
.expect("ensure rando");
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel_id,
|
||||
&rando,
|
||||
MemberRole::Member,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("rando self-joins open channel");
|
||||
let denied = add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel_id,
|
||||
&owner_b,
|
||||
MemberRole::Owner,
|
||||
Some(&rando),
|
||||
)
|
||||
.await;
|
||||
assert!(
|
||||
matches!(denied, Err(DbError::AccessDenied(_))),
|
||||
"an unprivileged actor must not re-add anyone at `owner`, got {denied:?}"
|
||||
);
|
||||
|
||||
// The remaining owner can.
|
||||
add_member(
|
||||
&pool,
|
||||
community,
|
||||
channel_id,
|
||||
&owner_b,
|
||||
MemberRole::Owner,
|
||||
Some(&owner_a),
|
||||
)
|
||||
.await
|
||||
.expect("a current owner may restore ownership");
|
||||
let restored = get_member_role(&pool, community, channel_id, &owner_b)
|
||||
.await
|
||||
.expect("read role after restore");
|
||||
assert_eq!(restored.as_deref(), Some("owner"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -34,9 +34,10 @@ use sqlx::{PgPool, QueryBuilder};
|
||||
use uuid::Uuid;
|
||||
|
||||
use buzz_core::kind::{
|
||||
KIND_FORUM_COMMENT, KIND_FORUM_POST, KIND_JOB_PROGRESS, KIND_JOB_REQUEST, KIND_JOB_RESULT,
|
||||
KIND_STREAM_MESSAGE, KIND_STREAM_MESSAGE_V2, KIND_STREAM_REMINDER,
|
||||
KIND_WORKFLOW_APPROVAL_REQUESTED,
|
||||
KIND_FORUM_COMMENT, KIND_FORUM_POST, KIND_GIT_ISSUE, KIND_GIT_PR_UPDATE, KIND_GIT_PULL_REQUEST,
|
||||
KIND_GIT_STATUS_CLOSED, KIND_GIT_STATUS_DRAFT, KIND_GIT_STATUS_MERGED, KIND_GIT_STATUS_OPEN,
|
||||
KIND_JOB_PROGRESS, KIND_JOB_REQUEST, KIND_JOB_RESULT, KIND_STREAM_MESSAGE,
|
||||
KIND_STREAM_MESSAGE_V2, KIND_STREAM_REMINDER, KIND_TEXT_NOTE, KIND_WORKFLOW_APPROVAL_REQUESTED,
|
||||
};
|
||||
use buzz_core::{CommunityId, StoredEvent};
|
||||
|
||||
@@ -103,7 +104,9 @@ fn build_mentions_query(
|
||||
qb.push(" AND e.deleted_at IS NULL");
|
||||
qb.push(format!(
|
||||
" AND e.kind IN ({KIND_STREAM_MESSAGE}, {KIND_STREAM_MESSAGE_V2}, \
|
||||
{KIND_FORUM_POST}, {KIND_FORUM_COMMENT})"
|
||||
{KIND_TEXT_NOTE}, {KIND_FORUM_POST}, {KIND_FORUM_COMMENT}, {KIND_GIT_PULL_REQUEST}, \
|
||||
{KIND_GIT_PR_UPDATE}, {KIND_GIT_ISSUE}, {KIND_GIT_STATUS_OPEN}, \
|
||||
{KIND_GIT_STATUS_MERGED}, {KIND_GIT_STATUS_CLOSED}, {KIND_GIT_STATUS_DRAFT})"
|
||||
));
|
||||
push_visible_channel_filter(&mut qb, "e.channel_id", accessible_channel_ids);
|
||||
if let Some(s) = since {
|
||||
@@ -252,7 +255,7 @@ mod tests {
|
||||
use nostr::{EventBuilder, Keys, Kind, Tag};
|
||||
use uuid::Uuid;
|
||||
|
||||
const TEST_DB_URL: &str = "postgres://buzz:buzz_dev@localhost:5432/buzz";
|
||||
const TEST_DB_URL: &str = "postgres://buzz:buzz_dev@localhost:5432/buzz"; // sadscan:disable np.postgres.1 -- local test-only credentials
|
||||
|
||||
async fn setup_pool() -> PgPool {
|
||||
let database_url = std::env::var("BUZZ_TEST_DATABASE_URL")
|
||||
@@ -777,6 +780,12 @@ mod tests {
|
||||
sql.contains("AND m.community_id = "),
|
||||
"mentions feed must also bind event_mentions.community_id: {sql}"
|
||||
);
|
||||
assert!(
|
||||
sql.contains(&KIND_GIT_PULL_REQUEST.to_string())
|
||||
&& sql.contains(&KIND_GIT_ISSUE.to_string())
|
||||
&& sql.contains(&KIND_TEXT_NOTE.to_string()),
|
||||
"mentions feed must include Buzz Git roots and comments: {sql}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -291,35 +291,39 @@ pub async fn validate_admin_event(
|
||||
|
||||
match kind {
|
||||
9000 => {
|
||||
// Validate role tag if present
|
||||
let role_str = extract_tag_value(event, "role").unwrap_or_else(|| "member".to_string());
|
||||
if role_str.parse::<buzz_db::channel::MemberRole>().is_err() {
|
||||
return Err(anyhow::anyhow!("invalid role: {role_str}"));
|
||||
}
|
||||
// An absent role tag means "no role change requested": for an existing
|
||||
// member that preserves the role they already hold, and only defaults
|
||||
// to Member for a genuinely new member. Defaulting unconditionally to
|
||||
// Member made a bare self-targeted PUT_USER silently demote an owner.
|
||||
let role_str = extract_tag_value(event, "role");
|
||||
let requested_role = match role_str {
|
||||
Some(ref s) => match s.parse::<buzz_db::channel::MemberRole>() {
|
||||
Ok(r) => Some(r),
|
||||
Err(_) => return Err(anyhow::anyhow!("invalid role: {s}")),
|
||||
},
|
||||
None => None,
|
||||
};
|
||||
|
||||
let members = state.db.get_members(tenant.community(), channel_id).await?;
|
||||
let actor_role: Option<buzz_db::channel::MemberRole> = members
|
||||
.iter()
|
||||
.find(|m| m.pubkey == actor_bytes)
|
||||
.and_then(|m| m.role.parse().ok());
|
||||
|
||||
// PUT_USER: open channels allow any authenticated user; private channels
|
||||
// require the actor to be an existing member (any role can invite).
|
||||
if channel.visibility == "private" {
|
||||
let members = state.db.get_members(tenant.community(), channel_id).await?;
|
||||
let actor_member = members.iter().find(|m| m.pubkey == actor_bytes);
|
||||
match actor_member {
|
||||
Some(_) => {}
|
||||
None => return Err(anyhow::anyhow!("actor not authorized")),
|
||||
if actor_role.is_none() {
|
||||
return Err(anyhow::anyhow!("actor not authorized"));
|
||||
}
|
||||
|
||||
// Only owners/admins may grant elevated roles.
|
||||
let role: buzz_db::channel::MemberRole = role_str.parse().unwrap();
|
||||
if role.is_elevated() {
|
||||
let actor_role: buzz_db::channel::MemberRole = actor_member
|
||||
.unwrap()
|
||||
.role
|
||||
.parse()
|
||||
.unwrap_or(buzz_db::channel::MemberRole::Member);
|
||||
if !actor_role.is_elevated() {
|
||||
return Err(anyhow::anyhow!(
|
||||
"only owners/admins may grant elevated roles"
|
||||
));
|
||||
}
|
||||
if requested_role.is_some_and(|r| r.is_elevated())
|
||||
&& !actor_role.is_some_and(|r| r.is_elevated())
|
||||
{
|
||||
return Err(anyhow::anyhow!(
|
||||
"only owners/admins may grant elevated roles"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -327,6 +331,39 @@ pub async fn validate_admin_event(
|
||||
let target_pubkey =
|
||||
extract_p_tag(event).ok_or_else(|| anyhow::anyhow!("missing p tag"))?;
|
||||
|
||||
// Changing an ACTIVE existing member's role is privileged in both
|
||||
// directions, on every visibility. `get_members` filters
|
||||
// `removed_at IS NULL`, so a soft-removed row is deliberately not an
|
||||
// "existing member" here: its stored role is history, not live
|
||||
// authority, and reactivation is governed by the elevated-granter
|
||||
// check above rather than by the role the row remembers.
|
||||
//
|
||||
// `add_member` is the authority (it also covers the desktop/admin
|
||||
// callers that skip this validator); rejecting here too means the
|
||||
// client gets a real error instead of an OK for an event whose side
|
||||
// effect then fails. Re-adding at the same role stays idempotent —
|
||||
// the huddle bot-add path relies on that.
|
||||
if let Some((target, role)) = members
|
||||
.iter()
|
||||
.find(|m| m.pubkey == target_pubkey)
|
||||
.zip(requested_role)
|
||||
.filter(|(m, role)| m.role != role.as_str())
|
||||
{
|
||||
if !actor_role.is_some_and(|r| r.is_elevated()) {
|
||||
return Err(anyhow::anyhow!(
|
||||
"only owners/admins may change an active member's role"
|
||||
));
|
||||
}
|
||||
if target.role == "owner"
|
||||
&& role != buzz_db::channel::MemberRole::Owner
|
||||
&& members.iter().filter(|m| m.role == "owner").count() <= 1
|
||||
{
|
||||
return Err(anyhow::anyhow!(
|
||||
"cannot demote the last owner — transfer ownership first"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
// Self-add: always allowed regardless of policy.
|
||||
if target_pubkey == actor_bytes {
|
||||
return Ok(());
|
||||
@@ -1208,10 +1245,22 @@ async fn handle_put_user(
|
||||
let channel_id =
|
||||
extract_h_tag_channel(event).ok_or_else(|| anyhow::anyhow!("missing h tag"))?;
|
||||
let target_pubkey = extract_p_tag(event).ok_or_else(|| anyhow::anyhow!("missing p tag"))?;
|
||||
let role_str = extract_tag_value(event, "role").unwrap_or_else(|| "member".to_string());
|
||||
let role: MemberRole = role_str
|
||||
.parse()
|
||||
.map_err(|_| anyhow::anyhow!("invalid role: {role_str}"))?;
|
||||
// No role tag = no role change: preserve an existing member's current role and
|
||||
// fall back to Member only for a new member. Unconditionally defaulting to
|
||||
// Member let a bare PUT_USER silently demote an existing owner/admin.
|
||||
let role: MemberRole = match extract_tag_value(event, "role") {
|
||||
Some(role_str) => role_str
|
||||
.parse()
|
||||
.map_err(|_| anyhow::anyhow!("invalid role: {role_str}"))?,
|
||||
None => state
|
||||
.db
|
||||
.get_members(tenant.community(), channel_id)
|
||||
.await?
|
||||
.iter()
|
||||
.find(|m| m.pubkey == target_pubkey)
|
||||
.and_then(|m| m.role.parse().ok())
|
||||
.unwrap_or(MemberRole::Member),
|
||||
};
|
||||
|
||||
let actor_bytes = event.pubkey.to_bytes().to_vec();
|
||||
|
||||
|
||||
@@ -2475,3 +2475,441 @@ async fn test_reply_ingest_pushes_live_thread_summary() {
|
||||
|
||||
client.disconnect().await.expect("disconnect");
|
||||
}
|
||||
|
||||
/// Read a member's authoritative role from the relay-signed kind:39002 member
|
||||
/// list. The relay's own view of membership, not the client's — a kind:9000 can
|
||||
/// be `accepted` (stored) while its membership side effect fails, so asserting
|
||||
/// on the OK alone cannot see a broken write.
|
||||
async fn member_role(url: &str, keys: &Keys, channel_id: &str, pubkey_hex: &str) -> Option<String> {
|
||||
let mut ws = BuzzTestClient::connect(url, keys).await.expect("connect");
|
||||
let sid = sub_id("members");
|
||||
let filter = Filter::new()
|
||||
.kind(Kind::Custom(39002))
|
||||
.custom_tags(SingleLetterTag::lowercase(Alphabet::D), [channel_id]);
|
||||
ws.subscribe(&sid, vec![filter])
|
||||
.await
|
||||
.expect("subscribe 39002");
|
||||
let events = ws
|
||||
.collect_until_eose(&sid, Duration::from_secs(5))
|
||||
.await
|
||||
.expect("39002 EOSE");
|
||||
ws.disconnect().await.ok();
|
||||
// Latest 39002 wins; find this pubkey's tag. Shape is
|
||||
// ["p", pubkey, relay_url, role] (side_effects.rs:1058-1062) — role is
|
||||
// index 3, not 2.
|
||||
events.iter().max_by_key(|e| e.created_at).and_then(|e| {
|
||||
e.tags.iter().find_map(|t| {
|
||||
let p = t.as_slice();
|
||||
(p.len() >= 4 && p[0] == "p" && p[1] == pubkey_hex).then(|| p[3].clone())
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
/// SECURITY REPRO (Dawn): can an unprivileged NON-MEMBER demote the owner of an
|
||||
/// OPEN channel to `member` with a single kind:9000? Asserts the reported
|
||||
/// vulnerability is FIXED; it fails on vulnerable code.
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn test_nip29_put_user_cannot_demote_owner() {
|
||||
let url = relay_url();
|
||||
|
||||
let victim_keys = Keys::generate();
|
||||
let victim_hex = victim_keys.public_key().to_hex();
|
||||
let attacker_keys = Keys::generate();
|
||||
|
||||
// Victim creates an open channel -> victim is seeded as its owner.
|
||||
let channel_id = create_test_channel(&victim_keys).await;
|
||||
|
||||
let before = member_role(&url, &victim_keys, &channel_id, &victim_hex).await;
|
||||
assert_eq!(
|
||||
before.as_deref(),
|
||||
Some("owner"),
|
||||
"victim must start as owner (got {before:?})"
|
||||
);
|
||||
|
||||
// The attack: attacker (not a member, not the creator) publishes
|
||||
// kind:9000 { h=channel, p=victim, role=member }.
|
||||
let mut ws = BuzzTestClient::connect(&url, &attacker_keys)
|
||||
.await
|
||||
.expect("connect as attacker");
|
||||
let event = EventBuilder::new(Kind::Custom(9000), "")
|
||||
.tags([
|
||||
Tag::parse(["h", &channel_id]).unwrap(),
|
||||
Tag::parse(["p", &victim_hex]).unwrap(),
|
||||
Tag::parse(["role", "member"]).unwrap(),
|
||||
])
|
||||
.sign_with_keys(&attacker_keys)
|
||||
.expect("sign kind 9000");
|
||||
let ok = ws.send_event(event).await.expect("send kind 9000");
|
||||
ws.disconnect().await.ok();
|
||||
|
||||
let after = member_role(&url, &victim_keys, &channel_id, &victim_hex).await;
|
||||
println!(
|
||||
"attacker kind:9000 accepted = {} ({})",
|
||||
ok.accepted, ok.message
|
||||
);
|
||||
println!("victim role before = {before:?}, after = {after:?}");
|
||||
|
||||
assert_eq!(
|
||||
after.as_deref(),
|
||||
Some("owner"),
|
||||
"PRIVESC: unprivileged non-member demoted the channel owner to {after:?}"
|
||||
);
|
||||
assert!(
|
||||
!ok.accepted,
|
||||
"unprivileged non-member's role-demotion kind:9000 must be rejected"
|
||||
);
|
||||
}
|
||||
|
||||
/// SECURITY REPRO (Dawn), follow-on questions the report asserts but does not test:
|
||||
/// after the owner is demoted, (a) can the attacker promote itself to owner, and
|
||||
/// (b) can the ex-owner restore its own role? Both must be rejected — which is
|
||||
/// exactly what makes the demotion unrecoverable over the relay.
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn test_nip29_owner_demotion_recovery_paths() {
|
||||
let url = relay_url();
|
||||
|
||||
let victim_keys = Keys::generate();
|
||||
let victim_hex = victim_keys.public_key().to_hex();
|
||||
let attacker_keys = Keys::generate();
|
||||
let attacker_hex = attacker_keys.public_key().to_hex();
|
||||
|
||||
let channel_id = create_test_channel(&victim_keys).await;
|
||||
|
||||
let put_user = |signer: Keys, target_hex: String, role: &'static str| {
|
||||
let channel_id = channel_id.clone();
|
||||
let url = url.clone();
|
||||
async move {
|
||||
let mut ws = BuzzTestClient::connect(&url, &signer)
|
||||
.await
|
||||
.expect("connect");
|
||||
// `allow_self_tagging` is REQUIRED: EventBuilder otherwise silently
|
||||
// drops any `p` tag matching the signer (nostr-0.44.3
|
||||
// builder.rs:435-449), which would make self-targeted PUT_USER
|
||||
// events fail as "missing p tag" and mask the real verdict.
|
||||
let event = EventBuilder::new(Kind::Custom(9000), "")
|
||||
.allow_self_tagging()
|
||||
.tags([
|
||||
Tag::parse(["h", &channel_id]).unwrap(),
|
||||
Tag::parse(["p", &target_hex]).unwrap(),
|
||||
Tag::parse(["role", role]).unwrap(),
|
||||
])
|
||||
.sign_with_keys(&signer)
|
||||
.expect("sign kind 9000");
|
||||
let ok = ws.send_event(event).await.expect("send kind 9000");
|
||||
ws.disconnect().await.ok();
|
||||
ok
|
||||
}
|
||||
};
|
||||
|
||||
// Step 1: strip the owner.
|
||||
let demote = put_user(attacker_keys.clone(), victim_hex.clone(), "member").await;
|
||||
println!(
|
||||
"1. attacker demotes owner -> accepted={} {}",
|
||||
demote.accepted, demote.message
|
||||
);
|
||||
|
||||
// Step 2: attacker tries to make itself owner.
|
||||
let self_promote = put_user(attacker_keys.clone(), attacker_hex.clone(), "owner").await;
|
||||
println!(
|
||||
"2. attacker self->owner -> accepted={} {}",
|
||||
self_promote.accepted, self_promote.message
|
||||
);
|
||||
|
||||
// Step 3: ex-owner tries to restore itself.
|
||||
let restore = put_user(victim_keys.clone(), victim_hex.clone(), "owner").await;
|
||||
println!(
|
||||
"3. ex-owner restores self -> accepted={} {}",
|
||||
restore.accepted, restore.message
|
||||
);
|
||||
|
||||
// `accepted` only means the event was stored — the membership side effect can
|
||||
// still fail. Read the authoritative roles back from the relay-signed 39002.
|
||||
let mut ws = BuzzTestClient::connect(&url, &victim_keys)
|
||||
.await
|
||||
.expect("connect");
|
||||
let sid = sub_id("members-final");
|
||||
let filter = Filter::new().kind(Kind::Custom(39002)).custom_tags(
|
||||
SingleLetterTag::lowercase(Alphabet::D),
|
||||
[channel_id.as_str()],
|
||||
);
|
||||
ws.subscribe(&sid, vec![filter])
|
||||
.await
|
||||
.expect("subscribe 39002");
|
||||
let events = ws
|
||||
.collect_until_eose(&sid, Duration::from_secs(5))
|
||||
.await
|
||||
.expect("39002 EOSE");
|
||||
ws.disconnect().await.ok();
|
||||
let latest = events.iter().max_by_key(|e| e.created_at).expect("a 39002");
|
||||
let roles: Vec<(String, String)> = latest
|
||||
.tags
|
||||
.iter()
|
||||
.filter_map(|t| {
|
||||
let p = t.as_slice();
|
||||
(p.len() >= 4 && p[0] == "p").then(|| (p[1].clone(), p[3].clone()))
|
||||
})
|
||||
.collect();
|
||||
let role_of = |hex: &str| {
|
||||
roles
|
||||
.iter()
|
||||
.find(|(pk, _)| pk == hex)
|
||||
.map(|(_, r)| r.clone())
|
||||
};
|
||||
println!("FINAL victim role = {:?}", role_of(&victim_hex));
|
||||
println!("FINAL attacker role = {:?}", role_of(&attacker_hex));
|
||||
assert_eq!(
|
||||
role_of(&victim_hex).as_deref(),
|
||||
Some("owner"),
|
||||
"owner must retain its role through all three attempts"
|
||||
);
|
||||
assert_eq!(
|
||||
role_of(&attacker_hex),
|
||||
None,
|
||||
"attacker must never gain a role"
|
||||
);
|
||||
}
|
||||
|
||||
/// SECURITY REPRO (Dawn), finding #2: a kind:9000 PUT_USER with NO `role` tag.
|
||||
///
|
||||
/// `handle_put_user` used to default an absent role tag to `member`, so a bare
|
||||
/// self-targeted PUT_USER silently demoted the sender — no attacker required.
|
||||
/// `test_nip29_put_user_self_add_bypasses_policy` sends exactly this event but
|
||||
/// asserts only `ok.accepted`, and side-effect failures are logged rather than
|
||||
/// surfaced (ingest.rs:2460-2467), so an `accepted` assertion structurally
|
||||
/// cannot observe the demotion. This asserts the resulting STATE instead.
|
||||
///
|
||||
/// TWO owners on purpose. With a sole owner the last-owner guard rejects the
|
||||
/// demotion anyway, so the test would pass even with the role-preservation
|
||||
/// removed — it would be measuring a different defect's fix. A second owner
|
||||
/// disarms that guard, leaving the absent-role-tag handling as the only thing
|
||||
/// that can decide the outcome.
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn test_nip29_put_user_without_role_tag_preserves_role() {
|
||||
let url = relay_url();
|
||||
|
||||
let owner_a = Keys::generate();
|
||||
let owner_b = Keys::generate();
|
||||
let b_hex = owner_b.public_key().to_hex();
|
||||
let channel_id = create_test_channel(&owner_a).await;
|
||||
|
||||
// owner_a promotes owner_b, so the channel has two owners.
|
||||
let mut ws = BuzzTestClient::connect(&url, &owner_a)
|
||||
.await
|
||||
.expect("connect as owner_a");
|
||||
let promote = EventBuilder::new(Kind::Custom(9000), "")
|
||||
.tags([
|
||||
Tag::parse(["h", &channel_id]).unwrap(),
|
||||
Tag::parse(["p", &b_hex]).unwrap(),
|
||||
Tag::parse(["role", "owner"]).unwrap(),
|
||||
])
|
||||
.sign_with_keys(&owner_a)
|
||||
.expect("sign promote");
|
||||
let ok = ws.send_event(promote).await.expect("send promote");
|
||||
ws.disconnect().await.ok();
|
||||
assert!(ok.accepted, "promote rejected: {}", ok.message);
|
||||
assert_eq!(
|
||||
member_role(&url, &owner_a, &channel_id, &b_hex)
|
||||
.await
|
||||
.as_deref(),
|
||||
Some("owner"),
|
||||
"owner_b must be a second owner before the probe"
|
||||
);
|
||||
|
||||
// The probe: owner_b sends a bare self-targeted PUT_USER — h + p, no `role`.
|
||||
// `allow_self_tagging` is required or EventBuilder drops the self `p` tag
|
||||
// (nostr-0.44.3 builder.rs:435-449) and the event fails as "missing p tag".
|
||||
let mut ws = BuzzTestClient::connect(&url, &owner_b)
|
||||
.await
|
||||
.expect("connect as owner_b");
|
||||
let bare = EventBuilder::new(Kind::Custom(9000), "")
|
||||
.allow_self_tagging()
|
||||
.tags([
|
||||
Tag::parse(["h", &channel_id]).unwrap(),
|
||||
Tag::parse(["p", &b_hex]).unwrap(),
|
||||
])
|
||||
.sign_with_keys(&owner_b)
|
||||
.expect("sign bare put_user");
|
||||
let ok = ws.send_event(bare).await.expect("send bare put_user");
|
||||
ws.disconnect().await.ok();
|
||||
assert!(
|
||||
ok.accepted,
|
||||
"self-targeted PUT_USER must stay accepted: {}",
|
||||
ok.message
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
member_role(&url, &owner_a, &channel_id, &b_hex)
|
||||
.await
|
||||
.as_deref(),
|
||||
Some("owner"),
|
||||
"an absent role tag means no role change — it must not demote an owner"
|
||||
);
|
||||
}
|
||||
|
||||
/// SECURITY (Dawn), relay-layer guard isolation — the *validator*, not the DB.
|
||||
///
|
||||
/// The DB guards in `add_member` are what actually stop the privesc, and every
|
||||
/// other test here asserts the resulting STATE ("the role didn't change").
|
||||
/// That makes them structurally blind to `validate_admin_event`: stubbing out
|
||||
/// either relay-side check leaves the whole nip29 suite green, because the DB
|
||||
/// still refuses the write and the role is still correct. Verified by mutation
|
||||
/// — the relay returns `accepted:true` and logs `Side effect failed: access
|
||||
/// denied: ...` while the state assertion happily passes.
|
||||
///
|
||||
/// The relay guards earn their keep by giving the client an honest
|
||||
/// `accepted:false` instead of an OK for an event that silently fails after the
|
||||
/// fact. So these two tests assert `accepted == false` — the one observable
|
||||
/// only the validator controls — and each is shaped so exactly one guard can
|
||||
/// fire.
|
||||
///
|
||||
/// Guard under test: "only owners/admins may change an active member's role".
|
||||
/// TWO owners on purpose, so the last-owner guard cannot fire and take the
|
||||
/// credit; a plain member targeting a co-owner leaves the actor check as the
|
||||
/// only thing that can reject.
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn test_nip29_relay_rejects_role_change_by_unprivileged_actor() {
|
||||
let url = relay_url();
|
||||
|
||||
let owner_a = Keys::generate();
|
||||
let owner_b = Keys::generate();
|
||||
let b_hex = owner_b.public_key().to_hex();
|
||||
let attacker = Keys::generate();
|
||||
let attacker_hex = attacker.public_key().to_hex();
|
||||
let channel_id = create_test_channel(&owner_a).await;
|
||||
|
||||
// owner_a promotes owner_b -> the channel has two owners.
|
||||
let mut ws = BuzzTestClient::connect(&url, &owner_a)
|
||||
.await
|
||||
.expect("connect as owner_a");
|
||||
let promote = EventBuilder::new(Kind::Custom(9000), "")
|
||||
.tags([
|
||||
Tag::parse(["h", &channel_id]).unwrap(),
|
||||
Tag::parse(["p", &b_hex]).unwrap(),
|
||||
Tag::parse(["role", "owner"]).unwrap(),
|
||||
])
|
||||
.sign_with_keys(&owner_a)
|
||||
.expect("sign promote");
|
||||
let ok = ws.send_event(promote).await.expect("send promote");
|
||||
ws.disconnect().await.ok();
|
||||
assert!(ok.accepted, "promote rejected: {}", ok.message);
|
||||
|
||||
// The attacker joins the open channel as a plain member.
|
||||
let mut ws = BuzzTestClient::connect(&url, &attacker)
|
||||
.await
|
||||
.expect("connect as attacker");
|
||||
let join = EventBuilder::new(Kind::Custom(9000), "")
|
||||
.allow_self_tagging()
|
||||
.tags([
|
||||
Tag::parse(["h", &channel_id]).unwrap(),
|
||||
Tag::parse(["p", &attacker_hex]).unwrap(),
|
||||
Tag::parse(["role", "member"]).unwrap(),
|
||||
])
|
||||
.sign_with_keys(&attacker)
|
||||
.expect("sign self-join");
|
||||
let ok = ws.send_event(join).await.expect("send self-join");
|
||||
ws.disconnect().await.ok();
|
||||
assert!(ok.accepted, "self-join rejected: {}", ok.message);
|
||||
assert_eq!(
|
||||
member_role(&url, &owner_a, &channel_id, &attacker_hex)
|
||||
.await
|
||||
.as_deref(),
|
||||
Some("member"),
|
||||
"attacker must be an active plain member before the probe"
|
||||
);
|
||||
|
||||
// The probe: a plain member demotes a co-owner. Two owners remain, so only
|
||||
// the actor-authorization guard can reject this.
|
||||
let mut ws = BuzzTestClient::connect(&url, &attacker)
|
||||
.await
|
||||
.expect("connect as attacker");
|
||||
let attack = EventBuilder::new(Kind::Custom(9000), "")
|
||||
.tags([
|
||||
Tag::parse(["h", &channel_id]).unwrap(),
|
||||
Tag::parse(["p", &b_hex]).unwrap(),
|
||||
Tag::parse(["role", "member"]).unwrap(),
|
||||
])
|
||||
.sign_with_keys(&attacker)
|
||||
.expect("sign attack");
|
||||
let ok = ws.send_event(attack).await.expect("send attack");
|
||||
ws.disconnect().await.ok();
|
||||
println!(
|
||||
"unprivileged co-owner demotion -> accepted={} {}",
|
||||
ok.accepted, ok.message
|
||||
);
|
||||
|
||||
assert!(
|
||||
!ok.accepted,
|
||||
"the relay validator must reject an unprivileged actor's role change, \
|
||||
not accept it and let the side effect fail silently"
|
||||
);
|
||||
assert_eq!(
|
||||
member_role(&url, &owner_a, &channel_id, &b_hex)
|
||||
.await
|
||||
.as_deref(),
|
||||
Some("owner"),
|
||||
"co-owner must keep their role"
|
||||
);
|
||||
}
|
||||
|
||||
/// SECURITY (Dawn), relay-layer guard isolation — see the test above for why
|
||||
/// `accepted` rather than state is the assertion that matters here.
|
||||
///
|
||||
/// Guard under test: the relay-side last-owner check. The actor is the SOLE
|
||||
/// owner demoting themselves, so the actor-authorization guard is satisfied
|
||||
/// (an owner is elevated) and cannot mask the result — only the last-owner
|
||||
/// check can reject.
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn test_nip29_relay_rejects_last_owner_self_demotion() {
|
||||
let url = relay_url();
|
||||
|
||||
let owner = Keys::generate();
|
||||
let owner_hex = owner.public_key().to_hex();
|
||||
let channel_id = create_test_channel(&owner).await;
|
||||
|
||||
assert_eq!(
|
||||
member_role(&url, &owner, &channel_id, &owner_hex)
|
||||
.await
|
||||
.as_deref(),
|
||||
Some("owner"),
|
||||
"creator must be the sole owner before the probe"
|
||||
);
|
||||
|
||||
// The probe: the sole owner demotes themselves. Elevated actor, so the
|
||||
// actor check passes; the last-owner guard is the only thing left.
|
||||
let mut ws = BuzzTestClient::connect(&url, &owner)
|
||||
.await
|
||||
.expect("connect as owner");
|
||||
let demote = EventBuilder::new(Kind::Custom(9000), "")
|
||||
.allow_self_tagging()
|
||||
.tags([
|
||||
Tag::parse(["h", &channel_id]).unwrap(),
|
||||
Tag::parse(["p", &owner_hex]).unwrap(),
|
||||
Tag::parse(["role", "member"]).unwrap(),
|
||||
])
|
||||
.sign_with_keys(&owner)
|
||||
.expect("sign self-demote");
|
||||
let ok = ws.send_event(demote).await.expect("send self-demote");
|
||||
ws.disconnect().await.ok();
|
||||
println!(
|
||||
"sole-owner self-demotion -> accepted={} {}",
|
||||
ok.accepted, ok.message
|
||||
);
|
||||
|
||||
assert!(
|
||||
!ok.accepted,
|
||||
"the relay validator must reject demoting the last owner, not accept \
|
||||
it and let the side effect fail silently"
|
||||
);
|
||||
assert_eq!(
|
||||
member_role(&url, &owner, &channel_id, &owner_hex)
|
||||
.await
|
||||
.as_deref(),
|
||||
Some("owner"),
|
||||
"the last owner must keep their role"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -98,6 +98,7 @@ export default defineConfig({
|
||||
"**/inbox-reactions.spec.ts",
|
||||
"**/send-channel-binding.spec.ts",
|
||||
"**/project-commit-detail.spec.ts",
|
||||
"**/project-inbox.spec.ts",
|
||||
"**/project-pr-review.spec.ts",
|
||||
"**/persona-model-combobox-screenshots.spec.ts",
|
||||
"**/drafts-screenshots.spec.ts",
|
||||
|
||||
@@ -68,7 +68,20 @@ pub async fn get_feed(
|
||||
|
||||
// Mentions: messages that reference me via #p.
|
||||
let mut mention_filter = serde_json::json!({
|
||||
"kinds": [9, 40002, 1, 45001, 45003],
|
||||
"kinds": [
|
||||
9,
|
||||
40002,
|
||||
1,
|
||||
45001,
|
||||
45003,
|
||||
buzz_core_pkg::kind::KIND_GIT_PULL_REQUEST,
|
||||
buzz_core_pkg::kind::KIND_GIT_PR_UPDATE,
|
||||
buzz_core_pkg::kind::KIND_GIT_ISSUE,
|
||||
buzz_core_pkg::kind::KIND_GIT_STATUS_OPEN,
|
||||
buzz_core_pkg::kind::KIND_GIT_STATUS_MERGED,
|
||||
buzz_core_pkg::kind::KIND_GIT_STATUS_CLOSED,
|
||||
buzz_core_pkg::kind::KIND_GIT_STATUS_DRAFT,
|
||||
],
|
||||
"#p": [my_pubkey],
|
||||
"limit": cap,
|
||||
});
|
||||
|
||||
@@ -6,6 +6,10 @@ import {
|
||||
getThreadReference,
|
||||
isBroadcastReply,
|
||||
} from "@/features/messages/lib/threading";
|
||||
import {
|
||||
getProjectInboxReference,
|
||||
isProjectInboxItem,
|
||||
} from "@/features/home/lib/projectInbox";
|
||||
import type { TimelineReaction } from "@/features/messages/types";
|
||||
import type {
|
||||
Channel,
|
||||
@@ -18,6 +22,7 @@ import { resolveMentionProps } from "@/shared/lib/resolveMentionNames";
|
||||
|
||||
export type InboxFilter =
|
||||
| "all"
|
||||
| "project"
|
||||
| "mention"
|
||||
| "thread"
|
||||
| "needs_action"
|
||||
@@ -29,10 +34,10 @@ export type InboxFilter =
|
||||
export type InboxItem = {
|
||||
avatarUrl: string | null;
|
||||
/**
|
||||
* Stable conversation identity: `rootId ?? parentId ?? event.id` for the
|
||||
* thread group. Does NOT change when a new reply advances the representative
|
||||
* latest event. Use this for lifecycle continuity: scroll gating, draft
|
||||
* keys, local-reply storage, and selection identity.
|
||||
* Stable conversation identity: the NIP-10 root for messages, or a
|
||||
* repository-scoped root for Buzz Git work. Does NOT change when a new reply
|
||||
* advances the representative latest event. Use this for lifecycle
|
||||
* continuity: scroll gating, draft keys, local-reply storage, and selection.
|
||||
*/
|
||||
conversationId: string;
|
||||
id: string;
|
||||
@@ -136,7 +141,33 @@ function diffInDays(from: Date, to: Date) {
|
||||
);
|
||||
}
|
||||
|
||||
function feedHeadline(item: FeedItem) {
|
||||
function tagValue(item: FeedItem, name: string) {
|
||||
return item.tags.find((tag) => tag[0] === name)?.[1]?.trim() || null;
|
||||
}
|
||||
|
||||
function projectRootItem(item: FeedItem, groupItems: readonly FeedItem[]) {
|
||||
return (
|
||||
groupItems.find(
|
||||
(candidate) => candidate.kind === 1618 || candidate.kind === 1621,
|
||||
) ?? item
|
||||
);
|
||||
}
|
||||
|
||||
function projectTypeLabel(item: FeedItem) {
|
||||
if (item.kind === 1618) return "Pull request";
|
||||
if (item.kind === 1621) return "Issue";
|
||||
return "Project update";
|
||||
}
|
||||
|
||||
function feedHeadline(item: FeedItem, groupItems: readonly FeedItem[] = []) {
|
||||
if (isProjectInboxItem(item)) {
|
||||
const root = projectRootItem(item, groupItems);
|
||||
return (
|
||||
(tagValue(root, "subject") ?? root.content.trim().split("\n")[0]) ||
|
||||
projectTypeLabel(root)
|
||||
);
|
||||
}
|
||||
|
||||
switch (item.kind) {
|
||||
case 40007:
|
||||
return "Reminder";
|
||||
@@ -242,6 +273,14 @@ function resolveGroupChannel(
|
||||
export function getInboxTypeLabel(item: InboxItem): InboxTypeLabel {
|
||||
const channelName = item.channelLabel;
|
||||
|
||||
if (item.groupItems.some(isProjectInboxItem)) {
|
||||
const root = projectRootItem(item.item, item.groupItems);
|
||||
return {
|
||||
text: projectTypeLabel(root),
|
||||
channelLabel: null,
|
||||
};
|
||||
}
|
||||
|
||||
if (item.item.channelType === "dm") {
|
||||
return {
|
||||
text: item.senderLabel ? `DM from ${item.senderLabel}` : "DM",
|
||||
@@ -300,23 +339,50 @@ function categoryPriority(category: FeedItemCategory) {
|
||||
}
|
||||
|
||||
function getInboxThreadKey(item: FeedItem) {
|
||||
const projectReference = getProjectInboxReference(item);
|
||||
if (projectReference) {
|
||||
return `project:${projectReference.repoAddress}:${projectReference.rootId}`;
|
||||
}
|
||||
|
||||
const thread = getThreadReference(item.tags);
|
||||
return thread.rootId ?? thread.parentId ?? item.id;
|
||||
}
|
||||
|
||||
function getStableConversationId(item: FeedItem) {
|
||||
return getInboxItemConversationId(item);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the stable conversation ID for any FeedItem or relay event: the
|
||||
* NIP-10 root tag id, falling back to parent-reply tag id, then event id.
|
||||
* Returns the stable conversation ID for any FeedItem or relay event. Buzz Git
|
||||
* roots include their repository coordinate; messages use the NIP-10 root,
|
||||
* parent-reply tag, then event id.
|
||||
* This is the same derivation used by `buildInboxItems` for `conversationId`.
|
||||
*/
|
||||
export function getInboxConversationId(
|
||||
tags: string[][],
|
||||
eventId: string,
|
||||
kind?: number,
|
||||
): string {
|
||||
if (kind !== undefined) {
|
||||
const projectReference = getProjectInboxReference({
|
||||
id: eventId,
|
||||
kind,
|
||||
tags,
|
||||
});
|
||||
if (projectReference) {
|
||||
return `project:${projectReference.repoAddress}:${projectReference.rootId}`;
|
||||
}
|
||||
}
|
||||
|
||||
const thread = getThreadReference(tags);
|
||||
return thread.rootId ?? thread.parentId ?? eventId;
|
||||
}
|
||||
|
||||
/** Returns the stable conversation identity for a complete Inbox feed item. */
|
||||
export function getInboxItemConversationId(item: FeedItem) {
|
||||
return getInboxConversationId(item.tags, item.id, item.kind);
|
||||
}
|
||||
|
||||
function formatInboxTimestamp(unixSeconds: number) {
|
||||
const date = new Date(unixSeconds * 1_000);
|
||||
const now = new Date();
|
||||
@@ -436,7 +502,7 @@ export function buildInboxItems({
|
||||
|
||||
group.items.push(item);
|
||||
group.latestActivityAt = Math.max(group.latestActivityAt, item.createdAt);
|
||||
if (item.id === threadKey) {
|
||||
if (item.id === getStableConversationId(item)) {
|
||||
group.rootItem = item;
|
||||
}
|
||||
|
||||
@@ -447,7 +513,8 @@ export function buildInboxItems({
|
||||
.sort(
|
||||
([, left], [, right]) => right.latestActivityAt - left.latestActivityAt,
|
||||
)
|
||||
.map(([conversationId, group]) => {
|
||||
.map(([, group]) => {
|
||||
const conversationId = getStableConversationId(group.items[0]);
|
||||
const latestItem = group.items.reduce((latest, current) =>
|
||||
current.createdAt > latest.createdAt ? current : latest,
|
||||
);
|
||||
@@ -461,7 +528,7 @@ export function buildInboxItems({
|
||||
profiles,
|
||||
preferResolvedSelfLabel: true,
|
||||
});
|
||||
const subject = feedHeadline(item);
|
||||
const subject = feedHeadline(item, group.items);
|
||||
const preview = feedPreview(item);
|
||||
const { mentionNames, mentionPubkeysByName } = resolveMentionProps(
|
||||
item.tags,
|
||||
|
||||
@@ -3,6 +3,7 @@ import {
|
||||
type InboxContextMessage,
|
||||
type InboxFilter,
|
||||
} from "@/features/home/lib/inbox";
|
||||
import { isProjectInboxItem } from "@/features/home/lib/projectInbox";
|
||||
import {
|
||||
getChannelIdFromTags,
|
||||
getThreadReference,
|
||||
@@ -39,6 +40,12 @@ export function matchesInboxFilter(
|
||||
);
|
||||
}
|
||||
|
||||
if (filter === "project") {
|
||||
return [item.item, ...(item.groupItems ?? [])].some(
|
||||
(groupItem) => groupItem && isProjectInboxItem(groupItem),
|
||||
);
|
||||
}
|
||||
|
||||
return item.categories.includes(filter);
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,235 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { buildInboxItems, getInboxTypeLabel } from "./inbox.ts";
|
||||
import { matchesInboxFilter } from "./inboxViewHelpers.ts";
|
||||
import {
|
||||
getProjectInboxReference,
|
||||
isProjectInboxItem,
|
||||
resolveProjectInboxWorkItem,
|
||||
} from "./projectInbox.ts";
|
||||
|
||||
const OWNER = "a".repeat(64);
|
||||
const REVIEWER = "b".repeat(64);
|
||||
const REPO_ADDRESS = `30617:${OWNER}:buzz`;
|
||||
const PR_ID = "c".repeat(64);
|
||||
const ISSUE_ID = "d".repeat(64);
|
||||
|
||||
function feedItem(overrides = {}) {
|
||||
return {
|
||||
id: PR_ID,
|
||||
kind: 1618,
|
||||
pubkey: OWNER,
|
||||
content: "Inbox support",
|
||||
createdAt: 1_700_000_000,
|
||||
channelId: null,
|
||||
channelName: "",
|
||||
tags: [
|
||||
["a", REPO_ADDRESS],
|
||||
["p", REVIEWER],
|
||||
["subject", "Add project work items to Inbox"],
|
||||
],
|
||||
category: "mention",
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
const project = {
|
||||
id: "buzz",
|
||||
name: "Buzz",
|
||||
owner: OWNER,
|
||||
repoAddress: REPO_ADDRESS,
|
||||
};
|
||||
|
||||
const pullRequest = {
|
||||
id: PR_ID,
|
||||
author: OWNER,
|
||||
title: "Add project work items to Inbox",
|
||||
};
|
||||
|
||||
const issue = {
|
||||
id: ISSUE_ID,
|
||||
author: REVIEWER,
|
||||
title: "Inbox issue",
|
||||
};
|
||||
|
||||
test("recognizes project roots and project thread activity", () => {
|
||||
assert.equal(isProjectInboxItem(feedItem()), true);
|
||||
assert.equal(
|
||||
isProjectInboxItem(
|
||||
feedItem({
|
||||
id: "e".repeat(64),
|
||||
kind: 1,
|
||||
tags: [
|
||||
["a", REPO_ADDRESS],
|
||||
["e", PR_ID, "", "root"],
|
||||
["p", REVIEWER],
|
||||
],
|
||||
}),
|
||||
),
|
||||
true,
|
||||
);
|
||||
assert.equal(
|
||||
isProjectInboxItem(
|
||||
feedItem({
|
||||
kind: 9,
|
||||
tags: [
|
||||
["a", REPO_ADDRESS],
|
||||
["e", PR_ID, "", "root"],
|
||||
],
|
||||
}),
|
||||
),
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test("resolves the canonical project root from status and comment events", () => {
|
||||
assert.deepEqual(getProjectInboxReference(feedItem()), {
|
||||
repoAddress: REPO_ADDRESS,
|
||||
rootId: PR_ID,
|
||||
});
|
||||
assert.deepEqual(
|
||||
getProjectInboxReference(
|
||||
feedItem({
|
||||
id: "f".repeat(64),
|
||||
kind: 1631,
|
||||
tags: [
|
||||
["a", REPO_ADDRESS],
|
||||
["e", PR_ID, "", "root"],
|
||||
],
|
||||
}),
|
||||
),
|
||||
{
|
||||
repoAddress: REPO_ADDRESS,
|
||||
rootId: PR_ID,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("matches a selected inbox event to its canonical pull request or issue", () => {
|
||||
const workItems = {
|
||||
pullRequests: {
|
||||
items: [{ project, pullRequest }],
|
||||
failedSections: [],
|
||||
},
|
||||
issues: {
|
||||
items: [{ project, issue }],
|
||||
failedSections: [],
|
||||
},
|
||||
};
|
||||
|
||||
assert.deepEqual(resolveProjectInboxWorkItem(feedItem(), workItems), {
|
||||
type: "pull-request",
|
||||
project,
|
||||
pullRequest,
|
||||
});
|
||||
assert.deepEqual(
|
||||
resolveProjectInboxWorkItem(
|
||||
feedItem({
|
||||
id: ISSUE_ID,
|
||||
kind: 1621,
|
||||
tags: [
|
||||
["a", REPO_ADDRESS],
|
||||
["p", OWNER],
|
||||
["subject", "Inbox issue"],
|
||||
],
|
||||
}),
|
||||
workItems,
|
||||
),
|
||||
{
|
||||
type: "issue",
|
||||
project,
|
||||
issue,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("presents project work with its canonical subject and project filter", () => {
|
||||
const [item] = buildInboxItems({
|
||||
feed: {
|
||||
feed: {
|
||||
mentions: [feedItem()],
|
||||
needsAction: [],
|
||||
activity: [],
|
||||
agentActivity: [],
|
||||
},
|
||||
meta: { since: 0, total: 1, generatedAt: 1_700_000_000 },
|
||||
},
|
||||
});
|
||||
|
||||
assert.equal(item.subject, "Add project work items to Inbox");
|
||||
assert.deepEqual(getInboxTypeLabel(item), {
|
||||
text: "Pull request",
|
||||
channelLabel: null,
|
||||
});
|
||||
assert.equal(matchesInboxFilter(item, "project"), true);
|
||||
assert.equal(
|
||||
matchesInboxFilter(
|
||||
{
|
||||
...item,
|
||||
item: feedItem({ kind: 9, tags: [["h", "channel-id"]] }),
|
||||
groupItems: [feedItem({ kind: 9, tags: [["h", "channel-id"]] })],
|
||||
},
|
||||
"project",
|
||||
),
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test("groups uppercase NIP-34 pull request updates with their root", () => {
|
||||
const update = feedItem({
|
||||
id: "f".repeat(64),
|
||||
kind: 1619,
|
||||
createdAt: 1_700_000_100,
|
||||
content: "Pushed another commit",
|
||||
tags: [
|
||||
["a", REPO_ADDRESS],
|
||||
["E", PR_ID],
|
||||
["p", REVIEWER],
|
||||
],
|
||||
});
|
||||
const items = buildInboxItems({
|
||||
feed: {
|
||||
feed: {
|
||||
mentions: [feedItem(), update],
|
||||
needsAction: [],
|
||||
activity: [],
|
||||
agentActivity: [],
|
||||
},
|
||||
meta: { since: 0, total: 2, generatedAt: 1_700_000_100 },
|
||||
},
|
||||
});
|
||||
|
||||
assert.equal(items.length, 1);
|
||||
assert.equal(items[0].id, update.id);
|
||||
assert.equal(items[0].subject, "Add project work items to Inbox");
|
||||
});
|
||||
|
||||
test("does not group project events from different repositories", () => {
|
||||
const otherRepoAddress = `30617:${"e".repeat(64)}:other`;
|
||||
const items = buildInboxItems({
|
||||
feed: {
|
||||
feed: {
|
||||
mentions: [
|
||||
feedItem(),
|
||||
feedItem({
|
||||
id: "f".repeat(64),
|
||||
kind: 1619,
|
||||
tags: [
|
||||
["a", otherRepoAddress],
|
||||
["E", PR_ID],
|
||||
["p", REVIEWER],
|
||||
],
|
||||
}),
|
||||
],
|
||||
needsAction: [],
|
||||
activity: [],
|
||||
agentActivity: [],
|
||||
},
|
||||
meta: { since: 0, total: 2, generatedAt: 1_700_000_100 },
|
||||
},
|
||||
});
|
||||
|
||||
assert.equal(items.length, 2);
|
||||
assert.notEqual(items[0].conversationId, items[1].conversationId);
|
||||
});
|
||||
@@ -0,0 +1,99 @@
|
||||
import type {
|
||||
Project,
|
||||
ProjectIssue,
|
||||
ProjectPullRequest,
|
||||
} from "@/features/projects/hooks";
|
||||
import type { ProjectsWorkItemsResult } from "@/features/projects/projectWorkItems";
|
||||
import type { FeedItem } from "@/shared/api/types";
|
||||
import {
|
||||
KIND_GIT_ISSUE,
|
||||
KIND_GIT_PR_UPDATE,
|
||||
KIND_GIT_PULL_REQUEST,
|
||||
KIND_GIT_STATUS_CLOSED,
|
||||
KIND_GIT_STATUS_DRAFT,
|
||||
KIND_GIT_STATUS_MERGED,
|
||||
KIND_GIT_STATUS_OPEN,
|
||||
KIND_TEXT_NOTE,
|
||||
} from "@/shared/constants/kinds";
|
||||
|
||||
const PROJECT_ROOT_KINDS = new Set([KIND_GIT_PULL_REQUEST, KIND_GIT_ISSUE]);
|
||||
const PROJECT_ACTIVITY_KINDS = new Set([
|
||||
KIND_TEXT_NOTE,
|
||||
KIND_GIT_PR_UPDATE,
|
||||
KIND_GIT_STATUS_OPEN,
|
||||
KIND_GIT_STATUS_MERGED,
|
||||
KIND_GIT_STATUS_CLOSED,
|
||||
KIND_GIT_STATUS_DRAFT,
|
||||
]);
|
||||
const REPO_ADDRESS_PATTERN = /^30617:[0-9a-f]{64}:.+$/i;
|
||||
|
||||
export type ProjectInboxWorkItem =
|
||||
| {
|
||||
type: "pull-request";
|
||||
project: Project;
|
||||
pullRequest: ProjectPullRequest;
|
||||
}
|
||||
| {
|
||||
type: "issue";
|
||||
project: Project;
|
||||
issue: ProjectIssue;
|
||||
};
|
||||
|
||||
function tagValue(item: Pick<FeedItem, "tags">, name: string) {
|
||||
return item.tags.find(
|
||||
(tag) => tag[0] === name && typeof tag[1] === "string" && tag[1].length > 0,
|
||||
)?.[1];
|
||||
}
|
||||
|
||||
/** Returns the canonical Buzz Git repository and root event for an Inbox row. */
|
||||
export function getProjectInboxReference(
|
||||
item: Pick<FeedItem, "id" | "kind" | "tags">,
|
||||
): { repoAddress: string; rootId: string } | null {
|
||||
const repoAddress = tagValue(item, "a");
|
||||
if (!repoAddress || !REPO_ADDRESS_PATTERN.test(repoAddress)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (PROJECT_ROOT_KINDS.has(item.kind)) {
|
||||
return { repoAddress, rootId: item.id };
|
||||
}
|
||||
|
||||
if (!PROJECT_ACTIVITY_KINDS.has(item.kind)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const rootId = tagValue(item, "e") ?? tagValue(item, "E");
|
||||
return rootId ? { repoAddress, rootId } : null;
|
||||
}
|
||||
|
||||
/** Whether a feed event belongs to a Buzz Git pull request or issue thread. */
|
||||
export function isProjectInboxItem(item: FeedItem) {
|
||||
return getProjectInboxReference(item) !== null;
|
||||
}
|
||||
|
||||
/** Resolves an Inbox event to the current canonical Buzz Git work item. */
|
||||
export function resolveProjectInboxWorkItem(
|
||||
item: FeedItem,
|
||||
workItems: ProjectsWorkItemsResult<Project> | undefined,
|
||||
): ProjectInboxWorkItem | null {
|
||||
const reference = getProjectInboxReference(item);
|
||||
if (!reference || !workItems) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const pullRequestEntry = workItems.pullRequests.items.find(
|
||||
({ project, pullRequest }) =>
|
||||
project.repoAddress === reference.repoAddress &&
|
||||
pullRequest.id === reference.rootId,
|
||||
);
|
||||
if (pullRequestEntry) {
|
||||
return { type: "pull-request", ...pullRequestEntry };
|
||||
}
|
||||
|
||||
const issueEntry = workItems.issues.items.find(
|
||||
({ issue, project }) =>
|
||||
project.repoAddress === reference.repoAddress &&
|
||||
issue.id === reference.rootId,
|
||||
);
|
||||
return issueEntry ? { type: "issue", ...issueEntry } : null;
|
||||
}
|
||||
@@ -14,7 +14,7 @@ import {
|
||||
type InboxReply,
|
||||
buildInboxItems,
|
||||
formatInboxFullTimestamp,
|
||||
getInboxConversationId,
|
||||
getInboxItemConversationId,
|
||||
} from "@/features/home/lib/inbox";
|
||||
import { useInboxSelectionAnchor } from "@/features/home/useInboxSelectionAnchor";
|
||||
import {
|
||||
@@ -413,7 +413,7 @@ export function HomeView({
|
||||
// correct row selected (by conversationId) even after the anchor event has
|
||||
// been displaced from groupItems by a newer representative.
|
||||
const latchedConversationId = activeLatchedItem
|
||||
? getInboxConversationId(activeLatchedItem.tags, activeLatchedItem.id)
|
||||
? getInboxItemConversationId(activeLatchedItem)
|
||||
: null;
|
||||
const selectedConversationId =
|
||||
selectedItemFromAll?.conversationId ?? latchedConversationId;
|
||||
|
||||
@@ -6,6 +6,8 @@ import type {
|
||||
InboxItem,
|
||||
InboxReply,
|
||||
} from "@/features/home/lib/inbox";
|
||||
import { getProjectInboxReference } from "@/features/home/lib/projectInbox";
|
||||
import { ProjectInboxDetail } from "@/features/home/ui/ProjectInboxDetail";
|
||||
import { ChannelMembersBar } from "@/features/channels/ui/ChannelMembersBar";
|
||||
import { useCommunities } from "@/features/communities/useCommunities";
|
||||
import { formatInboxTypeLabel } from "@/features/home/lib/inbox";
|
||||
@@ -102,7 +104,23 @@ type InboxDetailPaneProps = {
|
||||
) => Promise<void>;
|
||||
};
|
||||
|
||||
export function InboxDetailPane({
|
||||
/** Routes Inbox selections to their canonical message or Buzz Git detail. */
|
||||
export function InboxDetailPane(props: InboxDetailPaneProps) {
|
||||
if (props.item && getProjectInboxReference(props.item.item)) {
|
||||
return (
|
||||
<ProjectInboxDetail
|
||||
isSinglePanelView={props.isSinglePanelView}
|
||||
item={props.item}
|
||||
onBack={props.onBack}
|
||||
profiles={props.profiles}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
return <InboxMessageDetailPane {...props} />;
|
||||
}
|
||||
|
||||
function InboxMessageDetailPane({
|
||||
agentPubkeys,
|
||||
canDelete,
|
||||
canOpenChannel,
|
||||
|
||||
@@ -50,6 +50,7 @@ import { VirtualizedList } from "@/shared/ui/VirtualizedList";
|
||||
|
||||
const FILTER_OPTIONS: Array<{ label: string; value: InboxFilter }> = [
|
||||
{ value: "all", label: "All" },
|
||||
{ value: "project", label: "Projects" },
|
||||
{ value: "mention", label: "Mentions" },
|
||||
{ value: "thread", label: "Threads" },
|
||||
{ value: "needs_action", label: "Needs Action" },
|
||||
|
||||
@@ -0,0 +1,129 @@
|
||||
import { ArrowLeft } from "lucide-react";
|
||||
|
||||
import { useAppNavigation } from "@/app/navigation/useAppNavigation";
|
||||
import type { InboxItem } from "@/features/home/lib/inbox";
|
||||
import { resolveProjectInboxWorkItem } from "@/features/home/lib/projectInbox";
|
||||
import { ProjectInboxDetailPane } from "@/features/home/ui/ProjectInboxDetailPane";
|
||||
import {
|
||||
useProjectsQuery,
|
||||
useProjectsWorkItemsQuery,
|
||||
} from "@/features/projects/hooks";
|
||||
import type { UserProfileLookup } from "@/features/profile/lib/identity";
|
||||
import { Button } from "@/shared/ui/button";
|
||||
|
||||
type ProjectInboxDetailProps = {
|
||||
isSinglePanelView?: boolean;
|
||||
item: InboxItem;
|
||||
onBack?: () => void;
|
||||
profiles?: UserProfileLookup;
|
||||
};
|
||||
|
||||
function ProjectInboxStatus({
|
||||
message,
|
||||
onBack,
|
||||
onRetry,
|
||||
}: {
|
||||
message: string;
|
||||
onBack?: () => void;
|
||||
onRetry?: () => void;
|
||||
}) {
|
||||
return (
|
||||
<section className="flex min-h-0 min-w-0 flex-col bg-background/60">
|
||||
{onBack ? (
|
||||
<div className="flex min-h-13 items-center px-5 py-2">
|
||||
<Button
|
||||
aria-label="Back to Inbox"
|
||||
onClick={onBack}
|
||||
size="icon"
|
||||
type="button"
|
||||
variant="ghost"
|
||||
>
|
||||
<ArrowLeft className="h-4 w-4" />
|
||||
</Button>
|
||||
</div>
|
||||
) : null}
|
||||
<div className="flex min-h-0 flex-1 flex-col items-center justify-center gap-3 p-6 text-center">
|
||||
<p className="text-sm text-muted-foreground">{message}</p>
|
||||
{onRetry ? (
|
||||
<Button onClick={onRetry} size="sm" type="button" variant="outline">
|
||||
Retry
|
||||
</Button>
|
||||
) : null}
|
||||
</div>
|
||||
</section>
|
||||
);
|
||||
}
|
||||
|
||||
/** Resolves and renders the live Buzz Git object selected from Inbox. */
|
||||
export function ProjectInboxDetail({
|
||||
isSinglePanelView = false,
|
||||
item,
|
||||
onBack,
|
||||
profiles,
|
||||
}: ProjectInboxDetailProps) {
|
||||
const { goProject } = useAppNavigation();
|
||||
const projectsQuery = useProjectsQuery();
|
||||
const projectsWorkItemsQuery = useProjectsWorkItemsQuery(
|
||||
projectsQuery.data ?? [],
|
||||
);
|
||||
const workItem = resolveProjectInboxWorkItem(
|
||||
item.item,
|
||||
projectsWorkItemsQuery.data,
|
||||
);
|
||||
|
||||
if (!workItem) {
|
||||
const error = projectsQuery.error ?? projectsWorkItemsQuery.error;
|
||||
const isLoading =
|
||||
projectsQuery.isLoading || projectsWorkItemsQuery.isLoading;
|
||||
return (
|
||||
<ProjectInboxStatus
|
||||
message={
|
||||
error
|
||||
? "Could not load this project item."
|
||||
: isLoading
|
||||
? "Loading project item…"
|
||||
: "This project item could not be found."
|
||||
}
|
||||
onBack={onBack}
|
||||
onRetry={
|
||||
error
|
||||
? () => {
|
||||
void projectsQuery.refetch();
|
||||
void projectsWorkItemsQuery.refetch();
|
||||
}
|
||||
: undefined
|
||||
}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
const failedSections =
|
||||
workItem.type === "pull-request"
|
||||
? projectsWorkItemsQuery.data?.pullRequests.failedSections
|
||||
: projectsWorkItemsQuery.data?.issues.failedSections;
|
||||
if (failedSections && failedSections.length > 0) {
|
||||
return (
|
||||
<ProjectInboxStatus
|
||||
message="Some project activity could not be loaded. Actions are unavailable until the item is current."
|
||||
onBack={onBack}
|
||||
onRetry={() => void projectsWorkItemsQuery.refetch()}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<ProjectInboxDetailPane
|
||||
isSinglePanelView={isSinglePanelView}
|
||||
onBack={onBack}
|
||||
onOpenProject={() => {
|
||||
const workItemId =
|
||||
workItem.type === "pull-request"
|
||||
? { pullRequestId: workItem.pullRequest.id }
|
||||
: { issueId: workItem.issue.id };
|
||||
void goProject(workItem.project.id, workItemId);
|
||||
}}
|
||||
profiles={profiles}
|
||||
workItem={workItem}
|
||||
/>
|
||||
);
|
||||
}
|
||||
@@ -0,0 +1,178 @@
|
||||
import { ArrowLeft, ExternalLink } from "lucide-react";
|
||||
import * as React from "react";
|
||||
|
||||
import { useCommunities } from "@/features/communities/useCommunities";
|
||||
import type { ProjectInboxWorkItem } from "@/features/home/lib/projectInbox";
|
||||
import { ProjectIssueDetail } from "@/features/projects/ui/ProjectIssuesPanel";
|
||||
import {
|
||||
ProjectPullRequestDetail,
|
||||
PullRequestDetailHeader,
|
||||
PullRequestMetaRail,
|
||||
} from "@/features/projects/ui/ProjectPullRequestsPanel";
|
||||
import {
|
||||
resolveUserLabel,
|
||||
type UserProfileLookup,
|
||||
} from "@/features/profile/lib/identity";
|
||||
import { openProjectMergeRecoveryTerminal } from "@/shared/api/projectGit";
|
||||
import { useElementWidth } from "@/shared/hooks/use-mobile";
|
||||
import { TopChromeInsetHeader } from "@/shared/layout/TopChromeInsetHeader";
|
||||
import { cn } from "@/shared/lib/cn";
|
||||
import { normalizePubkey } from "@/shared/lib/pubkey";
|
||||
import { Button } from "@/shared/ui/button";
|
||||
import { UserAvatar } from "@/shared/ui/UserAvatar";
|
||||
|
||||
type ProjectInboxDetailPaneProps = {
|
||||
isSinglePanelView?: boolean;
|
||||
onBack?: () => void;
|
||||
onOpenProject: () => void;
|
||||
profiles?: UserProfileLookup;
|
||||
workItem: ProjectInboxWorkItem;
|
||||
};
|
||||
|
||||
/** Renders a canonical Buzz Git work item with its existing project actions. */
|
||||
export function ProjectInboxDetailPane({
|
||||
isSinglePanelView = false,
|
||||
onBack,
|
||||
onOpenProject,
|
||||
profiles,
|
||||
workItem,
|
||||
}: ProjectInboxDetailPaneProps) {
|
||||
const { activeCommunity } = useCommunities();
|
||||
const [detailContentRef, detailContentWidth] =
|
||||
useElementWidth<HTMLDivElement>();
|
||||
const showSideRail = detailContentWidth >= 760;
|
||||
const authorPubkey =
|
||||
workItem.type === "pull-request"
|
||||
? workItem.pullRequest.author
|
||||
: workItem.issue.author;
|
||||
const authorLabel = resolveUserLabel({ profiles, pubkey: authorPubkey });
|
||||
const authorAvatarUrl =
|
||||
profiles?.[normalizePubkey(authorPubkey)]?.avatarUrl ?? null;
|
||||
const inboxTitle = `${authorLabel} sent you ${
|
||||
workItem.type === "pull-request" ? "a pull request" : "an issue"
|
||||
}`;
|
||||
const handleOpenMergeRecoveryTerminal = React.useCallback(
|
||||
async (input: {
|
||||
expectedCommit: string;
|
||||
sourceBranch: string;
|
||||
sourceCloneUrl: string;
|
||||
targetBranch: string;
|
||||
}) => {
|
||||
if (workItem.type !== "pull-request") {
|
||||
throw new Error("Merge recovery is only available for pull requests.");
|
||||
}
|
||||
const targetCloneUrl = workItem.project.cloneUrls[0];
|
||||
if (!targetCloneUrl) {
|
||||
throw new Error("This project has no clone URL.");
|
||||
}
|
||||
return openProjectMergeRecoveryTerminal({
|
||||
...input,
|
||||
projectDtag: workItem.project.dtag,
|
||||
reposDir: activeCommunity?.reposDir,
|
||||
targetCloneUrl,
|
||||
});
|
||||
},
|
||||
[activeCommunity?.reposDir, workItem],
|
||||
);
|
||||
|
||||
return (
|
||||
<section
|
||||
className="flex min-h-0 min-w-0 flex-col overflow-hidden bg-background/60"
|
||||
data-testid="home-project-inbox-detail"
|
||||
>
|
||||
<TopChromeInsetHeader flush transparent>
|
||||
<div className="px-5 py-2">
|
||||
<div className="flex min-h-9 min-w-0 items-center justify-between gap-3">
|
||||
<div className="flex min-w-0 items-center gap-1">
|
||||
{isSinglePanelView && onBack ? (
|
||||
<Button
|
||||
aria-label="Back to Inbox"
|
||||
onClick={onBack}
|
||||
size="icon"
|
||||
type="button"
|
||||
variant="ghost"
|
||||
>
|
||||
<ArrowLeft className="h-4 w-4" />
|
||||
</Button>
|
||||
) : null}
|
||||
<UserAvatar
|
||||
avatarUrl={authorAvatarUrl}
|
||||
className="shrink-0"
|
||||
displayName={authorLabel}
|
||||
size="sm"
|
||||
testId="project-inbox-author-avatar"
|
||||
/>
|
||||
<h2
|
||||
className="min-w-0 translate-y-px truncate text-sm font-semibold leading-5 tracking-tight text-foreground"
|
||||
title={`${inboxTitle} · ${workItem.project.name}`}
|
||||
>
|
||||
{inboxTitle}
|
||||
</h2>
|
||||
</div>
|
||||
<Button
|
||||
aria-label="Open project"
|
||||
className="shrink-0"
|
||||
onClick={onOpenProject}
|
||||
size={showSideRail ? "sm" : "icon"}
|
||||
title="Open project"
|
||||
type="button"
|
||||
variant="ghost"
|
||||
>
|
||||
<ExternalLink className="h-4 w-4" />
|
||||
{showSideRail ? "Open project" : null}
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
</TopChromeInsetHeader>
|
||||
|
||||
<div
|
||||
className="min-h-0 flex-1 overflow-y-auto overscroll-contain"
|
||||
ref={detailContentRef}
|
||||
>
|
||||
<div className="p-3">
|
||||
<div
|
||||
className="overflow-hidden rounded-xl border border-border/60 bg-card"
|
||||
data-testid="project-inbox-work-item-card"
|
||||
>
|
||||
{workItem.type === "pull-request" ? (
|
||||
<div
|
||||
className={cn(
|
||||
"grid",
|
||||
showSideRail && "grid-cols-[minmax(0,1fr)_18rem]",
|
||||
)}
|
||||
data-testid="project-inbox-work-item-layout"
|
||||
>
|
||||
<div className="min-w-0">
|
||||
<PullRequestDetailHeader
|
||||
profiles={profiles}
|
||||
pullRequest={workItem.pullRequest}
|
||||
/>
|
||||
<ProjectPullRequestDetail
|
||||
mode="conversation"
|
||||
onOpenTerminal={handleOpenMergeRecoveryTerminal}
|
||||
profiles={profiles}
|
||||
project={workItem.project}
|
||||
pullRequest={workItem.pullRequest}
|
||||
/>
|
||||
</div>
|
||||
<PullRequestMetaRail
|
||||
profiles={profiles}
|
||||
project={workItem.project}
|
||||
pullRequest={workItem.pullRequest}
|
||||
stacked={!showSideRail}
|
||||
/>
|
||||
</div>
|
||||
) : (
|
||||
<ProjectIssueDetail
|
||||
issue={workItem.issue}
|
||||
profiles={profiles}
|
||||
project={workItem.project}
|
||||
stackMetaRail={!showSideRail}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</section>
|
||||
);
|
||||
}
|
||||
@@ -171,7 +171,11 @@ export function MachineOnboardingFlow({
|
||||
onClick={() => void loadFreshIdentity()}
|
||||
type="button"
|
||||
>
|
||||
{isPending ? "Saving identity…" : "Create a new identity key"}
|
||||
{isPending
|
||||
? "Loading identity…"
|
||||
: selectedPubkey
|
||||
? "Continue setup"
|
||||
: "Create a new identity key"}
|
||||
</Button>
|
||||
<Button
|
||||
className="h-9 rounded-full bg-foreground/10 px-5 hover:bg-foreground/15"
|
||||
@@ -180,7 +184,9 @@ export function MachineOnboardingFlow({
|
||||
type="button"
|
||||
variant="ghost"
|
||||
>
|
||||
Use an existing key
|
||||
{selectedPubkey
|
||||
? "Use a different key instead"
|
||||
: "Use an existing key"}
|
||||
</Button>
|
||||
</div>
|
||||
<IdentityKeyHelpDialog />
|
||||
|
||||
@@ -7,6 +7,7 @@ import type * as React from "react";
|
||||
* trailing cluster (hash, id, comment count) on the right.
|
||||
*/
|
||||
export function ProjectFeedRow({
|
||||
eventId,
|
||||
meta,
|
||||
onOpen,
|
||||
statusIcon,
|
||||
@@ -14,6 +15,7 @@ export function ProjectFeedRow({
|
||||
title,
|
||||
trailing,
|
||||
}: {
|
||||
eventId?: string;
|
||||
meta: React.ReactNode;
|
||||
onOpen?: () => void;
|
||||
statusIcon?: React.ReactNode;
|
||||
@@ -24,6 +26,7 @@ export function ProjectFeedRow({
|
||||
return (
|
||||
<article
|
||||
className="group/feed-item flex min-w-0 items-center justify-between gap-3 p-3 transition-colors hover:bg-muted/35"
|
||||
data-project-event-id={eventId}
|
||||
data-testid={testId}
|
||||
>
|
||||
<div className="min-w-0 flex-1 space-y-1">
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
} from "@/features/profile/lib/identity";
|
||||
import { relativeTime } from "@/features/projects/lib/projectsViewHelpers";
|
||||
import type { ChannelMember } from "@/shared/api/types";
|
||||
import { cn } from "@/shared/lib/cn";
|
||||
import { normalizePubkey } from "@/shared/lib/pubkey";
|
||||
import {
|
||||
ProjectFeedRow,
|
||||
@@ -158,14 +159,17 @@ function IssueRow({
|
||||
);
|
||||
}
|
||||
|
||||
function IssueDetail({
|
||||
/** Full issue conversation and comment composer. */
|
||||
export function ProjectIssueDetail({
|
||||
issue,
|
||||
profiles,
|
||||
project,
|
||||
stackMetaRail = false,
|
||||
}: {
|
||||
issue: ProjectIssue;
|
||||
profiles?: UserProfileLookup;
|
||||
project: Project;
|
||||
stackMetaRail?: boolean;
|
||||
}) {
|
||||
const commentMutation = useCreateProjectIssueCommentMutation(project);
|
||||
const authorLabel = resolveUserLabel({ profiles, pubkey: issue.author });
|
||||
@@ -198,7 +202,12 @@ function IssueDetail({
|
||||
);
|
||||
|
||||
return (
|
||||
<div className="grid xl:grid-cols-[minmax(0,1fr)_18rem]">
|
||||
<div
|
||||
className={cn(
|
||||
"grid",
|
||||
!stackMetaRail && "xl:grid-cols-[minmax(0,1fr)_18rem]",
|
||||
)}
|
||||
>
|
||||
<div className="min-w-0 divide-y divide-border/50">
|
||||
<header className="space-y-3 p-4">
|
||||
<div className="min-w-0">
|
||||
@@ -253,7 +262,11 @@ function IssueDetail({
|
||||
</section>
|
||||
</div>
|
||||
|
||||
<IssueMetaRail issue={issue} profiles={profiles} />
|
||||
<IssueMetaRail
|
||||
issue={issue}
|
||||
profiles={profiles}
|
||||
stacked={stackMetaRail}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -263,16 +276,23 @@ function IssueDetail({
|
||||
function IssueMetaRail({
|
||||
issue,
|
||||
profiles,
|
||||
stacked = false,
|
||||
}: {
|
||||
issue: ProjectIssue;
|
||||
profiles?: UserProfileLookup;
|
||||
stacked?: boolean;
|
||||
}) {
|
||||
const authorProfile = profiles?.[normalizePubkey(issue.author)];
|
||||
const authorLabel = resolveUserLabel({ profiles, pubkey: issue.author });
|
||||
const status = issueStatusVisual(issue.status);
|
||||
|
||||
return (
|
||||
<aside className="space-y-6 border-t border-border/60 p-4 xl:border-l xl:border-t-0">
|
||||
<aside
|
||||
className={cn(
|
||||
"space-y-6 border-border/60 p-4",
|
||||
stacked ? "border-t" : "border-t xl:border-l xl:border-t-0",
|
||||
)}
|
||||
>
|
||||
<OverviewRailSection title="Status">
|
||||
<span
|
||||
className={`inline-flex items-center gap-1.5 rounded-md border border-border/60 px-2.5 py-1 text-xs font-medium ${status.className}`}
|
||||
@@ -357,7 +377,7 @@ export function ProjectIssuesPanel({
|
||||
|
||||
if (selectedIssue) {
|
||||
return (
|
||||
<IssueDetail
|
||||
<ProjectIssueDetail
|
||||
issue={selectedIssue}
|
||||
profiles={profiles}
|
||||
project={project}
|
||||
|
||||
@@ -35,6 +35,7 @@ import { canReviewProjectPullRequest } from "@/features/projects/pullRequestRevi
|
||||
import type { UserProfileLookup } from "@/features/profile/lib/identity";
|
||||
import { useIdentityQuery } from "@/shared/api/hooks";
|
||||
import type { ChannelMember } from "@/shared/api/types";
|
||||
import { cn } from "@/shared/lib/cn";
|
||||
import { normalizePubkey, truncatePubkey } from "@/shared/lib/pubkey";
|
||||
import {
|
||||
ProjectFeedRow,
|
||||
@@ -252,6 +253,7 @@ function PullRequestRow({
|
||||
|
||||
return (
|
||||
<ProjectFeedRow
|
||||
eventId={pullRequest.id}
|
||||
meta={
|
||||
<>
|
||||
<ProfileIdentityButton
|
||||
@@ -393,10 +395,12 @@ export function PullRequestMetaRail({
|
||||
profiles,
|
||||
project,
|
||||
pullRequest,
|
||||
stacked = false,
|
||||
}: {
|
||||
profiles?: UserProfileLookup;
|
||||
project: Project;
|
||||
pullRequest: ProjectPullRequest;
|
||||
stacked?: boolean;
|
||||
}) {
|
||||
const identityQuery = useIdentityQuery();
|
||||
const authorProfile = profileForPubkey(pullRequest.author, profiles);
|
||||
@@ -414,7 +418,12 @@ export function PullRequestMetaRail({
|
||||
Boolean(viewer) && (isAuthor || isOwner || isManagedAgentOwner);
|
||||
|
||||
return (
|
||||
<aside className="min-w-0 space-y-6 border-t border-border/60 p-4 xl:border-l xl:border-t-0">
|
||||
<aside
|
||||
className={cn(
|
||||
"min-w-0 space-y-6 border-border/60 p-4",
|
||||
stacked ? "border-t" : "border-t xl:border-l xl:border-t-0",
|
||||
)}
|
||||
>
|
||||
<OverviewRailSection title="Status">
|
||||
<span
|
||||
className={`inline-flex items-center gap-1.5 rounded-md px-2.5 py-1 text-xs font-medium text-white ${pullRequestStatusBadgeClassName(pullRequest.status)}`}
|
||||
@@ -488,7 +497,8 @@ export function PullRequestMetaRail({
|
||||
);
|
||||
}
|
||||
|
||||
function PullRequestDetail({
|
||||
/** Full pull-request conversation, review actions, and comment composer. */
|
||||
export function ProjectPullRequestDetail({
|
||||
mode,
|
||||
onOpenInlineComment,
|
||||
onOpenCommit,
|
||||
@@ -951,7 +961,7 @@ export function PullRequestsPanel({
|
||||
|
||||
if (selectedPullRequest) {
|
||||
return (
|
||||
<PullRequestDetail
|
||||
<ProjectPullRequestDetail
|
||||
mode={mode}
|
||||
onOpenInlineComment={onOpenInlineComment}
|
||||
onOpenCommit={onOpenCommit}
|
||||
|
||||
@@ -6829,7 +6829,24 @@ async function handleGetFeed(
|
||||
// For e2e, return a minimal feed structure with mentions.
|
||||
const limit = args.limit ?? 50;
|
||||
const mentionEvents = await relayQuery(config, [
|
||||
{ kinds: [9, 40002, 45001, 45003], "#p": [identity.pubkey], limit },
|
||||
{
|
||||
kinds: [
|
||||
9,
|
||||
40002,
|
||||
1,
|
||||
45001,
|
||||
45003,
|
||||
KIND_GIT_PULL_REQUEST,
|
||||
KIND_GIT_PR_UPDATE,
|
||||
KIND_GIT_ISSUE,
|
||||
KIND_GIT_STATUS_OPEN,
|
||||
KIND_GIT_STATUS_MERGED,
|
||||
KIND_GIT_STATUS_CLOSED,
|
||||
KIND_GIT_STATUS_DRAFT,
|
||||
],
|
||||
"#p": [identity.pubkey],
|
||||
limit,
|
||||
},
|
||||
]);
|
||||
|
||||
// Look up channel names for feed items
|
||||
|
||||
@@ -85,8 +85,13 @@ test("backup step back button returns to machine identity choice", async ({
|
||||
await expect(page.getByTestId("onboarding-page-backup")).toBeVisible();
|
||||
await page.getByTestId("onboarding-back").click();
|
||||
|
||||
// Backing out preserves the loaded key — primary CTA continues setup rather
|
||||
// than minting another identity (#2318).
|
||||
await expect(
|
||||
page.getByRole("button", { name: "Create a new identity key" }),
|
||||
page.getByRole("button", { name: "Continue setup" }),
|
||||
).toBeVisible();
|
||||
await expect(
|
||||
page.getByRole("button", { name: "Use a different key instead" }),
|
||||
).toBeVisible();
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,117 @@
|
||||
import { expect, test } from "@playwright/test";
|
||||
|
||||
import { waitForAnimations } from "../helpers/animations";
|
||||
import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge";
|
||||
|
||||
const DEFAULT_MOCK_PUBKEY = "deadbeef".repeat(8);
|
||||
const BUZZ_REPO_ADDRESS = `30617:${DEFAULT_MOCK_PUBKEY}:buzz`;
|
||||
|
||||
test("Buzz Git pull request renders and stays actionable in Inbox", async ({
|
||||
page,
|
||||
}) => {
|
||||
await page.addInitScript(() => {
|
||||
window.localStorage.setItem(
|
||||
"buzz-feature-overrides-v1",
|
||||
JSON.stringify({ projects: true }),
|
||||
);
|
||||
});
|
||||
await installMockBridge(page);
|
||||
await page.setViewportSize({ width: 1024, height: 720 });
|
||||
|
||||
await page.goto("/", { waitUntil: "domcontentloaded" });
|
||||
await page.getByTestId("open-projects-view").click();
|
||||
await page.getByRole("button", { name: "Repositories", exact: true }).click();
|
||||
await page
|
||||
.locator(
|
||||
'[data-testid="project-card-buzz"], [data-testid="project-row-buzz"]',
|
||||
)
|
||||
.first()
|
||||
.click();
|
||||
await page.getByRole("tab", { name: "Pull Request" }).click();
|
||||
|
||||
const alicePullRequest = page
|
||||
.getByTestId("project-pull-request-row")
|
||||
.filter({ hasText: "alice" })
|
||||
.first();
|
||||
await expect(alicePullRequest).toBeVisible({ timeout: 10_000 });
|
||||
const pullRequestId = await alicePullRequest.getAttribute(
|
||||
"data-project-event-id",
|
||||
);
|
||||
expect(pullRequestId).toBeTruthy();
|
||||
|
||||
await page.getByRole("button", { name: "Inbox", exact: true }).click();
|
||||
await page.evaluate(
|
||||
({ author, id, repoAddress, viewer }) => {
|
||||
window.__BUZZ_E2E_PUSH_MOCK_FEED_ITEM__?.({
|
||||
id,
|
||||
kind: 1618,
|
||||
pubkey: author,
|
||||
content: "Inbox rendering verification",
|
||||
created_at: Math.floor(Date.now() / 1000) + 1,
|
||||
channel_id: null,
|
||||
channel_name: "",
|
||||
channel_type: null,
|
||||
tags: [
|
||||
["a", repoAddress],
|
||||
["p", viewer],
|
||||
["subject", "Inbox rendering verification"],
|
||||
],
|
||||
category: "mention",
|
||||
});
|
||||
},
|
||||
{
|
||||
author: TEST_IDENTITIES.alice.pubkey,
|
||||
id: pullRequestId as string,
|
||||
repoAddress: BUZZ_REPO_ADDRESS,
|
||||
viewer: DEFAULT_MOCK_PUBKEY,
|
||||
},
|
||||
);
|
||||
|
||||
const inboxRow = page.getByTestId(`home-inbox-item-${pullRequestId}`);
|
||||
await expect(inboxRow).toBeVisible({ timeout: 10_000 });
|
||||
await inboxRow.locator(":scope > div").first().click();
|
||||
|
||||
const detail = page.getByTestId("home-project-inbox-detail");
|
||||
const card = page.getByTestId("project-inbox-work-item-card");
|
||||
const layout = page.getByTestId("project-inbox-work-item-layout");
|
||||
await expect(detail).toBeVisible();
|
||||
await expect(card).toBeVisible();
|
||||
await expect(
|
||||
detail.locator('[data-testid^="project-inbox-author-avatar-"]'),
|
||||
).toBeVisible();
|
||||
await expect(
|
||||
detail.getByRole("heading", {
|
||||
name: "alice sent you a pull request",
|
||||
exact: true,
|
||||
}),
|
||||
).toBeVisible();
|
||||
await expect(page.getByText("Inbox rendering verification")).toBeVisible();
|
||||
await expect(
|
||||
page.getByRole("button", { name: "Approve", exact: true }),
|
||||
).toBeVisible();
|
||||
const commentComposer = page.getByTestId(
|
||||
"project-pull-request-comment-composer",
|
||||
);
|
||||
await commentComposer
|
||||
.getByRole("button", { name: "Comment", exact: true })
|
||||
.click();
|
||||
await expect(
|
||||
page.getByRole("menuitemradio", { name: "Request changes" }),
|
||||
).toBeVisible();
|
||||
await page.keyboard.press("Escape");
|
||||
await expect(
|
||||
page.getByRole("button", { name: "Merge", exact: true }),
|
||||
).toBeVisible();
|
||||
|
||||
const columnCount = await layout.evaluate(
|
||||
(element) =>
|
||||
getComputedStyle(element).gridTemplateColumns.split(" ").filter(Boolean)
|
||||
.length,
|
||||
);
|
||||
expect(columnCount).toBe(1);
|
||||
|
||||
await waitForAnimations(page);
|
||||
await detail.screenshot({
|
||||
path: "test-results/project-inbox/01-pull-request-detail.png",
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user