From b5b4bc2e03909ebd3f391520378033f248627170 Mon Sep 17 00:00:00 2001 From: kristianwind Date: Wed, 16 Sep 2026 14:54:08 +0200 Subject: [PATCH] Answer only for our own plugin, not every plugin on the host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🔴 Live-shop bug, found by the subscriptions session with a screenshot. `update_plugins_{$hostname}` is keyed on the HOSTNAME, not the repository. Every plugin updating from gitea.nolimit.dk shares the single hook `update_plugins_gitea.nolimit.dk`, and WordPress runs every callback on it for every one of those plugins: $update = apply_filters( "update_plugins_{$hostname}", false, $plugin_data, $plugin_file, $locales ); This updater registered with `10, 2` — so it never received $plugin_file — and returned its own release unconditionally. Result on a live shop: a subscriptions plugin was shown this plugin's 1.4.4 and told it was an update for itself. Load order decides which callback wins, so the direction it breaks in is arbitrary: a rename, a different folder name, a plugins_loaded priority change, and it flips. If it flips this way round, a plugin with auto-updates enabled installs somebody else's zip over itself, unattended, on a shop taking money. Both halves are the fix. `10, 3` so the argument arrives, and a check that the file WordPress is asking about is ours. With the count left at 2 the guard would compare against an empty string and silently do nothing — the same shape of bug it exists to fix. tests/smoke.php now calls offer_update() with a foreign plugin file and requires false back. The site test installs exactly one plugin from this host, so the collision cannot arise there on its own: that assertion is the defence. Co-Authored-By: Claude Opus 5 (1M context) --- includes/class-updater.php | 21 +++++++++++++++++++-- tests/smoke.php | 24 ++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/includes/class-updater.php b/includes/class-updater.php index e9c4d8e..e6cd9ca 100644 --- a/includes/class-updater.php +++ b/includes/class-updater.php @@ -63,7 +63,7 @@ class WC_EDM_Updater { return; } - add_filter( "update_plugins_{$host}", array( $this, 'offer_update' ), 10, 2 ); + add_filter( "update_plugins_{$host}", array( $this, 'offer_update' ), 10, 3 ); // Without this the old answer is served for up to six hours after an // update, and the plugins screen keeps offering a version already @@ -126,9 +126,26 @@ class WC_EDM_Updater { * * @param array|false $update Update offered so far. * @param array $plugin_data Headers of the installed plugin. + * @param string $plugin_file Which plugin WordPress is asking about. * @return array|false */ - public function offer_update( $update, $plugin_data ) { + public function offer_update( $update, $plugin_data, $plugin_file = '' ) { + // The filter name is built from the HOSTNAME, not the repository: + // every plugin that updates from gitea.nolimit.dk shares the one hook + // `update_plugins_gitea.nolimit.dk`, and WordPress runs every callback + // on it for every one of those plugins. Without this check a plugin + // answers for its neighbour and offers it its own release — measured + // on a live shop, where a subscriptions plugin was offered this one's + // 1.4.4 and told it was an update for itself. + // + // The `10, 3` on the add_filter above is half of this fix. With the + // accepted-argument count left at 2 the third argument never arrives, + // $plugin_file stays empty, and the guard silently does nothing — the + // same shape of bug it exists to fix. + if ( '' !== $plugin_file && plugin_basename( WC_EDM_PLUGIN_FILE ) !== $plugin_file ) { + return $update; + } + $release = $this->latest_release(); if ( ! $release ) { diff --git a/tests/smoke.php b/tests/smoke.php index ca8903a..b25a339 100644 --- a/tests/smoke.php +++ b/tests/smoke.php @@ -305,7 +305,7 @@ set_transient( 60 ); -$offered = wc_edm()->updater->offer_update( false, get_plugin_data( WC_EDM_PLUGIN_FILE, false, false ) ); +$offered = wc_edm()->updater->offer_update( false, get_plugin_data( WC_EDM_PLUGIN_FILE, false, false ), plugin_basename( WC_EDM_PLUGIN_FILE ) ); wc_edm_is( 'a newer release is offered as an update', '99.0.0', is_array( $offered ) ? $offered['version'] : null ); wc_edm_is( 'and it carries the folder name WordPress will unpack into', @@ -344,9 +344,29 @@ set_transient( ), 60 ); -$offered = wc_edm()->updater->offer_update( false, get_plugin_data( WC_EDM_PLUGIN_FILE, false, false ) ); +$offered = wc_edm()->updater->offer_update( false, get_plugin_data( WC_EDM_PLUGIN_FILE, false, false ), plugin_basename( WC_EDM_PLUGIN_FILE ) ); wc_edm_is( 'the current version is answered too, not passed over', WC_EDM_VERSION, is_array( $offered ) ? $offered['version'] : null ); +// 🔴 The filter name is built from the HOSTNAME, not the repository. Every +// plugin updating from gitea.nolimit.dk shares the hook +// update_plugins_gitea.nolimit.dk, and WordPress runs every callback on it for +// every one of those plugins. Answering unconditionally means offering the +// neighbour your own release — which happened on a live shop: a subscriptions +// plugin was shown this one's 1.4.4 and told it was an update for itself. Load +// order decides who wins, so the direction it breaks in is arbitrary. +// +// The site test installs exactly one plugin from this host, so the collision +// can never occur here. This assertion is the whole defence. +wc_edm_ok( + 'another plugin on the same host is not offered our release', + false === wc_edm()->updater->offer_update( + false, + get_plugin_data( WC_EDM_PLUGIN_FILE, false, false ), + 'kw-subscriptions/kw-subscriptions.php' + ), + 'we answered for a plugin that is not ours' +); + // The "View version details" link next to the update notice. WordPress builds // it as plugin-install.php?tab=plugin-information&plugin=, which calls // plugins_api() — and plugins_api() asks wordpress.org, where this plugin does