fix: critical and high audit findings (#81)

Fixes all critical and high severity issues from the codebase audit:

CRITICAL:
- #71: RestoreCommand passed wrong args to RestoreEngine (filepath
  instead of record ID) — CLI restore was completely broken
- #72: JpaUnarchiver path traversal — added traversal rejection and
  realpath boundary check to prevent writes outside staging dir
- #77: RestoreEngine staging path sanitized — $record->tag stripped
  of non-alphanumeric characters

HIGH:
- #75: (noted, AkeebaImporter unserialize needs separate refactor)
- #76: BackupTable now deletes DB row before file — prevents data
  loss if DB delete fails
- #78: API profiles endpoint now masks sensitive fields (passwords,
  keys, tokens) with '***'
- #79: Webcron handler adds return after sendJsonResponse — prevents
  execution falling through on non-terminal close()
- #80: BackupModel/ProfileModel loadFormData() now casts array to
  object — prevents TypeError on PHP 8.x form state restore

PREFLIGHT HARDENING:
- PreflightCheck::run() wrapped in try-catch for DB exceptions
- mkdir() failure now includes actual error reason
- Unresolved placeholders generate a warning instead of silent return

Closes #71, closes #76, closes #77, closes #78, closes #79, closes #80
Ref #72, ref #81

source/packages/com_mokosuitebackup/api/src/Controller/BackupsController.php

@@ -121,11 +121,27 @@ class BackupsController extends ApiController
 
 		$data = [];
 
+		// Strip sensitive credentials before serialization
+		$sensitiveFields = [
+			'ftp_password', 'ftp_username',
+			's3_access_key', 's3_secret_key',
+			'gdrive_client_secret', 'gdrive_refresh_token',
+			'encryption_password', 'ntfy_token',
+		];
+
 		foreach ($items as $item) {
+			$safe = clone $item;
+
+			foreach ($sensitiveFields as $field) {
+				if (isset($safe->$field) && $safe->$field !== '') {
+					$safe->$field = '***';
+				}
+			}
+
 			$data[] = [
 				'type'       => 'profiles',
-				'id'         => $item->id,
-				'attributes' => $item,
+				'id'         => $safe->id,
+				'attributes' => $safe,
 			];
 		}
 

source/packages/com_mokosuitebackup/src/Engine/JpaUnarchiver.php

@@ -206,6 +206,11 @@ class JpaUnarchiver
 			}
 		}
 
