[FIX] pre-commit: the size check word-split on paths and skipped them entirely
for file in $(git diff --cached --name-only) is unquoted, so a staged path
containing whitespace split into tokens, every token failed the [ -f ] guard, and
the file was never measured. A 17MB "my big file.dat" passed the 5MB ceiling
without the check ever running — REVIEWED-105's class (a check that passes
because it could not run), in the guard rather than in a declared check.
Now null-delimited (-z / read -d ''), fed by process substitution rather than a
pipe so `exit 1` still refuses the commit from inside the loop body.
Controls, run before committing:
space-in-name 17MB -> REFUSED (the fix; previously committed)
plain 17MB -> REFUSED (unchanged)
small file -> COMMITTED (unchanged)
staged deletion -> COMMITTED, no crash (the [ -f ] guard is intact)
newline+unicode name-> REFUSED (impossible under the old loop)
Narrows nothing and widens nothing: it makes the check do what it already said.
The 5MB ceiling and the LFS advice line are UNTOUCHED — that is the policy
question in PENDING-163, and it is the steward's.
Also files PENDING-163 AMENDMENT 1 (joins, replaces nothing), raised by the jurist
reading the item against REVIEWED-100/105 and verified empirically here:
- CONFIRMED: option (ii) does NOT widen permissions generally. git cat-file -s
reads the staged blob: an LFS-tracked 17MB file stages at 133 bytes, a plain
one stages at 17825792 and is still refused. The item's "widens what may be
committed everywhere" is withdrawn as false. That error is why the fork went
to the steward as a policy question at all.
- ACCEPTED: .gitattributes already is the per-repo versioned declaration that
option (iii) proposed to build. (iii) WITHDRAWN.
- CONFIRMED, and worse than visible from outside: (iii) inverts REVIEWED-100's
polarity, and the parser would refuse an exemption line as malformed.
- The jurist's fourth point does NOT hold — line 46's [ -f "$file" ] guard is
present, so staged deletions never reach wc -c. Flagged by them as inferred,
and it was. But the class they predicted is real, at line 45, by a different
mechanism. The inference was wrong; the instinct was not.
Recommendation changes from "(i) now, (iii) later" to "(ii)". Still the steward's.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NvZAKSf9aqratbqHbU9LK5
This commit is contained in:
co-authored by
Claude Opus 5
parent
171be14ab7
commit
ecee76b862
+40
@@ -5853,3 +5853,43 @@ central finding is that care is not a mechanism.
|
||||
|
||||
**Files affected:** `~/dotfiles/git/hooks/pre-commit` (not modified).
|
||||
**Awaiting:** Steward authorization.
|
||||
|
||||
### PENDING-163 — AMENDMENT 1: the item overstated (ii)'s cost by a category, and (iii) should be withdrawn
|
||||
**Date:** 2026-08-26
|
||||
**Raised by:** the jurist, reading PENDING-163 verbatim against REVIEWED-100 and REVIEWED-105 through the governance tools. **JOINS the item above; replaces nothing.**
|
||||
**Verified by:** the executor, empirically, in a throwaway repo — the jurist has no access to `~/dotfiles/git/hooks/pre-commit` and correctly flagged its fourth point as inferred rather than verified.
|
||||
|
||||
**(1) The jurist is right, and my framing was wrong by a category. CONFIRMED.**
|
||||
`git cat-file -s :"$file"` reads the **staged blob**, so option (ii) is gated on LFS tracking, not open to large files generally:
|
||||
|
||||
| file | working-tree size (measured today) | staged blob size (what (ii) measures) | under (ii) |
|
||||
|---|---|---|---|
|
||||
| `tracked.big`, LFS-tracked via `.gitattributes` | 17,825,792 | **133** | passes |
|
||||
| `plain.dat`, staged normally | 17,825,792 | **17,825,792** | **still refused** |
|
||||
|
||||
**PENDING-163's "widens what may be committed everywhere" is withdrawn as false.** (ii) admits exactly the files someone has deliberately declared. ⚠ This error is why the fork was put to the steward as a policy question at all: it made (i) look safer than it is and (ii) costlier than it is. **The steward paused that question, and was right to.**
|
||||
|
||||
**(2) The distance between (ii) and (iii) collapses. ACCEPTED.** LFS tracking lives in `.gitattributes` — per-repo, in-repo, versioned, diff-visible. That is the property (iii) proposed to build, and it already exists.
|
||||
|
||||
**(3) (iii) inverts REVIEWED-100's polarity — CONFIRMED, and worse than the jurist could see from outside.** REVIEWED-100 authorized a declaration that causes checks to **run**; an exemption is a declaration that causes a global check **not to** run. Same file, opposite sign. Two things visible only in the source:
|
||||
- the parser **refuses** any line lacking `|` (`refuse_declaration "no '|' separator"`), so an exemption line is not merely awkward — it is rejected as malformed. (iii) needs new syntax *and* new semantics.
|
||||
- REVIEWED-105 (e) fires as the jurist predicted: a triggers file declaring zero rules prints *"exists but declares no rules — this repo is opted in and unguarded."*
|
||||
|
||||
**⇒ Option (iii) is WITHDRAWN. Recommendation changes from "(i) now, (iii) later" to "(ii)".**
|
||||
|
||||
**(4) The jurist's co-located finding does NOT hold — and its failure class is present anyway, by a different mechanism.**
|
||||
|
||||
*Not held:* line 46 carries `if [ -f "$file" ]; then` before `wc -c`. A staged deletion has no working-tree file, `[ -f ]` is false, the body is skipped; `wc -c` never runs and `size` is never empty. Reproduced live: `git rm victim.txt` → `victim.txt -> [ -f ] FALSE -> body SKIPPED`. **The guard the jurist could not see is present.**
|
||||
|
||||
*⚠ But the predicted class IS there, at line 45.* `for file in $(git diff --cached --name-only)` is **unquoted**, so a path containing whitespace word-splits:
|
||||
|
||||
staged: my big file.dat (17,825,792 bytes)
|
||||
loop iterates: 'my' -> skipped · 'big' -> skipped · 'file.dat' -> skipped
|
||||
|
||||
**A 17 MB file passes the size check entirely if its name contains a space.** That is exactly *"a check that passes because it could not run"* — the REVIEWED-105 class — reached by a different route than the one inferred. The inference was wrong; the instinct behind it was not.
|
||||
|
||||
**Disposition, split by authorization level:**
|
||||
- **The line-45 quoting bug is a `[FIX]`** — a scoped defect against existing specification. The guard is specified to refuse files over 5 MB and currently fails to for a nameable class of them. Fixing it **narrows nothing and widens nothing**; it makes the check do what it already says. Implemented directly.
|
||||
- **(i) vs (ii) remains the steward's**, but is now a much smaller question than the item posed: not *"may large files enter these repos"* but *"may a repo exempt a declared class by committing a `.gitattributes` line."*
|
||||
|
||||
**Awaiting:** steward authorization on (i) vs (ii) only. The `[FIX]` is not awaiting.
|
||||
|
||||
Reference in New Issue
Block a user