From 447f7b572e71362f976b164f49bc86b049e02d7d Mon Sep 17 00:00:00 2001 From: Jonathan Miller Date: Sun, 5 Jul 2026 20:34:04 -0500 Subject: [PATCH 1/2] feat: prune remote backups on retention + consolidate backup helpers Retention (RetentionManager and the plugin's hourly cleanup) only ever deleted local files; remote archives grew unbounded. Add an idempotent delete() to RemoteUploaderInterface + all four uploaders and have RetentionManager remove each pruned archive from the profile's enabled remotes (best-effort; failures logged). The shared restore.php is left in place since every backup overwrites it. (#229) Consolidate duplicated plumbing (#230): - New RemoteUploaderFactory replaces the createUploaderFromParams copy duplicated in BackupEngine and SteppedBackupEngine. - RetentionManager becomes the single retention authority (global-default fallback + pruneOrphans()); the system plugin delegates to it and its duplicate doCleanup()/deleteBackupRecord() logic is removed. - Backend controller, API controller and legacy cli/mokosuitebackup.php now run through the shared BackupRunner instead of BackupEngine directly. Refs #229 #230 Claude-Session: https://claude.ai/code/session_01WbGBN9VyRK61zczYWcCQ2i --- CHANGELOG.md | 9 + .../api/src/Controller/BackupsController.php | 5 +- .../cli/mokosuitebackup.php | 5 +- .../src/Controller/BackupsController.php | 5 +- .../src/Engine/BackupEngine.php | 19 +-- .../src/Engine/FtpUploader.php | 46 +++++ .../src/Engine/GoogleDriveUploader.php | 53 ++++++ .../src/Engine/RemoteUploaderFactory.php | 69 ++++++++ .../src/Engine/RemoteUploaderInterface.php | 15 ++ .../src/Engine/RetentionManager.php | 160 ++++++++++++++++-- .../src/Engine/S3Uploader.php | 48 ++++++ .../src/Engine/SftpUploader.php | 34 ++++ .../src/Engine/SteppedBackupEngine.php | 19 +-- .../src/Extension/MokoSuiteBackup.php | 107 ++---------- 14 files changed, 440 insertions(+), 154 deletions(-) create mode 100644 source/packages/com_mokosuitebackup/src/Engine/RemoteUploaderFactory.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d684f15..cb620142 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## [Unreleased] +### Added +- Retention now prunes **remote** copies too: when a backup is pruned by age/count, its archive is deleted from every enabled remote destination (SFTP / FTP / S3 / Google Drive), not just the local copy. Each uploader gained an idempotent `delete()` method (already-absent file = success), and removal is best-effort — a failing destination is logged but never blocks local pruning. The shared standalone `restore.php` is intentionally left in place (every backup overwrites it, so newer backups still depend on it). (#229) + +### Changed +- Consolidated backup plumbing into shared helpers (#230): + - New `RemoteUploaderFactory` replaces the `createUploaderFromParams()` copy that was duplicated in `BackupEngine` and `SteppedBackupEngine`. + - `RetentionManager` is now the single retention authority — it takes the global `max_age_days`/`max_backups` fallback and gained `pruneOrphans()`; the system plugin's hourly cleanup delegates to it and its duplicate `deleteBackupRecord()` logic is removed. + - The backend controller, Web Services API controller, and legacy `cli/mokosuitebackup.php` now run backups through the shared `BackupRunner` (gaining the normalized complete/warning/fail status) instead of instantiating `BackupEngine` directly. + ### Fixed - Standalone restore script generation no longer aborts backups with `str_replace() expects at least 3 arguments, 2 given`. `MokoRestore::generateStandaloneScript()` had a `str_replace()` call (the "Backup Archive" pre-check rewrite) that was missing its `$php` subject argument, so **every** standalone-mode backup fatally errored while "Generating standalone restore.php…" — the archive still finalized and uploaded, but no `restore.php` was ever produced (the true root cause behind #226). (#226) - Remote upload: the standalone restore script upload is no longer silent — its result is now checked and logged, and a failed restore-script upload marks the backup as `warning` (previously the result was discarded, so a missing restore script on the remote went unreported while the archive still showed success). (#226) diff --git a/source/packages/com_mokosuitebackup/api/src/Controller/BackupsController.php b/source/packages/com_mokosuitebackup/api/src/Controller/BackupsController.php index 2cc5ebe5..5bce4504 100644 --- a/source/packages/com_mokosuitebackup/api/src/Controller/BackupsController.php +++ b/source/packages/com_mokosuitebackup/api/src/Controller/BackupsController.php @@ -13,7 +13,7 @@ namespace Joomla\Component\MokoSuiteBackup\Api\Controller; defined('_JEXEC') or die; use Joomla\CMS\MVC\Controller\ApiController; -use Joomla\Component\MokoSuiteBackup\Administrator\Engine\BackupEngine; +use Joomla\Component\MokoSuiteBackup\Administrator\Service\BackupRunner; class BackupsController extends ApiController { @@ -38,8 +38,7 @@ class BackupsController extends ApiController $profileId = (int) ($data['profile'] ?? 1); $description = $data['description'] ?? 'API backup ' . date('Y-m-d H:i:s'); - $engine = new BackupEngine(); - $result = $engine->run($profileId, $description, 'api'); + $result = (new BackupRunner())->run($profileId, $description, 'api'); if ($result['success']) { $this->app->setHeader('status', 200); diff --git a/source/packages/com_mokosuitebackup/cli/mokosuitebackup.php b/source/packages/com_mokosuitebackup/cli/mokosuitebackup.php index 29f1c936..7e78076f 100644 --- a/source/packages/com_mokosuitebackup/cli/mokosuitebackup.php +++ b/source/packages/com_mokosuitebackup/cli/mokosuitebackup.php @@ -30,7 +30,7 @@ if (!defined('JPATH_BASE')) { require_once JPATH_BASE . '/includes/framework.php'; use Joomla\CMS\Factory; -use Joomla\Component\MokoSuiteBackup\Administrator\Engine\BackupEngine; +use Joomla\Component\MokoSuiteBackup\Administrator\Service\BackupRunner; // Parse CLI arguments $profileId = 1; @@ -56,8 +56,7 @@ echo "Profile: {$profileId}\n"; echo "Description: {$description}\n"; echo "Starting backup...\n\n"; -$engine = new BackupEngine(); -$result = $engine->run($profileId, $description, 'cli'); +$result = (new BackupRunner())->run($profileId, $description, 'cli'); if ($result['success']) { echo "SUCCESS: " . $result['message'] . "\n"; diff --git a/source/packages/com_mokosuitebackup/src/Controller/BackupsController.php b/source/packages/com_mokosuitebackup/src/Controller/BackupsController.php index 59a725b5..31086528 100644 --- a/source/packages/com_mokosuitebackup/src/Controller/BackupsController.php +++ b/source/packages/com_mokosuitebackup/src/Controller/BackupsController.php @@ -16,8 +16,8 @@ use Joomla\CMS\Language\Text; use Joomla\CMS\MVC\Controller\AdminController; use Joomla\CMS\Router\Route; use Joomla\CMS\Session\Session; -use Joomla\Component\MokoSuiteBackup\Administrator\Engine\BackupEngine; use Joomla\Component\MokoSuiteBackup\Administrator\Engine\RestoreEngine; +use Joomla\Component\MokoSuiteBackup\Administrator\Service\BackupRunner; class BackupsController extends AdminController { @@ -54,8 +54,7 @@ class BackupsController extends AdminController $profileId = $this->input->getInt('profile_id', 1); $description = $this->input->getString('description', ''); - $engine = new BackupEngine(); - $result = $engine->run($profileId, $description, 'backend'); + $result = (new BackupRunner())->run($profileId, $description, 'backend'); // Surface preflight warnings as Joomla messages if (!empty($result['warnings'])) { diff --git a/source/packages/com_mokosuitebackup/src/Engine/BackupEngine.php b/source/packages/com_mokosuitebackup/src/Engine/BackupEngine.php index 71f014ab..5fd38bfa 100644 --- a/source/packages/com_mokosuitebackup/src/Engine/BackupEngine.php +++ b/source/packages/com_mokosuitebackup/src/Engine/BackupEngine.php @@ -511,24 +511,7 @@ class BackupEngine */ private function createUploaderFromParams(string $type, array $params): RemoteUploaderInterface { - $prefixMap = ['ftp' => 'ftp_', 'sftp' => 'sftp_', 's3' => 's3_', 'google_drive' => 'gdrive_']; - $prefix = $prefixMap[$type] ?? ''; - - $prefixed = []; - - foreach ($params as $key => $value) { - $prefixed[$prefix . $key] = $value; - } - - $fake = (object) $prefixed; - - return match ($type) { - 'ftp' => new FtpUploader($fake), - 'sftp' => new SftpUploader($fake), - 'google_drive' => new GoogleDriveUploader($fake), - 's3' => new S3Uploader($fake), - default => throw new \InvalidArgumentException('Unknown remote storage type: ' . $type), - }; + return RemoteUploaderFactory::create($type, $params); } /** diff --git a/source/packages/com_mokosuitebackup/src/Engine/FtpUploader.php b/source/packages/com_mokosuitebackup/src/Engine/FtpUploader.php index 465f0340..265c399c 100644 --- a/source/packages/com_mokosuitebackup/src/Engine/FtpUploader.php +++ b/source/packages/com_mokosuitebackup/src/Engine/FtpUploader.php @@ -89,6 +89,52 @@ class FtpUploader implements RemoteUploaderInterface } } + public function delete(string $remoteName): array + { + if (!extension_loaded('ftp')) { + return ['success' => false, 'message' => 'PHP ext-ftp is required for FTP deletes. Enable it in php.ini.']; + } + + if (empty($this->host)) { + return ['success' => false, 'message' => 'FTP host is not configured']; + } + + $conn = $this->connect(); + + if (!$conn) { + return ['success' => false, 'message' => 'Failed to connect to FTP server ' . $this->host . ':' . $this->port]; + } + + try { + if (!@ftp_login($conn, $this->username, $this->password)) { + throw new \RuntimeException('FTP login failed for user: ' . $this->username); + } + + if ($this->passive) { + ftp_pasv($conn, true); + } + + $remoteFile = $this->remotePath . '/' . $remoteName; + + /* Already-absent files: ftp_delete returns false, but if the file + isn't there the retention goal is already met, so probe size to + decide. ftp_size returns -1 when the file does not exist. */ + if (@ftp_size($conn, $remoteFile) < 0) { + return ['success' => true, 'message' => 'FTP: nothing to delete (' . $remoteFile . ')']; + } + + if (!@ftp_delete($conn, $remoteFile)) { + throw new \RuntimeException('ftp_delete failed for: ' . $remoteFile); + } + + return ['success' => true, 'message' => 'Deleted from FTP: ' . $remoteFile]; + } catch (\Throwable $e) { + return ['success' => false, 'message' => 'FTP delete failed: ' . $e->getMessage()]; + } finally { + @ftp_close($conn); + } + } + public function testConnection(): array { if (empty($this->host)) { diff --git a/source/packages/com_mokosuitebackup/src/Engine/GoogleDriveUploader.php b/source/packages/com_mokosuitebackup/src/Engine/GoogleDriveUploader.php index 7d1d83d9..c57d1478 100644 --- a/source/packages/com_mokosuitebackup/src/Engine/GoogleDriveUploader.php +++ b/source/packages/com_mokosuitebackup/src/Engine/GoogleDriveUploader.php @@ -81,6 +81,59 @@ class GoogleDriveUploader implements RemoteUploaderInterface } } + public function delete(string $remoteName): array + { + if (!extension_loaded('curl')) { + return ['success' => false, 'message' => 'PHP ext-curl is required for Google Drive deletes. Enable it in php.ini.']; + } + + if (empty($this->clientId) || empty($this->refreshToken)) { + return ['success' => false, 'message' => 'Google Drive credentials not configured']; + } + + try { + $accessToken = $this->getAccessToken(); + + /* Drive has no path addressing — resolve the file id(s) by name + within the configured folder, then delete each match. */ + $q = "name = '" . str_replace("'", "\\'", $remoteName) . "' and trashed = false"; + + if (!empty($this->folderId)) { + $q .= " and '" . $this->folderId . "' in parents"; + } + + $listUrl = self::API_URL . '/files?q=' . rawurlencode($q) . '&fields=' . rawurlencode('files(id,name)') . '&pageSize=25'; + $response = $this->curlRequest('GET', $listUrl, null, [ + 'Authorization: Bearer ' . $accessToken, + ]); + + if ($response['code'] !== 200) { + throw new \RuntimeException('Drive file lookup failed (HTTP ' . $response['code'] . ')'); + } + + $files = json_decode($response['body'], true)['files'] ?? []; + + if (empty($files)) { + return ['success' => true, 'message' => 'Google Drive: nothing to delete (' . $remoteName . ')']; + } + + foreach ($files as $file) { + $del = $this->curlRequest('DELETE', self::API_URL . '/files/' . rawurlencode($file['id']), null, [ + 'Authorization: Bearer ' . $accessToken, + ]); + + /* 204 = deleted, 404 = already gone — both acceptable. */ + if ($del['code'] !== 204 && $del['code'] !== 200 && $del['code'] !== 404) { + return ['success' => false, 'message' => 'Google Drive delete failed for ' . $remoteName . ' (HTTP ' . $del['code'] . ')']; + } + } + + return ['success' => true, 'message' => 'Deleted from Google Drive: ' . $remoteName]; + } catch (\Throwable $e) { + return ['success' => false, 'message' => 'Google Drive delete failed: ' . $e->getMessage()]; + } + } + public function testConnection(): array { if (empty($this->clientId) || empty($this->refreshToken)) { diff --git a/source/packages/com_mokosuitebackup/src/Engine/RemoteUploaderFactory.php b/source/packages/com_mokosuitebackup/src/Engine/RemoteUploaderFactory.php new file mode 100644 index 00000000..84e39c71 --- /dev/null +++ b/source/packages/com_mokosuitebackup/src/Engine/RemoteUploaderFactory.php @@ -0,0 +1,69 @@ + + * @copyright Copyright (C) 2026 Moko Consulting. All rights reserved. + * @license GNU General Public License version 3 or later; see LICENSE + */ + +namespace Joomla\Component\MokoSuiteBackup\Administrator\Engine; + +defined('_JEXEC') or die; + +/** + * Builds a remote uploader from a `#__mokosuitebackup_remotes` row. + * + * This was previously a private method duplicated in both BackupEngine and + * SteppedBackupEngine (and needed a third time by RetentionManager for remote + * deletion). It is centralised here so the type→class map and the param + * key-prefixing convention live in exactly one place. + * + * The remotes table stores per-type settings as a flat JSON object with + * un-prefixed keys (e.g. `{"host":"…","path":"/backups"}`). The concrete + * uploaders, however, read prefixed keys off a profile-shaped object + * (e.g. `sftp_host`, `s3_bucket`). This factory bridges the two. + */ +final class RemoteUploaderFactory +{ + /** + * Map of remote type → the key prefix its uploader expects. + */ + private const PREFIX_MAP = [ + 'ftp' => 'ftp_', + 'sftp' => 'sftp_', + 's3' => 's3_', + 'google_drive' => 'gdrive_', + ]; + + /** + * Create an uploader for the given remote type from decoded JSON params. + * + * @param string $type Remote type: ftp, sftp, s3, google_drive + * @param array $params Key-value params decoded from the remote's JSON + * + * @return RemoteUploaderInterface + * + * @throws \InvalidArgumentException On an unknown remote type + */ + public static function create(string $type, array $params): RemoteUploaderInterface + { + $prefix = self::PREFIX_MAP[$type] ?? ''; + $prefixed = []; + + foreach ($params as $key => $value) { + $prefixed[$prefix . $key] = $value; + } + + $fake = (object) $prefixed; + + return match ($type) { + 'ftp' => new FtpUploader($fake), + 'sftp' => new SftpUploader($fake), + 'google_drive' => new GoogleDriveUploader($fake), + 's3' => new S3Uploader($fake), + default => throw new \InvalidArgumentException('Unknown remote storage type: ' . $type), + }; + } +} diff --git a/source/packages/com_mokosuitebackup/src/Engine/RemoteUploaderInterface.php b/source/packages/com_mokosuitebackup/src/Engine/RemoteUploaderInterface.php index fdc67a55..9b64956e 100644 --- a/source/packages/com_mokosuitebackup/src/Engine/RemoteUploaderInterface.php +++ b/source/packages/com_mokosuitebackup/src/Engine/RemoteUploaderInterface.php @@ -24,6 +24,21 @@ interface RemoteUploaderInterface */ public function upload(string $localPath, string $remoteName): array; + /** + * Delete a previously-uploaded file from remote storage. + * + * Used by retention pruning so that expiring a local backup also removes + * its archive from every remote destination. Implementations should treat + * an already-absent remote file as success (idempotent delete), so a + * partially-cleaned destination never blocks pruning. + * + * @param string $remoteName Filename on the remote end (the same + * $remoteName that was passed to upload()) + * + * @return array{success: bool, message: string} + */ + public function delete(string $remoteName): array; + /** * Test the connection / credentials without uploading. * diff --git a/source/packages/com_mokosuitebackup/src/Engine/RetentionManager.php b/source/packages/com_mokosuitebackup/src/Engine/RetentionManager.php index 8a611ba9..d3f1ee2a 100644 --- a/source/packages/com_mokosuitebackup/src/Engine/RetentionManager.php +++ b/source/packages/com_mokosuitebackup/src/Engine/RetentionManager.php @@ -15,32 +15,54 @@ defined('_JEXEC') or die; use Joomla\Component\MokoSuiteBackup\Administrator\Utility\BackupDirectory; /** - * Enforces per-profile backup retention. + * Enforces per-profile backup retention — the single retention authority. * * A profile may cap retained backups by age (retention_days) and/or by - * number of copies (retention_count). A backup is pruned when EITHER rule - * matches: it is older than retention_days OR it falls outside the newest - * retention_count copies. Deleting a record also removes its archive and - * log file, mirroring the Backup table's delete(). + * number of copies (retention_count). When a profile leaves a rule at 0 the + * caller-supplied global default is used instead. A backup is pruned when + * EITHER rule matches: it is older than the effective days OR it falls outside + * the newest effective count of copies. + * + * Pruning a record removes, in this order: + * 1. the DB row (#__mokosuitebackup_records), + * 2. the local archive + its .log file, and + * 3. the archive copy on every enabled remote destination for the profile. + * + * The standalone restore script (restore.php) is intentionally NOT deleted + * from remotes: it is a single, fixed-name file that every backup overwrites, + * so newer backups still depend on it — removing it while pruning an old + * archive would break restore for the copies that remain. + * + * Because records do not store which destinations they were uploaded to, + * remote deletion targets the profile's *currently enabled* remotes. Remote + * deletes are idempotent (an already-absent file counts as success) and + * best-effort: a failing remote is logged but never blocks local pruning. */ final class RetentionManager { /** * Prune old backups for a profile according to its retention settings. * - * Called after a backup completes. Only 'complete' and 'warning' records - * are considered — pending/running/failed records are never pruned here. + * Called after a backup completes and from the periodic admin-side cleanup. + * Only 'complete' and 'warning' records are considered — pending/running/ + * failed records are never pruned here. * - * @param object $db Database driver - * @param object $profile Profile row (needs id, retention_days, retention_count) + * @param object $db Database driver + * @param object $profile Profile row (needs id, retention_days, retention_count) + * @param int $globalDays Fallback max age (days) when the profile's is 0 + * @param int $globalCount Fallback max copies when the profile's is 0 * * @return int Number of backup records deleted */ - public static function prune(object $db, object $profile): int + public static function prune(object $db, object $profile, int $globalDays = 0, int $globalCount = 0): int { $days = (int) ($profile->retention_days ?? 0); $count = (int) ($profile->retention_count ?? 0); + // A profile value of 0 means "use the global default". + $days = $days > 0 ? $days : $globalDays; + $count = $count > 0 ? $count : $globalCount; + // No retention configured — nothing to do. if ($days <= 0 && $count <= 0) { return 0; @@ -48,7 +70,7 @@ final class RetentionManager // Newest first, so the index is the copy's position from the top. $query = $db->getQuery(true) - ->select($db->quoteName(['id', 'absolute_path', 'backupstart'])) + ->select($db->quoteName(['id', 'archivename', 'absolute_path', 'backupstart'])) ->from($db->quoteName('#__mokosuitebackup_records')) ->where($db->quoteName('profile_id') . ' = ' . (int) $profile->id) ->where($db->quoteName('status') . ' IN (' . implode(',', array_map([$db, 'quote'], ['complete', 'warning'])) . ')') @@ -61,6 +83,7 @@ final class RetentionManager } $cutoffTs = $days > 0 ? (time() - ($days * 86400)) : null; + $remotes = self::loadEnabledRemotes($db, (int) $profile->id); $deleted = 0; foreach ($records as $index => $record) { @@ -72,7 +95,7 @@ final class RetentionManager continue; } - if (self::deleteRecord($db, $record)) { + if (self::deleteRecord($db, $record, $remotes)) { $deleted++; } } @@ -81,12 +104,54 @@ final class RetentionManager } /** - * Delete a single backup record and its on-disk archive + log file. + * Delete backup records whose profile no longer exists. * - * The DB row is removed first; the files are only unlinked if that - * succeeds, so a failed delete never orphans the record from its files. + * Orphans have no owning profile and therefore no remote destinations to + * reconcile, so only the local archive/log and DB row are removed. + * + * @param object $db Database driver + * + * @return int Number of orphaned records deleted */ - private static function deleteRecord(object $db, object $record): bool + public static function pruneOrphans(object $db): int + { + $query = $db->getQuery(true) + ->select($db->quoteName(['r.id', 'r.archivename', 'r.absolute_path'], ['id', 'archivename', 'absolute_path'])) + ->from($db->quoteName('#__mokosuitebackup_records', 'r')) + ->join( + 'LEFT', + $db->quoteName('#__mokosuitebackup_profiles', 'p') + . ' ON ' . $db->quoteName('p.id') . ' = ' . $db->quoteName('r.profile_id') + ) + ->where($db->quoteName('p.id') . ' IS NULL') + ->where($db->quoteName('r.status') . ' IN (' . implode(',', array_map([$db, 'quote'], ['complete', 'warning'])) . ')'); + $db->setQuery($query); + $orphans = $db->loadObjectList() ?: []; + + $deleted = 0; + + foreach ($orphans as $record) { + if (self::deleteRecord($db, $record, [])) { + $deleted++; + } + } + + return $deleted; + } + + /** + * Delete a single backup record, its on-disk archive + log file, and the + * archive copy on each enabled remote destination. + * + * The DB row is removed first; the local files are only unlinked if that + * succeeds, so a failed delete never orphans the record from its files. + * Remote deletion is best-effort and runs regardless. + * + * @param object $db Database driver + * @param object $record Record row (needs id, archivename, absolute_path) + * @param object[] $remotes Enabled remote rows (type, params) for the profile + */ + private static function deleteRecord(object $db, object $record, array $remotes): bool { $query = $db->getQuery(true) ->delete($db->quoteName('#__mokosuitebackup_records')) @@ -113,6 +178,69 @@ final class RetentionManager } } + self::deleteFromRemotes($record, $remotes); + return true; } + + /** + * Remove the record's archive from every enabled remote destination. + * + * Best-effort: each remote is tried independently and failures are logged, + * never thrown, so one unreachable destination can't stall retention. + * + * @param object $record Record row (needs archivename) + * @param object[] $remotes Enabled remote rows (type, params) + */ + private static function deleteFromRemotes(object $record, array $remotes): void + { + $archiveName = (string) ($record->archivename ?? ''); + + if ($archiveName === '' || empty($remotes)) { + return; + } + + foreach ($remotes as $remote) { + try { + $params = json_decode((string) ($remote->params ?? ''), true) ?: []; + $uploader = RemoteUploaderFactory::create((string) $remote->type, $params); + $result = $uploader->delete($archiveName); + + if (empty($result['success'])) { + error_log( + 'MokoSuiteBackup: retention could not delete ' . $archiveName + . ' from ' . $remote->type . ' remote: ' . ($result['message'] ?? 'unknown error') + ); + } + } catch (\Throwable $e) { + error_log( + 'MokoSuiteBackup: retention remote-delete error for ' . $archiveName + . ' (' . ($remote->type ?? '?') . '): ' . $e->getMessage() + ); + } + } + } + + /** + * Load the profile's enabled remote destinations (type + params only). + * + * Returns an empty array if the remotes table is missing (very old installs) + * or the profile has none, so callers can iterate unconditionally. + */ + private static function loadEnabledRemotes(object $db, int $profileId): array + { + try { + $query = $db->getQuery(true) + ->select($db->quoteName(['type', 'params'])) + ->from($db->quoteName('#__mokosuitebackup_remotes')) + ->where($db->quoteName('profile_id') . ' = ' . $profileId) + ->where($db->quoteName('enabled') . ' = 1') + ->order($db->quoteName('ordering') . ' ASC'); + $db->setQuery($query); + + return $db->loadObjectList() ?: []; + } catch (\Throwable $e) { + return []; + } + } } diff --git a/source/packages/com_mokosuitebackup/src/Engine/S3Uploader.php b/source/packages/com_mokosuitebackup/src/Engine/S3Uploader.php index 5c70a2c5..cea187e7 100644 --- a/source/packages/com_mokosuitebackup/src/Engine/S3Uploader.php +++ b/source/packages/com_mokosuitebackup/src/Engine/S3Uploader.php @@ -75,6 +75,54 @@ class S3Uploader implements RemoteUploaderInterface } } + public function delete(string $remoteName): array + { + if (!extension_loaded('curl')) { + return ['success' => false, 'message' => 'PHP ext-curl is required for S3 deletes. Enable it in php.ini.']; + } + + if (empty($this->accessKey) || empty($this->secretKey) || empty($this->bucket)) { + return ['success' => false, 'message' => 'S3 credentials or bucket not configured']; + } + + try { + $objectKey = ($this->path ? $this->path . '/' : '') . $remoteName; + $url = $this->getObjectUrl($objectKey); + $headers = $this->signRequest('DELETE', $url, hash('sha256', '')); + + $ch = curl_init(); + curl_setopt_array($ch, [ + CURLOPT_URL => $url, + CURLOPT_CUSTOMREQUEST => 'DELETE', + CURLOPT_RETURNTRANSFER => true, + CURLOPT_HTTPHEADER => $headers, + CURLOPT_TIMEOUT => 60, + ]); + + $response = curl_exec($ch); + $code = curl_getinfo($ch, CURLINFO_HTTP_CODE); + + if (curl_errno($ch)) { + $error = curl_error($ch); + curl_close($ch); + + throw new \RuntimeException('cURL error: ' . $error); + } + + curl_close($ch); + + /* S3 DELETE returns 204 on success and — by design — also 204 when + the key never existed. 404 is treated as already-gone too. */ + if ($code === 204 || $code === 200 || $code === 404) { + return ['success' => true, 'message' => 'Deleted from S3: s3://' . $this->bucket . '/' . $objectKey]; + } + + return ['success' => false, 'message' => 'S3 DELETE failed (HTTP ' . $code . '): ' . substr((string) $response, 0, 300)]; + } catch (\Throwable $e) { + return ['success' => false, 'message' => 'S3 delete failed: ' . $e->getMessage()]; + } + } + public function testConnection(): array { if (empty($this->accessKey) || empty($this->secretKey) || empty($this->bucket)) { diff --git a/source/packages/com_mokosuitebackup/src/Engine/SftpUploader.php b/source/packages/com_mokosuitebackup/src/Engine/SftpUploader.php index d32f4c64..f72e8960 100644 --- a/source/packages/com_mokosuitebackup/src/Engine/SftpUploader.php +++ b/source/packages/com_mokosuitebackup/src/Engine/SftpUploader.php @@ -100,6 +100,40 @@ class SftpUploader implements RemoteUploaderInterface } } + public function delete(string $remoteName): array + { + if (empty($this->host) || empty($this->username)) { + return ['success' => false, 'message' => 'SFTP is not configured']; + } + + $keyFile = null; + + try { + if (!empty($this->keyData)) { + $keyFile = $this->writeTempKey(); + } + + /* rm -f exits 0 even when the file is already gone, so an + already-deleted remote archive is treated as success. */ + $remoteFile = $this->remotePath . '/' . $remoteName; + $cmd = $this->buildSshCommand('rm -f ' . escapeshellarg($remoteFile), $keyFile); + + $output = []; + $exitCode = 0; + exec($cmd . ' 2>&1', $output, $exitCode); + + if ($exitCode !== 0) { + return ['success' => false, 'message' => 'SFTP delete failed: ' . implode(' ', $output)]; + } + + return ['success' => true, 'message' => 'Deleted from SFTP: ' . $remoteFile]; + } catch (\Throwable $e) { + return ['success' => false, 'message' => 'SFTP delete failed: ' . $e->getMessage()]; + } finally { + $this->cleanupTempKey($keyFile); + } + } + public function testConnection(): array { if (empty($this->host)) { diff --git a/source/packages/com_mokosuitebackup/src/Engine/SteppedBackupEngine.php b/source/packages/com_mokosuitebackup/src/Engine/SteppedBackupEngine.php index bbcf9069..811a8464 100644 --- a/source/packages/com_mokosuitebackup/src/Engine/SteppedBackupEngine.php +++ b/source/packages/com_mokosuitebackup/src/Engine/SteppedBackupEngine.php @@ -806,24 +806,7 @@ class SteppedBackupEngine */ private function createUploaderFromParams(string $type, array $params): RemoteUploaderInterface { - $prefixMap = ['ftp' => 'ftp_', 'sftp' => 'sftp_', 's3' => 's3_', 'google_drive' => 'gdrive_']; - $prefix = $prefixMap[$type] ?? ''; - - $prefixed = []; - - foreach ($params as $key => $value) { - $prefixed[$prefix . $key] = $value; - } - - $fake = (object) $prefixed; - - return match ($type) { - 'ftp' => new FtpUploader($fake), - 'sftp' => new SftpUploader($fake), - 'google_drive' => new GoogleDriveUploader($fake), - 's3' => new S3Uploader($fake), - default => throw new \InvalidArgumentException('Unknown remote storage type: ' . $type), - }; + return RemoteUploaderFactory::create($type, $params); } } diff --git a/source/packages/plg_system_mokosuitebackup/src/Extension/MokoSuiteBackup.php b/source/packages/plg_system_mokosuitebackup/src/Extension/MokoSuiteBackup.php index 1a97838f..6ec0c2d6 100644 --- a/source/packages/plg_system_mokosuitebackup/src/Extension/MokoSuiteBackup.php +++ b/source/packages/plg_system_mokosuitebackup/src/Extension/MokoSuiteBackup.php @@ -15,6 +15,7 @@ defined('_JEXEC') or die; use Joomla\CMS\Component\ComponentHelper; use Joomla\CMS\Factory; use Joomla\CMS\Plugin\CMSPlugin; +use Joomla\Component\MokoSuiteBackup\Administrator\Engine\RetentionManager; use Joomla\Component\MokoSuiteBackup\Administrator\Service\BackupRunner; use Joomla\Event\Event; use Joomla\Event\SubscriberInterface; @@ -241,108 +242,28 @@ final class MokoSuiteBackup extends CMSPlugin implements SubscriberInterface private function doCleanup(): void { - $db = Factory::getDbo(); - $globalMaxAge = (int) ComponentHelper::getParams('com_mokosuitebackup')->get('max_age_days', 30); - $globalMaxCount = (int) ComponentHelper::getParams('com_mokosuitebackup')->get('max_backups', 10); + $db = Factory::getDbo(); + $params = ComponentHelper::getParams('com_mokosuitebackup'); + $globalMaxAge = (int) $params->get('max_age_days', 30); + $globalMaxCount = (int) $params->get('max_backups', 10); - // Load all published profiles with their retention settings + // Load all published profiles with their retention settings. $query = $db->getQuery(true) - ->select([$db->quoteName('id'), $db->quoteName('retention_days'), $db->quoteName('retention_count')]) + ->select($db->quoteName(['id', 'retention_days', 'retention_count'])) ->from($db->quoteName('#__mokosuitebackup_profiles')) ->where($db->quoteName('published') . ' = 1'); $db->setQuery($query); - $profiles = $db->loadObjectList(); + $profiles = $db->loadObjectList() ?: []; + // Delegate to the single retention authority. RetentionManager prunes by + // age OR count (with these globals as the per-profile fallback) and also + // removes each pruned archive from the profile's remote destinations. foreach ($profiles as $profile) { - $maxAge = (int) $profile->retention_days > 0 ? (int) $profile->retention_days : $globalMaxAge; - $maxCount = (int) $profile->retention_count > 0 ? (int) $profile->retention_count : $globalMaxCount; - $pid = (int) $profile->id; - - // Delete by age for this profile - $cutoff = date('Y-m-d H:i:s', strtotime("-{$maxAge} days")); - $query = $db->getQuery(true) - ->select('id, absolute_path') - ->from($db->quoteName('#__mokosuitebackup_records')) - ->where($db->quoteName('profile_id') . ' = ' . $pid) - ->where($db->quoteName('backupstart') . ' < ' . $db->quote($cutoff)) - ->where($db->quoteName('status') . ' = ' . $db->quote('complete')); - $db->setQuery($query); - $expired = $db->loadObjectList(); - - foreach ($expired as $record) { - $this->deleteBackupRecord($db, $record); - } - - // Enforce max count for this profile (keep newest) - $query = $db->getQuery(true) - ->select('COUNT(*)') - ->from($db->quoteName('#__mokosuitebackup_records')) - ->where($db->quoteName('profile_id') . ' = ' . $pid) - ->where($db->quoteName('status') . ' = ' . $db->quote('complete')); - $db->setQuery($query); - $totalCount = (int) $db->loadResult(); - - if ($totalCount > $maxCount) { - $excess = $totalCount - $maxCount; - $query = $db->getQuery(true) - ->select('id, absolute_path') - ->from($db->quoteName('#__mokosuitebackup_records')) - ->where($db->quoteName('profile_id') . ' = ' . $pid) - ->where($db->quoteName('status') . ' = ' . $db->quote('complete')) - ->order($db->quoteName('backupstart') . ' ASC'); - $db->setQuery($query, 0, $excess); - $oldest = $db->loadObjectList(); - - foreach ($oldest as $record) { - $this->deleteBackupRecord($db, $record); - } - } + RetentionManager::prune($db, $profile, $globalMaxAge, $globalMaxCount); } - // Also clean up orphaned records (profile deleted but records remain) - $query = $db->getQuery(true) - ->select('r.id, r.absolute_path') - ->from($db->quoteName('#__mokosuitebackup_records', 'r')) - ->join('LEFT', $db->quoteName('#__mokosuitebackup_profiles', 'p') . ' ON p.id = r.profile_id') - ->where('p.id IS NULL') - ->where($db->quoteName('r.status') . ' = ' . $db->quote('complete')); - $db->setQuery($query); - $orphans = $db->loadObjectList(); - - foreach ($orphans as $record) { - $this->deleteBackupRecord($db, $record); - } - } - - /** - * Delete a backup record and its archive file. - */ - private function deleteBackupRecord(object $db, object $record): void - { - if (!empty($record->absolute_path) && is_file($record->absolute_path)) { - if (!@unlink($record->absolute_path)) { - error_log('MokoSuiteBackup: Could not delete backup file (id=' . $record->id . '): ' . $record->absolute_path); - - return; - } - - $logPath = preg_replace('/\.(zip|tar\.gz)$/i', '.log', $record->absolute_path); - - if (is_file($logPath)) { - @unlink($logPath); - } - } - - try { - $db->setQuery( - $db->getQuery(true) - ->delete($db->quoteName('#__mokosuitebackup_records')) - ->where($db->quoteName('id') . ' = ' . (int) $record->id) - ); - $db->execute(); - } catch (\Exception $e) { - error_log('MokoSuiteBackup: Could not delete backup record ' . $record->id . ': ' . $e->getMessage()); - } + // Records whose profile was deleted (local files + DB row only). + RetentionManager::pruneOrphans($db); } /** -- 2.52.0 From 7a829b6eca0d67ff75c35162aa637e64f673e27b Mon Sep 17 00:00:00 2001 From: "mokogitea-actions[bot]" Date: Mon, 6 Jul 2026 01:34:49 +0000 Subject: [PATCH 2/2] chore(version): pre-release bump to 02.58.06-dev [skip ci] --- .mokogitea/workflows/issue-branch.yml | 2 +- SECURITY.md | 2 +- source/packages/com_mokosuitebackup/mokosuitebackup.xml | 2 +- .../packages/com_mokosuitebackup/sql/updates/mysql/02.58.06.sql | 1 + .../mod_mokosuitebackup_cpanel/mod_mokosuitebackup_cpanel.xml | 2 +- .../packages/plg_actionlog_mokosuitebackup/mokosuitebackup.xml | 2 +- source/packages/plg_console_mokosuitebackup/mokosuitebackup.xml | 2 +- source/packages/plg_content_mokosuitebackup/mokosuitebackup.xml | 2 +- .../packages/plg_quickicon_mokosuitebackup/mokosuitebackup.xml | 2 +- source/packages/plg_system_mokosuitebackup/mokosuitebackup.xml | 2 +- source/packages/plg_task_mokosuitebackup/mokosuitebackup.xml | 2 +- .../plg_webservices_mokosuitebackup/mokosuitebackup.xml | 2 +- source/pkg_mokosuitebackup.xml | 2 +- 13 files changed, 13 insertions(+), 12 deletions(-) create mode 100644 source/packages/com_mokosuitebackup/sql/updates/mysql/02.58.06.sql diff --git a/.mokogitea/workflows/issue-branch.yml b/.mokogitea/workflows/issue-branch.yml index 6f668382..baee81b5 100644 --- a/.mokogitea/workflows/issue-branch.yml +++ b/.mokogitea/workflows/issue-branch.yml @@ -5,7 +5,7 @@ # FILE INFORMATION # DEFGROUP: MokoGitea.Workflow # INGROUP: mokocli.Automation -# VERSION: 02.58.05 +# VERSION: 02.58.06 # BRIEF: Auto-create feature branch when an issue is opened name: "Universal: Issue Branch" diff --git a/SECURITY.md b/SECURITY.md index 7ef06949..0ef3a52a 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -23,7 +23,7 @@ DEFGROUP: Template-Joomla INGROUP: Template-Joomla.Documentation REPO: https://git.mokoconsulting.tech/MokoConsulting/Template-Joomla PATH: /SECURITY.md -VERSION: 02.58.05 +VERSION: 02.58.06 BRIEF: Security vulnerability reporting and handling policy --> diff --git a/source/packages/com_mokosuitebackup/mokosuitebackup.xml b/source/packages/com_mokosuitebackup/mokosuitebackup.xml index 25d32596..9812805f 100644 --- a/source/packages/com_mokosuitebackup/mokosuitebackup.xml +++ b/source/packages/com_mokosuitebackup/mokosuitebackup.xml @@ -17,7 +17,7 @@ display label there. --> MokoSuiteBackup - 02.58.05 + 02.58.06 2026-06-02 Moko Consulting hello@mokoconsulting.tech diff --git a/source/packages/com_mokosuitebackup/sql/updates/mysql/02.58.06.sql b/source/packages/com_mokosuitebackup/sql/updates/mysql/02.58.06.sql new file mode 100644 index 00000000..6f510638 --- /dev/null +++ b/source/packages/com_mokosuitebackup/sql/updates/mysql/02.58.06.sql @@ -0,0 +1 @@ +/* 02.58.06 — no schema changes */ diff --git a/source/packages/mod_mokosuitebackup_cpanel/mod_mokosuitebackup_cpanel.xml b/source/packages/mod_mokosuitebackup_cpanel/mod_mokosuitebackup_cpanel.xml index 1aef54bd..ccd64acb 100644 --- a/source/packages/mod_mokosuitebackup_cpanel/mod_mokosuitebackup_cpanel.xml +++ b/source/packages/mod_mokosuitebackup_cpanel/mod_mokosuitebackup_cpanel.xml @@ -8,7 +8,7 @@ --> Module - MokoSuiteBackup - cPanel - 02.58.05 + 02.58.06 2026-06-23 Moko Consulting hello@mokoconsulting.tech diff --git a/source/packages/plg_actionlog_mokosuitebackup/mokosuitebackup.xml b/source/packages/plg_actionlog_mokosuitebackup/mokosuitebackup.xml index 780f2636..50710bf6 100644 --- a/source/packages/plg_actionlog_mokosuitebackup/mokosuitebackup.xml +++ b/source/packages/plg_actionlog_mokosuitebackup/mokosuitebackup.xml @@ -7,7 +7,7 @@ --> Action Log - MokoSuiteBackup - 02.58.05 + 02.58.06 2026-06-04 Moko Consulting hello@mokoconsulting.tech diff --git a/source/packages/plg_console_mokosuitebackup/mokosuitebackup.xml b/source/packages/plg_console_mokosuitebackup/mokosuitebackup.xml index 72c21305..c9457ba7 100644 --- a/source/packages/plg_console_mokosuitebackup/mokosuitebackup.xml +++ b/source/packages/plg_console_mokosuitebackup/mokosuitebackup.xml @@ -7,7 +7,7 @@ --> Console - MokoSuiteBackup - 02.58.05 + 02.58.06 2026-06-04 Moko Consulting hello@mokoconsulting.tech diff --git a/source/packages/plg_content_mokosuitebackup/mokosuitebackup.xml b/source/packages/plg_content_mokosuitebackup/mokosuitebackup.xml index ad424664..2b6fbe93 100644 --- a/source/packages/plg_content_mokosuitebackup/mokosuitebackup.xml +++ b/source/packages/plg_content_mokosuitebackup/mokosuitebackup.xml @@ -7,7 +7,7 @@ --> Content - MokoSuiteBackup - 02.58.05 + 02.58.06 2026-06-04 Moko Consulting hello@mokoconsulting.tech diff --git a/source/packages/plg_quickicon_mokosuitebackup/mokosuitebackup.xml b/source/packages/plg_quickicon_mokosuitebackup/mokosuitebackup.xml index a77ab126..e769f8da 100644 --- a/source/packages/plg_quickicon_mokosuitebackup/mokosuitebackup.xml +++ b/source/packages/plg_quickicon_mokosuitebackup/mokosuitebackup.xml @@ -1,7 +1,7 @@ Quick Icon - MokoSuiteBackup - 02.58.05 + 02.58.06 2026-06-02 Moko Consulting hello@mokoconsulting.tech diff --git a/source/packages/plg_system_mokosuitebackup/mokosuitebackup.xml b/source/packages/plg_system_mokosuitebackup/mokosuitebackup.xml index fb0a7228..3a4a40a8 100644 --- a/source/packages/plg_system_mokosuitebackup/mokosuitebackup.xml +++ b/source/packages/plg_system_mokosuitebackup/mokosuitebackup.xml @@ -7,7 +7,7 @@ --> System - MokoSuiteBackup - 02.58.05 + 02.58.06 2026-06-02 Moko Consulting hello@mokoconsulting.tech diff --git a/source/packages/plg_task_mokosuitebackup/mokosuitebackup.xml b/source/packages/plg_task_mokosuitebackup/mokosuitebackup.xml index 1b0f7b99..a0a584ef 100644 --- a/source/packages/plg_task_mokosuitebackup/mokosuitebackup.xml +++ b/source/packages/plg_task_mokosuitebackup/mokosuitebackup.xml @@ -7,7 +7,7 @@ --> Task - MokoSuiteBackup - 02.58.05 + 02.58.06 2026-06-02 Moko Consulting hello@mokoconsulting.tech diff --git a/source/packages/plg_webservices_mokosuitebackup/mokosuitebackup.xml b/source/packages/plg_webservices_mokosuitebackup/mokosuitebackup.xml index 3ad25bac..74ce1432 100644 --- a/source/packages/plg_webservices_mokosuitebackup/mokosuitebackup.xml +++ b/source/packages/plg_webservices_mokosuitebackup/mokosuitebackup.xml @@ -7,7 +7,7 @@ --> Web Services - MokoSuiteBackup - 02.58.05 + 02.58.06 2026-06-02 Moko Consulting hello@mokoconsulting.tech diff --git a/source/pkg_mokosuitebackup.xml b/source/pkg_mokosuitebackup.xml index 1d37622c..8ca57798 100644 --- a/source/pkg_mokosuitebackup.xml +++ b/source/pkg_mokosuitebackup.xml @@ -8,7 +8,7 @@ Package - MokoSuiteBackup mokosuitebackup - 02.58.05 + 02.58.06 2026-06-02 Moko Consulting hello@mokoconsulting.tech -- 2.52.0