entireio / entireio/cli

`entire enable --force` writes the hook wrapper through a symlink, clobbering the target file and creating a self-chaining hook

Open
#2,410 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
5.1k
Forks
475
Avg merge
1d 11h
Merged PRs (30d)
178

Description

Summary

When a git hook path is a symlink (a common pattern for repo-managed hooks, where .git/hooks/pre-push is symlinked to a script checked into the repository) and a <hook>.pre-entire backup already exists from a previous install, entire enable --force writes its wrapper through the symlink instead of replacing it.

This has two consequences:

  1. Data loss — the file the symlink points at (a version-controlled script in the working tree) is overwritten with the wrapper's contents.
  2. Infinite recursion<hook> and <hook>.pre-entire end up resolving to the same file, so the wrapper's "chain to pre-existing hook" call re-executes itself. Every subsequent commit/push forks shells without bound until the machine runs out of processes.

Verified on Entire CLI 0.10.6 (go1.26.6, darwin/arm64).

Reproduction
mkdir repro && cd repro && git init
mkdir scripts
printf '#!/bin/sh\necho "PROJECT HOOK RAN"\n' > scripts/post-commit.sh
chmod +x scripts/post-commit.sh

# Repo-managed hook, installed as a symlink
ln -nsf "$PWD/scripts/post-commit.sh" .git/hooks/post-commit

# 1st enable — correct behaviour
entire enable --force
# [entire] Backed up existing post-commit to post-commit.pre-entire
#   post-commit             -> regular file (the wrapper)
#   post-commit.pre-entire  -> symlink to scripts/post-commit.sh

# Anything that re-installs the repo's hooks now re-creates the symlink,
# while the .pre-entire backup is still present:
ln -nsf "$PWD/scripts/post-commit.sh" .git/hooks/post-commit

# 2nd enable — the bug
entire enable --force
# [entire] Warning: replacing post-commit (backup post-commit.pre-entire already exists from a previous install)
Actual result

.git/hooks/post-commit is still a symlink, and the wrapper was written straight through it:

$ cat scripts/post-commit.sh
#!/bin/sh
# Entire CLI hooks
# Post-commit hook: condense session data if commit has Entire-Checkpoint trailer
if command -v entire >/dev/null 2>&1; then entire hooks git post-commit 2>/dev/null || true; else :; fi
# Chain: run pre-existing hook
_entire_hook_dir="$(dirname "$0")"
if [ -x "$_entire_hook_dir/post-commit.pre-entire" ]; then
    "$_entire_hook_dir/post-commit.pre-entire" "$@"
fi

The original echo "PROJECT HOOK RAN" script is gone — overwritten in the working tree.

Both hook paths now resolve to the same inode:

resolved .git/hooks/post-commit:            2129733
resolved .git/hooks/post-commit.pre-entire: 2129733

So the wrapper's chain call invokes itself. A single git commit then recurses until the process limit is hit:

.git/hooks/post-commit.pre-entire: line 9: Terminated: 15  "$_entire_hook_dir/post-commit.pre-entire" "$@"
error: .git/hooks/post-commit died of signal 15

(Reproduced under ulimit -u to keep it bounded; without a limit it forks until the machine is unusable.)

Expected result

entire enable --force should replace the hook path itself rather than following it, and should never produce an installation where <hook> and <hook>.pre-entire are the same file.

Relationship to #2237 and #1349

This is the same InstallGitHook branch as #2237 — the backupExists path that
prints Warning: replacing <hook> (backup ... already exists from a previous install)
and overwrites the file at the hook name — but a different consequence, and it needs
the hook to be a symlink to reproduce.

  • #2237: the hook is a regular file. The overwrite loses another tool's newer hook and
    the chain runs a stale backup. Nothing outside .git/ is touched.
  • This issue: the hook is a symlink. The overwrite follows it and lands on the
    symlink target in the working tree, destroying a version-controlled file, and
    leaves <hook> and <hook>.pre-entire pointing at the same inode, which turns the
    chain call into unbounded recursion.

Replacing the hook path rather than writing through it is independent of how #2237 is
resolved, and is worth applying on its own since it is what causes the data loss and
the recursion here.

Suggested fix

Never write through the hook path. unlink the target first, or write to a
temporary file and rename() it into place, so the install always replaces the hook
path itself rather than following it. That prevents the working-tree file from being
destroyed and stops <hook> and <hook>.pre-entire from ever resolving to the same
file.

How the backupExists case should behave beyond that — refreshing the backup,
refusing the install, surfacing it in entire doctor — overlaps #2237, so I've left
that to you rather than guessing at constraints I can't see from outside.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the InstallGitHook branch described in the issue, especially the backupExists path, and reproduce the two-step symlink installation with a temporary Git repository. Verify that installation replaces the hook path rather than following the symlink, preserves the working-tree script, and leaves the hook and its .pre-entire backup resolving to different files.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, go
Domain
cli, developer-experience, devtools
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.