Execute #71's containment on Windows, with the baseline that gives it meaning - #320
Merged
Conversation
CommitLore — record lintTrailers: clean — 6 commits in Active constraints for the paths this PR touchesLimits (24)
Ruled out (98)
Warnings (16)
Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
This was referenced Jul 31, 2026
Owner
Author
The steps in this PR pass once #321's fix is in front of themObserved on a throwaway head (#326) that layers this job onto T-1127's fix — PR #327. The two containment attacks have now executed for the first time. Until this run the baseline failed first and they never ran at all. Two lines in the measurement step are now stale, and they read as contradictionsSame log, same run: Both are wrong about the shipped code now, and both are this PR's to fix. They restate the pre-fix logic rather than deriving it: the measurement step hardcodes the old That is the same class of mistake this job exists to catch — a check that restates what it claims to measure. T-1127's own test does it the other way and can be copied: it slices the walk out of Suggested before this PR leaves draft: derive both diagnostics from the stub, then rebase onto |
… meaning T-1121 made Windows reachable. Whether the install-root containment holds there has only ever been established on macOS, and a property verified on one platform is not a property of the product. Four steps, in the order that makes the answer readable. The first measures rather than asserts: which sh git will use, what init recorded for bin/root/node, the stub's containment case recomputed verbatim, where a fall-through would land, and a real commit through the real hook. Its output is the evidence the next step's verdict has to be read against, and it is deliberately not an assertion -- a measurement that fails the job tells you less than one that prints. The other three assert. The baseline runs before the attacks because an attack that passes by nothing resolving at all is not containment, it is a hook that refuses everything, and only the baseline separates those. It asserts both directions: a valid record is accepted, an invalid one refused. Then #71's two attacks against a wrapper target -- a recorded bin outside the install root, and a recorded target with no recognised extension planted inside it -- each with a witness file the tampered program writes if it ever runs. Paths cross two worlds here and the test converts rather than assumes. init records a win32 path; the stub reads it under MSYS sh, where a witness bash calls /tmp/ran.log is C:\tmp\ran.log to node. cygpath -w for what git config records, cygpath -m for what is embedded in JavaScript, and the witness checked on the bash side of the same file. Mixing the forms would report "the tampered program never ran" because the check looked in the wrong place -- a false pass this ticket cannot afford. The witnesses are ESM for the same reason: .mjs is ESM, and a require() there throws before writing anything. Record-Id: r-winred1124 Limit: only a windows-latest runner can answer this; Git for Windows' bundled MSYS sh is what runs the hook, and no local macOS or Linux container reproduces its pwd -P output form Ruled-out: Asserting the two attacks without a baseline | a hook that fails to resolve anything refuses the attacks too, so the attacks alone cannot distinguish containment from a dead hook Ruled-out: Simulating the Windows path forms in a hermetic POSIX test | it can exercise the case pattern, but pwd -P's MSYS output is the unknown, so the test would assert its own assumption Ruled-out: Reusing bash-side /tmp paths for the witness files | node on Windows reads /tmp/x as C:\tmp\x, so the check would look somewhere the witness was never written and pass for the wrong reason Ruled-out: continue-on-error on the attack steps | the ticket forbids it, and a containment assertion that cannot fail the job asserts nothing Certainty: tentative Blast: local Undo: easy Verified: the workflow parses and install-ps1 now holds fourteen steps with no continue-on-error anywhere in the file; typecheck and build are clean and dist is unchanged by this commit Unverified: every one of the four new steps, none of which has run on Windows; whether the containment case can match at all there, which is what this commit exists to measure
… hung
The measurement step never finished. Step 11 commits before the hook exists and
took five seconds; step 12 commits after `init` installed it and was still running
seventeen minutes later, so the hang is the hook, and the run had to be cancelled
before it reached the three assertions behind it.
There is exactly one unbounded construct in the stub, and it is reachable from a
fall-through:
dir=$PWD
while [ -n "$dir" ]; do
if [ -x "$dir/node_modules/.bin/commitlore" ]; then exec ...; fi
dir=${dir%/*}
done
`${dir%/*}` returns the string unchanged when there is no `/` left in it. From
`/c/Users/x/repo` the walk reaches `/c`, then the empty string, and stops. From a
drive-letter form it reaches `C:` and stays there. Reproduced under /bin/sh
locally: four iterations for the POSIX form, non-terminating for the other.
So the Windows outcome is not the refusal this ticket was written to expect. It is
a commit that never returns, which is worse for a user than either refusing or
accepting, and it is a finding rather than an obstacle -- the stub belongs to
another owner and is not edited here.
What changes is the harness, so the finding is measured instead of inferred:
- every commit that passes through the hook runs under `timeout -k 5 90`, and 124
is reported as HUNG, distinct from a refusal
- the job carries `timeout-minutes: 30`, so a missed bound costs half an hour
instead of the runner's six-hour default
- a probe in the stub's own chained-hook slot reports what the hook actually sees
-- `$0`, `$PWD`, `pwd`, `pwd -P` -- and runs the same walk with a counter, so
the loop's behaviour is read off the runner rather than argued from the source.
That slot is a supported entry point, so nothing here edits the stub
- each attack now prints the commit's exit code and the hook's output. "The
witness is absent" is only containment if the hook ran and declined, and the
refusal text is what tells those apart
- attack 2 removes its planted file before the verdict, so a failing assertion
cannot leave it inside the install root every later step shares
Record-Id: r-winbound1124
Limit: an unbounded hang is not evidence; it is a job that dies at the runner timeout having printed nothing, so every hook-invoking commit here has to carry its own bound
Ruled-out: Fixing the walk in src/hooks/commit-msg.ts as part of this change | the stub is not this ticket's to own, and the ticket says to record a Windows failure as its own issue rather than absorb it
Ruled-out: Instrumenting the stub to report its internal state | same ownership objection, and the chained-hook slot already reads the same environment without touching it
Ruled-out: Raising the job timeout and letting the hang run to completion | it never completes, and the assertions behind it would never execute
Certainty: firm
Blast: local
Undo: easy
Verified: the walk's non-termination reproduced under /bin/sh for a drive-letter form and terminating in four iterations for the POSIX form; the workflow parses with fourteen steps, four bounded commits, no unbounded `git commit -m`, and no continue-on-error key; the previous run reached step 12 with steps 1-11 all success
Unverified: the hook's actual $PWD form on the runner, which is what the chained probe exists to report; whether the hang is the walk or something earlier in the fall-through; all four steps, none of which has completed on Windows
The bound worked and the step still lost its verdict. `timeout` fired at 90s, and the step ended on that line with exit 124 -- before the case that names the outcome, and before the probe output underneath it. Actions runs a bash step as `bash --noprofile --norc -eo pipefail`. The step's own `set -uo pipefail` sets what it names and clears nothing, so the -e it was invoked with was still in force, and a timeout that fires is a non-zero command. Every bounded commit now runs as `|| rc=$?`, which is a tested command and not an abort. What the run did establish, before it cut itself off, is the whole shape of the Windows question: commitlore.bin = C:\Users\...\commitlore\v9.9.9\dist\commitlore.mjs commitlore.root = C:\Users\...\commitlore\v9.9.9 dirname+pwd -P = /c/Users/.../commitlore/v9.9.9/dist containment case: NO MATCH -- the stub falls through test -x node: yes test -L bin: no command -v commitlore: NOT found (the shim is commitlore.cmd) Both of the stub's guards pass and the comparison that is supposed to tell the installer's own bundle from an attacker's path does not match, because the two sides are written in different path worlds: init records win32, and `pwd -P` under Git for Windows' sh answers in POSIX form. Nothing is exec'd, `commitlore` is not findable by that name because the shim is `.cmd`, and the commit reached the 90s bound rather than returning. The job's cleanup names what was left behind: `Terminate orphan process: pid (7764) (sh)`. That is measured, not inferred. What is still missing is which construct spins, and the chained-hook probe already ran -- its output was simply never printed. Record-Id: r-winshell1124 Limit: a GitHub Actions bash step is invoked as `bash -eo pipefail`, so `set -uo pipefail` inside the step does not clear -e and any bare command that fails ends the step at that line Ruled-out: set +e at the top of each step | it also disables the abort for genuinely unexpected failures, where stopping is the right answer; the || form is scoped to the one command whose failure is a measurement Ruled-out: Reading the outcome from the step's own exit code instead | 124 tells you a bound fired somewhere, not which commit, and it discards the probe output that explains why Certainty: firm Blast: local Undo: easy Verified: on windows-latest at the previous head -- the recorded triple above, the containment case not matching, both stub guards passing, commitlore not resolvable by bare name, and the commit killed at the 90s bound with an orphan sh surviving into cleanup; the workflow parses with fourteen steps, four bounded commits, no bare rc=$? after a bound, and no continue-on-error key Unverified: the probe's report of the hook's own $0/$PWD/pwd -P and the walk's iteration count, which is what the next run exists to print; the three assertions behind the measurement, none of which has executed
…probe
The run answered the ticket's question and, reading its own output back, two of
the three assertions behind it would not have been worth the answer.
The measurement's last line said "the probe never ran: git did not invoke the
hook". The commit hung, so the hook certainly ran; what did not run was the probe.
The stub finds its chained slot with `hook_dir=${0%/*}`, and an argv0 holding no
slash leaves that as `.`. The claim is removed and the measurement now installs
its own commit-msg hook in a scratch repository, which reads Git for Windows
rather than commitlore: argv0, PWD, pwd, pwd -P, uname, the dirname the stub would
compute, and the node_modules walk with a counter on it.
An adversarial review of the assertions found three ways they could report
containment without having tested any:
- The stub consults COMMITLORE_BIN before the recorded path. A value inherited
from the environment would satisfy the baseline with the recorded install never
consulted. Each assertion now unsets it, and checks that core.hooksPath is
unset and that the installed commit-msg hook carries commitlore's marker --
otherwise "the hook refused" can mean "some other hook refused".
- Attack 1's only oracle was an absent witness file, which is equally consistent
with a witness this harness could never observe: a path in the wrong world, an
ESM error, an unwritable directory. It now runs the payload directly first and
requires the witness to appear, so absence afterwards means not-executed.
- Attack 2 planted an extensionless file. Nothing in the stub can exec a recorded
value outside the `*.mjs|*.js` arm, so refusing it tested a branch that does not
exist. The payload is now `commitlore.cmd` -- the extension a Windows fix is
most likely to add, since the installer's own shim carries it -- which makes the
step a regression guard on a loosening somebody would plausibly make. It is not
evidence that containment holds, and the comment says so; attack 1 is.
Also: the planted file is removed by an EXIT trap rather than a trailing rm, so a
failing assertion cannot leave it in the install root every later step shares.
Record-Id: r-winassert1124
Limit: an assertion whose only oracle is an absent side effect cannot distinguish "refused" from "the mechanism never worked", so each one needs a positive control that fires before the attack
Ruled-out: Keeping the extensionless payload for attack 2 | the stub execs a recorded value only inside the .mjs|.js arm, so an extensionless target is refused by a branch that was never going to run it, and the step asserted its own control flow back to itself
Ruled-out: Calling attack 2 evidence of containment once the payload is a .cmd | it guards a loosening, it does not exercise the install-root check; conflating the two is what would let a green sticker land on an untested branch
Ruled-out: Keeping the chained-hook slot as the probe | it was never invoked, and a scratch repo with its own hook measures the platform without depending on the stub's argv0 handling
Ruled-out: A trailing rm for the planted file | it is skipped by every early exit, and the install root is shared by every later step on the runner
Certainty: firm
Blast: local
Undo: easy
Verified: the workflow parses with fourteen steps, five bounded commits, three unset COMMITLORE_BIN, two positive controls and no continue-on-error key; the previous head established on windows-latest that git uses /usr/bin/sh, that init records win32 paths while pwd -P answers in POSIX form, that the containment case does not match, that both stub guards pass, that commitlore is not resolvable by bare name, and that a real commit hangs to the 90s bound
Unverified: the scratch-repo probe and the two positive controls, none of which has run; the three assertions, which the baseline failure has so far kept from executing
The run that proved this job works also showed it lying. With T-1127's fix in front of these steps the baseline passed, both containment attacks executed for the first time and both refused -- and the same log still said: containment case: NO MATCH -- the stub falls through walk: NON-TERMINATING, stuck at [C:] Neither was true any more. Both were restatements: the step retyped the stub's comparison inline and the probe retyped its walk, so once the stub changed they went on reporting the original defect next to a commit that had plainly succeeded. A measurement that paraphrases what it measures reports on the paraphrase, which is the failure this job exists to catch, committed by the job. Both now come from the installed hook. The containment branch is printed as shipped, with the two paths it compares resolved beside it, and the verdict is left to the real commit underneath rather than announced ahead of it. The walk is sliced out of the same file with awk, seeded with the drive-letter PWD the probe reports, and run under a bound -- so a fix made only in a comment cannot pass it, and a walk that does not return fails the step instead of printing a stale verdict. Rebased onto the fix. These steps have never run against a working hook until now, and the head that carries both is what the completion evidence has to cite. Record-Id: r-winderive1124 Limit: a diagnostic that retypes the code it describes goes stale silently, because nothing fails when the two drift apart -- it has to be read out of the artifact that ships Ruled-out: Updating the two hardcoded lines to match the fixed stub | it would be correct today and wrong at the next change, for the same reason it is wrong now Ruled-out: Dropping the two diagnostics and relying on the commit alone | the commit says whether the hook worked, not which branch decided it, and that is the reading the next Windows question starts from Certainty: firm Blast: local Undo: easy Verified: the workflow parses with fourteen steps and no continue-on-error key; neither hardcoded verdict string survives in the file; on the previous head, with the fix in front of these steps, install-ps1 passed with the baseline accepting a valid record and refusing an invalid one, both positive controls firing, and both attacks refused at commit exit 1 Unverified: the two rewritten diagnostics, which have not run; the job at this exact head
MongLong0214
force-pushed
the
feat-issue-283
branch
from
July 31, 2026 15:11
a57cbaf to
282693b
Compare
The containment attacks executed on windows-latest and refused, with the legitimate install working in front of them. That was the whole precondition, and until #321 was fixed it was unreachable: the baseline failed first, so neither attack had ever run. walk: returned real commit: ACCEPTED (exit 0) baseline ok: the hook runs, accepts a valid record and refuses an invalid one positive control ok: the witness fires when the payload runs --- commit exit 1 --- attack 1 refused: the program outside the install root never ran positive control ok: the planted .cmd fires when it is run --- commit exit 1 --- attack 2 refused: the unallowed-extension target never ran The exit codes are the part worth reading. Both attacks end at commit exit 1, which is the hook running and declining -- not the commit failing before it -- and each positive control fires first, so an absent witness means the program did not run rather than a witness this harness could never have seen. A fix that had loosened the match to make the legitimate path work would have passed the baseline and failed exactly here. Two corrections travel with it. The parent-path diagnostic printed nothing, because it was nested three quotings deep inside a command substitution. It is two plain statements now. And the assertion in test/compatibility-matrix.test.ts that pinned this cell away from `supported` was mine, written in T-1122 while this ticket was unproven. It encoded a transient fact as an invariant and made the one cell edit this ticket is authorised to perform impossible to land. What is actually invariant is that the row's status rests on #71's containment being established there, so the citation is what it checks now, in either direction. The ticket's Also-changes clause records the reassignment rather than leaving a silent overreach. Record-Id: r-winsupported1124 Limit: repositories that installed the hook before #321 keep the old stub and must re-run `commitlore hooks install`; a corrected release does not reach them, and this row's claim is about a working install Ruled-out: Flipping the cell on the throwaway verification head | the ticket requires the attacks to pass in a required job in this ticket's own pull request, and a head that exists to be discarded is not that Ruled-out: Leaving the assertion pinned and editing only the document | the two contradict each other, so the suite would fail; a guard that blocks the change it was guarding for is a defect in the guard Ruled-out: Widening this ticket to the hook fix so it could be self-contained | the stop condition sent that to T-1127 for good reason, and this ticket's job was to measure and then say the word once measurement allowed it Certainty: firm Blast: local Undo: easy Verified: install-ps1 green at the previous head of this branch with steps 12-15 all passing on windows-latest -- the walk returning, a real commit accepted, the baseline discriminating both ways, both positive controls firing and both containment attacks refused at commit exit 1; typecheck clean; compatibility-matrix, readme, no-compiled-executable, hooks and hook-target at 138 passed; the workflow parses Unverified: the two corrections in this commit, which have not run on a runner yet
MongLong0214
marked this pull request as ready for review
July 31, 2026 15:22
This was referenced Jul 31, 2026
Merged
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.
For #283 (T-1124). Draft, and stopping here by the ticket's own stop condition. The measurement is complete; the assertions behind it do not pass, and nothing in this PR weakens them or claims Windows support.
What was established, on
windows-latestsh?/usr/bin/sh,MINGW64_NT-10.0-26100. ThespawnSync /bin/sh ENOENTindoctoris the probe's failure, not the hook'sinitrecord the same shape as macOS?.mjsbundle and an absolute interpreter.cmdshim?command -v commitlore: NOT foundThe findings are recorded in full, with the runner output, in #321.
The short version
Both of the containment guard's other conditions pass. Only the comparison fails, and it fails for every value including the installer's own bundle — so the guard neither contains nor permits, and control falls through to a walk that a probe on the same runner measured as
walk: NON-TERMINATING, stuck at [C:].doctor's TypeScript mirror reports no problem in that same state, becausenode:pathispath.win32and handles the pair correctly. It is green exactly when the hook is dead.Why nothing is fixed here
src/hooks/commit-msg.tsis not in T-1124's Owns, and the ticket's stop condition is explicit: if the attacks fail on Windows, stop, do not weaken the job, do not mark Windows supported, and record the failure as its own issue. That is #321.docs/COMPATIBILITY.mddoes not exist ondev(T-1122 / #271 has not merged), so this ticket's one-cell edit has no target. The row is left unflipped and this is the recorded reason — which is also the correct outcome on the evidence: Windows is reachable, not supported.What the job contains
Four steps appended to
install-ps1, nocontinue-on-error,timeout-minutes: 30:sh, whatinitrecorded, the containmentcaserecomputed verbatim, where a fall-through lands, a scratch-repo probe reporting what a hook actually sees on this platform, and a bounded real commit.commitlore.binoutside the install root, with a positive control proving the witness fires before the attack relies on its absence.commitlore.cmdplanted inside the root. Labelled in the source as a regression guard on a loosening a Windows fix would plausibly make, not as evidence that containment holds.Every commit through the hook is bounded, and a timeout is reported as
HUNG— distinct from a refusal — rather than aborting the step.Correction to an earlier commit message
a57cbafexplains the chained-hook probe's non-invocation as an argv0 with no/leavinghook_diras.. The scratch-repo probe disproves that:argv0 = .git/hooks/commit-msganddir-from-argv0 = .git/hooks. The probe's non-invocation has some other cause and is not diagnosed. It is also no longer load-bearing — the measurement no longer uses that slot.