From 036e3eee303b596f4088f76d44796d896adf76f8 Mon Sep 17 00:00:00 2001 From: npub1jmc9dt2lyvzu3h0kxlwxt5zg4fxp9476awyxw6gwxn72g6cw7exqs64whm <96f056ad5f2305c8ddf637dc65d048aa4c12d7daeb8867690e34fca46b0ef64c@sprout-oss.stage.blox.sqprod.co> Date: Sat, 27 Jun 2026 00:04:40 -0400 Subject: [PATCH] feat(relay): emit AuthCheck on REQ membership decision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire the conformance read seam at `req.rs` channel-membership confirmation. When the relay falls through to the DB-uncached membership check, record one `AuthCheck` step on the tracer mapping `is_member` → `Allow`/`Deny`. Design notes: - `trace_state` is built once at request entry, after `pubkey_bytes` is available. Reused by every downstream emit (matches ingest's `state_for_request` discipline). The `Option` only goes `None` on malformed pubkey bytes — a separate failure path. - `claimed_community: None` is the load-bearing choice on the read path: the REQ wire has NO client-asserted community (the `h` filter is a channel-id, not a community-id). Encoding `None` here rather than copying the resolved community means a future regression that ever starts reading a wire-community on REQ would need to put a real value in the field — that surfaces at code-review time instead of silently projecting away the M2 (claim ≠ resolved) bite. - No `EmitGuard` at REQ entry: read paths legitimately skip the DB on cache hit (no `is_member` call), so a coverage-breach guard would false-positive on the common case. Coverage for the read seam comes from the upcoming row-emit fixtures, not from a guard at the entry point. Implementation: - New `crate::conformance::record_req_authcheck` helper centralises the emit so the call site stays one line and the helper carries the design rationale in its doc comment. - Two unit tests pin the verdict mapping table: `record_req_authcheck_emits_allow_with_none_claim_when_member` and `record_req_authcheck_emits_deny_when_not_member`. Mutate→red→ restore verified: inverting the `if member` branches reds both tests with explicit panic messages ("member=true must map to Allow", "member=false must map to Deny"); restored both green. Test surfaces: - `cargo test -p buzz-relay --lib` → 389/0 (was 387 baseline). - `cargo test -p buzz-conformance` → 14/0 unchanged. - `cargo clippy -p buzz-relay --all-targets -- -D warnings` clean. - `cargo fmt --all -- --check` clean. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- crates/buzz-relay/src/conformance/mod.rs | 106 +++++++++++++++++++++++ crates/buzz-relay/src/handlers/req.rs | 20 ++++- 2 files changed, 125 insertions(+), 1 deletion(-) diff --git a/crates/buzz-relay/src/conformance/mod.rs b/crates/buzz-relay/src/conformance/mod.rs index 7256e5608..f53f70de6 100644 --- a/crates/buzz-relay/src/conformance/mod.rs +++ b/crates/buzz-relay/src/conformance/mod.rs @@ -128,6 +128,43 @@ pub fn emit(tracer: &Arc, action: TraceAction, state: AbstractState) tracer.record(TraceStep::new(action, state)); } +/// Record an [`TraceAction::AuthCheck`] step for a REQ-path membership +/// decision. Callers pass the already-computed [`AbstractState`] for the +/// request (built once at entry, see [`state_for_request`]) so the +/// emit stays cheap on the hot path. +/// +/// `claimed_community` is unconditionally `None` on the read path: the +/// REQ wire has NO client-asserted community. The `h` filter carries +/// a channel-id, not a community-id; tenant is host-resolved via +/// `TenantContext`. Encoding `None` here (rather than copying the +/// resolved community) is load-bearing — if a future regression ever +/// starts reading a wire-community on REQ, the field would need a real +/// value and that surfaces at code-review time instead of silently +/// projecting away the M2 (claim ≠ resolved) bite. +/// +/// The verdict mapping is `member → Allow`, `!member → Deny`, matching +/// the relay's actual access decision at the membership-cache call +/// site. +pub fn record_req_authcheck( + tracer: &Arc, + state: &AbstractState, + channel_id: Uuid, + member: bool, +) { + tracer.record(TraceStep::new( + TraceAction::AuthCheck { + channel: channel_label(channel_id), + claimed_community: None, + verdict: if member { + Verdict::Allow + } else { + Verdict::Deny + }, + }, + state.clone(), + )); +} + /// RAII coverage-breach guard. Constructed at the top of any critical /// seam (currently: `ingest_event`); the guard observes a [`Tracer`] /// wrapper that counts emits. If the seam exits without any emit @@ -319,4 +356,73 @@ mod tests { other => panic!("expected ImplBug action, got {other:?}"), } } + + /// Verify `record_req_authcheck` lands exactly one `AuthCheck` step + /// on the tracer with the expected channel label and `Allow` verdict + /// when the membership check returned true, and that + /// `claimed_community` is `None` (load-bearing on the read path — + /// see the helper's doc comment). + #[test] + fn record_req_authcheck_emits_allow_with_none_claim_when_member() { + let typed = Arc::new(VecTracer::default()); + let inner: Arc = typed.clone(); + let state = dummy_state(); + let ch_id = Uuid::from_u128(0xCAFE_F00D); + + record_req_authcheck(&inner, &state, ch_id, true); + + let steps = typed.steps.lock().expect("vec tracer mutex"); + assert_eq!(steps.len(), 1, "exactly one AuthCheck step"); + match &steps[0].action { + TraceAction::AuthCheck { + channel, + claimed_community, + verdict, + } => { + assert_eq!( + channel, + &channel_label(ch_id), + "channel must come from the helper's `channel_id` arg" + ); + assert!( + claimed_community.is_none(), + "REQ path has no wire-community claim — must be None" + ); + assert!( + matches!(verdict, Verdict::Allow), + "member=true must map to Allow" + ); + } + other => panic!("expected AuthCheck action, got {other:?}"), + } + assert_eq!( + steps[0].state_after, state, + "state must be the snapshot built at request entry" + ); + } + + /// Companion to the Allow test: confirms `member=false → Deny`. The + /// two together pin the full verdict-mapping table — a mutation that + /// inverts the boolean reds exactly one of them. + #[test] + fn record_req_authcheck_emits_deny_when_not_member() { + let typed = Arc::new(VecTracer::default()); + let inner: Arc = typed.clone(); + let state = dummy_state(); + let ch_id = Uuid::from_u128(0xBADD_BEEF); + + record_req_authcheck(&inner, &state, ch_id, false); + + let steps = typed.steps.lock().expect("vec tracer mutex"); + assert_eq!(steps.len(), 1); + match &steps[0].action { + TraceAction::AuthCheck { verdict, .. } => { + assert!( + matches!(verdict, Verdict::Deny), + "member=false must map to Deny" + ); + } + other => panic!("expected AuthCheck action, got {other:?}"), + } + } } diff --git a/crates/buzz-relay/src/handlers/req.rs b/crates/buzz-relay/src/handlers/req.rs index 0a0cb5e87..dcf833d49 100644 --- a/crates/buzz-relay/src/handlers/req.rs +++ b/crates/buzz-relay/src/handlers/req.rs @@ -95,6 +95,14 @@ pub async fn handle_req( let channel_id = extract_channel_id_from_filters(&filters); + // Build the conformance `AbstractState` once at request entry. The + // `Option` only goes `None` on malformed pubkey bytes (already a + // separate failure path elsewhere); on the hot read path this is + // always `Some` and shared by every emit below. + let trace_state = buzz_core::PublicKey::from_slice(&pubkey_bytes) + .ok() + .map(|pk| crate::conformance::state_for_request(&conn.tenant, &pk)); + // Confirm channel access up front so the repaired `accessible_channels` // vector reaches every downstream consumer: the NIP-50 search branch // below, subscription registration, historical delivery, and COUNT. A @@ -117,7 +125,17 @@ pub async fn handle_req( .is_member(conn.tenant.community(), ch_id, &pubkey_bytes) .await { - Ok(member) => Some(member), + Ok(member) => { + if let Some(state_snap) = trace_state.as_ref() { + crate::conformance::record_req_authcheck( + &state.tracer, + state_snap, + ch_id, + member, + ); + } + Some(member) + } Err(e) => { warn!(conn_id = %conn_id, "Channel membership confirmation failed: {e}"); conn.send(RelayMessage::closed(&sub_id, "error: database error"));