Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/reference/repo-gotchas.md
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,15 @@ builder's self-check ("0 bypasses in 4032") was wrong each time.
`opaque-var-verb` asks and 98 `source-file` asks. Those three ship shadow. Residue: zsh does not split
$IFS (bash and dash do); `. file` with no candidate word never reaches python; `mk$(true)fs` matches,
`${X}` as argv0 does not read as mkfs.
- **shlex cuts one shell word into several tokens** (GH #375). With `punctuation_chars` and no
`whitespace_split`, a character outside its wordchars ends the token: `$PWD` is `$`, `PWD`, and `a:b`,
`a@b.c` and a Thai path split too. A check that takes "the next token" as one word misreads it: the git
global walk took `PWD` in `git -C $PWD reset --hard` for the subcommand. `_tokens` now returns a token
cut from the one before it, with no blank between, as `_Glued` (an equal `str`), and the git block
re-reads its global flags as joined words in a window of its own; the token reading stays, so a deny in
either wins. Other value-taking walks (sudo `-u`, env `-u`, `timeout`'s duration, a `X=$Y` prefix)
still read one token (GH #382). Replay of 115,308 real commands (main and agent): 3 newly denied, two
that run `reset --hard` / `checkout --` and one `git -C $PCR add .` (git-add-all, not data loss).
- **Oracle:** run the real shells (the system `/bin/sh`, bash 3.2, a current bash, dash, zsh, ksh)
with argv-logging stubs, never one shell or argv0-based shell guessing: whether `{fd}>` is a
redirect depends on the binary and version (GH #219).
Expand Down
54 changes: 53 additions & 1 deletion hooks/gates/irrecoverable.py
Original file line number Diff line number Diff line change
Expand Up @@ -1093,13 +1093,31 @@ def _blanked(c, named_fd=True):
# charges a fresh depth-scan budget and may set _DEPTH_BUDGET_BLOWN.
return _blank_redirections(_blank_substitutions(_newlines_to_seps(_normalize_ansi_c_quotes(c))), named_fd)

# GH #375: shlex splits one shell word at each character outside its wordchars ($, :, @, a Thai
# letter): `$PWD` is "$", "PWD". A token that shlex cut from the one before it, with no blank
# between them, comes back as _Glued (an equal str, so every other check reads it unchanged).
class _Glued(str):
__slots__ = ()

def _tokens(src, extra_wordchars="", whitespace_split=False):
# shlex tokens of a _blanked() text; the placeholders are word characters. Raises ValueError.
lex = shlex.shlex(src, posix=True, punctuation_chars=True)
lex.wordchars += PH + HASH_LIT + PSUB + extra_wordchars
if whitespace_split:
lex.whitespace_split = True
return list(lex)
return list(lex)
out, glued = [], False
while True:
tok = lex.get_token()
if tok is None:
return out
out.append(_Glued(tok) if glued else tok)
# A token cut at a non-word character pushes that character back; a one-character token
# ("$") ends without reading on. Either way the next token is glued unless a blank comes first.
back = getattr(lex, "_pushback_chars", ())
end = lex.instream.tell() - len(back)
glued = bool(back) or (end < len(src) and src[end - 1] not in lex.whitespace
and src[end] not in lex.whitespace)

# shlex.split() only recognizes ;/&&/||/|/& as separators when whitespace
# surrounds them ("echo hi;rm -rf x" glued "hi;rm"); punctuation_chars=True
Expand Down Expand Up @@ -1642,6 +1660,33 @@ def _bundled_flag(t, stop_chars, flag="f"):
return True
return False

def _git_words(rest):
# GH #375: git's leading global flags with each glued token run (_Glued) joined into one shell
# word, then the subcommand and its arguments unchanged; None when no flag or value was glued.
# The same walk as the git block: a value-taking global takes the next word.
out, i, n, joined = [], 0, len(rest), False
def word():
nonlocal i, joined
j = i + 1
while j < n and isinstance(rest[j], _Glued):
j += 1
if j - i > 1:
joined = True
t, i = "".join(rest[i:j]) if j - i > 1 else rest[i], j # join once: a long glued run stays linear
return t
while i < n:
start, was_joined = i, joined
t = word()
if not t.replace(PH, "").startswith("-"):
# The subcommand stays as tokens and its own glue does not count: the window this
# returns would be the same window again, appended forever.
i, joined = start, was_joined
break
out.append(t)
if t.replace(PH, "") in GIT_VALUE_GLOBALS and i < n:
out.append(word())
return out + rest[i:] if joined else None

def _sets_hooks_path(w):
# Does a `-c KEY=VAL` / `-cKEY=VAL` / `--config-env KEY=VAR` / `--config-env=KEY=VAR` word in
# window w set core.hooksPath (key case-insensitive) to a non-empty value?
Expand Down Expand Up @@ -1933,6 +1978,13 @@ def _check_window(_wi, w):
# this; this arm predated it and was missed.
if _sets_hooks_path(w):
deny("-c core.hooksPath=<path> re-points git at a different hooks dir — same bypass as --no-verify", rule="git-hooks-path")
# GH #375: the global flags re-read as shell words (glued tokens joined), so `-C $R`,
# `-C$R`, `--git-dir=$G` and `-c k=a@b` hold one value, are checked as a window of their
# own. This window keeps the token reading below: a deny in either wins.
_gw = _git_words(rest)
if _gw:
_WDEPTH[len(windows)] = _cur_depth
windows.append(["git"] + _gw)
# Walk past leading global flags so ` git -C /repo push --force`
# (or -Cpath, --no-pager) does not set sub="-C" and bypass the gate.
i = 0
Expand Down
5 changes: 3 additions & 2 deletions tests/hooks/fixtures/irrecoverable-336-cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ ALLOW IFS=: read -r a b <<< "x:y"; echo "$a"
ALLOW sh -c 'printf %s ${IFS:-"}" rm'
#
# 2. a verb held in a variable assigned earlier on the line. var-verb ships shadow: in a 55,000-command
# replay it denied one real command (`git -C $R checkout -- <file>`, a true discard develop misses).
# replay it denied one real command (`git -C $R checkout -- <file>`, a true discard develop missed).
# GH #375 now denies that command through the git rules, before any view runs.
WOULD_DENY var-verb X=rm; $X -rf build
WOULD_DENY var-verb X=rm && $X -rf build
# control: develop already denies a ${..} argv0 (it reads as every dangerous verb)
Expand All @@ -43,7 +44,7 @@ WOULD_DENY var-verb X=mkfs.ext4; $X /dev/sda1
WOULD_DENY var-verb X=rm; bash -c "$X -rf build"
WOULD_DENY var-verb X=rm; echo hi; $X -rf build
WOULD_DENY var-verb X=rm; $X -rf build; X=ls
WOULD_DENY var-verb R=/tmp/repo; git -C $R checkout -- notes.txt
DENY R=/tmp/repo; git -C $R checkout -- notes.txt
ALLOW X=1; echo $X
ALLOW R=/tmp/x; RW=$R/w; git -C $RW remote add origin git@example.com:o/r.git
ALLOW X=rm; echo $X -rf build
Expand Down
76 changes: 76 additions & 0 deletions tests/hooks/fixtures/irrecoverable-375-cases.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# GH #375 cases for hooks/gates/irrecoverable.sh, replayed by tests/hooks/test-gates.sh.
# DENY <cmd>: exit 2. ALLOW <cmd>: exit 0, empty stdout, no journal row.
# shlex splits one shell word at a character outside its word characters ($, :, @, a Thai letter),
# so a git global's value (-C $R) took the variable's name for the subcommand. Each DENY shape in 1-5 was
# run in /bin/bash 3.2, bash 5, /bin/dash and /bin/zsh with the variables set, against a logging git stub,
# and ran a destructive git argv in each.
#
# 1. an unquoted variable in a -C value, spaced or glued
DENY git -C $PWD reset --hard
DENY git -C $HOME/x push --force origin main
DENY git -C $R clean -fd
DENY git -C $R checkout -- f.txt
DENY git -C$R reset --hard
DENY git -C $A$B reset --hard
DENY git -C ~/$R reset --hard
DENY git -C $R/sub restore f.txt
DENY git -C "$R"$S reset --hard
DENY git -C $(pwd)/$R reset --hard
DENY git -C ${R}x$S reset --hard
DENY git -C $1 reset --hard
DENY git --no-pager -C $R push -f origin main
DENY R=/tmp/r; git -C $R checkout -- f.txt
# 2. every destructive git rule after the variable
DENY git -C $R branch -D main
DENY git -C $R stash drop
DENY git -C $R stash clear
DENY git -C $R switch -f main
DENY git -C $R commit --amend -m x
DENY git -C $R commit -n -m x
DENY git -C $R add -A
DENY git -C $R push origin +main
# 3. the other value-taking globals
DENY git --git-dir=$G reset --hard
DENY git --git-dir $G/.git --work-tree $W reset --hard
DENY git --work-tree=$W checkout .
DENY git --namespace=$N push --force origin main
DENY git -c user.name=$N reset --hard
DENY git -c core.hooksPath=$H commit -m x
DENY git -ccore.hooksPath=$H commit -m x
DENY git --config-env=core.hooksPath=$H commit -m x
# 4. other characters shlex splits a word at
DENY git -c user.email=a@b.c commit --amend -m x
DENY git -C a:b reset --hard
DENY git -C ~/โปรเจกต์ reset --hard
DENY git -C a$ reset --hard
DENY git -C ~/my\ $R reset --hard
# 5. the same through a wrapper
DENY sudo git -C $R reset --hard
DENY env A=1 git -C $R reset --hard
DENY echo f | xargs git -C $R reset --hard
DENY find . -maxdepth 0 -exec git -C $R reset --hard \;
# 6. an unquoted -C value now reads like the quoted one ("$R"). develop denies `git checkout $BR`:
# bash and dash split BR="HEAD f.txt" into a tree-ish and a path.
DENY git -C $R checkout $BR
# 7. the token reading stays beside the word reading, so no develop deny is lost (not destructive:
# with reset unset, git gets `-C --hard`)
DENY git -C $reset --hard
#
# controls
ALLOW git -C $REPO status
ALLOW git -C "$d" log
ALLOW git -C $x diff
ALLOW git -C $R log --oneline -5
ALLOW git -C $HOME/x fetch origin
ALLOW git -C $R push origin main
ALLOW git -C $R checkout -b feat
ALLOW git -C $R reset HEAD~1
ALLOW git -C $R stash list
ALLOW git -C $R commit -m "reset --hard"
ALLOW git --git-dir=$G rev-parse HEAD
ALLOW git -c user.email=a@b.c commit -m x
ALLOW git -c core.hooksPath= status
ALLOW git -C ~/โปรเจกต์ status
ALLOW git -C a:b diff
ALLOW git -C $ status
ALLOW for d in */; do git -C $d status --short; done
34 changes: 34 additions & 0 deletions tests/hooks/test-gates.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3020,6 +3020,40 @@ else
echo " ❌ GH #336: corpus replayed $_got336 of $_want336 cases" >&2; fail=$((fail + 1))
fi

# GH #375: a git global's value split by shlex at $, :, @ or a non-ASCII letter (-C $R) took the
# variable's name for the subcommand. Cases (each deny shape checked in real shells) live in a fixture.
_c375="$ROOT/tests/hooks/fixtures/irrecoverable-375-cases.txt"
_want375=$(/usr/bin/grep -cE '^(DENY|ALLOW) ' "$_c375"); _got375=0
while IFS= read -r _line; do
case "$_line" in
DENY\ *) test_deny "$IRRECOVERABLE" "GH #375: ${_line#DENY }" "$(bash_payload "${_line#DENY }")" ;;
ALLOW\ *)
: > "$_JOURNAL_TMP/a375.jsonl"
test_allow "$IRRECOVERABLE" "GH #375 control: ${_line#ALLOW }" "$(bash_payload "${_line#ALLOW }")" "MH_GATE_JOURNAL_PATH=$_JOURNAL_TMP/a375.jsonl"
if [ -s "$_JOURNAL_TMP/a375.jsonl" ]; then
echo " ❌ GH #375 control journaled a row: ${_line#ALLOW }" >&2; fail=$((fail + 1))
fi ;;
*) continue ;;
esac
_got375=$((_got375 + 1))
done < "$_c375"
# The re-read global part is one more window per git window; a hook timeout allows.
_timed_case "GH #375: 4000 '-C \$R' globals before reset --hard, denied in bounded time" 2 \
"git $(printf -- '-C $R %.0s' $(seq 4000))reset --hard"
_timed_case "GH #375: 4000 '-C \$R' globals before status, allowed in bounded time" 0 \
"git $(printf -- '-C $R %.0s' $(seq 4000))status"
_timed_case "GH #375: reset --hard after 2000 benign '-C \$R' git statements, denied in bounded time" 2 \
"$(printf 'git -C $R status; %.0s' $(seq 2000))git -C \$R reset --hard"
_timed_case "GH #375: glue only inside the subcommand word appends no window, allowed in bounded time" 0 \
"git -C \$R status\$X"
_timed_case "GH #375: 20 KB value of glued \$a\$b words before reset --hard, denied in bounded time" 2 \
"git -C $(printf '$a%.0s' $(seq 10000)) reset --hard"
if [ "$_want375" -ge 50 ] && [ "$_got375" = "$_want375" ]; then
echo " ✅ GH #375: corpus replayed every case ($_got375)"; pass=$((pass + 1))
else
echo " ❌ GH #375: corpus replayed $_got375 of $_want375 cases" >&2; fail=$((fail + 1))
fi

echo ""
total=$((pass + fail))
echo "=== $pass/$total passed ==="
Expand Down
Loading