fix(guardrails): unwind a guard's exit before running the next guard

.changes/guardrails/fix-run-guards-deep-exit-unwind-7667a1c3.md

@@ -0,0 +1,7 @@
+---
+bump: patch
+---
+
+### Fixed
+
+- **`run-guards.sh` no longer loses a deep guard's deny on a small stack (#7667).** A guard's `exit` ran the next guard from inside the `exit` call, on top of the finished guard's whole stack, so a guard that exited deep (`block-dangerous-git`'s alias walk at its 128-analysis cap) left too little stack for the rest of the chain; the process crashed after the deny was printed (exit 139 on Linux, exit 0 on Git Bash, which does not block the call). `exit` now records the status and unwinds the guard back to the dispatcher's loop, and after a deep exit the rest of the chain runs in a forked process, so a later guard that runs out of stack cannot take the verdict with it: a rest that reports nothing denies the call. Not covered: a stack so small that the first guard's own alias walk overflows it (under about 700 KB on Linux), which needs a smaller walk, not a dispatcher change.

plugins/guardrails/hooks/run-guards.sh

@@ -42,8 +42,8 @@
 #     Bash every fork is a Win32 process creation, so eight command-substitution
 #     subshells were eight processes per tool call. A sourced guard ends by
 #     calling `exit`, which here is a shell function: it records the guard's
-#     status and runs the NEXT guard from inside that call, so the chain never
-#     returns into a guard that has exited. A guard that falls off its end
+#     status and unwinds the guard, so the chain never returns into a guard
+#     that has exited. A guard that falls off its end
 #     returns from `source` and is recorded the same way. Inside a real subshell
 #     (`$(...)`, `( )`, a pipeline, `&`) the function sees a different BASHPID
 #     and exits that subshell as the builtin would. Its `source hook-utils.sh`
@@ -66,13 +66,19 @@
 #     second hard error in a dying shell would end the process with no result.
 #     That subshell reports its documents and status back on its stdout; a
 #     further hard error there costs one more subshell, never the result.
-#   * Running the next guard from inside `exit` keeps the exiting guard's
-#     frames on the stack. Past RUN_GUARDS_MAX_DEPTH frames the rest of the
-#     chain runs in a fresh dispatcher process instead, which reports back the
-#     same way. A rest that reports no status denies the call.
+#   * `exit` does not run the next guard from inside its own call: that would
+#     keep the exiting guard's frames on the stack under every later guard, and
+#     a guard that exits deep (the alias walk's budget deny is about 140 frames
+#     down) leaves too little stack for the rest of the chain on a small one,
+#     where the crash loses the guard's exit 2 (Git Bash reads it as exit 0).
+#     `exit` records the status and unwinds the guard back to the dispatcher's
+#     loop instead. A guard that exited past RUN_GUARDS_MAX_DEPTH frames, and a
+#     guard that dies of a hard error (the rest runs from its EXIT trap), hand
+#     the rest of the chain to a forked process, which reports back the same
+#     way, so a later guard that runs out of stack cannot take the verdict so
+#     far with it. A rest that reports no status denies the call.
 #   * Guards share one process, so a guard's functions and globals remain
-#     defined while the later guards run, and a guard that exits from inside a
-#     function leaves that function's locals visible to them. Every guard
+#     defined while the later guards run. Every guard
 #     assigns what it reads before it reads it; a guard must not read a global
 #     it did not set expecting it unset.
 #
@@ -811,8 +817,8 @@ OUTS=()
 # dispatcher's own abort.
 RUN_GUARDS_CUR=-1
 # The process the chain runs in. `exit` below compares BASHPID against it: a
-# guard's `exit` inside a real subshell must end that subshell, not run the
-# next guard there.
+# guard's `exit` inside a real subshell must end that subshell, not unwind to
+# the dispatcher's loop there.
 RUN_GUARDS_PID=$BASHPID
 RUN_GUARDS_T0=""
 
@@ -832,29 +838,45 @@ hook::require_jq_blocking() {
   exit 2
 }
 
