External/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-08-18 06:50:31 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(cli): reject unparseable --kinds instead of silently defaulting

Browse Source 
`--kinds` parsed with `filter_map(|s| s.trim().parse().ok())`, which
discarded every unparseable token. An all-garbage list left the default
kinds in place and exited 0, so `--kinds '*'`, `--kinds all` and
`--kinds ''` all reported success while measuring the DEFAULT narrow
list — exactly the blind spot the flag exists to widen. Measured on the
prior parser: all three returned rc=0 with kinds [9].

Parsing now returns a usage error naming the offending token, and it
happens inside `build_messages_filter` so no code path can reach the
wire with a partially discarded list. Verified on the built binary: the
six garbage shapes exit 1 with the token quoted, while `9`, `7` and the
padded ` 9 , 1984 ` spelling still exit 0. Mutation-checked by restoring
the old `filter_map` body — two unit tests fail and the live binary
returns to rc=0 with kinds [9] for `*` and `all`.

Also documents kind scope in `--help` on both commands, because the CLI
cannot report a channel total and a reply count is not an event count.
Each now states its default list, that `--kinds` REPLACES rather than
extends it, what is EXCLUDED (reactions 7, deletions 5, and edits 40003
on `get`), and that the two commands deliberately use different default
lists so their counts are not comparable. `messages get` sends
[9,40002,40008,45001,45003]; `messages thread` sends
[9,40002,40003,40008,45003].

The reactions gap has a ready-made server-side answer — `include_aux`
and `WINDOW_AUX_KINDS` (bridge.rs:383-394, :494) already walk a two-hop
reaction/deletion closure with zero call sites in buzz-cli — but
plumbing it adds a second surface (aux closure semantics, dedup of
two-hop deletions), so it is filed as a follow-up rather than ridden in
here.

