Skip to content

fix(restore): verify backup before extraction and abort on failed restore steps #484

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(restore): verify backup before extraction and abort on failed restore steps #484

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bd980a1
fix(restore): verify backup integrity and abort on failed restore steps
mrrobot47 Jun 29, 2026
3c69f3c
fix(restore): correct integrity check, comments and uploads recovery
mrrobot47 Jun 29, 2026
a2badc6
fix(restore): use escapeshellarg for mysql restore credentials
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
233 changes: 190 additions & 43 deletions src/helper/Site_Backup_Restore.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,20 +713,17 @@ private function maybe_restore_wp_config( $backup_dir ) {
$db_user = $this->site_data['db_user'];
$db_password = $this->site_data['db_password'];
$db_host = $this->site_data['db_host'];
$args = [ 'shell', $this->site_data['site_url'] ];
$options = [ 'skip-tty' => true ];

$command = sprintf( 'wp config set DB_NAME %s', $db_name );
EE::run_command( $args, [ 'command' => $command ], $options );

$command = sprintf( 'wp config set DB_USER %s', $db_user );
EE::run_command( $args, [ 'command' => $command ], $options );

$command = sprintf( 'wp config set DB_PASSWORD %s', $db_password );
EE::run_command( $args, [ 'command' => $command ], $options );

$command = sprintf( 'wp config set DB_HOST %s', $db_host );
EE::run_command( $args, [ 'command' => $command ], $options );
// Abort on failure: wrong/unset DB constants leave the restored site unable to
// connect. escapeshellarg() guards layer-1 word-splitting for the host `ee`
// invocation; note the inner `ee shell` wrapper still runs `bash -c "$command"`
// (a pre-existing limitation), which re-exposes $, backticks and ", so a value
// containing those is not fully carried through. EE's DB credentials are
// alphanumeric by default, so this only affects operator-set special-char creds.
$this->run_checked_shell_command( sprintf( 'wp config set DB_NAME %s', escapeshellarg( $db_name ) ), 'Failed to set DB_NAME in wp-config.' );
$this->run_checked_shell_command( sprintf( 'wp config set DB_USER %s', escapeshellarg( $db_user ) ), 'Failed to set DB_USER in wp-config.' );
$this->run_checked_shell_command( sprintf( 'wp config set DB_PASSWORD %s', escapeshellarg( $db_password ) ), 'Failed to set DB_PASSWORD in wp-config.' );
$this->run_checked_shell_command( sprintf( 'wp config set DB_HOST %s', escapeshellarg( $db_host ) ), 'Failed to set DB_HOST in wp-config.' );
}

