Skip to content

guardrails: block-dangerous-git resolves lease hash-width for the wrong repository across a ! alias hop #1469

Description

@kyle-sexton

Problem

repo_oid_width() in plugins/guardrails/hooks/block-dangerous-git.sh determines the repository's object-id width — which decides whether a --force-with-lease=<ref>:<expect> expectation is an immutable full-width object id or a movable ref name — by probing with the hook process's own cwd plus the segment's replayed locating options:

case "$(git "$@" rev-parse --show-object-format 2>/dev/null) in

That never composes a directory across a ! shell-alias reparse boundary. When an alias body re-invokes git against a different repository, the width is still resolved for the original one. If the two repositories use different hash formats, a lease expectation is judged against the wrong width.

This is the same structural miss that #1085 fixed in block-noncanonical-commit: the guard resolved a directory per segment and never propagated where a ! body actually runs. That guard now asks git (git -C <dir> rev-parse --show-toplevel) and composes the answer across hops; block-dangerous-git still does not.

Exploit shape

Narrow — it needs all of:

  1. A ! shell alias (inline -c alias.x='!git …' or persisted) whose body invokes git against a different repository than the outer command.
  2. That target repository using the other hash format (SHA-256 when the outer is SHA-1, or vice versa).
  3. A git push --force-with-lease=<refname>:<expect> in the body where <expect> is a full-width hex string for the outer repo's format — so it reads as an immutable object id there — that is also a valid ref name in the target repository.

Condition 3 is the same residual the file already documents for the --force-with-lease width check: a hex-named ref of the other width is an ordinary movable ref name, so the lease passes while clobbering work the pusher never saw.

Why it was deferred out of #1085

Recorded per the maintainer's call on that PR. Closing it means replicating the effective-base machinery (HOOK_EFFECTIVE_BASE propagation plus a git-resolved identity primitive) into a guard that otherwise resolves no paths at all — block-dangerous-git has no persisted-config lookup, no HOOK_CWD read, and no shell-alias seen-set. That is real scope growth, and #1085 was four review rounds deep on a merge-blocking branch.

Sibling residual already documented

block-dangerous-git.sh documents a related gap in the same family at the lease_expect_is_immutable / repo_oid_width comments:

Known gap: a compound cd <elsewhere> && git push … pushes from a directory no option names, so the probe cannot see it. Resolving the cd target would mean evaluating arbitrary shell word expansion, which this guard deliberately does not do.

Both are "the guard cannot see which repository the push will run in". A fix should consider them together rather than patching the ! case alone — otherwise this is a third round of the same class in a different guard.

Fix direction

Adopt the mechanism block-noncanonical-commit now uses, rather than inventing a second one:

  • Ask git for the directory instead of modelling it. Do not normalize paths lexically, and do not resolve them with a shell primitive — verified on git 2.54.0.windows.1 that cd -P link/.. and git -C link/.. disagree, because Win32 normalizes .. textually while POSIX resolves it through the kernel.
  • Propagate the resolved repository across each ! reparse (git runs a shell body from the repository top level), and key the width cache on that resolved identity rather than on the replayed option list alone.
  • Fail closed when git cannot resolve the target — consistent with repo_oid_width's existing 0 means undeterminable → fails closed posture.

Verification shape

A fixture pairing a SHA-1 repository with a SHA-256 one, a ! alias crossing between them, and a lease pinned to a 40-hex string that is also a tag name in the SHA-256 repo. block-dangerous-git.test.sh already builds both REPO_SHA1 and REPO_SHA256 fixtures, so the scaffolding exists.

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

    area: securitySecurity-relevant: vulnerability, hardening, or disclosure follow-up.needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.priority: highSignificant impact, or blocks an imminent release; staff this cycle.

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions