Merge pull request #15 from kristianwind/repo-rename

Rename the repository to woo-discount-manager, keep the installed slug

README.txt

@@ -4,7 +4,7 @@ Tags: woocommerce, discount, email, shipping, coupon
 Requires at least: 5.8
 Tested up to: 7.1
 Requires PHP: 7.4
-Stable tag: 1.4.4
+Stable tag: 1.4.6
 WC requires at least: 6.0
 WC tested up to: 11.1.0
 License: GPLv2 or later
@@ -96,6 +96,13 @@ The most specific match wins: exact email, then domain, then partial match.
 
 == Changelog ==
 
+= 1.4.6 =
+* Housekeeping: the plugin's source moved to a shorter address. Updates continue to arrive as before, and nothing needs doing.
+
+= 1.4.5 =
+* Fixed: the plugin could offer its own release as an update to a different plugin. Plugins that update from the same place share one check inside WordPress, and this one answered for all of them instead of only for itself. If another such plugin is installed, update it as well.
+* "Vis detaljer" next to the update notice now opens the plugin's own details, with the release notes as the changelog. It previously showed "Plugin not found", because WordPress was asking wordpress.org about a plugin that is not published there.
+
 = 1.4.4 =
 * A new version now shows up within about fifteen minutes instead of being hidden behind a six-hour cache. The plugin was holding on to its own answer for longer than WordPress waits before asking again, so a release could be published and the shop shown nothing.
 

ci/plugin.conf

@@ -7,6 +7,13 @@
 # Directory name the plugin gets inside wp-content/plugins, and the name of the
 # built zip. WordPress identifies a plugin by "<slug>/<main file>", so changing
 # this after release orphans every installed copy.
+#
+# 🔴 THIS DELIBERATELY DOES NOT MATCH THE REPOSITORY NAME, and it must not be
+# "tidied up" to. The repo is woo-discount-manager; the installed directory has
+# been woocommerce-email-discount-manager since the first release and is on a
+# live shop. Renaming it would not move that installation — it would put a
+# SECOND copy beside it and leave the old one active. The repo name is free to
+# change (Gitea keeps a 301, measured), the installed slug never is.
 PLUGIN_SLUG=woocommerce-email-discount-manager
 
 # The file carrying the plugin header. Version: in here is the version.

ci/static.sh

@@ -143,6 +143,23 @@ for readme in readme.txt README.txt; do
     || note_fail "$readme has 'Stable tag: $stable' but the plugin header says $version."
 done
 
+ci_step "no secrets in tracked files"
+# Suggested by the AI-manager port, and it costs nothing: a pushed key cannot be
+# unpushed. It has to be rotated, and the window between the push and noticing
+# is the whole problem.
+#
+# Deliberately narrow. Broad entropy heuristics fire on minified assets, hashes
+# and base64 images, and a check that cries wolf gets commented out — which is
+# worse than not having it. These are prefixes that mean one thing only.
+before=$fails
+while IFS= read -r -d '' f; do
+  [ -f "$f" ] || continue
+  if LC_ALL=C grep -nE '(sk-ant-[A-Za-z0-9_-]{16,}|sk-[A-Za-z0-9]{32,}|ghp_[A-Za-z0-9]{30,}|gho_[A-Za-z0-9]{30,}|github_pat_[A-Za-z0-9_]{50,}|AKIA[0-9A-Z]{16}|-----BEGIN [A-Z ]*PRIVATE KEY-----)' "$f"; then
+    note_fail "$f looks like it contains a credential. It cannot be unpushed — rotate it."
+  fi
+done < <(git ls-files -z)
+[ "$fails" = "$before" ] && echo "  nothing that looks like a key or a private key"
+
 ci_step "no placeholders left in the plugin header"
 # The header is the plugin's business card: wp-admin shows the author and the
 # links to whoever installs it, so a placeholder there survives for as long as

ci/update-smoke.sh

@@ -92,13 +92,39 @@ echo "  status:               $status"
 [ "$now" = "$expected" ] || { echo "Expected $expected after the update." >&2; exit 1; }
 [ "$status" = active ] || { echo "The plugin is no longer active after updating itself." >&2; exit 1; }
 
-# A zip whose top-level directory is not the slug installs a SECOND copy
-# beside the first instead of replacing it, and the old one stays active.
-count=$(docker exec "$id-cli" sh -c 'ls /var/www/html/wp-content/plugins | wc -l')
+ci_step "one copy of the plugin, not two"
+# A zip whose top-level directory is not the slug installs a SECOND copy beside
+# the first instead of replacing it, and the old one stays active. This used to
+# count the directories into a variable and then never look at it — a check
+# that could not fail, sitting under a comment explaining exactly what it was
+# for.
 docker exec "$id-cli" ls /var/www/html/wp-content/plugins | sed 's/^/  /'
+copies=$(docker exec "$id-cli" sh -c \
+  "ls -d /var/www/html/wp-content/plugins/$PLUGIN_SLUG* 2>/dev/null | wc -l" | tr -d '[:space:]')
+echo "  directories starting with $PLUGIN_SLUG: $copies"
+[ "$copies" = 1 ] || {
+  echo "Expected exactly one, got $copies. The zip unpacked to a directory that is not the slug." >&2
+  exit 1
+}
+
+ci_step "the code that is now running reports the new version"
+# Files can be replaced while the running code still reports the old version —
+# an opcache that was not invalidated, or a constant defined somewhere else.
+# The constant name is read out of the plugin header rather than hard-coded:
+# this script is copied between plugins, and a name that does not exist over
+# there prints an empty value and carries on cheerfully.
+const=$(sed -n -E "s/^[[:space:]]*define\([[:space:]]*'([A-Z_]*VERSION)'.*/\1/p" "$PLUGIN_MAIN" | head -1)
+if [ -n "$const" ]; then
+  running=$(wp eval "echo defined( '$const' ) ? $const : '';" | tr -d '[:space:]')
+  echo "  $const = ${running:-<undefined>}"
+  [ "$running" = "$expected" ] || {
+    echo "The files say $expected but the running code says '${running:-undefined}'." >&2
+    exit 1
+  }
+else
+  echo "  $PLUGIN_MAIN defines no *_VERSION constant — nothing to cross-check."
+fi
 
-ci_step "and it still works afterwards"
-wp eval 'echo WC_EDM_VERSION;' | sed 's/^/  WC_EDM_VERSION: /'
 wp plugin list --name="$PLUGIN_SLUG" --field=update | grep -qx none \
   && echo "  WordPress no longer offers an update — the header and the feed agree."
 

docs/REUSE.md

@@ -126,6 +126,79 @@ measures something else. It cost two rounds here, once on
 If a script reports something you just fixed as still missing, `git status`
 before you debug the script.
 
+## Make a check fail once before you believe it
+
+Three checks in this directory turned out to be incapable of failing, and all
+three sat under a comment explaining exactly what they were for:
+
+- `tests/smoke.php` printed `FAIL` and exited 0 for four releases.
+- `ci/static.sh`'s PHP lint passed while linting an empty directory.
+- `ci/update-smoke.sh` counted the plugin directories into a variable and never
+  looked at it — under a comment about a zip installing a second copy.
+
+None of them were caught by reading the code, and a green build is exactly what
+a broken check looks like. So: break it on purpose once, watch it go red, then
+put it back. It costs one run.
+
+## The bug you cannot find by reading your own repo
+
+🔴 **`update_plugins_{$hostname}` is keyed on the hostname, not the
+repository.** Every plugin whose `Update URI` points at the same host shares one
+hook, and WordPress runs **every** callback on it for **every** one of those
+plugins:
+
+```php
+// wp-includes/update.php
+$update = apply_filters( "update_plugins_{$hostname}", false, $plugin_data, $plugin_file, $locales );
+```
+
+Answer unconditionally and you offer your release to your neighbour. Measured 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 which plugin gets
+corrupted is arbitrary — and pointed at one with auto-updates enabled, it
+installs a stranger's zip over itself, unattended, on a shop taking money.
+
+```php
+add_filter( "update_plugins_{$host}", array( $this, 'offer_update' ), 10, 3 );
+
+public function offer_update( $update, $plugin_data, $plugin_file = '' ) {
+    if ( '' !== $plugin_file && plugin_basename( SELF_FILE ) !== $plugin_file ) {
+        return $update;
+    }
+```
+
+Both halves. Leave the accepted-argument count at 2 and the third argument never
+arrives, so the guard compares against an empty string and does nothing — the
+same shape of bug it exists to fix. `plugins_api` is safe if it guards on
+`$args->slug`.
+
+**No integration test can find this.** A site test installs one plugin from that
+host, so the collision cannot arise there. The assertion is the entire defence:
+
+```php
+wc_edm_ok( 'another plugin on the same host is not offered our release',
+    false === $updater->offer_update( false, $data, 'someone-else/someone-else.php' ) );
+```
+
+The same shape applies to anything else shared between sibling plugins. A
+`shared-*.php` file copied byte-identically into five plugins fatals with
+`Cannot redeclare` on the second activation unless everything in it is wrapped
+in `function_exists()` / `class_exists()` / `defined()` — and then the copy that
+loads first wins, which is alphabetical order, which is arbitrary. Freeze the
+contract rather than versioning the file.
+
+## The repository name and the installed slug are different things
+
+The repo may be renamed freely — Gitea keeps a 301 on both the API and the deep
+release-download paths (measured), and WordPress follows up to 5 redirects by
+default, so installed copies keep updating through the old `Update URI`.
+
+The **installed** slug can never change. It is `<slug>/<main file>` in
+`wp-content/plugins`, and a new one installs a second copy beside the first
+while the old one stays active. So `PLUGIN_SLUG` in `ci/plugin.conf` is allowed
+to disagree with the repository name forever, and if it does, say so in the file
+or somebody will helpfully align them.
+
 ## Two guards worth understanding before you move them
 
 **`.gitea/workflows/` hides `.github/workflows/` completely.** Gitea uses the
