fix(scripts): row 211 R3 push guard tags, installer hooks dir, busy check errors (#1709)

scripts/git-hooks/pre-push

@@ -3,8 +3,9 @@
 # .git/hooks by `scripts/install-git-hooks.sh`; never edit the installed copy.
 # For each ref git is about to push, it refuses a delete and any update whose
 # remote tip isn't an ancestor of the local tip: a force-push or a +refspec.
-# A new ref passes. It fetches nothing, so a remote tip that isn't in this
-# repository refuses. Any failure of the check refuses too.
+# It refuses any change to an existing tag, and an object name that isn't 40
+# or 64 hex characters. A new ref passes. It fetches nothing, so a remote tip
+# that isn't in this repository refuses. Any failure of the check refuses too.
 # `git push --no-verify` skips this hook; Gitea's branch protection is the
 # guard that holds (ops-01). Canon: never force-push.
 refused=0
@@ -16,6 +17,9 @@ zero() {
     case "$1" in *[!0]*) return 1 ;; esac
     return 0
 }
+idlen() { # $1: an object name. True at 40 (SHA-1) or 64 (SHA-256) characters.
+    [ "${#1}" -eq 40 ] || [ "${#1}" -eq 64 ]
+}
 while read -r lref lsha rref rsha extra || [ -n "${lref:-}" ]; do
     if [ -z "${rsha:-}" ] || [ -n "${extra:-}" ]; then
         refuse "unreadable ref line from git: $lref ${lsha:-} ${rref:-} ${rsha:-} ${extra:-}"
@@ -24,6 +28,10 @@ while read -r lref lsha rref rsha extra || [ -n "${lref:-}" ]; do
     case "$lsha$rsha" in
         *[!0-9a-f]*) refuse "$rref: a ref line with a non-hex object name (local $lsha, remote $rsha)"; continue ;;
     esac
+    if ! idlen "$lsha" || ! idlen "$rsha"; then
+        refuse "$rref: a ref line with an object name that isn't 40 or 64 hex characters (local $lsha, remote $rsha)"
+        continue
+    fi
     if zero "$lsha"; then
         refuse "$rref: a delete (remote $rsha); seat branches are never deleted by push"
         continue
@@ -31,6 +39,12 @@ while read -r lref lsha rref rsha extra || [ -n "${lref:-}" ]; do
     if zero "$rsha"; then
         continue
     fi
+    # An existing tag never changes. A new tag object on the same commit, or
+    # a tag moved to a descendant, would pass the ancestor check below.
+    if [ "${rref#refs/tags/}" != "$rref" ] && [ "$lsha" != "$rsha" ]; then
+        refuse "$rref: an existing tag can't be moved or replaced (remote $rsha, local $lsha); push a new tag instead"
+        continue
+    fi
     if ! git cat-file -e "$rsha^{commit}" 2>/dev/null; then
         refuse "$rref: the remote tip $rsha isn't in this repository; fetch first, then push again"
         continue

scripts/install-git-hooks.sh

@@ -7,8 +7,10 @@
 #
 # DIR defaults to this script's own checkout. The hook's bytes are this
 # checkout's scripts/git-hooks/pre-push. It refuses when core.hooksPath is set
-# (git would not run the hook), and when a different pre-push is installed,
-# unless --force-install is given. It never edits an installed hook in place:
+# (git would not run the hook), when the hooks directory is a symlink or
+# resolves anywhere but the common directory's hooks, and when a different
+# pre-push is installed, unless --force-install is given. It never edits an
+# installed hook in place:
 # a new hook is linked in, a replacement is renamed over the old one. A canary
 # then runs the installed hook through git and needs a delete refused.
 #
@@ -55,8 +57,20 @@ HOOK="$HOOKS/pre-push"
 out=$(g config --show-scope --get-all core.hooksPath 2>/dev/null)
 [ -z "$out" ] || die 2 "refused: core.hooksPath is set ($(printf '%s' "$out" | tr '\n\t' '; ')), so git would not run the push guard; unset it first"
 
