diff --git a/docs/reference/repo-gotchas.md b/docs/reference/repo-gotchas.md index 0e659b1c8..c818712cd 100644 --- a/docs/reference/repo-gotchas.md +++ b/docs/reference/repo-gotchas.md @@ -179,3 +179,9 @@ builder's self-check ("0 bypasses in 4032") was wrong each time. drift test, and a new word needs two awkward-shape cases per consumer: a glued `name=1`, and a multi-statement decoy (` x && git stash && git status`), because a wider wrapper list can also hide the statements after it (the guard's walk ran across `;` to the last `git`). + GH #320: each word may also be written as a path (`/usr/bin/env git stash`), so every copy takes a + directory prefix, and the drift test checks `/usr/bin/` too. The prefix is one plain word that + never starts with `-` and holds no `=`, `<` or `>`. The git word's `\S*/` was tried first and was + exponential: a token such as `-/command` or `2>/x/env` then reads two ways (flag or path command, + redirection or path wrapper), and the chain loops try every split (`eval command ` + `-/command ` x 40 + ran past 15 s, and a timed-out hook allows). diff --git a/hooks/gates/irrecoverable.py b/hooks/gates/irrecoverable.py index 7499e6d62..d00251b2e 100755 --- a/hooks/gates/irrecoverable.py +++ b/hooks/gates/irrecoverable.py @@ -248,7 +248,11 @@ def _mid_merge(): # wrapper nor an ordinary token, the regex dead-ends and the anchor never fires. # GH #248: `xargs claude -p x` runs claude. xargs is a wrapper for the spawn anchor only: adding it # to PREFIX_WRAPPERS would change the unwrap loop and every other consumer of that list. -_WRAPPER_ALT = r"(?:" + "|".join(PREFIX_WRAPPERS + ("xargs",)) + r")(?=\s)" +# GH #320: a wrapper written as a path (`/usr/bin/env claude -p x`) runs the same program; the rule +# loop compares basenames, this anchor matched the bare word only. The prefix is one plain word that +# never starts with `-` and holds no `=`, `<` or `>`, so a token is a path wrapper, a flag or an +# assignment, never two of them, and the walk stays unambiguous (same prefix as subagent-git-guard.py). +_WRAPPER_ALT = r"(?:(?!-)[^\s;&|()<>=]*/)?(?:" + "|".join(PREFIX_WRAPPERS + ("xargs",)) + r")(?=\s)" _WRAPPER_PREFIX = r"(?:" + _WRAPPER_ALT + r"\s+(?:(?!" + _WRAPPER_ALT + r")\S+\s+)*)*" # GH #248: the greedy walk lands on the LAST claude (`time claude -p x; claude --version` anchored # only the second). The lazy twin (`*?`) lands on the FIRST; it runs as one more last pass, so it diff --git a/hooks/gates/subagent-git-guard.py b/hooks/gates/subagent-git-guard.py index e914d5fb1..b99377a2b 100755 --- a/hooks/gates/subagent-git-guard.py +++ b/hooks/gates/subagent-git-guard.py @@ -188,7 +188,13 @@ def _mask(s): _WRAPPER_WORDS = ("env", "command", "nohup", "nice", "time", "sudo", "doas", "xargs", "exec", "setsid", "timeout", "gtimeout", "stdbuf", "ionice") _KEYWORDS = ("!", "if", "elif", "then", "else", "do", "while", "until", "coproc") -_WRAPPER_ALT = r"(?:" + "|".join(_WRAPPER_WORDS) + r")(?=\s)" +# GH #320: a wrapper written as a path (`/usr/bin/env git stash`) runs the same program, so each +# wrapper word (and rtk, command and exec below) takes an optional directory prefix. The prefix is +# one plain word that never starts with `-` and holds no `=`, `<` or `>`: then a token is a path +# wrapper, a flag, an assignment or a redirection, never two of them, so no repeated group here can +# split a run of tokens two ways (that ambiguity is what made the old chain loops exponential). +_PATH = r"(?:(?!-)[^\s;&|()<>=]*/)?" +_WRAPPER_ALT = _PATH + r"(?:" + "|".join(_WRAPPER_WORDS) + r")(?=\s)" _WRAPPER_PREFIX = r"(?:" + _WRAPPER_ALT + r"\s+(?:(?!" + _WRAPPER_ALT + r")\S+\s+)*)*" # GH #248: the greedy walk above lands on the LAST target, so `time git stash; git status` anchored # only the second `git`. The lazy twin (`*?`) lands on the FIRST target after the wrappers. It only @@ -207,8 +213,8 @@ def _mask(s): # other three treat `A=1` as a command name. Only eval takes assignments. _ASSIGN_RUN = r"(?:[A-Za-z_][A-Za-z0-9_]*=\S*[ \t]+)*" _EVAL_PASS = r"eval[ \t]+(?:--[ \t]+)?" + _ASSIGN_RUN -_SHELL_PASS = r"(?:" + _EVAL_PASS + r"|(?:builtin|command|exec)[ \t]+(?:--[ \t]+)?)" -_RTK_PREFIX = r"(?:rtk[ \t]+(?:-\S+[ \t]+)*(?:(?:proxy|run|err|test|summary)[ \t]+(?:-\S+[ \t]+)*)?)" +_SHELL_PASS = r"(?:" + _EVAL_PASS + r"|(?:builtin|" + _PATH + r"(?:command|exec))[ \t]+(?:--[ \t]+)?)" +_RTK_PREFIX = r"(?:" + _PATH + r"rtk[ \t]+(?:-\S+[ \t]+)*(?:(?:proxy|run|err|test|summary)[ \t]+(?:-\S+[ \t]+)*)?)" _CHAIN_PREFIX = r"(?:" + _SHELL_PASS + r"|" + _RTK_PREFIX + r")*" # GH #248: `{ git stash; }` -- a brace group opens a command position (`{` then blank). # GH #273: a chain word may come BEFORE a wrapper (eval sudo git stash, rtk proxy time git stash). @@ -220,12 +226,12 @@ def _mask(s): # The lookahead names the wrapper words minus command/exec: with them, `rtk exec git stash; git status` # lost the anchor develop gave it through _CHAIN_PREFIX (deep-audit whole-picture pass). GH #307: # command/exec are only stepped over (_LEAD_STEP) on the way to a real lead wrapper. -_LEAD_WRAPPER_ALT = r"(?:" + "|".join(w for w in _WRAPPER_WORDS if w not in ("command", "exec")) + r")(?=\s)" +_LEAD_WRAPPER_ALT = _PATH + r"(?:" + "|".join(w for w in _WRAPPER_WORDS if w not in ("command", "exec")) + r")(?=\s)" # GH #307: the lookahead may step over command/exec (with their flags) to reach a lead wrapper # (`eval command env git stash`). Only the lookahead does: the run still ends at the same chain word, and # the walk takes command/exec itself, so no second parse of any run appears. The stepped words are fixed # words and `-flags`, disjoint from each other, so the scan is linear in the stepped span. -_LEAD_STEP = r"(?:(?:command|exec)[ \t]+(?:-\S*[ \t]+)*)*" +_LEAD_STEP = r"(?:" + _PATH + r"(?:command|exec)[ \t]+(?:-\S*[ \t]+)*)*" _LEAD_CHAIN = (r"(?:(?:" + _EVAL_PASS + r"|builtin[ \t]+(?:--[ \t]+)?|" + _RTK_PREFIX + r")+(?=" + _LEAD_STEP + _LEAD_WRAPPER_ALT + r"))?") # GH #285: a redirection (operator + its word) may sit before the command word and hid it # (`}" "$ok" done +# --- (25) GH #320: a wrapper or rtk written as a path (`/usr/bin/env git stash`) runs the same +# program, so it is the same wrapper. Each deny row runs git stash/reset/clean in sh, bash 3.2, +# bash 5, dash, zsh and ksh (logging git stub; the glued-brace row in zsh only); the controls run none. +for _c in \ + '/usr/bin/env git stash' \ + '/usr/bin/nice git reset --hard' \ + '/usr/bin/time git clean -fd' \ + '/usr/bin/command git stash' \ + '/usr/bin/nohup git stash' \ + '/opt/homebrew/bin/timeout 5 git stash' \ + '( { /usr/bin/env git -C . clean -fd; } )' \ + '{/usr/bin/env git stash;}' \ + '/usr/bin/env A=1 git stash' \ + '/usr/bin/env -u X git stash' \ + '/usr/bin/env /usr/bin/nice git stash' \ + 'sudo -u root /usr/bin/env git stash' \ + '/usr/bin/env true && git stash && git status' \ + 'true; /usr/bin/time git stash' \ + 'eval /usr/bin/env git stash' \ + 'eval /usr/bin/command /usr/bin/env git stash' \ + 'eval /usr/bin/command git stash' \ + 'true && eval /usr/bin/command -- git stash pop' \ + 'rtk proxy /usr/bin/command -- git clean -fd' \ + '/opt/homebrew/bin/rtk proxy git stash' \ + '/opt/homebrew/bin/rtk proxy /usr/bin/env git stash' \ + 'rtk proxy /usr/bin/env git stash' \ + 'bash -c "/usr/bin/env git stash"' \ + 'echo $(/usr/bin/env git stash)' \ + '/usr/bin/e\nv git stash' ; do + rc=$(sgg_rc "$_c"); ok=1; [ "$rc" = "2" ] && ok=0 + check "GH #320 denied (wrapper written as a path): $_c" "$ok" +done +for _c in \ + '/usr/bin/env git status' \ + '/usr/bin/env git stash list' \ + 'echo /usr/bin/env git stash' \ + 'ls /usr/bin/env && git status' \ + '/usr/bin/env=1 git stash' \ + '/usr/bin/envy git stash' \ + '/usr/bin/xenv git stash' \ + 'git commit -m "/usr/bin/env git stash"' ; do + rc=$(sgg_rc "$_c"); ok=1; [ "$rc" = "0" ] && ok=0 + check "GH #320 control allowed: $_c" "$ok" +done +# The path prefix is a new regex piece (`\S*/` before every wrapper word); its worst cases must +# still be decided inside 8 s. +for _c in \ + "$(_pad '/usr/bin/env ' 3000)git stash" \ + "eval $(_pad '/usr/bin/command ' 3000)/usr/bin/env git stash" ; do + rc=$(sgg_rc8 "$_c"); ok=1; [ "$rc" = "2" ] && ok=0 + check "GH #320 padded path wrappers decided inside 8 s as deny (rc $rc, ${#_c} bytes): ${_c:0:24}" "$ok" +done +# A prefix that may start with `-` or hold `<`/`>` lets one token read two ways (a flag or a path +# command, a redirection or a path wrapper), and the chain loops then try every split of a run: +# with `\S*/` each of the -/command, -x/rtk and 2>/x/env rows below ran past 15 s. +for _c in \ + "$(_pad '/usr/bin/env ' 3000)ls" \ + "$(_pad ';/' 4000)env ls" \ + "x $(_pad '/' 20000)env ls" \ + "eval $(_pad '/usr/bin/command ' 3000)ls" \ + "eval command $(_pad '-/command ' 40)ls" \ + "eval rtk $(_pad '-/rtk ' 40)ls" \ + "rtk $(_pad '-x/rtk ' 40)ls" \ + "$(_pad '2>/x/env ' 3000)ls" \ + "eval $(_pad 'A=/rtk ' 3000)ls" ; do + rc=$(sgg_rc8 "$_c"); ok=1; [ "$rc" != "124" ] && ok=0 + check "GH #320 padded path wrappers allow shape finishes inside 8 s (rc $rc, ${#_c} bytes): ${_c:0:24}" "$ok" +done + echo "" echo "=== $pass passed, $fail failed ===" [ "$fail" -eq 0 ]