Skip to content

fix(guardrails): block-dangerous-git still clears a lease via an inherited --git-dir, plus two documented scope boundaries #2151

Description

@kyle-sexton

Filed by AI. These are the three residuals #2147 documented in code comments but did not close. Merging that PR removed the last natural reminder they exist — a comment inside a function is only found by someone already reading the thing the comment would warn them about. All evidence below is against shipped origin/main (guardrails 0.24.0, post-#2147), not a branch.

Read this first: one of these is a live bypass, two are scope boundaries

A reader who cannot tell them apart will either panic or dismiss all three.

  • (A) is a LIVE BYPASS. Narrow, pre-existing, unfixed by fix(guardrails): read the payload cwd, and stop env -S hiding commands from every git guard #2147. block-dangerous-git clears an unsafe --force-with-lease today.
  • (B) and (C) are SCOPE BOUNDARIES — limits of what a static PreToolUse hook can see without evaluating arbitrary shell word expansion, which this guard deliberately does not do. They are recorded so the boundary is written down where someone can find it, not because they are actionable defects.

Evidence, all against shipped main

Exit 2 = BLOCKED, 0 = ALLOWED. widths is the hash width the guard's own probe resolved, read from bash -x (_repo_oid_width); 0 means it could not resolve the repository and fell closed. "git really" substitutes rev-parse --show-object-format for the push and runs the identical form for real — a form that never reaches git is not a bypass.

# case hook widths git really runs in verdict
A git --git-dir=<sha256>/.git --work-tree=<sha256> -c alias.y='!git push --force-with-lease=main:<40-hex> …' y, payload cwd = SHA-1 repo 0 ALLOWED 40 sha256 live bypass
A-control same with -C <sha256> instead of --git-dir 2 BLOCKED 64 sha256 closed by #2147
B cd <sha256> && git push --force-with-lease=main:<40-hex> …, payload cwd = SHA-1 repo 0 ALLOWED 40 sha256 scope boundary
C cd <sha1> && git push --force-with-lease=main:<40-hex> …, payload cwd = a non-repository 2 BLOCKED 0 sha1 scope boundary, false block

(A) An explicit --git-dir / --work-tree is inherited by a ! shell-alias body

git exports an explicit --git-dir/--work-tree into the environment of a ! alias body (verified on git 2.54.0 — the body prints sha256 from a SHA-1 directory and sees GIT_DIR set). So the body operates on a repository that the directory the guard computed does not name, and a 40-hex lease is judged against the wrong repository. Where the push lands, that word is a movable ref name — precisely what --force-with-lease exists to prevent.

#2147 made the guard compose the effective base across -C, which is correct and is why the A-control blocks. It composes only -C, deliberately: only -C relocates a ! body's directory (git -C <other> -c alias.wd='!pwd' wd moves; git --git-dir=<other> -c alias.wd='!pwd' wd does not). The --git-dir case is not a directory relocation at all — it is an inherited environment variable — which is why composing directories cannot reach it.

What closing it actually costs. Not a small extension of #2147. That PR's whole mechanism is replaying a directory as a leading -C. Fixing this one means replaying the inherited globals themselves into the reparse — a different mechanism, with its own ordering rules against the body's own globals (git's --git-dir is last-wins, and an inherited GIT_DIR is overridden by an explicit one on the body's command line). Anyone scoping this should budget for a new mechanism, not a parameter change.

(B) A shell cd relocation

cd X && git push …, (cd X && …), sh -c 'cd X && …', pushd. The push runs from a directory no option and no payload field names. Resolving it means evaluating arbitrary shell word expansion; the guard is static matching over the literal command string only.

(C) The same boundary's commoner symptom is a FALSE BLOCK

This is the one most likely to be met in practice, and it is not a bypass. With a shell cd, the base the guard measures is frequently not a repository at all, the probe answers 0, and the guard fails closed. Row C is cd <repo> && git push --force-with-lease=main:<literal full-width sha> origin main — the exact form the guard's own block message prescribes — denied from a session root that is not itself a repository.

Fail-closed is the right default for an unresolvable base. But a guard that refuses correct usage it just recommended teaches people to route around it, so this should be measured as the primary symptom of (B), not as an afterthought to the bypass.

Observed incidentally while producing this evidence: the guard also blocked the verification harness itself — a command that performs no push at all, whose --force-with-lease text existed only inside a JSON payload string being fed to the hook under test. Static matching over the literal command string has no way to tell a payload from an invocation. Noted as a real instance, not proposed as a fix.

(D) The PowerShell arm of the env -S surface is UNVERIFIED, not known-broken

Stated precisely because the distinction matters: nobody has tested it, and no defect is claimed. The guard's matcher is Bash|PowerShell, and #2147's adversarial pass used no PowerShell payloads at all, so the env -S and lease-width surfaces are simply unexercised on that arm. #2145 already covers the PowerShell/Bash contract mismatch for this guard; this is the testing-coverage half of the same area and should be folded in there rather than restated here.

Acceptance

There is no separate baseline to diff against. Post-merge, main is the pre-fix tree for (A) — so "reproduce against origin/main first" is the wrong instruction here and will waste the next person's time. State the discriminating fixture instead:

  • The control that fails against current main is row A: it must go ALLOWED → BLOCKED, with the probe resolving 64 rather than 40. A fixture that blocks with widths=[0] proves nothing — that is the fail-closed path, not the fix.
  • Row A-control must stay BLOCKED (-C instead of --git-dir), or the change has broken what fix(guardrails): read the payload cwd, and stop env -S hiding commands from every git guard #2147 fixed.
  • An opposite-direction control is required, so the fix is not merely "block more": --git-dir pointing at a SHA-1 repository with the payload cwd in a SHA-256 one must be ALLOWED, because a 40-hex word is a genuine object id where that command runs.
  • Every ALLOWED assertion must additionally prove the command executes — substitute rev-parse --show-object-format and run the identical form. env FOO=1 -C <dir> git … is the worked counter-example: GNU coreutils stops option parsing at the first NAME=VALUE, so -C becomes the command name, rc=127, and git never runs.

(B) and (C) need no acceptance criteria unless someone decides to narrow the boundary; if they do, (C) is the symptom to measure.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageNot yet classified. Floor until a type and one priority tier are set.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions