Skip to content
This repository was archived by the owner on Aug 27, 2026. It is now read-only.

fix(guard): scan a command substitution in its own quote context (169) - #171

Merged
karlkfi merged 2 commits into
mainfrom
claude/workspace-guard-169-7ce026
Aug 21, 2026
Merged

fix(guard): scan a command substitution in its own quote context (169)#171
karlkfi merged 2 commits into
mainfrom
claude/workspace-guard-169-7ce026

Conversation

@karlkfi

@karlkfi karlkfi commented Aug 21, 2026

Copy link
Copy Markdown
Owner

Fixes #169.

What

strip_heredoc_bodies tracked quote state flat, so a heredoc opened inside a
$(…) or backtick substitution was invisible whenever the substitution sat in
double quotes. Bash opens a fresh quoting context there; the hook did not, so it
stayed in in_double from the opening " onward and the << branch was never
reached.

The body then survived into shlex. With an odd number of " in it the parse
raised No closing quotation, _analyze_command returned its defer tuple, and
nothing in the command was analyzed — including a guarded outside-workspace
path sitting in plain sight at the end of the line:

git commit -aqF "$(cat <<'MSG'
he said "hello
MSG
)" && cp secrets.txt /etc/newfile

That is the shape of an ordinary multi-paragraph commit message. Measured on the
tip before this change, a quote character in an unrelated commit message turned
a hard deny into silence:

trailing command no heredoc even-quote body odd-quote body
cp secrets.txt /tmp/q169-fake deny deny silent
tee /tmp/q169-fake deny deny silent
rm -f /var/tmp/q169-other/x deny deny silent
mv notes.md ~/q169-fake.md ask ask silent

How

Push the enclosing quote state on $( and on a backtick, scan the body
unquoted, restore it on the terminator. Paren depth is tracked per context so a
subshell's ) does not end the substitution early.

Recursing into $(…) alone is not sufficient, as the issue notes:
_scan_dollar_paren tracks quotes flat too, so the body's " swallows its
closing paren and there is nothing to recurse into. Stripping the body first
hands that scan a balanced string, so it needs no change — the fix stays in one
function.

Backticks are the same bug one character away ("cat <<'M' … M" was silent
too), so both openers are handled rather than only the reported one.

Unchanged by design: $((…)) still reads as arithmetic and never arms a
delimiter; a << in quotes or in a # comment is still not a heredoc; and the
expanded list still collects unquoted-delimiter bodies wherever they sit, so
the Q35 command-substitution scan is unaffected.

Testing

python3 scripts/run-tests.py — 1311 tests, OK (8 skipped).

12 new tests. Nine assert the moved behavior and were confirmed to fail against
the pre-fix script, each with the reported symptom (expected a decision, got defer); three are regression fences that hold on both sides, pinning the cases
where the fresh context must not reach — "$((1<<2))", "$(grep -c '<<' f)",
"$(echo "a<<b")".

Also checked by hand that the change only ever removes heredoc bodies: for a
dozen substitution-heavy commands with no heredoc in them, strip_heredoc_bodies
returns its input unchanged.

Docs

README.md — the "How it works" tokenization step and the heredoc entry under
Limitations both now state that a substitution body is scanned in its own
quoting context.

Known residual

A case pattern's ) inside a quoted substitution still pops the context
early, so this narrower shape stays silent (measured on this branch):

echo "$(case $x in a) cat <<'M'
he said "hi
M
;; esac)" && cat /etc/q169-fake

Closing it needs case/esac keyword tracking, and mis-detecting those
over-strips — the unsafe direction — so it is not a follow-on one-liner.
Queued as Q81 rather than bundled here.

Release note

A heredoc inside a quoted command substitution — git commit -F "$(cat <<'MSG' … MSG)" — no longer hides the rest of the command: an odd number of quotes in the message body used to abort the parse, silently skipping an outside-workspace path later on the line.

`strip_heredoc_bodies` tracked quote state flat, so a heredoc opened
inside `$(…)` or backticks was invisible whenever the substitution sat
in double quotes — the shape a multi-paragraph commit message takes:

    git commit -aqF "$(cat <<'MSG'
    he said "hello
    MSG
    )" && cp secrets.txt /etc/newfile

The enclosing `"` kept the scanner in `in_double` from the opening quote
onward, so the `<<` branch was never reached, the body survived into
shlex, and its lone `"` raised `No closing quotation`. `_analyze_command`
caught that and returned its defer tuple, so nothing in the command was
analyzed — including the outside-workspace `cp` target at the end of it.
An `rm` that hard-denies went silent because of a quote in an unrelated
commit message.

Bash opens a fresh quoting context inside a substitution, so do the
same: push the enclosing state on `$(` and on a backtick, scan the body
unquoted, and restore on the terminator. Paren depth is tracked per
context so a subshell's `)` does not end the substitution early.
Recursing alone was not enough — `_scan_dollar_paren` tracks quotes flat
too, and the body's `"` swallowed its closing paren — but stripping the
body first leaves that scan a balanced string, so it needs no change.

`$((…))` still reads as arithmetic and never arms a delimiter, and the
`expanded` list still collects unquoted-delimiter bodies wherever they
sit, so the Q35 substitution scan is unaffected.
@karlkfi
karlkfi force-pushed the claude/workspace-guard-169-7ce026 branch from 7f39100 to d657514 Compare August 21, 2026 05:22
@karlkfi
karlkfi merged commit 7558056 into main Aug 21, 2026
8 checks passed
@karlkfi
karlkfi deleted the claude/workspace-guard-169-7ce026 branch August 21, 2026 05:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: a heredoc inside "$(…)" keeps its body, so an unbalanced quote in a commit message defers the whole command

1 participant