diff --git a/PENDING.md b/PENDING.md index 1c1c5cc..dc5b852 100644 --- a/PENDING.md +++ b/PENDING.md @@ -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. diff --git a/git/hooks/pre-commit b/git/hooks/pre-commit index 3760511..a216261 100755 --- a/git/hooks/pre-commit +++ b/git/hooks/pre-commit @@ -42,16 +42,26 @@ if echo "$added_lines" | grep -qE "(console\.log|debugger|binding\.pry|TODO:|FIX fi # Check for large files (>5MB) — operates on file size, not diff content -for file in $(git diff --cached --name-only); do +# +# -z / read -d '': the path list MUST be null-delimited. Unquoted $(git diff +# --cached --name-only) word-splits, so a staged path containing whitespace broke +# into tokens, every token failed the [ -f ] test below, and the file was skipped +# ENTIRELY — a 17 MB "my big file.dat" passed this check without it ever running. +# That is REVIEWED-105's class again (a check that passes because it could not +# run), and it is why the loop is fed by process substitution rather than a pipe: +# a pipe would put the body in a subshell where `exit 1` cannot refuse the commit. +# Measured 2026-08-26 (PENDING-163 AMENDMENT 1). Narrows nothing, widens nothing — +# it makes this check do what it already claimed to do. +while IFS= read -r -d '' file; do if [ -f "$file" ]; then size=$(wc -c < "$file") - if [ $size -gt 5242880 ]; then + if [ "$size" -gt 5242880 ]; then echo -e "${RED}Error: $file is larger than 5MB${NC}" echo "Consider using Git LFS for large files" exit 1 fi fi -done +done < <(git diff --cached --name-only -z) # Check for secrets in ADDED lines only (basic check) if echo "$added_lines" | grep -qE "(password|secret|token|api_key)[[:space:]]*=[[:space:]]*[\"'][^\"']+[\"']"; then