Add Template Library and move Site Exporter to core #304
Conversation
## Template Library Feature - Add new Template Library admin page for browsing and installing templates - Add template categories (Business, Portfolio, Blog, E-commerce, Agency, SaaS, Community) - Add API client for fetching templates from ultimatemultisite.com - Add template installer with OAuth authentication support - Add template repository with caching - Add upload template feature for importing custom templates from ZIP files - Add JavaScript UI for template browsing with search and filtering ## Site Exporter in Core - Move Site Exporter addon functionality into core plugin - Add namespace WP_Ultimo\Site_Exporter for all exporter classes - Add database classes (Manager, Import, Replace, Max_Execution_Time) - Add MU-Migration library for export/import operations - Add exporter and importer function files with backward-compatible aliases - Add auto-deactivation of Site Exporter addon when core is updated ## WordPress Sites Integration - Add export action link to each site row in wp-admin/network/sites.php - Add bulk export action for multiple sites - Add Export & Import submenu page under Sites menu - Works without requiring Ultimate Multisite setup wizard completion ## Ultimate Multisite Integration - Add Export Site and Import Site action links to Sites list page - Add Site Export widget to Site edit page with previous exports list - Add extensibility hooks for action links and widgets ## Code Quality - Enforce 'ultimate-multisite' text domain via PHPCS - Update all translation calls from 'wp-ultimo' to 'ultimate-multisite' - Add phpcs:disable for ported third-party database classes - Exclude mu-migration from PHPStan checks - Fix all security escaping issues in new files 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThis PR introduces a comprehensive site export/import system with a dedicated WP-CLI migration package, a new Template Library feature for managing remote templates, multiple helper APIs for export/import operations, and updates branding references from 'wp-ultimo' to 'ultimate-multisite' throughout the codebase. New admin pages, hooks, database utilities, and extensive Behat tests are included. Changes
Sequence DiagramssequenceDiagram
participant Admin as Admin User
participant Page as Template Library Page
participant AJAX as AJAX Handler
participant API as API Client
participant Cache as Transient Cache
participant Installer as Template Installer
Admin->>Page: Load Template Library
Page->>AJAX: serve_templates_list (AJAX)
AJAX->>Cache: Check cached templates
alt Cache Hit
Cache-->>AJAX: Return cached data
else Cache Miss
AJAX->>API: get_templates()
API->>API: Fetch from remote API
API-->>AJAX: Template data
AJAX->>Cache: Store in transient
end
AJAX-->>Page: Template list (JSON)
Page->>Page: Filter by category/search
Page->>Page: Render template grid
Admin->>Page: Click "Install Now"
Page->>Page: Validate install
Page->>Installer: install_template()
Installer->>Installer: Get download URL
Installer->>Installer: Download ZIP
Installer->>Installer: Import via exporter
Installer->>Installer: Mark as installed
Installer-->>Page: Installation complete
Page->>Page: Show success message
sequenceDiagram
participant CLI as WP-CLI
participant Export as ExportCommand
participant Site as WordPress Site
participant DB as Database
participant FS as File System
participant Zip as Zip Handler
CLI->>Export: wp mu-migration export
Export->>Site: Switch to target blog
Export->>Export: Get CSV headers
Export->>DB: Query users
DB-->>Export: User data
Export->>FS: Write users.csv
Export->>Export: Get tables list
Export->>DB: Export tables
DB-->>Export: SQL dump
Export->>FS: Write tables.sql
Export->>FS: Write meta.json
Export->>Zip: Create zip with files
Zip-->>FS: Export package (ZIP)
Export->>Site: Restore blog context
Export-->>CLI: Export complete
sequenceDiagram
participant CLI as WP-CLI
participant Import as ImportCommand
participant FS as File System
participant DB as Database
participant Site as WordPress
participant Helpers as Migration Helpers
CLI->>Import: wp mu-migration import --zip-file=export.zip
Import->>FS: Extract ZIP
FS-->>Import: Extracted files
Import->>Import: Parse meta.json
Import->>Site: Create/select target site
Import->>FS: Read users.csv
FS-->>Import: User data
Import->>DB: Import users
Import->>Import: Build ID map (old→new)
Import->>FS: Write map file
Import->>FS: Read tables.sql
FS-->>Import: SQL dump
Import->>Helpers: Perform prefix replacement
Helpers->>DB: Run search-replace
Import->>DB: Import tables
Import->>Site: Update post authors
Import->>FS: Move plugins/themes/uploads
Import->>Site: Update blog settings
Import->>FS: Cleanup temp files
Import-->>CLI: Import complete
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes This is a substantial PR introducing three major new subsystems (Site Exporter with 10+ classes, MU-Migration WP-CLI package with 50+ files including extensive Behat tests, and Template Library with 4 core classes and admin integration). The changes are highly heterogeneous: database utilities, WP-CLI commands, Behat test infrastructure, JavaScript components, view templates, PHP helpers, admin pages, and configuration updates. Logic density is high in areas like search-replace, database import/export, and migration orchestration. File diversity spans multiple namespaces and concerns, requiring separate reasoning for each subsystem and integration points. Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/helpers/class-hash.php (1)
43-63: Fix three Hash::decode() calls to use explicit group parameter.The default group change breaks existing hashes in three list table search functions. Hashids decoding requires the instance configuration to match the encoding configuration, otherwise the returned value will be empty. These three decode calls without an explicit group will fail to decode hashes previously created with the old default:
inc/list-tables/class-base-list-table.php:257inc/list-tables/class-membership-list-table-widget.php:75inc/list-tables/class-payment-list-table-widget.php:75Update each to pass the appropriate group parameter matching their encode partner, or standardize to a permanent default and update both encode and decode calls consistently.
🟠 Major comments (17)
inc/site-exporter/database/class-max-execution-time.php-34-41 (1)
34-41: Logic issue:store()only called when$time === 0.The current implementation only stores the original limit when setting to unlimited (
$time = 0). If a caller usesset(60)followed byrestore(), the restored value will be0(the class default) instead of the actual previous limit.Consider always storing before setting:
🔎 Proposed fix
public function set(int $time = 0): void { - if (0 === $time) { - $this->store(); - } + $this->store(); @set_time_limit($time); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged }inc/site-exporter/class-site-exporter.php-1543-1553 (1)
1543-1553: Therunningflag is set tofalsewhich seems incorrect.When marking an import as "in progress", setting
$options['running'] = falseappears to be a logic error. This should likely betrueto indicate the import is currently running.🔎 Proposed fix
if (empty($file_name)) { return false; } - $options['running'] = false; + $options['running'] = true; $base = [ $file_name, $options, $hash, ];inc/site-exporter/class-site-exporter.php-1307-1330 (1)
1307-1330:reset_upload_limits()has security and reliability concerns.
@ini_set()calls may silently fail if PHP is configured to ignore runtime INI changes- Defining
ALLOW_UNFILTERED_UPLOADSglobally is a significant security risk as it allows uploading any file type network-wideConsider limiting the scope of
ALLOW_UNFILTERED_UPLOADS:- if (! defined('ALLOW_UNFILTERED_UPLOADS')) { - define('ALLOW_UNFILTERED_UPLOADS', true); - } + // Use filter instead of constant for more controlled unfiltered upload access + add_filter('wp_check_filetype_and_ext', function($data, $file, $filename, $mimes) { + $ext = pathinfo($filename, PATHINFO_EXTENSION); + if (in_array($ext, ['zip', 'gz'], true)) { + $data['ext'] = $ext; + $data['type'] = $ext === 'zip' ? 'application/zip' : 'application/x-gzip'; + } + return $data; + }, 10, 4);inc/site-exporter/mu-migration/features/bootstrap/utils.php-598-615 (1)
598-615: SSL verification fallback weakens security posture.When SSL verification fails, the code falls back to disabling verification (
$options['verify'] = false). While this helps with development environments, it could mask certificate issues in production and expose users to MITM attacks.Consider adding a configuration option to control this behavior rather than always falling back:
} catch ( \Requests_Exception $ex ) { // CURLE_SSL_CACERT_BADFILE only defined for PHP >= 7. if ( 'curlerror' !== $ex->getType() || ! in_array(curl_errno($ex->getData()), [CURLE_SSL_CONNECT_ERROR, CURLE_SSL_CERTPROBLEM, 77 /*CURLE_SSL_CACERT_BADFILE*/], true) ) { \WP_CLI::error(sprintf("Failed to get url '%s': %s.", $url, $ex->getMessage())); } - // Handle SSL certificate issues gracefully - \WP_CLI::warning(sprintf("Re-trying without verify after failing to get verified url '%s' %s.", $url, $ex->getMessage())); - $options['verify'] = false; - try { - return \Requests::request($url, $headers, $data, $method, $options); - } catch ( \Requests_Exception $ex ) { - \WP_CLI::error(sprintf("Failed to get non-verified url '%s' %s.", $url, $ex->getMessage())); - } + // Only retry without verification if explicitly allowed + if ( getenv('WP_CLI_ALLOW_INSECURE_SSL') ) { + \WP_CLI::warning(sprintf("Re-trying without verify after failing to get verified url '%s' %s.", $url, $ex->getMessage())); + $options['verify'] = false; + try { + return \Requests::request($url, $headers, $data, $method, $options); + } catch ( \Requests_Exception $ex ) { + \WP_CLI::error(sprintf("Failed to get non-verified url '%s' %s.", $url, $ex->getMessage())); + } + } else { + \WP_CLI::error(sprintf("SSL verification failed for '%s': %s. Set WP_CLI_ALLOW_INSECURE_SSL=1 to retry without verification.", $url, $ex->getMessage())); + } }inc/functions/importer.php-35-42 (1)
35-42: Incorrectdo_action_ref_arrayusage.The third argument to
do_action_ref_arrayis not valid. This function only accepts two parameters: the hook name and an array of arguments. The'site-import'string will be silently ignored, but this appears to be a mistake.🔎 Proposed fix
} else { - do_action_ref_array( - 'wu_import_site', - [ - 'file_name' => $file_name, - 'options' => $options, - ], - 'site-import' - ); + do_action( + 'wu_import_site', + $file_name, + $options + ); }Or if you need to pass an associative array:
} else { - do_action_ref_array( + do_action( 'wu_import_site', [ 'file_name' => $file_name, 'options' => $options, - ], - 'site-import' + ] ); }inc/site-exporter/mu-migration/mu-migration.php-29-40 (1)
29-40: Relative paths in require statements may cause issues.The
require_oncestatements use relative paths which depend on the current working directory. When this file is included from a different context (e.g., fromSite_Exporter::load_dependencies()), these paths may fail.🔎 Proposed fix using __DIR__ for reliable paths
if ( file_exists(__DIR__ . '/vendor/autoload.php') ) { - require_once 'vendor/autoload.php'; + require_once __DIR__ . '/vendor/autoload.php'; } -require_once 'includes/helpers.php'; +require_once __DIR__ . '/includes/helpers.php'; -require_once TENUP_MU_MIGRATION_COMMANDS_PATH . 'class-mu-migration.php'; -require_once TENUP_MU_MIGRATION_COMMANDS_PATH . 'class-mu-migration-base.php'; -require_once TENUP_MU_MIGRATION_COMMANDS_PATH . 'class-mu-migration-export.php'; -require_once TENUP_MU_MIGRATION_COMMANDS_PATH . 'class-mu-migration-import.php'; -require_once TENUP_MU_MIGRATION_COMMANDS_PATH . 'class-mu-migration-posts.php'; -require_once TENUP_MU_MIGRATION_COMMANDS_PATH . 'class-mu-migration-users.php'; +require_once __DIR__ . '/' . TENUP_MU_MIGRATION_COMMANDS_PATH . 'class-mu-migration.php'; +require_once __DIR__ . '/' . TENUP_MU_MIGRATION_COMMANDS_PATH . 'class-mu-migration-base.php'; +require_once __DIR__ . '/' . TENUP_MU_MIGRATION_COMMANDS_PATH . 'class-mu-migration-export.php'; +require_once __DIR__ . '/' . TENUP_MU_MIGRATION_COMMANDS_PATH . 'class-mu-migration-import.php'; +require_once __DIR__ . '/' . TENUP_MU_MIGRATION_COMMANDS_PATH . 'class-mu-migration-posts.php'; +require_once __DIR__ . '/' . TENUP_MU_MIGRATION_COMMANDS_PATH . 'class-mu-migration-users.php';inc/site-exporter/mu-migration/utils/behat-tags.php-20-20 (1)
20-20: Security: Command injection risk with exec().Lines 20 and 56 use
exec()with string interpolation without proper escaping. The$prefixvariable (line 20) is interpolated directly into the grep command, creating a potential command injection vulnerability if$prefixever comes from untrusted input.While
$prefixis currently hardcoded in the calling code, this is a defensive coding concern that could become exploitable if the code is refactored.🔎 Proposed fix using escapeshellarg()
- exec("grep '@{$prefix}-[0-9\.]*' -h -o features/*.feature | uniq", $existing_tags); + $safe_prefix = escapeshellarg("@{$prefix}-"); + exec("grep {$safe_prefix} -h -o features/*.feature | uniq", $existing_tags);For line 56:
- exec("grep '@require-extension-[A-Za-z_]*' -h -o features/*.feature | uniq", $extension_tags); + exec("grep " . escapeshellarg('@require-extension-') . "[A-Za-z_]* -h -o features/*.feature | uniq", $extension_tags);Also applies to: 56-56
inc/site-exporter/mu-migration/features/tests/csv_matches_user.php-53-53 (1)
53-53: Use strict comparison for metadata validation.Line 53 uses loose comparison (
!=) which can lead to unexpected type coercion. Metadata validation should use strict comparison to ensure type safety.🔎 Proposed fix
- if ( isset($actual_user_meta[ $key ]) && $actual_user_meta[ $key ][0] != $expected_user_data[ $key ] ) { + if ( isset($actual_user_meta[ $key ]) && $actual_user_meta[ $key ][0] !== $expected_user_data[ $key ] ) {inc/site-exporter/mu-migration/features/tests/csv_matches_user.php-40-40 (1)
40-40: Use strict comparison for data validation.Line 40 uses loose comparison (
!=) which can lead to unexpected type coercion. User data validation should use strict comparison to ensure type safety.🔎 Proposed fix
- if ( isset($actual_user_data->$key) && $actual_user_data->$key != $expected_user_data[ $key ] ) { + if ( isset($actual_user_data->$key) && $actual_user_data->$key !== $expected_user_data[ $key ] ) {inc/site-exporter/mu-migration/includes/commands/class-mu-migration-base.php-116-153 (1)
116-153: Add memory management to all_records method.The
all_postsmethod callsHelpers\stop_the_insanity()(line 89) to prevent memory leaks during batch processing, butall_recordsdoesn't. This could lead to memory exhaustion when processing large tables.🔎 Proposed fix
foreach ( $results as $result ) { $callback($result); $progress_bar->tick(); } } + Helpers\stop_the_insanity(); $offset += $step; } while ( $results );inc/site-exporter/database/class-import.php-180-186 (1)
180-186: Multiple SQL statements in a single query() call may fail.PDO by default does not allow multiple statements in a single
query()call. The compound statementSET FOREIGN_KEY_CHECKS=0; DROP TABLE ...; SET FOREIGN_KEY_CHECKS=1;requiresPDO::ATTR_EMULATE_PREPARES = true, which is not set.🔎 Proposed fix: Execute statements separately
if ($this->forceDropTables === true) { // Delete table with foreign key checks disabled - $this->query('SET FOREIGN_KEY_CHECKS=0; DROP TABLE `' . $table . '`; SET FOREIGN_KEY_CHECKS=1;'); + $this->query('SET FOREIGN_KEY_CHECKS=0'); + $this->query('DROP TABLE `' . $table . '`'); + $this->query('SET FOREIGN_KEY_CHECKS=1'); } else {inc/site-exporter/mu-migration/includes/commands/class-mu-migration-import.php-706-713 (1)
706-713: Command injection vulnerability in sed command.The
$filename,$old_db_prefix, and$new_prefixvariables are interpolated directly into a shell command without escaping, creating a potential command injection vulnerability.🔎 Proposed fix: Use escapeshellarg()
foreach ( $sed_commands as $sed_command ) { - $full_command = "sed '$sed_command' -i $filename"; + $full_command = 'sed ' . escapeshellarg($sed_command) . ' -i ' . escapeshellarg($filename); $sed_result = \WP_CLI::launch($full_command, false, false); if ( 0 !== $sed_result ) { \WP_CLI::warning(__('Something went wrong while running sed', 'mu-migration')); } }Note: The
$sed_commandvariable contains$old_db_prefixand$new_prefixwhich should also be validated/sanitized before use in the regex pattern on line 703.inc/site-exporter/database/class-import.php-126-135 (1)
126-135: Connection failure doesn't halt execution.When
connect()throws aPDOException, the exception is caught and an error is echoed, but$this->dbremains uninitialized. Subsequent calls to$this->db->query()inopenfile()ordropTables()will throw a fatal error.🔎 Proposed fix: Re-throw exception or set a flag
private function connect(): void { try { $this->db = new PDO('mysql:host=' . $this->host . ';dbname=' . $this->database, $this->username, $this->password); $this->db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); } catch (PDOException $e) { // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped echo 'Cannot connect: ' . $e->getMessage() . "\n"; + throw $e; // Allow caller to handle connection failure } }inc/site-exporter/database/class-replace.php-237-246 (1)
237-246: Bug: CSV replacement uses stale data instead of cumulative edits.When applying CSV replacements, the non-serialized branch uses
$data_to_fix(original data) instead of$edited_data(already modified by previous search/replace). This means CSV replacements won't stack correctly with the primary search/replace.🔎 Proposed fix
// Run a search replace by CSV parameters if CSV input present if (null !== $csv) { foreach ($this->csv_data as $entry) { $edited_data = is_serialized($edited_data, false) ? $this->recursive_unserialize_replace( $entry['search'], $entry['replace'], $edited_data - ) : str_replace($entry['search'], $entry['replace'], $data_to_fix); + ) : str_replace($entry['search'], $entry['replace'], $edited_data); } }inc/admin-pages/class-template-library-admin-page.php-120-125 (1)
120-125: Missing nonce verification for AJAX endpointserve_templates_list.The
serve_templates_listAJAX action lacks nonce verification and capability checks, making it publicly accessible. While it only returns template data, this could expose template metadata to unauthenticated users.🔎 Proposed fix
public function serve_templates_list(): void { + if (! current_user_can('manage_network')) { + wp_send_json_error(new \WP_Error('forbidden', __('Permission denied.', 'ultimate-multisite'))); + } + $templates_list = $this->get_templates_list(); wp_send_json_success($templates_list); }Additionally, consider adding a nonce check if this endpoint is meant to be called from the frontend JS.
Committable suggestion skipped: line range outside the PR's diff.
inc/site-exporter/database/class-manager.php-53-68 (1)
53-68: Potential SQL injection in LIKE pattern and missing space in query.Line 57 has a missing space before
LIKEwhich could cause query failures. Additionally, the base prefix is directly concatenated into the query without escaping special LIKE characters (%,_).🔎 Proposed fix
public function get_tables(int $blog_id = 0): array { if (function_exists('is_multisite') && is_multisite()) { if (is_main_site() && $blog_id === 0) { - $tables = $this->wpdb->get_col("SHOW TABLES LIKE'" . $this->wpdb->base_prefix . "%'"); + $prefix = $this->wpdb->esc_like($this->wpdb->base_prefix); + $tables = $this->wpdb->get_col("SHOW TABLES LIKE '" . $prefix . "%'"); } else { + $prefix = $this->wpdb->esc_like($this->wpdb->base_prefix . absint($blog_id)); $tables = $this->wpdb->get_col( - "SHOW TABLES LIKE '" . $this->wpdb->base_prefix . absint($blog_id) . "\_%'" + "SHOW TABLES LIKE '" . $prefix . "\_%'" ); } } else { - $tables = $this->wpdb->get_col("SHOW TABLES LIKE'" . $this->wpdb->base_prefix . "%'"); + $prefix = $this->wpdb->esc_like($this->wpdb->base_prefix); + $tables = $this->wpdb->get_col("SHOW TABLES LIKE '" . $prefix . "%'"); } return $tables; }inc/site-exporter/database/class-manager.php-136-141 (1)
136-141: SQL injection risk: Table name and LIMIT parameters are not parameterized.The
$table,$start, and$endvalues are directly interpolated into the query. While$startand$endare typed asint,$tableshould be validated or properly parameterized.🔎 Proposed fix using wpdb->prepare for LIMIT
public function get_table_content(string $table, int $start, int $end): ?array { - $data = $this->wpdb->get_results("SELECT * FROM $table LIMIT $start, $end", ARRAY_A); + $data = $this->wpdb->get_results( + $this->wpdb->prepare("SELECT * FROM `%i` LIMIT %d, %d", $table, $start, $end), + ARRAY_A + ); return $data; }Note: The
%iidentifier placeholder requires WordPress 6.2+. For older versions, validate$tableagainstget_tables()output.
🟡 Minor comments (24)
tests/WP_Ultimo/Helpers/Unique_Customer_Email_Test.php-1-1 (1)
1-1: Filename does not follow WordPress coding standards.Static analysis reports this file should follow WordPress naming conventions for test files. The filename should be lowercase with hyphens as word separators.
Expected:
class-unique-customer-email-test.php
Found:Unique_Customer_Email_Test.phpRename the file to comply with the project's PHPCS rules.
tests/WP_Ultimo/Helpers/Unique_Customer_Email_Test.php-56-58 (1)
56-58: Multi-line function call formatting violates PHPCS rules.The opening parenthesis of multi-line function calls must be the last content on the line, and the closing parenthesis must be on a line by itself. This applies to lines 56-58, 89-93, and 130-134.
🔎 Proposed fix for lines 56-58
- $user_id = $this->factory()->user->create([ - 'user_email' => $email, - ]); + $user_id = $this->factory()->user->create( + [ + 'user_email' => $email, + ] + );🔎 Proposed fix for lines 89-93
- $customer = wu_create_customer([ - 'email' => $email, - 'username' => 'testcustomer' . wp_generate_password(8, false), - 'password' => 'password123', - ]); + $customer = wu_create_customer( + [ + 'email' => $email, + 'username' => 'testcustomer' . wp_generate_password(8, false), + 'password' => 'password123', + ] + );🔎 Proposed fix for lines 130-134
- $customer = wu_create_customer([ - 'email' => $email, - 'username' => 'validatorcust' . wp_generate_password(8, false), - 'password' => 'password123', - ]); + $customer = wu_create_customer( + [ + 'email' => $email, + 'username' => 'validatorcust' . wp_generate_password(8, false), + 'password' => 'password123', + ] + );Also applies to: 89-93, 130-134
inc/checkout/composer.json-1-14 (1)
1-14: Replace placeholder values or remove the file.This composer.json file contains placeholder values that should not be committed to the repository:
"vendor_name/checkout"as the package name"description"as the description"dave"and"email@example.com"as author credentials- Empty
requireblockIf this file is not needed, consider removing it. If it serves a purpose, populate it with proper metadata.
inc/site-exporter/mu-migration/features/extra/no-mail.php-3-6 (1)
3-6: Match the WordPresswp_mail()signature.The stub function signature doesn't match the real
wp_mail()function, which could cause PHP errors if tests call it with standard parameters like subject, message, headers, or attachments. The function should also return a boolean value.🔎 Proposed fix
-function wp_mail($to) { +function wp_mail($to, $subject = '', $message = '', $headers = '', $attachments = array()) { // Log for testing purposes WP_CLI::log("WP-CLI test suite: Sent email to {$to}."); + return true; }inc/site-exporter/mu-migration/composer.json-10-10 (1)
10-10: Consider upgradingalchemy/zippyto a more recent version.While 0.4.8 has no reported security vulnerabilities, it is from 2017. Consider upgrading to 0.4.9 (same release branch) or 1.0.0 (latest, requires PHP >= 7.1) for access to newer features and ongoing maintenance.
inc/site-exporter/mu-migration/includes/helpers.php-283-289 (1)
283-289: Missing directory existence check beforemkdir().The
mkdir($dest_dir)call will emit a warning if the directory already exists. Consider adding a check for consistency withmove_folder().🔎 Proposed fix
function extract($filename, $dest_dir) { $zippy = Zippy::load(); $site_package = $zippy->open($filename); - mkdir($dest_dir); + if ( ! file_exists($dest_dir) ) { + mkdir($dest_dir, 0755, true); + } $site_package->extract($dest_dir); }inc/functions/exporter.php-416-432 (1)
416-432: Missing_deprecated_function()calls in transient aliases.These two deprecated wrapper functions don't emit deprecation notices, unlike all other aliases in this file.
🔎 Proposed fix
function wp_ultimo_site_exporter_set_transient(string $transient, $value, int $expiration = 0): bool { + _deprecated_function(__FUNCTION__, '2.5.0', 'wu_exporter_set_transient'); + return wu_exporter_set_transient($transient, $value, $expiration); } ... function wp_ultimo_site_exporter_delete_transient(string $transient): bool { + _deprecated_function(__FUNCTION__, '2.5.0', 'wu_exporter_delete_transient'); + return wu_exporter_delete_transient($transient); }inc/functions/exporter.php-46-55 (1)
46-55: Incorrect use ofdo_action_ref_array.The third parameter
'site-exporter'is not valid fordo_action_ref_array. This function takes only two parameters: the hook name and an array of arguments. The'site-exporter'string will be silently ignored.🔎 Proposed fix
} else { - do_action_ref_array( - 'wu_export_site', - [ + do_action( + 'wu_export_site', + [ 'site_id' => $site_id, 'options' => $options, - ], - 'site-exporter' + ] ); }inc/site-exporter/mu-migration/mu-migration.php-6-6 (1)
6-6: Version mismatch between plugin header and constant.The plugin header declares
Version: 0.3.1but the constant definesTENUP_MU_MIGRATION_VERSIONas'0.3.2'. These should be synchronized to avoid confusion.🔎 Proposed fix
- * Version: 0.3.1 + * Version: 0.3.2Also applies to: 15-18
inc/site-exporter/class-site-exporter.php-1580-1588 (1)
1580-1588: Logic error in delete_file condition check.Using
isset($options['delete_file'])only checks if the key exists, not if it's truthy. Ifdelete_fileis explicitly set tofalse, the file would still be deleted.🔎 Proposed fix
- $delete_file = isset($options['delete_file']); + $delete_file = ! empty($options['delete_file']); if ($delete_file) { $attachment_id = attachment_url_to_postid($options['zip_url']); wp_delete_attachment($attachment_id, true); }inc/site-exporter/class-site-exporter.php-1137-1148 (1)
1137-1148:wp_delete_file()returns void, not a boolean.The
$successvariable is assigned the return value ofwp_delete_file(), but this function returnsvoid. The variable is unused and the deletion success cannot be verified this way.🔎 Proposed fix
- $success = wp_delete_file($path); + wp_delete_file($path); wp_send_json_success( [ 'redirect_url' => wu_network_admin_url('wp-ultimo-sites', ['deleted' => 1]), ] );If you need to verify deletion succeeded:
- $success = wp_delete_file($path); + wp_delete_file($path); + + if (file_exists($path)) { + wp_send_json_error(new \WP_Error('delete-failed', __('Failed to delete export file.', 'ultimate-multisite'))); + } wp_send_json_success(inc/functions/importer.php-288-300 (1)
288-300: Missing_deprecated_functioncall in backward-compatibility alias.Unlike the other deprecated aliases (lines 205-286),
wp_ultimo_site_exporter_maybe_get_site_by_pathdoes not call_deprecated_function()to notify developers of its deprecation.🔎 Proposed fix
function wp_ultimo_site_exporter_maybe_get_site_by_path(string $domain, string $path) { + _deprecated_function(__FUNCTION__, '2.5.0', 'wu_exporter_maybe_get_site_by_path'); + return wu_exporter_maybe_get_site_by_path($domain, $path); }inc/site-exporter/mu-migration/features/bootstrap/utils.php-680-703 (1)
680-703: Unused variable$patchinget_named_sem_ver.The variable
$patchis assigned on line 693 but never used in the function logic. This appears to be leftover code.🔎 Proposed fix
$parts = explode('-', $original_version); $bits = explode('.', $parts[0]); $major = $bits[0]; if ( isset($bits[1]) ) { $minor = $bits[1]; } - if ( isset($bits[2]) ) { - $patch = $bits[2]; - }inc/functions/importer.php-139-148 (1)
139-148: Potential type mismatch: function may returnfalsebut declaresstringreturn type.
get_attached_file()returnsfalsewhen the attachment is not found. This conflicts with the declared: stringreturn type and could cause a TypeError.🔎 Proposed fix
-function wu_exporter_url_to_path(string $url): string { +function wu_exporter_url_to_path(string $url): string|false { $path = str_replace(set_url_scheme(site_url('/'), 'https'), ABSPATH, set_url_scheme($url, 'https')); if (file_exists($path)) { return $path; } - return get_attached_file(attachment_url_to_postid($url)); + return get_attached_file(attachment_url_to_postid($url)) ?: ''; }inc/site-exporter/mu-migration/features/tests/csv_matches_user.php-74-74 (1)
74-74: Fix typo in error message.Line 74 contains a typo: "Faiure" should be "Failure".
🔎 Proposed fix
- echo 'Faiure: ' . $e->getMessage(); + echo 'Failure: ' . $e->getMessage();inc/site-exporter/mu-migration/features/bootstrap/support.php-209-218 (1)
209-218: Debug statement and resource leak.Line 211 contains a
var_dump($regex)debug statement that should be removed. Additionally,opendir()returns a resource that should be closed withclosedir(), and there's no error handling ifopendir()fails.🔎 Proposed fix
function file_exists_regex($regex, $dir = './') { $open = opendir($dir); - var_dump($regex); + if ( false === $open ) { + return false; + } while ( ($file = readdir($open)) !== false ) { if ( preg_match("/$regex/", $file) ) { + closedir($open); return true; } } + closedir($open); return false; }inc/site-exporter/mu-migration/includes/commands/class-mu-migration-posts.php-100-106 (1)
100-106: Missing text domain.Line 102 uses
__('#%d New user ID equals to the old user ID')without the'mu-migration'text domain, breaking translation consistency with other strings in the file.🔎 Proposed fix
- __('#%d New user ID equals to the old user ID'), + __('#%d New user ID equals to the old user ID', 'mu-migration'),inc/site-exporter/mu-migration/includes/commands/class-mu-migration-users.php-105-122 (1)
105-122: Inconsistent text domain usage.Several
__()calls in this method are missing the'mu-migration'text domain, while other strings in this file use it consistently. This breaks translation consistency.Lines 105, 107, 108, 109, 122 use
__()without a text domain.🔎 Proposed fix
- $message = __('A password reset has been requested for the following account:') . "\r\n\r\n"; + $message = __('A password reset has been requested for the following account:', 'mu-migration') . "\r\n\r\n"; $message .= network_home_url('/') . "\r\n\r\n"; - $message .= sprintf(__('Username: %s'), $user_login) . "\r\n\r\n"; - $message .= __('In order to log in again you have to reset your password.') . "\r\n\r\n"; - $message .= __('To reset your password, visit the following address:') . "\r\n\r\n"; + $message .= sprintf(__('Username: %s', 'mu-migration'), $user_login) . "\r\n\r\n"; + $message .= __('In order to log in again you have to reset your password.', 'mu-migration') . "\r\n\r\n"; + $message .= __('To reset your password, visit the following address:', 'mu-migration') . "\r\n\r\n"; $message .= '<' . network_site_url("wp-login.php?action=rp&key=$key&login=" . rawurlencode($user_login), 'login') . ">\r\n"; - $title = sprintf(__('[%s] Password Reset'), $blogname); + $title = sprintf(__('[%s] Password Reset', 'mu-migration'), $blogname);inc/site-exporter/mu-migration/includes/commands/class-mu-migration-import.php-653-676 (1)
653-676: Missing WP_Error check on wp_insert_site().The
wp_insert_site()function can return aWP_Errorobject on failure, but only a falsy check is performed. This could mask the actual error.🔎 Proposed fix
$blog_id = wp_insert_site( [ 'domain' => $parsed_url['host'], 'path' => $parsed_url['path'], 'network_id' => $site_id, ] ); -if ( ! $blog_id ) { +if ( ! $blog_id || is_wp_error($blog_id) ) { return false; } return $blog_id;inc/site-exporter/database/class-import.php-205-229 (1)
205-229: Missing fopen() failure check and potential resource leak.If
fopen()fails,$fpwill befalse, causing the while loop to fail. Additionally, if an exception occurs during processing, the file handle may not be closed.🔎 Proposed fix
// Read in entire file $fp = fopen($this->filename, 'r'); +if ($fp === false) { + throw new Exception("Error: Unable to open file.\n"); +} + // Temporary variable, used to store current query $templine = ''; +try { // Loop through each line while (($line = fgets($fp)) !== false) { // Skip it if it's a comment if (substr($line, 0, 2) === '--' || $line === '') { continue; } // Add this line to the current segment $templine .= $line; // If it has a semicolon at the end, it's the end of the query if (substr(trim($line), -1, 1) === ';') { $this->query($templine); // Reset temp variable to empty $templine = ''; } } +} finally { + // Close the file + fclose($fp); +} - -// Close the file -fclose($fp);inc/site-exporter/database/class-manager.php-165-179 (1)
165-179: Table names in DESCRIBE and SHOW CREATE TABLE are unvalidated.Similar to other methods, table names are directly interpolated. Consider validating against known tables or using identifier escaping.
inc/site-exporter/mu-migration/features/bootstrap/FeatureContext.php-181-182 (1)
181-182: Duplicate line assignment.Line 181 and 182 contain identical assignments to
$wp_version_suffix.🔎 Proposed fix
// Remove install cache if any (not setting the static var). $wp_version_suffix = ($wp_version = getenv('WP_VERSION')) ? "-$wp_version" : ''; - $wp_version_suffix = ($wp_version = getenv('WP_VERSION')) ? "-$wp_version" : ''; $install_cache_dir = sys_get_temp_dir() . '/wp-cli-test-core-install-cache' . $wp_version_suffix;inc/site-exporter/database/class-replace.php-188-194 (1)
188-194: CSV parsing lacks validation for malformed rows.When parsing CSV data,
array_combinewill fail or produce unexpected results if a CSV line doesn't have exactly 2 columns. This could cause silent data corruption or warnings.🔎 Proposed fix to validate CSV row structure
// Prepare CSV data if (null !== $csv) { $csv_lines = explode("\n", $csv); $csv_head = str_getcsv('search,replace'); foreach ($csv_lines as $line) { - $this->csv_data[] = array_combine($csv_head, str_getcsv($line)); + $parsed = str_getcsv($line); + if (count($parsed) === 2) { + $this->csv_data[] = array_combine($csv_head, $parsed); + } } }inc/site-exporter/mu-migration/features/bootstrap/FeatureContext.php-258-280 (1)
258-280: Shell command injection risk in terminate_proc.The
$master_pidis passed directly into a backtick shell command without validation. While this is test code and PIDs come fromproc_get_status, adding validation would be safer.🔎 Proposed fix
private static function terminate_proc($master_pid) { + $master_pid = (int) $master_pid; + if ($master_pid <= 0) { + return; + } - $output = `ps -o ppid,pid,command | grep $master_pid`; + $output = shell_exec('ps -o ppid,pid,command | grep ' . escapeshellarg($master_pid));
🧹 Nitpick comments (40)
inc/helpers/validation-rules/class-unique-customer-email.php (1)
35-55: Consider initializing the translated message only once.The
$messageproperty is initialized on line 35 with a hardcoded English string, then overwritten with a translated version insidecheck()on line 55. Consider using a lazy initialization pattern or moving the translation to where the message is actually retrieved, to avoid redundant work on every validation call.🔎 Proposed refactor
- protected $message = 'A customer with the same email address already exists.'; + protected $message = ''; public function check($value): bool { if (empty($value)) { return true; } - // Set a translated error message - $this->message = __('A customer with the same email address already exists.', 'ultimate-multisite'); + if (empty($this->message)) { + $this->message = __('A customer with the same email address already exists.', 'ultimate-multisite'); + }inc/compat/class-multiple-accounts-compat.php (1)
128-130: Add inline comment explaining why main site is exempted from user query scoping.The guard clause logic is correct—is_main_site() determines whether a site is the main site of the current network, and returning unfiltered for the main site is the right approach in a multisite context. However, add a brief comment explaining why:
$site_id = get_current_blog_id(); + // Skip transformation for main site to preserve network-wide user access if (is_main_site($site_id)) { return $query; }phpstan.neon.dist (1)
13-14: Consider analyzing mu-migration code if modifications are made.Excluding the mu-migration directory from PHPStan analysis is reasonable if this is unmodified third-party code. However, if any modifications or integrations are made to this code, consider running PHPStan on it to catch potential type safety issues at the integration boundaries.
.phpcs.xml.dist (1)
11-11: Verify mu-migration code quality if modifications are planned.Excluding the mu-migration directory from PHPCS is reasonable for third-party code. However, if any modifications are made to this code, consider running PHPCS on those changes to maintain code quality standards.
inc/site-exporter/mu-migration/bin/install-package-tests.sh (1)
6-7: Consider using environment variables for database credentials.While this is a test script, hardcoded credentials (
password1) and passwordless root access are security smells. Consider using environment variables for the password and root credentials to align with security best practices, even in test environments.🔎 Suggested approach
install_db() { - mysql -e 'CREATE DATABASE IF NOT EXISTS wp_cli_test;' -uroot - mysql -e 'GRANT ALL PRIVILEGES ON wp_cli_test.* TO "wp_cli_test"@"localhost" IDENTIFIED BY "password1"' -uroot + mysql -e 'CREATE DATABASE IF NOT EXISTS wp_cli_test;' -u"${DB_ROOT_USER:-root}" -p"${DB_ROOT_PASS:-}" + mysql -e 'GRANT ALL PRIVILEGES ON wp_cli_test.* TO "wp_cli_test"@"localhost" IDENTIFIED BY "'"${DB_TEST_PASS:-password1}"'"' -u"${DB_ROOT_USER:-root}" -p"${DB_ROOT_PASS:-}" }inc/site-exporter/mu-migration/composer.json (1)
23-23: Consider updating the Behat version.The development dependency
behat/behat~2.5 is significantly outdated (current major version is 3.x). While this may not be critical for a dev dependency, using an outdated testing framework could limit access to bug fixes and modern PHP compatibility.inc/site-exporter/mu-migration/includes/helpers.php (1)
372-388: Unused$importvariable (static analysis finding).The
Importclass constructor performs the import as a side effect, so the result isn't needed. Consider either suppressing the warning or usingnewwithout assignment for clarity.🔎 Proposed fix
- $import = new \WP_Ultimo\Site_Exporter\Database\Import( + new \WP_Ultimo\Site_Exporter\Database\Import( $file_name, DB_USER, DB_PASSWORD, DB_NAME, DB_HOST, true, // drop_tables true, // force_drop_tables $wu_site_exporter_site_id );inc/functions/importer.php (1)
84-111: Direct database queries with proper escaping.The SQL queries use proper LIKE pattern escaping with backslashes. However, consider using
$wpdb->prepare()with$wpdb->esc_like()for additional safety and clarity.🔎 Optional refactor using prepare()
function wu_exporter_get_pending_imports(): array { global $wpdb; + $like_pattern = $wpdb->esc_like('_site_transient_wu_pending_site_import_') . '%'; + if (is_multisite()) { $table = "{$wpdb->base_prefix}sitemeta"; - $query = "SELECT meta_key, meta_value as options FROM {$table} WHERE meta_key LIKE '\\_site\\_transient\\_wu\\_pending\\_site\\_import\\_%'"; + $query = $wpdb->prepare("SELECT meta_key, meta_value as options FROM {$table} WHERE meta_key LIKE %s", $like_pattern); } else { $table = "{$wpdb->base_prefix}options"; - $query = "SELECT option_name, option_value as options FROM {$table} WHERE option_name LIKE '\\_transient\\_wu\\_pending\\_site\\_import\\_%'"; + $query = $wpdb->prepare("SELECT option_name, option_value as options FROM {$table} WHERE option_name LIKE %s", $like_pattern); } - $results = $wpdb->get_results($query); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery + $results = $wpdb->get_results($query); // phpcs:ignore WordPress.DB.DirectDatabaseQueryinc/site-exporter/mu-migration/features/bootstrap/utils.php (1)
1-13: Vendored WP-CLI utilities file.This appears to be a forked/vendored copy of WP-CLI utility functions. Consider adding a header comment indicating the source, license, and any modifications made for maintainability.
inc/site-exporter/class-site-exporter.php (1)
569-573: Direct$_POSTaccess without sanitization.While the values are used as booleans (empty check), it's better practice to use
wu_request()consistently or apply sanitization.🔎 Proposed fix for consistency
$options = [ - 'themes' => ! empty($_POST['include_themes']), - 'plugins' => ! empty($_POST['include_plugins']), - 'uploads' => ! empty($_POST['include_uploads']), + 'themes' => (bool) wu_request('include_themes', false), + 'plugins' => (bool) wu_request('include_plugins', false), + 'uploads' => (bool) wu_request('include_uploads', false), ]; - $background = ! empty($_POST['background_run']); + $background = (bool) wu_request('background_run', false);inc/site-exporter/mu-migration/README.md (1)
38-125: Consider optional documentation polish.The README is comprehensive and informative. To enhance accessibility and consistency, consider:
- Adding language specifiers to code blocks (e.g.,
bash instead of)- Adding periods after "etc." abbreviations (lines 61, 112)
- Adding alt text to the banner image (line 125) for screen reader accessibility
These are style improvements that don't affect functionality but improve the documentation quality.
inc/site-exporter/mu-migration/features/tests/csv_matches_user.php (1)
3-6: Add WordPress availability check.The script uses WordPress functions (
get_userdata,get_user_meta, etc.) without verifying that WordPress is loaded. This could lead to fatal errors if the script is executed outside the WordPress context.🔎 Proposed fix
<?php if ( ! isset($args) ) { echo 'No Arguments'; return; } + +if ( ! function_exists('get_userdata') ) { + echo 'WordPress functions not available'; + return; +}inc/class-wp-ultimo.php (1)
1028-1056: Consider increasing transient TTL for deactivation notices.The transient TTL is set to 60 seconds (lines 1041, 1054). If the admin page takes longer than 60 seconds to load after deactivation (due to slow hosting, other plugins, etc.), users might miss the notice. Consider increasing this to 5 minutes (300 seconds) for better reliability.
🔎 Proposed adjustment
if (isset($network_plugins[ $addon_file ])) { unset($network_plugins[ $addon_file ]); update_site_option('active_sitewide_plugins', $network_plugins); // Set a transient to show a notice after redirect - set_site_transient('wu_site_exporter_addon_deactivated', true, 60); + set_site_transient('wu_site_exporter_addon_deactivated', true, 300); } } // Check if the addon is activated on the current site $active_plugins = get_option('active_plugins', []); $key = array_search($addon_file, $active_plugins, true); if (false !== $key) { unset($active_plugins[ $key ]); update_option('active_plugins', array_values($active_plugins)); // Set a transient to show a notice after redirect - set_transient('wu_site_exporter_addon_deactivated', true, 60); + set_transient('wu_site_exporter_addon_deactivated', true, 300); }views/template-library/template-library.php (1)
178-178: Consider extracting inline styles to CSS classes.Lines 178 and 212 contain inline
styleattributes within Vue templates. While functional, extracting these to Tailwind utility classes or a dedicated CSS file would improve maintainability.Also applies to: 212-212
inc/site-exporter/mu-migration/features/bootstrap/ProcessRun.php (2)
47-51: Add type validation to constructor.The constructor accepts any
$propsarray without validating its structure or types. Consider adding basic validation to ensure required properties exist and have expected types.🔎 Example validation
public function __construct($props) { + if (!is_array($props)) { + throw new \InvalidArgumentException('Props must be an array'); + } foreach ( $props as $key => $value ) { $this->$key = $value; } }
58-66: Handle null values in __toString().Lines 61-63 concatenate properties that could be null, resulting in output like "cwd: null". Consider adding null checks or default values for cleaner output.
🔎 Proposed fix
public function __toString() { $out = "$ $this->command\n"; $out .= "$this->stdout\n$this->stderr"; - $out .= "cwd: $this->cwd\n"; + $out .= "cwd: " . ($this->cwd ?? '(not set)') . "\n"; $out .= "run time: $this->run_time\n"; $out .= "exit status: $this->return_code"; return $out; }inc/site-exporter/mu-migration/features/steps/then.php (1)
167-167: Use consistent exception types.Lines 167 and 178 throw generic
Exceptioninstead ofRuntimeException, which is used elsewhere (e.g., line 10). Consider usingRuntimeExceptionconsistently for runtime assertion failures.🔎 Proposed fix
- if ( '' === rtrim($world->result->$stream, "\n") ) { - throw new Exception($world->result); - } + if ( '' === rtrim($world->result->$stream, "\n") ) { + throw new RuntimeException($world->result); + } // ... and on line 178: - if ( false === version_compare(trim($world->result->$stream, "\n"), $goal_ver, $operator) ) { - throw new Exception($world->result); - } + if ( false === version_compare(trim($world->result->$stream, "\n"), $goal_ver, $operator) ) { + throw new RuntimeException($world->result); + }Also applies to: 178-178
inc/site-exporter/mu-migration/utils/behat-tags.php (2)
20-20: Add error handling for exec() failures.Lines 20 and 56 use
exec()without checking the return status. If grep fails (e.g., due to missing files or permissions issues), the script continues silently with empty arrays, which could lead to unexpected test behavior.🔎 Proposed fix
- exec("grep '@{$prefix}-[0-9\.]*' -h -o features/*.feature | uniq", $existing_tags); + exec("grep '@{$prefix}-[0-9\.]*' -h -o features/*.feature | uniq", $existing_tags, $return_code); + if ($return_code > 1) { // grep returns 1 if no matches, >1 for errors + error_log("Warning: Failed to grep for version tags with prefix '{$prefix}'"); + }Apply similar logic to line 56.
Also applies to: 56-56
20-20: Hardcoded path assumes specific directory structure.Lines 20 and 56 hardcode
features/*.featurewhich assumes the script is executed from a specific directory. If the script is called from a different location, the grep commands will fail silently.Consider using
__DIR__to make paths relative to the script location:🔎 Proposed fix
- exec("grep '@{$prefix}-[0-9\.]*' -h -o features/*.feature | uniq", $existing_tags); + $features_dir = __DIR__ . '/../features'; + exec("grep '@{$prefix}-[0-9\.]*' -h -o {$features_dir}/*.feature | uniq", $existing_tags);Also applies to: 56-56
inc/site-exporter/mu-migration/features/bootstrap/Process.php (1)
27-31: STDIN constant usage assumes CLI context.The static
$descriptorsarray uses theSTDINconstant directly. This constant is only defined in CLI SAPI. If this code is ever loaded outside a CLI context (e.g., during autoload discovery), it could cause a fatal error.Since this is test infrastructure for Behat/WP-CLI, this is likely acceptable, but consider defensive initialization in
create()if broader compatibility is needed.assets/js/template-library.js (2)
108-131: Missing CSRF nonce in AJAX request.The
fetch_templates_listAJAX call does not include a WordPress nonce for CSRF protection. While this appears to be a read-only endpoint, WordPress best practices recommend including nonce verification for all AJAX actions.🔎 Proposed fix
$.ajax({ method: 'GET', url: ajaxurl, data: { action: 'serve_templates_list', + _wpnonce: wu_template_library.nonce, },Ensure the nonce is localized in PHP:
wp_localize_script('template-library', 'wu_template_library', [ 'nonce' => wp_create_nonce('serve_templates_list'), // ... other data ]);
137-147: Fragile mount point selector.Using a class selector (
.wp-heading-inline) for Vue'seloption is fragile. If multiple elements match or the class is removed/renamed, this Vue instance may silently fail or mount on the wrong element. Consider using an ID selector for deterministic mounting.inc/site-exporter/mu-migration/includes/commands/class-mu-migration-users.php (1)
151-156: Email failure return value inconsistency.The method logs a failure message when
wp_mailreturns false, but still returnstrue. This could mislead callers into thinking the email was sent successfully. Consider returningfalseor aWP_Erroron mail failure.🔎 Proposed fix
if ( $message && ! wp_mail($user_email, wp_specialchars_decode($title), $message) ) { WP_CLI::log(__('The email could not be sent', 'mu-migration')); + return false; } return true;inc/site-exporter/mu-migration/includes/commands/class-mu-migration-posts.php (2)
121-127: Redundant computation inside loop.The
uid_fieldsexplode and WooCommerce check are executed on every iteration of the posts loop. These values don't change per-post and should be computed once before the loop.🔎 Proposed fix
Move these lines before the
all_recordscall (around line 70):+ // Parse uid_fields once before iteration + $uid_fields = explode(',', $this->assoc_args['uid_fields']); + // Automatically add Woocommerce user id field + if ( $is_woocommerce ) { + $uid_fields[] = '_customer_user'; + } + $uid_fields = array_filter($uid_fields); + $this->all_records( __('Updating posts authors', 'mu-migration'), $wpdb->posts, - function ($result) use (&$equals_id, &$author_not_found, $ids_map, $verbose, $is_woocommerce) { + function ($result) use (&$equals_id, &$author_not_found, $ids_map, $verbose, $uid_fields) { // ... existing author update code ... - // Parse uid_fields - $uid_fields = explode(',', $this->assoc_args['uid_fields']); - // Automatically add Woocommerce user id field - if ( $is_woocommerce ) { - $uid_fields[] = '_customer_user'; - } // Iterate over fields and update them. - foreach ( array_filter($uid_fields) as $f ) { + foreach ( $uid_fields as $f ) {
62-68: Missing file read error handling.
file_get_contents($filename)can returnfalseon failure, which would causejson_decode(false)to returnnull. While the null check on line 64 catches this, the error message "An error has occurred when parsing the json file" is misleading when the actual issue is a file read failure.🔎 Proposed fix
- $ids_map = json_decode(file_get_contents($filename)); + $file_contents = file_get_contents($filename); + + if ( false === $file_contents ) { + WP_CLI::error(__('Failed to read the input file', 'mu-migration')); + } + + $ids_map = json_decode($file_contents); if ( null === $ids_map ) {inc/site-exporter/mu-migration/features/bootstrap/support.php (1)
81-92: Missing property/key existence checks.When comparing objects (line 83) and arrays (line 89), the code accesses
$actual->$nameand$actual[$key]without first verifying the property/key exists. This will produce PHP warnings or errors if the expected structure has fields that actual lacks.🔎 Proposed fix
if ( is_object($expected) ) { foreach ( get_object_vars($expected) as $name => $value ) { + if ( ! property_exists($actual, $name) ) { + return false; + } if ( ! compareContents($value, $actual->$name) ) { return false; } } } elseif ( is_array($expected) ) { foreach ( $expected as $key => $value ) { + if ( ! array_key_exists($key, $actual) ) { + return false; + } if ( ! compareContents($value, $actual[ $key ]) ) { return false; } }inc/site-exporter/mu-migration/features/03-export.feature (1)
40-40: Minor: Double space in command.There's a double space in
cat users-subsite.csv. This works but is inconsistent with similar commands elsewhere in the file.inc/site-exporter/database/class-import.php (1)
168-175: Variable shadowing:$tablereused in loop.The variable
$tableis first assigned on line 168 as the table prefix, then reused in the foreach loop on line 175. This shadows the original value and could cause confusion.🔎 Proposed fix
- $table = is_multisite() ? $wpdb->base_prefix . $site_id : $wpdb->base_prefix; + $table_prefix = is_multisite() ? $wpdb->base_prefix . $site_id : $wpdb->base_prefix; // Get list of tables - $tables = $this->query("SHOW TABLES LIKE '" . $table . "%'"); + $tables = $this->query("SHOW TABLES LIKE '" . $table_prefix . "%'"); if ($tables !== null && $tables !== false) { // Loop through tables $results = $tables->fetchAll(PDO::FETCH_COLUMN); foreach ($results as $table) {inc/template-library/class-api-client.php (1)
191-213: Inefficient single template lookup fetches all templates.
get_template()fetches all templates viaget_templates()just to find one by slug. If the API supports filtering by slug, this would be more efficient.Consider adding a direct API endpoint call if the WooCommerce Store API supports fetching a single product by slug:
public function get_template(string $slug) { + // Try direct lookup first if API supports it + $result = $this->execute_request('products', ['slug' => $slug]); + + if (!is_wp_error($result) && !empty($result)) { + return $this->parse_template_data($result[0]); + } + + // Fallback to fetching all templates $templates = $this->get_templates();inc/site-exporter/mu-migration/includes/commands/class-mu-migration-export.php (1)
452-452: Use ofrand()for uniqueness is weak.
rand()without seeding provides weak randomness. For file naming to avoid collisions, consider usingwp_generate_uuid4()oruniqid('', true).🔎 Proposed fix
- $rand = rand(); + $rand = uniqid('', true);inc/site-exporter/mu-migration/includes/commands/class-mu-migration-import.php (2)
579-579: Unused loop variable$plugin.The
$pluginvariable in the foreach loop is never used. Consider using$_or removing the value assignment.🔎 Proposed fix
- foreach ( $plugins as $plugin_name => $plugin ) { + foreach ( $plugins as $plugin_name => $_ ) {Or simply:
- foreach ( $plugins as $plugin_name => $plugin ) { + foreach ( array_keys($plugins) as $plugin_name ) {
210-212: Use wp_json_encode() for consistency with WordPress coding standards.The code uses
json_encode()directly. For consistency with WordPress APIs and proper escaping, usewp_json_encode().🔎 Proposed fix
if ( ! empty($ids_maps) ) { // Saving the ids_maps to a file. $output_file_handler = fopen($this->assoc_args['map_file'], 'w+'); - fwrite($output_file_handler, json_encode($ids_maps)); + fwrite($output_file_handler, wp_json_encode($ids_maps)); fclose($output_file_handler);inc/site-exporter/database/class-replace.php (1)
305-320: Consider using$wpdb->_real_escape()for consistency.While
mysql_escape_mimicworks, using WordPress's built-in escaping through the database manager would be more consistent with the codebase and handle edge cases like charset-specific escaping.inc/site-exporter/database/class-manager.php (2)
97-102: Table name should be validated or whitelisted.While
esc_sqlis applied, table names should ideally be validated against known tables fromget_tables()to prevent potential injection through table name manipulation.
151-157: The update method relies on pre-escaped values from Replace class.This is acceptable given the tight coupling with
Replace::replace_values(), but adding a docblock note about the expectation that$update_sqland$where_sqlare pre-escaped would improve maintainability.inc/template-library/class-template-repository.php (3)
70-74: LGTM - Clean constructor with dependency instantiation.The repository correctly initializes its dependencies. Consider accepting these as constructor parameters for better testability.
191-202: Consider usingarray_valuesto reindex after filtering.The
array_filterpreserves array keys, which could cause issues if the consuming code expects a zero-indexed array.🔎 Proposed fix
- return array_filter( + return array_values(array_filter( $templates, function ($template) use ($category) { foreach ($template['categories'] as $cat) { if ($cat['slug'] === $category) { return true; } } return false; } - ); + )); }
221-254: Search implementation is correct but could benefit fromarray_values.Same reindexing consideration as
get_templates_by_category. Otherwise, the multi-field search across name, description, slug, industry_type, and categories is well-implemented.inc/site-exporter/mu-migration/features/bootstrap/FeatureContext.php (2)
77-83: Hardcoded database credentials in test code.While this is test infrastructure with overridable env vars, consider adding a comment clarifying these are test-only defaults.
533-535: Arbitrary sleep(1) could cause flaky tests.The hardcoded 1-second sleep may not be sufficient on slower systems or excessive on fast ones. Consider a polling approach instead.
| // Convert URL to file path | ||
| $upload_dir = wp_upload_dir(); | ||
| $file_path = str_replace($upload_dir['baseurl'], $upload_dir['basedir'], $zip_url); | ||
|
|
||
| if (! file_exists($file_path)) { | ||
| wp_send_json_error(new \WP_Error('file-not-found', __('ZIP file not found.', 'ultimate-multisite'))); | ||
| } |
There was a problem hiding this comment.
Path traversal vulnerability in ZIP file handling.
The str_replace to convert URL to file path could be exploited if $zip_url contains path traversal sequences or points to a file outside the uploads directory. An attacker could potentially read arbitrary files.
🔎 Proposed fix to validate the file path
// Convert URL to file path
$upload_dir = wp_upload_dir();
$file_path = str_replace($upload_dir['baseurl'], $upload_dir['basedir'], $zip_url);
+ // Ensure the file is within the uploads directory
+ $real_path = realpath($file_path);
+ $real_upload_dir = realpath($upload_dir['basedir']);
+
+ if (false === $real_path || 0 !== strpos($real_path, $real_upload_dir . DIRECTORY_SEPARATOR)) {
+ wp_send_json_error(new \WP_Error('invalid-path', __('Invalid file path.', 'ultimate-multisite')));
+ }
+
if (! file_exists($file_path)) {
wp_send_json_error(new \WP_Error('file-not-found', __('ZIP file not found.', 'ultimate-multisite')));
}🤖 Prompt for AI Agents
In inc/admin-pages/class-template-library-admin-page.php around lines 268-274,
the current str_replace conversion of $zip_url to a filesystem path is
vulnerable to path traversal and can resolve outside the uploads directory;
replace this logic by resolving and validating the real filesystem path and
ensuring it is contained within WP_UPLOAD_DIR: normalize and resolve the uploads
base directory (wp_normalize_path and realpath on $upload_dir['basedir']),
resolve the candidate file path via realpath after converting the URL->path
safely (avoid trusting str_replace on the raw URL), then check the resolved file
path starts with the normalized uploads basedir; if realpath fails or the
resolved path is not inside the uploads dir, return the WP_Error file-not-found
response; also verify the file is readable and has a .zip mime/type before
proceeding.
| wp_send_json_success( | ||
| [ | ||
| 'redirect_url' => wu_network_admin_url('wp-ultimo'), | ||
| 'redirect_url' => wu_network_admin_url('ultimate-multisite'), | ||
| ] | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the target file and examine it
echo "=== Examining inc/debug/class-debug.php lines around redirects ==="
sed -n '68,78p; 380,395p; 467,480p; 535,545p' inc/debug/class-debug.php | cat -n
# Search for admin page registrations in the codebase
echo -e "\n=== Searching for admin page registrations ==="
rg -nP "(add_menu_page|add_submenu_page|add_admin_page|register_admin_page)" --type=php -A 5 | head -100Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 5843
🏁 Script executed:
#!/bin/bash
# Find all admin page class files
echo "=== Admin page files in inc/admin-pages ==="
find inc/admin-pages -name "*.php" -type f | head -20
echo -e "\n=== Search for 'ultimate-multisite' page ID registration ==="
rg -n "ultimate-multisite|wp-ultimo-setup|wp-ultimo-debug-pages" --type=php inc/admin-pages/ | head -50
echo -e "\n=== Check the main admin pages registration ==="
rg -n "class.*Admin_Page" --type=php inc/admin-pages/ | grep -E "(setup|dashboard|debug|settings)" | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 8646
🏁 Script executed:
#!/bin/bash
# Check the Dashboard_Admin_Page class for its page ID
echo "=== Dashboard_Admin_Page class definition ==="
head -50 inc/admin-pages/class-dashboard-admin-page.php | cat -n
echo -e "\n=== Check the id property in Dashboard_Admin_Page ==="
rg -n "public \$id|protected \$id|private \$id|\$this->id =" inc/admin-pages/class-dashboard-admin-page.php
echo -e "\n=== Check Setup_Wizard_Admin_Page ==="
head -50 inc/admin-pages/class-setup-wizard-admin-page.php | cat -n
echo -e "\n=== Check Debug_Admin_Page ==="
head -100 inc/admin-pages/debug/class-debug-admin-page.php | cat -nRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 5748
🏁 Script executed:
#!/bin/bash
# Search for any admin page with 'ultimate-multisite' as the ID
echo "=== Search for 'ultimate-multisite' as page ID in admin pages ==="
rg -n "protected \\\$id.*ultimate-multisite|public \\\$id.*ultimate-multisite" --type=php inc/admin-pages/
echo -e "\n=== Search all files for 'ultimate-multisite' page ID ==="
rg -n "\\\$id = ['\"]ultimate-multisite['\"]" --type=php
echo -e "\n=== Check if Dashboard has alternative ID ==="
rg -n "ultimate-multisite" inc/admin-pages/class-dashboard-admin-page.php | head -5
echo -e "\n=== List all Admin Page ID definitions ==="
rg -n "protected \\\$id = " --type=php inc/admin-pages/ | head -30Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 3537
🏁 Script executed:
#!/bin/bash
# Check if Dashboard page might have registered with 'ultimate-multisite' somewhere else
echo "=== Check full Dashboard class for any alternate ID references ==="
rg -n "ultimate-multisite|wp-ultimo" inc/admin-pages/class-dashboard-admin-page.php | head -30
# Check what the actual dashboard page ID should be
echo -e "\n=== Check Dashboard menu label ==="
sed -n '540,560p' inc/admin-pages/class-dashboard-admin-page.php | cat -n
# Search for where pages are instantiated/registered
echo -e "\n=== Search for admin page instantiation ==="
rg -n "new.*Admin_Page|Dashboard_Admin_Page" --type=php inc/ | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 5461
🏁 Script executed:
#!/bin/bash
# Final verification: search entire codebase for 'ultimate-multisite' as a page slug/ID
echo "=== Final search for 'ultimate-multisite' as admin page ID or slug ==="
rg -n "wu_network_admin_url\(['\"]ultimate-multisite" --type=php
echo -e "\n=== Check if this is referenced elsewhere ==="
rg -n "'ultimate-multisite'" --type=php inc/debug/class-debug.php
echo -e "\n=== Confirm 'wp-ultimo' is the dashboard ID ==="
rg -n "wu_network_admin_url\(['\"]wp-ultimo['\"]" --type=php | head -10
echo -e "\n=== Check what wu_network_admin_url does with the page ID ==="
rg -n "function wu_network_admin_url" --type=php -A 10Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 4043
🏁 Script executed:
#!/bin/bash
# Check if 'ultimate-multisite' might be aliased or if there's a filter that maps it
echo "=== Search for any page ID mapping or aliasing of 'ultimate-multisite' ==="
rg -n "ultimate-multisite.*wp-ultimo|wp-ultimo.*ultimate-multisite" --type=php
echo -e "\n=== Check the host-integrations ready.php context ==="
sed -n '20,40p' views/wizards/host-integrations/ready.php | cat -n
echo -e "\n=== Check the tax dashboard tab context ==="
sed -n '95,115p' inc/tax/class-dashboard-taxes-tab.php | cat -n
echo -e "\n=== Summary: All uses of 'ultimate-multisite' page ID ==="
rg -B 3 "wu_network_admin_url\(['\"]ultimate-multisite['\"]" --type=phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 12887
Redirect page ID 'ultimate-multisite' is not registered—use 'wp-ultimo' instead.
Line 388 redirects to a non-existent admin page. 'ultimate-multisite' is not a registered admin page ID. The Dashboard admin page is registered with ID 'wp-ultimo' (see Dashboard_Admin_Page class). Change line 388 to:
'redirect_url' => wu_network_admin_url('wp-ultimo'),Also check the redirect in views/wizards/host-integrations/ready.php:29 and inc/tax/class-dashboard-taxes-tab.php:106, which have the same issue.
🤖 Prompt for AI Agents
In inc/debug/class-debug.php around lines 386–390 the AJAX success redirect uses
the unregistered page ID 'ultimate-multisite'; change the redirect to use the
registered Dashboard page ID by replacing the value with 'wp-ultimo' (i.e. call
wu_network_admin_url('wp-ultimo') for the redirect_url). Also search and update
the same incorrect page ID in views/wizards/host-integrations/ready.php (line
~29) and inc/tax/class-dashboard-taxes-tab.php (line ~106) to use 'wp-ultimo' so
all redirects point to the registered admin page.
| public function add_overview_panel($output) { | ||
|
|
||
| $collector = \QM_Collectors::get('wp-ultimo'); | ||
| $collector = \QM_Collectors::get('ultimate-multisite'); |
There was a problem hiding this comment.
Fix collector key mismatch.
The collector is retrieved using the key 'ultimate-multisite' (line 341), but it's registered with the key 'wp-ultimo' in the register_collector_overview method (line 326). This mismatch will cause the collector retrieval to fail.
Ensure both the registration and retrieval use the same key.
🔎 Proposed fix
Update the registration key to match:
public function register_collector_overview(array $collectors, \QueryMonitor $qm) { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter
- $collectors['wp-ultimo'] = new Query_Monitor\Collectors\Collector_Overview();
+ $collectors['ultimate-multisite'] = new Query_Monitor\Collectors\Collector_Overview();
return $collectors;
}Or update the output array key as well for consistency:
public function add_overview_panel($output) {
$collector = \QM_Collectors::get('ultimate-multisite');
- $output['wp-ultimo'] = new Query_Monitor\Panel\Overview($collector);
+ $output['ultimate-multisite'] = new Query_Monitor\Panel\Overview($collector);
return $output;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In inc/development/class-toolkit.php around lines 326 and 341, the collector is
registered under the key 'wp-ultimo' but later retrieved using
'ultimate-multisite', causing retrieval to fail; fix by making the keys
identical — either change the register_collector_overview registration key to
'ultimate-multisite' or change the retrieval at line 341 to 'wp-ultimo' (and
update any related output array keys to match) so registration and retrieval use
the exact same collector key.
| function wu_exporter_get_pending(): array { | ||
|
|
||
| global $wpdb; | ||
|
|
||
| $table = is_multisite() ? "{$wpdb->base_prefix}sitemeta" : "{$wpdb->base_prefix}options"; | ||
|
|
||
| $like = is_multisite() ? '\\_site\\_transient\\_wu\\_pending\\_site\\_export\\_%' : '\\_transient\\_wu\\_pending\\_site\\_export\\_%'; | ||
|
|
||
| $query = "SELECT meta_key, meta_value as site_id FROM {$table} WHERE meta_key LIKE '{$like}'"; | ||
|
|
||
| return $wpdb->get_results($query); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery | ||
| } |
There was a problem hiding this comment.
SQL column names incorrect for single-site installations.
The query uses meta_key and meta_value column names, but the wp_options table (used for single-site) has columns named option_name and option_value. This will cause the query to fail on non-multisite installations.
🔎 Proposed fix
function wu_exporter_get_pending(): array {
global $wpdb;
- $table = is_multisite() ? "{$wpdb->base_prefix}sitemeta" : "{$wpdb->base_prefix}options";
-
- $like = is_multisite() ? '\\_site\\_transient\\_wu\\_pending\\_site\\_export\\_%' : '\\_transient\\_wu\\_pending\\_site\\_export\\_%';
-
- $query = "SELECT meta_key, meta_value as site_id FROM {$table} WHERE meta_key LIKE '{$like}'";
+ if (is_multisite()) {
+ $table = "{$wpdb->base_prefix}sitemeta";
+ $like = '\\_site\\_transient\\_wu\\_pending\\_site\\_export\\_%';
+ $query = "SELECT meta_key, meta_value as site_id FROM {$table} WHERE meta_key LIKE '{$like}'";
+ } else {
+ $table = "{$wpdb->base_prefix}options";
+ $like = '\\_transient\\_wu\\_pending\\_site\\_export\\_%';
+ $query = "SELECT option_name as meta_key, option_value as site_id FROM {$table} WHERE option_name LIKE '{$like}'";
+ }
return $wpdb->get_results($query); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function wu_exporter_get_pending(): array { | |
| global $wpdb; | |
| $table = is_multisite() ? "{$wpdb->base_prefix}sitemeta" : "{$wpdb->base_prefix}options"; | |
| $like = is_multisite() ? '\\_site\\_transient\\_wu\\_pending\\_site\\_export\\_%' : '\\_transient\\_wu\\_pending\\_site\\_export\\_%'; | |
| $query = "SELECT meta_key, meta_value as site_id FROM {$table} WHERE meta_key LIKE '{$like}'"; | |
| return $wpdb->get_results($query); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery | |
| } | |
| function wu_exporter_get_pending(): array { | |
| global $wpdb; | |
| if (is_multisite()) { | |
| $table = "{$wpdb->base_prefix}sitemeta"; | |
| $like = '\\_site\\_transient\\_wu\\_pending\\_site\\_export\\_%'; | |
| $query = "SELECT meta_key, meta_value as site_id FROM {$table} WHERE meta_key LIKE '{$like}'"; | |
| } else { | |
| $table = "{$wpdb->base_prefix}options"; | |
| $like = '\\_transient\\_wu\\_pending\\_site\\_export\\_%'; | |
| $query = "SELECT option_name as meta_key, option_value as site_id FROM {$table} WHERE option_name LIKE '{$like}'"; | |
| } | |
| return $wpdb->get_results($query); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery | |
| } |
🤖 Prompt for AI Agents
In inc/functions/exporter.php around lines 191 to 202, the SELECT uses
meta_key/meta_value unconditionally which breaks single-site queries against
wp_options; change the SELECT to use meta_key, meta_value when is_multisite()
(sitemeta table) and option_name, option_value when single-site (options table),
preserving the alias for the second column as site_id and keeping the LIKE
pattern logic unchanged so the query selects the correct columns for each table.
| $table = is_multisite() ? $wpdb->base_prefix . $site_id : $wpdb->base_prefix; | ||
|
|
||
| // Get list of tables | ||
| $tables = $this->query("SHOW TABLES LIKE'" . $table . "%'"); |
There was a problem hiding this comment.
SQL syntax error: Missing space before LIKE pattern.
The query "SHOW TABLES LIKE'" . $table . "%'" is missing a space before the opening quote, resulting in SHOW TABLES LIKE'prefix%' instead of SHOW TABLES LIKE 'prefix%'.
🔎 Proposed fix
- $tables = $this->query("SHOW TABLES LIKE'" . $table . "%'");
+ $tables = $this->query("SHOW TABLES LIKE '" . $table . "%'");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $tables = $this->query("SHOW TABLES LIKE'" . $table . "%'"); | |
| $tables = $this->query("SHOW TABLES LIKE '" . $table . "%'"); |
🤖 Prompt for AI Agents
In inc/site-exporter/database/class-import.php around line 171, the SQL string
concatenation lacks a space before the LIKE pattern resulting in "SHOW TABLES
LIKE'prefix%'" — add a space before the opening quote so it becomes "SHOW TABLES
LIKE '...%'" and ensure the table prefix variable is safely escaped/quoted (e.g.
use the class's escaping method or a proper prepared/escaped value) when
concatenating the pattern.
| protected function all_records($message, $table, $callback) { | ||
| global $wpdb; | ||
|
|
||
| $offset = 0; | ||
| $step = 1000; | ||
|
|
||
| $found_posts = $wpdb->get_col("SELECT COUNT(ID) FROM {$table}"); | ||
|
|
||
| if ( ! $found_posts ) { | ||
| return false; | ||
| } | ||
|
|
||
| $found_posts = $found_posts[0]; | ||
|
|
||
| $progress_bar = \WP_CLI\Utils\make_progress_bar(sprintf('[%d] %s', $found_posts, $message), (int) $found_posts, 1); | ||
| $progress_bar->display(); | ||
|
|
||
| do { | ||
| $results = $wpdb->get_results( | ||
| $wpdb->prepare( | ||
| "SELECT * FROM {$table} LIMIT %d OFFSET %d", | ||
| [ | ||
| $step, | ||
| $offset, | ||
| ] | ||
| ) | ||
| ); | ||
|
|
||
| if ( $results ) { | ||
| foreach ( $results as $result ) { | ||
| $callback($result); | ||
| $progress_bar->tick(); | ||
| } | ||
| } | ||
|
|
||
| $offset += $step; | ||
| } while ( $results ); | ||
| } |
There was a problem hiding this comment.
Security: SQL injection risk and incorrect prepare() usage.
Lines 122 and 134-141 have security and correctness issues:
- Line 122: The
$tableparameter is directly interpolated into the SQL query without escaping, creating an SQL injection risk if the table name comes from user input. - Lines 134-141: The
$wpdb->prepare()call is incorrect. The second parameter should not be an array when using individual placeholders; instead, pass the values directly.
🔎 Proposed fixes
For line 122, if $table is always a trusted internal table name, add a comment documenting this assumption. Otherwise, validate it against a whitelist:
+ // Note: $table must be a trusted table name (not user input)
$found_posts = $wpdb->get_col("SELECT COUNT(ID) FROM {$table}");For lines 134-141, fix the prepare() syntax:
$results = $wpdb->get_results(
$wpdb->prepare(
- "SELECT * FROM {$table} LIMIT %d OFFSET %d",
- [
- $step,
- $offset,
- ]
+ "SELECT * FROM {$table} LIMIT %d OFFSET %d",
+ $step,
+ $offset
)
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| protected function all_records($message, $table, $callback) { | |
| global $wpdb; | |
| $offset = 0; | |
| $step = 1000; | |
| $found_posts = $wpdb->get_col("SELECT COUNT(ID) FROM {$table}"); | |
| if ( ! $found_posts ) { | |
| return false; | |
| } | |
| $found_posts = $found_posts[0]; | |
| $progress_bar = \WP_CLI\Utils\make_progress_bar(sprintf('[%d] %s', $found_posts, $message), (int) $found_posts, 1); | |
| $progress_bar->display(); | |
| do { | |
| $results = $wpdb->get_results( | |
| $wpdb->prepare( | |
| "SELECT * FROM {$table} LIMIT %d OFFSET %d", | |
| [ | |
| $step, | |
| $offset, | |
| ] | |
| ) | |
| ); | |
| if ( $results ) { | |
| foreach ( $results as $result ) { | |
| $callback($result); | |
| $progress_bar->tick(); | |
| } | |
| } | |
| $offset += $step; | |
| } while ( $results ); | |
| } | |
| protected function all_records($message, $table, $callback) { | |
| global $wpdb; | |
| $offset = 0; | |
| $step = 1000; | |
| // Note: $table must be a trusted table name (not user input) | |
| $found_posts = $wpdb->get_col("SELECT COUNT(ID) FROM {$table}"); | |
| if ( ! $found_posts ) { | |
| return false; | |
| } | |
| $found_posts = $found_posts[0]; | |
| $progress_bar = \WP_CLI\Utils\make_progress_bar(sprintf('[%d] %s', $found_posts, $message), (int) $found_posts, 1); | |
| $progress_bar->display(); | |
| do { | |
| $results = $wpdb->get_results( | |
| $wpdb->prepare( | |
| "SELECT * FROM {$table} LIMIT %d OFFSET %d", | |
| $step, | |
| $offset | |
| ) | |
| ); | |
| if ( $results ) { | |
| foreach ( $results as $result ) { | |
| $callback($result); | |
| $progress_bar->tick(); | |
| } | |
| } | |
| $offset += $step; | |
| } while ( $results ); | |
| } |
🤖 Prompt for AI Agents
In inc/site-exporter/mu-migration/includes/commands/class-mu-migration-base.php
around lines 116–153: the code interpolates $table directly into SQL (SQL
injection risk) and calls $wpdb->prepare() with an array for the second argument
(incorrect usage). Fix by validating/escaping the table identifier before
interpolation – ideally restrict $table to a whitelist of allowed table names or
build it from trusted pieces like $wpdb->prefix and run esc_sql() on the
identifier if dynamic; add a short comment documenting the trust assumption if
you choose whitelist/trusted. Then correct the prepare() call to pass the
placeholder values as separate arguments (not an array), e.g.
$wpdb->prepare("SELECT * FROM {$safe_table} LIMIT %d OFFSET %d", $step,
$offset); ensuring $safe_table is the validated/escaped table name.
| function light_add_user_to_blog($blog_id, $user_id, $role) { | ||
| $user = get_userdata($user_id); | ||
|
|
||
| if ( ! $user ) { | ||
| restore_current_blog(); | ||
| return new \WP_Error('user_does_not_exist', __('The requested user does not exist.')); | ||
| } | ||
|
|
||
| if ( ! get_user_meta($user_id, 'primary_blog', true) ) { | ||
| update_user_meta($user_id, 'primary_blog', $blog_id); | ||
| $details = get_blog_details($blog_id); | ||
| update_user_meta($user_id, 'source_domain', $details->domain); | ||
| } | ||
|
|
||
| $user->set_role($role); | ||
|
|
||
| /** | ||
| * Fires immediately after a user is added to a site. | ||
| * | ||
| * @since MU | ||
| * | ||
| * @param int $user_id User ID. | ||
| * @param string $role User role. | ||
| * @param int $blog_id Blog ID. | ||
| */ | ||
| do_action('add_user_to_blog', $user_id, $role, $blog_id); | ||
| wp_cache_delete($user_id, 'users'); | ||
| wp_cache_delete($blog_id . '_user_count', 'blog-details'); | ||
| } |
There was a problem hiding this comment.
Misplaced restore_current_blog() call corrupts blog stack.
restore_current_blog() is called on error (line 153) without a prior switch_to_blog() in this function. Since light_add_user_to_blog is designed to work without switching blogs (per the docblock), this call will pop an unrelated blog from the stack, corrupting the caller's context.
🔎 Proposed fix
function light_add_user_to_blog($blog_id, $user_id, $role) {
$user = get_userdata($user_id);
if ( ! $user ) {
- restore_current_blog();
return new \WP_Error('user_does_not_exist', __('The requested user does not exist.'));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function light_add_user_to_blog($blog_id, $user_id, $role) { | |
| $user = get_userdata($user_id); | |
| if ( ! $user ) { | |
| restore_current_blog(); | |
| return new \WP_Error('user_does_not_exist', __('The requested user does not exist.')); | |
| } | |
| if ( ! get_user_meta($user_id, 'primary_blog', true) ) { | |
| update_user_meta($user_id, 'primary_blog', $blog_id); | |
| $details = get_blog_details($blog_id); | |
| update_user_meta($user_id, 'source_domain', $details->domain); | |
| } | |
| $user->set_role($role); | |
| /** | |
| * Fires immediately after a user is added to a site. | |
| * | |
| * @since MU | |
| * | |
| * @param int $user_id User ID. | |
| * @param string $role User role. | |
| * @param int $blog_id Blog ID. | |
| */ | |
| do_action('add_user_to_blog', $user_id, $role, $blog_id); | |
| wp_cache_delete($user_id, 'users'); | |
| wp_cache_delete($blog_id . '_user_count', 'blog-details'); | |
| } | |
| function light_add_user_to_blog($blog_id, $user_id, $role) { | |
| $user = get_userdata($user_id); | |
| if ( ! $user ) { | |
| return new \WP_Error('user_does_not_exist', __('The requested user does not exist.')); | |
| } | |
| if ( ! get_user_meta($user_id, 'primary_blog', true) ) { | |
| update_user_meta($user_id, 'primary_blog', $blog_id); | |
| $details = get_blog_details($blog_id); | |
| update_user_meta($user_id, 'source_domain', $details->domain); | |
| } | |
| $user->set_role($role); | |
| /** | |
| * Fires immediately after a user is added to a site. | |
| * | |
| * @since MU | |
| * | |
| * @param int $user_id User ID. | |
| * @param string $role User role. | |
| * @param int $blog_id Blog ID. | |
| */ | |
| do_action('add_user_to_blog', $user_id, $role, $blog_id); | |
| wp_cache_delete($user_id, 'users'); | |
| wp_cache_delete($blog_id . '_user_count', 'blog-details'); | |
| } |
🤖 Prompt for AI Agents
inc/site-exporter/mu-migration/includes/helpers.php lines 149-177: the call to
restore_current_blog() on the user-not-found error (line ~153) improperly pops
the global blog stack even though this function never calls switch_to_blog();
remove the restore_current_blog() invocation so the function returns the
WP_Error directly, leaving the blog stack untouched; confirm there are no other
blog stack operations added in this function and run tests to ensure callers'
blog context remains intact.
| public function add_auth_headers(array $args, string $url): array { | ||
|
|
||
| if (strpos($url, 'ultimatemultisite.com') !== false) { | ||
| $addon_repo = \WP_Ultimo::get_instance()->get_addon_repository(); | ||
| $access_token = $addon_repo->get_access_token(); | ||
|
|
||
| if ($access_token) { | ||
| $args['headers']['Authorization'] = 'Bearer ' . $access_token; | ||
| } | ||
| } | ||
|
|
||
| return $args; | ||
| } |
There was a problem hiding this comment.
Security: Authorization header may leak to attacker-controlled domains.
The check strpos($url, 'ultimatemultisite.com') !== false can match URLs like https://ultimatemultisite.com.attacker.com/ or https://attacker.com/ultimatemultisite.com/path, potentially leaking the Bearer token to malicious servers.
Use a stricter host check:
🔎 Proposed fix
public function add_auth_headers(array $args, string $url): array {
- if (strpos($url, 'ultimatemultisite.com') !== false) {
+ $host = wp_parse_url($url, PHP_URL_HOST);
+ if ($host && (str_ends_with($host, '.ultimatemultisite.com') || $host === 'ultimatemultisite.com')) {
$addon_repo = \WP_Ultimo::get_instance()->get_addon_repository();
$access_token = $addon_repo->get_access_token();
if ($access_token) {
$args['headers']['Authorization'] = 'Bearer ' . $access_token;
}
}
return $args;
}🤖 Prompt for AI Agents
In inc/template-library/class-template-installer.php around lines 82–94, the
current strpos check can match attacker-controlled hosts and leak the Bearer
token; replace it with a strict host validation: parse the URL to extract the
host (use parse_url or wp_parse_url), normalize to lowercase, ensure host is
exactly "ultimatemultisite.com" or is a legitimate subdomain (host ends with
".ultimatemultisite.com" with a preceding dot), and only then add the
Authorization header (also ensure $args['headers'] is an array before
assigning).
| public function test_validation_rule_works_with_validator() { | ||
|
|
||
| $validator = new \WP_Ultimo\Helpers\Validator(); | ||
|
|
||
| // Test with non-existent email (should pass) | ||
| $data = [ | ||
| 'email' => 'new-email-' . wp_generate_uuid4() . '@example.com', | ||
| ]; | ||
|
|
||
| $rules = [ | ||
| 'email' => 'unique_customer_email', | ||
| ]; | ||
|
|
||
| $result = $validator->validate($data, $rules); | ||
| $this->assertFalse($result->fails(), 'Validation should pass for new email'); | ||
|
|
||
| $email = 'existing-customer-' . wp_generate_uuid4() . '@example.com'; | ||
|
|
||
| // Create a customer | ||
| $customer = wu_create_customer([ | ||
| 'email' => $email, | ||
| 'username' => 'validatorcust' . wp_generate_password(8, false), | ||
| 'password' => 'password123', | ||
| ]); | ||
|
|
||
| $user = $customer->get_user(); | ||
|
|
||
| // Test with existing customer email (should fail) | ||
| $data = [ | ||
| 'email' => $user->user_email, | ||
| ]; | ||
|
|
||
| $result = $validator->validate($data, $rules); | ||
| $this->assertTrue($result->fails(), 'Validation should fail for existing customer email'); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for rule registration in the Validator class
rg -n "unique_customer_email" --type=php -B 2 -A 2Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 434
🏁 Script executed:
#!/bin/bash
# Find the Validator class implementation
find . -name "*.php" -path "*/Validator*" | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 125
🏁 Script executed:
#!/bin/bash
# Search for rule registration patterns in the Validator class
rg -n "register.*rule\|add.*rule\|unique_customer_email" --type=php | head -30Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search for where custom validation rules are defined or registered
rg -n "class Validator" --type=php -A 20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 2448
🏁 Script executed:
#!/bin/bash
# Read the full Validator class to see how custom rules are registered
cat -n inc/helpers/class-validator.php | head -150Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 5958
🏁 Script executed:
#!/bin/bash
# Search for Unique_Customer_Email class or rule definition
rg -n "Unique_Customer_Email\|unique_customer_email" --type=phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Check what validation rule files exist in the Validation_Rules directory
find . -path "*/Validation_Rules/*.php" -type fRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search for where the test might be setting up or registering this custom rule
rg -n "unique_customer_email" --type=php -B 5 -A 5Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1002
🏁 Script executed:
#!/bin/bash
# Search for any file containing Unique_Customer_Email class
find . -name "*.php" -type f -exec grep -l "Unique_Customer_Email\|unique_customer_email" {} \;Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 200
🏁 Script executed:
#!/bin/bash
# Check all validation rule files
ls -la inc/helpers/Validation_Rules/ 2>/dev/null || echo "Directory not found"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Read the Unique_Customer_Email rule class
cat -n ./inc/helpers/validation-rules/class-unique-customer-email.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 2931
🏁 Script executed:
#!/bin/bash
# Check if the rule is imported and registered in the Validator class
grep -n "Unique_Customer_Email\|unique_customer_email" inc/helpers/class-validator.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Check if Unique_Customer_Email is imported at the top of Validator class
head -30 inc/helpers/class-validator.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 985
🏁 Script executed:
#!/bin/bash
# Verify if the test actually runs and passes or if it might fail
grep -n "addValidator.*unique_customer_email" inc/helpers/class-validator.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Check the full constructor to see all registered rules
sed -n '64,113p' inc/helpers/class-validator.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 2641
Rule is not registered with the Validator class.
The test references the unique_customer_email rule, but this rule is not imported or registered in the Validator class constructor. The rule class exists at inc/helpers/validation-rules/class-unique-customer-email.php, but it needs to be added to the imports and registered via $this->validator->addValidator('unique_customer_email', new Unique_Customer_Email()); in the Validator constructor for the test to pass.
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 134-134:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 130-130:
Opening parenthesis of a multi-line function call must be the last content on the line
🤖 Prompt for AI Agents
In tests/WP_Ultimo/Helpers/Unique_Customer_Email_Test.php around lines 111 to
145, the test references the 'unique_customer_email' validation rule but the
Validator never registers that rule; update the Validator class (where its
constructor lives) to import/require the rule class from
inc/helpers/validation-rules/class-unique-customer-email.php and register it by
calling $this->addValidator('unique_customer_email', new
Unique_Customer_Email()); (or $this->validator->addValidator(...) depending on
the property) inside the Validator constructor so the rule is available during
validate() calls.
| </p> | ||
|
|
||
| <!-- Short Description --> | ||
| <div class="wu-text-sm wu-text-gray-600 wu-mb-3 wu-line-clamp-2" v-html="template.short_description"></div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# List directory structure to understand the project layout
find . -type f -name "template-library.php" | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 126
🏁 Script executed:
#!/bin/bash
# Search for short_description and price_html in the codebase
echo "=== Searching for short_description ==="
rg -nP 'short_description' --type=php -g '!vendor/**' | head -30
echo ""
echo "=== Searching for price_html ==="
rg -nP 'price_html' --type=php -g '!vendor/**' | head -30
echo ""
echo "=== Checking template-library.php lines 170-205 ==="
sed -n '170,205p' ./views/template-library/template-library.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 2856
🏁 Script executed:
#!/bin/bash
# Check the full context of class-api-client.php around lines 156-157
echo "=== class-api-client.php content ==="
wc -l inc/template-library/class-api-client.php
echo ""
echo "=== Lines 140-180 of class-api-client.php ==="
sed -n '140,180p' inc/template-library/class-api-client.php
echo ""
echo "=== Searching for wp_kses in template-library directory ==="
rg -n 'wp_kses|sanitize' inc/template-library/ --type=phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 2224
Sanitize server-side data before rendering with v-html.
Lines 173 and 200 render template.short_description and template.price_html using v-html. These fields are returned unsanitized from the API client (inc/template-library/class-api-client.php lines 156-157). Since v-html does not automatically sanitize HTML, apply wp_kses_post() to these fields in the parse_template_data() method before returning them to prevent XSS vulnerabilities.
Also applies to: 200-200
🤖 Prompt for AI Agents
In views/template-library/template-library.php around lines 173 and 200,
template.short_description and template.price_html are rendered with v-html
using values returned unsanitized from inc/template-library/class-api-client.php
(around lines 156-157); update the API client's parse_template_data() to apply
wp_kses_post() to the short_description and price_html fields before returning
the template object/array so the HTML is sanitized server-side and safe to
render with v-html.
Summary
Template Library Feature
Site Exporter in Core
WP_Ultimo\Site_ExporterWordPress Sites Integration
wp-admin/network/sites.phpTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.