extension-command
extension-command copied to clipboard
wp-cli
wp theme update testtheme --version=xxx deletes theme if not in WP gallery
Summary: "wp theme update" deletes theme folder if theme is not in WordPress.org theme directory
Steps to reproduce: Create empty folder testtheme in wp-content/themes folder wp theme update testtheme --version=2.0
Environment: wp-cli 2.0.1 Amazon Linux 2 PHP 7.2.11 WP 5.0
Severity - High (Deletes data & breaks sites for themes not in WordPress.org theme directory)
Expected Results:
No change to theme
Output: ERROR: Theme not found in WordPress.org theme directory and cannot be updated with WP CLI"
Actual Results: Theme folder and all contents are deleted. Site broken if active theme. Outputs as follows: Warning: Couldn't find 'testtheme' in the WordPress.org theme directory. Error: No themes installed.
This is indeed a bug in the version-specific update code path: https://github.com/wp-cli/extension-command/blob/v2.0.2/src/Theme_Command.php#L678-L684
WP-CLI first deletes the theme unconditionally and only then tries to install it again.
I can see two different ways of solving this:
- Move the theme instead of deleting it. Once the install succeeded, delete the previous theme, otherwise clean up and move the old one back in place.
- Start with a check to make sure that the theme is actually available for installation for deleting the old one.
I'd say 1. is the safer bet, as we can also handle unexpected errors that way.
Option 1 sounds good! If "BackupAddon" and "RestoreAddon" can be made generic in a helper class then this process can be leveraged for any update as I'd guess there are other conditions where an addon may get deleted unintentionally. The only issue I see is if there isn't enough disk space to store 2 copies of an addon, but an addon-size check and a free space check would mitigate that risk.
Failing to create a backup and throwing an error is still preferable to "delete-first", so I don't see the size argument as a blocker.
Good point. Whatever approach resolves this issue before breaks occur is best.
I do want to flag that the thinking behind this case of "delete-first" may be elsewhere in the cli, but I've only seen it happen in themes. In the version before 2.0 I saw theme deletion when trying to update any theme that wasn't in the gallery, but in current version I've only seen it when using this --version= flag.
Thanks for all your work on this project! I can't imagine working with Wordpress sites without it.
I can see two different ways of solving this
Why not a combo of both? Do a check that the theme exists in the gallery before doing anything (and alert the user), and also move the theme temporarily and wait for successful update before deleting (also alert the user).
Continues to happen, WP-CLI v2.4.0.
