fix: security + data-integrity fixes (P0/P1)
Universal: PR Check / Branch Policy (pull_request) Successful in 1s
Joomla: Extension CI / Release Readiness Check (pull_request) Failing after 6s
Universal: PR Check / Wiki Update Reminder (pull_request) Successful in 2s
Universal: PR Check / Require Docs Update (pull_request) Failing after 8s
Universal: PR Check / Secret Scan (pull_request) Successful in 9s
Generic: Repo Health / Access control (pull_request) Successful in 2s
Generic: Repo Health / Site Health (pull_request) Has been skipped
Universal: PR Check / Validate PR (pull_request) Failing after 13s
Joomla: Metadata Validation / Validate Joomla Metadata (pull_request) Successful in 12s
Universal: Pre-Release / Build Pre-Release (${{ inputs.stability || github.ref_name }}) (push) Successful in 28s
Universal: Build & Release / Promote to RC (pull_request) Has been skipped
Universal: Build & Release / Build & Release Pipeline (pull_request) Has been skipped
Generic: Project CI / Lint & Validate (pull_request) Successful in 46s
Joomla: Extension CI / Lint & Validate (pull_request) Failing after 50s
Generic: Project CI / Tests (pull_request) 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
Joomla: Extension CI / Build RC Pre-Release (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: Scripts Governance (pull_request) Has been cancelled
Generic: Repo Health / Report: Repository Health (pull_request) Has been cancelled

Security (P0):
- #169 mask FTP password in AjaxController maskSecrets()/mergeExistingSecrets()
- #187 stop leaking absolute_path in API item view (JsonapiView)
- #182 SftpUploader StrictHostKeyChecking=no -> accept-new (MITM)

Data integrity / correctness (P1):
- #179 DatabaseDumper: real newline after DROP TABLE (was literal backslash-n)
- #180 profile-picker forms ORDER BY id (the 'ordering' column is dropped on
  upgraded sites, breaking the query) — run_profile.xml + plg_content xml
- #181 uninstall.mysql.sql: add missing DROP for snapshots table
- #184 CLI snapshot delete uses data_file column (file_path does not exist,
  so snapshot files were never deleted)
- #186 remove duplicate pre-update backup: content plugin no longer subscribes
  to onExtensionBeforeUpdate (owned by the system plugin); keeps pre-install
- #188 DatabaseImporter now collects non-fatal statement errors
  (getErrors()/hasErrors()) so restores are no longer silent and callers can
  surface a warning status

Also folds in the .mokogitea/CLAUDE.md mokoplatform -> mokocli fix (stale repo
name after the rename).

Claude-Session: https://claude.ai/code/session_01WbGBN9VyRK61zczYWcCQ2i
This commit is contained in:
2026-07-05 16:31:22 -05:00
committed by Jonathan Miller
parent 75bbb55e29
commit 99b844e04a
11 changed files with 49 additions and 25 deletions
+1 -1
View File
@@ -60,7 +60,7 @@ Joomla **package** with four sub-extensions:
- **Workflow directory**: `.mokogitea/` (not `.gitea/` or `.github/`)
- **Minification**: handled at build time (CI)
- **Wiki**: documentation lives in the Gitea wiki, not `docs/` files
- **Standards**: [MokoStandards](https://git.mokoconsulting.tech/MokoConsulting/mokoplatform/wiki/Home)
- **Standards**: [MokoStandards](https://git.mokoconsulting.tech/MokoConsulting/mokocli/wiki/Home)
## Coding Standards
@@ -24,7 +24,6 @@ class JsonapiView extends BaseApiView
'origin',
'backup_type',
'archivename',
'absolute_path',
'total_size',
'db_size',
'files_count',
@@ -1,3 +1,4 @@
DROP TABLE IF EXISTS `#__mokosuitebackup_remotes`;
DROP TABLE IF EXISTS `#__mokosuitebackup_records`;
DROP TABLE IF EXISTS `#__mokosuitebackup_snapshots`;
DROP TABLE IF EXISTS `#__mokosuitebackup_profiles`;
@@ -1201,6 +1201,7 @@ class AjaxController extends BaseController
private function maskSecrets(array $config, string $type): array
{
$secrets = [
'ftp' => ['password'],
'sftp' => ['password', 'passphrase', 'key_data'],
's3' => ['secret_key'],
'google_drive' => ['client_secret', 'refresh_token'],
@@ -1223,6 +1224,7 @@ class AjaxController extends BaseController
private function mergeExistingSecrets(int $id, array $config, string $type): array
{
$secrets = [
'ftp' => ['password'],
'sftp' => ['password', 'passphrase', 'key_data'],
's3' => ['secret_key'],
'google_drive' => ['client_secret', 'refresh_token'],
@@ -326,7 +326,7 @@ class DatabaseDumper
}
$createSql = str_replace('`' . $prefix, '`#__', $createRow[1]);
fwrite($fp, 'DROP TABLE IF EXISTS `' . $abstractName . "`;\\n");
fwrite($fp, 'DROP TABLE IF EXISTS `' . $abstractName . "`;\n");
fwrite($fp, $createSql . ";\n\n");
}
@@ -20,6 +20,13 @@ use Joomla\CMS\Factory;
class DatabaseImporter
{
/**
* Non-fatal per-statement errors collected during the last import().
*
* @var string[]
*/
private array $errors = [];
/**
* Import a SQL dump file into the database.
*
@@ -31,6 +38,8 @@ class DatabaseImporter
*/
public function import(string $sqlFile): int
{
$this->errors = [];
if (!is_file($sqlFile) || !is_readable($sqlFile)) {
throw new \RuntimeException('SQL file not readable: ' . $sqlFile);
}
@@ -97,8 +106,10 @@ class DatabaseImporter
} catch (\Exception $e) {
// Log but don't abort — some statements may fail on
// different MySQL versions (e.g. charset differences)
// but the overall restore should continue.
// but the overall restore should continue. Errors are
// collected so the caller can surface a warning status.
error_log('MokoSuiteBackup SQL import warning: ' . $e->getMessage());
$this->errors[] = $e->getMessage();
}
}
}
@@ -115,6 +126,7 @@ class DatabaseImporter
$statementsExecuted++;
} catch (\Exception $e) {
error_log('MokoSuiteBackup SQL import warning (final): ' . $e->getMessage());
$this->errors[] = $e->getMessage();
}
}
} finally {
@@ -123,4 +135,24 @@ class DatabaseImporter
return $statementsExecuted;
}
/**
* Non-fatal errors from the last import(), if any. A non-empty result
* means the restore completed with problems and should be treated as a
* warning rather than a clean success.
*
* @return string[]
*/
public function getErrors(): array
{
return $this->errors;
}
/**
* Whether the last import() had any non-fatal statement errors.
*/
public function hasErrors(): bool
{
return $this->errors !== [];
}
}
@@ -207,7 +207,7 @@ class SftpUploader implements RemoteUploaderInterface
*/
private function buildScpCommand(string $localPath, string $remoteTarget, ?string $keyFile): string
{
$parts = ['scp', '-o', 'StrictHostKeyChecking=no', '-o', 'BatchMode=yes'];
$parts = ['scp', '-o', 'StrictHostKeyChecking=accept-new', '-o', 'BatchMode=yes'];
if ($this->port !== 22) {
$parts[] = '-P';
@@ -235,7 +235,7 @@ class SftpUploader implements RemoteUploaderInterface
*/
private function buildSshCommand(string $remoteCmd, ?string $keyFile): string
{
$parts = ['ssh', '-o', 'StrictHostKeyChecking=no', '-o', 'BatchMode=yes'];
$parts = ['ssh', '-o', 'StrictHostKeyChecking=accept-new', '-o', 'BatchMode=yes'];
if ($this->port !== 22) {
$parts[] = '-p';
@@ -227,11 +227,11 @@ class SnapshotCommand extends AbstractCommand
}
// Delete the snapshot file if it exists
if (!empty($record->file_path) && is_file($record->file_path)) {
if (!@unlink($record->file_path)) {
$io->warning('Could not delete snapshot file: ' . $record->file_path);
if (!empty($record->data_file) && is_file($record->data_file)) {
if (!@unlink($record->data_file)) {
$io->warning('Could not delete snapshot file: ' . $record->data_file);
} else {
$io->text('Deleted file: ' . $record->file_path);
$io->text('Deleted file: ' . $record->data_file);
}
}
@@ -59,7 +59,7 @@
type="sql"
label="PLG_CONTENT_MOKOJOOMBACKUP_FIELD_PROFILE"
description="PLG_CONTENT_MOKOJOOMBACKUP_FIELD_PROFILE_DESC"
query="SELECT id AS value, title AS text FROM #__mokosuitebackup_profiles WHERE published = 1 ORDER BY ordering ASC"
query="SELECT id AS value, title AS text FROM #__mokosuitebackup_profiles WHERE published = 1 ORDER BY id ASC"
default="1"
>
<option value="1">Default Backup Profile</option>
@@ -24,9 +24,11 @@ final class MokoSuiteBackupContent extends CMSPlugin implements SubscriberInterf
public static function getSubscribedEvents(): array
{
// Pre-update backups are owned by plg_system_mokosuitebackup, which also
// subscribes to onExtensionBeforeUpdate. Only pre-install is handled here
// to avoid running the backup twice on a single extension update.
return [
'onExtensionBeforeInstall' => 'onExtensionBeforeInstall',
'onExtensionBeforeUpdate' => 'onExtensionBeforeUpdate',
];
}
@@ -42,18 +44,6 @@ final class MokoSuiteBackupContent extends CMSPlugin implements SubscriberInterf
$this->triggerAutoBackup('Pre-install backup');
}
/**
* Trigger a backup before an extension is updated.
*/
public function onExtensionBeforeUpdate(Event $event): void
{
if (!(int) $this->params->get('backup_before_update', 1)) {
return;
}
$this->triggerAutoBackup('Pre-update backup');
}
/**
* Run a backup using the configured profile.
*/
@@ -11,7 +11,7 @@
type="sql"
label="PLG_TASK_MOKOJOOMBACKUP_FIELD_PROFILE"
description="PLG_TASK_MOKOJOOMBACKUP_FIELD_PROFILE_DESC"
query="SELECT id AS value, title AS text FROM #__mokosuitebackup_profiles WHERE published = 1 ORDER BY ordering ASC"
query="SELECT id AS value, title AS text FROM #__mokosuitebackup_profiles WHERE published = 1 ORDER BY id ASC"
default="1"
required="true"
>