Plugin Conflicts Guardian: Pre-flight activation gate #48261
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Significance: minor | ||
| Type: added | ||
|
|
||
| Plugin Conflicts Guardian: new pre-flight check that blocks a plugin activation (via plugins.php or update.php) when a short-lived HTTP probe captures a fatal during load or the init cycle; gated behind the pcg_guard_activation filter. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| # Plugin Conflicts Guardian | ||
|
|
||
| Pre-flight plugin-activation check. When an admin clicks Activate (or finishes an Upload Plugin install), this feature loads the plugin in an isolated HTTP request and refuses the activation if that probe captures a fatal — the site stays up instead of entering recovery mode. | ||
|
|
||
| Ships dark: gated behind `apply_filters( 'pcg_guard_activation', false )`. Set the filter to `true` to enable both the activation and update guards. | ||
|
|
||
| ## Files | ||
|
|
||
| | File | Role | | ||
| | --- | --- | | ||
| | `plugin-conflicts-guardian.php` | Bootstrap. Wires the requires for the other files. | | ||
| | `class-pcg-load-tester.php` | Client: fires the probe HTTP request and parses the verdict. | | ||
| | `probe-endpoint.php` | Server: handles `?pcg_probe=1`, requires the plugin, captures any fatal. | | ||
| | `activation-guard.php` | Hooks `load-plugins.php` / `load-update.php` and blocks failing activations. | | ||
| | `update-guard.php` | Hooks `upgrader_source_selection` to refuse installs/updates with PHP parse errors. | | ||
|
|
||
| ## 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 the plugin path in a short-lived transient keyed by a random token, then `wp_remote_get`s `?pcg_probe=1&token=…` on this same site. | ||
| 4. `probe-endpoint.php` runs synchronously at require time (already inside `plugins_loaded` priority 10 via `load_features()`), validates + consumes the token, defines `WP_SANDBOX_SCRAPING` so core's fatal handler steps aside, arms a shutdown handler, and `require`s the plugin's main file. | ||
| 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. | ||
|
|
||
| ``` | ||
| Admin click Activate | ||
| │ | ||
| ▼ | ||
| activation-guard.php ──► verify nonce + capability | ||
| │ | ||
| ▼ | ||
| PCG_Load_Tester::test() | ||
| │ | ||
| │ stash {path} in transient (random token) | ||
| ▼ | ||
| GET /?pcg_probe=1&token=… ◄── HTTP self-request | ||
| │ | ||
| ▼ | ||
| probe-endpoint.php | ||
| validate + consume token | ||
| define WP_SANDBOX_SCRAPING | ||
| register shutdown handler | ||
| require $plugin_main | ||
| │ | ||
| ├───► fatal / throwable ──► {status: fatal|throwable} (HTTP 200) | ||
| │ │ | ||
| │ ▼ | ||
| │ Guard stashes reason, | ||
| │ 302 → plugins.php?pcg_blocked=1 | ||
| │ | ||
| └───► clean load | ||
| │ | ||
| ▼ | ||
| wait for init / admin_init / wp_loaded | ||
| │ | ||
| ▼ | ||
| {status: ok} (HTTP 200) | ||
| │ | ||
| ▼ | ||
| Guard hands off to core activate_plugin() | ||
| ``` | ||
|
|
||
| ## Why HTTP, not a CLI subprocess | ||
|
|
||
| Atomic and some managed hosts sandbox web-PHP so `proc_open` can't find/exec a CLI binary (`open_basedir` + restricted exec). A separate HTTP request is isolated from the admin request: if the plugin fatals, the probe 500s but the parent sees JSON via the shutdown handler, and the admin page keeps rendering. | ||
|
|
||
| ## Limitations | ||
|
|
||
| - Only catches errors hit while `require`-ing the main file and during `plugins_loaded` / `init` / `admin_init` callbacks. Errors that surface only on later hooks (e.g. `template_redirect`, REST) are invisible. | ||
| - The probe endpoint is wired up via jetpack-mu-wpcom's `load_features()` at `plugins_loaded` priority 10, so plugin callbacks registered for `plugins_loaded` at priority < 10 will have already fired before the plugin under test is `require`d. Fatals from those earlier-priority callbacks are missed. Hooking the probe handler earlier would require splitting it out of `load_features()`. | ||
| - Other active plugins are live during the probe, so cross-plugin conflicts CAN surface (a full SHORTINIT sandbox would avoid that, but isn't portable here). | ||
|
|
||
| ## Update flow (syntax-only) | ||
|
|
||
| `update-guard.php` hooks `upgrader_source_selection` after WP extracts the install/update zip and before it copies files over the live plugin. It tokenizes every `.php` in the source with `token_get_all(…, TOKEN_PARSE)`. If any file fails to parse, it returns a `WP_Error` whose message names the first parse error and whose `$data['errors']` array carries the full list, aborting the operation without touching the live files. | ||
|
|
||
| The scan has an 8-second wall-clock budget (`PCG_UPDATE_GUARD_BUDGET_SECONDS`). Big packages (WooCommerce, Yoast, etc.) can have thousands of PHP files and we'd rather not blow the cron / request timeout. On bail with no errors found we don't fail-closed — we let the install/update through and `error_log` the slug + action so we can see how often this fires and on which packages. | ||
|
|
||
| Loaded unconditionally (not gated on `is_admin()`) so cron auto-updates also hit the gate. | ||
|
|
||
| Why not the load probe at this stage: during an *update* the active version is already loaded in the probe request, so `require`-ing the new main file would always fatal with "Cannot redeclare class/function". Parse errors are the high-frequency release failure mode; runtime errors still trip on the next Activate click. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| <?php | ||
| /** | ||
| * Activation guard — blocks plugin activations that fail a pre-flight | ||
| * load probe, so a bad Activate click can't fatal the site. | ||
| * | ||
| * @package automattic/jetpack-mu-wpcom | ||
| */ | ||
|
|
||
| add_action( 'load-plugins.php', 'pcg_guard_maybe_block_activation', 0 ); | ||
| add_action( 'load-update.php', 'pcg_guard_maybe_block_activation', 0 ); | ||
| add_action( 'admin_notices', 'pcg_guard_render_block_notice' ); | ||
|
|
||
| /** | ||
| * Entry point on `load-plugins.php` / `load-update.php`. Probes each | ||
| * plugin being activated and redirects with a notice on any failure. | ||
| */ | ||
| function pcg_guard_maybe_block_activation() { | ||
| if ( ! apply_filters( 'pcg_guard_activation', false ) ) { | ||
| return; | ||
| } | ||
| if ( ! current_user_can( 'activate_plugins' ) ) { | ||
| return; | ||
| } | ||
|
|
||
| // Bulk-action submissions from the bottom dropdown send `action=-1` | ||
| // and the real action in `action2`. | ||
| $action = sanitize_text_field( wp_unslash( $_REQUEST['action'] ?? '' ) ); | ||
| if ( '' === $action || '-1' === $action ) { | ||
| $action = sanitize_text_field( wp_unslash( $_REQUEST['action2'] ?? '' ) ); | ||
| } | ||
| if ( ! in_array( $action, array( 'activate', 'activate-plugin', 'activate-selected' ), true ) ) { | ||
| return; | ||
| } | ||
|
|
||
| if ( 'activate-selected' === $action ) { | ||
| $bulk_raw = is_array( $_REQUEST['checked'] ?? null ) ? (array) wp_unslash( $_REQUEST['checked'] ) : array(); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- each entry is sanitized below. | ||
| $plugins_to_check = array_values( | ||
| array_filter( | ||
| array_map( static fn( $b ) => sanitize_text_field( (string) $b ), $bulk_raw ) | ||
| ) | ||
| ); | ||
| $nonce_action = 'bulk-plugins'; | ||
| } else { | ||
| // Single-plugin path (plugins.php Activate link / update.php post-upload link). | ||
| $plugin = sanitize_text_field( wp_unslash( $_REQUEST['plugin'] ?? '' ) ); | ||
| $plugins_to_check = '' !== $plugin ? array( $plugin ) : array(); | ||
| $nonce_action = 'activate-plugin_' . $plugin; | ||
| } | ||
| if ( empty( $plugins_to_check ) ) { | ||
| return; | ||
| } | ||
|
|
||
| // Verify the nonce up front so we don't run probes for a request core | ||
| // will reject anyway. check_admin_referer() die()s on a bad nonce, so | ||
| // we don't need to check its return value. | ||
| if ( ! isset( $_REQUEST['_wpnonce'] ) ) { | ||
| return; | ||
| } | ||
| check_admin_referer( $nonce_action ); | ||
|
|
||
| $blocked = pcg_guard_evaluate_plugins( $plugins_to_check ); | ||
| if ( empty( $blocked ) ) { | ||
| return; | ||
| } | ||
|
|
||
| set_transient( | ||
| 'pcg_guard_notice_' . get_current_user_id(), | ||
| $blocked, | ||
| MINUTE_IN_SECONDS | ||
| ); | ||
|
|
||
| wp_safe_redirect( self_admin_url( 'plugins.php?pcg_blocked=1' ) ); | ||
| exit; | ||
| } | ||
|
|
||
| /** | ||
| * Probe each plugin; return map of basename => reason for those that failed. | ||
| * | ||
| * @param string[] $plugins Plugin basenames (e.g. "akismet/akismet.php"). | ||
| * @return array<string,string> | ||
| */ | ||
| function pcg_guard_evaluate_plugins( $plugins ) { | ||
| $blocked = array(); | ||
| $tester = new PCG_Load_Tester(); | ||
|
|
||
| foreach ( $plugins as $plugin ) { | ||
|
Contributor
There was a problem hiding this comment. There's also performance consideration here that doesn't scale well with multiple plugins (2 probes per plugin), but we can follow up later.
Contributor
Author
There was a problem hiding this comment. Acknowledged — memo'd as a follow-up. Batching all probes for a bulk activation into a single |
||
| if ( 0 !== validate_file( $plugin ) ) { | ||
| continue; | ||
| } | ||
| if ( is_plugin_active( $plugin ) ) { | ||
| continue; | ||
| } | ||
| $path = WP_PLUGIN_DIR . '/' . ltrim( $plugin, '/' ); | ||
| 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 ); | ||
| } | ||
| } | ||
|
|
||
| return $blocked; | ||
| } | ||
|
|
||
| /** | ||
| * Build a human-readable sentence describing the captured fatal, e.g. | ||
| * "PCG fatal (in pcg-fatal-tester.php, line 6)." for the admin notice. | ||
| * | ||
| * @param array $result Probe result from PCG_Load_Tester::test(). | ||
| * @return string | ||
| */ | ||
| function pcg_guard_format_block_reason( $result ) { | ||
| $message = trim( (string) ( $result['message'] ?? '' ) ); | ||
|
|
||
| $where = ''; | ||
| if ( ! empty( $result['file'] ) ) { | ||
| $file = basename( (string) $result['file'] ); | ||
| $line = (int) ( $result['line'] ?? 0 ); | ||
| $where = $line > 0 | ||
| ? sprintf( | ||
| /* translators: location fragment, e.g. "in plugin.php, line 42". 1: file name, 2: line number. */ | ||
| __( 'in %1$s, line %2$d', 'jetpack-mu-wpcom' ), | ||
| $file, | ||
| $line | ||
| ) | ||
| : sprintf( | ||
| /* translators: location fragment without a line number, e.g. "in plugin.php". %s: file name. */ | ||
| __( 'in %s', 'jetpack-mu-wpcom' ), | ||
| $file | ||
| ); | ||
| } | ||
|
|
||
| if ( '' !== $message ) { | ||
| return '' !== $where ? sprintf( '%s (%s).', $message, $where ) : $message . '.'; | ||
| } | ||
| if ( '' !== $where ) { | ||
| return sprintf( | ||
| /* translators: %s: location fragment from the strings above, which already begins with "in". */ | ||
| __( 'A fatal PHP error was detected %s.', 'jetpack-mu-wpcom' ), | ||
| $where | ||
| ); | ||
| } | ||
| return __( 'A fatal PHP error was detected.', 'jetpack-mu-wpcom' ); | ||
| } | ||
|
|
||
| /** | ||
| * Render the admin notice. Messages are pulled from a per-user transient | ||
| * set by the guard before the redirect. | ||
| */ | ||
| function pcg_guard_render_block_notice() { | ||
| if ( empty( $_GET['pcg_blocked'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- read-only flag for rendering a flash notice. | ||
| return; | ||
| } | ||
| $key = 'pcg_guard_notice_' . get_current_user_id(); | ||
| $messages = get_transient( $key ); | ||
| delete_transient( $key ); | ||
|
|
||
| if ( ! is_array( $messages ) || empty( $messages ) ) { | ||
| return; | ||
| } | ||
| ?> | ||
| <div class="notice notice-error"> | ||
| <p><strong><?php esc_html_e( 'WordPress.com blocked activation because the pre-flight check detected a fatal:', 'jetpack-mu-wpcom' ); ?></strong></p> | ||
| <ul style="list-style:disc;padding-inline-start:24px;"> | ||
| <?php foreach ( $messages as $plugin => $reason ) : ?> | ||
| <li><code><?php echo esc_html( $plugin ); ?></code> — <?php echo esc_html( $reason ); ?></li> | ||
| <?php endforeach; ?> | ||
| </ul> | ||
| <p><?php esc_html_e( 'The plugin was not activated. Investigate the error before trying again.', 'jetpack-mu-wpcom' ); ?></p> | ||
| </div> | ||
| <?php | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.