@@ -144,9 +217,17 @@ the *last* id in the payload, and a release object carries an embedded
 `author` and, once anything is attached, an `assets` array — each with an id of
 its own. `ci/publish-release.sh` uses a four-line `json_field()` over `python3`
 instead. Measured here: the greedy pattern returns the asset ids 1/2/3/4 for
-releases whose real ids are 416/417/419/421. Four releases were published
-correctly anyway, purely because this instance's author id is **-1** and the
-minus sign stops `[0-9]+` from matching. That is not a safety margin.
+releases whose real ids are 416/417/419/421.
+
+Four releases were published correctly anyway, and the reason is **who
+published them, not which instance**: a release created by the Actions job
+token has the ghost user as its author (`"id":-1`), and the minus sign stops
+`[0-9]+` from matching. A release created by a real user — a cross-repo PAT, or
+one published by hand — has `"id":1` and the greedy match lands on it. Do not
+read the `-1` as a safety margin; it lasts until the first release somebody
+publishes by hand. And on a re-run the lookup fetches a response that *has*
+assets, where the last id is an asset id: positive, on any instance, with any
+token.
 
 **A forge's own download URL is not the address your users can reach.**
 `browser_download_url` is built from the forge's configured root URL;

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
@@ -72,6 +72,9 @@ class WC_EDM_Updater {
 
 		// And "Check again" has to actually check again.
 		add_action( 'admin_init', array( $this, 'forget_on_force_check' ) );
+
+		// "View version details" next to the update notice.
+		add_filter( 'plugins_api', array( $this, 'plugin_details' ), 10, 3 );
 	}
 
 	/**
@@ -123,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 ) {
@@ -155,6 +175,58 @@ class WC_EDM_Updater {
 		);
 	}
 
+	/**
+	 * Answer the "View version details" modal.
+	 *
+	 * WordPress builds that link as
+	 * plugin-install.php?tab=plugin-information&plugin=<slug>, which calls
+	 * plugins_api() — and plugins_api() asks **wordpress.org**. This plugin is
+	 * not there, so without this filter the link opens a modal containing an
+	 * error, on the same screen that is otherwise telling the shop owner an
+	 * update is ready. Nothing warns you: the link looks like every other
+	 * plugin's.
+	 *
+	 * @param false|object|array $result Response so far.
+	 * @param string             $action Requested action.
+	 * @param object             $args   Request arguments.
+	 * @return false|object|array
+	 */
+	public function plugin_details( $result, $action, $args ) {
+		$slug = dirname( plugin_basename( WC_EDM_PLUGIN_FILE ) );
+
+		if ( 'plugin_information' !== $action || empty( $args->slug ) || $args->slug !== $slug ) {
+			return $result;
+		}
+
+		require_once ABSPATH . 'wp-admin/includes/plugin.php';
+		$plugin  = get_plugin_data( WC_EDM_PLUGIN_FILE, false, false );
+		$release = $this->latest_release();
+
+		$notes = $release && '' !== $release['notes'] ? $release['notes'] : '';
+
+		return (object) array(
+			'name'          => $plugin['Name'],
+			'slug'          => $slug,
+			'version'       => $release ? $release['version'] : $plugin['Version'],
+			'author'        => $plugin['Author'],
+			'author_profile'=> $plugin['AuthorURI'],
+			'homepage'      => $plugin['PluginURI'],
+			'requires'      => $plugin['RequiresWP'],
+			'requires_php'  => $plugin['RequiresPHP'],
+			'tested'        => $release ? $release['tested'] : '',
+			'download_link' => $release ? $release['package'] : '',
+			'icons'         => array( 'svg' => WC_EDM_PLUGIN_URL . 'assets/icon.svg' ),
+			'sections'      => array(
+				'description' => wpautop( esc_html( $plugin['Description'] ) ),
+				// Plain text from the release notes, not markdown: rendering it
+				// as HTML would mean trusting whatever is in a release body.
+				'changelog'   => '' !== $notes
+					? wpautop( esc_html( $notes ) )
+					: wpautop( esc_html__( 'Release notes are published with each release.', 'wc-email-discount' ) ),
+			),
+		);
+	}
+
 	/**
 	 * Drop the cached feed.
 	 */
