mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(relay/core): plug COUNT existence-leak and StopReason forward-compat for NIP-AM
Two Thufir-flagged IMPORTANT fixes for PR #1445. Count gate (COUNT existence leak): - Add RESULT_GATED_KINDS = [KIND_DM_VISIBILITY, KIND_AGENT_TURN_METRIC] to kind.rs — explicit list of kinds that require per-event owner verification even for COUNT queries. - Add filter_can_match_result_gated_kinds() to req.rs — returns true when filter has no kinds constraint (wildcard) or includes a result-gated kind. - Add result_gated_count_safe_for_pushdown() to req.rs — safe to use fast SQL count_events() only when filter's #p tag is non-empty and all values equal the authenticated reader's pubkey. - Apply the guard in count.rs (WS): both with-channel and without-channel fast-path conditions now require !needs_result_gated_filtering; both fallback loops now call reader_authorized_for_event per event. - Apply the guard in bridge.rs (HTTP): same two fast-path conditions and same two fallback loops. - 6 unit tests covering wildcard/explicit/safe-pushdown/unsafe cases. StopReason forward-compatibility: - Replace #[derive(Deserialize)] on StopReason with a custom impl that maps any unrecognized string to StopReason::Unknown instead of returning an error; NIP-AM requires consumers to accept future stopReason values. - Add test unknown_stop_reason_maps_to_unknown_not_error: future value tool_limit deserializes to Unknown; token counts remain intact. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
co-authored by
Will Pfleger
parent
23b522d992
commit
c9a1458f5e
@@ -46,7 +46,11 @@ pub struct TokenCounts {
|
||||
}
|
||||
|
||||
/// Why a turn ended.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
///
|
||||
/// NIP-AM: consumers MUST treat unrecognized `stopReason` values as `Unknown`
|
||||
/// and keep the token counts valid. Custom deserialization maps any unrecognized
|
||||
/// string to `Unknown` instead of failing the whole payload.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum StopReason {
|
||||
/// Model reached a natural end-of-turn.
|
||||
@@ -57,10 +61,24 @@ pub enum StopReason {
|
||||
Cancelled,
|
||||
/// Turn ended with an error.
|
||||
Error,
|
||||
/// Stop reason is unknown.
|
||||
/// Stop reason is unknown or unrecognized.
|
||||
Unknown,
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for StopReason {
|
||||
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
|
||||
let s = String::deserialize(deserializer)?;
|
||||
Ok(match s.as_str() {
|
||||
"end_turn" => StopReason::EndTurn,
|
||||
"max_tokens" => StopReason::MaxTokens,
|
||||
"cancelled" => StopReason::Cancelled,
|
||||
"error" => StopReason::Error,
|
||||
"unknown" => StopReason::Unknown,
|
||||
_ => StopReason::Unknown,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
/// Decrypted payload of a `kind:44200` Agent Turn Metric event.
|
||||
///
|
||||
/// `harness` and `timestamp` are REQUIRED. All other fields are optional or
|
||||
@@ -261,4 +279,33 @@ mod tests {
|
||||
let back: TokenCounts = serde_json::from_str(&json).unwrap();
|
||||
assert_eq!(back, counts);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unknown_stop_reason_maps_to_unknown_not_error() {
|
||||
// NIP-AM: consumers MUST treat unrecognized stopReason values as Unknown;
|
||||
// the token counts remain valid and the whole payload must not be rejected.
|
||||
let json = r#"{
|
||||
"harness": "goose",
|
||||
"timestamp": "2026-07-01T20:11:03Z",
|
||||
"stopReason": "tool_limit",
|
||||
"turn": {
|
||||
"inputTokens": 1234,
|
||||
"outputTokens": 567,
|
||||
"totalTokens": 1801,
|
||||
"costUsd": null
|
||||
}
|
||||
}"#;
|
||||
let payload: AgentTurnMetricPayload =
|
||||
serde_json::from_str(json).expect("payload with future stopReason must parse");
|
||||
assert_eq!(
|
||||
payload.stop_reason,
|
||||
Some(StopReason::Unknown),
|
||||
"unrecognized stopReason must map to Unknown"
|
||||
);
|
||||
// Token counts must be preserved.
|
||||
let turn = payload.turn.expect("turn must be present");
|
||||
assert_eq!(turn.input_tokens, Some(1234));
|
||||
assert_eq!(turn.output_tokens, Some(567));
|
||||
assert_eq!(turn.total_tokens, Some(1801));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -110,6 +110,15 @@ pub const KIND_EVENT_REMINDER: u32 = 30300;
|
||||
/// a compile-time bitset or sorted array with binary search for hot-path use.
|
||||
pub const AUTHOR_ONLY_KINDS: &[u32] = &[KIND_EVENT_REMINDER];
|
||||
|
||||
/// Kinds that require a result-level read gate beyond the filter-layer
|
||||
/// `#p` check: even a reader who knows an event id MUST match the event's
|
||||
/// `#p` tag to receive the event. This closes the kindless `{ids:[…]}` read
|
||||
/// path for events whose existence must not be leaked.
|
||||
///
|
||||
/// Used by `filter_can_match_result_gated_kinds` to force the per-event
|
||||
/// fallback path in COUNT rather than the fast SQL `count_events()`.
|
||||
pub const RESULT_GATED_KINDS: &[u32] = &[KIND_DM_VISIBILITY, KIND_AGENT_TURN_METRIC];
|
||||
|
||||
/// Kinds whose stored events have `#p`-bound read access — readable only by
|
||||
/// subscribers whose pubkey appears in the event's `#p` tag.
|
||||
///
|
||||
|
||||
@@ -708,6 +708,15 @@ pub async fn count_events(
|
||||
for filter in &filters {
|
||||
let needs_author_only_filtering =
|
||||
crate::handlers::req::filter_can_match_author_only_kinds(filter);
|
||||
// Same result-gated guard as the WS COUNT handler: force the per-event
|
||||
// fallback for filters that can match 44200 or 30622 unless #p=[self]
|
||||
// is safely pushed down (existence leak otherwise).
|
||||
let needs_result_gated_filtering =
|
||||
crate::handlers::req::filter_can_match_result_gated_kinds(filter)
|
||||
&& !crate::handlers::req::result_gated_count_safe_for_pushdown(
|
||||
filter,
|
||||
&authed_pubkey_hex,
|
||||
);
|
||||
|
||||
// If filter targets a specific channel, verify access.
|
||||
if let Some(ch_id) = extract_channel_from_filter(filter) {
|
||||
@@ -730,6 +739,7 @@ pub async fn count_events(
|
||||
});
|
||||
if crate::handlers::req::filter_fully_pushable(filter)
|
||||
&& (!needs_author_only_filtering || author_is_self)
|
||||
&& !needs_result_gated_filtering
|
||||
{
|
||||
match state.db.count_events(&query).await {
|
||||
Ok(n) => total += n as u64,
|
||||
@@ -759,6 +769,12 @@ pub async fn count_events(
|
||||
{
|
||||
continue;
|
||||
}
|
||||
if !buzz_core::filter::reader_authorized_for_event(
|
||||
&se.event,
|
||||
&authed_pubkey_hex,
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
total += 1;
|
||||
}
|
||||
}
|
||||
@@ -787,6 +803,7 @@ pub async fn count_events(
|
||||
});
|
||||
if crate::handlers::req::filter_fully_pushable(filter)
|
||||
&& (!needs_author_only_filtering || author_is_self)
|
||||
&& !needs_result_gated_filtering
|
||||
{
|
||||
query.limit = None;
|
||||
match state.db.count_events(&query).await {
|
||||
@@ -816,6 +833,12 @@ pub async fn count_events(
|
||||
{
|
||||
continue;
|
||||
}
|
||||
if !buzz_core::filter::reader_authorized_for_event(
|
||||
&se.event,
|
||||
&authed_pubkey_hex,
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
total += 1;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,7 +6,9 @@ use nostr::Filter;
|
||||
use tracing::warn;
|
||||
|
||||
use crate::connection::{AuthState, ConnectionState};
|
||||
use crate::handlers::req::is_author_only_event;
|
||||
use crate::handlers::req::{
|
||||
filter_can_match_result_gated_kinds, is_author_only_event, result_gated_count_safe_for_pushdown,
|
||||
};
|
||||
use crate::protocol::RelayMessage;
|
||||
use crate::state::AppState;
|
||||
|
||||
@@ -100,6 +102,14 @@ pub async fn handle_count(
|
||||
// fast-path count_events() cannot be used because it doesn't do
|
||||
// per-event author filtering.
|
||||
let needs_author_only_filtering = super::req::filter_can_match_author_only_kinds(filter);
|
||||
// Determine if this filter can match result-gated kinds (44200, 30622)
|
||||
// that require a per-event owner check. When the fast SQL path would
|
||||
// count matching rows without calling reader_authorized_for_event, a
|
||||
// non-owner learns the existence of events they are not allowed to see.
|
||||
// The only safe pushdown is when #p is pinned to the authenticated
|
||||
// reader's own pubkey.
|
||||
let needs_result_gated_filtering = filter_can_match_result_gated_kinds(filter)
|
||||
&& !result_gated_count_safe_for_pushdown(filter, &authed_pubkey_hex);
|
||||
|
||||
if let Some(ch_id) = extract_channel_from_filter(filter) {
|
||||
// Filter targets a specific channel — verify access. Mirrors the WS
|
||||
@@ -149,6 +159,7 @@ pub async fn handle_count(
|
||||
});
|
||||
if super::req::filter_fully_pushable(filter)
|
||||
&& (!needs_author_only_filtering || author_is_self)
|
||||
&& !needs_result_gated_filtering
|
||||
{
|
||||
match state.db.count_events(&query).await {
|
||||
Ok(n) => total += n as u64,
|
||||
@@ -179,6 +190,12 @@ pub async fn handle_count(
|
||||
if is_author_only_event(&se.event, &pubkey_bytes) {
|
||||
continue;
|
||||
}
|
||||
if !buzz_core::filter::reader_authorized_for_event(
|
||||
&se.event,
|
||||
&authed_pubkey_hex,
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
total += 1;
|
||||
}
|
||||
}
|
||||
@@ -212,6 +229,7 @@ pub async fn handle_count(
|
||||
});
|
||||
if super::req::filter_fully_pushable(filter)
|
||||
&& (!needs_author_only_filtering || author_is_self)
|
||||
&& !needs_result_gated_filtering
|
||||
{
|
||||
query.limit = None; // COUNT doesn't need a row limit
|
||||
match state.db.count_events(&query).await {
|
||||
@@ -242,6 +260,12 @@ pub async fn handle_count(
|
||||
if is_author_only_event(&se.event, &pubkey_bytes) {
|
||||
continue;
|
||||
}
|
||||
if !buzz_core::filter::reader_authorized_for_event(
|
||||
&se.event,
|
||||
&authed_pubkey_hex,
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
total += 1;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,7 +8,7 @@ use tracing::{debug, warn};
|
||||
use buzz_core::filter::filters_match;
|
||||
use buzz_core::kind::{
|
||||
AUTHOR_ONLY_KINDS, KIND_AGENT_ENGRAM, KIND_AGENT_TURN_METRIC, KIND_DM_VISIBILITY,
|
||||
P_GATED_KINDS,
|
||||
P_GATED_KINDS, RESULT_GATED_KINDS,
|
||||
};
|
||||
use buzz_core::tenant::TenantContext;
|
||||
use buzz_db::EventQuery;
|
||||
@@ -1064,6 +1064,44 @@ pub(crate) fn filter_can_match_author_only_kinds(filter: &Filter) -> bool {
|
||||
})
|
||||
}
|
||||
|
||||
/// Returns `true` if the filter CAN match result-gated kinds — meaning it
|
||||
/// either has no `kinds` constraint (wildcard) or includes at least one kind
|
||||
/// that carries a per-event result-level read gate (currently
|
||||
/// `KIND_DM_VISIBILITY` and `KIND_AGENT_TURN_METRIC`).
|
||||
///
|
||||
/// Used by the COUNT handler to force the per-event fallback path instead of
|
||||
/// the fast SQL `count_events()`, which cannot enforce the owner-only result
|
||||
/// gate. An existence count leaks private event activity even though no content
|
||||
/// is returned, violating the NIP-AM / NIP-DM requirement that knowing an id
|
||||
/// MUST NOT grant access.
|
||||
pub(crate) fn filter_can_match_result_gated_kinds(filter: &Filter) -> bool {
|
||||
filter.kinds.as_ref().is_none_or(|ks| {
|
||||
ks.iter()
|
||||
.any(|k| RESULT_GATED_KINDS.contains(&(k.as_u16() as u32)))
|
||||
})
|
||||
}
|
||||
|
||||
/// Returns `true` if a result-gated-kind COUNT filter can safely use the fast
|
||||
/// SQL pushdown path — specifically, when the filter's `#p` tag is non-empty
|
||||
/// and every entry equals the authenticated reader's pubkey.
|
||||
///
|
||||
/// In that case the SQL `WHERE #p = self` pushdown scopes the query to the
|
||||
/// reader's own events, so the fast path cannot leak another owner's event
|
||||
/// existence. This mirrors the owner's own subscription pattern from the NIP:
|
||||
/// `{kinds:[44200], #p:[self]}`.
|
||||
///
|
||||
/// When this returns `false`, the COUNT handler MUST use the per-event fallback
|
||||
/// and apply `reader_authorized_for_event` on each row.
|
||||
pub(crate) fn result_gated_count_safe_for_pushdown(
|
||||
filter: &Filter,
|
||||
authed_pubkey_hex: &str,
|
||||
) -> bool {
|
||||
let p_tag = nostr::SingleLetterTag::lowercase(nostr::Alphabet::P);
|
||||
filter.generic_tags.get(&p_tag).is_some_and(|values| {
|
||||
!values.is_empty() && values.iter().all(|v| v == authed_pubkey_hex)
|
||||
})
|
||||
}
|
||||
|
||||
/// Returns `true` if the event is an author-only kind and the requester is NOT
|
||||
/// the author. Used as a per-event filter during historical delivery and fan-out
|
||||
/// to silently omit unauthorized events from mixed-kind result sets.
|
||||
@@ -1635,4 +1673,64 @@ mod tests {
|
||||
.search("x");
|
||||
assert!(!p_gated_filters_authorized(&[f], &agent));
|
||||
}
|
||||
|
||||
// ── filter_can_match_result_gated_kinds + result_gated_count_safe_for_pushdown ──
|
||||
|
||||
#[test]
|
||||
fn result_gated_wildcard_filter_can_match() {
|
||||
// No kinds constraint — could match anything, including 44200 / 30622.
|
||||
let f = Filter::new();
|
||||
assert!(filter_can_match_result_gated_kinds(&f));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn result_gated_explicit_44200_can_match() {
|
||||
let f = Filter::new()
|
||||
.kind(nostr::Kind::Custom(buzz_core::kind::KIND_AGENT_TURN_METRIC as u16));
|
||||
assert!(filter_can_match_result_gated_kinds(&f));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn result_gated_explicit_30622_can_match() {
|
||||
let f = Filter::new()
|
||||
.kind(nostr::Kind::Custom(buzz_core::kind::KIND_DM_VISIBILITY as u16));
|
||||
assert!(filter_can_match_result_gated_kinds(&f));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn result_gated_kind_9_only_cannot_match() {
|
||||
let f = Filter::new().kind(nostr::Kind::TextNote);
|
||||
assert!(!filter_can_match_result_gated_kinds(&f));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn result_gated_safe_pushdown_requires_p_self() {
|
||||
let (owner, _agent, _other) = three_pubkeys();
|
||||
let p_tag = nostr::SingleLetterTag::lowercase(nostr::Alphabet::P);
|
||||
let f = nostr::Filter::new()
|
||||
.kind(nostr::Kind::Custom(buzz_core::kind::KIND_AGENT_TURN_METRIC as u16))
|
||||
.custom_tags(p_tag, [owner.clone()]);
|
||||
// Owner querying their own metrics — safe to push down.
|
||||
assert!(result_gated_count_safe_for_pushdown(&f, &owner));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn result_gated_safe_pushdown_rejects_when_p_is_other() {
|
||||
let (owner, _agent, other) = three_pubkeys();
|
||||
let p_tag = nostr::SingleLetterTag::lowercase(nostr::Alphabet::P);
|
||||
let f = nostr::Filter::new()
|
||||
.kind(nostr::Kind::Custom(buzz_core::kind::KIND_AGENT_TURN_METRIC as u16))
|
||||
.custom_tags(p_tag, [other.clone()]);
|
||||
// Authenticated as owner but #p is someone else — NOT safe.
|
||||
assert!(!result_gated_count_safe_for_pushdown(&f, &owner));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn result_gated_safe_pushdown_rejects_when_no_p_tag() {
|
||||
let (owner, _agent, _other) = three_pubkeys();
|
||||
let f = nostr::Filter::new()
|
||||
.kind(nostr::Kind::Custom(buzz_core::kind::KIND_AGENT_TURN_METRIC as u16));
|
||||
// No #p tag — fallback required.
|
||||
assert!(!result_gated_count_safe_for_pushdown(&f, &owner));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user