private function maybe_restore_custom_docker_compose( $backup_dir ) {
Expand Down Expand Up @@ -758,28 +755,106 @@ private function maybe_restore_custom_docker_compose( $backup_dir ) {
private function restore_db( $sql_file, $container_path ) {
EE::log( 'Restoring database.' );

$site_url = $this->site_data['site_url'];
$db_user = $this->site_data['db_user'];
$db_password = $this->site_data['db_password'];
$db_host = $this->site_data['db_host'];
$db_name = $this->site_data['db_name'];
$sql_path = "/var/www/$container_path/" . basename( $sql_file ); // Use basename for safety

// Corrected command with proper escaping and error suppression for password
$restore_command = sprintf( "mysql --skip-ssl -u '%s' -p'%s' -h '%s' '%s' < '%s' 2>/dev/null", $db_user, $db_password, $db_host, $db_name, $sql_path );
// Keep mysql's own stderr (2>&1) instead of discarding it to /dev/null so a
// failed import surfaces a real diagnostic (mysql never echoes the password
// value, so this does not leak it). escapeshellarg() each value (matching
// get_db_size()) for layer-1 word-splitting/quote safety; `-p%s` yields
// `-p'pass'` with no space, as mysql requires. Note the inner `ee shell`
// `bash -c "$command"` wrapper (a pre-existing limitation) still re-exposes $,
// backticks and ", so a credential containing those is not fully carried
// through. EE's DB credentials are alphanumeric by default, so this only
// affects operator-set special chars.
$restore_command = sprintf(
'mysql --skip-ssl -u %s -p%s -h %s %s < %s 2>&1',
escapeshellarg( $db_user ),
escapeshellarg( $db_password ),
escapeshellarg( $db_host ),
escapeshellarg( $db_name ),
escapeshellarg( $sql_path )
);

$args = [ 'shell', $site_url ];
$assoc_args = [ 'command' => $restore_command ];
$options = [ 'skip-tty' => true ];
EE::run_command( $args, $assoc_args, $options );
// A failed import must abort the restore instead of being reported as success.
$this->run_checked_shell_command( $restore_command, 'Database restore failed.' );
}

private function restore_site( $backup_dir ) {
$backup_app = $backup_dir . '/' . $this->site_data['site_url'] . '.zip';
/**
* Run a command inside the site container via `ee shell` and abort on failure.
*
* EE::run_command() does not surface the in-container command's exit code, so
* critical restore steps are launched as a child `ee shell` process (which exits
* non-zero when the command fails) and the return code is checked here.
*
* @param string $command Command to run inside the container.
* @param string $error_message Message for EE::error() if the command fails.
*/
private function run_checked_shell_command( $command, $error_message ) {
$output = EE::launch( sprintf( 'ee shell %s --skip-tty --command=%s', $this->site_data['site_url'], escapeshellarg( $command ) ) );

if ( $output->return_code ) {
EE::error( $error_message . ' ' . trim( $output->stdout . ' ' . $output->stderr ) );
}
}

/**
* Ensure the site archive exists and is intact before any destructive restore.
*
* The download target is reused if it already exists on disk, so a truncated
* archive left by an interrupted prior run would otherwise be extracted right
* after `rm -rf <app>/*`. Verify the local archive with a `unzip -t` integrity
* test; on failure re-download once and re-test, then abort rather than
* extracting a corrupt archive over the live site.
*
* @param string $backup_dir Local directory holding the downloaded backup.
* @param string $backup_app Path to the site `.zip` archive within $backup_dir.
*/
private function ensure_valid_backup_archive( $backup_dir, $backup_app ) {
if ( ! $this->fs->exists( $backup_app ) ) {
$this->rclone_download( $backup_dir );
} elseif ( ! $this->is_backup_archive_valid( $backup_app ) ) {
// A pre-existing archive that fails verification is likely a partial
// download from an interrupted run; discard it and fetch a fresh copy.
EE::warning( 'Existing backup archive is incomplete or corrupt. Re-downloading.' );
$this->fs->remove( $backup_app );
$this->rclone_download( $backup_dir );
}

if ( ! $this->fs->exists( $backup_app ) || ! $this->is_backup_archive_valid( $backup_app ) ) {
EE::error( 'Downloaded backup archive is incomplete or corrupt. Aborting restore to avoid destroying the existing site.' );
}
}

/**
* Validate a downloaded backup archive with `unzip -t`.
*
* `unzip -t` reads the entire archive and verifies every entry's CRC, so it is
* the definitive guard against a truncated/partial download or a corrupt archive.
* (A size comparison against `rclone size` cannot help: that figure covers the
* whole remote folder -- archive + conf.zip + metadata.json + ... -- so a
* truncated single archive is always smaller and would pass.)
*
* @param string $backup_app Path to the `.zip` archive.
*
* @return bool True if the archive exists, is non-empty and passes `unzip -t`.
*/
private function is_backup_archive_valid( $backup_app ) {
// Cheap guards before the full read: a missing or zero-byte file is invalid.
if ( ! $this->fs->exists( $backup_app ) || 0 === filesize( $backup_app ) ) {
return false;
}

return (bool) EE::exec( sprintf( 'unzip -t %s', escapeshellarg( $backup_app ) ) );
}

private function restore_site( $backup_dir ) {
$backup_app = $backup_dir . '/' . $this->site_data['site_url'] . '.zip';

$this->ensure_valid_backup_archive( $backup_dir, $backup_app );

EE::log( 'Restoring site files.' );

Expand All @@ -788,8 +863,12 @@ private function restore_site( $backup_dir ) {
$remove_command = sprintf( 'rm -rf %s/*', $site_app_dir );
EE::exec( $remove_command );

// The app dir was just wiped; a failed extraction here leaves an empty site,
// so abort instead of proceeding to report a successful restore.
$restore_command = sprintf( 'unzip -o %s -d %s', $backup_app, $site_app_dir );
EE::exec( $restore_command );
if ( ! EE::exec( $restore_command ) ) {
EE::error( 'Failed to extract site files from backup archive.' );
}

$chown_command = sprintf( 'chown -R www-data:www-data %s', \EE\Utils\trailingslashit( $site_app_dir ) );
EE::exec( $chown_command );
Expand All @@ -811,9 +890,7 @@ private function restore_site( $backup_dir ) {
private function restore_wp( $backup_dir ) {
$backup_app = $backup_dir . '/' . $this->site_data['site_url'] . '.zip';

if ( ! $this->fs->exists( $backup_app ) ) {
$this->rclone_download( $backup_dir );
}
$this->ensure_valid_backup_archive( $backup_dir, $backup_app );

EE::log( 'Restoring site files.' );

Expand All @@ -824,7 +901,14 @@ private function restore_wp( $backup_dir ) {
$unzip_meta_command = sprintf( 'unzip -o %s meta.json -d %s', $backup_app, $backup_dir );
EE::exec( $unzip_meta_command );

$meta_data = json_decode( file_get_contents( $backup_dir . '/meta.json' ), true );
// A missing/corrupt meta.json (or one without wordpressVersion) would make
// $wp_version empty, and `wp core download` with an empty --version silently
// installs the LATEST WordPress -- a different version than was backed up.
$meta_file = $backup_dir . '/meta.json';
$meta_data = $this->fs->exists( $meta_file ) ? json_decode( file_get_contents( $meta_file ), true ) : null;
if ( ! is_array( $meta_data ) || empty( $meta_data['wordpressVersion'] ) ) {
EE::error( 'Backup metadata (meta.json) is missing or corrupt. Cannot determine the WordPress version to restore.' );
}
$wp_version = $meta_data['wordpressVersion'];

// $wp_version is read from the backup's meta.json and interpolated into the
Expand All @@ -833,46 +917,73 @@ private function restore_wp( $backup_dir ) {
// only ever contains [0-9A-Za-z.-]; strip anything else so no shell
// metacharacter can survive either shell layer.
$wp_version = preg_replace( '/[^0-9A-Za-z.\-]/', '', (string) $wp_version );
if ( '' === $wp_version ) {
EE::error( 'Backup metadata contains an invalid WordPress version.' );
}

// wp core download extracts the WordPress archive in PHP, which needs more
// than a typical site's 128M memory_limit and OOMs on low-RAM hosts. Run it
// under a higher limit via `php -d memory_limit=256M $(which wp)`, matching
// the site-creation path in site-type-wp. The command runs through `bash -c`
// in the container, so the `$` in `$(which wp)` is escaped here to defer the
// substitution to the container's shell (EE's `wp` is the phar, invoked
// directly, so the WP_CLI_PHP_ARGS env var would not apply).
$args = [ 'shell', $this->site_data['site_url'] ];
$assoc_args = [ 'command' => sprintf( "php -d memory_limit=256M \\$(which wp) core download --force --version=%s", $wp_version ) ];
$options = [ 'skip-tty' => true ];
EE::run_command( $args, $assoc_args, $options );
// the site-creation path in site-type-wp. A failed download must abort: the
// restore otherwise removes wp-content with no fresh core to replace it.
// $(which wp) stays unescaped so escapeshellarg single-quotes it for the host
// and the container's `bash -c` does the substitution (EE's `wp` is the phar,
// invoked directly, so the WP_CLI_PHP_ARGS env var would not apply).
$core_download_command = sprintf( 'php -d memory_limit=256M $(which wp) core download --force --version=%s', $wp_version );
$this->run_checked_shell_command( $core_download_command, 'Failed to download WordPress core for restore.' );

$this->maybe_restore_wp_config( $backup_dir );

// Extract and import the DB dump only if the archive actually contains one.
// `unzip` exits 11 ("nothing matched") for a DB-less backup, which is not an
// error here -- archive integrity was already verified with `unzip -t`, so a
// missing member just means no DB was backed up (mirrors restore_site()'s
// guard). Any other non-zero code is a real extraction failure and aborts.
$restore_command = sprintf( 'unzip -o %s sql/%s.sql -d %s/app/', $backup_app, $this->site_data['site_url'], $this->site_data['site_fs_path'] );
EE::exec( $restore_command );
$unzip_sql = EE::launch( $restore_command );

$this->restore_db( $this->site_data['site_url'] . '.sql', 'sql' );
$this->fs->remove( $this->site_data['site_fs_path'] . '/app/sql' );
if ( 0 === $unzip_sql->return_code ) {
$this->restore_db( $this->site_data['site_url'] . '.sql', 'sql' );
$this->fs->remove( $this->site_data['site_fs_path'] . '/app/sql' );
} elseif ( 11 === $unzip_sql->return_code ) {
EE::debug( 'No database dump found in backup archive; skipping database restore.' );
} else {
EE::error( 'Failed to extract database dump from backup archive.' );
}

$uploads_moved = false;
// if wp-content/uploads is symlink, then move it one level up
if ( is_link( $site_dir . '/wp-content/uploads' ) ) {
// move the symlink one level up for time being
$mv_command = sprintf( 'mv %s/wp-content/uploads %s/uploads', $site_dir, $site_dir );
EE::exec( $mv_command );
$uploads_moved = true;
// Only mark as moved if the mv actually succeeded -- otherwise the
// `rm -rf wp-content` below would delete the live uploads dir and the
// move-back step would silently recreate nothing.
if ( EE::exec( $mv_command ) ) {
$uploads_moved = true;
} else {
EE::error( 'Failed to preserve wp-content/uploads before restore. Aborting to avoid data loss.' );
}
}

// Remove all files from wp-content except uploads
$this->fs->remove( $site_dir . '/wp-content' );

$wp_content_command = sprintf( "unzip -o %s 'wp-content/*' -x 'wp-content/uploads/*' -d %s", $backup_app, $site_dir );
EE::exec( $wp_content_command );
if ( ! EE::exec( $wp_content_command ) ) {
// wp-content was just removed; if uploads was moved aside it is now
// orphaned at $site_dir/uploads and a retry can't detect it. Put it back
// (best effort) before aborting so the site keeps its uploads reference.
$this->restore_moved_uploads( $site_dir, $uploads_moved );
EE::error( 'Failed to restore wp-content from backup archive.' );
}

if ( $uploads_moved ) {
// move the uploads directory back to wp-content
$mv_command = sprintf( 'mv %s/uploads %s/wp-content/uploads', $site_dir, $site_dir );
EE::exec( $mv_command );
if ( ! EE::exec( $mv_command ) ) {
EE::error( 'Failed to restore the preserved wp-content/uploads directory. The original uploads symlink is preserved at ' . $site_dir . '/uploads.' );
}
}

$uploads_command = sprintf( "unzip -o %s 'wp-content/uploads/*' -d %s", $backup_app, $site_dir );
Expand All @@ -893,6 +1004,29 @@ private function restore_wp( $backup_dir ) {
EE::run_command( $args, $assoc_args, $options );
}

/**
* Best-effort move of the uploads symlink that was set aside during the
* restore dance back into wp-content, used on abort paths where wp-content has
* already been removed so the orphaned symlink at $site_dir/uploads would
* otherwise be undetectable (and lost) on a retry.
*
* @param string $site_dir WordPress content parent dir (holds wp-content).
* @param bool $uploads_moved Whether the uploads symlink was moved aside.
*/
private function restore_moved_uploads( $site_dir, $uploads_moved ) {
if ( ! $uploads_moved || ! is_link( $site_dir . '/uploads' ) ) {
return;
}

// Recreate wp-content if the failed step left it missing, then move the
// symlink back. Failures here are non-fatal: the caller aborts regardless,
// and the symlink is left in place at $site_dir/uploads either way.
if ( ! $this->fs->exists( $site_dir . '/wp-content' ) ) {
$this->fs->mkdir( $site_dir . '/wp-content' );
}
EE::exec( sprintf( 'mv %s/uploads %s/wp-content/uploads', $site_dir, $site_dir ) );
}

/**
* Verify rclone is installed and the configured backend exists.
*
Expand Down Expand Up @@ -1062,7 +1196,14 @@ private function pre_restore_check() {
EE::error( 'Failed to get remote backup size.' );
}

$remote_size = json_decode( $output->stdout, true )['bytes'];
// `rclone size --json` must decode to an object with a numeric `bytes`;
// otherwise `null['bytes']` would make the disk-space guard below a no-op.
$remote_size_data = json_decode( $output->stdout, true );
if ( ! is_array( $remote_size_data ) || ! isset( $remote_size_data['bytes'] ) || ! is_numeric( $remote_size_data['bytes'] ) ) {
EE::error( 'Could not determine remote backup size: rclone returned invalid data.' );
}

$remote_size = (int) $remote_size_data['bytes'];
EE::debug( 'Remote backup size: ' . $remote_size );

$free_space = disk_free_space( EE_BACKUP_DIR );
Expand Down Expand Up @@ -1110,6 +1251,12 @@ private function pre_restore_check() {

$backup_site_data = json_decode( file_get_contents( $backup_site_info ), true );

// A missing/corrupt metadata.json (or one lacking the keys compared below)
// would otherwise silently bypass the site-type / public-dir match checks.
if ( ! is_array( $backup_site_data ) || ! isset( $backup_site_data['site_type'], $backup_site_data['site_container_fs_path'] ) ) {
EE::error( 'Backup metadata is missing or corrupt. Cannot safely restore this backup.' );
}

if ( $this->site_data['site_type'] !== $backup_site_data['site_type'] ) {
EE::error( 'Site type does not match with the backed up site.' );
}
Expand Down
Loading