From 49765afee02d81cf20edfee4cacb2017f03f1dd4 Mon Sep 17 00:00:00 2001 From: MongLong0214 Date: Tue, 18 Aug 2026 12:09:26 +0900 Subject: [PATCH] Stop charging every repository for an ordinary upgrade 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 in #749: 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. The `exec` lives in the matching arm only, so 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. 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` writes `bin` as the literal `/current/dist/commitlore.mjs` and `root` as the physical `v` it resolved to. An upgrade moves the installer-owned symlink to a sibling: the recorded string does not change and the new target sits beside the recorded root. #71 replaces the string itself, and a `.git/config` editor can write neither the installer's symlink nor a directory beside its versioned trees. So that arm rebinds the trust root to what `current` resolves to now and runs the same containment check again. Two weaker rules are recorded in the comment as rejected rather than left to be rediscovered: "share a common ancestor" admits `/` and therefore everything, and "follow `current` wherever bin points" is satisfied by a planted `/tmp/current/dist/commitlore.mjs`. The second has a test, so reintroducing it fails rather than passes. 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 Blast: system Undo: easy Certainty: firm Record-Id: r-upgraderebind Provenance: authored Verified: removing the rebind fails the upgrade case and leaves the other three passing, which is the shape a real guard has -- 88 tests across the five hook suites pass with it in place, including #71's witness assertion and the planted-layout case CommitLore-Version: 2.0.0 --- dist/commitlore.mjs | 44 +++++++-- dist/hooks/commit-msg.js | 44 +++++++-- dist/hooks/commit-msg.js.map | 2 +- installer/canonical-artifact.json | 10 +- src/hooks/commit-msg.ts | 44 +++++++-- test/hook-upgrade-rebind.test.ts | 149 ++++++++++++++++++++++++++++++ 6 files changed, 269 insertions(+), 24 deletions(-) create mode 100644 test/hook-upgrade-rebind.test.ts diff --git a/dist/commitlore.mjs b/dist/commitlore.mjs index f48de218..d9d0bb03 100755 --- a/dist/commitlore.mjs +++ b/dist/commitlore.mjs @@ -19076,12 +19076,44 @@ var stubText = (unresolved) => [ ' exec "$recorded_node" "$recorded" validate --message-file "$1"', " ;;", " *)", - " # Resolved and refused without looking at the leaf, which is", - " # what the ending below is careful to claim and no more.", - " # Both sides are already resolved, so these are the physical", - " # paths the comparison actually used rather than what was", - " # recorded -- which is the whole point, since an upgrade is the", - " # difference between the two (#746).", + " # An upgrade and a repointed `commitlore.bin` both land here,", + " # and exactly one of them can be told apart by shape.", + " #", + " # `hooks install` writes `bin` as the literal string", + " # `/current/dist/commitlore.mjs` and `root` as the", + " # physical `v` it resolved to at the time. An upgrade moves", + " # the installer-owned `current` symlink to a sibling `v`:", + " # the recorded string does not change, and the new target is a", + " # sibling of the recorded root. #71 is the opposite shape --", + " # the string itself is replaced with an arbitrary `.js`, and a", + " # `.git/config` editor cannot write the installer-owned", + " # symlink or place a directory beside its versioned trees.", + " #", + " # So the trust root is rebound to what `current` resolves to", + " # now, and the same containment check is applied again. Two", + ' # weaker rules were rejected: "share a common ancestor" admits', + ' # `/` and therefore everything, and "follow `current` wherever', + ' # bin points" is satisfied by a planted', + " # `/tmp/current/dist/commitlore.mjs`.", + " commitlore_rebound=", + ' case "$recorded_slashed" in', + " */current/dist/commitlore.mjs)", + " commitlore_link=${recorded_slashed%/dist/commitlore.mjs}", + ' if [ -L "$commitlore_link" ]; then', + ' commitlore_now=$(cd "$recorded_dir/.." 2>/dev/null && pwd -P) || commitlore_now=', + ' commitlore_rp=$(cd "$root_dir/.." 2>/dev/null && pwd -P) || commitlore_rp=', + ' commitlore_np=$(cd "$commitlore_now/.." 2>/dev/null && pwd -P) || commitlore_np=', + ' if [ -n "$commitlore_now" ] && [ -n "$commitlore_rp" ] && [ "$commitlore_rp" = "$commitlore_np" ]; then', + ' case "$recorded_dir" in', + ' "$commitlore_now"|"$commitlore_now"/*) commitlore_rebound=1 ;;', + " esac", + " fi", + " fi", + " ;;", + " esac", + ' if [ -n "$commitlore_rebound" ]; then', + ' exec "$recorded_node" "$recorded" validate --message-file "$1"', + " fi", " commitlore_outside=$recorded_dir", " commitlore_trusted=$root_dir", " ;;", diff --git a/dist/hooks/commit-msg.js b/dist/hooks/commit-msg.js index f7fdfb09..e423af30 100644 --- a/dist/hooks/commit-msg.js +++ b/dist/hooks/commit-msg.js @@ -250,12 +250,44 @@ const stubText = (unresolved) => [ ' exec "$recorded_node" "$recorded" validate --message-file "$1"', ' ;;', ' *)', - ' # Resolved and refused without looking at the leaf, which is', - ' # what the ending below is careful to claim and no more.', - ' # Both sides are already resolved, so these are the physical', - ' # paths the comparison actually used rather than what was', - ' # recorded -- which is the whole point, since an upgrade is the', - ' # difference between the two (#746).', + ' # An upgrade and a repointed `commitlore.bin` both land here,', + ' # and exactly one of them can be told apart by shape.', + ' #', + ' # `hooks install` writes `bin` as the literal string', + ' # `/current/dist/commitlore.mjs` and `root` as the', + ' # physical `v` it resolved to at the time. An upgrade moves', + ' # the installer-owned `current` symlink to a sibling `v`:', + ' # the recorded string does not change, and the new target is a', + ' # sibling of the recorded root. #71 is the opposite shape --', + ' # the string itself is replaced with an arbitrary `.js`, and a', + ' # `.git/config` editor cannot write the installer-owned', + ' # symlink or place a directory beside its versioned trees.', + ' #', + ' # So the trust root is rebound to what `current` resolves to', + ' # now, and the same containment check is applied again. Two', + ' # weaker rules were rejected: "share a common ancestor" admits', + ' # `/` and therefore everything, and "follow `current` wherever', + ' # bin points" is satisfied by a planted', + ' # `/tmp/current/dist/commitlore.mjs`.', + ' commitlore_rebound=', + ' case "$recorded_slashed" in', + ' */current/dist/commitlore.mjs)', + ' commitlore_link=${recorded_slashed%/dist/commitlore.mjs}', + ' if [ -L "$commitlore_link" ]; then', + ' commitlore_now=$(cd "$recorded_dir/.." 2>/dev/null && pwd -P) || commitlore_now=', + ' commitlore_rp=$(cd "$root_dir/.." 2>/dev/null && pwd -P) || commitlore_rp=', + ' commitlore_np=$(cd "$commitlore_now/.." 2>/dev/null && pwd -P) || commitlore_np=', + ' if [ -n "$commitlore_now" ] && [ -n "$commitlore_rp" ] && [ "$commitlore_rp" = "$commitlore_np" ]; then', + ' case "$recorded_dir" in', + ' "$commitlore_now"|"$commitlore_now"/*) commitlore_rebound=1 ;;', + ' esac', + ' fi', + ' fi', + ' ;;', + ' esac', + ' if [ -n "$commitlore_rebound" ]; then', + ' exec "$recorded_node" "$recorded" validate --message-file "$1"', + ' fi', ' commitlore_outside=$recorded_dir', ' commitlore_trusted=$root_dir', ' ;;', diff --git a/dist/hooks/commit-msg.js.map b/dist/hooks/commit-msg.js.map index 457f3232..814e6876 100644 --- a/dist/hooks/commit-msg.js.map +++ b/dist/hooks/commit-msg.js.map @@ -1 +1 @@ -{"version":3,"file":"commit-msg.js","sourceRoot":"","sources":["../../src/hooks/commit-msg.ts"],"names":[],"mappings":"AAAA;;;;;;;;;;;;GAYG;AAEH;;;GAGG;AACH,MAAM,CAAC,MAAM,WAAW,GAAG,4BAA4B,CAAC;AAExD,kEAAkE;AAClE,MAAM,CAAC,MAAM,cAAc,GAAG,qBAAqB,CAAC;AAEpD,MAAM,CAAC,MAAM,SAAS,GAAG,YAAY,CAAC;AAEtC,MAAM,CAAC,MAAM,iBAAiB,GAAG,GAAG,SAAS,GAAG,cAAc,EAAE,CAAC;AAEjE,MAAM,CAAC,MAAM,SAAS,GAAG,KAAK,CAAC;AAE/B;;;;;;;;GAQG;AACH;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;GAgCG;AACH,MAAM,kBAAkB,GAAG,CAAC,MAAc,EAAE,IAAe,EAAqB,EAAE,CAAC;IACjF,2CAA2C;IAC3C,gDAAgD;IAChD,6EAA6E;IAC7E,uDAAuD;IACvD,uDAAuD;IACvD,oFAAoF;IACpF,oFAAoF;IACpF,wCAAwC;IACxC,QAAQ;IACR,wFAAwF;IACxF,0DAA0D;IAC1D,0DAA0D;IAC1D,mFAAmF;IACnF,+EAA+E;IAC/E,MAAM;IACN,aAAa,MAAM,OAAO;IAC1B,UAAU,IAAI,EAAE;IAChB,IAAI;CACL,CAAC;AAEF,MAAM,eAAe,GAAG;IACtB,GAAG,kBAAkB,CAAC,4CAA4C,EAAE,GAAG,CAAC;IACxE,2EAA2E;IAC3E,cAAc;IACd,0EAA0E;IAC1E,gFAAgF;IAChF,QAAQ;CACA,CAAC;AAEX;;;;;;;;;;GAUG;AACH,MAAM,kBAAkB,GAAG;IACzB,GAAG,kBAAkB,CAAC,mCAAmC,EAAE,GAAG,CAAC;IAC/D,uEAAuE;IACvE,yEAAyE;IACzE,0EAA0E;IAC1E,mGAAmG;IACnG,QAAQ;CACA,CAAC;AAEX;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;GA0CG;AACH,MAAM,QAAQ,GAAG,CAAC,UAA6B,EAAU,EAAE,CACzD;IACE,WAAW;IACX,WAAW;IACX,4CAA4C;IAC5C,+EAA+E;IAC/E,sBAAsB,iBAAiB,wBAAwB;IAC/D,QAAQ;IACR,EAAE;IACF,yEAAyE;IACzE,2EAA2E;IAC3E,6CAA6C;IAC7C,cAAc;IACd,4BAA4B;IAC5B,oBAAoB;IACpB,MAAM;IACN,sBAAsB,iBAAiB,GAAG;IAC1C,EAAE;IACF,uEAAuE;IACvE,+DAA+D;IAC/D,4BAA4B;IAC5B,8BAA8B;IAC9B,IAAI;IACJ,EAAE;IACF,uCAAuC;IACvC,8EAA8E;IAC9E,6EAA6E;IAC7E,KAAK;IACL,wEAAwE;IACxE,4EAA4E;IAC5E,+EAA+E;IAC/E,8EAA8E;IAC9E,8EAA8E;IAC9E,+CAA+C;IAC/C,6BAA6B;IAC7B,iBAAiB;IACjB,yCAAyC;IACzC,6DAA6D;IAC7D,UAAU;IACV,UAAU;IACV,QAAQ;IACR,IAAI;IACJ,EAAE;IACF,0EAA0E;IAC1E,6EAA6E;IAC7E,8EAA8E;IAC9E,0EAA0E;IAC1E,4EAA4E;IAC5E,qBAAqB;IACrB,GAAG;IACH,+EAA+E;IAC/E,wEAAwE;IACxE,4DAA4D;IAC5D,8EAA8E;IAC9E,+EAA+E;IAC/E,gFAAgF;IAChF,gFAAgF;IAChF,yEAAyE;IACzE,6BAA6B;IAC7B,2EAA2E;IAC3E,qDAAqD;IACrD,iFAAiF;IACjF,uBAAuB;IACvB,iBAAiB;IACjB,4EAA4E;IAC5E,4EAA4E;IAC5E,+DAA+D;IAC/D,qFAAqF;IACrF,8EAA8E;IAC9E,8EAA8E;IAC9E,+EAA+E;IAC/E,gFAAgF;IAChF,4EAA4E;IAC5E,0DAA0D;IAC1D,2FAA2F;IAC3F,+EAA+E;IAC/E,4EAA4E;IAC5E,4EAA4E;IAC5E,+EAA+E;IAC/E,8EAA8E;IAC9E,WAAW;IACX,2EAA2E;IAC3E,8EAA8E;IAC9E,6EAA6E;IAC7E,mEAAmE;IACnE,oEAAoE;IACpE,qCAAqC;IACrC,0DAA0D;IAC1D,mCAAmC;IACnC,cAAc;IACd,sFAAsF;IACtF,2EAA2E;IAC3E,+DAA+D;IAC/D,mCAAmC;IACnC,wCAAwC;IACxC,8EAA8E;IAC9E,kBAAkB;IAClB,gBAAgB;IAChB,4EAA4E;IAC5E,wEAAwE;IACxE,4EAA4E;IAC5E,yEAAyE;IACzE,+EAA+E;IAC/E,oDAAoD;IACpD,gDAAgD;IAChD,4CAA4C;IAC5C,kBAAkB;IAClB,gBAAgB;IAChB,cAAc;IACd,6EAA6E;IAC7E,8EAA8E;IAC9E,6EAA6E;IAC7E,+EAA+E;IAC/E,2EAA2E;IAC3E,aAAa;IACb,0EAA0E;IAC1E,oDAAoD;IACpD,mCAAmC;IACnC,wCAAwC;IACxC,6CAA6C;IAC7C,YAAY;IACZ,UAAU;IACV,UAAU;IACV,QAAQ;IACR,IAAI;IACJ,EAAE;IACF,gDAAgD;IAChD,gDAAgD;IAChD,IAAI;IACJ,EAAE;IACF,6EAA6E;IAC7E,mDAAmD;IACnD,GAAG;IACH,6EAA6E;IAC7E,0EAA0E;IAC1E,8EAA8E;IAC9E,4EAA4E;IAC5E,+CAA+C;IAC/C,UAAU;IACV,yBAAyB;IACzB,uDAAuD;IACvD,2EAA2E;IAC3E,MAAM;IACN,oBAAoB;IACpB,mCAAmC;IACnC,WAAW;IACX,MAAM;IACN,eAAe;IACf,MAAM;IACN,EAAE;IACF,GAAG,UAAU;IACb,EAAE;CACH,CAAC,IAAI,CAAC,IAAI,CAAC,CAAC;AAEf,0DAA0D;AAC1D,MAAM,CAAC,MAAM,aAAa,GAAG,GAAW,EAAE,CAAC,QAAQ,CAAC,eAAe,CAAC,CAAC;AAErE;;;;GAIG;AACH,MAAM,CAAC,MAAM,eAAe,GAAG,GAAW,EAAE,CAAC,QAAQ,CAAC,kBAAkB,CAAC,CAAC"} \ No newline at end of file +{"version":3,"file":"commit-msg.js","sourceRoot":"","sources":["../../src/hooks/commit-msg.ts"],"names":[],"mappings":"AAAA;;;;;;;;;;;;GAYG;AAEH;;;GAGG;AACH,MAAM,CAAC,MAAM,WAAW,GAAG,4BAA4B,CAAC;AAExD,kEAAkE;AAClE,MAAM,CAAC,MAAM,cAAc,GAAG,qBAAqB,CAAC;AAEpD,MAAM,CAAC,MAAM,SAAS,GAAG,YAAY,CAAC;AAEtC,MAAM,CAAC,MAAM,iBAAiB,GAAG,GAAG,SAAS,GAAG,cAAc,EAAE,CAAC;AAEjE,MAAM,CAAC,MAAM,SAAS,GAAG,KAAK,CAAC;AAE/B;;;;;;;;GAQG;AACH;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;GAgCG;AACH,MAAM,kBAAkB,GAAG,CAAC,MAAc,EAAE,IAAe,EAAqB,EAAE,CAAC;IACjF,2CAA2C;IAC3C,gDAAgD;IAChD,6EAA6E;IAC7E,uDAAuD;IACvD,uDAAuD;IACvD,oFAAoF;IACpF,oFAAoF;IACpF,wCAAwC;IACxC,QAAQ;IACR,wFAAwF;IACxF,0DAA0D;IAC1D,0DAA0D;IAC1D,mFAAmF;IACnF,+EAA+E;IAC/E,MAAM;IACN,aAAa,MAAM,OAAO;IAC1B,UAAU,IAAI,EAAE;IAChB,IAAI;CACL,CAAC;AAEF,MAAM,eAAe,GAAG;IACtB,GAAG,kBAAkB,CAAC,4CAA4C,EAAE,GAAG,CAAC;IACxE,2EAA2E;IAC3E,cAAc;IACd,0EAA0E;IAC1E,gFAAgF;IAChF,QAAQ;CACA,CAAC;AAEX;;;;;;;;;;GAUG;AACH,MAAM,kBAAkB,GAAG;IACzB,GAAG,kBAAkB,CAAC,mCAAmC,EAAE,GAAG,CAAC;IAC/D,uEAAuE;IACvE,yEAAyE;IACzE,0EAA0E;IAC1E,mGAAmG;IACnG,QAAQ;CACA,CAAC;AAEX;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;GA0CG;AACH,MAAM,QAAQ,GAAG,CAAC,UAA6B,EAAU,EAAE,CACzD;IACE,WAAW;IACX,WAAW;IACX,4CAA4C;IAC5C,+EAA+E;IAC/E,sBAAsB,iBAAiB,wBAAwB;IAC/D,QAAQ;IACR,EAAE;IACF,yEAAyE;IACzE,2EAA2E;IAC3E,6CAA6C;IAC7C,cAAc;IACd,4BAA4B;IAC5B,oBAAoB;IACpB,MAAM;IACN,sBAAsB,iBAAiB,GAAG;IAC1C,EAAE;IACF,uEAAuE;IACvE,+DAA+D;IAC/D,4BAA4B;IAC5B,8BAA8B;IAC9B,IAAI;IACJ,EAAE;IACF,uCAAuC;IACvC,8EAA8E;IAC9E,6EAA6E;IAC7E,KAAK;IACL,wEAAwE;IACxE,4EAA4E;IAC5E,+EAA+E;IAC/E,8EAA8E;IAC9E,8EAA8E;IAC9E,+CAA+C;IAC/C,6BAA6B;IAC7B,iBAAiB;IACjB,yCAAyC;IACzC,6DAA6D;IAC7D,UAAU;IACV,UAAU;IACV,QAAQ;IACR,IAAI;IACJ,EAAE;IACF,0EAA0E;IAC1E,6EAA6E;IAC7E,8EAA8E;IAC9E,0EAA0E;IAC1E,4EAA4E;IAC5E,qBAAqB;IACrB,GAAG;IACH,+EAA+E;IAC/E,wEAAwE;IACxE,4DAA4D;IAC5D,8EAA8E;IAC9E,+EAA+E;IAC/E,gFAAgF;IAChF,gFAAgF;IAChF,yEAAyE;IACzE,6BAA6B;IAC7B,2EAA2E;IAC3E,qDAAqD;IACrD,iFAAiF;IACjF,uBAAuB;IACvB,iBAAiB;IACjB,4EAA4E;IAC5E,4EAA4E;IAC5E,+DAA+D;IAC/D,qFAAqF;IACrF,8EAA8E;IAC9E,8EAA8E;IAC9E,+EAA+E;IAC/E,gFAAgF;IAChF,4EAA4E;IAC5E,0DAA0D;IAC1D,2FAA2F;IAC3F,+EAA+E;IAC/E,4EAA4E;IAC5E,4EAA4E;IAC5E,+EAA+E;IAC/E,8EAA8E;IAC9E,WAAW;IACX,2EAA2E;IAC3E,8EAA8E;IAC9E,6EAA6E;IAC7E,mEAAmE;IACnE,oEAAoE;IACpE,qCAAqC;IACrC,0DAA0D;IAC1D,mCAAmC;IACnC,cAAc;IACd,sFAAsF;IACtF,2EAA2E;IAC3E,+DAA+D;IAC/D,mCAAmC;IACnC,wCAAwC;IACxC,8EAA8E;IAC9E,kBAAkB;IAClB,gBAAgB;IAChB,6EAA6E;IAC7E,qEAAqE;IACrE,iBAAiB;IACjB,oEAAoE;IACpE,6EAA6E;IAC7E,8EAA8E;IAC9E,4EAA4E;IAC5E,8EAA8E;IAC9E,4EAA4E;IAC5E,8EAA8E;IAC9E,uEAAuE;IACvE,0EAA0E;IAC1E,iBAAiB;IACjB,4EAA4E;IAC5E,2EAA2E;IAC3E,8EAA8E;IAC9E,8EAA8E;IAC9E,uDAAuD;IACvD,qDAAqD;IACrD,mCAAmC;IACnC,2CAA2C;IAC3C,gDAAgD;IAChD,4EAA4E;IAC5E,sDAAsD;IACtD,sGAAsG;IACtG,gGAAgG;IAChG,sGAAsG;IACtG,6HAA6H;IAC7H,+CAA+C;IAC/C,wFAAwF;IACxF,4BAA4B;IAC5B,wBAAwB;IACxB,sBAAsB;IACtB,sBAAsB;IACtB,oBAAoB;IACpB,qDAAqD;IACrD,gFAAgF;IAChF,kBAAkB;IAClB,gDAAgD;IAChD,4CAA4C;IAC5C,kBAAkB;IAClB,gBAAgB;IAChB,cAAc;IACd,6EAA6E;IAC7E,8EAA8E;IAC9E,6EAA6E;IAC7E,+EAA+E;IAC/E,2EAA2E;IAC3E,aAAa;IACb,0EAA0E;IAC1E,oDAAoD;IACpD,mCAAmC;IACnC,wCAAwC;IACxC,6CAA6C;IAC7C,YAAY;IACZ,UAAU;IACV,UAAU;IACV,QAAQ;IACR,IAAI;IACJ,EAAE;IACF,gDAAgD;IAChD,gDAAgD;IAChD,IAAI;IACJ,EAAE;IACF,6EAA6E;IAC7E,mDAAmD;IACnD,GAAG;IACH,6EAA6E;IAC7E,0EAA0E;IAC1E,8EAA8E;IAC9E,4EAA4E;IAC5E,+CAA+C;IAC/C,UAAU;IACV,yBAAyB;IACzB,uDAAuD;IACvD,2EAA2E;IAC3E,MAAM;IACN,oBAAoB;IACpB,mCAAmC;IACnC,WAAW;IACX,MAAM;IACN,eAAe;IACf,MAAM;IACN,EAAE;IACF,GAAG,UAAU;IACb,EAAE;CACH,CAAC,IAAI,CAAC,IAAI,CAAC,CAAC;AAEf,0DAA0D;AAC1D,MAAM,CAAC,MAAM,aAAa,GAAG,GAAW,EAAE,CAAC,QAAQ,CAAC,eAAe,CAAC,CAAC;AAErE;;;;GAIG;AACH,MAAM,CAAC,MAAM,eAAe,GAAG,GAAW,EAAE,CAAC,QAAQ,CAAC,kBAAkB,CAAC,CAAC"} \ No newline at end of file diff --git a/installer/canonical-artifact.json b/installer/canonical-artifact.json index 6c137348..b755a4c3 100644 --- a/installer/canonical-artifact.json +++ b/installer/canonical-artifact.json @@ -15,10 +15,10 @@ "tsconfig.json", "src" ], - "sha256": "c2a9710550e0fa33bfb97945d73d6743a9d42234d6b38c56da9d3644bd1aa5b8" + "sha256": "8a3c589cf02a26d841a91d6ea9669492466753c9673e8e61c058bb3248e4c875" }, "artifact": { - "sha256": "b9e7110c64788123ec3b20ad93cd6fb97af35c7523c4c1275060e193a1849e45", + "sha256": "c8475b5c41228ac4f7cfdeea090b19541338ca944111dc9d9918b516e09a93ec", "files": [ { "path": "dist/cli.d.ts", @@ -598,7 +598,7 @@ }, { "path": "dist/commitlore.mjs", - "sha256": "47fff7597a38bb2188943debd59bf90bdc8e80060a7d4acd21590b2a64b6a236" + "sha256": "f3af51a7479467128c562874012fd3183f7393f6cd8aed045896e737f8239646" }, { "path": "dist/core/agent-configs.d.ts", @@ -1074,11 +1074,11 @@ }, { "path": "dist/hooks/commit-msg.js", - "sha256": "ad314ead4a0242a10fb052e018b2755cb5f61bae7b2e9aa2c677a138b4ce3d9b" + "sha256": "b2260805e270db23bb0f7c886e41bcb5862ff0f44fea61f7d049a9d37dbcb36b" }, { "path": "dist/hooks/commit-msg.js.map", - "sha256": "c6a71203f9f80305d9b42f576eee18e2d7c4bc65b7ae4585e540295148612f29" + "sha256": "1a341e4f3d17e02498c662be004b4215c16ccd13a7aad574d800c466d2be4f36" }, { "path": "dist/hooks/post-commit.d.ts", diff --git a/src/hooks/commit-msg.ts b/src/hooks/commit-msg.ts index d9faaaba..b777aaff 100644 --- a/src/hooks/commit-msg.ts +++ b/src/hooks/commit-msg.ts @@ -260,12 +260,44 @@ const stubText = (unresolved: readonly string[]): string => ' exec "$recorded_node" "$recorded" validate --message-file "$1"', ' ;;', ' *)', - ' # Resolved and refused without looking at the leaf, which is', - ' # what the ending below is careful to claim and no more.', - ' # Both sides are already resolved, so these are the physical', - ' # paths the comparison actually used rather than what was', - ' # recorded -- which is the whole point, since an upgrade is the', - ' # difference between the two (#746).', + ' # An upgrade and a repointed `commitlore.bin` both land here,', + ' # and exactly one of them can be told apart by shape.', + ' #', + ' # `hooks install` writes `bin` as the literal string', + ' # `/current/dist/commitlore.mjs` and `root` as the', + ' # physical `v` it resolved to at the time. An upgrade moves', + ' # the installer-owned `current` symlink to a sibling `v`:', + ' # the recorded string does not change, and the new target is a', + ' # sibling of the recorded root. #71 is the opposite shape --', + ' # the string itself is replaced with an arbitrary `.js`, and a', + ' # `.git/config` editor cannot write the installer-owned', + ' # symlink or place a directory beside its versioned trees.', + ' #', + ' # So the trust root is rebound to what `current` resolves to', + ' # now, and the same containment check is applied again. Two', + ' # weaker rules were rejected: "share a common ancestor" admits', + ' # `/` and therefore everything, and "follow `current` wherever', + ' # bin points" is satisfied by a planted', + ' # `/tmp/current/dist/commitlore.mjs`.', + ' commitlore_rebound=', + ' case "$recorded_slashed" in', + ' */current/dist/commitlore.mjs)', + ' commitlore_link=${recorded_slashed%/dist/commitlore.mjs}', + ' if [ -L "$commitlore_link" ]; then', + ' commitlore_now=$(cd "$recorded_dir/.." 2>/dev/null && pwd -P) || commitlore_now=', + ' commitlore_rp=$(cd "$root_dir/.." 2>/dev/null && pwd -P) || commitlore_rp=', + ' commitlore_np=$(cd "$commitlore_now/.." 2>/dev/null && pwd -P) || commitlore_np=', + ' if [ -n "$commitlore_now" ] && [ -n "$commitlore_rp" ] && [ "$commitlore_rp" = "$commitlore_np" ]; then', + ' case "$recorded_dir" in', + ' "$commitlore_now"|"$commitlore_now"/*) commitlore_rebound=1 ;;', + ' esac', + ' fi', + ' fi', + ' ;;', + ' esac', + ' if [ -n "$commitlore_rebound" ]; then', + ' exec "$recorded_node" "$recorded" validate --message-file "$1"', + ' fi', ' commitlore_outside=$recorded_dir', ' commitlore_trusted=$root_dir', ' ;;', diff --git a/test/hook-upgrade-rebind.test.ts b/test/hook-upgrade-rebind.test.ts new file mode 100644 index 00000000..5abb9d14 --- /dev/null +++ b/test/hook-upgrade-rebind.test.ts @@ -0,0 +1,149 @@ +/** + * #749: after an upgrade, a repository wired before it went on refusing commits + * under the PATH git actually gives a hook. + * + * What held the security property was never the exit code. The `exec` lives in + * the matching arm only, so 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. The refusal + * at the end is the ending for "no CLI could be resolved anywhere", and reusing + * it for "`current` moved and `root` did not" charged every already-wired + * repository for an ordinary upgrade. + * + * An upgrade is distinguishable by shape. `hooks install` writes `bin` as the + * literal `/current/dist/commitlore.mjs` and `root` as the physical + * `v` it resolved to; an upgrade moves the installer-owned symlink to a + * sibling, leaving the recorded string untouched. #71 is the opposite: the + * string itself is replaced, and a `.git/config` editor can write neither the + * installer's symlink nor a directory beside its versioned trees. + * + * These build that layout rather than describing it, because the property is in + * shell text that only git runs. + */ + +import { execFileSync } from 'node:child_process'; +import { existsSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync, copyFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { afterAll, describe, expect, it } from 'vitest'; + +import { execGit } from '../src/core/git.js'; +import { createTestRepo } from './git-fixtures.js'; + +const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); +const BUNDLE = join(REPO_ROOT, 'dist', 'commitlore.mjs'); +const VALID = 'feat: a change\n\nLimit: something true\nRecord-Id: r-reb001\nProvenance: authored\n'; + +const scratch: string[] = []; +afterAll(() => { + for (const dir of scratch) rmSync(dir, { recursive: true, force: true }); +}); + +const temp = (label: string): string => { + const dir = mkdtempSync(join(tmpdir(), `commitlore-${label}-`)); + scratch.push(dir); + return dir; +}; + +/** A `` shaped the way `install.sh` shapes one. */ +const dataRoot = (versions: string[]): string => { + const root = temp('rebind-data'); + for (const v of versions) { + mkdirSync(join(root, v, 'dist'), { recursive: true }); + copyFileSync(BUNDLE, join(root, v, 'dist', 'commitlore.mjs')); + writeFileSync( + join(root, v, 'package.json'), + `${JSON.stringify({ name: 'commitlore', version: v.slice(1), type: 'module' })}\n`, + ); + symlinkSync(join(REPO_ROOT, 'spec'), join(root, v, 'spec'), 'dir'); + } + symlinkSync(join(root, versions[0]!), join(root, 'current'), 'dir'); + return root; +}; + +const wired = (label: string, root: string, pinned: string): string => { + const dir = createTestRepo({ path: temp(label) }); + execGit(['config', 'user.email', `${label}@example.invalid`], { cwd: dir }); + execGit(['config', 'user.name', label], { cwd: dir }); + execFileSync(process.execPath, [BUNDLE, 'hooks', 'install'], { cwd: dir, shell: false }); + // Rewritten to the installed layout: the source checkout this suite runs from + // is not one, and the property under test is about that layout. + execGit(['config', '--local', 'commitlore.bin', join(root, 'current', 'dist', 'commitlore.mjs')], { cwd: dir }); + execGit(['config', '--local', 'commitlore.root', join(root, pinned)], { cwd: dir }); + return dir; +}; + +const runHook = (cwd: string): { code: number; stderr: string } => { + const messageFile = join(cwd, 'MESSAGE'); + writeFileSync(messageFile, VALID); + try { + execFileSync('sh', [join(cwd, '.git', 'hooks', 'commit-msg'), messageFile], { + cwd, + shell: false, + encoding: 'utf8', + env: { HOME: process.env.HOME ?? '', PATH: '/usr/bin:/bin' }, + }); + return { code: 0, stderr: '' }; + } catch (error) { + const failure = error as { status?: number; stderr?: string }; + return { code: failure.status ?? 1, stderr: failure.stderr ?? '' }; + } +}; + +describe('#749 an upgrade stops charging repositories wired before it', () => { + it('runs when current and the recorded root are the same tree — the control', () => { + const root = dataRoot(['v1.0.0']); + const result = runHook(wired('reb-control', root, 'v1.0.0')); + expect(result.stderr, `the control could not run at all: ${result.stderr}`).not.toMatch( + /cannot find the CLI|points outside/, + ); + }, 60_000); + + it('runs after current moved to a sibling, with the root left behind', () => { + // The whole point. Before this, the recorded pair stopped matching and the + // commit was refused under a restricted PATH until somebody re-ran + // `hooks install` in that repository. + const root = dataRoot(['v1.0.0', 'v1.1.0']); + const dir = wired('reb-upgrade', root, 'v1.0.0'); + rmSync(join(root, 'current')); + symlinkSync(join(root, 'v1.1.0'), join(root, 'current'), 'dir'); + + const result = runHook(dir); + expect(result.stderr).not.toMatch(/points outside the install/); + expect(result.stderr).not.toMatch(/cannot find the CLI/); + }, 60_000); + + it('still refuses a recorded path outside the install, and does not run it', () => { + // #71, unchanged. The rebind is bound to the string `hooks install` writes + // and to the installer's layout; a planted path is neither. + const root = dataRoot(['v1.0.0']); + const dir = wired('reb-attack', root, 'v1.0.0'); + const outside = temp('reb-outside'); + const witness = join(outside, 'ran.log'); + const evil = join(outside, 'evil.js'); + writeFileSync(evil, `import { writeFileSync } from 'node:fs';\nwriteFileSync(${JSON.stringify(witness)}, 'ran');\n`); + execGit(['config', '--local', 'commitlore.bin', evil], { cwd: dir }); + + const result = runHook(dir); + expect(result.stderr).toMatch(/points outside the install/); + expect(existsSync(witness), 'the planted file was executed').toBe(false); + }, 60_000); + + it('refuses a planted directory that imitates the layout', () => { + // The rejected weaker rule, kept as a test so it cannot be reintroduced: + // "follow `current` wherever bin points" is satisfied by a `current` + // somebody else created. The recorded root has to be a sibling of what the + // installer's `current` resolves to. + const root = dataRoot(['v1.0.0']); + const dir = wired('reb-imitate', root, 'v1.0.0'); + const fake = temp('reb-fake'); + mkdirSync(join(fake, 'v9.9.9', 'dist'), { recursive: true }); + copyFileSync(BUNDLE, join(fake, 'v9.9.9', 'dist', 'commitlore.mjs')); + symlinkSync(join(fake, 'v9.9.9'), join(fake, 'current'), 'dir'); + execGit(['config', '--local', 'commitlore.bin', join(fake, 'current', 'dist', 'commitlore.mjs')], { cwd: dir }); + + expect(runHook(dir).stderr).toMatch(/points outside the install/); + }, 60_000); +});