From c9367123daa0055ae6108edb8d157373e9ef7b5a Mon Sep 17 00:00:00 2001 From: Michael Schmid Date: Mon, 17 Aug 2026 00:08:01 -0400 Subject: [PATCH] =?UTF-8?q?fix(buzz-relay):=20record=20the=20agent?= =?UTF-8?q?=E2=86=92owner=20relationship=20for=20direct=20members?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An agent registered as a relay member in its own right never gets `users.agent_owner_pubkey` written, so every owner-gated feature is silently disabled for it — and nothing anywhere reports why. `check_relay_membership` tests direct membership first and short-circuits on `MembershipDecision::Member`. The owner is only resolved on the `ViaOwner` branch, reached solely by agents that are *not* members and are admitted by NIP-OA delegation; the other backfill path in auth.rs is gated on `!require_relay_membership`. So a closed relay throws away a NIP-OA proof the agent did present, on every single connection, purely because the agent was also granted membership. The visible symptom is Buzz Desktop's per-agent ACP activity tab. It is fed by kind 24200 observer frames, the relay gates those on `users.agent_owner_pubkey`, and with no mapping it rejects all of them: restricted: observer frame is not authorized for this agent owner That rejection is invisible from both sides. The relay only increments `buzz_events_rejected_total{reason="auth"}` and logs nothing at all, while buzz-acp does not surface the `OK=false` — so the agent reports `relay observer enabled`, resolves an owner, looks entirely healthy, and every frame it sends is dropped. The operator sees an empty tab and reasonably concludes the UI is broken. Desktop-managed agents are unaffected, which makes this harder to place: they are minted with an owner rather than added to `relay_members`, so the same agent works when created in the app and fails when run by the harness. Granting an agent membership is what breaks it. This resolves the owner for a direct member too, adding `MembershipDecision::MemberWithOwner`. Membership is still decided exactly as before — the new variant only carries a relationship that was already proved out of the check so it can be recorded. The tag verification and owner-is-member lookup are shared with the delegation path in `resolve_nip_oa_owner`, so the security posture is unchanged: an owner that is not itself a relay member is still not recorded, which matters on a closed relay where that would hand agent-management authority to someone who cannot connect. The extra lookup runs only for callers that present an auth tag, so ordinary client connections do no additional work. Both call sites that consume the owner — `handlers/auth.rs` and `api/bridge.rs` — already pass it straight to `materialize_nip_oa_owner`, so they need no change and the fix reaches every transport at once. The remaining callers discard the value. Signed-off-by: Michael Schmid --- crates/buzz-relay/src/api/mod.rs | 179 ++++++++++++++++++++----- crates/buzz-relay/src/handlers/auth.rs | 3 +- 2 files changed, 147 insertions(+), 35 deletions(-) diff --git a/crates/buzz-relay/src/api/mod.rs b/crates/buzz-relay/src/api/mod.rs index 2a942bc8039..92c696458e0 100644 --- a/crates/buzz-relay/src/api/mod.rs +++ b/crates/buzz-relay/src/api/mod.rs @@ -49,12 +49,33 @@ pub mod relay_members { OpenRelay, /// Caller is directly present in `relay_members`. Member, + /// Caller is directly present in `relay_members` *and* presented a valid + /// NIP-OA auth tag naming an owner that is also a relay member. + /// + /// Distinct from [`Self::ViaOwner`]: membership does not depend on the + /// owner here, it is already established. The relationship is carried + /// out of the check so it can still be recorded. + MemberWithOwner(nostr::PublicKey), /// Caller is admitted through a NIP-OA owner that is a relay member. ViaOwner(nostr::PublicKey), /// Caller is not admitted. Denied, } + impl MembershipDecision { + /// The NIP-OA owner proved during the check, if any. + /// + /// Deliberately independent of *why* the caller was admitted: both a + /// delegated agent and a direct member can prove an owner, and the + /// relationship is worth recording either way. + pub fn nip_oa_owner(&self) -> Option { + match self { + Self::ViaOwner(owner) | Self::MemberWithOwner(owner) => Some(*owner), + Self::OpenRelay | Self::Member | Self::Denied => None, + } + } + } + /// Check relay membership without committing to an HTTP response shape. /// /// `community` is the server-resolved tenant of the request; membership is @@ -75,53 +96,104 @@ pub mod relay_members { .is_relay_member(community, &pubkey_hex) .await .map_err(|e| format!("relay membership check failed: {e}"))?; + // A direct member still has its NIP-OA owner resolved. Membership is + // already settled, but the agent→owner relationship is not, and this is + // the only point where the auth tag is examined. Returning plain + // `Member` here discards a proof the agent did present — so an agent + // registered as a relay member in its own right never gets an owner + // recorded, and every owner-gated feature silently fails for it. + // + // The visible symptom is kind 24200 observer frames: the relay gates + // them on `users.agent_owner_pubkey` and rejects all of them, so Buzz + // Desktop's ACP activity tab stays empty for a working agent while the + // harness reports `relay observer enabled`. Desktop-managed agents are + // unaffected because they are minted with an owner rather than added to + // `relay_members`. if is_member { - return Ok(MembershipDecision::Member); + let owner = + resolve_nip_oa_owner(state, community, pubkey_bytes, auth_tag_header, &pubkey_hex) + .await?; + return Ok(match owner { + Some(owner) => MembershipDecision::MemberWithOwner(owner), + None => MembershipDecision::Member, + }); } - if state.config.allow_nip_oa_auth { - if let Some(tag_json) = auth_tag_header { - let agent_pubkey = nostr::PublicKey::from_slice(pubkey_bytes) - .map_err(|e| format!("invalid agent pubkey for NIP-OA check: {e}"))?; - - match buzz_sdk::nip_oa::verify_auth_tag(tag_json, &agent_pubkey) { - Ok(owner_pubkey) => { - let owner_hex = owner_pubkey.to_hex(); - let owner_is_member = state - .db - .is_relay_member(community, &owner_hex) - .await - .map_err(|e| format!("relay membership check (owner) failed: {e}"))?; - if owner_is_member { - debug!( - agent = %pubkey_hex, - owner = %owner_hex, - "NIP-OA membership granted via owner" - ); - return Ok(MembershipDecision::ViaOwner(owner_pubkey)); - } - } - Err(e) => { - info!(agent = %pubkey_hex, "NIP-OA auth tag invalid: {e}"); - } - } - } + if let Some(owner_pubkey) = + resolve_nip_oa_owner(state, community, pubkey_bytes, auth_tag_header, &pubkey_hex) + .await? + { + debug!( + agent = %pubkey_hex, + owner = %owner_pubkey.to_hex(), + "NIP-OA membership granted via owner" + ); + return Ok(MembershipDecision::ViaOwner(owner_pubkey)); } Ok(MembershipDecision::Denied) } + /// Resolve the NIP-OA owner named by an auth tag. + /// + /// Returns `Ok(None)` — never an error — when NIP-OA auth is disabled, no + /// tag was presented, the tag does not verify, or the named owner is not + /// itself a relay member. Only a malformed caller pubkey or a failed DB + /// lookup is an error. + /// + /// The owner-is-member requirement is inherited from the delegation path + /// rather than newly imposed: on a closed relay, recording an owner that + /// cannot itself connect would hand agent-management authority to a + /// non-member. + async fn resolve_nip_oa_owner( + state: &AppState, + community: CommunityId, + pubkey_bytes: &[u8], + auth_tag_header: Option<&str>, + pubkey_hex: &str, + ) -> Result, String> { + if !state.config.allow_nip_oa_auth { + return Ok(None); + } + // Costs nothing for ordinary clients: only agents send an auth tag, so + // the extra owner lookup below never runs for a human's connection. + let Some(tag_json) = auth_tag_header else { + return Ok(None); + }; + + let agent_pubkey = nostr::PublicKey::from_slice(pubkey_bytes) + .map_err(|e| format!("invalid agent pubkey for NIP-OA check: {e}"))?; + + let owner_pubkey = match buzz_sdk::nip_oa::verify_auth_tag(tag_json, &agent_pubkey) { + Ok(owner_pubkey) => owner_pubkey, + Err(e) => { + info!(agent = %pubkey_hex, "NIP-OA auth tag invalid: {e}"); + return Ok(None); + } + }; + + let owner_is_member = state + .db + .is_relay_member(community, &owner_pubkey.to_hex()) + .await + .map_err(|e| format!("relay membership check (owner) failed: {e}"))?; + + Ok(owner_is_member.then_some(owner_pubkey)) + } + /// Enforce relay membership for a pubkey, with NIP-OA agent delegation fallback. /// - /// Returns `Ok(Some(owner_pubkey))` when the agent is not a direct member but - /// its NIP-OA owner *is* — access is granted via delegation. + /// Returns `Ok(Some(owner_pubkey))` whenever a NIP-OA owner was proved, + /// whether or not membership depended on it: for an agent admitted *by* + /// delegation, and for a direct member that also carries a valid auth tag. + /// Callers use it to record the agent→owner relationship, which is needed + /// in both cases. /// /// On open relays (`require_relay_membership = false`), returns `Ok(None)` /// immediately — no membership check is performed. Callers that need NIP-OA /// owner extraction on open relays should call [`extract_nip_oa_owner`] directly. /// - /// Returns `Ok(None)` when the caller is a direct member (closed relay) or when - /// no NIP-OA tag is present/applicable (open relay without auth tag). + /// Returns `Ok(None)` when no NIP-OA tag is present or applicable. pub async fn enforce_relay_membership( state: &AppState, community: CommunityId, @@ -129,8 +201,12 @@ pub mod relay_members { auth_tag_header: Option<&str>, ) -> Result, (StatusCode, Json)> { match check_relay_membership(state, community, pubkey_bytes, auth_tag_header).await { - Ok(MembershipDecision::OpenRelay) | Ok(MembershipDecision::Member) => Ok(None), - Ok(MembershipDecision::ViaOwner(owner)) => Ok(Some(owner)), + Ok( + decision @ (MembershipDecision::OpenRelay + | MembershipDecision::Member + | MembershipDecision::MemberWithOwner(_) + | MembershipDecision::ViaOwner(_)), + ) => Ok(decision.nip_oa_owner()), Ok(MembershipDecision::Denied) => Err(( StatusCode::FORBIDDEN, Json(serde_json::json!({ @@ -274,5 +350,40 @@ pub mod relay_members { assert_eq!(result, None); } + + /// A direct member that proved an owner must surface it. This is the + /// regression: returning a plain `Member` here drops the relationship, + /// and an agent registered as a relay member in its own right then + /// never gets `users.agent_owner_pubkey` written — which silently + /// disables observer frames and every other owner-gated feature. + #[test] + fn member_with_owner_surfaces_the_owner() { + let owner = Keys::generate().public_key(); + + assert_eq!( + MembershipDecision::MemberWithOwner(owner).nip_oa_owner(), + Some(owner) + ); + } + + /// Delegation keeps surfacing the owner exactly as before. + #[test] + fn via_owner_surfaces_the_owner() { + let owner = Keys::generate().public_key(); + + assert_eq!( + MembershipDecision::ViaOwner(owner).nip_oa_owner(), + Some(owner) + ); + } + + /// Admission without a proved owner surfaces nothing — a member that + /// sent no auth tag must not acquire one. + #[test] + fn admission_without_proof_surfaces_no_owner() { + assert_eq!(MembershipDecision::Member.nip_oa_owner(), None); + assert_eq!(MembershipDecision::OpenRelay.nip_oa_owner(), None); + assert_eq!(MembershipDecision::Denied.nip_oa_owner(), None); + } } } diff --git a/crates/buzz-relay/src/handlers/auth.rs b/crates/buzz-relay/src/handlers/auth.rs index 127f1fc40e0..487ba1a2a8b 100644 --- a/crates/buzz-relay/src/handlers/auth.rs +++ b/crates/buzz-relay/src/handlers/auth.rs @@ -239,7 +239,8 @@ pub async fn handle_auth(event: nostr::Event, conn: Arc, state: // Open relay NIP-OA backfill: extract owner for agent→owner DB mapping // (needed for observer frame auth). Only runs on open relays — on closed - // relays, enforce_relay_membership already handles NIP-OA delegation. + // relays enforce_relay_membership has already resolved the owner, for a + // direct member as well as for an agent admitted by delegation. // No feature flag needed: NIP-OA is cryptographically self-proving. let nip_oa_owner = nip_oa_owner.or_else(|| { if !state.config.require_relay_membership && auth_tag_json.is_some() {