From 5a6626b5882e61daada3d38eaabcc38eeac695ba Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:27:51 +0100 Subject: [PATCH 01/19] fix(hooks): the name guard announces once per commit 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 --- .githooks/pre-commit | 47 ++++++------ internal/core/ahoy/banlist_scaffold_test.go | 82 +++++++++++++++++++++ internal/core/ahoy/defaults/pre-commit | 47 ++++++------ internal/core/banlist/hook_test.go | 63 ++++++++++++++++ 4 files changed, 193 insertions(+), 46 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 35b031756..3c84543e8 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -443,6 +443,7 @@ fi rc=0 entries=0 +notice="" keys=() pats=() lines=() @@ -467,7 +468,7 @@ load_store() { _inherited="$3" # 1 when this is the primary checkout's store, read as a fallback _tag="" if [ "$_inherited" -eq 1 ]; then - _tag=" (inherited from the primary checkout)" + _tag=" (inherited from $_label)" fi # Refusals raised while parsing THIS store belong to it. Compared against the # entering value, so the zero-entry warning below is not suppressed by a refusal @@ -517,14 +518,12 @@ load_store() { fi fi - # Announce the FORMAT before any line is read, so a refusal below carries the one - # piece of context that decides what every other line means. The count follows the - # loop, because it is not known until then. - if [ "$_keyed" -eq 1 ]; then - echo "pre-commit: abcd name-guard: reading a keyed store$_tag" >&2 - else - echo "pre-commit: abcd name-guard: reading a legacy store$_tag" >&2 - fi + # The format is NOT announced here, before the loop. Every refusal the loop raises + # already names the reading it applies (a keyed line that does not parse says it is + # not a KEYPATTERN entry), and a notice per store per phase is what + # made one clean commit from a linked worktree print up to five lines, which reads + # as the hook running five times (iss-2609181122202952). The format and the count + # are announced together, once per commit, after every store has been read. _lineno=0 _count=0 @@ -616,20 +615,18 @@ load_store() { stores+=("$_label") done < "$_store" - # Announce the entry count actually read, BEFORE the scan; the format was announced - # before the loop, so a refusal raised inside it already carried the one fact that - # decides what every other line means. A silent downgrade — a stripped - # `# abcd-banlist: keyed` first line — turns every keyed entry into a non-matching - # whole-line pattern while the count stays >=1, so the zero-entry warning below never - # fires. The announced pair makes what the guard actually read visible at commit - # time, so a downgrade cannot be silent. No line content is printed here — only the - # count and the format. + # Record the format and the entry count actually read, for the one notice printed + # BEFORE the scan. A silent downgrade — a stripped `# abcd-banlist: keyed` first + # line — turns every keyed entry into a non-matching whole-line pattern while the + # count stays >=1, so the zero-entry warning below never fires. The announced pair + # makes what the guard actually read visible at commit time, so a downgrade cannot + # be silent. No line content is recorded here — only the count and the format. if [ "$_keyed" -eq 1 ]; then if [ "$_count" -eq 1 ]; then noun="entry"; else noun="entries"; fi - echo "pre-commit: abcd name-guard: keyed store$_tag, $_count $noun" >&2 + notice="${notice:+$notice; }keyed store$_tag, $_count $noun" else if [ "$_count" -eq 1 ]; then noun="pattern"; else noun="patterns"; fi - echo "pre-commit: abcd name-guard: legacy store$_tag, $_count $noun" >&2 + notice="${notice:+$notice; }legacy store$_tag, $_count $noun" fi # A store that parsed to NOTHING checks exactly as much as an absent one, so it is @@ -651,16 +648,20 @@ load_store() { } # The inherited store is read FIRST, so a developer standing in the linked worktree -# sees what they inherit before what they declared. Its path is named once, here: a -# refusal below names the entry's key, and the key alone is a phantom in a checkout -# whose own store does not carry it. +# sees what they inherit before what they declared. Its path is named once, in the +# notice: a refusal below names the entry's key, and the key alone is a phantom in a +# checkout whose own store does not carry it. if [ "$primary_present" -eq 1 ]; then - echo "pre-commit: abcd name-guard: inheriting $primary_label" >&2 load_store "$primary_banlist" "$primary_label" 1 fi if [ "$local_present" -eq 1 ]; then load_store "$banlist" "$banlist" 0 fi +# ONE notice line per commit, naming every store read: its format, its count, and +# where an inherited one lives. +if [ -n "$notice" ]; then + echo "pre-commit: abcd name-guard: $notice" >&2 +fi # --- the candidate content: staged BLOBS, not diff text ---------------------- diff --git a/internal/core/ahoy/banlist_scaffold_test.go b/internal/core/ahoy/banlist_scaffold_test.go index 1b4344f85..c1ce714d5 100644 --- a/internal/core/ahoy/banlist_scaffold_test.go +++ b/internal/core/ahoy/banlist_scaffold_test.go @@ -1034,6 +1034,88 @@ func TestScaffoldedGuardHookInheritsThePrimaryStoreInAWorktree(t *testing.T) { } } +// TestScaffoldedGuardHookAnnouncesOncePerCommit is iss-2609181122202952 on the +// SCAFFOLDED template, which is the copy the report came from: a managed repo's +// commit from a linked worktree printed three name-guard notice lines (the +// inheritance, then the format and the count of the one store it read), which reads +// as the hook running three times. One commit, one notice line. +func TestScaffoldedGuardHookAnnouncesOncePerCommit(t *testing.T) { + if _, err := exec.LookPath("bash"); err != nil { + t.Skip("bash unavailable") + } + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git unavailable") + } + setupHermetic(t) + repo := t.TempDir() + env := gittest.Env(t) + gitIn := func(dir string, args ...string) (string, error) { + cmd := exec.Command("git", append([]string{"-C", dir}, args...)...) + cmd.Env = env + out, err := cmd.CombinedOutput() + return string(out), err + } + for _, args := range [][]string{ + {"init"}, + {"config", "user.name", "Alice Example"}, + {"config", "user.email", "alice@example.com"}, + } { + if out, err := gitIn(repo, args...); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + if _, err := Install(repo, installOpts(), RefusingPrompter{}); err != nil { + t.Fatal(err) + } + src, err := os.ReadFile(filepath.Join(repo, filepath.FromSlash(GuardHookRelPath))) + if err != nil { + t.Fatal(err) + } + hooksDir := filepath.Join(repo, ".git", "hooks") + if err := os.MkdirAll(hooksDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(hooksDir, "pre-commit"), src, 0o755); err != nil { + t.Fatal(err) + } + store := filepath.Join(repo, filepath.FromSlash(banlist.PrivateRelPath)) + if err := os.WriteFile(store, []byte("# abcd-banlist: keyed\nlab-host carol-server\\.example\\.net\n"), 0o600); err != nil { + t.Fatal(err) + } + if out, err := gitIn(repo, "add", "-A"); err != nil { + t.Fatalf("git add: %v\n%s", err, out) + } + if out, err := gitIn(repo, "-c", "core.hooksPath=/dev/null", "commit", "-m", "seed"); err != nil { + t.Fatalf("seed commit: %v\n%s", err, out) + } + linked := filepath.Join(t.TempDir(), "linked") + if out, err := gitIn(repo, "worktree", "add", "-b", "linked", linked); err != nil { + t.Skipf("git worktree add unavailable: %v\n%s", err, out) + } + if err := os.WriteFile(filepath.Join(linked, "notes.md"), []byte("nothing sensitive here\n"), 0o644); err != nil { + t.Fatal(err) + } + if out, err := gitIn(linked, "add", "notes.md"); err != nil { + t.Fatalf("git add: %v\n%s", err, out) + } + out, err := gitIn(linked, "commit", "-m", "clean") + if err != nil { + t.Fatalf("clean content was refused:\n%s", out) + } + var notices []string + for _, l := range strings.Split(out, "\n") { + if strings.Contains(l, "abcd name-guard:") { + notices = append(notices, l) + } + } + if len(notices) != 1 { + t.Fatalf("a clean commit from a linked worktree printed %d name-guard notice lines, want 1:\n%s", len(notices), out) + } + if !strings.Contains(notices[0], "primary checkout") || !strings.Contains(notices[0], "keyed store") { + t.Errorf("the one notice does not say which store was inherited and in what format:\n%s", notices[0]) + } +} + // TestScaffoldedGuardHookRefusesAMirrorInsideAnotherCheckout is the worktree // resolution's confinement, pinned on the SCAFFOLDED template rather than on this // repo's dogfood copy — the two halves must stay in lockstep, and a fix applied to diff --git a/internal/core/ahoy/defaults/pre-commit b/internal/core/ahoy/defaults/pre-commit index 259564d51..401ca19e5 100644 --- a/internal/core/ahoy/defaults/pre-commit +++ b/internal/core/ahoy/defaults/pre-commit @@ -440,6 +440,7 @@ fi rc=0 entries=0 +notice="" keys=() pats=() lines=() @@ -464,7 +465,7 @@ load_store() { _inherited="$3" # 1 when this is the primary checkout's store, read as a fallback _tag="" if [ "$_inherited" -eq 1 ]; then - _tag=" (inherited from the primary checkout)" + _tag=" (inherited from $_label)" fi # Refusals raised while parsing THIS store belong to it. Compared against the # entering value, so the zero-entry warning below is not suppressed by a refusal @@ -514,14 +515,12 @@ load_store() { fi fi - # Announce the FORMAT before any line is read, so a refusal below carries the one - # piece of context that decides what every other line means. The count follows the - # loop, because it is not known until then. - if [ "$_keyed" -eq 1 ]; then - echo "pre-commit: abcd name-guard: reading a keyed store$_tag" >&2 - else - echo "pre-commit: abcd name-guard: reading a legacy store$_tag" >&2 - fi + # The format is NOT announced here, before the loop. Every refusal the loop raises + # already names the reading it applies (a keyed line that does not parse says it is + # not a KEYPATTERN entry), and a notice per store per phase is what + # made one clean commit from a linked worktree print up to five lines, which reads + # as the hook running five times. The format and the count are announced + # together, once per commit, after every store has been read. _lineno=0 _count=0 @@ -613,20 +612,18 @@ load_store() { stores+=("$_label") done < "$_store" - # Announce the entry count actually read, BEFORE the scan; the format was announced - # before the loop, so a refusal raised inside it already carried the one fact that - # decides what every other line means. A silent downgrade — a stripped - # `# abcd-banlist: keyed` first line — turns every keyed entry into a non-matching - # whole-line pattern while the count stays >=1, so the zero-entry warning below never - # fires. The announced pair makes what the guard actually read visible at commit - # time, so a downgrade cannot be silent. No line content is printed here — only the - # count and the format. + # Record the format and the entry count actually read, for the one notice printed + # BEFORE the scan. A silent downgrade — a stripped `# abcd-banlist: keyed` first + # line — turns every keyed entry into a non-matching whole-line pattern while the + # count stays >=1, so the zero-entry warning below never fires. The announced pair + # makes what the guard actually read visible at commit time, so a downgrade cannot + # be silent. No line content is recorded here — only the count and the format. if [ "$_keyed" -eq 1 ]; then if [ "$_count" -eq 1 ]; then noun="entry"; else noun="entries"; fi - echo "pre-commit: abcd name-guard: keyed store$_tag, $_count $noun" >&2 + notice="${notice:+$notice; }keyed store$_tag, $_count $noun" else if [ "$_count" -eq 1 ]; then noun="pattern"; else noun="patterns"; fi - echo "pre-commit: abcd name-guard: legacy store$_tag, $_count $noun" >&2 + notice="${notice:+$notice; }legacy store$_tag, $_count $noun" fi # A store that parsed to NOTHING checks exactly as much as an absent one, so it is @@ -648,16 +645,20 @@ load_store() { } # The inherited store is read FIRST, so a developer standing in the linked worktree -# sees what they inherit before what they declared. Its path is named once, here: a -# refusal below names the entry's key, and the key alone is a phantom in a checkout -# whose own store does not carry it. +# sees what they inherit before what they declared. Its path is named once, in the +# notice: a refusal below names the entry's key, and the key alone is a phantom in a +# checkout whose own store does not carry it. if [ "$primary_present" -eq 1 ]; then - echo "pre-commit: abcd name-guard: inheriting $primary_label" >&2 load_store "$primary_banlist" "$primary_label" 1 fi if [ "$local_present" -eq 1 ]; then load_store "$banlist" "$banlist" 0 fi +# ONE notice line per commit, naming every store read: its format, its count, and +# where an inherited one lives. +if [ -n "$notice" ]; then + echo "pre-commit: abcd name-guard: $notice" >&2 +fi # --- the candidate content: staged BLOBS, not diff text ---------------------- diff --git a/internal/core/banlist/hook_test.go b/internal/core/banlist/hook_test.go index 7876e0f1f..c853a4d9b 100644 --- a/internal/core/banlist/hook_test.go +++ b/internal/core/banlist/hook_test.go @@ -944,6 +944,69 @@ func TestPreCommitHook_LinkedWorktreeStoreWinsOverThePrimary(t *testing.T) { }) } +// noticeLines returns the hook's name-guard NOTICE lines: every `abcd name-guard:` +// line, which is the success-path announcement. Refusals and the loud banners carry +// their own prefixes and are not counted. +func noticeLines(out string) []string { + var got []string + for _, l := range strings.Split(out, "\n") { + if strings.Contains(l, "abcd name-guard:") { + got = append(got, l) + } + } + return got +} + +// TestPreCommitHook_AnnouncesOncePerCommit is iss-2609181122202952. The guard +// announced each store it read on two lines (the format before the parse, the count +// after it) and prefixed an inherited store with a third, so a clean commit from a +// linked worktree printed three notice lines, and five once the worktree carried a +// store of its own. A notice that repeats reads as a hook running several times. One +// commit, one notice line, however many stores were read — and that line still names +// each store's format, its count, and where an inherited one lives. +func TestPreCommitHook_AnnouncesOncePerCommit(t *testing.T) { + t.Run("standalone checkout", func(t *testing.T) { + r := newHookRepo(t, keyedBanlist) + r.write("note.md", "nothing sensitive here\n") + r.git("add", "note.md") + if blocked, out := r.commit(); blocked { + t.Fatalf("clean content was refused\n%s", out) + } else if got := noticeLines(out); len(got) != 1 { + t.Errorf("a clean commit printed %d name-guard notice lines, want 1\n%s", len(got), out) + } else if !strings.Contains(got[0], "keyed store") || !strings.Contains(got[0], "1 entry") { + t.Errorf("the notice does not name the format and the count it read\n%s", got[0]) + } + }) + + for name, local := range map[string]string{ + "linked worktree inheriting": "", + "linked worktree inheriting beside its own": "legacy-name\n", + } { + t.Run(name, func(t *testing.T) { + _, linked := newWorktreeCase(t, keyedBanlist) + if local != "" { + linked.writeBanlist(local) + } + linked.write("note.md", "nothing sensitive here\n") + linked.git("add", "note.md") + blocked, out := linked.commit() + if blocked { + t.Fatalf("clean content was refused\n%s", out) + } + got := noticeLines(out) + if len(got) != 1 { + t.Fatalf("a clean commit from a linked worktree printed %d name-guard notice lines, want 1\n%s", len(got), out) + } + if !strings.Contains(got[0], "primary checkout") || !strings.Contains(got[0], "keyed store") { + t.Errorf("the one notice does not say which store was inherited and in what format\n%s", got[0]) + } + if local != "" && !strings.Contains(got[0], "legacy store") { + t.Errorf("the one notice does not name the worktree's own store beside the inherited one\n%s", got[0]) + } + }) + } +} + // TestPreCommitHook_LinkedWorktreeSaysWhereTheEntryCameFrom: an inherited refusal // names a key the developer will not find in the checkout they are standing in, so // the guard says which store it came from. Without it the remedy — edit the primary From 2563f8d02ce8870655a402de48f3b459026bd7a9 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:28:03 +0100 Subject: [PATCH 02/19] =?UTF-8?q?chore:=20resolve=20iss-2609181122202952?= =?UTF-8?q?=20=E2=80=94=20the=20name=20guard=20announces=20once=20per=20co?= =?UTF-8?q?mmit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves: iss-2609181122202952 Assisted-by: Claude:claude-opus-5-5 --- ...commit-name-guard-prints-its-inherited-store-notice.md | 8 ++++++++ 1 file changed, 8 insertions(+) rename .abcd/work/issues/{open => resolved}/iss-2609181122202952-the-pre-commit-name-guard-prints-its-inherited-store-notice.md (67%) diff --git a/.abcd/work/issues/open/iss-2609181122202952-the-pre-commit-name-guard-prints-its-inherited-store-notice.md b/.abcd/work/issues/resolved/iss-2609181122202952-the-pre-commit-name-guard-prints-its-inherited-store-notice.md similarity index 67% rename from .abcd/work/issues/open/iss-2609181122202952-the-pre-commit-name-guard-prints-its-inherited-store-notice.md rename to .abcd/work/issues/resolved/iss-2609181122202952-the-pre-commit-name-guard-prints-its-inherited-store-notice.md index 6a59a18b6..dddbae577 100644 --- a/.abcd/work/issues/open/iss-2609181122202952-the-pre-commit-name-guard-prints-its-inherited-store-notice.md +++ b/.abcd/work/issues/resolved/iss-2609181122202952-the-pre-commit-name-guard-prints-its-inherited-store-notice.md @@ -9,6 +9,14 @@ found_during: "Gropius managed-repo session gropiusllm-56, commits from a second origin: researcher-authored production_mode: hand-written found_at: ".githooks/pre-commit" +resolution: "The guard ran once per commit; it printed a notice per store per phase (three lines from a linked worktree inheriting one store, five when the worktree carried its own). Each store now records its format and count and one line names every store read, in this repository's hook and the scaffolded template." +impact: fix +resolved_by: + commit: "5a6626b5882e61daada3d38eaabcc38eeac695ba" --- The pre-commit name guard prints its inherited-store notice three times per commit from a linked worktree. The committed .githooks/pre-commit announces the itd-150 fallback once per run, at a single site ("pre-commit: abcd name-guard: inheriting the primary checkout's " on stderr), so three copies on one commit mean the hook body ran three times for that commit, which points at the dispatch (the global hooks dispatcher plus the repo hook, or a pre-merge-commit run) rather than at the notice itself. Relayed from the Gropius managed-repo session gropiusllm-56 on 2026-09-18 at v0.9.0, which called it cosmetic. Recorded because the repetition is also a measurement: if the guard runs three times, the scan costs three times what it should, and one run is the one whose refusal counts. Wanted: establish which invocations produce the three runs and either de-duplicate the dispatch or print the notice once per commit. + +## Grounds + +- pursued: a clean commit from a linked worktree prints exactly one name-guard notice line naming each store's format, count and an inherited store's location; a second notice line from any store layout would show it wrong From aebace46ea69c27bcf2db33c85dfdc57ce33d75a Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:34:15 +0100 Subject: [PATCH 03/19] feat(hooks): refuse a session URL or tool footer before the commit exists 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 --- .githooks/commit-msg | 114 ++++++++++ AGENTS.md | 6 +- CONTRIBUTING.md | 5 +- .../surface/cli/githook_commitmsg_test.go | 204 ++++++++++++++++++ 4 files changed, 327 insertions(+), 2 deletions(-) create mode 100755 .githooks/commit-msg create mode 100644 internal/surface/cli/githook_commitmsg_test.go diff --git a/.githooks/commit-msg b/.githooks/commit-msg new file mode 100755 index 000000000..b5eaecd2e --- /dev/null +++ b/.githooks/commit-msg @@ -0,0 +1,114 @@ +#!/usr/bin/env bash +# Outbound gate, commit-message half — run on every clone via the global +# ~/.githooks dispatcher (core.hooksPath), or by pointing this clone's hooks at +# this directory: git config core.hooksPath .githooks +# +# Refuses a commit whose message carries a LIVE AGENT-SESSION URL or a tool's own +# attribution footer, the two halves of the outbound policy AGENTS.md states +# (§ Attribution and acknowledgements). One reached three commit messages and two +# pull-request bodies of a managed public repository (iss-2609061438431625), and a +# merged commit message comes out only by rewriting a protected branch. CI judges +# every commit message of a pull request with the same check +# (scripts/check-attribution.sh), but by then the message is already in the +# author's history and on the forge: this hook is the half that fails before the +# commit exists. +# +# The judgement is `abcd lint outbound`, never a regex here: the session-URL +# detector is a pattern plus an opacity classifier that POSIX ERE cannot express +# (see internal/surface/cli/lint_outbound.go), so a shell copy would be weaker than +# the policy it claims to enforce. It runs through `go run ./cmd/abcd` from this +# checkout, the rule for every abcd invocation in a source checkout: an installed +# abcd is whatever was last released, which in this repository is stale by +# construction. +# +# FAILS CLOSED. A checkout with no abcd source or no Go toolchain cannot judge the +# message, and a hook that cannot judge must not pass silently: a green commit that +# means "the check was skipped" is indistinguishable from one that means "there was +# nothing to find". Go is a prerequisite for developing this repository, so the +# refusal names the missing piece rather than asking for anything new. +# +# REACH, stated rather than implied. git runs this hook for `git commit` (an +# amend and a reword through `git commit` included) and for a `git merge` that +# creates a commit. `git commit --no-verify`, `git rebase`, `git cherry-pick`, +# `git am` and a message edited on the forge bypass it by construction; the CI +# gate over the pull request's range is the backstop for all of them. +set -euo pipefail +case $- in *x*) set +x ;; esac + +msg_file="${1:-}" +if [ -z "$msg_file" ] || [ ! -f "$msg_file" ]; then + echo "commit-msg: BLOCKED — git passed no message file to judge." >&2 + exit 1 +fi + +# Where the abcd source is. The hook's own directory first: its parent is the +# checkout the hook was committed in, whichever path git invoked it by (the global +# dispatcher's absolute path, or `.githooks/commit-msg` relative to the working +# tree under core.hooksPath). The working tree next, for a copy installed under +# .git/hooks. Pure parameter expansion resolves the first candidate before any +# directory change. +hook_dir="${0%/*}" +case "$hook_dir" in /*) ;; *) hook_dir="$PWD/$hook_dir" ;; esac +src="" +for candidate in "$hook_dir/.." "$(git rev-parse --show-toplevel 2>/dev/null || true)"; do + [ -n "$candidate" ] || continue + if [ -f "$candidate/go.mod" ] && [ -d "$candidate/cmd/abcd" ]; then + src="$(cd "$candidate" && pwd)" + break + fi +done +if [ -z "$src" ]; then + echo "commit-msg: BLOCKED — no abcd source to judge this commit message with." >&2 + echo " the outbound check runs \`go run ./cmd/abcd lint outbound\` from the checkout" >&2 + echo " that holds this hook, and neither the hook's checkout nor this working tree" >&2 + echo " carries cmd/abcd. Point the clone's hooks at the committed directory:" >&2 + echo " git config core.hooksPath .githooks" >&2 + exit 1 +fi +if ! command -v go >/dev/null 2>&1; then + echo "commit-msg: BLOCKED — go is not on PATH, so the outbound check cannot run." >&2 + echo " the check refuses a live session URL or a tool attribution footer in the" >&2 + echo " message, and a check that cannot run must not pass. Install the Go toolchain" >&2 + echo " go.mod declares." >&2 + exit 1 +fi + +# The text git will record. Everything below a scissors line is the `git commit -v` +# diff, which git discards: judging it would refuse a commit for a fixture it +# stages rather than for anything in the message. Comment lines are KEPT and judged: +# whether git strips them depends on the cleanup mode (`-m` keeps a `#` line), and +# over-judging a comment only ever refuses more. +artefact="$(mktemp "${TMPDIR:-/tmp}/abcd-commit-msg.XXXXXX")" +trap 'rm -f "$artefact"' EXIT INT TERM HUP +awk '/^[^[:alnum:][:space:]]+ -+ >8 -+$/ { exit } { print }' "$msg_file" >"$artefact" + +# An empty message is git's to refuse (or to allow, under --allow-empty-message); +# there is nothing in it that could leak. +if ! grep -q '[^[:space:]]' "$artefact"; then + exit 0 +fi + +# Git exports its private environment (GIT_DIR, GIT_INDEX_FILE et al.) to a hook; +# the Go build must not see it. -buildvcs=false keeps the build from asking git +# about this checkout at all while a commit holds its index. +rc=0 +out="$(cd "$src" && env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE -u GIT_OBJECT_DIRECTORY \ + -u GIT_ALTERNATE_OBJECT_DIRECTORIES -u GIT_COMMON_DIR -u GIT_PREFIX \ + go run -buildvcs=false ./cmd/abcd lint outbound --label "commit message" --root "$src" "$artefact" 2>&1)" || rc=$? + +case "$rc" in + 0) + printf '%s\n' "$out" | sed 's/^/commit-msg: /' >&2 + ;; + 1) + echo "commit-msg: BLOCKED — the commit message breaks the outbound policy." >&2 + printf '%s\n' "$out" | sed 's/^/ /' >&2 + echo " delete the line named above and commit again." >&2 + exit 1 + ;; + *) + echo "commit-msg: BLOCKED — the outbound check could not judge the message (exit $rc)." >&2 + printf '%s\n' "$out" | sed 's/^/ /' >&2 + exit 1 + ;; +esac diff --git a/AGENTS.md b/AGENTS.md index 394fa5872..df95e90f1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -411,7 +411,11 @@ irreversible; guessing downward costs nothing.** class into every store-before-commit redactor, `abcd lint`'s privacy rule refuses either shape in any committed file, and the `harness_leak` lint rule refuses it in the record and the docs. One definition, three wired surfaces - (itd-152). A fourth exists as a primitive with no front door: + (itd-152). A commit message is judged by the check-direction front door onto + the same policy, `abcd lint outbound`, twice: the committed + `.githooks/commit-msg` hook refuses either shape before the commit exists, and + the attribution gate in CI judges every commit message of a pull request and + its body again. A fourth exists as a primitive with no front door: `scanner.ScrubOutbound` sanitises one outbound artefact and is covered by tests, but no command or plugin verb calls it, because `spc-45` deliberately scopes a forge client out. The three wired surfaces judge text that is already diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9c56621fe..53796338c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -58,7 +58,10 @@ inbound = outbound statement is the whole of it. runs; `make fmt` applies it). The repository ships its hooks in [`.githooks/`](.githooks/); they are per-machine opt-in — run `git config core.hooksPath .githooks` once per clone to arm the - pre-commit name guard and the pre-push preflight. + pre-commit name guard, the commit-msg outbound check (it refuses a live + agent-session URL or a tool's attribution footer in a commit message, through + `go run ./cmd/abcd lint outbound`, and refuses the commit when it cannot run + the check) and the pre-push preflight. - **Conventional-commit prefixes** (`feat`/`fix`/`docs`/`chore`/`refactor`/`test`/`ci`), no scopes; short title, body explains why. - A user-facing change **resolves its issue or ships its intent in the same diff**; diff --git a/internal/surface/cli/githook_commitmsg_test.go b/internal/surface/cli/githook_commitmsg_test.go new file mode 100644 index 000000000..02857c97b --- /dev/null +++ b/internal/surface/cli/githook_commitmsg_test.go @@ -0,0 +1,204 @@ +package cli + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/intentdriven/abcd/internal/gittest" +) + +// The committed commit-msg hook is the LOCAL half of the outbound gate +// (iss-2609061438431625): a front door onto `abcd lint outbound` that git runs +// before a commit exists. CI judges every commit message of a pull request with +// the same verb, but by then the message is in the author's history and on the +// forge; these tests hold that the hook refuses first, through a real `git +// commit` and `git merge`, and that it fails closed when it cannot judge. + +// commitMsgHookCase is a throwaway repository whose hooks path is this checkout's +// committed .githooks directory, so git runs the hook exactly as a clone does. +type commitMsgHookCase struct { + t *testing.T + root string // this checkout: the hook and the abcd source + dir string // the throwaway repository + env []string + hooks string +} + +func newCommitMsgHookCase(t *testing.T) *commitMsgHookCase { + t.Helper() + for _, tool := range []string{"bash", "git", "go"} { + if _, err := exec.LookPath(tool); err != nil { + t.Skipf("%s unavailable", tool) + } + } + // The Go caches are read BEFORE gittest.Env moves HOME: the hook runs `go run`, + // and a HOME-relative default cache would rebuild abcd from cold per test. + goEnv, err := exec.Command("go", "env", "GOCACHE", "GOMODCACHE", "GOPATH").Output() + if err != nil { + t.Skipf("go env: %v", err) + } + vals := strings.Split(strings.TrimSpace(string(goEnv)), "\n") + if len(vals) != 3 { + t.Fatalf("go env returned %d values, want 3", len(vals)) + } + top := exec.Command("git", "rev-parse", "--show-toplevel") + top.Env = gittest.Env(t) + out, err := top.Output() + if err != nil { + t.Skip("not in a git checkout") + } + root := strings.TrimSpace(string(out)) + hooks := filepath.Join(root, ".githooks") + c := &commitMsgHookCase{ + t: t, root: root, dir: t.TempDir(), hooks: hooks, + env: append(gittest.Env(t), "GOCACHE="+vals[0], "GOMODCACHE="+vals[1], "GOPATH="+vals[2], "GOFLAGS=-mod=mod", "GIT_EDITOR=true"), + } + c.git("init", "-q", "-b", "main") + c.git("config", "user.name", "Alice Example") + c.git("config", "user.email", "alice@example.com") + c.git("config", "core.hooksPath", hooks) + return c +} + +func (c *commitMsgHookCase) tryGit(args ...string) (string, error) { + c.t.Helper() + cmd := exec.Command("git", append([]string{"-C", c.dir}, args...)...) + cmd.Env = c.env + out, err := cmd.CombinedOutput() + return string(out), err +} + +func (c *commitMsgHookCase) git(args ...string) string { + c.t.Helper() + out, err := c.tryGit(args...) + if err != nil { + c.t.Fatalf("git %v: %v\n%s", args, err, out) + } + return out +} + +// commitWith stages one file and commits it with the message read from a file, +// returning whether the commit was refused and everything git and the hooks wrote. +func (c *commitMsgHookCase) commitWith(file, msg string, extra ...string) (refused bool, out string) { + c.t.Helper() + if err := os.WriteFile(filepath.Join(c.dir, file), []byte("content of "+file+"\n"), 0o644); err != nil { + c.t.Fatal(err) + } + c.git("add", file) + msgFile := filepath.Join(c.t.TempDir(), "msg") + if err := os.WriteFile(msgFile, []byte(msg), 0o644); err != nil { + c.t.Fatal(err) + } + out, err := c.tryGit(append([]string{"commit", "-q", "-F", msgFile}, extra...)...) + return err != nil, out +} + +// sessionURL assembles a live-shaped session URL at runtime, so no committed file +// carries one (the payload scan and `abcd lint` refuse a literal). +func sessionURL() string { + return "https://agent-host.dev/code/" + "session_" + testOutboundSessionID +} + +func TestCommitMsgHookRefusesASessionURL(t *testing.T) { + c := newCommitMsgHookCase(t) + refused, out := c.commitWith("a.txt", "fix: the walk\n\nSession: "+sessionURL()+"\n\nAssisted-by: Claude:claude-opus-5\n") + if !refused { + t.Fatalf("a commit message carrying a live session URL was committed\n%s", out) + } + if !strings.Contains(out, "commit-msg: BLOCKED") { + t.Errorf("the refusal does not say the commit-msg hook refused it\n%s", out) + } + if strings.Contains(out, testOutboundSessionID) { + t.Errorf("the refusal republished the session id it refused\n%s", out) + } + if _, err := c.tryGit("rev-parse", "--verify", "HEAD"); err == nil { + t.Errorf("a commit exists after the refusal; the hook must fail before the commit is made") + } +} + +func TestCommitMsgHookRefusesAToolFooter(t *testing.T) { + c := newCommitMsgHookCase(t) + refused, out := c.commitWith("a.txt", "fix: the walk\n\n🤖 Generated with [Some Tool](https://sometool.dev)\n") + if !refused { + t.Fatalf("a commit message carrying a tool attribution footer was committed\n%s", out) + } +} + +func TestCommitMsgHookPassesACleanMessage(t *testing.T) { + c := newCommitMsgHookCase(t) + refused, out := c.commitWith("a.txt", "fix: the walk skips a record family\n\nAssisted-by: Claude:claude-opus-5\n") + if refused { + t.Fatalf("a clean commit message was refused\n%s", out) + } + if !strings.Contains(out, "commit-msg:") { + t.Errorf("the hook passed silently; a pass must say what it checked, or it reads the same as a hook that never ran\n%s", out) + } +} + +// Everything below a scissors line is the `git commit -v` diff, which git discards. +// Judging it would refuse a commit for a fixture it stages, not for its message. +func TestCommitMsgHookIgnoresTheVerboseDiffBelowTheScissors(t *testing.T) { + c := newCommitMsgHookCase(t) + msg := "fix: the walk\n\nAssisted-by: Claude:claude-opus-5\n" + + "# ------------------------ >8 ------------------------\n" + + "+" + sessionURL() + "\n" + // git truncates at the scissors only for a message it opened an editor on, so + // the commit is "edited" through an editor that changes nothing. + refused, out := c.commitWith("a.txt", msg, "--cleanup=scissors", "-e") + if refused { + t.Fatalf("a session URL in the discarded diff below the scissors refused the commit\n%s", out) + } + if logged := c.git("log", "-1", "--format=%B"); strings.Contains(logged, testOutboundSessionID) { + t.Fatalf("the fixture's premise is wrong: git kept the text below the scissors\n%s", logged) + } +} + +// git runs commit-msg for a merge that creates a commit, and git does NOT run +// pre-commit for one: the merge message is one of the places a leaked URL lands. +func TestCommitMsgHookRefusesASessionURLInAMergeMessage(t *testing.T) { + c := newCommitMsgHookCase(t) + if refused, out := c.commitWith("a.txt", "chore: seed\n\nAssisted-by: None\n"); refused { + t.Fatalf("seed refused\n%s", out) + } + c.git("checkout", "-q", "-b", "side") + if refused, out := c.commitWith("b.txt", "feat: side\n\nAssisted-by: None\n"); refused { + t.Fatalf("side commit refused\n%s", out) + } + c.git("checkout", "-q", "main") + out, err := c.tryGit("merge", "--no-ff", "-m", "Merge side\n\n"+sessionURL(), "side") + if err == nil { + t.Fatalf("a merge commit carrying a live session URL was made\n%s", out) + } + if strings.Contains(out, testOutboundSessionID) { + t.Errorf("the refusal republished the session id it refused\n%s", out) + } +} + +// A copy installed where no abcd source can be found cannot judge the message, and +// must refuse rather than pass: a pass that means "skipped" reads exactly like a +// pass that means "clean". +func TestCommitMsgHookFailsClosedWithoutAnAbcdSource(t *testing.T) { + c := newCommitMsgHookCase(t) + src, err := os.ReadFile(filepath.Join(c.hooks, "commit-msg")) + if err != nil { + t.Fatal(err) + } + local := filepath.Join(c.dir, ".git", "hooks") + if err := os.MkdirAll(local, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(local, "commit-msg"), src, 0o755); err != nil { + t.Fatal(err) + } + c.git("config", "core.hooksPath", local) + refused, out := c.commitWith("a.txt", "fix: the walk\n\nAssisted-by: None\n") + if !refused { + t.Fatalf("a hook with no abcd source to run passed the commit\n%s", out) + } + if !strings.Contains(out, "no abcd source") { + t.Errorf("the refusal does not name what is missing\n%s", out) + } +} From 955d7e8ddad7a6d48f2b46f89deb77a865550525 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:34:32 +0100 Subject: [PATCH 04/19] =?UTF-8?q?chore:=20resolve=20iss-2609061438431625?= =?UTF-8?q?=20=E2=80=94=20commit=20messages=20are=20judged=20before=20the?= =?UTF-8?q?=20commit=20exists?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ...pository-still-has-no-local-gate-on-a-commit.md | 14 ++++++++++++++ ...-url-reached-three-commit-messages-and-two-p.md | 8 ++++++++ 2 files changed, 22 insertions(+) create mode 100644 .abcd/work/issues/open/iss-2609250834251447-a-managed-repository-still-has-no-local-gate-on-a-commit.md rename .abcd/work/issues/{open => resolved}/iss-2609061438431625-a-claude-session-url-reached-three-commit-messages-and-two-p.md (75%) diff --git a/.abcd/work/issues/open/iss-2609250834251447-a-managed-repository-still-has-no-local-gate-on-a-commit.md b/.abcd/work/issues/open/iss-2609250834251447-a-managed-repository-still-has-no-local-gate-on-a-commit.md new file mode 100644 index 000000000..19bd7da42 --- /dev/null +++ b/.abcd/work/issues/open/iss-2609250834251447-a-managed-repository-still-has-no-local-gate-on-a-commit.md @@ -0,0 +1,14 @@ +--- +schema_version: 1 +id: "iss-2609250834251447" +slug: "a-managed-repository-still-has-no-local-gate-on-a-commit" +severity: "major" +category: "security" +source: "agent-finding" +found_during: "autonomous run A resumed 2026-09-25" +origin: researcher-authored +production_mode: hand-written +found_at: ".githooks/commit-msg" +--- + +A managed repository still has no local gate on a commit message carrying a live agent-session URL or a tool attribution footer, and nothing guards the text handed to the forge CLI for a pull request, an issue or a comment. abcd's own repository refuses both shapes in a commit message through its committed commit-msg hook, which runs go run ./cmd/abcd lint outbound from the source checkout; a managed repository has no source checkout, so the scaffolded form of that hook needs three product decisions first: how it finds an abcd binary (a git hook has no plugin root, so only the PATH rung survives), whether it fails closed or open when none is found, and whether abcd ahoy installs it by default or on opt-in. Split out of iss-2609061438431625 when its local half landed for this repository. diff --git a/.abcd/work/issues/open/iss-2609061438431625-a-claude-session-url-reached-three-commit-messages-and-two-p.md b/.abcd/work/issues/resolved/iss-2609061438431625-a-claude-session-url-reached-three-commit-messages-and-two-p.md similarity index 75% rename from .abcd/work/issues/open/iss-2609061438431625-a-claude-session-url-reached-three-commit-messages-and-two-p.md rename to .abcd/work/issues/resolved/iss-2609061438431625-a-claude-session-url-reached-three-commit-messages-and-two-p.md index fd9ca6de7..a2b53ef5a 100644 --- a/.abcd/work/issues/open/iss-2609061438431625-a-claude-session-url-reached-three-commit-messages-and-two-p.md +++ b/.abcd/work/issues/resolved/iss-2609061438431625-a-claude-session-url-reached-three-commit-messages-and-two-p.md @@ -11,6 +11,14 @@ production_mode: hand-written deferred_after: "v0.8.0" deferral_reason: "Half of this is fixed in this cut and half of it cannot be, which is why the record stays open rather than being resolved. The CI half is closed: a check-direction primitive now judges outbound text against the same policy the scrubber holds, with its own front door, wired over every commit message in a pull request's range and over the pull request body. What is not closed is the local gate, and it cannot be without three product decisions nobody has taken. A git hook has no plugin root, so only the PATH rung of the hardened four-rung ladder survives in one, and the choice between failing closed on a missing binary, failing open, or baking an absolute path that a plugin update then invalidates is a decision whose blast radius is every managed repository. Whether such a hook installs by default or opt-in is a second decision with the same reach. Deferred so those are taken deliberately rather than inside a release. The exposure that remains is named in the record: a leaked message is already in the author's history before CI ever runs." found_at: "hooks (pre-commit name guard), internal (lint privacy-hygiene, guard)" +resolution: "The local gate landed for this repository: the committed .githooks/commit-msg hook runs go run ./cmd/abcd lint outbound on every commit and merge message and refuses a live session URL or a tool attribution footer before the commit exists, failing closed when it cannot run. The CI half (every commit message of a pull request and its body) landed earlier. The managed-repository form of the hook and the forge-CLI text guard need three product decisions and are carried by iss-2609250834251447." +impact: fix +resolved_by: + commit: "aebace46ea69c27bcf2db33c85dfdc57ce33d75a" --- A Claude session URL reached three commit messages and two PR bodies of a managed public repo and nothing in the abcd guard stack stopped it — not acceptable. The lint policy text says a live session URL or a tool attribution footer is refused wherever it is committed, but privacy-hygiene only scans tracked files; the committed pre-commit name guard checks the private banlist (empty by default) and never the commit message; there is no commit-msg hook, and nothing looks at the text handed to the GitHub CLI for pull requests or issues. The harness's own attribution instruction (a Claude-Session trailer on every commit and PR) is exactly the pattern the policy names, so the guard must catch it mechanically: a commit-msg hook (and pre-merge-commit) that rejects claude.ai/code/session links and known AI attribution footers; a public banned-token family for those patterns so CI enforces it on every pushed commit message in the PR range, not only on files; and guard coverage of the GitHub CLI's pull-request and issue text. Recovery is expensive — a merged commit message can only be removed by rewriting a protected branch — so this has to fail before the commit exists. + +## Grounds + +- pursued: a commit or merge whose message carries a live session URL or a tool footer is refused in this repository before any commit object exists; a leaked URL reaching a commit made through git commit or git merge here with the hooks path armed would show it wrong From b5b0897e4a5c1ac920500d2ae287a05648bd8d15 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:38:32 +0100 Subject: [PATCH 05/19] fix(hooks): gate a push before it connects, on the committed tree 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 --- .abcd/work/DECISIONS.md | 1 + .githooks/pre-push | 87 +++-- AGENTS.md | 11 + CONTRIBUTING.md | 5 +- Makefile | 21 +- internal/surface/cli/githook_prepush_test.go | 330 +++++++++++++++++++ scripts/preflight-receipt.sh | 141 ++++++++ 7 files changed, 571 insertions(+), 25 deletions(-) create mode 100644 internal/surface/cli/githook_prepush_test.go create mode 100755 scripts/preflight-receipt.sh diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 5e1b2f2e2..b9cf939af 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -2531,3 +2531,4 @@ together (the script's header says why there is no escape hatch). - 2026-09-24 — Correction to the entry above on the 02:02Z ruling: the clause "a fork of an agent is still an agent" is not part of the product thinker's ruling. It is the run's own lane rule for autonomous run A, which counts a fork toward the ceiling because a fork is an agent alive. The ruling itself is the rest of that entry: no pauses, a ceiling of four sub-agents in any mix of roles, and rotation at about 60% of context unchanged (lane implementer, on review of the followup lane; the ledger is append-only, so the entry above stands as written). - 2026-09-24 — Release v0.10.0 is cut by autonomous run A, and the run's agenda line is: approve the publish step. Under ruling A2 of the product thinker's run A interview (2026-09-23 07:52Z, "approve the publish step": the product thinker authorises the run to approve the release environment itself once every gate is green), the run approves the `release` environment's deployment of v0.10.0 only after the merge queue, the verify job and every other gate on the tagged commit report green, and stops with a handover instead if any does not. The cut: v0.10.0, impact breaking, 43 records since v0.9.0 (nine shipped intents, thirty-four resolved or declined issues, four of them breaking), content commit 64ea8f62, the composer's payload accepted on the first ingest and recomposed once for two docs-currency findings. Both semantic gates ran at tier full: docs-currency-reviewer (Fable 5.1) with three findings, two fixed and one deferred because the load check's intent stays planned pending the product thinker's ruling on the stray definition (iss-2609231947544298); the brief-surface cross-check (40 pinned checkers, Opus 5.5, four at a time under the run's ceiling) with 154 findings, all deferred to their records: four user-facing ones captured as iss-2609240519413467, iss-2609240519418856, iss-2609240519471816 and iss-2609240519427388, one inside an appendix chapter captured as iss-2609240519422232, which records itd-147's ac-6 as not met, and the design-record drift to the systematic brief pass iss-2609091956001547. - 2026-09-24 — v0.10.0 is published. PR #693 merged as 1ac8b3a0, and auto-release run 35963282477 tagged it. The `release` environment was approved under ruling A2 once all 30 checks on the tagged commit had settled (25 success, 5 skipped by design). The release was published at 2026-09-24T06:33:10Z with four binaries, `checksums.txt`, the plugin archive `abcd-plugin-v0.10.0.zip` (its sha256 equals the marketplace pin, 1ab2acd1…) and the rendered site, which was deployed. Verified locally afterwards: the darwin-arm64 binary's checksum and its build attestation, and the binary reports v0.10.0. +- 2026-09-25 — A push is gated before it opens its connection, by receipt (iss-2608290810036869, iss-2608210738378295; product thinker's ruling M16 of 2026-09-23, "check before connect"). The committed `.githooks/pre-push` no longer runs `make preflight`: git opens the connection before the hook, and a preflight inside it outlasted the transport's idle timeout, so a push reported success and moved nothing. `make preflight` ends by minting a receipt for HEAD under the checkout's local tier (`scripts/preflight-receipt.sh`), and only when the working tree matched HEAD (nothing staged, unstaged or untracked) both when the run began, read while the Makefile is parsed, and when it ended, with HEAD unmoved, so the gates read the tree CI checks out. The hook refuses a push whose commit is new to the remote and carries no receipt from any worktree of the repository; a commit the remote already holds (a tag on a merged commit) passes. Alternatives not taken: a push wrapper that runs the preflight and then `git push --no-verify` (it normalises `--no-verify`, and a plain `git push` would go ungated); keepalive settings on the transport (the hook would still hold the connection for ten minutes); a preflight on a clean export of HEAD (a second full tree and build per push, where refusing a divergent tree costs nothing). `git push --no-verify` skips the hook exactly as before, and CI stays the authority (lane hooks, autonomous run A). diff --git a/.githooks/pre-push b/.githooks/pre-push index fd4ad4c88..e4090550f 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -1,6 +1,6 @@ #!/usr/bin/env bash # Pre-push gate, run on every clone via the global ~/.githooks dispatcher -# (core.hooksPath). Two jobs: +# (core.hooksPath). Two jobs, and neither runs the preflight: # # 1. Local branch protection — a fast local echo of the server-side rulesets # (applied and mirrored under .abcd/work/rulesets/), so a bad push fails @@ -8,26 +8,48 @@ # this layer only covers pushes that go through these hooks. It refuses: # a direct push to the protected branch, a deletion of it, and any # non-fast-forward (force) push to any branch. -# 2. `make preflight` must pass natively before a push leaves this machine — -# the load check first (load-check, a warning, never a failure), then -# the six lint gates (lint-reviews, lint-issues, lint-decisions, -# record-lint, issue-drift, docs-lint), -# the site-render gate and both tagged eval lanes (smoke, -# evals-cold-reading), then build, vet, test and the race-enabled -# internal tests. The eval lanes are named because the untagged `go test` -# step cannot compile them, so a defect there would otherwise reach a push -# unopposed (iss-2608311632382737). CI's check job adds the `make fmt-check` -# format gate on top of those Go steps — run it before pushing, since this -# hook does not; the secret-scan, workflow-audit, -# dependency-review and govulncheck lanes run in Actions only, and the -# record-lint job there repeats the reviews-charter, issue-resolution and -# decisions-append gates preflight already ran here. +# 2. A passing `make preflight` is REQUIRED for every commit a push would +# put on the remote for the first time, and this hook checks for its +# receipt rather than running it. git opens the push's connection BEFORE +# it runs this hook, so a preflight run here held the connection open for +# its whole length; one outlasted the transport's idle timeout, the server +# closed the connection, and the push reported success while moving +# nothing (iss-2608290810036869). The gate therefore runs first, as its own +# command: `make preflight` mints a receipt for HEAD when it passes on a +# tree that matches HEAD (scripts/preflight-receipt.sh), and this hook +# refuses a push whose new commit carries none, in milliseconds. A commit +# the remote already holds needs no receipt — a tag on a merged commit, a +# branch re-pushed at a published tip — because it leaves nothing new. +# +# The receipt also closes the working-tree gap (iss-2608210738378295): the +# gates read the working tree, CI reads the commit, and a receipt is minted +# only when the two were the same for the whole run. A staged rename with +# its follow-up edit left unstaged passes every gate on the tree and fails +# CI on the commit; here it earns no receipt, so the push is refused. +# +# What the preflight runs: the load check first (load-check, a warning, +# never a failure), then the six lint gates (lint-reviews, lint-issues, +# lint-decisions, record-lint, issue-drift, docs-lint), the site-render +# gate and both tagged eval lanes (smoke, evals-cold-reading), then build, +# vet, test and the race-enabled internal tests. The eval lanes are named +# because the untagged `go test` step cannot compile them, so a defect +# there would otherwise reach a push unopposed (iss-2608311632382737). CI's +# check job adds the `make fmt-check` format gate on top of those Go steps — +# run it before pushing, since the preflight does not; the secret-scan, +# workflow-audit, dependency-review and govulncheck lanes run in Actions +# only, and the record-lint job there repeats the reviews-charter, +# issue-resolution and decisions-append gates preflight already ran here. +# +# `git push --no-verify` skips this hook, as it skips every hook; CI re-runs +# every gate on the pushed commit and is the authority either way. set -euo pipefail cd "$(git rev-parse --show-toplevel)" protected="main" zero="0000000000000000000000000000000000000000" +remote_name="${1:-}" +missing="" while read -r local_ref local_sha remote_ref remote_sha; do branch="${remote_ref#refs/heads/}" @@ -40,8 +62,11 @@ while read -r local_ref local_sha remote_ref remote_sha; do exit 1 fi + # A deletion carries no commit to gate. + [ "$local_sha" != "$zero" ] || continue + # Non-fast-forward (force) push guard on an existing remote branch. - if [ "$remote_sha" != "$zero" ] && [ "$local_sha" != "$zero" ]; then + if [ "$remote_sha" != "$zero" ]; then if git merge-base --is-ancestor "$remote_sha" "$local_sha" 2>/dev/null; then : # fast-forward — fine else @@ -54,12 +79,28 @@ while read -r local_ref local_sha remote_ref remote_sha; do fi fi fi -done -# Git exports its private environment (GIT_DIR et al.) to hooks; anything the -# gate spawns that runs git — the test suite's throwaway repos above all — -# would silently operate on THIS repository instead of its own (iss-28). -unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_OBJECT_DIRECTORY \ - GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_COMMON_DIR GIT_PREFIX + # The commit the ref will point at: a tag is peeled to the commit it names. + commit="$(git rev-parse --verify --quiet "$local_sha^{commit}" 2>/dev/null || true)" + [ -n "$commit" ] || continue + + # Already on a remote: pushing it moves no new commit off this machine. + if [ -n "$(git for-each-ref --count=1 --contains "$commit" refs/remotes 2>/dev/null)" ]; then + continue + fi + + if ! scripts/preflight-receipt.sh check "$commit"; then + missing="$missing $commit:$local_ref" + fi +done -make preflight +if [ -n "$missing" ]; then + for entry in $missing; do + echo "pre-push: BLOCKED — no passing preflight for ${entry%%:*} (${entry#*:})." >&2 + done + echo " the gate runs BEFORE the push opens its connection, never inside it: run" >&2 + echo " \`make preflight\` with that commit checked out and nothing uncommitted beside it" >&2 + echo " (staged, unstaged and untracked changes all withhold the receipt), then push" >&2 + echo " again${remote_name:+ to $remote_name}." >&2 + exit 1 +fi diff --git a/AGENTS.md b/AGENTS.md index df95e90f1..9953999a1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -103,6 +103,17 @@ go test ./internal/core/ # a single package go test -run TestStatus ./internal/core/ # a single test ``` +**A push is gated before it connects.** The committed `.githooks/pre-push` hook +never runs the preflight: git opens a push's connection before it runs the hook, +and a preflight inside it outlasted the transport's idle timeout, so the push +reported success and moved nothing. `make preflight` ends by minting a receipt +for HEAD instead, and only when the working tree matched HEAD — nothing staged, +unstaged or untracked — both when the run began and when it ended, so the gates +read exactly the tree CI checks out. The hook refuses a push whose new commit has +no receipt. The sequence is: commit everything, `make preflight`, then a plain +`git push`. A receipt minted in any worktree of the checkout counts, and a commit +the remote already holds (a tag on a merged commit) needs none. + **In a source checkout of abcd, every abcd invocation is `go run ./cmd/abcd ` from the repo root** — never the plugin-root binary and never an `abcd` on PATH. Both are whatever version was last published, and in this repository diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 53796338c..9d7a78314 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -61,7 +61,10 @@ inbound = outbound statement is the whole of it. pre-commit name guard, the commit-msg outbound check (it refuses a live agent-session URL or a tool's attribution footer in a commit message, through `go run ./cmd/abcd lint outbound`, and refuses the commit when it cannot run - the check) and the pre-push preflight. + the check) and the pre-push receipt check: a push of a commit the remote does + not hold yet needs a passing `make preflight` run on that commit with nothing + uncommitted beside it, and the hook checks the receipt that run mints rather + than running the preflight while the push holds its connection open. - **Conventional-commit prefixes** (`feat`/`fix`/`docs`/`chore`/`refactor`/`test`/`ci`), no scopes; short title, body explains why. - A user-facing change **resolves its issue or ships its intent in the same diff**; diff --git a/Makefile b/Makefile index c4f060697..6d09fec49 100644 --- a/Makefile +++ b/Makefile @@ -278,7 +278,8 @@ scaffold-sync: scaffold-sync-check: @go run ./cmd/scaffold-sync -check -# Pre-push gate (invoked by .githooks/pre-push): the load check first (a +# Pre-push gate (run before a push, never by it: .githooks/pre-push checks the +# receipt the last step mints, below): the load check first (a # warning, never a failure: load-check), then the six lint gates # (lint-reviews, lint-issues, lint-decisions, record-lint, issue-drift, # docs-lint), the @@ -308,6 +309,24 @@ preflight: load-check lint-reviews lint-issues lint-decisions record-lint issue- go vet ./... go test ./... go test -race ./internal/... + @scripts/preflight-receipt.sh mint "$(PREFLIGHT_BEGAN)" + +# The push receipt (iss-2608290810036869, iss-2608210738378295). The pre-push hook +# never runs this target: git opens a push's connection before it runs the hook, +# and a preflight inside it held that connection open until the transport's idle +# timeout closed it. So the gate runs first, as its own command, and its last step +# mints a receipt the hook checks in milliseconds. A receipt vouches for HEAD only +# when the tree matched HEAD — nothing staged, unstaged or untracked — both when +# the run began and when it ended, which is what makes the gates' reading of the +# working tree a reading of the commit CI will check out. +# +# The starting state is read while this Makefile is PARSED, which is before any +# prerequisite runs; a recipe line or a prerequisite would run after, or in parallel +# with, the gates. It is read only when `preflight` is named on the command line, so +# no other target pays for a git status. +ifneq ($(filter preflight,$(MAKECMDGOALS)),) +PREFLIGHT_BEGAN := $(shell scripts/preflight-receipt.sh state) +endif # The marker tells a check started inside this preflight (the eval harness's own, # under `smoke` and `evals-cold-reading`) that the preflight's check already diff --git a/internal/surface/cli/githook_prepush_test.go b/internal/surface/cli/githook_prepush_test.go new file mode 100644 index 000000000..998578d41 --- /dev/null +++ b/internal/surface/cli/githook_prepush_test.go @@ -0,0 +1,330 @@ +package cli + +import ( + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + "testing" + + "github.com/intentdriven/abcd/internal/gittest" +) + +// The committed pre-push hook and the preflight receipt it reads +// (scripts/preflight-receipt.sh). Two records meet here: +// +// - iss-2608290810036869: git opens a push's connection before it runs +// pre-push, so a preflight run inside the hook held the connection for its +// whole length, and one outlasted the idle timeout — the push reported +// success and moved nothing. The gate runs BEFORE the push (ruling M16, +// 2026-09-23): `make preflight` mints a receipt, and the hook only checks it. +// - iss-2608210738378295: the gates read the working tree while CI reads the +// commit, so a staged/unstaged divergence passed locally and failed CI. A +// receipt is minted only when the tree matched HEAD for the whole run. +// +// The fixture is a throwaway clone carrying copies of the committed hook and +// script, with a bare repository as its remote, and a Makefile whose preflight +// only leaves a marker: the hook must never run it. + +type prePushCase struct { + t *testing.T + dir string // the working clone + origin string // its bare remote + env []string +} + +func newPrePushCase(t *testing.T) *prePushCase { + t.Helper() + for _, tool := range []string{"bash", "git"} { + if _, err := exec.LookPath(tool); err != nil { + t.Skipf("%s unavailable", tool) + } + } + top := exec.Command("git", "rev-parse", "--show-toplevel") + top.Env = gittest.Env(t) + out, err := top.Output() + if err != nil { + t.Skip("not in a git checkout") + } + root := strings.TrimSpace(string(out)) + + base := t.TempDir() + c := &prePushCase{t: t, dir: filepath.Join(base, "work"), origin: filepath.Join(base, "origin.git"), env: gittest.Env(t)} + c.run(base, "git", "init", "-q", "--bare", "-b", "main", c.origin) + c.run(base, "git", "init", "-q", "-b", "main", c.dir) + c.git("config", "user.name", "Alice Example") + c.git("config", "user.email", "alice@example.com") + + for _, rel := range []string{".githooks/pre-push", "scripts/preflight-receipt.sh"} { + src, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(rel))) + if err != nil { + t.Fatalf("read the committed %s: %v", rel, err) + } + c.write(rel, string(src), 0o755) + } + c.write(".gitignore", ".abcd/.work.local/\n", 0o644) + // A preflight that passes and leaves a marker. The hook must never run it: a + // preflight inside pre-push is the connection-holding shape this replaces. + c.write("Makefile", "preflight:\n\t@mkdir -p .abcd/.work.local && touch .abcd/.work.local/preflight-ran\n", 0o644) + c.write("seed.md", "seed\n", 0o644) + c.git("add", "-A") + c.git("-c", "core.hooksPath=/dev/null", "commit", "-q", "-m", "seed") + c.git("remote", "add", "origin", c.origin) + c.git("-c", "core.hooksPath=/dev/null", "push", "-q", "origin", "main") + c.git("config", "core.hooksPath", ".githooks") + c.git("checkout", "-q", "-b", "feature") + return c +} + +func (c *prePushCase) run(dir string, name string, args ...string) (string, error) { + c.t.Helper() + cmd := exec.Command(name, args...) + cmd.Dir = dir + cmd.Env = c.env + out, err := cmd.CombinedOutput() + return string(out), err +} + +func (c *prePushCase) git(args ...string) string { + c.t.Helper() + out, err := c.run(c.dir, "git", args...) + if err != nil { + c.t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(out) +} + +func (c *prePushCase) write(rel, content string, mode os.FileMode) { + c.t.Helper() + p := filepath.Join(c.dir, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + c.t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), mode); err != nil { + c.t.Fatal(err) + } +} + +func (c *prePushCase) commit(rel, content string) string { + c.t.Helper() + c.write(rel, content, 0o644) + c.git("add", rel) + c.git("commit", "-q", "-m", "add "+rel) + return c.git("rev-parse", "HEAD") +} + +// receipt runs the script's subcommands in the clone. +func (c *prePushCase) receipt(dir string, args ...string) (string, error) { + c.t.Helper() + return c.run(dir, "bash", append([]string{filepath.Join(dir, "scripts", "preflight-receipt.sh")}, args...)...) +} + +// preflight stands in for `make preflight`: the state before, the gates, the mint +// after. The Makefile wiring that does the same is pinned separately below. +func (c *prePushCase) preflight(dir string) string { + c.t.Helper() + began, err := c.receipt(dir, "state") + if err != nil { + c.t.Fatalf("state: %v\n%s", err, began) + } + out, err := c.receipt(dir, "mint", strings.TrimSpace(began)) + if err != nil { + c.t.Fatalf("mint: %v\n%s", err, out) + } + return out +} + +func (c *prePushCase) push(args ...string) (string, error) { + c.t.Helper() + return c.run(c.dir, "git", append([]string{"push"}, args...)...) +} + +// remoteTip is the commit the bare remote holds for a branch, or "". +func (c *prePushCase) remoteTip(branch string) string { + c.t.Helper() + out, err := c.run(c.origin, "git", "rev-parse", "--verify", "--quiet", "refs/heads/"+branch) + if err != nil { + return "" + } + return strings.TrimSpace(out) +} + +func TestPrePushRefusesANewCommitWithoutAReceiptAndNeverRunsThePreflight(t *testing.T) { + c := newPrePushCase(t) + c.commit("feature.md", "a feature\n") + out, err := c.push("origin", "feature") + if err == nil { + t.Fatalf("a commit with no passing preflight was pushed\n%s", out) + } + if !strings.Contains(out, "no passing preflight") { + t.Errorf("the refusal does not say what is missing\n%s", out) + } + if tip := c.remoteTip("feature"); tip != "" { + t.Errorf("the remote moved to %s although the push was refused", tip) + } + if _, err := os.Stat(filepath.Join(c.dir, ".abcd", ".work.local", "preflight-ran")); err == nil { + t.Errorf("the hook ran the preflight itself, which holds the push's connection open for its whole length") + } +} + +func TestPrePushPassesACommitWithAReceipt(t *testing.T) { + c := newPrePushCase(t) + head := c.commit("feature.md", "a feature\n") + if out := c.preflight(c.dir); !strings.Contains(out, "receipt minted for "+head[:12]) { + t.Fatalf("a preflight on a clean tree minted no receipt\n%s", out) + } + if out, err := c.push("origin", "feature"); err != nil { + t.Fatalf("a commit with a passing preflight was refused\n%s", out) + } + if tip := c.remoteTip("feature"); tip != head { + t.Errorf("the remote holds %q, want %s", tip, head) + } +} + +// The iss-2608210738378295 shape, reproduced: a staged rename committed while the +// follow-up edit to the renamed file stays unstaged. The working tree holds the +// edit and the commit does not, so a gate reading the tree passes what CI fails. +func TestPreflightReceiptIsWithheldFromATreeThatDiffersFromTheCommit(t *testing.T) { + cases := map[string]func(c *prePushCase){ + "staged rename, unstaged edit": func(c *prePushCase) { + c.commit("iss-1.md", "id: iss-1\n") + c.git("mv", "iss-1.md", "iss-2.md") + c.git("commit", "-q", "-m", "renumber") + c.write("iss-2.md", "id: iss-2\n", 0o644) // the edit that never got staged + }, + "staged change left uncommitted": func(c *prePushCase) { + c.commit("a.md", "one\n") + c.write("a.md", "two\n", 0o644) + c.git("add", "a.md") + }, + "untracked file": func(c *prePushCase) { + c.commit("a.md", "one\n") + c.write("b.md", "untracked\n", 0o644) + }, + } + for name, diverge := range cases { + t.Run(name, func(t *testing.T) { + c := newPrePushCase(t) + diverge(c) + out := c.preflight(c.dir) + if strings.Contains(out, "receipt minted for") { + t.Fatalf("a preflight on a tree that differs from HEAD minted a receipt\n%s", out) + } + if !strings.Contains(out, "no push receipt") { + t.Errorf("the preflight does not say why no receipt was minted\n%s", out) + } + if pushed, err := c.push("origin", "feature"); err == nil { + t.Fatalf("a commit whose tree diverged during its preflight was pushed\n%s", pushed) + } + }) + } +} + +func TestPreflightReceiptIsWithheldWhenHeadMovesDuringTheRun(t *testing.T) { + c := newPrePushCase(t) + c.commit("a.md", "one\n") + began, err := c.receipt(c.dir, "state") + if err != nil { + t.Fatal(err) + } + c.commit("b.md", "two\n") // a commit made while the gates were running + out, err := c.receipt(c.dir, "mint", strings.TrimSpace(began)) + if err != nil { + t.Fatal(err) + } + if strings.Contains(out, "receipt minted for") { + t.Fatalf("a preflight during which HEAD moved minted a receipt\n%s", out) + } + if pushed, err := c.push("origin", "feature"); err == nil { + t.Fatalf("a commit no preflight ran on was pushed\n%s", pushed) + } +} + +// A commit the remote already holds leaves nothing new: a tag on a merged commit +// needs no receipt, or cutting a release from the forge's merge would be refused. +func TestPrePushLetsACommitTheRemoteHoldsThroughWithoutAReceipt(t *testing.T) { + c := newPrePushCase(t) + c.git("fetch", "-q", "origin") + c.git("tag", "-a", "v0.0.1", "-m", "v0.0.1", "origin/main") + if out, err := c.push("origin", "v0.0.1"); err != nil { + t.Fatalf("a tag on a commit the remote already holds was refused\n%s", out) + } +} + +// A receipt minted in a sibling worktree of the same repository counts: the same +// commit is the same tree everywhere, and a branch preflighted in its own worktree +// is often pushed from the primary checkout. +func TestPrePushHonoursAReceiptFromASiblingWorktree(t *testing.T) { + c := newPrePushCase(t) + sibling := filepath.Join(t.TempDir(), "sibling") + c.git("worktree", "add", "-q", "-b", "lane", sibling) + if err := os.WriteFile(filepath.Join(sibling, "lane.md"), []byte("lane\n"), 0o644); err != nil { + t.Fatal(err) + } + if out, err := c.run(sibling, "git", "add", "lane.md"); err != nil { + t.Fatal(out) + } + if out, err := c.run(sibling, "git", "commit", "-q", "-m", "lane"); err != nil { + t.Fatal(out) + } + if out := c.preflight(sibling); !strings.Contains(out, "receipt minted for") { + t.Fatalf("the sibling's clean preflight minted no receipt\n%s", out) + } + if out, err := c.push("origin", "lane"); err != nil { + t.Fatalf("a commit preflighted in a sibling worktree was refused from the primary checkout\n%s", out) + } +} + +// The protected-branch refusal is unchanged, and it needs no receipt to fire. +func TestPrePushStillRefusesADirectPushToMain(t *testing.T) { + c := newPrePushCase(t) + c.git("checkout", "-q", "main") + c.commit("direct.md", "direct\n") + c.preflight(c.dir) + out, err := c.push("origin", "main") + if err == nil { + t.Fatalf("a direct push to main was accepted\n%s", out) + } + if !strings.Contains(out, "protected branch") { + t.Errorf("the refusal does not name the protected branch\n%s", out) + } +} + +// `make preflight` is what mints the receipt, so its recipe must record the tree's +// state before any gate runs and mint after the last one. Read through `make -n`, +// which prints the recipe the real target would run without running it. +func TestMakePreflightMintsTheReceiptAfterItsLastGate(t *testing.T) { + if _, err := exec.LookPath("make"); err != nil { + t.Skip("make unavailable") + } + top := exec.Command("git", "rev-parse", "--show-toplevel") + top.Env = gittest.Env(t) + out, err := top.Output() + if err != nil { + t.Skip("not in a git checkout") + } + cmd := exec.Command("make", "-n", "preflight") + cmd.Dir = strings.TrimSpace(string(out)) + cmd.Env = gittest.Env(t) + dry, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("make -n preflight: %v\n%s", err, dry) + } + lines := strings.Split(strings.TrimSpace(string(dry)), "\n") + last := lines[len(lines)-1] + mint := regexp.MustCompile(`^scripts/preflight-receipt\.sh mint "[0-9a-f]{40} (clean|dirty)"$`) + if !mint.MatchString(last) { + t.Fatalf("the preflight recipe's last step is %q; want the receipt minted from the state recorded "+ + "before the first gate ran\n%s", last, dry) + } + raceAt, mintAt := -1, len(lines)-1 + for i, l := range lines { + if strings.HasPrefix(l, "go test -race") { + raceAt = i + } + } + if raceAt < 0 || raceAt > mintAt { + t.Errorf("the receipt is not minted after the race-enabled tests\n%s", dry) + } +} diff --git a/scripts/preflight-receipt.sh b/scripts/preflight-receipt.sh new file mode 100755 index 000000000..980a3a547 --- /dev/null +++ b/scripts/preflight-receipt.sh @@ -0,0 +1,141 @@ +#!/usr/bin/env bash +# The preflight receipt: how `make preflight` tells the pre-push hook that the +# gates passed on exactly the commit being pushed, so the hook never runs the +# preflight itself (iss-2608290810036869, iss-2608210738378295). +# +# WHY A RECEIPT. The pre-push hook used to run the whole preflight, and git opens +# the 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 +# died while the preflight's own passing output scrolled past the failure — a +# push that reported success and moved nothing. The ruling (M16, 2026-09-23) is +# check before connect: the preflight runs to completion before the push opens +# its connection. So the preflight runs first, as its own command, and mints a +# receipt; the hook only reads receipts, which takes milliseconds. +# +# WHY "ON A CLEAN TREE". The gates read the WORKING TREE. A tree that differs from +# HEAD — a staged rename whose follow-up edit is unstaged, an untracked record — +# can pass every gate while the committed tree fails CI, because CI checks out the +# commit. A receipt is minted only when the tree matched HEAD (no staged, unstaged +# or untracked change) when the preflight began AND when it ended, with HEAD +# unmoved between: then what the gates read is what the push ships. Files git +# ignores are outside that comparison, as they are outside the commit. +# +# The receipt is a file named by the full commit id under the checkout's local +# tier, .abcd/.work.local/preflight-receipts/. It is a local convenience gate, not +# a security boundary: CI re-runs every gate on the pushed commit and is the +# authority, and `git push --no-verify` skips this layer exactly as it always did. +# +# Usage: +# preflight-receipt.sh state print " clean|dirty" for this tree +# preflight-receipt.sh mint "" mint a receipt for HEAD if the tree was +# clean at and is clean at the same +# HEAD now; otherwise say why none is minted +# preflight-receipt.sh check exit 0 when a receipt for exists +# in any worktree of this repository +# +# Exit 0 on success; `check` exits 1 when there is no receipt; 2 on a usage fault. +set -euo pipefail + +cd "$(git rev-parse --show-toplevel)" + +receipts_rel=".abcd/.work.local/preflight-receipts" +# How many receipts a checkout keeps. Each is a few bytes; the cap only stops the +# directory growing without bound across months of preflights. +keep=50 + +state() { + local head status + head="$(git rev-parse --verify --quiet HEAD 2>/dev/null || true)" + [ -n "$head" ] || head="none" + status="$(git status --porcelain --untracked-files=normal 2>/dev/null)" || { + echo "$head dirty" + return 0 + } + if [ -z "$status" ]; then + echo "$head clean" + else + echo "$head dirty" + fi +} + +mint() { + local began="$1" began_head began_tree now now_head now_tree + began_head="${began%% *}" + began_tree="${began##* }" + now="$(state)" + now_head="${now%% *}" + now_tree="${now##* }" + if [ -z "$began" ]; then + echo "preflight: no push receipt minted — the tree's state when the run began was not recorded" + echo " (the receipt is minted only by \`make preflight\` named on the command line)." + return 0 + fi + if [ "$began_head" = "none" ] || [ "$now_head" = "none" ]; then + echo "preflight: no push receipt minted — there is no commit to vouch for." + return 0 + fi + if [ "$began_tree" != "clean" ] || [ "$now_tree" != "clean" ]; then + echo "preflight: no push receipt minted — the working tree differed from HEAD (staged, unstaged" + echo " or untracked changes), so these gates did not read the tree a push ships." + echo " Commit or set aside the changes and run \`make preflight\` again before pushing." + return 0 + fi + if [ "$began_head" != "$now_head" ]; then + echo "preflight: no push receipt minted — HEAD moved during the run (${began_head:0:12} -> ${now_head:0:12})," + echo " so no single commit is what these gates read. Run \`make preflight\` again." + return 0 + fi + mkdir -p "$receipts_rel" + printf 'commit %s\nminted %s\n' "$now_head" "$(date -u +%Y-%m-%dT%H:%M:%SZ)" >"$receipts_rel/$now_head" + # Keep the newest $keep receipts. The names are full commit ids, so a plain + # listing is safe to word-split. + # shellcheck disable=SC2012 + ls -t "$receipts_rel" | tail -n +"$((keep + 1))" | while IFS= read -r old; do + rm -f "$receipts_rel/$old" + done + echo "preflight: push receipt minted for ${now_head:0:12} — a push of that commit passes the pre-push gate." +} + +check() { + local commit="$1" wt + case "$commit" in + *[!0-9a-f]* | "") return 2 ;; + esac + if [ -f "$receipts_rel/$commit" ]; then + return 0 + fi + # A commit's receipt is valid in whichever worktree of this repository it was + # minted in: the same commit is the same tracked tree everywhere, and the + # receipt says the gates passed on it with nothing uncommitted beside it. This + # is what lets a branch preflighted in its own worktree be pushed from the + # primary checkout. Only worktrees git itself lists are read. + while IFS= read -r wt; do + case "$wt" in + "worktree "*) wt="${wt#worktree }" ;; + *) continue ;; + esac + if [ -f "$wt/$receipts_rel/$commit" ]; then + return 0 + fi + done <<<"$(git worktree list --porcelain 2>/dev/null || true)" + return 1 +} + +case "${1:-}" in +state) + [ $# -eq 1 ] || exit 2 + state + ;; +mint) + [ $# -eq 2 ] || exit 2 + mint "$2" + ;; +check) + [ $# -eq 2 ] || exit 2 + check "$2" + ;; +*) + echo "usage: preflight-receipt.sh state | mint \"\" | check " >&2 + exit 2 + ;; +esac From bac3c61dab72d07e609577f2379ace22059af483 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:38:47 +0100 Subject: [PATCH 06/19] =?UTF-8?q?chore:=20resolve=20iss-2608290810036869?= =?UTF-8?q?=20=E2=80=94=20a=20push=20is=20gated=20before=20it=20connects?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves: iss-2608290810036869 Assisted-by: Claude:claude-opus-5-5 --- ...-report-success-while-pushing-nothing-when-the-p.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) rename .abcd/work/issues/{open => resolved}/iss-2608290810036869-git-push-can-report-success-while-pushing-nothing-when-the-p.md (68%) diff --git a/.abcd/work/issues/open/iss-2608290810036869-git-push-can-report-success-while-pushing-nothing-when-the-p.md b/.abcd/work/issues/resolved/iss-2608290810036869-git-push-can-report-success-while-pushing-nothing-when-the-p.md similarity index 68% rename from .abcd/work/issues/open/iss-2608290810036869-git-push-can-report-success-while-pushing-nothing-when-the-p.md rename to .abcd/work/issues/resolved/iss-2608290810036869-git-push-can-report-success-while-pushing-nothing-when-the-p.md index deee6e26b..1a5bb1c0e 100644 --- a/.abcd/work/issues/open/iss-2608290810036869-git-push-can-report-success-while-pushing-nothing-when-the-p.md +++ b/.abcd/work/issues/resolved/iss-2608290810036869-git-push-can-report-success-while-pushing-nothing-when-the-p.md @@ -9,6 +9,14 @@ found_during: "intent-implementation-run" found_at: ".githooks/pre-push" deferred_after: "v0.9.0" deferral_reason: "Ruled by the product thinker at the 2026-09-23 run A interview (M16: check before connect: preflight runs to completion before the push opens its connection (a push path that gates first); a build lane owed, not holding the tag)." +resolution: "The pre-push hook no longer runs the preflight, so it no longer holds the push's connection open: make preflight runs first and mints a receipt for HEAD, and the hook refuses in milliseconds a push whose new commit has none (ruling M16, check before connect)." +impact: fix +resolved_by: + commit: "b5b0897e4a5c1ac920500d2ae287a05648bd8d15" --- -git push can report success while pushing nothing, when the pre-push preflight outlasts the SSH idle timeout. The hook runs the full preflight, which takes minutes, and the connection is opened before the hook runs, so the server closes it mid-hook: the push dies with a connection-closed message, the remote ref is unchanged, and the preflight's own passing output scrolls past the failure so the whole thing reads as a clean run. Hit twice in one session. The second time the push was also piped to another command, which returns that command's exit status, so a failed push reported zero. Remedies that worked: set SSH keepalive options for the push, never pipe the push, and confirm with a remote ref listing that the tip actually moved. Worth considering whether the hook should run before the connection is opened, or whether a push wrapper should verify the remote tip afterwards; the silent half is the defect, not the slowness. \ No newline at end of file +git push can report success while pushing nothing, when the pre-push preflight outlasts the SSH idle timeout. The hook runs the full preflight, which takes minutes, and the connection is opened before the hook runs, so the server closes it mid-hook: the push dies with a connection-closed message, the remote ref is unchanged, and the preflight's own passing output scrolls past the failure so the whole thing reads as a clean run. Hit twice in one session. The second time the push was also piped to another command, which returns that command's exit status, so a failed push reported zero. Remedies that worked: set SSH keepalive options for the push, never pipe the push, and confirm with a remote ref listing that the tip actually moved. Worth considering whether the hook should run before the connection is opened, or whether a push wrapper should verify the remote tip afterwards; the silent half is the defect, not the slowness. + +## Grounds + +- pursued: no push waits on a gate while its connection is open, so an idle-timeout close can no longer eat a push; a pre-push hook that still runs a multi-minute step, or a plain git push of an unpreflighted new commit that reaches the remote, would show it wrong From 60179194c97964d3e4154c9a673895302f2c8c06 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:38:48 +0100 Subject: [PATCH 07/19] =?UTF-8?q?chore:=20resolve=20iss-2608210738378295?= =?UTF-8?q?=20=E2=80=94=20the=20push-time=20gate=20reads=20the=20committed?= =?UTF-8?q?=20tree?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves: iss-2608210738378295 Assisted-by: Claude:claude-opus-5-5 --- ...8378295-local-gates-lint-working-tree-not-commit.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) rename .abcd/work/issues/{open => resolved}/iss-2608210738378295-local-gates-lint-working-tree-not-commit.md (65%) diff --git a/.abcd/work/issues/open/iss-2608210738378295-local-gates-lint-working-tree-not-commit.md b/.abcd/work/issues/resolved/iss-2608210738378295-local-gates-lint-working-tree-not-commit.md similarity index 65% rename from .abcd/work/issues/open/iss-2608210738378295-local-gates-lint-working-tree-not-commit.md rename to .abcd/work/issues/resolved/iss-2608210738378295-local-gates-lint-working-tree-not-commit.md index 9fdfa17c4..4b1b6a9b0 100644 --- a/.abcd/work/issues/open/iss-2608210738378295-local-gates-lint-working-tree-not-commit.md +++ b/.abcd/work/issues/resolved/iss-2608210738378295-local-gates-lint-working-tree-not-commit.md @@ -6,6 +6,14 @@ severity: "minor" category: "future-work-seed" source: "review-followup" found_during: "itd-130 session; #395 renumber" +resolution: "The push-time gate is now the committed tree: make preflight mints its push receipt only when the working tree matched HEAD (nothing staged, unstaged or untracked) when the run began and when it ended, and the pre-push hook refuses a new commit without one, so a staged/unstaged divergence fails locally." +impact: fix +resolved_by: + commit: "b5b0897e4a5c1ac920500d2ae287a05648bd8d15" --- -Local record gates validate the WORKING TREE, not the committed/pushed tree, so a partial commit passes them while the committed tree fails CI. Demonstrated live this session: git mv staged a rename (iss-370->377) but the follow-up sed edit to the frontmatter id was left unstaged; git commit captured only the rename, so the committed file had filename iss-377 with frontmatter iss-370 — a record_schema blocker. record-lint had passed at commit time because it reads the working tree (which held the sed edit), and the pre-push make preflight passes for the same reason (the unstaged edit is still present); only CI, checking out the commit, sees the divergence. Fix direction: run the push-time gate against the tree that will actually ship (e.g. a clean worktree of HEAD, or git stash --keep-index before linting), so a working-tree/index divergence cannot pass locally and fail CI. Sibling of iss-147 (guard-load reads guard.json from the working tree). \ No newline at end of file +Local record gates validate the WORKING TREE, not the committed/pushed tree, so a partial commit passes them while the committed tree fails CI. Demonstrated live this session: git mv staged a rename (iss-370->377) but the follow-up sed edit to the frontmatter id was left unstaged; git commit captured only the rename, so the committed file had filename iss-377 with frontmatter iss-370 — a record_schema blocker. record-lint had passed at commit time because it reads the working tree (which held the sed edit), and the pre-push make preflight passes for the same reason (the unstaged edit is still present); only CI, checking out the commit, sees the divergence. Fix direction: run the push-time gate against the tree that will actually ship (e.g. a clean worktree of HEAD, or git stash --keep-index before linting), so a working-tree/index divergence cannot pass locally and fail CI. Sibling of iss-147 (guard-load reads guard.json from the working tree). + +## Grounds + +- pursued: a commit whose working tree diverged from it during its preflight cannot be pushed through the hook; a divergence that still earns a receipt, such as one git status --porcelain does not report, would show it wrong From 1fea1d61228f095175d1af26bf1756c8fdda6787 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:51:31 +0100 Subject: [PATCH 08/19] fix(hooks): give this repository's name guard the template's hardening 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 --- .githooks/pre-commit | 54 +++++++++++++++--- .githooks/pre-merge-commit | 12 +++- internal/core/banlist/hook_test.go | 91 ++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 10 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 3c84543e8..1d0252a0f 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -59,10 +59,35 @@ case $- in *x*) set +x ;; esac # does nothing against `BASH_FUNC_grep%%=() { return 1; }` in the environment: a # function wins over PATH lookup, so the guard would report a clean check for ever # while announcing its entry count. Same class as the PATH prepend, different route. -unset -f git grep mktemp tr cat mkdir rm chmod printf sed head command 2>/dev/null || true +# `read` and `echo` are on the list because they are the two most load-bearing +# names here: with either shadowed, the store parse loop reads zero entries and +# every diagnostic goes quiet — the silent pass this pin exists to prevent. +# `exit`, `test` and `[` are on it because the sweep below can be neutered by a +# shadowed `declare`, and a surviving `exit` function turns every refusal into a +# printed BLOCK that does not block. This list is expansion-free and runs before +# `declare` is ever consulted, so it holds even then. +unset -f git grep mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ 2>/dev/null || true # A field separator the caller cannot choose: an inherited IFS changes how every -# unquoted expansion below splits. +# unquoted expansion below splits — including the sweep loop's, so it is pinned +# before the sweep runs. IFS=$' \t\n' +# Then sweep EVERY function that survives: the list above pins the names this hook +# runs today, but any inherited function left standing is a name a future edit might +# call. Word-splitting `declare -F` output ("declare -f NAME" or "declare -fx NAME" +# per line) under the pinned IFS needs no `read`; globbing is off around the loop so +# a function named with a glob metacharacter cannot dodge it via pathname expansion. +# The sweep is best-effort on top of the fixed list, not a substitute for it: a +# shadowed `declare` silences the sweep entirely, which is exactly why every name +# the refusal paths depend on sits on the expansion-free list above. A shadowed +# `unset` or `builtin` has no in-script remedy (the same capability class as +# running with SHELLOPTS=noexec). +set -f +for _fn in $(declare -F); do + case "$_fn" in declare|-f|-fx) continue ;; esac + unset -f "$_fn" 2>/dev/null || true +done +set +f +unset _fn 2>/dev/null || true # C locale so the byte classes below and grep's case folding do not depend on the # committer's environment. The Go parser pins the same ASCII-only reading. LC_ALL=C @@ -303,7 +328,12 @@ declares_format() { # Windows) `.ABCD/.WORK.LOCAL/Private-Names.txt` is the same file, and a case-exact # test would let the store be staged under a differently-cased path. refuse_tier_path() { - _p="$1" + # Control bytes are stripped before the path is matched or printed: a staged + # path is attacker-authored on the merge half, and a raw escape sequence echoed + # to the terminal can redraw or forge the refusal text around it. Stripping + # before the match can only ever ADD a refusal (the tier's real path carries no + # control bytes), so the scrub fails closed. + _p=$(printf '%s' "$1" | tr -d '[:cntrl:]') [ -n "$_p" ] || return 0 _lc=$(printf '%s' "$_p" | tr '[:upper:]' '[:lower:]') _dir_lc=$(printf '%s' "$banlist_dir" | tr '[:upper:]' '[:lower:]') @@ -329,7 +359,9 @@ refuse_tier_path() { # Exempt when the blob's SECOND line is the example marker. Content scanning is not # affected: an exempt blob is still matched against every entry. refuse_store_copy() { - _p="$1" + # Same control-byte scrub as refuse_tier_path, for the same reason: this path is + # printed. Scrubbing before the basename test can only add a refusal. + _p=$(printf '%s' "$1" | tr -d '[:cntrl:]') _first="$2" _second="$3" if [ "$(normalise_decl "$_second")" = "$example_marker" ]; then @@ -774,8 +806,16 @@ while IFS= read -r -d '' meta; do refuse_tier_path "$staged_path" refuse_tier_path "$src_path" + # The staged PATH itself is scanned like content: a banned name in a FILENAME + # (widgetworks-notes.md) enters history just as surely as one in a file's bytes, so + # a path matching a private pattern refuses by key exactly as content does. + # Appended BEFORE the gitlink skip below: a gitlink has no blob, but its path + # still enters history. + printf '%s\n' "$staged_path" >>"$candidate" + # A gitlink has no blob here; skip it rather than fail-close on a `git show` that - # cannot succeed. Every OTHER git-show failure still refuses (fail closed). + # cannot succeed. Every OTHER git-show failure still refuses (fail closed). Its + # path was already appended above, so a banned name as a submodule path refuses. case "$dstmode" in 160000) continue ;; esac @@ -802,10 +842,6 @@ while IFS= read -r -d '' meta; do cat "$blob" >>"$candidate" # A separator, so a pattern cannot match across the join between two files. printf '\n' >>"$candidate" - # The staged PATH itself is scanned like content: a banned name in a FILENAME - # (widgetworks-notes.md) enters history just as surely as one in a file's bytes, so - # a path matching a private pattern refuses by key exactly as content does. - printf '%s\n' "$staged_path" >>"$candidate" done <"$staged_paths" # Nothing staged to check. Any refusal already recorded above still stands: a store diff --git a/.githooks/pre-merge-commit b/.githooks/pre-merge-commit index d111de270..c026833a8 100755 --- a/.githooks/pre-merge-commit +++ b/.githooks/pre-merge-commit @@ -27,8 +27,18 @@ case $- in *x*) set +x ;; esac # does nothing against `BASH_FUNC_grep%%=() { return 1; }` in the environment: a # function wins over PATH lookup, so the guard would report a clean check for ever # while announcing its entry count. Same class as the PATH prepend, different route. -unset -f git grep mktemp tr cat mkdir rm chmod printf sed head command 2>/dev/null || true +# `read`, `echo`, `exit`, `test` and `[` are pinned for the delegate's sake, and +# every surviving function is swept after the fixed list — see the pre-commit +# half for why, including why the fixed list must carry `exit`. +unset -f git grep mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ 2>/dev/null || true IFS=$' \t\n' +set -f +for _fn in $(declare -F); do + case "$_fn" in declare|-f|-fx) continue ;; esac + unset -f "$_fn" 2>/dev/null || true +done +set +f +unset _fn 2>/dev/null || true LC_ALL=C export LC_ALL PATH=/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin diff --git a/internal/core/banlist/hook_test.go b/internal/core/banlist/hook_test.go index c853a4d9b..bdaed96d9 100644 --- a/internal/core/banlist/hook_test.go +++ b/internal/core/banlist/hook_test.go @@ -1320,3 +1320,94 @@ func TestPreCommitHook_AMirrorInsideAnotherCheckoutDoesNotInheritItsStore(t *tes }) } } + +// TestPreCommitHook_ResistsInheritedShellState holds this repository's own guard to +// the hardening the scaffolded template received on 2026-08-26 and this copy never +// did. Four ways hostile inherited state, or an attacker-authored staged path, got a +// banned name past the guard or forged its output; each is driven through BASH_ENV, +// which bash reads at startup whatever its version. +func TestPreCommitHook_ResistsInheritedShellState(t *testing.T) { + const banned = "the widgetworks deal closes friday\n" + bashEnv := func(t *testing.T, body string) string { + p := filepath.Join(t.TempDir(), "hostile.sh") + if err := os.WriteFile(p, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + return "BASH_ENV=" + p + } + withEnv := func(r *hookRepo, extra string) *hookRepo { + return &hookRepo{t: r.t, dir: r.dir, env: append(append([]string{}, r.env...), extra)} + } + + // read and echo shadowed: the parse loop read zero entries, every diagnostic went + // quiet, and the banned name was committed. + t.Run("read and echo shadowed", func(t *testing.T) { + r := newHookRepo(t, keyedBanlist) + r.write("a.md", banned) + r.git("add", "a.md") + blocked, out := withEnv(r, bashEnv(t, "read() { :; }\necho() { :; }\n")).commit() + if !blocked { + t.Fatalf("the guard passed a banned name under a read/echo shadow\n%s", out) + } + if !strings.Contains(out, "widget-partner") { + t.Errorf("the refusal does not name the key\n%s", out) + } + }) + + // declare and exit shadowed: a surviving exit function turned every refusal into + // a printed BLOCKED that committed anyway. + t.Run("declare and exit shadowed", func(t *testing.T) { + r := newHookRepo(t, keyedBanlist) + r.write("seed.md", "nothing sensitive here\n") + r.git("add", "seed.md") + if blocked, out := r.commit(); blocked { + t.Fatalf("seed refused\n%s", out) + } + r.write("b.md", banned) + r.git("add", "b.md") + withEnv(r, bashEnv(t, "declare() { return 0; }\nexit() { return 0; }\n")).commit() + if tree := r.git("ls-tree", "-r", "--name-only", "HEAD"); strings.Contains(tree, "b.md") { + t.Fatalf("the banned commit landed in history despite the refusal\n%s", tree) + } + }) + + // A gitlink whose PATH is a banned name: the path was appended after the gitlink + // skip, so a submodule path was never scanned. + t.Run("gitlink path", func(t *testing.T) { + r := newHookRepo(t, keyedBanlist) + r.write("seed.md", "nothing sensitive here\n") + r.git("add", "seed.md") + if blocked, out := r.commit(); blocked { + t.Fatalf("seed refused\n%s", out) + } + head := strings.TrimSpace(r.git("rev-parse", "HEAD")) + if out, err := r.tryGit("update-index", "--add", "--cacheinfo", "160000,"+head+",widgetworks-vendored"); err != nil { + t.Skipf("cannot stage a gitlink in this sandbox: %v\n%s", err, out) + } + blocked, out := r.commit() + if !blocked { + t.Fatalf("the guard passed a gitlink named with a banned path\n%s", out) + } + if !strings.Contains(out, "widget-partner") { + t.Errorf("the refusal does not name the key\n%s", out) + } + }) + + // A refused path carrying control bytes was echoed raw, so an escape sequence could + // redraw or forge the refusal text around it. + t.Run("control bytes scrubbed", func(t *testing.T) { + r := newHookRepo(t, keyedBanlist) + evil := ".abcd/.work.local/\x1b[2Kevil.txt" + r.write(evil, "x\n") + if out, err := r.tryGit("add", "-f", evil); err != nil { + t.Skipf("cannot stage a control-byte path here: %v\n%s", err, out) + } + blocked, out := r.commit() + if !blocked { + t.Fatalf("a file inside the gitignored local tier was committed\n%s", out) + } + if strings.ContainsRune(out, '\x1b') { + t.Errorf("a raw ESC byte from the staged path reached the output\n%q", out) + } + }) +} From 9e950006c4c53144249dfdff8861eed22e8ea75a Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:51:32 +0100 Subject: [PATCH 09/19] =?UTF-8?q?chore:=20resolve=20iss-2609250850380420?= =?UTF-8?q?=20=E2=80=94=20this=20repository's=20name=20guard=20carries=20t?= =?UTF-8?q?he=20template's=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves: iss-2609250850380420 Assisted-by: Claude:claude-opus-5-5 --- ...-own-committed-githooks-pre-commit-name.md | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .abcd/work/issues/resolved/iss-2609250850380420-this-repository-s-own-committed-githooks-pre-commit-name.md diff --git a/.abcd/work/issues/resolved/iss-2609250850380420-this-repository-s-own-committed-githooks-pre-commit-name.md b/.abcd/work/issues/resolved/iss-2609250850380420-this-repository-s-own-committed-githooks-pre-commit-name.md new file mode 100644 index 000000000..6172a25cb --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609250850380420-this-repository-s-own-committed-githooks-pre-commit-name.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609250850380420" +slug: "this-repository-s-own-committed-githooks-pre-commit-name" +severity: "major" +category: "security" +source: "agent-finding" +found_during: "autonomous run A resumed 2026-09-25" +origin: researcher-authored +production_mode: hand-written +found_at: ".githooks/pre-commit" +resolution: "The four hardening fixes the template received on 2026-08-26 are ported into this repository's .githooks/pre-commit (function pin and sweep, gitlink path scanned before the skip, control-byte scrub) and the pin into .githooks/pre-merge-commit, with a test driving all four against this repository's hook." +impact: fix +resolved_by: + commit: "1fea1d61228f095175d1af26bf1756c8fdda6787" +--- + +This repository's own committed .githooks/pre-commit name guard lacks the four hardening fixes the scaffolded template received on 2026-08-26 (f95650de): it does not pin read, echo, exit, test and [ against inherited shell functions or sweep the survivors, so a BASH_ENV defining read and echo makes it read zero entries and commit a banned name, and a shadowed declare plus exit turns a printed refusal into a commit; it appends a staged path after the gitlink skip, so a submodule path carrying a banned name is never scanned; and it echoes staged paths raw, so control bytes in a refused path can forge the refusal text. The fix landed on the template alone, and its test drives only the template, so nothing held the dogfood copy to it. The .githooks/pre-merge-commit half lacks the same pin. + +## Grounds + +- pursued: this repository's guard now refuses under a read/echo or declare/exit shadow, scans a gitlink path and never echoes a control byte; any of the four BASH_ENV cases passing a banned name or a raw ESC would show it wrong From fda78cc994109c93f93fffbda76b2d0d172f2b83 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:04:27 +0100 Subject: [PATCH 10/19] chore: defer the managed-repository commit gate out loud 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 --- ...-a-managed-repository-still-has-no-local-gate-on-a-commit.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.abcd/work/issues/open/iss-2609250834251447-a-managed-repository-still-has-no-local-gate-on-a-commit.md b/.abcd/work/issues/open/iss-2609250834251447-a-managed-repository-still-has-no-local-gate-on-a-commit.md index 19bd7da42..76d740b2d 100644 --- a/.abcd/work/issues/open/iss-2609250834251447-a-managed-repository-still-has-no-local-gate-on-a-commit.md +++ b/.abcd/work/issues/open/iss-2609250834251447-a-managed-repository-still-has-no-local-gate-on-a-commit.md @@ -9,6 +9,8 @@ found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written found_at: ".githooks/commit-msg" +deferred_after: "v0.10.0" +deferral_reason: "ruling owed to the product thinker (away; run A 2026-09-25): for the scaffolded commit-msg hook in a managed repository, how does a hook with no plugin root find an abcd binary, does it fail open or closed when none is found, and is it installed by default or on opt-in?" --- A managed repository still has no local gate on a commit message carrying a live agent-session URL or a tool attribution footer, and nothing guards the text handed to the forge CLI for a pull request, an issue or a comment. abcd's own repository refuses both shapes in a commit message through its committed commit-msg hook, which runs go run ./cmd/abcd lint outbound from the source checkout; a managed repository has no source checkout, so the scaffolded form of that hook needs three product decisions first: how it finds an abcd binary (a git hook has no plugin root, so only the PATH rung survives), whether it fails closed or open when none is found, and whether abcd ahoy installs it by default or on opt-in. Split out of iss-2609061438431625 when its local half landed for this repository. From 23c671d531dd1fbf1353d615536dbb0e0ccebfa5 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:34:45 +0100 Subject: [PATCH 11/19] fix(hooks): pin the commit-msg hook's environment before it judges 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 --- .githooks/commit-msg | 112 ++++++++++++++++-- .../surface/cli/githook_commitmsg_test.go | 84 +++++++++++++ 2 files changed, 183 insertions(+), 13 deletions(-) diff --git a/.githooks/commit-msg b/.githooks/commit-msg index b5eaecd2e..ec9c64b31 100755 --- a/.githooks/commit-msg +++ b/.githooks/commit-msg @@ -33,7 +33,83 @@ # `git am` and a message edited on the forge bypass it by construction; the CI # gate over the pull request's range is the backstop for all of them. set -euo pipefail +# --- environment pin: the FIRST statements, before anything is read or run ------- +# The same pin as .githooks/pre-commit, for the same reason: git starts this hook as +# a fresh bash with the committer's environment, and a hook that judges a commit +# must not let that environment answer for it. An exported function +# (`BASH_FUNC_grep%%=() { return 1; }`), a BASH_ENV file or a PATH-prepended tool +# once turned the "nothing to judge" test into yes, and the hook passed a message +# carrying a session URL without judging a byte of it. case $- in *x*) set +x ;; esac +# Drop any inherited function shadowing a name this hook runs. A function wins over +# PATH lookup, so pinning PATH alone does nothing against one. `exit`, `test` and +# `[` are here because a surviving `exit` function turns every refusal into a +# printed BLOCK that does not block; `go` because it is the one tool resolved +# outside the pinned PATH. The list is expansion-free and runs before `declare` is +# consulted, so it holds even when the sweep below is neutered. +unset -f git grep awk mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ go cd pwd trap 2>/dev/null || true +# A field separator the caller cannot choose, pinned before the sweep splits on it. +IFS=$' \t\n' +# Then sweep every function that survives: best-effort on top of the fixed list (a +# shadowed `declare` silences it, which is why the refusal paths' names are on the +# list above). A shadowed `unset` or `builtin` has no in-script remedy. +set -f +for _fn in $(declare -F); do + case "$_fn" in declare|-f|-fx) continue ;; esac + unset -f "$_fn" 2>/dev/null || true +done +set +f +unset _fn 2>/dev/null || true +LC_ALL=C +export LC_ALL +# The Go toolchain lives wherever its installer put it (~/.local/bin, ~/go/bin, +# /usr/local/go/bin), so it is resolved on the INHERITED PATH, to an absolute path, +# before the pin below. That is a deliberate boundary, not an oversight: any real +# `go` builds this checkout's own source and judges the message the same way, so an +# accidental substitute cannot pass what the policy refuses; a fake `go` that forges +# a pass is a deliberate act, the class of `--no-verify`, which CI's check over the +# pull request is the backstop for. +go_bin=$(command -v go 2>/dev/null || true) +case "$go_bin" in /*) ;; *) go_bin="" ;; esac +# PATH is pinned to the standard system directories for every other tool, with the +# same repo-local extension the pre-commit guard reads: +# git config --local abcd.guardPath /nix/var/nix/profiles/default/bin +# This NARROWS the class of a substituted tool; it does not close it (two pinned +# directories are user-writable on a typical developer machine, and +# `#!/usr/bin/env bash` resolves the interpreter through the inherited PATH). +PATH=/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin +guard_path=$(git config --local --get abcd.guardPath 2>/dev/null || true) +if [ -n "$guard_path" ]; then + case ":$guard_path:" in + *::*) + echo "commit-msg: BLOCKED — abcd.guardPath contains an empty element, which means the current" >&2 + echo " directory to every PATH consumer and would put this repo's working tree on the" >&2 + echo " hook's PATH. Set it to directories only: git config --local abcd.guardPath /path/to/bin" >&2 + exit 1 + ;; + esac + PATH="$PATH:$guard_path" +fi +export PATH +for tool in git mktemp rm; do + if ! command -v "$tool" >/dev/null 2>&1; then + echo "commit-msg: BLOCKED — $tool is not on the hook's pinned PATH ($PATH)." >&2 + echo " the hook pins PATH so a repo-scoped override cannot substitute a fake tool," >&2 + echo " and it refuses rather than run unchecked. Install $tool there, or point the" >&2 + echo " hook at its directory: git config --local abcd.guardPath /path/to/bin" >&2 + exit 1 + fi +done +# --- end environment pin -------------------------------------------------------- + +# Every line of output below goes through this, a builtin loop, so no external tool +# stands between a refusal and the terminal. +say() { + local prefix="$1" line + while IFS= read -r line || [[ -n $line ]]; do + printf '%s%s\n' "$prefix" "$line" >&2 + done <<<"$2" +} msg_file="${1:-}" if [ -z "$msg_file" ] || [ ! -f "$msg_file" ]; then @@ -65,7 +141,7 @@ if [ -z "$src" ]; then echo " git config core.hooksPath .githooks" >&2 exit 1 fi -if ! command -v go >/dev/null 2>&1; then +if [ -z "$go_bin" ]; then echo "commit-msg: BLOCKED — go is not on PATH, so the outbound check cannot run." >&2 echo " the check refuses a live session URL or a tool attribution footer in the" >&2 echo " message, and a check that cannot run must not pass. Install the Go toolchain" >&2 @@ -77,38 +153,48 @@ fi # diff, which git discards: judging it would refuse a commit for a fixture it # stages rather than for anything in the message. Comment lines are KEPT and judged: # whether git strips them depends on the cleanup mode (`-m` keeps a `#` line), and -# over-judging a comment only ever refuses more. -artefact="$(mktemp "${TMPDIR:-/tmp}/abcd-commit-msg.XXXXXX")" -trap 'rm -f "$artefact"' EXIT INT TERM HUP -awk '/^[^[:alnum:][:space:]]+ -+ >8 -+$/ { exit } { print }' "$msg_file" >"$artefact" +# over-judging a comment only ever refuses more. The cut is bash builtins end to +# end, so no external tool decides what is judged. +scissors_re='^[^[:alnum:][:space:]]+ -+ >8 -+$' +message="" +while IFS= read -r line || [[ -n $line ]]; do + [[ $line =~ $scissors_re ]] && break + message+="$line"$'\n' +done <"$msg_file" # An empty message is git's to refuse (or to allow, under --allow-empty-message); -# there is nothing in it that could leak. -if ! grep -q '[^[:space:]]' "$artefact"; then +# there is nothing in it that could leak. The test is a bash builtin: no tool +# outside this shell can answer "nothing to judge". +if ! [[ $message =~ [^[:space:]] ]]; then exit 0 fi +work="$(mktemp -d "${TMPDIR:-/tmp}/abcd-commit-msg.XXXXXX")" +trap 'rm -rf "$work" || true' EXIT INT TERM HUP +artefact="$work/message" +printf '%s' "$message" >"$artefact" + # Git exports its private environment (GIT_DIR, GIT_INDEX_FILE et al.) to a hook; # the Go build must not see it. -buildvcs=false keeps the build from asking git # about this checkout at all while a commit holds its index. rc=0 -out="$(cd "$src" && env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE -u GIT_OBJECT_DIRECTORY \ - -u GIT_ALTERNATE_OBJECT_DIRECTORIES -u GIT_COMMON_DIR -u GIT_PREFIX \ - go run -buildvcs=false ./cmd/abcd lint outbound --label "commit message" --root "$src" "$artefact" 2>&1)" || rc=$? +out="$(cd "$src" && unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_OBJECT_DIRECTORY \ + GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_COMMON_DIR GIT_PREFIX && + "$go_bin" run -buildvcs=false ./cmd/abcd lint outbound --label "commit message" --root "$src" "$artefact" 2>&1)" || rc=$? case "$rc" in 0) - printf '%s\n' "$out" | sed 's/^/commit-msg: /' >&2 + say "commit-msg: " "$out" ;; 1) echo "commit-msg: BLOCKED — the commit message breaks the outbound policy." >&2 - printf '%s\n' "$out" | sed 's/^/ /' >&2 + say " " "$out" echo " delete the line named above and commit again." >&2 exit 1 ;; *) echo "commit-msg: BLOCKED — the outbound check could not judge the message (exit $rc)." >&2 - printf '%s\n' "$out" | sed 's/^/ /' >&2 + say " " "$out" exit 1 ;; esac diff --git a/internal/surface/cli/githook_commitmsg_test.go b/internal/surface/cli/githook_commitmsg_test.go index 02857c97b..e2d01c6ec 100644 --- a/internal/surface/cli/githook_commitmsg_test.go +++ b/internal/surface/cli/githook_commitmsg_test.go @@ -202,3 +202,87 @@ func TestCommitMsgHookFailsClosedWithoutAnAbcdSource(t *testing.T) { t.Errorf("the refusal does not name what is missing\n%s", out) } } + +// withEnv is the case with extra environment for every later git call, which git +// hands on to the hook it runs. +func (c *commitMsgHookCase) withEnv(extra ...string) *commitMsgHookCase { + return &commitMsgHookCase{t: c.t, root: c.root, dir: c.dir, hooks: c.hooks, + env: append(append([]string{}, c.env...), extra...)} +} + +// assertRefusedBeforeACommit is the fail-closed contract: the commit was refused, +// and no commit exists. +func (c *commitMsgHookCase) assertRefusedBeforeACommit(refused bool, out string) { + c.t.Helper() + if !refused { + c.t.Fatalf("a commit message carrying a live session URL was committed\n%s", out) + } + if _, err := c.tryGit("rev-parse", "--verify", "HEAD"); err == nil { + c.t.Fatalf("a commit exists after the refusal\n%s", out) + } + if !strings.Contains(out, "breaks the outbound policy") { + c.t.Errorf("the commit was refused, but not because the message was judged\n%s", out) + } +} + +// The hook is a fresh bash that git starts with the committer's environment, so it +// inherits whatever the session exports: a function shadowing a tool the hook runs, +// or a directory prepended to PATH with a tool of the same name. Either one made the +// hook's "nothing to judge" test answer yes and pass a message it never judged — the +// fail-open the pre-commit guard was hardened against (iss-2609250850380420), in the +// hook that judges the other half of the same commit. +func TestCommitMsgHookResistsInheritedShellState(t *testing.T) { + msg := "fix: the walk\n\nSession: " + sessionURL() + "\n\nAssisted-by: Claude:claude-opus-5\n" + + // Exported functions arrive in the environment. Both spellings are set, so the + // case holds whichever bash `env bash` resolves to. + t.Run("exported functions", func(t *testing.T) { + c := newCommitMsgHookCase(t) + var fns []string + for _, fn := range []string{"grep() { return 1; }", "awk() { return 0; }", "go() { return 0; }"} { + name := fn[:strings.Index(fn, "(")] + body := fn[strings.Index(fn, "("):] + fns = append(fns, "BASH_FUNC_"+name+"%%="+body, "BASH_FUNC_"+name+"()="+body) + } + c.withEnv(fns...).assertRefusedBeforeACommit(c.withEnv(fns...).commitWith("a.txt", msg)) + }) + + // BASH_ENV is read by every non-interactive bash at startup, so it can shadow + // the names the refusal paths depend on as well as the tools. + t.Run("functions through BASH_ENV", func(t *testing.T) { + c := newCommitMsgHookCase(t) + p := filepath.Join(t.TempDir(), "hostile.sh") + body := "grep() { return 1; }\nawk() { return 0; }\ndeclare() { return 0; }\nexit() { return 0; }\n" + if err := os.WriteFile(p, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + h := c.withEnv("BASH_ENV=" + p) + h.assertRefusedBeforeACommit(h.commitWith("a.txt", msg)) + }) + + // A directory prepended to PATH whose awk prints nothing and whose grep finds + // nothing: the message the hook judged was empty, so it passed. + t.Run("tools shimmed on PATH", func(t *testing.T) { + c := newCommitMsgHookCase(t) + shims := t.TempDir() + for name, script := range map[string]string{ + "awk": "#!/bin/sh\nexit 0\n", + "grep": "#!/bin/sh\nexit 1\n", + } { + if err := os.WriteFile(filepath.Join(shims, name), []byte(script), 0o755); err != nil { + t.Fatal(err) + } + } + path := "" + for _, kv := range c.env { + if strings.HasPrefix(kv, "PATH=") { + path = strings.TrimPrefix(kv, "PATH=") + } + } + if path == "" { + path = os.Getenv("PATH") + } + h := c.withEnv("PATH=" + shims + string(os.PathListSeparator) + path) + h.assertRefusedBeforeACommit(h.commitWith("a.txt", msg)) + }) +} From 82ab5b82f6fd984143963b80fb1c0a31c5d56a6b Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:36:55 +0100 Subject: [PATCH 12/19] fix(hooks): cut a commit message only where git itself cuts it 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 --- .githooks/commit-msg | 59 +++++++++++++--- .../surface/cli/githook_commitmsg_test.go | 69 +++++++++++++++---- 2 files changed, 105 insertions(+), 23 deletions(-) diff --git a/.githooks/commit-msg b/.githooks/commit-msg index ec9c64b31..1c3dab0e2 100755 --- a/.githooks/commit-msg +++ b/.githooks/commit-msg @@ -149,18 +149,57 @@ if [ -z "$go_bin" ]; then exit 1 fi -# The text git will record. Everything below a scissors line is the `git commit -v` -# diff, which git discards: judging it would refuse a commit for a fixture it -# stages rather than for anything in the message. Comment lines are KEPT and judged: -# whether git strips them depends on the cleanup mode (`-m` keeps a `#` line), and -# over-judging a comment only ever refuses more. The cut is bash builtins end to -# end, so no external tool decides what is judged. -scissors_re='^[^[:alnum:][:space:]]+ -+ >8 -+$' -message="" +# The text git will record. git discards what follows its scissors line only when it +# truncates the message, which is a verbose commit (`git commit -v`, whose staged +# diff it appends below the line) or the scissors cleanup mode; a message given with +# -F or -m keeps everything, a look-alike line and the text under it included. The +# hook cannot see which mode git runs in, so it cuts only where git's own cut would +# be unmistakable: at git's exact scissors line, prefixed with this clone's comment +# character, and only when a `diff --git` line follows it. Everywhere else it judges +# the whole file — an over-judged comment only ever refuses more. Comment lines are +# KEPT and judged: whether git strips them depends on the cleanup mode (`-m` keeps a +# `#` line). What this cannot tell apart: a -F message that forges git's scissors +# and a diff header below them is cut here and kept by git; CI's check over the +# pull request (scripts/check-attribution.sh reads the recorded message) is the +# backstop for that deliberate shape. The cut is bash builtins end to end, so no +# external tool decides what is judged. +# +# The comment character: core.commentString or core.commentChar, whichever git +# config sets last, `#` when neither is set; under `auto` git picks one of its +# candidates per message, so any of them marks the line. +comment="#" +while IFS= read -r line || [[ -n $line ]]; do + [[ -n $line ]] && comment="${line#* }" +done <<<"$(git config --get-regexp '^core\.comment(char|string)$' 2>/dev/null || true)" +auto_candidates='#;@!$%^&|:' +cut_line=" ------------------------ >8 ------------------------" +is_scissors() { + if [[ $comment == auto ]]; then + [[ ${#1} -gt 1 && $auto_candidates == *"${1:0:1}"* && ${1:1} == "$cut_line" ]] + else + [[ $1 == "$comment$cut_line" ]] + fi +} +lines=() while IFS= read -r line || [[ -n $line ]]; do - [[ $line =~ $scissors_re ]] && break - message+="$line"$'\n' + lines+=("$line") done <"$msg_file" +keep=${#lines[@]} +for ((i = 0; i < ${#lines[@]}; i++)); do + is_scissors "${lines[i]}" || continue + # git cuts at its first scissors line; cut there only if the diff follows it. + for ((j = i + 1; j < ${#lines[@]}; j++)); do + if [[ ${lines[j]} == "diff --git "* ]]; then + keep=$i + break + fi + done + break +done +message="" +for ((i = 0; i < keep; i++)); do + message+="${lines[i]}"$'\n' +done # An empty message is git's to refuse (or to allow, under --allow-empty-message); # there is nothing in it that could leak. The test is a bash builtin: no tool diff --git a/internal/surface/cli/githook_commitmsg_test.go b/internal/surface/cli/githook_commitmsg_test.go index e2d01c6ec..fbd9e445d 100644 --- a/internal/surface/cli/githook_commitmsg_test.go +++ b/internal/surface/cli/githook_commitmsg_test.go @@ -138,21 +138,64 @@ func TestCommitMsgHookPassesACleanMessage(t *testing.T) { } } -// Everything below a scissors line is the `git commit -v` diff, which git discards. -// Judging it would refuse a commit for a fixture it stages, not for its message. +// Everything below git's scissors line in a `git commit -v` message is the staged +// diff, which git discards. Judging it would refuse a commit for a fixture it +// stages, not for its message, so the hook cuts there — under the default comment +// character and under one the clone configures. func TestCommitMsgHookIgnoresTheVerboseDiffBelowTheScissors(t *testing.T) { - c := newCommitMsgHookCase(t) - msg := "fix: the walk\n\nAssisted-by: Claude:claude-opus-5\n" + - "# ------------------------ >8 ------------------------\n" + - "+" + sessionURL() + "\n" - // git truncates at the scissors only for a message it opened an editor on, so - // the commit is "edited" through an editor that changes nothing. - refused, out := c.commitWith("a.txt", msg, "--cleanup=scissors", "-e") - if refused { - t.Fatalf("a session URL in the discarded diff below the scissors refused the commit\n%s", out) + for name, config := range map[string][]string{ + "default comment character": nil, + "configured comment character": {"-c", "core.commentChar=;"}, + } { + t.Run(name, func(t *testing.T) { + c := newCommitMsgHookCase(t) + if err := os.WriteFile(filepath.Join(c.dir, "fixture.txt"), []byte(sessionURL()+"\n"), 0o644); err != nil { + t.Fatal(err) + } + c.git("add", "fixture.txt") + msgFile := filepath.Join(t.TempDir(), "msg") + if err := os.WriteFile(msgFile, []byte("test: stage a fixture\n\nAssisted-by: None\n"), 0o644); err != nil { + t.Fatal(err) + } + // -e opens the editor (GIT_EDITOR=true changes nothing), and -v appends the + // staged diff below the scissors: exactly the file a verbose commit hands the hook. + args := append(append([]string{}, config...), "commit", "-q", "-e", "-v", "-F", msgFile) + if out, err := c.tryGit(args...); err != nil { + t.Fatalf("a session URL in the discarded verbose diff refused the commit\n%s", out) + } + if logged := c.git("log", "-1", "--format=%B"); strings.Contains(logged, testOutboundSessionID) { + t.Fatalf("the fixture's premise is wrong: git kept the text below the scissors\n%s", logged) + } + }) } - if logged := c.git("log", "-1", "--format=%B"); strings.Contains(logged, testOutboundSessionID) { - t.Fatalf("the fixture's premise is wrong: git kept the text below the scissors\n%s", logged) +} + +// A scissors-shaped line is not a licence to stop reading. git cuts only at its own +// scissors (its comment character, its exact cut line) and only when it truncates +// — a verbose commit, whose diff follows the line. A message given with -F keeps +// everything under the default cleanup, so a session URL written below a +// look-alike line is in the commit git records, and the hook must judge it. +func TestCommitMsgHookJudgesTextBelowAScissorsLineGitKeeps(t *testing.T) { + for name, line := range map[string]string{ + "short hash scissors": "# ---- >8 ----", + "semicolon scissors": "; ------------------------ >8 ------------------------", + "git's scissors with no diff": "# ------------------------ >8 ------------------------", + } { + t.Run(name, func(t *testing.T) { + msg := "fix: the walk\n\nAssisted-by: None\n" + line + "\nSession: " + sessionURL() + "\n" + + // The premise, with the hooks skipped: git records the text below the line. + c := newCommitMsgHookCase(t) + if refused, out := c.commitWith("a.txt", msg, "--no-verify"); refused { + t.Fatalf("premise: the commit failed with the hooks skipped\n%s", out) + } + if logged := c.git("log", "-1", "--format=%B"); !strings.Contains(logged, testOutboundSessionID) { + t.Fatalf("the fixture's premise is wrong: git discarded the text below %q\n%s", line, logged) + } + c.git("update-ref", "-d", "HEAD") + + c.assertRefusedBeforeACommit(c.commitWith("a.txt", msg)) + }) } } From 4e0c8d39ea2c77b7d2014a2fc134d6ad21e6652c Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:37:55 +0100 Subject: [PATCH 13/19] fix(hooks): say a tree that does not build cannot judge the message 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 --- .githooks/commit-msg | 40 +++++++++++++------ .../surface/cli/githook_commitmsg_test.go | 37 +++++++++++++++++ 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/.githooks/commit-msg b/.githooks/commit-msg index 1c3dab0e2..1fba0e8c0 100755 --- a/.githooks/commit-msg +++ b/.githooks/commit-msg @@ -16,16 +16,19 @@ # The judgement is `abcd lint outbound`, never a regex here: the session-URL # detector is a pattern plus an opacity classifier that POSIX ERE cannot express # (see internal/surface/cli/lint_outbound.go), so a shell copy would be weaker than -# the policy it claims to enforce. It runs through `go run ./cmd/abcd` from this -# checkout, the rule for every abcd invocation in a source checkout: an installed -# abcd is whatever was last released, which in this repository is stale by -# construction. +# the policy it claims to enforce. The hook builds ./cmd/abcd from this checkout and +# runs that build, the rule for every abcd invocation in a source checkout: an +# installed abcd is whatever was last released, which in this repository is stale +# by construction. # -# FAILS CLOSED. A checkout with no abcd source or no Go toolchain cannot judge the -# message, and a hook that cannot judge must not pass silently: a green commit that -# means "the check was skipped" is indistinguishable from one that means "there was -# nothing to find". Go is a prerequisite for developing this repository, so the -# refusal names the missing piece rather than asking for anything new. +# FAILS CLOSED. A checkout with no abcd source, no Go toolchain or a cmd/abcd that +# does not compile cannot judge the message, and a hook that cannot judge must not +# pass silently: a green commit that means "the check was skipped" is +# indistinguishable from one that means "there was nothing to find". Each refusal +# names the missing piece and never blames the message for it. The build is its +# own step for that reason: `go run` exits 1 on a compile error, the same code the +# check gives a finding. Go is a prerequisite for developing this repository, so +# the refusal asks for nothing new. # # REACH, stated rather than implied. git runs this hook for `git commit` (an # amend and a reword through `git commit` included) and for a `git merge` that @@ -135,8 +138,8 @@ for candidate in "$hook_dir/.." "$(git rev-parse --show-toplevel 2>/dev/null || done if [ -z "$src" ]; then echo "commit-msg: BLOCKED — no abcd source to judge this commit message with." >&2 - echo " the outbound check runs \`go run ./cmd/abcd lint outbound\` from the checkout" >&2 - echo " that holds this hook, and neither the hook's checkout nor this working tree" >&2 + echo " the outbound check builds ./cmd/abcd from the checkout that holds this hook" >&2 + echo " and runs \`abcd lint outbound\`, and neither the hook's checkout nor this working tree" >&2 echo " carries cmd/abcd. Point the clone's hooks at the committed directory:" >&2 echo " git config core.hooksPath .githooks" >&2 exit 1 @@ -219,7 +222,20 @@ printf '%s' "$message" >"$artefact" rc=0 out="$(cd "$src" && unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_OBJECT_DIRECTORY \ GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_COMMON_DIR GIT_PREFIX && - "$go_bin" run -buildvcs=false ./cmd/abcd lint outbound --label "commit message" --root "$src" "$artefact" 2>&1)" || rc=$? + "$go_bin" build -buildvcs=false -o "$work/abcd" ./cmd/abcd 2>&1)" || rc=$? +if [ "$rc" -ne 0 ] || [ ! -x "$work/abcd" ]; then + echo "commit-msg: BLOCKED — could not judge the message (the tree does not build)." >&2 + echo " the outbound check is this checkout's own abcd, and \`go build ./cmd/abcd\` failed" >&2 + echo " in $src:" >&2 + say " " "$out" + echo " nothing in the message was found wrong; fix the build and commit again." >&2 + exit 1 +fi + +rc=0 +out="$(cd "$src" && unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_OBJECT_DIRECTORY \ + GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_COMMON_DIR GIT_PREFIX && + "$work/abcd" lint outbound --label "commit message" --root "$src" "$artefact" 2>&1)" || rc=$? case "$rc" in 0) diff --git a/internal/surface/cli/githook_commitmsg_test.go b/internal/surface/cli/githook_commitmsg_test.go index fbd9e445d..21bc5d21d 100644 --- a/internal/surface/cli/githook_commitmsg_test.go +++ b/internal/surface/cli/githook_commitmsg_test.go @@ -329,3 +329,40 @@ func TestCommitMsgHookResistsInheritedShellState(t *testing.T) { h.assertRefusedBeforeACommit(h.commitWith("a.txt", msg)) }) } + +// A checkout whose abcd does not compile cannot judge any message, and must say so +// rather than blame the message: `go run` exits 1 on a compile error, the same code +// as a policy finding, so the hook reported compiler output as "the commit message +// breaks the outbound policy". It still fails closed. +func TestCommitMsgHookSaysATreeThatDoesNotBuildCannotJudge(t *testing.T) { + c := newCommitMsgHookCase(t) + src := t.TempDir() + hook, err := os.ReadFile(filepath.Join(c.hooks, "commit-msg")) + if err != nil { + t.Fatal(err) + } + for rel, body := range map[string]string{ + "go.mod": "module example.com/broken\n\ngo 1.21\n", + "cmd/abcd/main.go": "package main\n\nfunc main() {\n", + ".githooks/commit-msg": string(hook), + } { + p := filepath.Join(src, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(body), 0o755); err != nil { + t.Fatal(err) + } + } + c.git("config", "core.hooksPath", filepath.Join(src, ".githooks")) + refused, out := c.commitWith("a.txt", "fix: the walk\n\nAssisted-by: None\n") + if !refused { + t.Fatalf("a hook whose abcd does not build passed the commit\n%s", out) + } + if !strings.Contains(out, "the tree does not build") { + t.Errorf("the refusal does not say the checkout failed to build\n%s", out) + } + if strings.Contains(out, "breaks the outbound policy") { + t.Errorf("a build failure was reported as a finding against the message\n%s", out) + } +} From 3cd65914e7f0f0d42b2317f56ba5a321bd84d105 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:39:08 +0100 Subject: [PATCH 14/19] fix(receipt): refuse to mint while an index flag hides an edit 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 --- .abcd/work/DECISIONS.md | 1 + internal/surface/cli/githook_prepush_test.go | 34 +++++++++++++++++- scripts/preflight-receipt.sh | 38 +++++++++++++++++--- 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index b9cf939af..bc7543fe3 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -2532,3 +2532,4 @@ together (the script's header says why there is no escape hatch). - 2026-09-24 — Release v0.10.0 is cut by autonomous run A, and the run's agenda line is: approve the publish step. Under ruling A2 of the product thinker's run A interview (2026-09-23 07:52Z, "approve the publish step": the product thinker authorises the run to approve the release environment itself once every gate is green), the run approves the `release` environment's deployment of v0.10.0 only after the merge queue, the verify job and every other gate on the tagged commit report green, and stops with a handover instead if any does not. The cut: v0.10.0, impact breaking, 43 records since v0.9.0 (nine shipped intents, thirty-four resolved or declined issues, four of them breaking), content commit 64ea8f62, the composer's payload accepted on the first ingest and recomposed once for two docs-currency findings. Both semantic gates ran at tier full: docs-currency-reviewer (Fable 5.1) with three findings, two fixed and one deferred because the load check's intent stays planned pending the product thinker's ruling on the stray definition (iss-2609231947544298); the brief-surface cross-check (40 pinned checkers, Opus 5.5, four at a time under the run's ceiling) with 154 findings, all deferred to their records: four user-facing ones captured as iss-2609240519413467, iss-2609240519418856, iss-2609240519471816 and iss-2609240519427388, one inside an appendix chapter captured as iss-2609240519422232, which records itd-147's ac-6 as not met, and the design-record drift to the systematic brief pass iss-2609091956001547. - 2026-09-24 — v0.10.0 is published. PR #693 merged as 1ac8b3a0, and auto-release run 35963282477 tagged it. The `release` environment was approved under ruling A2 once all 30 checks on the tagged commit had settled (25 success, 5 skipped by design). The release was published at 2026-09-24T06:33:10Z with four binaries, `checksums.txt`, the plugin archive `abcd-plugin-v0.10.0.zip` (its sha256 equals the marketplace pin, 1ab2acd1…) and the rendered site, which was deployed. Verified locally afterwards: the darwin-arm64 binary's checksum and its build attestation, and the binary reports v0.10.0. - 2026-09-25 — A push is gated before it opens its connection, by receipt (iss-2608290810036869, iss-2608210738378295; product thinker's ruling M16 of 2026-09-23, "check before connect"). The committed `.githooks/pre-push` no longer runs `make preflight`: git opens the connection before the hook, and a preflight inside it outlasted the transport's idle timeout, so a push reported success and moved nothing. `make preflight` ends by minting a receipt for HEAD under the checkout's local tier (`scripts/preflight-receipt.sh`), and only when the working tree matched HEAD (nothing staged, unstaged or untracked) both when the run began, read while the Makefile is parsed, and when it ended, with HEAD unmoved, so the gates read the tree CI checks out. The hook refuses a push whose commit is new to the remote and carries no receipt from any worktree of the repository; a commit the remote already holds (a tag on a merged commit) passes. Alternatives not taken: a push wrapper that runs the preflight and then `git push --no-verify` (it normalises `--no-verify`, and a plain `git push` would go ungated); keepalive settings on the transport (the hook would still hold the connection for ten minutes); a preflight on a clean export of HEAD (a second full tree and build per push, where refusing a divergent tree costs nothing). `git push --no-verify` skips the hook exactly as before, and CI stays the authority (lane hooks, autonomous run A). +- 2026-09-25 — Reach of the push-receipt entry above, stated after review (iss-2608210738378295). The receipt's clean-tree test is `git status` read when the preflight begins and when it ends, and a tree `git status` cannot see is not vouched for. So `scripts/preflight-receipt.sh` also refuses to mint while any tracked file is flagged skip-worktree or assume-unchanged (`git ls-files -v` tags it `S` or in lower case), because either flag hides that file's edits from the status read; a sparse checkout sets skip-worktree and so never mints. What the receipt still cannot see, stated in the script's header: files git ignores, which are outside the commit yet can be read by a gate (a `go.work`, which `.gitignore` lists, changes every Go gate's module resolution); HEAD moved and moved back, or the tree changed and restored, between the two reads; and anything a gate reads from outside the checkout. The receipt stays a local convenience gate and CI the authority; closing those would mean running the gates on a clean export of HEAD, the alternative the entry above did not take (lane hooks fix round, autonomous run A). diff --git a/internal/surface/cli/githook_prepush_test.go b/internal/surface/cli/githook_prepush_test.go index 998578d41..5d5c58f9d 100644 --- a/internal/surface/cli/githook_prepush_test.go +++ b/internal/surface/cli/githook_prepush_test.go @@ -221,6 +221,38 @@ func TestPreflightReceiptIsWithheldFromATreeThatDiffersFromTheCommit(t *testing. } } +// git status hides a tracked file flagged skip-worktree or assume-unchanged: its +// local edit is invisible to the clean-tree comparison, so the gates read content +// the commit does not carry while the tree reads as clean. No receipt is minted +// while any such flag is set, edited or not, because the flag is exactly what +// stops the script from knowing. +func TestPreflightReceiptIsWithheldWhileAnIndexFlagHidesAnEdit(t *testing.T) { + for name, flag := range map[string]string{ + "skip-worktree": "--skip-worktree", + "assume-unchanged": "--assume-unchanged", + } { + t.Run(name, func(t *testing.T) { + c := newPrePushCase(t) + c.commit("a.md", "one\n") + c.git("update-index", flag, "a.md") + c.write("a.md", "two\n", 0o644) // the edit git status no longer reports + if st := c.git("status", "--porcelain"); st != "" { + t.Fatalf("the fixture's premise is wrong: git status reports the edit\n%s", st) + } + out := c.preflight(c.dir) + if strings.Contains(out, "receipt minted for") { + t.Fatalf("a preflight over a %s edit minted a receipt\n%s", name, out) + } + if !strings.Contains(out, flag[2:]) { + t.Errorf("the refusal does not name the flag that hides the edit\n%s", out) + } + if pushed, err := c.push("origin", "feature"); err == nil { + t.Fatalf("a commit preflighted over a hidden edit was pushed\n%s", pushed) + } + }) + } +} + func TestPreflightReceiptIsWithheldWhenHeadMovesDuringTheRun(t *testing.T) { c := newPrePushCase(t) c.commit("a.md", "one\n") @@ -313,7 +345,7 @@ func TestMakePreflightMintsTheReceiptAfterItsLastGate(t *testing.T) { } lines := strings.Split(strings.TrimSpace(string(dry)), "\n") last := lines[len(lines)-1] - mint := regexp.MustCompile(`^scripts/preflight-receipt\.sh mint "[0-9a-f]{40} (clean|dirty)"$`) + mint := regexp.MustCompile(`^scripts/preflight-receipt\.sh mint "[0-9a-f]{40} (clean|dirty|hidden)"$`) if !mint.MatchString(last) { t.Fatalf("the preflight recipe's last step is %q; want the receipt minted from the state recorded "+ "before the first gate ran\n%s", last, dry) diff --git a/scripts/preflight-receipt.sh b/scripts/preflight-receipt.sh index 980a3a547..aab6df9b7 100755 --- a/scripts/preflight-receipt.sh +++ b/scripts/preflight-receipt.sh @@ -17,8 +17,17 @@ # can pass every gate while the committed tree fails CI, because CI checks out the # commit. A receipt is minted only when the tree matched HEAD (no staged, unstaged # or untracked change) when the preflight began AND when it ended, with HEAD -# unmoved between: then what the gates read is what the push ships. Files git -# ignores are outside that comparison, as they are outside the commit. +# unmoved between: then what the gates read is what the push ships. A tracked file +# flagged skip-worktree or assume-unchanged is hidden from `git status`, so its +# local edit would read as clean: while any such flag is set (a sparse checkout +# sets skip-worktree on every file it leaves out), no receipt is minted. +# +# LIMITS, stated rather than implied. The comparison is `git status` at two +# instants, so it cannot see: a file git ignores, which is outside the commit yet +# can be read by a gate (a `go.work` beside go.mod, which .gitignore lists, changes +# every Go gate's module resolution); HEAD moved and moved back, or the tree changed +# and restored, between the two instants; and anything a gate reads from outside +# the checkout. Each passes a receipt for a tree the gates did not read exactly. # # The receipt is a file named by the full commit id under the checkout's local # tier, .abcd/.work.local/preflight-receipts/. It is a local convenience gate, not @@ -26,7 +35,8 @@ # authority, and `git push --no-verify` skips this layer exactly as it always did. # # Usage: -# preflight-receipt.sh state print " clean|dirty" for this tree +# preflight-receipt.sh state print " clean|dirty|hidden" for this +# tree (hidden: an index flag hides edits) # preflight-receipt.sh mint "" mint a receipt for HEAD if the tree was # clean at and is clean at the same # HEAD now; otherwise say why none is minted @@ -44,14 +54,24 @@ receipts_rel=".abcd/.work.local/preflight-receipts" keep=50 state() { - local head status + local head status flags head="$(git rev-parse --verify --quiet HEAD 2>/dev/null || true)" [ -n "$head" ] || head="none" status="$(git status --porcelain --untracked-files=normal 2>/dev/null)" || { echo "$head dirty" return 0 } - if [ -z "$status" ]; then + # `git ls-files -v` tags a skip-worktree entry S and an assume-unchanged one in + # lower case: either flag hides an edit from the status read above. + flags="$(git ls-files -v 2>/dev/null)" || { + echo "$head dirty" + return 0 + } + # A here-string, not a pipe: under pipefail an early-exiting `grep -q` can fail + # the writer with SIGPIPE and turn a match into "no match". + if grep -q '^[Shs] ' <<<"$flags"; then + echo "$head hidden" + elif [ -z "$status" ]; then echo "$head clean" else echo "$head dirty" @@ -74,6 +94,14 @@ mint() { echo "preflight: no push receipt minted — there is no commit to vouch for." return 0 fi + if [ "$began_tree" = "hidden" ] || [ "$now_tree" = "hidden" ]; then + echo "preflight: no push receipt minted — a tracked file is flagged skip-worktree or assume-unchanged" + echo " (git ls-files -v tags it S or in lower case), which hides its edits from git status," + echo " so the tree these gates read cannot be shown to be the commit. Clear the flag" + echo " (git update-index --no-skip-worktree / --no-assume-unchanged ) and run" + echo " \`make preflight\` again." + return 0 + fi if [ "$began_tree" != "clean" ] || [ "$now_tree" != "clean" ]; then echo "preflight: no push receipt minted — the working tree differed from HEAD (staged, unstaged" echo " or untracked changes), so these gates did not read the tree a push ships." From bee959934fa9c33df43effecf83757fa879f2805 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:40:29 +0100 Subject: [PATCH 15/19] chore: note in two resolved records' grounds what review showed 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 --- ...-2608210738378295-local-gates-lint-working-tree-not-commit.md | 1 + ...claude-session-url-reached-three-commit-messages-and-two-p.md | 1 + 2 files changed, 2 insertions(+) diff --git a/.abcd/work/issues/resolved/iss-2608210738378295-local-gates-lint-working-tree-not-commit.md b/.abcd/work/issues/resolved/iss-2608210738378295-local-gates-lint-working-tree-not-commit.md index 4b1b6a9b0..c0329c1ab 100644 --- a/.abcd/work/issues/resolved/iss-2608210738378295-local-gates-lint-working-tree-not-commit.md +++ b/.abcd/work/issues/resolved/iss-2608210738378295-local-gates-lint-working-tree-not-commit.md @@ -17,3 +17,4 @@ Local record gates validate the WORKING TREE, not the committed/pushed tree, so ## Grounds - pursued: a commit whose working tree diverged from it during its preflight cannot be pushed through the hook; a divergence that still earns a receipt, such as one git status --porcelain does not report, would show it wrong +- pursued: review met the falsifier above — an edit to a file flagged skip-worktree or assume-unchanged is invisible to git status and earned a receipt; the receipt is now withheld while any such flag is set, and the divergences it still cannot see (an ignored file a gate reads such as go.work, HEAD or the tree moved and restored between its two reads) are stated in scripts/preflight-receipt.sh and .abcd/work/DECISIONS.md rather than closed diff --git a/.abcd/work/issues/resolved/iss-2609061438431625-a-claude-session-url-reached-three-commit-messages-and-two-p.md b/.abcd/work/issues/resolved/iss-2609061438431625-a-claude-session-url-reached-three-commit-messages-and-two-p.md index a2b53ef5a..7116f5007 100644 --- a/.abcd/work/issues/resolved/iss-2609061438431625-a-claude-session-url-reached-three-commit-messages-and-two-p.md +++ b/.abcd/work/issues/resolved/iss-2609061438431625-a-claude-session-url-reached-three-commit-messages-and-two-p.md @@ -22,3 +22,4 @@ A Claude session URL reached three commit messages and two PR bodies of a manage ## Grounds - pursued: a commit or merge whose message carries a live session URL or a tool footer is refused in this repository before any commit object exists; a leaked URL reaching a commit made through git commit or git merge here with the hooks path armed would show it wrong +- pursued: review of the lane met the falsifier above before the resolution merged — an inherited grep or awk function, a PATH-prepended awk, and a scissors look-alike in a `git commit -F` message each let a leaked URL through, and a tree that did not build was reported as a finding against the message; the hook now pins its environment as the pre-commit guard does, cuts only at git's own scissors line with a verbose diff below it, and builds ./cmd/abcd before it judges, so the resolution's `go run` names the earlier mechanism; a -F message forging both the scissors and a diff header, or a substituted go that forges a pass, would still show it wrong, and CI's check over the recorded message is the backstop for both From f78715aeab755d0c2a9aa1a6b652e0b955302d05 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 11:11:14 +0100 Subject: [PATCH 16/19] fix: pin the builtins that steer the commit-msg scissors cut 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 bee95993 (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 --- .githooks/commit-msg | 46 +++++++++++++------ .githooks/pre-commit | 10 +++- .githooks/pre-merge-commit | 10 +++- internal/core/ahoy/defaults/pre-commit | 10 +++- .../surface/cli/githook_commitmsg_test.go | 28 +++++++++++ 5 files changed, 86 insertions(+), 18 deletions(-) diff --git a/.githooks/commit-msg b/.githooks/commit-msg index 1fba0e8c0..308d69aa2 100755 --- a/.githooks/commit-msg +++ b/.githooks/commit-msg @@ -35,6 +35,9 @@ # creates a commit. `git commit --no-verify`, `git rebase`, `git cherry-pick`, # `git am` and a message edited on the forge bypass it by construction; the CI # gate over the pull request's range is the backstop for all of them. +# `set` is dropped before its first use: a shadowed one would leave errexit off for +# the whole hook. The fixed list below drops it again with the rest. +unset -f set 2>/dev/null || true set -euo pipefail # --- environment pin: the FIRST statements, before anything is read or run ------- # The same pin as .githooks/pre-commit, for the same reason: git starts this hook as @@ -45,12 +48,19 @@ set -euo pipefail # carrying a session URL without judging a byte of it. case $- in *x*) set +x ;; esac # Drop any inherited function shadowing a name this hook runs. A function wins over -# PATH lookup, so pinning PATH alone does nothing against one. `exit`, `test` and -# `[` are here because a surviving `exit` function turns every refusal into a -# printed BLOCK that does not block; `go` because it is the one tool resolved -# outside the pinned PATH. The list is expansion-free and runs before `declare` is -# consulted, so it holds even when the sweep below is neutered. -unset -f git grep awk mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ go cd pwd trap 2>/dev/null || true +# PATH lookup — and over a builtin — so pinning PATH alone does nothing against one. +# The list names every command this hook runs that is not its own function or a +# shell keyword: the tools, `go` (the one tool resolved outside the pinned PATH), +# and the builtins that decide a value, a refusal or which lines are judged. +# `exit`, `test` and `[` because a surviving `exit` function turns every refusal +# into a printed BLOCK that does not block; `continue` and `break` because a loop +# that steers on them judges the lines they choose (a no-op `continue` once sent +# every line into the scissors search and cut the message to nothing); `declare` +# because the sweep below reads it. The list is expansion-free and runs before +# `declare` is consulted, so what it names is dropped even when the sweep is +# neutered — and the scissors cut below is steered by no builtin at all. +unset -f git grep awk mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ go cd pwd trap \ + declare continue break return local export set true 2>/dev/null || true # A field separator the caller cannot choose, pinned before the sweep splits on it. IFS=$' \t\n' # Then sweep every function that survives: best-effort on top of the fixed list (a @@ -187,18 +197,24 @@ lines=() while IFS= read -r line || [[ -n $line ]]; do lines+=("$line") done <"$msg_file" -keep=${#lines[@]} -for ((i = 0; i < ${#lines[@]}; i++)); do - is_scissors "${lines[i]}" || continue - # git cuts at its first scissors line; cut there only if the diff follows it. - for ((j = i + 1; j < ${#lines[@]}; j++)); do +# git cuts at its first scissors line; cut there only if the diff follows it. The +# loops steer on arithmetic and `if` alone, never on `continue` or `break`, so +# what is judged does not rest on a builtin an inherited function could shadow. +n=${#lines[@]} +keep=$n +first=-1 +for ((i = 0; i < n && first < 0; i++)); do + if is_scissors "${lines[i]}"; then + first=$i + fi +done +if ((first >= 0)); then + for ((j = first + 1; j < n && keep == n; j++)); do if [[ ${lines[j]} == "diff --git "* ]]; then - keep=$i - break + keep=$first fi done - break -done +fi message="" for ((i = 0; i < keep; i++)); do message+="${lines[i]}"$'\n' diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 1d0252a0f..4dda5bb7b 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -48,6 +48,9 @@ # Companion to the public docs-lint banned-token family: that one bans *public* # tokens in CI; this one bans *private* ones locally, keeping their literal string # out of every published artifact. `abcd banlist` maintains both. +# `set` is dropped before its first use: a shadowed one would leave errexit off for +# the whole hook. The fixed list below drops it again with the rest. +unset -f set 2>/dev/null || true set -euo pipefail # --- environment pin: the FIRST statements, before anything is read or run ------- # Turn OFF xtrace however it was inherited (a caller's `set -x`, SHELLOPTS, BASH_ENV): @@ -66,7 +69,12 @@ case $- in *x*) set +x ;; esac # shadowed `declare`, and a surviving `exit` function turns every refusal into a # printed BLOCK that does not block. This list is expansion-free and runs before # `declare` is ever consulted, so it holds even then. -unset -f git grep mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ 2>/dev/null || true +# `declare`, `continue`, `break`, `return`, `local`, `export`, `set` and `true` are the +# builtins this hook steers on; a shadowed `continue` or `break` changes which lines +# a loop reads or never ends it, so they are pinned with the tools (parity with +# .githooks/commit-msg, where a no-op `continue` once cut the judged message to nothing). +unset -f git grep mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ \ + declare continue break return local export set true 2>/dev/null || true # A field separator the caller cannot choose: an inherited IFS changes how every # unquoted expansion below splits — including the sweep loop's, so it is pinned # before the sweep runs. diff --git a/.githooks/pre-merge-commit b/.githooks/pre-merge-commit index c026833a8..0c402667a 100755 --- a/.githooks/pre-merge-commit +++ b/.githooks/pre-merge-commit @@ -16,6 +16,9 @@ # `git commit --no-verify` and a server-side push bypass both by construction. This # layer covers the commits git asks a hook about, on machines that have opted in — # nothing more, and the reach every abcd surface states says exactly that. +# `set` is dropped before its first use: a shadowed one would leave errexit off for +# the whole hook. The fixed list below drops it again with the rest. +unset -f set 2>/dev/null || true set -euo pipefail # --- environment pin: the FIRST statements, matching the guard this delegates to --- # Without them this half is the soft way in: it would exec the delegate under an @@ -30,7 +33,12 @@ case $- in *x*) set +x ;; esac # `read`, `echo`, `exit`, `test` and `[` are pinned for the delegate's sake, and # every surviving function is swept after the fixed list — see the pre-commit # half for why, including why the fixed list must carry `exit`. -unset -f git grep mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ 2>/dev/null || true +# `declare`, `continue`, `break`, `return`, `local`, `export`, `set` and `true` are the +# builtins this hook steers on; a shadowed `continue` or `break` changes which lines +# a loop reads or never ends it, so they are pinned with the tools (parity with +# .githooks/commit-msg, where a no-op `continue` once cut the judged message to nothing). +unset -f git grep mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ \ + declare continue break return local export set true 2>/dev/null || true IFS=$' \t\n' set -f for _fn in $(declare -F); do diff --git a/internal/core/ahoy/defaults/pre-commit b/internal/core/ahoy/defaults/pre-commit index 401ca19e5..bdb541ea7 100644 --- a/internal/core/ahoy/defaults/pre-commit +++ b/internal/core/ahoy/defaults/pre-commit @@ -55,6 +55,9 @@ # is a REFUSAL naming its line number — never a silent skip, and never an echo of # the line's content. A store that exists but yields NO entries warns as loudly as # an absent one: it checks exactly as much. +# `set` is dropped before its first use: a shadowed one would leave errexit off for +# the whole hook. The fixed list below drops it again with the rest. +unset -f set 2>/dev/null || true set -euo pipefail # --- environment pin: the FIRST statements, before anything is read or run ------- # Turn OFF xtrace however it was inherited (a caller's `set -x`, SHELLOPTS, BASH_ENV): @@ -73,7 +76,12 @@ case $- in *x*) set +x ;; esac # shadowed `declare`, and a surviving `exit` function turns every refusal into a # printed BLOCK that does not block. This list is expansion-free and runs before # `declare` is ever consulted, so it holds even then. -unset -f git grep mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ 2>/dev/null || true +# `declare`, `continue`, `break`, `return`, `local`, `export`, `set` and `true` are the +# builtins this hook steers on; a shadowed `continue` or `break` changes which lines +# a loop reads or never ends it, so they are pinned with the tools (parity with +# .githooks/commit-msg, where a no-op `continue` once cut the judged message to nothing). +unset -f git grep mktemp tr cat mkdir rm chmod printf sed head command read echo exit test [ \ + declare continue break return local export set true 2>/dev/null || true # A field separator the caller cannot choose: an inherited IFS changes how every # unquoted expansion below splits — including the sweep loop's, so it is pinned # before the sweep runs. diff --git a/internal/surface/cli/githook_commitmsg_test.go b/internal/surface/cli/githook_commitmsg_test.go index 21bc5d21d..5eff07b1c 100644 --- a/internal/surface/cli/githook_commitmsg_test.go +++ b/internal/surface/cli/githook_commitmsg_test.go @@ -303,6 +303,34 @@ func TestCommitMsgHookResistsInheritedShellState(t *testing.T) { h.assertRefusedBeforeACommit(h.commitWith("a.txt", msg)) }) + // The scissors cut decides which lines are judged, so a builtin that steers it + // is as load-bearing as a tool. With `declare` shadowed the sweep leaves every + // function standing, and a `continue` that does nothing sent every line into + // the search for a diff below a scissors line: a `diff --git ` line anywhere + // in the message cut it to nothing, and the hook passed the URL unjudged. + cutMsg := "fix: the walk\n\nSession: " + sessionURL() + "\n\ndiff --git a/a.txt b/a.txt\n\nAssisted-by: Claude:claude-opus-5\n" + controlFlow := []string{"declare() { return 0; }", "continue() { :; }", "break() { :; }"} + t.Run("control-flow builtins through BASH_ENV", func(t *testing.T) { + c := newCommitMsgHookCase(t) + p := filepath.Join(t.TempDir(), "hostile.sh") + if err := os.WriteFile(p, []byte(strings.Join(controlFlow, "\n")+"\n"), 0o644); err != nil { + t.Fatal(err) + } + h := c.withEnv("BASH_ENV=" + p) + h.assertRefusedBeforeACommit(h.commitWith("a.txt", cutMsg)) + }) + t.Run("control-flow builtins as exported functions", func(t *testing.T) { + c := newCommitMsgHookCase(t) + var fns []string + for _, fn := range controlFlow { + name := fn[:strings.Index(fn, "(")] + body := fn[strings.Index(fn, "("):] + fns = append(fns, "BASH_FUNC_"+name+"%%="+body, "BASH_FUNC_"+name+"()="+body) + } + h := c.withEnv(fns...) + h.assertRefusedBeforeACommit(h.commitWith("a.txt", cutMsg)) + }) + // A directory prepended to PATH whose awk prints nothing and whose grep finds // nothing: the message the hook judged was empty, so it passed. t.Run("tools shimmed on PATH", func(t *testing.T) { From fdf636df1b2a55bb75074e3f5b89898cfa4865e5 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 11:12:33 +0100 Subject: [PATCH 17/19] fix: refuse a GOFLAGS that makes the commit-msg build other source 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 f78715ae (the overlay commit carried the URL; the toolexec commit was made), green after. Refs: iss-2609061438431625 Assisted-by: Claude:claude-opus-5-5 --- .githooks/commit-msg | 42 +++++++++++++-- .../surface/cli/githook_commitmsg_test.go | 52 +++++++++++++++++++ 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/.githooks/commit-msg b/.githooks/commit-msg index 308d69aa2..858e58c9d 100755 --- a/.githooks/commit-msg +++ b/.githooks/commit-msg @@ -77,11 +77,18 @@ LC_ALL=C export LC_ALL # The Go toolchain lives wherever its installer put it (~/.local/bin, ~/go/bin, # /usr/local/go/bin), so it is resolved on the INHERITED PATH, to an absolute path, -# before the pin below. That is a deliberate boundary, not an oversight: any real -# `go` builds this checkout's own source and judges the message the same way, so an -# accidental substitute cannot pass what the policy refuses; a fake `go` that forges -# a pass is a deliberate act, the class of `--no-verify`, which CI's check over the -# pull request is the backstop for. +# before the pin below. That is a deliberate boundary, not an oversight, and it +# takes the Go environment with it: the toolchain AND the GOFLAGS, GOROOT, +# GOTOOLCHAIN and GOCACHE it runs under (the environment or the go env file) are +# the committer's, and the hook trusts them to build this checkout's own source. +# Two GOFLAGS entries say outright that the build is something else — -overlay +# swaps source files, -toolexec runs every compile step through a program of the +# caller's choosing — so the hook refuses either before it builds. Short of those, +# a real `go` builds this checkout's own source and judges the message the same +# way, so an accidental substitute cannot pass what the policy refuses; a fake `go`, +# a doctored GOROOT or a poisoned build cache that forges a pass is a deliberate +# act, the class of `--no-verify`, which CI's check over the pull request is the +# backstop for. go_bin=$(command -v go 2>/dev/null || true) case "$go_bin" in /*) ;; *) go_bin="" ;; esac # PATH is pinned to the standard system directories for every other tool, with the @@ -161,6 +168,31 @@ if [ -z "$go_bin" ]; then echo " go.mod declares." >&2 exit 1 fi +# GOFLAGS as `go` itself resolves it, from the environment or the go env file. +rc=0 +goflags="$(cd "$src" && "$go_bin" env GOFLAGS 2>&1)" || rc=$? +if [ "$rc" -ne 0 ]; then + echo "commit-msg: BLOCKED — could not read GOFLAGS from \`go env\`, so the build the outbound" >&2 + echo " check runs cannot be vouched for:" >&2 + say " " "$goflags" + echo " nothing in the message was found wrong; fix the Go environment and commit again." >&2 + exit 1 +fi +set -f +for flag in $goflags; do + case "$flag" in + -overlay | -overlay=* | --overlay | --overlay=* | -toolexec | -toolexec=* | --toolexec | --toolexec=*) + set +f + echo "commit-msg: BLOCKED — GOFLAGS carries ${flag%%=*}, so \`go build ./cmd/abcd\` would not build" >&2 + echo " this checkout's own source (-overlay swaps source files, -toolexec runs every" >&2 + echo " compile step through another program), and the outbound check cannot judge" >&2 + echo " the message with it. Nothing in the message was found wrong; commit with" >&2 + echo " the flag removed from GOFLAGS (the environment or \`go env -w\`)." >&2 + exit 1 + ;; + esac +done +set +f # The text git will record. git discards what follows its scissors line only when it # truncates the message, which is a verbose commit (`git commit -v`, whose staged diff --git a/internal/surface/cli/githook_commitmsg_test.go b/internal/surface/cli/githook_commitmsg_test.go index 5eff07b1c..0990a8c9a 100644 --- a/internal/surface/cli/githook_commitmsg_test.go +++ b/internal/surface/cli/githook_commitmsg_test.go @@ -4,6 +4,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "testing" @@ -394,3 +395,54 @@ func TestCommitMsgHookSaysATreeThatDoesNotBuildCannotJudge(t *testing.T) { t.Errorf("a build failure was reported as a finding against the message\n%s", out) } } + +// The hook's judgement is this checkout's own abcd only while the build compiles +// this checkout's own source. GOFLAGS reaches `go build` from the committer's +// environment (or the go env file), and two of its flags break that: -overlay +// swaps source files for others, and -toolexec runs every compile step through a +// program of the caller's choosing. An overlay that replaced cmd/abcd/main.go by +// a no-op built cleanly, judged nothing and passed a session URL. The hook +// refuses either flag before it builds, and says the message was not judged. +func TestCommitMsgHookRefusesAGOFLAGSThatSwapsTheSource(t *testing.T) { + msg := "fix: the walk\n\nSession: " + sessionURL() + "\n\nAssisted-by: Claude:claude-opus-5\n" + assertRefusedUnjudged := func(t *testing.T, c *commitMsgHookCase, flag string, refused bool, out string) { + t.Helper() + if !refused { + t.Fatalf("a commit built with GOFLAGS carrying %s was committed\n%s", flag, out) + } + if _, err := c.tryGit("rev-parse", "--verify", "HEAD"); err == nil { + t.Fatalf("a commit exists after the refusal\n%s", out) + } + if !strings.Contains(out, "GOFLAGS") || !strings.Contains(out, flag) { + t.Errorf("the refusal does not name GOFLAGS and %s\n%s", flag, out) + } + if strings.Contains(out, "breaks the outbound policy") { + t.Errorf("a refused toolchain setting was reported as a finding against the message\n%s", out) + } + } + + t.Run("overlay", func(t *testing.T) { + c := newCommitMsgHookCase(t) + dir := t.TempDir() + noop := filepath.Join(dir, "main.go") + if err := os.WriteFile(noop, []byte("package main\n\nfunc main() {}\n"), 0o644); err != nil { + t.Fatal(err) + } + target := filepath.Join(c.root, "cmd", "abcd", "main.go") + overlay := filepath.Join(dir, "overlay.json") + body := `{"Replace":{` + strconv.Quote(target) + `:` + strconv.Quote(noop) + `}}` + if err := os.WriteFile(overlay, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + h := c.withEnv("GOFLAGS=-mod=mod -overlay=" + overlay) + refused, out := h.commitWith("a.txt", msg) + assertRefusedUnjudged(t, h, "-overlay", refused, out) + }) + + t.Run("toolexec", func(t *testing.T) { + c := newCommitMsgHookCase(t) + h := c.withEnv("GOFLAGS=-mod=mod --toolexec=/usr/bin/env") + refused, out := h.commitWith("a.txt", "fix: the walk\n\nAssisted-by: None\n") + assertRefusedUnjudged(t, h, "-toolexec", refused, out) + }) +} From e4337f569dffa3710880118b413cf7713eb220ef Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 11:57:41 +0100 Subject: [PATCH 18/19] fix: split GOFLAGS the way go does before refusing -overlay 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 --- .githooks/commit-msg | 15 ++++++++-- .../surface/cli/githook_commitmsg_test.go | 29 +++++++++++++++++-- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/.githooks/commit-msg b/.githooks/commit-msg index 858e58c9d..06315a1d7 100755 --- a/.githooks/commit-msg +++ b/.githooks/commit-msg @@ -86,7 +86,8 @@ export LC_ALL # caller's choosing — so the hook refuses either before it builds. Short of those, # a real `go` builds this checkout's own source and judges the message the same # way, so an accidental substitute cannot pass what the policy refuses; a fake `go`, -# a doctored GOROOT or a poisoned build cache that forges a pass is a deliberate +# a -modfile naming another module file, a doctored GOROOT or a poisoned build +# cache that forges a pass is a deliberate # act, the class of `--no-verify`, which CI's check over the pull request is the # backstop for. go_bin=$(command -v go 2>/dev/null || true) @@ -178,11 +179,20 @@ if [ "$rc" -ne 0 ]; then echo " nothing in the message was found wrong; fix the Go environment and commit again." >&2 exit 1 fi +# go splits GOFLAGS with its own tokeniser (space, tab, newline and carriage +# return separate fields; a field may be wrapped in single or double quotes), so +# the words are taken the same way here: quotes become separators, and the split +# is on all four blanks, never on the inherited IFS. +goflags_words=${goflags//[\"\']/ } +goflags_words=${goflags_words//$'\r'/ } set -f -for flag in $goflags; do +old_ifs=$IFS +IFS=$' \t\n' +for flag in $goflags_words; do case "$flag" in -overlay | -overlay=* | --overlay | --overlay=* | -toolexec | -toolexec=* | --toolexec | --toolexec=*) set +f + IFS=$old_ifs echo "commit-msg: BLOCKED — GOFLAGS carries ${flag%%=*}, so \`go build ./cmd/abcd\` would not build" >&2 echo " this checkout's own source (-overlay swaps source files, -toolexec runs every" >&2 echo " compile step through another program), and the outbound check cannot judge" >&2 @@ -192,6 +202,7 @@ for flag in $goflags; do ;; esac done +IFS=$old_ifs set +f # The text git will record. git discards what follows its scissors line only when it diff --git a/internal/surface/cli/githook_commitmsg_test.go b/internal/surface/cli/githook_commitmsg_test.go index 0990a8c9a..16f69858c 100644 --- a/internal/surface/cli/githook_commitmsg_test.go +++ b/internal/surface/cli/githook_commitmsg_test.go @@ -1,6 +1,7 @@ package cli import ( + "fmt" "os" "os/exec" "path/filepath" @@ -421,8 +422,9 @@ func TestCommitMsgHookRefusesAGOFLAGSThatSwapsTheSource(t *testing.T) { } } - t.Run("overlay", func(t *testing.T) { - c := newCommitMsgHookCase(t) + // writeOverlay writes an overlay that replaces cmd/abcd/main.go by a no-op. + writeOverlay := func(t *testing.T, c *commitMsgHookCase) string { + t.Helper() dir := t.TempDir() noop := filepath.Join(dir, "main.go") if err := os.WriteFile(noop, []byte("package main\n\nfunc main() {}\n"), 0o644); err != nil { @@ -434,11 +436,32 @@ func TestCommitMsgHookRefusesAGOFLAGSThatSwapsTheSource(t *testing.T) { if err := os.WriteFile(overlay, []byte(body), 0o644); err != nil { t.Fatal(err) } - h := c.withEnv("GOFLAGS=-mod=mod -overlay=" + overlay) + return overlay + } + + t.Run("overlay", func(t *testing.T) { + c := newCommitMsgHookCase(t) + h := c.withEnv("GOFLAGS=-mod=mod -overlay=" + writeOverlay(t, c)) refused, out := h.commitWith("a.txt", msg) assertRefusedUnjudged(t, h, "-overlay", refused, out) }) + // go splits GOFLAGS with its own tokeniser, which honours quotes and treats a + // carriage return as a separator, so a flag the shell's word split does not + // isolate still reaches the build. + for name, goflags := range map[string]string{ + "single-quoted overlay": "-mod=mod '-overlay=%s'", + "double-quoted overlay": "\"-overlay=%s\"", + "CR-separated overlay": "-mod=mod\r-overlay=%s", + } { + t.Run(name, func(t *testing.T) { + c := newCommitMsgHookCase(t) + h := c.withEnv("GOFLAGS=" + fmt.Sprintf(goflags, writeOverlay(t, c))) + refused, out := h.commitWith("a.txt", msg) + assertRefusedUnjudged(t, h, "-overlay", refused, out) + }) + } + t.Run("toolexec", func(t *testing.T) { c := newCommitMsgHookCase(t) h := c.withEnv("GOFLAGS=-mod=mod --toolexec=/usr/bin/env") From c63cdf99b48a7d81d25550e5545f10b7a316cdd2 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 15:33:22 +0100 Subject: [PATCH 19/19] test: turn Go telemetry off under the HOME gittest.Env hands a test 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 --- internal/gittest/gittest.go | 21 +++++++++++++++++++++ internal/gittest/telemetry_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 internal/gittest/telemetry_test.go diff --git a/internal/gittest/gittest.go b/internal/gittest/gittest.go index 0715e8c41..7b87b0608 100644 --- a/internal/gittest/gittest.go +++ b/internal/gittest/gittest.go @@ -59,6 +59,7 @@ func Env(t *testing.T) []string { } t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config")) t.Setenv("XDG_DATA_HOME", filepath.Join(home, ".local", "share")) + turnGoTelemetryOff(t) // Never block on a credential/terminal prompt in a test. t.Setenv("GIT_TERMINAL_PROMPT", "0") t.Setenv(isolatedSentinel, "1") @@ -76,3 +77,23 @@ func testOwnedHome(home string) bool { tmp := filepath.Clean(os.TempDir()) + string(os.PathSeparator) return strings.HasPrefix(filepath.Clean(home)+string(os.PathSeparator), tmp) } + +// turnGoTelemetryOff writes Go's telemetry mode file under the test HOME's user +// config directory. With the default mode the go command spawns a detached child +// that keeps writing counters there after the command exits, so a test that runs +// go (the commit hooks build abcd) loses its TempDir cleanup to "directory not +// empty". The mode file is the setting `go telemetry off` writes. +func turnGoTelemetryOff(t *testing.T) { + t.Helper() + cfg, err := os.UserConfigDir() + if err != nil { + t.Fatalf("gittest: user config dir: %v", err) + } + dir := filepath.Join(cfg, "go", "telemetry") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatalf("gittest: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, "mode"), []byte("off"), 0o644); err != nil { + t.Fatalf("gittest: %v", err) + } +} diff --git a/internal/gittest/telemetry_test.go b/internal/gittest/telemetry_test.go new file mode 100644 index 000000000..d336ce3df --- /dev/null +++ b/internal/gittest/telemetry_test.go @@ -0,0 +1,27 @@ +package gittest + +import ( + "os/exec" + "strings" + "testing" +) + +// A test that runs the go command under the HOME Env hands it must not start +// Go's telemetry: with the default mode the go command spawns a detached child +// that keeps writing counters under the temp HOME after the test ends, and the +// test's TempDir cleanup then fails with "directory not empty" (a CI flake on +// both platforms in the commit-msg hook tests). +func TestEnvTurnsGoTelemetryOff(t *testing.T) { + if _, err := exec.LookPath("go"); err != nil { + t.Skip("go is not on PATH") + } + cmd := exec.Command("go", "env", "GOTELEMETRY") + cmd.Env = Env(t) + out, err := cmd.Output() + if err != nil { + t.Fatalf("go env GOTELEMETRY: %v", err) + } + if got := strings.TrimSpace(string(out)); got != "off" { + t.Fatalf("go telemetry mode under the test HOME is %q, want off", got) + } +}