Skip to content

Commit d72dea2

Browse files
jonfroehlichjonfroehlich
authored
Merge pull request #1422 from makeabilitylab/fix-autodeploy-tag-hijack-and-pull-order
auto-deploy: stop tag pushes hijacking branch hosts; fix pull order + deploy-on-failure
2 parents e27e48d + 38d65fc commit d72dea2

1 file changed

Lines changed: 98 additions & 16 deletions

File tree

‎auto-deploy/index.php‎

Lines changed: 98 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,17 @@
6464

6565
//Make a determiniation if the incoming operation is right for this system.
6666
if($gitSystem && $HOSTNAME && $OPERATION){
67-
$refs=explode("/",$req['ref']);
67+
// The ref is interpolated into shell commands further down (`git checkout
68+
// <ref>`) and this endpoint is unauthenticated, so accept only a plain,
69+
// well-formed ref name and refuse anything else. Without this, a crafted
70+
// payload could smuggle shell metacharacters through to shell_exec().
71+
$incoming_ref = isset($req['ref']) ? trim($req['ref']) : "";
72+
if(!preg_match('#^refs/(heads|tags)/[A-Za-z0-9._/-]+$#', $incoming_ref)){
73+
_log("Refusing to act on a malformed or unsupported ref: " . json_encode($incoming_ref) . "\n");
74+
exit;
75+
}
76+
77+
$refs=explode("/",$incoming_ref);
6878
$incomingOp=$refs[1]; // should be tags or heads
6979

7080
//Some initial validation, make sure this is a configured repo and stuff
@@ -86,16 +96,82 @@
8696
}
8797

8898
//Make sure we're doing a TAG or BRANCH as appropriate
89-
if(($incomingOp == "tags" && $OPERATION = "TAG") || ($incomingOp == "heads" && $refs[2] == $OPERATION)){
90-
//possibly need to clone it or pull it first:
91-
92-
$out = do_clone_or_pull(DEPLOY_KEY,$url,$deploy_to, $OPERATION);
93-
_debug("$out\n");
94-
95-
//The following will ensure we're either on the right branch, or the right tag:
96-
$out = do_checkout_branch($req,$deploy_to, $OPERATION);
97-
_debug("$out\n");
98-
99+
// NOTE: the first clause below is `==`, not `=`. It was previously an
100+
// assignment, which made the condition true for EVERY tag push regardless of
101+
// how this host is configured -- and, worse, reassigned $OPERATION to "TAG"
102+
// for the rest of the request. The effect was that a tag push drove a
103+
// branch-tracking host (our test server) down the tag code path, leaving its
104+
// checkout detached at that tag.
105+
if(($incomingOp == "tags" && $OPERATION == "TAG") || ($incomingOp == "heads" && $refs[2] == $OPERATION)){
106+
107+
// Order matters, and it differs between the two cases:
108+
//
109+
// - TAG: fetch first. The tag usually doesn't exist locally yet, so
110+
// checking it out before fetching would fail.
111+
// - BRANCH: check out the branch first. `git pull` aborts outright
112+
// from a detached HEAD ("You are not currently on a branch"), and
113+
// the old fetch-then-checkout order could never recover from that:
114+
// the pull failed, the local branch stayed behind, and the deploy
115+
// ran anyway (see below) -- so the container rebuilt from stale
116+
// source while looking freshly deployed.
117+
$repo_exists = is_dir($deploy_to) && file_exists($deploy_to . "/.git");
118+
119+
if($OPERATION != "TAG" && $repo_exists){
120+
$out = do_checkout_branch($req,$deploy_to, $OPERATION);
121+
_debug("$out\n");
122+
123+
$out = do_clone_or_pull(DEPLOY_KEY,$url,$deploy_to, $OPERATION);
124+
_debug("$out\n");
125+
}
126+
else{
127+
//possibly need to clone it or pull it first:
128+
$out = do_clone_or_pull(DEPLOY_KEY,$url,$deploy_to, $OPERATION);
129+
_debug("$out\n");
130+
131+
//The following will ensure we're either on the right branch, or the right tag:
132+
$out = do_checkout_branch($req,$deploy_to, $OPERATION);
133+
_debug("$out\n");
134+
}
135+
136+
// Guard against silently deploying stale source. Historically the
137+
// container build below ran unconditionally after checkout/pull, so a
138+
// failed pull (detached HEAD, merge conflict, network/ssh error) would
139+
// rebuild from STALE source while looking freshly deployed. Confirm the
140+
// checked-out HEAD is the commit that was actually pushed ($req['after'])
141+
// before building. If the payload carries no usable SHA we fall back to
142+
// deploying (preserves prior behavior rather than risk blocking a deploy).
143+
// (On rapid successive pushes HEAD may already be a *newer* commit than
144+
// this event's 'after'; skipping here is safe -- the newer push's hook
145+
// deploys the newer state.)
146+
//
147+
// IMPORTANT: 'after' is the ref's new VALUE, which for an ANNOTATED tag is
148+
// the tag OBJECT's sha -- not the commit's. `git checkout refs/tags/X`
149+
// leaves HEAD at the COMMIT, so a raw sha comparison would abort every
150+
// annotated-tag deploy, i.e. every production release (2.27.0/.2/.3 are all
151+
// annotated). Peel with `^{commit}` before comparing.
152+
$expected_sha = isset($req['after']) ? strtolower(trim($req['after'])) : "";
153+
$has_sha = preg_match('/^[0-9a-f]{40}$/', $expected_sha) && !preg_match('/^0+$/', $expected_sha);
154+
155+
$head_sha = "";
156+
$expected_commit = "";
157+
if($has_sha){
158+
$head_sha = strtolower(trim(shell_exec("bash -c 'cd $deploy_to; git rev-parse HEAD' 2>/dev/null")));
159+
$expected_commit = strtolower(trim(shell_exec(
160+
"bash -c 'cd $deploy_to; git rev-parse --verify --quiet {$expected_sha}^{commit}' 2>/dev/null")));
161+
}
162+
163+
// Fail SAFE: abort only when we positively know HEAD is the wrong commit.
164+
// A sha we can't resolve locally (fetch failed, unusual payload, ...) falls
165+
// through to the historical deploy-anyway behavior rather than blocking a
166+
// release on a check we aren't sure about.
167+
if($head_sha !== "" && $expected_commit !== "" && $head_sha !== $expected_commit){
168+
_log("ABORTING DEPLOY: checked-out HEAD ($head_sha) does not match the ".
169+
"pushed commit ($expected_commit, from ref value $expected_sha) -- ".
170+
"the pull/checkout above likely ".
171+
"failed. Refusing to rebuild the container from stale source.\n");
172+
exit;
173+
}
174+
99175
//If we're using docker, then do someother stuff
100176
if($USE_DOCKER){
101177

@@ -195,7 +271,7 @@ function do_checkout_branch($req, $deployto, $operation){
195271
*/
196272
function determine_branch_name( $request) {
197273
// push request, branch is in request[ref]
198-
if (isset($reqest['ref'])) {
274+
if (isset($request['ref'])) {
199275
// strip out the refs/head nonsense -- doesn't look like bare
200276
// branch is listed anywhere in the request
201277
return preg_replace("|refs/heads/|", "", $request['ref']);
@@ -280,18 +356,24 @@ function do_clone($key,$url,$path) {
280356
* Log a message, only here to make some messages easy to turn off.
281357
*/
282358
function _trace($message) {
283-
if ($trace = TRUE) {
359+
// NOTE: was `if ($trace = TRUE)` -- an assignment, not a comparison, so it
360+
// was always true and could never be turned off (same =/== footgun fixed in
361+
// the deploy condition above). Default to on to preserve prior behavior;
362+
// config.php can silence it with `define('AUTODEPLOY_TRACE', false);`.
363+
if (defined('AUTODEPLOY_TRACE') ? AUTODEPLOY_TRACE : TRUE) {
284364
_log($message);
285-
}
365+
}
286366
}
287367

288368
/**
289369
* Log a message, only here to make some messages easy to turn off.
290370
*/
291371
function _debug($message) {
292-
if ($debug = TRUE) {
372+
// Same fix as _trace(): `$debug = TRUE` was an assignment, always true.
373+
// Default on; silence via `define('AUTODEPLOY_DEBUG', false);` in config.php.
374+
if (defined('AUTODEPLOY_DEBUG') ? AUTODEPLOY_DEBUG : TRUE) {
293375
_log($message);
294-
}
376+
}
295377
}
296378

297379
/**

0 commit comments

Comments
 (0)