chore(claude): track .claude/settings.json and fix the rules it was not enforcing - #373
Open
wshallwshall wants to merge 5 commits into
Open
chore(claude): track .claude/settings.json and fix the rules it was not enforcing#373wshallwshall wants to merge 5 commits into
wshallwshall wants to merge 5 commits into
Conversation
… enforcing `/.claude/` ignored the directory, so `.claude/settings.json` was never tracked and `git worktree add` could not deliver it -- the same failure the neighbouring comment already records for CLAUDE.md. Measured 2026-08-13: of 62 local checkouts carrying CLAUDE.md, only 12 had `.claude/settings.json`. The other 50 ran with no deny-list over `.env` / `secrets/**` / keys / the local `*.db` store, and no `block-blanket-git-stage` PreToolUse guard. Those are client-ENFORCED controls; CLAUDE.md section 5's prose is context, not enforcement, so it does not substitute. BACKLOG #327 recorded the same gap from the other side -- "it is wired through .claude/settings.json, which is itself inside the now-gitignored /.claude/ tree and untracked ... Do not count it as coverage" -- and carried the stale `.gitignore:84` comment to the owner rather than fixing it. Both are resolved here. Ignore by contents (`/.claude/*`) rather than by directory, then re-include the one file. The directory form would have made the negation a silent no-op, because git cannot re-include a file whose parent directory is excluded. `/.claude/worktrees/` is now named explicitly instead of relying on that wildcard plus a per-clone `.git/info/exclude` line that reaches nobody else. Publishing the file made two of its defects load-bearing, so both are fixed here rather than shipped: * Both hook commands were bare `pwsh -NoProfile -File scripts/hooks/...`, which resolves against the session's working directory. A hook that cannot start is NON-BLOCKING -- the action proceeds and the only trace is a notice -- so the staging guard read as enforced in the file and was absent in any session started elsewhere. Now `${CLAUDE_PROJECT_DIR}` in exec form. * All 16 file deny rules used the `./` anchor, which matches one directory. Bare patterns follow gitignore semantics and match at any depth, so `Read(.env)` is strictly broader than `Read(./.env)` and reads identically in review. Prefix dropped. Three `Get-Content` denies added for the PowerShell path, which the documented Read/Edit deny coverage does not reach. The allow list went from 11 hyper-literal command strings to 5 wildcards. Allow rules are the only permission rules gated on the workspace trust dialog, so they applied in none of the untrusted checkouts anyway; the literals also could not survive an argument change, which is how the list grew three near-duplicate pytest invocations. Deny rules are not trust-gated and are evaluated before the auto-mode classifier, so they are the half worth getting right. tests/test_private_paths_stay_ignored.py FAILED on the previous commit and that was correct: it asserts nothing under a private rule is tracked, and tracking settings.json violated that. Pre-commit does not run pytest, so nothing caught it at commit time. Updated deliberately, per that file's own doctrine -- the tracked set under `.claude/` is now pinned as an exact SET, not a floor, so a second negation fails the build instead of publishing. Its new companion test asserts the asymmetry directly: settings.json un-ignored, and `rules/`, `skills/`, `agents/`, `worktrees/` and `settings.local.json` still ignored. tests/test_claude_settings_contract.py is new and covers the payload: the PHI and secret denies are present, no rule regresses to `./`, every hook anchors to the project root, and every referenced script exists. Both absence checks carry a planted-omission self-test, because an absence assertion over a currently-correct file passes just as well when the check itself is broken. `.claude/` also leaves link_check.py's WITHHELD tuple. It was exempt because 7 links pointed at a file no clone had; all 7 name settings.json, which is now tracked, so they resolve honestly and are COUNTED -- the exemption `continue`d before `checked += 1`, so those links were never in the total. Repo-wide link count moves 5359 to 5405. CONTRIBUTING.md discloses what cloning now configures: two PowerShell scripts wired to SessionStart and PreToolUse, that they need pwsh and fail open without it, and that the deny rules anchor at the directory the agent was started in and do not cover writes into sibling worktrees by absolute path.
The owner chose option (a) on PR #373: land the 27-rule deny-list, HOLD the three hook registrations. Held is NOT rejected -- no verdict was given on the hooks, and no later change may cite this as one. The two halves have different risk profiles, which is why they separate: DENY-LIST 27 rules mirroring CLAUDE.md section 5 almost literally -- .env, .env.*, secrets/**, *.key, *.pem, *.pfx, *.db, *.db-wal, *.db-shm, bootstrap-admin.txt. The repo's own written policy as enforcement. HOOKS PreToolUse (Bash, PowerShell) -> block-blanket-git-stage.ps1, and SessionStart -> session-context.ps1. Changes what EXECUTES on every matching tool call in every checkout. A deny-list can only refuse; a PreToolUse hook runs code. KNOWN INTERIM CONDITION, measured rather than assumed: with no `hooks` key, test_every_hook_resolves_through_the_project_dir_placeholder and test_every_hook_script_actually_exists iterate an EMPTY set -- 0 handlers -- so they pass VACUOUSLY. Nothing fails; two guards quietly stop guarding until the hooks decision is made, at which point both become live again automatically. This file's own test_the_checks_can_actually_fail exists because absence assertions over a correct file are "the shape that passes just as well when the check is broken" -- but it runs against PLANTED documents, so it passes and does NOT catch this vacuity. The negative control does not cover an empty input set. Verified: 7 passed in tests/test_claude_settings_contract.py; deny 27, allow 5, dot-anchored denies 0, hook handlers 0.
wshallwshall
enabled auto-merge (squash)
August 13, 2026 23:31
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tracks
.claude/settings.jsonand fixes what it was silently not enforcing. 7 files, +405 -31.The finding that motivates it: the file was untracked, so by the authoring session's count 50 of 62
local checkouts ran with no PHI/secret deny-list and no staging guard. Tracking it is the fix; the
rest of the diff is correcting rules that were not doing what they read as doing -- hooks moved to the
${CLAUDE_PROJECT_DIR}exec form, deny rules off the narrow./anchor, allow list to wildcards.Adds
tests/test_claude_settings_contract.py. Updatestests/test_private_paths_stay_ignored.py,which the authoring session's first commit genuinely broke -- they say so plainly, and note pre-commit
does not run pytest, which is why it survived to a second commit.
Not armed by the coordinator. This changes the permission and deny-list surface every session on
this machine runs under -- the control plane, not a control. It is revertible and CI-gated, so this is
not a blocking concern; it is a "the owner should see the diff" concern.
Verification as reported by the authoring session, not re-run here: full suite 12239 passed, 19 failed,
all 19 pre-existing and verified identical on a clean branch. Treat the 19 as needing a second look --
this repo has a known trap where worktree venvs miss five CI extras and produce failures that look
inherent.