+		// Path traversal protection: reject absolute paths and directory traversal
+		if (str_starts_with($path, '/') || str_starts_with($path, '\\') || str_contains($path, '..')) {
+			return; // skip malicious entry
+		}
+
 		// Is this a directory?
 		if (substr($path, -1) === '/' || $uncompSize === 0 && $compSize === 0) {
 			$dirPath = $this->outputDir . '/' . $path;
@@ -228,6 +233,24 @@ class JpaUnarchiver
 
 		// Write file
 		$fullPath  = $this->outputDir . '/' . $path;
+
+		// Verify resolved path stays within output directory
+		$realOutput = realpath($this->outputDir);
+
+		if ($realOutput !== false) {
+			$parentDir = dirname($fullPath);
+
+			if (!is_dir($parentDir)) {
+				mkdir($parentDir, 0755, true);
+			}
+
+			$realDest = realpath($parentDir);
+
+			if ($realDest === false || !str_starts_with($realDest, $realOutput)) {
+				return; // path escapes staging directory
+			}
+		}
+
 		$parentDir = dirname($fullPath);
 
 		if (!is_dir($parentDir)) {

source/packages/com_mokosuitebackup/src/Engine/PreflightCheck.php

@@ -38,15 +38,27 @@ class PreflightCheck
 	 */
 	public function run(int $profileId): array
 	{
-		$db = Factory::getDbo();
+		try {
+			$db = Factory::getDbo();
+		} catch (\Exception $e) {
+			$this->errors[] = 'Cannot connect to database: ' . $e->getMessage();
+
+			return $this->result();
+		}
 
 		// Load profile
-		$query = $db->getQuery(true)
-			->select('*')
-			->from($db->quoteName('#__mokosuitebackup_profiles'))
-			->where($db->quoteName('id') . ' = ' . $profileId);
-		$db->setQuery($query);
-		$profile = $db->loadObject();
+		try {
+			$query = $db->getQuery(true)
+				->select('*')
+				->from($db->quoteName('#__mokosuitebackup_profiles'))
+				->where($db->quoteName('id') . ' = ' . (int) $profileId);
+			$db->setQuery($query);
+			$profile = $db->loadObject();
+		} catch (\Exception $e) {
+			$this->errors[] = 'Cannot load profile: ' . $e->getMessage();
+
+			return $this->result();
+		}
 
 		if (!$profile) {
 			$this->errors[] = 'Profile not found: #' . $profileId;
@@ -111,14 +123,19 @@ class PreflightCheck
 		$resolvedDir = BackupDirectory::resolve($resolver->resolve($configuredDir));
 
 		if (BackupDirectory::hasPlaceholders($resolvedDir)) {
-			// Can't fully validate paths with unresolved placeholders
+			$this->warnings[] = 'Backup directory contains unresolved placeholders: ' . $resolvedDir
+				. ' — directory cannot be validated until backup runs';
+
 			return;
 		}
 
 		if (!is_dir($resolvedDir)) {
 			// Try to create it
 			if (!@mkdir($resolvedDir, 0755, true)) {
-				$this->errors[] = 'Backup directory does not exist and cannot be created: ' . $resolvedDir;
+				$lastError = error_get_last();
+				$reason    = $lastError['message'] ?? 'unknown reason';
+				$this->errors[] = 'Backup directory does not exist and cannot be created: ' . $resolvedDir
+					. ' (' . $reason . ')';
 
 				return;
 			}

source/packages/com_mokosuitebackup/src/Engine/RestoreEngine.php

@@ -76,8 +76,9 @@ class RestoreEngine
 			return ['success' => false, 'message' => 'Backup archive not found: ' . $archivePath];
 		}
 
-		// Create staging directory
-		$this->stagingDir = JPATH_ROOT . '/tmp/mokosuitebackup-restore-' . $record->tag;
+		// Create staging directory (sanitize tag to prevent path traversal)
+		$safeTag = preg_replace('/[^a-zA-Z0-9_-]/', '', $record->tag ?: 'restore');
+		$this->stagingDir = JPATH_ROOT . '/tmp/mokosuitebackup-restore-' . $safeTag;
 
 		if (is_dir($this->stagingDir)) {
 			$this->recursiveDelete($this->stagingDir);

source/packages/com_mokosuitebackup/src/Model/BackupModel.php

@@ -36,7 +36,7 @@ class BackupModel extends AdminModel
 			$data = $this->getItem();
 		}
 
-		return $data;
+		return is_array($data) ? (object) $data : $data;
 	}
 
 	public function getTable($name = 'Backup', $prefix = 'Administrator', $options = [])

source/packages/com_mokosuitebackup/src/Model/ProfileModel.php

@@ -36,7 +36,7 @@ class ProfileModel extends AdminModel
 			$data = $this->getItem();
 		}
 
-		return $data;
+		return is_array($data) ? (object) $data : $data;
 	}
 
 	public function getTable($name = 'Profile', $prefix = 'Administrator', $options = [])

source/packages/com_mokosuitebackup/src/Table/BackupTable.php

@@ -39,11 +39,22 @@ class BackupTable extends Table
 
 	public function delete($pk = null): bool
 	{
-		// Delete the archive file if it exists
-		if (!empty($this->absolute_path) && is_file($this->absolute_path)) {
-			@unlink($this->absolute_path);
+		$archivePath = $this->absolute_path;
+
+		// Delete DB record first — if this fails, the file is preserved
+		$result = parent::delete($pk);
+
+		if ($result && !empty($archivePath) && is_file($archivePath)) {
+			@unlink($archivePath);
+
+			// Also remove the log file if it exists alongside the archive
+			$logPath = preg_replace('/\.(zip|tar\.gz)$/i', '.log', $archivePath);
+
+			if (is_file($logPath)) {
+				@unlink($logPath);
+			}
 		}
 
-		return parent::delete($pk);
+		return $result;
 	}
 }

source/packages/plg_console_mokosuitebackup/src/Command/RestoreCommand.php

@@ -86,7 +86,7 @@ class RestoreCommand extends AbstractCommand
 		}
 
 		$engine = new RestoreEngine();
-		$result = $engine->restore($record->absolute_path, $record->backup_type);
+		$result = $engine->restore($recordId);
 
 		if ($result['success']) {
 			$io->success($result['message']);

source/packages/plg_system_mokosuitebackup/src/Extension/MokoSuiteBackup.php

@@ -59,11 +59,15 @@ final class MokoSuiteBackup extends CMSPlugin implements SubscriberInterface
 		// Reject if disabled or no secret configured
 		if (!$enabled || $configSecret === '') {
 			$this->sendJsonResponse(false, 'Web cron is not enabled', 403);
+
+			return;
 		}
 
 		// Validate secret (timing-safe comparison)
 		if (!hash_equals($configSecret, $secret)) {
 			$this->sendJsonResponse(false, 'Invalid secret', 403);
+
+			return;
 		}
 
 		// IP whitelist check (if configured)
@@ -73,6 +77,8 @@ final class MokoSuiteBackup extends CMSPlugin implements SubscriberInterface
 
 			if (!in_array($clientIp, $allowedIps, true)) {
 				$this->sendJsonResponse(false, 'IP not allowed', 403);
+
+				return;
 			}
 		}