Skip to content

fix(hooks): ward no longer blocks literal mentions of warded paths (#92) - #93

Open
cdrn wants to merge 7 commits into
mainfrom
fix/ward-literal-false-positives
Open

cdrn wants to merge 7 commits into
mainfrom
fix/ward-literal-false-positives

Conversation

@cdrn

@cdrn cdrn commented Sep 13, 2026

Copy link
Copy Markdown
Owner

The Bash ward split a command into statements on every ;, &, |,
$( and backtick, ignoring quotes and heredocs, then treated whatever
word followed as a program. Prose in a commit message or an echo that
merely mentioned a warded path could therefore be blocked as if it
were a read — text that cannot leak a file's contents.

splitStatements now respects shell quoting: separators split only
outside quotes; $( and backticks still start a statement inside
double quotes (they execute there) but not inside single quotes; a
heredoc body is data — fully skipped with a quoted delimiter, and with
an unquoted one only the $(…)/backtick substitutions inside it are
surfaced. Anything that can actually read a file still surfaces as a
statement whose first token is the reader, so every true positive
stays blocked.

Tests: the exact commit-message heredoc that bit during development;
-m prose with separators; printf/echo mentions; substitutions inside
double quotes and unquoted heredocs still blocked; single-quoted and
quoted-heredoc substitutions literal; reads after newlines, ;, &&,
||, pipes and after a heredoc still blocked; heredoc command-line
operands still scanned; splitStatements semantics.

Closes #92.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Sgkb3Gi9A7w33BzfyDti6C

cdrn and others added 7 commits September 12, 2026 20:45
The Bash ward split a command into statements on every `;`, `&`, `|`,
`$(` and backtick, ignoring quotes and heredocs, then treated whatever
word followed as a program. Prose in a commit message or an echo that
merely mentioned a warded path could therefore be blocked as if it
were a read — text that cannot leak a file's contents.

splitStatements now respects shell quoting: separators split only
outside quotes; `$(` and backticks still start a statement inside
double quotes (they execute there) but not inside single quotes; a
heredoc body is data — fully skipped with a quoted delimiter, and with
an unquoted one only the `$(…)`/backtick substitutions inside it are
surfaced. Anything that can actually read a file still surfaces as a
statement whose first token is the reader, so every true positive
stays blocked.

Tests: the exact commit-message heredoc that bit during development;
-m prose with separators; printf/echo mentions; substitutions inside
double quotes and unquoted heredocs still blocked; single-quoted and
quoted-heredoc substitutions literal; reads after newlines, `;`, `&&`,
`||`, pipes and after a heredoc still blocked; heredoc command-line
operands still scanned; splitStatements semantics.

Closes #92.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Sgkb3Gi9A7w33BzfyDti6C
Review round 1 of #93 showed the first quote-aware splitter under-blocked
in sixteen concrete ways. Replaced with a context-stack scanner that
follows Bash's lexical rules: `$(…)` and backticks parsed recursively
with a fresh quote context (nested, multi-line); `#` comments; literal
backslashes inside single quotes; `((…))`/`$((…))` arithmetic opaque;
`<<<` here-strings distinguished from heredocs; heredoc delimiter is the
whole word with any quoting removed (so <<'EOF'X, <<EOF-X, <<E\OF all
terminate correctly), the rest of the opener line is still command
text, `<<-` alone strips tabs, backslash-newline joins lines in an
expanding body, queued heredocs are consumed in order; an expanding
body's substitutions are surfaced (quotes are literal there); a quoted
body is data unless the opener's program is a shell interpreter.
Statements whose program is an interpreter (bash -c, sh, eval, sudo,
env, xargs, …) have their arguments scanned as commands, recursively.
`< file` / `$(< file)` count as reads; `dd` joins the reader list;
backslash-newline is whitespace to the tokenizer.

Tests: every bypass from the review as a named regression (31), plus
the benign prose cases from #92 and a semantics test for the splitter.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Sgkb3Gi9A7w33BzfyDti6C
…y provably inert text

Two rounds of review showed a hand-written Bash lexer under-blocks in
ways a defensive hook cannot afford. This drops that approach entirely:
the original statement scan is back exactly as it was (plus newlines
as separators and backslash-newline joined, both of which only add
refusals), and a pre-pass blanks text that is provably inert before it
runs. Whatever the scanner refused before, it still refuses; the
carve-outs only remove false positives.

Carved:
- the body of a heredoc with a simply quoted delimiter (<<'WORD' or
  <<"WORD", WORD an identifier with nothing glued after the quote) when
  the opener's program, after VAR=value prefixes, is not a shell or
  other interpreter and the opener line neither pipes, substitutes nor
  redirects its output — except `> FILE` where every other mention of
  FILE in the command is an argument to git or gh, which read it as
  text (the commit-message / PR-body workflow). Unterminated, glued,
  unquoted, doubled-up or piped heredocs are left to the scan.
- message strings handed to git or gh (-m, --message, --body, --title,
  --notes and their = forms) when they contain no $( or backtick.

Tests: the commit-message workflow via a file and via stdin; message
strings; substitutions inside messages still refused; bodies feeding
bash/sh/sudo, piped, substituted, written then executed/sourced, or
with unquoted/glued/unterminated/doubled delimiters all still refused;
the constructs from both reviews still refused; the three over-blocks
the lexer introduced now allowed; stripInertText leaves other lines
untouched.

Closes #92.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Sgkb3Gi9A7w33BzfyDti6C
…ne git/gh call (#92)

Three review rounds showed that any heuristic carve-out — a lexer, or
a set of "this text can't be executed" conditions — can be defeated by
a command that controls its own surrounding syntax. So the scanner is
main's, byte for byte, and the pre-pass blanks message text only when
the ENTIRE command matches one of two shapes:

  git|gh <plain args> (-F|--file|--body-file) - <<'EOF'
  <message>
  EOF

  git|gh <plain args> (-m|--message|--body|--title|--notes) '…'|"…" <plain args>

"Plain args" are unquoted words without `=` (which rules out `-c
key=value` overrides such as gpg.program or core.pager) or quoted words
containing nothing the shell or git could act on; the message string
may not contain `$(` or a backtick; the heredoc delimiter must be a
simply quoted identifier and terminate. Because the whole command must
match, no other statement, pipe, redirection, substitution, function,
subshell or second heredoc can be present, so the text has exactly one
consumer, which stores it. Any command that doesn't match exactly is
returned untouched and scanned precisely as before.

The file-based workflow (`cat > msg <<'EOF' … git commit -F msg`) is
still refused when the message mentions a warded path — that shape has
two consumers. Use `git commit -F - <<'EOF'` instead.

Tests: both shapes carved (incl. --amend --file, gh --body-file, a
trailing newline, and a quoted delimiter suppressing `$(`); inline
strings for git/gh; substitutions inside a message still refused;
twenty deviations from the shapes each left byte-identical by the
pre-pass; what the unchanged scanner refused, it still refuses; what it
allowed, it still allows.

Closes #92.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Sgkb3Gi9A7w33BzfyDti6C
… body containing its delimiter

Review round 4: \s in the shapes matched newlines, so a second
statement could follow the git call and consume the message; and the
lazy body match could run past an earlier terminator line, blanking
real commands. Separators are now [ \t]+, the program word is
anchored, and a heredoc body containing its own delimiter line is not
carved. Both reviewed commands are regressions (untouched and refused).

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Sgkb3Gi9A7w33BzfyDti6C
Review round 5: 'git bisect run …' and similar execute their arguments,
so 'git' alone was too broad. The shapes now name the subcommands that
store a message — git commit, and gh pr/issue/release create, comment,
edit, review — with nothing between the program and the subcommand.
Both reviewed commands are regressions (untouched and refused).

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Sgkb3Gi9A7w33BzfyDti6C

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ward hook false positives: sigil paths that appear as literal text are blocked as if read

1 participant