mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
feat(relay): emit AuthCheck on REQ membership decision
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 <tlongwell@block.xyz>
Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
This commit is contained in:
co-authored by
Tyler Longwell
parent
473075561e
commit
036e3eee30
@@ -128,6 +128,43 @@ pub fn emit(tracer: &Arc<dyn Tracer>, 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<dyn Tracer>,
|
||||
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<dyn Tracer> = 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<dyn Tracer> = 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:?}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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"));
|
||||
|
||||
Reference in New Issue
Block a user