mirror of
https://github.com/block/buzz.git
synced 2026-10-31 14:31:02 +00:00
fix(desktop): keep escape-form substitutions conservative across shells
Codex found a hole in the previous commit. Backslash-escaping earned literal
credit -- `--content "literal \$MESSAGE"` was displayed as sent text -- on the
assumption that a backslash escape is portable. It is not, and `BUZZ_SHELL`
explicitly supports the shells where it is not (`buzz-dev-mcp/src/shell.rs`
dispatches `-Command` for powershell/pwsh and `/C` for cmd).
Verified by running PowerShell 7 rather than reasoning about it:
'Fixed `labelForStatus` in the rail' -> Fixed `labelForStatus` in the rail
'Costs $5 per run, not $MESSAGE' -> Costs $5 per run, not $MESSAGE
"literal \$MESSAGE" -> literal \ (variable EXPANDED)
So single quoting is literal in PowerShell too, and the fix this PR exists for
is portable. Backslash is not an escape character there at all: the shell keeps
the backslash and still expands the variable, which is exactly the #2201 failure
the guard prevents, and the old character test returned null for it.
Escaped `$` and backtick therefore stay flagged as substitutions even though
bash reads them as literal. Only single quoting is trusted, because only single
quoting means the same thing in every shell an operator can select. The affected
test now asserts null and says why.
The bash-argv probe still agrees on all nine command lines, with the escape case
marked as intentionally more conservative than bash. Mutating the new guard away
fails exactly one test -- the new one -- so it is not vacuous.
Desktop suite 5516 passing / 0 failing; typecheck, check and the file-size
ratchet clean.
Refs #6834
Signed-off-by: ss-dev-01 <11939edb7df583f855dbef923f2358f1184538f88ca452e19e7e35e42ad6d796@buzz.block.builderlab.xyz>
Co-authored-by: Bradley Axen <baxen@squareup.com>
Signed-off-by: Bradley Axen <baxen@squareup.com>
This commit is contained in:
@@ -149,12 +149,16 @@ test("parseBuzzCliCommand preserves single-quoted literals in the --content=valu
|
||||
assert.equal(descriptor?.preview, "use `pnpm test`");
|
||||
});
|
||||
|
||||
test("parseBuzzCliCommand preserves backslash-escaped substitution characters", () => {
|
||||
test("parseBuzzCliCommand rejects backslash-escaped substitution characters as non-portable", () => {
|
||||
// bash treats `\$MESSAGE` as literal text, but PowerShell keeps the backslash
|
||||
// and expands the variable anyway, so the escaped form cannot be trusted as a
|
||||
// literal. Only single quoting is literal across the shells BUZZ_SHELL allows.
|
||||
const descriptor = parseBuzzCliCommand(
|
||||
'buzz messages send --channel agents --content "literal \\$MESSAGE and \\`code\\`"',
|
||||
);
|
||||
|
||||
assert.equal(descriptor?.preview, "literal $MESSAGE and `code`");
|
||||
assert.equal(descriptor?.renderClass, "message");
|
||||
assert.equal(descriptor?.preview, null);
|
||||
});
|
||||
|
||||
test("parseBuzzCliCommand still rejects an unquoted substitution in --content", () => {
|
||||
|
||||
@@ -519,15 +519,21 @@ function findBuzzCommand(tokens: ShellToken[]): BuzzCommandRange | null {
|
||||
*
|
||||
* `hasSubstitution` is what quote context buys us: `$` and a backtick mean
|
||||
* expansion when they are unquoted or inside double quotes, and mean nothing at
|
||||
* all inside single quotes or after a backslash. Without this distinction a
|
||||
* markdown code span in `--content 'use `pnpm test`'` is indistinguishable from
|
||||
* a real `--content "$(cat file)"`, and both have to be discarded.
|
||||
* all inside single quotes. Without this distinction a markdown code span in
|
||||
* `--content 'use `pnpm test`'` is indistinguishable from a real
|
||||
* `--content "$(cat file)"`, and both have to be discarded.
|
||||
*
|
||||
* The `$` rule is deliberately conservative — any unescaped `$` outside single
|
||||
* quotes flags the token, even where bash would leave it literal (a trailing
|
||||
* `$`, or `$` before a space). Displaying an unexpanded shell expression as if
|
||||
* it were the sent text is the failure this guard exists to prevent, so it errs
|
||||
* towards dropping the preview.
|
||||
* Only single quoting is trusted, because only single quoting is literal in
|
||||
* every shell `BUZZ_SHELL` supports. A backslash escape is not: PowerShell
|
||||
* keeps the backslash and still expands `"literal \$MESSAGE"`. So escaped
|
||||
* substitution characters stay flagged even though bash would treat them as
|
||||
* literal text.
|
||||
*
|
||||
* The `$` rule is deliberately conservative in the same direction — any `$`
|
||||
* outside single quotes flags the token, even where bash would leave it literal
|
||||
* (a trailing `$`, or `$` before a space). Displaying an unexpanded shell
|
||||
* expression as if it were the sent text is the failure this guard exists to
|
||||
* prevent, so it errs towards dropping the preview.
|
||||
*/
|
||||
export type ShellToken = {
|
||||
value: string;
|
||||
@@ -555,6 +561,13 @@ export function tokenizeShellCommandTokens(command: string): ShellToken[] {
|
||||
|
||||
for (const char of command) {
|
||||
if (escaping) {
|
||||
// The value is built as bash would build it, but a backslash escape does
|
||||
// not earn substitution credit: backslash is not an escape character in
|
||||
// PowerShell (`"literal \$MESSAGE"` keeps the backslash AND expands the
|
||||
// variable) or cmd, and `BUZZ_SHELL` explicitly supports both. Only
|
||||
// single quoting is literal across all of them, so escape forms stay
|
||||
// conservative and keep #2201's behaviour.
|
||||
if (isSubstitutionChar(char)) hasSubstitution = true;
|
||||
current += char;
|
||||
escaping = false;
|
||||
continue;
|
||||
|
||||
Reference in New Issue
Block a user