From 4fc3d0a4a9624283c1ea7fe49c6e56ffc9eda15e Mon Sep 17 00:00:00 2001
From: Jonathan Miller <jmiller-moko@noreply.git.mokoconsulting.tech>
Date: Sat, 20 Jun 2026 23:40:54 -0500
Subject: [PATCH] fix: address review findings in deploy-and-verify.php

- Fix #1: replace rm -rf with cross-platform PHP removeDirectory()
- Fix #2: sanitize URL in audit log (log hostname only)
- Fix #3: remove unused buildHealthArgs() and $healthArgs
- Fix #4: add random suffix to snapshot dir name for uniqueness
- Fix #5: fix constructor to match CliFramework pattern (no args)
- Fix #6: trigger rollback on deploy failure (partial deploy risk)
---
 deploy/deploy-and-verify.php | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/deploy/deploy-and-verify.php b/deploy/deploy-and-verify.php
index 584f4a6..2315404 100644
--- a/deploy/deploy-and-verify.php
+++ b/deploy/deploy-and-verify.php
@@ -70,15 +70,14 @@ class DeployAndVerify extends CliFramework
             // Non-fatal — proceed without audit logging
         }
 
-        $this->audit('start', ['path' => $path, 'env' => $env, 'url' => $url]);
+        $this->audit('start', ['path' => $path, 'env' => $env, 'url' => parse_url($url, PHP_URL_HOST) ?? $url]);
 
         // ── Build subprocess args ────────────────────────────────────
         $deployArgs = $this->buildDeployArgs($path, $env, $config);
-        $healthArgs = $this->buildHealthArgs($url, $checks, $timeout);
 
         // ── Step 1: Backup ───────────────────────────────────────────
         $this->section('Step 1: Pre-deploy backup');
-        $snapshotDir = sys_get_temp_dir() . '/moko_deploy_snapshot_' . date('Ymd_His') . '_' . getmypid();
+        $snapshotDir = sys_get_temp_dir() . '/moko_deploy_snapshot_' . date('Ymd_His') . '_' . getmypid() . '_' . bin2hex(random_bytes(4));
 
         if ($this->dryRun) {
             $this->log('INFO', "[dry-run] Would create snapshot at {$snapshotDir}");
@@ -104,8 +103,11 @@ class DeployAndVerify extends CliFramework
             $deployExit = $this->runSubprocess('deploy-sftp.php', $deployArgs);
 
             if ($deployExit !== 0) {
-                $this->log('ERROR', 'Deploy failed — no rollback needed (files unchanged)');
+                $this->log('ERROR', 'Deploy failed — rolling back to pre-deploy state');
                 $this->audit('deploy_failed', ['exit_code' => $deployExit]);
+                $this->runSubprocess('rollback-joomla.php', array_merge(
+                    $deployArgs, ['--snapshot-dir', $snapshotDir]
+                ));
                 $this->cleanup($snapshotDir);
                 return self::EXIT_FAILURE;
             }
@@ -297,11 +299,6 @@ class DeployAndVerify extends CliFramework
         return $args;
     }
 
-    private function buildHealthArgs(string $url, string $checks, int $timeout): array
-    {
-        return ['--url', $url, '--checks', $checks, '--timeout', (string) $timeout];
-    }
-
     // ── Audit ────────────────────────────────────────────────────────
 
     private function audit(string $event, array $data): void
@@ -321,11 +318,27 @@ class DeployAndVerify extends CliFramework
     private function cleanup(string $snapshotDir): void
     {
         if (is_dir($snapshotDir)) {
-            exec(sprintf('rm -rf %s', escapeshellarg($snapshotDir)));
+            $this->removeDirectory($snapshotDir);
             $this->log('DEBUG', "Cleaned up snapshot: {$snapshotDir}");
         }
     }
+
+    private function removeDirectory(string $dir): void
+    {
+        $entries = scandir($dir);
+        if ($entries === false) {
+            return;
+        }
+        foreach ($entries as $entry) {
+            if ($entry === '.' || $entry === '..') {
+                continue;
+            }
+            $path = $dir . DIRECTORY_SEPARATOR . $entry;
+            is_dir($path) ? $this->removeDirectory($path) : unlink($path);
+        }
+        rmdir($dir);
+    }
 }
 
-$app = new DeployAndVerify('deploy_and_verify', 'Deploy with automatic health check and rollback');
+$app = new DeployAndVerify();
 exit($app->execute());