auto-deploy: stop tag pushes hijacking branch hosts; fix pull order + deploy-on-failure - #1422
Merged
Merged
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>
…oming ref
Two fixes to the deploy gate added in the previous commit:
1. The gate compared `git rev-parse HEAD` to the raw `$req['after']`. For an
ANNOTATED tag push, `after` is the ref's new value -- the tag OBJECT's sha --
while `git checkout refs/tags/X` leaves HEAD at the COMMIT. Our last three
releases (2.27.0, 2.27.2, 2.27.3) are annotated, so as written the gate would
have aborted every production deploy. The expected sha is now peeled with
`^{commit}` before comparison, and the check is skipped (deploy proceeds, as
it always did) whenever the sha can't be resolved locally.
2. `$req['ref']` is interpolated into `git checkout <ref>` via shell_exec on an
unauthenticated endpoint. It is now required to match
`refs/(heads|tags)/[A-Za-z0-9._/-]+` before any of it reaches a shell.
Verified: `php -l` clean; peel/detached-HEAD semantics confirmed against real
git; ref and sha patterns exercised over accept/refuse cases.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jonfroehlich
added a commit
that referenced
this pull request
Jul 27, 2026
Co-Authored-By: Claude Opus 5 (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.6. Deploy gate: peel annotated tags (would have blocked every prod release)
Follow-up to #3.
$req['after']is the ref's new value, which for anannotated tag is the tag object's sha, while
git checkout refs/tags/Xleaves
HEADat the commit. Tags 2.27.0 / 2.27.2 / 2.27.3 are all annotated,so the gate as first written would have aborted every production deploy. The
expected sha is now peeled with
^{commit}before comparing, and the gate isskipped entirely (deploy proceeds, as before) if the sha can't be resolved
locally. Verified against real git: annotated tag object
e109d61…peels tocommit
3c746b3…, which is whatHEADreads after the tag checkout.7. Validate the incoming ref before it reaches a shell
$req['ref']was interpolated straight intogit checkout <ref>viashell_exec()on an unauthenticated endpoint (no webhook-secret /signature check anywhere in this script), so a crafted payload naming our
public repo URL could smuggle shell metacharacters onto the deploy host. The
ref must now match
refs/(heads|tags)/[A-Za-z0-9._/-]+; anything else islogged and refused. Normal refs (
refs/heads/master,refs/heads/dependabot/pip/pypdf-6.14.2,refs/tags/2.27.3) are unaffected.Testing
php -lclean (no local PHP runtime; linted in a throwawayphp:8-cli-alpinecontainer).🤖 Generated with Claude Code