Skip to content

fix(backup): verify DB dump and archive integrity to prevent silent corrupt backups #483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mrrobot47 wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

fix(backup): verify DB dump and archive integrity to prevent silent corrupt backups #483

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ea117b6
fix(backup): harden backup integrity against silent data loss
mrrobot47 Jun 29, 2026
e93a6c8
fix(backup): close DB-less backup gap and widen archive integrity checks
mrrobot47 Jun 29, 2026
ac60000
docs(backup): correct DB-credential quoting comment
mrrobot47 Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 65 additions & 8 deletions src/helper/Site_Backup_Restore.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,10 @@ private function maybe_backup_custom_docker_compose( $backup_dir ) {
// This is optional, so we just log a warning instead of failing
if ( $result->return_code >= 2 ) {
EE::warning( 'Failed to backup custom docker-compose directory. Continuing with backup.' );
} elseif ( EE::launch( sprintf( '7z t %s', escapeshellarg( $custom_docker_compose_dir_archive ) ) )->return_code >= 2 ) {
// Optional archive: warn (and drop the corrupt zip) instead of aborting the whole backup.
EE::warning( 'Custom docker-compose archive failed integrity check. Excluding it from the backup.' );
$this->fs->remove( $custom_docker_compose_dir_archive );
}
}
}
Expand All @@ -488,9 +492,33 @@ private function backup_site_dir( $backup_dir ) {
EE::error( 'Failed to create backup archive. Please check disk space and file permissions.' );
}

$this->verify_archive_integrity( $backup_file );

return $backup_file;
}

/**
* Run `7z t` on a freshly-created backup archive and abort if it is corrupt.
*
* Catches silently-truncated/corrupt archives before they are uploaded, so a
* broken backup never replaces a good one in remote storage.
*
* @param string $archive Absolute path to the archive to test.
*/
private function verify_archive_integrity( $archive ) {
// 7z exit codes: 0=success, 1=warning (non-fatal), 2+=fatal error.
if ( EE::launch( sprintf( '7z t %s', escapeshellarg( $archive ) ) )->return_code < 2 ) {
return;
}

$this->capture_error(
sprintf( 'Backup archive failed integrity check: %s', $archive ),
self::ERROR_TYPE_FILESYSTEM,
3003
);
EE::error( 'Backup archive failed integrity verification. Aborting before upload to avoid overwriting a good backup.' );
}

private function backup_wp_content_dir( $backup_dir ) {
EE::log( 'Backing up site files.' );
EE::log( 'This may take some time.' );
Expand Down Expand Up @@ -572,6 +600,8 @@ private function backup_wp_content_dir( $backup_dir ) {
EE::error( 'Failed to create backup archive. Please check disk space and file permissions.' );
}

$this->verify_archive_integrity( $backup_file );

return $backup_file;
}

Expand All @@ -593,6 +623,8 @@ private function backup_nginx_conf( $backup_dir ) {
);
EE::error( 'Failed to create nginx configuration backup archive. Please check disk space and file permissions.' );
}

$this->verify_archive_integrity( $backup_file );
}

private function backup_php_conf( $backup_dir ) {
Expand All @@ -613,6 +645,8 @@ private function backup_php_conf( $backup_dir ) {
);
EE::error( 'Failed to create PHP configuration backup archive. Please check disk space and file permissions.' );
}

$this->verify_archive_integrity( $backup_file );
}

private function backup_html( $backup_dir ) {
Expand Down Expand Up @@ -655,17 +689,28 @@ private function backup_db( $backup_dir ) {

$this->fs->mkdir( $backup_dir . '/sql' );

$backup_command = sprintf( 'mysqldump --skip-ssl -u %s -p%s -h %s --single-transaction %s > /var/www/htdocs/%s', $db_user, $db_password, $db_host, $db_name, $sql_filename );
$args = [ 'shell', $this->site_data['site_url'] ];
$assoc_args = [ 'command' => $backup_command ];
$options = [ 'skip-tty' => true ];
// Best-effort layer-1 quoting of DB credentials (consistent with get_db_size()).
// NOTE: the value still passes through a second double-quoted `bash -c "$command"` layer
// inside `ee shell` that escapeshellarg cannot protect, so a password containing ` " or $
// can still break the dump. Fully hardening that inner wrapper is out of scope here.
$backup_command = sprintf(
'mysqldump --skip-ssl -u %s -p%s -h %s --single-transaction %s > /var/www/htdocs/%s',
escapeshellarg( $db_user ),
escapeshellarg( $db_password ),
escapeshellarg( $db_host ),
escapeshellarg( $db_name ),
$sql_filename
);

EE::run_command( $args, $assoc_args, $options );
// Launch via `ee shell` so the dump's exit code is captured. The shell `>` redirect
// creates/truncates the target before mysqldump runs, so a failed dump leaves a 0-byte
// file that passes exists(); rely on the exit code + filesize instead.
$dump_result = EE::launch( sprintf( 'ee shell %s --skip-tty --command=%s', escapeshellarg( $this->site_data['site_url'] ), escapeshellarg( $backup_command ) ) );

$sql_dump_path = EE_ROOT_DIR . '/sites/' . $this->site_data['site_url'] . '/app/htdocs/' . $sql_filename;

// Check if database dump was created successfully
if ( ! $this->fs->exists( $sql_dump_path ) ) {
// A 0-byte or missing dump, or a non-zero exit, means the backup failed.
if ( 0 !== $dump_result->return_code || ! $this->fs->exists( $sql_dump_path ) || filesize( $sql_dump_path ) <= 0 ) {
$this->capture_error(
sprintf( 'Database backup failed for database: %s', $db_name ),
self::ERROR_TYPE_DATABASE,
Expand All @@ -674,7 +719,19 @@ private function backup_db( $backup_dir ) {
EE::error( 'Database backup failed. Please check database credentials and connectivity.' );
}

EE::exec( sprintf( 'mv %s %s', $sql_dump_path, $sql_file ) );
// A failed mv (cross-device, permissions, disk-full, etc.) would leave sql/ empty;
// `7z u` on an empty dir exits 0 and `7z t` passes, shipping a DB-less "successful"
// backup. Fail loudly unless the dump actually landed and is non-empty.
if ( ! EE::exec( sprintf( 'mv %s %s', escapeshellarg( $sql_dump_path ), escapeshellarg( $sql_file ) ) )
|| ! $this->fs->exists( $sql_file ) || filesize( $sql_file ) <= 0 ) {
$this->capture_error(
sprintf( 'Failed to stage database dump for database: %s', $db_name ),
self::ERROR_TYPE_DATABASE,
4003
);
EE::error( 'Database backup failed while staging the dump file.' );
}

$backup_command = sprintf( 'cd %s && 7z u -mx=1 %s sql', $backup_dir, $backup_file );

$result = EE::launch( $backup_command );
Expand Down
Loading