fix: address PR review — error handling, security, code quality
Generic: Repo Health / Site Health (push) Has been skipped
Generic: Repo Health / Access control (push) Successful in 2s
Universal: PR Check / Branch Policy (pull_request) Successful in 2s
Generic: Repo Health / Site Health (pull_request) Has been skipped
Generic: Repo Health / Access control (pull_request) Successful in 2s
Joomla: Extension CI / Release Readiness Check (pull_request) Failing after 7s
Universal: Secret Scanning / Gitleaks Secret Scan (pull_request) Successful in 8s
Joomla: Extension CI / Lint & Validate (pull_request) Failing after 12s
Universal: Auto Version Bump / Version Bump (push) Successful in 15s
Universal: Pre-Release / Build Pre-Release (${{ inputs.stability || github.ref_name }}) (push) Successful in 7s
Universal: PR Check / Validate PR (pull_request) Failing after 26s
Generic: Repo Health / Scripts governance (push) Has been cancelled
Generic: Repo Health / Repository health (push) Has been cancelled
Generic: Repo Health / Report Issues (push) Has been cancelled
Joomla: Extension CI / Tests (PHP 8.2) (pull_request) Has been cancelled
Joomla: Extension CI / Tests (PHP 8.3) (pull_request) Has been cancelled
Joomla: Extension CI / PHPStan Analysis (pull_request) Has been cancelled
Universal: PR Check / Build RC Package (pull_request) Has been cancelled
Universal: PR Check / Report Issues (pull_request) Has been cancelled
Generic: Repo Health / Scripts governance (pull_request) Has been cancelled
Generic: Repo Health / Repository health (pull_request) Has been cancelled
Generic: Repo Health / Report Issues (pull_request) Has been cancelled

- Move uninstall guard to top of postflight()
- Refactor plugin enables into loop with per-plugin try-catch
- Replace @mkdir with createBackupDirectory() — check result, warn
  user, add .htaccess + index.html protection
- Merge menu_icon into existing params instead of overwriting
- Add HtmlDocument type check in boot(), narrow catch to RuntimeException
- Add Joomla version check in preflight()
- Add error_log on missing parent/component in ensureSubmenuItems()
- Rename warnDefaultBackupDir → migrateDefaultBackupDir
- Narrow all \Throwable catches to \Exception
- Warn user on restoreDownloadKey failure via enqueueMessage
- Use null-safe operator for getIdentity()?->id
- Remove orphaned docblock, fix ensureSubmenuItems docblock
- Tighten syncMenuIcons LIKE pattern
This commit is contained in:
Jonathan Miller
2026-06-12 19:22:12 -05:00
parent 59fac1bf08
commit 5393180eb9
2 changed files with 137 additions and 125 deletions
+128 -123
View File
@@ -40,6 +40,15 @@ class Pkg_MokoSuiteBackupInstallerScript
*/
public function preflight(string $type, InstallerAdapter $parent): bool
{
if (version_compare(JVERSION, $this->minimumJoomla, '<')) {
Factory::getApplication()->enqueueMessage(
Text::sprintf('PKG_MOKOJOOMBACKUP_JOOMLA_VERSION_ERROR', $this->minimumJoomla),
'error'
);
return false;
}
if (version_compare(PHP_VERSION, $this->minimumPhp, '<')) {
Factory::getApplication()->enqueueMessage(
Text::sprintf('PKG_MOKOJOOMBACKUP_PHP_VERSION_ERROR', $this->minimumPhp),
@@ -57,14 +66,6 @@ class Pkg_MokoSuiteBackupInstallerScript
return true;
}
/**
* Called after install/update.
*
* @param string $type Action type
* @param InstallerAdapter $parent Installer adapter
*
* @return void
*/
/**
* Called before install/update to preserve the download key.
*
@@ -100,113 +101,43 @@ class Pkg_MokoSuiteBackupInstallerScript
if (!empty($key)) {
$this->savedDownloadKey = $key;
}
} catch (\Throwable $e) {
} catch (\Exception $e) {
error_log('MokoSuiteBackup: Could not save download key: ' . $e->getMessage());
}
}
/**
* Called after install/update/uninstall.
*
* @param string $type Action type (install, update, uninstall)
* @param InstallerAdapter $parent Installer adapter
*
* @return void
*/
public function postflight(string $type, InstallerAdapter $parent): void
{
if ($type === 'uninstall') {
return;
}
// Restore download key if it was saved before update
if ($this->savedDownloadKey !== null) {
$this->restoreDownloadKey();
}
if ($type === 'install') {
// Enable the system plugin automatically on fresh install
$db = Factory::getDbo();
$query = $db->getQuery(true)
->update($db->quoteName('#__extensions'))
->set($db->quoteName('enabled') . ' = 1')
->where($db->quoteName('type') . ' = ' . $db->quote('plugin'))
->where($db->quoteName('folder') . ' = ' . $db->quote('system'))
->where($db->quoteName('element') . ' = ' . $db->quote('mokosuitebackup'));
$db->setQuery($query);
$db->execute();
// Enable the quickicon plugin automatically
$query = $db->getQuery(true)
->update($db->quoteName('#__extensions'))
->set($db->quoteName('enabled') . ' = 1')
->where($db->quoteName('type') . ' = ' . $db->quote('plugin'))
->where($db->quoteName('folder') . ' = ' . $db->quote('quickicon'))
->where($db->quoteName('element') . ' = ' . $db->quote('mokosuitebackup'));
$db->setQuery($query);
$db->execute();
// Enable the task plugin automatically
$query = $db->getQuery(true)
->update($db->quoteName('#__extensions'))
->set($db->quoteName('enabled') . ' = 1')
->where($db->quoteName('type') . ' = ' . $db->quote('plugin'))
->where($db->quoteName('folder') . ' = ' . $db->quote('task'))
->where($db->quoteName('element') . ' = ' . $db->quote('mokosuitebackup'));
$db->setQuery($query);
$db->execute();
// Enable the webservices plugin automatically
$query = $db->getQuery(true)
->update($db->quoteName('#__extensions'))
->set($db->quoteName('enabled') . ' = 1')
->where($db->quoteName('type') . ' = ' . $db->quote('plugin'))
->where($db->quoteName('folder') . ' = ' . $db->quote('webservices'))
->where($db->quoteName('element') . ' = ' . $db->quote('mokosuitebackup'));
$db->setQuery($query);
$db->execute();
// Enable the console plugin automatically
$query = $db->getQuery(true)
->update($db->quoteName('#__extensions'))
->set($db->quoteName('enabled') . ' = 1')
->where($db->quoteName('type') . ' = ' . $db->quote('plugin'))
->where($db->quoteName('folder') . ' = ' . $db->quote('console'))
->where($db->quoteName('element') . ' = ' . $db->quote('mokosuitebackup'));
$db->setQuery($query);
$db->execute();
// Enable the content plugin automatically
$query = $db->getQuery(true)
->update($db->quoteName('#__extensions'))
->set($db->quoteName('enabled') . ' = 1')
->where($db->quoteName('type') . ' = ' . $db->quote('plugin'))
->where($db->quoteName('folder') . ' = ' . $db->quote('content'))
->where($db->quoteName('element') . ' = ' . $db->quote('mokosuitebackup'));
$db->setQuery($query);
$db->execute();
// Enable the actionlog plugin automatically
$query = $db->getQuery(true)
->update($db->quoteName('#__extensions'))
->set($db->quoteName('enabled') . ' = 1')
->where($db->quoteName('type') . ' = ' . $db->quote('plugin'))
->where($db->quoteName('folder') . ' = ' . $db->quote('actionlog'))
->where($db->quoteName('element') . ' = ' . $db->quote('mokosuitebackup'));
$db->setQuery($query);
$db->execute();
// Enable all bundled plugins on fresh install
$this->enableBundledPlugins();
// Create default backup directory in site root
$backupDir = JPATH_ROOT . '/backups';
$this->createBackupDirectory();
if (!is_dir($backupDir)) {
@mkdir($backupDir, 0755, true);
}
// Create default scheduled task — every 30 days, profile 1
// Create default scheduled task for backup automation
$this->createDefaultScheduledTask();
}
if ($type === 'uninstall') {
return;
}
// Ensure submenu items exist (Joomla only creates them on fresh install)
// Ensure submenu items exist and are up to date
// (Joomla may not add new submenu entries or update params on upgrades)
$this->ensureSubmenuItems();
// Sync submenu icons in #__menu (Joomla doesn't update icons on upgrades)
@@ -215,8 +146,8 @@ class Pkg_MokoSuiteBackupInstallerScript
// Warn if no license key configured
$this->warnMissingLicenseKey();
// Warn if any profile still uses the default backup directory
$this->warnDefaultBackupDir();
// Migrate profiles with old default backup_dir values to [DEFAULT_DIR] placeholder
$this->migrateDefaultBackupDir();
// Remind user to review backup profile settings
if ($type === 'install') {
@@ -232,11 +163,70 @@ class Pkg_MokoSuiteBackupInstallerScript
}
}
private function warnDefaultBackupDir(): void
private function enableBundledPlugins(): void
{
$folders = ['system', 'quickicon', 'task', 'webservices', 'console', 'content', 'actionlog'];
$db = Factory::getDbo();
foreach ($folders as $folder) {
try {
$query = $db->getQuery(true)
->update($db->quoteName('#__extensions'))
->set($db->quoteName('enabled') . ' = 1')
->where($db->quoteName('type') . ' = ' . $db->quote('plugin'))
->where($db->quoteName('folder') . ' = ' . $db->quote($folder))
->where($db->quoteName('element') . ' = ' . $db->quote('mokosuitebackup'));
$db->setQuery($query);
$db->execute();
} catch (\Exception $e) {
error_log('MokoSuiteBackup: Failed to enable ' . $folder . ' plugin: ' . $e->getMessage());
Factory::getApplication()->enqueueMessage(
'MokoSuiteBackup: Could not enable the ' . $folder . ' plugin. '
. 'Please enable it manually in Extensions &rarr; Plugins.',
'warning'
);
}
}
}
private function createBackupDirectory(): void
{
$backupDir = JPATH_ROOT . '/backups';
if (is_dir($backupDir)) {
return;
}
if (!mkdir($backupDir, 0755, true)) {
error_log('MokoSuiteBackup: Failed to create default backup directory: ' . $backupDir);
Factory::getApplication()->enqueueMessage(
'MokoSuiteBackup could not create the default backup directory at <code>'
. htmlspecialchars($backupDir) . '</code>. '
. 'Please create it manually and ensure the web server has write permissions.',
'warning'
);
return;
}
// Protect directory from direct web access
$htaccess = $backupDir . '/.htaccess';
if (!file_exists($htaccess)) {
file_put_contents($htaccess, "Order Deny,Allow\nDeny from all\n");
}
$indexHtml = $backupDir . '/index.html';
if (!file_exists($indexHtml)) {
file_put_contents($indexHtml, '<!DOCTYPE html><title></title>');
}
}
private function migrateDefaultBackupDir(): void
{
try {
$db = Factory::getDbo();
// Check for profiles using old literal defaults — migrate to [DEFAULT_DIR]
$oldDefaults = [
'administrator/components/com_mokosuitebackup/backups',
'administrator/components/com_mokojoombackup/backups',
@@ -254,7 +244,6 @@ class Pkg_MokoSuiteBackupInstallerScript
$db->setQuery($query);
if ((int) $db->loadResult() > 0) {
// Auto-migrate old defaults to [DEFAULT_DIR] placeholder
$update = $db->getQuery(true)
->update($db->quoteName('#__mokosuitebackup_profiles'))
->set($db->quoteName('backup_dir') . ' = ' . $db->quote('[DEFAULT_DIR]'))
@@ -264,9 +253,15 @@ class Pkg_MokoSuiteBackupInstallerScript
. ' OR ' . $db->quoteName('backup_dir') . ' IS NULL)');
$db->setQuery($update);
$db->execute();
$migrated = $db->getAffectedRows();
if ($migrated > 0) {
error_log('MokoSuiteBackup: Migrated ' . $migrated . ' profile(s) from legacy backup_dir to [DEFAULT_DIR]');
}
}
} catch (\Throwable $e) {
error_log('MokoSuiteBackup: warnDefaultBackupDir() failed: ' . $e->getMessage());
} catch (\Exception $e) {
error_log('MokoSuiteBackup: migrateDefaultBackupDir() failed: ' . $e->getMessage());
}
}
@@ -320,12 +315,12 @@ class Pkg_MokoSuiteBackupInstallerScript
'cli_exclusive' => 0,
'note' => '',
'created' => $now,
'created_by' => Factory::getApplication()->getIdentity()->id ?? 0,
'created_by' => Factory::getApplication()->getIdentity()?->id ?? 0,
'next_execution' => date('Y-m-d 03:00:00', strtotime('+1 day')),
];
$db->insertObject('#__scheduler_tasks', $task);
} catch (\Throwable $e) {
} catch (\Exception $e) {
error_log('MokoSuiteBackup: createDefaultScheduledTask() failed: ' . $e->getMessage());
}
}
@@ -333,9 +328,9 @@ class Pkg_MokoSuiteBackupInstallerScript
/**
* Ensure admin submenu items exist in #__menu.
*
* On updates Joomla does not re-create submenu entries from the manifest,
* so we use the Installer's own _buildAdminMenus pathway via the
* component's MenuTable API to create any missing items.
* On updates Joomla may not add new submenu entries or update params,
* so we manually create missing items using MenuTable for correct
* nested set positioning (lft/rgt values).
*/
private function ensureSubmenuItems(): void
{
@@ -375,6 +370,7 @@ class Pkg_MokoSuiteBackupInstallerScript
$parent = $db->loadObject();
if (!$parent) {
error_log('MokoSuiteBackup: ensureSubmenuItems() — parent menu item not found');
return;
}
@@ -389,28 +385,31 @@ class Pkg_MokoSuiteBackupInstallerScript
$componentId = (int) $db->loadResult();
if (!$componentId) {
error_log('MokoSuiteBackup: ensureSubmenuItems() — component extension_id not found');
return;
}
foreach ($submenus as $submenu) {
$params = json_encode(['menu_icon' => $submenu['menu_icon']]);
// Check if this submenu item already exists
$query = $db->getQuery(true)
->select($db->quoteName('id'))
->select([$db->quoteName('id'), $db->quoteName('params')])
->from($db->quoteName('#__menu'))
->where($db->quoteName('client_id') . ' = 1')
->where($db->quoteName('link') . ' = ' . $db->quote($submenu['link']))
->setLimit(1);
$db->setQuery($query);
$existingId = (int) $db->loadResult();
$existing = $db->loadObject();
if ($existing) {
// Merge menu_icon into existing params to preserve other settings
$existingParams = json_decode($existing->params ?? '{}', true) ?: [];
$existingParams['menu_icon'] = $submenu['menu_icon'];
$mergedParams = json_encode($existingParams);
if ($existingId > 0) {
// Update params on existing item to ensure menu_icon is set
$query = $db->getQuery(true)
->update($db->quoteName('#__menu'))
->set($db->quoteName('params') . ' = ' . $db->quote($params))
->where($db->quoteName('id') . ' = ' . $existingId);
->set($db->quoteName('params') . ' = ' . $db->quote($mergedParams))
->where($db->quoteName('id') . ' = ' . (int) $existing->id);
$db->setQuery($query);
$db->execute();
continue;
@@ -422,6 +421,8 @@ class Pkg_MokoSuiteBackupInstallerScript
->getMVCFactory()
->createTable('Menu', 'Administrator');
$params = json_encode(['menu_icon' => $submenu['menu_icon']]);
$table->menutype = $parent->menutype;
$table->title = $submenu['title'];
$table->alias = strtolower(str_replace(' ', '-', $submenu['title']));
@@ -443,7 +444,7 @@ class Pkg_MokoSuiteBackupInstallerScript
error_log('MokoSuiteBackup: Failed to create submenu "' . $submenu['title'] . '": ' . $table->getError());
}
}
} catch (\Throwable $e) {
} catch (\Exception $e) {
error_log('MokoSuiteBackup: ensureSubmenuItems() failed: ' . $e->getMessage());
}
}
@@ -464,12 +465,11 @@ class Pkg_MokoSuiteBackupInstallerScript
->update($db->quoteName('#__menu'))
->set($db->quoteName('img') . ' = ' . $db->quote($icon))
->where($db->quoteName('client_id') . ' = 1')
->where($db->quoteName('link') . ' LIKE ' . $db->quote('%com_mokosuitebackup%' . $linkFragment . '%'));
->where($db->quoteName('link') . ' LIKE ' . $db->quote('index.php?option=com_mokosuitebackup%' . $linkFragment . '%'));
$db->setQuery($query);
$db->execute();
}
// Set top-level component menu icon
$query = $db->getQuery(true)
->update($db->quoteName('#__menu'))
->set($db->quoteName('img') . ' = ' . $db->quote('class:archive'))
@@ -478,7 +478,7 @@ class Pkg_MokoSuiteBackupInstallerScript
->where($db->quoteName('level') . ' = 1');
$db->setQuery($query);
$db->execute();
} catch (\Throwable $e) {
} catch (\Exception $e) {
error_log('MokoSuiteBackup: syncMenuIcons() failed: ' . $e->getMessage());
}
}
@@ -517,8 +517,13 @@ class Pkg_MokoSuiteBackupInstallerScript
$db->setQuery($query);
$db->execute();
}
} catch (\Throwable $e) {
} catch (\Exception $e) {
error_log('MokoSuiteBackup: Could not restore download key: ' . $e->getMessage());
Factory::getApplication()->enqueueMessage(
'MokoSuiteBackup: Your download/license key could not be preserved during the update. '
. 'Please re-enter it in the Update Sites configuration to continue receiving updates.',
'warning'
);
}
}
@@ -554,7 +559,7 @@ class Pkg_MokoSuiteBackupInstallerScript
'warning'
);
}
catch (\Throwable $e) {
catch (\Exception $e) {
error_log('MokoSuiteBackup: License key check failed: ' . $e->getMessage());
}
}