+# The hooks directory must be the common directory's own. Through a symlink
+# to a shared directory, the install would set the hook of every repository
+# that shares it. Checked before and after mkdir, before any file is written.
+own_hooks() {
+  local real want
+  [ ! -L "$HOOKS" ] || die 2 "refused: $HOOKS is a symlink; install only into the repository's own hooks directory"
+  [ -e "$HOOKS" ] || return 0
+  real=$(cd -P -- "$HOOKS" && pwd -P) && want="$(cd -P -- "$common" && pwd -P)/hooks" || die 1 "cannot resolve $HOOKS"
+  [ "$real" = "$want" ] || die 2 "refused: $HOOKS resolves to $real, not $want"
+}
+own_hooks
 if [ -L "$HOOK" ]; then die 2 "refused: $HOOK is a symlink; remove it by hand if it is not wanted"; fi
 mkdir -p "$HOOKS" || die 1 "cannot create $HOOKS"
+own_hooks
 tmp="$HOOKS/.pre-push.install-git-hooks.$$"
 if [ -e "$HOOK" ]; then
   [ -f "$HOOK" ] || die 2 "refused: $HOOK is not a regular file"

scripts/slot-busy.sh

@@ -6,17 +6,21 @@
 #   scripts/slot-busy.sh [--proc-root DIR]
 #
 # It prints one BUSY line per matching process, or one CLEAR line. Exit codes:
-# 0 CLEAR, 1 BUSY, 2 its own error. Chain it on the exit code, never on a
-# filter of its output:
+# 0 CLEAR, 1 BUSY, 2 its own error, which includes a process it can't read
+# that hasn't exited (it still prints any BUSY lines, never CLEAR). Chain it
+# on the exit code, never on a filter of its output:
 #   bash scripts/slot-busy.sh > LOG && gate          (set -o pipefail if piped)
 # never `bash scripts/slot-busy.sh | tail -1 && gate`, which runs while BUSY.
 #
 # A process matches when its command line holds `node ... --test`, a
 # `*gate*.sh`, `mutant`, a `test-*.sh` or `docker compose`/`docker-compose`.
 # It skips itself and its ancestors (a gate script may run the check), and an
-# agent harness's shell-snapshot wrapper, whose command line is the agent's
-# command text; that shell's real children are still matched. It reads only
-# /proc/<pid>/cmdline and the cwd link, never a file under a seat's directory.
+# agent harness's shell-snapshot wrapper (`bash -c 'source .../shell-snapshots/
+# snapshot-*.sh ...'`), whose command line is the agent's command text; that
+# shell's real children are still matched, and so is any other command line
+# that only names a shell-snapshots path. It reads only /proc/<pid>/cmdline,
+# the cwd link and its own and its ancestors' /proc/<pid>/status, never a
+# file under a seat's directory.
 # --proc-root names a stand-in for /proc in the selftests.
 set -uo pipefail
 
