Skip to content

Scope relay runtime state by community#1658

Open
jmecom wants to merge 1 commit into
mainfrom
jm/fix-cross-tenant-runtime-state
Open

Scope relay runtime state by community#1658
jmecom wants to merge 1 commit into
mainfrom
jm/fix-cross-tenant-runtime-state

Conversation

@jmecom

@jmecom jmecom commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

What changed

This changes the relay's in-memory huddle room registry and observer owner cache to include community_id in their keys.

AudioRoomManager now keys rooms by (community_id, channel_id), and the audio connection handler passes the resolved tenant community through every room lookup and cleanup path. Two communities that reuse the same channel UUID no longer share peer lists, protocol pins, or audio frames.

The observer owner cache now keys (community_id, agent_pubkey, owner_pubkey) for both event-path lookups and NIP-OA prewarming. A cached allow in one community can no longer authorize observer traffic in another community for the same agent/owner pair.

Why

Both structures were process-global while their previous keys only named community-local identifiers. That meant a local two-tenant collision could cross the host-derived community boundary after authentication had already succeeded.

An attacker needs two things:

  • visibility of the victim huddle UUID
  • an account in any other community on the same relay

The attack is:

  1. The attacker learns the victim huddle's ephemeral UUID from the kind:48100 huddle-started event in the victim's parent channel.
  2. The attacker goes to their own community and creates a channel with that exact same UUID using a signed kind:9007 event.
  3. The attacker joins /huddle//audio in their own community with their own valid credentials.
  4. The relay authenticates them against their own community and confirms they are allowed to join their own channel.
  5. Because the relay's in-memory huddle registry was keyed only by UUID, the attacker's room and the victim's room collapse into the same live room.
  6. The attacker is now effectively inside the victim huddle: they see the victim participants, receive victim audio frames, and can inject audio into the victim huddle.

Safety and tradeoffs

This does not change wire formats or database schema. The only runtime cost is carrying one additional CommunityId in the room and cache keys, which is required to preserve tenant isolation.

Testing

  • cargo test -p buzz-relay
  • cargo fmt -p buzz-relay --check
  • cargo clippy -p buzz-relay --all-targets -- -D warnings
  • git diff --check

@jmecom jmecom changed the title [codex] Scope relay runtime state by community Scope relay runtime state by community Jul 8, 2026
@jmecom jmecom marked this pull request as ready for review July 8, 2026 21:01
@jmecom jmecom changed the base branch from jm/acp-author-gate-diagnostics to main July 8, 2026 21:02

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review on behalf of Wes (posted by Pinky, agent).

The relay fix itself (f4acc0e9e) — excellent, would approve alone

  • Real cross-tenant confidentiality bug: an attacker who learns a victim huddle UUID (kind:48100 event) can recreate it in their own community and land in the victim's live room. Re-keying AudioRoomManager to (CommunityId, Uuid) and threading tenant.community() through get_or_create and every cleanup_if_empty path is the minimal causal fix, sourced from the already-resolved tenant — no new trust surface.
  • observer_owner_cache re-key to (CommunityId, agent, owner) covers all three touchpoints (event-path get/insert, NIP-OA prewarm). I grepped for unscoped stragglers; found none.
  • Both vulns get real regression tests (same-UUID two-community room isolation; cached allow in A not bypassing B's deny, asserting the restricted OK response). Nice work — this should land fast.

Requesting changes: 2 of 3 commits don't belong in this PR

  1. a80920229 (CLI inbox/TUI + ATTACK_PLAN.md, +4,076 lines) is dead code. inbox.rs and tui.rs are not declared in commands/mod.rs, and they import ratatui/crossterm which aren't in buzz-cli's Cargo.toml — they never compile, so CI green tells us nothing about them. ATTACK_PLAN.md at repo root reads as session scratch from the security sweep. Please drop this commit.

  2. 58a2849d8 (ACP author gate diagnostics, +935 lines) is live behavior change to a different subsystem. The AuthorGateDecision refactor + reason logging looks genuinely useful, and the base-prompt "Work Intake" section is plausibly valuable prompt-injection hardening — but base-prompt changes affect every deployed agent and deserve review on their own merits, not riding under a relay-tenancy title. Please move to its own PR.

  3. FOUND_VULNS.md at repo root is a deliberate policy call — a public vuln write-up with repro shape in-tree vs. an advisory/internal doc. Fine if intentional, but worth deciding explicitly rather than by drive-by.

Suggested path: rebase this PR to just f4acc0e9e (it merges clean and is CI-covered), and open a separate PR for the ACP gate diagnostics. Happy to re-review immediately — the fix itself is ready.

@jmecom jmecom force-pushed the jm/fix-cross-tenant-runtime-state branch from f4acc0e to 00a7698 Compare July 8, 2026 21:36

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review on behalf of Wes (posted by Pinky, agent).

All requested changes addressed: the PR is now the single relay-tenancy commit touching only crates/buzz-relay (+173/−24) — dead CLI code, lab scripts, ATTACK_PLAN.md, FOUND_VULNS.md, and the ACP gate-diagnostics work are all gone.

I verified the new head's relay diff is line-for-line identical to the previously reviewed fix commit (f4acc0e9e), which passed full CI. The rekeying of AudioRoomManager to (CommunityId, Uuid) and observer_owner_cache to (CommunityId, agent, owner) covers every lookup/insert/cleanup site, with regression tests for both vulns. Approving — merge once the fresh CI run is green. Thanks for the fast turnaround; this is an important fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants