buzz-acp: owner-only sibling gate caches a transport failure as not-a-sibling, silently blacklisting a teammate bot until restart
- Dominant language
- Rust
- Stars
- 32.7k
- Forks
- 4.3k
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 253
Description
# buzz-acp: owner-only sibling gate caches a transport failure as "not a sibling", permanently blacklisting a teammate bot until process restart
## Summary
In `owner-only` respond mode, an inbound event from a **sibling** agent (a bot proving the same owner via its NIP-OA auth tag) is validated by querying the author's `kind:0` profile over REST. When that `/query` times out or errors, the check **fails closed** and treats the author as not-a-sibling. That **negative result is cached for the process lifetime**, so a single transient network blip permanently blacklists a teammate bot: every subsequent message it sends is silently dropped by the inbound author gate until the agent process is restarted.
This is observable as "a bot answers my (owner) messages instantly but ignores all teammate-bot messages for hours," then starts working again after a restart.
## Repro / evidence
Setup: multiple managed agents on one box, all `owner-only` respond mode, all owned by the same owner (so they are siblings to each other). A teammate bot triggers the agent (e.g. `@` in the channel).
1. Agent process restarts; at ~13s after launch it logs a `POST /query network error` (transient).
2. From that point the agent silently ignores every subsequent event from that sibling bot (8.5h in the observed case), while remaining fully responsive to the owner.
3. Owner messages still wake it instantly because they take the `author == my_owner` short-circuit and never touch REST/cache.
4. Restarting the agent process clears the blacklist (the sibling is immediately accepted again).
## Root cause (with source refs)
All in `block/buzz` `crates/buzz-acp/src/lib.rs`.
- **`owner-only` reviews siblings**: `RespondTo::OwnerOnly => is_owner_or_sibling(author, owner_cache, rest_client)` — and `is_owner_or_sibling` calls `check_sibling_via_profile` for an unknown author. (search `is_owner_or_sibling` + `RespondTo::OwnerOnly`)
- **Fail-closed**: `check_sibling_via_profile` returns `false` on any `/query` timeout or error:
```rust
let resp = match tokio::time::timeout(..., rest_client.query(&[filter])).await {
Ok(Ok(v)) => v,
_ => return false, // timeout or error — fail closed
};
```
- **Negative result is cached unconditionally, treating a transport error as an attestation**:
```rust
let is_sibling = check_sibling_via_profile(author, my_owner, rest_client).await;
owner_cache.cache_sibling(author.to_string(), is_sibling); // cached regardless of success/failure
```
and `is_known_sibling` returns the cached value (including `false`) on all later calls. `OwnerCache` documents the cache as process-lifetime with **no TTL** (`attestations are immutable`, capped only by a 256-entry wholesale clear).
- **The drop is silent**: the gate drops the event with
```rust
tracing::debug!("inbound author gate — dropping event");
```
which is invisible at default INFO level.
A transport error / timeout is **not** an attestation. Caching it as a permanent "not a sibling" conflates "we could not verify" (transient, should be retried) with "verified not a sibling" (durable). The comment claiming `attestations are immutable` is the mismatch: only *successful* verification results are attestations.
## Suggested fix
1. In `is_owner_or_sibling`, write to `OwnerCache` only when the sibling verification **succeeded**; on a transport failure, do **not** cache (or use a short TTL), so the next message from the same author retries the check instead of being dropped forever.
2. Upgrade the gate's drop log from `debug!` to `warn!` so these drops are observable.
3. Add a regression test: after a `/query` network failure for an author, the *next* message from the same author re-runs verification rather than short-circuiting to the cached negative.
## Workaround
Broad enough: in the affected agent, use `allowlist` respond mode with the teammate pubkeys added (allowlist lookup is an in-process set check, no REST), or restart the agent process to clear the stale cached negative.
Contributor guide
Research direction
Start in crates/buzz-acp/src/lib.rs by tracing RespondTo::OwnerOnly through is_owner_or_sibling, check_sibling_via_profile, and OwnerCache. Reproduce the failed /query path, then add the regression test described in the issue: the next message from that author retries verification instead of using a cached negative. Also verify that inbound drops are visible at warn level.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- authentication, backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100