Repository navigation
Agent hooks: an account flag known only at run time gives can't tell; line variables are substituted (#160) - #166
Merged
Merged
Conversation
A pure move, no behaviour change: the Token type and tokenize() leave src/shellwords.ts, which imports tokenize back. shellwords.ts was near the file-size cap, and the fixes for #161, #160, #159 and #162 all touch the tokenizer. AGENTS.md lists the new module. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Failing tests first. Each expected reading is what Git Bash 5.2 passed to an argv printer, run as a script file and under bash -c. They cover Claude Code's commit idiom followed by an az call (an apostrophe, an odd double quote, a parenthesis, a backtick or a comment sign in the message), a backslash-CRLF continuation, a CR inside a word, (( 1 << 3 )) read as a heredoc, and $'...' / $"..." quotes, plus hook-level wrong-pin controls through runHook with the real azure provider and a fake Exec. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The --body value is the substitution's output, which #160 will read differently, so the test checks the bins and the az call only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
readParen looked for the closing parenthesis with its own quote tracking, which knew nothing of heredocs or comments. An apostrophe or an odd double quote in a heredoc inside $( ) (Claude Code's commit idiom) opened a quote that never closed, so no later call on the line was found: `git commit -m "$(cat <<'EOF' ... EOF )" && az group delete ...` was allowed. An unterminated $( also lost its last character. tokenize() now runs on scan(), which can start inside $( ) and stops at the parenthesis that closes it, so quotes, comments, subshells and heredoc bodies inside are read exactly as on the outer line. The inner text is still collected as before, so the change stays inside the tokenizer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In `az group list \` + CRLF + `--subscription other`, the backslash escaped the CR, so the line did not continue and the flag was lost. Git Bash 5.2, run as a script file and under bash -c, drops every CR it reads: after a backslash, in the middle of a word, and inside single or double quotes. Only a $'\r' it decodes keeps one. tokenize() now removes every CR first. That also stops a CR inside a word from splitting it (`--sub` CR `scription`). bash on Linux and macOS keeps CRs; agents there rarely send CRLF, and the call it would run then carries a stray CR argument. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`(( n = 1 << 3 ))` started a heredoc whose body swallowed the next lines,
so the calls on them were never checked. An arithmetic command, a C-style
for ((...)) and a $(( )) expansion are now skipped to their closing "))",
and only the $( ) and backticks inside them are read as commands, as bash
runs them. Where the ")" that closes the first "(" is not followed by
another, bash reads nested subshells, `((az ...) )` or `$((az ...) )`, and
so does cloudpin; a quote inside also falls back to that reading.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`$'--subscription' other` was read as the word `$--subscription`, so the flag was not seen. A $'...' word now has its escapes decoded as bash decodes them (bash(1), QUOTING; checked against Git Bash 5.2): the one-letter escapes, \xHH (one or two hex digits) and \NNN (one to three octal digits). bash keeps the backslash of an escape it does not know, and so does cloudpin. \u, \U, \c, a NUL (where bash ends the word) or a byte past ASCII make the word known only when it runs. A locale string, $"...", reads like "...". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Failing first. `$((` followed by a ")" that is not "))" is tried as arithmetic and then read again as nested subshells, so every level read the levels inside it twice: 20 levels (394 characters) took 5.6 seconds, and the time doubled with each level. The hook's deadline timer can't fire while the parser runs. The test gives 22 levels two seconds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A nested scan depends only on the line and where it starts, so its end is kept in a map for the rest of the line. A $(( that turns out to be nested subshells is read a second time, but the $( ) inside it are not, and 1,000 nested levels (18 KB) now take well under a second instead of doubling with each level. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Checked against Git Bash 5.2. `(( n = a << 2` + newline fails without the arithmetic reader. The other two pass on the old $( ) reader, which ignored heredocs and counted parentheses itself; they guard the new one, where $(( spanning lines after << and a subshell's ) inside "$( )" would hide the call after them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Failing tests first. A word from $( ), backticks, "$@" or a variable not set on the line must reach guard as something it can recognise; an account flag with such a value, or an unquoted one that could expand to flags, is "can't tell". A variable set earlier on the line is substituted, as bash does. Each provider declares the flags its resolve reads, checked against its source. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The tokenizer now notes where each word has an expansion ($NAME, ${...},
"$@", $1, $( ), backticks, <( ), $(( ))), and whether bash may split it.
findInvocations substitutes a variable set earlier on the line (split on
spaces when unquoted, whole when quoted) and passes anything else on as
DYNAMIC_WORD (one word) or DYNAMIC_WORDS (may be several), keeping a flag
name written before it (--context=<...>). An argument known only at run
time used to reach the provider as "" or as the literal $VAR.
A variable from the environment the hook was given only narrows an
unknown one to a single word; its value never enters the arguments.
eval, bash -c, find -exec and script blocks still get the words as typed.
The #156 tests that expected "" for <( ) now expect DYNAMIC_WORD.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each provider now lists the flags its resolve reads (targetFlags). Before resolve, guard stops a call as "can't tell" when one of them has a value known only when the command runs, when a flag name is completed at run time, or when an unquoted expansion or "$@" could split into flags. The message names the flag, never a value, and no such value reaches a CLI. A quoted value elsewhere (--file "$(ls *.zip)") is still allowed, and exempt commands stay exempt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Failing tests first, for the owner's rule: an unquoted $( ) or backticks, alone or joined to text, is one word that is not a flag, so `kubectl logs $(kubectl get pod ... -o name)` and `az resource delete --ids $(az ... -o tsv)` pass when the pin matches. An account flag whose value is $( ), a flag name completed at run time, and an unquoted unknown variable, "$@" or $* still give "can't tell". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner decision: an unquoted $( ) or backticks gives DYNAMIC_WORD, not DYNAMIC_WORDS, so the common `kubectl logs $(kubectl get pod ... -o name)` and `--ids $(az ... -o tsv)` idioms pass when the pin matches. As an account flag's value, or completing a flag name, it is still "can't tell"; an unquoted unknown variable, "$@" and $* still are too. The split-case message no longer mentions $( ) or backticks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 26, 2026
# Conflicts: # src/shell-tokens.ts # src/shellwords.ts
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.
Closes #160. Milestone 1.2.1 (must-fix patch). This is a draft stacked on #164 (base
fix/161-heredoc-quotes-crlf). Merge #164 first, then retarget this PR to main.What changes (agent hooks and
cloudpin explain)"".findInvocationssubstitutes variables set earlier on the same line: an unquoted one is word-split, a quoted one stays whole. This also fixes the false blocks=pinned; az ... --subscription $s.DYNAMIC_WORD(one word) orDYNAMIC_WORDS(may be several), keeping a flag prefix (--context=<marker>).resolve, so no marker ever reaches a CLI. The message names the flag, never a value. It does so when:targetFlagshas a run-time value (--subscription $(cat f),--context="$X",-itn "$NS");-x$(..));$VAR,$*or"$@"could expand to flags.$(...)or backticks counts as one word that is not a flag, sokubectl logs $(kubectl get pod -o name)andaz resource delete --ids $(...)pass when the pin matches. The trade-off accepted is that a$(...)that prints flags is not caught.ProviderDef.targetFlagsis filled for every provider. A test per provider greps itsflagValue/flagreads, so a newly read flag can't be left off the list. CONTRIBUTING's provider checklist mentions it.--subscription ""falls back to the default; aws--profile ""counts as unset; kubectl--context ""and--namespace ""fall back; vercel--scope ""falls back; supabase--project-ref ""falls back; gcloud--project ""errors.Tests
The tests are in the new files
test/shellwords-dynamic-values.test.tsandtest/hook-dynamic-values.test.ts, committed failing first. Hook-level cases run throughrunHook(AGENTS.claude)with every real provider and a fakeExec, and also check that no marker is sent to any CLI. Two existing tests that asserted the buggy""were updated.Typecheck passes. Full suite: 57 files, 1657 passed, 8 skipped, the fuzz test included.
--subscription $(cat sub.txt)and--context $(cat ctx)now carry the marker, andFLAGS="--subscription x"; az group list $FLAGSreads--subscription x.Left open
"$(echo --subscription=x)") is not caught.bash -c "$CMD",eval "$CMD"andcd "$D"with unknown values belong to Agent hooks: fail closed where the parser can't follow the line (design pass for the rest of the 10-26 audit) #163.$_/$env:Xin a pinned call are now "can't tell"; Agent hooks read PowerShell commands with bash rules #159's dialect should read them as one word.--impersonate-service-account/--access-token-fileare gcloud: --impersonate-service-account and --access-token-file run as another identity than the one checked #165.🤖 Generated with Claude Code