fix(hooks): the push guard asked where a push LANDS, and nothing asked what it CARRIES - #213
Merged
Merged
Conversation
…d what it CARRIES Two guards, both for paths the existing PROTECTED check waves through. GUARD A -- namespace allowlist. Refuse any push whose remote ref is outside refs/heads/ or refs/tags/. That is the shape of git push --mirror, which offers every ref in the clone including remote-tracking namespaces. A mirror push was refused before only INCIDENTALLY: it also offers local main as an update of refs/heads/main, so PROTECTED happened to fire. That is a property of one branch's state, not a rule, and it evaporates the moment main is up to date. GUARD B -- content check. Refuse a push whose ref's tip tree carries docs/security. That directory is gitignored, and an ignore rule governs only UNTRACKED paths, so it does nothing about a ref whose history already tracks those files. The path this closes is the likeliest of the set and is not a mirror at all: branch off a ref of that lineage and push it as an ordinary branch, which every other check here permits by design. PROVEN, not assumed. Both guards exercised via crafted pre-push stdin against a throwaway repo, with the fixture self-checked in both directions first (a fixture whose add -f lost to the ignore rule would make every assertion pass vacuously): case new old(HEAD) remote-tracking ref, mirror shape 1 0 ordinary branch, clean tip 0 0 ordinary branch, tip carries docs/security 1 0 tag push, clean tip 0 0 delete an unprotected branch 0 0 direct push to main 1 1 The old-guard column is the negative control: it returned 0 for exactly the two cases these guards add, so this is new coverage rather than restated behaviour. Refusals were checked to name the right reason, not merely to exit 1. WHAT THESE ARE NOT, stated in the code because the difference decides what a green run entitles anyone to conclude. Guard B reads the TIP TREE only -- a branch that added and then removed the files passes with a dirty history, so it is not a history check. It matches paths, not content. Every check here is skipped by --no-verify, by MEFOR_ALLOW_DIRECT_PUSH=1, and by the installed shim's own fail-open, which prints "THE PUSH GUARD IS OFF for this push" and exits 0 when python does not resolve. A fresh clone or new worktree has no hook at all until install-git-hooks.ps1 runs. A client-side hook cannot be the sole control and the docstring says so. Also fixes a false docstring in the test file, which asserted that git push --all sends every ref. It does not -- --all is refs/heads only, while bundle create --all and rev-list --all mean every ref. That belief is what makes someone treat --all and --mirror as interchangeable. 109 tests pass; ruff and mypy clean.
… was true when written 489 refs carrying docs/security content were deleted from this clone on 2026-08-05 with git update-ref -d, across THREE namespaces: refs/remotes/vault (20), refs/remotes/vaultall (466), and refs/vault (3). That third sits outside refs/remotes entirely and held the newest, densest content, so a cleanup scoped to refs/remotes would have missed it. THE STANDING WARNING AGAINST THIS IS NOW STALE, NOT WRONG. LEDGER-GATE.md and alloc.ps1 both named "deleting its refs" as the hazard the allocator ratchet defends against. Re-measured directly: BACKLOG max is 1032 and sub-floor max 353 both with and without the refs, ADR max 0161 either way, the allocator emits max+1 and never fills gaps, and the ratchets already persist 1031 / 1000 / 160. The warning was accurate when written, in the era when the floor did depend on the ref sweep; the ratchet and the public-boundary split made it independent since. It is updated rather than deleted, because the principle it teaches still holds. THE MULTISESSION PLAN GAVE A COMMAND THAT NO LONGER WORKS, and its description of the ref was wrong when written. It called vault/main a remote-tracking ref; git rev-parse --symbolic-full-name resolved it to refs/vault/main, and refs/remotes/vault/main never existed. Nor was a remote named vault ever configured -- only origin. The refs were orphaned namespaces from two direct-URL fetches on 2026-07-28, 45 seconds apart. Sessions should read the vault ledger from the separate MessageFoundry-vault clone instead. REVERSIBILITY, since deleting refs is only safe if it is undoable. A manifest of 489 refname/SHA pairs (464 unique commits -- 25 refs share a tip) is held outside this repo, durably, inside the vault clone's own .git. The objects remain addressable here, and every tip is REACHABLE from the vault clone's own refs, so they are gc-safe there rather than merely undeleted. gc.auto is set to 0 in this clone: it was unset with 7060 loose objects against a default threshold of 6700, already over, so a routine command could have fired an auto-gc and converted a reversible ref deletion into permanent loss. NOTHING WAS EVER PUBLISHED FROM THESE REFS. origin/main, all 30 origin refs and all 195 local branches carry zero docs/security files at tip and in history, confirmed three independent ways; the two graphs share no merge base.
wshallwshall
enabled auto-merge (squash)
August 5, 2026 22:12
… surfaced 1033. The rubric cites its own eleven signals as bare #N, and six of those numbers are real backlog items -- #3 is OPEN today, and #6/#7/#8/#10/#11 are closed items. Ten citations on four lines, re-measured against 780ee1d. Owner ruled on 2026-08-05 that they get disambiguated. The four-digit PR citations in the same file were fixed in PR #209; this is the short-number half that was deliberately left out of that scope. Two traps are recorded because each has already caught a reader. The #3 at L120 is a markdown ANCHOR FRAGMENT inside a link target, not a citation -- converting it silently breaks the link, and a prior census listed it as a signal because it counted tokens without printing context. And L299/L319 use backslash-escaped forms: a grep attempt during this triage returned ZERO matches on a file that demonstrably contains them, and the empty result was believed until a self-tested pattern contradicted it. The item says to prove the pattern fires before trusting a count from it. 1034. The pre-push shim exits 0 with "THE PUSH GUARD IS OFF for this push" when python is not on PATH. With enforce_admins OFF, push_guard.py is the only thing refusing an admin's direct push to main, and since the cutover that push is publication -- so the one control has a silent off switch that depends on an environment variable. As of today the shim switches off three guards rather than one, the two added alongside it being the namespace allowlist and the tip-tree check. Filed with the adjacent gaps in the same class rather than separately: a fresh clone or new worktree has no hook at all until install-git-hooks.ps1 runs, and --no-verify and MEFOR_ALLOW_DIRECT_PUSH=1 skip everything by design. The item states plainly that a client-side hook cannot be the sole control and that the durable answer is server-side, with the shim as defence in depth. Numbers allocated via scripts/coord/alloc.ps1, never by grepping for the next free one. Validated with parse_items rather than a hand-rolled scan: 114 items, zero duplicate numbers, 1033 and 1034 each carrying exactly one open banner. Hygiene gate OK at 309 across both ledger files.
wshallwshall
added a commit
that referenced
this pull request
Aug 5, 2026
A SEMANTIC conflict that no file-overlap check could see. #213 added test_the_shim_fails_open_when_python_is_absent_and_says_so, asserting the string "THE PUSH GUARD IS OFF for this push" is present in install-git-hooks.ps1. This branch removes that string, because it IS the fail-open. Disjoint files -- #213 touched the test module, this branch touches the installer -- so merge-tree reported CLEAN and the fork-point file intersection was empty. Two independent reviewers certified the merge on that basis and both were right about files. Measured before this commit: main's test file plus this branch's installer gives 1 failed, 1 passed. After the merge, and before this fix, the branch reproduced it exactly -- 1 failed, 24 passed. The old test was right about the bypass and did its job. It called itself "NOT a change request -- a pinned statement of the widest bypass in the whole mechanism", which is what made BACKLOG #1034 a decision rather than a discovery. It is now collecting on that: the behaviour changed, so the pin is re-aimed rather than deleted. It now asserts the inverse -- the fail-open notice is GONE, the shim refuses, and the refusal names --no-verify, because a fail-closed gate that does not say how to proceed deliberately gets "fixed" by deleting the gate. Scope stated in the docstring: this is a string property of the generator, which is all this module can see. The real coverage lives in tests/test_installed_coord_hooks.py, which reads the interpreter-resolution branch specifically and EXECUTES the shim under sh with an empty PATH to check the exit code git would act on. A text scan proves the source says exit 1, not that the branch is reached. Merged rather than rebased, deliberately: this PR is armed, and a force-push on an armed PR is the manoeuvre this repo's coordination has spent the day telling sessions not to make.
wshallwshall
added a commit
that referenced
this pull request
Aug 6, 2026
…t run (#215) * test(hooks): nothing asserted the coordination hooks were installed at all The payload parity test compares an installed copy against source and SKIPS when nothing is installed. So a box that never ran the installer and a box that is installed and current both rendered as green, and the two states that matter most were indistinguishable in the output. A guard whose presence cannot be demonstrated is not a control, and prose crediting it with one rests on a false premise. Assert presence off CI; skip on it. CI genuinely installs no hooks -- the installer is run per box by a human and no workflow invokes it -- so failing there would be a false red on every job. The skip prints the absent state rather than a bare reason, so a CI log still says what was missing instead of leaving a bare dot. The discriminator is an environment marker, not the hooks directory's own emptiness: inferring intent from the state under test can only ever conclude that absence is normal, which is the defect. Both branches are proven rather than claimed. Pointing core.hooksPath at an empty directory fails the new assertion off CI and skips it with GITHUB_ACTIONS=true. The negative control exercises the presence and marker predicates directly and never by removing a real hook: those files live in the common git dir and fire for every worktree on this box, so a test may not take one out from under a concurrent session. Shim markers are parsed out of the installer rather than copied into the test, so a rename cannot leave the test quietly checking nothing, and a guard test fails if the parse yields an empty set. Scope, stated so this is not over-read: pre-push guards protected BRANCH NAMES and commit-msg is the claim gate. Neither inspects file content, so neither is a leak gate and this passing is not evidence that anything scans what you push. * fix(hooks): the shims exited 0 at the one moment they provably had not run Both generated shims resolved an interpreter and, finding none, printed "THE PUSH GUARD IS OFF for this push" (or the claim-gate equivalent) and exited 0. Git reads 0 as permission, so the single condition in which the gate is known not to be evaluating anything was also the condition in which it permitted everything. Silent, and biased toward allowing. Not theoretical on this box: the only python on PATH is a per-user Microsoft Store app-execution alias, exactly the kind of entry that stops resolving after a profile or Store change. Both shims now refuse, name what is unguarded, and name the deliberate way forward (--no-verify). A fail-closed gate that does not say how to proceed is one that gets "fixed" by deletion. Naming --no-verify is not a weakening: a client-side hook is bypassable by construction, so the choice is only whether the bypass is deliberate or accidental. The -Status text that described the old behaviour is corrected in the same commit; it would otherwise have been a compensating control resting on a false premise, which is the defect this repo's standards name. PROOF, rather than assertion in prose. Against the OLD shims, all four new assertions fail (2 text, 2 behavioural); against the new ones they pass. The behavioural pair runs the real shim body under sh with PATH pointed at an empty directory, which is precisely the state the branch exists for, and checks the exit code git would act on -- a text scan proves the source says "exit 1", not that the branch is reached. ALSO CLOSES the gap that would have made this change inert: nothing compared the installed SHIM against the generator. Payload parity covers the .py files and the marker check covers "a hook of ours is present"; neither reads the body where the interpreter branch lives. So a fixed source with a stale install would have looked green everywhere while the old shim kept exiting 0 -- the same never-installed-reads-as-installed defect one level over. That new parity test is RED on this box right now, correctly: the installed shims still hash to the pre-fix generator. It skips on CI, which installs no hooks, so CI stays green. Clearing it needs a deliberate machine-global step: pwsh -NoProfile -File scripts\coord\install-git-hooks.ps1 Verified the direction first, as that test's own message demands: the installed shims match HEAD's generator exactly, so installing from this checkout is an upgrade and not the stale-checkout downgrade. NOT IN SCOPE, flagged rather than swept in: .mefor-hooks/pre-commit and the identical string the IDE extension writes in ide/src/sourceControl.ts carry the same fail-open ("python not found; skipping checks"). Different subsystem, different risk profile -- those land in a scaffolded user config repo rather than in the published artifact -- so they want their own decision. * feat(security): the leak gate could not see the class it most needed to catch Every detector in scan_forbidden.py judges a file by its BYTES -- customer and partner names, routable IPs, site codes, secrets. The private security corpus is prose ABOUT this repo (threat models, ASVS assessments, remediation plans), so it carries none of those. The scanner reported it clean by working exactly as designed. Measured against the 89-document vault corpus, the content detectors would have missed 55 of them. Add a LOCATION detector: docs/security/ is a hit whatever the file contains. Ordered BEFORE the binary early-return, deliberately. The content path drops any file with a NUL in its head, so a location rule placed after it would have been blind to every PDF, image and diagram in the corpus -- the widest possible hole in a leak gate. Proven end to end: a probe tree containing one markdown and one binary under docs/security/ produces two hits and exit 1, while an ordinary ADR beside them stays clean. This covers the path .gitignore cannot. docs/security/ is gitignored, so the ordinary route never reaches a commit. But a branch created from a fetched vault ref delivers those files inside a commit TREE, never through the index, so no ignore rule is ever consulted and the files end up legitimately tracked. That is the realistic leak path, and no instrument could see it. Reported at line 0: the finding IS the path, and there is no line inside the file to point at. Kept narrow. docs/reviews/ and docs/marketing/ are gitignored too, but for tidiness rather than because publishing them hands an attacker a map, and a detector that cries wolf gets deleted. The negative control pins the sharp edge: scripts/security/, tests/test_scan_forbidden.py, docs/SECURITY.md and messagefoundry/auth/security.py must NOT match, because a rule written as a bare "security" substring would flag the gate's own source on every run. The real repo scans clean, exit 0. SCOPE, so this is not over-read: this closes the LOCAL half. The CI leak job still triggers only on pull_request, push to main, and cron, so a branch pushed without a PR is not scanned by it at all -- the detector cannot fire where the job does not run. That trigger is a separate change. * test(hooks): the shim test pinned the fail-open this branch removes A SEMANTIC conflict that no file-overlap check could see. #213 added test_the_shim_fails_open_when_python_is_absent_and_says_so, asserting the string "THE PUSH GUARD IS OFF for this push" is present in install-git-hooks.ps1. This branch removes that string, because it IS the fail-open. Disjoint files -- #213 touched the test module, this branch touches the installer -- so merge-tree reported CLEAN and the fork-point file intersection was empty. Two independent reviewers certified the merge on that basis and both were right about files. Measured before this commit: main's test file plus this branch's installer gives 1 failed, 1 passed. After the merge, and before this fix, the branch reproduced it exactly -- 1 failed, 24 passed. The old test was right about the bypass and did its job. It called itself "NOT a change request -- a pinned statement of the widest bypass in the whole mechanism", which is what made BACKLOG #1034 a decision rather than a discovery. It is now collecting on that: the behaviour changed, so the pin is re-aimed rather than deleted. It now asserts the inverse -- the fail-open notice is GONE, the shim refuses, and the refusal names --no-verify, because a fail-closed gate that does not say how to proceed deliberately gets "fixed" by deleting the gate. Scope stated in the docstring: this is a string property of the generator, which is all this module can see. The real coverage lives in tests/test_installed_coord_hooks.py, which reads the interpreter-resolution branch specifically and EXECUTES the shim under sh with an empty PATH to check the exit code git would act on. A text scan proves the source says exit 1, not that the branch is reached. Merged rather than rebased, deliberately: this PR is armed, and a force-push on an armed PR is the manoeuvre this repo's coordination has spent the day telling sessions not to make.
wshallwshall
added a commit
that referenced
this pull request
Aug 6, 2026
…ocumented (#217) Follow-up to #213, which built the content guard. These are the two adjacent gaps BACKLOG #1034 lists as "worth deciding together". Both were DOCUMENTED rather than accidental, which is the reason to change them: writing a fail-open down does not close it, and a comment explaining why a guard permits is still a guard permitting. 1. AN UNREADABLE TIP TREE WAS TREATED AS CLEAN. _private_paths_in_tip returned [] both when a tree carried nothing and when the question could not be answered. The prior reasoning was that git only ever hands a pre-push hook local objects, so the unreadable case "does not arise". That holds for the OBJECT and not for the other branch of the same failure: the guard resolves `git` through PATH while git invokes the hook by absolute path, so a GUI client can run this where bare `git` does not resolve. OSError, [], clean, push allowed, nothing printed. It now returns None for "could not look" and the caller refuses, naming which of the two happened so an operator can tell a broken environment from a real leak. The cost is real and stated: an unreadable object now blocks the push, and the refusal names --no-verify for the case where that is spurious. 2. THE ESCAPE HATCH DISARMED EVERY GUARD, NOT THE ONE IT IS NAMED FOR. MEFOR_ALLOW_DIRECT_PUSH returned before any ref was read, so it switched off the namespace and content guards too. The variable's name is the argument: "allow direct push" is a claim about where a push LANDS, and letting it decide what a push CARRIES is a bypass nobody asked for. Pushing to main is a judgement a human can reasonably make; publishing the private security corpus is not. The test that pinned the old behaviour asked to be told: "if the variable is ever narrowed to the protected-branch check alone, this test is what says so." It has been narrowed, so that tripwire is re-aimed rather than deleted -- and a second test now pins that the hatch still permits the case it exists for, so scoping it cannot silently become removing it. TESTS: four PERMIT cases moved off the synthetic sha onto tree_repo's real clean commit. They previously passed only because Guard B could not read the object -- asserting "the guard did not look" while reading as "the guard looked and approved", which is the distinction this file exists to keep apart. Fail-closed removes that crutch, so the permits now prove something. AND A WEAKENING THIS CHANGE INTRODUCED, fixed in the same commit: with Guard B failing closed, a synthetic sha is refused on its own, so three refusal tests asserting only `returncode == 1` plus "REFUSED" would have passed with the protected-branch guard deleted entirely. Each now names the refusal it means to prove. Verified against the merged #213 implementation before writing anything: my tests were run against main's guard, and the two cases that FAILED are exactly these two. The content check on branches and on tags already worked, so nothing here re-implements them.
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.
The push guard asked where a push lands. Nothing asked what it carries.
What this adds
Two new refusals in
scripts/hooks/push_guard.py, alongside the existing protected-branch check:refs/heads/andrefs/tags/(the--mirrorshape)docs/security/)This is the structural half of the vault-lineage cleanup: 489 orphaned refs under
refs/remotes/vault/**,refs/remotes/vaultall/**andrefs/vault/**were deleted from this clonebecause a single
git push --mirrorwould have published them. Deleting the refs removed today'sexposure; this guard removes the shape of it.
Verified by attack, with a negative control
Crafted pre-push stdin against a throwaway repo, fixture self-checked in both directions first:
docs/securitymainThe old-guard column is the negative control: HEAD's guard returns 0 for exactly the two cases this
adds, so this is new coverage, not restated behaviour. Refusals were also checked to name the
right reason -- a refusal for an unrelated cause would otherwise pass as this one.
Honest limits, stated in the code rather than only here
dirty history.
--no-verify, byMEFOR_ALLOW_DIRECT_PUSH=1, and by the shim's fail-open.install-git-hooks.ps1runs.A client-side hook cannot be the sole control, and the docstring says so.
Also in this PR
docs/LEDGER-GATE.mdrecords the ref cleanup and retires a warning that was true when written: itsaid deleting those refs would break the allocator floor. Measured directly -- BACKLOG max is 1032 and
sub-floor max 353 both with and without them, and the ratchet already holds 1031/1000/160. The warning
is retired with its reasoning rather than deleted.
Verification
ruffclean (1043 files),mypyclean (278 files), 109 tests pass, backlog hygiene OK at 307, banditpassed via pre-commit. Glyph delta measured per file rather than assumed: zero cp1252-breaking
characters introduced in any of the five changed files.
Based on
780ee1d9==origin/main; two-dot and three-dot diffs agree, so no squash-merge revert.Follow-ups filed with this work (BACKLOG #1033, #1034)
Both surfaced by the guard work rather than being separate errands, so they ship with it:
#Nsignal citations. Ten on four lines; six of the seven distinct numbersare real backlog items and
#3is open today. Records the trap that#3at line 120 is amarkdown anchor fragment, not a citation -- converting it silently breaks the link -- and that
two of the lines use backslash-escaped forms a naive lookbehind cannot see. Sets the verification
bar explicitly: print the token list, never a count.
PATH. Withenforce_adminsOFF this is the only thing refusing an admin's direct push to
main, and after this PR it switchesoff three guards rather than one. Filed alongside the adjacent gaps (no hook in a fresh clone,
--no-verify,MEFOR_ALLOW_DIRECT_PUSH=1) so they are decided together, and it states plainly thata client-side hook cannot be the sole control -- the durable answer is server-side.
Numbers allocated via
alloc.ps1. Validated withparse_items: 114 items, zero duplicate numbers,exactly one status banner each, hygiene gate OK.