Skip to content

fix: make the commit hooks fail closed and gate every push on a preflight receipt - #700

Merged
REPPL merged 22 commits into
mainfrom
fix/githooks-gates
Sep 25, 2026
Merged

REPPL merged 22 commits into
mainfrom
fix/githooks-gates

Conversation

@REPPL

@REPPL REPPL commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator

The git hooks gate earlier and on firmer ground. A commit message carrying a live agent-session URL or a tool attribution footer is refused before the commit exists. A push is gated before it opens its connection, on the committed tree. This repository's own name guard carries the hardening its scaffolded template already had.

  • .githooks/commit-msg (new) judges the message with abcd lint outbound, built from the checkout. It pins its environment first, so an inherited shell function or a tool shimmed on PATH cannot turn it into a pass. It cuts the message only where git itself cuts it. It fails closed when there is no Go or no source, and it says so when the tree does not build rather than blaming the message.
  • Push gate: make preflight mints a receipt for HEAD, but only when the tree is clean at the start and the end, HEAD is unchanged, and no index flag hides an edit. pre-push refuses a new commit without a receipt and never runs the preflight itself. The sequence is commit, then make preflight, then git push. CI still runs every gate; the receipt is a local convenience. A DECISIONS entry records the design, the options not taken, and what the receipt can and cannot reach.
  • .githooks/pre-commit and pre-merge-commit gain the template's hardening: the function pin and sweep, gitlink paths, and control bytes in printed paths. The name guard's notice prints once per commit.
  • AGENTS.md and CONTRIBUTING.md describe the new sequence.
  • Records resolved:
    • the critical iss-2609061438431625 (this repository's half);
    • iss-2608290810036869 (a push is gated before it connects);
    • iss-2608210738378295 (the push-time gate reads the committed tree);
    • iss-2609181122202952 (one notice per commit);
    • iss-2609250850380420 (a major found during the lane: the repository's own name guard lacked the hardening).
  • Deferred out loud: the managed-repository half (iss-2609250834251447, major) needs three product decisions: how a hook with no plugin root finds a binary, whether it fails open or closed, and whether it installs by default.

Reviews (Fable 5.1, ruthless and security): FIX FIRST on the commit-msg hook failing open under inherited shell state, plus a scissors mismatch and two notes. All are fixed with tests watched failing first, and the fix round was verified.

Review: three Fable 5.1 rounds. Round 1 found four findings, which were fixed. Round 2 found that the scissors cut steered on unpinned builtins, and that GOFLAGS -overlay went unchecked; both were fixed and verified. Round 3 gave SHIP, and its one minor (a quoted or carriage-return-separated GOFLAGS -overlay) was fixed and watched red, then green.

Resolves: iss-2609061438431625
Resolves: iss-2608290810036869
Resolves: iss-2608210738378295
Resolves: iss-2609181122202952
Resolves: iss-2609250850380420
Refs: iss-2609250834251447
Assisted-by: Claude:claude-opus-5-5

The pre-commit name guard printed a notice per store per phase: the
format before the parse, the count after it, and an inheritance line
before an inherited store. A clean commit from a linked worktree
therefore printed three notice lines, and five once the worktree
carried a store of its own (a sync-banlist refresh creates one), which
reads as the hook running several times. The dispatch was not the
cause: the global dispatcher and git run the hook once per commit.

Each store now records its format and count, and one line names every
store read, with an inherited store's location in its tag. Refusals
raised while parsing already name the reading they apply, so the
pre-parse format line carried nothing a refusal does not. Applied to
this repository's hook and to the scaffolded template, which is the
copy the report came from.

Refs: iss-2609181122202952

Assisted-by: Claude:claude-opus-5-5
…er commit

Resolves: iss-2609181122202952

Assisted-by: Claude:claude-opus-5-5
…ists

A live agent-session URL reached three commit messages and two
pull-request bodies of a managed public repository. CI now judges every
commit message of a pull request and its body through `abcd lint
outbound`, but by then the message is in the author's history and on the
forge. The committed commit-msg hook runs the same verb on the message
before the commit is made, and git runs it for a merge that creates a
commit, which pre-commit never sees.

Decisions the record left open, taken for this repository:
- Binary resolution: `go run ./cmd/abcd` from the checkout that holds
  the hook (or the working tree, for a copy under .git/hooks), the rule
  for every abcd invocation in a source checkout. No PATH rung: an
  installed abcd is stale by construction here.
- Fail closed: no abcd source or no Go toolchain refuses the commit and
  names the missing piece. A pass that means "skipped" reads the same as
  a pass that means "clean".
- Arming: committed in .githooks beside the name guard, so it runs
  wherever the clone's hooks path points there, as the name guard does.
Scaffolding the hook into managed repositories is not part of this
change: there a hook has no source checkout to run, and the
missing-binary and default-versus-opt-in choices have the blast radius
of every managed repository.

The text below a `git commit -v` scissors line is discarded by git and
is not judged; comment lines are, since `-m` keeps them.

Refs: iss-2609061438431625

Assisted-by: Claude:claude-opus-5-5
…re the commit exists

The managed-repository form of the hook and the forge-CLI text guard
are split out as iss-2609250834251447, which records the three product
decisions they need.

Resolves: iss-2609061438431625
Refs: iss-2609250834251447

Assisted-by: Claude:claude-opus-5-5
The pre-push hook ran `make preflight`, and git opens a push's
connection before it runs the hook: a ten-minute preflight outlasted the
transport's idle timeout, the server closed the connection, and the push
reported success while moving nothing. Ruling M16 (2026-09-23) is check
before connect.

Design: the preflight runs first, as its own command, and its last step
mints a receipt for HEAD under the local tier
(scripts/preflight-receipt.sh). The pre-push hook keeps its branch
protection and otherwise only checks receipts: a push whose commit is
new to the remote and carries no receipt from any worktree of this
repository is refused in milliseconds, with the remedy named. A commit
the remote already holds, such as a tag on a merged commit, needs none.
A plain `git push` after a clean `make preflight` passes, so the
orchestrator's helper keeps working, and a plain `git push` without one
is refused, so nothing relies on `--no-verify` going unused. Rejected: a
wrapper that runs the preflight and then pushes with `--no-verify` (it
normalises the flag and leaves a plain push ungated), transport
keepalives (the hook would still hold the connection), and a preflight
over a clean export of HEAD (a second tree and build per push).

The receipt also closes the working-tree gap: the gates read the working
tree and CI reads the commit, so a staged rename whose follow-up edit
stayed unstaged passed locally and failed CI. A receipt is minted only
when the tree matched HEAD (nothing staged, unstaged or untracked) both
when the run began, read while the Makefile is parsed and so before any
gate, and when it ended, with HEAD unmoved.

Recorded in .abcd/work/DECISIONS.md, since it changes how every push
from this repository works.

Refs: iss-2608290810036869, iss-2608210738378295

Assisted-by: Claude:claude-opus-5-5
Resolves: iss-2608290810036869

Assisted-by: Claude:claude-opus-5-5
…mmitted tree

Resolves: iss-2608210738378295

Assisted-by: Claude:claude-opus-5-5
The scaffolded name-guard template was hardened on 2026-08-26 against
inherited shell state and attacker-authored paths, and this repository's
own copy never received the change; the test drove the template alone.
Four holes stood in the dogfood copy:

- read and echo were not pinned against inherited functions, so a
  BASH_ENV defining them made the guard read zero entries and commit a
  banned name;
- a shadowed declare plus an exit function turned a printed refusal
  into a commit, because exit was not on the expansion-free unset list
  and no sweep followed it;
- a staged path was appended to the scan after the gitlink skip, so a
  submodule path carrying a banned name was never scanned;
- refused paths were echoed raw, so control bytes could forge the
  refusal text.

The template's hunks are ported verbatim into .githooks/pre-commit and
the pin into .githooks/pre-merge-commit, and a test drives all four
through BASH_ENV against this repository's hook. The remaining
differences from the template are deliberate: the identity gate, the
sources-corpus refresh, and the comments.

Refs: iss-2609250850380420

Assisted-by: Claude:claude-opus-5-5
…rries the template's hardening

Resolves: iss-2609250850380420

Assisted-by: Claude:claude-opus-5-5
The managed half of the session-URL gate needs three product decisions
nobody in the run can take while the product thinker is away: how a
hook with no plugin root finds a binary, whether it fails open or
closed, and whether it installs by default. The record carries the
question until then.

Refs: iss-2609250834251447
Assisted-by: Claude:claude-opus-5-5
The commit-msg hook is a fresh bash that inherits the committer's
environment, and it pinned nothing. An exported grep function, a BASH_ENV
file shadowing grep and exit, or a PATH-prepended awk that prints nothing
made its "nothing to judge" test answer yes, and a message carrying a live
session URL was committed without being judged.

The hook now opens with the pre-commit guard's prologue: xtrace off, a
fixed expansion-free unset -f list (awk, grep, sed, mktemp, cat, rm,
printf, command, read, echo, exit, test, [ and go among them), the
declare -F sweep, a pinned IFS and LC_ALL, and PATH pinned to the system
directories with the abcd.guardPath extension. go is the one tool resolved
on the inherited PATH, to an absolute path, because the toolchain lives
where its installer put it; any real go builds this checkout's source and
judges the same way. The scissors cut and the empty test are bash builtins,
so no external tool can decide what is judged, and output goes through a
builtin loop instead of sed.

Refs: iss-2609061438431625
Assisted-by: Claude:claude-opus-5-5
The commit-msg hook stopped reading at any punctuation-prefixed
scissors-shaped line, in every cleanup mode. A message given with -F keeps
everything under the default cleanup, so `# ---- >8 ----` or a `;` look-alike
followed by a session URL passed the hook and the URL was recorded.

The hook now cuts only at git's exact scissors line, prefixed with the
clone's comment character (core.commentString or core.commentChar, `#`
by default, any of git's candidates under `auto`), and only when a
`diff --git` line follows it: the shape of a verbose commit, which is
when git truncates. Everywhere else it judges the whole file. A -F message
that forges both the scissors and a diff header is the residual shape; the
header says so and names CI's check over the recorded message as the
backstop.

The verbose-diff test now drives a real `git commit -e -v` whose staged
fixture carries the URL, under the default and a configured comment
character.

Refs: iss-2609061438431625
Assisted-by: Claude:claude-opus-5-5
The commit-msg hook ran `go run ./cmd/abcd lint outbound`, and go run
exits 1 on a compile error, the same code the check gives a finding. A
checkout whose cmd/abcd did not compile was refused with "the commit
message breaks the outbound policy" over compiler output, blaming a message
nothing had judged.

The hook now builds ./cmd/abcd into its temporary directory first. A build
that fails is refused as "could not judge the message (the tree does not
build)", with the compiler output and a line saying nothing in the message
was found wrong; the refusal still fails closed. Only a build that succeeds
is run to judge the message. A warm build costs about 0.7s per commit.

Refs: iss-2609061438431625
Assisted-by: Claude:claude-opus-5-5
git status does not report an edit to a tracked file flagged
skip-worktree or assume-unchanged, so a preflight over such an edit read as
a clean tree and minted a receipt for a tree the gates did not read.

preflight-receipt.sh state now reports "hidden" when `git ls-files -v`
tags any entry S (skip-worktree) or in lower case (assume-unchanged; the
lower-case s covers both flags at once), read through a here-string so
pipefail cannot turn a match into a miss, and mint refuses on it with the
command that clears the flag. The script header and a new DECISIONS entry
state the limits that remain: ignored files a gate reads (go.work), HEAD
or the tree moved and restored between the two reads, and anything read
from outside the checkout.

Refs: iss-2608210738378295
Assisted-by: Claude:claude-opus-5-5
Review of the hooks lane met the falsifier each record's grounds names
before the resolutions merged. The records stay resolved and nothing in
them is amended; a pursued line is appended to each Grounds saying what
review showed, what the fix commits changed, and what would still show the
record wrong.

Refs: iss-2609061438431625
Refs: iss-2608210738378295
Assisted-by: Claude:claude-opus-5-5
The scissors cut decided what was judged through `continue` and `break`,
which the fixed `unset -f` list did not name. With `declare` shadowed the
sweep that would have caught them was neutered, so an inherited no-op
`continue` (BASH_ENV or a BASH_FUNC_ export) sent every line into the
search for a diff, a `diff --git ` line anywhere cut the message to
nothing, and a -F message carrying a session URL was committed unjudged.

The fixed list now names `declare continue break return local export set
true` beside the tools, `set` is dropped before `set -euo pipefail` runs,
and the cut steers on arithmetic and `if` alone, with no `continue` or
`break` in it. The comment on the list says what it covers and is true.
The same builtins join the fixed lists of .githooks/pre-commit,
.githooks/pre-merge-commit and the scaffolded pre-commit template for
parity: there a shadowed `break` made a read loop spin for ever.

TestCommitMsgHookResistsInheritedShellState gains two cases, BASH_ENV and
exported functions shadowing declare/continue/break with a URL and a
`diff --git ` line in the message: red on bee9599 (the commit was made;
with `break` also shadowed the pre-commit hook hung), green after.

Refs: iss-2609061438431625

Assisted-by: Claude:claude-opus-5-5
The hook's header said any real `go` builds this checkout's own source, and
GOFLAGS falsified it: an -overlay replacing cmd/abcd/main.go by a no-op
built cleanly, judged nothing and passed a session URL. -toolexec, which
runs every compile step through a program of the caller's choosing, is the
same class.

The hook now reads GOFLAGS through `go env` (so the go env file counts as
well as the environment) and refuses, before it builds, when it carries
-overlay or -toolexec in any spelling, saying the message was not judged.
A `go env` that fails refuses too. The header states the trust boundary as
the toolchain and the GOFLAGS, GOROOT, GOTOOLCHAIN and GOCACHE it runs
under, with a doctored GOROOT or poisoned cache in the deliberate class
CI's check is the backstop for.

TestCommitMsgHookRefusesAGOFLAGSThatSwapsTheSource: red on f78715a (the
overlay commit carried the URL; the toolexec commit was made), green after.

Refs: iss-2609061438431625

Assisted-by: Claude:claude-opus-5-5
Review 3 of the hooks lane found the GOFLAGS refusal bypassed by go's own
tokeniser: go splits GOFLAGS on space, tab, newline and carriage return and
lets a field be wrapped in quotes, while the hook split on the inherited IFS
and matched the bare word. A quoted -overlay, or one after a carriage
return, reached the build and the URL was committed. The hook now turns
quotes and carriage returns into separators and splits on the four blanks,
and the header names -modfile among the deliberate settings it trusts.

Refs: iss-2609061438431625
Assisted-by: Claude:claude-opus-5-5
Assisted-by: Claude:claude-opus-5-5
@REPPL
REPPL enabled auto-merge September 25, 2026 11:09
@REPPL
REPPL added this pull request to the merge queue Sep 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 25, 2026
Assisted-by: Claude:claude-opus-5-5
@REPPL
REPPL enabled auto-merge September 25, 2026 12:39
@REPPL
REPPL added this pull request to the merge queue Sep 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 25, 2026
The merge queue ejected the hooks change twice. On each platform one commit-msg
hook subtest failed its TempDir cleanup with "directory not empty": the hook
runs the go command under the test's temp HOME, and with the default telemetry
mode go spawns a detached child that keeps writing counters under that HOME
after the command exits. Env now writes the mode file `go telemetry off` writes,
under the test HOME's user config directory, so no child starts.

Refs: iss-2609061438431625
Assisted-by: Claude:claude-opus-5-5
Assisted-by: Claude:claude-opus-5-5
@REPPL
REPPL enabled auto-merge September 25, 2026 14:43
@REPPL
REPPL added this pull request to the merge queue Sep 25, 2026
Merged via the queue into main with commit 0b9fa8f Sep 25, 2026
12 checks passed
@REPPL
REPPL deleted the fix/githooks-gates branch September 25, 2026 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant