auto-deploy: stop tag pushes hijacking branch hosts; fix pull order + deploy-on-failure#1422
Open
jonfroehlich wants to merge 2 commits into
Open
auto-deploy: stop tag pushes hijacking branch hosts; fix pull order + deploy-on-failure#1422jonfroehlich wants to merge 2 commits into
jonfroehlich wants to merge 2 commits into
Conversation
Two related bugs that together froze makeabilitylab-test for four days (Jul 18-22) while making it look freshly deployed. 1. `$OPERATION = "TAG"` was an assignment, not a comparison. Every tag push therefore satisfied the condition on EVERY configured host, and reassigned $OPERATION to "TAG" for the remainder of the request. A branch-tracking host (the test server) was thus driven down the tag path -- `git fetch` + `git checkout <tag>` -- leaving its checkout detached at that tag. Confirmed: tag 2.27.0 points at 578d643, which is exactly the commit the test host was frozen on. 2. Once detached, it could not recover. `git pull` aborts from a detached HEAD, and do_clone_or_pull() ran BEFORE do_checkout_branch(), so the pull failed, the local branch never advanced, and the checkout stayed behind. For branch deploys we now check out the branch first and pull second, which both fixes the failure and self-heals an already-detached checkout. Tag deploys keep the original order, since a tag generally isn't present locally until after the fetch. Not fixed here, but worth noting: index.php calls deploy_container() regardless of whether the pull or checkout succeeded, so a failed update still produces a successful-looking rebuild. That is what hid this -- /version.json reported built_at advancing on every push while git_sha sat at a four-day-old commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lings
Follow-up hardening from code review of the tag-hijack/pull-order fix:
- Deploy gate: the container build ran unconditionally after checkout/pull, so
any pull failure (merge conflict, network/ssh error, or a detached HEAD the
reorder doesn't cover) rebuilt from stale source while looking freshly
deployed. Now confirm the checked-out HEAD equals the pushed commit
($req['after']) before building; abort and log loudly on mismatch. Fail-safe:
if the payload carries no usable SHA, deploy as before (never blocks a legit
deploy).
- _trace()/_debug(): `if ($x = TRUE)` was an assignment, not a comparison --
the same =/== footgun this branch fixes in the deploy condition -- so they
always logged and could never be disabled. Default on (preserves behavior),
disable via define('AUTODEPLOY_TRACE'/'AUTODEPLOY_DEBUG', false) in config.php.
- determine_branch_name(): guarded on misspelled $reqest, so it always returned
null and the branch-name fast path was dead code. Fixed to $request.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Hardens the GitLab/GitHub auto-deploy webhook (
auto-deploy/index.php).1. Tag pushes hijacking branch hosts (the headline bug)
The condition
($incomingOp == "tags" && $OPERATION = "TAG")used=(assignment) instead of==. That made the clause true for every tag push regardless of host config, and reassigned$OPERATIONto"TAG"for the rest of the request — so a tag push drove the branch-tracking test server down the tag code path, leaving its checkout detached at that tag. Fixed to==.2. Pull/checkout order
git pullaborts from a detached HEAD ("You are not currently on a branch"). For branch hosts we nowcheckoutfirst, thenpull; for tag hosts we fetch first (the tag usually isn't local yet). Prod's tag path is unchanged in the happy case.3. Deploy-on-failure gate (from code review)
The container build ran unconditionally after checkout/pull, so any pull failure still rebuilt from stale source while looking freshly deployed. Now we confirm the checked-out
HEADequals the pushed commit ($req['after']) before building; abort + log on mismatch. Fail-safe: if the payload has no usable SHA, deploy as before (never blocks a legit deploy).4. Same
=/==footgun, twice more_trace()/_debug()guarded onif ($x = TRUE)— always true, never disableable. Fixed to adefined()toggle (default on; silence viadefine('AUTODEPLOY_TRACE'/'AUTODEPLOY_DEBUG', false)in config.php).5.
determine_branch_name()typoGuarded on misspelled
$reqest→ always returned null (branch fast-path was dead code). Fixed to$request.Testing
php -lclean (no local PHP runtime; linted in a throwawayphp:8-cli-alpinecontainer).🤖 Generated with Claude Code