diff options
Diffstat (limited to 'todo.org')
| -rw-r--r-- | todo.org | 105 |
1 files changed, 51 insertions, 54 deletions
@@ -39,60 +39,6 @@ Tags are assigned and refreshed by =task-audit=; =task-review= keeps them honest * Rulesets Open Work -** DONE [#C] claude-templates/bin/ gets no lint coverage at all :bug:quick:solo: -CLOSED: [2026-07-24 Fri] -:PROPERTIES: -:LAST_REVIEWED: 2026-07-24 -:END: -Fixed 2026-07-24 (sentry fire 8, f91feef). lint.sh now sweeps =claude-templates/bin/*= through =check_hook=, matching the extensionless-file shape =languages/*/githooks/*= already uses. Added =scripts/tests/lint-coverage.bats=, which pins the *coverage* rather than current cleanliness: it plants a broken file in each swept location and asserts lint.sh complains, so a location that silently stops being swept fails the suite. The broader question this surfaced — whether rulesets should run shellcheck on its own shell at all — is the VERIFY below and stays Craig's call. -=scripts/lint.sh= sweeps =scripts/*.sh=, =languages/*/claude/hooks/*.sh=, and =languages/*/githooks/*= through =check_hook= (shebang present, executable bit set). It never touches =claude-templates/bin/=. Verified: zero references to that path in the file. - -Those four scripts — =ai=, =agent-text=, =agent-page=, =install-ai= — are the ones =make install= symlinks into =~/.local/bin=, so they run on Craig's PATH on every machine. They are the *most* exposed shell in the repo and the only shell with no gate over it. - -All four are clean today (shebangs present, mode 755, shellcheck-clean when run by hand), so nothing is broken. This is a missing gate, not an active defect: the =ai= launcher was hardened to 42 tests recently and that cleanliness is not enforced going forward. - -Grading: Minor severity (nothing broken now; the exposure is a future regression in a PATH-installed script going uncaught) x every user, every time (every =make lint= silently skips them) = P2 = [#C]. - -Fix: add =claude-templates/bin/*= to the =check_hook= loop, the same shape =languages/*/githooks/*= already uses for extensionless files. A no-op today by design — it passes immediately — which is exactly what a guard should do. - -** DONE [#A] Applied: the secret-scan pre-commit fails open in ALL FIVE bundles (from .emacs.d) -:PROPERTIES: -:LAST_REVIEWED: 2026-07-24 -:END: -CLOSED: [2026-07-24 Fri] -Applied 2026-07-24 across all five bundles (11 sites: the secret-scan input in each, plus each bundle's staged-file list, typescript having two). Verified on all five: refuses when the diff can't be read, still blocks a real staged secret, still passes a clean commit. Adopted .emacs.d's elisp bats suite (8 tests) and extended the repo-level cross-bundle suite with two fail-closed assertions. - -Separate defect found while wiring that up: the cross-bundle suite's VARIANTS list read "elisp bash go" while python and typescript also shipped hooks, so every "in every variant" assertion had silently skipped two bundles since I added them. VARIANTS is now discovered from the tree rather than enumerated — the same failure class the tests exist to catch. - -What arrived: .emacs.d found that =languages/elisp/githooks/pre-commit= builds its scan input as =added_lines="$(git diff --cached -U0 ... | grep '^+' | grep -v '^+++' || true)"=. With no pipefail and =|| true= swallowing everything, any git failure yields an empty string, so the scan searches nothing, finds nothing, reports clean, and the commit proceeds with the secret in it. The staged-file list feeding the paren check has the identical hole. - -*Verified independently, and it is worse than reported.* I reproduced the fail-open with a stub git that fails only the staged-diff call: exit 0 with an AWS-shaped key staged. Then I checked the other bundles, which the sender did not: *bash, go, python, and typescript all carry the same pattern and all fail open the same way.* Confirmed live on all five. Two of those (python, typescript) are hooks I wrote on 2026-07-24 by copying the bash one, so I propagated the defect while closing a different gap. - -Graded [#A] on severity alone, per the todo-format security carve-out: a credential-scanning gate that reports clean without having looked is a showstopper regardless of how rarely git fails. - -The sender's fix is correct and I verified all three axes on it: refuses to proceed when the diff cannot be read (exit 1), still blocks a real staged secret (exit 1), still passes a clean commit (exit 0). It splits the git read from the greps so a git failure aborts while "grep matched nothing" stays the ordinary case. - -Decision needed: the fix as sent covers elisp only. It should be applied to all five bundles, which is my scope expansion rather than the sender's proposal — hence a VERIFY rather than a silent apply. Also unresolved: where the two attached bats suites live, since the hooks are rulesets-owned and the tests currently sit in the consuming project. - -Prepared: [[file:working/hook-fail-open/pre-commit.diff]], plus =test-pre-commit-hook.bats= (8 tests) in the same dir. - -** DONE [#B] Applied: remove the validate-el auto-test cap (from .emacs.d, Craig's call) -:PROPERTIES: -:LAST_REVIEWED: 2026-07-24 -:END: -CLOSED: [2026-07-24 Fri] -Applied 2026-07-24. Cap and the unreachable notice helper removed; the gate is now count -ge 1. Adopted the rewritten bats suite (6 tests), with its hook path corrected from the installed .claude/hooks/ layout to the bundle's claude/hooks/ — the re-homing mismatch the sender flagged. - -What arrived: a superseding handoff. The first proposed a loud notice when the test count exceeds =MAX_AUTO_TEST_FILES=20= (above the cap the block was skipped, nothing printed, exit 0 — indistinguishable from a pass). Craig chose in the .emacs.d session to remove the cap instead. - -The reasoning is the valuable part. Measured, a whole family runs in under two seconds (calendar-sync 63 files / 633 tests / 0.9s), so the cap bought nothing. Worse, it was *concealing* a real cross-test pollution bug: calendar-sync exits 1 when its 63 files run in one process, because one test marks a calendar as syncing and never resets it, and a sibling file's test then hits the stale guard. Invisible from both directions — =make test= runs each file in its own Emacs, and the hook skipped the family for being over the cap. - -Verified the superseding file drops the cap entirely (zero references, gate is now =count -ge 1=) and removes the now-unreachable notice helper. - -Since Craig already made this call, the remaining decision is only adoption scope: removing the cap means other consuming projects run every stem-matched test file per edit, and one may go red on first use by surfacing pollution that per-file runs hid. The sender flags that as intended. - -Prepared: [[file:working/hook-fail-open/validate-el.diff]], plus =test-validate-el-hook.bats= (6 tests, two of which stage a deliberately failing test so a quiet pass proves the run happened). - ** TODO [#C] lint-org invalid-block false-positives inside example/src blocks :bug:solo: :PROPERTIES: :LAST_REVIEWED: 2026-07-24 @@ -2059,3 +2005,54 @@ So this is a hardening gap rather than an active bug: a future defect in a mecha Grading: Minor severity (no known active defect; the exposure is that any future one is unrecoverable within a session) x every user, every time (it runs on every wrap and every sentry fire) = P2 = [#C]. Fix direction: back up before the first mutation, matching =lint-org.el='s convention exactly — =/tmp/<basename>.before-todo-cleanup.<YYYYMMDD-HHMMSS>=. One copy per invocation, not per pass, and skip it under =--check= (which writes nothing). Test by asserting the backup exists and holds the pre-edit content after a real mutation. +** DONE [#C] claude-templates/bin/ gets no lint coverage at all :bug:quick:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed 2026-07-24 (sentry fire 8, f91feef). lint.sh now sweeps =claude-templates/bin/*= through =check_hook=, matching the extensionless-file shape =languages/*/githooks/*= already uses. Added =scripts/tests/lint-coverage.bats=, which pins the *coverage* rather than current cleanliness: it plants a broken file in each swept location and asserts lint.sh complains, so a location that silently stops being swept fails the suite. The broader question this surfaced — whether rulesets should run shellcheck on its own shell at all — is the VERIFY below and stays Craig's call. +=scripts/lint.sh= sweeps =scripts/*.sh=, =languages/*/claude/hooks/*.sh=, and =languages/*/githooks/*= through =check_hook= (shebang present, executable bit set). It never touches =claude-templates/bin/=. Verified: zero references to that path in the file. + +Those four scripts — =ai=, =agent-text=, =agent-page=, =install-ai= — are the ones =make install= symlinks into =~/.local/bin=, so they run on Craig's PATH on every machine. They are the *most* exposed shell in the repo and the only shell with no gate over it. + +All four are clean today (shebangs present, mode 755, shellcheck-clean when run by hand), so nothing is broken. This is a missing gate, not an active defect: the =ai= launcher was hardened to 42 tests recently and that cleanliness is not enforced going forward. + +Grading: Minor severity (nothing broken now; the exposure is a future regression in a PATH-installed script going uncaught) x every user, every time (every =make lint= silently skips them) = P2 = [#C]. + +Fix: add =claude-templates/bin/*= to the =check_hook= loop, the same shape =languages/*/githooks/*= already uses for extensionless files. A no-op today by design — it passes immediately — which is exactly what a guard should do. +** DONE [#A] Applied: the secret-scan pre-commit fails open in ALL FIVE bundles (from .emacs.d) +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +CLOSED: [2026-07-24 Fri] +Applied 2026-07-24 across all five bundles (11 sites: the secret-scan input in each, plus each bundle's staged-file list, typescript having two). Verified on all five: refuses when the diff can't be read, still blocks a real staged secret, still passes a clean commit. Adopted .emacs.d's elisp bats suite (8 tests) and extended the repo-level cross-bundle suite with two fail-closed assertions. + +Separate defect found while wiring that up: the cross-bundle suite's VARIANTS list read "elisp bash go" while python and typescript also shipped hooks, so every "in every variant" assertion had silently skipped two bundles since I added them. VARIANTS is now discovered from the tree rather than enumerated — the same failure class the tests exist to catch. + +What arrived: .emacs.d found that =languages/elisp/githooks/pre-commit= builds its scan input as =added_lines="$(git diff --cached -U0 ... | grep '^+' | grep -v '^+++' || true)"=. With no pipefail and =|| true= swallowing everything, any git failure yields an empty string, so the scan searches nothing, finds nothing, reports clean, and the commit proceeds with the secret in it. The staged-file list feeding the paren check has the identical hole. + +*Verified independently, and it is worse than reported.* I reproduced the fail-open with a stub git that fails only the staged-diff call: exit 0 with an AWS-shaped key staged. Then I checked the other bundles, which the sender did not: *bash, go, python, and typescript all carry the same pattern and all fail open the same way.* Confirmed live on all five. Two of those (python, typescript) are hooks I wrote on 2026-07-24 by copying the bash one, so I propagated the defect while closing a different gap. + +Graded [#A] on severity alone, per the todo-format security carve-out: a credential-scanning gate that reports clean without having looked is a showstopper regardless of how rarely git fails. + +The sender's fix is correct and I verified all three axes on it: refuses to proceed when the diff cannot be read (exit 1), still blocks a real staged secret (exit 1), still passes a clean commit (exit 0). It splits the git read from the greps so a git failure aborts while "grep matched nothing" stays the ordinary case. + +Decision needed: the fix as sent covers elisp only. It should be applied to all five bundles, which is my scope expansion rather than the sender's proposal — hence a VERIFY rather than a silent apply. Also unresolved: where the two attached bats suites live, since the hooks are rulesets-owned and the tests currently sit in the consuming project. + +Prepared: [[file:working/hook-fail-open/pre-commit.diff]], plus =test-pre-commit-hook.bats= (8 tests) in the same dir. +** DONE [#B] Applied: remove the validate-el auto-test cap (from .emacs.d, Craig's call) +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +CLOSED: [2026-07-24 Fri] +Applied 2026-07-24. Cap and the unreachable notice helper removed; the gate is now count -ge 1. Adopted the rewritten bats suite (6 tests), with its hook path corrected from the installed .claude/hooks/ layout to the bundle's claude/hooks/ — the re-homing mismatch the sender flagged. + +What arrived: a superseding handoff. The first proposed a loud notice when the test count exceeds =MAX_AUTO_TEST_FILES=20= (above the cap the block was skipped, nothing printed, exit 0 — indistinguishable from a pass). Craig chose in the .emacs.d session to remove the cap instead. + +The reasoning is the valuable part. Measured, a whole family runs in under two seconds (calendar-sync 63 files / 633 tests / 0.9s), so the cap bought nothing. Worse, it was *concealing* a real cross-test pollution bug: calendar-sync exits 1 when its 63 files run in one process, because one test marks a calendar as syncing and never resets it, and a sibling file's test then hits the stale guard. Invisible from both directions — =make test= runs each file in its own Emacs, and the hook skipped the family for being over the cap. + +Verified the superseding file drops the cap entirely (zero references, gate is now =count -ge 1=) and removes the now-unreachable notice helper. + +Since Craig already made this call, the remaining decision is only adoption scope: removing the cap means other consuming projects run every stem-matched test file per edit, and one may go red on first use by surfacing pollution that per-file runs hid. The sender flags that as intended. + +Prepared: [[file:working/hook-fail-open/validate-el.diff]], plus =test-validate-el-hook.bats= (6 tests, two of which stage a deliberately failing test so a quiet pass proves the run happened). |