Stop charging every repository for an ordinary upgrade (#749)

[MongLong0214]

Relates to #749 — it does not close it. See the last section.

After an upgrade, a repository wired before it refused commits under the PATH git actually gives a hook — a GUI client, an IDE, a launcher — until somebody re-ran hooks install there. #746 made that refusal say what it was; this stops it happening.

I had this backwards, and said so on the issue

I wrote that relaxing the exit code was a fail-closed policy change I should not make alone. The exit code was never what held the property.

src/hooks/commit-msg.ts
  :260   exec "$recorded_node" ...      ← inside the MATCHING arm only
  :269   commitlore_outside=...         ← the refusing arm sets a variable and falls through
  :291   command -v commitlore          ← then the PATH fallback

A path the containment check refuses is already not executed by the time anything decides what to print. test/hooks.test.ts asserts that directly, with a witness file that must not appear — and it only sees exit 1 at all because it passes a PATH with no commitlore on it.

So the refusal at the end is the ending for "no CLI could be resolved anywhere". Reusing it for "current moved and root did not" was a tax, not a fence.

An upgrade is distinguishable by shape — and #71 is the opposite shape

hooks install   bin  = "<data-root>/current/dist/commitlore.mjs"   literal string
                root = realpath(PACKAGE_ROOT)                       physical v<x>

upgrade         the installer-owned symlink moves to a sibling
                the recorded string is unchanged; the new target sits beside root

#71             the recorded string itself is replaced with an arbitrary .js
                root and node stay as installed

A .git/config editor can write neither the installer's symlink nor a directory beside its versioned trees. So the arm rebinds the trust root to what current resolves to now and runs the same containment check again.

Two weaker rules are recorded as rejected, in the comment and in a test:

rejected rule	why
share a common ancestor	/ is a common ancestor, so it admits everything
follow current wherever bin points	satisfied by a planted /tmp/current/dist/commitlore.mjs

The second has its own case, so reintroducing it fails rather than passes.

Verification

test/hook-upgrade-rebind.test.ts builds the <data-root> layout rather than describing it, because the property is in shell text only git runs.

case	result
current and root are the same tree	runs — the control
current moved to a sibling, root left behind	runs — this is the fix
a planted .js outside the install	refused, and the witness file is not created
a planted directory imitating the layout	refused

Negative control: removing the rebind fails the upgrade case and leaves the other three passing. 88 tests across the five hook suites pass with it in place.

Limit

This reaches a repository only when its stub already carries the arm, so one installed before this still needs hooks install once — the same boundary #746 recorded, and the reason that command stays the named remedy.

Why this does not close #749

#749 asked two things. This answers the second — the refusal was a tax rather than a fence, and the tax is gone.

The first is still true after this ships. The rebind lives in the stub, and the stub is written into .git/hooks at install time. A repository wired before this release keeps the old text, so the upgrade that delivers this fix is not fixed by it. Every affected repository still needs commitlore hooks install run in it once.

That is question 1 — "the repair cannot reach them" — and closing this issue would hide it behind a fix that does not apply to the machines that have the problem today.

Two corrections to what this PR claimed

"Does not widen the attack surface" was imprecise, and the precise version is more useful.

somebody who can write .git/config     surface unchanged — measured, and #71's witness still not created
somebody who can write <data-root>     was already trusted, before this change

The second was checked on origin/main, without the rebind: overwriting dist/commitlore.mjs inside the recorded root executes the planted code, because that root is exactly what the check trusts. So the rebind does not hand data-root write a capability it lacked — it extends an existing trust to a diverged state.

A TOCTOU window exists, and it is not this change's.

the check      resolves with `pwd -P` and compares physical paths
the exec       exec "$recorded_node" "$recorded"    ← the unresolved recorded string
                                                      `current` is followed again here

Both exec sites have it — :260, the pre-existing matching arm, and :299, the rebind. So it is duplicated rather than introduced, and it is recorded here because it now appears twice and would otherwise be found later as a regression of this PR.

[github-actions]

CommitLore — record lint

Trailers: clean — 2 commits in origin/main..a9f0906a2a43d172d4f59b6820e81d178d717b62
Active constraints: not read — commitlore: git log --follow accepts exactly one pathspec, so renames are not followed for 6 paths; query one path at a time to follow its rename chain (6 changed paths)

_{Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR.}

[MongLong0214]

Known residual, recorded rather than discovered later

One arm this does not reach, and should not: deleting the old release directory after upgrading.

current → v1.1.0, v1.0.0 still present   exit 0   "shape ok · references ok"   ← fixed here
current → v1.1.0, v1.0.0 deleted         exit 1   "One of those two does not exist,
                                                    so the trust check could not run"

That is the commitlore_unresolved arm, and the same rule cannot fix it. The sibling test's anchor is the recorded root; when that root is gone there is nothing to be a sibling of, and rebinding without an anchor is a rule anybody can satisfy. Refusing is correct there, and #746 already made the message say which of the two failed.

install.sh does not remove old trees — a real data root here holds v0.8.0 through v1.1.1 alongside current — so reaching that arm takes a deliberate manual delete. This change covers the ordinary upgrade path.

Where the sibling check actually bears

Worth writing down, because only one axis of the rule is unforgeable:

1  pattern match    */current/dist/commitlore.mjs      an attacker can satisfy this
2  [ -L <link> ]    a symlink they created             an attacker can satisfy this
3  parent(root) == parent(realpath(current))           ← this is the one that refuses

A .git/config editor can build 1 and 2 in /tmp. What they cannot do is make their tree a sibling of the root the installer recorded, which is why the planted-layout case is a test rather than a comment.

Forging the whole data root — bin and root both under one attacker-owned directory — passes, and passes today as well: bin=/tmp/x/v1/dist/… with root=/tmp/x/v1 satisfies the existing prefix check unchanged. This does not widen that door.