@@ -288,6 +360,9 @@ class WC_EDM_Updater {
 			'package' => $base . '/releases/download/' . rawurlencode( $tag ) . '/' . rawurlencode( $zip ),
 			'url'     => $base . '/releases/tag/' . rawurlencode( $tag ),
 			'tested'  => $tested,
+			// The release notes are the changelog. Kept so the "View version
+			// details" modal has something true to show.
+			'notes'   => isset( $body['body'] ) ? (string) $body['body'] : '',
 		);
 	}
 

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,79 @@ 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=<slug>, which calls
+// plugins_api() — and plugins_api() asks wordpress.org, where this plugin does
+// not exist. Unanswered, that link opens a modal reading "Plugin not found."
+// on the same screen that is telling the shop owner an update is ready.
+// Measured on a live shop before this was written.
+set_transient(
+	WC_EDM_Updater::TRANSIENT,
+	array(
+		'version' => '99.0.0',
+		'package' => 'https://gitea.nolimit.dk/kw/demo/releases/download/v99.0.0/demo.zip',
+		'url'     => 'https://gitea.nolimit.dk/kw/demo',
+		'tested'  => '7.1',
+		'notes'   => "v99.0.0 — a headline\n\nA line of release notes.",
+	),
+	600
+);
+
+// plugins_api() lives in the admin includes, which a WP-CLI context does not
+// load. wp-admin has it; this does not.
+require_once ABSPATH . 'wp-admin/includes/plugin-install.php';
+
+$wc_edm_slug = dirname( plugin_basename( WC_EDM_PLUGIN_FILE ) );
+$details     = plugins_api( 'plugin_information', array( 'slug' => $wc_edm_slug ) );
+
+wc_edm_ok(
+	'the details modal is answered here, not by wordpress.org',
+	! is_wp_error( $details ),
+	is_wp_error( $details ) ? $details->get_error_message() : ''
+);
+wc_edm_is( 'and it is about this plugin', $wc_edm_slug, is_wp_error( $details ) ? null : $details->slug );
+wc_edm_is( 'it reports the version from the feed', '99.0.0', is_wp_error( $details ) ? null : $details->version );
+wc_edm_ok(
+	'the changelog is the release notes',
+	! is_wp_error( $details ) && false !== strpos( $details->sections['changelog'], 'A line of release notes' ),
+	'a modal with an empty changelog is barely better than an error'
+);
+wc_edm_ok(
+	'and the notes are escaped rather than rendered',
+	! is_wp_error( $details ) && false === strpos( $details->sections['changelog'], '<script' ),
+	'a release body is not a template language'
+);
+wc_edm_ok(
+	'another plugin\'s details request is left alone',
+	false === wc_edm()->updater->plugin_details( false, 'plugin_information', (object) array( 'slug' => 'akismet' ) ),
+	'answering for somebody else would replace their modal with ours'
+);
+
+delete_transient( WC_EDM_Updater::TRANSIENT );
+
 /*
  * The "Enable auto-updates" link on the Plugins screen.
  *

woocommerce-email-discount-manager.php

@@ -1,10 +1,10 @@
 <?php
 /**
  * Plugin Name: WooCommerce Email Discount Manager
- * Plugin URI:  https://gitea.nolimit.dk/kw/woocommerce-email-discount-manager
- * Update URI:  https://gitea.nolimit.dk/kw/woocommerce-email-discount-manager
+ * Plugin URI:  https://gitea.nolimit.dk/kw/woo-discount-manager
+ * Update URI:  https://gitea.nolimit.dk/kw/woo-discount-manager
  * Description: Apply discounts and show/hide shipping methods based on the customer's email address (full address, domain or partial match).
- * Version:     1.4.4
+ * Version:     1.4.6
  * Author:      Kristian Wind
  * Author URI:  https://gitea.nolimit.dk/kw
  * License:     GPL-2.0-or-later
@@ -28,7 +28,7 @@ if ( ! defined( 'ABSPATH' ) ) {
 	exit;
 }
 
-define( 'WC_EDM_VERSION', '1.4.4' );
+define( 'WC_EDM_VERSION', '1.4.6' );
 define( 'WC_EDM_PLUGIN_FILE', __FILE__ );
 define( 'WC_EDM_PLUGIN_DIR', plugin_dir_path( __FILE__ ) );
 define( 'WC_EDM_PLUGIN_URL', plugin_dir_url( __FILE__ ) );