fix: address final review — garbled code, error handling, write checks
Generic: Repo Health / Site Health (push) Has been skipped
Generic: Repo Health / Access control (push) Successful in 3s
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 3s
Joomla: Extension CI / Release Readiness Check (pull_request) Failing after 8s
Joomla: Extension CI / Lint & Validate (pull_request) Failing after 13s
Universal: Secret Scanning / Gitleaks Secret Scan (pull_request) Successful in 15s
Universal: Auto Version Bump / Version Bump (push) Successful in 18s
Universal: Pre-Release / Build Pre-Release (${{ inputs.stability || github.ref_name }}) (push) Successful in 16s
Universal: PR Check / Validate PR (pull_request) Failing after 47s
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

Critical:
- Fix garbled getDbConnection() in MokoRestore — duplicated lines and
  broken regex causing parse errors in the standalone restore script

High:
- fixPackageClientId() now warns user via enqueueMessage on failure
- sanitizeConfiguration() logs error when file read fails
- actionConfig() checks file_put_contents return value on both paths
- writeDefaultHtaccess() returns status string, checks copy and write,
  callers append warnings to response message

Medium:
- Remove @unlink suppression before archive rename, log warning
- viewLog() catch block now logs exception message for diagnostics
This commit is contained in:
Jonathan Miller
2026-06-15 00:49:24 -05:00
parent a6de692639
commit b0fa2cceba
4 changed files with 49 additions and 16 deletions
@@ -218,6 +218,7 @@ class AjaxController extends BaseController
$db->setQuery($query);
$record = $db->loadObject();
} catch (\Exception $e) {
error_log('MokoSuiteBackup: viewLog() DB error for record ' . $id . ': ' . $e->getMessage());
$this->sendJson(['error' => true, 'message' => 'Failed to load backup record'], 500);
return;
@@ -223,7 +223,9 @@ class BackupEngine
MokoRestore::wrap($archivePath, $mokoRestorePath);
// Replace the original archive with the wrapped one
@unlink($archivePath);
if (is_file($archivePath) && !unlink($archivePath)) {
$this->log('WARNING: Could not remove pre-wrap archive');
}
rename($mokoRestorePath, $archivePath);
$totalSize = filesize($archivePath);
$sizeHuman = number_format($totalSize / 1048576, 2) . ' MB';
@@ -521,6 +523,8 @@ class BackupEngine
$content = file_get_contents($path);
if ($content === false) {
error_log('MokoSuiteBackup: sanitizeConfiguration() failed to read: ' . $path);
return '';
}
@@ -445,7 +445,9 @@ function actionConfig(array $data): array
$config = preg_replace($pattern, $replacement, $config);
}
file_put_contents($configPath, $config);
if (file_put_contents($configPath, $config) === false) {
return ['success' => false, 'message' => 'Failed to write Joomla config file — check directory permissions'];
}
// Remove .bak after successful rebuild
if (is_file($bakPath)) {
@@ -453,11 +455,19 @@ function actionConfig(array $data): array
}
// Reset .htaccess to Joomla defaults if requested
$htWarn = '';
if (($data['reset_htaccess'] ?? '0') === '1') {
writeDefaultHtaccess(RESTORE_DIR);
$htWarn = writeDefaultHtaccess(RESTORE_DIR);
}
return ['success' => true, 'message' => 'Joomla configuration rebuilt with fresh credentials and secret'];
$msg = 'Joomla configuration rebuilt with fresh credentials and secret';
if ($htWarn !== '') {
$msg .= ' (Warning: ' . $htWarn . ')';
}
return ['success' => true, 'message' => $msg];
}
// Create new configuration.php from scratch
@@ -504,31 +514,43 @@ class JConfig {
}
JCONFIG;
file_put_contents($configFile, $newConfig);
if (file_put_contents($configFile, $newConfig) === false) {
return ['success' => false, 'message' => 'Failed to write Joomla config file — check directory permissions'];
}
// Ensure directories exist
@mkdir($tmpPath, 0755, true);
@mkdir($logPath, 0755, true);
// Reset .htaccess to Joomla defaults if requested
$htWarn = '';
if (($data['reset_htaccess'] ?? '0') === '1') {
writeDefaultHtaccess(RESTORE_DIR);
$htWarn = writeDefaultHtaccess(RESTORE_DIR);
}
return ['success' => true, 'message' => 'Joomla configuration created from scratch with fresh secret'];
$msg = 'Joomla configuration created from scratch with fresh secret';
if ($htWarn !== '') {
$msg .= ' (Warning: ' . $htWarn . ')';
}
return ['success' => true, 'message' => $msg];
}
/**
* Write a clean Joomla default .htaccess file.
* Backs up the existing one as .htaccess.bak first.
*/
function writeDefaultHtaccess(string $siteRoot): void
function writeDefaultHtaccess(string $siteRoot): string
{
$htaccess = $siteRoot . '/.htaccess';
// Backup existing .htaccess before overwriting
if (is_file($htaccess)) {
copy($htaccess, $htaccess . '.bak');
if (!copy($htaccess, $htaccess . '.bak')) {
return 'Could not back up existing .htaccess — reset skipped for safety';
}
}
$default = <<<'HTACCESS'
@@ -625,7 +647,11 @@ RewriteRule .* index.php [L]
## End - Joomla! core SEF Section.
HTACCESS;
file_put_contents($htaccess, $default);
if (file_put_contents($htaccess, $default) === false) {
return 'Could not write .htaccess — check directory permissions';
}
return '';
}
function actionListAdmins(array $data): array
@@ -776,18 +802,14 @@ function actionCleanup(): array
function getDbConnection(array $data): PDO
{
$host = $data['db_host'] ?? 'localhost';
// Validate db_prefix to prevent SQL injection $prefix = $data['db_prefix'] ?? 'moko_'; if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$/', $prefix)) { throw new RuntimeException('Invalid table prefix format'); }
$name = $data['db_name'] ?? '';
// Validate db_prefix to prevent SQL injection $prefix = $data['db_prefix'] ?? 'moko_'; if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$/', $prefix)) { throw new RuntimeException('Invalid table prefix format'); }
$user = $data['db_user'] ?? '';
// Validate db_prefix to prevent SQL injection $prefix = $data['db_prefix'] ?? 'moko_'; if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$/', $prefix)) { throw new RuntimeException('Invalid table prefix format'); }
$pass = $data['db_pass'] ?? '';
// Validate db_prefix to prevent SQL injection $prefix = $data['db_prefix'] ?? 'moko_'; if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$/', $prefix)) { throw new RuntimeException('Invalid table prefix format'); }
// Validate db_prefix to prevent SQL injection
$prefix = $data['db_prefix'] ?? 'moko_';
// Validate db_prefix to prevent SQL injection $prefix = $data['db_prefix'] ?? 'moko_'; if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$/', $prefix)) { throw new RuntimeException('Invalid table prefix format'); }
if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$\/', $prefix)) {
if (!preg_match('/^[a-zA-Z][a-zA-Z0-9_]{0,20}$/', $prefix)) {
throw new RuntimeException('Invalid table prefix format');
}
+6
View File
@@ -495,6 +495,12 @@ class Pkg_MokoSuiteBackupInstallerScript
$db->execute();
} catch (\Exception $e) {
error_log('MokoSuiteBackup: fixPackageClientId() failed: ' . $e->getMessage());
Factory::getApplication()->enqueueMessage(
'MokoSuiteBackup could not correct the package registration. '
. 'Automatic updates may not appear. If you do not see update notifications, '
. 'please reinstall the package.',
'warning'
);
}
}