fix(cli): reject JSON-escaped newlines before signing - #5114
Open
johnely19 wants to merge 1 commit into
Open
Conversation
Messages built by shell wrappers could reach the relay with literal `\n` escape text instead of real LF bytes, so recipients saw backslash-n where paragraph and list breaks were intended. Validate content before signing and fail closed when escape text appears outside Markdown code regions at paragraph/list boundaries, pointing the caller at `--content -`. Intentional `\n` inside inline and fenced code is left untouched rather than rewritten. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: John Ely <john@heyricordo.com>
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
Messages built by shell wrappers can reach the relay carrying literal
\nescape text instead of real LF bytes, so recipients see\nwhere paragraph and list breaks were intended. The CLI signs and publishes the mangled content without complaint, and the damage is only visible after the fact in someone else's client.This validates content before signing and fails closed when escape text appears outside Markdown code regions at paragraph/list boundaries (
\n\n,\n-,\n*,\n#), pointing the caller at--content -.The check is deliberately narrow. Intentional
\ninside inline and fenced code is preserved, because that is legitimate content — a message explainingprintf 'a\nb'must still be sendable. It only rejects the structural positions that a JSON-escaped multiline message produces, which is where the mistake actually shows up.Why this is separate from #2121
#2121 (
fix(acp): teach agents to send real newlines, merged July) addressed this at the prompt level inbase_prompt.md. That reduces the mistake but cannot prevent it: any wrapper, script, or non-ACP caller building a message with single-quoted shell strings still publishes broken content, and instructions are not a mechanism.I hit this in practice today, well after #2121 shipped, which is what prompted the change. Guidance tells the caller what to do; this makes the signing boundary refuse to do the wrong thing.
Related issue
None found. Searched open and merged PRs and issues for newline/escape handling; the closest is #2121 above, which is complementary rather than overlapping — it changes ACP prompt text, this changes CLI validation. No open issue covers the CLI path.
Testing
cargo test -p buzz-cliatc92b5b0a: 324 passed, 0 failed.Two new cases cover both directions, because a validator that only proves the failure case is half-tested:
multiline_markdown_keeps_real_lf_bytes— real LF content passes untouched, including\ninside inline and fenced code.json_escaped_markdown_boundaries_fail_before_signing— escaped boundaries are rejected before signing, so nothing reaches the relay.The second is the one that matters: the failure has to happen ahead of the signature, not after, or the event is already published by the time anyone notices.
No UI change.