Co-authored-by: Sami <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@buzz.block.builderlab.xyz>
Signed-off-by: Sami <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@buzz.block.builderlab.xyz>
This commit is contained in:
Sami
2026-08-15 20:52:32 -04:00
parent 8aad722246
commit da612711cf
2 changed files with 104 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/buzz-cli/src/commands/messages.rs
+100 -14
View File
@@ -370,6 +370,34 @@ fn validate_cursor_pair(
Ok(())
}
/// Kinds returned by `messages get` when `--kinds` is omitted.
///
/// Quoted by file:line in field notes, because the CLI has no way to report
/// the kind universe of a channel — a pull can only ever state which kinds it
/// asked for.
const DEFAULT_MESSAGE_KINDS: [u64; 5] = [9, 40002, 40008, 45001, 45003];
/// Parse a `--kinds` list, refusing anything that is not an event kind.
///
/// The previous form was `filter_map(|s| s.trim().parse().ok())`, which
/// discarded unparseable tokens silently: `--kinds '*'` and `--kinds all`
/// produced an empty list, left the default kinds in place, and exited 0 — so
/// a caller measuring a widened pull was handed the narrow default while being
/// told it succeeded. A typo is now a usage error naming the token.
fn parse_kinds(kinds: &str) -> Result<Vec<u64>, CliError> {
kinds
.split(',')
.map(|token| {
let token = token.trim();
token.parse::<u64>().map_err(|_| {
CliError::Usage(format!(
"--kinds: `{token}` is not an event kind (expected comma-separated integers, e.g. 9,1984)"
))
})
})
.collect()
}
/// Build the filter for a channel message query.
///
/// Split out from [`cmd_get_messages`] so the cursor grammar is testable
@@ -381,19 +409,18 @@ fn build_messages_filter(
before_id: Option<&str>,
since: Option<i64>,
kinds: Option<&str>,
) -> serde_json::Value {
) -> Result<serde_json::Value, CliError> {
let mut filter = serde_json::json!({
"kinds": [9, 40002, 40008, 45001, 45003],
"kinds": DEFAULT_MESSAGE_KINDS,
"#h": [channel_id],
"limit": limit
});
// If specific kinds requested, override
// If specific kinds requested, override. Parsing happens here rather than
// at the caller so no code path can reach the wire with a partially
// discarded kind list.
if let Some(k) = kinds {
let kind_list: Vec<u64> = k.split(',').filter_map(|s| s.trim().parse().ok()).collect();
if !kind_list.is_empty() {
filter["kinds"] = serde_json::json!(kind_list);
}
filter["kinds"] = serde_json::json!(parse_kinds(k)?);
}
if let Some(b) = before {
@@ -407,7 +434,7 @@ fn build_messages_filter(
filter["since"] = serde_json::json!(s);
}
filter
Ok(filter)
}
#[allow(clippy::too_many_arguments)]
@@ -425,7 +452,7 @@ pub async fn cmd_get_messages(
validate_cursor_pair(before, before_id, "--before-id", "--before")?;
let limit = limit.unwrap_or(50).min(200);
let filter = build_messages_filter(channel_id, limit, before, before_id, since, kinds);
let filter = build_messages_filter(channel_id, limit, before, before_id, since, kinds)?;
let resp = client.query(&filter).await?;
let mut events: Vec<serde_json::Value> = serde_json::from_str(&resp).unwrap_or_default();
@@ -1606,7 +1633,7 @@ mod messages_cursor_tests {
#[test]
fn head_request_sends_no_cursor_fields() {
// The default pull must keep its existing wire shape.
let f = build_messages_filter(CH, 50, None, None, None, None);
let f = build_messages_filter(CH, 50, None, None, None, None).expect("no kinds to parse");
assert!(f.get("until").is_none());
assert!(f.get("before_id").is_none());
assert_eq!(f["limit"], serde_json::json!(50));
@@ -1617,14 +1644,22 @@ mod messages_cursor_tests {
fn timestamp_only_cursor_still_works() {
// Back-compat: `--before` alone is the existing (inclusive) cursor and
// must keep sending a bare `until`, never a half composite.
let f = build_messages_filter(CH, 200, Some(1_786_800_000), None, None, None);
let f = build_messages_filter(CH, 200, Some(1_786_800_000), None, None, None)
.expect("no kinds to parse");
assert_eq!(f["until"], serde_json::json!(1_786_800_000_i64));
assert!(f.get("before_id").is_none());
}
#[test]
fn composite_cursor_sends_both_halves() {
let f = build_messages_filter(CH, 200, Some(1_786_800_000), Some(BEFORE_ID), None, None);
// Wire-verified discriminator (buzz-security corpus, `bff3110a0`):
// window B=1785282528, cap=7, tie second T=1785163671 with
// multiplicity 2. A decrement-always backward walk recovers 1/2 there
// (drops `fa81da0b`); the same walk with `--before-id` recovers 2/2.
// So the composite cursor removes a correctness dependency on the
// caller's loop shape, not merely a truncation at a large tie.
let f = build_messages_filter(CH, 200, Some(1_786_800_000), Some(BEFORE_ID), None, None)
.expect("no kinds to parse");
assert_eq!(f["until"], serde_json::json!(1_786_800_000_i64));
assert_eq!(f["before_id"], serde_json::json!(BEFORE_ID));
}
@@ -1633,7 +1668,8 @@ mod messages_cursor_tests {
fn cursor_id_is_dropped_without_a_timestamp() {
// The relay 400s on `before_id` without `until`; the builder must not
// emit a half cursor even if the caller-level guard is bypassed.
let f = build_messages_filter(CH, 200, None, Some(BEFORE_ID), None, None);
let f = build_messages_filter(CH, 200, None, Some(BEFORE_ID), None, None)
.expect("no kinds to parse");
assert!(f.get("before_id").is_none());
assert!(f.get("until").is_none());
}
@@ -1647,7 +1683,8 @@ mod messages_cursor_tests {
Some(BEFORE_ID),
Some(1_786_000_000),
Some("9,1984"),
);
)
.expect("a well-formed kind list parses");
assert_eq!(f["since"], serde_json::json!(1_786_000_000_i64));
assert_eq!(f["kinds"], serde_json::json!([9, 1984]));
assert_eq!(f["before_id"], serde_json::json!(BEFORE_ID));
@@ -1688,4 +1725,53 @@ mod messages_cursor_tests {
// And a bare timestamp cursor is legal on both surfaces.
assert!(validate_cursor_pair(Some(1_786_800_000), None, "--before-id", "--before").is_ok());
}
// ── `--kinds` must not substitute the default for what you asked for ──
//
// Measured at bff3110a0, before this change: `--kinds ''`, `--kinds '*'`
// and `--kinds all` all exited 0 having sent the DEFAULT kind list, so a
// caller trying to widen a pull was handed the narrow default and told it
// worked. Each shape below is one of those commands.
#[test]
fn a_wildcard_kind_list_is_refused_instead_of_silently_defaulting() {
for garbage in ["*", "all", "", "9,*", "9, ,1984", "-1", "1984abc"] {
let err = build_messages_filter(CH, 50, None, None, None, Some(garbage))
.expect_err("unparseable --kinds must be a usage error");
assert!(
err.to_string().contains("is not an event kind"),
"unexpected error for {garbage:?}: {err}"
);
}
}
#[test]
fn a_refused_kind_list_never_reaches_the_wire_as_the_default() {
// The specific failure this closes: the error must not be recoverable
// into a filter at all, so there is no shape where `*` measures [9].
assert!(build_messages_filter(CH, 50, None, None, None, Some("*")).is_err());
let ok = build_messages_filter(CH, 50, None, None, None, Some("7"))
.expect("a real token still works");
assert_eq!(ok["kinds"], serde_json::json!([7]));
}
#[test]
fn whitespace_around_real_tokens_is_still_tolerated() {
// Positive control for the stricter parser: it must reject typos
// without also rejecting the documented ` 9, 1984 ` spelling.
let f = build_messages_filter(CH, 50, None, None, None, Some(" 9 , 1984 "))
.expect("padded integers parse");
assert_eq!(f["kinds"], serde_json::json!([9, 1984]));
}
#[test]
fn omitting_kinds_sends_the_documented_default_list() {
// The default is what `--help` and any field note must quote; pin it so
// a change to the list is a deliberate edit here.
let f = build_messages_filter(CH, 50, None, None, None, None).expect("no kinds to parse");
assert_eq!(
f["kinds"],
serde_json::json!([9, 40002, 40008, 45001, 45003])
);
}
}
crates/buzz-cli/src/lib.rs
+4 -3
View File
@@ -461,7 +461,7 @@ pub enum MessagesCmd {
},
/// Retrieve messages from a channel
#[command(
after_help = "Pagination:\n Returns up to --limit messages (default 50, max 200), NEWEST-first.\n For channels larger than the cap, page backwards with --before:\n\n buzz messages get --channel <UUID> --limit 200\n buzz messages get --channel <UUID> --limit 200 \\\n --before <created_at of oldest message seen> --before-id <its event id>\n\n --before alone is INCLUSIVE (<=), so a timestamp-only cursor re-returns\n every message sharing that second; if one second holds more messages than\n --limit, paging stalls. Pass --before-id to make the cursor exclusive.\n\nExamples:\n buzz messages get --channel <UUID>\n buzz messages get --channel <UUID> --limit 50 --kinds 1,1984"
after_help = "Pagination:\n Returns up to --limit messages (default 50, max 200), NEWEST-first.\n For channels larger than the cap, page backwards with --before:\n\n buzz messages get --channel <UUID> --limit 200\n buzz messages get --channel <UUID> --limit 200 \\\n --before <created_at of oldest message seen> --before-id <its event id>\n\n --before alone is INCLUSIVE (<=), so a timestamp-only cursor re-returns\n every message sharing that second; if one second holds more messages than\n --limit, paging stalls. Pass --before-id to make the cursor exclusive.\n\nKind scope:\n Without --kinds this returns kinds 9,40002,40008,45001,45003 (message,\n message v2, diff, forum post, forum comment) and NOTHING ELSE. Notably\n EXCLUDED: reactions (7), deletions (5), and message edits (40003) — a\n channel can hold many events this command never shows, and it cannot\n report a channel total. --kinds REPLACES that list; it does not add to\n it, and an unparseable token is an error rather than a silent default.\n `messages thread` uses a DIFFERENT default list (it includes edits 40003\n and omits forum posts 45001), so counts from the two commands are not\n comparable.\n\nExamples:\n buzz messages get --channel <UUID>\n buzz messages get --channel <UUID> --limit 50 --kinds 1,1984"
)]
Get {
/// Channel UUID
@@ -481,13 +481,14 @@ pub enum MessagesCmd {
/// Unix timestamp — return messages after this time
#[arg(long)]
since: Option<i64>,
/// Comma-separated event kinds to filter (e.g. 1,1984)
/// Comma-separated event kinds, REPLACING the default list (e.g.
/// 1,1984). Unparseable tokens are rejected, not ignored
#[arg(long)]
kinds: Option<String>,
},
/// Get a message thread (replies to a root message)
#[command(
after_help = "Pagination:\n Returns up to --limit replies (default 100, max 500) plus the root event.\n Replies without a cursor and without --depth-limit are NEWEST-first; either\n one selects the OLDEST-first walk, so the cursor picks which end you see.\n\n To page a thread larger than the cap, walk FORWARD from the oldest reply.\n Seed with --after 0, then pass the newest reply of each page back in:\n\n buzz messages thread --channel <UUID> --event <ID> --limit 500 --after 0\n buzz messages thread --channel <UUID> --event <ID> --limit 500 \\\n --after <created_at of newest reply seen> --after-id <its event id>\n\n Always pass --after-id. The timestamp-only cursor is STRICTLY greater-than,\n so a page boundary landing inside a second shared by several replies skips\n the rest of that second silently (rc=0). --after-id carries the tiebreak, so\n no reply is skipped regardless of where the boundary lands."
after_help = "Pagination:\n Returns up to --limit replies (default 100, max 500) plus the root event.\n Replies without a cursor and without --depth-limit are NEWEST-first; either\n one selects the OLDEST-first walk, so the cursor picks which end you see.\n\n To page a thread larger than the cap, walk FORWARD from the oldest reply.\n Seed with --after 0, then pass the newest reply of each page back in:\n\n buzz messages thread --channel <UUID> --event <ID> --limit 500 --after 0\n buzz messages thread --channel <UUID> --event <ID> --limit 500 \\\n --after <created_at of newest reply seen> --after-id <its event id>\n\n Always pass --after-id. The timestamp-only cursor is STRICTLY greater-than,\n so a page boundary landing inside a second shared by several replies skips\n the rest of that second silently (rc=0). --after-id carries the tiebreak, so\n no reply is skipped regardless of where the boundary lands.\n\nKind scope:\n Replies are limited to kinds 9,40002,40003,40008,45003 (message, message\n v2, edit, diff, forum comment) and NOTHING ELSE. Notably EXCLUDED:\n reactions (7) and deletions (5) — on a busy thread these outnumber the\n replies, and no flag here surfaces them, so a reply count is not a thread\n event count. This list intentionally differs from `messages get`, which\n omits edits (40003) and includes forum posts (45001)."
)]
Thread {
/// Channel UUID