fix(git): resolve absolute bare repo dir in post-receive hook#304
fix(git): resolve absolute bare repo dir in post-receive hook#304cjunxiang wants to merge 2 commits into
Conversation
|
Hey sorry folks, issue is bugging me for awhile, decided to push ahead with a straight forward fix after seeing similar reported issues. Do close this if it's too intrusive and pushy. |
|
Thanks for the contribution, @cjunxiang! One process note on this repo: PRs need to be raised through no-mistakes ( If working from a fork was the blocker, that's fixed as of v1.30.0 (#306). Per CONTRIBUTING.md: point I won't be merging PRs that bypass the gate going forward, but I'd genuinely love to land your work once it's re-raised. Thanks for understanding! 🙏 |
…enguid#269) The post-receive hook passed --gate "$(pwd)", but git can invoke the hook from a cwd whose pwd collapses to ".". The daemon then rejected the push with "invalid gate path: ." and the pipeline never started; git printed "Everything up-to-date" with no hint, and `no-mistakes runs` showed none. Resolve the bare repo dir explicitly via `git rev-parse --absolute-git-dir` (queries git directly; always returns the true path regardless of cwd/PWD state) and reuse it for both --gate and notify-push.log. Falls back to pwd only if git itself is unavailable. Regression test poisons PWD="." to reproduce the reported failure deterministically (the POSIX pwd builtin returns "." exactly as the reporter saw), then asserts the recorded --gate is the absolute bare dir. Closes kunchenguid#269.
a36c186 to
5369df6
Compare
|
pushed with no mistake. 👍🏼 Thanks @kunchenguid appreciate the work |
|
thanks for this. quick note on process - i require PRs to come through no-mistakes, which is why the "PR must be raised via no-mistakes" check is red here (this was opened directly). the reason i ask for it: no-mistakes runs review, tests, lint, and docs on your branch before the PR is opened, so contributions arrive already validated. that keeps the quality bar high and keeps me from having to hand-review every change - i maintain this solo, so it's the only way i keep up. can you re-raise it through no-mistakes? running the pipeline on your branch opens/updates the PR through the gate and turns that check green. the change looks useful, it just needs to come through that path. ping me if the setup gives you trouble. |
I've moved to #358 as advised, and closed this MR. |
What Changed
post-receivehook resolves the bare repo directory viagit rev-parse --absolute-git-dir(falling back topwdonly if git is unavailable) instead of relying onpwd, so it yields the true bare-repo path regardless of the cwd/PWD state git invoked the hook from.notify-push --gateargument and thenotify-push.logpath now use the resolvedGATE_DIRrather than$(pwd).PWD=.-poisoned invocations where the oldpwd-based path collapsed to an invalid--gate ..Risk Assessment
✅ Low: Well-bounded one-commit fix (a single shell snippet plus a focused, deterministic regression test) that correctly addresses the diagnosed root cause; the daemon matches repos by basename only, so the new absolute/symlink-resolved path cannot regress downstream path matching, and the change includes a sensible defensive fallback.
Testing
Verified the #269 fix end-to-end through the actual product surfaces (real hook script, real git push, real daemon over real IPC), not by re-running unit tests. Proof chain: (1) The shipped hook (PostReceiveHookScript, rendered to 01-real-shipped-hook.sh) now computesGATE_DIR=$(git rev-parse --absolute-git-dir 2>/dev/null || pwd). (2) Driving BOTH the NEW and OLD hook script under the three #269 conditions (02-gate-resolution-transcript.txt) shows the NEW hook resolves the absolute bare dir every time (A/B/C) and on a genuinegit push(D), while the OLD hook produces the bug values: '.' under PWD=. poison and a non-.git sibling dir under cwd-divergence. (3) Most decisive — feeding each candidate value to the REAL daemon (no-mistakes daemon in an isolated NM_HOME, reached via real unix-socket IPC with daemon notify-push) and reading the server's own slog verdict (03-daemon-gate-validation.txt): the daemon rejects '.' withinvalid gate path: .and rejects the sibling dir withinvalid gate path: <dir>— exactly the #269 fatal error that left pipelines never starting — but it ACCEPTS the NEW hook's absolute<id>.gitvalue at the gate layer (unknown repo for gate …, i.e. repoIDFromGatePath accepted the path and the failure is the unrelated 'repo not registered' check). That closes the loop: the value the OLD hook computes is fatal at the layer that broke; the value the NEW hook computes passes it. Method note: client-sidedaemon notify-pushreturns empty output on error because the root cobra command sets SilenceErrors=true, so the authoritative verdict is the daemon's server-side slog line "ipc request failed method=push_received error=…" — read from the daemon log.Evidence: 01 — Real shipped post-receive hook (contains the fix)
Rendered PostReceiveHookScript(). Line 17: GATE_DIR=$(git rev-parse --absolute-git-dir 2>/dev/null || pwd) — the #269 fix. Old '$(pwd)' gate path is gone. Includes the rationale comment explaining why pwd collapses to '.' and citing Git 2.13+.Evidence: 02 — NEW vs OLD hook gate resolution under all three #269 conditions + genuine git push
A) normal cwd — both correct. B) cwd diverges to sibling, GIT_DIR=bare — NEW resolves absolute bare dir; OLD yields non-.git sibling dir (daemon-rejectable). C) PWD=. poisoned — NEW resolves absolute bare dir; OLD yields '.' (the reported bug). D) genuinegit pushwith real NEW hook installed — hook fires, records absolute push.git gate, push succeeds.Evidence: 03 — Real daemon verdicts (decisive before/after proof)
Real daemon server slog log. --gate '.' -> 'invalid gate path: .' (the #269 fatal). --gate sibling dir -> 'invalid gate path: <dir>'. --gate absolute '<id>.git' (NEW hook value) -> 'unknown repo for gate …' (ACCEPTED at gate layer; fails only for unrelated unregistered-repo reason).Pipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
Shipped hook content: rendered PostReceiveHookScript() and confirmed line 'GATE_DIR=$(git rev-parse --absolute-git-dir 2>/dev/null || pwd)' plus the #269 rationale comment are present; old 'pwd' gate path is gone (01-real-shipped-hook.sh).NEW hook gate resolution — case A (normal, cwd==bare dir): resolves absolute bare dir. CORRECT.NEW hook gate resolution — case B (cwd diverges to a sibling dir while GIT_DIR points at the bare repo, the #269 'pwd no longer names the bare repo' class): resolves absolute bare dir. CORRECT (OLD hook yields the sibling dir, which is not a .git path).NEW hook gate resolution — case C (PWD=. poisoned with cwd==bare, the author's deterministic repro from hook_test.go): resolves absolute bare dir. CORRECT (OLD hook yields exactly '.').NEW hook gate resolution — case D (genuinegit pushto a bare repo with the real shipped NEW hook installed): hook fires on real push and records the absolute push.git gate; push succeeds. CORRECT.OLD (pre-fix) hook reproduction: confirmed it computes '.' under PWD=. poison and a non-.git sibling dir under cwd-divergence — reproducing the exact reported bug.Real daemon gate validation (isolated NM_HOME, real daemon process, real unix-socket IPC via daemon notify-push): --gate '.' -> 'invalid gate path: .' (fatal); --gate sibling dir -> 'invalid gate path: <dir>' (fatal); --gate absolute '<id>.git' (NEW hook value) -> 'unknown repo for gate …' (ACCEPTED at the gate layer; fails only for unrelated unregistered-repo reason).Worktree hygiene: removed throwaway zz_dump_hook_test.go and ./bin build output; verifiedgit statusclean and HEAD at 5369df6.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.