-# `exit` for the sourced guards. The status a guard exits with is recorded
-# and the chain continues from inside this call, so control never returns to
-# the guard. No argument means the status of the guard's last command, as the
-# builtin does.
+# `exit` for the sourced guards. The status a guard exits with is recorded and
+# the guard is unwound, so control never returns to the code after the call:
+# every frame of the guard returns at once, `source` returns to
+# run_guards::run_from, and the next guard starts on that frame's stack, not on
+# top of the exiting guard's. No argument means the status of the guard's last
+# command, as the builtin does.
+#
+# The unwind is a DEBUG trap under extdebug: a trap that returns 2 makes bash
+# simulate a `return` from the function or sourced script the next command
+# belongs to, and the trap fires again for the command after that return, until
+# run_from itself holds the next command; the trap then removes itself.
 # shellcheck disable=SC2329  # invoked by every guard sourced below
 exit() {
   local __rg_rc="${1:-$?}"
   [[ "$BASHPID" == "$RUN_GUARDS_PID" ]] || builtin exit "$__rg_rc"
   ((RUN_GUARDS_CUR >= 0)) || builtin exit "$__rg_rc"
-  run_guards::guard_done "$__rg_rc"
+  RUN_GUARDS_EXIT_RC=$__rg_rc
+  RUN_GUARDS_EXIT_DEPTH=${#FUNCNAME[@]}
+  set -T
+  shopt -s extdebug
+  # Inline, not a function: the first firing is at the guard's deepest frame,
+  # and a call there costs stack the guard may not have left.
+  trap '[[ ${FUNCNAME[0]-} == run_guards::run_from ]] && { trap - DEBUG; shopt -u extdebug; set +T; RUN_GUARDS_UNWOUND=1; } || return 2' DEBUG
+  return 0
 }
 
+RUN_GUARDS_EXIT_RC=0
+RUN_GUARDS_EXIT_DEPTH=0
+RUN_GUARDS_UNWOUND=0
+
 # A guard died of a hard error: the shell is exiting, and the EXIT trap the
 # guard installed through guard::abort_boundary handed the status here (the
 # library's chain slot, _GAB_CONTINUE, set right before the first guard runs
 # and cleared before the merge). A deliberate `exit` in a guard never comes
 # this way; that is the function above. Never returns.
 # shellcheck disable=SC2329  # invoked through _GAB_CONTINUE by the trap handler
 run_guards::guard_died() {
-  RUN_GUARDS_DYING=1
   run_guards::guard_done "$1"
 }
-RUN_GUARDS_DYING=0
 
 # run_guards::record <rc>: the guard at RUN_GUARDS_CUR ended with <rc>. Applies
 # its abort boundary (when it installed one: a stub that never called
@@ -881,35 +903,32 @@ run_guards::record() {
 }
 
 # The call depth past which the rest of the chain does not run on this stack.
-# A guard's `exit` runs the next guard from inside its own call, so the frames
-# below a guard that exits deep (the alias walk exits about 140 frames down)
-# stay under every guard after it. Git Bash's main thread has a 2 MB stack, and
-# a second alias walk on top of the first overflowed it: bash died of an access
-# violation that its parent read as exit 0 with no output, an allow. A normal
-# chain of eleven guards reaches about 45 frames; past this bound the rest runs
-# in a fresh process, whose stack starts empty, as each guard's did when it ran
-# alone.
+# `exit` unwinds its guard, so only a guard that dies of a hard error (its EXIT
+# trap runs the rest) leaves its frames under whatever runs next. Git Bash's
+# main thread has a 2 MB stack, and a second alias walk on top of the first
+# overflowed it: bash died of an access violation that its parent read as exit
+# 0 with no output, an allow. A guard that exited past this depth, and a hard
+# error past it, hand the rest to another process: a fork after the deep
+# `exit`, a fresh dispatcher from the dying shell's trap.
 RUN_GUARDS_MAX_DEPTH=64
 
-# run_guards::guard_done <rc>: record the guard that just ended and run the
-# rest. Never returns.
+# run_guards::guard_done <rc>: record the guard that died of a hard error and
+# run the rest. Never returns.
+# shellcheck disable=SC2329  # invoked by run_guards::guard_died, itself invoked through _GAB_CONTINUE
 run_guards::guard_done() {
   run_guards::record "$1"
   ((RUN_GUARDS_OVERLONG && RC == 2)) && run_guards::finish
   local __rg_next=$((RUN_GUARDS_CUR + 1))
   ((__rg_next < ${#GUARDS[@]})) || run_guards::finish
-  if ((RUN_GUARDS_DYING || ${#FUNCNAME[@]} > RUN_GUARDS_MAX_DEPTH)); then
-    run_guards::run_rest "$__rg_next"
-    run_guards::finish
-  fi
-  run_guards::run_from "$__rg_next"
+  run_guards::run_rest "$__rg_next"
+  run_guards::finish
 }
 
 # run_guards::run_from <index>: run the guards from <index> on, then finish.
-# Never returns: a guard's `exit` continues the chain from inside the call,
-# and a guard that falls off its end is recorded right here.
+# Never returns. A guard that ends through `exit` has been unwound back to the
+# `source` below (see `exit`), so every guard starts on this frame's stack.
 run_guards::run_from() {
-  local __rg_i
+  local __rg_i __rg_rc
   for ((__rg_i = $1; __rg_i < ${#GUARDS[@]}; __rg_i++)); do
     RUN_GUARDS_CUR=$__rg_i
     local __rg_path
@@ -932,18 +951,34 @@ run_guards::run_from() {
     RUN_GUARDS_T0=${EPOCHREALTIME:-}
     # shellcheck disable=SC1090
     source "$__rg_path" </dev/null
-    run_guards::guard_done "$?"
+    __rg_rc=$?
+    if ((RUN_GUARDS_UNWOUND)); then
+      RUN_GUARDS_UNWOUND=0
+      __rg_rc=$RUN_GUARDS_EXIT_RC
+    fi
+    run_guards::record "$__rg_rc"
+    ((RUN_GUARDS_OVERLONG && RC == 2)) && break
+    # A guard that exited this deep may have come close to the stack limit,
+    # and the next guard's own walk can run out of it where the first did not.
+    # In a forked child that crash cannot take this process, and the verdict
+    # already recorded, with it: a rest that reports nothing denies the call.
+    if ((RUN_GUARDS_EXIT_DEPTH > RUN_GUARDS_MAX_DEPTH && __rg_i + 1 < ${#GUARDS[@]})); then
+      RUN_GUARDS_EXIT_DEPTH=0
+      run_guards::run_rest $((__rg_i + 1))
+      break
+    fi
+    RUN_GUARDS_EXIT_DEPTH=0
   done
   run_guards::finish
 }
 
 # run_guards::run_rest <index>: the guards from <index> on, run in another
-# process, from inside a dying shell's EXIT trap or past RUN_GUARDS_MAX_DEPTH.
-# Its collected documents and status come back on its stdout as
+# process, after a guard exited past RUN_GUARDS_MAX_DEPTH or from inside a dying
+# shell's EXIT trap. Its collected documents and status come back on its stdout as
 # <doc>RS ... RS<doc>RS RC=<n>. RS (0x1e) cannot occur inside a document (JSON
 # escapes every control byte).
 #
-# Past the depth bound it is a fresh dispatcher fed the same payload, so the
+# A dying shell past the depth bound gets a fresh dispatcher fed the same payload, so the
 # guards run on an empty stack. That needs a payload the fresh process reads
 # back the same: read whole (stdin rc 0), and primed, so it carried no NUL that
 # the shell variable dropped. Any other payload past the bound is denied:
@@ -964,7 +999,6 @@ run_guards::run_rest() {
     # shellcheck disable=SC2030  # the subshell's own OUTS and RC come back on its stdout
     __rg_rest=$(
       RUN_GUARDS_PID=$BASHPID
-      RUN_GUARDS_DYING=0
       RUN_GUARDS_REPORT=1
       OUTS=()
       RC=0

plugins/guardrails/hooks/run-guards.test.sh

@@ -248,6 +248,24 @@ if (ulimit -s 1024) 2>/dev/null; then
   ) || RC=$?
   assert_exit "attended row, 1 MB stack: reset --hard, then past the alias budget, denies" 2 "$RC"
 fi
+# A guard's `exit` unwinds the guard before the next one runs, and the rest of
+# the chain runs in a forked process after a deep exit, so the deny of a guard
+# that exits 140 frames down survives a stack that small, whether or not the
+# next guard's own walk fits in it: the alias chain alone, through the shipped
+# Bash row, at the default stack and at 1 MB.
+alias_p=$(jq -nc --arg c "git$rg_hops a1" --arg d "$ATT_REPO" '{tool_name:"Bash",tool_input:{command:$c},cwd:$d}')
+for rg_stack in default 1024; do
+  if [[ "$rg_stack" != default ]] && ! (ulimit -s "$rg_stack") 2>/dev/null; then continue; fi
+  RC=0
+  ERR=$(
+    [[ "$rg_stack" == default ]] || ulimit -s "$rg_stack"
+    bash "$DISPATCH" --lib lib/powershell/ps-command.sh \
+      block-no-verify.sh block-dangerous-git.sh block-noncanonical-commit.sh \
+      block-root-delete-target.sh block-credential-read.sh <<<"$alias_p" 2>&1 >/dev/null
+  ) || RC=$?
+  assert_exit "alias chain past the budget, $rg_stack stack: denies" 2 "$RC"
+  assert_contains "alias chain past the budget, $rg_stack stack: the reason is on stderr" "$ERR" "over 128 re-expansions"
+done
 att_run 1 'rm -rf "$X"; rm -rf /etc/foo'
 assert_eq "attended row: two asking segments ask" ask "$(hsd permissionDecision)"
 assert_contains "attended row: the reason names the bare variable" "$(hsd permissionDecisionReason)" "bare variable"
@@ -616,13 +634,13 @@ assert_contains "hard error, closed posture: the fail-closed line is on stderr"
 assert_silent "hard error, closed posture: no stdout document" "$OUT"
 assert_eq "hard error, closed posture: the next guard still ran" $'hardc\ngit status --short\nBash' "$(cat "$SEEN")"
 
-# A guard's `exit` runs the next guard from inside its own call, so a guard
-# that exits deep leaves its frames under every guard after it. Git Bash has a
+# A guard's `exit` once ran the next guard from inside its own call, so a guard
+# that exited deep left its frames under every guard after it. Git Bash has a
 # 2 MB stack: an attended alias walk that exited about 140 frames down, then a
 # second walk, overflowed it, and bash died with exit 0 and no output, an
-# allow. Past the depth bound the rest runs in a fresh process with an empty
-# stack, and its documents and status still count. The depth a later guard
-# sees is the structural pin; it holds on every platform.
+# allow. Now `exit` unwinds the guard, and after a deep exit the rest runs in a
+# forked process; its documents and status still count. The depth a later
+# guard sees is the structural pin; it holds on every platform.
 deep_stub() { # <name> <status>: recurse 100 frames, then exit <status> from the bottom
   stub "$1" 'd() { if (($1)); then d $(($1 - 1)); else exit '"$2"'; fi; }
 d 100'
@@ -634,23 +652,47 @@ stub die.sh 'echo die >>"'"$SEEN"'"; kill -9 "$BASHPID"'
 run "$PAYLOAD" "$TEST_TMPDIR/deep0.sh" "$TEST_TMPDIR/depth.sh" "$TEST_TMPDIR/allow.sh"
 assert_exit "deep exit: the rest of the chain still runs" 0 "$RC"
 deep_seen=$(cat "$SEEN")
-assert_eq "deep exit: the next guard starts on a fresh stack" "depth=3" "${deep_seen%%$'\n'*}"
-assert_eq "deep exit: each later guard ran once" $'depth=3\ngit status --short\nBash' "$deep_seen"
+assert_eq "deep exit: the next guard starts on a fresh stack" "depth=5" "${deep_seen%%$'\n'*}"
+assert_eq "deep exit: each later guard ran once" $'depth=5\ngit status --short\nBash' "$deep_seen"
+stub depth1.sh 'echo "depth=${#FUNCNAME[@]}" >>"'"$SEEN"'"; exit 0'
+run "$PAYLOAD" "$TEST_TMPDIR/deep0.sh" "$TEST_TMPDIR/depth1.sh"
+assert_eq "deep exit: the guard's frames are gone, not under the next guard" "depth=5" "$(cat "$SEEN")"
+stub shallow0.sh 'exit 0'
+run "$PAYLOAD" "$TEST_TMPDIR/shallow0.sh" "$TEST_TMPDIR/depth1.sh"
+assert_eq "shallow exit: the next guard runs on the dispatcher's own stack" "depth=3" "$(cat "$SEEN")"
 run "$PAYLOAD" "$TEST_TMPDIR/deep2.sh" "$TEST_TMPDIR/ask1.sh" "$TEST_TMPDIR/ctx1.sh"
-assert_exit "deep exit 2: the block wins over the fresh process's documents" 2 "$RC"
-assert_absent "deep exit 2: the ask from the fresh process is dropped" "$OUT" '"ask"'
-assert_contains "deep exit 2: the fresh process's context is kept" "$(hsd additionalContext)" "ctx one"
+assert_exit "deep exit 2: the block wins over the forked rest's documents" 2 "$RC"
+assert_absent "deep exit 2: the ask from the forked rest is dropped" "$OUT" '"ask"'
+assert_contains "deep exit 2: the forked rest's context is kept" "$(hsd additionalContext)" "ctx one"
+# A guard that dies of a hard error more than 64 frames down hands the rest to
+# a fresh dispatcher: the only caller of run_rest's deep branch. That needs a
+# payload the fresh process reads back the same; any other is denied.
+stub harddeep.sh 'source "'"$HOOK_DIR"'/abort-boundary.sh"
+guard::abort_boundary harddeep PreToolUse open 0 2
+d() { if (($1)); then d $(($1 - 1)); else : "${RUN_GUARDS_TEST_UNBOUND?forced abort}"; fi; }
+d 100'
+run "$PAYLOAD" "$TEST_TMPDIR/harddeep.sh" "$TEST_TMPDIR/ctx1.sh" "$TEST_TMPDIR/block.sh"
+assert_exit "deep hard error: the fresh dispatcher's block comes back" 2 "$RC"
+run "$PAYLOAD" "$TEST_TMPDIR/harddeep.sh" "$TEST_TMPDIR/ctx1.sh" "$TEST_TMPDIR/allow.sh"
+assert_exit "deep hard error: the fresh dispatcher's status comes back" 0 "$RC"
+assert_contains "deep hard error: the fresh dispatcher's documents come back" "$(hsd additionalContext)" "ctx one"
+# The depth bound is reached here by lowering it, not by recursing: an EXIT trap
+# on Linux bash runs after the frames unwind, so a real deep abort never shows
+# run_rest more than 64 frames. The stub is sourced into the dispatcher, so it
+# calls run_rest the way guard_done does, on a stack the lowered bound counts as deep.
+stub restdeep.sh 'RUN_GUARDS_MAX_DEPTH=0
+run_guards::run_rest $((RUN_GUARDS_CUR + 1))
+run_guards::finish'
+run "not json" "$TEST_TMPDIR/restdeep.sh" "$TEST_TMPDIR/allow.sh"
+assert_exit "deep hard error, unprimed payload: denied" 2 "$RC"
+assert_contains "deep hard error, unprimed payload: the reason names it" "$ERR" "reported no verdict"
 # A rest of the chain that dies before reporting has an unknown verdict, so
-# the call is denied, on the fresh-process path and on the hard-error subshell.
+# the call is denied, on the forked-subshell path and on the hard-error subshell.
 run "$PAYLOAD" "$TEST_TMPDIR/deep0.sh" "$TEST_TMPDIR/die.sh" "$TEST_TMPDIR/allow.sh"
 assert_exit "deep exit, then the rest dies unreported: denied" 2 "$RC"
 assert_contains "deep exit, then the rest dies unreported: the reason names it" "$ERR" "reported no verdict"
 run "$PAYLOAD" "$TEST_TMPDIR/hard.sh" "$TEST_TMPDIR/die.sh" "$TEST_TMPDIR/allow.sh"
 assert_exit "hard error, then the rest dies unreported: denied" 2 "$RC"
-# Past the bound with a payload a fresh process could not read back the same
-# (a malformed one here): the rest does not run, and the call is denied.
-run "not json" "$TEST_TMPDIR/deep0.sh" "$TEST_TMPDIR/depth.sh"
-assert_exit "deep exit on an unprimed payload: denied" 2 "$RC"
 # The dispatcher's own boundary is fail-open, but not once a guard has
 # blocked: an abort in the merge after that denies. The stub makes the merge
 # fail and its fallback die of an unbound variable.