From d3df0afa23a1c4d9a18054011ecdd8741a195193 Mon Sep 17 00:00:00 2001 From: Jon Froehlich Date: Wed, 22 Jul 2026 14:15:38 -0700 Subject: [PATCH 1/2] auto-deploy: stop tag pushes hijacking branch hosts; fix pull order 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 ` -- leaving its checkout detached at that tag. Confirmed: tag 2.27.0 points at 578d6438, 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) --- auto-deploy/index.php | 47 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/auto-deploy/index.php b/auto-deploy/index.php index 0c07417c..19adf55c 100644 --- a/auto-deploy/index.php +++ b/auto-deploy/index.php @@ -86,16 +86,43 @@ } //Make sure we're doing a TAG or BRANCH as appropriate - if(($incomingOp == "tags" && $OPERATION = "TAG") || ($incomingOp == "heads" && $refs[2] == $OPERATION)){ - //possibly need to clone it or pull it first: - - $out = do_clone_or_pull(DEPLOY_KEY,$url,$deploy_to, $OPERATION); - _debug("$out\n"); - - //The following will ensure we're either on the right branch, or the right tag: - $out = do_checkout_branch($req,$deploy_to, $OPERATION); - _debug("$out\n"); - + // NOTE: the first clause below is `==`, not `=`. It was previously an + // assignment, which made the condition true for EVERY tag push regardless of + // how this host is configured -- and, worse, reassigned $OPERATION to "TAG" + // for the rest of the request. The effect was that a tag push drove a + // branch-tracking host (our test server) down the tag code path, leaving its + // checkout detached at that tag. + if(($incomingOp == "tags" && $OPERATION == "TAG") || ($incomingOp == "heads" && $refs[2] == $OPERATION)){ + + // Order matters, and it differs between the two cases: + // + // - TAG: fetch first. The tag usually doesn't exist locally yet, so + // checking it out before fetching would fail. + // - BRANCH: check out the branch first. `git pull` aborts outright + // from a detached HEAD ("You are not currently on a branch"), and + // the old fetch-then-checkout order could never recover from that: + // the pull failed, the local branch stayed behind, and the deploy + // ran anyway (see below) -- so the container rebuilt from stale + // source while looking freshly deployed. + $repo_exists = is_dir($deploy_to) && file_exists($deploy_to . "/.git"); + + if($OPERATION != "TAG" && $repo_exists){ + $out = do_checkout_branch($req,$deploy_to, $OPERATION); + _debug("$out\n"); + + $out = do_clone_or_pull(DEPLOY_KEY,$url,$deploy_to, $OPERATION); + _debug("$out\n"); + } + else{ + //possibly need to clone it or pull it first: + $out = do_clone_or_pull(DEPLOY_KEY,$url,$deploy_to, $OPERATION); + _debug("$out\n"); + + //The following will ensure we're either on the right branch, or the right tag: + $out = do_checkout_branch($req,$deploy_to, $OPERATION); + _debug("$out\n"); + } + //If we're using docker, then do someother stuff if($USE_DOCKER){ From ba4c9a8a343aa0d87972d56a55fef1680ff1ae38 Mon Sep 17 00:00:00 2001 From: Jon Froehlich Date: Wed, 22 Jul 2026 16:12:28 -0700 Subject: [PATCH 2/2] auto-deploy: gate deploy on HEAD matching pushed commit; fix =/== siblings 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) --- auto-deploy/index.php | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/auto-deploy/index.php b/auto-deploy/index.php index 19adf55c..d1e1722d 100644 --- a/auto-deploy/index.php +++ b/auto-deploy/index.php @@ -123,6 +123,27 @@ _debug("$out\n"); } + // Guard against silently deploying stale source. Historically the + // container build below ran unconditionally after checkout/pull, so a + // failed pull (detached HEAD, merge conflict, network/ssh error) would + // rebuild from STALE source while looking freshly deployed. Confirm the + // checked-out HEAD is the commit that was actually pushed ($req['after']) + // before building. If the payload carries no usable SHA we fall back to + // deploying (preserves prior behavior rather than risk blocking a deploy). + // (On rapid successive pushes HEAD may already be a *newer* commit than + // this event's 'after'; skipping here is safe -- the newer push's hook + // deploys the newer state.) + $expected_sha = isset($req['after']) ? strtolower(trim($req['after'])) : ""; + $has_sha = ($expected_sha !== "" && !preg_match('/^0+$/', $expected_sha)); + $head_sha = strtolower(trim(shell_exec("bash -c 'cd $deploy_to; git rev-parse HEAD' 2>/dev/null"))); + + if($has_sha && $head_sha !== "" && $head_sha !== $expected_sha){ + _log("ABORTING DEPLOY: checked-out HEAD ($head_sha) does not match the ". + "pushed commit ($expected_sha) -- the pull/checkout above likely ". + "failed. Refusing to rebuild the container from stale source.\n"); + exit; + } + //If we're using docker, then do someother stuff if($USE_DOCKER){ @@ -222,7 +243,7 @@ function do_checkout_branch($req, $deployto, $operation){ */ function determine_branch_name( $request) { // push request, branch is in request[ref] - if (isset($reqest['ref'])) { + if (isset($request['ref'])) { // strip out the refs/head nonsense -- doesn't look like bare // branch is listed anywhere in the request return preg_replace("|refs/heads/|", "", $request['ref']); @@ -307,18 +328,24 @@ function do_clone($key,$url,$path) { * Log a message, only here to make some messages easy to turn off. */ function _trace($message) { - if ($trace = TRUE) { + // NOTE: was `if ($trace = TRUE)` -- an assignment, not a comparison, so it + // was always true and could never be turned off (same =/== footgun fixed in + // the deploy condition above). Default to on to preserve prior behavior; + // config.php can silence it with `define('AUTODEPLOY_TRACE', false);`. + if (defined('AUTODEPLOY_TRACE') ? AUTODEPLOY_TRACE : TRUE) { _log($message); - } + } } /** * Log a message, only here to make some messages easy to turn off. */ function _debug($message) { - if ($debug = TRUE) { + // Same fix as _trace(): `$debug = TRUE` was an assignment, always true. + // Default on; silence via `define('AUTODEPLOY_DEBUG', false);` in config.php. + if (defined('AUTODEPLOY_DEBUG') ? AUTODEPLOY_DEBUG : TRUE) { _log($message); - } + } } /**