diff --git a/projects/packages/jetpack-mu-wpcom/changelog/add-pcg-batch-probes b/projects/packages/jetpack-mu-wpcom/changelog/add-pcg-batch-probes new file mode 100644 index 000000000000..9cb5e83ba0ba --- /dev/null +++ b/projects/packages/jetpack-mu-wpcom/changelog/add-pcg-batch-probes @@ -0,0 +1,4 @@ +Significance: patch +Type: changed + +Plugin Conflicts Guardian: probe all selected plugins together in one loopback request pair so bulk activation cost no longer scales with the number of plugins. diff --git a/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/README.md b/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/README.md index fad577cbe7ae..4202a52bf9af 100644 --- a/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/README.md +++ b/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/README.md @@ -23,11 +23,11 @@ Ships dark. Three independent filters, all default `false`: ## Activation flow 1. Admin submits an Activate request (`plugins.php?action=activate`, `…=activate-selected`, or `update.php?action=activate-plugin`). -2. `activation-guard.php` intercepts on `load-plugins.php` / `load-update.php` priority 0, verifies the nonce, and for each plugin calls `PCG_Load_Tester::test()`. -3. The load tester stashes `{ plugin, mode }` in a short-lived transient keyed by a random token, then `wp_remote_get`s `?pcg_probe=1&token=…` on this same site. Activation flows pass `mode = activation`; the post-update health check passes `mode = update` (see "Post-update health check" below). -4. `probe-endpoint.php` runs synchronously at require time (already inside `plugins_loaded` priority 10 via `load_features()`), validates + consumes the token, gates on the per-mode filter (`pcg_guard_activation` for activation, `pcg_guard_updates` for update), defines `WP_SANDBOX_SCRAPING` so core's fatal handler steps aside, arms a shutdown handler, and (in activation mode only) `require`s the plugin's main file. In update mode the file is already loaded by WP's normal bootstrap and re-requiring would fatal with "Cannot redeclare class/function" — the probe just verifies that bootstrap completed cleanly. +2. `activation-guard.php` intercepts on `load-plugins.php` / `load-update.php` priority 0, verifies the nonce, filters the request down to eligible plugins (passes `validate_file`, not already active, file exists), and calls `PCG_Load_Tester::test()` once with the full batch. +3. The load tester stashes `{ plugins, mode }` in a short-lived transient keyed by a random token, then fires the probe against `?pcg_probe=1&token=…` on this same site. Activation flows pass `mode = activation`; the post-update health check passes `mode = update` (see "Post-update health check" below). +4. `probe-endpoint.php` runs synchronously at require time (already inside `plugins_loaded` priority 10 via `load_features()`), validates + consumes the token, gates on the per-mode filter (`pcg_guard_activation` for activation, `pcg_guard_updates` for update), defines `WP_SANDBOX_SCRAPING` so core's fatal handler steps aside, arms a shutdown handler, and in activation mode `require_once`s each plugin's main file in order under that single request. Probe cost is constant regardless of how many plugins are activated, and conflicts that only fire when two plugins load together (duplicate class, shared global) are caught — which a per-plugin probe model couldn't see. In update mode the files are already loaded by WP's normal bootstrap and re-requiring would fatal with "Cannot redeclare class/function" — the probe just verifies that bootstrap completed cleanly. 5. Two probes fire in parallel via `\WpOrg\Requests\Requests::request_multiple()`: one against `home_url('/')` (front-end) and one against `admin_url('index.php')` with `pcg_admin=1` and the admin's WP auth cookies forwarded so `auth_redirect()` clears. The admin probe defers its verdict to `admin_init` priority `PHP_INT_MAX`; the front-end probe emits on `wp_loaded`. A captured `fatal` / `throwable` from either probe wins; otherwise the front-end verdict is returned. A 302 on the admin probe (cookies missing/expired) becomes a distinct `ok-inconclusive` status that's still treated as a non-blocking pass — that way transport quirks don't break activation, but the signal can be measured separately from a clean `ok`. -6. If any plugin failed, the guard stashes reasons in a per-user transient and redirects to `plugins.php?pcg_blocked=1`; the admin notice reads the transient and renders it. +6. On a fatal/throwable the guard attributes the failure to one plugin in the batch — preferring the explicit `plugin` field (set when a `Throwable` is caught around the `require`), then falling back to matching the captured `file` against each plugin's directory. The whole batch is blocked as a unit; the notice tells the admin which plugin caused the fatal so they can retry without it. ``` Admin click Activate @@ -36,9 +36,9 @@ Ships dark. Three independent filters, all default `false`: activation-guard.php ──► verify nonce + capability │ ▼ - PCG_Load_Tester::test() + PCG_Load_Tester::test( [paths…] ) │ - │ stash { plugin, mode } in transient (random token) + │ stash { plugins, mode } in transient (random token) ▼ GET /?pcg_probe=1&token=… ◄── HTTP self-request │ @@ -49,8 +49,9 @@ Ships dark. Three independent filters, all default `false`: (pcg_guard_activation | pcg_guard_updates) define WP_SANDBOX_SCRAPING register shutdown handler - require $plugin_main (activation mode only — update mode - skips this; plugin already loaded by WP) + foreach $plugin_main: require_once (activation mode only — update + mode skips this; plugins already + loaded by WP's bootstrap) │ ├───► fatal / throwable ──► {status: fatal|throwable} (HTTP 200) │ │ @@ -96,7 +97,7 @@ Gated on `pcg_guard_updates`. Runs *after* files are swapped, in a fresh HTTP re 1. `upgrader_pre_install` — `PCG_Snapshot::capture()` reads the current plugin's `Version` and `is_plugin_active()`, stashes them in a transient keyed by the plugin basename, **and copies the live plugin files to `/pcg-backups//`** (override via the `pcg_backup_root` filter) so we can restore offline without re-downloading. 2. Core extracts + copies the new files (the original copy is still safely tucked away under `pcg-backups/`). -3. `upgrader_process_complete` (priority 99) — `update-healthcheck.php` drains the snapshots for every plugin in `hook_extra['plugins']`, keeps the ones that were active and whose new files are still on disk, and runs **one** `PCG_Load_Tester::test( $candidate, PCG_Load_Tester::MODE_UPDATE )` for the whole batch. MODE_UPDATE checks whether the site as a whole bootstraps; it doesn't isolate a specific plugin, so a single probe is enough. The probe endpoint skips the `require` in update mode and just observes whether the (already-loaded) new code completes the bootstrap cleanly. +3. `upgrader_process_complete` (priority 99) — `update-healthcheck.php` drains the snapshots for every plugin in `hook_extra['plugins']`, keeps the ones that were active and whose new files are still on disk, and runs **one** `PCG_Load_Tester::test( $plugin_mains, PCG_Load_Tester::MODE_UPDATE )` for the whole batch. MODE_UPDATE checks whether the site as a whole bootstraps; it doesn't isolate a specific plugin, so a single probe is enough. The probe endpoint skips the `require_once` in update mode and just observes whether the (already-loaded) new code completes the bootstrap cleanly. 4. On `ok` (or any inconclusive non-fatal status), every backup in the batch is deleted and we're done. 5. On `fatal` / `throwable`, `PCG_Rollback::to_snapshot()` runs for **every** snapshot in the batch — deactivating each broken plugin, **swapping the new files for the saved local backup** via rename (or copy + delete-source as a fallback for cross-fs cases), and reactivating if the plugin was active. We can't tell which plugin in the batch caused the fatal, so restoring the whole batch is the safe call. 6. If a local backup is missing or the swap fails, `PCG_Rollback` falls back to fetching `https://downloads.wordpress.org/plugin/{slug}.{old_version}.zip` and reinstalling via `Plugin_Upgrader`. This still helps for .org plugins on hosts where the local backup couldn't be created (full disk, restrictive perms). diff --git a/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/activation-guard.php b/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/activation-guard.php index 528164058339..441ca8f3a6fb 100644 --- a/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/activation-guard.php +++ b/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/activation-guard.php @@ -74,15 +74,20 @@ function pcg_guard_maybe_block_activation() { } /** - * Probe each plugin; return map of basename => reason for those that failed. + * Probe the requested plugins together in a single loopback request pair + * and return a map of basename => reason for any that fail. + * + * Eligible plugins (passes `validate_file`, not already active, file + * exists on disk) are passed to `PCG_Load_Tester::test()` as one batch, + * so probe cost is constant in N rather than 2N round-trips. As a side + * effect this also surfaces conflicts that only fire when two plugins + * load together (duplicate class, shared global, etc.). * * @param string[] $plugins Plugin basenames (e.g. "akismet/akismet.php"). * @return array */ function pcg_guard_evaluate_plugins( $plugins ) { - $blocked = array(); - $tester = new PCG_Load_Tester(); - + $paths = array(); foreach ( $plugins as $plugin ) { if ( 0 !== validate_file( $plugin ) ) { continue; @@ -94,14 +99,87 @@ function pcg_guard_evaluate_plugins( $plugins ) { if ( ! is_file( $path ) ) { continue; } - $result = $tester->test( $path ); - $status = (string) ( $result['status'] ?? '' ); - if ( 'fatal' === $status || 'throwable' === $status ) { - $blocked[ $plugin ] = pcg_guard_format_block_reason( $result ); + $paths[ $plugin ] = $path; + } + if ( empty( $paths ) ) { + return array(); + } + + $tester = new PCG_Load_Tester(); + $result = $tester->test( array_values( $paths ) ); + $status = (string) ( $result['status'] ?? '' ); + if ( 'fatal' !== $status && 'throwable' !== $status ) { + return array(); + } + + $blocked_plugin = pcg_guard_get_blocked_plugin( $result, $paths ); + if ( '' !== $blocked_plugin ) { + return array( + $blocked_plugin => pcg_guard_format_block_reason( $result ), + ); + } + + // Verdict didn't pin a specific plugin (e.g. probe terminated without a + // JSON body, or the captured `file` was outside any candidate's tree). + // Surface a batch-level message so we don't blame an arbitrary plugin. + $reason = sprintf( + /* translators: 1: locale-formatted list of plugin basenames; 2: probe verdict reason. */ + __( 'One of these plugins caused a fatal during the pre-flight check: %1$s. Reason: %2$s', 'jetpack-mu-wpcom' ), + wp_sprintf_l( '%l', array_keys( $paths ) ), + pcg_guard_format_block_reason( $result ) + ); + return array( '' => $reason ); +} + +/** + * Map a fatal/throwable verdict back to the plugin basename that caused + * it. Tries, in order: the explicit `plugin` field on the verdict (set + * when a `Throwable` was caught around `require`), an exact match of + * the captured `file` against a plugin's main file (covers flat-file + * plugins like `hello.php`), and a prefix match of the captured `file` + * against a plugin's own subdirectory under `WP_PLUGIN_DIR`. + * + * Returns `''` when none of those match (e.g. the verdict has no + * `file`/`plugin`, or `file` lies outside any candidate's tree). The + * caller is expected to surface a batch-level message in that case + * rather than guessing a plugin. + * + * @param array $result A fatal/throwable probe verdict. + * @param array $paths Map of plugin basename => absolute main file path. + * @return string Plugin basename to attribute the failure to, or '' if undetermined. + */ +function pcg_guard_get_blocked_plugin( $result, $paths ) { + $explicit = (string) ( $result['plugin'] ?? '' ); + if ( '' !== $explicit ) { + foreach ( $paths as $basename => $path ) { + if ( $path === $explicit ) { + return $basename; + } + } + } + + $fatal_file = (string) ( $result['file'] ?? '' ); + if ( '' !== $fatal_file ) { + foreach ( $paths as $basename => $path ) { + if ( $path === $fatal_file ) { + return $basename; + } + } + // Subdirectory plugins only — a flat-file plugin's dirname is + // `WP_PLUGIN_DIR`, which would prefix-match every other plugin's + // files in the batch and produce false attributions. + foreach ( $paths as $basename => $path ) { + $plugin_dir = dirname( $path ); + if ( WP_PLUGIN_DIR === $plugin_dir ) { + continue; + } + if ( str_starts_with( $fatal_file, $plugin_dir . '/' ) ) { + return $basename; + } } } - return $blocked; + return ''; } /** @@ -165,10 +243,16 @@ function pcg_guard_render_block_notice() {

    $reason ) : ?> -
  • —
    • +
  • + + — + + + +
