diff --git a/docs/conventions/hook-telemetry/data/block-dangerous-git.schema.json b/docs/conventions/hook-telemetry/data/block-dangerous-git.schema.json index 1072d9679..80039167e 100644 --- a/docs/conventions/hook-telemetry/data/block-dangerous-git.schema.json +++ b/docs/conventions/hook-telemetry/data/block-dangerous-git.schema.json @@ -17,7 +17,7 @@ }, "form": { "type": "string", - "description": "The irreversible git form when blocked: \"push-force\" | \"reset-hard\" | \"clean-force\" | \"checkout-dot\" | \"restore-dot\" | \"checkout-force\", or \"too-long\" when the command exceeded the parse-length ceiling. Empty string when the command was allowed (status ok)." + "description": "The irreversible git form when blocked: \"push-force\" | \"reset-hard\" | \"clean-force\" | \"checkout-dot\" | \"restore-dot\" | \"checkout-force\", or \"too-long\" when the command exceeded the parse-length ceiling, or \"alias-traversal-cap\" when checking the command's git alias chain exceeded the re-expansion budget. Empty string when the command was allowed (status ok)." } } } diff --git a/docs/conventions/hook-telemetry/data/block-noncanonical-commit.schema.json b/docs/conventions/hook-telemetry/data/block-noncanonical-commit.schema.json index a0bcd2641..36975f11d 100644 --- a/docs/conventions/hook-telemetry/data/block-noncanonical-commit.schema.json +++ b/docs/conventions/hook-telemetry/data/block-noncanonical-commit.schema.json @@ -17,7 +17,7 @@ }, "form": { "type": "string", - "description": "The non-canonical commit-message form when blocked (e.g. \"message-flag\" for a bare -m/-F not paired with the canonical marker). Empty string when the command was allowed (status ok)." + "description": "The non-canonical commit-message form when blocked (e.g. \"message-flag\" for a bare -m/-F not paired with the canonical marker), or \"alias-traversal-cap\" when checking the command's git alias chain exceeded the re-expansion budget. Empty string when the command was allowed (status ok)." } } } diff --git a/plugins/guardrails/.claude-plugin/plugin.json b/plugins/guardrails/.claude-plugin/plugin.json index f838cb2f8..566d38645 100644 --- a/plugins/guardrails/.claude-plugin/plugin.json +++ b/plugins/guardrails/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "guardrails", - "version": "0.17.0", + "version": "0.17.1", "description": "Twelve safety guards that block secret/credential writes, hardcoded machine-specific paths, git hook-bypass attempts, irreversible git operations (force-push, reset --hard, worktree-wide checkout/restore discards), Bash file-write workarounds that circumvent Write/Edit hooks, commit subjects and gh pr create titles that violate the repo's tracked team convention (when one is declared in .claude/source-control.md), (advisory) hallucinated CLI flags, (advisory) /plugin:skill references that do not resolve, (advisory) markdown citing a repo path the repo's own history shows was removed, (advisory) un-throttled Workflow fan-out that risks burst 529s, and (advisory) direct git commit/gh pr create calls bypassing this marketplace's own commit/pull-request skills — each independently toggleable.", "author": { "name": "Melodic Software", diff --git a/plugins/guardrails/CHANGELOG.md b/plugins/guardrails/CHANGELOG.md index aeb230218..3a88fe46f 100644 --- a/plugins/guardrails/CHANGELOG.md +++ b/plugins/guardrails/CHANGELOG.md @@ -3,6 +3,263 @@ All notable changes to the `guardrails` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.17.1] + +### Fixed + +- **Git-alias CHAINS no longer bypass the git guards (`block-dangerous-git`, + `block-noncanonical-commit`).** Both guards re-expand an inline/persisted git + alias to re-check the real subcommand, but two coupled defects in that + re-expansion let a dangerous op or a non-canonical commit reached through a + SECOND alias hop slip past (verified rc=0 → fail open): + - **The nested re-parse dropped the command-line globals.** The splice fed the + recursive check words `0..gi` (wrappers + `git`) plus the expansion, dropping + everything between `git` and the subcommand — i.e. the `-c` / `--config` / + `--config-env` options. So the nested hop saw empty config: no second-hop + alias definition and no `--config-env` shape to refuse. The splice now spans + `0..sub_idx`, carrying every command-line global into each hop, so the + already-value-blind `--config-env` shape refusal and the plain/`.command` + max-danger union fire at every depth (closes the `--config-env`-second-hop + manifestation and its `.command`-spelled variant by construction). + - **Re-expansion was capped at one level** on the false premise that git does + not chain aliases (it does — an expansion whose first word is itself an alias + is expanded again). The one-level cap is replaced by a save/restore seen-set + of resolved subcommand names: recursion follows the chain to the real op, and + a repeat is git's own alias-loop stop (nothing runs — allow-safe), with + termination guaranteed by the finite set of distinct alias keys. Covers plain + inline chains, `--config-env` hops, the `alias..command` spelling, and + the commit guard's persisted-config alias chain. + - **A `!` shell-alias body no longer inherits the outer chain's alias-loop + state** (review finding on the fix above). git's alias-loop guard is + in-process only: a `!` alias spawns a NEW git process whose loop guard + starts empty, so a body that re-invokes a name from the outer chain + (`git -c alias.a='!git -c alias.a="reset --hard" a' a`) is re-expanded + there, not stopped. Every `!` reparse now runs under an emptied seen-set + (restored afterwards). Termination: inline definitions reachable from a + reparse are strict substrings of the parent segment's text, and the commit + guard's persisted-config `!` hops — whose bodies never shrink — are bounded + by a second save/restore seen-set of persisted name/expansion pairs, where + a repeat models real git's endless fork of a self-referential persisted + shell alias (`a = !git a`): nothing ever runs, so skipping is allow-safe. + - **Chain traversal is now proportional to the chain's LENGTH, not exponential + in it** (review finding on the fix above). Because each hop re-checks BOTH + alias spellings independently, following the chain to the real op branched 2x + per hop: a *benign* 10-hop, 402-character command cost 5.4s in + `block-dangerous-git`, and an 8-hop, 356-character one cost 14.6s in + `block-noncanonical-commit` (every leaf forked a `git config`) — and a hook + that stalls stops guarding. Two bounds, both guard-local: + - **Equivalent analysis states collapse.** A verdict is a pure function of + (alias seen-set, argv); every other input is invocation-constant, and a + block is a process-wide `exit 2`, so a state reached a second time while + the process still runs provably did not block and cannot decide otherwise + now. Skipping the repeat is exact, not a coverage trade — and it is what + collapses the common shape, where both spellings of a hop expand to the + same thing, to one path per hop. Persisted-alias lookups are cached per + (directory, subcommand) for the same reason, removing the per-leaf fork. + - **A total re-expansion budget, fail-CLOSED.** Collapsing cannot bound a + chain whose two spellings DIFFER, because each path carries its own + trailing text forward and no two states are equal. The ceiling counts + ANALYSES, not seconds — a wall clock is host- and command-length-dependent + — and is calibrated against the linear walk the guards already accept: a + memoized traversal spends one analysis per hop, so a branching walk is + capped at the same order as a long non-branching chain (in + `block-dangerous-git`, at strictly less than the ~430-hop chain its 16 KB + command ceiling admits). It sits far above real usage; every legitimate + command measured spends single digits. Exhausting it blocks: the guard + could not finish deciding, so it must not allow. + - **A persisted `!` alias chain that DESCENDS through nested repositories is no + longer mistaken for a self-cycle** (`block-noncanonical-commit`; review + finding on the fix above). One alias text can mean a different hop in every + repository it appears in: with `alias.a = !git -C child a` in a repository + *and* in its child, plus `alias.a = commit --allow-empty -m bypass` in the + grandchild, real git descends twice and creates the non-canonical commit — + but the cycle key was the name and expansion only, so the second hop read as + a repeat, the walk stopped, and the guard returned 0 (verified fail-open). + The effective repository is now part of that key, and it is COMPOSED across + each `!` reparse rather than restarting from the payload cwd, because a `!` + body runs as a new git invocation from the repository the outer one resolved + — so a relative `-C` inside it stacks. Termination is unchanged where it came + from the set: a body with no `-C` leaves the directory alone, so + `a = !git a` and mutually referential pairs still stop on the first repeat. + A body naming the directory it is already in (`-C .`) would otherwise mint a + fresh key per hop and walk instead of stopping (measured 34.6s); it now + collapses to a repeat (0.8s) via the identity described in the next bullet, + which is also what supplies the `!` body's base — so a body invoked from a + SUBDIRECTORY composes from the outer repository's top level, as git does. + `block-dangerous-git` is not affected — it resolves inline aliases only, with + no persisted lookup and no shell-alias seen-set. + - **The guard no longer MODELS git's path semantics; it asks git** + (`block-noncanonical-commit`; two review findings on the fix above, one root + cause). Modelling resolution in shell text produced a bypass every time it was + attempted, in both directions: + - **Lexical `x/..` cancellation is wrong when `x` is a symlink.** With + `base/link -> target/child`, `git -C link/.. …` enters `target` on a POSIX + host, but textual cancellation reduced the lookup to `base` — so a + `commit -m` alias in `target` went unseen. + - **Resolving physically instead would be just as wrong, with the opposite + bias.** Verified on git 2.54.0.windows.1: `cd -P link/..` reports the link + target's parent while `git -C link/..` reports "not a git repository", + because Win32 normalizes `..` textually. A shell resolver would send the + guard to a repository git never enters. + - **A `!` body starts at the outer repository's TOP LEVEL**, not where the + outer command ran. Invoked from `/sub` with `alias.a = !git -C child + a`, git reaches `/child`; carrying the subdirectory forward made the + guard probe `/sub/child` and miss a nested repository's `commit -m`. + + The lexical normalizer is deleted rather than patched. Composed `-C` paths are + now handed to git verbatim, and one primitive — + `git -C rev-parse --show-toplevel --show-prefix` — supplies both the `!` + body's launch directory and the canonical repository identity in the + shell-alias cycle key, so the guard tracks git's behavior on every platform by + construction. Where git chdirs the body (a nonempty prefix, or pure discovery) + identity canonicalizes for free: `-C .`, `link/..`, a subdirectory, and every + other spelling of one repository collapse to a single key, which is what stops + a self-rewriting `-C` chain. Resolution failure **falls back to the literal + composed directory** (best-available, not a gate), scoped to the alias walk + only, so an ordinary `git commit -F -` resolves nothing and forks nothing + (measured: 0 git subprocesses; the `-C .` chain stops after 4, not the 128 + traversal budget; a 20-hop inline chain still forks 0). + + - **A `!` shell-alias body under explicit locating globals launches where the + CALLER stands, not at the work-tree top level** (`block-noncanonical-commit`; + review finding on the fix above). git chdirs a `!` body to the top level only + when it can compute a prefix — when the caller's directory sits INSIDE the + effective work tree, which repository discovery always satisfies. An explicit + `--git-dir`/`--work-tree` whose work tree does not contain the caller skips + that chdir: verified on git 2.54.0.windows.1 (reported by review on 2.43.0), + from ``, `git --git-dir --work-tree -c alias.a='!git -C child p' + a` runs `/child`'s persisted `p`, while the same invocation from + `/sub` runs from ``. Collapsing to the top level UNCONDITIONALLY probed + the benign `/child` and allowed while real git ran `/child`'s + `commit -m` (verified fail-open) — and its mirror false-blocked a canonical + commit. The launch directory is now read from git's own answer: nonempty + `--show-prefix` (or a probe with no locating globals, i.e. pure discovery) + returns the top level, an empty prefix under explicit globals returns the + caller's composed directory. The inside-the-work-tree branch is unchanged, so + a subdirectory caller still resolves from the top level, as git does. + + - **The launch-directory probe is boundary- and newline-safe** (two review + findings on the fix above). Both were fail-open holes in the launch-directory + lookup itself: + - **The launch-directory CACHE key encoded each argv word `%q`, not `$*`.** + Joining the replayed locating globals with `$*` flattened argv boundaries, + so `--git-dir 'X --work-tree' --namespace Z` and `--git-dir X --work-tree + '--namespace Z'` — which git interprets as different repositories — produced + one key. In a payload with two git segments, the first poisoned the shared + cache for the second, handing it the first segment's directory while git + launched the second elsewhere and ran the caller's non-canonical alias. + Keying each word `%q`-encoded makes the key injective on the argv, so no two + distinct argvs collide. + - **The toplevel and prefix are read in SEPARATE `rev-parse` calls.** One + combined `--show-toplevel --show-prefix` call split on the first newline; a + repository path containing an INTERIOR newline truncated the toplevel and + misread the remainder as a prefix, switching the walk to the wrong directory. + Two calls put each field in its own capture, so an interior newline can no + longer be read as the boundary into the next field. The prefix call is + skipped when the toplevel is empty, so the common allow path pays no extra + fork. + - **Each field is captured byte-exact through a sentinel** (a third review + finding, on the two-call fix above). `$(…)` strips EVERY trailing newline, + but a top-level path may itself END in one (POSIX permits any byte but NUL + and `/`), so the strip returned a different sibling directory — the same + fail-open, now at the tail rather than the interior. A sentinel byte printed + after git's output absorbs the strip; git's terminator is then removed + explicitly. git ends these two `rev-parse` forms with a BARE LF, not a CRLF, + even on Windows (verified on git 2.54.0.windows.1 via `od -c`), so exactly + one trailing `\n` is peeled and nothing else — a `tr -d '\r'`/`%$'\r'` peel + would corrupt a path that legitimately ends in `\r`, the identical hole one + byte over. Interior and trailing newlines (and a trailing `\r`) now survive + in both fields. The framing is unit-verified against every newline position + (interior, single- and double-trailing, CRLF terminator); an end-to-end + fixture is impractical because reaching a newline top level requires either a + literal newline in the parsed command or a newline-ending payload `cwd`, and + the latter is stripped one layer earlier — a SEPARATE, pre-existing entry + point shared with `main`, tracked as + [#1536](https://github.com/melodic-software/claude-code-plugins/issues/1536) + rather than absorbed here. + + The invocation's LOCATING globals are replayed onto that probe, not just its + `-C`. `--git-dir` and `--work-tree` locate a repository as surely as `-C` does + (git's own usage lists both as globals before ``), and asking without + them answered "no work tree" for a perfectly locatable one — so + `git --git-dir=/.git --work-tree= -c alias.a='!git commit -F -' a` run + outside a tree had a **valid canonical commit refused**. The replay keeps the + ask-git property intact: `git --git-dir=X --work-tree=Y rev-parse + --show-toplevel` is still git's answer, not a reconstruction of one. Its `-m` + twin is pinned too, so the replay did not simply switch the fail-closed branch + off. + + `.` and `..` both need special handling only if the guard resolves paths + itself, and it no longer does. Every `.` spelling (`.`, `./././.`) resolves to + one identity, so a self-rewriting chain stops on the cycle key without a + `.`-cancelling pass. `..` is left to git as well rather than refused outright: + refusing every `..` path would be cheap and fork-free, but it false-blocks a + legitimate `git -C sub/.. commit -F -`, which is now a regression case + alongside its `commit -m` twin — asking git separates the two, blanket refusal + cannot. + + Words after the subcommand are no longer read as repository globals. They are + that subcommand's own arguments — or, for an alias, text git APPENDS to the + expansion — so a trailing `-C` is not a global: + `git -c alias.a='!git b #' a -C ` resolved to `` and + missed a `commit -m` reached in the CURRENT one, because git starts the body at + the current repository's top level and the `#` discards the appended words. + Directory resolution now sees only the invocation prefix, which also stops + `git commit -C HEAD` (`--reuse-message`) reading as a directory named `HEAD`. + + **Standing limitation, unchanged and still open:** the guard does not evaluate + shell relocation, so a `!` body that moves the process (`!cd child && git …`) + is analyzed against the invoking repository rather than the destination. Real + git resolves the destination's aliases, so an alias defined only there is not + seen. Modelling this means evaluating arbitrary shell word expansion, which + this guard deliberately does not do; asking git cannot help either, because + git is never told about the `cd`. Tracked in + [#1486](https://github.com/melodic-software/claude-code-plugins/issues/1486), + with a reverted working attempt and the four findings that landed against it as + a map of what a real fix must handle. + + Identity is a **best-available answer, not a gate**: when git cannot resolve a + work tree the walk continues with the literal composed directory, which is the + behavior this guard already had. An interim revision failed CLOSED there and was + dropped, because it never earned its place — its own justification was that a + commit could not have succeeded there anyway (so it protected against nothing), + while it produced three separate false positives, each refusing a VALID + canonical commit reached through a repository the OUTER probe could not see: + `--git-dir`/`--work-tree` on the invocation, then `-C` inside the body. The class + it was added for is open either way — the persisted-alias lookup still drops the + locating globals, here and on `main` alike + ([#1501](https://github.com/melodic-software/claude-code-plugins/issues/1501)). + Deferring resolution to the nested invocation is the real fix and is tracked as + its own design question + ([#1500](https://github.com/melodic-software/claude-code-plugins/issues/1500)) + rather than bolted on at this depth. + + Guard-local change only (no `hook-utils.sh` change, no cross-plugin sync). + Test matrices extended in both guards with two- and three-hop chains, the + `--config-env` and `.command` second-hop variants, a persisted-config alias + chain (fixture repo), the shell-alias outer-chain re-invocation (blocked) with + its canonical/undefined twins (allowed), a persisted chain crossing a `!` hop + (blocked / `-F -` allowed), 20-hop dual-spelling chains under a hard wall-clock + ceiling (safe terminal allowed, dangerous terminal still blocked — the collapse + costs no coverage), a 60-hop single-spelling chain (allowed: the budget bounds + branching, not depth), a divergent-spelling chain (blocked on the budget), a + three-level nested-repository fixture whose grandchild `commit -m` must block + (with its canonical twin allowed), a `-C .` self-reference that must collapse to + a cycle (with a twin proving a real `commit -m` behind a `-C .` hop still + blocks), a `!` body invoked from a SUBDIRECTORY that must resolve from the outer + repository's top level (canonical twin allowed), a `!` body under explicit + `--git-dir`/`--work-tree` whose caller sits OUTSIDE the work tree that must + launch in the caller's own child repository (bypass + false-block twins), with + the inside-work-tree pair proving the top-level branch is unchanged, a + symlinked-parent `-C link/..` + fixture gated on the platform actually resolving through the symlink (asserted on + POSIX, skipped loudly on Windows, where git is itself textual), `-C ./././.` + collapsing to a cycle without a `.`-cancelling pass, `-C sub/..` reaching a + `commit -m` (blocked) beside its canonical twin (allowed, which is why `..` is + not refused outright), and benign controls (safe multi-hop chain allowed; alias cycle, self- + and mutually referential persisted shell aliases terminate and allow without + hanging). Closes #964. + ## [0.17.0] ### Added diff --git a/plugins/guardrails/hooks/block-dangerous-git.sh b/plugins/guardrails/hooks/block-dangerous-git.sh index 59dbc4308..f488223f7 100755 --- a/plugins/guardrails/hooks/block-dangerous-git.sh +++ b/plugins/guardrails/hooks/block-dangerous-git.sh @@ -353,6 +353,66 @@ is_exclude_pathspec() { esac } +# Alias re-expansion is this guard's only recursive path, and it BRANCHES: every +# hop re-checks both alias spellings (`alias.` and `alias..command`) +# independently, so a chain where each hop defines both walks 2^depth analysis +# paths — a benign 10-hop, 402-character command measured 5.4s, and a guard that +# stalls stops guarding. Every recursion is admitted through this one gate, which +# applies two bounds: +# +# MEMO — a verdict is a pure function of (analysis state, argv); every other input +# is invocation-constant (the payload's command, the repository's config and object +# format). A block is a process-wide `exit 2`, so a state reached a SECOND time +# while this process still runs provably did not block the first time and cannot +# decide differently now. Skipping the repeat is exact rather than a coverage +# trade, and it is what collapses the common blowup — both spellings of a hop +# expanding to the same thing — to one path per hop. +# +# BUDGET — memoization alone cannot bound a chain whose two spellings DIFFER: the +# splice carries each path's own trailing text forward, so every argv is distinct +# and the 2^depth walk survives (10 hops of `-c alias.aN='a(N+1) --xN' +# -c alias.aN.command='a(N+1) --yN'` measured 5.7s). A total re-expansion budget +# for the invocation caps the work, and exhausting it fails CLOSED — the guard +# could not finish deciding, so it must not allow. +# +# The ceiling counts ANALYSES rather than seconds, because a wall clock is host- +# and command-length-dependent. It is calibrated against the linear walk this guard +# already accepts: a memoized traversal spends one analysis per hop, and +# MAX_COMMAND_LEN admits chains of roughly 430 hops (a dual-spelling hop costs ~38 +# characters), so this ceiling caps a branching walk at strictly less work than the +# longest non-branching chain the guard must already handle. It sits far above real +# usage — a chain deeper than a couple of hops is already exotic, and every +# legitimate command measured spends single digits. +HOOK_ALIAS_WORK_MAX=128 + +# Call as: alias_reexpand_admit ... — returns 1 when this exact +# state was already analyzed. The kind tag keeps a `!` reparse STRING from ever +# keying the same as a one-word argv, `%q` keeps a word containing a newline from +# merging into its neighbour, and the seen-set's length prefixes its own words so +# the set/argv boundary cannot shift. `printf -v` keeps the whole key build +# fork-free — a `$(printf …)` per word would cost more than the walk it bounds. +# shellcheck disable=SC2329 # reached via the hook::bash_parse_segments callback chain +alias_reexpand_admit() { + local kind="$1" key q w + shift + key="$kind"$'\n'"${#HOOK_ALIAS_SEEN[@]}"$'\n' + for w in ${HOOK_ALIAS_SEEN[@]+"${HOOK_ALIAS_SEEN[@]}"}; do + printf -v q '%q' "$w" + key+="$q"$'\n' + done + for w in "$@"; do + printf -v q '%q' "$w" + key+="$q"$'\n' + done + [[ -n "${HOOK_ALIAS_MEMO[$key]+x}" ]] && return 1 + HOOK_ALIAS_MEMO["$key"]=1 + ((++HOOK_ALIAS_WORK <= HOOK_ALIAS_WORK_MAX)) && return 0 + echo "BLOCKED: checking this command's git alias chain needs more than $HOOK_ALIAS_WORK_MAX re-expansions — failing closed rather than stalling the guard." >&2 + echo "Run the subcommand directly, shorten the alias chain, or set the guardrails block_dangerous_git_enabled option to false to bypass." >&2 + emit_tel "blocked" "alias-traversal-cap" + exit 2 +} + # Inspect one already-tokenized segment (its argv words passed as "$@"). Blocks # when the segment is a real git invocation carrying a default-blocked # irreversible form. Parsing spine lives in hook-utils.sh; only the form @@ -395,11 +455,21 @@ check_segment() { # The SHAPE refusal must fire at EVERY recursion depth: a wrapping inline alias # can expand to `--config-env=alias.=` defining the invoked sub # (`git -c alias.rh='--config-env=alias.foo=AV foo' rh`), which git runs. It is - # value-blind, cheap, and terminal, so it is NOT gated by HOOK_NO_ALIAS. Only the - # INLINE-alias re-expansion is one-level (HOOK_NO_ALIAS bounds the recursion — - # git does not re-expand the first word of an expansion as another alias). - local exp reparse a alias_rc - local -a expw=() + # value-blind, cheap, and terminal. + # + # git DOES chain aliases: when an expansion's first word is itself an alias git + # expands it again, until a non-alias subcommand is reached OR git detects a + # loop — a subcommand name it already expanded in this chain — and runs nothing. + # So the inline re-expansion recurses at EVERY hop, carrying the command-line + # -c/--config/--config-env globals into each hop (the splice starts at index 0 + # through sub_idx, not gi+1, so no global between git and the subcommand is + # dropped). Recursion TERMINATES on HOOK_ALIAS_SEEN, a save/restore seen-set of + # resolved subcommand names: a repeat is git's own alias-loop stop (allow-safe), + # and finite distinct alias keys guarantee termination. Terminating is not the + # same as tractable — the walk branches per hop, and alias_reexpand_admit is what + # keeps its cost proportional to the chain's length. + local exp reparse a alias_rc s seen_hit=0 + local -a expw=() saved_seen=() nextw=() hook::git_alias_expansion "$sub" alias_rc=$? if ((alias_rc == 2)); then @@ -414,32 +484,57 @@ check_segment() { emit_tel "blocked" "config-env-alias" exit 2 fi - if ((alias_rc == 0)) && ((${HOOK_NO_ALIAS:-0} == 0)); then + # git stops (runs nothing) if the resolved subcommand is one it already expanded + # in this chain, so skip the re-expansion on a repeat and let the plain scan + # decide. The set models git's IN-PROCESS alias-loop guard only: a `!` shell + # alias spawns a fresh git process whose loop guard starts empty, so its + # reparse below runs under an emptied set. + for s in ${HOOK_ALIAS_SEEN[@]+"${HOOK_ALIAS_SEEN[@]}"}; do + [[ "$s" == "$sub" ]] && { + seen_hit=1 + break + } + done + if ((alias_rc == 0)) && ((seen_hit == 0)); then # Inline alias (-c/--config): each spelling's expansion is literally present. Re-check # EVERY spelling (plain and `.command`) independently so a benign expansion in one - # never suppresses a dangerous sibling in the other. + # never suppresses a dangerous sibling in the other. Save/restore the seen-set around + # the recursion so sibling segments and unwound hops start clean. # shellcheck disable=SC2154 # HOOK_GIT_ALIAS_EXPS is set by hook::git_alias_expansion + saved_seen=(${HOOK_ALIAS_SEEN[@]+"${HOOK_ALIAS_SEEN[@]}"}) + HOOK_ALIAS_SEEN+=("$sub") for exp in ${HOOK_GIT_ALIAS_EXPS[@]+"${HOOK_GIT_ALIAS_EXPS[@]}"}; do [[ -n "$exp" ]] || continue if [[ "$exp" == '!'* ]]; then # Shell alias: git runs the expansion as a shell command with the # invocation's trailing args appended (positional), so append them # (shell-quoted) before re-parsing the whole string as a command. + # That command runs in a NEW git process whose alias-loop guard starts + # empty, so the reparse must not inherit this chain's seen-set: a body + # that re-invokes a name from the outer chain (`git -c + # alias.a='!git -c alias.a="reset --hard" a' a`) is re-expanded there, + # not stopped. Termination stays text-bounded — this guard resolves + # only inline aliases, and every definition reachable from the reparse + # is a strict substring of the parent segment's text. reparse="${exp#!}" for a in "${w[@]:sub_idx+1}"; do reparse+=" $(printf '%q' "$a")"; done - hook::bash_parse_segments "$reparse" check_segment + HOOK_ALIAS_SEEN=() + alias_reexpand_admit shell "$reparse" && + hook::bash_parse_segments "$reparse" check_segment + HOOK_ALIAS_SEEN=(${saved_seen[@]+"${saved_seen[@]}"} "$sub") else # Git alias: its expansion is dequoted with shell quoting rules # (so `push "--force"` yields --force, not "--force"). Splice the - # dequoted words in place of the alias name and keep the trailing - # invocation args, which git appends to the expanded argv. + # dequoted words in place of the alias name, keeping every command-line + # global (indices 0..sub_idx) and the trailing invocation args, which + # git appends to the expanded argv. hook::env_s_split "$exp" expw=(${HOOK_ENV_S_WORDS[@]+"${HOOK_ENV_S_WORDS[@]}"}) - HOOK_NO_ALIAS=1 - check_segment "${w[@]:0:gi+1}" ${expw[@]+"${expw[@]}"} "${w[@]:sub_idx+1}" - HOOK_NO_ALIAS=0 + nextw=("${w[@]:0:sub_idx}" ${expw[@]+"${expw[@]}"} "${w[@]:sub_idx+1}") + alias_reexpand_admit git "${nextw[@]}" && check_segment "${nextw[@]}" fi done + HOOK_ALIAS_SEEN=(${saved_seen[@]+"${saved_seen[@]}"}) fi case "$sub" in @@ -985,6 +1080,18 @@ case $? in *) COMMAND="$PS_SAFE_COMMAND" ;; esac +# Resolved-subcommand names already expanded in the CURRENT alias chain — git's +# own alias-loop guard. Initialized here (not in check_segment, which recurses +# and would reset it) and save/restored around each recursion; a multi-command +# line runs check_segment once per top-level segment, each starting from empty. +HOOK_ALIAS_SEEN=() + +# The alias-traversal bounds (alias_reexpand_admit). Both are invocation-wide and +# deliberately NOT save/restored: a state analyzed anywhere is analyzed, and the +# budget bounds the whole command's work rather than one path's. +declare -A HOOK_ALIAS_MEMO=() +HOOK_ALIAS_WORK=0 + hook::bash_parse_segments "$COMMAND" check_segment emit_tel "ok" "" diff --git a/plugins/guardrails/hooks/block-dangerous-git.test.sh b/plugins/guardrails/hooks/block-dangerous-git.test.sh index 04e7c2efb..2c241e363 100755 --- a/plugins/guardrails/hooks/block-dangerous-git.test.sh +++ b/plugins/guardrails/hooks/block-dangerous-git.test.sh @@ -392,6 +392,105 @@ run "git restore --staged --no-w . (abbrev no-worktree, allowed)" "git restore - run "git restore --no-worktree --worktree . (worktree re-armed, blocked)" "git restore --no-worktree --worktree ." 2 run "git restore --staged --no-staged . (staged cleared, worktree discard, blocked)" "git restore --staged --no-staged ." 2 +# --- #964: git chains aliases — re-expansion recurses to the invoked op -------- +# git expands an alias whose first word is itself an alias, so a dangerous op +# reached through a SECOND (or later) hop must still block. Every command-line +# -c/--config-env global rides into each hop (so a second-hop --config-env alias +# is refused by shape), and the recursion stops on git's own alias-loop. +# Case C — plain two-hop inline chain (rh -> foo -> reset --hard). +run "#964 case C: two-hop inline alias chain to reset --hard (blocked)" \ + "git -c alias.rh=foo -c alias.foo='reset --hard' rh" 2 +# H1 — second hop defined via --config-env (env-shaped, refused by shape); the +# global is carried into the nested hop by the widened splice. +run "#964 H1: inline first hop, --config-env second hop (blocked by shape)" \ + "git -c alias.rh=foo --config-env=alias.foo=AV rh" 2 "AV=reset --hard" +# H2 — same defect with the --config-env global placed BEFORE the -c global. +run "#964 H2: --config-env before -c, chained to the invoked sub (blocked)" \ + "git --config-env=alias.foo=AV -c alias.rh=foo rh" 2 "AV=reset --hard" +# Three inline hops. +run "#964 three-hop inline chain to reset --hard (blocked)" \ + "git -c alias.a=b -c alias.b=c -c alias.c='reset --hard' a" 2 +# Second hop spelled via the alias..command subkey. +run "#964 .command-spelled second hop to reset --hard (blocked)" \ + "git -c alias.rh=foo -c alias.foo.command='reset --hard' rh" 2 +# Benign controls — a chain to a safe terminal op still ALLOWS, and an alias +# cycle terminates (git's alias-loop stop) and allows without hanging. +run "#964 benign two-hop chain to a safe subcommand (allowed)" \ + "git -c alias.a=b -c alias.b=status a" 0 +run "#964 alias cycle terminates and allows (no hang)" \ + "git -c alias.a=b -c alias.b=a a" 0 +# A `!` shell alias runs in a NEW git process whose alias-loop guard starts +# empty, so a body that re-invokes a name from the outer chain is re-expanded +# there — the reparse must not inherit the outer chain's seen-set. +run "#964 shell-alias body re-invoking the outer chain name (blocked)" \ + "git -c alias.a='!git -c alias.a=\"reset --hard\" a' a" 2 +run "#964 shell-alias body re-invoking an undefined inner name (allowed, no hang)" \ + "git -c alias.a='!git a' a" 0 + +# --- alias-chain traversal stays proportional to the chain's LENGTH ------------ +# Each hop re-checks BOTH alias spellings, so re-expansion branches 2x per hop +# unless equivalent states collapse: before the traversal bounds a 10-hop chain +# defining both spellings cost 5.4s, and each further hop doubled it. These cases +# therefore assert a hard wall-clock CEILING as well as the exit code — an +# exit-code-only assertion passes at any runtime and would not see the regression. +# +# run_bounded