From 6468eda49533dd4de0aff942e874da0cbf1cd069 Mon Sep 17 00:00:00 2001 From: bchanot Date: Wed, 7 Oct 2026 12:34:56 +0200 Subject: [PATCH] fix(push-guard): fail closed on token floods, git failures and quoted cd targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hardening after the security gate on a2ac018 (3 MEDIUM, all closed and re-measured): - dir tokens are deduplicated and capped: more than 20 distinct cd/-C targets in one command denies before any git fork (20000 tokens: 0.15 s against the 10 s hook timeout that used to turn a flood into an allow) - a git or cd failure while reading gitflow.autopush denies instead of reading as auto (git absent, usage error, unenterable dir); the key being unset is the only "auto" answer; the decision is recorded only after one candidate was evaluated cleanly, else the EXIT trap denies - cd/pushd/-C targets that follow a quote or backtick (bash -c '…') are extracted; quote characters are excluded from unquoted tokens User decision (contract, gated): both fail-closed cases also fire in auto mode on such pathological commands; silence in auto mode holds for every ordinary push. Header limits list the residual misses (quotes or backslashes inside a token, cumulative relative cd, unparseable payload) backed by the soft_deny rule. Tests: 71 checks (T48–T50b added). --- hooks/push-guard.sh | 80 +++++++++++++++++++++++++----------- lib/tests/push-guard.test.sh | 36 ++++++++++++++++ 2 files changed, 92 insertions(+), 24 deletions(-) diff --git a/hooks/push-guard.sh b/hooks/push-guard.sh index a3c86f7..12f96ed 100644 --- a/hooks/push-guard.sh +++ b/hooks/push-guard.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # push-guard.sh — PreToolUse (Bash|Monitor): refuse `git push` in manual # push mode (BDR-111). Manual mode = `gitflow.autopush` reads false (or is -# unparseable: fail closed) in the payload cwd or in any literal -C / cd dir +# unparseable or unreadable: fail closed) in the payload cwd or in any literal -C / cd dir # the command names; outside a repo `git config` reads global/system. # # Deny form: JSON on stdout, exit 0 (hookSpecificOutput.permissionDecision @@ -14,9 +14,13 @@ # a `git` token (git subtree push, git stash push, git log -S "git push", # grep -rn "git push" skills/, git config --get push.default, git add # push.sh, git help push, a commit message quoting "git push"). -# MISSES: "git" push, git "push"; ~ / $VAR / $(...) in -C or cd (never -# resolved, never eval'd); --git-dir / GIT_DIR; a push hidden in a script, -# Makefile target or user alias (the soft_deny rule covers those). +# MISSES: "git" push, git "push", git pu\sh, git $'push', $g push; ~ / $VAR / +# $(...) in -C or cd (never resolved, never eval'd); --git-dir / GIT_DIR; a +# push hidden in a script, Makefile target, user alias or an alias planted +# by a redirect into .git/config; cumulative relative `cd a && cd b` (each +# dir is resolved from cwd, not from the previous cd). LIMITS: more than 20 +# distinct cd/-C dir tokens in one command is refused outright. The +# soft_deny rule covers every miss above. set -u unset CDPATH @@ -68,37 +72,53 @@ unquote() { } # arg_tokens: the directory argument of every `cd`/`pushd`/`-C` in the text. +# A quote or backtick may precede the command word (bash -c 'cd d && ...'). arg_tokens() { - local arg='(--[[:space:]]+)?("[^"]*"|'"'[^']*'"'|[^[:space:];&|()]+)' + local pre='[[:space:];&|()"'"'"'`]' + local arg='(--[[:space:]]+)?("[^"]*"|'"'[^']*'"'|[^[:space:];&|()"'"'"'`]+)' { - printf '%s' "$one" | grep -oE "(^|[[:space:];&|()])(cd|pushd)[[:space:]]+$arg" - printf '%s' "$one" | grep -oE "(^|[[:space:]])-C[[:space:]]+$arg" - } | sed -E 's/^[[:space:];&|()]*(cd|pushd|-C)[[:space:]]+(--[[:space:]]+)?//' + printf '%s' "$one" | grep -oE "(^|$pre)(cd|pushd)[[:space:]]+$arg" + printf '%s' "$one" | grep -oE "(^|$pre)-C[[:space:]]+$arg" + } | sed -E "s/^$pre*(cd|pushd|-C)[[:space:]]+(--[[:space:]]+)?//" } -# candidates: cwd, then each literal dir the command names (resolved from -# cwd; unresolvable ones are skipped, never an allow). +# resolve_dir : absolute dir for a literal token, from cwd. A missing +# dir yields nothing (skipped); an existing but unenterable one yields its +# path so mode_in fails closed on it. +resolve_dir() { + ( + cd -- "$cwd" 2>/dev/null || exit 1 + [ -d "$1" ] || exit 1 + if cd -- "$1" 2>/dev/null; then pwd -P; exit 0; fi + case "$1" in /*) printf '%s\n' "$1" ;; *) printf '%s/%s\n' "$PWD" "$1" ;; esac + ) +} + +# candidates: cwd, then each distinct literal dir of $tokens, deduplicated +# after resolution (unresolvable ones are skipped, never an allow). candidates() { - local tok dir + local tok printf '%s\n' "$cwd" - arg_tokens | while IFS= read -r tok; do + printf '%s\n' "$tokens" | while IFS= read -r tok; do tok=$(unquote "$tok") case "$tok" in ''|-) continue ;; esac - dir=$( cd -- "$cwd" && cd -- "$tok" 2>/dev/null && pwd -P ) || continue - printf '%s\n' "$dir" - done + resolve_dir "$tok" + done | sort -u } -# mode_in : prints `manual`, `invalid:` or nothing. +# mode_in : prints `manual`, `auto` (key unset or true), +# `invalid:` (not a boolean) or `failed:` (git or cd failed). mode_in() { ( - cd -- "$1" || exit 0 - raw=$(git config gitflow.autopush 2>/dev/null) - val=$(git config --bool --default true gitflow.autopush 2>/dev/null) - [ "$val" = false ] && echo manual - [ -n "$raw" ] && ! git config --bool gitflow.autopush >/dev/null 2>&1 \ - && echo "invalid:$raw" - exit 0 + cd -- "$1" 2>/dev/null || { echo "failed:cannot enter the directory"; exit 0; } + val=$(git config --bool gitflow.autopush 2>/dev/null); rc=$? + case "$rc" in + 0) if [ "$val" = false ]; then echo manual; else echo auto; fi ;; + 1) echo auto ;; + *) raw=$(git config gitflow.autopush 2>/dev/null) + if [ -n "$raw" ]; then echo "invalid:$raw" + else echo "failed:git exited $rc"; fi ;; + esac ) } @@ -111,6 +131,14 @@ deny() { exit 0 } +# Cap the distinct dir tokens before resolving any (hook timeout is 10 s). +tokens=$(arg_tokens | sort -u) +ntok=$(printf '%s\n' "$tokens" | grep -c .) +if [ "$ntok" -gt 20 ]; then + deny "push-guard: too many directory tokens in one command ($ntok > 20) — push refused (fail closed). Split the command, or run it yourself: ! $cmd" +fi + +evaluated=0 while IFS= read -r dir; do mode=$(mode_in "$dir" | head -n 1) case "$mode" in @@ -118,8 +146,12 @@ while IFS= read -r dir; do deny "push-guard: manual push mode (gitflow.autopush=false in $dir) — Claude never pushes. Run it yourself in the terminal: ! $cmd" ;; invalid:*) deny "push-guard: gitflow.autopush='${mode#invalid:}' is not a boolean in $dir — treated as manual push mode (fail closed). Fix the value by hand, or run it yourself: ! $cmd" ;; + failed:*) + deny "push-guard: could not read gitflow.autopush in $dir (${mode#failed:}) — git failed, push refused (fail closed). Run it yourself in the terminal: ! $cmd" ;; + auto) evaluated=$((evaluated + 1)) ;; esac done < <(candidates) -decided=1 +# Zero cleanly evaluated candidates: leave decided=0, the EXIT trap denies. +[ "$evaluated" -gt 0 ] && decided=1 exit 0 diff --git a/lib/tests/push-guard.test.sh b/lib/tests/push-guard.test.sh index 08549e2..feef387 100644 --- a/lib/tests/push-guard.test.sh +++ b/lib/tests/push-guard.test.sh @@ -145,6 +145,42 @@ nocwd=$(jq -n '{tool_input:{command:"git push"}}') out=$(cd "$M" && printf '%s' "$nocwd" | bash "$H" 2>/dev/null) check T39-no-cwd "$(jq -r '.hookSpecificOutput.permissionDecision' <<<"$out")" deny +# ── hardening: candidate cap, git failure, quote-prefixed cd ── +cmd48=""; for i in $(seq 1 25); do cmd48="${cmd48}cd /x$i;"; done +run "$cmd48 git push" "$WORK/auto" +check T48-cap-deny "$(verdict)" deny +check T48-cap-reason "$(grep -c 'too many directory tokens' <<<"$(reason)")" 1 +cmd48b=""; for i in 1 2 3 4 5; do cmd48b="${cmd48b}cd \"$M\";"; done +run "$cmd48b git push" "$WORK/plain" +check T48b-dedup-detect "$(verdict)" deny +check T48b-manual-reason "$(grep -c 'manual push mode' <<<"$(reason)")" 1 + +# T49: git absent from PATH: the mode cannot be read, so deny (fail closed). +mkdir -p "$WORK/nogit" +for tool in bash cat grep sed tr jq dirname basename mktemp head sort wc; do + real=$(command -v "$tool") || continue + case "$real" in /*) ln -sf "$real" "$WORK/nogit/$tool" ;; esac +done +payload49=$(jq -n --arg c 'git push' --arg d "$M" \ + '{tool_input:{command:$c},cwd:$d}') +out49=$(printf '%s' "$payload49" | PATH="$WORK/nogit" "$(command -v bash)" "$H" 2>/dev/null); rc49=$? +check T49-rc "$rc49" 0 +check T49-deny "$(jq -r '.hookSpecificOutput.permissionDecision' <<<"$out49")" deny +check T49-reason "$(jq -r '.hookSpecificOutput.permissionDecisionReason' <<<"$out49" | grep -cE 'git|internal error')" 1 + +# T49b: an existing but unenterable candidate dir fails closed. +mkdir -p "$WORK/locked"; chmod 000 "$WORK/locked" +if [ -r "$WORK/locked" ] || (cd "$WORK/locked" 2>/dev/null); then + echo "SKIP T49b-unreadable (chmod 000 ineffective for this user)" +else + check T49b-unreadable "$(fire "cd \"$WORK/locked\" && git push" "$WORK/plain")" deny +fi +chmod 755 "$WORK/locked" + +# T50: a cd that follows a quote is still extracted. +check T50-bash-c-cd "$(fire "bash -c 'cd \"$M\" && git push'" "$WORK/plain")" deny +check T50b-unquoted-arg "$(fire "bash -c 'cd $M && git push'" "$WORK/plain")" deny + # ── settings.json wiring (file content only) ── S="$ROOT/settings.json" check T40-wiring "$(jq -e '.hooks.PreToolUse[]