fix(cli): reject empty message content in buzz messages send - #7490
Open
SomSamantray wants to merge 3 commits into
Open
fix(cli): reject empty message content in buzz messages send#7490SomSamantray wants to merge 3 commits into
SomSamantray wants to merge 3 commits into
Conversation
`buzz messages send --content -` published a whitespace-only kind-9 event when stdin carried only whitespace, exited 0, and reported success. A broken upstream pipeline therefore produced a ghost message with no signal that anything failed. Reject empty or whitespace-only content with a Usage error before the mention preflight and before any upload, and add `--allow-empty` as an explicit opt-out mirroring `notes set` and `mem set`. A send that attaches a file stays valid because media assembly makes the published content non-empty. The guard reads the trimmed content, so the reported repro (`printf '%s\n' "$UNSET_VAR"`) is caught; a raw `is_empty()` check would miss it. Signed-off-by: Som Samantray <som.samantray@gmail.com>
Review follow-up for the `messages send` empty-content guard. - Extract `content_comes_from_stdin` so the `--content -` sentinel has one production site, and cover it with a falsifiable predicate test. - Add a process-level test that runs the real binary with piped stdin and asserts the exit code and stderr contract. Flipping the dispatch sentinel to a constant, or removing the guard, now fails it. - Treat a blank `--file` value as no attachment, so it cannot exempt the guard and mask the empty-content diagnosis behind an upload failure. - Assert the inline and stdin error wordings separately, and drop two `query_count` assertions that could not observe the round-trip they claimed to rule out. - Cover whitespace-only content with `--allow-empty`. Signed-off-by: Som Samantray <som.samantray@gmail.com>
Signed-off-by: Som Samantray <som.samantray@gmail.com>
🔐 Codex Security Review
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
buzz messages send --content -published a whitespace-only kind-9 message when stdin carried only whitespace. It exited 0 and reported"message": "", so a caller whose pipeline had failed got a success response and a ghost empty bubble in the channel.The command now refuses to publish empty or whitespace-only content:
printf '%s\n' "$UNSET_VAR" | buzz messages send --channel <UUID> --content --> exit 0, empty message published.{"error":"user_error","message":"refusing to publish an empty message from stdin (an upstream pipeline step likely failed). Pass --allow-empty to confirm.","retryable":false}, nothing published.Key decisions:
"\n", so anis_empty()check would have missed it.--content "". That is deliberately stronger than thenotes set/mem setguards, which only check their stdin branch.--filevalue is not an attachment and does not exempt the guard.--allow-emptyis the escape hatch, matchingnotes setandmem set.read_or_stdinis unchanged, so the other commands that share it are unaffected.Related issue
Fixes #7448.
No existing PR addresses this issue (open and closed PRs searched).
Testing
cargo test -p buzz-cli-> 472 unit tests and 3 process-level tests pass.cargo fmt --all -- --checkandcargo clippy -p buzz-cli --all-targets -- -D warningsare clean.crates/buzz-cli/tests/empty_content_guard.rsruns the real binary with piped stdin and asserts the exit code and stderr contract for empty, whitespace-only, and--allow-emptysends. Flipping the guard's sentinel derivation, or removing the guard, fails it.--content ""exits 1 with the inline diagnosis;--content "" --allow-emptyclears the guard and fails at the network layer;--content "" --file /nonexistentstill reportsupload failed;--content "" --mention <hex64>reports the empty-content error rather than the membership error.Follow-up work deferred to a future PR
messages edit --content ""andmessages send-diff --diff -still publish empty content.U+200B,U+FEFF) passestrim()and still publishes.messages sendbuffers stdin unbounded before the size check, unlike the capped reads innotes setandmem set.