Skip to content

guardrails: repo_oid_width's width-0 fallback is silent, cached, and blamed on the operator's SHA #2227

Description

@kyle-sexton

Summary

repo_oid_width() in plugins/guardrails/hooks/block-dangerous-git.sh falls back to a width of 0
whenever it cannot read the repository's object format, with the underlying git error discarded. Width
0 makes the length test at :323 reject every hex string of every length, and the operator is then
shown a message at :847 asserting a specific, wrong cause — that their <expect> was an abbreviation
or "hex of the wrong width for this repository's hash format".

The guard is right to fail closed. The problem is that it fails undiagnosably, and actively
misdirects: the one thing the message tells you to check is the one thing that is not wrong.

The code

plugins/guardrails/hooks/block-dangerous-git.sh:305-313

repo_oid_width() {
  local key="$*"
  [[ -n "$_repo_oid_width" && "$key" == "$_repo_oid_width_key" ]] && return 0
  _repo_oid_width_key="$key"
  case "$(git "$@" rev-parse --show-object-format 2>/dev/null)" in
  sha1) _repo_oid_width=40 ;;
  sha256) _repo_oid_width=64 ;;
  *) _repo_oid_width=0 ;;
  esac
}

Consumed at :323:

((_repo_oid_width)) && ((${#expect} == _repo_oid_width))

With _repo_oid_width=0, ((0)) is false, so the conjunction is false for any $expect. A correct
40-hex SHA-1 fails exactly as an abbreviation would.

Emitted at :847:

BLOCKED: git push --force-with-lease=<refname>:<expect> whose <expect> is a name git resolves at push time (origin/main, HEAD, a tag, an abbreviated object id, or hex of the wrong width for this repository's hash format) …

Three distinct problems

  1. 2>/dev/null discards the only evidence of why the read failed. Whatever git said —
    permissions, a corrupt or unreadable config, a safe.directory refusal, not-a-repository — is
    gone.
  2. The message asserts a cause that is not the cause. An operator holding a correct 40-hex SHA is
    told to look for an abbreviation. That is worse than a generic refusal: it sends them hunting a
    defect that does not exist.
  3. The poisoned value is cached. [[ -n "$_repo_oid_width" ]] is TRUE for the string 0, so once
    the fallback fires, every later call with the same key short-circuits on the cached 0 without
    retrying. One transient git failure disables literal-SHA leases for the rest of the invocation.

Evidence

  • The fallback path is reachable and silent: in a non-repository directory,
    git rev-parse --show-object-format exits 128 and prints nothing to stdout, so the *) arm is
    taken with no signal.
  • The guard is not broken for well-formed SHAs under normal conditions. Two independent
    non-reproductions, in this repository, where --show-object-format correctly reports sha1:
    git push --dry-run --force-with-lease=<ref>:<literal-40-hex> origin HEAD:<ref> → exit 0, and the
    same without --dry-run/refspec → accepted. So this is specifically about the failure path.

Where it was actually observed

An agent was refused three times with that message while passing
--force-with-lease=fix/babysit-merge-ruleset-context-union:f211c6742de08d7f9a0ae5c6b14772de85cab021
— a literal 40-character hex string, typed into the command, not abbreviated and not a shell
substitution, and the branch's then-current remote head. The same command now passes unchanged.

Hypothesis for the transience, labelled as such and NOT established: at that time this clone's
local config was polluted by the git_init() harness described in #2162 (user.email=t@t.test,
commit.gpgsign=false written into the real repository). If that state made a git read in this path
fail, --show-object-format returned empty, the width went to 0, and every literal SHA was refused.
The timing and the symptom fit. The polluted state has since been removed, so the causal link cannot
be reproduced retroactively — treat it as a plausible mechanism, not a finding.

The defect stands on its own regardless of what triggered it that day: the fallback is silent, the
message misdiagnoses, and the bad value is cached.

Suggested fix

  • Do not discard the error. Capture stderr and surface it, or at minimum say that the object format
    could not be read.
  • Distinguish the two refusal reasons in the message. "This <expect> is not a full object id for
    this repository" and "the repository's hash format could not be determined, so no <expect> can be
    validated" are different failures and should read differently.
  • Do not cache the failure sentinel, or cache it distinguishably from a successfully read width, so a
    transient failure does not persist for the rest of the invocation.

Fail-closed is correct and should stay. What needs fixing is that the operator cannot tell why.

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions