diff --git a/.githooks/pre-commit b/.githooks/pre-commit index ea44ac8..c27bd11 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -29,6 +29,37 @@ is_uint() { case "${1:-}" in ''|*[!0-9]*) return 1;; *) return 0;; esac; } echo "== CLAUDE.md section 4 commit gates ==" +# ---------- BRANCH ---------- +# Work is sliced (section 2) and lands on main by rebase-merged PR (section 4). A commit +# authored directly on main is a slip, and one already happened: it landed because no hook +# refused it, after a reviewer flagged the branch discrepancy and it was read past. A rule +# enforced by attention failed where the same rule as a gate has held every time. +# +# POSITIVE MATCH ONLY. A detached HEAD is allowed, deliberately: `git rebase` replays commits +# detached, `git bisect` runs detached, and the reviewer's index checkout is a detached +# worktree. Refusing on "cannot determine the branch" would block the rebase this repository +# merges with. The gate refuses when it can see the name `main`, and is silent otherwise -- +# which is the one place in this file where an unmeasurable condition is not a failure, and it +# is named rather than left as a fallthrough. +# +# This is a slip-guard, NOT an authorization control. `--no-verify` bypasses it like any hook, +# and a clone that never ran `mix setup` has no local hook at all. The remote is what actually +# prevents it: ruleset 22066749 requires a pull request, blocks non-fast-forward, and has no +# bypass actors. +# --short is DEFEATED BY REF AMBIGUITY: shorten_unambiguous_ref returns "heads/main" once a +# tag or a refs/main also exists, and "heads/main" != "main". Measured by reviewer 1 -- two +# commands and the refusal is gone. The full ref is unambiguous by construction. +branch_ref=$(git symbolic-ref --quiet HEAD || true) +branch=${branch_ref#refs/heads/} +if [ "$branch_ref" = "refs/heads/main" ]; then + echo + echo "COMMIT REJECTED -- CLAUDE.md sections 2 and 7: this commit is on 'main'." + echo " Work is sliced. Branch first: git switch -c slice/NN-kebab-name" + echo " main is reached by rebase-merged PR, never by a local commit." + exit 1 +fi +note "branch" "${branch:-detached HEAD (allowed: rebase/bisect/review checkout)}" + # ---------- index isolation ---------- # Gates measure the WORKING TREE. If it differs from the index the numbers do not # describe the commit. Section 4 forbids `git stash`, so we refuse rather than guess. @@ -43,6 +74,168 @@ if [ -n "$dirty" ]; then printf '%s\n' "$dirty" | sed 's/^/ /' fi +# ---------- DELETED AND RENAMED PATHS ---------- +# Every commit that moves or deletes a tracked path greps every tracked document for the old +# path before staging. +# +# WHAT THIS CATCHES, stated exactly, because the first version of this comment claimed more: +# a path that is TRACKED, that this commit DELETES OR RENAMES, and that a TRACKED .md cites by +# EXACT STRING, in the INDEX. Everything outside that is not covered, and the boundary is not +# incidental -- see below. +# +# THE THREE INSTANCES THAT MOTIVATED THIS GATE WOULD NOT HAVE BEEN CAUGHT BY IT. Reviewer 2 +# measured all three and the earlier claim here ("it would have caught all three") was false of +# every one: +# - docs/residuals.md's "test_failures is 0" (C2) is a retired baseline JSON KEY, not a path; +# - HANDOFF.md's stale tools/gate.sh citation (C2b) is a stale LINE NUMBER into a file that +# was never deleted; +# - the contracts commit's internal/incoming/ citations are the sharpest case: internal/ has +# never been tracked, so removing a file there produces NO --diff-filter=DR entry at all, +# and the surviving citation in BACKLOG.md names the parent DIRECTORY, which an exact-string +# match does not match. Run against that commit the gate correctly prints "none deleted or +# renamed in this commit" -- it is silent because there is nothing in its domain. +# +# The gate is kept anyway, on a narrower and honest claim: the tracked-path-rename case is real +# and a mechanical check beats attention for it. But it is NOT the general "a commit made a +# tracked document false" detector the first comment implied, and nothing here should be read as +# covering the three defects that produced the rule. +# +# An example that is true of THIS tree, not of a future one. The line below is machine-readable +# and gated: apps/hacktui_core/test/deleted_path_example_test.exs asserts the path is tracked +# and that a tracked .md cites it. The previous example named assets/images/UI.png "cited by +# HANDOFF.md" -- true only after the contracts commit lands, and `git grep` for it in the tree +# this ships in returns rc=1. A justification's example is a claim, so it gets the same gate as +# any other claim. +# EXAMPLE-PATH: docs/not_production_ready.md +# +# Further limits, all measured: an exact-string match flags a DOCUMENT CITING A LONGER PATH that +# merely starts with the deleted one (victim.md flags a citation of victim.md.bak) -- a false +# positive that fails closed; untracked .md are never scanned; and a path containing a newline +# is refused rather than searched. +# +# A hit passes only if the literal marker below is on the hit line or the line IMMEDIATELY +# above it. Two lines above does not count: one marker at the top of a file must not whitewash +# every citation beneath it. +HISTORICAL_MARKER='' +NL=$'\n' + +# EVERYTHING here is NUL-delimited, and that is not fastidiousness. The first version parsed +# `git diff --name-status` line by line and fed the field to `git grep -F`. `core.quotePath` +# defaults to true, so git C-quotes any path holding a non-ASCII byte, a tab, a quote or a +# newline -- and the quoted literal ("caf\303\251.md") cannot match the real bytes in the +# document. The gate then printed "not cited in any tracked .md" AND its affirmative summary +# line for a path it had never really searched for. A fail-OPEN, printed as a verdict, in the +# gate this slice exists to make fail-closed. Measured by reviewer 1: three of four hostile +# path shapes read as clean while their citations sat in the index verbatim. +# +# `-z` removes the quoting entirely, and `git ls-files -z` + `git show :path` removes the +# second parse -- we no longer read `git grep`'s "path:line:text" output at all, so a path +# containing a colon or a newline, or a file git calls binary, cannot confuse the reader. Line +# numbers are counted here, from the blob, which is also the only way to be sure the number +# indexes the same bytes the window is read from. +# THE ENUMERATION'S EXIT STATUS IS CHECKED, not discarded. `done < <(git ...)` throws git's +# status away: one unreadable object -- a partial clone, a failed fetch, a gc race, disk +# corruption -- makes git exit 128 with EMPTY output, so `gone` is empty and this gate prints +# "none deleted or renamed in this commit" over a staged deletion cited verbatim in a tracked +# .md. Measured at rc=128. Same class as the C-quoting and binary fail-opens before it: an +# affirmative verdict over an input that was never enumerated. A temp file is used instead of +# a process substitution precisely so the status survives to be tested. +: > "$LOGDIR/gone.z" +git diff --cached --name-status --find-renames --diff-filter=DR -z > "$LOGDIR/gone.z"; enum_rc=$? +path_enum_failed=0 +if [ "$enum_rc" -ne 0 ]; then + die "deleted paths" "FAIL -- could not enumerate deleted/renamed paths (git exited $enum_rc); refusing to pass unmeasured" + path_enum_failed=1 +fi + +gone=() +while IFS= read -r -d '' _status; do + IFS= read -r -d '' _oldpath || break + case "$_status" in + R*|C*) IFS= read -r -d '' _newpath || break ;; + esac + gone+=("$_oldpath") +done < "$LOGDIR/gone.z" + +if [ "$path_enum_failed" -eq 1 ]; then + : # already reported; no affirmative may follow an unmeasured enumeration +elif [ "${#gone[@]}" -eq 0 ]; then + note "deleted paths" "none deleted or renamed in this commit" +else + path_fail=0 + for old in "${gone[@]}"; do + [ -n "$old" ] || continue + # A newline inside the path makes `grep -F` read it as an ALTERNATION of substrings, and a + # path ending in a newline yields an empty alternative that matches every line of every + # .md. Neither answer is a search for that path, so it is refused rather than approximated. + case "$old" in + *"$NL"*) + die "deleted paths" "FAIL -- '$old' contains a newline; refusing to search for it -- resolve this citation by hand" + path_fail=1; continue ;; + esac + cited=0 + read_failed=0 + # The inner enumeration gets the same treatment. Reviewer 1 could not reach this one with + # stock git -- only with a PATH shim -- but "I could not construct it" is not "it cannot + # happen", and an unchecked status here would falsify the sentence below that says the + # affirmative is printed only when every .md was actually read. + : > "$LOGDIR/mdfiles.z" + git ls-files -z -- ':(icase)*.md' > "$LOGDIR/mdfiles.z"; ls_rc=$? + if [ "$ls_rc" -ne 0 ]; then + die "deleted paths" "FAIL -- could not enumerate tracked .md files (git exited $ls_rc); refusing to pass unmeasured" + path_fail=1; read_failed=1 + fi + while IFS= read -r -d '' mdfile; do + # $(...) on file bytes: safe HERE and unsafe in tools/gate.sh, for a reason worth stating + # rather than leaving as an inconsistency. Command substitution strips trailing newlines; + # that changes a HASH but not whether a substring occurs, and the line numbers below are + # counted from the same stripped bytes the window is read from, so the number and the + # window cannot disagree. gate.sh hashes, so it may not do this. + blob=$(git show ":$mdfile" 2>/dev/null) || { + die "deleted paths" "FAIL -- cannot read $mdfile from the index; refusing to pass unmeasured" + path_fail=1; read_failed=1; continue; } + # grep -n over the blob: exit 0 = matched, 1 = no match, >1 = error. Only 1 is "clean". + # -a is LOAD-BEARING. GNU grep calls input binary on an encoding error, then exits 0, + # writes "binary file matches" to STDERR and emits NOTHING on stdout -- so `matches` came + # back empty, the loop below never ran, and the gate printed "not cited in any tracked + # .md" plus its affirmative summary for a citation sitting in the blob byte-for-byte. + # The same fail-open as the C-quoting defect, reached through blob CONTENT instead of + # path encoding, and measured on a .md holding one invalid UTF-8 byte. + matches=$(printf '%s\n' "$blob" | grep -anF -- "$old"); rc=$? + if [ "$rc" -gt 1 ]; then + die "deleted paths" "FAIL -- grep failed reading $mdfile (rc=$rc); refusing to pass unmeasured" + path_fail=1; read_failed=1; continue + fi + [ "$rc" -eq 0 ] || continue + while IFS= read -r m; do + hl=${m%%:*} + case "$hl" in ''|*[!0-9]*) continue ;; esac + cited=1 + # The hit line and the one immediately above it. Two above must NOT count, or one + # marker at the top of a file would whitewash every citation beneath it. + prev=$((hl - 1)); [ "$prev" -lt 1 ] && prev=1 + window=$(printf '%s\n' "$blob" | sed -n "${prev},${hl}p") + if printf '%s' "$window" | grep -qF -- "$HISTORICAL_MARKER"; then + note "deleted paths" "$mdfile:$hl cites $old -- marked historical, allowed" + else + die "deleted paths" "FAIL -- $mdfile:$hl cites '$old', which this commit deletes or renames" + echo " mark the line (or the one directly above) with $HISTORICAL_MARKER, or fix the citation" + path_fail=1 + fi + done <"). This check is not what closes that window. + # + # What it does catch, each measured: + # 1. a signoff with the right hash and NO Reviewed-tree line -- i.e. one not written by + # tools/signoff.sh, which is what enforces the two-reviewer index-checkout protocol; + # 2. a signoff whose hash was copied in by hand while the tree line says something else; + # 3. any future case where the diff scope and the tree diverge -- the exclusion of + # internal/** above is exactly such a scope, and would become one if internal/ were + # ever tracked. + # It is defence in depth over a hash that is written by the party it certifies, not the + # primary guard. Signoffs written before this gate existed carry no Reviewed-tree line and + # fail closed here; they are historical records, and no commit is expected to match one. + reviewed_tree=$(sed -n 's/^Reviewed-tree:[[:space:]]*\([0-9a-f]\{40\}\).*/\1/p' "$matched" | head -1) + current_tree=$(git write-tree 2>/dev/null || true) + if [ -z "$reviewed_tree" ]; then + die "review signoff" "FAIL -- $matched carries no Reviewed-tree line; write it with tools/signoff.sh" + elif ! printf '%s' "$current_tree" | grep -qE '^[0-9a-f]{40}$'; then + die "review signoff" "FAIL -- git write-tree gave no tree hash; refusing to pass unmeasured" + elif [ "$reviewed_tree" != "$current_tree" ]; then + die "review signoff" "FAIL -- the index has moved since it was reviewed" + printf ' reviewed: %s\n index is: %s\n' "$reviewed_tree" "$current_tree" + else + note "review tree" "index still equals the reviewed tree (${reviewed_tree:0:12}...)" + fi fi fi diff --git a/BACKLOG.md b/BACKLOG.md index 7c13c34..5a2dad3 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -255,3 +255,32 @@ control's limits: the funnel carries others that are **visible in the implementa described in the moduledoc**, so reading the moduledoc alone returns this item and gives a false sense of completeness. Anyone assessing what leaves the MCP boundary should read the bodies of `egress.ex` and `privacy_mask.ex`, not this entry and not the doc comments. + +## 11. `is_uint` is defined twice — advisory print only + +`.githooks/pre-commit` carries a **byte-identical copy** of the `is_uint` helper defined in +`tools/gate.sh`. Found by a reviewer in slice 16b while checking that slice's own +one-invariant-one-implementation criterion. + +**It governs an advisory print only.** `is_uint` has exactly one call site in the hook, inside +the `ADVISORY` block, and neither branch of it sets `fail` — so the commit verdict is +unreachable from the duplicated function. It decides whether a dependency-audit count is +printable, nothing more. **No baseline or ratchet logic is duplicated** — that is the point of +recording it here rather than filing it as a defect. + +*(An earlier draft of this sentence said the hook "delegates every gate verdict to +`tools/gate.sh`". That was false of the tree and was falsified by the very commit that wrote +it: the hook renders several verdicts of its own — the branch refusal, the deleted-path check +and the `Reviewed-tree` check — and `CLAUDE.md` §4 lists all three as gates. The narrow claim +above is the one that was measured.)* + +It is still the exact hazard the surrounding code names: **two byte-identical copies pass a +grep count.** Today they agree; nothing makes them agree tomorrow. A count is not proof of one +implementation — the only proof is a mutation, and a mutation needs one thing to mutate. + +**Assigned to slice 17**, which removes the copy and imports the single definition; the hook +sourcing the function from `tools/gate.sh` is the obvious shape, and 17's PLAN decides it. + +Deliberately cited by **construct, not line number**: both files change under active work, and +a line citation in a file under change is the stale-citation class this repository has already +committed once inside the document recording the fix for it. diff --git a/apps/hacktui_core/test/deleted_path_example_test.exs b/apps/hacktui_core/test/deleted_path_example_test.exs new file mode 100644 index 0000000..2b5657c --- /dev/null +++ b/apps/hacktui_core/test/deleted_path_example_test.exs @@ -0,0 +1,79 @@ +defmodule HacktuiCore.DeletedPathExampleTest do + use ExUnit.Case, async: true + + # The deleted-path gate's justification comment carries one concrete example. An example in a + # justification is a CLAIM, and this repository's standing rule is that a claim in a tracked + # file is true of the tree it ships in — so the example gets the same gate as any other claim. + # + # This exists because it was already wrong once. The comment named `assets/images/UI.png` + # "cited by HANDOFF.md": true only after the contracts commit lands, while `git grep` for it in + # the tree that shipped the comment returned rc=1. A reviewer caught it by running the grep. + # A reviewer catching it is the thing this test replaces. + @root Path.expand("../../..", __DIR__) + @hook Path.join(@root, ".githooks/pre-commit") + + defp example_path do + src = File.read!(@hook) + + case Regex.run(~r/^#\s*EXAMPLE-PATH:\s*(\S+)\s*$/m, src, capture: :all_but_first) do + [path] -> + path + + _ -> + flunk(""" + .githooks/pre-commit carries no `# EXAMPLE-PATH: ` line. + + The deleted-path gate's comment must name its example on a machine-readable line, so + that the example can be checked rather than trusted. If the example was removed, remove + this test in the same commit and say so; do not leave an unchecked example behind. + """) + end + end + + defp tracked_md do + {out, 0} = System.cmd("git", ["ls-files", "-z", "--", "*.md"], cd: @root) + out |> String.split(<<0>>, trim: true) + end + + test "the gate's example path is tracked" do + path = example_path() + + {_, status} = + System.cmd("git", ["ls-files", "--error-unmatch", "--", path], + cd: @root, + stderr_to_stdout: true + ) + + assert status == 0, + "the deleted-path gate's example is `#{path}`, which is not tracked. " <> + "The gate only ever sees tracked paths, so an untracked example illustrates " <> + "nothing it can do." + end + + test "a tracked .md cites the gate's example path, at a line this test can name" do + path = example_path() + + citations = + for file <- tracked_md(), + {line, n} <- Enum.with_index(String.split(File.read!(Path.join(@root, file)), "\n"), 1), + String.contains?(line, path), + do: "#{file}:#{n}" + + refute citations == [], + """ + the deleted-path gate's comment names `#{path}` as its example, but no tracked .md + cites that path in this tree. + + That is the exact defect this test exists to catch: the comment's previous example + named a path whose only citation lived in an unlanded commit, so the justification + was true of a future tree and false of the shipping one. + + Either point EXAMPLE-PATH at a path a tracked .md actually cites, or drop the example. + """ + + # Named, not just counted: the failure message above is only useful if a passing run can say + # where the citation is. A test that proves existence without locating it makes the next + # person re-derive what this one already knew. + assert is_binary(hd(citations)) + end +end diff --git a/apps/hacktui_core/test/diff_recipe_test.exs b/apps/hacktui_core/test/diff_recipe_test.exs new file mode 100644 index 0000000..b6e97c7 --- /dev/null +++ b/apps/hacktui_core/test/diff_recipe_test.exs @@ -0,0 +1,143 @@ +defmodule HacktuiCore.DiffRecipeTest do + use ExUnit.Case, async: true + + # The reviewable-diff recipe decides what a commit's attestation covers. It has ONE + # definition, in tools/gate.sh (`DIFF_RECIPE` and `DIFF_SCOPE`), read by `derive_diff_hash` + # for the attestation gate and by `staged_diff_hash` for .githooks/pre-commit and + # tools/signoff.sh. + # + # .githooks/commit-msg still carries a second copy. It runs in a context where sourcing + # tools/gate.sh is a behaviour change, so removing that copy is slice 17's work. Until then + # the two are held together HERE rather than on faith: if they drift, the trailer a commit + # writes stops matching the hash the gate derives, and `Gate - attestation` goes red on main + # for a reason nobody would connect to an edit in a hook. + # + # One copy is the goal. Two copies with an equality gate is the acceptable interim. Two + # copies on faith is the defect class this repository keeps re-finding -- and a grep COUNT + # is not proof, because two byte-identical copies pass a count. This test compares the token + # sequences, so a change to either side fails it. + @root Path.expand("../../..", __DIR__) + @gate Path.join(@root, "tools/gate.sh") + @commit_msg Path.join(@root, ".githooks/commit-msg") + @pre_commit Path.join(@root, ".githooks/pre-commit") + + defp tokens(s), do: s |> String.split(~r/\s+/, trim: true) + + # `DIFF_RECIPE=(-c diff.noprefix=false ...)` -> the tokens between the parentheses. + # `^` alone was blind to an indented duplicate -- and bash executes an indented assignment + # exactly like a column-0 one, so ` DIFF_RECIPE=(-c diff.context=9)` inserted before + # DIFF_SCOPE changed the hash the gate produces while this test still reported 3 tests, 0 + # failures. A canary that a real duplicate walks past is not a canary. `^[ \t]*` counts them. + defp array(src, name) do + case Regex.run(~r/^[ \t]*#{name}=\((.*)\)\s*$/m, src, capture: :all_but_first) do + [body] -> tokens(body) + _ -> flunk("#{name}=(...) not found in tools/gate.sh, or spans more than one line") + end + end + + # Comment lines are stripped, then assignments are counted. The forms this matcher is known to + # count and known not to count are listed in the test below; it is a matcher, not a bash parser. + defp non_comment(src) do + src + |> String.split("\n") + |> Enum.reject(&Regex.match?(~r/^\s*#/, &1)) + |> Enum.join("\n") + end + + # `NAME=(`, `NAME+=(` and `NAME[i]=` are all assignments bash executes. Counting only `=(` + # was the THIRD form-blindness in this one construct: `^` missed an indented duplicate, + # `^[ \t]*` missed `;`/`then`/`eval`, and this missed `+=` and `[i]=` — each time while the + # duplicate measurably moved the hash the gate produces and the canary reported one + # definition. The forms are now covered by a test rather than by the next reviewer. + # An ASSIGNMENT, not a use: the bracket form must close and be followed by `=`, and `$`/`{` are + # excluded from the lookbehind, so `"${NAME[@]}"` does not count. Measured: without that, this + # test reported 3 definitions of a variable defined once, counting the two array expansions in + # derive_diff_hash and staged_diff_hash. + defp count_defs(src, name) do + length(Regex.scan(~r/(? + "Two byte-identical copies pass a grep count, so the only honest test is a " <> + "mutation, and a mutation needs exactly one thing to mutate." + end + end + + # Nine forms, measured: six that bash executes and the counter counts, three inert ones it does + # not. Regression locks for forms this file has been bitten by. Asserted against synthetic + # source rather than by mutating the real file, so they run in the ordinary suite. + test "nine known assignment forms: six counted, three not" do + base = "DIFF_RECIPE=(-c diff.context=3)\n" + + live = [ + {"plain second definition", "DIFF_RECIPE=(-c diff.context=9)"}, + {"indented", " DIFF_RECIPE=(-c diff.context=9)"}, + {"after a semicolon", "true; DIFF_RECIPE=(-c diff.context=9)"}, + {"after then", "if true; then DIFF_RECIPE=(-c diff.context=9); fi"}, + {"append with +=", "DIFF_RECIPE+=(-c diff.context=9)"}, + {"element assignment", "DIFF_RECIPE[1]=--patience"} + ] + + for {label, form} <- live do + assert count_defs(base <> form <> "\n", "DIFF_RECIPE") == 2, + "the counter does not see a duplicate #{label} (#{inspect(form)}), " <> + "which bash executes." + end + + # Three inert forms. A counter that matched everything would fail these. + assert count_defs(base <> "# DIFF_RECIPE=(-c diff.context=9)\n", "DIFF_RECIPE") == 1, + "a duplicate quoted inside a comment is inert and must not count" + + assert count_defs(base <> "MY_DIFF_RECIPE=(-c diff.context=9)\n", "DIFF_RECIPE") == 1, + "a longer variable name that merely ends in DIFF_RECIPE must not count" + + assert count_defs(base <> ~S| git "${DIFF_RECIPE[@]}" diff --cached| <> "\n", "DIFF_RECIPE") == + 1, + "an array EXPANSION is a use, not a definition, and must not count" + end + + test ".githooks/pre-commit holds no copy of the recipe" do + refute File.read!(@pre_commit) =~ "diff.noprefix", + ".githooks/pre-commit carries its own copy of the diff recipe again. It should call " <> + "`./tools/gate.sh staged-diff-hash` instead." + end + + test ".githooks/commit-msg's recipe is token-identical to the one in tools/gate.sh" do + gate = File.read!(@gate) + recipe = array(gate, "DIFF_RECIPE") + scope = array(gate, "DIFF_SCOPE") + + # The commit-msg invocation runs from `git ` to the pipe into sha256sum, across + # backslash-continued lines. Join continuations, then compare token sequences -- not a + # substring match, which is what fails on line-wrapped text (slice 16b, three times). + src = File.read!(@commit_msg) + + invocation = + case Regex.run(~r/(git\s+-c\s+diff\.noprefix.*?)\|\s*sha256sum/s, src, + capture: :all_but_first + ) do + [inv] -> inv |> String.replace("\\\n", " ") |> tokens() + _ -> flunk(".githooks/commit-msg: could not find the `git ... | sha256sum` invocation") + end + + expected = ["git"] ++ recipe ++ ["diff", "--cached"] ++ scope + + assert invocation == expected, """ + .githooks/commit-msg and tools/gate.sh disagree about the reviewable-diff recipe. + + commit-msg: #{Enum.join(invocation, " ")} + gate.sh: #{Enum.join(expected, " ")} + + These two must produce byte-identical diffs: commit-msg writes the Reviewed-diff trailer + and tools/gate.sh derives the value CI compares it against. A drift here turns every new + commit red on `Gate - attestation`. Change both, or finish removing the copy (slice 17). + """ + end +end diff --git a/tools/gate.sh b/tools/gate.sh index 2334382..42e2ddc 100755 --- a/tools/gate.sh +++ b/tools/gate.sh @@ -263,9 +263,14 @@ baseline_gate() { fi # RULE 1 -- immutable and append-only. Every entry present at the previous ref must still - # be present now. Equality is jq -S canonical over the ENTIRE entry object, every field, - # not a selected subset: a changed `reason` or `slice` is caught as readily as a changed - # `retired_at`, and key order or whitespace cannot disguise an edit. + # be present now. Equality is jq -S canonical NUMERIC equality over the ENTIRE entry object, + # every field, not a selected subset: a changed `reason` or `slice` is caught as readily as a + # changed `retired_at`, and key order or whitespace cannot disguise an edit. + # + # "numeric equality" is the precise word and the earlier "equality over every field" was + # marginally stronger than the mechanism: jq treats 76 and 76.0 as ONE number, so editing an + # existing entry's retired_at from 76 to 76.0 is accepted as unchanged (rc=0), while 76 -> 77 + # is caught (rc=1). Semantically a no-op, and stated rather than left for a reader to discover. if ! jq -e -n --argjson p "$prev_ret" --argjson q "$now_ret" \ '(($p - $q) | length) == 0' >/dev/null 2>&1; then note baseline "FAIL -- a _retired entry present at $ref was removed or altered; the record is append-only and immutable" @@ -304,6 +309,12 @@ baseline_gate() { # A previous version ran `tostring` over any JSON value: 76.0 came through as "76.0", # failed is_uint, and was reported as "carries no numeric retired_at" -- which was false, # since it IS a number. 7.6e1 meanwhile became "76" and passed. Same value, two verdicts. + # + # Not eliminated at the extremes, and said plainly rather than implied: this fails closed on + # inputs no real count can produce; the message is wrong of the input. For 1e20 the reader + # emits "1e+20" rather than "NaN", so the gate says "not a whole number" of a value that IS + # a non-negative integer; -1 produces the same wording. Both are unreachable with real + # baseline counts, and both refuse rather than pass, which is the property that matters. if [ "$rat" = "-" ]; then note baseline "FAIL -- new _retired entry for $rk carries no retired_at" return 1 @@ -456,10 +467,60 @@ baseline_gate() { # against a tracked allowed-signers file. This slice removes the author's ability to hand # the gate its own answer; it does not yet make the attester a different party. # --------------------------------------------------------------------------- +# ONE definition of the reviewable-diff recipe. Every consumer reads these two arrays: +# attestation (derive_diff_hash), the pre-commit signoff check and tools/signoff.sh +# (staged_diff_hash, via the `staged-diff-hash` subcommand). This is the class-(a) rule +# applied to itself -- two byte-identical copies pass a grep count, so the only honest test +# is a mutation, and a mutation needs exactly one thing to mutate. +# +# .githooks/commit-msg still carries its own copy: it runs in a context where sourcing this +# script is a behaviour change, and removing that copy is slice 17's work. It does not sit +# on faith in the meantime -- apps/hacktui_core/test/diff_recipe_test.exs asserts the two +# token sequences are identical, so they cannot drift silently inside that window. +DIFF_RECIPE=(-c diff.noprefix=false -c diff.context=3 -c diff.algorithm=myers -c core.abbrev=40) +DIFF_SCOPE=(--binary --no-ext-diff --no-textconv -- . ':(exclude)internal/**') + derive_diff_hash() { - git -c diff.noprefix=false -c diff.context=3 -c diff.algorithm=myers -c core.abbrev=40 \ - diff "$1" "$2" --binary --no-ext-diff --no-textconv -- . ':(exclude)internal/**' \ - | sha256sum | cut -d' ' -f1 + git "${DIFF_RECIPE[@]}" diff "$1" "$2" "${DIFF_SCOPE[@]}" | sha256sum | cut -d' ' -f1 +} + +# The same recipe against the index. `git diff --cached` takes no ref pair, which is why it +# cannot simply call derive_diff_hash -- but it reads the same two arrays, so a change to the +# recipe moves both or neither. +# +# THREE things this has to get right, all of them found by review rather than by design: +# +# 1. It must FAIL when it cannot measure. The first version piped `git diff` straight into +# sha256sum, so when git errored -- outside a repository, say -- sha256sum hashed empty +# input and produced e3b0c442..., a perfectly well-formed 64-hex string, with rc=0. A +# caller that cannot measure must not receive something that looks like a measurement; +# that is the defect class this whole file was written to remove, committed inside the +# commit that removes it. PIPESTATUS carries git's status out of the pipeline. +# 2. It must not depend on the caller's directory. DIFF_SCOPE ends in `-- .`, which is +# relative, so the same index answered differently from the root and from a subdirectory +# -- returning the empty-diff hash, with rc=0, while a change was staged. It resolves the +# toplevel itself rather than trusting the caller to have cd'd. +# 3. It must not use $(...) on the diff BYTES. Command substitution strips trailing newlines, +# so the hash would silently differ from every other consumer's. Measured: pipeline +# 9fed0f8d..., $(...) 0cad12fe..., and re-appending the one stripped newline restores +# 9fed0f8d... exactly. Only the finished hex string passes through a substitution. +# (An earlier version of this comment also blamed NULs "that --binary hunks contain". That +# is wrong and was corrected by review: --binary hunks are base85 ASCII -- `tr -dc '\000'` +# over such a diff counts zero. Bash does drop NULs from a substitution, but the diff of a +# binary file is not where they come from. The trailing newline alone is the reason.) +staged_diff_hash() { + local top h rc + top=$(git rev-parse --show-toplevel 2>/dev/null) || return 1 + [ -n "$top" ] || return 1 + h=$( + cd "$top" || exit 1 + git "${DIFF_RECIPE[@]}" diff --cached "${DIFF_SCOPE[@]}" | sha256sum | cut -d' ' -f1 + exit "${PIPESTATUS[0]}" + ) + rc=$? + [ "$rc" -eq 0 ] || return 1 + printf '%s' "$h" | grep -qE '^[0-9a-f]{64}$' || return 1 + printf '%s' "$h" } attestation_gate() { @@ -504,7 +565,7 @@ attestation_gate() { return 0 } -case "${1:?usage: tools/gate.sh }" in +case "${1:?usage: tools/gate.sh }" in format) mix format --check-formatted >/dev/null 2>&1 \ @@ -563,5 +624,23 @@ case "${1:?usage: tools/gate.sh &2; exit 2; } + h=$(staged_diff_hash) || { + echo "staged-diff-hash: could not measure the index; refusing to print a hash" >&2; exit 2; } + [ "$h" = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" ] && { + echo "staged-diff-hash: the staged reviewable diff is empty; nothing to hash" >&2; exit 2; } + printf '%s\n' "$h" + ;; + *) echo "unknown gate: $1" >&2; exit 2 ;; esac diff --git a/tools/signoff.sh b/tools/signoff.sh new file mode 100755 index 0000000..05839c8 --- /dev/null +++ b/tools/signoff.sh @@ -0,0 +1,171 @@ +#!/usr/bin/env bash +# +# Writes internal/slices//REVIEW.signoff, and refuses unless the index is still the +# tree the reviewers actually read. +# +# Why this exists: the attestation covers INDEX bytes while the drift test hashes WORKTREE +# bytes, so a signoff computed after a mid-review edit attests a diff that no longer exists. +# That happened -- three of six staged files changed on disk during slice 16c's round 2, and +# the reviewer reported "the diff I reviewed and hashed is not the diff on disk". The edits +# were legitimate; doing the right work at the wrong moment silently invalidated a review in +# flight. The protocol said "hold edits during review" and had nothing behind it but the +# author's attention, which is the third rule this session to fail that way. +# +# So the binding is mechanical: reviewers read a checkout of the INDEX, each writes the tree +# hash it read to logs/round.r.tree, and this script refuses to write unless +# `git write-tree` still equals both of them. +# +# EXACTLY TWO tree files are required for the round. "All equal" over a single file is +# vacuous -- it compares a value to itself. +# +# The two-per-round count is THIS SLICE'S rule, recorded in CLAUDE.md section 6. It is not in +# section 8, which this comment used to cite: section 8 says to spawn independent reviewer +# subagents, plural, and names no number. A tracked file that hard-fails a round on the +# authority of a section that does not carry the rule is the founding defect of +# internal/REVIEWER-PROTOCOL.md -- a claim about a governing file's contents that the file does +# not contain -- so the citation is corrected rather than left to be inherited. +# +# NOT in scope: proving a review happened, or who performed it. This records WHICH tree was +# read and WHICH diff is being certified. The hash is still written by the party it +# certifies. See CLAUDE.md section 0. +# +# A single bounded record-lane read is deliberately NOT expressible here. It is not a round, +# and a slice closing on one writes its signoff by hand with that stated in the header. +set -uo pipefail +cd "$(git rev-parse --show-toplevel)" || exit 1 + +die() { echo "SIGNOFF REFUSED -- $*" >&2; exit 1; } + +slice_dir="${1:-}" +[ -n "$slice_dir" ] || die "usage: tools/signoff.sh [--replace]" +slice_dir="${slice_dir%/}" +[ -d "$slice_dir" ] || die "$slice_dir/ does not exist" +replace="${2:-}" + +logs="$slice_dir/logs" +[ -d "$logs" ] || die "$logs/ does not exist; reviewers write their tree hash there" + +# ---------- find the highest round that has tree files ---------- +# Fail closed on every branch: a round that cannot be identified is not round 0. +round="" +for f in "$logs"/round*.r*.tree; do + [ -f "$f" ] || continue + n=${f##*/round}; n=${n%%.r*} + # Three shapes, all found by review, all of which defeated the naive check: + # - non-numeric -> `[ "$n" -gt ... ]` errors and the `if` silently takes the false + # branch, so the file is SKIPPED rather than refused; + # - 99999999999999999999 -> same, because it is numeric but outside intmax; + # - round01 vs round1 -> two globs for ONE logical round, so three reviewer files split + # into 2 + 1 and the "exactly two" invariant passed over a subset. + # A round is 1-4 digits with no leading zero. Anything else is refused, not skipped. + case "$n" in + 0*|''|*[!0-9]*) die "malformed round number in $f (expected round.r.tree, N a 1-4 digit number with no leading zero)" ;; + esac + [ "${#n}" -le 4 ] || die "implausible round number in $f: $n" + if [ -z "$round" ] || [ "$n" -gt "$round" ]; then round="$n"; fi +done +[ -n "$round" ] || die "no logs/round.r.tree files in $logs/ -- nothing to check is not a pass" + +# ---------- exactly two, one per reviewer ---------- +# The suffix must be r and the two must DIFFER. Unvalidated, "exactly two files" was +# not "two reviewers": round1.r1.tree + round1.r1b.tree, or + round1.rZZZ.tree, both satisfied +# the count from one party -- which is the whole property the count exists to enforce. +files=() +suffixes="" +for f in "$logs"/round"$round".r*.tree; do + [ -f "$f" ] || continue + s=${f##*.r}; s=${s%.tree} + case "$s" in ''|*[!0-9]*) die "malformed reviewer suffix in $f (expected round.r.tree, M a number)" ;; esac + case " $suffixes " in *" $s "*) die "two tree files for round $round carry the same reviewer number r$s" ;; esac + suffixes="$suffixes $s" + files+=("$f") +done +found=${#files[@]} +[ "$found" -eq 2 ] || die "round $round has $found tree file(s); exactly 2 are required, one per reviewer" + +# ---------- read them ---------- +# There WAS a `read $read_ok of $found; refusing on a partial read` assertion here, modelled on +# the slice-16b row-count check. It was removed rather than kept, because it could not fail: +# every failure inside this loop calls `die`, which exits, so the counter always equalled the +# total by the time it was compared. A check that can only pass is worse than no check, because +# it reads as coverage -- this file's own CRITERIA say a row that cannot fail is not a +# criterion, and reviewer 1 applied that to the code rather than to the table. +# +# The real guarantee is structural and is stated instead of asserted: the loop is fail-closed +# at every step -- an unreadable file or a non-40-hex body exits non-zero and no signoff is +# written. `found` is separately pinned at exactly 2 above. +trees=() +for f in "${files[@]}"; do + t=$(tr -d '[:space:]' < "$f") || die "cannot read $f" + printf '%s' "$t" | grep -qE '^[0-9a-f]{40}$' \ + || die "$f does not contain a 40-hex tree hash (got: '${t:0:60}')" + trees+=("$t") +done +# There is deliberately NO count assertion here. The first one could not fail; its replacement +# could not fail either, for the identical reason -- `found` is pinned at 2 above and every +# in-loop failure exits -- and it was written three lines under a comment explaining that a +# check which can only pass is worse than none. Twice in one file is a pattern, not a slip, so +# the property is stated instead: this loop is fail-closed at every step, and `found` is the +# one place the count is actually enforced. + +[ "${trees[0]}" = "${trees[1]}" ] || { + echo " reviewer A: ${trees[0]}" >&2 + echo " reviewer B: ${trees[1]}" >&2 + die "the two reviewers of round $round read DIFFERENT trees" +} +reviewed="${trees[0]}" + +# ---------- the index must still be that tree ---------- +# 40-hex assumes a SHA-1 object format. In a SHA-256 repository every hash here is 64 and this +# script dies on all of them -- fail-closed, and recorded rather than handled, because this +# repository is SHA-1 and a widened pattern would accept a 40-hex hash from a 64-hex repo. +current=$(git write-tree) || die "git write-tree failed; refusing to sign an unmeasured index" +printf '%s' "$current" | grep -qE '^[0-9a-f]{40}$' || die "git write-tree did not return a tree hash" + +if [ "$current" != "$reviewed" ]; then + echo " reviewed: $reviewed" >&2 + echo " index is: $current" >&2 + die "the index has moved since round $round was reviewed; re-review the delta, do not sign" +fi + +# ---------- the diff being certified ---------- +# One definition of the recipe, in tools/gate.sh. This script deliberately holds no copy. +# LOGDIR is passed so gate.sh does not mktemp -d a directory it never cleans up; it has no +# EXIT trap, so every unpassed call leaked one. +tmp_logs=$(mktemp -d "${TMPDIR:-/tmp}/hacktui-signoff.XXXXXXXX") || die "cannot create a temp dir" +trap 'rm -rf "$tmp_logs"' EXIT +diff_hash=$(LOGDIR="$tmp_logs" ./tools/gate.sh staged-diff-hash) \ + || die "could not derive the staged-diff hash (unmeasurable, or the reviewable diff is empty)" +if [ "$diff_hash" = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" ]; then + die "the staged diff is empty; an empty-diff hash must never read as a signed review" +fi + +out="$slice_dir/REVIEW.signoff" +if [ -f "$out" ] && [ "$replace" != "--replace" ]; then + if grep -qF -- "$diff_hash" "$out" && grep -qF -- "Reviewed-tree: $reviewed" "$out"; then + echo "signoff already records this tree and diff; nothing to do." + exit 0 + fi + die "$out exists and records something else; pass --replace if that is intended" +fi + +{ + echo "# Slice: $slice_dir" + echo "#" + echo "# Round $round, two independent reviewers, both on a checkout of this index." + echo "# Written by tools/signoff.sh, which refused until \`git write-tree\` equalled the tree" + echo "# both reviewers printed. It records WHICH tree was read and WHICH diff is certified." + echo "# It does not prove a review happened -- the hash is written by the party it certifies." + echo "# See CLAUDE.md section 0 and .githooks/pre-commit." + echo "#" + echo "# Reviewer tree files:" + for f in "${files[@]}"; do echo "# $f"; done + echo "" + echo "Reviewed-tree: $reviewed" + echo "" + echo "$diff_hash" +} > "$out" + +echo "signoff written: $out" +echo " reviewed-tree: $reviewed" +echo " staged-diff: $diff_hash"