@@ -30,22 +34,28 @@ while [ $# -gt 0 ]; do
 done
 [ -d "$PROC" ] || { echo "slot-busy: error: $PROC is not a directory" >&2; exit 2; }
 
-# This process and its ancestors, from the real /proc.
+# This process and its ancestors, from the real /proc. Its own status must
+# read and name a parent; a failed read further up only ends the walk, which
+# can give a false BUSY but never a false CLEAR.
 declare -A SKIP=()
-p=$$
+p=$(awk '/^PPid:/{print $2; exit}' "/proc/$$/status" 2>/dev/null) && [[ "$p" =~ ^[0-9]+$ ]] \
+  || { echo "slot-busy: error: cannot read its own process" >&2; exit 2; }
+SKIP[$$]=1
 while [ -n "$p" ] && [ "$p" != 0 ]; do
   SKIP[$p]=1
   p=$(awk '/^PPid:/{print $2; exit}' "/proc/$p/status" 2>/dev/null) || p=""
 done
-[ "${#SKIP[@]}" -ge 1 ] && [ -n "${SKIP[$$]:-}" ] || { echo "slot-busy: error: cannot read its own process" >&2; exit 2; }
 
 RE_NODE='[ /]node[^ ]* (.* )?--test '
 RE_GATE='[ /_-]gate[^ /]*\.sh '
 RE_TEST='[ /]test-[^ /]*\.sh '
 RE_COMPOSE='[ /]docker( compose|-compose) '
+# The wrapper's shape: a shell, its options with -c, then `source` (or `.`)
+# of a shell-snapshots/snapshot-*.sh file as the first command.
+RE_SNAPSHOT='^ (/[^ ]*/)?(ba|z)?sh( -[a-zA-Z]+)* -[a-zA-Z]*c[a-zA-Z]*( -[a-zA-Z]+)* (source|\.) [^ ]*/shell-snapshots/snapshot-[^ /]*\.sh '
 match() { # $1: the command line, NULs as spaces. Prints the rule that matched.
   local c=" $1 "
-  if [[ "$c" == *"/shell-snapshots/snapshot-"* ]]; then return 1; fi
+  if [[ "$c" =~ $RE_SNAPSHOT ]]; then return 1; fi
   if [[ "$c" =~ $RE_NODE ]]; then echo "node --test"; return 0; fi
   if [[ "$c" =~ $RE_GATE ]]; then echo "gate"; return 0; fi
   if [[ "$c" == *mutant* ]]; then echo "mutant"; return 0; fi
@@ -56,6 +66,7 @@ match() { # $1: the command line, NULs as spaces. Prints the rule that matched.
 
 seen=0
 busy=0
+errors=0
 for d in "$PROC"/[0-9]*; do
   [ -d "$d" ] || continue
   pid=${d##*/}
@@ -63,7 +74,14 @@ for d in "$PROC"/[0-9]*; do
   seen=$((seen + 1))
   [ -z "${SKIP[$pid]:-}" ] || continue
   # A process can exit between the listing and the read; that's not an error.
-  cmd=$(tr '\0' ' ' < "$d/cmdline" 2>/dev/null) || continue
+  # A failed read while its directory is still there is: the scan can't say
+  # what that process runs.
+  if ! cmd=$(tr '\0' ' ' < "$d/cmdline" 2>/dev/null); then
+    [ -e "$d" ] || continue
+    echo "slot-busy: error: cannot read $d/cmdline" >&2
+    errors=$((errors + 1))
+    continue
+  fi
   cmd=${cmd% }
   [ -n "$cmd" ] || continue
   rule=$(match "$cmd") || continue
@@ -72,6 +90,7 @@ for d in "$PROC"/[0-9]*; do
   busy=1
 done
 [ "$seen" -ge 1 ] || { echo "slot-busy: error: no processes under $PROC" >&2; exit 2; }
+[ "$errors" = 0 ] || { echo "slot-busy: error: $errors process(es) could not be read, so the host isn't known to be idle" >&2; exit 2; }
 if [ "$busy" = 1 ]; then exit 1; fi
 echo "CLEAR"
 exit 0

scripts/test-git-guard.sh

@@ -117,6 +117,32 @@ check "a push over a remote tip missing locally is refused, saying fetch first"
 check "the remote ref is unchanged after that refusal" "$([ "$(rtip seat/w)" = "$moved" ]; echo $?)"
 check "the guard fetched nothing" "$(! git -C "$C" cat-file -e "$moved^{commit}" 2>/dev/null; echo $?)"
 
+echo "== tags =="
+# An existing tag never changes. Both cases below pass the ancestor check: a
+# new tag object on the same commit peels to that commit, and a moved
+# lightweight tag's old commit is an ancestor of its new one.
+tagtip() { git -C "$B" rev-parse -q --verify "refs/tags/$1"; }
+git -C "$W" tag -a -m first t-ann HEAD
+out=$(git -C "$W" push origin t-ann 2>&1); rc=$?
+check "a new annotated tag passes" "$([ "$rc" = 0 ] && [ "$(tagtip t-ann)" = "$(git -C "$W" rev-parse t-ann)" ]; echo $?)"
+first=$(tagtip t-ann)
+git -C "$W" tag -f -a -m second t-ann HEAD >/dev/null
+out=$(git -C "$W" push --force origin t-ann 2>&1); rc=$?
+check "push --force of a new tag object on the same commit is refused" \
+    "$([ "$rc" -ne 0 ] && has "$out" "mosaic push guard: refused: refs/tags/t-ann: an existing tag" && has "$out" "$first"; echo $?)"
+check "the remote tag is still the first tag object" "$([ "$(tagtip t-ann)" = "$first" ]; echo $?)"
+git -C "$W" tag t-light HEAD~1
+out=$(git -C "$W" push origin t-light 2>&1); rc=$?
+check "a new lightweight tag passes" "$rc"
+git -C "$W" tag -f t-light HEAD >/dev/null
+out=$(git -C "$W" push --force origin t-light 2>&1); rc=$?
+check "push --force of a lightweight tag moved to a descendant is refused" \
+    "$([ "$rc" -ne 0 ] && has "$out" "mosaic push guard: refused: refs/tags/t-light: an existing tag"; echo $?)"
+check "the remote lightweight tag hasn't moved" "$([ "$(tagtip t-light)" = "$(git -C "$W" rev-parse HEAD~1)" ]; echo $?)"
+out=$(git -C "$W" push origin :refs/tags/t-light 2>&1); rc=$?
+check "a tag delete is refused, and the tag stays" \
+    "$([ "$rc" -ne 0 ] && has "$out" "refs/tags/t-light: a delete" && [ -n "$(tagtip t-light)" ]; echo $?)"
+
 echo "== the hook, fed by hand =="
 z=0000000000000000000000000000000000000000
 hookrun() { (cd "$C" && printf '%s\n' "$1" | sh "$C/.git/hooks/pre-push" origin "$B" 2>&1); }
@@ -124,6 +150,14 @@ out=$(hookrun "refs/heads/x $(git -C "$C" rev-parse HEAD) refs/heads/x"); rc=$?
 check "a short ref line is refused" "$([ "$rc" -ne 0 ] && has "$out" "unreadable ref line"; echo $?)"
 out=$(hookrun "refs/heads/x HEAD refs/heads/x $z"); rc=$?
 check "a non-hex object name is refused" "$([ "$rc" -ne 0 ] && has "$out" "non-hex"; echo $?)"
+out=$(hookrun "refs/heads/x $(git -C "$C" rev-parse HEAD) refs/heads/x 0"); rc=$?
+check "a remote object name of 0 is refused, not read as a new ref" \
+    "$([ "$rc" -ne 0 ] && has "$out" "isn't 40 or 64 hex characters"; echo $?)"
+out=$(hookrun "refs/heads/x $(git -C "$C" rev-parse HEAD | cut -c1-39) refs/heads/x $z"); rc=$?
+check "a 39-character local object name is refused" "$([ "$rc" -ne 0 ] && has "$out" "isn't 40 or 64 hex characters"; echo $?)"
+out=$(hookrun "refs/heads/x ${z}${z:0:24} refs/heads/x $(git -C "$C" rev-parse HEAD)"); rc=$?
+check "a 64-character object name passes the length check (64 zeros refuse as a delete)" \
+    "$([ "$rc" -ne 0 ] && has "$out" "refs/heads/x: a delete" && ! has "$out" "isn't 40 or 64"; echo $?)"
 out=$(hookrun "refs/heads/x 1234567890123456789012345678901234567890 refs/heads/x $(git -C "$C" rev-parse HEAD)"); rc=$?
 check "a failed ancestor check (an unknown local object) is refused" \
     "$([ "$rc" -ne 0 ] && has "$out" "the ancestor check failed"; echo $?)"
@@ -162,6 +196,18 @@ ln -s /dev/null "$SANDBOX/ln/.git/hooks/pre-push"
 out=$(bash "$INSTALL" --repo "$SANDBOX/ln" --force-install 2>&1); rc=$?
 check "the install refuses a symlinked pre-push, even with --force-install" "$([ "$rc" = 2 ] && has "$out" "is a symlink"; echo $?)"
 
+# A hooks directory that links to a shared one would put the hook outside
+# the repository, in every repository that shares it.
+fresh "$SANDBOX/sl"
+mkdir -p "$SANDBOX/shared-hooks"
+rm -rf "$SANDBOX/sl/.git/hooks"
+ln -s "$SANDBOX/shared-hooks" "$SANDBOX/sl/.git/hooks"
+out=$(bash "$INSTALL" --repo "$SANDBOX/sl" 2>&1); rc=$?
+check "the install refuses a symlinked hooks directory (exit 2)" "$([ "$rc" = 2 ] && has "$out" "/.git/hooks is a symlink"; echo $?)"
+out=$(bash "$INSTALL" --repo "$SANDBOX/sl" --force-install 2>&1); rc=$?
+check "it refuses it with --force-install too" "$([ "$rc" = 2 ] && has "$out" "/.git/hooks is a symlink"; echo $?)"
+check "nothing was written in the directory the symlink names" "$([ -z "$(ls -A "$SANDBOX/shared-hooks")" ]; echo $?)"
+
 fresh "$SANDBOX/wd"
 git -C "$SANDBOX/wd" worktree add -q -b seat/x "$SANDBOX/wd-wt" origin/main
 out=$(bash "$INSTALL" --repo "$SANDBOX/wd-wt" 2>&1); rc=$?
@@ -187,11 +233,13 @@ fakeproc() { # $1: pid, $2: cwd, rest: argv
 busyrun() { bash "$BUSY" --proc-root "$P" 2>&1; }
 fakeproc 4194401 /srv node server.mjs
 fakeproc 4194402 /srv grep -r gate .
-fakeproc 4194403 /srv /bin/bash -c "source /home/u/.claude/shell-snapshots/snapshot-bash-1.sh && eval 'node --test x.test.mjs'"
+# Both wrappers' text would match a rule without the snapshot exclusion.
+fakeproc 4194403 /srv /bin/bash -c "source /home/u/.claude/shell-snapshots/snapshot-bash-1.sh && node --test x.test.mjs"
+fakeproc 4194405 /srv /usr/bin/bash -c "source /home/u/.claude/shell-snapshots/snapshot-bash-2-ab.sh 2>/dev/null || true && eval 'bash scripts/test-x.sh > x.log' && pwd -P >| /home/u/cwd"
 mkdir -p "$P/4194404"; : > "$P/4194404/cmdline"
 out=$(busyrun); rc=$?
 check "no matching process: exit 0 and CLEAR" "$([ "$rc" = 0 ] && [ "$out" = CLEAR ]; echo $?)"
-check "an agent's shell-snapshot wrapper isn't a match" "$(! has "$out" "4194403"; echo $?)"
+check "an agent's shell-snapshot wrapper isn't a match" "$(! has "$out" "4194403" && ! has "$out" "4194405"; echo $?)"
 
 fakeproc 4194410 /seat/a node --test packages/x/tests/a.test.mjs
 out=$(busyrun); rc=$?
@@ -215,8 +263,58 @@ rule "mutant" bash agents/x/work/queue-1/mutants.sh M3
 rule "test script" bash scripts/test-land-work.sh
 rule "docker compose" docker compose -p x up -d
 rule "docker compose" /usr/bin/docker-compose up
+# The snapshot exclusion is for the wrapper's shape only: the same command
+# without the snapshot source, and real runs whose arguments hold the path.
+rule "node --test" /bin/bash -c "node --test x.test.mjs"
+rule "node --test" node --test /seat/tests/shell-snapshots/snapshot-guard.test.mjs
+rule "gate" bash /seat/shell-snapshots/snapshot-gate.sh
 rm -rf "${P:?}/4194420"
 
+echo "== slot-busy, read errors =="
+# A pid directory that stays while its cmdline can't be read is an error,
+# never a vanished process: exit 2 and no CLEAR.
+mkdir -p "$P/4194430"
+out=$(busyrun); rc=$?
+check "a pid directory with no cmdline: exit 2" "$([ "$rc" = 2 ]; echo $?)"
+check "it names that cmdline and never says CLEAR" "$(has "$out" "cannot read $P/4194430/cmdline" && ! has "$out" CLEAR; echo $?)"
+rm -rf "${P:?}/4194430"
+mkdir -p "$P/4194431/cmdline"
+out=$(busyrun); rc=$?
+check "a cmdline that can't be read (a directory): exit 2, no CLEAR" "$([ "$rc" = 2 ] && ! has "$out" CLEAR; echo $?)"
+fakeproc 4194410 /seat/a node --test a.test.mjs
+out=$(busyrun); rc=$?
+check "a read error beside a BUSY process: exit 2, the BUSY line kept, no CLEAR" \
+    "$([ "$rc" = 2 ] && has "$out" "BUSY 4194410 node --test" && ! has "$out" CLEAR; echo $?)"
+rm -rf "${P:?}/4194431"
+chmod 000 "$P/4194410/cmdline"
+out=$(busyrun); rc=$?
+check "a node --test process whose cmdline is unreadable (mode 000) is never CLEAR" "$([ "$rc" != 0 ] && ! has "$out" CLEAR; echo $?)"
+rm -rf "${P:?}/4194410"
+
+mkdir -p "$SANDBOX/failtr" "$SANDBOX/vanishtr" "$SANDBOX/failawk"
+printf '#!/bin/sh\nexit 1\n' > "$SANDBOX/failtr/tr"
+printf '#!/bin/sh\nexit 1\n' > "$SANDBOX/failawk/awk"
+# This tr stands in for a process that exits during the read: for pid
+# 4194440 it removes the pid directory and fails; otherwise it is tr.
+cat > "$SANDBOX/vanishtr/tr" <<EOF
+#!/bin/sh
+f=\$(readlink "/proc/\$\$/fd/0")
+case "\$f" in */4194440/cmdline) rm -rf "\${f%/cmdline}"; exit 1 ;; esac
+exec $(command -v tr) "\$@"
+EOF
+chmod +x "$SANDBOX/failtr/tr" "$SANDBOX/failawk/awk" "$SANDBOX/vanishtr/tr"
+out=$(PATH="$SANDBOX/failtr:$PATH" bash "$BUSY" --proc-root "$P" 2>&1); rc=$?
+check "a failing read (tr exits 1) with every pid directory still there: exit 2, no CLEAR" \
+    "$([ "$rc" = 2 ] && has "$out" "cannot read" && ! has "$out" CLEAR; echo $?)"
+fakeproc 4194440 /seat/v node --test v.test.mjs
+out=$(PATH="$SANDBOX/vanishtr:$PATH" bash "$BUSY" --proc-root "$P" 2>&1); rc=$?
+check "a process that vanishes during its read isn't an error: exit 0 and CLEAR" \
+    "$([ "$rc" = 0 ] && [ "$out" = CLEAR ] && [ ! -e "$P/4194440" ]; echo $?)"
+rm -rf "${P:?}/4194440"
+out=$(PATH="$SANDBOX/failawk:$PATH" bash "$BUSY" --proc-root "$P" 2>&1); rc=$?
+check "its own process unreadable (awk exits 1): exit 2, no CLEAR" \
+    "$([ "$rc" = 2 ] && has "$out" "cannot read its own process" && ! has "$out" CLEAR; echo $?)"
+
 out=$(bash "$BUSY" --proc-root "$SANDBOX/no-such-proc" 2>&1); rc=$?
 check "a missing proc root: exit 2" "$([ "$rc" = 2 ]; echo $?)"
 mkdir -p "$SANDBOX/empty-proc"