-

+

(string) $p, $plugin_mains ), + static fn( $p ) => '' !== $p && is_file( $p ) && is_readable( $p ) + ) + ); + if ( empty( $plugin_mains ) ) { return array( 'status' => 'error', - 'reason' => 'Plugin main file not found for load probe.', + 'reason' => 'No probable plugin main files supplied.', ); } - $front = $this->prepare_probe( $plugin_main, home_url( '/' ), false, $mode ); - $admin = $this->prepare_probe( $plugin_main, admin_url( 'index.php' ), true, $mode ); + $front = $this->prepare_probe( $plugin_mains, home_url( '/' ), false, $mode ); + $admin = $this->prepare_probe( $plugin_mains, admin_url( 'index.php' ), true, $mode ); try { $responses = \WpOrg\Requests\Requests::request_multiple( @@ -75,8 +73,15 @@ public function test( $plugin_main, $mode = self::MODE_ACTIVATION ) { delete_transient( self::transient_key( $admin['token'] ) ); } - $front_result = $this->parse_response( $responses['front'], $plugin_main, false ); - $admin_result = $this->parse_response( $responses['admin'], $plugin_main, true ); + $front_result = $this->parse_response( $responses['front'], false ); + $admin_result = $this->parse_response( $responses['admin'], true ); + + // Log transport-level errors (most often timeouts at PROBE_TIMEOUT) + // so we can see how often they fire before deciding whether to + // scale the timeout with batch size. + if ( $this->is_error( $front_result ) || $this->is_error( $admin_result ) ) { + $this->log_probe_error( $mode, $plugin_mains, $front_result, $admin_result ); + } // fatal/throwable wins; an inconclusive `error` from one probe must // not shadow a real fatal from the other. Front-end is the canonical @@ -101,6 +106,87 @@ protected function is_block( $result ) { return 'fatal' === $status || 'throwable' === $status; } + /** + * Whether a verdict is a transport-level error (timeout, connection + * failure, non-JSON body). Distinct from `is_block` — errors are + * inconclusive and don't block activation, but are worth logging. + * + * @param array $result Probe verdict. + * @return bool + */ + protected function is_error( $result ) { + $status = is_array( $result ) ? (string) ( $result['status'] ?? '' ) : ''; + return 'error' === $status; + } + + /** + * Log a probe transport error to logstash whenever either probe came + * back as `error` (timeout at PROBE_TIMEOUT, connection failure, + * non-JSON body). Lets us measure timeout frequency vs. batch size + * before deciding whether to scale `PROBE_TIMEOUT` with N. No-op + * outside WordPress.com (no `log2logstash` available). + * + * @param string $mode Probe mode constant. + * @param string[] $plugin_mains Absolute paths to plugin main PHP files. + * @param array $front_result Front-end probe verdict. + * @param array $admin_result Admin probe verdict. + * @return void + */ + protected function log_probe_error( $mode, array $plugin_mains, array $front_result, array $admin_result ) { + if ( ! function_exists( 'log2logstash' ) ) { + $log2logstash_path = WP_CONTENT_DIR . '/lib/log2logstash/log2logstash.php'; + if ( ! is_readable( $log2logstash_path ) ) { + return; + } + require_once $log2logstash_path; + } + + log2logstash( + array( + 'feature' => 'plugin-conflicts-guardian', + 'message' => 'Probe transport error', + 'extra' => wp_json_encode( + array( + 'mode' => $mode, + 'plugins' => $this->relative_basenames( $plugin_mains ), + 'front' => $this->probe_error_reason( $front_result ), + 'admin' => $this->probe_error_reason( $admin_result ), + ), + JSON_UNESCAPED_SLASHES + ), + ) + ); + } + + /** + * Strip `WP_PLUGIN_DIR/` from each absolute path so log entries carry + * the canonical plugin basename (e.g. `akismet/akismet.php`). + * + * @param string[] $plugin_mains Absolute paths to plugin main PHP files. + * @return string[] + */ + protected function relative_basenames( array $plugin_mains ) { + $prefix = WP_PLUGIN_DIR . '/'; + $out = array(); + foreach ( $plugin_mains as $path ) { + $out[] = str_starts_with( (string) $path, $prefix ) + ? substr( (string) $path, strlen( $prefix ) ) + : (string) $path; + } + return $out; + } + + /** + * One-line reason from a probe verdict — `reason` if set, else + * `status`, else empty. Used for diagnostic logs. + * + * @param array $result Probe verdict. + * @return string + */ + protected function probe_error_reason( array $result ) { + return (string) ( $result['reason'] ?? $result['status'] ?? '' ); + } + /** * Build the transient payload that the probe endpoint will consume. * @@ -108,30 +194,30 @@ protected function is_block( $result ) { * needing a live HTTP loopback. Not part of the public API. * * @internal - * @param string $plugin_main Absolute path to the plugin's main PHP file. - * @param string $mode Probe mode constant. - * @return array{plugin:string,mode:string} + * @param string[] $plugin_mains Absolute paths to plugin main PHP files. + * @param string $mode Probe mode constant. + * @return array{plugins:string[],mode:string} */ - public static function build_probe_payload( $plugin_main, $mode = self::MODE_ACTIVATION ) { + public static function build_probe_payload( array $plugin_mains, $mode = self::MODE_ACTIVATION ) { return array( - 'plugin' => (string) $plugin_main, - 'mode' => self::MODE_UPDATE === $mode ? self::MODE_UPDATE : self::MODE_ACTIVATION, + 'plugins' => array_values( array_map( static fn( $p ) => (string) $p, $plugin_mains ) ), + 'mode' => self::MODE_UPDATE === $mode ? self::MODE_UPDATE : self::MODE_ACTIVATION, ); } /** - * Stash a probe transient and build the request descriptor for - * `Requests::request_multiple`. + * Stash a probe transient and build the `Requests::request_multiple` + * descriptor for one of the two parallel probes. * - * @param string $plugin_main Absolute path to the plugin's main PHP file. - * @param string $base_url Base URL to probe (front-end or admin). - * @param bool $is_admin Adds `pcg_admin=1` and forwards auth cookies. - * @param string $mode Probe mode constant. + * @param string[] $plugin_mains Absolute paths to plugin main PHP files. + * @param string $base_url Front-end or admin base URL. + * @param bool $is_admin Adds `pcg_admin=1` and forwards auth cookies. + * @param string $mode Probe mode constant. * @return array{token:string,request:array} */ - protected function prepare_probe( $plugin_main, $base_url, $is_admin, $mode = self::MODE_ACTIVATION ) { + protected function prepare_probe( array $plugin_mains, $base_url, $is_admin, $mode = self::MODE_ACTIVATION ) { $token = wp_generate_password( 32, false ); - set_transient( self::transient_key( $token ), self::build_probe_payload( $plugin_main, $mode ), self::TOKEN_LIFETIME ); + set_transient( self::transient_key( $token ), self::build_probe_payload( $plugin_mains, $mode ), self::TOKEN_LIFETIME ); $query = array( 'pcg_probe' => '1', @@ -159,13 +245,12 @@ protected function prepare_probe( $plugin_main, $base_url, $is_admin, $mode = se /** * Translate a `Requests::request_multiple` response into a probe verdict. * - * @param mixed $response A `WpOrg\Requests\Response`, or an exception - * thrown for that single request. - * @param string $plugin_main Plugin main file (for fallback diagnostics). - * @param bool $is_admin True when this was the admin probe. - * @return array{status:string,reason?:string,errno?:int,class?:string,message?:string,file?:string,line?:int} + * @param mixed $response A `WpOrg\Requests\Response`, or an exception + * thrown for that single request. + * @param bool $is_admin True when this was the admin probe. + * @return array{status:string,reason?:string,errno?:int,class?:string,message?:string,file?:string,line?:int,plugin?:string} */ - protected function parse_response( $response, $plugin_main, $is_admin ) { + protected function parse_response( $response, $is_admin ) { if ( $response instanceof \Throwable ) { return array( 'status' => 'error', @@ -194,8 +279,6 @@ protected function parse_response( $response, $plugin_main, $is_admin ) { return array( 'status' => 'fatal', 'message' => 'Probe request returned HTTP 500 without a JSON verdict; the plugin likely fatals during load.', - 'file' => basename( $plugin_main ), - 'line' => 0, ); } @@ -206,11 +289,9 @@ protected function parse_response( $response, $plugin_main, $is_admin ) { return array( 'status' => 'fatal', 'message' => sprintf( - 'Probe completed without a verdict (HTTP %d, non-JSON body). The plugin may have terminated the request during load, init, or admin_init.', + 'Probe completed without a verdict (HTTP %d, non-JSON body). A plugin in the batch may have terminated the request during load, init, or admin_init.', $code ), - 'file' => basename( $plugin_main ), - 'line' => 0, ); } diff --git a/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/probe-endpoint.php b/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/probe-endpoint.php index d06a2dc4a135..8e7a5f62f9eb 100644 --- a/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/probe-endpoint.php +++ b/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/probe-endpoint.php @@ -32,12 +32,17 @@ function pcg_maybe_handle_probe() { $payload = get_transient( $key ); delete_transient( $key ); - if ( ! is_array( $payload ) || ! isset( $payload['plugin'] ) || ! isset( $payload['mode'] ) ) { + if ( ! is_array( $payload ) || ! isset( $payload['plugins'] ) || ! isset( $payload['mode'] ) ) { pcg_probe_bail_error( 'Invalid or expired probe token.', 403 ); } - $plugin_main = (string) $payload['plugin']; - $mode = (string) $payload['mode']; - if ( '' === $plugin_main || ! in_array( $mode, array( PCG_Load_Tester::MODE_ACTIVATION, PCG_Load_Tester::MODE_UPDATE ), true ) ) { + $plugin_mains = is_array( $payload['plugins'] ) ? array_values( + array_filter( + array_map( static fn( $p ) => (string) $p, $payload['plugins'] ), + static fn( $p ) => '' !== $p + ) + ) : array(); + $mode = (string) $payload['mode']; + if ( empty( $plugin_mains ) || ! in_array( $mode, array( PCG_Load_Tester::MODE_ACTIVATION, PCG_Load_Tester::MODE_UPDATE ), true ) ) { pcg_probe_bail_error( 'Invalid or expired probe token.', 403 ); } @@ -49,8 +54,18 @@ function pcg_maybe_handle_probe() { pcg_probe_bail_error( 'Plugin Conflicts Guardian is disabled.', 403 ); } - if ( ! is_file( $plugin_main ) || ! is_readable( $plugin_main ) ) { - pcg_probe_bail_error( 'Probe target is no longer readable.', 404 ); + // Drop unreadable entries instead of bailing on the first one. Bailing + // would emit `error`, which the activation guard treats as a non-block + // and lets the activation through — masking a fatal in a later + // readable plugin. Only bail when nothing readable remains. + $plugin_mains = array_values( + array_filter( + $plugin_mains, + static fn( $p ) => is_file( $p ) && is_readable( $p ) + ) + ); + if ( empty( $plugin_mains ) ) { + pcg_probe_bail_error( 'No probe targets are readable.', 404 ); } // Tell WP's fatal handler to stand down so ours can emit JSON. @@ -63,24 +78,25 @@ function pcg_maybe_handle_probe() { register_shutdown_function( 'pcg_probe_shutdown' ); - // Activation mode: the plugin is inactive, so explicitly require it - // to exercise its load path. Update mode: the plugin is already loaded - // by WP's normal bootstrap (it was active before the update); we only - // need to verify that bootstrap completed cleanly with the new code, - // and re-requiring would fatal with "Cannot redeclare class/function". + // Activation: load each plugin to exercise its load path. Update: skip; + // re-requiring an already-loaded plugin would fatal with + // "Cannot redeclare". The shutdown handler catches either way. if ( PCG_Load_Tester::MODE_ACTIVATION === $mode ) { - try { - require $plugin_main; - } catch ( \Throwable $t ) { - pcg_probe_respond( - array( - 'status' => 'throwable', - 'class' => get_class( $t ), - 'message' => $t->getMessage(), - 'file' => basename( $t->getFile() ), - 'line' => $t->getLine(), - ) - ); + foreach ( $plugin_mains as $plugin_main ) { + try { + require_once $plugin_main; + } catch ( \Throwable $t ) { + pcg_probe_respond( + array( + 'status' => 'throwable', + 'plugin' => $plugin_main, + 'class' => get_class( $t ), + 'message' => $t->getMessage(), + 'file' => $t->getFile(), + 'line' => $t->getLine(), + ) + ); + } } } @@ -114,7 +130,7 @@ function pcg_probe_shutdown() { 'status' => 'fatal', 'errno' => (int) $error['type'], 'message' => (string) $error['message'], - 'file' => basename( (string) $error['file'] ), + 'file' => (string) $error['file'], 'line' => (int) $error['line'], ) ); diff --git a/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/update-healthcheck.php b/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/update-healthcheck.php index 02f9ba17107b..2705d9b13009 100644 --- a/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/update-healthcheck.php +++ b/projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/update-healthcheck.php @@ -36,19 +36,14 @@ function pcg_healthcheck_capture_snapshot( $return, $hook_extra ) { if ( '' === $plugin_file ) { return $return; } - // Inactive plugins can't take the site down on load, so the - // post-update probe bails on them. Skip the snapshot entirely - // here too — otherwise we'd stage a backup we never consume. + // Skip inactive (probe ignores them) and network-active (probe is + // per-site, rollback flips one plugin — wrong shape for network). if ( ! function_exists( 'is_plugin_active' ) ) { require_once ABSPATH . 'wp-admin/includes/plugin.php'; } if ( ! is_plugin_active( $plugin_file ) ) { return $return; } - // Network-activated plugins on multisite affect every site in the - // network. Our probe runs against the current site only and our - // rollback flips a single plugin off — neither is the right shape - // for a network plugin, so bail before touching anything. if ( is_multisite() && is_plugin_active_for_network( $plugin_file ) ) { return $return; } @@ -124,12 +119,12 @@ function pcg_healthcheck_after_update( $upgrader, $hook_extra ) { // phpcs:ignor return; } - // One probe for the whole batch. The plugin_main argument is just - // a target for the load tester to attribute fatals to in its - // diagnostics — the probe itself boots the full site. - $tester = new PCG_Load_Tester(); - $result = $tester->test( $candidates[0]['plugin_main'], PCG_Load_Tester::MODE_UPDATE ); - $status = (string) ( $result['status'] ?? '' ); + // MODE_UPDATE skips require_once and just observes the bootstrap, so one + // probe suffices for the whole batch. The paths are only readability checks. + $tester = new PCG_Load_Tester(); + $plugin_mains = array_values( array_column( $candidates, 'plugin_main' ) ); + $result = $tester->test( $plugin_mains, PCG_Load_Tester::MODE_UPDATE ); + $status = (string) ( $result['status'] ?? '' ); // Anything other than a captured fatal is a no-op rollback-wise: ok = // the update is fine; error = inconclusive transport failure we don't diff --git a/projects/packages/jetpack-mu-wpcom/tests/php/features/plugin-conflicts-guardian/Plugin_Conflicts_Guardian_Test.php b/projects/packages/jetpack-mu-wpcom/tests/php/features/plugin-conflicts-guardian/Plugin_Conflicts_Guardian_Test.php index 049a294fd5c6..4f4a8f8afb1a 100644 --- a/projects/packages/jetpack-mu-wpcom/tests/php/features/plugin-conflicts-guardian/Plugin_Conflicts_Guardian_Test.php +++ b/projects/packages/jetpack-mu-wpcom/tests/php/features/plugin-conflicts-guardian/Plugin_Conflicts_Guardian_Test.php @@ -213,20 +213,20 @@ public function test_load_tester_mode_constant_values() { * the explicit mode argument when supplied. */ public function test_build_probe_payload_carries_mode() { - $default = PCG_Load_Tester::build_probe_payload( '/abs/foo/foo.php' ); + $default = PCG_Load_Tester::build_probe_payload( array( '/abs/foo/foo.php' ) ); $this->assertSame( array( - 'plugin' => '/abs/foo/foo.php', - 'mode' => 'activation', + 'plugins' => array( '/abs/foo/foo.php' ), + 'mode' => 'activation', ), $default ); - $update = PCG_Load_Tester::build_probe_payload( '/abs/foo/foo.php', PCG_Load_Tester::MODE_UPDATE ); + $update = PCG_Load_Tester::build_probe_payload( array( '/abs/foo/foo.php', '/abs/bar/bar.php' ), PCG_Load_Tester::MODE_UPDATE ); $this->assertSame( array( - 'plugin' => '/abs/foo/foo.php', - 'mode' => 'update', + 'plugins' => array( '/abs/foo/foo.php', '/abs/bar/bar.php' ), + 'mode' => 'update', ), $update ); @@ -237,7 +237,7 @@ public function test_build_probe_payload_carries_mode() { * the transient with a value the endpoint will reject. */ public function test_build_probe_payload_rejects_unknown_mode() { - $payload = PCG_Load_Tester::build_probe_payload( '/abs/foo/foo.php', 'bogus' ); + $payload = PCG_Load_Tester::build_probe_payload( array( '/abs/foo/foo.php' ), 'bogus' ); $this->assertSame( 'activation', $payload['mode'] ); } @@ -314,6 +314,155 @@ public function test_format_block_reason( array $result, string $expected ) { $this->assertSame( $expected, pcg_guard_format_block_reason( $result ) ); } + /** + * The load tester's `test()` rejects an empty / non-existent input list + * before doing any HTTP work, returning an `error` verdict. + */ + public function test_load_tester_rejects_empty_plugin_list() { + $tester = new PCG_Load_Tester(); + + $verdict = $tester->test( array() ); + $this->assertSame( 'error', $verdict['status'] ); + $this->assertNotEmpty( $verdict['reason'] ?? '' ); + + $verdict = $tester->test( array( '', '/no/such/file/pcg-missing.php' ) ); + $this->assertSame( 'error', $verdict['status'] ); + $this->assertNotEmpty( $verdict['reason'] ?? '' ); + } + + /** + * The explicit `plugin` field on a Throwable verdict wins when it + * matches a known path in the batch. + */ + public function test_blame_uses_explicit_plugin_field() { + $paths = array( + 'foo/foo.php' => WP_PLUGIN_DIR . '/foo/foo.php', + 'bar/bar.php' => WP_PLUGIN_DIR . '/bar/bar.php', + ); + + $blamed = pcg_guard_get_blocked_plugin( + array( + 'status' => 'throwable', + 'plugin' => WP_PLUGIN_DIR . '/bar/bar.php', + 'file' => WP_PLUGIN_DIR . '/bar/bar.php', + ), + $paths + ); + + $this->assertSame( 'bar/bar.php', $blamed ); + } + + /** + * When the explicit `plugin` field doesn't match anything in the batch, + * attribution falls through to the captured `file`. + */ + public function test_blame_falls_back_to_file_when_explicit_plugin_unknown() { + $paths = array( + 'foo/foo.php' => WP_PLUGIN_DIR . '/foo/foo.php', + 'bar/bar.php' => WP_PLUGIN_DIR . '/bar/bar.php', + ); + + $blamed = pcg_guard_get_blocked_plugin( + array( + 'status' => 'throwable', + 'plugin' => '/some/path/we/dont/recognise.php', + 'file' => WP_PLUGIN_DIR . '/bar/lib/helper.php', + ), + $paths + ); + + $this->assertSame( 'bar/bar.php', $blamed ); + } + + /** + * An exact-path match against a plugin's main file wins — covers + * flat-file plugins where the prefix match would be unsafe. + */ + public function test_blame_matches_flat_file_plugin_exactly() { + $paths = array( + 'hello.php' => WP_PLUGIN_DIR . '/hello.php', + 'foo/foo.php' => WP_PLUGIN_DIR . '/foo/foo.php', + ); + + $blamed = pcg_guard_get_blocked_plugin( + array( + 'status' => 'fatal', + 'file' => WP_PLUGIN_DIR . '/hello.php', + ), + $paths + ); + + $this->assertSame( 'hello.php', $blamed ); + } + + /** + * A fatal in a file inside a plugin's own subdirectory is attributed + * via prefix match. + */ + public function test_blame_matches_subdirectory_plugin_prefix() { + $paths = array( + 'foo/foo.php' => WP_PLUGIN_DIR . '/foo/foo.php', + 'bar/bar.php' => WP_PLUGIN_DIR . '/bar/bar.php', + ); + + $blamed = pcg_guard_get_blocked_plugin( + array( + 'status' => 'fatal', + 'file' => WP_PLUGIN_DIR . '/bar/lib/deeply/nested.php', + ), + $paths + ); + + $this->assertSame( 'bar/bar.php', $blamed ); + } + + /** + * A fatal at `WP_PLUGIN_DIR/something.php` must NOT be attributed to a + * flat-file plugin in the batch via the prefix arm — that would + * produce a false attribution because the dirname is the plugins root. + * Falls through to the undetermined branch (returns ''). + */ + public function test_blame_does_not_false_match_flat_file_plugins_via_prefix() { + $paths = array( + 'hello.php' => WP_PLUGIN_DIR . '/hello.php', + 'world.php' => WP_PLUGIN_DIR . '/world.php', + ); + + $blamed = pcg_guard_get_blocked_plugin( + array( + 'status' => 'fatal', + 'file' => WP_PLUGIN_DIR . '/something-unrelated.php', + ), + $paths + ); + + $this->assertSame( '', $blamed ); + } + + /** + * With nothing on the verdict to attribute against, the helper returns + * `''` so the caller can surface a batch-level message instead of + * blaming an arbitrary plugin. + */ + public function test_blame_returns_empty_when_unattributable() { + $paths = array( + 'foo/foo.php' => WP_PLUGIN_DIR . '/foo/foo.php', + 'bar/bar.php' => WP_PLUGIN_DIR . '/bar/bar.php', + ); + + $blamed = pcg_guard_get_blocked_plugin( array( 'status' => 'fatal' ), $paths ); + $this->assertSame( '', $blamed ); + + $blamed = pcg_guard_get_blocked_plugin( + array( + 'status' => 'fatal', + 'file' => '/var/www/wp-includes/load.php', + ), + $paths + ); + $this->assertSame( '', $blamed ); + } + /** * A package with only valid PHP files scans clean. */