Plugin Conflicts Guardian: log blocked activations, blocked updates, and post-update rollbacks to logstash #48565
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: patch | ||
| Type: added | ||
|
|
||
| Plugin Conflicts Guardian: emit logstash events when the guard refuses or recovers from a bad change — `Activation blocked` (refused activation), `Update blocked` (refused install/update with a parse error), and `Update rolled back` (post-update fatal triggered a rollback). All three share the `plugin-conflicts-guardian` feature bucket so the full PCG-block surface can be measured from one 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
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,69 @@ | ||
| <?php | ||
| /** | ||
| * Shared logstash dispatch for the Plugin Conflicts Guardian feature. | ||
| * | ||
| * @package automattic/jetpack-mu-wpcom | ||
| */ | ||
|
|
||
| /** | ||
| * Emit an event to the `plugin-conflicts-guardian` logstash bucket. | ||
| * | ||
| * Best-effort: a logging failure must never escalate into a fatal, | ||
| * since callers run on activation / install / update request paths. | ||
| * No-op outside WordPress.com (no `log2logstash` available). | ||
| * | ||
| * @param string $message Event message slug (e.g. "Activation blocked"). | ||
| * @param array $extra Event-specific properties; JSON-encoded into the `extra` field. | ||
| * @return void | ||
| */ | ||
| function pcg_log_event( $message, array $extra ) { | ||
|
Contributor
There was a problem hiding this comment. Since this is best-effort , should we wrap it in a try/catch block? That way we prevent exceptions here blocking the rest of the flows.
Contributor
Author
There was a problem hiding this comment. Done in 8ceac93 — wrapped the whole dispatcher in try/catch with a fail-open empty handler. The require + json_encode + log2logstash chain now can't escalate into a fatal on the activation / install / update request paths. |
||
| try { | ||
| 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' => (string) $message, | ||
| 'extra' => wp_json_encode( pcg_log_redact_paths( $extra ), JSON_UNESCAPED_SLASHES ), | ||
| ) | ||
| ); | ||
| } catch ( \Throwable $e ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedCatch -- best-effort: a logging failure must not escalate on activation / install / update request paths. | ||
| unset( $e ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Recursively replace `ABSPATH` and `WP_CONTENT_DIR` prefixes inside string | ||
| * values with `.../` so log lines don't leak the install layout. Keeps the | ||
| * relative tail (`plugins/foo/bar.php`), which is the useful part for triage. | ||
| * | ||
| * @param mixed $value Scalar or array. | ||
| * @return mixed | ||
| */ | ||
| function pcg_log_redact_paths( $value ) { | ||
| if ( is_array( $value ) ) { | ||
| return array_map( 'pcg_log_redact_paths', $value ); | ||
| } | ||
| if ( ! is_string( $value ) || '' === $value ) { | ||
| return $value; | ||
| } | ||
| // WP_CONTENT_DIR first — it's a longer prefix that's typically *under* | ||
| // ABSPATH on standard installs, so swapping ABSPATH first would shadow it. | ||
| $replacements = array(); | ||
| if ( defined( 'WP_CONTENT_DIR' ) && '' !== WP_CONTENT_DIR ) { | ||
| $replacements[ rtrim( WP_CONTENT_DIR, '/' ) . '/' ] = '.../'; | ||
| } | ||
| if ( defined( 'ABSPATH' ) && '' !== ABSPATH ) { | ||
| $replacements[ rtrim( ABSPATH, '/' ) . '/' ] = '.../'; | ||
| } | ||
| if ( empty( $replacements ) ) { | ||
| return $value; | ||
| } | ||
| return strtr( $value, $replacements ); | ||
| } | ||
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reasonmight also leak absolute paths if it's just raw PHP error text 😅Since this is log, it should be fine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, fine for log purposes —
reasonis the raw probe verdict (PHP fatal/parse-error message). It can carry an absolute path, but everything stays in our internal logstash bucket; no PII, no exposure to the site owner. We accept that as the cost of having an actually usefulreasonfield for triage. Thefilefield stays sanitized to basename only because that one's structured/queried, so worth keeping consistent.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually went ahead and sanitized in ef3e3ca — added
pcg_log_redact_paths()that walks theextrapayload before JSON-encoding and rewrites anyABSPATH/WP_CONTENT_DIRprefix to.../. Keeps the relative tail (plugins/foo/bar.php) which is the useful part for triage, drops the install-layout leak. Applied centrally inpcg_log_event()so all events (including the pre-existingProbe transport error) get it for free.